From: "Hogander, Jouni" <jouni.hogander@intel.com>
To: "intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>,
"Manna, Animesh" <animesh.manna@intel.com>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v2 1/2] drm/i915/alpm: Compute ALPM parameters into crtc_state->alpm_state
Date: Thu, 9 Oct 2025 06:49:47 +0000 [thread overview]
Message-ID: <ace23df2dbe0dd6dfe193e9f8184f7189463ed24.camel@intel.com> (raw)
In-Reply-To: <DS0PR11MB80496FBCD0575803A7DB8C94F9E0A@DS0PR11MB8049.namprd11.prod.outlook.com>
On Tue, 2025-10-07 at 09:28 +0000, Manna, Animesh wrote:
>
>
> > -----Original Message-----
> > From: Hogander, Jouni <jouni.hogander@intel.com>
> > Sent: Monday, September 29, 2025 6:30 PM
> > To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > Cc: Manna, Animesh <animesh.manna@intel.com>; Hogander, Jouni
> > <jouni.hogander@intel.com>
> > Subject: [PATCH v2 1/2] drm/i915/alpm: Compute ALPM parameters into
> > crtc_state->alpm_state
> >
> > Currently ALPM parameters are computed directly into intel_dp-
> > > alpm_parameters. This is a problem when compute config ends up to
> > > not
> > using the computed state.
> >
> > Fix this by adding ALPM parameters into intel_crtc_state and
> > compute into
> > there. Copy needed parameters (io_wake_lines and fast_wake_lines)
> > from
> > crtc_state->alpm_state into intel_dp->alpm.alpm_parameters
> > (io_wake_lines
> > and fast_wake_lines) when they are configured into HW.
>
> Good to add why copy needed for these two parameters in commit
> description.
> Other changes looks good to me. With the above fix.
>
> Reviewed-by: Animesh Manna <animesh.manna@intel.com>
Thank you Animesh for the review. These are both now pushed to drm-
intel-next with you suggestion here included.
BR,
Jouni Högander
>
> >
> > v2:
> > - store io/fast wake lines into intel_dp->dp instead of
> > intel_dp->alpm_parameters and do it in intel_psr_enable_locked
> > - rename crtc_state->alpm_parameters -> crtc_state->alpm_state
> > - clarify commit message
> >
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_alpm.c | 36 +++++++++------
> > ----
> > drivers/gpu/drm/i915/display/intel_alpm.h | 2 +-
> > .../drm/i915/display/intel_display_types.h | 21 +++++++----
> > drivers/gpu/drm/i915/display/intel_psr.c | 36 ++++++++++-----
> > ----
> > 4 files changed, 53 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index 749119cc0b28..9f4a9d8b4dec 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -122,7 +122,7 @@ static int
> > _lnl_compute_aux_less_wake_time(const
> > struct intel_crtc_state *crtc_s
> >
> > static int
> > _lnl_compute_aux_less_alpm_params(struct intel_dp *intel_dp,
> > - const struct intel_crtc_state
> > *crtc_state)
> > + struct intel_crtc_state
> > *crtc_state)
> > {
> > struct intel_display *display =
> > to_intel_display(intel_dp);
> > int aux_less_wake_time, aux_less_wake_lines,
> > silence_period, @@ -
> > 144,15 +144,15 @@ _lnl_compute_aux_less_alpm_params(struct intel_dp
> > *intel_dp,
> > if (display->params.psr_safest_params)
> > aux_less_wake_lines =
> > ALPM_CTL_AUX_LESS_WAKE_TIME_MASK;
> >
> > - intel_dp->alpm_parameters.aux_less_wake_lines =
> > aux_less_wake_lines;
> > - intel_dp->alpm_parameters.silence_period_sym_clocks =
> > silence_period;
> > - intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms =
> > lfps_half_cycle;
> > + crtc_state->alpm_state.aux_less_wake_lines =
> > aux_less_wake_lines;
> > + crtc_state->alpm_state.silence_period_sym_clocks =
> > silence_period;
> > + crtc_state->alpm_state.lfps_half_cycle_num_of_syms =
> > lfps_half_cycle;
> >
> > return true;
> > }
> >
> > static bool _lnl_compute_alpm_params(struct intel_dp *intel_dp,
> > - const struct intel_crtc_state
> > *crtc_state)
> > + struct intel_crtc_state
> > *crtc_state)
> > {
> > struct intel_display *display =
> > to_intel_display(intel_dp);
> > int check_entry_lines;
> > @@ -173,7 +173,7 @@ static bool _lnl_compute_alpm_params(struct
> > intel_dp *intel_dp,
> > if (display->params.psr_safest_params)
> > check_entry_lines = 15;
> >
> > - intel_dp->alpm_parameters.check_entry_lines =
> > check_entry_lines;
> > + crtc_state->alpm_state.check_entry_lines =
> > check_entry_lines;
> >
> > return true;
> > }
> > @@ -204,7 +204,7 @@ static int io_buffer_wake_time(const struct
> > intel_crtc_state *crtc_state) }
> >
> > bool intel_alpm_compute_params(struct intel_dp *intel_dp,
> > - const struct intel_crtc_state
> > *crtc_state)
> > + struct intel_crtc_state
> > *crtc_state)
> > {
> > struct intel_display *display =
> > to_intel_display(intel_dp);
> > int io_wake_lines, io_wake_time, fast_wake_lines,
> > fast_wake_time;
> > @@ -242,8 +242,8 @@ bool intel_alpm_compute_params(struct intel_dp
> > *intel_dp,
> > io_wake_lines = fast_wake_lines = max_wake_lines;
> >
> > /* According to Bspec lower limit should be set as 7
> > lines. */
> > - intel_dp->alpm_parameters.io_wake_lines =
> > max(io_wake_lines, 7);
> > - intel_dp->alpm_parameters.fast_wake_lines =
> > max(fast_wake_lines,
> > 7);
> > + crtc_state->alpm_state.io_wake_lines = max(io_wake_lines,
> > 7);
> > + crtc_state->alpm_state.fast_wake_lines =
> > max(fast_wake_lines, 7);
> >
> > return true;
> > }
> > @@ -293,9 +293,9 @@ void intel_alpm_lobf_compute_config(struct
> > intel_dp
> > *intel_dp,
> > adjusted_mode->crtc_vdisplay -
> > context_latency;
> > first_sdp_position = adjusted_mode->crtc_vtotal -
> > adjusted_mode-
> > > crtc_vsync_start;
> > if (intel_alpm_aux_less_wake_supported(intel_dp))
> > - waketime_in_lines = intel_dp-
> > > alpm_parameters.io_wake_lines;
> > + waketime_in_lines = crtc_state-
> > >alpm_state.io_wake_lines;
> > else
> > - waketime_in_lines = intel_dp-
> > > alpm_parameters.aux_less_wake_lines;
> > + waketime_in_lines = crtc_state-
> > > alpm_state.aux_less_wake_lines;
> >
> > crtc_state->has_lobf = (context_latency + guardband) >
> > (first_sdp_position + waketime_in_lines); @@ -
> > 321,7 +321,7
> > @@ static void lnl_alpm_configure(struct intel_dp *intel_dp,
> > alpm_ctl = ALPM_CTL_ALPM_ENABLE |
> > ALPM_CTL_ALPM_AUX_LESS_ENABLE |
> >
> > ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_50_SYMBOLS |
> > - ALPM_CTL_AUX_LESS_WAKE_TIME(intel_dp-
> > > alpm_parameters.aux_less_wake_lines);
> > +
> > +ALPM_CTL_AUX_LESS_WAKE_TIME(crtc_state-
> > > alpm_state.aux_less_wake_lines)
> > +;
> >
> > if (intel_dp->as_sdp_supported) {
> > u32 pr_alpm_ctl =
> > PR_ALPM_CTL_ADAPTIVE_SYNC_SDP_POSITION_T1;
> > @@ -339,7 +339,7 @@ static void lnl_alpm_configure(struct intel_dp
> > *intel_dp,
> >
> > } else {
> > alpm_ctl = ALPM_CTL_EXTENDED_FAST_WAKE_ENABLE |
> > - ALPM_CTL_EXTENDED_FAST_WAKE_TIME(intel_dp-
> > > alpm_parameters.fast_wake_lines);
> > +
> > +ALPM_CTL_EXTENDED_FAST_WAKE_TIME(crtc_state-
> > > alpm_state.fast_wake_lines
> > +);
> > }
> >
> > if (crtc_state->has_lobf) {
> > @@ -347,7 +347,7 @@ static void lnl_alpm_configure(struct intel_dp
> > *intel_dp,
> > drm_dbg_kms(display->drm, "Link off between frames
> > (LOBF) enabled\n");
> > }
> >
> > - alpm_ctl |= ALPM_CTL_ALPM_ENTRY_CHECK(intel_dp-
> > > alpm_parameters.check_entry_lines);
> > + alpm_ctl |=
> > +ALPM_CTL_ALPM_ENTRY_CHECK(crtc_state-
> > > alpm_state.check_entry_lines);
> >
> > intel_de_write(display, ALPM_CTL(display, cpu_transcoder),
> > alpm_ctl);
> > mutex_unlock(&intel_dp->alpm_parameters.lock);
> > @@ -375,14 +375,14 @@ void intel_alpm_port_configure(struct
> > intel_dp
> > *intel_dp,
> > PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15) |
> > PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(0) |
> > PORT_ALPM_CTL_SILENCE_PERIOD(
> > - intel_dp-
> > > alpm_parameters.silence_period_sym_clocks);
> > + crtc_state-
> > > alpm_state.silence_period_sym_clocks);
> > lfps_ctl_val =
> > PORT_ALPM_LFPS_CTL_LFPS_CYCLE_COUNT(LFPS_CYCLE_COUNT) |
> >
> > PORT_ALPM_LFPS_CTL_LFPS_HALF_CYCLE_DURATION(
> > - intel_dp-
> > > alpm_parameters.lfps_half_cycle_num_of_syms) |
> > + crtc_state-
> > > alpm_state.lfps_half_cycle_num_of_syms) |
> >
> > PORT_ALPM_LFPS_CTL_FIRST_LFPS_HALF_CYCLE_DURATION(
> > - intel_dp-
> > > alpm_parameters.lfps_half_cycle_num_of_syms) |
> > + crtc_state-
> > > alpm_state.lfps_half_cycle_num_of_syms) |
> >
> > PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION(
> > - intel_dp-
> > > alpm_parameters.lfps_half_cycle_num_of_syms);
> > + crtc_state-
> > > alpm_state.lfps_half_cycle_num_of_syms);
> > }
> >
> > intel_de_write(display, PORT_ALPM_CTL(port),
> > alpm_ctl_val); diff --
> > git a/drivers/gpu/drm/i915/display/intel_alpm.h
> > b/drivers/gpu/drm/i915/display/intel_alpm.h
> > index a861c20b5d79..53599b464dea 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.h
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h
> > @@ -17,7 +17,7 @@ struct intel_crtc;
> >
> > void intel_alpm_init(struct intel_dp *intel_dp); bool
> > intel_alpm_compute_params(struct intel_dp *intel_dp,
> > - const struct intel_crtc_state
> > *crtc_state);
> > + struct intel_crtc_state
> > *crtc_state);
> > void intel_alpm_lobf_compute_config(struct intel_dp *intel_dp,
> > struct intel_crtc_state
> > *crtc_state,
> > struct drm_connector_state
> > *conn_state);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 029c47743f8b..57879cc5546e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1344,6 +1344,17 @@ struct intel_crtc_state {
> >
> > /* W2 window or 'set context latency' lines */
> > u16 set_context_latency;
> > +
> > + struct {
> > + u8 io_wake_lines;
> > + u8 fast_wake_lines;
> > +
> > + /* LNL and beyond */
> > + u8 check_entry_lines;
> > + u8 aux_less_wake_lines;
> > + u8 silence_period_sym_clocks;
> > + u8 lfps_half_cycle_num_of_syms;
> > + } alpm_state;
> > };
> >
> > enum intel_pipe_crc_source {
> > @@ -1688,6 +1699,9 @@ struct intel_psr {
> > struct delayed_work dc3co_work;
> > u8 entry_setup_frames;
> >
> > + u8 io_wake_lines;
> > + u8 fast_wake_lines;
> > +
> > bool link_ok;
> > bool pkg_c_latency_used;
> >
> > @@ -1847,16 +1861,9 @@ struct intel_dp {
> > bool colorimetry_support;
> >
> > struct {
> > - u8 io_wake_lines;
> > - u8 fast_wake_lines;
> > enum transcoder transcoder;
> > struct mutex lock;
> >
> > - /* LNL and beyond */
> > - u8 check_entry_lines;
> > - u8 aux_less_wake_lines;
> > - u8 silence_period_sym_clocks;
> > - u8 lfps_half_cycle_num_of_syms;
> > bool lobf_disable_debug;
> > bool sink_alpm_error;
> > } alpm_parameters;
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index f7115969b4c5..2131473cead6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -956,15 +956,16 @@ static u32 intel_psr2_get_tp_time(struct
> > intel_dp
> > *intel_dp)
> > return val;
> > }
> >
> > -static int psr2_block_count_lines(struct intel_dp *intel_dp)
> > +static int
> > +psr2_block_count_lines(u8 io_wake_lines, u8 fast_wake_lines)
> > {
> > - return intel_dp->alpm_parameters.io_wake_lines < 9 &&
> > - intel_dp->alpm_parameters.fast_wake_lines < 9 ? 8
> > : 12;
> > + return io_wake_lines < 9 && fast_wake_lines < 9 ? 8 : 12;
> > }
> >
> > static int psr2_block_count(struct intel_dp *intel_dp) {
> > - return psr2_block_count_lines(intel_dp) / 4;
> > + return psr2_block_count_lines(intel_dp->psr.io_wake_lines,
> > + intel_dp-
> > >psr.fast_wake_lines) / 4;
> > }
> >
> > static u8 frames_before_su_entry(struct intel_dp *intel_dp) @@ -
> > 1059,20
> > +1060,20 @@ static void hsw_activate_psr2(struct intel_dp
> > *intel_dp)
> > */
> > int tmp;
> >
> > - tmp = map[intel_dp->alpm_parameters.io_wake_lines
> > -
> > + tmp = map[intel_dp->psr.io_wake_lines -
> > TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
> > val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(tmp +
> > TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES);
> >
> > - tmp = map[intel_dp-
> > >alpm_parameters.fast_wake_lines -
> > TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
> > + tmp = map[intel_dp->psr.fast_wake_lines -
> > +TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
> > val |= TGL_EDP_PSR2_FAST_WAKE(tmp +
> > TGL_EDP_PSR2_FAST_WAKE_MIN_LINES);
> > } else if (DISPLAY_VER(display) >= 20) {
> > - val |= LNL_EDP_PSR2_IO_BUFFER_WAKE(intel_dp-
> > > alpm_parameters.io_wake_lines);
> > + val |= LNL_EDP_PSR2_IO_BUFFER_WAKE(intel_dp-
> > > psr.io_wake_lines);
> > } else if (DISPLAY_VER(display) >= 12) {
> > - val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(intel_dp-
> > > alpm_parameters.io_wake_lines);
> > - val |= TGL_EDP_PSR2_FAST_WAKE(intel_dp-
> > > alpm_parameters.fast_wake_lines);
> > + val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(intel_dp-
> > > psr.io_wake_lines);
> > + val |= TGL_EDP_PSR2_FAST_WAKE(intel_dp-
> > > psr.fast_wake_lines);
> > } else if (DISPLAY_VER(display) >= 9) {
> > - val |= EDP_PSR2_IO_BUFFER_WAKE(intel_dp-
> > > alpm_parameters.io_wake_lines);
> > - val |= EDP_PSR2_FAST_WAKE(intel_dp-
> > > alpm_parameters.fast_wake_lines);
> > + val |= EDP_PSR2_IO_BUFFER_WAKE(intel_dp-
> > > psr.io_wake_lines);
> > + val |= EDP_PSR2_FAST_WAKE(intel_dp-
> > > psr.fast_wake_lines);
> > }
> >
> > if (intel_dp->psr.req_psr2_sdp_prior_scanline)
> > @@ -1370,11 +1371,12 @@ static bool
> > wake_lines_fit_into_vblank(struct
> > intel_dp *intel_dp,
> > int wake_lines;
> >
> > if (aux_less)
> > - wake_lines = intel_dp-
> > > alpm_parameters.aux_less_wake_lines;
> > + wake_lines = crtc_state-
> > >alpm_state.aux_less_wake_lines;
> > else
> > wake_lines = DISPLAY_VER(display) < 20 ?
> > - psr2_block_count_lines(intel_dp) :
> > - intel_dp->alpm_parameters.io_wake_lines;
> > + psr2_block_count_lines(crtc_state-
> > > alpm_state.io_wake_lines,
> > + crtc_state-
> > > alpm_state.fast_wake_lines) :
> > + crtc_state->alpm_state.io_wake_lines;
> >
> > if (crtc_state->req_psr2_sdp_prior_scanline)
> > vblank -= 1;
> > @@ -1387,7 +1389,7 @@ static bool wake_lines_fit_into_vblank(struct
> > intel_dp *intel_dp, }
> >
> > static bool alpm_config_valid(struct intel_dp *intel_dp,
> > - const struct intel_crtc_state
> > *crtc_state,
> > + struct intel_crtc_state *crtc_state,
> > bool aux_less)
> > {
> > struct intel_display *display =
> > to_intel_display(intel_dp); @@ -1592,7
> > +1594,7 @@ static bool _psr_compute_config(struct intel_dp
> > *intel_dp,
> >
> > static bool
> > _panel_replay_compute_config(struct intel_dp *intel_dp,
> > - const struct intel_crtc_state
> > *crtc_state,
> > + struct intel_crtc_state *crtc_state,
> > const struct drm_connector_state
> > *conn_state) {
> > struct intel_display *display =
> > to_intel_display(intel_dp); @@ -2022,6
> > +2024,8 @@ static void intel_psr_enable_locked(struct intel_dp
> > *intel_dp,
> > crtc_state->req_psr2_sdp_prior_scanline;
> > intel_dp->psr.active_non_psr_pipes = crtc_state-
> > > active_non_psr_pipes;
> > intel_dp->psr.pkg_c_latency_used = crtc_state-
> > >pkg_c_latency_used;
> > + intel_dp->psr.io_wake_lines = crtc_state-
> > >alpm_state.io_wake_lines;
> > + intel_dp->psr.fast_wake_lines =
> > +crtc_state->alpm_state.fast_wake_lines;
> >
> > if (!psr_interrupt_error_check(intel_dp))
> > return;
> > --
> > 2.43.0
>
prev parent reply other threads:[~2025-10-09 6:49 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-29 13:00 [PATCH v2 1/2] drm/i915/alpm: Compute ALPM parameters into crtc_state->alpm_state Jouni Högander
2025-09-29 13:00 ` [PATCH v2 2/2] drm/i915/alpm: Remove parameters suffix from intel_dp->alpm_parameters Jouni Högander
2025-10-07 9:31 ` Manna, Animesh
2025-09-29 15:08 ` ✓ i915.CI.BAT: success for series starting with [v2,1/2] drm/i915/alpm: Compute ALPM parameters into crtc_state->alpm_state Patchwork
2025-09-29 18:13 ` ✗ i915.CI.Full: failure " Patchwork
2025-10-09 6:12 ` Hogander, Jouni
2025-10-07 9:28 ` [PATCH v2 1/2] " Manna, Animesh
2025-10-09 6:49 ` Hogander, Jouni [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ace23df2dbe0dd6dfe193e9f8184f7189463ed24.camel@intel.com \
--to=jouni.hogander@intel.com \
--cc=animesh.manna@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).