[Intel-gfx] [PATCH] drm/i915: Finish enabling rps before use by sysfs or debugfs

[Daniel Vetter]

On Tue, Sep 17, 2013 at 12:12 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Mon, Sep 16, 2013 at 02:56:43PM -0700, Tom.O'Rourke at intel.com wrote:
>> From: Tom O'Rourke <Tom.O'Rourke at intel.com>
>>
>> Enabling rps (turbo setup) was put in a work queue because it may
>> take quite awhile.  This change flushes the work queue to initialize
>> rps values before use by sysfs or debugfs.  Specifically,
>> rps.delayed_resume_work is flushed before using rps.hw_max,
>> rps.max_delay, rps.min_delay, or rps.cur_delay.
>>
>> This change fixes a problem in sysfs where show functions using
>> uninitialized values show incorrect values and store functions
>> using uninitialized values in range checks incorrectly fail to
>> store valid input values.  This change also addresses similar use
>> before initialized problems in debugfs.
>>
>> Change-Id: Ib9c4f2066b65013094cb9278fc17958a964836e7
>> Signed-off-by: Tom O'Rourke <Tom.O'Rourke at intel.com>
>
> I think we can exercise this by going through a suspend/resume cycle and
> racing concurrent reads/writes of the sysfs files in a 2nd process. With
> the igt_fork and igt_system_suspend_autoresume helpers in our kernel
> testsuite repro this should be a really quick testcase to write ...
>
> I'd go for a generic "read all sysfs files" approach to increase the
> chances of catching other similar fallout in the future.

To clarify: I'd like to see a testcase for this so that we can make
sure it keeps working. I guess we unfortunately can't test for
functional correctness by just resuming since we won't see
uninitialized data. But at least we can test for the
flush_delayed_work deadlock implications, which have bitten us badly
in the past on numerous occasions. And maybe we can provoke hangs if
we adjust the rps values while the setup hasn't completed yet, so
might also give us weak coverage there.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch