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 6A6CEC2BD09 for ; Mon, 24 Jun 2024 20:25:10 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D04A710E077; Mon, 24 Jun 2024 20:25:09 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="AF6sWGCH"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.11]) by gabe.freedesktop.org (Postfix) with ESMTPS id C027E10E077 for ; Mon, 24 Jun 2024 20:25:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1719260709; x=1750796709; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=1D7HHu2lN7Nne0JXUtZk927+GMU2+Dlvx2xL81VcEs8=; b=AF6sWGCH/U/Clz9xnoXQ1CGHXWyWhTNLQyt1V8lwGMNT2xxkXhOLbsWc sKDTGBSFVa89D95FgRLAeSSWcoJAzW/Ym0Ly0bk1yAhvmIX74lbdR22ce zAXJAT/ZaxdEKyYhS8kEp9P3CH+hWLuJD2Zi0btvTUYXiz//9uNMuoBq0 oX7Pf63+PhGa2VBePzV4D2xO1/1M/rEWvmzd0locxocypiuTIPSpIVGUS U/+BiczqENlM+TR1snt1BPDw/fMhnSakSeTtdKdzYasXFnDGEm1hsci6M 6Vu4VuDHhuo2/R/FYlwwMzkq5bm4jjVo7DxZGr5uffEbIYNAVj/55vcfB g==; X-CSE-ConnectionGUID: Z4Fe5V73R5OMGoLCw7NOzw== X-CSE-MsgGUID: ns2E8qVyRqy98e3mXwng0g== X-IronPort-AV: E=McAfee;i="6700,10204,11113"; a="26876126" X-IronPort-AV: E=Sophos;i="6.08,262,1712646000"; d="scan'208";a="26876126" Received: from orviesa007.jf.intel.com ([10.64.159.147]) by fmvoesa105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jun 2024 13:25:08 -0700 X-CSE-ConnectionGUID: veT03wFgTEWLHq0+X22cLg== X-CSE-MsgGUID: WkXShhbbTF+7xxiKIaId2w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.08,262,1712646000"; d="scan'208";a="44066229" Received: from tlhardin-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.125.225.168]) by orviesa007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jun 2024 13:25:07 -0700 Date: Mon, 24 Jun 2024 13:16:56 -0700 Message-ID: <87tthixfif.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: "Souza, Jose" Cc: "Wajdeczko, Michal" , "intel-xe@lists.freedesktop.org" , "Vivi,\ Rodrigo" Subject: Re: [PATCH v3] drm/xe/oa: Fix kernel doc warnings in xe_drm.h In-Reply-To: <91631dc09610974b9ca5aa1756ee72c258c09dd4.camel@intel.com> References: <20240623203119.3840283-1-ashutosh.dixit@intel.com> <9de03fae-edce-41c6-8b1f-421c83d6c23a@intel.com> <851q4mqf98.wl-ashutosh.dixit@intel.com> <91631dc09610974b9ca5aa1756ee72c258c09dd4.camel@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=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 Mon, 24 Jun 2024 13:18:20 -0700, Souza, Jose wrote: > > On Mon, 2024-06-24 at 13:04 -0700, Dixit, Ashutosh wrote: > > On Mon, 24 Jun 2024 05:30:37 -0700, Michal Wajdeczko wrote: > > > > > > > Hi Michal, > > > > And Cc: Jose below. > > > > > > > > > > > On 23.06.2024 22:31, Ashutosh Dixit wrote: > > > > Fix kernel doc warnings in xe_drm.h. Also eliminate private/non-abi enum > > > > definitions. > > > > > > > > v2: Remove __DRM_XE_PERF_TYPE_MAX since it is unused (Michal) > > > > v3: Also remove DRM_XE_OA_PROPERTY_MAX since it can also be eliminated (Michal) > > > > > > > > Suggested-by: Michal Wajdeczko > > > > Signed-off-by: Ashutosh Dixit > > > > > > some nits below, but in general LGTM, so > > > > > > Reviewed-by: Michal Wajdeczko > > > > Thanks. > > > > > > > > and I'm assuming it's not too late for such uabi fixups, but better to > > > wait for ack from Rodrigo > > > > Copying Jose: in case Mesa were using DRM_XE_OA_PROPERTY_MAX, they can > > probably also use a value like 16 like we've done below in Xe (I will make > > this change in IGT). Mesa PR is not merged yet so should be ok I think, but > > they will need to make a tiny change. I am assuming as long as this all > > gets into 6.11 it should be ok. > > > > Jose please confirm this is ok. Thanks. > > Mesa is not using and even if were we can change it until it gets > merged(after KMD patches reaches drm-next). OK, thanks for the confirmation Jose. > > While at it, do we have any UMD using drm_xe_query_oa_units? Because > Mesa don't use it, so I think it should be removed until a UMD makes use > of it. So drm_xe_query_oa_units and related UAPIs could be removed. Actually we do use drm_xe_query_oa_units in gpuvis (actually tools/xe-perf/xe_perf_recorder.c in IGT, which is used for gpuvis OA recordings, see assign_oa_unit()). drm_xe_query_oa_units is needed because it defines the mapping between an OA unit and the hardware engines connected to that OA unit. Otherwise userspace has no way of knowing that information. In case of RCS, mostly you can assume that RCS is connected to OA unit 0. But for CCS etc. we need the query to correctly determine which OA unit that engine is connected to. Thanks. -- Ashutosh > > > > > > > > > > --- > > > > drivers/gpu/drm/xe/xe_oa.c | 3 ++- > > > > include/uapi/drm/xe_drm.h | 5 +---- > > > > 2 files changed, 3 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c > > > > index 4168b51cf7b5..9263ae9a864e 100644 > > > > --- a/drivers/gpu/drm/xe/xe_oa.c > > > > +++ b/drivers/gpu/drm/xe/xe_oa.c > > > > @@ -1684,6 +1684,7 @@ static const xe_oa_user_extension_fn xe_oa_user_extension_funcs[] = { > > > > [DRM_XE_OA_EXTENSION_SET_PROPERTY] = xe_oa_user_ext_set_property, > > > > }; > > > > > > > > +#define MAX_USER_EXTENSIONS 16 > > > > > > nit: maybe it's worth to put small comment saying this is our choice to > > > limit number of nested user extensions we want to support (or at least > > > this is how I understood this) > > > > > > nit: and this doesn't really look like OA specific limitation, so maybe > > > it's time to promote MAX_USER_EXTENSIONS to some shared location to make > > > it unified across driver > > > > xe_exec_queue.c also uses a similar mechanism. But I think better to leave > > them separate in different modules so that each module can tweak the value > > to what is best for that module. > > > > Thanks. > > -- > > Ashutosh > > > > > > > > > > > static int xe_oa_user_extensions(struct xe_oa *oa, u64 extension, int ext_number, > > > > struct xe_oa_open_param *param) > > > > { > > > > @@ -1692,7 +1693,7 @@ static int xe_oa_user_extensions(struct xe_oa *oa, u64 extension, int ext_number > > > > int err; > > > > u32 idx; > > > > > > > > - if (XE_IOCTL_DBG(oa->xe, ext_number >= DRM_XE_OA_PROPERTY_MAX)) > > > > + if (XE_IOCTL_DBG(oa->xe, ext_number >= MAX_USER_EXTENSIONS)) > > > > return -E2BIG; > > > > > > > > err = __copy_from_user(&ext, address, sizeof(ext)); > > > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h > > > > index 93e00be44b2d..b410553faa9b 100644 > > > > --- a/include/uapi/drm/xe_drm.h > > > > +++ b/include/uapi/drm/xe_drm.h > > > > @@ -1379,8 +1379,8 @@ struct drm_xe_wait_user_fence { > > > > * enum drm_xe_perf_type - Perf stream types > > > > */ > > > > enum drm_xe_perf_type { > > > > + /** @DRM_XE_PERF_TYPE_OA: OA perf stream type */ > > > > DRM_XE_PERF_TYPE_OA, > > > > - __DRM_XE_PERF_TYPE_MAX, /* non-ABI */ > > > > }; > > > > > > > > /** > > > > @@ -1611,9 +1611,6 @@ enum drm_xe_oa_property_id { > > > > * pass along with @DRM_XE_OA_PROPERTY_EXEC_QUEUE_ID or will default to 0. > > > > */ > > > > DRM_XE_OA_PROPERTY_OA_ENGINE_INSTANCE, > > > > - > > > > - /** @DRM_XE_OA_PROPERTY_MAX: non-ABI */ > > > > - DRM_XE_OA_PROPERTY_MAX > > > > }; > > > > > > > > /** >