Re: [Intel-gfx] [PATCH 1/2] drm/i915: Add support for LMEM PCIe resizable bar

From: Thomas Hellström (Intel)
Date: Wed Jun 29 2022 - 02:00:31 EST

Next message: jie1zhan: "[PATCH v1] Fix: SYNCOBJ TIMELINE Test failed."
Previous message: Greg KH: "Re: 回复：[PATCH] usb: gadget: Fix unsigned comparison with less than zero"
In reply to: Dandamudi, Priyanka: "RE: [Intel-gfx] [PATCH 1/2] drm/i915: Add support for LMEM PCIe resizable bar"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi,

On 6/24/22 06:02, Dandamudi, Priyanka wrote:

-----Original Message-----
From: Christian König <christian.koenig@xxxxxxx>
Sent: 18 June 2022 08:45 PM
To: De Marchi, Lucas <lucas.demarchi@xxxxxxxxx>; Bjorn Helgaas
<helgaas@xxxxxxxxxx>
Cc: linux-pci@xxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Sergei
Miroshnichenko <s.miroshnichenko@xxxxxxxxx>; linux-
kernel@xxxxxxxxxxxxxxx; Dandamudi, Priyanka
<priyanka.dandamudi@xxxxxxxxx>; Auld, Matthew
<matthew.auld@xxxxxxxxx>; Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915: Add support for LMEM PCIe
resizable bar

Am 17.06.22 um 23:27 schrieb Lucas De Marchi:

On Fri, Jun 17, 2022 at 03:32:52PM -0500, Bjorn Helgaas wrote:

[+cc Christian, author of pci_resize_resource(), Sergei, author of
rebalancing patches]

Hi Lucas,

On Fri, Jun 17, 2022 at 11:44:41AM -0700, Lucas De Marchi wrote:

Cc'ing intel-pci, lkml, Bjorn

On Fri, Jun 17, 2022 at 11:32:37AM +0300, Jani Nikula wrote:

On Thu, 16 Jun 2022, priyanka.dandamudi@xxxxxxxxx wrote:

From: Akeem G Abodunrin <akeem.g.abodunrin@xxxxxxxxx>

Add support for the local memory PICe resizable bar, so that
local memory can be resized to the maximum size supported by the

device,

and mapped correctly to the PCIe memory bar. It is usual that
GPU devices expose only 256MB BARs primarily to be compatible
with

32-bit

systems. So, those devices cannot claim larger memory BAR

windows size due

to the system BIOS limitation. With this change, it would be

possible to

reprogram the windows of the bridge directly above the

requesting device

on the same BAR type.

There is a big caveat here that this may be too late as other
drivers may have already mapped their BARs - so probably too late in
the pci scan for it to be effective. In fact, after using this for a
while, it seems to fail too often, particularly on CFL systems.

Help me understand the "too late" part. Do you mean that there is
enough available space for the max BAR size, but it's fragmented and
therefore not usable? And that if we could do something earlier,
before drivers have claimed their devices, we might be able to
compact the BARs of other devices to make a larger contiguous available

space?

yes. I will dig some logs I had in the past to confirm.

That is theoretically possible, but I think the current
pci_resize_resource() only supports resizing of the specified BAR and
any upstream bridge windows. I don't think it supports moving BARs
of other devices.

Sergei did some nice work that might help with this situation because
it can move BARs around more generally. It hasn't quite achieved
critical mass yet, but maybe this would help get there:

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flor

e.kernel.org%2Flinux-pci%2F20201218174011.340514-1-

s.miroshnichenko%4
0yadro.com%2F&data=05%7C01%7Cchristian.koenig%40amd.com%7C8
096027
f68484d0656b108da50a82e7d%7C3dd8961fe4884e608e11a82d994e183d%7C
0%7C0%
7C637910980509199388%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
wMDAiLCJQ
IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&
sdata=
%2FfntE2FTQ8wmLnz4wnzk94R0GMLEwVs7Mj18%2B9Q6PJk%3D&reser
ved=0

oh... I hadn't thought about pause/ioremap/unpause. That looks rad :).
So it seems this would integrate neatly with
pci_resize_resource() (what this patch is doing), as long as drivers
for devices affected implement
.bar_fixed()/.rescan_prepare()/.rescan_done(). That seems it would
solve our issues too.

Well we never ran into any of the issues you describe with PCIe BAR resize
for GPUs so there must be something you do differently to AMD hardware
regarding this.

Additional to that keep in mind that you can't resize the BAR before kicking
out vgacon/efifb or otherwise it can happen that the just released 256MiB
window is still used while you try to resize it. When you do that you usually
end up with a hard lockup of the system.

Regards,
Christian.

thanks
Lucas De Marchi

If I understand Sergei's series correctly, this rebalancing actually
cannot be done during enumeration because we only move BARs if a
driver for the device indicates that it supports it, so there would
be no requirement to do this early.

Do we have any alternative to be done in the PCI subsystem during
the scan? There is other work in progress to allow i915 to use the
rest of the device memory even with a smaller BAR, but it would be
better if we can improve our chances of succeeding the resize.

Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@xxxxxxxxx>
Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx>
Cc: Stuart Summers <stuart.summers@xxxxxxxxx>
Cc: Michael J Ruhl <michael.j.ruhl@xxxxxxxxx>
Cc: Prathap Kumar Valsan <prathap.kumar.valsan@xxxxxxxxx>
Signed-off-by: Priyanka Dandamudi

<priyanka.dandamudi@xxxxxxxxx>

Reviewed-by: Matthew Auld <matthew.auld@xxxxxxxxx>

Please see

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
re.kernel.org%2Fr%2F87pmj8vesm.fsf%40intel.com&data=05%7C01%7C
ch
ristian.koenig%40amd.com%7C8096027f68484d0656b108da50a82e7d%7C3d
d896
1fe4884e608e11a82d994e183d%7C0%7C0%7C637910980509199388%7CUnk
nown%7C
TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
CJX
VCI6Mn0%3D%7C3000%7C%7C%7C&sdata=d4cf7HQ6t7y1Xobwjdt8im%
2Fh0E5IZ

sXgzQDpsB2vCU4%3D&reserved=0

---
drivers/gpu/drm/i915/i915_driver.c | 92

++++++++++++++++++++++++++++++

1 file changed, 92 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_driver.c

b/drivers/gpu/drm/i915/i915_driver.c

index d26dcca7e654..4bdb471cb2e2 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -303,6 +303,95 @@ static void sanitize_gpu(struct

drm_i915_private *i915)

          __intel_gt_reset(to_gt(i915), ALL_ENGINES);
}

+static void __release_bars(struct pci_dev *pdev) {
+    int resno;
+
+    for (resno = PCI_STD_RESOURCES; resno <

PCI_STD_RESOURCE_END; resno++) {

+        if (pci_resource_len(pdev, resno))
+            pci_release_resource(pdev, resno);
+    }
+}
+
+static void
+__resize_bar(struct drm_i915_private *i915, int resno,

resource_size_t size)

+{
+    struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
+    int bar_size = pci_rebar_bytes_to_size(size);
+    int ret;
+
+    __release_bars(pdev);
+
+    ret = pci_resize_resource(pdev, resno, bar_size);
+    if (ret) {
+        drm_info(&i915->drm, "Failed to resize BAR%d to %dM

(%pe)\n",

+             resno, 1 << bar_size, ERR_PTR(ret));
+        return;
+    }
+
+    drm_info(&i915->drm, "BAR%d resized to %dM\n", resno, 1 <<

bar_size);

+}
+
+/* BAR size starts from 1MB - 2^20 */ #define BAR_SIZE_SHIFT 20
+static resource_size_t __lmem_rebar_size(struct
+drm_i915_private *i915, int resno) {
+    struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
+    u32 rebar = pci_rebar_get_possible_sizes(pdev, resno);
+    resource_size_t size;
+
+    if (!rebar)
+        return 0;
+
+    size = 1ULL << (__fls(rebar) + BAR_SIZE_SHIFT);
+
+    if (size <= pci_resource_len(pdev, resno))
+        return 0;
+
+    return size;
+}
+
+#define LMEM_BAR_NUM 2
+static void i915_resize_lmem_bar(struct drm_i915_private *i915)
+{
+    struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
+    struct pci_bus *root = pdev->bus;
+    struct resource *root_res;
+    resource_size_t rebar_size = __lmem_rebar_size(i915,

LMEM_BAR_NUM);

+    u32 pci_cmd;
+    int i;
+
+    if (!rebar_size)
+        return;
+
+    /* Find out if root bus contains 64bit memory addressing */
+    while (root->parent)
+        root = root->parent;
+
+    pci_bus_for_each_resource(root, root_res, i) {
+        if (root_res && root_res->flags & (IORESOURCE_MEM |
+                    IORESOURCE_MEM_64) && root_res->start >

0x100000000ull)

+            break;
+    }
+
+    /* pci_resize_resource will fail anyways */
+    if (!root_res) {
+        drm_info(&i915->drm, "Can't resize LMEM BAR - platform

support is missing\n");

+        return;
+    }
+
+    /* First disable PCI memory decoding references */
+    pci_read_config_dword(pdev, PCI_COMMAND, &pci_cmd);
+    pci_write_config_dword(pdev, PCI_COMMAND,
+                   pci_cmd & ~PCI_COMMAND_MEMORY);
+
+    __resize_bar(i915, LMEM_BAR_NUM, rebar_size);
+
+ pci_assign_unassigned_bus_resources(pdev->bus);
+    pci_write_config_dword(pdev, PCI_COMMAND, pci_cmd); }
+
/**
   * i915_driver_early_probe - setup state not requiring device

access

   * @dev_priv: device private
@@ -852,6 +941,9 @@ int i915_driver_probe(struct pci_dev *pdev,

const struct pci_device_id *ent)

disable_rpm_wakeref_asserts(&i915->runtime_pm);

+    if (HAS_LMEM(i915))
+        i915_resize_lmem_bar(i915);
+
      intel_vgpu_detect(i915);

      ret = intel_gt_probe_all(i915);

--
Jani Nikula, Intel Open Source Graphics Center

[Dandamudi, Priyanka]
@De Marchi, Lucas
Can I proceed with the current approach or is there anything I need to add to it?

IMO we should be good to go. From my understanding, the problem that Lucas brings up doesn't yet have a solution other than WIP, so if we end up not being able to resize, we'd fall back to using the small BAR with Matthew's work.

That said, If we keep hitting errors when resizing, we should, as Christian says, compare with AMD (since they are not seeing this) and see what, if anything, can be done differently.

Thanks,

Thomas

Next message: jie1zhan: "[PATCH v1] Fix: SYNCOBJ TIMELINE Test failed."
Previous message: Greg KH: "Re: 回复：[PATCH] usb: gadget: Fix unsigned comparison with less than zero"
In reply to: Dandamudi, Priyanka: "RE: [Intel-gfx] [PATCH 1/2] drm/i915: Add support for LMEM PCIe resizable bar"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]