[PATCH] drm/ast: Treat AST2600 like AST2500 in most places

[PATCH] drm/ast: Treat AST2600 like AST2500 in most places

[Thomas Zimmermann]

Include AST2600 in most of the branches for AST2500. Thereby revert
most effects of commit f9bd00e0ea9d ("drm/ast: Create chip AST2600").

The AST2600 used to be treated like an AST2500, which at least gave
usable display output. After introducing AST2600 in the driver without
further updates, lots of functions take the wrong branches.

Handling AST2600 in the AST2500 branches reverts back to the original
settings. The exception are cases where AST2600 meanwhile got its own
branch.

Reported-by: Jocelyn Falempe <jfalempe@redhat.com>
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Suggested-by: Jocelyn Falempe <jfalempe@redhat.com>
Fixes: f9bd00e0ea9d ("drm/ast: Create chip AST2600")
Cc: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: dri-devel@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v5.11+
---
 drivers/gpu/drm/ast/ast_main.c | 4 ++--
 drivers/gpu/drm/ast/ast_mode.c | 6 +++---
 drivers/gpu/drm/ast/ast_post.c | 6 +++---
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index d770d5a23c1a..56b2ac138375 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -307,7 +307,7 @@ static int ast_get_dram_info(struct drm_device *dev)
 	default:
 		ast->dram_bus_width = 16;
 		ast->dram_type = AST_DRAM_1Gx16;
-		if (ast->chip == AST2500)
+		if ((ast->chip == AST2500) || (ast->chip == AST2600))
 			ast->mclk = 800;
 		else
 			ast->mclk = 396;
@@ -319,7 +319,7 @@ static int ast_get_dram_info(struct drm_device *dev)
 	else
 		ast->dram_bus_width = 32;
 
-	if (ast->chip == AST2500) {
+	if ((ast->chip == AST2600) || (ast->chip == AST2500)) {
 		switch (mcr_cfg & 0x03) {
 		case 0:
 			ast->dram_type = AST_DRAM_1Gx16;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 323af2746aa9..1dde30b98317 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -310,7 +310,7 @@ static void ast_set_crtc_reg(struct ast_private *ast,
 	u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD = 0, jregAE = 0;
 	u16 temp, precache = 0;
 
-	if ((ast->chip == AST2500) &&
+	if (((ast->chip == AST2600) || (ast->chip == AST2500)) &&
 	    (vbios_mode->enh_table->flags & AST2500PreCatchCRT))
 		precache = 40;
 
@@ -428,7 +428,7 @@ static void ast_set_dclk_reg(struct ast_private *ast,
 {
 	const struct ast_vbios_dclk_info *clk_info;
 
-	if (ast->chip == AST2500)
+	if ((ast->chip == AST2600) || (ast->chip == AST2500))
 		clk_info = &dclk_table_ast2500[vbios_mode->enh_table->dclk_index];
 	else
 		clk_info = &dclk_table[vbios_mode->enh_table->dclk_index];
@@ -476,7 +476,7 @@ static void ast_set_crtthd_reg(struct ast_private *ast)
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0xe0);
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0xa0);
 	} else if (ast->chip == AST2300 || ast->chip == AST2400 ||
-	    ast->chip == AST2500) {
+		   ast->chip == AST2500 || ast->chip == AST2600) {
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0x78);
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0x60);
 	} else if (ast->chip == AST2100 ||
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 0aa9cf0fb5c3..eb1ff9084034 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -80,7 +80,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);
 
 	if (ast->chip == AST2300 || ast->chip == AST2400 ||
-	    ast->chip == AST2500) {
+	    ast->chip == AST2500 || ast->chip == AST2600) {
 		if (pdev->revision >= 0x20)
 			ext_reg_info = extreginfo_ast2300;
 		else
@@ -105,7 +105,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
 	/* Enable RAMDAC for A1 */
 	reg = 0x04;
 	if (ast->chip == AST2300 || ast->chip == AST2400 ||
-	    ast->chip == AST2500)
+	    ast->chip == AST2500 || ast->chip == AST2600)
 		reg |= 0x20;
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6, 0xff, reg);
 }
@@ -382,7 +382,7 @@ void ast_post_gpu(struct drm_device *dev)
 	if (ast->chip == AST2600) {
 		ast_dp_launch(dev, 1);
 	} else if (ast->config_mode == ast_use_p2a) {
-		if (ast->chip == AST2500)
+		if (ast->chip == AST2500 || ast->chip == AST2600)
 			ast_post_chip_2500(dev);
 		else if (ast->chip == AST2300 || ast->chip == AST2400)
 			ast_post_chip_2300(dev);
-- 
2.36.1

Re: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 5062 bytes --]

Jocelyn, do you have a way of getting this patch tested?

Am 07.06.22 um 14:02 schrieb Thomas Zimmermann:
> Include AST2600 in most of the branches for AST2500. Thereby revert
> most effects of commit f9bd00e0ea9d ("drm/ast: Create chip AST2600").
> 
> The AST2600 used to be treated like an AST2500, which at least gave
> usable display output. After introducing AST2600 in the driver without
> further updates, lots of functions take the wrong branches.
> 
> Handling AST2600 in the AST2500 branches reverts back to the original
> settings. The exception are cases where AST2600 meanwhile got its own
> branch.
> 
> Reported-by: Jocelyn Falempe <jfalempe@redhat.com>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Suggested-by: Jocelyn Falempe <jfalempe@redhat.com>
> Fixes: f9bd00e0ea9d ("drm/ast: Create chip AST2600")
> Cc: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v5.11+
> ---
>   drivers/gpu/drm/ast/ast_main.c | 4 ++--
>   drivers/gpu/drm/ast/ast_mode.c | 6 +++---
>   drivers/gpu/drm/ast/ast_post.c | 6 +++---
>   3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index d770d5a23c1a..56b2ac138375 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -307,7 +307,7 @@ static int ast_get_dram_info(struct drm_device *dev)
>   	default:
>   		ast->dram_bus_width = 16;
>   		ast->dram_type = AST_DRAM_1Gx16;
> -		if (ast->chip == AST2500)
> +		if ((ast->chip == AST2500) || (ast->chip == AST2600))
>   			ast->mclk = 800;
>   		else
>   			ast->mclk = 396;
> @@ -319,7 +319,7 @@ static int ast_get_dram_info(struct drm_device *dev)
>   	else
>   		ast->dram_bus_width = 32;
>   
> -	if (ast->chip == AST2500) {
> +	if ((ast->chip == AST2600) || (ast->chip == AST2500)) {
>   		switch (mcr_cfg & 0x03) {
>   		case 0:
>   			ast->dram_type = AST_DRAM_1Gx16;
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 323af2746aa9..1dde30b98317 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -310,7 +310,7 @@ static void ast_set_crtc_reg(struct ast_private *ast,
>   	u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD = 0, jregAE = 0;
>   	u16 temp, precache = 0;
>   
> -	if ((ast->chip == AST2500) &&
> +	if (((ast->chip == AST2600) || (ast->chip == AST2500)) &&
>   	    (vbios_mode->enh_table->flags & AST2500PreCatchCRT))
>   		precache = 40;
>   
> @@ -428,7 +428,7 @@ static void ast_set_dclk_reg(struct ast_private *ast,
>   {
>   	const struct ast_vbios_dclk_info *clk_info;
>   
> -	if (ast->chip == AST2500)
> +	if ((ast->chip == AST2600) || (ast->chip == AST2500))
>   		clk_info = &dclk_table_ast2500[vbios_mode->enh_table->dclk_index];
>   	else
>   		clk_info = &dclk_table[vbios_mode->enh_table->dclk_index];
> @@ -476,7 +476,7 @@ static void ast_set_crtthd_reg(struct ast_private *ast)
>   		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0xe0);
>   		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0xa0);
>   	} else if (ast->chip == AST2300 || ast->chip == AST2400 ||
> -	    ast->chip == AST2500) {
> +		   ast->chip == AST2500 || ast->chip == AST2600) {
>   		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0x78);
>   		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0x60);
>   	} else if (ast->chip == AST2100 ||
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index 0aa9cf0fb5c3..eb1ff9084034 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -80,7 +80,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
>   		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);
>   
>   	if (ast->chip == AST2300 || ast->chip == AST2400 ||
> -	    ast->chip == AST2500) {
> +	    ast->chip == AST2500 || ast->chip == AST2600) {
>   		if (pdev->revision >= 0x20)
>   			ext_reg_info = extreginfo_ast2300;
>   		else
> @@ -105,7 +105,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
>   	/* Enable RAMDAC for A1 */
>   	reg = 0x04;
>   	if (ast->chip == AST2300 || ast->chip == AST2400 ||
> -	    ast->chip == AST2500)
> +	    ast->chip == AST2500 || ast->chip == AST2600)
>   		reg |= 0x20;
>   	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6, 0xff, reg);
>   }
> @@ -382,7 +382,7 @@ void ast_post_gpu(struct drm_device *dev)
>   	if (ast->chip == AST2600) {
>   		ast_dp_launch(dev, 1);
>   	} else if (ast->config_mode == ast_use_p2a) {
> -		if (ast->chip == AST2500)
> +		if (ast->chip == AST2500 || ast->chip == AST2600)
>   			ast_post_chip_2500(dev);
>   		else if (ast->chip == AST2300 || ast->chip == AST2400)
>   			ast_post_chip_2300(dev);

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places

[Jocelyn Falempe]

On 07/06/2022 14:07, Thomas Zimmermann wrote:
> Jocelyn, do you have a way of getting this patch tested?

Thanks for sending this patch.
I'm sorry I'm not able to test it directly.
It's a bit complex to try an upstream kernel on the machine we found the 
regression, I will do my best to get it tested, but no promise.

Also be careful when backporting to stable kernel, as the patch may 
apply, but some other "if (AST2500)" logic might be missing the AST2600 
check.

> 
> Am 07.06.22 um 14:02 schrieb Thomas Zimmermann:
>> Include AST2600 in most of the branches for AST2500. Thereby revert
>> most effects of commit f9bd00e0ea9d ("drm/ast: Create chip AST2600").
>>
>> The AST2600 used to be treated like an AST2500, which at least gave
>> usable display output. After introducing AST2600 in the driver without
>> further updates, lots of functions take the wrong branches.
>>
>> Handling AST2600 in the AST2500 branches reverts back to the original
>> settings. The exception are cases where AST2600 meanwhile got its own
>> branch.
>>
>> Reported-by: Jocelyn Falempe <jfalempe@redhat.com>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Suggested-by: Jocelyn Falempe <jfalempe@redhat.com>
>> Fixes: f9bd00e0ea9d ("drm/ast: Create chip AST2600")
>> Cc: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: <stable@vger.kernel.org> # v5.11+
>> ---
>>   drivers/gpu/drm/ast/ast_main.c | 4 ++--
>>   drivers/gpu/drm/ast/ast_mode.c | 6 +++---
>>   drivers/gpu/drm/ast/ast_post.c | 6 +++---
>>   3 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_main.c 
>> b/drivers/gpu/drm/ast/ast_main.c
>> index d770d5a23c1a..56b2ac138375 100644
>> --- a/drivers/gpu/drm/ast/ast_main.c
>> +++ b/drivers/gpu/drm/ast/ast_main.c
>> @@ -307,7 +307,7 @@ static int ast_get_dram_info(struct drm_device *dev)
>>       default:
>>           ast->dram_bus_width = 16;
>>           ast->dram_type = AST_DRAM_1Gx16;
>> -        if (ast->chip == AST2500)
>> +        if ((ast->chip == AST2500) || (ast->chip == AST2600))
>>               ast->mclk = 800;
>>           else
>>               ast->mclk = 396;
>> @@ -319,7 +319,7 @@ static int ast_get_dram_info(struct drm_device *dev)
>>       else
>>           ast->dram_bus_width = 32;
>> -    if (ast->chip == AST2500) {
>> +    if ((ast->chip == AST2600) || (ast->chip == AST2500)) {
>>           switch (mcr_cfg & 0x03) {
>>           case 0:
>>               ast->dram_type = AST_DRAM_1Gx16;
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c 
>> b/drivers/gpu/drm/ast/ast_mode.c
>> index 323af2746aa9..1dde30b98317 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -310,7 +310,7 @@ static void ast_set_crtc_reg(struct ast_private *ast,
>>       u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD = 0, 
>> jregAE = 0;
>>       u16 temp, precache = 0;
>> -    if ((ast->chip == AST2500) &&
>> +    if (((ast->chip == AST2600) || (ast->chip == AST2500)) &&
>>           (vbios_mode->enh_table->flags & AST2500PreCatchCRT))
>>           precache = 40;
>> @@ -428,7 +428,7 @@ static void ast_set_dclk_reg(struct ast_private *ast,
>>   {
>>       const struct ast_vbios_dclk_info *clk_info;
>> -    if (ast->chip == AST2500)
>> +    if ((ast->chip == AST2600) || (ast->chip == AST2500))
>>           clk_info = 
>> &dclk_table_ast2500[vbios_mode->enh_table->dclk_index];
>>       else
>>           clk_info = &dclk_table[vbios_mode->enh_table->dclk_index];
>> @@ -476,7 +476,7 @@ static void ast_set_crtthd_reg(struct ast_private 
>> *ast)
>>           ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0xe0);
>>           ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0xa0);
>>       } else if (ast->chip == AST2300 || ast->chip == AST2400 ||
>> -        ast->chip == AST2500) {
>> +           ast->chip == AST2500 || ast->chip == AST2600) {
>>           ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0x78);
>>           ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0x60);
>>       } else if (ast->chip == AST2100 ||
>> diff --git a/drivers/gpu/drm/ast/ast_post.c 
>> b/drivers/gpu/drm/ast/ast_post.c
>> index 0aa9cf0fb5c3..eb1ff9084034 100644
>> --- a/drivers/gpu/drm/ast/ast_post.c
>> +++ b/drivers/gpu/drm/ast/ast_post.c
>> @@ -80,7 +80,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
>>           ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);
>>       if (ast->chip == AST2300 || ast->chip == AST2400 ||
>> -        ast->chip == AST2500) {
>> +        ast->chip == AST2500 || ast->chip == AST2600) {
>>           if (pdev->revision >= 0x20)
>>               ext_reg_info = extreginfo_ast2300;
>>           else
>> @@ -105,7 +105,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
>>       /* Enable RAMDAC for A1 */
>>       reg = 0x04;
>>       if (ast->chip == AST2300 || ast->chip == AST2400 ||
>> -        ast->chip == AST2500)
>> +        ast->chip == AST2500 || ast->chip == AST2600)
>>           reg |= 0x20;
>>       ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6, 0xff, reg);
>>   }
>> @@ -382,7 +382,7 @@ void ast_post_gpu(struct drm_device *dev)
>>       if (ast->chip == AST2600) {
>>           ast_dp_launch(dev, 1);
>>       } else if (ast->config_mode == ast_use_p2a) {
>> -        if (ast->chip == AST2500)
>> +        if (ast->chip == AST2500 || ast->chip == AST2600)
>>               ast_post_chip_2500(dev);
>>           else if (ast->chip == AST2300 || ast->chip == AST2400)
>>               ast_post_chip_2300(dev);
> 

RE: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places

[Kuo-Hsiang Chou]

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

Hi Thomas



Thanks for your suggestions!

I answer each revision inline that followed by [KH]:.



Regards,

        Kuo-Hsiang Chou



-----Original Message-----

From: Thomas Zimmermann [mailto:tzimmermann@suse.de]

Sent: Tuesday, June 07, 2022 8:03 PM

To: airlied@redhat.com; airlied@linux.ie; daniel@ffwll.ch; jfalempe@redhat.com; regressions@leemhuis.info; Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>

Subject: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places



Include AST2600 in most of the branches for AST2500. Thereby revert most effects of commit f9bd00e0ea9d ("drm/ast: Create chip AST2600").



The AST2600 used to be treated like an AST2500, which at least gave usable display output. After introducing AST2600 in the driver without further updates, lots of functions take the wrong branches.



Handling AST2600 in the AST2500 branches reverts back to the original settings. The exception are cases where AST2600 meanwhile got its own branch.



[KH]: Based on CVE_2019_6260 item3, P2A is disallowed anymore.

P2A (PCIe to AMBA) is a bridge that is able to revise any BMC registers.

Yes, P2A is dangerous on security issue, because Host open a backdoor and someone malicious SW/APP will be easy to take control of BMC.

Therefore, P2A is disabled forever.



Now, return to this patch, there is no need to add AST2600 condition on the P2A flow.



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

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

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

Fixes: f9bd00e0ea9d ("drm/ast: Create chip AST2600")

Cc: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>

Cc: Dave Airlie <airlied@redhat.com>

Cc: dri-devel@lists.freedesktop.org

Cc: <stable@vger.kernel.org> # v5.11+

---

drivers/gpu/drm/ast/ast_main.c | 4 ++--  drivers/gpu/drm/ast/ast_mode.c | 6 +++---  drivers/gpu/drm/ast/ast_post.c | 6 +++---

3 files changed, 8 insertions(+), 8 deletions(-)



diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index d770d5a23c1a..56b2ac138375 100644

--- a/drivers/gpu/drm/ast/ast_main.c

+++ b/drivers/gpu/drm/ast/ast_main.c

@@ -307,7 +307,7 @@ static int ast_get_dram_info(struct drm_device *dev)

      default:

              ast->dram_bus_width = 16;

              ast->dram_type = AST_DRAM_1Gx16;

-               if (ast->chip == AST2500)

+              if ((ast->chip == AST2500) || (ast->chip == AST2600))

                      ast->mclk = 800;

              else

                      ast->mclk = 396;

@@ -319,7 +319,7 @@ static int ast_get_dram_info(struct drm_device *dev)

      else

              ast->dram_bus_width = 32;

-       if (ast->chip == AST2500) {

+      if ((ast->chip == AST2600) || (ast->chip == AST2500)) {

              switch (mcr_cfg & 0x03) {

              case 0:

                      ast->dram_type = AST_DRAM_1Gx16;

[KH]: P2A is disabled, there is no need to take care of BMC DRAM setting.



diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 323af2746aa9..1dde30b98317 100644

--- a/drivers/gpu/drm/ast/ast_mode.c

+++ b/drivers/gpu/drm/ast/ast_mode.c

@@ -310,7 +310,7 @@ static void ast_set_crtc_reg(struct ast_private *ast,

      u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD = 0, jregAE = 0;

      u16 temp, precache = 0;

-       if ((ast->chip == AST2500) &&

+      if (((ast->chip == AST2600) || (ast->chip == AST2500)) &&

          (vbios_mode->enh_table->flags & AST2500PreCatchCRT))

              precache = 40;

[KH]: after checking on register value, AST2600 doesn't run this.



@@ -428,7 +428,7 @@ static void ast_set_dclk_reg(struct ast_private *ast,  {

      const struct ast_vbios_dclk_info *clk_info;

-       if (ast->chip == AST2500)

+      if ((ast->chip == AST2600) || (ast->chip == AST2500))

              clk_info = &dclk_table_ast2500[vbios_mode->enh_table->dclk_index];

      else

              clk_info = &dclk_table[vbios_mode->enh_table->dclk_index];

[KH]: after checking on register value, AST2600 doesn't run this.

        The difference between 2 table of " dclk_table_ast2500" and " dclk_table" are the setting of Reduce Blanking.



@@ -476,7 +476,7 @@ static void ast_set_crtthd_reg(struct ast_private *ast)

              ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0xe0);

              ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0xa0);

      } else if (ast->chip == AST2300 || ast->chip == AST2400 ||

-           ast->chip == AST2500) {

+                 ast->chip == AST2500 || ast->chip == AST2600) {

              ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0x78);

              ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0x60);

      } else if (ast->chip == AST2100 ||

[KH]: Yes, the patch is "drm/ast: Create threshold values for AST2600" that is the root cause of whites lines on AST2600
commit
bcc77411e8a65929655cef7b63a36000724cdc4b<https://cgit.freedesktop.org/drm/drm/commit/?id=bcc77411e8a65929655cef7b63a36000724cdc4b> (patch<https://cgit.freedesktop.org/drm/drm/patch/?id=bcc77411e8a65929655cef7b63a36000724cdc4b>)





diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c index 0aa9cf0fb5c3..eb1ff9084034 100644

--- a/drivers/gpu/drm/ast/ast_post.c

+++ b/drivers/gpu/drm/ast/ast_post.c

@@ -80,7 +80,7 @@ ast_set_def_ext_reg(struct drm_device *dev)

              ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);

       if (ast->chip == AST2300 || ast->chip == AST2400 ||

-           ast->chip == AST2500) {

+          ast->chip == AST2500 || ast->chip == AST2600) {

              if (pdev->revision >= 0x20)

                      ext_reg_info = extreginfo_ast2300;

              else

[KH]: after checking on register value, AST2600 doesn't run this.



@@ -105,7 +105,7 @@ ast_set_def_ext_reg(struct drm_device *dev)

      /* Enable RAMDAC for A1 */

      reg = 0x04;

      if (ast->chip == AST2300 || ast->chip == AST2400 ||

-           ast->chip == AST2500)

+          ast->chip == AST2500 || ast->chip == AST2600)

              reg |= 0x20;

      ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6, 0xff, reg);  }

[KH]: Yes, it should be a patch, because the value is set by GOP/VBIOS during booting.

ALTHOUGH, after checking on register value, AST2600 doesn't run this.



@@ -382,7 +382,7 @@ void ast_post_gpu(struct drm_device *dev)

      if (ast->chip == AST2600) {

              ast_dp_launch(dev, 1);

      } else if (ast->config_mode == ast_use_p2a) {

-               if (ast->chip == AST2500)

+              if (ast->chip == AST2500 || ast->chip == AST2600)

                      ast_post_chip_2500(dev);

              else if (ast->chip == AST2300 || ast->chip == AST2400)

                      ast_post_chip_2300(dev);

[KH]: NO, AST2600 has its flow that is ast_dp_lauch()

        And "ast_use_p2a" means using P2A bridge that is no longer allowed.





Again, Thanks all who works on drm/ast. Thank you!

Regards,

        Kuo-Hsiang Chou



--

2.36.1



[-- Attachment #2: Type: text/html, Size: 23026 bytes --]

Re: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places

[Jocelyn Falempe]

On 08/06/2022 10:09, Kuo-Hsiang Chou wrote:
> Hi Thomas
> 
> Thanks for your suggestions!
> 
> I answer each revision inline that followed by [KH]:.

Thanks for reviewing this.
> 
> Regards,
> 
>          Kuo-Hsiang Chou
> 
> -----Original Message-----
> 
> From: Thomas Zimmermann [mailto:tzimmermann@suse.de]
> 
> Sent: Tuesday, June 07, 2022 8:03 PM
> 
> To: airlied@redhat.com; airlied@linux.ie; daniel@ffwll.ch; 
> jfalempe@redhat.com; regressions@leemhuis.info; Kuo-Hsiang Chou 
> <kuohsiang_chou@aspeedtech.com>
> 
> Subject: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places
> 
> Include AST2600 in most of the branches for AST2500. Thereby revert most 
> effects of commit f9bd00e0ea9d ("drm/ast: Create chip AST2600").
> 
> The AST2600 used to be treated like an AST2500, which at least gave 
> usable display output. After introducing AST2600 in the driver without 
> further updates, lots of functions take the wrong branches.
> 
> Handling AST2600 in the AST2500 branches reverts back to the original 
> settings. The exception are cases where AST2600 meanwhile got its own 
> branch.
> 
> [KH]: Based on CVE_2019_6260 item3, P2A is disallowed anymore.
> 
> P2A (PCIe to AMBA) is a bridge that is able to revise any BMC registers.
> 
> Yes, P2A is dangerous on security issue, because Host open a backdoor 
> and someone malicious SW/APP will be easy to take control of BMC.
> 
> Therefore, P2A is disabled forever.
> 
> Now, return to this patch, there is no need to add AST2600 condition on 
> the P2A flow.
> 

[snip]
> 
> [KH]: Yes, the patch is "drm/ast: Create threshold values for AST2600" 
> that is the root cause of whites lines on AST2600
> 
> commit
> 
> 
> bcc77411e8a65929655cef7b63a36000724cdc4b 
> <https://cgit.freedesktop.org/drm/drm/commit/?id=bcc77411e8a65929655cef7b63a36000724cdc4b> (patch 
> <https://cgit.freedesktop.org/drm/drm/patch/?id=bcc77411e8a65929655cef7b63a36000724cdc4b>)
> 


So basically this commit should be enough to fix the white lines  and 
flickering with VGA output on AST2600 ?
I will try to have it tested, and if it's good, we may want to have it 
on stable kernel.

Best regards,

-- 

Jocelyn

RE: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places

[Kuo-Hsiang Chou]

Hi Jocelyn Falempe,

-----Original Message-----
From: Jocelyn Falempe [mailto:jfalempe@redhat.com] 
Sent: Wednesday, June 08, 2022 9:17 PM
To: Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>; Thomas Zimmermann <tzimmermann@suse.de>; airlied@redhat.com; airlied@linux.ie; daniel@ffwll.ch; regressions@leemhuis.info
Cc: dri-devel@lists.freedesktop.org; stable@vger.kernel.org; Luke Chen <luke_chen@aspeedtech.com>; Hungju Huang <hungju_huang@aspeedtech.com>; Charles Kuan <charles_kuan@aspeedtech.com>
Subject: Re: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places

On 08/06/2022 10:09, Kuo-Hsiang Chou wrote:
> Hi Thomas
> 
> Thanks for your suggestions!
> 
> I answer each revision inline that followed by [KH]:.

Thanks for reviewing this.
> 
> Regards,
> 
>          Kuo-Hsiang Chou
> 
> -----Original Message-----
> 
> From: Thomas Zimmermann [mailto:tzimmermann@suse.de]
> 
> Sent: Tuesday, June 07, 2022 8:03 PM
> 
> To: airlied@redhat.com; airlied@linux.ie; daniel@ffwll.ch; 
> jfalempe@redhat.com; regressions@leemhuis.info; Kuo-Hsiang Chou 
> <kuohsiang_chou@aspeedtech.com>
> 
> Subject: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places
> 
> Include AST2600 in most of the branches for AST2500. Thereby revert 
> most effects of commit f9bd00e0ea9d ("drm/ast: Create chip AST2600").
> 
> The AST2600 used to be treated like an AST2500, which at least gave 
> usable display output. After introducing AST2600 in the driver without 
> further updates, lots of functions take the wrong branches.
> 
> Handling AST2600 in the AST2500 branches reverts back to the original 
> settings. The exception are cases where AST2600 meanwhile got its own 
> branch.
> 
> [KH]: Based on CVE_2019_6260 item3, P2A is disallowed anymore.
> 
> P2A (PCIe to AMBA) is a bridge that is able to revise any BMC registers.
> 
> Yes, P2A is dangerous on security issue, because Host open a backdoor 
> and someone malicious SW/APP will be easy to take control of BMC.
> 
> Therefore, P2A is disabled forever.
> 
> Now, return to this patch, there is no need to add AST2600 condition 
> on the P2A flow.
> 

[snip]
> 
> [KH]: Yes, the patch is "drm/ast: Create threshold values for AST2600" 
> that is the root cause of whites lines on AST2600
> 
> commit
> 
> 
> bcc77411e8a65929655cef7b63a36000724cdc4b
> <https://cgit.freedesktop.org/drm/drm/commit/?id=bcc77411e8a65929655ce
> f7b63a36000724cdc4b> (patch
> <https://cgit.freedesktop.org/drm/drm/patch/?id=bcc77411e8a65929655cef
> 7b63a36000724cdc4b>)
> 


So basically this commit should be enough to fix the white lines  and flickering with VGA output on AST2600 ?
[KH]: Yes. 
	You are welcome to tell me something if you consider there is other strange issue.
	Thanks for your efforts on drm/ast project!
Regards,
	Kuo-Hsiang Chou

I will try to have it tested, and if it's good, we may want to have it on stable kernel.

Best regards,

-- 

Jocelyn

Re: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places

[Jocelyn Falempe]

On 09/06/2022 04:32, Kuo-Hsiang Chou wrote:
> Hi Jocelyn Falempe,
> 
> -----Original Message-----
> From: Jocelyn Falempe [mailto:jfalempe@redhat.com]
> Sent: Wednesday, June 08, 2022 9:17 PM
> To: Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>; Thomas Zimmermann <tzimmermann@suse.de>; airlied@redhat.com; airlied@linux.ie; daniel@ffwll.ch; regressions@leemhuis.info
> Cc: dri-devel@lists.freedesktop.org; stable@vger.kernel.org; Luke Chen <luke_chen@aspeedtech.com>; Hungju Huang <hungju_huang@aspeedtech.com>; Charles Kuan <charles_kuan@aspeedtech.com>
> Subject: Re: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places
> 
> On 08/06/2022 10:09, Kuo-Hsiang Chou wrote:
>> Hi Thomas
>>
>> Thanks for your suggestions!
>>
>> I answer each revision inline that followed by [KH]:.
> 
> Thanks for reviewing this.
>>
>> Regards,
>>
>>           Kuo-Hsiang Chou
>>
>> -----Original Message-----
>>
>> From: Thomas Zimmermann [mailto:tzimmermann@suse.de]
>>
>> Sent: Tuesday, June 07, 2022 8:03 PM
>>
>> To: airlied@redhat.com; airlied@linux.ie; daniel@ffwll.ch;
>> jfalempe@redhat.com; regressions@leemhuis.info; Kuo-Hsiang Chou
>> <kuohsiang_chou@aspeedtech.com>
>>
>> Subject: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places
>>
>> Include AST2600 in most of the branches for AST2500. Thereby revert
>> most effects of commit f9bd00e0ea9d ("drm/ast: Create chip AST2600").
>>
>> The AST2600 used to be treated like an AST2500, which at least gave
>> usable display output. After introducing AST2600 in the driver without
>> further updates, lots of functions take the wrong branches.
>>
>> Handling AST2600 in the AST2500 branches reverts back to the original
>> settings. The exception are cases where AST2600 meanwhile got its own
>> branch.
>>
>> [KH]: Based on CVE_2019_6260 item3, P2A is disallowed anymore.
>>
>> P2A (PCIe to AMBA) is a bridge that is able to revise any BMC registers.
>>
>> Yes, P2A is dangerous on security issue, because Host open a backdoor
>> and someone malicious SW/APP will be easy to take control of BMC.
>>
>> Therefore, P2A is disabled forever.
>>
>> Now, return to this patch, there is no need to add AST2600 condition
>> on the P2A flow.
>>
> 
> [snip]
>>
>> [KH]: Yes, the patch is "drm/ast: Create threshold values for AST2600"
>> that is the root cause of whites lines on AST2600
>>
>> commit
>>
>>
>> bcc77411e8a65929655cef7b63a36000724cdc4b
>> <https://cgit.freedesktop.org/drm/drm/commit/?id=bcc77411e8a65929655ce
>> f7b63a36000724cdc4b> (patch
>> <https://cgit.freedesktop.org/drm/drm/patch/?id=bcc77411e8a65929655cef
>> 7b63a36000724cdc4b>)
>>
> 
> 
> So basically this commit should be enough to fix the white lines  and flickering with VGA output on AST2600 ?
> [KH]: Yes.
> 	You are welcome to tell me something if you consider there is other strange issue.
> 	Thanks for your efforts on drm/ast project!
> Regards,
> 	Kuo-Hsiang Chou

I've got confirmation that this commit is enough to fix the issue 
introduced with f9bd00e0ea9d.

So let's drop this patch, and submit bcc77411e8a6 "drm/ast: Create 
threshold values for AST2600" to stable kernel instead.

Thanks,

> 
> I will try to have it tested, and if it's good, we may want to have it on stable kernel.
> 
> Best regards,
> 

Re: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3829 bytes --]



Am 09.06.22 um 09:06 schrieb Jocelyn Falempe:
> On 09/06/2022 04:32, Kuo-Hsiang Chou wrote:
>> Hi Jocelyn Falempe,
>>
>> -----Original Message-----
>> From: Jocelyn Falempe [mailto:jfalempe@redhat.com]
>> Sent: Wednesday, June 08, 2022 9:17 PM
>> To: Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>; Thomas Zimmermann 
>> <tzimmermann@suse.de>; airlied@redhat.com; airlied@linux.ie; 
>> daniel@ffwll.ch; regressions@leemhuis.info
>> Cc: dri-devel@lists.freedesktop.org; stable@vger.kernel.org; Luke Chen 
>> <luke_chen@aspeedtech.com>; Hungju Huang 
>> <hungju_huang@aspeedtech.com>; Charles Kuan <charles_kuan@aspeedtech.com>
>> Subject: Re: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places
>>
>> On 08/06/2022 10:09, Kuo-Hsiang Chou wrote:
>>> Hi Thomas
>>>
>>> Thanks for your suggestions!
>>>
>>> I answer each revision inline that followed by [KH]:.
>>
>> Thanks for reviewing this.
>>>
>>> Regards,
>>>
>>>           Kuo-Hsiang Chou
>>>
>>> -----Original Message-----
>>>
>>> From: Thomas Zimmermann [mailto:tzimmermann@suse.de]
>>>
>>> Sent: Tuesday, June 07, 2022 8:03 PM
>>>
>>> To: airlied@redhat.com; airlied@linux.ie; daniel@ffwll.ch;
>>> jfalempe@redhat.com; regressions@leemhuis.info; Kuo-Hsiang Chou
>>> <kuohsiang_chou@aspeedtech.com>
>>>
>>> Subject: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places
>>>
>>> Include AST2600 in most of the branches for AST2500. Thereby revert
>>> most effects of commit f9bd00e0ea9d ("drm/ast: Create chip AST2600").
>>>
>>> The AST2600 used to be treated like an AST2500, which at least gave
>>> usable display output. After introducing AST2600 in the driver without
>>> further updates, lots of functions take the wrong branches.
>>>
>>> Handling AST2600 in the AST2500 branches reverts back to the original
>>> settings. The exception are cases where AST2600 meanwhile got its own
>>> branch.
>>>
>>> [KH]: Based on CVE_2019_6260 item3, P2A is disallowed anymore.
>>>
>>> P2A (PCIe to AMBA) is a bridge that is able to revise any BMC registers.
>>>
>>> Yes, P2A is dangerous on security issue, because Host open a backdoor
>>> and someone malicious SW/APP will be easy to take control of BMC.
>>>
>>> Therefore, P2A is disabled forever.
>>>
>>> Now, return to this patch, there is no need to add AST2600 condition
>>> on the P2A flow.
>>>
>>
>> [snip]
>>>
>>> [KH]: Yes, the patch is "drm/ast: Create threshold values for AST2600"
>>> that is the root cause of whites lines on AST2600
>>>
>>> commit
>>>
>>>
>>> bcc77411e8a65929655cef7b63a36000724cdc4b
>>> <https://cgit.freedesktop.org/drm/drm/commit/?id=bcc77411e8a65929655ce
>>> f7b63a36000724cdc4b> (patch
>>> <https://cgit.freedesktop.org/drm/drm/patch/?id=bcc77411e8a65929655cef
>>> 7b63a36000724cdc4b>)
>>>
>>
>>
>> So basically this commit should be enough to fix the white lines  and 
>> flickering with VGA output on AST2600 ?
>> [KH]: Yes.
>>     You are welcome to tell me something if you consider there is 
>> other strange issue.
>>     Thanks for your efforts on drm/ast project!
>> Regards,
>>     Kuo-Hsiang Chou
> 
> I've got confirmation that this commit is enough to fix the issue 
> introduced with f9bd00e0ea9d.
> 
> So let's drop this patch, and submit bcc77411e8a6 "drm/ast: Create 
> threshold values for AST2600" to stable kernel instead.

Thanks to both of you for your help with this bug.

Best regards
Thomas

> 
> Thanks,
> 
>>
>> I will try to have it tested, and if it's good, we may want to have it 
>> on stable kernel.
>>
>> Best regards,
>>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]