[PATCH 0/3] drm/fsl-dcu: fix driver remove/DRM unload

[PATCH 0/3] drm/fsl-dcu: fix driver remove/DRM unload

[Stefan Agner]

Hi All,

The first patch is a better alternative to the previously posted
patch ("drm/fsl-dcu: only init fbdev if required") as suggested
by Daniel.

The second and third are fix related issue uncovered during tests
with bind/unbind:
echo 40058000.dcu > /sys/bus/platform/drivers/fsl-dcu/unbind
echo 40058000.dcu > /sys/bus/platform/drivers/fsl-dcu/bind

Especially the third patch I am not sure if that is a reasonable
strategy to fix the issue. I did not saw another SoC DRM driver
which is making use of drm_crtc_force_disable_all...

Also, when the X Server is running (with modesetting driver) I
still get a warning:

WARNING: CPU: 0 PID: 452 at drivers/gpu/drm/drm_crtc.c:1154 drm_mode_config_cleanup+0x210/0x220

The comment says it is the drivers fault, but as far as I can
tell it is user space which does not free up this framebuffers.
Is there something missing in my driver?

Any ideas?

--
Stefan

Stefan Agner (3):
  drm/fb_cma_helper: do not free fbdev if there is none
  drm/fsl-dcu: unload driver before disabling clocks
  drm/fsl-dcu: disable outputs before unloading driver

 drivers/gpu/drm/drm_fb_cma_helper.c       | 3 ++-
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

-- 
2.10.0

[PATCH 1/3] drm/fb_cma_helper: do not free fbdev if there is none

[Stefan Agner]

If fbdev emulation is not in use (or not built-in), fb_helper.fbdev
is NULL. Don't call calling drm_fbdev_cma_defio_fini in this case.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/drm_fb_cma_helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 1fd6eac..12d654d 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -557,7 +557,8 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);
 void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
 {
 	drm_fb_helper_unregister_fbi(&fbdev_cma->fb_helper);
-	drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
+	if (fbdev_cma->fb_helper.fbdev)
+		drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
 	drm_fb_helper_release_fbi(&fbdev_cma->fb_helper);
 
 	if (fbdev_cma->fb) {
-- 
2.10.0

[PATCH 1/3] drm/fb_cma_helper: do not free fbdev if there is none

[Stefan Agner]

If fbdev emulation is not in use (or not built-in), fb_helper.fbdev
is NULL. Don't call calling drm_fbdev_cma_defio_fini in this case.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/drm_fb_cma_helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 1fd6eac..12d654d 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -557,7 +557,8 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);
 void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
 {
 	drm_fb_helper_unregister_fbi(&fbdev_cma->fb_helper);
-	drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
+	if (fbdev_cma->fb_helper.fbdev)
+		drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
 	drm_fb_helper_release_fbi(&fbdev_cma->fb_helper);
 
 	if (fbdev_cma->fb) {
-- 
2.10.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/fb_cma_helper: do not free fbdev if there is none

[Daniel Vetter]

On Wed, Oct 19, 2016 at 05:32:19PM -0700, Stefan Agner wrote:
> If fbdev emulation is not in use (or not built-in), fb_helper.fbdev
> is NULL. Don't call calling drm_fbdev_cma_defio_fini in this case.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Yeah makes sense. Applied to drm-misc.
-Daniel

> ---
>  drivers/gpu/drm/drm_fb_cma_helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 1fd6eac..12d654d 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -557,7 +557,8 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);
>  void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
>  {
>  	drm_fb_helper_unregister_fbi(&fbdev_cma->fb_helper);
> -	drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
> +	if (fbdev_cma->fb_helper.fbdev)
> +		drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
>  	drm_fb_helper_release_fbi(&fbdev_cma->fb_helper);
>  
>  	if (fbdev_cma->fb) {
> -- 
> 2.10.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/3] drm/fb_cma_helper: do not free fbdev if there is none

[Daniel Vetter]

On Wed, Oct 19, 2016 at 05:32:19PM -0700, Stefan Agner wrote:
> If fbdev emulation is not in use (or not built-in), fb_helper.fbdev
> is NULL. Don't call calling drm_fbdev_cma_defio_fini in this case.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Yeah makes sense. Applied to drm-misc.
-Daniel

> ---
>  drivers/gpu/drm/drm_fb_cma_helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 1fd6eac..12d654d 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -557,7 +557,8 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);
>  void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
>  {
>  	drm_fb_helper_unregister_fbi(&fbdev_cma->fb_helper);
> -	drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
> +	if (fbdev_cma->fb_helper.fbdev)
> +		drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
>  	drm_fb_helper_release_fbi(&fbdev_cma->fb_helper);
>  
>  	if (fbdev_cma->fb) {
> -- 
> 2.10.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/fsl-dcu: unload driver before disabling clocks

[Stefan Agner]

Use drm_put_dev to unload the driver before disabling clocks.
Otherwise the driver might read a register during unload which
leads to an external abort.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 07969f5..66f3d2c 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -429,9 +429,9 @@ static int fsl_dcu_drm_remove(struct platform_device *pdev)
 {
 	struct fsl_dcu_drm_device *fsl_dev = platform_get_drvdata(pdev);
 
+	drm_put_dev(fsl_dev->drm);
 	clk_disable_unprepare(fsl_dev->clk);
 	clk_unregister(fsl_dev->pix_clk);
-	drm_put_dev(fsl_dev->drm);
 
 	return 0;
 }
-- 
2.10.0

[PATCH 3/3] drm/fsl-dcu: disable outputs before unloading driver

[Stefan Agner]

Make sure that all outputs are disabled before unloading the DRM
driver. Otherwise vblank handling is not shut down properly and
warnings such as this appear:
WARNING: CPU: 0 PID: 540 at drivers/gpu/drm/drm_irq.c:339 drm_vblank_cleanup+0x5c/0x94

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 66f3d2c..4a31f20 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -108,6 +108,7 @@ static int fsl_dcu_unload(struct drm_device *dev)
 {
 	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
 
+	drm_crtc_force_disable_all(dev);
 	drm_kms_helper_poll_fini(dev);
 
 	if (fsl_dev->fbdev)
-- 
2.10.0

Re: [PATCH 0/3] drm/fsl-dcu: fix driver remove/DRM unload

[Stefan Agner]

On 2016-10-19 17:32, Stefan Agner wrote:
> Hi All,
> 
> The first patch is a better alternative to the previously posted
> patch ("drm/fsl-dcu: only init fbdev if required") as suggested
> by Daniel.
> 
> The second and third are fix related issue uncovered during tests
> with bind/unbind:
> echo 40058000.dcu > /sys/bus/platform/drivers/fsl-dcu/unbind
> echo 40058000.dcu > /sys/bus/platform/drivers/fsl-dcu/bind
> 
> Especially the third patch I am not sure if that is a reasonable
> strategy to fix the issue. I did not saw another SoC DRM driver
> which is making use of drm_crtc_force_disable_all...
> 
> Also, when the X Server is running (with modesetting driver) I
> still get a warning:
> 
> WARNING: CPU: 0 PID: 452 at drivers/gpu/drm/drm_crtc.c:1154
> drm_mode_config_cleanup+0x210/0x220
> 
> The comment says it is the drivers fault, but as far as I can
> tell it is user space which does not free up this framebuffers.
> Is there something missing in my driver?
> 
> Any ideas?
> 
> --
> Stefan
> 
> Stefan Agner (3):
>   drm/fb_cma_helper: do not free fbdev if there is none
>   drm/fsl-dcu: unload driver before disabling clocks
>   drm/fsl-dcu: disable outputs before unloading driver
> 
>  drivers/gpu/drm/drm_fb_cma_helper.c       | 3 ++-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)


Applied 2/3 to my for-next branch.

--
Stefan

Re: [PATCH 0/3] drm/fsl-dcu: fix driver remove/DRM unload

[Stefan Agner]

On 2016-10-19 17:32, Stefan Agner wrote:
> Hi All,
> 
> The first patch is a better alternative to the previously posted
> patch ("drm/fsl-dcu: only init fbdev if required") as suggested
> by Daniel.
> 
> The second and third are fix related issue uncovered during tests
> with bind/unbind:
> echo 40058000.dcu > /sys/bus/platform/drivers/fsl-dcu/unbind
> echo 40058000.dcu > /sys/bus/platform/drivers/fsl-dcu/bind
> 
> Especially the third patch I am not sure if that is a reasonable
> strategy to fix the issue. I did not saw another SoC DRM driver
> which is making use of drm_crtc_force_disable_all...
> 
> Also, when the X Server is running (with modesetting driver) I
> still get a warning:
> 
> WARNING: CPU: 0 PID: 452 at drivers/gpu/drm/drm_crtc.c:1154
> drm_mode_config_cleanup+0x210/0x220
> 
> The comment says it is the drivers fault, but as far as I can
> tell it is user space which does not free up this framebuffers.
> Is there something missing in my driver?
> 
> Any ideas?
> 
> --
> Stefan
> 
> Stefan Agner (3):
>   drm/fb_cma_helper: do not free fbdev if there is none
>   drm/fsl-dcu: unload driver before disabling clocks
>   drm/fsl-dcu: disable outputs before unloading driver
> 
>  drivers/gpu/drm/drm_fb_cma_helper.c       | 3 ++-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)


Applied 2/3 to my for-next branch.

--
Stefan
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel