Cleanup patches for mali-dp driver

Cleanup patches for mali-dp driver

[Liviu Dudau]

Hi,

A couple of patches to fix issues that I have discovered during the development of
more Mali DP features :)

I would like to take these patches through drm-misc and don't go through the mali-dp tree
as I will be going on holiday for a month. If anyone has patches for Mali DP driver
(or HDLCD to an extent) please Cc Brian Starkey or use the formal Mali DP mailing list.

Many thanks,
Liviu

[PATCH 1/2] drm: mali-dp: Clear the config_valid flag before using it in wait_event.

[Liviu Dudau]

config_valid variable is used to signal the activation of the CVAL
request when the vsync interrupt has fired. malidp_set_and_wait_config_valid()
uses the variable in wait_event_interruptible_timeout without clearing it
first, so the wait is skipped.

Cc: Brian Starkey <Brian.Starkey@arm.com>
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 drivers/gpu/drm/arm/malidp_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 82171d2..87c9249 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -42,6 +42,7 @@ static int malidp_set_and_wait_config_valid(struct drm_device *drm)
 	struct malidp_hw_device *hwdev = malidp->dev;
 	int ret;
 
+	atomic_set(&malidp->config_valid, 0);
 	hwdev->set_config_valid(hwdev);
 	/* don't wait for config_valid flag if we are in config mode */
 	if (hwdev->in_config_mode(hwdev))
-- 
2.9.0

[PATCH 2/2] drm: mali-dp: Set the drm->irq_enabled flag to match driver's state.

[Liviu Dudau]

Mali DP driver does not use drm_irq_{un,}install() function so the
drm->irq_enabled flag does not get managed automatically. Among other
DRM framework functions, drm_wait_vblank() checks the value of the
flag and return -EINVAL if not set.

Cc: Brian Starkey <Brian.Starkey@arm.com>
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 drivers/gpu/drm/arm/malidp_drv.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 87c9249..a1c01e4 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -377,6 +377,8 @@ static int malidp_bind(struct device *dev)
 	if (ret < 0)
 		goto irq_init_fail;
 
+	drm->irq_enabled = true;
+
 	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
 	if (ret < 0) {
 		DRM_ERROR("failed to initialise vblank\n");
@@ -402,6 +404,7 @@ fbdev_fail:
 vblank_fail:
 	malidp_se_irq_fini(drm);
 	malidp_de_irq_fini(drm);
+	drm->irq_enabled = false;
 irq_init_fail:
 	component_unbind_all(dev, drm);
 bind_fail:
@@ -439,6 +442,7 @@ static void malidp_unbind(struct device *dev)
 	drm_kms_helper_poll_fini(drm);
 	malidp_se_irq_fini(drm);
 	malidp_de_irq_fini(drm);
+	drm->irq_enabled = false;
 	drm_vblank_cleanup(drm);
 	component_unbind_all(dev, drm);
 	of_node_put(malidp->crtc.port);
-- 
2.9.0

Re: [PATCH 2/2] drm: mali-dp: Set the drm->irq_enabled flag to match driver's state.

[Daniel Vetter]

On Fri, Jul 29, 2016 at 03:44:16PM +0100, Liviu Dudau wrote:
> Mali DP driver does not use drm_irq_{un,}install() function so the
> drm->irq_enabled flag does not get managed automatically. Among other
> DRM framework functions, drm_wait_vblank() checks the value of the
> flag and return -EINVAL if not set.
> 
> Cc: Brian Starkey <Brian.Starkey@arm.com>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> ---
>  drivers/gpu/drm/arm/malidp_drv.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 87c9249..a1c01e4 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -377,6 +377,8 @@ static int malidp_bind(struct device *dev)
>  	if (ret < 0)
>  		goto irq_init_fail;
>  
> +	drm->irq_enabled = true;

I wonder whether we should have a cocci script or something with catching
some of these issues that every new driver seems to hit. Another one is
filling out legacy helpers for all the legacy entry points using atomic
functions. Or still using ->load/unload. Documentation is great, but it
seems like most folks don't read it until they hit a snag ;-)

Just idle musings ...
-Daniel

> +
>  	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
>  	if (ret < 0) {
>  		DRM_ERROR("failed to initialise vblank\n");
> @@ -402,6 +404,7 @@ fbdev_fail:
>  vblank_fail:
>  	malidp_se_irq_fini(drm);
>  	malidp_de_irq_fini(drm);
> +	drm->irq_enabled = false;
>  irq_init_fail:
>  	component_unbind_all(dev, drm);
>  bind_fail:
> @@ -439,6 +442,7 @@ static void malidp_unbind(struct device *dev)
>  	drm_kms_helper_poll_fini(drm);
>  	malidp_se_irq_fini(drm);
>  	malidp_de_irq_fini(drm);
> +	drm->irq_enabled = false;
>  	drm_vblank_cleanup(drm);
>  	component_unbind_all(dev, drm);
>  	of_node_put(malidp->crtc.port);
> -- 
> 2.9.0
> 

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