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

[Daniel Vetter]

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 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.

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. 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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch