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 40FF7C02181 for ; Thu, 23 Jan 2025 02:19:03 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D38CA10E0DC; Thu, 23 Jan 2025 02:19:02 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="FMKfyw4H"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.12]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9A13110E0DC for ; Thu, 23 Jan 2025 02:19:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1737598741; x=1769134741; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version:content-transfer-encoding; bh=Eeqyc82XBi3XSK8Zk3d9HhSigfICKNJrBPZ3e6LU4/U=; b=FMKfyw4Hctk8e4BSRGMxiO/1tyf1GUVogD8+Ox9ZuNNgUn51DMNUK0ml F0Yjusz8vBgDelAZZ8Ya1O9tdfUbrTm08i1vJthOycKcU+baBU6fQAiNX O5f4cObncaMZ0mPUy2k9ztItRICpmIuGO8CxSzEp41qJUNYokNoz2owqb Uk/c8+FmPg54cV9NRS2V6iLmDzu4UiPMKhnF5mof2Q8BpWR68mIA3o8gQ BI8K/ydoGNKOorG8+a3sAbG90C0VCvrDziKzaNpJEL5C5EERYBN8mJkQ1 vpzuyelbefqZSPxYJ1kzu7Pb05kC6lXWZE5rkkyHvyxNNF68HrEvDST1c g==; X-CSE-ConnectionGUID: fAUPTgp1ToymDAINfqJE0A== X-CSE-MsgGUID: 4GUyXQYORIqEGb6uIe6vzw== X-IronPort-AV: E=McAfee;i="6700,10204,11323"; a="42015938" X-IronPort-AV: E=Sophos;i="6.13,227,1732608000"; d="scan'208";a="42015938" Received: from fmviesa006.fm.intel.com ([10.60.135.146]) by fmvoesa106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Jan 2025 18:19:01 -0800 X-CSE-ConnectionGUID: 8I1YoIxRR8a2yDXeX1FWBA== X-CSE-MsgGUID: UPP8PV2oReGTifRq5rfzLA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.13,227,1732608000"; d="scan'208";a="107141895" Received: from orsosgc001.jf.intel.com (HELO orsosgc001.intel.com) ([10.165.21.142]) by fmviesa006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Jan 2025 18:19:01 -0800 Date: Wed, 22 Jan 2025 18:19:00 -0800 Message-ID: <85h65qff1n.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Harish Chegondi Cc: , , , , , , , , Subject: Re: [PATCH v8 2/7] drm/xe/uapi: Introduce API for EU stall sampling In-Reply-To: References: <85jzate00v.wl-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/28.2 (x86_64-redhat-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=ISO-8859-7 Content-Transfer-Encoding: quoted-printable 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 Wed, 22 Jan 2025 15:44:31 -0800, Harish Chegondi wrote: > Hi Harish, > On Fri, Jan 17, 2025 at 11:02:56AM -0800, Dixit, Ashutosh wrote: > > On Wed, 15 Jan 2025 12:02:08 -0800, Harish Chegondi wrote: > > > > > > Hi Ashutosh, > > This patch doesn't compile :/ > I am able to compile this patch without any errors. Which config file > are you using? Earlier there was a 32-bit build error due to 64-bit > division. I fixed it in this patch with a div_u64(). I get this: HDRTEST xe_force_wake.h In file included from : ./xe_eu_stall.h:10:29: error: unknown type name =A1u64=A2 10 | u64 data, | ^~~ make[3]: *** [Makefile:321: xe_eu_stall.hdrtest] Error 1 Not sure if this comes from 'CONFIG_UAPI_HEADER_TEST=3Dy'. > > > > > A new hardware feature first introduced in PVC gives capability to > > > periodically sample EU stall state and record counts for different st= all > > > reasons, on a per IP basis, aggregate across all EUs in a subslice and > > > record the samples in a buffer in each subslice. Eventually, the aggr= egated > > > data is written out to a buffer in the memory. This feature is also > > > supported in XE2 and later architecture GPUs. > > > > > > Use an existing IOCTL - DRM_IOCTL_XE_OBSERVATION as the interface int= o the > > > driver from the user space to do initial setup and obtain a file desc= riptor > > > for the EU stall data stream. Input parameter to the IOCTL is a stru= ct > > > drm_xe_observation_param in which observation_type should be set to > > > DRM_XE_OBSERVATION_TYPE_EU_STALL, observation_op should be > > > DRM_XE_OBSERVATION_OP_STREAM_OPEN and param should point to a chain of > > > drm_xe_ext_set_property structures in which each structure has a pair= of > > > property and value. The EU stall sampling input properties are define= d in > > > drm_xe_eu_stall_property_id enum. > > > > > > With the file descriptor obtained from DRM_IOCTL_XE_OBSERVATION, user= space > > > can enable and disable EU stall sampling with the IOCTLs: > > > DRM_XE_OBSERVATION_IOCTL_ENABLE and DRM_XE_OBSERVATION_IOCTL_DISABLE. > > > User space can also call poll() to check for availability of data in = the > > > buffer. The data can be read with read(). Finally, the file descriptor > > > can be closed with close(). > > > > > > v8: Used div_u64 instead of / to fix 32-bit build issue. > > > Changed copyright year in xe_eu_stall.c/h to 2025. > > > v7: Renamed input property DRM_XE_EU_STALL_PROP_EVENT_REPORT_COUNT > > > to DRM_XE_EU_STALL_PROP_WAIT_NUM_REPORTS to be consistent with > > > OA. Renamed the corresponding internal variables. > > > Fixed some commit messages based on review feedback. > > > v6: Change the input sampling rate to GPU cycles instead of > > > GPU cycles multiplier. > > > > > > Cc: Felix Degrood > > > Signed-off-by: Harish Chegondi > > > --- > > > drivers/gpu/drm/xe/Makefile | 1 + > > > drivers/gpu/drm/xe/xe_eu_stall.c | 283 ++++++++++++++++++++++++++= ++ > > > drivers/gpu/drm/xe/xe_eu_stall.h | 12 ++ > > > drivers/gpu/drm/xe/xe_observation.c | 14 ++ > > > include/uapi/drm/xe_drm.h | 38 ++++ > > > 5 files changed, 348 insertions(+) > > > create mode 100644 drivers/gpu/drm/xe/xe_eu_stall.c > > > create mode 100644 drivers/gpu/drm/xe/xe_eu_stall.h > > > > > > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile > > > index 7730e0596299..259ccbb0c031 100644 > > > --- a/drivers/gpu/drm/xe/Makefile > > > +++ b/drivers/gpu/drm/xe/Makefile > > > @@ -33,6 +33,7 @@ xe-y +=3D xe_bb.o \ > > > xe_device_sysfs.o \ > > > xe_dma_buf.o \ > > > xe_drm_client.o \ > > > + xe_eu_stall.o \ > > > xe_exec.o \ > > > xe_execlist.o \ > > > xe_exec_queue.o \ > > > diff --git a/drivers/gpu/drm/xe/xe_eu_stall.c b/drivers/gpu/drm/xe/xe= _eu_stall.c > > > new file mode 100644 > > > index 000000000000..48dcc7cb7791 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/xe/xe_eu_stall.c > > > @@ -0,0 +1,283 @@ > > > +// SPDX-License-Identifier: MIT > > > +/* > > > + * Copyright =A9 2025 Intel Corporation > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#include > > > + > > > +#include "xe_macros.h" > > > +#include "xe_device.h" > > > +#include "xe_eu_stall.h" > > > +#include "xe_gt_printk.h" > > > +#include "xe_gt_topology.h" > > > +#include "xe_observation.h" > > > + > > > +/** > > > + * struct eu_stall_open_properties - EU stall sampling properties re= ceived > > > + * from user space at open. > > > + * @eu_stall_sampling_rate: EU stall sampling rate multiplier. > > > + * HW will sample every (eu_stall_sampling_rate x 251) cycles. > > > + * @wait_num_reports: Minimum number of EU stall data reports to unb= lock poll(). > > > + * @gt: GT on which EU stall data will be captured. > > > + */ > > > +struct eu_stall_open_properties { > > > + u8 eu_stall_sampling_rate; > > > + u32 wait_num_reports; > > > > nit but the recommendation is to just use generic types, viz 'int', unl= ess > > you have specific needs to use u8/u32 etc. > > > > > + struct xe_gt *gt; > > > +}; > > > + > > > +/** > > > + * num_data_rows - Return the number of EU stall data rows of 64B ea= ch > > > + * for a given data size. > > > + * > > > + * @data_size: EU stall data size > > > + */ > > > +static inline u32 > > > +num_data_rows(u32 data_size) > > > > Remove inline, put on a single line. > Why remove inline? In .c files inline should not be used, the decision whether to inline functions is left to the compiler, inline is anyway only a hint to the compiler. > > > > Is something like num_cachelines() a better name? > As of now, each data record is cacheline sized, but it may increase in > the future. Ok. > > > > > +{ > > > + return (data_size >> 6); > > > +} > > > + > > > +static int set_prop_eu_stall_sampling_rate(struct xe_device *xe, u64= value, > > > + struct eu_stall_open_properties *props) > > > +{ > > > + value =3D div_u64(value, 251); > > > + if (value =3D=3D 0 || value > 7) { > > > + drm_dbg(&xe->drm, "Invalid EU stall sampling rate %llu\n", value); > > > + return -EINVAL; > > > + } > > > + props->eu_stall_sampling_rate =3D value; > > > + return 0; > > > +} > > > + > > > +static int set_prop_eu_stall_wait_num_reports(struct xe_device *xe, = u64 value, > > > + struct eu_stall_open_properties *props) > > > +{ > > > + u32 max_wait_num_reports; > > > + > > > + max_wait_num_reports =3D num_data_rows(SZ_512K * XE_MAX_DSS_FUSE_BI= TS); > > > > Can't we just add the final function to this patch? > Yes, I can make this change. > > > > max_wait_num_reports =3D num_data_rows(per_xecore_buf_size * XE= _MAX_DSS_FUSE_BITS); > > > > > + if (value =3D=3D 0 || value > max_wait_num_reports) { > > > + drm_dbg(&xe->drm, "Invalid EU stall event report count %llu\n", va= lue); > > > + drm_dbg(&xe->drm, "Minimum event report count is 1, maximum is %u\= n", > > > + max_wait_num_reports); > > > + return -EINVAL; > > > + } > > > + props->wait_num_reports =3D value; > > > + return 0; > > > +} > > > + > > > +static int set_prop_eu_stall_gt_id(struct xe_device *xe, u64 value, > > > + struct eu_stall_open_properties *props) > > > +{ > > > + if (value >=3D xe->info.gt_count) { > > > + drm_dbg(&xe->drm, "Invalid GT ID %llu for EU stall sampling\n", va= lue); > > > + return -EINVAL; > > > + } > > > + props->gt =3D xe_device_get_gt(xe, value); > > > + return 0; > > > +} > > > + > > > +typedef int (*set_eu_stall_property_fn)(struct xe_device *xe, u64 va= lue, > > > + struct eu_stall_open_properties *props); > > > + > > > +static const set_eu_stall_property_fn xe_set_eu_stall_property_funcs= [] =3D { > > > + [DRM_XE_EU_STALL_PROP_SAMPLE_RATE] =3D set_prop_eu_stall_sampling_r= ate, > > > + [DRM_XE_EU_STALL_PROP_WAIT_NUM_REPORTS] =3D set_prop_eu_stall_wait_= num_reports, > > > + [DRM_XE_EU_STALL_PROP_GT_ID] =3D set_prop_eu_stall_gt_id, > > > +}; > > > + > > > +static int xe_eu_stall_user_ext_set_property(struct xe_device *xe, u= 64 extension, > > > + struct eu_stall_open_properties *props) > > > +{ > > > + u64 __user *address =3D u64_to_user_ptr(extension); > > > + struct drm_xe_ext_set_property ext; > > > + int err; > > > + u32 idx; > > > + > > > + err =3D __copy_from_user(&ext, address, sizeof(ext)); > > > + if (XE_IOCTL_DBG(xe, err)) > > > + return -EFAULT; > > > + > > > + if (XE_IOCTL_DBG(xe, ext.property >=3D ARRAY_SIZE(xe_set_eu_stall_p= roperty_funcs)) || > > > + XE_IOCTL_DBG(xe, ext.pad)) > > > + return -EINVAL; > > > + > > > + idx =3D array_index_nospec(ext.property, ARRAY_SIZE(xe_set_eu_stall= _property_funcs)); > > > + return xe_set_eu_stall_property_funcs[idx](xe, ext.value, props); > > > +} > > > + > > > +typedef int (*xe_eu_stall_user_extension_fn)(struct xe_device *xe, u= 64 extension, > > > + struct eu_stall_open_properties *props); > > > +static const xe_eu_stall_user_extension_fn xe_eu_stall_user_extensio= n_funcs[] =3D { > > > + [DRM_XE_EU_STALL_EXTENSION_SET_PROPERTY] =3D xe_eu_stall_user_ext_s= et_property, > > > +}; > > > + > > > +static int xe_eu_stall_user_extensions(struct xe_device *xe, u64 ext= ension, > > > + struct eu_stall_open_properties *props) > > > +{ > > > + u64 __user *address =3D u64_to_user_ptr(extension); > > > + struct drm_xe_user_extension ext; > > > + int err; > > > + u32 idx; > > > + > > > + err =3D __copy_from_user(&ext, address, sizeof(ext)); > > > + if (XE_IOCTL_DBG(xe, err)) > > > + return -EFAULT; > > > + > > > + if (XE_IOCTL_DBG(xe, ext.pad) || > > > + XE_IOCTL_DBG(xe, ext.name >=3D ARRAY_SIZE(xe_eu_stall_user_exte= nsion_funcs))) > > > + return -EINVAL; > > > + > > > + idx =3D array_index_nospec(ext.name, ARRAY_SIZE(xe_eu_stall_user_ex= tension_funcs)); > > > + err =3D xe_eu_stall_user_extension_funcs[idx](xe, extension, props); > > > + if (XE_IOCTL_DBG(xe, err)) > > > + return err; > > > + > > > + if (ext.next_extension) > > > + return xe_eu_stall_user_extensions(xe, ext.next_extension, props); > > > + > > > + return 0; > > > +} > > > + > > > +/** > > > + * xe_eu_stall_stream_read - handles userspace read() of a EU stall = data stream fd. > > > + * > > > + * @file: An xe EU stall data stream file > > > + * @buf: destination buffer given by userspace > > > + * @count: the number of bytes userspace wants to read > > > + * @ppos: (inout) file seek position (unused) > > > + * > > > + * Userspace must enable the EU stall stream with DRM_XE_OBSERVATION= _IOCTL_ENABLE > > > + * before calling read(). > > > + * > > > + * Returns: The number of bytes copied or a negative error code on f= ailure. > > > + */ > > > +static ssize_t xe_eu_stall_stream_read(struct file *file, char __use= r *buf, > > > + size_t count, loff_t *ppos) > > > +{ > > > + ssize_t ret =3D 0; > > > + > > > + return ret; > > > +} > > > + > > > +/** > > > + * xe_eu_stall_stream_poll - handles userspace poll() of a EU stall = data stream fd. > > > + * > > > + * @file: An xe EU stall data stream file > > > + * @wait: Poll table > > > + * > > > + * Returns: Bit mask of returned events. > > > + */ > > > +static __poll_t > > > +xe_eu_stall_stream_poll(struct file *file, poll_table *wait) > > > +{ > > > + __poll_t ret =3D 0; > > > + > > > + return ret; > > > +} > > > + > > > +/** > > > + * xe_eu_stall_stream_ioctl - support ioctl() of a xe EU stall data = stream fd. > > > + * > > > + * @file: An xe EU stall data stream file > > > + * @cmd: the ioctl request > > > + * @arg: the ioctl data > > > + * > > > + * Returns: zero on success or a negative error code. > > > + * -EINVAL for an unknown ioctl request. > > > + */ > > > > What is the purpose of providing documenatation for static functions, s= uch > > as the functions above? There is no useful information in such > > documentation at all. > > > > Let's save some lines of code and vertical real estate and provide > > documenation only for functions which are exposed in a .h. Unless there= is > > a "real" reason for providing documentation for something. > As per the https://docs.kernel.org/doc-guide/kernel-doc.html , > while the documentation is not required for static functions, > it is recommended for consistency. https://github.com/torvalds/linux/blob/master/Documentation/process/4.Codin= g.rst#documentation "Anybody who reads through a significant amount of existing kernel code will note that, often, comments are most notable by their absence. ... That said, there is little desire for verbosely-commented code. The code should, itself, be readable, with comments explaining the more subtle aspects". So apart from what I already said before, useless documentation doesn't help and is additional burden to maintain. So exercise judgement in adding documentation. > > > > > +static long xe_eu_stall_stream_ioctl(struct file *file, > > > + unsigned int cmd, > > > + unsigned long arg) > > > +{ > > > + switch (cmd) { > > > + case DRM_XE_OBSERVATION_IOCTL_ENABLE: > > > + return 0; > > > + case DRM_XE_OBSERVATION_IOCTL_DISABLE: > > > + return 0; > > > + } > > > + > > > + return -EINVAL; > > > +} > > > + > > > +/** > > > + * xe_eu_stall_stream_close - handles userspace close() of a EU stal= l data > > > + * stream file. > > > + * @inode: anonymous inode associated with file > > > + * @file: An xe EU stall data stream file > > > + * > > > + * Cleans up any resources associated with an open EU stall data str= eam file. > > > + */ > > > +static int xe_eu_stall_stream_close(struct inode *inode, struct file= *file) > > > +{ > > > + return 0; > > > +} > > > + > > > > Not sure why you've introduced these empty functions in this patch, they > > should just be introduced in later patches where they have valid bodies. > > > From a review feedback in series V3: > > Initial patch that implements the basic interface > and such, but just stubs out the parts that actually go touch > hardware. > > Since I didn't have any code in this patch that touches the hardware, > some functions are empty. I really don't see why this should be needed at all. Anyway I have nothing to add to what I already said previously. > > > The idea is that the initial patches should just add lines of code, not > > remove lines of code. If you see the initial OA patches, there is not e= ven > > a single line of deleted code: > I have moved some code around into different functions in the next > patch. There isn't much code that is deleted. Will try to improve it in > the next patch series. > > > > git log --stat 67977882a2f1..392bf22238ff > > > > Don't worry about adding a few lines of code to the later patches. > > > > Anyway I don't want you to spend too much time changing patches again, = so > > I'm ok leaving things as is too. Just let me know if it's going to be a > > problem and what you prefer. I am just letting you know what the prefer= red > > practice is. > > > > > +static const struct file_operations fops_eu_stall =3D { > > > + .owner =3D THIS_MODULE, > > > + .llseek =3D noop_llseek, > > > + .release =3D xe_eu_stall_stream_close, > > > + .poll =3D xe_eu_stall_stream_poll, > > > + .read =3D xe_eu_stall_stream_read, > > > + .unlocked_ioctl =3D xe_eu_stall_stream_ioctl, > > > + .compat_ioctl =3D xe_eu_stall_stream_ioctl, > > > +}; > > > + > > > +static inline bool has_eu_stall_sampling_support(struct xe_device *x= e) > > > +{ > > > + return false; > > > +} > > > + > > > +/** > > > + * xe_eu_stall_stream_open - Open a xe EU stall data stream fd > > > + * > > > + * @dev: DRM device pointer > > > + * @data: pointer to first struct @drm_xe_ext_set_property in > > > + * the chain of input properties from the user space. > > > + * @file: DRM file pointer > > > + * > > > + * This function opens a EU stall data stream with input properties = from > > > + * the user space. > > > + * > > > + * Returns: EU stall data stream fd on success or a negative error c= ode. > > > + */ > > > +int xe_eu_stall_stream_open(struct drm_device *dev, > > > + u64 data, > > > + struct drm_file *file) > > > +{ > > > + struct xe_device *xe =3D to_xe_device(dev); > > > + struct eu_stall_open_properties props; > > > + int ret, stream_fd; > > > + > > > + memset(&props, 0, sizeof(struct eu_stall_open_properties)); > > > > struct eu_stall_open_properties props =3D {}; > Will change. > > > > > + > > > + ret =3D xe_eu_stall_user_extensions(xe, data, &props); > > > + if (ret) > > > + return ret; > > > + > > > + if (!props.gt) { > > > + drm_dbg(&xe->drm, "GT ID not provided for EU stall sampling\n"); > > > + return -EINVAL; > > > + } > > > + > > > + if (xe_observation_paranoid && !perfmon_capable()) { > > > + xe_gt_dbg(props.gt, "Insufficient privileges for EU stall monitori= ng\n"); > > > + return -EACCES; > > > + } > > > > This check should be the first check right at the top of the function. > Will change. > > > > > + > > > + if (!has_eu_stall_sampling_support(xe)) { > > > + xe_gt_dbg(props.gt, "EU stall monitoring is not supported on this = platform\n"); > > > + return -EPERM; > > > + } > > > + stream_fd =3D anon_inode_getfd("[xe_eu_stall]", &fops_eu_stall, > > > + NULL, 0); > > > > Move to previous line, lines can be upto 100 char in length.. > Will change. > > > > > + if (stream_fd < 0) > > > + xe_gt_dbg(props.gt, "EU stall inode get fd failed : %d\n", stream_= fd); > > > + > > > + return stream_fd; > > > +} > > > diff --git a/drivers/gpu/drm/xe/xe_eu_stall.h b/drivers/gpu/drm/xe/xe= _eu_stall.h > > > new file mode 100644 > > > index 000000000000..3447958a7a22 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/xe/xe_eu_stall.h > > > @@ -0,0 +1,12 @@ > > > +/* SPDX-License-Identifier: MIT */ > > > +/* > > > + * Copyright =A9 2025 Intel Corporation > > > + */ > > > + > > > +#ifndef __XE_EU_STALL_H__ > > > +#define __XE_EU_STALL_H__ > > > + > > > +int xe_eu_stall_stream_open(struct drm_device *dev, > > > + u64 data, > > > + struct drm_file *file); > > > +#endif > > > diff --git a/drivers/gpu/drm/xe/xe_observation.c b/drivers/gpu/drm/xe= /xe_observation.c > > > index 8ec1b84cbb9e..cca661de60ac 100644 > > > --- a/drivers/gpu/drm/xe/xe_observation.c > > > +++ b/drivers/gpu/drm/xe/xe_observation.c > > > @@ -9,6 +9,7 @@ > > > #include > > > > > > #include "xe_oa.h" > > > +#include "xe_eu_stall.h" > > > #include "xe_observation.h" > > > > > > u32 xe_observation_paranoid =3D true; > > > @@ -29,6 +30,17 @@ static int xe_oa_ioctl(struct drm_device *dev, str= uct drm_xe_observation_param * > > > } > > > } > > > > > > +static int xe_eu_stall_ioctl(struct drm_device *dev, struct drm_xe_o= bservation_param *arg, > > > + struct drm_file *file) > > > +{ > > > + switch (arg->observation_op) { > > > + case DRM_XE_OBSERVATION_OP_STREAM_OPEN: > > > + return xe_eu_stall_stream_open(dev, arg->param, file); > > > + default: > > > + return -EINVAL; > > > + } > > > +} > > > + > > > /** > > > * xe_observation_ioctl - The top level observation layer ioctl > > > * @dev: @drm_device > > > @@ -51,6 +63,8 @@ int xe_observation_ioctl(struct drm_device *dev, vo= id *data, struct drm_file *fi > > > switch (arg->observation_type) { > > > case DRM_XE_OBSERVATION_TYPE_OA: > > > return xe_oa_ioctl(dev, arg, file); > > > + case DRM_XE_OBSERVATION_TYPE_EU_STALL: > > > + return xe_eu_stall_ioctl(dev, arg, file); > > > default: > > > return -EINVAL; > > > } > > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h > > > index f62689ca861a..d9b20afc57c1 100644 > > > --- a/include/uapi/drm/xe_drm.h > > > +++ b/include/uapi/drm/xe_drm.h > > > @@ -1397,6 +1397,8 @@ struct drm_xe_wait_user_fence { > > > enum drm_xe_observation_type { > > > /** @DRM_XE_OBSERVATION_TYPE_OA: OA observation stream type */ > > > DRM_XE_OBSERVATION_TYPE_OA, > > > + /** @DRM_XE_OBSERVATION_TYPE_EU_STALL: EU stall sampling observatio= n stream type */ > > > + DRM_XE_OBSERVATION_TYPE_EU_STALL, > > > }; > > > > > > /** > > > @@ -1729,6 +1731,42 @@ struct drm_xe_oa_stream_info { > > > __u64 reserved[3]; > > > }; > > > > > > +/** > > > + * enum drm_xe_eu_stall_property_id - EU stall sampling input proper= ty ids. > > > + * > > > + * These properties are passed to the driver at open as a chain of > > > + * @drm_xe_ext_set_property structures with @property set to these > > > + * properties' enums and @value set to the corresponding values of t= hese > > > + * properties. @drm_xe_user_extension base.name should be set to > > > + * @DRM_XE_EU_STALL_EXTENSION_SET_PROPERTY. > > > + * > > > + * With the file descriptor obtained from open, user space must enab= le > > > + * the EU stall stream fd with @DRM_XE_OBSERVATION_IOCTL_ENABLE befo= re > > > + * calling read(). EIO errno from read() indicates HW dropped data > > > + * due to full buffer. > > > + */ > > > +enum drm_xe_eu_stall_property_id { > > > +#define DRM_XE_EU_STALL_EXTENSION_SET_PROPERTY 0 > > > + /** > > > + * @DRM_XE_EU_STALL_PROP_GT_ID: @gt_id of the GT on which > > > + * EU stall data will be captured. > > > + */ > > > + DRM_XE_EU_STALL_PROP_GT_ID =3D 1, > > > + > > > + /** > > > + * @DRM_XE_EU_STALL_PROP_SAMPLE_RATE: Sampling rate > > > + * in GPU cycles. > > > + */ > > > + DRM_XE_EU_STALL_PROP_SAMPLE_RATE, > > > + > > > + /** > > > + * @DRM_XE_EU_STALL_PROP_WAIT_NUM_REPORTS: Minimum number of > > > + * EU stall data reports to be present in the kernel buffer > > > + * before unblocking poll or read that is blocked. > > > + */ > > > + DRM_XE_EU_STALL_PROP_WAIT_NUM_REPORTS, > > > +}; > > > + > > > #if defined(__cplusplus) > > > } > > > #endif > > > -- > > > 2.47.1 > > > Thanks. -- Ashutosh