[Intel-gfx] [PATCH v3 3/6] drm/i915: Amalgamate flushing of display objects

[Yaroslav Shabalin]

On Thu, Sep 14, 2017 at 1:57 PM, Chris Wilson <chris at chris-wilson.co.uk>
wrote:

> Quoting Yaroslav Shabalin (2017-09-12 23:11:03)
> > Hi!
> >
> > I would like to report a problem which I believe is connected to changes
> > introduced in this patch (https://lists.freedesktop.
> org/archives/intel-gfx/
> > 2017-February/120777.html). Specifically the problem is: screen freezes
> for 0.5
> > - 5 seconds (mostly for 2-3 seconds) when PSR is enabled
> (i915.enable_psr=1
> > kernel boot option). Freezes are more frequent with rich, dynamic screen
> > content. The most reliable way to reproduce it, I discovered so far, is
> to
> > start Gnome "Disk Usage Analyzer", analyze some catalog and move mouse
> pointer
> > over color rectangles for a while. Usually that way I get 3-5 freezes in
> a
> > minute. During normal PC usage freezes are happening less frequently,
> but still
> > annoyingly. For the first time this problem appeared after update to
> kernel
> > 4.12 and still there in recent 4.13 too.
> > I've looked into it a little and seems that mentioned patch is the one
> that
> > brought it in. Git bisect indicates it as the first bad commit. After
> realizing
> > that I tried to partly revert changes in the patch and found that
> following
> > change:
> >
> >         /* Treat this as an end-of-frame, like
> intel_user_framebuffer_dirty() */
> > -       if (obj->cache_dirty || obj->base.write_domain ==
> I915_GEM_DOMAIN_CPU) {
> > -               i915_gem_clflush_object(obj, true);
> > -               intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);
> > -       }
> > +       __i915_gem_object_flush_to_display(obj);
> > +       intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);
> >
> > 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:
>
> Honestly, that doesn't seem like causation for the PSR failure.
> -Chriss
>

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.
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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20170914/6bc41fba/attachment.html>