Hi, On Fri, Sep 1, 2023 at 4:42 PM Douglas Anderson <dianders@xxxxxxxxxxxx> wrote: > > Based on grepping through the source code this driver appears to be > missing a call to drm_atomic_helper_shutdown() at system shutdown > time. Among other things, this means that if a panel is in use that it > won't be cleanly powered off at system shutdown time. > > The fact that we should call drm_atomic_helper_shutdown() in the case > of OS shutdown/restart comes straight out of the kernel doc "driver > instance overview" in drm_drv.c. > > Since this driver uses the component model and shutdown happens at the > base driver, we communicate whether we have to call > drm_atomic_helper_shutdown() by seeing if drvdata is non-NULL. > > Suggested-by: Maxime Ripard <mripard@xxxxxxxxxx> > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > --- > This commit is only compile-time tested. > > NOTE: this patch touches a lot more than other similar patches since > the bind() function is long and we want to make sure that we unset the > drvdata if bind() fails. > > While making this patch, I noticed that the bind() function of this > driver is using "devm" and thus assumes it doesn't need to do much > explicit error handling. That's actually a bug. As per kernel docs [1] > "the lifetime of the aggregate driver does not align with any of the > underlying struct device instances. Therefore devm cannot be used and > all resources acquired or allocated in this callback must be > explicitly released in the unbind callback". Fixing that is outside > the scope of this commit. > > [1] https://docs.kernel.org/driver-api/component.html > > drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 66 +++++++++++++++-------- > 1 file changed, 44 insertions(+), 22 deletions(-) [ ... cut ... ] > @@ -1612,6 +1633,7 @@ static struct platform_driver ingenic_drm_driver = { > }, > .probe = ingenic_drm_probe, > .remove = ingenic_drm_remove, > + .shutdown = ingenic_drm_shutdown, I resolved the trivial conflict with commit 2b9b0a9fc548 ("drm/ingenic: Convert to platform remove callback returning void"), then pushed to drm-misc-next: c3ca98396ffa (HEAD -> drm-misc-next) drm/ingenic: Call drm_atomic_helper_shutdown() at shutdown time
