[Intel-gfx] [PATCH 1/4] drm/i915: Increase PSR Idle Frame to 2.

[Daniel Vetter]

On Thu, Sep 4, 2014 at 8:20 PM, Rodrigo Vivi <rodrigo.vivi at gmail.com> wrote:
> On Thu, Sep 4, 2014 at 2:22 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
>>
>> On Wed, Sep 03, 2014 at 10:49:56PM -0400, Rodrigo Vivi wrote:
>> > With Software tracking we are going to PSR sooner than we should and
>> > staying
>> > with blank screens in many cases.
>> >
>> > Using 2 identical frames to detect idleness is safier.
>> >
>> > Discovered and validated with refactored igt/kms_sink_psr_crc.
>> >
>> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> > b/drivers/gpu/drm/i915/intel_dp.c
>> > index f79473b..a796831 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -1813,7 +1813,7 @@ static void intel_edp_psr_enable_source(struct
>> > intel_dp *intel_dp)
>> >       struct drm_device *dev = dig_port->base.base.dev;
>> >       struct drm_i915_private *dev_priv = dev->dev_private;
>> >       uint32_t max_sleep_time = 0x1f;
>> > -     uint32_t idle_frames = 1;
>> > +     uint32_t idle_frames = 2;
>>
>> Hm, that sounds like we allow psr before it is really possible. And we
>> still delay the actual re-enable work by 100ms, so it very much looks like
>> something is broken here.
>
>
>
> Agree. leaving idle_frame = 1 and increasing time to 600ms also works.
> 500ms fails.
> So I thought we had an issue with frontbuffer tracking but couldn find any.
> So this idle_frame = 2 was the most clear way I could find for now.

600ms is roughly 40 frames ... that's a lot.

>>
>> Which exact subcases do fail?
>
>
> Here are the results with idle_frame=1
> IGT-Version: 1.7-gdee8754 (x86_64) (Linux: 3.17.0-rc2+ x86_64)
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:
> Failed assertion: strcmp(crc, CRC_BLACK) != 0
> Subtest primary_page_flip: FAIL
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:
> Failed assertion: strcmp(crc, CRC_BLACK) != 0
> Subtest primary_mmap_gtt: FAIL
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:
> Failed assertion: strcmp(crc, CRC_BLACK) != 0
> Subtest primary_mmap_gtt_waiting: FAIL
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:
> Failed assertion: strcmp(crc, CRC_BLACK) != 0
> Subtest primary_mmap_cpu: FAIL
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:
> Failed assertion: strcmp(crc, CRC_BLACK) != 0
> Subtest primary_blt: FAIL
> Subtest primary_render: SUCCESS
> Subtest sprite_mmap_gtt: SUCCESS
> Waiting 10s...
> Subtest sprite_mmap_gtt_waiting: SUCCESS
> Subtest sprite_mmap_cpu: SUCCESS
> Subtest sprite_blt: SUCCESS
> Subtest sprite_render: SUCCESS
> Subtest sprite_plane_move: SUCCESS
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:
> Failed assertion: strcmp(crc, CRC_BLACK) != 0
> Subtest sprite_plane_onoff: FAIL
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:
> Failed assertion: strcmp(crc, CRC_BLACK) != 0
> Subtest cursor_mmap_gtt: FAIL
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:
> Failed assertion: strcmp(crc, CRC_BLACK) != 0
> Subtest cursor_mmap_gtt_waiting: FAIL
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:
> Failed assertion: strcmp(crc, CRC_BLACK) != 0
> Subtest cursor_mmap_cpu: FAIL
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:
> Failed assertion: strcmp(crc, CRC_BLACK) != 0
> Subtest cursor_blt: FAIL
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:
> Failed assertion: strcmp(crc, CRC_BLACK) != 0
> Subtest cursor_render: FAIL
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:
> Failed assertion: strcmp(crc, CRC_BLACK) != 0
> Subtest cursor_plane_move: FAIL
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:
> Failed assertion: strcmp(crc, CRC_BLACK) != 0
> Subtest cursor_plane_onoff: FAIL

Hm, I don't see a pattern at all. And that sprites seem to work best
also looks funky. Are the results stable when you randomize them
(piglit can do that for you)? Can you add a residency checks in the
testcase (i.e. before the update and after the update) so that we know
it's really psr and not something else funny going on? I assume this
is on bdw, any difference in results on bdw?

I have no idea tbh what's going on here, but it looks strange. At best
my guess would be that psr exit isn't reliable ... We could check that
by making sure that in the middle of the delayed gtt mmap (when psr
should be disabled) psr residency is 0.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch