On 10/12/2017 03:35 AM, Joonas Lahtinen wrote:
On Wed, 2017-10-11 at 11:15 -0700, Oscar Mateo wrote:
By doing this, we can dump these workarounds in debugfs for
validation (which,
at the moment, we are only able to do for the contexts WAs).
v2:
- Wrong macro used for MMIO set bit masked
- Improved naming
- Rebased
Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
<SNIP>
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1960,12 +1960,16 @@ struct i915_wa_reg {
u32 mask;
};
-#define I915_MAX_WA_REGS 16
+#define I915_MAX_CTX_WA_REGS 16
+#define I915_MAX_MMIO_WA_REGS 32
struct i915_workarounds {
- struct i915_wa_reg ctx_wa_reg[I915_MAX_WA_REGS];
+ struct i915_wa_reg ctx_wa_reg[I915_MAX_CTX_WA_REGS];
u32 ctx_wa_count;
+ struct i915_wa_reg mmio_wa_reg[I915_MAX_MMIO_WA_REGS];
+ u32 mmio_wa_count;
+
u32 hw_whitelist_count[I915_NUM_ENGINES];
};
Could we instead consider a constant structure with platform bitmasks?
If that's not dynamic enough, then a bitmap which is initialized by the
platform bitmasks as a default. So instead of running code to add to
list, make it bit more declarative. Pseudo-code;
struct i915_workaround {
u16 gen_mask;
enum {
I915_WA_CTX,
I915_WA_MMIO,
I915_WA_WHITELIST,
} type;
u32 reg;
}
... elsewhere in .c file
static const struct i915_workaround i915_workarounds[] = { {
/* WaSomethingSomewhereUMDFoo:skl */
.gen_mask = INTEL_GEN_MASK(X, Y),
.type = I915_WA_CTX,
.reg = ...
} };
Regards, Joonas
I considered it, but we have some workarounds that are even more dynamic
than that. E.g.:
* WaCompressedResourceSamplerPbeMediaNewHashMode depends on
HAS_LLC(dev_priv)
* skl_tune_iz_hashing depends on number of subslices (although maybe
this is not technically a WA?)
* WaGttCachingOffByDefault needs HAS_PAGE_SIZES(dev_priv,
I915_GTT_PAGE_SIZE_2M)
* WaPsrDPRSUnmaskVBlankInSRD is applied for_each_pipe
* Wa #1181 needs HAS_PCH_CNP(dev_priv)
* ...
We even have a WA (WaTempDisableDOPClkGating) where the new design is
not dynamic enough :(
I guess we could add a callback pointer to the table for those WAs that
need extra information. Maybe even a "pre" and a "post" callback
pointers (to cover that pesky WaTempDisableDOPClkGating).
If this is where we want to go, I can write a patch, but I believe it
would be better to land this first (code review is critical for these
kind of changes, and it is easier to first review that all WAs make it
to i915_workarounds.c correctly, and then review that they are all
transformed to a static table correctly).
-- Oscar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
