Re: [RFC 1/8] drm/i915: Add a new PMU for handling non-OA counter data profiling requests

From: Chris Wilson <chris@chris-wilson.co.uk>
To: "Gupta, Sourab" <sourab.gupta@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Wu, Jabin" <jabin.wu@intel.com>,
	"Woo, Insoo" <insoo.woo@intel.com>
Subject: Re: [RFC 1/8] drm/i915: Add a new PMU for handling non-OA counter data profiling requests
Date: Wed, 5 Aug 2015 12:08:54 +0100	[thread overview]
Message-ID: <20150805110854.GM10387@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <1438768320.13551.40.camel@sourabgu-desktop>

On Wed, Aug 05, 2015 at 09:49:39AM +0000, Gupta, Sourab wrote:
> On Wed, 2015-08-05 at 15:17 +0530, sourab gupta wrote:
> > On Wed, 2015-08-05 at 09:38 +0000, Chris Wilson wrote:
> > > On Wed, Aug 05, 2015 at 11:25:37AM +0530, sourab.gupta@intel.com wrote:
> > > > From: Sourab Gupta <sourab.gupta@intel.com>
> > > > 
> > > > The current perf PMU driver is specific for collection of OA counter
> > > > statistics (which may be done in a periodic or asynchronous way). Since
> > > > this enables us (and limits us) to render ring, we have no means for
> > > > collection of data pertaining to other rings.
> > > > 
> > > > To overcome this limitation, we need to have a new PMU driver which enables
> > > > data collection for other rings also (in a non-OA specific mode).
> > > > This patch adds a new perf PMU to i915 device private, for handling
> > > > profiling requests for non-OA counter data.This data may encompass
> > > > timestamps, mmio register values, etc. for the relevant ring.
> > > > The new perf PMU will serve these purposes, without constraining itself to
> > > > type of data being dumped (which may restrict the user to specific ring
> > > > like in case of OA counters).
> > > > 
> > > > The patch introduces this PMU driver alongwith its associated callbacks.
> > > > 
> > > > Signed-off-by: Sourab Gupta <sourab.gupta@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_dma.c     |   2 +
> > > >  drivers/gpu/drm/i915/i915_drv.h     |  19 ++++
> > > >  drivers/gpu/drm/i915/i915_oa_perf.c | 215 ++++++++++++++++++++++++++++++++++++
> > > 
> > > You have to admit it is a bit odd for the object to be called
> > > i915_oa_pmu/i915_gen_pmu and the file i915_oa_perf.c
> > > -Chris
> > > 
> > 
> > Well, yes.. If the nomenclature of i915_gen_pmu is agreed upon, I can
> > have a new file named as i915_gen_pmu.c which will hold the routines for
> > this pmu, leaving oa pmu stuff behind in i915_oa_pmu.c
> Sorry, I meant to say 'oa pmu stuff behind in i915_oa_perf.c'. Does
> i915_gen_pmu.c sound fine?

Aiui, there is some common code with i915_oa_perf and a fair amount of
perf_event boilerplate. At the moment, I'm leaning towards just
i915_pmu.c for both (and if need be i915_pmu_gen.c i915_pmu_oa.c if and
only if splitting the boilerplate away helps maintainablity).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx