linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] PCI: add resizeable BAR infrastructure v2
@ 2017-03-06 11:40 Christian König
  2017-03-06 11:40 ` [PATCH 2/5] PCI: add functionality for resizing resources Christian König
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Christian König @ 2017-03-06 11:40 UTC (permalink / raw)
  To: linux-pci, dri-devel, platform-driver-x86, amd-gfx, linux-kernel

From: Christian König <christian.koenig@amd.com>

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.

v2: provide read helper as well

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 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..9658aa7 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_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_sizes(struct pci_dev *pdev, int bar)
+{
+	int pos, nbars;
+	u32 ctrl, cap;
+	int i;
+
+	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+	if (!pos)
+		return 0x0;
+
+	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 0x0;
+}
+
+/**
+ * pci_rbar_get_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_size(struct pci_dev *pdev, int bar)
+{
+	int pos, nbars;
+	u32 ctrl;
+	int 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.
+ *
+ * Set the new size of a BAR as defined in the spec (0=1MB, 19=512GB).
+ * Returns true if resizing was successful, false otherwise.
+ */
+bool pci_rbar_set_size(struct pci_dev *pdev, int bar, int size)
+{
+	int pos, nbars;
+	u32 ctrl;
+	int i;
+
+	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+	if (!pos)
+		return false;
+
+	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 true;
+	}
+
+	return false;
+}
+
+/**
  * 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..9f26ca4 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_sizes(struct pci_dev *pdev, int bar);
+int pci_rbar_get_size(struct pci_dev *pdev, int bar);
+bool 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/5] PCI: add functionality for resizing resources
  2017-03-06 11:40 [PATCH 1/5] PCI: add resizeable BAR infrastructure v2 Christian König
@ 2017-03-06 11:40 ` Christian König
  2017-03-06 11:40 ` [PATCH 3/5] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors Christian König
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2017-03-06 11:40 UTC (permalink / raw)
  To: linux-pci, dri-devel, platform-driver-x86, amd-gfx, linux-kernel

From: Christian König <christian.koenig@amd.com>

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.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/pci/setup-bus.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/setup-res.c | 45 ++++++++++++++++++++++++++++++++++++
 include/linux/pci.h     |  2 ++
 3 files changed, 108 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..d03e6f1 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -363,6 +363,51 @@ 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_sizes(dev, resno);
+	int old = pci_rbar_get_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);
+	}
+
+	if (pci_rbar_set_size(dev, resno, size))
+		res->end = res->start + bytes - 1;
+	else
+		return -EIO;
+
+	ret = pci_reassign_bridge_resources(dev->bus->self, res->flags);
+	if (ret) {
+		pci_rbar_set_size(dev, resno, old);
+		res->end = res->start + (1ULL << (old + 20)) - 1;
+
+		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 9f26ca4..c85d8d7 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/5] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors
  2017-03-06 11:40 [PATCH 1/5] PCI: add resizeable BAR infrastructure v2 Christian König
  2017-03-06 11:40 ` [PATCH 2/5] PCI: add functionality for resizing resources Christian König
@ 2017-03-06 11:40 ` Christian König
  2017-03-06 11:40 ` [PATCH 4/5] drm/amdgpu: fix printing the doorbell BAR info Christian König
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2017-03-06 11:40 UTC (permalink / raw)
  To: linux-pci, dri-devel, platform-driver-x86, amd-gfx, linux-kernel

From: Christian König <christian.koenig@amd.com>

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 <christian.koenig@amd.com>
---
 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/5] drm/amdgpu: fix printing the doorbell BAR info
  2017-03-06 11:40 [PATCH 1/5] PCI: add resizeable BAR infrastructure v2 Christian König
  2017-03-06 11:40 ` [PATCH 2/5] PCI: add functionality for resizing resources Christian König
  2017-03-06 11:40 ` [PATCH 3/5] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors Christian König
@ 2017-03-06 11:40 ` Christian König
  2017-03-06 12:00   ` Andy Shevchenko
  2017-03-06 11:40 ` [PATCH 5/5] drm/amdgpu: resize VRAM BAR for CPU access Christian König
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2017-03-06 11:40 UTC (permalink / raw)
  To: linux-pci, dri-devel, platform-driver-x86, amd-gfx, linux-kernel

From: Christian König <christian.koenig@amd.com>

The address is 64bit, not 32bit.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index bf31aaf..a470869 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -385,7 +385,7 @@ static int amdgpu_doorbell_init(struct amdgpu_device *adev)
 	if (adev->doorbell.ptr == NULL) {
 		return -ENOMEM;
 	}
-	DRM_INFO("doorbell mmio base: 0x%08X\n", (uint32_t)adev->doorbell.base);
+	DRM_INFO("doorbell mmio base: 0x%llX\n", (uint64_t)adev->doorbell.base);
 	DRM_INFO("doorbell mmio size: %u\n", (unsigned)adev->doorbell.size);
 
 	return 0;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/5] drm/amdgpu: resize VRAM BAR for CPU access
  2017-03-06 11:40 [PATCH 1/5] PCI: add resizeable BAR infrastructure v2 Christian König
                   ` (2 preceding siblings ...)
  2017-03-06 11:40 ` [PATCH 4/5] drm/amdgpu: fix printing the doorbell BAR info Christian König
@ 2017-03-06 11:40 ` Christian König
  2017-03-06 12:06   ` Andy Shevchenko
  2017-03-06 11:50 ` [PATCH 1/5] PCI: add resizeable BAR infrastructure v2 Christian König
  2017-03-06 12:20 ` Andy Shevchenko
  5 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2017-03-06 11:40 UTC (permalink / raw)
  To: linux-pci, dri-devel, platform-driver-x86, amd-gfx, linux-kernel

From: Christian König <christian.koenig@amd.com>

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

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 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 8a5f8cb..1e888d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1754,6 +1754,7 @@ uint32_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 a470869..f038195 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -616,6 +616,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 552bf6b..cd5828c 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 f2bd016..e277130 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/5] PCI: add resizeable BAR infrastructure v2
  2017-03-06 11:40 [PATCH 1/5] PCI: add resizeable BAR infrastructure v2 Christian König
                   ` (3 preceding siblings ...)
  2017-03-06 11:40 ` [PATCH 5/5] drm/amdgpu: resize VRAM BAR for CPU access Christian König
@ 2017-03-06 11:50 ` Christian König
  2017-03-06 12:20 ` Andy Shevchenko
  5 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2017-03-06 11:50 UTC (permalink / raw)
  To: linux-pci, dri-devel, platform-driver-x86, amd-gfx, linux-kernel

Sorry I've hit enter to soon.

This set of patches tries to implement support for resizeable BARs 
including an example of how the AMD GFX device driver can make use of it 
to gain full CPU access to the VRAM on the hardware.

Patch #1 is just the second version of the basic RBAR support I've send 
out more than a year ago.

Patch #2 adds functionality to resize a single resource of a device by 
only touching parts of the PCIe tree which we can be sure are save to 
modify.

Patch #3 adds a quirk for AMD Kaveri/Kabini APUs which adds another 64GB 
BAR on bootup to make sure we have enough address space assigned to the 
root hub.

Patch #4 & #5 then uses the new functionality to resize the BAR of 
recent AMD GPUs to allow the CPU full access to the memory behind it.

Please comment and review.

Thanks,
Christian.

Am 06.03.2017 um 12:40 schrieb Christian König:
> From: Christian König <christian.koenig@amd.com>
>
> 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.
>
> v2: provide read helper as well
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   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..9658aa7 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_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_sizes(struct pci_dev *pdev, int bar)
> +{
> +	int pos, nbars;
> +	u32 ctrl, cap;
> +	int i;
> +
> +	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
> +	if (!pos)
> +		return 0x0;
> +
> +	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 0x0;
> +}
> +
> +/**
> + * pci_rbar_get_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_size(struct pci_dev *pdev, int bar)
> +{
> +	int pos, nbars;
> +	u32 ctrl;
> +	int 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.
> + *
> + * Set the new size of a BAR as defined in the spec (0=1MB, 19=512GB).
> + * Returns true if resizing was successful, false otherwise.
> + */
> +bool pci_rbar_set_size(struct pci_dev *pdev, int bar, int size)
> +{
> +	int pos, nbars;
> +	u32 ctrl;
> +	int i;
> +
> +	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
> +	if (!pos)
> +		return false;
> +
> +	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 true;
> +	}
> +
> +	return false;
> +}
> +
> +/**
>    * 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..9f26ca4 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_sizes(struct pci_dev *pdev, int bar);
> +int pci_rbar_get_size(struct pci_dev *pdev, int bar);
> +bool 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 */

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] drm/amdgpu: fix printing the doorbell BAR info
  2017-03-06 11:40 ` [PATCH 4/5] drm/amdgpu: fix printing the doorbell BAR info Christian König
@ 2017-03-06 12:00   ` Andy Shevchenko
  2017-03-06 12:09     ` Christian König
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2017-03-06 12:00 UTC (permalink / raw)
  To: Christian König
  Cc: linux-pci, dri-devel, Platform Driver, amd-gfx, linux-kernel

On Mon, Mar 6, 2017 at 1:40 PM, Christian K=C3=B6nig <deathsimple@vodafone.=
de> wrote:
> From: Christian K=C3=B6nig <christian.koenig@amd.com>
>
> The address is 64bit, not 32bit.
>
> Signed-off-by: Christian K=C3=B6nig <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm=
/amd/amdgpu/amdgpu_device.c
> index bf31aaf..a470869 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -385,7 +385,7 @@ static int amdgpu_doorbell_init(struct amdgpu_device =
*adev)
>         if (adev->doorbell.ptr =3D=3D NULL) {
>                 return -ENOMEM;
>         }


> -       DRM_INFO("doorbell mmio base: 0x%08X\n", (uint32_t)adev->doorbell=
.base);
> +       DRM_INFO("doorbell mmio base: 0x%llX\n", (uint64_t)adev->doorbell=
.base);
>         DRM_INFO("doorbell mmio size: %u\n", (unsigned)adev->doorbell.siz=
e);

It seems I sent patch to remove those at all, but if you wish to leave
them, please convert to %pap and remove explicit casting.

--=20
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] drm/amdgpu: resize VRAM BAR for CPU access
  2017-03-06 11:40 ` [PATCH 5/5] drm/amdgpu: resize VRAM BAR for CPU access Christian König
@ 2017-03-06 12:06   ` Andy Shevchenko
  2017-03-06 12:34     ` Christian König
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2017-03-06 12:06 UTC (permalink / raw)
  To: Christian König
  Cc: linux-pci, dri-devel, Platform Driver, amd-gfx, linux-kernel

On Mon, Mar 6, 2017 at 1:40 PM, Christian K=C3=B6nig <deathsimple@vodafone.=
de> wrote:
> From: Christian K=C3=B6nig <christian.koenig@amd.com>
>
> Try to resize BAR0 to let CPU access all of VRAM.

> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -616,6 +616,35 @@ void amdgpu_gtt_location(struct amdgpu_device *adev,=
 struct amdgpu_mc *mc)

> +void amdgpu_resize_bar0(struct amdgpu_device *adev)
> +{
> +       u32 size =3D max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20=
;

Too complicated.

unsigned long =3D fls_long(real_vram_size | BIT(20));

And the result is not a size, right? It's a logarithm from size.

> +       int r;
> +
> +       r =3D pci_resize_resource(adev->pdev, 0, size);

> +

Redundant line.

> +       if (r =3D=3D -ENOTSUPP) {
> +               /* The hardware don't support the extension. */
> +               return;
> +
> +       } else if (r =3D=3D -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));

No way to recover?!

> +}
> +

--=20
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] drm/amdgpu: fix printing the doorbell BAR info
  2017-03-06 12:00   ` Andy Shevchenko
@ 2017-03-06 12:09     ` Christian König
  0 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2017-03-06 12:09 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-pci, dri-devel, Platform Driver, amd-gfx, linux-kernel

Am 06.03.2017 um 13:00 schrieb Andy Shevchenko:
> On Mon, Mar 6, 2017 at 1:40 PM, Christian König <deathsimple@vodafone.de> wrote:
>> From: Christian König <christian.koenig@amd.com>
>>
>> The address is 64bit, not 32bit.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index bf31aaf..a470869 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -385,7 +385,7 @@ static int amdgpu_doorbell_init(struct amdgpu_device *adev)
>>          if (adev->doorbell.ptr == NULL) {
>>                  return -ENOMEM;
>>          }
>
>> -       DRM_INFO("doorbell mmio base: 0x%08X\n", (uint32_t)adev->doorbell.base);
>> +       DRM_INFO("doorbell mmio base: 0x%llX\n", (uint64_t)adev->doorbell.base);
>>          DRM_INFO("doorbell mmio size: %u\n", (unsigned)adev->doorbell.size);
> It seems I sent patch to remove those at all, but if you wish to leave
> them, please convert to %pap and remove explicit casting.
Sorry, looked like both Alex and I missed your patch. But yes that the 
PCI subsystem prints that info anyway is a good argument.

Going to put my rb on your patch and push it into our internal repo, 
Alex should then pick it up for the next drm-next pull request.

Thanks,
Christian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/5] PCI: add resizeable BAR infrastructure v2
  2017-03-06 11:40 [PATCH 1/5] PCI: add resizeable BAR infrastructure v2 Christian König
                   ` (4 preceding siblings ...)
  2017-03-06 11:50 ` [PATCH 1/5] PCI: add resizeable BAR infrastructure v2 Christian König
@ 2017-03-06 12:20 ` Andy Shevchenko
  2017-03-13  9:43   ` Christian König
  5 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2017-03-06 12:20 UTC (permalink / raw)
  To: Christian König
  Cc: linux-pci, dri-devel, Platform Driver, amd-gfx, linux-kernel

On Mon, Mar 6, 2017 at 1:40 PM, Christian K=C3=B6nig <deathsimple@vodafone.=
de> wrote:
> From: Christian K=C3=B6nig <christian.koenig@amd.com>
>
> 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_Re=
sizable-BAR_24Apr2008.pdf.
>
> v2: provide read helper as well

Commit message left away the explanation at which point this API might
be useful and how it fits in managed resources model?

>  /**
> + * pci_rbar_get_sizes - get possible sizes for BAR

Why not simple pci_rbar_get_possible_sizes() ?

> +u32 pci_rbar_get_sizes(struct pci_dev *pdev, int bar)
> +{
> +       int pos, nbars;
> +       u32 ctrl, cap;
> +       int i;
> +
> +       pos =3D pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
> +       if (!pos)

> +               return 0x0;

return 0;

> +
> +       pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
> +       nbars =3D (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >> PCI_REBAR_CTRL_NBA=
R_SHIFT;
> +

> +       for (i =3D 0; i < nbars; ++i, pos +=3D 8) {

8 is defined somewhere in the spec? (Yes, I understand that is just 64
bits shift)

> +               int bar_idx;
> +
> +               pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
> +               bar_idx =3D (ctrl & PCI_REBAR_CTRL_BAR_IDX_MASK) >>
> +                               PCI_REBAR_CTRL_BAR_IDX_SHIFT;
> +               if (bar_idx !=3D 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 0x0;

return 0;

> +/**
> + * pci_rbar_get_size - get the current size of a BAR

pci_rbar_get_current_size() ?

> +/**
> + * 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.

 * @size: bitmasked value of new size (bit 0=3D1MB, ..., bit 19=3D512G)

?

It will briefly get a clue without reading either spec or long description.

> + *
> + * Set the new size of a BAR as defined in the spec (0=3D1MB, 19=3D512GB=
).
> + * Returns true if resizing was successful, false otherwise.
> + */

> +bool pci_rbar_set_size(struct pci_dev *pdev, int bar, int size)

I would return int and error code. It would be better in the future
and seems in alignment with above.

> +{
> +       int pos, nbars;
> +       u32 ctrl;
> +       int i;

All ints are unsigned?

--=20
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] drm/amdgpu: resize VRAM BAR for CPU access
  2017-03-06 12:06   ` Andy Shevchenko
@ 2017-03-06 12:34     ` Christian König
  0 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2017-03-06 12:34 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-pci, dri-devel, Platform Driver, amd-gfx, linux-kernel

Am 06.03.2017 um 13:06 schrieb Andy Shevchenko:
> On Mon, Mar 6, 2017 at 1:40 PM, Christian König <deathsimple@vodafone.de> wrote:
>> From: Christian König <christian.koenig@amd.com>
>>
>> Try to resize BAR0 to let CPU access all of VRAM.
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -616,6 +616,35 @@ void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc)
>> +void amdgpu_resize_bar0(struct amdgpu_device *adev)
>> +{
>> +       u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
> Too complicated.
>
> unsigned long = fls_long(real_vram_size | BIT(20));

That would round down, not up. We got boards with 6GB VRAM as well and 
then need a 8GB BAR.

And the vram size won't fit into a long on 32bit systems. What I really 
need is order_base_2 for 64bit values.

But wait a second, thinking more about it we could do 
"order_base_2((real_vram_size >> 20) | 1)".

> And the result is not a size, right? It's a logarithm from size.

Yeah, and subtracted by 20. Thought about a better wording as well, but 
couldn't come up with something.

"size" is just what the spec uses. How about rbar_size to note that it 
is size as the meaning in the RBAR specification?

>
>> +       int r;
>> +
>> +       r = pci_resize_resource(adev->pdev, 0, size);
>> +
> Redundant line.
>
>> +       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));
> No way to recover?!

Nope, I actually thought about calling panic() here instead.

If we hit this we have messed things so badly up that we can't access 
the hardware any more, so no way to tell it to shut down or something 
like this.

Well, I could completely rewrite the call chain to signal modprobe that 
loading the driver didn't worked at all. But that comes pretty near to 
calling BUG_ON() as well.

Thanks for the comments,
Christian.

>
>> +}
>> +

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/5] PCI: add resizeable BAR infrastructure v2
  2017-03-06 12:20 ` Andy Shevchenko
@ 2017-03-13  9:43   ` Christian König
  0 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2017-03-13  9:43 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-pci, dri-devel, Platform Driver, amd-gfx, linux-kernel

Am 06.03.2017 um 13:20 schrieb Andy Shevchenko:
> On Mon, Mar 6, 2017 at 1:40 PM, Christian König <deathsimple@vodafone.de> wrote:
>> +       for (i = 0; i < nbars; ++i, pos += 8) {
> 8 is defined somewhere in the spec? (Yes, I understand that is just 64
> bits shift)

Yes, it is defined in the spec, see "Figure 7-x1 Resizable BAR Capability".

It just doesn't have a name or something if that's what you are asking for.

I've addressed all other comments in code and will send out the next 
version of the patch set today.

Thanks for the comments,
Christian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-03-13  9:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-06 11:40 [PATCH 1/5] PCI: add resizeable BAR infrastructure v2 Christian König
2017-03-06 11:40 ` [PATCH 2/5] PCI: add functionality for resizing resources Christian König
2017-03-06 11:40 ` [PATCH 3/5] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors Christian König
2017-03-06 11:40 ` [PATCH 4/5] drm/amdgpu: fix printing the doorbell BAR info Christian König
2017-03-06 12:00   ` Andy Shevchenko
2017-03-06 12:09     ` Christian König
2017-03-06 11:40 ` [PATCH 5/5] drm/amdgpu: resize VRAM BAR for CPU access Christian König
2017-03-06 12:06   ` Andy Shevchenko
2017-03-06 12:34     ` Christian König
2017-03-06 11:50 ` [PATCH 1/5] PCI: add resizeable BAR infrastructure v2 Christian König
2017-03-06 12:20 ` Andy Shevchenko
2017-03-13  9:43   ` Christian König

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).