[Intel-gfx] [PATCH] drm/i915/display: Cleanup intel_phy_is_combo()

[Srivatsa, Anusha]

> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper at intel.com>
> Sent: Thursday, July 21, 2022 1:50 PM
> To: Srivatsa, Anusha <anusha.srivatsa at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; Murthy, Arun R
> <arun.r.murthy at intel.com>
> Subject: Re: [PATCH] drm/i915/display: Cleanup intel_phy_is_combo()
> 
> On Thu, Jul 21, 2022 at 01:17:54PM -0700, Anusha Srivatsa wrote:
> > No functional change. Cleanup the intel_phy_is_combo
> 
> But there actually is a functional change here --- display version 14 will now
> (properly) fall through to the 'else' branch instead of being picked up by the
> 11/12/adl branch.  I believe that was your original motivation for this patch,
> so you may want to mention that in the commit message (and drop the "no
> functional change" statement).
> 
> The code change itself looks fine to me since it seems like the traditional
> combo PHYs may be a thing of the past and we don't want to keep assuming
> future platforms will have any.
> 
With the change in commit message can I add your reviewed-by laong with Arun's?

Anusha
> Matt
> 
> > to accommodate for cases where combo phy is not available.
> >
> > v2: retain comment that explains DG2 returning false from
> > intel_phy_is_combo() (Arun)
> >
> > Cc: Arun R Murthy <arun.r.murthy at intel.com>
> > Cc: Matt Roper <matthew.d.roper at intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index a0f84cbe974f..b9d0be7753a8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -2082,22 +2082,20 @@ 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
> > +		/*
> > +		 * DG2 outputs labelled as "combo PHY" in the bspec use
> > +		 * SNPS PHYs with completely different programming,
> > +		 * hence we always return false here.
> > +		 */
> >  		return false;
> >  }
> >
> > --
> > 2.25.1
> >
> 
> --
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation