nouveau.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible
@ 2021-09-16 21:15 Fernando Ramos
  2021-09-16 21:15 ` [Nouveau] [PATCH 01/15] dmr: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN() Fernando Ramos
                   ` (15 more replies)
  0 siblings, 16 replies; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

Hi all,

One of the things in the DRM TODO list ("Documentation/gpu/todo.rst") was to
"use DRM_MODESET_LOCAL_ALL_* helpers instead of boilerplate". That's what this
patch series is about.

You will find two types of changes here:

  - Replacing "drm_modeset_lock_all_ctx()" (and surrounding boilerplate) with
    "DRM_MODESET_LOCK_ALL_BEGIN()/END()" in the remaining places (as it has
    already been done in previous commits such as b7ea04d2)

  - Replacing "drm_modeset_lock_all()" with "DRM_MODESET_LOCK_ALL_BEGIN()/END()"
    in the remaining places (as it has already been done in previous commits
    such as 57037094)
    
Most of the changes are straight forward, except for a few cases in the "amd"
and "i915" drivers where some extra dancing was needed to overcome the
limitation that the DRM_MODESET_LOCK_ALL_BEGIN()/END() macros can only be used
once inside the same function (the reason being that the macro expansion
includes *labels*, and you can not have two labels named the same inside one
function)

Notice that, even after this patch series, some places remain where
"drm_modeset_lock_all()" and "drm_modeset_lock_all_ctx()" are still present,
all inside drm core (which makes sense), except for two (in "amd" and "i915")
which cannot be replaced due to the way they are being used.

Fernando Ramos (15):
  dmr: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
  dmr/i915: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
  dmr/msm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
  drm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  drm/vmwgfx: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  drm/tegra: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  drm/shmobile: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  drm/radeon: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  drm/omapdrm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  drm/nouveau: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  drm/msm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  drm/gma500: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  doc: drm: remove TODO entry regarding DRM_MODSET_LOCK_ALL cleanup

 Documentation/gpu/todo.rst                    | 17 -------
 Documentation/locking/ww-mutex-design.rst     |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 13 +++--
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 +++++++++----------
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 23 +++++----
 drivers/gpu/drm/drm_client_modeset.c          | 14 +++---
 drivers/gpu/drm/drm_crtc_helper.c             | 18 ++++---
 drivers/gpu/drm/drm_fb_helper.c               | 10 ++--
 drivers/gpu/drm/drm_framebuffer.c             |  6 ++-
 drivers/gpu/drm/gma500/psb_device.c           | 14 ++++--
 drivers/gpu/drm/i915/display/intel_audio.c    | 12 +++--
 drivers/gpu/drm/i915/display/intel_display.c  | 22 +++-----
 .../drm/i915/display/intel_display_debugfs.c  | 35 ++++++++-----
 drivers/gpu/drm/i915/display/intel_overlay.c  | 45 ++++++++---------
 drivers/gpu/drm/i915/display/intel_pipe_crc.c |  5 +-
 drivers/gpu/drm/i915/i915_drv.c               | 12 +++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      |  6 ++-
 .../gpu/drm/msm/disp/msm_disp_snapshot_util.c | 10 ++--
 drivers/gpu/drm/nouveau/dispnv50/disp.c       | 12 +++--
 drivers/gpu/drm/omapdrm/omap_fb.c             |  6 ++-
 drivers/gpu/drm/radeon/radeon_device.c        | 13 +++--
 drivers/gpu/drm/radeon/radeon_dp_mst.c        |  7 ++-
 drivers/gpu/drm/shmobile/shmob_drm_drv.c      |  6 ++-
 drivers/gpu/drm/tegra/dsi.c                   |  6 ++-
 drivers/gpu/drm/tegra/hdmi.c                  |  5 +-
 drivers/gpu/drm/tegra/sor.c                   | 10 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c         | 11 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           | 12 +++--
 28 files changed, 222 insertions(+), 180 deletions(-)

-- 
2.33.0


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 01/15] dmr: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:28   ` Sean Paul
  2021-09-16 21:15 ` [Nouveau] [PATCH 02/15] dmr/i915: " Fernando Ramos
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace the boilerplate code
surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN()
and DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/drm_client_modeset.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index ced09c7c06f9..5f5184f071ed 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -574,6 +574,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
 	int num_connectors_detected = 0;
 	int num_tiled_conns = 0;
 	struct drm_modeset_acquire_ctx ctx;
+	int err;
 
 	if (!drm_drv_uses_atomic_modeset(dev))
 		return false;
@@ -585,10 +586,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
 	if (!save_enabled)
 		return false;
 
-	drm_modeset_acquire_init(&ctx, 0);
-
-	while (drm_modeset_lock_all_ctx(dev, &ctx) != 0)
-		drm_modeset_backoff(&ctx);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, err);
 
 	memcpy(save_enabled, enabled, count);
 	mask = GENMASK(count - 1, 0);
@@ -743,8 +741,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
 		ret = false;
 	}
 
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, err);
 
 	kfree(save_enabled);
 	return ret;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 02/15] dmr/i915: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
  2021-09-16 21:15 ` [Nouveau] [PATCH 01/15] dmr: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN() Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:31   ` Sean Paul
  2021-09-16 21:15 ` [Nouveau] [PATCH 03/15] dmr/msm: " Fernando Ramos
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace the boilerplate code
surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN()
and DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/i915/display/intel_display.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 134a6acbd8fb..997a16e85c85 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -13476,22 +13476,13 @@ void intel_display_resume(struct drm_device *dev)
 	if (state)
 		state->acquire_ctx = &ctx;
 
-	drm_modeset_acquire_init(&ctx, 0);
-
-	while (1) {
-		ret = drm_modeset_lock_all_ctx(dev, &ctx);
-		if (ret != -EDEADLK)
-			break;
-
-		drm_modeset_backoff(&ctx);
-	}
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
-	if (!ret)
-		ret = __intel_display_resume(dev, state, &ctx);
+	ret = __intel_display_resume(dev, state, &ctx);
 
 	intel_enable_ipc(dev_priv);
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
+
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 	if (ret)
 		drm_err(&dev_priv->drm,
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 03/15] dmr/msm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
  2021-09-16 21:15 ` [Nouveau] [PATCH 01/15] dmr: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN() Fernando Ramos
  2021-09-16 21:15 ` [Nouveau] [PATCH 02/15] dmr/i915: " Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:29   ` Sean Paul
  2021-09-20  1:54   ` kernel test robot
  2021-09-16 21:15 ` [Nouveau] [PATCH 04/15] drm: cleanup: drm_modeset_lock_all() " Fernando Ramos
                   ` (12 subsequent siblings)
  15 siblings, 2 replies; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace the boilerplate code
surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN()
and DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
index cabe15190ec1..c83db90b0e02 100644
--- a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
+++ b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
@@ -99,20 +99,18 @@ static void msm_disp_capture_atomic_state(struct msm_disp_state *disp_state)
 {
 	struct drm_device *ddev;
 	struct drm_modeset_acquire_ctx ctx;
+	int ret;
 
 	disp_state->timestamp = ktime_get();
 
 	ddev = disp_state->drm_dev;
 
-	drm_modeset_acquire_init(&ctx, 0);
-
-	while (drm_modeset_lock_all_ctx(ddev, &ctx) != 0)
-		drm_modeset_backoff(&ctx);
+	DRM_MODESET_LOCK_ALL_BEGIN(ddev, ctx, 0, ret);
 
 	disp_state->atomic_state = drm_atomic_helper_duplicate_state(ddev,
 			&ctx);
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
+
+	DRM_MODESET_LOCK_ALL_END(ddev, ctx, ret);
 }
 
 void msm_disp_snapshot_capture_state(struct msm_disp_state *disp_state)
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 04/15] drm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
                   ` (2 preceding siblings ...)
  2021-09-16 21:15 ` [Nouveau] [PATCH 03/15] dmr/msm: " Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:35   ` Sean Paul
  2021-09-16 21:15 ` [Nouveau] [PATCH 05/15] drm/vmwgfx: " Fernando Ramos
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/drm_client_modeset.c |  5 +++--
 drivers/gpu/drm/drm_crtc_helper.c    | 18 ++++++++++++------
 drivers/gpu/drm/drm_fb_helper.c      | 10 ++++++----
 drivers/gpu/drm/drm_framebuffer.c    |  6 ++++--
 4 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index 5f5184f071ed..43f772543d2a 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -1062,9 +1062,10 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
 	struct drm_device *dev = client->dev;
 	struct drm_mode_set *mode_set;
 	struct drm_plane *plane;
+	struct drm_modeset_acquire_ctx ctx;
 	int ret = 0;
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	drm_for_each_plane(plane, dev) {
 		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
 			drm_plane_force_disable(plane);
@@ -1093,7 +1094,7 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
 			goto out;
 	}
 out:
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index bff917531f33..f3ce073dff79 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -218,11 +218,14 @@ static void __drm_helper_disable_unused_functions(struct drm_device *dev)
  */
 void drm_helper_disable_unused_functions(struct drm_device *dev)
 {
+	struct drm_modeset_acquire_ctx ctx;
+	int ret;
+
 	WARN_ON(drm_drv_uses_atomic_modeset(dev));
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	__drm_helper_disable_unused_functions(dev);
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 }
 EXPORT_SYMBOL(drm_helper_disable_unused_functions);
 
@@ -942,12 +945,14 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
 	struct drm_crtc *crtc;
 	struct drm_encoder *encoder;
 	const struct drm_crtc_helper_funcs *crtc_funcs;
+	struct drm_modeset_acquire_ctx ctx;
 	int encoder_dpms;
 	bool ret;
+	int err;
 
 	WARN_ON(drm_drv_uses_atomic_modeset(dev));
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, err);
 	drm_for_each_crtc(crtc, dev) {
 
 		if (!crtc->enabled)
@@ -982,7 +987,7 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
 
 	/* disable the unused connectors while restoring the modesetting */
 	__drm_helper_disable_unused_functions(dev);
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, err);
 }
 EXPORT_SYMBOL(drm_helper_resume_force_mode);
 
@@ -1002,9 +1007,10 @@ EXPORT_SYMBOL(drm_helper_resume_force_mode);
 int drm_helper_force_disable_all(struct drm_device *dev)
 {
 	struct drm_crtc *crtc;
+	struct drm_modeset_acquire_ctx ctx;
 	int ret = 0;
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	drm_for_each_crtc(crtc, dev)
 		if (crtc->enabled) {
 			struct drm_mode_set set = {
@@ -1016,7 +1022,7 @@ int drm_helper_force_disable_all(struct drm_device *dev)
 				goto out;
 		}
 out:
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 	return ret;
 }
 EXPORT_SYMBOL(drm_helper_force_disable_all);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 3ab078321045..6860223f0068 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -940,10 +940,11 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info)
 	struct drm_fb_helper *fb_helper = info->par;
 	struct drm_mode_set *modeset;
 	struct drm_crtc *crtc;
+	struct drm_modeset_acquire_ctx ctx;
 	u16 *r, *g, *b;
 	int ret = 0;
 
-	drm_modeset_lock_all(fb_helper->dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(fb_helper->dev, ctx, 0, ret);
 	drm_client_for_each_modeset(modeset, &fb_helper->client) {
 		crtc = modeset->crtc;
 		if (!crtc->funcs->gamma_set || !crtc->gamma_size) {
@@ -970,7 +971,7 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info)
 			goto out;
 	}
 out:
-	drm_modeset_unlock_all(fb_helper->dev);
+	DRM_MODESET_LOCK_ALL_END(fb_helper->dev, ctx, ret);
 
 	return ret;
 }
@@ -1441,10 +1442,11 @@ static int pan_display_legacy(struct fb_var_screeninfo *var,
 	struct drm_fb_helper *fb_helper = info->par;
 	struct drm_client_dev *client = &fb_helper->client;
 	struct drm_mode_set *modeset;
+	struct drm_modeset_acquire_ctx ctx;
 	int ret = 0;
 
 	mutex_lock(&client->modeset_mutex);
-	drm_modeset_lock_all(fb_helper->dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(fb_helper->dev, ctx, 0, ret);
 	drm_client_for_each_modeset(modeset, client) {
 		modeset->x = var->xoffset;
 		modeset->y = var->yoffset;
@@ -1457,7 +1459,7 @@ static int pan_display_legacy(struct fb_var_screeninfo *var,
 			}
 		}
 	}
-	drm_modeset_unlock_all(fb_helper->dev);
+	DRM_MODESET_LOCK_ALL_END(fb_helper->dev, ctx, ret);
 	mutex_unlock(&client->modeset_mutex);
 
 	return ret;
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 07f5abc875e9..205e9aa9a409 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -1059,8 +1059,10 @@ static void legacy_remove_fb(struct drm_framebuffer *fb)
 	struct drm_device *dev = fb->dev;
 	struct drm_crtc *crtc;
 	struct drm_plane *plane;
+	struct drm_modeset_acquire_ctx ctx;
+	int ret;
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	/* remove from any CRTC */
 	drm_for_each_crtc(crtc, dev) {
 		if (crtc->primary->fb == fb) {
@@ -1082,7 +1084,7 @@ static void legacy_remove_fb(struct drm_framebuffer *fb)
 			drm_plane_force_disable(plane);
 		}
 	}
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 }
 
 /**
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 05/15] drm/vmwgfx: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
                   ` (3 preceding siblings ...)
  2021-09-16 21:15 ` [Nouveau] [PATCH 04/15] drm: cleanup: drm_modeset_lock_all() " Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:37   ` Sean Paul
  2021-09-16 21:15 ` [Nouveau] [PATCH 06/15] drm/tegra: " Fernando Ramos
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c | 11 +++++++----
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   | 12 ++++++++----
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
index 28af34ab6ed6..7df35c6f1458 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
@@ -28,6 +28,7 @@
 #include "vmwgfx_drv.h"
 #include "vmwgfx_devcaps.h"
 #include <drm/vmwgfx_drm.h>
+#include <drm/drm_drv.h>
 #include "vmwgfx_kms.h"
 
 int vmw_getparam_ioctl(struct drm_device *dev, void *data,
@@ -172,6 +173,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
 	struct drm_vmw_rect __user *clips_ptr;
 	struct drm_vmw_rect *clips = NULL;
 	struct drm_framebuffer *fb;
+	struct drm_modeset_acquire_ctx ctx;
 	struct vmw_framebuffer *vfb;
 	struct vmw_resource *res;
 	uint32_t num_clips;
@@ -203,7 +205,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
 		goto out_no_copy;
 	}
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
 	fb = drm_framebuffer_lookup(dev, file_priv, arg->fb_id);
 	if (!fb) {
@@ -231,7 +233,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
 out_no_surface:
 	drm_framebuffer_put(fb);
 out_no_fb:
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 out_no_copy:
 	kfree(clips);
 out_clips:
@@ -250,6 +252,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void *data,
 	struct drm_vmw_rect __user *clips_ptr;
 	struct drm_vmw_rect *clips = NULL;
 	struct drm_framebuffer *fb;
+	struct drm_modeset_acquire_ctx ctx;
 	struct vmw_framebuffer *vfb;
 	uint32_t num_clips;
 	int ret;
@@ -280,7 +283,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void *data,
 		goto out_no_copy;
 	}
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
 	fb = drm_framebuffer_lookup(dev, file_priv, arg->fb_id);
 	if (!fb) {
@@ -303,7 +306,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void *data,
 out_no_ttm_lock:
 	drm_framebuffer_put(fb);
 out_no_fb:
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 out_no_copy:
 	kfree(clips);
 out_clips:
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 74fa41909213..268095cb8c84 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -33,6 +33,7 @@
 #include <drm/drm_rect.h>
 #include <drm/drm_sysfs.h>
 #include <drm/drm_vblank.h>
+#include <drm/drm_drv.h>
 
 #include "vmwgfx_kms.h"
 
@@ -243,15 +244,17 @@ void vmw_kms_legacy_hotspot_clear(struct vmw_private *dev_priv)
 	struct drm_device *dev = &dev_priv->drm;
 	struct vmw_display_unit *du;
 	struct drm_crtc *crtc;
+	struct drm_modeset_acquire_ctx ctx;
+	int ret;
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	drm_for_each_crtc(crtc, dev) {
 		du = vmw_crtc_to_du(crtc);
 
 		du->hotspot_x = 0;
 		du->hotspot_y = 0;
 	}
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 }
 
 void vmw_kms_cursor_post_execbuf(struct vmw_private *dev_priv)
@@ -1012,9 +1015,10 @@ static int vmw_framebuffer_bo_dirty(struct drm_framebuffer *framebuffer,
 	struct vmw_framebuffer_bo *vfbd =
 		vmw_framebuffer_to_vfbd(framebuffer);
 	struct drm_clip_rect norect;
+	struct drm_modeset_acquire_ctx ctx;
 	int ret, increment = 1;
 
-	drm_modeset_lock_all(&dev_priv->drm);
+	DRM_MODESET_LOCK_ALL_BEGIN((&dev_priv->drm), ctx, 0, ret);
 
 	if (!num_clips) {
 		num_clips = 1;
@@ -1040,7 +1044,7 @@ static int vmw_framebuffer_bo_dirty(struct drm_framebuffer *framebuffer,
 
 	vmw_cmd_flush(dev_priv, false);
 
-	drm_modeset_unlock_all(&dev_priv->drm);
+	DRM_MODESET_LOCK_ALL_END((&dev_priv->drm), ctx, ret);
 
 	return ret;
 }
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 06/15] drm/tegra: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
                   ` (4 preceding siblings ...)
  2021-09-16 21:15 ` [Nouveau] [PATCH 05/15] drm/vmwgfx: " Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:38   ` Sean Paul
  2021-09-16 21:15 ` [Nouveau] [PATCH 07/15] drm/shmobile: " Fernando Ramos
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/tegra/dsi.c  |  6 ++++--
 drivers/gpu/drm/tegra/hdmi.c |  5 +++--
 drivers/gpu/drm/tegra/sor.c  | 10 ++++++----
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index f46d377f0c30..77a496f6a2e9 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -202,10 +202,12 @@ static int tegra_dsi_show_regs(struct seq_file *s, void *data)
 	struct tegra_dsi *dsi = node->info_ent->data;
 	struct drm_crtc *crtc = dsi->output.encoder.crtc;
 	struct drm_device *drm = node->minor->dev;
+	struct drm_modeset_acquire_ctx ctx;
 	unsigned int i;
 	int err = 0;
+	int ret;
 
-	drm_modeset_lock_all(drm);
+	DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, ret);
 
 	if (!crtc || !crtc->state->active) {
 		err = -EBUSY;
@@ -220,7 +222,7 @@ static int tegra_dsi_show_regs(struct seq_file *s, void *data)
 	}
 
 unlock:
-	drm_modeset_unlock_all(drm);
+	DRM_MODESET_LOCK_ALL_END(drm, ctx, ret);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index e5d2a4026028..669a2ebb55ae 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -1031,10 +1031,11 @@ static int tegra_hdmi_show_regs(struct seq_file *s, void *data)
 	struct tegra_hdmi *hdmi = node->info_ent->data;
 	struct drm_crtc *crtc = hdmi->output.encoder.crtc;
 	struct drm_device *drm = node->minor->dev;
+	struct drm_modeset_acquire_ctx ctx;
 	unsigned int i;
 	int err = 0;
 
-	drm_modeset_lock_all(drm);
+	DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
 
 	if (!crtc || !crtc->state->active) {
 		err = -EBUSY;
@@ -1049,7 +1050,7 @@ static int tegra_hdmi_show_regs(struct seq_file *s, void *data)
 	}
 
 unlock:
-	drm_modeset_unlock_all(drm);
+	DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 0ea320c1092b..323d95eb0cac 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -1490,10 +1490,11 @@ static int tegra_sor_show_crc(struct seq_file *s, void *data)
 	struct tegra_sor *sor = node->info_ent->data;
 	struct drm_crtc *crtc = sor->output.encoder.crtc;
 	struct drm_device *drm = node->minor->dev;
+	struct drm_modeset_acquire_ctx ctx;
 	int err = 0;
 	u32 value;
 
-	drm_modeset_lock_all(drm);
+	DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
 
 	if (!crtc || !crtc->state->active) {
 		err = -EBUSY;
@@ -1522,7 +1523,7 @@ static int tegra_sor_show_crc(struct seq_file *s, void *data)
 	seq_printf(s, "%08x\n", value);
 
 unlock:
-	drm_modeset_unlock_all(drm);
+	DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
 	return err;
 }
 
@@ -1652,10 +1653,11 @@ static int tegra_sor_show_regs(struct seq_file *s, void *data)
 	struct tegra_sor *sor = node->info_ent->data;
 	struct drm_crtc *crtc = sor->output.encoder.crtc;
 	struct drm_device *drm = node->minor->dev;
+	struct drm_modeset_acquire_ctx ctx;
 	unsigned int i;
 	int err = 0;
 
-	drm_modeset_lock_all(drm);
+	DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
 
 	if (!crtc || !crtc->state->active) {
 		err = -EBUSY;
@@ -1670,7 +1672,7 @@ static int tegra_sor_show_regs(struct seq_file *s, void *data)
 	}
 
 unlock:
-	drm_modeset_unlock_all(drm);
+	DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
 	return err;
 }
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 07/15] drm/shmobile: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
                   ` (5 preceding siblings ...)
  2021-09-16 21:15 ` [Nouveau] [PATCH 06/15] drm/tegra: " Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:38   ` Sean Paul
  2021-09-16 21:15 ` [Nouveau] [PATCH 08/15] drm/radeon: " Fernando Ramos
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/shmobile/shmob_drm_drv.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
index 7db01904d18d..8ee215ab614e 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
@@ -156,10 +156,12 @@ static int shmob_drm_pm_suspend(struct device *dev)
 static int shmob_drm_pm_resume(struct device *dev)
 {
 	struct shmob_drm_device *sdev = dev_get_drvdata(dev);
+	struct drm_modeset_acquire_ctx ctx;
+	int ret;
 
-	drm_modeset_lock_all(sdev->ddev);
+	DRM_MODESET_LOCK_ALL_BEGIN(sdev->ddev, ctx, 0, ret);
 	shmob_drm_crtc_resume(&sdev->crtc);
-	drm_modeset_unlock_all(sdev->ddev);
+	DRM_MODESET_LOCK_ALL_END(sdev->ddev, ctx, ret);
 
 	drm_kms_helper_poll_enable(sdev->ddev);
 	return 0;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 08/15] drm/radeon: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
                   ` (6 preceding siblings ...)
  2021-09-16 21:15 ` [Nouveau] [PATCH 07/15] drm/shmobile: " Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:40   ` Sean Paul
  2021-09-16 21:15 ` [Nouveau] [PATCH 09/15] drm/omapdrm: " Fernando Ramos
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/radeon/radeon_device.c | 13 +++++++++----
 drivers/gpu/drm/radeon/radeon_dp_mst.c |  7 +++++--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 4f0fbf667431..3feb1fd44409 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -37,6 +37,7 @@
 #include <drm/drm_cache.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_device.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_file.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/radeon_drm.h>
@@ -1559,7 +1560,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
 	struct pci_dev *pdev;
 	struct drm_crtc *crtc;
 	struct drm_connector *connector;
+	struct drm_modeset_acquire_ctx ctx;
 	int i, r;
+	int ret;
 
 	if (dev == NULL || dev->dev_private == NULL) {
 		return -ENODEV;
@@ -1573,12 +1576,12 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
 
 	drm_kms_helper_poll_disable(dev);
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	/* turn off display hw */
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 		drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
 	}
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 	/* unpin the front buffers and cursors */
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
@@ -1663,7 +1666,9 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
 	struct radeon_device *rdev = dev->dev_private;
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	struct drm_crtc *crtc;
+	struct drm_modeset_acquire_ctx ctx;
 	int r;
+	int ret;
 
 	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
@@ -1741,11 +1746,11 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
 	if (fbcon) {
 		drm_helper_resume_force_mode(dev);
 		/* turn on display hw */
-		drm_modeset_lock_all(dev);
+		DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 		list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 			drm_helper_connector_dpms(connector, DRM_MODE_DPMS_ON);
 		}
-		drm_modeset_unlock_all(dev);
+		DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 	}
 
 	drm_kms_helper_poll_enable(dev);
diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
index ec867fa880a4..04fe7b0a6746 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
@@ -4,6 +4,7 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_file.h>
 #include <drm/drm_probe_helper.h>
+#include <drm/drm_drv.h>
 
 #include "atom.h"
 #include "ni_reg.h"
@@ -737,11 +738,13 @@ static int radeon_debugfs_mst_info_show(struct seq_file *m, void *unused)
 	struct radeon_device *rdev = (struct radeon_device *)m->private;
 	struct drm_device *dev = rdev->ddev;
 	struct drm_connector *connector;
+	struct drm_modeset_acquire_ctx ctx;
 	struct radeon_connector *radeon_connector;
 	struct radeon_connector_atom_dig *dig_connector;
 	int i;
+	int ret;
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 		if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort)
 			continue;
@@ -759,7 +762,7 @@ static int radeon_debugfs_mst_info_show(struct seq_file *m, void *unused)
 				   radeon_connector->cur_stream_attribs[i].fe,
 				   radeon_connector->cur_stream_attribs[i].slots);
 	}
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 	return 0;
 }
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 09/15] drm/omapdrm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
                   ` (7 preceding siblings ...)
  2021-09-16 21:15 ` [Nouveau] [PATCH 08/15] drm/radeon: " Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:40   ` Sean Paul
  2021-09-17 15:41   ` Sean Paul
  2021-09-16 21:15 ` [Nouveau] [PATCH 10/15] drm/nouveau: " Fernando Ramos
                   ` (6 subsequent siblings)
  15 siblings, 2 replies; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/omapdrm/omap_fb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index 190afc564914..56013c3ef701 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -62,13 +62,15 @@ static int omap_framebuffer_dirty(struct drm_framebuffer *fb,
 				  unsigned num_clips)
 {
 	struct drm_crtc *crtc;
+	struct drm_modeset_acquire_ctx ctx;
+	int ret;
 
-	drm_modeset_lock_all(fb->dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(fb->dev, ctx, 0, ret);
 
 	drm_for_each_crtc(crtc, fb->dev)
 		omap_crtc_flush(crtc);
 
-	drm_modeset_unlock_all(fb->dev);
+	DRM_MODESET_LOCK_ALL_END(fb->dev, ctx, ret);
 
 	return 0;
 }
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 10/15] drm/nouveau: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
                   ` (8 preceding siblings ...)
  2021-09-16 21:15 ` [Nouveau] [PATCH 09/15] drm/omapdrm: " Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:42   ` Sean Paul
  2021-09-16 21:15 ` [Nouveau] [PATCH 11/15] drm/msm: " Fernando Ramos
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index d7b9f7f8c9e3..eb613af4cdd5 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -667,15 +667,17 @@ nv50_audio_component_bind(struct device *kdev, struct device *hda_kdev,
 	struct drm_device *drm_dev = dev_get_drvdata(kdev);
 	struct nouveau_drm *drm = nouveau_drm(drm_dev);
 	struct drm_audio_component *acomp = data;
+	struct drm_modeset_acquire_ctx ctx;
+	int ret;
 
 	if (WARN_ON(!device_link_add(hda_kdev, kdev, DL_FLAG_STATELESS)))
 		return -ENOMEM;
 
-	drm_modeset_lock_all(drm_dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(drm_dev, ctx, 0, ret);
 	acomp->ops = &nv50_audio_component_ops;
 	acomp->dev = kdev;
 	drm->audio.component = acomp;
-	drm_modeset_unlock_all(drm_dev);
+	DRM_MODESET_LOCK_ALL_END(drm_dev, ctx, ret);
 	return 0;
 }
 
@@ -686,12 +688,14 @@ nv50_audio_component_unbind(struct device *kdev, struct device *hda_kdev,
 	struct drm_device *drm_dev = dev_get_drvdata(kdev);
 	struct nouveau_drm *drm = nouveau_drm(drm_dev);
 	struct drm_audio_component *acomp = data;
+	struct drm_modeset_acquire_ctx ctx;
+	int ret;
 
-	drm_modeset_lock_all(drm_dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(drm_dev, ctx, 0, ret);
 	drm->audio.component = NULL;
 	acomp->ops = NULL;
 	acomp->dev = NULL;
-	drm_modeset_unlock_all(drm_dev);
+	DRM_MODESET_LOCK_ALL_END(drm_dev, ctx, ret);
 }
 
 static const struct component_ops nv50_audio_component_bind_ops = {
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 11/15] drm/msm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
                   ` (9 preceding siblings ...)
  2021-09-16 21:15 ` [Nouveau] [PATCH 10/15] drm/nouveau: " Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:42   ` Sean Paul
  2021-09-16 21:15 ` [Nouveau] [PATCH 12/15] drm/i915: " Fernando Ramos
                   ` (4 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 768012243b44..4cbc79eaee17 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1172,14 +1172,16 @@ static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
 	struct drm_display_mode *mode;
 	struct drm_framebuffer *fb;
 	struct drm_plane_state *state;
+	struct drm_modeset_acquire_ctx ctx;
 	struct dpu_crtc_state *cstate;
 
 	int i, out_width;
+	int ret;
 
 	dpu_crtc = s->private;
 	crtc = &dpu_crtc->base;
 
-	drm_modeset_lock_all(crtc->dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(crtc->dev, ctx, 0, ret);
 	cstate = to_dpu_crtc_state(crtc->state);
 
 	mode = &crtc->state->adjusted_mode;
@@ -1263,7 +1265,7 @@ static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
 		dpu_crtc->vblank_cb_time = ktime_set(0, 0);
 	}
 
-	drm_modeset_unlock_all(crtc->dev);
+	DRM_MODESET_LOCK_ALL_END(crtc->dev, ctx, ret);
 
 	return 0;
 }
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 12/15] drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
                   ` (10 preceding siblings ...)
  2021-09-16 21:15 ` [Nouveau] [PATCH 11/15] drm/msm: " Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:48   ` Sean Paul
  2021-09-16 21:15 ` [Nouveau] [PATCH 13/15] drm/gma500: " Fernando Ramos
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/i915/display/intel_audio.c    | 12 +++--
 drivers/gpu/drm/i915/display/intel_display.c  |  5 ++-
 .../drm/i915/display/intel_display_debugfs.c  | 35 ++++++++++-----
 drivers/gpu/drm/i915/display/intel_overlay.c  | 45 +++++++++----------
 drivers/gpu/drm/i915/display/intel_pipe_crc.c |  5 ++-
 drivers/gpu/drm/i915/i915_drv.c               | 12 +++--
 6 files changed, 67 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index 532237588511..ab6a5a734b95 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -1214,7 +1214,9 @@ static int i915_audio_component_bind(struct device *i915_kdev,
 {
 	struct i915_audio_component *acomp = data;
 	struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
+	struct drm_modeset_acquire_ctx ctx;
 	int i;
+	int ret;
 
 	if (drm_WARN_ON(&dev_priv->drm, acomp->base.ops || acomp->base.dev))
 		return -EEXIST;
@@ -1224,14 +1226,14 @@ static int i915_audio_component_bind(struct device *i915_kdev,
 					 DL_FLAG_STATELESS)))
 		return -ENOMEM;
 
-	drm_modeset_lock_all(&dev_priv->drm);
+	DRM_MODESET_LOCK_ALL_BEGIN((&dev_priv->drm), ctx, 0, ret);
 	acomp->base.ops = &i915_audio_component_ops;
 	acomp->base.dev = i915_kdev;
 	BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS);
 	for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++)
 		acomp->aud_sample_rate[i] = 0;
 	dev_priv->audio_component = acomp;
-	drm_modeset_unlock_all(&dev_priv->drm);
+	DRM_MODESET_LOCK_ALL_END((&dev_priv->drm), ctx, ret);
 
 	return 0;
 }
@@ -1241,12 +1243,14 @@ static void i915_audio_component_unbind(struct device *i915_kdev,
 {
 	struct i915_audio_component *acomp = data;
 	struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
+	struct drm_modeset_acquire_ctx ctx;
+	int ret;
 
-	drm_modeset_lock_all(&dev_priv->drm);
+	DRM_MODESET_LOCK_ALL_BEGIN((&dev_priv->drm), ctx, 0, ret);
 	acomp->base.ops = NULL;
 	acomp->base.dev = NULL;
 	dev_priv->audio_component = NULL;
-	drm_modeset_unlock_all(&dev_priv->drm);
+	DRM_MODESET_LOCK_ALL_END((&dev_priv->drm), ctx, ret);
 
 	device_link_remove(hda_kdev, i915_kdev);
 
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 997a16e85c85..dc2e4d89e5aa 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -12511,6 +12511,7 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
 int intel_modeset_init_nogem(struct drm_i915_private *i915)
 {
 	struct drm_device *dev = &i915->drm;
+	struct drm_modeset_acquire_ctx ctx;
 	enum pipe pipe;
 	struct intel_crtc *crtc;
 	int ret;
@@ -12562,9 +12563,9 @@ int intel_modeset_init_nogem(struct drm_i915_private *i915)
 	intel_vga_disable(i915);
 	intel_setup_outputs(i915);
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	intel_modeset_setup_hw_state(dev, dev->mode_config.acquire_ctx);
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 	for_each_intel_crtc(dev, crtc) {
 		struct intel_initial_plane_config plane_config = {};
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 8fdacb252bb1..d73af228862e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1057,11 +1057,13 @@ static int i915_display_info(struct seq_file *m, void *unused)
 	struct intel_crtc *crtc;
 	struct drm_connector *connector;
 	struct drm_connector_list_iter conn_iter;
+	struct drm_modeset_acquire_ctx ctx;
 	intel_wakeref_t wakeref;
+	int ret;
 
 	wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
 	seq_printf(m, "CRTC info\n");
 	seq_printf(m, "---------\n");
@@ -1076,7 +1078,7 @@ static int i915_display_info(struct seq_file *m, void *unused)
 		intel_connector_info(m, connector);
 	drm_connector_list_iter_end(&conn_iter);
 
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
 
@@ -1087,9 +1089,11 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
 	struct drm_device *dev = &dev_priv->drm;
+	struct drm_modeset_acquire_ctx ctx;
 	int i;
+	int ret;
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
 	seq_printf(m, "PLL refclks: non-SSC: %d kHz, SSC: %d kHz\n",
 		   dev_priv->dpll.ref_clks.nssc,
@@ -1132,7 +1136,7 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
 		seq_printf(m, " mg_pll_tdc_coldst_bias: 0x%08x\n",
 			   pll->state.hw_state.mg_pll_tdc_coldst_bias);
 	}
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 	return 0;
 }
@@ -1193,13 +1197,15 @@ static int i915_ddb_info(struct seq_file *m, void *unused)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
 	struct drm_device *dev = &dev_priv->drm;
+	struct drm_modeset_acquire_ctx ctx;
 	struct skl_ddb_entry *entry;
 	struct intel_crtc *crtc;
+	int ret;
 
 	if (DISPLAY_VER(dev_priv) < 9)
 		return -ENODEV;
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
 	seq_printf(m, "%-15s%8s%8s%8s\n", "", "Start", "End", "Size");
 
@@ -1223,7 +1229,7 @@ static int i915_ddb_info(struct seq_file *m, void *unused)
 			   entry->end, skl_ddb_entry_size(entry));
 	}
 
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 	return 0;
 }
@@ -1303,10 +1309,12 @@ static int i915_drrs_status(struct seq_file *m, void *unused)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
 	struct drm_device *dev = &dev_priv->drm;
+	struct drm_modeset_acquire_ctx ctx;
 	struct intel_crtc *crtc;
 	int active_crtc_cnt = 0;
+	int ret;
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	for_each_intel_crtc(dev, crtc) {
 		if (crtc->base.state->active) {
 			active_crtc_cnt++;
@@ -1315,7 +1323,7 @@ static int i915_drrs_status(struct seq_file *m, void *unused)
 			drrs_status_per_crtc(m, dev, crtc);
 		}
 	}
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 	if (!active_crtc_cnt)
 		seq_puts(m, "No active crtc found\n");
@@ -1607,8 +1615,10 @@ static void wm_latency_show(struct seq_file *m, const u16 wm[8])
 {
 	struct drm_i915_private *dev_priv = m->private;
 	struct drm_device *dev = &dev_priv->drm;
+	struct drm_modeset_acquire_ctx ctx;
 	int level;
 	int num_levels;
+	int ret;
 
 	if (IS_CHERRYVIEW(dev_priv))
 		num_levels = 3;
@@ -1619,7 +1629,7 @@ static void wm_latency_show(struct seq_file *m, const u16 wm[8])
 	else
 		num_levels = ilk_wm_max_level(dev_priv) + 1;
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
 	for (level = 0; level < num_levels; level++) {
 		unsigned int latency = wm[level];
@@ -1640,7 +1650,7 @@ static void wm_latency_show(struct seq_file *m, const u16 wm[8])
 			   level, wm[level], latency / 10, latency % 10);
 	}
 
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 }
 
 static int pri_wm_latency_show(struct seq_file *m, void *data)
@@ -1724,6 +1734,7 @@ static ssize_t wm_latency_write(struct file *file, const char __user *ubuf,
 	struct seq_file *m = file->private_data;
 	struct drm_i915_private *dev_priv = m->private;
 	struct drm_device *dev = &dev_priv->drm;
+	struct drm_modeset_acquire_ctx ctx;
 	u16 new[8] = { 0 };
 	int num_levels;
 	int level;
@@ -1753,12 +1764,12 @@ static ssize_t wm_latency_write(struct file *file, const char __user *ubuf,
 	if (ret != num_levels)
 		return -EINVAL;
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
 	for (level = 0; level < num_levels; level++)
 		wm[level] = new[level];
 
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 	return len;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
index 7e3f5c6ca484..79c6940807a7 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -1104,6 +1104,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 	struct drm_crtc *drmmode_crtc;
 	struct intel_crtc *crtc;
 	struct drm_i915_gem_object *new_bo;
+	struct drm_modeset_acquire_ctx ctx;
 	int ret;
 
 	overlay = dev_priv->overlay;
@@ -1112,24 +1113,24 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 		return -ENODEV;
 	}
 
-	if (!(params->flags & I915_OVERLAY_ENABLE)) {
-		drm_modeset_lock_all(dev);
-		ret = intel_overlay_switch_off(overlay);
-		drm_modeset_unlock_all(dev);
+	if (params->flags & I915_OVERLAY_ENABLE) {
 
-		return ret;
-	}
+		drmmode_crtc = drm_crtc_find(dev, file_priv, params->crtc_id);
+		if (!drmmode_crtc)
+			return -ENOENT;
+		crtc = to_intel_crtc(drmmode_crtc);
 
-	drmmode_crtc = drm_crtc_find(dev, file_priv, params->crtc_id);
-	if (!drmmode_crtc)
-		return -ENOENT;
-	crtc = to_intel_crtc(drmmode_crtc);
+		new_bo = i915_gem_object_lookup(file_priv, params->bo_handle);
+		if (!new_bo)
+			return -ENOENT;
+	}
 
-	new_bo = i915_gem_object_lookup(file_priv, params->bo_handle);
-	if (!new_bo)
-		return -ENOENT;
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
-	drm_modeset_lock_all(dev);
+	if (!(params->flags & I915_OVERLAY_ENABLE)) {
+		ret = intel_overlay_switch_off(overlay);
+		goto out_unlock;
+	}
 
 	if (i915_gem_object_is_tiled(new_bo)) {
 		drm_dbg_kms(&dev_priv->drm,
@@ -1194,14 +1195,11 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 	if (ret != 0)
 		goto out_unlock;
 
-	drm_modeset_unlock_all(dev);
-	i915_gem_object_put(new_bo);
-
-	return 0;
-
 out_unlock:
-	drm_modeset_unlock_all(dev);
-	i915_gem_object_put(new_bo);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+	if (params->flags & I915_OVERLAY_ENABLE)
+		i915_gem_object_put(new_bo);
 
 	return ret;
 }
@@ -1263,6 +1261,7 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_intel_overlay_attrs *attrs = data;
 	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_modeset_acquire_ctx ctx;
 	struct intel_overlay *overlay;
 	int ret;
 
@@ -1272,7 +1271,7 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
 		return -ENODEV;
 	}
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
 	ret = -EINVAL;
 	if (!(attrs->flags & I915_OVERLAY_UPDATE_ATTRS)) {
@@ -1329,7 +1328,7 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
 
 	ret = 0;
 out_unlock:
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
index 8ac263f471be..e50e514e4897 100644
--- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
@@ -76,6 +76,7 @@ static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv,
 				     enum intel_pipe_crc_source *source)
 {
 	struct drm_device *dev = &dev_priv->drm;
+	struct drm_modeset_acquire_ctx ctx;
 	struct intel_encoder *encoder;
 	struct intel_crtc *crtc;
 	struct intel_digital_port *dig_port;
@@ -83,7 +84,7 @@ static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv,
 
 	*source = INTEL_PIPE_CRC_SOURCE_PIPE;
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	for_each_intel_encoder(dev, encoder) {
 		if (!encoder->base.crtc)
 			continue;
@@ -120,7 +121,7 @@ static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv,
 			break;
 		}
 	}
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 59fb4c710c8c..7a30e2ff2fed 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1009,31 +1009,35 @@ static void i915_driver_postclose(struct drm_device *dev, struct drm_file *file)
 static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = &dev_priv->drm;
+	struct drm_modeset_acquire_ctx ctx;
 	struct intel_encoder *encoder;
+	int ret;
 
 	if (!HAS_DISPLAY(dev_priv))
 		return;
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	for_each_intel_encoder(dev, encoder)
 		if (encoder->suspend)
 			encoder->suspend(encoder);
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 }
 
 static void intel_shutdown_encoders(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = &dev_priv->drm;
+	struct drm_modeset_acquire_ctx ctx;
 	struct intel_encoder *encoder;
+	int ret;
 
 	if (!HAS_DISPLAY(dev_priv))
 		return;
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	for_each_intel_encoder(dev, encoder)
 		if (encoder->shutdown)
 			encoder->shutdown(encoder);
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 }
 
 void i915_driver_shutdown(struct drm_i915_private *i915)
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 13/15] drm/gma500: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
                   ` (11 preceding siblings ...)
  2021-09-16 21:15 ` [Nouveau] [PATCH 12/15] drm/i915: " Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:49   ` Sean Paul
  2021-09-16 21:15 ` [Nouveau] [PATCH 14/15] drm/amd: " Fernando Ramos
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/gma500/psb_device.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/gma500/psb_device.c b/drivers/gpu/drm/gma500/psb_device.c
index 951725a0f7a3..4e27f65a1f16 100644
--- a/drivers/gpu/drm/gma500/psb_device.c
+++ b/drivers/gpu/drm/gma500/psb_device.c
@@ -8,6 +8,7 @@
 #include <linux/backlight.h>
 
 #include <drm/drm.h>
+#include <drm/drm_drv.h>
 
 #include "gma_device.h"
 #include "intel_bios.h"
@@ -169,8 +170,10 @@ static int psb_save_display_registers(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
+	struct drm_modeset_acquire_ctx ctx;
 	struct gma_connector *connector;
 	struct psb_state *regs = &dev_priv->regs.psb;
+	int ret;
 
 	/* Display arbitration control + watermarks */
 	regs->saveDSPARB = PSB_RVDC32(DSPARB);
@@ -183,7 +186,7 @@ static int psb_save_display_registers(struct drm_device *dev)
 	regs->saveCHICKENBIT = PSB_RVDC32(DSPCHICKENBIT);
 
 	/* Save crtc and output state */
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 		if (drm_helper_crtc_in_use(crtc))
 			dev_priv->ops->save_crtc(crtc);
@@ -193,7 +196,8 @@ static int psb_save_display_registers(struct drm_device *dev)
 		if (connector->save)
 			connector->save(&connector->base);
 
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
 	return 0;
 }
 
@@ -207,8 +211,10 @@ static int psb_restore_display_registers(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
+	struct drm_modeset_acquire_ctx ctx;
 	struct gma_connector *connector;
 	struct psb_state *regs = &dev_priv->regs.psb;
+	int ret;
 
 	/* Display arbitration + watermarks */
 	PSB_WVDC32(regs->saveDSPARB, DSPARB);
@@ -223,7 +229,7 @@ static int psb_restore_display_registers(struct drm_device *dev)
 	/*make sure VGA plane is off. it initializes to on after reset!*/
 	PSB_WVDC32(0x80000000, VGACNTRL);
 
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
 		if (drm_helper_crtc_in_use(crtc))
 			dev_priv->ops->restore_crtc(crtc);
@@ -232,7 +238,7 @@ static int psb_restore_display_registers(struct drm_device *dev)
 		if (connector->restore)
 			connector->restore(&connector->base);
 
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 	return 0;
 }
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 14/15] drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
                   ` (12 preceding siblings ...)
  2021-09-16 21:15 ` [Nouveau] [PATCH 13/15] drm/gma500: " Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:55   ` Sean Paul
  2021-09-16 21:15 ` [Nouveau] [PATCH 15/15] doc: drm: remove TODO entry regarding DRM_MODSET_LOCK_ALL cleanup Fernando Ramos
  2021-09-17 15:24 ` [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Daniel Vetter
  15 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 13 +++--
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 +++++++++----------
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 23 +++++----
 3 files changed, 46 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 7a7316731911..55ecc4aa859f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -40,6 +40,7 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_vblank.h>
+#include <drm/drm_drv.h>
 
 static void amdgpu_display_flip_callback(struct dma_fence *f,
 					 struct dma_fence_cb *cb)
@@ -1543,16 +1544,18 @@ int amdgpu_display_suspend_helper(struct amdgpu_device *adev)
 	struct drm_crtc *crtc;
 	struct drm_connector *connector;
 	struct drm_connector_list_iter iter;
+	struct drm_modeset_acquire_ctx ctx;
 	int r;
+	int ret;
 
 	/* turn off display hw */
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	drm_connector_list_iter_begin(dev, &iter);
 	drm_for_each_connector_iter(connector, &iter)
 		drm_helper_connector_dpms(connector,
 					  DRM_MODE_DPMS_OFF);
 	drm_connector_list_iter_end(&iter);
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 	/* unpin the front buffers and cursors */
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 		struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
@@ -1590,7 +1593,9 @@ int amdgpu_display_resume_helper(struct amdgpu_device *adev)
 	struct drm_connector *connector;
 	struct drm_connector_list_iter iter;
 	struct drm_crtc *crtc;
+	struct drm_modeset_acquire_ctx ctx;
 	int r;
+	int ret;
 
 	/* pin cursors */
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
@@ -1612,7 +1617,7 @@ int amdgpu_display_resume_helper(struct amdgpu_device *adev)
 	drm_helper_resume_force_mode(dev);
 
 	/* turn on display hw */
-	drm_modeset_lock_all(dev);
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
 	drm_connector_list_iter_begin(dev, &iter);
 	drm_for_each_connector_iter(connector, &iter)
@@ -1620,7 +1625,7 @@ int amdgpu_display_resume_helper(struct amdgpu_device *adev)
 					  DRM_MODE_DPMS_ON);
 	drm_connector_list_iter_end(&iter);
 
-	drm_modeset_unlock_all(dev);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 9b1fc54555ee..5196c1d26f87 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -80,6 +80,7 @@
 #include <drm/drm_edid.h>
 #include <drm/drm_vblank.h>
 #include <drm/drm_audio_component.h>
+#include <drm/drm_drv.h>
 
 #if defined(CONFIG_DRM_AMD_DC_DCN)
 #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h"
@@ -2621,6 +2622,9 @@ static void handle_hpd_irq(void *param)
 #ifdef CONFIG_DRM_AMD_DC_HDCP
 	struct dm_connector_state *dm_con_state = to_dm_connector_state(connector->state);
 #endif
+	struct drm_modeset_acquire_ctx ctx;
+	int ret;
+
 
 	if (adev->dm.disable_hpd_irq)
 		return;
@@ -2646,14 +2650,6 @@ static void handle_hpd_irq(void *param)
 	if (aconnector->base.force && new_connection_type == dc_connection_none) {
 		emulated_link_detect(aconnector->dc_link);
 
-
-		drm_modeset_lock_all(dev);
-		dm_restore_drm_connector_state(dev, connector);
-		drm_modeset_unlock_all(dev);
-
-		if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
-			drm_kms_helper_hotplug_event(dev);
-
 	} else if (dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD)) {
 		if (new_connection_type == dc_connection_none &&
 		    aconnector->dc_link->type == dc_connection_none)
@@ -2661,13 +2657,18 @@ static void handle_hpd_irq(void *param)
 
 		amdgpu_dm_update_connector_after_detect(aconnector);
 
-		drm_modeset_lock_all(dev);
-		dm_restore_drm_connector_state(dev, connector);
-		drm_modeset_unlock_all(dev);
-
-		if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
-			drm_kms_helper_hotplug_event(dev);
+	} else {
+		goto out;
 	}
+
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
+	dm_restore_drm_connector_state(dev, connector);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+	if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
+		drm_kms_helper_hotplug_event(dev);
+
+out:
 	mutex_unlock(&aconnector->hpd_lock);
 
 }
@@ -2756,12 +2757,14 @@ static void handle_hpd_rx_irq(void *param)
 	struct drm_connector *connector = &aconnector->base;
 	struct drm_device *dev = connector->dev;
 	struct dc_link *dc_link = aconnector->dc_link;
+	struct drm_modeset_acquire_ctx ctx;
 	bool is_mst_root_connector = aconnector->mst_mgr.mst_state;
 	bool result = false;
 	enum dc_connection_type new_connection_type = dc_connection_none;
 	struct amdgpu_device *adev = drm_to_adev(dev);
 	union hpd_irq_data hpd_irq_data;
 	bool lock_flag = 0;
+	int ret;
 
 	memset(&hpd_irq_data, 0, sizeof(hpd_irq_data));
 
@@ -2828,12 +2831,6 @@ static void handle_hpd_rx_irq(void *param)
 
 			amdgpu_dm_update_connector_after_detect(aconnector);
 
-
-			drm_modeset_lock_all(dev);
-			dm_restore_drm_connector_state(dev, connector);
-			drm_modeset_unlock_all(dev);
-
-			drm_kms_helper_hotplug_event(dev);
 		} else if (dc_link_detect(dc_link, DETECT_REASON_HPDRX)) {
 
 			if (aconnector->fake_enable)
@@ -2841,14 +2838,17 @@ static void handle_hpd_rx_irq(void *param)
 
 			amdgpu_dm_update_connector_after_detect(aconnector);
 
+		} else {
+			goto finish;
+		}
 
-			drm_modeset_lock_all(dev);
-			dm_restore_drm_connector_state(dev, connector);
-			drm_modeset_unlock_all(dev);
+		DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
+		dm_restore_drm_connector_state(dev, connector);
+		DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
-			drm_kms_helper_hotplug_event(dev);
-		}
+		drm_kms_helper_hotplug_event(dev);
 	}
+finish:
 #ifdef CONFIG_DRM_AMD_DC_HDCP
 	if (hpd_irq_data.bytes.device_service_irq.bits.CP_IRQ) {
 		if (adev->dm.hdcp_workqueue)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 87daa78a32b8..6e5a8f4fc6bd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -37,6 +37,8 @@
 #include "link_hwss.h"
 #include "dc/dc_dmub_srv.h"
 
+#include <drm/drm_drv.h>
+
 struct dmub_debugfs_trace_header {
 	uint32_t entry_count;
 	uint32_t reserved[3];
@@ -1191,12 +1193,14 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
 	struct drm_connector *connector = &aconnector->base;
 	struct dc_link *link = NULL;
 	struct drm_device *dev = connector->dev;
+	struct drm_modeset_acquire_ctx ctx;
 	enum dc_connection_type new_connection_type = dc_connection_none;
 	char *wr_buf = NULL;
 	uint32_t wr_buf_size = 42;
 	int max_param_num = 1;
 	long param[1] = {0};
 	uint8_t param_nums = 0;
+	int ret;
 
 	if (!aconnector || !aconnector->dc_link)
 		return -EINVAL;
@@ -1236,12 +1240,6 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
 			goto unlock;
 
 		amdgpu_dm_update_connector_after_detect(aconnector);
-
-		drm_modeset_lock_all(dev);
-		dm_restore_drm_connector_state(dev, connector);
-		drm_modeset_unlock_all(dev);
-
-		drm_kms_helper_hotplug_event(dev);
 	} else if (param[0] == 0) {
 		if (!aconnector->dc_link)
 			goto unlock;
@@ -1259,13 +1257,16 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
 
 		amdgpu_dm_update_connector_after_detect(aconnector);
 
-		drm_modeset_lock_all(dev);
-		dm_restore_drm_connector_state(dev, connector);
-		drm_modeset_unlock_all(dev);
-
-		drm_kms_helper_hotplug_event(dev);
+	} else {
+		goto unlock;
 	}
 
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
+	dm_restore_drm_connector_state(dev, connector);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+	drm_kms_helper_hotplug_event(dev);
+
 unlock:
 	mutex_unlock(&aconnector->hpd_lock);
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Nouveau] [PATCH 15/15] doc: drm: remove TODO entry regarding DRM_MODSET_LOCK_ALL cleanup
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
                   ` (13 preceding siblings ...)
  2021-09-16 21:15 ` [Nouveau] [PATCH 14/15] drm/amd: " Fernando Ramos
@ 2021-09-16 21:15 ` Fernando Ramos
  2021-09-17 15:56   ` Sean Paul
  2021-09-17 15:24 ` [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Daniel Vetter
  15 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-16 21:15 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, sean, linux-doc, amd-gfx, intel-gfx, linux-arm-msm,
	freedreno, nouveau, linux-renesas-soc, linux-tegra

The previous commits do exactly what this entry in the TODO file asks
for, thus we can remove it now as it is no longer applicable.

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 Documentation/gpu/todo.rst                | 17 -----------------
 Documentation/locking/ww-mutex-design.rst |  2 +-
 2 files changed, 1 insertion(+), 18 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 12e61869939e..6613543955e9 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -353,23 +353,6 @@ converted, except for struct drm_driver.gem_prime_mmap.
 
 Level: Intermediate
 
-Use DRM_MODESET_LOCK_ALL_* helpers instead of boilerplate
----------------------------------------------------------
-
-For cases where drivers are attempting to grab the modeset locks with a local
-acquire context. Replace the boilerplate code surrounding
-drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() and
-DRM_MODESET_LOCK_ALL_END() instead.
-
-This should also be done for all places where drm_modeset_lock_all() is still
-used.
-
-As a reference, take a look at the conversions already completed in drm core.
-
-Contact: Sean Paul, respective driver maintainers
-
-Level: Starter
-
 Rename CMA helpers to DMA helpers
 ---------------------------------
 
diff --git a/Documentation/locking/ww-mutex-design.rst b/Documentation/locking/ww-mutex-design.rst
index 6a4d7319f8f0..6a8f8beb9ec4 100644
--- a/Documentation/locking/ww-mutex-design.rst
+++ b/Documentation/locking/ww-mutex-design.rst
@@ -60,7 +60,7 @@ Concepts
 Compared to normal mutexes two additional concepts/objects show up in the lock
 interface for w/w mutexes:
 
-Acquire context: To ensure eventual forward progress it is important the a task
+Acquire context: To ensure eventual forward progress it is important that a task
 trying to acquire locks doesn't grab a new reservation id, but keeps the one it
 acquired when starting the lock acquisition. This ticket is stored in the
 acquire context. Furthermore the acquire context keeps track of debugging state
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible
  2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
                   ` (14 preceding siblings ...)
  2021-09-16 21:15 ` [Nouveau] [PATCH 15/15] doc: drm: remove TODO entry regarding DRM_MODSET_LOCK_ALL cleanup Fernando Ramos
@ 2021-09-17 15:24 ` Daniel Vetter
  2021-09-17 21:41   ` Fernando Ramos
  15 siblings, 1 reply; 45+ messages in thread
From: Daniel Vetter @ 2021-09-17 15:24 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:37PM +0200, Fernando Ramos wrote:
> Hi all,
> 
> One of the things in the DRM TODO list ("Documentation/gpu/todo.rst") was to
> "use DRM_MODESET_LOCAL_ALL_* helpers instead of boilerplate". That's what this
> patch series is about.
> 
> You will find two types of changes here:
> 
>   - Replacing "drm_modeset_lock_all_ctx()" (and surrounding boilerplate) with
>     "DRM_MODESET_LOCK_ALL_BEGIN()/END()" in the remaining places (as it has
>     already been done in previous commits such as b7ea04d2)
> 
>   - Replacing "drm_modeset_lock_all()" with "DRM_MODESET_LOCK_ALL_BEGIN()/END()"
>     in the remaining places (as it has already been done in previous commits
>     such as 57037094)
>     
> Most of the changes are straight forward, except for a few cases in the "amd"
> and "i915" drivers where some extra dancing was needed to overcome the
> limitation that the DRM_MODESET_LOCK_ALL_BEGIN()/END() macros can only be used
> once inside the same function (the reason being that the macro expansion
> includes *labels*, and you can not have two labels named the same inside one
> function)
> 
> Notice that, even after this patch series, some places remain where
> "drm_modeset_lock_all()" and "drm_modeset_lock_all_ctx()" are still present,
> all inside drm core (which makes sense), except for two (in "amd" and "i915")
> which cannot be replaced due to the way they are being used.

Can we at least replace those with drm_modeset_lock_all_ctx and delete
drm_modeset_lock_all? That would be really nice goal to make sure these
don't spread further.

Otherwise great stuff, I'm trying to volunteer a few reviewers.
-Daniel

> 
> Fernando Ramos (15):
>   dmr: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   dmr/i915: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   dmr/msm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   drm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   drm/vmwgfx: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   drm/tegra: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   drm/shmobile: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   drm/radeon: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   drm/omapdrm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   drm/nouveau: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   drm/msm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   drm/gma500: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
>   doc: drm: remove TODO entry regarding DRM_MODSET_LOCK_ALL cleanup
> 
>  Documentation/gpu/todo.rst                    | 17 -------
>  Documentation/locking/ww-mutex-design.rst     |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 13 +++--
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 +++++++++----------
>  .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 23 +++++----
>  drivers/gpu/drm/drm_client_modeset.c          | 14 +++---
>  drivers/gpu/drm/drm_crtc_helper.c             | 18 ++++---
>  drivers/gpu/drm/drm_fb_helper.c               | 10 ++--
>  drivers/gpu/drm/drm_framebuffer.c             |  6 ++-
>  drivers/gpu/drm/gma500/psb_device.c           | 14 ++++--
>  drivers/gpu/drm/i915/display/intel_audio.c    | 12 +++--
>  drivers/gpu/drm/i915/display/intel_display.c  | 22 +++-----
>  .../drm/i915/display/intel_display_debugfs.c  | 35 ++++++++-----
>  drivers/gpu/drm/i915/display/intel_overlay.c  | 45 ++++++++---------
>  drivers/gpu/drm/i915/display/intel_pipe_crc.c |  5 +-
>  drivers/gpu/drm/i915/i915_drv.c               | 12 +++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      |  6 ++-
>  .../gpu/drm/msm/disp/msm_disp_snapshot_util.c | 10 ++--
>  drivers/gpu/drm/nouveau/dispnv50/disp.c       | 12 +++--
>  drivers/gpu/drm/omapdrm/omap_fb.c             |  6 ++-
>  drivers/gpu/drm/radeon/radeon_device.c        | 13 +++--
>  drivers/gpu/drm/radeon/radeon_dp_mst.c        |  7 ++-
>  drivers/gpu/drm/shmobile/shmob_drm_drv.c      |  6 ++-
>  drivers/gpu/drm/tegra/dsi.c                   |  6 ++-
>  drivers/gpu/drm/tegra/hdmi.c                  |  5 +-
>  drivers/gpu/drm/tegra/sor.c                   | 10 ++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c         | 11 ++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           | 12 +++--
>  28 files changed, 222 insertions(+), 180 deletions(-)
> 
> -- 
> 2.33.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 01/15] dmr: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 01/15] dmr: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN() Fernando Ramos
@ 2021-09-17 15:28   ` Sean Paul
  2021-09-17 22:07     ` Fernando Ramos
  0 siblings, 1 reply; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:28 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:38PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace the boilerplate code
> surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN()
> and DRM_MODESET_LOCK_ALL_END()
> 

Hi Fernando,
Thank you for your patch. Could you please fix the subject, changing dmr to drm?

> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  drivers/gpu/drm/drm_client_modeset.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index ced09c7c06f9..5f5184f071ed 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -574,6 +574,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
>  	int num_connectors_detected = 0;
>  	int num_tiled_conns = 0;
>  	struct drm_modeset_acquire_ctx ctx;
> +	int err;

I think you can just reuse 'ret' instead of creating a new variable. That
ensures if the lock fails we return the error from the macros.

Sean

>  
>  	if (!drm_drv_uses_atomic_modeset(dev))
>  		return false;
> @@ -585,10 +586,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
>  	if (!save_enabled)
>  		return false;
>  
> -	drm_modeset_acquire_init(&ctx, 0);
> -
> -	while (drm_modeset_lock_all_ctx(dev, &ctx) != 0)
> -		drm_modeset_backoff(&ctx);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, err);
>  
>  	memcpy(save_enabled, enabled, count);
>  	mask = GENMASK(count - 1, 0);
> @@ -743,8 +741,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
>  		ret = false;
>  	}
>  
> -	drm_modeset_drop_locks(&ctx);
> -	drm_modeset_acquire_fini(&ctx);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, err);
>  
>  	kfree(save_enabled);
>  	return ret;
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 03/15] dmr/msm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 03/15] dmr/msm: " Fernando Ramos
@ 2021-09-17 15:29   ` Sean Paul
  2021-09-20  1:54   ` kernel test robot
  1 sibling, 0 replies; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:29 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:40PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace the boilerplate code
> surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN()
> and DRM_MODESET_LOCK_ALL_END()
> 

With the subject fixed (s/dmr/drm/),

Reviewed-by: Sean Paul <sean@poorly.run>

> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
> index cabe15190ec1..c83db90b0e02 100644
> --- a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
> +++ b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
> @@ -99,20 +99,18 @@ static void msm_disp_capture_atomic_state(struct msm_disp_state *disp_state)
>  {
>  	struct drm_device *ddev;
>  	struct drm_modeset_acquire_ctx ctx;
> +	int ret;
>  
>  	disp_state->timestamp = ktime_get();
>  
>  	ddev = disp_state->drm_dev;
>  
> -	drm_modeset_acquire_init(&ctx, 0);
> -
> -	while (drm_modeset_lock_all_ctx(ddev, &ctx) != 0)
> -		drm_modeset_backoff(&ctx);
> +	DRM_MODESET_LOCK_ALL_BEGIN(ddev, ctx, 0, ret);
>  
>  	disp_state->atomic_state = drm_atomic_helper_duplicate_state(ddev,
>  			&ctx);
> -	drm_modeset_drop_locks(&ctx);
> -	drm_modeset_acquire_fini(&ctx);
> +
> +	DRM_MODESET_LOCK_ALL_END(ddev, ctx, ret);
>  }
>  
>  void msm_disp_snapshot_capture_state(struct msm_disp_state *disp_state)
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 02/15] dmr/i915: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 02/15] dmr/i915: " Fernando Ramos
@ 2021-09-17 15:31   ` Sean Paul
  0 siblings, 0 replies; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:31 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:39PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace the boilerplate code
> surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN()
> and DRM_MODESET_LOCK_ALL_END()
> 

With the subject fixed (s/dmr/drm),

Reviewed-by: Sean Paul <sean@poorly.run>

> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 134a6acbd8fb..997a16e85c85 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13476,22 +13476,13 @@ void intel_display_resume(struct drm_device *dev)
>  	if (state)
>  		state->acquire_ctx = &ctx;
>  
> -	drm_modeset_acquire_init(&ctx, 0);
> -
> -	while (1) {
> -		ret = drm_modeset_lock_all_ctx(dev, &ctx);
> -		if (ret != -EDEADLK)
> -			break;
> -
> -		drm_modeset_backoff(&ctx);
> -	}
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  
> -	if (!ret)
> -		ret = __intel_display_resume(dev, state, &ctx);
> +	ret = __intel_display_resume(dev, state, &ctx);
>  
>  	intel_enable_ipc(dev_priv);
> -	drm_modeset_drop_locks(&ctx);
> -	drm_modeset_acquire_fini(&ctx);
> +
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  
>  	if (ret)
>  		drm_err(&dev_priv->drm,
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 04/15] drm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 04/15] drm: cleanup: drm_modeset_lock_all() " Fernando Ramos
@ 2021-09-17 15:35   ` Sean Paul
  0 siblings, 0 replies; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:35 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:41PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace driver calls to
> drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
> DRM_MODESET_LOCK_ALL_END()
> 
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>

Reviewed-by: Sean Paul <sean@poorly.run>

> ---
>  drivers/gpu/drm/drm_client_modeset.c |  5 +++--
>  drivers/gpu/drm/drm_crtc_helper.c    | 18 ++++++++++++------
>  drivers/gpu/drm/drm_fb_helper.c      | 10 ++++++----
>  drivers/gpu/drm/drm_framebuffer.c    |  6 ++++--
>  4 files changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 5f5184f071ed..43f772543d2a 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -1062,9 +1062,10 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
>  	struct drm_device *dev = client->dev;
>  	struct drm_mode_set *mode_set;
>  	struct drm_plane *plane;
> +	struct drm_modeset_acquire_ctx ctx;
>  	int ret = 0;
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	drm_for_each_plane(plane, dev) {
>  		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
>  			drm_plane_force_disable(plane);
> @@ -1093,7 +1094,7 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
>  			goto out;
>  	}
>  out:
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index bff917531f33..f3ce073dff79 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -218,11 +218,14 @@ static void __drm_helper_disable_unused_functions(struct drm_device *dev)
>   */
>  void drm_helper_disable_unused_functions(struct drm_device *dev)
>  {
> +	struct drm_modeset_acquire_ctx ctx;
> +	int ret;
> +
>  	WARN_ON(drm_drv_uses_atomic_modeset(dev));
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	__drm_helper_disable_unused_functions(dev);
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  }
>  EXPORT_SYMBOL(drm_helper_disable_unused_functions);
>  
> @@ -942,12 +945,14 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
>  	struct drm_crtc *crtc;
>  	struct drm_encoder *encoder;
>  	const struct drm_crtc_helper_funcs *crtc_funcs;
> +	struct drm_modeset_acquire_ctx ctx;
>  	int encoder_dpms;
>  	bool ret;
> +	int err;
>  
>  	WARN_ON(drm_drv_uses_atomic_modeset(dev));
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, err);
>  	drm_for_each_crtc(crtc, dev) {
>  
>  		if (!crtc->enabled)
> @@ -982,7 +987,7 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
>  
>  	/* disable the unused connectors while restoring the modesetting */
>  	__drm_helper_disable_unused_functions(dev);
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, err);
>  }
>  EXPORT_SYMBOL(drm_helper_resume_force_mode);
>  
> @@ -1002,9 +1007,10 @@ EXPORT_SYMBOL(drm_helper_resume_force_mode);
>  int drm_helper_force_disable_all(struct drm_device *dev)
>  {
>  	struct drm_crtc *crtc;
> +	struct drm_modeset_acquire_ctx ctx;
>  	int ret = 0;
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	drm_for_each_crtc(crtc, dev)
>  		if (crtc->enabled) {
>  			struct drm_mode_set set = {
> @@ -1016,7 +1022,7 @@ int drm_helper_force_disable_all(struct drm_device *dev)
>  				goto out;
>  		}
>  out:
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  	return ret;
>  }
>  EXPORT_SYMBOL(drm_helper_force_disable_all);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 3ab078321045..6860223f0068 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -940,10 +940,11 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info)
>  	struct drm_fb_helper *fb_helper = info->par;
>  	struct drm_mode_set *modeset;
>  	struct drm_crtc *crtc;
> +	struct drm_modeset_acquire_ctx ctx;
>  	u16 *r, *g, *b;
>  	int ret = 0;
>  
> -	drm_modeset_lock_all(fb_helper->dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(fb_helper->dev, ctx, 0, ret);
>  	drm_client_for_each_modeset(modeset, &fb_helper->client) {
>  		crtc = modeset->crtc;
>  		if (!crtc->funcs->gamma_set || !crtc->gamma_size) {
> @@ -970,7 +971,7 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info)
>  			goto out;
>  	}
>  out:
> -	drm_modeset_unlock_all(fb_helper->dev);
> +	DRM_MODESET_LOCK_ALL_END(fb_helper->dev, ctx, ret);
>  
>  	return ret;
>  }
> @@ -1441,10 +1442,11 @@ static int pan_display_legacy(struct fb_var_screeninfo *var,
>  	struct drm_fb_helper *fb_helper = info->par;
>  	struct drm_client_dev *client = &fb_helper->client;
>  	struct drm_mode_set *modeset;
> +	struct drm_modeset_acquire_ctx ctx;
>  	int ret = 0;
>  
>  	mutex_lock(&client->modeset_mutex);
> -	drm_modeset_lock_all(fb_helper->dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(fb_helper->dev, ctx, 0, ret);
>  	drm_client_for_each_modeset(modeset, client) {
>  		modeset->x = var->xoffset;
>  		modeset->y = var->yoffset;
> @@ -1457,7 +1459,7 @@ static int pan_display_legacy(struct fb_var_screeninfo *var,
>  			}
>  		}
>  	}
> -	drm_modeset_unlock_all(fb_helper->dev);
> +	DRM_MODESET_LOCK_ALL_END(fb_helper->dev, ctx, ret);
>  	mutex_unlock(&client->modeset_mutex);
>  
>  	return ret;
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 07f5abc875e9..205e9aa9a409 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -1059,8 +1059,10 @@ static void legacy_remove_fb(struct drm_framebuffer *fb)
>  	struct drm_device *dev = fb->dev;
>  	struct drm_crtc *crtc;
>  	struct drm_plane *plane;
> +	struct drm_modeset_acquire_ctx ctx;
> +	int ret;
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	/* remove from any CRTC */
>  	drm_for_each_crtc(crtc, dev) {
>  		if (crtc->primary->fb == fb) {
> @@ -1082,7 +1084,7 @@ static void legacy_remove_fb(struct drm_framebuffer *fb)
>  			drm_plane_force_disable(plane);
>  		}
>  	}
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  }
>  
>  /**
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 05/15] drm/vmwgfx: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 05/15] drm/vmwgfx: " Fernando Ramos
@ 2021-09-17 15:37   ` Sean Paul
  0 siblings, 0 replies; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:37 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:42PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace driver calls to
> drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
> DRM_MODESET_LOCK_ALL_END()
> 

Reviewed-by: Sean Paul <sean@poorly.run>

> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c | 11 +++++++----
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   | 12 ++++++++----
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
> index 28af34ab6ed6..7df35c6f1458 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
> @@ -28,6 +28,7 @@
>  #include "vmwgfx_drv.h"
>  #include "vmwgfx_devcaps.h"
>  #include <drm/vmwgfx_drm.h>
> +#include <drm/drm_drv.h>
>  #include "vmwgfx_kms.h"
>  
>  int vmw_getparam_ioctl(struct drm_device *dev, void *data,
> @@ -172,6 +173,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
>  	struct drm_vmw_rect __user *clips_ptr;
>  	struct drm_vmw_rect *clips = NULL;
>  	struct drm_framebuffer *fb;
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct vmw_framebuffer *vfb;
>  	struct vmw_resource *res;
>  	uint32_t num_clips;
> @@ -203,7 +205,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
>  		goto out_no_copy;
>  	}
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  
>  	fb = drm_framebuffer_lookup(dev, file_priv, arg->fb_id);
>  	if (!fb) {
> @@ -231,7 +233,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
>  out_no_surface:
>  	drm_framebuffer_put(fb);
>  out_no_fb:
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  out_no_copy:
>  	kfree(clips);
>  out_clips:
> @@ -250,6 +252,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void *data,
>  	struct drm_vmw_rect __user *clips_ptr;
>  	struct drm_vmw_rect *clips = NULL;
>  	struct drm_framebuffer *fb;
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct vmw_framebuffer *vfb;
>  	uint32_t num_clips;
>  	int ret;
> @@ -280,7 +283,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void *data,
>  		goto out_no_copy;
>  	}
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  
>  	fb = drm_framebuffer_lookup(dev, file_priv, arg->fb_id);
>  	if (!fb) {
> @@ -303,7 +306,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void *data,
>  out_no_ttm_lock:
>  	drm_framebuffer_put(fb);
>  out_no_fb:
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  out_no_copy:
>  	kfree(clips);
>  out_clips:
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 74fa41909213..268095cb8c84 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -33,6 +33,7 @@
>  #include <drm/drm_rect.h>
>  #include <drm/drm_sysfs.h>
>  #include <drm/drm_vblank.h>
> +#include <drm/drm_drv.h>
>  
>  #include "vmwgfx_kms.h"
>  
> @@ -243,15 +244,17 @@ void vmw_kms_legacy_hotspot_clear(struct vmw_private *dev_priv)
>  	struct drm_device *dev = &dev_priv->drm;
>  	struct vmw_display_unit *du;
>  	struct drm_crtc *crtc;
> +	struct drm_modeset_acquire_ctx ctx;
> +	int ret;
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	drm_for_each_crtc(crtc, dev) {
>  		du = vmw_crtc_to_du(crtc);
>  
>  		du->hotspot_x = 0;
>  		du->hotspot_y = 0;
>  	}
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  }
>  
>  void vmw_kms_cursor_post_execbuf(struct vmw_private *dev_priv)
> @@ -1012,9 +1015,10 @@ static int vmw_framebuffer_bo_dirty(struct drm_framebuffer *framebuffer,
>  	struct vmw_framebuffer_bo *vfbd =
>  		vmw_framebuffer_to_vfbd(framebuffer);
>  	struct drm_clip_rect norect;
> +	struct drm_modeset_acquire_ctx ctx;
>  	int ret, increment = 1;
>  
> -	drm_modeset_lock_all(&dev_priv->drm);
> +	DRM_MODESET_LOCK_ALL_BEGIN((&dev_priv->drm), ctx, 0, ret);
>  
>  	if (!num_clips) {
>  		num_clips = 1;
> @@ -1040,7 +1044,7 @@ static int vmw_framebuffer_bo_dirty(struct drm_framebuffer *framebuffer,
>  
>  	vmw_cmd_flush(dev_priv, false);
>  
> -	drm_modeset_unlock_all(&dev_priv->drm);
> +	DRM_MODESET_LOCK_ALL_END((&dev_priv->drm), ctx, ret);
>  
>  	return ret;
>  }
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 06/15] drm/tegra: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 06/15] drm/tegra: " Fernando Ramos
@ 2021-09-17 15:38   ` Sean Paul
  2021-09-17 22:34     ` Fernando Ramos
  0 siblings, 1 reply; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:38 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:43PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace driver calls to
> drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
> DRM_MODESET_LOCK_ALL_END()
> 
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  drivers/gpu/drm/tegra/dsi.c  |  6 ++++--
>  drivers/gpu/drm/tegra/hdmi.c |  5 +++--
>  drivers/gpu/drm/tegra/sor.c  | 10 ++++++----
>  3 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
> index f46d377f0c30..77a496f6a2e9 100644
> --- a/drivers/gpu/drm/tegra/dsi.c
> +++ b/drivers/gpu/drm/tegra/dsi.c
> @@ -202,10 +202,12 @@ static int tegra_dsi_show_regs(struct seq_file *s, void *data)
>  	struct tegra_dsi *dsi = node->info_ent->data;
>  	struct drm_crtc *crtc = dsi->output.encoder.crtc;
>  	struct drm_device *drm = node->minor->dev;
> +	struct drm_modeset_acquire_ctx ctx;
>  	unsigned int i;
>  	int err = 0;
> +	int ret;

You can use err here instead. With that fixed,

Reviewed-by: Sean Paul <sean@poorly.run>


>  
> -	drm_modeset_lock_all(drm);
> +	DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, ret);
>  
>  	if (!crtc || !crtc->state->active) {
>  		err = -EBUSY;
> @@ -220,7 +222,7 @@ static int tegra_dsi_show_regs(struct seq_file *s, void *data)
>  	}
>  
>  unlock:
> -	drm_modeset_unlock_all(drm);
> +	DRM_MODESET_LOCK_ALL_END(drm, ctx, ret);
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
> index e5d2a4026028..669a2ebb55ae 100644
> --- a/drivers/gpu/drm/tegra/hdmi.c
> +++ b/drivers/gpu/drm/tegra/hdmi.c
> @@ -1031,10 +1031,11 @@ static int tegra_hdmi_show_regs(struct seq_file *s, void *data)
>  	struct tegra_hdmi *hdmi = node->info_ent->data;
>  	struct drm_crtc *crtc = hdmi->output.encoder.crtc;
>  	struct drm_device *drm = node->minor->dev;
> +	struct drm_modeset_acquire_ctx ctx;
>  	unsigned int i;
>  	int err = 0;
>  
> -	drm_modeset_lock_all(drm);
> +	DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
>  
>  	if (!crtc || !crtc->state->active) {
>  		err = -EBUSY;
> @@ -1049,7 +1050,7 @@ static int tegra_hdmi_show_regs(struct seq_file *s, void *data)
>  	}
>  
>  unlock:
> -	drm_modeset_unlock_all(drm);
> +	DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index 0ea320c1092b..323d95eb0cac 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -1490,10 +1490,11 @@ static int tegra_sor_show_crc(struct seq_file *s, void *data)
>  	struct tegra_sor *sor = node->info_ent->data;
>  	struct drm_crtc *crtc = sor->output.encoder.crtc;
>  	struct drm_device *drm = node->minor->dev;
> +	struct drm_modeset_acquire_ctx ctx;
>  	int err = 0;
>  	u32 value;
>  
> -	drm_modeset_lock_all(drm);
> +	DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
>  
>  	if (!crtc || !crtc->state->active) {
>  		err = -EBUSY;
> @@ -1522,7 +1523,7 @@ static int tegra_sor_show_crc(struct seq_file *s, void *data)
>  	seq_printf(s, "%08x\n", value);
>  
>  unlock:
> -	drm_modeset_unlock_all(drm);
> +	DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
>  	return err;
>  }
>  
> @@ -1652,10 +1653,11 @@ static int tegra_sor_show_regs(struct seq_file *s, void *data)
>  	struct tegra_sor *sor = node->info_ent->data;
>  	struct drm_crtc *crtc = sor->output.encoder.crtc;
>  	struct drm_device *drm = node->minor->dev;
> +	struct drm_modeset_acquire_ctx ctx;
>  	unsigned int i;
>  	int err = 0;
>  
> -	drm_modeset_lock_all(drm);
> +	DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
>  
>  	if (!crtc || !crtc->state->active) {
>  		err = -EBUSY;
> @@ -1670,7 +1672,7 @@ static int tegra_sor_show_regs(struct seq_file *s, void *data)
>  	}
>  
>  unlock:
> -	drm_modeset_unlock_all(drm);
> +	DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
>  	return err;
>  }
>  
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 07/15] drm/shmobile: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 07/15] drm/shmobile: " Fernando Ramos
@ 2021-09-17 15:38   ` Sean Paul
  0 siblings, 0 replies; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:38 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:44PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace driver calls to
> drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
> DRM_MODESET_LOCK_ALL_END()
> 
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>

Reviewed-by: Sean Paul <sean@poorly.run>

> ---
>  drivers/gpu/drm/shmobile/shmob_drm_drv.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> index 7db01904d18d..8ee215ab614e 100644
> --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> @@ -156,10 +156,12 @@ static int shmob_drm_pm_suspend(struct device *dev)
>  static int shmob_drm_pm_resume(struct device *dev)
>  {
>  	struct shmob_drm_device *sdev = dev_get_drvdata(dev);
> +	struct drm_modeset_acquire_ctx ctx;
> +	int ret;
>  
> -	drm_modeset_lock_all(sdev->ddev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(sdev->ddev, ctx, 0, ret);
>  	shmob_drm_crtc_resume(&sdev->crtc);
> -	drm_modeset_unlock_all(sdev->ddev);
> +	DRM_MODESET_LOCK_ALL_END(sdev->ddev, ctx, ret);
>  
>  	drm_kms_helper_poll_enable(sdev->ddev);
>  	return 0;
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 08/15] drm/radeon: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 08/15] drm/radeon: " Fernando Ramos
@ 2021-09-17 15:40   ` Sean Paul
  2021-09-17 22:32     ` Fernando Ramos
  0 siblings, 1 reply; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:40 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:45PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace driver calls to
> drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
> DRM_MODESET_LOCK_ALL_END()
> 
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  drivers/gpu/drm/radeon/radeon_device.c | 13 +++++++++----
>  drivers/gpu/drm/radeon/radeon_dp_mst.c |  7 +++++--
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index 4f0fbf667431..3feb1fd44409 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -37,6 +37,7 @@
>  #include <drm/drm_cache.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_device.h>
> +#include <drm/drm_drv.h>
>  #include <drm/drm_file.h>
>  #include <drm/drm_probe_helper.h>
>  #include <drm/radeon_drm.h>
> @@ -1559,7 +1560,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
>  	struct pci_dev *pdev;
>  	struct drm_crtc *crtc;
>  	struct drm_connector *connector;
> +	struct drm_modeset_acquire_ctx ctx;
>  	int i, r;
> +	int ret;

Could you please tuck this up with i & r?

>  
>  	if (dev == NULL || dev->dev_private == NULL) {
>  		return -ENODEV;
> @@ -1573,12 +1576,12 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
>  
>  	drm_kms_helper_poll_disable(dev);
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	/* turn off display hw */
>  	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>  		drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
>  	}
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);

You should check ret here

>  
>  	/* unpin the front buffers and cursors */
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> @@ -1663,7 +1666,9 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
>  	struct radeon_device *rdev = dev->dev_private;
>  	struct pci_dev *pdev = to_pci_dev(dev->dev);
>  	struct drm_crtc *crtc;
> +	struct drm_modeset_acquire_ctx ctx;
>  	int r;
> +	int ret;

Same suggestion here, move up with r

>  
>  	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>  		return 0;
> @@ -1741,11 +1746,11 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
>  	if (fbcon) {
>  		drm_helper_resume_force_mode(dev);
>  		/* turn on display hw */
> -		drm_modeset_lock_all(dev);
> +		DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  		list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>  			drm_helper_connector_dpms(connector, DRM_MODE_DPMS_ON);
>  		}
> -		drm_modeset_unlock_all(dev);
> +		DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);

Also check ret here


>  	}
>  
>  	drm_kms_helper_poll_enable(dev);
> diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> index ec867fa880a4..04fe7b0a6746 100644
> --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> @@ -4,6 +4,7 @@
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_file.h>
>  #include <drm/drm_probe_helper.h>
> +#include <drm/drm_drv.h>
>  
>  #include "atom.h"
>  #include "ni_reg.h"
> @@ -737,11 +738,13 @@ static int radeon_debugfs_mst_info_show(struct seq_file *m, void *unused)
>  	struct radeon_device *rdev = (struct radeon_device *)m->private;
>  	struct drm_device *dev = rdev->ddev;
>  	struct drm_connector *connector;
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct radeon_connector *radeon_connector;
>  	struct radeon_connector_atom_dig *dig_connector;
>  	int i;
> +	int ret;

Move up with i

>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>  		if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort)
>  			continue;
> @@ -759,7 +762,7 @@ static int radeon_debugfs_mst_info_show(struct seq_file *m, void *unused)
>  				   radeon_connector->cur_stream_attribs[i].fe,
>  				   radeon_connector->cur_stream_attribs[i].slots);
>  	}
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  	return 0;
>  }
>  
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 09/15] drm/omapdrm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 09/15] drm/omapdrm: " Fernando Ramos
@ 2021-09-17 15:40   ` Sean Paul
  2021-09-17 15:41   ` Sean Paul
  1 sibling, 0 replies; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:40 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:46PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace driver calls to
> drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
> DRM_MODESET_LOCK_ALL_END()
> 
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  drivers/gpu/drm/omapdrm/omap_fb.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
> index 190afc564914..56013c3ef701 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> @@ -62,13 +62,15 @@ static int omap_framebuffer_dirty(struct drm_framebuffer *fb,
>  				  unsigned num_clips)
>  {
>  	struct drm_crtc *crtc;
> +	struct drm_modeset_acquire_ctx ctx;
> +	int ret;
>  
> -	drm_modeset_lock_all(fb->dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(fb->dev, ctx, 0, ret);
>  
>  	drm_for_each_crtc(crtc, fb->dev)
>  		omap_crtc_flush(crtc);
>  
> -	drm_modeset_unlock_all(fb->dev);
> +	DRM_MODESET_LOCK_ALL_END(fb->dev, ctx, ret);
>  
>  	return 0;

Return ret here

>  }
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 09/15] drm/omapdrm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 09/15] drm/omapdrm: " Fernando Ramos
  2021-09-17 15:40   ` Sean Paul
@ 2021-09-17 15:41   ` Sean Paul
  2021-09-17 22:37     ` Fernando Ramos
  1 sibling, 1 reply; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:41 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:46PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace driver calls to
> drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
> DRM_MODESET_LOCK_ALL_END()
> 
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  drivers/gpu/drm/omapdrm/omap_fb.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
> index 190afc564914..56013c3ef701 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> @@ -62,13 +62,15 @@ static int omap_framebuffer_dirty(struct drm_framebuffer *fb,
>  				  unsigned num_clips)
>  {
>  	struct drm_crtc *crtc;
> +	struct drm_modeset_acquire_ctx ctx;
> +	int ret;
>  
> -	drm_modeset_lock_all(fb->dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(fb->dev, ctx, 0, ret);
>  
>  	drm_for_each_crtc(crtc, fb->dev)
>  		omap_crtc_flush(crtc);
>  
> -	drm_modeset_unlock_all(fb->dev);
> +	DRM_MODESET_LOCK_ALL_END(fb->dev, ctx, ret);
>  
>  	return 0;

Return ret here, with that,

Reviewed-by: Sean Paul <sean@poorly.run>


>  }
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 10/15] drm/nouveau: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 10/15] drm/nouveau: " Fernando Ramos
@ 2021-09-17 15:42   ` Sean Paul
  0 siblings, 0 replies; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:42 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:47PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace driver calls to
> drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
> DRM_MODESET_LOCK_ALL_END()
> 
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  drivers/gpu/drm/nouveau/dispnv50/disp.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index d7b9f7f8c9e3..eb613af4cdd5 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -667,15 +667,17 @@ nv50_audio_component_bind(struct device *kdev, struct device *hda_kdev,
>  	struct drm_device *drm_dev = dev_get_drvdata(kdev);
>  	struct nouveau_drm *drm = nouveau_drm(drm_dev);
>  	struct drm_audio_component *acomp = data;
> +	struct drm_modeset_acquire_ctx ctx;
> +	int ret;
>  
>  	if (WARN_ON(!device_link_add(hda_kdev, kdev, DL_FLAG_STATELESS)))
>  		return -ENOMEM;
>  
> -	drm_modeset_lock_all(drm_dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(drm_dev, ctx, 0, ret);
>  	acomp->ops = &nv50_audio_component_ops;
>  	acomp->dev = kdev;
>  	drm->audio.component = acomp;
> -	drm_modeset_unlock_all(drm_dev);
> +	DRM_MODESET_LOCK_ALL_END(drm_dev, ctx, ret);
>  	return 0;

Return ret here, with that fixed,

Reviewed-by: Sean Paul <sean@poorly.run>


>  }
>  
> @@ -686,12 +688,14 @@ nv50_audio_component_unbind(struct device *kdev, struct device *hda_kdev,
>  	struct drm_device *drm_dev = dev_get_drvdata(kdev);
>  	struct nouveau_drm *drm = nouveau_drm(drm_dev);
>  	struct drm_audio_component *acomp = data;
> +	struct drm_modeset_acquire_ctx ctx;
> +	int ret;
>  
> -	drm_modeset_lock_all(drm_dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(drm_dev, ctx, 0, ret);
>  	drm->audio.component = NULL;
>  	acomp->ops = NULL;
>  	acomp->dev = NULL;
> -	drm_modeset_unlock_all(drm_dev);
> +	DRM_MODESET_LOCK_ALL_END(drm_dev, ctx, ret);
>  }
>  
>  static const struct component_ops nv50_audio_component_bind_ops = {
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 11/15] drm/msm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 11/15] drm/msm: " Fernando Ramos
@ 2021-09-17 15:42   ` Sean Paul
  2021-09-17 22:41     ` Fernando Ramos
  0 siblings, 1 reply; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:42 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:48PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace driver calls to
> drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
> DRM_MODESET_LOCK_ALL_END()
> 
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 768012243b44..4cbc79eaee17 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1172,14 +1172,16 @@ static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
>  	struct drm_display_mode *mode;
>  	struct drm_framebuffer *fb;
>  	struct drm_plane_state *state;
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct dpu_crtc_state *cstate;
>  
>  	int i, out_width;
> +	int ret;

Please put ret with i & out_width

>  
>  	dpu_crtc = s->private;
>  	crtc = &dpu_crtc->base;
>  
> -	drm_modeset_lock_all(crtc->dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(crtc->dev, ctx, 0, ret);
>  	cstate = to_dpu_crtc_state(crtc->state);
>  
>  	mode = &crtc->state->adjusted_mode;
> @@ -1263,7 +1265,7 @@ static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
>  		dpu_crtc->vblank_cb_time = ktime_set(0, 0);
>  	}
>  
> -	drm_modeset_unlock_all(crtc->dev);
> +	DRM_MODESET_LOCK_ALL_END(crtc->dev, ctx, ret);
>  
>  	return 0;

Return ret here

>  }
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 12/15] drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 12/15] drm/i915: " Fernando Ramos
@ 2021-09-17 15:48   ` Sean Paul
  2021-09-17 22:57     ` Fernando Ramos
  0 siblings, 1 reply; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:48 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:49PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace driver calls to
> drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
> DRM_MODESET_LOCK_ALL_END()
> 
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  drivers/gpu/drm/i915/display/intel_audio.c    | 12 +++--
>  drivers/gpu/drm/i915/display/intel_display.c  |  5 ++-
>  .../drm/i915/display/intel_display_debugfs.c  | 35 ++++++++++-----
>  drivers/gpu/drm/i915/display/intel_overlay.c  | 45 +++++++++----------
>  drivers/gpu/drm/i915/display/intel_pipe_crc.c |  5 ++-
>  drivers/gpu/drm/i915/i915_drv.c               | 12 +++--
>  6 files changed, 67 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index 532237588511..ab6a5a734b95 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -1214,7 +1214,9 @@ static int i915_audio_component_bind(struct device *i915_kdev,
>  {
>  	struct i915_audio_component *acomp = data;
>  	struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
> +	struct drm_modeset_acquire_ctx ctx;
>  	int i;
> +	int ret;

Please move up with i

>  
>  	if (drm_WARN_ON(&dev_priv->drm, acomp->base.ops || acomp->base.dev))
>  		return -EEXIST;
> @@ -1224,14 +1226,14 @@ static int i915_audio_component_bind(struct device *i915_kdev,
>  					 DL_FLAG_STATELESS)))
>  		return -ENOMEM;
>  
> -	drm_modeset_lock_all(&dev_priv->drm);
> +	DRM_MODESET_LOCK_ALL_BEGIN((&dev_priv->drm), ctx, 0, ret);
>  	acomp->base.ops = &i915_audio_component_ops;
>  	acomp->base.dev = i915_kdev;
>  	BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS);
>  	for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++)
>  		acomp->aud_sample_rate[i] = 0;
>  	dev_priv->audio_component = acomp;
> -	drm_modeset_unlock_all(&dev_priv->drm);
> +	DRM_MODESET_LOCK_ALL_END((&dev_priv->drm), ctx, ret);
>  
>  	return 0;

Return ret here

>  }
> @@ -1241,12 +1243,14 @@ static void i915_audio_component_unbind(struct device *i915_kdev,
>  {
>  	struct i915_audio_component *acomp = data;
>  	struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
> +	struct drm_modeset_acquire_ctx ctx;
> +	int ret;
>  
> -	drm_modeset_lock_all(&dev_priv->drm);
> +	DRM_MODESET_LOCK_ALL_BEGIN((&dev_priv->drm), ctx, 0, ret);
>  	acomp->base.ops = NULL;
>  	acomp->base.dev = NULL;
>  	dev_priv->audio_component = NULL;
> -	drm_modeset_unlock_all(&dev_priv->drm);
> +	DRM_MODESET_LOCK_ALL_END((&dev_priv->drm), ctx, ret);
>  
>  	device_link_remove(hda_kdev, i915_kdev);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 997a16e85c85..dc2e4d89e5aa 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -12511,6 +12511,7 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  int intel_modeset_init_nogem(struct drm_i915_private *i915)
>  {
>  	struct drm_device *dev = &i915->drm;
> +	struct drm_modeset_acquire_ctx ctx;
>  	enum pipe pipe;
>  	struct intel_crtc *crtc;
>  	int ret;
> @@ -12562,9 +12563,9 @@ int intel_modeset_init_nogem(struct drm_i915_private *i915)
>  	intel_vga_disable(i915);
>  	intel_setup_outputs(i915);
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	intel_modeset_setup_hw_state(dev, dev->mode_config.acquire_ctx);
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  
>  	for_each_intel_crtc(dev, crtc) {
>  		struct intel_initial_plane_config plane_config = {};
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 8fdacb252bb1..d73af228862e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1057,11 +1057,13 @@ static int i915_display_info(struct seq_file *m, void *unused)
>  	struct intel_crtc *crtc;
>  	struct drm_connector *connector;
>  	struct drm_connector_list_iter conn_iter;
> +	struct drm_modeset_acquire_ctx ctx;
>  	intel_wakeref_t wakeref;
> +	int ret;
>  
>  	wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  
>  	seq_printf(m, "CRTC info\n");
>  	seq_printf(m, "---------\n");
> @@ -1076,7 +1078,7 @@ static int i915_display_info(struct seq_file *m, void *unused)
>  		intel_connector_info(m, connector);
>  	drm_connector_list_iter_end(&conn_iter);
>  
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  
>  	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
>  
> @@ -1087,9 +1089,11 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>  	struct drm_device *dev = &dev_priv->drm;
> +	struct drm_modeset_acquire_ctx ctx;
>  	int i;
> +	int ret;

Please move up with i

>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  
>  	seq_printf(m, "PLL refclks: non-SSC: %d kHz, SSC: %d kHz\n",
>  		   dev_priv->dpll.ref_clks.nssc,
> @@ -1132,7 +1136,7 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
>  		seq_printf(m, " mg_pll_tdc_coldst_bias: 0x%08x\n",
>  			   pll->state.hw_state.mg_pll_tdc_coldst_bias);
>  	}
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  
>  	return 0;

Return ret

>  }
> @@ -1193,13 +1197,15 @@ static int i915_ddb_info(struct seq_file *m, void *unused)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>  	struct drm_device *dev = &dev_priv->drm;
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct skl_ddb_entry *entry;
>  	struct intel_crtc *crtc;
> +	int ret;
>  
>  	if (DISPLAY_VER(dev_priv) < 9)
>  		return -ENODEV;
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  
>  	seq_printf(m, "%-15s%8s%8s%8s\n", "", "Start", "End", "Size");
>  
> @@ -1223,7 +1229,7 @@ static int i915_ddb_info(struct seq_file *m, void *unused)
>  			   entry->end, skl_ddb_entry_size(entry));
>  	}
>  
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  
>  	return 0;

Return ret

>  }
> @@ -1303,10 +1309,12 @@ static int i915_drrs_status(struct seq_file *m, void *unused)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>  	struct drm_device *dev = &dev_priv->drm;
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct intel_crtc *crtc;
>  	int active_crtc_cnt = 0;
> +	int ret;
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	for_each_intel_crtc(dev, crtc) {
>  		if (crtc->base.state->active) {
>  			active_crtc_cnt++;
> @@ -1315,7 +1323,7 @@ static int i915_drrs_status(struct seq_file *m, void *unused)
>  			drrs_status_per_crtc(m, dev, crtc);
>  		}
>  	}
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  
>  	if (!active_crtc_cnt)
>  		seq_puts(m, "No active crtc found\n");
> @@ -1607,8 +1615,10 @@ static void wm_latency_show(struct seq_file *m, const u16 wm[8])
>  {
>  	struct drm_i915_private *dev_priv = m->private;
>  	struct drm_device *dev = &dev_priv->drm;
> +	struct drm_modeset_acquire_ctx ctx;
>  	int level;
>  	int num_levels;
> +	int ret;
>  
>  	if (IS_CHERRYVIEW(dev_priv))
>  		num_levels = 3;
> @@ -1619,7 +1629,7 @@ static void wm_latency_show(struct seq_file *m, const u16 wm[8])
>  	else
>  		num_levels = ilk_wm_max_level(dev_priv) + 1;
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  
>  	for (level = 0; level < num_levels; level++) {
>  		unsigned int latency = wm[level];
> @@ -1640,7 +1650,7 @@ static void wm_latency_show(struct seq_file *m, const u16 wm[8])
>  			   level, wm[level], latency / 10, latency % 10);
>  	}
>  
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  }
>  
>  static int pri_wm_latency_show(struct seq_file *m, void *data)
> @@ -1724,6 +1734,7 @@ static ssize_t wm_latency_write(struct file *file, const char __user *ubuf,
>  	struct seq_file *m = file->private_data;
>  	struct drm_i915_private *dev_priv = m->private;
>  	struct drm_device *dev = &dev_priv->drm;
> +	struct drm_modeset_acquire_ctx ctx;
>  	u16 new[8] = { 0 };
>  	int num_levels;
>  	int level;
> @@ -1753,12 +1764,12 @@ static ssize_t wm_latency_write(struct file *file, const char __user *ubuf,
>  	if (ret != num_levels)
>  		return -EINVAL;
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  
>  	for (level = 0; level < num_levels; level++)
>  		wm[level] = new[level];
>  
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  

Check ret here and return an error if it's != 0

>  	return len;
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
> index 7e3f5c6ca484..79c6940807a7 100644
> --- a/drivers/gpu/drm/i915/display/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c
> @@ -1104,6 +1104,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
>  	struct drm_crtc *drmmode_crtc;
>  	struct intel_crtc *crtc;
>  	struct drm_i915_gem_object *new_bo;
> +	struct drm_modeset_acquire_ctx ctx;
>  	int ret;
>  
>  	overlay = dev_priv->overlay;
> @@ -1112,24 +1113,24 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
>  		return -ENODEV;
>  	}
>  
> -	if (!(params->flags & I915_OVERLAY_ENABLE)) {
> -		drm_modeset_lock_all(dev);
> -		ret = intel_overlay_switch_off(overlay);
> -		drm_modeset_unlock_all(dev);
> +	if (params->flags & I915_OVERLAY_ENABLE) {
>  
> -		return ret;
> -	}
> +		drmmode_crtc = drm_crtc_find(dev, file_priv, params->crtc_id);
> +		if (!drmmode_crtc)
> +			return -ENOENT;
> +		crtc = to_intel_crtc(drmmode_crtc);
>  
> -	drmmode_crtc = drm_crtc_find(dev, file_priv, params->crtc_id);
> -	if (!drmmode_crtc)
> -		return -ENOENT;
> -	crtc = to_intel_crtc(drmmode_crtc);
> +		new_bo = i915_gem_object_lookup(file_priv, params->bo_handle);
> +		if (!new_bo)
> +			return -ENOENT;
> +	}
>  
> -	new_bo = i915_gem_object_lookup(file_priv, params->bo_handle);
> -	if (!new_bo)
> -		return -ENOENT;
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  
> -	drm_modeset_lock_all(dev);
> +	if (!(params->flags & I915_OVERLAY_ENABLE)) {
> +		ret = intel_overlay_switch_off(overlay);
> +		goto out_unlock;
> +	}
>  
>  	if (i915_gem_object_is_tiled(new_bo)) {
>  		drm_dbg_kms(&dev_priv->drm,
> @@ -1194,14 +1195,11 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
>  	if (ret != 0)
>  		goto out_unlock;
>  
> -	drm_modeset_unlock_all(dev);
> -	i915_gem_object_put(new_bo);
> -
> -	return 0;
> -
>  out_unlock:
> -	drm_modeset_unlock_all(dev);
> -	i915_gem_object_put(new_bo);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> +
> +	if (params->flags & I915_OVERLAY_ENABLE)
> +		i915_gem_object_put(new_bo);

This function refactor is a bit more involved than the
s/drm_modeset_lock_all/DRM_MODESET_LOCK_ALL_*/ changes in the rest of the patch.
Could you split it out into a separate patch so it's not hidden away?

>  
>  	return ret;
>  }
> @@ -1263,6 +1261,7 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>  {
>  	struct drm_intel_overlay_attrs *attrs = data;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct intel_overlay *overlay;
>  	int ret;
>  
> @@ -1272,7 +1271,7 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>  		return -ENODEV;
>  	}
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  
>  	ret = -EINVAL;
>  	if (!(attrs->flags & I915_OVERLAY_UPDATE_ATTRS)) {
> @@ -1329,7 +1328,7 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>  
>  	ret = 0;
>  out_unlock:
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
> index 8ac263f471be..e50e514e4897 100644
> --- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c
> +++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
> @@ -76,6 +76,7 @@ static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv,
>  				     enum intel_pipe_crc_source *source)
>  {
>  	struct drm_device *dev = &dev_priv->drm;
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct intel_encoder *encoder;
>  	struct intel_crtc *crtc;
>  	struct intel_digital_port *dig_port;
> @@ -83,7 +84,7 @@ static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv,
>  
>  	*source = INTEL_PIPE_CRC_SOURCE_PIPE;
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	for_each_intel_encoder(dev, encoder) {
>  		if (!encoder->base.crtc)
>  			continue;
> @@ -120,7 +121,7 @@ static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv,
>  			break;
>  		}
>  	}
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 59fb4c710c8c..7a30e2ff2fed 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1009,31 +1009,35 @@ static void i915_driver_postclose(struct drm_device *dev, struct drm_file *file)
>  static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = &dev_priv->drm;
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct intel_encoder *encoder;
> +	int ret;
>  
>  	if (!HAS_DISPLAY(dev_priv))
>  		return;
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	for_each_intel_encoder(dev, encoder)
>  		if (encoder->suspend)
>  			encoder->suspend(encoder);
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  }
>  
>  static void intel_shutdown_encoders(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = &dev_priv->drm;
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct intel_encoder *encoder;
> +	int ret;
>  
>  	if (!HAS_DISPLAY(dev_priv))
>  		return;
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	for_each_intel_encoder(dev, encoder)
>  		if (encoder->shutdown)
>  			encoder->shutdown(encoder);
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  }
>  
>  void i915_driver_shutdown(struct drm_i915_private *i915)
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 13/15] drm/gma500: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 13/15] drm/gma500: " Fernando Ramos
@ 2021-09-17 15:49   ` Sean Paul
  0 siblings, 0 replies; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:49 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:50PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace driver calls to
> drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
> DRM_MODESET_LOCK_ALL_END()
> 
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  drivers/gpu/drm/gma500/psb_device.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gma500/psb_device.c b/drivers/gpu/drm/gma500/psb_device.c
> index 951725a0f7a3..4e27f65a1f16 100644
> --- a/drivers/gpu/drm/gma500/psb_device.c
> +++ b/drivers/gpu/drm/gma500/psb_device.c
> @@ -8,6 +8,7 @@
>  #include <linux/backlight.h>
>  
>  #include <drm/drm.h>
> +#include <drm/drm_drv.h>
>  
>  #include "gma_device.h"
>  #include "intel_bios.h"
> @@ -169,8 +170,10 @@ static int psb_save_display_registers(struct drm_device *dev)
>  {
>  	struct drm_psb_private *dev_priv = dev->dev_private;
>  	struct drm_crtc *crtc;
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct gma_connector *connector;
>  	struct psb_state *regs = &dev_priv->regs.psb;
> +	int ret;
>  
>  	/* Display arbitration control + watermarks */
>  	regs->saveDSPARB = PSB_RVDC32(DSPARB);
> @@ -183,7 +186,7 @@ static int psb_save_display_registers(struct drm_device *dev)
>  	regs->saveCHICKENBIT = PSB_RVDC32(DSPCHICKENBIT);
>  
>  	/* Save crtc and output state */
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
>  		if (drm_helper_crtc_in_use(crtc))
>  			dev_priv->ops->save_crtc(crtc);
> @@ -193,7 +196,8 @@ static int psb_save_display_registers(struct drm_device *dev)
>  		if (connector->save)
>  			connector->save(&connector->base);
>  
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> +
>  	return 0;

Return ret here please

>  }
>  
> @@ -207,8 +211,10 @@ static int psb_restore_display_registers(struct drm_device *dev)
>  {
>  	struct drm_psb_private *dev_priv = dev->dev_private;
>  	struct drm_crtc *crtc;
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct gma_connector *connector;
>  	struct psb_state *regs = &dev_priv->regs.psb;
> +	int ret;
>  
>  	/* Display arbitration + watermarks */
>  	PSB_WVDC32(regs->saveDSPARB, DSPARB);
> @@ -223,7 +229,7 @@ static int psb_restore_display_registers(struct drm_device *dev)
>  	/*make sure VGA plane is off. it initializes to on after reset!*/
>  	PSB_WVDC32(0x80000000, VGACNTRL);
>  
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
>  		if (drm_helper_crtc_in_use(crtc))
>  			dev_priv->ops->restore_crtc(crtc);
> @@ -232,7 +238,7 @@ static int psb_restore_display_registers(struct drm_device *dev)
>  		if (connector->restore)
>  			connector->restore(&connector->base);
>  
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  	return 0;

Here too

>  }
>  
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 14/15] drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 14/15] drm/amd: " Fernando Ramos
@ 2021-09-17 15:55   ` Sean Paul
  2021-09-17 23:17     ` Fernando Ramos
  0 siblings, 1 reply; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:55 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:51PM +0200, Fernando Ramos wrote:
> As requested in Documentation/gpu/todo.rst, replace driver calls to
> drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
> DRM_MODESET_LOCK_ALL_END()
> 
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 13 +++--
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 +++++++++----------
>  .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 23 +++++----
>  3 files changed, 46 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 7a7316731911..55ecc4aa859f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -40,6 +40,7 @@
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_vblank.h>
> +#include <drm/drm_drv.h>
>  
>  static void amdgpu_display_flip_callback(struct dma_fence *f,
>  					 struct dma_fence_cb *cb)
> @@ -1543,16 +1544,18 @@ int amdgpu_display_suspend_helper(struct amdgpu_device *adev)
>  	struct drm_crtc *crtc;
>  	struct drm_connector *connector;
>  	struct drm_connector_list_iter iter;
> +	struct drm_modeset_acquire_ctx ctx;
>  	int r;
> +	int ret;

Relocate ret with r please

>  
>  	/* turn off display hw */
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  	drm_connector_list_iter_begin(dev, &iter);
>  	drm_for_each_connector_iter(connector, &iter)
>  		drm_helper_connector_dpms(connector,
>  					  DRM_MODE_DPMS_OFF);
>  	drm_connector_list_iter_end(&iter);
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);


You should check ret here

>  	/* unpin the front buffers and cursors */
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
>  		struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> @@ -1590,7 +1593,9 @@ int amdgpu_display_resume_helper(struct amdgpu_device *adev)
>  	struct drm_connector *connector;
>  	struct drm_connector_list_iter iter;
>  	struct drm_crtc *crtc;
> +	struct drm_modeset_acquire_ctx ctx;
>  	int r;
> +	int ret;

Relocate ret with r

>  
>  	/* pin cursors */
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> @@ -1612,7 +1617,7 @@ int amdgpu_display_resume_helper(struct amdgpu_device *adev)
>  	drm_helper_resume_force_mode(dev);
>  
>  	/* turn on display hw */
> -	drm_modeset_lock_all(dev);
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>  
>  	drm_connector_list_iter_begin(dev, &iter);
>  	drm_for_each_connector_iter(connector, &iter)
> @@ -1620,7 +1625,7 @@ int amdgpu_display_resume_helper(struct amdgpu_device *adev)
>  					  DRM_MODE_DPMS_ON);
>  	drm_connector_list_iter_end(&iter);
>  
> -	drm_modeset_unlock_all(dev);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>  
>  	return 0;

Return ret

>  }
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 9b1fc54555ee..5196c1d26f87 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -80,6 +80,7 @@
>  #include <drm/drm_edid.h>
>  #include <drm/drm_vblank.h>
>  #include <drm/drm_audio_component.h>
> +#include <drm/drm_drv.h>
>  
>  #if defined(CONFIG_DRM_AMD_DC_DCN)
>  #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h"
> @@ -2621,6 +2622,9 @@ static void handle_hpd_irq(void *param)
>  #ifdef CONFIG_DRM_AMD_DC_HDCP
>  	struct dm_connector_state *dm_con_state = to_dm_connector_state(connector->state);
>  #endif
> +	struct drm_modeset_acquire_ctx ctx;
> +	int ret;
> +
>  
>  	if (adev->dm.disable_hpd_irq)
>  		return;
> @@ -2646,14 +2650,6 @@ static void handle_hpd_irq(void *param)
>  	if (aconnector->base.force && new_connection_type == dc_connection_none) {
>  		emulated_link_detect(aconnector->dc_link);
>  
> -
> -		drm_modeset_lock_all(dev);
> -		dm_restore_drm_connector_state(dev, connector);
> -		drm_modeset_unlock_all(dev);
> -
> -		if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
> -			drm_kms_helper_hotplug_event(dev);
> -
>  	} else if (dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD)) {
>  		if (new_connection_type == dc_connection_none &&
>  		    aconnector->dc_link->type == dc_connection_none)
> @@ -2661,13 +2657,18 @@ static void handle_hpd_irq(void *param)
>  
>  		amdgpu_dm_update_connector_after_detect(aconnector);
>  
> -		drm_modeset_lock_all(dev);
> -		dm_restore_drm_connector_state(dev, connector);
> -		drm_modeset_unlock_all(dev);
> -
> -		if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
> -			drm_kms_helper_hotplug_event(dev);
> +	} else {
> +		goto out;
>  	}
> +
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
> +	dm_restore_drm_connector_state(dev, connector);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);

Check ret here please

> +
> +	if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
> +		drm_kms_helper_hotplug_event(dev);
> +
> +out:
>  	mutex_unlock(&aconnector->hpd_lock);
>  
>  }
> @@ -2756,12 +2757,14 @@ static void handle_hpd_rx_irq(void *param)
>  	struct drm_connector *connector = &aconnector->base;
>  	struct drm_device *dev = connector->dev;
>  	struct dc_link *dc_link = aconnector->dc_link;
> +	struct drm_modeset_acquire_ctx ctx;
>  	bool is_mst_root_connector = aconnector->mst_mgr.mst_state;
>  	bool result = false;
>  	enum dc_connection_type new_connection_type = dc_connection_none;
>  	struct amdgpu_device *adev = drm_to_adev(dev);
>  	union hpd_irq_data hpd_irq_data;
>  	bool lock_flag = 0;
> +	int ret;
>  
>  	memset(&hpd_irq_data, 0, sizeof(hpd_irq_data));
>  
> @@ -2828,12 +2831,6 @@ static void handle_hpd_rx_irq(void *param)
>  
>  			amdgpu_dm_update_connector_after_detect(aconnector);
>  
> -
> -			drm_modeset_lock_all(dev);
> -			dm_restore_drm_connector_state(dev, connector);
> -			drm_modeset_unlock_all(dev);
> -
> -			drm_kms_helper_hotplug_event(dev);
>  		} else if (dc_link_detect(dc_link, DETECT_REASON_HPDRX)) {
>  
>  			if (aconnector->fake_enable)
> @@ -2841,14 +2838,17 @@ static void handle_hpd_rx_irq(void *param)
>  
>  			amdgpu_dm_update_connector_after_detect(aconnector);
>  
> +		} else {
> +			goto finish;

You used 'out' above and 'finish' here. It would be nice to be consistent with
naming, I see 'out' a lot more than 'finish', so my vote would be to change this
label to 'out'.

> +		}
>  
> -			drm_modeset_lock_all(dev);
> -			dm_restore_drm_connector_state(dev, connector);
> -			drm_modeset_unlock_all(dev);
> +		DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
> +		dm_restore_drm_connector_state(dev, connector);
> +		DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);

Check ret here?

>  
> -			drm_kms_helper_hotplug_event(dev);
> -		}
> +		drm_kms_helper_hotplug_event(dev);
>  	}
> +finish:
>  #ifdef CONFIG_DRM_AMD_DC_HDCP
>  	if (hpd_irq_data.bytes.device_service_irq.bits.CP_IRQ) {
>  		if (adev->dm.hdcp_workqueue)
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> index 87daa78a32b8..6e5a8f4fc6bd 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> @@ -37,6 +37,8 @@
>  #include "link_hwss.h"
>  #include "dc/dc_dmub_srv.h"
>  
> +#include <drm/drm_drv.h>
> +

Top-level headers generally come above the driver headers. Also, now that I think
about this a bit more, all of the new includes in this set should probably be
for 'drm_modeset_lock.h' instead of 'drm_drv.h'.

>  struct dmub_debugfs_trace_header {
>  	uint32_t entry_count;
>  	uint32_t reserved[3];
> @@ -1191,12 +1193,14 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
>  	struct drm_connector *connector = &aconnector->base;
>  	struct dc_link *link = NULL;
>  	struct drm_device *dev = connector->dev;
> +	struct drm_modeset_acquire_ctx ctx;
>  	enum dc_connection_type new_connection_type = dc_connection_none;
>  	char *wr_buf = NULL;
>  	uint32_t wr_buf_size = 42;
>  	int max_param_num = 1;
>  	long param[1] = {0};
>  	uint8_t param_nums = 0;
> +	int ret;
>  
>  	if (!aconnector || !aconnector->dc_link)
>  		return -EINVAL;
> @@ -1236,12 +1240,6 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
>  			goto unlock;
>  
>  		amdgpu_dm_update_connector_after_detect(aconnector);
> -
> -		drm_modeset_lock_all(dev);
> -		dm_restore_drm_connector_state(dev, connector);
> -		drm_modeset_unlock_all(dev);
> -
> -		drm_kms_helper_hotplug_event(dev);
>  	} else if (param[0] == 0) {
>  		if (!aconnector->dc_link)
>  			goto unlock;
> @@ -1259,13 +1257,16 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
>  
>  		amdgpu_dm_update_connector_after_detect(aconnector);
>  
> -		drm_modeset_lock_all(dev);
> -		dm_restore_drm_connector_state(dev, connector);
> -		drm_modeset_unlock_all(dev);
> -
> -		drm_kms_helper_hotplug_event(dev);
> +	} else {
> +		goto unlock;
>  	}
>  
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
> +	dm_restore_drm_connector_state(dev, connector);
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);

Check ret here?

> +
> +	drm_kms_helper_hotplug_event(dev);
> +
>  unlock:
>  	mutex_unlock(&aconnector->hpd_lock);
>  
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 15/15] doc: drm: remove TODO entry regarding DRM_MODSET_LOCK_ALL cleanup
  2021-09-16 21:15 ` [Nouveau] [PATCH 15/15] doc: drm: remove TODO entry regarding DRM_MODSET_LOCK_ALL cleanup Fernando Ramos
@ 2021-09-17 15:56   ` Sean Paul
  2021-09-17 23:21     ` Fernando Ramos
  0 siblings, 1 reply; 45+ messages in thread
From: Sean Paul @ 2021-09-17 15:56 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On Thu, Sep 16, 2021 at 11:15:52PM +0200, Fernando Ramos wrote:
> The previous commits do exactly what this entry in the TODO file asks
> for, thus we can remove it now as it is no longer applicable.

Thanks for doing this work!

Can we remove drm_modeset_lock_all[_ctx] now? If so, let's queue that up as part
of the set.


Reviewed-by: Sean Paul <sean@poorly.run>


> 
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  Documentation/gpu/todo.rst                | 17 -----------------
>  Documentation/locking/ww-mutex-design.rst |  2 +-
>  2 files changed, 1 insertion(+), 18 deletions(-)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 12e61869939e..6613543955e9 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -353,23 +353,6 @@ converted, except for struct drm_driver.gem_prime_mmap.
>  
>  Level: Intermediate
>  
> -Use DRM_MODESET_LOCK_ALL_* helpers instead of boilerplate
> ----------------------------------------------------------
> -
> -For cases where drivers are attempting to grab the modeset locks with a local
> -acquire context. Replace the boilerplate code surrounding
> -drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() and
> -DRM_MODESET_LOCK_ALL_END() instead.
> -
> -This should also be done for all places where drm_modeset_lock_all() is still
> -used.
> -
> -As a reference, take a look at the conversions already completed in drm core.
> -
> -Contact: Sean Paul, respective driver maintainers
> -
> -Level: Starter
> -
>  Rename CMA helpers to DMA helpers
>  ---------------------------------
>  
> diff --git a/Documentation/locking/ww-mutex-design.rst b/Documentation/locking/ww-mutex-design.rst
> index 6a4d7319f8f0..6a8f8beb9ec4 100644
> --- a/Documentation/locking/ww-mutex-design.rst
> +++ b/Documentation/locking/ww-mutex-design.rst
> @@ -60,7 +60,7 @@ Concepts
>  Compared to normal mutexes two additional concepts/objects show up in the lock
>  interface for w/w mutexes:
>  
> -Acquire context: To ensure eventual forward progress it is important the a task
> +Acquire context: To ensure eventual forward progress it is important that a task
>  trying to acquire locks doesn't grab a new reservation id, but keeps the one it
>  acquired when starting the lock acquisition. This ticket is stored in the
>  acquire context. Furthermore the acquire context keeps track of debugging state
> -- 
> 2.33.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible
  2021-09-17 15:24 ` [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Daniel Vetter
@ 2021-09-17 21:41   ` Fernando Ramos
  0 siblings, 0 replies; 45+ messages in thread
From: Fernando Ramos @ 2021-09-17 21:41 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: dri-devel, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On 21/09/17 05:24PM, Daniel Vetter wrote:
>
> Can we at least replace those with drm_modeset_lock_all_ctx and delete
> drm_modeset_lock_all? That would be really nice goal to make sure these
> don't spread further.

I just checked and turns out no one else is using "drm_modeset_lock_all()"
anymore.

The only reference is the definition of the function itself, which I did not
remove because it was being EXPORT_SYMBOL'ed and I was not sure whether it could
be removed or not (to prevent breaking third party modules maybe?)

The same goes true for its sibling "dmr_modeset_unlock_all()".

But if you give me the green light I'll remove both of them right away :)


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 01/15] dmr: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-17 15:28   ` Sean Paul
@ 2021-09-17 22:07     ` Fernando Ramos
  0 siblings, 0 replies; 45+ messages in thread
From: Fernando Ramos @ 2021-09-17 22:07 UTC (permalink / raw)
  To: Sean Paul
  Cc: dri-devel, linux-kernel, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

> 
> Could you please fix the subject, changing dmr to drm?
> 

Ups! Sure, I'll fix that. Thanks for noticing.


>
> I think you can just reuse 'ret' instead of creating a new variable. That
> ensures if the lock fails we return the error from the macros.
> 

I didn't reuse "ret" because otherwise I would have had to change the prototype
of the function (which currently returns a "bool" instead of an "int").

However I could, for example, check for any error and convert that into "false".
Would that be ok?

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 08/15] drm/radeon: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-17 15:40   ` Sean Paul
@ 2021-09-17 22:32     ` Fernando Ramos
  0 siblings, 0 replies; 45+ messages in thread
From: Fernando Ramos @ 2021-09-17 22:32 UTC (permalink / raw)
  To: Sean Paul
  Cc: dri-devel, linux-kernel, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

> > +	struct drm_modeset_acquire_ctx ctx;
> >  	int i, r;
> > +	int ret;
> 
> Could you please tuck this up with i & r?

Done!


> > -	drm_modeset_unlock_all(dev);
> > +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> 
> You should check ret here

Would it be save to return at this point if the lock fails?

In other words, can I just add this? --> "if (ret) return ret;"


> > +	struct drm_modeset_acquire_ctx ctx;
> >  	int r;
> > +	int ret;
> 
> Same suggestion here, move up with r

Done!


> > -		drm_modeset_unlock_all(dev);
> > +		DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> 
> Also check ret here

Same question. Would "if (ret) return ret;" be safe here?


> >  	int i;
> > +	int ret;
> 
> Move up with i

Done!


> > -	drm_modeset_unlock_all(dev);
> > +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> >  	return 0;

I can also "return ret;" instead of "0".

What happens when a DEFINE_SHOW_ATTRIBUTE'd function returns non-zero? Is it ok?
Or do we want to always return "0" to print whatever we can?



^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 06/15] drm/tegra: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-17 15:38   ` Sean Paul
@ 2021-09-17 22:34     ` Fernando Ramos
  0 siblings, 0 replies; 45+ messages in thread
From: Fernando Ramos @ 2021-09-17 22:34 UTC (permalink / raw)
  To: Sean Paul
  Cc: dri-devel, linux-kernel, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

> >  	int err = 0;
> > +	int ret;
> 
> You can use err here instead.

Done!

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 09/15] drm/omapdrm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-17 15:41   ` Sean Paul
@ 2021-09-17 22:37     ` Fernando Ramos
  0 siblings, 0 replies; 45+ messages in thread
From: Fernando Ramos @ 2021-09-17 22:37 UTC (permalink / raw)
  To: Sean Paul
  Cc: dri-devel, linux-kernel, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

> > -	drm_modeset_unlock_all(fb->dev);
> > +	DRM_MODESET_LOCK_ALL_END(fb->dev, ctx, ret);
> >  
> >  	return 0;
> 
> Return ret here.

Done!

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 11/15] drm/msm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-17 15:42   ` Sean Paul
@ 2021-09-17 22:41     ` Fernando Ramos
  0 siblings, 0 replies; 45+ messages in thread
From: Fernando Ramos @ 2021-09-17 22:41 UTC (permalink / raw)
  To: Sean Paul
  Cc: dri-devel, linux-kernel, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

> >  	int i, out_width;
> > +	int ret;
> 
> Please put ret with i & out_width

Done!


> > -	drm_modeset_unlock_all(crtc->dev);
> > +	DRM_MODESET_LOCK_ALL_END(crtc->dev, ctx, ret);
> >  
> >  	return 0;
> 
> Return ret here

Done!

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 12/15] drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-17 15:48   ` Sean Paul
@ 2021-09-17 22:57     ` Fernando Ramos
  0 siblings, 0 replies; 45+ messages in thread
From: Fernando Ramos @ 2021-09-17 22:57 UTC (permalink / raw)
  To: Sean Paul
  Cc: dri-devel, linux-kernel, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

> >  	int i;
> > +	int ret;
> 
> Please move up with i

Done!


> > +	DRM_MODESET_LOCK_ALL_END((&dev_priv->drm), ctx, ret);
> >  
> >  	return 0;
> 
> Return ret here

Done!


> > +	struct drm_modeset_acquire_ctx ctx;
> >  	int i;
> > +	int ret;
> 
> Please move up with i

Done!


> > -	drm_modeset_unlock_all(dev);
> > +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> >  
> >  	return 0;
> 
> Return ret

Done!


> > -	drm_modeset_unlock_all(dev);
> > +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> >  
> >  	return 0;
> 
> Return ret

Done!


> > -	drm_modeset_unlock_all(dev);
> > +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> >  
> 
> Check ret here and return an error if it's != 0

Done!


> > @@ -1194,14 +1195,11 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
> >  	if (ret != 0)
> >  		goto out_unlock;
> >  
> > -	drm_modeset_unlock_all(dev);
> > -	i915_gem_object_put(new_bo);
> > -
> > -	return 0;
> > -
> >  out_unlock:
> > -	drm_modeset_unlock_all(dev);
> > -	i915_gem_object_put(new_bo);
> > +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> > +
> > +	if (params->flags & I915_OVERLAY_ENABLE)
> > +		i915_gem_object_put(new_bo);
> 
> This function refactor is a bit more involved than the
> s/drm_modeset_lock_all/DRM_MODESET_LOCK_ALL_*/ changes in the rest of the patch.
> Could you split it out into a separate patch so it's not hidden away?

Sure, no problem.


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 14/15] drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-17 15:55   ` Sean Paul
@ 2021-09-17 23:17     ` Fernando Ramos
  2021-09-18  9:42       ` Fernando Ramos
  0 siblings, 1 reply; 45+ messages in thread
From: Fernando Ramos @ 2021-09-17 23:17 UTC (permalink / raw)
  To: Sean Paul
  Cc: dri-devel, linux-kernel, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

> > +	struct drm_modeset_acquire_ctx ctx;
> >  	int r;
> > +	int ret;
> 
> Relocate ret with r please

Done!

> > -	drm_modeset_unlock_all(dev);
> > +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> 
> You should check ret here

Done!


> >  	int r;
> > +	int ret;
> 
> Relocate ret with r

Done!


> > -	drm_modeset_unlock_all(dev);
> > +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> >  
> >  	return 0;
> 
> Return ret

Done!


> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 9b1fc54555ee..5196c1d26f87 100644
> > @@ -2661,13 +2657,18 @@ static void handle_hpd_irq(void *param)
> >  
> >  		amdgpu_dm_update_connector_after_detect(aconnector);
> >  
> > -		drm_modeset_lock_all(dev);
> > -		dm_restore_drm_connector_state(dev, connector);
> > -		drm_modeset_unlock_all(dev);
> > -
> > -		if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
> > -			drm_kms_helper_hotplug_event(dev);
> > +	} else {
> > +		goto out;
> >  	}
> > +
> > +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
> > +	dm_restore_drm_connector_state(dev, connector);
> > +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> 
> Check ret here please

This function ("handle_hpd_irq") returns void. What would the appropriate way of
checking the error be?


> > @@ -2841,14 +2838,17 @@ static void handle_hpd_rx_irq(void *param)
> >  
> >  			amdgpu_dm_update_connector_after_detect(aconnector);
> >  
> > +		} else {
> > +			goto finish;
> 
> You used 'out' above and 'finish' here. It would be nice to be consistent with
> naming, I see 'out' a lot more than 'finish', so my vote would be to change this
> label to 'out'.

I originally used "out", but turns out there is already an "out" label in this
function :)

I then searched for other "go to end" labels and found "finish" being used in 
this same file.

But I can rename it to somehitng else ("out2" maybe?) to make it less confusing.

 
> > +		}
> >  
> > -			drm_modeset_lock_all(dev);
> > -			dm_restore_drm_connector_state(dev, connector);
> > -			drm_modeset_unlock_all(dev);
> > +		DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
> > +		dm_restore_drm_connector_state(dev, connector);
> > +		DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> 
> Check ret here?

This is another irq-like function returning void.

What can we do here after having checked the error?


> > +#include <drm/drm_drv.h>
> 
> Top-level headers generally come above the driver headers. Also, now that I think
> about this a bit more, all of the new includes in this set should probably be
> for 'drm_modeset_lock.h' instead of 'drm_drv.h'.

Ok. Let me try that.


> > @@ -1259,13 +1257,16 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
> > +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
> > +	dm_restore_drm_connector_state(dev, connector);
> > +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> 
> Check ret here?

This is a .write file_operations function expected to return a "size". Is it ok
for it to return an error? I guess so, right?


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 15/15] doc: drm: remove TODO entry regarding DRM_MODSET_LOCK_ALL cleanup
  2021-09-17 15:56   ` Sean Paul
@ 2021-09-17 23:21     ` Fernando Ramos
  0 siblings, 0 replies; 45+ messages in thread
From: Fernando Ramos @ 2021-09-17 23:21 UTC (permalink / raw)
  To: Sean Paul
  Cc: dri-devel, linux-kernel, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

> Can we remove drm_modeset_lock_all[_ctx] now? If so, let's queue that up as part
> of the set.
> 

drm_modeset_lock_all() and drm_modeset_unlock_all() can be removed (I'll do that
on v2 of this patch series).

drm_modset_lock_all_ctx() is a different story and there are still two places
(one in the i915 driver and another one in the amd driver) where they are
needed.

I would need to understand the code better before trying to remove those :)

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 14/15] drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-17 23:17     ` Fernando Ramos
@ 2021-09-18  9:42       ` Fernando Ramos
  0 siblings, 0 replies; 45+ messages in thread
From: Fernando Ramos @ 2021-09-18  9:42 UTC (permalink / raw)
  To: Sean Paul
  Cc: dri-devel, linux-kernel, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc,
	linux-tegra

On 21/09/18 01:17AM, Fernando Ramos wrote:

> > > +#include <drm/drm_drv.h>
> > 
> > Top-level headers generally come above the driver headers. Also, now that I think
> > about this a bit more, all of the new includes in this set should probably be
> > for 'drm_modeset_lock.h' instead of 'drm_drv.h'.
> 
> Ok. Let me try that.

Turns out that the DRM_MODESET_LOCK_ALL_*() macros expansion includes a call
to drm_drv_uses_atomic_modeset() which is defined in "drm_drv.h".

Thus, #include'ing <drm/drm_drv.h> cannot be avoided.

This makes me wonder...

  1. "drm_drv.h" includes "drm_device.h", which includes "drm_mode_config.h",
     which includes "drm_modeset_lock.h"

  2. "drm_modeset_lock.h" defines DRM_MODESET_LOCK_ALL_*() which expands into
     drm_drv_uses_atomic_modeset()

  3. drm_drv_uses_atomic_modeset() is declared in "drm_drv.h"

There seems to be a circular dependency here.

We can try to fix this, but I suggest to do it in a different patch series.


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 03/15] dmr/msm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-16 21:15 ` [Nouveau] [PATCH 03/15] dmr/msm: " Fernando Ramos
  2021-09-17 15:29   ` Sean Paul
@ 2021-09-20  1:54   ` kernel test robot
  2021-09-20 21:25     ` Fernando Ramos
  1 sibling, 1 reply; 45+ messages in thread
From: kernel test robot @ 2021-09-20  1:54 UTC (permalink / raw)
  To: Fernando Ramos, dri-devel
  Cc: kbuild-all, linux-kernel, sean, linux-doc, amd-gfx, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-renesas-soc

[-- Attachment #1: Type: text/plain, Size: 5970 bytes --]

Hi Fernando,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-exynos/exynos-drm-next]
[also build test ERROR on tegra-drm/drm/tegra/for-next linus/master v5.15-rc2 next-20210917]
[cannot apply to drm-intel/for-linux-next tegra/for-next drm-tip/drm-tip airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Fernando-Ramos/drm-cleanup-Use-DRM_MODESET_LOCK_ALL_-helpers-where-possible/20210917-051842
base:   https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/0da6630f04d8f86f9d3c9a65fe61a6c6d182c87f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Fernando-Ramos/drm-cleanup-Use-DRM_MODESET_LOCK_ALL_-helpers-where-possible/20210917-051842
        git checkout 0da6630f04d8f86f9d3c9a65fe61a6c6d182c87f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/drm/drm_crtc.h:36,
                    from include/drm/drm_atomic_helper.h:31,
                    from drivers/gpu/drm/msm/disp/msm_disp_snapshot.h:9,
                    from drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c:8:
   drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c: In function 'msm_disp_capture_atomic_state':
>> include/drm/drm_modeset_lock.h:167:14: error: implicit declaration of function 'drm_drv_uses_atomic_modeset' [-Werror=implicit-function-declaration]
     167 |         if (!drm_drv_uses_atomic_modeset(dev))                          \
         |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c:108:9: note: in expansion of macro 'DRM_MODESET_LOCK_ALL_BEGIN'
     108 |         DRM_MODESET_LOCK_ALL_BEGIN(ddev, ctx, 0, ret);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/drm_drv_uses_atomic_modeset +167 include/drm/drm_modeset_lock.h

a6a8bb848d5ca4 Daniel Vetter  2014-07-25  138  
06eaae46381737 Thierry Reding 2015-12-02  139  int drm_modeset_lock_all_ctx(struct drm_device *dev,
51fd371bbaf940 Rob Clark      2013-11-19  140  			     struct drm_modeset_acquire_ctx *ctx);
51fd371bbaf940 Rob Clark      2013-11-19  141  
b7ea04d299c78b Sean Paul      2018-11-29  142  /**
b7ea04d299c78b Sean Paul      2018-11-29  143   * DRM_MODESET_LOCK_ALL_BEGIN - Helper to acquire modeset locks
b7ea04d299c78b Sean Paul      2018-11-29  144   * @dev: drm device
b7ea04d299c78b Sean Paul      2018-11-29  145   * @ctx: local modeset acquire context, will be dereferenced
b7ea04d299c78b Sean Paul      2018-11-29  146   * @flags: DRM_MODESET_ACQUIRE_* flags to pass to drm_modeset_acquire_init()
b7ea04d299c78b Sean Paul      2018-11-29  147   * @ret: local ret/err/etc variable to track error status
b7ea04d299c78b Sean Paul      2018-11-29  148   *
b7ea04d299c78b Sean Paul      2018-11-29  149   * Use these macros to simplify grabbing all modeset locks using a local
b7ea04d299c78b Sean Paul      2018-11-29  150   * context. This has the advantage of reducing boilerplate, but also properly
b7ea04d299c78b Sean Paul      2018-11-29  151   * checking return values where appropriate.
b7ea04d299c78b Sean Paul      2018-11-29  152   *
b7ea04d299c78b Sean Paul      2018-11-29  153   * Any code run between BEGIN and END will be holding the modeset locks.
b7ea04d299c78b Sean Paul      2018-11-29  154   *
b7ea04d299c78b Sean Paul      2018-11-29  155   * This must be paired with DRM_MODESET_LOCK_ALL_END(). We will jump back and
b7ea04d299c78b Sean Paul      2018-11-29  156   * forth between the labels on deadlock and error conditions.
b7ea04d299c78b Sean Paul      2018-11-29  157   *
b7ea04d299c78b Sean Paul      2018-11-29  158   * Drivers can acquire additional modeset locks. If any lock acquisition
b7ea04d299c78b Sean Paul      2018-11-29  159   * fails, the control flow needs to jump to DRM_MODESET_LOCK_ALL_END() with
b7ea04d299c78b Sean Paul      2018-11-29  160   * the @ret parameter containing the return value of drm_modeset_lock().
b7ea04d299c78b Sean Paul      2018-11-29  161   *
b7ea04d299c78b Sean Paul      2018-11-29  162   * Returns:
b7ea04d299c78b Sean Paul      2018-11-29  163   * The only possible value of ret immediately after DRM_MODESET_LOCK_ALL_BEGIN()
b7ea04d299c78b Sean Paul      2018-11-29  164   * is 0, so no error checking is necessary
b7ea04d299c78b Sean Paul      2018-11-29  165   */
b7ea04d299c78b Sean Paul      2018-11-29  166  #define DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, flags, ret)		\
77ef38574beb3e Daniel Vetter  2020-08-14 @167  	if (!drm_drv_uses_atomic_modeset(dev))				\
77ef38574beb3e Daniel Vetter  2020-08-14  168  		mutex_lock(&dev->mode_config.mutex);			\
b7ea04d299c78b Sean Paul      2018-11-29  169  	drm_modeset_acquire_init(&ctx, flags);				\
b7ea04d299c78b Sean Paul      2018-11-29  170  modeset_lock_retry:							\
b7ea04d299c78b Sean Paul      2018-11-29  171  	ret = drm_modeset_lock_all_ctx(dev, &ctx);			\
b7ea04d299c78b Sean Paul      2018-11-29  172  	if (ret)							\
b7ea04d299c78b Sean Paul      2018-11-29  173  		goto modeset_lock_fail;
b7ea04d299c78b Sean Paul      2018-11-29  174  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54502 bytes --]

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Nouveau] [PATCH 03/15] dmr/msm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
  2021-09-20  1:54   ` kernel test robot
@ 2021-09-20 21:25     ` Fernando Ramos
  0 siblings, 0 replies; 45+ messages in thread
From: Fernando Ramos @ 2021-09-20 21:25 UTC (permalink / raw)
  To: kernel test robot
  Cc: dri-devel, kbuild-all, linux-kernel, sean, linux-doc, amd-gfx,
	intel-gfx, linux-arm-msm, freedreno, nouveau, linux-renesas-soc

On 21/09/20 09:54AM, kernel test robot wrote:
> 
> [auto build test ERROR on drm-exynos/exynos-drm-next]
> [also build test ERROR on tegra-drm/drm/tegra/for-next linus/master v5.15-rc2 next-20210917]

I forgot to #include <drm/drm_drv.h> for those platforms and didn't notice
because I only tried to build for X86. I'll fix it.


> [cannot apply to drm-intel/for-linux-next tegra/for-next drm-tip/drm-tip airlied/drm-next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base'.

I built this patch against drm-next, which currently points to v5.15-rc1.

Should I be targeting a different branch? In any case, as suggested, I'll
remember to use "--base" in the future to make it easier to apply. Thanks for
the hint.


> All errors (new ones prefixed by >>):
> 
>    In file included from include/drm/drm_crtc.h:36,
>                     from include/drm/drm_atomic_helper.h:31,
>                     from drivers/gpu/drm/msm/disp/msm_disp_snapshot.h:9,
>                     from drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c:8:
>    drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c: In function 'msm_disp_capture_atomic_state':
> >> include/drm/drm_modeset_lock.h:167:14: error: implicit declaration of function 'drm_drv_uses_atomic_modeset' [-Werror=implicit-function-declaration]
>      167 |         if (!drm_drv_uses_atomic_modeset(dev))                          \
>          |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c:108:9: note: in expansion of macro 'DRM_MODESET_LOCK_ALL_BEGIN'
>      108 |         DRM_MODESET_LOCK_ALL_BEGIN(ddev, ctx, 0, ret);
>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
>    cc1: some warnings being treated as errors

Out of curiosity: The top comment says there were two build errors (one on
exynos and another one on tegra), but there is only one reported bug (on msm).

Is this because the bot only reports the first error found? Is there a link to
a report with each of the build errors on each of the platforms?

Thanks.

^ permalink raw reply	[flat|nested] 45+ messages in thread

end of thread, other threads:[~2021-09-23 14:32 UTC | newest]

Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-16 21:15 [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Fernando Ramos
2021-09-16 21:15 ` [Nouveau] [PATCH 01/15] dmr: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN() Fernando Ramos
2021-09-17 15:28   ` Sean Paul
2021-09-17 22:07     ` Fernando Ramos
2021-09-16 21:15 ` [Nouveau] [PATCH 02/15] dmr/i915: " Fernando Ramos
2021-09-17 15:31   ` Sean Paul
2021-09-16 21:15 ` [Nouveau] [PATCH 03/15] dmr/msm: " Fernando Ramos
2021-09-17 15:29   ` Sean Paul
2021-09-20  1:54   ` kernel test robot
2021-09-20 21:25     ` Fernando Ramos
2021-09-16 21:15 ` [Nouveau] [PATCH 04/15] drm: cleanup: drm_modeset_lock_all() " Fernando Ramos
2021-09-17 15:35   ` Sean Paul
2021-09-16 21:15 ` [Nouveau] [PATCH 05/15] drm/vmwgfx: " Fernando Ramos
2021-09-17 15:37   ` Sean Paul
2021-09-16 21:15 ` [Nouveau] [PATCH 06/15] drm/tegra: " Fernando Ramos
2021-09-17 15:38   ` Sean Paul
2021-09-17 22:34     ` Fernando Ramos
2021-09-16 21:15 ` [Nouveau] [PATCH 07/15] drm/shmobile: " Fernando Ramos
2021-09-17 15:38   ` Sean Paul
2021-09-16 21:15 ` [Nouveau] [PATCH 08/15] drm/radeon: " Fernando Ramos
2021-09-17 15:40   ` Sean Paul
2021-09-17 22:32     ` Fernando Ramos
2021-09-16 21:15 ` [Nouveau] [PATCH 09/15] drm/omapdrm: " Fernando Ramos
2021-09-17 15:40   ` Sean Paul
2021-09-17 15:41   ` Sean Paul
2021-09-17 22:37     ` Fernando Ramos
2021-09-16 21:15 ` [Nouveau] [PATCH 10/15] drm/nouveau: " Fernando Ramos
2021-09-17 15:42   ` Sean Paul
2021-09-16 21:15 ` [Nouveau] [PATCH 11/15] drm/msm: " Fernando Ramos
2021-09-17 15:42   ` Sean Paul
2021-09-17 22:41     ` Fernando Ramos
2021-09-16 21:15 ` [Nouveau] [PATCH 12/15] drm/i915: " Fernando Ramos
2021-09-17 15:48   ` Sean Paul
2021-09-17 22:57     ` Fernando Ramos
2021-09-16 21:15 ` [Nouveau] [PATCH 13/15] drm/gma500: " Fernando Ramos
2021-09-17 15:49   ` Sean Paul
2021-09-16 21:15 ` [Nouveau] [PATCH 14/15] drm/amd: " Fernando Ramos
2021-09-17 15:55   ` Sean Paul
2021-09-17 23:17     ` Fernando Ramos
2021-09-18  9:42       ` Fernando Ramos
2021-09-16 21:15 ` [Nouveau] [PATCH 15/15] doc: drm: remove TODO entry regarding DRM_MODSET_LOCK_ALL cleanup Fernando Ramos
2021-09-17 15:56   ` Sean Paul
2021-09-17 23:21     ` Fernando Ramos
2021-09-17 15:24 ` [Nouveau] [PATCH 00/15] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible Daniel Vetter
2021-09-17 21:41   ` Fernando Ramos

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).