On Thu, Sep 4, 2014 at 12:05 PM, Daniel Vetter <daniel@xxxxxxxx> wrote:
600ms is roughly 40 frames ... that's a lot.On Thu, Sep 4, 2014 at 8:20 PM, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote:
> On Thu, Sep 4, 2014 at 2:22 AM, Daniel Vetter <daniel@xxxxxxxx> 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@xxxxxxxxx>
>> > ---
>> > 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.
Hm, I don't see a pattern at all. And that sprites seem to work best
>>
>> 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
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?
Yes, this is really strange. I couldn't find a pattern at all as well.
Do you have any example of piglit helping randomization?
The bad things with residency check is that vlv/chv doesn't have performance counters.
So this test would just work on hsw/bdw/
I did this work on a HSW. Test on BDW is one of tests I should do next here.
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.
Yeah, this wasn't designed to exit and get back so often.
Also the disable sequece by spec says we have to disable, wait for a vblank, than disable vsc.
I tried that and the results got even worse.
Another idea was to use srd_debug register to force exit and normal opperation instead of disabled and reenable constantly as Ville also suggested, but my tests with this approach didn't go anywhere...
-Daniel
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
