[PATCH xf86-video-ati] Only change Set/MoveCursor hooks from what we expect

[Michel Dänzer]

On 2018-03-07 10:33 AM, Mario Kleiner wrote:
> On 03/07/2018 09:50 AM, Michel Dänzer wrote:
>> On 2018-03-07 04:45 AM, Mario Kleiner wrote:
>>> On 03/05/2018 10:56 PM, Alex Deucher wrote:
>>>> On Mon, Mar 5, 2018 at 12:44 PM, Michel Dänzer <michel at daenzer.net>
>>>> wrote:
>>>>> From: Michel Dänzer <michel.daenzer at amd.com>
>>>>>
>>>>> Since xf86CursorCloseScreen runs after RADEONCloseScreen_KMS,
>>>>> PointPriv->spriteFuncs doesn't point to the same struct in the
>>>>> latter as
>>>>> in RADEONCursorInit_KMS. So we were restoring info->Set/MoveCursor to
>>>>> the wrong struct. Then in the next server generation,
>>>>> info->Set/MoveCursor would end up pointing to
>>>>> drmmode_sprite_set/move_cursor, resulting in an infinite loop if
>>>>> one of
>>>>> them was called.
>>>>>
>>>>> To avoid this, only change the Set/MoveCursor hooks if their values
>>>>> match our expectations, otherwise leave them as is. This is kind of a
>>>>> hack, but the alternative would be invasive and thus risky changes to
>>>>> the way we're wrapping CloseScreen, and it's not even clear that can
>>>>> work without changing xserver code.
>>>>>
>>>>> Fixes: 1fe8ca75974c ("Keep track of how many SW cursors are visible on
>>>>>                         each screen")
>>>>> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
>>>>
>>>> Acked-by: Alex Deucher <alexander.deucher at amd.com>
>>>>
>>>
>>> Nope, not quite, unfortunately. Tested against x-server master, mesa
>>> master, ati-ddx master, with sddm login manager. With a freshly started
>>> server, now on a dual-x-screen setup, instead of an infinite loop, i get
>>> a server crash as soon as i move the mouse cursor from X-Screen 0 to
>>> X-Screen 1:
>>
>> Well, that's not the same issue I was seeing after all. I'll take
>> another look.
> 
> Bedtime here, but fwiw from some debug statements i added, it seems as
> if on dual-x-screen init, x-screen 1 somehow inherits the
> PointPriv->spriteFuncs->SetCursor = drmmode_sprite_set_cursor assignment
> done on x-screen 0, and therefore already has it ==
> drmmode_sprite_set_cursor, so during RADEONCursorInit_KMS for x-screen 1
> the assignments get skipped, which leaves the info->SetCursor for
> x-screen 1 == NULL. Therefore cursor update for x-screen 1 = boom!

Yeah, I was already suspecting something like that.


I've pushed fixes for this to both drivers. If you're still seeing an
issue with those, I need to know by early next week, otherwise I may not
be able to make releases with a fix before April.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer