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

Oleksandr Andrushchenko andr2000 at gmail.com
Tue Mar 27 05:47:51 UTC 2018 

On 03/26/2018 07:36 PM, Noralf Trønnes wrote:
>
> Den 26.03.2018 15.02, skrev Oleksandr Andrushchenko:
>> Hi, Noralf!
>>
>> On 03/17/2018 04:40 PM, 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 at 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.
>> Could you please estimate when unplug series can be on review?
>> I am doing some unplug work in my PV DRM driver and would like
>> to understand if it is feasible to wait for you or post patches as 
>> they are
>> and then plumb in drm_dev_{enter|exit} later after your work is merged
>>
>
> Ok, so I have looked at the patches and some work I have lying around.
> As things stand now I see an opportunity to move some stuff from tinydrm
> into drm_simple_kms_helper as part of adding unplug support to tinydrm.
> This also depends on the generic fbdev emulation I'm working on.
> This all means that it won't be some trivial tweaking to the unplug
> patchset before resending it. It will take some time.
>
> My suggestion is that you just add the core unplug patch as part of your
> driver submission instead of waiting for me.
>
> drm: Use srcu to protect drm_device.unplugged
> https://patchwork.freedesktop.org/patch/175779/
>
This is exactly the patch I need. Daniel, could you please
review this single patch?
> I believe this patch should be good to go as-is. Please CC
> intel-gfx at lists.freedesktop.org if you do so to have the Intel CI 
> verify it.
>
I will
> As for drm_fb_helper unplug support:
>
> drm/fb-helper: Support device unplug
> https://patchwork.freedesktop.org/patch/175785/
>
> I'm not as confident in this one since I'm not sure that those
> drm_dev_is_unplugged() checks are really necessary. The driver has to do
> the check anyways. But this patch isn't necessary for you to do most of
> your driver side unplug protection though. You can do testing without
> fbdev enabled and let me to care of this when I get around to it.
>
> I you pick up the patch(es) and need to change something, you don't have
> to bother with retaining my authorship 
I will retain
> (but please cc me).
Ok
> Just claim it
> for your own and make it work.
> Less work for me when I get there (eventually).
>
> Noralf.
>
Thank you,
Oleksandr
>> Thank you,
>> Oleksandr
>>>
>>> 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 at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>

More information about the dri-devel mailing list