drm: Push dirtyfb ioctl kms locking down to drivers

Commit Message

Ville Syrjala Dec. 4, 2013, 12:13 p.m. UTC

From: Ville Syrjälä <[e-mail]>

Not all drivers will need take all the modeset locks for dirtyfb, so
push the locking down to the drivers.

Signed-off-by: Ville Syrjälä <[e-mail]>
Reviewed-by: Daniel Vetter <[e-mail]>
---
 drivers/gpu/drm/drm_crtc.c          |  2 --
 drivers/gpu/drm/omapdrm/omap_fb.c   |  4 ++++
 drivers/gpu/drm/qxl/qxl_display.c   |  9 ++++++++-
 drivers/gpu/drm/udl/udl_fb.c        | 12 +++++++++---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 18 ++++++++++++++++--
 5 files changed, 37 insertions(+), 8 deletions(-)

Comments

Jakob Bornecrantz Dec. 4, 2013, 12:30 p.m. UTC | #1

Looks good to me
Reviewed-by: Jakob Bornecrant <[e-mail]>

On Wed, Dec 4, 2013 at 1:13 PM,  <[e-mail]> wrote:
> From: Ville Syrjälä <[e-mail]>
>
> Not all drivers will need take all the modeset locks for dirtyfb, so
> push the locking down to the drivers.
>
> Signed-off-by: Ville Syrjälä <[e-mail]>
> Reviewed-by: Daniel Vetter <[e-mail]>
> ---
>  drivers/gpu/drm/drm_crtc.c          |  2 --
>  drivers/gpu/drm/omapdrm/omap_fb.c   |  4 ++++
>  drivers/gpu/drm/qxl/qxl_display.c   |  9 ++++++++-
>  drivers/gpu/drm/udl/udl_fb.c        | 12 +++++++++---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 18 ++++++++++++++++--
>  5 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index d6cf77c..266a01d 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2767,10 +2767,8 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
>         }
>
>         if (fb->funcs->dirty) {
> -               drm_modeset_lock_all(dev);
>                 ret = fb->funcs->dirty(fb, file_priv, flags, r->color,
>                                        clips, num_clips);
> -               drm_modeset_unlock_all(dev);
>         } else {
>                 ret = -ENOSYS;
>         }
> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
> index f2b8f06..f466c4a 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> @@ -123,12 +123,16 @@ static int omap_framebuffer_dirty(struct drm_framebuffer *fb,
>  {
>         int i;
>
> +       drm_modeset_lock_all(fb->dev);
> +
>         for (i = 0; i < num_clips; i++) {
>                 omap_framebuffer_flush(fb, clips[i].x1, clips[i].y1,
>                                         clips[i].x2 - clips[i].x1,
>                                         clips[i].y2 - clips[i].y1);
>         }
>
> +       drm_modeset_unlock_all(fb->dev);
> +
>         return 0;
>  }
>
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
> index 5e827c2..b8f3bc7 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -399,10 +399,14 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
>         struct qxl_bo *qobj;
>         int inc = 1;
>
> +       drm_modeset_lock_all(fb->dev);
> +
>         qobj = gem_to_qxl_bo(qxl_fb->obj);
>         /* if we aren't primary surface ignore this */
> -       if (!qobj->is_primary)
> +       if (!qobj->is_primary) {
> +               drm_modeset_unlock_all(fb->dev);
>                 return 0;
> +       }
>
>         if (!num_clips) {
>                 num_clips = 1;
> @@ -417,6 +421,9 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
>
>         qxl_draw_dirty_fb(qdev, qxl_fb, qobj, flags, color,
>                           clips, num_clips, inc);
> +
> +       drm_modeset_unlock_all(fb->dev);
> +
>         return 0;
>  }
>
> diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
> index 97e9d61..dbadd49 100644
> --- a/drivers/gpu/drm/udl/udl_fb.c
> +++ b/drivers/gpu/drm/udl/udl_fb.c
> @@ -403,15 +403,17 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb,
>         int i;
>         int ret = 0;
>
> +       drm_modeset_lock_all(fb->dev);
> +
>         if (!ufb->active_16)
> -               return 0;
> +               goto unlock;
>
>         if (ufb->obj->base.import_attach) {
>                 ret = dma_buf_begin_cpu_access(ufb->obj->base.import_attach->dmabuf,
>                                                0, ufb->obj->base.size,
>                                                DMA_FROM_DEVICE);
>                 if (ret)
> -                       return ret;
> +                       goto unlock;
>         }
>
>         for (i = 0; i < num_clips; i++) {
> @@ -419,7 +421,7 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb,
>                                   clips[i].x2 - clips[i].x1,
>                                   clips[i].y2 - clips[i].y1);
>                 if (ret)
> -                       break;
> +                       goto unlock;
>         }
>
>         if (ufb->obj->base.import_attach) {
> @@ -427,6 +429,10 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb,
>                                        0, ufb->obj->base.size,
>                                        DMA_FROM_DEVICE);
>         }
> +
> + unlock:
> +       drm_modeset_unlock_all(fb->dev);
> +
>         return ret;
>  }
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index ecb3d86..ab0b88f 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -608,9 +608,13 @@ int vmw_framebuffer_surface_dirty(struct drm_framebuffer *framebuffer,
>         if (!dev_priv->sou_priv)
>                 return -EINVAL;
>
> +       drm_modeset_lock_all(dev_priv->dev);
> +
>         ret = ttm_read_lock(&vmaster->lock, true);
> -       if (unlikely(ret != 0))
> +       if (unlikely(ret != 0)) {
> +               drm_modeset_unlock_all(dev_priv->dev);
>                 return ret;
> +       }
>
>         if (!num_clips) {
>                 num_clips = 1;
> @@ -628,6 +632,9 @@ int vmw_framebuffer_surface_dirty(struct drm_framebuffer *framebuffer,
>                                    clips, num_clips, inc, NULL);
>
>         ttm_read_unlock(&vmaster->lock);
> +
> +       drm_modeset_unlock_all(dev_priv->dev);
> +
>         return 0;
>  }
>
> @@ -952,9 +959,13 @@ int vmw_framebuffer_dmabuf_dirty(struct drm_framebuffer *framebuffer,
>         struct drm_clip_rect norect;
>         int ret, increment = 1;
>
> +       drm_modeset_lock_all(dev_priv->dev);
> +
>         ret = ttm_read_lock(&vmaster->lock, true);
> -       if (unlikely(ret != 0))
> +       if (unlikely(ret != 0)) {
> +               drm_modeset_unlock_all(dev_priv->dev);
>                 return ret;
> +       }
>
>         if (!num_clips) {
>                 num_clips = 1;
> @@ -978,6 +989,9 @@ int vmw_framebuffer_dmabuf_dirty(struct drm_framebuffer *framebuffer,
>         }
>
>         ttm_read_unlock(&vmaster->lock);
> +
> +       drm_modeset_unlock_all(dev_priv->dev);
> +
>         return ret;
>  }
>
> --
> 1.8.3.2
>
> _______________________________________________
> dri-devel mailing list
> [e-mail]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Patch

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d6cf77c..266a01d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2767,10 +2767,8 @@  int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
 	}
 
 	if (fb->funcs->dirty) {
-		drm_modeset_lock_all(dev);
 		ret = fb->funcs->dirty(fb, file_priv, flags, r->color,
 				       clips, num_clips);
-		drm_modeset_unlock_all(dev);
 	} else {
 		ret = -ENOSYS;
 	}
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index f2b8f06..f466c4a 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -123,12 +123,16 @@  static int omap_framebuffer_dirty(struct drm_framebuffer *fb,
 {
 	int i;
 
+	drm_modeset_lock_all(fb->dev);
+
 	for (i = 0; i < num_clips; i++) {
 		omap_framebuffer_flush(fb, clips[i].x1, clips[i].y1,
 					clips[i].x2 - clips[i].x1,
 					clips[i].y2 - clips[i].y1);
 	}
 
+	drm_modeset_unlock_all(fb->dev);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 5e827c2..b8f3bc7 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -399,10 +399,14 @@  static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
 	struct qxl_bo *qobj;
 	int inc = 1;
 
+	drm_modeset_lock_all(fb->dev);
+
 	qobj = gem_to_qxl_bo(qxl_fb->obj);
 	/* if we aren't primary surface ignore this */
-	if (!qobj->is_primary)
+	if (!qobj->is_primary) {
+		drm_modeset_unlock_all(fb->dev);
 		return 0;
+	}
 
 	if (!num_clips) {
 		num_clips = 1;
@@ -417,6 +421,9 @@  static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
 
 	qxl_draw_dirty_fb(qdev, qxl_fb, qobj, flags, color,
 			  clips, num_clips, inc);
+
+	drm_modeset_unlock_all(fb->dev);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index 97e9d61..dbadd49 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -403,15 +403,17 @@  static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb,
 	int i;
 	int ret = 0;
 
+	drm_modeset_lock_all(fb->dev);
+
 	if (!ufb->active_16)
-		return 0;
+		goto unlock;
 
 	if (ufb->obj->base.import_attach) {
 		ret = dma_buf_begin_cpu_access(ufb->obj->base.import_attach->dmabuf,
 					       0, ufb->obj->base.size,
 					       DMA_FROM_DEVICE);
 		if (ret)
-			return ret;
+			goto unlock;
 	}
 
 	for (i = 0; i < num_clips; i++) {
@@ -419,7 +421,7 @@  static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb,
 				  clips[i].x2 - clips[i].x1,
 				  clips[i].y2 - clips[i].y1);
 		if (ret)
-			break;
+			goto unlock;
 	}
 
 	if (ufb->obj->base.import_attach) {
@@ -427,6 +429,10 @@  static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb,
 				       0, ufb->obj->base.size,
 				       DMA_FROM_DEVICE);
 	}
+
+ unlock:
+	drm_modeset_unlock_all(fb->dev);
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index ecb3d86..ab0b88f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -608,9 +608,13 @@  int vmw_framebuffer_surface_dirty(struct drm_framebuffer *framebuffer,
 	if (!dev_priv->sou_priv)
 		return -EINVAL;
 
+	drm_modeset_lock_all(dev_priv->dev);
+
 	ret = ttm_read_lock(&vmaster->lock, true);
-	if (unlikely(ret != 0))
+	if (unlikely(ret != 0)) {
+		drm_modeset_unlock_all(dev_priv->dev);
 		return ret;
+	}
 
 	if (!num_clips) {
 		num_clips = 1;
@@ -628,6 +632,9 @@  int vmw_framebuffer_surface_dirty(struct drm_framebuffer *framebuffer,
 				   clips, num_clips, inc, NULL);
 
 	ttm_read_unlock(&vmaster->lock);
+
+	drm_modeset_unlock_all(dev_priv->dev);
+
 	return 0;
 }
 
@@ -952,9 +959,13 @@  int vmw_framebuffer_dmabuf_dirty(struct drm_framebuffer *framebuffer,
 	struct drm_clip_rect norect;
 	int ret, increment = 1;
 
+	drm_modeset_lock_all(dev_priv->dev);
+
 	ret = ttm_read_lock(&vmaster->lock, true);
-	if (unlikely(ret != 0))
+	if (unlikely(ret != 0)) {
+		drm_modeset_unlock_all(dev_priv->dev);
 		return ret;
+	}
 
 	if (!num_clips) {
 		num_clips = 1;
@@ -978,6 +989,9 @@  int vmw_framebuffer_dmabuf_dirty(struct drm_framebuffer *framebuffer,
 	}
 
 	ttm_read_unlock(&vmaster->lock);
+
+	drm_modeset_unlock_all(dev_priv->dev);
+
 	return ret;
 }