Re: [Mesa-dev] Re mesa/st: add tgsi-lowering code for depth-clamp
Z is 0 at the near plane and 1 at the far plane, but the planes can have an arbitrary distance from the viewer, therefore Z Is irrelevant. W is the real distance from the viewer. The greater W is, the smaller (farther away) the object is, because W is the divisor (which makes objects smaller as it increases). The W=0 plane intersects the viewer. It is exactly in the middle of the "eye ball". W < 0 means behind the viewer. Marek On Fri., Aug. 2, 2019, 21:22 Ilia Mirkin, wrote: > Compare > > > generated_tests/spec/glsl-1.30/execution/interpolation/interpolation-smooth-other-smooth-none.shader_test > > generated_tests/spec/glsl-1.30/execution/interpolation/interpolation-noperspective-other-smooth-none.shader_test > > as you can see, the first one is perspective-interpolated and has a > lot more red and less blue (due to the depth). The second one is even, > since depth is ignored. gl_Position.w is the same in both cases, but > the "other" varying gets interpolated differently. > > I think you're right that 1/w plays into it -- you're supposed to > divide the interpolated result by w in the frag shader. But I think > the ij coefficients themselves are different? Not sure, it just seems > like z has to be taken into account *somewhere* otherwise the whole > thing can't possibly work. > > -ilia > > On Fri, Aug 2, 2019 at 8:11 PM Marek Olšák wrote: > > > > IIRC, perspective interpolation is driven by W, not Z. Interpolating W > and then computing barycentric coordinates using 1/W is what causes the > perspective distortion. > > > > Marek > > > > On Fri, Aug 2, 2019 at 4:59 PM Ilia Mirkin wrote: > >> > >> On Fri, Aug 2, 2019 at 3:46 PM Gert Wollny > wrote: > >> > > >> > Am Freitag, den 02.08.2019, 15:09 -0400 schrieb Ilia Mirkin: > >> > > On Fri, Aug 2, 2019 at 1:28 PM Gert Wollny < > gert.wol...@collabora.com > >> > > > wrote: > >> > > > Am Donnerstag, den 01.08.2019, 07:22 -0400 schrieb Ilia Mirkin: > >> > > > > Hey Gert, > >> > > > > > >> > > > > I'm looking at > >> > > > > > https://cgit.freedesktop.org/mesa/mesa/commit/?id=b048d8bf8f056759d1845a799d4ba2ac84bce30f > >> > > > > , which attempts to implement depth clamping (rather than > >> > > > > clipping) > >> > > > > with shader tricks. > >> > > > > > >> > > > > You're forcing the final vertex stage's position's depth to 0, > >> > > > > and > >> > > > > then making up for it in the frag shader with an extra varying. > >> > > > > However won't this screw up the barycentric coordinates for > >> > > > > perspective interpolation? i.e. won't you effectively always > just > >> > > > > get > >> > > > > noperspective interp everywhere as a result? > >> > > > That is probably true, I was following @kumas lead [1], in fact > >> > > > he implemented the initial version of this code, and I only fixed > >> > > > it up > >> > > > for ARB_clip_control and GS and TES shaders. > >> > > > > >> > > > One fix I could think is maybe clamp the z value to the clip range > >> > > > [-1, > >> > > > 1] or [0,1] depending on the clip control z accuracy, so that the > >> > > > error > >> > > > only happens only for the clamped areas where the result is > >> > > > distorted > >> > > > anyway, what do you think? > >> > > > >> > > Unfortunately I can't think of a way to generically emulate it > >> > > without implementing clipping in a geometry shader. > >> > Yeah, when I first looked into this, this is also what I thought of. > >> > > >> > > >> > > >> > > Basically if depth is clipped, the triangle gets split into 2 -- one > >> > > half which is shown, and one half which isn't. When depth is > clamped, > >> > > the half which isn't shown appears as a different polygon with the > >> > > clamped depth for all of its vertices instead. An interesting > >> > > question is whether it should be using the original z coords for its > >> > > barycentric coords or not -- I have no idea, and the spec doesn't > >> > > seem to explain it in a manner which is accessible to me. > >> > I think that the spec suggest to use the original z coords and the > >> > clamping only happens when the depth test is executed: > >> > > >> > RESOLUTION: ... Eliminating far and near plane clipping and > >> > clamping *interpolated* depth values to the depth range is much > >> > simpler to specify. > >> > > >> > > If it should be the original z coords, then perhaps you can just > >> > > disable clipping entirely in that case, > >> > My guess is that hardware that disables clipping completely also > >> > supports clamping the depth values. Our use case (virgl on top of e > >> > >> Yeah, that's how NVIDIA hardware works. There's a DEPTH_CLAMP_NEAR/FAR > >> value which you set to a value between 0 and 1, based on the viewport > >> transform, and that just clamps it prior to being supplied to the frag > >> shader. (And separately, you disable clipping near/far planes.) Adreno > >> hw works similarly. > >> > >> > GLES host that doesn't support EXT_depth_clamp) repesents the > opposite: > >> > a
[Mesa-dev] [Bug 111290] undefined symbol _dri2_get_mapping_by_fourcc
https://bugs.freedesktop.org/show_bug.cgi?id=111290 Vinson Lee changed: What|Removed |Added Keywords||bisected CC||michael.blumenkrantz@gmail. ||com --- Comment #1 from Vinson Lee --- 8af1990ad7703dd6805af9beb3a32ca7170c10c2 is the first bad commit commit 8af1990ad7703dd6805af9beb3a32ca7170c10c2 Author: Mike Blumenkrantz Date: Thu Jun 6 19:47:23 2019 -0400 st/dri: simplify dri_get_egl_image by reusing dri2_format_table this makes dri2_get_mapping_by_fourcc accessible from dri_helpers.h and does a direct lookup on the fourcc id to match the pipe format v2 (Ken): Allow map to be NULL, use img->texture->format. Reviewed-by: Kenneth Graunke Reviewed-by: Eric Anholt -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111290] undefined symbol _dri2_get_mapping_by_fourcc
https://bugs.freedesktop.org/show_bug.cgi?id=111290 Bug ID: 111290 Summary: undefined symbol _dri2_get_mapping_by_fourcc Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: Mac OS X (All) Status: NEW Keywords: regression Severity: normal Priority: medium Component: Other Assignee: mesa-dev@lists.freedesktop.org Reporter: v...@freedesktop.org QA Contact: mesa-dev@lists.freedesktop.org macOS build error Undefined symbols for architecture x86_64: "_dri2_get_mapping_by_fourcc", referenced from: _dri_get_egl_image in libdri.a(dri_screen.c.o) -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Re mesa/st: add tgsi-lowering code for depth-clamp
Compare generated_tests/spec/glsl-1.30/execution/interpolation/interpolation-smooth-other-smooth-none.shader_test generated_tests/spec/glsl-1.30/execution/interpolation/interpolation-noperspective-other-smooth-none.shader_test as you can see, the first one is perspective-interpolated and has a lot more red and less blue (due to the depth). The second one is even, since depth is ignored. gl_Position.w is the same in both cases, but the "other" varying gets interpolated differently. I think you're right that 1/w plays into it -- you're supposed to divide the interpolated result by w in the frag shader. But I think the ij coefficients themselves are different? Not sure, it just seems like z has to be taken into account *somewhere* otherwise the whole thing can't possibly work. -ilia On Fri, Aug 2, 2019 at 8:11 PM Marek Olšák wrote: > > IIRC, perspective interpolation is driven by W, not Z. Interpolating W and > then computing barycentric coordinates using 1/W is what causes the > perspective distortion. > > Marek > > On Fri, Aug 2, 2019 at 4:59 PM Ilia Mirkin wrote: >> >> On Fri, Aug 2, 2019 at 3:46 PM Gert Wollny wrote: >> > >> > Am Freitag, den 02.08.2019, 15:09 -0400 schrieb Ilia Mirkin: >> > > On Fri, Aug 2, 2019 at 1:28 PM Gert Wollny > > > > wrote: >> > > > Am Donnerstag, den 01.08.2019, 07:22 -0400 schrieb Ilia Mirkin: >> > > > > Hey Gert, >> > > > > >> > > > > I'm looking at >> > > > > https://cgit.freedesktop.org/mesa/mesa/commit/?id=b048d8bf8f056759d1845a799d4ba2ac84bce30f >> > > > > , which attempts to implement depth clamping (rather than >> > > > > clipping) >> > > > > with shader tricks. >> > > > > >> > > > > You're forcing the final vertex stage's position's depth to 0, >> > > > > and >> > > > > then making up for it in the frag shader with an extra varying. >> > > > > However won't this screw up the barycentric coordinates for >> > > > > perspective interpolation? i.e. won't you effectively always just >> > > > > get >> > > > > noperspective interp everywhere as a result? >> > > > That is probably true, I was following @kumas lead [1], in fact >> > > > he implemented the initial version of this code, and I only fixed >> > > > it up >> > > > for ARB_clip_control and GS and TES shaders. >> > > > >> > > > One fix I could think is maybe clamp the z value to the clip range >> > > > [-1, >> > > > 1] or [0,1] depending on the clip control z accuracy, so that the >> > > > error >> > > > only happens only for the clamped areas where the result is >> > > > distorted >> > > > anyway, what do you think? >> > > >> > > Unfortunately I can't think of a way to generically emulate it >> > > without implementing clipping in a geometry shader. >> > Yeah, when I first looked into this, this is also what I thought of. >> > >> > >> > >> > > Basically if depth is clipped, the triangle gets split into 2 -- one >> > > half which is shown, and one half which isn't. When depth is clamped, >> > > the half which isn't shown appears as a different polygon with the >> > > clamped depth for all of its vertices instead. An interesting >> > > question is whether it should be using the original z coords for its >> > > barycentric coords or not -- I have no idea, and the spec doesn't >> > > seem to explain it in a manner which is accessible to me. >> > I think that the spec suggest to use the original z coords and the >> > clamping only happens when the depth test is executed: >> > >> > RESOLUTION: ... Eliminating far and near plane clipping and >> > clamping *interpolated* depth values to the depth range is much >> > simpler to specify. >> > >> > > If it should be the original z coords, then perhaps you can just >> > > disable clipping entirely in that case, >> > My guess is that hardware that disables clipping completely also >> > supports clamping the depth values. Our use case (virgl on top of e >> >> Yeah, that's how NVIDIA hardware works. There's a DEPTH_CLAMP_NEAR/FAR >> value which you set to a value between 0 and 1, based on the viewport >> transform, and that just clamps it prior to being supplied to the frag >> shader. (And separately, you disable clipping near/far planes.) Adreno >> hw works similarly. >> >> > GLES host that doesn't support EXT_depth_clamp) repesents the opposite: >> > a hardware that doesn't support disabling clipping because OpenGL (ES) >> > doesn't allow this and also not doesn't support depth clamping. >> > >> > I still have to think about whether the interpolation really goes >> > wrong. I think I need to write another piglit to get an idea. >> >> Should be easy - take one of the interpolation piglit tests, and >> enable depth clamp without doing anything else. >> >> Cheers, >> >> -ilia >> ___ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org
Re: [Mesa-dev] Re mesa/st: add tgsi-lowering code for depth-clamp
IIRC, perspective interpolation is driven by W, not Z. Interpolating W and then computing barycentric coordinates using 1/W is what causes the perspective distortion. Marek On Fri, Aug 2, 2019 at 4:59 PM Ilia Mirkin wrote: > On Fri, Aug 2, 2019 at 3:46 PM Gert Wollny > wrote: > > > > Am Freitag, den 02.08.2019, 15:09 -0400 schrieb Ilia Mirkin: > > > On Fri, Aug 2, 2019 at 1:28 PM Gert Wollny > > > wrote: > > > > Am Donnerstag, den 01.08.2019, 07:22 -0400 schrieb Ilia Mirkin: > > > > > Hey Gert, > > > > > > > > > > I'm looking at > > > > > > https://cgit.freedesktop.org/mesa/mesa/commit/?id=b048d8bf8f056759d1845a799d4ba2ac84bce30f > > > > > , which attempts to implement depth clamping (rather than > > > > > clipping) > > > > > with shader tricks. > > > > > > > > > > You're forcing the final vertex stage's position's depth to 0, > > > > > and > > > > > then making up for it in the frag shader with an extra varying. > > > > > However won't this screw up the barycentric coordinates for > > > > > perspective interpolation? i.e. won't you effectively always just > > > > > get > > > > > noperspective interp everywhere as a result? > > > > That is probably true, I was following @kumas lead [1], in fact > > > > he implemented the initial version of this code, and I only fixed > > > > it up > > > > for ARB_clip_control and GS and TES shaders. > > > > > > > > One fix I could think is maybe clamp the z value to the clip range > > > > [-1, > > > > 1] or [0,1] depending on the clip control z accuracy, so that the > > > > error > > > > only happens only for the clamped areas where the result is > > > > distorted > > > > anyway, what do you think? > > > > > > Unfortunately I can't think of a way to generically emulate it > > > without implementing clipping in a geometry shader. > > Yeah, when I first looked into this, this is also what I thought of. > > > > > > > > > Basically if depth is clipped, the triangle gets split into 2 -- one > > > half which is shown, and one half which isn't. When depth is clamped, > > > the half which isn't shown appears as a different polygon with the > > > clamped depth for all of its vertices instead. An interesting > > > question is whether it should be using the original z coords for its > > > barycentric coords or not -- I have no idea, and the spec doesn't > > > seem to explain it in a manner which is accessible to me. > > I think that the spec suggest to use the original z coords and the > > clamping only happens when the depth test is executed: > > > > RESOLUTION: ... Eliminating far and near plane clipping and > > clamping *interpolated* depth values to the depth range is much > > simpler to specify. > > > > > If it should be the original z coords, then perhaps you can just > > > disable clipping entirely in that case, > > My guess is that hardware that disables clipping completely also > > supports clamping the depth values. Our use case (virgl on top of e > > Yeah, that's how NVIDIA hardware works. There's a DEPTH_CLAMP_NEAR/FAR > value which you set to a value between 0 and 1, based on the viewport > transform, and that just clamps it prior to being supplied to the frag > shader. (And separately, you disable clipping near/far planes.) Adreno > hw works similarly. > > > GLES host that doesn't support EXT_depth_clamp) repesents the opposite: > > a hardware that doesn't support disabling clipping because OpenGL (ES) > > doesn't allow this and also not doesn't support depth clamping. > > > > I still have to think about whether the interpolation really goes > > wrong. I think I need to write another piglit to get an idea. > > Should be easy - take one of the interpolation piglit tests, and > enable depth clamp without doing anything else. > > Cheers, > > -ilia > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] radeonsi: fix an assertion failure: assert(!res->b.is_shared)
From: Marek Olšák
This only appears to happen on Raven2.
Possible way to reproduce:
resource_get_handle(WINSYS_HANDLE_TYPE_KMS) --> sets is_shared = true
resource_get_handle(WINSYS_HANDLE_TYPE_DMABUF) --> fail
Cc: 19.1 19.2
---
src/gallium/drivers/radeonsi/si_texture.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/gallium/drivers/radeonsi/si_texture.c
b/src/gallium/drivers/radeonsi/si_texture.c
index 904a39b6fed..be614e66440 100644
--- a/src/gallium/drivers/radeonsi/si_texture.c
+++ b/src/gallium/drivers/radeonsi/si_texture.c
@@ -912,22 +912,21 @@ static bool si_texture_get_handle(struct pipe_screen*
screen,
/* This is not supported now, but it might be required for
OpenCL
* interop in the future.
*/
if (resource->nr_samples > 1 || tex->is_depth)
return false;
/* Move a suballocated texture into a non-suballocated
allocation. */
if (sscreen->ws->buffer_is_suballocated(res->buf) ||
tex->surface.tile_swizzle ||
(tex->buffer.flags & RADEON_FLAG_NO_INTERPROCESS_SHARING &&
-sscreen->info.has_local_buffers &&
-whandle->type != WINSYS_HANDLE_TYPE_KMS)) {
+sscreen->info.has_local_buffers)) {
assert(!res->b.is_shared);
si_reallocate_texture_inplace(sctx, tex,
PIPE_BIND_SHARED,
false);
flush = true;
assert(res->b.b.bind & PIPE_BIND_SHARED);
assert(res->flags & RADEON_FLAG_NO_SUBALLOC);
assert(!(res->flags &
RADEON_FLAG_NO_INTERPROCESS_SHARING));
assert(tex->surface.tile_swizzle == 0);
}
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/mesa: don't allocate mipmapped texture for NEAREST_MIPMAP_LINEAR
From: Marek Olšák --- src/mesa/state_tracker/st_cb_texture.c | 12 1 file changed, 12 insertions(+) diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c index 0edb3ea5c7e..1ace61863ff 100644 --- a/src/mesa/state_tracker/st_cb_texture.c +++ b/src/mesa/state_tracker/st_cb_texture.c @@ -516,20 +516,32 @@ allocate_full_mipmap(const struct st_texture_object *stObj, return FALSE; if (stObj->base.BaseLevel == 0 && stObj->base.MaxLevel == 0) return FALSE; if (stObj->base.Sampler.MinFilter == GL_NEAREST || stObj->base.Sampler.MinFilter == GL_LINEAR) /* not a mipmap minification filter */ return FALSE; + /* If the following sequence of GL calls is used: +* glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, w, h, 0, GL_RGB, ... +* glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR); +* +* we would needlessly allocate a mipmapped texture, because the initial +* MinFilter is GL_NEAREST_MIPMAP_LINEAR. Catch this case and don't +* allocate a mipmapped texture by default. This may cause texture +* reallocation later, but GL_NEAREST_MIPMAP_LINEAR is pretty rare. +*/ + if (stObj->base.Sampler.MinFilter == GL_NEAREST_MIPMAP_LINEAR) + return FALSE; + if (stObj->base.Target == GL_TEXTURE_3D) /* 3D textures are seldom mipmapped */ return FALSE; return TRUE; } /** * Try to allocate a pipe_resource object for the given st_texture_object. -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111288] Memory leak in minecraft (supposedly related to rendering)
https://bugs.freedesktop.org/show_bug.cgi?id=111288 --- Comment #1 from Greg --- Created attachment 144935 --> https://bugs.freedesktop.org/attachment.cgi?id=144935=edit glxinfo -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111288] Memory leak in minecraft (supposedly related to rendering)
https://bugs.freedesktop.org/show_bug.cgi?id=111288 Bug ID: 111288 Summary: Memory leak in minecraft (supposedly related to rendering) Product: Mesa Version: 19.1 Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Mesa core Assignee: mesa-dev@lists.freedesktop.org Reporter: remordemef...@yandex.ru QA Contact: mesa-dev@lists.freedesktop.org Created attachment 144934 --> https://bugs.freedesktop.org/attachment.cgi?id=144934=edit apitrace Minecraft 1.7.10 (also tested on 1.0 release) leaks memory in form of 1Mb memory regions mapped to `/dev/dri/renderD128` (observed in /proc/[id]/maps)(it seems new ones appear and old ones stay until program termination; probably it has something to do with chunk loading/unloading since it seems new ones don't appear when you stay still). Tested on java 8, happens with both openjdk (from ubuntu 18.04 repositories) and oracle java (taken from here https://www.java.com/en/download/ ). Leak also happens with LIBGL_ALWAYS_SOFTWARE=1 set. Might be worth noting that I use additional kernel boot parameters (iommu=soft idle=nomwait rcu_nocbs=0-7 acpi_enforce_resources=lax) (some or all of those are said to lessen system freezes (known problem with Ryzen 2500u)). -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Extension help
On Fri, Aug 2, 2019 at 3:03 PM Ian Romanick wrote: > > On 8/2/19 1:13 PM, Fritz Koenig wrote: > > Hi, > > > > I would like to be able to use the MESA_framebuffer_flip_y extension > > in GLES 3.0. The blocker is that FramebufferParameteri is not part of > > GLES 3.0. I have explored a couple of ways of achieving this. > > > > 1. FramebufferParameteri was first provided by the GL extension > > ARB_framebuffer_no_attachments. However, > > ARB_framebuffer_no_attachments was never a GLES extension. An option > > would be to modify the ARB_framebuffer_no_attachments spec to make it > > an extension for GLES. > > > > 2. Change the MESA_framebuffer_flip_y spec to allow the extension to > > implement FramebufferParameteri. > > GLES has more strict rules about what functions can be advertised than > desktop OpenGL. Since glFramebufferParameteri is part of the reserved > namespace, I'm not 100% sure that it's legal to expose it in pre-3.1. I > would suggest making a PR to the registry first. > I have opened a pr with Khronos. https://github.com/KhronosGroup/OpenGL-Registry/pull/289 > My gut tells me you're going to have to add a FramebufferParameteriMESA > function that is an alias of FramebufferParameteri. > Thanks for the feedback. I was hoping to be able to drop in FramebufferParameteri, but I believe you are correct that I need FramebufferParameteriMESA. > Has this extension already shipped in a Mesa release? > Yes, it has was present in Mesa 19 > > I choose the 2nd option and put together a pr for it[1]. > > > > I didn't get the implementation quite right, and I'm looking for some > > help. My change is failing the DispatchSanity_test.GLES3 test: > > > > [ RUN ] DispatchSanity_test.GLES3 > > ../src/mesa/main/tests/dispatch_sanity.cpp:174: Failure > > Expected: nop_table[i] > > Which is: 0x563d50d6fa01 > > To be equal to: table[i] > > Which is: 0x563d50db158a > > i = 888 (FramebufferParameteri) > > ../src/mesa/main/tests/dispatch_sanity.cpp:174: Failure > > Expected: nop_table[i] > > Which is: 0x563d50d6fa01 > > To be equal to: table[i] > > Which is: 0x563d50db1add > > i = 889 (GetFramebufferParameteriv) > > [ FAILED ] DispatchSanity_test.GLES3 (1 ms) > > > > It appears there is a problem with the way that I'm > > defining/redefining FramebufferParameteri. I've tried adding it to > > src/mapi/glapi/gen/es_EXT.xml, but the only way I was able to achieve > > that was to give it a different function name and an alias. > > > > Does anyone have experience with this sort of backporting of > > extensions that could offer some insight on what I might be doing > > wrong? > > > > Thanks. > > > > -Fritz > > > > [1]: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1560 > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Extension help
On 8/2/19 1:13 PM, Fritz Koenig wrote: > Hi, > > I would like to be able to use the MESA_framebuffer_flip_y extension > in GLES 3.0. The blocker is that FramebufferParameteri is not part of > GLES 3.0. I have explored a couple of ways of achieving this. > > 1. FramebufferParameteri was first provided by the GL extension > ARB_framebuffer_no_attachments. However, > ARB_framebuffer_no_attachments was never a GLES extension. An option > would be to modify the ARB_framebuffer_no_attachments spec to make it > an extension for GLES. > > 2. Change the MESA_framebuffer_flip_y spec to allow the extension to > implement FramebufferParameteri. GLES has more strict rules about what functions can be advertised than desktop OpenGL. Since glFramebufferParameteri is part of the reserved namespace, I'm not 100% sure that it's legal to expose it in pre-3.1. I would suggest making a PR to the registry first. My gut tells me you're going to have to add a FramebufferParameteriMESA function that is an alias of FramebufferParameteri. Has this extension already shipped in a Mesa release? > I choose the 2nd option and put together a pr for it[1]. > > I didn't get the implementation quite right, and I'm looking for some > help. My change is failing the DispatchSanity_test.GLES3 test: > > [ RUN ] DispatchSanity_test.GLES3 > ../src/mesa/main/tests/dispatch_sanity.cpp:174: Failure > Expected: nop_table[i] > Which is: 0x563d50d6fa01 > To be equal to: table[i] > Which is: 0x563d50db158a > i = 888 (FramebufferParameteri) > ../src/mesa/main/tests/dispatch_sanity.cpp:174: Failure > Expected: nop_table[i] > Which is: 0x563d50d6fa01 > To be equal to: table[i] > Which is: 0x563d50db1add > i = 889 (GetFramebufferParameteriv) > [ FAILED ] DispatchSanity_test.GLES3 (1 ms) > > It appears there is a problem with the way that I'm > defining/redefining FramebufferParameteri. I've tried adding it to > src/mapi/glapi/gen/es_EXT.xml, but the only way I was able to achieve > that was to give it a different function name and an alias. > > Does anyone have experience with this sort of backporting of > extensions that could offer some insight on what I might be doing > wrong? > > Thanks. > > -Fritz > > [1]: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1560 > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Extension help
I made another pass and I think I solved it in the correct way. I think the problem was that the test needed to be updated to take into consideration that the extension can apply to GLES 3.0 and not just GLES 3.1. On Fri, Aug 2, 2019 at 1:13 PM Fritz Koenig wrote: > > Hi, > > I would like to be able to use the MESA_framebuffer_flip_y extension > in GLES 3.0. The blocker is that FramebufferParameteri is not part of > GLES 3.0. I have explored a couple of ways of achieving this. > > 1. FramebufferParameteri was first provided by the GL extension > ARB_framebuffer_no_attachments. However, > ARB_framebuffer_no_attachments was never a GLES extension. An option > would be to modify the ARB_framebuffer_no_attachments spec to make it > an extension for GLES. > > 2. Change the MESA_framebuffer_flip_y spec to allow the extension to > implement FramebufferParameteri. > > I choose the 2nd option and put together a pr for it[1]. > > I didn't get the implementation quite right, and I'm looking for some > help. My change is failing the DispatchSanity_test.GLES3 test: > > [ RUN ] DispatchSanity_test.GLES3 > ../src/mesa/main/tests/dispatch_sanity.cpp:174: Failure > Expected: nop_table[i] > Which is: 0x563d50d6fa01 > To be equal to: table[i] > Which is: 0x563d50db158a > i = 888 (FramebufferParameteri) > ../src/mesa/main/tests/dispatch_sanity.cpp:174: Failure > Expected: nop_table[i] > Which is: 0x563d50d6fa01 > To be equal to: table[i] > Which is: 0x563d50db1add > i = 889 (GetFramebufferParameteriv) > [ FAILED ] DispatchSanity_test.GLES3 (1 ms) > > It appears there is a problem with the way that I'm > defining/redefining FramebufferParameteri. I've tried adding it to > src/mapi/glapi/gen/es_EXT.xml, but the only way I was able to achieve > that was to give it a different function name and an alias. > > Does anyone have experience with this sort of backporting of > extensions that could offer some insight on what I might be doing > wrong? > > Thanks. > > -Fritz > > [1]: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1560 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Re mesa/st: add tgsi-lowering code for depth-clamp
On Fri, Aug 2, 2019 at 3:46 PM Gert Wollny wrote: > > Am Freitag, den 02.08.2019, 15:09 -0400 schrieb Ilia Mirkin: > > On Fri, Aug 2, 2019 at 1:28 PM Gert Wollny > > wrote: > > > Am Donnerstag, den 01.08.2019, 07:22 -0400 schrieb Ilia Mirkin: > > > > Hey Gert, > > > > > > > > I'm looking at > > > > https://cgit.freedesktop.org/mesa/mesa/commit/?id=b048d8bf8f056759d1845a799d4ba2ac84bce30f > > > > , which attempts to implement depth clamping (rather than > > > > clipping) > > > > with shader tricks. > > > > > > > > You're forcing the final vertex stage's position's depth to 0, > > > > and > > > > then making up for it in the frag shader with an extra varying. > > > > However won't this screw up the barycentric coordinates for > > > > perspective interpolation? i.e. won't you effectively always just > > > > get > > > > noperspective interp everywhere as a result? > > > That is probably true, I was following @kumas lead [1], in fact > > > he implemented the initial version of this code, and I only fixed > > > it up > > > for ARB_clip_control and GS and TES shaders. > > > > > > One fix I could think is maybe clamp the z value to the clip range > > > [-1, > > > 1] or [0,1] depending on the clip control z accuracy, so that the > > > error > > > only happens only for the clamped areas where the result is > > > distorted > > > anyway, what do you think? > > > > Unfortunately I can't think of a way to generically emulate it > > without implementing clipping in a geometry shader. > Yeah, when I first looked into this, this is also what I thought of. > > > > > Basically if depth is clipped, the triangle gets split into 2 -- one > > half which is shown, and one half which isn't. When depth is clamped, > > the half which isn't shown appears as a different polygon with the > > clamped depth for all of its vertices instead. An interesting > > question is whether it should be using the original z coords for its > > barycentric coords or not -- I have no idea, and the spec doesn't > > seem to explain it in a manner which is accessible to me. > I think that the spec suggest to use the original z coords and the > clamping only happens when the depth test is executed: > > RESOLUTION: ... Eliminating far and near plane clipping and > clamping *interpolated* depth values to the depth range is much > simpler to specify. > > > If it should be the original z coords, then perhaps you can just > > disable clipping entirely in that case, > My guess is that hardware that disables clipping completely also > supports clamping the depth values. Our use case (virgl on top of e Yeah, that's how NVIDIA hardware works. There's a DEPTH_CLAMP_NEAR/FAR value which you set to a value between 0 and 1, based on the viewport transform, and that just clamps it prior to being supplied to the frag shader. (And separately, you disable clipping near/far planes.) Adreno hw works similarly. > GLES host that doesn't support EXT_depth_clamp) repesents the opposite: > a hardware that doesn't support disabling clipping because OpenGL (ES) > doesn't allow this and also not doesn't support depth clamping. > > I still have to think about whether the interpolation really goes > wrong. I think I need to write another piglit to get an idea. Should be easy - take one of the interpolation piglit tests, and enable depth clamp without doing anything else. Cheers, -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Extension help
Hi, I would like to be able to use the MESA_framebuffer_flip_y extension in GLES 3.0. The blocker is that FramebufferParameteri is not part of GLES 3.0. I have explored a couple of ways of achieving this. 1. FramebufferParameteri was first provided by the GL extension ARB_framebuffer_no_attachments. However, ARB_framebuffer_no_attachments was never a GLES extension. An option would be to modify the ARB_framebuffer_no_attachments spec to make it an extension for GLES. 2. Change the MESA_framebuffer_flip_y spec to allow the extension to implement FramebufferParameteri. I choose the 2nd option and put together a pr for it[1]. I didn't get the implementation quite right, and I'm looking for some help. My change is failing the DispatchSanity_test.GLES3 test: [ RUN ] DispatchSanity_test.GLES3 ../src/mesa/main/tests/dispatch_sanity.cpp:174: Failure Expected: nop_table[i] Which is: 0x563d50d6fa01 To be equal to: table[i] Which is: 0x563d50db158a i = 888 (FramebufferParameteri) ../src/mesa/main/tests/dispatch_sanity.cpp:174: Failure Expected: nop_table[i] Which is: 0x563d50d6fa01 To be equal to: table[i] Which is: 0x563d50db1add i = 889 (GetFramebufferParameteriv) [ FAILED ] DispatchSanity_test.GLES3 (1 ms) It appears there is a problem with the way that I'm defining/redefining FramebufferParameteri. I've tried adding it to src/mapi/glapi/gen/es_EXT.xml, but the only way I was able to achieve that was to give it a different function name and an alias. Does anyone have experience with this sort of backporting of extensions that could offer some insight on what I might be doing wrong? Thanks. -Fritz [1]: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1560 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Re mesa/st: add tgsi-lowering code for depth-clamp
Am Freitag, den 02.08.2019, 15:09 -0400 schrieb Ilia Mirkin: > On Fri, Aug 2, 2019 at 1:28 PM Gert Wollny > wrote: > > Am Donnerstag, den 01.08.2019, 07:22 -0400 schrieb Ilia Mirkin: > > > Hey Gert, > > > > > > I'm looking at > > > https://cgit.freedesktop.org/mesa/mesa/commit/?id=b048d8bf8f056759d1845a799d4ba2ac84bce30f > > > , which attempts to implement depth clamping (rather than > > > clipping) > > > with shader tricks. > > > > > > You're forcing the final vertex stage's position's depth to 0, > > > and > > > then making up for it in the frag shader with an extra varying. > > > However won't this screw up the barycentric coordinates for > > > perspective interpolation? i.e. won't you effectively always just > > > get > > > noperspective interp everywhere as a result? > > That is probably true, I was following @kumas lead [1], in fact > > he implemented the initial version of this code, and I only fixed > > it up > > for ARB_clip_control and GS and TES shaders. > > > > One fix I could think is maybe clamp the z value to the clip range > > [-1, > > 1] or [0,1] depending on the clip control z accuracy, so that the > > error > > only happens only for the clamped areas where the result is > > distorted > > anyway, what do you think? > > Unfortunately I can't think of a way to generically emulate it > without implementing clipping in a geometry shader. Yeah, when I first looked into this, this is also what I thought of. > Basically if depth is clipped, the triangle gets split into 2 -- one > half which is shown, and one half which isn't. When depth is clamped, > the half which isn't shown appears as a different polygon with the > clamped depth for all of its vertices instead. An interesting > question is whether it should be using the original z coords for its > barycentric coords or not -- I have no idea, and the spec doesn't > seem to explain it in a manner which is accessible to me. I think that the spec suggest to use the original z coords and the clamping only happens when the depth test is executed: RESOLUTION: ... Eliminating far and near plane clipping and clamping *interpolated* depth values to the depth range is much simpler to specify. > If it should be the original z coords, then perhaps you can just > disable clipping entirely in that case, My guess is that hardware that disables clipping completely also supports clamping the depth values. Our use case (virgl on top of e GLES host that doesn't support EXT_depth_clamp) repesents the opposite: a hardware that doesn't support disabling clipping because OpenGL (ES) doesn't allow this and also not doesn't support depth clamping. I still have to think about whether the interpolation really goes wrong. I think I need to write another piglit to get an idea. Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: fix OpenGL 3.1 context creation
On 8/2/19 12:39 PM, Ian Romanick wrote: > On 8/1/19 6:38 PM, Timothy Arceri wrote: >> From the EGL_KHR_create_context spec: >> >>"* If OpenGL 3.1 is requested, the context returned may implement >>any of the following versions: >> >> * Version 3.1. The GL_ARB_compatibility extension may or may >>not be implemented, as determined by the implementation. >> * The core profile of version 3.2 or greater." >> >> Fixes CTS tests: >> >> dEQP-EGL.functional.create_context_ext.gl_31.rgb888_depth_stencil >> dEQP-EGL.functional.create_context_ext.robust_gl_31.rgb888_depth_stencil >> dEQP-EGL.functional.create_context_ext.gl_31.rgb888_depth_no_stencil >> >> dEQP-EGL.functional.create_context_ext.robust_gl_31.rgb888_depth_no_stencil >> dEQP-EGL.functional.create_context_ext.gl_31.rgba_depth_no_stencil >> dEQP-EGL.functional.create_context_ext.gl_31.rgb888_no_depth_no_stencil >> >> dEQP-EGL.functional.create_context_ext.robust_gl_31.rgba_depth_no_stencil >> >> dEQP-EGL.functional.create_context_ext.robust_gl_31.rgb888_no_depth_no_stencil >> dEQP-EGL.functional.create_context_ext.gl_31.rgba_no_depth_no_stencil >> >> dEQP-EGL.functional.create_context_ext.robust_gl_31.rgba_no_depth_no_stencil >> dEQP-EGL.functional.create_context_ext.gl_31.rgba_depth_stencil >> >> dEQP-EGL.functional.create_context_ext.robust_gl_31.rgba_depth_stencil >> --- >> src/egl/drivers/dri2/egl_dri2.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/src/egl/drivers/dri2/egl_dri2.c >> b/src/egl/drivers/dri2/egl_dri2.c >> index c712c106b06..918d61a1e9b 100644 >> --- a/src/egl/drivers/dri2/egl_dri2.c >> +++ b/src/egl/drivers/dri2/egl_dri2.c >> @@ -1245,6 +1245,9 @@ dri2_create_context(_EGLDriver *drv, _EGLDisplay >> *disp, _EGLConfig *conf, >> && dri2_ctx->base.ClientMinorVersion >= 2)) >>&& dri2_ctx->base.Profile == >> EGL_CONTEXT_OPENGL_CORE_PROFILE_BIT_KHR) >> api = __DRI_API_OPENGL_CORE; >> + else if (dri2_ctx->base.ClientMajorVersion == 3 && >> + dri2_ctx->base.ClientMinorVersion == 1) >> + api = __DRI_API_OPENGL_CORE; > > If my recollection of the way these are handled in the driver is > correct, I think this will prevent us from ever exposing > GL_ARB_compatibility when the context is created with EGL. I /think/ > the API choice should be further conditioned by > dri_screen->max_gl_compat_version. Something like It looks like dri2_convert_glx_attribs (src/glx/dri_common.c) has the old behavior too... which makes me wonder how creating an OpenGL 3.1 context has ever worked. Either way, EGL and GLX should behave the same for this kind of thing. > > else if (dri2_ctx->base.ClientMajorVersion == 3 && >dri2_ctx->base.ClientMinorVersion == 1) > api = dri2_dpy->dri_screen->max_gl_compat_version >= 31 > ? __DRI_API_OPENGL : __DRI_API_OPENGL_CORE; > >>else >> api = __DRI_API_OPENGL; >>break; >> > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: fix OpenGL 3.1 context creation
On 8/1/19 6:38 PM, Timothy Arceri wrote: > From the EGL_KHR_create_context spec: > >"* If OpenGL 3.1 is requested, the context returned may implement >any of the following versions: > > * Version 3.1. The GL_ARB_compatibility extension may or may >not be implemented, as determined by the implementation. > * The core profile of version 3.2 or greater." > > Fixes CTS tests: > > dEQP-EGL.functional.create_context_ext.gl_31.rgb888_depth_stencil > dEQP-EGL.functional.create_context_ext.robust_gl_31.rgb888_depth_stencil > dEQP-EGL.functional.create_context_ext.gl_31.rgb888_depth_no_stencil > > dEQP-EGL.functional.create_context_ext.robust_gl_31.rgb888_depth_no_stencil > dEQP-EGL.functional.create_context_ext.gl_31.rgba_depth_no_stencil > dEQP-EGL.functional.create_context_ext.gl_31.rgb888_no_depth_no_stencil > > dEQP-EGL.functional.create_context_ext.robust_gl_31.rgba_depth_no_stencil > > dEQP-EGL.functional.create_context_ext.robust_gl_31.rgb888_no_depth_no_stencil > dEQP-EGL.functional.create_context_ext.gl_31.rgba_no_depth_no_stencil > > dEQP-EGL.functional.create_context_ext.robust_gl_31.rgba_no_depth_no_stencil > dEQP-EGL.functional.create_context_ext.gl_31.rgba_depth_stencil > dEQP-EGL.functional.create_context_ext.robust_gl_31.rgba_depth_stencil > --- > src/egl/drivers/dri2/egl_dri2.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c > index c712c106b06..918d61a1e9b 100644 > --- a/src/egl/drivers/dri2/egl_dri2.c > +++ b/src/egl/drivers/dri2/egl_dri2.c > @@ -1245,6 +1245,9 @@ dri2_create_context(_EGLDriver *drv, _EGLDisplay *disp, > _EGLConfig *conf, > && dri2_ctx->base.ClientMinorVersion >= 2)) >&& dri2_ctx->base.Profile == > EGL_CONTEXT_OPENGL_CORE_PROFILE_BIT_KHR) > api = __DRI_API_OPENGL_CORE; > + else if (dri2_ctx->base.ClientMajorVersion == 3 && > + dri2_ctx->base.ClientMinorVersion == 1) > + api = __DRI_API_OPENGL_CORE; If my recollection of the way these are handled in the driver is correct, I think this will prevent us from ever exposing GL_ARB_compatibility when the context is created with EGL. I /think/ the API choice should be further conditioned by dri_screen->max_gl_compat_version. Something like else if (dri2_ctx->base.ClientMajorVersion == 3 && dri2_ctx->base.ClientMinorVersion == 1) api = dri2_dpy->dri_screen->max_gl_compat_version >= 31 ? __DRI_API_OPENGL : __DRI_API_OPENGL_CORE; >else > api = __DRI_API_OPENGL; >break; > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Mesa (update-reviewers-for-vmware): i965/clear: clear_value better precision
On 8/2/19 9:12 AM, Brian Paul wrote: > On 08/02/2019 09:56 AM, Eric Engestrom wrote: >> On Friday, 2019-08-02 17:50:17 +0200, Michel Dänzer wrote: >>> On 2019-08-02 5:37 p.m., Brian Paul wrote: Ugh, I didn't mean to do this. I'm trying to figure out how to make a merge request with gitlab. >>> >>> Just push to a branch in your personal repository, and the output of git >>> push contains an URL for creating an MR for it. >> >> Precisely, but just to be extra clear: your personal repo needs to be >> forked from the main mesa repo [1], not just "a repo containing the mesa >> git history". >> GitLab needs to know the two are linked and pressing that "fork" button >> is how you tell it. > > Yeah, I just figured that out a few minutes ago. After I figure out all > the detailed steps from scratch I'll add it to the documentation. > > I've really never done anything with gitlab, github, etc. and have been > busy with non-Mesa work for over a year now. I have a lot of catch-up > to do. Welcome back. :) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Re mesa/st: add tgsi-lowering code for depth-clamp
On Fri, Aug 2, 2019 at 1:28 PM Gert Wollny wrote: > > Am Donnerstag, den 01.08.2019, 07:22 -0400 schrieb Ilia Mirkin: > > Hey Gert, > > > > I'm looking at > > https://cgit.freedesktop.org/mesa/mesa/commit/?id=b048d8bf8f056759d1845a799d4ba2ac84bce30f > > , which attempts to implement depth clamping (rather than clipping) > > with shader tricks. > > > > You're forcing the final vertex stage's position's depth to 0, and > > then making up for it in the frag shader with an extra varying. > > However won't this screw up the barycentric coordinates for > > perspective interpolation? i.e. won't you effectively always just get > > noperspective interp everywhere as a result? > That is probably true, I was following @kumas lead [1], in fact > he implemented the initial version of this code, and I only fixed it up > for ARB_clip_control and GS and TES shaders. > > One fix I could think is maybe clamp the z value to the clip range [-1, > 1] or [0,1] depending on the clip control z accuracy, so that the error > only happens only for the clamped areas where the result is distorted > anyway, what do you think? Unfortunately I can't think of a way to generically emulate it without implementing clipping in a geometry shader. (I believe Marek has done something like this in a compute shader, actually, to pre-clip geometry-heavy workloads.) Basically if depth is clipped, the triangle gets split into 2 -- one half which is shown, and one half which isn't. When depth is clamped, the half which isn't shown appears as a different polygon with the clamped depth for all of its vertices instead. An interesting question is whether it should be using the original z coords for its barycentric coords or not -- I have no idea, and the spec doesn't seem to explain it in a manner which is accessible to me. If it should be the original z coords, then perhaps you can just disable clipping entirely in that case, and then in the frag shader, just output clamp(gl_FragCoord.z, minz, maxz) or something [and those would be set based on the transform/halfz settings]. You may want to consult someone with a clearer understanding of depth than me though -- it has always been a concept just out of reach for me, unfortunately. Cheers, -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111264] u_thread.h:64:39: error: too many arguments to function call, expected 1, have 2
https://bugs.freedesktop.org/show_bug.cgi?id=111264 Matt Turner changed: What|Removed |Added Resolution|--- |FIXED Status|ASSIGNED|RESOLVED --- Comment #4 from Matt Turner --- Fixed with https://gitlab.freedesktop.org/mesa/mesa/commit/dcf9d91a80ee76f46e162afef9bd1b2ddb53ecc3 -- You are receiving this mail because: You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111102] lp_bld_misc.cpp:834:156: error: no matching function for call to ‘llvm::IRBuilder<>::CreateAtomicCmpXchg(llvm::Value*, llvm::Value*, llvm::Value*, llvm::AtomicOrdering, llvm::S
https://bugs.freedesktop.org/show_bug.cgi?id=02 Roland Scheidegger changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #1 from Roland Scheidegger --- Should be fixed by 74baeacafc7c2e9bae0f4db9651b0c664d33f5ac. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 1/9] panfrost: Make ctx->job useful
R-b! Nice! :) signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Re mesa/st: add tgsi-lowering code for depth-clamp
Am Donnerstag, den 01.08.2019, 07:22 -0400 schrieb Ilia Mirkin: > Hey Gert, > > I'm looking at > https://cgit.freedesktop.org/mesa/mesa/commit/?id=b048d8bf8f056759d1845a799d4ba2ac84bce30f > , which attempts to implement depth clamping (rather than clipping) > with shader tricks. > > You're forcing the final vertex stage's position's depth to 0, and > then making up for it in the frag shader with an extra varying. > However won't this screw up the barycentric coordinates for > perspective interpolation? i.e. won't you effectively always just get > noperspective interp everywhere as a result? That is probably true, I was following @kumas lead [1], in fact he implemented the initial version of this code, and I only fixed it up for ARB_clip_control and GS and TES shaders. One fix I could think is maybe clamp the z value to the clip range [-1, 1] or [0,1] depending on the clip control z accuracy, so that the error only happens only for the clamped areas where the result is distorted anyway, what do you think? Best, Gert [1] https://stackoverflow.com/questions/5960757/how-to-emulate-gl-depth-clamp-nv > > -ilia > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 5/9] panfrost: Bail out early when doing a wallpaper blit
The wallpaper blit is a bit special in that the operation is targetting
the current FB, but the u_blitter logic creates a new surface for it
which makes util_framebuffer_state_equal() return false. In that case
we don't want a new FB descriptor to be emitted/attached, so let's just
copy the new state into ctx->pipe_framebuffer and exit the function.
Signed-off-by: Boris Brezillon
Reviewed-by: Alyssa Rosenzweig
---
Changes in v2:
* Add Alyssa's R-b
---
src/gallium/drivers/panfrost/pan_context.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/gallium/drivers/panfrost/pan_context.c
b/src/gallium/drivers/panfrost/pan_context.c
index 65e6824a9b03..a6412de76469 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -2369,10 +2369,22 @@ panfrost_set_framebuffer_state(struct pipe_context
*pctx,
if (util_framebuffer_state_equal(>pipe_framebuffer, fb))
return;
-if (!ctx->wallpaper_batch && (!is_scanout || has_draws)) {
-panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME);
+/* The wallpaper logic sets a new FB state before doing the blit and
+ * restore the old one when it's done. Those FB states are reported to
+ * be different because the surface they are pointing to are different,
+ * but those surfaces actually point to the same cbufs/zbufs. In that
+ * case we definitely don't want new FB descs to be emitted/attached
+ * since the job is expected to be flushed just after the blit is done,
+ * so let's just copy the new state and return here.
+ */
+if (ctx->wallpaper_batch) {
+util_copy_framebuffer_state(>pipe_framebuffer, fb);
+return;
}
+if (!is_scanout || has_draws)
+panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME);
+
/* Invalidate the FBO job cache since we've just been assigned a new
* FB state.
*/
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 2/9] panfrost: Remove job from ctx->jobs at submission time
This guarantees that new draws targetting the same framebuffer will get a new job instance. Signed-off-by: Boris Brezillon Reviewed-by: Alyssa Rosenzweig --- Changes in v2: * Add Alyssa's R-b --- src/gallium/drivers/panfrost/pan_job.c | 8 1 file changed, 8 insertions(+) diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c index cc81d475165e..d75af0c53308 100644 --- a/src/gallium/drivers/panfrost/pan_job.c +++ b/src/gallium/drivers/panfrost/pan_job.c @@ -198,6 +198,14 @@ panfrost_job_submit(struct panfrost_context *ctx, struct panfrost_job *job) */ assert(!ctx->job || job == ctx->job); ctx->job = NULL; + +/* Remove the job from the ctx->jobs set so that future + * panfrost_get_job() calls don't see it. + * We must reset the job key to avoid removing another valid entry when + * the job is freed. + */ +_mesa_hash_table_remove_key(ctx->jobs, >key); +memset(>key, 0, sizeof(job->key)); } void -- 2.21.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 9/9] panfrost: Allocate polygon lists on-demand
From: Alyssa Rosenzweig
Rather than alloacting a huge (64MB) polygon list on context creation
and sharing it across framebuffers, we instead allocate polygon lists as
BOs (which consistently hit the cache) sized appropriately; for about a
month, we've known how to calculate the polygon list size so this has
only recently become possible.
The good news is we can render to truly massive framebuffers without
crashing and, more importantly, we eliminate the 64MB upfront overhead.
If a list that size isn't actually needed, it's not allocated.
Signed-off-by: Alyssa Rosenzweig
Signed-off-by: Boris Brezillon
---
Changes in v2:
* None
---
src/gallium/drivers/panfrost/pan_context.c| 8 ++-
src/gallium/drivers/panfrost/pan_context.h| 1 -
src/gallium/drivers/panfrost/pan_drm.c| 2 +-
src/gallium/drivers/panfrost/pan_job.c| 24 +++
src/gallium/drivers/panfrost/pan_job.h| 6 +
src/gallium/drivers/panfrost/pan_scoreboard.c | 5 ++--
6 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/src/gallium/drivers/panfrost/pan_context.c
b/src/gallium/drivers/panfrost/pan_context.c
index d261a2842124..e9d18605dd02 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -62,6 +62,7 @@ panfrost_emit_midg_tiler(
unsigned vertex_count)
{
struct midgard_tiler_descriptor t = {};
+struct panfrost_job *batch = panfrost_get_job_for_fbo(ctx);
t.hierarchy_mask =
panfrost_choose_hierarchy_mask(width, height, vertex_count);
@@ -77,10 +78,7 @@ panfrost_emit_midg_tiler(
/* Sanity check */
if (t.hierarchy_mask) {
-assert(ctx->tiler_polygon_list.bo->size >= (header_size +
body_size));
-
-/* Specify allocated tiler structures */
-t.polygon_list = ctx->tiler_polygon_list.bo->gpu;
+t.polygon_list = panfrost_job_get_polygon_list(batch,
header_size + body_size);
/* Allow the entire tiler heap */
t.heap_start = ctx->tiler_heap.bo->gpu;
@@ -2532,7 +2530,6 @@ panfrost_destroy(struct pipe_context *pipe)
panfrost_drm_free_slab(screen, >scratchpad);
panfrost_drm_free_slab(screen, >shaders);
panfrost_drm_free_slab(screen, >tiler_heap);
-panfrost_drm_free_slab(screen, >tiler_polygon_list);
panfrost_drm_free_slab(screen, >tiler_dummy);
ralloc_free(pipe);
@@ -2678,7 +2675,6 @@ panfrost_setup_hardware(struct panfrost_context *ctx)
panfrost_drm_allocate_slab(screen, >scratchpad, 64*4, false, 0,
0, 0);
panfrost_drm_allocate_slab(screen, >shaders, 4096, true,
PAN_ALLOCATE_EXECUTE, 0, 0);
panfrost_drm_allocate_slab(screen, >tiler_heap, 4096, false,
PAN_ALLOCATE_INVISIBLE | PAN_ALLOCATE_GROWABLE, 1, 128);
-panfrost_drm_allocate_slab(screen, >tiler_polygon_list, 128*128,
false, PAN_ALLOCATE_INVISIBLE | PAN_ALLOCATE_GROWABLE, 1, 128);
panfrost_drm_allocate_slab(screen, >tiler_dummy, 1, false,
PAN_ALLOCATE_INVISIBLE, 0, 0);
}
diff --git a/src/gallium/drivers/panfrost/pan_context.h
b/src/gallium/drivers/panfrost/pan_context.h
index d8dbd4d66f6c..e8d2417e0cb2 100644
--- a/src/gallium/drivers/panfrost/pan_context.h
+++ b/src/gallium/drivers/panfrost/pan_context.h
@@ -111,7 +111,6 @@ struct panfrost_context {
struct panfrost_memory shaders;
struct panfrost_memory scratchpad;
struct panfrost_memory tiler_heap;
-struct panfrost_memory tiler_polygon_list;
struct panfrost_memory tiler_dummy;
struct panfrost_memory depth_stencil_buffer;
diff --git a/src/gallium/drivers/panfrost/pan_drm.c
b/src/gallium/drivers/panfrost/pan_drm.c
index 89c7019dd9c7..42cf17503344 100644
--- a/src/gallium/drivers/panfrost/pan_drm.c
+++ b/src/gallium/drivers/panfrost/pan_drm.c
@@ -288,7 +288,7 @@ panfrost_drm_submit_vs_fs_job(struct panfrost_context *ctx,
bool has_draws, bool
panfrost_job_add_bo(job, ctx->shaders.bo);
panfrost_job_add_bo(job, ctx->scratchpad.bo);
panfrost_job_add_bo(job, ctx->tiler_heap.bo);
-panfrost_job_add_bo(job, ctx->tiler_polygon_list.bo);
+panfrost_job_add_bo(job, job->polygon_list);
if (job->first_job.gpu) {
ret = panfrost_drm_submit_job(ctx, job->first_job.gpu, 0);
diff --git a/src/gallium/drivers/panfrost/pan_job.c
b/src/gallium/drivers/panfrost/pan_job.c
index d75af0c53308..661f8ae154e2 100644
--- a/src/gallium/drivers/panfrost/pan_job.c
+++ b/src/gallium/drivers/panfrost/pan_job.c
@@ -72,6 +72,9 @@ panfrost_free_job(struct panfrost_context *ctx, struct
panfrost_job *job)
BITSET_SET(screen->free_transient, *index);
}
+/* Unreference the polygon list */
+panfrost_bo_unreference(ctx->base.screen, job->polygon_list);
+
_mesa_hash_table_remove_key(ctx->jobs, >key);
if
Re: [Mesa-dev] [PATCH] gallivm: fix issue with AtomicCmpXchg wrapper on llvm 3.5-3.8
The patch looks good to me.
It replaces my earlier patch request on the same issue.
Reviewed-by: Charmaine Lee
On 8/2/19, 9:54 AM, "Brian Paul" wrote:
On 08/02/2019 10:36 AM, srol...@vmware.com wrote:
> From: Roland Scheidegger
>
> These versions still need wrapper but already have both success and
> failure ordering.
> (Compile tested on llvm 3.7, llvm 3.8.)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=02
> ---
> src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 16 +++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
> index 79d10293e80..723c84d57c2 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
> @@ -822,15 +822,29 @@ static llvm::AtomicOrdering
mapFromLLVMOrdering(LLVMAtomicOrdering Ordering) {
> llvm_unreachable("Invalid LLVMAtomicOrdering value!");
> }
>
> +#if HAVE_LLVM < 0x305
> LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr,
> LLVMValueRef Cmp, LLVMValueRef New,
> LLVMAtomicOrdering SuccessOrdering,
> LLVMAtomicOrdering FailureOrdering,
> LLVMBool SingleThread)
> {
> - /* LLVM 3.8 doesn't have a second ordering and uses old
SynchronizationScope enum */
> + /* LLVM < 3.5 doesn't have a second ordering and uses old
SynchronizationScope enum */
> return
llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr),
llvm::unwrap(Cmp),
>
llvm::unwrap(New), mapFromLLVMOrdering(SuccessOrdering),
> SingleThread
? llvm::SynchronizationScope::SingleThread :
llvm::SynchronizationScope::CrossThread));
> }
> +#else
> +LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr,
> +LLVMValueRef Cmp, LLVMValueRef New,
> +LLVMAtomicOrdering SuccessOrdering,
> +LLVMAtomicOrdering FailureOrdering,
> +LLVMBool SingleThread)
> +{
> + return
llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr),
llvm::unwrap(Cmp),
> +
llvm::unwrap(New), mapFromLLVMOrdering(SuccessOrdering),
> +
mapFromLLVMOrdering(FailureOrdering),
> + SingleThread ?
llvm::SynchronizationScope::SingleThread :
llvm::SynchronizationScope::CrossThread));
> +}
> +#endif
> #endif
>
Could the #if / #endif logic be moved into the body of
LLVMBuildAtomicCmpXchg() so the whole function isn't duplicated?
Other than that,
Reviewed-by: Brian Paul
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 8/9] panfrost: Handle the bo == NULL case in panfrost_bo_[un]reference()
Allows us to pass BOs without checking if they're NULL or not.
Signed-off-by: Boris Brezillon
Reviewed-by: Alyssa Rosenzweig
---
Changes in v2:
* Add Alyssa's R-b
---
src/gallium/drivers/panfrost/pan_resource.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/panfrost/pan_resource.c
b/src/gallium/drivers/panfrost/pan_resource.c
index 71da383d4c7a..f74a39555b45 100644
--- a/src/gallium/drivers/panfrost/pan_resource.c
+++ b/src/gallium/drivers/panfrost/pan_resource.c
@@ -443,12 +443,16 @@ panfrost_resource_create(struct pipe_screen *screen,
void
panfrost_bo_reference(struct panfrost_bo *bo)
{
-pipe_reference(NULL, >reference);
+if (bo)
+pipe_reference(NULL, >reference);
}
void
panfrost_bo_unreference(struct pipe_screen *screen, struct panfrost_bo *bo)
{
+if (!bo)
+return;
+
/* When the reference count goes to zero, we need to cleanup */
if (pipe_reference(>reference, NULL))
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 3/9] panfrost: Delay FB descriptor allocation
No need to emit SFBD/MFBD at frame invalidation. They can be emitted
when the framebuffer is attached, which saves us a potential FB desc
re-allocation if a new FB is bound after the swap.
Signed-off-by: Boris Brezillon
Reviewed-by: Alyssa Rosenzweig
---
Changes in v2:
* Add Alyssa's R-b
---
src/gallium/drivers/panfrost/pan_context.c | 21 ++---
src/gallium/drivers/panfrost/pan_context.h | 3 ---
2 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/src/gallium/drivers/panfrost/pan_context.c
b/src/gallium/drivers/panfrost/pan_context.c
index ba2df2ce66ae..a88e6a4607d5 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -189,13 +189,17 @@ panfrost_clear(
static mali_ptr
panfrost_attach_vt_mfbd(struct panfrost_context *ctx)
{
-return panfrost_upload_transient(ctx, >vt_framebuffer_mfbd,
sizeof(ctx->vt_framebuffer_mfbd)) | MALI_MFBD;
+struct bifrost_framebuffer mfbd = panfrost_emit_mfbd(ctx, ~0);
+
+return panfrost_upload_transient(ctx, , sizeof(mfbd)) | MALI_MFBD;
}
static mali_ptr
panfrost_attach_vt_sfbd(struct panfrost_context *ctx)
{
-return panfrost_upload_transient(ctx, >vt_framebuffer_sfbd,
sizeof(ctx->vt_framebuffer_sfbd)) | MALI_SFBD;
+struct mali_single_framebuffer sfbd = panfrost_emit_sfbd(ctx, ~0);
+
+return panfrost_upload_transient(ctx, , sizeof(sfbd)) | MALI_SFBD;
}
static void
@@ -223,13 +227,6 @@ panfrost_attach_vt_framebuffer(struct panfrost_context
*ctx, bool skippable)
static void
panfrost_invalidate_frame(struct panfrost_context *ctx)
{
-struct panfrost_screen *screen = pan_screen(ctx->base.screen);
-
-if (screen->require_sfbd)
-ctx->vt_framebuffer_sfbd = panfrost_emit_sfbd(ctx, ~0);
-else
-ctx->vt_framebuffer_mfbd = panfrost_emit_mfbd(ctx, ~0);
-
for (unsigned i = 0; i < PIPE_SHADER_TYPES; ++i)
ctx->payloads[i].postfix.framebuffer = 0;
@@ -2383,12 +2380,6 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx,
struct panfrost_screen *screen = pan_screen(ctx->base.screen);
panfrost_hint_afbc(screen, >pipe_framebuffer);
-
-if (screen->require_sfbd)
-ctx->vt_framebuffer_sfbd = panfrost_emit_sfbd(ctx, ~0);
-else
-ctx->vt_framebuffer_mfbd = panfrost_emit_mfbd(ctx, ~0);
-
panfrost_attach_vt_framebuffer(ctx, false);
}
diff --git a/src/gallium/drivers/panfrost/pan_context.h
b/src/gallium/drivers/panfrost/pan_context.h
index d930f12a537b..d8dbd4d66f6c 100644
--- a/src/gallium/drivers/panfrost/pan_context.h
+++ b/src/gallium/drivers/panfrost/pan_context.h
@@ -138,9 +138,6 @@ struct panfrost_context {
union mali_attr attributes[PIPE_MAX_ATTRIBS];
-struct mali_single_framebuffer vt_framebuffer_sfbd;
-struct bifrost_framebuffer vt_framebuffer_mfbd;
-
/* TODO: Multiple uniform buffers (index =/= 0), finer updates? */
struct panfrost_constant_buffer constant_buffer[PIPE_SHADER_TYPES];
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 6/9] panfrost: Don't emit a new FB desc when setting a new FB state
The FB desc will be emitted/attached on the first draw targetting this new FB. Signed-off-by: Boris Brezillon Reviewed-by: Alyssa Rosenzweig --- Changes in v2: * Add Alyssa's R-b --- src/gallium/drivers/panfrost/pan_context.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c index a6412de76469..85b9d4d41ceb 100644 --- a/src/gallium/drivers/panfrost/pan_context.c +++ b/src/gallium/drivers/panfrost/pan_context.c @@ -2384,6 +2384,9 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx, if (!is_scanout || has_draws) panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME); +else +assert(!ctx->payloads[PIPE_SHADER_VERTEX].postfix.framebuffer && + !ctx->payloads[PIPE_SHADER_FRAGMENT].postfix.framebuffer); /* Invalidate the FBO job cache since we've just been assigned a new * FB state. @@ -2396,7 +2399,8 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx, struct panfrost_screen *screen = pan_screen(ctx->base.screen); panfrost_hint_afbc(screen, >pipe_framebuffer); -panfrost_attach_vt_framebuffer(ctx, false); +for (unsigned i = 0; i < PIPE_SHADER_TYPES; ++i) +ctx->payloads[i].postfix.framebuffer = 0; } static void * -- 2.21.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 7/9] panfrost: Get rid of the skippable param in attach_vt_framebuffer()
The only user of this function always passes true.
Signed-off-by: Boris Brezillon
Reviewed-by: Alyssa Rosenzweig
---
Changes in v2:
* Add Alyssa's R-b
---
src/gallium/drivers/panfrost/pan_context.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/gallium/drivers/panfrost/pan_context.c
b/src/gallium/drivers/panfrost/pan_context.c
index 85b9d4d41ceb..d261a2842124 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -203,11 +203,11 @@ panfrost_attach_vt_sfbd(struct panfrost_context *ctx)
}
static void
-panfrost_attach_vt_framebuffer(struct panfrost_context *ctx, bool skippable)
+panfrost_attach_vt_framebuffer(struct panfrost_context *ctx)
{
/* Skip the attach if we can */
-if (skippable &&
ctx->payloads[PIPE_SHADER_VERTEX].postfix.framebuffer) {
+if (ctx->payloads[PIPE_SHADER_VERTEX].postfix.framebuffer) {
assert(ctx->payloads[PIPE_SHADER_FRAGMENT].postfix.framebuffer);
return;
}
@@ -1013,7 +1013,7 @@ panfrost_emit_for_draw(struct panfrost_context *ctx, bool
with_vertex_data)
struct panfrost_job *job = panfrost_get_job_for_fbo(ctx);
struct panfrost_screen *screen = pan_screen(ctx->base.screen);
-panfrost_attach_vt_framebuffer(ctx, true);
+panfrost_attach_vt_framebuffer(ctx);
if (with_vertex_data) {
panfrost_emit_vertex_data(job);
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 0/9] panfrost: Allocate the polygon lists on-demand
Hello, This patch series is getting rid of the 64MB polygon-list allocation that was done at context init time in favor of a per-job polygon-list allocation allowing us to shrink the BO size considerably and thus reduce memory consumption. The first 8 patches are random cleanups. Most of them are needed to simplify patch 9 (patch 2, 3, 4, 5, 6 and 8), others are just things I decided to get rid off because they were not used (patch 1 and 7). Not much has changed in this v2 apart from the 1st patch (the one removing the ctx->job field) being replaced by another one making this field useful (to limit the number of hash lookups). Regards, Boris Alyssa Rosenzweig (1): panfrost: Allocate polygon lists on-demand Boris Brezillon (8): panfrost: Make ctx->job useful panfrost: Remove job from ctx->jobs at submission time panfrost: Delay FB descriptor allocation panfrost: Bail out early when new and current FB states are equal panfrost: Bail out early when doing a wallpaper blit panfrost: Don't emit a new FB desc when setting a new FB state panfrost: Get rid of the skippable param in attach_vt_framebuffer() panfrost: Handle the bo == NULL case in panfrost_bo_[un]reference() src/gallium/drivers/panfrost/pan_context.c| 66 +++ src/gallium/drivers/panfrost/pan_context.h| 4 -- src/gallium/drivers/panfrost/pan_drm.c| 2 +- src/gallium/drivers/panfrost/pan_job.c| 51 +- src/gallium/drivers/panfrost/pan_job.h| 6 ++ src/gallium/drivers/panfrost/pan_resource.c | 6 +- src/gallium/drivers/panfrost/pan_scoreboard.c | 5 +- 7 files changed, 104 insertions(+), 36 deletions(-) -- 2.21.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 4/9] panfrost: Bail out early when new and current FB states are equal
If the current FB matches the new one there's nothing to be done in
panfrost_set_framebuffer_state(). By bailing out early in that case we
avoid emitting new FB descriptors (the old ones are still valid).
Signed-off-by: Boris Brezillon
Reviewed-by: Alyssa Rosenzweig
---
Changes in v2:
* Add Alyssa's R-b
---
src/gallium/drivers/panfrost/pan_context.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/gallium/drivers/panfrost/pan_context.c
b/src/gallium/drivers/panfrost/pan_context.c
index a88e6a4607d5..65e6824a9b03 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -2365,6 +2365,10 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx,
bool is_scanout = panfrost_is_scanout(ctx);
bool has_draws = job->last_job.gpu;
+/* Bail out early when the current and new states are the same. */
+if (util_framebuffer_state_equal(>pipe_framebuffer, fb))
+return;
+
if (!ctx->wallpaper_batch && (!is_scanout || has_draws)) {
panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME);
}
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 1/9] panfrost: Make ctx->job useful
ctx->job is supposed to serve as a cache to avoid an hash table lookup
everytime we access the job attached to the currently bound FB, except
it was never assigned to anything but NULL.
Fix that by adding the missing assignment in panfrost_get_job_for_fbo().
Also add a missing NULL assignment in the ->set_framebuffer_state()
path.
While at it, add extra assert()s to make sure ctx->job is consistent.
Fixes: 59c9623d0a75 ("panfrost: Import job data structures from v3d")
Signed-off-by: Boris Brezillon
Changes in v2:
* New patch (suggested by Alyssa)
---
src/gallium/drivers/panfrost/pan_context.c | 5 +
src/gallium/drivers/panfrost/pan_job.c | 19 ++-
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/panfrost/pan_context.c
b/src/gallium/drivers/panfrost/pan_context.c
index 014f8f6a9d07..ba2df2ce66ae 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -2372,6 +2372,11 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx,
panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME);
}
+/* Invalidate the FBO job cache since we've just been assigned a new
+ * FB state.
+ */
+ctx->job = NULL;
+
util_copy_framebuffer_state(>pipe_framebuffer, fb);
/* Given that we're rendering, we'd love to have compression */
diff --git a/src/gallium/drivers/panfrost/pan_job.c
b/src/gallium/drivers/panfrost/pan_job.c
index 6339b39d29a0..cc81d475165e 100644
--- a/src/gallium/drivers/panfrost/pan_job.c
+++ b/src/gallium/drivers/panfrost/pan_job.c
@@ -23,6 +23,8 @@
*
*/
+#include
+
#include "pan_context.h"
#include "util/hash_table.h"
#include "util/ralloc.h"
@@ -124,8 +126,13 @@ panfrost_get_job_for_fbo(struct panfrost_context *ctx)
/* If we already began rendering, use that */
-if (ctx->job)
+if (ctx->job) {
+assert(ctx->job->key.zsbuf == ctx->pipe_framebuffer.zsbuf &&
+ !memcmp(ctx->job->key.cbufs,
+ ctx->pipe_framebuffer.cbufs,
+ sizeof(ctx->job->key.cbufs)));
return ctx->job;
+}
/* If not, look up the job */
@@ -133,6 +140,10 @@ panfrost_get_job_for_fbo(struct panfrost_context *ctx)
struct pipe_surface *zsbuf = ctx->pipe_framebuffer.zsbuf;
struct panfrost_job *job = panfrost_get_job(ctx, cbufs, zsbuf);
+/* Set this job as the current FBO job. Will be reset when updating the
+ * FB state and when submitting or releasing a job.
+ */
+ctx->job = job;
return job;
}
@@ -181,6 +192,12 @@ panfrost_job_submit(struct panfrost_context *ctx, struct
panfrost_job *job)
if (ret)
fprintf(stderr, "panfrost_job_submit failed: %d\n", ret);
+
+/* The job has been submitted, let's invalidate the current FBO job
+ * cache.
+*/
+assert(!ctx->job || job == ctx->job);
+ctx->job = NULL;
}
void
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallivm: fix issue with AtomicCmpXchg wrapper on llvm 3.5-3.8
Am 02.08.19 um 18:54 schrieb Brian Paul:
> On 08/02/2019 10:36 AM, srol...@vmware.com wrote:
>> From: Roland Scheidegger
>>
>> These versions still need wrapper but already have both success and
>> failure ordering.
>> (Compile tested on llvm 3.7, llvm 3.8.)
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=02
>> ---
>> src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 16 +++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> index 79d10293e80..723c84d57c2 100644
>> --- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> @@ -822,15 +822,29 @@ static llvm::AtomicOrdering
>> mapFromLLVMOrdering(LLVMAtomicOrdering Ordering) {
>> llvm_unreachable("Invalid LLVMAtomicOrdering value!");
>> }
>> +#if HAVE_LLVM < 0x305
>> LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr,
>> LLVMValueRef Cmp, LLVMValueRef New,
>> LLVMAtomicOrdering SuccessOrdering,
>> LLVMAtomicOrdering FailureOrdering,
>> LLVMBool SingleThread)
>> {
>> - /* LLVM 3.8 doesn't have a second ordering and uses old
>> SynchronizationScope enum */
>> + /* LLVM < 3.5 doesn't have a second ordering and uses old
>> SynchronizationScope enum */
>> return
>> llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr),
>> llvm::unwrap(Cmp),
>>
>> llvm::unwrap(New), mapFromLLVMOrdering(SuccessOrdering),
>>
>> SingleThread ? llvm::SynchronizationScope::SingleThread :
>> llvm::SynchronizationScope::CrossThread));
>> }
>> +#else
>> +LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr,
>> + LLVMValueRef Cmp, LLVMValueRef New,
>> + LLVMAtomicOrdering SuccessOrdering,
>> + LLVMAtomicOrdering FailureOrdering,
>> + LLVMBool SingleThread)
>> +{
>> + return
>> llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr),
>> llvm::unwrap(Cmp),
>> +
>> llvm::unwrap(New), mapFromLLVMOrdering(SuccessOrdering),
>> +
>> mapFromLLVMOrdering(FailureOrdering),
>> +
>> SingleThread ? llvm::SynchronizationScope::SingleThread :
>> llvm::SynchronizationScope::CrossThread));
>> +}
>> +#endif
>> #endif
>>
>
> Could the #if / #endif logic be moved into the body of
> LLVMBuildAtomicCmpXchg() so the whole function isn't duplicated?
Ah yes sure. Somehow I didn't think of that...
Will change this before submit.
Roland
>
> Other than that,
> Reviewed-by: Brian Paul
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallivm: fix issue with AtomicCmpXchg wrapper on llvm 3.5-3.8
On 08/02/2019 10:36 AM, srol...@vmware.com wrote:
From: Roland Scheidegger
These versions still need wrapper but already have both success and
failure ordering.
(Compile tested on llvm 3.7, llvm 3.8.)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=02
---
src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
index 79d10293e80..723c84d57c2 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
+++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
@@ -822,15 +822,29 @@ static llvm::AtomicOrdering
mapFromLLVMOrdering(LLVMAtomicOrdering Ordering) {
llvm_unreachable("Invalid LLVMAtomicOrdering value!");
}
+#if HAVE_LLVM < 0x305
LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr,
LLVMValueRef Cmp, LLVMValueRef New,
LLVMAtomicOrdering SuccessOrdering,
LLVMAtomicOrdering FailureOrdering,
LLVMBool SingleThread)
{
- /* LLVM 3.8 doesn't have a second ordering and uses old
SynchronizationScope enum */
+ /* LLVM < 3.5 doesn't have a second ordering and uses old
SynchronizationScope enum */
return llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr),
llvm::unwrap(Cmp),
llvm::unwrap(New),
mapFromLLVMOrdering(SuccessOrdering),
SingleThread ?
llvm::SynchronizationScope::SingleThread :
llvm::SynchronizationScope::CrossThread));
}
+#else
+LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr,
+LLVMValueRef Cmp, LLVMValueRef New,
+LLVMAtomicOrdering SuccessOrdering,
+LLVMAtomicOrdering FailureOrdering,
+LLVMBool SingleThread)
+{
+ return llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr),
llvm::unwrap(Cmp),
+ llvm::unwrap(New),
mapFromLLVMOrdering(SuccessOrdering),
+
mapFromLLVMOrdering(FailureOrdering),
+ SingleThread ?
llvm::SynchronizationScope::SingleThread :
llvm::SynchronizationScope::CrossThread));
+}
+#endif
#endif
Could the #if / #endif logic be moved into the body of
LLVMBuildAtomicCmpXchg() so the whole function isn't duplicated?
Other than that,
Reviewed-by: Brian Paul
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] gallivm: fix issue with AtomicCmpXchg wrapper on llvm 3.5-3.8
From: Roland Scheidegger
These versions still need wrapper but already have both success and
failure ordering.
(Compile tested on llvm 3.7, llvm 3.8.)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=02
---
src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
index 79d10293e80..723c84d57c2 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
+++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
@@ -822,15 +822,29 @@ static llvm::AtomicOrdering
mapFromLLVMOrdering(LLVMAtomicOrdering Ordering) {
llvm_unreachable("Invalid LLVMAtomicOrdering value!");
}
+#if HAVE_LLVM < 0x305
LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr,
LLVMValueRef Cmp, LLVMValueRef New,
LLVMAtomicOrdering SuccessOrdering,
LLVMAtomicOrdering FailureOrdering,
LLVMBool SingleThread)
{
- /* LLVM 3.8 doesn't have a second ordering and uses old
SynchronizationScope enum */
+ /* LLVM < 3.5 doesn't have a second ordering and uses old
SynchronizationScope enum */
return llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr),
llvm::unwrap(Cmp),
llvm::unwrap(New),
mapFromLLVMOrdering(SuccessOrdering),
SingleThread ?
llvm::SynchronizationScope::SingleThread :
llvm::SynchronizationScope::CrossThread));
}
+#else
+LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr,
+LLVMValueRef Cmp, LLVMValueRef New,
+LLVMAtomicOrdering SuccessOrdering,
+LLVMAtomicOrdering FailureOrdering,
+LLVMBool SingleThread)
+{
+ return llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr),
llvm::unwrap(Cmp),
+ llvm::unwrap(New),
mapFromLLVMOrdering(SuccessOrdering),
+
mapFromLLVMOrdering(FailureOrdering),
+ SingleThread ?
llvm::SynchronizationScope::SingleThread :
llvm::SynchronizationScope::CrossThread));
+}
+#endif
#endif
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Mesa (update-reviewers-for-vmware): i965/clear: clear_value better precision
On 2019-08-02 6:12 p.m., Brian Paul wrote: > On 08/02/2019 09:56 AM, Eric Engestrom wrote: >> On Friday, 2019-08-02 17:50:17 +0200, Michel Dänzer wrote: >>> On 2019-08-02 5:37 p.m., Brian Paul wrote: Ugh, I didn't mean to do this. I'm trying to figure out how to make a merge request with gitlab. >>> >>> Just push to a branch in your personal repository, and the output of git >>> push contains an URL for creating an MR for it. >> >> Precisely, but just to be extra clear: your personal repo needs to be >> forked from the main mesa repo [1], not just "a repo containing the mesa >> git history". >> GitLab needs to know the two are linked and pressing that "fork" button >> is how you tell it. > > Yeah, I just figured that out a few minutes ago. After I figure out all > the detailed steps from scratch I'll add it to the documentation. > > I've really never done anything with gitlab, github, etc. and have been > busy with non-Mesa work for over a year now. I have a lot of catch-up > to do. It sounds like you're doing fine so far considering that! Don't hesitate to ask if you run into any other issues. -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Mesa (update-reviewers-for-vmware): i965/clear: clear_value better precision
On 08/02/2019 09:56 AM, Eric Engestrom wrote: On Friday, 2019-08-02 17:50:17 +0200, Michel Dänzer wrote: On 2019-08-02 5:37 p.m., Brian Paul wrote: Ugh, I didn't mean to do this. I'm trying to figure out how to make a merge request with gitlab. Just push to a branch in your personal repository, and the output of git push contains an URL for creating an MR for it. Precisely, but just to be extra clear: your personal repo needs to be forked from the main mesa repo [1], not just "a repo containing the mesa git history". GitLab needs to know the two are linked and pressing that "fork" button is how you tell it. Yeah, I just figured that out a few minutes ago. After I figure out all the detailed steps from scratch I'll add it to the documentation. I've really never done anything with gitlab, github, etc. and have been busy with non-Mesa work for over a year now. I have a lot of catch-up to do. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Mesa (update-reviewers-for-vmware): i965/clear: clear_value better precision
On Friday, 2019-08-02 17:50:17 +0200, Michel Dänzer wrote: > On 2019-08-02 5:37 p.m., Brian Paul wrote: > > Ugh, I didn't mean to do this. I'm trying to figure out how to make a > > merge request with gitlab. > > Just push to a branch in your personal repository, and the output of git > push contains an URL for creating an MR for it. Precisely, but just to be extra clear: your personal repo needs to be forked from the main mesa repo [1], not just "a repo containing the mesa git history". GitLab needs to know the two are linked and pressing that "fork" button is how you tell it. [1] https://gitlab.freedesktop.org/mesa/mesa ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Mesa (update-reviewers-for-vmware): i965/clear: clear_value better precision
On 2019-08-02 5:37 p.m., Brian Paul wrote: > Ugh, I didn't mean to do this. I'm trying to figure out how to make a > merge request with gitlab. Just push to a branch in your personal repository, and the output of git push contains an URL for creating an MR for it. -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 8/9] panfrost: Handle the bo == NULL case in panfrost_bo_[un]reference()
> Was hitting a NULL pointer dereference when on the
> panfrost_bo_unreference(job->polygon_list) done in panfrost_free_job().
> Don't know having a NULL polygon_list is valid case (I guess that's
> possible for a job that does not involve the tiler) but I was facing
> this problem when running the deqp testsuite.
Ah, yeah. I'm not sure we actually hit that case -- and I don't know why
we would in ES2 -- but it can certainly happen in ES3. Compute-like
workloads (both honest-to-goodness compute shaders as well as rasterizer
discard to facilitate transform feedback) don't touch the tiler and thus
don't inherently need the polygon list. In OpenCL, there's no
framebuffer at all, right? :)
(There is still a stub SFBD allocated but whatever)
> >
> > On Fri, Aug 02, 2019 at 12:12:56PM +0200, Boris Brezillon wrote:
> > > Allows us to pass BOs without checking if they're NULL or not.
> > >
> > > Signed-off-by: Boris Brezillon
> > > ---
> > > src/gallium/drivers/panfrost/pan_resource.c | 6 +-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/gallium/drivers/panfrost/pan_resource.c
> > > b/src/gallium/drivers/panfrost/pan_resource.c
> > > index 71da383d4c7a..f74a39555b45 100644
> > > --- a/src/gallium/drivers/panfrost/pan_resource.c
> > > +++ b/src/gallium/drivers/panfrost/pan_resource.c
> > > @@ -443,12 +443,16 @@ panfrost_resource_create(struct pipe_screen *screen,
> > > void
> > > panfrost_bo_reference(struct panfrost_bo *bo)
> > > {
> > > -pipe_reference(NULL, >reference);
> > > +if (bo)
> > > +pipe_reference(NULL, >reference);
> > > }
> > >
> > > void
> > > panfrost_bo_unreference(struct pipe_screen *screen, struct panfrost_bo
> > > *bo)
> > > {
> > > +if (!bo)
> > > +return;
> > > +
> > > /* When the reference count goes to zero, we need to cleanup */
> > >
> > > if (pipe_reference(>reference, NULL))
> > > --
> > > 2.21.0
> > >
>
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Mesa (update-reviewers-for-vmware): i965/clear: clear_value better precision
Ugh, I didn't mean to do this. I'm trying to figure out how to make a merge request with gitlab. -Brian On 08/02/2019 09:35 AM, GitLab Mirror wrote: Module: Mesa Branch: update-reviewers-for-vmware Commit: a86eccfb78092493b3999849db62613838951756 URL: https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcgit.freedesktop.org%2Fmesa%2Fmesa%2Fcommit%2F%3Fid%3Da86eccfb78092493b3999849db62613838951756data=02%7C01%7Cbrianp%40vmware.com%7C6aa9bf59501d420b03af08d7175f0b0f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637003569293293608sdata=bLEb7XY3mLFAkME7v8QJ0GugG%2FbzFoNONSSGTjKSfFs%3Dreserved=0 Author: Sergii Romantsov Date: Fri Jul 12 16:46:45 2019 +0300 i965/clear: clear_value better precision Test-case with depth-clear 0.5 and format MESA_FORMAT_Z24_UNORM_X8_UINT fails due inconsistent clear-value of 0.497. Maybe its better to improve? CC: Jason Ekstrand Fixes: 0ae9ce0f29ea (i965/clear: Quantize the depth clear value based on the format) Bugzilla: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.freedesktop.org%2Fshow_bug.cgi%3Fid%3D13data=02%7C01%7Cbrianp%40vmware.com%7C6aa9bf59501d420b03af08d7175f0b0f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637003569293293608sdata=OD1YuhGY%2B4IZG7g8PMh%2Bk3amc6O9HjDV92pOFPd8RlE%3Dreserved=0 Signed-off-by: Sergii Romantsov Signed-off-by: Danylo Piliaiev Reviewed-by: Jason Ekstrand --- src/mesa/drivers/dri/i965/brw_clear.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/brw_clear.c b/src/mesa/drivers/dri/i965/brw_clear.c index 30e09861491..1508171da10 100644 --- a/src/mesa/drivers/dri/i965/brw_clear.c +++ b/src/mesa/drivers/dri/i965/brw_clear.c @@ -167,7 +167,7 @@ brw_fast_clear_depth(struct gl_context *ctx) */ float clear_value = mt->format == MESA_FORMAT_Z_FLOAT32 ? ctx->Depth.Clear : - (unsigned)(ctx->Depth.Clear * fb->_DepthMax) / (float)fb->_DepthMax; + _mesa_lroundeven(ctx->Depth.Clear * fb->_DepthMax) / (float)(fb->_DepthMax); const uint32_t num_layers = depth_att->Layered ? depth_irb->layer_count : 1; ___ mesa-commit mailing list mesa-com...@lists.freedesktop.org https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-commitdata=02%7C01%7Cbrianp%40vmware.com%7C6aa9bf59501d420b03af08d7175f0b0f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637003569293293608sdata=UWAkli3USsLDpnoElsuycRJ8KTrJV%2FC0r%2FarUay7SNQ%3Dreserved=0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/9] panfrost: Remove job from ctx->jobs at submission time
> Note that before this patch, someone trying to get a new job for an FB > that already has a job running would actually get the old job object > and might start messing up with it. Not sure that's what we want. I don't think that's what we want either, and I think I took pains to ensure this would never happen (at the expense of perf). I recall reading the blob will just keep rendering to future frames speculatively "until SurfaceFlinger stops giving us framebuffers", so that matches moreso the behaviour you're adding. On further thought, I think this is R-b, thank you :) > > > > On Fri, Aug 02, 2019 at 12:12:50PM +0200, Boris Brezillon wrote: > > > This guarantees that new draws targetting the same framebuffer will > > > get a new job instance. > > > > > > Signed-off-by: Boris Brezillon > > > --- > > > src/gallium/drivers/panfrost/pan_job.c | 8 > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/src/gallium/drivers/panfrost/pan_job.c > > > b/src/gallium/drivers/panfrost/pan_job.c > > > index 960c8556e2f0..d2a4c8c3c600 100644 > > > --- a/src/gallium/drivers/panfrost/pan_job.c > > > +++ b/src/gallium/drivers/panfrost/pan_job.c > > > @@ -173,6 +173,14 @@ panfrost_job_submit(struct panfrost_context *ctx, > > > struct panfrost_job *job) > > > > > > if (ret) > > > fprintf(stderr, "panfrost_job_submit failed: %d\n", ret); > > > + > > > +/* Remove the job from the ctx->jobs set so that future > > > + * panfrost_get_job() calls don't see it. > > > + * We must reset the job key to avoid removing another valid > > > entry when > > > + * the job is freed. > > > + */ > > > +_mesa_hash_table_remove_key(ctx->jobs, >key); > > > +memset(>key, 0, sizeof(job->key)); > > > } > > > > > > void > > > -- > > > 2.21.0 > > > > signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/9] panfrost: Don't emit a new FB desc when setting a new FB state
On Fri, 2 Aug 2019 08:12:15 -0700 Alyssa Rosenzweig wrote: > So, in the future, we'll want to have multiple jobs for different > framebuffers independently in flight at the same time. As an > illustrative example, we would like the app to be able to do a sequence > like: > > bind scanout > clear > draw something > bind FBO > clear > draw something > bind scanout > draw something else > bind FBO > draw something else > bind scanout > draw the fbo > flush > > Ideally, nothing should happen until the final flush. So the order > submitted to the hardware would be: > > FBO: > clear > draw something > draw something else > draw something else > > Scanout: > clear > draw something > draw something else > draw the fbo > > Unfortunately, right now that panfrost_flush() call makes the order > instead: > > Scanout: > clear > draw something > > FBO: > clear > draw something > > Scanout: > clear > wallpaper scanout > draw something else > > FBO: > clear > wallpaper fbo > draw something else > > Scanout: > clear > wallpaper scanout > draw FBO > > Clearly this is extremely suboptimal (I believe the extra wallpaper > blits are related to our poor performance on FBO-heavy apps. glmark's > -bdesktop and -bterrain are suspects here.) > > The solution is to be able to have switching framebuffers be a no-op, > since all the important stuff is in the panfrost_job/batch. So > panfrost_set_framebuffer_state is just doing the copy, but no flush and > almost no logic, so we can switch back and forth freely without > incurring a flush/clear/wallpaper cycle each time. Sounds quite interesting :-). > > This patch series is not the right time to focus on pipelined > performance (though I'd love if somebody gave it some love at some > point). But I do want to caution from a code smell that will make > whoever that someone is -- quite possibly you! -- quite grumpy when they > work on this in future. Yep, I can imagine this will lead to some heavy refactoring. > > On Fri, Aug 02, 2019 at 12:12:54PM +0200, Boris Brezillon wrote: > > The FB desc will be emitted/attached on the first draw targetting > > this new FB. > > > > Signed-off-by: Boris Brezillon > > --- > > src/gallium/drivers/panfrost/pan_context.c | 6 +- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/src/gallium/drivers/panfrost/pan_context.c > > b/src/gallium/drivers/panfrost/pan_context.c > > index 1091caeb1148..2b7906eea155 100644 > > --- a/src/gallium/drivers/panfrost/pan_context.c > > +++ b/src/gallium/drivers/panfrost/pan_context.c > > @@ -2384,6 +2384,9 @@ panfrost_set_framebuffer_state(struct pipe_context > > *pctx, > > > > if (!is_scanout || has_draws) > > panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME); > > +else > > + > > assert(!ctx->payloads[PIPE_SHADER_VERTEX].postfix.framebuffer && > > + > > !ctx->payloads[PIPE_SHADER_FRAGMENT].postfix.framebuffer); > > > > util_copy_framebuffer_state(>pipe_framebuffer, fb); > > > > @@ -2391,7 +2394,8 @@ panfrost_set_framebuffer_state(struct pipe_context > > *pctx, > > struct panfrost_screen *screen = pan_screen(ctx->base.screen); > > > > panfrost_hint_afbc(screen, >pipe_framebuffer); > > -panfrost_attach_vt_framebuffer(ctx, false); > > +for (unsigned i = 0; i < PIPE_SHADER_TYPES; ++i) > > +ctx->payloads[i].postfix.framebuffer = 0; > > } > > > > static void * > > -- > > 2.21.0 > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/9] panfrost: Get rid of ctx->job
On Fri, 2 Aug 2019 07:59:02 -0700
Alyssa Rosenzweig wrote:
> Theoretically we were supposed to set ctx->job to the current job, I
> guess cacheing the result of panfrost_get_job_for_fbo()
>
> See v3d's implementation which this is a carbon clone of.
Okay, so it's something we want to have to avoid the search into the
hash table? If we do that, we should at least check that the keys match
before returning the current job.
>
> On Fri, Aug 02, 2019 at 12:12:49PM +0200, Boris Brezillon wrote:
> > This field is never set to anything but NULL, which means the test
> > done in panfrost_free_job() and panfrost_get_job_for_fbo() will always
> > evaluate to false. Let's get rid of this field.
> >
> > Signed-off-by: Boris Brezillon
> > ---
> > src/gallium/drivers/panfrost/pan_context.h | 3 +--
> > src/gallium/drivers/panfrost/pan_job.c | 8
> > 2 files changed, 1 insertion(+), 10 deletions(-)
> >
> > diff --git a/src/gallium/drivers/panfrost/pan_context.h
> > b/src/gallium/drivers/panfrost/pan_context.h
> > index d930f12a537b..a90dbb04e833 100644
> > --- a/src/gallium/drivers/panfrost/pan_context.h
> > +++ b/src/gallium/drivers/panfrost/pan_context.h
> > @@ -95,8 +95,7 @@ struct panfrost_context {
> > /* Compiler context */
> > struct midgard_screen compiler;
> >
> > -/* Bound job and map of panfrost_job_key to jobs */
> > -struct panfrost_job *job;
> > +/* Map of panfrost_job_key to jobs */
> > struct hash_table *jobs;
> >
> > /* panfrost_resource -> panfrost_job */
> > diff --git a/src/gallium/drivers/panfrost/pan_job.c
> > b/src/gallium/drivers/panfrost/pan_job.c
> > index 6339b39d29a0..960c8556e2f0 100644
> > --- a/src/gallium/drivers/panfrost/pan_job.c
> > +++ b/src/gallium/drivers/panfrost/pan_job.c
> > @@ -72,9 +72,6 @@ panfrost_free_job(struct panfrost_context *ctx, struct
> > panfrost_job *job)
> >
> > _mesa_hash_table_remove_key(ctx->jobs, >key);
> >
> > -if (ctx->job == job)
> > -ctx->job = NULL;
> > -
> > ralloc_free(job);
> > }
> >
> > @@ -122,11 +119,6 @@ panfrost_get_job_for_fbo(struct panfrost_context *ctx)
> > if (ctx->wallpaper_batch)
> > return ctx->wallpaper_batch;
> >
> > -/* If we already began rendering, use that */
> > -
> > -if (ctx->job)
> > -return ctx->job;
> > -
> > /* If not, look up the job */
> >
> > struct pipe_surface **cbufs = ctx->pipe_framebuffer.cbufs;
> > --
> > 2.21.0
> >
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 8/9] panfrost: Handle the bo == NULL case in panfrost_bo_[un]reference()
On Fri, 2 Aug 2019 08:14:05 -0700
Alyssa Rosenzweig wrote:
> Is there any cleanup we can simultaneously? (Where the check was done
> outside?) Or is this a futureproofing?
Was hitting a NULL pointer dereference when on the
panfrost_bo_unreference(job->polygon_list) done in panfrost_free_job().
Don't know having a NULL polygon_list is valid case (I guess that's
possible for a job that does not involve the tiler) but I was facing
this problem when running the deqp testsuite.
>
> On Fri, Aug 02, 2019 at 12:12:56PM +0200, Boris Brezillon wrote:
> > Allows us to pass BOs without checking if they're NULL or not.
> >
> > Signed-off-by: Boris Brezillon
> > ---
> > src/gallium/drivers/panfrost/pan_resource.c | 6 +-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/gallium/drivers/panfrost/pan_resource.c
> > b/src/gallium/drivers/panfrost/pan_resource.c
> > index 71da383d4c7a..f74a39555b45 100644
> > --- a/src/gallium/drivers/panfrost/pan_resource.c
> > +++ b/src/gallium/drivers/panfrost/pan_resource.c
> > @@ -443,12 +443,16 @@ panfrost_resource_create(struct pipe_screen *screen,
> > void
> > panfrost_bo_reference(struct panfrost_bo *bo)
> > {
> > -pipe_reference(NULL, >reference);
> > +if (bo)
> > +pipe_reference(NULL, >reference);
> > }
> >
> > void
> > panfrost_bo_unreference(struct pipe_screen *screen, struct panfrost_bo *bo)
> > {
> > +if (!bo)
> > +return;
> > +
> > /* When the reference count goes to zero, we need to cleanup */
> >
> > if (pipe_reference(>reference, NULL))
> > --
> > 2.21.0
> >
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111272] [DRI_PRIME] Error on multi GPU with only one enabled
https://bugs.freedesktop.org/show_bug.cgi?id=111272 Michel Dänzer changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTABUG -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 9/9] panfrost: Allocate polygon lists on-demand
R-b, but I guess I'm biased :) signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/9] panfrost: Remove job from ctx->jobs at submission time
On Fri, 2 Aug 2019 08:00:28 -0700 Alyssa Rosenzweig wrote: > What happens if the CPU creates jobs significiantly faster than the GPU > processes them? Could you have four jobs for the same framebuffer in > flight at once? That's probably what will happen, yes. > > At present, we do some heavy flushing so "no" but in the future we'll > want to lax up for performance. Limiting the number of in-flight jobs per FB would require some kind of per-FB jobs tracking. But let's assume we want to limit ourselves to one in-flight job per FB at first and keep in-flight jobs in the ctx->jobs set. We'd need to add some kind of job->in_flight state that we set to true when submitting, and then wait for the job fence to be signaled before allocating a new job. Note that before this patch, someone trying to get a new job for an FB that already has a job running would actually get the old job object and might start messing up with it. Not sure that's what we want. > > On Fri, Aug 02, 2019 at 12:12:50PM +0200, Boris Brezillon wrote: > > This guarantees that new draws targetting the same framebuffer will > > get a new job instance. > > > > Signed-off-by: Boris Brezillon > > --- > > src/gallium/drivers/panfrost/pan_job.c | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/src/gallium/drivers/panfrost/pan_job.c > > b/src/gallium/drivers/panfrost/pan_job.c > > index 960c8556e2f0..d2a4c8c3c600 100644 > > --- a/src/gallium/drivers/panfrost/pan_job.c > > +++ b/src/gallium/drivers/panfrost/pan_job.c > > @@ -173,6 +173,14 @@ panfrost_job_submit(struct panfrost_context *ctx, > > struct panfrost_job *job) > > > > if (ret) > > fprintf(stderr, "panfrost_job_submit failed: %d\n", ret); > > + > > +/* Remove the job from the ctx->jobs set so that future > > + * panfrost_get_job() calls don't see it. > > + * We must reset the job key to avoid removing another valid entry > > when > > + * the job is freed. > > + */ > > +_mesa_hash_table_remove_key(ctx->jobs, >key); > > +memset(>key, 0, sizeof(job->key)); > > } > > > > void > > -- > > 2.21.0 > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 8/9] panfrost: Handle the bo == NULL case in panfrost_bo_[un]reference()
Is there any cleanup we can simultaneously? (Where the check was done
outside?) Or is this a futureproofing?
On Fri, Aug 02, 2019 at 12:12:56PM +0200, Boris Brezillon wrote:
> Allows us to pass BOs without checking if they're NULL or not.
>
> Signed-off-by: Boris Brezillon
> ---
> src/gallium/drivers/panfrost/pan_resource.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/panfrost/pan_resource.c
> b/src/gallium/drivers/panfrost/pan_resource.c
> index 71da383d4c7a..f74a39555b45 100644
> --- a/src/gallium/drivers/panfrost/pan_resource.c
> +++ b/src/gallium/drivers/panfrost/pan_resource.c
> @@ -443,12 +443,16 @@ panfrost_resource_create(struct pipe_screen *screen,
> void
> panfrost_bo_reference(struct panfrost_bo *bo)
> {
> -pipe_reference(NULL, >reference);
> +if (bo)
> +pipe_reference(NULL, >reference);
> }
>
> void
> panfrost_bo_unreference(struct pipe_screen *screen, struct panfrost_bo *bo)
> {
> +if (!bo)
> +return;
> +
> /* When the reference count goes to zero, we need to cleanup */
>
> if (pipe_reference(>reference, NULL))
> --
> 2.21.0
>
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 7/9] panfrost: Get rid of the skippable param in attach_vt_framebuffer()
R-b, I think.
On Fri, Aug 02, 2019 at 12:12:55PM +0200, Boris Brezillon wrote:
> The only user of this function always passes true.
>
> Signed-off-by: Boris Brezillon
> ---
> src/gallium/drivers/panfrost/pan_context.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/pan_context.c
> b/src/gallium/drivers/panfrost/pan_context.c
> index 2b7906eea155..e781d809812b 100644
> --- a/src/gallium/drivers/panfrost/pan_context.c
> +++ b/src/gallium/drivers/panfrost/pan_context.c
> @@ -203,11 +203,11 @@ panfrost_attach_vt_sfbd(struct panfrost_context *ctx)
> }
>
> static void
> -panfrost_attach_vt_framebuffer(struct panfrost_context *ctx, bool skippable)
> +panfrost_attach_vt_framebuffer(struct panfrost_context *ctx)
> {
> /* Skip the attach if we can */
>
> -if (skippable &&
> ctx->payloads[PIPE_SHADER_VERTEX].postfix.framebuffer) {
> +if (ctx->payloads[PIPE_SHADER_VERTEX].postfix.framebuffer) {
>
> assert(ctx->payloads[PIPE_SHADER_FRAGMENT].postfix.framebuffer);
> return;
> }
> @@ -1013,7 +1013,7 @@ panfrost_emit_for_draw(struct panfrost_context *ctx,
> bool with_vertex_data)
> struct panfrost_job *job = panfrost_get_job_for_fbo(ctx);
> struct panfrost_screen *screen = pan_screen(ctx->base.screen);
>
> -panfrost_attach_vt_framebuffer(ctx, true);
> +panfrost_attach_vt_framebuffer(ctx);
>
> if (with_vertex_data) {
> panfrost_emit_vertex_data(job);
> --
> 2.21.0
>
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/9] panfrost: Don't emit a new FB desc when setting a new FB state
So, in the future, we'll want to have multiple jobs for different framebuffers independently in flight at the same time. As an illustrative example, we would like the app to be able to do a sequence like: bind scanout clear draw something bind FBO clear draw something bind scanout draw something else bind FBO draw something else bind scanout draw the fbo flush Ideally, nothing should happen until the final flush. So the order submitted to the hardware would be: FBO: clear draw something draw something else draw something else Scanout: clear draw something draw something else draw the fbo Unfortunately, right now that panfrost_flush() call makes the order instead: Scanout: clear draw something FBO: clear draw something Scanout: clear wallpaper scanout draw something else FBO: clear wallpaper fbo draw something else Scanout: clear wallpaper scanout draw FBO Clearly this is extremely suboptimal (I believe the extra wallpaper blits are related to our poor performance on FBO-heavy apps. glmark's -bdesktop and -bterrain are suspects here.) The solution is to be able to have switching framebuffers be a no-op, since all the important stuff is in the panfrost_job/batch. So panfrost_set_framebuffer_state is just doing the copy, but no flush and almost no logic, so we can switch back and forth freely without incurring a flush/clear/wallpaper cycle each time. This patch series is not the right time to focus on pipelined performance (though I'd love if somebody gave it some love at some point). But I do want to caution from a code smell that will make whoever that someone is -- quite possibly you! -- quite grumpy when they work on this in future. On Fri, Aug 02, 2019 at 12:12:54PM +0200, Boris Brezillon wrote: > The FB desc will be emitted/attached on the first draw targetting > this new FB. > > Signed-off-by: Boris Brezillon > --- > src/gallium/drivers/panfrost/pan_context.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/gallium/drivers/panfrost/pan_context.c > b/src/gallium/drivers/panfrost/pan_context.c > index 1091caeb1148..2b7906eea155 100644 > --- a/src/gallium/drivers/panfrost/pan_context.c > +++ b/src/gallium/drivers/panfrost/pan_context.c > @@ -2384,6 +2384,9 @@ panfrost_set_framebuffer_state(struct pipe_context > *pctx, > > if (!is_scanout || has_draws) > panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME); > +else > + > assert(!ctx->payloads[PIPE_SHADER_VERTEX].postfix.framebuffer && > + > !ctx->payloads[PIPE_SHADER_FRAGMENT].postfix.framebuffer); > > util_copy_framebuffer_state(>pipe_framebuffer, fb); > > @@ -2391,7 +2394,8 @@ panfrost_set_framebuffer_state(struct pipe_context > *pctx, > struct panfrost_screen *screen = pan_screen(ctx->base.screen); > > panfrost_hint_afbc(screen, >pipe_framebuffer); > -panfrost_attach_vt_framebuffer(ctx, false); > +for (unsigned i = 0; i < PIPE_SHADER_TYPES; ++i) > +ctx->payloads[i].postfix.framebuffer = 0; > } > > static void * > -- > 2.21.0 > signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gitlab-ci: remove software-properties-common
On 2019-08-01 6:51 p.m., Emil Velikov wrote: > On Thu, 1 Aug 2019 at 16:03, Emil Velikov wrote: >> >> On Thu, 1 Aug 2019 at 14:26, Michel Dänzer wrote: >>> On 2019-08-01 2:32 p.m., Emil Velikov wrote: >> Sure I'll do that in a moment. >>> >>> Why don't you just follow my suggestion above? >>> >> That's what I was planning to do :-) >> > Done and dusted. Thanks for the help and extensive how-to, I've > followed it literally. Thanks! Just a detail: None of the pipelines referenced on the MR page actually generated the docker image. Looking at https://gitlab.freedesktop.org/evelikov/mesa/pipelines , I guess the 2019-08-01 image already existed in your registry from when you pushed https://gitlab.freedesktop.org/evelikov/mesa/commit/ab218119ae865d0a3b11d8753e9eada828b8a9b8 , so either that shouldn't have been pushed, or the image removed again before creating the MR. Something to perfect next time. :) -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/9] panfrost: Bail out early when doing a wallpaper blit
R-b, thank you especially for the comment deconstructing this magic.
On Fri, Aug 02, 2019 at 12:12:53PM +0200, Boris Brezillon wrote:
> The wallpaper blit is a bit special in that the operation is targetting
> the current FB, but the u_blitter logic creates a new surface for it
> which makes util_framebuffer_state_equal() return false. In that case
> we don't want a new FB descriptor to be emitted/attached, so let's just
> copy the new state into ctx->pipe_framebuffer and exit the function.
>
> Signed-off-by: Boris Brezillon
> ---
> src/gallium/drivers/panfrost/pan_context.c | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/pan_context.c
> b/src/gallium/drivers/panfrost/pan_context.c
> index d442ae1f2433..1091caeb1148 100644
> --- a/src/gallium/drivers/panfrost/pan_context.c
> +++ b/src/gallium/drivers/panfrost/pan_context.c
> @@ -2369,10 +2369,22 @@ panfrost_set_framebuffer_state(struct pipe_context
> *pctx,
> if (util_framebuffer_state_equal(>pipe_framebuffer, fb))
> return;
>
> -if (!ctx->wallpaper_batch && (!is_scanout || has_draws)) {
> -panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME);
> +/* The wallpaper logic sets a new FB state before doing the blit and
> + * restore the old one when it's done. Those FB states are reported
> to
> + * be different because the surface they are pointing to are
> different,
> + * but those surfaces actually point to the same cbufs/zbufs. In that
> + * case we definitely don't want new FB descs to be emitted/attached
> + * since the job is expected to be flushed just after the blit is
> done,
> + * so let's just copy the new state and return here.
> + */
> +if (ctx->wallpaper_batch) {
> +util_copy_framebuffer_state(>pipe_framebuffer, fb);
> +return;
> }
>
> +if (!is_scanout || has_draws)
> +panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME);
> +
> util_copy_framebuffer_state(>pipe_framebuffer, fb);
>
> /* Given that we're rendering, we'd love to have compression */
> --
> 2.21.0
>
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] panfrost: Bail out early when new and current FB states are equal
R-b
On Fri, Aug 02, 2019 at 12:12:52PM +0200, Boris Brezillon wrote:
> If the current FB matches the new one there's nothing to be done in
> panfrost_set_framebuffer_state(). By bailing out early in that case we
> avoid emitting new FB descriptors (the old ones are still valid).
>
> Signed-off-by: Boris Brezillon
> ---
> src/gallium/drivers/panfrost/pan_context.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/src/gallium/drivers/panfrost/pan_context.c
> b/src/gallium/drivers/panfrost/pan_context.c
> index b63023a16cda..d442ae1f2433 100644
> --- a/src/gallium/drivers/panfrost/pan_context.c
> +++ b/src/gallium/drivers/panfrost/pan_context.c
> @@ -2365,6 +2365,10 @@ panfrost_set_framebuffer_state(struct pipe_context
> *pctx,
> bool is_scanout = panfrost_is_scanout(ctx);
> bool has_draws = job->last_job.gpu;
>
> +/* Bail out early when the current and new states are the same. */
> +if (util_framebuffer_state_equal(>pipe_framebuffer, fb))
> +return;
> +
> if (!ctx->wallpaper_batch && (!is_scanout || has_draws)) {
> panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME);
> }
> --
> 2.21.0
>
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/9] panfrost: Delay FB descriptor allocation
R-b, thank you :)
On Fri, Aug 02, 2019 at 12:12:51PM +0200, Boris Brezillon wrote:
> No need to emit SFBD/MFBD at frame invalidation. They can be emitted
> when the framebuffer is attached, which saves us a potential FB desc
> re-allocation if a new FB is bound after the swap.
>
> Signed-off-by: Boris Brezillon
> ---
> src/gallium/drivers/panfrost/pan_context.c | 21 ++---
> src/gallium/drivers/panfrost/pan_context.h | 3 ---
> 2 files changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/pan_context.c
> b/src/gallium/drivers/panfrost/pan_context.c
> index 014f8f6a9d07..b63023a16cda 100644
> --- a/src/gallium/drivers/panfrost/pan_context.c
> +++ b/src/gallium/drivers/panfrost/pan_context.c
> @@ -189,13 +189,17 @@ panfrost_clear(
> static mali_ptr
> panfrost_attach_vt_mfbd(struct panfrost_context *ctx)
> {
> -return panfrost_upload_transient(ctx, >vt_framebuffer_mfbd,
> sizeof(ctx->vt_framebuffer_mfbd)) | MALI_MFBD;
> +struct bifrost_framebuffer mfbd = panfrost_emit_mfbd(ctx, ~0);
> +
> +return panfrost_upload_transient(ctx, , sizeof(mfbd)) |
> MALI_MFBD;
> }
>
> static mali_ptr
> panfrost_attach_vt_sfbd(struct panfrost_context *ctx)
> {
> -return panfrost_upload_transient(ctx, >vt_framebuffer_sfbd,
> sizeof(ctx->vt_framebuffer_sfbd)) | MALI_SFBD;
> +struct mali_single_framebuffer sfbd = panfrost_emit_sfbd(ctx, ~0);
> +
> +return panfrost_upload_transient(ctx, , sizeof(sfbd)) |
> MALI_SFBD;
> }
>
> static void
> @@ -223,13 +227,6 @@ panfrost_attach_vt_framebuffer(struct panfrost_context
> *ctx, bool skippable)
> static void
> panfrost_invalidate_frame(struct panfrost_context *ctx)
> {
> -struct panfrost_screen *screen = pan_screen(ctx->base.screen);
> -
> -if (screen->require_sfbd)
> -ctx->vt_framebuffer_sfbd = panfrost_emit_sfbd(ctx, ~0);
> -else
> -ctx->vt_framebuffer_mfbd = panfrost_emit_mfbd(ctx, ~0);
> -
> for (unsigned i = 0; i < PIPE_SHADER_TYPES; ++i)
> ctx->payloads[i].postfix.framebuffer = 0;
>
> @@ -2378,12 +2375,6 @@ panfrost_set_framebuffer_state(struct pipe_context
> *pctx,
> struct panfrost_screen *screen = pan_screen(ctx->base.screen);
>
> panfrost_hint_afbc(screen, >pipe_framebuffer);
> -
> -if (screen->require_sfbd)
> -ctx->vt_framebuffer_sfbd = panfrost_emit_sfbd(ctx, ~0);
> -else
> -ctx->vt_framebuffer_mfbd = panfrost_emit_mfbd(ctx, ~0);
> -
> panfrost_attach_vt_framebuffer(ctx, false);
> }
>
> diff --git a/src/gallium/drivers/panfrost/pan_context.h
> b/src/gallium/drivers/panfrost/pan_context.h
> index a90dbb04e833..ac4b21678e65 100644
> --- a/src/gallium/drivers/panfrost/pan_context.h
> +++ b/src/gallium/drivers/panfrost/pan_context.h
> @@ -137,9 +137,6 @@ struct panfrost_context {
>
> union mali_attr attributes[PIPE_MAX_ATTRIBS];
>
> -struct mali_single_framebuffer vt_framebuffer_sfbd;
> -struct bifrost_framebuffer vt_framebuffer_mfbd;
> -
> /* TODO: Multiple uniform buffers (index =/= 0), finer updates? */
>
> struct panfrost_constant_buffer constant_buffer[PIPE_SHADER_TYPES];
> --
> 2.21.0
>
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/9] panfrost: Remove job from ctx->jobs at submission time
What happens if the CPU creates jobs significiantly faster than the GPU processes them? Could you have four jobs for the same framebuffer in flight at once? At present, we do some heavy flushing so "no" but in the future we'll want to lax up for performance. On Fri, Aug 02, 2019 at 12:12:50PM +0200, Boris Brezillon wrote: > This guarantees that new draws targetting the same framebuffer will > get a new job instance. > > Signed-off-by: Boris Brezillon > --- > src/gallium/drivers/panfrost/pan_job.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/src/gallium/drivers/panfrost/pan_job.c > b/src/gallium/drivers/panfrost/pan_job.c > index 960c8556e2f0..d2a4c8c3c600 100644 > --- a/src/gallium/drivers/panfrost/pan_job.c > +++ b/src/gallium/drivers/panfrost/pan_job.c > @@ -173,6 +173,14 @@ panfrost_job_submit(struct panfrost_context *ctx, struct > panfrost_job *job) > > if (ret) > fprintf(stderr, "panfrost_job_submit failed: %d\n", ret); > + > +/* Remove the job from the ctx->jobs set so that future > + * panfrost_get_job() calls don't see it. > + * We must reset the job key to avoid removing another valid entry > when > + * the job is freed. > + */ > +_mesa_hash_table_remove_key(ctx->jobs, >key); > +memset(>key, 0, sizeof(job->key)); > } > > void > -- > 2.21.0 > signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/9] panfrost: Get rid of ctx->job
Theoretically we were supposed to set ctx->job to the current job, I
guess cacheing the result of panfrost_get_job_for_fbo()
See v3d's implementation which this is a carbon clone of.
On Fri, Aug 02, 2019 at 12:12:49PM +0200, Boris Brezillon wrote:
> This field is never set to anything but NULL, which means the test
> done in panfrost_free_job() and panfrost_get_job_for_fbo() will always
> evaluate to false. Let's get rid of this field.
>
> Signed-off-by: Boris Brezillon
> ---
> src/gallium/drivers/panfrost/pan_context.h | 3 +--
> src/gallium/drivers/panfrost/pan_job.c | 8
> 2 files changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/pan_context.h
> b/src/gallium/drivers/panfrost/pan_context.h
> index d930f12a537b..a90dbb04e833 100644
> --- a/src/gallium/drivers/panfrost/pan_context.h
> +++ b/src/gallium/drivers/panfrost/pan_context.h
> @@ -95,8 +95,7 @@ struct panfrost_context {
> /* Compiler context */
> struct midgard_screen compiler;
>
> -/* Bound job and map of panfrost_job_key to jobs */
> -struct panfrost_job *job;
> +/* Map of panfrost_job_key to jobs */
> struct hash_table *jobs;
>
> /* panfrost_resource -> panfrost_job */
> diff --git a/src/gallium/drivers/panfrost/pan_job.c
> b/src/gallium/drivers/panfrost/pan_job.c
> index 6339b39d29a0..960c8556e2f0 100644
> --- a/src/gallium/drivers/panfrost/pan_job.c
> +++ b/src/gallium/drivers/panfrost/pan_job.c
> @@ -72,9 +72,6 @@ panfrost_free_job(struct panfrost_context *ctx, struct
> panfrost_job *job)
>
> _mesa_hash_table_remove_key(ctx->jobs, >key);
>
> -if (ctx->job == job)
> -ctx->job = NULL;
> -
> ralloc_free(job);
> }
>
> @@ -122,11 +119,6 @@ panfrost_get_job_for_fbo(struct panfrost_context *ctx)
> if (ctx->wallpaper_batch)
> return ctx->wallpaper_batch;
>
> -/* If we already began rendering, use that */
> -
> -if (ctx->job)
> -return ctx->job;
> -
> /* If not, look up the job */
>
> struct pipe_surface **cbufs = ctx->pipe_framebuffer.cbufs;
> --
> 2.21.0
>
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111272] [DRI_PRIME] Error on multi GPU with only one enabled
https://bugs.freedesktop.org/show_bug.cgi?id=111272 --- Comment #15 from Parker Reed --- Ahh thanks. Wasn't aware of that. This can be closed out in that case. I'll bug the software I was using about not setting the DRI_PRIME in certain cases. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111272] [DRI_PRIME] Error on multi GPU with only one enabled
https://bugs.freedesktop.org/show_bug.cgi?id=111272 --- Comment #14 from Michel Dänzer --- (In reply to Parker Reed from comment #13) > But the Intel card isn't enabled in the conf. That doesn't matter with DRI3. With it, libGL can use an alternative GPU without any explicit support for it in Xorg. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111272] [DRI_PRIME] Error on multi GPU with only one enabled
https://bugs.freedesktop.org/show_bug.cgi?id=111272 --- Comment #13 from Parker Reed --- But the Intel card isn't enabled in the conf. Why is it trying to init it if it's not there? i915 is loaded kernel side but it's just dead displaying a TTY. I agree the DRI_PRIME shouldn't be called in this case. It's just odd behavior if it is called accidentally. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111272] [DRI_PRIME] Error on multi GPU with only one enabled
https://bugs.freedesktop.org/show_bug.cgi?id=111272 --- Comment #12 from Michel Dänzer --- In other words, the only thing preventing the Intel GPU from being used in this case is the i965 driver missing the blitImage functionality. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111272] [DRI_PRIME] Error on multi GPU with only one enabled
https://bugs.freedesktop.org/show_bug.cgi?id=111272 --- Comment #11 from Michel Dänzer --- (In reply to Parker Reed from comment #10) > So would DRI_PRIME=1 acting like 0 if only one GPU is present, be a > possibility? The fact it's trying to load the non-existent GPU It's not really "non-existent", or libGL wouldn't try to use it. > in the first place seems to be the root issue. I think the real issue is using DRI_PRIME=1 when it can't work as intended. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111272] [DRI_PRIME] Error on multi GPU with only one enabled
https://bugs.freedesktop.org/show_bug.cgi?id=111272 --- Comment #10 from Parker Reed --- So would DRI_PRIME=1 acting like 0 if only one GPU is present, be a possibility? The fact it's trying to load the non-existent GPU in the first place seems to be the root issue. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111272] [DRI_PRIME] Error on multi GPU with only one enabled
https://bugs.freedesktop.org/show_bug.cgi?id=111272 --- Comment #9 from Parker Reed --- I need to wakeup and read the comment. So it's because DRI2. Thanks for the reply. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111272] [DRI_PRIME] Error on multi GPU with only one enabled
https://bugs.freedesktop.org/show_bug.cgi?id=111272 --- Comment #8 from Parker Reed --- Aye, I was more intrigued by the vsync bypass. It even uncapped games with a forced 60 (regardless of vsync being on or off) Just a consequence of that DRI fallback? -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] radv: fix image_has_{cmask,fmask}() helpers
The driver should now rely on cmask_offset because CMASK can be
disabled by the driver for some reasons (eg. mipmaps). Apply the
same change for FMASK, although it should be useless.
Fixes: ad1bc8621df ("radv: remove radv_get_image_fmask_info()")
Fixes: 10d08da52c6 ("radv/gfx10: add missing dcc_tile_swizzle tweak")
Signed-off-by: Samuel Pitoiset
---
src/amd/vulkan/radv_private.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
index 49d3c78db98..ee0761e69fe 100644
--- a/src/amd/vulkan/radv_private.h
+++ b/src/amd/vulkan/radv_private.h
@@ -1633,7 +1633,7 @@ bool radv_layout_dcc_compressed(const struct radv_image
*image,
static inline bool
radv_image_has_cmask(const struct radv_image *image)
{
- return image->planes[0].surface.cmask_size;
+ return image->cmask_offset;
}
/**
@@ -1642,7 +1642,7 @@ radv_image_has_cmask(const struct radv_image *image)
static inline bool
radv_image_has_fmask(const struct radv_image *image)
{
- return image->planes[0].surface.fmask_size;
+ return image->fmask_offset;
}
/**
--
2.22.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111141] [REGRESSION] [BISECTED] [DXVK] 1-bit booleans and Elite Dangerous shader mis-optimization
https://bugs.freedesktop.org/show_bug.cgi?id=41 --- Comment #22 from Steven Newbury --- Essentially reverting 3371de38f282c77461bbe5007a2fec2a975776df makes it work... ...why? -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] radv: only account for tile_swizzle for color surfaces with DCC
r-b for both
On Thu, Aug 1, 2019 at 3:41 PM Samuel Pitoiset
wrote:
>
> It's 0 for depth surfaces with TC compat HTILE enabled.
>
> Signed-off-by: Samuel Pitoiset
> ---
> src/amd/vulkan/radv_image.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c
> index f3237dd5985..221b554e73e 100644
> --- a/src/amd/vulkan/radv_image.c
> +++ b/src/amd/vulkan/radv_image.c
> @@ -483,6 +483,8 @@ si_set_mutable_tex_desc_fields(struct radv_device *device,
> meta_va = gpu_address + image->dcc_offset;
> if (chip_class <= GFX8)
> meta_va += base_level_info->dcc_offset;
> +
> + meta_va |= (uint32_t)plane->surface.tile_swizzle << 8;
> } else if (!is_storage_image &&
>radv_image_is_tc_compat_htile(image)) {
> meta_va = gpu_address + image->htile_offset;
> @@ -490,10 +492,8 @@ si_set_mutable_tex_desc_fields(struct radv_device
> *device,
>
> if (meta_va) {
> state[6] |= S_008F28_COMPRESSION_EN(1);
> - if (chip_class <= GFX9) {
> + if (chip_class <= GFX9)
> state[7] = meta_va >> 8;
> - state[7] |= plane->surface.tile_swizzle;
> - }
> }
> }
>
> --
> 2.22.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] radv: remove radv_get_image_cmask_info()
r-b for both
On Thu, Aug 1, 2019 at 5:56 PM Samuel Pitoiset
wrote:
>
> It's unnecessary to duplicate fields in another struct.
>
> Signed-off-by: Samuel Pitoiset
> ---
> src/amd/vulkan/radv_device.c | 4 ++--
> src/amd/vulkan/radv_image.c | 38 +---
> src/amd/vulkan/radv_meta_clear.c | 11 +
> src/amd/vulkan/radv_private.h| 13 ++-
> 4 files changed, 21 insertions(+), 45 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index 29be192443a..9aa731a252c 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -4400,7 +4400,7 @@ radv_initialise_color_surface(struct radv_device
> *device,
>
> cb->cb_color_pitch = S_028C64_TILE_MAX(pitch_tile_max);
> cb->cb_color_slice = S_028C68_TILE_MAX(slice_tile_max);
> - cb->cb_color_cmask_slice = iview->image->cmask.slice_tile_max;
> + cb->cb_color_cmask_slice =
> surf->u.legacy.cmask_slice_tile_max;
>
> cb->cb_color_attrib |=
> S_028C74_TILE_MODE_INDEX(tile_mode_index);
>
> @@ -4420,7 +4420,7 @@ radv_initialise_color_surface(struct radv_device
> *device,
>
> /* CMASK variables */
> va = radv_buffer_get_va(iview->bo) + iview->image->offset;
> - va += iview->image->cmask.offset;
> + va += iview->image->cmask_offset;
> cb->cb_color_cmask = va >> 8;
>
> va = radv_buffer_get_va(iview->bo) + iview->image->offset;
> diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c
> index 8ff93e4344c..aaaf15ec8dc 100644
> --- a/src/amd/vulkan/radv_image.c
> +++ b/src/amd/vulkan/radv_image.c
> @@ -939,7 +939,7 @@ si_make_texture_descriptor(struct radv_device *device,
>
> S_008F24_META_RB_ALIGNED(image->planes[0].surface.u.gfx9.cmask.rb_aligned);
>
> if (radv_image_is_tc_compat_cmask(image)) {
> - va = gpu_address + image->offset +
> image->cmask.offset;
> + va = gpu_address + image->offset +
> image->cmask_offset;
>
> fmask_state[5] |=
> S_008F24_META_DATA_ADDRESS(va >> 40);
> fmask_state[6] |= S_008F28_COMPRESSION_EN(1);
> @@ -952,7 +952,7 @@ si_make_texture_descriptor(struct radv_device *device,
> fmask_state[5] |= S_008F24_LAST_ARRAY(last_layer);
>
> if (radv_image_is_tc_compat_cmask(image)) {
> - va = gpu_address + image->offset +
> image->cmask.offset;
> + va = gpu_address + image->offset +
> image->cmask_offset;
>
> fmask_state[6] |= S_008F28_COMPRESSION_EN(1);
> fmask_state[7] |= va >> 8;
> @@ -1138,45 +1138,27 @@ radv_image_alloc_fmask(struct radv_device *device,
> image->alignment = MAX2(image->alignment, image->fmask.alignment);
> }
>
> -static void
> -radv_image_get_cmask_info(struct radv_device *device,
> - struct radv_image *image,
> - struct radv_cmask_info *out)
> -{
> - assert(image->plane_count == 1);
> -
> - if (device->physical_device->rad_info.chip_class >= GFX9) {
> - out->alignment = image->planes[0].surface.cmask_alignment;
> - out->size = image->planes[0].surface.cmask_size;
> - return;
> - }
> -
> - out->slice_tile_max =
> image->planes[0].surface.u.legacy.cmask_slice_tile_max;
> - out->alignment = image->planes[0].surface.cmask_alignment;
> - out->slice_size = image->planes[0].surface.cmask_slice_size;
> - out->size = image->planes[0].surface.cmask_size;
> -}
> -
> static void
> radv_image_alloc_cmask(struct radv_device *device,
>struct radv_image *image)
> {
> + unsigned cmask_alignment = image->planes[0].surface.cmask_alignment;
> + unsigned cmask_size = image->planes[0].surface.cmask_size;
> uint32_t clear_value_size = 0;
> - radv_image_get_cmask_info(device, image, >cmask);
>
> - if (!image->cmask.size)
> + if (!cmask_size)
> return;
>
> - assert(image->cmask.alignment);
> + assert(cmask_alignment);
>
> - image->cmask.offset = align64(image->size, image->cmask.alignment);
> + image->cmask_offset = align64(image->size, cmask_alignment);
> /* + 8 for storing the clear values */
> if (!image->clear_value_offset) {
> - image->clear_value_offset = image->cmask.offset +
> image->cmask.size;
> + image->clear_value_offset = image->cmask_offset + cmask_size;
> clear_value_size = 8;
> }
> - image->size = image->cmask.offset + image->cmask.size +
> clear_value_size;
> - image->alignment = MAX2(image->alignment,
Re: [Mesa-dev] [PATCH] gallivm: fix a missing argument to CreateAtomicCmpXchg
LGTM Reviewed-by: Neha Bhende Regards, Neha From: Charmaine Lee Sent: Thursday, August 1, 2019 3:56 PM To: mesa-dev@lists.freedesktop.org; Brian Paul; Neha Bhende; Roland Scheidegger; Jose Fonseca; airl...@redhat.com Cc: Charmaine Lee Subject: [PATCH] gallivm: fix a missing argument to CreateAtomicCmpXchg This patch fixes a missing argument to CreateAtomicCmpXchg for older version of LLVM. --- src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp index 79d1029..8205d24 100644 --- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp +++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp @@ -831,6 +831,7 @@ LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr, /* LLVM 3.8 doesn't have a second ordering and uses old SynchronizationScope enum */ return llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr), llvm::unwrap(Cmp), llvm::unwrap(New), mapFromLLVMOrdering(SuccessOrdering), + mapFromLLVMOrdering(FailureOrdering), SingleThread ? llvm::SynchronizationScope::SingleThread : llvm::SynchronizationScope::CrossThread)); } #endif -- 1.8.5.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 8/9] panfrost: Handle the bo == NULL case in panfrost_bo_[un]reference()
Allows us to pass BOs without checking if they're NULL or not.
Signed-off-by: Boris Brezillon
---
src/gallium/drivers/panfrost/pan_resource.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/panfrost/pan_resource.c
b/src/gallium/drivers/panfrost/pan_resource.c
index 71da383d4c7a..f74a39555b45 100644
--- a/src/gallium/drivers/panfrost/pan_resource.c
+++ b/src/gallium/drivers/panfrost/pan_resource.c
@@ -443,12 +443,16 @@ panfrost_resource_create(struct pipe_screen *screen,
void
panfrost_bo_reference(struct panfrost_bo *bo)
{
-pipe_reference(NULL, >reference);
+if (bo)
+pipe_reference(NULL, >reference);
}
void
panfrost_bo_unreference(struct pipe_screen *screen, struct panfrost_bo *bo)
{
+if (!bo)
+return;
+
/* When the reference count goes to zero, we need to cleanup */
if (pipe_reference(>reference, NULL))
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 6/9] panfrost: Don't emit a new FB desc when setting a new FB state
The FB desc will be emitted/attached on the first draw targetting this new FB. Signed-off-by: Boris Brezillon --- src/gallium/drivers/panfrost/pan_context.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c index 1091caeb1148..2b7906eea155 100644 --- a/src/gallium/drivers/panfrost/pan_context.c +++ b/src/gallium/drivers/panfrost/pan_context.c @@ -2384,6 +2384,9 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx, if (!is_scanout || has_draws) panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME); +else +assert(!ctx->payloads[PIPE_SHADER_VERTEX].postfix.framebuffer && + !ctx->payloads[PIPE_SHADER_FRAGMENT].postfix.framebuffer); util_copy_framebuffer_state(>pipe_framebuffer, fb); @@ -2391,7 +2394,8 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx, struct panfrost_screen *screen = pan_screen(ctx->base.screen); panfrost_hint_afbc(screen, >pipe_framebuffer); -panfrost_attach_vt_framebuffer(ctx, false); +for (unsigned i = 0; i < PIPE_SHADER_TYPES; ++i) +ctx->payloads[i].postfix.framebuffer = 0; } static void * -- 2.21.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/9] panfrost: Bail out early when new and current FB states are equal
If the current FB matches the new one there's nothing to be done in
panfrost_set_framebuffer_state(). By bailing out early in that case we
avoid emitting new FB descriptors (the old ones are still valid).
Signed-off-by: Boris Brezillon
---
src/gallium/drivers/panfrost/pan_context.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/gallium/drivers/panfrost/pan_context.c
b/src/gallium/drivers/panfrost/pan_context.c
index b63023a16cda..d442ae1f2433 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -2365,6 +2365,10 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx,
bool is_scanout = panfrost_is_scanout(ctx);
bool has_draws = job->last_job.gpu;
+/* Bail out early when the current and new states are the same. */
+if (util_framebuffer_state_equal(>pipe_framebuffer, fb))
+return;
+
if (!ctx->wallpaper_batch && (!is_scanout || has_draws)) {
panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME);
}
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/9] panfrost: Get rid of ctx->job
This field is never set to anything but NULL, which means the test
done in panfrost_free_job() and panfrost_get_job_for_fbo() will always
evaluate to false. Let's get rid of this field.
Signed-off-by: Boris Brezillon
---
src/gallium/drivers/panfrost/pan_context.h | 3 +--
src/gallium/drivers/panfrost/pan_job.c | 8
2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/src/gallium/drivers/panfrost/pan_context.h
b/src/gallium/drivers/panfrost/pan_context.h
index d930f12a537b..a90dbb04e833 100644
--- a/src/gallium/drivers/panfrost/pan_context.h
+++ b/src/gallium/drivers/panfrost/pan_context.h
@@ -95,8 +95,7 @@ struct panfrost_context {
/* Compiler context */
struct midgard_screen compiler;
-/* Bound job and map of panfrost_job_key to jobs */
-struct panfrost_job *job;
+/* Map of panfrost_job_key to jobs */
struct hash_table *jobs;
/* panfrost_resource -> panfrost_job */
diff --git a/src/gallium/drivers/panfrost/pan_job.c
b/src/gallium/drivers/panfrost/pan_job.c
index 6339b39d29a0..960c8556e2f0 100644
--- a/src/gallium/drivers/panfrost/pan_job.c
+++ b/src/gallium/drivers/panfrost/pan_job.c
@@ -72,9 +72,6 @@ panfrost_free_job(struct panfrost_context *ctx, struct
panfrost_job *job)
_mesa_hash_table_remove_key(ctx->jobs, >key);
-if (ctx->job == job)
-ctx->job = NULL;
-
ralloc_free(job);
}
@@ -122,11 +119,6 @@ panfrost_get_job_for_fbo(struct panfrost_context *ctx)
if (ctx->wallpaper_batch)
return ctx->wallpaper_batch;
-/* If we already began rendering, use that */
-
-if (ctx->job)
-return ctx->job;
-
/* If not, look up the job */
struct pipe_surface **cbufs = ctx->pipe_framebuffer.cbufs;
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/9] panfrost: Allocate the polygon lists on-demand
Hello, This patch series is getting rid of the 64MB polygon-list allocation that was done at context init time in favor of a per-job polygon-list allocation allowing us to shrink the BO size considerably and thus reduce memory consumption. The first 8 patches are random cleanups. Most of them are needed to get patch 9 working (patch 2, 3, 4, 5, 6 and 8), others are just things I decided to get rid off along the way (patches 1 and 7). Regards, Boris Alyssa Rosenzweig (1): panfrost: Allocate polygon lists on-demand Boris Brezillon (8): panfrost: Get rid of ctx->job panfrost: Remove job from ctx->jobs at submission time panfrost: Delay FB descriptor allocation panfrost: Bail out early when new and current FB states are equal panfrost: Bail out early when doing a wallpaper blit panfrost: Don't emit a new FB desc when setting a new FB state panfrost: Get rid of the skippable param in attach_vt_framebuffer() panfrost: Handle the bo == NULL case in panfrost_bo_[un]reference() src/gallium/drivers/panfrost/pan_context.c| 61 +++ src/gallium/drivers/panfrost/pan_context.h| 7 +-- src/gallium/drivers/panfrost/pan_drm.c| 2 +- src/gallium/drivers/panfrost/pan_job.c| 40 +--- src/gallium/drivers/panfrost/pan_job.h| 6 ++ src/gallium/drivers/panfrost/pan_resource.c | 6 +- src/gallium/drivers/panfrost/pan_scoreboard.c | 5 +- 7 files changed, 82 insertions(+), 45 deletions(-) -- 2.21.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/9] panfrost: Remove job from ctx->jobs at submission time
This guarantees that new draws targetting the same framebuffer will get a new job instance. Signed-off-by: Boris Brezillon --- src/gallium/drivers/panfrost/pan_job.c | 8 1 file changed, 8 insertions(+) diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c index 960c8556e2f0..d2a4c8c3c600 100644 --- a/src/gallium/drivers/panfrost/pan_job.c +++ b/src/gallium/drivers/panfrost/pan_job.c @@ -173,6 +173,14 @@ panfrost_job_submit(struct panfrost_context *ctx, struct panfrost_job *job) if (ret) fprintf(stderr, "panfrost_job_submit failed: %d\n", ret); + +/* Remove the job from the ctx->jobs set so that future + * panfrost_get_job() calls don't see it. + * We must reset the job key to avoid removing another valid entry when + * the job is freed. + */ +_mesa_hash_table_remove_key(ctx->jobs, >key); +memset(>key, 0, sizeof(job->key)); } void -- 2.21.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 9/9] panfrost: Allocate polygon lists on-demand
From: Alyssa Rosenzweig
Rather than alloacting a huge (64MB) polygon list on context creation
and sharing it across framebuffers, we instead allocate polygon lists as
BOs (which consistently hit the cache) sized appropriately; for about a
month, we've known how to calculate the polygon list size so this has
only recently become possible.
The good news is we can render to truly massive framebuffers without
crashing and, more importantly, we eliminate the 64MB upfront overhead.
If a list that size isn't actually needed, it's not allocated.
Signed-off-by: Alyssa Rosenzweig
Signed-off-by: Boris Brezillon
---
src/gallium/drivers/panfrost/pan_context.c| 8 ++-
src/gallium/drivers/panfrost/pan_context.h| 1 -
src/gallium/drivers/panfrost/pan_drm.c| 2 +-
src/gallium/drivers/panfrost/pan_job.c| 24 +++
src/gallium/drivers/panfrost/pan_job.h| 6 +
src/gallium/drivers/panfrost/pan_scoreboard.c | 5 ++--
6 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/src/gallium/drivers/panfrost/pan_context.c
b/src/gallium/drivers/panfrost/pan_context.c
index e781d809812b..26bd0082a339 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -62,6 +62,7 @@ panfrost_emit_midg_tiler(
unsigned vertex_count)
{
struct midgard_tiler_descriptor t = {};
+struct panfrost_job *batch = panfrost_get_job_for_fbo(ctx);
t.hierarchy_mask =
panfrost_choose_hierarchy_mask(width, height, vertex_count);
@@ -77,10 +78,7 @@ panfrost_emit_midg_tiler(
/* Sanity check */
if (t.hierarchy_mask) {
-assert(ctx->tiler_polygon_list.bo->size >= (header_size +
body_size));
-
-/* Specify allocated tiler structures */
-t.polygon_list = ctx->tiler_polygon_list.bo->gpu;
+t.polygon_list = panfrost_job_get_polygon_list(batch,
header_size + body_size);
/* Allow the entire tiler heap */
t.heap_start = ctx->tiler_heap.bo->gpu;
@@ -2527,7 +2525,6 @@ panfrost_destroy(struct pipe_context *pipe)
panfrost_drm_free_slab(screen, >scratchpad);
panfrost_drm_free_slab(screen, >shaders);
panfrost_drm_free_slab(screen, >tiler_heap);
-panfrost_drm_free_slab(screen, >tiler_polygon_list);
panfrost_drm_free_slab(screen, >tiler_dummy);
ralloc_free(pipe);
@@ -2673,7 +2670,6 @@ panfrost_setup_hardware(struct panfrost_context *ctx)
panfrost_drm_allocate_slab(screen, >scratchpad, 64*4, false, 0,
0, 0);
panfrost_drm_allocate_slab(screen, >shaders, 4096, true,
PAN_ALLOCATE_EXECUTE, 0, 0);
panfrost_drm_allocate_slab(screen, >tiler_heap, 4096, false,
PAN_ALLOCATE_INVISIBLE | PAN_ALLOCATE_GROWABLE, 1, 128);
-panfrost_drm_allocate_slab(screen, >tiler_polygon_list, 128*128,
false, PAN_ALLOCATE_INVISIBLE | PAN_ALLOCATE_GROWABLE, 1, 128);
panfrost_drm_allocate_slab(screen, >tiler_dummy, 1, false,
PAN_ALLOCATE_INVISIBLE, 0, 0);
}
diff --git a/src/gallium/drivers/panfrost/pan_context.h
b/src/gallium/drivers/panfrost/pan_context.h
index ac4b21678e65..7556500ae72d 100644
--- a/src/gallium/drivers/panfrost/pan_context.h
+++ b/src/gallium/drivers/panfrost/pan_context.h
@@ -110,7 +110,6 @@ struct panfrost_context {
struct panfrost_memory shaders;
struct panfrost_memory scratchpad;
struct panfrost_memory tiler_heap;
-struct panfrost_memory tiler_polygon_list;
struct panfrost_memory tiler_dummy;
struct panfrost_memory depth_stencil_buffer;
diff --git a/src/gallium/drivers/panfrost/pan_drm.c
b/src/gallium/drivers/panfrost/pan_drm.c
index 89c7019dd9c7..42cf17503344 100644
--- a/src/gallium/drivers/panfrost/pan_drm.c
+++ b/src/gallium/drivers/panfrost/pan_drm.c
@@ -288,7 +288,7 @@ panfrost_drm_submit_vs_fs_job(struct panfrost_context *ctx,
bool has_draws, bool
panfrost_job_add_bo(job, ctx->shaders.bo);
panfrost_job_add_bo(job, ctx->scratchpad.bo);
panfrost_job_add_bo(job, ctx->tiler_heap.bo);
-panfrost_job_add_bo(job, ctx->tiler_polygon_list.bo);
+panfrost_job_add_bo(job, job->polygon_list);
if (job->first_job.gpu) {
ret = panfrost_drm_submit_job(ctx, job->first_job.gpu, 0);
diff --git a/src/gallium/drivers/panfrost/pan_job.c
b/src/gallium/drivers/panfrost/pan_job.c
index d2a4c8c3c600..9c39181d6e48 100644
--- a/src/gallium/drivers/panfrost/pan_job.c
+++ b/src/gallium/drivers/panfrost/pan_job.c
@@ -70,6 +70,9 @@ panfrost_free_job(struct panfrost_context *ctx, struct
panfrost_job *job)
BITSET_SET(screen->free_transient, *index);
}
+/* Unreference the polygon list */
+panfrost_bo_unreference(ctx->base.screen, job->polygon_list);
+
_mesa_hash_table_remove_key(ctx->jobs, >key);
ralloc_free(job);
@@ -141,6 +144,27
[Mesa-dev] [PATCH 5/9] panfrost: Bail out early when doing a wallpaper blit
The wallpaper blit is a bit special in that the operation is targetting
the current FB, but the u_blitter logic creates a new surface for it
which makes util_framebuffer_state_equal() return false. In that case
we don't want a new FB descriptor to be emitted/attached, so let's just
copy the new state into ctx->pipe_framebuffer and exit the function.
Signed-off-by: Boris Brezillon
---
src/gallium/drivers/panfrost/pan_context.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/gallium/drivers/panfrost/pan_context.c
b/src/gallium/drivers/panfrost/pan_context.c
index d442ae1f2433..1091caeb1148 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -2369,10 +2369,22 @@ panfrost_set_framebuffer_state(struct pipe_context
*pctx,
if (util_framebuffer_state_equal(>pipe_framebuffer, fb))
return;
-if (!ctx->wallpaper_batch && (!is_scanout || has_draws)) {
-panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME);
+/* The wallpaper logic sets a new FB state before doing the blit and
+ * restore the old one when it's done. Those FB states are reported to
+ * be different because the surface they are pointing to are different,
+ * but those surfaces actually point to the same cbufs/zbufs. In that
+ * case we definitely don't want new FB descs to be emitted/attached
+ * since the job is expected to be flushed just after the blit is done,
+ * so let's just copy the new state and return here.
+ */
+if (ctx->wallpaper_batch) {
+util_copy_framebuffer_state(>pipe_framebuffer, fb);
+return;
}
+if (!is_scanout || has_draws)
+panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME);
+
util_copy_framebuffer_state(>pipe_framebuffer, fb);
/* Given that we're rendering, we'd love to have compression */
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/9] panfrost: Delay FB descriptor allocation
No need to emit SFBD/MFBD at frame invalidation. They can be emitted
when the framebuffer is attached, which saves us a potential FB desc
re-allocation if a new FB is bound after the swap.
Signed-off-by: Boris Brezillon
---
src/gallium/drivers/panfrost/pan_context.c | 21 ++---
src/gallium/drivers/panfrost/pan_context.h | 3 ---
2 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/src/gallium/drivers/panfrost/pan_context.c
b/src/gallium/drivers/panfrost/pan_context.c
index 014f8f6a9d07..b63023a16cda 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -189,13 +189,17 @@ panfrost_clear(
static mali_ptr
panfrost_attach_vt_mfbd(struct panfrost_context *ctx)
{
-return panfrost_upload_transient(ctx, >vt_framebuffer_mfbd,
sizeof(ctx->vt_framebuffer_mfbd)) | MALI_MFBD;
+struct bifrost_framebuffer mfbd = panfrost_emit_mfbd(ctx, ~0);
+
+return panfrost_upload_transient(ctx, , sizeof(mfbd)) | MALI_MFBD;
}
static mali_ptr
panfrost_attach_vt_sfbd(struct panfrost_context *ctx)
{
-return panfrost_upload_transient(ctx, >vt_framebuffer_sfbd,
sizeof(ctx->vt_framebuffer_sfbd)) | MALI_SFBD;
+struct mali_single_framebuffer sfbd = panfrost_emit_sfbd(ctx, ~0);
+
+return panfrost_upload_transient(ctx, , sizeof(sfbd)) | MALI_SFBD;
}
static void
@@ -223,13 +227,6 @@ panfrost_attach_vt_framebuffer(struct panfrost_context
*ctx, bool skippable)
static void
panfrost_invalidate_frame(struct panfrost_context *ctx)
{
-struct panfrost_screen *screen = pan_screen(ctx->base.screen);
-
-if (screen->require_sfbd)
-ctx->vt_framebuffer_sfbd = panfrost_emit_sfbd(ctx, ~0);
-else
-ctx->vt_framebuffer_mfbd = panfrost_emit_mfbd(ctx, ~0);
-
for (unsigned i = 0; i < PIPE_SHADER_TYPES; ++i)
ctx->payloads[i].postfix.framebuffer = 0;
@@ -2378,12 +2375,6 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx,
struct panfrost_screen *screen = pan_screen(ctx->base.screen);
panfrost_hint_afbc(screen, >pipe_framebuffer);
-
-if (screen->require_sfbd)
-ctx->vt_framebuffer_sfbd = panfrost_emit_sfbd(ctx, ~0);
-else
-ctx->vt_framebuffer_mfbd = panfrost_emit_mfbd(ctx, ~0);
-
panfrost_attach_vt_framebuffer(ctx, false);
}
diff --git a/src/gallium/drivers/panfrost/pan_context.h
b/src/gallium/drivers/panfrost/pan_context.h
index a90dbb04e833..ac4b21678e65 100644
--- a/src/gallium/drivers/panfrost/pan_context.h
+++ b/src/gallium/drivers/panfrost/pan_context.h
@@ -137,9 +137,6 @@ struct panfrost_context {
union mali_attr attributes[PIPE_MAX_ATTRIBS];
-struct mali_single_framebuffer vt_framebuffer_sfbd;
-struct bifrost_framebuffer vt_framebuffer_mfbd;
-
/* TODO: Multiple uniform buffers (index =/= 0), finer updates? */
struct panfrost_constant_buffer constant_buffer[PIPE_SHADER_TYPES];
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 7/9] panfrost: Get rid of the skippable param in attach_vt_framebuffer()
The only user of this function always passes true.
Signed-off-by: Boris Brezillon
---
src/gallium/drivers/panfrost/pan_context.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/gallium/drivers/panfrost/pan_context.c
b/src/gallium/drivers/panfrost/pan_context.c
index 2b7906eea155..e781d809812b 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -203,11 +203,11 @@ panfrost_attach_vt_sfbd(struct panfrost_context *ctx)
}
static void
-panfrost_attach_vt_framebuffer(struct panfrost_context *ctx, bool skippable)
+panfrost_attach_vt_framebuffer(struct panfrost_context *ctx)
{
/* Skip the attach if we can */
-if (skippable &&
ctx->payloads[PIPE_SHADER_VERTEX].postfix.framebuffer) {
+if (ctx->payloads[PIPE_SHADER_VERTEX].postfix.framebuffer) {
assert(ctx->payloads[PIPE_SHADER_FRAGMENT].postfix.framebuffer);
return;
}
@@ -1013,7 +1013,7 @@ panfrost_emit_for_draw(struct panfrost_context *ctx, bool
with_vertex_data)
struct panfrost_job *job = panfrost_get_job_for_fbo(ctx);
struct panfrost_screen *screen = pan_screen(ctx->base.screen);
-panfrost_attach_vt_framebuffer(ctx, true);
+panfrost_attach_vt_framebuffer(ctx);
if (with_vertex_data) {
panfrost_emit_vertex_data(job);
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111272] [DRI_PRIME] Error on multi GPU with only one enabled
https://bugs.freedesktop.org/show_bug.cgi?id=111272 Michel Dänzer changed: What|Removed |Added Assignee|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop. |.org|org Component|DRM/AMDgpu |GLX Version|XOrg git|unspecified Product|DRI |Mesa QA Contact||mesa-dev@lists.freedesktop. ||org --- Comment #7 from Michel Dänzer --- Due to DRI_PRIME=1, libGL tries to initialize the i965 driver with DRI3, which fails: libGL error: Different GPU, but blitImage not implemented for this driver libGL error: failed to load driver: i965 So it falls back to DRI2, which uses the AMD GPU, because the Intel one wasn't initialized in Xorg. DRI2 doesn't support sync-to-vblank via PRIME. Reassigning to Mesa for now, but DRI_PRIME=1 really isn't intended to be used with a single GPU. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Mesa 19.2.0 release plan
Hi all I would suggest to add this issue to the release - https://bugs.freedesktop.org/show_bug.cgi?id=110814 Since, the isn't actual with 19.0 version of the Mesa - it's a regression issue and we proposed the fix to it and would ask to merge it to 19.2 release it in this release. Regards, Paul ср, 31 июл. 2019 г. в 16:56, Eric Engestrom : > On 2019-07-31 at 09:38, Emil Velikov wrote: > > Hi all, > > > > Here is the tentative release plan for 19.2.0. > > > > As many of you are well aware, it's time to the next branch point. > > The calendar is already updated, so these are the tentative dates: > > > > Aug 06 2019 - Feature freeze/Release candidate 1 > > Aug 13 2019 - Release candidate 2 > > Aug 20 2019 - Release candidate 3 > > Aug 27 2019 - Release candidate 4/final release > > > > This gives us around 1 week until the branch point. > > > > Note: In the spirit of keeping things clearer and more transparent, we > > will be keeping track of any features planned for the release in > > Bugzilla [1]. > > > > Do add a separate "Depends on" for each work you have planned. > > Alternatively you can reply to this email and I'll add them for you. > > > > [1] https://bugs.freedesktop.org/show_bug.cgi?id=111265 > > Thanks! > > As per previous discussions (I don't remember where, sorry) as well as > internal discussions, I think we should add all currently open regressions > since 19.1 as blockers for this release. > > We should also add that to the procedure in our docs; I'll do that later > today if nobody beats me to it. > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallivm: fix a missing argument to CreateAtomicCmpXchg
Looks good to me. Reviewed-by: Jose Fonseca From: Charmaine Lee Sent: Thursday, August 1, 2019 23:56 To: mesa-dev@lists.freedesktop.org ; Brian Paul ; Neha Bhende ; Roland Scheidegger ; Jose Fonseca ; airl...@redhat.com Cc: Charmaine Lee Subject: [PATCH] gallivm: fix a missing argument to CreateAtomicCmpXchg This patch fixes a missing argument to CreateAtomicCmpXchg for older version of LLVM. --- src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp index 79d1029..8205d24 100644 --- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp +++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp @@ -831,6 +831,7 @@ LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr, /* LLVM 3.8 doesn't have a second ordering and uses old SynchronizationScope enum */ return llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr), llvm::unwrap(Cmp), llvm::unwrap(New), mapFromLLVMOrdering(SuccessOrdering), + mapFromLLVMOrdering(FailureOrdering), SingleThread ? llvm::SynchronizationScope::SingleThread : llvm::SynchronizationScope::CrossThread)); } #endif -- 1.8.5.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: fix OpenGL 3.1 context creation
On Thursday, August 1, 2019 6:38:45 PM PDT Timothy Arceri wrote: > From the EGL_KHR_create_context spec: > >"* If OpenGL 3.1 is requested, the context returned may implement >any of the following versions: > > * Version 3.1. The GL_ARB_compatibility extension may or may >not be implemented, as determined by the implementation. > * The core profile of version 3.2 or greater." > > Fixes CTS tests: > > dEQP-EGL.functional.create_context_ext.gl_31.rgb888_depth_stencil > dEQP-EGL.functional.create_context_ext.robust_gl_31.rgb888_depth_stencil > dEQP-EGL.functional.create_context_ext.gl_31.rgb888_depth_no_stencil > > dEQP-EGL.functional.create_context_ext.robust_gl_31.rgb888_depth_no_stencil > dEQP-EGL.functional.create_context_ext.gl_31.rgba_depth_no_stencil > dEQP-EGL.functional.create_context_ext.gl_31.rgb888_no_depth_no_stencil > > dEQP-EGL.functional.create_context_ext.robust_gl_31.rgba_depth_no_stencil > > dEQP-EGL.functional.create_context_ext.robust_gl_31.rgb888_no_depth_no_stencil > dEQP-EGL.functional.create_context_ext.gl_31.rgba_no_depth_no_stencil > > dEQP-EGL.functional.create_context_ext.robust_gl_31.rgba_no_depth_no_stencil > dEQP-EGL.functional.create_context_ext.gl_31.rgba_depth_stencil > dEQP-EGL.functional.create_context_ext.robust_gl_31.rgba_depth_stencil > --- > src/egl/drivers/dri2/egl_dri2.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c > index c712c106b06..918d61a1e9b 100644 > --- a/src/egl/drivers/dri2/egl_dri2.c > +++ b/src/egl/drivers/dri2/egl_dri2.c > @@ -1245,6 +1245,9 @@ dri2_create_context(_EGLDriver *drv, _EGLDisplay *disp, > _EGLConfig *conf, > && dri2_ctx->base.ClientMinorVersion >= 2)) >&& dri2_ctx->base.Profile == > EGL_CONTEXT_OPENGL_CORE_PROFILE_BIT_KHR) > api = __DRI_API_OPENGL_CORE; > + else if (dri2_ctx->base.ClientMajorVersion == 3 && > + dri2_ctx->base.ClientMinorVersion == 1) > + api = __DRI_API_OPENGL_CORE; >else > api = __DRI_API_OPENGL; >break; > Good find! Reviewed-by: Kenneth Graunke signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
