[igt-dev] [PATCH i-g-t v2] intel-gpu-top: Rewrite the tool to be safe to use

[Tvrtko Ursulin]

On 03/04/2018 15:06, Eero Tamminen wrote:
> Hi,
> 
> On 03.04.2018 12:36, Tvrtko Ursulin wrote:
>> On 29/03/2018 15:30, Eero Tamminen wrote:
>>> I tested this on HSW GT2, BYT, BDW GT3, SKL GT2 and KBL GT3e,
>>> with Ubuntu 16.04 and 17.10, using Ubuntu default kernels (4.4 to 4.13)
>>> and latest drm-tip build (4.16.0-rc7).
>>>
>>>
>>> General comments
>>> ----------------
>>>
>>> This will be used by our customers and people who aren't necessarily
>>> familiar with i915 internal details.  Therefore it should use
>>> common terminology in the field and in similar tools, instead of
>>> I3As (Intel 3-letter Acronyms).
>>>
>>> For example:
>>>   - rcs -> 3D render
>>>   - bcs -> blitter
>>>   - vecs -> video
>>>   - vcs -> video decode
>>> etc.
>>
>> Done. And I am open to bike-shedding of the names and display format 
>> for instance reporting.
> 
> New names look fine to me!
> 
> 
>>> Old tool showed also GPU system memory interface (GAM) busyness.
>>> That was valuable info, and reasonably accurate for stable loads.
>>>
>>> Could this tool show also either that information (preferred), or
>>> bandwidth utilized by GPU/CPU/display?
>>>
>>> (Latest kernels offer GPU memory bandwidth usage through perf
>>> "uncore_imc" "data_reads" & "date_writes" counters.)
>>
>> Excellent suggestion and I've added IMC data_reads and data_writes to 
>> the tool.
> 
> Thanks, it looks fine too.  I'm just wondering about the numbers
> it's reporting on SKL GT2...
> 
> AFAIK IMC counters are for uncore, so I though that they should
> correspond to GTI (memory interface to outside of GPU) read and
> write HW counter values.  While it seemed in some cases quite close,
> in some cases the it showed a lot smaller (2/3) value than expected.
> 
> I can understand why reads are sometimes larger, because I think
> uncore will include also display engine display content reads.
> 
> However, I don't see how uncore writes could be considerably smaller
> than the GTI interface write amount.
> 
> (GTI interface reports the expected value which corresponds directly
> to what my test application is doing (64x blended FullHD layer writes).)
> 
> Idle machine read amounts are also much smaller (60-65MB/s) than what
> I think display update read should be (1920*1080*4*60Hz = 475MiB/s).
> 
> Any ideas for these two discrepancies?

I'm afraid I am not familiar with the uncore IMC, but we could always 
approach its authors?

>>> Is "wait" value supposed to be IO-wait for given engine interface?
>>>
>>> I never saw that change from 0%, although IO-wait in top jumped
>>> from 0 to 20-30% with my test GPU load.
>>
>> No, that is time spent in MI_WAIT_FOR_EVENT.
> 
> Could you add that info to the UI?
> 
> E.g. just have "MI" on top of the "wait" column.

Like a full header strip? Yeah makes sense, I'll add it.

>  > I think not very used in current codebase.
> 
> What you're using to validate that it reports correct value?

That would be igt/tests/perf_pmu/event-wait-rcs0.

> 
>>> HW specific test results
>>> ------------------------
>>>
>>> BYT:
>>> * Reports "Failed to initialize PMU!" although old intel_gpu_top
>>>    works fine.
>>>
>>> HSW GT2,  BDW GT3, SKL GT2 and KBL GT3e seems to work fine except
>>> for the "wait" value.
>>>
>>> I never saw blitter engine to do anything, but that's because
>>> modesetting uses just 3D pipeline, and because I couldn't get
>>> Intel DDX to work with rest of latest git version of X / 3D stack.
>>
>> Thank you for testing this so thoroughly - this was really invaluable 
>> since I don't have access too such number of platforms. I've tried to 
>> fix all this in the latest version.
> 
> Machines are currently running tests, I'll check these tomorrow.

Thanks!

> 
>>> Kernel version support
>>> ----------------------
>>>
>>> My HW specific testing above was with drm-tip kernel, but I did one test
>>> also with Ubuntu 16.04 v4.4 kernel (which includes v4.6 or v4.8 i915 
>>> backport) on KBL.  For that, the tool reported:
>>> "Failed to detect engines!"
>>>
>>> Although the previous intel_gpu_top works fine with that kernel version.
>>>
>>> Same happens also with Ubuntu 17.04 v4.13 kernel.
>>>
>>>
>>> -> If new version needs a certain kernel version, it should tell
>>>     which version is required.
>>
>> Yep, at least 4.16 is needed so I have added this info to the error 
>> message.
> 
> IMHO the message is a bit ambivalent:
>      Failed to detect engines! Kernel 4.16 or newer?
> 
> I would suggest checking whether kernel is new enough, and if not:
>      Kernel X.YY detected, 4.16 or newer required.

Maybe yeah. I was planning to improve error messages altogether but 
forgot. Will see what improvements make sense.

Regards,

Tvrtko