From 92512d4827f140fcdd8ae86639858f05c8f21ecc Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 26 Mar 2025 18:25:32 +0200 Subject: [PATCH 01/16] drm/i915: s/intel_crtc_bw/intel_dbuf_bw/ MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Rename the intel_crtc_bw struct to intel_dbuf_bw to better reflect what it does. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250326162544.3642-3-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_bw.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index 15c2377193f7..b34db55f5a7e 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -1157,15 +1157,15 @@ static bool intel_bw_state_changed(struct intel_display *display, enum pipe pipe; for_each_pipe(display, pipe) { - const struct intel_dbuf_bw *old_crtc_bw = + const struct intel_dbuf_bw *old_dbuf_bw = &old_bw_state->dbuf_bw[pipe]; - const struct intel_dbuf_bw *new_crtc_bw = + const struct intel_dbuf_bw *new_dbuf_bw = &new_bw_state->dbuf_bw[pipe]; enum dbuf_slice slice; for_each_dbuf_slice(display, slice) { - if (old_crtc_bw->max_bw[slice] != new_crtc_bw->max_bw[slice] || - old_crtc_bw->active_planes[slice] != new_crtc_bw->active_planes[slice]) + if (old_dbuf_bw->max_bw[slice] != new_dbuf_bw->max_bw[slice] || + old_dbuf_bw->active_planes[slice] != new_dbuf_bw->active_planes[slice]) return true; } @@ -1185,7 +1185,7 @@ static void skl_plane_calc_dbuf_bw(struct intel_bw_state *bw_state, { struct intel_display *display = to_intel_display(crtc); struct drm_i915_private *i915 = to_i915(display->drm); - struct intel_dbuf_bw *crtc_bw = &bw_state->dbuf_bw[crtc->pipe]; + struct intel_dbuf_bw *dbuf_bw = &bw_state->dbuf_bw[crtc->pipe]; unsigned int dbuf_mask = skl_ddb_dbuf_slice_mask(i915, ddb); enum dbuf_slice slice; @@ -1194,8 +1194,8 @@ static void skl_plane_calc_dbuf_bw(struct intel_bw_state *bw_state, * equal share of the total bw to each plane. */ for_each_dbuf_slice_in_mask(display, slice, dbuf_mask) { - crtc_bw->max_bw[slice] = max(crtc_bw->max_bw[slice], data_rate); - crtc_bw->active_planes[slice] |= BIT(plane_id); + dbuf_bw->max_bw[slice] = max(dbuf_bw->max_bw[slice], data_rate); + dbuf_bw->active_planes[slice] |= BIT(plane_id); } } @@ -1204,10 +1204,10 @@ static void skl_crtc_calc_dbuf_bw(struct intel_bw_state *bw_state, { struct intel_display *display = to_intel_display(crtc_state); struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); - struct intel_dbuf_bw *crtc_bw = &bw_state->dbuf_bw[crtc->pipe]; + struct intel_dbuf_bw *dbuf_bw = &bw_state->dbuf_bw[crtc->pipe]; enum plane_id plane_id; - memset(crtc_bw, 0, sizeof(*crtc_bw)); + memset(dbuf_bw, 0, sizeof(*dbuf_bw)); if (!crtc_state->hw.active) return; @@ -1249,10 +1249,10 @@ intel_bw_dbuf_min_cdclk(struct intel_display *display, * equal share of the total bw to each plane. */ for_each_pipe(display, pipe) { - const struct intel_dbuf_bw *crtc_bw = &bw_state->dbuf_bw[pipe]; + const struct intel_dbuf_bw *dbuf_bw = &bw_state->dbuf_bw[pipe]; - max_bw = max(crtc_bw->max_bw[slice], max_bw); - num_active_planes += hweight8(crtc_bw->active_planes[slice]); + max_bw = max(dbuf_bw->max_bw[slice], max_bw); + num_active_planes += hweight8(dbuf_bw->active_planes[slice]); } max_bw *= num_active_planes; -- 2.51.0 From 8261fbacd9bb523ddebc3bee624f36c7d8d0b8d8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 26 Mar 2025 18:25:33 +0200 Subject: [PATCH 02/16] drm/i915: Extract intel_dbuf_bw_changed() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Extract the struct intel_dbuf_bw comparison into a small helper. We'll get more users later. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250326162544.3642-4-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_bw.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index b34db55f5a7e..898ddaf7e76b 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -1150,6 +1150,21 @@ static int intel_bw_check_qgv_points(struct intel_display *display, old_bw_state, new_bw_state); } +static bool intel_dbuf_bw_changed(struct intel_display *display, + const struct intel_dbuf_bw *old_dbuf_bw, + const struct intel_dbuf_bw *new_dbuf_bw) +{ + enum dbuf_slice slice; + + for_each_dbuf_slice(display, slice) { + if (old_dbuf_bw->max_bw[slice] != new_dbuf_bw->max_bw[slice] || + old_dbuf_bw->active_planes[slice] != new_dbuf_bw->active_planes[slice]) + return true; + } + + return false; +} + static bool intel_bw_state_changed(struct intel_display *display, const struct intel_bw_state *old_bw_state, const struct intel_bw_state *new_bw_state) @@ -1161,13 +1176,9 @@ static bool intel_bw_state_changed(struct intel_display *display, &old_bw_state->dbuf_bw[pipe]; const struct intel_dbuf_bw *new_dbuf_bw = &new_bw_state->dbuf_bw[pipe]; - enum dbuf_slice slice; - for_each_dbuf_slice(display, slice) { - if (old_dbuf_bw->max_bw[slice] != new_dbuf_bw->max_bw[slice] || - old_dbuf_bw->active_planes[slice] != new_dbuf_bw->active_planes[slice]) - return true; - } + if (intel_dbuf_bw_changed(display, old_dbuf_bw, new_dbuf_bw)) + return true; if (intel_bw_crtc_min_cdclk(display, old_bw_state->data_rate[pipe]) != intel_bw_crtc_min_cdclk(display, new_bw_state->data_rate[pipe])) -- 2.51.0 From 18e6866615698f9dd714e47353022bd597a90a57 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 26 Mar 2025 18:25:34 +0200 Subject: [PATCH 03/16] drm/i915: Pass intel_dbuf_bw to skl_*_calc_dbuf_bw() explicitly MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Make skl_*_calc_dbuf_bw() a bit lower level passing in the to be mutated dbuf_bw struct in explicitly. This will allow more reuse later. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250326162544.3642-5-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_bw.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index 898ddaf7e76b..67d088da1f38 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -1188,7 +1188,7 @@ static bool intel_bw_state_changed(struct intel_display *display, return false; } -static void skl_plane_calc_dbuf_bw(struct intel_bw_state *bw_state, +static void skl_plane_calc_dbuf_bw(struct intel_dbuf_bw *dbuf_bw, struct intel_crtc *crtc, enum plane_id plane_id, const struct skl_ddb_entry *ddb, @@ -1196,7 +1196,6 @@ static void skl_plane_calc_dbuf_bw(struct intel_bw_state *bw_state, { struct intel_display *display = to_intel_display(crtc); struct drm_i915_private *i915 = to_i915(display->drm); - struct intel_dbuf_bw *dbuf_bw = &bw_state->dbuf_bw[crtc->pipe]; unsigned int dbuf_mask = skl_ddb_dbuf_slice_mask(i915, ddb); enum dbuf_slice slice; @@ -1210,12 +1209,11 @@ static void skl_plane_calc_dbuf_bw(struct intel_bw_state *bw_state, } } -static void skl_crtc_calc_dbuf_bw(struct intel_bw_state *bw_state, +static void skl_crtc_calc_dbuf_bw(struct intel_dbuf_bw *dbuf_bw, const struct intel_crtc_state *crtc_state) { struct intel_display *display = to_intel_display(crtc_state); struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); - struct intel_dbuf_bw *dbuf_bw = &bw_state->dbuf_bw[crtc->pipe]; enum plane_id plane_id; memset(dbuf_bw, 0, sizeof(*dbuf_bw)); @@ -1231,12 +1229,12 @@ static void skl_crtc_calc_dbuf_bw(struct intel_bw_state *bw_state, if (plane_id == PLANE_CURSOR) continue; - skl_plane_calc_dbuf_bw(bw_state, crtc, plane_id, + skl_plane_calc_dbuf_bw(dbuf_bw, crtc, plane_id, &crtc_state->wm.skl.plane_ddb[plane_id], crtc_state->data_rate[plane_id]); if (DISPLAY_VER(display) < 11) - skl_plane_calc_dbuf_bw(bw_state, crtc, plane_id, + skl_plane_calc_dbuf_bw(dbuf_bw, crtc, plane_id, &crtc_state->wm.skl.plane_ddb_y[plane_id], crtc_state->data_rate[plane_id]); } @@ -1311,7 +1309,8 @@ int intel_bw_calc_min_cdclk(struct intel_atomic_state *state, old_bw_state = intel_atomic_get_old_bw_state(state); - skl_crtc_calc_dbuf_bw(new_bw_state, crtc_state); + skl_crtc_calc_dbuf_bw(&new_bw_state->dbuf_bw[crtc->pipe], + crtc_state); } if (!old_bw_state) -- 2.51.0 From 074c31271a1d9ec74c5a1965589671337be11d0f Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 26 Mar 2025 18:25:35 +0200 Subject: [PATCH 04/16] drm/i915: Avoid triggering unwanted cdclk changes due to dbuf bandwidth changes MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Currently intel_bw_calc_min_cdclk() always adds the bw_state to the atomic state. Not only does it result in potentially redundant work later, it's also currently causing unwanted cdclk changes during driver load. Check if the dbuf bw is actually changing before we decide to pull in the bw state. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250326162544.3642-6-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_bw.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index 67d088da1f38..19b516084fac 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -1294,7 +1294,8 @@ int intel_bw_calc_min_cdclk(struct intel_atomic_state *state, struct intel_bw_state *new_bw_state = NULL; const struct intel_bw_state *old_bw_state = NULL; const struct intel_cdclk_state *cdclk_state; - const struct intel_crtc_state *crtc_state; + const struct intel_crtc_state *old_crtc_state; + const struct intel_crtc_state *new_crtc_state; int old_min_cdclk, new_min_cdclk; struct intel_crtc *crtc; int i; @@ -1302,15 +1303,23 @@ int intel_bw_calc_min_cdclk(struct intel_atomic_state *state, if (DISPLAY_VER(display) < 9) return 0; - for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) { + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, + new_crtc_state, i) { + struct intel_dbuf_bw old_dbuf_bw, new_dbuf_bw; + + skl_crtc_calc_dbuf_bw(&old_dbuf_bw, old_crtc_state); + skl_crtc_calc_dbuf_bw(&new_dbuf_bw, new_crtc_state); + + if (!intel_dbuf_bw_changed(display, &old_dbuf_bw, &new_dbuf_bw)) + continue; + new_bw_state = intel_atomic_get_bw_state(state); if (IS_ERR(new_bw_state)) return PTR_ERR(new_bw_state); old_bw_state = intel_atomic_get_old_bw_state(state); - skl_crtc_calc_dbuf_bw(&new_bw_state->dbuf_bw[crtc->pipe], - crtc_state); + new_bw_state->dbuf_bw[crtc->pipe] = new_dbuf_bw; } if (!old_bw_state) -- 2.51.0 From 0029d2f739383fcd3269d5ac34d5290c15ac8213 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 26 Mar 2025 18:25:36 +0200 Subject: [PATCH 05/16] drm/i915: Do more bw readout MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Update a bunch of bw related stuff during readout: - bw_state->dbuf_bw possible now that the wm readout has given us access to the plane ddb data - cdclk_state->bw_min_cdclk Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250326162544.3642-7-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_bw.c | 3 +++ drivers/gpu/drm/i915/display/intel_cdclk.c | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index 19b516084fac..69f3de0bba6a 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -1495,6 +1495,8 @@ void intel_bw_update_hw_state(struct intel_display *display) if (DISPLAY_VER(display) >= 11) intel_bw_crtc_update(bw_state, crtc_state); + + skl_crtc_calc_dbuf_bw(&bw_state->dbuf_bw[pipe], crtc_state); } } @@ -1510,6 +1512,7 @@ void intel_bw_crtc_disable_noatomic(struct intel_crtc *crtc) bw_state->data_rate[pipe] = 0; bw_state->num_active_planes[pipe] = 0; + memset(&bw_state->dbuf_bw[pipe], 0, sizeof(bw_state->dbuf_bw[pipe])); } static struct intel_global_state * diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 984fd9f98c9f..ea2fbee2d62f 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -3341,6 +3341,8 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state) void intel_cdclk_update_hw_state(struct intel_display *display) { + const struct intel_bw_state *bw_state = + to_intel_bw_state(display->bw.obj.state); struct intel_cdclk_state *cdclk_state = to_intel_cdclk_state(display->cdclk.obj.state); struct intel_crtc *crtc; @@ -3358,6 +3360,8 @@ void intel_cdclk_update_hw_state(struct intel_display *display) cdclk_state->min_cdclk[pipe] = intel_crtc_compute_min_cdclk(crtc_state); cdclk_state->min_voltage_level[pipe] = crtc_state->min_voltage_level; } + + cdclk_state->bw_min_cdclk = intel_bw_min_cdclk(display, bw_state); } void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc) -- 2.51.0 From da1c27e4aef40b9c9710647ac3f8c4144711c1fa Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 26 Mar 2025 18:25:37 +0200 Subject: [PATCH 06/16] drm/i915: Flag even inactive crtcs as "inherited" MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit I want to use the crtc_state->inherited flag to clean up some of the early SAGV handling. To make that work nicely I need to flag even the inactive crtcs as "inherited". Since we can't expect user space to perform any real commits on inactive crtcs we'll clear the flag already during initial_commit(). Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250326162544.3642-8-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_display.c | 3 +++ .../drm/i915/display/intel_modeset_setup.c | 22 +++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index c540e2cae1f0..d448ee4f4a10 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -8100,6 +8100,9 @@ retry: goto out; } + if (!crtc_state->hw.active) + crtc_state->inherited = false; + if (crtc_state->hw.active) { struct intel_encoder *encoder; diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c index 3cc915739677..2dc641da0c3b 100644 --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c @@ -821,18 +821,18 @@ static void intel_modeset_readout_hw_state(struct drm_i915_private *i915) to_intel_crtc_state(crtc->base.state); struct intel_plane *plane; - if (crtc_state->hw.active) { - /* - * The initial mode needs to be set in order to keep - * the atomic core happy. It wants a valid mode if the - * crtc's enabled, so we do the above call. - * - * But we don't set all the derived state fully, hence - * set a flag to indicate that a full recalculation is - * needed on the next commit. - */ - crtc_state->inherited = true; + /* + * The initial mode needs to be set in order to keep + * the atomic core happy. It wants a valid mode if the + * crtc's enabled, so we do the above call. + * + * But we don't set all the derived state fully, hence + * set a flag to indicate that a full recalculation is + * needed on the next commit. + */ + crtc_state->inherited = true; + if (crtc_state->hw.active) { intel_crtc_update_active_timings(crtc_state, crtc_state->vrr.enable); -- 2.51.0 From 67ad5b9babdc99c352e9cc244ca3d7dffa5daf97 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 26 Mar 2025 18:25:38 +0200 Subject: [PATCH 07/16] drm/i915: Drop force_check_qgv MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Remove the force_check_qgv flag and just fill the pipe_sagv_reject bitmask properly during readout. This will cause the initial commit to re-enable SAGV if possible. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250326162544.3642-9-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_bw.c | 12 ++++++------ drivers/gpu/drm/i915/display/intel_bw.h | 6 ------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index 69f3de0bba6a..47de106c608f 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -1435,9 +1435,8 @@ int intel_bw_atomic_check(struct intel_atomic_state *state) new_bw_state = intel_atomic_get_new_bw_state(state); if (new_bw_state && - (intel_can_enable_sagv(i915, old_bw_state) != - intel_can_enable_sagv(i915, new_bw_state) || - new_bw_state->force_check_qgv)) + intel_can_enable_sagv(i915, old_bw_state) != + intel_can_enable_sagv(i915, new_bw_state)) changed = true; /* @@ -1451,8 +1450,6 @@ int intel_bw_atomic_check(struct intel_atomic_state *state) if (ret) return ret; - new_bw_state->force_check_qgv = false; - return 0; } @@ -1466,7 +1463,6 @@ static void intel_bw_crtc_update(struct intel_bw_state *bw_state, intel_bw_crtc_data_rate(crtc_state); bw_state->num_active_planes[crtc->pipe] = intel_bw_crtc_num_active_planes(crtc_state); - bw_state->force_check_qgv = true; drm_dbg_kms(display->drm, "pipe %c data rate %u num active planes %u\n", pipe_name(crtc->pipe), @@ -1484,6 +1480,7 @@ void intel_bw_update_hw_state(struct intel_display *display) return; bw_state->active_pipes = 0; + bw_state->pipe_sagv_reject = 0; for_each_intel_crtc(display->drm, crtc) { const struct intel_crtc_state *crtc_state = @@ -1497,6 +1494,9 @@ void intel_bw_update_hw_state(struct intel_display *display) intel_bw_crtc_update(bw_state, crtc_state); skl_crtc_calc_dbuf_bw(&bw_state->dbuf_bw[pipe], crtc_state); + + /* initially SAGV has been forced off */ + bw_state->pipe_sagv_reject |= BIT(pipe); } } diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h index 3e4397c85774..4a6a033f11e4 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.h +++ b/drivers/gpu/drm/i915/display/intel_bw.h @@ -48,12 +48,6 @@ struct intel_bw_state { */ u16 qgv_points_mask; - /* - * Flag to force the QGV comparison in atomic check right after the - * hw state readout - */ - bool force_check_qgv; - unsigned int data_rate[I915_MAX_PIPES]; u8 num_active_planes[I915_MAX_PIPES]; }; -- 2.51.0 From dacbfc5e9ee94a9c064bfbd0f03fa3880a5fa0c2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 26 Mar 2025 18:25:39 +0200 Subject: [PATCH 08/16] drm/i915: Extract intel_bw_modeset_checks() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Pull the new_bw_state->active_pipes computation out from intel_compute_sagv_mask() and move it into the intel_bw.c (which is arguably the correct place for it). Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250326162544.3642-10-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_bw.c | 29 ++++++++++++++++++++ drivers/gpu/drm/i915/display/intel_bw.h | 1 + drivers/gpu/drm/i915/display/intel_display.c | 6 ++++ drivers/gpu/drm/i915/display/skl_watermark.c | 9 ------ 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index 47de106c608f..dcf2b711b83a 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -1414,6 +1414,35 @@ static int intel_bw_check_data_rate(struct intel_atomic_state *state, bool *chan return 0; } +int intel_bw_modeset_checks(struct intel_atomic_state *state) +{ + struct intel_display *display = to_intel_display(state); + const struct intel_bw_state *old_bw_state; + struct intel_bw_state *new_bw_state; + + if (DISPLAY_VER(display) < 9) + return 0; + + new_bw_state = intel_atomic_get_bw_state(state); + if (IS_ERR(new_bw_state)) + return PTR_ERR(new_bw_state); + + old_bw_state = intel_atomic_get_old_bw_state(state); + + new_bw_state->active_pipes = + intel_calc_active_pipes(state, old_bw_state->active_pipes); + + if (new_bw_state->active_pipes != old_bw_state->active_pipes) { + int ret; + + ret = intel_atomic_lock_global_state(&new_bw_state->base); + if (ret) + return ret; + } + + return 0; +} + int intel_bw_atomic_check(struct intel_atomic_state *state) { struct intel_display *display = to_intel_display(state); diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h index 4a6a033f11e4..ac435674c3ed 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.h +++ b/drivers/gpu/drm/i915/display/intel_bw.h @@ -66,6 +66,7 @@ intel_atomic_get_bw_state(struct intel_atomic_state *state); void intel_bw_init_hw(struct intel_display *display); int intel_bw_init(struct intel_display *display); +int intel_bw_modeset_checks(struct intel_atomic_state *state); int intel_bw_atomic_check(struct intel_atomic_state *state); int icl_pcode_restrict_qgv_points(struct intel_display *display, u32 points_mask); diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index d448ee4f4a10..2a58a1a16cdf 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -6443,6 +6443,12 @@ int intel_atomic_check(struct drm_device *dev, if (ret) goto fail; + if (any_ms) { + ret = intel_bw_modeset_checks(state); + if (ret) + goto fail; + } + ret = intel_compute_global_watermarks(state); if (ret) goto fail; diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index a6af5e4ba4d4..1da1bfeadd9c 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -501,15 +501,6 @@ static int intel_compute_sagv_mask(struct intel_atomic_state *state) if (!new_bw_state) return 0; - new_bw_state->active_pipes = - intel_calc_active_pipes(state, old_bw_state->active_pipes); - - if (new_bw_state->active_pipes != old_bw_state->active_pipes) { - ret = intel_atomic_lock_global_state(&new_bw_state->base); - if (ret) - return ret; - } - if (intel_can_enable_sagv(i915, new_bw_state) != intel_can_enable_sagv(i915, old_bw_state)) { ret = intel_atomic_serialize_global_state(&new_bw_state->base); -- 2.51.0 From 014ea4d39c09c85b4120111927ce22b31fd0ceea Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 26 Mar 2025 18:25:40 +0200 Subject: [PATCH 09/16] drm/i915: Extract intel_bw_check_sagv_mask() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Move the bw_state->pipe_sagv_reject computation into intel_bw.c where it belongs. Previously we had a complicated dance between watermarks and sagv which required this to be computed earlier, but that was changed in commit 5e8146251f7b ("extract intel_bw_check_sagv_mask()") which allows the whole thing to be cleaned up quite a bit. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250326162544.3642-11-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_bw.c | 40 ++++++++++++++++++++ drivers/gpu/drm/i915/display/intel_bw.h | 1 + drivers/gpu/drm/i915/display/skl_watermark.c | 31 ++------------- drivers/gpu/drm/i915/display/skl_watermark.h | 1 + 4 files changed, 46 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index dcf2b711b83a..0553e902727e 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -1443,6 +1443,46 @@ int intel_bw_modeset_checks(struct intel_atomic_state *state) return 0; } +int intel_bw_check_sagv_mask(struct intel_atomic_state *state) +{ + struct intel_display *display = to_intel_display(state); + struct drm_i915_private *i915 = to_i915(display->drm); + const struct intel_crtc_state *new_crtc_state; + const struct intel_bw_state *old_bw_state = NULL; + struct intel_bw_state *new_bw_state = NULL; + struct intel_crtc *crtc; + int ret, i; + + for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { + new_bw_state = intel_atomic_get_bw_state(state); + if (IS_ERR(new_bw_state)) + return PTR_ERR(new_bw_state); + + old_bw_state = intel_atomic_get_old_bw_state(state); + + if (intel_crtc_can_enable_sagv(new_crtc_state)) + new_bw_state->pipe_sagv_reject &= ~BIT(crtc->pipe); + else + new_bw_state->pipe_sagv_reject |= BIT(crtc->pipe); + } + + if (!new_bw_state) + return 0; + + if (intel_can_enable_sagv(i915, new_bw_state) != + intel_can_enable_sagv(i915, old_bw_state)) { + ret = intel_atomic_serialize_global_state(&new_bw_state->base); + if (ret) + return ret; + } else if (new_bw_state->pipe_sagv_reject != old_bw_state->pipe_sagv_reject) { + ret = intel_atomic_lock_global_state(&new_bw_state->base); + if (ret) + return ret; + } + + return 0; +} + int intel_bw_atomic_check(struct intel_atomic_state *state) { struct intel_display *display = to_intel_display(state); diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h index ac435674c3ed..ee1d437340f3 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.h +++ b/drivers/gpu/drm/i915/display/intel_bw.h @@ -67,6 +67,7 @@ intel_atomic_get_bw_state(struct intel_atomic_state *state); void intel_bw_init_hw(struct intel_display *display); int intel_bw_init(struct intel_display *display); int intel_bw_modeset_checks(struct intel_atomic_state *state); +int intel_bw_check_sagv_mask(struct intel_atomic_state *state); int intel_bw_atomic_check(struct intel_atomic_state *state); int icl_pcode_restrict_qgv_points(struct intel_display *display, u32 points_mask); diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 1da1bfeadd9c..747b2b5c31bd 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -426,7 +426,7 @@ static bool tgl_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state) return true; } -static bool intel_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state) +bool intel_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state) { struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); struct drm_i915_private *i915 = to_i915(crtc->base.dev); @@ -457,20 +457,12 @@ static int intel_compute_sagv_mask(struct intel_atomic_state *state) int ret; struct intel_crtc *crtc; struct intel_crtc_state *new_crtc_state; - struct intel_bw_state *new_bw_state = NULL; - const struct intel_bw_state *old_bw_state = NULL; int i; for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { struct skl_pipe_wm *pipe_wm = &new_crtc_state->wm.skl.optimal; - new_bw_state = intel_atomic_get_bw_state(state); - if (IS_ERR(new_bw_state)) - return PTR_ERR(new_bw_state); - - old_bw_state = intel_atomic_get_old_bw_state(state); - /* * We store use_sagv_wm in the crtc state rather than relying on * that bw state since we have no convenient way to get at the @@ -491,26 +483,11 @@ static int intel_compute_sagv_mask(struct intel_atomic_state *state) pipe_wm->use_sagv_wm = !HAS_HW_SAGV_WM(display) && DISPLAY_VER(i915) >= 12 && intel_crtc_can_enable_sagv(new_crtc_state); - - if (intel_crtc_can_enable_sagv(new_crtc_state)) - new_bw_state->pipe_sagv_reject &= ~BIT(crtc->pipe); - else - new_bw_state->pipe_sagv_reject |= BIT(crtc->pipe); } - if (!new_bw_state) - return 0; - - if (intel_can_enable_sagv(i915, new_bw_state) != - intel_can_enable_sagv(i915, old_bw_state)) { - ret = intel_atomic_serialize_global_state(&new_bw_state->base); - if (ret) - return ret; - } else if (new_bw_state->pipe_sagv_reject != old_bw_state->pipe_sagv_reject) { - ret = intel_atomic_lock_global_state(&new_bw_state->base); - if (ret) - return ret; - } + ret = intel_bw_check_sagv_mask(state); + if (ret) + return ret; return 0; } diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h index d9cff6c54310..7e8107f808b6 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.h +++ b/drivers/gpu/drm/i915/display/skl_watermark.h @@ -27,6 +27,7 @@ u8 intel_enabled_dbuf_slices_mask(struct intel_display *display); void intel_sagv_pre_plane_update(struct intel_atomic_state *state); void intel_sagv_post_plane_update(struct intel_atomic_state *state); +bool intel_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state); bool intel_can_enable_sagv(struct drm_i915_private *i915, const struct intel_bw_state *bw_state); bool intel_has_sagv(struct drm_i915_private *i915); -- 2.51.0 From d1381206b13cbfa276dab0f399f3a46757c59f54 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 26 Mar 2025 18:25:41 +0200 Subject: [PATCH 10/16] drm/i915: Make intel_bw_check_sagv_mask() internal to intel_bw.c MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The only thing between the current intel_bw_check_sagv_mask() call site and intel_bw_atomic_check() is skl_wm_add_affected_planes() which no longer depends on the sagv mask, so we can make life a lot less confusing by calling intel_bw_check_sagv_mask() from intel_bw_atomic_check() instead. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250326162544.3642-12-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_bw.c | 9 ++++++++- drivers/gpu/drm/i915/display/intel_bw.h | 1 - drivers/gpu/drm/i915/display/skl_watermark.c | 5 ----- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index 0553e902727e..b54704f9af4e 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -1443,7 +1443,7 @@ int intel_bw_modeset_checks(struct intel_atomic_state *state) return 0; } -int intel_bw_check_sagv_mask(struct intel_atomic_state *state) +static int intel_bw_check_sagv_mask(struct intel_atomic_state *state) { struct intel_display *display = to_intel_display(state); struct drm_i915_private *i915 = to_i915(display->drm); @@ -1492,6 +1492,13 @@ int intel_bw_atomic_check(struct intel_atomic_state *state) const struct intel_bw_state *old_bw_state; int ret; + if (DISPLAY_VER(i915) < 9) + return 0; + + ret = intel_bw_check_sagv_mask(state); + if (ret) + return ret; + /* FIXME earlier gens need some checks too */ if (DISPLAY_VER(display) < 11) return 0; diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h index ee1d437340f3..ac435674c3ed 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.h +++ b/drivers/gpu/drm/i915/display/intel_bw.h @@ -67,7 +67,6 @@ intel_atomic_get_bw_state(struct intel_atomic_state *state); void intel_bw_init_hw(struct intel_display *display); int intel_bw_init(struct intel_display *display); int intel_bw_modeset_checks(struct intel_atomic_state *state); -int intel_bw_check_sagv_mask(struct intel_atomic_state *state); int intel_bw_atomic_check(struct intel_atomic_state *state); int icl_pcode_restrict_qgv_points(struct intel_display *display, u32 points_mask); diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 747b2b5c31bd..baf345039deb 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -454,7 +454,6 @@ static int intel_compute_sagv_mask(struct intel_atomic_state *state) { struct intel_display *display = to_intel_display(state); struct drm_i915_private *i915 = to_i915(state->base.dev); - int ret; struct intel_crtc *crtc; struct intel_crtc_state *new_crtc_state; int i; @@ -485,10 +484,6 @@ static int intel_compute_sagv_mask(struct intel_atomic_state *state) intel_crtc_can_enable_sagv(new_crtc_state); } - ret = intel_bw_check_sagv_mask(state); - if (ret) - return ret; - return 0; } -- 2.51.0 From 4d5204b3045473cc007aa119e6501db367979aa6 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 26 Mar 2025 18:25:42 +0200 Subject: [PATCH 11/16] drm/i915: Make intel_bw_modeset_checks() internal to intel_bw_atomic_check() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Now that all the sagv computation has been moved from the skl+ watermark code into intel_bw_atomic_check() there is no point in calling intel_bw_modeset_checks() before the wm computation. Hide it within intel_bw_atomic_check(). Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250326162544.3642-13-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_bw.c | 10 ++++++++-- drivers/gpu/drm/i915/display/intel_bw.h | 3 +-- drivers/gpu/drm/i915/display/intel_display.c | 8 +------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index b54704f9af4e..b8bb2e395bb7 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -1414,7 +1414,7 @@ static int intel_bw_check_data_rate(struct intel_atomic_state *state, bool *chan return 0; } -int intel_bw_modeset_checks(struct intel_atomic_state *state) +static int intel_bw_modeset_checks(struct intel_atomic_state *state) { struct intel_display *display = to_intel_display(state); const struct intel_bw_state *old_bw_state; @@ -1483,7 +1483,7 @@ static int intel_bw_check_sagv_mask(struct intel_atomic_state *state) return 0; } -int intel_bw_atomic_check(struct intel_atomic_state *state) +int intel_bw_atomic_check(struct intel_atomic_state *state, bool any_ms) { struct intel_display *display = to_intel_display(state); struct drm_i915_private *i915 = to_i915(display->drm); @@ -1495,6 +1495,12 @@ int intel_bw_atomic_check(struct intel_atomic_state *state) if (DISPLAY_VER(i915) < 9) return 0; + if (any_ms) { + ret = intel_bw_modeset_checks(state); + if (ret) + return ret; + } + ret = intel_bw_check_sagv_mask(state); if (ret) return ret; diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h index ac435674c3ed..eb2cc883e9c1 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.h +++ b/drivers/gpu/drm/i915/display/intel_bw.h @@ -66,8 +66,7 @@ intel_atomic_get_bw_state(struct intel_atomic_state *state); void intel_bw_init_hw(struct intel_display *display); int intel_bw_init(struct intel_display *display); -int intel_bw_modeset_checks(struct intel_atomic_state *state); -int intel_bw_atomic_check(struct intel_atomic_state *state); +int intel_bw_atomic_check(struct intel_atomic_state *state, bool any_ms); int icl_pcode_restrict_qgv_points(struct intel_display *display, u32 points_mask); int intel_bw_calc_min_cdclk(struct intel_atomic_state *state, diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 2a58a1a16cdf..dc7517da2ed5 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -6443,17 +6443,11 @@ int intel_atomic_check(struct drm_device *dev, if (ret) goto fail; - if (any_ms) { - ret = intel_bw_modeset_checks(state); - if (ret) - goto fail; - } - ret = intel_compute_global_watermarks(state); if (ret) goto fail; - ret = intel_bw_atomic_check(state); + ret = intel_bw_atomic_check(state, any_ms); if (ret) goto fail; -- 2.51.0 From da885fdaf9c1a07f9b1c99e105139ef4053004e6 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 26 Mar 2025 18:25:43 +0200 Subject: [PATCH 12/16] drm/i915: Skip bw stuff if per-crtc sagv state doesn't change MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit If there are no changes to intel_crtc_can_enable_sagv() there is no need to do all the sagv bw_state recomputation. The only slight caveat here is hw state takeover where we initially disable SAGV, and want it to get re-enabled once we've determined that it's safe to do so. That can now be achieved by having intel_crtc_can_enable_sagv() reject SAGV as long as the crtc_state->inherited flag is set. Once the flag gets cleared (during initial commit for inactive pipes, during the first userspace commit for active pipes), we will naturally recompute all the sagv related state. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250326162544.3642-14-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_bw.c | 8 +++++++- drivers/gpu/drm/i915/display/skl_watermark.c | 11 +++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index b8bb2e395bb7..e819db78dfbd 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -1447,13 +1447,19 @@ static int intel_bw_check_sagv_mask(struct intel_atomic_state *state) { struct intel_display *display = to_intel_display(state); struct drm_i915_private *i915 = to_i915(display->drm); + const struct intel_crtc_state *old_crtc_state; const struct intel_crtc_state *new_crtc_state; const struct intel_bw_state *old_bw_state = NULL; struct intel_bw_state *new_bw_state = NULL; struct intel_crtc *crtc; int ret, i; - for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, + new_crtc_state, i) { + if (intel_crtc_can_enable_sagv(old_crtc_state) == + intel_crtc_can_enable_sagv(new_crtc_state)) + continue; + new_bw_state = intel_atomic_get_bw_state(state); if (IS_ERR(new_bw_state)) return PTR_ERR(new_bw_state); diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index baf345039deb..8b51f0b42785 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -434,6 +434,14 @@ bool intel_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state) if (!i915->display.params.enable_sagv) return false; + /* + * SAGV is initially forced off because its current + * state can't be queried from pcode. Allow SAGV to + * be enabled upon the first real commit. + */ + if (crtc_state->inherited) + return false; + if (DISPLAY_VER(i915) >= 12) return tgl_crtc_can_enable_sagv(crtc_state); else @@ -458,8 +466,7 @@ static int intel_compute_sagv_mask(struct intel_atomic_state *state) struct intel_crtc_state *new_crtc_state; int i; - for_each_new_intel_crtc_in_state(state, crtc, - new_crtc_state, i) { + for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { struct skl_pipe_wm *pipe_wm = &new_crtc_state->wm.skl.optimal; /* -- 2.51.0 From 669cdda66efb983524985c94482481b7c90715a9 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 26 Mar 2025 18:25:44 +0200 Subject: [PATCH 13/16] drm/i915: Eliminate intel_compute_sagv_mask() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit intel_compute_sagv_mask() has become pointless. Just inline its contents into the existing loop in skl_compute_wm(). Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250326162544.3642-15-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/skl_watermark.c | 64 ++++++++------------ 1 file changed, 24 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 8b51f0b42785..9381aec797c9 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -458,42 +458,6 @@ bool intel_can_enable_sagv(struct drm_i915_private *i915, return bw_state->pipe_sagv_reject == 0; } -static int intel_compute_sagv_mask(struct intel_atomic_state *state) -{ - struct intel_display *display = to_intel_display(state); - struct drm_i915_private *i915 = to_i915(state->base.dev); - struct intel_crtc *crtc; - struct intel_crtc_state *new_crtc_state; - int i; - - for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { - struct skl_pipe_wm *pipe_wm = &new_crtc_state->wm.skl.optimal; - - /* - * We store use_sagv_wm in the crtc state rather than relying on - * that bw state since we have no convenient way to get at the - * latter from the plane commit hooks (especially in the legacy - * cursor case). - * - * drm_atomic_check_only() gets upset if we pull more crtcs - * into the state, so we have to calculate this based on the - * individual intel_crtc_can_enable_sagv() rather than - * the overall intel_can_enable_sagv(). Otherwise the - * crtcs not included in the commit would not switch to the - * SAGV watermarks when we are about to enable SAGV, and that - * would lead to underruns. This does mean extra power draw - * when only a subset of the crtcs are blocking SAGV as the - * other crtcs can't be allowed to use the more optimal - * normal (ie. non-SAGV) watermarks. - */ - pipe_wm->use_sagv_wm = !HAS_HW_SAGV_WM(display) && - DISPLAY_VER(i915) >= 12 && - intel_crtc_can_enable_sagv(new_crtc_state); - } - - return 0; -} - static u16 skl_ddb_entry_init(struct skl_ddb_entry *entry, u16 start, u16 end) { @@ -3028,6 +2992,7 @@ intel_program_dpkgc_latency(struct intel_atomic_state *state) static int skl_compute_wm(struct intel_atomic_state *state) { + struct intel_display *display = to_intel_display(state); struct intel_crtc *crtc; struct intel_crtc_state __maybe_unused *new_crtc_state; int ret, i; @@ -3042,16 +3007,35 @@ skl_compute_wm(struct intel_atomic_state *state) if (ret) return ret; - ret = intel_compute_sagv_mask(state); - if (ret) - return ret; - /* * skl_compute_ddb() will have adjusted the final watermarks * based on how much ddb is available. Now we can actually * check if the final watermarks changed. */ for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { + struct skl_pipe_wm *pipe_wm = &new_crtc_state->wm.skl.optimal; + + /* + * We store use_sagv_wm in the crtc state rather than relying on + * that bw state since we have no convenient way to get at the + * latter from the plane commit hooks (especially in the legacy + * cursor case). + * + * drm_atomic_check_only() gets upset if we pull more crtcs + * into the state, so we have to calculate this based on the + * individual intel_crtc_can_enable_sagv() rather than + * the overall intel_can_enable_sagv(). Otherwise the + * crtcs not included in the commit would not switch to the + * SAGV watermarks when we are about to enable SAGV, and that + * would lead to underruns. This does mean extra power draw + * when only a subset of the crtcs are blocking SAGV as the + * other crtcs can't be allowed to use the more optimal + * normal (ie. non-SAGV) watermarks. + */ + pipe_wm->use_sagv_wm = !HAS_HW_SAGV_WM(display) && + DISPLAY_VER(display) >= 12 && + intel_crtc_can_enable_sagv(new_crtc_state); + ret = skl_wm_add_affected_planes(state, crtc); if (ret) return ret; -- 2.51.0 From ced5e64f011cb5cd541988442997ceaa7385827e Mon Sep 17 00:00:00 2001 From: Ankit Nautiyal Date: Fri, 4 Apr 2025 13:35:40 +0530 Subject: [PATCH 14/16] drm/i915/vrr: Add vrr.vsync_{start, end} in vrr_params_changed MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Add the missing vrr parameters in vrr_params_changed() helper. This ensures that changes in vrr.vsync_{start,end} trigger a call to appropriate helpers to update the VRR registers. Fixes: e8cd188e91bb ("drm/i915/display: Compute vrr_vsync params") Cc: Mitul Golani Cc: Arun R Murthy Cc: Ankit Nautiyal Cc: Jani Nikula Cc: # v6.10+ Signed-off-by: Ankit Nautiyal Reviewed-by: Ville Syrjälä Link: https://lore.kernel.org/r/20250404080540.2059511-1-ankit.k.nautiyal@intel.com --- drivers/gpu/drm/i915/display/intel_display.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index dc7517da2ed5..16fd7c00ba01 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -969,7 +969,9 @@ static bool vrr_params_changed(const struct intel_crtc_state *old_crtc_state, old_crtc_state->vrr.vmin != new_crtc_state->vrr.vmin || old_crtc_state->vrr.vmax != new_crtc_state->vrr.vmax || old_crtc_state->vrr.guardband != new_crtc_state->vrr.guardband || - old_crtc_state->vrr.pipeline_full != new_crtc_state->vrr.pipeline_full; + old_crtc_state->vrr.pipeline_full != new_crtc_state->vrr.pipeline_full || + old_crtc_state->vrr.vsync_start != new_crtc_state->vrr.vsync_start || + old_crtc_state->vrr.vsync_end != new_crtc_state->vrr.vsync_end; } static bool cmrr_params_changed(const struct intel_crtc_state *old_crtc_state, -- 2.51.0 From db0f7e20c284dba5fb087dd09f56db9faed560f4 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Fri, 4 Apr 2025 18:03:05 +0300 Subject: [PATCH 15/16] drm/i915/dp: Rename intel_dp::link_trained to link.active The intel_dp::link_trained flag indicates whether the link is active, regardless of whether the link training passed or failed. For clarity rename the flag to 'active'. While at it move the flag under intel_dp::link. Suggested-by: Jani Nikula Reviewed-by: Jani Nikula Signed-off-by: Imre Deak Link: https://lore.kernel.org/r/20250404150310.1156696-2-imre.deak@intel.com --- drivers/gpu/drm/i915/display/g4x_dp.c | 2 +- drivers/gpu/drm/i915/display/intel_ddi.c | 2 +- drivers/gpu/drm/i915/display/intel_display_types.h | 2 +- drivers/gpu/drm/i915/display/intel_dp.c | 6 +++--- drivers/gpu/drm/i915/display/intel_dp_link_training.c | 8 ++++---- drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c index 55b9e9bfcc4d..b39aae9165df 100644 --- a/drivers/gpu/drm/i915/display/g4x_dp.c +++ b/drivers/gpu/drm/i915/display/g4x_dp.c @@ -519,7 +519,7 @@ static void intel_disable_dp(struct intel_atomic_state *state, { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); - intel_dp->link_trained = false; + intel_dp->link.active = false; /* * Make sure the panel is off before trying to change the mode. diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 56068b2ef964..f381ceb59c7d 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -3550,7 +3550,7 @@ static void intel_ddi_disable_dp(struct intel_atomic_state *state, struct intel_connector *connector = to_intel_connector(old_conn_state->connector); - intel_dp->link_trained = false; + intel_dp->link.active = false; intel_psr_disable(intel_dp, old_crtc_state); intel_edp_backlight_off(old_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 367b53a9eae2..769e3a48d9c5 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1658,7 +1658,6 @@ struct intel_dp { int link_rate; u8 lane_count; u8 sink_count; - bool link_trained; bool needs_modeset_retry; bool use_max_params; u8 dpcd[DP_RECEIVER_CAP_SIZE]; @@ -1683,6 +1682,7 @@ struct intel_dp { int common_rates[DP_MAX_SUPPORTED_RATES]; struct { /* TODO: move the rest of link specific fields to here */ + bool active; /* common rate,lane_count configs in bw order */ int num_configs; #define INTEL_DP_MAX_LANE_COUNT 4 diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 7b95d62730e6..df8c86fd133e 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3222,7 +3222,7 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp, int link_rate, int lane_count) { memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set)); - intel_dp->link_trained = false; + intel_dp->link.active = false; intel_dp->needs_modeset_retry = false; intel_dp->link_rate = link_rate; intel_dp->lane_count = lane_count; @@ -3586,7 +3586,7 @@ void intel_dp_sync_state(struct intel_encoder *encoder, if (crtc_state) { intel_dp_reset_link_params(intel_dp); intel_dp_set_link_params(intel_dp, crtc_state->port_clock, crtc_state->lane_count); - intel_dp->link_trained = true; + intel_dp->link.active = true; } } @@ -5081,7 +5081,7 @@ intel_dp_needs_link_retrain(struct intel_dp *intel_dp) { u8 link_status[DP_LINK_STATUS_SIZE]; - if (!intel_dp->link_trained) + if (!intel_dp->link.active) return false; /* diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c index 53480914f239..6efdd6293302 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c @@ -168,7 +168,7 @@ static int intel_dp_init_lttpr_phys(struct intel_dp *intel_dp, const u8 dpcd[DP_ * resetting its internal state when the mode is changed from * non-transparent to transparent. */ - if (intel_dp->link_trained) { + if (intel_dp->link.active) { if (lttpr_count < 0 || intel_dp_lttpr_transparent_mode_enabled(intel_dp)) goto out_reset_lttpr_count; @@ -1142,7 +1142,7 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp, struct intel_display *display = to_intel_display(intel_dp); struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; - intel_dp->link_trained = true; + intel_dp->link.active = true; intel_dp_disable_dpcd_training_pattern(intel_dp, DP_PHY_DPRX); intel_dp_program_link_training_pattern(intel_dp, crtc_state, DP_PHY_DPRX, @@ -1735,7 +1735,7 @@ static int i915_dp_force_link_rate_show(struct seq_file *m, void *data) if (err) return err; - if (intel_dp->link_trained) + if (intel_dp->link.active) current_rate = intel_dp->link_rate; force_rate = intel_dp->link.force_rate; @@ -1833,7 +1833,7 @@ static int i915_dp_force_lane_count_show(struct seq_file *m, void *data) if (err) return err; - if (intel_dp->link_trained) + if (intel_dp->link.active) current_lane_count = intel_dp->lane_count; force_lane_count = intel_dp->link.force_lane_count; diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index d2988b9a6e7b..325c2a034655 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1004,7 +1004,7 @@ static void mst_stream_disable(struct intel_atomic_state *state, intel_dp->mst.active_links); if (intel_dp->mst.active_links == 1) - intel_dp->link_trained = false; + intel_dp->link.active = false; intel_hdcp_disable(intel_mst->connector); @@ -2108,7 +2108,7 @@ void intel_dp_mst_prepare_probe(struct intel_dp *intel_dp) u8 rate_select; u8 link_bw; - if (intel_dp->link_trained) + if (intel_dp->link.active) return; if (intel_mst_probed_link_params_valid(intel_dp, link_rate, lane_count)) -- 2.51.0 From 59e8cd62f5a6f09cfdaf2847d051bac99ea4a8f3 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Fri, 4 Apr 2025 18:03:06 +0300 Subject: [PATCH 16/16] drm/i915/dp_mst: Add intel_dp_mst_{inc, dec}_active_streams() Add helpers to increment/decrement the active MST stream count, instead of open-coding these. In mst_stream_pre_enable(), the increment will happen earlier, this is ok, since nothing depends on the counter between the two points. Reviewed-by: Jani Nikula Signed-off-by: Imre Deak Link: https://lore.kernel.org/r/20250404150310.1156696-3-imre.deak@intel.com --- drivers/gpu/drm/i915/display/intel_dp_mst.c | 43 +++++++++++++-------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 325c2a034655..8084e7e8cf11 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -105,6 +105,29 @@ static struct intel_dp *to_primary_dp(struct intel_encoder *encoder) return &dig_port->dp; } +static bool intel_dp_mst_dec_active_streams(struct intel_dp *intel_dp) +{ + struct intel_display *display = to_intel_display(intel_dp); + + drm_dbg_kms(display->drm, "active MST streams %d -> %d\n", + intel_dp->mst.active_links, intel_dp->mst.active_links - 1); + + if (drm_WARN_ON(display->drm, intel_dp->mst.active_links == 0)) + return true; + + return --intel_dp->mst.active_links == 0; +} + +static bool intel_dp_mst_inc_active_streams(struct intel_dp *intel_dp) +{ + struct intel_display *display = to_intel_display(intel_dp); + + drm_dbg_kms(display->drm, "active MST streams %d -> %d\n", + intel_dp->mst.active_links, intel_dp->mst.active_links + 1); + + return intel_dp->mst.active_links++ == 0; +} + static int intel_dp_mst_max_dpt_bpp(const struct intel_crtc_state *crtc_state, bool dsc) { @@ -1000,9 +1023,6 @@ static void mst_stream_disable(struct intel_atomic_state *state, to_intel_connector(old_conn_state->connector); enum transcoder trans = old_crtc_state->cpu_transcoder; - drm_dbg_kms(display->drm, "active links %d\n", - intel_dp->mst.active_links); - if (intel_dp->mst.active_links == 1) intel_dp->link.active = false; @@ -1037,8 +1057,8 @@ static void mst_stream_post_disable(struct intel_atomic_state *state, bool last_mst_stream; int i; - intel_dp->mst.active_links--; - last_mst_stream = intel_dp->mst.active_links == 0; + last_mst_stream = intel_dp_mst_dec_active_streams(intel_dp); + drm_WARN_ON(display->drm, DISPLAY_VER(display) >= 12 && last_mst_stream && !intel_dp_mst_is_master_trans(old_crtc_state)); @@ -1109,8 +1129,6 @@ static void mst_stream_post_disable(struct intel_atomic_state *state, primary_encoder->post_disable(state, primary_encoder, old_crtc_state, NULL); - drm_dbg_kms(display->drm, "active links %d\n", - intel_dp->mst.active_links); } static void mst_stream_post_pll_disable(struct intel_atomic_state *state, @@ -1194,13 +1212,11 @@ static void mst_stream_pre_enable(struct intel_atomic_state *state, */ connector->encoder = encoder; intel_mst->connector = connector; - first_mst_stream = intel_dp->mst.active_links == 0; + + first_mst_stream = intel_dp_mst_inc_active_streams(intel_dp); drm_WARN_ON(display->drm, DISPLAY_VER(display) >= 12 && first_mst_stream && !intel_dp_mst_is_master_trans(pipe_config)); - drm_dbg_kms(display->drm, "active links %d\n", - intel_dp->mst.active_links); - if (first_mst_stream) intel_dp_set_power(intel_dp, DP_SET_POWER_D0); @@ -1215,8 +1231,6 @@ static void mst_stream_pre_enable(struct intel_atomic_state *state, intel_mst_reprobe_topology(intel_dp, pipe_config); } - intel_dp->mst.active_links++; - ret = drm_dp_add_payload_part1(&intel_dp->mst.mgr, mst_state, drm_atomic_get_mst_payload_state(mst_state, connector->mst.port)); if (ret < 0) @@ -1335,9 +1349,6 @@ static void mst_stream_enable(struct intel_atomic_state *state, intel_de_rmw(display, TRANS_DDI_FUNC_CTL(display, trans), 0, TRANS_DDI_DP_VC_PAYLOAD_ALLOC); - drm_dbg_kms(display->drm, "active links %d\n", - intel_dp->mst.active_links); - intel_ddi_wait_for_act_sent(encoder, pipe_config); drm_dp_check_act_status(&intel_dp->mst.mgr); -- 2.51.0