RE: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places
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
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
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
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
-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
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
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
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
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
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
-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
-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
-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
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
-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
-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
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
-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
-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
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
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
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
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
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
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