Re: [PATCH v2 10/12] drm/i915/display: Add function to configure LFPS sending

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Wed, 2025-05-21 at 10:30 +0530, Nautiyal, Ankit K wrote:
> 
> On 5/20/2025 10:23 PM, Jouni Högander wrote:
> > Add function to configre LFPS sending for Panel Replay according to
> > link
> > training sequence in HAS document.
> > 
> > This assumes we are using AUX Less always if it's supported by the
> > sink and
> > the source.
> > 
> > v2:
> >    - drop HAS reference
> >    - replay kerneldoc comment with a generic comment
> >    - check display version in intel_lnl_mac_transmit_lfps
> > 
> > Bspec: 68849
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >   drivers/gpu/drm/i915/display/intel_cx0_phy.c | 31
> > ++++++++++++++++++++
> >   drivers/gpu/drm/i915/display/intel_cx0_phy.h |  2 ++
> >   drivers/gpu/drm/i915/display/intel_ddi.c     |  8 ++++-
> >   3 files changed, 40 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > index 91118d115fd3..75caccb65513 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > @@ -3225,6 +3225,37 @@ void intel_mtl_pll_enable(struct
> > intel_encoder *encoder,
> >   		intel_cx0pll_enable(encoder, crtc_state);
> >   }
> >   
> > +/*
> > + * According to HAS we need to enable MAC Transmitting LFPS in the
> > "PHY Common
> > + * Control 0" PIPE register in case of AUX Less ALPM is going to
> > be used. This
> > + * function is doing that and is called by link retrain sequence.
> > + */
> > +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);
> > +	int i;
> > +
> > +	if (DISPLAY_VER(display) < 20)
> > +		return;
> > +
> > +	for (i = 0; i < 4; i++) {
> > +		int tx = i % 2 + 1;
> > +		u8 lane_mask = i < 2 ? INTEL_CX0_LANE0 :
> > INTEL_CX0_LANE1;
> > +
> > +		if (!(owned_lane_mask & lane_mask))
> > +			continue;
> 
> Perhaps a macro for the loop pattern would make sense, as this is
> used 
> for other CX0 register.
> 
> Regardless of the above, patch looks good to me.

I was supposed to check with you if it's ok leave that out from my
patch set. Thank you for the review.

BR,

Jouni Högander

> 
> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> 
> 
> > +
> > +		intel_cx0_rmw(encoder, lane_mask,
> > PHY_CMN1_CONTROL(tx, 0),
> > +			      CONTROL0_MAC_TRANSMIT_LFPS,
> > +			      enable ? CONTROL0_MAC_TRANSMIT_LFPS
> > : 0,
> > +			      MB_WRITE_COMMITTED);
> > +	}
> > +}
> > +
> >   static u8 cx0_power_control_disable_val(struct intel_encoder
> > *encoder)
> >   {
> >   	struct intel_display *display = to_intel_display(encoder);
> > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > index a8f811ca5e7b..c5a7b529955b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > @@ -43,5 +43,7 @@ void intel_cx0_phy_set_signal_levels(struct
> > intel_encoder *encoder,
> >   				     const struct intel_crtc_state
> > *crtc_state);
> >   int intel_mtl_tbt_calc_port_clock(struct intel_encoder *encoder);
> >   void intel_cx0_pll_power_save_wa(struct intel_display *display);
> > +void intel_lnl_mac_transmit_lfps(struct intel_encoder *encoder,
> > +				 const struct intel_crtc_state
> > *crtc_state);
> >   
> >   #endif /* __INTEL_CX0_PHY_H__ */
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 65ab012c3eae..70de99cc4ead 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3745,10 +3745,16 @@ static void
> > mtl_ddi_prepare_link_retrain(struct intel_dp *intel_dp,
> >   	intel_dp->DP |= DDI_BUF_CTL_ENABLE;
> >   
> >   	/*
> > -	 * 6.k If AUX-Less ALPM is going to be enabled
> > +	 * 6.k If AUX-Less ALPM is going to be enabled:
> >   	 *     i. Configure PORT_ALPM_CTL and PORT_ALPM_LFPS_CTL
> > here
> >   	 */
> >   	intel_alpm_port_configure(intel_dp, crtc_state);
> > +
> > +	/*
> > +	 *     ii. Enable MAC Transmits LFPS in the "PHY Common
> > Control 0" PIPE
> > +	 *         register
> > +	 */
> > +	intel_lnl_mac_transmit_lfps(encoder, crtc_state);
> >   }
> >   
> >   static void intel_ddi_prepare_link_retrain(struct intel_dp
> > *intel_dp,