Thierry Reding | 0e70dad | 2017-02-07 18:51:13 +0100 | [diff] [blame] | 1 | .. _todo: |
| 2 | |
| 3 | ========= |
| 4 | TODO list |
| 5 | ========= |
| 6 | |
| 7 | This section contains a list of smaller janitorial tasks in the kernel DRM |
| 8 | graphics subsystem useful as newbie projects. Or for slow rainy days. |
| 9 | |
| 10 | Subsystem-wide refactorings |
| 11 | =========================== |
| 12 | |
| 13 | De-midlayer drivers |
| 14 | ------------------- |
| 15 | |
| 16 | With the recent ``drm_bus`` cleanup patches for 3.17 it is no longer required |
| 17 | to have a ``drm_bus`` structure set up. Drivers can directly set up the |
| 18 | ``drm_device`` structure instead of relying on bus methods in ``drm_usb.c`` |
Daniel Vetter | 085c6c0 | 2017-04-04 11:52:54 +0200 | [diff] [blame] | 19 | and ``drm_pci.c``. The goal is to get rid of the driver's ``->load`` / |
Thierry Reding | 0e70dad | 2017-02-07 18:51:13 +0100 | [diff] [blame] | 20 | ``->unload`` callbacks and open-code the load/unload sequence properly, using |
| 21 | the new two-stage ``drm_device`` setup/teardown. |
| 22 | |
| 23 | Once all existing drivers are converted we can also remove those bus support |
| 24 | files for USB and platform devices. |
| 25 | |
| 26 | All you need is a GPU for a non-converted driver (currently almost all of |
| 27 | them, but also all the virtual ones used by KVM, so everyone qualifies). |
| 28 | |
| 29 | Contact: Daniel Vetter, Thierry Reding, respective driver maintainers |
| 30 | |
| 31 | Switch from reference/unreference to get/put |
| 32 | -------------------------------------------- |
| 33 | |
| 34 | For some reason DRM core uses ``reference``/``unreference`` suffixes for |
| 35 | refcounting functions, but kernel uses ``get``/``put`` (e.g. |
| 36 | ``kref_get``/``put()``). It would be good to switch over for consistency, and |
| 37 | it's shorter. Needs to be done in 3 steps for each pair of functions: |
| 38 | |
| 39 | * Create new ``get``/``put`` functions, define the old names as compatibility |
| 40 | wrappers |
| 41 | * Switch over each file/driver using a cocci-generated spatch. |
| 42 | * Once all users of the old names are gone, remove them. |
| 43 | |
| 44 | This way drivers/patches in the progress of getting merged won't break. |
| 45 | |
| 46 | Contact: Daniel Vetter |
| 47 | |
| 48 | Convert existing KMS drivers to atomic modesetting |
| 49 | -------------------------------------------------- |
| 50 | |
| 51 | 3.19 has the atomic modeset interfaces and helpers, so drivers can now be |
| 52 | converted over. Modern compositors like Wayland or Surfaceflinger on Android |
| 53 | really want an atomic modeset interface, so this is all about the bright |
| 54 | future. |
| 55 | |
| 56 | There is a conversion guide for atomic and all you need is a GPU for a |
| 57 | non-converted driver (again virtual HW drivers for KVM are still all |
| 58 | suitable). |
| 59 | |
| 60 | As part of this drivers also need to convert to universal plane (which means |
| 61 | exposing primary & cursor as proper plane objects). But that's much easier to |
| 62 | do by directly using the new atomic helper driver callbacks. |
| 63 | |
| 64 | Contact: Daniel Vetter, respective driver maintainers |
| 65 | |
Daniel Vetter | 1a80cc1 | 2017-02-26 20:38:50 +0100 | [diff] [blame] | 66 | Clean up the clipped coordination confusion around planes |
| 67 | --------------------------------------------------------- |
| 68 | |
| 69 | We have a helper to get this right with drm_plane_helper_check_update(), but |
| 70 | it's not consistently used. This should be fixed, preferrably in the atomic |
| 71 | helpers (and drivers then moved over to clipped coordinates). Probably the |
| 72 | helper should also be moved from drm_plane_helper.c to the atomic helpers, to |
| 73 | avoid confusion - the other helpers in that file are all deprecated legacy |
| 74 | helpers. |
| 75 | |
| 76 | Contact: Ville Syrjälä, Daniel Vetter, driver maintainers |
| 77 | |
Daniel Vetter | 4e8be45 | 2017-02-26 20:38:51 +0100 | [diff] [blame] | 78 | Implement deferred fbdev setup in the helper |
| 79 | -------------------------------------------- |
| 80 | |
| 81 | Many (especially embedded drivers) want to delay fbdev setup until there's a |
| 82 | real screen plugged in. This is to avoid the dreaded fallback to the low-res |
| 83 | fbdev default. Many drivers have a hacked-up (and often broken) version of this, |
| 84 | better to do it once in the shared helpers. Thierry has a patch series, but that |
| 85 | one needs to be rebased and final polish applied. |
| 86 | |
| 87 | Contact: Thierry Reding, Daniel Vetter, driver maintainers |
| 88 | |
Thierry Reding | 0e70dad | 2017-02-07 18:51:13 +0100 | [diff] [blame] | 89 | Convert early atomic drivers to async commit helpers |
| 90 | ---------------------------------------------------- |
| 91 | |
| 92 | For the first year the atomic modeset helpers didn't support asynchronous / |
| 93 | nonblocking commits, and every driver had to hand-roll them. This is fixed |
| 94 | now, but there's still a pile of existing drivers that easily could be |
| 95 | converted over to the new infrastructure. |
| 96 | |
| 97 | One issue with the helpers is that they require that drivers handle completion |
| 98 | events for atomic commits correctly. But fixing these bugs is good anyway. |
| 99 | |
| 100 | Contact: Daniel Vetter, respective driver maintainers |
| 101 | |
Daniel Vetter | f217f55 | 2017-03-22 09:36:10 +0100 | [diff] [blame] | 102 | Better manual-upload support for atomic |
| 103 | --------------------------------------- |
| 104 | |
| 105 | This would be especially useful for tinydrm: |
| 106 | |
| 107 | - Add a struct drm_rect dirty_clip to drm_crtc_state. When duplicating the |
| 108 | crtc state, clear that to the max values, x/y = 0 and w/h = MAX_INT, in |
| 109 | __drm_atomic_helper_crtc_duplicate_state(). |
| 110 | |
Thierry Reding | 0cac6ac | 2017-07-31 14:42:59 +0200 | [diff] [blame] | 111 | - Move tinydrm_merge_clips into drm_framebuffer.c, dropping the tinydrm\_ |
| 112 | prefix ofc and using drm_fb\_. drm_framebuffer.c makes sense since this |
Daniel Vetter | f217f55 | 2017-03-22 09:36:10 +0100 | [diff] [blame] | 113 | is a function useful to implement the fb->dirty function. |
| 114 | |
| 115 | - Create a new drm_fb_dirty function which does essentially what e.g. |
| 116 | mipi_dbi_fb_dirty does. You can use e.g. drm_atomic_helper_update_plane as the |
| 117 | template. But instead of doing a simple full-screen plane update, this new |
| 118 | helper also sets crtc_state->dirty_clip to the right coordinates. And of |
| 119 | course it needs to check whether the fb is actually active (and maybe where), |
| 120 | so there's some book-keeping involved. There's also some good fun involved in |
| 121 | scaling things appropriately. For that case we might simply give up and |
| 122 | declare the entire area covered by the plane as dirty. |
| 123 | |
| 124 | Contact: Noralf Trønnes, Daniel Vetter |
| 125 | |
Thierry Reding | 0e70dad | 2017-02-07 18:51:13 +0100 | [diff] [blame] | 126 | Fallout from atomic KMS |
| 127 | ----------------------- |
| 128 | |
| 129 | ``drm_atomic_helper.c`` provides a batch of functions which implement legacy |
| 130 | IOCTLs on top of the new atomic driver interface. Which is really nice for |
| 131 | gradual conversion of drivers, but unfortunately the semantic mismatches are |
| 132 | a bit too severe. So there's some follow-up work to adjust the function |
| 133 | interfaces to fix these issues: |
| 134 | |
| 135 | * atomic needs the lock acquire context. At the moment that's passed around |
| 136 | implicitly with some horrible hacks, and it's also allocate with |
| 137 | ``GFP_NOFAIL`` behind the scenes. All legacy paths need to start allocating |
| 138 | the acquire context explicitly on stack and then also pass it down into |
| 139 | drivers explicitly so that the legacy-on-atomic functions can use them. |
| 140 | |
| 141 | * A bunch of the vtable hooks are now in the wrong place: DRM has a split |
| 142 | between core vfunc tables (named ``drm_foo_funcs``), which are used to |
| 143 | implement the userspace ABI. And then there's the optional hooks for the |
| 144 | helper libraries (name ``drm_foo_helper_funcs``), which are purely for |
| 145 | internal use. Some of these hooks should be move from ``_funcs`` to |
| 146 | ``_helper_funcs`` since they are not part of the core ABI. There's a |
| 147 | ``FIXME`` comment in the kerneldoc for each such case in ``drm_crtc.h``. |
| 148 | |
| 149 | * There's a new helper ``drm_atomic_helper_best_encoder()`` which could be |
| 150 | used by all atomic drivers which don't select the encoder for a given |
| 151 | connector at runtime. That's almost all of them, and would allow us to get |
| 152 | rid of a lot of ``best_encoder`` boilerplate in drivers. |
| 153 | |
| 154 | Contact: Daniel Vetter |
| 155 | |
| 156 | Get rid of dev->struct_mutex from GEM drivers |
| 157 | --------------------------------------------- |
| 158 | |
| 159 | ``dev->struct_mutex`` is the Big DRM Lock from legacy days and infested |
| 160 | everything. Nowadays in modern drivers the only bit where it's mandatory is |
| 161 | serializing GEM buffer object destruction. Which unfortunately means drivers |
| 162 | have to keep track of that lock and either call ``unreference`` or |
| 163 | ``unreference_locked`` depending upon context. |
| 164 | |
| 165 | Core GEM doesn't have a need for ``struct_mutex`` any more since kernel 4.8, |
| 166 | and there's a ``gem_free_object_unlocked`` callback for any drivers which are |
| 167 | entirely ``struct_mutex`` free. |
| 168 | |
| 169 | For drivers that need ``struct_mutex`` it should be replaced with a driver- |
| 170 | private lock. The tricky part is the BO free functions, since those can't |
| 171 | reliably take that lock any more. Instead state needs to be protected with |
| 172 | suitable subordinate locks or some cleanup work pushed to a worker thread. For |
| 173 | performance-critical drivers it might also be better to go with a more |
| 174 | fine-grained per-buffer object and per-context lockings scheme. Currently the |
| 175 | following drivers still use ``struct_mutex``: ``msm``, ``omapdrm`` and |
| 176 | ``udl``. |
| 177 | |
Daniel Vetter | 085c6c0 | 2017-04-04 11:52:54 +0200 | [diff] [blame] | 178 | Contact: Daniel Vetter, respective driver maintainers |
Thierry Reding | 0e70dad | 2017-02-07 18:51:13 +0100 | [diff] [blame] | 179 | |
Thierry Reding | 0e70dad | 2017-02-07 18:51:13 +0100 | [diff] [blame] | 180 | Core refactorings |
| 181 | ================= |
| 182 | |
| 183 | Use new IDR deletion interface to clean up drm_gem_handle_delete() |
| 184 | ------------------------------------------------------------------ |
| 185 | |
| 186 | See the "This is gross" comment -- apparently the IDR system now can return an |
| 187 | error code instead of oopsing. |
| 188 | |
| 189 | Clean up the DRM header mess |
| 190 | ---------------------------- |
| 191 | |
| 192 | Currently the DRM subsystem has only one global header, ``drmP.h``. This is |
| 193 | used both for functions exported to helper libraries and drivers and functions |
| 194 | only used internally in the ``drm.ko`` module. The goal would be to move all |
| 195 | header declarations not needed outside of ``drm.ko`` into |
| 196 | ``drivers/gpu/drm/drm_*_internal.h`` header files. ``EXPORT_SYMBOL`` also |
| 197 | needs to be dropped for these functions. |
| 198 | |
| 199 | This would nicely tie in with the below task to create kerneldoc after the API |
| 200 | is cleaned up. Or with the "hide legacy cruft better" task. |
| 201 | |
| 202 | Note that this is well in progress, but ``drmP.h`` is still huge. The updated |
| 203 | plan is to switch to per-file driver API headers, which will also structure |
| 204 | the kerneldoc better. This should also allow more fine-grained ``#include`` |
| 205 | directives. |
| 206 | |
Daniel Vetter | 085c6c0 | 2017-04-04 11:52:54 +0200 | [diff] [blame] | 207 | In the end no .c file should need to include ``drmP.h`` anymore. |
| 208 | |
Thierry Reding | 0e70dad | 2017-02-07 18:51:13 +0100 | [diff] [blame] | 209 | Contact: Daniel Vetter |
| 210 | |
| 211 | Add missing kerneldoc for exported functions |
| 212 | -------------------------------------------- |
| 213 | |
| 214 | The DRM reference documentation is still lacking kerneldoc in a few areas. The |
| 215 | task would be to clean up interfaces like moving functions around between |
| 216 | files to better group them and improving the interfaces like dropping return |
| 217 | values for functions that never fail. Then write kerneldoc for all exported |
Mauro Carvalho Chehab | ff41c419 | 2017-05-14 11:50:11 -0300 | [diff] [blame] | 218 | functions and an overview section and integrate it all into the drm book. |
Thierry Reding | 0e70dad | 2017-02-07 18:51:13 +0100 | [diff] [blame] | 219 | |
| 220 | See https://dri.freedesktop.org/docs/drm/ for what's there already. |
| 221 | |
| 222 | Contact: Daniel Vetter |
| 223 | |
| 224 | Hide legacy cruft better |
| 225 | ------------------------ |
| 226 | |
| 227 | Way back DRM supported only drivers which shadow-attached to PCI devices with |
| 228 | userspace or fbdev drivers setting up outputs. Modern DRM drivers take charge |
| 229 | of the entire device, you can spot them with the DRIVER_MODESET flag. |
| 230 | |
| 231 | Unfortunately there's still large piles of legacy code around which needs to |
| 232 | be hidden so that driver writers don't accidentally end up using it. And to |
| 233 | prevent security issues in those legacy IOCTLs from being exploited on modern |
| 234 | drivers. This has multiple possible subtasks: |
| 235 | |
Thierry Reding | 0e70dad | 2017-02-07 18:51:13 +0100 | [diff] [blame] | 236 | * Extract support code for legacy features into a ``drm-legacy.ko`` kernel |
| 237 | module and compile it only when one of the legacy drivers is enabled. |
Thierry Reding | 0e70dad | 2017-02-07 18:51:13 +0100 | [diff] [blame] | 238 | |
| 239 | This is mostly done, the only thing left is to split up ``drm_irq.c`` into |
| 240 | legacy cruft and the parts needed by modern KMS drivers. |
| 241 | |
| 242 | Contact: Daniel Vetter |
| 243 | |
| 244 | Make panic handling work |
| 245 | ------------------------ |
| 246 | |
| 247 | This is a really varied tasks with lots of little bits and pieces: |
| 248 | |
| 249 | * The panic path can't be tested currently, leading to constant breaking. The |
| 250 | main issue here is that panics can be triggered from hardirq contexts and |
| 251 | hence all panic related callback can run in hardirq context. It would be |
| 252 | awesome if we could test at least the fbdev helper code and driver code by |
| 253 | e.g. trigger calls through drm debugfs files. hardirq context could be |
| 254 | achieved by using an IPI to the local processor. |
| 255 | |
| 256 | * There's a massive confusion of different panic handlers. DRM fbdev emulation |
| 257 | helpers have one, but on top of that the fbcon code itself also has one. We |
| 258 | need to make sure that they stop fighting over each another. |
| 259 | |
| 260 | * ``drm_can_sleep()`` is a mess. It hides real bugs in normal operations and |
| 261 | isn't a full solution for panic paths. We need to make sure that it only |
| 262 | returns true if there's a panic going on for real, and fix up all the |
| 263 | fallout. |
| 264 | |
| 265 | * The panic handler must never sleep, which also means it can't ever |
| 266 | ``mutex_lock()``. Also it can't grab any other lock unconditionally, not |
| 267 | even spinlocks (because NMI and hardirq can panic too). We need to either |
| 268 | make sure to not call such paths, or trylock everything. Really tricky. |
| 269 | |
| 270 | * For the above locking troubles reasons it's pretty much impossible to |
| 271 | attempt a synchronous modeset from panic handlers. The only thing we could |
| 272 | try to achive is an atomic ``set_base`` of the primary plane, and hope that |
| 273 | it shows up. Everything else probably needs to be delayed to some worker or |
| 274 | something else which happens later on. Otherwise it just kills the box |
| 275 | harder, prevent the panic from going out on e.g. netconsole. |
| 276 | |
| 277 | * There's also proposal for a simplied DRM console instead of the full-blown |
| 278 | fbcon and DRM fbdev emulation. Any kind of panic handling tricks should |
| 279 | obviously work for both console, in case we ever get kmslog merged. |
| 280 | |
| 281 | Contact: Daniel Vetter |
| 282 | |
Daniel Vetter | 0cad7f7 | 2017-03-22 21:54:01 +0100 | [diff] [blame] | 283 | Clean up the debugfs support |
| 284 | ---------------------------- |
| 285 | |
| 286 | There's a bunch of issues with it: |
| 287 | |
| 288 | - The drm_info_list ->show() function doesn't even bother to cast to the drm |
| 289 | structure for you. This is lazy. |
| 290 | |
| 291 | - We probably want to have some support for debugfs files on crtc/connectors and |
| 292 | maybe other kms objects directly in core. There's even drm_print support in |
| 293 | the funcs for these objects to dump kms state, so it's all there. And then the |
| 294 | ->show() functions should obviously give you a pointer to the right object. |
| 295 | |
| 296 | - The drm_info_list stuff is centered on drm_minor instead of drm_device. For |
| 297 | anything we want to print drm_device (or maybe drm_file) is the right thing. |
| 298 | |
| 299 | - The drm_driver->debugfs_init hooks we have is just an artifact of the old |
| 300 | midlayered load sequence. DRM debugfs should work more like sysfs, where you |
| 301 | can create properties/files for an object anytime you want, and the core |
| 302 | takes care of publishing/unpuplishing all the files at register/unregister |
| 303 | time. Drivers shouldn't need to worry about these technicalities, and fixing |
| 304 | this (together with the drm_minor->drm_device move) would allow us to remove |
| 305 | debugfs_init. |
| 306 | |
| 307 | Contact: Daniel Vetter |
| 308 | |
Thierry Reding | 0e70dad | 2017-02-07 18:51:13 +0100 | [diff] [blame] | 309 | Better Testing |
| 310 | ============== |
| 311 | |
| 312 | Enable trinity for DRM |
| 313 | ---------------------- |
| 314 | |
| 315 | And fix up the fallout. Should be really interesting ... |
| 316 | |
| 317 | Make KMS tests in i-g-t generic |
| 318 | ------------------------------- |
| 319 | |
| 320 | The i915 driver team maintains an extensive testsuite for the i915 DRM driver, |
| 321 | including tons of testcases for corner-cases in the modesetting API. It would |
| 322 | be awesome if those tests (at least the ones not relying on Intel-specific GEM |
| 323 | features) could be made to run on any KMS driver. |
| 324 | |
| 325 | Basic work to run i-g-t tests on non-i915 is done, what's now missing is mass- |
| 326 | converting things over. For modeset tests we also first need a bit of |
| 327 | infrastructure to use dumb buffers for untiled buffers, to be able to run all |
| 328 | the non-i915 specific modeset tests. |
| 329 | |
| 330 | Contact: Daniel Vetter |
| 331 | |
| 332 | Create a virtual KMS driver for testing (vkms) |
| 333 | ---------------------------------------------- |
| 334 | |
| 335 | With all the latest helpers it should be fairly simple to create a virtual KMS |
| 336 | driver useful for testing, or for running X or similar on headless machines |
| 337 | (to be able to still use the GPU). This would be similar to vgem, but aimed at |
| 338 | the modeset side. |
| 339 | |
| 340 | Once the basics are there there's tons of possibilities to extend it. |
| 341 | |
| 342 | Contact: Daniel Vetter |
| 343 | |
| 344 | Driver Specific |
| 345 | =============== |
| 346 | |
Daniel Vetter | f217f55 | 2017-03-22 09:36:10 +0100 | [diff] [blame] | 347 | tinydrm |
| 348 | ------- |
| 349 | |
| 350 | Tinydrm is the helper driver for really simple fb drivers. The goal is to make |
| 351 | those drivers as simple as possible, so lots of room for refactoring: |
| 352 | |
| 353 | - backlight helpers, probably best to put them into a new drm_backlight.c. |
| 354 | This is because drivers/video is de-facto unmaintained. We could also |
| 355 | move drivers/video/backlight to drivers/gpu/backlight and take it all |
| 356 | over within drm-misc, but that's more work. |
| 357 | |
| 358 | - spi helpers, probably best put into spi core/helper code. Thierry said |
| 359 | the spi maintainer is fast&reactive, so shouldn't be a big issue. |
| 360 | |
| 361 | - extract the mipi-dbi helper (well, the non-tinydrm specific parts at |
| 362 | least) into a separate helper, like we have for mipi-dsi already. Or follow |
| 363 | one of the ideas for having a shared dsi/dbi helper, abstracting away the |
| 364 | transport details more. |
| 365 | |
| 366 | - tinydrm_lastclose could be drm_fb_helper_lastclose. Only thing we need |
| 367 | for that is to store the drm_fb_helper pointer somewhere in |
| 368 | drm_device->mode_config. And then we could roll that out to all the |
| 369 | drivers. |
| 370 | |
| 371 | - tinydrm_gem_cma_prime_import_sg_table should probably go into the cma |
| 372 | helpers, as a _vmapped variant (since not every driver needs the vmap). |
| 373 | And tinydrm_gem_cma_free_object could the be merged into |
| 374 | drm_gem_cma_free_object(). |
| 375 | |
| 376 | - tinydrm_fb_create we could move into drm_simple_pipe, only need to add |
| 377 | the fb_create hook to drm_simple_pipe_funcs, which would again simplify a |
| 378 | bunch of things (since it gives you a one-stop vfunc for simple drivers). |
| 379 | |
| 380 | - Quick aside: The unregister devm stuff is kinda getting the lifetimes of |
| 381 | a drm_device wrong. Doesn't matter, since everyone else gets it wrong |
| 382 | too :-) |
| 383 | |
| 384 | - With the fbdev pointer in dev->mode_config we could also make |
| 385 | suspend/resume helpers entirely generic, at least if we add a |
| 386 | dev->mode_config.suspend_state. We could even provide a generic pm_ops |
| 387 | structure with those. |
| 388 | |
| 389 | - also rework the drm_framebuffer_funcs->dirty hook wire-up, see above. |
| 390 | |
| 391 | Contact: Noralf Trønnes, Daniel Vetter |
| 392 | |
Thierry Reding | 0e70dad | 2017-02-07 18:51:13 +0100 | [diff] [blame] | 393 | Outside DRM |
| 394 | =========== |