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 5956DE936F9 for ; Thu, 5 Oct 2023 04:33:51 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0A49E10E188; Thu, 5 Oct 2023 04:33:51 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id BD45310E188 for ; Thu, 5 Oct 2023 04:33:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1696480428; x=1728016428; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version:content-transfer-encoding; bh=8prrnr8ymRzJ9TvKt9qPURnDI7MhhRwy98mnz5BxJyA=; b=gLNAndq+ReZqX7ovPigHb2tTF+ABFuqcs9tW9/g1zMK/nkbKJ19wLVR/ lHFNrAvGe3kud5u8Sf7BegmeWFHwTFyhNPqppMZqL8UxuF62Wk7o/v+32 R1zZzsSeXvzoevYNyC7SRHvkh5zZXOEk5tPQ5mFn1inAKmyQFoNMasdsL cX5AewnqY4Xru4wzmNNVgyN63BvdMqT2hSUmseOQ9spndZJVirIsUmNfo ALEjyMSXyd/sv4nYlOZakTRq++IdApDLtLv6BM/h44nwtIJzWu33/Z7s+ zMcTBQd0n25eZp48hewa25T+GjuY+jVawem303N8kTlWewT3CF+HA7gI6 w==; X-IronPort-AV: E=McAfee;i="6600,9927,10853"; a="373748433" X-IronPort-AV: E=Sophos;i="6.03,201,1694761200"; d="scan'208";a="373748433" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Oct 2023 21:33:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10853"; a="728303173" X-IronPort-AV: E=Sophos;i="6.03,201,1694761200"; d="scan'208";a="728303173" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.101.181]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Oct 2023 21:33:41 -0700 Date: Wed, 04 Oct 2023 21:33:41 -0700 Message-ID: <87pm1t1ye2.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa In-Reply-To: References: <20230919161049.2307855-1-ashutosh.dixit@intel.com> <20230919161049.2307855-13-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=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Intel-xe] [PATCH 12/21] 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: intel-xe@lists.freedesktop.org Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Tue, 03 Oct 2023 19:13:31 -0700, Umesh Nerlige Ramappa wrote: > > On Tue, Sep 19, 2023 at 09:10:40AM -0700, Ashutosh Dixit wrote: > > In XE, the plan is to support multiple types of perf counter streams (O= A is > > only one type of these streams). This requires addition of a PERF layer= to > > multiplex these different stream types through a single set of PERF > > ioctl's. > > > > Signed-off-by: Ashutosh Dixit > > --- > > drivers/gpu/drm/xe/Makefile | 1 + > > drivers/gpu/drm/xe/xe_device.c | 8 +++--- > > drivers/gpu/drm/xe/xe_oa.c | 43 +++++++++++++++++----------- > > drivers/gpu/drm/xe/xe_perf.c | 52 ++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/xe/xe_perf.h | 18 ++++++++++++ > > include/uapi/drm/xe_drm.h | 44 +++++++++++++++++++--------- > > 6 files changed, 133 insertions(+), 33 deletions(-) > > create mode 100644 drivers/gpu/drm/xe/xe_perf.c > > create mode 100644 drivers/gpu/drm/xe/xe_perf.h > > > > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile > > index a40c4827b9c85..294874681cc6c 100644 > > --- a/drivers/gpu/drm/xe/Makefile > > +++ b/drivers/gpu/drm/xe/Makefile > > @@ -88,6 +88,7 @@ xe-y +=3D xe_bb.o \ > > xe_pat.o \ > > xe_pci.o \ > > xe_pcode.o \ > > + xe_perf.o \ > > xe_pm.o \ > > xe_preempt_fence.o \ > > xe_pt.o \ > > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_dev= ice.c > > index 7a179c4515633..770b9fe6e65df 100644 > > --- a/drivers/gpu/drm/xe/xe_device.c > > +++ b/drivers/gpu/drm/xe/xe_device.c > > @@ -25,8 +25,8 @@ > > #include "xe_irq.h" > > #include "xe_mmio.h" > > #include "xe_module.h" > > -#include "xe_oa.h" > > #include "xe_pcode.h" > > +#include "xe_perf.h" > > #include "xe_pm.h" > > #include "xe_query.h" > > #include "xe_tile.h" > > @@ -115,9 +115,9 @@ static const struct drm_ioctl_desc xe_ioctls[] =3D { > > DRM_RENDER_ALLOW), > > DRM_IOCTL_DEF_DRV(XE_VM_MADVISE, xe_vm_madvise_ioctl, DRM_RENDER_ALLOW), > > > > - DRM_IOCTL_DEF_DRV(XE_OA_OPEN, xe_oa_stream_open_ioctl, DRM_RENDER_ALL= OW), > > - DRM_IOCTL_DEF_DRV(XE_OA_ADD_CONFIG, xe_oa_add_config_ioctl, DRM_RENDE= R_ALLOW), > > - DRM_IOCTL_DEF_DRV(XE_OA_REMOVE_CONFIG, xe_oa_remove_config_ioctl, DRM= _RENDER_ALLOW), > > + DRM_IOCTL_DEF_DRV(XE_PERF_OPEN, xe_perf_open_ioctl, DRM_RENDER_ALLOW), > > + DRM_IOCTL_DEF_DRV(XE_PERF_ADD_CONFIG, xe_perf_add_config_ioctl, DRM_R= ENDER_ALLOW), > > + DRM_IOCTL_DEF_DRV(XE_PERF_REMOVE_CONFIG, xe_perf_remove_config_ioctl,= DRM_RENDER_ALLOW), > > > > }; > > > > diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c > > index 506dd056805b2..63db0969a86b2 100644 > > --- a/drivers/gpu/drm/xe/xe_oa.c > > +++ b/drivers/gpu/drm/xe/xe_oa.c > > @@ -1173,13 +1173,13 @@ static long xe_oa_ioctl_locked(struct xe_oa_str= eam *stream, > > unsigned long arg) > > { > > switch (cmd) { > > - case XE_OA_IOCTL_ENABLE: > > + case XE_PERF_IOCTL_ENABLE: > > xe_oa_enable_locked(stream); > > return 0; > > - case XE_OA_IOCTL_DISABLE: > > + case XE_PERF_IOCTL_DISABLE: > > xe_oa_disable_locked(stream); > > return 0; > > - case XE_OA_IOCTL_CONFIG: > > + case XE_PERF_IOCTL_CONFIG: > > return xe_oa_config_locked(stream, arg); > > } > > > > @@ -1692,12 +1692,11 @@ static int xe_oa_read_properties_unlocked(struc= t xe_oa *oa, u64 __user *uprops, > > return 0; > > } > > > > -int xe_oa_stream_open_ioctl(struct drm_device *dev, void *data, > > - struct drm_file *file) > > +int xe_oa_stream_open_ioctl(struct drm_device *dev, void *data, struct= drm_file *file) > > { > > struct xe_oa *oa =3D &to_xe_device(dev)->oa; > > - struct drm_xe_oa_open_param *param =3D data; > > struct xe_oa_open_properties props =3D {}; > > + struct drm_xe_oa_open_param param; > > u32 known_open_flags; > > struct xe_gt *gt; > > int ret; > > @@ -1707,14 +1706,18 @@ int xe_oa_stream_open_ioctl(struct drm_device *= dev, void *data, > > return -ENODEV; > > } > > > > + ret =3D __copy_from_user(¶m, data, sizeof(param)); > > + if (XE_IOCTL_DBG(oa->xe, ret)) > > + return -EFAULT; > > + > > known_open_flags =3D XE_OA_FLAG_FD_CLOEXEC | XE_OA_FLAG_FD_NONBLOCK | X= E_OA_FLAG_DISABLED; > > - if (param->flags & ~known_open_flags) { > > + if (param.flags & ~known_open_flags) { > > drm_dbg(&oa->xe->drm, "Unknown drm_xe_oa_open_param flag\n"); > > return -EINVAL; > > } > > > > - ret =3D xe_oa_read_properties_unlocked(oa, u64_to_user_ptr(param->pro= perties_ptr), > > - param->num_properties, > > + ret =3D xe_oa_read_properties_unlocked(oa, u64_to_user_ptr(param.prop= erties_ptr), > > + param.num_properties, > > &props); > > if (ret) > > return ret; > > @@ -1722,7 +1725,7 @@ int xe_oa_stream_open_ioctl(struct drm_device *de= v, void *data, > > gt =3D props.hwe->gt; > > > > mutex_lock(>->oa.lock); > > - ret =3D xe_oa_stream_open_ioctl_locked(oa, param, &props, file); > > + ret =3D xe_oa_stream_open_ioctl_locked(oa, ¶m, &props, file); > > mutex_unlock(>->oa.lock); > > > > return ret; > > @@ -1918,7 +1921,8 @@ int xe_oa_add_config_ioctl(struct drm_device *dev= , void *data, > > struct drm_file *file) > > { > > struct xe_oa *oa =3D &to_xe_device(dev)->oa; > > - struct drm_xe_oa_config *arg =3D data; > > + struct drm_xe_oa_config param; > > + struct drm_xe_oa_config *arg =3D ¶m; > > struct xe_oa_config *oa_config, *tmp; > > struct xe_oa_reg *regs; > > int err, id; > > @@ -1933,6 +1937,10 @@ int xe_oa_add_config_ioctl(struct drm_device *de= v, void *data, > > return -EACCES; > > } > > > > + err =3D __copy_from_user(¶m, data, sizeof(param)); > > + if (XE_IOCTL_DBG(oa->xe, err)) > > + return -EFAULT; > > + > > if ((!arg->mux_regs_ptr || !arg->n_mux_regs) && > > (!arg->boolean_regs_ptr || !arg->n_boolean_regs) && > > (!arg->flex_regs_ptr || !arg->n_flex_regs)) { > > @@ -2035,7 +2043,7 @@ int xe_oa_remove_config_ioctl(struct drm_device *= dev, void *data, > > { > > struct xe_oa *oa =3D &to_xe_device(dev)->oa; > > struct xe_oa_config *oa_config; > > - u64 *arg =3D data; > > + u64 arg, *ptr =3D data; > > int ret; > > > > if (!oa->xe) { > > @@ -2048,22 +2056,25 @@ int xe_oa_remove_config_ioctl(struct drm_device= *dev, void *data, > > return -EACCES; > > } > > > > + ret =3D get_user(arg, ptr); > > + if (XE_IOCTL_DBG(oa->xe, ret)) > > + return ret; > > + > > ret =3D mutex_lock_interruptible(&oa->metrics_lock); > > if (ret) > > return ret; > > > > - oa_config =3D idr_find(&oa->metrics_idr, *arg); > > + oa_config =3D idr_find(&oa->metrics_idr, arg); > > if (!oa_config) { > > drm_dbg(&oa->xe->drm, "Failed to remove unknown OA config\n"); > > ret =3D -ENOENT; > > goto err_unlock; > > } > > > > - WARN_ON(*arg !=3D oa_config->id); > > + WARN_ON(arg !=3D oa_config->id); > > > > sysfs_remove_group(oa->metrics_kobj, &oa_config->sysfs_metric); > > - > > - idr_remove(&oa->metrics_idr, *arg); > > + idr_remove(&oa->metrics_idr, arg); > > > > mutex_unlock(&oa->metrics_lock); > > > > diff --git a/drivers/gpu/drm/xe/xe_perf.c b/drivers/gpu/drm/xe/xe_perf.c > > new file mode 100644 > > index 0000000000000..0f747af59f245 > > --- /dev/null > > +++ b/drivers/gpu/drm/xe/xe_perf.c > > @@ -0,0 +1,52 @@ > > +// SPDX-License-Identifier: MIT > > +/* > > + * Copyright =A9 2023 Intel Corporation > > + */ > > + > > +#include "xe_oa.h" > > +#include "xe_perf.h" > > + > > +int xe_perf_open_ioctl(struct drm_device *dev, void *data, struct drm_= file *file) > > +{ > > + struct drm_xe_perf_param *arg =3D data; > > + > > + if (arg->extensions) > > + return -EINVAL; > > + > > + switch (arg->perf_type) { > > + case XE_PERF_TYPE_OA: > > + return xe_oa_stream_open_ioctl(dev, (void *)arg->param, file); > > + default: > > + return -EINVAL; > > Wondering if a different unique error must be returned to indicate that a > particular perf module is not supported. Other than that, this lgtm, Somthing weird like ECONNABORTED? If ops are included here probably EOPNOSUPP would also work. Probably doesn't matter much. Maybe I will change to EOPNOSUPP for now. > Reviewed-by: Umesh Nerlige Ramappa Thanks. -- Ashutosh