On Wed, May 13, 2020 at 05:03:12PM +0200, Thomas Zimmermann wrote:
> The udl driver contains an implementation of GEM vmap and mmap
> operations that is identical to the common SHMEM helper; except
> that udl's code does not support writecombine mappings.
>
> Convert udl to regular SHMEM helper functions. There's no reason
> to have udl behave differently from all other SHMEM drivers.
>
> Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
So I remember the problem again I think ... on some x86 gpu drivers
(*cough* i915 on specific chipsets) you get an uncached dma-buf.
Which means if you set up a cached mapping, you get corruption.
But if the shmem helpers to correctly forward _all_ calls to the dma-buf
functions of the exporter, this works. I've tried to clean up that a bit
with my patch series that I just posted this week, to make sure there's no
bugs like that.
I think once we have that fully sorted, we could land this and be happy.
-Daniel
> ---
> drivers/gpu/drm/udl/Makefile | 2 +-
> drivers/gpu/drm/udl/udl_drv.c | 3 -
> drivers/gpu/drm/udl/udl_drv.h | 3 -
> drivers/gpu/drm/udl/udl_gem.c | 106 ----------------------------------
> 4 files changed, 1 insertion(+), 113 deletions(-)
> delete mode 100644 drivers/gpu/drm/udl/udl_gem.c
>
> diff --git a/drivers/gpu/drm/udl/Makefile b/drivers/gpu/drm/udl/Makefile
> index b50179bb4de06..24d61f61d7db2 100644
> --- a/drivers/gpu/drm/udl/Makefile
> +++ b/drivers/gpu/drm/udl/Makefile
> @@ -1,4 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_main.o udl_transfer.o
> udl_gem.o
> +udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_main.o udl_transfer.o
>
> obj-$(CONFIG_DRM_UDL) := udl.o
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index d1aa50fd6d65a..cf5b679bf58bb 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -37,9 +37,6 @@ DEFINE_DRM_GEM_FOPS(udl_driver_fops);
> static struct drm_driver driver = {
> .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
>
> - /* gem hooks */
> - .gem_create_object = udl_driver_gem_create_object,
> -
> .fops = &udl_driver_fops,
> DRM_GEM_SHMEM_DRIVER_OPS,
>
> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
> index 2642f94a63fc8..b1461f30780bc 100644
> --- a/drivers/gpu/drm/udl/udl_drv.h
> +++ b/drivers/gpu/drm/udl/udl_drv.h
> @@ -81,9 +81,6 @@ int udl_render_hline(struct drm_device *dev, int log_bpp,
> struct urb **urb_ptr,
> const char *front, char **urb_buf_ptr,
> u32 byte_offset, u32 device_byte_offset, u32 byte_width);
>
> -struct drm_gem_object *udl_driver_gem_create_object(struct drm_device *dev,
> - size_t size);
> -
> int udl_drop_usb(struct drm_device *dev);
>
> #define CMD_WRITE_RAW8 "\xAF\x60" /**< 8 bit raw write command. */
> diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
> deleted file mode 100644
> index b6e26f98aa0af..0000000000000
> --- a/drivers/gpu/drm/udl/udl_gem.c
> +++ /dev/null
> @@ -1,106 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Copyright (C) 2012 Red Hat
> - */
> -
> -#include <linux/dma-buf.h>
> -#include <linux/vmalloc.h>
> -
> -#include <drm/drm_drv.h>
> -#include <drm/drm_gem_shmem_helper.h>
> -#include <drm/drm_mode.h>
> -#include <drm/drm_prime.h>
> -
> -#include "udl_drv.h"
> -
> -/*
> - * GEM object funcs
> - */
> -
> -static int udl_gem_object_mmap(struct drm_gem_object *obj,
> - struct vm_area_struct *vma)
> -{
> - int ret;
> -
> - ret = drm_gem_shmem_mmap(obj, vma);
> - if (ret)
> - return ret;
> -
> - vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> - if (obj->import_attach)
> - vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> - vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
> -
> - return 0;
> -}
> -
> -static void *udl_gem_object_vmap(struct drm_gem_object *obj)
> -{
> - struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> - int ret;
> -
> - ret = mutex_lock_interruptible(&shmem->vmap_lock);
> - if (ret)
> - return ERR_PTR(ret);
> -
> - if (shmem->vmap_use_count++ > 0)
> - goto out;
> -
> - ret = drm_gem_shmem_get_pages(shmem);
> - if (ret)
> - goto err_zero_use;
> -
> - if (obj->import_attach)
> - shmem->vaddr = dma_buf_vmap(obj->import_attach->dmabuf);
> - else
> - shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT,
> - VM_MAP, PAGE_KERNEL);
> -
> - if (!shmem->vaddr) {
> - DRM_DEBUG_KMS("Failed to vmap pages\n");
> - ret = -ENOMEM;
> - goto err_put_pages;
> - }
> -
> -out:
> - mutex_unlock(&shmem->vmap_lock);
> - return shmem->vaddr;
> -
> -err_put_pages:
> - drm_gem_shmem_put_pages(shmem);
> -err_zero_use:
> - shmem->vmap_use_count = 0;
> - mutex_unlock(&shmem->vmap_lock);
> - return ERR_PTR(ret);
> -}
> -
> -static const struct drm_gem_object_funcs udl_gem_object_funcs = {
> - .free = drm_gem_shmem_free_object,
> - .print_info = drm_gem_shmem_print_info,
> - .pin = drm_gem_shmem_pin,
> - .unpin = drm_gem_shmem_unpin,
> - .get_sg_table = drm_gem_shmem_get_sg_table,
> - .vmap = udl_gem_object_vmap,
> - .vunmap = drm_gem_shmem_vunmap,
> - .mmap = udl_gem_object_mmap,
> -};
> -
> -/*
> - * Helpers for struct drm_driver
> - */
> -
> -struct drm_gem_object *udl_driver_gem_create_object(struct drm_device *dev,
> - size_t size)
> -{
> - struct drm_gem_shmem_object *shmem;
> - struct drm_gem_object *obj;
> -
> - shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
> - if (!shmem)
> - return NULL;
> -
> - obj = &shmem->base;
> - obj->funcs = &udl_gem_object_funcs;
> -
> - return obj;
> -}
> --
> 2.26.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
