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

[Mario Kleiner]

On 03/08/2018 06:00 PM, Michel Dänzer wrote:
> 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.
> 
> 

Works now, as tested on the ati-ddx, can't test on amdgpu-ddx atm. 
Thanks for fixing this quickly!

-mario