On Wed, 4 Dec 2019 at 13:24, Thomas Zimmermann <tzimmermann@xxxxxxx> 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@xxxxxxx>
> ---
> 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
