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 79BC6CA0FE1 for ; Fri, 1 Sep 2023 01:25:25 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 27A1F10E70A; Fri, 1 Sep 2023 01:25:25 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0EB6210E70A for ; Fri, 1 Sep 2023 01:25:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1693531523; x=1725067523; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=WBEONEOI0wu1E1RYNj3fv15L0cQBsITkJlLfQSj3xtc=; b=bgUOoQRTnaPe7MiH8bJ0kSMVpG3iTkLfi2gdSSN+jW7bppsNbCAq2W9R lXZxkoX11fRy1ggBIhONenSCysKGUWgdDUW7VGxDqURmN5AtoognQtpNA 3z1ZYgQV0kFURHB7YGxCYi+Wcodnn7vdh4MeZ2P1bcEOlDY6N8IAmCcPn CIuIsGmezWXwf0HFq8Js/FfMd3yXDLHnHsWCVctsHzbH2i7BZ/4I+ivWu JNEtUBfc26CxTGz6uSOrs8UXE8YSiYuNd+/Sv2jcjcxxZnSyP29lm4lyr 1Daq2m0SbjCj/eWKexy15WPotL7PnsvBUOC/R4EHfbulQlWzqYVFa+Q96 w==; X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="376044414" X-IronPort-AV: E=Sophos;i="6.02,218,1688454000"; d="scan'208";a="376044414" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Aug 2023 18:25:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="805243895" X-IronPort-AV: E=Sophos;i="6.02,218,1688454000"; d="scan'208";a="805243895" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.212.199.173]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Aug 2023 18:25:10 -0700 Date: Thu, 31 Aug 2023 18:07:52 -0700 Message-ID: <87zg26zonr.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa In-Reply-To: <87pm3gs5ni.wl-ashutosh.dixit@intel.com> References: <20230808013159.38811-1-ashutosh.dixit@intel.com> <20230808013159.38811-2-ashutosh.dixit@intel.com> <87350kt87c.wl-ashutosh.dixit@intel.com> <87pm3gs5ni.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.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 01/10] drm/xe/oa: Introduce OA uapi 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: intel-xe@lists.freedesktop.org Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Mon, 21 Aug 2023 09:48:33 -0700, Dixit, Ashutosh wrote: > Hi Umesh, About just the 'enum drm_xe_oa_format' below: > On Tue, 15 Aug 2023 19:14:40 -0700, Umesh Nerlige Ramappa wrote: > > On Tue, Aug 15, 2023 at 12:29:59PM -0700, Dixit, Ashutosh wrote: > > > On Tue, 08 Aug 2023 15:59:24 -0700, Umesh Nerlige Ramappa wrote: > > >> > > >> On Mon, Aug 07, 2023 at 06:31:50PM -0700, Ashutosh Dixit wrote: > > >> > +enum drm_xe_oa_format { > > >> > + XE_OA_FORMAT_C4_B8 = 7, > > >> > + > > >> > + /* Gen8+ */ > > >> > + XE_OA_FORMAT_A12, > > >> > + XE_OA_FORMAT_A12_B8_C8, > > >> > + XE_OA_FORMAT_A32u40_A4u32_B8_C8, > > >> > + > > >> > + /* DG2 */ > > >> > + XE_OAR_FORMAT_A32u40_A4u32_B8_C8, > > >> > + XE_OA_FORMAT_A24u40_A14u32_B8_C8, > > >> > + > > >> > + /* MTL OAM */ > > >> > + XE_OAM_FORMAT_MPEC8u64_B8_C8, > > >> > + XE_OAM_FORMAT_MPEC8u32_B8_C8, > > >> > + > > >> > + XE_OA_FORMAT_MAX /* non-ABI */ > > >> > +}; > > >> > > >> If we support a query to get the supported OA formats, we can do away with > > >> this. > > > > > > OK, yeah get using user_ext's. > > > > > >> Also thinking if we can provide the format definition in such a way > > >> that UMD can just decode the counters from it with some generic code. That > > >> way, we don't have to constantly keep updating UMD code for every new > > >> platforms (may be far-fetched, but could save us a lot of > > >> time). Thoughts? > > > > > > Ok, but I am not sure how to do this. > > > > Just a thought, we can ignore that for now. > > I am not sure if exposing valid counter_select values has much use since > userland has metrics files so already knows supported counter_select > values. How about not exposing anything at all regarding this in xe_drm.h? > Userland can just send the counter_select values it already knows about and > XE can just validate these. LNL etc. need a couple of other fields, so we > can just define the bitfields XE is expecting and have userspace fill these > in. XE will validate all of these. > > I think we need a little bit of encapsulation (provided the OA format) if > we remove this, but if it causes problems in uapi updates etc. we can > probably get rid of it. > > Again, assuming we'll change the python scripts etc. for this. Maybe we > /should/ prefer a clean interface to these userspace modifications? I think instead of including the enum above as we've been doing, it is possible to describe the formats in terms of a few quantities. Looking at Bspec, these would be something like: - Format Type: OAG/OAR/OAC/OAM/PEC - Counter Select - Counter Size (LNL+) - BC Report (LNL+) So I think from these (together with the platform (MTL/LNL etc.) we can figure out what the report format (the oa_formats[] array) should be. So it is possible I think to replace 'enum drm_xe_oa_format' above with a struct or a bitfield with these quantities. I think this will be a little more resilient than the enum above, we won't have to change xe_drm.h each time a new format is introduced. We will only have to change xe_drm.h when HW format representation in the OACONTROL register changes basically. So e.g. if we had this struct/bitfield in i915_drm.h we would have been ok till LNL at which point we'd have to make some changes (introduce 'Counter Size' and 'BC Report'). So what do you think? Should we switch to this way of getting the formats through the uapi and discontinue the enum? Or do you just want to see the patch? Or any other ideas? Thanks. -- Ashutosh