Hi, On Tue, Jun 11, 2024 at 07:48:51AM GMT, Douglas Anderson wrote: > At shutdown if you've got a _properly_ coded DRM modeset driver then > you'll get these two warnings at shutdown time: > > Skipping disable of already disabled panel > Skipping unprepare of already unprepared panel > > These warnings are ugly and sound concerning, but they're actually a > sign of a properly working system. That's not great. > > It's not easy to get rid of these warnings. Until we know that all DRM > modeset drivers used with panel-simple and panel-edp are properly > calling drm_atomic_helper_shutdown() or drm_helper_force_disable_all() > then the panel drivers _need_ to disable/unprepare themselves in order > to power off the panel cleanly. However, there are lots of DRM modeset > drivers used with panel-edp and panel-simple and it's hard to know > when we've got them all. Since the warning happens only on the drivers > that _are_ updated there's nothing to encourage broken DRM modeset > drivers to get fixed. > > In order to flip the warning to the proper place, we need to know > which modeset drivers are going to shutdown properly. Though ugly, do > this by creating a list of everyone that shuts down properly. This > allows us to generate a warning for the correct case and also lets us > get rid of the warning for drivers that are shutting down properly. > > Maintaining this list is ugly, but the idea is that it's only short > term. Once everyone is converted we can delete the list and call it > done. The list is ugly enough and adding to it is annoying enough that > people should push to make this happen. > > Implement this all in a shared "header" file included by the two panel > drivers that need it. This avoids us adding an new exports while still > allowing the panel drivers to be modules. The code waste should be > small and, as per above, the whole solution is temporary. > > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > --- > I came up with this idea to help us move forward since otherwise I > couldn't see how we were ever going to fix panel-simple and panel-edp > since they're used by so many DRM Modeset drivers. It's a bit ugly but > I don't hate it. What do others think? I don't think it's the right approach, even more so since we're so close now to having it in every driver. I ran the coccinelle script we started with, and here are the results: ./drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:1640:25-39: ERROR: KMS driver vmw_pci_driver is missing shutdown implementation ./drivers/gpu/drm/kmb/kmb_drv.c:621:30-49: ERROR: KMS driver kmb_platform_driver is missing shutdown implementation ./drivers/gpu/drm/tiny/arcpgu.c:422:30-52: ERROR: KMS driver arcpgu_platform_driver is missing shutdown implementation Looking at the drivers by hand, it seems consistent. Moving forward, I think having a collection of coccinelle scripts that we ask new driver authors to run or put them in CI somehow would be a better path. We have other similar candidates that can't really be dealt with any other way, like not using drmm_ memory allocations, or not using drm_dev_enter / drm_dev_exit. Maxime
Attachment:
signature.asc
Description: PGP signature
