Sui Jingfeng
2023-Jul-11 16:31 UTC
[Nouveau] [PATCH v3 0/9] PCI/VGA: Improve the default VGA device selection
From: Sui Jingfeng <suijingfeng at loongson.cn>
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
Sui Jingfeng
2023-Jul-11 16:31 UTC
[Nouveau] [PATCH v3 1/9] video/aperture: Add a helper to detect if an aperture contains firmware FB
From: Sui Jingfeng <suijingfeng at loongson.cn>
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 <tzimmermann at suse.de>
Cc: Javier Martinez Canillas <javierm at redhat.com>
Cc: Bjorn Helgaas <bhelgaas at google.com>
Cc: Helge Deller <deller at gmx.de>
Signed-off-by: Sui Jingfeng <suijingfeng at 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..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
Sui Jingfeng
2023-Jul-11 16:31 UTC
[Nouveau] [PATCH v3 2/9] video/aperture: Add a helper for determining if an unmoved aperture contain FB
From: Sui Jingfeng <suijingfeng at loongson.cn>
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 <tzimmermann at suse.de>
Cc: Javier Martinez Canillas <javierm at redhat.com>
Cc: Helge Deller <deller at gmx.de>
Signed-off-by: Sui Jingfeng <suijingfeng at loongson.cn>
---
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
Sui Jingfeng
2023-Jul-11 16:31 UTC
[Nouveau] [PATCH v3 3/9] PCI/VGA: Switch to aperture_contain_firmware_fb_nonreloc()
From: Sui Jingfeng <suijingfeng at loongson.cn>
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 <suijingfeng at loongson.cn>
---
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
Sui Jingfeng
2023-Jul-11 16:31 UTC
[Nouveau] [PATCH v3 4/9] PCI/VGA: Improve the default VGA device selection
From: Sui Jingfeng <suijingfeng at loongson.cn>
Currently, the strategy of selecting the default boot on a multiple video
card coexistence system 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, but this is still a policy because it doesn't give the
user a choice to override.
With the observation that device drivers 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 could provide
a xx_vga_is_primary_gpu() 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 just pass a NULL pointer
to keep the original behavior.
This patch is intended to introducing the mechanism only, the specific
implementation is left to the authors of various device driver. Also honor
the comment: "Clients have *TWO* callback mechanisms they can use"
Cc: Alex Deucher <alexander.deucher at amd.com>
Cc: Christian Konig <christian.koenig at amd.com>
Cc: Pan Xinhui <Xinhui.Pan at amd.com>
Cc: David Airlie <airlied at gmail.com>
Cc: Daniel Vetter <daniel at ffwll.ch>
Cc: Jani Nikula <jani.nikula at linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
Cc: Ben Skeggs <bskeggs at redhat.com>
Cc: Karol Herbst <kherbst at redhat.com>
Cc: Lyude Paul <lyude at redhat.com>
Cc: Bjorn Helgaas <bhelgaas at google.com>
Cc: Alex Williamson <alex.williamson at redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Cc: Maxime Ripard <mripard at kernel.org>
Cc: Thomas Zimmermann <tzimmermann at suse.de>
Cc: Hawking Zhang <Hawking.Zhang at amd.com>
Cc: Mario Limonciello <mario.limonciello at amd.com>
Cc: Lijo Lazar <lijo.lazar at amd.com>
Cc: YiPeng Chai <YiPeng.Chai at amd.com>
Cc: Bokun Zhang <Bokun.Zhang at amd.com>
Cc: Likun Gao <Likun.Gao at amd.com>
Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
Cc: Jason Gunthorpe <jgg at ziepe.ca>
CC: Kevin Tian <kevin.tian at intel.com>
Cc: Cornelia Huck <cohuck at redhat.com>
Cc: Yishai Hadas <yishaih at nvidia.com>
Cc: Abhishek Sahu <abhsahu at nvidia.com>
Cc: Yi Liu <yi.l.liu at intel.com>
Acked-by: Jani Nikula <jani.nikula at intel.com> # i915
Reviewed-by: Lyude Paul <lyude at redhat.com> # nouveau
Signed-off-by: Sui Jingfeng <suijingfeng at loongson.cn>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
drivers/gpu/drm/i915/display/intel_vga.c | 3 +-
drivers/gpu/drm/loongson/lsdc_drv.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_vga.c | 2 +-
drivers/gpu/drm/radeon/radeon_device.c | 2 +-
drivers/pci/vgaarb.c | 55 ++++++++++++++++++++--
drivers/vfio/pci/vfio_pci_core.c | 2 +-
include/linux/vgaarb.h | 8 ++--
8 files changed, 61 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index a92c6189b4b6..d98f0801ac77 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4103,7 +4103,7 @@ 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);
+ vga_client_register(adev->pdev, amdgpu_device_vga_set_decode, NULL);
px = amdgpu_device_supports_px(ddev);
diff --git a/drivers/gpu/drm/i915/display/intel_vga.c
b/drivers/gpu/drm/i915/display/intel_vga.c
index 286a0bdd28c6..98d7d4dffe9f 100644
--- a/drivers/gpu/drm/i915/display/intel_vga.c
+++ b/drivers/gpu/drm/i915/display/intel_vga.c
@@ -115,7 +115,6 @@ intel_vga_set_decode(struct pci_dev *pdev, bool
enable_decode)
int intel_vga_register(struct drm_i915_private *i915)
{
-
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
int ret;
@@ -127,7 +126,7 @@ int intel_vga_register(struct drm_i915_private *i915)
* then we do not take part in VGA arbitration and the
* vga_client_register() fails with -ENODEV.
*/
- ret = vga_client_register(pdev, intel_vga_set_decode);
+ ret = vga_client_register(pdev, intel_vga_set_decode, NULL);
if (ret && ret != -ENODEV)
return ret;
diff --git a/drivers/gpu/drm/loongson/lsdc_drv.c
b/drivers/gpu/drm/loongson/lsdc_drv.c
index 188ec82afcfb..d10a28c2c494 100644
--- a/drivers/gpu/drm/loongson/lsdc_drv.c
+++ b/drivers/gpu/drm/loongson/lsdc_drv.c
@@ -289,7 +289,7 @@ static int lsdc_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
pci_set_drvdata(pdev, ddev);
- vga_client_register(pdev, lsdc_vga_set_decode);
+ vga_client_register(pdev, lsdc_vga_set_decode, NULL);
drm_kms_helper_poll_init(ddev);
diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c
b/drivers/gpu/drm/nouveau/nouveau_vga.c
index f8bf0ec26844..162b4f4676c7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_vga.c
+++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
@@ -92,7 +92,7 @@ nouveau_vga_init(struct nouveau_drm *drm)
return;
pdev = to_pci_dev(dev->dev);
- vga_client_register(pdev, nouveau_vga_set_decode);
+ vga_client_register(pdev, nouveau_vga_set_decode, NULL);
/* don't register Thunderbolt eGPU with vga_switcheroo */
if (pci_is_thunderbolt_attached(pdev))
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index afbb3a80c0c6..71f2ff39d6a1 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1425,7 +1425,7 @@ int radeon_device_init(struct radeon_device *rdev,
/* if we have > 1 VGA cards, then disable the radeon VGA resources */
/* this will fail for cards that aren't VGA class devices, just
* ignore it */
- vga_client_register(rdev->pdev, radeon_vga_set_decode);
+ vga_client_register(rdev->pdev, radeon_vga_set_decode, NULL);
if (rdev->flags & RADEON_IS_PX)
runtime = true;
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index 953daf731b2c..610ddcccef24 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -53,6 +53,7 @@ struct vga_device {
bool bridge_has_one_vga;
bool is_firmware_default; /* device selected by firmware */
unsigned int (*set_decode)(struct pci_dev *pdev, bool decode);
+ bool (*is_primary_gpu)(struct pci_dev *pdev);
};
static LIST_HEAD(vga_list);
@@ -958,6 +959,13 @@ EXPORT_SYMBOL(vga_set_legacy_decoding);
* @set_decode callback: If a client can disable its GPU VGA resource, it
* will get a callback from this to set the encode/decode state.
*
+ * @is_primary_gpu callback: call back to the device driver, query if a PCI
+ * GPU client is the primary display device, as device drivers (drm-based
+ * or fbdev-based) may have better knowledge if a specific device is the
+ * default boot device or should be the default boot device. But this
+ * callback is optional. A device driver can simply pass a NULL pointer to
+ * adhere to the original rules of arbitration.
+ *
* Rationale: we cannot disable VGA decode resources unconditionally, some
* single GPU laptops seem to require ACPI or BIOS access to the VGA registers
* to control things like backlights etc. Hopefully newer multi-GPU laptops do
@@ -973,7 +981,8 @@ EXPORT_SYMBOL(vga_set_legacy_decoding);
* Returns: 0 on success, -1 on failure
*/
int vga_client_register(struct pci_dev *pdev,
- unsigned int (*set_decode)(struct pci_dev *pdev, bool decode))
+ unsigned int (*set_decode)(struct pci_dev *pdev, bool decode),
+ bool (*is_primary_gpu)(struct pci_dev *pdev))
{
int ret = -ENODEV;
struct vga_device *vgadev;
@@ -985,6 +994,7 @@ int vga_client_register(struct pci_dev *pdev,
goto bail;
vgadev->set_decode = set_decode;
+ vgadev->is_primary_gpu = is_primary_gpu;
ret = 0;
bail:
@@ -1490,6 +1500,30 @@ static void vga_arbiter_notify_clients(void)
spin_unlock_irqrestore(&vga_lock, flags);
}
+static void vga_arbiter_do_arbitration(struct pci_dev *pdev)
+{
+ struct vga_device *vgadev;
+ unsigned long flags;
+
+ spin_lock_irqsave(&vga_lock, flags);
+ list_for_each_entry(vgadev, &vga_list, list) {
+ if (vgadev->pdev != pdev)
+ continue;
+
+ /* This device already the boot device, do nothing */
+ if (pdev == vga_default_device())
+ break;
+
+ if (vgadev->is_primary_gpu) {
+ if (vgadev->is_primary_gpu(pdev)) {
+ vgaarb_info(&pdev->dev, "Overriding as primary GPU\n");
+ vga_set_default_device(pdev);
+ }
+ }
+ }
+ spin_unlock_irqrestore(&vga_lock, flags);
+}
+
static int pci_notify(struct notifier_block *nb, unsigned long action,
void *data)
{
@@ -1509,13 +1543,24 @@ static int pci_notify(struct notifier_block *nb,
unsigned long action,
* cases of hotplugable vga cards.
*/
- if (action == BUS_NOTIFY_ADD_DEVICE)
+ switch (action) {
+ case BUS_NOTIFY_ADD_DEVICE:
notify = vga_arbiter_add_pci_device(pdev);
- else if (action == BUS_NOTIFY_DEL_DEVICE)
+ if (notify)
+ vga_arbiter_notify_clients();
+ break;
+ case BUS_NOTIFY_DEL_DEVICE:
notify = vga_arbiter_del_pci_device(pdev);
+ if (notify)
+ vga_arbiter_notify_clients();
+ break;
+ case BUS_NOTIFY_BOUND_DRIVER:
+ vga_arbiter_do_arbitration(pdev);
+ break;
+ default:
+ break;
+ }
- if (notify)
- vga_arbiter_notify_clients();
return 0;
}
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 20d7b69ea6ff..531c4d8ef26e 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -2108,7 +2108,7 @@ static int vfio_pci_vga_init(struct vfio_pci_core_device
*vdev)
if (ret)
return ret;
- ret = vga_client_register(pdev, vfio_pci_set_decode);
+ ret = vga_client_register(pdev, vfio_pci_set_decode, NULL);
if (ret)
return ret;
vga_set_legacy_decoding(pdev, vfio_pci_set_decode(pdev, false));
diff --git a/include/linux/vgaarb.h b/include/linux/vgaarb.h
index 97129a1bbb7d..e4102be21f47 100644
--- a/include/linux/vgaarb.h
+++ b/include/linux/vgaarb.h
@@ -33,7 +33,8 @@ struct pci_dev *vga_default_device(void);
void vga_set_default_device(struct pci_dev *pdev);
int vga_remove_vgacon(struct pci_dev *pdev);
int vga_client_register(struct pci_dev *pdev,
- unsigned int (*set_decode)(struct pci_dev *pdev, bool state));
+ unsigned int (*set_decode)(struct pci_dev *pdev, bool state),
+ bool (*is_primary_gpu)(struct pci_dev *pdev));
#else /* CONFIG_VGA_ARB */
static inline void vga_set_legacy_decoding(struct pci_dev *pdev,
unsigned int decodes)
@@ -59,7 +60,8 @@ static inline int vga_remove_vgacon(struct pci_dev *pdev)
return 0;
}
static inline int vga_client_register(struct pci_dev *pdev,
- unsigned int (*set_decode)(struct pci_dev *pdev, bool state))
+ unsigned int (*set_decode)(struct pci_dev *pdev, bool state),
+ bool (*is_primary_gpu)(struct pci_dev *pdev))
{
return 0;
}
@@ -97,7 +99,7 @@ static inline int vga_get_uninterruptible(struct pci_dev
*pdev,
static inline void vga_client_unregister(struct pci_dev *pdev)
{
- vga_client_register(pdev, NULL);
+ vga_client_register(pdev, NULL, NULL);
}
#endif /* LINUX_VGA_H */
--
2.25.1
Sui Jingfeng
2023-Jul-11 16:31 UTC
[Nouveau] [PATCH v3 5/9] drm/amdgpu: Implement the is_primary_gpu callback of vga_client_register()
From: Sui Jingfeng <suijingfeng at loongson.cn>
[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 <alexander.deucher at amd.com>
Cc: Christian Konig <christian.koenig at amd.com>
Cc: Pan Xinhui <Xinhui.Pan at amd.com>
Cc: David Airlie <airlied at gmail.com>
Cc: Daniel Vetter <daniel at ffwll.ch>
Cc: Hawking Zhang <Hawking.Zhang at amd.com>
Cc: Mario Limonciello <mario.limonciello at amd.com>
Cc: Lijo Lazar <lijo.lazar at amd.com>
Cc: YiPeng Chai <YiPeng.Chai at amd.com>
Cc: Bokun Zhang <Bokun.Zhang at amd.com>
CC: Likun Gao <Likun.Gao at amd.com>
Signed-off-by: Sui Jingfeng <suijingfeng at loongson.cn>
---
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