[prev in list] [next in list] [prev in thread] [next in thread] 

List:       dri-devel
Subject:    Re: [PATCH 1/3] drm/vblank: Introduce drm_crtc_vblank_get_accurate()
From:       Daniel Vetter <daniel () ffwll ! ch>
Date:       2017-06-30 18:18:19
Message-ID: CAKMK7uEpTfkyHLzqiDQA0izz1MQskmP-z+EgB4PbyvOTB2KjLA () mail ! gmail ! com
[Download RAW message or body]

On Fri, Jun 30, 2017 at 5:26 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Jun 30, 2017 at 05:18:41PM +0300, ville.syrjala@linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Add drm_crtc_vblank_get_accurate() which is the same as
>> drm_crtc_vblank_get() except that it will forcefully update the
>> the current vblank counter/timestamp value even if the interrupt
>> is already enabled.
>>
>> And we'll switch drm_wait_one_vblank() over to using it to make sure it
>> will really wait at least one vblank even if it races with the irq
>> handler.
>>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Hm, I just thought of doing an
> s/drm_vblank_count/drm_crtc_accurate_vblank_count/ in
> drm_crtc_arm_vblank_event.
>
> What does this buy us above&beyond the other patch? And why isn't
> vblank_get accurate always?

This also seems to have the risk of not fixing the arm_vblank_event
race if someone puts the vblank_get_accurate outside of the critical
section. Then we're back to the same old race. And since that means
you need to have a vblank_get_accurate right before the
arm_vblank_event in all cases, we might as well just put it into the
helper. You wrote in the other thread putting it into arm_vblank_event
might be wasteful, but I don't see any scenario where that could be
the case ...

Or do I miss something?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[prev in list] [next in list] [prev in thread] [next in thread]