| Message ID | 20230718222753.1075713-17-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Reduce MTL-specific platform checks | expand |
Quoting Matt Roper (2023-07-18 19:28:01-03:00) >Most of the IS_METEORLAKE checks in the display code shouldn't actually >be tied to MTL as a platform, but rather to the Xe_LPD+ display IP >(which is used in MTL, but may show up again in future platforms). In >cases where we're trying to match that specific IP, use a version check >against IP_VER(14, 0). For cases where we're just handling new behavior >introduced by this IP (but which may also be inherited by future IP as >well), use a ver >= 14 check. > >The one exception here is the stolen memory workaround Wa_13010847436 >(which is mislabelled as "Wa_22018444074" in the code). That's truly a >MTL-specific issue rather than being tied to any of the IP blocks, so >leaving the condition as IS_METEORLAKE is correct there. I grepped and also saw usage for IS_METEORLAKE() for RC6-related code and around clock gating as well. I'm yet not familiar with those to be able to tell if they are platform-specific or rather specific to sub IP(s). Just thought it would be worth noting here, just in case. > >Signed-off-by: Matt Roper <matthew.d.roper@intel.com> >--- > drivers/gpu/drm/i915/display/intel_cdclk.c | 4 ++-- > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 2 +- > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > drivers/gpu/drm/i915/display/intel_dmc.c | 2 +- > 4 files changed, 5 insertions(+), 5 deletions(-) > >diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c >index dcc1f6941b60..4cb1dc397b62 100644 >--- a/drivers/gpu/drm/i915/display/intel_cdclk.c >+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c >@@ -1840,7 +1840,7 @@ static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i91 > > static bool pll_enable_wa_needed(struct drm_i915_private *dev_priv) > { >- return ((IS_DG2(dev_priv) || IS_METEORLAKE(dev_priv)) && >+ return ((IS_DG2(dev_priv) || DISPLAY_VER_FULL(dev_priv) == IP_VER(14, 0)) && > dev_priv->display.cdclk.hw.vco > 0 && > HAS_CDCLK_SQUASH(dev_priv)); > } >@@ -3559,7 +3559,7 @@ static const struct intel_cdclk_funcs i830_cdclk_funcs = { > */ > void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) > { >- if (IS_METEORLAKE(dev_priv)) { >+ if (DISPLAY_VER(dev_priv) > 14) { I think you missed the equality part here, should be DISPLAY_VER(dev_priv) >= 14, right? -- Gustavo Sousa > dev_priv->display.funcs.cdclk = &mtl_cdclk_funcs; > dev_priv->display.cdclk.table = mtl_cdclk_table; > } else if (IS_DG2(dev_priv)) { >diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c >index 1b00ef2c6185..025c80b9fece 100644 >--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c >+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c >@@ -31,7 +31,7 @@ > > bool intel_is_c10phy(struct drm_i915_private *i915, enum phy phy) > { >- if (IS_METEORLAKE(i915) && (phy < PHY_C)) >+ if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && (phy < PHY_C)) > return true; > > return false; >diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >index 43cba98f7753..85efd77f491b 100644 >--- a/drivers/gpu/drm/i915/display/intel_display.c >+++ b/drivers/gpu/drm/i915/display/intel_display.c >@@ -1767,7 +1767,7 @@ bool intel_phy_is_tc(struct drm_i915_private *dev_priv, enum phy phy) > if (IS_DG2(dev_priv)) > /* DG2's "TC1" output uses a SNPS PHY */ > return false; >- else if (IS_ALDERLAKE_P(dev_priv) || IS_METEORLAKE(dev_priv)) >+ else if (IS_ALDERLAKE_P(dev_priv) || DISPLAY_VER_FULL(dev_priv) == IP_VER(14, 0)) > return phy >= PHY_F && phy <= PHY_I; > else if (IS_TIGERLAKE(dev_priv)) > return phy >= PHY_D && phy <= PHY_I; >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c >index 5f479f3828bb..1623c0c5e8a1 100644 >--- a/drivers/gpu/drm/i915/display/intel_dmc.c >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c >@@ -998,7 +998,7 @@ void intel_dmc_init(struct drm_i915_private *i915) > > INIT_WORK(&dmc->work, dmc_load_work_fn); > >- if (IS_METEORLAKE(i915)) { >+ if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0)) { > dmc->fw_path = MTL_DMC_PATH; > dmc->max_fw_size = XELPDP_DMC_MAX_FW_SIZE; > } else if (IS_DG2(i915)) { >-- >2.41.0 >
Quoting Gustavo Sousa (2023-07-19 16:39:25-03:00) >Quoting Matt Roper (2023-07-18 19:28:01-03:00) >>Most of the IS_METEORLAKE checks in the display code shouldn't actually >>be tied to MTL as a platform, but rather to the Xe_LPD+ display IP >>(which is used in MTL, but may show up again in future platforms). In >>cases where we're trying to match that specific IP, use a version check >>against IP_VER(14, 0). For cases where we're just handling new behavior >>introduced by this IP (but which may also be inherited by future IP as >>well), use a ver >= 14 check. >> >>The one exception here is the stolen memory workaround Wa_13010847436 >>(which is mislabelled as "Wa_22018444074" in the code). That's truly a >>MTL-specific issue rather than being tied to any of the IP blocks, so >>leaving the condition as IS_METEORLAKE is correct there. > >I grepped and also saw usage for IS_METEORLAKE() for RC6-related code >and around clock gating as well. I'm yet not familiar with those to be >able to tell if they are platform-specific or rather specific to sub >IP(s). Just thought it would be worth noting here, just in case. Oops. I was too quick on sending this without realizing that this patch is specific to display... > >> >>Signed-off-by: Matt Roper <matthew.d.roper@intel.com> >>--- >> drivers/gpu/drm/i915/display/intel_cdclk.c | 4 ++-- >> drivers/gpu/drm/i915/display/intel_cx0_phy.c | 2 +- >> drivers/gpu/drm/i915/display/intel_display.c | 2 +- >> drivers/gpu/drm/i915/display/intel_dmc.c | 2 +- >> 4 files changed, 5 insertions(+), 5 deletions(-) >> >>diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c >>index dcc1f6941b60..4cb1dc397b62 100644 >>--- a/drivers/gpu/drm/i915/display/intel_cdclk.c >>+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c >>@@ -1840,7 +1840,7 @@ static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i91 >> >> static bool pll_enable_wa_needed(struct drm_i915_private *dev_priv) >> { >>- return ((IS_DG2(dev_priv) || IS_METEORLAKE(dev_priv)) && >>+ return ((IS_DG2(dev_priv) || DISPLAY_VER_FULL(dev_priv) == IP_VER(14, 0)) && >> dev_priv->display.cdclk.hw.vco > 0 && >> HAS_CDCLK_SQUASH(dev_priv)); >> } >>@@ -3559,7 +3559,7 @@ static const struct intel_cdclk_funcs i830_cdclk_funcs = { >> */ >> void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) >> { >>- if (IS_METEORLAKE(dev_priv)) { >>+ if (DISPLAY_VER(dev_priv) > 14) { > >I think you missed the equality part here, should be DISPLAY_VER(dev_priv) >= >14, right? ...so, with this fix, Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> > >-- >Gustavo Sousa > >> dev_priv->display.funcs.cdclk = &mtl_cdclk_funcs; >> dev_priv->display.cdclk.table = mtl_cdclk_table; >> } else if (IS_DG2(dev_priv)) { >>diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c >>index 1b00ef2c6185..025c80b9fece 100644 >>--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c >>+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c >>@@ -31,7 +31,7 @@ >> >> bool intel_is_c10phy(struct drm_i915_private *i915, enum phy phy) >> { >>- if (IS_METEORLAKE(i915) && (phy < PHY_C)) >>+ if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && (phy < PHY_C)) >> return true; >> >> return false; >>diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >>index 43cba98f7753..85efd77f491b 100644 >>--- a/drivers/gpu/drm/i915/display/intel_display.c >>+++ b/drivers/gpu/drm/i915/display/intel_display.c >>@@ -1767,7 +1767,7 @@ bool intel_phy_is_tc(struct drm_i915_private *dev_priv, enum phy phy) >> if (IS_DG2(dev_priv)) >> /* DG2's "TC1" output uses a SNPS PHY */ >> return false; >>- else if (IS_ALDERLAKE_P(dev_priv) || IS_METEORLAKE(dev_priv)) >>+ else if (IS_ALDERLAKE_P(dev_priv) || DISPLAY_VER_FULL(dev_priv) == IP_VER(14, 0)) >> return phy >= PHY_F && phy <= PHY_I; >> else if (IS_TIGERLAKE(dev_priv)) >> return phy >= PHY_D && phy <= PHY_I; >>diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c >>index 5f479f3828bb..1623c0c5e8a1 100644 >>--- a/drivers/gpu/drm/i915/display/intel_dmc.c >>+++ b/drivers/gpu/drm/i915/display/intel_dmc.c >>@@ -998,7 +998,7 @@ void intel_dmc_init(struct drm_i915_private *i915) >> >> INIT_WORK(&dmc->work, dmc_load_work_fn); >> >>- if (IS_METEORLAKE(i915)) { >>+ if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0)) { >> dmc->fw_path = MTL_DMC_PATH; >> dmc->max_fw_size = XELPDP_DMC_MAX_FW_SIZE; >> } else if (IS_DG2(i915)) { >>-- >>2.41.0 >>
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index dcc1f6941b60..4cb1dc397b62 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1840,7 +1840,7 @@ static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i91 static bool pll_enable_wa_needed(struct drm_i915_private *dev_priv) { - return ((IS_DG2(dev_priv) || IS_METEORLAKE(dev_priv)) && + return ((IS_DG2(dev_priv) || DISPLAY_VER_FULL(dev_priv) == IP_VER(14, 0)) && dev_priv->display.cdclk.hw.vco > 0 && HAS_CDCLK_SQUASH(dev_priv)); } @@ -3559,7 +3559,7 @@ static const struct intel_cdclk_funcs i830_cdclk_funcs = { */ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) { - if (IS_METEORLAKE(dev_priv)) { + if (DISPLAY_VER(dev_priv) > 14) { dev_priv->display.funcs.cdclk = &mtl_cdclk_funcs; dev_priv->display.cdclk.table = mtl_cdclk_table; } else if (IS_DG2(dev_priv)) { diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c index 1b00ef2c6185..025c80b9fece 100644 --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c @@ -31,7 +31,7 @@ bool intel_is_c10phy(struct drm_i915_private *i915, enum phy phy) { - if (IS_METEORLAKE(i915) && (phy < PHY_C)) + if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && (phy < PHY_C)) return true; return false; diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 43cba98f7753..85efd77f491b 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -1767,7 +1767,7 @@ bool intel_phy_is_tc(struct drm_i915_private *dev_priv, enum phy phy) if (IS_DG2(dev_priv)) /* DG2's "TC1" output uses a SNPS PHY */ return false; - else if (IS_ALDERLAKE_P(dev_priv) || IS_METEORLAKE(dev_priv)) + else if (IS_ALDERLAKE_P(dev_priv) || DISPLAY_VER_FULL(dev_priv) == IP_VER(14, 0)) return phy >= PHY_F && phy <= PHY_I; else if (IS_TIGERLAKE(dev_priv)) return phy >= PHY_D && phy <= PHY_I; diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c index 5f479f3828bb..1623c0c5e8a1 100644 --- a/drivers/gpu/drm/i915/display/intel_dmc.c +++ b/drivers/gpu/drm/i915/display/intel_dmc.c @@ -998,7 +998,7 @@ void intel_dmc_init(struct drm_i915_private *i915) INIT_WORK(&dmc->work, dmc_load_work_fn); - if (IS_METEORLAKE(i915)) { + if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0)) { dmc->fw_path = MTL_DMC_PATH; dmc->max_fw_size = XELPDP_DMC_MAX_FW_SIZE; } else if (IS_DG2(i915)) {
Most of the IS_METEORLAKE checks in the display code shouldn't actually be tied to MTL as a platform, but rather to the Xe_LPD+ display IP (which is used in MTL, but may show up again in future platforms). In cases where we're trying to match that specific IP, use a version check against IP_VER(14, 0). For cases where we're just handling new behavior introduced by this IP (but which may also be inherited by future IP as well), use a ver >= 14 check. The one exception here is the stolen memory workaround Wa_13010847436 (which is mislabelled as "Wa_22018444074" in the code). That's truly a MTL-specific issue rather than being tied to any of the IP blocks, so leaving the condition as IS_METEORLAKE is correct there. Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/display/intel_cdclk.c | 4 ++-- drivers/gpu/drm/i915/display/intel_cx0_phy.c | 2 +- drivers/gpu/drm/i915/display/intel_display.c | 2 +- drivers/gpu/drm/i915/display/intel_dmc.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-)