Re: [PATCH] drm/i915/display: Write PHY_CMN1_CONTROL only when using AUXLess ALPM

[Gustavo Sousa <gustavo.sousa@intel.com>]

Quoting Jouni Högander (2025-07-17 03:32:58-03:00)
>We are seeing "dmesg-warn/abort - *ERROR* PHY * failed after 3 retries"
>since we started configuring LFPS sending. According to Bspec Configuring
>LFPS sending is needed only when using AUXLess ALPM. This patch avoids
>these failures by configuring LFPS sending only when using AUXLess ALPM.

Hm... But then with this patch we are missing writing zero to that bit
when necessary, no?

Could the timeouts be happening because intel_cx0_rmw() is getting
called without calling
intel_cx0_phy_transaction_begin()/intel_cx0_phy_transaction_end()?

>
>Fixes: 9dc619680de4 ("drm/i915/display: Add function to configure LFPS sending")
>Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_cx0_phy.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>index ed8e640b96b0..9cfc3187aeab 100644
>--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>@@ -3239,14 +3239,14 @@ void intel_lnl_mac_transmit_lfps(struct intel_encoder *encoder,
>                                  const struct intel_crtc_state *crtc_state)
> {
>         struct intel_display *display = to_intel_display(encoder);
>-        u8 owned_lane_mask = intel_cx0_get_owned_lane_mask(encoder);
>-        bool enable = intel_alpm_is_alpm_aux_less(enc_to_intel_dp(encoder),
>-                                                  crtc_state);
>+        u8 owned_lane_mask;
>         int i;
> 
>-        if (DISPLAY_VER(display) < 20)
>+        if (DISPLAY_VER(display) < 20 ||
>+            !intel_alpm_is_alpm_aux_less(enc_to_intel_dp(encoder), crtc_state))
>                 return;
> 
>+        owned_lane_mask = intel_cx0_get_owned_lane_mask(encoder);

This optimization could be on it's own patch.

--
Gustavo Sousa

>         for (i = 0; i < 4; i++) {
>                 int tx = i % 2 + 1;
>                 u8 lane_mask = i < 2 ? INTEL_CX0_LANE0 : INTEL_CX0_LANE1;
>@@ -3256,8 +3256,7 @@ void intel_lnl_mac_transmit_lfps(struct intel_encoder *encoder,
> 
>                 intel_cx0_rmw(encoder, lane_mask, PHY_CMN1_CONTROL(tx, 0),
>                               CONTROL0_MAC_TRANSMIT_LFPS,
>-                              enable ? CONTROL0_MAC_TRANSMIT_LFPS : 0,
>-                              MB_WRITE_COMMITTED);
>+                              CONTROL0_MAC_TRANSMIT_LFPS, MB_WRITE_COMMITTED);
>         }
> }
> 
>-- 
>2.43.0
>