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 A7290C27C78 for ; Wed, 12 Jun 2024 02:09:13 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2486A10E1FA; Wed, 12 Jun 2024 02:09:13 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="mFbbdg9U"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.21]) by gabe.freedesktop.org (Postfix) with ESMTPS id E743B10E1DA for ; Wed, 12 Jun 2024 02:09:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1718158151; x=1749694151; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version:content-transfer-encoding; bh=VQhrzI7oGkVM26AQ8ut60Fi4wtks6mj/DZWIJvMA9wY=; b=mFbbdg9U1wf1QGLQygkiVt4k+tP9ukNWn4IqusMVv8smAchgC6wx/Cdp 2ce63KIBQ21stBjwahxO4CCSPY8xSkeuKM+YY2JR7W6bnHXO5TZ2TESpo ROF6M6ur/lBglAlMEGQuZE6sDdQ+VA/uLIdnw86hqemVZ8YqOWSbQeEeR YPgklWmtA9iBRJyWMc6BcYkb9vgxarDSC+ViW4zn9xNVpl37urfJERMRA zsdC5wgkZEl/aeIQyw4G8WojyeBw0TbVhkPc7KXqSMb8fQCdTXWJpXZD2 rYoY7CNY7HTdxBBv8c8etoTpBMpFfb4t5VduSEhTBgCiVTvrYDbTv0pdE w==; X-CSE-ConnectionGUID: nUIh/X/ES4antxm37EOf1w== X-CSE-MsgGUID: RZjlFG1QSaybKWxUMfQTnA== X-IronPort-AV: E=McAfee;i="6600,9927,11100"; a="14864451" X-IronPort-AV: E=Sophos;i="6.08,231,1712646000"; d="scan'208";a="14864451" Received: from orviesa005.jf.intel.com ([10.64.159.145]) by orvoesa113.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Jun 2024 19:09:11 -0700 X-CSE-ConnectionGUID: 1s9GneGNSDi2DLhffnH9+Q== X-CSE-MsgGUID: /hoNdemFTgO+/WXuasCJZA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.08,231,1712646000"; d="scan'208";a="44575989" Received: from jrglassb-mobl1.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.125.0.65]) by orviesa005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Jun 2024 19:09:10 -0700 Date: Tue, 11 Jun 2024 19:03:23 -0700 Message-ID: <871q5229zo.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Michal Wajdeczko Cc: Subject: Re: [PATCH 03/17] drm/xe/oa/uapi: Add OA data formats In-Reply-To: <35b5e693-edf1-4924-85f2-56f3ba146206@intel.com> References: <20240607204322.1966831-1-ashutosh.dixit@intel.com> <20240607204322.1966831-4-ashutosh.dixit@intel.com> <35b5e693-edf1-4924-85f2-56f3ba146206@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.3 (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=ISO-8859-1 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 Sat, 08 Jun 2024 03:54:16 -0700, Michal Wajdeczko wrote: > Hi Michal, > On 07.06.2024 22:43, Ashutosh Dixit wrote: > > Add and initialize supported OA data formats for various platforms > > (including Xe2). User can request OA data in any supported format. > > > > Bspec: 52198, 60942, 61101 > > > > v2: Start 'xe_oa_format_name' enum from 0 (Umesh) > > Fix error rewind with OA (Umesh) > > v3: Use graphics versions rather than absolute platform names > > > > Acked-by: Jos=E9 Roberto de Souza > > Reviewed-by: Umesh Nerlige Ramappa > > Signed-off-by: Ashutosh Dixit > > --- > > drivers/gpu/drm/xe/Makefile | 1 + > > drivers/gpu/drm/xe/xe_device.c | 12 +++- > > drivers/gpu/drm/xe/xe_device_types.h | 4 ++ > > drivers/gpu/drm/xe/xe_module.c | 1 + > > drivers/gpu/drm/xe/xe_oa.c | 101 +++++++++++++++++++++++++++ > > drivers/gpu/drm/xe/xe_oa.h | 16 +++++ > > drivers/gpu/drm/xe/xe_oa_types.h | 76 ++++++++++++++++++++ > > include/uapi/drm/xe_drm.h | 10 +++ > > 8 files changed, 220 insertions(+), 1 deletion(-) > > create mode 100644 drivers/gpu/drm/xe/xe_oa.c > > create mode 100644 drivers/gpu/drm/xe/xe_oa.h > > create mode 100644 drivers/gpu/drm/xe/xe_oa_types.h > > > > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile > > index 7a91b318efa5..c3127f10f886 100644 > > --- a/drivers/gpu/drm/xe/Makefile > > +++ b/drivers/gpu/drm/xe/Makefile > > @@ -92,6 +92,7 @@ xe-y +=3D xe_bb.o \ > > xe_mmio.o \ > > xe_mocs.o \ > > xe_module.o \ > > + xe_oa.o \ > > xe_pat.o \ > > xe_pci.o \ > > xe_pcode.o \ > > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_dev= ice.c > > index 31ab04b4bd5c..e225eb09fc17 100644 > > --- a/drivers/gpu/drm/xe/xe_device.c > > +++ b/drivers/gpu/drm/xe/xe_device.c > > @@ -41,6 +41,7 @@ > > #include "xe_memirq.h" > > #include "xe_mmio.h" > > #include "xe_module.h" > > +#include "xe_oa.h" > > #include "xe_pat.h" > > #include "xe_pcode.h" > > #include "xe_perf.h" > > @@ -655,10 +656,14 @@ int xe_device_probe(struct xe_device *xe) > > > > xe_heci_gsc_init(xe); > > > > - err =3D xe_display_init(xe); > > + err =3D xe_oa_init(xe); > > if (err) > > goto err_fini_gt; > > > > + err =3D xe_display_init(xe); > > + if (err) > > + goto err_fini_oa; > > + > > err =3D drm_dev_register(&xe->drm, 0); > > if (err) > > goto err_fini_display; > > @@ -674,6 +679,9 @@ int xe_device_probe(struct xe_device *xe) > > err_fini_display: > > xe_display_driver_remove(xe); > > > > +err_fini_oa: > > + xe_oa_fini(xe); > > + > > err_fini_gt: > > for_each_gt(gt, xe, id) { > > if (id < last_gt) > > @@ -706,6 +714,8 @@ void xe_device_remove(struct xe_device *xe) > > > > xe_display_fini(xe); > > > > + xe_oa_fini(xe); > > + > > xe_heci_gsc_fini(xe); > > > > for_each_gt(gt, xe, id) > > diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/= xe_device_types.h > > index 52bc461171d5..185986e1d586 100644 > > --- a/drivers/gpu/drm/xe/xe_device_types.h > > +++ b/drivers/gpu/drm/xe/xe_device_types.h > > @@ -17,6 +17,7 @@ > > #include "xe_gt_types.h" > > #include "xe_lmtt_types.h" > > #include "xe_memirq_types.h" > > +#include "xe_oa.h" > > #include "xe_platform_types.h" > > #include "xe_pt_types.h" > > #include "xe_sriov_types.h" > > @@ -462,6 +463,9 @@ struct xe_device { > > /** @heci_gsc: graphics security controller */ > > struct xe_heci_gsc heci_gsc; > > > > + /** @oa: oa perf counter subsystem */ > > + struct xe_oa oa; > > + > > /** @needs_flr_on_fini: requests function-reset on fini */ > > bool needs_flr_on_fini; > > > > diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_mod= ule.c > > index 893858a2eea0..4e9f31f11920 100644 > > --- a/drivers/gpu/drm/xe/xe_module.c > > +++ b/drivers/gpu/drm/xe/xe_module.c > > @@ -10,6 +10,7 @@ > > > > #include "xe_drv.h" > > #include "xe_hw_fence.h" > > +#include "xe_oa.h" > > #include "xe_pci.h" > > #include "xe_perf.h" > > #include "xe_sched_job.h" > > diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c > > new file mode 100644 > > index 000000000000..3349e645cb72 > > --- /dev/null > > +++ b/drivers/gpu/drm/xe/xe_oa.c > > @@ -0,0 +1,101 @@ > > +// SPDX-License-Identifier: MIT > > +/* > > + * Copyright =A9 2023 Intel Corporation > > + */ > > + > > +#include > > + > > +#include "xe_assert.h" > > +#include "xe_device.h" > > +#include "xe_macros.h" > > +#include "xe_oa.h" > > + > > +#define DRM_FMT(x) DRM_XE_OA_FMT_TYPE_##x > > + > > +static const struct xe_oa_format oa_formats[] =3D { > > + [XE_OA_FORMAT_C4_B8] =3D { 7, 64, DRM_FMT(OAG) }, > > + [XE_OA_FORMAT_A12] =3D { 0, 64, DRM_FMT(OAG) }, > > + [XE_OA_FORMAT_A12_B8_C8] =3D { 2, 128, DRM_FMT(OAG) }, > > + [XE_OA_FORMAT_A32u40_A4u32_B8_C8] =3D { 5, 256, DRM_FMT(OAG) }, > > + [XE_OAR_FORMAT_A32u40_A4u32_B8_C8] =3D { 5, 256, DRM_FMT(OAR) }, > > + [XE_OA_FORMAT_A24u40_A14u32_B8_C8] =3D { 5, 256, DRM_FMT(OAG) }, > > + [XE_OAC_FORMAT_A24u64_B8_C8] =3D { 1, 320, DRM_FMT(OAC), HDR_64_BIT = }, > > + [XE_OAC_FORMAT_A22u32_R2u32_B8_C8] =3D { 2, 192, DRM_FMT(OAC), HDR_64= _BIT }, > > + [XE_OAM_FORMAT_MPEC8u64_B8_C8] =3D { 1, 192, DRM_FMT(OAM_MPEC), HDR_= 64_BIT }, > > + [XE_OAM_FORMAT_MPEC8u32_B8_C8] =3D { 2, 128, DRM_FMT(OAM_MPEC), HDR_= 64_BIT }, > > + [XE_OA_FORMAT_PEC64u64] =3D { 1, 576, DRM_FMT(PEC), HDR_64_BIT, 1, = 0 }, > > + [XE_OA_FORMAT_PEC64u64_B8_C8] =3D { 1, 640, DRM_FMT(PEC), HDR_64_BIT= , 1, 1 }, > > + [XE_OA_FORMAT_PEC64u32] =3D { 1, 320, DRM_FMT(PEC), HDR_64_BIT }, > > + [XE_OA_FORMAT_PEC32u64_G1] =3D { 5, 320, DRM_FMT(PEC), HDR_64_BIT, 1= , 0 }, > > + [XE_OA_FORMAT_PEC32u32_G1] =3D { 5, 192, DRM_FMT(PEC), HDR_64_BIT }, > > + [XE_OA_FORMAT_PEC32u64_G2] =3D { 6, 320, DRM_FMT(PEC), HDR_64_BIT, 1= , 0 }, > > + [XE_OA_FORMAT_PEC32u32_G2] =3D { 6, 192, DRM_FMT(PEC), HDR_64_BIT }, > > + [XE_OA_FORMAT_PEC36u64_G1_32_G2_4] =3D { 3, 320, DRM_FMT(PEC), HDR_64= _BIT, 1, 0 }, > > + [XE_OA_FORMAT_PEC36u64_G1_4_G2_32] =3D { 4, 320, DRM_FMT(PEC), HDR_64= _BIT, 1, 0 }, > > +}; > > + > > +static void oa_format_add(struct xe_oa *oa, enum xe_oa_format_name for= mat) > > +{ > > + __set_bit(format, oa->format_mask); > > +} > > + > > +static void xe_oa_init_supported_formats(struct xe_oa *oa) > > +{ > > + if (GRAPHICS_VER(oa->xe) >=3D 20) { > > + /* Xe2+ */ > > + oa_format_add(oa, XE_OAM_FORMAT_MPEC8u64_B8_C8); > > + oa_format_add(oa, XE_OAM_FORMAT_MPEC8u32_B8_C8); > > + oa_format_add(oa, XE_OA_FORMAT_PEC64u64); > > + oa_format_add(oa, XE_OA_FORMAT_PEC64u64_B8_C8); > > + oa_format_add(oa, XE_OA_FORMAT_PEC64u32); > > + oa_format_add(oa, XE_OA_FORMAT_PEC32u64_G1); > > + oa_format_add(oa, XE_OA_FORMAT_PEC32u32_G1); > > + oa_format_add(oa, XE_OA_FORMAT_PEC32u64_G2); > > + oa_format_add(oa, XE_OA_FORMAT_PEC32u32_G2); > > + oa_format_add(oa, XE_OA_FORMAT_PEC36u64_G1_32_G2_4); > > + oa_format_add(oa, XE_OA_FORMAT_PEC36u64_G1_4_G2_32); > > + } else if (GRAPHICS_VERx100(oa->xe) >=3D 1270) { > > + /* XE_METEORLAKE */ > > + oa_format_add(oa, XE_OAR_FORMAT_A32u40_A4u32_B8_C8); > > + oa_format_add(oa, XE_OA_FORMAT_A24u40_A14u32_B8_C8); > > + oa_format_add(oa, XE_OAC_FORMAT_A24u64_B8_C8); > > + oa_format_add(oa, XE_OAC_FORMAT_A22u32_R2u32_B8_C8); > > + oa_format_add(oa, XE_OAM_FORMAT_MPEC8u64_B8_C8); > > + oa_format_add(oa, XE_OAM_FORMAT_MPEC8u32_B8_C8); > > + } else if (GRAPHICS_VERx100(oa->xe) >=3D 1255) { > > + /* XE_DG2, XE_PVC */ > > + oa_format_add(oa, XE_OAR_FORMAT_A32u40_A4u32_B8_C8); > > + oa_format_add(oa, XE_OA_FORMAT_A24u40_A14u32_B8_C8); > > + oa_format_add(oa, XE_OAC_FORMAT_A24u64_B8_C8); > > + oa_format_add(oa, XE_OAC_FORMAT_A22u32_R2u32_B8_C8); > > + } else { > > + /* Gen12+ */ > > + xe_assert(oa->xe, GRAPHICS_VER(oa->xe) >=3D 12); > > + oa_format_add(oa, XE_OA_FORMAT_A12); > > + oa_format_add(oa, XE_OA_FORMAT_A12_B8_C8); > > + oa_format_add(oa, XE_OA_FORMAT_A32u40_A4u32_B8_C8); > > + oa_format_add(oa, XE_OA_FORMAT_C4_B8); > > + } > > +} > > + > > +int xe_oa_init(struct xe_device *xe) > > +{ > > + struct xe_oa *oa =3D &xe->oa; > > + > > + /* Support OA only with GuC submission and Gen12+ */ > > + if (XE_WARN_ON(!xe_device_uc_enabled(xe)) || XE_WARN_ON(GRAPHICS_VER(= xe) < 12)) > > + return 0; > > + > > + oa->xe =3D xe; > > + oa->oa_formats =3D oa_formats; > > + > > + xe_oa_init_supported_formats(oa); > > + return 0; > > +} > > + > > +void xe_oa_fini(struct xe_device *xe) > > +{ > > + struct xe_oa *oa =3D &xe->oa; > > + > > + oa->xe =3D NULL; > > +} > > diff --git a/drivers/gpu/drm/xe/xe_oa.h b/drivers/gpu/drm/xe/xe_oa.h > > new file mode 100644 > > index 000000000000..a2f301e2be57 > > --- /dev/null > > +++ b/drivers/gpu/drm/xe/xe_oa.h > > @@ -0,0 +1,16 @@ > > +/* SPDX-License-Identifier: MIT */ > > +/* > > + * Copyright =A9 2023 Intel Corporation > > + */ > > + > > +#ifndef _XE_OA_H_ > > +#define _XE_OA_H_ > > + > > +#include "xe_oa_types.h" > > don't include full header if you already have required forward decl This one is unrelated. xe_oa.h is included in other files such as xe_device.c and those files need declarations in xe_oa_types.h. So I have left this as is. This is the same pattern as followed e.g. in xe_bb.h/xe_bb_types.h, xe_bo.h/xe_bo_types.h, xe_device.h/xe_device_types.h etc. > > > + > > +struct xe_device; > > + > > +int xe_oa_init(struct xe_device *xe); > > +void xe_oa_fini(struct xe_device *xe); > > + > > +#endif > > diff --git a/drivers/gpu/drm/xe/xe_oa_types.h b/drivers/gpu/drm/xe/xe_o= a_types.h > > new file mode 100644 > > index 000000000000..1e339090c90d > > --- /dev/null > > +++ b/drivers/gpu/drm/xe/xe_oa_types.h > > @@ -0,0 +1,76 @@ > > +/* SPDX-License-Identifier: MIT */ > > +/* > > + * Copyright =A9 2023 Intel Corporation > > + */ > > + > > +#ifndef _XE_OA_TYPES_H_ > > +#define _XE_OA_TYPES_H_ > > + > > +#include > > +#include > > + > > +enum xe_oa_report_header { > > + HDR_32_BIT =3D 0, > > + HDR_64_BIT, > > +}; > > + > > +enum xe_oa_format_name { > > + XE_OA_FORMAT_C4_B8, > > + > > + /* 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, > > + > > + /* DG2/MTL OAC */ > > + XE_OAC_FORMAT_A24u64_B8_C8, > > + XE_OAC_FORMAT_A22u32_R2u32_B8_C8, > > + > > + /* MTL OAM */ > > + XE_OAM_FORMAT_MPEC8u64_B8_C8, > > + XE_OAM_FORMAT_MPEC8u32_B8_C8, > > + > > + /* Xe2+ */ > > + XE_OA_FORMAT_PEC64u64, > > + XE_OA_FORMAT_PEC64u64_B8_C8, > > + XE_OA_FORMAT_PEC64u32, > > + XE_OA_FORMAT_PEC32u64_G1, > > + XE_OA_FORMAT_PEC32u32_G1, > > + XE_OA_FORMAT_PEC32u64_G2, > > + XE_OA_FORMAT_PEC32u32_G2, > > + XE_OA_FORMAT_PEC36u64_G1_32_G2_4, > > + XE_OA_FORMAT_PEC36u64_G1_4_G2_32, > > + > > + XE_OA_FORMAT_MAX, > > maybe add some prefix like __ to clearly indicate that this is a special > enumerator not to be used like other above Done. > > > +}; > > + > > +/** struct xe_oa_format - Format fields for supported OA formats */ > > +struct xe_oa_format { > > + u32 counter_select; > > + int size; > > + int type; > > + enum xe_oa_report_header header; > > + u16 counter_size; > > + u16 bc_report; > > missing kernel-docs for members Added. > > > +}; > > + > > +/** > > + * struct xe_oa - OA device level information > > + */ > > +struct xe_oa { > > + /** @xe: back pointer to xe device */ > > + struct xe_device *xe; > > + > > + /** @oa_formats: tracks all OA formats across platforms */ > > + const struct xe_oa_format *oa_formats; > > + > > +#define FORMAT_MASK_SIZE DIV_ROUND_UP(XE_OA_FORMAT_MAX - 1, BITS_PER_L= ONG) > > hmm, are you sure this -1 is correct ? > and maybe all you need is BITS_TO_LONGS ? Changed to BITS_TO_LONGS(__XE_OA_FORMAT_MAX). > > > + > > + /** @format_mask: tracks valid OA formats for a platform */ > > + unsigned long format_mask[FORMAT_MASK_SIZE]; > > +}; > > +#endif > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h > > index 03df64600aa0..bb87bf0c96f9 100644 > > --- a/include/uapi/drm/xe_drm.h > > +++ b/include/uapi/drm/xe_drm.h > > @@ -1433,6 +1433,16 @@ enum drm_xe_perf_ioctls { > > DRM_XE_PERF_IOCTL_INFO =3D _IO('i', 0x4), > > }; > > > > +/** enum drm_xe_oa_format_type - OA format types */ > > it doesn't look like rest of top-level kernel-doc > > and all below enumerators are undocumented Done. > > > +enum drm_xe_oa_format_type { > > + DRM_XE_OA_FMT_TYPE_OAG, > > + DRM_XE_OA_FMT_TYPE_OAR, > > + DRM_XE_OA_FMT_TYPE_OAM, > > + DRM_XE_OA_FMT_TYPE_OAC, > > + DRM_XE_OA_FMT_TYPE_OAM_MPEC, > > + DRM_XE_OA_FMT_TYPE_PEC, > > +}; > > + > > #if defined(__cplusplus) > > } > > #endif Thanks. -- Ashutosh