Re: [PATCH v2 00/11] drm/tinydrm: Support device unplug

[Oleksandr Andrushchenko <andr2000@gmail.com>]

On 03/19/2018 03:45 PM, Daniel Vetter wrote:
> On Sat, Mar 17, 2018 at 03:40:29PM +0100, Noralf Trønnes wrote:
>> Den 16.03.2018 09.03, skrev Daniel Vetter:
>>> On Fri, Sep 8, 2017 at 6:33 PM, Daniel Vetter<daniel@ffwll.ch>  wrote:
>>>> Hi Noralf,
>>>>
>>>> On Fri, Sep 08, 2017 at 05:07:19PM +0200, Noralf Trønnes wrote:
>>>>> This adds device unplug support to drm_fb_helper, drm_fb_cma_helper
>>>>> (fbdev) and tinydrm.
>>>>>
>>>>> There are several changes in this version:
>>>>>
>>>>> I've used Daniel's idea of protecting drm_device.unplugged with srcu to
>>>>> provide race free drm_dev_unplug().
>>>>>
>>>>> The fbdev helper unplug patch has become very small with Daniel's help.
>>>>> Ref is now taken and dropped in the existing helpers, so I could drop
>>>>> drm_fb_helper_simple_init().
>>>>>
>>>>> I has annoyed me that fbdev is restored in drm_driver.last_close even if
>>>>> fbdev isn't used. I've added a patch to fix that. This means I can drop
>>>>> calling drm_atomic_helper_shutdown() in tinydrm_unregister(), since I
>>>>> now can easily disable the pipeline from userspace by just closing the
>>>>> users. Disabled pipeline means balanced regulator_enable/disable.
>>>>>
>>>>> The 'Embed drm_device in tinydrm_device' patch has gained
>>>>> drm_driver.release functions after a discussion with Laurent. My
>>>>> previous version relied on obscure freeing in tinydrm_release().
>>>>> This means that I didn't retain the ack's.
>>>>>
>>>>> Laurent also caught an ugly devm_kmalloc() in
>>>>> tinydrm_display_pipe_init() that I've fixed.
>>>> I'm practically packing for my 2 weeks of conference travel already, so
>>>> only very cursory read of the initial patches for core&fb-helpers. I think
>>>> this looks really splendid now.
>>>>
>>>> But I won't have time for review for the next few week, would be good if
>>>> you could drag some others into this discussions. Iirc there's recently
>>>> been a few different people interested in udl (even some patches I think),
>>>> they might be able to help out with testing&review.
>>>>
>>>> Also, would be great if you can submit this to intel-gfx on the next
>>>> round, so that our CI can pick it up and give it a solid beating. Touching
>>>> critical core paths like in patch 1 is always a bit scary.
>>> While reviewing xen-front's hotunplug handling I just realized this
>>> never landed. Can you pls resend and nag me to review it properly? I'd
>>> really like to get the drm_dev_unplug stuff sorted out better.
>> My plan was to pick this up after switching tinydrm over to vmalloc buffers,
>> but that work is now waiting for the generic fbdev emulation to land.
>>
>> I'm currently wandering around inside drm_fb_helper looking for a way out,
>> I can feel the draft so there has to be an exit somewhere. I hope that in
>> a week or two I'm done with the next version of the RFC using the
>> drm_fb_helper mode setting code instead of the ioctl's.
>>
>> At that point it will be a good thing to flush my "caches" of the
>> drm_fb_helper code, since after looking at it for a long time, I really
>> don't see the details anymore. So I'll pick up the unplug series then, at
>> least the core patches should be trivial to review and get merged if the CI
>> agrees.
> Sounds great. I chatted some more with Oleksandr on irc and explained how
> he can at least prototype correct unplug code using the current upstream
> stuff. He's also willing to help get your stuff landed I think.
Yes, I can try helping with this - please let me know
if you need something.
And at least I can plumb that into my driver and have it tested.
>   And afair
> the unplug stuff pretty much looked ready for merging already, at least I
> don't remember anything big pending.
> -Daniel
>
>> Noralf.
>>
>>> Thanks, Daniel
>>>
>>>> Thanks, Daniel
>>>>
>>>>> Noralf.
>>>>>
>>>>> Noralf Trønnes (11):
>>>>>     drm: Use srcu to protect drm_device.unplugged
>>>>>     drm/fb-helper: Support device unplug
>>>>>     drm/fb-helper: Ensure driver module is pinned in fb_open()
>>>>>     drm/fb-helper: Don't restore if fbdev is not in use
>>>>>     drm/fb-cma-helper: Make struct drm_fbdev_cma public
>>>>>     drm/fb-cma-helper: Support device unplug
>>>>>     drm/tinydrm: Drop driver registered message
>>>>>     drm/tinydrm: Embed drm_device in tinydrm_device
>>>>>     drm/tinydrm: Support device unplug in core
>>>>>     drm/tinydrm/mi0283qt: Let the display pipe handle power
>>>>>     drm/tinydrm: Support device unplug in drivers
>>>>>
>>>>>    drivers/gpu/drm/drm_drv.c                   |  54 +++++++++--
>>>>>    drivers/gpu/drm/drm_fb_cma_helper.c         |  13 +--
>>>>>    drivers/gpu/drm/drm_fb_helper.c             | 108 ++++++++++++++++++++--
>>>>>    drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 135 +++++++++++++++++++---------
>>>>>    drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  28 +++---
>>>>>    drivers/gpu/drm/tinydrm/mi0283qt.c          |  81 +++++------------
>>>>>    drivers/gpu/drm/tinydrm/mipi-dbi.c          |  58 +++++++++---
>>>>>    drivers/gpu/drm/tinydrm/repaper.c           |  62 ++++++++-----
>>>>>    drivers/gpu/drm/tinydrm/st7586.c            |  54 ++++++-----
>>>>>    include/drm/drm_device.h                    |   9 +-
>>>>>    include/drm/drm_drv.h                       |  15 +++-
>>>>>    include/drm/drm_fb_cma_helper.h             |  11 ++-
>>>>>    include/drm/drm_fb_helper.h                 |  28 ++++++
>>>>>    include/drm/tinydrm/mipi-dbi.h              |   1 +
>>>>>    include/drm/tinydrm/tinydrm.h               |  10 ++-
>>>>>    15 files changed, 469 insertions(+), 198 deletions(-)
>>>>>
>>>>> --
>>>>> 2.7.4
>>>>>
>>>> --
>>>> 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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel