diff mbox series

[10/12] drm/i915/gt: Refactor l3cc/mocs availability

Message ID 20200218162150.1300405-10-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [01/12] drm/i915/selftests: Check for any sign of request starting in wait_for_submit() | expand

Commit Message

Chris Wilson Feb. 18, 2020, 4:21 p.m. UTC 
On dgfx, we only use l3cc and not mocs, but we share the table of
register definitions with Tigerlake (which includes the mocs). This
confuses our selftest that verifies that the registers do contain the
values in our tables after various events (idling, reset, activity etc).

When constructing the table of register definitions, also include the
flags for which registers are valid so that information is computed
centrally and available to all callers.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Brian Welty <brian.welty@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_mocs.c    | 72 +++++++++++++++++--------
 drivers/gpu/drm/i915/gt/selftest_mocs.c | 24 ++++++---
 2 files changed, 67 insertions(+), 29 deletions(-)

Comments

Welty, Brian Feb. 18, 2020, 6:26 p.m. UTC | #1 
On 2/18/2020 8:21 AM, Chris Wilson wrote:
> On dgfx, we only use l3cc and not mocs, but we share the table of
> register definitions with Tigerlake (which includes the mocs). This
> confuses our selftest that verifies that the registers do contain the
> values in our tables after various events (idling, reset, activity etc).
> 
> When constructing the table of register definitions, also include the
> flags for which registers are valid so that information is computed
> centrally and available to all callers.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Brian Welty <brian.welty@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_mocs.c    | 72 +++++++++++++++++--------
>  drivers/gpu/drm/i915/gt/selftest_mocs.c | 24 ++++++---
>  2 files changed, 67 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
> index 0afc1eb3c20f..632e08a4592b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> @@ -280,9 +280,32 @@ static const struct drm_i915_mocs_entry icl_mocs_table[] = {
>  	GEN11_MOCS_ENTRIES
>  };
>  
> -static bool get_mocs_settings(const struct drm_i915_private *i915,
> -			      struct drm_i915_mocs_table *table)
> +enum {
> +	HAS_GLOBAL_MOCS = BIT(0),
> +	HAS_ENGINE_MOCS = BIT(1),
> +	HAS_RENDER_L3CC = BIT(2),
> +};
> +
> +static bool has_l3cc(const struct drm_i915_private *i915)
>  {
> +	return true;
> +}
> +
> +static bool has_global_mocs(const struct drm_i915_private *i915)
> +{
> +	return HAS_GLOBAL_MOCS_REGISTERS(i915);
> +}
> +
> +static bool has_mocs(const struct drm_i915_private *i915)
> +{
> +	return !IS_DGFX(i915);
> +}
> +
> +static unsigned int get_mocs_settings(const struct drm_i915_private *i915,
> +				      struct drm_i915_mocs_table *table)
> +{
> +	unsigned int flags;
> +
>  	if (INTEL_GEN(i915) >= 12) {
>  		table->size  = ARRAY_SIZE(tgl_mocs_table);
>  		table->table = tgl_mocs_table;
> @@ -302,11 +325,11 @@ static bool get_mocs_settings(const struct drm_i915_private *i915,
>  	} else {
>  		drm_WARN_ONCE(&i915->drm, INTEL_GEN(i915) >= 9,
>  			      "Platform that should have a MOCS table does not.\n");
> -		return false;
> +		return 0;
>  	}
>  
>  	if (GEM_DEBUG_WARN_ON(table->size > table->n_entries))
> -		return false;
> +		return 0;
>  
>  	/* WaDisableSkipCaching:skl,bxt,kbl,glk */
>  	if (IS_GEN(i915, 9)) {
> @@ -315,10 +338,20 @@ static bool get_mocs_settings(const struct drm_i915_private *i915,
>  		for (i = 0; i < table->size; i++)
>  			if (GEM_DEBUG_WARN_ON(table->table[i].l3cc_value &
>  					      (L3_ESC(1) | L3_SCC(0x7))))
> -				return false;
> +				return 0;
>  	}
>  
> -	return true;
> +	flags = 0;
> +	if (has_mocs(i915)) {
> +		if (has_global_mocs(i915))
> +			flags |= HAS_GLOBAL_MOCS;
> +		else
> +			flags |= HAS_ENGINE_MOCS;
> +	}
> +	if (has_l3cc(i915))
> +		flags |= HAS_RENDER_L3CC;
> +
> +	return flags;
>  }
>  
>  /*
> @@ -411,18 +444,20 @@ static void init_l3cc_table(struct intel_engine_cs *engine,
>  void intel_mocs_init_engine(struct intel_engine_cs *engine)
>  {
>  	struct drm_i915_mocs_table table;
> +	unsigned int flags;
>  
>  	/* Called under a blanket forcewake */
>  	assert_forcewakes_active(engine->uncore, FORCEWAKE_ALL);
>  
> -	if (!get_mocs_settings(engine->i915, &table))
> +	flags = get_mocs_settings(engine->i915, &table);
> +	if (!flags)
>  		return;
>  
>  	/* Platforms with global MOCS do not need per-engine initialization. */
> -	if (!HAS_GLOBAL_MOCS_REGISTERS(engine->i915))
> +	if (flags & HAS_ENGINE_MOCS)
>  		init_mocs_table(engine, &table);
>  
> -	if (engine->class == RENDER_CLASS)
> +	if (flags & HAS_RENDER_L3CC && engine->class == RENDER_CLASS)
>  		init_l3cc_table(engine, &table);
>  }
>  
> @@ -431,26 +466,17 @@ static u32 global_mocs_offset(void)
>  	return i915_mmio_reg_offset(GEN12_GLOBAL_MOCS(0));
>  }
>  
> -static void init_global_mocs(struct intel_gt *gt)
> +void intel_mocs_init(struct intel_gt *gt)
>  {
>  	struct drm_i915_mocs_table table;
> +	unsigned int flags;
>  
>  	/*
>  	 * LLC and eDRAM control values are not applicable to dgfx
>  	 */
> -	if (IS_DGFX(gt->i915))
> -		return;
> -
> -	if (!get_mocs_settings(gt->i915, &table))
> -		return;
> -
> -	__init_mocs_table(gt->uncore, &table, global_mocs_offset());
> -}
> -
> -void intel_mocs_init(struct intel_gt *gt)
> -{
> -	if (HAS_GLOBAL_MOCS_REGISTERS(gt->i915))
> -		init_global_mocs(gt);
> +	flags = get_mocs_settings(gt->i915, &table);
> +	if (flags & HAS_GLOBAL_MOCS)
> +		__init_mocs_table(gt->uncore, &table, global_mocs_offset());
>  }
>  
>  #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> index de1f83100fb6..8831ffee2061 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> @@ -12,7 +12,8 @@
>  #include "selftests/igt_spinner.h"
>  
>  struct live_mocs {
> -	struct drm_i915_mocs_table table;
> +	struct drm_i915_mocs_table mocs;
> +	struct drm_i915_mocs_table l3cc;
>  	struct i915_vma *scratch;
>  	void *vaddr;
>  };
> @@ -70,11 +71,22 @@ static struct i915_vma *create_scratch(struct intel_gt *gt)
>  
>  static int live_mocs_init(struct live_mocs *arg, struct intel_gt *gt)
>  {
> +	struct drm_i915_mocs_table table;
> +	unsigned int flags;
>  	int err;
>  
> -	if (!get_mocs_settings(gt->i915, &arg->table))
> +	memset(arg, 0, sizeof(*arg));
> +
> +	flags = get_mocs_settings(gt->i915, &table);
> +	if (!flags)
>  		return -EINVAL;
>  
> +	if (flags & HAS_RENDER_L3CC)
> +		arg->l3cc = table;
> +
> +	if (flags & (HAS_GLOBAL_MOCS | HAS_ENGINE_MOCS))
> +		arg->mocs = table;
> +
>  	arg->scratch = create_scratch(gt);
>  	if (IS_ERR(arg->scratch))
>  		return PTR_ERR(arg->scratch);
> @@ -223,9 +235,9 @@ static int check_mocs_engine(struct live_mocs *arg,
>  	/* Read the mocs tables back using SRM */
>  	offset = i915_ggtt_offset(vma);
>  	if (!err)
> -		err = read_mocs_table(rq, &arg->table, &offset);
> +		err = read_mocs_table(rq, &arg->mocs, &offset);
>  	if (!err && ce->engine->class == RENDER_CLASS)
> -		err = read_l3cc_table(rq, &arg->table, &offset);
> +		err = read_l3cc_table(rq, &arg->l3cc, &offset);


Above functions will call read_regs().
I thought we'd still want to avoid this.  Shouldn't you test flags here?
Maybe store flags in the struct live_mocs and something like:

 	if (!err && (arg->flags & (HAS_GLOBAL_MOCS | HAS_ENGINE_MOCS)))
		err = read_mocs_table(rq, &arg->mocs, &offset);
 	if (!err && (arg->flags & HAS_RENDER_L3CC) && ce->engine->class == RENDER_CLASS)
		err = read_l3cc_table(rq, &arg->l3cc, &offset);


>  	offset -= i915_ggtt_offset(vma);
>  	GEM_BUG_ON(offset > PAGE_SIZE);
>  
> @@ -236,9 +248,9 @@ static int check_mocs_engine(struct live_mocs *arg,
>  	/* Compare the results against the expected tables */
>  	vaddr = arg->vaddr;
>  	if (!err)
> -		err = check_mocs_table(ce->engine, &arg->table, &vaddr);
> +		err = check_mocs_table(ce->engine, &arg->mocs, &vaddr);
>  	if (!err && ce->engine->class == RENDER_CLASS)
> -		err = check_l3cc_table(ce->engine, &arg->table, &vaddr);
> +		err = check_l3cc_table(ce->engine, &arg->l3cc, &vaddr);
>  	if (err)
>  		return err;
>  
>
Chris Wilson Feb. 18, 2020, 6:33 p.m. UTC | #2 
Quoting Brian Welty (2020-02-18 18:26:40)
> 
> On 2/18/2020 8:21 AM, Chris Wilson wrote:
> > @@ -223,9 +235,9 @@ static int check_mocs_engine(struct live_mocs *arg,
> >       /* Read the mocs tables back using SRM */
> >       offset = i915_ggtt_offset(vma);
> >       if (!err)
> > -             err = read_mocs_table(rq, &arg->table, &offset);
> > +             err = read_mocs_table(rq, &arg->mocs, &offset);
> >       if (!err && ce->engine->class == RENDER_CLASS)
> > -             err = read_l3cc_table(rq, &arg->table, &offset);
> > +             err = read_l3cc_table(rq, &arg->l3cc, &offset);
> 
> 
> Above functions will call read_regs().

And do nothing.

Perhaps using the flags for read_mocs_table() would be nice to eliminate
the HAS_GLOBAL_MOCS_REGISTERS(). Or creating global_mocs, engine_mocs,
render_l3cc. That might be overkill.
-Chris
Daniele Ceraolo Spurio Feb. 18, 2020, 9:24 p.m. UTC | #3 
On 2/18/20 8:21 AM, Chris Wilson wrote:
> On dgfx, we only use l3cc and not mocs, but we share the table of
> register definitions with Tigerlake (which includes the mocs). This

Just a small correction here: the problem is not that the Tigerlake 
definitions will be shared (which is not necessarily going to happen), 
but that our table entry definition contains both l3cc and mocs and 
there is currently no way to know if only one of the 2 is valid. We 
could split the table, but IMO that'd be overkill and it'll make things 
messier for integrated platforms that have both, so I prefer the 
approach in this patch.

> confuses our selftest that verifies that the registers do contain the
> values in our tables after various events (idling, reset, activity etc).
> 
> When constructing the table of register definitions, also include the
> flags for which registers are valid so that information is computed
> centrally and available to all callers.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Brian Welty <brian.welty@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Was confused for a moment by the uninitialized table passed to 
read_mocs_table(), but we're ok because we memset it to 0 and therefore 
table->n_entries is zero. Maybe worth adding a check to avoid calling 
ring_begin() and ring_advance() in read_regs() that scenario?

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele

> ---
>   drivers/gpu/drm/i915/gt/intel_mocs.c    | 72 +++++++++++++++++--------
>   drivers/gpu/drm/i915/gt/selftest_mocs.c | 24 ++++++---
>   2 files changed, 67 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
> index 0afc1eb3c20f..632e08a4592b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> @@ -280,9 +280,32 @@ static const struct drm_i915_mocs_entry icl_mocs_table[] = {
>   	GEN11_MOCS_ENTRIES
>   };
>   
> -static bool get_mocs_settings(const struct drm_i915_private *i915,
> -			      struct drm_i915_mocs_table *table)
> +enum {
> +	HAS_GLOBAL_MOCS = BIT(0),
> +	HAS_ENGINE_MOCS = BIT(1),
> +	HAS_RENDER_L3CC = BIT(2),
> +};
> +
> +static bool has_l3cc(const struct drm_i915_private *i915)
>   {
> +	return true;
> +}
> +
> +static bool has_global_mocs(const struct drm_i915_private *i915)
> +{
> +	return HAS_GLOBAL_MOCS_REGISTERS(i915);
> +}
> +
> +static bool has_mocs(const struct drm_i915_private *i915)
> +{
> +	return !IS_DGFX(i915);
> +}
> +
> +static unsigned int get_mocs_settings(const struct drm_i915_private *i915,
> +				      struct drm_i915_mocs_table *table)
> +{
> +	unsigned int flags;
> +
>   	if (INTEL_GEN(i915) >= 12) {
>   		table->size  = ARRAY_SIZE(tgl_mocs_table);
>   		table->table = tgl_mocs_table;
> @@ -302,11 +325,11 @@ static bool get_mocs_settings(const struct drm_i915_private *i915,
>   	} else {
>   		drm_WARN_ONCE(&i915->drm, INTEL_GEN(i915) >= 9,
>   			      "Platform that should have a MOCS table does not.\n");
> -		return false;
> +		return 0;
>   	}
>   
>   	if (GEM_DEBUG_WARN_ON(table->size > table->n_entries))
> -		return false;
> +		return 0;
>   
>   	/* WaDisableSkipCaching:skl,bxt,kbl,glk */
>   	if (IS_GEN(i915, 9)) {
> @@ -315,10 +338,20 @@ static bool get_mocs_settings(const struct drm_i915_private *i915,
>   		for (i = 0; i < table->size; i++)
>   			if (GEM_DEBUG_WARN_ON(table->table[i].l3cc_value &
>   					      (L3_ESC(1) | L3_SCC(0x7))))
> -				return false;
> +				return 0;
>   	}
>   
> -	return true;
> +	flags = 0;
> +	if (has_mocs(i915)) {
> +		if (has_global_mocs(i915))
> +			flags |= HAS_GLOBAL_MOCS;
> +		else
> +			flags |= HAS_ENGINE_MOCS;
> +	}
> +	if (has_l3cc(i915))
> +		flags |= HAS_RENDER_L3CC;
> +
> +	return flags;
>   }
>   
>   /*
> @@ -411,18 +444,20 @@ static void init_l3cc_table(struct intel_engine_cs *engine,
>   void intel_mocs_init_engine(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_mocs_table table;
> +	unsigned int flags;
>   
>   	/* Called under a blanket forcewake */
>   	assert_forcewakes_active(engine->uncore, FORCEWAKE_ALL);
>   
> -	if (!get_mocs_settings(engine->i915, &table))
> +	flags = get_mocs_settings(engine->i915, &table);
> +	if (!flags)
>   		return;
>   
>   	/* Platforms with global MOCS do not need per-engine initialization. */
> -	if (!HAS_GLOBAL_MOCS_REGISTERS(engine->i915))
> +	if (flags & HAS_ENGINE_MOCS)
>   		init_mocs_table(engine, &table);
>   
> -	if (engine->class == RENDER_CLASS)
> +	if (flags & HAS_RENDER_L3CC && engine->class == RENDER_CLASS)
>   		init_l3cc_table(engine, &table);
>   }
>   
> @@ -431,26 +466,17 @@ static u32 global_mocs_offset(void)
>   	return i915_mmio_reg_offset(GEN12_GLOBAL_MOCS(0));
>   }
>   
> -static void init_global_mocs(struct intel_gt *gt)
> +void intel_mocs_init(struct intel_gt *gt)
>   {
>   	struct drm_i915_mocs_table table;
> +	unsigned int flags;
>   
>   	/*
>   	 * LLC and eDRAM control values are not applicable to dgfx
>   	 */
> -	if (IS_DGFX(gt->i915))
> -		return;
> -
> -	if (!get_mocs_settings(gt->i915, &table))
> -		return;
> -
> -	__init_mocs_table(gt->uncore, &table, global_mocs_offset());
> -}
> -
> -void intel_mocs_init(struct intel_gt *gt)
> -{
> -	if (HAS_GLOBAL_MOCS_REGISTERS(gt->i915))
> -		init_global_mocs(gt);
> +	flags = get_mocs_settings(gt->i915, &table);
> +	if (flags & HAS_GLOBAL_MOCS)
> +		__init_mocs_table(gt->uncore, &table, global_mocs_offset());
>   }
>   
>   #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> index de1f83100fb6..8831ffee2061 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> @@ -12,7 +12,8 @@
>   #include "selftests/igt_spinner.h"
>   
>   struct live_mocs {
> -	struct drm_i915_mocs_table table;
> +	struct drm_i915_mocs_table mocs;
> +	struct drm_i915_mocs_table l3cc;
>   	struct i915_vma *scratch;
>   	void *vaddr;
>   };
> @@ -70,11 +71,22 @@ static struct i915_vma *create_scratch(struct intel_gt *gt)
>   
>   static int live_mocs_init(struct live_mocs *arg, struct intel_gt *gt)
>   {
> +	struct drm_i915_mocs_table table;
> +	unsigned int flags;
>   	int err;
>   
> -	if (!get_mocs_settings(gt->i915, &arg->table))
> +	memset(arg, 0, sizeof(*arg));
> +
> +	flags = get_mocs_settings(gt->i915, &table);
> +	if (!flags)
>   		return -EINVAL;
>   
> +	if (flags & HAS_RENDER_L3CC)
> +		arg->l3cc = table;
> +
> +	if (flags & (HAS_GLOBAL_MOCS | HAS_ENGINE_MOCS))
> +		arg->mocs = table;
> +
>   	arg->scratch = create_scratch(gt);
>   	if (IS_ERR(arg->scratch))
>   		return PTR_ERR(arg->scratch);
> @@ -223,9 +235,9 @@ static int check_mocs_engine(struct live_mocs *arg,
>   	/* Read the mocs tables back using SRM */
>   	offset = i915_ggtt_offset(vma);
>   	if (!err)
> -		err = read_mocs_table(rq, &arg->table, &offset);
> +		err = read_mocs_table(rq, &arg->mocs, &offset);
>   	if (!err && ce->engine->class == RENDER_CLASS)
> -		err = read_l3cc_table(rq, &arg->table, &offset);
> +		err = read_l3cc_table(rq, &arg->l3cc, &offset);
>   	offset -= i915_ggtt_offset(vma);
>   	GEM_BUG_ON(offset > PAGE_SIZE);
>   
> @@ -236,9 +248,9 @@ static int check_mocs_engine(struct live_mocs *arg,
>   	/* Compare the results against the expected tables */
>   	vaddr = arg->vaddr;
>   	if (!err)
> -		err = check_mocs_table(ce->engine, &arg->table, &vaddr);
> +		err = check_mocs_table(ce->engine, &arg->mocs, &vaddr);
>   	if (!err && ce->engine->class == RENDER_CLASS)
> -		err = check_l3cc_table(ce->engine, &arg->table, &vaddr);
> +		err = check_l3cc_table(ce->engine, &arg->l3cc, &vaddr);
>   	if (err)
>   		return err;
>   
>
Chris Wilson Feb. 18, 2020, 9:38 p.m. UTC | #4 
Quoting Daniele Ceraolo Spurio (2020-02-18 21:24:47)
> 
> 
> On 2/18/20 8:21 AM, Chris Wilson wrote:
> > On dgfx, we only use l3cc and not mocs, but we share the table of
> > register definitions with Tigerlake (which includes the mocs). This

-share the table of register definitions
+share the table containing both register definitions
 
> Just a small correction here: the problem is not that the Tigerlake 
> definitions will be shared (which is not necessarily going to happen), 
> but that our table entry definition contains both l3cc and mocs and 
> there is currently no way to know if only one of the 2 is valid. We 
> could split the table, but IMO that'd be overkill and it'll make things 
> messier for integrated platforms that have both, so I prefer the 
> approach in this patch.
> 
> > confuses our selftest that verifies that the registers do contain the
> > values in our tables after various events (idling, reset, activity etc).
> > 
> > When constructing the table of register definitions, also include the
> > flags for which registers are valid so that information is computed
> > centrally and available to all callers.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Brian Welty <brian.welty@intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> Was confused for a moment by the uninitialized table passed to 
> read_mocs_table(), but we're ok because we memset it to 0 and therefore 

I did put the memset there to try and reassure :)

> table->n_entries is zero. Maybe worth adding a check to avoid calling 
> ring_begin() and ring_advance() in read_regs() that scenario?

ring_advance is just a debug aide; ring_begin becomes a no-op, after a
few twists and turns. (At worst it is a intel_ring_wrap.)

I liked the simple look of not having to special case 0.
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
index 0afc1eb3c20f..632e08a4592b 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
@@ -280,9 +280,32 @@  static const struct drm_i915_mocs_entry icl_mocs_table[] = {
 	GEN11_MOCS_ENTRIES
 };
 
-static bool get_mocs_settings(const struct drm_i915_private *i915,
-			      struct drm_i915_mocs_table *table)
+enum {
+	HAS_GLOBAL_MOCS = BIT(0),
+	HAS_ENGINE_MOCS = BIT(1),
+	HAS_RENDER_L3CC = BIT(2),
+};
+
+static bool has_l3cc(const struct drm_i915_private *i915)
 {
+	return true;
+}
+
+static bool has_global_mocs(const struct drm_i915_private *i915)
+{
+	return HAS_GLOBAL_MOCS_REGISTERS(i915);
+}
+
+static bool has_mocs(const struct drm_i915_private *i915)
+{
+	return !IS_DGFX(i915);
+}
+
+static unsigned int get_mocs_settings(const struct drm_i915_private *i915,
+				      struct drm_i915_mocs_table *table)
+{
+	unsigned int flags;
+
 	if (INTEL_GEN(i915) >= 12) {
 		table->size  = ARRAY_SIZE(tgl_mocs_table);
 		table->table = tgl_mocs_table;
@@ -302,11 +325,11 @@  static bool get_mocs_settings(const struct drm_i915_private *i915,
 	} else {
 		drm_WARN_ONCE(&i915->drm, INTEL_GEN(i915) >= 9,
 			      "Platform that should have a MOCS table does not.\n");
-		return false;
+		return 0;
 	}
 
 	if (GEM_DEBUG_WARN_ON(table->size > table->n_entries))
-		return false;
+		return 0;
 
 	/* WaDisableSkipCaching:skl,bxt,kbl,glk */
 	if (IS_GEN(i915, 9)) {
@@ -315,10 +338,20 @@  static bool get_mocs_settings(const struct drm_i915_private *i915,
 		for (i = 0; i < table->size; i++)
 			if (GEM_DEBUG_WARN_ON(table->table[i].l3cc_value &
 					      (L3_ESC(1) | L3_SCC(0x7))))
-				return false;
+				return 0;
 	}
 
-	return true;
+	flags = 0;
+	if (has_mocs(i915)) {
+		if (has_global_mocs(i915))
+			flags |= HAS_GLOBAL_MOCS;
+		else
+			flags |= HAS_ENGINE_MOCS;
+	}
+	if (has_l3cc(i915))
+		flags |= HAS_RENDER_L3CC;
+
+	return flags;
 }
 
 /*
@@ -411,18 +444,20 @@  static void init_l3cc_table(struct intel_engine_cs *engine,
 void intel_mocs_init_engine(struct intel_engine_cs *engine)
 {
 	struct drm_i915_mocs_table table;
+	unsigned int flags;
 
 	/* Called under a blanket forcewake */
 	assert_forcewakes_active(engine->uncore, FORCEWAKE_ALL);
 
-	if (!get_mocs_settings(engine->i915, &table))
+	flags = get_mocs_settings(engine->i915, &table);
+	if (!flags)
 		return;
 
 	/* Platforms with global MOCS do not need per-engine initialization. */
-	if (!HAS_GLOBAL_MOCS_REGISTERS(engine->i915))
+	if (flags & HAS_ENGINE_MOCS)
 		init_mocs_table(engine, &table);
 
-	if (engine->class == RENDER_CLASS)
+	if (flags & HAS_RENDER_L3CC && engine->class == RENDER_CLASS)
 		init_l3cc_table(engine, &table);
 }
 
@@ -431,26 +466,17 @@  static u32 global_mocs_offset(void)
 	return i915_mmio_reg_offset(GEN12_GLOBAL_MOCS(0));
 }
 
-static void init_global_mocs(struct intel_gt *gt)
+void intel_mocs_init(struct intel_gt *gt)
 {
 	struct drm_i915_mocs_table table;
+	unsigned int flags;
 
 	/*
 	 * LLC and eDRAM control values are not applicable to dgfx
 	 */
-	if (IS_DGFX(gt->i915))
-		return;
-
-	if (!get_mocs_settings(gt->i915, &table))
-		return;
-
-	__init_mocs_table(gt->uncore, &table, global_mocs_offset());
-}
-
-void intel_mocs_init(struct intel_gt *gt)
-{
-	if (HAS_GLOBAL_MOCS_REGISTERS(gt->i915))
-		init_global_mocs(gt);
+	flags = get_mocs_settings(gt->i915, &table);
+	if (flags & HAS_GLOBAL_MOCS)
+		__init_mocs_table(gt->uncore, &table, global_mocs_offset());
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
index de1f83100fb6..8831ffee2061 100644
--- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
@@ -12,7 +12,8 @@ 
 #include "selftests/igt_spinner.h"
 
 struct live_mocs {
-	struct drm_i915_mocs_table table;
+	struct drm_i915_mocs_table mocs;
+	struct drm_i915_mocs_table l3cc;
 	struct i915_vma *scratch;
 	void *vaddr;
 };
@@ -70,11 +71,22 @@  static struct i915_vma *create_scratch(struct intel_gt *gt)
 
 static int live_mocs_init(struct live_mocs *arg, struct intel_gt *gt)
 {
+	struct drm_i915_mocs_table table;
+	unsigned int flags;
 	int err;
 
-	if (!get_mocs_settings(gt->i915, &arg->table))
+	memset(arg, 0, sizeof(*arg));
+
+	flags = get_mocs_settings(gt->i915, &table);
+	if (!flags)
 		return -EINVAL;
 
+	if (flags & HAS_RENDER_L3CC)
+		arg->l3cc = table;
+
+	if (flags & (HAS_GLOBAL_MOCS | HAS_ENGINE_MOCS))
+		arg->mocs = table;
+
 	arg->scratch = create_scratch(gt);
 	if (IS_ERR(arg->scratch))
 		return PTR_ERR(arg->scratch);
@@ -223,9 +235,9 @@  static int check_mocs_engine(struct live_mocs *arg,
 	/* Read the mocs tables back using SRM */
 	offset = i915_ggtt_offset(vma);
 	if (!err)
-		err = read_mocs_table(rq, &arg->table, &offset);
+		err = read_mocs_table(rq, &arg->mocs, &offset);
 	if (!err && ce->engine->class == RENDER_CLASS)
-		err = read_l3cc_table(rq, &arg->table, &offset);
+		err = read_l3cc_table(rq, &arg->l3cc, &offset);
 	offset -= i915_ggtt_offset(vma);
 	GEM_BUG_ON(offset > PAGE_SIZE);
 
@@ -236,9 +248,9 @@  static int check_mocs_engine(struct live_mocs *arg,
 	/* Compare the results against the expected tables */
 	vaddr = arg->vaddr;
 	if (!err)
-		err = check_mocs_table(ce->engine, &arg->table, &vaddr);
+		err = check_mocs_table(ce->engine, &arg->mocs, &vaddr);
 	if (!err && ce->engine->class == RENDER_CLASS)
-		err = check_l3cc_table(ce->engine, &arg->table, &vaddr);
+		err = check_l3cc_table(ce->engine, &arg->l3cc, &vaddr);
 	if (err)
 		return err;