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.

And now the whole patch and subject and commit message talk of
completely different things.

Please read the review comments, and ask questions if they comments are
not clear.

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

Reply via email to