Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian

["Christian König" <deathsimple@vodafone.de>]

On 06.12.2015 08:29, Oded Gabbay wrote:
> On Sat, Dec 5, 2015 at 12:23 PM, Christian König
> <deathsimple@vodafone.de> wrote:
>> Patch #1 & #2 are Reviewed-by: Christian König <christian.koenig@amd.com>
>>
>> For patch #3:
>>
>> Couldn't we just in a loop go over all the dw in the IB and swap them after
>> writing them? That would simplify the patch massively.
> I guess we could do that, however I made the fix similar to the same
> code in the uvd module (See radeon_uvd_get_create_msg() for example)
>
>> And line like the one below just look a bit odd to me:
>>>          for (i = ib.length_dw; i < ib_size_dw; ++i)
>>> -               ib.ptr[i] = 0x0;
>>> +               ib.ptr[i] = cpu_to_le32(0x0);
> Same remark as above. I added it in the last second before sending the
> patch just to be similar to uvd code. I guess maybe it is to remind
> people to do it if they ever change that zero value to something else
> ???
>
>> Alternatively a helper function adding DW to an IB with swapping could do it
>> as well.
>>
> Don't you think its an overkill ? It's just a few places in the code.

Yeah, true indeed. Ok then let's just stick with the coding style like 
it is in radeon_uvd_get_create_msg().

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

It sound like you have hardware to test this combination, so could you 
try to get it working on for amdgpu as well?

Shouldn't be to much of a hassle, since amdgpu only supports CIK and VI 
generation for now and I think we never enabled the hardware swapping in 
amdgpu.

Thanks in advance,
Christian.

>
>> Regards,
>> Christian.
>>
>>
>> On 04.12.2015 22:09, Oded Gabbay wrote:
>>> This patch makes the IB test on the GFX ring pass for CI-based cards
>>> installed in Big-Endian machines.
>>>
>>> Signed-off-by: Oded Gabbay <oded.gabbay@gmail.com>
>>> Cc: stable@vger.kernel.org
>>> ---
>>>    drivers/gpu/drm/radeon/cik.c | 6 +-----
>>>    1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
>>> index 248953d..05d43a0 100644
>>> --- a/drivers/gpu/drm/radeon/cik.c
>>> +++ b/drivers/gpu/drm/radeon/cik.c
>>> @@ -4173,11 +4173,7 @@ void cik_ring_ib_execute(struct radeon_device
>>> *rdev, struct radeon_ib *ib)
>>>          control |= ib->length_dw | (vm_id << 24);
>>>          radeon_ring_write(ring, header);
>>> -       radeon_ring_write(ring,
>>> -#ifdef __BIG_ENDIAN
>>> -                         (2 << 0) |
>>> -#endif
>>> -                         (ib->gpu_addr & 0xFFFFFFFC));
>>> +       radeon_ring_write(ring, (ib->gpu_addr & 0xFFFFFFFC));
>>>          radeon_ring_write(ring, upper_32_bits(ib->gpu_addr) & 0xFFFF);
>>>          radeon_ring_write(ring, control);
>>>    }
>>