On Tue, Feb 27, 2018 at 11:46:12AM -0800, Jason Ekstrand wrote:
> On Tue, Feb 27, 2018 at 9:35 AM, Rafael Antognolli
> <rafael.antogno...@intel.com
> > wrote:
>
> On Mon, Feb 26, 2018 at 05:04:37PM -0800, Jason Ekstrand wrote:
> > On Wed, Feb 21, 2018 at 1:45 PM, Rafael Antognolli <
> rafael.antogno...@intel.com
> > > 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.antogno...@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
> >
> >
> > We need gen11 as well
> >
> >
> > @@ -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"/>
> >
> >
> > Might be good to put Converted Clear Value Hi/Low in here as well.
> >
> >
> > + <!-- Reserved - MBZ -->
> > + </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;
> >
> >
> > Ugh... Let's just make these two separate things. clear_value_size will
> be 4 *
> > 4 on gen9-10 and clear_color_state_size will be 8*4 on gen10+
>
> I'm not sure I understand/agree with you here. clear_value_size should
> be 4 * 4 everywhere, since we use this to memcpy the 4 dwords of the
> clear color. So, are you suggesting we remove clear_value_state_size
> from gen9?
>
>
> I mean that we have two separate things here: clear_value_size which is 4 B on
> gen7-8, 16 B on gen9-10, and doesn't exist on gen11+ and
> clear_color_state_size
> which doesn't exist prior to gen10 and is 32 B on gen10+. Does that make more
> sense?
Hmm... I was planning to use clear_value_size on gen11+ as well. If I'm
not wrong, there are some loops where we cycle through the dwords in the
clear color state buffer using clear_value_size as a limit, but we can
simply drop it and use 4 (dwords). But yeah, I can drop it...
I agree with clear_color_state_size, though.
> > I think this means dropping the previous patch entirely and just making
> this
> > patch add a size field.
>
> Ack.
>
> > }
> > assert(RENDER_SURFACE_STATE_SurfaceBaseAddress_start(info) % 8
> ==
> 0);
> > dev->ss.addr_offset =
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
