Hi, On Mon, Sep 4, 2023 at 12:28 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > Hi Douglas, > > On Sat, Sep 2, 2023 at 1:42 AM 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(), or in this case the > > non-atomic equivalent drm_helper_force_disable_all(), at system > > shutdown time and at driver remove time. This is important because > > drm_helper_force_disable_all() will cause panels to get disabled > > cleanly which may be important for their power sequencing. Future > > changes will remove any custom powering off in individual panel > > drivers so the DRM drivers need to start getting this right. > > > > The fact that we should call drm_atomic_helper_shutdown(), or in this > > case the non-atomic equivalent drm_helper_force_disable_all(), in the > > case of OS shutdown/restart comes straight out of the kernel doc > > "driver instance overview" in drm_drv.c. > > > > Suggested-by: Maxime Ripard <mripard@xxxxxxxxxx> > > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > > Thanks for your patch! > > > --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c > > +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c > > @@ -179,12 +180,20 @@ static int shmob_drm_remove(struct platform_device *pdev) > > > > drm_dev_unregister(ddev); > > drm_kms_helper_poll_fini(ddev); > > + drm_helper_force_disable_all(ddev); > > After "[PATCH v3 36/41] drm: renesas: shmobile: Atomic conversion part > 1"[1], this function will already call drm_atomic_helper_shutdown()... > > > free_irq(sdev->irq, ddev); > > drm_dev_put(ddev); > > > > return 0; > > } > > > > +static void shmob_drm_shutdown(struct platform_device *pdev) > > +{ > > + struct shmob_drm_device *sdev = platform_get_drvdata(pdev); > > + > > + drm_helper_force_disable_all(sdev->ddev); > > ... and this should be replaced by a call to drm_atomic_helper_shutdown(). > > [1] https://lore.kernel.org/dri-devel/fd7a6702490bd431f314d6591551bb39e77e3304.1692178020.git.geert+renesas@xxxxxxxxx Ah, thanks! I will put this patch on hold and check back in a few weeks to see how things are looking. If you wanted to fold it into your series I certainly wouldn't object to it, but if not that's fine too. ;-) -Doug