<div dir="ltr">Hi,<div class="gmail_extra"><br><div class="gmail_quote">On 12 January 2015 at 21:13, Gustavo Padovan <span dir="ltr"><<a href="mailto:gustavo@padovan.org" target="_blank">gustavo@padovan.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">2014-12-30 Inki Dae <<a href="mailto:inki.dae@samsung.com">inki.dae@samsung.com</a>>:<div><div class="h5">
> On 2014년 12월 18일 22:58, Gustavo Padovan wrote:<br>> > +    * This works around a race between a page_flip request and the latency<br>
> > +    * between vblank interrupt and this irq_handler:<br>
> > +    *   => FIMD vblank: BUF_START_S[0] := BUF_START[0], and asserts irq<br>
> > +    *   | => fimd_win_commit(0) writes new BUF_START[0]<br>
> > +    *   |    exynos_drm_crtc_try_do_flip() marks exynos_fb as prepared<br>
> > +    *   => fimd_irq_handler()<br>
> > +    *       exynos_drm_crtc_finish_pageflip() sees prepared exynos_fb,<br>
> > +    *           and unmaps "old" fb<br>
> > +    *   ==> but, since BUF_START_S[0] still points to that "old" fb...<br>
> > +    *   ==> FIMD iommu fault<br>
> > +    */<br>
> > +   start = readl(ctx->regs + VIDWx_BUF_START(0, 0));<br>
> > +   start_s = readl(ctx->regs + VIDWx_BUF_START_S(0, 0));<br>
> > +   if (start == start_s)<br>
> >             exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);<br>
><span style="color:rgb(34,34,34)"> And what if 'start_s' has a value different from one of 'start'? Is it</span><br></div></div><span class="">
> ok to skip finish_pageflip request this time? Shouldn't it ensure to<br>
> wait for that until 'start_s' has a value same as one of 'start'?<br>
<br>
</span>I think it is okay to skip finish_pageflip, but we could return directly<br>
if they are different, so we keep the wait_vsync_queue running until the next<br>
irq happens or it timeouts. How does this look to you?<br></blockquote><div><br></div><div>Right, you'd need to keep the vblank IRQ alive until start == start_s.</div><div><br></div><div>As an aside, there is currently some refcounting code (e.g. in the DPMS off and/or framebuffer final-unref paths if I remember correctly) which assumes that waiting one vblank is enough to be able to unpin resources. Obviously, as this patch shows, this is not actually true. This patch doesn't make this any better or worse; just something to bear in mind. (I have WIP code which fixes this.)</div><div><br></div><div>Cheers,</div><div>Daniel</div></div></div></div>