Re: [PATCH v3] drm/panthor: Add DEV_QUERY_TIMESTAMP_INFO dev query

[Mihail Atanassov <mihail.atanassov@arm.com>]

On 28/08/2024 12:30, Boris Brezillon wrote:
> On Mon, 19 Aug 2024 13:25:08 +0200
> Mary Guillemard <mary.guillemard@collabora.com> wrote:
> 
>> +/**
>> + * struct drm_panthor_timestamp_info - Timestamp information
>> + *
>> + * Structure grouping all queryable information relating to the GPU timestamp.
>> + */
>> +struct drm_panthor_timestamp_info {
>> +	/** @timestamp_frequency: The frequency of the timestamp timer. */
>> +	__u64 timestamp_frequency;
>> +
>> +	/** @current_timestamp: The current timestamp. */
>> +	__u64 current_timestamp;
>> +
>> +	/** @timestamp_offset: The offset of the timestamp timer. */
>> +	__u64 timestamp_offset;
> 
> I'm not sure we should expose the offset to be honest. It looks like
> something that's related to suspend/resume which only the kernel needs
> to know.

Just for some context: it's necessary information to verify that the GPU 
timestamp correlates well with the CPU timestamp. It's untestable (in 
userspace) without the offset.

The struct can be extended later if really needed, so while I'd like to 
have the field stay, I'm not going to argue very hard against removing it.

> 
>> +};

-- 
Mihail Atanassov <mihail.atanassov@arm.com>