From: Sui Jingfeng <[email protected]>
Currently, the default VGA device selection is not perfect. Potential
problems are:
1) This function is a no-op on non-x86 architectures.
2) It does not take the PCI Bar may get relocated into consideration.
3) It is not effective for the PCI device without a dedicated VRAM Bar.
4) It is device-agnostic, thus it has to waste the effort to iterate all
of the PCI Bar to find the VRAM aperture.
5) It has invented lots of methods to determine which one is the default
boot device on a multiple video card coexistence system. But this is
still a policy because it doesn't give the user a choice to override.
With the observation that device drivers or video aperture helpers may
have better knowledge about which PCI bar contains the firmware FB,
This patch tries to solve the above problems by introducing a function
callback to the vga_client_register() function interface. DRM device
drivers for the PCI device need to register the is_boot_device() function
callback during the driver loading time. Once the driver binds the device
successfully, VRAARB will call back to the driver. This gives the device
drivers a chance to provide accurate boot device identification. Which in
turn unlock the abitration service to non-x86 architectures. A device
driver can also pass a NULL pointer to keep the original behavior.
This series is applied on the drm-tip branch (with a cleanup patch set[1]
applied beforehand)
[1] https://patchwork.freedesktop.org/series/120548/
v2:
* Add a simple implemment for drm/i915 and drm/ast
* Pick up all tags (Mario)
v3:
* Fix a mistake for drm/i915 implement
* Fix patch can not be applied problem because of drm/amdgpu merged
other people's patch.
Sui Jingfeng (9):
video/aperture: Add a helper to detect if an aperture contains
firmware FB
video/aperture: Add a helper for determining if an unmoved aperture
contain FB
PCI/VGA: Switch to aperture_contain_firmware_fb_nonreloc()
PCI/VGA: Improve the default VGA device selection
drm/amdgpu: Implement the is_primary_gpu callback of
vga_client_register()
drm/radeon: Add an implement for the is_primary_gpu function callback
drm/i915: Add an implement for the is_primary_gpu hook
drm/ast: Register as a vga client to vgaarb by calling
vga_client_register()
drm/loongson: Add an implement for the is_primary_gpu function
callback
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++-
drivers/gpu/drm/ast/ast_drv.c | 29 +++++++++
drivers/gpu/drm/drm_aperture.c | 16 +++++
drivers/gpu/drm/i915/display/intel_vga.c | 31 ++++++++-
drivers/gpu/drm/loongson/lsdc_drv.c | 10 ++-
drivers/gpu/drm/nouveau/nouveau_vga.c | 2 +-
drivers/gpu/drm/radeon/radeon_device.c | 12 +++-
drivers/pci/vgaarb.c | 74 ++++++++++++++++------
drivers/vfio/pci/vfio_pci_core.c | 2 +-
drivers/video/aperture.c | 65 +++++++++++++++++++
include/drm/drm_aperture.h | 2 +
include/linux/aperture.h | 14 ++++
include/linux/vgaarb.h | 8 ++-
13 files changed, 247 insertions(+), 30 deletions(-)
--
2.25.1
From: Sui Jingfeng <[email protected]>
This patch adds the aperture_contain_firmware_fb() function to do the
determination. Unfortunately, due to the fact that the 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 solve this problem by introducing two static variables that 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 resources may get
reallocated (the VRAM BAR could be moved) during the kernel boot stage.
Cc: Thomas Zimmermann <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Helge Deller <[email protected]>
Signed-off-by: Sui Jingfeng <[email protected]>
---
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..f9c957aa5874 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 if there exist a firmware framebuffer inside of 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..34eb962cfae8 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 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
/**
--
2.25.1
From: Sui Jingfeng <[email protected]>
[why]
The vga_is_firmware_default() function defined in drivers/pci/vgaarb.c is
arch-dependent, it's a dummy on non-x86 architectures. This made VGAARB
lost an important condition for the arbitration on non-x86 platform. The
rules about which GPU is (or should be) the primary display device get used
by userspace are obscure on non-x86 platform, let's made the things clear.
[how]
The device that owns the firmware framebuffer should be the default boot
device. This patch adds an arch-independent function to implement this
rule. The vgaarb subsystem will call back to amdgpu_is_primary_gpu() when
drm/amdgpu is bound to an AMDGPU device successfully.
Cc: Alex Deucher <[email protected]>
Cc: Christian Konig <[email protected]>
Cc: Pan Xinhui <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Hawking Zhang <[email protected]>
Cc: Mario Limonciello <[email protected]>
Cc: Lijo Lazar <[email protected]>
Cc: YiPeng Chai <[email protected]>
Cc: Bokun Zhang <[email protected]>
CC: Likun Gao <[email protected]>
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index d98f0801ac77..b638eff58636 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3690,6 +3690,15 @@ static void amdgpu_device_set_mcbp(struct amdgpu_device *adev)
DRM_INFO("MCBP is enabled\n");
}
+static bool amdgpu_is_primary_gpu(struct pci_dev *pdev)
+{
+ struct drm_device *dev = pci_get_drvdata(pdev);
+ struct amdgpu_device *adev = drm_to_adev(dev);
+ struct amdgpu_gmc *gmc = &adev->gmc;
+
+ return drm_aperture_contain_firmware_fb(gmc->aper_base, gmc->aper_size);
+}
+
/**
* amdgpu_device_init - initialize the driver
*
@@ -4103,7 +4112,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
/* this will fail for cards that aren't VGA class devices, just
* ignore it */
if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
- vga_client_register(adev->pdev, amdgpu_device_vga_set_decode, NULL);
+ vga_client_register(adev->pdev, amdgpu_device_vga_set_decode,
+ amdgpu_is_primary_gpu);
px = amdgpu_device_supports_px(ddev);
--
2.25.1
From: Sui Jingfeng <[email protected]>
This patch is intended to form a uniform approach to determining if an
unmoved aperture contains the firmware FB. I believe that the global
screen_info is more about video-specific things.
Putting it in video/aperture.c helps form a uniform approach.
Cc: Thomas Zimmermann <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Cc: Helge Deller <[email protected]>
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/video/aperture.c | 36 ++++++++++++++++++++++++++++++++++++
include/linux/aperture.h | 7 +++++++
2 files changed, 43 insertions(+)
diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index 34eb962cfae8..f03dfcabc303 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -6,6 +6,7 @@
#include <linux/mutex.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
+#include <linux/screen_info.h>
#include <linux/slab.h>
#include <linux/sysfb.h>
#include <linux/types.h>
@@ -406,3 +407,38 @@ bool aperture_contain_firmware_fb(resource_size_t ap_start, resource_size_t ap_e
return false;
}
EXPORT_SYMBOL(aperture_contain_firmware_fb);
+
+/**
+ * aperture_contain_firmware_fb_nonreloc - Detect if the firmware framebuffer
+ * belong to a non-relocatable aperture, such as the aperture of platform
+ * device. Note that this function relay on the global screen info.
+ * @ap_start: the aperture's start address in physical memory
+ * @ap_end: the aperture's end address in physical memory
+ *
+ * Returns:
+ * true if there is a firmware framebuffer belong to the aperture passed in,
+ * or false otherwise.
+ */
+bool aperture_contain_firmware_fb_nonreloc(resource_size_t ap_start, resource_size_t ap_end)
+{
+ u64 fb_start;
+ u64 fb_end;
+
+ if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE) {
+ fb_start = (u64)screen_info.ext_lfb_base << 32 | screen_info.lfb_base;
+ fb_end = fb_start + screen_info.lfb_size;
+ } else {
+ fb_start = screen_info.lfb_base;
+ fb_end = fb_start + screen_info.lfb_size;
+ }
+
+ /* No firmware framebuffer support */
+ if (!fb_start || !fb_end)
+ return false;
+
+ if (fb_start >= ap_start && fb_end <= ap_end)
+ return true;
+
+ return false;
+}
+EXPORT_SYMBOL(aperture_contain_firmware_fb_nonreloc);
diff --git a/include/linux/aperture.h b/include/linux/aperture.h
index d4dc5917c49b..906d23532b56 100644
--- a/include/linux/aperture.h
+++ b/include/linux/aperture.h
@@ -21,6 +21,8 @@ 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);
+
+bool aperture_contain_firmware_fb_nonreloc(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,
@@ -49,6 +51,11 @@ static inline bool aperture_contain_firmware_fb(resource_size_t ap_start, resour
{
return false;
}
+
+static bool aperture_contain_firmware_fb_nonreloc(resource_size_t ap_start, resource_size_t ap_end)
+{
+ return false;
+}
#endif
/**
--
2.25.1
From: Sui Jingfeng <[email protected]>
The observation behind this is that we should avoid accessing the global
screen_info directly. Call the aperture_contain_firmware_fb_nonreloc()
function to implement the detection of whether an aperture contains the
firmware FB.
This patch helps to decouple the determination from the implementation.
Or, in other words, we intend to make the determination opaque to the
caller. The determination may choose to be arch-dependent or
arch-independent. But vgaarb, as a consumer of the determination,
shouldn't care how the does determination is implemented.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/pci/vgaarb.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index bf96e085751d..953daf731b2c 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -14,6 +14,7 @@
#define vgaarb_info(dev, fmt, arg...) dev_info(dev, "vgaarb: " fmt, ##arg)
#define vgaarb_err(dev, fmt, arg...) dev_err(dev, "vgaarb: " fmt, ##arg)
+#include <linux/aperture.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/pci.h>
@@ -26,7 +27,6 @@
#include <linux/poll.h>
#include <linux/miscdevice.h>
#include <linux/slab.h>
-#include <linux/screen_info.h>
#include <linux/vt.h>
#include <linux/console.h>
#include <linux/acpi.h>
@@ -558,20 +558,11 @@ void vga_put(struct pci_dev *pdev, unsigned int rsrc)
}
EXPORT_SYMBOL(vga_put);
+/* Select the device owning the boot framebuffer if there is one */
static bool vga_is_firmware_default(struct pci_dev *pdev)
{
#if defined(CONFIG_X86) || defined(CONFIG_IA64)
- u64 base = screen_info.lfb_base;
- u64 size = screen_info.lfb_size;
struct resource *r;
- u64 limit;
-
- /* Select the device owning the boot framebuffer if there is one */
-
- if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
- base |= (u64)screen_info.ext_lfb_base << 32;
-
- limit = base + size;
/* Does firmware framebuffer belong to us? */
pci_dev_for_each_resource(pdev, r) {
@@ -581,10 +572,8 @@ static bool vga_is_firmware_default(struct pci_dev *pdev)
if (!r->start || !r->end)
continue;
- if (base < r->start || limit >= r->end)
- continue;
-
- return true;
+ if (aperture_contain_firmware_fb_nonreloc(r->start, r->end))
+ return true;
}
#endif
return false;
--
2.25.1