diff mbox series

[v7,7/9] drm/i915/gt: Ensure memory quiesced before invalidation for all engines

Message ID 20230720210737.761400-8-andi.shyti@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series Update AUX invalidation sequence | expand

Commit Message

Andi Shyti July 20, 2023, 9:07 p.m. UTC 
Commit af9e423a8aae ("drm/i915/gt: Ensure memory quiesced before
invalidation") has made sure that the memory is quiesced before
invalidating the AUX CCS table. Do it for all the other engines
and not just RCS.

Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: <stable@vger.kernel.org> # v5.8+
---
 drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 36 ++++++++++++++++--------
 1 file changed, 25 insertions(+), 11 deletions(-)

Comments

Krzysztofik, Janusz July 21, 2023, 12:10 p.m. UTC | #1 
Hi Andi,

On Thursday, 20 July 2023 23:07:35 CEST Andi Shyti wrote:
> Commit af9e423a8aae 

You can't use this commit ID, it is invalid (the patch you are referring to 
belongs to your series, then is not available in any official repository, 
hence no stable commit ID yet).

> ("drm/i915/gt: Ensure memory quiesced before
> invalidation") has made sure that the memory is quiesced before
> invalidating the AUX CCS table. Do it for all the other engines
> and not just RCS.

Why do we need that now for other engines, while that former patch seemed to 
suggest we didn't?

Thanks,
Janusz

> 
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: <stable@vger.kernel.org> # v5.8+
> ---
>  drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 36 ++++++++++++++++--------
>  1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> index 202d6ff8b5264..b6dd22eb2d9b2 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> @@ -316,26 +316,40 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
>  int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode)
>  {
>  	intel_engine_mask_t aux_inv = 0;
> -	u32 cmd, *cs;
> +	u32 cmd_flush = 0;
> +	u32 cmd = 4;
> +	u32 *cs;
>  
> -	cmd = 4;
> -	if (mode & EMIT_INVALIDATE) {
> +	if (mode & EMIT_INVALIDATE)
>  		cmd += 2;
>  
> -		if (HAS_AUX_CCS(rq->engine->i915) &&
> -		    (rq->engine->class == VIDEO_DECODE_CLASS ||
> -		     rq->engine->class == VIDEO_ENHANCEMENT_CLASS)) {
> -			aux_inv = rq->engine->mask &
> -				~GENMASK(_BCS(I915_MAX_BCS - 1), BCS0);
> -			if (aux_inv)
> -				cmd += 4;
> -		}
> +	if (HAS_AUX_CCS(rq->engine->i915))
> +		aux_inv = rq->engine->mask &
> +			  ~GENMASK(_BCS(I915_MAX_BCS - 1), BCS0);
> +
> +	/*
> +	 * On Aux CCS platforms the invalidation of the Aux
> +	 * table requires quiescing memory traffic beforehand
> +	 */
> +	if (aux_inv) {
> +		cmd += 4; /* for the AUX invalidation */
> +		cmd += 2; /* for the engine quiescing */
> +
> +		cmd_flush = MI_FLUSH_DW;
> +
> +		if (rq->engine->class == COPY_ENGINE_CLASS)
> +			cmd_flush |= MI_FLUSH_DW_CCS;
>  	}
>  
>  	cs = intel_ring_begin(rq, cmd);
>  	if (IS_ERR(cs))
>  		return PTR_ERR(cs);
>  
> +	if (cmd_flush) {
> +		*cs++ = cmd_flush;
> +		*cs++ = 0;
> +	}
> +
>  	if (mode & EMIT_INVALIDATE)
>  		*cs++ = preparser_disable(true);
>  
> 

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach handlowych.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.
Andi Shyti July 21, 2023, 12:45 p.m. UTC | #2 
Hi Janusz,

On Fri, Jul 21, 2023 at 12:10:22PM +0000, Krzysztofik, Janusz wrote:
> Hi Andi,
> 
> On Thursday, 20 July 2023 23:07:35 CEST Andi Shyti wrote:
> > Commit af9e423a8aae 
> 
> You can't use this commit ID, it is invalid (the patch you are referring to 
> belongs to your series, then is not available in any official repository, 
> hence no stable commit ID yet).

yes, I need to update it, I know... this has changed at every
revision, but I was lazy and decided to do it at the end after
the whole review process was done. I didn't think that anyone
would go and check it :-D

It's good to know that there is a thorough review here! Thanks!

> > ("drm/i915/gt: Ensure memory quiesced before
> > invalidation") has made sure that the memory is quiesced before
> > invalidating the AUX CCS table. Do it for all the other engines
> > and not just RCS.
> 
> Why do we need that now for other engines, while that former patch seemed to 
> suggest we didn't?

This whole work started from a a couple of patches from Jonathan
and slowly exploded in this series of 9 patches. I tried to
preserve his work and just add things around them.

What Jonathan originally did was to add aux invalidation support
only for RCS and Compute engines, but then hardware guys updated
the docs asking to do it for basically all the engines.

Thank you,
Andi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index 202d6ff8b5264..b6dd22eb2d9b2 100644
--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
@@ -316,26 +316,40 @@  int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
 int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode)
 {
 	intel_engine_mask_t aux_inv = 0;
-	u32 cmd, *cs;
+	u32 cmd_flush = 0;
+	u32 cmd = 4;
+	u32 *cs;
 
-	cmd = 4;
-	if (mode & EMIT_INVALIDATE) {
+	if (mode & EMIT_INVALIDATE)
 		cmd += 2;
 
-		if (HAS_AUX_CCS(rq->engine->i915) &&
-		    (rq->engine->class == VIDEO_DECODE_CLASS ||
-		     rq->engine->class == VIDEO_ENHANCEMENT_CLASS)) {
-			aux_inv = rq->engine->mask &
-				~GENMASK(_BCS(I915_MAX_BCS - 1), BCS0);
-			if (aux_inv)
-				cmd += 4;
-		}
+	if (HAS_AUX_CCS(rq->engine->i915))
+		aux_inv = rq->engine->mask &
+			  ~GENMASK(_BCS(I915_MAX_BCS - 1), BCS0);
+
+	/*
+	 * On Aux CCS platforms the invalidation of the Aux
+	 * table requires quiescing memory traffic beforehand
+	 */
+	if (aux_inv) {
+		cmd += 4; /* for the AUX invalidation */
+		cmd += 2; /* for the engine quiescing */
+
+		cmd_flush = MI_FLUSH_DW;
+
+		if (rq->engine->class == COPY_ENGINE_CLASS)
+			cmd_flush |= MI_FLUSH_DW_CCS;
 	}
 
 	cs = intel_ring_begin(rq, cmd);
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
+	if (cmd_flush) {
+		*cs++ = cmd_flush;
+		*cs++ = 0;
+	}
+
 	if (mode & EMIT_INVALIDATE)
 		*cs++ = preparser_disable(true);