All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] radeon: use memcpy_to/fromio for UVD fw upload
@ 2021-06-03  5:35 ` Chen Li
  0 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-03  5:35 UTC (permalink / raw)
  To: Alex Deucher, "Christian König", dri-devel, amd-gfx


I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Also fix some coding issues reported from sparse.

Signed-off-by: Chen Li <chenli@uniontech.com>
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 30 ++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index dfa9fdbe98da..0d6a5cfa2abf 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
 
 			rdev->uvd.fw_header_present = true;
 
-			family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
-			version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
-			version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
+			family_id = (__force u32)(hdr->ucode_version) & 0xff;
+			version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 24) & 0xff;
+			version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 8) & 0xff;
 			DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
 				 version_major, version_minor, family_id);
 
@@ -286,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	if (rdev->uvd.vcpu_bo == NULL)
 		return -EINVAL;
 
-	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
+				rdev->uvd_fw->data,
+				le32_to_cpu((__force __le32)rdev->uvd_fw->size));
 
 	size = radeon_bo_size(rdev->uvd.vcpu_bo);
 	size -= rdev->uvd_fw->size;
@@ -294,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	ptr = rdev->uvd.cpu_addr;
 	ptr += rdev->uvd_fw->size;
 
-	memset(ptr, 0, size);
+	memset_io((void __iomem *)ptr, 0, size);
 
 	return 0;
 }
@@ -791,17 +795,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD create msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
 	writel(0x0, (void __iomem *)&msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	writel(0x0, &msg[4]);
 	writel(0x0, &msg[5]);
 	writel(0x0, &msg[6]);
-	writel(cpu_to_le32(0x00000780), &msg[7]);
-	writel(cpu_to_le32(0x00000440), &msg[8]);
+	writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
+	writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
 	writel(0x0, &msg[9]);
-	writel(cpu_to_le32(0x01b37000), &msg[10]);
+	writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
 	for (i = 11; i < 1024; ++i)
 		writel(0x0, &msg[i]);
 
@@ -827,9 +831,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD destroy msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
-	writel(cpu_to_le32(0x00000002), &msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	for (i = 4; i < 1024; ++i)
 		writel(0x0, &msg[i]);
-- 
2.31.1




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

* [PATCH] radeon: use memcpy_to/fromio for UVD fw upload
@ 2021-06-03  5:35 ` Chen Li
  0 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-03  5:35 UTC (permalink / raw)
  To: Alex Deucher, "Christian König", dri-devel, amd-gfx


I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Also fix some coding issues reported from sparse.

Signed-off-by: Chen Li <chenli@uniontech.com>
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 30 ++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index dfa9fdbe98da..0d6a5cfa2abf 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
 
 			rdev->uvd.fw_header_present = true;
 
-			family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
-			version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
-			version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
+			family_id = (__force u32)(hdr->ucode_version) & 0xff;
+			version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 24) & 0xff;
+			version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 8) & 0xff;
 			DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
 				 version_major, version_minor, family_id);
 
@@ -286,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	if (rdev->uvd.vcpu_bo == NULL)
 		return -EINVAL;
 
-	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
+				rdev->uvd_fw->data,
+				le32_to_cpu((__force __le32)rdev->uvd_fw->size));
 
 	size = radeon_bo_size(rdev->uvd.vcpu_bo);
 	size -= rdev->uvd_fw->size;
@@ -294,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	ptr = rdev->uvd.cpu_addr;
 	ptr += rdev->uvd_fw->size;
 
-	memset(ptr, 0, size);
+	memset_io((void __iomem *)ptr, 0, size);
 
 	return 0;
 }
@@ -791,17 +795,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD create msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
 	writel(0x0, (void __iomem *)&msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	writel(0x0, &msg[4]);
 	writel(0x0, &msg[5]);
 	writel(0x0, &msg[6]);
-	writel(cpu_to_le32(0x00000780), &msg[7]);
-	writel(cpu_to_le32(0x00000440), &msg[8]);
+	writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
+	writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
 	writel(0x0, &msg[9]);
-	writel(cpu_to_le32(0x01b37000), &msg[10]);
+	writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
 	for (i = 11; i < 1024; ++i)
 		writel(0x0, &msg[i]);
 
@@ -827,9 +831,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD destroy msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
-	writel(cpu_to_le32(0x00000002), &msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	for (i = 4; i < 1024; ++i)
 		writel(0x0, &msg[i]);
-- 
2.31.1



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

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

* Re: [PATCH] radeon: use memcpy_to/fromio for UVD fw upload
  2021-06-03  5:35 ` Chen Li
@ 2021-06-03 19:21   ` Alex Deucher
  -1 siblings, 0 replies; 40+ messages in thread
From: Alex Deucher @ 2021-06-03 19:21 UTC (permalink / raw)
  To: Chen Li
  Cc: Alex Deucher, amd-gfx list, Christian König,
	Maling list - DRI developers

On Thu, Jun 3, 2021 at 3:35 AM Chen Li <chenli@uniontech.com> wrote:
>
>
> I met a gpu addr bug recently and the kernel log
> tells me the pc is memcpy/memset and link register is
> radeon_uvd_resume.
>
> As we know, in some architectures, optimized memcpy/memset
> may not work well on device memory. Trival memcpy_toio/memset_io
> can fix this problem.
>
> BTW, amdgpu has already done it in:
> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> that's why it has no this issue on the same gpu and platform.
>
> Also fix some coding issues reported from sparse.

Can you split the sparse fixes and the mmio fixes into two patches?

Thanks,

Alex

>
> Signed-off-by: Chen Li <chenli@uniontech.com>
> ---
>  drivers/gpu/drm/radeon/radeon_uvd.c | 30 ++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index dfa9fdbe98da..0d6a5cfa2abf 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
>
>                         rdev->uvd.fw_header_present = true;
>
> -                       family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
> -                       version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
> -                       version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
> +                       family_id = (__force u32)(hdr->ucode_version) & 0xff;
> +                       version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
> +                                                        >> 24) & 0xff;
> +                       version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
> +                                                        >> 8) & 0xff;
>                         DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
>                                  version_major, version_minor, family_id);
>
> @@ -286,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>         if (rdev->uvd.vcpu_bo == NULL)
>                 return -EINVAL;
>
> -       memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> +       memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
> +                               rdev->uvd_fw->data,
> +                               le32_to_cpu((__force __le32)rdev->uvd_fw->size));
>
>         size = radeon_bo_size(rdev->uvd.vcpu_bo);
>         size -= rdev->uvd_fw->size;
> @@ -294,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>         ptr = rdev->uvd.cpu_addr;
>         ptr += rdev->uvd_fw->size;
>
> -       memset(ptr, 0, size);
> +       memset_io((void __iomem *)ptr, 0, size);
>
>         return 0;
>  }
> @@ -791,17 +795,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
>                 return r;
>
>         /* stitch together an UVD create msg */
> -       writel(cpu_to_le32(0x00000de4), &msg[0]);
> +       writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
>         writel(0x0, (void __iomem *)&msg[1]);
> -       writel(cpu_to_le32(handle), &msg[2]);
> +       writel((__force u32)cpu_to_le32(handle), &msg[2]);
>         writel(0x0, &msg[3]);
>         writel(0x0, &msg[4]);
>         writel(0x0, &msg[5]);
>         writel(0x0, &msg[6]);
> -       writel(cpu_to_le32(0x00000780), &msg[7]);
> -       writel(cpu_to_le32(0x00000440), &msg[8]);
> +       writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
> +       writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
>         writel(0x0, &msg[9]);
> -       writel(cpu_to_le32(0x01b37000), &msg[10]);
> +       writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
>         for (i = 11; i < 1024; ++i)
>                 writel(0x0, &msg[i]);
>
> @@ -827,9 +831,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
>                 return r;
>
>         /* stitch together an UVD destroy msg */
> -       writel(cpu_to_le32(0x00000de4), &msg[0]);
> -       writel(cpu_to_le32(0x00000002), &msg[1]);
> -       writel(cpu_to_le32(handle), &msg[2]);
> +       writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
> +       writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
> +       writel((__force u32)cpu_to_le32(handle), &msg[2]);
>         writel(0x0, &msg[3]);
>         for (i = 4; i < 1024; ++i)
>                 writel(0x0, &msg[i]);
> --
> 2.31.1
>
>
>

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

* Re: [PATCH] radeon: use memcpy_to/fromio for UVD fw upload
@ 2021-06-03 19:21   ` Alex Deucher
  0 siblings, 0 replies; 40+ messages in thread
From: Alex Deucher @ 2021-06-03 19:21 UTC (permalink / raw)
  To: Chen Li
  Cc: Alex Deucher, amd-gfx list, Christian König,
	Maling list - DRI developers

On Thu, Jun 3, 2021 at 3:35 AM Chen Li <chenli@uniontech.com> wrote:
>
>
> I met a gpu addr bug recently and the kernel log
> tells me the pc is memcpy/memset and link register is
> radeon_uvd_resume.
>
> As we know, in some architectures, optimized memcpy/memset
> may not work well on device memory. Trival memcpy_toio/memset_io
> can fix this problem.
>
> BTW, amdgpu has already done it in:
> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> that's why it has no this issue on the same gpu and platform.
>
> Also fix some coding issues reported from sparse.

Can you split the sparse fixes and the mmio fixes into two patches?

Thanks,

Alex

>
> Signed-off-by: Chen Li <chenli@uniontech.com>
> ---
>  drivers/gpu/drm/radeon/radeon_uvd.c | 30 ++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index dfa9fdbe98da..0d6a5cfa2abf 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
>
>                         rdev->uvd.fw_header_present = true;
>
> -                       family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
> -                       version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
> -                       version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
> +                       family_id = (__force u32)(hdr->ucode_version) & 0xff;
> +                       version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
> +                                                        >> 24) & 0xff;
> +                       version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
> +                                                        >> 8) & 0xff;
>                         DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
>                                  version_major, version_minor, family_id);
>
> @@ -286,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>         if (rdev->uvd.vcpu_bo == NULL)
>                 return -EINVAL;
>
> -       memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> +       memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
> +                               rdev->uvd_fw->data,
> +                               le32_to_cpu((__force __le32)rdev->uvd_fw->size));
>
>         size = radeon_bo_size(rdev->uvd.vcpu_bo);
>         size -= rdev->uvd_fw->size;
> @@ -294,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>         ptr = rdev->uvd.cpu_addr;
>         ptr += rdev->uvd_fw->size;
>
> -       memset(ptr, 0, size);
> +       memset_io((void __iomem *)ptr, 0, size);
>
>         return 0;
>  }
> @@ -791,17 +795,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
>                 return r;
>
>         /* stitch together an UVD create msg */
> -       writel(cpu_to_le32(0x00000de4), &msg[0]);
> +       writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
>         writel(0x0, (void __iomem *)&msg[1]);
> -       writel(cpu_to_le32(handle), &msg[2]);
> +       writel((__force u32)cpu_to_le32(handle), &msg[2]);
>         writel(0x0, &msg[3]);
>         writel(0x0, &msg[4]);
>         writel(0x0, &msg[5]);
>         writel(0x0, &msg[6]);
> -       writel(cpu_to_le32(0x00000780), &msg[7]);
> -       writel(cpu_to_le32(0x00000440), &msg[8]);
> +       writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
> +       writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
>         writel(0x0, &msg[9]);
> -       writel(cpu_to_le32(0x01b37000), &msg[10]);
> +       writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
>         for (i = 11; i < 1024; ++i)
>                 writel(0x0, &msg[i]);
>
> @@ -827,9 +831,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
>                 return r;
>
>         /* stitch together an UVD destroy msg */
> -       writel(cpu_to_le32(0x00000de4), &msg[0]);
> -       writel(cpu_to_le32(0x00000002), &msg[1]);
> -       writel(cpu_to_le32(handle), &msg[2]);
> +       writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
> +       writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
> +       writel((__force u32)cpu_to_le32(handle), &msg[2]);
>         writel(0x0, &msg[3]);
>         for (i = 4; i < 1024; ++i)
>                 writel(0x0, &msg[i]);
> --
> 2.31.1
>
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* [PATCH v2 0/2] use memcpy_to/fromio for UVD fw upload
  2021-06-03 19:21   ` Alex Deucher
@ 2021-06-04  3:00     ` Chen Li
  -1 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  3:00 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Alex Deucher, amd-gfx list, Christian König,
	Maling list - DRI developers


changelog:
    v1->v2: split sparse and memcp/memset fix

Chen Li (2):
  radeon: fix coding issues reported from sparse
  radeon: use memcpy_to/fromio for UVD fw upload

 drivers/gpu/drm/radeon/radeon_uvd.c | 30 ++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

-- 
2.31.1




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

* [PATCH v2 0/2] use memcpy_to/fromio for UVD fw upload
@ 2021-06-04  3:00     ` Chen Li
  0 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  3:00 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Alex Deucher, amd-gfx list, Christian König,
	Maling list - DRI developers


changelog:
    v1->v2: split sparse and memcp/memset fix

Chen Li (2):
  radeon: fix coding issues reported from sparse
  radeon: use memcpy_to/fromio for UVD fw upload

 drivers/gpu/drm/radeon/radeon_uvd.c | 30 ++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

-- 
2.31.1



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

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

* [PATCH v2 1/2] radeon: fix coding issues reported from sparse
  2021-06-03  5:35 ` Chen Li
@ 2021-06-04  3:02   ` Chen Li
  -1 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  3:02 UTC (permalink / raw)
  To: Alex Deucher, "Christian König", dri-devel, amd-gfx


Also fix some coding issue reported from sparse.

Signed-off-by: Chen Li <chenli@uniontech.com>
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index dfa9fdbe98da..85a1f2c31749 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
 
 			rdev->uvd.fw_header_present = true;
 
-			family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
-			version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
-			version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
+			family_id = (__force u32)(hdr->ucode_version) & 0xff;
+			version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 24) & 0xff;
+			version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 8) & 0xff;
 			DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
 				 version_major, version_minor, family_id);
 
@@ -791,17 +793,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD create msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
 	writel(0x0, (void __iomem *)&msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	writel(0x0, &msg[4]);
 	writel(0x0, &msg[5]);
 	writel(0x0, &msg[6]);
-	writel(cpu_to_le32(0x00000780), &msg[7]);
-	writel(cpu_to_le32(0x00000440), &msg[8]);
+	writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
+	writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
 	writel(0x0, &msg[9]);
-	writel(cpu_to_le32(0x01b37000), &msg[10]);
+	writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
 	for (i = 11; i < 1024; ++i)
 		writel(0x0, &msg[i]);
 
@@ -827,9 +829,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD destroy msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
-	writel(cpu_to_le32(0x00000002), &msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	for (i = 4; i < 1024; ++i)
 		writel(0x0, &msg[i]);
-- 
2.31.1




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

* [PATCH v2 1/2] radeon: fix coding issues reported from sparse
@ 2021-06-04  3:02   ` Chen Li
  0 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  3:02 UTC (permalink / raw)
  To: Alex Deucher, "Christian König", dri-devel, amd-gfx


Also fix some coding issue reported from sparse.

Signed-off-by: Chen Li <chenli@uniontech.com>
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index dfa9fdbe98da..85a1f2c31749 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
 
 			rdev->uvd.fw_header_present = true;
 
-			family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
-			version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
-			version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
+			family_id = (__force u32)(hdr->ucode_version) & 0xff;
+			version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 24) & 0xff;
+			version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 8) & 0xff;
 			DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
 				 version_major, version_minor, family_id);
 
@@ -791,17 +793,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD create msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
 	writel(0x0, (void __iomem *)&msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	writel(0x0, &msg[4]);
 	writel(0x0, &msg[5]);
 	writel(0x0, &msg[6]);
-	writel(cpu_to_le32(0x00000780), &msg[7]);
-	writel(cpu_to_le32(0x00000440), &msg[8]);
+	writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
+	writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
 	writel(0x0, &msg[9]);
-	writel(cpu_to_le32(0x01b37000), &msg[10]);
+	writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
 	for (i = 11; i < 1024; ++i)
 		writel(0x0, &msg[i]);
 
@@ -827,9 +829,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD destroy msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
-	writel(cpu_to_le32(0x00000002), &msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	for (i = 4; i < 1024; ++i)
 		writel(0x0, &msg[i]);
-- 
2.31.1



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

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

* [PATCH v2 2/2] radeon: use memcpy_to/fromio for UVD fw upload
  2021-06-03  5:35 ` Chen Li
@ 2021-06-04  3:04   ` Chen Li
  -1 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  3:04 UTC (permalink / raw)
  To: Alex Deucher, "Christian König", dri-devel, amd-gfx


I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Signed-off-by: Chen Li <chenli@uniontech.com>
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index 85a1f2c31749..0d6a5cfa2abf 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	if (rdev->uvd.vcpu_bo == NULL)
 		return -EINVAL;
 
-	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
+				rdev->uvd_fw->data,
+				le32_to_cpu((__force __le32)rdev->uvd_fw->size));
 
 	size = radeon_bo_size(rdev->uvd.vcpu_bo);
 	size -= rdev->uvd_fw->size;
@@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	ptr = rdev->uvd.cpu_addr;
 	ptr += rdev->uvd_fw->size;
 
-	memset(ptr, 0, size);
+	memset_io((void __iomem *)ptr, 0, size);
 
 	return 0;
 }
-- 
2.31.1




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

* [PATCH v2 2/2] radeon: use memcpy_to/fromio for UVD fw upload
@ 2021-06-04  3:04   ` Chen Li
  0 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  3:04 UTC (permalink / raw)
  To: Alex Deucher, "Christian König", dri-devel, amd-gfx


I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Signed-off-by: Chen Li <chenli@uniontech.com>
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index 85a1f2c31749..0d6a5cfa2abf 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	if (rdev->uvd.vcpu_bo == NULL)
 		return -EINVAL;
 
-	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
+				rdev->uvd_fw->data,
+				le32_to_cpu((__force __le32)rdev->uvd_fw->size));
 
 	size = radeon_bo_size(rdev->uvd.vcpu_bo);
 	size -= rdev->uvd_fw->size;
@@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	ptr = rdev->uvd.cpu_addr;
 	ptr += rdev->uvd_fw->size;
 
-	memset(ptr, 0, size);
+	memset_io((void __iomem *)ptr, 0, size);
 
 	return 0;
 }
-- 
2.31.1



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

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

* Re: [PATCH v2 1/2] radeon: fix coding issues reported from sparse
  2021-06-04  3:02   ` Chen Li
@ 2021-06-04  7:14     ` Christian König
  -1 siblings, 0 replies; 40+ messages in thread
From: Christian König @ 2021-06-04  7:14 UTC (permalink / raw)
  To: Chen Li, Alex Deucher, Christian König, dri-devel, amd-gfx

Am 04.06.21 um 05:02 schrieb Chen Li:
> Also fix some coding issue reported from sparse.
>
> Signed-off-by: Chen Li <chenli@uniontech.com>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/radeon/radeon_uvd.c | 24 +++++++++++++-----------
>   1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index dfa9fdbe98da..85a1f2c31749 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
>   
>   			rdev->uvd.fw_header_present = true;
>   
> -			family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
> -			version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
> -			version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
> +			family_id = (__force u32)(hdr->ucode_version) & 0xff;
> +			version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
> +							 >> 24) & 0xff;
> +			version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
> +							 >> 8) & 0xff;
>   			DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
>   				 version_major, version_minor, family_id);
>   
> @@ -791,17 +793,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
>   		return r;
>   
>   	/* stitch together an UVD create msg */
> -	writel(cpu_to_le32(0x00000de4), &msg[0]);
> +	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
>   	writel(0x0, (void __iomem *)&msg[1]);
> -	writel(cpu_to_le32(handle), &msg[2]);
> +	writel((__force u32)cpu_to_le32(handle), &msg[2]);
>   	writel(0x0, &msg[3]);
>   	writel(0x0, &msg[4]);
>   	writel(0x0, &msg[5]);
>   	writel(0x0, &msg[6]);
> -	writel(cpu_to_le32(0x00000780), &msg[7]);
> -	writel(cpu_to_le32(0x00000440), &msg[8]);
> +	writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
> +	writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
>   	writel(0x0, &msg[9]);
> -	writel(cpu_to_le32(0x01b37000), &msg[10]);
> +	writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
>   	for (i = 11; i < 1024; ++i)
>   		writel(0x0, &msg[i]);
>   
> @@ -827,9 +829,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
>   		return r;
>   
>   	/* stitch together an UVD destroy msg */
> -	writel(cpu_to_le32(0x00000de4), &msg[0]);
> -	writel(cpu_to_le32(0x00000002), &msg[1]);
> -	writel(cpu_to_le32(handle), &msg[2]);
> +	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
> +	writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
> +	writel((__force u32)cpu_to_le32(handle), &msg[2]);
>   	writel(0x0, &msg[3]);
>   	for (i = 4; i < 1024; ++i)
>   		writel(0x0, &msg[i]);


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

* Re: [PATCH v2 1/2] radeon: fix coding issues reported from sparse
@ 2021-06-04  7:14     ` Christian König
  0 siblings, 0 replies; 40+ messages in thread
From: Christian König @ 2021-06-04  7:14 UTC (permalink / raw)
  To: Chen Li, Alex Deucher, Christian König, dri-devel, amd-gfx

Am 04.06.21 um 05:02 schrieb Chen Li:
> Also fix some coding issue reported from sparse.
>
> Signed-off-by: Chen Li <chenli@uniontech.com>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/radeon/radeon_uvd.c | 24 +++++++++++++-----------
>   1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index dfa9fdbe98da..85a1f2c31749 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
>   
>   			rdev->uvd.fw_header_present = true;
>   
> -			family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
> -			version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
> -			version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
> +			family_id = (__force u32)(hdr->ucode_version) & 0xff;
> +			version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
> +							 >> 24) & 0xff;
> +			version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
> +							 >> 8) & 0xff;
>   			DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
>   				 version_major, version_minor, family_id);
>   
> @@ -791,17 +793,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
>   		return r;
>   
>   	/* stitch together an UVD create msg */
> -	writel(cpu_to_le32(0x00000de4), &msg[0]);
> +	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
>   	writel(0x0, (void __iomem *)&msg[1]);
> -	writel(cpu_to_le32(handle), &msg[2]);
> +	writel((__force u32)cpu_to_le32(handle), &msg[2]);
>   	writel(0x0, &msg[3]);
>   	writel(0x0, &msg[4]);
>   	writel(0x0, &msg[5]);
>   	writel(0x0, &msg[6]);
> -	writel(cpu_to_le32(0x00000780), &msg[7]);
> -	writel(cpu_to_le32(0x00000440), &msg[8]);
> +	writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
> +	writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
>   	writel(0x0, &msg[9]);
> -	writel(cpu_to_le32(0x01b37000), &msg[10]);
> +	writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
>   	for (i = 11; i < 1024; ++i)
>   		writel(0x0, &msg[i]);
>   
> @@ -827,9 +829,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
>   		return r;
>   
>   	/* stitch together an UVD destroy msg */
> -	writel(cpu_to_le32(0x00000de4), &msg[0]);
> -	writel(cpu_to_le32(0x00000002), &msg[1]);
> -	writel(cpu_to_le32(handle), &msg[2]);
> +	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
> +	writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
> +	writel((__force u32)cpu_to_le32(handle), &msg[2]);
>   	writel(0x0, &msg[3]);
>   	for (i = 4; i < 1024; ++i)
>   		writel(0x0, &msg[i]);

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

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

* Re: [PATCH v2 2/2] radeon: use memcpy_to/fromio for UVD fw upload
  2021-06-04  3:04   ` Chen Li
@ 2021-06-04  7:18     ` Christian König
  -1 siblings, 0 replies; 40+ messages in thread
From: Christian König @ 2021-06-04  7:18 UTC (permalink / raw)
  To: Chen Li, Alex Deucher, Christian König, dri-devel, amd-gfx



Am 04.06.21 um 05:04 schrieb Chen Li:
> I met a gpu addr bug recently and the kernel log
> tells me the pc is memcpy/memset and link register is
> radeon_uvd_resume.
>
> As we know, in some architectures, optimized memcpy/memset
> may not work well on device memory. Trival memcpy_toio/memset_io
> can fix this problem.
>
> BTW, amdgpu has already done it in:
> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> that's why it has no this issue on the same gpu and platform.
>
> Signed-off-by: Chen Li <chenli@uniontech.com>
> ---
>   drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index 85a1f2c31749..0d6a5cfa2abf 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	if (rdev->uvd.vcpu_bo == NULL)
>   		return -EINVAL;
>   
> -	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> +	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
> +				rdev->uvd_fw->data,
> +				le32_to_cpu((__force __le32)rdev->uvd_fw->size));

The le32_to_cpu and coding style looks wrong here. The uvd_fw->size is 
always in CPU byte order and is used as such.

And please keep in mind that architectures where memcpy() on MMIO 
doesn't work in the kernel usually doesn't work in userspace as well.

So this is actually now necessary a valid workaround.

Christian.

>   
>   	size = radeon_bo_size(rdev->uvd.vcpu_bo);
>   	size -= rdev->uvd_fw->size;
> @@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	ptr = rdev->uvd.cpu_addr;
>   	ptr += rdev->uvd_fw->size;
>   
> -	memset(ptr, 0, size);
> +	memset_io((void __iomem *)ptr, 0, size);
>   
>   	return 0;
>   }


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

* Re: [PATCH v2 2/2] radeon: use memcpy_to/fromio for UVD fw upload
@ 2021-06-04  7:18     ` Christian König
  0 siblings, 0 replies; 40+ messages in thread
From: Christian König @ 2021-06-04  7:18 UTC (permalink / raw)
  To: Chen Li, Alex Deucher, Christian König, dri-devel, amd-gfx



Am 04.06.21 um 05:04 schrieb Chen Li:
> I met a gpu addr bug recently and the kernel log
> tells me the pc is memcpy/memset and link register is
> radeon_uvd_resume.
>
> As we know, in some architectures, optimized memcpy/memset
> may not work well on device memory. Trival memcpy_toio/memset_io
> can fix this problem.
>
> BTW, amdgpu has already done it in:
> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> that's why it has no this issue on the same gpu and platform.
>
> Signed-off-by: Chen Li <chenli@uniontech.com>
> ---
>   drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index 85a1f2c31749..0d6a5cfa2abf 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	if (rdev->uvd.vcpu_bo == NULL)
>   		return -EINVAL;
>   
> -	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> +	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
> +				rdev->uvd_fw->data,
> +				le32_to_cpu((__force __le32)rdev->uvd_fw->size));

The le32_to_cpu and coding style looks wrong here. The uvd_fw->size is 
always in CPU byte order and is used as such.

And please keep in mind that architectures where memcpy() on MMIO 
doesn't work in the kernel usually doesn't work in userspace as well.

So this is actually now necessary a valid workaround.

Christian.

>   
>   	size = radeon_bo_size(rdev->uvd.vcpu_bo);
>   	size -= rdev->uvd_fw->size;
> @@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	ptr = rdev->uvd.cpu_addr;
>   	ptr += rdev->uvd_fw->size;
>   
> -	memset(ptr, 0, size);
> +	memset_io((void __iomem *)ptr, 0, size);
>   
>   	return 0;
>   }

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

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

* [PATCH v3 0/2] use memcpy_to/fromio for UVD fw upload
  2021-06-04  7:18     ` Christian König
@ 2021-06-04  7:50       ` Chen Li
  -1 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  7:50 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König



changelog:
    v1->v2: split sparse and memcp/memset fix
    v2->v3: fix coding issue and misuse of le32_to_cpu

Chen Li (2):
  radeon: fix coding issues reported from sparse
  radeon: use memcpy_to/fromio for UVD fw upload

 drivers/gpu/drm/radeon/radeon_uvd.c | 30 ++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

-- 
2.31.1




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

* [PATCH v3 0/2] use memcpy_to/fromio for UVD fw upload
@ 2021-06-04  7:50       ` Chen Li
  0 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  7:50 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König



changelog:
    v1->v2: split sparse and memcp/memset fix
    v2->v3: fix coding issue and misuse of le32_to_cpu

Chen Li (2):
  radeon: fix coding issues reported from sparse
  radeon: use memcpy_to/fromio for UVD fw upload

 drivers/gpu/drm/radeon/radeon_uvd.c | 30 ++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

-- 
2.31.1



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

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

* [PATCH v3 1/2] radeon: fix coding issues reported from sparse
  2021-06-04  7:18     ` Christian König
@ 2021-06-04  7:52       ` Chen Li
  -1 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  7:52 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König


Also fix some coding issues reported from sparse.

Signed-off-by: Chen Li <chenli@uniontech.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index dfa9fdbe98da..85a1f2c31749 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
 
 			rdev->uvd.fw_header_present = true;
 
-			family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
-			version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
-			version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
+			family_id = (__force u32)(hdr->ucode_version) & 0xff;
+			version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 24) & 0xff;
+			version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 8) & 0xff;
 			DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
 				 version_major, version_minor, family_id);
 
@@ -791,17 +793,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD create msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
 	writel(0x0, (void __iomem *)&msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	writel(0x0, &msg[4]);
 	writel(0x0, &msg[5]);
 	writel(0x0, &msg[6]);
-	writel(cpu_to_le32(0x00000780), &msg[7]);
-	writel(cpu_to_le32(0x00000440), &msg[8]);
+	writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
+	writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
 	writel(0x0, &msg[9]);
-	writel(cpu_to_le32(0x01b37000), &msg[10]);
+	writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
 	for (i = 11; i < 1024; ++i)
 		writel(0x0, &msg[i]);
 
@@ -827,9 +829,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD destroy msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
-	writel(cpu_to_le32(0x00000002), &msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	for (i = 4; i < 1024; ++i)
 		writel(0x0, &msg[i]);
-- 
2.31.1




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

* [PATCH v3 1/2] radeon: fix coding issues reported from sparse
@ 2021-06-04  7:52       ` Chen Li
  0 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  7:52 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König


Also fix some coding issues reported from sparse.

Signed-off-by: Chen Li <chenli@uniontech.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index dfa9fdbe98da..85a1f2c31749 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
 
 			rdev->uvd.fw_header_present = true;
 
-			family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
-			version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
-			version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
+			family_id = (__force u32)(hdr->ucode_version) & 0xff;
+			version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 24) & 0xff;
+			version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 8) & 0xff;
 			DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
 				 version_major, version_minor, family_id);
 
@@ -791,17 +793,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD create msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
 	writel(0x0, (void __iomem *)&msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	writel(0x0, &msg[4]);
 	writel(0x0, &msg[5]);
 	writel(0x0, &msg[6]);
-	writel(cpu_to_le32(0x00000780), &msg[7]);
-	writel(cpu_to_le32(0x00000440), &msg[8]);
+	writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
+	writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
 	writel(0x0, &msg[9]);
-	writel(cpu_to_le32(0x01b37000), &msg[10]);
+	writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
 	for (i = 11; i < 1024; ++i)
 		writel(0x0, &msg[i]);
 
@@ -827,9 +829,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD destroy msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
-	writel(cpu_to_le32(0x00000002), &msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	for (i = 4; i < 1024; ++i)
 		writel(0x0, &msg[i]);
-- 
2.31.1



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

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

* [PATCH v3 2/2] radeon: use memcpy_to/fromio for UVD fw upload
  2021-06-04  7:18     ` Christian König
@ 2021-06-04  7:53       ` Chen Li
  -1 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  7:53 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König


I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Signed-off-by: Chen Li <chenli@uniontech.com>
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index 85a1f2c31749..55abf9a9623b 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	if (rdev->uvd.vcpu_bo == NULL)
 		return -EINVAL;
 
-	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
+				rdev->uvd_fw->data,
+				rdev->uvd_fw->size);
 
 	size = radeon_bo_size(rdev->uvd.vcpu_bo);
 	size -= rdev->uvd_fw->size;
@@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	ptr = rdev->uvd.cpu_addr;
 	ptr += rdev->uvd_fw->size;
 
-	memset(ptr, 0, size);
+	memset_io((void __iomem *)ptr, 0, size);
 
 	return 0;
 }
-- 
2.31.1




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

* [PATCH v3 2/2] radeon: use memcpy_to/fromio for UVD fw upload
@ 2021-06-04  7:53       ` Chen Li
  0 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  7:53 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König


I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Signed-off-by: Chen Li <chenli@uniontech.com>
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index 85a1f2c31749..55abf9a9623b 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	if (rdev->uvd.vcpu_bo == NULL)
 		return -EINVAL;
 
-	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
+				rdev->uvd_fw->data,
+				rdev->uvd_fw->size);
 
 	size = radeon_bo_size(rdev->uvd.vcpu_bo);
 	size -= rdev->uvd_fw->size;
@@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	ptr = rdev->uvd.cpu_addr;
 	ptr += rdev->uvd_fw->size;
 
-	memset(ptr, 0, size);
+	memset_io((void __iomem *)ptr, 0, size);
 
 	return 0;
 }
-- 
2.31.1



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

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

* Re: [PATCH v3 2/2] radeon: use memcpy_to/fromio for UVD fw upload
  2021-06-04  7:53       ` Chen Li
@ 2021-06-04  8:08         ` Christian König
  -1 siblings, 0 replies; 40+ messages in thread
From: Christian König @ 2021-06-04  8:08 UTC (permalink / raw)
  To: Chen Li; +Cc: Alex Deucher, amd-gfx, Christian König, dri-devel



Am 04.06.21 um 09:53 schrieb Chen Li:
> I met a gpu addr bug recently and the kernel log
> tells me the pc is memcpy/memset and link register is
> radeon_uvd_resume.
>
> As we know, in some architectures, optimized memcpy/memset
> may not work well on device memory. Trival memcpy_toio/memset_io
> can fix this problem.
>
> BTW, amdgpu has already done it in:
> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> that's why it has no this issue on the same gpu and platform.
>
> Signed-off-by: Chen Li <chenli@uniontech.com>
> ---
>   drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index 85a1f2c31749..55abf9a9623b 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	if (rdev->uvd.vcpu_bo == NULL)
>   		return -EINVAL;
>   
> -	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> +	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
> +				rdev->uvd_fw->data,
> +				rdev->uvd_fw->size);

The coding style still looks wrong here, e.g. it is indented to far to 
the right and data/size can be on one line.

Apart from that the patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>

Regards,
Christian.

>   
>   	size = radeon_bo_size(rdev->uvd.vcpu_bo);
>   	size -= rdev->uvd_fw->size;
> @@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	ptr = rdev->uvd.cpu_addr;
>   	ptr += rdev->uvd_fw->size;
>   
> -	memset(ptr, 0, size);
> +	memset_io((void __iomem *)ptr, 0, size);
>   
>   	return 0;
>   }


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

* Re: [PATCH v3 2/2] radeon: use memcpy_to/fromio for UVD fw upload
@ 2021-06-04  8:08         ` Christian König
  0 siblings, 0 replies; 40+ messages in thread
From: Christian König @ 2021-06-04  8:08 UTC (permalink / raw)
  To: Chen Li; +Cc: Alex Deucher, amd-gfx, Christian König, dri-devel



Am 04.06.21 um 09:53 schrieb Chen Li:
> I met a gpu addr bug recently and the kernel log
> tells me the pc is memcpy/memset and link register is
> radeon_uvd_resume.
>
> As we know, in some architectures, optimized memcpy/memset
> may not work well on device memory. Trival memcpy_toio/memset_io
> can fix this problem.
>
> BTW, amdgpu has already done it in:
> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> that's why it has no this issue on the same gpu and platform.
>
> Signed-off-by: Chen Li <chenli@uniontech.com>
> ---
>   drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index 85a1f2c31749..55abf9a9623b 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	if (rdev->uvd.vcpu_bo == NULL)
>   		return -EINVAL;
>   
> -	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> +	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
> +				rdev->uvd_fw->data,
> +				rdev->uvd_fw->size);

The coding style still looks wrong here, e.g. it is indented to far to 
the right and data/size can be on one line.

Apart from that the patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>

Regards,
Christian.

>   
>   	size = radeon_bo_size(rdev->uvd.vcpu_bo);
>   	size -= rdev->uvd_fw->size;
> @@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	ptr = rdev->uvd.cpu_addr;
>   	ptr += rdev->uvd_fw->size;
>   
> -	memset(ptr, 0, size);
> +	memset_io((void __iomem *)ptr, 0, size);
>   
>   	return 0;
>   }

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

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

* Re: [PATCH v3 2/2] radeon: use memcpy_to/fromio for UVD fw upload
  2021-06-04  8:08         ` Christian König
@ 2021-06-04  8:28           ` Chen Li
  -1 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  8:28 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König

On Fri, 04 Jun 2021 16:08:26 +0800,
Christian König wrote:
> 
> 
> 
> Am 04.06.21 um 09:53 schrieb Chen Li:
> > I met a gpu addr bug recently and the kernel log
> > tells me the pc is memcpy/memset and link register is
> > radeon_uvd_resume.
> > 
> > As we know, in some architectures, optimized memcpy/memset
> > may not work well on device memory. Trival memcpy_toio/memset_io
> > can fix this problem.
> > 
> > BTW, amdgpu has already done it in:
> > commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> > that's why it has no this issue on the same gpu and platform.
> > 
> > Signed-off-by: Chen Li <chenli@uniontech.com>
> > ---
> >   drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> > index 85a1f2c31749..55abf9a9623b 100644
> > --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> > +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> > @@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
> >   	if (rdev->uvd.vcpu_bo == NULL)
> >   		return -EINVAL;
> >   -	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> > +	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
> > +				rdev->uvd_fw->data,
> > +				rdev->uvd_fw->size);
> 
> The coding style still looks wrong here, e.g. it is indented to far to the right
> and data/size can be on one line.

It's really werid that the patch before being replyed has not this coding style issue and do always indent the same with previous memcpy(in all of v1, v2 and v3),
you can check at https://patchwork.kernel.org/project/dri-devel/patch/87im2ufhyz.wl-chenli@uniontech.com/ Cannot figure out what happened, sorry.

I'll take merge them in single line in the next series, thanks.
> 
> Apart from that the patch is Reviewed-by: Christian König
> <christian.koenig@amd.com>
> 
> Regards,
> Christian.
> 
> >     	size = radeon_bo_size(rdev->uvd.vcpu_bo);
> >   	size -= rdev->uvd_fw->size;
> > @@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
> >   	ptr = rdev->uvd.cpu_addr;
> >   	ptr += rdev->uvd_fw->size;
> >   -	memset(ptr, 0, size);
> > +	memset_io((void __iomem *)ptr, 0, size);
> >     	return 0;
> >   }
> 
> 
> 



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

* Re: [PATCH v3 2/2] radeon: use memcpy_to/fromio for UVD fw upload
@ 2021-06-04  8:28           ` Chen Li
  0 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  8:28 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König

On Fri, 04 Jun 2021 16:08:26 +0800,
Christian König wrote:
> 
> 
> 
> Am 04.06.21 um 09:53 schrieb Chen Li:
> > I met a gpu addr bug recently and the kernel log
> > tells me the pc is memcpy/memset and link register is
> > radeon_uvd_resume.
> > 
> > As we know, in some architectures, optimized memcpy/memset
> > may not work well on device memory. Trival memcpy_toio/memset_io
> > can fix this problem.
> > 
> > BTW, amdgpu has already done it in:
> > commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> > that's why it has no this issue on the same gpu and platform.
> > 
> > Signed-off-by: Chen Li <chenli@uniontech.com>
> > ---
> >   drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> > index 85a1f2c31749..55abf9a9623b 100644
> > --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> > +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> > @@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
> >   	if (rdev->uvd.vcpu_bo == NULL)
> >   		return -EINVAL;
> >   -	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> > +	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
> > +				rdev->uvd_fw->data,
> > +				rdev->uvd_fw->size);
> 
> The coding style still looks wrong here, e.g. it is indented to far to the right
> and data/size can be on one line.

It's really werid that the patch before being replyed has not this coding style issue and do always indent the same with previous memcpy(in all of v1, v2 and v3),
you can check at https://patchwork.kernel.org/project/dri-devel/patch/87im2ufhyz.wl-chenli@uniontech.com/ Cannot figure out what happened, sorry.

I'll take merge them in single line in the next series, thanks.
> 
> Apart from that the patch is Reviewed-by: Christian König
> <christian.koenig@amd.com>
> 
> Regards,
> Christian.
> 
> >     	size = radeon_bo_size(rdev->uvd.vcpu_bo);
> >   	size -= rdev->uvd_fw->size;
> > @@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
> >   	ptr = rdev->uvd.cpu_addr;
> >   	ptr += rdev->uvd_fw->size;
> >   -	memset(ptr, 0, size);
> > +	memset_io((void __iomem *)ptr, 0, size);
> >     	return 0;
> >   }
> 
> 
> 


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

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

* Re: [PATCH v3 2/2] radeon: use memcpy_to/fromio for UVD fw upload
  2021-06-04  8:28           ` Chen Li
@ 2021-06-04  8:31             ` Christian König
  -1 siblings, 0 replies; 40+ messages in thread
From: Christian König @ 2021-06-04  8:31 UTC (permalink / raw)
  To: Chen Li, Christian König; +Cc: Alex Deucher, amd-gfx, dri-devel



Am 04.06.21 um 10:28 schrieb Chen Li:
> On Fri, 04 Jun 2021 16:08:26 +0800,
> Christian König wrote:
>>
>>
>> Am 04.06.21 um 09:53 schrieb Chen Li:
>>> I met a gpu addr bug recently and the kernel log
>>> tells me the pc is memcpy/memset and link register is
>>> radeon_uvd_resume.
>>>
>>> As we know, in some architectures, optimized memcpy/memset
>>> may not work well on device memory. Trival memcpy_toio/memset_io
>>> can fix this problem.
>>>
>>> BTW, amdgpu has already done it in:
>>> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
>>> that's why it has no this issue on the same gpu and platform.
>>>
>>> Signed-off-by: Chen Li <chenli@uniontech.com>
>>> ---
>>>    drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
>>> index 85a1f2c31749..55abf9a9623b 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
>>> @@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>>>    	if (rdev->uvd.vcpu_bo == NULL)
>>>    		return -EINVAL;
>>>    -	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
>>> +	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
>>> +				rdev->uvd_fw->data,
>>> +				rdev->uvd_fw->size);
>> The coding style still looks wrong here, e.g. it is indented to far to the right
>> and data/size can be on one line.
> It's really werid that the patch before being replyed has not this coding style issue and do always indent the same with previous memcpy(in all of v1, v2 and v3),
> you can check at https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fproject%2Fdri-devel%2Fpatch%2F87im2ufhyz.wl-chenli%40uniontech.com%2F&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7C3faf061c19b54a68e72508d92732cd5e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637583921450406148%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=b0726ORwyeLQsKVzqjfZEMaU4Vi543szpFYoHekPMIU%3D&amp;reserved=0 Cannot figure out what happened, sorry.
>
> I'll take merge them in single line in the next series, thanks.

It's not much of an issue, just make sure that checkpatch.pl doesn't 
complain.

Christian.

>> Apart from that the patch is Reviewed-by: Christian König
>> <christian.koenig@amd.com>
>>
>> Regards,
>> Christian.
>>
>>>      	size = radeon_bo_size(rdev->uvd.vcpu_bo);
>>>    	size -= rdev->uvd_fw->size;
>>> @@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>>>    	ptr = rdev->uvd.cpu_addr;
>>>    	ptr += rdev->uvd_fw->size;
>>>    -	memset(ptr, 0, size);
>>> +	memset_io((void __iomem *)ptr, 0, size);
>>>      	return 0;
>>>    }
>>
>>
>


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

* Re: [PATCH v3 2/2] radeon: use memcpy_to/fromio for UVD fw upload
@ 2021-06-04  8:31             ` Christian König
  0 siblings, 0 replies; 40+ messages in thread
From: Christian König @ 2021-06-04  8:31 UTC (permalink / raw)
  To: Chen Li, Christian König; +Cc: Alex Deucher, amd-gfx, dri-devel



Am 04.06.21 um 10:28 schrieb Chen Li:
> On Fri, 04 Jun 2021 16:08:26 +0800,
> Christian König wrote:
>>
>>
>> Am 04.06.21 um 09:53 schrieb Chen Li:
>>> I met a gpu addr bug recently and the kernel log
>>> tells me the pc is memcpy/memset and link register is
>>> radeon_uvd_resume.
>>>
>>> As we know, in some architectures, optimized memcpy/memset
>>> may not work well on device memory. Trival memcpy_toio/memset_io
>>> can fix this problem.
>>>
>>> BTW, amdgpu has already done it in:
>>> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
>>> that's why it has no this issue on the same gpu and platform.
>>>
>>> Signed-off-by: Chen Li <chenli@uniontech.com>
>>> ---
>>>    drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
>>> index 85a1f2c31749..55abf9a9623b 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
>>> @@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>>>    	if (rdev->uvd.vcpu_bo == NULL)
>>>    		return -EINVAL;
>>>    -	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
>>> +	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
>>> +				rdev->uvd_fw->data,
>>> +				rdev->uvd_fw->size);
>> The coding style still looks wrong here, e.g. it is indented to far to the right
>> and data/size can be on one line.
> It's really werid that the patch before being replyed has not this coding style issue and do always indent the same with previous memcpy(in all of v1, v2 and v3),
> you can check at https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fproject%2Fdri-devel%2Fpatch%2F87im2ufhyz.wl-chenli%40uniontech.com%2F&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7C3faf061c19b54a68e72508d92732cd5e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637583921450406148%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=b0726ORwyeLQsKVzqjfZEMaU4Vi543szpFYoHekPMIU%3D&amp;reserved=0 Cannot figure out what happened, sorry.
>
> I'll take merge them in single line in the next series, thanks.

It's not much of an issue, just make sure that checkpatch.pl doesn't 
complain.

Christian.

>> Apart from that the patch is Reviewed-by: Christian König
>> <christian.koenig@amd.com>
>>
>> Regards,
>> Christian.
>>
>>>      	size = radeon_bo_size(rdev->uvd.vcpu_bo);
>>>    	size -= rdev->uvd_fw->size;
>>> @@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>>>    	ptr = rdev->uvd.cpu_addr;
>>>    	ptr += rdev->uvd_fw->size;
>>>    -	memset(ptr, 0, size);
>>> +	memset_io((void __iomem *)ptr, 0, size);
>>>      	return 0;
>>>    }
>>
>>
>

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

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

* Re: [PATCH v3 2/2] radeon: use memcpy_to/fromio for UVD fw upload
  2021-06-04  8:31             ` Christian König
@ 2021-06-04  8:37               ` Chen Li
  -1 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  8:37 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, Christian König, amd-gfx, Chen Li, dri-devel

On Fri, 04 Jun 2021 16:31:28 +0800,
Christian König wrote:
> 
> 
> 
> Am 04.06.21 um 10:28 schrieb Chen Li:
> > On Fri, 04 Jun 2021 16:08:26 +0800,
> > Christian König wrote:
> >> 
> >> 
> >> Am 04.06.21 um 09:53 schrieb Chen Li:
> >>> I met a gpu addr bug recently and the kernel log
> >>> tells me the pc is memcpy/memset and link register is
> >>> radeon_uvd_resume.
> >>> 
> >>> As we know, in some architectures, optimized memcpy/memset
> >>> may not work well on device memory. Trival memcpy_toio/memset_io
> >>> can fix this problem.
> >>> 
> >>> BTW, amdgpu has already done it in:
> >>> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> >>> that's why it has no this issue on the same gpu and platform.
> >>> 
> >>> Signed-off-by: Chen Li <chenli@uniontech.com>
> >>> ---
> >>>    drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
> >>>    1 file changed, 4 insertions(+), 2 deletions(-)
> >>> 
> >>> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> >>> index 85a1f2c31749..55abf9a9623b 100644
> >>> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> >>> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> >>> @@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
> >>>    	if (rdev->uvd.vcpu_bo == NULL)
> >>>    		return -EINVAL;
> >>>    -	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> >>> +	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
> >>> +				rdev->uvd_fw->data,
> >>> +				rdev->uvd_fw->size);
> >> The coding style still looks wrong here, e.g. it is indented to far to the right
> >> and data/size can be on one line.
> > It's really werid that the patch before being replyed has not this coding style issue and do always indent the same with previous memcpy(in all of v1, v2 and v3),
> > you can check at https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fproject%2Fdri-devel%2Fpatch%2F87im2ufhyz.wl-chenli%40uniontech.com%2F&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7C3faf061c19b54a68e72508d92732cd5e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637583921450406148%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=b0726ORwyeLQsKVzqjfZEMaU4Vi543szpFYoHekPMIU%3D&amp;reserved=0 Cannot figure out what happened, sorry.
> > 
> > I'll take merge them in single line in the next series, thanks.
> 
> It's not much of an issue, just make sure that checkpatch.pl doesn't complain.

Yes, have already done checkpatch in all these series.
> 
> Christian.
> 
> >> Apart from that the patch is Reviewed-by: Christian König
> >> <christian.koenig@amd.com>
> >> 
> >> Regards,
> >> Christian.
> >> 
> >>>      	size = radeon_bo_size(rdev->uvd.vcpu_bo);
> >>>    	size -= rdev->uvd_fw->size;
> >>> @@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
> >>>    	ptr = rdev->uvd.cpu_addr;
> >>>    	ptr += rdev->uvd_fw->size;
> >>>    -	memset(ptr, 0, size);
> >>> +	memset_io((void __iomem *)ptr, 0, size);
> >>>      	return 0;
> >>>    }
> >> 
> >> 
> > 
> 
> 
> 



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

* Re: [PATCH v3 2/2] radeon: use memcpy_to/fromio for UVD fw upload
@ 2021-06-04  8:37               ` Chen Li
  0 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  8:37 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, Christian König, amd-gfx, Chen Li, dri-devel

On Fri, 04 Jun 2021 16:31:28 +0800,
Christian König wrote:
> 
> 
> 
> Am 04.06.21 um 10:28 schrieb Chen Li:
> > On Fri, 04 Jun 2021 16:08:26 +0800,
> > Christian König wrote:
> >> 
> >> 
> >> Am 04.06.21 um 09:53 schrieb Chen Li:
> >>> I met a gpu addr bug recently and the kernel log
> >>> tells me the pc is memcpy/memset and link register is
> >>> radeon_uvd_resume.
> >>> 
> >>> As we know, in some architectures, optimized memcpy/memset
> >>> may not work well on device memory. Trival memcpy_toio/memset_io
> >>> can fix this problem.
> >>> 
> >>> BTW, amdgpu has already done it in:
> >>> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> >>> that's why it has no this issue on the same gpu and platform.
> >>> 
> >>> Signed-off-by: Chen Li <chenli@uniontech.com>
> >>> ---
> >>>    drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
> >>>    1 file changed, 4 insertions(+), 2 deletions(-)
> >>> 
> >>> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> >>> index 85a1f2c31749..55abf9a9623b 100644
> >>> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> >>> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> >>> @@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
> >>>    	if (rdev->uvd.vcpu_bo == NULL)
> >>>    		return -EINVAL;
> >>>    -	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> >>> +	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
> >>> +				rdev->uvd_fw->data,
> >>> +				rdev->uvd_fw->size);
> >> The coding style still looks wrong here, e.g. it is indented to far to the right
> >> and data/size can be on one line.
> > It's really werid that the patch before being replyed has not this coding style issue and do always indent the same with previous memcpy(in all of v1, v2 and v3),
> > you can check at https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fproject%2Fdri-devel%2Fpatch%2F87im2ufhyz.wl-chenli%40uniontech.com%2F&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7C3faf061c19b54a68e72508d92732cd5e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637583921450406148%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=b0726ORwyeLQsKVzqjfZEMaU4Vi543szpFYoHekPMIU%3D&amp;reserved=0 Cannot figure out what happened, sorry.
> > 
> > I'll take merge them in single line in the next series, thanks.
> 
> It's not much of an issue, just make sure that checkpatch.pl doesn't complain.

Yes, have already done checkpatch in all these series.
> 
> Christian.
> 
> >> Apart from that the patch is Reviewed-by: Christian König
> >> <christian.koenig@amd.com>
> >> 
> >> Regards,
> >> Christian.
> >> 
> >>>      	size = radeon_bo_size(rdev->uvd.vcpu_bo);
> >>>    	size -= rdev->uvd_fw->size;
> >>> @@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
> >>>    	ptr = rdev->uvd.cpu_addr;
> >>>    	ptr += rdev->uvd_fw->size;
> >>>    -	memset(ptr, 0, size);
> >>> +	memset_io((void __iomem *)ptr, 0, size);
> >>>      	return 0;
> >>>    }
> >> 
> >> 
> > 
> 
> 
> 


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

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

* [PATCH v4 0/2] use memcpy_to/fromio for UVD fw upload
  2021-06-04  8:08         ` Christian König
@ 2021-06-04  8:38           ` Chen Li
  -1 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  8:38 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König


changelog:
    v1->v2: split sparse and memcp/memset fix
    v2->v3: fix coding issue and misuse of le32_to_cpu
    v3->v4: merge memcpy_toio's arguments to one line
Chen Li (2):
  radeon: fix coding issues reported from sparse
  radeon: use memcpy_to/fromio for UVD fw upload

 drivers/gpu/drm/radeon/radeon_uvd.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

-- 
2.31.1




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

* [PATCH v4 0/2] use memcpy_to/fromio for UVD fw upload
@ 2021-06-04  8:38           ` Chen Li
  0 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  8:38 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König


changelog:
    v1->v2: split sparse and memcp/memset fix
    v2->v3: fix coding issue and misuse of le32_to_cpu
    v3->v4: merge memcpy_toio's arguments to one line
Chen Li (2):
  radeon: fix coding issues reported from sparse
  radeon: use memcpy_to/fromio for UVD fw upload

 drivers/gpu/drm/radeon/radeon_uvd.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

-- 
2.31.1



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

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

* [PATCH v4 1/2] radeon: fix coding issues reported from sparse
  2021-06-04  8:08         ` Christian König
@ 2021-06-04  8:40           ` Chen Li
  -1 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  8:40 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König


Also fix some coding issues reported from sparse.

Signed-off-by: Chen Li <chenli@uniontech.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index dfa9fdbe98da..85a1f2c31749 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
 
 			rdev->uvd.fw_header_present = true;
 
-			family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
-			version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
-			version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
+			family_id = (__force u32)(hdr->ucode_version) & 0xff;
+			version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 24) & 0xff;
+			version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 8) & 0xff;
 			DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
 				 version_major, version_minor, family_id);
 
@@ -791,17 +793,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD create msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
 	writel(0x0, (void __iomem *)&msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	writel(0x0, &msg[4]);
 	writel(0x0, &msg[5]);
 	writel(0x0, &msg[6]);
-	writel(cpu_to_le32(0x00000780), &msg[7]);
-	writel(cpu_to_le32(0x00000440), &msg[8]);
+	writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
+	writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
 	writel(0x0, &msg[9]);
-	writel(cpu_to_le32(0x01b37000), &msg[10]);
+	writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
 	for (i = 11; i < 1024; ++i)
 		writel(0x0, &msg[i]);
 
@@ -827,9 +829,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD destroy msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
-	writel(cpu_to_le32(0x00000002), &msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	for (i = 4; i < 1024; ++i)
 		writel(0x0, &msg[i]);
-- 
2.31.1






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

* [PATCH v4 1/2] radeon: fix coding issues reported from sparse
@ 2021-06-04  8:40           ` Chen Li
  0 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  8:40 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König


Also fix some coding issues reported from sparse.

Signed-off-by: Chen Li <chenli@uniontech.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index dfa9fdbe98da..85a1f2c31749 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
 
 			rdev->uvd.fw_header_present = true;
 
-			family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
-			version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
-			version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
+			family_id = (__force u32)(hdr->ucode_version) & 0xff;
+			version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 24) & 0xff;
+			version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
+							 >> 8) & 0xff;
 			DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
 				 version_major, version_minor, family_id);
 
@@ -791,17 +793,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD create msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
 	writel(0x0, (void __iomem *)&msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	writel(0x0, &msg[4]);
 	writel(0x0, &msg[5]);
 	writel(0x0, &msg[6]);
-	writel(cpu_to_le32(0x00000780), &msg[7]);
-	writel(cpu_to_le32(0x00000440), &msg[8]);
+	writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
+	writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
 	writel(0x0, &msg[9]);
-	writel(cpu_to_le32(0x01b37000), &msg[10]);
+	writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
 	for (i = 11; i < 1024; ++i)
 		writel(0x0, &msg[i]);
 
@@ -827,9 +829,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
 		return r;
 
 	/* stitch together an UVD destroy msg */
-	writel(cpu_to_le32(0x00000de4), &msg[0]);
-	writel(cpu_to_le32(0x00000002), &msg[1]);
-	writel(cpu_to_le32(handle), &msg[2]);
+	writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
+	writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
+	writel((__force u32)cpu_to_le32(handle), &msg[2]);
 	writel(0x0, &msg[3]);
 	for (i = 4; i < 1024; ++i)
 		writel(0x0, &msg[i]);
-- 
2.31.1





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

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

* [PATCH v4 2/2] radeon: use memcpy_to/fromio for UVD fw upload
  2021-06-04  8:08         ` Christian König
@ 2021-06-04  8:43           ` Chen Li
  -1 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  8:43 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König


I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Signed-off-by: Chen Li <chenli@uniontech.com>
Reviewed-by: Christian König
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index 85a1f2c31749..753da95e6abb 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -288,7 +288,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	if (rdev->uvd.vcpu_bo == NULL)
 		return -EINVAL;
 
-	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
 
 	size = radeon_bo_size(rdev->uvd.vcpu_bo);
 	size -= rdev->uvd_fw->size;
@@ -296,7 +296,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	ptr = rdev->uvd.cpu_addr;
 	ptr += rdev->uvd_fw->size;
 
-	memset(ptr, 0, size);
+	memset_io((void __iomem *)ptr, 0, size);
 
 	return 0;
 }
-- 
2.31.1





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

* [PATCH v4 2/2] radeon: use memcpy_to/fromio for UVD fw upload
@ 2021-06-04  8:43           ` Chen Li
  0 siblings, 0 replies; 40+ messages in thread
From: Chen Li @ 2021-06-04  8:43 UTC (permalink / raw)
  To: Christian König
  Cc: Alex Deucher, amd-gfx, Chen Li, dri-devel, Christian König


I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Signed-off-by: Chen Li <chenli@uniontech.com>
Reviewed-by: Christian König
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index 85a1f2c31749..753da95e6abb 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -288,7 +288,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	if (rdev->uvd.vcpu_bo == NULL)
 		return -EINVAL;
 
-	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
 
 	size = radeon_bo_size(rdev->uvd.vcpu_bo);
 	size -= rdev->uvd_fw->size;
@@ -296,7 +296,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
 	ptr = rdev->uvd.cpu_addr;
 	ptr += rdev->uvd_fw->size;
 
-	memset(ptr, 0, size);
+	memset_io((void __iomem *)ptr, 0, size);
 
 	return 0;
 }
-- 
2.31.1




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

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

* Re: [PATCH v4 2/2] radeon: use memcpy_to/fromio for UVD fw upload
  2021-06-04  8:43           ` Chen Li
@ 2021-06-04  8:47             ` Christian König
  -1 siblings, 0 replies; 40+ messages in thread
From: Christian König @ 2021-06-04  8:47 UTC (permalink / raw)
  To: Chen Li, Christian König; +Cc: Alex Deucher, amd-gfx, dri-devel

Am 04.06.21 um 10:43 schrieb Chen Li:
> I met a gpu addr bug recently and the kernel log
> tells me the pc is memcpy/memset and link register is
> radeon_uvd_resume.
>
> As we know, in some architectures, optimized memcpy/memset
> may not work well on device memory. Trival memcpy_toio/memset_io
> can fix this problem.
>
> BTW, amdgpu has already done it in:
> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> that's why it has no this issue on the same gpu and platform.
>
> Signed-off-by: Chen Li <chenli@uniontech.com>
> Reviewed-by: Christian König

Reviewed-by: Christian König <christian.koenig@amd.com>

Alex will probably now pick them up for upstreaming.

Christian.

> ---
>   drivers/gpu/drm/radeon/radeon_uvd.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index 85a1f2c31749..753da95e6abb 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -288,7 +288,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	if (rdev->uvd.vcpu_bo == NULL)
>   		return -EINVAL;
>   
> -	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> +	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
>   
>   	size = radeon_bo_size(rdev->uvd.vcpu_bo);
>   	size -= rdev->uvd_fw->size;
> @@ -296,7 +296,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	ptr = rdev->uvd.cpu_addr;
>   	ptr += rdev->uvd_fw->size;
>   
> -	memset(ptr, 0, size);
> +	memset_io((void __iomem *)ptr, 0, size);
>   
>   	return 0;
>   }


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

* Re: [PATCH v4 2/2] radeon: use memcpy_to/fromio for UVD fw upload
@ 2021-06-04  8:47             ` Christian König
  0 siblings, 0 replies; 40+ messages in thread
From: Christian König @ 2021-06-04  8:47 UTC (permalink / raw)
  To: Chen Li, Christian König; +Cc: Alex Deucher, amd-gfx, dri-devel

Am 04.06.21 um 10:43 schrieb Chen Li:
> I met a gpu addr bug recently and the kernel log
> tells me the pc is memcpy/memset and link register is
> radeon_uvd_resume.
>
> As we know, in some architectures, optimized memcpy/memset
> may not work well on device memory. Trival memcpy_toio/memset_io
> can fix this problem.
>
> BTW, amdgpu has already done it in:
> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> that's why it has no this issue on the same gpu and platform.
>
> Signed-off-by: Chen Li <chenli@uniontech.com>
> Reviewed-by: Christian König

Reviewed-by: Christian König <christian.koenig@amd.com>

Alex will probably now pick them up for upstreaming.

Christian.

> ---
>   drivers/gpu/drm/radeon/radeon_uvd.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index 85a1f2c31749..753da95e6abb 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -288,7 +288,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	if (rdev->uvd.vcpu_bo == NULL)
>   		return -EINVAL;
>   
> -	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> +	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
>   
>   	size = radeon_bo_size(rdev->uvd.vcpu_bo);
>   	size -= rdev->uvd_fw->size;
> @@ -296,7 +296,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	ptr = rdev->uvd.cpu_addr;
>   	ptr += rdev->uvd_fw->size;
>   
> -	memset(ptr, 0, size);
> +	memset_io((void __iomem *)ptr, 0, size);
>   
>   	return 0;
>   }

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

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

* Re: [PATCH v4 1/2] radeon: fix coding issues reported from sparse
  2021-06-04  8:40           ` Chen Li
@ 2021-06-04 17:19             ` Alex Deucher
  -1 siblings, 0 replies; 40+ messages in thread
From: Alex Deucher @ 2021-06-04 17:19 UTC (permalink / raw)
  To: Chen Li
  Cc: Alex Deucher, Christian König, Maling list - DRI developers,
	amd-gfx list, Christian König

Applied.  Thanks!

Alex

On Fri, Jun 4, 2021 at 7:53 AM Chen Li <chenli@uniontech.com> wrote:
>
>
> Also fix some coding issues reported from sparse.
>
> Signed-off-by: Chen Li <chenli@uniontech.com>
> Acked-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/radeon/radeon_uvd.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index dfa9fdbe98da..85a1f2c31749 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
>
>                         rdev->uvd.fw_header_present = true;
>
> -                       family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
> -                       version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
> -                       version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
> +                       family_id = (__force u32)(hdr->ucode_version) & 0xff;
> +                       version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
> +                                                        >> 24) & 0xff;
> +                       version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
> +                                                        >> 8) & 0xff;
>                         DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
>                                  version_major, version_minor, family_id);
>
> @@ -791,17 +793,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
>                 return r;
>
>         /* stitch together an UVD create msg */
> -       writel(cpu_to_le32(0x00000de4), &msg[0]);
> +       writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
>         writel(0x0, (void __iomem *)&msg[1]);
> -       writel(cpu_to_le32(handle), &msg[2]);
> +       writel((__force u32)cpu_to_le32(handle), &msg[2]);
>         writel(0x0, &msg[3]);
>         writel(0x0, &msg[4]);
>         writel(0x0, &msg[5]);
>         writel(0x0, &msg[6]);
> -       writel(cpu_to_le32(0x00000780), &msg[7]);
> -       writel(cpu_to_le32(0x00000440), &msg[8]);
> +       writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
> +       writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
>         writel(0x0, &msg[9]);
> -       writel(cpu_to_le32(0x01b37000), &msg[10]);
> +       writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
>         for (i = 11; i < 1024; ++i)
>                 writel(0x0, &msg[i]);
>
> @@ -827,9 +829,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
>                 return r;
>
>         /* stitch together an UVD destroy msg */
> -       writel(cpu_to_le32(0x00000de4), &msg[0]);
> -       writel(cpu_to_le32(0x00000002), &msg[1]);
> -       writel(cpu_to_le32(handle), &msg[2]);
> +       writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
> +       writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
> +       writel((__force u32)cpu_to_le32(handle), &msg[2]);
>         writel(0x0, &msg[3]);
>         for (i = 4; i < 1024; ++i)
>                 writel(0x0, &msg[i]);
> --
> 2.31.1
>
>
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH v4 1/2] radeon: fix coding issues reported from sparse
@ 2021-06-04 17:19             ` Alex Deucher
  0 siblings, 0 replies; 40+ messages in thread
From: Alex Deucher @ 2021-06-04 17:19 UTC (permalink / raw)
  To: Chen Li
  Cc: Alex Deucher, Christian König, Maling list - DRI developers,
	amd-gfx list, Christian König

Applied.  Thanks!

Alex

On Fri, Jun 4, 2021 at 7:53 AM Chen Li <chenli@uniontech.com> wrote:
>
>
> Also fix some coding issues reported from sparse.
>
> Signed-off-by: Chen Li <chenli@uniontech.com>
> Acked-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/radeon/radeon_uvd.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index dfa9fdbe98da..85a1f2c31749 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -152,9 +152,11 @@ int radeon_uvd_init(struct radeon_device *rdev)
>
>                         rdev->uvd.fw_header_present = true;
>
> -                       family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
> -                       version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
> -                       version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
> +                       family_id = (__force u32)(hdr->ucode_version) & 0xff;
> +                       version_major = (le32_to_cpu((__force __le32)(hdr->ucode_version))
> +                                                        >> 24) & 0xff;
> +                       version_minor = (le32_to_cpu((__force __le32)(hdr->ucode_version))
> +                                                        >> 8) & 0xff;
>                         DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
>                                  version_major, version_minor, family_id);
>
> @@ -791,17 +793,17 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
>                 return r;
>
>         /* stitch together an UVD create msg */
> -       writel(cpu_to_le32(0x00000de4), &msg[0]);
> +       writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
>         writel(0x0, (void __iomem *)&msg[1]);
> -       writel(cpu_to_le32(handle), &msg[2]);
> +       writel((__force u32)cpu_to_le32(handle), &msg[2]);
>         writel(0x0, &msg[3]);
>         writel(0x0, &msg[4]);
>         writel(0x0, &msg[5]);
>         writel(0x0, &msg[6]);
> -       writel(cpu_to_le32(0x00000780), &msg[7]);
> -       writel(cpu_to_le32(0x00000440), &msg[8]);
> +       writel((__force u32)cpu_to_le32(0x00000780), &msg[7]);
> +       writel((__force u32)cpu_to_le32(0x00000440), &msg[8]);
>         writel(0x0, &msg[9]);
> -       writel(cpu_to_le32(0x01b37000), &msg[10]);
> +       writel((__force u32)cpu_to_le32(0x01b37000), &msg[10]);
>         for (i = 11; i < 1024; ++i)
>                 writel(0x0, &msg[i]);
>
> @@ -827,9 +829,9 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
>                 return r;
>
>         /* stitch together an UVD destroy msg */
> -       writel(cpu_to_le32(0x00000de4), &msg[0]);
> -       writel(cpu_to_le32(0x00000002), &msg[1]);
> -       writel(cpu_to_le32(handle), &msg[2]);
> +       writel((__force u32)cpu_to_le32(0x00000de4), &msg[0]);
> +       writel((__force u32)cpu_to_le32(0x00000002), &msg[1]);
> +       writel((__force u32)cpu_to_le32(handle), &msg[2]);
>         writel(0x0, &msg[3]);
>         for (i = 4; i < 1024; ++i)
>                 writel(0x0, &msg[i]);
> --
> 2.31.1
>
>
>
>
>
> _______________________________________________
> 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

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

* Re: [PATCH v4 2/2] radeon: use memcpy_to/fromio for UVD fw upload
  2021-06-04  8:43           ` Chen Li
@ 2021-06-04 17:21             ` Alex Deucher
  -1 siblings, 0 replies; 40+ messages in thread
From: Alex Deucher @ 2021-06-04 17:21 UTC (permalink / raw)
  To: Chen Li
  Cc: Alex Deucher, Christian König, Maling list - DRI developers,
	amd-gfx list, Christian König

Applied with the RB fixed.

Thanks!

Alex

On Fri, Jun 4, 2021 at 7:53 AM Chen Li <chenli@uniontech.com> wrote:
>
>
> I met a gpu addr bug recently and the kernel log
> tells me the pc is memcpy/memset and link register is
> radeon_uvd_resume.
>
> As we know, in some architectures, optimized memcpy/memset
> may not work well on device memory. Trival memcpy_toio/memset_io
> can fix this problem.
>
> BTW, amdgpu has already done it in:
> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> that's why it has no this issue on the same gpu and platform.
>
> Signed-off-by: Chen Li <chenli@uniontech.com>
> Reviewed-by: Christian König
> ---
>  drivers/gpu/drm/radeon/radeon_uvd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index 85a1f2c31749..753da95e6abb 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -288,7 +288,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>         if (rdev->uvd.vcpu_bo == NULL)
>                 return -EINVAL;
>
> -       memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> +       memcpy_toio((void __iomem *)rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
>
>         size = radeon_bo_size(rdev->uvd.vcpu_bo);
>         size -= rdev->uvd_fw->size;
> @@ -296,7 +296,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>         ptr = rdev->uvd.cpu_addr;
>         ptr += rdev->uvd_fw->size;
>
> -       memset(ptr, 0, size);
> +       memset_io((void __iomem *)ptr, 0, size);
>
>         return 0;
>  }
> --
> 2.31.1
>
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH v4 2/2] radeon: use memcpy_to/fromio for UVD fw upload
@ 2021-06-04 17:21             ` Alex Deucher
  0 siblings, 0 replies; 40+ messages in thread
From: Alex Deucher @ 2021-06-04 17:21 UTC (permalink / raw)
  To: Chen Li
  Cc: Alex Deucher, Christian König, Maling list - DRI developers,
	amd-gfx list, Christian König

Applied with the RB fixed.

Thanks!

Alex

On Fri, Jun 4, 2021 at 7:53 AM Chen Li <chenli@uniontech.com> wrote:
>
>
> I met a gpu addr bug recently and the kernel log
> tells me the pc is memcpy/memset and link register is
> radeon_uvd_resume.
>
> As we know, in some architectures, optimized memcpy/memset
> may not work well on device memory. Trival memcpy_toio/memset_io
> can fix this problem.
>
> BTW, amdgpu has already done it in:
> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> that's why it has no this issue on the same gpu and platform.
>
> Signed-off-by: Chen Li <chenli@uniontech.com>
> Reviewed-by: Christian König
> ---
>  drivers/gpu/drm/radeon/radeon_uvd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index 85a1f2c31749..753da95e6abb 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -288,7 +288,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>         if (rdev->uvd.vcpu_bo == NULL)
>                 return -EINVAL;
>
> -       memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> +       memcpy_toio((void __iomem *)rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
>
>         size = radeon_bo_size(rdev->uvd.vcpu_bo);
>         size -= rdev->uvd_fw->size;
> @@ -296,7 +296,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>         ptr = rdev->uvd.cpu_addr;
>         ptr += rdev->uvd_fw->size;
>
> -       memset(ptr, 0, size);
> +       memset_io((void __iomem *)ptr, 0, size);
>
>         return 0;
>  }
> --
> 2.31.1
>
>
>
>
> _______________________________________________
> 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

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

end of thread, other threads:[~2021-06-05  6:48 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-03  5:35 [PATCH] radeon: use memcpy_to/fromio for UVD fw upload Chen Li
2021-06-03  5:35 ` Chen Li
2021-06-03 19:21 ` Alex Deucher
2021-06-03 19:21   ` Alex Deucher
2021-06-04  3:00   ` [PATCH v2 0/2] " Chen Li
2021-06-04  3:00     ` Chen Li
2021-06-04  3:02 ` [PATCH v2 1/2] radeon: fix coding issues reported from sparse Chen Li
2021-06-04  3:02   ` Chen Li
2021-06-04  7:14   ` Christian König
2021-06-04  7:14     ` Christian König
2021-06-04  3:04 ` [PATCH v2 2/2] radeon: use memcpy_to/fromio for UVD fw upload Chen Li
2021-06-04  3:04   ` Chen Li
2021-06-04  7:18   ` Christian König
2021-06-04  7:18     ` Christian König
2021-06-04  7:50     ` [PATCH v3 0/2] " Chen Li
2021-06-04  7:50       ` Chen Li
2021-06-04  7:52     ` [PATCH v3 1/2] radeon: fix coding issues reported from sparse Chen Li
2021-06-04  7:52       ` Chen Li
2021-06-04  7:53     ` [PATCH v3 2/2] radeon: use memcpy_to/fromio for UVD fw upload Chen Li
2021-06-04  7:53       ` Chen Li
2021-06-04  8:08       ` Christian König
2021-06-04  8:08         ` Christian König
2021-06-04  8:28         ` Chen Li
2021-06-04  8:28           ` Chen Li
2021-06-04  8:31           ` Christian König
2021-06-04  8:31             ` Christian König
2021-06-04  8:37             ` Chen Li
2021-06-04  8:37               ` Chen Li
2021-06-04  8:38         ` [PATCH v4 0/2] " Chen Li
2021-06-04  8:38           ` Chen Li
2021-06-04  8:40         ` [PATCH v4 1/2] radeon: fix coding issues reported from sparse Chen Li
2021-06-04  8:40           ` Chen Li
2021-06-04 17:19           ` Alex Deucher
2021-06-04 17:19             ` Alex Deucher
2021-06-04  8:43         ` [PATCH v4 2/2] radeon: use memcpy_to/fromio for UVD fw upload Chen Li
2021-06-04  8:43           ` Chen Li
2021-06-04  8:47           ` Christian König
2021-06-04  8:47             ` Christian König
2021-06-04 17:21           ` Alex Deucher
2021-06-04 17:21             ` Alex Deucher

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.