Re: [PATCHv4] drm/i915/dp: Read ALPM caps after DPCD init

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

On Wed, 2026-03-04 at 08:17 +0200, Hogander, Jouni wrote:
> On Wed, 2026-03-04 at 09:35 +0530, Arun R Murthy wrote:
> > For eDP read the ALPM DPCD caps after DPCD initalization and just
> > before
> > the PSR init.
> > 
> > v2: Move intel_alpm_init to intel_edp_init_dpcd (Jouni)
> > v3: Add Fixes with commit-id (Jouni)
> > v4: Separated the alpm dpcd read caps from alpm_init and moved to
> > intel_edp_init_dpcd.
> > 
> > Fixes: 15438b325987 ("drm/i915/alpm: Add compute config for lobf")
> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_alpm.c | 6 ------
> >  drivers/gpu/drm/i915/display/intel_dp.c   | 9 +++++++++
> >  2 files changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index b3334bc4d0f9..a7350ce8e716 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -44,12 +44,6 @@ bool intel_alpm_is_alpm_aux_less(struct intel_dp
> > *intel_dp,
> >  
> >  void intel_alpm_init(struct intel_dp *intel_dp)
> >  {
> > -	u8 dpcd;
> > -
> > -	if (drm_dp_dpcd_readb(&intel_dp->aux,
> > DP_RECEIVER_ALPM_CAP,
> > &dpcd) < 0)
> > -		return;
> > -
> > -	intel_dp->alpm_dpcd = dpcd;
> >  	mutex_init(&intel_dp->alpm.lock);
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 025e906b63a9..e781d4d0dd1b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4709,6 +4709,7 @@ static bool
> >  intel_edp_init_dpcd(struct intel_dp *intel_dp, struct
> > intel_connector *connector)
> >  {
> >  	struct intel_display *display =
> > to_intel_display(intel_dp);
> > +	int ret;
> >  
> >  	/* this function is meant to be called only once */
> >  	drm_WARN_ON(display->drm, intel_dp->dpcd[DP_DPCD_REV] !=
> > 0);
> > @@ -4748,6 +4749,14 @@ intel_edp_init_dpcd(struct intel_dp
> > *intel_dp,
> > struct intel_connector *connector
> >  	 */
> >  	intel_dp_init_source_oui(intel_dp);
> >  
> > +	/* Read the ALPM DPCD caps */
> > +	if (intel_dp->edp_dpcd[0] >= DP_EDP_14) {
> 
> I think you should not check eDP revision. DP_RECEIVER_ALPM_CAP is
> supposed to return 0 if ALPM isn't supported.
> 
> > +		ret = drm_dp_dpcd_readb(&intel_dp->aux,
> > DP_RECEIVER_ALPM_CAP,
> > +					&intel_dp->alpm_dpcd);
> > +		if (ret < 0)
> > +			return false;
> > +	}
> > +
> 
> Rather add e.g. intel_alpm_init_dpcd. Animesh is working on that DP
> ALPM support (https://patchwork.freedesktop.org/series/156417/) and
> your added dpcd init will be shared with DP path.

Now just noticed it's different DPCD address for DP. I.e. no reason to
add init dpcd. Still please consider my other comment about checking
eDP revision.

BR,
Jouni Högander

> 
> BR,
> Jouni Högander
> 
> >  	/*
> >  	 * This has to be called after intel_dp->edp_dpcd is
> > filled,
> > PSR checks
> >  	 * for SET_POWER_CAPABLE bit in intel_dp->edp_dpcd[1]
>