Re: [PATCH 09/11] drm/i915/display: Add function to configure LFPS sending

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

On Tue, 2025-05-20 at 14:38 +0530, Nautiyal, Ankit K wrote:
> 
> On 5/2/2025 2:29 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.
> > 
> > HAS: 14012758795
> 
> Lets drop reference to HAS and just use Bspec: 68849
> 
> 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >   drivers/gpu/drm/i915/display/intel_cx0_phy.c | 29
> > ++++++++++++++++++++
> >   drivers/gpu/drm/i915/display/intel_cx0_phy.h |  2 ++
> >   drivers/gpu/drm/i915/display/intel_ddi.c     |  8 +++++-
> >   3 files changed, 38 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 1318886e34dd7..d8c1d372339c6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > @@ -3232,6 +3232,35 @@ void intel_mtl_pll_enable(struct
> > intel_encoder *encoder,
> >   		intel_cx0pll_enable(encoder, crtc_state);
> >   }
> >   
> > +/**
> > + * intel_lnl_mac_transmit_lfps - Enable MAC transmitting LFPS
> > + * @encoder: Intel encoder
> > + * @crtc_state: Intel CRTC State
> > + *
> > + * Enable MAC transmitting LFPS in case of AUX Less ALPM.
> > + */
> > +void intel_lnl_mac_transmit_lfps(struct intel_encoder *encoder,
> > +				 const struct intel_crtc_state
> > *crtc_state)
> > +{
> > +	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;
> > +
> > +	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;
> 
> Similar loop is used in other place while writing other similar 
> register. Would it make sense to use a macro?
> 
> 
> > +
> > +		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 a8f811ca5e7bc..c5a7b529955b0 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 ce7a4765b10ea..f8cdaae52ccfe 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);
> 
> Here too a platform check should be there to avoid writing registers
> for 
> older platforms.

I have added platform check. Please check latest version.

BR,

Jouni Högander

> 
> 
> Regards,
> 
> Ankit
> 
> >   }
> >   
> >   static void intel_ddi_prepare_link_retrain(struct intel_dp
> > *intel_dp,