Message-ID: <35169619-479d-24f4-c830-a95a9ef49bb1@mailbox.org>
Date:   Fri, 3 Dec 2021 19:03:08 +0100
MIME-Version: 1.0
Subject: Re: [PATCH] drm: send vblank event with the attached sequence rather
 than current
Content-Language: en-CA
To:     Matthias Brugger <matthias.bgg@gmail.com>,
        Mark Yacoub <markyacoub@chromium.org>,
        dri-devel@lists.freedesktop.org
Cc:     chunkuang.hu@kernel.org, Thomas Zimmermann <tzimmermann@suse.de>,
        David Airlie <airlied@linux.ie>, jason-jh.lin@mediatek.com,
        linux-kernel@vger.kernel.org, tzungbi@google.com,
        seanpaul@chromium.org, Mark Yacoub <markyacoub@google.com>
References: <20211202151200.3125685-1-markyacoub@chromium.org>
 <392a239a-14da-c544-a1f9-09d8b25d3e07@gmail.com>
From:   =?UTF-8?Q?Michel_D=c3=a4nzer?= <michel.daenzer@mailbox.org>
In-Reply-To: <392a239a-14da-c544-a1f9-09d8b25d3e07@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 2021-12-03 16:58, Matthias Brugger wrote:
> Hi Mark,
> 
> On 02/12/2021 16:11, Mark Yacoub wrote:
>> From: Mark Yacoub <markyacoub@google.com>
>>
> 
> please make sure to add the linux-mediatek mailinglist in any follow-up communication.
> 
> Regards,
> Matthias
> 
>> [Why]
>> drm_handle_vblank_events loops over vblank_event_list to send any event
>> that is current or has passed.
>> More than 1 event could be pending with past sequence time that need to
>> be send. This can be a side effect of drivers without hardware vblank
>> counter and they depend on the difference in the timestamps and the
>> frame/field duration calculated in drm_update_vblank_count. This can
>> lead to 1 vblirq being ignored due to very small diff,

That sounds like the real issue which needs to be addressed. As Ville wrote, the sequence value in the event is supposed to be the current sequence, not the one which was specified originally by user space. So this change would basically break the universe. ;)


>> resulting in a subsequent vblank with 2 pending vblank events to be sent, each with a
>> unique sequence expected by user space.
>>
>> [How]
>> Send each pending vblank event with the sequence it's waiting on instead
>> of assigning the current sequence to all of them.
>>
>> Fixes igt@kms_flip "Unexpected frame sequence"
>> Tested on Jacuzzi (MT8183)
>>
>> Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
>> ---
>>   drivers/gpu/drm/drm_vblank.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
>> index 3417e1ac79185..47da8056abc14 100644
>> --- a/drivers/gpu/drm/drm_vblank.c
>> +++ b/drivers/gpu/drm/drm_vblank.c
>> @@ -1902,7 +1902,7 @@ static void drm_handle_vblank_events(struct drm_device *dev, unsigned int pipe)
>>             list_del(&e->base.link);
>>           drm_vblank_put(dev, pipe);
>> -        send_vblank_event(dev, e, seq, now);
>> +        send_vblank_event(dev, e, e->sequence, now);
>>       }
>>         if (crtc && crtc->funcs->get_vblank_timestamp)
>>


-- 
Earthling Michel Dänzer            |                  https://redhat.com
Libre software enthusiast          |         Mesa and Xwayland developer