On Tue, Jan 22, 2013 at 06:27:24PM +0100, Mario Kleiner wrote:
> On 22.01.13 09:31, Terje Bergström wrote:
> >On 14.01.2013 18:06, Thierry Reding wrote:
> >>+static int tegra_dc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
> >>+ struct drm_pending_vblank_event *event)
> >>+{
> >>+ struct tegra_framebuffer *newfb = to_tegra_fb(fb);
> >>+ struct tegra_dc *dc = to_tegra_dc(crtc);
> >>+ struct drm_device *drm = crtc->dev;
> >>+ unsigned long flags;
> >>+
> >>+ if (dc->event)
> >>+ return -EBUSY;
>
> Hi
>
> I haven't read the Tegra programming manual or played with this, so
> maybe i'm wrong for some reason, but i think there is a race here
> between actual pageflip completion - latching newfb into the scanout
> base register and the completion routine that gets called from the
> vblank irq handler.
>
> If this code gets called close to vblank or inside vblank, couldn't
> it happen that tegra_dc_set_base() programs a pageflip that gets
> executed immediately - the new fb gets latched into the crtc, but
> the corresponding vblank irq handler for the vblank of flip
> completion runs before the "if (event)" block can set dc->event =
> event;? Or vblank irq's are off because drm_vblank_get() is only
> called at the end of the routine? In both cases the completion
> routine would miss the correct vblank and only timestamp and emit
> the completion event 1 vblank after actual flip completion.
>
> In that case it would be better to place tegra_dc_set_base() after
> the "if (event)" block and have some check inside the flip
> completion routine to make sure the pageflip completion event is
> only emitted if the scanout is really already latched with the
> newfb.
An easier solution might be to expand the scope of the lock a bit to
encompass the call to tegra_dc_set_base() and extend until the end of
tegra_dc_page_flip(). That should properly keep the page-flip completion
from interfering, right?
spin_lock_irqsave(&drm->event_lock, flags);
tegra_dc_set_base(dc, 0, 0, newfb);
if (event) {
event->pipe = dc->pipe;
dc->event = event;
drm_vblank_get(drm, dc->pipe);
}
spin_unlock_irqrestore(&drm->event_lock, flags);
Thierry
Attachment:
pgpr0rkrQeNXk.pgp
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
