Received-SPF: pass (google.com: domain of linux-kernel+bounces-151702-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Fri, 19 Apr 2024 16:49:13 +0100
From: Will Deacon <will@kernel.org>
To: Xu Yang <xu.yang_2@nxp.com>
Cc: frank.li@nxp.com, mark.rutland@arm.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de, kernel@pengutronix.de,
	festevam@gmail.com, john.g.garry@oracle.com, jolsa@kernel.org,
	namhyung@kernel.org, irogers@google.com, mike.leach@linaro.org,
	peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
	alexander.shishkin@linux.intel.com, adrian.hunter@intel.com,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	imx@lists.linux.dev
Subject: Re: [PATCH v10 3/8] perf: imx_perf: let the driver manage the
 counter usage rather the user
Message-ID: <20240419154913.GA3983@willie-the-truck>
References: <20240415061320.3948707-1-xu.yang_2@nxp.com>
 <20240415061320.3948707-3-xu.yang_2@nxp.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240415061320.3948707-3-xu.yang_2@nxp.com>
User-Agent: Mutt/1.10.1 (2018-07-13)

On Mon, Apr 15, 2024 at 02:13:15PM +0800, Xu Yang wrote:
> In current design, the user of perf app needs to input counter ID to count
> events. However, this is not user-friendly since the user needs to lookup
> the map table to find the counter. Instead of letting the user to input
> the counter, let this driver to manage the counters in this patch.
> 
> This will be implemented by:
>  1. allocate counter 0 for cycle event.
>  2. find unused counter from 1-10 for reference events.
>  3. allocate specific counter for counter-specific events.
> 
> In this patch, counter attr will be kept for back-compatible but all the
> value passed down by counter=<n> will be ignored. To mark counter-specific
> events, counter ID will be encoded into perf_pmu_events_attr.id.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v6:
>  - new patch
> Changes in v7:
>  - no changes
> Changes in v8:
>  - add Rb tag
> Changes in v9:
>  - keep 'counter' attr for back-compatible
> Changes in v10:
>  - add some explanation about 'counter' attr in commit message
> ---
>  drivers/perf/fsl_imx9_ddr_perf.c | 168 ++++++++++++++++++-------------
>  1 file changed, 100 insertions(+), 68 deletions(-)

[...]

> @@ -245,8 +249,8 @@ static const struct attribute_group ddr_perf_events_attr_group = {
>  	.attrs = ddr_perf_events_attrs,
>  };
>  
> -PMU_FORMAT_ATTR(event, "config:0-7");
> -PMU_FORMAT_ATTR(counter, "config:8-15");
> +PMU_FORMAT_ATTR(event, "config:0-15");
> +PMU_FORMAT_ATTR(counter, "config:16-23");

Although these mappings are advertised in sysfs, I don't think we can
change them because userspace could be relying on them. I also can't
find any examples of other PMU drivers in the kernel changing these
mappings after being merged, so please keep tthem the same.

If you need to expand the properties to be 16-bit, then you'll need to
split them into 2x8-bit fields.

Will