linux-kernel - Re: [PATCH] drm/vblank: Fix delta_ns to an absolute value

Open Source and information security mailing list archives

Message-id: <b54fc435-5ffd-389d-2c51-0bcb0b524a49@samsung.com>
Date:   Tue, 05 Sep 2017 15:36:52 +0900
From:   Hoegeun Kwon <hoegeun.kwon@...sung.com>
To:     Ville Syrjälä <ville.syrjala@...ux.intel.com>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        daniel.vetter@...el.com
Cc:     Hoegeun Kwon <hoegeun.kwon@...sung.com>
Subject: Re: [PATCH] drm/vblank: Fix delta_ns to an absolute value

On 09/04/2017 04:36 PM, Daniel Vetter wrote:
> On Fri, Sep 01, 2017 at 04:36:25PM +0300, Ville Syrjälä wrote:
>> On Fri, Sep 01, 2017 at 04:07:16PM +0900, Hoegeun Kwon wrote:
>>> If scanout started, we should reduce etime by delta_ns. But delta_ns
>>> is negative if scanout has not started. If delta_ns is negative,
>>> subtraction of delta_ns from etime increases etime. This is wrong, the
>>> etime should not be increased, so you have to make delta_ns an
>>> absolute value.
>>>
>>> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@...sung.com>
>>> ---
>>>
>>> Hello all,
>>>
>>> I think that the etime should not be increased.
>>> In cases where delta_ns is negative, if you get time again after an
>>> interrupt call, there is a problem that the time obtained from the
>>> interrupt becomes the future time instead of the past time.
>>>
>>> Please let me know if this patch is wrong.
>> It is wrong. The timestamp corresponds to the first active pixel of the
>> frame/field. So while between vblank start and active start we expect
>> to get a timestamp that is in the future.
> A bit more strict, it's up to drivers when exactly the vblank interrupt
> fires. Some fire at vblank start, some at the end, some somewhen around
> that. The only rule is that the vblank must not fire before the point of
> no return for another page_flip call, i.e. after the vblank event goes
> out, the next page_flip (or atomic ioctl) must go to the next vblank.
>
> So yeah both positive and negative deltas make perfect sense.
> -Daniel

Thanks Ville and Daniel,

Your response has been really helpful.
Thanks so much.

Best regards,
Hoegeun

>>> Best regards,
>>> Hoegeun
>>>
>>>   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 70f2b95..a3e0176 100644
>>> --- a/drivers/gpu/drm/drm_vblank.c
>>> +++ b/drivers/gpu/drm/drm_vblank.c
>>> @@ -684,7 +684,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>>>   	/* Subtract time delta from raw timestamp to get final
>>>   	 * vblank_time timestamp for end of vblank.
>>>   	 */
>>> -	etime = ktime_sub_ns(etime, delta_ns);
>>> +	etime = ktime_sub_ns(etime, abs(delta_ns));
>>>   	*vblank_time = ktime_to_timeval(etime);
>>>   
>>>   	DRM_DEBUG_VBL("crtc %u : v p(%d,%d)@ %ld.%ld -> %ld.%ld [e %d us, %d rep]\n",
>>> -- 
>>> 1.9.1
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@...ts.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> -- 
>> Ville Syrjälä
>> Intel OTC
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@...ts.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives