<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Sep 14, 2017 at 1:57 PM, Chris Wilson <span dir="ltr"><<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Yaroslav Shabalin (2017-09-12 23:11:03)<br>
<span class="gmail-">> Hi!<br>
><br>
> I would like to report a problem which I believe is connected to changes<br>
> introduced in this patch (<a href="https://lists.freedesktop.org/archives/intel-gfx/" rel="noreferrer" target="_blank">https://lists.freedesktop.<wbr>org/archives/intel-gfx/</a><br>
> 2017-February/120777.html). Specifically the problem is: screen freezes for 0.5<br>
> - 5 seconds (mostly for 2-3 seconds) when PSR is enabled (i915.enable_psr=1<br>
> kernel boot option). Freezes are more frequent with rich, dynamic screen<br>
> content. The most reliable way to reproduce it, I discovered so far, is to<br>
> start Gnome "Disk Usage Analyzer", analyze some catalog and move mouse pointer<br>
> over color rectangles for a while. Usually that way I get 3-5 freezes in a<br>
> minute. During normal PC usage freezes are happening less frequently, but still<br>
> annoyingly. For the first time this problem appeared after update to kernel<br>
> 4.12 and still there in recent 4.13 too.<br>
> I've looked into it a little and seems that mentioned patch is the one that<br>
> brought it in. Git bisect indicates it as the first bad commit. After realizing<br>
> that I tried to partly revert changes in the patch and found that following<br>
> change:<br>
><br>
>         /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */<br>
> -       if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) {<br>
> -               i915_gem_clflush_object(obj, true);<br>
> -               intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);<br>
> -       }<br>
> +       __i915_gem_object_flush_to_<wbr>display(obj);<br>
> +       intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);<br>
><br>
> when reverted solves the problem completely. One of the main difference is that in old version intel_fb_obj_flush was called conditionally and now it is called every time. So I surrounded the call with "if" just to check and it worked:<br>
<br>
</span>Honestly, that doesn't seem like causation for the PSR failure.<br>
<span class="gmail-HOEnZb"><font color="#888888">-Chriss <br></font></span></blockquote><div><br></div><div>I see your doubts about connection between this two facts, but still that only change makes PSR unusable on my laptop. I am pretty comfortable with applying the patch every time I compile kernel to get it working. Just wanted to point out potential problem for the benefit of other users. Actually it took me some time to git bisect and find the root reason. I wouldn't bother you if wasn't sure about it, please take it seriously. And despite that there is no visible connection between PSR and that commit I presume there may exist some kind of side effects.</div><div>Actually I can't give any technical argument since I don't know a thing about how the driver works, it's internal structure, etc. But there are facts that I know and I try to find some reasoning based on logic and common sense. As a result there are questions, that I guess may get us closer to understanding it. Was moving intel_fb_obj_flush call out of conditional check an intentional change? Is it really needed to call intel_fb_obj_flush every time even if cache is not dirty or write_domain is not equal to that specific value? I would really appreciate if you take some time to answer it.</div><div><br></div><div><br></div></div></div></div>