Resizeable PCI BAR support V4

Resizeable PCI BAR support V4

[Christian König]

Hi everyone,

This is the fourth incarnation of this set of patches. It enables 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.

Some changes since V3:
1. A lot of minor style cleanups.
2. Make internal  functions for changing BARs directly private to the PCI subsystem.
3. Fail if any BAR is still in use when we try to change it.
4. Handle intermediate bridges as well.
5. Print some more messages when changing something.

Please review and/or comment,
Christian.

[PATCH 1/4] PCI: add resizeable BAR infrastructure v4

[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
and PCIe r3.1, sec 7.22.

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.
v4: move definition, improve commit message, s/bar/BAR/

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/pci/pci.c             | 115 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h             |   4 ++
 include/uapi/linux/pci_regs.h |  11 +++-
 3 files changed, 128 insertions(+), 2 deletions(-)

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/drivers/pci/pci.h b/drivers/pci/pci.h
index 4518562..1d27c22 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -356,4 +356,8 @@ static inline int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 }
 #endif
 
+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);
+
 #endif /* DRIVERS_PCI_H */
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index e5a2e68..a75429e 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_NBAR_MASK	(7 << 5)	/* mask for # bars */
-#define  PCI_REBAR_CTRL_NBAR_SHIFT	5	/* shift for # bars */
+#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

Re: [PATCH 1/4] PCI: add resizeable BAR infrastructure v4

[Alex Deucher]

On Tue, Apr 25, 2017 at 9:19 AM, Christian König
<deathsimple@vodafone.de> wrote:
> 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
> and PCIe r3.1, sec 7.22.
>
> 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.
> v4: move definition, improve commit message, s/bar/BAR/
>
> Signed-off-by: Christian König <christian.koenig@amd.com>


Patches 1, 2:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/pci/pci.c             | 115 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/pci.h             |   4 ++
>  include/uapi/linux/pci_regs.h |  11 +++-
>  3 files changed, 128 insertions(+), 2 deletions(-)
>
> 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/drivers/pci/pci.h b/drivers/pci/pci.h
> index 4518562..1d27c22 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -356,4 +356,8 @@ static inline int pci_dev_specific_reset(struct pci_dev *dev, int probe)
>  }
>  #endif
>
> +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);
> +
>  #endif /* DRIVERS_PCI_H */
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index e5a2e68..a75429e 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_NBAR_MASK      (7 << 5)        /* mask for # bars */
> -#define  PCI_REBAR_CTRL_NBAR_SHIFT     5       /* shift for # bars */
> +#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
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] PCI: add resizeable BAR infrastructure v4

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 4:19 PM, Christian König
<deathsimple@vodafone.de> wrote:
> 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
> and PCIe r3.1, sec 7.22.
>
> This is useful for hardware with large local storage (mostly GFX) which only
> expose 256MB BARs initially to be compatible with 32bit systems.

> +u32 pci_rbar_get_possible_sizes(struct pci_dev *pdev, int bar)
> +{

> +       unsigned pos, nbars;
> +       u32 ctrl, cap;
> +       unsigned i;

Are we supposed to use plain 'unsigned' nowadays? I would go with
'unsigned int'.

> +}

> + * Returns size if found or negativ error code.

Typo: negative.

> +int pci_rbar_get_current_size(struct pci_dev *pdev, int bar)
> +{
> +       unsigned pos, nbars;

> +       u32 ctrl;
> +       unsigned i;

Reversed tree order?

> +       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;
> +       }

This one the same as previous function, the difference only in what is
returned. CAre to split static helper function for both?

> +       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)

Not clear is it rounded up / down. I would go with "...in the spec
(0=1MB, 19=512GB)".

> + *
> + * 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;

Above is duplicating previous.

So,
static int ..._find_rbar(..., u32 *ctrl)
{
}

Returns: (i.e.) 0 - found, 1 - not found, -ERRNO.

ret = _find_rbar();
if (ret < 0)
 return ret;
if (ret)
 return -ENOENT;
...
return 0;

So, please refactor.

> +
> +               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;
> +}

> -#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_NBAR_MASK      (7 << 5)        /* mask for # BARs */
> +#define  PCI_REBAR_CTRL_NBAR_SHIFT     5       /* shift for # BARs */

I understand why, but I dunno it worth to do.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/4] PCI: add resizeable BAR infrastructure v4

[Christian König]

Am 26.04.2017 um 18:45 schrieb Andy Shevchenko:
> [SNIP]
>> -#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_NBAR_MASK      (7 << 5)        /* mask for # BARs */
>> +#define  PCI_REBAR_CTRL_NBAR_SHIFT     5       /* shift for # BARs */
> I understand why, but I dunno it worth to do.
Bjorn suggested that. Either way is fine with me, but he needs to stick 
his signed-of-by on it when pushing it upstream. So his opinion usually 
wins.

All other comments will be integrated in the next version of the patch.

Thanks for the review,
Christian.

[PATCH 2/4] PCI: add functionality for resizing resources v3

[Christian König]

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.

v2: rebase on changes in rbar support
v3: style cleanups, fail if memory decoding is enabled or resources
    still allocated, resize all unused bridge BARs,
    drop calling pci_reenable_device

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/pci/setup-bus.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/setup-res.c |  60 ++++++++++++++++++++++++++++
 include/linux/pci.h     |   3 ++
 3 files changed, 165 insertions(+)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index f30ca75..39351cf 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1923,6 +1923,108 @@ 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 pci_dev_resource *dev_res;
+	LIST_HEAD(saved);
+	LIST_HEAD(added);
+	LIST_HEAD(failed);
+	unsigned i;
+	int ret = 0;
+
+	/* Walk to the root BUS, releasing bridge BARs when possible */
+	while (1) {
+		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;
+
+			/* Ignore BARs which are still in use */
+			if (res->child)
+				continue;
+
+			ret = add_to_list(&saved, bridge, res, 0, 0);
+			if (ret)
+				goto cleanup;
+
+			if (res->parent)
+				release_resource(res);
+			res->start = 0;
+			res->end = 0;
+			dev_info(&bridge->dev, "BAR %d: released %pR\n",
+				 i, res);
+			break;
+		}
+		if (i == PCI_BRIDGE_RESOURCE_END)
+			break;
+
+		if (!bridge->bus || !bridge->bus->self)
+			break;
+
+		bridge = bridge->bus->self;
+	}
+
+	if (list_empty(&saved))
+		return -ENOENT;
+
+	__pci_bus_size_bridges(bridge->subordinate, &added);
+	__pci_bridge_assign_resources(bridge, &added, &failed);
+	BUG_ON(!list_empty(&added));
+
+	if (!list_empty(&failed)) {
+		ret = -ENOSPC;
+		goto cleanup;
+	}
+
+
+	list_for_each_entry(dev_res, &saved, list) {
+		/* Skip the bridge we just assigned resources for. */
+		if (bridge == dev_res->dev)
+			continue;
+
+		bridge = dev_res->dev;
+		pci_setup_bridge(bridge->subordinate);
+	}
+
+	free_list(&saved);
+	free_list(&failed);
+	return ret;
+
+cleanup:
+	/* restore size and flags */
+	list_for_each_entry(dev_res, &failed, list) {
+		struct resource *res = dev_res->res;
+
+		res->start = dev_res->start;
+		res->end = dev_res->end;
+		res->flags = dev_res->flags;
+	}
+	free_list(&failed);
+
+	/* Revert to the old configuration */
+	list_for_each_entry(dev_res, &saved, list) {
+		struct resource *res = dev_res->res;
+
+		bridge = dev_res->dev;
+		i = res - bridge->resource;
+
+		res->start = dev_res->start;
+		res->end = dev_res->end;
+		res->flags = dev_res->flags;
+
+		pci_claim_resource(bridge, i);
+		pci_setup_bridge(bridge->subordinate);
+	}
+	free_list(&saved);
+
+	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..0d40adb 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -363,6 +363,66 @@ int pci_reassign_resource(struct pci_dev *dev, int resno, resource_size_t addsiz
 	return 0;
 }
 
+void pci_release_resource(struct pci_dev *dev, int resno)
+{
+	struct resource *res = dev->resource + resno;
+
+	release_resource(res);
+	res->end = resource_size(res) - 1;
+	res->start = 0;
+	res->flags |= IORESOURCE_UNSET;
+	dev_info(&dev->dev, "BAR %d: released %pR\n", resno, res);
+}
+EXPORT_SYMBOL(pci_release_resource);
+
+int pci_resize_resource(struct pci_dev *dev, int resno, int size)
+{
+	struct resource *res = dev->resource + resno;
+	u64 bytes = 1ULL << (size + 20);
+	u16 cmd;
+	u32 sizes;
+	int old, ret = 0;
+
+	pci_read_config_word(dev, PCI_COMMAND, &cmd);
+	if (cmd & PCI_COMMAND_MEMORY)
+		return -EBUSY;
+
+	sizes = pci_rbar_get_possible_sizes(dev, resno);
+	if (!sizes)
+		return -ENOTSUPP;
+
+	if (!(sizes & BIT(size)))
+		return -EINVAL;
+
+	old = pci_rbar_get_current_size(dev, resno);
+	if (old < 0)
+		return old;
+
+	/* Make sure the resource isn't assigned before making it larger. */
+	pci_release_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;
+
+	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);
+	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 a38772a..f0a630a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1055,6 +1055,8 @@ 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);
+void pci_release_resource(struct pci_dev *dev, int resno);
+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 +1137,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

Re: [PATCH 2/4] PCI: add functionality for resizing resources v3

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 4:19 PM, Christian König
<deathsimple@vodafone.de> wrote:
> 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.

> +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;

> +

Redundant.

> +       struct pci_dev_resource *dev_res;
> +       LIST_HEAD(saved);
> +       LIST_HEAD(added);
> +       LIST_HEAD(failed);

> +       unsigned i;

unsigned int i;

> +       int ret = 0;
> +
> +       /* Walk to the root BUS, releasing bridge BARs when possible */

> +       while (1) {

This raises red flag. Care to refactor?
Also do {} while () syntax allows faster to get that the loop body
goes at least once.

> +               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))

I would rather go with

if ((res->flags ^ type) & type_mask)

> +                               continue;
> +
> +                       /* Ignore BARs which are still in use */
> +                       if (res->child)
> +                               continue;
> +
> +                       ret = add_to_list(&saved, bridge, res, 0, 0);
> +                       if (ret)
> +                               goto cleanup;
> +
> +                       if (res->parent)
> +                               release_resource(res);

> +                       res->start = 0;
> +                       res->end = 0;

> +                       dev_info(&bridge->dev, "BAR %d: released %pR\n",
> +                                i, res);

I doesn't make much sense to me after zeroing a range.

> +                       break;
> +               }
> +               if (i == PCI_BRIDGE_RESOURCE_END)
> +                       break;
> +
> +               if (!bridge->bus || !bridge->bus->self)
> +                       break;
> +
> +               bridge = bridge->bus->self;
> +       }
> +
> +       if (list_empty(&saved))
> +               return -ENOENT;
> +
> +       __pci_bus_size_bridges(bridge->subordinate, &added);
> +       __pci_bridge_assign_resources(bridge, &added, &failed);
> +       BUG_ON(!list_empty(&added));
> +
> +       if (!list_empty(&failed)) {
> +               ret = -ENOSPC;
> +               goto cleanup;
> +       }

> +
> +

Remove extra empty line.

> +       list_for_each_entry(dev_res, &saved, list) {
> +               /* Skip the bridge we just assigned resources for. */
> +               if (bridge == dev_res->dev)
> +                       continue;
> +
> +               bridge = dev_res->dev;
> +               pci_setup_bridge(bridge->subordinate);
> +       }
> +

> +       free_list(&saved);
> +       free_list(&failed);
> +       return ret;

You might re-use two lines with below, but perhaps better to show
which case returns 0 explicitly and drop assignment ret = 0 above.

> +
> +cleanup:
> +       /* restore size and flags */
> +       list_for_each_entry(dev_res, &failed, list) {
> +               struct resource *res = dev_res->res;
> +
> +               res->start = dev_res->start;
> +               res->end = dev_res->end;
> +               res->flags = dev_res->flags;
> +       }
> +       free_list(&failed);
> +
> +       /* Revert to the old configuration */
> +       list_for_each_entry(dev_res, &saved, list) {
> +               struct resource *res = dev_res->res;
> +
> +               bridge = dev_res->dev;
> +               i = res - bridge->resource;
> +
> +               res->start = dev_res->start;
> +               res->end = dev_res->end;
> +               res->flags = dev_res->flags;
> +
> +               pci_claim_resource(bridge, i);
> +               pci_setup_bridge(bridge->subordinate);
> +       }

> +       free_list(&saved);
> +
> +       return ret;

> +}

> +void pci_release_resource(struct pci_dev *dev, int resno)
> +{
> +       struct resource *res = dev->resource + resno;
> +
> +       release_resource(res);

> +       res->end = resource_size(res) - 1;
> +       res->start = 0;
> +       res->flags |= IORESOURCE_UNSET;

> +       dev_info(&dev->dev, "BAR %d: released %pR\n", resno, res);

Makes little sense to me after you cleared information out.

> +}
> +EXPORT_SYMBOL(pci_release_resource);
> +
> +int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> +{
> +       struct resource *res = dev->resource + resno;

> +       u64 bytes = 1ULL << (size + 20);

> +       res->end = res->start + (1ULL << (old + 20)) - 1;

Perhaps macro or helper?

static inline u64 rbar_size_to_bytes(size)
{
return 1ULL << (size + 20);
}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/4] PCI: add functionality for resizing resources v3

[Christian König]

Am 26.04.2017 um 19:00 schrieb Andy Shevchenko:
> On Tue, Apr 25, 2017 at 4:19 PM, Christian König
> <deathsimple@vodafone.de> wrote:
>> 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.
>> +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;
>> +
> Redundant.

Redundant, but also a reminder to myself that I wanted to ask something 
about that.

This type_mask is used already three times in this file, shouldn't we 
add a define for that?

> [SNIP]
>> +       list_for_each_entry(dev_res, &saved, list) {
>> +               /* Skip the bridge we just assigned resources for. */
>> +               if (bridge == dev_res->dev)
>> +                       continue;
>> +
>> +               bridge = dev_res->dev;
>> +               pci_setup_bridge(bridge->subordinate);
>> +       }
>> +
>> +       free_list(&saved);
>> +       free_list(&failed);
>> +       return ret;
> You might re-use two lines with below, but perhaps better to show
> which case returns 0 explicitly and drop assignment ret = 0 above.

Good point, but actually the free_list(&failed) is superfluous here 
since when the failed list isn't empty we end up in the cleanup path.

Going to fix all other comments in the next version.

Regards,
Christian.

Re: [PATCH 2/4] PCI: add functionality for resizing resources v3

[Andy Shevchenko]

On Tue, May 2, 2017 at 6:51 PM, Christian König <deathsimple@vodafone.de> wrote:
> Am 26.04.2017 um 19:00 schrieb Andy Shevchenko:
>> On Tue, Apr 25, 2017 at 4:19 PM, Christian König
>> <deathsimple@vodafone.de> wrote:

>>> +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;
>>> +
>>
>> Redundant.
>
>
> Redundant, but also a reminder to myself that I wanted to ask something
> about that.

Missed context I suppose. Usually I comment in one word something
obvious, i.e. redundant empty line.
Sorry for missing my point.

> This type_mask is used already three times in this file, shouldn't we add a
> define for that?

Yes, that's wxactly what I commented somewhere (in one of the rest cases).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/4] PCI: add functionality for resizing resources v3

[Christian König]

Am 26.04.2017 um 19:00 schrieb Andy Shevchenko:
> [SNIP]
>> +       while (1) {
> This raises red flag. Care to refactor?
> Also do {} while () syntax allows faster to get that the loop body
> goes at least once.

I've tried to refactor this, but couldn't come up with something which 
works and is readable at the same time.

Any suggestion how this should look like?

Christian.

Re: [PATCH 2/4] PCI: add functionality for resizing resources v3

[Andy Shevchenko]

On Thu, May 4, 2017 at 12:23 PM, Christian König
<deathsimple@vodafone.de> wrote:
> Am 26.04.2017 um 19:00 schrieb Andy Shevchenko:

>>> +       while (1) {
>>
>> This raises red flag. Care to refactor?
>> Also do {} while () syntax allows faster to get that the loop body
>> goes at least once.
>
>
> I've tried to refactor this, but couldn't come up with something which works
> and is readable at the same time.
>
> Any suggestion how this should look like?

This is original code.

--- 8< --- 8< ---

    while (1) {
            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;

                    /* Ignore BARs which are still in use */
                    if (res->child)
                            continue;

                    ret = add_to_list(&saved, bridge, res, 0, 0);
                    if (ret)
                            goto cleanup;

                    if (res->parent)
                            release_resource(res);
                    res->start = 0;
                    res->end = 0;
                    dev_info(&bridge->dev, "BAR %d: released %pR\n", i, res);
                    break;
            }
            if (i == PCI_BRIDGE_RESOURCE_END)
                    break;

            if (!bridge->bus || !bridge->bus->self)
                    break;

            bridge = bridge->bus->self;
    }

--- 8< --- 8< ---

I would think about something like below

static int ...xxx...(...)
{
    unsigned int i;
    int ret;

    for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END; i++) {
            struct resource *res = &bridge->resource[i];

            /* Ignore BARs which are still in use */
            if (((res->flags ^ type) & type_mask) == 0 && !res->child)
                    break;
    }
    if (i == PCI_BRIDGE_RESOURCE_END)
        return -EBUSY;

    ret = add_to_list(&saved, bridge, res, 0, 0);
    if (ret)
            return ret;

    if (res->parent)
            release_resource(res);

    res->start = 0;
    res->end = 0;
    dev_info(&bridge->dev, "BAR %d: released %pR\n", i, res);
    return i;
}


struct pci_dev *next = bridge;
...
    do {
        bridge = next;

        ret = ...xxx...(...);
        if (ret)
            goto cleanup;

        next = bridge->bus ? bridge->bus->self : NULL;
    } while (next);
...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/4] PCI: add functionality for resizing resources v3

[Andy Shevchenko]

On Thu, May 4, 2017 at 1:15 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, May 4, 2017 at 12:23 PM, Christian König
> <deathsimple@vodafone.de> wrote:
>> Am 26.04.2017 um 19:00 schrieb Andy Shevchenko:

> static int ...xxx...(...)
> {
>     unsigned int i;
>     int ret;
>

>     if (res->parent)
>             release_resource(res);
>

dev_info() should be here. I commented on this earlier, just forgot.

>     res->start = 0;
>     res->end = 0;
>     dev_info(&bridge->dev, "BAR %d: released %pR\n", i, res);
>     return i;
> }
>
>
> struct pci_dev *next = bridge;
> ...
>     do {
>         bridge = next;
>
>         ret = ...xxx...(...);

>         if (ret)

Here, of course,

if (ret < 0)

>             goto cleanup;
>
>         next = bridge->bus ? bridge->bus->self : NULL;
>     } while (next);


-- 
With Best Regards,
Andy Shevchenko

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

[Christian König]

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.

v2: style cleanups, increase size, add resource name, set correct flags,
    print message that windows was added

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..8d949c4 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)
+{
+	uint32_t base, limit, high;
+	struct resource *res, *conflict;
+	unsigned i;
+
+	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);
+	if (!res)
+		return;
+
+	res->name = "PCI Bus 0000:00";
+	res->flags = IORESOURCE_PREFETCH | IORESOURCE_MEM |
+		IORESOURCE_MEM_64 | IORESOURCE_WINDOW;
+	res->start = 0x100000000;
+	res->end = 0xfd00000000 - 1;
+
+	/* Just grab the free area behind system memory for this */
+	while ((conflict = request_resource_conflict(&iomem_resource, res)))
+		res->start = conflict->end + 1;
+
+	dev_info(&dev->dev, "adding root bus resource %pR\n", res);
+
+	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

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

[Alex Deucher]

On Tue, Apr 25, 2017 at 9:19 AM, Christian König
<deathsimple@vodafone.de> wrote:
> 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.
>
> v2: style cleanups, increase size, add resource name, set correct flags,
>     print message that windows was added
>
> 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..8d949c4 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)
> +{
> +       uint32_t base, limit, high;
> +       struct resource *res, *conflict;
> +       unsigned i;
> +
> +       for (i = 0; i < 8; ++i) {
> +               pci_read_config_dword(dev, 0x80 + i * 0x8, &base);
> +               pci_read_config_dword(dev, 0x180 + i * 0x4, &high);

Might be nice to define names for the register offsets.


> +
> +               /* 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);
> +       if (!res)
> +               return;
> +
> +       res->name = "PCI Bus 0000:00";
> +       res->flags = IORESOURCE_PREFETCH | IORESOURCE_MEM |
> +               IORESOURCE_MEM_64 | IORESOURCE_WINDOW;
> +       res->start = 0x100000000;
> +       res->end = 0xfd00000000 - 1;
> +
> +       /* Just grab the free area behind system memory for this */
> +       while ((conflict = request_resource_conflict(&iomem_resource, res)))
> +               res->start = conflict->end + 1;
> +
> +       dev_info(&dev->dev, "adding root bus resource %pR\n", res);
> +
> +       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);

Same here.  Either way:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> +
> +       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
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 4:19 PM, Christian König
<deathsimple@vodafone.de> wrote:
> 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.

> +static void pci_amd_enable_64bit_bar(struct pci_dev *dev)
> +{

> +       uint32_t base, limit, high;

Is u32 or uint32_t used actually in this file?

> +       struct resource *res, *conflict;

> +       unsigned i;

unsigned int i;

Reversed tree order.

> +       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);
> +       if (!res)
> +               return;
> +
> +       res->name = "PCI Bus 0000:00";

> +       res->flags = IORESOURCE_PREFETCH | IORESOURCE_MEM |
> +               IORESOURCE_MEM_64 | IORESOURCE_WINDOW;

You are using this constant twice AFAICS, perhaps define it in generic
pci.h and re-use?

> +       res->start = 0x100000000;
> +       res->end = 0xfd00000000 - 1;

Perhaps you forgot ul / ull in many places in your code. Care to
compile for 32-bit and fix the warnings?

> +       dev_info(&dev->dev, "adding root bus resource %pR\n", res);

Some of your messages I think are rather dev_dbg() than dev_info().

> +       base = ((res->start >> 8) & 0xffffff00) | 0x3;
> +       limit = ((res->end + 1) >> 8) & 0xffffff00;
> +       high = ((res->start >> 40) & 0xff) |
> +               ((((res->end + 1) >> 40) & 0xff) << 16);

It would be nice to have the magics defined (just on top of this function) like

#define PCI_AMD_BAR_XXX_MASK GENMASK(...)
#define PCI_AMD_BAR_XXX_SHIFT nn

> +       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);

Ditto for pos:s.

> +
> +       pci_bus_add_resource(dev->bus, res, 0);
> +}

-- 
With Best Regards,
Andy Shevchenko

[PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access v2

[Christian König]

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

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

v2: rebased, style cleanups, disable mem decode before resize,
    handle gmc_v9 as well, round size up to power of two.

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 | 37 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c      |  8 ++++---
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c      |  8 ++++---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c      | 10 ++++----
 5 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 4a16e3c..9484062 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1879,6 +1879,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 a09ad3cf..d5ca77a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -695,6 +695,43 @@ 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)
+{
+	u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size);
+	u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) -1;
+	u16 cmd;
+	int r;
+
+	/* Free the doorbell mapping, it most likely needs to move as well */
+	amdgpu_doorbell_fini(adev);
+	pci_release_resource(adev->pdev, 2);
+
+	/* Disable memory decoding while we change the BAR addresses and size */
+	pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd);
+	pci_write_config_word(adev->pdev, PCI_COMMAND,
+			      cmd & ~PCI_COMMAND_MEMORY);
+
+	r = pci_resize_resource(adev->pdev, 0, rbar_size);
+	if (r == -ENOSPC)
+		DRM_INFO("Not enough PCI address space for a large BAR.");
+	else if (r && r != -ENOTSUPP)
+		DRM_ERROR("Problem resizing BAR0 (%d).", r);
+
+	pci_write_config_word(adev->pdev, PCI_COMMAND, cmd);
+
+	/* When the doorbell BAR isn't available we have no chance of
+	 * using the device.
+	 */
+	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 a9083a1..ae71524 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -372,13 +372,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
 		}
 		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 4ac9978..1655de2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -534,13 +534,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
 		}
 		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_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index e5d4dfe..4bbef79 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -469,16 +469,18 @@ static int gmc_v9_0_mc_init(struct amdgpu_device *adev)
 	}
 	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 =
 		nbio_v6_1_get_memsize(adev) * 1024ULL * 1024ULL;
 	adev->mc.real_vram_size = adev->mc.mc_vram_size;
-	adev->mc.visible_vram_size = adev->mc.aper_size;
+
+	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);
 
 	/* In case the PCI BAR is larger than the actual amount of vram */
+	adev->mc.visible_vram_size = adev->mc.aper_size;
 	if (adev->mc.visible_vram_size > adev->mc.real_vram_size)
 		adev->mc.visible_vram_size = adev->mc.real_vram_size;
 
-- 
2.7.4

Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access v2

[Alex Deucher]

On Tue, Apr 25, 2017 at 9:19 AM, 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.
>
> v2: rebased, style cleanups, disable mem decode before resize,
>     handle gmc_v9 as well, round size up to power of two.
>
> 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 | 37 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c      |  8 ++++---
>  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c      |  8 ++++---
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c      | 10 ++++----
>  5 files changed, 54 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 4a16e3c..9484062 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1879,6 +1879,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 a09ad3cf..d5ca77a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -695,6 +695,43 @@ 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)

I'd suggest renaming this to amdgpu_device_resize_bar() to try and
impose some consistency in this file, but not a big deal either way.

> +{
> +       u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size);
> +       u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) -1;
> +       u16 cmd;
> +       int r;
> +
> +       /* Free the doorbell mapping, it most likely needs to move as well */
> +       amdgpu_doorbell_fini(adev);
> +       pci_release_resource(adev->pdev, 2);

This should check for if (adev->asic_type >= CHIP_BONAIRE) since SI
didn't have doorbells.

> +
> +       /* Disable memory decoding while we change the BAR addresses and size */
> +       pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd);
> +       pci_write_config_word(adev->pdev, PCI_COMMAND,
> +                             cmd & ~PCI_COMMAND_MEMORY);
> +
> +       r = pci_resize_resource(adev->pdev, 0, rbar_size);
> +       if (r == -ENOSPC)
> +               DRM_INFO("Not enough PCI address space for a large BAR.");
> +       else if (r && r != -ENOTSUPP)
> +               DRM_ERROR("Problem resizing BAR0 (%d).", r);
> +
> +       pci_write_config_word(adev->pdev, PCI_COMMAND, cmd);
> +
> +       /* When the doorbell BAR isn't available we have no chance of
> +        * using the device.
> +        */
> +       BUG_ON(amdgpu_doorbell_init(adev));

Same here.

> +}
> +
>  /*
>   * 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 a9083a1..ae71524 100644

What about SI (gmc_v6_0.c)?

Alex

> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -372,13 +372,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
>                 }
>                 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 4ac9978..1655de2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -534,13 +534,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
>                 }
>                 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_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index e5d4dfe..4bbef79 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -469,16 +469,18 @@ static int gmc_v9_0_mc_init(struct amdgpu_device *adev)
>         }
>         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 =
>                 nbio_v6_1_get_memsize(adev) * 1024ULL * 1024ULL;
>         adev->mc.real_vram_size = adev->mc.mc_vram_size;
> -       adev->mc.visible_vram_size = adev->mc.aper_size;
> +
> +       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);
>
>         /* In case the PCI BAR is larger than the actual amount of vram */
> +       adev->mc.visible_vram_size = adev->mc.aper_size;
>         if (adev->mc.visible_vram_size > adev->mc.real_vram_size)
>                 adev->mc.visible_vram_size = adev->mc.real_vram_size;
>
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access v2

[Christian König]

Am 25.04.2017 um 16:34 schrieb Alex Deucher:
> On Tue, Apr 25, 2017 at 9:19 AM, 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.
>>
>> v2: rebased, style cleanups, disable mem decode before resize,
>>      handle gmc_v9 as well, round size up to power of two.
>>
>> 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 | 37 ++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c      |  8 ++++---
>>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c      |  8 ++++---
>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c      | 10 ++++----
>>   5 files changed, 54 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 4a16e3c..9484062 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -1879,6 +1879,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 a09ad3cf..d5ca77a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -695,6 +695,43 @@ 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)
> I'd suggest renaming this to amdgpu_device_resize_bar() to try and
> impose some consistency in this file, but not a big deal either way.
>
>> +{
>> +       u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size);
>> +       u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) -1;
>> +       u16 cmd;
>> +       int r;
>> +
>> +       /* Free the doorbell mapping, it most likely needs to move as well */
>> +       amdgpu_doorbell_fini(adev);
>> +       pci_release_resource(adev->pdev, 2);
> This should check for if (adev->asic_type >= CHIP_BONAIRE) since SI
> didn't have doorbells.
>
>> +
>> +       /* Disable memory decoding while we change the BAR addresses and size */
>> +       pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd);
>> +       pci_write_config_word(adev->pdev, PCI_COMMAND,
>> +                             cmd & ~PCI_COMMAND_MEMORY);
>> +
>> +       r = pci_resize_resource(adev->pdev, 0, rbar_size);
>> +       if (r == -ENOSPC)
>> +               DRM_INFO("Not enough PCI address space for a large BAR.");
>> +       else if (r && r != -ENOTSUPP)
>> +               DRM_ERROR("Problem resizing BAR0 (%d).", r);
>> +
>> +       pci_write_config_word(adev->pdev, PCI_COMMAND, cmd);
>> +
>> +       /* When the doorbell BAR isn't available we have no chance of
>> +        * using the device.
>> +        */
>> +       BUG_ON(amdgpu_doorbell_init(adev));
> Same here.
>
>> +}
>> +
>>   /*
>>    * 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 a9083a1..ae71524 100644
> What about SI (gmc_v6_0.c)?

As far as I know there is no SI hardware with resizeable BAR support.

The all VI generation can do this, but I'm not 100% sure if there aren't 
any Bonaires out there which can handle it as well that's why I added 
gmc_v7 support.

Still need to double check that.

Christian.

>
> Alex
>
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -372,13 +372,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
>>                  }
>>                  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 4ac9978..1655de2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -534,13 +534,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
>>                  }
>>                  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_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index e5d4dfe..4bbef79 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -469,16 +469,18 @@ static int gmc_v9_0_mc_init(struct amdgpu_device *adev)
>>          }
>>          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 =
>>                  nbio_v6_1_get_memsize(adev) * 1024ULL * 1024ULL;
>>          adev->mc.real_vram_size = adev->mc.mc_vram_size;
>> -       adev->mc.visible_vram_size = adev->mc.aper_size;
>> +
>> +       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);
>>
>>          /* In case the PCI BAR is larger than the actual amount of vram */
>> +       adev->mc.visible_vram_size = adev->mc.aper_size;
>>          if (adev->mc.visible_vram_size > adev->mc.real_vram_size)
>>                  adev->mc.visible_vram_size = adev->mc.real_vram_size;
>>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access v2

[Alex Deucher]

On Tue, Apr 25, 2017 at 11:09 AM, Christian König
<deathsimple@vodafone.de> wrote:
> Am 25.04.2017 um 16:34 schrieb Alex Deucher:
>>
>> On Tue, Apr 25, 2017 at 9:19 AM, 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.
>>>
>>> v2: rebased, style cleanups, disable mem decode before resize,
>>>      handle gmc_v9 as well, round size up to power of two.
>>>
>>> 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 | 37
>>> ++++++++++++++++++++++++++++++
>>>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c      |  8 ++++---
>>>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c      |  8 ++++---
>>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c      | 10 ++++----
>>>   5 files changed, 54 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> index 4a16e3c..9484062 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -1879,6 +1879,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 a09ad3cf..d5ca77a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -695,6 +695,43 @@ 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)
>>
>> I'd suggest renaming this to amdgpu_device_resize_bar() to try and
>> impose some consistency in this file, but not a big deal either way.
>>
>>> +{
>>> +       u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size);
>>> +       u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) -1;
>>> +       u16 cmd;
>>> +       int r;
>>> +
>>> +       /* Free the doorbell mapping, it most likely needs to move as
>>> well */
>>> +       amdgpu_doorbell_fini(adev);
>>> +       pci_release_resource(adev->pdev, 2);
>>
>> This should check for if (adev->asic_type >= CHIP_BONAIRE) since SI
>> didn't have doorbells.
>>
>>> +
>>> +       /* Disable memory decoding while we change the BAR addresses and
>>> size */
>>> +       pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd);
>>> +       pci_write_config_word(adev->pdev, PCI_COMMAND,
>>> +                             cmd & ~PCI_COMMAND_MEMORY);
>>> +
>>> +       r = pci_resize_resource(adev->pdev, 0, rbar_size);
>>> +       if (r == -ENOSPC)
>>> +               DRM_INFO("Not enough PCI address space for a large
>>> BAR.");
>>> +       else if (r && r != -ENOTSUPP)
>>> +               DRM_ERROR("Problem resizing BAR0 (%d).", r);
>>> +
>>> +       pci_write_config_word(adev->pdev, PCI_COMMAND, cmd);
>>> +
>>> +       /* When the doorbell BAR isn't available we have no chance of
>>> +        * using the device.
>>> +        */
>>> +       BUG_ON(amdgpu_doorbell_init(adev));
>>
>> Same here.
>>
>>> +}
>>> +
>>>   /*
>>>    * 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 a9083a1..ae71524 100644
>>
>> What about SI (gmc_v6_0.c)?
>
>
> As far as I know there is no SI hardware with resizeable BAR support.
>
> The all VI generation can do this, but I'm not 100% sure if there aren't any
> Bonaires out there which can handle it as well that's why I added gmc_v7
> support.
>
> Still need to double check that.

I think NI or even evergreen supported resizeable bars in theory,
although I'm not sure if any boards were fused to expose it.

Alex


>
> Christian.
>
>
>>
>> Alex
>>
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> @@ -372,13 +372,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device
>>> *adev)
>>>                  }
>>>                  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 4ac9978..1655de2 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> @@ -534,13 +534,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device
>>> *adev)
>>>                  }
>>>                  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_v9_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> index e5d4dfe..4bbef79 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> @@ -469,16 +469,18 @@ static int gmc_v9_0_mc_init(struct amdgpu_device
>>> *adev)
>>>          }
>>>          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 =
>>>                  nbio_v6_1_get_memsize(adev) * 1024ULL * 1024ULL;
>>>          adev->mc.real_vram_size = adev->mc.mc_vram_size;
>>> -       adev->mc.visible_vram_size = adev->mc.aper_size;
>>> +
>>> +       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);
>>>
>>>          /* In case the PCI BAR is larger than the actual amount of vram
>>> */
>>> +       adev->mc.visible_vram_size = adev->mc.aper_size;
>>>          if (adev->mc.visible_vram_size > adev->mc.real_vram_size)
>>>                  adev->mc.visible_vram_size = adev->mc.real_vram_size;
>>>
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>

Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access v2

[Christian König]

Am 25.04.2017 um 17:14 schrieb Alex Deucher:
> On Tue, Apr 25, 2017 at 11:09 AM, Christian König
> <deathsimple@vodafone.de> wrote:
>> Am 25.04.2017 um 16:34 schrieb Alex Deucher:
>>> On Tue, Apr 25, 2017 at 9:19 AM, 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.
>>>>
>>>> v2: rebased, style cleanups, disable mem decode before resize,
>>>>       handle gmc_v9 as well, round size up to power of two.
>>>>
>>>> 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 | 37
>>>> ++++++++++++++++++++++++++++++
>>>>    drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c      |  8 ++++---
>>>>    drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c      |  8 ++++---
>>>>    drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c      | 10 ++++----
>>>>    5 files changed, 54 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> index 4a16e3c..9484062 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> @@ -1879,6 +1879,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 a09ad3cf..d5ca77a 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> @@ -695,6 +695,43 @@ 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)
>>> I'd suggest renaming this to amdgpu_device_resize_bar() to try and
>>> impose some consistency in this file, but not a big deal either way.
>>>
>>>> +{
>>>> +       u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size);
>>>> +       u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) -1;
>>>> +       u16 cmd;
>>>> +       int r;
>>>> +
>>>> +       /* Free the doorbell mapping, it most likely needs to move as
>>>> well */
>>>> +       amdgpu_doorbell_fini(adev);
>>>> +       pci_release_resource(adev->pdev, 2);
>>> This should check for if (adev->asic_type >= CHIP_BONAIRE) since SI
>>> didn't have doorbells.
>>>
>>>> +
>>>> +       /* Disable memory decoding while we change the BAR addresses and
>>>> size */
>>>> +       pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd);
>>>> +       pci_write_config_word(adev->pdev, PCI_COMMAND,
>>>> +                             cmd & ~PCI_COMMAND_MEMORY);
>>>> +
>>>> +       r = pci_resize_resource(adev->pdev, 0, rbar_size);
>>>> +       if (r == -ENOSPC)
>>>> +               DRM_INFO("Not enough PCI address space for a large
>>>> BAR.");
>>>> +       else if (r && r != -ENOTSUPP)
>>>> +               DRM_ERROR("Problem resizing BAR0 (%d).", r);
>>>> +
>>>> +       pci_write_config_word(adev->pdev, PCI_COMMAND, cmd);
>>>> +
>>>> +       /* When the doorbell BAR isn't available we have no chance of
>>>> +        * using the device.
>>>> +        */
>>>> +       BUG_ON(amdgpu_doorbell_init(adev));
>>> Same here.
>>>
>>>> +}
>>>> +
>>>>    /*
>>>>     * 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 a9083a1..ae71524 100644
>>> What about SI (gmc_v6_0.c)?
>>
>> As far as I know there is no SI hardware with resizeable BAR support.
>>
>> The all VI generation can do this, but I'm not 100% sure if there aren't any
>> Bonaires out there which can handle it as well that's why I added gmc_v7
>> support.
>>
>> Still need to double check that.
> I think NI or even evergreen supported resizeable bars in theory,
> although I'm not sure if any boards were fused to expose it.

In theory yes, in practice no.

Their PCIE config space defines the necessary registers, but I haven't 
found any board from Evergreen, NI, SI or CIK generation where that is 
actually validated.

Tonga is the first one I'm 100% sure that it is supported and I have 
hardware to test it.

I would rather drop Bonaire support as well and wait for somebody with 
the hardware to complain so that we can test and enable it.

Christian.

>
> Alex
>
>
>> Christian.
>>
>>
>>> Alex
>>>
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>> @@ -372,13 +372,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device
>>>> *adev)
>>>>                   }
>>>>                   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 4ac9978..1655de2 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>> @@ -534,13 +534,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device
>>>> *adev)
>>>>                   }
>>>>                   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_v9_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> index e5d4dfe..4bbef79 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> @@ -469,16 +469,18 @@ static int gmc_v9_0_mc_init(struct amdgpu_device
>>>> *adev)
>>>>           }
>>>>           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 =
>>>>                   nbio_v6_1_get_memsize(adev) * 1024ULL * 1024ULL;
>>>>           adev->mc.real_vram_size = adev->mc.mc_vram_size;
>>>> -       adev->mc.visible_vram_size = adev->mc.aper_size;
>>>> +
>>>> +       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);
>>>>
>>>>           /* In case the PCI BAR is larger than the actual amount of vram
>>>> */
>>>> +       adev->mc.visible_vram_size = adev->mc.aper_size;
>>>>           if (adev->mc.visible_vram_size > adev->mc.real_vram_size)
>>>>                   adev->mc.visible_vram_size = adev->mc.real_vram_size;
>>>>
>>>> --
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>>

Re: [PATCH 4/4] drm/amdgpu: resize VRAM BAR for CPU access v2

[Alex Deucher]

On Tue, Apr 25, 2017 at 12:22 PM, Christian König
<deathsimple@vodafone.de> wrote:
> Am 25.04.2017 um 17:14 schrieb Alex Deucher:
>>
>> On Tue, Apr 25, 2017 at 11:09 AM, Christian König
>> <deathsimple@vodafone.de> wrote:
>>>
>>> Am 25.04.2017 um 16:34 schrieb Alex Deucher:
>>>>
>>>> On Tue, Apr 25, 2017 at 9:19 AM, 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.
>>>>>
>>>>> v2: rebased, style cleanups, disable mem decode before resize,
>>>>>       handle gmc_v9 as well, round size up to power of two.
>>>>>
>>>>> 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 | 37
>>>>> ++++++++++++++++++++++++++++++
>>>>>    drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c      |  8 ++++---
>>>>>    drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c      |  8 ++++---
>>>>>    drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c      | 10 ++++----
>>>>>    5 files changed, 54 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> index 4a16e3c..9484062 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> @@ -1879,6 +1879,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 a09ad3cf..d5ca77a 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> @@ -695,6 +695,43 @@ 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)
>>>>
>>>> I'd suggest renaming this to amdgpu_device_resize_bar() to try and
>>>> impose some consistency in this file, but not a big deal either way.
>>>>
>>>>> +{
>>>>> +       u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size);
>>>>> +       u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) -1;
>>>>> +       u16 cmd;
>>>>> +       int r;
>>>>> +
>>>>> +       /* Free the doorbell mapping, it most likely needs to move as
>>>>> well */
>>>>> +       amdgpu_doorbell_fini(adev);
>>>>> +       pci_release_resource(adev->pdev, 2);
>>>>
>>>> This should check for if (adev->asic_type >= CHIP_BONAIRE) since SI
>>>> didn't have doorbells.
>>>>
>>>>> +
>>>>> +       /* Disable memory decoding while we change the BAR addresses
>>>>> and
>>>>> size */
>>>>> +       pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd);
>>>>> +       pci_write_config_word(adev->pdev, PCI_COMMAND,
>>>>> +                             cmd & ~PCI_COMMAND_MEMORY);
>>>>> +
>>>>> +       r = pci_resize_resource(adev->pdev, 0, rbar_size);
>>>>> +       if (r == -ENOSPC)
>>>>> +               DRM_INFO("Not enough PCI address space for a large
>>>>> BAR.");
>>>>> +       else if (r && r != -ENOTSUPP)
>>>>> +               DRM_ERROR("Problem resizing BAR0 (%d).", r);
>>>>> +
>>>>> +       pci_write_config_word(adev->pdev, PCI_COMMAND, cmd);
>>>>> +
>>>>> +       /* When the doorbell BAR isn't available we have no chance of
>>>>> +        * using the device.
>>>>> +        */
>>>>> +       BUG_ON(amdgpu_doorbell_init(adev));
>>>>
>>>> Same here.
>>>>
>>>>> +}
>>>>> +
>>>>>    /*
>>>>>     * 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 a9083a1..ae71524 100644
>>>>
>>>> What about SI (gmc_v6_0.c)?
>>>
>>>
>>> As far as I know there is no SI hardware with resizeable BAR support.
>>>
>>> The all VI generation can do this, but I'm not 100% sure if there aren't
>>> any
>>> Bonaires out there which can handle it as well that's why I added gmc_v7
>>> support.
>>>
>>> Still need to double check that.
>>
>> I think NI or even evergreen supported resizeable bars in theory,
>> although I'm not sure if any boards were fused to expose it.
>
>
> In theory yes, in practice no.
>
> Their PCIE config space defines the necessary registers, but I haven't found
> any board from Evergreen, NI, SI or CIK generation where that is actually
> validated.
>
> Tonga is the first one I'm 100% sure that it is supported and I have
> hardware to test it.
>
> I would rather drop Bonaire support as well and wait for somebody with the
> hardware to complain so that we can test and enable it.

I may have some workstation CI parts that support it.

Alex

>
> Christian.
>
>
>>
>> Alex
>>
>>
>>> Christian.
>>>
>>>
>>>> Alex
>>>>
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>>> @@ -372,13 +372,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device
>>>>> *adev)
>>>>>                   }
>>>>>                   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 4ac9978..1655de2 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>>> @@ -534,13 +534,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device
>>>>> *adev)
>>>>>                   }
>>>>>                   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_v9_0.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> index e5d4dfe..4bbef79 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> @@ -469,16 +469,18 @@ static int gmc_v9_0_mc_init(struct amdgpu_device
>>>>> *adev)
>>>>>           }
>>>>>           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 =
>>>>>                   nbio_v6_1_get_memsize(adev) * 1024ULL * 1024ULL;
>>>>>           adev->mc.real_vram_size = adev->mc.mc_vram_size;
>>>>> -       adev->mc.visible_vram_size = adev->mc.aper_size;
>>>>> +
>>>>> +       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);
>>>>>
>>>>>           /* In case the PCI BAR is larger than the actual amount of
>>>>> vram
>>>>> */
>>>>> +       adev->mc.visible_vram_size = adev->mc.aper_size;
>>>>>           if (adev->mc.visible_vram_size > adev->mc.real_vram_size)
>>>>>                   adev->mc.visible_vram_size = adev->mc.real_vram_size;
>>>>>
>>>>> --
>>>>> 2.7.4
>>>>>
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>>
>>>
>

Re: Resizeable PCI BAR support V4

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 4:19 PM, Christian König
<deathsimple@vodafone.de> wrote:
> Hi everyone,
>
> This is the fourth incarnation of this set of patches.

It would be nice if you put (next time) version of the series in each
patch. Usually it's done quite easy by passing -vX (where X is version
number) to git format-patch.

> It enables 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.
>
> Some changes since V3:
> 1. A lot of minor style cleanups.
> 2. Make internal  functions for changing BARs directly private to the PCI subsystem.
> 3. Fail if any BAR is still in use when we try to change it.
> 4. Handle intermediate bridges as well.
> 5. Print some more messages when changing something.
>
> Please review and/or comment,

Would look later (perhaps this week),

-- 
With Best Regards,
Andy Shevchenko