Re: [PATCH v5 02/15] drm/i915/alpm: Move alpm sink capabality readout in separate function

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

On Tue, 2026-08-25 at 07:57 +0000, Kandpal, Suraj wrote:
> > Subject: [PATCH v5 02/15] drm/i915/alpm: Move alpm sink capabality
> > readout
> > in separate function
> > 
> > Non functional change.
> 
> Please add a definition as to what the patch is doing something like
> "Abstract ALPM DPCD initiailization into its own function"

Addition to comment from Suraj: Please drop "Non functional change"
completely as it is not true. Currently failure in drm_dp_dpcd_readb is
triggering early exit. After you commit it is not.

BR,
Jouni Högander

> 
> Regards,
> Suraj Kandpal
> 
> > 
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_alpm.c | 10 ++++++++++
> > drivers/gpu/drm/i915/display/intel_alpm.h |  1 +
> >  drivers/gpu/drm/i915/display/intel_dp.c   |  7 +------
> >  3 files changed, 12 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index f1383764b702..542900e29a01 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -43,6 +43,16 @@ bool intel_alpm_is_alpm_aux_less(struct intel_dp
> > *intel_dp,
> >  		(crtc_state->has_lobf &&
> > intel_alpm_aux_less_wake_supported(intel_dp));
> >  }
> > 
> > +void intel_alpm_init_dpcd(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;
> > +}
> > +
> >  void intel_alpm_init(struct intel_dp *intel_dp)  {
> >  	mutex_init(&intel_dp->alpm.lock);
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h
> > b/drivers/gpu/drm/i915/display/intel_alpm.h
> > index 1cf70668ab1b..fae68d557960 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.h
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h
> > @@ -15,6 +15,7 @@ struct intel_connector;  struct
> > intel_atomic_state;  struct
> > intel_crtc;
> > 
> > +void intel_alpm_init_dpcd(struct intel_dp *intel_dp);
> >  void intel_alpm_init(struct intel_dp *intel_dp);  bool
> > intel_alpm_compute_params(struct intel_dp *intel_dp,
> >  			       struct intel_crtc_state
> > *crtc_state); diff --git
> > a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 6b60e0a1c8ee..77d516f0bd07 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4764,7 +4764,6 @@ 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);
> > @@ -4804,11 +4803,7 @@ 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 */
> > -	ret = drm_dp_dpcd_read_byte(&intel_dp->aux,
> > DP_RECEIVER_ALPM_CAP,
> > -				    &intel_dp->alpm_dpcd);
> > -	if (ret < 0)
> > -		return false;
> > +	intel_alpm_init_dpcd(intel_dp);
> > 
> >  	/*
> >  	 * This has to be called after intel_dp->edp_dpcd is
> > filled, PSR checks
> > --
> > 2.29.0
>