Re: PROBLEM: AST2500 BMC video output disabled by reboot (regression)

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2049 bytes --]

Hi

Am 04.10.25 um 02:31 schrieb Nick Bowler:
> On Wed, Oct 01, 2025 at 09:26:28AM +0200, Thomas Zimmermann wrote:
>> Am 18.09.25 um 13:17 schrieb Thomas Zimmermann:
>>> Am 18.09.25 um 04:04 schrieb Nick Bowler:
>>>> On Wed, Sep 17, 2025 at 11:14:45AM -0400, Nick Bowler wrote:
>>>>> On Fri, Aug 29, 2025 at 03:07:14PM +0200, Thomas Zimmermann wrote:
>>>>>> The ast driver doesn't do much during shutdown. Could you
>>>>>> please out-comment the lines at either [2] xor [3] and report
>>>>>> on either effect?
> [...]
>>>>>> [2] https://elixir.bootlin.com/linux/v6.16.3/source/drivers/gpu/drm/ast/ast_mode.c#L835
>>>>>> [3] https://elixir.bootlin.com/linux/v6.16.3/source/drivers/gpu/drm/ast/ast_mode.c#L839
> [...]
>>>>     - Deleting [3] (only) appears sufficient to make things work again,
>>>>       that is, deleting the following line in ast_mode.c:
>>>>
>>>>         ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xb6, 0xfc, vgacrb6);
>>> Please test if the attached patch fixes the problem for you.
>> Have you been able to test the patch?
> In the normal scenario where everything is working and I reboot, then
> the display remains on for the firmware (this would seem to be an
> improvement).
>
> But it introduces a new problem: the screen no longer turns back on if
> I boot the patched kernel from the "display off" state.  The unpatched
> 6.17 kernel would at least turn the display back on from this.
> Furthermore, rebooting from this state keeps the display off.
>
> The earlier change [3] above has no such problems.

Thanks again for testing. Looks like your BMC is especially picky about 
these settings. :)

Attached are two patches; each trying to eliminate one of the possible 
causes. Could you please test them individually and report the results?

Best regards
Thomas

>
> Thanks,
>    Nick

-- 
--
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)


[-- Attachment #2: 0001-ast-Use-VGACR17-sync-enable-and-VGACRB6-sync-off.patch --]
[-- Type: text/x-patch, Size: 2573 bytes --]

From 3ea91762ad077f49143ae6cbf48020386a2ecc65 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Thu, 18 Sep 2025 09:50:28 +0200
Subject: [PATCH] ast: Use VGACR17 sync enable and VGACRB6 sync off

Blank the display by disabling sync pulses with VGACR17<7>. Some
BMC's don't handle VGACRB6 correctly. And don't modify VGASR1's SD
bit, which only disables GPU access to VRAM.
---
 drivers/gpu/drm/ast/ast_mode.c | 18 ++++++++++--------
 drivers/gpu/drm/ast/ast_reg.h  |  1 +
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 6b9d510c509d..fe8089266db5 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -836,22 +836,24 @@ ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
 static void ast_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state)
 {
 	struct ast_device *ast = to_ast_device(crtc->dev);
+	u8 vgacr17 = 0x00;
+	u8 vgacrb6 = 0x00;
 
-	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xb6, 0xfc, 0x00);
-	ast_set_index_reg_mask(ast, AST_IO_VGASRI, 0x01, 0xdf, 0x00);
+	vgacr17 |= AST_IO_VGACR17_SYNC_ENABLE;
+	vgacrb6 &= ~(AST_IO_VGACRB6_VSYNC_OFF | AST_IO_VGACRB6_HSYNC_OFF);
+
+	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0x17, 0x7f, vgacr17);
+	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xb6, 0xfc, vgacrb6);
 }
 
 static void ast_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *state)
 {
 	struct drm_crtc_state *old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc);
 	struct ast_device *ast = to_ast_device(crtc->dev);
-	u8 vgacrb6;
+	u8 vgacr17 = 0xff;
 
-	ast_set_index_reg_mask(ast, AST_IO_VGASRI, 0x01, 0xdf, AST_IO_VGASR1_SD);
-
-	vgacrb6 = AST_IO_VGACRB6_VSYNC_OFF |
-		  AST_IO_VGACRB6_HSYNC_OFF;
-	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xb6, 0xfc, vgacrb6);
+	vgacr17 &= ~AST_IO_VGACR17_SYNC_ENABLE;
+	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0x17, 0x7f, vgacr17);
 
 	/*
 	 * HW cursors require the underlying primary plane and CRTC to
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index e15adaf3a80e..30578e3b07e4 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -29,6 +29,7 @@
 #define AST_IO_VGAGRI			(0x4E)
 
 #define AST_IO_VGACRI			(0x54)
+#define AST_IO_VGACR17_SYNC_ENABLE	BIT(7) /* called "Hardware reset" in docs */
 #define AST_IO_VGACR80_PASSWORD		(0xa8)
 #define AST_IO_VGACR99_VGAMEM_RSRV_MASK	GENMASK(1, 0)
 #define AST_IO_VGACRA1_VGAIO_DISABLED	BIT(1)
-- 
2.51.0


[-- Attachment #3: 0001-ast-Use-VGACR17-sync-enable-and-VGASR01-screen-disab.patch --]
[-- Type: text/x-patch, Size: 2545 bytes --]

From 67e9cbb66be47fd2aeadd2ad91fe15e076340140 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Thu, 18 Sep 2025 09:50:28 +0200
Subject: [PATCH] ast: Use VGACR17 sync enable and VGASR01 screen disable

Blank the display by disabling sync pulses with VGACR17<7>. Some
BMC's don't handle VGACRB6 correctly. Keep the change to VGASR1's
SD bit, as some BMCs disable GPU access to VRAM.
---
 drivers/gpu/drm/ast/ast_mode.c | 18 ++++++++++--------
 drivers/gpu/drm/ast/ast_reg.h  |  1 +
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 6b9d510c509d..8e3d56798e9e 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -836,22 +836,24 @@ ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
 static void ast_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state)
 {
 	struct ast_device *ast = to_ast_device(crtc->dev);
+	u8 vgacr17 = 0x00;
+	u8 vgasr01 = 0x00;
 
-	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xb6, 0xfc, 0x00);
-	ast_set_index_reg_mask(ast, AST_IO_VGASRI, 0x01, 0xdf, 0x00);
+	vgacr17 |= AST_IO_VGACR17_SYNC_ENABLE;
+	vgasr01 &= ~AST_IO_VGASR1_SD;
+
+	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0x17, 0x7f, vgacr17);
+	ast_set_index_reg_mask(ast, AST_IO_VGASRI, 0x01, 0xdf, vgasr01);
 }
 
 static void ast_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *state)
 {
 	struct drm_crtc_state *old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc);
 	struct ast_device *ast = to_ast_device(crtc->dev);
-	u8 vgacrb6;
-
-	ast_set_index_reg_mask(ast, AST_IO_VGASRI, 0x01, 0xdf, AST_IO_VGASR1_SD);
+	u8 vgacr17 = 0xff;
 
-	vgacrb6 = AST_IO_VGACRB6_VSYNC_OFF |
-		  AST_IO_VGACRB6_HSYNC_OFF;
-	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xb6, 0xfc, vgacrb6);
+	vgacr17 &= ~AST_IO_VGACR17_SYNC_ENABLE;
+	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0x17, 0x7f, vgacr17);
 
 	/*
 	 * HW cursors require the underlying primary plane and CRTC to
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index e15adaf3a80e..30578e3b07e4 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -29,6 +29,7 @@
 #define AST_IO_VGAGRI			(0x4E)
 
 #define AST_IO_VGACRI			(0x54)
+#define AST_IO_VGACR17_SYNC_ENABLE	BIT(7) /* called "Hardware reset" in docs */
 #define AST_IO_VGACR80_PASSWORD		(0xa8)
 #define AST_IO_VGACR99_VGAMEM_RSRV_MASK	GENMASK(1, 0)
 #define AST_IO_VGACRA1_VGAIO_DISABLED	BIT(1)
-- 
2.51.0