Message ID | 1465390613-15905-1-git-send-email-Liviu.Dudau@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 08/06/16 13:56, Liviu Dudau wrote: > In hdlcd_drm_bind()/hdlcd_drm_unbind() we unwind the DRM setup in the > wrong order (drm_mode_config_cleanup() before connector and encoder > had a chance to cleanup their memory or before drm_dev_unregister()). > The correct order should match in both functions. > > Reported-by: Robin Murphy <Robin.Murphy@arm.com> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> > --- > > Robin, > > I believe this should fix your problems with HDLCD failing to allocate CMA > memory and then crashing. Heh, I'm not sure I'd even clocked that there was yet another propblem beyond the original gem_free_object() crash. I've just tried 4.7-rc3 with the relevant fixes for that, and indeed drm_connector_cleanup() does go bang, and this patch makes it happy again. Tested-by: Robin Murphy <robin.murphy@arm.com> Thanks, Robin. > > Best regards, > Liviu > > drivers/gpu/drm/arm/hdlcd_drv.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c > index 3422ca2..2c6eddb 100644 > --- a/drivers/gpu/drm/arm/hdlcd_drv.c > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c > @@ -382,7 +382,6 @@ static int hdlcd_drm_bind(struct device *dev) > > err_fbdev: > drm_kms_helper_poll_fini(drm); > - drm_mode_config_cleanup(drm); > drm_vblank_cleanup(drm); > err_vblank: > pm_runtime_disable(drm->dev); > @@ -390,6 +389,7 @@ err_pm_active: > component_unbind_all(dev, drm); > err_unregister: > drm_dev_unregister(drm); > + drm_mode_config_cleanup(drm); > err_unload: > drm_irq_uninstall(drm); > of_reserved_mem_device_release(drm->dev); > @@ -410,15 +410,15 @@ static void hdlcd_drm_unbind(struct device *dev) > hdlcd->fbdev = NULL; > } > drm_kms_helper_poll_fini(drm); > - component_unbind_all(dev, drm); > drm_vblank_cleanup(drm); > + component_unbind_all(dev, drm); > pm_runtime_get_sync(drm->dev); > drm_irq_uninstall(drm); > pm_runtime_put_sync(drm->dev); > pm_runtime_disable(drm->dev); > - of_reserved_mem_device_release(drm->dev); > - drm_mode_config_cleanup(drm); > drm_dev_unregister(drm); > + drm_mode_config_cleanup(drm); > + of_reserved_mem_device_release(drm->dev); > drm_dev_unref(drm); > drm->dev_private = NULL; > dev_set_drvdata(dev, NULL); >
On Mon, Jun 13, 2016 at 12:02:31PM +0100, Robin Murphy wrote: > On 08/06/16 13:56, Liviu Dudau wrote: > >In hdlcd_drm_bind()/hdlcd_drm_unbind() we unwind the DRM setup in the > >wrong order (drm_mode_config_cleanup() before connector and encoder > >had a chance to cleanup their memory or before drm_dev_unregister()). > >The correct order should match in both functions. > > > >Reported-by: Robin Murphy <Robin.Murphy@arm.com> > >Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> > >--- > > > >Robin, > > > >I believe this should fix your problems with HDLCD failing to allocate CMA > >memory and then crashing. > > Heh, I'm not sure I'd even clocked that there was yet another propblem > beyond the original gem_free_object() crash. I've just tried 4.7-rc3 with > the relevant fixes for that, and indeed drm_connector_cleanup() does go > bang, and this patch makes it happy again. > > Tested-by: Robin Murphy <robin.murphy@arm.com> Cheers! I will push it for the next -rc. Best regards, Liviu > > Thanks, > Robin. > > > > >Best regards, > >Liviu > > > > drivers/gpu/drm/arm/hdlcd_drv.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > >diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c > >index 3422ca2..2c6eddb 100644 > >--- a/drivers/gpu/drm/arm/hdlcd_drv.c > >+++ b/drivers/gpu/drm/arm/hdlcd_drv.c > >@@ -382,7 +382,6 @@ static int hdlcd_drm_bind(struct device *dev) > > > > err_fbdev: > > drm_kms_helper_poll_fini(drm); > >- drm_mode_config_cleanup(drm); > > drm_vblank_cleanup(drm); > > err_vblank: > > pm_runtime_disable(drm->dev); > >@@ -390,6 +389,7 @@ err_pm_active: > > component_unbind_all(dev, drm); > > err_unregister: > > drm_dev_unregister(drm); > >+ drm_mode_config_cleanup(drm); > > err_unload: > > drm_irq_uninstall(drm); > > of_reserved_mem_device_release(drm->dev); > >@@ -410,15 +410,15 @@ static void hdlcd_drm_unbind(struct device *dev) > > hdlcd->fbdev = NULL; > > } > > drm_kms_helper_poll_fini(drm); > >- component_unbind_all(dev, drm); > > drm_vblank_cleanup(drm); > >+ component_unbind_all(dev, drm); > > pm_runtime_get_sync(drm->dev); > > drm_irq_uninstall(drm); > > pm_runtime_put_sync(drm->dev); > > pm_runtime_disable(drm->dev); > >- of_reserved_mem_device_release(drm->dev); > >- drm_mode_config_cleanup(drm); > > drm_dev_unregister(drm); > >+ drm_mode_config_cleanup(drm); > >+ of_reserved_mem_device_release(drm->dev); > > drm_dev_unref(drm); > > drm->dev_private = NULL; > > dev_set_drvdata(dev, NULL); > > >
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c index 3422ca2..2c6eddb 100644 --- a/drivers/gpu/drm/arm/hdlcd_drv.c +++ b/drivers/gpu/drm/arm/hdlcd_drv.c @@ -382,7 +382,6 @@ static int hdlcd_drm_bind(struct device *dev) err_fbdev: drm_kms_helper_poll_fini(drm); - drm_mode_config_cleanup(drm); drm_vblank_cleanup(drm); err_vblank: pm_runtime_disable(drm->dev); @@ -390,6 +389,7 @@ err_pm_active: component_unbind_all(dev, drm); err_unregister: drm_dev_unregister(drm); + drm_mode_config_cleanup(drm); err_unload: drm_irq_uninstall(drm); of_reserved_mem_device_release(drm->dev); @@ -410,15 +410,15 @@ static void hdlcd_drm_unbind(struct device *dev) hdlcd->fbdev = NULL; } drm_kms_helper_poll_fini(drm); - component_unbind_all(dev, drm); drm_vblank_cleanup(drm); + component_unbind_all(dev, drm); pm_runtime_get_sync(drm->dev); drm_irq_uninstall(drm); pm_runtime_put_sync(drm->dev); pm_runtime_disable(drm->dev); - of_reserved_mem_device_release(drm->dev); - drm_mode_config_cleanup(drm); drm_dev_unregister(drm); + drm_mode_config_cleanup(drm); + of_reserved_mem_device_release(drm->dev); drm_dev_unref(drm); drm->dev_private = NULL; dev_set_drvdata(dev, NULL);
In hdlcd_drm_bind()/hdlcd_drm_unbind() we unwind the DRM setup in the wrong order (drm_mode_config_cleanup() before connector and encoder had a chance to cleanup their memory or before drm_dev_unregister()). The correct order should match in both functions. Reported-by: Robin Murphy <Robin.Murphy@arm.com> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> --- Robin, I believe this should fix your problems with HDLCD failing to allocate CMA memory and then crashing. Best regards, Liviu drivers/gpu/drm/arm/hdlcd_drv.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)