2016-10-20 00:32:48

by Stefan Agner

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

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


2016-10-20 00:32:52

by Stefan Agner

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

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 <[email protected]>
---
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

2016-10-20 00:32:59

by Stefan Agner

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

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 <[email protected]>
---
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

2016-10-20 00:33:40

by Stefan Agner

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

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 <[email protected]>
---
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

2016-10-20 06:42:27

by Daniel Vetter

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

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 <[email protected]>

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

2016-11-17 01:05:42

by Stefan Agner

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

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