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

[Michel Dänzer]

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. :)


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>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer