| Message ID | 2c28c0a137854d39b6bc997a21bd6d2db1f7a0a5.1687423204.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | drm: renesas: shmobile: Atomic conversion + DT support | expand |
Hi Geert, Thank you for the patch. On Thu, Jun 22, 2023 at 11:21:44AM +0200, Geert Uytterhoeven wrote: > When the device is unbound from the driver, the display may be active. > Make sure it gets shut down. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c > index a29c0c1093725b6e..636f1888b815579b 100644 > --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c > +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c > @@ -16,6 +16,7 @@ > #include <linux/pm_runtime.h> > #include <linux/slab.h> > > +#include <drm/drm_crtc_helper.h> > #include <drm/drm_drv.h> > #include <drm/drm_fbdev_generic.h> > #include <drm/drm_gem_dma_helper.h> > @@ -145,6 +146,7 @@ static int shmob_drm_remove(struct platform_device *pdev) > struct drm_device *ddev = &sdev->ddev; > > drm_dev_unregister(ddev); > + drm_helper_force_disable_all(ddev); I assume this will be turned into drm_atomic_helper_shutdown() later. Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > drm_kms_helper_poll_fini(ddev); > return 0; > }
Hi On 2023/6/22 17:21, Geert Uytterhoeven wrote: > When the device is unbound from the driver, the display may be active. > Make sure it gets shut down. would you mind to give a short description why this is necessary. > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c > index a29c0c1093725b6e..636f1888b815579b 100644 > --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c > +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c > @@ -16,6 +16,7 @@ > #include <linux/pm_runtime.h> > #include <linux/slab.h> > > +#include <drm/drm_crtc_helper.h> > #include <drm/drm_drv.h> > #include <drm/drm_fbdev_generic.h> > #include <drm/drm_gem_dma_helper.h> > @@ -145,6 +146,7 @@ static int shmob_drm_remove(struct platform_device *pdev) > struct drm_device *ddev = &sdev->ddev; > > drm_dev_unregister(ddev); > + drm_helper_force_disable_all(ddev); Is it that the DRM core recommend us to use drm_atomic_helper_disable_all() ? > drm_kms_helper_poll_fini(ddev); > return 0; > }
Hi Sui, On Tue, Jun 27, 2023 at 4:57 PM Sui Jingfeng <suijingfeng@loongson.cn> wrote: > On 2023/6/22 17:21, Geert Uytterhoeven wrote: > > When the device is unbound from the driver, the display may be active. > > Make sure it gets shut down. > > would you mind to give a short description why this is necessary. That's a good comment. It turned out that this is not really necessary here, but to avoid a regression with "[PATCH 34/39] drm: renesas: shmobile: Atomic conversion part 1", where it is needed to call drm_atomic_helper_shutdown(). As the comments for drm_atomic_helper_shutdown() says it is the atomic version of drm_helper_force_disable_all(), I figured I had to introduce a call to the latter first, before doing the atomic conversion. Does that make sense? > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c > > +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c > > @@ -16,6 +16,7 @@ > > #include <linux/pm_runtime.h> > > #include <linux/slab.h> > > > > +#include <drm/drm_crtc_helper.h> > > #include <drm/drm_drv.h> > > #include <drm/drm_fbdev_generic.h> > > #include <drm/drm_gem_dma_helper.h> > > @@ -145,6 +146,7 @@ static int shmob_drm_remove(struct platform_device *pdev) > > struct drm_device *ddev = &sdev->ddev; > > > > drm_dev_unregister(ddev); > > + drm_helper_force_disable_all(ddev); > > Is it that the DRM core recommend us to use > drm_atomic_helper_disable_all() ? Well, drm_atomic_helper_shutdown() is a convenience wrapper around drm_atomic_helper_disable_all()... But we can't call any atomic helpers yet, before the conversion to atomic modesetting. > > > drm_kms_helper_poll_fini(ddev); > > return 0; > > } Gr{oetje,eeting}s, Geert
diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c index a29c0c1093725b6e..636f1888b815579b 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c @@ -16,6 +16,7 @@ #include <linux/pm_runtime.h> #include <linux/slab.h> +#include <drm/drm_crtc_helper.h> #include <drm/drm_drv.h> #include <drm/drm_fbdev_generic.h> #include <drm/drm_gem_dma_helper.h> @@ -145,6 +146,7 @@ static int shmob_drm_remove(struct platform_device *pdev) struct drm_device *ddev = &sdev->ddev; drm_dev_unregister(ddev); + drm_helper_force_disable_all(ddev); drm_kms_helper_poll_fini(ddev); return 0; }
When the device is unbound from the driver, the display may be active. Make sure it gets shut down. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 2 ++ 1 file changed, 2 insertions(+)