| Message ID | 20191204132430.16874-7-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/udl: Prepare damage handler for simple-pipe KMS | expand |
On Wed, 4 Dec 2019 at 13:24, Thomas Zimmermann <tzimmermann@suse.de> wrote: > > The damage-handler code now invokes dma_buf_{begin,end}_access() > for imported buffers. These calls were missing from the page-flip > and modesetting code paths. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/udl/udl_fb.c | 38 ++++++++++++++++++------------------ > 1 file changed, 19 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c > index 482786eeea6c..7d184ff96a1f 100644 > --- a/drivers/gpu/drm/udl/udl_fb.c > +++ b/drivers/gpu/drm/udl/udl_fb.c > @@ -92,6 +92,7 @@ int udl_handle_damage(struct drm_framebuffer *fb, int x, int y, > { > struct drm_device *dev = fb->dev; > struct udl_device *udl = to_udl(dev); > + struct dma_buf_attachment *import_attach = fb->obj[0]->import_attach; > int i, ret; > char *cmd; > struct urb *urb; > @@ -117,15 +118,22 @@ int udl_handle_damage(struct drm_framebuffer *fb, int x, int y, > else if ((clip.x2 > fb->width) || (clip.y2 > fb->height)) > return -EINVAL; > > + if (import_attach) { > + ret = dma_buf_begin_cpu_access(import_attach->dmabuf, > + DMA_FROM_DEVICE); > + if (ret) > + return ret; > + } > + > vaddr = drm_gem_shmem_vmap(fb->obj[0]); > if (IS_ERR(vaddr)) { > DRM_ERROR("failed to vmap fb\n"); > - return 0; > + goto out_dma_buf_end_cpu_access; > } > > urb = udl_get_urb(dev); > if (!urb) > - goto out; > + goto out_drm_gem_shmem_vunmap; > cmd = urb->transfer_buffer; > > for (i = clip.y1; i < clip.y2; i++) { > @@ -136,7 +144,7 @@ int udl_handle_damage(struct drm_framebuffer *fb, int x, int y, > if (udl_render_hline(dev, log_bpp, &urb, (char *)vaddr, > &cmd, byte_offset, dev_byte_offset, > byte_width)) > - goto out; > + goto out_drm_gem_shmem_vunmap; > } > > if (cmd > (char *) urb->transfer_buffer) { > @@ -149,10 +157,16 @@ int udl_handle_damage(struct drm_framebuffer *fb, int x, int y, > } else > udl_urb_completion(urb); > > -out: > + ret = 0; > + > +out_drm_gem_shmem_vunmap: > drm_gem_shmem_vunmap(fb->obj[0], vaddr); > +out_dma_buf_end_cpu_access: > + if (import_attach) > + ret = dma_buf_end_cpu_access(import_attach->dmabuf, > + DMA_FROM_DEVICE); > > - return 0; > + return ret; Since you're touching the end_cpu_access call, we might as well address the following bug. Namely: Even though we get a failure from udl_render_hline() or otherwise, the function return "OK" when end_cpu_access() is successful. If you really want to, one can keep it separate (+ CC stable), although it's fine to squash it here with a small note in the commit message. HTH Emil
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index 482786eeea6c..7d184ff96a1f 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -92,6 +92,7 @@ int udl_handle_damage(struct drm_framebuffer *fb, int x, int y, { struct drm_device *dev = fb->dev; struct udl_device *udl = to_udl(dev); + struct dma_buf_attachment *import_attach = fb->obj[0]->import_attach; int i, ret; char *cmd; struct urb *urb; @@ -117,15 +118,22 @@ int udl_handle_damage(struct drm_framebuffer *fb, int x, int y, else if ((clip.x2 > fb->width) || (clip.y2 > fb->height)) return -EINVAL; + if (import_attach) { + ret = dma_buf_begin_cpu_access(import_attach->dmabuf, + DMA_FROM_DEVICE); + if (ret) + return ret; + } + vaddr = drm_gem_shmem_vmap(fb->obj[0]); if (IS_ERR(vaddr)) { DRM_ERROR("failed to vmap fb\n"); - return 0; + goto out_dma_buf_end_cpu_access; } urb = udl_get_urb(dev); if (!urb) - goto out; + goto out_drm_gem_shmem_vunmap; cmd = urb->transfer_buffer; for (i = clip.y1; i < clip.y2; i++) { @@ -136,7 +144,7 @@ int udl_handle_damage(struct drm_framebuffer *fb, int x, int y, if (udl_render_hline(dev, log_bpp, &urb, (char *)vaddr, &cmd, byte_offset, dev_byte_offset, byte_width)) - goto out; + goto out_drm_gem_shmem_vunmap; } if (cmd > (char *) urb->transfer_buffer) { @@ -149,10 +157,16 @@ int udl_handle_damage(struct drm_framebuffer *fb, int x, int y, } else udl_urb_completion(urb); -out: + ret = 0; + +out_drm_gem_shmem_vunmap: drm_gem_shmem_vunmap(fb->obj[0], vaddr); +out_dma_buf_end_cpu_access: + if (import_attach) + ret = dma_buf_end_cpu_access(import_attach->dmabuf, + DMA_FROM_DEVICE); - return 0; + return ret; } static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb, @@ -162,7 +176,6 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb, unsigned num_clips) { struct udl_device *udl = fb->dev->dev_private; - struct dma_buf_attachment *import_attach; int i; int ret = 0; @@ -175,15 +188,6 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb, } spin_unlock(&udl->active_fb_16_lock); - import_attach = fb->obj[0]->import_attach; - - if (import_attach) { - ret = dma_buf_begin_cpu_access(import_attach->dmabuf, - DMA_FROM_DEVICE); - if (ret) - goto unlock; - } - for (i = 0; i < num_clips; i++) { ret = udl_handle_damage(fb, clips[i].x1, clips[i].y1, clips[i].x2 - clips[i].x1, @@ -192,10 +196,6 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb, break; } - if (import_attach) - ret = dma_buf_end_cpu_access(import_attach->dmabuf, - DMA_FROM_DEVICE); - unlock: drm_modeset_unlock_all(fb->dev);
The damage-handler code now invokes dma_buf_{begin,end}_access() for imported buffers. These calls were missing from the page-flip and modesetting code paths. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/udl/udl_fb.c | 38 ++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 19 deletions(-)