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 8B3DCC3DA4A for ; Thu, 22 Aug 2024 22:55:27 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 552C610E4ED; Thu, 22 Aug 2024 22:55:27 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="EvEoDC/S"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.16]) by gabe.freedesktop.org (Postfix) with ESMTPS id E65E410E4ED for ; Thu, 22 Aug 2024 22:55:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1724367327; x=1755903327; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=XgZBYeUYpLdFaqGmuAqj1i9l2IwWFPm6Shb0UW4w1mo=; b=EvEoDC/SMHkP5EFNmAGHRAM7C7Nh4Zw920iQGRiAYR+0ayx7E1kDTLri An3ACxYqOB072FlvzNjs5mjD8itga4WFFquTLqZ0qIdBokr9xx4tcMHcl rx/9KNseA0iRpq8EKWPSdh1Dg2bBx6Rt4uxiqizaWw6mu/Plf9r0PfgEe yQjpKhHrwsiCo7o5brN41sIFaJDql4ro2rGoCCy0/blrBSbvtOTcJHdqZ +No6O4FQH8wcAHPAEfLw0zwroxpz/7nrrCgXA1oMVh1Hz8O6SS6vtBxU5 d+KDkQaKBTbfAa7sWHcp1nOgBqpz2mtU9L2QPNvemWprNk7pBhCGI9Nrc A==; X-CSE-ConnectionGUID: mrxvTOjBSXO3mad54LhZFA== X-CSE-MsgGUID: RguvJLUGTtmIRS5pbnW8gQ== X-IronPort-AV: E=McAfee;i="6700,10204,11172"; a="13160056" X-IronPort-AV: E=Sophos;i="6.10,168,1719903600"; d="scan'208";a="13160056" Received: from orviesa007.jf.intel.com ([10.64.159.147]) by fmvoesa110.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Aug 2024 15:55:26 -0700 X-CSE-ConnectionGUID: kqdPPt6FQgemrVQ4ABJw3A== X-CSE-MsgGUID: r3ukK37WT9ScU1vJ9X1QMg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.10,168,1719903600"; d="scan'208";a="62137538" Received: from kumarrag-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.124.128.198]) by orviesa007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Aug 2024 15:55:26 -0700 Date: Thu, 22 Aug 2024 15:53:39 -0700 Message-ID: <87frqwyxsc.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: "Cabral, Matias A" Cc: "intel-xe@lists.freedesktop.org" , "Chegondi, Harish" , "Nerlige Ramappa, Umesh" , "Degrood, Felix J" , "Souza, Jose" , "Ranjan,\ Joshua Santhosh" , "Kumar, Shubham" , James Ausmus Subject: Re: [PATCH v2 1/1] drm/xe/eustall: Add support for EU stall sampling In-Reply-To: References: <20240707224141.2865472-1-ashutosh.dixit@intel.com> <20240707224141.2865472-2-ashutosh.dixit@intel.com> <878qww3xh8.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/29.4 (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 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, 21 Aug 2024 12:35:51 -0700, Cabral, Matias A wrote: Hi Matias, Thanks for responding, the input is _very_ helpful. Mesa folks: would it be possible for you to provide similar input too? Thanks. -- Ashutosh > > Hi Ashutosh, > > Some inline questions below [MAC] > > Thanks, > _MAC > > -----Original Message----- > From: Dixit, Ashutosh > Sent: Friday, August 16, 2024 3:38 PM > To: intel-xe@lists.freedesktop.org > Cc: Chegondi, Harish ; Nerlige Ramappa, Umesh ; Degrood, Felix J ; Souza, Jose ; Cabral, Matias A > Subject: Re: [PATCH v2 1/1] drm/xe/eustall: Add support for EU stall sampling > > On Sun, 07 Jul 2024 15:41:41 -0700, Ashutosh Dixit wrote: > > Hi Harish, > > Some comments below on just the uapi first, towards finalizing the uapi > with the UMD's who consume this data. And also comparing the uapi with > what we did in OA. > > > > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h > > index 19619d4952a8..343de700d10d 100644 > > /snip/ > > > +/** > > + * struct drm_xe_eu_stall_data_header - EU stall data header. > > + * Header with additional information that the driver adds > > + * before EU stall data of each subslice during read(). > > One question to resolve is if we really need this header and if UMD's are > actually using the information in this header. In OA we dropped the > header and are providing information in the header via different means > (see below). > > Another option is to actually add a property for the header. So headers > are added only when user space requests headers. > > > + */ > > +struct drm_xe_eu_stall_data_header { > > + /** @subslice: subslice number from which the following data > > + * has been captured. > > + */ > > + __u16 subslice; > > Do UMD's use this subslice information? We should check with L0 and Mesa about this. > > [MAC] L0 does not currently use this. > > Also about whether UMD's need or want the header itself. For OA, UMD's > were happy not having to parse the header. > > > + /** @flags: flags */ > > + __u16 flags; > > +/* EU stall data dropped by the HW due to memory buffer being full */ > > +#define XE_EU_STALL_FLAG_OVERFLOW_DROP (1 << 0) > > In OA such information is returned via > DRM_XE_OBSERVATION_IOCTL_STATUS. For EU stall, e.g. we could return a bit > mask of subslices which reporting drops. So similar to OA, we could > return -EIO when HW reports drops and userspace optionally issues > DRM_XE_OBSERVATION_IOCTL_STATUS to retrieve which subslices are reporting > drops. > > [MAC] having a return code to notify of reports drops would be much > preferable. This would allow the UMD detecting this condition during the > read phase without needing to process/parse each report. > > > + /** @record_size: size of each EU stall data record */ > > + __u16 record_size; > > This is static information. Does it need to be in each packet header? > E.g. it can be returned via DRM_XE_OBSERVATION_IOCTL_INFO after a EU > Stall stream has been opened. > > [MAC] since the size is constant, it seems an overhead including the info > in every report. > > The INFO data struct could also include a capabilities field. So if new > features are added to EU stall in the future, they would be advertized to > user space using the capabilities field. > > > + /** @num_records: number of records following the header */ > > + __u16 num_records; > > This will not be needed if just return raw EU Stall data without > headers. Or even otherwise it is probably not needed, it is the total > size of returned data minus the size of the header. Provided we return > all available data. > > [MAC] the KMD will always return atomic units of reports, right? Then > this is not needed, having UMD the possibility to query report size when > opening the stream, the UMD can know how many reports are in each read. > > > + /** @reserved: Reserved */ > > + __u16 reserved[4]; > > This can be handled via 'extensions'. And if headers change they can be > advertized in capabilities. > > > +}; > > + > > #if defined(__cplusplus) > > } > > #endif > > -- > > 2.41.0 > > > > Thanks. > -- > Ashutosh