From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Zijlstra Subject: Re: [RFC PATCH 00/11] drm/i915: Expose OA metrics via perf PMU Date: Tue, 19 May 2015 16:53:37 +0200 Message-ID: <20150519145337.GD3644@twins.programming.kicks-ass.net> References: <1431008154-6833-1-git-send-email-robert@sixbynine.org> <20150508162452.GR27504@twins.programming.kicks-ass.net> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Robert Bragg Cc: dri-devel@lists.freedesktop.org, David Airlie , linux-api@vger.kernel.org, intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, Ingo Molnar , Paul Mackerras , Arnaldo Carvalho de Melo , Daniel Vetter List-Id: linux-api@vger.kernel.org T24gRnJpLCBNYXkgMTUsIDIwMTUgYXQgMDI6MDc6MjlBTSArMDEwMCwgUm9iZXJ0IEJyYWdnIHdy b3RlOgo+IE9uIEZyaSwgTWF5IDgsIDIwMTUgYXQgNToyNCBQTSwgUGV0ZXIgWmlqbHN0cmEgPHBl dGVyekBpbmZyYWRlYWQub3JnPiB3cm90ZToKPiA+IE9uIFRodSwgTWF5IDA3LCAyMDE1IGF0IDAz OjE1OjQzUE0gKzAxMDAsIFJvYmVydCBCcmFnZyB3cm90ZToKPiA+Cj4gPj4gSSd2ZSBjaGFuZ2Vk IHRoZSB1YXBpIGZvciBjb25maWd1cmluZyB0aGUgaTkxNV9vYSBzcGVjaWZpYyBhdHRyaWJ1dGVz Cj4gPj4gd2hlbiBjYWxsaW5nIHBlcmZfZXZlbnRfb3BlbigyKSB3aGVyZWJ5IGluc3RlYWQgb2Yg Y3JhbW1pbmcgbG90cyBvZgo+ID4+IGJpdGZpZWxkcyBpbnRvIHRoZSBwZXJmX2V2ZW50X2F0dHIg Y29uZmlnIG1lbWJlcnMsIEknbSBub3cKPiA+PiBkYWlzeS1jaGFpbmluZyBhIGRybV9pOTE1X29h X2V2ZW50X2F0dHJfdCBzdHJ1Y3R1cmUgb2ZmIG9mIGEgc2luZ2xlCj4gPj4gY29uZmlnIG1lbWJl ciB0aGF0J3MgZXh0ZW5zaWJsZSBhbmQgdmFsaWRhdGVkIGluIHRoZSBzYW1lIHdheSBhcyB0aGUK PiA+PiBwZXJmX2V2ZW50X2F0dHIgc3RydWN0LiBJJ3ZlIGZvdW5kIHRoaXMgbXVjaCBuaWNlciB0 byB3b3JrIHdpdGggd2hpbGUKPiA+PiBiZWluZyBuZWF0bHkgZXh0ZW5zaWJsZSB0b28uCj4gPgo+ ID4gVGhpcyB3b3JyaWVzIG1lIGEgYml0Li4gaXMgdGhlcmUgbW9yZSBiYWNrZ3JvdW5kIGZvciB0 aGlzPwo+IAo+IFdvdWxkIGl0IG1heWJlIGJlIGhlbHBmdWwgdG8gc2VlIHRoZSBiZWZvcmUgYW5k IGFmdGVyPyBJIGhhZCBrZXB0IHRoaXMKPiB1YXBpIGNoYW5nZSBpbiBhIHNlcGFyYXRlIHBhdGNo IGZvciBhIHdoaWxlIGxvY2FsbHkgYnV0IGluIHRoZSBlbmQKPiBkZWNpZGVkIHRvIHNxdWFzaCBp dCBiZWZvcmUgc2VuZGluZyBvdXQgbXkgdXBkYXRlZCBzZXJpZXMuCj4gCj4gQWx0aG91Z2ggSSBk aWQgZmluZCBpdCBhIGJpdCBhd2t3YXJkIHdpdGggdGhlIGJpdGZpZWxkcywgSSB3YXMgbWFpbmx5 Cj4gY29uY2VybmVkIGFib3V0IHRoZSBleHRlbnNpYmlsaXR5IG9mIHBhY2tpbmcgbG9naWNhbGx5 IHNlcGFyYXRlCj4gYXR0cmlidXRlcyBpbnRvIHRoZSBjb25maWcgbWVtYmVycyBhbmQgaGFkIGhl YXJkIHNpbWlsYXIgY29uY2VybnMgZnJvbQo+IGEgZmV3IG90aGVycyB3aG8gaGFkIGJlZW4gZXhw ZXJpbWVudGluZyB3aXRoIG15IHBhdGNoZXMgdG9vLgo+IAo+IEEgZmV3IHNpbXBsZSBhdHRyaWJ1 dGVzIEkgY2FuIHRoaW5rIG9mIGEudC5tIHRoYXQgd2UgbWlnaHQgd2FudCB0byBhZGQKPiBpbiB0 aGUgZnV0dXJlIGFyZToKPiAtIGNvbnRyb2wgb2YgdGhlIE9BQlVGRkVSIHNpemUKPiAtIGEgd2F5 IHRvIGFzayB0aGUga2VybmVsIHRvIGNvbGxlY3QgcmVwb3J0cyBhdCB0aGUgYmVnaW5uaW5nIGFu ZCBlbmQKPiBvZiBiYXRjaCBidWZmZXJzLCBpbiBhZGRpdGlvbiB0byBwZXJpb2RpYyByZXBvcnRz Cj4gLSBhbHRlcm5hdGl2ZSB3YXlzIHRvIHVuaXF1ZWx5IGlkZW50aWZ5IGEgY29udGV4dCB0byBz dXBwb3J0IHRvb2xzCj4gcHJvZmlsaW5nIGEgc2luZ2xlIGNvbnRleHQgbm90IG5lY2Vzc2FyaWx5 IG93bmVkIGJ5IHRoZSBjdXJyZW50Cj4gcHJvY2Vzcwo+IAo+IEl0IGNvdWxkIGFsc28gYmUgaW50 ZXJlc3RpbmcgdG8gZXhwb3NlIHNvbWUgY291bnRlciBjb25maWd1cmF0aW9uCj4gdGhyb3VnaCB0 aGVzZSBhdHRyaWJ1dGVzIHRvby4gRS5nLiBvbiBCcm9hZHdlbGwrIHdlIGhhdmUgMTQgJ0ZsZXhp YmxlCj4gRVUnIGNvdW50ZXJzIGluY2x1ZGVkIGluIHRoZSBPQSB1bml0IHJlcG9ydHMsIGVhY2gg d2l0aCBhIDE2Yml0Cj4gY29uZmlndXJhdGlvbi4KPiAKPiBJbiBhIG1vcmUgZXh0cmVtZSBjYXNl IGl0IG1pZ2h0IGFsc28gYmUgdXNlZnVsIHRvIGFsbG93IHVzZXJzcGFjZSB0bwo+IHNwZWNpZnkg YSBjb21wbGV0ZSBjb3VudGVyIGNvbmZpZywgd2hpY2ggKGRlcGVuZGluZyBvbiB0aGUKPiBjb25m aWd1cmF0aW9uKSBjb3VsZCBiZSBvdmVyIDEwMCAzMmJpdCB2YWx1ZXMgdG8gc2VsZWN0IHRoZSBj b3VudGVyCj4gc2lnbmFscyArIGNvbmZpZ3VyZSB0aGUgY29ycmVzcG9uZGluZyBjb21iaW5pbmcg bG9naWMuCj4gCj4gU2luY2UgdGhpcyBwbXUgaXMgaW4gYSBkZXZpY2UgZHJpdmVyIGl0IGFsc28g c2VlbWVkIHJlYXNvbmFibHkKPiBhcHByb3ByaWF0ZSB0byBkZS1jb3VwbGUgaXQgc2xpZ2h0bHkg ZnJvbSB0aGUgY29yZSBwZXJmX2V2ZW50X2F0dHIKPiBzdHJ1Y3R1cmUgYnkgYWxsb3dpbmcgZHJp dmVyIGV4dGVuc2libGUgYXR0cmlidXRlcy4KPiAKPiBJIHdvbmRlciBpZiBpdCBtaWdodCBiZSBs ZXNzIHdvcnJpc29tZSBpZiB0aGUgaTkxNV9vYV9jb3B5X2F0dHIoKSBjb2RlCj4gd2VyZSBpbnN0 ZWFkIGEgcmUtdXNhYmxlIHV0aWxpdHkgcGVyaGFwcyBtYWludGFpbmVkIGluIGV2ZW50cy9jb3Jl LmMsCj4gc28gaWYgb3RoZXIgcG11IGRyaXZlcnMgd2VyZSB0byBmb2xsb3cgc3VpdGUgdGhlcmUg d291bGQgYmUgbGVzcyByaXNrCj4gb2YgYSBtaXN0YWtlIGJlaW5nIG1hZGUgaGVyZT8KCgpTbyBJ IGhhZCBhIHBlZWsgYXQ6CgogIGh0dHBzOi8vMDEub3JnL3NpdGVzL2RlZmF1bHQvZmlsZXMvZG9j dW1lbnRhdGlvbi9vYnNlcnZhYmlsaXR5X3BlcmZvcm1hbmNlX2NvdW50ZXJzX2hhc3dlbGwucGRm CgpJbiBhbiBhdHRlbXB0IHRvIGluZm9ybSBteXNlbGYgb2YgaG93IHRoZSBoYXJkd2FyZSB3b3Jr ZWQuIEJ1dCB0aGUKZG9jdW1lbnQgaXMgcmF0aGVyIHNwYXJzZSAoYW5kIGRvZXMgbm90IGluY2x1 ZGUgYnJvYWR3ZWxsKS4KClNvIGZyb20gd2hhdCBJIGNhbiBnYXRoZXIgdGhlcmUncyB0d28gd2F5 cyB0byBvYnNlcnZlIHRoZSBjb3VudGVycywKdGhyb3VnaCBNTUlPIG9yIHRyb3VnaCB0aGUgcmlu Zy1idWZmZXIsIHdoaWNoIGluIHR1cm4gc2VlbXMgdHJpZ2dlcmVkIGJ5CmEgTUlfUkVQT1JUX1BF UkZfQ09VTlQgY29tbWFuZC4KClsgTm93IHRoZSBkb2N1bWVudCB0YWxrcyBhYm91dCBzaG9ydGNv bWluZ3Mgb2YgdGhpcyBzY2hlbWUsIHdoZXJlIHRoZQpNSV9SRVBPUlRfUEVSRl9DT1VOVCBjb21t YW5kIGNhbiBvbmx5IGJlIHBsYWNlZCBldmVyeSBvdGhlciBjb21tYW5kLCBidXQKYSBzaW5nbGUg Y29tbWFuZCBjYW4gY29udGFpbiBzbyBtdWNoIGNvbXB1dGF0aW9uIHRoYXQgdGhpcyBpcyBub3Qg ZmluZQpncmFpbmVkIGVub3VnaCAtLSBsZWFkaW5nIHRvIHRoZSBzdWdnZXN0aW9uIG9mIGEgdGlt ZXIvY3ljbGUgYmFzZWQKcmVwb3J0aW5nLCBidXQgdGhhdCBpcyBub3QgYWN0dWFsbHkgbWVudGlv bmVkIGFmYWljdCBdCgpOb3cgdGhlIE1JX1JFUE9SVF9QRVJGX0NPVU5UIGNhbiBzZWxlY3QgYSB2 ZWN0b3IgKENvdW50ZXIgU2VsZWN0KSBvZgp3aGljaCBldmVudHMgaXQgd2lsbCB3cml0ZSBvdXQu CgpUaGlzIGNvdmVycyB0aGUgcmVndWxhciAnQScgY291bnRlcnMuIElzIHRoaXMgY29ycmVjdD8K ClRoZW4gdGhlcmUgYXJlIHRoZSAnQicgY291bnRlcnMsIHdoaWNoIGFwcGVhciB0byBiZSBwcm9n cmFtbWFibGUgdGhyb3VnaAp0aGUgQ0VDIE1NSU8gcmVnaXN0ZXJzLgoKVGhlc2UgQiBldmVudHMg Y2FuIGFsc28gYmUgcGFydCBvZiB0aGUgTUlfUkVQT1JUX1BFUkZfQ09VTlQgdmVjdG9yLgoKUmln aHQ/CgoKU28gZm9yIG1lIHRoZSAnbmF0dXJhbCcgd2F5IHRvIHJlcHJlc2VudCB0aGlzIGluIHBl cmYgd291bGQgYmUgdGhyb3VnaApldmVudCBncm91cHMuIENyZWF0ZSBhIHBlcmYgZXZlbnQgZm9y IGV2ZXJ5IHNpbmdsZSBldmVudCAtLSB5ZXMgdGhpcyBpcwo1MyBldmVudHMuCgpVc2UgdGhlIE1N SU8gcmVhZHMgZm9yIHRoZSByZWd1bGFyIHJlYWQoKSBpbnRlcmZhY2UsIGFuZCB1c2UgYSBocnRp bWVyCnBsYWNpbmcgTUlfUkVQT1JUX1BFUkZfQ09VTlQgY29tbWFuZHMsIHdpdGggYSBjb3VudGVy IHNlbGVjdCBtYXNrCmNvdmVyaW5nIHRoZSBhbGwgZXZlbnRzIGluIHRoZSBjdXJyZW50IGdyb3Vw LCBmb3Igc2FtcGxpbmcuCgpUaGVuIG9idGFpbiB0aGUgdmVjdG9yIHZhbHVlcyB1c2luZyBQRVJG X1NBTVBMRV9SRUFEIGFuZApQRVJGX0ZPUk1BVF9SRUFEIC0tIGFuZCB1c2UgdGhlIHJlYWQgdHhu IHN1cHBvcnQgdG8gY29uc3VtZSB0aGUKcmluZy1idWZmZXIgdmVjdG9ycyBpbnN0ZWFkIG9mIHRo ZSBNTUlPIHJlZ2lzdGVycy4KCllvdSBjYW4gdXNlIHRoZSBwZXJmX2V2ZW50X2F0dHI6OmNvbmZp ZyB0byBzZWxlY3QgdGhlIGNvdW50ZXIgKEEwLUE0NCwKQjAtQjcpIGFuZCB1c2UgcGVyZl9ldmVu dF9hdHRyOjpjb25maWcxIChsb3cgYW5kIGhpZ2ggZHdvcmQpIGZvciB0aGUKY29ycmVzcG9uZGlu ZyBDRUMgcmVnaXN0ZXJzLgoKClRoaXMgZG9lcyBub3QgcmVxdWlyZSByYW5kb20gcGVyIGRyaXZl ciBBQkkgZXh0ZW50aW9ucyBmb3IKcGVyZl9ldmVudF9hdHRyLCBub3IgeW91ciBjdXN0b20gb3V0 cHV0IGZvcm1hdC4KCkFtIEkgbWlzc2luZyBzb21ldGhpbmcgb2J2aW91cyBoZXJlPwpfX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpJbnRlbC1nZnggbWFpbGlu ZyBsaXN0CkludGVsLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cDovL2xpc3RzLmZyZWVk ZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2ludGVsLWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964885AbbESOxu (ORCPT ); Tue, 19 May 2015 10:53:50 -0400 Received: from casper.infradead.org ([85.118.1.10]:53754 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932735AbbESOxq (ORCPT ); Tue, 19 May 2015 10:53:46 -0400 Date: Tue, 19 May 2015 16:53:37 +0200 From: Peter Zijlstra To: Robert Bragg Cc: intel-gfx@lists.freedesktop.org, Daniel Vetter , Jani Nikula , David Airlie , Paul Mackerras , Ingo Molnar , Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-api@vger.kernel.org Subject: Re: [RFC PATCH 00/11] drm/i915: Expose OA metrics via perf PMU Message-ID: <20150519145337.GD3644@twins.programming.kicks-ass.net> References: <1431008154-6833-1-git-send-email-robert@sixbynine.org> <20150508162452.GR27504@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 15, 2015 at 02:07:29AM +0100, Robert Bragg wrote: > On Fri, May 8, 2015 at 5:24 PM, Peter Zijlstra wrote: > > On Thu, May 07, 2015 at 03:15:43PM +0100, Robert Bragg wrote: > > > >> I've changed the uapi for configuring the i915_oa specific attributes > >> when calling perf_event_open(2) whereby instead of cramming lots of > >> bitfields into the perf_event_attr config members, I'm now > >> daisy-chaining a drm_i915_oa_event_attr_t structure off of a single > >> config member that's extensible and validated in the same way as the > >> perf_event_attr struct. I've found this much nicer to work with while > >> being neatly extensible too. > > > > This worries me a bit.. is there more background for this? > > Would it maybe be helpful to see the before and after? I had kept this > uapi change in a separate patch for a while locally but in the end > decided to squash it before sending out my updated series. > > Although I did find it a bit awkward with the bitfields, I was mainly > concerned about the extensibility of packing logically separate > attributes into the config members and had heard similar concerns from > a few others who had been experimenting with my patches too. > > A few simple attributes I can think of a.t.m that we might want to add > in the future are: > - control of the OABUFFER size > - a way to ask the kernel to collect reports at the beginning and end > of batch buffers, in addition to periodic reports > - alternative ways to uniquely identify a context to support tools > profiling a single context not necessarily owned by the current > process > > It could also be interesting to expose some counter configuration > through these attributes too. E.g. on Broadwell+ we have 14 'Flexible > EU' counters included in the OA unit reports, each with a 16bit > configuration. > > In a more extreme case it might also be useful to allow userspace to > specify a complete counter config, which (depending on the > configuration) could be over 100 32bit values to select the counter > signals + configure the corresponding combining logic. > > Since this pmu is in a device driver it also seemed reasonably > appropriate to de-couple it slightly from the core perf_event_attr > structure by allowing driver extensible attributes. > > I wonder if it might be less worrisome if the i915_oa_copy_attr() code > were instead a re-usable utility perhaps maintained in events/core.c, > so if other pmu drivers were to follow suite there would be less risk > of a mistake being made here? So I had a peek at: https://01.org/sites/default/files/documentation/observability_performance_counters_haswell.pdf In an attempt to inform myself of how the hardware worked. But the document is rather sparse (and does not include broadwell). So from what I can gather there's two ways to observe the counters, through MMIO or trough the ring-buffer, which in turn seems triggered by a MI_REPORT_PERF_COUNT command. [ Now the document talks about shortcomings of this scheme, where the MI_REPORT_PERF_COUNT command can only be placed every other command, but a single command can contain so much computation that this is not fine grained enough -- leading to the suggestion of a timer/cycle based reporting, but that is not actually mentioned afaict ] Now the MI_REPORT_PERF_COUNT can select a vector (Counter Select) of which events it will write out. This covers the regular 'A' counters. Is this correct? Then there are the 'B' counters, which appear to be programmable through the CEC MMIO registers. These B events can also be part of the MI_REPORT_PERF_COUNT vector. Right? So for me the 'natural' way to represent this in perf would be through event groups. Create a perf event for every single event -- yes this is 53 events. Use the MMIO reads for the regular read() interface, and use a hrtimer placing MI_REPORT_PERF_COUNT commands, with a counter select mask covering the all events in the current group, for sampling. Then obtain the vector values using PERF_SAMPLE_READ and PERF_FORMAT_READ -- and use the read txn support to consume the ring-buffer vectors instead of the MMIO registers. You can use the perf_event_attr::config to select the counter (A0-A44, B0-B7) and use perf_event_attr::config1 (low and high dword) for the corresponding CEC registers. This does not require random per driver ABI extentions for perf_event_attr, nor your custom output format. Am I missing something obvious here?