| Message ID | 20220718191212.166141-1-anusha.srivatsa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/display: Cleanup intel_phy_is_combo() | expand |
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of > Anusha Srivatsa > Sent: Tuesday, July 19, 2022 12:42 AM > To: intel-gfx@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH] drm/i915/display: Cleanup intel_phy_is_combo() > > No functional change. Cleanup the intel_phy_is_combo to accomodate for > cases where combo phy is not available. > > Cc: Matt Roper <matthew.d.roper@intel.com> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index a0f84cbe974f..b69208cf9a5e 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2082,20 +2082,13 @@ bool intel_phy_is_combo(struct > drm_i915_private *dev_priv, enum phy phy) { > if (phy == PHY_NONE) > return false; > - else if (IS_DG2(dev_priv)) > - /* > - * DG2 outputs labelled as "combo PHY" in the bspec use > - * SNPS PHYs with completely different programming, > - * hence we always return false here. > - */ > - return false; I feel it would be good to retain this. This is very well commented. In future upon adding something like DISPLAY_VER(dev_priv) >= 11, like the one done below can create confusion. > else if (IS_ALDERLAKE_S(dev_priv)) > return phy <= PHY_E; > else if (IS_DG1(dev_priv) || IS_ROCKETLAKE(dev_priv)) > return phy <= PHY_D; > else if (IS_JSL_EHL(dev_priv)) > return phy <= PHY_C; > - else if (DISPLAY_VER(dev_priv) >= 11) > + else if (IS_ALDERLAKE_P(dev_priv) || IS_DISPLAY_VER(dev_priv, 11, > 12)) > return phy <= PHY_B; > else > return false; > -- > 2.25.1 Thanks and Regards, Arun R Murthy --------------------
> -----Original Message----- > From: Murthy, Arun R <arun.r.murthy@intel.com> > Sent: Monday, July 18, 2022 8:32 PM > To: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel- > gfx@lists.freedesktop.org > Subject: RE: [Intel-gfx] [PATCH] drm/i915/display: Cleanup > intel_phy_is_combo() > > > -----Original Message----- > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of > > Anusha Srivatsa > > Sent: Tuesday, July 19, 2022 12:42 AM > > To: intel-gfx@lists.freedesktop.org > > Subject: [Intel-gfx] [PATCH] drm/i915/display: Cleanup > > intel_phy_is_combo() > > > > No functional change. Cleanup the intel_phy_is_combo to accomodate for > > cases where combo phy is not available. > > > > Cc: Matt Roper <matthew.d.roper@intel.com> > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 9 +-------- > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index a0f84cbe974f..b69208cf9a5e 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -2082,20 +2082,13 @@ bool intel_phy_is_combo(struct > > drm_i915_private *dev_priv, enum phy phy) { > > if (phy == PHY_NONE) > > return false; > > - else if (IS_DG2(dev_priv)) > > - /* > > - * DG2 outputs labelled as "combo PHY" in the bspec use > > - * SNPS PHYs with completely different programming, > > - * hence we always return false here. > > - */ > > - return false; > I feel it would be good to retain this. This is very well commented. In future > upon adding something like DISPLAY_VER(dev_priv) >= 11, like the one done > below can create confusion. What if I retain the comments with the code change? Anusha > > > else if (IS_ALDERLAKE_S(dev_priv)) > > return phy <= PHY_E; > > else if (IS_DG1(dev_priv) || IS_ROCKETLAKE(dev_priv)) > > return phy <= PHY_D; > > else if (IS_JSL_EHL(dev_priv)) > > return phy <= PHY_C; > > - else if (DISPLAY_VER(dev_priv) >= 11) > > + else if (IS_ALDERLAKE_P(dev_priv) || IS_DISPLAY_VER(dev_priv, 11, > > 12)) > > return phy <= PHY_B; > > else > > return false; > > -- > > 2.25.1 > > Thanks and Regards, > Arun R Murthy > --------------------
> > -----Original Message----- > > From: Murthy, Arun R <arun.r.murthy@intel.com> > > Sent: Monday, July 18, 2022 8:32 PM > > To: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel- > > gfx@lists.freedesktop.org > > Subject: RE: [Intel-gfx] [PATCH] drm/i915/display: Cleanup > > intel_phy_is_combo() > > > > > -----Original Message----- > > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf > > > Of Anusha Srivatsa > > > Sent: Tuesday, July 19, 2022 12:42 AM > > > To: intel-gfx@lists.freedesktop.org > > > Subject: [Intel-gfx] [PATCH] drm/i915/display: Cleanup > > > intel_phy_is_combo() > > > > > > No functional change. Cleanup the intel_phy_is_combo to accomodate > > > for cases where combo phy is not available. > > > > > > Cc: Matt Roper <matthew.d.roper@intel.com> > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 9 +-------- > > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > index a0f84cbe974f..b69208cf9a5e 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -2082,20 +2082,13 @@ bool intel_phy_is_combo(struct > > > drm_i915_private *dev_priv, enum phy phy) { > > > if (phy == PHY_NONE) > > > return false; > > > - else if (IS_DG2(dev_priv)) > > > - /* > > > - * DG2 outputs labelled as "combo PHY" in the bspec use > > > - * SNPS PHYs with completely different programming, > > > - * hence we always return false here. > > > - */ > > > - return false; > > I feel it would be good to retain this. This is very well commented. > > In future upon adding something like DISPLAY_VER(dev_priv) >= 11, like > > the one done below can create confusion. > > What if I retain the comments with the code change? > Retaining this comment will do, but I feel this cleanup can be taken upon updating this with the future platforms. Other can comment over here. Thanks and Regards, Arun R Murthy --------------------
> -----Original Message----- > From: Murthy, Arun R <arun.r.murthy@intel.com> > Sent: Tuesday, July 19, 2022 7:34 PM > To: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel- > gfx@lists.freedesktop.org > Subject: RE: [Intel-gfx] [PATCH] drm/i915/display: Cleanup > intel_phy_is_combo() > > > > -----Original Message----- > > > From: Murthy, Arun R <arun.r.murthy@intel.com> > > > Sent: Monday, July 18, 2022 8:32 PM > > > To: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel- > > > gfx@lists.freedesktop.org > > > Subject: RE: [Intel-gfx] [PATCH] drm/i915/display: Cleanup > > > intel_phy_is_combo() > > > > > > > -----Original Message----- > > > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On > > > > Behalf Of Anusha Srivatsa > > > > Sent: Tuesday, July 19, 2022 12:42 AM > > > > To: intel-gfx@lists.freedesktop.org > > > > Subject: [Intel-gfx] [PATCH] drm/i915/display: Cleanup > > > > intel_phy_is_combo() > > > > > > > > No functional change. Cleanup the intel_phy_is_combo to accomodate > > > > for cases where combo phy is not available. > > > > > > > > Cc: Matt Roper <matthew.d.roper@intel.com> > > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_display.c | 9 +-------- > > > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > index a0f84cbe974f..b69208cf9a5e 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > @@ -2082,20 +2082,13 @@ bool intel_phy_is_combo(struct > > > > drm_i915_private *dev_priv, enum phy phy) { > > > > if (phy == PHY_NONE) > > > > return false; > > > > - else if (IS_DG2(dev_priv)) > > > > - /* > > > > - * DG2 outputs labelled as "combo PHY" in the bspec use > > > > - * SNPS PHYs with completely different programming, > > > > - * hence we always return false here. > > > > - */ > > > > - return false; > > > I feel it would be good to retain this. This is very well commented. > > > In future upon adding something like DISPLAY_VER(dev_priv) >= 11, > > > like the one done below can create confusion. > > > > What if I retain the comments with the code change? > > > Retaining this comment will do, but I feel this cleanup can be taken upon > updating this with the future platforms. Other can comment over here. We need this for MTL. Hence the cleanup :) Anusha > Thanks and Regards, > Arun R Murthy > --------------------
> > > > > -----Original Message----- > > > > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On > > > > > Behalf Of Anusha Srivatsa > > > > > Sent: Tuesday, July 19, 2022 12:42 AM > > > > > To: intel-gfx@lists.freedesktop.org > > > > > Subject: [Intel-gfx] [PATCH] drm/i915/display: Cleanup > > > > > intel_phy_is_combo() > > > > > > > > > > No functional change. Cleanup the intel_phy_is_combo to > > > > > accomodate for cases where combo phy is not available. > > > > > > > > > > Cc: Matt Roper <matthew.d.roper@intel.com> > > > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_display.c | 9 +-------- > > > > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > > index a0f84cbe974f..b69208cf9a5e 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > @@ -2082,20 +2082,13 @@ bool intel_phy_is_combo(struct > > > > > drm_i915_private *dev_priv, enum phy phy) { > > > > > if (phy == PHY_NONE) > > > > > return false; > > > > > - else if (IS_DG2(dev_priv)) > > > > > - /* > > > > > - * DG2 outputs labelled as "combo PHY" in the bspec > use > > > > > - * SNPS PHYs with completely different programming, > > > > > - * hence we always return false here. > > > > > - */ > > > > > - return false; > > > > I feel it would be good to retain this. This is very well commented. > > > > In future upon adding something like DISPLAY_VER(dev_priv) >= 11, > > > > like the one done below can create confusion. > > > > > > What if I retain the comments with the code change? > > > > > Retaining this comment will do, but I feel this cleanup can be taken > > upon updating this with the future platforms. Other can comment over > here. > > We need this for MTL. Hence the cleanup :) > Can you float another version retaining the DG2 comments. Thanks and Regards, Arun R Murthy --------------------
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index a0f84cbe974f..b69208cf9a5e 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -2082,20 +2082,13 @@ bool intel_phy_is_combo(struct drm_i915_private *dev_priv, enum phy phy) { if (phy == PHY_NONE) return false; - else if (IS_DG2(dev_priv)) - /* - * DG2 outputs labelled as "combo PHY" in the bspec use - * SNPS PHYs with completely different programming, - * hence we always return false here. - */ - return false; else if (IS_ALDERLAKE_S(dev_priv)) return phy <= PHY_E; else if (IS_DG1(dev_priv) || IS_ROCKETLAKE(dev_priv)) return phy <= PHY_D; else if (IS_JSL_EHL(dev_priv)) return phy <= PHY_C; - else if (DISPLAY_VER(dev_priv) >= 11) + else if (IS_ALDERLAKE_P(dev_priv) || IS_DISPLAY_VER(dev_priv, 11, 12)) return phy <= PHY_B; else return false;
No functional change. Cleanup the intel_phy_is_combo to accomodate for cases where combo phy is not available. Cc: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)