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 5E02AEB64DA for ; Wed, 5 Jul 2023 22:46:30 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id EAF5710E3C7; Wed, 5 Jul 2023 22:46:29 +0000 (UTC) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3F4B210E3C7 for ; Wed, 5 Jul 2023 22:46:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1688597188; x=1720133188; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version:content-transfer-encoding; bh=Rbi1MSPXglIkDVIrxymf3SJbWR0vWBV0vOn1tb2sfTc=; b=IFNtKfc8EpkVumSKQ7R+n2DzWv8cMPlb1/iLL+nHHKUmP+ykPntIJu5J ZAuk+Nzg/4i0PcSu8imdG6zRFNHzQPbtuAAglK9q8KHOu0neLQwrv2hf2 9cKta25BE/0KsjN/EF3uBnT9/Ixufvc7IG/aGtFu5+s5IJJ4FDZy5k15g BvpDK1HthgdfJJy8EfVQAc1PA0/yTgi+xCaHRnRGU0XkkhgwRA7BLrD8g sDgrAgitQEBhq+/057+AHbHoA/XTg7C2vGlA3qeh/XA1Rg6xz4WQqq34Y 0LvmnC7GACNgNkQ3vdx/Yp11cjHCgzx6hzmxbDxNdj7VsxK8l6Yg7I2CI Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10762"; a="427140724" X-IronPort-AV: E=Sophos;i="6.01,184,1684825200"; d="scan'208";a="427140724" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jul 2023 15:46:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10762"; a="1049859966" X-IronPort-AV: E=Sophos;i="6.01,184,1684825200"; d="scan'208";a="1049859966" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.1.248]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jul 2023 15:46:25 -0700 Date: Wed, 05 Jul 2023 15:33:20 -0700 Message-ID: <87v8eyc7kf.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Matt Roper In-Reply-To: <20230705211923.GP6953@mdroper-desk1.amr.corp.intel.com> References: <20230630100059.122881-1-thomas.hellstrom@linux.intel.com> <87bkgw5xix.wl-ashutosh.dixit@intel.com> <20230705211923.GP6953@mdroper-desk1.amr.corp.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-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 Subject: Re: [Intel-xe] [PATCH DONTMERGE] drm/xe: uapi review submission 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, Lionel Landwerlin Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Wed, 05 Jul 2023 14:19:23 -0700, Matt Roper wrote: > > On Fri, Jun 30, 2023 at 04:40:54PM -0700, Dixit, Ashutosh wrote: > > On Fri, 30 Jun 2023 03:00:59 -0700, Thomas Hellstr=F6m wrote: > > > > > > > I have a question about the toplogy query below. I am not hugely famili= ar > > with why/how this particular struct was chosen nor the history here, but > > anyway. > > > > > +/** > > > + * struct drm_xe_query_topology_mask - describe the topology mask of= a GT > > > + * > > > + * This is the hardware topology which reflects the internal physical > > > + * structure of the GPU. > > > + * > > > + * If a query is made with a struct drm_xe_device_query where .query > > > + * is equal to DRM_XE_DEVICE_QUERY_GT_TOPOLOGY, then the reply uses > > > + * struct drm_xe_query_topology_mask in .data. > > > + */ > > > +struct drm_xe_query_topology_mask { > > > + /** @gt_id: GT ID the mask is associated with */ > > > + __u16 gt_id; > > > + > > > + /* > > > + * To query the mask of Dual Sub Slices (DSS) available for geometry > > > + * operations. For example a query response containing the following > > > + * in mask: > > > + * DSS_GEOMETRY ff ff ff ff 00 00 00 00 > > > + * means 32 DSS are available for geometry. > > > + */ > > > +#define XE_TOPO_DSS_GEOMETRY (1 << 0) > > > + /* > > > + * To query the mask of Dual Sub Slices (DSS) available for compute > > > + * operations. For example a query response containing the following > > > + * in mask: > > > + * DSS_COMPUTE ff ff ff ff 00 00 00 00 > > > + * means 32 DSS are available for compute. > > > + */ > > > +#define XE_TOPO_DSS_COMPUTE (1 << 1) > > > + /* > > > + * To query the mask of Execution Units (EU) available per Dual Sub > > > + * Slices (DSS). For example a query response containing the follow= ing > > > + * in mask: > > > + * EU_PER_DSS ff ff 00 00 00 00 00 00 > > > + * means each DSS has 16 EU. > > > + */ > > > +#define XE_TOPO_EU_PER_DSS (1 << 2) > > > + /** @type: type of mask */ > > > + __u16 type; > > > + > > > + /** @num_bytes: number of bytes in requested mask */ > > > + __u32 num_bytes; > > > + > > > + /** @mask: little-endian mask of @num_bytes */ > > > + __u8 mask[]; > > > +}; > > > > So typically to consume the above struct, userspace needs additional > > information, specifically 'max_subslices' and 'max_eus_per_subslice' wh= ich > > was included in i915 'struct drm_i915_query_topology_info'. > > > > For example to consume 'struct drm_xe_query_topology_mask' I had recent= ly > > to write the following code in IGT because this information was not > > available through 'struct drm_xe_query_topology_mask': > > > > /* Fixed fields, see fill_topology_info() and intel_sseu_set_info() in = i915 */ > > i915_topinfo.max_slices =3D 1; /* always 1 */ > > if (IS_PONTEVECCHIO(xe_dev_id(drm_fd))) { > > i915_topinfo.max_subslices =3D 64; > > i915_topinfo.max_eus_per_subslice =3D 8; > > } else if (intel_graphics_ver(xe_dev_id(drm_fd)) >=3D IP_VER(12, 50)) { > > i915_topinfo.max_subslices =3D 32; > > i915_topinfo.max_eus_per_subslice =3D 16; > > } else if (intel_graphics_ver(xe_dev_id(drm_fd)) >=3D IP_VER(12, 0)) { > > i915_topinfo.max_subslices =3D 6; > > i915_topinfo.max_eus_per_subslice =3D 16; > > } else { > > igt_assert(0); > > } > > > > So, if we are going to expose 'struct drm_xe_query_topology_mask' as it= is > > above through xe uapi, are we assuming that userspace has out of band > > knowledge of 'max_subslices' and 'max_eus_per_subslice'? Or should this > > information be (somehow) added to the above struct? > > > > Another option (which sort of works but only approximately) would be to= set > > 'num_bytes' field in 'struct drm_xe_query_topology_mask' to actual numb= er > > of bytes in the mask. At present it is set unconditionally to 8 in > > query_gt_topology(), irrespective of the actual num bytes in the masks > > which is basically (again in i915 parlance): > > I'm not sure what you're saying here. num_bytes is set to the actual > size of the mask data returned; it's not hardcoded as a constant value, > although in practice it won't change very often since it's not tied to > the specific platform you're running on. > > The size of the mask may be larger than the true set of possible DSS/EUs > on a given platform, but that's expected. For example, if some platform > can only ever have a maximum of 4 DSS, but we tell userspace they're > getting 8 bytes (64 bits), that's fine since the upper bits will always > be 0 and can just be ignored. > > Remember --- this API is not intended to be give information about > platform theoretical maximums (e.g., "Foobar Lake will never have more > than 23 DSS") since if userspace drivers actually need that information > it must come from the GuC's hwconfig, not from the driver code. This > interface is exclusively about conveying the fusing information of which > units are physically present on your specific chip. Ah, thanks Matt, what I missed was the GuC hwconfig part. That should solve my problem. Thanks. -- Ashutosh > > i915_topinfo.subslice_stride =3D DIV_ROUND_UP(i915_topinfo.max_subslice= s, 8); > > i915_topinfo.eu_stride =3D DIV_ROUND_UP(i915_topinfo.max_eus_per_subsli= ce, 8); > > > > If we go the 'num_bytes' route, that would be just an implementation ch= ange > > not a uapi change. > > > > Thanks. > > -- > > Ashutosh > > -- > Matt Roper > Graphics Software Engineer > Linux GPU Platform Enablement > Intel Corporation