> -----Original Message-----
> From: Hogander, Jouni <[email protected]>
> Sent: Thursday, April 17, 2025 7:12 PM
> To: [email protected]; Manna, Animesh
> <[email protected]>; [email protected]
> Cc: Nikula, Jani <[email protected]>; B, Jeevan <[email protected]>
> Subject: Re: [PATCH v9 10/10] drm/i915/display: Disintegrate sink alpm
> enable from psr with lobf
>
> 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 <[email protected]>
> > ---
> > 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?
As we are trying to disintegrate alpm from psr, intel_psr_needs_alpm() cannot
be used for lobf.
So we need two different api - intel_lobf_needs_alpm() and one for psr.
Do you want me to add the above or something better.
Regards,
Animesh
>
> 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);
