| Message ID | 20230613030151.216625-1-15330273260@189.cn |
|---|---|
| Headers | show |
| Series | PCI/VGA: Introduce is_boot_device function callback to vga_client_register | expand |
PING ? On 2023/6/13 11:01, Sui Jingfeng wrote: > From: Sui Jingfeng <suijingfeng@loongson.cn> > > [why] > > The vga_is_firmware_default() defined in drivers/pci/vgaarb.c is > arch-dependent, it's a dummy on non-x86 architectures currently. > This made VGAARB lost an important condition for the arbitration. > It could still be wrong even if we remove the #ifdef and #endif guards. > because the PCI bar will move (resource re-allocation). > > [how] > > The device that owns the firmware framebuffer should be the default boot > device. This patch adds an arch-independent function to enforce this rule
PING ? On 2023/6/13 11:01, Sui Jingfeng wrote: > From: Sui Jingfeng <suijingfeng@loongson.cn> > > This patch adds the aperture_contain_firmware_fb() function to do the > determination. Unfortunately due to the fact that apertures list will be > freed dynamically, the location and size information of the firmware fb > will be lost after dedicated drivers call > aperture_remove_conflicting_devices(), > aperture_remove_conflicting_pci_devices() or > aperture_remove_all_conflicting_devices() functions > > We handle this problem by introducing two static variables which record the > firmware framebuffer's start addrness and end addrness. It assumes that the > system has only one active firmware framebuffer driver at a time. > > We don't use the global structure screen_info here, because PCI resource > may get reallocated(the VRAM BAR could be moved) at kernel boot stage. > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Javier Martinez Canillas <javierm@redhat.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: David Airlie <airlied@gmail.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Helge Deller <deller@gmx.de> > Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> > --- > drivers/gpu/drm/drm_aperture.c | 16 ++++++++++++++++ > drivers/video/aperture.c | 29 +++++++++++++++++++++++++++++ > include/drm/drm_aperture.h | 2 ++ > include/linux/aperture.h | 7 +++++++ > 4 files changed, 54 insertions(+) > > diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c > index 5729f3bb4398..6e5d8a08683c 100644 > --- a/drivers/gpu/drm/drm_aperture.c > +++ b/drivers/gpu/drm/drm_aperture.c > @@ -190,3 +190,19 @@ int drm_aperture_remove_conflicting_pci_framebuffers(struct pci_dev *pdev, > return aperture_remove_conflicting_pci_devices(pdev, req_driver->name); > } > EXPORT_SYMBOL(drm_aperture_remove_conflicting_pci_framebuffers); > + > +/** > + * drm_aperture_contain_firmware_fb - Determine if a aperture contains firmware framebuffer > + * > + * @base: the aperture's base address in physical memory > + * @size: aperture size in bytes > + * > + * Returns: > + * true on if there is a firmware framebuffer belong to the aperture passed in, > + * or false otherwise. > + */ > +bool drm_aperture_contain_firmware_fb(resource_size_t base, resource_size_t size) > +{ > + return aperture_contain_firmware_fb(base, base + size); > +} > +EXPORT_SYMBOL(drm_aperture_contain_firmware_fb); > diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c > index 561be8feca96..5a5422cec669 100644 > --- a/drivers/video/aperture.c > +++ b/drivers/video/aperture.c > @@ -141,6 +141,9 @@ struct aperture_range { > static LIST_HEAD(apertures); > static DEFINE_MUTEX(apertures_lock); > > +static resource_size_t firm_fb_start; > +static resource_size_t firm_fb_end; > + > static bool overlap(resource_size_t base1, resource_size_t end1, > resource_size_t base2, resource_size_t end2) > { > @@ -170,6 +173,9 @@ static int devm_aperture_acquire(struct device *dev, > > mutex_lock(&apertures_lock); > > + firm_fb_start = base; > + firm_fb_end = end; > + > list_for_each(pos, &apertures) { > ap = container_of(pos, struct aperture_range, lh); > if (overlap(base, end, ap->base, ap->base + ap->size)) { > @@ -377,3 +383,26 @@ int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *na > > } > EXPORT_SYMBOL(aperture_remove_conflicting_pci_devices); > + > +/** > + * aperture_contain_firmware_fb - Detect if the firmware framebuffer belong to > + * a aperture. > + * @ap_start: the aperture's start address in physical memory > + * @ap_end: the aperture's end address in physical memory > + * > + * Returns: > + * true on if there is a firmware framebuffer belong to the aperture passed in, > + * or false otherwise. > + */ > +bool aperture_contain_firmware_fb(resource_size_t ap_start, resource_size_t ap_end) > +{ > + /* No firmware framebuffer support */ > + if (!firm_fb_start || !firm_fb_end) > + return false; > + > + if (firm_fb_start >= ap_start && firm_fb_end <= ap_end) > + return true; > + > + return false; > +} > +EXPORT_SYMBOL(aperture_contain_firmware_fb); > diff --git a/include/drm/drm_aperture.h b/include/drm/drm_aperture.h > index cbe33b49fd5d..6a0b9bacb081 100644 > --- a/include/drm/drm_aperture.h > +++ b/include/drm/drm_aperture.h > @@ -35,4 +35,6 @@ drm_aperture_remove_framebuffers(const struct drm_driver *req_driver) > req_driver); > } > > +bool drm_aperture_contain_firmware_fb(resource_size_t base, resource_size_t size); > + > #endif > diff --git a/include/linux/aperture.h b/include/linux/aperture.h > index 1a9a88b11584..d4dc5917c49b 100644 > --- a/include/linux/aperture.h > +++ b/include/linux/aperture.h > @@ -19,6 +19,8 @@ int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t si > int __aperture_remove_legacy_vga_devices(struct pci_dev *pdev); > > int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *name); > + > +bool aperture_contain_firmware_fb(resource_size_t ap_start, resource_size_t ap_end); > #else > static inline int devm_aperture_acquire_for_platform_device(struct platform_device *pdev, > resource_size_t base, > @@ -42,6 +44,11 @@ static inline int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, > { > return 0; > } > + > +static inline bool aperture_contain_firmware_fb(resource_size_t ap_start, resource_size_t ap_end) > +{ > + return false; > +} > #endif > > /**
The vga_is_firmware_default() function is arch-dependent, it's probably wrong if we simply remove the arch guard. As the VRAM BAR which contains firmware framebuffer may move, while the lfb_base and lfb_size members of the screen_info does not change accordingly. In short, it should take the re-allocation of the PCI BAR into consideration. With the observation that device drivers or video aperture helpers may have better knowledge about which PCI bar contains the firmware fb, which could avoid the need to iterate all of the PCI BARs. But as a PCI function at pci/vgaarb.c, vga_is_firmware_default() is not suitable to make such an optimization since it is loaded too early. There are PCI display controllers that don't have a dedicated VRAM bar, this function will lose its effectiveness in such a case. Luckily, the device driver can provide an accurate workaround. Therefore, this patch introduces a callback that allows the device driver to tell the VGAARB if the device is the default boot device. Also honor the comment: "Clients have two callback mechanisms they can use" Sui Jingfeng (8): PCI/VGA: Use unsigned type for the io_state variable PCI/VGA: Deal only with VGA class devices PCI/VGA: Tidy up the code and comment format PCI/VGA: Replace full MIT license text with SPDX identifier video/aperture: Add a helper to detect if an aperture contains firmware FB PCI/VGA: Introduce is_boot_device function callback to vga_client_register drm/amdgpu: Implement the is_boot_device callback function drm/radeon: Implement the is_boot_device callback function drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +- drivers/gpu/drm/drm_aperture.c | 16 +++ drivers/gpu/drm/i915/display/intel_vga.c | 3 +- drivers/gpu/drm/nouveau/nouveau_vga.c | 2 +- drivers/gpu/drm/radeon/radeon_device.c | 12 +- drivers/pci/vgaarb.c | 153 +++++++++++++-------- drivers/vfio/pci/vfio_pci_core.c | 2 +- drivers/video/aperture.c | 29 ++++ include/drm/drm_aperture.h | 2 + include/linux/aperture.h | 7 + include/linux/vgaarb.h | 35 ++--- 11 files changed, 184 insertions(+), 89 deletions(-)