Re: [PATCHv2 2/2] drm/i915/dp: Rename alpm_init to alpm_init_dpcd

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

On Tue, 2026-03-03 at 04:10 +0000, Murthy, Arun R wrote:
> 
> > -----Original Message-----
> > From: Dibin Moolakadan Subrahmanian
> > <dibin.moolakadan.subrahmanian@intel.com>
> > Sent: Monday, March 2, 2026 8:28 PM
> > To: Murthy, Arun R <arun.r.murthy@intel.com>;
> > intel-gfx@lists.freedesktop.org;
> > intel-xe@lists.freedesktop.org
> > Cc: Manna, Animesh <animesh.manna@intel.com>; Nautiyal, Ankit K
> > <ankit.k.nautiyal@intel.com>; Nikula, Jani <jani.nikula@intel.com>;
> > Hogander,
> > Jouni <jouni.hogander@intel.com>
> > Subject: Re: [PATCHv2 2/2] drm/i915/dp: Rename alpm_init to
> > alpm_init_dpcd
> > 
> > 
> > On 02-03-2026 14:10, Arun R Murthy wrote:
> > > In the function intel_alpm_init we are reading the
> > > ALPM_CAPABILITIES
> > > and storing them in intel_dp, so appending the function name to
> > > _dpcd
> > > so as to align with other function such as intel_psr_init_dpcd
> > > referenced in the same function.
> > 
> > Rename looks okay, but mutex_init(&intel_dp->alpm.lock) looks to be
> > out of the
> > place in intel_psr_init_dpcd.
> > 
> A patch for this is already floated by Animesh.
> https://patchwork.freedesktop.org/patch/687711/?series=156417&rev=4

you could already do this in your patch (set). I.e. split
intel_alpm_init as intel_alpm_init_dpcd and intel_alpm_init. Move edp
counterpart as you are doing in this patch and then move
intel_alpm_init into intel_dp_init_connector. Intel_alpm_init
initializing the mutex and intel_alpm_init_dpcd reading sink alpm
capabilities.

Anyways this patch is clearly fixing one issue and not introducing any
new. We are now talking about existing problem where mutex is
initialize inf wrong place:

Reviewed-by: Jouni Högander <jouni.hogander@intel.com>

BR,
Jouni Högander

> 
> Thanks and Regards,
> Arun R Murthy
> --------------------
> 
> > Regards,
> > Dibin
> > 
> > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/display/intel_alpm.c | 2 +-
> > >   drivers/gpu/drm/i915/display/intel_alpm.h | 2 +-
> > >   drivers/gpu/drm/i915/display/intel_dp.c   | 2 +-
> > >   3 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > > index b3334bc4d0f9..8ba7463d7fe1 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > > @@ -42,7 +42,7 @@ 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(struct intel_dp *intel_dp)
> > > +void intel_alpm_init_dpcd(struct intel_dp *intel_dp)
> > >   {
> > >   	u8 dpcd;
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h
> > > b/drivers/gpu/drm/i915/display/intel_alpm.h
> > > index 1cf70668ab1b..a24a7a03bdaa 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_alpm.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h
> > > @@ -15,7 +15,7 @@ struct intel_connector;
> > >   struct intel_atomic_state;
> > >   struct intel_crtc;
> > > 
> > > -void intel_alpm_init(struct intel_dp *intel_dp);
> > > +void intel_alpm_init_dpcd(struct intel_dp *intel_dp);
> > >   bool intel_alpm_compute_params(struct intel_dp *intel_dp,
> > >   			       struct intel_crtc_state
> > > *crtc_state);
> > >   void intel_alpm_lobf_compute_config(struct intel_dp *intel_dp,
> > > diff
> > > --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 1544758c0bbc..4e9df88b90cd 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -4749,7 +4749,7 @@ intel_edp_init_dpcd(struct intel_dp
> > > *intel_dp,
> > struct intel_connector *connector
> > >   	intel_dp_init_source_oui(intel_dp);
> > > 
> > >   	/* Read ALPM DPCD caps before reading the PSR CAPS */
> > > -	intel_alpm_init(intel_dp);
> > > +	intel_alpm_init_dpcd(intel_dp);
> > > 
> > >   	/*
> > >   	 * This has to be called after intel_dp->edp_dpcd is
> > > filled, PSR
> > > checks