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 84469C0032E for ; Sat, 28 Oct 2023 03:44:13 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4ABBF10E02D; Sat, 28 Oct 2023 03:44:13 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1FC5810E02D for ; Sat, 28 Oct 2023 03:44:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1698464652; x=1730000652; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=eyQ1v8VTvkIc+KgGMABbwpDOr9B2kziIIK/QJlbDZ2c=; b=hfaPS6GcglLOCPLmXQ7IwHf8MOoCzUPRD3vzXqZBh1OfiqT048YC62DC ni6xpbySxZV/vhx2UV1AKyiTrSRgvAsZ6JQf1mN/ds2nuDR2VhBZxiJbC 71K8Jcy3Z0fudNgV4+m5IvxItJkILqbW6n7kDsmXGj3X0cRztZAmINzBv v3nTt5V1q48vEqpyKpew6oGClKcAWosp0rQ7giRpUiul4nL27FuAq6pAo h00QJfRVF1veKWv16gtJLvSHjWOflRx3GVePhksOm5AOFMVkIs6nNc0yU vRU5qWQDXoqwbtvg55BcmKBRSp6bE0hpVk4CAZe8/k1R634GJIn8qbXdt w==; X-IronPort-AV: E=McAfee;i="6600,9927,10876"; a="391745510" X-IronPort-AV: E=Sophos;i="6.03,258,1694761200"; d="scan'208";a="391745510" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Oct 2023 20:44:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10876"; a="759779526" X-IronPort-AV: E=Sophos;i="6.03,258,1694761200"; d="scan'208";a="759779526" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.34.111]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Oct 2023 20:44:11 -0700 Date: Fri, 27 Oct 2023 20:44:10 -0700 Message-ID: <87il6rcsxh.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa In-Reply-To: References: <20231011172643.826452-1-ashutosh.dixit@intel.com> <20231011172643.826452-2-ashutosh.dixit@intel.com> <877cnr3efj.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.1 (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 Subject: Re: [Intel-xe] [PATCH 1/2] drm/xe/uapi: "Perf" layer to support multiple perf counter stream types 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: robert.krzemien@intel.com, ogabbay@habana.ai, gzadicario@habana.ai, Harish Chegondi , bdotan@habana.ai, talbo@habana.ai, intel-xe@lists.freedesktop.org Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Thu, 12 Oct 2023 19:05:24 -0700, Umesh Nerlige Ramappa wrote: > Hi Umesh, > >> I would just squash the 2 patches here since this is a new interface and we > >> don't care much about what existed earlier. > > > > OK, I'll squash the two patches. Done: https://patchwork.freedesktop.org/series/125724/ > >> > +/** > >> > + * struct drm_xe_perf_param - XE perf layer param > >> > + * > >> > + * The perf layer enables multiplexing perf counter streams of multiple > >> > + * types. The actual params for a particular stream operation are supplied > >> > + * via the @param pointer (use __copy_from_user to get these params). > >> > + */ > >> > +struct drm_xe_perf_param { > >> > + /** @extensions: Pointer to the first extension struct, if any */ > >> > + __u64 extensions; > >> > + /** @perf_type: Perf stream type, of enum @drm_xe_perf_type */ > >> > + __u64 perf_type; > >> > + /** @param_size: size of data struct pointed to by @param */ > >> > + __u64 param_size; > >> > >> Since this structure is expandable using extensions, maybe we don't need to > >> worry about the param_size. We can drop it. The size would matter if the > >> only way to extend a structure was to add members to the end. > > > > Hmm, note param_size is not the size of this struct, it is the "size of > > data struct pointed to by @param". Even the struct pointed to by @param > > probably will also have an extensions member. > > > > Even then I am thinking no harm in having the param_size member since it > > helps in cases where UMD and KMD versions are different and the struct size > > has changed, so each has a different notion of what the struct size > > is. Having param_size available enables the kernel to resolve such cases if > > the need arises in the future (as is done in drm_ioctl). UMD just needs to > > fill in param_size as sizeof(struct) it is passing in via @param, so > > doesn't look too horrible. So I'm thinking of leaving it in just in case. > > > > What do you think? > > IMO, if extensions are members of all structs in our uApi, then we should > just use that for extending the structs. I think the size is added burden > on UMDs to populate. In case of drm_ioctl, the UMD does not see this, so > it's probably okay. I have dropped param_size for now in the above series to allow for the basic patch to get merged first. I might submit a separate patch for param_size later with additional justification and then we can debate that. Thanks. -- Ashutosh