From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 02011E7D0A1 for ; Fri, 22 Sep 2023 00:09:40 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8EFF510E0C4; Fri, 22 Sep 2023 00:09:40 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id E261110E0C4 for ; Fri, 22 Sep 2023 00:09:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1695341378; x=1726877378; h=date:message-id:from:to:subject:in-reply-to:references: mime-version; bh=6NHPobWbKrnfmRISC42jyi8XOxjbIf4BbqXwXfvfqIk=; b=kpMTc1gBORZKskgvqHnZTQkplGBF1/k3ihHi+lKDAbsukvYRosDdKGi/ wgr2BNZcbMGW587+w2ixPiNRKOrnVtYC8dacbYWESfP6FaU04SOncMCq3 FMpQJqwvWdcybRqymp14qgOZZR4ZcTf5ukQDZx4PN0+qj1dglIlY9gy54 yAc1s0pqSgwL5wRyTk3yYd/2GsasKpVyG37CgzTJ2A2+Nzqvs0n4OPvVd 5euN570x8Necj17hrNFEkWky2Lw2RKn3lghvr15Wy3sT3GwOx/heL05Bz 66p/QMedNsDxvY4Cyt8PmsuRZwMkQ72NpdbBw+EY2k2aJDOtHrqrnhSOA w==; X-IronPort-AV: E=McAfee;i="6600,9927,10840"; a="447185113" X-IronPort-AV: E=Sophos;i="6.03,166,1694761200"; d="scan'208";a="447185113" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Sep 2023 17:09:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10840"; a="871047887" X-IronPort-AV: E=Sophos;i="6.03,166,1694761200"; d="scan'208";a="871047887" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.251.19.132]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Sep 2023 17:09:38 -0700 Date: Thu, 21 Sep 2023 16:53:20 -0700 Message-ID: <878r8zhymn.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: intel-xe@lists.freedesktop.org In-Reply-To: <20230919161049.2307855-22-ashutosh.dixit@intel.com> References: <20230919161049.2307855-1-ashutosh.dixit@intel.com> <20230919161049.2307855-22-ashutosh.dixit@intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/29.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [Intel-xe] [PATCH 21/21] drm/xe/uapi: Convert OA property key/value pairs to a struct X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" 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. Thanks. -- Ashutosh > Suggested-by: Umesh Nerlige Ramappa > Signed-off-by: Ashutosh Dixit /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 >