All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/radeon: fix atombios on big endian
@ 2017-10-30 10:56 Roman Kapl
       [not found] ` <20171030105613.26465-1-rka-uSbOeAmDUekAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 2+ messages in thread
From: Roman Kapl @ 2017-10-30 10:56 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Roman Kapl

The function for byteswapping the data send to/from atombios was buggy for
num_bytes not divisible by four. The function must be aware of the fact
that after byte-swapping the u32 units, valid bytes might end up after the
num_bytes boundary.

This patch was tested on kernel 3.12 and allowed us to sucesfully use
DisplayPort on and Radeon SI card. Namely it fixed the link training and
EDID readout.

The function is patched both in radeon and amd drivers, since the functions
and the fixes are identical.

Signed-off-by: Roman Kapl <rka@sysgo.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 38 +++++++++++++---------------
 drivers/gpu/drm/radeon/atombios_dp.c         | 38 +++++++++++++---------------
 2 files changed, 36 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
index ce443586a0c7..cc4e18dcd8b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
@@ -1766,34 +1766,32 @@ bool amdgpu_atombios_scratch_need_asic_init(struct amdgpu_device *adev)
 		return true;
 }
 
-/* Atom needs data in little endian format
- * so swap as appropriate when copying data to
- * or from atom. Note that atom operates on
- * dw units.
+/* Atom needs data in little endian format so swap as appropriate when copying
+ * data to or from atom. Note that atom operates on dw units.
+ *
+ * Use to_le=true when sending data to atom and provide at least
+ * ALIGN(num_bytes,4) bytes in the dst buffer.
+ *
+ * Use to_le=false when receiving data from atom and provide ALIGN(num_bytes,4)
+ * byes in the src buffer.
  */
 void amdgpu_atombios_copy_swap(u8 *dst, u8 *src, u8 num_bytes, bool to_le)
 {
 #ifdef __BIG_ENDIAN
-	u8 src_tmp[20], dst_tmp[20]; /* used for byteswapping */
-	u32 *dst32, *src32;
+	u32 src_tmp[5], dst_tmp[5];
 	int i;
+	u8 align_num_bytes = ALIGN(num_bytes, 4);
 
-	memcpy(src_tmp, src, num_bytes);
-	src32 = (u32 *)src_tmp;
-	dst32 = (u32 *)dst_tmp;
 	if (to_le) {
-		for (i = 0; i < ((num_bytes + 3) / 4); i++)
-			dst32[i] = cpu_to_le32(src32[i]);
-		memcpy(dst, dst_tmp, num_bytes);
+		memcpy(src_tmp, src, num_bytes);
+		for (i = 0; i < align_num_bytes / 4; i++)
+			dst_tmp[i] = cpu_to_le32(src_tmp[i]);
+		memcpy(dst, dst_tmp, align_num_bytes);
 	} else {
-		u8 dws = num_bytes & ~3;
-		for (i = 0; i < ((num_bytes + 3) / 4); i++)
-			dst32[i] = le32_to_cpu(src32[i]);
-		memcpy(dst, dst_tmp, dws);
-		if (num_bytes % 4) {
-			for (i = 0; i < (num_bytes % 4); i++)
-				dst[dws+i] = dst_tmp[dws+i];
-		}
+		memcpy(src_tmp, src, align_num_bytes);
+		for (i = 0; i < align_num_bytes / 4; i++)
+			dst_tmp[i] = le32_to_cpu(src_tmp[i]);
+		memcpy(dst, dst_tmp, num_bytes);
 	}
 #else
 	memcpy(dst, src, num_bytes);
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
index 432cb46f6a34..fd7682bf335d 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -45,34 +45,32 @@ static char *pre_emph_names[] = {
 
 /***** radeon AUX functions *****/
 
-/* Atom needs data in little endian format
- * so swap as appropriate when copying data to
- * or from atom. Note that atom operates on
- * dw units.
+/* Atom needs data in little endian format so swap as appropriate when copying
+ * data to or from atom. Note that atom operates on dw units.
+ *
+ * Use to_le=true when sending data to atom and provide at least
+ * ALIGN(num_bytes,4) bytes in the dst buffer.
+ *
+ * Use to_le=false when receiving data from atom and provide ALIGN(num_bytes,4)
+ * byes in the src buffer.
  */
 void radeon_atom_copy_swap(u8 *dst, u8 *src, u8 num_bytes, bool to_le)
 {
 #ifdef __BIG_ENDIAN
-	u8 src_tmp[20], dst_tmp[20]; /* used for byteswapping */
-	u32 *dst32, *src32;
+	u32 src_tmp[5], dst_tmp[5];
 	int i;
+	u8 align_num_bytes = ALIGN(num_bytes, 4);
 
-	memcpy(src_tmp, src, num_bytes);
-	src32 = (u32 *)src_tmp;
-	dst32 = (u32 *)dst_tmp;
 	if (to_le) {
-		for (i = 0; i < ((num_bytes + 3) / 4); i++)
-			dst32[i] = cpu_to_le32(src32[i]);
-		memcpy(dst, dst_tmp, num_bytes);
+		memcpy(src_tmp, src, num_bytes);
+		for (i = 0; i < align_num_bytes / 4; i++)
+			dst_tmp[i] = cpu_to_le32(src_tmp[i]);
+		memcpy(dst, dst_tmp, align_num_bytes);
 	} else {
-		u8 dws = num_bytes & ~3;
-		for (i = 0; i < ((num_bytes + 3) / 4); i++)
-			dst32[i] = le32_to_cpu(src32[i]);
-		memcpy(dst, dst_tmp, dws);
-		if (num_bytes % 4) {
-			for (i = 0; i < (num_bytes % 4); i++)
-				dst[dws+i] = dst_tmp[dws+i];
-		}
+		memcpy(src_tmp, src, align_num_bytes);
+		for (i = 0; i < align_num_bytes / 4; i++)
+			dst_tmp[i] = le32_to_cpu(src_tmp[i]);
+		memcpy(dst, dst_tmp, num_bytes);
 	}
 #else
 	memcpy(dst, src, num_bytes);
-- 
2.11.0

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

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

* Re: [PATCH] drm/radeon: fix atombios on big endian
       [not found] ` <20171030105613.26465-1-rka-uSbOeAmDUekAvxtiuMwx3w@public.gmane.org>
@ 2017-10-31 21:40   ` Alex Deucher
  0 siblings, 0 replies; 2+ messages in thread
From: Alex Deucher @ 2017-10-31 21:40 UTC (permalink / raw)
  To: Roman Kapl; +Cc: amd-gfx list

On Mon, Oct 30, 2017 at 6:56 AM, Roman Kapl <rka@sysgo.com> wrote:
> The function for byteswapping the data send to/from atombios was buggy for
> num_bytes not divisible by four. The function must be aware of the fact
> that after byte-swapping the u32 units, valid bytes might end up after the
> num_bytes boundary.
>
> This patch was tested on kernel 3.12 and allowed us to sucesfully use
> DisplayPort on and Radeon SI card. Namely it fixed the link training and
> EDID readout.
>
> The function is patched both in radeon and amd drivers, since the functions
> and the fixes are identical.
>
> Signed-off-by: Roman Kapl <rka@sysgo.com>

Applied.  thanks!

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 38 +++++++++++++---------------
>  drivers/gpu/drm/radeon/atombios_dp.c         | 38 +++++++++++++---------------
>  2 files changed, 36 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> index ce443586a0c7..cc4e18dcd8b6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> @@ -1766,34 +1766,32 @@ bool amdgpu_atombios_scratch_need_asic_init(struct amdgpu_device *adev)
>                 return true;
>  }
>
> -/* Atom needs data in little endian format
> - * so swap as appropriate when copying data to
> - * or from atom. Note that atom operates on
> - * dw units.
> +/* Atom needs data in little endian format so swap as appropriate when copying
> + * data to or from atom. Note that atom operates on dw units.
> + *
> + * Use to_le=true when sending data to atom and provide at least
> + * ALIGN(num_bytes,4) bytes in the dst buffer.
> + *
> + * Use to_le=false when receiving data from atom and provide ALIGN(num_bytes,4)
> + * byes in the src buffer.
>   */
>  void amdgpu_atombios_copy_swap(u8 *dst, u8 *src, u8 num_bytes, bool to_le)
>  {
>  #ifdef __BIG_ENDIAN
> -       u8 src_tmp[20], dst_tmp[20]; /* used for byteswapping */
> -       u32 *dst32, *src32;
> +       u32 src_tmp[5], dst_tmp[5];
>         int i;
> +       u8 align_num_bytes = ALIGN(num_bytes, 4);
>
> -       memcpy(src_tmp, src, num_bytes);
> -       src32 = (u32 *)src_tmp;
> -       dst32 = (u32 *)dst_tmp;
>         if (to_le) {
> -               for (i = 0; i < ((num_bytes + 3) / 4); i++)
> -                       dst32[i] = cpu_to_le32(src32[i]);
> -               memcpy(dst, dst_tmp, num_bytes);
> +               memcpy(src_tmp, src, num_bytes);
> +               for (i = 0; i < align_num_bytes / 4; i++)
> +                       dst_tmp[i] = cpu_to_le32(src_tmp[i]);
> +               memcpy(dst, dst_tmp, align_num_bytes);
>         } else {
> -               u8 dws = num_bytes & ~3;
> -               for (i = 0; i < ((num_bytes + 3) / 4); i++)
> -                       dst32[i] = le32_to_cpu(src32[i]);
> -               memcpy(dst, dst_tmp, dws);
> -               if (num_bytes % 4) {
> -                       for (i = 0; i < (num_bytes % 4); i++)
> -                               dst[dws+i] = dst_tmp[dws+i];
> -               }
> +               memcpy(src_tmp, src, align_num_bytes);
> +               for (i = 0; i < align_num_bytes / 4; i++)
> +                       dst_tmp[i] = le32_to_cpu(src_tmp[i]);
> +               memcpy(dst, dst_tmp, num_bytes);
>         }
>  #else
>         memcpy(dst, src, num_bytes);
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
> index 432cb46f6a34..fd7682bf335d 100644
> --- a/drivers/gpu/drm/radeon/atombios_dp.c
> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
> @@ -45,34 +45,32 @@ static char *pre_emph_names[] = {
>
>  /***** radeon AUX functions *****/
>
> -/* Atom needs data in little endian format
> - * so swap as appropriate when copying data to
> - * or from atom. Note that atom operates on
> - * dw units.
> +/* Atom needs data in little endian format so swap as appropriate when copying
> + * data to or from atom. Note that atom operates on dw units.
> + *
> + * Use to_le=true when sending data to atom and provide at least
> + * ALIGN(num_bytes,4) bytes in the dst buffer.
> + *
> + * Use to_le=false when receiving data from atom and provide ALIGN(num_bytes,4)
> + * byes in the src buffer.
>   */
>  void radeon_atom_copy_swap(u8 *dst, u8 *src, u8 num_bytes, bool to_le)
>  {
>  #ifdef __BIG_ENDIAN
> -       u8 src_tmp[20], dst_tmp[20]; /* used for byteswapping */
> -       u32 *dst32, *src32;
> +       u32 src_tmp[5], dst_tmp[5];
>         int i;
> +       u8 align_num_bytes = ALIGN(num_bytes, 4);
>
> -       memcpy(src_tmp, src, num_bytes);
> -       src32 = (u32 *)src_tmp;
> -       dst32 = (u32 *)dst_tmp;
>         if (to_le) {
> -               for (i = 0; i < ((num_bytes + 3) / 4); i++)
> -                       dst32[i] = cpu_to_le32(src32[i]);
> -               memcpy(dst, dst_tmp, num_bytes);
> +               memcpy(src_tmp, src, num_bytes);
> +               for (i = 0; i < align_num_bytes / 4; i++)
> +                       dst_tmp[i] = cpu_to_le32(src_tmp[i]);
> +               memcpy(dst, dst_tmp, align_num_bytes);
>         } else {
> -               u8 dws = num_bytes & ~3;
> -               for (i = 0; i < ((num_bytes + 3) / 4); i++)
> -                       dst32[i] = le32_to_cpu(src32[i]);
> -               memcpy(dst, dst_tmp, dws);
> -               if (num_bytes % 4) {
> -                       for (i = 0; i < (num_bytes % 4); i++)
> -                               dst[dws+i] = dst_tmp[dws+i];
> -               }
> +               memcpy(src_tmp, src, align_num_bytes);
> +               for (i = 0; i < align_num_bytes / 4; i++)
> +                       dst_tmp[i] = le32_to_cpu(src_tmp[i]);
> +               memcpy(dst, dst_tmp, num_bytes);
>         }
>  #else
>         memcpy(dst, src, num_bytes);
> --
> 2.11.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

end of thread, other threads:[~2017-10-31 21:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-30 10:56 [PATCH] drm/radeon: fix atombios on big endian Roman Kapl
     [not found] ` <20171030105613.26465-1-rka-uSbOeAmDUekAvxtiuMwx3w@public.gmane.org>
2017-10-31 21:40   ` 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.