linux-kernel - Re: [PATCH] drm/panthor: add query for calibrated timstamp info

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPaKu7Qo1N4iw+JAd-Kcq0GdAw6u0F83iwPjH-u1u406yxAQTA@mail.gmail.com>
Date: Thu, 2 Oct 2025 18:10:11 -0700
From: Chia-I Wu <olvaffe@...il.com>
To: Marcin Ślusarz <marcin.slusarz@....com>
Cc: Boris Brezillon <boris.brezillon@...labora.com>, Steven Price <steven.price@....com>, 
	Liviu Dudau <liviu.dudau@....com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	Grant Likely <grant.likely@...aro.org>, Heiko Stuebner <heiko@...ech.de>, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, nd@....com, 
	Lukas Zapolskas <lukas.zapolskas@....com>
Subject: Re: [PATCH] drm/panthor: add query for calibrated timstamp info

On Thu, Sep 25, 2025 at 2:06 AM Marcin Ślusarz <marcin.slusarz@....com> wrote:
>
> Hi Chia-I,
>
> On Tue, Sep 16, 2025 at 01:07:51PM -0700, Chia-I Wu wrote:
> > DRM_PANTHOR_DEV_QUERY_CALIBRATED_TIMESTAMP_INFO provides a way to query
> > and calibrate CPU and GPU timestamps.
>
> I worked on a similar patch for Panthor, with a plan of submitting
> it upstream soon, but with slightly different requirements, so maybe
> we could merge both efforts in a single patch?
Yeah, that should be the best!

>
> The first requirement was that it should be possible to get both CPU
> and GPU timestamps, with the expectation that they should be taken as
> close as possible (within 50us).
>
> The second requirement was that it should be possible to also get
> the value of GPU_CYCLE_COUNT register.
>
> What I did is extend the existing DRM_PANTHOR_DEV_QUERY_TIMESTAMP_INFO
> query in backward compatible manner with those new fields and obtaining
> gpu and cpu timestamps with preemption and local irqs disabled (more on
> that later).
>
> Backward compatibility was achieved by adding new fields at the end of
> struct drm_panthor_timestamp_info, and relying on the fact that if user
> space passes smaller object it will be silently truncated.
I chose a new query because userspace does not zero-initialize
drm_panthor_timestamp_info. We will get garbage if we add an input
field to the struct.

But this is a non-issue if we agree to do it this way, and make sure
userspace zero-initialize before it updates the uapi header.

>
> Obtaining all kind of timing information with a single syscall might
> be a bit too much, when user space might be interested only in some
> data and not the complete view, so I'd propose this as a solution:
>
> 1) Extend existing query in backward compatible manner, by adding new
> fields at the end.
> 2) Add flags, cpu timestamp, cycle count, and duration.
> 3) Flags would be:
> DRM_PANTHOR_TIMESTAMP_GPU (1<<0)
> DRM_PANTHOR_TIMESTAMP_CPU (1<<1)
> DRM_PANTHOR_TIMESTAMP_OFFSET (1<<2)
> DRM_PANTHOR_TIMESTAMP_FREQ (1<<3)
> DRM_PANTHOR_TIMESTAMP_DURATION (1<<4)
> DRM_PANTHOR_TIMESTAMP_SAME_TIME (1<<5)
>
> DRM_PANTHOR_TIMESTAMP_CPU_MONOTONIC (0<<8)
> DRM_PANTHOR_TIMESTAMP_CPU_MONOTONIC_RAW (1<<8)
> DRM_PANTHOR_TIMESTAMP_CPU_REALTIME (2<<8)
> DRM_PANTHOR_TIMESTAMP_CPU_BOOTTIME (3<<8)
> DRM_PANTHOR_TIMESTAMP_CPU_TAI (4<<8)
>
> and DRM_PANTHOR_TIMESTAMP_CPU_TYPE_MASK would be (7<<8).
>
> If flags is 0 it would become
> (DRM_PANTHOR_TIMESTAMP_GPU |
>  DRM_PANTHOR_TIMESTAMP_OFFSET |
>  DRM_PANTHOR_TIMESTAMP_FREQ)
It is more typical to have NO_GPU/NO_OFFSET/NO_FREQ, but I think
handling 0 specially can work too.

>
> For VK_KHR_calibrated_timestamps flags would be set as
> (DRM_PANTHOR_TIMESTAMP_GPU |
>  DRM_PANTHOR_TIMESTAMP_CPU |
>  DRM_PANTHOR_TIMESTAMP_DURATION |
>  DRM_PANTHOR_TIMESTAMP_SAME_TIME |
>  (raw ? DRM_PANTHOR_TIMESTAMP_CPU_MONOTONIC_RAW : DRM_PANTHOR_TIMESTAMP_CPU_MONOTONIC))
>
> 4) The core of the functionality would query all required timing
> information with preemption and irqs disabled iif SAME_TIME flag is set.
> Probably we should exclude OFFSET and FREQ from that.
>
> Why also interrupts disabled?
> Recently we discovered that unrelated devices can raise interrupts for
> so long that the assumption of timestamps being taken at the same time
> completely breaks down (they are hundreds of microseconds apart).
>
> What do you think?
I am happy to use your version. Do you plan to work on the userpsace
change as well? Otherwise, I can update my userspace change to use
your version as well.