Created attachment 141159 [details]
Dmesg during playback

Created attachment 141160 [details]
intel_ reg dump --all

Created attachment 141161 [details]
vbios dump

Created attachment 141162 [details]
xrandr --verbose

commit 6a2c4232ece145d8b5a8f95f767bd6d0d2d2f2bb was required to fix some cursor corruption; but it should only affect the cursor plane on creation. There should be no need for frequent recreation of the cursor (as I recall the cursor plane is kept for as long as it sized correctly, and even if disabled only destroyed on the next modeset).

Put a WARN in i915_gem_object_attach_phys() to see how frequently that code is being called. If it is being called, grab an xorg debug log (xf86-video-intel compiled with --enable-debug=full) for the next question is why. If not, the problem is nothing to do with the phys object.

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 592b847db88e..d47ec9fd4af4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12966,8 +12966,11 @@ static int intel_plane_pin_fb(struct intel_plane_state *plane_state)
            INTEL_INFO(dev_priv)->cursor_needs_physical) {
                struct drm_i915_gem_object *obj = intel_fb_obj(fb);
                const int align = intel_cursor_alignment(dev_priv);
+               int err;
 
-               return i915_gem_object_attach_phys(obj, align);
+               err = i915_gem_object_attach_phys(obj, align);
+               if (err)
+                       return err;
        }
 
        vma = intel_pin_and_fence_fb_obj(fb,

See https://patchwork.freedesktop.org/series/48379/

(In reply to Chris Wilson from comment #7)
> See https://patchwork.freedesktop.org/series/48379/

Many thanks for looking at this issue. I'm compiling with your patch now, but I'm unsure whether I'm supposed to do something about comment #5:

"Put a WARN in i915_gem_object_attach_phys() to see how frequently that code is being called. If it is being called, grab an xorg debug log (xf86-video-intel compiled with --enable-debug=full) for the next question is why. If not, the problem is nothing to do with the phys object."

My guess is comment #5 was your first thought to collect debugging info and then you had a better idea, namely this patch. So, I am assuming I should not put a WARN in i915_gem_object_attach_phys(). If you do want a WARN added to i915_gem_object_attach_phys(), I would appreciate a patch, because I am unfamiliar with kernel WARN macros and how to use them.

It wasn't clear to me, reading the commit msg for 6a2c4232ece145d8b5a8f95f767bd6d0d2d2f2bb, that it had anything to do with cursors (and I assume we're talking mouse cursors). FWIW, the mythtv frontend/player normally doesn't display a cursor, unless the mouse is moved. The frontend does have an option to disable the mouse cursor entirely, which I have not enabled. It also has options to change the output mode, refresh rate, etc. based on the video being played, which I haven't enabled either. Those options are intended to take advantage of a display's deinterlacing capabilities (especially TVs), which I have never explored.

(In reply to H Buus from comment #8)
> (In reply to Chris Wilson from comment #7)
> > See https://patchwork.freedesktop.org/series/48379/
> 
> Many thanks for looking at this issue. I'm compiling with your patch now,
> but I'm unsure whether I'm supposed to do something about comment #5:
> 
> "Put a WARN in i915_gem_object_attach_phys() to see how frequently that code
> is being called. If it is being called, grab an xorg debug log
> (xf86-video-intel compiled with --enable-debug=full) for the next question
> is why. If not, the problem is nothing to do with the phys object."
> 
> My guess is comment #5 was your first thought to collect debugging info and
> then you had a better idea, namely this patch. So, I am assuming I should
> not put a WARN in i915_gem_object_attach_phys(). If you do want a WARN added
> to i915_gem_object_attach_phys(), I would appreciate a patch, because I am
> unfamiliar with kernel WARN macros and how to use them.

Stick with investigating the patch. I checked the ddx and afaict it should be caching the cursor just fine, so the likely cause is aperture thrashing amongst GTT mmaps rather than the cost of instantiating new physical cursors.

If the patch doesn't resolve it, then I think looking at the ddx debug log is the next step. There we are just looking for how the cursor is being used.

> It wasn't clear to me, reading the commit msg for
> 6a2c4232ece145d8b5a8f95f767bd6d0d2d2f2bb, that it had anything to do with
> cursors (and I assume we're talking mouse cursors).

Physical objects are only used for 2 things, the register file for the overlay plane (static, allocated on module load) and cursors which are dynamically bound  objects.

> FWIW, the mythtv
> frontend/player normally doesn't display a cursor, unless the mouse is
> moved. The frontend does have an option to disable the mouse cursor
> entirely, which I have not enabled. It also has options to change the output
> mode, refresh rate, etc. based on the video being played, which I haven't
> enabled either. Those options are intended to take advantage of a display's
> deinterlacing capabilities (especially TVs), which I have never explored.

Yeah, which is why it was quite surprising. My guess is that even though you don't see the cursor, it is redrawing an invisible cursor. That's all guess work, but it's the only way I could connect the patch to MythTV. :|

I'm afraid https://patchwork.freedesktop.org/series/48379/ did not help.

I first tested without drm.debug set. FWIW, I noticed the cursor is briefly displayed when playback starts and stops, but definitely not when I see the stuttering.

I experimented with turning on mythtv's "hide cursor" option. Didn't help, I still see stuttering. So I rebooted with debug enabled and log_buf_len=512K. The output is in dmesg4.txt

I started playback after about 120 seconds from boot time, with "hide cursor" disabled. Same stuttering. Stopped playback.

Some time after 190 seconds, I turned on the "hide cursor" option and even unplugged the USB optical mouse I use when doing maintenance (never been fond of touchpads). Then started playback. I still see stuttering.

You mentioned looking at the ddx debug log. Is that the same as the "xorg debug log (xf86-video-intel compiled with --enable-debug=full)" mentioned in comment #5? If so, would it be sufficient to download and compile the deb-src package that matches the binary version currently installed on the laptop? It is called xserver-xorg-video-intel and is at version 2:2.99.917+git20171229-1. That version contains the most recent tag albeit from 4 years ago, and apparently has some patches from last December. Guess I'm worried compiling from master could introduce ABI incompatibilities with other xorg packages.

Also, would I be correct in assuming the xorg/ddx debug log will go to /var/log/Xorg.0.log?

Created attachment 141173 [details]
dmesg with patch https://patchwork.freedesktop.org/series/48379/

(In reply to H Buus from comment #10)
> I'm afraid https://patchwork.freedesktop.org/series/48379/ did not help.
> 
> I first tested without drm.debug set. FWIW, I noticed the cursor is briefly
> displayed when playback starts and stops, but definitely not when I see the
> stuttering.
> 
> I experimented with turning on mythtv's "hide cursor" option. Didn't help, I
> still see stuttering. So I rebooted with debug enabled and log_buf_len=512K.
> The output is in dmesg4.txt
> 
> I started playback after about 120 seconds from boot time, with "hide
> cursor" disabled. Same stuttering. Stopped playback.
> 
> Some time after 190 seconds, I turned on the "hide cursor" option and even
> unplugged the USB optical mouse I use when doing maintenance (never been
> fond of touchpads). Then started playback. I still see stuttering.

Darn. How frequent is the stutter. The dmesg shows it is reprobing the displays (which will cause it to stutter) every 15s which is very odd, as that just happens to be the same as the ddx's cache. That suggests MythTV is constantly reprobing? Really, really odd.

One thing you can quickly try is to change the value reported for HAS_COHERENT_PHYS_GTT, say

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 35a012ffc03b..175fdad11352 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -312,6 +312,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data,
        case I915_PARAM_ALLOW_BATCHBUFFER:
        case I915_PARAM_LAST_DISPATCH:
        case I915_PARAM_HAS_EXEC_CONSTANTS:
+       case I915_PARAM_HAS_COHERENT_PHYS_GTT:
                /* Reject all old ums/dri params. */
                return -ENODEV;
        case I915_PARAM_CHIPSET_ID:
@@ -411,7 +412,6 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data,
        case I915_PARAM_HAS_PINNED_BATCHES:
        case I915_PARAM_HAS_EXEC_NO_RELOC:
        case I915_PARAM_HAS_EXEC_HANDLE_LUT:
-       case I915_PARAM_HAS_COHERENT_PHYS_GTT:
        case I915_PARAM_HAS_EXEC_SOFTPIN:
        case I915_PARAM_HAS_EXEC_ASYNC:
        case I915_PARAM_HAS_EXEC_FENCE:

and that will cause the ddx to use the same old pwrite interface it previously used. But I suspect the cursor is just my wild goose chase.

> You mentioned looking at the ddx debug log. Is that the same as the "xorg
> debug log (xf86-video-intel compiled with --enable-debug=full)" mentioned in
> comment #5? If so, would it be sufficient to download and compile the
> deb-src package that matches the binary version currently installed on the
> laptop? It is called xserver-xorg-video-intel and is at version
> 2:2.99.917+git20171229-1. That version contains the most recent tag albeit
> from 4 years ago, and apparently has some patches from last December. Guess
> I'm worried compiling from master could introduce ABI incompatibilities with
> other xorg packages.

It's API compatible with Xorg-1.6 through 1.19 (there's one tiny change required for 1.20 and Prime offload :(, but not going to affect you.

> Also, would I be correct in assuming the xorg/ddx debug log will go to
> /var/log/Xorg.0.log?

Yes.

(In reply to Chris Wilson from comment #12)
> (In reply to H Buus from comment #10)
> > I'm afraid https://patchwork.freedesktop.org/series/48379/ did not help.
> > 
> > I first tested without drm.debug set. FWIW, I noticed the cursor is briefly
> > displayed when playback starts and stops, but definitely not when I see the
> > stuttering.
> > 
> > I experimented with turning on mythtv's "hide cursor" option. Didn't help, I
> > still see stuttering. So I rebooted with debug enabled and log_buf_len=512K.
> > The output is in dmesg4.txt
> > 
> > I started playback after about 120 seconds from boot time, with "hide
> > cursor" disabled. Same stuttering. Stopped playback.
> > 
> > Some time after 190 seconds, I turned on the "hide cursor" option and even
> > unplugged the USB optical mouse I use when doing maintenance (never been
> > fond of touchpads). Then started playback. I still see stuttering.
> 
> Darn. How frequent is the stutter. The dmesg shows it is reprobing the
> displays (which will cause it to stutter) every 15s which is very odd, as
> that just happens to be the same as the ddx's cache. That suggests MythTV is
> constantly reprobing? Really, really odd.

It's definitely more often than 15 seconds, although it's possible there's a greater stutter at 15 sec intervals. Like I said, it's slight, but noticeable and distracting during panning. Also during zooms that are relatively slow & smooth. There's also some variability in severity, from run to run. It might not bother everyone, but it bothers me. I could try to record it using a 720p camera mounted to a tripod. My phone would probably be better, but I don't have a way to mount it to my tripod.

> One thing you can quickly try is to change the value reported for
> HAS_COHERENT_PHYS_GTT, say
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> index 35a012ffc03b..175fdad11352 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -312,6 +312,7 @@ static int i915_getparam_ioctl(struct drm_device *dev,
> void *data,
>         case I915_PARAM_ALLOW_BATCHBUFFER:
>         case I915_PARAM_LAST_DISPATCH:
>         case I915_PARAM_HAS_EXEC_CONSTANTS:
> +       case I915_PARAM_HAS_COHERENT_PHYS_GTT:
>                 /* Reject all old ums/dri params. */
>                 return -ENODEV;
>         case I915_PARAM_CHIPSET_ID:
> @@ -411,7 +412,6 @@ static int i915_getparam_ioctl(struct drm_device *dev,
> void *data,
>         case I915_PARAM_HAS_PINNED_BATCHES:
>         case I915_PARAM_HAS_EXEC_NO_RELOC:
>         case I915_PARAM_HAS_EXEC_HANDLE_LUT:
> -       case I915_PARAM_HAS_COHERENT_PHYS_GTT:
>         case I915_PARAM_HAS_EXEC_SOFTPIN:
>         case I915_PARAM_HAS_EXEC_ASYNC:
>         case I915_PARAM_HAS_EXEC_FENCE:
> 
> and that will cause the ddx to use the same old pwrite interface it
> previously used. But I suspect the cursor is just my wild goose chase.

I'm compiling it now. Probably won't have results until late tonight or tomorrow AM, EDT.

> > You mentioned looking at the ddx debug log. Is that the same as the "xorg
> > debug log (xf86-video-intel compiled with --enable-debug=full)" mentioned in
> > comment #5? If so, would it be sufficient to download and compile the
> > deb-src package that matches the binary version currently installed on the
> > laptop? It is called xserver-xorg-video-intel and is at version
> > 2:2.99.917+git20171229-1. That version contains the most recent tag albeit
> > from 4 years ago, and apparently has some patches from last December. Guess
> > I'm worried compiling from master could introduce ABI incompatibilities with
> > other xorg packages.
> 
> It's API compatible with Xorg-1.6 through 1.19 (there's one tiny change
> required for 1.20 and Prime offload :(, but not going to affect you.

So you'd prefer I compile from master, right? I'm experimenting with the source package at the moment, but I can switch to a git repo. The deb source did help installing required packages and giving me a starting point with configure.

> > Also, would I be correct in assuming the xorg/ddx debug log will go to
> > /var/log/Xorg.0.log?
> 
> Yes.

(In reply to H Buus from comment #13)
> (In reply to Chris Wilson from comment #12)
> > It's API compatible with Xorg-1.6 through 1.19 (there's one tiny change
> > required for 1.20 and Prime offload :(, but not going to affect you.
> 
> So you'd prefer I compile from master, right? I'm experimenting with the
> source package at the moment, but I can switch to a git repo. The deb source
> did help installing required packages and giving me a starting point with
> configure.

Indeed deb-src will have given you all the builddeps you need, and I'd prefer to go with master as quite likely we may be playing with a few patches there as well.

Can you also grab x[11]trace and record the stutter in MythTV using that? (xtrace -n -o stutter.txt mythtv...)

The second-to-last piece of the puzzle would be something like drm.debug=0x7 so that we get all the ioctls being called, and the final would be an ftrace of all the i915 tracepoints.

Sadly all of that will produce a lot of information, completely hiding the stutter. I'm just hoping to hit upon an idea worth investigating (the i915:i915_request_wait_begin tracepoint would be a good candidate).

Created attachment 141175 [details]
with HAS_COHERENT_PHYS_GTT patch

The HAS_COHERENT_PHYS_GTT patch made no difference, see dmesg4.txt. I applied it on top of the first patch. Hope that was OK.

I ran xtrace, see stutter.txt. It didn't obscure the stutter, although it took a few seconds longer to navigate the menus to start playback. Before this test, I did reboot without drm.debug set. Hope that was OK.

Let me know if you want the drm.debug=0x7 output and/or ftrace. I've never done an ftrace. Something new to learn about.

Created attachment 141176 [details]
xtrace output

Comment on attachment 141176 [details]
xtrace output

I gzipped stutter.txt and uploaded it. stutter.txt was 64MB, stutter.gz was over 5MB, over the limit of 4MB documented on the upload page. Just glad it let me do it.

The xtrace shows it is using the XVideo overlay with a mix of DRI2 and XRender (for text). Not much cursor activity, so that was indeed a red herring.

Your machine also uses a phys_attach for the overlay registers, but that should not have been affected as it isn't exposed to userspace and so doesn't take part in mmap or pwrite. Weird.

Honestly, there's a slim chance it's unhappy with the overlay register mapping.

Created attachment 141178 [details] [review]
phys overlay in stolen

This should avoid using the physical bo on your machine. Only compile tested though.

Created attachment 141180 [details] [review]
phys overlay in stolen

(In reply to Chris Wilson from comment #20)
> Created attachment 141178 [details] [review] [review]
> phys overlay in stolen
> 
> This should avoid using the physical bo on your machine. Only compile tested
> though.

Should I revert the other two patches?

I don't understand what "Only compile tested though." means.

(In reply to H Buus from comment #22)
> (In reply to Chris Wilson from comment #20)
> > Created attachment 141178 [details] [review] [review] [review]
> > phys overlay in stolen
> > 
> > This should avoid using the physical bo on your machine. Only compile tested
> > though.
> 
> Should I revert the other two patches?

For simplicity, without. (But since we know they don't fix the stutter, it doesn't really matter.)
 
> I don't understand what "Only compile tested though." means.

I haven't actually run it myself, only compiled it.

Bad news. With:
https://bugs.freedesktop.org/attachment.cgi?id=141180
I only get a blue screen when playing video. Outside of video playback, the screen looks fine.

Created attachment 141183 [details]
dmesg with 'phys overlay in stolen' patch, 141180, blue screen playing video

I recorded some videos that demonstrate the stuttering problem and put them up on google drive. You can find them here:
https://drive.google.com/open?id=1k165w4Ok6blFyr9uHFn2LTbVhstyZoT8
There are four mp4 videos recorded at 1080p 30fps. Each are about 2 minutes long and average around 250GB. They are named:
linux<linux-version>-<season-episode>.mp4
where:

<linux-version> is either 3.13 or 4.18. 3.13 refers to the Ubuntu supplied kernel for ubuntu 14.04, 4.18 refers to the linux kernel compiled from a clone of git://anongit.freedesktop.org/drm-tip as of 8/12/18.

<season-episode> is either s01e01 or s02e20.

The linux4.18* videos show the stuttering in both the opening scenes of two different episodes as well as in the credits where there is panning and zooming over still images.

The linux3.13* videos are smooth in comparison.

The recordings also capture some info in a terminal window, namely the output from "uname -a" as well as date and sensors output, showing that the laptop doesn't seem to be struggling with thermal problems as a result of playing the videos. The sensors output suggests the most stressful activity is booting up.

I hope you find these clips useful in understanding the issue. If the files are too big, I could cut the credits. Or I could re-record at 60 fps in the hopes the video quality might be better. The laptop's video playback quality is much better than these recordings suggest.

(In reply to H Buus from comment #26)
> I recorded some videos that demonstrate the stuttering problem and put them
> up on google drive. You can find them here:
> https://drive.google.com/open?id=1k165w4Ok6blFyr9uHFn2LTbVhstyZoT8
> There are four mp4 videos recorded at 1080p 30fps. Each are about 2 minutes
> long and average around 250GB.
Oops, I meant 250MB!

Created attachment 141386 [details] [review]
Allocate overlay from stolen

Took a bit of time before I had my i915gm in a usable test once again, sorry for the delay. Revamp patch to avoid using the phys object, primary purpose is to confirm that is what is causing the stutter. (Still baffling to me, the phys object is attached just once for the lifetime of the driver and just stays there!)

Created attachment 141404 [details]
dmesg with attachment #141386 [details] [review]

Better late than never. I was thinking maybe this old hardware was no longer worth your time. I'm glad you're still interested in figuring out what's going on.

I'm sure it's not what you want to hear, but playback IS smooth with this patch. dmesg6.txt is the output with drm.debug=0xe. I hope it gives you what you need.

Sorry it took me a while to get back with results. I found out you can't resync a drm-tip clone using the usual git tools. So I switched to a linux-stable clone. For some reason, compiling the master branch gave me a kernel with no working Ethernet. Rather than spend time trying to figure out why, I switched to the linux-4.18.y branch. I somehow built a 64 bit kernel and had to start over, but I finally got a reasonably recent (4.18.5) 32 bit kernel with working Ethernet and this patch.

H Buus, is this issue still open to you?

Though it doesn't explain what's going wrong, it does paper over the problem with better utilization of memory:

commit c8124d399224d626728e2ffb95a1d564a7c06968
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu Sep 6 20:01:43 2018 +0100

    drm/i915/overlay: Allocate physical registers from stolen
    
    Given that we are now reasonably confident in our ability to detect and
    reserve the stolen memory (physical memory reserved for graphics by the
    BIOS) for ourselves on most machines, we can put it to use. In this
    case, we need a page to hold the overlay registers.
    
    On an i915g running MythTv, H Buus noticed that
    
            commit 6a2c4232ece145d8b5a8f95f767bd6d0d2d2f2bb
            Author: Chris Wilson <chris@chris-wilson.co.uk>
            Date:   Tue Nov 4 04:51:40 2014 -0800
            drm/i915: Make the physical object coherent with GTT
    
    introduced stuttering into his video playback. After discarding the
    likely suspect of it being the physical cursor updates, we were left
    with the use of the phys object for the overlay. And lo, if we
    completely avoid using the phys object (allocated just once on module
    load!) by switching to stolen memory, the stuttering goes away.
    
    For lack of a better explanation, claim victory and kill two birds with
    one stone.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107600
    Fixes: 6a2c4232ece1 ("drm/i915: Make the physical object coherent with GTT")
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
    Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
    Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
    Link: https://patchwork.freedesktop.org/patch/msgid/20180906190144.1272-1-chris@chris-wilson.co.uk


We should keep this bug open for a little while longer to see if we can narrow down where phys is playing up.

(In reply to Chris Wilson from comment #31)
<snip>
> We should keep this bug open for a little while longer to see if we can
> narrow down where phys is playing up.

Let me know if there's something more I can do. In the meantime, I'll wait until this patch makes it way upstream to linux-stable and then retest.

> Let me know if there's something more I can do. In the meantime, I'll wait
> until this patch makes it way upstream to linux-stable and then retest.

Buus, this is now available in mainline 4.19-rc8.

No feedback from more than two months.

The issue have been fixed, closing now. Feel free to reopen if the
issue persists with latest drmtip. Remember to attach dmesg log from boot with
kernel parameters.
(https://cgit.freedesktop.org/drm-tip).

I agree this can be closed. I have been running 4.19 since October (currently running 4.19.18) and all seems well. My apologies for not reporting back sooner.