[PATCH 8/9] drm/i915/wcl: C10 phy connected to port A and B

[Matt Roper]

On Tue, Jun 10, 2025 at 09:31:51AM -0700, Taylor, Clinton A wrote:
> On Tue, 2025-06-10 at 08:13 -0700, Matt Roper wrote:
> > On Tue, Jun 10, 2025 at 06:49:01AM -0700, Bhadane, Dnyaneshwar wrote:
> > > 
> > > 
> > > > -----Original Message-----
> > > > From: Roper, Matthew D <matthew.d.roper at intel.com>
> > > > Sent: Monday, June 9, 2025 8:35 PM
> > > > To: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane at intel.com>
> > > > Cc: intel-xe at lists.freedesktop.org
> > > > Subject: Re: [PATCH 8/9] drm/i915/wcl: C10 phy connected to port
> > > > A and B
> > > > 
> > > > On Sat, Jun 07, 2025 at 01:21:03AM +0530, Dnyaneshwar Bhadane
> > > > wrote:
> > > > > WCL added a c10 phy connected to port B. PTL code is currently
> > > > > restricting c10 to phy_a only.
> > > > 
> > > > This seems like it's a characteristic of the platform rather than
> > > > the IP itself.  We
> > > > should probably go ahead and break WCL out into its own display
> > > > platform that
> > > > we can match against rather than assuming WCL only uses display
> > > > 30.02 and
> > > > vice-versa.  Or maybe we can just update the existing PTL
> > > > condition if this
> > > > function would never be called on PTL with a non-existent PHY B?
> > > Hi Matt, 
> > > 1. We could add the WCL as display platform. But for now, there is
> > > not much reason to add the WCL display platform define for single
> > > instance. Thoughts ?
> > > 
> > > 2. We could not extend the condition directly with display-
> > > >platform.pantherlake && phy < PHY_C as it will wrongly select the
> > > c10 PHY type for the PTL itself.
> > 
> > What do you mean here?  If there's no PHY B at all on PTL (based on
> > 72571), then this function should never get called with phy == PHY_B,
> > right?
> 
> This function is called for very PHY. This function is the only way to
> determine in the PHY is C10 or C20.

It's called for non-existent PHYs that a platform doesn't have?  What's
the flow to get there?  It's been a while (and the design has changed a
bit) since I looked at this part of the code.

> 
> > 
> > >         
> > >         But can be done
> > >                  if ((display->platform.pantherlake && phy ==
> > > PHY_A) ||
> > >                  ((DISPLAY_VERx100(display) == 3002) && phy ==
> > > PHY_B) )
> > >                         return true;
> > 
> > The PHY types are a characteristic of the platform, not the display
> > IP.
> > So we want to try to avoid using DISPLAY_VER checks since then we'd
> > do
> > the wrong thing if this IP gets re-used on a platform with different
> > PHY
> > configuration, or if other WCL SKUs show up at some point with a
> > display
> > IP other than 30.02.
> 
> If this is the case, we need to make a new INTEL_DISPLAY_DEVICE for
> WCL. Currently WCL shows up as a PTL to the driver.

Yeah, if there are legitimate platform-level / soc-level changes outside
the display IP that can't be handled with common code then we should
define it as a separate platform.


Matt

> 
> Clint
> 
> > 
> > 
> > Matt
> > 
> > > 
> > > Dnyaneshwar, 
> > > > 
> > > > 
> > > > Matt
> > > > 
> > > > > 
> > > > > Signed-off-by: Dnyaneshwar Bhadane
> > > > > <dnyaneshwar.bhadane at intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > > > index 75caccb65513..59eee0edef79 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > > > @@ -40,6 +40,9 @@ bool intel_encoder_is_c10phy(struct
> > > > > intel_encoder
> > > > *encoder)
> > > > >         struct intel_display *display =
> > > > > to_intel_display(encoder);
> > > > >         enum phy phy = intel_encoder_to_phy(encoder);
> > > > > 
> > > > > +       if ((DISPLAY_VERx100(display) == 3002) && phy < PHY_C)
> > > > > +               return true;
> > > > > +
> > > > >         if (display->platform.pantherlake && phy == PHY_A)
> > > > >                 return true;
> > > > > 
> > > > > --
> > > > > 2.34.1
> > > > > 
> > > > 
> > > > --
> > > > Matt Roper
> > > > Graphics Software Engineer
> > > > Linux GPU Platform Enablement
> > > > Intel Corporation
> > 
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation