> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper@xxxxxxxxx>
> Sent: Thursday, July 21, 2022 1:50 PM
> To: Srivatsa, Anusha <anusha.srivatsa@xxxxxxxxx>
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Murthy, Arun R
> <arun.r.murthy@xxxxxxxxx>
> 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@xxxxxxxxx>
> > Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
> > ---
> > 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
