On Wed, Nov 06, 2019 at 11:47:21AM +0100, Thomas Zimmermann wrote:
> Udl's GEM code and the generic SHMEM are almost identical. Replace
> the former with SHMEM. The dmabuf support in udl is being replaced
> with generic GEM PRIME functions.
>
> The main difference is in the caching flags for mmap pages. By
> default, SHMEM always sets (uncached) write combining. In udl's
> memory management code, only imported buffers use write combining.
> Memory pages of locally created buffer objects are mmap'ed with
> caching enabled. To keep the optimization, udl provides its own
> mmap function for GEM objects where it fixes up the mapping flags.
>
> v2:
> - remove obsolete code in a separate patch
That indeed makes the patch more readable as the context stays intact
this way.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
Acked-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> ---
> drivers/gpu/drm/udl/Kconfig | 1 +
> drivers/gpu/drm/udl/udl_drv.c | 29 ++-------------
> drivers/gpu/drm/udl/udl_drv.h | 1 +
> drivers/gpu/drm/udl/udl_fb.c | 66 +++++++++++++++++++----------------
> drivers/gpu/drm/udl/udl_gem.c | 61 +++++++++++++++++++++++++++++---
> 5 files changed, 98 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/gpu/drm/udl/Kconfig b/drivers/gpu/drm/udl/Kconfig
> index b4d179b87f01..145b2a95ce58 100644
> --- a/drivers/gpu/drm/udl/Kconfig
> +++ b/drivers/gpu/drm/udl/Kconfig
> @@ -5,6 +5,7 @@ config DRM_UDL
> depends on USB_SUPPORT
> depends on USB_ARCH_HAS_HCD
> select USB
> + select DRM_GEM_SHMEM_HELPER
> select DRM_KMS_HELPER
> help
> This is a KMS driver for the USB displaylink video adapters.
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index 778a0b652f64..563cc5809e56 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -8,6 +8,7 @@
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_file.h>
> +#include <drm/drm_gem_shmem_helper.h>
> #include <drm/drm_ioctl.h>
> #include <drm/drm_probe_helper.h>
> #include <drm/drm_print.h>
> @@ -32,23 +33,7 @@ static int udl_usb_resume(struct usb_interface *interface)
> return 0;
> }
>
> -static const struct vm_operations_struct udl_gem_vm_ops = {
> - .fault = udl_gem_fault,
> - .open = drm_gem_vm_open,
> - .close = drm_gem_vm_close,
> -};
> -
> -static const struct file_operations udl_driver_fops = {
> - .owner = THIS_MODULE,
> - .open = drm_open,
> - .mmap = udl_drm_gem_mmap,
> - .poll = drm_poll,
> - .read = drm_read,
> - .unlocked_ioctl = drm_ioctl,
> - .release = drm_release,
> - .compat_ioctl = drm_compat_ioctl,
> - .llseek = noop_llseek,
> -};
> +DEFINE_DRM_GEM_FOPS(udl_driver_fops);
>
> static void udl_driver_release(struct drm_device *dev)
> {
> @@ -63,18 +48,10 @@ static struct drm_driver driver = {
> .release = udl_driver_release,
>
> /* gem hooks */
> - .gem_free_object_unlocked = udl_gem_free_object,
> .gem_create_object = udl_driver_gem_create_object,
> - .gem_vm_ops = &udl_gem_vm_ops,
>
> - .dumb_create = udl_dumb_create,
> - .dumb_map_offset = udl_gem_mmap,
> .fops = &udl_driver_fops,
> -
> - .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> - .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> - .gem_prime_export = udl_gem_prime_export,
> - .gem_prime_import = udl_gem_prime_import,
> + DRM_GEM_SHMEM_DRIVER_OPS,
>
> .name = DRIVER_NAME,
> .desc = DRIVER_DESC,
> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
> index fc312e791d18..630e64abc986 100644
> --- a/drivers/gpu/drm/udl/udl_drv.h
> +++ b/drivers/gpu/drm/udl/udl_drv.h
> @@ -85,6 +85,7 @@ struct udl_gem_object {
> struct udl_framebuffer {
> struct drm_framebuffer base;
> struct udl_gem_object *obj;
> + struct drm_gem_shmem_object *shmem;
> bool active_16; /* active on the 16-bit channel */
> };
>
> diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
> index ef3504d06343..f8153b726343 100644
> --- a/drivers/gpu/drm/udl/udl_fb.c
> +++ b/drivers/gpu/drm/udl/udl_fb.c
> @@ -15,6 +15,7 @@
> #include <drm/drm_drv.h>
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_fourcc.h>
> +#include <drm/drm_gem_shmem_helper.h>
> #include <drm/drm_modeset_helper.h>
>
> #include "udl_drv.h"
> @@ -94,16 +95,14 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
> if (!fb->active_16)
> return 0;
>
> - if (!fb->obj->vmapping) {
> - ret = udl_gem_vmap(fb->obj);
> - if (ret == -ENOMEM) {
> + if (!fb->shmem->vaddr) {
> + void *vaddr;
> +
> + vaddr = drm_gem_shmem_vmap(&fb->shmem->base);
> + if (IS_ERR(vaddr)) {
> DRM_ERROR("failed to vmap fb\n");
> return 0;
> }
> - if (!fb->obj->vmapping) {
> - DRM_ERROR("failed to vmapping\n");
> - return 0;
> - }
> }
>
> aligned_x = DL_ALIGN_DOWN(x, sizeof(unsigned long));
> @@ -127,7 +126,7 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
> const int byte_offset = line_offset + (x << log_bpp);
> const int dev_byte_offset = (fb->base.width * i + x) << log_bpp;
> if (udl_render_hline(dev, log_bpp, &urb,
> - (char *) fb->obj->vmapping,
> + (char *) fb->shmem->vaddr,
> &cmd, byte_offset, dev_byte_offset,
> width << log_bpp,
> &bytes_identical, &bytes_sent))
> @@ -281,6 +280,7 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb,
> unsigned num_clips)
> {
> struct udl_framebuffer *ufb = to_udl_fb(fb);
> + struct dma_buf_attachment *import_attach;
> int i;
> int ret = 0;
>
> @@ -289,8 +289,10 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb,
> if (!ufb->active_16)
> goto unlock;
>
> - if (ufb->obj->base.import_attach) {
> - ret = dma_buf_begin_cpu_access(ufb->obj->base.import_attach->dmabuf,
> + import_attach = ufb->shmem->base.import_attach;
> +
> + if (import_attach) {
> + ret = dma_buf_begin_cpu_access(import_attach->dmabuf,
> DMA_FROM_DEVICE);
> if (ret)
> goto unlock;
> @@ -304,10 +306,9 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb,
> break;
> }
>
> - if (ufb->obj->base.import_attach) {
> - ret = dma_buf_end_cpu_access(ufb->obj->base.import_attach->dmabuf,
> + if (import_attach)
> + ret = dma_buf_end_cpu_access(import_attach->dmabuf,
> DMA_FROM_DEVICE);
> - }
>
> unlock:
> drm_modeset_unlock_all(fb->dev);
> @@ -319,8 +320,8 @@ static void udl_user_framebuffer_destroy(struct drm_framebuffer *fb)
> {
> struct udl_framebuffer *ufb = to_udl_fb(fb);
>
> - if (ufb->obj)
> - drm_gem_object_put_unlocked(&ufb->obj->base);
> + if (ufb->shmem)
> + drm_gem_object_put_unlocked(&ufb->shmem->base);
>
> drm_framebuffer_cleanup(fb);
> kfree(ufb);
> @@ -336,11 +337,11 @@ static int
> udl_framebuffer_init(struct drm_device *dev,
> struct udl_framebuffer *ufb,
> const struct drm_mode_fb_cmd2 *mode_cmd,
> - struct udl_gem_object *obj)
> + struct drm_gem_shmem_object *shmem)
> {
> int ret;
>
> - ufb->obj = obj;
> + ufb->shmem = shmem;
> drm_helper_mode_fill_fb_struct(dev, &ufb->base, mode_cmd);
> ret = drm_framebuffer_init(dev, &ufb->base, &udlfb_funcs);
> return ret;
> @@ -356,7 +357,8 @@ static int udlfb_create(struct drm_fb_helper *helper,
> struct fb_info *info;
> struct drm_framebuffer *fb;
> struct drm_mode_fb_cmd2 mode_cmd;
> - struct udl_gem_object *obj;
> + struct drm_gem_shmem_object *shmem;
> + void *vaddr;
> uint32_t size;
> int ret = 0;
>
> @@ -373,12 +375,15 @@ static int udlfb_create(struct drm_fb_helper *helper,
> size = mode_cmd.pitches[0] * mode_cmd.height;
> size = ALIGN(size, PAGE_SIZE);
>
> - obj = udl_gem_alloc_object(dev, size);
> - if (!obj)
> + shmem = drm_gem_shmem_create(dev, size);
> + if (IS_ERR(shmem)) {
> + ret = PTR_ERR(shmem);
> goto out;
> + }
>
> - ret = udl_gem_vmap(obj);
> - if (ret) {
> + vaddr = drm_gem_shmem_vmap(&shmem->base);
> + if (IS_ERR(vaddr)) {
> + ret = PTR_ERR(vaddr);
> DRM_ERROR("failed to vmap fb\n");
> goto out_gfree;
> }
> @@ -389,7 +394,7 @@ static int udlfb_create(struct drm_fb_helper *helper,
> goto out_gfree;
> }
>
> - ret = udl_framebuffer_init(dev, &ufbdev->ufb, &mode_cmd, obj);
> + ret = udl_framebuffer_init(dev, &ufbdev->ufb, &mode_cmd, shmem);
> if (ret)
> goto out_gfree;
>
> @@ -397,20 +402,20 @@ static int udlfb_create(struct drm_fb_helper *helper,
>
> ufbdev->helper.fb = fb;
>
> - info->screen_base = ufbdev->ufb.obj->vmapping;
> + info->screen_base = vaddr;
> info->fix.smem_len = size;
> - info->fix.smem_start = (unsigned long)ufbdev->ufb.obj->vmapping;
> + info->fix.smem_start = (unsigned long)vaddr;
>
> info->fbops = &udlfb_ops;
> drm_fb_helper_fill_info(info, &ufbdev->helper, sizes);
>
> DRM_DEBUG_KMS("allocated %dx%d vmal %p\n",
> fb->width, fb->height,
> - ufbdev->ufb.obj->vmapping);
> + ufbdev->ufb.shmem->vaddr);
>
> return ret;
> out_gfree:
> - drm_gem_object_put_unlocked(&ufbdev->ufb.obj->base);
> + drm_gem_object_put_unlocked(&ufbdev->ufb.shmem->base);
> out:
> return ret;
> }
> @@ -424,10 +429,10 @@ static void udl_fbdev_destroy(struct drm_device *dev,
> {
> drm_fb_helper_unregister_fbi(&ufbdev->helper);
> drm_fb_helper_fini(&ufbdev->helper);
> - if (ufbdev->ufb.obj) {
> + if (ufbdev->ufb.shmem) {
> drm_framebuffer_unregister_private(&ufbdev->ufb.base);
> drm_framebuffer_cleanup(&ufbdev->ufb.base);
> - drm_gem_object_put_unlocked(&ufbdev->ufb.obj->base);
> + drm_gem_object_put_unlocked(&ufbdev->ufb.shmem->base);
> }
> }
>
> @@ -518,7 +523,8 @@ udl_fb_user_fb_create(struct drm_device *dev,
> if (ufb == NULL)
> return ERR_PTR(-ENOMEM);
>
> - ret = udl_framebuffer_init(dev, ufb, mode_cmd, to_udl_bo(obj));
> + ret = udl_framebuffer_init(dev, ufb, mode_cmd,
> + to_drm_gem_shmem_obj(obj));
> if (ret) {
> kfree(ufb);
> return ERR_PTR(-EINVAL);
> diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
> index 628749cc1143..3554fffbf1db 100644
> --- a/drivers/gpu/drm/udl/udl_gem.c
> +++ b/drivers/gpu/drm/udl/udl_gem.c
> @@ -7,11 +7,60 @@
> #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 void udl_gem_object_free_object(struct drm_gem_object *obj)
> +{
> + struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> +
> + /* Fbdev emulation vmaps the buffer. Unmap it here for consistency
> + * with the original udl GEM code.
> + *
> + * TODO: Switch to generic fbdev emulation and release the
> + * GEM object with drm_gem_shmem_free_object().
> + */
> + if (shmem->vaddr)
> + drm_gem_shmem_vunmap(obj, shmem->vaddr);
> +
> + drm_gem_shmem_free_object(obj);
> +}
> +
> +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 const struct drm_gem_object_funcs udl_gem_object_funcs = {
> + .free = udl_gem_object_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 = drm_gem_shmem_vmap,
> + .vunmap = drm_gem_shmem_vunmap,
> + .mmap = udl_gem_object_mmap,
> +};
> +
> /*
> * Helpers for struct drm_driver
> */
> @@ -19,13 +68,17 @@
> struct drm_gem_object *udl_driver_gem_create_object(struct drm_device *dev,
> size_t size)
> {
> - struct udl_gem_object *obj;
> + struct drm_gem_shmem_object *shmem;
> + struct drm_gem_object *obj;
>
> - obj = kzalloc(sizeof(*obj), GFP_KERNEL);
> - if (!obj)
> + shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
> + if (!shmem)
> return NULL;
>
> - return &obj->base;
> + obj = &shmem->base;
> + obj->funcs = &udl_gem_object_funcs;
> +
> + return obj;
> }
>
> struct udl_gem_object *udl_gem_alloc_object(struct drm_device *dev,
> --
> 2.23.0
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
