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 36DBAC3DA6E for ; Fri, 5 Jan 2024 23:30:22 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B597B10E6BE; Fri, 5 Jan 2024 23:30:21 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.9]) by gabe.freedesktop.org (Postfix) with ESMTPS id 816C110E6BE for ; Fri, 5 Jan 2024 23:30:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1704497420; x=1736033420; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=cXSVECjc92Cc+wFn1e2z/5CXqbH6MHVVV+xopP4AMtk=; b=Xij7EiiysJbX63l558n6xCByi9QxadtmX/cr4ayMw5X6ndeXN9ytXmQN pMJzRG193c7w/sWuCOFOa6klXqLxEhLHTrizIphCYAJ6lQPbK9g1Am15U T9p0OYNwLOfNzL1i6nOUiUH0t7Mo+3jVhDYUffYda3/Q+fz6ZdAc7XEtS QBjcyI10U5ihVyM2kFafsqzW+UH66si1IWV7ZCkaRJhaHKP8N1RK27nNY /2bdblmeAUOtAqWiY1sNFROkQdr5megwqwa7cMHv5du7MItJNyeT2iNVV XGawIBxnAlyuBJUNTHDB5I2ZPKU1FZeaplPObUiW/cL2q34rIHOHCJmk4 A==; X-IronPort-AV: E=McAfee;i="6600,9927,10944"; a="16221413" X-IronPort-AV: E=Sophos;i="6.04,335,1695711600"; d="scan'208";a="16221413" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orvoesa101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jan 2024 15:30:20 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10944"; a="904274575" X-IronPort-AV: E=Sophos;i="6.04,335,1695711600"; d="scan'208";a="904274575" Received: from orsosgc001.jf.intel.com (HELO unerlige-ril.intel.com) ([10.165.21.138]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jan 2024 15:30:19 -0800 Date: Fri, 05 Jan 2024 15:30:19 -0800 Message-ID: <85jzonxss4.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: "intel-xe@lists.freedesktop.org" Subject: Re: FW: [01/17] drm/xe/perf/uapi: "Perf" layer to support multiple perf counter stream types In-Reply-To: References: <202312170845.3BH8jqwL1524331@gzadicario-vm-u22.habana-labs.com> <877clavr7y.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=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: , Cc: "Chegondi, Harish" , Guy Zadicario Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Mon, 18 Dec 2023 19:04:32 -0800, Dixit, Ashutosh wrote: > > On Sun, 17 Dec 2023 00:45:52 -0800, Guy Zadicario wrote: > > > > >diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h > > >index eb03a49c17a13..3539e0781d700 100644 > > >--- a/include/uapi/drm/xe_drm.h > > >+++ b/include/uapi/drm/xe_drm.h > > > > .. > > > > >+ > > >+/** > > >+ * enum drm_xe_perf_ioctls - Perf fd ioctl's > > >+ */ > > >+enum drm_xe_perf_ioctls { > > >+ /** @DRM_XE_PERF_IOCTL_ENABLE: Enable data capture for a stream */ > > >+ DRM_XE_PERF_IOCTL_ENABLE = _IO('i', 0x0), > > >+ > > >+ /** @DRM_XE_PERF_IOCTL_DISABLE: Disable data capture for a stream */ > > >+ DRM_XE_PERF_IOCTL_DISABLE = _IO('i', 0x1), > > >+ > > >+ /** @DRM_XE_PERF_IOCTL_CONFIG: Change stream configuration */ > > >+ DRM_XE_PERF_IOCTL_CONFIG = _IO('i', 0x2), > > >+}; > > >+ > > > > Hi Guy, > > Let's discuss this a bit and then decide what to do. > > > It apears that in the hwtrace PERF stream I need to pass some arguments > > in the ENABLE/DISABLE ioctls. This is required since firmware will > > implement the enable/disable functionality. I would suggest to use > > different enums for the stream ioctls, you can rename drm_xe_perf_ioctls > > to drm_xe_oa_ioctls and I will define drm_xe_hwtrace_ioctls. > > I meant the above ioctls to be ioctl's which could be common between the > different stream types. A couple of points: > > * The above ioctl's do not prevent you from passing in any data. E.g. with: > > static long xe_hwtrace_ioctl(struct file *file, unsigned int cmd, unsigned long arg); > > If 'arg' is a pointer, any data you want can be passed into any of the > ioctls (enable/disable/config). So there are no restrictions. Each stream > type is free to do what it needs. > > * Each stream type is free to add whatever ioctl's it needs, or ignore > these common ioctls. E.g. this is possible for HWTRACE: > > #define DRM_XE_PERF_IOCTL_ENABLE _IO('i', 0x0) > #define DRM_XE_PERF_IOCTL_DISABLE _IO('i', 0x1) > #define DRM_XE_PERF_IOCTL_CONFIG _IO('i', 0x2) > #define DRM_XE_HWTRACE_IOCTL_ABC _IO('i', 0x3) > #define DRM_XE_HWTRACE_IOCTL_XYZ _IO('j', 0x4) > #define DRM_XE_HWTRACE_IOCTL_XYZ1 _IO('q', 0x9) > > * Currently we expect to have 3 stream types: OA, EuStall and HwTrace. The > above ioctl's are used by both OA and Eustall (and also it appears > HwTrace). EuStall just uses enable/disable, it doesn't have 'config'. > > > I'd also like to add another stream ioctl to retrieve the stream status > > and avaialable bytes. > > So I believe I have already answered this above. > > Maybe having each stream type (OA, EuStall, HwTrace) have its own separate > ioctl's is cleaner, and we can switch to that if people want that. But I > think the current scheme doesn't prevent the sort of operation you want. Hi Guy, Actually, thinking a bit more about this, you could just add a new DRM_XE_PERF_IOCTL_STATUS (rather than say DRM_XE_HWTRACE_IOCTL_STATUS) if neeeded. We can keep the ioctl's common at the PERF layer, but individual stream types can implement only what they need. Thanks. -- Ashutosh