Quoting Jani Nikula (2023-04-13 06:47:33)
> Split out the display reset functionality to a separate file to
> declutter intel_display.c. Rename the functions accordingly. The minor
> downside is having to expose __intel_display_resume().
>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
Reviewed-by: Gustavo Sousa <gustavo.sousa@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/display/intel_display.c | 123 +---------------
> drivers/gpu/drm/i915/display/intel_display.h | 8 +-
> .../drm/i915/display/intel_display_reset.c | 135 ++++++++++++++++++
> .../drm/i915/display/intel_display_reset.h | 14 ++
> drivers/gpu/drm/i915/gt/intel_reset.c | 6 +-
> 6 files changed, 160 insertions(+), 127 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/display/intel_display_reset.c
> create mode 100644 drivers/gpu/drm/i915/display/intel_display_reset.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 91f0c214ef28..39e22cf85e55 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -241,6 +241,7 @@ i915-y += \
> display/intel_display_power.o \
> display/intel_display_power_map.o \
> display/intel_display_power_well.o \
> + display/intel_display_reset.o \
> display/intel_display_rps.o \
> display/intel_dmc.o \
> display/intel_dpio_phy.o \
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index f425e5ed155b..e89e9473a744 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -693,7 +693,7 @@ intel_plane_fence_y_offset(const struct intel_plane_state *plane_state)
> return y;
> }
>
> -static int
> +int
> __intel_display_resume(struct drm_i915_private *i915,
> struct drm_atomic_state *state,
> struct drm_modeset_acquire_ctx *ctx)
> @@ -733,127 +733,6 @@ __intel_display_resume(struct drm_i915_private *i915,
> return ret;
> }
>
> -static bool gpu_reset_clobbers_display(struct drm_i915_private *dev_priv)
> -{
> - return (INTEL_INFO(dev_priv)->gpu_reset_clobbers_display &&
> - intel_has_gpu_reset(to_gt(dev_priv)));
> -}
> -
> -void intel_display_prepare_reset(struct drm_i915_private *dev_priv)
> -{
> - struct drm_modeset_acquire_ctx *ctx = &dev_priv->display.restore.reset_ctx;
> - struct drm_atomic_state *state;
> - int ret;
> -
> - if (!HAS_DISPLAY(dev_priv))
> - return;
> -
> - /* reset doesn't touch the display */
> - if (!dev_priv->params.force_reset_modeset_test &&
> - !gpu_reset_clobbers_display(dev_priv))
> - 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);
> -
> - if (atomic_read(&dev_priv->gpu_error.pending_fb_pin)) {
> - drm_dbg_kms(&dev_priv->drm,
> - "Modeset potentially stuck, unbreaking through wedging\n");
> - intel_gt_set_wedged(to_gt(dev_priv));
> - }
> -
> - /*
> - * Need mode_config.mutex so that we don't
> - * trample ongoing ->detect() and whatnot.
> - */
> - mutex_lock(&dev_priv->drm.mode_config.mutex);
> - drm_modeset_acquire_init(ctx, 0);
> - while (1) {
> - ret = drm_modeset_lock_all_ctx(&dev_priv->drm, ctx);
> - if (ret != -EDEADLK)
> - break;
> -
> - drm_modeset_backoff(ctx);
> - }
> - /*
> - * 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);
> - if (IS_ERR(state)) {
> - ret = PTR_ERR(state);
> - drm_err(&dev_priv->drm, "Duplicating state failed with %i\n",
> - ret);
> - return;
> - }
> -
> - ret = drm_atomic_helper_disable_all(&dev_priv->drm, ctx);
> - if (ret) {
> - drm_err(&dev_priv->drm, "Suspending crtc's failed with %i\n",
> - ret);
> - drm_atomic_state_put(state);
> - return;
> - }
> -
> - dev_priv->display.restore.modeset_state = state;
> - state->acquire_ctx = ctx;
> -}
> -
> -void intel_display_finish_reset(struct drm_i915_private *i915)
> -{
> - struct drm_modeset_acquire_ctx *ctx = &i915->display.restore.reset_ctx;
> - struct drm_atomic_state *state;
> - int ret;
> -
> - if (!HAS_DISPLAY(i915))
> - return;
> -
> - /* reset doesn't touch the display */
> - if (!test_bit(I915_RESET_MODESET, &to_gt(i915)->reset.flags))
> - return;
> -
> - state = fetch_and_zero(&i915->display.restore.modeset_state);
> - if (!state)
> - goto unlock;
> -
> - /* reset doesn't touch the display */
> - if (!gpu_reset_clobbers_display(i915)) {
> - /* 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,
> - "Restoring old state failed with %i\n", ret);
> - }
> - } else {
> - /*
> - * The display has been reset as well,
> - * so need a full re-initialization.
> - */
> - intel_pps_unlock_regs_wa(i915);
> - intel_display_driver_init_hw(i915);
> - intel_clock_gating_init(i915);
> - intel_hpd_init(i915);
> -
> - ret = __intel_display_resume(i915, state, ctx);
> - if (ret)
> - drm_err(&i915->drm,
> - "Restoring old state failed with %i\n", ret);
> -
> - intel_hpd_poll_disable(i915);
> - }
> -
> - drm_atomic_state_put(state);
> -unlock:
> - drm_modeset_drop_locks(ctx);
> - drm_modeset_acquire_fini(ctx);
> - mutex_unlock(&i915->drm.mode_config.mutex);
> -
> - clear_bit_unlock(I915_RESET_MODESET, &to_gt(i915)->reset.flags);
> -}
> -
> static void icl_set_pipe_chicken(const struct intel_crtc_state *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index f82987fbc94a..e5bf8ef8e06b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -468,8 +468,6 @@ intel_framebuffer_create(struct drm_i915_gem_object *obj,
>
> bool intel_fuzzy_clock_check(int clock1, int clock2);
>
> -void intel_display_prepare_reset(struct drm_i915_private *dev_priv);
> -void intel_display_finish_reset(struct drm_i915_private *dev_priv);
> void intel_zero_m_n(struct intel_link_m_n *m_n);
> void intel_set_m_n(struct drm_i915_private *i915,
> const struct intel_link_m_n *m_n,
> @@ -545,6 +543,12 @@ int intel_atomic_check(struct drm_device *dev, struct drm_atomic_state *_state);
>
> void intel_hpd_poll_fini(struct drm_i915_private *i915);
>
> +/* interface for intel_display_reset.c */
> +int
> +__intel_display_resume(struct drm_i915_private *i915,
> + struct drm_atomic_state *state,
> + struct drm_modeset_acquire_ctx *ctx);
> +
> /* modesetting asserts */
> void assert_transcoder(struct drm_i915_private *dev_priv,
> enum transcoder cpu_transcoder, bool state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.c b/drivers/gpu/drm/i915/display/intel_display_reset.c
> new file mode 100644
> index 000000000000..166aa0cab1fc
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_display_reset.c
> @@ -0,0 +1,135 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#include <drm/drm_atomic_helper.h>
> +
> +#include "i915_drv.h"
> +#include "intel_clock_gating.h"
> +#include "intel_display_driver.h"
> +#include "intel_display_reset.h"
> +#include "intel_display_types.h"
> +#include "intel_hotplug.h"
> +#include "intel_pps.h"
> +
> +static bool gpu_reset_clobbers_display(struct drm_i915_private *dev_priv)
> +{
> + return (INTEL_INFO(dev_priv)->gpu_reset_clobbers_display &&
> + intel_has_gpu_reset(to_gt(dev_priv)));
> +}
> +
> +void intel_display_reset_prepare(struct drm_i915_private *dev_priv)
> +{
> + struct drm_modeset_acquire_ctx *ctx = &dev_priv->display.restore.reset_ctx;
> + struct drm_atomic_state *state;
> + int ret;
> +
> + if (!HAS_DISPLAY(dev_priv))
> + return;
> +
> + /* reset doesn't touch the display */
> + if (!dev_priv->params.force_reset_modeset_test &&
> + !gpu_reset_clobbers_display(dev_priv))
> + 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);
> +
> + if (atomic_read(&dev_priv->gpu_error.pending_fb_pin)) {
> + drm_dbg_kms(&dev_priv->drm,
> + "Modeset potentially stuck, unbreaking through wedging\n");
> + intel_gt_set_wedged(to_gt(dev_priv));
> + }
> +
> + /*
> + * Need mode_config.mutex so that we don't
> + * trample ongoing ->detect() and whatnot.
> + */
> + mutex_lock(&dev_priv->drm.mode_config.mutex);
> + drm_modeset_acquire_init(ctx, 0);
> + while (1) {
> + ret = drm_modeset_lock_all_ctx(&dev_priv->drm, ctx);
> + if (ret != -EDEADLK)
> + break;
> +
> + drm_modeset_backoff(ctx);
> + }
> + /*
> + * 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);
> + if (IS_ERR(state)) {
> + ret = PTR_ERR(state);
> + drm_err(&dev_priv->drm, "Duplicating state failed with %i\n",
> + ret);
> + return;
> + }
> +
> + ret = drm_atomic_helper_disable_all(&dev_priv->drm, ctx);
> + if (ret) {
> + drm_err(&dev_priv->drm, "Suspending crtc's failed with %i\n",
> + ret);
> + drm_atomic_state_put(state);
> + return;
> + }
> +
> + dev_priv->display.restore.modeset_state = state;
> + state->acquire_ctx = ctx;
> +}
> +
> +void intel_display_reset_finish(struct drm_i915_private *i915)
> +{
> + struct drm_modeset_acquire_ctx *ctx = &i915->display.restore.reset_ctx;
> + struct drm_atomic_state *state;
> + int ret;
> +
> + if (!HAS_DISPLAY(i915))
> + return;
> +
> + /* reset doesn't touch the display */
> + if (!test_bit(I915_RESET_MODESET, &to_gt(i915)->reset.flags))
> + return;
> +
> + state = fetch_and_zero(&i915->display.restore.modeset_state);
> + if (!state)
> + goto unlock;
> +
> + /* reset doesn't touch the display */
> + if (!gpu_reset_clobbers_display(i915)) {
> + /* 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,
> + "Restoring old state failed with %i\n", ret);
> + }
> + } else {
> + /*
> + * The display has been reset as well,
> + * so need a full re-initialization.
> + */
> + intel_pps_unlock_regs_wa(i915);
> + intel_display_driver_init_hw(i915);
> + intel_clock_gating_init(i915);
> + intel_hpd_init(i915);
> +
> + ret = __intel_display_resume(i915, state, ctx);
> + if (ret)
> + drm_err(&i915->drm,
> + "Restoring old state failed with %i\n", ret);
> +
> + intel_hpd_poll_disable(i915);
> + }
> +
> + drm_atomic_state_put(state);
> +unlock:
> + drm_modeset_drop_locks(ctx);
> + drm_modeset_acquire_fini(ctx);
> + mutex_unlock(&i915->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
> new file mode 100644
> index 000000000000..f06d0d35b86b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_display_reset.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef __INTEL_RESET_H__
> +#define __INTEL_RESET_H__
> +
> +struct drm_i915_private;
> +
> +void intel_display_reset_prepare(struct drm_i915_private *i915);
> +void intel_display_reset_finish(struct drm_i915_private *i915);
> +
> +#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 797ea8340467..6194212e8650 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -7,7 +7,7 @@
> #include <linux/stop_machine.h>
> #include <linux/string_helpers.h>
>
> -#include "display/intel_display.h"
> +#include "display/intel_display_reset.h"
> #include "display/intel_overlay.h"
>
> #include "gem/i915_gem_context.h"
> @@ -1370,11 +1370,11 @@ 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_prepare_reset(gt->i915);
> + intel_display_reset_prepare(gt->i915);
>
> intel_gt_reset(gt, engine_mask, reason);
>
> - intel_display_finish_reset(gt->i915);
> + intel_display_reset_finish(gt->i915);
> }
>
> if (!test_bit(I915_WEDGED, >->reset.flags))
> --
> 2.39.2
>
