[PATCH 8/8] drm/exynos: fimd: add complete_scanout function

[Mandeep Singh Baines]

On Tue, Jan 8, 2013 at 9:43 PM, Prathyush K <prathyush at chromium.org> wrote:
>
>
>
> On Tue, Jan 8, 2013 at 4:18 AM, Mandeep Singh Baines <msb at chromium.org>
> wrote:
>>
>> On Wed, Dec 26, 2012 at 3:27 AM, Prathyush K <prathyush.k at samsung.com>
>> wrote:
>> > The wait_for_vblank interface is modified to the complete_scanout
>> > function in fimd. This patch adds the fimd_complete_scanout function
>> >
>>
>> With this series, you have a race if the rmfb happens before the flip
>> has completed.
>>
> How will there be a race? Can you explain the scenario in more detail?
>

I'm sorry. There's no race. I had misunderstood something.

Reference counting might still be preferable to avoid blocking the
Xserver till vblank.

You can implement it by grabbing a reference on page_flip and release
the reference
on finish_page_flip (when flipping to a new fb).

Regards,
Mandeep

> If the current fb (fb1) is being removed, (i.e the shadow register has fb1),
> then we wait for vblank before returning. This ensures that the flip has
> been handled
> before remove fb is complete.
>
>>
>> Why not just use reference counts as is done in the i915 driver: see
>> unpin_work.
>> The reference counts also allow you to avoid blocking in rmfb.
>>
> Will look into it. Thanks.
>
> Regards,
> Prathyush
>
>
>>
>> Regards,
>> Mandeep
>>
>> > Inside fimd_complete_scanout, we read the shadow register for each
>> > window and compare it with the dma address of the framebuffer. If
>> > the dma_address is in the shadow register, then the function waits
>> > for the next vblank and returns.
>> >
>> > Signed-off-by: Prathyush K <prathyush.k at samsung.com>
>> > ---
>> >  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 32
>> > +++++++++++++++++++++++++++++++-
>> >  include/video/samsung_fimd.h             |  1 +
>> >  2 files changed, 32 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> > b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> > index 3aeedf5..190ffde9 100644
>> > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> > @@ -46,6 +46,7 @@
>> >  #define VIDOSD_D(win)          (VIDOSD_BASE + 0x0C + (win) * 16)
>> >
>> >  #define VIDWx_BUF_START(win, buf)      (VIDW_BUF_START(buf) + (win) *
>> > 8)
>> > +#define VIDWx_BUF_START_S(win, buf)    (VIDW_BUF_START_S(buf) + (win) *
>> > 8)
>> >  #define VIDWx_BUF_END(win, buf)                (VIDW_BUF_END(buf) +
>> > (win) * 8)
>> >  #define VIDWx_BUF_SIZE(win, buf)       (VIDW_BUF_SIZE(buf) + (win) * 4)
>> >
>> > @@ -363,13 +364,42 @@ static void fimd_wait_for_vblank(struct device
>> > *dev)
>> >                 fimd_disable_vblank(dev);
>> >  }
>> >
>> > +static void fimd_complete_scanout(struct device *dev, dma_addr_t
>> > dma_addr,
>> > +                                       unsigned long size)
>> > +{
>> > +       struct fimd_context *ctx = get_fimd_context(dev);
>> > +       int win;
>> > +       bool in_use = false;
>> > +
>> > +       if (ctx->suspended)
>> > +               return;
>> > +
>> > +       for (win = 0; win < WINDOWS_NR; win++) {
>> > +               dma_addr_t dma_addr_in_use;
>> > +
>> > +               if (!ctx->win_data[win].enabled)
>> > +                       continue;
>> > +
>> > +               dma_addr_in_use = readl(ctx->regs +
>> > VIDWx_BUF_START_S(win, 0));
>> > +               if (dma_addr_in_use >= dma_addr &&
>> > +                       dma_addr_in_use < (dma_addr + size)) {
>> > +                               in_use = true;
>> > +                               break;
>> > +               }
>> > +       }
>> > +
>> > +       if (in_use)
>> > +               fimd_wait_for_vblank(dev);
>> > +       return;
>> > +}
>> > +
>> >  static struct exynos_drm_manager_ops fimd_manager_ops = {
>> >         .dpms = fimd_dpms,
>> >         .apply = fimd_apply,
>> >         .commit = fimd_commit,
>> >         .enable_vblank = fimd_enable_vblank,
>> >         .disable_vblank = fimd_disable_vblank,
>> > -       .wait_for_vblank = fimd_wait_for_vblank,
>> > +       .complete_scanout = fimd_complete_scanout,
>> >  };
>> >
>> >  static void fimd_win_mode_set(struct device *dev,
>> > diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h
>> > index 7ae6c07..382eaec 100644
>> > --- a/include/video/samsung_fimd.h
>> > +++ b/include/video/samsung_fimd.h
>> > @@ -274,6 +274,7 @@
>> >
>> >  /* Video buffer addresses */
>> >  #define VIDW_BUF_START(_buff)                  (0xA0 + ((_buff) * 8))
>> > +#define VIDW_BUF_START_S(_buff)                        (0x40A0 +
>> > ((_buff) * 8))
>> >  #define VIDW_BUF_START1(_buff)                 (0xA4 + ((_buff) * 8))
>> >  #define VIDW_BUF_END(_buff)                    (0xD0 + ((_buff) * 8))
>> >  #define VIDW_BUF_END1(_buff)                   (0xD4 + ((_buff) * 8))
>> > --
>> > 1.8.0
>> >
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>