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

[Bhadane, Dnyaneshwar]

> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper at intel.com>
> Sent: Tuesday, June 10, 2025 10:24 PM
> To: Taylor, Clinton A <clinton.a.taylor at intel.com>
> Cc: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane at intel.com>; intel-
> xe at lists.freedesktop.org
> Subject: Re: [PATCH 8/9] drm/i915/wcl: C10 phy connected to port A and B
> 
> 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.
HI, 
For now,  I have just extended the condition with existing PTL conditions, 
Please take a look at
1.  gfx-xe - https://patchwork.freedesktop.org/series/150255/
2. gfx-intel - https://patchwork.freedesktop.org/series/150101/

Dnyaneshwar, 

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