[v4,12/22] drm: omapdrm: Prevent processing the same event multiple times

Commit Message

Laurent Pinchart Dec. 14, 2016, 12:27 a.m. UTC

The vblank interrupt is disabled after one occurrence, preventing the
atomic update event from being processed twice. However, this also
prevents the software frame counter from being updated correctly that
would require vblank interrupts to be kept enabled while the CRTC is
active.

In preparation for vblank interrupt fixes, make sure that the atomic
update event will be processed once only when the vblank interrupt will
be kept enabled.

Signed-off-by: Laurent Pinchart <[e-mail]>
---
Changes since v3:

- Don't release and reacquire the spinlock for just one return
- Remove unneeded (and unbalanced) drm_crtc_vblank_get()
- Store the event in the atomic flush handler to avoid race condition
- Use spin_lock_irq instead of spin_lock_irsave in flush handler
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

Comments

Tomi Valkeinen Dec. 15, 2016, 12:20 p.m. UTC | #1

On 14/12/16 02:27, Laurent Pinchart wrote:
> The vblank interrupt is disabled after one occurrence, preventing the
> atomic update event from being processed twice. However, this also
> prevents the software frame counter from being updated correctly that
> would require vblank interrupts to be kept enabled while the CRTC is
> active.
> 
> In preparation for vblank interrupt fixes, make sure that the atomic
> update event will be processed once only when the vblank interrupt will
> be kept enabled.
> 
> Signed-off-by: Laurent Pinchart <[e-mail]>
> ---
> Changes since v3:
> 
> - Don't release and reacquire the spinlock for just one return
> - Remove unneeded (and unbalanced) drm_crtc_vblank_get()
> - Store the event in the atomic flush handler to avoid race condition
> - Use spin_lock_irq instead of spin_lock_irsave in flush handler
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)

Reviewed-by: Tomi Valkeinen <[e-mail]>

 Tomi

Patch

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index a0511cd5d380..7b67ecb02b74 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -43,6 +43,7 @@  struct omap_crtc {
 	bool enabled;
 	bool pending;
 	wait_queue_head_t pending_wait;
+	struct drm_pending_vblank_event *event;
 };
 
 /* -----------------------------------------------------------------------------
@@ -263,17 +264,17 @@  static const struct dss_mgr_ops mgr_ops = {
 
 static void omap_crtc_complete_page_flip(struct drm_crtc *crtc)
 {
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 	struct drm_pending_vblank_event *event;
 	struct drm_device *dev = crtc->dev;
 	unsigned long flags;
 
-	event = crtc->state->event;
-
-	if (!event)
-		return;
-
 	spin_lock_irqsave(&dev->event_lock, flags);
-	drm_crtc_send_vblank_event(crtc, event);
+	event = omap_crtc->event;
+	omap_crtc->event = NULL;
+
+	if (event)
+		drm_crtc_send_vblank_event(crtc, event);
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
@@ -390,12 +391,12 @@  static int omap_crtc_atomic_check(struct drm_crtc *crtc,
 }
 
 static void omap_crtc_atomic_begin(struct drm_crtc *crtc,
-                                  struct drm_crtc_state *old_crtc_state)
+				   struct drm_crtc_state *old_crtc_state)
 {
 }
 
 static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
-                                  struct drm_crtc_state *old_crtc_state)
+				   struct drm_crtc_state *old_crtc_state)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 
@@ -431,6 +432,12 @@  static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 	omap_crtc->pending = true;
 	wmb();
 
+	if (crtc->state->event) {
+		spin_lock_irq(&crtc->dev->event_lock);
+		omap_crtc->event = crtc->state->event;
+		spin_unlock_irq(&crtc->dev->event_lock);
+	}
+
 	dispc_mgr_go(omap_crtc->channel);
 	omap_irq_register(crtc->dev, &omap_crtc->vblank_irq);
 }