On Thu, Sep 25, 2025 at 2:06 AM Marcin Ślusarz <[email protected]> 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.
