diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 2ea6ffc9b22b..96c453980ab6 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -475,25 +475,22 @@ Remove disable/unprepare in remove/shutdown in panel-simple and panel-edp As of commit d2aacaf07395 ("drm/panel: Check for already prepared/enabled in drm_panel"), we have a check in the drm_panel core to make sure nobody double-calls prepare/enable/disable/unprepare. Eventually that should probably -be turned into a WARN_ON() or somehow made louder, but right now we actually -expect it to trigger and so we don't want it to be too loud. +be turned into a WARN_ON() or somehow made louder. -Specifically, that warning will trigger for panel-edp and panel-simple at -shutdown time because those panels hardcode a call to drm_panel_disable() -and drm_panel_unprepare() at shutdown and remove time that they call regardless -of panel state. On systems with a properly coded DRM modeset driver that -calls drm_atomic_helper_shutdown() this is pretty much guaranteed to cause -the warning to fire. +At the moment, we expect that we may still encounter the warnings in the +drm_panel core when using panel-simple and panel-edp. Since those panel +drivers are used with a lot of different DRM modeset drivers they still +make an extra effort to disable/unprepare the panel themsevles at shutdown +time. Specifically we could still encounter those warnings if the panel +driver gets shutdown() _before_ the DRM modeset driver and the DRM modeset +driver properly calls drm_atomic_helper_shutdown() in its own shutdown() +callback. Warnings could be avoided in such a case by using something like +device links to ensure that the panel gets shutdown() after the DRM modeset +driver. -Unfortunately we can't safely remove the calls in panel-edp and panel-simple -until we're sure that all DRM modeset drivers that are used with those panels -properly call drm_atomic_helper_shutdown(). This TODO item is to validate -that all DRM modeset drivers used with panel-edp and panel-simple properly -call drm_atomic_helper_shutdown() and then remove the calls to -disable/unprepare from those panels. Alternatively, this TODO item could be -removed by convincing stakeholders that those calls are fine and downgrading -the error message in drm_panel_disable() / drm_panel_unprepare() to a -debug-level message. +Once all DRM modeset drivers are known to shutdown properly, the extra +calls to disable/unprepare in remove/shutdown in panel-simple and panel-edp +should be removed and this TODO item marked complete. Contact: Douglas Anderson diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index cfbe020de54e..19ab0a794add 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -161,6 +161,15 @@ int drm_panel_unprepare(struct drm_panel *panel) if (!panel) return -EINVAL; + /* + * If you are seeing the warning below it likely means one of two things: + * - Your panel driver incorrectly calls drm_panel_unprepare() in its + * shutdown routine. You should delete this. + * - You are using panel-edp or panel-simple and your DRM modeset + * driver's shutdown() callback happened after the panel's shutdown(). + * In this case the warning is harmless though ideally you should + * figure out how to reverse the order of the shutdown() callbacks. + */ if (!panel->prepared) { dev_warn(panel->dev, "Skipping unprepare of already unprepared panel\n"); return 0; @@ -245,6 +254,15 @@ int drm_panel_disable(struct drm_panel *panel) if (!panel) return -EINVAL; + /* + * If you are seeing the warning below it likely means one of two things: + * - Your panel driver incorrectly calls drm_panel_disable() in its + * shutdown routine. You should delete this. + * - You are using panel-edp or panel-simple and your DRM modeset + * driver's shutdown() callback happened after the panel's shutdown(). + * In this case the warning is harmless though ideally you should + * figure out how to reverse the order of the shutdown() callbacks. + */ if (!panel->enabled) { dev_warn(panel->dev, "Skipping disable of already disabled panel\n"); return 0; diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index 6b16e263d137..e17e0d9e7ebb 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -954,16 +954,24 @@ static void panel_edp_shutdown(struct device *dev) * drm_atomic_helper_shutdown() at shutdown time and that should * cause the panel to be disabled / unprepared if needed. For now, * however, we'll keep these calls due to the sheer number of - * different DRM modeset drivers used with panel-edp. The fact that - * we're calling these and _also_ the drm_atomic_helper_shutdown() - * will try to disable/unprepare means that we can get a warning about - * trying to disable/unprepare an already disabled/unprepared panel, - * but that's something we'll have to live with until we've confirmed - * that all DRM modeset drivers are properly calling - * drm_atomic_helper_shutdown(). + * different DRM modeset drivers used with panel-edp. Once we've + * confirmed that all DRM modeset drivers using this panel properly + * call drm_atomic_helper_shutdown() we can simply delete the two + * calls below. + * + * TO BE EXPLICIT: THE CALLS BELOW SHOULDN'T BE COPIED TO ANY NEW + * PANEL DRIVERS. + * + * FIXME: If we're still haven't figured out if all DRM modeset + * drivers properly call drm_atomic_helper_shutdown() but we _have_ + * managed to make sure that DRM modeset drivers get their shutdown() + * callback before the panel's shutdown() callback (perhaps using + * device link), we could add a WARN_ON here to help move forward. */ - drm_panel_disable(&panel->base); - drm_panel_unprepare(&panel->base); + if (panel->base.enabled) + drm_panel_disable(&panel->base); + if (panel->base.prepared) + drm_panel_unprepare(&panel->base); } static void panel_edp_remove(struct device *dev) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 4a3264130ddc..7ad5996b563b 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -726,16 +726,24 @@ static void panel_simple_shutdown(struct device *dev) * drm_atomic_helper_shutdown() at shutdown time and that should * cause the panel to be disabled / unprepared if needed. For now, * however, we'll keep these calls due to the sheer number of - * different DRM modeset drivers used with panel-simple. The fact that - * we're calling these and _also_ the drm_atomic_helper_shutdown() - * will try to disable/unprepare means that we can get a warning about - * trying to disable/unprepare an already disabled/unprepared panel, - * but that's something we'll have to live with until we've confirmed - * that all DRM modeset drivers are properly calling - * drm_atomic_helper_shutdown(). + * different DRM modeset drivers used with panel-simple. Once we've + * confirmed that all DRM modeset drivers using this panel properly + * call drm_atomic_helper_shutdown() we can simply delete the two + * calls below. + * + * TO BE EXPLICIT: THE CALLS BELOW SHOULDN'T BE COPIED TO ANY NEW + * PANEL DRIVERS. + * + * FIXME: If we're still haven't figured out if all DRM modeset + * drivers properly call drm_atomic_helper_shutdown() but we _have_ + * managed to make sure that DRM modeset drivers get their shutdown() + * callback before the panel's shutdown() callback (perhaps using + * device link), we could add a WARN_ON here to help move forward. */ - drm_panel_disable(&panel->base); - drm_panel_unprepare(&panel->base); + if (panel->base.enabled) + drm_panel_disable(&panel->base); + if (panel->base.prepared) + drm_panel_unprepare(&panel->base); } static void panel_simple_remove(struct device *dev)