| rjw | 1f88458 | 2022-01-06 17:20:42 +0800 | [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`` | 
|  | 19 | and ``drm_pci.c``. The goal is to get rid of the driver's ``->load`` / | 
|  | 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 |  | 
|  | 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 |  | 
|  | 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 |  | 
|  | 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 |  | 
|  | 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 |  | 
|  | 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 | 
|  | 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 |  | 
|  | 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 |  | 
|  | 178 | Contact: Daniel Vetter, respective driver maintainers | 
|  | 179 |  | 
|  | 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 |  | 
|  | 207 | In the end no .c file should need to include ``drmP.h`` anymore. | 
|  | 208 |  | 
|  | 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 | 
|  | 218 | functions and an overview section and integrate it all into the drm book. | 
|  | 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 |  | 
|  | 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. | 
|  | 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 |  | 
|  | 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 |  | 
|  | 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 |  | 
|  | 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 |  | 
|  | 393 | Outside DRM | 
|  | 394 | =========== |