[Intel-gfx] i915 PSR test results and cursor lag

Commit Message

Andy Lutomirski Feb. 5, 2018, 9:56 p.m. UTC

On Mon, Feb 5, 2018 at 9:17 PM, Pandiyan, Dhinakaran
<[e-mail]> wrote:
>
> On Mon, 2018-02-05 at 20:35 +0000, Andy Lutomirski wrote:
>> On Mon, Feb 5, 2018 at 6:53 PM, Pandiyan, Dhinakaran
>> <[e-mail]> wrote:
>> >
>> >
>> >
>> > On Sun, 2018-02-04 at 21:50 +0000, Andy Lutomirski wrote:
>> >> On Sat, Feb 3, 2018 at 5:08 PM, Andy Lutomirski <[e-mail]> wrote:
>> >> > On Sat, Feb 3, 2018 at 5:20 AM, Pandiyan, Dhinakaran
>> >> > <[e-mail]> wrote:
>> >> >>
>> >> >> On Fri, 2018-02-02 at 19:18 +0000, Andy Lutomirski wrote:
>> >> >>> I updated to 4.15, and the situation is much worse.  With
>> >> >>> enable_psr=1, the system survives for several seconds and then the
>> >> >>> screen stops updating entirely.  If I boot with i915.enable_psr=1, I
>> >> >>> get to the Fedora login screen and then the system dies.  If I set
>> >> >>> enable_psr=1 using sysfs, it does a bit after the next resume.  It
>> >> >>> seems like it also sometimes hangs even worse a bit after the screen
>> >> >>> stops updating, but it's hard to tell.
>> >> >>
>> >> >> The login screen freeze sounds like what I have. Does this system have
>> >> >> DMC firmware? If yes, can you try this series
>> >> >> https://patchwork.freedesktop.org/series/37598/. You'll only need
>> >> >> patches 1,8,9 and 10.
>> >> >
>> >> > That fixes the hang.  Feel free to add:
>> >> >
>> >> > Tested-by: Andy Lutomirski <[e-mail]>
>> >> >
>> >> > to the i915 parts.  Also, any chance of getting it into the 4.15 stable kernels?
>> >>
>> >> Correction: I'm still getting a second or two of complete screen
>> >> freezing every now and then.  The kernel says:
>> > Thanks a lot for testing. How do you trigger this freeze? Moving the
>> > cursor? Did you apply these patches on top of drm-tip or was it
>> > mainline?
>> >
>> > I also have another patch here that addresses screen freezes in console
>> > mode with PSR - https://patchwork.freedesktop.org/patch/201144/ in case
>> > that is what you are interested in.
>> >>
>> >> [69400.016524] [drm:intel_pipe_update_end [i915]] *ERROR* Atomic
>> >> update failure on pipe A (start=19 end=20) time 198 us, min 1073, max
>> >> 1079, scanline start 1068, end 1082
>> >>
>> >> So something might still be a bit buggy.
>> >
>> > This series fixes only the long freezes due to frame counter resets, I
>> > am sure there are still other issues with PSR.
>> >
>> > BTW does your patch on top of these patches help with the cursor lag?
>>
>> Maybe, but I'm not 100% sure.  I'm not currently seeing the lag with
>> or without the patch.  I also think my distro fixed the cursor in the
>> mean time so that it uses the HW cursor even after suspend/resume.
>>
>> A couple of questions, though:
>>
>> 1. Does moving the HW cursor cause the hardware to automatically turn off PSR?
>>
> That is correct.
>
>> 2 When something enables vblank interrupts (using drm_*_vblank_get(),
>> for example), are vblank interrupts generated even if PSR is on?
>
> Enabling vblank interrupts deactivates PSR (except on Braswell afaik)
>
>>   And
>> is the scanline, as returned by intel_get_crtc_scanline(), updated?
>
> I don't think so, I have not really checked but there are no frames
> generated, so the timing related registers will not get updated. This is
> the case with the frame counter register.
>

I bet that's the cause of some of the glitches I'm seeing.  I
instrumented intel_pipe_update_start() like this:

     vblank_start = adjusted_mode->crtc_vblank_start;
     if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
@@ -131,9 +132,12 @@ void intel_pipe_update_start(const struct
intel_crtc_state *new_crtc_state)
         if (scanline < min || scanline > max)
             break;

+        if (first_scanline == -1)
+            first_scanline = scanline;
+
         if (timeout <= 0) {
-            DRM_ERROR("Potential atomic update failure on pipe %c\n",
-                  pipe_name(crtc->pipe));
+            DRM_ERROR("Potential atomic update failure on pipe %c.
scanline=%d, first_scanline=%d\n",
+                  pipe_name(crtc->pipe), scanline, first_scanline);
             break;
         }

and I got:

[drm:intel_pipe_update_start [i915]] *ERROR* Potential atomic update
failure on pipe A.  scanline=1079, first_scanline=1079

so it looks like it can get stuck if called at the wrong time.

Anyway, I'll send my patch.  I'm not convinced it fixes any of the
glitches I'm seeing, but I think it's a good improvement regardless.

Patch

diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 4a8a5d918a83..6ce0a35187fb 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -97,6 +97,7 @@  void intel_pipe_update_start(const struct
intel_crtc_state *new_crtc_state)
     bool need_vlv_dsi_wa = (IS_VALLEYVIEW(dev_priv) ||
IS_CHERRYVIEW(dev_priv)) &&
         intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI);
     DEFINE_WAIT(wait);
+    int first_scanline = -1;