[Intel-gfx] [PATCH 10/12] drm/i915/gt: Refactor l3cc/mocs availability
Daniele Ceraolo Spurio
daniele.ceraolospurio at intel.com
Tue Feb 18 21:24:47 UTC 2020
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 at chris-wilson.co.uk>
> Cc: Brian Welty <brian.welty at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at 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 at 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;
>
>
More information about the Intel-gfx
mailing list