From: Jani Nikula <jani.nikula@linux.intel.com>
To: "José Roberto de Souza" <jose.souza@intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds
Date: Thu, 27 Feb 2020 08:23:47 +0200 [thread overview]
Message-ID: <871rqgsg3w.fsf@intel.com> (raw)
In-Reply-To: <20200226014603.42190-1-jose.souza@intel.com>
On Tue, 25 Feb 2020, José Roberto de Souza <jose.souza@intel.com> 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@intel.com>
> Cc: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> .../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
> + };
> + 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)
> +
IOW, you could just define display steppings in terms of the revids, and
would not have to introduce two separate revid test macros?
#define TGL_DISPLAY_REVID_A0 TGL_REVID_A0
#define TGL_DISPLAY_REVID_B0 TGL_REVID_A0
#define TGL_DISPLAY_REVID_C0 TGL_REVID_B0
#define TGL_DISPLAY_REVID_D0 TGL_REVID_C0
And then you could simply use:
if (IS_TGL_REVID(i915, TGL_DISPLAY_REVID_A0, TGL_DISPLAY_REVID_B0))
At this point, I don't think there's any reason at all to add the extra
indirection and function, and two separate revid check macros. Because
ultimately it all depends on a single revid, not two independent revids
for GT and display.
BR,
Jani.
> #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);
> }
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
prev parent reply other threads:[~2020-02-27 6:23 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-26 1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
2020-02-26 1:45 ` [Intel-gfx] [PATCH v2 02/14] drm/i915/tgl: Extend Wa_1409825376 stepping José Roberto de Souza
2020-02-26 23:43 ` Radhakrishna Sripada
2020-02-26 1:45 ` [Intel-gfx] [PATCH v2 03/14] drm/i915/tgl: Implement Wa_1409804808 José Roberto de Souza
2020-02-26 1:45 ` [Intel-gfx] [PATCH v2 04/14] drm/i915/tgl: Implement Wa_1806527549 José Roberto de Souza
2020-02-26 1:45 ` [Intel-gfx] [PATCH v2 05/14] drm/i915/tgl: Add Wa_1409085225, Wa_14010229206 José Roberto de Souza
2020-02-26 1:45 ` [Intel-gfx] [PATCH v2 06/14] drm/i915/tgl: Extend Wa_1606931601 for all steppings José Roberto de Souza
2020-02-26 1:45 ` [Intel-gfx] [PATCH v2 07/14] drm/i915/tgl: Add note to Wa_1607297627 José Roberto de Souza
2020-02-26 1:45 ` [Intel-gfx] [PATCH v2 08/14] drm/i915/tgl: Add note about Wa_1607063988 José Roberto de Souza
2020-02-26 1:45 ` [Intel-gfx] [PATCH v2 09/14] drm/i915/tgl: Extend Wa_1409767108 to B0 José Roberto de Souza
2020-02-26 1:45 ` [Intel-gfx] [PATCH v2 10/14] drm/i915/tgl: Fix the Wa number of a fix José Roberto de Souza
2020-02-26 1:46 ` [Intel-gfx] [PATCH v2 11/14] drm/i915/tgl: Add note about Wa_1409142259 José Roberto de Souza
2020-02-26 1:46 ` [Intel-gfx] [PATCH v2 12/14] drm/i915/tgl: Restrict Wa_1408615072 to GT A0 stepping José Roberto de Souza
2020-02-26 1:46 ` [Intel-gfx] [PATCH v2 13/14] drm/i915/tgl: Add Wa number to WaAllowPMDepthAndInvocationCountAccessFromUMD José Roberto de Souza
2020-02-26 1:46 ` [Intel-gfx] [PATCH v2 14/14] drm/i915/tgl: Implement Wa_1407901919 José Roberto de Souza
2020-02-26 14:57 ` Chris Wilson
2020-02-26 15:06 ` [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds Tvrtko Ursulin
2020-02-26 18:42 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,01/14] " Patchwork
2020-02-26 19:08 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-02-27 0:02 ` [Intel-gfx] [PATCH v2 01/14] " Lucas De Marchi
2020-02-27 0:42 ` Souza, Jose
2020-02-27 6:19 ` Lucas De Marchi
2020-02-27 6:31 ` Jani Nikula
2020-02-27 6:23 ` Jani Nikula [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=871rqgsg3w.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jose.souza@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.