On Wed, Jan 17, 2024 at 2:42 AM Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> wrote:
>
> Am 16.01.24 um 15:39 schrieb Alex Deucher:
> > Seems calling drm_aperture_remove_conflicting_pci_framebuffers()
> > will take away the apertures for unrelated devices on some kernel
> > versions. E.g., calling this on a PCIe accelerator with no display
> > IP may take the apertures away from the actual PCIe display adapter
> > on the system, breaking efifb, depending on the kernel version.
> >
> > Just do this if there is display IP present.
>
> I would have checked the PCI device type instead, because a system BIOS
> most likely has no idea that a VGA device doesn't has a connector.
True, but we have GPUs of PCI class PCI_CLASS_DISPLAY_OTHER that can
have efifb bound to it. Unfortunately, the combinations of PCI
classes and displays is complicated:
PCI_CLASS_DISPLAY_VGA - boards both with and without display hardware
PCI_CLASS_DISPLAY_OTHER - boards both with and without display hardware
PCI_CLASS_ACCELERATOR_PROCESSING - no display hardware
>
> > Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 62772b58ef3d..353c38f008e8 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -4056,10 +4056,12 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> >
> > amdgpu_device_set_mcbp(adev);
> >
> > - /* Get rid of things like offb */
> > - r = drm_aperture_remove_conflicting_pci_framebuffers(adev->pdev, &amdgpu_kms_driver);
> > - if (r)
> > - return r;
> > + if (amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_DCE)) {
>
> This certainly worth a comment why we do this.
>
> Apart from that I'm not sure we should upstream this, the customer
> kernel is most likely just missing this fix here:
>
> commit 5ae3716cfdcd286268133867f67d0803847acefc
> Author: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Date: Thu Apr 6 15:21:07 2023 +0200
>
> video/aperture: Only remove sysfb on the default vga pci device
>
> Instead of calling aperture_remove_conflicting_devices() to remove the
> conflicting devices, just call to aperture_detach_devices() to detach
> the device that matches the same PCI BAR / aperture range. Since the
> former is just a wrapper of the latter plus a sysfb_disable() call,
> and now that's done in this function but only for the primary devices.
>
> This fixes a regression introduced by commit ee7a69aa38d8 ("fbdev:
> Disable sysfb device registration when removing conflicting FBs"),
> where we remove the sysfb when loading a driver for an unrelated pci
> device, resulting in the user losing their efifb console or similar.
>
> Note that in practice this only is a problem with the nvidia blob,
> because that's the only gpu driver people might install which does not
> come with an fbdev driver of it's own. For everyone else the real gpu
> driver will restore a working console.
>
> Also note that in the referenced bug there's confusion that this same
> bug also happens on amdgpu. But that was just another amdgpu specific
> regression, which just happened to happen at roughly the same time and
> with the same user-observable symptoms. That bug is fixed now, see
> https://bugzilla.kernel.org/show_bug.cgi?id=216331#c15
>
yeah, that looks like the right fix.
Alex
> Regards,
> Christian.
>
> > + /* Get rid of things like offb */
> > + r = drm_aperture_remove_conflicting_pci_framebuffers(adev->pdev, &amdgpu_kms_driver);
> > + if (r)
> > + return r;
> > + }
> >
> > /* Enable TMZ based on IP_VERSION */
> > amdgpu_gmc_tmz_set(adev);
>
