> -----Original Message-----
> From: Kandpal, Suraj <[email protected]>
> Sent: Friday, November 7, 2025 9:19 AM
> To: Manna, Animesh <[email protected]>; intel-
> [email protected]; [email protected]; dri-
> [email protected]
> Cc: Nikula, Jani <[email protected]>; Manna, Animesh
> <[email protected]>; Hogander, Jouni
> <[email protected]>
> Subject: RE: [PATCH v2 10/10] drm/i915/alpm: Introduce has_alpm to
> decouple from pr/psr2/lobf
> 
> > Subject: [PATCH v2 10/10] drm/i915/alpm: Introduce has_alpm to
> > decouple from pr/psr2/lobf
> >
> > For DP2.1, ALPM is optional and there can be scenario where panel
> > replay will be supported without ALPM. So decouple ALPM feaure by
> > has_alpm flag from
> 
> Typo *feature
> 
> > related display features.
> >
> > Cc: Jouni Högander <[email protected]>
> > Signed-off-by: Animesh Manna <[email protected]>
> > ---
> >  drivers/gpu/drm/i915/display/intel_alpm.c     |  7 +++---
> >  .../drm/i915/display/intel_display_types.h    |  2 ++
> >  drivers/gpu/drm/i915/display/intel_psr.c      | 23 ++++++++-----------
> >  drivers/gpu/drm/i915/display/intel_psr.h      |  1 -
> >  4 files changed, 15 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index 16fe66a75f05..e1988f43cbb9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -376,6 +376,8 @@ void intel_alpm_lobf_compute_config(struct
> > intel_dp *intel_dp,
> >
> >     crtc_state->has_lobf = (context_latency + guardband) >
> >             (first_sdp_position + waketime_in_lines);
> > +
> > +   crtc_state->has_alpm = crtc_state->has_lobf;
> >  }
> >
> >  static void lnl_alpm_configure(struct intel_dp *intel_dp, @@ -385,8
> > +387,7 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp,
> >     enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >     u32 alpm_ctl;
> >
> > -   if (DISPLAY_VER(display) < 20 || (!intel_psr_needs_alpm(intel_dp,
> > crtc_state) &&
> > -                                     !crtc_state->has_lobf))
> > +   if (!crtc_state->has_alpm)
> >             return;
> >
> >     mutex_lock(&intel_dp->alpm.lock);
> > @@ -510,7 +511,7 @@ void intel_alpm_enable_sink(struct intel_dp
> > *intel_dp,  {
> >     u8 val;
> >
> > -   if (!intel_psr_needs_alpm(intel_dp, crtc_state) && !crtc_state-
> > >has_lobf)
> > +   if (!crtc_state->has_alpm)
> >             return;
> >
> >     val = DP_ALPM_ENABLE |
> > DP_ALPM_LOCK_ERROR_IRQ_HPD_ENABLE;
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 00600134bda0..0b60ada4b972 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1392,6 +1392,8 @@ struct intel_crtc_state {
> >             u8 silence_period_sym_clocks;
> >             u8 lfps_half_cycle_num_of_syms;
> >     } alpm_state;
> > +
> > +   bool has_alpm;
> >  };
> >
> >  enum intel_pipe_crc_source {
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 05014ffe3ce1..eeb4255b5e5e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1681,6 +1681,8 @@ static bool intel_sel_update_config_valid(struct
> > intel_dp *intel_dp,
> >     crtc_state->enable_psr2_su_region_et =
> >             psr2_su_region_et_valid(intel_dp, crtc_state-
> > >has_panel_replay);
> >
> > +   crtc_state->has_alpm = intel_alpm_is_possible(intel_dp);
> > +
> >     return true;
> >
> >  unsupported:
> > @@ -1750,11 +1752,17 @@ _panel_replay_compute_config(struct intel_dp
> > *intel_dp,
> >             return false;
> >     }
> >
> > +   if (intel_alpm_is_possible(intel_dp))
> > +           crtc_state->has_alpm = alpm_config_valid(intel_dp,
> > crtc_state, true,
> > +true, false);
> > +
> >     if (!intel_dp_is_edp(intel_dp))
> >             return true;
> >
> >     /* Remaining checks are for eDP only */
> >
> > +   if (!crtc_state->has_alpm)
> > +           return false;
> 
> Why return false here didn't you state that ALPM is optional and PR can Be
> supported without it.

This check is for EDP and ALPM is mandatory for EDP-PR.

Regards,
Animesh

> 
> Regards,
> Suraj Kandpal
> 
> > +
> >     if (to_intel_crtc(crtc_state->uapi.crtc)->pipe != PIPE_A &&
> >         to_intel_crtc(crtc_state->uapi.crtc)->pipe != PIPE_B)
> >             return false;
> > @@ -1777,9 +1785,6 @@ _panel_replay_compute_config(struct intel_dp
> > *intel_dp,
> >             return false;
> >     }
> >
> > -   if (!alpm_config_valid(intel_dp, crtc_state, true, true, false))
> > -           return false;
> > -
> >     return true;
> >  }
> >
> > @@ -4445,16 +4450,6 @@ void intel_psr_connector_debugfs_add(struct
> > intel_connector *connector)
> >                                 connector, &i915_psr_status_fops);  }
> >
> > -bool intel_psr_needs_alpm(struct intel_dp *intel_dp, const struct
> > intel_crtc_state *crtc_state) -{
> > -   /*
> > -    * eDP Panel Replay uses always ALPM
> > -    * PSR2 uses ALPM but PSR1 doesn't
> > -    */
> > -   return intel_dp_is_edp(intel_dp) && (crtc_state->has_sel_update ||
> > -                                        crtc_state->has_panel_replay);
> > -}
> > -
> >  bool intel_psr_needs_alpm_aux_less(struct intel_dp *intel_dp,
> >                                const struct intel_crtc_state *crtc_state)  {
> @@ -4470,7
> > +4465,7 @@ void intel_psr_compute_config_late(struct
> > intel_dp *intel_dp,
> >
> >     if (intel_psr_needs_alpm_aux_less(intel_dp, crtc_state))
> >             wake_lines = crtc_state->alpm_state.aux_less_wake_lines;
> > -   else if (intel_psr_needs_alpm(intel_dp, crtc_state))
> > +   else if (crtc_state->has_alpm)
> >             wake_lines = DISPLAY_VER(display) < 20 ?
> >                          psr2_block_count_lines(crtc_state-
> > >alpm_state.io_wake_lines,
> >                                                 crtc_state-
> > >alpm_state.fast_wake_lines) :
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> > b/drivers/gpu/drm/i915/display/intel_psr.h
> > index 620b35928832..3cb4fa20e427 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> > @@ -80,7 +80,6 @@ void intel_psr_trigger_frame_change_event(struct
> > intel_dsb *dsb,  int intel_psr_min_set_context_latency(const struct
> > intel_crtc_state *crtc_state);  void
> > intel_psr_connector_debugfs_add(struct
> > intel_connector *connector);  void intel_psr_debugfs_register(struct
> > intel_display *display); -bool intel_psr_needs_alpm(struct intel_dp
> > *intel_dp, const struct intel_crtc_state *crtc_state);  bool
> > intel_psr_needs_alpm_aux_less(struct intel_dp *intel_dp,
> >                                const struct intel_crtc_state *crtc_state);
> void
> > intel_psr_compute_config_late(struct intel_dp *intel_dp,
> > --
> > 2.29.0

Reply via email to