diff mbox series

[2/6,SRU,OEM-5.17] drm/mgag200: Protect concurrent access to I/O registers with lock

Message ID 20221108042634.24176-3-acelan.kao@canonical.com
State New
Headers show
Series Screen cannot turn on after screen off with Matrox G200eW3 [102b:0536] | expand

Commit Message

AceLan Kao Nov. 8, 2022, 4:26 a.m. UTC 
From: Thomas Zimmermann <tzimmermann@suse.de>

BugLink: https://launchpad.net/bugs/1995573

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
(backported from commit 931e3f3a0e997c41eafbc88e4fc07ba9fef28f29)
(Fix the cherry pick failure due to header files conflict)
Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
---
 drivers/gpu/drm/mgag200/mgag200_drv.c  |  6 ++++++
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  1 +
 drivers/gpu/drm/mgag200/mgag200_mode.c | 14 ++++++++++++++
 3 files changed, 21 insertions(+)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 740108a006ba..c463d27562c3 100644
--- 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_pciids.h>
 
 #include "mgag200_drv.h"
@@ -64,6 +65,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 4368112023f7..a18384c41fc4 100644
--- 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 cd9ba13ad5fc..92d37bb125ab 100644
--- 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 *