[PATCH 6/7] drm/udl: Begin/end access to imported buffers in damage-handler

[Emil Velikov]

On Wed, 4 Dec 2019 at 13:24, Thomas Zimmermann <tzimmermann at 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 at 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