On Tue, Feb 25, 2020 at 5:45 PM José Roberto de Souza
<jose.souza@xxxxxxxxx> wrote:
>
> Spliting GT and display revisions id to correctly apply workarounds
> because we have some issues that were fixed in display B0 but no
> hardware was made with B0 stepping, so to keep consistent with BSpec
> splitting it from GT and adding this adittional handling.
>
> BSpec: 52890
> BSpec: 44455
> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> Cc: James Ausmus <james.ausmus@xxxxxxxxx>
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> ---
> .../drm/i915/display/intel_display_power.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 10 +++---
> drivers/gpu/drm/i915/i915_drv.h | 36 +++++++++++++++++--
> drivers/gpu/drm/i915/intel_pm.c | 2 +-
> 5 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 6e25a1317161..82af963106ab 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -5010,7 +5010,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
> const struct buddy_page_mask *table;
> int i;
>
> - if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> + if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
> /* Wa_1409767108: tgl */
> table = wa_1409767108_buddy_page_masks;
> else
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 39b0125b7143..852981d533a8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -4124,7 +4124,7 @@ static int gen12_emit_flush_render(struct i915_request *request,
> /*
> * Wa_1604544889:tgl
> */
> - if (IS_TGL_REVID(request->i915, TGL_REVID_A0, TGL_REVID_A0)) {
> + if (IS_TGL_GT_REVID(request->i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0)) {
> flags = 0;
> flags |= PIPE_CONTROL_CS_STALL;
> flags |= PIPE_CONTROL_HDC_PIPELINE_FLUSH;
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 887e0dc701f7..bc6114b6dc8f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -596,8 +596,8 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
> * the read of FF_MODE2.
> */
> wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
> - IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ? 0 :
> - FF_MODE2_TDS_TIMER_MASK);
> + IS_TGL_GT_REVID(engine->i915, TGL_GT_REVID_A0,
> + TGL_GT_REVID_A0) ? 0 : FF_MODE2_TDS_TIMER_MASK);
> }
>
> static void
> @@ -931,13 +931,13 @@ static void
> tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
> {
> /* Wa_1409420604:tgl */
> - if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> + if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
> wa_write_or(wal,
> SUBSLICE_UNIT_LEVEL_CLKGATE2,
> CPSSUNIT_CLKGATE_DIS);
>
> /* Wa_1409180338:tgl */
> - if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> + if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
> wa_write_or(wal,
> SLICE_UNIT_LEVEL_CLKGATE,
> L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
> @@ -1329,7 +1329,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> {
> struct drm_i915_private *i915 = engine->i915;
>
> - if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
> + if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0)) {
> /* Wa_1606700617:tgl */
> wa_masked_en(wal,
> GEN9_CS_DEBUG_MODE1,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ea13fc0b409b..4fa01f8d3f33 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1574,11 +1574,43 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> #define IS_ICL_REVID(p, since, until) \
> (IS_ICELAKE(p) && IS_REVID(p, since, until))
>
> -#define TGL_REVID_A0 0x0
> +#define TGL_GT_REVID_A0 0x0
>
> -#define IS_TGL_REVID(p, since, until) \
> +#define IS_TGL_GT_REVID(p, since, until) \
> (IS_TIGERLAKE(p) && IS_REVID(p, since, until))
>
> +/*
> + * revid=0x0 = DISP_REVID_A0
> + * revid=0x1 = DISP_REVID_C0
> + * revid=0x2 = DISP_REVID_D0
> + *
> + * So ids bellow will not match PCI revid and the function bellow is used.
> + */
> +#define TGL_DISP_REVID_A0 0x0
> +#define TGL_DISP_REVID_B0 0x1
> +#define TGL_DISP_REVID_C0 0x2
> +#define TGL_DISP_REVID_D0 0x3
> +
> +static inline bool
> +_tgl_disp_revid(struct drm_i915_private *p, u8 since, u8 until)
> +{
> + const u8 gt2_disp_revid[] = {
> + TGL_DISP_REVID_A0,
> + TGL_DISP_REVID_C0,
> + TGL_DISP_REVID_D0
So we are hardcoding the mapping in the code using the same
information. Why do we even need any of this?
Just define it like this:
#define TGL_DISP_REVID_A0 0x0
#define TGL_DISP_REVID_C0 0x1
#define TGL_DISP_REVID_D0 0x2
Then in the workarounds we continue to use IS_TGL_REVID(). This means
we document it is a "display wa",
but since we don't have a INTEL_DISPLAY_REVID(), IMO it doesn't make
sense to have all this function to do
the mapping.... just make it a compile-time map.
Lucas De Marchi
> + };
> + u8 disp_revid;
> +
> + if (INTEL_REVID(p) >= ARRAY_SIZE(gt2_disp_revid))
> + disp_revid = TGL_DISP_REVID_D0;
> + else
> + disp_revid = gt2_disp_revid[INTEL_REVID(p)];
> +
> + return IS_TIGERLAKE(p) && disp_revid >= since && disp_revid <= until;
> +}
> +
> +#define IS_TGL_DISP_REVID(p, since, until) _tgl_disp_revid(p, since, until)
> +
> #define IS_LP(dev_priv) (INTEL_INFO(dev_priv)->is_lp)
> #define IS_GEN9_LP(dev_priv) (IS_GEN(dev_priv, 9) && IS_LP(dev_priv))
> #define IS_GEN9_BC(dev_priv) (IS_GEN(dev_priv, 9) && !IS_LP(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 22aa205793e5..49484d5f5f84 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6852,7 +6852,7 @@ static void tgl_init_clock_gating(struct drm_i915_private *dev_priv)
> I915_READ(POWERGATE_ENABLE) | vd_pg_enable);
>
> /* Wa_1409825376:tgl (pre-prod)*/
> - if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> + if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
> I915_WRITE(GEN9_CLKGATE_DIS_3, I915_READ(GEN9_CLKGATE_DIS_3) |
> TGL_VRH_GATING_DIS);
> }
> --
> 2.25.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
