| Message ID | 20230926142401.25687-1-nirmoy.das@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Don't set PIPE_CONTROL_FLUSH_L3 for aux inval | expand |
Hi Nirmoy, On Tue, Sep 26, 2023 at 04:24:01PM +0200, Nirmoy Das wrote: > PIPE_CONTROL_FLUSH_L3 is not needed for aux invalidation > so don't set that. > > Fixes: 78a6ccd65fa3 ("drm/i915/gt: Ensure memory quiesced before invalidation") > Cc: Jonathan Cavitt <jonathan.cavitt@intel.com> > Cc: Andi Shyti <andi.shyti@linux.intel.com> > Cc: <stable@vger.kernel.org> # v5.8+ > Cc: Andrzej Hajda <andrzej.hajda@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> > Cc: Tapani Pälli <tapani.palli@intel.com> > Cc: Mark Janes <mark.janes@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> looks better :) Tapani, you mind giving this a test? Andi
On Tue, Sep 26, 2023 at 04:24:01PM +0200, Nirmoy Das wrote: > PIPE_CONTROL_FLUSH_L3 is not needed for aux invalidation > so don't set that. > > Fixes: 78a6ccd65fa3 ("drm/i915/gt: Ensure memory quiesced before invalidation") > Cc: Jonathan Cavitt <jonathan.cavitt@intel.com> > Cc: Andi Shyti <andi.shyti@linux.intel.com> > Cc: <stable@vger.kernel.org> # v5.8+ > Cc: Andrzej Hajda <andrzej.hajda@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> > Cc: Tapani Pälli <tapani.palli@intel.com> > Cc: Mark Janes <mark.janes@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> Acked-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > index 0143445dba83..ba4c2422b340 100644 > --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > @@ -271,8 +271,17 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) > if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70)) > bit_group_0 |= PIPE_CONTROL_CCS_FLUSH; > > + /* > + * L3 fabric flush is needed for AUX CCS invalidation > + * which happens as part of pipe-control so we can > + * ignore PIPE_CONTROL_FLUSH_L3. Also PIPE_CONTROL_FLUSH_L3 > + * deals with Protected Memory which is not needed for > + * AUX CCS invalidation and lead to unwanted side effects. > + */ > + if (mode & EMIT_FLUSH) > + bit_group_1 |= PIPE_CONTROL_FLUSH_L3; > + > bit_group_1 |= PIPE_CONTROL_TILE_CACHE_FLUSH; > - bit_group_1 |= PIPE_CONTROL_FLUSH_L3; > bit_group_1 |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; > bit_group_1 |= PIPE_CONTROL_DEPTH_CACHE_FLUSH; > /* Wa_1409600907:tgl,adl-p */ > -- > 2.41.0 >
Hi Nirmoy, ... > > PIPE_CONTROL_FLUSH_L3 is not needed for aux invalidation > > so don't set that. > > > > Fixes: 78a6ccd65fa3 ("drm/i915/gt: Ensure memory quiesced before invalidation") > > Cc: Jonathan Cavitt <jonathan.cavitt@intel.com> > > Cc: Andi Shyti <andi.shyti@linux.intel.com> > > Cc: <stable@vger.kernel.org> # v5.8+ > > Cc: Andrzej Hajda <andrzej.hajda@intel.com> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Cc: Matt Roper <matthew.d.roper@intel.com> > > Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > > Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> > > Cc: Tapani Pälli <tapani.palli@intel.com> > > Cc: Mark Janes <mark.janes@intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> > > looks better :) this was supposed to be: Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Andi
Fixes all regressions we saw, I also run some extra vulkan and GL workloads, no regressions observed. Tested-by: Tapani Pälli <tapani.palli@intel.com> On 26.9.2023 17.24, Nirmoy Das wrote: > PIPE_CONTROL_FLUSH_L3 is not needed for aux invalidation > so don't set that. > > Fixes: 78a6ccd65fa3 ("drm/i915/gt: Ensure memory quiesced before invalidation") > Cc: Jonathan Cavitt <jonathan.cavitt@intel.com> > Cc: Andi Shyti <andi.shyti@linux.intel.com> > Cc: <stable@vger.kernel.org> # v5.8+ > Cc: Andrzej Hajda <andrzej.hajda@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> > Cc: Tapani Pälli <tapani.palli@intel.com> > Cc: Mark Janes <mark.janes@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> > --- > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > index 0143445dba83..ba4c2422b340 100644 > --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > @@ -271,8 +271,17 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) > if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70)) > bit_group_0 |= PIPE_CONTROL_CCS_FLUSH; > > + /* > + * L3 fabric flush is needed for AUX CCS invalidation > + * which happens as part of pipe-control so we can > + * ignore PIPE_CONTROL_FLUSH_L3. Also PIPE_CONTROL_FLUSH_L3 > + * deals with Protected Memory which is not needed for > + * AUX CCS invalidation and lead to unwanted side effects. > + */ > + if (mode & EMIT_FLUSH) > + bit_group_1 |= PIPE_CONTROL_FLUSH_L3; > + > bit_group_1 |= PIPE_CONTROL_TILE_CACHE_FLUSH; > - bit_group_1 |= PIPE_CONTROL_FLUSH_L3; > bit_group_1 |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; > bit_group_1 |= PIPE_CONTROL_DEPTH_CACHE_FLUSH; > /* Wa_1409600907:tgl,adl-p */
On 26.09.2023 16:24, Nirmoy Das wrote: > PIPE_CONTROL_FLUSH_L3 is not needed for aux invalidation > so don't set that. > > Fixes: 78a6ccd65fa3 ("drm/i915/gt: Ensure memory quiesced before invalidation") > Cc: Jonathan Cavitt <jonathan.cavitt@intel.com> > Cc: Andi Shyti <andi.shyti@linux.intel.com> > Cc: <stable@vger.kernel.org> # v5.8+ > Cc: Andrzej Hajda <andrzej.hajda@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> > Cc: Tapani Pälli <tapani.palli@intel.com> > Cc: Mark Janes <mark.janes@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Regards Andrzej > --- > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > index 0143445dba83..ba4c2422b340 100644 > --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > @@ -271,8 +271,17 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) > if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70)) > bit_group_0 |= PIPE_CONTROL_CCS_FLUSH; > > + /* > + * L3 fabric flush is needed for AUX CCS invalidation > + * which happens as part of pipe-control so we can > + * ignore PIPE_CONTROL_FLUSH_L3. Also PIPE_CONTROL_FLUSH_L3 > + * deals with Protected Memory which is not needed for > + * AUX CCS invalidation and lead to unwanted side effects. > + */ > + if (mode & EMIT_FLUSH) > + bit_group_1 |= PIPE_CONTROL_FLUSH_L3; > + > bit_group_1 |= PIPE_CONTROL_TILE_CACHE_FLUSH; > - bit_group_1 |= PIPE_CONTROL_FLUSH_L3; > bit_group_1 |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; > bit_group_1 |= PIPE_CONTROL_DEPTH_CACHE_FLUSH; > /* Wa_1409600907:tgl,adl-p */
Hi Tapani, On 9/27/2023 6:13 AM, Tapani Pälli wrote: > Fixes all regressions we saw, I also run some extra vulkan and GL > workloads, no regressions observed. > > Tested-by: Tapani Pälli <tapani.palli@intel.com> Thanks to testing it. The patch is now merged with" <stable@vger.kernel.org> # v5.8+" tag so it should trickle down to v6.4.10. as normal stable release process. Thanks, Nirmoy > > On 26.9.2023 17.24, Nirmoy Das wrote: >> PIPE_CONTROL_FLUSH_L3 is not needed for aux invalidation >> so don't set that. >> >> Fixes: 78a6ccd65fa3 ("drm/i915/gt: Ensure memory quiesced before >> invalidation") >> Cc: Jonathan Cavitt <jonathan.cavitt@intel.com> >> Cc: Andi Shyti <andi.shyti@linux.intel.com> >> Cc: <stable@vger.kernel.org> # v5.8+ >> Cc: Andrzej Hajda <andrzej.hajda@intel.com> >> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Matt Roper <matthew.d.roper@intel.com> >> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> >> Cc: Lucas De Marchi <lucas.demarchi@intel.com> >> Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> >> Cc: Tapani Pälli <tapani.palli@intel.com> >> Cc: Mark Janes <mark.janes@intel.com> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> >> --- >> drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c >> b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c >> index 0143445dba83..ba4c2422b340 100644 >> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c >> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c >> @@ -271,8 +271,17 @@ int gen12_emit_flush_rcs(struct i915_request >> *rq, u32 mode) >> if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70)) >> bit_group_0 |= PIPE_CONTROL_CCS_FLUSH; >> + /* >> + * L3 fabric flush is needed for AUX CCS invalidation >> + * which happens as part of pipe-control so we can >> + * ignore PIPE_CONTROL_FLUSH_L3. Also PIPE_CONTROL_FLUSH_L3 >> + * deals with Protected Memory which is not needed for >> + * AUX CCS invalidation and lead to unwanted side effects. >> + */ >> + if (mode & EMIT_FLUSH) >> + bit_group_1 |= PIPE_CONTROL_FLUSH_L3; >> + >> bit_group_1 |= PIPE_CONTROL_TILE_CACHE_FLUSH; >> - bit_group_1 |= PIPE_CONTROL_FLUSH_L3; >> bit_group_1 |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; >> bit_group_1 |= PIPE_CONTROL_DEPTH_CACHE_FLUSH; >> /* Wa_1409600907:tgl,adl-p */
diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index 0143445dba83..ba4c2422b340 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -271,8 +271,17 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70)) bit_group_0 |= PIPE_CONTROL_CCS_FLUSH; + /* + * L3 fabric flush is needed for AUX CCS invalidation + * which happens as part of pipe-control so we can + * ignore PIPE_CONTROL_FLUSH_L3. Also PIPE_CONTROL_FLUSH_L3 + * deals with Protected Memory which is not needed for + * AUX CCS invalidation and lead to unwanted side effects. + */ + if (mode & EMIT_FLUSH) + bit_group_1 |= PIPE_CONTROL_FLUSH_L3; + bit_group_1 |= PIPE_CONTROL_TILE_CACHE_FLUSH; - bit_group_1 |= PIPE_CONTROL_FLUSH_L3; bit_group_1 |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; bit_group_1 |= PIPE_CONTROL_DEPTH_CACHE_FLUSH; /* Wa_1409600907:tgl,adl-p */
PIPE_CONTROL_FLUSH_L3 is not needed for aux invalidation so don't set that. Fixes: 78a6ccd65fa3 ("drm/i915/gt: Ensure memory quiesced before invalidation") Cc: Jonathan Cavitt <jonathan.cavitt@intel.com> Cc: Andi Shyti <andi.shyti@linux.intel.com> Cc: <stable@vger.kernel.org> # v5.8+ Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> Cc: Tapani Pälli <tapani.palli@intel.com> Cc: Mark Janes <mark.janes@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> --- drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)