Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
From: Fernando Ramos <greenfoo@u92.eu>
To: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org, sean@poorly.run,
	linux-doc@vger.kernel.org, amd-gfx@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org, linux-arm-msm@vger.kernel.org,
	freedreno@lists.freedesktop.org, nouveau@lists.freedesktop.org,
	linux-renesas-soc@vger.kernel.org, linux-tegra@vger.kernel.org
Subject: [PATCH v4 18/20] drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() [part 3]
Date: Wed, 13 Oct 2021 22:48:44 +0200	[thread overview]
Message-ID: <20211013204846.90026-19-greenfoo@u92.eu> (raw)
In-Reply-To: <20211013204846.90026-1-greenfoo@u92.eu>

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()

NOTE:

While this change is similar to the one done two commits ago, it
contains an important extra nuances that I'm going to explain next.

The only difference between the old drm_modeset_{lock,unlock}_all()
functions and the new DRM_MODESET_LOCK_ALL_{BEGIN,END}() macros is that
the former use a global context stored in dev->mode_config.acquire_ctx
while the latter depend on a user provided one (typically in the stack).

This means that as long as no one accesses the global
dev->mode_config.acquire_ctx context in the block that runs between
lock/BEGIN and unlock/END, the code should be equivalent before and
after my changes.

Turns out that, while not obvious at first sight, the call to
dm_restore_drm_connector_state() done between drm_modset_lock_all() and
drm_modeset_unlock_all() ends up using that global context structure
stored in dev.

To fix this we need to update some function prototypes to accept the
new stack allocated variable as an argument.

Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 27 ++++++++++++-------
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  3 ++-
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 13 ++++++---
 3 files changed, 29 insertions(+), 14 deletions(-)

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 10ed1f8ad514..7a3c5def9fb9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -81,6 +81,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"
@@ -2906,6 +2907,8 @@ static void handle_hpd_irq_helper(struct amdgpu_dm_connector *aconnector)
 	struct amdgpu_device *adev = drm_to_adev(dev);
 	struct dm_connector_state *dm_con_state = to_dm_connector_state(connector->state);
 	struct dm_crtc_state *dm_crtc_state = NULL;
+	struct drm_modeset_acquire_ctx ctx;
+	int ret;
 
 	if (adev->dm.disable_hpd_irq)
 		return;
@@ -2947,9 +2950,9 @@ static void handle_hpd_irq_helper(struct amdgpu_dm_connector *aconnector)
 		goto 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, &ctx);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 	if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
 		drm_kms_helper_hotplug_event(dev);
@@ -3070,6 +3073,7 @@ 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;
@@ -3079,6 +3083,7 @@ static void handle_hpd_rx_irq(void *param)
 	bool has_left_work = false;
 	int idx = aconnector->base.index;
 	struct hpd_rx_irq_offload_work_queue *offload_wq = &adev->dm.hpd_rx_offload_wq[idx];
+	int ret;
 
 	memset(&hpd_irq_data, 0, sizeof(hpd_irq_data));
 
@@ -3153,9 +3158,9 @@ static void handle_hpd_rx_irq(void *param)
 			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, &ctx);
+		DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
 		drm_kms_helper_hotplug_event(dev);
 	}
@@ -9703,7 +9708,8 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
 }
 
 
-static int dm_force_atomic_commit(struct drm_connector *connector)
+static int dm_force_atomic_commit(struct drm_connector *connector,
+				  struct drm_modeset_acquire_ctx *ctx)
 {
 	int ret = 0;
 	struct drm_device *ddev = connector->dev;
@@ -9717,7 +9723,7 @@ static int dm_force_atomic_commit(struct drm_connector *connector)
 	if (!state)
 		return -ENOMEM;
 
-	state->acquire_ctx = ddev->mode_config.acquire_ctx;
+	state->acquire_ctx = ctx;
 
 	/* Construct an atomic state to restore previous display setting */
 
@@ -9764,7 +9770,8 @@ static int dm_force_atomic_commit(struct drm_connector *connector)
  * same port and when running without usermode desktop manager supprot
  */
 void dm_restore_drm_connector_state(struct drm_device *dev,
-				    struct drm_connector *connector)
+				    struct drm_connector *connector,
+				    struct drm_modeset_acquire_ctx *ctx)
 {
 	struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector);
 	struct amdgpu_crtc *disconnected_acrtc;
@@ -9787,7 +9794,7 @@ void dm_restore_drm_connector_state(struct drm_device *dev,
 	 * to turn on the display, so we do it here
 	 */
 	if (acrtc_state->stream->sink != aconnector->dc_sink)
-		dm_force_atomic_commit(&aconnector->base);
+		dm_force_atomic_commit(&aconnector->base, ctx);
 }
 
 /*
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 37e61a88d49e..1a7c8fbf149a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -706,7 +706,8 @@ enum drm_mode_status amdgpu_dm_connector_mode_valid(struct drm_connector *connec
 				   struct drm_display_mode *mode);
 
 void dm_restore_drm_connector_state(struct drm_device *dev,
-				    struct drm_connector *connector);
+				    struct drm_connector *connector,
+				    struct drm_modeset_acquire_ctx *ctx);
 
 void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
 					struct edid *edid);
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 7751038d5788..f4542760b006 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
@@ -24,6 +24,7 @@
  */
 
 #include <linux/uaccess.h>
+#include <drm/drm_drv.h>
 
 #include "dc.h"
 #include "amdgpu.h"
@@ -1192,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;
@@ -1258,9 +1261,13 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
 		goto unlock;
 	}
 
-	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, &ctx);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+	if (ret)
+		return ret;
+
 	drm_kms_helper_hotplug_event(dev);
 
 unlock:
-- 
2.33.0


  parent reply	other threads:[~2021-10-13 20:50 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-13 20:48 [PATCH v4 00/20] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 01/20] drm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN() Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 02/20] drm/i915: " Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 03/20] drm/msm: " Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 04/20] drm: cleanup: drm_modeset_lock_all() " Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 05/20] drm/vmwgfx: " Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 06/20] drm/tegra: " Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 07/20] drm/shmobile: " Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 08/20] drm/radeon: " Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 09/20] drm/omapdrm: " Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 10/20] drm/nouveau: " Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 11/20] drm/msm: " Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 12/20] drm/i915: " Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 13/20] drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() [part 2] Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 14/20] drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() [part 3] Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 15/20] drm/gma500: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 16/20] drm/amd: " Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 17/20] drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() [part 2] Fernando Ramos
2021-10-13 20:48 ` Fernando Ramos [this message]
2021-10-13 20:48 ` [PATCH v4 19/20] drm: cleanup: remove drm_modeset_(un)lock_all() Fernando Ramos
2021-10-13 20:48 ` [PATCH v4 20/20] drm: cleanup: remove acquire_ctx from drm_mode_config Fernando Ramos

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211013204846.90026-19-greenfoo@u92.eu \
    --to=greenfoo@u92.eu \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=nouveau@lists.freedesktop.org \
    --cc=sean@poorly.run \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox