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

[Alex Deucher]

On Tue, Mar 6, 2018 at 12:02 PM, Michel Dänzer  wrote:
> From: Michel Dänzer 
>
> Since xf86CursorCloseScreen runs after AMDGPUCloseScreen_KMS,
> PointPriv->spriteFuncs doesn't point to the same struct in the latter as
> in AMDGPUCursorInit_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: 69e20839bfeb ("Keep track of how many SW cursors are visible on
>   each screen")
> (Ported from radeon commit 504b8721b17a672caf1ed3eab087027c02458cab)
>
> Signed-off-by: Michel Dänzer 

Acked-by: Alex Deucher 

> ---
>  src/amdgpu_kms.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
> index e1aae9952..90e1f2531 100644
> --- a/src/amdgpu_kms.c
> +++ b/src/amdgpu_kms.c
> @@ -1551,10 +1551,12 @@ static Bool AMDGPUCursorInit_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))
> @@ -1731,8 +1733,10 @@ static Bool AMDGPUCloseScreen_KMS(ScreenPtr pScreen)
> miPointerScreenPtr PointPriv =
> dixLookupPrivate(>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

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

[Michel Dänzer]

From: Michel Dänzer 

Since xf86CursorCloseScreen runs after AMDGPUCloseScreen_KMS,
PointPriv->spriteFuncs doesn't point to the same struct in the latter as
in AMDGPUCursorInit_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: 69e20839bfeb ("Keep track of how many SW cursors are visible on
  each screen")
(Ported from radeon commit 504b8721b17a672caf1ed3eab087027c02458cab)

Signed-off-by: Michel Dänzer 
---
 src/amdgpu_kms.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index e1aae9952..90e1f2531 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -1551,10 +1551,12 @@ static Bool AMDGPUCursorInit_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))
@@ -1731,8 +1733,10 @@ static Bool AMDGPUCloseScreen_KMS(ScreenPtr pScreen)
miPointerScreenPtr PointPriv =
dixLookupPrivate(>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