2014-10-10 12:32:18

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 0/4] drm/exynos: misc fixes

Hi Inki,

These four independent patches fixes few problems I have encountered recently.

1st patch is a trivial fix.

2nd patch(kms poll init) fixes old issue, which became more visible after my patch
'drm/exynos: init vblank with real number of crtcs' - driver tried to handle HPD
events even if it was not fully initialized, it caused WARNs and app locks.

Vblank patch is slightly different version of the one proposed in [1]. The only
difference is that it enables vblank after dpms on, it is neccessary because
drm_crtc_vblank_on can call drm_vblank_enable which can call exynos_drm_crtc_enable_vblank
and exynos_drm_crtc_enable_vblank will return error if exynos_crtc->dpms != DRM_MODE_DPMS_ON.

DPMS patch was previously sent as a part of other series, but as the series have been dropped
I have resend it in this patchset.

The patchset is based on exynos-drm-next branch plus following patches:
drm/exynos: remove explicit encoder/connector de-initialization
drm/exynos: init vblank with real number of crtcs
drm/exynos: remove ifdeferry from initialization code


It has been tested on trats board.

[1]: https://lkml.org/lkml/2014/10/2/154

Regards
Andrzej


Andrzej Hajda (4):
drm/exynos: propagate plane initialization errors
drm/exynos: init kms poll at the end of initialization
drm/exynos: enable vblank after DPMS on
drm/exynos: correct connector->dpms field before resuming

drivers/gpu/drm/exynos/exynos_drm_crtc.c | 5 ++++-
drivers/gpu/drm/exynos/exynos_drm_drv.c | 27 +++++++++++++++++----------
2 files changed, 21 insertions(+), 11 deletions(-)

--
1.9.1


2014-10-10 12:32:24

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 1/4] drm/exynos: propagate plane initialization errors

In case of error during plane initialization load callback
incorrectly return success, this patch fixes it.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_drv.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index cf19e60..322e7bf 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -87,8 +87,11 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags)

plane = exynos_plane_init(dev, possible_crtcs,
DRM_PLANE_TYPE_OVERLAY);
- if (IS_ERR(plane))
- goto err_mode_config_cleanup;
+ if (!IS_ERR(plane))
+ continue;
+
+ ret = PTR_ERR(plane);
+ goto err_mode_config_cleanup;
}

/* init kms poll for handling hpd */
--
1.9.1

2014-10-10 12:32:28

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 2/4] drm/exynos: init kms poll at the end of initialization

HPD events can be generated by components even if drm_dev is not fully
initialized, to skip such events kms poll initialization should
be performed at the end of load callback followed directly by forced
connection detection.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_drv.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 322e7bf..ce6c14c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -94,9 +94,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
goto err_mode_config_cleanup;
}

- /* init kms poll for handling hpd */
- drm_kms_helper_poll_init(dev);
-
/* setup possible_clones. */
exynos_drm_encoder_setup(dev);

@@ -116,9 +113,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
if (ret)
goto err_cleanup_vblank;

- /* force connectors detection */
- drm_helper_hpd_irq_event(dev);
-
/*
* enable drm irq mode.
* - with irq_enabled = true, we can use the vblank feature.
@@ -136,6 +130,12 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
*/
dev->vblank_disable_allowed = true;

+ /* init kms poll for handling hpd */
+ drm_kms_helper_poll_init(dev);
+
+ /* force connectors detection */
+ drm_helper_hpd_irq_event(dev);
+
return 0;

err_cleanup_vblank:
--
1.9.1

2014-10-10 12:32:30

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 4/4] drm/exynos: correct connector->dpms field before resuming

During system suspend after connector switch off its dpms field
is set to connector previous dpms state. To properly resume dpms field
should be set to its actual state (off) before resuming to previous dpms state.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index ce6c14c..fe250e4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -194,8 +194,12 @@ static int exynos_drm_resume(struct drm_device *dev)

drm_modeset_lock_all(dev);
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
- if (connector->funcs->dpms)
- connector->funcs->dpms(connector, connector->dpms);
+ if (connector->funcs->dpms) {
+ int dpms = connector->dpms;
+
+ connector->dpms = DRM_MODE_DPMS_OFF;
+ connector->funcs->dpms(connector, dpms);
+ }
}
drm_modeset_unlock_all(dev);

--
1.9.1

2014-10-10 12:33:03

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 3/4] drm/exynos: enable vblank after DPMS on

Before DPMS off driver disables vblank.
It should be balanced by vblank enable after DPMS on.
The patch fixes issue with page_flip ioctl not being able
to acquire vblank counter introduced by patch:
drm: Always reject drm_vblank_get() after drm_vblank_off()

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 8e38e9f..45026e6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -71,13 +71,16 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
!atomic_read(&exynos_crtc->pending_flip),
HZ/20))
atomic_set(&exynos_crtc->pending_flip, 0);
- drm_vblank_off(crtc->dev, exynos_crtc->pipe);
+ drm_crtc_vblank_off(crtc);
}

if (manager->ops->dpms)
manager->ops->dpms(manager, mode);

exynos_crtc->dpms = mode;
+
+ if (mode == DRM_MODE_DPMS_ON)
+ drm_crtc_vblank_on(crtc);
}

static void exynos_drm_crtc_prepare(struct drm_crtc *crtc)
--
1.9.1

2014-10-11 18:35:27

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH 3/4] drm/exynos: enable vblank after DPMS on

On Fri, Oct 10, 2014 at 02:31:55PM +0200, Andrzej Hajda wrote:
> Before DPMS off driver disables vblank.
> It should be balanced by vblank enable after DPMS on.
> The patch fixes issue with page_flip ioctl not being able
> to acquire vblank counter introduced by patch:
> drm: Always reject drm_vblank_get() after drm_vblank_off()
>
> Signed-off-by: Andrzej Hajda <[email protected]>

Yeah, you should always call vblank_on again when you (re)enable a crtc,
whether this is through a set_config call or through dpms. This is
Reviewed-by: Daniel Vetter <[email protected]>

Sorry that we didn't catch the impact of this additional check on existing
drivers, I've thought I've reviewed them and checked that they all call
vblank_on. But I didn't take into account that the codepaths might differ
for dpms and set_config paths. Otoh most drivers really should implement
one in terms of the other.

Cheers, Daniel

> ---
> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index 8e38e9f..45026e6 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -71,13 +71,16 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
> !atomic_read(&exynos_crtc->pending_flip),
> HZ/20))
> atomic_set(&exynos_crtc->pending_flip, 0);
> - drm_vblank_off(crtc->dev, exynos_crtc->pipe);
> + drm_crtc_vblank_off(crtc);
> }
>
> if (manager->ops->dpms)
> manager->ops->dpms(manager, mode);
>
> exynos_crtc->dpms = mode;
> +
> + if (mode == DRM_MODE_DPMS_ON)
> + drm_crtc_vblank_on(crtc);
> }
>
> static void exynos_drm_crtc_prepare(struct drm_crtc *crtc)
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch