[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAKMK7uEtCOfT5orgZ9qegb-Nzr8QiAnsNX8a9f75j0pJPwtmrg@mail.gmail.com>
Date: Mon, 25 Jan 2016 23:05:35 +0100
From: Daniel Vetter <daniel@...ll.ch>
To: Mario Kleiner <mario.kleiner.de@...il.com>
Cc: Ville Syrjälä <ville.syrjala@...ux.intel.com>,
Michel Dänzer <michel@...nzer.net>,
Alex Deucher <alexander.deucher@....com>,
Vlastimil Babka <vbabka@...e.cz>,
LKML <linux-kernel@...r.kernel.org>,
dri-devel <dri-devel@...ts.freedesktop.org>,
Christian König <christian.koenig@....com>
Subject: Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
On Mon, Jan 25, 2016 at 10:42 PM, Mario Kleiner
<mario.kleiner.de@...il.com> wrote:
>>
>>> Now the patch i want to try next to fix the drm_vblank_pre/post_modeset
>>> regression in Linux 4.4/4.5 is to add a ...
>>>
>>> if ((diff > 1) && vblank->inmodeset) diff = 1;
>>>
>>> ... to the bottom of drm_update_vblank_count(). That should hopefully
>>> restore the pre/post_modeset behavior as close to the original behavior
>>> as
>>> possible. As a side effect it would also prevent the counter jump caused
>>> by
>>> redundant calls to drm_vblank_off().
>>
>>
>> Hm, can we just frob pre/post_modeset only with some checks? I'd like to
>> not put that kind of "I have no idea about my hw state" hacks into the new
>> helpers. Otherwise not even atomic drivers can start to gain WARN_ONs to
>> enforce correct usage, which would be a real bummer imo.
>> -Daniel
>>
>
> We could check for only (vblank->inmodeset & 0x2) to only apply it to the
> legacy pre/post path, trusting that the drm_vblank_off/on path will be made
> robust in a different way, e.g., by the stuff discussed above and careful
> implementation in each kms driver that uses those. Atm. radeon doesn't use
> off/on, so your enablement patch set can make sure it does the right thing
> from the beginning.
>
> rockchip-kms may need similar treatment to radeon to avoid redundant calls.
>
> Btw. how the patch to drm_update_vblank_count() close to the bottom would
> actually look is more like:
>
> if ((diff > 1) &&
> ((vblank->inmodeset & 0x2) || (flags & DRM_CALLED_FROM_VBLIRQ)))
> diff = 1;
Yeah I think that should work as a short-term fix for radoen. When you
do that, can you pls do a second patch to give the magic 0x2 and 0x1
some meaning? Otherwise this is super-hard to understand code ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Powered by blists - more mailing lists