2021-06-02 16:12:58

by Dan Sneddon

Subject: [PATCH] drm:atmel: Enable the crtc vblank prior to crtc usage.

'commit eec44d44a3d2 ("drm/atmel: Use drm_atomic_helper_commit")'
removed the home-grown handling of atomic commits and exposed an issue
in the crtc atomic commit handling where vblank is expected to be
enabled but hasn't yet, causing kernel warnings during boot. This patch
cleans up the crtc vblank handling thus removing the warning on boot.

Fixes: eec44d44a3d2 ("drm/atmel: Use drm_atomic_helper_commit")

Signed-off-by: Dan Sneddon <[email protected]>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 05ad75d155e8..cfe4fc69277e 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -232,7 +232,6 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,

pm_runtime_put_sync(dev->dev);

- drm_crtc_vblank_on(c);
}

#define ATMEL_HLCDC_RGB444_OUTPUT BIT(0)
@@ -343,8 +342,17 @@ static int atmel_hlcdc_crtc_atomic_check(struct drm_crtc *c,

static void atmel_hlcdc_crtc_atomic_begin(struct drm_crtc *c,
struct drm_atomic_state *state)
+{
+ drm_crtc_vblank_on(c);
+}
+
+static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *c,
+ struct drm_atomic_state *state)
{
struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c);
+ unsigned long flags;
+
+ spin_lock_irqsave(&c->dev->event_lock, flags);

if (c->state->event) {
c->state->event->pipe = drm_crtc_index(c);
@@ -354,12 +362,7 @@ static void atmel_hlcdc_crtc_atomic_begin(struct drm_crtc *c,
crtc->event = c->state->event;
c->state->event = NULL;
}
-}
-
-static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *crtc,
- struct drm_atomic_state *state)
-{
- /* TODO: write common plane control register if available */
+ spin_unlock_irqrestore(&c->dev->event_lock, flags);
}

static const struct drm_crtc_helper_funcs lcdc_crtc_helper_funcs = {
--
2.17.1

2021-06-18 09:19:26

by Nicolas Ferre

Subject: Re: [PATCH] drm:atmel: Enable the crtc vblank prior to crtc usage.

Hi Daniel, Sam,

On 02/06/2021 at 18:08, Dan Sneddon wrote:
> 'commit eec44d44a3d2 ("drm/atmel: Use drm_atomic_helper_commit")'
> removed the home-grown handling of atomic commits and exposed an issue
> in the crtc atomic commit handling where vblank is expected to be
> enabled but hasn't yet, causing kernel warnings during boot. This patch
> cleans up the crtc vblank handling thus removing the warning on boot.
>
> Fixes: eec44d44a3d2 ("drm/atmel: Use drm_atomic_helper_commit")
>
> Signed-off-by: Dan Sneddon <[email protected]>

Is this fix need more input from us, is there something missing (aka ping)?

Regards,
Nicolas


> ---
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 05ad75d155e8..cfe4fc69277e 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -232,7 +232,6 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
>
> pm_runtime_put_sync(dev->dev);
>
> - drm_crtc_vblank_on(c);
> }
>
> #define ATMEL_HLCDC_RGB444_OUTPUT BIT(0)
> @@ -343,8 +342,17 @@ static int atmel_hlcdc_crtc_atomic_check(struct drm_crtc *c,
>
> static void atmel_hlcdc_crtc_atomic_begin(struct drm_crtc *c,
> struct drm_atomic_state *state)
> +{
> + drm_crtc_vblank_on(c);
> +}
> +
> +static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *c,
> + struct drm_atomic_state *state)
> {
> struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&c->dev->event_lock, flags);
>
> if (c->state->event) {
> c->state->event->pipe = drm_crtc_index(c);
> @@ -354,12 +362,7 @@ static void atmel_hlcdc_crtc_atomic_begin(struct drm_crtc *c,
> crtc->event = c->state->event;
> c->state->event = NULL;
> }
> -}
> -
> -static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *crtc,
> - struct drm_atomic_state *state)
> -{
> - /* TODO: write common plane control register if available */
> + spin_unlock_irqrestore(&c->dev->event_lock, flags);
> }
>
> static const struct drm_crtc_helper_funcs lcdc_crtc_helper_funcs = {
>


--
Nicolas Ferre