Skip to content

L
linux
Switch branch/tag
  1. 01 Apr, 2020 2 commits
  3. 31 Mar, 2020 4 commits
  5. 30 Mar, 2020 5 commits
  7. 28 Mar, 2020 12 commits
  9. 27 Mar, 2020 5 commits
  11. 26 Mar, 2020 12 commits
    • Daniel Vetter's avatar
      drm: Add docs for managed resources · 9e1ed9fb
      Daniel Vetter authored 
      All collected together to provide a consistent story in one patch,
instead of the somewhat bumpy refactor-evolution leading to this.

Also some thoughts on what the next steps could be:

- Create a macro called devm_drm_dev_alloc() which essentially wraps
  the kzalloc(); devm_drm_dev_init(); drmm_add_final_kfree() combo.
  Needs to be a macro since we'll have to do some typeof trickery and
  casting to make this fully generic for all drivers that embed struct
  drm_device into their own thing.

- A lot of the simple drivers now have essentially just
  drm_dev_unplug(); drm_atomic_helper_shutdown(); as their
  $bus_driver->remove hook. We could create a devm_mode_config_reset
  which sets drm_atomic_helper_shutdown as it's cleanup action, and a
  devm_drm_dev_register with drm_dev_unplug as it's cleanup action,
  and simple drivers wouldn't have a need for a ->remove function at
  all, and we could delete them.

- For more complicated drivers we need drmm_ versions of a _lot_ more
  things. All the userspace visible objects (crtc, plane, encoder,
  crtc), anything else hanging of those (maybe a drmm_get_edid, at
  least for panels and other built-in stuff).

Also some more thoughts on why we're not reusing devm_ with maybe a
fake struct device embedded into the drm_device (we can't use the
kdev, since that's in each drm_minor).

- Code review gets extremely tricky, since every time you see a devm_
  you need to carefully check whether the fake device (with the
  drm_device lifetim) or the real device (with the lifetim of the
  underlying physical device and driver binding) are used. That's not
  going to help at all, and we have enormous amounts of drivers who
  use devm_ where they really shouldn't. Having different types makes
  sure the compiler type checks this for us and ensures correctness.

- The set of functions are very much non-overlapping. E.g.
  devm_ioremap makes total sense, drmm_ioremap has the wrong lifetime,
  since hw resources need to be cleaned out at driver unbind and wont
  outlive that like a drm_device. Similar, but other way round for
  drmm_connector_init (which is the only correct version, devm_ for
  drm_connector is just buggy). Simply not having the wrong version
  again prevents bugs.

Finally I guess this opens a huge todo for all the drivers. I'm
semi-tempted to do a tree-wide s/devm_kzalloc/drmm_kzalloc/ since most
likely that'll fix an enormous amount of bugs and most likely not
cause any issues at all (aside from maybe holding onto memory slightly
too long).

v2:
- Doc improvements from Laurent.
- Also add kerneldoc for the new drmm_add_action_or_reset.

v3:
- Remove kerneldoc for drmm_remove_action.
Reviewed-by: default avatarSam Ravnborg <sam@ravnborg.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>

fixup docs
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-52-daniel.vetter@ffwll.ch
      9e1ed9fb
    • Daniel Vetter's avatar
      drm/udl: drop drm_driver.release hook · b1a981bd
      Daniel Vetter authored 
      There's only two functions called from that:
drm_kms_helper_poll_fini() and udl_free_urb_list(). Both of these are
also called from the ubs_driver->disconnect hook, so entirely
pointless to do the same again in the ->release hook.

Furthermore by the time we clean up the drm_driver we really shouldn't
be touching hardware anymore, so stopping the poll worker and freeing
the urb allocations in ->disconnect is the right thing to do.

Now disconnect still cleans things up before unregistering the driver,
but that's a different issue.

v2: Use _fini, not _disable in unplug, motivated by discussions with
Thomas. _disable/_enable are for suspend/resume.
Acked-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Emil Velikov <emil.l.velikov@gmail.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Alex Deucher <alexander.deucher@amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-51-daniel.vetter@ffwll.ch
      b1a981bd
    • Daniel Vetter's avatar
      drm/udl: Drop explicit drm_mode_config_cleanup call · fe5b7c86
      Daniel Vetter authored 
      It's right above the drm_dev_put().

This allows us to delete a bit of onion unwinding in
udl_modeset_init().

This is made possible by a preceeding patch which added a drmm_
cleanup action to drm_mode_config_init(), hence all we need to do to
ensure that drm_mode_config_cleanup() is run on final drm_device
cleanup is check the new error code for _init().

v2: Explain why this cleanup is possible (Laurent).

v3: Use drmm_mode_config_init() for more clarity (Sam, Thomas)
Acked-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: default avatarSam Ravnborg <sam@ravnborg.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Emil Velikov <emil.l.velikov@gmail.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Sam Ravnborg <sam@ravnborg.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-50-daniel.vetter@ffwll.ch
      fe5b7c86
    • Daniel Vetter's avatar
      drm/mipi-dbi: Drop explicit drm_mode_config_cleanup call · 3421a6c4
      Daniel Vetter authored 
      Allows us to drop the drm_driver.release callback from all
drivers, and remove the mipi_dbi_release() function.

This is made possible by a preceeding patch which added a drmm_
cleanup action to drm_mode_config_init(), hence all we need to do to
ensure that drm_mode_config_cleanup() is run on final drm_device
cleanup is check the new error code for _init().

v2: Explain why this cleanup is possible (Laurent).

v3: Use drmm_mode_config_init() for more clarity (Sam, Thomas)

Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: default avatarSam Ravnborg <sam@ravnborg.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org> (v2)
Tested-by: default avatarNoralf Trønnes <noralf@tronnes.org>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Eric Anholt <eric@anholt.net>
Cc: David Lechner <david@lechnology.com>
Cc: Kamlesh Gurudasani <kamlesh.gurudasani@gmail.com>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-49-daniel.vetter@ffwll.ch
      3421a6c4
    • Daniel Vetter's avatar
      drm/mipi-dbi: Move drm_mode_config_init into mipi library · 53bdebf7
      Daniel Vetter authored 
      7/7 drivers agree that's the right choice, let's do this.

This avoids duplicating the same old error checking code over all 7
drivers, which is the motivation here.
Acked-by: default avatarSam Ravnborg <sam@ravnborg.org>
Reviewed-by: default avatarNoralf Trønnes <noralf@tronnes.org>
Tested-by: default avatarNoralf Trønnes <noralf@tronnes.org>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Eric Anholt <eric@anholt.net>
Cc: David Lechner <david@lechnology.com>
Cc: Kamlesh Gurudasani <kamlesh.gurudasani@gmail.com>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-48-daniel.vetter@ffwll.ch
      53bdebf7
    • Daniel Vetter's avatar
      drm/repaper: Drop explicit drm_mode_config_cleanup call · be9f2b03
      Daniel Vetter authored 
      Allows us to drop the drm_driver.release callback.

This is made possible by a preceeding patch which added a drmm_
cleanup action to drm_mode_config_init(), hence all we need to do to
ensure that drm_mode_config_cleanup() is run on final drm_device
cleanup is check the new error code for _init().

v2: Explain why this cleanup is possible (Laurent).

v3: Use drmm_mode_config_init() for more clarity (Sam, Thomas)
I also noticed that I've failed to add the error checking,
__must_check caught that.

Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org> (v2)
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-47-daniel.vetter@ffwll.ch
      be9f2b03
    • Daniel Vetter's avatar
      drm/gm12u320: Simplify upload work · 8f2cb937
      Daniel Vetter authored 
      Instead of having a work item that never stops (which really should be
a kthread), with a dedicated workqueue to not upset anyone else, use a
delayed work. A bunch of changes:

- We can throw out all the custom wakeup and requeue logic and state
  tracking. If we schedule the work with a 0 delay it'll get
  scheduled immediately.

- Persistent state (frame & draw_status_timeout) need to be moved out
  of the work.

- diff is bigger than the changes, biggest chunk is reindenting the
  work fn because it lost its while loop.

Lots of code deleting as consequence all over. Specifically we can
delete the drm_driver.release code now!

v2: Review from Hans:
- Use mod_delayed_work in the plane update path to make sure we do
  actually schedule immediately). In the worker we still want
  queue_delayed_work, which won't modify the timeout when the work is
  already scheduled. Which is exactly what we want if the work races
  with a plane update.
- Switch to system_long_wq, Hans says on usb2 a plane upload can take
  80 ms.
Reviewed-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-46-daniel.vetter@ffwll.ch
      8f2cb937
    • Daniel Vetter's avatar
      drm/gm12u320: Use helpers for shutdown/suspend/resume · 7ef64ed1
      Daniel Vetter authored 
      Also there's a race in the disconnect implemenation. First shut
down, then unplug, leaves a window where userspace could sneak
in and restart the entire machinery.

With this we can also delete the very un-atomic global pipe_enabled
tracking.
Reviewed-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-45-daniel.vetter@ffwll.ch
      7ef64ed1
    • Daniel Vetter's avatar
      drm/gm12u320: Use devm_drm_dev_init · 993f5b19
      Daniel Vetter authored 
      Only drops the drm_dev_put, but hey a few lines!
Reviewed-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-44-daniel.vetter@ffwll.ch
      993f5b19
    • Daniel Vetter's avatar
      drm/gm12u320: More drmm_ · 08373edc
      Daniel Vetter authored 
      The drm_mode_config_cleanup call we can drop, and all the allocations
we can switch over to drmm_kzalloc. Unfortunately the work queue is
still present, so can't get rid of the drm_driver->release function
outright.

v2: Use drmm_mode_config_init() for more clarity (Sam, Thomas)

Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Hans de Goede <hdegoede@redhat.com> (v1)
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-43-daniel.vetter@ffwll.ch
      08373edc
    • Daniel Vetter's avatar
      drm/tidss: Drop explicit drm_mode_config_cleanup call · c792098b
      Daniel Vetter authored 
      It's right above the drm_dev_put().

This is made possible by a preceeding patch which added a drmm_
cleanup action to drm_mode_config_init(), hence all we need to do to
ensure that drm_mode_config_cleanup() is run on final drm_device
cleanup is check the new error code for _init().

Aside: Another driver with a bit much devm_kzalloc, which should
probably use drmm_kzalloc instead ...

I'm pretty sure this one blows up already under KASAN because it's
using devm_drm_dev_init, and later on devm_kzalloc. Hence the memory
will get freed before the final drm_dev_put (all from the devres
code), but the cleanup in that final drm_dev_put will access the just
freed memory.

Unfortunately fixing this properly needs slightly more work, namely
drmm_ versions for all the drm objects (planes, crtc, ...), so that
the cleanup actually happens before even drmm_kzalloc would release
the underlying memory. Not quite there yet.

v2: Explain why this cleanup is possible (Laurent).

v3: Use drmm_mode_config_init() for more clarity (Sam, Thomas)

Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: default avatarJyri Sarha <jsarha@ti.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Cc: Jyri Sarha <jsarha@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-42-daniel.vetter@ffwll.ch
      c792098b
    • Daniel Vetter's avatar
      drm/mtk: Drop explicit drm_mode_config_cleanup call · 18c62222
      Daniel Vetter authored 
      It's right above the drm_dev_put().

This is made possible by a preceeding patch which added a drmm_
cleanup action to drm_mode_config_init(), hence all we need to do to
ensure that drm_mode_config_cleanup() is run on final drm_device
cleanup is check the new error code for _init().

Aside: Another driver with a bit much devm_kzalloc, which should
probably use drmm_kzalloc instead ...

v2: Explain why this cleanup is possible (Laurent).

v3: Use drmm_mode_config_init() for more clarity (Sam, Thomas)
Acked-by: default avatarChun-Kuang Hu <chunkuang.hu@kernel.org>
Acked-by: default avatarSam Ravnborg <sam@ravnborg.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-41-daniel.vetter@ffwll.ch
      18c62222
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7