From 9a112dd8c17fa6397785f2227dfe4f6f175ed524 Mon Sep 17 00:00:00 2001 From: Vinod Govindapillai Date: Fri, 28 Feb 2025 11:37:55 +0200 Subject: [PATCH 01/16] drm/damage-helper: add const qualifier in drm_atomic_helper_damage_merged() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Add a const qualifier for the "state" parameter as well as we could use this helper to get the combined damage in cases of const drm_plane_state as well. Needed mainly for xe driver big joiner cases where we need to track the damage from immutable plane state. Reviewed-by: Ville Syrjälä Signed-off-by: Vinod Govindapillai Acked-by: Thomas Zimmermann Signed-off-by: Mika Kahola Link: https://patchwork.freedesktop.org/patch/msgid/20250228093802.27091-3-vinod.govindapillai@intel.com --- drivers/gpu/drm/drm_damage_helper.c | 2 +- include/drm/drm_damage_helper.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c index afb02aae707b..44a5a36806e3 100644 --- a/drivers/gpu/drm/drm_damage_helper.c +++ b/drivers/gpu/drm/drm_damage_helper.c @@ -308,7 +308,7 @@ EXPORT_SYMBOL(drm_atomic_helper_damage_iter_next); * True if there is valid plane damage otherwise false. */ bool drm_atomic_helper_damage_merged(const struct drm_plane_state *old_state, - struct drm_plane_state *state, + const struct drm_plane_state *state, struct drm_rect *rect) { struct drm_atomic_helper_damage_iter iter; diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h index effda42cce31..a58cbcd11276 100644 --- a/include/drm/drm_damage_helper.h +++ b/include/drm/drm_damage_helper.h @@ -78,7 +78,7 @@ bool drm_atomic_helper_damage_iter_next(struct drm_atomic_helper_damage_iter *iter, struct drm_rect *rect); bool drm_atomic_helper_damage_merged(const struct drm_plane_state *old_state, - struct drm_plane_state *state, + const struct drm_plane_state *state, struct drm_rect *rect); #endif -- 2.51.0 From 6f60de67d7e4ae0f6c7aebcb9b62d89fed7233a4 Mon Sep 17 00:00:00 2001 From: Vinod Govindapillai Date: Fri, 28 Feb 2025 11:37:56 +0200 Subject: [PATCH 02/16] drm/i915/display: update and store the plane damage clips MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Userspace can pass damage area clips per plane to track changes in a plane and some display components can utilze these damage clips for efficiently handling use cases like FBC, PSR etc. A merged damage area is generated and its coordinates are updated relative to viewport and HW and stored in the plane_state. This merged damage areas will be used for FBC dirty rect support in xe3 in the follow-up patch. Big thanks to Ville Syrjala for his contribuitions in shaping up of this series. v1: - Move damage_merged helper to cover bigjoiner case and use the correct plane state for damage find helper (Ville) - Damage handling code under HAS_FBC_DIRTY_RECT() so the the related part will be executed only for xe3+ - Changed dev_priv to i915 in one of the functions v2: - damage reported is stored in the plane state after coords adjustmentments irrespective of fbc dirty rect support. - Damage to be empty in case of plane not visible (Ville) - Handle fb could be NULL and plane not visible cases (Ville) v3: - No need to empty damage in case disp ver < 12 (Ville) - update to the patch subject Reviewed-by: Ville Syrjälä Signed-off-by: Vinod Govindapillai Signed-off-by: Mika Kahola Link: https://patchwork.freedesktop.org/patch/msgid/20250228093802.27091-4-vinod.govindapillai@intel.com --- .../gpu/drm/i915/display/intel_atomic_plane.c | 29 ++++++++++++ .../drm/i915/display/intel_display_types.h | 2 + .../drm/i915/display/skl_universal_plane.c | 46 ++++++++++++++++++- 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 4a3a7125152f..9ac3c008f375 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -36,6 +36,7 @@ #include #include +#include #include #include #include @@ -132,6 +133,7 @@ intel_plane_duplicate_state(struct drm_plane *plane) intel_state->ggtt_vma = NULL; intel_state->dpt_vma = NULL; intel_state->flags = 0; + intel_state->damage = DRM_RECT_INIT(0, 0, 0, 0); /* add reference to fb */ if (intel_state->hw.fb) @@ -337,6 +339,25 @@ static void intel_plane_clear_hw_state(struct intel_plane_state *plane_state) memset(&plane_state->hw, 0, sizeof(plane_state->hw)); } +static void +intel_plane_copy_uapi_plane_damage(struct intel_plane_state *new_plane_state, + const struct intel_plane_state *old_uapi_plane_state, + const struct intel_plane_state *new_uapi_plane_state) +{ + struct intel_display *display = to_intel_display(new_plane_state); + struct drm_rect *damage = &new_plane_state->damage; + + /* damage property tracking enabled from display version 12 onwards */ + if (DISPLAY_VER(display) < 12) + return; + + if (!drm_atomic_helper_damage_merged(&old_uapi_plane_state->uapi, + &new_uapi_plane_state->uapi, + damage)) + /* Incase helper fails, mark whole plane region as damage */ + *damage = drm_plane_state_src(&new_uapi_plane_state->uapi); +} + void intel_plane_copy_uapi_to_hw_state(struct intel_plane_state *plane_state, const struct intel_plane_state *from_plane_state, struct intel_crtc *crtc) @@ -706,6 +727,7 @@ int intel_plane_atomic_check(struct intel_atomic_state *state, const struct intel_plane_state *old_plane_state = intel_atomic_get_old_plane_state(state, plane); const struct intel_plane_state *new_primary_crtc_plane_state; + const struct intel_plane_state *old_primary_crtc_plane_state; struct intel_crtc *crtc = intel_crtc_for_pipe(display, plane->pipe); const struct intel_crtc_state *old_crtc_state = intel_atomic_get_old_crtc_state(state, crtc); @@ -720,10 +742,17 @@ int intel_plane_atomic_check(struct intel_atomic_state *state, new_primary_crtc_plane_state = intel_atomic_get_new_plane_state(state, primary_crtc_plane); + old_primary_crtc_plane_state = + intel_atomic_get_old_plane_state(state, primary_crtc_plane); } else { new_primary_crtc_plane_state = new_plane_state; + old_primary_crtc_plane_state = old_plane_state; } + intel_plane_copy_uapi_plane_damage(new_plane_state, + old_primary_crtc_plane_state, + new_primary_crtc_plane_state); + intel_plane_copy_uapi_to_hw_state(new_plane_state, new_primary_crtc_plane_state, crtc); diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index ffa311845d05..99a6fd2900b9 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -693,6 +693,8 @@ struct intel_plane_state { u64 ccval; const char *no_fbc_reason; + + struct drm_rect damage; }; struct intel_initial_plane_config { diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index 87d003498722..70e550539bb2 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -2269,6 +2269,44 @@ static void check_protection(struct intel_plane_state *plane_state) !plane_state->decrypt; } +static void +make_damage_viewport_relative(struct intel_plane_state *plane_state) +{ + const struct drm_framebuffer *fb = plane_state->hw.fb; + const struct drm_rect *src = &plane_state->uapi.src; + unsigned int rotation = plane_state->hw.rotation; + struct drm_rect *damage = &plane_state->damage; + + if (!drm_rect_visible(damage)) + return; + + if (!fb || !plane_state->uapi.visible) { + plane_state->damage = DRM_RECT_INIT(0, 0, 0, 0); + return; + } + + if (drm_rotation_90_or_270(rotation)) { + drm_rect_rotate(damage, fb->width, fb->height, + DRM_MODE_ROTATE_270); + drm_rect_translate(damage, -(src->y1 >> 16), -(src->x1 >> 16)); + } else { + drm_rect_translate(damage, -(src->x1 >> 16), -(src->y1 >> 16)); + } +} + +static void clip_damage(struct intel_plane_state *plane_state) +{ + struct drm_rect *damage = &plane_state->damage; + struct drm_rect src; + + if (!drm_rect_visible(damage)) + return; + + drm_rect_fp_to_int(&src, &plane_state->uapi.src); + drm_rect_translate(damage, src.x1, src.y1); + drm_rect_intersect(damage, &src); +} + static int skl_plane_check(struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state) { @@ -2294,6 +2332,8 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state, if (ret) return ret; + make_damage_viewport_relative(plane_state); + ret = skl_check_plane_surface(plane_state); if (ret) return ret; @@ -2309,6 +2349,8 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state, if (ret) return ret; + clip_damage(plane_state); + ret = skl_plane_check_nv12_rotation(plane_state); if (ret) return ret; @@ -2316,8 +2358,10 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state, check_protection(plane_state); /* HW only has 8 bits pixel precision, disable plane if invisible */ - if (!(plane_state->hw.alpha >> 8)) + if (!(plane_state->hw.alpha >> 8)) { plane_state->uapi.visible = false; + plane_state->damage = DRM_RECT_INIT(0, 0, 0, 0); + } plane_state->ctl = skl_plane_ctl(crtc_state, plane_state); -- 2.51.0 From 22a28633a40fd419f91ec8304336841d0f9c880d Mon Sep 17 00:00:00 2001 From: Vinod Govindapillai Date: Fri, 28 Feb 2025 11:37:57 +0200 Subject: [PATCH 03/16] drm/i915/fbc: add register definitions for fbc dirty rect support MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Register definitions for FBC dirty rect support v2: - update to the patch subject Bspec: 71675, 73424 Reviewed-by: Ville Syrjälä Signed-off-by: Vinod Govindapillai Signed-off-by: Mika Kahola Link: https://patchwork.freedesktop.org/patch/msgid/20250228093802.27091-5-vinod.govindapillai@intel.com --- drivers/gpu/drm/i915/display/intel_fbc_regs.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_fbc_regs.h b/drivers/gpu/drm/i915/display/intel_fbc_regs.h index ae0699c3c2fe..b1d0161a3196 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc_regs.h +++ b/drivers/gpu/drm/i915/display/intel_fbc_regs.h @@ -100,6 +100,15 @@ #define FBC_STRIDE_MASK REG_GENMASK(14, 0) #define FBC_STRIDE(x) REG_FIELD_PREP(FBC_STRIDE_MASK, (x)) +#define XE3_FBC_DIRTY_RECT(fbc_id) _MMIO_PIPE((fbc_id), 0x43230, 0x43270) +#define FBC_DIRTY_RECT_END_LINE_MASK REG_GENMASK(31, 16) +#define FBC_DIRTY_RECT_END_LINE(val) REG_FIELD_PREP(FBC_DIRTY_RECT_END_LINE_MASK, (val)) +#define FBC_DIRTY_RECT_START_LINE_MASK REG_GENMASK(15, 0) +#define FBC_DIRTY_RECT_START_LINE(val) REG_FIELD_PREP(FBC_DIRTY_RECT_START_LINE_MASK, (val)) + +#define XE3_FBC_DIRTY_CTL(fbc_id) _MMIO_PIPE((fbc_id), 0x43234, 0x43274) +#define FBC_DIRTY_RECT_EN REG_BIT(31) + #define ILK_FBC_RT_BASE _MMIO(0x2128) #define ILK_FBC_RT_VALID REG_BIT(0) #define SNB_FBC_FRONT_BUFFER REG_BIT(1) -- 2.51.0 From c931a0aa82c65964bf62d02d3fb7e69153ff37eb Mon Sep 17 00:00:00 2001 From: Vinod Govindapillai Date: Fri, 28 Feb 2025 11:37:58 +0200 Subject: [PATCH 04/16] drm/i915/fbc: introduce HAS_FBC_DIRTY_RECT() for FBC dirty rect support MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Introduce a macro to check if the platform supports FBC dirty rect capability. v2: - update to the patch subject Reviewed-by: Ville Syrjälä Signed-off-by: Vinod Govindapillai Signed-off-by: Mika Kahola Link: https://patchwork.freedesktop.org/patch/msgid/20250228093802.27091-6-vinod.govindapillai@intel.com --- drivers/gpu/drm/i915/display/intel_display_device.h | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h index fc33791f02b9..717286981687 100644 --- a/drivers/gpu/drm/i915/display/intel_display_device.h +++ b/drivers/gpu/drm/i915/display/intel_display_device.h @@ -163,6 +163,7 @@ struct intel_display_platforms { #define HAS_DSC(__display) (DISPLAY_RUNTIME_INFO(__display)->has_dsc) #define HAS_DSC_MST(__display) (DISPLAY_VER(__display) >= 12 && HAS_DSC(__display)) #define HAS_FBC(__display) (DISPLAY_RUNTIME_INFO(__display)->fbc_mask != 0) +#define HAS_FBC_DIRTY_RECT(__display) (DISPLAY_VER(__display) >= 30) #define HAS_FPGA_DBG_UNCLAIMED(__display) (DISPLAY_INFO(__display)->has_fpga_dbg) #define HAS_FW_BLC(__display) (DISPLAY_VER(__display) >= 3) #define HAS_GMBUS_IRQ(__display) (DISPLAY_VER(__display) >= 4) -- 2.51.0 From 5adac4c9f321db0b2efb1b6ac6d6d9791ecb6fc0 Mon Sep 17 00:00:00 2001 From: Vinod Govindapillai Date: Fri, 28 Feb 2025 11:37:59 +0200 Subject: [PATCH 05/16] drm/i915/fbc: avoid calling fbc activate if fbc is active MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit If FBC is already active, we don't need to call FBC activate routine again unless there are changes to the fences. So skip this on all platforms that don't have fences. Any FBC register updates done after enabling the dirty rect support in xe3 will trigger nuke by FBC which is counter productive to the fbc dirty rect feature. The front buffer rendering sequence will call intel_fbc_flush() and which will call intel_fbc_nuke() or intel_fbc_activate() based on FBC status explicitly and won't get impacted by this change. v2: use HAS_FBC_DIRTY_RECT() move this functionality within intel_fbc_activate() v3: update to intel_fbc_activate logic (Ville) update to the patch description Reviewed-by: Ville Syrjälä Signed-off-by: Vinod Govindapillai Signed-off-by: Mika Kahola Link: https://patchwork.freedesktop.org/patch/msgid/20250228093802.27091-7-vinod.govindapillai@intel.com --- drivers/gpu/drm/i915/display/intel_fbc.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 33142427f121..ca44cec73fd2 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -734,8 +734,19 @@ static void intel_fbc_nuke(struct intel_fbc *fbc) static void intel_fbc_activate(struct intel_fbc *fbc) { + struct intel_display *display = fbc->display; + lockdep_assert_held(&fbc->lock); + /* only the fence can change for a flip nuke */ + if (fbc->active && !intel_fbc_has_fences(display)) + return; + /* + * In case of FBC dirt rect, any updates to the FBC registers will + * trigger the nuke. + */ + drm_WARN_ON(display->drm, fbc->active && HAS_FBC_DIRTY_RECT(display)); + intel_fbc_hw_activate(fbc); intel_fbc_nuke(fbc); -- 2.51.0 From 194ecad0b5fcd6f1a325e31ade9c19490260b40f Mon Sep 17 00:00:00 2001 From: Vinod Govindapillai Date: Fri, 28 Feb 2025 11:38:00 +0200 Subject: [PATCH 06/16] drm/i915/fbc: dirty rect support for FBC MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Dirty rectangle feature allows FBC to recompress a subsection of a frame. When this feature is enabled, display will read the scan lines between dirty rectangle start line and dirty rectangle end line in subsequent frames. Use the merged damage clip stored in the plane state to configure the FBC dirty rect areas. v2: - Move dirty rect handling to fbc state (Ville) v3: - Use intel_fbc_dirty_rect_update_noarm (Ville) - Split plane damage collection and dirty rect preparation - Handle case where dirty rect fall outside the visible region v4: - A state variable to check if we need to update dirty rect registers in case intel_fbc_can_flip_nuke() (Ville) v5: - No need to use a separate valid flag, updates to the conditions for prepare damage rect (Ville) - Usage of locks in fbc dirty rect related functions (Ville) v6: - updates dirty rect handling (Ville) v7: - Loop through all planes in atomic state is good enough (Ville) Bspec: 68881, 71675, 73424 Reviewed-by: Ville Syrjälä Signed-off-by: Vinod Govindapillai Signed-off-by: Mika Kahola Link: https://patchwork.freedesktop.org/patch/msgid/20250228093802.27091-8-vinod.govindapillai@intel.com --- .../gpu/drm/i915/display/intel_atomic_plane.c | 3 + drivers/gpu/drm/i915/display/intel_display.c | 2 + drivers/gpu/drm/i915/display/intel_fbc.c | 84 +++++++++++++++++++ drivers/gpu/drm/i915/display/intel_fbc.h | 4 + 4 files changed, 93 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 9ac3c008f375..a26b54185d5b 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -818,6 +818,9 @@ void intel_plane_update_noarm(struct intel_dsb *dsb, trace_intel_plane_update_noarm(plane_state, crtc); + if (plane->fbc) + intel_fbc_dirty_rect_update_noarm(dsb, plane); + if (plane->update_noarm) plane->update_noarm(dsb, plane, crtc_state, plane_state); } diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index b795de186602..bdc25b8153e0 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7272,6 +7272,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) intel_atomic_prepare_plane_clear_colors(state); + intel_fbc_prepare_dirty_rect(state); + for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) intel_atomic_dsb_finish(state, crtc); diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index ca44cec73fd2..ee2d75303e41 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -88,6 +88,7 @@ struct intel_fbc_state { u16 override_cfb_stride; u16 interval; s8 fence_id; + struct drm_rect dirty_rect; }; struct intel_fbc { @@ -523,6 +524,9 @@ static void ilk_fbc_deactivate(struct intel_fbc *fbc) struct intel_display *display = fbc->display; u32 dpfc_ctl; + if (HAS_FBC_DIRTY_RECT(display)) + intel_de_write(display, XE3_FBC_DIRTY_CTL(fbc->id), 0); + /* Disable compression */ dpfc_ctl = intel_de_read(display, ILK_DPFC_CONTROL(fbc->id)); if (dpfc_ctl & DPFC_CTL_EN) { @@ -665,6 +669,10 @@ static void ivb_fbc_activate(struct intel_fbc *fbc) if (DISPLAY_VER(display) >= 20) intel_de_write(display, ILK_DPFC_CONTROL(fbc->id), dpfc_ctl); + if (HAS_FBC_DIRTY_RECT(display)) + intel_de_write(display, XE3_FBC_DIRTY_CTL(fbc->id), + FBC_DIRTY_RECT_EN); + intel_de_write(display, ILK_DPFC_CONTROL(fbc->id), DPFC_CTL_EN | dpfc_ctl); } @@ -1196,6 +1204,82 @@ static bool tiling_is_valid(const struct intel_plane_state *plane_state) return i8xx_fbc_tiling_valid(plane_state); } +static void +intel_fbc_dirty_rect_update(struct intel_dsb *dsb, struct intel_fbc *fbc) +{ + struct intel_display *display = fbc->display; + const struct drm_rect *fbc_dirty_rect = &fbc->state.dirty_rect; + + lockdep_assert_held(&fbc->lock); + + intel_de_write_dsb(display, dsb, XE3_FBC_DIRTY_RECT(fbc->id), + FBC_DIRTY_RECT_START_LINE(fbc_dirty_rect->y1) | + FBC_DIRTY_RECT_END_LINE(fbc_dirty_rect->y2 - 1)); +} + +void +intel_fbc_dirty_rect_update_noarm(struct intel_dsb *dsb, + struct intel_plane *plane) +{ + struct intel_display *display = to_intel_display(plane); + struct intel_fbc *fbc = plane->fbc; + + if (!HAS_FBC_DIRTY_RECT(display)) + return; + + mutex_lock(&fbc->lock); + + if (fbc->state.plane == plane) + intel_fbc_dirty_rect_update(dsb, fbc); + + mutex_unlock(&fbc->lock); +} + +static void +__intel_fbc_prepare_dirty_rect(const struct intel_plane_state *plane_state) +{ + struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane); + struct intel_fbc *fbc = plane->fbc; + struct drm_rect *fbc_dirty_rect = &fbc->state.dirty_rect; + int width = drm_rect_width(&plane_state->uapi.src) >> 16; + const struct drm_rect *damage = &plane_state->damage; + int y_offset = plane_state->view.color_plane[0].y; + + lockdep_assert_held(&fbc->lock); + + if (drm_rect_visible(damage)) + *fbc_dirty_rect = *damage; + else + /* dirty rect must cover at least one line */ + *fbc_dirty_rect = DRM_RECT_INIT(0, y_offset, width, 1); +} + +void +intel_fbc_prepare_dirty_rect(struct intel_atomic_state *state) +{ + struct intel_display *display = to_intel_display(state); + struct intel_plane_state *plane_state; + struct intel_plane *plane; + int i; + + if (!HAS_FBC_DIRTY_RECT(display)) + return; + + for_each_new_intel_plane_in_state(state, plane, plane_state, i) { + struct intel_fbc *fbc = plane->fbc; + + if (!fbc) + continue; + + mutex_lock(&fbc->lock); + + if (fbc->state.plane == plane) + __intel_fbc_prepare_dirty_rect(plane_state); + + mutex_unlock(&fbc->lock); + } +} + static void intel_fbc_update_state(struct intel_atomic_state *state, struct intel_crtc *crtc, struct intel_plane *plane) diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h b/drivers/gpu/drm/i915/display/intel_fbc.h index 2e1dd7e8a18f..08743057ff14 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.h +++ b/drivers/gpu/drm/i915/display/intel_fbc.h @@ -13,6 +13,7 @@ struct intel_atomic_state; struct intel_crtc; struct intel_crtc_state; struct intel_display; +struct intel_dsb; struct intel_fbc; struct intel_plane; struct intel_plane_state; @@ -47,5 +48,8 @@ void intel_fbc_handle_fifo_underrun_irq(struct intel_display *display); void intel_fbc_reset_underrun(struct intel_display *display); void intel_fbc_crtc_debugfs_add(struct intel_crtc *crtc); void intel_fbc_debugfs_register(struct intel_display *display); +void intel_fbc_prepare_dirty_rect(struct intel_atomic_state *state); +void intel_fbc_dirty_rect_update_noarm(struct intel_dsb *dsb, + struct intel_plane *plane); #endif /* __INTEL_FBC_H__ */ -- 2.51.0 From e2364a56ad47d3299b1bf2fdb854359d4a770230 Mon Sep 17 00:00:00 2001 From: Vinod Govindapillai Date: Fri, 28 Feb 2025 11:38:01 +0200 Subject: [PATCH 07/16] drm/i915/fbc: disable FBC if PSR2 selective fetch is enabled MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit It is not recommended to have both FBC dirty rect and PSR2 selective fetch be enabled at the same time. Mark FBC as not possible, if PSR2 selective fetch is enabled. v2: fix the condition to disable FBC if PSR2 enabled (Jani) v3: use HAS_FBC_DIRTY_RECT() v4: Update to patch description Bspec: 68881 Reviewed-by: Ville Syrjälä Signed-off-by: Vinod Govindapillai Signed-off-by: Mika Kahola Link: https://patchwork.freedesktop.org/patch/msgid/20250228093802.27091-9-vinod.govindapillai@intel.com --- drivers/gpu/drm/i915/display/intel_fbc.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index ee2d75303e41..5b6a9315fa8f 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1415,9 +1415,14 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, * Display 12+ is not supporting FBC with PSR2. * Recommendation is to keep this combination disabled * Bspec: 50422 HSD: 14010260002 + * + * In Xe3, PSR2 selective fetch and FBC dirty rect feature cannot + * coexist. So if PSR2 selective fetch is supported then mark that + * FBC is not supported. + * TODO: Need a logic to decide between PSR2 and FBC Dirty rect */ - if (IS_DISPLAY_VER(display, 12, 14) && crtc_state->has_sel_update && - !crtc_state->has_panel_replay) { + if ((IS_DISPLAY_VER(display, 12, 14) || HAS_FBC_DIRTY_RECT(display)) && + crtc_state->has_sel_update && !crtc_state->has_panel_replay) { plane_state->no_fbc_reason = "PSR2 enabled"; return 0; } -- 2.51.0 From af23476af8a9ee881bd7a6ef5c94b6f4049ad096 Mon Sep 17 00:00:00 2001 From: Vinod Govindapillai Date: Fri, 28 Feb 2025 11:38:02 +0200 Subject: [PATCH 08/16] drm/i915/fbc: handle dirty rect coords for the first frame MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit During enabling FBC, for the very first frame, the prepare dirty rect routine wouldnt have executed as at that time the plane reference in the fbc_state would be NULL. So this could make driver program some invalid entries as the damage area. Though fbc hw ignores the dirty rect values programmed for the first frame after enabling FBC, driver must ensure that valid dirty rect coords are programmed. So ensure that for the first frame correct dirty rect coords are updated to the HW. Signed-off-by: Vinod Govindapillai Reviewed-by: Ville Syrjälä Signed-off-by: Mika Kahola Link: https://patchwork.freedesktop.org/patch/msgid/20250228093802.27091-10-vinod.govindapillai@intel.com --- drivers/gpu/drm/i915/display/intel_display.c | 3 +- drivers/gpu/drm/i915/display/intel_fbc.c | 137 +++++++++++++------ drivers/gpu/drm/i915/display/intel_fbc.h | 3 +- 3 files changed, 99 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index bdc25b8153e0..c4b0ec60fded 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7272,7 +7272,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) intel_atomic_prepare_plane_clear_colors(state); - intel_fbc_prepare_dirty_rect(state); + for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) + intel_fbc_prepare_dirty_rect(state, crtc); for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) intel_atomic_dsb_finish(state, crtc); diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 5b6a9315fa8f..b6978135e8ad 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1205,18 +1205,39 @@ static bool tiling_is_valid(const struct intel_plane_state *plane_state) } static void -intel_fbc_dirty_rect_update(struct intel_dsb *dsb, struct intel_fbc *fbc) +intel_fbc_invalidate_dirty_rect(struct intel_fbc *fbc) +{ + lockdep_assert_held(&fbc->lock); + + fbc->state.dirty_rect = DRM_RECT_INIT(0, 0, 0, 0); +} + +static void +intel_fbc_program_dirty_rect(struct intel_dsb *dsb, struct intel_fbc *fbc, + const struct drm_rect *fbc_dirty_rect) { struct intel_display *display = fbc->display; - const struct drm_rect *fbc_dirty_rect = &fbc->state.dirty_rect; - lockdep_assert_held(&fbc->lock); + drm_WARN_ON(display->drm, fbc_dirty_rect->y2 == 0); intel_de_write_dsb(display, dsb, XE3_FBC_DIRTY_RECT(fbc->id), FBC_DIRTY_RECT_START_LINE(fbc_dirty_rect->y1) | FBC_DIRTY_RECT_END_LINE(fbc_dirty_rect->y2 - 1)); } +static void +intel_fbc_dirty_rect_update(struct intel_dsb *dsb, struct intel_fbc *fbc) +{ + const struct drm_rect *fbc_dirty_rect = &fbc->state.dirty_rect; + + lockdep_assert_held(&fbc->lock); + + if (!drm_rect_visible(fbc_dirty_rect)) + return; + + intel_fbc_program_dirty_rect(dsb, fbc, fbc_dirty_rect); +} + void intel_fbc_dirty_rect_update_noarm(struct intel_dsb *dsb, struct intel_plane *plane) @@ -1236,48 +1257,19 @@ intel_fbc_dirty_rect_update_noarm(struct intel_dsb *dsb, } static void -__intel_fbc_prepare_dirty_rect(const struct intel_plane_state *plane_state) -{ - struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane); - struct intel_fbc *fbc = plane->fbc; - struct drm_rect *fbc_dirty_rect = &fbc->state.dirty_rect; - int width = drm_rect_width(&plane_state->uapi.src) >> 16; - const struct drm_rect *damage = &plane_state->damage; - int y_offset = plane_state->view.color_plane[0].y; - - lockdep_assert_held(&fbc->lock); - - if (drm_rect_visible(damage)) - *fbc_dirty_rect = *damage; - else - /* dirty rect must cover at least one line */ - *fbc_dirty_rect = DRM_RECT_INIT(0, y_offset, width, 1); -} - -void -intel_fbc_prepare_dirty_rect(struct intel_atomic_state *state) +intel_fbc_hw_intialize_dirty_rect(struct intel_fbc *fbc, + const struct intel_plane_state *plane_state) { - struct intel_display *display = to_intel_display(state); - struct intel_plane_state *plane_state; - struct intel_plane *plane; - int i; - - if (!HAS_FBC_DIRTY_RECT(display)) - return; - - for_each_new_intel_plane_in_state(state, plane, plane_state, i) { - struct intel_fbc *fbc = plane->fbc; + struct drm_rect src; - if (!fbc) - continue; - - mutex_lock(&fbc->lock); - - if (fbc->state.plane == plane) - __intel_fbc_prepare_dirty_rect(plane_state); + /* + * Initializing the FBC HW with the whole plane area as the dirty rect. + * This is to ensure that we have valid coords be written to the + * HW as dirty rect. + */ + drm_rect_fp_to_int(&src, &plane_state->uapi.src); - mutex_unlock(&fbc->lock); - } + intel_fbc_program_dirty_rect(NULL, fbc, &src); } static void intel_fbc_update_state(struct intel_atomic_state *state, @@ -1353,6 +1345,62 @@ static bool intel_fbc_is_ok(const struct intel_plane_state *plane_state) intel_fbc_is_cfb_ok(plane_state); } +static void +__intel_fbc_prepare_dirty_rect(const struct intel_plane_state *plane_state, + const struct intel_crtc_state *crtc_state) +{ + struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane); + struct intel_fbc *fbc = plane->fbc; + struct drm_rect *fbc_dirty_rect = &fbc->state.dirty_rect; + int width = drm_rect_width(&plane_state->uapi.src) >> 16; + const struct drm_rect *damage = &plane_state->damage; + int y_offset = plane_state->view.color_plane[0].y; + + lockdep_assert_held(&fbc->lock); + + if (intel_crtc_needs_modeset(crtc_state) || + !intel_fbc_is_ok(plane_state)) { + intel_fbc_invalidate_dirty_rect(fbc); + return; + } + + if (drm_rect_visible(damage)) + *fbc_dirty_rect = *damage; + else + /* dirty rect must cover at least one line */ + *fbc_dirty_rect = DRM_RECT_INIT(0, y_offset, width, 1); +} + +void +intel_fbc_prepare_dirty_rect(struct intel_atomic_state *state, + struct intel_crtc *crtc) +{ + struct intel_display *display = to_intel_display(state); + const struct intel_crtc_state *crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + struct intel_plane_state *plane_state; + struct intel_plane *plane; + int i; + + if (!HAS_FBC_DIRTY_RECT(display)) + return; + + for_each_new_intel_plane_in_state(state, plane, plane_state, i) { + struct intel_fbc *fbc = plane->fbc; + + if (!fbc || plane->pipe != crtc->pipe) + continue; + + mutex_lock(&fbc->lock); + + if (fbc->state.plane == plane) + __intel_fbc_prepare_dirty_rect(plane_state, + crtc_state); + + mutex_unlock(&fbc->lock); + } +} + static int intel_fbc_check_plane(struct intel_atomic_state *state, struct intel_plane *plane) { @@ -1629,6 +1677,8 @@ static void __intel_fbc_disable(struct intel_fbc *fbc) drm_dbg_kms(display->drm, "Disabling FBC on [PLANE:%d:%s]\n", plane->base.base.id, plane->base.name); + intel_fbc_invalidate_dirty_rect(fbc); + __intel_fbc_cleanup_cfb(fbc); fbc->state.plane = NULL; @@ -1814,6 +1864,9 @@ static void __intel_fbc_enable(struct intel_atomic_state *state, intel_fbc_update_state(state, crtc, plane); + if (HAS_FBC_DIRTY_RECT(display)) + intel_fbc_hw_intialize_dirty_rect(fbc, plane_state); + intel_fbc_program_workarounds(fbc); intel_fbc_program_cfb(fbc); } diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h b/drivers/gpu/drm/i915/display/intel_fbc.h index 08743057ff14..0e715cb6b4e6 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.h +++ b/drivers/gpu/drm/i915/display/intel_fbc.h @@ -48,7 +48,8 @@ void intel_fbc_handle_fifo_underrun_irq(struct intel_display *display); void intel_fbc_reset_underrun(struct intel_display *display); void intel_fbc_crtc_debugfs_add(struct intel_crtc *crtc); void intel_fbc_debugfs_register(struct intel_display *display); -void intel_fbc_prepare_dirty_rect(struct intel_atomic_state *state); +void intel_fbc_prepare_dirty_rect(struct intel_atomic_state *state, + struct intel_crtc *crtc); void intel_fbc_dirty_rect_update_noarm(struct intel_dsb *dsb, struct intel_plane *plane); -- 2.51.0 From 6498a5e010fdeeab14b678fba58a6097ffad7e31 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Mon, 3 Mar 2025 13:27:03 +0200 Subject: [PATCH 09/16] drm/i915/display: convert display reset to struct intel_display * Going forward, struct intel_display will be the main display device structure. Convert display reset to it as much as possible. Reviewed-by: Matt Roper Link: https://patchwork.freedesktop.org/patch/msgid/060c309189f1c084e012521822f4a0247f64528e.1741001054.git.jani.nikula@intel.com Signed-off-by: Jani Nikula --- .../drm/i915/display/intel_display_reset.c | 51 ++++++++++--------- .../drm/i915/display/intel_display_reset.h | 6 +-- drivers/gpu/drm/i915/gt/intel_reset.c | 7 ++- 3 files changed, 35 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.c b/drivers/gpu/drm/i915/display/intel_display_reset.c index a690968885bf..c1e448e8a26e 100644 --- a/drivers/gpu/drm/i915/display/intel_display_reset.c +++ b/drivers/gpu/drm/i915/display/intel_display_reset.c @@ -14,24 +14,27 @@ #include "intel_hotplug.h" #include "intel_pps.h" -static bool gpu_reset_clobbers_display(struct drm_i915_private *dev_priv) +static bool gpu_reset_clobbers_display(struct intel_display *display) { - return (INTEL_INFO(dev_priv)->gpu_reset_clobbers_display && - intel_has_gpu_reset(to_gt(dev_priv))); + struct drm_i915_private *i915 = to_i915(display->drm); + + return (INTEL_INFO(i915)->gpu_reset_clobbers_display && + intel_has_gpu_reset(to_gt(i915))); } -void intel_display_reset_prepare(struct drm_i915_private *dev_priv) +void intel_display_reset_prepare(struct intel_display *display) { - struct drm_modeset_acquire_ctx *ctx = &dev_priv->display.restore.reset_ctx; + struct drm_i915_private *dev_priv = to_i915(display->drm); + struct drm_modeset_acquire_ctx *ctx = &display->restore.reset_ctx; struct drm_atomic_state *state; int ret; - if (!HAS_DISPLAY(dev_priv)) + if (!HAS_DISPLAY(display)) return; /* reset doesn't touch the display */ - if (!dev_priv->display.params.force_reset_modeset_test && - !gpu_reset_clobbers_display(dev_priv)) + if (!display->params.force_reset_modeset_test && + !gpu_reset_clobbers_display(display)) return; /* We have a modeset vs reset deadlock, defensively unbreak it. */ @@ -40,7 +43,7 @@ void intel_display_reset_prepare(struct drm_i915_private *dev_priv) wake_up_bit(&to_gt(dev_priv)->reset.flags, I915_RESET_MODESET); if (atomic_read(&dev_priv->gpu_error.pending_fb_pin)) { - drm_dbg_kms(&dev_priv->drm, + drm_dbg_kms(display->drm, "Modeset potentially stuck, unbreaking through wedging\n"); intel_gt_set_wedged(to_gt(dev_priv)); } @@ -49,10 +52,10 @@ void intel_display_reset_prepare(struct drm_i915_private *dev_priv) * Need mode_config.mutex so that we don't * trample ongoing ->detect() and whatnot. */ - mutex_lock(&dev_priv->drm.mode_config.mutex); + mutex_lock(&display->drm->mode_config.mutex); drm_modeset_acquire_init(ctx, 0); while (1) { - ret = drm_modeset_lock_all_ctx(&dev_priv->drm, ctx); + ret = drm_modeset_lock_all_ctx(display->drm, ctx); if (ret != -EDEADLK) break; @@ -62,34 +65,34 @@ void intel_display_reset_prepare(struct drm_i915_private *dev_priv) * Disabling the crtcs gracefully seems nicer. Also the * g33 docs say we should at least disable all the planes. */ - state = drm_atomic_helper_duplicate_state(&dev_priv->drm, ctx); + state = drm_atomic_helper_duplicate_state(display->drm, ctx); if (IS_ERR(state)) { ret = PTR_ERR(state); - drm_err(&dev_priv->drm, "Duplicating state failed with %i\n", + drm_err(display->drm, "Duplicating state failed with %i\n", ret); return; } - ret = drm_atomic_helper_disable_all(&dev_priv->drm, ctx); + ret = drm_atomic_helper_disable_all(display->drm, ctx); if (ret) { - drm_err(&dev_priv->drm, "Suspending crtc's failed with %i\n", + drm_err(display->drm, "Suspending crtc's failed with %i\n", ret); drm_atomic_state_put(state); return; } - dev_priv->display.restore.modeset_state = state; + display->restore.modeset_state = state; state->acquire_ctx = ctx; } -void intel_display_reset_finish(struct drm_i915_private *i915) +void intel_display_reset_finish(struct intel_display *display) { - struct intel_display *display = &i915->display; + struct drm_i915_private *i915 = to_i915(display->drm); struct drm_modeset_acquire_ctx *ctx = &display->restore.reset_ctx; struct drm_atomic_state *state; int ret; - if (!HAS_DISPLAY(i915)) + if (!HAS_DISPLAY(display)) return; /* reset doesn't touch the display */ @@ -101,12 +104,12 @@ void intel_display_reset_finish(struct drm_i915_private *i915) goto unlock; /* reset doesn't touch the display */ - if (!gpu_reset_clobbers_display(i915)) { + if (!gpu_reset_clobbers_display(display)) { /* for testing only restore the display */ ret = drm_atomic_helper_commit_duplicated_state(state, ctx); if (ret) { - drm_WARN_ON(&i915->drm, ret == -EDEADLK); - drm_err(&i915->drm, + drm_WARN_ON(display->drm, ret == -EDEADLK); + drm_err(display->drm, "Restoring old state failed with %i\n", ret); } } else { @@ -122,7 +125,7 @@ void intel_display_reset_finish(struct drm_i915_private *i915) ret = __intel_display_driver_resume(display, state, ctx); if (ret) - drm_err(&i915->drm, + drm_err(display->drm, "Restoring old state failed with %i\n", ret); intel_hpd_poll_disable(i915); @@ -132,7 +135,7 @@ void intel_display_reset_finish(struct drm_i915_private *i915) unlock: drm_modeset_drop_locks(ctx); drm_modeset_acquire_fini(ctx); - mutex_unlock(&i915->drm.mode_config.mutex); + mutex_unlock(&display->drm->mode_config.mutex); clear_bit_unlock(I915_RESET_MODESET, &to_gt(i915)->reset.flags); } diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.h b/drivers/gpu/drm/i915/display/intel_display_reset.h index f06d0d35b86b..9a1fe99bfcd4 100644 --- a/drivers/gpu/drm/i915/display/intel_display_reset.h +++ b/drivers/gpu/drm/i915/display/intel_display_reset.h @@ -6,9 +6,9 @@ #ifndef __INTEL_RESET_H__ #define __INTEL_RESET_H__ -struct drm_i915_private; +struct intel_display; -void intel_display_reset_prepare(struct drm_i915_private *i915); -void intel_display_reset_finish(struct drm_i915_private *i915); +void intel_display_reset_prepare(struct intel_display *display); +void intel_display_reset_finish(struct intel_display *display); #endif /* __INTEL_RESET_H__ */ diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index b33007cd1504..a4b1fec52b21 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -1413,11 +1413,14 @@ static void intel_gt_reset_global(struct intel_gt *gt, /* Use a watchdog to ensure that our reset completes */ intel_wedge_on_timeout(&w, gt, 60 * HZ) { - intel_display_reset_prepare(gt->i915); + struct drm_i915_private *i915 = gt->i915; + struct intel_display *display = &i915->display; + + intel_display_reset_prepare(display); intel_gt_reset(gt, engine_mask, reason); - intel_display_reset_finish(gt->i915); + intel_display_reset_finish(display); } if (!test_bit(I915_WEDGED, >->reset.flags)) -- 2.51.0 From 30f2581b639e6a4a7c3139f3e5086447db5bb9e6 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Mon, 3 Mar 2025 13:27:04 +0200 Subject: [PATCH 10/16] drm/i915: move pending_fb_pin to struct intel_display pending_fb_pin is more about display than GPU reset. Move it to struct intel_display. The restore sub-struct already contains reset related members, so move it there. Reviewed-by: Matt Roper Link: https://patchwork.freedesktop.org/patch/msgid/ff779ae318610e6f6813474bcaa53851ffff909d.1741001054.git.jani.nikula@intel.com Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_display_core.h | 2 ++ drivers/gpu/drm/i915/display/intel_display_reset.c | 2 +- drivers/gpu/drm/i915/display/intel_dpt.c | 5 +++-- drivers/gpu/drm/i915/display/intel_fb_pin.c | 10 ++++++---- drivers/gpu/drm/i915/display/intel_overlay.c | 5 ++--- drivers/gpu/drm/i915/i915_gpu_error.h | 2 -- 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h index 554870d2494b..1970d4c15090 100644 --- a/drivers/gpu/drm/i915/display/intel_display_core.h +++ b/drivers/gpu/drm/i915/display/intel_display_core.h @@ -512,6 +512,8 @@ struct intel_display { /* restore state for suspend/resume and display reset */ struct drm_atomic_state *modeset_state; struct drm_modeset_acquire_ctx reset_ctx; + /* modeset stuck tracking for reset */ + atomic_t pending_fb_pin; u32 saveDSPARB; u32 saveSWF0[16]; u32 saveSWF1[16]; diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.c b/drivers/gpu/drm/i915/display/intel_display_reset.c index c1e448e8a26e..cef9536c461c 100644 --- a/drivers/gpu/drm/i915/display/intel_display_reset.c +++ b/drivers/gpu/drm/i915/display/intel_display_reset.c @@ -42,7 +42,7 @@ void intel_display_reset_prepare(struct intel_display *display) smp_mb__after_atomic(); wake_up_bit(&to_gt(dev_priv)->reset.flags, I915_RESET_MODESET); - if (atomic_read(&dev_priv->gpu_error.pending_fb_pin)) { + if (atomic_read(&display->restore.pending_fb_pin)) { drm_dbg_kms(display->drm, "Modeset potentially stuck, unbreaking through wedging\n"); intel_gt_set_wedged(to_gt(dev_priv)); diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c index fca7294b1def..0d8ebe38226e 100644 --- a/drivers/gpu/drm/i915/display/intel_dpt.c +++ b/drivers/gpu/drm/i915/display/intel_dpt.c @@ -125,6 +125,7 @@ struct i915_vma *intel_dpt_pin_to_ggtt(struct i915_address_space *vm, unsigned int alignment) { struct drm_i915_private *i915 = vm->i915; + struct intel_display *display = &i915->display; struct i915_dpt *dpt = i915_vm_to_dpt(vm); intel_wakeref_t wakeref; struct i915_vma *vma; @@ -137,7 +138,7 @@ struct i915_vma *intel_dpt_pin_to_ggtt(struct i915_address_space *vm, pin_flags |= PIN_MAPPABLE; wakeref = intel_runtime_pm_get(&i915->runtime_pm); - atomic_inc(&i915->gpu_error.pending_fb_pin); + atomic_inc(&display->restore.pending_fb_pin); for_i915_gem_ww(&ww, err, true) { err = i915_gem_object_lock(dpt->obj, &ww); @@ -167,7 +168,7 @@ struct i915_vma *intel_dpt_pin_to_ggtt(struct i915_address_space *vm, dpt->obj->mm.dirty = true; - atomic_dec(&i915->gpu_error.pending_fb_pin); + atomic_dec(&display->restore.pending_fb_pin); intel_runtime_pm_put(&i915->runtime_pm, wakeref); return err ? ERR_PTR(err) : vma; diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c b/drivers/gpu/drm/i915/display/intel_fb_pin.c index 204e7e3e48ca..30ac9b089ad6 100644 --- a/drivers/gpu/drm/i915/display/intel_fb_pin.c +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c @@ -25,6 +25,7 @@ intel_fb_pin_to_dpt(const struct drm_framebuffer *fb, struct i915_address_space *vm) { struct drm_device *dev = fb->dev; + struct intel_display *display = to_intel_display(dev); struct drm_i915_private *dev_priv = to_i915(dev); struct drm_gem_object *_obj = intel_fb_bo(fb); struct drm_i915_gem_object *obj = to_intel_bo(_obj); @@ -42,7 +43,7 @@ intel_fb_pin_to_dpt(const struct drm_framebuffer *fb, if (WARN_ON(!i915_gem_object_is_framebuffer(obj))) return ERR_PTR(-EINVAL); - atomic_inc(&dev_priv->gpu_error.pending_fb_pin); + atomic_inc(&display->restore.pending_fb_pin); for_i915_gem_ww(&ww, ret, true) { ret = i915_gem_object_lock(obj, &ww); @@ -97,7 +98,7 @@ intel_fb_pin_to_dpt(const struct drm_framebuffer *fb, i915_vma_get(vma); err: - atomic_dec(&dev_priv->gpu_error.pending_fb_pin); + atomic_dec(&display->restore.pending_fb_pin); return vma; } @@ -112,6 +113,7 @@ intel_fb_pin_to_ggtt(const struct drm_framebuffer *fb, unsigned long *out_flags) { struct drm_device *dev = fb->dev; + struct intel_display *display = to_intel_display(dev); struct drm_i915_private *dev_priv = to_i915(dev); struct drm_gem_object *_obj = intel_fb_bo(fb); struct drm_i915_gem_object *obj = to_intel_bo(_obj); @@ -136,7 +138,7 @@ intel_fb_pin_to_ggtt(const struct drm_framebuffer *fb, */ wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm); - atomic_inc(&dev_priv->gpu_error.pending_fb_pin); + atomic_inc(&display->restore.pending_fb_pin); /* * Valleyview is definitely limited to scanning out the first @@ -212,7 +214,7 @@ err: if (ret) vma = ERR_PTR(ret); - atomic_dec(&dev_priv->gpu_error.pending_fb_pin); + atomic_dec(&display->restore.pending_fb_pin); intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref); return vma; } diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c index 5c1b20af2a07..aff9a3455c1b 100644 --- a/drivers/gpu/drm/i915/display/intel_overlay.c +++ b/drivers/gpu/drm/i915/display/intel_overlay.c @@ -800,7 +800,6 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay, struct drm_intel_overlay_put_image *params) { struct intel_display *display = overlay->display; - struct drm_i915_private *dev_priv = to_i915(display->drm); struct overlay_registers __iomem *regs = overlay->regs; u32 swidth, swidthsw, sheight, ostride; enum pipe pipe = overlay->crtc->pipe; @@ -815,7 +814,7 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay, if (ret != 0) return ret; - atomic_inc(&dev_priv->gpu_error.pending_fb_pin); + atomic_inc(&display->restore.pending_fb_pin); vma = intel_overlay_pin_fb(new_bo); if (IS_ERR(vma)) { @@ -903,7 +902,7 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay, out_unpin: i915_vma_unpin(vma); out_pin_section: - atomic_dec(&dev_priv->gpu_error.pending_fb_pin); + atomic_dec(&display->restore.pending_fb_pin); return ret; } diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h index 78a8928562a9..749e1c55613e 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.h +++ b/drivers/gpu/drm/i915/i915_gpu_error.h @@ -224,8 +224,6 @@ struct i915_gpu_error { /* Protected by the above dev->gpu_error.lock. */ struct i915_gpu_coredump *first_error; - atomic_t pending_fb_pin; - /** Number of times the device has been reset (global) */ atomic_t reset_count; -- 2.51.0 From 711c39ea9885028a674a669d5b66e7f5e0651db8 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Mon, 3 Mar 2025 13:27:05 +0200 Subject: [PATCH 11/16] drm/i915/reset: add intel_gt_gpu_reset_clobbers_display() helper Add a helper for checking the gpu_reset_clobbers_display flag to make it easier to relocate the flag later. Reviewed-by: Matt Roper Link: https://patchwork.freedesktop.org/patch/msgid/434d5db7675ed9717b3beae1389008b68a961855.1741001054.git.jani.nikula@intel.com Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 2 +- drivers/gpu/drm/i915/gt/intel_reset.c | 15 +++++++++++---- drivers/gpu/drm/i915/gt/intel_reset.h | 2 ++ drivers/gpu/drm/i915/i915_driver.c | 2 +- 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index ec136eb12d48..39f6ba4bf1ab 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -677,7 +677,7 @@ void intel_engines_release(struct intel_gt *gt) * in case we aborted before completely initialising the engines. */ GEM_BUG_ON(intel_gt_pm_is_awake(gt)); - if (!INTEL_INFO(gt->i915)->gpu_reset_clobbers_display) + if (!intel_gt_gpu_reset_clobbers_display(gt)) intel_gt_reset_all_engines(gt); /* Decouple the backend; but keep the layout for late GPU resets */ diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index 175fa2db0551..3182f19b9837 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -158,7 +158,7 @@ void intel_gt_pm_init(struct intel_gt *gt) static bool reset_engines(struct intel_gt *gt) { - if (INTEL_INFO(gt->i915)->gpu_reset_clobbers_display) + if (intel_gt_gpu_reset_clobbers_display(gt)) return false; return intel_gt_reset_all_engines(gt) == 0; diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index a4b1fec52b21..2dbfb5c39d69 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -986,7 +986,7 @@ static void __intel_gt_set_wedged(struct intel_gt *gt) awake = reset_prepare(gt); /* Even if the GPU reset fails, it should still stop the engines */ - if (!INTEL_INFO(gt->i915)->gpu_reset_clobbers_display) + if (!intel_gt_gpu_reset_clobbers_display(gt)) intel_gt_reset_all_engines(gt); for_each_engine(engine, gt, id) @@ -1106,7 +1106,7 @@ static bool __intel_gt_unset_wedged(struct intel_gt *gt) /* We must reset pending GPU events before restoring our submission */ ok = !HAS_EXECLISTS(gt->i915); /* XXX better agnosticism desired */ - if (!INTEL_INFO(gt->i915)->gpu_reset_clobbers_display) + if (!intel_gt_gpu_reset_clobbers_display(gt)) ok = intel_gt_reset_all_engines(gt) == 0; if (!ok) { /* @@ -1178,6 +1178,13 @@ static int resume(struct intel_gt *gt) return 0; } +bool intel_gt_gpu_reset_clobbers_display(struct intel_gt *gt) +{ + struct drm_i915_private *i915 = gt->i915; + + return INTEL_INFO(i915)->gpu_reset_clobbers_display; +} + /** * intel_gt_reset - reset chip after a hang * @gt: #intel_gt to reset @@ -1234,7 +1241,7 @@ void intel_gt_reset(struct intel_gt *gt, goto error; } - if (INTEL_INFO(gt->i915)->gpu_reset_clobbers_display) + if (intel_gt_gpu_reset_clobbers_display(gt)) intel_irq_suspend(gt->i915); if (do_reset(gt, stalled_mask)) { @@ -1242,7 +1249,7 @@ void intel_gt_reset(struct intel_gt *gt, goto taint; } - if (INTEL_INFO(gt->i915)->gpu_reset_clobbers_display) + if (intel_gt_gpu_reset_clobbers_display(gt)) intel_irq_resume(gt->i915); intel_overlay_reset(display); diff --git a/drivers/gpu/drm/i915/gt/intel_reset.h b/drivers/gpu/drm/i915/gt/intel_reset.h index c00de353075c..724ea6d64f33 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.h +++ b/drivers/gpu/drm/i915/gt/intel_reset.h @@ -28,6 +28,8 @@ void intel_gt_handle_error(struct intel_gt *gt, const char *fmt, ...); #define I915_ERROR_CAPTURE BIT(0) +bool intel_gt_gpu_reset_clobbers_display(struct intel_gt *gt); + void intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t stalled_mask, const char *reason); diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index 613084fd0097..59bf2d45403f 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -200,7 +200,7 @@ static void intel_detect_preproduction_hw(struct drm_i915_private *dev_priv) static void sanitize_gpu(struct drm_i915_private *i915) { - if (!INTEL_INFO(i915)->gpu_reset_clobbers_display) { + if (!intel_gt_gpu_reset_clobbers_display(to_gt(i915))) { struct intel_gt *gt; unsigned int i; -- 2.51.0 From fddbcd1532930fc8732f3018135e75f5779a8f3a Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Mon, 3 Mar 2025 13:27:06 +0200 Subject: [PATCH 12/16] drm/i915/reset: add intel_display_reset_test() Add a helper for checking if we want to test display reset regardless of whether it's strictly necessary. This will come in handy in follow-up work where we want to check this from gt reset side. v2: Drop superfluous newline Cc: Matt Roper Reviewed-by: Matt Roper Link: https://patchwork.freedesktop.org/patch/msgid/487dec72f753302cd565c3a8164afa7fc1e12ed7.1741001054.git.jani.nikula@intel.com Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_display_reset.c | 7 ++++++- drivers/gpu/drm/i915/display/intel_display_reset.h | 3 +++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.c b/drivers/gpu/drm/i915/display/intel_display_reset.c index cef9536c461c..121679b4230f 100644 --- a/drivers/gpu/drm/i915/display/intel_display_reset.c +++ b/drivers/gpu/drm/i915/display/intel_display_reset.c @@ -22,6 +22,11 @@ static bool gpu_reset_clobbers_display(struct intel_display *display) intel_has_gpu_reset(to_gt(i915))); } +bool intel_display_reset_test(struct intel_display *display) +{ + return display->params.force_reset_modeset_test; +} + void intel_display_reset_prepare(struct intel_display *display) { struct drm_i915_private *dev_priv = to_i915(display->drm); @@ -33,7 +38,7 @@ void intel_display_reset_prepare(struct intel_display *display) return; /* reset doesn't touch the display */ - if (!display->params.force_reset_modeset_test && + if (!intel_display_reset_test(display) && !gpu_reset_clobbers_display(display)) return; diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.h b/drivers/gpu/drm/i915/display/intel_display_reset.h index 9a1fe99bfcd4..c1dd2e8d0914 100644 --- a/drivers/gpu/drm/i915/display/intel_display_reset.h +++ b/drivers/gpu/drm/i915/display/intel_display_reset.h @@ -6,8 +6,11 @@ #ifndef __INTEL_RESET_H__ #define __INTEL_RESET_H__ +#include + struct intel_display; +bool intel_display_reset_test(struct intel_display *display); void intel_display_reset_prepare(struct intel_display *display); void intel_display_reset_finish(struct intel_display *display); -- 2.51.0 From ea349ec038c40b4bc6f20a61137282569d944ee0 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Mon, 3 Mar 2025 13:27:07 +0200 Subject: [PATCH 13/16] drm/i915/reset: remove I915_RESET_MODESET flag Since commit d59cf7bb73f3 ("drm/i915/display: Use dma_fence interfaces instead of i915_sw_fence") we don't have anyone waiting on the I915_RESET_MODESET bit, and there's no need for its semantics. Instead, simply return true from intel_display_reset_prepare() to indicate that intel_display_reset_finish() should be called. Cc: Matt Roper Reviewed-by: Matt Roper Link: https://patchwork.freedesktop.org/patch/msgid/294690db3fae8fec7f356edf467e79882ed494db.1741001054.git.jani.nikula@intel.com Signed-off-by: Jani Nikula --- .../drm/i915/display/intel_display_reset.c | 24 +++++++------------ .../drm/i915/display/intel_display_reset.h | 2 +- drivers/gpu/drm/i915/gt/intel_reset.c | 8 ++++--- drivers/gpu/drm/i915/gt/intel_reset_types.h | 3 +-- 4 files changed, 15 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.c b/drivers/gpu/drm/i915/display/intel_display_reset.c index 121679b4230f..acc728c75328 100644 --- a/drivers/gpu/drm/i915/display/intel_display_reset.c +++ b/drivers/gpu/drm/i915/display/intel_display_reset.c @@ -27,7 +27,8 @@ bool intel_display_reset_test(struct intel_display *display) return display->params.force_reset_modeset_test; } -void intel_display_reset_prepare(struct intel_display *display) +/* returns true if intel_display_reset_finish() needs to be called */ +bool intel_display_reset_prepare(struct intel_display *display) { struct drm_i915_private *dev_priv = to_i915(display->drm); struct drm_modeset_acquire_ctx *ctx = &display->restore.reset_ctx; @@ -35,17 +36,12 @@ void intel_display_reset_prepare(struct intel_display *display) int ret; if (!HAS_DISPLAY(display)) - return; + return false; /* reset doesn't touch the display */ if (!intel_display_reset_test(display) && !gpu_reset_clobbers_display(display)) - return; - - /* We have a modeset vs reset deadlock, defensively unbreak it. */ - set_bit(I915_RESET_MODESET, &to_gt(dev_priv)->reset.flags); - smp_mb__after_atomic(); - wake_up_bit(&to_gt(dev_priv)->reset.flags, I915_RESET_MODESET); + return false; if (atomic_read(&display->restore.pending_fb_pin)) { drm_dbg_kms(display->drm, @@ -75,7 +71,7 @@ void intel_display_reset_prepare(struct intel_display *display) ret = PTR_ERR(state); drm_err(display->drm, "Duplicating state failed with %i\n", ret); - return; + return true; } ret = drm_atomic_helper_disable_all(display->drm, ctx); @@ -83,11 +79,13 @@ void intel_display_reset_prepare(struct intel_display *display) drm_err(display->drm, "Suspending crtc's failed with %i\n", ret); drm_atomic_state_put(state); - return; + return true; } display->restore.modeset_state = state; state->acquire_ctx = ctx; + + return true; } void intel_display_reset_finish(struct intel_display *display) @@ -100,10 +98,6 @@ void intel_display_reset_finish(struct intel_display *display) if (!HAS_DISPLAY(display)) return; - /* reset doesn't touch the display */ - if (!test_bit(I915_RESET_MODESET, &to_gt(i915)->reset.flags)) - return; - state = fetch_and_zero(&display->restore.modeset_state); if (!state) goto unlock; @@ -141,6 +135,4 @@ unlock: drm_modeset_drop_locks(ctx); drm_modeset_acquire_fini(ctx); mutex_unlock(&display->drm->mode_config.mutex); - - clear_bit_unlock(I915_RESET_MODESET, &to_gt(i915)->reset.flags); } diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.h b/drivers/gpu/drm/i915/display/intel_display_reset.h index c1dd2e8d0914..311b5af8ca0c 100644 --- a/drivers/gpu/drm/i915/display/intel_display_reset.h +++ b/drivers/gpu/drm/i915/display/intel_display_reset.h @@ -11,7 +11,7 @@ struct intel_display; bool intel_display_reset_test(struct intel_display *display); -void intel_display_reset_prepare(struct intel_display *display); +bool intel_display_reset_prepare(struct intel_display *display); void intel_display_reset_finish(struct intel_display *display); #endif /* __INTEL_RESET_H__ */ diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index 2dbfb5c39d69..d8425ce019df 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -1422,12 +1422,14 @@ static void intel_gt_reset_global(struct intel_gt *gt, intel_wedge_on_timeout(&w, gt, 60 * HZ) { struct drm_i915_private *i915 = gt->i915; struct intel_display *display = &i915->display; + bool reset_display; - intel_display_reset_prepare(display); + reset_display = intel_display_reset_prepare(display); intel_gt_reset(gt, engine_mask, reason); - intel_display_reset_finish(display); + if (reset_display) + intel_display_reset_finish(display); } if (!test_bit(I915_WEDGED, >->reset.flags)) @@ -1495,7 +1497,7 @@ void intel_gt_handle_error(struct intel_gt *gt, intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) { local_bh_disable(); for_each_engine_masked(engine, gt, engine_mask, tmp) { - BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE); + BUILD_BUG_ON(I915_RESET_BACKOFF >= I915_RESET_ENGINE); if (test_and_set_bit(I915_RESET_ENGINE + engine->id, >->reset.flags)) continue; diff --git a/drivers/gpu/drm/i915/gt/intel_reset_types.h b/drivers/gpu/drm/i915/gt/intel_reset_types.h index 80351f0a856c..4f5fd393af6f 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset_types.h +++ b/drivers/gpu/drm/i915/gt/intel_reset_types.h @@ -41,8 +41,7 @@ struct intel_reset { */ unsigned long flags; #define I915_RESET_BACKOFF 0 -#define I915_RESET_MODESET 1 -#define I915_RESET_ENGINE 2 +#define I915_RESET_ENGINE 1 #define I915_WEDGED_ON_INIT (BITS_PER_LONG - 3) #define I915_WEDGED_ON_FINI (BITS_PER_LONG - 2) #define I915_WEDGED (BITS_PER_LONG - 1) -- 2.51.0 From 4684498cf9991e97a001ef5814391c7f7321ff99 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Mon, 3 Mar 2025 13:27:08 +0200 Subject: [PATCH 14/16] drm/i915/reset: decide whether display reset is needed on gt side Move the checks for whether display reset is needed at all to gt side of things. This way, we can decide to skip the display calls altogether if display reset is not required. Cc: Matt Roper Reviewed-by: Matt Roper Link: https://patchwork.freedesktop.org/patch/msgid/c32a88f292f516ec702bd07001ac609b8acc2888.1741001054.git.jani.nikula@intel.com Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_display_reset.c | 5 ----- drivers/gpu/drm/i915/gt/intel_reset.c | 10 +++++++++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.c b/drivers/gpu/drm/i915/display/intel_display_reset.c index acc728c75328..c48d822db58e 100644 --- a/drivers/gpu/drm/i915/display/intel_display_reset.c +++ b/drivers/gpu/drm/i915/display/intel_display_reset.c @@ -38,11 +38,6 @@ bool intel_display_reset_prepare(struct intel_display *display) if (!HAS_DISPLAY(display)) return false; - /* reset doesn't touch the display */ - if (!intel_display_reset_test(display) && - !gpu_reset_clobbers_display(display)) - return false; - if (atomic_read(&display->restore.pending_fb_pin)) { drm_dbg_kms(display->drm, "Modeset potentially stuck, unbreaking through wedging\n"); diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index d8425ce019df..23f3fdaadb33 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -1422,9 +1422,17 @@ static void intel_gt_reset_global(struct intel_gt *gt, intel_wedge_on_timeout(&w, gt, 60 * HZ) { struct drm_i915_private *i915 = gt->i915; struct intel_display *display = &i915->display; + bool need_display_reset; bool reset_display; - reset_display = intel_display_reset_prepare(display); + need_display_reset = intel_gt_gpu_reset_clobbers_display(gt) && + intel_has_gpu_reset(gt); + + reset_display = intel_display_reset_test(display) || + need_display_reset; + + if (reset_display) + reset_display = intel_display_reset_prepare(display); intel_gt_reset(gt, engine_mask, reason); -- 2.51.0 From d1b97b121e3c2bbb3c74fe91e42d13e59fd9d96e Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Mon, 3 Mar 2025 13:27:09 +0200 Subject: [PATCH 15/16] drm/i915/reset: pass test only parameter to intel_display_reset_finish() Deduplicate the gpu_reset_clobbers_display() part by passing the information in from gt side. Cc: Matt Roper Reviewed-by: Matt Roper Link: https://patchwork.freedesktop.org/patch/msgid/a36481db334fedcde50ae0e66c4d57825cae8cb7.1741001054.git.jani.nikula@intel.com Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_display_reset.c | 12 ++---------- drivers/gpu/drm/i915/display/intel_display_reset.h | 2 +- drivers/gpu/drm/i915/gt/intel_reset.c | 2 +- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.c b/drivers/gpu/drm/i915/display/intel_display_reset.c index c48d822db58e..d5ce0ac43377 100644 --- a/drivers/gpu/drm/i915/display/intel_display_reset.c +++ b/drivers/gpu/drm/i915/display/intel_display_reset.c @@ -14,14 +14,6 @@ #include "intel_hotplug.h" #include "intel_pps.h" -static bool gpu_reset_clobbers_display(struct intel_display *display) -{ - struct drm_i915_private *i915 = to_i915(display->drm); - - return (INTEL_INFO(i915)->gpu_reset_clobbers_display && - intel_has_gpu_reset(to_gt(i915))); -} - bool intel_display_reset_test(struct intel_display *display) { return display->params.force_reset_modeset_test; @@ -83,7 +75,7 @@ bool intel_display_reset_prepare(struct intel_display *display) return true; } -void intel_display_reset_finish(struct intel_display *display) +void intel_display_reset_finish(struct intel_display *display, bool test_only) { struct drm_i915_private *i915 = to_i915(display->drm); struct drm_modeset_acquire_ctx *ctx = &display->restore.reset_ctx; @@ -98,7 +90,7 @@ void intel_display_reset_finish(struct intel_display *display) goto unlock; /* reset doesn't touch the display */ - if (!gpu_reset_clobbers_display(display)) { + if (test_only) { /* for testing only restore the display */ ret = drm_atomic_helper_commit_duplicated_state(state, ctx); if (ret) { diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.h b/drivers/gpu/drm/i915/display/intel_display_reset.h index 311b5af8ca0c..f518147199a1 100644 --- a/drivers/gpu/drm/i915/display/intel_display_reset.h +++ b/drivers/gpu/drm/i915/display/intel_display_reset.h @@ -12,6 +12,6 @@ struct intel_display; bool intel_display_reset_test(struct intel_display *display); bool intel_display_reset_prepare(struct intel_display *display); -void intel_display_reset_finish(struct intel_display *display); +void intel_display_reset_finish(struct intel_display *display, bool test_only); #endif /* __INTEL_RESET_H__ */ diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index 23f3fdaadb33..9a92afcd9b0b 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -1437,7 +1437,7 @@ static void intel_gt_reset_global(struct intel_gt *gt, intel_gt_reset(gt, engine_mask, reason); if (reset_display) - intel_display_reset_finish(display); + intel_display_reset_finish(display, !need_display_reset); } if (!test_bit(I915_WEDGED, >->reset.flags)) -- 2.51.0 From 916f2740b82a1b58dce2bbd51c9130ae77a56e25 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Mon, 3 Mar 2025 13:27:10 +0200 Subject: [PATCH 16/16] drm/i915/reset: add modeset_stuck callback to intel_display_reset_prepare() Drop the dependency on gt by providing a callback for trying to unbreak stuck modeset. Do intel_gt_set_wedged() via the callback. It's by no means pretty, but this is perhaps the most straightforward alternative. Cc: Matt Roper Reviewed-by: Matt Roper Link: https://patchwork.freedesktop.org/patch/msgid/d322e20927326afa47c0df8a4d4776ee77010e6d.1741001054.git.jani.nikula@intel.com Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_display_reset.c | 6 +++--- drivers/gpu/drm/i915/display/intel_display_reset.h | 5 ++++- drivers/gpu/drm/i915/gt/intel_reset.c | 9 ++++++++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.c b/drivers/gpu/drm/i915/display/intel_display_reset.c index d5ce0ac43377..1f2798404f2c 100644 --- a/drivers/gpu/drm/i915/display/intel_display_reset.c +++ b/drivers/gpu/drm/i915/display/intel_display_reset.c @@ -20,9 +20,9 @@ bool intel_display_reset_test(struct intel_display *display) } /* returns true if intel_display_reset_finish() needs to be called */ -bool intel_display_reset_prepare(struct intel_display *display) +bool intel_display_reset_prepare(struct intel_display *display, + modeset_stuck_fn modeset_stuck, void *context) { - struct drm_i915_private *dev_priv = to_i915(display->drm); struct drm_modeset_acquire_ctx *ctx = &display->restore.reset_ctx; struct drm_atomic_state *state; int ret; @@ -33,7 +33,7 @@ bool intel_display_reset_prepare(struct intel_display *display) if (atomic_read(&display->restore.pending_fb_pin)) { drm_dbg_kms(display->drm, "Modeset potentially stuck, unbreaking through wedging\n"); - intel_gt_set_wedged(to_gt(dev_priv)); + modeset_stuck(context); } /* diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.h b/drivers/gpu/drm/i915/display/intel_display_reset.h index f518147199a1..8b3bda134454 100644 --- a/drivers/gpu/drm/i915/display/intel_display_reset.h +++ b/drivers/gpu/drm/i915/display/intel_display_reset.h @@ -10,8 +10,11 @@ struct intel_display; +typedef void modeset_stuck_fn(void *context); + bool intel_display_reset_test(struct intel_display *display); -bool intel_display_reset_prepare(struct intel_display *display); +bool intel_display_reset_prepare(struct intel_display *display, + modeset_stuck_fn modeset_stuck, void *context); void intel_display_reset_finish(struct intel_display *display, bool test_only); #endif /* __INTEL_RESET_H__ */ diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index 9a92afcd9b0b..3ee544e7c203 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -1403,6 +1403,11 @@ int intel_engine_reset(struct intel_engine_cs *engine, const char *msg) return err; } +static void display_reset_modeset_stuck(void *gt) +{ + intel_gt_set_wedged(gt); +} + static void intel_gt_reset_global(struct intel_gt *gt, u32 engine_mask, const char *reason) @@ -1432,7 +1437,9 @@ static void intel_gt_reset_global(struct intel_gt *gt, need_display_reset; if (reset_display) - reset_display = intel_display_reset_prepare(display); + reset_display = intel_display_reset_prepare(display, + display_reset_modeset_stuck, + gt); intel_gt_reset(gt, engine_mask, reason); -- 2.51.0