Re: [Nouveau] [PATCH v7 6/8] PCI/VGA: Introduce is_boot_device function callback to vga_client_register
On Fri, 30 Jun 2023, Bjorn Helgaas wrote: > On Fri, Jun 30, 2023 at 10:14:11AM +0800, suijingfeng wrote: >> On 2023/6/30 01:44, Limonciello, Mario wrote: >> > > On 2023/6/29 23:54, Bjorn Helgaas wrote: >> > > > On Thu, Jun 22, 2023 at 01:08:15PM +0800, Sui Jingfeng wrote: > >> > > > 4) Right now we're in the middle of the v6.5 merge window, so new >> > > > content, e.g., this series, is too late for v6.5. Most >> > > > maintainers, including me, wait to merge new content until the >> > > > merge window closes and a new -rc1 is tagged. This merge window >> > > > should close on July 9, and people will start merging content for >> > > > v6.6, typically based on v6.5-rc1. >> > > >> > > Would you will merge all of the patches in this series (e.g. including >> > > the patch for drm/amdgpu(7/8) and drm/radeon(8/8)) ? >> > > >> > > Or just part of them? > > The bulk of this series is drivers/pci changes, so typically I would > merge all the patches after getting Acked-by tags from the other > subsystems (DRM and VFIO). For the (negligible) i915 parts, Acked-by: Jani Nikula >> Is it possible to merge the PCI/VGA part as fast as possible, >> especially the PATCH-0006 PCI/VGA: Introduce is_boot_device function >> callback to vga_client_register > > We're in the middle of the v6.5 merge window, so it's too late to add > things to v6.5-rc1. The most likely path for new material like this > would be to queue it for v6.6, which means I would merge it after > v6.5-rc1 is tagged (that tag will probably happen on July 9). Perhaps the part that causes confusion here is that the drm-misc-next and drm-intel-next branches, for example, are always open for new patches; it's just that there's a cutoff at around rc5/rc6 after which they start targeting the next+1 release. We basically hide the merge window from a lot of drm developers. > It would then be in -next until the v6.6 merge window opens (likely in > September), when it would be merged into Linus' tree. > > If the series fixes a regression or other major defect, it's > *possible* to merge things earlier, so they appear in v6.5. But this > series doesn't seem to fall in that category, so I think v6.6 is a > more realistic target. > > Merging for v6.6 would include both the PCI parts and the DRM parts at > the same time, so hopefully that addresses your dependency concerns. I guess the main question is whether Sui Jingfeng has follow-up work planned in drm that depends on these being merged. This would set that back by a full release. (But it happens.) BR, Jani. > > I suggest that you wait until v6.5-rc1, rebase your patches so they > apply cleanly on that tag, collect all the Reviewed-by and Acked-by > tags, include them in your commit logs, and then repost them. > > Bjorn -- Jani Nikula, Intel Open Source Graphics Center
Re: [Nouveau] [PATCH v7 6/8] PCI/VGA: Introduce is_boot_device function callback to vga_client_register
On Fri, Jun 30, 2023 at 10:14:11AM +0800, suijingfeng wrote: > On 2023/6/30 01:44, Limonciello, Mario wrote: > > > On 2023/6/29 23:54, Bjorn Helgaas wrote: > > > > On Thu, Jun 22, 2023 at 01:08:15PM +0800, Sui Jingfeng wrote: > > > > 4) Right now we're in the middle of the v6.5 merge window, so new > > > > content, e.g., this series, is too late for v6.5. Most > > > > maintainers, including me, wait to merge new content until the > > > > merge window closes and a new -rc1 is tagged. This merge window > > > > should close on July 9, and people will start merging content for > > > > v6.6, typically based on v6.5-rc1. > > > > > > Would you will merge all of the patches in this series (e.g. including > > > the patch for drm/amdgpu(7/8) and drm/radeon(8/8)) ? > > > > > > Or just part of them? The bulk of this series is drivers/pci changes, so typically I would merge all the patches after getting Acked-by tags from the other subsystems (DRM and VFIO). > Is it possible to merge the PCI/VGA part as fast as possible, > especially the PATCH-0006 PCI/VGA: Introduce is_boot_device function > callback to vga_client_register We're in the middle of the v6.5 merge window, so it's too late to add things to v6.5-rc1. The most likely path for new material like this would be to queue it for v6.6, which means I would merge it after v6.5-rc1 is tagged (that tag will probably happen on July 9). It would then be in -next until the v6.6 merge window opens (likely in September), when it would be merged into Linus' tree. If the series fixes a regression or other major defect, it's *possible* to merge things earlier, so they appear in v6.5. But this series doesn't seem to fall in that category, so I think v6.6 is a more realistic target. Merging for v6.6 would include both the PCI parts and the DRM parts at the same time, so hopefully that addresses your dependency concerns. I suggest that you wait until v6.5-rc1, rebase your patches so they apply cleanly on that tag, collect all the Reviewed-by and Acked-by tags, include them in your commit logs, and then repost them. Bjorn
[Nouveau] [PATCH] drm/nouveau/disp/g94: enable HDMI
Cc: Ben Skeggs
Cc: Lyude Paul
Fixes: f530bc60a30b ("drm/nouveau/disp: move HDMI config into acquire +
infoframe methods")
Signed-off-by: Karol Herbst
---
drivers/gpu/drm/nouveau/nvkm/engine/disp/g94.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/g94.c
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/g94.c
index a4853c4e5ee3..67ef889a0c5f 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/g94.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/g94.c
@@ -295,6 +295,7 @@ g94_sor = {
.clock = nv50_sor_clock,
.war_2 = g94_sor_war_2,
.war_3 = g94_sor_war_3,
+ .hdmi = _sor_hdmi,
.dp = _sor_dp,
};
--
2.41.0
[Nouveau] [PATCH v1 2/4] PCI/VGA: Improve the default VGA device selection
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 the keep the original behavior.
This patch is to introduce the mechanism only, while the 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
Cc: Christian Konig
Cc: Pan Xinhui
Cc: David Airlie
Cc: Daniel Vetter
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: Ben Skeggs
Cc: Karol Herbst
Cc: Lyude Paul
Cc: Bjorn Helgaas
Cc: Alex Williamson
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: Hawking Zhang
Cc: Mario Limonciello
Cc: Lijo Lazar
Cc: YiPeng Chai
Cc: Bokun Zhang
Cc: Likun Gao
Cc: Ville Syrjala
Cc: Jason Gunthorpe
CC: Kevin Tian
Cc: Cornelia Huck
Cc: Yishai Hadas
Cc: Abhishek Sahu
Cc: Yi Liu
Reviewed-by: Lyude Paul # nouveau
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
drivers/gpu/drm/i915/display/intel_vga.c | 3 +--
drivers/gpu/drm/nouveau/nouveau_vga.c | 2 +-
drivers/gpu/drm/radeon/radeon_device.c | 2 +-
drivers/pci/vgaarb.c | 21 -
drivers/vfio/pci/vfio_pci_core.c | 2 +-
include/linux/vgaarb.h | 8 +---
7 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index e25f085ee886..c5bdf6eff29e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4082,7 +4082,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/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
[Nouveau] [PATCH v1 0/4] PCI/VGA: Improve the default VGA device selection
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 the keep the original behavior. Sui Jingfeng (4): video/aperture: Add a helper to detect if an aperture contains firmware FB PCI/VGA: Improve the default VGA device selection 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 | 21 +++- 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 | 8 +++--- 11 files changed, 104 insertions(+), 10 deletions(-) -- 2.25.1
[Nouveau] [PATCH v1 1/4] video/aperture: Add a helper to detect if an aperture contains firmware FB
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
Cc: Javier Martinez Canillas
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: David Airlie
Cc: Daniel Vetter
Cc: Helge Deller
Signed-off-by: Sui Jingfeng
---
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(_lock);
+ firm_fb_start = base;
+ firm_fb_end = end;
+
list_for_each(pos, ) {
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,
