diff mbox series

[v2,8/9] drm/i915/display: Eliminate IS_METEORLAKE checks

Message ID 20230725001312.1907319-19-matthew.d.roper@intel.com (mailing list archive)
State New, archived
Headers show
Series Reduce MTL-specific platform checks | expand

Commit Message

Matt Roper July 25, 2023, 12:13 a.m. UTC 
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.

v2:
 - cdclk check should be >=, not >.  (Gustavo)

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Gustavo Sousa <gustavo.sousa@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(-)

Comments

Lucas De Marchi July 27, 2023, 6:38 p.m. UTC | #1 
On Mon, Jul 24, 2023 at 05:13:21PM -0700, Matt Roper wrote:
>diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c

for the phy checks what I have locally is:

>index 1b00ef2c6185..a42b3c4c0ed7 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)

	if (DISPLAY_VER(i915) >= 14 && !IS_DGFX(i915) && (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))

	else if (DISPLAY_VER(dev_priv) >= 13 && !IS_DGFX(dev_priv))

> 		return phy >= PHY_F && phy <= PHY_I;
> 	else if (IS_TIGERLAKE(dev_priv))
> 		return phy >= PHY_D && phy <= PHY_I;


would that be better or do you have something else in mind?

Lucas De Marchi
Matt Roper July 27, 2023, 7:05 p.m. UTC | #2 
On Thu, Jul 27, 2023 at 03:38:46PM -0300, Lucas De Marchi wrote:
> On Mon, Jul 24, 2023 at 05:13:21PM -0700, Matt Roper wrote:
> > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> 
> for the phy checks what I have locally is:
> 
> > index 1b00ef2c6185..a42b3c4c0ed7 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)
> 
> 	if (DISPLAY_VER(i915) >= 14 && !IS_DGFX(i915) && (phy < PHY_C))

Why the !IS_DGFX check?  We can assume the PHYs will remain the same for
future platforms in general (at least until a platform shows up and
requires a change), but I don't think there's any reason yet to assume
that only igpu's will inherit the behavior and dgpu's won't.

> 
> > 		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))
> 
> 	else if (DISPLAY_VER(dev_priv) >= 13 && !IS_DGFX(dev_priv))

Same here.  This is probably fine if we switch the IS_DGFX to IS_DG2 to
exclude the single existing platform that we know doesn't follow this,
but I don't think we should assume the same will be true on future
dgpus.


Matt

> 
> > 		return phy >= PHY_F && phy <= PHY_I;
> > 	else if (IS_TIGERLAKE(dev_priv))
> > 		return phy >= PHY_D && phy <= PHY_I;
> 
> 
> would that be better or do you have something else in mind?
> 
> Lucas De Marchi
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index dcc1f6941b60..5ec39cc15332 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..a42b3c4c0ed7 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)) {