On Tue, Nov 14, 2017 at 02:30:31PM +0000, Lionel Landwerlin wrote:
> On 13/11/17 22:07, Jason Ekstrand wrote:
> > On Mon, Nov 13, 2017 at 1:30 PM, Nanley Chery <nanleych...@gmail.com
> > <mailto:nanleych...@gmail.com>> wrote:
> >
> > On Mon, Nov 13, 2017 at 08:12:41AM -0800, Jason Ekstrand wrote:
> > > Found by inspection
> > >
> >
> > Good catch.
> >
> > > Cc: mesa-sta...@lists.freedesktop.org
> > <mailto:mesa-sta...@lists.freedesktop.org>
> > > ---
> > > src/intel/vulkan/genX_cmd_buffer.c | 9 ++++++---
> > > 1 file changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > b/src/intel/vulkan/genX_cmd_buffer.c
> > > index fbb5706..2564976 100644
> > > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > > @@ -557,12 +557,13 @@ init_fast_clear_state_entry(struct
> > anv_cmd_buffer *cmd_buffer,
> > > /* Other combinations of auxiliary buffers and platforms
> > require specific
> > > * values in the clear value dword(s).
> > > */
> > > + struct anv_address addr =
> > > + get_fast_clear_state_address(cmd_buffer->device, image,
> > aspect, level,
> > > + FAST_CLEAR_STATE_FIELD_CLEAR_COLOR);
> > > unsigned i = 0;
> > > for (; i < cmd_buffer->device->isl_dev.ss.clear_value_size;
> > i += 4) {
> > > anv_batch_emit(&cmd_buffer->batch,
> > GENX(MI_STORE_DATA_IMM), sdi) {
> > > - sdi.Address =
> > > - get_fast_clear_state_address(cmd_buffer->device,
> > image, aspect, level,
> > > - FAST_CLEAR_STATE_FIELD_CLEAR_COLOR);
> > > + sdi.Address = addr;
> >
> > The loop increments the variable i by 4 with every iteration. How
> > about
> > the following instead:
> > sdi.Address = addr + i;
> >
> >
> > I really wish we could do that but it's a struct. I could do
> >
Whoops.
> > sdi.Address = addr;
> > sdi.Address.offset += i;
> >
That looks good to me. No worries if you choose to go with the fix you
mentioned below. With one of these fixes, this patch is
Reviewed-by: Nanley Chery <nanley.g.ch...@intel.com>
> > --Jason
> >
> > -Nanley
> >
> > >
> > > if (GEN_GEN >= 9) {
> > > /* MCS buffers on SKL+ can only have 1/0 clear
> > colors. */
> > > @@ -586,6 +587,8 @@ init_fast_clear_state_entry(struct
> > anv_cmd_buffer *cmd_buffer,
> > > sdi.ImmediateData = 0;
> > > }
> > > }
> > > +
> > > + addr += 4;
> >
> >
> > Aparently, I didn't compile-test this because I need a .offset here. :/
>
> Heh, I was confused too :)
> With that fixed :
>
> Reviewed-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>
>
> > > }
> > > }
> > >
> > > --
> > > 2.5.0.400.gff86faf
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > <mailto:mesa-dev@lists.freedesktop.org>
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > <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
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
