Re: [PATCH v3 12/20] drm: omapdrm: Prevent processing the same event multiple times

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

On Tuesday 27 Sep 2016 15:24:47 Tomi Valkeinen wrote:
> On 19/09/16 15: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 <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/gpu/drm/omapdrm/omap_crtc.c | 22 +++++++++++++++++++---
> >  1 file changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> > b/drivers/gpu/drm/omapdrm/omap_crtc.c index a0c26592fc69..8fef6558197b
> > 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;
> >  };
> >  
> >  /* ----------------------------------------------------------------------
> > @@ -260,11 +261,15 @@ 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;
> > +	spin_lock_irqsave(&dev->event_lock, flags);
> > +	event = omap_crtc->event;
> > +	omap_crtc->event = NULL;
> > +	spin_unlock_irqrestore(&dev->event_lock, flags);
> > 
> >  	if (!event)
> >  		return;
> > @@ -384,12 +389,23 @@ 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)
> >  {
> > +	struct drm_pending_vblank_event *event = crtc->state->event;
> > +	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> > +	unsigned long flags;
> > +
> > +	if (event) {
> > +		WARN_ON(drm_crtc_vblank_get(crtc) != 0);
> 
> Where's the matching put?

It's missing, or rather the get shouldn't be added here. The vblank get/put 
calls are added by patch 14/20 in this series. I'll remove the call from here.

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel