Message ID | 20210119094053.6919-1-chris@chris-wilson.co.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/5] drm/i915/gt: One more flush for Baytrail clear residuals | expand |
> -----Original Message----- > From: Chris Wilson <chris@chris-wilson.co.uk> > Sent: Tuesday, January 19, 2021 1:41 AM > To: intel-gfx@lists.freedesktop.org > Cc: mika.kuoppala@linux.intel.com; Chris Wilson <chris@chris-wilson.co.uk>; > Abodunrin, Akeem G <akeem.g.abodunrin@intel.com> > Subject: [PATCH 1/5] drm/i915/gt: One more flush for Baytrail clear residuals > > CI reports that Baytail requires one more invalidate after CACHE_MODE for it > to be happy. > > Fixes: ace44e13e577 ("drm/i915/gt: Clear CACHE_MODE prior to clearing > residuals") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Akeem G Abodunrin <akeem.g.abodunrin@intel.com> > --- > drivers/gpu/drm/i915/gt/gen7_renderclear.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c > b/drivers/gpu/drm/i915/gt/gen7_renderclear.c > index 39478712769f..8551e6de50e8 100644 > --- a/drivers/gpu/drm/i915/gt/gen7_renderclear.c > +++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c > @@ -353,19 +353,21 @@ static void gen7_emit_pipeline_flush(struct > batch_chunk *batch) > > static void gen7_emit_pipeline_invalidate(struct batch_chunk *batch) { > - u32 *cs = batch_alloc_items(batch, 0, 8); > + u32 *cs = batch_alloc_items(batch, 0, 10); > > /* ivb: Stall before STATE_CACHE_INVALIDATE */ > - *cs++ = GFX_OP_PIPE_CONTROL(4); > + *cs++ = GFX_OP_PIPE_CONTROL(5); > *cs++ = PIPE_CONTROL_STALL_AT_SCOREBOARD | > PIPE_CONTROL_CS_STALL; > *cs++ = 0; > *cs++ = 0; > + *cs++ = 0; > > - *cs++ = GFX_OP_PIPE_CONTROL(4); > + *cs++ = GFX_OP_PIPE_CONTROL(5); > *cs++ = PIPE_CONTROL_STATE_CACHE_INVALIDATE; > *cs++ = 0; > *cs++ = 0; > + *cs++ = 0; > > batch_advance(batch, cs); > } > @@ -397,6 +399,7 @@ static void emit_batch(struct i915_vma * const vma, > batch_add(&cmds, 0xffff0000); > batch_add(&cmds, i915_mmio_reg_offset(CACHE_MODE_1)); > batch_add(&cmds, 0xffff0000 | > PIXEL_SUBSPAN_COLLECT_OPT_DISABLE); > + gen7_emit_pipeline_invalidate(&cmds); > gen7_emit_pipeline_flush(&cmds); > > /* Switch to the media pipeline and our base address */ > -- > 2.20.1 Reviewed-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com> Thank you! ~Akeem
Chris Wilson <chris@chris-wilson.co.uk> writes: > CI reports that Baytail requires one more invalidate after CACHE_MODE > for it to be happy. > > Fixes: ace44e13e577 ("drm/i915/gt: Clear CACHE_MODE prior to clearing residuals") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Akeem G Abodunrin <akeem.g.abodunrin@intel.com> > --- > drivers/gpu/drm/i915/gt/gen7_renderclear.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c b/drivers/gpu/drm/i915/gt/gen7_renderclear.c > index 39478712769f..8551e6de50e8 100644 > --- a/drivers/gpu/drm/i915/gt/gen7_renderclear.c > +++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c > @@ -353,19 +353,21 @@ static void gen7_emit_pipeline_flush(struct batch_chunk *batch) > > static void gen7_emit_pipeline_invalidate(struct batch_chunk *batch) > { > - u32 *cs = batch_alloc_items(batch, 0, 8); > + u32 *cs = batch_alloc_items(batch, 0, 10); > > /* ivb: Stall before STATE_CACHE_INVALIDATE */ > - *cs++ = GFX_OP_PIPE_CONTROL(4); > + *cs++ = GFX_OP_PIPE_CONTROL(5); > *cs++ = PIPE_CONTROL_STALL_AT_SCOREBOARD | > PIPE_CONTROL_CS_STALL; > *cs++ = 0; > *cs++ = 0; > + *cs++ = 0; dw[5] seems to be only for gen8+. Does it make a difference? -Mika > > - *cs++ = GFX_OP_PIPE_CONTROL(4); > + *cs++ = GFX_OP_PIPE_CONTROL(5); > *cs++ = PIPE_CONTROL_STATE_CACHE_INVALIDATE; > *cs++ = 0; > *cs++ = 0; > + *cs++ = 0; > > batch_advance(batch, cs); > } > @@ -397,6 +399,7 @@ static void emit_batch(struct i915_vma * const vma, > batch_add(&cmds, 0xffff0000); > batch_add(&cmds, i915_mmio_reg_offset(CACHE_MODE_1)); > batch_add(&cmds, 0xffff0000 | PIXEL_SUBSPAN_COLLECT_OPT_DISABLE); > + gen7_emit_pipeline_invalidate(&cmds); > gen7_emit_pipeline_flush(&cmds); > > /* Switch to the media pipeline and our base address */ > -- > 2.20.1
Quoting Mika Kuoppala (2021-01-19 10:25:14) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > CI reports that Baytail requires one more invalidate after CACHE_MODE > > for it to be happy. > > > > Fixes: ace44e13e577 ("drm/i915/gt: Clear CACHE_MODE prior to clearing residuals") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Cc: Akeem G Abodunrin <akeem.g.abodunrin@intel.com> > > --- > > drivers/gpu/drm/i915/gt/gen7_renderclear.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c b/drivers/gpu/drm/i915/gt/gen7_renderclear.c > > index 39478712769f..8551e6de50e8 100644 > > --- a/drivers/gpu/drm/i915/gt/gen7_renderclear.c > > +++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c > > @@ -353,19 +353,21 @@ static void gen7_emit_pipeline_flush(struct batch_chunk *batch) > > > > static void gen7_emit_pipeline_invalidate(struct batch_chunk *batch) > > { > > - u32 *cs = batch_alloc_items(batch, 0, 8); > > + u32 *cs = batch_alloc_items(batch, 0, 10); > > > > /* ivb: Stall before STATE_CACHE_INVALIDATE */ > > - *cs++ = GFX_OP_PIPE_CONTROL(4); > > + *cs++ = GFX_OP_PIPE_CONTROL(5); > > *cs++ = PIPE_CONTROL_STALL_AT_SCOREBOARD | > > PIPE_CONTROL_CS_STALL; > > *cs++ = 0; > > *cs++ = 0; > > + *cs++ = 0; > > dw[5] seems to be only for gen8+. Does it make a difference? Pipe-control has always supported a qword-write, so a packet-length of 4 or 5 on gen4-7. As I recall the debate for gen8+ was whether pipe-control still supported only a dword-write, and so we went with always using the qword-length. -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Mika Kuoppala (2021-01-19 10:25:14) >> Chris Wilson <chris@chris-wilson.co.uk> writes: >> >> > CI reports that Baytail requires one more invalidate after CACHE_MODE >> > for it to be happy. >> > >> > Fixes: ace44e13e577 ("drm/i915/gt: Clear CACHE_MODE prior to clearing residuals") >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> > Cc: Akeem G Abodunrin <akeem.g.abodunrin@intel.com> >> > --- >> > drivers/gpu/drm/i915/gt/gen7_renderclear.c | 9 ++++++--- >> > 1 file changed, 6 insertions(+), 3 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c b/drivers/gpu/drm/i915/gt/gen7_renderclear.c >> > index 39478712769f..8551e6de50e8 100644 >> > --- a/drivers/gpu/drm/i915/gt/gen7_renderclear.c >> > +++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c >> > @@ -353,19 +353,21 @@ static void gen7_emit_pipeline_flush(struct batch_chunk *batch) >> > >> > static void gen7_emit_pipeline_invalidate(struct batch_chunk *batch) >> > { >> > - u32 *cs = batch_alloc_items(batch, 0, 8); >> > + u32 *cs = batch_alloc_items(batch, 0, 10); >> > >> > /* ivb: Stall before STATE_CACHE_INVALIDATE */ >> > - *cs++ = GFX_OP_PIPE_CONTROL(4); >> > + *cs++ = GFX_OP_PIPE_CONTROL(5); >> > *cs++ = PIPE_CONTROL_STALL_AT_SCOREBOARD | >> > PIPE_CONTROL_CS_STALL; >> > *cs++ = 0; >> > *cs++ = 0; >> > + *cs++ = 0; >> >> dw[5] seems to be only for gen8+. Does it make a difference? > > Pipe-control has always supported a qword-write, so a packet-length of 4 > or 5 on gen4-7. As I recall the debate for gen8+ was whether > pipe-control still supported only a dword-write, and so we went with > always using the qword-length. Yes, there seem to ample evidence that 5 is fine. Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > -Chris
diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c b/drivers/gpu/drm/i915/gt/gen7_renderclear.c index 39478712769f..8551e6de50e8 100644 --- a/drivers/gpu/drm/i915/gt/gen7_renderclear.c +++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c @@ -353,19 +353,21 @@ static void gen7_emit_pipeline_flush(struct batch_chunk *batch) static void gen7_emit_pipeline_invalidate(struct batch_chunk *batch) { - u32 *cs = batch_alloc_items(batch, 0, 8); + u32 *cs = batch_alloc_items(batch, 0, 10); /* ivb: Stall before STATE_CACHE_INVALIDATE */ - *cs++ = GFX_OP_PIPE_CONTROL(4); + *cs++ = GFX_OP_PIPE_CONTROL(5); *cs++ = PIPE_CONTROL_STALL_AT_SCOREBOARD | PIPE_CONTROL_CS_STALL; *cs++ = 0; *cs++ = 0; + *cs++ = 0; - *cs++ = GFX_OP_PIPE_CONTROL(4); + *cs++ = GFX_OP_PIPE_CONTROL(5); *cs++ = PIPE_CONTROL_STATE_CACHE_INVALIDATE; *cs++ = 0; *cs++ = 0; + *cs++ = 0; batch_advance(batch, cs); } @@ -397,6 +399,7 @@ static void emit_batch(struct i915_vma * const vma, batch_add(&cmds, 0xffff0000); batch_add(&cmds, i915_mmio_reg_offset(CACHE_MODE_1)); batch_add(&cmds, 0xffff0000 | PIXEL_SUBSPAN_COLLECT_OPT_DISABLE); + gen7_emit_pipeline_invalidate(&cmds); gen7_emit_pipeline_flush(&cmds); /* Switch to the media pipeline and our base address */
CI reports that Baytail requires one more invalidate after CACHE_MODE for it to be happy. Fixes: ace44e13e577 ("drm/i915/gt: Clear CACHE_MODE prior to clearing residuals") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Akeem G Abodunrin <akeem.g.abodunrin@intel.com> --- drivers/gpu/drm/i915/gt/gen7_renderclear.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)