[PATCH 1/4] drm/amdgpu: always make gart.table_addr 64bit

[PATCH 1/4] drm/amdgpu: always make gart.table_addr 64bit

[Christian König]

Fixing warning/compile errors on 32bit kernels.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
index f15e319580ec..5eb1a6800f72 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
@@ -39,7 +39,7 @@ struct amdgpu_gart_funcs;
 #define AMDGPU_GPU_PAGE_ALIGN(a) (((a) + AMDGPU_GPU_PAGE_MASK) & ~AMDGPU_GPU_PAGE_MASK)
 
 struct amdgpu_gart {
-	dma_addr_t			table_addr;
+	u64				table_addr;
 	struct amdgpu_bo		*robj;
 	void				*ptr;
 	unsigned			num_gpu_pages;
-- 
2.11.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/4] drm/amdgpu: remove VRAM size reduction

[Christian König]

Remove some outdated comments and all code which tries to reduce the VRAM size
mapped into the MC.

This is superfluous and misleading since we never actually program the size.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 30 +-----------------------------
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c      |  6 ------
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c      |  6 ------
 3 files changed, 1 insertion(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 65fba5fb537e..f9bee76e7071 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -578,41 +578,13 @@ void amdgpu_wb_free(struct amdgpu_device *adev, u32 wb)
  * @base: base address at which to put VRAM
  *
  * Function will try to place VRAM at base address provided
- * as parameter (which is so far either PCI aperture address or
- * for IGP TOM base address).
- *
- * If there is not enough space to fit the unvisible VRAM in the 32bits
- * address space then we limit the VRAM size to the aperture.
- *
- * Note: We don't explicitly enforce VRAM start to be aligned on VRAM size,
- * this shouldn't be a problem as we are using the PCI aperture as a reference.
- * Otherwise this would be needed for rv280, all r3xx, and all r4xx, but
- * not IGP.
- *
- * Note: we use mc_vram_size as on some board we need to program the mc to
- * cover the whole aperture even if VRAM size is inferior to aperture size
- * Novell bug 204882 + along with lots of ubuntu ones
- *
- * Note: when limiting vram it's safe to overwritte real_vram_size because
- * we are not in case where real_vram_size is inferior to mc_vram_size (ie
- * note afected by bogus hw of Novell bug 204882 + along with lots of ubuntu
- * ones)
- *
- * Note: IGP TOM addr should be the same as the aperture addr, we don't
- * explicitly check for that though.
- *
- * FIXME: when reducing VRAM size align new size on power of 2.
+ * as parameter.
  */
 void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base)
 {
 	uint64_t limit = (uint64_t)amdgpu_vram_limit << 20;
 
 	mc->vram_start = base;
-	if (mc->mc_vram_size > (adev->mc.mc_mask - base + 1)) {
-		dev_warn(adev->dev, "limiting VRAM to PCI aperture size\n");
-		mc->real_vram_size = mc->aper_size;
-		mc->mc_vram_size = mc->aper_size;
-	}
 	mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
 	if (limit && limit < mc->real_vram_size)
 		mc->real_vram_size = limit;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index de7a249f0e24..d521862804ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -240,12 +240,6 @@ static void gmc_v7_0_vram_gtt_location(struct amdgpu_device *adev,
 	u64 base = RREG32(mmMC_VM_FB_LOCATION) & 0xFFFF;
 	base <<= 24;
 
-	if (mc->mc_vram_size > 0xFFC0000000ULL) {
-		/* leave room for at least 1024M GTT */
-		dev_warn(adev->dev, "limiting VRAM\n");
-		mc->real_vram_size = 0xFFC0000000ULL;
-		mc->mc_vram_size = 0xFFC0000000ULL;
-	}
 	amdgpu_vram_location(adev, &adev->mc, base);
 	amdgpu_gart_location(adev, mc);
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 67778744da5a..bd3f842cca00 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -405,12 +405,6 @@ static void gmc_v8_0_vram_gtt_location(struct amdgpu_device *adev,
 		base = RREG32(mmMC_VM_FB_LOCATION) & 0xFFFF;
 	base <<= 24;
 
-	if (mc->mc_vram_size > 0xFFC0000000ULL) {
-		/* leave room for at least 1024M GTT */
-		dev_warn(adev->dev, "limiting VRAM\n");
-		mc->real_vram_size = 0xFFC0000000ULL;
-		mc->mc_vram_size = 0xFFC0000000ULL;
-	}
 	amdgpu_vram_location(adev, &adev->mc, base);
 	amdgpu_gart_location(adev, mc);
 }
-- 
2.11.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/4] drm/amdgpu: align GTT start to 4GB

[Christian König]

For VCE to work properly the start of the GTT space must be aligned to a
4GB boundary.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f9bee76e7071..bbaab31218a8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -622,7 +622,7 @@ void amdgpu_gart_location(struct amdgpu_device *adev, struct amdgpu_mc *mc)
 			dev_warn(adev->dev, "limiting GTT\n");
 			mc->gart_size = size_af;
 		}
-		mc->gart_start = mc->vram_end + 1;
+		mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
 	}
 	mc->gart_end = mc->gart_start + mc->gart_size - 1;
 	dev_info(adev->dev, "GTT: %lluM 0x%016llX - 0x%016llX\n",
-- 
2.11.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/4] drm/amdgpu: fix VCE buffer placement restrictions

[Christian König]

Turned out that VCE still has a placement restriction that BOs can't
cross a 4GB boundary.

Fix this by adding a command submission parser prepass to correctly
place the buffers.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index 92477e67087c..2843e5b728e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -543,6 +543,43 @@ int amdgpu_vce_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
 	return r;
 }
 
+static int amdgpu_vce_validate_bo(struct amdgpu_cs_parser *p, uint32_t ib_idx,
+				  int lo, int hi, unsigned size, int32_t index)
+{
+	int64_t offset = ((uint64_t)size) * ((uint64_t)index);
+	struct amdgpu_bo_va_mapping *mapping;
+	unsigned i, fpfn, lpfn;
+	struct amdgpu_bo *bo;
+	uint64_t addr;
+	int r;
+
+	addr = ((uint64_t)amdgpu_get_ib_value(p, ib_idx, lo)) |
+	       ((uint64_t)amdgpu_get_ib_value(p, ib_idx, hi)) << 32;
+	if (index >= 0) {
+		addr += offset;
+		fpfn = PAGE_ALIGN(offset) >> PAGE_SHIFT;
+		lpfn = 0x100000000ULL >> PAGE_SHIFT;
+	} else {
+		fpfn = 0;
+		lpfn = (0x100000000ULL - PAGE_ALIGN(offset)) >> PAGE_SHIFT;
+	}
+
+	r = amdgpu_cs_find_mapping(p, addr, &bo, &mapping);
+	if (r) {
+		DRM_ERROR("Can't find BO for addr 0x%010Lx %d %d %d %d\n",
+			  addr, lo, hi, size, index);
+		return r;
+	}
+
+	for (i = 0; i < bo->placement.num_placement; ++i) {
+		bo->placements[i].fpfn = max(bo->placements[i].fpfn, fpfn);
+		bo->placements[i].lpfn = bo->placements[i].fpfn ?
+			min(bo->placements[i].fpfn, lpfn) : lpfn;
+	}
+	return ttm_bo_validate(&bo->tbo, &bo->placement, false, false);
+}
+
+
 /**
  * amdgpu_vce_cs_reloc - command submission relocation
  *
@@ -648,12 +685,13 @@ int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p, uint32_t ib_idx)
 	uint32_t allocated = 0;
 	uint32_t tmp, handle = 0;
 	uint32_t *size = &tmp;
-	int i, r = 0, idx = 0;
+	unsigned idx;
+	int i, r = 0;
 
 	p->job->vm = NULL;
 	ib->gpu_addr = amdgpu_sa_bo_gpu_addr(ib->sa_bo);
 
-	while (idx < ib->length_dw) {
+	for (idx = 0; idx < ib->length_dw;) {
 		uint32_t len = amdgpu_get_ib_value(p, ib_idx, idx);
 		uint32_t cmd = amdgpu_get_ib_value(p, ib_idx, idx + 1);
 
@@ -664,6 +702,54 @@ int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p, uint32_t ib_idx)
 		}
 
 		switch (cmd) {
+		case 0x00000002: /* task info */
+			fb_idx = amdgpu_get_ib_value(p, ib_idx, idx + 6);
+			bs_idx = amdgpu_get_ib_value(p, ib_idx, idx + 7);
+			break;
+
+		case 0x03000001: /* encode */
+			r = amdgpu_vce_validate_bo(p, ib_idx, idx + 10,
+						   idx + 9, 0, 0);
+			if (r)
+				goto out;
+
+			r = amdgpu_vce_validate_bo(p, ib_idx, idx + 12,
+						   idx + 11, 0, 0);
+			if (r)
+				goto out;
+			break;
+
+		case 0x05000001: /* context buffer */
+			r = amdgpu_vce_validate_bo(p, ib_idx, idx + 3,
+						   idx + 2, 0, 0);
+			if (r)
+				goto out;
+			break;
+
+		case 0x05000004: /* video bitstream buffer */
+			tmp = amdgpu_get_ib_value(p, ib_idx, idx + 4);
+			r = amdgpu_vce_validate_bo(p, ib_idx, idx + 3, idx + 2,
+						   tmp, bs_idx);
+			if (r)
+				goto out;
+			break;
+
+		case 0x05000005: /* feedback buffer */
+			r = amdgpu_vce_validate_bo(p, ib_idx, idx + 3, idx + 2,
+						   4096, fb_idx);
+			if (r)
+				goto out;
+			break;
+		}
+
+		idx += len / 4;
+	}
+
+	for (idx = 0; idx < ib->length_dw;) {
+		uint32_t len = amdgpu_get_ib_value(p, ib_idx, idx);
+		uint32_t cmd = amdgpu_get_ib_value(p, ib_idx, idx + 1);
+
+		switch (cmd) {
 		case 0x00000001: /* session */
 			handle = amdgpu_get_ib_value(p, ib_idx, idx + 2);
 			session_idx = amdgpu_vce_validate_handle(p, handle,
-- 
2.11.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/4] drm/amdgpu: always make gart.table_addr 64bit

[Alex Deucher]

On Fri, Nov 17, 2017 at 5:12 AM, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
> Fixing warning/compile errors on 32bit kernels.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
> index f15e319580ec..5eb1a6800f72 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
> @@ -39,7 +39,7 @@ struct amdgpu_gart_funcs;
>  #define AMDGPU_GPU_PAGE_ALIGN(a) (((a) + AMDGPU_GPU_PAGE_MASK) & ~AMDGPU_GPU_PAGE_MASK)
>
>  struct amdgpu_gart {
> -       dma_addr_t                      table_addr;
> +       u64                             table_addr;
>         struct amdgpu_bo                *robj;
>         void                            *ptr;
>         unsigned                        num_gpu_pages;
> --
> 2.11.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/4] drm/amdgpu: remove VRAM size reduction

[Alex Deucher]

On Fri, Nov 17, 2017 at 5:12 AM, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
> Remove some outdated comments and all code which tries to reduce the VRAM size
> mapped into the MC.
>
> This is superfluous and misleading since we never actually program the size.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 30 +-----------------------------
>  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c      |  6 ------
>  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c      |  6 ------

Please also fix up gmc_v6_0.c.  With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>  3 files changed, 1 insertion(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 65fba5fb537e..f9bee76e7071 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -578,41 +578,13 @@ void amdgpu_wb_free(struct amdgpu_device *adev, u32 wb)
>   * @base: base address at which to put VRAM
>   *
>   * Function will try to place VRAM at base address provided
> - * as parameter (which is so far either PCI aperture address or
> - * for IGP TOM base address).
> - *
> - * If there is not enough space to fit the unvisible VRAM in the 32bits
> - * address space then we limit the VRAM size to the aperture.
> - *
> - * Note: We don't explicitly enforce VRAM start to be aligned on VRAM size,
> - * this shouldn't be a problem as we are using the PCI aperture as a reference.
> - * Otherwise this would be needed for rv280, all r3xx, and all r4xx, but
> - * not IGP.
> - *
> - * Note: we use mc_vram_size as on some board we need to program the mc to
> - * cover the whole aperture even if VRAM size is inferior to aperture size
> - * Novell bug 204882 + along with lots of ubuntu ones
> - *
> - * Note: when limiting vram it's safe to overwritte real_vram_size because
> - * we are not in case where real_vram_size is inferior to mc_vram_size (ie
> - * note afected by bogus hw of Novell bug 204882 + along with lots of ubuntu
> - * ones)
> - *
> - * Note: IGP TOM addr should be the same as the aperture addr, we don't
> - * explicitly check for that though.
> - *
> - * FIXME: when reducing VRAM size align new size on power of 2.
> + * as parameter.
>   */
>  void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base)
>  {
>         uint64_t limit = (uint64_t)amdgpu_vram_limit << 20;
>
>         mc->vram_start = base;
> -       if (mc->mc_vram_size > (adev->mc.mc_mask - base + 1)) {
> -               dev_warn(adev->dev, "limiting VRAM to PCI aperture size\n");
> -               mc->real_vram_size = mc->aper_size;
> -               mc->mc_vram_size = mc->aper_size;
> -       }
>         mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
>         if (limit && limit < mc->real_vram_size)
>                 mc->real_vram_size = limit;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index de7a249f0e24..d521862804ea 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -240,12 +240,6 @@ static void gmc_v7_0_vram_gtt_location(struct amdgpu_device *adev,
>         u64 base = RREG32(mmMC_VM_FB_LOCATION) & 0xFFFF;
>         base <<= 24;
>
> -       if (mc->mc_vram_size > 0xFFC0000000ULL) {
> -               /* leave room for at least 1024M GTT */
> -               dev_warn(adev->dev, "limiting VRAM\n");
> -               mc->real_vram_size = 0xFFC0000000ULL;
> -               mc->mc_vram_size = 0xFFC0000000ULL;
> -       }
>         amdgpu_vram_location(adev, &adev->mc, base);
>         amdgpu_gart_location(adev, mc);
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 67778744da5a..bd3f842cca00 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -405,12 +405,6 @@ static void gmc_v8_0_vram_gtt_location(struct amdgpu_device *adev,
>                 base = RREG32(mmMC_VM_FB_LOCATION) & 0xFFFF;
>         base <<= 24;
>
> -       if (mc->mc_vram_size > 0xFFC0000000ULL) {
> -               /* leave room for at least 1024M GTT */
> -               dev_warn(adev->dev, "limiting VRAM\n");
> -               mc->real_vram_size = 0xFFC0000000ULL;
> -               mc->mc_vram_size = 0xFFC0000000ULL;
> -       }
>         amdgpu_vram_location(adev, &adev->mc, base);
>         amdgpu_gart_location(adev, mc);
>  }
> --
> 2.11.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/4] drm/amdgpu: align GTT start to 4GB

[Alex Deucher]

On Fri, Nov 17, 2017 at 5:12 AM, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
> For VCE to work properly the start of the GTT space must be aligned to a
> 4GB boundary.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index f9bee76e7071..bbaab31218a8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -622,7 +622,7 @@ void amdgpu_gart_location(struct amdgpu_device *adev, struct amdgpu_mc *mc)
>                         dev_warn(adev->dev, "limiting GTT\n");
>                         mc->gart_size = size_af;
>                 }
> -               mc->gart_start = mc->vram_end + 1;
> +               mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);

Please add a comment here as to why we are doing this.  With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>


>         }
>         mc->gart_end = mc->gart_start + mc->gart_size - 1;
>         dev_info(adev->dev, "GTT: %lluM 0x%016llX - 0x%016llX\n",
> --
> 2.11.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/4] drm/amdgpu: fix VCE buffer placement restrictions

[Alex Deucher]

On Fri, Nov 17, 2017 at 5:12 AM, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
> Turned out that VCE still has a placement restriction that BOs can't
> cross a 4GB boundary.
>
> Fix this by adding a command submission parser prepass to correctly
> place the buffers.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

Acked-by: Alex Deucher <alexander.deucher@amd.com>


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 90 ++++++++++++++++++++++++++++++++-
>  1 file changed, 88 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index 92477e67087c..2843e5b728e5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -543,6 +543,43 @@ int amdgpu_vce_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
>         return r;
>  }
>
> +static int amdgpu_vce_validate_bo(struct amdgpu_cs_parser *p, uint32_t ib_idx,
> +                                 int lo, int hi, unsigned size, int32_t index)
> +{
> +       int64_t offset = ((uint64_t)size) * ((uint64_t)index);
> +       struct amdgpu_bo_va_mapping *mapping;
> +       unsigned i, fpfn, lpfn;
> +       struct amdgpu_bo *bo;
> +       uint64_t addr;
> +       int r;
> +
> +       addr = ((uint64_t)amdgpu_get_ib_value(p, ib_idx, lo)) |
> +              ((uint64_t)amdgpu_get_ib_value(p, ib_idx, hi)) << 32;
> +       if (index >= 0) {
> +               addr += offset;
> +               fpfn = PAGE_ALIGN(offset) >> PAGE_SHIFT;
> +               lpfn = 0x100000000ULL >> PAGE_SHIFT;
> +       } else {
> +               fpfn = 0;
> +               lpfn = (0x100000000ULL - PAGE_ALIGN(offset)) >> PAGE_SHIFT;
> +       }
> +
> +       r = amdgpu_cs_find_mapping(p, addr, &bo, &mapping);
> +       if (r) {
> +               DRM_ERROR("Can't find BO for addr 0x%010Lx %d %d %d %d\n",
> +                         addr, lo, hi, size, index);
> +               return r;
> +       }
> +
> +       for (i = 0; i < bo->placement.num_placement; ++i) {
> +               bo->placements[i].fpfn = max(bo->placements[i].fpfn, fpfn);
> +               bo->placements[i].lpfn = bo->placements[i].fpfn ?
> +                       min(bo->placements[i].fpfn, lpfn) : lpfn;
> +       }
> +       return ttm_bo_validate(&bo->tbo, &bo->placement, false, false);
> +}
> +
> +
>  /**
>   * amdgpu_vce_cs_reloc - command submission relocation
>   *
> @@ -648,12 +685,13 @@ int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p, uint32_t ib_idx)
>         uint32_t allocated = 0;
>         uint32_t tmp, handle = 0;
>         uint32_t *size = &tmp;
> -       int i, r = 0, idx = 0;
> +       unsigned idx;
> +       int i, r = 0;
>
>         p->job->vm = NULL;
>         ib->gpu_addr = amdgpu_sa_bo_gpu_addr(ib->sa_bo);
>
> -       while (idx < ib->length_dw) {
> +       for (idx = 0; idx < ib->length_dw;) {
>                 uint32_t len = amdgpu_get_ib_value(p, ib_idx, idx);
>                 uint32_t cmd = amdgpu_get_ib_value(p, ib_idx, idx + 1);
>
> @@ -664,6 +702,54 @@ int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p, uint32_t ib_idx)
>                 }
>
>                 switch (cmd) {
> +               case 0x00000002: /* task info */
> +                       fb_idx = amdgpu_get_ib_value(p, ib_idx, idx + 6);
> +                       bs_idx = amdgpu_get_ib_value(p, ib_idx, idx + 7);
> +                       break;
> +
> +               case 0x03000001: /* encode */
> +                       r = amdgpu_vce_validate_bo(p, ib_idx, idx + 10,
> +                                                  idx + 9, 0, 0);
> +                       if (r)
> +                               goto out;
> +
> +                       r = amdgpu_vce_validate_bo(p, ib_idx, idx + 12,
> +                                                  idx + 11, 0, 0);
> +                       if (r)
> +                               goto out;
> +                       break;
> +
> +               case 0x05000001: /* context buffer */
> +                       r = amdgpu_vce_validate_bo(p, ib_idx, idx + 3,
> +                                                  idx + 2, 0, 0);
> +                       if (r)
> +                               goto out;
> +                       break;
> +
> +               case 0x05000004: /* video bitstream buffer */
> +                       tmp = amdgpu_get_ib_value(p, ib_idx, idx + 4);
> +                       r = amdgpu_vce_validate_bo(p, ib_idx, idx + 3, idx + 2,
> +                                                  tmp, bs_idx);
> +                       if (r)
> +                               goto out;
> +                       break;
> +
> +               case 0x05000005: /* feedback buffer */
> +                       r = amdgpu_vce_validate_bo(p, ib_idx, idx + 3, idx + 2,
> +                                                  4096, fb_idx);
> +                       if (r)
> +                               goto out;
> +                       break;
> +               }
> +
> +               idx += len / 4;
> +       }
> +
> +       for (idx = 0; idx < ib->length_dw;) {
> +               uint32_t len = amdgpu_get_ib_value(p, ib_idx, idx);
> +               uint32_t cmd = amdgpu_get_ib_value(p, ib_idx, idx + 1);
> +
> +               switch (cmd) {
>                 case 0x00000001: /* session */
>                         handle = amdgpu_get_ib_value(p, ib_idx, idx + 2);
>                         session_idx = amdgpu_vce_validate_handle(p, handle,
> --
> 2.11.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/4] drm/amdgpu: fix VCE buffer placement restrictions

[Christian König]

Leo and Andy could you two give that patch set a try?

It should fix occasional VCE fall outs when by coincident a buffers is 
placed on a 4GB boundary.

Regards,
Christian.

Am 17.11.2017 um 11:12 schrieb Christian König:
> Turned out that VCE still has a placement restriction that BOs can't
> cross a 4GB boundary.
>
> Fix this by adding a command submission parser prepass to correctly
> place the buffers.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 90 ++++++++++++++++++++++++++++++++-
>   1 file changed, 88 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index 92477e67087c..2843e5b728e5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -543,6 +543,43 @@ int amdgpu_vce_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
>   	return r;
>   }
>   
> +static int amdgpu_vce_validate_bo(struct amdgpu_cs_parser *p, uint32_t ib_idx,
> +				  int lo, int hi, unsigned size, int32_t index)
> +{
> +	int64_t offset = ((uint64_t)size) * ((uint64_t)index);
> +	struct amdgpu_bo_va_mapping *mapping;
> +	unsigned i, fpfn, lpfn;
> +	struct amdgpu_bo *bo;
> +	uint64_t addr;
> +	int r;
> +
> +	addr = ((uint64_t)amdgpu_get_ib_value(p, ib_idx, lo)) |
> +	       ((uint64_t)amdgpu_get_ib_value(p, ib_idx, hi)) << 32;
> +	if (index >= 0) {
> +		addr += offset;
> +		fpfn = PAGE_ALIGN(offset) >> PAGE_SHIFT;
> +		lpfn = 0x100000000ULL >> PAGE_SHIFT;
> +	} else {
> +		fpfn = 0;
> +		lpfn = (0x100000000ULL - PAGE_ALIGN(offset)) >> PAGE_SHIFT;
> +	}
> +
> +	r = amdgpu_cs_find_mapping(p, addr, &bo, &mapping);
> +	if (r) {
> +		DRM_ERROR("Can't find BO for addr 0x%010Lx %d %d %d %d\n",
> +			  addr, lo, hi, size, index);
> +		return r;
> +	}
> +
> +	for (i = 0; i < bo->placement.num_placement; ++i) {
> +		bo->placements[i].fpfn = max(bo->placements[i].fpfn, fpfn);
> +		bo->placements[i].lpfn = bo->placements[i].fpfn ?
> +			min(bo->placements[i].fpfn, lpfn) : lpfn;
> +	}
> +	return ttm_bo_validate(&bo->tbo, &bo->placement, false, false);
> +}
> +
> +
>   /**
>    * amdgpu_vce_cs_reloc - command submission relocation
>    *
> @@ -648,12 +685,13 @@ int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p, uint32_t ib_idx)
>   	uint32_t allocated = 0;
>   	uint32_t tmp, handle = 0;
>   	uint32_t *size = &tmp;
> -	int i, r = 0, idx = 0;
> +	unsigned idx;
> +	int i, r = 0;
>   
>   	p->job->vm = NULL;
>   	ib->gpu_addr = amdgpu_sa_bo_gpu_addr(ib->sa_bo);
>   
> -	while (idx < ib->length_dw) {
> +	for (idx = 0; idx < ib->length_dw;) {
>   		uint32_t len = amdgpu_get_ib_value(p, ib_idx, idx);
>   		uint32_t cmd = amdgpu_get_ib_value(p, ib_idx, idx + 1);
>   
> @@ -664,6 +702,54 @@ int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p, uint32_t ib_idx)
>   		}
>   
>   		switch (cmd) {
> +		case 0x00000002: /* task info */
> +			fb_idx = amdgpu_get_ib_value(p, ib_idx, idx + 6);
> +			bs_idx = amdgpu_get_ib_value(p, ib_idx, idx + 7);
> +			break;
> +
> +		case 0x03000001: /* encode */
> +			r = amdgpu_vce_validate_bo(p, ib_idx, idx + 10,
> +						   idx + 9, 0, 0);
> +			if (r)
> +				goto out;
> +
> +			r = amdgpu_vce_validate_bo(p, ib_idx, idx + 12,
> +						   idx + 11, 0, 0);
> +			if (r)
> +				goto out;
> +			break;
> +
> +		case 0x05000001: /* context buffer */
> +			r = amdgpu_vce_validate_bo(p, ib_idx, idx + 3,
> +						   idx + 2, 0, 0);
> +			if (r)
> +				goto out;
> +			break;
> +
> +		case 0x05000004: /* video bitstream buffer */
> +			tmp = amdgpu_get_ib_value(p, ib_idx, idx + 4);
> +			r = amdgpu_vce_validate_bo(p, ib_idx, idx + 3, idx + 2,
> +						   tmp, bs_idx);
> +			if (r)
> +				goto out;
> +			break;
> +
> +		case 0x05000005: /* feedback buffer */
> +			r = amdgpu_vce_validate_bo(p, ib_idx, idx + 3, idx + 2,
> +						   4096, fb_idx);
> +			if (r)
> +				goto out;
> +			break;
> +		}
> +
> +		idx += len / 4;
> +	}
> +
> +	for (idx = 0; idx < ib->length_dw;) {
> +		uint32_t len = amdgpu_get_ib_value(p, ib_idx, idx);
> +		uint32_t cmd = amdgpu_get_ib_value(p, ib_idx, idx + 1);
> +
> +		switch (cmd) {
>   		case 0x00000001: /* session */
>   			handle = amdgpu_get_ib_value(p, ib_idx, idx + 2);
>   			session_idx = amdgpu_vce_validate_handle(p, handle,


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/4] drm/amdgpu: fix VCE buffer placement restrictions

[Andy Furniss]

Christian König wrote:
> Leo and Andy could you two give that patch set a try?
> 
> It should fix occasional VCE fall outs when by coincident a buffers is 
> placed on a 4GB boundary.

On drm-next-4.15 vanilla the corruption like in -

https://bugs.freedesktop.org/show_bug.cgi?id=102296

is still present

On both vanilla and patched kernels "lesser" test cases work OK.

With the same test case as the bug (gstreamer encoding 500 frames 2160p 
nv12 in ram) with the patches I get

amdgpu: Not enough memory for command submission.
0:00:01.551998246  1092      0x238bd40 ERROR            vaapiencode 
gstvaapiencode.c:214:gst_vaapiencode_default_alloc_buffer: invalid 
GstVaapiCodedBuffer size (0 bytes)
0:00:01.552090124  1092      0x238bd40 ERROR            vaapiencode 
gstvaapiencode.c:332:gst_vaapiencode_push_frame: failed to allocate 
encoded buffer in system memory
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/4] drm/amdgpu: fix VCE buffer placement restrictions

[Andy Furniss]

Andy Furniss wrote:
> Christian König wrote:
>> Leo and Andy could you two give that patch set a try?
>>
>> It should fix occasional VCE fall outs when by coincident a buffers is 
>> placed on a 4GB boundary.
> 
> On drm-next-4.15 vanilla the corruption like in -
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=102296
> 
> is still present
> 
> On both vanilla and patched kernels "lesser" test cases work OK.

More testing, and it seems this regresses other previously working
gstreamer tests like a transcode, giving same as below.

ffmpeg seems OK, also disabling dual instance "fixes" like in the bug.

> 
> With the same test case as the bug (gstreamer encoding 500 frames 2160p 
> nv12 in ram) with the patches I get
> 
> amdgpu: Not enough memory for command submission.
> 0:00:01.551998246  1092      0x238bd40 ERROR            vaapiencode 
> gstvaapiencode.c:214:gst_vaapiencode_default_alloc_buffer: invalid 
> GstVaapiCodedBuffer size (0 bytes)
> 0:00:01.552090124  1092      0x238bd40 ERROR            vaapiencode 
> gstvaapiencode.c:332:gst_vaapiencode_push_frame: failed to allocate 
> encoded buffer in system memory
> amdgpu: The CS has been cancelled because the context is lost.
> amdgpu: The CS has been cancelled because the context is lost.
> amdgpu: The CS has been cancelled because the context is lost.
> amdgpu: The CS has been cancelled because the context is lost.
> amdgpu: The CS has been cancelled because the context is lost.
> amdgpu: The CS has been cancelled because the context is lost.
> amdgpu: The CS has been cancelled because the context is lost.
> amdgpu: The CS has been cancelled because the context is lost.
> amdgpu: The CS has been cancelled because the context is lost.
> amdgpu: The CS has been cancelled because the context is lost.
> amdgpu: The CS has been cancelled because the context is lost.
> amdgpu: The CS has been cancelled because the context is lost.
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx