* [PATCH v2] drm: Reverse lock order in DRM display-panning helpers()
@ 2019-06-11 14:31 Thomas Zimmermann
2019-06-11 15:46 ` Daniel Vetter
0 siblings, 1 reply; 2+ messages in thread
From: Thomas Zimmermann @ 2019-06-11 14:31 UTC (permalink / raw)
To: noralf, sam, daniel; +Cc: Thomas Zimmermann, dri-devel
During display panning, acquiring drm_client_dev.modeset_mutex after the
locks in drm_fb_helper.dev creates a deadlock with drm_setup_crtcs() as
shown below:
[ 4.959319] fbcon: radeondrmfb (fb0) is primary device
[ 4.993952] Console: switching to colour frame buffer device 240x67
[ 4.994040]
[ 4.994041] ======================================================
[ 4.994041] WARNING: possible circular locking dependency detected
[ 4.994042] 5.2.0-rc4-1-default+ #39 Tainted: G E
[ 4.994043] ------------------------------------------------------
[ 4.994043] systemd-udevd/369 is trying to acquire lock:
[ 4.994044] 00000000fb622acb (&client->modeset_mutex){+.+.}, at: drm_fb_helper_pan_display+0x103/0x1f0 [drm_kms_helper]
[ 4.994055]
[ 4.994055] but task is already holding lock:
[ 4.994055] 0000000028767ae4 (crtc_ww_class_mutex){+.+.}, at: drm_modeset_lock+0x42/0xf0 [drm]
[ 4.994072]
[ 4.994072] which lock already depends on the new lock.
[ 4.994072]
[ 4.994072]
[ 4.994072] the existing dependency chain (in reverse order) is:
[ 4.994073]
[ 4.994073] -> #3 (crtc_ww_class_mutex){+.+.}:
[ 4.994076] lock_acquire+0x9e/0x170
[ 4.994079] __ww_mutex_lock.constprop.18+0x97/0xf40
[ 4.994080] ww_mutex_lock+0x30/0x90
[ 4.994091] drm_modeset_lock+0x42/0xf0 [drm]
[ 4.994102] drm_modeset_lock_all_ctx+0x1f/0xe0 [drm]
[ 4.994113] drm_modeset_lock_all+0x5e/0x1a0 [drm]
[ 4.994163] intel_modeset_init+0x60b/0xda0 [i915]
..
[ 4.994253]
[ 4.994253] -> #2 (crtc_ww_class_acquire){+.+.}:
[ 4.994255] lock_acquire+0x9e/0x170
[ 4.994270] drm_modeset_acquire_init+0xcc/0x100 [drm]
[ 4.994280] drm_modeset_lock_all+0x44/0x1a0 [drm]
[ 4.994320] intel_modeset_init+0x60b/0xda0 [i915]
..
[ 4.994403]
[ 4.994403] -> #1 (&dev->mode_config.mutex){+.+.}:
[ 4.994405] lock_acquire+0x9e/0x170
[ 4.994408] __mutex_lock+0x62/0x8c0
[ 4.994413] drm_setup_crtcs+0x17c/0xc50 [drm_kms_helper]
[ 4.994418] __drm_fb_helper_initial_config_and_unlock+0x34/0x530 [drm_kms_helper]
[ 4.994450] radeon_fbdev_init+0x110/0x130 [radeon]
..
[ 4.994535]
[ 4.994535] -> #0 (&client->modeset_mutex){+.+.}:
[ 4.994537] __lock_acquire+0xa85/0xe90
[ 4.994538] lock_acquire+0x9e/0x170
[ 4.994540] __mutex_lock+0x62/0x8c0
[ 4.994545] drm_fb_helper_pan_display+0x103/0x1f0 [drm_kms_helper]
[ 4.994547] fb_pan_display+0x92/0x120
[ 4.994549] bit_update_start+0x1a/0x40
[ 4.994550] fbcon_switch+0x392/0x580
[ 4.994552] redraw_screen+0x12c/0x220
[ 4.994553] do_bind_con_driver.cold.30+0xe1/0x10d
[ 4.994554] do_take_over_console+0x113/0x190
[ 4.994555] do_fbcon_takeover+0x58/0xb0
[ 4.994557] notifier_call_chain+0x47/0x70
[ 4.994558] blocking_notifier_call_chain+0x44/0x60
[ 4.994559] register_framebuffer+0x231/0x310
[ 4.994564] __drm_fb_helper_initial_config_and_unlock+0x2fd/0x530 [drm_kms_helper]
[ 4.994590] radeon_fbdev_init+0x110/0x130 [radeon]
..
This problem was introduced in
d81294afe drm/fb-helper: Remove drm_fb_helper_crtc
Reversing the lock ordering in the display-panning code fixes the issue. In
addition to the fix, the DRM client now implements the panning. The call to
drm_client_modeset_pan_display() acquires drm_client_dev.modeset_mutex once
for the whole operation.
The change was suggested by Daniel Vetter and Noralf Tronnes.
v2:
* move display panning to DRM client
* acquire modeset_mutex once in drm_client_modeset_pan_display()
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: d81294afeecdacc8d84804ba0bcb3d39e64d0f27
---
drivers/gpu/drm/drm_client_modeset.c | 110 +++++++++++++++++++++++++--
drivers/gpu/drm/drm_fb_helper.c | 79 ++-----------------
include/drm/drm_client.h | 4 +
3 files changed, 117 insertions(+), 76 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index b1984f901a6d..20c8fe94249e 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -288,6 +288,19 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
return ret;
}
+static int __drm_client_modeset_commit_force(struct drm_client_dev *client)
+{
+ struct drm_device *dev = client->dev;
+ int ret;
+
+ if (drm_drv_uses_atomic_modeset(dev))
+ ret = drm_client_modeset_commit_atomic(client, true);
+ else
+ ret = drm_client_modeset_commit_legacy(client);
+
+ return ret;
+}
+
/**
* drm_client_modeset_commit_force() - Force commit CRTC configuration
* @client: DRM client
@@ -299,14 +312,10 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
*/
int drm_client_modeset_commit_force(struct drm_client_dev *client)
{
- struct drm_device *dev = client->dev;
int ret;
mutex_lock(&client->modeset_mutex);
- if (drm_drv_uses_atomic_modeset(dev))
- ret = drm_client_modeset_commit_atomic(client, true);
- else
- ret = drm_client_modeset_commit_legacy(client);
+ ret = __drm_client_modeset_commit_force(client);
mutex_unlock(&client->modeset_mutex);
return ret;
@@ -390,3 +399,94 @@ int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
return ret;
}
EXPORT_SYMBOL(drm_client_modeset_dpms);
+
+static void pan_set(struct drm_client_dev *client, unsigned int x,
+ unsigned int y)
+{
+ struct drm_mode_set *mode_set;
+
+ drm_client_for_each_modeset(mode_set, client) {
+ mode_set->x = x;
+ mode_set->y = y;
+ }
+}
+
+static int drm_client_modeset_pan_display_atomic(struct drm_client_dev *client,
+ unsigned int x,
+ unsigned int y,
+ unsigned int default_x,
+ unsigned int default_y)
+{
+ int ret;
+
+ pan_set(client, x, y);
+
+ ret = __drm_client_modeset_commit_force(client);
+ if (ret)
+ pan_set(client, default_x, default_y);
+
+ return ret;
+}
+
+static int drm_client_modeset_pan_display_legacy(struct drm_client_dev *client,
+ unsigned int x,
+ unsigned int y)
+{
+ struct drm_device *dev = client->dev;
+ struct drm_mode_set *modeset;
+ int ret = 0;
+
+ drm_modeset_lock_all(dev);
+ drm_client_for_each_modeset(modeset, client) {
+ modeset->x = x;
+ modeset->y = y;
+
+ if (modeset->num_connectors)
+ ret = drm_mode_set_config_internal(modeset);
+ }
+ drm_modeset_unlock_all(dev);
+
+ return ret;
+}
+
+/**
+ * drm_client_modeset_dpms() - Set display panning
+ * @client: DRM client
+ * @x: Horizontal coordinate
+ * @y: Vertical coordinate
+ * @default_x: Fallback horizontal coordinate
+ * @default_y: Fallback vertical coordinate
+ *
+ * This function sets the display panning for screens with a virtual
+ * resolution. The x and y coordinates refer to the display's upper
+ * left pixel. If updating fails, the fallback coordinates provide a
+ * safe default value, such as (0,0).
+ *
+ * Returns:
+ * Zero on success or negative error code on failure.
+ */
+int drm_client_modeset_pan_display(struct drm_client_dev *client,
+ unsigned int x, unsigned int y,
+ unsigned int default_x,
+ unsigned int default_y)
+{
+ struct drm_device *dev = client->dev;
+ int ret;
+
+ if (!drm_master_internal_acquire(dev))
+ return -EBUSY;
+
+ mutex_lock(&client->modeset_mutex);
+ if (drm_drv_uses_atomic_modeset(dev))
+ ret = drm_client_modeset_pan_display_atomic(client, x, y,
+ default_x,
+ default_y);
+ else
+ ret = drm_client_modeset_pan_display_legacy(client, x, y);
+ mutex_unlock(&client->modeset_mutex);
+
+ drm_master_internal_release(dev);
+
+ return ret;
+}
+EXPORT_SYMBOL(drm_client_modeset_pan_display);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 7b388674a456..482e92c182c8 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1548,64 +1548,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
}
EXPORT_SYMBOL(drm_fb_helper_set_par);
-static void pan_set(struct drm_fb_helper *fb_helper, int x, int y)
-{
- struct drm_mode_set *mode_set;
-
- mutex_lock(&fb_helper->client.modeset_mutex);
- drm_client_for_each_modeset(mode_set, &fb_helper->client) {
- mode_set->x = x;
- mode_set->y = y;
- }
- mutex_unlock(&fb_helper->client.modeset_mutex);
-}
-
-static int pan_display_atomic(struct fb_var_screeninfo *var,
- struct fb_info *info)
-{
- struct drm_fb_helper *fb_helper = info->par;
- int ret;
-
- pan_set(fb_helper, var->xoffset, var->yoffset);
-
- ret = drm_client_modeset_commit_force(&fb_helper->client);
- if (!ret) {
- info->var.xoffset = var->xoffset;
- info->var.yoffset = var->yoffset;
- } else
- pan_set(fb_helper, info->var.xoffset, info->var.yoffset);
-
- return ret;
-}
-
-static int pan_display_legacy(struct fb_var_screeninfo *var,
- struct fb_info *info)
-{
- struct drm_fb_helper *fb_helper = info->par;
- struct drm_client_dev *client = &fb_helper->client;
- struct drm_mode_set *modeset;
- int ret = 0;
-
- drm_modeset_lock_all(fb_helper->dev);
- mutex_lock(&client->modeset_mutex);
- drm_client_for_each_modeset(modeset, client) {
- modeset->x = var->xoffset;
- modeset->y = var->yoffset;
-
- if (modeset->num_connectors) {
- ret = drm_mode_set_config_internal(modeset);
- if (!ret) {
- info->var.xoffset = var->xoffset;
- info->var.yoffset = var->yoffset;
- }
- }
- }
- mutex_unlock(&client->modeset_mutex);
- drm_modeset_unlock_all(fb_helper->dev);
-
- return ret;
-}
-
/**
* drm_fb_helper_pan_display - implementation for &fb_ops.fb_pan_display
* @var: updated screen information
@@ -1615,27 +1557,22 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
struct fb_info *info)
{
struct drm_fb_helper *fb_helper = info->par;
- struct drm_device *dev = fb_helper->dev;
int ret;
if (oops_in_progress)
return -EBUSY;
mutex_lock(&fb_helper->lock);
- if (!drm_master_internal_acquire(dev)) {
- ret = -EBUSY;
- goto unlock;
- }
-
- if (drm_drv_uses_atomic_modeset(dev))
- ret = pan_display_atomic(var, info);
- else
- ret = pan_display_legacy(var, info);
-
- drm_master_internal_release(dev);
-unlock:
+ ret = drm_client_modeset_pan_display(&fb_helper->client, var->xoffset,
+ var->yoffset, info->var.xoffset,
+ info->var.yoffset);
mutex_unlock(&fb_helper->lock);
+ if (!ret) {
+ info->var.xoffset = var->xoffset;
+ info->var.yoffset = var->yoffset;
+ }
+
return ret;
}
EXPORT_SYMBOL(drm_fb_helper_pan_display);
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 6cf48419f77f..37bf55634227 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -159,6 +159,10 @@ bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotat
int drm_client_modeset_commit_force(struct drm_client_dev *client);
int drm_client_modeset_commit(struct drm_client_dev *client);
int drm_client_modeset_dpms(struct drm_client_dev *client, int mode);
+int drm_client_modeset_pan_display(struct drm_client_dev *client,
+ unsigned int x, unsigned int y,
+ unsigned int default_x,
+ unsigned int default_y);
/**
* drm_client_for_each_modeset() - Iterate over client modesets
--
2.21.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] drm: Reverse lock order in DRM display-panning helpers()
2019-06-11 14:31 [PATCH v2] drm: Reverse lock order in DRM display-panning helpers() Thomas Zimmermann
@ 2019-06-11 15:46 ` Daniel Vetter
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Vetter @ 2019-06-11 15:46 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: sam, dri-devel
On Tue, Jun 11, 2019 at 04:31:14PM +0200, Thomas Zimmermann wrote:
> During display panning, acquiring drm_client_dev.modeset_mutex after the
> locks in drm_fb_helper.dev creates a deadlock with drm_setup_crtcs() as
> shown below:
>
> [ 4.959319] fbcon: radeondrmfb (fb0) is primary device
> [ 4.993952] Console: switching to colour frame buffer device 240x67
> [ 4.994040]
> [ 4.994041] ======================================================
> [ 4.994041] WARNING: possible circular locking dependency detected
> [ 4.994042] 5.2.0-rc4-1-default+ #39 Tainted: G E
> [ 4.994043] ------------------------------------------------------
> [ 4.994043] systemd-udevd/369 is trying to acquire lock:
> [ 4.994044] 00000000fb622acb (&client->modeset_mutex){+.+.}, at: drm_fb_helper_pan_display+0x103/0x1f0 [drm_kms_helper]
> [ 4.994055]
> [ 4.994055] but task is already holding lock:
> [ 4.994055] 0000000028767ae4 (crtc_ww_class_mutex){+.+.}, at: drm_modeset_lock+0x42/0xf0 [drm]
> [ 4.994072]
> [ 4.994072] which lock already depends on the new lock.
> [ 4.994072]
> [ 4.994072]
> [ 4.994072] the existing dependency chain (in reverse order) is:
> [ 4.994073]
> [ 4.994073] -> #3 (crtc_ww_class_mutex){+.+.}:
> [ 4.994076] lock_acquire+0x9e/0x170
> [ 4.994079] __ww_mutex_lock.constprop.18+0x97/0xf40
> [ 4.994080] ww_mutex_lock+0x30/0x90
> [ 4.994091] drm_modeset_lock+0x42/0xf0 [drm]
> [ 4.994102] drm_modeset_lock_all_ctx+0x1f/0xe0 [drm]
> [ 4.994113] drm_modeset_lock_all+0x5e/0x1a0 [drm]
> [ 4.994163] intel_modeset_init+0x60b/0xda0 [i915]
> ..
> [ 4.994253]
> [ 4.994253] -> #2 (crtc_ww_class_acquire){+.+.}:
> [ 4.994255] lock_acquire+0x9e/0x170
> [ 4.994270] drm_modeset_acquire_init+0xcc/0x100 [drm]
> [ 4.994280] drm_modeset_lock_all+0x44/0x1a0 [drm]
> [ 4.994320] intel_modeset_init+0x60b/0xda0 [i915]
> ..
> [ 4.994403]
> [ 4.994403] -> #1 (&dev->mode_config.mutex){+.+.}:
> [ 4.994405] lock_acquire+0x9e/0x170
> [ 4.994408] __mutex_lock+0x62/0x8c0
> [ 4.994413] drm_setup_crtcs+0x17c/0xc50 [drm_kms_helper]
> [ 4.994418] __drm_fb_helper_initial_config_and_unlock+0x34/0x530 [drm_kms_helper]
> [ 4.994450] radeon_fbdev_init+0x110/0x130 [radeon]
> ..
> [ 4.994535]
> [ 4.994535] -> #0 (&client->modeset_mutex){+.+.}:
> [ 4.994537] __lock_acquire+0xa85/0xe90
> [ 4.994538] lock_acquire+0x9e/0x170
> [ 4.994540] __mutex_lock+0x62/0x8c0
> [ 4.994545] drm_fb_helper_pan_display+0x103/0x1f0 [drm_kms_helper]
> [ 4.994547] fb_pan_display+0x92/0x120
> [ 4.994549] bit_update_start+0x1a/0x40
> [ 4.994550] fbcon_switch+0x392/0x580
> [ 4.994552] redraw_screen+0x12c/0x220
> [ 4.994553] do_bind_con_driver.cold.30+0xe1/0x10d
> [ 4.994554] do_take_over_console+0x113/0x190
> [ 4.994555] do_fbcon_takeover+0x58/0xb0
> [ 4.994557] notifier_call_chain+0x47/0x70
> [ 4.994558] blocking_notifier_call_chain+0x44/0x60
> [ 4.994559] register_framebuffer+0x231/0x310
> [ 4.994564] __drm_fb_helper_initial_config_and_unlock+0x2fd/0x530 [drm_kms_helper]
> [ 4.994590] radeon_fbdev_init+0x110/0x130 [radeon]
> ..
>
> This problem was introduced in
>
> d81294afe drm/fb-helper: Remove drm_fb_helper_crtc
>
> Reversing the lock ordering in the display-panning code fixes the issue. In
> addition to the fix, the DRM client now implements the panning. The call to
> drm_client_modeset_pan_display() acquires drm_client_dev.modeset_mutex once
> for the whole operation.
>
> The change was suggested by Daniel Vetter and Noralf Tronnes.
>
> v2:
> * move display panning to DRM client
> * acquire modeset_mutex once in drm_client_modeset_pan_display()
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: d81294afeecdacc8d84804ba0bcb3d39e64d0f27
This won't pass dim's checks. See
$ dim fixes <sha1>
for a tool for this.
Wrt the bugfix, I'd say lets go with your simpler fix first, since I think
this one here needs a bit more work to provide a nice interface.
> ---
> drivers/gpu/drm/drm_client_modeset.c | 110 +++++++++++++++++++++++++--
> drivers/gpu/drm/drm_fb_helper.c | 79 ++-----------------
> include/drm/drm_client.h | 4 +
> 3 files changed, 117 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index b1984f901a6d..20c8fe94249e 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -288,6 +288,19 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
> return ret;
> }
>
> +static int __drm_client_modeset_commit_force(struct drm_client_dev *client)
> +{
> + struct drm_device *dev = client->dev;
> + int ret;
> +
> + if (drm_drv_uses_atomic_modeset(dev))
> + ret = drm_client_modeset_commit_atomic(client, true);
> + else
> + ret = drm_client_modeset_commit_legacy(client);
> +
> + return ret;
> +}
> +
> /**
> * drm_client_modeset_commit_force() - Force commit CRTC configuration
> * @client: DRM client
> @@ -299,14 +312,10 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
> */
> int drm_client_modeset_commit_force(struct drm_client_dev *client)
> {
> - struct drm_device *dev = client->dev;
> int ret;
>
> mutex_lock(&client->modeset_mutex);
> - if (drm_drv_uses_atomic_modeset(dev))
> - ret = drm_client_modeset_commit_atomic(client, true);
> - else
> - ret = drm_client_modeset_commit_legacy(client);
> + ret = __drm_client_modeset_commit_force(client);
> mutex_unlock(&client->modeset_mutex);
>
> return ret;
> @@ -390,3 +399,94 @@ int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
> return ret;
> }
> EXPORT_SYMBOL(drm_client_modeset_dpms);
> +
> +static void pan_set(struct drm_client_dev *client, unsigned int x,
> + unsigned int y)
> +{
> + struct drm_mode_set *mode_set;
> +
> + drm_client_for_each_modeset(mode_set, client) {
> + mode_set->x = x;
> + mode_set->y = y;
> + }
> +}
> +
> +static int drm_client_modeset_pan_display_atomic(struct drm_client_dev *client,
> + unsigned int x,
> + unsigned int y,
> + unsigned int default_x,
> + unsigned int default_y)
> +{
> + int ret;
> +
> + pan_set(client, x, y);
> +
> + ret = __drm_client_modeset_commit_force(client);
> + if (ret)
> + pan_set(client, default_x, default_y);
> +
> + return ret;
> +}
> +
> +static int drm_client_modeset_pan_display_legacy(struct drm_client_dev *client,
> + unsigned int x,
> + unsigned int y)
> +{
> + struct drm_device *dev = client->dev;
> + struct drm_mode_set *modeset;
> + int ret = 0;
> +
> + drm_modeset_lock_all(dev);
> + drm_client_for_each_modeset(modeset, client) {
> + modeset->x = x;
> + modeset->y = y;
> +
> + if (modeset->num_connectors)
> + ret = drm_mode_set_config_internal(modeset);
> + }
> + drm_modeset_unlock_all(dev);
> +
> + return ret;
> +}
> +
> +/**
> + * drm_client_modeset_dpms() - Set display panning
> + * @client: DRM client
> + * @x: Horizontal coordinate
> + * @y: Vertical coordinate
> + * @default_x: Fallback horizontal coordinate
> + * @default_y: Fallback vertical coordinate
Why do we need these? Can't we make a local copy of modeset->x/y and
restore them if it all goes boom? Would be a much nicer interface imo.
-Daniel
> + *
> + * This function sets the display panning for screens with a virtual
> + * resolution. The x and y coordinates refer to the display's upper
> + * left pixel. If updating fails, the fallback coordinates provide a
> + * safe default value, such as (0,0).
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_client_modeset_pan_display(struct drm_client_dev *client,
> + unsigned int x, unsigned int y,
> + unsigned int default_x,
> + unsigned int default_y)
> +{
> + struct drm_device *dev = client->dev;
> + int ret;
> +
> + if (!drm_master_internal_acquire(dev))
> + return -EBUSY;
> +
> + mutex_lock(&client->modeset_mutex);
> + if (drm_drv_uses_atomic_modeset(dev))
> + ret = drm_client_modeset_pan_display_atomic(client, x, y,
> + default_x,
> + default_y);
> + else
> + ret = drm_client_modeset_pan_display_legacy(client, x, y);
> + mutex_unlock(&client->modeset_mutex);
> +
> + drm_master_internal_release(dev);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_client_modeset_pan_display);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 7b388674a456..482e92c182c8 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1548,64 +1548,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
> }
> EXPORT_SYMBOL(drm_fb_helper_set_par);
>
> -static void pan_set(struct drm_fb_helper *fb_helper, int x, int y)
> -{
> - struct drm_mode_set *mode_set;
> -
> - mutex_lock(&fb_helper->client.modeset_mutex);
> - drm_client_for_each_modeset(mode_set, &fb_helper->client) {
> - mode_set->x = x;
> - mode_set->y = y;
> - }
> - mutex_unlock(&fb_helper->client.modeset_mutex);
> -}
> -
> -static int pan_display_atomic(struct fb_var_screeninfo *var,
> - struct fb_info *info)
> -{
> - struct drm_fb_helper *fb_helper = info->par;
> - int ret;
> -
> - pan_set(fb_helper, var->xoffset, var->yoffset);
> -
> - ret = drm_client_modeset_commit_force(&fb_helper->client);
> - if (!ret) {
> - info->var.xoffset = var->xoffset;
> - info->var.yoffset = var->yoffset;
> - } else
> - pan_set(fb_helper, info->var.xoffset, info->var.yoffset);
> -
> - return ret;
> -}
> -
> -static int pan_display_legacy(struct fb_var_screeninfo *var,
> - struct fb_info *info)
> -{
> - struct drm_fb_helper *fb_helper = info->par;
> - struct drm_client_dev *client = &fb_helper->client;
> - struct drm_mode_set *modeset;
> - int ret = 0;
> -
> - drm_modeset_lock_all(fb_helper->dev);
> - mutex_lock(&client->modeset_mutex);
> - drm_client_for_each_modeset(modeset, client) {
> - modeset->x = var->xoffset;
> - modeset->y = var->yoffset;
> -
> - if (modeset->num_connectors) {
> - ret = drm_mode_set_config_internal(modeset);
> - if (!ret) {
> - info->var.xoffset = var->xoffset;
> - info->var.yoffset = var->yoffset;
> - }
> - }
> - }
> - mutex_unlock(&client->modeset_mutex);
> - drm_modeset_unlock_all(fb_helper->dev);
> -
> - return ret;
> -}
> -
> /**
> * drm_fb_helper_pan_display - implementation for &fb_ops.fb_pan_display
> * @var: updated screen information
> @@ -1615,27 +1557,22 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
> struct fb_info *info)
> {
> struct drm_fb_helper *fb_helper = info->par;
> - struct drm_device *dev = fb_helper->dev;
> int ret;
>
> if (oops_in_progress)
> return -EBUSY;
>
> mutex_lock(&fb_helper->lock);
> - if (!drm_master_internal_acquire(dev)) {
> - ret = -EBUSY;
> - goto unlock;
> - }
> -
> - if (drm_drv_uses_atomic_modeset(dev))
> - ret = pan_display_atomic(var, info);
> - else
> - ret = pan_display_legacy(var, info);
> -
> - drm_master_internal_release(dev);
> -unlock:
> + ret = drm_client_modeset_pan_display(&fb_helper->client, var->xoffset,
> + var->yoffset, info->var.xoffset,
> + info->var.yoffset);
> mutex_unlock(&fb_helper->lock);
>
> + if (!ret) {
> + info->var.xoffset = var->xoffset;
> + info->var.yoffset = var->yoffset;
> + }
> +
> return ret;
> }
> EXPORT_SYMBOL(drm_fb_helper_pan_display);
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 6cf48419f77f..37bf55634227 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -159,6 +159,10 @@ bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotat
> int drm_client_modeset_commit_force(struct drm_client_dev *client);
> int drm_client_modeset_commit(struct drm_client_dev *client);
> int drm_client_modeset_dpms(struct drm_client_dev *client, int mode);
> +int drm_client_modeset_pan_display(struct drm_client_dev *client,
> + unsigned int x, unsigned int y,
> + unsigned int default_x,
> + unsigned int default_y);
>
> /**
> * drm_client_for_each_modeset() - Iterate over client modesets
> --
> 2.21.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-06-11 15:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-11 14:31 [PATCH v2] drm: Reverse lock order in DRM display-panning helpers() Thomas Zimmermann
2019-06-11 15:46 ` Daniel Vetter
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.