| Message ID | 1349725849-22433-12-git-send-email-rob.clark@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Mon, 2012-10-08 at 14:50 -0500, Rob Clark wrote: > From: Rob Clark <rob@ti.com> > > Userspace might not request a vblank event. So it is not an error > for 'event' to be NULL, and we shouldn't use it to determine if > there is a pending flip already. > > Signed-off-by: Rob Clark <rob@ti.com> > --- > drivers/staging/omapdrm/omap_crtc.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/omapdrm/omap_crtc.c b/drivers/staging/omapdrm/omap_crtc.c > index 74e019a..317b854 100644 > --- a/drivers/staging/omapdrm/omap_crtc.c > +++ b/drivers/staging/omapdrm/omap_crtc.c > @@ -119,7 +119,6 @@ static void vblank_cb(void *arg) > struct omap_crtc *omap_crtc = to_omap_crtc(crtc); > unsigned long flags; > > - WARN_ON(!event); > spin_lock_irqsave(&dev->event_lock, flags); > > /* wakeup userspace */ > @@ -127,6 +126,7 @@ static void vblank_cb(void *arg) > drm_send_vblank_event(dev, -1, omap_crtc->event); > > omap_crtc->event = NULL; > + omap_crtc->old_fb = NULL; Is old_fb used anywhere? If not we could just remove it. Otherwise nice work! On the series: Reviewed-by: Imre Deak <imre.deak@intel.com> > > spin_unlock_irqrestore(&dev->event_lock, flags); > } > @@ -138,8 +138,6 @@ static void page_flip_cb(void *arg) > struct drm_framebuffer *old_fb = omap_crtc->old_fb; > struct drm_gem_object *bo; > > - omap_crtc->old_fb = NULL; > - > omap_crtc_mode_set_base(crtc, crtc->x, crtc->y, old_fb); > > /* really we'd like to setup the callback atomically w/ setting the > @@ -162,7 +160,7 @@ static int omap_crtc_page_flip_locked(struct drm_crtc *crtc, > > DBG("%d -> %d", crtc->fb ? crtc->fb->base.id : -1, fb->base.id); > > - if (omap_crtc->event) { > + if (omap_crtc->old_fb) { > dev_err(dev->dev, "already a pending flip\n"); > return -EINVAL; > }
On Tue, 2012-10-09 at 12:35 +0300, Imre Deak wrote: > On Mon, 2012-10-08 at 14:50 -0500, Rob Clark wrote: > > From: Rob Clark <rob@ti.com> > > > > Userspace might not request a vblank event. So it is not an error > > for 'event' to be NULL, and we shouldn't use it to determine if > > there is a pending flip already. > > > > Signed-off-by: Rob Clark <rob@ti.com> > > --- > > drivers/staging/omapdrm/omap_crtc.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/staging/omapdrm/omap_crtc.c b/drivers/staging/omapdrm/omap_crtc.c > > index 74e019a..317b854 100644 > > --- a/drivers/staging/omapdrm/omap_crtc.c > > +++ b/drivers/staging/omapdrm/omap_crtc.c > > @@ -119,7 +119,6 @@ static void vblank_cb(void *arg) > > struct omap_crtc *omap_crtc = to_omap_crtc(crtc); > > unsigned long flags; > > > > - WARN_ON(!event); > > spin_lock_irqsave(&dev->event_lock, flags); > > > > /* wakeup userspace */ > > @@ -127,6 +126,7 @@ static void vblank_cb(void *arg) > > drm_send_vblank_event(dev, -1, omap_crtc->event); > > > > omap_crtc->event = NULL; > > + omap_crtc->old_fb = NULL; > > Is old_fb used anywhere? If not we could just remove it. > > Otherwise nice work! On the series: > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > > > > spin_unlock_irqrestore(&dev->event_lock, flags); > > } > > @@ -138,8 +138,6 @@ static void page_flip_cb(void *arg) > > struct drm_framebuffer *old_fb = omap_crtc->old_fb; > > struct drm_gem_object *bo; > > > > - omap_crtc->old_fb = NULL; > > - > > omap_crtc_mode_set_base(crtc, crtc->x, crtc->y, old_fb); > > > > /* really we'd like to setup the callback atomically w/ setting the > > @@ -162,7 +160,7 @@ static int omap_crtc_page_flip_locked(struct drm_crtc *crtc, > > > > DBG("%d -> %d", crtc->fb ? crtc->fb->base.id : -1, fb->base.id); > > > > - if (omap_crtc->event) { > > + if (omap_crtc->old_fb) { Ah, just noticed this adds the use for it :) So ignore my comment above. --Imre > > dev_err(dev->dev, "already a pending flip\n"); > > return -EINVAL; > > } >
diff --git a/drivers/staging/omapdrm/omap_crtc.c b/drivers/staging/omapdrm/omap_crtc.c index 74e019a..317b854 100644 --- a/drivers/staging/omapdrm/omap_crtc.c +++ b/drivers/staging/omapdrm/omap_crtc.c @@ -119,7 +119,6 @@ static void vblank_cb(void *arg) struct omap_crtc *omap_crtc = to_omap_crtc(crtc); unsigned long flags; - WARN_ON(!event); spin_lock_irqsave(&dev->event_lock, flags); /* wakeup userspace */ @@ -127,6 +126,7 @@ static void vblank_cb(void *arg) drm_send_vblank_event(dev, -1, omap_crtc->event); omap_crtc->event = NULL; + omap_crtc->old_fb = NULL; spin_unlock_irqrestore(&dev->event_lock, flags); } @@ -138,8 +138,6 @@ static void page_flip_cb(void *arg) struct drm_framebuffer *old_fb = omap_crtc->old_fb; struct drm_gem_object *bo; - omap_crtc->old_fb = NULL; - omap_crtc_mode_set_base(crtc, crtc->x, crtc->y, old_fb); /* really we'd like to setup the callback atomically w/ setting the @@ -162,7 +160,7 @@ static int omap_crtc_page_flip_locked(struct drm_crtc *crtc, DBG("%d -> %d", crtc->fb ? crtc->fb->base.id : -1, fb->base.id); - if (omap_crtc->event) { + if (omap_crtc->old_fb) { dev_err(dev->dev, "already a pending flip\n"); return -EINVAL; }