Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes to get kwin_wayland starting with Lima #51

Closed
wants to merge 307 commits into from

Conversation

shadeslayer
Copy link

No description provided.

yuq added 30 commits March 29, 2018 09:28
This is needed for lima gp compiler.

Signed-off-by: Qiang Yu <[email protected]>
Signed-off-by: Qiang Yu <[email protected]>
Signed-off-by: Erico Nunes <[email protected]>
Reviewed-by: Qiang Yu <[email protected]>
v2:
  (Qiang Yu)
  no need to record draw info in lima_context

Signed-off-by: Erico Nunes <[email protected]>
Signed-off-by: Qiang Yu <[email protected]>
No other place except mesa need the libdrm_lima, so we can just
move it into mesa so that no need to maintain the libdrm ABI
backward compatibility and reduce some overhead.

Advised-by: Rob Clark <[email protected]>
Signed-off-by: Qiang Yu <[email protected]>
enunes and others added 18 commits April 12, 2018 06:28
Implement 'gpir_op_lt' alu opcode from nir 'nir_op_slt'.

Tested on Mali400 on the Allwinner A20.

Signed-off-by: Erico Nunes <[email protected]>
Implement 'gpir_op_select' alu opcode from nir 'nir_op_bcsel'.

Tested on Mali400 on the Allwinner A20.

Signed-off-by: Erico Nunes <[email protected]>
ir_binop_gequal needs to be converted to nir_op_sge when native integers
are not supported in the driver.
Otherwise it becomes no different than ir_binop_less after the
conversion.

Signed-off-by: Erico Nunes <[email protected]>
Implement 'gpir_op_ge' alu opcode from nir 'nir_op_sge'.

Tested on Mali400 on the Allwinner A20.

Signed-off-by: Erico Nunes <[email protected]>
Implement 'gpir_op_floor' alu opcode from nir 'nir_op_ffloor'.

Tested on Mali400 on the Allwinner A20.

Signed-off-by: Erico Nunes <[email protected]>
Implement 'gpir_op_sign' alu opcode from nir 'nir_op_fsign'.

Tested on Mali400 on the Allwinner A20.

Signed-off-by: Erico Nunes <[email protected]>
mali400 uses on-chip tile buffer to store depth and stencil values,
so no separate buffer is needed if FBO doesn't have depth/stencil attachment

We still need to figure out how to render to depth/stencil buffer.

Signed-off-by: Vasily Khoruzhick <[email protected]>
Signed-off-by: Vasily Khoruzhick <[email protected]>
There are no 'equal' or 'not-equal' opcodes in gp.
eq (a == b) is lowered to and(a >= b, b >= a)
ne (a != b) is lowered to or(a < b, b < a)

Signed-off-by: Erico Nunes <[email protected]>
Support nir opcodes nir_op_fand and nir_op_for with gpir_op_min and
gpir_op_max.

Tested on Mali400 on the Allwinner A20.

Signed-off-by: Erico Nunes <[email protected]>
@@ -91,7 +91,7 @@ lima_update_tex_desc(struct lima_context *ctx, struct lima_sampler_state *sample

/* TODO: - do we need to align width/height to 16?
- does hardware support stride different from width? */
width = prsc->width0;
width = align(prsc->width0, 16);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's probably not a right place to do it. We should align resource stride when it's allocated.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the resource stride is already aligned when allocation, but the descriptor setup need a way to tell the width and stride difference. Texture UV space [0, 1] is mapped to pixel space [0, width], but texture is stored in memory in stride X height way. So we have to program in the descriptor both width and stride value. I can't see where current code reflect this, so we need descriptor changes for the width != stride case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuq I don't think that hardware supports that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

robclark said that it's pretty common limitation of hardware, so we have to workaround it in the driver.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, let's dump to see how mali handle this case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuq I suspect it does extra copy in this case. Anyway, I'm running buildroot rootfs with no X11 nor wayland, so it's a bit tricky for me to implement new tests - Mali doesn't support EGL_MESA_platform_gbm so neither of demos from your gfx repo work.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, if it does extra copy, it'll be way much slower.

Maybe I can modify the gbm-surface-fbo for X11 and go for a dump next week after I get back from vacation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EGL_KHR_platform_gbm is supported by recent malion e.g. rockchip, see https://github.com/xbmc/xbmc/pull/13746/files#diff-2b5179a78cb072ff496b13892efaa8b2R41 for a bit more info.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@koenkooi I still can't get it working. I replaced EGL_PLATFORM_GBM_MESA with EGL_PLATFORM_GBM_KHR and now it fails in eglCreatePlatformWindowSurfaceEXT() - it returns EGL_NO_SURFACE.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I had to replace /dev/dri/renderD128 with /dev/dri/card0

@@ -310,6 +310,10 @@ lima_surface_destroy(struct pipe_context *pctx, struct pipe_surface *psurf)

struct hash_entry *entry =
_mesa_hash_table_search(ctx->plb_pp_stream, &key);

if (!entry)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this happen? lima_surface_create should always create the entry.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure if I can provide a minimal test case for this, but I added this check due to a crash I experienced when trying to move Plasma's widget popups.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but this case may lead to a design bug, if so, fix here is not the root cause.

BTW. how about the kwin wayland status? Seems you got something onscreen like the Plasma's widget with mesa-lima?

Copy link
Author

@shadeslayer shadeslayer Apr 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ill spend some more time looking at this then.

Re plasmashell, that's correct Plasmashell renders fine (though the render width seems to be off). Launching apps seems to lead to corruption though. Something I hope to tackle next week.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By design, all surface is created by lima_surface_create and freed by lima_surface_destroy. And lima_surface_create should always create or reference count the entry. So if these two functions are called in balance, this case should not happen.

Nice to hear one desktop works (partially), it's a little surprise to me, thanks.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I understood too, I was equally surprised that this was not the case.

And yeah, hopefully together we can get all of it working soon :D

@@ -91,7 +91,7 @@ lima_update_tex_desc(struct lima_context *ctx, struct lima_sampler_state *sample

/* TODO: - do we need to align width/height to 16?
- does hardware support stride different from width? */
width = prsc->width0;
width = align(prsc->width0, 16);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the resource stride is already aligned when allocation, but the descriptor setup need a way to tell the width and stride difference. Texture UV space [0, 1] is mapped to pixel space [0, width], but texture is stored in memory in stride X height way. So we have to program in the descriptor both width and stride value. I can't see where current code reflect this, so we need descriptor changes for the width != stride case.

@yuq yuq force-pushed the lima-18.0 branch 2 times, most recently from ee7c7ee to ac95540 Compare May 8, 2018 01:49
@shadeslayer
Copy link
Author

Hey
Any chance you got that dump to help align stride and width? :D

@anarsoul
Copy link
Contributor

anarsoul commented May 9, 2018 via email

@shadeslayer
Copy link
Author

Oh where is the dump?

@anarsoul
Copy link
Contributor

anarsoul commented May 9, 2018 via email

@koenkooi
Copy link
Contributor

koenkooi commented May 9, 2018

#53

@shadeslayer
Copy link
Author

Aha, thanks :D

@shadeslayer
Copy link
Author

Closing since we're working on a better approach with tiled textures.

@shadeslayer shadeslayer closed this Jun 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants