[RFC 4/4] drm/amdgpu: Document the magic big endian bit

[Alex Deucher]

On Tue, Oct 8, 2024 at 11:10 AM Christian König
<ckoenig.leichtzumerken at gmail.com> wrote:
>
> Am 08.10.24 um 17:05 schrieb Tvrtko Ursulin:
> > From: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
> >
> > Similar to the previous patch but with the addition of a magic bit1 set on
> > big endian platforms. No idea what it is but maybe adding a helper and
> > giving both it and the magic bit a proper name would be worth it.
>
> As far as I know that bit is deprecated and not guaranteed to work any
> more in the CP engine.
>
> I think we should just completely drop trying to use the bigendian
> support like that.

Agreed.  Those are mostly just copy and paste leftovers from when the
driver was forked from radeon.

For reference, back when the hardware supported it, most blocks had a
field that used the following:
ENDIAN_NONE 0
ENDIAN_8IN16 1
ENDIAN_8IN32 2
See the radeon driver for examples.  I think the last generation where
endian swapping was actually validated was probably r5xx.  I think a
few of the fields persisted through r6xx and newer, but they weren't
validated any more by the hw teams.

Alex

>
> Regards,
> Christian.
>
> >
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
> > Cc: Christian König <ckoenig.leichtzumerken at gmail.com>
> > Cc: Sunil Khatri <sunil.khatri at amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 13 +++++++++++++
> >   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c   | 14 ++------------
> >   2 files changed, 15 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > index aeab6fe21db6..68f3799f173c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > @@ -503,6 +503,19 @@ static inline void amdgpu_ring_write_addr(struct amdgpu_ring *ring, u64 addr)
> >       amdgpu_ring_write64(ring, v);
> >   }
> >
> > +static inline void
> > +amdgpu_ring_write_addr_xbe(struct amdgpu_ring *ring, u64 addr)
> > +{
> > +     u64 v;
> > +
> > +#ifdef __BIG_ENDIAN
> > +     v = (u64)lower_32_bits(addr) << 32 | upper_32_bits(addr) | (2 << 0); /* Magic bit name? */
> > +#else
> > +     v = addr;
> > +#endif
> > +     amdgpu_ring_write64(ring, v);
> > +}
> > +
> >   /**
> >    * amdgpu_ring_patch_cond_exec - patch dw count of conditional execute
> >    * @ring: amdgpu_ring structure
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > index 24fb58543041..de1909c1de49 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > @@ -8549,12 +8549,7 @@ static void gfx_v10_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
> >
> >       amdgpu_ring_write(ring, header);
> >       BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
> > -     amdgpu_ring_write(ring,
> > -#ifdef __BIG_ENDIAN
> > -             (2 << 0) |
> > -#endif
> > -             lower_32_bits(ib->gpu_addr));
> > -     amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> > +     amdgpu_ring_write_addr_xbe(ring, ib->gpu_addr);
> >       amdgpu_ring_write(ring, control);
> >   }
> >
> > @@ -8584,12 +8579,7 @@ static void gfx_v10_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
> >
> >       amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> >       BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
> > -     amdgpu_ring_write(ring,
> > -#ifdef __BIG_ENDIAN
> > -                             (2 << 0) |
> > -#endif
> > -                             lower_32_bits(ib->gpu_addr));
> > -     amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> > +     amdgpu_ring_write_addr_xbe(ring, ib->gpu_addr);
> >       amdgpu_ring_write(ring, control);
> >   }
> >
>