On 9/24/26 15:29, Francisco Beltrán Millalén wrote: > On a MacBookPro14,3 (Radeon Pro 560, POLARIS11) amdgpu has never recovered > from an ASIC reset: five attempts recorded, zero successes. Since suspend > to RAM goes through a reset, S3 fails the same way, and as the internal > panel hangs off the AMD GPU the machine comes back blind. > > The failure looks like VRAM going write-only-dead: writes are silently > dropped while reads still work, the driver reports success at every step, > and then it hands the SMU a pointer to a table that was never written: > > amdgpu_device_asic_init() -> 0 (reports success) > gmc_v8_0_hw_init() -> 0 (reports success) > memcpy_toio() (write silently discarded) > send_msg(0x251, ...) (SMU parses garbage) > smu7_check_fw_load_finish() -> -EINVAL -> black screen > > It is not VRAM dying. It is the framebuffer moving. > > On this machine the Apple firmware places VRAM at MC address 0 on a cold > boot, and gmc_v8_0_vram_gtt_location() reads MC_VM_FB_LOCATION once, at > init, to derive vram_start. A re-POST -- which is what an ASIC reset and > an S3 resume both trigger -- lets the VBIOS put the framebuffer back at > its own default instead, 0xf400_0000 here: > > cold boot: MC_VM_FB_LOCATION = 0x007f0000 > after reset: MC_VM_FB_LOCATION = 0xf47ff400
Mhm, interesting I'm really wondering where those values come from. > > gmc_v8_0_mc_program() programs the system aperture from the stale > vram_start, but only writes MC_VM_FB_LOCATION and HDP_NONSURFACE_BASE > under SR-IOV; on bare metal it trusts whatever the VBIOS left behind. > While the MC is still in pass-through everything appears to work, so the > mismatch goes unnoticed. Then gmc_v8_0_gart_enable() sets ENABLE_L1_TLB, > SYSTEM_ACCESS_MODE=3 and ENABLE_ADVANCED_DRIVER_MODEL, the MC starts > checking the system aperture, and every access lands outside it -- which > is why reads return data written before the reset, from a different > physical place than the writes are going to. > > Write the framebuffer location back when it does not match the one the > driver is working with, which is what the SR-IOV path already does. The > comparison keeps this a no-op on machines where the VBIOS restores the > same location, so nothing changes for them. That is still a rather bad idea for multiple reasons. You often run into suspend/resume and random memory corruption issues when stuff like that is done and we never fully implemented blocking VRAM access during a runtime ASIC reset. I think the more defensive approach is to do an ASIC reset on driver load and use the values the AtomBIOS init function comes up with. @Alex what's your take here? Regards, Christian. > > This runs after the VGA aperture has been locked out and with the display > suspended, so the MC does not need to be stopped; only CPU access through > the BAR could land while the FB and HDP bases disagree, so BIF_FB_EN is > cleared around the update and re-enabled below. > > With this the GPU survives resets and S3: the machine has since completed > twelve suspend/resume cycles in a single boot without a failure, and the > restore is visible on each resume: > > amdgpu 0000:01:00.0: amdgpu: FB location 0xf47ff400 does not match > vram_start, restoring 0x007f0000 > > To be precise about what those cycles prove: the kernel they were run on > also carries unrelated local patches for this machine's Thunderbolt > controller, which fails separately. This patch is the one that brings the > display back -- without it the GPU never recovered from a reset at all. > > Tested on 6.18.49 on a MacBookPro14,3. I have no other smu7 hardware, so > this is only known to matter on machines whose firmware boots the GPU at a > different framebuffer location than the VBIOS default; elsewhere the new > branch does nothing. > > Signed-off-by: Francisco Beltrán Millalén <[email protected]> > --- > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c > @@ -472,14 +472,42 @@ > WREG32(mmMC_VM_SYSTEM_APERTURE_DEFAULT_ADDR, > adev->mem_scratch.gpu_addr >> 12); > > + tmp = ((adev->gmc.vram_end >> 24) & 0xFFFF) << 16; > + tmp |= ((adev->gmc.vram_start >> 24) & 0xFFFF); > + > if (amdgpu_sriov_vf(adev)) { > - tmp = ((adev->gmc.vram_end >> 24) & 0xFFFF) << 16; > - tmp |= ((adev->gmc.vram_start >> 24) & 0xFFFF); > WREG32(mmMC_VM_FB_LOCATION, tmp); > /* XXX double check these! */ > WREG32(mmHDP_NONSURFACE_BASE, (adev->gmc.vram_start >> 8)); > WREG32(mmHDP_NONSURFACE_INFO, (2 << 7) | (1 << 30)); > WREG32(mmHDP_NONSURFACE_SIZE, 0x3FFFFFFF); > + } else { > + u32 fb_loc = RREG32(mmMC_VM_FB_LOCATION); > + > + /* > + * On bare metal vram_start is the FB base found at init (see > + * gmc_v8_0_vram_gtt_location()). Normally the VBIOS put it > + * there and a later re-POST puts it back in the same place. > + * On MacBookPros with switchable graphics VRAM is at 0 at > boot > + * instead, and a re-POST (S3 resume, ASIC reset) moves it to > + * the VBIOS default, away from the addresses the driver > + * already uses. Move it back. > + * > + * This only happens after a re-POST: the display is suspended > + * and the VGA aperture has been locked out above, so there is > + * no need to stop the MC. Only CPU access through the BAR > + * could land while the FB and HDP bases disagree, so block it > + * here; BIF_FB_EN is enabled again below. > + */ > + if (REG_GET_FIELD(fb_loc, MC_VM_FB_LOCATION, FB_BASE) != > + REG_GET_FIELD(tmp, MC_VM_FB_LOCATION, FB_BASE)) { > + dev_info(adev->dev, > + "FB location 0x%08x does not match > vram_start, restoring 0x%08x\n", > + fb_loc, tmp); > + WREG32(mmBIF_FB_EN, 0); > + WREG32(mmMC_VM_FB_LOCATION, tmp); > + WREG32(mmHDP_NONSURFACE_BASE, (adev->gmc.vram_start > >> 8)); > + } > } > > WREG32(mmMC_VM_AGP_BASE, 0);
