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 B94FFCA0FE0 for ; Thu, 31 Aug 2023 23:30:18 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4201910E703; Thu, 31 Aug 2023 23:30:18 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.151]) by gabe.freedesktop.org (Postfix) with ESMTPS id BED4F10E703 for ; Thu, 31 Aug 2023 23:30:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1693524615; x=1725060615; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version:content-transfer-encoding; bh=NkWlYMRMkjlLaT08dzUMGBtNu+EWkib86lDjzt1WqVg=; b=iftYI422WO4xQw8sNhYZBnpvGoNyjIw0abZnjZdOje5Zolxd4XtqLwDo zU1tauFICSbHLYo/++uvVUKLdSMDcDqTUgiV7rFGfiXQXenuSfGKTyxo+ v/dgYVrkPyaZuMUaQcbATQVq4Rf39AGtL9m9Pa9GUdrYxZGtJv15Q5bCa QosbeVVgN5oGfqoVX8YO69Why1SXB6RJ6bQayzyPM2yzDIZpzZ5lP9dBv 9U1a2wzppQoy/s5vAu99HMzUX8tn6+l7JjhylQu2mSdlAlAa1IsTaUKhG ugvp+PTDRC/Y2Ap2hnGJ6F3cEWZ83p4KEHaNVwu0tkBxbq8ENpez4PUN/ Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="356405080" X-IronPort-AV: E=Sophos;i="6.02,218,1688454000"; d="scan'208";a="356405080" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Aug 2023 16:30:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="774780802" X-IronPort-AV: E=Sophos;i="6.02,218,1688454000"; d="scan'208";a="774780802" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.212.199.173]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Aug 2023 16:30:14 -0700 Date: Thu, 31 Aug 2023 16:16:18 -0700 Message-ID: <8734zy2471.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: "Belgaumkar, Vinay" In-Reply-To: <3c4ab23e-3b63-2f6f-a397-217f1818ead2@intel.com> References: <20230830051544.369643-1-aravind.iddamsetty@linux.intel.com> <20230830051544.369643-4-aravind.iddamsetty@linux.intel.com> <87a5u724xe.wl-ashutosh.dixit@intel.com> <3b85e034-b1a4-29d1-2e4f-08ea6562b6ba@linux.intel.com> <877cpb173l.wl-ashutosh.dixit@intel.com> <496fa477-0361-5e65-e7a8-d3c0d02e114a@linux.intel.com> <77134894-47b2-98ce-d3e5-f30e611e03df@intel.com> <3c4ab23e-3b63-2f6f-a397-217f1818ead2@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 v5 3/3] drm/xe/pmu: Enable PMU interface 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: Bommu Krishnaiah , intel-xe@lists.freedesktop.org, Tvrtko Ursulin Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Thu, 31 Aug 2023 16:22:10 -0700, Belgaumkar, Vinay wrote: > > >>>>> +static void xe_pmu_event_read(struct perf_event *event) > >>>>> +{ > >>>>> +=A0=A0=A0 struct xe_device *xe =3D > >>>>> +=A0=A0=A0=A0=A0=A0=A0 container_of(event->pmu, typeof(*xe), pmu.ba= se); > >>>>> +=A0=A0=A0 struct hw_perf_event *hwc =3D &event->hw; > >>>>> +=A0=A0=A0 struct xe_pmu *pmu =3D &xe->pmu; > >>>>> +=A0=A0=A0 u64 prev, new; > >>>>> + > >>>>> +=A0=A0=A0 if (pmu->closed) { > >>>>> +=A0=A0=A0=A0=A0=A0=A0 event->hw.state =3D PERF_HES_STOPPED; > >>>>> +=A0=A0=A0=A0=A0=A0=A0 return; > >>>>> +=A0=A0=A0 } > >>>>> +again: > >>>>> +=A0=A0=A0 prev =3D local64_read(&hwc->prev_count); > >>>>> +=A0=A0=A0 new =3D __xe_pmu_event_read(event); > >>>>> + > >>>>> +=A0=A0=A0 if (local64_cmpxchg(&hwc->prev_count, prev, new) !=3D pr= ev) > >>>>> +=A0=A0=A0=A0=A0=A0=A0 goto again; > >>>>> + > >>>>> +=A0=A0=A0 local64_add(new - prev, &event->count); > >>>>> +} > >>>>> + > >>>>> +static void xe_pmu_enable(struct perf_event *event) > >>>>> +{ > >> The i915_pmu code checks which event is requested here and accordingly= sets pmu->enable (which doesn't seem to be defined here yet). Any reason w= e are not doing this yet? > > > > in i915 pmu->enable is only used by events for which there is an intern= al timer sampler > > which periodically samples those events, this series is not adding such= events. > > Ok, the tracked events are bound by I915_NUM_PMU_SAMPLERS in i915, whereas > you have already defined the max non/tracking ones as > __XE_NUM_PMU_SAMPLERS, hence my confusion. Should we use a different name > in lieu of the tracked events which will be defined in subsequent patches? > > enum { > > =A0 =A0 =A0 =A0 __I915_SAMPLE_FREQ_ACT =3D 0, > > =A0=A0=A0=A0=A0=A0=A0 __I915_SAMPLE_FREQ_REQ, > =A0=A0=A0=A0=A0=A0=A0 __I915_SAMPLE_RC6, > =A0=A0=A0=A0=A0=A0=A0 __I915_SAMPLE_RC6_LAST_REPORTED, > =A0=A0=A0=A0=A0=A0=A0 __I915_NUM_PMU_SAMPLERS > > }; +enum { + __XE_SAMPLE_RENDER_GROUP_BUSY, + __XE_SAMPLE_COPY_GROUP_BUSY, + __XE_SAMPLE_MEDIA_GROUP_BUSY, + __XE_SAMPLE_ANY_ENGINE_GROUP_BUSY, + __XE_NUM_PMU_SAMPLERS +}; + I'd think any future events will be added after __XE_SAMPLE_ANY_ENGINE_GROUP_BUSY, changing the value of __XE_NUM_PMU_SAMPLERS, no? Why do we need a different name?