On Thu, Sep 24, 2026 at 11:06 AM Christian König <[email protected]> wrote: > > 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?
Sounds like crazy Mac specific behavior. The asic_init bios table has to reprogram the FB aperture otherwise the pre-OS environment won't work. Moreover, at some point, these registers became privileged such that only firmware can program them. I agree that running asic_init on boot for Macs would make sense. I think we already have a similar workaround for some other mac models, probably to work around a similar issue: https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a5be7ad8f5f0e067613e9197638f216f46252946 Maybe add your device to that existing quirk or make the quirk generic for all VI boards with apple ssids? Alex > > 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); >
