From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759572AbcAKKzV (ORCPT ); Mon, 11 Jan 2016 05:55:21 -0500 Received: from foss.arm.com ([217.140.101.70]:54290 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758191AbcAKKzQ (ORCPT ); Mon, 11 Jan 2016 05:55:16 -0500 Date: Mon, 11 Jan 2016 10:54:54 +0000 From: Mark Rutland To: "Suzuki K. Poulose" Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, arm@kernel.org, punit.agrawal@arm.com, peterz@infradead.org Subject: Re: [PATCH v5 08/11] arm-cci: Provide hook for writing to PMU counters Message-ID: <20160111105454.GG6499@leverpostej> References: <1451908490-2615-1-git-send-email-suzuki.poulose@arm.com> <1451908490-2615-9-git-send-email-suzuki.poulose@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1451908490-2615-9-git-send-email-suzuki.poulose@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jan 04, 2016 at 11:54:47AM +0000, Suzuki K. Poulose wrote: > Add a hook for writing to CCI PMU counters. This callback > can be used for CCI models which requires some extra work > to program the PMU counter values. To accommodate group writes > and single counter writes, the call back accepts a bitmask > of the counter indices which need to be programmed with the > given value. > > Cc: Punit Agrawal > Cc: Mark Rutland > Signed-off-by: Suzuki K. Poulose > --- > drivers/bus/arm-cci.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c > index 2f1fcf0..47c9581 100644 > --- a/drivers/bus/arm-cci.c > +++ b/drivers/bus/arm-cci.c > @@ -134,6 +134,7 @@ struct cci_pmu_model { > struct event_range event_ranges[CCI_IF_MAX]; > int (*validate_hw_event)(struct cci_pmu *, unsigned long); > int (*get_event_idx)(struct cci_pmu *, struct cci_pmu_hw_events *, unsigned long); > + void (*write_counters)(struct cci_pmu *, unsigned long *, u32 val); > }; > > static struct cci_pmu_model cci_pmu_models[]; > @@ -846,7 +847,15 @@ static void pmu_write_counter(struct perf_event *event, u32 value) > dev_err(&cci_pmu->plat_device->dev, "Invalid CCI PMU counter %d\n", idx); > return; > } > - __pmu_write_counter(cci_pmu, value, idx); > + > + if (cci_pmu->model->write_counters) { > + unsigned long mask[BITS_TO_LONGS(cci_pmu->num_cntrs)]; > + > + memset(mask, 0, BITS_TO_LONGS(cci_pmu->num_cntrs) * sizeof(unsigned long)); > + set_bit(idx, mask); > + cci_pmu->model->write_counters(cci_pmu, mask, value); > + } else > + __pmu_write_counter(cci_pmu, value, idx); > } It would be much simpler to always log writes here, and only do the real wirtes in batches when we re-enable the PMU (with appropriate disable/enable calls in the IRQ handler). We'd still need special hooks for CCIs which require a special dance to program them, but all the logic to handle the writes would be in one place. This should work, regardless. Thanks, Mark. > > /* Write a value to a given set of counters */ > @@ -861,7 +870,10 @@ static void __pmu_write_counters(struct cci_pmu *cci_pmu, unsigned long *mask, u > static void __maybe_unused > pmu_write_counters(struct cci_pmu *cci_pmu, unsigned long *mask, u32 value) > { > - __pmu_write_counters(cci_pmu, mask, value); > + if (cci_pmu->model->write_counters) > + cci_pmu->model->write_counters(cci_pmu, mask, value); > + else > + __pmu_write_counters(cci_pmu, mask, value); > } > > static u64 pmu_event_update(struct perf_event *event) > -- > 1.7.9.5 >