Re: [Mesa-dev] [PATCH] radv: Really use correct HTILE expanded words.

[Bas Nieuwenhuizen]

On Fri, Feb 23, 2018 at 11:52 AM, James Legg  wrote:
> On Thu, 2018-02-22 at 22:48 +0100, Bas Nieuwenhuizen wrote:
>> since IIRC the last change was also done due to Feral noticing and we
>> are clearly lacking testcases in this area, can you check that that
>> case still works for you? Thanks a lot!
>
> I looked at an issue that was fixed with 5158603182fe7435 (and still
> occurs if I change the clear word back to 0x) and I can confirm
> this patch does not reintroduce it.

Nice, I fixed the Fixes tag and pushed it. Thanks a lot!

>
>> On Thu, Feb 22, 2018 at 5:57 PM, James Legg  
>> wrote:
>> > When transitioning to an htile compressed depth format, Set the full
>> > depth range, so later rasterization can pass HiZ. Previously, for depth
>> > only formats, the depth range was set to 0 to 0. This caused unwanted
>> > HiZ rejections with a VK_FORMAT_D16_UNORM depth buffer
>> > (VK_FORMAT_D32_SFLOAT was not affected somehow).
>> >
>> > These values are derived from PAL [0], since I can't find the
>> > specification describing the htile values.
>> >
>> > Fixes 5158603182fe7435: radv: Use correct HTILE expanded words.
>> >
>> > [0] 
>> > https://github.com/GPUOpen-Drivers/pal/blob/5cba4ecbda9452773f59692f5915301e7db4a183/src/core/hw/gfxip/gfx9/gfx9MaskRam.cpp#L1500
>> >
>> > CC: Dave Airlie 
>> > CC: Bas Nieuwenhuizen 
>> > Cc: mesa-sta...@lists.freedesktop.org
>> > ---
>> >  src/amd/vulkan/radv_cmd_buffer.c | 6 +++---
>> >  1 file changed, 3 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
>> > b/src/amd/vulkan/radv_cmd_buffer.c
>> > index 8a384b114c..2b41baea3d 100644
>> > --- a/src/amd/vulkan/radv_cmd_buffer.c
>> > +++ b/src/amd/vulkan/radv_cmd_buffer.c
>> > @@ -3440,8 +3440,8 @@ void radv_CmdEndRenderPass(
>> >
>> >  /*
>> >   * For HTILE we have the following interesting clear words:
>> > - *   0x030f: Uncompressed for depth+stencil HTILE.
>> > - *   0x000f: Uncompressed for depth only HTILE.
>> > + *   0xf30f: Uncompressed, full depth range, for depth+stencil HTILE
>> > + *   0xfffc000f: Uncompressed, full depth range, for depth only HTILE.
>> >   *   0xfff0: Clear depth to 1.0
>> >   *   0x: Clear depth to 0.0
>> >   */
>> > @@ -3489,7 +3489,7 @@ static void 
>> > radv_handle_depth_image_transition(struct radv_cmd_buffer *cmd_buffe
>> > radv_initialize_htile(cmd_buffer, image, range, 0);
>> > } else if (!radv_layout_is_htile_compressed(image, src_layout, 
>> > src_queue_mask) &&
>> >radv_layout_is_htile_compressed(image, dst_layout, 
>> > dst_queue_mask)) {
>> > -   uint32_t clear_value = 
>> > vk_format_is_stencil(image->vk_format) ? 0x30f : 0xf;
>> > +   uint32_t clear_value = 
>> > vk_format_is_stencil(image->vk_format) ? 0xf30f : 0xfffc000f;
>> > radv_initialize_htile(cmd_buffer, image, range, 
>> > clear_value);
>> > } else if (radv_layout_is_htile_compressed(image, src_layout, 
>> > src_queue_mask) &&
>> >!radv_layout_is_htile_compressed(image, dst_layout, 
>> > dst_queue_mask)) {
>> > --
>> > 2.14.3
>> >
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] radv: Really use correct HTILE expanded words.

[James Legg]

On Thu, 2018-02-22 at 22:48 +0100, Bas Nieuwenhuizen wrote:
> since IIRC the last change was also done due to Feral noticing and we
> are clearly lacking testcases in this area, can you check that that
> case still works for you? Thanks a lot!

I looked at an issue that was fixed with 5158603182fe7435 (and still
occurs if I change the clear word back to 0x) and I can confirm
this patch does not reintroduce it.

> On Thu, Feb 22, 2018 at 5:57 PM, James Legg  
> wrote:
> > When transitioning to an htile compressed depth format, Set the full
> > depth range, so later rasterization can pass HiZ. Previously, for depth
> > only formats, the depth range was set to 0 to 0. This caused unwanted
> > HiZ rejections with a VK_FORMAT_D16_UNORM depth buffer
> > (VK_FORMAT_D32_SFLOAT was not affected somehow).
> > 
> > These values are derived from PAL [0], since I can't find the
> > specification describing the htile values.
> > 
> > Fixes 5158603182fe7435: radv: Use correct HTILE expanded words.
> > 
> > [0] 
> > https://github.com/GPUOpen-Drivers/pal/blob/5cba4ecbda9452773f59692f5915301e7db4a183/src/core/hw/gfxip/gfx9/gfx9MaskRam.cpp#L1500
> > 
> > CC: Dave Airlie 
> > CC: Bas Nieuwenhuizen 
> > Cc: mesa-sta...@lists.freedesktop.org
> > ---
> >  src/amd/vulkan/radv_cmd_buffer.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
> > b/src/amd/vulkan/radv_cmd_buffer.c
> > index 8a384b114c..2b41baea3d 100644
> > --- a/src/amd/vulkan/radv_cmd_buffer.c
> > +++ b/src/amd/vulkan/radv_cmd_buffer.c
> > @@ -3440,8 +3440,8 @@ void radv_CmdEndRenderPass(
> > 
> >  /*
> >   * For HTILE we have the following interesting clear words:
> > - *   0x030f: Uncompressed for depth+stencil HTILE.
> > - *   0x000f: Uncompressed for depth only HTILE.
> > + *   0xf30f: Uncompressed, full depth range, for depth+stencil HTILE
> > + *   0xfffc000f: Uncompressed, full depth range, for depth only HTILE.
> >   *   0xfff0: Clear depth to 1.0
> >   *   0x: Clear depth to 0.0
> >   */
> > @@ -3489,7 +3489,7 @@ static void radv_handle_depth_image_transition(struct 
> > radv_cmd_buffer *cmd_buffe
> > radv_initialize_htile(cmd_buffer, image, range, 0);
> > } else if (!radv_layout_is_htile_compressed(image, src_layout, 
> > src_queue_mask) &&
> >radv_layout_is_htile_compressed(image, dst_layout, 
> > dst_queue_mask)) {
> > -   uint32_t clear_value = 
> > vk_format_is_stencil(image->vk_format) ? 0x30f : 0xf;
> > +   uint32_t clear_value = 
> > vk_format_is_stencil(image->vk_format) ? 0xf30f : 0xfffc000f;
> > radv_initialize_htile(cmd_buffer, image, range, 
> > clear_value);
> > } else if (radv_layout_is_htile_compressed(image, src_layout, 
> > src_queue_mask) &&
> >!radv_layout_is_htile_compressed(image, dst_layout, 
> > dst_queue_mask)) {
> > --
> > 2.14.3
> > 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] radv: Really use correct HTILE expanded words.

[Grazvydas Ignotas]

Seems to fix dxvk, nice! Also tested DOOM which still works.
Tested-by: Grazvydas Ignotas 

On Thu, Feb 22, 2018 at 6:57 PM, James Legg  wrote:
> When transitioning to an htile compressed depth format, Set the full
> depth range, so later rasterization can pass HiZ. Previously, for depth
> only formats, the depth range was set to 0 to 0. This caused unwanted
> HiZ rejections with a VK_FORMAT_D16_UNORM depth buffer
> (VK_FORMAT_D32_SFLOAT was not affected somehow).
>
> These values are derived from PAL [0], since I can't find the
> specification describing the htile values.
>
> Fixes 5158603182fe7435: radv: Use correct HTILE expanded words.

Please put this as "Fixes: ..." below along with other tags.

>
> [0] 
> https://github.com/GPUOpen-Drivers/pal/blob/5cba4ecbda9452773f59692f5915301e7db4a183/src/core/hw/gfxip/gfx9/gfx9MaskRam.cpp#L1500
>
> CC: Dave Airlie 
> CC: Bas Nieuwenhuizen 
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/amd/vulkan/radv_cmd_buffer.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
> b/src/amd/vulkan/radv_cmd_buffer.c
> index 8a384b114c..2b41baea3d 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -3440,8 +3440,8 @@ void radv_CmdEndRenderPass(
>
>  /*
>   * For HTILE we have the following interesting clear words:
> - *   0x030f: Uncompressed for depth+stencil HTILE.
> - *   0x000f: Uncompressed for depth only HTILE.
> + *   0xf30f: Uncompressed, full depth range, for depth+stencil HTILE
> + *   0xfffc000f: Uncompressed, full depth range, for depth only HTILE.
>   *   0xfff0: Clear depth to 1.0
>   *   0x: Clear depth to 0.0
>   */
> @@ -3489,7 +3489,7 @@ static void radv_handle_depth_image_transition(struct 
> radv_cmd_buffer *cmd_buffe
> radv_initialize_htile(cmd_buffer, image, range, 0);
> } else if (!radv_layout_is_htile_compressed(image, src_layout, 
> src_queue_mask) &&
>radv_layout_is_htile_compressed(image, dst_layout, 
> dst_queue_mask)) {
> -   uint32_t clear_value = vk_format_is_stencil(image->vk_format) 
> ? 0x30f : 0xf;
> +   uint32_t clear_value = vk_format_is_stencil(image->vk_format) 
> ? 0xf30f : 0xfffc000f;
> radv_initialize_htile(cmd_buffer, image, range, clear_value);
> } else if (radv_layout_is_htile_compressed(image, src_layout, 
> src_queue_mask) &&
>!radv_layout_is_htile_compressed(image, dst_layout, 
> dst_queue_mask)) {
> --
> 2.14.3
>
> ___
> 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] radv: Really use correct HTILE expanded words.

[Bas Nieuwenhuizen]

yeah, looks like I may have lifted the values from a driver which set
the ZRANGE_PRECISION to 0, but this at least mirrors PAL and fixes one
of the remaining transition issues there that I was aware of.

Reviewed-by: Bas Nieuwenhuizen 

since IIRC the last change was also done due to Feral noticing and we
are clearly lacking testcases in this area, can you check that that
case still works for you? Thanks a lot!



On Thu, Feb 22, 2018 at 5:57 PM, James Legg  wrote:
> When transitioning to an htile compressed depth format, Set the full
> depth range, so later rasterization can pass HiZ. Previously, for depth
> only formats, the depth range was set to 0 to 0. This caused unwanted
> HiZ rejections with a VK_FORMAT_D16_UNORM depth buffer
> (VK_FORMAT_D32_SFLOAT was not affected somehow).
>
> These values are derived from PAL [0], since I can't find the
> specification describing the htile values.
>
> Fixes 5158603182fe7435: radv: Use correct HTILE expanded words.
>
> [0] 
> https://github.com/GPUOpen-Drivers/pal/blob/5cba4ecbda9452773f59692f5915301e7db4a183/src/core/hw/gfxip/gfx9/gfx9MaskRam.cpp#L1500
>
> CC: Dave Airlie 
> CC: Bas Nieuwenhuizen 
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/amd/vulkan/radv_cmd_buffer.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
> b/src/amd/vulkan/radv_cmd_buffer.c
> index 8a384b114c..2b41baea3d 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -3440,8 +3440,8 @@ void radv_CmdEndRenderPass(
>
>  /*
>   * For HTILE we have the following interesting clear words:
> - *   0x030f: Uncompressed for depth+stencil HTILE.
> - *   0x000f: Uncompressed for depth only HTILE.
> + *   0xf30f: Uncompressed, full depth range, for depth+stencil HTILE
> + *   0xfffc000f: Uncompressed, full depth range, for depth only HTILE.
>   *   0xfff0: Clear depth to 1.0
>   *   0x: Clear depth to 0.0
>   */
> @@ -3489,7 +3489,7 @@ static void radv_handle_depth_image_transition(struct 
> radv_cmd_buffer *cmd_buffe
> radv_initialize_htile(cmd_buffer, image, range, 0);
> } else if (!radv_layout_is_htile_compressed(image, src_layout, 
> src_queue_mask) &&
>radv_layout_is_htile_compressed(image, dst_layout, 
> dst_queue_mask)) {
> -   uint32_t clear_value = vk_format_is_stencil(image->vk_format) 
> ? 0x30f : 0xf;
> +   uint32_t clear_value = vk_format_is_stencil(image->vk_format) 
> ? 0xf30f : 0xfffc000f;
> radv_initialize_htile(cmd_buffer, image, range, clear_value);
> } else if (radv_layout_is_htile_compressed(image, src_layout, 
> src_queue_mask) &&
>!radv_layout_is_htile_compressed(image, dst_layout, 
> dst_queue_mask)) {
> --
> 2.14.3
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] radv: Really use correct HTILE expanded words.

[James Legg]

When transitioning to an htile compressed depth format, Set the full
depth range, so later rasterization can pass HiZ. Previously, for depth
only formats, the depth range was set to 0 to 0. This caused unwanted
HiZ rejections with a VK_FORMAT_D16_UNORM depth buffer
(VK_FORMAT_D32_SFLOAT was not affected somehow).

These values are derived from PAL [0], since I can't find the
specification describing the htile values.

Fixes 5158603182fe7435: radv: Use correct HTILE expanded words.

[0] 
https://github.com/GPUOpen-Drivers/pal/blob/5cba4ecbda9452773f59692f5915301e7db4a183/src/core/hw/gfxip/gfx9/gfx9MaskRam.cpp#L1500

CC: Dave Airlie 
CC: Bas Nieuwenhuizen 
Cc: mesa-sta...@lists.freedesktop.org
---
 src/amd/vulkan/radv_cmd_buffer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 8a384b114c..2b41baea3d 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -3440,8 +3440,8 @@ void radv_CmdEndRenderPass(
 
 /*
  * For HTILE we have the following interesting clear words:
- *   0x030f: Uncompressed for depth+stencil HTILE.
- *   0x000f: Uncompressed for depth only HTILE.
+ *   0xf30f: Uncompressed, full depth range, for depth+stencil HTILE
+ *   0xfffc000f: Uncompressed, full depth range, for depth only HTILE.
  *   0xfff0: Clear depth to 1.0
  *   0x: Clear depth to 0.0
  */
@@ -3489,7 +3489,7 @@ static void radv_handle_depth_image_transition(struct 
radv_cmd_buffer *cmd_buffe
radv_initialize_htile(cmd_buffer, image, range, 0);
} else if (!radv_layout_is_htile_compressed(image, src_layout, 
src_queue_mask) &&
   radv_layout_is_htile_compressed(image, dst_layout, 
dst_queue_mask)) {
-   uint32_t clear_value = vk_format_is_stencil(image->vk_format) ? 
0x30f : 0xf;
+   uint32_t clear_value = vk_format_is_stencil(image->vk_format) ? 
0xf30f : 0xfffc000f;
radv_initialize_htile(cmd_buffer, image, range, clear_value);
} else if (radv_layout_is_htile_compressed(image, src_layout, 
src_queue_mask) &&
   !radv_layout_is_htile_compressed(image, dst_layout, 
dst_queue_mask)) {
-- 
2.14.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev