Re: [Mesa-dev] [PATCH 1/4] gallium/radeon: add clear_texture function

2016-04-16 Thread Grigori Goronzy 

On 2016-04-15 20:30, Jakob Sinclair wrote:

In other places in radeonsi that require reinterpretation (e.g.
si_blit.c), the surface template is modified instead of changing the
surface after creation. I'm not sure if r600/radeonsi like it if the
format is changed late like here. Seems to be cleaner and clearer to
change the template anyway.



Thanks for the information. Just want to make sure I got the surface
template solution right:

First you create an surface object which you will then copy over
information too. Then you
can edit things like the format without worrying about something going
horribly wrong.
Then you can use that object to create a new surface which the old
surface can now point too.



No, you just declare a regular struct pipe_surface and set it up by 
settings its fields. This is called the template. The template is then 
used for the surface creation call to describe the desired properties of 
the surface object. It can be a bit confusing because the same type is 
used for surfaces and surface templates, I guess.


Refer to si_blit.c:si_resource_copy_region to see how it's done.

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

Re: [Mesa-dev] [PATCH 1/4] gallium/radeon: add clear_texture function

2016-04-15 Thread Jakob Sinclair 

On 2016-04-15 19:23, Grigori Goronzy wrote:

On 2016-04-15 18:38, Ilia Mirkin wrote:

+   } else {
+   union pipe_color_union color;
+   switch (util_format_get_blocksizebits(res->format)) {
+   case 128:
+   sf->format = PIPE_FORMAT_R32G32B32A32_UINT;


Just as an FYI... this is safe on nouveau because I control all the
internals and know that this is safe to do. Please verify that it's
similarly safe to change the surface format after creation on radeon -
it might not be. (Esp for compressed textures...)



In other places in radeonsi that require reinterpretation (e.g.
si_blit.c), the surface template is modified instead of changing the
surface after creation. I'm not sure if r600/radeonsi like it if the
format is changed late like here. Seems to be cleaner and clearer to
change the template anyway.

Grigori


Thanks for the information. Just want to make sure I got the surface 
template solution right:


First you create an surface object which you will then copy over 
information too. Then you
can edit things like the format without worrying about something going 
horribly wrong.
Then you can use that object to create a new surface which the old 
surface can now point too.


Did I get the idea behind it or did I misunderstand the whole thing?

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

Re: [Mesa-dev] [PATCH 1/4] gallium/radeon: add clear_texture function

2016-04-15 Thread Grigori Goronzy 

On 2016-04-15 18:38, Ilia Mirkin wrote:

+   } else {
+   union pipe_color_union color;
+   switch (util_format_get_blocksizebits(res->format)) {
+   case 128:
+   sf->format = PIPE_FORMAT_R32G32B32A32_UINT;


Just as an FYI... this is safe on nouveau because I control all the
internals and know that this is safe to do. Please verify that it's
similarly safe to change the surface format after creation on radeon -
it might not be. (Esp for compressed textures...)



In other places in radeonsi that require reinterpretation (e.g. 
si_blit.c), the surface template is modified instead of changing the 
surface after creation. I'm not sure if r600/radeonsi like it if the 
format is changed late like here. Seems to be cleaner and clearer to 
change the template anyway.


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

Re: [Mesa-dev] [PATCH 1/4] gallium/radeon: add clear_texture function

2016-04-15 Thread Ilia Mirkin 
On Fri, Apr 15, 2016 at 12:33 PM, Jakob Sinclair
 wrote:
> This patch adds the needed function for ARB_clear_texture.
> The function itself is mostly based on the nouveau implementation.
>
> Signed-off-by: Jakob Sinclair 
> ---
>  src/gallium/drivers/radeon/r600_texture.c | 72 
> +++
>  1 file changed, 72 insertions(+)
>
> diff --git a/src/gallium/drivers/radeon/r600_texture.c 
> b/src/gallium/drivers/radeon/r600_texture.c
> index 72af534..ee77a37 100644
> --- a/src/gallium/drivers/radeon/r600_texture.c
> +++ b/src/gallium/drivers/radeon/r600_texture.c
> @@ -1392,6 +1392,77 @@ static void r600_surface_destroy(struct pipe_context 
> *pipe,
> FREE(surface);
>  }
>
> +static void r600_clear_texture(struct pipe_context *pipe,
> +   struct pipe_resource *res,
> +   unsigned level,
> +   const struct pipe_box *box,
> +   const void *data)
> +{
> +   struct pipe_surface tmpl = {{0}}, *sf;
> +
> +   tmpl.format = res->format;
> +   tmpl.u.tex.first_layer = box->z;
> +   tmpl.u.tex.last_layer = box->z + box->depth - 1;
> +   tmpl.u.tex.level = level;
> +   sf = pipe->create_surface(pipe, res, );
> +   if (!sf)
> +   return;
> +
> +   if (util_format_is_depth_or_stencil(res->format)) {
> +   float depth = 0;
> +   uint8_t stencil = 0;
> +   unsigned clear = 0;
> +   const struct util_format_description *desc =
> +   util_format_description(res->format);
> +
> +   if (util_format_has_depth(desc)) {
> +   clear |= PIPE_CLEAR_DEPTH;
> +   desc->unpack_z_float(, 0, data, 0, 1, 1);
> +   }
> +   if (util_format_has_stencil(desc)) {
> +   clear |= PIPE_CLEAR_STENCIL;
> +   desc->unpack_s_8uint(, 0, data, 0, 1, 1);
> +   }
> +   pipe->clear_depth_stencil(pipe, sf, clear, depth, stencil,
> + box->x, box->y, box->width, 
> box->height);
> +   } else {
> +   union pipe_color_union color;
> +   switch (util_format_get_blocksizebits(res->format)) {
> +   case 128:
> +   sf->format = PIPE_FORMAT_R32G32B32A32_UINT;
> +   memcpy(, data, 128 / 8);
> +   break;
> +   case 64:
> +   sf->format = PIPE_FORMAT_R32G32_UINT;
> +   memcpy(, data, 64 / 8);
> +   memset([2], 0, 64 / 8);
> +   break;
> +   case 32:
> +   sf->format = PIPE_FORMAT_R32_UINT;
> +   memcpy(, data, 32 / 8);
> +   memset([1], 0, 96 / 8);
> +   break;
> +   case 16:
> +   sf->format = PIPE_FORMAT_R16_UINT;
> +   color.ui[0] = util_cpu_to_le32(
> +   util_le16_to_cpu(*(unsigned short *)data));
> +   memset([1], 0, 96 / 8);
> +   break;
> +   case 8:
> +   sf->format = PIPE_FORMAT_R8_UINT;
> +   color.ui[0] = util_cpu_to_le32(*(unsigned char 
> *)data);
> +   memset([1], 0, 96 / 8);
> +   break;
> +   default:
> +   assert(!"Unknown texel element size");
> +   return;
> +   }
> +   pipe->clear_render_target(pipe, sf, ,
> + box->x, box->y, box->width, 
> box->height);

Oh, and another thing -- clear_render_target and clear_depth_stencil
are supposed to obey the render condition, while clear texture is not.
This is an issue on nouveau as well, which I solved by making
clear_render_target/clear_depth_stencil do the wrong thing (and
ignoring the render condition). However that's not a great generic
solution.

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

Re: [Mesa-dev] [PATCH 1/4] gallium/radeon: add clear_texture function

2016-04-15 Thread Jakob Sinclair 

On 2016-04-15 18:38, Ilia Mirkin wrote:


Just as an FYI... this is safe on nouveau because I control all the
internals and know that this is safe to do. Please verify that it's
similarly safe to change the surface format after creation on radeon -
it might not be. (Esp for compressed textures...)



Thanks for the heads up. It did at least not cause any errors on the 
piglit test.
I will need to probably check with other radeon developers to see if 
this is actually safe.


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

Re: [Mesa-dev] [PATCH 1/4] gallium/radeon: add clear_texture function

2016-04-15 Thread Ilia Mirkin 
On Fri, Apr 15, 2016 at 12:33 PM, Jakob Sinclair
 wrote:
> This patch adds the needed function for ARB_clear_texture.
> The function itself is mostly based on the nouveau implementation.
>
> Signed-off-by: Jakob Sinclair 
> ---
>  src/gallium/drivers/radeon/r600_texture.c | 72 
> +++
>  1 file changed, 72 insertions(+)
>
> diff --git a/src/gallium/drivers/radeon/r600_texture.c 
> b/src/gallium/drivers/radeon/r600_texture.c
> index 72af534..ee77a37 100644
> --- a/src/gallium/drivers/radeon/r600_texture.c
> +++ b/src/gallium/drivers/radeon/r600_texture.c
> @@ -1392,6 +1392,77 @@ static void r600_surface_destroy(struct pipe_context 
> *pipe,
> FREE(surface);
>  }
>
> +static void r600_clear_texture(struct pipe_context *pipe,
> +   struct pipe_resource *res,
> +   unsigned level,
> +   const struct pipe_box *box,
> +   const void *data)
> +{
> +   struct pipe_surface tmpl = {{0}}, *sf;
> +
> +   tmpl.format = res->format;
> +   tmpl.u.tex.first_layer = box->z;
> +   tmpl.u.tex.last_layer = box->z + box->depth - 1;
> +   tmpl.u.tex.level = level;
> +   sf = pipe->create_surface(pipe, res, );
> +   if (!sf)
> +   return;
> +
> +   if (util_format_is_depth_or_stencil(res->format)) {
> +   float depth = 0;
> +   uint8_t stencil = 0;
> +   unsigned clear = 0;
> +   const struct util_format_description *desc =
> +   util_format_description(res->format);
> +
> +   if (util_format_has_depth(desc)) {
> +   clear |= PIPE_CLEAR_DEPTH;
> +   desc->unpack_z_float(, 0, data, 0, 1, 1);
> +   }
> +   if (util_format_has_stencil(desc)) {
> +   clear |= PIPE_CLEAR_STENCIL;
> +   desc->unpack_s_8uint(, 0, data, 0, 1, 1);
> +   }
> +   pipe->clear_depth_stencil(pipe, sf, clear, depth, stencil,
> + box->x, box->y, box->width, 
> box->height);
> +   } else {
> +   union pipe_color_union color;
> +   switch (util_format_get_blocksizebits(res->format)) {
> +   case 128:
> +   sf->format = PIPE_FORMAT_R32G32B32A32_UINT;

Just as an FYI... this is safe on nouveau because I control all the
internals and know that this is safe to do. Please verify that it's
similarly safe to change the surface format after creation on radeon -
it might not be. (Esp for compressed textures...)

> +   memcpy(, data, 128 / 8);
> +   break;
> +   case 64:
> +   sf->format = PIPE_FORMAT_R32G32_UINT;
> +   memcpy(, data, 64 / 8);
> +   memset([2], 0, 64 / 8);
> +   break;
> +   case 32:
> +   sf->format = PIPE_FORMAT_R32_UINT;
> +   memcpy(, data, 32 / 8);
> +   memset([1], 0, 96 / 8);
> +   break;
> +   case 16:
> +   sf->format = PIPE_FORMAT_R16_UINT;
> +   color.ui[0] = util_cpu_to_le32(
> +   util_le16_to_cpu(*(unsigned short *)data));
> +   memset([1], 0, 96 / 8);
> +   break;
> +   case 8:
> +   sf->format = PIPE_FORMAT_R8_UINT;
> +   color.ui[0] = util_cpu_to_le32(*(unsigned char 
> *)data);
> +   memset([1], 0, 96 / 8);
> +   break;
> +   default:
> +   assert(!"Unknown texel element size");
> +   return;
> +   }
> +   pipe->clear_render_target(pipe, sf, ,
> + box->x, box->y, box->width, 
> box->height);
> +   }
> +   pipe->surface_destroy(pipe, sf);
> +}
> +
>  unsigned r600_translate_colorswap(enum pipe_format format)
>  {
> const struct util_format_description *desc = 
> util_format_description(format);
> @@ -1658,4 +1729,5 @@ void r600_init_context_texture_functions(struct 
> r600_common_context *rctx)
>  {
> rctx->b.create_surface = r600_create_surface;
> rctx->b.surface_destroy = r600_surface_destroy;
> +rctx->b.clear_texture = r600_clear_texture;
>  }
> --
> 2.8.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