[Mesa-dev] [PATCH v3 05/13] intel/genxml: Add Clear Color struct.

[Rafael Antognolli]

On Mon, Feb 26, 2018 at 01:07:54PM -0800, Jordan Justen wrote:
> On 2018-02-21 13:45:14, Rafael Antognolli wrote:
> > The size of the clear color struct (expected by the hardware) is 8
> > dwords (isl_dev.ss.clear_value_state_size here). But we still need to
> > track the size of the clear color, used when memcopying it to/from the
> > state buffer. For that we keep isl_dev.ss.clear_value_size.
> > 
> > Signed-off-by: Rafael Antognolli <rafael.antognolli at intel.com>
> > ---
> >  src/intel/genxml/gen10.xml     | 8 ++++++++
> >  src/intel/isl/isl.c            | 4 +++-
> >  src/intel/isl/isl.h            | 5 +++++
> >  src/intel/vulkan/anv_image.c   | 2 +-
> >  src/intel/vulkan/anv_private.h | 2 +-
> >  5 files changed, 18 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/intel/genxml/gen10.xml b/src/intel/genxml/gen10.xml
> > index b434d1b0f66..58b83954c4c 100644
> > --- a/src/intel/genxml/gen10.xml
> > +++ b/src/intel/genxml/gen10.xml
> > @@ -809,6 +809,14 @@
> >      <field name="Alpha Clear Color" start="480" end="511" type="int"/>
> >    </struct>
> >  
> > +  <struct name="CLEAR_COLOR" length="8">
> > +    <field name="Raw Clear Color Red" start="0" end="31" type="int"/>
> > +    <field name="Raw Clear Color Green" start="32" end="63" type="int"/>
> > +    <field name="Raw Clear Color Blue" start="64" end="95" type="int"/>
> > +    <field name="Raw Clear Color Alpha" start="96" end="127" type="int"/>
> > +    <!-- Reserved - MBZ -->
> 
> I'm not sure about this comment. Maybe:
> 
>     <!-- DW4-7 required by hardware, but not used by software -->

OK, will update that.

> I was going to ask about gen11.xml, but since isl.c is not compiled
> per gen, I'm not quite sure how things work with structs in the
> genxml.

Hmmm... good point, I just forgot gen11. Yeah, it's not compiled per gen
but isl.c uses genxml/genX_bits.h, that is generated from all xml. I'll
update that.

> > +  </struct>
> > +
> >    <struct name="SAMPLER_INDIRECT_STATE_BORDER_COLOR" length="4">
> >      <field name="Border Color Red As S31" start="0" end="31" type="int"/>
> >      <field name="Border Color Red As U32" start="0" end="31" type="uint"/>
> > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> > index 77641a89f86..e94470362e2 100644
> > --- a/src/intel/isl/isl.c
> > +++ b/src/intel/isl/isl.c
> > @@ -79,9 +79,10 @@ isl_device_init(struct isl_device *dev,
> >         * - 2 dwords that can be used by the hardware for converted clear color
> >         *   + some extra bits.
> >         */
> > -      dev->ss.clear_value_size = 8 * 4;
> > +      dev->ss.clear_value_size = 4 * 4;
> >        dev->ss.clear_value_offset =
> >           RENDER_SURFACE_STATE_ClearValueAddress_start(info) / 32 * 4;
> > +      dev->ss.clear_value_state_size = CLEAR_COLOR_length(info) * 4;
> >     } else {
> >        dev->ss.clear_value_size =
> >           isl_align(RENDER_SURFACE_STATE_RedClearColor_bits(info) +
> > @@ -90,6 +91,7 @@ isl_device_init(struct isl_device *dev,
> >                     RENDER_SURFACE_STATE_AlphaClearColor_bits(info), 32) / 8;
> >        dev->ss.clear_value_offset =
> >           RENDER_SURFACE_STATE_RedClearColor_start(info) / 32 * 4;
> > +      dev->ss.clear_value_state_size = dev->ss.clear_value_size;
> >     }
> >     assert(RENDER_SURFACE_STATE_SurfaceBaseAddress_start(info) % 8 == 0);
> >     dev->ss.addr_offset =
> > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> > index 209769a9a99..f1b38efed44 100644
> > --- a/src/intel/isl/isl.h
> > +++ b/src/intel/isl/isl.h
> > @@ -963,6 +963,11 @@ struct isl_device {
> >        uint8_t aux_addr_offset;
> >  
> >        /* Rounded up to the nearest dword to simplify GPU memcpy operations. */
> > +
> > +      /* size of the state buffer used to store the clear value + extra
> > +       * additional space used by the hardware */
> > +      uint8_t clear_value_state_size;
> > +      /* size of the clear color itself - used to copy it to/from a BO */
> >        uint8_t clear_value_size;
> >        uint8_t clear_value_offset;
> >     } ss;
> > diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> > index a0aee43bd21..0dafe03442d 100644
> > --- a/src/intel/vulkan/anv_image.c
> > +++ b/src/intel/vulkan/anv_image.c
> > @@ -264,7 +264,7 @@ add_aux_state_tracking_buffer(struct anv_image *image,
> >     }
> >  
> >     /* Clear color and fast clear type */
> > -   unsigned state_size = device->isl_dev.ss.clear_value_size + 4;
> > +   unsigned state_size = device->isl_dev.ss.clear_value_state_size + 4;
> >  
> >     /* We only need to track compression on CCS_E surfaces. */
> >     if (image->planes[plane].aux_usage == ISL_AUX_USAGE_CCS_E) {
> > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> > index 37e63f56aa0..b8c381d2665 100644
> > --- a/src/intel/vulkan/anv_private.h
> > +++ b/src/intel/vulkan/anv_private.h
> > @@ -2566,7 +2566,7 @@ anv_image_get_fast_clear_type_addr(const struct anv_device *device,
> >  {
> >     struct anv_address addr =
> >        anv_image_get_clear_color_addr(device, image, aspect);
> > -   addr.offset += device->isl_dev.ss.clear_value_size;
> > +   addr.offset += device->isl_dev.ss.clear_value_state_size;
> >     return addr;
> >  }
> >  
> > -- 
> > 2.14.3
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev