[PATCH] drm/radeon: fix atombios on big endian

[Alex Deucher]

On Mon, Oct 30, 2017 at 6:56 AM, Roman Kapl <rka at 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 at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx