On Wed, Sep 11, 2019 at 11:19:59AM +0100, Chris Wilson wrote:
> If the display is inactive, we need not worry about the gpu reset
> clobbering the display! To prevent the display changing state between us
> checking the active state and doing the hard reset, we introduce the
> lightweight reset lock to the atomic commit for the affected (legacy)
> platforms.
>
> Testcase: igt/gem_eio/kms
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 11 +++++++++++
> drivers/gpu/drm/i915/gt/intel_reset.c | 18 +++++++++++++++++-
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 4ee750fa3ef0..a92487d8f4cf 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13986,6 +13986,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
> struct intel_crtc *crtc;
> u64 put_domains[I915_MAX_PIPES] = {};
> intel_wakeref_t wakeref = 0;
> + int srcu;
> int i;
>
> intel_atomic_commit_fence_wait(state);
> @@ -14005,6 +14006,12 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
> }
> }
>
> + /* Prevent starting a GPU reset while we change global display state */
> + srcu = -ENODEV;
> + if (INTEL_INFO(dev_priv)->gpu_reset_clobbers_display)
> + /* only fails if interrupted */
> + srcu = intel_gt_reset_trylock(&dev_priv->gt);
Hmm. What happens if we're holding the modeset locks and there's an
ongoing reset blocked on said locks?
> +
> intel_commit_modeset_disables(state);
>
> /* FIXME: Eventually get rid of our crtc->config pointer */
> @@ -14049,6 +14056,10 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
> /* Now enable the clocks, plane, pipe, and connectors that we set up. */
> dev_priv->display.commit_modeset_enables(state);
>
> +
> + if (srcu > 0)
> + intel_gt_reset_unlock(&dev_priv->gt, srcu);
> +
> if (state->modeset) {
> intel_encoders_update_complete(state);
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index d8b1498d7ab7..df4a86bdb6f6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -7,6 +7,7 @@
> #include <linux/sched/mm.h>
> #include <linux/stop_machine.h>
>
> +#include "display/intel_display.h"
> #include "display/intel_display_types.h"
> #include "display/intel_overlay.h"
>
> @@ -729,6 +730,21 @@ static void nop_submit_request(struct i915_request *request)
> intel_engine_queue_breadcrumbs(engine);
> }
>
> +static bool reset_clobbers_display(struct drm_i915_private *i915)
> +{
> + struct intel_crtc *crtc;
> +
> + if (!INTEL_INFO(i915)->gpu_reset_clobbers_display)
> + return false;
> +
> + for_each_intel_crtc(&i915->drm, crtc) {
> + if (crtc->active)
> + return true;
> + }
> +
> + return false;
> +}
> +
> static void __intel_gt_set_wedged(struct intel_gt *gt)
> {
> struct intel_engine_cs *engine;
> @@ -755,7 +771,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 (!reset_clobbers_display(gt->i915))
> __intel_gt_reset(gt, ALL_ENGINES);
>
> for_each_engine(engine, gt->i915, id)
> --
> 2.23.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
