Message-ID: <CAKMK7uFQ1qSaKyn4qjVmkbFcO+kbMQ90wKcxO49DhkVEM_PpWw@mail.gmail.com>
Date:   Fri, 26 Aug 2016 16:40:46 +0200
From:   Daniel Vetter <daniel@...ll.ch>
To:     Noralf Trønnes <noralf@...nnes.org>
Cc:     dri-devel <dri-devel@...ts.freedesktop.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 3/5] drm: add SimpleDRM driver

On Fri, Aug 26, 2016 at 12:11 AM, Noralf Trønnes <noralf@...nnes.org> wrote:
> I'm really walking in the dark here. I have deleted drm_driver.gem_vm_ops
> and used this function:
>
> int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma)
> {
>     struct drm_file *priv = filp->private_data;
>     struct drm_device *dev = priv->minor->dev;
>     struct drm_gem_object *obj = NULL;
>     struct drm_vma_offset_node *node;
>     int ret;
>
>     drm_vma_offset_lock_lookup(dev->vma_offset_manager);
>     node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
>                           vma->vm_pgoff,
>                           vma_pages(vma));
>     if (likely(node)) {
>         obj = container_of(node, struct drm_gem_object, vma_node);
>         /*
>         * When the object is being freed, after it hits 0-refcnt it
>         * proceeds to tear down the object. In the process it will
>         * attempt to remove the VMA offset and so acquire this
>         * mgr->vm_lock.  Therefore if we find an object with a 0-refcnt
>         * that matches our range, we know it is in the process of being
>         * destroyed and will be freed as soon as we release the lock -
>         * so we have to check for the 0-refcnted object and treat it as
>         * invalid.
>         */
>         if (!kref_get_unless_zero(&obj->refcount))
>             obj = NULL;
>     }
>     drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
>
>     if (!obj)
>         return -EINVAL;
>
>     if (!drm_vma_node_is_allowed(node, filp)) {
>         drm_gem_object_unreference_unlocked(obj);
>         return -EACCES;
>     }
>
>     /* redirect to shmem mmap */
>     vma->vm_file = obj->filp;
>     vma->vm_pgoff = 0;
>
>     ret = obj->filp->f_op->mmap(obj->filp, vma);
>
>     drm_gem_object_unreference_unlocked(obj);
>
>     return ret;
> }

Yup, this looks good.

> But that works only partly. Using modetest I get a picture, but fbdev
> doesn't return.
> Turning on drm debug the two variants are identical up to
> DRM_IOCTL_MODE_DESTROY_DUMB.
>
> The shmem mmap version:
> [identical]
> [   74.939660] [drm:drm_ioctl] pid=721, dev=0xe200, auth=1,
> DRM_IOCTL_MODE_DESTROY_DUMB
> And nothing more

Hm, what does your fbdev support code now look like? fbdev doesn't do
paging, so for that you still need the get_pages(); vmap; thing at
setup time, and then when you tear down the fbdev stuff a vunmap();
and put_pages();

And of course the dirty stuff all needs to be wired up, but since it
works for modeset I think we can assume it's in good shape.
-Daniel

> Whereas the working one gives me this:
> [identical]
> [   70.373029] [drm:drm_ioctl] pid=721, dev=0xe200, auth=1,
> DRM_IOCTL_MODE_DESTROY_DUMB
> [   70.393401] [drm:drm_release] open_count = 1
> [   70.393429] [drm:drm_release] pid = 721, device = 0xe200, open_count = 1
> [   70.393468] [drm:drm_lastclose]
> [   70.393501] [drm:drm_atomic_state_init] Allocated atomic state dad61e00
> [   70.393521] [drm:drm_atomic_get_plane_state] Added [PLANE:24:plane-0]
> dad61e40 state to dad61e00
> [   70.393543] [drm:drm_atomic_get_crtc_state] Added [CRTC:25:crtc-0]
> dad73a00 state to dad61e00
> [   70.393588] [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1824x984] for
> CRTC state dad73a00
> [   70.393604] [drm:drm_atomic_set_crtc_for_plane] Link plane state dad61e40
> to [CRTC:25:crtc-0]
> [   70.393619] [drm:drm_mode_object_reference] OBJ ID: 29 (1)
> [   70.393629] [drm:drm_atomic_set_fb_for_plane] Set [FB:29] for plane state
> dad61e40
> [   70.393643] [drm:drm_atomic_add_affected_connectors] Adding all current
> connectors for [CRTC:25:crtc-0] to dad61e00
> [   70.393662] [drm:drm_mode_object_reference] OBJ ID: 23 (2)
> [   70.393674] [drm:drm_atomic_get_connector_state] Added [CONNECTOR:23]
> dad613c0 state to dad61e00
> [   70.393835] [drm:drm_mode_object_reference] OBJ ID: 23 (3)
> [   70.393848] [drm:drm_atomic_set_crtc_for_connector] Link connector state
> dad613c0 to [CRTC:25:crtc-0]
> [   70.393859] [drm:drm_atomic_check_only] checking dad61e00
> [   70.393873] [drm:drm_atomic_helper_check_modeset] [CRTC:25:crtc-0] mode
> changed
> [   70.393881] [drm:drm_atomic_helper_check_modeset] [CRTC:25:crtc-0] enable
> changed
> [   70.403886] [drm:update_connector_routing] Updating routing for
> [CONNECTOR:23:Virtual-1]
> [   70.403916] [drm:update_connector_routing] [CONNECTOR:23:Virtual-1] using
> [ENCODER:26:None-26] on [CRTC:25:crtc-0]
> [   70.403926] [drm:drm_atomic_helper_check_modeset] [CRTC:25:crtc-0] active
> changed
> [   70.403956] [drm:drm_atomic_helper_check_modeset] [CRTC:25:crtc-0] needs
> all connectors, enable: y, active: y
> [   70.403972] [drm:drm_atomic_add_affected_connectors] Adding all current
> connectors for [CRTC:25:crtc-0] to dad61e00
> [   70.404006] [drm:drm_atomic_commit] commiting dad61e00
> [   70.404043] [drm:crtc_set_mode] modeset on [ENCODER:26:None-26]
> [   70.422427] [drm:drm_atomic_helper_commit_modeset_enables] enabling
> [CRTC:25:crtc-0]
> [   70.422465] [drm:drm_atomic_helper_commit_modeset_enables] enabling
> [ENCODER:26:None-26]
> [   70.422490] [drm:drm_atomic_state_default_clear] Clearing atomic state
> dad61e00
> [   70.422504] [drm:drm_mode_object_unreference] OBJ ID: 23 (4)
> [   70.422519] [drm:drm_atomic_state_free] Freeing atomic state dad61e00
> [   70.422532] [drm:drm_mode_object_reference] OBJ ID: 29 (2)
> [   70.422546] [drm:drm_lastclose] driver lastclose completed
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Powered by blists - more mailing lists