| Message ID | 1490706496-4959-30-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tomi, Thank you for the patch. On Tuesday 28 Mar 2017 16:08:15 Tomi Valkeinen wrote: > Use drm_atomic_helper_shutdown() to ensure that all crtcs are disabled > when unloading the driver. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > > --- > drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c > b/drivers/gpu/drm/omapdrm/omap_drv.c index ad8d16cf819c..7b917c0c1a27 > 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > @@ -865,6 +865,8 @@ static int pdev_remove(struct platform_device *pdev) > if (priv->fbdev) > omap_fbdev_free(ddev); > > + drm_atomic_helper_shutdown(ddev); > + Can the hardware still be enabled at this point ? If pdev_remove() is called we shouldn't have any application holding the device open (otherwise we'll crash miserably), and the fbdev compatibility layer is disabled on the previous line. > drm_mode_config_cleanup(ddev); > > omap_drm_irq_uninstall(ddev);
On 29/03/17 11:49, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Tuesday 28 Mar 2017 16:08:15 Tomi Valkeinen wrote: >> Use drm_atomic_helper_shutdown() to ensure that all crtcs are disabled >> when unloading the driver. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> Cc: Daniel Vetter <daniel@ffwll.ch> >> >> --- >> drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c >> b/drivers/gpu/drm/omapdrm/omap_drv.c index ad8d16cf819c..7b917c0c1a27 >> 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c >> @@ -865,6 +865,8 @@ static int pdev_remove(struct platform_device *pdev) >> if (priv->fbdev) >> omap_fbdev_free(ddev); >> >> + drm_atomic_helper_shutdown(ddev); >> + > > Can the hardware still be enabled at this point ? If pdev_remove() is called > we shouldn't have any application holding the device open (otherwise we'll > crash miserably), and the fbdev compatibility layer is disabled on the > previous line. The hw is disabled before drm_atomic_helper_shutdown() is called only if you have fbdev. And fbdev on all the connectors. If that's not the case, we need drm_atomic_helper_shutdown() to disable the hw. Tomi
Hi Tomi, On Wednesday 29 Mar 2017 12:08:19 Tomi Valkeinen wrote: > On 29/03/17 11:49, Laurent Pinchart wrote: > > On Tuesday 28 Mar 2017 16:08:15 Tomi Valkeinen wrote: > >> Use drm_atomic_helper_shutdown() to ensure that all crtcs are disabled > >> when unloading the driver. > >> > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > >> Cc: Daniel Vetter <daniel@ffwll.ch> > >> > >> --- > >> > >> drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c > >> b/drivers/gpu/drm/omapdrm/omap_drv.c index ad8d16cf819c..7b917c0c1a27 > >> 100644 > >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c > >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > >> @@ -865,6 +865,8 @@ static int pdev_remove(struct platform_device *pdev) > >> > >> if (priv->fbdev) > >> > >> omap_fbdev_free(ddev); > >> > >> + drm_atomic_helper_shutdown(ddev); > >> + > > > > Can the hardware still be enabled at this point ? If pdev_remove() is > > called we shouldn't have any application holding the device open > > (otherwise we'll crash miserably), and the fbdev compatibility layer is > > disabled on the previous line. > > The hw is disabled before drm_atomic_helper_shutdown() is called only if > you have fbdev. And fbdev on all the connectors. If that's not the case, > we need drm_atomic_helper_shutdown() to disable the hw. But if you have no fbdev, as userspace must not hold the device node open here, isn't the device disabled already ?
On 29/03/17 12:11, Laurent Pinchart wrote: >> The hw is disabled before drm_atomic_helper_shutdown() is called only if >> you have fbdev. And fbdev on all the connectors. If that's not the case, >> we need drm_atomic_helper_shutdown() to disable the hw. > > But if you have no fbdev, as userspace must not hold the device node open > here, isn't the device disabled already ? Unfortunately not. See the "drm/atomic: Introduce drm_atomic_helper_shutdown" thread on dri-devel. Tomi
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index ad8d16cf819c..7b917c0c1a27 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -865,6 +865,8 @@ static int pdev_remove(struct platform_device *pdev) if (priv->fbdev) omap_fbdev_free(ddev); + drm_atomic_helper_shutdown(ddev); + drm_mode_config_cleanup(ddev); omap_drm_irq_uninstall(ddev);
Use drm_atomic_helper_shutdown() to ensure that all crtcs are disabled when unloading the driver. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Cc: Daniel Vetter <daniel@ffwll.ch> --- drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++ 1 file changed, 2 insertions(+)