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

2022-06-08 Thread Kuo-Hsiang Chou
Hi Jocelyn Falempe,

-Original Message-
From: Jocelyn Falempe [mailto:jfale...@redhat.com] 
Sent: Wednesday, June 08, 2022 9:17 PM
To: Kuo-Hsiang Chou ; Thomas Zimmermann 
; airl...@redhat.com; airl...@linux.ie; dan...@ffwll.ch; 
regressi...@leemhuis.info
Cc: dri-devel@lists.freedesktop.org; sta...@vger.kernel.org; Luke Chen 
; Hungju Huang ; Charles 
Kuan 
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:tzimmerm...@suse.de]
> 
> Sent: Tuesday, June 07, 2022 8:03 PM
> 
> To: airl...@redhat.com; airl...@linux.ie; dan...@ffwll.ch; 
> jfale...@redhat.com; regressi...@leemhuis.info; Kuo-Hsiang Chou 
> 
> 
> 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

2022-06-08 Thread Kuo-Hsiang Chou
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:tzimmerm...@suse.de]

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

To: airl...@redhat.com; airl...@linux.ie; dan...@ffwll.ch; jfale...@redhat.com; 
regressi...@leemhuis.info; Kuo-Hsiang Chou 

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 

Signed-off-by: Thomas Zimmermann 

Suggested-by: Jocelyn Falempe 

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

Cc: KuoHsiang Chou 

Cc: Dave Airlie 

Cc: dri-devel@lists.freedesktop.org

Cc:  # 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/comm

RE: [PATCH v4] drm/ast: Create the driver for ASPEED proprietory Display-Port

2022-05-29 Thread Kuo-Hsiang Chou
Hi Thomas,

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Kuo-Hsiang Chou
Sent: Friday, May 13, 2022 6:39 PM
To: Thomas Zimmermann ; dri-devel@lists.freedesktop.org; 
linux-ker...@vger.kernel.org
Subject: RE: [PATCH v4] drm/ast: Create the driver for ASPEED proprietory 
Display-Port

Hi Thomas,

-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Friday, May 13, 2022 6:21 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH v4] drm/ast: Create the driver for ASPEED proprietory 
Display-Port


Hi

Am 13.05.22 um 11:07 schrieb Kuo-Hsiang Chou:
> 
> 
> -Original Message-
> From: Thomas Zimmermann [mailto:tzimmerm...@suse.de]
> Sent: Tuesday, May 10, 2022 6:56 PM
> To: Kuo-Hsiang Chou ; 
> dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH v4] drm/ast: Create the driver for ASPEED 
> proprietory Display-Port
> 
> Hi
> 
> Am 04.05.22 um 10:49 schrieb Kuo-Hsiang Chou:
>> Hi Thomas,
>>
>> Thanks for your efforts to review this patch.
>>
>> Now, I observe a change that after DP unplugged and then the system 
>> is unable to get EDID from D-sub connecting.
>>
>> The reason seems that TXs are merged into union structure in 
>> /drivers/gpu/drm/ast/ast_drv.h
>>    (a59b026419f33040d7d28b8e3b1cea681b9ce7a7
>> <https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a59b026419f3304
>> 0
>> d7d28b8e3b1cea681b9ce7a7>)
> 
> I have posted a patch that enables multiple parallel outputs. See
> 
>   
> <https://lore.kernel.org/dri-devel/20220510105010.20712-1-tzimmermann@
> suse.de/T/#u>
> 
> If you have the time, I'd appreciate if you could test it.
> 
> Hi Thomas,
> First, thanks for your efforts on this patch of " enables multiple parallel 
> outputs ".
> But it doesn't work. There is still NO EDID got from D-sub after DP unplugged.
> 
> Base on your patch, I am trying to find out the solution also.
> Final, thanks for your help again!

Let's try to find a solution. I might be able to come up with something if I 
have enough information.

How are these outputs connected to each each other?
1. DP and D-sub both connected: it is correct to get EDID and monitor type is 
correctly shown on display setting. Resolution is up to 1920*1200.
2. DP unplugged and only D-sub connected: Unknown Display is shown on display 
setting. Only 800*600 and 1024*768 are available for Resolution.

They are both served by the same CRTC. Can they be used at the same time?  
No, 2 different monitors are connected at same time. 
One is connected by D-sub and another is connected by DP connecting.

Are they mutually exclusive?
Yes, If DP and D-sub are all connecting, DP's EDID is first priority for 
Display setting.
When DP unplugged, EDID got from D-sub are used for Display setting.

Now, I try to add the codes of ast_vga_connector_helper_get_modes() into 
ast_astdp_connector_helper_get_modes().
The result is failed.
Anyway, I will continue to try and test the feature next Monday. 
Thanks for your great help!

Hi Thomas,
Your patch is workable to switch resolution on Single Display option of Display 
setting.

And I test resolution switching on Single Display between ASTDP and VGA.
Yes, the left-top number indicates the current resolution decided by ASTDP or 
VGA.

I also try to change resolution on Mirror and Join Display option.
But there is on Apply button shown in the right-top corner of Display setting.

Thanks for your help!
Regards,
Kuo-Hsiang Chou

Regards,
Kuo-Hsiang Chou

Best regards
Thomas

> 
> Regards,
>   Kuo-Hsiang Chou
> 
> Best regards
> Thomas
> 
>>
>> Another, do you need the ast2600 EVB to ease verification on "drm/ast" ?
>>
>> Regards,
>>
>>       Kuo-Hsiang Chou
>>
>> -Original Message-
>> From: Thomas Zimmermann [mailto:tzimmerm...@suse.de]
>> Sent: Wednesday, May 04, 2022 3:28 PM
>> To: Kuo-Hsiang Chou ; 
>> dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
>> Subject: Re: [PATCH v4] drm/ast: Create the driver for ASPEED 
>> proprietory Display-Port
>>
>> Hi
>>
>> Am 28.04.22 um 09:56 schrieb KuoHsiang Chou:
>>
>>> V1:
>>
>>> 1. The MCU FW controling ASPEED DP is loaded by BMC boot loader.
>>
>>> 2. Driver starts after CR[3:1] == 111b that indicates Tx is ASTDP,
>>
>>>   and CRD1[5] has been asserted by BMVC boot loader.
>>
>>> 3. EDID is prioritized by DP monitor.
>>
>>> 4. DP's EDID has high priority to decide resolution supporting.
>>
>>>

RE: [PATCH v4] drm/ast: Create the driver for ASPEED proprietory Display-Port

2022-05-13 Thread Kuo-Hsiang Chou
Hi Thomas,

-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Friday, May 13, 2022 6:21 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH v4] drm/ast: Create the driver for ASPEED proprietory 
Display-Port


Hi

Am 13.05.22 um 11:07 schrieb Kuo-Hsiang Chou:
> 
> 
> -Original Message-
> From: Thomas Zimmermann [mailto:tzimmerm...@suse.de]
> Sent: Tuesday, May 10, 2022 6:56 PM
> To: Kuo-Hsiang Chou ; 
> dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH v4] drm/ast: Create the driver for ASPEED 
> proprietory Display-Port
> 
> Hi
> 
> Am 04.05.22 um 10:49 schrieb Kuo-Hsiang Chou:
>> Hi Thomas,
>>
>> Thanks for your efforts to review this patch.
>>
>> Now, I observe a change that after DP unplugged and then the system 
>> is unable to get EDID from D-sub connecting.
>>
>> The reason seems that TXs are merged into union structure in 
>> /drivers/gpu/drm/ast/ast_drv.h
>>    (a59b026419f33040d7d28b8e3b1cea681b9ce7a7
>> <https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a59b026419f3304
>> 0
>> d7d28b8e3b1cea681b9ce7a7>)
> 
> I have posted a patch that enables multiple parallel outputs. See
> 
>   
> <https://lore.kernel.org/dri-devel/20220510105010.20712-1-tzimmermann@
> suse.de/T/#u>
> 
> If you have the time, I'd appreciate if you could test it.
> 
> Hi Thomas,
> First, thanks for your efforts on this patch of " enables multiple parallel 
> outputs ".
> But it doesn't work. There is still NO EDID got from D-sub after DP unplugged.
> 
> Base on your patch, I am trying to find out the solution also.
> Final, thanks for your help again!

Let's try to find a solution. I might be able to come up with something if I 
have enough information.

How are these outputs connected to each each other?
1. DP and D-sub both connected: it is correct to get EDID and monitor type is 
correctly shown on display setting. Resolution is up to 1920*1200.
2. DP unplugged and only D-sub connected: Unknown Display is shown on display 
setting. Only 800*600 and 1024*768 are available for Resolution.

They are both served by the same CRTC. Can they be used at the same time?  
No, 2 different monitors are connected at same time. 
One is connected by D-sub and another is connected by DP connecting.

Are they mutually exclusive?
Yes, If DP and D-sub are all connecting, DP's EDID is first priority for 
Display setting.
When DP unplugged, EDID got from D-sub are used for Display setting.

Now, I try to add the codes of ast_vga_connector_helper_get_modes() into 
ast_astdp_connector_helper_get_modes().
The result is failed.
Anyway, I will continue to try and test the feature next Monday. 
Thanks for your great help!

Regards,
Kuo-Hsiang Chou

Best regards
Thomas

> 
> Regards,
>   Kuo-Hsiang Chou
> 
> Best regards
> Thomas
> 
>>
>> Another, do you need the ast2600 EVB to ease verification on "drm/ast" ?
>>
>> Regards,
>>
>>       Kuo-Hsiang Chou
>>
>> -Original Message-
>> From: Thomas Zimmermann [mailto:tzimmerm...@suse.de]
>> Sent: Wednesday, May 04, 2022 3:28 PM
>> To: Kuo-Hsiang Chou ; 
>> dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
>> Subject: Re: [PATCH v4] drm/ast: Create the driver for ASPEED 
>> proprietory Display-Port
>>
>> Hi
>>
>> Am 28.04.22 um 09:56 schrieb KuoHsiang Chou:
>>
>>> V1:
>>
>>> 1. The MCU FW controling ASPEED DP is loaded by BMC boot loader.
>>
>>> 2. Driver starts after CR[3:1] == 111b that indicates Tx is ASTDP,
>>
>>>   and CRD1[5] has been asserted by BMVC boot loader.
>>
>>> 3. EDID is prioritized by DP monitor.
>>
>>> 4. DP's EDID has high priority to decide resolution supporting.
>>
>>>
>>
>>> V2:
>>
>>> Modules description:
>>
>>> 1. ASTDP (ASPEED DisplayPort) is controlled by dedicated
>>
>>>   AST-MCU (ASPEED propriatary MCU).
>>
>>> 2. MCU is looping in charged of HPD, Read EDID, Link Training with
>>
>>>   DP sink.
>>
>>> 3. ASTDP and AST-MUC reside in BMC (Baseboard Management controller)
>>
>>>   addressing-space.
>>
>>> 4. ASPEED DRM driver requests MCU to get HPD and EDID by 
>>> CR-scratched
>>
>>>   register.
>>
>>>
>>
>>> Booting sequence:
>>
>>> 1. Check if TX is ASTDP    //
>>> ast_dp_launch()
>>
>

RE: [PATCH v4] drm/ast: Create the driver for ASPEED proprietory Display-Port

2022-05-13 Thread Kuo-Hsiang Chou


-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Tuesday, May 10, 2022 6:56 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH v4] drm/ast: Create the driver for ASPEED proprietory 
Display-Port

Hi

Am 04.05.22 um 10:49 schrieb Kuo-Hsiang Chou:
> Hi Thomas,
> 
> Thanks for your efforts to review this patch.
> 
> Now, I observe a change that after DP unplugged and then the system is 
> unable to get EDID from D-sub connecting.
> 
> The reason seems that TXs are merged into union structure in 
> /drivers/gpu/drm/ast/ast_drv.h
>   (a59b026419f33040d7d28b8e3b1cea681b9ce7a7
> <https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a59b026419f33040
> d7d28b8e3b1cea681b9ce7a7>)

I have posted a patch that enables multiple parallel outputs. See

 
<https://lore.kernel.org/dri-devel/20220510105010.20712-1-tzimmerm...@suse.de/T/#u>

If you have the time, I'd appreciate if you could test it.

Hi Thomas,
First, thanks for your efforts on this patch of " enables multiple parallel 
outputs ".
But it doesn't work. There is still NO EDID got from D-sub after DP unplugged.

Base on your patch, I am trying to find out the solution also.
Final, thanks for your help again!

Regards,
Kuo-Hsiang Chou

Best regards
Thomas

> 
> Another, do you need the ast2600 EVB to ease verification on "drm/ast" ?
> 
> Regards,
> 
>      Kuo-Hsiang Chou
> 
> -Original Message-
> From: Thomas Zimmermann [mailto:tzimmerm...@suse.de]
> Sent: Wednesday, May 04, 2022 3:28 PM
> To: Kuo-Hsiang Chou ; 
> dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH v4] drm/ast: Create the driver for ASPEED 
> proprietory Display-Port
> 
> Hi
> 
> Am 28.04.22 um 09:56 schrieb KuoHsiang Chou:
> 
>> V1:
> 
>> 1. The MCU FW controling ASPEED DP is loaded by BMC boot loader.
> 
>> 2. Driver starts after CR[3:1] == 111b that indicates Tx is ASTDP,
> 
>> and CRD1[5] has been asserted by BMVC boot loader.
> 
>> 3. EDID is prioritized by DP monitor.
> 
>> 4. DP's EDID has high priority to decide resolution supporting.
> 
>> 
> 
>> V2:
> 
>> Modules description:
> 
>> 1. ASTDP (ASPEED DisplayPort) is controlled by dedicated
> 
>> AST-MCU (ASPEED propriatary MCU).
> 
>> 2. MCU is looping in charged of HPD, Read EDID, Link Training with
> 
>> DP sink.
> 
>> 3. ASTDP and AST-MUC reside in BMC (Baseboard Management controller)
> 
>> addressing-space.
> 
>> 4. ASPEED DRM driver requests MCU to get HPD and EDID by CR-scratched
> 
>> register.
> 
>> 
> 
>> Booting sequence:
> 
>> 1. Check if TX is ASTDP    // 
>> ast_dp_launch()
> 
>> 2. Check if DP-MCU FW has loaded    
>> // ast_dp_launch()
> 
>> 3. Read EDID    // 
>> ast_dp_read_edid()
> 
>> 4. Resolution switch // 
>> ast_dp_SetOutput()
> 
>> 
> 
>> V3:
> 
>> 1. Remove unneeded semicolon.
> 
>> 2. Apply to git://anongit.freedesktop.org/drm/drm, instead of
> 
>> git://anongit.freedesktop.org/drm/drm-misc
> 
>> 3. Resolve auto build test WARNINGs on V1 patch.
> 
>> 
> 
>> V4:
> 
>> 1. Sync code-base with kernel 5.17_rc6 2. Remove the define of
> 
>> DPControlPower, because DP chips need to be
> 
>> powered on to be used.
> 
>> 3. Remove the switches of PHY and Display from EDID procedure.
> 
>> 4. Revise increaing delay to fixed delay, because this version kernel
> 
>> doesn't detect minitor consistenntly.
> 
>> 5. Create clean-up code used for reset of power state on errors with
> 
>> -EIO manner.
> 
>> 6. Revise the DP detection by TX type and its DP-FW status during
> 
>> booting and resume.
> 
>> 7. Correct the CamelCase Style.
> 
>> 8. Use register reading while needing, and remove to hold full
> 
>> register.
> 
>> 9. Instead of 'u8', revise to 'bool' on swwitch of PHY and video.
> 
>> 10.Correct typo
> 
>> 11.Remove the duplicated copy of TX definition.
> 
>> 12.Use EDID_LENGTH as the constant of 128.
> 
>> 
> 
>> Signed-off-by: KuoHsiang Chou > <mailto:kuohsiang_c...@aspeedtech.com>>
> 
> Reviewed-by: Thomas Zimmermann  <mailto:tzimmerm...@suse.de>>
> 
> I've meanwhile added your patch to drm-misc-next. It should show up in 
>

RE: [PATCH v4] drm/ast: Create the driver for ASPEED proprietory Display-Port

2022-05-04 Thread Kuo-Hsiang Chou
Hi Luke,

Please help on the EVB request that DRM reviewer, Thomas, needs one AST2600 EVB 
to ease Display-Port verification on ASPEED DRM diver. 
Thanks

Regards,
Kuo-Hsiang Chou

-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Wednesday, May 04, 2022 5:38 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH v4] drm/ast: Create the driver for ASPEED proprietory 
Display-Port

Hi Thomas,

Hi

Am 04.05.22 um 10:49 schrieb Kuo-Hsiang Chou:
> Hi Thomas,
> 
> Thanks for your efforts to review this patch.
> 
> Now, I observe a change that after DP unplugged and then the system is 
> unable to get EDID from D-sub connecting.
> 
> The reason seems that TXs are merged into union structure in 
> /drivers/gpu/drm/ast/ast_drv.h
>   (a59b026419f33040d7d28b8e3b1cea681b9ce7a7
> <https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a59b026419f33040
> d7d28b8e3b1cea681b9ce7a7>)

The test system has a DP and a VGA adapter?  We need create two separate 
connectors and encoders in that case.  I'll try to provide a patch in the next 
days.

Yes, the same content are displayed (as mirror mode) on DP and VGA monitor as 
same time.

In previous drm/ast version, the EDID read from DP is first priority for 
resolution validation.
If DP is detected as un-connected, and then reading EDID from VGA is used for 
the second run of resolution validation.
Finally, if both DP and VGA are detected as un-connected, only 1024*768 and 
800*600 are able to be selected as default resolution.

> 
> Another, do you need the ast2600 EVB to ease verification on "drm/ast" ?

The EVB is an evaluation board? 
Yes. the EVB is developed to clarify the differences between custom's CRB.
The EVB is connect to Host by PCI-e 1-Lane adapter.

I'd be happy to have one of these. How can I get one?
I forward the request to ASPEED VP about EVB delivery to you.

Best regards
Thomas

> 
> Regards,
> 
>      Kuo-Hsiang Chou
> 
> -Original Message-
> From: Thomas Zimmermann [mailto:tzimmerm...@suse.de]
> Sent: Wednesday, May 04, 2022 3:28 PM
> To: Kuo-Hsiang Chou ; 
> dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH v4] drm/ast: Create the driver for ASPEED 
> proprietory Display-Port
> 
> Hi
> 
> Am 28.04.22 um 09:56 schrieb KuoHsiang Chou:
> 
>> V1:
> 
>> 1. The MCU FW controling ASPEED DP is loaded by BMC boot loader.
> 
>> 2. Driver starts after CR[3:1] == 111b that indicates Tx is ASTDP,
> 
>> and CRD1[5] has been asserted by BMVC boot loader.
> 
>> 3. EDID is prioritized by DP monitor.
> 
>> 4. DP's EDID has high priority to decide resolution supporting.
> 
>> 
> 
>> V2:
> 
>> Modules description:
> 
>> 1. ASTDP (ASPEED DisplayPort) is controlled by dedicated
> 
>> AST-MCU (ASPEED propriatary MCU).
> 
>> 2. MCU is looping in charged of HPD, Read EDID, Link Training with
> 
>> DP sink.
> 
>> 3. ASTDP and AST-MUC reside in BMC (Baseboard Management controller)
> 
>> addressing-space.
> 
>> 4. ASPEED DRM driver requests MCU to get HPD and EDID by CR-scratched
> 
>> register.
> 
>> 
> 
>> Booting sequence:
> 
>> 1. Check if TX is ASTDP    // 
>> ast_dp_launch()
> 
>> 2. Check if DP-MCU FW has loaded    
>> // ast_dp_launch()
> 
>> 3. Read EDID    // 
>> ast_dp_read_edid()
> 
>> 4. Resolution switch // 
>> ast_dp_SetOutput()
> 
>> 
> 
>> V3:
> 
>> 1. Remove unneeded semicolon.
> 
>> 2. Apply to git://anongit.freedesktop.org/drm/drm, instead of
> 
>> git://anongit.freedesktop.org/drm/drm-misc
> 
>> 3. Resolve auto build test WARNINGs on V1 patch.
> 
>> 
> 
>> V4:
> 
>> 1. Sync code-base with kernel 5.17_rc6 2. Remove the define of
> 
>> DPControlPower, because DP chips need to be
> 
>> powered on to be used.
> 
>> 3. Remove the switches of PHY and Display from EDID procedure.
> 
>> 4. Revise increaing delay to fixed delay, because this version kernel
> 
>> doesn't detect minitor consistenntly.
> 
>> 5. Create clean-up code used for reset of power state on errors with
> 
>> -EIO manner.
> 
>> 6. Revise the DP detection by TX type and its DP-FW status during
> 
>> booting and resume.
> 
>> 7. Correct the CamelCase Style.
> 
>> 8. Use register reading while needing, and remove to hold full
> 
>> re

RE: [PATCH] drm/ast: Atomic CR/SR reg R/W

2022-05-02 Thread Kuo-Hsiang Chou
Hi Thomas,

Thanks for your kindly assistance.

Regards
Kuo-Hsiang Chou

-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Monday, May 02, 2022 10:33 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
Cc: Ryan Chen ; airl...@linux.ie; 
清水修(o-shimizu)-台灣NEC ; Jenmin Yuan 
; airl...@redhat.com; Arc Sung 
; Luke Chen 
Subject: Re: [PATCH] drm/ast: Atomic CR/SR reg R/W

Hi

Am 06.12.21 um 02:38 schrieb Kuo-Hsiang Chou:

> I'm not going to merge this patch. As I said, I don't think it fixes the 
> problem. Mouse movement and resolution switching should not interfere with 
> each other. The DRM framework should guarantee that.
> OK, thanks for your confirmation.
> 
> I cannot reproduce the issue, but there's most likely something else 
> happening here. How can the system switch resolution and change the mouse at 
> the same time?
> Sure, we will check if there is a 100 percent method to reproduce the issue.
> Thanks for your responses.

I've been away for a while; sorry for all this taking so long.  I've meanwhile 
been able to reproduce the problem. It happens when GNOME concurrently tries to 
set the video mode and read the available video modes from EDID. Reading EDID 
is not protected against concurrent mode setting or cursor movement.

I've posted a patchset that should fix the problem. See [1].

Best regards
Thomas

[1]
https://lore.kernel.org/dri-devel/20220502142514.2174-1-tzimmerm...@suse.de/T/#t

> 
> Regards,
>   Kuo-Hsiang Chou
> 
> Best regards
> Thomas
> 
>>
>> Hi Tomas,
>> Good day!
>> May I understand the review status, or is there anything I can do to improve 
>> it? Thanks!
>>
>> Best Regards,
>>  Kuo-Hsiang Chou
>>
>> Best Regards,
>>  Kuo-Hsiang Chou
>>
>> Best regards
>> Thomas
>>
>>>
>>> Signed-off-by: KuoHsiang Chou 
>>> ---
>>> drivers/gpu/drm/ast/ast_main.c | 48 +-
>>> 1 file changed, 36 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ast/ast_main.c 
>>> b/drivers/gpu/drm/ast/ast_main.c index 79a361867..1d8fa70c5 100644
>>> --- a/drivers/gpu/drm/ast/ast_main.c
>>> +++ b/drivers/gpu/drm/ast/ast_main.c
>>> @@ -41,28 +41,52 @@ void ast_set_index_reg_mask(struct ast_private *ast,
>>> uint32_t base, uint8_t index,
>>> uint8_t mask, uint8_t val)
>>> {
>>> -   u8 tmp;
>>> -   ast_io_write8(ast, base, index);
>>> -   tmp = (ast_io_read8(ast, base + 1) & mask) | val;
>>> -   ast_set_index_reg(ast, base, index, tmp);
>>> +   uint16_t volatile usData;
>>> +   uint8_t  volatile jData;
>>> +
>>> +   do {
>>> +   ast_io_write8(ast, base, index);
>>> +   usData = ast_io_read16(ast, base);
>>> +   } while ((uint8_t)(usData) != index);
>>> +
>>> +   jData  = (uint8_t)(usData >> 8);
>>> +   jData &= mask;
>>> +   jData |= val;
>>> +   usData = ((uint16_t) jData << 8) | (uint16_t) index;
>>> +   ast_io_write16(ast, base, usData);
>>> }
>>>
>>> uint8_t ast_get_index_reg(struct ast_private *ast,
>>>   uint32_t base, uint8_t index)
>>> {
>>> -   uint8_t ret;
>>> -   ast_io_write8(ast, base, index);
>>> -   ret = ast_io_read8(ast, base + 1);
>>> -   return ret;
>>> +   uint16_t volatile usData;
>>> +   uint8_t  volatile jData;
>>> +
>>> +   do {
>>> +   ast_io_write8(ast, base, index);
>>> +   usData = ast_io_read16(ast, base);
>>> +   } while ((uint8_t)(usData) != index);
>>> +
>>> +   jData  = (uint8_t)(usData >> 8);
>>> +
>>> +   return jData;
>>> }
>>>
>>> uint8_t ast_get_index_reg_mask(struct ast_private *ast,
>>>uint32_t base, uint8_t index, uint8_t 
>>> mask)
>>> {
>>> -   uint8_t ret;
>>> -   ast_io_write8(ast, base, index);
>>> -   ret = ast_io_read8(ast, base + 1) & mask;
>>> -   return ret;
>>> +   uint16_t volatile usData;
>>> +   uint8_t  volatile jData;
>>> +
>>> +   do {
>>> +   ast_io_write8(ast, base, index);
>>> +   usData = ast_io_read16(ast, base);
>>> +   } while ((uint8_t)(usData) != index);
>>> +
>>> +   jData  = (uint8_t)(usData >> 8);
>>> +   jData &= mask;
>>> +
>>> +   return jData;
>>> }
>>>
>>> static void ast_detect_config_mode(struct drm_device *dev, u32
>>> *scu_rev)
>>> --
>>> 2.18.4
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Felix Imendörffer
>>
> 
> --
> 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

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


RE: [PATCH] drm/ast: Support 1600x900 with 108MHz PCLK

2021-12-21 Thread Kuo-Hsiang Chou
Hi

-Original Message-
From: Dave Airlie [mailto:airl...@gmail.com] 
Sent: Wednesday, December 22, 2021 5:56 AM
To: Thomas Zimmermann 

Subject: Re: [PATCH] drm/ast: Support 1600x900 with 108MHz PCLK

On Mon, 2 Nov 2020 at 17:57, Thomas Zimmermann  wrote:
>
> Hi
>
> Am 30.10.20 um 08:42 schrieb KuoHsiang Chou:
> > [New] Create the setting for 1600x900 @60Hz refresh rate
> >   by 108MHz pixel-clock.
> >
> > Signed-off-by: KuoHsiang Chou 
>
> Acked-by: Thomas Zimmermann 
>
> I'll add your patch to drm-misc-next.
>
> As Sam mentioned, you should use scripts/get_maintainers.pl to 
> retrieve the relevant people. These include those in MAINTAINERS, but 
> also developers that have previously worked on the code.

We are seeing a possible report of a regression on an ast2600 server with this 
patch.

I haven't ascertained that reverting it fixes it for the customer yet, but this 
is a heads up in case anyone else has seen issues.

Hi Dave,

Yes, you're right, The patch needs to be removed. The patch occurs incorrect 
timing on CRT and ASTDP when 1600x900 are selected.
So, do I need to commit a new patch to remove/revert it from drm/ast? 

Regards,
Kuo-Hsiang Chou

Dave.


RE: [PATCH] drm/ast: Atomic CR/SR reg R/W

2021-12-06 Thread Kuo-Hsiang Chou
Hi

-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Friday, December 03, 2021 4:47 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH] drm/ast: Atomic CR/SR reg R/W

Hi

Am 03.12.21 um 02:23 schrieb Kuo-Hsiang Chou:
> 
> 
> -Original Message-
> From: Kuo-Hsiang Chou
> Sent: Thursday, September 30, 2021 3:19 PM
> To: Thomas Zimmermann ; 
> dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
> Subject: RE: [PATCH] drm/ast: Atomic CR/SR reg R/W
> 
> Hi
> 
> -Original Message-
> From: Thomas Zimmermann [mailto:tzimmerm...@suse.de]
> Sent: Monday, September 20, 2021 4:17 PM
> To: Kuo-Hsiang Chou ; 
> dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH] drm/ast: Atomic CR/SR reg R/W
> 
> Hi
> 
> Am 17.09.21 um 09:22 schrieb KuoHsiang Chou:
>> 1. Avoid IO-index racing
>> 2. IO-index racing happened on resolustion switching
>>  and mouse moving at the same time 3. System hung while IO-index 
>> racing occurred.
> 
> I'd say that there's something else going one here. Mode setting and cursor 
> movement should be protected against each other by DRM locking.
> Changing these low-level functions would not solve the issues. I'll try to 
> reproduce the problem ASAP.
> 
> Hi Thomas,
> 
> Sorry to interrupt you again!
> May I understand the review's situation? Thanks!
Hi Thomas,

Look, you really have to work with us during the review process. Don't just 
expect us to tell you what to do.
Thanks! Got it.

I'm not going to merge this patch. As I said, I don't think it fixes the 
problem. Mouse movement and resolution switching should not interfere with each 
other. The DRM framework should guarantee that.
OK, thanks for your confirmation.

I cannot reproduce the issue, but there's most likely something else happening 
here. How can the system switch resolution and change the mouse at the same 
time?
Sure, we will check if there is a 100 percent method to reproduce the issue. 
Thanks for your responses.

Regards,
Kuo-Hsiang Chou

Best regards
Thomas

> 
> Hi Tomas,
> Good day!
> May I understand the review status, or is there anything I can do to improve 
> it? Thanks!
> 
> Best Regards,
>   Kuo-Hsiang Chou
> 
> Best Regards,
>   Kuo-Hsiang Chou
> 
> Best regards
> Thomas
> 
>>
>> Signed-off-by: KuoHsiang Chou 
>> ---
>>drivers/gpu/drm/ast/ast_main.c | 48 +-
>>1 file changed, 36 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_main.c 
>> b/drivers/gpu/drm/ast/ast_main.c index 79a361867..1d8fa70c5 100644
>> --- a/drivers/gpu/drm/ast/ast_main.c
>> +++ b/drivers/gpu/drm/ast/ast_main.c
>> @@ -41,28 +41,52 @@ void ast_set_index_reg_mask(struct ast_private *ast,
>>  uint32_t base, uint8_t index,
>>  uint8_t mask, uint8_t val)
>>{
>> -u8 tmp;
>> -ast_io_write8(ast, base, index);
>> -tmp = (ast_io_read8(ast, base + 1) & mask) | val;
>> -ast_set_index_reg(ast, base, index, tmp);
>> +uint16_t volatile usData;
>> +uint8_t  volatile jData;
>> +
>> +do {
>> +ast_io_write8(ast, base, index);
>> +usData = ast_io_read16(ast, base);
>> +} while ((uint8_t)(usData) != index);
>> +
>> +jData  = (uint8_t)(usData >> 8);
>> +jData &= mask;
>> +jData |= val;
>> +usData = ((uint16_t) jData << 8) | (uint16_t) index;
>> +ast_io_write16(ast, base, usData);
>>}
>>
>>uint8_t ast_get_index_reg(struct ast_private *ast,
>>uint32_t base, uint8_t index)
>>{
>> -uint8_t ret;
>> -ast_io_write8(ast, base, index);
>> -ret = ast_io_read8(ast, base + 1);
>> -return ret;
>> +uint16_t volatile usData;
>> +uint8_t  volatile jData;
>> +
>> +do {
>> +ast_io_write8(ast, base, index);
>> +usData = ast_io_read16(ast, base);
>> +} while ((uint8_t)(usData) != index);
>> +
>> +jData  = (uint8_t)(usData >> 8);
>> +
>> +return jData;
>>}
>>
>>uint8_t ast_get_index_reg_mask(struct ast_private *ast,
>> uint32_t base, uint8_t index, uint8_t mask)
>>{
>> -uint8_t ret;
>> -ast_io_write8(ast, base, index);
>> -ret = ast_io_read8(ast, base + 1) & mask;
>> -return ret;
>&g

RE: [PATCH] drm/ast: Create the driver for ASPEED proprietory Display-Port

2021-12-02 Thread Kuo-Hsiang Chou
Hi Thomas,

Hi Tomas,
Good day! 
May I get the review status, or is there anything I can do to improve it? 
Thanks!

Best Regards,
Kuo-Hsiang Chou

-Original Message-
From: Kuo-Hsiang Chou 
Sent: Monday, November 22, 2021 6:36 PM
To: tzimmerm...@suse.de; dri-devel@lists.freedesktop.org; 
linux-ker...@vger.kernel.org
Subject: [PATCH] drm/ast: Create the driver for ASPEED proprietory Display-Port

1. The MCU FW controling ASPEED DP is loaded by BMC boot loader.
2. Driver starts after CR[3:1] == 111b that indicates Tx is ASTDP,
   and CRD1[5] has been asserted by BMVC boot loader.
3. EDID is prioritized by DP monitor.
4. DP's EDID has high priority to decide resolution supporting.

Signed-off-by: KuoHsiang Chou 
---
 drivers/gpu/drm/ast/Makefile   |   2 +-
 drivers/gpu/drm/ast/ast_dp.c   | 286 +
 drivers/gpu/drm/ast/ast_drv.h  |  13 ++
 drivers/gpu/drm/ast/ast_main.c |   7 +-
 drivers/gpu/drm/ast/ast_mode.c |  50 +-
 drivers/gpu/drm/ast/ast_post.c |   4 +-
 6 files changed, 353 insertions(+), 9 deletions(-)  create mode 100644 
drivers/gpu/drm/ast/ast_dp.c

diff --git a/drivers/gpu/drm/ast/Makefile b/drivers/gpu/drm/ast/Makefile index 
438a2d05b..9bd0756e0 100644
--- a/drivers/gpu/drm/ast/Makefile
+++ b/drivers/gpu/drm/ast/Makefile
@@ -3,6 +3,6 @@
 # Makefile for the drm device driver.  This driver provides support for the  # 
Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
-ast-y := ast_drv.o ast_main.o ast_mm.o ast_mode.o ast_post.o ast_dp501.o
+ast-y := ast_drv.o ast_main.o ast_mm.o ast_mode.o ast_post.o 
+ast_dp501.o ast_dp.o
 
 obj-$(CONFIG_DRM_AST) := ast.o
diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c new 
file mode 100644 index 0..537eaf4fa
--- /dev/null
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -0,0 +1,286 @@
+/*
+ * Copyright 2012 Red Hat Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person 
+obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject 
+to
+ * the following conditions:
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
+EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
+MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT 
+SHALL
+ * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR 
+ANY CLAIM,
+ * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT 
+OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE 
+OR THE
+ * USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial 
+portions
+ * of the Software.
+ *
+ */
+/*
+ * Authors: KuoHsiang Chou   */
+
+#include 
+#include 
+#include 
+#include "ast_drv.h"
+
+bool ast_dp_read_edid(struct drm_device *dev, u8 *ediddata) {
+   struct ast_private *ast = to_ast_private(dev);
+   u8 i = 0, j = 0;
+
+#ifdef DPControlPower
+   u8 bDPState_Change = false;
+
+   // Check DP power off or not.
+   if (ast->ASTDP_State & 0x10) {
+   // DP power on
+   ast_dp_PowerOnOff(dev, 1);
+   bDPState_Change = true;
+   }
+#endif
+
+   /*
+* CRD1[b5]: DP MCU FW is executing
+* CRDC[b0]: DP link success
+* CRDF[b0]: DP HPD
+* CRE5[b0]: Host reading EDID process is done
+*/
+   if (!(ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 0x20) &&
+   ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDC, 0x01) &&
+   ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDF, 0x01) &&
+   ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5, 0x01))) { 
#ifdef 
+DPControlPower
+   // Set back power off
+   if (bDPState_Change)
+   ast_dp_PowerOnOff(dev, 0);
+#endif
+   return false;
+   }
+
+   ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5, 0x00, 0x00);
+
+   for (i = 0; i < 32; i++) {
+   /*
+* CRE4[7:0]: Read-Pointer for EDID (Unit: 4bytes); valid 
range: 0~64
+*/
+   ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE4, 0x00, (u8) 
i);
+   j = 0;
+
+   /*
+* CRD7[b0]: valid flag for EDID
+* CRD6[b0]: mirror read pointer for EDID
+*/
+   while ((ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD7, 
0x01) != 0x01) ||
+   (ast_get_index_reg_mask(ast, AST_I

RE: [PATCH] drm/ast: Atomic CR/SR reg R/W

2021-12-02 Thread Kuo-Hsiang Chou


-Original Message-
From: Kuo-Hsiang Chou 
Sent: Thursday, September 30, 2021 3:19 PM
To: Thomas Zimmermann ; dri-devel@lists.freedesktop.org; 
linux-ker...@vger.kernel.org
Subject: RE: [PATCH] drm/ast: Atomic CR/SR reg R/W

Hi

-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de]
Sent: Monday, September 20, 2021 4:17 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH] drm/ast: Atomic CR/SR reg R/W

Hi

Am 17.09.21 um 09:22 schrieb KuoHsiang Chou:
> 1. Avoid IO-index racing
> 2. IO-index racing happened on resolustion switching
> and mouse moving at the same time
> 3. System hung while IO-index racing occurred.

I'd say that there's something else going one here. Mode setting and cursor 
movement should be protected against each other by DRM locking. 
Changing these low-level functions would not solve the issues. I'll try to 
reproduce the problem ASAP.

Hi Thomas,

Sorry to interrupt you again!
May I understand the review's situation? Thanks!

Hi Tomas, 
Good day! 
May I understand the review status, or is there anything I can do to improve 
it? Thanks!

Best Regards,
Kuo-Hsiang Chou

Best Regards,
Kuo-Hsiang Chou

Best regards
Thomas

> 
> Signed-off-by: KuoHsiang Chou 
> ---
>   drivers/gpu/drm/ast/ast_main.c | 48 +-
>   1 file changed, 36 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_main.c 
> b/drivers/gpu/drm/ast/ast_main.c index 79a361867..1d8fa70c5 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -41,28 +41,52 @@ void ast_set_index_reg_mask(struct ast_private *ast,
>   uint32_t base, uint8_t index,
>   uint8_t mask, uint8_t val)
>   {
> - u8 tmp;
> - ast_io_write8(ast, base, index);
> - tmp = (ast_io_read8(ast, base + 1) & mask) | val;
> - ast_set_index_reg(ast, base, index, tmp);
> + uint16_t volatile usData;
> + uint8_t  volatile jData;
> +
> + do {
> + ast_io_write8(ast, base, index);
> + usData = ast_io_read16(ast, base);
> + } while ((uint8_t)(usData) != index);
> +
> + jData  = (uint8_t)(usData >> 8);
> + jData &= mask;
> + jData |= val;
> + usData = ((uint16_t) jData << 8) | (uint16_t) index;
> + ast_io_write16(ast, base, usData);
>   }
> 
>   uint8_t ast_get_index_reg(struct ast_private *ast,
> uint32_t base, uint8_t index)
>   {
> - uint8_t ret;
> - ast_io_write8(ast, base, index);
> - ret = ast_io_read8(ast, base + 1);
> - return ret;
> + uint16_t volatile usData;
> + uint8_t  volatile jData;
> +
> + do {
> + ast_io_write8(ast, base, index);
> + usData = ast_io_read16(ast, base);
> + } while ((uint8_t)(usData) != index);
> +
> + jData  = (uint8_t)(usData >> 8);
> +
> + return jData;
>   }
> 
>   uint8_t ast_get_index_reg_mask(struct ast_private *ast,
>  uint32_t base, uint8_t index, uint8_t mask)
>   {
> - uint8_t ret;
> - ast_io_write8(ast, base, index);
> - ret = ast_io_read8(ast, base + 1) & mask;
> - return ret;
> + uint16_t volatile usData;
> + uint8_t  volatile jData;
> +
> + do {
> + ast_io_write8(ast, base, index);
> + usData = ast_io_read16(ast, base);
> + } while ((uint8_t)(usData) != index);
> +
> + jData  = (uint8_t)(usData >> 8);
> + jData &= mask;
> +
> + return jData;
>   }
> 
>   static void ast_detect_config_mode(struct drm_device *dev, u32
> *scu_rev)
> --
> 2.18.4
> 

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


RE: [PATCH] drm/ast: Atomic CR/SR reg R/W

2021-09-30 Thread Kuo-Hsiang Chou


-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Monday, September 20, 2021 4:17 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH] drm/ast: Atomic CR/SR reg R/W

Hi

Am 17.09.21 um 09:22 schrieb KuoHsiang Chou:
> 1. Avoid IO-index racing
> 2. IO-index racing happened on resolustion switching
> and mouse moving at the same time
> 3. System hung while IO-index racing occurred.

I'd say that there's something else going one here. Mode setting and cursor 
movement should be protected against each other by DRM locking. 
Changing these low-level functions would not solve the issues. I'll try to 
reproduce the problem ASAP.

Hi Thomas,

Sorry to interrupt you again!
May I understand the review's situation? Thanks!

Best Regards,
Kuo-Hsiang Chou

Best regards
Thomas

> 
> Signed-off-by: KuoHsiang Chou 
> ---
>   drivers/gpu/drm/ast/ast_main.c | 48 +-
>   1 file changed, 36 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_main.c 
> b/drivers/gpu/drm/ast/ast_main.c index 79a361867..1d8fa70c5 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -41,28 +41,52 @@ void ast_set_index_reg_mask(struct ast_private *ast,
>   uint32_t base, uint8_t index,
>   uint8_t mask, uint8_t val)
>   {
> - u8 tmp;
> - ast_io_write8(ast, base, index);
> - tmp = (ast_io_read8(ast, base + 1) & mask) | val;
> - ast_set_index_reg(ast, base, index, tmp);
> + uint16_t volatile usData;
> + uint8_t  volatile jData;
> +
> + do {
> + ast_io_write8(ast, base, index);
> + usData = ast_io_read16(ast, base);
> + } while ((uint8_t)(usData) != index);
> +
> + jData  = (uint8_t)(usData >> 8);
> + jData &= mask;
> + jData |= val;
> + usData = ((uint16_t) jData << 8) | (uint16_t) index;
> + ast_io_write16(ast, base, usData);
>   }
> 
>   uint8_t ast_get_index_reg(struct ast_private *ast,
> uint32_t base, uint8_t index)
>   {
> - uint8_t ret;
> - ast_io_write8(ast, base, index);
> - ret = ast_io_read8(ast, base + 1);
> - return ret;
> + uint16_t volatile usData;
> + uint8_t  volatile jData;
> +
> + do {
> + ast_io_write8(ast, base, index);
> + usData = ast_io_read16(ast, base);
> + } while ((uint8_t)(usData) != index);
> +
> + jData  = (uint8_t)(usData >> 8);
> +
> + return jData;
>   }
> 
>   uint8_t ast_get_index_reg_mask(struct ast_private *ast,
>  uint32_t base, uint8_t index, uint8_t mask)
>   {
> - uint8_t ret;
> - ast_io_write8(ast, base, index);
> - ret = ast_io_read8(ast, base + 1) & mask;
> - return ret;
> + uint16_t volatile usData;
> + uint8_t  volatile jData;
> +
> + do {
> + ast_io_write8(ast, base, index);
> + usData = ast_io_read16(ast, base);
> + } while ((uint8_t)(usData) != index);
> +
> + jData  = (uint8_t)(usData >> 8);
> + jData &= mask;
> +
> + return jData;
>   }
> 
>   static void ast_detect_config_mode(struct drm_device *dev, u32 
> *scu_rev)
> --
> 2.18.4
> 

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


RE: [PATCH v5] drm/ast: Fixed CVE for DP501

2021-08-03 Thread Kuo-Hsiang Chou


-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Tuesday, August 03, 2021 4:58 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH v5] drm/ast: Fixed CVE for DP501

Hi

Am 29.04.21 um 11:21 schrieb Kuo-Hsiang Chou:
> More generally speaking, the DP501 code needs a major refactoring. It's 
> currently bolted onto the regular VGA connector code. It should rather be a 
> separate connector or a DRM bridge. I always wanted to work on this, but 
> don't have a device for testing. If I'd provide patches, would you be in a 
> position to test them?
> 
> NO, I can't. The patch was verified on AST2500+DP501 before, so the 
> correctness of this patch is promised. But customer always requested to send 
> the platform back after bug fixed. Now, no DP501 platform on my hand, but I 
> try to convince custom to get the someone platform.

What's the hardware platform that your customer provides to you? I'd like to do 
more development for the DP501 code, but the hardware is hard to find.

Hi Tomas
The platform was a whole server platform borrowed from Lenovo, but Lenovo had 
get it back after issue fixed.
The reason that DP501 hardware hard to find is the IC vendor, parade, isn't 
support it anymore.
But ASPEED needs to maintain DP501 for some of customers who use AST2500 and 
DP501, though, IC vendor doesn't support it anymore.

Please understand the condition that I can be the position to test DP501. 
Thanks very much! 

Regards,
        Kuo-Hsiang Chou

Best regards
Thomas

> 
> Best Regards,
>   Kuo-Hsiang Chou
> 
> Best regards
> Thomas
> 
> 
>> Signed-off-by: KuoHsiang Chou 
>> Reported-by: kernel test robot 
>> ---
>>drivers/gpu/drm/ast/ast_dp501.c | 139 +++-
>>drivers/gpu/drm/ast/ast_drv.h   |  12 +++
>>drivers/gpu/drm/ast/ast_main.c  |  11 ++-
>>3 files changed, 125 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_dp501.c 
>> b/drivers/gpu/drm/ast/ast_dp501.c index 88121c0e0..cd93c44f2 100644
>> --- a/drivers/gpu/drm/ast/ast_dp501.c
>> +++ b/drivers/gpu/drm/ast/ast_dp501.c
>> @@ -189,6 +189,9 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 
>> size)
>>  u32 i, data;
>>  u32 boot_address;
>>
>> +if (ast->config_mode != ast_use_p2a)
>> +return false;
>> +
>>  data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>>  if (data) {
>>  boot_address = get_fw_base(ast); @@ -207,6 +210,9 @@ static 
>> bool 
>> ast_launch_m68k(struct drm_device *dev)
>>  u8 *fw_addr = NULL;
>>  u8 jreg;
>>
>> +if (ast->config_mode != ast_use_p2a)
>> +return false;
>> +
>>  data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>>  if (!data) {
>>
>> @@ -271,25 +277,55 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
>>  struct ast_private *ast = to_ast_private(dev);
>>  u32 boot_address, offset, data;
>>  u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
>> +u32 *plinkcap;
>>
>> -boot_address = get_fw_base(ast);
>> -
>> -/* validate FW version */
>> -offset = 0xf000;
>> -data = ast_mindwm(ast, boot_address + offset);
>> -if ((data & 0xf0) != 0x10) /* version: 1x */
>> -return maxclk;
>> -
>> -/* Read Link Capability */
>> -offset  = 0xf014;
>> -*(u32 *)linkcap = ast_mindwm(ast, boot_address + offset);
>> -if (linkcap[2] == 0) {
>> -linkrate = linkcap[0];
>> -linklanes = linkcap[1];
>> -data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
>> -if (data > 0xff)
>> -data = 0xff;
>> -maxclk = (u8)data;
>> +if (ast->config_mode == ast_use_p2a) {
>> +boot_address = get_fw_base(ast);
>> +
>> +/* validate FW version */
>> +offset = AST_DP501_GBL_VERSION;
>> +data = ast_mindwm(ast, boot_address + offset);
>> +if ((data & AST_DP501_FW_VERSION_MASK) != 
>> AST_DP501_FW_VERSION_1) /* version: 1x */
>> +return maxclk;
>> +
>> +/* Read Link Capability */
>> +offset  = AST_DP501_LINKRATE;
>> +plinkcap = (u32 *)linkcap;
>> +*plinkcap  = ast_mindwm(ast, boot_address + offset);
>> +if (linkcap[2] == 0) {
>> +linkrate = lin

RE: [PATCH v4] drm/ast: Disable fast reset after DRAM initial

2021-06-21 Thread Kuo-Hsiang Chou
Hi Thomas, 

May I know if I need to port this patch to the latest drm-misc-next again, 
because the patch has send to review for a while.
If the porting or any other thing can reduce your review effort, please 
instruct me. Thanks!

Best Regards,
Kuo-Hsiang Chou

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Kuo-Hsiang Chou
Sent: Wednesday, May 26, 2021 6:24 PM
To: tzimmerm...@suse.de; dri-devel@lists.freedesktop.org; 
linux-ker...@vger.kernel.org
Subject: RE: [PATCH v4] drm/ast: Disable fast reset after DRAM initial



-Original Message-
From: Kuo-Hsiang Chou 
Sent: Friday, May 07, 2021 5:27 PM
To: tzimmerm...@suse.de; dri-devel@lists.freedesktop.org; 
linux-ker...@vger.kernel.org
Cc: airl...@redhat.com; airl...@linux.ie; dan...@ffwll.ch; Jenmin Yuan 
; Kuo-Hsiang Chou ; 
Arc Sung 
Subject: [PATCH v4] drm/ast: Disable fast reset after DRAM initial

Hi Thomas,

May I know if this patch has sth wrong. Or something I need to improve on it, I 
can fix it right now. Thanks!

Regards,
Kuo-Hsiang Chou

[Bug][AST2500]

V1:
When AST2500 acts as stand-alone VGA so that DRAM and DVO initialization have 
to be achieved by VGA driver with P2A (PCI to AHB) enabling.
However, HW suggests disable Fast reset mode after DRAM initializaton, because 
fast reset mode is mainly designed for ARM ICE debugger.
Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should be first 
enabled to avoid system deadlock before disable fast reset mode.

V2:
Use to_pci_dev() to get revision of PCI configuration.

V3:
If SCU00 is not unlocked, just enter its password again.
It is unnecessary to clear AHB lock condition and restore WDT default setting 
again, before Fast-reset clearing.

V4:
repatch after "error : could not build fake ancestor" resolved.

Signed-off-by: KuoHsiang Chou 
---
 drivers/gpu/drm/ast/ast_drv.h  |  1 +
 drivers/gpu/drm/ast/ast_main.c |  4 ++
 drivers/gpu/drm/ast/ast_post.c | 68 +-
 3 files changed, 47 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h 
index 911f9f414..5ebb5905d 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -346,6 +346,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);  void 
ast_post_gpu(struct drm_device *dev);
 u32 ast_mindwm(struct ast_private *ast, u32 r);  void ast_moutdwm(struct 
ast_private *ast, u32 r, u32 v);
+void ast_patch_ahb_2500(struct ast_private *ast);
 /* ast dp501 */
 void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);  bool 
ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size); diff --git 
a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 
2aff2e6cf..cfb56ea3a 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -97,6 +97,10 @@ static void ast_detect_config_mode(struct drm_device *dev, 
u32 *scu_rev)
jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
+   /* Patch AST2500 */
+   if (((pdev->revision & 0xF0) == 0x40) && ((jregd0 & 0xC0) == 0))
+   ast_patch_ahb_2500(ast);
+
/* Double check it's actually working */
data = ast_read32(ast, 0xf004);
if ((data != 0x) && (data != 0x00)) { diff --git 
a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c index 
0607658dd..56428798a 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -2028,6 +2028,30 @@ static bool ast_dram_init_2500(struct ast_private *ast)
return true;
 }

+void ast_patch_ahb_2500(struct ast_private *ast) {
+   u32 data;
+
+   /* Clear bus lock condition */
+   ast_moutdwm(ast, 0x1e60, 0xAEED1A03);
+   ast_moutdwm(ast, 0x1e600084, 0x0001);
+   ast_moutdwm(ast, 0x1e600088, 0x);
+   ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+   data = ast_mindwm(ast, 0x1e6e2070);
+   if (data & 0x0800) {/* 
check fast reset */
+
+   ast_moutdwm(ast, 0x1E785004, 0x0010);
+   ast_moutdwm(ast, 0x1E785008, 0x4755);
+   ast_moutdwm(ast, 0x1E78500c, 0x0033);
+   udelay(1000);
+   }
+   do {
+   ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+   data = ast_mindwm(ast, 0x1e6e2000);
+   }   while (data != 1);
+   ast_moutdwm(ast, 0x1e6e207c, 0x0800);   /* clear fast reset */
+}
+
 void ast_post_chip_2500(struct drm_device *dev)  {
struct ast_private *ast = to_ast_private(dev); @@ -2035,39 +2059,31 @@ 
void ast_post_chip_2500(struct drm_device *dev)
u8 reg;

reg = ast_get_index_reg_mas

RE: [PATCH v4] drm/ast: Disable fast reset after DRAM initial

2021-05-26 Thread Kuo-Hsiang Chou



-Original Message-
From: Kuo-Hsiang Chou 
Sent: Friday, May 07, 2021 5:27 PM
To: tzimmerm...@suse.de; dri-devel@lists.freedesktop.org; 
linux-ker...@vger.kernel.org
Cc: airl...@redhat.com; airl...@linux.ie; dan...@ffwll.ch; Jenmin Yuan 
; Kuo-Hsiang Chou ; 
Arc Sung 
Subject: [PATCH v4] drm/ast: Disable fast reset after DRAM initial

Hi Thomas,

May I know if this patch has sth wrong. Or something I need to improve on it, I 
can fix it right now. Thanks!

Regards,
Kuo-Hsiang Chou

[Bug][AST2500]

V1:
When AST2500 acts as stand-alone VGA so that DRAM and DVO initialization have 
to be achieved by VGA driver with P2A (PCI to AHB) enabling.
However, HW suggests disable Fast reset mode after DRAM initializaton, because 
fast reset mode is mainly designed for ARM ICE debugger.
Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should be first 
enabled to avoid system deadlock before disable fast reset mode.

V2:
Use to_pci_dev() to get revision of PCI configuration.

V3:
If SCU00 is not unlocked, just enter its password again.
It is unnecessary to clear AHB lock condition and restore WDT default setting 
again, before Fast-reset clearing.

V4:
repatch after "error : could not build fake ancestor" resolved.

Signed-off-by: KuoHsiang Chou 
---
 drivers/gpu/drm/ast/ast_drv.h  |  1 +
 drivers/gpu/drm/ast/ast_main.c |  4 ++
 drivers/gpu/drm/ast/ast_post.c | 68 +-
 3 files changed, 47 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h 
index 911f9f414..5ebb5905d 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -346,6 +346,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);  void 
ast_post_gpu(struct drm_device *dev);
 u32 ast_mindwm(struct ast_private *ast, u32 r);  void ast_moutdwm(struct 
ast_private *ast, u32 r, u32 v);
+void ast_patch_ahb_2500(struct ast_private *ast);
 /* ast dp501 */
 void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);  bool 
ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size); diff --git 
a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 
2aff2e6cf..cfb56ea3a 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -97,6 +97,10 @@ static void ast_detect_config_mode(struct drm_device *dev, 
u32 *scu_rev)
jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
+   /* Patch AST2500 */
+   if (((pdev->revision & 0xF0) == 0x40) && ((jregd0 & 0xC0) == 0))
+   ast_patch_ahb_2500(ast);
+
/* Double check it's actually working */
data = ast_read32(ast, 0xf004);
if ((data != 0x) && (data != 0x00)) { diff --git 
a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c index 
0607658dd..56428798a 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -2028,6 +2028,30 @@ static bool ast_dram_init_2500(struct ast_private *ast)
return true;
 }

+void ast_patch_ahb_2500(struct ast_private *ast) {
+   u32 data;
+
+   /* Clear bus lock condition */
+   ast_moutdwm(ast, 0x1e60, 0xAEED1A03);
+   ast_moutdwm(ast, 0x1e600084, 0x0001);
+   ast_moutdwm(ast, 0x1e600088, 0x);
+   ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+   data = ast_mindwm(ast, 0x1e6e2070);
+   if (data & 0x0800) {/* 
check fast reset */
+
+   ast_moutdwm(ast, 0x1E785004, 0x0010);
+   ast_moutdwm(ast, 0x1E785008, 0x4755);
+   ast_moutdwm(ast, 0x1E78500c, 0x0033);
+   udelay(1000);
+   }
+   do {
+   ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+   data = ast_mindwm(ast, 0x1e6e2000);
+   }   while (data != 1);
+   ast_moutdwm(ast, 0x1e6e207c, 0x0800);   /* clear fast reset */
+}
+
 void ast_post_chip_2500(struct drm_device *dev)  {
struct ast_private *ast = to_ast_private(dev); @@ -2035,39 +2059,31 @@ 
void ast_post_chip_2500(struct drm_device *dev)
u8 reg;

reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
-   if ((reg & 0x80) == 0) {/* vga only */
+   if ((reg & 0xC0) == 0) {/* vga only */
/* Clear bus lock condition */
-   ast_moutdwm(ast, 0x1e60, 0xAEED1A03);
-   ast_moutdwm(ast, 0x1e600084, 0x0001);
-   ast_moutdwm(ast, 0x1e600088, 0x);
-   ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
-   ast_write32(ast, 0xf004, 0x1e6e);
-   ast_write32(ast, 0xf000, 0x1);
-   ast_write32(ast, 0x12000, 0x168

RE: [PATCH v5] drm/ast: Fixed CVE for DP501

2021-04-29 Thread Kuo-Hsiang Chou


-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Tuesday, April 27, 2021 7:02 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org

Subject: Re: [PATCH v5] drm/ast: Fixed CVE for DP501

Hi Thomas,
Hi

Am 21.04.21 um 10:58 schrieb KuoHsiang Chou:
> [Bug][DP501]
> If ASPEED P2A (PCI to AHB) bridge is disabled and disallowed for
> CVE_2019_6260 item3, and then the monitor's EDID is unable read 
> through Parade DP501.
> The reason is the DP501's FW is mapped to BMC addressing space rather 
> than Host addressing space.
> The resolution is that using "pci_iomap_range()" maps to DP501's FW 
> that stored on the end of FB (Frame Buffer).
> In this case, FrameBuffer reserves the last 2MB used for the image of 
> DP501.
> 

Your patches are missing a short changelog, so that reviewers can see what 
changed between versions. Anyway, I merged your patch into drm-misc-next now. 
Thanks for the fix.

Thanks, I must keep this rule about the short changelog.

More generally speaking, the DP501 code needs a major refactoring. It's 
currently bolted onto the regular VGA connector code. It should rather be a 
separate connector or a DRM bridge. I always wanted to work on this, but don't 
have a device for testing. If I'd provide patches, would you be in a position 
to test them?

NO, I can't. The patch was verified on AST2500+DP501 before, so the correctness 
of this patch is promised. But customer always requested to send the platform 
back after bug fixed. Now, no DP501 platform on my hand, but I try to convince 
custom to get the someone platform.

Best Regards,
Kuo-Hsiang Chou

Best regards
Thomas


> Signed-off-by: KuoHsiang Chou 
> Reported-by: kernel test robot 
> ---
>   drivers/gpu/drm/ast/ast_dp501.c | 139 +++-
>   drivers/gpu/drm/ast/ast_drv.h   |  12 +++
>   drivers/gpu/drm/ast/ast_main.c  |  11 ++-
>   3 files changed, 125 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_dp501.c 
> b/drivers/gpu/drm/ast/ast_dp501.c index 88121c0e0..cd93c44f2 100644
> --- a/drivers/gpu/drm/ast/ast_dp501.c
> +++ b/drivers/gpu/drm/ast/ast_dp501.c
> @@ -189,6 +189,9 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 
> size)
>   u32 i, data;
>   u32 boot_address;
> 
> + if (ast->config_mode != ast_use_p2a)
> + return false;
> +
>   data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>   if (data) {
>   boot_address = get_fw_base(ast);
> @@ -207,6 +210,9 @@ static bool ast_launch_m68k(struct drm_device *dev)
>   u8 *fw_addr = NULL;
>   u8 jreg;
> 
> + if (ast->config_mode != ast_use_p2a)
> + return false;
> +
>   data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>   if (!data) {
> 
> @@ -271,25 +277,55 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
>   struct ast_private *ast = to_ast_private(dev);
>   u32 boot_address, offset, data;
>   u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
> + u32 *plinkcap;
> 
> - boot_address = get_fw_base(ast);
> -
> - /* validate FW version */
> - offset = 0xf000;
> - data = ast_mindwm(ast, boot_address + offset);
> - if ((data & 0xf0) != 0x10) /* version: 1x */
> - return maxclk;
> -
> - /* Read Link Capability */
> - offset  = 0xf014;
> - *(u32 *)linkcap = ast_mindwm(ast, boot_address + offset);
> - if (linkcap[2] == 0) {
> - linkrate = linkcap[0];
> - linklanes = linkcap[1];
> - data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
> - if (data > 0xff)
> - data = 0xff;
> - maxclk = (u8)data;
> + if (ast->config_mode == ast_use_p2a) {
> + boot_address = get_fw_base(ast);
> +
> + /* validate FW version */
> + offset = AST_DP501_GBL_VERSION;
> + data = ast_mindwm(ast, boot_address + offset);
> + if ((data & AST_DP501_FW_VERSION_MASK) != 
> AST_DP501_FW_VERSION_1) /* version: 1x */
> + return maxclk;
> +
> + /* Read Link Capability */
> + offset  = AST_DP501_LINKRATE;
> + plinkcap = (u32 *)linkcap;
> + *plinkcap  = ast_mindwm(ast, boot_address + offset);
> + if (linkcap[2] == 0) {
> + linkrate = linkcap[0];
> + linklanes = linkcap[1];
> + data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * 
> linklanes);
> + if (data > 0xff)
> +   

RE: [PATCH V3] drm/ast: Disable fast reset after DRAM initial

2021-03-30 Thread Kuo-Hsiang Chou
Message-ID: <20201228030823.294147-1-kuohsiang_c...@aspeedtech.com>

-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Monday, March 29, 2021 5:17 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org

Subject: Re: [PATCH V3] drm/ast: Disable fast reset after DRAM initial

Hi,

I cannot apply this patch. The error is shown below. Which tree do you use? Can 
you please move to drm-misc-next?

Applying: drm/ast: Disable fast reset after DRAM initial
error: sha1 information is lacking or useless (drivers/gpu/drm/ast/ast_drv.h).
error: could not build fake ancestor
Patch failed at 0001 drm/ast: Disable fast reset after DRAM initial
hint: Use 'git am --show-current-patch=diff' to see the failed patch When you 
have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
dim: ERROR: git apply-mbox failed

Hi, Thomas,

Thanks for the comments, I still use kernel_5.9. Yes, I will move to the latest 
version of drm-misc-next.
The errors seem to be caused by a pending patch(Message-ID: 
<20201228030823.294147-1-kuohsiang_c...@aspeedtech.com>). 
And I submitted current patch before reviewer result of pending 
patch(Message-ID: <20201228030823.294147-1-kuohsiang_c...@aspeedtech.com>).

Please give an instruction how to works to next step. 
Continue waiting for the reviewer result, or move to kernel_5.12-rc1 and submit 
the pending patch again? Or other suggestions?
Thanks!

Best Regards,
Kuo-Hsiang Chou

Best regards
Thomas


Am 19.03.21 um 10:23 schrieb KuoHsiang Chou:
> [Bug][AST2500]
> 
> V1:
> When AST2500 acts as stand-alone VGA so that DRAM and DVO 
> initialization have to be achieved by VGA driver with P2A (PCI to AHB) 
> enabling.
> However, HW suggests disable Fast reset mode after DRAM initializaton, 
> because fast reset mode is mainly designed for ARM ICE debugger.
> Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should 
> be first enabled to avoid system deadlock before disable fast reset mode.
> 
> V2:
> Use to_pci_dev() to get revision of PCI configuration.
> 
> V3:
> If SCU00 is not unlocked, just enter its password again.
> It is unnecessary to clear AHB lock condition and restore WDT default 
> setting again, before Fast-reset clearing.
> 
> Signed-off-by: KuoHsiang Chou 
> ---
>   drivers/gpu/drm/ast/ast_drv.h  |  1 +
>   drivers/gpu/drm/ast/ast_main.c |  5 +++
>   drivers/gpu/drm/ast/ast_post.c | 68 +-
>   3 files changed, 48 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h 
> b/drivers/gpu/drm/ast/ast_drv.h index da6dfb677540..a2cf5fef2399 
> 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -320,6 +320,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);
>   void ast_post_gpu(struct drm_device *dev);
>   u32 ast_mindwm(struct ast_private *ast, u32 r);
>   void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
> +void ast_patch_ahb_2500(struct ast_private *ast);
>   /* ast dp501 */
>   void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
>   bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size); diff 
> --git a/drivers/gpu/drm/ast/ast_main.c 
> b/drivers/gpu/drm/ast/ast_main.c index 3775fe26f792..0e4dfcc25623 
> 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -69,6 +69,7 @@ static void ast_detect_config_mode(struct drm_device *dev, 
> u32 *scu_rev)
>   {
>   struct device_node *np = dev->pdev->dev.of_node;
>   struct ast_private *ast = to_ast_private(dev);
> + struct pci_dev *pdev = to_pci_dev(dev->dev);
>   uint32_t data, jregd0, jregd1;
> 
>   /* Defaults */
> @@ -96,6 +97,10 @@ static void ast_detect_config_mode(struct drm_device *dev, 
> u32 *scu_rev)
>   jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
>   jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
>   if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
> + /* Patch AST2500 */
> + if (((pdev->revision & 0xF0) == 0x40) && ((jregd0 & 0xC0) == 0))
> + ast_patch_ahb_2500(ast);
> +
>   /* Double check it's actually working */
>   data = ast_read32(ast, 0xf004);
>   if (data != 0x) {
> diff --git a/drivers/gpu/drm/ast/ast_post.c 
> b/drivers/gpu/drm/ast/ast_post.c index 8902c2f84bf9..4f194c5fd2c2 
> 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -2026,6 +2

RE: [PATCH] drm/ast: Update the sequence of Clearing Fast-reset

2021-01-21 Thread Kuo-Hsiang Chou
-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Thursday, January 21, 2021 3:56 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH] drm/ast: Update the sequence of Clearing Fast-reset

Hi

Am 18.01.21 um 09:57 schrieb KuoHsiang Chou:
> [Bug][AST2500]
> If SCU00 is not unlocked, just enter its password again.
> It is unnecessary to clear AHB lock condition and restore WDT default 
> setting again, before Fast-reset clearing.
> 
> Signed-off-by: KuoHsiang Chou 

Is this a separate issue? This patch looks like a fix for the previous patch. 
[1] Can you add this change to v3 of the other patch?

Hi, 
Based on the result of driver released, this patch is a fix for previous one 
[1], so that I will merge this two patches into a single one as v3 of [1]. 
Thanks!

Regards,
    Kuo-Hsiang Chou

Best regards
Thomas

[1]
https://lore.kernel.org/dri-devel/20210112075811.9354-1-kuohsiang_c...@aspeedtech.com/

> ---
>   drivers/gpu/drm/ast/ast_post.c | 5 +
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_post.c 
> b/drivers/gpu/drm/ast/ast_post.c index 1f0007daa005..4f194c5fd2c2 
> 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -2030,7 +2030,6 @@ void ast_patch_ahb_2500(struct ast_private *ast)
>   {
>   u32 data;
> 
> -patch_ahb_lock:
>   /* Clear bus lock condition */
>   ast_moutdwm(ast, 0x1e60, 0xAEED1A03);
>   ast_moutdwm(ast, 0x1e600084, 0x0001); @@ -2044,11 +2043,9 @@ 
> void ast_patch_ahb_2500(struct ast_private *ast)
>   ast_moutdwm(ast, 0x1E78500c, 0x0033);
>   udelay(1000);
>   }
> - ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
>   do {
> + ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
>   data = ast_mindwm(ast, 0x1e6e2000);
> - if (data == 0x)
> - goto patch_ahb_lock;
>   }   while (data != 1);
>   ast_moutdwm(ast, 0x1e6e207c, 0x0800);   /* clear fast reset */
>   }
> --
> 2.18.4
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


RE: [PATCH v2] drm/ast: Disable fast reset after DRAM initial

2021-01-18 Thread Kuo-Hsiang Chou
-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Monday, January 18, 2021 5:06 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org

Subject: Re: [PATCH v2] drm/ast: Disable fast reset after DRAM initial

Hi, Thomas,

Hi

Am 12.01.21 um 08:58 schrieb KuoHsiang Chou:
> [Bug][AST2500]
> 
> V1:
> When AST2500 acts as stand-alone VGA so that DRAM and DVO 
> initialization have to be achieved by VGA driver with P2A (PCI to AHB) 
> enabling.
> However, HW suggests disable Fast reset mode after DRAM initializaton, 
> because fast reset mode is mainly designed for ARM ICE debugger.
> Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should 
> be first enabled to avoid system deadlock before disable fast reset mode.
> 
> V2:
> Use to_pci_dev() to get revision of PCI configuration.
> 
> Signed-off-by: KuoHsiang Chou 
> ---
>   drivers/gpu/drm/ast/ast_drv.h  |  1 +
>   drivers/gpu/drm/ast/ast_main.c |  5 +++
>   drivers/gpu/drm/ast/ast_post.c | 71 +-
>   3 files changed, 51 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h 
> b/drivers/gpu/drm/ast/ast_drv.h index da6dfb677540..a2cf5fef2399 
> 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -320,6 +320,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);
>   void ast_post_gpu(struct drm_device *dev);
>   u32 ast_mindwm(struct ast_private *ast, u32 r);
>   void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
> +void ast_patch_ahb_2500(struct ast_private *ast);
>   /* ast dp501 */
>   void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
>   bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size); diff 
> --git a/drivers/gpu/drm/ast/ast_main.c 
> b/drivers/gpu/drm/ast/ast_main.c index 3775fe26f792..0e4dfcc25623 
> 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -69,6 +69,7 @@ static void ast_detect_config_mode(struct drm_device *dev, 
> u32 *scu_rev)
>   {
>   struct device_node *np = dev->pdev->dev.of_node;
>   struct ast_private *ast = to_ast_private(dev);
> + struct pci_dev *pdev = to_pci_dev(dev->dev);
>   uint32_t data, jregd0, jregd1;
> 
>   /* Defaults */
> @@ -96,6 +97,10 @@ static void ast_detect_config_mode(struct drm_device *dev, 
> u32 *scu_rev)
>   jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
>   jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
>   if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
> + /* Patch AST2500 */
> + if (((pdev->revision & 0xF0) == 0x40) && ((jregd0 & 0xC0) == 0))
> + ast_patch_ahb_2500(ast);
> +
>   /* Double check it's actually working */
>   data = ast_read32(ast, 0xf004);
>   if (data != 0x) {
> diff --git a/drivers/gpu/drm/ast/ast_post.c 
> b/drivers/gpu/drm/ast/ast_post.c index 8902c2f84bf9..1f0007daa005 
> 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -2026,6 +2026,33 @@ static bool ast_dram_init_2500(struct ast_private *ast)
>   return true;
>   }
> 
> +void ast_patch_ahb_2500(struct ast_private *ast) {
> + u32 data;
> +
> +patch_ahb_lock:
> + /* Clear bus lock condition */
> + ast_moutdwm(ast, 0x1e60, 0xAEED1A03);
> + ast_moutdwm(ast, 0x1e600084, 0x0001);
> + ast_moutdwm(ast, 0x1e600088, 0x);
> + ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> + data = ast_mindwm(ast, 0x1e6e2070);
> + if (data & 0x0800) {/* 
> check fast reset */
> +
> + ast_moutdwm(ast, 0x1E785004, 0x0010);
> + ast_moutdwm(ast, 0x1E785008, 0x4755);
> + ast_moutdwm(ast, 0x1E78500c, 0x0033);
> + udelay(1000);
> + }
> + ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> + do {
> + data = ast_mindwm(ast, 0x1e6e2000);
> + if (data == 0x)
> + goto patch_ahb_lock;
> + }   while (data != 1);
> + ast_moutdwm(ast, 0x1e6e207c, 0x0800);   /* clear fast reset */
> +}
> +
>   void ast_post_chip_2500(struct drm_device *dev)
>   {
>   struct ast_private *ast = to_ast_private(dev); @@ -2033,39 +2060,31 
> @@ void ast_post_chip_2500(struct drm_device *dev)
>   u8 reg;
> 
>   reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
> - if ((reg & 0x80) == 0) {/* vga only */
> + if ((reg & 0xC0) == 0) {/* vga only

RE: [PATCH] drm/ast: Create chip AST2600

2020-11-09 Thread Kuo-Hsiang Chou
Hi, Thomas,

OK, I will describe the patch more precisely in the future. Thanks again!!!

Have a good day.
Kuo-Hsiang Chou

-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Monday, November 09, 2020 6:26 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
Cc: airl...@redhat.com; airl...@linux.ie; dan...@ffwll.ch; Jenmin Yuan 
; Arc Sung ; Tommy Huang 

Subject: Re: [PATCH] drm/ast: Create chip AST2600

Hi

Am 09.11.20 um 10:38 schrieb KuoHsiang Chou:
> [New] Support AST2600

A style issue: better write a nice sentence than these tags.  For the patch at 
hand, I'd preferred something like: "Only add an id for supporting AST2600. No 
functional changes are required."

I assume that there areno further changes required for AST2600.

> 
> Signed-off-by: KuoHsiang Chou 

Reviewed-by: Thomas Zimmermann 

I'll add the patch to drm-misc-next. Thanks!

Best regards
Thomas

> ---
>  drivers/gpu/drm/ast/ast_drv.h  | 1 +
>  drivers/gpu/drm/ast/ast_main.c | 5 -
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h 
> b/drivers/gpu/drm/ast/ast_drv.h index 467049ca8430..6b9e3b94a712 
> 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -63,6 +63,7 @@ enum ast_chip {
>   AST2300,
>   AST2400,
>   AST2500,
> + AST2600,
>  };
> 
>  enum ast_tx_chip {
> diff --git a/drivers/gpu/drm/ast/ast_main.c 
> b/drivers/gpu/drm/ast/ast_main.c index 77066bca8793..4ec6884f6c65 
> 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -143,7 +143,10 @@ static int ast_detect_chip(struct drm_device *dev, bool 
> *need_post)
>   ast_detect_config_mode(dev, &scu_rev);
> 
>   /* Identify chipset */
> - if (dev->pdev->revision >= 0x40) {
> + if (dev->pdev->revision >= 0x50) {
> + ast->chip = AST2600;
> + drm_info(dev, "AST 2600 detected\n");
> + } else if (dev->pdev->revision >= 0x40) {
>   ast->chip = AST2500;
>   drm_info(dev, "AST 2500 detected\n");
>   } else if (dev->pdev->revision >= 0x30) {
> --
> 2.18.4
> 

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


RE: [PATCH] drm/ast: Fixed 1920x1080 sync. polarity issue

2020-11-05 Thread Kuo-Hsiang Chou
To Sir,

Is the mail-address of "daniel.vet...@intel.com" correct for Daniel? 
Because, got the message of " The current directory does not appear to be a 
linux kernel source tree. " after running get_maintainers.pl
So, find Daniel out by gitk /drivers/gpu/drm/ast

Have a good day,
    Kuo-Hsiang Chou

-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Thursday, November 05, 2020 8:22 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org
Cc: Jenmin Yuan ; e...@suse.com; Tommy Huang 
; Arc Sung ; 
airl...@redhat.com
Subject: Re: [PATCH] drm/ast: Fixed 1920x1080 sync. polarity issue

Hi

Am 05.11.20 um 13:08 schrieb Thomas Zimmermann:
> Hi,
> 
> please follow the advise we're giving you. Had you run 
> scripts/checkpatch.pl on the patch file it would have told you
> 
> <<<
> ERROR: patch seems to be corrupt (line wrapped?)
> #102: FILE: drivers/gpu/drm/ast/ast_tables.h:294:
> 
> 
> ERROR: DOS line endings
> #106: FILE: drivers/gpu/drm/ast/ast_tables.h:297:
> +^I (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | 
> +NewModeInfo |^M$
> 
> ERROR: DOS line endings
> #110: FILE: drivers/gpu/drm/ast/ast_tables.h:300:
> +^I (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | 
> +NewModeInfo |^M$
> 
> total: 3 errors, 0 warnings, 19 lines checked
>>>>
> 

This might not be your fault but the result of my mail client.

> As said before, the mail e...@suse.de is still not relevant any longer.
> But you did not add Daniel into CC. Did you run scripts/get_maintainers.pl?

This still applies.

Best regards
Thomas

> 
> Best regards
> Thomas
> 
> Am 05.11.20 um 10:47 schrieb KuoHsiang Chou:
>> [Bug] Change the vertical synchroous polary of 1920x1080 @60Hz
>>   from  Negtive to Positive
>>
>> Signed-off-by: KuoHsiang Chou 
>> ---
>>  drivers/gpu/drm/ast/ast_tables.h | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_tables.h 
>> b/drivers/gpu/drm/ast/ast_tables.h
>> index 8414e543f260..51efc5b4a55c 100644
>> --- a/drivers/gpu/drm/ast/ast_tables.h
>> +++ b/drivers/gpu/drm/ast/ast_tables.h
>> @@ -295,10 +295,10 @@ static const struct ast_vbios_enhtable 
>> res_1600x900[] = {
>>
>>  static const struct ast_vbios_enhtable res_1920x1080[] = {
>>  {2200, 1920, 88, 44, 1125, 1080, 4, 5, VCLK148_5,   /* 60Hz */
>> - (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo |
>> + (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | 
>> +NewModeInfo |
>>AST2500PreCatchCRT), 60, 1, 0x38 },
>>  {2200, 1920, 88, 44, 1125, 1080, 4, 5, VCLK148_5,   /* 60Hz */
>> - (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo |
>> + (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | 
>> +NewModeInfo |
>>AST2500PreCatchCRT), 0xFF, 1, 0x38 },  };
>>
>> --
>> 2.18.4
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> 
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


RE: [PATCH] drm/ast: Fixed 1920x1080 sync. polarity issue

2020-11-05 Thread Kuo-Hsiang Chou
To Sir

Yes, I ran scripts/checkpatch.pl and cleared 3 errors about tailing spaces.

OK, would you please give me the mail-address of Daniel? And  
will be removed from CC. 
Because I ran get_maintainers.pl and got the result of " The current directory 
does not appear to be a linux kernel source tree. "
So, I got the information from MAINTAINERS by keyword of "/drm/ast", likes as:
DRM DRIVER FOR AST SERVER GRAPHICS CHIPS
M:  Dave Airlie 
R:  Thomas Zimmermann 
L:  dri-devel@lists.freedesktop.org
S:  Supported
T:  git git://anongit.freedesktop.org/drm/drm-misc
F:  drivers/gpu/drm/ast/

Thanks and Have a good day,
    Kuo-Hsiang Chou

-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Thursday, November 05, 2020 8:09 PM
To: Kuo-Hsiang Chou ; 
dri-devel@lists.freedesktop.org
Cc: e...@suse.com; Tommy Huang ; Jenmin Yuan 
; airl...@redhat.com; Arc Sung 

Subject: Re: [PATCH] drm/ast: Fixed 1920x1080 sync. polarity issue

Hi,

please follow the advise we're giving you. Had you run scripts/checkpatch.pl on 
the patch file it would have told you

<<<
ERROR: patch seems to be corrupt (line wrapped?)
#102: FILE: drivers/gpu/drm/ast/ast_tables.h:294:


ERROR: DOS line endings
#106: FILE: drivers/gpu/drm/ast/ast_tables.h:297:
+^I (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo 
+|^M$

ERROR: DOS line endings
#110: FILE: drivers/gpu/drm/ast/ast_tables.h:300:
+^I (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo 
+|^M$

total: 3 errors, 0 warnings, 19 lines checked
>>>

As said before, the mail e...@suse.de is still not relevant any longer.
But you did not add Daniel into CC. Did you run scripts/get_maintainers.pl?

Best regards
Thomas

Am 05.11.20 um 10:47 schrieb KuoHsiang Chou:
> [Bug] Change the vertical synchroous polary of 1920x1080 @60Hz
>   from  Negtive to Positive
> 
> Signed-off-by: KuoHsiang Chou 
> ---
>  drivers/gpu/drm/ast/ast_tables.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_tables.h 
> b/drivers/gpu/drm/ast/ast_tables.h
> index 8414e543f260..51efc5b4a55c 100644
> --- a/drivers/gpu/drm/ast/ast_tables.h
> +++ b/drivers/gpu/drm/ast/ast_tables.h
> @@ -295,10 +295,10 @@ static const struct ast_vbios_enhtable 
> res_1600x900[] = {
> 
>  static const struct ast_vbios_enhtable res_1920x1080[] = {
>   {2200, 1920, 88, 44, 1125, 1080, 4, 5, VCLK148_5,   /* 60Hz */
> -  (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo |
> +  (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo 
> +|
> AST2500PreCatchCRT), 60, 1, 0x38 },
>   {2200, 1920, 88, 44, 1125, 1080, 4, 5, VCLK148_5,   /* 60Hz */
> -  (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo |
> +  (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo 
> +|
> AST2500PreCatchCRT), 0xFF, 1, 0x38 },  };
> 
> --
> 2.18.4
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


RE: [PATCH] drm/ast: Support 1600x900 with 108MHz PCLK

2020-11-01 Thread Kuo-Hsiang Chou
Hi, Sam

Thanks for your teaching and the information got from v5.10-rc2 likes as

DRM DRIVER FOR AST SERVER GRAPHICS CHIPS
M:  Dave Airlie 
R:  Thomas Zimmermann 
L:  dri-devel@lists.freedesktop.org
S:  Supported
T:  git git://anongit.freedesktop.org/drm/drm-misc
F:  drivers/gpu/drm/ast/

Regards,
Kuo-Hsiang Chou

-Original Message-
From: Sam Ravnborg [mailto:s...@ravnborg.org] 
Sent: Sunday, November 01, 2020 5:19 PM
To: Kuo-Hsiang Chou ; Thomas Zimmermann 

Cc: dri-devel@lists.freedesktop.org; e...@suse.com; Tommy Huang 
; Jenmin Yuan ; 
airl...@redhat.com; Arc Sung 
Subject: Re: [PATCH] drm/ast: Support 1600x900 with 108MHz PCLK

Hi KuoHsiang

On Fri, Oct 30, 2020 at 03:42:12PM +0800, KuoHsiang Chou wrote:
> [New] Create the setting for 1600x900 @60Hz refresh rate
>   by 108MHz pixel-clock.
> 
> Signed-off-by: KuoHsiang Chou 

Thomas Zimmermann  is listed as reviewer in MAINTAINERS, 
so included him in the list of receiver.
get_maintainer.pl should have told you that.

Sam

> ---
>  drivers/gpu/drm/ast/ast_tables.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/ast/ast_tables.h 
> b/drivers/gpu/drm/ast/ast_tables.h
> index d665dd5af5dd..8414e543f260 100644
> --- a/drivers/gpu/drm/ast/ast_tables.h
> +++ b/drivers/gpu/drm/ast/ast_tables.h
> @@ -282,6 +282,8 @@ static const struct ast_vbios_enhtable 
> res_1360x768[] = {  };
> 
>  static const struct ast_vbios_enhtable res_1600x900[] = {
> + {1800, 1600, 24, 80,1000,  900, 1, 3, VCLK108,  /* 60Hz */
> +  (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | 
> +NewModeInfo), 60, 3, 0x3A },
>   {1760, 1600, 48, 32, 926, 900, 3, 5, VCLK97_75, /* 60Hz CVT RB 
> */
>(SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo |
> AST2500PreCatchCRT), 60, 1, 0x3A },
> --
> 2.18.4
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


RE: [PATCH] drm/ast: Avoid to access BMC addressing when P2A is disabled

2020-10-19 Thread Kuo-Hsiang Chou
To Thomas,

Thanks and got it.

Regards,
Kuo-Hsiang Chou

-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Monday, October 19, 2020 3:42 PM
To: Kuo-Hsiang Chou ; David Airlie 

Cc: Jenmin Yuan ; e...@suse.com; Arc Sung 
; dri-devel ; Tommy 
Huang 
Subject: Re: [PATCH] drm/ast: Avoid to access BMC addressing when P2A is 
disabled

Hi

On 19.10.20 09:21, Kuo-Hsiang Chou wrote:
> To Sirs,
> 
> Thanks for your answers about the patch and next upstream to kernel-5.9 must 
> follow these rules.

Thanks for wanting to contribute.

> 
> This patch based on kernel-4.9.237 is required by our customers, because my 
> customers said the native kernel of RHEL/CentOS 7.x is 3.10 and is required 
> to update to 4.9.
> 
> BTW, how to verify the correctness before upstream my patch . As I known, the 
> kernel version of RHEL8.2 is v4.18, so I need to update the kernel to v5.9 by 
> myself, right?
> Or should I test my patch on Fedora-rawhide whose kernel is v5.10.0?

v4.9 is way too old for active development. DRM drivers are developed in 
drm-tip, which is available at

  git://anongit.freedesktop.org/drm/drm-tip

It usually contains the latest Linux kernel plus graphics drivers. 
Patches merged into DRM move into the official Linux kernel within 2 to
3 months. From there, bug fixes move into the older versions, such as v4.9.

I know that this looks like overhead in the short term. But long-term your 
customers will again update their kernels and then you already have the 
necessary changes available and merged.

To get started, I'd advice to get the kernel in the drm-tip git repo and build 
it on your distribution (RHEL/CentOS). That should still be possible. Then 
recreate the individual changes on top of the most recent version. Each change 
has to go into it's own commit with a nice commit message.

And that's it basically. Once you have the changes ready, you can send them out 
for review.

Best regards
Thomas

> 
> Regards and have a good day,
>   Kuo-Hsiang Chou
> 
> -Original Message-
> From: Thomas Zimmermann [mailto:tzimmerm...@suse.de]
> Sent: Monday, October 19, 2020 3:08 PM
> To: David Airlie ; Kuo-Hsiang Chou 
> 
> Cc: Jenmin Yuan ; e...@suse.com; Arc Sung 
> ; dri-devel 
> ; Tommy Huang 
> 
> Subject: Re: [PATCH] drm/ast: Avoid to access BMC addressing when P2A 
> is disabled
> 
> Hi
> 
> On 16.10.20 23:01, David Airlie wrote:
>> On Fri, Oct 16, 2020 at 6:03 PM KuoHsiang Chou 
>>  wrote:
>>>
>>> The patch is upstreamed
>>> 1. For RHEL7.x, because its native kernel is suggested to update
>>> from 3.10 to 4.9 on 2 ODM's platform.
>>> 2. For AST2600.
>>> 3. For ASTDP.
>>> 4. v1.11
>>
>> Hi,
>>
>> I've cc'ed Thomas who is maintaining this upstream, but this patch in 
>> it's current form isn't acceptable, Maybe Thomas can provide more 
>> detailed info, but the basic rules are
>>
>> One patch should do one thing.
>> Patches should be bisectable so any issues can be tracked down easier.
>> Fixes should be separated from new code.
>> New features and chip support should be separate.
>>
>> https://www.kernel.org/doc/html/v5.9/process/submitting-patches.html
> 
> Please follow this document when submitting patches.
> 
>>
>> Please maybe work with Thomas on having a better upstream development 
>> process for ast chipsets.
> 
> I'd welcome more support and contributions from Aspeed developers. But in its 
> current form, the patch is not acceptable.
> 
> Best regards
> Thomas
> 
>>
>> Thanks,
>> Dave.
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


RE: [PATCH] drm/ast: Avoid to access BMC addressing when P2A is disabled

2020-10-19 Thread Kuo-Hsiang Chou
To Sirs, 

Thanks for your answers about the patch and next upstream to kernel-5.9 must 
follow these rules.

This patch based on kernel-4.9.237 is required by our customers, because my 
customers said the native kernel of RHEL/CentOS 7.x is 3.10 and is required to 
update to 4.9.

BTW, how to verify the correctness before upstream my patch . As I known, the 
kernel version of RHEL8.2 is v4.18, so I need to update the kernel to v5.9 by 
myself, right?
Or should I test my patch on Fedora-rawhide whose kernel is v5.10.0?

Regards and have a good day,
Kuo-Hsiang Chou

-Original Message-
From: Thomas Zimmermann [mailto:tzimmerm...@suse.de] 
Sent: Monday, October 19, 2020 3:08 PM
To: David Airlie ; Kuo-Hsiang Chou 

Cc: Jenmin Yuan ; e...@suse.com; Arc Sung 
; dri-devel ; Tommy 
Huang 
Subject: Re: [PATCH] drm/ast: Avoid to access BMC addressing when P2A is 
disabled

Hi

On 16.10.20 23:01, David Airlie wrote:
> On Fri, Oct 16, 2020 at 6:03 PM KuoHsiang Chou 
>  wrote:
>>
>> The patch is upstreamed
>> 1. For RHEL7.x, because its native kernel is suggested to update
>>from 3.10 to 4.9 on 2 ODM's platform.
>> 2. For AST2600.
>> 3. For ASTDP.
>> 4. v1.11
> 
> Hi,
> 
> I've cc'ed Thomas who is maintaining this upstream, but this patch in 
> it's current form isn't acceptable, Maybe Thomas can provide more 
> detailed info, but the basic rules are
> 
> One patch should do one thing.
> Patches should be bisectable so any issues can be tracked down easier.
> Fixes should be separated from new code.
> New features and chip support should be separate.
> 
> https://www.kernel.org/doc/html/v5.9/process/submitting-patches.html

Please follow this document when submitting patches.

> 
> Please maybe work with Thomas on having a better upstream development 
> process for ast chipsets.

I'd welcome more support and contributions from Aspeed developers. But in its 
current form, the patch is not acceptable.

Best regards
Thomas

> 
> Thanks,
> Dave.
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel