[PATCH V2 1/5] drm/vkms: Add dumb operations
Daniel Vetter
daniel at ffwll.ch
Wed Jul 11 15:05:39 UTC 2018
On Wed, Jul 11, 2018 at 4:21 PM, Rodrigo Siqueira
<rodrigosiqueiramelo at gmail.com> wrote:
> On 07/11, Daniel Vetter wrote:
>> On Thu, Jul 05, 2018 at 11:21:19PM +0300, Haneen Mohammed wrote:
>> > On Thu, Jun 21, 2018 at 09:16:13AM -0300, Rodrigo Siqueira wrote:
>> > > VKMS currently does not handle dumb data, and as a consequence, it does
>> > > not provide mechanisms for handling gem. This commit adds the necessary
>> > > support for gem object/handler and the dumb functions.
>> > >
>> > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo at gmail.com>
>> > > ---
>> >
>> > This looks good to me except with missing gem_free_object_unlocked callback,
>> > which causes warning: Memory manager not clean during takedown.
>> >
>> > Maybe it will be easier if we add this in another patch instead of creating v3 of
>> > this patchset?
>>
>> Ah this is the patch series that didn't land yet ...
>
> Hi, thanks for all the feedbacks.
>
>> Rodrigo, can you pls respin with the issue fixed that Haneen spotted?
>
> Yes,
>
> I will prepare a new version of this patchset based on all the comments.
> Just one question, should I create a new version of the patchset that
> includes the patch related with the hrtimer simulation or keep it
> separate?
Since the hrtimer stuff doesn't apply without this prep work I'd say
all together. Separate patch series only makes sense if it's truly
separate stuff, without any dependencies between the 2 patch series.
-Daniel
>
>> Haneen, can you pls review the other patches in this series too?
>>
>> Also, do we have any other patch series that's not yet applied? With two
>> interns working on the same thing it's a bit harder to keep track of stuff
>> ...
>>
>> Thanks, Daniel
>>
>> >
>> > > drivers/gpu/drm/vkms/Makefile | 2 +-
>> > > drivers/gpu/drm/vkms/vkms_drv.c | 9 ++
>> > > drivers/gpu/drm/vkms/vkms_drv.h | 21 ++++
>> > > drivers/gpu/drm/vkms/vkms_gem.c | 168 ++++++++++++++++++++++++++++++++
>> > > 4 files changed, 199 insertions(+), 1 deletion(-)
>> > > create mode 100644 drivers/gpu/drm/vkms/vkms_gem.c
>> > >
>> > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
>> > > index 3f774a6a9c58..986297da51bf 100644
>> > > --- a/drivers/gpu/drm/vkms/Makefile
>> > > +++ b/drivers/gpu/drm/vkms/Makefile
>> > > @@ -1,3 +1,3 @@
>> > > -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o
>> > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
>> > >
>> > > obj-$(CONFIG_DRM_VKMS) += vkms.o
>> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
>> > > index 740a4cbfed91..638bab9083b5 100644
>> > > --- a/drivers/gpu/drm/vkms/vkms_drv.c
>> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
>> > > @@ -37,6 +37,12 @@ static const struct file_operations vkms_driver_fops = {
>> > > .release = drm_release,
>> > > };
>> > >
>> > > +static const struct vm_operations_struct vkms_gem_vm_ops = {
>> > > + .fault = vkms_gem_fault,
>> > > + .open = drm_gem_vm_open,
>> > > + .close = drm_gem_vm_close,
>> > > +};
>> > > +
>> > > static void vkms_release(struct drm_device *dev)
>> > > {
>> > > struct vkms_device *vkms = container_of(dev, struct vkms_device, drm);
>> > > @@ -50,6 +56,9 @@ static struct drm_driver vkms_driver = {
>> > > .driver_features = DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
>> > > .release = vkms_release,
>> > > .fops = &vkms_driver_fops,
>> > > + .dumb_create = vkms_dumb_create,
>> > > + .dumb_map_offset = vkms_dumb_map,
>> > > + .gem_vm_ops = &vkms_gem_vm_ops,
>> > >
>> > > .name = DRIVER_NAME,
>> > > .desc = DRIVER_DESC,
>> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
>> > > index b0f9d2e61a42..54bb3dd2b2c1 100644
>> > > --- a/drivers/gpu/drm/vkms/vkms_drv.h
>> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
>> > > @@ -3,6 +3,7 @@
>> > >
>> > > #include <drm/drmP.h>
>> > > #include <drm/drm.h>
>> > > +#include <drm/drm_gem.h>
>> > > #include <drm/drm_encoder.h>
>> > >
>> > > static const u32 vkms_formats[] = {
>> > > @@ -21,6 +22,12 @@ struct vkms_device {
>> > > struct vkms_output output;
>> > > };
>> > >
>> > > +struct vkms_gem_object {
>> > > + struct drm_gem_object gem;
>> > > + struct mutex pages_lock; /* Page lock used in page fault handler */
>> > > + struct page **pages;
>> > > +};
>> > > +
>> > > int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>> > > struct drm_plane *primary, struct drm_plane *cursor);
>> > >
>> > > @@ -28,4 +35,18 @@ int vkms_output_init(struct vkms_device *vkmsdev);
>> > >
>> > > struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);
>> > >
>> > > +/* Gem stuff */
>> > > +struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
>> > > + struct drm_file *file,
>> > > + u32 *handle,
>> > > + u64 size);
>> > > +
>> > > +int vkms_gem_fault(struct vm_fault *vmf);
>> > > +
>> > > +int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
>> > > + struct drm_mode_create_dumb *args);
>> > > +
>> > > +int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,
>> > > + u32 handle, u64 *offset);
>> > > +
>> > > #endif /* _VKMS_DRV_H_ */
>> > > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
>> > > new file mode 100644
>> > > index 000000000000..9f820f56b9e0
>> > > --- /dev/null
>> > > +++ b/drivers/gpu/drm/vkms/vkms_gem.c
>> > > @@ -0,0 +1,168 @@
>> > > +// SPDX-License-Identifier: GPL-2.0
>> > > +/*
>> > > + * This program is free software; you can redistribute it and/or modify
>> > > + * it under the terms of the GNU General Public License as published by
>> > > + * the Free Software Foundation; either version 2 of the License, or
>> > > + * (at your option) any later version.
>> > > + */
>> > > +
>> > > +#include <linux/shmem_fs.h>
>> > > +
>> > > +#include "vkms_drv.h"
>> > > +
>> > > +static struct vkms_gem_object *__vkms_gem_create(struct drm_device *dev,
>> > > + u64 size)
>> > > +{
>> > > + struct vkms_gem_object *obj;
>> > > + int ret;
>> > > +
>> > > + obj = kzalloc(sizeof(*obj), GFP_KERNEL);
>> > > + if (!obj)
>> > > + return ERR_PTR(-ENOMEM);
>> > > +
>> > > + size = roundup(size, PAGE_SIZE);
>> > > + ret = drm_gem_object_init(dev, &obj->gem, size);
>> > > + if (ret) {
>> > > + kfree(obj);
>> > > + return ERR_PTR(ret);
>> > > + }
>> > > +
>> > > + mutex_init(&obj->pages_lock);
>> > > +
>> > > + return obj;
>> > > +}
>> > > +
>> > > +int vkms_gem_fault(struct vm_fault *vmf)
>> > > +{
>> > > + struct vm_area_struct *vma = vmf->vma;
>> > > + struct vkms_gem_object *obj = vma->vm_private_data;
>> > > + unsigned long vaddr = vmf->address;
>> > > + pgoff_t page_offset;
>> > > + loff_t num_pages;
>> > > + int ret;
>> > > +
>> > > + page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT;
>> > > + num_pages = DIV_ROUND_UP(obj->gem.size, PAGE_SIZE);
>> > > +
>> > > + if (page_offset > num_pages)
>> > > + return VM_FAULT_SIGBUS;
>> > > +
>> > > + ret = -ENOENT;
>> > > + mutex_lock(&obj->pages_lock);
>> > > + if (obj->pages) {
>> > > + get_page(obj->pages[page_offset]);
>> > > + vmf->page = obj->pages[page_offset];
>> > > + ret = 0;
>> > > + }
>> > > + mutex_unlock(&obj->pages_lock);
>> > > + if (ret) {
>> > > + struct page *page;
>> > > + struct address_space *mapping;
>> > > +
>> > > + mapping = file_inode(obj->gem.filp)->i_mapping;
>> > > + page = shmem_read_mapping_page(mapping, page_offset);
>> > > +
>> > > + if (!IS_ERR(page)) {
>> > > + vmf->page = page;
>> > > + ret = 0;
>> > > + } else {
>> > > + switch (PTR_ERR(page)) {
>> > > + case -ENOSPC:
>> > > + case -ENOMEM:
>> > > + ret = VM_FAULT_OOM;
>> > > + break;
>> > > + case -EBUSY:
>> > > + ret = VM_FAULT_RETRY;
>> > > + break;
>> > > + case -EFAULT:
>> > > + case -EINVAL:
>> > > + ret = VM_FAULT_SIGBUS;
>> > > + break;
>> > > + default:
>> > > + WARN_ON(PTR_ERR(page));
>> > > + ret = VM_FAULT_SIGBUS;
>> > > + break;
>> > > + }
>> > > + }
>> > > + }
>> > > + return ret;
>> > > +}
>> > > +
>> > > +struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
>> > > + struct drm_file *file,
>> > > + u32 *handle,
>> > > + u64 size)
>> > > +{
>> > > + struct vkms_gem_object *obj;
>> > > + int ret;
>> > > +
>> > > + if (!file || !dev || !handle)
>> > > + return ERR_PTR(-EINVAL);
>> > > +
>> > > + obj = __vkms_gem_create(dev, size);
>> > > + if (IS_ERR(obj))
>> > > + return ERR_CAST(obj);
>> > > +
>> > > + ret = drm_gem_handle_create(file, &obj->gem, handle);
>> > > + drm_gem_object_put_unlocked(&obj->gem);
>> > > + if (ret) {
>> > > + drm_gem_object_release(&obj->gem);
>> > > + kfree(obj);
>> > > + return ERR_PTR(ret);
>> > > + }
>> > > +
>> > > + return &obj->gem;
>> > > +}
>> > > +
>> > > +int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
>> > > + struct drm_mode_create_dumb *args)
>> > > +{
>> > > + struct drm_gem_object *gem_obj;
>> > > + u64 pitch, size;
>> > > +
>> > > + if (!args || !dev || !file)
>> > > + return -EINVAL;
>> > > +
>> > > + pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
>> > > + size = pitch * args->height;
>> > > +
>> > > + if (!size)
>> > > + return -EINVAL;
>> > > +
>> > > + gem_obj = vkms_gem_create(dev, file, &args->handle, size);
>> > > + if (IS_ERR(gem_obj))
>> > > + return PTR_ERR(gem_obj);
>> > > +
>> > > + args->size = gem_obj->size;
>> > > + args->pitch = pitch;
>> > > +
>> > > + DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
>> > > +
>> > > + return 0;
>> > > +}
>> > > +
>> > > +int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,
>> > > + u32 handle, u64 *offset)
>> > > +{
>> > > + struct drm_gem_object *obj;
>> > > + int ret;
>> > > +
>> > > + obj = drm_gem_object_lookup(file, handle);
>> > > + if (!obj)
>> > > + return -ENOENT;
>> > > +
>> > > + if (!obj->filp) {
>> > > + ret = -EINVAL;
>> > > + goto unref;
>> > > + }
>> > > +
>> > > + ret = drm_gem_create_mmap_offset(obj);
>> > > + if (ret)
>> > > + goto unref;
>> > > +
>> > > + *offset = drm_vma_node_offset_addr(&obj->vma_node);
>> > > +unref:
>> > > + drm_gem_object_put_unlocked(obj);
>> > > +
>> > > + return ret;
>> > > +}
>> > > --
>> > > 2.17.1
>> > >
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the dri-devel
mailing list