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.
thanks,
-mario
>> +
>> + tegra_dc_set_base(dc, 0, 0, newfb);
>> +
>> + if (event) {
>> + event->pipe = dc->pipe;
>> +
>> + spin_lock_irqsave(&drm->event_lock, flags);
>> + dc->event = event;
>> + spin_unlock_irqrestore(&drm->event_lock, flags);
>> +
>> + drm_vblank_get(drm, dc->pipe);
>> + }
>> +
>> + return 0;
>> +}
>
