On Mon, Jul 25, 2022 at 09:45:57AM -0700, Srivatsa, Anusha wrote:
>
>
> > -----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?
Yeah, with an updated commit message,
Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
>
> 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
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
