diff mbox series

drm/i915/display: Cleanup intel_phy_is_combo()

Message ID 20220721201754.534870-1-anusha.srivatsa@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/display: Cleanup intel_phy_is_combo() | expand

Commit Message

Srivatsa, Anusha July 21, 2022, 8:17 p.m. UTC 
No functional change. Cleanup the intel_phy_is_combo
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@intel.com>
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 | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

Comments

Matt Roper July 21, 2022, 8:49 p.m. UTC | #1 
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.


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@intel.com>
> 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 | 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
>
Murthy, Arun R July 24, 2022, 12:30 p.m. UTC | #2 
> -----Original Message-----
> From: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Sent: Friday, July 22, 2022 1:48 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; Murthy, Arun R
> <arun.r.murthy@intel.com>; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: [PATCH] drm/i915/display: Cleanup intel_phy_is_combo()
> 
> No functional change. Cleanup the intel_phy_is_combo 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@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
--------------------
Srivatsa, Anusha July 25, 2022, 4:45 p.m. UTC | #3 
> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper@intel.com>
> Sent: Thursday, July 21, 2022 1:50 PM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Murthy, Arun R
> <arun.r.murthy@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@intel.com>
> > 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 | 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 July 25, 2022, 8:40 p.m. UTC | #4 
On Mon, Jul 25, 2022 at 09:45:57AM -0700, Srivatsa, Anusha wrote:
> 
> 
> > -----Original Message-----
> > From: Roper, Matthew D <matthew.d.roper@intel.com>
> > Sent: Thursday, July 21, 2022 1:50 PM
> > To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Murthy, Arun R
> > <arun.r.murthy@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?

Yeah, with an updated commit message,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> 
> 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@intel.com>
> > > 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 | 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
diff mbox series

Patch

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;
 }