On Thu, May 04, 2023 at 02:10:47AM +0300, Imre Deak wrote:
> This patch simplifying the handling of modeset locks and atomic state
> for an atomic commit is based on
>
> https://lore.kernel.org/all/20210715184954.7794-2-ville.syrj...@linux.intel.com/
>
> adding the helper to i915. I find this approach preferrable than
> open-coding the corresponding steps (fixed for me an atomic
> state reset during a DEADLK retry, which I missed in the open-coded
> version) and also better than the existing
> DRM_MODESET_LOCK_ALL_BEGIN/END macros for the reasons described in the
> above original patchset.
>
> This change takes the helper into use only for atomic commits during DDI
> hotplug handling, as a preparation for a follow-up patch adding a
> similar commit started from the same spot. Other places doing a
> driver-internal atomic commit is to be converted by a follow-up
> patchset.
>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Signed-off-by: Imre Deak <imre.d...@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/display/intel_ddi.c | 17 ++-----
> .../gpu/drm/i915/display/intel_modeset_lock.c | 50 +++++++++++++++++++
> .../gpu/drm/i915/display/intel_modeset_lock.h | 33 ++++++++++++
> 4 files changed, 87 insertions(+), 14 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/display/intel_modeset_lock.c
> create mode 100644 drivers/gpu/drm/i915/display/intel_modeset_lock.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 9af76e376ca91..ef6feb2aad2ad 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -263,6 +263,7 @@ i915-y += \
> display/intel_hti.o \
> display/intel_load_detect.o \
> display/intel_lpe_audio.o \
> + display/intel_modeset_lock.o \
> display/intel_modeset_verify.o \
> display/intel_modeset_setup.o \
> display/intel_overlay.o \
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 55f36d9d509c6..eb391fff0f1be 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -63,6 +63,7 @@
> #include "intel_hti.h"
> #include "intel_lspcon.h"
> #include "intel_mg_phy_regs.h"
> +#include "intel_modeset_lock.h"
> #include "intel_pps.h"
> #include "intel_psr.h"
> #include "intel_quirks.h"
> @@ -4400,26 +4401,14 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
>
> state = intel_encoder_hotplug(encoder, connector);
>
> - drm_modeset_acquire_init(&ctx, 0);
> -
> - for (;;) {
> + intel_modeset_lock_ctx_retry(&ctx, NULL, 0, ret) {
> if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
> ret = intel_hdmi_reset_link(encoder, &ctx);
> else
> ret = intel_dp_retrain_link(encoder, &ctx);
> -
> - if (ret == -EDEADLK) {
> - drm_modeset_backoff(&ctx);
> - continue;
> - }
> -
> - break;
> }
>
> - drm_modeset_drop_locks(&ctx);
> - drm_modeset_acquire_fini(&ctx);
> - drm_WARN(encoder->base.dev, ret,
> - "Acquiring modeset locks failed with %i\n", ret);
> + drm_WARN_ON(encoder->base.dev, ret);
>
> /*
> * Unpowered type-c dongles can take some time to boot and be
> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_lock.c
> b/drivers/gpu/drm/i915/display/intel_modeset_lock.c
> new file mode 100644
> index 0000000000000..8fb6fd849a75d
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_modeset_lock.c
> @@ -0,0 +1,50 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#include <drm/drm_modeset_lock.h>
> +
> +#include "intel_display_types.h"
> +#include "intel_modeset_lock.h"
> +
> +void _intel_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> + struct intel_atomic_state *state,
> + unsigned int flags, int *ret)
> +{
> + drm_modeset_acquire_init(ctx, flags);
> +
> + if (state)
> + state->base.acquire_ctx = ctx;
> +
> + *ret = -EDEADLK;
> +}
> +
> +bool _intel_modeset_lock_loop(int *ret)
> +{
> + if (*ret == -EDEADLK) {
> + *ret = 0;
> + return true;
> + }
> +
> + return false;
> +}
> +
> +void _intel_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> + struct intel_atomic_state *state,
> + int *ret)
> +{
> + if (*ret == -EDEADLK) {
> + if (state)
> + drm_atomic_state_clear(&state->base);
> +
> + *ret = drm_modeset_backoff(ctx);
> + if (*ret == 0) {
> + *ret = -EDEADLK;
> + return;
> + }
> + }
> +
> + drm_modeset_drop_locks(ctx);
> + drm_modeset_acquire_fini(ctx);
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_lock.h
> b/drivers/gpu/drm/i915/display/intel_modeset_lock.h
> new file mode 100644
> index 0000000000000..edb5099bcd99c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_modeset_lock.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef __INTEL_MODESET_LOCK_H__
> +#define __INTEL_MODESET_LOCK_H__
> +
> +#include <linux/types.h>
> +
> +struct drm_modeset_acquire_ctx;
> +struct intel_atomic_state;
> +
> +void _intel_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> + struct intel_atomic_state *state,
> + unsigned int flags,
> + int *ret);
> +bool _intel_modeset_lock_loop(int *ret);
> +void _intel_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> + struct intel_atomic_state *state,
> + int *ret);
> +
> +/*
> + * Note that one must always use "continue" rather than
> + * "break" or "return" to handle errors within the
> + * intel_modeset_lock_ctx_retry() block.
> + */
> +#define intel_modeset_lock_ctx_retry(ctx, state, flags, ret) \
> + for (_intel_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
> + _intel_modeset_lock_loop(&(ret)); \
> + _intel_modeset_lock_end((ctx), (state), &(ret)))
> +
> +#endif /* __INTEL_MODESET_LOCK_H__ */
> --
> 2.37.2
--
Ville Syrjälä
Intel
