[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1114806: Bisect result



Hi Niklas,

On Sat, Sep 13, 2025 at 08:23:01PM +0200, Salvatore Bonaccorso wrote:
> Hi Niklas,
> 
> On Fri, Sep 12, 2025 at 08:02:02PM +0200, Niklas Cathor wrote:
> > Hi Salvatore,
> > 
> > I encountered the same issue, and was able to bisect. I'm pasting the result
> > below.
> > Thank you for looking into this. Let me know if I should report it upstream
> > instead.
> > 
> > cheers,
> > Niklas
> > 
> > 
> > 165a69a87d6bde85cac2c051fa6da611ca4524f6 is the first bad commit
> > commit 165a69a87d6bde85cac2c051fa6da611ca4524f6 (HEAD)
> > Author: Lijo Lazar <lijo.lazar@amd.com>
> > Date:   Mon Jun 2 12:55:14 2025 +0530
> > 
> >     drm/amdgpu: Add more checks to PSP mailbox
> > 
> >     [ Upstream commit 8345a71fc54b28e4d13a759c45ce2664d8540d28 ]
> > 
> >     Instead of checking the response flag, use status mask also to check
> >     against any unexpected failures like a device drop. Also, log error if
> >     waiting on a psp response fails/times out.
> > 
> >     Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
> >     Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>
> >     Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >     Signed-off-by: Sasha Levin <sashal@kernel.org>
> > 
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c  |  4 ++++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h  | 11 +++++++++++
> >  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c   |  4 ++--
> >  drivers/gpu/drm/amd/amdgpu/psp_v11_0.c   | 31
> > +++++++++++++++++++------------
> >  drivers/gpu/drm/amd/amdgpu/psp_v11_0_8.c | 25 +++++++++++++++----------
> >  drivers/gpu/drm/amd/amdgpu/psp_v12_0.c   | 18 +++++++++++-------
> >  drivers/gpu/drm/amd/amdgpu/psp_v13_0.c   | 25 +++++++++++++++----------
> >  drivers/gpu/drm/amd/amdgpu/psp_v13_0_4.c | 25 +++++++++++++++----------
> >  drivers/gpu/drm/amd/amdgpu/psp_v14_0.c   | 25 +++++++++++++++----------
> >  9 files changed, 107 insertions(+), 61 deletions(-)
> 
> Ok that is great you found the offending commit. Can you try if
> applying 440cec4ca1c2 ("drm/amdgpu: Wait for bootloader after PSPv11
> reset") fixes the issue?

One thing: the commit won't apply cleanly pre 9888f73679b7
("drm/amdgpu: Add a noverbose flag to psp_wait_for") changes. So
either test mainline at the commit and the previous comit to confirm
the fix, and if possible then still with a backported variant.

An attempt of it is attached here which should apply on top of
6.16.7-1.

Regards,
Salvatore
From: Lijo Lazar <lijo.lazar@amd.com>
Date: Fri, 18 Jul 2025 18:50:58 +0530
Subject: drm/amdgpu: Wait for bootloader after PSPv11 reset
Origin: https://git.kernel.org/linus/440cec4ca1c242d72e309a801995584a55af25c6
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/4531
Bug-Debian: https://bugs.debian.org/1114806

Some PSPv11 SOCs take a longer time for PSP based mode-1 reset. Instead
of checking for C2PMSG_33 status, add the callback wait_for_bootloader.
Wait for bootloader to be back to steady state is already part of the
generic mode-1 reset flow. Increase the retry count for bootloader wait
and also fix the mask to prevent fake pass.

Fixes: 8345a71fc54b ("drm/amdgpu: Add more checks to PSP mailbox")
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4531
Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
(cherry picked from commit 32f73741d6ee41fd5db8791c1163931e313d0fdc)
[Salvatore Bonaccorso: Backport for v6.16: Context changes for code
before 9888f73679b7 ("drm/amdgpu: Add a noverbose flag to psp_wait_for")]
---
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -149,13 +149,13 @@ static int psp_v11_0_wait_for_bootloader
 	int ret;
 	int retry_loop;
 
-	for (retry_loop = 0; retry_loop < 10; retry_loop++) {
+	for (retry_loop = 0; retry_loop < 20; retry_loop++) {
 		/* Wait for bootloader to signify that is
 		    ready having bit 31 of C2PMSG_35 set to 1 */
 		ret = psp_wait_for(psp,
 				   SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_35),
 				   0x80000000,
-				   0x80000000,
+				   0x8000FFFF,
 				   false);
 
 		if (ret == 0)
@@ -399,18 +399,6 @@ static int psp_v11_0_mode1_reset(struct
 
 	msleep(500);
 
-	offset = SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_33);
-
-	ret = psp_wait_for(psp, offset, MBOX_TOS_RESP_FLAG, MBOX_TOS_RESP_MASK,
-			   false);
-
-	if (ret) {
-		DRM_INFO("psp mode 1 reset failed!\n");
-		return -EINVAL;
-	}
-
-	DRM_INFO("psp mode1 reset succeed \n");
-
 	return 0;
 }
 
@@ -666,7 +654,8 @@ static const struct psp_funcs psp_v11_0_
 	.ring_get_wptr = psp_v11_0_ring_get_wptr,
 	.ring_set_wptr = psp_v11_0_ring_set_wptr,
 	.load_usbc_pd_fw = psp_v11_0_load_usbc_pd_fw,
-	.read_usbc_pd_fw = psp_v11_0_read_usbc_pd_fw
+	.read_usbc_pd_fw = psp_v11_0_read_usbc_pd_fw,
+	.wait_for_bootloader = psp_v11_0_wait_for_bootloader
 };
 
 void psp_v11_0_set_psp_funcs(struct psp_context *psp)

Reply to: