* [PATCH 0/3] drm/{ast, mgag200}: Protect I/O regs against concurrent access
@ 2022-05-02 14:25 Thomas Zimmermann
2022-05-02 14:25 ` [PATCH 1/3] drm: Add DRM-managed mutex_init() Thomas Zimmermann
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Thomas Zimmermann @ 2022-05-02 14:25 UTC (permalink / raw)
To: airlied, airlied, daniel, maarten.lankhorst, mripard
Cc: Thomas Zimmermann, dri-devel
Protect access to I/O registers in ast and mgag200 via lock. Commit-
tail functions and get-modes operations use the same registers and can
interfere with each other. This can result in failed mode-setting
operations.
As both drivers use fully managed cleanup, the patchset adds a new helper
that initializes a mutex with auto-cleanup.
Thomas Zimmermann (3):
drm: Add DRM-managed mutex_init()
drm/ast: Protect concurrent access to I/O registers with lock
drm/mgag200: Protect concurrent access to I/O registers with lock
drivers/gpu/drm/ast/ast_drv.h | 1 +
drivers/gpu/drm/ast/ast_main.c | 4 +++
drivers/gpu/drm/ast/ast_mode.c | 48 ++++++++++++++++++++++++--
drivers/gpu/drm/drm_managed.c | 27 +++++++++++++++
drivers/gpu/drm/mgag200/mgag200_drv.c | 6 ++++
drivers/gpu/drm/mgag200/mgag200_drv.h | 1 +
drivers/gpu/drm/mgag200/mgag200_mode.c | 14 ++++++++
include/drm/drm_managed.h | 3 ++
8 files changed, 101 insertions(+), 3 deletions(-)
base-commit: 3ae2e00290c290713e21118220a817a24b44d39f
prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
--
2.36.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] drm: Add DRM-managed mutex_init()
2022-05-02 14:25 [PATCH 0/3] drm/{ast, mgag200}: Protect I/O regs against concurrent access Thomas Zimmermann
@ 2022-05-02 14:25 ` Thomas Zimmermann
2022-05-04 9:50 ` Daniel Vetter
2022-05-02 14:25 ` [PATCH 2/3] drm/ast: Protect concurrent access to I/O registers with lock Thomas Zimmermann
2022-05-02 14:25 ` [PATCH 3/3] drm/mgag200: " Thomas Zimmermann
2 siblings, 1 reply; 6+ messages in thread
From: Thomas Zimmermann @ 2022-05-02 14:25 UTC (permalink / raw)
To: airlied, airlied, daniel, maarten.lankhorst, mripard
Cc: Thomas Zimmermann, dri-devel
Add drmm_mutex_init(), a helper that provides managed mutex cleanup. The
mutex will be destroyed with the final reference of the DRM device.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_managed.c | 27 +++++++++++++++++++++++++++
include/drm/drm_managed.h | 3 +++
2 files changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
index 37d7db6223be6..4cf214de50c40 100644
--- a/drivers/gpu/drm/drm_managed.c
+++ b/drivers/gpu/drm/drm_managed.c
@@ -8,6 +8,7 @@
#include <drm/drm_managed.h>
#include <linux/list.h>
+#include <linux/mutex.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
@@ -262,3 +263,29 @@ void drmm_kfree(struct drm_device *dev, void *data)
free_dr(dr_match);
}
EXPORT_SYMBOL(drmm_kfree);
+
+static void drmm_mutex_release(struct drm_device *dev, void *res)
+{
+ struct mutex *lock = res;
+
+ mutex_destroy(lock);
+}
+
+/**
+ * drmm_mutex_init - &drm_device-managed mutex_init()
+ * @dev: DRM device
+ * @lock: lock to be initialized
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ *
+ * This is a &drm_device-managed version of mutex_init(). The initialized
+ * lock is automatically destroyed on the final drm_dev_put().
+ */
+int drmm_mutex_init(struct drm_device *dev, struct mutex *lock)
+{
+ mutex_init(lock);
+
+ return drmm_add_action_or_reset(dev, drmm_mutex_release, lock);
+}
+EXPORT_SYMBOL(drmm_mutex_init);
diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
index b45c6fbf53ac4..359883942612e 100644
--- a/include/drm/drm_managed.h
+++ b/include/drm/drm_managed.h
@@ -8,6 +8,7 @@
#include <linux/types.h>
struct drm_device;
+struct mutex;
typedef void (*drmres_release_t)(struct drm_device *dev, void *res);
@@ -104,4 +105,6 @@ char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp);
void drmm_kfree(struct drm_device *dev, void *data);
+int drmm_mutex_init(struct drm_device *dev, struct mutex *lock);
+
#endif
--
2.36.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] drm/ast: Protect concurrent access to I/O registers with lock
2022-05-02 14:25 [PATCH 0/3] drm/{ast, mgag200}: Protect I/O regs against concurrent access Thomas Zimmermann
2022-05-02 14:25 ` [PATCH 1/3] drm: Add DRM-managed mutex_init() Thomas Zimmermann
@ 2022-05-02 14:25 ` Thomas Zimmermann
2022-05-02 14:25 ` [PATCH 3/3] drm/mgag200: " Thomas Zimmermann
2 siblings, 0 replies; 6+ messages in thread
From: Thomas Zimmermann @ 2022-05-02 14:25 UTC (permalink / raw)
To: airlied, airlied, daniel, maarten.lankhorst, mripard
Cc: Thomas Zimmermann, dri-devel
Add a mutex lock to protect concurrent access to I/O registers
against each other. This happens between invokataion of commit-
tail functions and get-mode operations. Both with use the CRTC
index register AST_IO_CRTC_PORT. Concurrent access can lead to
failed mode-setting operations.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reported-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
---
drivers/gpu/drm/ast/ast_drv.h | 1 +
drivers/gpu/drm/ast/ast_main.c | 4 +++
drivers/gpu/drm/ast/ast_mode.c | 48 +++++++++++++++++++++++++++++++---
3 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index a19315b2f7e56..43cedd767f8f1 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -158,6 +158,7 @@ to_ast_sil164_connector(struct drm_connector *connector)
struct ast_private {
struct drm_device base;
+ struct mutex ioregs_lock; /* Protects access to I/O registers in ioregs */
void __iomem *regs;
void __iomem *ioregs;
void __iomem *dp501_fw_buf;
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 22e9e2d3c71ab..5ad473aeaea2b 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -420,6 +420,10 @@ struct ast_private *ast_device_create(const struct drm_driver *drv,
pci_set_drvdata(pdev, dev);
+ ret = drmm_mutex_init(dev, &ast->ioregs_lock);
+ if (ret)
+ return ERR_PTR(ret);
+
ast->regs = pcim_iomap(pdev, 1, 0);
if (!ast->regs)
return ERR_PTR(-EIO);
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 45b56b39ad473..f7849638fcb7e 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1099,6 +1099,20 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
return 0;
}
+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)
@@ -1107,7 +1121,8 @@ ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
crtc);
struct drm_crtc_state *old_crtc_state = drm_atomic_get_old_crtc_state(state,
crtc);
- struct ast_private *ast = to_ast_private(crtc->dev);
+ struct drm_device *dev = crtc->dev;
+ struct ast_private *ast = to_ast_private(dev);
struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc_state);
struct ast_crtc_state *old_ast_crtc_state = to_ast_crtc_state(old_crtc_state);
@@ -1117,6 +1132,8 @@ ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
*/
if (old_ast_crtc_state->format != ast_crtc_state->format)
ast_crtc_load_lut(ast, crtc);
+
+ mutex_unlock(&ast->ioregs_lock);
}
static void
@@ -1175,6 +1192,7 @@ 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,
@@ -1260,21 +1278,33 @@ static int ast_crtc_init(struct drm_device *dev)
static int ast_vga_connector_helper_get_modes(struct drm_connector *connector)
{
struct ast_vga_connector *ast_vga_connector = to_ast_vga_connector(connector);
+ struct drm_device *dev = connector->dev;
+ struct ast_private *ast = to_ast_private(dev);
struct edid *edid;
int count;
if (!ast_vga_connector->i2c)
goto err_drm_connector_update_edid_property;
+ /*
+ * Protect access to I/O registers from concurrent modesetting
+ * by acquiring the I/O-register lock.
+ */
+ mutex_lock(&ast->ioregs_lock);
+
edid = drm_get_edid(connector, &ast_vga_connector->i2c->adapter);
if (!edid)
- goto err_drm_connector_update_edid_property;
+ goto err_mutex_unlock;
+
+ mutex_unlock(&ast->ioregs_lock);
count = drm_add_edid_modes(connector, edid);
kfree(edid);
return count;
+err_mutex_unlock:
+ mutex_unlock(&ast->ioregs_lock);
err_drm_connector_update_edid_property:
drm_connector_update_edid_property(connector, NULL);
return 0;
@@ -1354,21 +1384,33 @@ static int ast_vga_output_init(struct ast_private *ast)
static int ast_sil164_connector_helper_get_modes(struct drm_connector *connector)
{
struct ast_sil164_connector *ast_sil164_connector = to_ast_sil164_connector(connector);
+ struct drm_device *dev = connector->dev;
+ struct ast_private *ast = to_ast_private(dev);
struct edid *edid;
int count;
if (!ast_sil164_connector->i2c)
goto err_drm_connector_update_edid_property;
+ /*
+ * Protect access to I/O registers from concurrent modesetting
+ * by acquiring the I/O-register lock.
+ */
+ mutex_lock(&ast->ioregs_lock);
+
edid = drm_get_edid(connector, &ast_sil164_connector->i2c->adapter);
if (!edid)
- goto err_drm_connector_update_edid_property;
+ goto err_mutex_unlock;
+
+ mutex_unlock(&ast->ioregs_lock);
count = drm_add_edid_modes(connector, edid);
kfree(edid);
return count;
+err_mutex_unlock:
+ mutex_unlock(&ast->ioregs_lock);
err_drm_connector_update_edid_property:
drm_connector_update_edid_property(connector, NULL);
return 0;
--
2.36.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] drm/mgag200: Protect concurrent access to I/O registers with lock
2022-05-02 14:25 [PATCH 0/3] drm/{ast, mgag200}: Protect I/O regs against concurrent access Thomas Zimmermann
2022-05-02 14:25 ` [PATCH 1/3] drm: Add DRM-managed mutex_init() Thomas Zimmermann
2022-05-02 14:25 ` [PATCH 2/3] drm/ast: Protect concurrent access to I/O registers with lock Thomas Zimmermann
@ 2022-05-02 14:25 ` Thomas Zimmermann
2022-05-04 12:31 ` Jocelyn Falempe
2 siblings, 1 reply; 6+ messages in thread
From: Thomas Zimmermann @ 2022-05-02 14:25 UTC (permalink / raw)
To: airlied, airlied, daniel, maarten.lankhorst, mripard
Cc: Thomas Zimmermann, dri-devel
Add a mutex lock to protect concurrent access to I/O registers
against each other. This happens between invokataion 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.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
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 --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 217844d71ab5c..08839460606fe 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_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 4368112023f7c..18829eb8398a0 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;
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 6e18d3bbd7207..abde7655477db 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 *
--
2.36.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] drm: Add DRM-managed mutex_init()
2022-05-02 14:25 ` [PATCH 1/3] drm: Add DRM-managed mutex_init() Thomas Zimmermann
@ 2022-05-04 9:50 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2022-05-04 9:50 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: airlied, dri-devel, airlied
On Mon, May 02, 2022 at 04:25:12PM +0200, Thomas Zimmermann wrote:
> Add drmm_mutex_init(), a helper that provides managed mutex cleanup. The
> mutex will be destroyed with the final reference of the DRM device.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> on this one, ack for
the other two driver patches.
-Daniel
> ---
> drivers/gpu/drm/drm_managed.c | 27 +++++++++++++++++++++++++++
> include/drm/drm_managed.h | 3 +++
> 2 files changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
> index 37d7db6223be6..4cf214de50c40 100644
> --- a/drivers/gpu/drm/drm_managed.c
> +++ b/drivers/gpu/drm/drm_managed.c
> @@ -8,6 +8,7 @@
> #include <drm/drm_managed.h>
>
> #include <linux/list.h>
> +#include <linux/mutex.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
>
> @@ -262,3 +263,29 @@ void drmm_kfree(struct drm_device *dev, void *data)
> free_dr(dr_match);
> }
> EXPORT_SYMBOL(drmm_kfree);
> +
> +static void drmm_mutex_release(struct drm_device *dev, void *res)
> +{
> + struct mutex *lock = res;
> +
> + mutex_destroy(lock);
> +}
> +
> +/**
> + * drmm_mutex_init - &drm_device-managed mutex_init()
> + * @dev: DRM device
> + * @lock: lock to be initialized
> + *
> + * Returns:
> + * 0 on success, or a negative errno code otherwise.
> + *
> + * This is a &drm_device-managed version of mutex_init(). The initialized
> + * lock is automatically destroyed on the final drm_dev_put().
> + */
> +int drmm_mutex_init(struct drm_device *dev, struct mutex *lock)
> +{
> + mutex_init(lock);
> +
> + return drmm_add_action_or_reset(dev, drmm_mutex_release, lock);
> +}
> +EXPORT_SYMBOL(drmm_mutex_init);
> diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
> index b45c6fbf53ac4..359883942612e 100644
> --- a/include/drm/drm_managed.h
> +++ b/include/drm/drm_managed.h
> @@ -8,6 +8,7 @@
> #include <linux/types.h>
>
> struct drm_device;
> +struct mutex;
>
> typedef void (*drmres_release_t)(struct drm_device *dev, void *res);
>
> @@ -104,4 +105,6 @@ char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp);
>
> void drmm_kfree(struct drm_device *dev, void *data);
>
> +int drmm_mutex_init(struct drm_device *dev, struct mutex *lock);
> +
> #endif
> --
> 2.36.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] drm/mgag200: Protect concurrent access to I/O registers with lock
2022-05-02 14:25 ` [PATCH 3/3] drm/mgag200: " Thomas Zimmermann
@ 2022-05-04 12:31 ` Jocelyn Falempe
0 siblings, 0 replies; 6+ messages in thread
From: Jocelyn Falempe @ 2022-05-04 12:31 UTC (permalink / raw)
To: Thomas Zimmermann, airlied, airlied, daniel, maarten.lankhorst, mripard
Cc: dri-devel
On 02/05/2022 16:25, Thomas Zimmermann wrote:
> Add a mutex lock to protect concurrent access to I/O registers
> against each other. This happens between invokataion of commit-
Typo in commit message, invokataion => invocation
> 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.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
There is a trivial conflict with
https://lists.freedesktop.org/archives/dri-devel/2022-April/352966.html
But I will need to send a v2 for it anyway.
It looks good to me,
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.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 --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index 217844d71ab5c..08839460606fe 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_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 4368112023f7c..18829eb8398a0 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;
> 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 6e18d3bbd7207..abde7655477db 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 *
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-04 12:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-02 14:25 [PATCH 0/3] drm/{ast, mgag200}: Protect I/O regs against concurrent access Thomas Zimmermann
2022-05-02 14:25 ` [PATCH 1/3] drm: Add DRM-managed mutex_init() Thomas Zimmermann
2022-05-04 9:50 ` Daniel Vetter
2022-05-02 14:25 ` [PATCH 2/3] drm/ast: Protect concurrent access to I/O registers with lock Thomas Zimmermann
2022-05-02 14:25 ` [PATCH 3/3] drm/mgag200: " Thomas Zimmermann
2022-05-04 12:31 ` Jocelyn Falempe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.