2017-03-13 12:42:00

by Christian König

[permalink] [raw]
Subject: Resizeable PCI BAR support V3

Hi Bjorn and others on the lists,

it's over a year that I initially came up with that, but now I
finally have the time to finish it up.

This set of patches allows device drivers to resize and most likely also
relocate the PCI BAR of devices they manage to allow the CPU to access
all of the device local memory at once.

This is very useful for GFX device drivers where the default PCI BAR is only
about 256MB in size for compatibility reasons, but the device easily have
multiple gigabyte of local memory.

Additional to the pure resize functionality this set also adds a quirk to
enable a 64bit bar above 4GB on AMD Family 15h CPUs/APUs. Doing so is
actually trivial and I plan to extend this to all recent AMD CPUs/APUs.

Open questions:

1. Is this the right location to implement the 64bit BAR above 4GB for AMD CPUs?
I guess that this needs a bit more cleanup.

2. Any idea how to better handle intermediate bridges? Reprogramming them isn't
possible after drivers are loaded, but at least for GFX drivers we need to load
them first to know how large we actually want our BAR to be (and to kick our
vesafb/efifb).

Thanks in advance,
Christian.


2017-03-13 12:42:24

by Christian König

[permalink] [raw]
Subject: [PATCH 1/4] PCI: add resizeable BAR infrastructure v3

From: Christian König <[email protected]>

Just the defines and helper functions to read the possible sizes of a BAR and
update it's size.

See https://pcisig.com/sites/default/files/specification_documents/ECN_Resizable-BAR_24Apr2008.pdf.

This is useful for hardware with large local storage (mostly GFX) which only
expose 256MB BARs initially to be compatible with 32bit systems.

v2: provide read helper as well
v3: improve function names, use unsigned values, add better comments.

Signed-off-by: Christian König <[email protected]>
---
drivers/pci/pci.c | 115 ++++++++++++++++++++++++++++++++++++++++++
include/linux/pci.h | 3 ++
include/uapi/linux/pci_regs.h | 7 +++
3 files changed, 125 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ba34907..c2d9f78 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2944,6 +2944,121 @@ bool pci_acs_path_enabled(struct pci_dev *start,
}

/**
+ * pci_rbar_get_possible_sizes - get possible sizes for BAR
+ * @dev: PCI device
+ * @bar: BAR to query
+ *
+ * Get the possible sizes of a resizeable BAR as bitmask defined in the spec
+ * (bit 0=1MB, bit 19=512GB). Returns 0 if BAR isn't resizeable.
+ */
+u32 pci_rbar_get_possible_sizes(struct pci_dev *pdev, int bar)
+{
+ unsigned pos, nbars;
+ u32 ctrl, cap;
+ unsigned i;
+
+ pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+ if (!pos)
+ return 0;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
+ nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >> PCI_REBAR_CTRL_NBAR_SHIFT;
+
+ for (i = 0; i < nbars; ++i, pos += 8) {
+ int bar_idx;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
+ bar_idx = (ctrl & PCI_REBAR_CTRL_BAR_IDX_MASK) >>
+ PCI_REBAR_CTRL_BAR_IDX_SHIFT;
+ if (bar_idx != bar)
+ continue;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CAP, &cap);
+ return (cap & PCI_REBAR_CTRL_SIZES_MASK) >>
+ PCI_REBAR_CTRL_SIZES_SHIFT;
+ }
+
+ return 0;
+}
+
+/**
+ * pci_rbar_get_current_size - get the current size of a BAR
+ * @dev: PCI device
+ * @bar: BAR to set size to
+ *
+ * Read the size of a BAR from the resizeable BAR config.
+ * Returns size if found or negativ error code.
+ */
+int pci_rbar_get_current_size(struct pci_dev *pdev, int bar)
+{
+ unsigned pos, nbars;
+ u32 ctrl;
+ unsigned i;
+
+ pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+ if (!pos)
+ return -ENOTSUPP;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
+ nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >> PCI_REBAR_CTRL_NBAR_SHIFT;
+
+ for (i = 0; i < nbars; ++i, pos += 8) {
+ int bar_idx;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
+ bar_idx = (ctrl & PCI_REBAR_CTRL_BAR_IDX_MASK) >>
+ PCI_REBAR_CTRL_BAR_IDX_SHIFT;
+ if (bar_idx != bar)
+ continue;
+
+ return (ctrl & PCI_REBAR_CTRL_BAR_SIZE_MASK) >>
+ PCI_REBAR_CTRL_BAR_SIZE_SHIFT;
+ }
+
+ return -ENOENT;
+}
+
+/**
+ * pci_rbar_set_size - set a new size for a BAR
+ * @dev: PCI device
+ * @bar: BAR to set size to
+ * @size: new size as defined in the spec (log2(size in bytes) - 20)
+ *
+ * Set the new size of a BAR as defined in the spec (0=1MB, 19=512GB).
+ * Returns true if resizing was successful, false otherwise.
+ */
+int pci_rbar_set_size(struct pci_dev *pdev, int bar, int size)
+{
+ unsigned pos, nbars;
+ u32 ctrl;
+ unsigned i;
+
+ pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+ if (!pos)
+ return -ENOTSUPP;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
+ nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >> PCI_REBAR_CTRL_NBAR_SHIFT;
+
+ for (i = 0; i < nbars; ++i, pos += 8) {
+ int bar_idx;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
+ bar_idx = (ctrl & PCI_REBAR_CTRL_BAR_IDX_MASK) >>
+ PCI_REBAR_CTRL_BAR_IDX_SHIFT;
+ if (bar_idx != bar)
+ continue;
+
+ ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE_MASK;
+ ctrl |= size << PCI_REBAR_CTRL_BAR_SIZE_SHIFT;
+ pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
+ return 0;
+ }
+
+ return -ENOENT;
+}
+
+/**
* pci_swizzle_interrupt_pin - swizzle INTx for device behind bridge
* @dev: the PCI device
* @pin: the INTx pin (1=INTA, 2=INTB, 3=INTC, 4=INTD)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index a38772a..6954e50 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1946,6 +1946,9 @@ void pci_request_acs(void);
bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags);
bool pci_acs_path_enabled(struct pci_dev *start,
struct pci_dev *end, u16 acs_flags);
+u32 pci_rbar_get_possible_sizes(struct pci_dev *pdev, int bar);
+int pci_rbar_get_current_size(struct pci_dev *pdev, int bar);
+int pci_rbar_set_size(struct pci_dev *pdev, int bar, int size);

#define PCI_VPD_LRDT 0x80 /* Large Resource Data Type */
#define PCI_VPD_LRDT_ID(x) ((x) | PCI_VPD_LRDT)
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index e5a2e68..6de29d6 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -932,9 +932,16 @@
#define PCI_SATA_SIZEOF_LONG 16

/* Resizable BARs */
+#define PCI_REBAR_CAP 4 /* capability register */
+#define PCI_REBAR_CTRL_SIZES_MASK (0xFFFFF << 4) /* mask for sizes */
+#define PCI_REBAR_CTRL_SIZES_SHIFT 4 /* shift for sizes */
#define PCI_REBAR_CTRL 8 /* control register */
+#define PCI_REBAR_CTRL_BAR_IDX_MASK (7 << 0) /* mask for bar index */
+#define PCI_REBAR_CTRL_BAR_IDX_SHIFT 0 /* shift for bar index */
#define PCI_REBAR_CTRL_NBAR_MASK (7 << 5) /* mask for # bars */
#define PCI_REBAR_CTRL_NBAR_SHIFT 5 /* shift for # bars */
+#define PCI_REBAR_CTRL_BAR_SIZE_MASK (0x1F << 8) /* mask for bar size */
+#define PCI_REBAR_CTRL_BAR_SIZE_SHIFT 8 /* shift for bar size */

/* Dynamic Power Allocation */
#define PCI_DPA_CAP 4 /* capability register */
--
2.7.4

2017-03-13 12:42:14

by Christian König

[permalink] [raw]
Subject: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

From: Christian König <[email protected]>

Try to resize BAR0 to let CPU access all of VRAM.

Signed-off-by: Christian König <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29 +++++++++++++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 +++++---
drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 +++++---
4 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 3b81ded..905ded9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_tt *ttm,
struct ttm_mem_reg *mem);
void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base);
void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc);
+void amdgpu_resize_bar0(struct amdgpu_device *adev);
void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size);
int amdgpu_ttm_init(struct amdgpu_device *adev);
void amdgpu_ttm_fini(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 118f4e6..92955fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc)
mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
}

+/**
+ * amdgpu_resize_bar0 - try to resize BAR0
+ *
+ * @adev: amdgpu_device pointer
+ *
+ * Try to resize BAR0 to make all VRAM CPU accessible.
+ */
+void amdgpu_resize_bar0(struct amdgpu_device *adev)
+{
+ u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
+ int r;
+
+ r = pci_resize_resource(adev->pdev, 0, size);
+
+ if (r == -ENOTSUPP) {
+ /* The hardware don't support the extension. */
+ return;
+
+ } else if (r == -ENOSPC) {
+ DRM_INFO("Not enoigh PCI address space for a large BAR.");
+ } else if (r) {
+ DRM_ERROR("Problem resizing BAR0 (%d).", r);
+ }
+
+ /* Reinit the doorbell mapping, it is most likely moved as well */
+ amdgpu_doorbell_fini(adev);
+ BUG_ON(amdgpu_doorbell_init(adev));
+}
+
/*
* GPU helpers function.
*/
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index dc9b6d6..36a7aa5 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
break;
}
adev->mc.vram_width = numchan * chansize;
- /* Could aper size report 0 ? */
- adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
- adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
/* size in MB on si */
adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;

+ if (!(adev->flags & AMD_IS_APU))
+ amdgpu_resize_bar0(adev);
+ adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
+ adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
+
#ifdef CONFIG_X86_64
if (adev->flags & AMD_IS_APU) {
adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index c087b00..7761ad3 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
break;
}
adev->mc.vram_width = numchan * chansize;
- /* Could aper size report 0 ? */
- adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
- adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
/* size in MB on si */
adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;

+ if (!(adev->flags & AMD_IS_APU))
+ amdgpu_resize_bar0(adev);
+ adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
+ adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
+
#ifdef CONFIG_X86_64
if (adev->flags & AMD_IS_APU) {
adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
--
2.7.4

2017-03-13 12:43:43

by Christian König

[permalink] [raw]
Subject: [PATCH 3/4] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors

From: Christian König <[email protected]>

Most BIOS don't enable this because of compatibility reasons.

Manually enable a 64bit BAR of 64GB size so that we have
enough room for PCI devices.

Signed-off-by: Christian König <[email protected]>
---
arch/x86/pci/fixup.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)

diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 6d52b94..bff5242 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -571,3 +571,56 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2fc0, pci_invalid_bar);
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_invalid_bar);
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_invalid_bar);
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_invalid_bar);
+
+static void pci_amd_enable_64bit_bar(struct pci_dev *dev)
+{
+ const uint64_t size = 64ULL * 1024 * 1024 * 1024;
+ uint32_t base, limit, high;
+ struct resource *res;
+ unsigned i;
+ int r;
+
+ for (i = 0; i < 8; ++i) {
+
+ pci_read_config_dword(dev, 0x80 + i * 0x8, &base);
+ pci_read_config_dword(dev, 0x180 + i * 0x4, &high);
+
+ /* Is this slot free? */
+ if ((base & 0x3) == 0x0)
+ break;
+
+ base >>= 8;
+ base |= high << 24;
+
+ /* Abort if a slot already configures a 64bit BAR. */
+ if (base > 0x10000)
+ return;
+
+ }
+
+ if (i == 8)
+ return;
+
+ res = kzalloc(sizeof(*res), GFP_KERNEL);
+ res->flags = IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_MEM_64 |
+ IORESOURCE_WINDOW;
+ res->name = dev->bus->name;
+ r = allocate_resource(&iomem_resource, res, size, 0x100000000,
+ 0xfd00000000, size, NULL, NULL);
+ if (r) {
+ kfree(res);
+ return;
+ }
+
+ base = ((res->start >> 8) & 0xffffff00) | 0x3;
+ limit = ((res->end + 1) >> 8) & 0xffffff00;
+ high = ((res->start >> 40) & 0xff) |
+ ((((res->end + 1) >> 40) & 0xff) << 16);
+
+ pci_write_config_dword(dev, 0x180 + i * 0x4, high);
+ pci_write_config_dword(dev, 0x84 + i * 0x8, limit);
+ pci_write_config_dword(dev, 0x80 + i * 0x8, base);
+
+ pci_bus_add_resource(dev->bus, res, 0);
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_AMD, 0x141b, pci_amd_enable_64bit_bar);
--
2.7.4

2017-03-13 12:44:04

by Christian König

[permalink] [raw]
Subject: [PATCH 2/4] PCI: add functionality for resizing resources v2

From: Christian König <[email protected]>

This allows device drivers to request resizing their BARs.

The function only tries to reprogram the windows of the bridge directly above
the requesting device and only the BAR of the same type (usually mem, 64bit,
prefetchable). This is done to make sure not to disturb other drivers by
changing the BARs of their devices.

If reprogramming the bridge BAR fails the old status is restored and -ENOSPC
returned to the calling device driver.

v2: rebase on changes in rbar support

Signed-off-by: Christian König <[email protected]>
---
drivers/pci/setup-bus.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/pci/setup-res.c | 51 +++++++++++++++++++++++++++++++++++++++++
include/linux/pci.h | 2 ++
3 files changed, 114 insertions(+)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index f30ca75..cfab2c7 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1923,6 +1923,67 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
}
EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);

+int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
+{
+ const unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
+ IORESOURCE_PREFETCH | IORESOURCE_MEM_64;
+
+ struct resource saved;
+ LIST_HEAD(add_list);
+ LIST_HEAD(fail_head);
+ struct pci_dev_resource *fail_res;
+ unsigned i;
+ int ret = 0;
+
+ /* Release all children from the matching bridge resource */
+ for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END; ++i) {
+ struct resource *res = &bridge->resource[i];
+
+ if ((res->flags & type_mask) != (type & type_mask))
+ continue;
+
+ saved = *res;
+ if (res->parent) {
+ release_child_resources(res);
+ release_resource(res);
+ }
+ res->start = 0;
+ res->end = 0;
+ break;
+ }
+
+ if (i == PCI_BRIDGE_RESOURCE_END)
+ return -ENOENT;
+
+ __pci_bus_size_bridges(bridge->subordinate, &add_list);
+ __pci_bridge_assign_resources(bridge, &add_list, &fail_head);
+ BUG_ON(!list_empty(&add_list));
+
+ /* restore size and flags */
+ list_for_each_entry(fail_res, &fail_head, list) {
+ struct resource *res = fail_res->res;
+
+ res->start = fail_res->start;
+ res->end = fail_res->end;
+ res->flags = fail_res->flags;
+ }
+
+ /* Revert to the old configuration */
+ if (!list_empty(&fail_head)) {
+ struct resource *res = &bridge->resource[i];
+
+ res->start = saved.start;
+ res->end = saved.end;
+ res->flags = saved.flags;
+
+ pci_claim_resource(bridge, i);
+ ret = -ENOSPC;
+ }
+
+ free_list(&fail_head);
+ return ret;
+}
+
void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
{
struct pci_dev *dev;
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 9526e34..3bb1e29 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -363,6 +363,57 @@ int pci_reassign_resource(struct pci_dev *dev, int resno, resource_size_t addsiz
return 0;
}

+int pci_resize_resource(struct pci_dev *dev, int resno, int size)
+{
+ struct resource *res = dev->resource + resno;
+ u32 sizes = pci_rbar_get_possible_sizes(dev, resno);
+ int old = pci_rbar_get_current_size(dev, resno);
+ u64 bytes = 1ULL << (size + 20);
+ int ret = 0;
+
+ if (!sizes)
+ return -ENOTSUPP;
+
+ if (!(sizes & (1 << size)))
+ return -EINVAL;
+
+ if (old < 0)
+ return old;
+
+ /* Make sure the resource isn't assigned before making it larger. */
+ if (resource_size(res) < bytes && res->parent) {
+ release_resource(res);
+ res->end = resource_size(res) - 1;
+ res->start = 0;
+ if (resno < PCI_BRIDGE_RESOURCES)
+ pci_update_resource(dev, resno);
+ }
+
+ ret = pci_rbar_set_size(dev, resno, size);
+ if (ret)
+ goto error_reassign;
+
+ res->end = res->start + bytes - 1;
+
+ ret = pci_reassign_bridge_resources(dev->bus->self, res->flags);
+ if (ret)
+ goto error_resize;
+
+ pci_reenable_device(dev->bus->self);
+ return 0;
+
+error_resize:
+ pci_rbar_set_size(dev, resno, old);
+ res->end = res->start + (1ULL << (old + 20)) - 1;
+
+error_reassign:
+ pci_assign_unassigned_bus_resources(dev->bus);
+ pci_setup_bridge(dev->bus);
+ pci_reenable_device(dev->bus->self);
+ return ret;
+}
+EXPORT_SYMBOL(pci_resize_resource);
+
int pci_enable_resources(struct pci_dev *dev, int mask)
{
u16 cmd, old_cmd;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 6954e50..8eaebb4 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1055,6 +1055,7 @@ void pci_reset_bridge_secondary_bus(struct pci_dev *dev);
void pci_update_resource(struct pci_dev *dev, int resno);
int __must_check pci_assign_resource(struct pci_dev *dev, int i);
int __must_check pci_reassign_resource(struct pci_dev *dev, int i, resource_size_t add_size, resource_size_t align);
+int __must_check pci_resize_resource(struct pci_dev *dev, int i, int size);
int pci_select_bars(struct pci_dev *dev, unsigned long flags);
bool pci_device_is_present(struct pci_dev *pdev);
void pci_ignore_hotplug(struct pci_dev *dev);
@@ -1135,6 +1136,7 @@ void pci_assign_unassigned_resources(void);
void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
void pci_assign_unassigned_bus_resources(struct pci_bus *bus);
void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus);
+int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type);
void pdev_enable_device(struct pci_dev *);
int pci_enable_resources(struct pci_dev *, int mask);
void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),
--
2.7.4

2017-03-13 16:46:50

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 2/4] PCI: add functionality for resizing resources v2

On Mon, Mar 13, 2017 at 2:41 PM, Christian König
<[email protected]> wrote:
> From: Christian König <[email protected]>
>
> This allows device drivers to request resizing their BARs.
>
> The function only tries to reprogram the windows of the bridge directly above
> the requesting device and only the BAR of the same type (usually mem, 64bit,
> prefetchable). This is done to make sure not to disturb other drivers by
> changing the BARs of their devices.
>
> If reprogramming the bridge BAR fails the old status is restored and -ENOSPC
> returned to the calling device driver.

Some style comments.

> v2: rebase on changes in rbar support

This kind of comments usually goes after --- delimiter below.

> Signed-off-by: Christian König <[email protected]>
> ---

...here.

> +int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
> +{
> + const unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
> + IORESOURCE_PREFETCH | IORESOURCE_MEM_64;
> +
> + struct resource saved;
> + LIST_HEAD(add_list);
> + LIST_HEAD(fail_head);

> + struct pci_dev_resource *fail_res;

Perhaps move before definition of 'saved'?

> + unsigned i;
> + int ret = 0;
> +
> + /* Release all children from the matching bridge resource */
> + for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END; ++i) {
> + struct resource *res = &bridge->resource[i];
> +


> + if ((res->flags & type_mask) != (type & type_mask))

IIUC it would be
if ((res->flags ^ type) & type_mask)

(I consider 'diff' as XOR operation is more understandable, but it's up to you)

> + continue;

> + /* restore size and flags */
> + list_for_each_entry(fail_res, &fail_head, list) {
> + struct resource *res = fail_res->res;
> +
> + res->start = fail_res->start;
> + res->end = fail_res->end;
> + res->flags = fail_res->flags;
> + }
> +
> + /* Revert to the old configuration */
> + if (!list_empty(&fail_head)) {
> + struct resource *res = &bridge->resource[i];
> +

> + res->start = saved.start;
> + res->end = saved.end;
> + res->flags = saved.flags;

Would
*res = saved;
work?

> +int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> +{
> + struct resource *res = dev->resource + resno;
> + u32 sizes = pci_rbar_get_possible_sizes(dev, resno);
> + int old = pci_rbar_get_current_size(dev, resno);
> + u64 bytes = 1ULL << (size + 20);
> + int ret = 0;
> +

I would put
sizes = pci_rbar_get_possible_sizes(dev, resno);
here

> + if (!sizes)
> + return -ENOTSUPP;
> +
> + if (!(sizes & (1 << size)))

BIT(size) ?

> + return -EINVAL;
> +

and
old = pci_rbar_get_current_size(dev, resno);
here

> + if (old < 0)
> + return old;

> +error_resize:
> + pci_rbar_set_size(dev, resno, old);
> + res->end = res->start + (1ULL << (old + 20)) - 1;

BIT_ULL(old + 20) ?

--
With Best Regards,
Andy Shevchenko

2017-03-13 16:49:25

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 3/4] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors

On Mon, Mar 13, 2017 at 2:41 PM, Christian König
<[email protected]> wrote:

> Most BIOS don't enable this because of compatibility reasons.
>
> Manually enable a 64bit BAR of 64GB size so that we have
> enough room for PCI devices.

> +static void pci_amd_enable_64bit_bar(struct pci_dev *dev)
> +{
> + const uint64_t size = 64ULL * 1024 * 1024 * 1024;

Perhaps extend <linux/sizes.h> and use SZ_64G here?

It would be nice to do, since some of the drivers already are using
sizes like 4GB and alike.

> + uint32_t base, limit, high;
> + struct resource *res;
> + unsigned i;
> + int r;
> +

> + for (i = 0; i < 8; ++i) {

> +

Redundant empty line.

> + pci_read_config_dword(dev, 0x80 + i * 0x8, &base);
> + pci_read_config_dword(dev, 0x180 + i * 0x4, &high);
> +
> + /* Is this slot free? */
> + if ((base & 0x3) == 0x0)
> + break;
> +
> + base >>= 8;
> + base |= high << 24;
> +
> + /* Abort if a slot already configures a 64bit BAR. */
> + if (base > 0x10000)
> + return;

> +

Ditto.

> + }

> +

Ditto.

> + if (i == 8)
> + return;
> +
> + res = kzalloc(sizeof(*res), GFP_KERNEL);
> + res->flags = IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_MEM_64 |
> + IORESOURCE_WINDOW;
> + res->name = dev->bus->name;
> + r = allocate_resource(&iomem_resource, res, size, 0x100000000,
> + 0xfd00000000, size, NULL, NULL);
> + if (r) {
> + kfree(res);
> + return;
> + }
> +
> + base = ((res->start >> 8) & 0xffffff00) | 0x3;
> + limit = ((res->end + 1) >> 8) & 0xffffff00;
> + high = ((res->start >> 40) & 0xff) |
> + ((((res->end + 1) >> 40) & 0xff) << 16);

Perhaps some of constants can be replaced by defines (I think some of
them are already defined in ioport.h or somewhere else).

--
With Best Regards,
Andy Shevchenko

2017-03-13 16:51:22

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

On Mon, Mar 13, 2017 at 2:41 PM, Christian König
<[email protected]> wrote:
> From: Christian König <[email protected]>
>
> Try to resize BAR0 to let CPU access all of VRAM.

> +void amdgpu_resize_bar0(struct amdgpu_device *adev)
> +{
> + u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
> + int r;
> +
> + r = pci_resize_resource(adev->pdev, 0, size);

> +

Redundant.

> + if (r == -ENOTSUPP) {
> + /* The hardware don't support the extension. */
> + return;

> +

Ditto.

> + } else if (r == -ENOSPC) {

Useless use of else. And thus of curly braces.

> + DRM_INFO("Not enoigh PCI address space for a large BAR.");
> + } else if (r) {
> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
> + }
> +
> + /* Reinit the doorbell mapping, it is most likely moved as well */
> + amdgpu_doorbell_fini(adev);

> + BUG_ON(amdgpu_doorbell_init(adev));

Comment why it's used here.
--
With Best Regards,
Andy Shevchenko

2017-03-14 09:02:34

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/4] PCI: add functionality for resizing resources v2

Hi Christian,

[auto build test WARNING on pci/next]
[also build test WARNING on v4.11-rc2 next-20170310]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Christian-K-nig/PCI-add-resizeable-BAR-infrastructure-v3/20170314-163334
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: i386-randconfig-x077-201711 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

All warnings (new ones prefixed by >>):

drivers/pci/setup-res.c: In function 'pci_resize_resource':
>> drivers/pci/setup-res.c:422:2: warning: ignoring return value of 'pci_reenable_device', declared with attribute warn_unused_result [-Wunused-result]
pci_reenable_device(dev->bus->self);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/setup-res.c:432:2: warning: ignoring return value of 'pci_reenable_device', declared with attribute warn_unused_result [-Wunused-result]
pci_reenable_device(dev->bus->self);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/pci_reenable_device +422 drivers/pci/setup-res.c

406 res->end = resource_size(res) - 1;
407 res->start = 0;
408 if (resno < PCI_BRIDGE_RESOURCES)
409 pci_update_resource(dev, resno);
410 }
411
412 ret = pci_rbar_set_size(dev, resno, size);
413 if (ret)
414 goto error_reassign;
415
416 res->end = res->start + bytes - 1;
417
418 ret = pci_reassign_bridge_resources(dev->bus->self, res->flags);
419 if (ret)
420 goto error_resize;
421
> 422 pci_reenable_device(dev->bus->self);
423 return 0;
424
425 error_resize:
426 pci_rbar_set_size(dev, resno, old);
427 res->end = res->start + (1ULL << (old + 20)) - 1;
428
429 error_reassign:
430 pci_assign_unassigned_bus_resources(dev->bus);

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (2.08 kB)
.config.gz (23.57 kB)
Download all attachments

2017-03-14 09:25:36

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 3/4] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors

Hi Christian,

[auto build test WARNING on pci/next]
[also build test WARNING on v4.11-rc2 next-20170310]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Christian-K-nig/PCI-add-resizeable-BAR-infrastructure-v3/20170314-163334
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: i386-randconfig-s0-201711 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

All warnings (new ones prefixed by >>):

arch/x86/pci/fixup.c: In function 'pci_amd_enable_64bit_bar':
>> arch/x86/pci/fixup.c:608:52: warning: large integer implicitly truncated to unsigned type [-Woverflow]
r = allocate_resource(&iomem_resource, res, size, 0x100000000,
^~~~~~~~~~~
arch/x86/pci/fixup.c:609:10: warning: large integer implicitly truncated to unsigned type [-Woverflow]
0xfd00000000, size, NULL, NULL);
^~~~~~~~~~~~
>> arch/x86/pci/fixup.c:617:22: warning: right shift count >= width of type [-Wshift-count-overflow]
high = ((res->start >> 40) & 0xff) |
^~
arch/x86/pci/fixup.c:618:21: warning: right shift count >= width of type [-Wshift-count-overflow]
((((res->end + 1) >> 40) & 0xff) << 16);
^~

vim +608 arch/x86/pci/fixup.c

602 return;
603
604 res = kzalloc(sizeof(*res), GFP_KERNEL);
605 res->flags = IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_MEM_64 |
606 IORESOURCE_WINDOW;
607 res->name = dev->bus->name;
> 608 r = allocate_resource(&iomem_resource, res, size, 0x100000000,
609 0xfd00000000, size, NULL, NULL);
610 if (r) {
611 kfree(res);
612 return;
613 }
614
615 base = ((res->start >> 8) & 0xffffff00) | 0x3;
616 limit = ((res->end + 1) >> 8) & 0xffffff00;
> 617 high = ((res->start >> 40) & 0xff) |
618 ((((res->end + 1) >> 40) & 0xff) << 16);
619
620 pci_write_config_dword(dev, 0x180 + i * 0x4, high);

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (2.29 kB)
.config.gz (26.63 kB)
Download all attachments

2017-03-14 13:10:59

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/4] PCI: add resizeable BAR infrastructure v3

Hi Christian,

[auto build test WARNING on pci/next]
[also build test WARNING on v4.11-rc2 next-20170310]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Christian-K-nig/PCI-add-resizeable-BAR-infrastructure-v3/20170314-163334
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

lib/crc32.c:148: warning: No description found for parameter 'tab)[256]'
lib/crc32.c:148: warning: Excess function parameter 'tab' description in 'crc32_le_generic'
lib/crc32.c:293: warning: No description found for parameter 'tab)[256]'
lib/crc32.c:293: warning: Excess function parameter 'tab' description in 'crc32_be_generic'
lib/crc32.c:1: warning: no structured comments found
>> drivers/pci/pci.c:2951: warning: No description found for parameter 'pdev'
>> drivers/pci/pci.c:2951: warning: Excess function parameter 'dev' description in 'pci_rbar_get_possible_sizes'
drivers/pci/pci.c:2989: warning: No description found for parameter 'pdev'
>> drivers/pci/pci.c:2989: warning: Excess function parameter 'dev' description in 'pci_rbar_get_current_size'
drivers/pci/pci.c:3027: warning: No description found for parameter 'pdev'
>> drivers/pci/pci.c:3027: warning: Excess function parameter 'dev' description in 'pci_rbar_set_size'

vim +/pdev +2951 drivers/pci/pci.c

2945 * @bar: BAR to query
2946 *
2947 * Get the possible sizes of a resizeable BAR as bitmask defined in the spec
2948 * (bit 0=1MB, bit 19=512GB). Returns 0 if BAR isn't resizeable.
2949 */
2950 u32 pci_rbar_get_possible_sizes(struct pci_dev *pdev, int bar)
> 2951 {
2952 unsigned pos, nbars;
2953 u32 ctrl, cap;
2954 unsigned i;
2955
2956 pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
2957 if (!pos)
2958 return 0;
2959
2960 pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
2961 nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >> PCI_REBAR_CTRL_NBAR_SHIFT;
2962
2963 for (i = 0; i < nbars; ++i, pos += 8) {
2964 int bar_idx;
2965
2966 pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
2967 bar_idx = (ctrl & PCI_REBAR_CTRL_BAR_IDX_MASK) >>
2968 PCI_REBAR_CTRL_BAR_IDX_SHIFT;
2969 if (bar_idx != bar)
2970 continue;
2971
2972 pci_read_config_dword(pdev, pos + PCI_REBAR_CAP, &cap);
2973 return (cap & PCI_REBAR_CTRL_SIZES_MASK) >>
2974 PCI_REBAR_CTRL_SIZES_SHIFT;
2975 }
2976
2977 return 0;
2978 }
2979
2980 /**
2981 * pci_rbar_get_current_size - get the current size of a BAR
2982 * @dev: PCI device
2983 * @bar: BAR to set size to
2984 *
2985 * Read the size of a BAR from the resizeable BAR config.
2986 * Returns size if found or negativ error code.
2987 */
2988 int pci_rbar_get_current_size(struct pci_dev *pdev, int bar)
> 2989 {
2990 unsigned pos, nbars;
2991 u32 ctrl;
2992 unsigned i;
2993
2994 pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
2995 if (!pos)
2996 return -ENOTSUPP;
2997
2998 pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
2999 nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >> PCI_REBAR_CTRL_NBAR_SHIFT;
3000
3001 for (i = 0; i < nbars; ++i, pos += 8) {
3002 int bar_idx;
3003
3004 pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
3005 bar_idx = (ctrl & PCI_REBAR_CTRL_BAR_IDX_MASK) >>
3006 PCI_REBAR_CTRL_BAR_IDX_SHIFT;
3007 if (bar_idx != bar)
3008 continue;
3009
3010 return (ctrl & PCI_REBAR_CTRL_BAR_SIZE_MASK) >>
3011 PCI_REBAR_CTRL_BAR_SIZE_SHIFT;
3012 }
3013
3014 return -ENOENT;
3015 }
3016
3017 /**
3018 * pci_rbar_set_size - set a new size for a BAR
3019 * @dev: PCI device
3020 * @bar: BAR to set size to
3021 * @size: new size as defined in the spec (log2(size in bytes) - 20)
3022 *
3023 * Set the new size of a BAR as defined in the spec (0=1MB, 19=512GB).
3024 * Returns true if resizing was successful, false otherwise.
3025 */
3026 int pci_rbar_set_size(struct pci_dev *pdev, int bar, int size)
> 3027 {
3028 unsigned pos, nbars;
3029 u32 ctrl;
3030 unsigned i;

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (4.35 kB)
.config.gz (6.42 kB)
Download all attachments

2017-03-15 07:23:51

by Ayyappa Ch

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

Is it possible on Carrizo asics? Or only supports on newer asics?

On Mon, Mar 13, 2017 at 6:11 PM, Christian König
<[email protected]> wrote:
> From: Christian König <[email protected]>
>
> Try to resize BAR0 to let CPU access all of VRAM.
>
> Signed-off-by: Christian König <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29 +++++++++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 +++++---
> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 +++++---
> 4 files changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 3b81ded..905ded9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_tt *ttm,
> struct ttm_mem_reg *mem);
> void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base);
> void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc);
> +void amdgpu_resize_bar0(struct amdgpu_device *adev);
> void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size);
> int amdgpu_ttm_init(struct amdgpu_device *adev);
> void amdgpu_ttm_fini(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 118f4e6..92955fe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc)
> mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
> }
>
> +/**
> + * amdgpu_resize_bar0 - try to resize BAR0
> + *
> + * @adev: amdgpu_device pointer
> + *
> + * Try to resize BAR0 to make all VRAM CPU accessible.
> + */
> +void amdgpu_resize_bar0(struct amdgpu_device *adev)
> +{
> + u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
> + int r;
> +
> + r = pci_resize_resource(adev->pdev, 0, size);
> +
> + if (r == -ENOTSUPP) {
> + /* The hardware don't support the extension. */
> + return;
> +
> + } else if (r == -ENOSPC) {
> + DRM_INFO("Not enoigh PCI address space for a large BAR.");
> + } else if (r) {
> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
> + }
> +
> + /* Reinit the doorbell mapping, it is most likely moved as well */
> + amdgpu_doorbell_fini(adev);
> + BUG_ON(amdgpu_doorbell_init(adev));
> +}
> +
> /*
> * GPU helpers function.
> */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index dc9b6d6..36a7aa5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
> break;
> }
> adev->mc.vram_width = numchan * chansize;
> - /* Could aper size report 0 ? */
> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> /* size in MB on si */
> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
>
> + if (!(adev->flags & AMD_IS_APU))
> + amdgpu_resize_bar0(adev);
> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> +
> #ifdef CONFIG_X86_64
> if (adev->flags & AMD_IS_APU) {
> adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index c087b00..7761ad3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
> break;
> }
> adev->mc.vram_width = numchan * chansize;
> - /* Could aper size report 0 ? */
> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> /* size in MB on si */
> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
>
> + if (!(adev->flags & AMD_IS_APU))
> + amdgpu_resize_bar0(adev);
> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> +
> #ifdef CONFIG_X86_64
> if (adev->flags & AMD_IS_APU) {
> adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

2017-03-15 07:38:09

by Christian König

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

Carizzo is an APU and resizing BARs isn't needed nor supported there.
The CPU can access the full stolen VRAM directly on that hardware.

As far as I know ASICs with support for this are Tonga, Fiji and all
Polaris variants.

Christian.

Am 15.03.2017 um 08:23 schrieb Ayyappa Ch:
> Is it possible on Carrizo asics? Or only supports on newer asics?
>
> On Mon, Mar 13, 2017 at 6:11 PM, Christian König
> <[email protected]> wrote:
>> From: Christian König <[email protected]>
>>
>> Try to resize BAR0 to let CPU access all of VRAM.
>>
>> Signed-off-by: Christian König <[email protected]>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29 +++++++++++++++++++++++++++++
>> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 +++++---
>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 +++++---
>> 4 files changed, 40 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 3b81ded..905ded9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_tt *ttm,
>> struct ttm_mem_reg *mem);
>> void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base);
>> void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc);
>> +void amdgpu_resize_bar0(struct amdgpu_device *adev);
>> void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size);
>> int amdgpu_ttm_init(struct amdgpu_device *adev);
>> void amdgpu_ttm_fini(struct amdgpu_device *adev);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 118f4e6..92955fe 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc)
>> mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
>> }
>>
>> +/**
>> + * amdgpu_resize_bar0 - try to resize BAR0
>> + *
>> + * @adev: amdgpu_device pointer
>> + *
>> + * Try to resize BAR0 to make all VRAM CPU accessible.
>> + */
>> +void amdgpu_resize_bar0(struct amdgpu_device *adev)
>> +{
>> + u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
>> + int r;
>> +
>> + r = pci_resize_resource(adev->pdev, 0, size);
>> +
>> + if (r == -ENOTSUPP) {
>> + /* The hardware don't support the extension. */
>> + return;
>> +
>> + } else if (r == -ENOSPC) {
>> + DRM_INFO("Not enoigh PCI address space for a large BAR.");
>> + } else if (r) {
>> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
>> + }
>> +
>> + /* Reinit the doorbell mapping, it is most likely moved as well */
>> + amdgpu_doorbell_fini(adev);
>> + BUG_ON(amdgpu_doorbell_init(adev));
>> +}
>> +
>> /*
>> * GPU helpers function.
>> */
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> index dc9b6d6..36a7aa5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
>> break;
>> }
>> adev->mc.vram_width = numchan * chansize;
>> - /* Could aper size report 0 ? */
>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>> /* size in MB on si */
>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
>>
>> + if (!(adev->flags & AMD_IS_APU))
>> + amdgpu_resize_bar0(adev);
>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>> +
>> #ifdef CONFIG_X86_64
>> if (adev->flags & AMD_IS_APU) {
>> adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> index c087b00..7761ad3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
>> break;
>> }
>> adev->mc.vram_width = numchan * chansize;
>> - /* Could aper size report 0 ? */
>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>> /* size in MB on si */
>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
>>
>> + if (!(adev->flags & AMD_IS_APU))
>> + amdgpu_resize_bar0(adev);
>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>> +
>> #ifdef CONFIG_X86_64
>> if (adev->flags & AMD_IS_APU) {
>> adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> [email protected]
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel


2017-03-15 08:25:52

by Zhou, David(ChunMing)

[permalink] [raw]
Subject: RE: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

Does that means we don't need invisible vram later?

David

-----Original Message-----
From: dri-devel [mailto:[email protected]] On Behalf Of Christian K?nig
Sent: Wednesday, March 15, 2017 3:38 PM
To: Ayyappa Ch <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

Carizzo is an APU and resizing BARs isn't needed nor supported there.
The CPU can access the full stolen VRAM directly on that hardware.

As far as I know ASICs with support for this are Tonga, Fiji and all Polaris variants.

Christian.

Am 15.03.2017 um 08:23 schrieb Ayyappa Ch:
> Is it possible on Carrizo asics? Or only supports on newer asics?
>
> On Mon, Mar 13, 2017 at 6:11 PM, Christian König
> <[email protected]> wrote:
>> From: Christian König <[email protected]>
>>
>> Try to resize BAR0 to let CPU access all of VRAM.
>>
>> Signed-off-by: Christian König <[email protected]>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29 +++++++++++++++++++++++++++++
>> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 +++++---
>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 +++++---
>> 4 files changed, 40 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 3b81ded..905ded9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_tt *ttm,
>> struct ttm_mem_reg *mem);
>> void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base);
>> void amdgpu_gtt_location(struct amdgpu_device *adev, struct
>> amdgpu_mc *mc);
>> +void amdgpu_resize_bar0(struct amdgpu_device *adev);
>> void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size);
>> int amdgpu_ttm_init(struct amdgpu_device *adev);
>> void amdgpu_ttm_fini(struct amdgpu_device *adev); diff --git
>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 118f4e6..92955fe 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc)
>> mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
>> }
>>
>> +/**
>> + * amdgpu_resize_bar0 - try to resize BAR0
>> + *
>> + * @adev: amdgpu_device pointer
>> + *
>> + * Try to resize BAR0 to make all VRAM CPU accessible.
>> + */
>> +void amdgpu_resize_bar0(struct amdgpu_device *adev) {
>> + u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
>> + int r;
>> +
>> + r = pci_resize_resource(adev->pdev, 0, size);
>> +
>> + if (r == -ENOTSUPP) {
>> + /* The hardware don't support the extension. */
>> + return;
>> +
>> + } else if (r == -ENOSPC) {
>> + DRM_INFO("Not enoigh PCI address space for a large BAR.");
>> + } else if (r) {
>> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
>> + }
>> +
>> + /* Reinit the doorbell mapping, it is most likely moved as well */
>> + amdgpu_doorbell_fini(adev);
>> + BUG_ON(amdgpu_doorbell_init(adev));
>> +}
>> +
>> /*
>> * GPU helpers function.
>> */
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> index dc9b6d6..36a7aa5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
>> break;
>> }
>> adev->mc.vram_width = numchan * chansize;
>> - /* Could aper size report 0 ? */
>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>> /* size in MB on si */
>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL
>> * 1024ULL;
>>
>> + if (!(adev->flags & AMD_IS_APU))
>> + amdgpu_resize_bar0(adev);
>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>> +
>> #ifdef CONFIG_X86_64
>> if (adev->flags & AMD_IS_APU) {
>> adev->mc.aper_base =
>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22; diff --git
>> a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> index c087b00..7761ad3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
>> break;
>> }
>> adev->mc.vram_width = numchan * chansize;
>> - /* Could aper size report 0 ? */
>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>> /* size in MB on si */
>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL
>> * 1024ULL;
>>
>> + if (!(adev->flags & AMD_IS_APU))
>> + amdgpu_resize_bar0(adev);
>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>> +
>> #ifdef CONFIG_X86_64
>> if (adev->flags & AMD_IS_APU) {
>> adev->mc.aper_base =
>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> [email protected]
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel


_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

2017-03-15 09:29:29

by Christian König

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

Yes, exactly that.

Christian.

Am 15.03.2017 um 09:25 schrieb Zhou, David(ChunMing):
> Does that means we don't need invisible vram later?
>
> David
>
> -----Original Message-----
> From: dri-devel [mailto:[email protected]] On Behalf Of Christian K?nig
> Sent: Wednesday, March 15, 2017 3:38 PM
> To: Ayyappa Ch <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
>
> Carizzo is an APU and resizing BARs isn't needed nor supported there.
> The CPU can access the full stolen VRAM directly on that hardware.
>
> As far as I know ASICs with support for this are Tonga, Fiji and all Polaris variants.
>
> Christian.
>
> Am 15.03.2017 um 08:23 schrieb Ayyappa Ch:
>> Is it possible on Carrizo asics? Or only supports on newer asics?
>>
>> On Mon, Mar 13, 2017 at 6:11 PM, Christian König
>> <[email protected]> wrote:
>>> From: Christian König <[email protected]>
>>>
>>> Try to resize BAR0 to let CPU access all of VRAM.
>>>
>>> Signed-off-by: Christian König <[email protected]>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29 +++++++++++++++++++++++++++++
>>> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 +++++---
>>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 +++++---
>>> 4 files changed, 40 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> index 3b81ded..905ded9 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_tt *ttm,
>>> struct ttm_mem_reg *mem);
>>> void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base);
>>> void amdgpu_gtt_location(struct amdgpu_device *adev, struct
>>> amdgpu_mc *mc);
>>> +void amdgpu_resize_bar0(struct amdgpu_device *adev);
>>> void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size);
>>> int amdgpu_ttm_init(struct amdgpu_device *adev);
>>> void amdgpu_ttm_fini(struct amdgpu_device *adev); diff --git
>>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 118f4e6..92955fe 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc)
>>> mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
>>> }
>>>
>>> +/**
>>> + * amdgpu_resize_bar0 - try to resize BAR0
>>> + *
>>> + * @adev: amdgpu_device pointer
>>> + *
>>> + * Try to resize BAR0 to make all VRAM CPU accessible.
>>> + */
>>> +void amdgpu_resize_bar0(struct amdgpu_device *adev) {
>>> + u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
>>> + int r;
>>> +
>>> + r = pci_resize_resource(adev->pdev, 0, size);
>>> +
>>> + if (r == -ENOTSUPP) {
>>> + /* The hardware don't support the extension. */
>>> + return;
>>> +
>>> + } else if (r == -ENOSPC) {
>>> + DRM_INFO("Not enoigh PCI address space for a large BAR.");
>>> + } else if (r) {
>>> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
>>> + }
>>> +
>>> + /* Reinit the doorbell mapping, it is most likely moved as well */
>>> + amdgpu_doorbell_fini(adev);
>>> + BUG_ON(amdgpu_doorbell_init(adev));
>>> +}
>>> +
>>> /*
>>> * GPU helpers function.
>>> */
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> index dc9b6d6..36a7aa5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> @@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
>>> break;
>>> }
>>> adev->mc.vram_width = numchan * chansize;
>>> - /* Could aper size report 0 ? */
>>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>> /* size in MB on si */
>>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
>>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL
>>> * 1024ULL;
>>>
>>> + if (!(adev->flags & AMD_IS_APU))
>>> + amdgpu_resize_bar0(adev);
>>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>> +
>>> #ifdef CONFIG_X86_64
>>> if (adev->flags & AMD_IS_APU) {
>>> adev->mc.aper_base =
>>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22; diff --git
>>> a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> index c087b00..7761ad3 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> @@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
>>> break;
>>> }
>>> adev->mc.vram_width = numchan * chansize;
>>> - /* Could aper size report 0 ? */
>>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>> /* size in MB on si */
>>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
>>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL
>>> * 1024ULL;
>>>
>>> + if (!(adev->flags & AMD_IS_APU))
>>> + amdgpu_resize_bar0(adev);
>>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>> +
>>> #ifdef CONFIG_X86_64
>>> if (adev->flags & AMD_IS_APU) {
>>> adev->mc.aper_base =
>>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> [email protected]
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> amd-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


2017-03-15 10:42:41

by Ayyappa Ch

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

It also needs any support from VBIOS side ? I mean PCIe large bar support?

Thanks,
Ayyappa.

On Wed, Mar 15, 2017 at 1:07 PM, Christian König
<[email protected]> wrote:
> Carizzo is an APU and resizing BARs isn't needed nor supported there. The
> CPU can access the full stolen VRAM directly on that hardware.
>
> As far as I know ASICs with support for this are Tonga, Fiji and all Polaris
> variants.
>
> Christian.
>
>
> Am 15.03.2017 um 08:23 schrieb Ayyappa Ch:
>>
>> Is it possible on Carrizo asics? Or only supports on newer asics?
>>
>> On Mon, Mar 13, 2017 at 6:11 PM, Christian König
>> <[email protected]> wrote:
>>>
>>> From: Christian König <[email protected]>
>>>
>>> Try to resize BAR0 to let CPU access all of VRAM.
>>>
>>> Signed-off-by: Christian König <[email protected]>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29
>>> +++++++++++++++++++++++++++++
>>> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 +++++---
>>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 +++++---
>>> 4 files changed, 40 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> index 3b81ded..905ded9 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct
>>> amdgpu_device *adev, struct ttm_tt *ttm,
>>> struct ttm_mem_reg *mem);
>>> void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc
>>> *mc, u64 base);
>>> void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc
>>> *mc);
>>> +void amdgpu_resize_bar0(struct amdgpu_device *adev);
>>> void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64
>>> size);
>>> int amdgpu_ttm_init(struct amdgpu_device *adev);
>>> void amdgpu_ttm_fini(struct amdgpu_device *adev);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 118f4e6..92955fe 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct amdgpu_device *adev,
>>> struct amdgpu_mc *mc)
>>> mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
>>> }
>>>
>>> +/**
>>> + * amdgpu_resize_bar0 - try to resize BAR0
>>> + *
>>> + * @adev: amdgpu_device pointer
>>> + *
>>> + * Try to resize BAR0 to make all VRAM CPU accessible.
>>> + */
>>> +void amdgpu_resize_bar0(struct amdgpu_device *adev)
>>> +{
>>> + u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
>>> + int r;
>>> +
>>> + r = pci_resize_resource(adev->pdev, 0, size);
>>> +
>>> + if (r == -ENOTSUPP) {
>>> + /* The hardware don't support the extension. */
>>> + return;
>>> +
>>> + } else if (r == -ENOSPC) {
>>> + DRM_INFO("Not enoigh PCI address space for a large
>>> BAR.");
>>> + } else if (r) {
>>> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
>>> + }
>>> +
>>> + /* Reinit the doorbell mapping, it is most likely moved as well
>>> */
>>> + amdgpu_doorbell_fini(adev);
>>> + BUG_ON(amdgpu_doorbell_init(adev));
>>> +}
>>> +
>>> /*
>>> * GPU helpers function.
>>> */
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> index dc9b6d6..36a7aa5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> @@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device
>>> *adev)
>>> break;
>>> }
>>> adev->mc.vram_width = numchan * chansize;
>>> - /* Could aper size report 0 ? */
>>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>> /* size in MB on si */
>>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL *
>>> 1024ULL;
>>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL *
>>> 1024ULL;
>>>
>>> + if (!(adev->flags & AMD_IS_APU))
>>> + amdgpu_resize_bar0(adev);
>>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>> +
>>> #ifdef CONFIG_X86_64
>>> if (adev->flags & AMD_IS_APU) {
>>> adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) <<
>>> 22;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> index c087b00..7761ad3 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> @@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device
>>> *adev)
>>> break;
>>> }
>>> adev->mc.vram_width = numchan * chansize;
>>> - /* Could aper size report 0 ? */
>>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>> /* size in MB on si */
>>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL *
>>> 1024ULL;
>>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL *
>>> 1024ULL;
>>>
>>> + if (!(adev->flags & AMD_IS_APU))
>>> + amdgpu_resize_bar0(adev);
>>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>> +
>>> #ifdef CONFIG_X86_64
>>> if (adev->flags & AMD_IS_APU) {
>>> adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) <<
>>> 22;
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> [email protected]
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>

2017-03-15 11:04:23

by Christian König

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

No, we resize the BAR on the fly during driver load without help from
the BIOS or VBIOS.

Christian.

Am 15.03.2017 um 11:42 schrieb Ayyappa Ch:
> It also needs any support from VBIOS side ? I mean PCIe large bar support?
>
> Thanks,
> Ayyappa.
>
> On Wed, Mar 15, 2017 at 1:07 PM, Christian König
> <[email protected]> wrote:
>> Carizzo is an APU and resizing BARs isn't needed nor supported there. The
>> CPU can access the full stolen VRAM directly on that hardware.
>>
>> As far as I know ASICs with support for this are Tonga, Fiji and all Polaris
>> variants.
>>
>> Christian.
>>
>>
>> Am 15.03.2017 um 08:23 schrieb Ayyappa Ch:
>>> Is it possible on Carrizo asics? Or only supports on newer asics?
>>>
>>> On Mon, Mar 13, 2017 at 6:11 PM, Christian König
>>> <[email protected]> wrote:
>>>> From: Christian König <[email protected]>
>>>>
>>>> Try to resize BAR0 to let CPU access all of VRAM.
>>>>
>>>> Signed-off-by: Christian König <[email protected]>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29
>>>> +++++++++++++++++++++++++++++
>>>> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 +++++---
>>>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 +++++---
>>>> 4 files changed, 40 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> index 3b81ded..905ded9 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> @@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct
>>>> amdgpu_device *adev, struct ttm_tt *ttm,
>>>> struct ttm_mem_reg *mem);
>>>> void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc
>>>> *mc, u64 base);
>>>> void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc
>>>> *mc);
>>>> +void amdgpu_resize_bar0(struct amdgpu_device *adev);
>>>> void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64
>>>> size);
>>>> int amdgpu_ttm_init(struct amdgpu_device *adev);
>>>> void amdgpu_ttm_fini(struct amdgpu_device *adev);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> index 118f4e6..92955fe 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> @@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct amdgpu_device *adev,
>>>> struct amdgpu_mc *mc)
>>>> mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
>>>> }
>>>>
>>>> +/**
>>>> + * amdgpu_resize_bar0 - try to resize BAR0
>>>> + *
>>>> + * @adev: amdgpu_device pointer
>>>> + *
>>>> + * Try to resize BAR0 to make all VRAM CPU accessible.
>>>> + */
>>>> +void amdgpu_resize_bar0(struct amdgpu_device *adev)
>>>> +{
>>>> + u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
>>>> + int r;
>>>> +
>>>> + r = pci_resize_resource(adev->pdev, 0, size);
>>>> +
>>>> + if (r == -ENOTSUPP) {
>>>> + /* The hardware don't support the extension. */
>>>> + return;
>>>> +
>>>> + } else if (r == -ENOSPC) {
>>>> + DRM_INFO("Not enoigh PCI address space for a large
>>>> BAR.");
>>>> + } else if (r) {
>>>> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
>>>> + }
>>>> +
>>>> + /* Reinit the doorbell mapping, it is most likely moved as well
>>>> */
>>>> + amdgpu_doorbell_fini(adev);
>>>> + BUG_ON(amdgpu_doorbell_init(adev));
>>>> +}
>>>> +
>>>> /*
>>>> * GPU helpers function.
>>>> */
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>> index dc9b6d6..36a7aa5 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>> @@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device
>>>> *adev)
>>>> break;
>>>> }
>>>> adev->mc.vram_width = numchan * chansize;
>>>> - /* Could aper size report 0 ? */
>>>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>>> /* size in MB on si */
>>>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL *
>>>> 1024ULL;
>>>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL *
>>>> 1024ULL;
>>>>
>>>> + if (!(adev->flags & AMD_IS_APU))
>>>> + amdgpu_resize_bar0(adev);
>>>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>>> +
>>>> #ifdef CONFIG_X86_64
>>>> if (adev->flags & AMD_IS_APU) {
>>>> adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) <<
>>>> 22;
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>> index c087b00..7761ad3 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>> @@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device
>>>> *adev)
>>>> break;
>>>> }
>>>> adev->mc.vram_width = numchan * chansize;
>>>> - /* Could aper size report 0 ? */
>>>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>>> /* size in MB on si */
>>>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL *
>>>> 1024ULL;
>>>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL *
>>>> 1024ULL;
>>>>
>>>> + if (!(adev->flags & AMD_IS_APU))
>>>> + amdgpu_resize_bar0(adev);
>>>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>>> +
>>>> #ifdef CONFIG_X86_64
>>>> if (adev->flags & AMD_IS_APU) {
>>>> adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) <<
>>>> 22;
>>>> --
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> [email protected]
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>>

2017-03-15 16:09:33

by Deucher, Alexander

[permalink] [raw]
Subject: RE: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

> -----Original Message-----
> From: amd-gfx [mailto:[email protected]] On Behalf
> Of Christian König
> Sent: Wednesday, March 15, 2017 3:38 AM
> To: Ayyappa Ch
> Cc: [email protected]; [email protected]; amd-
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
>
> Carizzo is an APU and resizing BARs isn't needed nor supported there.
> The CPU can access the full stolen VRAM directly on that hardware.
>
> As far as I know ASICs with support for this are Tonga, Fiji and all
> Polaris variants.

I think resizable BARs are supported as far back as evergreen or NI.

Alex

>
> Christian.
>
> Am 15.03.2017 um 08:23 schrieb Ayyappa Ch:
> > Is it possible on Carrizo asics? Or only supports on newer asics?
> >
> > On Mon, Mar 13, 2017 at 6:11 PM, Christian König
> > <[email protected]> wrote:
> >> From: Christian König <[email protected]>
> >>
> >> Try to resize BAR0 to let CPU access all of VRAM.
> >>
> >> Signed-off-by: Christian König <[email protected]>
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29
> +++++++++++++++++++++++++++++
> >> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 +++++---
> >> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 +++++---
> >> 4 files changed, 40 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> index 3b81ded..905ded9 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> @@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct
> amdgpu_device *adev, struct ttm_tt *ttm,
> >> struct ttm_mem_reg *mem);
> >> void amdgpu_vram_location(struct amdgpu_device *adev, struct
> amdgpu_mc *mc, u64 base);
> >> void amdgpu_gtt_location(struct amdgpu_device *adev, struct
> amdgpu_mc *mc);
> >> +void amdgpu_resize_bar0(struct amdgpu_device *adev);
> >> void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev,
> u64 size);
> >> int amdgpu_ttm_init(struct amdgpu_device *adev);
> >> void amdgpu_ttm_fini(struct amdgpu_device *adev);
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> index 118f4e6..92955fe 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> @@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct amdgpu_device
> *adev, struct amdgpu_mc *mc)
> >> mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
> >> }
> >>
> >> +/**
> >> + * amdgpu_resize_bar0 - try to resize BAR0
> >> + *
> >> + * @adev: amdgpu_device pointer
> >> + *
> >> + * Try to resize BAR0 to make all VRAM CPU accessible.
> >> + */
> >> +void amdgpu_resize_bar0(struct amdgpu_device *adev)
> >> +{
> >> + u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
> >> + int r;
> >> +
> >> + r = pci_resize_resource(adev->pdev, 0, size);
> >> +
> >> + if (r == -ENOTSUPP) {
> >> + /* The hardware don't support the extension. */
> >> + return;
> >> +
> >> + } else if (r == -ENOSPC) {
> >> + DRM_INFO("Not enoigh PCI address space for a large BAR.");
> >> + } else if (r) {
> >> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
> >> + }
> >> +
> >> + /* Reinit the doorbell mapping, it is most likely moved as well */
> >> + amdgpu_doorbell_fini(adev);
> >> + BUG_ON(amdgpu_doorbell_init(adev));
> >> +}
> >> +
> >> /*
> >> * GPU helpers function.
> >> */
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >> index dc9b6d6..36a7aa5 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >> @@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct
> amdgpu_device *adev)
> >> break;
> >> }
> >> adev->mc.vram_width = numchan * chansize;
> >> - /* Could aper size report 0 ? */
> >> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >> /* size in MB on si */
> >> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> 1024ULL * 1024ULL;
> >> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> 1024ULL * 1024ULL;
> >>
> >> + if (!(adev->flags & AMD_IS_APU))
> >> + amdgpu_resize_bar0(adev);
> >> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >> +
> >> #ifdef CONFIG_X86_64
> >> if (adev->flags & AMD_IS_APU) {
> >> adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET))
> << 22;
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >> index c087b00..7761ad3 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >> @@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct
> amdgpu_device *adev)
> >> break;
> >> }
> >> adev->mc.vram_width = numchan * chansize;
> >> - /* Could aper size report 0 ? */
> >> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >> /* size in MB on si */
> >> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> 1024ULL * 1024ULL;
> >> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> 1024ULL * 1024ULL;
> >>
> >> + if (!(adev->flags & AMD_IS_APU))
> >> + amdgpu_resize_bar0(adev);
> >> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >> +
> >> #ifdef CONFIG_X86_64
> >> if (adev->flags & AMD_IS_APU) {
> >> adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET))
> << 22;
> >> --
> >> 2.7.4
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> [email protected]
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
> _______________________________________________
> amd-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

2017-03-16 02:19:41

by Zhang, Jerry(Junwei)

[permalink] [raw]
Subject: RE: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

> -----Original Message-----
> From: dri-devel [mailto:[email protected]] On Behalf Of
> Christian K?nig
> Sent: Wednesday, March 15, 2017 17:29
> To: Zhou, David(ChunMing); Ayyappa Ch
> Cc: [email protected]; [email protected]; amd-
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
>
> Yes, exactly that.

(I'm not familiar with PCI too much.)
Is there any restrict for PCI device?
I'm concerning if any PCI couldn't support it on some motherboard.

>
> Christian.
>
> Am 15.03.2017 um 09:25 schrieb Zhou, David(ChunMing):
> > Does that means we don't need invisible vram later?
> >
> > David
> >
> > -----Original Message-----
> > From: dri-devel [mailto:[email protected]] On
> > Behalf Of Christian K?nig
> > Sent: Wednesday, March 15, 2017 3:38 PM
> > To: Ayyappa Ch <[email protected]>
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]
> > Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
> >
> > Carizzo is an APU and resizing BARs isn't needed nor supported there.
> > The CPU can access the full stolen VRAM directly on that hardware.
> >
> > As far as I know ASICs with support for this are Tonga, Fiji and all Polaris variants.
> >
> > Christian.
> >
> > Am 15.03.2017 um 08:23 schrieb Ayyappa Ch:
> >> Is it possible on Carrizo asics? Or only supports on newer asics?
> >>
> >> On Mon, Mar 13, 2017 at 6:11 PM, Christian König
> >> <[email protected]> wrote:
> >>> From: Christian König <[email protected]>
> >>>
> >>> Try to resize BAR0 to let CPU access all of VRAM.
> >>>
> >>> Signed-off-by: Christian König <[email protected]>
> >>> ---
> >>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29
> +++++++++++++++++++++++++++++
> >>> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 +++++---
> >>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 +++++---
> >>> 4 files changed, 40 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> index 3b81ded..905ded9 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> @@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct
> amdgpu_device *adev, struct ttm_tt *ttm,
> >>> struct ttm_mem_reg *mem);
> >>> void amdgpu_vram_location(struct amdgpu_device *adev, struct
> amdgpu_mc *mc, u64 base);
> >>> void amdgpu_gtt_location(struct amdgpu_device *adev, struct
> >>> amdgpu_mc *mc);
> >>> +void amdgpu_resize_bar0(struct amdgpu_device *adev);
> >>> void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev,
> u64 size);
> >>> int amdgpu_ttm_init(struct amdgpu_device *adev);
> >>> void amdgpu_ttm_fini(struct amdgpu_device *adev); diff --git
> >>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> index 118f4e6..92955fe 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> @@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct amdgpu_device
> *adev, struct amdgpu_mc *mc)
> >>> mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
> >>> }
> >>>
> >>> +/**
> >>> + * amdgpu_resize_bar0 - try to resize BAR0
> >>> + *
> >>> + * @adev: amdgpu_device pointer
> >>> + *
> >>> + * Try to resize BAR0 to make all VRAM CPU accessible.
> >>> + */
> >>> +void amdgpu_resize_bar0(struct amdgpu_device *adev) {
> >>> + u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
> >>> + int r;
> >>> +
> >>> + r = pci_resize_resource(adev->pdev, 0, size);
> >>> +
> >>> + if (r == -ENOTSUPP) {
> >>> + /* The hardware don't support the extension. */
> >>> + return;
> >>> +
> >>> + } else if (r == -ENOSPC) {
> >>> + DRM_INFO("Not enoigh PCI address space for a large BAR.");
> >>> + } else if (r) {
> >>> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
> >>> + }
> >>> +
> >>> + /* Reinit the doorbell mapping, it is most likely moved as well */
> >>> + amdgpu_doorbell_fini(adev);
> >>> + BUG_ON(amdgpu_doorbell_init(adev));
> >>> +}
> >>> +
> >>> /*
> >>> * GPU helpers function.
> >>> */
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >>> index dc9b6d6..36a7aa5 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >>> @@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device
> *adev)
> >>> break;
> >>> }
> >>> adev->mc.vram_width = numchan * chansize;
> >>> - /* Could aper size report 0 ? */
> >>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >>> /* size in MB on si */
> >>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL *
> 1024ULL;
> >>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> >>> 1024ULL
> >>> * 1024ULL;
> >>>
> >>> + if (!(adev->flags & AMD_IS_APU))
> >>> + amdgpu_resize_bar0(adev);
> >>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >>> +
> >>> #ifdef CONFIG_X86_64
> >>> if (adev->flags & AMD_IS_APU) {
> >>> adev->mc.aper_base =
> >>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22; diff --git
> >>> a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >>> index c087b00..7761ad3 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >>> @@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device
> *adev)
> >>> break;
> >>> }
> >>> adev->mc.vram_width = numchan * chansize;
> >>> - /* Could aper size report 0 ? */
> >>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >>> /* size in MB on si */
> >>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL *
> 1024ULL;
> >>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> >>> 1024ULL
> >>> * 1024ULL;
> >>>
> >>> + if (!(adev->flags & AMD_IS_APU))
> >>> + amdgpu_resize_bar0(adev);
> >>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >>> +
> >>> #ifdef CONFIG_X86_64
> >>> if (adev->flags & AMD_IS_APU) {
> >>> adev->mc.aper_base =
> >>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
> >>> --
> >>> 2.7.4
> >>>
> >>> _______________________________________________
> >>> dri-devel mailing list
> >>> [email protected]
> >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > _______________________________________________
> > dri-devel mailing list
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > _______________________________________________
> > amd-gfx mailing list
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

2017-03-16 02:25:13

by Alex Deucher

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

On Wed, Mar 15, 2017 at 10:19 PM, Zhang, Jerry <[email protected]> wrote:
>> -----Original Message-----
>> From: dri-devel [mailto:[email protected]] On Behalf Of
>> Christian K?nig
>> Sent: Wednesday, March 15, 2017 17:29
>> To: Zhou, David(ChunMing); Ayyappa Ch
>> Cc: [email protected]; [email protected]; amd-
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
>>
>> Yes, exactly that.
>
> (I'm not familiar with PCI too much.)
> Is there any restrict for PCI device?
> I'm concerning if any PCI couldn't support it on some motherboard.

It depends on the PCI root bridge. This patch set only implements
support for AMD root bridges. Intel and other vendors would need
similar code.

Alex

>
>>
>> Christian.
>>
>> Am 15.03.2017 um 09:25 schrieb Zhou, David(ChunMing):
>> > Does that means we don't need invisible vram later?
>> >
>> > David
>> >
>> > -----Original Message-----
>> > From: dri-devel [mailto:[email protected]] On
>> > Behalf Of Christian K?nig
>> > Sent: Wednesday, March 15, 2017 3:38 PM
>> > To: Ayyappa Ch <[email protected]>
>> > Cc: [email protected]; [email protected];
>> > [email protected]; [email protected];
>> > [email protected]; [email protected]
>> > Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
>> >
>> > Carizzo is an APU and resizing BARs isn't needed nor supported there.
>> > The CPU can access the full stolen VRAM directly on that hardware.
>> >
>> > As far as I know ASICs with support for this are Tonga, Fiji and all Polaris variants.
>> >
>> > Christian.
>> >
>> > Am 15.03.2017 um 08:23 schrieb Ayyappa Ch:
>> >> Is it possible on Carrizo asics? Or only supports on newer asics?
>> >>
>> >> On Mon, Mar 13, 2017 at 6:11 PM, Christian König
>> >> <[email protected]> wrote:
>> >>> From: Christian König <[email protected]>
>> >>>
>> >>> Try to resize BAR0 to let CPU access all of VRAM.
>> >>>
>> >>> Signed-off-by: Christian König <[email protected]>
>> >>> ---
>> >>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
>> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29
>> +++++++++++++++++++++++++++++
>> >>> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 +++++---
>> >>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 +++++---
>> >>> 4 files changed, 40 insertions(+), 6 deletions(-)
>> >>>
>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> >>> index 3b81ded..905ded9 100644
>> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> >>> @@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct
>> amdgpu_device *adev, struct ttm_tt *ttm,
>> >>> struct ttm_mem_reg *mem);
>> >>> void amdgpu_vram_location(struct amdgpu_device *adev, struct
>> amdgpu_mc *mc, u64 base);
>> >>> void amdgpu_gtt_location(struct amdgpu_device *adev, struct
>> >>> amdgpu_mc *mc);
>> >>> +void amdgpu_resize_bar0(struct amdgpu_device *adev);
>> >>> void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev,
>> u64 size);
>> >>> int amdgpu_ttm_init(struct amdgpu_device *adev);
>> >>> void amdgpu_ttm_fini(struct amdgpu_device *adev); diff --git
>> >>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >>> index 118f4e6..92955fe 100644
>> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >>> @@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct amdgpu_device
>> *adev, struct amdgpu_mc *mc)
>> >>> mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
>> >>> }
>> >>>
>> >>> +/**
>> >>> + * amdgpu_resize_bar0 - try to resize BAR0
>> >>> + *
>> >>> + * @adev: amdgpu_device pointer
>> >>> + *
>> >>> + * Try to resize BAR0 to make all VRAM CPU accessible.
>> >>> + */
>> >>> +void amdgpu_resize_bar0(struct amdgpu_device *adev) {
>> >>> + u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
>> >>> + int r;
>> >>> +
>> >>> + r = pci_resize_resource(adev->pdev, 0, size);
>> >>> +
>> >>> + if (r == -ENOTSUPP) {
>> >>> + /* The hardware don't support the extension. */
>> >>> + return;
>> >>> +
>> >>> + } else if (r == -ENOSPC) {
>> >>> + DRM_INFO("Not enoigh PCI address space for a large BAR.");
>> >>> + } else if (r) {
>> >>> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
>> >>> + }
>> >>> +
>> >>> + /* Reinit the doorbell mapping, it is most likely moved as well */
>> >>> + amdgpu_doorbell_fini(adev);
>> >>> + BUG_ON(amdgpu_doorbell_init(adev));
>> >>> +}
>> >>> +
>> >>> /*
>> >>> * GPU helpers function.
>> >>> */
>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> >>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> >>> index dc9b6d6..36a7aa5 100644
>> >>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> >>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> >>> @@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device
>> *adev)
>> >>> break;
>> >>> }
>> >>> adev->mc.vram_width = numchan * chansize;
>> >>> - /* Could aper size report 0 ? */
>> >>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>> >>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>> >>> /* size in MB on si */
>> >>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL *
>> 1024ULL;
>> >>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) *
>> >>> 1024ULL
>> >>> * 1024ULL;
>> >>>
>> >>> + if (!(adev->flags & AMD_IS_APU))
>> >>> + amdgpu_resize_bar0(adev);
>> >>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>> >>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>> >>> +
>> >>> #ifdef CONFIG_X86_64
>> >>> if (adev->flags & AMD_IS_APU) {
>> >>> adev->mc.aper_base =
>> >>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22; diff --git
>> >>> a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> >>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> >>> index c087b00..7761ad3 100644
>> >>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> >>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> >>> @@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device
>> *adev)
>> >>> break;
>> >>> }
>> >>> adev->mc.vram_width = numchan * chansize;
>> >>> - /* Could aper size report 0 ? */
>> >>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>> >>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>> >>> /* size in MB on si */
>> >>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL *
>> 1024ULL;
>> >>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) *
>> >>> 1024ULL
>> >>> * 1024ULL;
>> >>>
>> >>> + if (!(adev->flags & AMD_IS_APU))
>> >>> + amdgpu_resize_bar0(adev);
>> >>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>> >>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>> >>> +
>> >>> #ifdef CONFIG_X86_64
>> >>> if (adev->flags & AMD_IS_APU) {
>> >>> adev->mc.aper_base =
>> >>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
>> >>> --
>> >>> 2.7.4
>> >>>
>> >>> _______________________________________________
>> >>> dri-devel mailing list
>> >>> [email protected]
>> >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> >
>> > _______________________________________________
>> > dri-devel mailing list
>> > [email protected]
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> > _______________________________________________
>> > amd-gfx mailing list
>> > [email protected]
>> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> [email protected]
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> amd-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

2017-03-16 02:41:33

by Zhang, Jerry(Junwei)

[permalink] [raw]
Subject: RE: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

Thanks for your info.
I see.

Regards,
Jerry (Junwei Zhang)

Linux Base Graphics
SRDC Software Development
_____________________________________


> -----Original Message-----
> From: Alex Deucher [mailto:[email protected]]
> Sent: Thursday, March 16, 2017 10:25
> To: Zhang, Jerry
> Cc: Christian König; Zhou, David(ChunMing); Ayyappa Ch; linux-
> [email protected]; [email protected]; dri-
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
>
> On Wed, Mar 15, 2017 at 10:19 PM, Zhang, Jerry <[email protected]> wrote:
> >> -----Original Message-----
> >> From: dri-devel [mailto:[email protected]] On
> >> Behalf Of Christian K?nig
> >> Sent: Wednesday, March 15, 2017 17:29
> >> To: Zhou, David(ChunMing); Ayyappa Ch
> >> Cc: [email protected]; [email protected]; amd-
> >> [email protected]; [email protected];
> >> [email protected]; [email protected]
> >> Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
> >>
> >> Yes, exactly that.
> >
> > (I'm not familiar with PCI too much.)
> > Is there any restrict for PCI device?
> > I'm concerning if any PCI couldn't support it on some motherboard.
>
> It depends on the PCI root bridge. This patch set only implements support for
> AMD root bridges. Intel and other vendors would need similar code.
>
> Alex
>
> >
> >>
> >> Christian.
> >>
> >> Am 15.03.2017 um 09:25 schrieb Zhou, David(ChunMing):
> >> > Does that means we don't need invisible vram later?
> >> >
> >> > David
> >> >
> >> > -----Original Message-----
> >> > From: dri-devel [mailto:[email protected]] On
> >> > Behalf Of Christian K?nig
> >> > Sent: Wednesday, March 15, 2017 3:38 PM
> >> > To: Ayyappa Ch <[email protected]>
> >> > Cc: [email protected]; [email protected];
> >> > [email protected]; [email protected];
> >> > [email protected]; [email protected]
> >> > Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
> >> >
> >> > Carizzo is an APU and resizing BARs isn't needed nor supported there.
> >> > The CPU can access the full stolen VRAM directly on that hardware.
> >> >
> >> > As far as I know ASICs with support for this are Tonga, Fiji and all Polaris
> variants.
> >> >
> >> > Christian.
> >> >
> >> > Am 15.03.2017 um 08:23 schrieb Ayyappa Ch:
> >> >> Is it possible on Carrizo asics? Or only supports on newer asics?
> >> >>
> >> >> On Mon, Mar 13, 2017 at 6:11 PM, Christian König
> >> >> <[email protected]> wrote:
> >> >>> From: Christian König <[email protected]>
> >> >>>
> >> >>> Try to resize BAR0 to let CPU access all of VRAM.
> >> >>>
> >> >>> Signed-off-by: Christian König <[email protected]>
> >> >>> ---
> >> >>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> >> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29
> >> +++++++++++++++++++++++++++++
> >> >>> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 +++++---
> >> >>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 +++++---
> >> >>> 4 files changed, 40 insertions(+), 6 deletions(-)
> >> >>>
> >> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> >>> index 3b81ded..905ded9 100644
> >> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> >>> @@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct
> >> amdgpu_device *adev, struct ttm_tt *ttm,
> >> >>> struct ttm_mem_reg *mem);
> >> >>> void amdgpu_vram_location(struct amdgpu_device *adev, struct
> >> amdgpu_mc *mc, u64 base);
> >> >>> void amdgpu_gtt_location(struct amdgpu_device *adev, struct
> >> >>> amdgpu_mc *mc);
> >> >>> +void amdgpu_resize_bar0(struct amdgpu_device *adev);
> >> >>> void amdgpu_ttm_set_active_vram_size(struct amdgpu_device
> >> >>> *adev,
> >> u64 size);
> >> >>> int amdgpu_ttm_init(struct amdgpu_device *adev);
> >> >>> void amdgpu_ttm_fini(struct amdgpu_device *adev); diff --git
> >> >>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> >>> index 118f4e6..92955fe 100644
> >> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> >>> @@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct
> >> >>> amdgpu_device
> >> *adev, struct amdgpu_mc *mc)
> >> >>> mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
> >> >>> }
> >> >>>
> >> >>> +/**
> >> >>> + * amdgpu_resize_bar0 - try to resize BAR0
> >> >>> + *
> >> >>> + * @adev: amdgpu_device pointer
> >> >>> + *
> >> >>> + * Try to resize BAR0 to make all VRAM CPU accessible.
> >> >>> + */
> >> >>> +void amdgpu_resize_bar0(struct amdgpu_device *adev) {
> >> >>> + u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
> >> >>> + int r;
> >> >>> +
> >> >>> + r = pci_resize_resource(adev->pdev, 0, size);
> >> >>> +
> >> >>> + if (r == -ENOTSUPP) {
> >> >>> + /* The hardware don't support the extension. */
> >> >>> + return;
> >> >>> +
> >> >>> + } else if (r == -ENOSPC) {
> >> >>> + DRM_INFO("Not enoigh PCI address space for a large BAR.");
> >> >>> + } else if (r) {
> >> >>> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
> >> >>> + }
> >> >>> +
> >> >>> + /* Reinit the doorbell mapping, it is most likely moved as well */
> >> >>> + amdgpu_doorbell_fini(adev);
> >> >>> + BUG_ON(amdgpu_doorbell_init(adev));
> >> >>> +}
> >> >>> +
> >> >>> /*
> >> >>> * GPU helpers function.
> >> >>> */
> >> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >> >>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >> >>> index dc9b6d6..36a7aa5 100644
> >> >>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >> >>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >> >>> @@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct
> >> >>> amdgpu_device
> >> *adev)
> >> >>> break;
> >> >>> }
> >> >>> adev->mc.vram_width = numchan * chansize;
> >> >>> - /* Could aper size report 0 ? */
> >> >>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >> >>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >> >>> /* size in MB on si */
> >> >>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> >> >>> 1024ULL *
> >> 1024ULL;
> >> >>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> >> >>> 1024ULL
> >> >>> * 1024ULL;
> >> >>>
> >> >>> + if (!(adev->flags & AMD_IS_APU))
> >> >>> + amdgpu_resize_bar0(adev);
> >> >>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >> >>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >> >>> +
> >> >>> #ifdef CONFIG_X86_64
> >> >>> if (adev->flags & AMD_IS_APU) {
> >> >>> adev->mc.aper_base =
> >> >>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22; diff --git
> >> >>> a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >> >>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >> >>> index c087b00..7761ad3 100644
> >> >>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >> >>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >> >>> @@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct
> >> >>> amdgpu_device
> >> *adev)
> >> >>> break;
> >> >>> }
> >> >>> adev->mc.vram_width = numchan * chansize;
> >> >>> - /* Could aper size report 0 ? */
> >> >>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >> >>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >> >>> /* size in MB on si */
> >> >>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> >> >>> 1024ULL *
> >> 1024ULL;
> >> >>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> >> >>> 1024ULL
> >> >>> * 1024ULL;
> >> >>>
> >> >>> + if (!(adev->flags & AMD_IS_APU))
> >> >>> + amdgpu_resize_bar0(adev);
> >> >>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >> >>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >> >>> +
> >> >>> #ifdef CONFIG_X86_64
> >> >>> if (adev->flags & AMD_IS_APU) {
> >> >>> adev->mc.aper_base =
> >> >>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
> >> >>> --
> >> >>> 2.7.4
> >> >>>
> >> >>> _______________________________________________
> >> >>> dri-devel mailing list
> >> >>> [email protected]
> >> >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >> >
> >> > _______________________________________________
> >> > dri-devel mailing list
> >> > [email protected]
> >> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >> > _______________________________________________
> >> > amd-gfx mailing list
> >> > [email protected]
> >> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >>
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> [email protected]
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > _______________________________________________
> > amd-gfx mailing list
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx

2017-03-23 14:30:13

by Sagalovitch, Serguei

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

Christian,

- Are we going to support resizing BAR when kernel
modesetting is not enabled and we are running in console
under VBIOS control (VESA/VGA)?

- Should we restore PCI configuration if amdgpu
will be unloaded?

- In function amdgpu_resize_bar0():
If resizing for "max" size failed should we try other
sizes? What do you think?


Sincerely yours,
Serguei Sagalovitch


From: amd-gfx <[email protected]> on behalf of Zhang, Jerry <[email protected]>
Sent: March 15, 2017 10:41 PM
To: Alex Deucher
Cc: Zhou, David(ChunMing); Ayyappa Ch; [email protected]; [email protected]; [email protected]; [email protected]; Christian K?nig; [email protected]; [email protected]
Subject: RE: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
?
Thanks for your info.
I see.

Regards,
Jerry (Junwei Zhang)

Linux Base Graphics
SRDC Software Development
_____________________________________


> -----Original Message-----
> From: Alex Deucher [mailto:[email protected]]
> Sent: Thursday, March 16, 2017 10:25
> To: Zhang, Jerry
> Cc: Christian K?nig; Zhou, David(ChunMing); Ayyappa Ch; linux-
> [email protected]; [email protected]; dri-
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
>
> On Wed, Mar 15, 2017 at 10:19 PM, Zhang, Jerry <[email protected]> wrote:
> >> -----Original Message-----
> >> From: dri-devel [mailto:[email protected]] On
> >> Behalf Of Christian K?nig
> >> Sent: Wednesday, March 15, 2017 17:29
> >> To: Zhou, David(ChunMing); Ayyappa Ch
> >> Cc: [email protected]; [email protected]; amd-
> >> [email protected]; [email protected];
> >> [email protected]; [email protected]
> >> Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
> >>
> >> Yes, exactly that.
> >
> > (I'm not familiar with PCI too much.)
> > Is there any restrict for PCI device?
> > I'm concerning if any PCI couldn't support it on some motherboard.
>
> It depends on the PCI root bridge.? This patch set only implements support for
> AMD root bridges.? Intel and other vendors would need similar code.
>
> Alex
>
> >
> >>
> >> Christian.
> >>
> >> Am 15.03.2017 um 09:25 schrieb Zhou, David(ChunMing):
> >> > Does that means we don't need invisible vram later?
> >> >
> >> > David
> >> >
> >> > -----Original Message-----
> >> > From: dri-devel [mailto:[email protected]] On
> >> > Behalf Of Christian K?nig
> >> > Sent: Wednesday, March 15, 2017 3:38 PM
> >> > To: Ayyappa Ch <[email protected]>
> >> > Cc: [email protected]; [email protected];
> >> > [email protected]; [email protected];
> >> > [email protected]; [email protected]
> >> > Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
> >> >
> >> > Carizzo is an APU and resizing BARs isn't needed nor supported there.
> >> > The CPU can access the full stolen VRAM directly on that hardware.
> >> >
> >> > As far as I know ASICs with support for this are Tonga, Fiji and all Polaris
> variants.
> >> >
> >> > Christian.
> >> >
> >> > Am 15.03.2017 um 08:23 schrieb Ayyappa Ch:
> >> >> Is it possible on Carrizo asics? Or only supports on newer asics?
> >> >>
> >> >> On Mon, Mar 13, 2017 at 6:11 PM, Christian K?nig
> >> >> <[email protected]> wrote:
> >> >>> From: Christian K?nig <[email protected]>
> >> >>>
> >> >>> Try to resize BAR0 to let CPU access all of VRAM.
> >> >>>
> >> >>> Signed-off-by: Christian K?nig <[email protected]>
> >> >>> ---
> >> >>>??? drivers/gpu/drm/amd/amdgpu/amdgpu.h??????? |? 1 +
> >> >>>??? drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29
> >> +++++++++++++++++++++++++++++
> >> >>>??? drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c????? |? 8 +++++---
> >> >>>??? drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c????? |? 8 +++++---
> >> >>>??? 4 files changed, 40 insertions(+), 6 deletions(-)
> >> >>>
> >> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> >>> index 3b81ded..905ded9 100644
> >> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> >>> @@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct
> >> amdgpu_device *adev, struct ttm_tt *ttm,
> >> >>>??????????????????????????????????? struct ttm_mem_reg *mem);
> >> >>>??? void amdgpu_vram_location(struct amdgpu_device *adev, struct
> >> amdgpu_mc *mc, u64 base);
> >> >>>??? void amdgpu_gtt_location(struct amdgpu_device *adev, struct
> >> >>> amdgpu_mc *mc);
> >> >>> +void amdgpu_resize_bar0(struct amdgpu_device *adev);
> >> >>>??? void amdgpu_ttm_set_active_vram_size(struct amdgpu_device
> >> >>> *adev,
> >> u64 size);
> >> >>>??? int amdgpu_ttm_init(struct amdgpu_device *adev);
> >> >>>??? void amdgpu_ttm_fini(struct amdgpu_device *adev); diff --git
> >> >>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> >>> index 118f4e6..92955fe 100644
> >> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> >>> @@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct
> >> >>> amdgpu_device
> >> *adev, struct amdgpu_mc *mc)
> >> >>>?????????????????????????? mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
> >> >>>??? }
> >> >>>
> >> >>> +/**
> >> >>> + * amdgpu_resize_bar0 - try to resize BAR0
> >> >>> + *
> >> >>> + * @adev: amdgpu_device pointer
> >> >>> + *
> >> >>> + * Try to resize BAR0 to make all VRAM CPU accessible.
> >> >>> + */
> >> >>> +void amdgpu_resize_bar0(struct amdgpu_device *adev) {
> >> >>> +?????? u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
> >> >>> +?????? int r;
> >> >>> +
> >> >>> +?????? r = pci_resize_resource(adev->pdev, 0, size);
> >> >>> +
> >> >>> +?????? if (r == -ENOTSUPP) {
> >> >>> +?????????????? /* The hardware don't support the extension. */
> >> >>> +?????????????? return;
> >> >>> +
> >> >>> +?????? } else if (r == -ENOSPC) {
> >> >>> +?????????????? DRM_INFO("Not enoigh PCI address space for a large BAR.");
> >> >>> +?????? } else if (r) {
> >> >>> +?????????????? DRM_ERROR("Problem resizing BAR0 (%d).", r);
> >> >>> +?????? }
> >> >>> +
> >> >>> +?????? /* Reinit the doorbell mapping, it is most likely moved as well */
> >> >>> +?????? amdgpu_doorbell_fini(adev);
> >> >>> +?????? BUG_ON(amdgpu_doorbell_init(adev));
> >> >>> +}
> >> >>> +
> >> >>>??? /*
> >> >>>???? * GPU helpers function.
> >> >>>???? */
> >> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >> >>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >> >>> index dc9b6d6..36a7aa5 100644
> >> >>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >> >>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> >> >>> @@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct
> >> >>> amdgpu_device
> >> *adev)
> >> >>>?????????????????? break;
> >> >>>?????????? }
> >> >>>?????????? adev->mc.vram_width = numchan * chansize;
> >> >>> -?????? /* Could aper size report 0 ? */
> >> >>> -?????? adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >> >>> -?????? adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >> >>>?????????? /* size in MB on si */
> >> >>>?????????? adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> >> >>> 1024ULL *
> >> 1024ULL;
> >> >>>?????????? adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> >> >>> 1024ULL
> >> >>> * 1024ULL;
> >> >>>
> >> >>> +?????? if (!(adev->flags & AMD_IS_APU))
> >> >>> +?????????????? amdgpu_resize_bar0(adev);
> >> >>> +?????? adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >> >>> +?????? adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >> >>> +
> >> >>>??? #ifdef CONFIG_X86_64
> >> >>>?????????? if (adev->flags & AMD_IS_APU) {
> >> >>>?????????????????? adev->mc.aper_base =
> >> >>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22; diff --git
> >> >>> a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >> >>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >> >>> index c087b00..7761ad3 100644
> >> >>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >> >>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> >> >>> @@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct
> >> >>> amdgpu_device
> >> *adev)
> >> >>>?????????????????? break;
> >> >>>?????????? }
> >> >>>?????????? adev->mc.vram_width = numchan * chansize;
> >> >>> -?????? /* Could aper size report 0 ? */
> >> >>> -?????? adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >> >>> -?????? adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >> >>>?????????? /* size in MB on si */
> >> >>>?????????? adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> >> >>> 1024ULL *
> >> 1024ULL;
> >> >>>?????????? adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) *
> >> >>> 1024ULL
> >> >>> * 1024ULL;
> >> >>>
> >> >>> +?????? if (!(adev->flags & AMD_IS_APU))
> >> >>> +?????????????? amdgpu_resize_bar0(adev);
> >> >>> +?????? adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >> >>> +?????? adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> >> >>> +
> >> >>>??? #ifdef CONFIG_X86_64
> >> >>>?????????? if (adev->flags & AMD_IS_APU) {
> >> >>>?????????????????? adev->mc.aper_base =
> >> >>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
> >> >>> --
> >> >>> 2.7.4
> >> >>>
> >> >>> _______________________________________________
> >> >>> dri-devel mailing list
> >> >>> [email protected]
> >> >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >> >
> >> > _______________________________________________
> >> > dri-devel mailing list
> >> > [email protected]
> >> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >> > _______________________________________________
> >> > amd-gfx mailing list
> >> > [email protected]
> >> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >>
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> [email protected]
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > _______________________________________________
> > amd-gfx mailing list
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


2017-03-23 15:57:09

by Christian König

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

> - Are we going to support resizing BAR when kernel
> modesetting is not enabled and we are running in console
> under VBIOS control (VESA/VGA)?
No, initial I've tried to resize the PCI BAR during probing without the
help of the driver at all. But the VESA/EFI/VBIOS don't seem to be able
to handle addresses above 4GB for some reason.

So the approach is to let the driver kick the VESA/EFI drivers out and
then resize when we know that nobody is accessing the BAR.

That's the only approach I've found without either blacklisting VESA/EFI
drivers or crashing the system during the resize.

> - Should we restore PCI configuration if amdgpu
> will be unloaded?
Yeah, thought about the as well. I'm just not sure how to do it.

There is a lot of stuff we need to save and reset when the driver
unloads for not much gain.

> - In function amdgpu_resize_bar0():
> If resizing for "max" size failed should we try other
> sizes? What do you think?
Probably not worth it. If we get the BAR moved to a 64bit address we
should have enough address space in almost all cases, so setting it to
the maximum should succeed.

But I think we could add another parameter to allow limiting the resized
size for all corner cases and for testing.

Regards,
Christian.

Am 23.03.2017 um 15:30 schrieb Sagalovitch, Serguei:
> Christian,
>
> - Are we going to support resizing BAR when kernel
> modesetting is not enabled and we are running in console
> under VBIOS control (VESA/VGA)?
>
> - Should we restore PCI configuration if amdgpu
> will be unloaded?
>
> - In function amdgpu_resize_bar0():
> If resizing for "max" size failed should we try other
> sizes? What do you think?
>
>
> Sincerely yours,
> Serguei Sagalovitch
>
>
> From: amd-gfx <[email protected]> on behalf of Zhang, Jerry <[email protected]>
> Sent: March 15, 2017 10:41 PM
> To: Alex Deucher
> Cc: Zhou, David(ChunMing); Ayyappa Ch; [email protected]; [email protected]; [email protected]; [email protected]; Christian K?nig; [email protected]; [email protected]
> Subject: RE: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
>
> Thanks for your info.
> I see.
>
> Regards,
> Jerry (Junwei Zhang)
>
> Linux Base Graphics
> SRDC Software Development
> _____________________________________
>
>
>> -----Original Message-----
>> From: Alex Deucher [mailto:[email protected]]
>> Sent: Thursday, March 16, 2017 10:25
>> To: Zhang, Jerry
>> Cc: Christian K?nig; Zhou, David(ChunMing); Ayyappa Ch; linux-
>> [email protected]; [email protected]; dri-
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
>>
>> On Wed, Mar 15, 2017 at 10:19 PM, Zhang, Jerry <[email protected]> wrote:
>>>> -----Original Message-----
>>>> From: dri-devel [mailto:[email protected]] On
>>>> Behalf Of Christian K?nig
>>>> Sent: Wednesday, March 15, 2017 17:29
>>>> To: Zhou, David(ChunMing); Ayyappa Ch
>>>> Cc: [email protected]; [email protected]; amd-
>>>> [email protected]; [email protected];
>>>> [email protected]; [email protected]
>>>> Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
>>>>
>>>> Yes, exactly that.
>>> (I'm not familiar with PCI too much.)
>>> Is there any restrict for PCI device?
>>> I'm concerning if any PCI couldn't support it on some motherboard.
>> It depends on the PCI root bridge. This patch set only implements support for
>> AMD root bridges. Intel and other vendors would need similar code.
>>
>> Alex
>>
>>>> Christian.
>>>>
>>>> Am 15.03.2017 um 09:25 schrieb Zhou, David(ChunMing):
>>>>> Does that means we don't need invisible vram later?
>>>>>
>>>>> David
>>>>>
>>>>> -----Original Message-----
>>>>> From: dri-devel [mailto:[email protected]] On
>>>>> Behalf Of Christian K?nig
>>>>> Sent: Wednesday, March 15, 2017 3:38 PM
>>>>> To: Ayyappa Ch <[email protected]>
>>>>> Cc: [email protected]; [email protected];
>>>>> [email protected]; [email protected];
>>>>> [email protected]; [email protected]
>>>>> Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access
>>>>>
>>>>> Carizzo is an APU and resizing BARs isn't needed nor supported there.
>>>>> The CPU can access the full stolen VRAM directly on that hardware.
>>>>>
>>>>> As far as I know ASICs with support for this are Tonga, Fiji and all Polaris
>> variants.
>>>>> Christian.
>>>>>
>>>>> Am 15.03.2017 um 08:23 schrieb Ayyappa Ch:
>>>>>> Is it possible on Carrizo asics? Or only supports on newer asics?
>>>>>>
>>>>>> On Mon, Mar 13, 2017 at 6:11 PM, Christian K?nig
>>>>>> <[email protected]> wrote:
>>>>>>> From: Christian K?nig <[email protected]>
>>>>>>>
>>>>>>> Try to resize BAR0 to let CPU access all of VRAM.
>>>>>>>
>>>>>>> Signed-off-by: Christian K?nig <[email protected]>
>>>>>>> ---
>>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
>>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29
>>>> +++++++++++++++++++++++++++++
>>>>>>> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 +++++---
>>>>>>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 +++++---
>>>>>>> 4 files changed, 40 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>>>> index 3b81ded..905ded9 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>>>> @@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct
>>>> amdgpu_device *adev, struct ttm_tt *ttm,
>>>>>>> struct ttm_mem_reg *mem);
>>>>>>> void amdgpu_vram_location(struct amdgpu_device *adev, struct
>>>> amdgpu_mc *mc, u64 base);
>>>>>>> void amdgpu_gtt_location(struct amdgpu_device *adev, struct
>>>>>>> amdgpu_mc *mc);
>>>>>>> +void amdgpu_resize_bar0(struct amdgpu_device *adev);
>>>>>>> void amdgpu_ttm_set_active_vram_size(struct amdgpu_device
>>>>>>> *adev,
>>>> u64 size);
>>>>>>> int amdgpu_ttm_init(struct amdgpu_device *adev);
>>>>>>> void amdgpu_ttm_fini(struct amdgpu_device *adev); diff --git
>>>>>>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>>> index 118f4e6..92955fe 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>>> @@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct
>>>>>>> amdgpu_device
>>>> *adev, struct amdgpu_mc *mc)
>>>>>>> mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
>>>>>>> }
>>>>>>>
>>>>>>> +/**
>>>>>>> + * amdgpu_resize_bar0 - try to resize BAR0
>>>>>>> + *
>>>>>>> + * @adev: amdgpu_device pointer
>>>>>>> + *
>>>>>>> + * Try to resize BAR0 to make all VRAM CPU accessible.
>>>>>>> + */
>>>>>>> +void amdgpu_resize_bar0(struct amdgpu_device *adev) {
>>>>>>> + u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
>>>>>>> + int r;
>>>>>>> +
>>>>>>> + r = pci_resize_resource(adev->pdev, 0, size);
>>>>>>> +
>>>>>>> + if (r == -ENOTSUPP) {
>>>>>>> + /* The hardware don't support the extension. */
>>>>>>> + return;
>>>>>>> +
>>>>>>> + } else if (r == -ENOSPC) {
>>>>>>> + DRM_INFO("Not enoigh PCI address space for a large BAR.");
>>>>>>> + } else if (r) {
>>>>>>> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
>>>>>>> + }
>>>>>>> +
>>>>>>> + /* Reinit the doorbell mapping, it is most likely moved as well */
>>>>>>> + amdgpu_doorbell_fini(adev);
>>>>>>> + BUG_ON(amdgpu_doorbell_init(adev));
>>>>>>> +}
>>>>>>> +
>>>>>>> /*
>>>>>>> * GPU helpers function.
>>>>>>> */
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>>>>> index dc9b6d6..36a7aa5 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>>>>> @@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct
>>>>>>> amdgpu_device
>>>> *adev)
>>>>>>> break;
>>>>>>> }
>>>>>>> adev->mc.vram_width = numchan * chansize;
>>>>>>> - /* Could aper size report 0 ? */
>>>>>>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>>>>>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>>>>>> /* size in MB on si */
>>>>>>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) *
>>>>>>> 1024ULL *
>>>> 1024ULL;
>>>>>>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) *
>>>>>>> 1024ULL
>>>>>>> * 1024ULL;
>>>>>>>
>>>>>>> + if (!(adev->flags & AMD_IS_APU))
>>>>>>> + amdgpu_resize_bar0(adev);
>>>>>>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>>>>>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>>>>>> +
>>>>>>> #ifdef CONFIG_X86_64
>>>>>>> if (adev->flags & AMD_IS_APU) {
>>>>>>> adev->mc.aper_base =
>>>>>>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22; diff --git
>>>>>>> a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>>>>> index c087b00..7761ad3 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>>>>> @@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct
>>>>>>> amdgpu_device
>>>> *adev)
>>>>>>> break;
>>>>>>> }
>>>>>>> adev->mc.vram_width = numchan * chansize;
>>>>>>> - /* Could aper size report 0 ? */
>>>>>>> - adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>>>>>> - adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>>>>>> /* size in MB on si */
>>>>>>> adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) *
>>>>>>> 1024ULL *
>>>> 1024ULL;
>>>>>>> adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) *
>>>>>>> 1024ULL
>>>>>>> * 1024ULL;
>>>>>>>
>>>>>>> + if (!(adev->flags & AMD_IS_APU))
>>>>>>> + amdgpu_resize_bar0(adev);
>>>>>>> + adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>>>>>> + adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>>>>>>> +
>>>>>>> #ifdef CONFIG_X86_64
>>>>>>> if (adev->flags & AMD_IS_APU) {
>>>>>>> adev->mc.aper_base =
>>>>>>> ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
>>>>>>> --
>>>>>>> 2.7.4
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> dri-devel mailing list
>>>>>>> [email protected]
>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> [email protected]
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> [email protected]
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> [email protected]
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>> _______________________________________________
>>> amd-gfx mailing list
>>> [email protected]
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>


2017-03-24 15:29:42

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 1/4] PCI: add resizeable BAR infrastructure v3

On Mon, Mar 13, 2017 at 01:41:33PM +0100, Christian K?nig wrote:
> From: Christian K?nig <[email protected]>
>
> Just the defines and helper functions to read the possible sizes of a BAR and
> update it's size.

s/it's/its/

> See https://pcisig.com/sites/default/files/specification_documents/ECN_Resizable-BAR_24Apr2008.pdf.

It's good to have the public ECN that anybody can read, but we should
also have a reference to the full spec that incorporates it, e.g.,
PCIe r3.1, sec 7.22.

> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1946,6 +1946,9 @@ void pci_request_acs(void);
> bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags);
> bool pci_acs_path_enabled(struct pci_dev *start,
> struct pci_dev *end, u16 acs_flags);
> +u32 pci_rbar_get_possible_sizes(struct pci_dev *pdev, int bar);
> +int pci_rbar_get_current_size(struct pci_dev *pdev, int bar);
> +int pci_rbar_set_size(struct pci_dev *pdev, int bar, int size);

These should be declared in drivers/pci/pci.h unless they're needed
outside drivers/pci. I hope they aren't needed outside, because
they're not safe to use after the PCI core has claimed resources.

> #define PCI_VPD_LRDT 0x80 /* Large Resource Data Type */
> #define PCI_VPD_LRDT_ID(x) ((x) | PCI_VPD_LRDT)
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index e5a2e68..6de29d6 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -932,9 +932,16 @@
> #define PCI_SATA_SIZEOF_LONG 16
>
> /* Resizable BARs */
> +#define PCI_REBAR_CAP 4 /* capability register */
> +#define PCI_REBAR_CTRL_SIZES_MASK (0xFFFFF << 4) /* mask for sizes */
> +#define PCI_REBAR_CTRL_SIZES_SHIFT 4 /* shift for sizes */
> #define PCI_REBAR_CTRL 8 /* control register */
> +#define PCI_REBAR_CTRL_BAR_IDX_MASK (7 << 0) /* mask for bar index */
> +#define PCI_REBAR_CTRL_BAR_IDX_SHIFT 0 /* shift for bar index */
> #define PCI_REBAR_CTRL_NBAR_MASK (7 << 5) /* mask for # bars */
> #define PCI_REBAR_CTRL_NBAR_SHIFT 5 /* shift for # bars */
> +#define PCI_REBAR_CTRL_BAR_SIZE_MASK (0x1F << 8) /* mask for bar size */
> +#define PCI_REBAR_CTRL_BAR_SIZE_SHIFT 8 /* shift for bar size */

s/bar/BAR/ several places above

> /* Dynamic Power Allocation */
> #define PCI_DPA_CAP 4 /* capability register */
> --
> 2.7.4
>

2017-03-24 15:48:15

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 3/4] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors

On Mon, Mar 13, 2017 at 01:41:35PM +0100, Christian K?nig wrote:
> From: Christian K?nig <[email protected]>
>
> Most BIOS don't enable this because of compatibility reasons.

Can you give any more details here? Without more hints, it's hard to
know whether any of the compatibility reasons might apply to Linux as
well.

> Manually enable a 64bit BAR of 64GB size so that we have
> enough room for PCI devices.

>From the context, I'm guessing this is enabling a new 64GB window
through the PCI host bridge? That might be documented as a "BAR", but
it's not anything the Linux PCI core would recognize as a BAR.

I think the specs would envision this being done via an ACPI _SRS
method on the PNP0A03 host bridge device. That would be a more
generic path that would work on any host bridge. Did you explore that
possibility? I would prefer to avoid adding device-specific code if
that's possible.

> Signed-off-by: Christian K?nig <[email protected]>
> ---
> arch/x86/pci/fixup.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index 6d52b94..bff5242 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -571,3 +571,56 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2fc0, pci_invalid_bar);
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_invalid_bar);
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_invalid_bar);
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_invalid_bar);
> +
> +static void pci_amd_enable_64bit_bar(struct pci_dev *dev)
> +{
> + const uint64_t size = 64ULL * 1024 * 1024 * 1024;
> + uint32_t base, limit, high;
> + struct resource *res;
> + unsigned i;
> + int r;
> +
> + for (i = 0; i < 8; ++i) {
> +
> + pci_read_config_dword(dev, 0x80 + i * 0x8, &base);
> + pci_read_config_dword(dev, 0x180 + i * 0x4, &high);
> +
> + /* Is this slot free? */
> + if ((base & 0x3) == 0x0)
> + break;
> +
> + base >>= 8;
> + base |= high << 24;
> +
> + /* Abort if a slot already configures a 64bit BAR. */
> + if (base > 0x10000)
> + return;
> +
> + }
> +
> + if (i == 8)
> + return;
> +
> + res = kzalloc(sizeof(*res), GFP_KERNEL);
> + res->flags = IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_MEM_64 |
> + IORESOURCE_WINDOW;
> + res->name = dev->bus->name;
> + r = allocate_resource(&iomem_resource, res, size, 0x100000000,
> + 0xfd00000000, size, NULL, NULL);
> + if (r) {
> + kfree(res);
> + return;
> + }
> +
> + base = ((res->start >> 8) & 0xffffff00) | 0x3;
> + limit = ((res->end + 1) >> 8) & 0xffffff00;
> + high = ((res->start >> 40) & 0xff) |
> + ((((res->end + 1) >> 40) & 0xff) << 16);
> +
> + pci_write_config_dword(dev, 0x180 + i * 0x4, high);
> + pci_write_config_dword(dev, 0x84 + i * 0x8, limit);
> + pci_write_config_dword(dev, 0x80 + i * 0x8, base);
> +
> + pci_bus_add_resource(dev->bus, res, 0);

We would need some sort of printk here to explain how this new window
magically appeared.

> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_AMD, 0x141b, pci_amd_enable_64bit_bar);
> --
> 2.7.4
>

2017-03-24 21:35:00

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 2/4] PCI: add functionality for resizing resources v2

On Mon, Mar 13, 2017 at 01:41:34PM +0100, Christian K?nig wrote:
> From: Christian K?nig <[email protected]>
>
> This allows device drivers to request resizing their BARs.
>
> The function only tries to reprogram the windows of the bridge directly above
> the requesting device and only the BAR of the same type (usually mem, 64bit,
> prefetchable). This is done to make sure not to disturb other drivers by
> changing the BARs of their devices.
>
> If reprogramming the bridge BAR fails the old status is restored and -ENOSPC
> returned to the calling device driver.
>
> v2: rebase on changes in rbar support
>
> Signed-off-by: Christian K?nig <[email protected]>
> ---
> drivers/pci/setup-bus.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/pci/setup-res.c | 51 +++++++++++++++++++++++++++++++++++++++++
> include/linux/pci.h | 2 ++
> 3 files changed, 114 insertions(+)
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index f30ca75..cfab2c7 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1923,6 +1923,67 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
> }
> EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);
>
> +int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
> +{
> + const unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
> + IORESOURCE_PREFETCH | IORESOURCE_MEM_64;
> +
> + struct resource saved;
> + LIST_HEAD(add_list);
> + LIST_HEAD(fail_head);
> + struct pci_dev_resource *fail_res;
> + unsigned i;
> + int ret = 0;
> +
> + /* Release all children from the matching bridge resource */
> + for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END; ++i) {

Nit: use post-increment unless you need pre-increment.

> + struct resource *res = &bridge->resource[i];
> +
> + if ((res->flags & type_mask) != (type & type_mask))
> + continue;
> +
> + saved = *res;
> + if (res->parent) {
> + release_child_resources(res);

Doesn't this recursively release *all* child resources? There could
be BARs from several devices, or even windows of downstream bridges,
inside this window. The drivers of those other devices aren't
expecting things to change here.

> + release_resource(res);
> + }
> + res->start = 0;
> + res->end = 0;
> + break;
> + }
> +
> + if (i == PCI_BRIDGE_RESOURCE_END)
> + return -ENOENT;
> +
> + __pci_bus_size_bridges(bridge->subordinate, &add_list);
> + __pci_bridge_assign_resources(bridge, &add_list, &fail_head);
> + BUG_ON(!list_empty(&add_list));
> +
> + /* restore size and flags */
> + list_for_each_entry(fail_res, &fail_head, list) {
> + struct resource *res = fail_res->res;
> +
> + res->start = fail_res->start;
> + res->end = fail_res->end;
> + res->flags = fail_res->flags;
> + }
> +
> + /* Revert to the old configuration */
> + if (!list_empty(&fail_head)) {
> + struct resource *res = &bridge->resource[i];
> +
> + res->start = saved.start;
> + res->end = saved.end;
> + res->flags = saved.flags;
> +
> + pci_claim_resource(bridge, i);
> + ret = -ENOSPC;
> + }
> +
> + free_list(&fail_head);
> + return ret;
> +}
> +
> void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
> {
> struct pci_dev *dev;
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 9526e34..3bb1e29 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -363,6 +363,57 @@ int pci_reassign_resource(struct pci_dev *dev, int resno, resource_size_t addsiz
> return 0;
> }
>
> +int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> +{
> + struct resource *res = dev->resource + resno;
> + u32 sizes = pci_rbar_get_possible_sizes(dev, resno);
> + int old = pci_rbar_get_current_size(dev, resno);
> + u64 bytes = 1ULL << (size + 20);
> + int ret = 0;

I think we should fail the request if the device is enabled, i.e., if
the PCI_COMMAND_MEMORY bit is set. We can't safely change the BAR
while memory decoding is enabled.

I know there's code in pci_std_update_resource() that turns off
PCI_COMMAND_MEMORY, but I think that's a mistake: I think it should
fail when asked to update an enabled BAR the same way
pci_iov_update_resource() does.

I'm not sure why you call pci_reenable_device() below, but I think I
would rather have the driver do something like this:

pci_disable_device(dev);
pci_resize_resource(dev, 0, size);
pci_enable_device(dev);

That way it's very clear to the driver that it must re-read all BARs
after resizing this one.

> + if (!sizes)
> + return -ENOTSUPP;
> +
> + if (!(sizes & (1 << size)))
> + return -EINVAL;
> +
> + if (old < 0)
> + return old;
> +
> + /* Make sure the resource isn't assigned before making it larger. */
> + if (resource_size(res) < bytes && res->parent) {
> + release_resource(res);
> + res->end = resource_size(res) - 1;
> + res->start = 0;
> + if (resno < PCI_BRIDGE_RESOURCES)
> + pci_update_resource(dev, resno);

Why do we need to write this zero to the BAR? If PCI_COMMAND_MEMORY
is set, I think this is dangerous, and if it's not set, I think it's
unnecessary.

I think we should set the IORESOURCE_UNSET bit to indicate that the
resource does not have an address assigned to it. It should get
cleared later anyway, but having IORESOURCE_UNSET will make any debug
messages emitted while reassigning resources make more sense.

> + }
> +
> + ret = pci_rbar_set_size(dev, resno, size);
> + if (ret)
> + goto error_reassign;
> +
> + res->end = res->start + bytes - 1;
> +
> + ret = pci_reassign_bridge_resources(dev->bus->self, res->flags);
> + if (ret)
> + goto error_resize;
> +
> + pci_reenable_device(dev->bus->self);

> + return 0;
> +
> +error_resize:
> + pci_rbar_set_size(dev, resno, old);
> + res->end = res->start + (1ULL << (old + 20)) - 1;
> +
> +error_reassign:
> + pci_assign_unassigned_bus_resources(dev->bus);
> + pci_setup_bridge(dev->bus);

Could this bridge-related recovery code go inside
pci_reassign_bridge_resources()?

> + pci_reenable_device(dev->bus->self);
> + return ret;
> +}
> +EXPORT_SYMBOL(pci_resize_resource);
> +
> int pci_enable_resources(struct pci_dev *dev, int mask)
> {
> u16 cmd, old_cmd;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 6954e50..8eaebb4 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1055,6 +1055,7 @@ void pci_reset_bridge_secondary_bus(struct pci_dev *dev);
> void pci_update_resource(struct pci_dev *dev, int resno);
> int __must_check pci_assign_resource(struct pci_dev *dev, int i);
> int __must_check pci_reassign_resource(struct pci_dev *dev, int i, resource_size_t add_size, resource_size_t align);
> +int __must_check pci_resize_resource(struct pci_dev *dev, int i, int size);
> int pci_select_bars(struct pci_dev *dev, unsigned long flags);
> bool pci_device_is_present(struct pci_dev *pdev);
> void pci_ignore_hotplug(struct pci_dev *dev);
> @@ -1135,6 +1136,7 @@ void pci_assign_unassigned_resources(void);
> void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
> void pci_assign_unassigned_bus_resources(struct pci_bus *bus);
> void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus);
> +int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type);
> void pdev_enable_device(struct pci_dev *);
> int pci_enable_resources(struct pci_dev *, int mask);
> void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),
> --
> 2.7.4
>

2017-03-24 21:42:30

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access

On Mon, Mar 13, 2017 at 01:41:36PM +0100, Christian K?nig wrote:
> From: Christian K?nig <[email protected]>
>
> Try to resize BAR0 to let CPU access all of VRAM.
>
> Signed-off-by: Christian K?nig <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 29 +++++++++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 +++++---
> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 +++++---
> 4 files changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 3b81ded..905ded9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1719,6 +1719,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_tt *ttm,
> struct ttm_mem_reg *mem);
> void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base);
> void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc);
> +void amdgpu_resize_bar0(struct amdgpu_device *adev);
> void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size);
> int amdgpu_ttm_init(struct amdgpu_device *adev);
> void amdgpu_ttm_fini(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 118f4e6..92955fe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -692,6 +692,35 @@ void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc)
> mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
> }
>
> +/**
> + * amdgpu_resize_bar0 - try to resize BAR0
> + *
> + * @adev: amdgpu_device pointer
> + *
> + * Try to resize BAR0 to make all VRAM CPU accessible.
> + */
> +void amdgpu_resize_bar0(struct amdgpu_device *adev)
> +{
> + u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
> + int r;
> +
> + r = pci_resize_resource(adev->pdev, 0, size);
> +
> + if (r == -ENOTSUPP) {
> + /* The hardware don't support the extension. */
> + return;
> +
> + } else if (r == -ENOSPC) {
> + DRM_INFO("Not enoigh PCI address space for a large BAR.");

s/enoigh/enough/

> + } else if (r) {
> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
> + }
> +
> + /* Reinit the doorbell mapping, it is most likely moved as well */

I think you should assume all BARs moved (I don't know how many you have;
maybe this already covers all of them).

> + amdgpu_doorbell_fini(adev);
> + BUG_ON(amdgpu_doorbell_init(adev));

I think things inside BUG_ON() tend to get overlooked, so I avoid things
that have side-effects. But that's just my personal preference.

2017-04-11 09:17:41

by Christian König

[permalink] [raw]
Subject: Re: [PATCH 2/4] PCI: add functionality for resizing resources v2

Sorry for the delayed response, have been busy with other stuff recently.

Am 13.03.2017 um 17:43 schrieb Andy Shevchenko:
>> v2: rebase on changes in rbar support
> This kind of comments usually goes after --- delimiter below.

That would remove it from the commit message which I don't want.


>> + unsigned i;
>> + int ret = 0;
>> +
>> + /* Release all children from the matching bridge resource */
>> + for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END; ++i) {
>> + struct resource *res = &bridge->resource[i];
>> +
>
>> + if ((res->flags & type_mask) != (type & type_mask))
> IIUC it would be
> if ((res->flags ^ type) & type_mask)
>
> (I consider 'diff' as XOR operation is more understandable, but it's up to you)

I think like it is is easier to read.


>> + res->start = saved.start;
>> + res->end = saved.end;
>> + res->flags = saved.flags;
> Would
> *res = saved;
> work?

No, res also contains a bunch of pointers into the tree which we should
not override.

>> +int pci_resize_resource(struct pci_dev *dev, int resno, int size)
>> +{
>> + struct resource *res = dev->resource + resno;
>> + u32 sizes = pci_rbar_get_possible_sizes(dev, resno);
>> + int old = pci_rbar_get_current_size(dev, resno);
>> + u64 bytes = 1ULL << (size + 20);
>> + int ret = 0;
>> +
> I would put
> sizes = pci_rbar_get_possible_sizes(dev, resno);
> here

Good idea, done.


>> + if (!sizes)
>> + return -ENOTSUPP;
>> +
>> + if (!(sizes & (1 << size)))
> BIT(size) ?

Done.

> and
> old = pci_rbar_get_current_size(dev, resno);
> here

Good idea as well.

>> +error_resize:
>> + pci_rbar_set_size(dev, resno, old);
>> + res->end = res->start + (1ULL << (old + 20)) - 1;
> BIT_ULL(old + 20) ?

Nope, that is actually a size in bytes. Not a bitfield. So while BIT_ULL
yields the right result it would be harder to understand.

Christian.

2017-04-11 09:21:32

by Christian König

[permalink] [raw]
Subject: Re: [PATCH 3/4] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors

Am 13.03.2017 um 17:49 schrieb Andy Shevchenko:
> On Mon, Mar 13, 2017 at 2:41 PM, Christian König
> <[email protected]> wrote:
>
>> Most BIOS don't enable this because of compatibility reasons.
>>
>> Manually enable a 64bit BAR of 64GB size so that we have
>> enough room for PCI devices.
>> +static void pci_amd_enable_64bit_bar(struct pci_dev *dev)
>> +{
>> + const uint64_t size = 64ULL * 1024 * 1024 * 1024;
> Perhaps extend <linux/sizes.h> and use SZ_64G here?
>
> It would be nice to do, since some of the drivers already are using
> sizes like 4GB and alike.

Actually using 64GB here was just for testing and to get some initial
feedback.

I think we want to use all the remaining address space for PCIe, but for
this we would need a new function in the resource management I think.

Going to take a deeper look when I'm sure we actually want this.

>> + if (i == 8)
>> + return;
>> +
>> + res = kzalloc(sizeof(*res), GFP_KERNEL);
>> + res->flags = IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_MEM_64 |
>> + IORESOURCE_WINDOW;
>> + res->name = dev->bus->name;
>> + r = allocate_resource(&iomem_resource, res, size, 0x100000000,
>> + 0xfd00000000, size, NULL, NULL);
>> + if (r) {
>> + kfree(res);
>> + return;
>> + }
>> +
>> + base = ((res->start >> 8) & 0xffffff00) | 0x3;
>> + limit = ((res->end + 1) >> 8) & 0xffffff00;
>> + high = ((res->start >> 40) & 0xff) |
>> + ((((res->end + 1) >> 40) & 0xff) << 16);
> Perhaps some of constants can be replaced by defines (I think some of
> them are already defined in ioport.h or somewhere else).

Yeah, good idea. But that stuff is purely AMD CPU specific, so won't
belong into ioport.h or similar common code.

Does anybody have any idea where I could put this?

Regards,

Christian.

2017-04-11 15:37:21

by Christian König

[permalink] [raw]
Subject: Re: [PATCH 2/4] PCI: add functionality for resizing resources v2

Hi Bjorn,

first of all sorry for the delay, had been busy with other stuff in the
last few weeks.

Am 24.03.2017 um 22:34 schrieb Bjorn Helgaas:
>> + release_child_resources(res);
> Doesn't this recursively release *all* child resources? There could
> be BARs from several devices, or even windows of downstream bridges,
> inside this window. The drivers of those other devices aren't
> expecting things to change here.

Yes, the original idea was that the driver calling this knows that the
BARs will be changed for the bridge it belongs to.

But you're right. I've changed it in the way that our device driver will
release all resource first and then call the function to resize its BAR.

The function will return an error in the next version of the patch if
the bridge BAR which needs to be moved for this is still used by child
resources.

>> +int pci_resize_resource(struct pci_dev *dev, int resno, int size)
>> +{
>> + struct resource *res = dev->resource + resno;
>> + u32 sizes = pci_rbar_get_possible_sizes(dev, resno);
>> + int old = pci_rbar_get_current_size(dev, resno);
>> + u64 bytes = 1ULL << (size + 20);
>> + int ret = 0;
> I think we should fail the request if the device is enabled, i.e., if
> the PCI_COMMAND_MEMORY bit is set. We can't safely change the BAR
> while memory decoding is enabled.
>
> I know there's code in pci_std_update_resource() that turns off
> PCI_COMMAND_MEMORY, but I think that's a mistake: I think it should
> fail when asked to update an enabled BAR the same way
> pci_iov_update_resource() does.
>
> I'm not sure why you call pci_reenable_device() below, but I think I
> would rather have the driver do something like this:
>
> pci_disable_device(dev);
> pci_resize_resource(dev, 0, size);
> pci_enable_device(dev);
>
> That way it's very clear to the driver that it must re-read all BARs
> after resizing this one.

I've tried it, but this actually doesn't seem to work.

At least on the board I've tried pci_disable_device() doesn't clear the
PCI_COMMAND_MEMORY bit, it just clears the master bit.

Additional to that the power management reference counting
pci_disable_device() and pci_enable_device() doesn't look like what I
want for this functionality.

How about the driver needs to disabled memory decoding itself before
trying to resize anything?

>> + if (!sizes)
>> + return -ENOTSUPP;
>> +
>> + if (!(sizes & (1 << size)))
>> + return -EINVAL;
>> +
>> + if (old < 0)
>> + return old;
>> +
>> + /* Make sure the resource isn't assigned before making it larger. */
>> + if (resource_size(res) < bytes && res->parent) {
>> + release_resource(res);
>> + res->end = resource_size(res) - 1;
>> + res->start = 0;
>> + if (resno < PCI_BRIDGE_RESOURCES)
>> + pci_update_resource(dev, resno);
> Why do we need to write this zero to the BAR? If PCI_COMMAND_MEMORY
> is set, I think this is dangerous, and if it's not set, I think it's
> unnecessary.
>
> I think we should set the IORESOURCE_UNSET bit to indicate that the
> resource does not have an address assigned to it. It should get
> cleared later anyway, but having IORESOURCE_UNSET will make any debug
> messages emitted while reassigning resources make more sense.

Makes sense, changed.

>> + return 0;
>> +
>> +error_resize:
>> + pci_rbar_set_size(dev, resno, old);
>> + res->end = res->start + (1ULL << (old + 20)) - 1;
>> +
>> +error_reassign:
>> + pci_assign_unassigned_bus_resources(dev->bus);
>> + pci_setup_bridge(dev->bus);
> Could this bridge-related recovery code go inside
> pci_reassign_bridge_resources()?

No, we need to restore the original size of the resource before trying
the recovery code when something goes wrong.

I will address all other comments in the next version of the patch as well.

Regards,
Christian.

2017-04-11 15:48:47

by Christian König

[permalink] [raw]
Subject: Re: [PATCH 3/4] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors

Am 24.03.2017 um 16:47 schrieb Bjorn Helgaas:
> On Mon, Mar 13, 2017 at 01:41:35PM +0100, Christian K?nig wrote:
>> From: Christian K?nig <[email protected]>
>>
>> Most BIOS don't enable this because of compatibility reasons.
> Can you give any more details here? Without more hints, it's hard to
> know whether any of the compatibility reasons might apply to Linux as
> well.

Unfortunately not, I could try to ask a few more people at AMD if they
know the background.

I was told that there are a few boards which offers that as a BIOS
option, but so far I haven't found any (and I have quite a few here).

My best guess is that older windows versions have a problem with that.

>> Manually enable a 64bit BAR of 64GB size so that we have
>> enough room for PCI devices.
> From the context, I'm guessing this is enabling a new 64GB window
> through the PCI host bridge?

Yes, exactly. Sorry for the confusion.

> That might be documented as a "BAR", but
> it's not anything the Linux PCI core would recognize as a BAR.

At least the AMD NB documentation calls this the host BARs. But I'm
perfectly fine with any terminology.

How about calling it host bridge window instead?

> I think the specs would envision this being done via an ACPI _SRS
> method on the PNP0A03 host bridge device. That would be a more
> generic path that would work on any host bridge. Did you explore that
> possibility? I would prefer to avoid adding device-specific code if
> that's possible.

I've checked quite a few boards, but none of them actually implements it
this way.

M$ is working on a new ACPI table to enable this vendor neutral, but I
guess that will still take a while.

I want to support this for all AMD CPU released in the past 5 years or
so, so we are going to deal with a bunch of older boards as well.


>> + pci_bus_add_resource(dev->bus, res, 0);
> We would need some sort of printk here to explain how this new window
> magically appeared.

Good point, consider this done.

But is this actually the right place of doing it? Or would you prefer
something to be added to the probing code?

I think those fixups are applied a bit later, aren't they?

Best regards,
Christian.

2017-04-12 16:38:02

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 2/4] PCI: add functionality for resizing resources v2

On Tue, Apr 11, 2017 at 05:37:04PM +0200, Christian K?nig wrote:

> >>+int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> >>+{
> >>+ struct resource *res = dev->resource + resno;
> >>+ u32 sizes = pci_rbar_get_possible_sizes(dev, resno);
> >>+ int old = pci_rbar_get_current_size(dev, resno);
> >>+ u64 bytes = 1ULL << (size + 20);
> >>+ int ret = 0;
> >I think we should fail the request if the device is enabled, i.e., if
> >the PCI_COMMAND_MEMORY bit is set. We can't safely change the BAR
> >while memory decoding is enabled.
> >
> >I know there's code in pci_std_update_resource() that turns off
> >PCI_COMMAND_MEMORY, but I think that's a mistake: I think it should
> >fail when asked to update an enabled BAR the same way
> >pci_iov_update_resource() does.
> >
> >I'm not sure why you call pci_reenable_device() below, but I think I
> >would rather have the driver do something like this:
> >
> > pci_disable_device(dev);
> > pci_resize_resource(dev, 0, size);
> > pci_enable_device(dev);
> >
> >That way it's very clear to the driver that it must re-read all BARs
> >after resizing this one.
>
> I've tried it, but this actually doesn't seem to work.
>
> At least on the board I've tried pci_disable_device() doesn't clear
> the PCI_COMMAND_MEMORY bit, it just clears the master bit.

Yeah, you're right. We generally turn *on* PCI_COMMAND_MEMORY in the
pci_enable_device() path, but the pci_disable_device() path doesn't
turn it off.

> Additional to that the power management reference counting
> pci_disable_device() and pci_enable_device() doesn't look like what
> I want for this functionality.
>
> How about the driver needs to disabled memory decoding itself before
> trying to resize anything?

There are a few places that do that, so maybe that's the best option.

Bjorn

2017-04-12 16:55:37

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 3/4] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors

On Tue, Apr 11, 2017 at 05:48:25PM +0200, Christian K?nig wrote:
> Am 24.03.2017 um 16:47 schrieb Bjorn Helgaas:
> >On Mon, Mar 13, 2017 at 01:41:35PM +0100, Christian K?nig wrote:
> >>From: Christian K?nig <[email protected]>
> >>
> >>Most BIOS don't enable this because of compatibility reasons.
> >Can you give any more details here? Without more hints, it's hard to
> >know whether any of the compatibility reasons might apply to Linux as
> >well.
>
> Unfortunately not, I could try to ask a few more people at AMD if
> they know the background.
>
> I was told that there are a few boards which offers that as a BIOS
> option, but so far I haven't found any (and I have quite a few
> here).
>
> My best guess is that older windows versions have a problem with that.
>
> >>Manually enable a 64bit BAR of 64GB size so that we have
> >>enough room for PCI devices.
> > From the context, I'm guessing this is enabling a new 64GB window
> >through the PCI host bridge?
>
> Yes, exactly. Sorry for the confusion.
>
> >That might be documented as a "BAR", but
> >it's not anything the Linux PCI core would recognize as a BAR.
>
> At least the AMD NB documentation calls this the host BARs. But I'm
> perfectly fine with any terminology.
>
> How about calling it host bridge window instead?

That works for me.

> >I think the specs would envision this being done via an ACPI _SRS
> >method on the PNP0A03 host bridge device. That would be a more
> >generic path that would work on any host bridge. Did you explore that
> >possibility? I would prefer to avoid adding device-specific code if
> >that's possible.
>
> I've checked quite a few boards, but none of them actually
> implements it this way.
>
> M$ is working on a new ACPI table to enable this vendor neutral, but
> I guess that will still take a while.
>
> I want to support this for all AMD CPU released in the past 5 years
> or so, so we are going to deal with a bunch of older boards as well.

I've never seen _SRS for host bridges either. I'm curious about what
sort of new table will be proposed. It seems like the existing ACPI
resource framework could manage it, but I certainly don't know all the
issues.

> >>+ pci_bus_add_resource(dev->bus, res, 0);
> >We would need some sort of printk here to explain how this new window
> >magically appeared.
>
> Good point, consider this done.
>
> But is this actually the right place of doing it? Or would you
> prefer something to be added to the probing code?
>
> I think those fixups are applied a bit later, aren't they?

Logically, this should be done before we enumerate the PCI devices
below the host bridge, so a PCI device fixup is not the ideal place
for it, but it might be the most practical.

I could imagine some sort of quirk like the ones in
drivers/pnp/quirks.c that could add the window to the host bridge _CRS
and program the bridge to open it. But the PCI host bridges aren't
handled through the path that applies those fixups, and it would be
messy to identify your bridges (you currently use PCI vendor/device
IDs, which are only available after enumerating the device). So this
doesn't seem like a viable strategy.

Bjorn

2017-04-25 13:01:51

by Christian König

[permalink] [raw]
Subject: Re: [PATCH 3/4] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors

Am 12.04.2017 um 18:55 schrieb Bjorn Helgaas:
> [SNIP]
>>> I think the specs would envision this being done via an ACPI _SRS
>>> method on the PNP0A03 host bridge device. That would be a more
>>> generic path that would work on any host bridge. Did you explore that
>>> possibility? I would prefer to avoid adding device-specific code if
>>> that's possible.
>> I've checked quite a few boards, but none of them actually
>> implements it this way.
>>
>> M$ is working on a new ACPI table to enable this vendor neutral, but
>> I guess that will still take a while.
>>
>> I want to support this for all AMD CPU released in the past 5 years
>> or so, so we are going to deal with a bunch of older boards as well.
> I've never seen _SRS for host bridges either. I'm curious about what
> sort of new table will be proposed. It seems like the existing ACPI
> resource framework could manage it, but I certainly don't know all the
> issues.

No idea either since I'm not involved into that. My job is to get it
working on the existing hw generations and that alone is enough work :)

My best guess is that MS is going to either make _SRS on the host bridge
or a pre-configured 64bit window mandatory for the BIOS.

>>>> + pci_bus_add_resource(dev->bus, res, 0);
>>> We would need some sort of printk here to explain how this new window
>>> magically appeared.
>> Good point, consider this done.
>>
>> But is this actually the right place of doing it? Or would you
>> prefer something to be added to the probing code?
>>
>> I think those fixups are applied a bit later, aren't they?
> Logically, this should be done before we enumerate the PCI devices
> below the host bridge, so a PCI device fixup is not the ideal place
> for it, but it might be the most practical.

Since the modification must be done on a device connected to the root
bus I run into quite a chicken and egg problem if I try to do it before
the enumeration.

> I could imagine some sort of quirk like the ones in
> drivers/pnp/quirks.c that could add the window to the host bridge _CRS
> and program the bridge to open it. But the PCI host bridges aren't
> handled through the path that applies those fixups, and it would be
> messy to identify your bridges (you currently use PCI vendor/device
> IDs, which are only available after enumerating the device). So this
> doesn't seem like a viable strategy.

I've tried that, but gave up rather quickly. Looks like the current
approach indeed work find even with "pci=realloc", so I'm going to stick
with that.

Regards,
Christian.