Hi Andy,

On Thursday, 20 July 2023 23:07:30 CEST Andi Shyti wrote:

We always assumed that a device might either have AUX or FLAT CCS, but this is an approximation that is not always true

If there exists a device that can have CCSs that fall into either none or both of those categories then I think we should have that device or two listed here as an example, regardless of deducible from the change or not. Or if there are no such devices so far, but we are going to introduce some, then I think we should provide that information here.

as it requires some further per device checks.

Add the "has_aux_ccs" flag in the intel_device_info structure in order to have a per device flag indicating of the AUX CCS.

Signed-off-by: Andi Shyti andi.shyti@linux.intel.com Cc: Matt Roper matthew.d.roper@intel.com Cc: Jonathan Cavitt jonathan.cavitt@intel.com Cc: stable@vger.kernel.org # v5.8+

---

drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 4 ++-- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_pci.c | 5 ++++- drivers/gpu/drm/i915/intel_device_info.h | 1 + 4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/

i915/gt/gen8_engine_cs.c

index 563efee055602..0d4d5e0407a2d 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -267,7 +267,7 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32

mode)

else if (engine->class == COMPUTE_CLASS)
	flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;

• if (!HAS_FLAT_CCS(rq->engine->i915))
  

• if (HAS_AUX_CCS(rq->engine->i915))
  count = 8 + 4;
  

  else count = 8;

@@ -307,7 +307,7 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32

mode)

if (mode & EMIT_INVALIDATE) { cmd += 2;

• if (!HAS_FLAT_CCS(rq->engine->i915) &&
  

• if (HAS_AUX_CCS(rq->engine->i915) &&
  

  (rq->engine->class == VIDEO_DECODE_CLASS || rq->engine->class == VIDEO_ENHANCEMENT_CLASS)) { aux_inv = rq->engine->mask &

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/

i915_drv.h

index 682ef2b5c7d59..e9cc048b5727a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -848,6 +848,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,

• stored in lmem to support the 3D and media compression formats.

*/ #define HAS_FLAT_CCS(i915) (INTEL_INFO(i915)->has_flat_ccs) +#define HAS_AUX_CCS(i915) (INTEL_INFO(i915)->has_aux_ccs) #define HAS_GT_UC(i915) (INTEL_INFO(i915)->has_gt_uc) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/

i915_pci.c

index fcacdc21643cf..c9ff1d11a9fce 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -643,7 +643,8 @@ static const struct intel_device_info jsl_info = { TGL_CACHELEVEL, \ .has_global_mocs = 1, \ .has_pxp = 1, \

• .max_pat_index = 3

• .max_pat_index = 3, \
• .has_aux_ccs = 1

NIT: Can we please have the last element also followed by comma, like in other places (e.g. see below)? That will simplify future patches.

Other than that, LGTM.

Thanks, Janusz

static const struct intel_device_info tgl_info = { GEN12_FEATURES, @@ -775,6 +776,7 @@ static const struct intel_device_info dg2_info = { static const struct intel_device_info ats_m_info = { DG2_FEATURES,

• .has_aux_ccs = 1, .require_force_probe = 1, .tuning_thread_rr_after_dep = 1,

}; @@ -827,6 +829,7 @@ static const struct intel_device_info mtl_info = { .__runtime.media.ip.ver = 13, PLATFORM(INTEL_METEORLAKE), .extra_gt_list = xelpmp_extra_gt,

• .has_aux_ccs = 1, .has_flat_ccs = 0, .has_gmd_id = 1, .has_guc_deprivilege = 1,

diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/

i915/intel_device_info.h

index dbfe6443457b5..93485507506cc 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -151,6 +151,7 @@ enum intel_ppgtt_type { func(has_reset_engine); \ func(has_3d_pipeline); \ func(has_4tile); \

• func(has_aux_ccs); \ func(has_flat_ccs); \ func(has_global_mocs); \ func(has_gmd_id); \

--------------------------------------------------------------------- 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.

On 20.07.2023 23:07, Andi Shyti wrote:

We always assumed that a device might either have AUX or FLAT CCS, but this is an approximation that is not always true as it requires some further per device checks.

Add the "has_aux_ccs" flag in the intel_device_info structure in order to have a per device flag indicating of the AUX CCS.

As Matt mentioned in v6, aux_ccs is present also in older platforms. This is about presence/necessity (?) of aux_ccs table invalidation. Maybe has_aux_ccs_inv, dunno?

Moreover you define flag per device, but this is rather per engine, theoretically could be also: MTL: .aux_ccs_inv_mask = BIT(RCS0) | BIT(BCS0) | ... Others: .aux_ccs_inv_mask = BIT(RCS0) | ...

looks overkill, maybe helper function would be simpler, up to you.

Regards Andrzej

Signed-off-by: Andi Shyti andi.shyti@linux.intel.com Cc: Matt Roper matthew.d.roper@intel.com Cc: Jonathan Cavitt jonathan.cavitt@intel.com Cc: stable@vger.kernel.org # v5.8+

---

drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 4 ++-- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_pci.c | 5 ++++- drivers/gpu/drm/i915/intel_device_info.h | 1 + 4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index 563efee055602..0d4d5e0407a2d 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -267,7 +267,7 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) else if (engine->class == COMPUTE_CLASS) flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;

• if (!HAS_FLAT_CCS(rq->engine->i915))
  

• if (HAS_AUX_CCS(rq->engine->i915))
  count = 8 + 4;
  

  else count = 8;

@@ -307,7 +307,7 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode) if (mode & EMIT_INVALIDATE) { cmd += 2;

• if (!HAS_FLAT_CCS(rq->engine->i915) &&
  

• if (HAS_AUX_CCS(rq->engine->i915) &&
  

  (rq->engine->class == VIDEO_DECODE_CLASS || rq->engine->class == VIDEO_ENHANCEMENT_CLASS)) { aux_inv = rq->engine->mask &

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 682ef2b5c7d59..e9cc048b5727a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -848,6 +848,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,

• stored in lmem to support the 3D and media compression formats.

*/ #define HAS_FLAT_CCS(i915) (INTEL_INFO(i915)->has_flat_ccs) +#define HAS_AUX_CCS(i915) (INTEL_INFO(i915)->has_aux_ccs) #define HAS_GT_UC(i915) (INTEL_INFO(i915)->has_gt_uc) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index fcacdc21643cf..c9ff1d11a9fce 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -643,7 +643,8 @@ static const struct intel_device_info jsl_info = { TGL_CACHELEVEL, \ .has_global_mocs = 1, \ .has_pxp = 1, \

• .max_pat_index = 3

• .max_pat_index = 3, \
• .has_aux_ccs = 1

static const struct intel_device_info tgl_info = { GEN12_FEATURES, @@ -775,6 +776,7 @@ static const struct intel_device_info dg2_info = { static const struct intel_device_info ats_m_info = { DG2_FEATURES,

• .has_aux_ccs = 1, .require_force_probe = 1, .tuning_thread_rr_after_dep = 1, };

@@ -827,6 +829,7 @@ static const struct intel_device_info mtl_info = { .__runtime.media.ip.ver = 13, PLATFORM(INTEL_METEORLAKE), .extra_gt_list = xelpmp_extra_gt,

• .has_aux_ccs = 1, .has_flat_ccs = 0, .has_gmd_id = 1, .has_guc_deprivilege = 1,

diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index dbfe6443457b5..93485507506cc 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -151,6 +151,7 @@ enum intel_ppgtt_type { func(has_reset_engine); \ func(has_3d_pipeline); \ func(has_4tile); \

• func(has_aux_ccs); \ func(has_flat_ccs); \ func(has_global_mocs); \ func(has_gmd_id); \

Hi Nirmoy,

On Fri, Jul 21, 2023 at 12:05:10PM +0200, Andrzej Hajda wrote:

On 20.07.2023 23:07, Andi Shyti wrote:

...

Enable the CCS_FLUSH bit 13 in the control pipe for render and compute engines in platforms starting from Meteor Lake (BSPEC 43904 and 47112).

Fixes: 972282c4cf24 ("drm/i915/gen12: Add aux table invalidate for all engines") Signed-off-by: Andi Shyti andi.shyti@linux.intel.com Cc: Jonathan Cavitt jonathan.cavitt@intel.com Cc: Nirmoy Das nirmoy.das@intel.com Cc: stable@vger.kernel.org # v5.8+

---

drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 7 +++++++ drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 1 + 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index 7566c89d9def3..9d050b9a19194 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -218,6 +218,13 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) bit_group_0 |= PIPE_CONTROL0_HDC_PIPELINE_FLUSH;

• /*
  

•  * When required, in MTL+ platforms we need to
  

•  * set the CCS_FLUSH bit in the pipe control
  

•  */
  

• if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70))
  

• 	bit_group_0 |= PIPE_CONTROL_CCS_FLUSH;
  

BSpec 43904 mentions also other platforms. Why only MTL+?

This is the process of quiescing the engine and that is done in the pipe control sequence.

In the pipe control sequence each engine has its own sequence, even though render and compute overlap on some bits, while the others overlap on other bits.

Besides that MTL+ need this extra bit to be set in the pipe control and that is bit 13 defined as PIPE_CONTROL_CCS_FLUSH.

Thanks, Andi

Hi Andrzej,

...

diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index 7566c89d9def3..9d050b9a19194 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -218,6 +218,13 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) bit_group_0 |= PIPE_CONTROL0_HDC_PIPELINE_FLUSH;

• /*
  

•  * When required, in MTL+ platforms we need to
  

•  * set the CCS_FLUSH bit in the pipe control
  

•  */
  

• if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70))
  

• 	bit_group_0 |= PIPE_CONTROL_CCS_FLUSH;
  

Btw, not for this patch, but related: rcs and ccs have slightly different set of flushes according to bspec but this functions is the same for both. Is it sth we should address, or just safe simplification.

I guess this is not only used for ccs aux invalidation. I think the BSPEC is specifying the minimum set of bits that need to be set in the pipe control. So that I left it as it is and just added this bit for MTL+.

Thanks, Andi

On 20.07.2023 23:07, Andi Shyti wrote:

Just a trivial refactoring for reducing the number of code duplicate. This will come at handy in the next commits.

Signed-off-by: Andi Shyti andi.shyti@linux.intel.com Cc: stable@vger.kernel.org # v5.8+

---

drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 44 +++++++++++++----------- 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index 9d050b9a19194..202d6ff8b5264 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -177,23 +177,31 @@ u32 *gen12_emit_aux_table_inv(struct intel_gt *gt, u32 *cs, const i915_reg_t inv return cs; } +static u32 *intel_emit_pipe_control_cs(struct i915_request *rq, u32 bit_group_0,

• 		       u32 bit_group_1, u32 offset)
  

s/intel/gen12/

but this and few other issues were raised already by Matt in v6.

Regards Andrzej

+{

• u32 *cs;
• cs = intel_ring_begin(rq, 6);
• if (IS_ERR(cs))

• return cs;
  

• cs = gen12_emit_pipe_control(cs, bit_group_0, bit_group_1,

• 		     LRC_PPHWSP_SCRATCH_ADDR);
  

• intel_ring_advance(rq, cs);
• return cs;

+}

• static int mtl_dummy_pipe_control(struct i915_request *rq) { /* Wa_14016712196 */ if (IS_MTL_GRAPHICS_STEP(rq->engine->i915, M, STEP_A0, STEP_B0) ||

•    IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0)) {
  

• u32 *cs;
  

• /* dummy PIPE_CONTROL + depth flush */
  

• cs = intel_ring_begin(rq, 6);
  

• if (IS_ERR(cs))
  

• 	return PTR_ERR(cs);
  

• cs = gen12_emit_pipe_control(cs,
  

• 			     0,
  

• 			     PIPE_CONTROL_DEPTH_CACHE_FLUSH,
  

• 			     LRC_PPHWSP_SCRATCH_ADDR);
  

• intel_ring_advance(rq, cs);
  

• }

•    IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0))
  

• intel_emit_pipe_control_cs(rq,
  

• 			   0,
  

• 			   PIPE_CONTROL_DEPTH_CACHE_FLUSH,
  

• 			   LRC_PPHWSP_SCRATCH_ADDR);
  

return 0; } @@ -210,7 +218,6 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) u32 bit_group_0 = 0; u32 bit_group_1 = 0; int err;

• u32 *cs;
  

err = mtl_dummy_pipe_control(rq); if (err) @@ -244,13 +251,8 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) else if (engine->class == COMPUTE_CLASS) bit_group_1 &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;

• cs = intel_ring_begin(rq, 6);
  

• if (IS_ERR(cs))
  

• 	return PTR_ERR(cs);
  

• cs = gen12_emit_pipe_control(cs, bit_group_0, bit_group_1,
  

• 			     LRC_PPHWSP_SCRATCH_ADDR);
  

• intel_ring_advance(rq, cs);
  

• intel_emit_pipe_control_cs(rq, bit_group_0, bit_group_1,
  

• 			   LRC_PPHWSP_SCRATCH_ADDR);
  

  }

if (mode & EMIT_INVALIDATE) {

Hi Andi,

On Thursday, 20 July 2023 23:07:34 CEST Andi Shyti wrote:

Just a trivial refactoring for reducing the number of code duplicate. This will come at handy in the next commits.

Signed-off-by: Andi Shyti andi.shyti@linux.intel.com Cc: stable@vger.kernel.org # v5.8+

---

drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 44 +++++++++++++----------- 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index 9d050b9a19194..202d6ff8b5264 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -177,23 +177,31 @@ u32 *gen12_emit_aux_table_inv(struct intel_gt *gt, u32 *cs, const i915_reg_t inv return cs; } +static u32 *intel_emit_pipe_control_cs(struct i915_request *rq, u32 bit_group_0,

• 		       u32 bit_group_1, u32 offset)
  

+{

• u32 *cs;
• cs = intel_ring_begin(rq, 6);
• if (IS_ERR(cs))

• return cs;
  

• cs = gen12_emit_pipe_control(cs, bit_group_0, bit_group_1,

• 		     LRC_PPHWSP_SCRATCH_ADDR);
  

• intel_ring_advance(rq, cs);
• return cs;

+}

static int mtl_dummy_pipe_control(struct i915_request *rq) { /* Wa_14016712196 */ if (IS_MTL_GRAPHICS_STEP(rq->engine->i915, M, STEP_A0, STEP_B0) ||

•    IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0)) {
  

• u32 *cs;
  

• /* dummy PIPE_CONTROL + depth flush */
  

NIT: Maybe you could keep that comment, above intel_emit_pipe_control_cs(...).

Anyway, LGTM.

Reviewed-by: Janusz Krzysztofik janusz.krzysztofik@linux.intel.com

• cs = intel_ring_begin(rq, 6);
  

• if (IS_ERR(cs))
  

• 	return PTR_ERR(cs);
  

• cs = gen12_emit_pipe_control(cs,
  

• 			     0,
  

• 			     PIPE_CONTROL_DEPTH_CACHE_FLUSH,
  

• 			     LRC_PPHWSP_SCRATCH_ADDR);
  

• intel_ring_advance(rq, cs);
  

• }

•    IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0))
  

• intel_emit_pipe_control_cs(rq,
  

• 			   0,
  

• 			   PIPE_CONTROL_DEPTH_CACHE_FLUSH,
  

• 			   LRC_PPHWSP_SCRATCH_ADDR);
  

return 0; } @@ -210,7 +218,6 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) u32 bit_group_0 = 0; u32 bit_group_1 = 0; int err;

• u32 *cs;
  

err = mtl_dummy_pipe_control(rq); if (err) @@ -244,13 +251,8 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) else if (engine->class == COMPUTE_CLASS) bit_group_1 &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;

• cs = intel_ring_begin(rq, 6);
  

• if (IS_ERR(cs))
  

• 	return PTR_ERR(cs);
  

• cs = gen12_emit_pipe_control(cs, bit_group_0, bit_group_1,
  

• 			     LRC_PPHWSP_SCRATCH_ADDR);
  

• intel_ring_advance(rq, cs);
  

• intel_emit_pipe_control_cs(rq, bit_group_0, bit_group_1,
  

• 			   LRC_PPHWSP_SCRATCH_ADDR);
  

  }

if (mode & EMIT_INVALIDATE) {

--------------------------------------------------------------------- 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.

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.

Hi Andi,

On Thursday, 20 July 2023 23:07:37 CEST Andi Shyti wrote:

Perform some refactoring with the purpose of keeping in one single place all the operations around the aux table invalidation.

With this refactoring add more engines where the invalidation should be performed.

Fixes: 972282c4cf24 ("drm/i915/gen12: Add aux table invalidate for all engines") 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 | 58 +++++++++++++++--------- drivers/gpu/drm/i915/gt/gen8_engine_cs.h | 3 +- drivers/gpu/drm/i915/gt/intel_lrc.c | 17 +------ 3 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index 3ded597f002a2..30fb4e0af6134 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -165,9 +165,36 @@ static u32 preparser_disable(bool state) return MI_ARB_CHECK | 1 << 8 | state; } -u32 *gen12_emit_aux_table_inv(struct intel_gt *gt, u32 *cs, const i915_reg_t inv_reg) +static i915_reg_t gen12_get_aux_inv_reg(struct intel_engine_cs *engine) {

• u32 gsi_offset = gt->uncore->gsi_offset;

• if (!HAS_AUX_CCS(engine->i915))

• return INVALID_MMIO_REG;
  

• switch (engine->id) {
• case RCS0:

• return GEN12_CCS_AUX_INV;
  

• case BCS0:

• return GEN12_BCS0_AUX_INV;
  

• case VCS0:

• return GEN12_VD0_AUX_INV;
  

• case VCS2:

• return GEN12_VD2_AUX_INV;
  

• case VECS0:

• return GEN12_VE0_AUX_INV;
  

• case CCS0:

• return GEN12_CCS0_AUX_INV;
  

• default:

• return INVALID_MMIO_REG;
  

• }

+}

+u32 *gen12_emit_aux_table_inv(struct intel_engine_cs *engine, u32 *cs) +{

• i915_reg_t inv_reg = gen12_get_aux_inv_reg(engine);
• u32 gsi_offset = engine->gt->uncore->gsi_offset;
• if (i915_mmio_reg_valid(inv_reg))

• return cs;
  

Is that correct? Now the original body of gen12_emit_aux_table_inv() will be executed only if either (!HAS_AUX_CCS(engine->i915) or the engine is not one of (RCS0, BCS0, VCS0, VCS2 or CCS0), ...

*cs++ = MI_LOAD_REGISTER_IMM(1) | MI_LRI_MMIO_REMAP_EN; *cs++ = i915_mmio_reg_offset(inv_reg) + gsi_offset; @@ -201,6 +228,11 @@ static u32 *intel_emit_pipe_control_cs(struct i915_request *rq, u32 bit_group_0, return cs; } +static bool gen12_engine_has_aux_inv(struct intel_engine_cs *engine) +{

• return i915_mmio_reg_valid(gen12_get_aux_inv_reg(engine));

+}

static int mtl_dummy_pipe_control(struct i915_request *rq) { /* Wa_14016712196 */ @@ -307,11 +339,7 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) cs = gen8_emit_pipe_control(cs, flags, LRC_PPHWSP_SCRATCH_ADDR);

• if (!HAS_FLAT_CCS(rq->engine->i915)) {
  

... while before it was executed only if (!HAS_FLAT_CCS(rq->engine->i915)), which, according to commit description of PATCH 2/9, rather had the opposite meaning. Am I missing something?

Thanks, Janusz

• 	/* hsdes: 1809175790 */
  

• 	cs = gen12_emit_aux_table_inv(rq->engine->gt, cs,
  

• 				      GEN12_CCS_AUX_INV);
  

• }
  

• cs = gen12_emit_aux_table_inv(engine, cs);
  

*cs++ = preparser_disable(false); intel_ring_advance(rq, cs); @@ -322,7 +350,6 @@ 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_flush = 0; u32 cmd = 4; u32 *cs;

@@ -330,15 +357,11 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode) if (mode & EMIT_INVALIDATE) cmd += 2;

• 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) {

• if (gen12_engine_has_aux_inv(rq->engine)) { cmd += 8; /* for the AUX invalidation */ cmd += 2; /* for the engine quiescing */

@@ -381,14 +404,7 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode) *cs++ = 0; /* upper addr */ *cs++ = 0; /* value */

• if (aux_inv) { /* hsdes: 1809175790 */

• if (rq->engine->class == VIDEO_DECODE_CLASS)
  

• 	cs = gen12_emit_aux_table_inv(rq->engine->gt,
  

• 				      cs, GEN12_VD0_AUX_INV);
  

• else
  

• 	cs = gen12_emit_aux_table_inv(rq->engine->gt,
  

• 				      cs, GEN12_VE0_AUX_INV);
  

• }

• cs = gen12_emit_aux_table_inv(rq->engine, cs);

if (mode & EMIT_INVALIDATE) *cs++ = preparser_disable(false); diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.h b/drivers/gpu/drm/i915/gt/gen8_engine_cs.h index a44eda096557c..867ba697aceb8 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.h +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.h @@ -13,6 +13,7 @@ #include "intel_gt_regs.h" #include "intel_gpu_commands.h" +struct intel_engine_cs; struct intel_gt; struct i915_request; @@ -46,7 +47,7 @@ u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs); u32 *gen11_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs); u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs); -u32 *gen12_emit_aux_table_inv(struct intel_gt *gt, u32 *cs, const i915_reg_t inv_reg); +u32 *gen12_emit_aux_table_inv(struct intel_engine_cs *engine, u32 *cs); static inline u32 * __gen8_emit_pipe_control(u32 *batch, u32 bit_group_0, diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 235f3fab60a98..119deb9f938c7 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -1371,10 +1371,7 @@ gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs) IS_DG2_G11(ce->engine->i915)) cs = gen8_emit_pipe_control(cs, PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE, 0);

• /* hsdes: 1809175790 */
• if (!HAS_FLAT_CCS(ce->engine->i915))

• cs = gen12_emit_aux_table_inv(ce->engine->gt,
  

• 			      cs, GEN12_CCS_AUX_INV);
  

• cs = gen12_emit_aux_table_inv(ce->engine, cs);

/* Wa_16014892111 */ if (IS_MTL_GRAPHICS_STEP(ce->engine->i915, M, STEP_A0, STEP_B0) || @@ -1399,17 +1396,7 @@ gen12_emit_indirect_ctx_xcs(const struct intel_context *ce, u32 *cs) PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE, 0);

• /* hsdes: 1809175790 */
• if (!HAS_FLAT_CCS(ce->engine->i915)) {

• if (ce->engine->class == VIDEO_DECODE_CLASS)
  

• 	cs = gen12_emit_aux_table_inv(ce->engine->gt,
  

• 				      cs, GEN12_VD0_AUX_INV);
  

• else if (ce->engine->class == VIDEO_ENHANCEMENT_CLASS)
  

• 	cs = gen12_emit_aux_table_inv(ce->engine->gt,
  

• 				      cs, GEN12_VE0_AUX_INV);
  

• }
• return cs;

• return gen12_emit_aux_table_inv(ce->engine, cs);

} static void

--------------------------------------------------------------------- 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.