> -----Original Message----- > From: Nikula, Jani <[email protected]> > Sent: Monday, November 17, 2025 8:59 PM > To: Manna, Animesh <[email protected]>; intel- > [email protected]; [email protected]; dri- > [email protected] > Cc: Kandpal, Suraj <[email protected]>; Manna, Animesh > <[email protected]>; Hogander, Jouni > <[email protected]> > Subject: Re: [PATCH v4 02/10] drm/i915/alpm: alpm_init() for DP2.1 > > On Thu, 13 Nov 2025, Animesh Manna <[email protected]> wrote: > > Initialize ALPM for DP2.1 and separate out ALPM mutex-init from > > alpm-init. > > I thought I said you're going to need multiple init functions. Don't move the > alpm mutex init away from alpm code. It needs to stay in alpm code.
Only for mutex-init do you want me to add a separate function? > > And now the whole patch and subject and commit message talk of > completely different things. Earlier alpm is initialized for EDP only, now its extended for DP2.1 in this patch. Earlier mutex init is part of alpm-init. Now after feedback I also felt it can be separate out because reading dpcd always not possible if the display is disconnected for dp-connector but mutex-init can be done. So mentioned as separate out ALPM mutex-init from alpm-init(). > > Please read the review comments, and ask questions if they comments are > not clear. Currently I am little confused, need change in code or commit description. From here onwards if you can point out some specific change will modify accordingly in next version. Regards, Animesh > > BR, > Jani. > > > > > > v2: Separate out mutex-init. [Jani] > > v3: Refactor further to avoid DISPLAY_VER check in multiple places. > > [Jani] > > V4: Cosmetic changes. [Suraj] > > > > Cc: Jouni Högander <[email protected]> > > Reviewed-by: Suraj Kandpal <[email protected]> > > Signed-off-by: Animesh Manna <[email protected]> > > --- > > drivers/gpu/drm/i915/display/intel_alpm.c | 16 ++++++++++++++-- > > drivers/gpu/drm/i915/display/intel_alpm.h | 3 ++- > > drivers/gpu/drm/i915/display/intel_dp.c | 8 +++++++- > > 3 files changed, 23 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c > > b/drivers/gpu/drm/i915/display/intel_alpm.c > > index 6372f533f65b..14acd6717e59 100644 > > --- a/drivers/gpu/drm/i915/display/intel_alpm.c > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c > > @@ -41,7 +41,20 @@ 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) > > +bool intel_alpm_source_supported(struct intel_connector *connector) { > > + struct intel_display *display = to_intel_display(connector); > > + > > + if (!((connector->base.connector_type == > DRM_MODE_CONNECTOR_DisplayPort && > > + DISPLAY_VER(display) >= 35) || > > + (connector->base.connector_type == > DRM_MODE_CONNECTOR_eDP && > > + DISPLAY_VER(display) >= 20))) > > + return false; > > + > > + return true; > > +} > > + > > +void intel_alpm_get_sink_capability(struct intel_dp *intel_dp) > > { > > u8 dpcd; > > > > @@ -49,7 +62,6 @@ void intel_alpm_init(struct intel_dp *intel_dp) > > return; > > > > intel_dp->alpm_dpcd = dpcd; > > - mutex_init(&intel_dp->alpm.lock); > > } > > > > static int get_silence_period_symbols(const struct intel_crtc_state > > *crtc_state) diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h > > b/drivers/gpu/drm/i915/display/intel_alpm.h > > index 53599b464dea..bcc354a46a1d 100644 > > --- a/drivers/gpu/drm/i915/display/intel_alpm.h > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h > > @@ -15,7 +15,8 @@ struct intel_connector; struct intel_atomic_state; > > struct intel_crtc; > > > > -void intel_alpm_init(struct intel_dp *intel_dp); > > +bool intel_alpm_source_supported(struct intel_connector *connector); > > +void intel_alpm_get_sink_capability(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 0ec82fcbcf48..81dd5bf7e3c5 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -6074,6 +6074,9 @@ intel_dp_detect(struct drm_connector > *_connector, > > if (ret == 1) > > connector->base.epoch_counter++; > > > > + if (intel_alpm_source_supported(connector)) > > + intel_alpm_get_sink_capability(intel_dp); > > + > > if (!intel_dp_is_edp(intel_dp)) > > intel_psr_init_dpcd(intel_dp); > > > > @@ -6716,7 +6719,7 @@ static bool intel_edp_init_connector(struct > intel_dp *intel_dp, > > */ > > intel_hpd_enable_detection(encoder); > > > > - intel_alpm_init(intel_dp); > > + intel_alpm_get_sink_capability(intel_dp); > > > > /* Cache DPCD and EDID for edp. */ > > has_dpcd = intel_edp_init_dpcd(intel_dp, connector); @@ -6932,6 > > +6935,9 @@ intel_dp_init_connector(struct intel_digital_port > > *dig_port, > > > > intel_psr_init(intel_dp); > > > > + if (intel_alpm_source_supported(connector)) > > + mutex_init(&intel_dp->alpm.lock); > > + > > return true; > > > > fail: > > -- > Jani Nikula, Intel
