* [PATCH xf86-video-ati] Only change Set/MoveCursor hooks from what we expect
@ 2018-03-05 17:44 Michel Dänzer
[not found] ` <20180305174459.18765-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Michel Dänzer @ 2018-03-05 17:44 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Mario Kleiner
From: Michel Dänzer <michel.daenzer@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@amd.com>
---
src/radeon_kms.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/src/radeon_kms.c b/src/radeon_kms.c
index 85390e306..790d4be16 100644
--- a/src/radeon_kms.c
+++ b/src/radeon_kms.c
@@ -2017,10 +2017,12 @@ static Bool RADEONCursorInit_KMS(ScreenPtr pScreen)
return FALSE;
}
- info->SetCursor = PointPriv->spriteFuncs->SetCursor;
- info->MoveCursor = PointPriv->spriteFuncs->MoveCursor;
- PointPriv->spriteFuncs->SetCursor = drmmode_sprite_set_cursor;
- PointPriv->spriteFuncs->MoveCursor = drmmode_sprite_move_cursor;
+ if (PointPriv->spriteFuncs->SetCursor != drmmode_sprite_set_cursor) {
+ info->SetCursor = PointPriv->spriteFuncs->SetCursor;
+ info->MoveCursor = PointPriv->spriteFuncs->MoveCursor;
+ PointPriv->spriteFuncs->SetCursor = drmmode_sprite_set_cursor;
+ PointPriv->spriteFuncs->MoveCursor = drmmode_sprite_move_cursor;
+ }
}
if (xf86ReturnOptValBool(info->Options, OPTION_SW_CURSOR, FALSE))
@@ -2184,8 +2186,10 @@ static Bool RADEONCloseScreen_KMS(ScreenPtr pScreen)
miPointerScreenPtr PointPriv =
dixLookupPrivate(&pScreen->devPrivates, miPointerScreenKey);
- PointPriv->spriteFuncs->SetCursor = info->SetCursor;
- PointPriv->spriteFuncs->MoveCursor = info->MoveCursor;
+ if (PointPriv->spriteFuncs->SetCursor == drmmode_sprite_set_cursor) {
+ PointPriv->spriteFuncs->SetCursor = info->SetCursor;
+ PointPriv->spriteFuncs->MoveCursor = info->MoveCursor;
+ }
}
pScreen->BlockHandler = info->BlockHandler;
--
2.16.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH xf86-video-ati] Only change Set/MoveCursor hooks from what we expect
[not found] ` <20180305174459.18765-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org>
@ 2018-03-05 21:56 ` Alex Deucher
[not found] ` <CADnq5_Me0zYe3sLJ3j8kkr7mw6X6nQQCUHzVgYsXh=u9FtGvnw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Alex Deucher @ 2018-03-05 21:56 UTC (permalink / raw)
To: Michel Dänzer; +Cc: Mario Kleiner, amd-gfx list
On Mon, Mar 5, 2018 at 12:44 PM, Michel Dänzer <michel@daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer@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@amd.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> src/radeon_kms.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/src/radeon_kms.c b/src/radeon_kms.c
> index 85390e306..790d4be16 100644
> --- a/src/radeon_kms.c
> +++ b/src/radeon_kms.c
> @@ -2017,10 +2017,12 @@ static Bool RADEONCursorInit_KMS(ScreenPtr pScreen)
> return FALSE;
> }
>
> - info->SetCursor = PointPriv->spriteFuncs->SetCursor;
> - info->MoveCursor = PointPriv->spriteFuncs->MoveCursor;
> - PointPriv->spriteFuncs->SetCursor = drmmode_sprite_set_cursor;
> - PointPriv->spriteFuncs->MoveCursor = drmmode_sprite_move_cursor;
> + if (PointPriv->spriteFuncs->SetCursor != drmmode_sprite_set_cursor) {
> + info->SetCursor = PointPriv->spriteFuncs->SetCursor;
> + info->MoveCursor = PointPriv->spriteFuncs->MoveCursor;
> + PointPriv->spriteFuncs->SetCursor = drmmode_sprite_set_cursor;
> + PointPriv->spriteFuncs->MoveCursor = drmmode_sprite_move_cursor;
> + }
> }
>
> if (xf86ReturnOptValBool(info->Options, OPTION_SW_CURSOR, FALSE))
> @@ -2184,8 +2186,10 @@ static Bool RADEONCloseScreen_KMS(ScreenPtr pScreen)
> miPointerScreenPtr PointPriv =
> dixLookupPrivate(&pScreen->devPrivates, miPointerScreenKey);
>
> - PointPriv->spriteFuncs->SetCursor = info->SetCursor;
> - PointPriv->spriteFuncs->MoveCursor = info->MoveCursor;
> + if (PointPriv->spriteFuncs->SetCursor == drmmode_sprite_set_cursor) {
> + PointPriv->spriteFuncs->SetCursor = info->SetCursor;
> + PointPriv->spriteFuncs->MoveCursor = info->MoveCursor;
> + }
> }
>
> pScreen->BlockHandler = info->BlockHandler;
> --
> 2.16.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH xf86-video-ati] Only change Set/MoveCursor hooks from what we expect
[not found] ` <CADnq5_Me0zYe3sLJ3j8kkr7mw6X6nQQCUHzVgYsXh=u9FtGvnw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2018-03-07 3:45 ` Mario Kleiner
[not found] ` <aac0c084-812e-5994-bea8-0690ce90ede8-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Mario Kleiner @ 2018-03-07 3:45 UTC (permalink / raw)
To: Alex Deucher, Michel Dänzer; +Cc: amd-gfx list
On 03/05/2018 10:56 PM, Alex Deucher wrote:
> On Mon, Mar 5, 2018 at 12:44 PM, Michel Dänzer <michel@daenzer.net> wrote:
>> From: Michel Dänzer <michel.daenzer@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@amd.com>
>
> Acked-by: Alex Deucher <alexander.deucher@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:
(gdb) c
Continuing.
Thread 1 "X" received signal SIGSEGV, Segmentation fault.
0x0000000000000000 in ?? ()
(gdb) bt
#0 0x0000000000000000 in ?? ()
#1 0x00000000004bcad2 in xf86CursorSetCursor (pDev=0x11912e0,
pScreen=0xf9a510, pCurs=0x1177610, x=32, y=358) at xf86CursorRD.c:345
#2 0x000000000057a758 in miPointerUpdateSprite (pDev=0x11912e0) at
mipointer.c:453
#3 0x000000000057aada in miPointerDisplayCursor (pDev=0x11912e0,
pScreen=0xf9a510, pCursor=0x1177610) at mipointer.c:206
#4 0x00000000004cb1b6 in CursorDisplayCursor (pDev=<optimized out>,
pScreen=0xf9a510, pCursor=0x1177610) at cursor.c:168
#5 0x000000000050f78b in AnimCurDisplayCursor (pDev=0x11912e0,
pScreen=0xf9a510, pCursor=0x1177610) at animcur.c:196
#6 0x00000000004473b8 in ChangeToCursor (pDev=0x11912e0,
cursor=0x1177610) at events.c:936
#7 0x000000000044b20a in CheckMotion (ev=ev@entry=0x7ffdad7a7060,
pDev=pDev@entry=0x11912e0) at events.c:3081
#8 0x000000000051ee6b in ProcessDeviceEvent
(ev=ev@entry=0x7ffdad7a7060, device=device@entry=0x11912e0) at
exevents.c:1716
#9 0x000000000051f5fb in ProcessOtherEvent (ev=0x7ffdad7a7060,
device=0x11912e0) at exevents.c:1873
#10 0x00000000005403a2 in ProcessPointerEvent (ev=0x7ffdad7a7060,
mouse=0x11912e0) at xkbAccessX.c:756
#11 0x0000000000571712 in mieqProcessDeviceEvent (dev=0x1368e10,
event=0x7ffdad7a7cc0, screen=0xf9a510) at mieq.c:496
#12 0x000000000057184a in mieqProcessInputEvents () at mieq.c:551
#13 0x000000000047a8a9 in ProcessInputEvents () at xf86Events.c:151
#14 0x000000000043e2a7 in Dispatch () at dispatch.c:417
#15 0x0000000000442568 in dix_main (argc=11, argv=0x7ffdad7a8ad8,
envp=<optimized out>) at main.c:276
#16 0x00007f20fdf60830 in __libc_start_main (main=0x42c520 <main>,
argc=11, argv=0x7ffdad7a8ad8, init=<optimized out>, fini=<optimized
out>, rtld_fini=<optimized out>, stack_end=0x7ffdad7a8ac8) at
../csu/libc-start.c:291
-mario
>> ---
>> src/radeon_kms.c | 16 ++++++++++------
>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/radeon_kms.c b/src/radeon_kms.c
>> index 85390e306..790d4be16 100644
>> --- a/src/radeon_kms.c
>> +++ b/src/radeon_kms.c
>> @@ -2017,10 +2017,12 @@ static Bool RADEONCursorInit_KMS(ScreenPtr pScreen)
>> return FALSE;
>> }
>>
>> - info->SetCursor = PointPriv->spriteFuncs->SetCursor;
>> - info->MoveCursor = PointPriv->spriteFuncs->MoveCursor;
>> - PointPriv->spriteFuncs->SetCursor = drmmode_sprite_set_cursor;
>> - PointPriv->spriteFuncs->MoveCursor = drmmode_sprite_move_cursor;
>> + if (PointPriv->spriteFuncs->SetCursor != drmmode_sprite_set_cursor) {
>> + info->SetCursor = PointPriv->spriteFuncs->SetCursor;
>> + info->MoveCursor = PointPriv->spriteFuncs->MoveCursor;
>> + PointPriv->spriteFuncs->SetCursor = drmmode_sprite_set_cursor;
>> + PointPriv->spriteFuncs->MoveCursor = drmmode_sprite_move_cursor;
>> + }
>> }
>>
>> if (xf86ReturnOptValBool(info->Options, OPTION_SW_CURSOR, FALSE))
>> @@ -2184,8 +2186,10 @@ static Bool RADEONCloseScreen_KMS(ScreenPtr pScreen)
>> miPointerScreenPtr PointPriv =
>> dixLookupPrivate(&pScreen->devPrivates, miPointerScreenKey);
>>
>> - PointPriv->spriteFuncs->SetCursor = info->SetCursor;
>> - PointPriv->spriteFuncs->MoveCursor = info->MoveCursor;
>> + if (PointPriv->spriteFuncs->SetCursor == drmmode_sprite_set_cursor) {
>> + PointPriv->spriteFuncs->SetCursor = info->SetCursor;
>> + PointPriv->spriteFuncs->MoveCursor = info->MoveCursor;
>> + }
>> }
>>
>> pScreen->BlockHandler = info->BlockHandler;
>> --
>> 2.16.2
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH xf86-video-ati] Only change Set/MoveCursor hooks from what we expect
[not found] ` <aac0c084-812e-5994-bea8-0690ce90ede8-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-03-07 8:50 ` Michel Dänzer
[not found] ` <5d491770-6af9-687e-c387-e95f70ad9d68-otUistvHUpPR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Michel Dänzer @ 2018-03-07 8:50 UTC (permalink / raw)
To: Mario Kleiner, Alex Deucher; +Cc: amd-gfx list
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@daenzer.net>
>> wrote:
>>> From: Michel Dänzer <michel.daenzer@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@amd.com>
>>
>> Acked-by: Alex Deucher <alexander.deucher@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.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH xf86-video-ati] Only change Set/MoveCursor hooks from what we expect
[not found] ` <5d491770-6af9-687e-c387-e95f70ad9d68-otUistvHUpPR7s880joybQ@public.gmane.org>
@ 2018-03-07 9:33 ` Mario Kleiner
[not found] ` <fddd98e1-f560-3892-8fa3-21a239425021-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Mario Kleiner @ 2018-03-07 9:33 UTC (permalink / raw)
To: Michel Dänzer, Alex Deucher; +Cc: amd-gfx list
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@daenzer.net>
>>> wrote:
>>>> From: Michel Dänzer <michel.daenzer@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@amd.com>
>>>
>>> Acked-by: Alex Deucher <alexander.deucher@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!
-mario
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH xf86-video-ati] Only change Set/MoveCursor hooks from what we expect
[not found] ` <fddd98e1-f560-3892-8fa3-21a239425021-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-03-08 17:00 ` Michel Dänzer
[not found] ` <42976fb8-c1c3-f685-1426-d3ff164dc062-otUistvHUpPR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Michel Dänzer @ 2018-03-08 17:00 UTC (permalink / raw)
To: Mario Kleiner; +Cc: amd-gfx list
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@daenzer.net>
>>>> wrote:
>>>>> From: Michel Dänzer <michel.daenzer@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@amd.com>
>>>>
>>>> Acked-by: Alex Deucher <alexander.deucher@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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH xf86-video-ati] Only change Set/MoveCursor hooks from what we expect
[not found] ` <42976fb8-c1c3-f685-1426-d3ff164dc062-otUistvHUpPR7s880joybQ@public.gmane.org>
@ 2018-03-10 18:54 ` Mario Kleiner
0 siblings, 0 replies; 7+ messages in thread
From: Mario Kleiner @ 2018-03-10 18:54 UTC (permalink / raw)
To: Michel Dänzer; +Cc: amd-gfx list
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@daenzer.net>
>>>>> wrote:
>>>>>> From: Michel Dänzer <michel.daenzer@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@amd.com>
>>>>>
>>>>> Acked-by: Alex Deucher <alexander.deucher@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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-10 18:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-05 17:44 [PATCH xf86-video-ati] Only change Set/MoveCursor hooks from what we expect Michel Dänzer
[not found] ` <20180305174459.18765-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-03-05 21:56 ` Alex Deucher
[not found] ` <CADnq5_Me0zYe3sLJ3j8kkr7mw6X6nQQCUHzVgYsXh=u9FtGvnw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-03-07 3:45 ` Mario Kleiner
[not found] ` <aac0c084-812e-5994-bea8-0690ce90ede8-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-03-07 8:50 ` Michel Dänzer
[not found] ` <5d491770-6af9-687e-c387-e95f70ad9d68-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-03-07 9:33 ` Mario Kleiner
[not found] ` <fddd98e1-f560-3892-8fa3-21a239425021-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-03-08 17:00 ` Michel Dänzer
[not found] ` <42976fb8-c1c3-f685-1426-d3ff164dc062-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-03-10 18:54 ` Mario Kleiner
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.