Quoting Bhadane, Dnyaneshwar (2025-09-09 15:38:49-03:00)
>
>
>> -----Original Message-----
>> From: Sousa, Gustavo <gustavo.so...@intel.com>
>> Sent: Tuesday, September 9, 2025 8:34 PM
>> To: Bhadane, Dnyaneshwar <dnyaneshwar.bhad...@intel.com>; intel-
>> g...@lists.freedesktop.org; intel...@lists.freedesktop.org
>> Cc: Bhadane, Dnyaneshwar <dnyaneshwar.bhad...@intel.com>; Nautiyal,
>> Ankit K <ankit.k.nauti...@intel.com>; Atwood, Matthew S
>> <matthew.s.atw...@intel.com>
>> Subject: Re: [RESEND] drm/i915/xe3: Restrict PTL intel_encoder_is_c10phy()
>> to only PHY A
>> 
>> Quoting Dnyaneshwar Bhadane (2025-09-09 06:45:35-03:00)
>> >On PTL, no combo PHY is connected to PORT B. However, PORT B can still
>> >be used for Type-C and will utilize the C20 PHY for eDP over Type-C. In
>> >such configurations, VBTs also enumerate PORT B.
>> >
>> >This leads to issues where PORT B is incorrectly identified as using
>> >the
>> >C10 PHY, due to the assumption that returning true for PORT B in
>> >intel_encoder_is_c10phy() would not cause problems.
>> >
>> >From PTL's perspective, only PORT A/PHY A uses the C10 PHY.
>> >
>> >Update the helper intel_encoder_is_c10phy() to return true only for
>> >PORT A/PHY on PTL.
>> >
>> 
>> I think we need a "Fixes" tag here.
>> 
>> Fixes: 9d10de78a37f ("drm/i915/wcl: C10 phy connected to port A and B")
>Sure will incorporate in next rev.
>> 
>> >Bspec: 72571,73944
>> >Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhad...@intel.com>
>> >Reviewed-by: Ankit Nautiyal <ankit.k.nauti...@intel.com>
>> >---
>> > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 9 ++-------
>> > 1 file changed, 2 insertions(+), 7 deletions(-)
>> >
>> >diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> >b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> >index 801235a5bc0a..33963ad14cfa 100644
>> >--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> >+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> >@@ -39,13 +39,8 @@ 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);
>> >
>> >-        /* PTL doesn't have a PHY connected to PORT B; as such,
>> >-         * there will never be a case where PTL uses PHY B.
>> >-         * WCL uses PORT A and B with the C10 PHY.
>> >-         * Reusing the condition for WCL and extending it for PORT B
>> >-         * should not cause any issues for PTL.
>> >-         */
>> >-        if (display->platform.pantherlake && phy < PHY_C)
>> >+        if ((display->platform.pantherlake && phy == PHY_A) ||
>> >+            ((DISPLAY_VERx100(display) == 3002) && phy == PHY_B))
>> 
>> This is usually a property of the SoC and not the display IP. So, we probably
>> want to know that we are on WCL rather than display version 3002, which
>> could theoretically be re-used in other platforms with different display 
>> PHY(s).
>> 
>Hi, 
>would you suggest that we should introduce something like platform.wildcatlake 
>in this case?

Perhaps as a subplatform (it could be only on display side) of
pantherlake?

Cc: Matt Roper

--
Gustavo Sousa

>
>Dnyaneshwar, 
>> Also, as side note, there are many superfluous parentheses in the condition. 
>> I
>> think only the one for the if-condition is necessary.
>
>> 
>> --
>> Gustavo Sousa
>> 
>> >                 return true;
>> >
>> >         if ((display->platform.lunarlake ||
>> >display->platform.meteorlake) && phy < PHY_C)
>> >--
>> >2.51.0
>> >

Reply via email to