After:
commit d059f652e73c35678d28d4cd09ab2cec89696af9 Author: Daniel Vetter daniel.vetter@ffwll.ch AuthorDate: Fri Jul 25 18:07:40 2014 +0200
drm: Handle legacy per-crtc locking with full acquire ctx
drm_mode_cursor_common() was switched to use drm_modeset_(un)lock_crtc() which uses full aquire ctx. So dropping/reaquiring the lock via drm_modeset_(un)lock() directly isn't the right thing to do, as lockdep kindly points out.
The 'FIXME's about sorting out whether vmwgfx *really* needs to lock-all for cursor updates still apply.
Signed-off-by: Rob Clark robdclark@gmail.com --- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index d2bc2b0..8fc1e38 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -187,7 +187,7 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, * can do this since the caller in the drm core doesn't check anything * which is protected by any looks. */ - drm_modeset_unlock(&crtc->mutex); + drm_modeset_unlock_crtc(crtc); drm_modeset_lock_all(dev_priv->dev);
/* A lot of the code assumes this */ @@ -252,7 +252,7 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, ret = 0; out: drm_modeset_unlock_all(dev_priv->dev); - drm_modeset_lock(&crtc->mutex, NULL); + drm_modeset_lock_crtc(crtc);
return ret; } @@ -273,7 +273,7 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) * can do this since the caller in the drm core doesn't check anything * which is protected by any looks. */ - drm_modeset_unlock(&crtc->mutex); + drm_modeset_unlock_crtc(crtc); drm_modeset_lock_all(dev_priv->dev);
vmw_cursor_update_position(dev_priv, shown, @@ -281,7 +281,7 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) du->cursor_y + du->hotspot_y);
drm_modeset_unlock_all(dev_priv->dev); - drm_modeset_lock(&crtc->mutex, NULL); + drm_modeset_lock_crtc(crtc);
return 0; }
On Thu, Oct 30, 2014 at 1:39 PM, Rob Clark robdclark@gmail.com wrote:
After:
commit d059f652e73c35678d28d4cd09ab2cec89696af9 Author: Daniel Vetter daniel.vetter@ffwll.ch AuthorDate: Fri Jul 25 18:07:40 2014 +0200
drm: Handle legacy per-crtc locking with full acquire ctxdrm_mode_cursor_common() was switched to use drm_modeset_(un)lock_crtc() which uses full aquire ctx. So dropping/reaquiring the lock via drm_modeset_(un)lock() directly isn't the right thing to do, as lockdep kindly points out.
The 'FIXME's about sorting out whether vmwgfx *really* needs to lock-all for cursor updates still apply.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1155825
Signed-off-by: Rob Clark robdclark@gmail.com
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index d2bc2b0..8fc1e38 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -187,7 +187,7 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, * can do this since the caller in the drm core doesn't check anything * which is protected by any looks. */
drm_modeset_unlock(&crtc->mutex);
drm_modeset_unlock_crtc(crtc); drm_modeset_lock_all(dev_priv->dev); /* A lot of the code assumes this */@@ -252,7 +252,7 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, ret = 0; out: drm_modeset_unlock_all(dev_priv->dev);
drm_modeset_lock(&crtc->mutex, NULL);
drm_modeset_lock_crtc(crtc); return ret;} @@ -273,7 +273,7 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) * can do this since the caller in the drm core doesn't check anything * which is protected by any looks. */
drm_modeset_unlock(&crtc->mutex);
drm_modeset_unlock_crtc(crtc); drm_modeset_lock_all(dev_priv->dev); vmw_cursor_update_position(dev_priv, shown,@@ -281,7 +281,7 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) du->cursor_y + du->hotspot_y);
drm_modeset_unlock_all(dev_priv->dev);
drm_modeset_lock(&crtc->mutex, NULL);
drm_modeset_lock_crtc(crtc); return 0;}
1.9.3
On Thu, Oct 30, 2014 at 6:40 PM, Rob Clark robdclark@gmail.com wrote:
On Thu, Oct 30, 2014 at 1:39 PM, Rob Clark robdclark@gmail.com wrote:
After:
commit d059f652e73c35678d28d4cd09ab2cec89696af9 Author: Daniel Vetter daniel.vetter@ffwll.ch AuthorDate: Fri Jul 25 18:07:40 2014 +0200
drm: Handle legacy per-crtc locking with full acquire ctxdrm_mode_cursor_common() was switched to use drm_modeset_(un)lock_crtc() which uses full aquire ctx. So dropping/reaquiring the lock via drm_modeset_(un)lock() directly isn't the right thing to do, as lockdep kindly points out.
The 'FIXME's about sorting out whether vmwgfx *really* needs to lock-all for cursor updates still apply.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1155825
Signed-off-by: Rob Clark robdclark@gmail.com
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Jakob Bornecrantz jakob@vmware.com
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index d2bc2b0..8fc1e38 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -187,7 +187,7 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, * can do this since the caller in the drm core doesn't check anything * which is protected by any looks. */
drm_modeset_unlock(&crtc->mutex);
drm_modeset_unlock_crtc(crtc); drm_modeset_lock_all(dev_priv->dev); /* A lot of the code assumes this */@@ -252,7 +252,7 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, ret = 0; out: drm_modeset_unlock_all(dev_priv->dev);
drm_modeset_lock(&crtc->mutex, NULL);
drm_modeset_lock_crtc(crtc); return ret;} @@ -273,7 +273,7 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) * can do this since the caller in the drm core doesn't check anything * which is protected by any looks. */
drm_modeset_unlock(&crtc->mutex);
drm_modeset_unlock_crtc(crtc); drm_modeset_lock_all(dev_priv->dev); vmw_cursor_update_position(dev_priv, shown,@@ -281,7 +281,7 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) du->cursor_y + du->hotspot_y);
drm_modeset_unlock_all(dev_priv->dev);
drm_modeset_lock(&crtc->mutex, NULL);
drm_modeset_lock_crtc(crtc); return 0;}
1.9.3
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 10/30/2014 06:39 PM, Rob Clark wrote:
After:
commit d059f652e73c35678d28d4cd09ab2cec89696af9 Author: Daniel Vetter daniel.vetter@ffwll.ch AuthorDate: Fri Jul 25 18:07:40 2014 +0200
drm: Handle legacy per-crtc locking with full acquire ctxdrm_mode_cursor_common() was switched to use drm_modeset_(un)lock_crtc() which uses full aquire ctx. So dropping/reaquiring the lock via drm_modeset_(un)lock() directly isn't the right thing to do, as lockdep kindly points out.
The 'FIXME's about sorting out whether vmwgfx *really* needs to lock-all for cursor updates still apply.
Signed-off-by: Rob Clark robdclark@gmail.com
Tested-by: Thomas Hellstrom thellstrom@vmware.com
Thanks, Rob.
/Thomas
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index d2bc2b0..8fc1e38 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -187,7 +187,7 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, * can do this since the caller in the drm core doesn't check anything * which is protected by any looks. */
- drm_modeset_unlock(&crtc->mutex);
drm_modeset_unlock_crtc(crtc); drm_modeset_lock_all(dev_priv->dev);
/* A lot of the code assumes this */
@@ -252,7 +252,7 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, ret = 0; out: drm_modeset_unlock_all(dev_priv->dev);
- drm_modeset_lock(&crtc->mutex, NULL);
drm_modeset_lock_crtc(crtc);
return ret;
} @@ -273,7 +273,7 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) * can do this since the caller in the drm core doesn't check anything * which is protected by any looks. */
- drm_modeset_unlock(&crtc->mutex);
drm_modeset_unlock_crtc(crtc); drm_modeset_lock_all(dev_priv->dev);
vmw_cursor_update_position(dev_priv, shown,
@@ -281,7 +281,7 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) du->cursor_y + du->hotspot_y);
drm_modeset_unlock_all(dev_priv->dev);
- drm_modeset_lock(&crtc->mutex, NULL);
drm_modeset_lock_crtc(crtc);
return 0;
}
On Thu, Oct 30, 2014 at 01:39:04PM -0400, Rob Clark wrote:
After:
commit d059f652e73c35678d28d4cd09ab2cec89696af9 Author: Daniel Vetter daniel.vetter@ffwll.ch AuthorDate: Fri Jul 25 18:07:40 2014 +0200
drm: Handle legacy per-crtc locking with full acquire ctxdrm_mode_cursor_common() was switched to use drm_modeset_(un)lock_crtc() which uses full aquire ctx. So dropping/reaquiring the lock via drm_modeset_(un)lock() directly isn't the right thing to do, as lockdep kindly points out.
The 'FIXME's about sorting out whether vmwgfx *really* needs to lock-all for cursor updates still apply.
Signed-off-by: Rob Clark robdclark@gmail.com
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index d2bc2b0..8fc1e38 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -187,7 +187,7 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, * can do this since the caller in the drm core doesn't check anything * which is protected by any looks. */
- drm_modeset_unlock(&crtc->mutex);
- drm_modeset_unlock_crtc(crtc); drm_modeset_lock_all(dev_priv->dev);
Oh right this is simpler, but it also grabs a new acquire context. So not too terribly fair really, but otoh neither was the old one.
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
/* A lot of the code assumes this */ @@ -252,7 +252,7 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, ret = 0; out: drm_modeset_unlock_all(dev_priv->dev);
- drm_modeset_lock(&crtc->mutex, NULL);
drm_modeset_lock_crtc(crtc);
return ret;
} @@ -273,7 +273,7 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) * can do this since the caller in the drm core doesn't check anything * which is protected by any looks. */
- drm_modeset_unlock(&crtc->mutex);
drm_modeset_unlock_crtc(crtc); drm_modeset_lock_all(dev_priv->dev);
vmw_cursor_update_position(dev_priv, shown,
@@ -281,7 +281,7 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) du->cursor_y + du->hotspot_y);
drm_modeset_unlock_all(dev_priv->dev);
- drm_modeset_lock(&crtc->mutex, NULL);
drm_modeset_lock_crtc(crtc);
return 0;
}
1.9.3
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Jakob Bornecrantz -
Rob Clark -
Thomas Hellstrom