[Intel-xe] [PATCH 21/21] drm/xe/uapi: Convert OA property key/value pairs to a struct
Umesh Nerlige Ramappa
umesh.nerlige.ramappa at intel.com
Thu Oct 5 19:26:08 UTC 2023
On Wed, Oct 04, 2023 at 10:37:09PM -0700, Dixit, Ashutosh wrote:
>On Thu, 21 Sep 2023 16:53:20 -0700, Dixit, Ashutosh wrote:
>>
>
>Hi Umesh,
>
>> On Tue, 19 Sep 2023 09:10:49 -0700, Ashutosh Dixit wrote:
>> >
>> > Change OA uapi to take a param struct rather than property key value
>> > pairs. A param struct is simpler and param structs can be extenended in the
>> > future using xe_user_extension so there seems to be no reason to use
>> > property key value pairs.
>>
>> There are two ways of doing this:
>>
>> 1. In this patch we have collected all OA properties into a single
>> struct. The assumption is that any future changes would be handled via
>> 'struct drm_xe_ext_set_property' chained structs (basically using
>> xe_user_extension):
>>
>> https://patchwork.freedesktop.org/patch/558715/
>>
>> 2. The second way to do it would be to use chained 'struct
>> drm_xe_ext_set_property' from the beginning as is being done for
>> DRM_XE_EXEC_QUEUE_SET_PROPERTY. This is basically the same as the
>> earlier OA property key/value pairs except that the properties are now
>> input via chained structs.
>>
>> This second way is a uniform way of specifying property values whereas
>> the first way in non-uniform.
>>
>> Just thought I'll point this out when we decide about this uapi during the
>> code review.
>
>Since we are almost certain that the OA uapi will need to be extended in
>the future, for the sake of uniformity maybe we should go with the approach
>2. above (this patch implements approach 1.)? Something to keep in mind for
>the review of this patch. This is what I was referring to earlier today.
Let's go with 2 then. I will skip reviewing this.
>
>Thanks.
>--
>Ashutosh
>
>>
>> > Suggested-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit at intel.com>
>>
>> /snip/
>>
>> > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
>> > index c0018abee4052..8ba11c4eb36b5 100644
>> > --- a/include/uapi/drm/xe_drm.h
>> > +++ b/include/uapi/drm/xe_drm.h
>> > @@ -1175,30 +1175,26 @@ struct drm_xe_query_oa_info {
>> > } oau[];
>> > };
>> >
>> > -enum drm_xe_oa_property_id {
>> > - /**
>> > - * ID of the OA unit on which to open the OA stream, see
>> > - * @oa_unit_id in 'struct drm_xe_engine_class_instance'. Defaults
>> > - * to 0 if not provided.
>> > - */
>> > - DRM_XE_OA_PROP_OA_UNIT_ID = 1,
>> > +struct drm_xe_oa_open_param {
>> > + /** @extensions: Pointer to the first extension struct, if any */
>> > + __u64 extensions;
>> >
>> > /**
>> > - * A value of 1 requests the inclusion of raw OA unit reports as
>> > - * part of stream samples.
>> > + * @oa_unit_id: ID of the OA unit on which to open the OA stream,
>> > + * see @oa_unit_id in struct @drm_xe_engine_class_instance
>> > */
>> > - DRM_XE_OA_PROP_SAMPLE_OA,
>> > + __u32 oa_unit_id;
>> >
>> > /**
>> > - * The value specifies which set of OA unit metrics should be
>> > - * configured, defining the contents of any OA unit reports.
>> > + * @sample_oa: A value of 1 requests the inclusion of raw OA unit
>> > + * reports as part of stream samples
>> > */
>> > - DRM_XE_OA_PROP_OA_METRICS_SET,
>> > + __u32 sample_oa;
>> >
>> > /**
>> > - * The value specifies the size and layout of OA unit reports.
>> > + * @oa_format: The value specifies the size and layout of OA unit reports
>> > */
>> > - DRM_XE_OA_PROP_OA_FORMAT,
>> > + __u64 oa_format;
>> > /**
>> > * OA_FORMAT's are specified the same way as in Bspec, in terms of
>> > * the following quantities: a. enum @drm_xe_oa_format_type
>> > @@ -1210,86 +1206,79 @@ enum drm_xe_oa_property_id {
>> > #define XE_OA_MASK_BC_REPORT (0xff << 24)
>> >
>> > /**
>> > - * Specifying this property implicitly requests periodic OA unit
>> > - * sampling and (at least on Haswell) the sampling frequency is derived
>> > - * from this exponent as follows:
>> > - *
>> > - * 80ns * 2^(period_exponent + 1)
>> > + * @metric_set: specifies which set of OA unit metrics should be
>> > + * configured, defining the contents of any OA unit reports. Metric
>> > + * set ID is returned by the XE_PERF_ADD_CONFIG op of the PREF ioctl
>> > */
>> > - DRM_XE_OA_PROP_OA_EXPONENT,
>> > + __u32 metric_set;
>> >
>> > /**
>> > - * Specifying this property is only valid when specify a context to
>> > - * filter with DRM_XE_OA_PROP_ENGINE_ID. Specifying this property
>> > - * will hold preemption of the particular engine we want to gather
>> > - * performance data about.
>> > + * @period_exponent: Specifying this property implicitly requests
>> > + * periodic OA unit sampling. The sampling period is:
>> > + *
>> > + * 2^(period_exponent + 1) / @oa_timestamp_freq
>> > + *
>> > + * Set period_exponent *negative* to disable periodic sampling
>> > */
>> > - DRM_XE_OA_PROP_HOLD_PREEMPTION,
>> > + __s32 period_exponent;
>> >
>> > /**
>> > - * Specify a global OA buffer size to be allocated in bytes. The
>> > - * size specified must be supported by HW (powers of 2 ranging from
>> > - * 128 KB to 128Mb depending on the platform)
>> > + * @oa_buffer_size: Specify a global OA buffer size to be allocated
>> > + * in bytes. The size specified must be supported by HW (powers of
>> > + * 2 ranging from 128 KB to 128Mb depending on the platform). A
>> > + * value of 0 will choose a default size of 16 MB.
>> > */
>> > - DRM_XE_OA_PROP_OA_BUFFER_SIZE,
>> > + __u32 oa_buffer_size;
>> >
>> > /**
>> > - * This optional parameter specifies the timer interval in nanoseconds
>> > - * at which the xe driver will check the OA buffer for available data.
>> > - * Minimum allowed value is 100 microseconds. A default value is used by
>> > - * the driver if this parameter is not specified. Note that larger timer
>> > - * values will reduce cpu consumption during OA perf captures. However,
>> > - * excessively large values would potentially result in OA buffer
>> > - * overwrites as captures reach end of the OA buffer.
>> > + * @poll_period: Specify timer interval in micro-seconds at which
>> > + * the xe driver will check the OA buffer for available
>> > + * data. Minimum allowed value is 100 microseconds. A value of 0
>> > + * selects a default value is used by the driver. Note that larger
>> > + * timer values will reduce cpu consumption during OA perf
>> > + * captures. However, excessively large values would potentially
>> > + * result in OA buffer overwrites as captures reach end of the OA
>> > + * buffer.
>> > */
>> > - DRM_XE_OA_PROP_POLL_OA_PERIOD,
>> > + __u32 poll_period_us;
>> > +
>> > + /** @open_flags: Flags */
>> > + __u32 open_flags;
>> > +#define XE_OA_FLAG_FD_CLOEXEC (1 << 0)
>> > +#define XE_OA_FLAG_FD_NONBLOCK (1 << 1)
>> > +#define XE_OA_FLAG_DISABLED (1 << 2)
>> >
>> > /**
>> > - * Open the stream for a specific exec queue id (as used with
>> > - * drm_xe_exec). A stream opened for a specific exec queue id this
>> > - * way won't typically require root privileges.
>> > + * @exec_queue_id: Open the stream for a specific exec queue id (as
>> > + * used with drm_xe_exec). A stream opened for a specific exec
>> > + * queue id this way won't typically require root
>> > + * privileges. Pass a value <= 0 to not specify an exec queue id.
>> > */
>> > - DRM_XE_OA_PROP_EXEC_QUEUE_ID,
>> > + __s32 exec_queue_id;
>> >
>> > /**
>> > - * This parameter specifies the engine instance and can be passed along
>> > - * with DRM_XE_OA_PROP_EXEC_QUEUE_ID or will default to 0.
>> > + * @engine_instance: engine instance to use with @exec_queue_id.
>> > */
>> > - DRM_XE_OA_PROP_OA_ENGINE_INSTANCE,
>> > + __u32 engine_instance;
>> >
>> > - DRM_XE_OA_PROP_MAX /* non-ABI */
>> > -};
>> > -
>> > -struct drm_xe_oa_open_param {
>> > - /** @extensions: Pointer to the first extension struct, if any */
>> > - __u64 extensions;
>> > + /**
>> > + * @hold_preemption: If true, this will disable preemption for the
>> > + * exec queue selected with @exec_queue_id
>> > + */
>> > + __u32 hold_preemption;
>> >
>> > /**
>> > - * @config_syncobj: (Output) handle to configuration syncobj
>> > + * @config_syncobj: (output) handle to configuration syncobj
>> > *
>> > * Handle to a syncobj which the kernel will signal after stream
>> > * configuration or re-configuration is complete (after return from
>> > * the ioctl). This handle can be provided as a dependency to the
>> > - * next XE exec ioctl.
>> > + * next xe exec ioctl to synchronize xe exec with oa config changes
>> > */
>> > __u32 config_syncobj;
>> >
>> > - __u32 reserved;
>> > -
>> > - /** @flags: Flags */
>> > - __u32 flags;
>> > -#define XE_OA_FLAG_FD_CLOEXEC (1 << 0)
>> > -#define XE_OA_FLAG_FD_NONBLOCK (1 << 1)
>> > -#define XE_OA_FLAG_DISABLED (1 << 2)
>> > -
>> > - /** The number of u64 (id, value) pairs */
>> > - __u32 num_properties;
>> > -
>> > - /**
>> > - * Pointer to array of u64 (id, value) pairs configuring the stream
>> > - * to open.
>> > - */
>> > - __u64 properties_ptr;
>> > + /** @reserved: reserved (MBZ) */
>> > + __u64 reserved[4];
>> > };
>> >
>> > struct drm_xe_oa_record_header {
>> > --
>> > 2.41.0
>> >
More information about the Intel-xe
mailing list