From: Mario Kleiner <mario.kleiner.de@gmail.com>
To: "Michel Dänzer" <michel@daenzer.net>,
"Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: "Alex Deucher" <alexander.deucher@amd.com>,
"Vlastimil Babka" <vbabka@suse.cz>,
LKML <linux-kernel@vger.kernel.org>,
dri-devel@lists.freedesktop.org,
"Christian König" <christian.koenig@amd.com>
Subject: Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
Date: Mon, 25 Jan 2016 14:16:45 +0100 [thread overview]
Message-ID: <56A6203D.3030803@gmail.com> (raw)
In-Reply-To: <56A5A171.7000205@daenzer.net>
On 01/25/2016 05:15 AM, Michel Dänzer wrote:
> On 23.01.2016 00:18, Ville Syrjälä wrote:
>> On Fri, Jan 22, 2016 at 12:06:00PM +0900, Michel Dänzer wrote:
>>>
>>> [ Trimming KDE folks from Cc ]
>>>
>>> On 21.01.2016 19:09, Daniel Vetter wrote:
>>>> On Thu, Jan 21, 2016 at 05:36:46PM +0900, Michel Dänzer wrote:
>>>>> On 21.01.2016 16:58, Daniel Vetter wrote:
>>>>>>
>>>>>> Can you please point me at the vblank on/off jump bug please?
>>>>>
>>>>> AFAIR I originally reported it in response to
>>>>> http://lists.freedesktop.org/archives/dri-devel/2015-August/087841.html
>>>>> , but I can't find that in the archives, so maybe that was just on IRC.
>>>>> See
>>>>> http://lists.freedesktop.org/archives/dri-devel/2016-January/099122.html
>>>>> . Basically, I ran into the bug fixed by your patch because the counter
>>>>> jumped forward on every DPMS off, so it hit the 32-bit boundary after
>>>>> just a few days.
>>>>
>>>> Ok, so just uncovered the overflow bug.
>>>
>>> Not sure what you mean by "just", but to be clear: The drm_vblank_on/off
>>> counter jumping bug (similar to the bug this thread is about), which
>>> exposed the overflow bug, is still alive and kicking in 4.5. It seems
>>> to happen when turning off the CRTC:
>>>
>>> [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=218104694, diff=0, hw=916 hw_last=916
>>> [drm:radeon_get_vblank_counter_kms] crtc 0: dist from vblank start 3
>>> [drm:drm_calc_vbltimestamp_from_scanoutpos] crtc 0 : v 0x7 p(2199,-45)@ 7304.307354 -> 7304.308006 [e 0 us, 0 rep]
>>> [drm:radeon_get_vblank_counter_kms] crtc 0: dist from vblank start 3
>>> [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=218104694, diff=16776301, hw=1 hw_last=916
>>
>> Not sure what bug we're talking about here, but here the hw counter
>> clearly jumps backwards.
>>
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3
>>> [drm:drm_update_vblank_count] updating vblank count on crtc 1: current=0, diff=0, hw=0 hw_last=0
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3
>>> [drm:drm_update_vblank_count] updating vblank count on crtc 2: current=0, diff=0, hw=0 hw_last=0
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3
>>> [drm:drm_update_vblank_count] updating vblank count on crtc 3: current=0, diff=0, hw=0 hw_last=0
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 1
>>> [drm:drm_calc_vbltimestamp_from_scanoutpos] crtc 0 : v 0x1 p(0,0)@ 7304.317140 -> 7304.317140 [e 0 us, 0 rep]
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 1
>>> [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=234880995, diff=16777215, hw=0 hw_last=1
>>
>> Same here.
>
> At least one of the jumps is expected, because this is around turning
> off the CRTC for DPMS off. Don't know yet why there are two jumps back
> though.
>
>
>> These things just don't happen on i915 because drm_vblank_off() and
>> drm_vblank_on() are always called around the times when the hw counter
>> might get reset. Or at least that's how it should be.
>
> Which is of course the idea of Daniel's patch (which is what I'm getting
> the above with) or Mario's patch as well, but clearly something's still
> wrong. It's certainly possible that it's something in the driver, but
> since calling drm_vblank_pre/post_modeset from the same places seems to
> work fine (ignoring the regression discussed in this thread)... Do
> drm_vblank_on/off require something else to handle this correctly?
>
>
I suspect it is because vblank_disable_and_save calls
drm_update_vblank_count() unconditionally, even if vblank irqs are
already off.
So on a manual display disable -> reenable you get something like
At disable:
Call to dpms-off --> atombios_crtc_dpms(DPMS_OFF) --> drm_vblank_off ->
vblank_disable_and_save -> irqs off, drm_update_vblank_count() computes
final count.
Then the crtc is shut down and its hw counter resets to zero.
At reenable:
Modesetting -> drm_crtc_helper_set_mode -> crtc_funcs->prepare(crtc) ->
atombios_crtc_prepare() -> atombios_crtc_dpms(DPMS_OFF) ->
drm_vblank_off -> vblank_disable_and_save -> A pointless
drm_update_vblank_count() while the hw counter is already reset to zero
--> Unwanted counter jump.
The problem doesn't happen on a pure modeset to a different video
resolution/refresh rate, as then we only have one call into
atombios_crtc_dpms(DPMS_OFF).
I think the fix is to fix vblank_disable_and_save() to only call
drm_update_vblank_count() if vblank irqs get actually disabled, not on
no-op calls. I will try that now.
Otherwise kms drivers would have to be careful to never call
drm_vblank_off multiple times before calling drm_vblank_on, but the help
text to drm_vblank_on() claims that unbalanced calls to these functions
are perfectly fine.
-mario
WARNING: multiple messages have this Message-ID (diff)
From: Mario Kleiner <mario.kleiner.de@gmail.com>
To: "Michel Dänzer" <michel@daenzer.net>,
"Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: "Alex Deucher" <alexander.deucher@amd.com>,
dri-devel@lists.freedesktop.org,
LKML <linux-kernel@vger.kernel.org>,
"Vlastimil Babka" <vbabka@suse.cz>,
"Christian König" <christian.koenig@amd.com>
Subject: Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
Date: Mon, 25 Jan 2016 14:16:45 +0100 [thread overview]
Message-ID: <56A6203D.3030803@gmail.com> (raw)
In-Reply-To: <56A5A171.7000205@daenzer.net>
On 01/25/2016 05:15 AM, Michel Dänzer wrote:
> On 23.01.2016 00:18, Ville Syrjälä wrote:
>> On Fri, Jan 22, 2016 at 12:06:00PM +0900, Michel Dänzer wrote:
>>>
>>> [ Trimming KDE folks from Cc ]
>>>
>>> On 21.01.2016 19:09, Daniel Vetter wrote:
>>>> On Thu, Jan 21, 2016 at 05:36:46PM +0900, Michel Dänzer wrote:
>>>>> On 21.01.2016 16:58, Daniel Vetter wrote:
>>>>>>
>>>>>> Can you please point me at the vblank on/off jump bug please?
>>>>>
>>>>> AFAIR I originally reported it in response to
>>>>> http://lists.freedesktop.org/archives/dri-devel/2015-August/087841.html
>>>>> , but I can't find that in the archives, so maybe that was just on IRC.
>>>>> See
>>>>> http://lists.freedesktop.org/archives/dri-devel/2016-January/099122.html
>>>>> . Basically, I ran into the bug fixed by your patch because the counter
>>>>> jumped forward on every DPMS off, so it hit the 32-bit boundary after
>>>>> just a few days.
>>>>
>>>> Ok, so just uncovered the overflow bug.
>>>
>>> Not sure what you mean by "just", but to be clear: The drm_vblank_on/off
>>> counter jumping bug (similar to the bug this thread is about), which
>>> exposed the overflow bug, is still alive and kicking in 4.5. It seems
>>> to happen when turning off the CRTC:
>>>
>>> [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=218104694, diff=0, hw=916 hw_last=916
>>> [drm:radeon_get_vblank_counter_kms] crtc 0: dist from vblank start 3
>>> [drm:drm_calc_vbltimestamp_from_scanoutpos] crtc 0 : v 0x7 p(2199,-45)@ 7304.307354 -> 7304.308006 [e 0 us, 0 rep]
>>> [drm:radeon_get_vblank_counter_kms] crtc 0: dist from vblank start 3
>>> [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=218104694, diff=16776301, hw=1 hw_last=916
>>
>> Not sure what bug we're talking about here, but here the hw counter
>> clearly jumps backwards.
>>
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3
>>> [drm:drm_update_vblank_count] updating vblank count on crtc 1: current=0, diff=0, hw=0 hw_last=0
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3
>>> [drm:drm_update_vblank_count] updating vblank count on crtc 2: current=0, diff=0, hw=0 hw_last=0
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3
>>> [drm:drm_update_vblank_count] updating vblank count on crtc 3: current=0, diff=0, hw=0 hw_last=0
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 1
>>> [drm:drm_calc_vbltimestamp_from_scanoutpos] crtc 0 : v 0x1 p(0,0)@ 7304.317140 -> 7304.317140 [e 0 us, 0 rep]
>>> [drm:radeon_get_vblank_counter_kms] Query failed! stat 1
>>> [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=234880995, diff=16777215, hw=0 hw_last=1
>>
>> Same here.
>
> At least one of the jumps is expected, because this is around turning
> off the CRTC for DPMS off. Don't know yet why there are two jumps back
> though.
>
>
>> These things just don't happen on i915 because drm_vblank_off() and
>> drm_vblank_on() are always called around the times when the hw counter
>> might get reset. Or at least that's how it should be.
>
> Which is of course the idea of Daniel's patch (which is what I'm getting
> the above with) or Mario's patch as well, but clearly something's still
> wrong. It's certainly possible that it's something in the driver, but
> since calling drm_vblank_pre/post_modeset from the same places seems to
> work fine (ignoring the regression discussed in this thread)... Do
> drm_vblank_on/off require something else to handle this correctly?
>
>
I suspect it is because vblank_disable_and_save calls
drm_update_vblank_count() unconditionally, even if vblank irqs are
already off.
So on a manual display disable -> reenable you get something like
At disable:
Call to dpms-off --> atombios_crtc_dpms(DPMS_OFF) --> drm_vblank_off ->
vblank_disable_and_save -> irqs off, drm_update_vblank_count() computes
final count.
Then the crtc is shut down and its hw counter resets to zero.
At reenable:
Modesetting -> drm_crtc_helper_set_mode -> crtc_funcs->prepare(crtc) ->
atombios_crtc_prepare() -> atombios_crtc_dpms(DPMS_OFF) ->
drm_vblank_off -> vblank_disable_and_save -> A pointless
drm_update_vblank_count() while the hw counter is already reset to zero
--> Unwanted counter jump.
The problem doesn't happen on a pure modeset to a different video
resolution/refresh rate, as then we only have one call into
atombios_crtc_dpms(DPMS_OFF).
I think the fix is to fix vblank_disable_and_save() to only call
drm_update_vblank_count() if vblank irqs get actually disabled, not on
no-op calls. I will try that now.
Otherwise kms drivers would have to be careful to never call
drm_vblank_off multiple times before calling drm_vblank_on, but the help
text to drm_vblank_on() claims that unbalanced calls to these functions
are perfectly fine.
-mario
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2016-01-25 13:16 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
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 [this message]
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=56A6203D.3030803@gmail.com \
--to=mario.kleiner.de@gmail.com \
--cc=alexander.deucher@amd.com \
--cc=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.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.