drm/ast: Acquire I/O-register lock in atomic_commit_tail function
authorThomas Zimmermann <tzimmermann@suse.de>
Thu, 13 Oct 2022 11:29:16 +0000 (13:29 +0200)
committerThomas Zimmermann <tzimmermann@suse.de>
Sun, 16 Oct 2022 09:15:31 +0000 (11:15 +0200)
Hold I/O-register lock in atomic_commit_tail to protect all pipeline
updates at once. Protects modesetting against concurrent EDID reads.

Complex modesetting operations involve mode changes and plane updates.
These steps used to be protected individually against concurrent I/O.
Make all this atomic wrt to reading display modes via EDID. The EDID
code in the connector's get_modes helper already acquires the necessary
lock.

A similar issue was fixed in commit 2d70b9a1482e ("drm/mgag200: Acquire
I/O-register lock in atomic_commit_tail function") for mgag200.

v2:
* fix typo in commit message (Jocelyn)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221013112923.769-2-tzimmermann@suse.de
drivers/gpu/drm/ast/ast_mode.c

index d5ee3ad538a8c5e671f985112288e4746892487e..e1e07928906e7a366c5da74f6f2699b606eeb198 100644 (file)
@@ -1200,20 +1200,6 @@ out:
        return drm_atomic_add_affected_planes(state, crtc);
 }
 
-static void ast_crtc_helper_atomic_begin(struct drm_crtc *crtc, struct drm_atomic_state *state)
-{
-       struct drm_device *dev = crtc->dev;
-       struct ast_private *ast = to_ast_private(dev);
-
-       /*
-        * Concurrent operations could possibly trigger a call to
-        * drm_connector_helper_funcs.get_modes by trying to read the
-        * display modes. Protect access to I/O registers by acquiring
-        * the I/O-register lock. Released in atomic_flush().
-        */
-       mutex_lock(&ast->ioregs_lock);
-}
-
 static void
 ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
                             struct drm_atomic_state *state)
@@ -1241,8 +1227,6 @@ ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
        //Set Aspeed Display-Port
        if (ast->tx_chip_types & AST_TX_ASTDP_BIT)
                ast_dp_set_mode(crtc, vbios_mode_info);
-
-       mutex_unlock(&ast->ioregs_lock);
 }
 
 static void
@@ -1301,7 +1285,6 @@ ast_crtc_helper_atomic_disable(struct drm_crtc *crtc,
 static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
        .mode_valid = ast_crtc_helper_mode_valid,
        .atomic_check = ast_crtc_helper_atomic_check,
-       .atomic_begin = ast_crtc_helper_atomic_begin,
        .atomic_flush = ast_crtc_helper_atomic_flush,
        .atomic_enable = ast_crtc_helper_atomic_enable,
        .atomic_disable = ast_crtc_helper_atomic_disable,
@@ -1771,8 +1754,23 @@ static int ast_astdp_output_init(struct ast_private *ast)
  * Mode config
  */
 
+static void ast_mode_config_helper_atomic_commit_tail(struct drm_atomic_state *state)
+{
+       struct ast_private *ast = to_ast_private(state->dev);
+
+       /*
+        * Concurrent operations could possibly trigger a call to
+        * drm_connector_helper_funcs.get_modes by trying to read the
+        * display modes. Protect access to I/O registers by acquiring
+        * the I/O-register lock. Released in atomic_flush().
+        */
+       mutex_lock(&ast->ioregs_lock);
+       drm_atomic_helper_commit_tail_rpm(state);
+       mutex_unlock(&ast->ioregs_lock);
+}
+
 static const struct drm_mode_config_helper_funcs ast_mode_config_helper_funcs = {
-       .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
+       .atomic_commit_tail = ast_mode_config_helper_atomic_commit_tail,
 };
 
 static const struct drm_mode_config_funcs ast_mode_config_funcs = {