Re: [PATCH v3] drm/rockchip: update cursors asynchronously through atomic.

[Eric Anholt <eric@anholt.net>]

[-- Attachment #1.1: Type: text/plain, Size: 2463 bytes --]

Michael Zoran <mzoran@crowfest.net> writes:

> On Fri, 2018-11-23 at 11:27 +0900, Tomasz Figa wrote:
>> 
>> The point here is not about setting and resetting the plane->fb
>> pointer. It's about what happens inside
>> drm_atomic_set_fb_for_plane().
>> 
>> It calls drm_framebuffer_get() for the new fb and
>> drm_framebuffer_put() for the old fb. In result, if the fb changes,
>> the old fb, which had its reference count incremented in the atomic
>> commit that set it to the plane before, has its reference count
>> decremented. Moreover, if the new reference count becomes 0,
>> drm_framebuffer_put() will immediately free the buffer.
>> 
>> Freeing a buffer when the hardware is still scanning out of it isn't
>> a
>> good idea, is it?
>
> No, it's not.  But the board I submitted the patch for doesn't have
> anything like hot swapable ram.  The ram access is still going to work,
> just it might display something it shouldn't. Say for example if that
> frame buffer got reused by somethig else and filled with new data in
> the very small window.
>
> But yes, I agree the best solution would be to not release the buffer
> until the next vblank.
>
> Perhaps a good solution would be for the DRM api to have the concept of
> a deferred release?  Meaning if the put() call just added the frame
> buffer to a list that DRM core could walk during the vblank.  That
> might be better then every single driver trying to work up a custom
> solution.
>
>> The vc4 driver seems to be able to program the hardware to switch the
>> scanout to the new buffer immediately:
>> 
>> https://elixir.bootlin.com/linux/v4.20-rc3/source/drivers/gpu/drm/vc4/vc4_plane.c#L794
>> 
>> Although I wonder if there isn't still a tiny race there - the
>> hardware may have just started refilling the FIFO from the old
>> address. Still, if the FIFO is small, the FIFO refill operation may
>> be
>> much shorter than it takes for the kernel code to actually free the
>> buffer. Eric and Michael, could you confirm?
>> 
>
> I don't have those boards anymore, and I don't have access to any
> technical documentation on the GPU so I can't really add much here.  
> Eric can probably provide the best information.

I don't think I understood my scanout hardware well enough when I
started on the async update stuff for rpi.  vc4 probably needs to wait
until the HW starts scanning out a new line before letting the old BO
get freed.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel