Re: [PATCH 8/8] drm/i915: Add drrs_interval module parameter

["Kannan, Vandana" <vandana.kannan@intel.com>]

On 15-Dec-14 7:46 PM, Daniel Vetter wrote:
> On Mon, Dec 15, 2014 at 04:25:32PM +0530, Kannan, Vandana wrote:
>>
>>
>> On 15-Dec-14 3:17 PM, Daniel Vetter wrote:
>>> On Thu, Dec 11, 2014 at 02:22:57AM +0530, Vandana Kannan wrote:
>>>> Adding i915 module parameter for setting drrs_interval. If this param is
>>>> set to 0, then drrs is disabled. If changed in runtime, then the new interval
>>>> value will be considered for scheduling the next drrs work.
>>>> drrs_interval is set to 0 by default, i.e. DRRS is disabled by default.
>>>
>>> Nope, please don't hide power saving features behind module options by
>>> default. New stuff must be enabled by default, otherwise it'll bitrot and
>>> merging to upstream is fairly useless since we still have the rebase pain
>>> (just spread out over more people) with none of the testing.
>> ok.. so, shall I just make the delay (drrs_interval) fixed at 1 second
>> or let user set this delay at runtime through the same module param
>> (excluding the disable feature if interval is 0 part) ?
>
> Imo we should just set an optimal value (does vbt have any hints?).
>
VBT does not contain a delay value..
Based on data collected from testing so far, 1 second seems stable..
Maybe it can go down to 800ms or so - I can test it out..
Anything as low as 100ms just triggered too many RR switches back and forth.
- Vandana

>>> Also such debug options need to be marked using the _debug variants to
>>> make sure that people report issues and don't keep using them.
>>>
>>> Now talking about validation gets me to the next point: Do we have a basic
>>> igt to make sure DRRS doesn't break right after merging? I don't think we
>>> need the full-blown test setup like for psr/fbc, but a basic test to make
>>> sure it enters/exit RR mode should be there.
>> libdrm has vbltest which prints refresh rate..
>> Apart from this, I'm adding downclock mode (if supported) in kms_flip.
>> Shall I add a test which involves low RR trigger after idle time and then
>> forces some activity on screen and switches back to high RR ?
>
> kms_flip is already huge, probably better to start a new kms_drrs
> testcase, similar to kms_fbc_crc (but without crc checks since that's not
> ineteresting here). And yeah you should trigger RR entry and then check in
> debugfs that it has indeed happened, then pageflip and check that we're
> out of RR again. That should be enough to have a decent smoketest for
> DRRS. We can add anything missing later on.
> -Daniel
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx