From: Thomas Zimmermann Date: Mon, 2 May 2022 14:25:13 +0000 (+0200) Subject: drm/ast: Protect concurrent access to I/O registers with lock X-Git-Url: http://git.maquefel.me/?a=commitdiff_plain;h=f870231fdd6ce2c6c040a3584a2eac97ae473780;p=linux.git drm/ast: 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 register AST_IO_CRTC_PORT. Concurrent access can lead to failed mode-setting operations. v2: * fix typo in commit description (Jocelyn) Signed-off-by: Thomas Zimmermann Reviewed-by: Jocelyn Falempe Acked-by: Daniel Vetter Reported-by: KuoHsiang Chou Link: https://patchwork.freedesktop.org/patch/msgid/20220502142514.2174-3-tzimmermann@suse.de --- diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index ff1ae314bd0dc..afebe35f205e4 100644 --- a/drivers/gpu/drm/ast/ast_drv.h +++ b/drivers/gpu/drm/ast/ast_drv.h @@ -159,6 +159,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 1113ee1cb7d46..d770d5a23c1af 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -421,6 +421,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 4728825b7b679..323af2746aa92 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -1119,6 +1119,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) @@ -1127,7 +1141,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); struct ast_vbios_mode_info *vbios_mode_info = &ast_crtc_state->vbios_mode_info; @@ -1142,6 +1157,8 @@ ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, //Set Aspeed Display-Port if (ast->tx_chip_type == AST_TX_ASTDP) ast_dp_set_mode(crtc, vbios_mode_info); + + mutex_unlock(&ast->ioregs_lock); } static void @@ -1200,6 +1217,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, @@ -1285,21 +1303,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; @@ -1379,21 +1409,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;