[PATCH] drm/ast: Fix soft lockup

[PATCH] drm/ast: Fix soft lockup

[Jammy Huang]

Avoid infinite-loop in ast_dp_set_on_off().

Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
 drivers/gpu/drm/ast/ast_dp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
index ebb6d8ebd44e..1e9259416980 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -180,6 +180,7 @@ void ast_dp_set_on_off(struct drm_device *dev, bool on)
 {
 	struct ast_device *ast = to_ast_device(dev);
 	u8 video_on_off = on;
+	u32 i = 0;
 
 	// Video On/Off
 	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE3, (u8) ~AST_DP_VIDEO_ENABLE, on);
@@ -192,6 +193,8 @@ void ast_dp_set_on_off(struct drm_device *dev, bool on)
 						ASTDP_MIRROR_VIDEO_ENABLE) != video_on_off) {
 			// wait 1 ms
 			mdelay(1);
+			if (++i > 200)
+				break;
 		}
 	}
 }

base-commit: b0546776ad3f332e215cebc0b063ba4351971cca
-- 
2.25.1

Re: [PATCH] drm/ast: Fix soft lockup

[Jocelyn Falempe]

Hi,

Thanks for your patch.
I'm wondering how you can trigger this infinite loop ?

Also this looks like a simple fix, that can be easily backported, so I'm 
adding stable in Cc.

If Thomas has no objections, I can push it to drm-misc-fixes.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

-- 

Jocelyn

On 25/03/2024 04:35, Jammy Huang wrote:
> Avoid infinite-loop in ast_dp_set_on_off().
> 
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
>   drivers/gpu/drm/ast/ast_dp.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
> index ebb6d8ebd44e..1e9259416980 100644
> --- a/drivers/gpu/drm/ast/ast_dp.c
> +++ b/drivers/gpu/drm/ast/ast_dp.c
> @@ -180,6 +180,7 @@ void ast_dp_set_on_off(struct drm_device *dev, bool on)
>   {
>   	struct ast_device *ast = to_ast_device(dev);
>   	u8 video_on_off = on;
> +	u32 i = 0;
>   
>   	// Video On/Off
>   	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE3, (u8) ~AST_DP_VIDEO_ENABLE, on);
> @@ -192,6 +193,8 @@ void ast_dp_set_on_off(struct drm_device *dev, bool on)
>   						ASTDP_MIRROR_VIDEO_ENABLE) != video_on_off) {
>   			// wait 1 ms
>   			mdelay(1);
> +			if (++i > 200)
> +				break;
>   		}
>   	}
>   }
> 
> base-commit: b0546776ad3f332e215cebc0b063ba4351971cca

Re: [PATCH] drm/ast: Fix soft lockup

[Thomas Zimmermann]

Hi

Am 27.03.24 um 09:53 schrieb Jocelyn Falempe:
> Hi,
>
> Thanks for your patch.
> I'm wondering how you can trigger this infinite loop ?

Yeah, a bit more context for this bug would be welcome. It's hard to 
judge the fix without.

Best regards
Thomas

>
> Also this looks like a simple fix, that can be easily backported, so 
> I'm adding stable in Cc.
>
> If Thomas has no objections, I can push it to drm-misc-fixes.
>
> Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
>



-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH] drm/ast: Fix soft lockup

[黃立銘]

Hi Thomas and Jocelyn,

What we do in ast_dp_set_on_off() is a handshake between host driver
and bmc-fw to confirm
the operation, on/off, is completed.

We use some scratch registers in bmc to handshake with host. This
handshake only work if
BMC's scu-lock is opened. If scu-lock is opened too late, then it
could lead to this issue.

Best regards
Jammy

Thomas Zimmermann <tzimmermann@suse.de> 於 2024年3月28日 週四 上午4:28寫道：
>
> Hi
>
> Am 27.03.24 um 09:53 schrieb Jocelyn Falempe:
> > Hi,
> >
> > Thanks for your patch.
> > I'm wondering how you can trigger this infinite loop ?
>
> Yeah, a bit more context for this bug would be welcome. It's hard to
> judge the fix without.
>
> Best regards
> Thomas
>
> >
> > Also this looks like a simple fix, that can be easily backported, so
> > I'm adding stable in Cc.
> >
> > If Thomas has no objections, I can push it to drm-misc-fixes.
> >
> > Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
> >
>
>
>
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
>

Re: [PATCH] drm/ast: Fix soft lockup

[Thomas Zimmermann]

Hi

Am 01.04.24 um 04:20 schrieb 黃立銘:
> Hi Thomas and Jocelyn,
>
> What we do in ast_dp_set_on_off() is a handshake between host driver
> and bmc-fw to confirm
> the operation, on/off, is completed.
>
> We use some scratch registers in bmc to handshake with host. This
> handshake only work if
> BMC's scu-lock is opened. If scu-lock is opened too late, then it
> could lead to this issue.

Thanks a lot. Can you please send a new version of this patch with this 
information in the commit message? Please also mention why 200 ms is a 
good upper limit.

The code currently waits and then possibly breaks the loop. Should the 
if-branch be located before the mdelay() statement to avoid any 
unnecessary waiting?

Please also send the patch from your Aspeed email address. Our scripts 
do not accept patches where the sender differs from the Signed-off-by tag.

Best regards
Thomas

>
> Best regards
> Jammy
>
> Thomas Zimmermann <tzimmermann@suse.de> 於 2024年3月28日 週四 上午4:28寫道：
>> Hi
>>
>> Am 27.03.24 um 09:53 schrieb Jocelyn Falempe:
>>> Hi,
>>>
>>> Thanks for your patch.
>>> I'm wondering how you can trigger this infinite loop ?
>> Yeah, a bit more context for this bug would be welcome. It's hard to
>> judge the fix without.
>>
>> Best regards
>> Thomas
>>
>>> Also this looks like a simple fix, that can be easily backported, so
>>> I'm adding stable in Cc.
>>>
>>> If Thomas has no objections, I can push it to drm-misc-fixes.
>>>
>>> Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
>>>
>>
>>
>> --
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)