[PATCH v5 2/9] drm/i915: Try to program PKG_C_LATENCY more correctly
Shankar, Uma
uma.shankar at intel.com
Tue Jun 24 20:20:06 UTC 2025
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Tuesday, June 24, 2025 10:31 PM
> To: intel-gfx at lists.freedesktop.org
> Cc: intel-xe at lists.freedesktop.org
> Subject: [PATCH v5 2/9] drm/i915: Try to program PKG_C_LATENCY more
> correctly
>
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> The current PKG_C_LATENCY stuff looks busted in several ways:
> - doesn't account for multiple pipes from different commits
> correctly
> - WM_LINETIME is in units of 0.125usec, PKG_C_LATENCY wants
> units on 1 usec
> - weird VRR state stuff being checked
> - use of pointless RMW
>
> Fix it all up. Note that it's still a bit unclear how all this works, especially how the
> added_wake_time ties into the flipq triggers in DMC, and how we need to
AFAIU, this can help wake up early to program Double buffer registers. Maybe we can
just program it to our worst case DSB execution time (~100us).
> sequence updates to PKG_C_LATENCY when enabling/disabling pipes/etc. We
I guess safe would be to just disable PKG_C when doing the same.
> may also need to think what to about the WM1+ disabling and the related PSR
> chicken bits when we can use PKG_C_LATENCY for early wake...
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 1 +
> .../gpu/drm/i915/display/intel_display_core.h | 6 ++
> drivers/gpu/drm/i915/display/skl_watermark.c | 97 +++++++++++--------
> 3 files changed, 61 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index de8bf292897c..72407cfffb60 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7393,6 +7393,7 @@ static void intel_atomic_commit_tail(struct
> intel_atomic_state *state)
> /* Now enable the clocks, plane, pipe, and connectors that we set up. */
> display->funcs.display->commit_modeset_enables(state);
>
> + /* FIXME probably need to sequence this properly */
> intel_program_dpkgc_latency(state);
>
> intel_wait_for_vblank_workers(state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h
> b/drivers/gpu/drm/i915/display/intel_display_core.h
> index 32cb0e59c81e..ad4d29e2af1b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -479,6 +479,12 @@ struct intel_display {
> u32 pipestat_irq_mask[I915_MAX_PIPES];
> } irq;
>
> + struct {
> + /* protected by wm.wm_mutex */
> + u16 linetime[I915_MAX_PIPES];
> + bool disable[I915_MAX_PIPES];
> + } pkgc;
> +
> struct {
> wait_queue_head_t waitqueue;
>
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index a71605e3a535..8184ec2611e2 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -2903,64 +2903,75 @@ static int skl_wm_add_affected_planes(struct
> intel_atomic_state *state,
> return 0;
> }
>
> -/*
> - * If Fixed Refresh Rate or For VRR case Vmin = Vmax = Flipline:
> - * Program DEEP PKG_C_LATENCY Pkg C with highest valid latency from
> - * watermark level1 and up and above. If watermark level 1 is
> - * invalid program it with all 1's.
> - * Program PKG_C_LATENCY Added Wake Time = DSB execution time
> - * If Variable Refresh Rate where Vmin != Vmax != Flipline:
> - * Program DEEP PKG_C_LATENCY Pkg C with all 1's.
> - * Program PKG_C_LATENCY Added Wake Time = 0
> - */
> +static int pkgc_max_linetime(struct intel_atomic_state *state) {
> + struct intel_display *display = to_intel_display(state);
> + const struct intel_crtc_state *crtc_state;
> + struct intel_crtc *crtc;
> + int i, max_linetime;
> +
> + /*
> + * Apparenty the hardware uses WM_LINETIME internally for
> + * this stuff, compute everything based on that.
> + */
> + for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> + display->pkgc.disable[crtc->pipe] = crtc_state->vrr.enable;
> + display->pkgc.linetime[crtc->pipe] = DIV_ROUND_UP(crtc_state-
> >linetime, 8);
> + }
> +
> + max_linetime = 0;
> + for_each_intel_crtc(display->drm, crtc) {
> + if (display->pkgc.disable[crtc->pipe])
> + return 0;
> +
> + max_linetime = max(display->pkgc.linetime[crtc->pipe],
> max_linetime);
> + }
> +
> + return max_linetime;
> +}
> +
> void
> intel_program_dpkgc_latency(struct intel_atomic_state *state) {
> struct intel_display *display = to_intel_display(state);
> - struct intel_crtc *crtc;
> - struct intel_crtc_state *new_crtc_state;
> - u32 latency = LNL_PKG_C_LATENCY_MASK;
> - u32 added_wake_time = 0;
> - u32 max_linetime = 0;
> - u32 clear, val;
> - bool fixed_refresh_rate = false;
> - int i;
> + int max_linetime, latency, added_wake_time = 0;
>
> if (DISPLAY_VER(display) < 20)
> return;
>
> - for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> - if (!new_crtc_state->vrr.enable ||
> - (new_crtc_state->vrr.vmin == new_crtc_state->vrr.vmax &&
> - new_crtc_state->vrr.vmin == new_crtc_state->vrr.flipline))
> - fixed_refresh_rate = true;
> + mutex_lock(&display->wm.wm_mutex);
>
> - max_linetime = max(new_crtc_state->linetime, max_linetime);
> - }
> + latency = skl_watermark_max_latency(display, 1);
>
> - if (fixed_refresh_rate) {
> - latency = skl_watermark_max_latency(display, 1);
> + /*
> + * Wa_22020432604
> + * "PKG_C_LATENCY Added Wake Time field is not working"
> + */
> + if (latency && (DISPLAY_VER(display) == 20 || DISPLAY_VER(display)
> == 30)) {
> + latency += added_wake_time;
> + added_wake_time = 0;
We haven't assigned any value to added_wake_time, do we really need this reset ?
Overall changes look good to me.
Reviewed-by: Uma Shankar <uma.shankar at intel.com>
> + }
>
> - /* Wa_22020432604 */
> - if ((DISPLAY_VER(display) == 20 || DISPLAY_VER(display) ==
> 30) && !latency) {
> - latency += added_wake_time;
> - added_wake_time = 0;
> - }
> + max_linetime = pkgc_max_linetime(state);
>
> - /* Wa_22020299601 */
> - if ((latency && max_linetime) &&
> - (DISPLAY_VER(display) == 20 || DISPLAY_VER(display) ==
> 30)) {
> - latency = max_linetime * DIV_ROUND_UP(latency,
> max_linetime);
> - } else if (!latency) {
> - latency = LNL_PKG_C_LATENCY_MASK;
> - }
> + if (max_linetime == 0 || latency == 0) {
> + latency = REG_FIELD_GET(LNL_PKG_C_LATENCY_MASK,
> + LNL_PKG_C_LATENCY_MASK);
> + added_wake_time = 0;
> + } else {
> + /*
> + * Wa_22020299601
> + * "Increase the latency programmed in PKG_C_LATENCY Pkg C
> Latency to be a
> + * multiple of the pipeline time from WM_LINETIME"
> + */
> + latency = roundup(latency, max_linetime);
> }
>
> - clear = LNL_ADDED_WAKE_TIME_MASK |
> LNL_PKG_C_LATENCY_MASK;
> - val = REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK, latency) |
> - REG_FIELD_PREP(LNL_ADDED_WAKE_TIME_MASK,
> added_wake_time);
> + intel_de_write(display, LNL_PKG_C_LATENCY,
> + REG_FIELD_PREP(LNL_ADDED_WAKE_TIME_MASK,
> added_wake_time) |
> + REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK,
> latency));
>
> - intel_de_rmw(display, LNL_PKG_C_LATENCY, clear, val);
> + mutex_unlock(&display->wm.wm_mutex);
> }
>
> static int
> --
> 2.49.0
More information about the Intel-xe
mailing list