Chris Wilson <[email protected]> writes:
> We repeatedly (and more so in future) use the same looping construct
> over the mocs definition table to setup the register state. Refactor the
> loop construct into a reusable macro.
>
> add/remove: 2/1 grow/shrink: 1/2 up/down: 113/-330 (-217)
> Function old new delta
> intel_mocs_init_engine.cold - 71 +71
> offset - 28 +28
> __func__ 17273 17287 +14
> intel_mocs_init 143 113 -30
> mocs_register.isra 91 - -91
> intel_mocs_init_engine 503 294 -209
>
> Signed-off-by: Chris Wilson <[email protected]>
> ---
> drivers/gpu/drm/i915/gt/intel_mocs.c | 128 ++++++++++-----------------
> 1 file changed, 47 insertions(+), 81 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c
> b/drivers/gpu/drm/i915/gt/intel_mocs.c
> index e6f3f36a3988..63d0fdf67215 100644
> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> @@ -329,27 +329,6 @@ static bool get_mocs_settings(const struct
> drm_i915_private *i915,
> return true;
> }
>
> -static i915_reg_t mocs_register(const struct intel_engine_cs *engine, int
> index)
> -{
> - switch (engine->id) {
> - case RCS0:
> - return GEN9_GFX_MOCS(index);
> - case VCS0:
> - return GEN9_MFX0_MOCS(index);
> - case BCS0:
> - return GEN9_BLT_MOCS(index);
> - case VECS0:
> - return GEN9_VEBOX_MOCS(index);
> - case VCS1:
> - return GEN9_MFX1_MOCS(index);
> - case VCS2:
> - return GEN11_MFX2_MOCS(index);
> - default:
> - MISSING_CASE(engine->id);
> - return INVALID_MMIO_REG;
> - }
> -}
> -
> /*
> * Get control_value from MOCS entry taking into account when it's not used:
> * I915_MOCS_PTE's value is returned in this case.
> @@ -357,29 +336,47 @@ static i915_reg_t mocs_register(const struct
> intel_engine_cs *engine, int index)
> static u32 get_entry_control(const struct drm_i915_mocs_table *table,
> unsigned int index)
> {
> - if (table->table[index].used)
> + if (index < table->size && table->table[index].used)
> return table->table[index].control_value;
>
> return table->table[I915_MOCS_PTE].control_value;
> }
>
> -static void init_mocs_table(struct intel_engine_cs *engine,
> - const struct drm_i915_mocs_table *table)
> +#define for_each_mocs(mocs, t, i) \
> + for (i = 0; \
> + i < t->n_entries ? (mocs = get_entry_control(t, i)), 1 : 0;\
> + i++)
checkpatch might complain about some param reusage.
> +
> +static void __init_mocs_table(struct intel_uncore *uncore,
> + const struct drm_i915_mocs_table *table,
> + u32 addr)
> {
> - struct intel_uncore *uncore = engine->uncore;
> - u32 unused_value = table->table[I915_MOCS_PTE].control_value;
> unsigned int i;
> + u32 mocs;
> +
> + for_each_mocs(mocs, table, i)
> + intel_uncore_write_fw(uncore, _MMIO(addr + i * 4), mocs);
> +}
>
> - for (i = 0; i < table->size; i++)
> - intel_uncore_write_fw(uncore,
> - mocs_register(engine, i),
> - get_entry_control(table, i));
> +static u32 mocs_register(const struct intel_engine_cs *engine)
> +{
> + static const u32 offset[] = {
> + [RCS0] = 0x0c800,
> + [VCS0] = 0x0c900,
> + [VCS1] = 0x0ca00,
> + [VECS0] = 0x0cb00,
> + [BCS0] = 0x0cc00,
> + [VCS2] = 0x10000,
Someone might complain about the lack of defines. Remove
the unused defines or change them to work without index.
> + };
> +
> + GEM_BUG_ON(engine->id > ARRAY_SIZE(offset));
engine->id >= ARRAY_SIZE(offset);
Also the comments throughout the file need massaging
after this change.
These 3 mocs patches looks good otherwise.
Please update resend as a separate series.
-Mika
> + return offset[engine->id];
> +}
>
> - /* All remaining entries are unused */
> - for (; i < table->n_entries; i++)
> - intel_uncore_write_fw(uncore,
> - mocs_register(engine, i),
> - unused_value);
> +static void init_mocs_table(struct intel_engine_cs *engine,
> + const struct drm_i915_mocs_table *table)
> +{
> + __init_mocs_table(engine->uncore, table, mocs_register(engine));
> }
>
> /*
> @@ -389,7 +386,7 @@ static void init_mocs_table(struct intel_engine_cs
> *engine,
> static u16 get_entry_l3cc(const struct drm_i915_mocs_table *table,
> unsigned int index)
> {
> - if (table->table[index].used)
> + if (index < table->size && table->table[index].used)
> return table->table[index].l3cc_value;
>
> return table->table[I915_MOCS_PTE].l3cc_value;
> @@ -400,37 +397,23 @@ static inline u32 l3cc_combine(u16 low, u16 high)
> return low | (u32)high << 16;
> }
>
> +#define for_each_l3cc(l3cc, t, i) \
> + for (i = 0; \
> + i < (t->n_entries + 1) / 2 ? \
> + (l3cc = l3cc_combine(get_entry_l3cc(t, 2 * i), \
> + get_entry_l3cc(t, 2 * i + 1))), 1 : \
> + 0; \
> + i++)
> +
> static void init_l3cc_table(struct intel_engine_cs *engine,
> const struct drm_i915_mocs_table *table)
> {
> struct intel_uncore *uncore = engine->uncore;
> - u16 unused_value = table->table[I915_MOCS_PTE].l3cc_value;
> unsigned int i;
> + u32 l3cc;
>
> - for (i = 0; i < table->size / 2; i++) {
> - u16 low = get_entry_l3cc(table, 2 * i);
> - u16 high = get_entry_l3cc(table, 2 * i + 1);
> -
> - intel_uncore_write(uncore,
> - GEN9_LNCFCMOCS(i),
> - l3cc_combine(low, high));
> - }
> -
> - /* Odd table size - 1 left over */
> - if (table->size & 1) {
> - u16 low = get_entry_l3cc(table, 2 * i);
> -
> - intel_uncore_write(uncore,
> - GEN9_LNCFCMOCS(i),
> - l3cc_combine(low, unused_value));
> - i++;
> - }
> -
> - /* All remaining entries are also unused */
> - for (; i < table->n_entries / 2; i++)
> - intel_uncore_write(uncore,
> - GEN9_LNCFCMOCS(i),
> - l3cc_combine(unused_value, unused_value));
> + for_each_l3cc(l3cc, table, i)
> + intel_uncore_write_fw(uncore, GEN9_LNCFCMOCS(i), l3cc);
> }
>
> void intel_mocs_init_engine(struct intel_engine_cs *engine)
> @@ -451,11 +434,9 @@ void intel_mocs_init_engine(struct intel_engine_cs
> *engine)
> init_l3cc_table(engine, &table);
> }
>
> -static void intel_mocs_init_global(struct intel_gt *gt)
> +static void init_global_mocs(struct intel_gt *gt)
> {
> - struct intel_uncore *uncore = gt->uncore;
> struct drm_i915_mocs_table table;
> - unsigned int index;
>
> /*
> * LLC and eDRAM control values are not applicable to dgfx
> @@ -463,29 +444,14 @@ static void intel_mocs_init_global(struct intel_gt *gt)
> if (IS_DGFX(gt->i915))
> return;
>
> - GEM_BUG_ON(!HAS_GLOBAL_MOCS_REGISTERS(gt->i915));
> -
> if (!get_mocs_settings(gt->i915, &table))
> return;
>
> - for (index = 0; index < table.size; index++)
> - intel_uncore_write(uncore,
> - GEN12_GLOBAL_MOCS(index),
> - table.table[index].control_value);
> -
> - /*
> - * Ok, now set the unused entries to the invalid entry (index 0). These
> - * entries are officially undefined and no contract for the contents and
> - * settings is given for these entries.
> - */
> - for (; index < table.n_entries; index++)
> - intel_uncore_write(uncore,
> - GEN12_GLOBAL_MOCS(index),
> - table.table[I915_MOCS_PTE].control_value);
> + __init_mocs_table(gt->uncore, &table, 0x4000);
> }
>
> void intel_mocs_init(struct intel_gt *gt)
> {
> if (HAS_GLOBAL_MOCS_REGISTERS(gt->i915))
> - intel_mocs_init_global(gt);
> + init_global_mocs(gt);
> }
> --
> 2.24.0
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
