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@xxxxxxx> >>>> >>>> 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@xxxxxxx> > > _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
