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 DCDA5C2BD09 for ; Mon, 24 Jun 2024 20:04:21 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9D15810E077; Mon, 24 Jun 2024 20:04:21 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="OQJkgN93"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.10]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9854C10E077 for ; Mon, 24 Jun 2024 20:04:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1719259460; x=1750795460; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=O0Ya8onvIVO4vyITZC7wTnJaKvXLKO+65MM4QQirF+g=; b=OQJkgN93etJU9DQAdkqSfnPcqZHxaAxmISnIh3+t8Pcg/MI9keE2BCPg 8SfrIIYLcQYFbEv4ks8FZUWFHEkL7kTWLZ1LbD4t9Nq9nGJ65tC8/g1RH Pg9N4XTJowJAIGoBtY0NfXbM4tobvkuWUG4uzlqf2L0DhCOVtvVDZm8IP f25SU+0S75LnFGmI8NzTx9gO95Mmt7yzMe/7bNQvwLQPbKgeaIxFLKvag W/kRrHKnA9YwFrUBmC0iCte6eTV4Uv5yLecnF5S2C2JVbSc7t7MYrZm6z cd/wrCl5I4q3rt2NiF3mcymFfCAV2DKH8BzWgsYvkLNE7BwTG5TD7gAQ8 Q==; X-CSE-ConnectionGUID: rR7QttPDTZujIIYO4WsAeA== X-CSE-MsgGUID: CQxWHfaYQuu+X0LSWF+Xhg== X-IronPort-AV: E=McAfee;i="6700,10204,11113"; a="27657710" X-IronPort-AV: E=Sophos;i="6.08,262,1712646000"; d="scan'208";a="27657710" Received: from fmviesa002.fm.intel.com ([10.60.135.142]) by fmvoesa104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jun 2024 13:04:20 -0700 X-CSE-ConnectionGUID: Bm5BP2JxSv2gcloBop3h3g== X-CSE-MsgGUID: keqdY4NuRdqoPjC6/2KdKA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.08,262,1712646000"; d="scan'208";a="66623499" Received: from orsosgc001.jf.intel.com (HELO orsosgc001.intel.com) ([10.165.21.138]) by fmviesa002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jun 2024 13:04:20 -0700 Date: Mon, 24 Jun 2024 13:04:19 -0700 Message-ID: <851q4mqf98.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Michal Wajdeczko Cc: , Rodrigo Vivi , Jose Souza Subject: Re: [PATCH v3] drm/xe/oa: Fix kernel doc warnings in xe_drm.h In-Reply-To: <9de03fae-edce-41c6-8b1f-421c83d6c23a@intel.com> References: <20240623203119.3840283-1-ashutosh.dixit@intel.com> <9de03fae-edce-41c6-8b1f-421c83d6c23a@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-redhat-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 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. > > > --- > > 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 > > }; > > > > /**