Re: [PATCH v2] drm/vc4: Add a debugfs entry to disable/enable the load tracker

[Eric Anholt]

Boris Brezillon <boris.brezillon@xxxxxxxxxxx> writes:

> On Fri, 30 Nov 2018 12:30:52 -0800
> Eric Anholt <eric@xxxxxxxxxx> wrote:
>
>> Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx> writes:
>> 
>> > In order to test whether the load tracker is working as expected, we
>> > need the ability to compare the commit result with the underrun
>> > indication. With the load tracker always enabled, commits that are
>> > expected to trigger an underrun are always rejected, so userspace
>> > cannot get the actual underrun indication from the hardware.
>> >
>> > Add a debugfs entry to disable/enable the load tracker, so that a DRM
>> > commit expected to trigger an underrun can go through with the load
>> > tracker disabled. The underrun indication is then available to
>> > userspace and can be checked against the commit result with the load
>> > tracker enabled.
>> >
>> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx>  
>> 
>> Given that the load tracker is going to be conservative and say things
>> will underrun even when they might not in practice, will this actually
>> be useful for automated testing? Or is the intent to make it easier to
>> tune the load tracker by disabling it so that you can experiment freely?
>
> Yes, that's one goal, though I'm not sure IGT is supposed to contain
> such debugging tools. But the main benefit is being able to track
> regressions in the load tracking algo that makes it more (too?)
> conservative. I think people won't like this sort of regressions. The
> idea would be to settle on an acceptable load tracking algo (maybe
> after refining the proposed one), record the results (both good and too
> conservative predictions) and use that as a reference for the IGT
> test.  

Yeah, I think I'm sold on it at this point -- having a tool that isn't
an automated regression test, but an automated thing that can help a
developer see how accurate the estimate is, would be useful and is worth
a bit of kernel code to support.

Attachment: signature.asc
Description: PGP signature