drm/mgag200: Protect concurrent access to I/O registers with lock

Add a mutex lock to protect concurrent access to I/O registers
against each other. This happens between invocation of commit-
tail functions and get-mode operations. Both with use the CRTC
index registers MGA1064_GEN_IO_DATA and MGA1064_GEN_IO_CTL.
Concurrent access can lead to failed mode-setting operations.

v2:
	* fix typo in commit description (Jocelyn)
	* add comment to explain rmmio_lock

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20220502142514.2174-4-tzimmermann@suse.de

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 217844d71ab5c444bbc0faba6e07d12b1c2ff92f..08839460606fee4acf7f5d17e50321d60e9f03a8 100644 (file)
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -14,6 +14,7 @@
 #include <drm/drm_drv.h>
 #include <drm/drm_file.h>
 #include <drm/drm_ioctl.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_module.h>
 #include <drm/drm_pciids.h>
 
@@ -65,6 +66,11 @@ static int mgag200_regs_init(struct mga_device *mdev)
        struct pci_dev *pdev = to_pci_dev(dev->dev);
        u32 option, option2;
        u8 crtcext3;
+       int ret;
+
+       ret = drmm_mutex_init(dev, &mdev->rmmio_lock);
+       if (ret)
+               return ret;
 
        switch (mdev->type) {
        case G200_PCI:

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 4368112023f7c7d5bc551fd8e76fe7ffa6435825..a18384c41fc4fd10b8dc6a55546afd67c360d08e 100644 (file)
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -213,6 +213,7 @@ struct mga_device {
        struct drm_device               base;
        unsigned long                   flags;
 
+       struct mutex                    rmmio_lock; /* Protects access to rmmio */
        resource_size_t                 rmmio_base;
        resource_size_t                 rmmio_size;
        void __iomem                    *rmmio;

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 6e18d3bbd7207d9b0e7965cc5002a1161dc3e735..abde7655477dba19bc33dccfd395c3f0949fa547 100644 (file)
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -881,6 +881,14 @@ mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
                .y2 = fb->height,
        };
 
+       /*
+        * 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.
+        */
+       mutex_lock(&mdev->rmmio_lock);
+
        if (mdev->type == G200_WB || mdev->type == G200_EW3)
                mgag200_g200wb_hold_bmc(mdev);
 
@@ -904,6 +912,8 @@ mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
        mgag200_enable_display(mdev);
 
        mgag200_handle_damage(mdev, fb, &fullscreen, &shadow_plane_state->data[0]);
+
+       mutex_unlock(&mdev->rmmio_lock);
 }
 
 static void
@@ -963,8 +973,12 @@ mgag200_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
        if (!fb)
                return;
 
+       mutex_lock(&mdev->rmmio_lock);
+
        if (drm_atomic_helper_damage_merged(old_state, state, &damage))
                mgag200_handle_damage(mdev, fb, &damage, &shadow_plane_state->data[0]);
+
+       mutex_unlock(&mdev->rmmio_lock);
 }
 
 static struct drm_crtc_state *