All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mario Kleiner <mario.kleiner.de@gmail.com>
To: "Michel Dänzer" <michel@daenzer.net>,
	"Vlastimil Babka" <vbabka@suse.cz>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: "Daniel Vetter" <daniel.vetter@ffwll.ch>,
	LKML <linux-kernel@vger.kernel.org>,
	dri-devel@lists.freedesktop.org, mgraesslin@kde.org,
	kwin@kde.org, "Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>
Subject: Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
Date: Thu, 21 Jan 2016 06:31:26 +0100	[thread overview]
Message-ID: <56A06D2E.4000008@gmail.com> (raw)
In-Reply-To: <56A053CE.7000500@daenzer.net>

On 01/21/2016 04:43 AM, Michel Dänzer wrote:
> On 21.01.2016 05:32, Mario Kleiner wrote:
>>
>> So the problem is that AMDs hardware frame counters reset to
>> zero during a modeset. The old DRM code dealt with drivers doing that by
>> keeping vblank irqs enabled during modesets and incrementing vblank
>> count by one during each vblank irq, i think that's what
>> drm_vblank_pre_modeset() and drm_vblank_post_modeset() were meant for.
>
> Right, looks like there's been a regression breaking this. I suspect the
> problem is that vblank->last isn't getting updated from
> drm_vblank_post_modeset. Not sure which change broke that though, or how
> to fix it. Ville?
>

The whole logic has changed and the software counter updates are now 
driven all the time by the hw counter.

>
> BTW, I'm seeing a similar issue with drm_vblank_on/off as well, which
> exposed the bug fixed by 209e4dbc ("drm/vblank: Use u32 consistently for
> vblank counters"). I've been meaning to track that down since then; one
> of these days hopefully, but if anybody has any ideas offhand...
>
>

I spent the last few hours reading through the drm and radeon code and i 
think what should probably work is to replace the 
drm_vblank_pre/post_modeset calls in radeon/amdgpu by drm_vblank_off/on 
calls. These are apparently meant for drivers whose hw counters reset 
during modeset, and seem to reinitialize stuff properly and release 
clients queued vblank events to avoid blocking - not tested so far, just 
looked at the code.

Once drm_vblank_off is called, drm_vblank_get will no-op and return an 
error, so clients can't enable vblank irqs during the modeset - pageflip 
ioctl and waitvblank ioctl would fail while a modeset happens - 
hopefully userspace handles this correctly everywhere.

It would also cause radeons power management to not sync its actions to 
vblank if it would get invoked during a modeset, but that seems to be 
handled by a 200 msec timeout and hopefully only cause visual glitches - 
or invisible glitches while the crtc is blanked during modeset?

There could be another tiny race with the new "vblank counter bumping" 
logic from commit 5b5561b ("drm/radeon: Fixup hw vblank counters/ts 
...") if drm_update_vblank_counter() would be called multiple times in 
quick succession within the "radeon_crtc->lb_vblank_lead_lines" 
scanlines before start of real vblank iff at the same time a modeset 
would happen and set radeon_crtc->lb_vblank_lead_lines to a smaller 
value due to a change in horizontal mode resolution. That needs a 
modeset to happen to a higher horizontal resolution just exactly when 
the scanout is in exactly the right 5 or so scanlines and some client is 
calling drm_vblank_get() to enable vblank irqs at the same time, but it 
would cause the same hang if it happened - not that likely to happen 
often, but still not nice, also Murphy's law... If we could switch to 
drm_vblank_off/on instead of drm_vblank_pre/post_modeset we could remove 
those race as well by forbidding any vblank irq related activity during 
a modeset.

I'll hack up a patch for demonstration now.

WARNING: multiple messages have this Message-ID (diff)
From: Mario Kleiner <mario.kleiner.de@gmail.com>
To: "Michel Dänzer" <michel@daenzer.net>,
	"Vlastimil Babka" <vbabka@suse.cz>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: "Daniel Vetter" <daniel.vetter@ffwll.ch>,
	LKML <linux-kernel@vger.kernel.org>,
	dri-devel@lists.freedesktop.org, mgraesslin@kde.org,
	kwin@kde.org, "Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>
Subject: Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
Date: Thu, 21 Jan 2016 06:31:26 +0100	[thread overview]
Message-ID: <56A06D2E.4000008@gmail.com> (raw)
In-Reply-To: <56A053CE.7000500@daenzer.net>

On 01/21/2016 04:43 AM, Michel Dänzer wrote:
> On 21.01.2016 05:32, Mario Kleiner wrote:
>>
>> So the problem is that AMDs hardware frame counters reset to
>> zero during a modeset. The old DRM code dealt with drivers doing that by
>> keeping vblank irqs enabled during modesets and incrementing vblank
>> count by one during each vblank irq, i think that's what
>> drm_vblank_pre_modeset() and drm_vblank_post_modeset() were meant for.
>
> Right, looks like there's been a regression breaking this. I suspect the
> problem is that vblank->last isn't getting updated from
> drm_vblank_post_modeset. Not sure which change broke that though, or how
> to fix it. Ville?
>

The whole logic has changed and the software counter updates are now 
driven all the time by the hw counter.

>
> BTW, I'm seeing a similar issue with drm_vblank_on/off as well, which
> exposed the bug fixed by 209e4dbc ("drm/vblank: Use u32 consistently for
> vblank counters"). I've been meaning to track that down since then; one
> of these days hopefully, but if anybody has any ideas offhand...
>
>

I spent the last few hours reading through the drm and radeon code and i 
think what should probably work is to replace the 
drm_vblank_pre/post_modeset calls in radeon/amdgpu by drm_vblank_off/on 
calls. These are apparently meant for drivers whose hw counters reset 
during modeset, and seem to reinitialize stuff properly and release 
clients queued vblank events to avoid blocking - not tested so far, just 
looked at the code.

Once drm_vblank_off is called, drm_vblank_get will no-op and return an 
error, so clients can't enable vblank irqs during the modeset - pageflip 
ioctl and waitvblank ioctl would fail while a modeset happens - 
hopefully userspace handles this correctly everywhere.

It would also cause radeons power management to not sync its actions to 
vblank if it would get invoked during a modeset, but that seems to be 
handled by a 200 msec timeout and hopefully only cause visual glitches - 
or invisible glitches while the crtc is blanked during modeset?

There could be another tiny race with the new "vblank counter bumping" 
logic from commit 5b5561b ("drm/radeon: Fixup hw vblank counters/ts 
...") if drm_update_vblank_counter() would be called multiple times in 
quick succession within the "radeon_crtc->lb_vblank_lead_lines" 
scanlines before start of real vblank iff at the same time a modeset 
would happen and set radeon_crtc->lb_vblank_lead_lines to a smaller 
value due to a change in horizontal mode resolution. That needs a 
modeset to happen to a higher horizontal resolution just exactly when 
the scanout is in exactly the right 5 or so scanlines and some client is 
calling drm_vblank_get() to enable vblank irqs at the same time, but it 
would cause the same hang if it happened - not that likely to happen 
often, but still not nice, also Murphy's law... If we could switch to 
drm_vblank_off/on instead of drm_vblank_pre/post_modeset we could remove 
those race as well by forbidding any vblank irq related activity during 
a modeset.

I'll hack up a patch for demonstration now.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2016-01-21  5:31 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-15 10:34 linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon Vlastimil Babka
2016-01-15 12:26 ` Ville Syrjälä
2016-01-15 12:40   ` Vlastimil Babka
2016-01-16  4:24   ` Mario Kleiner
2016-01-16  4:24     ` Mario Kleiner
2016-01-18 10:49     ` Vlastimil Babka
2016-01-18 14:06       ` Vlastimil Babka
2016-01-18 14:14         ` Christian König
2016-01-18 14:14           ` Christian König
2016-01-20 20:25       ` Vlastimil Babka
2016-01-20 20:25         ` Vlastimil Babka
2016-01-20 20:32       ` Mario Kleiner
2016-01-20 20:32         ` Mario Kleiner
2016-01-21  3:43         ` Michel Dänzer
2016-01-21  3:43           ` Michel Dänzer
2016-01-21  5:31           ` Mario Kleiner [this message]
2016-01-21  5:31             ` Mario Kleiner
2016-01-21  6:38             ` Michel Dänzer
2016-01-21  6:38               ` Michel Dänzer
2016-01-21  6:41               ` Michel Dänzer
2016-01-21  6:41                 ` Michel Dänzer
2016-01-21  7:58                 ` Daniel Vetter
2016-01-21  7:58                   ` Daniel Vetter
2016-01-21  8:36                   ` Michel Dänzer
2016-01-21  8:36                     ` Michel Dänzer
2016-01-21 10:09                     ` Daniel Vetter
2016-01-21 10:09                       ` Daniel Vetter
2016-01-22  3:06                       ` Michel Dänzer
2016-01-22  3:06                         ` Michel Dänzer
2016-01-22 15:18                         ` Ville Syrjälä
2016-01-22 15:18                           ` Ville Syrjälä
2016-01-22 18:29                           ` Mario Kleiner
2016-01-22 18:29                             ` Mario Kleiner
2016-01-23 18:23                             ` Mario Kleiner
2016-01-23 18:23                               ` Mario Kleiner
2016-01-25  4:15                           ` Michel Dänzer
2016-01-25  4:15                             ` Michel Dänzer
2016-01-25 13:16                             ` Mario Kleiner
2016-01-25 13:16                               ` Mario Kleiner
2016-01-25 13:23                               ` Ville Syrjälä
2016-01-25 13:44                                 ` Mario Kleiner
2016-01-25 13:44                                   ` Mario Kleiner
2016-01-25 14:53                                   ` Ville Syrjälä
2016-01-25 14:53                                     ` Ville Syrjälä
2016-01-25 16:38                                     ` Mario Kleiner
2016-01-25 18:51                                       ` Daniel Vetter
2016-01-25 18:51                                         ` Daniel Vetter
2016-01-25 19:30                                         ` Mario Kleiner
2016-01-25 19:30                                           ` Mario Kleiner
2016-01-25 20:32                                           ` Daniel Vetter
2016-01-25 20:32                                             ` Daniel Vetter
2016-01-25 21:42                                             ` Mario Kleiner
2016-01-25 21:42                                               ` Mario Kleiner
2016-01-25 22:05                                               ` Daniel Vetter
2016-01-25 22:05                                                 ` Daniel Vetter
2016-01-21  8:28               ` Mario Kleiner
2016-01-21  8:28                 ` Mario Kleiner
2016-01-21  9:15                 ` Vlastimil Babka
2016-01-21  9:15                   ` Vlastimil Babka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56A06D2E.4000008@gmail.com \
    --to=mario.kleiner.de@gmail.com \
    --cc=alexander.deucher@amd.com \
    --cc=christian.koenig@amd.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kwin@kde.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgraesslin@kde.org \
    --cc=michel@daenzer.net \
    --cc=vbabka@suse.cz \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.