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 7883EC30658 for ; Tue, 25 Jun 2024 17:22:41 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 29AE110E6F2; Tue, 25 Jun 2024 17:22:41 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="mDrFhTfM"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) by gabe.freedesktop.org (Postfix) with ESMTPS id F304610E6F2 for ; Tue, 25 Jun 2024 17:22:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1719336160; x=1750872160; h=date:message-id:from:to:subject:in-reply-to:references: mime-version; bh=h4mpTOeAYZOdP5+cqYZhfRVmSEqCaQzTXct7QijdeSk=; b=mDrFhTfMWsU5j1MdSBi0dWd7SrQBYKX1ez5mImMwyNz3cHwtUm7RkAsb +EHnJqjo/idB6l6cnThudqiOVJJhUgw2q3LdTERT0zPRWsYxWrnW2Syh/ pcSiRxXyJf1Hr7G010qFTkrQzrluzSCcv6g1XmQA45dfUHuVtFwvqZEfF KB7mHn7mkIfDVlxafypSawLnyu796Urs4D5yZLUrtwKNoSAFLx+Y629/l hI2IrTp3rVVghDvxBdUTm94+C42rXZEzI4lQKUO7TColMIB7XmHnIknOE Dbqwx70EVH64q1OjykgLm0jjbLkbpnzoHUmihq5hPc6zVxQqOYDy16+HS w==; X-CSE-ConnectionGUID: 8wwuqy5XTvi3VhoEFcEi6w== X-CSE-MsgGUID: vfxHOrD2Tjyp1Zxwz9mlMA== X-IronPort-AV: E=McAfee;i="6700,10204,11114"; a="33828274" X-IronPort-AV: E=Sophos;i="6.08,264,1712646000"; d="scan'208";a="33828274" Received: from orviesa008.jf.intel.com ([10.64.159.148]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jun 2024 10:22:40 -0700 X-CSE-ConnectionGUID: J4DzjA4iT+yOWWOgyGKC0g== X-CSE-MsgGUID: 9wzHRUxnSCC/624SxJHVCw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.08,264,1712646000"; d="scan'208";a="44432389" Received: from tlhardin-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.125.225.168]) by orviesa008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jun 2024 10:22:40 -0700 Date: Tue, 25 Jun 2024 10:22:33 -0700 Message-ID: <87msn9gco6.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: "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: <87tthixfif.wl-ashutosh.dixit@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> <87tthixfif.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.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:16:56 -0700, Dixit, Ashutosh wrote: > > 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. Thanks everyone, I went ahead and merged this so that it gets into 6.11. Thanks. -- Ashutosh > > > > > 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 > > > > > }; > > > > > > > > > > /** > >