From: "Christian König" <deathsimple@vodafone.de>
To: Oded Gabbay <oded.gabbay@gmail.com>
Cc: stable@vger.kernel.org,
Maling list - DRI developers <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian
Date: Sun, 6 Dec 2015 19:45:11 +0100 [thread overview]
Message-ID: <56648237.2070809@vodafone.de> (raw)
In-Reply-To: <CAFCwf11Lq3jJ_p6a0-6TfQ9aGaVdkq_yaGNkkRbgC+b_Nd7xHQ@mail.gmail.com>
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);
>>> }
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: "Christian König" <deathsimple@vodafone.de>
To: Oded Gabbay <oded.gabbay@gmail.com>
Cc: Maling list - DRI developers <dri-devel@lists.freedesktop.org>,
Alex Deucher <alexdeucher@gmail.com>,
stable@vger.kernel.org
Subject: Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian
Date: Sun, 6 Dec 2015 19:45:11 +0100 [thread overview]
Message-ID: <56648237.2070809@vodafone.de> (raw)
In-Reply-To: <CAFCwf11Lq3jJ_p6a0-6TfQ9aGaVdkq_yaGNkkRbgC+b_Nd7xHQ@mail.gmail.com>
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);
>>> }
>>
next prev parent reply other threads:[~2015-12-06 18:45 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-04 21:09 [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian Oded Gabbay
2015-12-04 21:09 ` Oded Gabbay
2015-12-04 21:09 ` [PATCH 2/3] radeon: Fix VCE ring test for Big-Endian systems Oded Gabbay
2015-12-07 7:51 ` Michel Dänzer
2015-12-07 17:49 ` Oded Gabbay
2015-12-08 3:00 ` Michel Dänzer
2015-12-09 10:20 ` Rafał Miłecki
2015-12-04 21:09 ` [PATCH 3/3] radeon: Fix VCE IB test on " Oded Gabbay
2015-12-05 10:23 ` [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian Christian König
2015-12-06 7:29 ` Oded Gabbay
2015-12-06 7:29 ` Oded Gabbay
2015-12-06 18:45 ` Christian König [this message]
2015-12-06 18:45 ` Christian König
2015-12-06 19:00 ` Oded Gabbay
2015-12-06 19:03 ` Christian König
2015-12-06 19:03 ` Christian König
2015-12-06 19:05 ` Oded Gabbay
2015-12-09 5:25 ` Alex Deucher
2015-12-09 5:25 ` Alex Deucher
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56648237.2070809@vodafone.de \
--to=deathsimple@vodafone.de \
--cc=dri-devel@lists.freedesktop.org \
--cc=oded.gabbay@gmail.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.