On Thu, 2025-04-17 at 15:11 +0530, Animesh Manna wrote:
> Make a generic alpm enable function for sink which can be used for
> PSR2/PR/Lobf.
>
> Signed-off-by: Animesh Manna <animesh.ma...@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_alpm.c | 27
> ++++++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_psr.c | 23 -------------------
> 2 files changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 3d4459881e7c..f4d869953045 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -429,6 +429,29 @@ void intel_alpm_pre_plane_update(struct
> intel_atomic_state *state,
> }
> }
>
> +static void intel_alpm_enable_sink(struct intel_dp *intel_dp,
> + const struct intel_crtc_state
> *crtc_state)
> +{
> + u8 val;
> +
> + /*
> + * eDP Panel Replay uses always ALPM
> + * PSR2 uses ALPM but PSR1 doesn't
> + */
> + if (!intel_dp_is_edp(intel_dp) || (!crtc_state-
> >has_panel_replay &&
> + !crtc_state-
> >has_sel_update &&
> + !crtc_state->has_lobf))
> + return;
> +
> + val = DP_ALPM_ENABLE | DP_ALPM_LOCK_ERROR_IRQ_HPD_ENABLE;
> +
> + if (crtc_state->has_panel_replay || (crtc_state->has_lobf &&
> +
> intel_alpm_aux_less_wake_supported(intel_dp)))
I know this is kind of late comment. I'm sorry for that. Instead of
spreading these ugly checks outside PSR code you could add:
intel_psr_needs_alpm(struct intel_dp *intel_dp, const struct
intel_crtc_state *crtc_state))
into intel_psr.[ch] and use it here and other relevant places in
intel_alpm.c. E.g lnl_alpm_configure:
if (DISPLAY_VER(display) < 20 || (!crtc_state->has_sel_update &&
!intel_dp_is_edp(intel_dp)))
return;
to
if (DISPLAY_VER(display) < 20 || !intel_psr_needs_alpm(intel_dp,
crtc_state)))
return;
and so on. What do you think?
BR,
Jouni Högander
> + val |= DP_ALPM_MODE_AUX_LESS;
> +
> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG,
> val);
> +}
> +
> void intel_alpm_post_plane_update(struct intel_atomic_state *state,
> struct intel_crtc *crtc)
> {
> @@ -452,8 +475,10 @@ void intel_alpm_post_plane_update(struct
> intel_atomic_state *state,
>
> intel_dp = enc_to_intel_dp(encoder);
>
> - if (intel_dp_is_edp(intel_dp))
> + if (intel_dp_is_edp(intel_dp)) {
> + intel_alpm_enable_sink(intel_dp,
> crtc_state);
> intel_alpm_configure(intel_dp, crtc_state);
> + }
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 43ed166007eb..68952f7bdd7c 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -794,32 +794,9 @@ static void _psr_enable_sink(struct intel_dp
> *intel_dp,
> drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, val);
> }
>
> -static void intel_psr_enable_sink_alpm(struct intel_dp *intel_dp,
> - const struct intel_crtc_state
> *crtc_state)
> -{
> - u8 val;
> -
> - /*
> - * eDP Panel Replay uses always ALPM
> - * PSR2 uses ALPM but PSR1 doesn't
> - */
> - if (!intel_dp_is_edp(intel_dp) || (!crtc_state-
> >has_panel_replay &&
> - !crtc_state-
> >has_sel_update))
> - return;
> -
> - val = DP_ALPM_ENABLE | DP_ALPM_LOCK_ERROR_IRQ_HPD_ENABLE;
> -
> - if (crtc_state->has_panel_replay)
> - val |= DP_ALPM_MODE_AUX_LESS;
> -
> - drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG,
> val);
> -}
> -
> static void intel_psr_enable_sink(struct intel_dp *intel_dp,
> const struct intel_crtc_state
> *crtc_state)
> {
> - intel_psr_enable_sink_alpm(intel_dp, crtc_state);
> -
> crtc_state->has_panel_replay ?
> _panel_replay_enable_sink(intel_dp, crtc_state) :
> _psr_enable_sink(intel_dp, crtc_state);
