[PATCH] drm/amd/display: Skip fast cursor updates for fb changes

[Grodzovsky, Andrey]

On 12/17/2018 04:53 AM, Michel Dänzer wrote:
> On 2018-12-15 6:25 a.m., Grodzovsky, Andrey wrote:
>> On 12/14/2018 02:17 PM, Kazlauskas, Nicholas wrote:
>>> On 12/14/18 2:06 PM, Grodzovsky, Andrey wrote:
>>>> In general I agree with Michel that  DRM solution is required to
>>>> properly address this but since now it's not really obvious what is the
>>>> proper solution it seems to me OK to go with this fix until it's found.
>>>>
>>>> Reviewed-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com>
>>>>
>>>> Andrey
>>> Thanks for the review.
>>>
>>> FWIW, we're not the only ones with the fb change check like this - msm
>>> does it too. The only other user of atomic_async_check and
>>> atomic_async_update is vc4 and they don't have it but I'd imagine they
>>> see a similar bug with interleaving framebuffers.
>>>
>>> It may be difficult to develop a "fix" for the behavior in DRM given the
>>> semantics of the function (in place update vs full swap). The
>>> old_plane_state is technically plane->state in this case, so even just
>>> adding cleanup_fb(plane, old_plane_state) after atomic_async_update
>>> isn't enough. What *should* be done here is the full state swap like in
>>> a regular atomic commit but that may cause breakages in other drivers.
>> Your change effectively does that for AMDGPU by forcing non async update
>> for when
>> new_plane->state->fb != curret_plane->state->fb.
>> But after looking more into it looks to me that this fix is the generic
>> solution, I don't see any way around it, if you swap the fb to a new
>> one you must not unreference it until after a new fb arrives and set as
>> current swapping out this one (in some following commit).
>> Why you think that making this the generic solution by moving this from
>> dm_plane_atomic_async_check to drm_atomic_helper_async_check
>> will break other drivers ?
> Please raise and discuss this with other developers on the dri-devel
> mailing list. :)

Sure, but to me seems the best way to discuss this with dri-devel is to 
move this to
the generic DRM code and submit a patch - this will trigger a discussion 
anyway.

Andrey

>
>
> Anyway, this looks like a much better solution for the time being than
> the previous patch,
>
> Acked-by: Michel Dänzer <michel.daenzer at amd.com>
>
>