<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Sep 4, 2014 at 12:05 PM, Daniel Vetter <span dir="ltr"><<a href="mailto:daniel@ffwll.ch" target="_blank">daniel@ffwll.ch</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Thu, Sep 4, 2014 at 8:20 PM, Rodrigo Vivi <<a href="mailto:rodrigo.vivi@gmail.com">rodrigo.vivi@gmail.com</a>> wrote:<br>

> On Thu, Sep 4, 2014 at 2:22 AM, Daniel Vetter <<a href="mailto:daniel@ffwll.ch">daniel@ffwll.ch</a>> wrote:<br>
>><br>
>> On Wed, Sep 03, 2014 at 10:49:56PM -0400, Rodrigo Vivi wrote:<br>
>> > With Software tracking we are going to PSR sooner than we should and<br>
>> > staying<br>
>> > with blank screens in many cases.<br>
>> ><br>
>> > Using 2 identical frames to detect idleness is safier.<br>
>> ><br>
>> > Discovered and validated with refactored igt/kms_sink_psr_crc.<br>
>> ><br>
>> > Signed-off-by: Rodrigo Vivi <<a href="mailto:rodrigo.vivi@intel.com">rodrigo.vivi@intel.com</a>><br>
>> > ---<br>
>> >  drivers/gpu/drm/i915/intel_dp.c | 2 +-<br>
>> >  1 file changed, 1 insertion(+), 1 deletion(-)<br>
>> ><br>
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c<br>
>> > b/drivers/gpu/drm/i915/intel_dp.c<br>
>> > index f79473b..a796831 100644<br>
>> > --- a/drivers/gpu/drm/i915/intel_dp.c<br>
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c<br>
>> > @@ -1813,7 +1813,7 @@ static void intel_edp_psr_enable_source(struct<br>
>> > intel_dp *intel_dp)<br>
>> >       struct drm_device *dev = dig_port->base.base.dev;<br>
>> >       struct drm_i915_private *dev_priv = dev->dev_private;<br>
>> >       uint32_t max_sleep_time = 0x1f;<br>
>> > -     uint32_t idle_frames = 1;<br>
>> > +     uint32_t idle_frames = 2;<br>
>><br>
>> Hm, that sounds like we allow psr before it is really possible. And we<br>
>> still delay the actual re-enable work by 100ms, so it very much looks like<br>
>> something is broken here.<br>
><br>
><br>
><br>
> Agree. leaving idle_frame = 1 and increasing time to 600ms also works.<br>
> 500ms fails.<br>
> So I thought we had an issue with frontbuffer tracking but couldn find any.<br>
> So this idle_frame = 2 was the most clear way I could find for now.<br>
<br>
</div></div>600ms is roughly 40 frames ... that's a lot.<br>
<div><div class="h5"><br>
>><br>
>> Which exact subcases do fail?<br>
><br>
><br>
> Here are the results with idle_frame=1<br>
> IGT-Version: 1.7-gdee8754 (x86_64) (Linux: 3.17.0-rc2+ x86_64)<br>
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:<br>
> Failed assertion: strcmp(crc, CRC_BLACK) != 0<br>
> Subtest primary_page_flip: FAIL<br>
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:<br>
> Failed assertion: strcmp(crc, CRC_BLACK) != 0<br>
> Subtest primary_mmap_gtt: FAIL<br>
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:<br>
> Failed assertion: strcmp(crc, CRC_BLACK) != 0<br>
> Subtest primary_mmap_gtt_waiting: FAIL<br>
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:<br>
> Failed assertion: strcmp(crc, CRC_BLACK) != 0<br>
> Subtest primary_mmap_cpu: FAIL<br>
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:<br>
> Failed assertion: strcmp(crc, CRC_BLACK) != 0<br>
> Subtest primary_blt: FAIL<br>
> Subtest primary_render: SUCCESS<br>
> Subtest sprite_mmap_gtt: SUCCESS<br>
> Waiting 10s...<br>
> Subtest sprite_mmap_gtt_waiting: SUCCESS<br>
> Subtest sprite_mmap_cpu: SUCCESS<br>
> Subtest sprite_blt: SUCCESS<br>
> Subtest sprite_render: SUCCESS<br>
> Subtest sprite_plane_move: SUCCESS<br>
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:<br>
> Failed assertion: strcmp(crc, CRC_BLACK) != 0<br>
> Subtest sprite_plane_onoff: FAIL<br>
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:<br>
> Failed assertion: strcmp(crc, CRC_BLACK) != 0<br>
> Subtest cursor_mmap_gtt: FAIL<br>
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:<br>
> Failed assertion: strcmp(crc, CRC_BLACK) != 0<br>
> Subtest cursor_mmap_gtt_waiting: FAIL<br>
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:<br>
> Failed assertion: strcmp(crc, CRC_BLACK) != 0<br>
> Subtest cursor_mmap_cpu: FAIL<br>
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:<br>
> Failed assertion: strcmp(crc, CRC_BLACK) != 0<br>
> Subtest cursor_blt: FAIL<br>
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:<br>
> Failed assertion: strcmp(crc, CRC_BLACK) != 0<br>
> Subtest cursor_render: FAIL<br>
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:<br>
> Failed assertion: strcmp(crc, CRC_BLACK) != 0<br>
> Subtest cursor_plane_move: FAIL<br>
> Test assertion failure function get_sink_crc, file kms_psr_sink_crc.c:271:<br>
> Failed assertion: strcmp(crc, CRC_BLACK) != 0<br>
> Subtest cursor_plane_onoff: FAIL<br>
<br>
</div></div>Hm, I don't see a pattern at all. And that sprites seem to work best<br>
also looks funky. Are the results stable when you randomize them<br>
(piglit can do that for you)? Can you add a residency checks in the<br>
testcase (i.e. before the update and after the update) so that we know<br>
it's really psr and not something else funny going on? I assume this<br>
is on bdw, any difference in results on bdw?<br></blockquote><div><br></div><div>Yes, this is really strange. I couldn't find a pattern at all as well.</div><div>Do you have any example of piglit helping randomization?</div>
<div><br></div><div>The bad things with residency check is that vlv/chv doesn't have performance counters.</div><div>So this test would just work on hsw/bdw/</div><div><br></div><div>I did this work on a HSW. Test on BDW is one of tests I should do next here.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I have no idea tbh what's going on here, but it looks strange. At best<br>
my guess would be that psr exit isn't reliable ... We could check that<br>
by making sure that in the middle of the delayed gtt mmap (when psr<br>
should be disabled) psr residency is 0.<br></blockquote><div><br></div><div>Yeah, this wasn't designed to exit and get back so often. </div><div>Also the disable sequece by spec says we have to disable, wait for a vblank, than disable vsc.</div>
<div>I tried that and the results got even worse.</div><div>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... </div>
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888">-Daniel<br>
</font></span><div class="HOEnZb"><div class="h5">--<br>
Daniel Vetter<br>
Software Engineer, Intel Corporation<br>
<a href="tel:%2B41%20%280%29%2079%20365%2057%2048" value="+41793655748">+41 (0) 79 365 57 48</a> - <a href="http://blog.ffwll.ch" target="_blank">http://blog.ffwll.ch</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Rodrigo Vivi</div><div>Blog: <a href="http://blog.vivi.eng.br" target="_blank">http://blog.vivi.eng.br</a></div><div> </div>
</div></div>