Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
On 8 May 2017 at 00:25, Dieter Nützel wrote: > Am 26.04.2017 14:37, schrieb tournier.elie: >> >> Hello, >> >> I'm not an r600g person but I will be pleased to have a look to this work. > > > Hello Elie, > > I know, read most of your posts about your 'former' fp64 lib which was > indeed HW-agnostic. > >> I'm currently working on the fp64 for r600g. Hope to be able to send a >> series in fews days. > > > Do you have something handy for testing? > My r600g (Turks XT / HD 6670) times run out... ;-) > https://www.phoronix.com/forums/forum/phoronix/latest-phoronix-articles/950505-radeon-rx-550-amdgpu-pro-vs-drm-next-mesa-17-2-dev?p=950577#post950577 > I can use LLVM pipe / softpipe to test the code. Moreover, Andreas Boll helps me to test the code. He own a r600g card. >> The fp64 code is not HW specific, I can test it on my laptop with an >> Intel card. I don't even own a r600g card. > > > Do you want my above card for testing r600g? Thanks you for the offer but I don't have a tower here. > >> Have a nice day. > > > Have a nice spring, late here in old (northern) Europe... > I'm not sure there are a spring or summer here in Cambridge, UK. ;-) Elie > Dieter > > >> On 25 April 2017 at 03:00, Dieter Nützel wrote: >>> >>> Am 12.04.2017 22:06, schrieb Dave Airlie: On 13 April 2017 at 06:03, Markus Trippelsdorf wrote: > > > On 2017.04.12 at 20:45 +0100, Emil Velikov wrote: >> >> >> On 12 April 2017 at 20:34, Constantine Kharlamov >> wrote: >> >> >> I suspect this breaks because r600 more often fails to >> >> compile some shaders, >> >> and the hw requires a fragment shader and we use the empty one as a >> >> fallback in that case. >> > >> > Ok, that wasn't obvious, I running the system with the patchset on >> > HD5730, including >> > piglit testing — so far is fine. >> > >> If in doubt ask the bug reporter(s) to test a patch on their machines. > > > > Well, the question is why bother with this legacy code at all? > I would expect a much more conservative approach. Except for essential > patches to keep the hardware running, everything else should be > rejected. Nobody is expecting new features for this ancient hardware > anyway. I do have 90% of GL4.3 for evergreen and newer done in a branch, It's just a lack of time to fix the last few hangs and motivate myself to get it posted/reviewied. Then of course getting sb support. Dave. >>> >>> >>> >>> Hello Dave, >>> >>> can you please send the link to your latest work on this? >>> So someone (we) can have a look and learn/maybe fix it. >>> Maybe Elie step in and offer fp64 for r600g if he has some time left. >>> My times on r600g come to an end... >>> RX 580, 8 GB, Nitro+ is coming on Friday/Saturday, Yea ;-) >>> >>> Then I have to decide if I hold the Turks XT (6670) or to comp it. >>> >>> Greetings around the world! >>> >>> Dieter ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
Am 26.04.2017 14:37, schrieb tournier.elie: Hello, I'm not an r600g person but I will be pleased to have a look to this work. Hello Elie, I know, read most of your posts about your 'former' fp64 lib which was indeed HW-agnostic. I'm currently working on the fp64 for r600g. Hope to be able to send a series in fews days. Do you have something handy for testing? My r600g (Turks XT / HD 6670) times run out... ;-) https://www.phoronix.com/forums/forum/phoronix/latest-phoronix-articles/950505-radeon-rx-550-amdgpu-pro-vs-drm-next-mesa-17-2-dev?p=950577#post950577 The fp64 code is not HW specific, I can test it on my laptop with an Intel card. I don't even own a r600g card. Do you want my above card for testing r600g? Have a nice day. Have a nice spring, late here in old (northern) Europe... Dieter On 25 April 2017 at 03:00, Dieter Nützel wrote: Am 12.04.2017 22:06, schrieb Dave Airlie: On 13 April 2017 at 06:03, Markus Trippelsdorf wrote: On 2017.04.12 at 20:45 +0100, Emil Velikov wrote: On 12 April 2017 at 20:34, Constantine Kharlamov wrote: >> I suspect this breaks because r600 more often fails to >> compile some shaders, >> and the hw requires a fragment shader and we use the empty one as a >> fallback in that case. > > Ok, that wasn't obvious, I running the system with the patchset on > HD5730, including > piglit testing — so far is fine. > If in doubt ask the bug reporter(s) to test a patch on their machines. Well, the question is why bother with this legacy code at all? I would expect a much more conservative approach. Except for essential patches to keep the hardware running, everything else should be rejected. Nobody is expecting new features for this ancient hardware anyway. I do have 90% of GL4.3 for evergreen and newer done in a branch, It's just a lack of time to fix the last few hangs and motivate myself to get it posted/reviewied. Then of course getting sb support. Dave. Hello Dave, can you please send the link to your latest work on this? So someone (we) can have a look and learn/maybe fix it. Maybe Elie step in and offer fp64 for r600g if he has some time left. My times on r600g come to an end... RX 580, 8 GB, Nitro+ is coming on Friday/Saturday, Yea ;-) Then I have to decide if I hold the Turks XT (6670) or to comp it. Greetings around the world! Dieter ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
Hello, I'm not an r600g person but I will be pleased to have a look to this work. I'm currently working on the fp64 for r600g. Hope to be able to send a series in fews days. The fp64 code is not HW specific, I can test it on my laptop with an Intel card. I don't even own a r600g card. Have a nice day. On 25 April 2017 at 03:00, Dieter Nützel wrote: > Am 12.04.2017 22:06, schrieb Dave Airlie: >> >> On 13 April 2017 at 06:03, Markus Trippelsdorf >> wrote: >>> >>> On 2017.04.12 at 20:45 +0100, Emil Velikov wrote: On 12 April 2017 at 20:34, Constantine Kharlamov wrote: >> I suspect this breaks because r600 more often fails to >> compile some shaders, >> and the hw requires a fragment shader and we use the empty one as a >> fallback in that case. > > Ok, that wasn't obvious, I running the system with the patchset on > HD5730, including > piglit testing — so far is fine. > If in doubt ask the bug reporter(s) to test a patch on their machines. >>> >>> >>> Well, the question is why bother with this legacy code at all? >>> I would expect a much more conservative approach. Except for essential >>> patches to keep the hardware running, everything else should be >>> rejected. Nobody is expecting new features for this ancient hardware >>> anyway. >> >> >> I do have 90% of GL4.3 for evergreen and newer done in a branch, >> >> It's just a lack of time to fix the last few hangs and motivate myself >> to get it posted/reviewied. >> >> Then of course getting sb support. >> >> Dave. > > > Hello Dave, > > can you please send the link to your latest work on this? > So someone (we) can have a look and learn/maybe fix it. > Maybe Elie step in and offer fp64 for r600g if he has some time left. > My times on r600g come to an end... > RX 580, 8 GB, Nitro+ is coming on Friday/Saturday, Yea ;-) > > Then I have to decide if I hold the Turks XT (6670) or to comp it. > > Greetings around the world! > > Dieter ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
Am 12.04.2017 22:06, schrieb Dave Airlie: On 13 April 2017 at 06:03, Markus Trippelsdorf wrote: On 2017.04.12 at 20:45 +0100, Emil Velikov wrote: On 12 April 2017 at 20:34, Constantine Kharlamov wrote: >> I suspect this breaks because r600 more often fails to >> compile some shaders, >> and the hw requires a fragment shader and we use the empty one as a >> fallback in that case. > > Ok, that wasn't obvious, I running the system with the patchset on HD5730, including > piglit testing — so far is fine. > If in doubt ask the bug reporter(s) to test a patch on their machines. Well, the question is why bother with this legacy code at all? I would expect a much more conservative approach. Except for essential patches to keep the hardware running, everything else should be rejected. Nobody is expecting new features for this ancient hardware anyway. I do have 90% of GL4.3 for evergreen and newer done in a branch, It's just a lack of time to fix the last few hangs and motivate myself to get it posted/reviewied. Then of course getting sb support. Dave. Hello Dave, can you please send the link to your latest work on this? So someone (we) can have a look and learn/maybe fix it. Maybe Elie step in and offer fp64 for r600g if he has some time left. My times on r600g come to an end... RX 580, 8 GB, Nitro+ is coming on Friday/Saturday, Yea ;-) Then I have to decide if I hold the Turks XT (6670) or to comp it. Greetings around the world! Dieter ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
On Apr 13, 2017 3:34 AM, "Markus Trippelsdorf" wrote: On 2017.04.12 at 20:45 +0100, Emil Velikov wrote: > On 12 April 2017 at 20:34, Constantine Kharlamov wrote: > > >> I suspect this breaks because r600 more often fails to > >> compile some shaders, > >> and the hw requires a fragment shader and we use the empty one as a > >> fallback in that case. > > > > Ok, that wasn't obvious, I running the system with the patchset on HD5730, including > > piglit testing — so far is fine. > > > If in doubt ask the bug reporter(s) to test a patch on their machines. Well, the question is why bother with this legacy code at all? I would expect a much more conservative approach. Except for essential patches to keep the hardware running, everything else should be rejected. Nobody is expecting new features for this ancient hardware anyway. As long as the new code is tested, there is no reason for not accepting it. It's how open source works. Marek -- Markus ___ 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 3/3 v2] r600g: get rid of dummy pixel shader
On 2017.04.12 at 20:45 +0100, Emil Velikov wrote: > On 12 April 2017 at 20:34, Constantine Kharlamov wrote: > > >> I suspect this breaks because r600 more often fails to > >> compile some shaders, > >> and the hw requires a fragment shader and we use the empty one as a > >> fallback in that case. > > > > Ok, that wasn't obvious, I running the system with the patchset on HD5730, > > including > > piglit testing — so far is fine. > > > If in doubt ask the bug reporter(s) to test a patch on their machines. Well, the question is why bother with this legacy code at all? I would expect a much more conservative approach. Except for essential patches to keep the hardware running, everything else should be rejected. Nobody is expecting new features for this ancient hardware anyway. -- Markus ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
On 2017.04.10 at 22:48 +0200, Marek Olšák wrote: > Pushed the series, thanks! > > Marek > > On Mon, Apr 10, 2017 at 10:04 PM, Constantine Kharlamov > wrote: > > The idea is taken from radeonsi. The code mostly was already checking for > > null > > pixel shader, so little checks had to be added. > > > > Interestingly, acc. to testing with GTAⅣ, though binding of null shader > > happens > > a lot at the start (then just stops), but draw_vbo() never actually sees > > null > > ps. > > > > v2: added a check I missed because of a macros using a prefix to choose > > a shader. This commit causes ring 0 stalls on r600: https://bugs.freedesktop.org/show_bug.cgi?id=100663 -- Markus ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
On 12.04.2017 23:03, Markus Trippelsdorf wrote: > On 2017.04.12 at 20:45 +0100, Emil Velikov wrote: >> On 12 April 2017 at 20:34, Constantine Kharlamov wrote: >> I suspect this breaks because r600 more often fails to compile some shaders, and the hw requires a fragment shader and we use the empty one as a fallback in that case. >>> >>> Ok, that wasn't obvious, I running the system with the patchset on HD5730, >>> including >>> piglit testing — so far is fine. >>> >> If in doubt ask the bug reporter(s) to test a patch on their machines. > > Well, the question is why bother with this legacy code at all? > I would expect a much more conservative approach. Except for essential > patches to keep the hardware running, everything else should be > rejected. Nobody is expecting new features for this ancient hardware > anyway. It's not that ancient if you consider notebooks where you can't replace a GPU, as it is in my case. For conservative approach there is a Mesa stable branch, which is what distros are using by default anyway. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
On 13 April 2017 at 06:03, Markus Trippelsdorf wrote: > On 2017.04.12 at 20:45 +0100, Emil Velikov wrote: >> On 12 April 2017 at 20:34, Constantine Kharlamov wrote: >> >> >> I suspect this breaks because r600 more often fails to >> >> compile some shaders, >> >> and the hw requires a fragment shader and we use the empty one as a >> >> fallback in that case. >> > >> > Ok, that wasn't obvious, I running the system with the patchset on HD5730, >> > including >> > piglit testing — so far is fine. >> > >> If in doubt ask the bug reporter(s) to test a patch on their machines. > > Well, the question is why bother with this legacy code at all? > I would expect a much more conservative approach. Except for essential > patches to keep the hardware running, everything else should be > rejected. Nobody is expecting new features for this ancient hardware > anyway. I do have 90% of GL4.3 for evergreen and newer done in a branch, It's just a lack of time to fix the last few hangs and motivate myself to get it posted/reviewied. Then of course getting sb support. Dave. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
On 12 April 2017 at 20:34, Constantine Kharlamov wrote: >> I suspect this breaks because r600 more often fails to >> compile some shaders, >> and the hw requires a fragment shader and we use the empty one as a >> fallback in that case. > > Ok, that wasn't obvious, I running the system with the patchset on HD5730, > including > piglit testing — so far is fine. > If in doubt ask the bug reporter(s) to test a patch on their machines. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
On 12.04.2017 22:21, Dave Airlie wrote: > On 13 April 2017 at 03:39, Constantine Kharlamov wrote: >> On 12.04.2017 18:53, Marek Olšák wrote: >>> On Wed, Apr 12, 2017 at 4:44 PM, Markus Trippelsdorf >>> wrote: On 2017.04.10 at 22:48 +0200, Marek Olšák wrote: > Pushed the series, thanks! > > Marek > > On Mon, Apr 10, 2017 at 10:04 PM, Constantine Kharlamov > wrote: >> The idea is taken from radeonsi. The code mostly was already checking >> for null >> pixel shader, so little checks had to be added. >> >> Interestingly, acc. to testing with GTAⅣ, though binding of null shader >> happens >> a lot at the start (then just stops), but draw_vbo() never actually sees >> null >> ps. >> >> v2: added a check I missed because of a macros using a prefix to choose >> a shader. This commit causes ring 0 stalls on r600: https://bugs.freedesktop.org/show_bug.cgi?id=100663 >>> >>> Thanks, I reverted the commit. >>> >>> Marek >> >> Marek: do you have an idea why does it work for radeonsi though? I have >> almost no >> experience to graphics, so the only assumption comes to my mind: some >> r600g-managed >> card-specific behavior. > > Blindly porting stuff from radeonsi without understanding the > differences in the hardware and > drivers isn't going to be a great strategy, as you aren't really > learning more than the moving > code around. I disagree on this. A month ago I didn't even know a meaning of a shader aside that it's a graphics buzzword. But now I more or less understand shaders pipeline, what each type of shader does, and a bunch of other small details. The thing is: I keep telling that I'm a newbie to graphics, so simple mistakes are to be expected, at least for some time. And for that matter, that's the reason I just asked the question. > I suspect this breaks because r600 more often fails to > compile some shaders, > and the hw requires a fragment shader and we use the empty one as a > fallback in that case. Ok, that wasn't obvious, I running the system with the patchset on HD5730, including piglit testing — so far is fine. > Dave. > ___ > 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 3/3 v2] r600g: get rid of dummy pixel shader
On 13 April 2017 at 03:39, Constantine Kharlamov wrote: > On 12.04.2017 18:53, Marek Olšák wrote: >> On Wed, Apr 12, 2017 at 4:44 PM, Markus Trippelsdorf >> wrote: >>> On 2017.04.10 at 22:48 +0200, Marek Olšák wrote: Pushed the series, thanks! Marek On Mon, Apr 10, 2017 at 10:04 PM, Constantine Kharlamov wrote: > The idea is taken from radeonsi. The code mostly was already checking for > null > pixel shader, so little checks had to be added. > > Interestingly, acc. to testing with GTAⅣ, though binding of null shader > happens > a lot at the start (then just stops), but draw_vbo() never actually sees > null > ps. > > v2: added a check I missed because of a macros using a prefix to choose > a shader. >>> >>> This commit causes ring 0 stalls on r600: >>> https://bugs.freedesktop.org/show_bug.cgi?id=100663 >> >> Thanks, I reverted the commit. >> >> Marek > > Marek: do you have an idea why does it work for radeonsi though? I have > almost no > experience to graphics, so the only assumption comes to my mind: some > r600g-managed > card-specific behavior. Blindly porting stuff from radeonsi without understanding the differences in the hardware and drivers isn't going to be a great strategy, as you aren't really learning more than the moving code around. I suspect this breaks because r600 more often fails to compile some shaders, and the hw requires a fragment shader and we use the empty one as a fallback in that case. Dave. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
On 12.04.2017 18:53, Marek Olšák wrote: > On Wed, Apr 12, 2017 at 4:44 PM, Markus Trippelsdorf > wrote: >> On 2017.04.10 at 22:48 +0200, Marek Olšák wrote: >>> Pushed the series, thanks! >>> >>> Marek >>> >>> On Mon, Apr 10, 2017 at 10:04 PM, Constantine Kharlamov >>> wrote: The idea is taken from radeonsi. The code mostly was already checking for null pixel shader, so little checks had to be added. Interestingly, acc. to testing with GTAⅣ, though binding of null shader happens a lot at the start (then just stops), but draw_vbo() never actually sees null ps. v2: added a check I missed because of a macros using a prefix to choose a shader. >> >> This commit causes ring 0 stalls on r600: >> https://bugs.freedesktop.org/show_bug.cgi?id=100663 > > Thanks, I reverted the commit. > > Marek Marek: do you have an idea why does it work for radeonsi though? I have almost no experience to graphics, so the only assumption comes to my mind: some r600g-managed card-specific behavior. > ___ > 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 3/3 v2] r600g: get rid of dummy pixel shader
On Wed, Apr 12, 2017 at 4:44 PM, Markus Trippelsdorf wrote: > On 2017.04.10 at 22:48 +0200, Marek Olšák wrote: >> Pushed the series, thanks! >> >> Marek >> >> On Mon, Apr 10, 2017 at 10:04 PM, Constantine Kharlamov >> wrote: >> > The idea is taken from radeonsi. The code mostly was already checking for >> > null >> > pixel shader, so little checks had to be added. >> > >> > Interestingly, acc. to testing with GTAⅣ, though binding of null shader >> > happens >> > a lot at the start (then just stops), but draw_vbo() never actually sees >> > null >> > ps. >> > >> > v2: added a check I missed because of a macros using a prefix to choose >> > a shader. > > This commit causes ring 0 stalls on r600: > https://bugs.freedesktop.org/show_bug.cgi?id=100663 Thanks, I reverted the commit. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
Pushed the series, thanks!
Marek
On Mon, Apr 10, 2017 at 10:04 PM, Constantine Kharlamov
wrote:
> The idea is taken from radeonsi. The code mostly was already checking for null
> pixel shader, so little checks had to be added.
>
> Interestingly, acc. to testing with GTAⅣ, though binding of null shader
> happens
> a lot at the start (then just stops), but draw_vbo() never actually sees null
> ps.
>
> v2: added a check I missed because of a macros using a prefix to choose
> a shader.
>
> Signed-off-by: Constantine Kharlamov
> Reviewed-by: Marek Olšák
> ---
> src/gallium/drivers/r600/r600_pipe.c | 9 -
> src/gallium/drivers/r600/r600_pipe.h | 3 --
> src/gallium/drivers/r600/r600_state_common.c | 58
> ++--
> 3 files changed, 30 insertions(+), 40 deletions(-)
>
> diff --git a/src/gallium/drivers/r600/r600_pipe.c
> b/src/gallium/drivers/r600/r600_pipe.c
> index 5014f2525c..7d8efd2c9b 100644
> --- a/src/gallium/drivers/r600/r600_pipe.c
> +++ b/src/gallium/drivers/r600/r600_pipe.c
> @@ -82,9 +82,6 @@ static void r600_destroy_context(struct pipe_context
> *context)
> if (rctx->fixed_func_tcs_shader)
> rctx->b.b.delete_tcs_state(&rctx->b.b,
> rctx->fixed_func_tcs_shader);
>
> - if (rctx->dummy_pixel_shader) {
> - rctx->b.b.delete_fs_state(&rctx->b.b,
> rctx->dummy_pixel_shader);
> - }
> if (rctx->custom_dsa_flush) {
> rctx->b.b.delete_depth_stencil_alpha_state(&rctx->b.b,
> rctx->custom_dsa_flush);
> }
> @@ -209,12 +206,6 @@ static struct pipe_context *r600_create_context(struct
> pipe_screen *screen,
>
> r600_begin_new_cs(rctx);
>
> - rctx->dummy_pixel_shader =
> - util_make_fragment_cloneinput_shader(&rctx->b.b, 0,
> -TGSI_SEMANTIC_GENERIC,
> -
> TGSI_INTERPOLATE_CONSTANT);
> - rctx->b.b.bind_fs_state(&rctx->b.b, rctx->dummy_pixel_shader);
> -
> return &rctx->b.b;
>
> fail:
> diff --git a/src/gallium/drivers/r600/r600_pipe.h
> b/src/gallium/drivers/r600/r600_pipe.h
> index 7f1ecc278b..e636ef0024 100644
> --- a/src/gallium/drivers/r600/r600_pipe.h
> +++ b/src/gallium/drivers/r600/r600_pipe.h
> @@ -432,9 +432,6 @@ struct r600_context {
> void*custom_blend_resolve;
> void*custom_blend_decompress;
> void*custom_blend_fastclear;
> - /* With rasterizer discard, there doesn't have to be a pixel shader.
> -* In that case, we bind this one: */
> - void*dummy_pixel_shader;
> /* These dummy CMASK and FMASK buffers are used to get around the
> R6xx hardware
> * bug where valid CMASK and FMASK are required to be present to avoid
> * a hardlock in certain operations but aren't actually used
> diff --git a/src/gallium/drivers/r600/r600_state_common.c
> b/src/gallium/drivers/r600/r600_state_common.c
> index 5be49dcdfe..0131ea80d2 100644
> --- a/src/gallium/drivers/r600/r600_state_common.c
> +++ b/src/gallium/drivers/r600/r600_state_common.c
> @@ -725,7 +725,8 @@ static inline void r600_shader_selector_key(const struct
> pipe_context *ctx,
> if (!key->vs.as_ls)
> key->vs.as_es = (rctx->gs_shader != NULL);
>
> - if (rctx->ps_shader->current->shader.gs_prim_id_input &&
> !rctx->gs_shader) {
> + if (rctx->ps_shader &&
> rctx->ps_shader->current->shader.gs_prim_id_input &&
> + !rctx->gs_shader) {
> key->vs.as_gs_a = true;
> key->vs.prim_id_out =
> rctx->ps_shader->current->shader.input[rctx->ps_shader->current->shader.ps_prim_id_input].spi_sid;
> }
> @@ -909,9 +910,6 @@ static void r600_bind_ps_state(struct pipe_context *ctx,
> void *state)
> {
> struct r600_context *rctx = (struct r600_context *)ctx;
>
> - if (!state)
> - state = rctx->dummy_pixel_shader;
> -
> rctx->ps_shader = (struct r600_pipe_shader_selector *)state;
> }
>
> @@ -1478,7 +1476,8 @@ static bool r600_update_derived_state(struct
> r600_context *rctx)
> }
> }
>
> - SELECT_SHADER_OR_FAIL(ps);
> + if (rctx->ps_shader)
> + SELECT_SHADER_OR_FAIL(ps);
>
> r600_mark_atom_dirty(rctx, &rctx->shader_stages.atom);
>
> @@ -1555,37 +1554,40 @@ static bool r600_update_derived_state(struct
> r600_context *rctx)
> rctx->b.streamout.enabled_stream_buffers_mask =
> clip_so_current->enabled_stream_buffers_mask;
> }
>
> - if (unlikely(ps_dirty ||
> rctx->hw_shader_stages[R600_HW_STAGE_PS].shader != rctx->ps_shader->current ||
> - rctx->rasterizer->sprite_coord_enable !=
> rctx->ps_shader->current->sprite_coord_enable ||
> -
Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
For patches 2-3:
Reviewed-by: Marek Olšák
Marek
On Mon, Apr 10, 2017 at 11:44 AM, Constantine Kharlamov
wrote:
> If that helps, I can split this patch to two: α) Adding checks for null ps,
> and β) removing the dummy ps. I didn't do that originally, because the patch
> was small anyway, it's in the 2-nd version that I had to re-indent a block of
> code, and now it looks bigger.
>
> On 10.04.2017 00:09, Constantine Kharlamov wrote:
>> The idea is taken from radeonsi. The code mostly was already checking for
>> null
>> pixel shader, so little checks had to be added.
>>
>> Interestingly, acc. to testing with GTAⅣ, though binding of null shader
>> happens
>> a lot at the start (then just stops), but draw_vbo() never actually sees null
>> ps.
>>
>> v2: added a check I missed because of a macros using a prefix to choose
>> a shader.
>>
>> Signed-off-by: Constantine Kharlamov
>> ---
>> src/gallium/drivers/r600/r600_pipe.c | 9 -
>> src/gallium/drivers/r600/r600_pipe.h | 3 --
>> src/gallium/drivers/r600/r600_state_common.c | 58
>> ++--
>> 3 files changed, 30 insertions(+), 40 deletions(-)
>>
>> diff --git a/src/gallium/drivers/r600/r600_pipe.c
>> b/src/gallium/drivers/r600/r600_pipe.c
>> index 5014f2525c..7d8efd2c9b 100644
>> --- a/src/gallium/drivers/r600/r600_pipe.c
>> +++ b/src/gallium/drivers/r600/r600_pipe.c
>> @@ -82,9 +82,6 @@ static void r600_destroy_context(struct pipe_context
>> *context)
>> if (rctx->fixed_func_tcs_shader)
>> rctx->b.b.delete_tcs_state(&rctx->b.b,
>> rctx->fixed_func_tcs_shader);
>>
>> - if (rctx->dummy_pixel_shader) {
>> - rctx->b.b.delete_fs_state(&rctx->b.b,
>> rctx->dummy_pixel_shader);
>> - }
>> if (rctx->custom_dsa_flush) {
>> rctx->b.b.delete_depth_stencil_alpha_state(&rctx->b.b,
>> rctx->custom_dsa_flush);
>> }
>> @@ -209,12 +206,6 @@ static struct pipe_context *r600_create_context(struct
>> pipe_screen *screen,
>>
>> r600_begin_new_cs(rctx);
>>
>> - rctx->dummy_pixel_shader =
>> - util_make_fragment_cloneinput_shader(&rctx->b.b, 0,
>> - TGSI_SEMANTIC_GENERIC,
>> -
>> TGSI_INTERPOLATE_CONSTANT);
>> - rctx->b.b.bind_fs_state(&rctx->b.b, rctx->dummy_pixel_shader);
>> -
>> return &rctx->b.b;
>>
>> fail:
>> diff --git a/src/gallium/drivers/r600/r600_pipe.h
>> b/src/gallium/drivers/r600/r600_pipe.h
>> index 7f1ecc278b..e636ef0024 100644
>> --- a/src/gallium/drivers/r600/r600_pipe.h
>> +++ b/src/gallium/drivers/r600/r600_pipe.h
>> @@ -432,9 +432,6 @@ struct r600_context {
>> void*custom_blend_resolve;
>> void*custom_blend_decompress;
>> void*custom_blend_fastclear;
>> - /* With rasterizer discard, there doesn't have to be a pixel shader.
>> - * In that case, we bind this one: */
>> - void*dummy_pixel_shader;
>> /* These dummy CMASK and FMASK buffers are used to get around the R6xx
>> hardware
>>* bug where valid CMASK and FMASK are required to be present to avoid
>>* a hardlock in certain operations but aren't actually used
>> diff --git a/src/gallium/drivers/r600/r600_state_common.c
>> b/src/gallium/drivers/r600/r600_state_common.c
>> index c9b41517cc..8d1193360b 100644
>> --- a/src/gallium/drivers/r600/r600_state_common.c
>> +++ b/src/gallium/drivers/r600/r600_state_common.c
>> @@ -725,7 +725,8 @@ static inline void r600_shader_selector_key(const struct
>> pipe_context *ctx,
>> if (!key->vs.as_ls)
>> key->vs.as_es = (rctx->gs_shader != NULL);
>>
>> - if (rctx->ps_shader->current->shader.gs_prim_id_input &&
>> !rctx->gs_shader) {
>> + if (rctx->ps_shader &&
>> rctx->ps_shader->current->shader.gs_prim_id_input &&
>> + !rctx->gs_shader) {
>> key->vs.as_gs_a = true;
>> key->vs.prim_id_out =
>> rctx->ps_shader->current->shader.input[rctx->ps_shader->current->shader.ps_prim_id_input].spi_sid;
>> }
>> @@ -909,9 +910,6 @@ static void r600_bind_ps_state(struct pipe_context *ctx,
>> void *state)
>> {
>> struct r600_context *rctx = (struct r600_context *)ctx;
>>
>> - if (!state)
>> - state = rctx->dummy_pixel_shader;
>> -
>> rctx->ps_shader = (struct r600_pipe_shader_selector *)state;
>> }
>>
>> @@ -1474,7 +1472,8 @@ static bool r600_update_derived_state(struct
>> r600_context *rctx)
>> }
>> }
>>
>> - SELECT_SHADER_OR_FAIL(ps);
>> + if (rctx->ps_shader)
>> + SELECT_SHADER_OR_FAIL(ps);
>>
>> r600_mark_atom_dirty(rctx, &rctx->shader_stages.atom);
>>
>> @@ -1551,37 +1550,40 @@ static bool r600_update_derived_state(struct
>> r600_context *rctx)
>>
Re: [Mesa-dev] [PATCH 3/3 v2] r600g: get rid of dummy pixel shader
If that helps, I can split this patch to two: α) Adding checks for null ps, and
β) removing the dummy ps. I didn't do that originally, because the patch was
small anyway, it's in the 2-nd version that I had to re-indent a block of code,
and now it looks bigger.
On 10.04.2017 00:09, Constantine Kharlamov wrote:
> The idea is taken from radeonsi. The code mostly was already checking for null
> pixel shader, so little checks had to be added.
>
> Interestingly, acc. to testing with GTAⅣ, though binding of null shader
> happens
> a lot at the start (then just stops), but draw_vbo() never actually sees null
> ps.
>
> v2: added a check I missed because of a macros using a prefix to choose
> a shader.
>
> Signed-off-by: Constantine Kharlamov
> ---
> src/gallium/drivers/r600/r600_pipe.c | 9 -
> src/gallium/drivers/r600/r600_pipe.h | 3 --
> src/gallium/drivers/r600/r600_state_common.c | 58
> ++--
> 3 files changed, 30 insertions(+), 40 deletions(-)
>
> diff --git a/src/gallium/drivers/r600/r600_pipe.c
> b/src/gallium/drivers/r600/r600_pipe.c
> index 5014f2525c..7d8efd2c9b 100644
> --- a/src/gallium/drivers/r600/r600_pipe.c
> +++ b/src/gallium/drivers/r600/r600_pipe.c
> @@ -82,9 +82,6 @@ static void r600_destroy_context(struct pipe_context
> *context)
> if (rctx->fixed_func_tcs_shader)
> rctx->b.b.delete_tcs_state(&rctx->b.b,
> rctx->fixed_func_tcs_shader);
>
> - if (rctx->dummy_pixel_shader) {
> - rctx->b.b.delete_fs_state(&rctx->b.b, rctx->dummy_pixel_shader);
> - }
> if (rctx->custom_dsa_flush) {
> rctx->b.b.delete_depth_stencil_alpha_state(&rctx->b.b,
> rctx->custom_dsa_flush);
> }
> @@ -209,12 +206,6 @@ static struct pipe_context *r600_create_context(struct
> pipe_screen *screen,
>
> r600_begin_new_cs(rctx);
>
> - rctx->dummy_pixel_shader =
> - util_make_fragment_cloneinput_shader(&rctx->b.b, 0,
> - TGSI_SEMANTIC_GENERIC,
> - TGSI_INTERPOLATE_CONSTANT);
> - rctx->b.b.bind_fs_state(&rctx->b.b, rctx->dummy_pixel_shader);
> -
> return &rctx->b.b;
>
> fail:
> diff --git a/src/gallium/drivers/r600/r600_pipe.h
> b/src/gallium/drivers/r600/r600_pipe.h
> index 7f1ecc278b..e636ef0024 100644
> --- a/src/gallium/drivers/r600/r600_pipe.h
> +++ b/src/gallium/drivers/r600/r600_pipe.h
> @@ -432,9 +432,6 @@ struct r600_context {
> void*custom_blend_resolve;
> void*custom_blend_decompress;
> void*custom_blend_fastclear;
> - /* With rasterizer discard, there doesn't have to be a pixel shader.
> - * In that case, we bind this one: */
> - void*dummy_pixel_shader;
> /* These dummy CMASK and FMASK buffers are used to get around the R6xx
> hardware
>* bug where valid CMASK and FMASK are required to be present to avoid
>* a hardlock in certain operations but aren't actually used
> diff --git a/src/gallium/drivers/r600/r600_state_common.c
> b/src/gallium/drivers/r600/r600_state_common.c
> index c9b41517cc..8d1193360b 100644
> --- a/src/gallium/drivers/r600/r600_state_common.c
> +++ b/src/gallium/drivers/r600/r600_state_common.c
> @@ -725,7 +725,8 @@ static inline void r600_shader_selector_key(const struct
> pipe_context *ctx,
> if (!key->vs.as_ls)
> key->vs.as_es = (rctx->gs_shader != NULL);
>
> - if (rctx->ps_shader->current->shader.gs_prim_id_input &&
> !rctx->gs_shader) {
> + if (rctx->ps_shader &&
> rctx->ps_shader->current->shader.gs_prim_id_input &&
> + !rctx->gs_shader) {
> key->vs.as_gs_a = true;
> key->vs.prim_id_out =
> rctx->ps_shader->current->shader.input[rctx->ps_shader->current->shader.ps_prim_id_input].spi_sid;
> }
> @@ -909,9 +910,6 @@ static void r600_bind_ps_state(struct pipe_context *ctx,
> void *state)
> {
> struct r600_context *rctx = (struct r600_context *)ctx;
>
> - if (!state)
> - state = rctx->dummy_pixel_shader;
> -
> rctx->ps_shader = (struct r600_pipe_shader_selector *)state;
> }
>
> @@ -1474,7 +1472,8 @@ static bool r600_update_derived_state(struct
> r600_context *rctx)
> }
> }
>
> - SELECT_SHADER_OR_FAIL(ps);
> + if (rctx->ps_shader)
> + SELECT_SHADER_OR_FAIL(ps);
>
> r600_mark_atom_dirty(rctx, &rctx->shader_stages.atom);
>
> @@ -1551,37 +1550,40 @@ static bool r600_update_derived_state(struct
> r600_context *rctx)
> rctx->b.streamout.enabled_stream_buffers_mask =
> clip_so_current->enabled_stream_buffers_mask;
> }
>
> - if (unlikely(ps_dirty ||
> rctx->hw_shader_stages[R600_HW_STAGE_PS].shader != rctx->ps_shader->current |
