Op 25-07-17 om 11:27 schreef Daniel Vetter:
> On Tue, Jul 25, 2017 at 11:11 AM, Maarten Lankhorst
> <maarten.lankhorst@xxxxxxxxxxxxxxx> wrote:
>> Op 25-07-17 om 10:23 schreef Daniel Vetter:
>>> On Wed, Jul 19, 2017 at 04:39:15PM +0200, Maarten Lankhorst wrote:
>>>> /*
>>>> - * Don't do an async update if there is an outstanding commit modifying
>>>> - * the plane. This prevents our async update's changes from getting
>>>> - * overridden by a previous synchronous update's state.
>>>> + * FIXME: We should prevent an async update if there is an outstanding
>>>> + * commit modifying the plane. This prevents our async update's
>>>> + * changes from getting overwritten by a previous synchronous update's
>>>> + * state.
>>>> */
>>>> - for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
>>>> - if (plane->crtc != crtc)
>>>> - continue;
>>>> -
>>>> - spin_lock(&crtc->commit_lock);
>>>> - commit = list_first_entry_or_null(&crtc->commit_list,
>>>> - struct drm_crtc_commit,
>>>> - commit_entry);
>>>> - if (!commit) {
>>>> - spin_unlock(&crtc->commit_lock);
>>>> - continue;
>>>> - }
>>>> - spin_unlock(&crtc->commit_lock);
>>>> -
>>>> - if (!crtc->state->state)
>>>> - continue;
>>>> -
>>>> - for_each_plane_in_state(crtc->state->state, __plane,
>>>> - __plane_state, j) {
>>> I'm pretty sure this oopses, because crtc->state->state is NULL after
>>> commit. I think Gustavo needs to review this first (and write a nasty igt
>>> testcase to catch it) before we remove this. I think the correct check is
>>> to simply bail out if our current crtc has a pending commit (i.e.
>>> !list_empty(&crtc->commit_list) should be all we need to check.
>> It didn't oops. Right above it was a null check. It was never executed. :)
>>
>> obj->state->state is always NULL. Excluding a brief moment during swap_state where this may oops, this code willl never do a thing.
> Oh right. It's still completely buggy, and I'd like to fix that first,
> testcase included. Can you pls poke Gustavo a bit (or maybe he's on
> vacation)?
> -Daniel
The only thing we have atm excercising it is kms_cursor_legacy, but that doesn't check if flips can be overwritten.
Perhaps we should make IGT tests a requirement for features in the future?
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
