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

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

[Christian König]

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

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

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 8a5f8cb..1e888d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1754,6 +1754,7 @@ uint32_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_tt *ttm,
 				 struct ttm_mem_reg *mem);
 void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base);
 void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc);
+void amdgpu_resize_bar0(struct amdgpu_device *adev);
 void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size);
 int amdgpu_ttm_init(struct amdgpu_device *adev);
 void amdgpu_ttm_fini(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index a470869..f038195 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -616,6 +616,35 @@ void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc)
 			mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
 }
 
+/**
+ * amdgpu_resize_bar0 - try to resize BAR0
+ *
+ * @adev: amdgpu_device pointer
+ *
+ * Try to resize BAR0 to make all VRAM CPU accessible.
+ */
+void amdgpu_resize_bar0(struct amdgpu_device *adev)
+{
+	u32 size = max(ilog2(adev->mc.real_vram_size - 1) + 1, 20) - 20;
+	int r;
+
+	r = pci_resize_resource(adev->pdev, 0, size);
+
+	if (r == -ENOTSUPP) {
+		/* The hardware don't support the extension. */
+		return;
+
+	} else if (r == -ENOSPC) {
+		DRM_INFO("Not enoigh PCI address space for a large BAR.");
+	} else if (r) {
+		DRM_ERROR("Problem resizing BAR0 (%d).", r);
+	}
+
+	/* Reinit the doorbell mapping, it is most likely moved as well */
+	amdgpu_doorbell_fini(adev);
+	BUG_ON(amdgpu_doorbell_init(adev));
+}
+
 /*
  * GPU helpers function.
  */
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 552bf6b..cd5828c 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -367,13 +367,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
 		break;
 	}
 	adev->mc.vram_width = numchan * chansize;
-	/* Could aper size report 0 ? */
-	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
-	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
 	/* size in MB on si */
 	adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
 	adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
 
+	if (!(adev->flags & AMD_IS_APU))
+		amdgpu_resize_bar0(adev);
+	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
+	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
+
 #ifdef CONFIG_X86_64
 	if (adev->flags & AMD_IS_APU) {
 		adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index f2bd016..e277130 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -459,13 +459,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
 		break;
 	}
 	adev->mc.vram_width = numchan * chansize;
-	/* Could aper size report 0 ? */
-	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
-	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
 	/* size in MB on si */
 	adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
 	adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
 
+	if (!(adev->flags & AMD_IS_APU))
+		amdgpu_resize_bar0(adev);
+	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
+	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
+
 #ifdef CONFIG_X86_64
 	if (adev->flags & AMD_IS_APU) {
 		adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
-- 
2.7.4

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

[Andy Shevchenko]

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

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

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

Too complicated.

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

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

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

> +

Redundant line.

> +       if (r =3D=3D -ENOTSUPP) {
> +               /* The hardware don't support the extension. */
> +               return;
> +
> +       } else if (r =3D=3D -ENOSPC) {
> +               DRM_INFO("Not enoigh PCI address space for a large BAR.")=
;
> +       } else if (r) {
> +               DRM_ERROR("Problem resizing BAR0 (%d).", r);
> +       }
> +
> +       /* Reinit the doorbell mapping, it is most likely moved as well *=
/
> +       amdgpu_doorbell_fini(adev);

> +       BUG_ON(amdgpu_doorbell_init(adev));

No way to recover?!

> +}
> +

--=20
With Best Regards,
Andy Shevchenko

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

[Christian König]

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

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

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

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

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

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

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

>
>> +       int r;
>> +
>> +       r = pci_resize_resource(adev->pdev, 0, size);
>> +
> Redundant line.
>
>> +       if (r == -ENOTSUPP) {
>> +               /* The hardware don't support the extension. */
>> +               return;
>> +
>> +       } else if (r == -ENOSPC) {
>> +               DRM_INFO("Not enoigh PCI address space for a large BAR.");
>> +       } else if (r) {
>> +               DRM_ERROR("Problem resizing BAR0 (%d).", r);
>> +       }
>> +
>> +       /* Reinit the doorbell mapping, it is most likely moved as well */
>> +       amdgpu_doorbell_fini(adev);
>> +       BUG_ON(amdgpu_doorbell_init(adev));
> No way to recover?!

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

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

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

Thanks for the comments,
Christian.

>
>> +}
>> +

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

[sonofagun]

> +		DRM_INFO("Not enoigh PCI address space for a large BAR.");

Please fix the typo of the message so that another patch is not needed 
in the future!