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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mary,

Thanks for your patch.

On 07/08/2024 16:35, Mary Guillemard wrote:
Expose system timestamp and frequency supported by the GPU with a
new device query.

Mali uses the generic arch timer as GPU system time so we currently wire cntvct_el0 and cntfrq_el0 respectively to a new device query. We
could have directly read those values from userland but handling this
here allows us to be future proof in case of architectural changes or
erratas related to timers for example.

This new uAPI will be used in Mesa to implement timestamp queries
and VK_KHR_calibrated_timestamps.

Since this extends the uAPI and because userland needs a way to
advertise those features conditionally, this also bumps the driver
minor version.> Signed-off-by: Mary Guillemard
<mary.guillemard@xxxxxxxxxxxxx> --- drivers/gpu/drm/panthor/panthor_drv.c | 23 ++++++++++++++++++++++- include/uapi/drm/panthor_drm.h | 16 ++++++++++++++++ 2 files
changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panthor/panthor_drv.c
b/drivers/gpu/drm/panthor/panthor_drv.c index
b8a84f26b3ef..4d62ff3fbe03 100644 ---
a/drivers/gpu/drm/panthor/panthor_drv.c +++
b/drivers/gpu/drm/panthor/panthor_drv.c @@ -164,6 +164,7 @@
panthor_get_uobj_array(const struct drm_panthor_obj_array *in, u32
min_stride, _Generic(_obj_name, \ PANTHOR_UOBJ_DECL(struct
drm_panthor_gpu_info, tiler_present), \ PANTHOR_UOBJ_DECL(struct
drm_panthor_csif_info, pad), \ +		 PANTHOR_UOBJ_DECL(struct
drm_panthor_timestamp_info, current_timestamp), \ PANTHOR_UOBJ_DECL(struct drm_panthor_sync_op, timeline_value), \ PANTHOR_UOBJ_DECL(struct drm_panthor_queue_submit, syncs), \ PANTHOR_UOBJ_DECL(struct drm_panthor_queue_create, ringbuf_size), \ @@ -750,10 +751,21 @@ static void panthor_submit_ctx_cleanup(struct
panthor_submit_ctx *ctx, kvfree(ctx->jobs); }

+static void panthor_ioctl_query_timestamp(struct
drm_panthor_timestamp_info *arg) +{ +#ifdef CONFIG_ARM_ARCH_TIMER +
arg->timestamp_frequency = arch_timer_get_cntfrq(); +
arg->current_timestamp = __arch_counter_get_cntvct();

The GPU HW doesn't use the arch counter directly, there's a dedicated
timestamp register that can, e.g.:
 - get dumped in perfcnt dumps
 - get sampled in shader programs
 - get dumped in perf instrumentation trace buffers by FW

This GPU timestamp sometimes/usually needs to have an offset applied to
it by SW via a dedicated register, for example, to:
 - account for errata like a slight difference in the clock frequencies
between the system and the GPU counters leading to drift, or
 - account for errata like an existing fixed offset between system and
GPU timestamps, or
 - provide a stable relationship to a known system clock source like
CLOCK_MONOTONIC_RAW, to assist in correlating events in perfcnt dumps &
traces to other system events

+#else +	memset(arg, 0, sizeof(*arg));

[aside] The GPU timestamp register semantics are also defined for a
non-ARM-CPU based system. How you get the frequency on such systems,
though, is not something I can help with :). [/aside]

+#endif +} + static int panthor_ioctl_dev_query(struct drm_device
*ddev, void *data, struct drm_file *file) { struct panthor_device
*ptdev = container_of(ddev, struct panthor_device, base); struct
drm_panthor_dev_query *args = data; +	struct
drm_panthor_timestamp_info timestamp_info;

if (!args->pointer) { switch (args->type) { @@ -765,6 +777,10 @@
static int panthor_ioctl_dev_query(struct drm_device *ddev, void
*data, struct d args->size = sizeof(ptdev->csif_info); return 0;

+		case DRM_PANTHOR_DEV_QUERY_TIMESTAMP_INFO: +			args->size =
sizeof(timestamp_info); +			return 0; + default: return -EINVAL; } @@
-777,6 +793,10 @@ static int panthor_ioctl_dev_query(struct
drm_device *ddev, void *data, struct d case
DRM_PANTHOR_DEV_QUERY_CSIF_INFO: return
PANTHOR_UOBJ_SET(args->pointer, args->size, ptdev->csif_info);

+	case DRM_PANTHOR_DEV_QUERY_TIMESTAMP_INFO: +
panthor_ioctl_query_timestamp(&timestamp_info); +		return
PANTHOR_UOBJ_SET(args->pointer, args->size, timestamp_info); + default: return -EINVAL; } @@ -1372,6 +1392,7 @@ static void
panthor_debugfs_init(struct drm_minor *minor) /* * PanCSF driver
version: * - 1.0 - initial interface + * - 1.1 - adds
DEV_QUERY_TIMESTAMP_INFO query */ static const struct drm_driver
panthor_drm_driver = { .driver_features = DRIVER_RENDER | DRIVER_GEM
| DRIVER_SYNCOBJ | @@ -1385,7 +1406,7 @@ static const struct
drm_driver panthor_drm_driver = { .desc = "Panthor DRM driver", .date
= "20230801", .major = 1, -	.minor = 0, +	.minor = 1,

.gem_create_object = panthor_gem_create_object, .gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, diff --git a/include/uapi/drm/panthor_drm.h
b/include/uapi/drm/panthor_drm.h index aaed8e12ad0b..8d39a2f91769
100644 --- a/include/uapi/drm/panthor_drm.h +++
b/include/uapi/drm/panthor_drm.h @@ -260,6 +260,9 @@ enum
drm_panthor_dev_query_type {

/** @DRM_PANTHOR_DEV_QUERY_CSIF_INFO: Query command-stream interface
information. */ DRM_PANTHOR_DEV_QUERY_CSIF_INFO, + +	/**
@DRM_PANTHOR_DEV_QUERY_TIMESTAMP_INFO: Query timestamp information.
*/ +	DRM_PANTHOR_DEV_QUERY_TIMESTAMP_INFO, };

/** @@ -377,6 +380,19 @@ struct drm_panthor_csif_info { __u32 pad; };

+/** + * 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;

As it stands, this query has nothing to do with the actual GPU so
doesn't really belong here.

It'd be more valuable, and can maybe give better calibration results
than querying the system timestamp separately in userspace, if you
reported all of:
 * the system timer value
 * the system timer frequency
 * the GPU timer value
* the GPU timer frequency (because it _could_ be different in some systems)
 * the GPU timer offset

+}; + /** * struct drm_panthor_dev_query - Arguments passed to
DRM_PANTHOR_IOCTL_DEV_QUERY */

base-commit: f7f3ddb6e5c8dc7b621fd8c0903ea42190d67452

--
Mihail Atanassov <mihail.atanassov@xxxxxxx>



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux