On Thu, 2025-07-17 at 10:31 -0300, Gustavo Sousa wrote:
> 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?
That shouldn't be necessary as 0 is the reset value.
>
> Could the timeouts be happening because intel_cx0_rmw() is getting
> called without calling
> intel_cx0_phy_transaction_begin()/intel_cx0_phy_transaction_end()?
I wasn't aware about these. I will try them.
BR,
Jouni Högander
>
> >
> > Fixes: 9dc619680de4 ("drm/i915/display: Add function to configure
> > LFPS sending")
> > Signed-off-by: Jouni Högander <jouni.hogan...@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
> >
