From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9E9996E0CF for ; Tue, 24 Aug 2021 20:04:00 +0000 (UTC) Date: Tue, 24 Aug 2021 13:03:59 -0700 Message-ID: <87wnoazk28.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <87zgt6zl5e.wl-ashutosh.dixit@intel.com> References: <20210803200737.30843-1-umesh.nerlige.ramappa@intel.com> <20210803200737.30843-4-umesh.nerlige.ramappa@intel.com> <877dgb1oxd.wl-ashutosh.dixit@intel.com> <20210824185032.GA18188@unerlige-ril-10.165.21.208> <87zgt6zl5e.wl-ashutosh.dixit@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [igt-dev] [PATCH 4/5] tools/i915-perf: Add mmapped OA buffer support to i915-perf-recorder List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Umesh Nerlige Ramappa Cc: igt-dev@lists.freedesktop.org, Lionel G Landwerlin List-ID: On Tue, 24 Aug 2021 12:40:29 -0700, Dixit, Ashutosh wrote: > > > > I haven't looked at everything so correct me if I am wrong but I have this > > > other concern about INTEL_PERF_RECORD_TYPE_MULTIPLE_SAMPLE. What is the > > > reason for introducing this? I would have thought that whether OA data has > > > been collected using read's or mmap is a property only of the recorder and > > > should not be exposed to the reader or gpuvis. So in the mmap case the > > > recorder should basically do what the kernel does but not introduce a new > > > perf record type. But now we are seeing changes both in the reader and > > > gpuvis because we have introduced INTEL_PERF_RECORD_TYPE_MULTIPLE_SAMPLE? > > > Thanks. > > > > Data coming from the mmaped buffer is different from that coming from the > > read. When we issue a read, the kernel attaches a record header to each > > report. The data from the mmapped buffer is just raw reports without this > > header. Lionel had mentioned that we add a new record here with multiple > > reports in a record rather than add a header to each report at this stage > > My point is that if the recorder adds the same header which the kernel does > we can have all changes done in one place, in the recorder, rather than > multiple places (reader and gpuvis). I also think this should avoid making > any changes to the reader and gpuvis and so will actually be simpler. > > > (which I think might make us a little slow in reading the OA buffer). > > Well we will just be doing in userspace what the kernel is already doing so > it shouldn't be any slower than the kernel. > > > Also note that this does not replace the old mechanism of capturing reports > > using read. Instead it's another way to capture reports. > > Yes that is clear and it also seems (from that drain code) that we can be > doing both (reads and mmap) simultaneously. Since this seems already implemented and probably also working we may still go with what we have, but at least I wanted to indicate my preference and also see if @Lionel has any further input. Thanks.