From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Christian_K=c3=b6nig?= Subject: Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian Date: Sun, 6 Dec 2015 20:03:35 +0100 Message-ID: <56648687.7040101@vodafone.de> References: <1449263345-4938-1-git-send-email-oded.gabbay@gmail.com> <5662BB2D.3040007@vodafone.de> <56648237.2070809@vodafone.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: Received: from pegasos-out.vodafone.de (pegasos-out.vodafone.de [80.84.1.38]) by gabe.freedesktop.org (Postfix) with ESMTP id 5D0CC6E06D for ; Sun, 6 Dec 2015 11:03:41 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Oded Gabbay Cc: stable@vger.kernel.org, Maling list - DRI developers List-Id: dri-devel@lists.freedesktop.org T24gMDYuMTIuMjAxNSAyMDowMCwgT2RlZCBHYWJiYXkgd3JvdGU6Cj4gT24gU3VuLCBEZWMgNiwg MjAxNSBhdCA4OjQ1IFBNLCBDaHJpc3RpYW4gS8O2bmlnIDxkZWF0aHNpbXBsZUB2b2RhZm9uZS5k ZT4gd3JvdGU6Cj4+IE9uIDA2LjEyLjIwMTUgMDg6MjksIE9kZWQgR2FiYmF5IHdyb3RlOgo+Pj4g T24gU2F0LCBEZWMgNSwgMjAxNSBhdCAxMjoyMyBQTSwgQ2hyaXN0aWFuIEvDtm5pZwo+Pj4gPGRl YXRoc2ltcGxlQHZvZGFmb25lLmRlPiB3cm90ZToKPj4+PiBQYXRjaCAjMSAmICMyIGFyZSBSZXZp ZXdlZC1ieTogQ2hyaXN0aWFuIEvDtm5pZyA8Y2hyaXN0aWFuLmtvZW5pZ0BhbWQuY29tPgo+Pj4+ Cj4+Pj4gRm9yIHBhdGNoICMzOgo+Pj4+Cj4+Pj4gQ291bGRuJ3Qgd2UganVzdCBpbiBhIGxvb3Ag Z28gb3ZlciBhbGwgdGhlIGR3IGluIHRoZSBJQiBhbmQgc3dhcCB0aGVtCj4+Pj4gYWZ0ZXIKPj4+ PiB3cml0aW5nIHRoZW0/IFRoYXQgd291bGQgc2ltcGxpZnkgdGhlIHBhdGNoIG1hc3NpdmVseS4K Pj4+IEkgZ3Vlc3Mgd2UgY291bGQgZG8gdGhhdCwgaG93ZXZlciBJIG1hZGUgdGhlIGZpeCBzaW1p bGFyIHRvIHRoZSBzYW1lCj4+PiBjb2RlIGluIHRoZSB1dmQgbW9kdWxlIChTZWUgcmFkZW9uX3V2 ZF9nZXRfY3JlYXRlX21zZygpIGZvciBleGFtcGxlKQo+Pj4KPj4+PiBBbmQgbGluZSBsaWtlIHRo ZSBvbmUgYmVsb3cganVzdCBsb29rIGEgYml0IG9kZCB0byBtZToKPj4+Pj4gICAgICAgICAgIGZv ciAoaSA9IGliLmxlbmd0aF9kdzsgaSA8IGliX3NpemVfZHc7ICsraSkKPj4+Pj4gLSAgICAgICAg ICAgICAgIGliLnB0cltpXSA9IDB4MDsKPj4+Pj4gKyAgICAgICAgICAgICAgIGliLnB0cltpXSA9 IGNwdV90b19sZTMyKDB4MCk7Cj4+PiBTYW1lIHJlbWFyayBhcyBhYm92ZS4gSSBhZGRlZCBpdCBp biB0aGUgbGFzdCBzZWNvbmQgYmVmb3JlIHNlbmRpbmcgdGhlCj4+PiBwYXRjaCBqdXN0IHRvIGJl IHNpbWlsYXIgdG8gdXZkIGNvZGUuIEkgZ3Vlc3MgbWF5YmUgaXQgaXMgdG8gcmVtaW5kCj4+PiBw ZW9wbGUgdG8gZG8gaXQgaWYgdGhleSBldmVyIGNoYW5nZSB0aGF0IHplcm8gdmFsdWUgdG8gc29t ZXRoaW5nIGVsc2UKPj4+ID8/Pwo+Pj4KPj4+PiBBbHRlcm5hdGl2ZWx5IGEgaGVscGVyIGZ1bmN0 aW9uIGFkZGluZyBEVyB0byBhbiBJQiB3aXRoIHN3YXBwaW5nIGNvdWxkIGRvCj4+Pj4gaXQKPj4+ PiBhcyB3ZWxsLgo+Pj4+Cj4+PiBEb24ndCB5b3UgdGhpbmsgaXRzIGFuIG92ZXJraWxsID8gSXQn cyBqdXN0IGEgZmV3IHBsYWNlcyBpbiB0aGUgY29kZS4KPj4KPj4gWWVhaCwgdHJ1ZSBpbmRlZWQu IE9rIHRoZW4gbGV0J3MganVzdCBzdGljayB3aXRoIHRoZSBjb2Rpbmcgc3R5bGUgbGlrZSBpdCBp cwo+PiBpbiByYWRlb25fdXZkX2dldF9jcmVhdGVfbXNnKCkuCj4+Cj4+IFNvIHRoZSBwYXRjaCBp cyBSZXZpZXdlZC1ieTogQ2hyaXN0aWFuIEvDtm5pZyA8Y2hyaXN0aWFuLmtvZW5pZ0BhbWQuY29t Pi4KPiBUaGFua3MhCj4KPj4gSXQgc291bmQgbGlrZSB5b3UgaGF2ZSBoYXJkd2FyZSB0byB0ZXN0 IHRoaXMgY29tYmluYXRpb24sIHNvIGNvdWxkIHlvdSB0cnkKPj4gdG8gZ2V0IGl0IHdvcmtpbmcg b24gZm9yIGFtZGdwdSBhcyB3ZWxsPwo+IFllcywgSSBoYXZlIGEgQkUgbWFjaGluZSB0aGF0IEkg Y2FuIHRlc3QgYW1kZ3B1IHdpdGguCj4gSG93ZXZlciwgSSBvbmx5IGhhdmUgYSBCb25haXJlIGNh cmQuIElzIGl0IHdvcnRoIGRvaW5nIGl0IGZvciBDSUsgPwo+IEl0J3Mgb25seSBmb3IgZGVidWcg c3VwcG9ydCwgbm8gPwoKWWVhaCwgQ0lLIHN1cHBvcnQgaW4gYW1kZ3B1IHdhcyBvbmx5IGZvciBk ZWJ1Z2dpbmcgYW5kIGJyaW5ndXAuCgpCdXQgc2luY2UgaXQncyBvbmx5IHNoYXJlZCBjb2RlIHlv dSB0b3VjaCB3aGVuIGl0IHdvcmtzIHdpdGggQ0lLIGl0IApzaG91bGQgd29yayB3aXRoIFZJIGFz IHdlbGwgYW5kIHRoYXQncyByYXRoZXIgaW50ZXJlc3RpbmcgdG8gdXMuCgpSZWdhcmRzLApDaHJp c3RpYW4uCgo+Cj4+IFNob3VsZG4ndCBiZSB0byBtdWNoIG9mIGEgaGFzc2xlLCBzaW5jZSBhbWRn cHUgb25seSBzdXBwb3J0cyBDSUsgYW5kIFZJCj4+IGdlbmVyYXRpb24gZm9yIG5vdyBhbmQgSSB0 aGluayB3ZSBuZXZlciBlbmFibGVkIHRoZSBoYXJkd2FyZSBzd2FwcGluZyBpbgo+PiBhbWRncHUu Cj4+Cj4+IFRoYW5rcyBpbiBhZHZhbmNlLAo+PiBDaHJpc3RpYW4uCj4+Cj4+Cj4+Pj4gUmVnYXJk cywKPj4+PiBDaHJpc3RpYW4uCj4+Pj4KPj4+Pgo+Pj4+IE9uIDA0LjEyLjIwMTUgMjI6MDksIE9k ZWQgR2FiYmF5IHdyb3RlOgo+Pj4+PiBUaGlzIHBhdGNoIG1ha2VzIHRoZSBJQiB0ZXN0IG9uIHRo ZSBHRlggcmluZyBwYXNzIGZvciBDSS1iYXNlZCBjYXJkcwo+Pj4+PiBpbnN0YWxsZWQgaW4gQmln LUVuZGlhbiBtYWNoaW5lcy4KPj4+Pj4KPj4+Pj4gU2lnbmVkLW9mZi1ieTogT2RlZCBHYWJiYXkg PG9kZWQuZ2FiYmF5QGdtYWlsLmNvbT4KPj4+Pj4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcK Pj4+Pj4gLS0tCj4+Pj4+ICAgICBkcml2ZXJzL2dwdS9kcm0vcmFkZW9uL2Npay5jIHwgNiArLS0t LS0KPj4+Pj4gICAgIDEgZmlsZSBjaGFuZ2VkLCAxIGluc2VydGlvbigrKSwgNSBkZWxldGlvbnMo LSkKPj4+Pj4KPj4+Pj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9yYWRlb24vY2lrLmMg Yi9kcml2ZXJzL2dwdS9kcm0vcmFkZW9uL2Npay5jCj4+Pj4+IGluZGV4IDI0ODk1M2QuLjA1ZDQz YTAgMTAwNjQ0Cj4+Pj4+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9yYWRlb24vY2lrLmMKPj4+Pj4g KysrIGIvZHJpdmVycy9ncHUvZHJtL3JhZGVvbi9jaWsuYwo+Pj4+PiBAQCAtNDE3MywxMSArNDE3 Myw3IEBAIHZvaWQgY2lrX3JpbmdfaWJfZXhlY3V0ZShzdHJ1Y3QgcmFkZW9uX2RldmljZQo+Pj4+ PiAqcmRldiwgc3RydWN0IHJhZGVvbl9pYiAqaWIpCj4+Pj4+ICAgICAgICAgICBjb250cm9sIHw9 IGliLT5sZW5ndGhfZHcgfCAodm1faWQgPDwgMjQpOwo+Pj4+PiAgICAgICAgICAgcmFkZW9uX3Jp bmdfd3JpdGUocmluZywgaGVhZGVyKTsKPj4+Pj4gLSAgICAgICByYWRlb25fcmluZ193cml0ZShy aW5nLAo+Pj4+PiAtI2lmZGVmIF9fQklHX0VORElBTgo+Pj4+PiAtICAgICAgICAgICAgICAgICAg ICAgICAgICgyIDw8IDApIHwKPj4+Pj4gLSNlbmRpZgo+Pj4+PiAtICAgICAgICAgICAgICAgICAg ICAgICAgIChpYi0+Z3B1X2FkZHIgJiAweEZGRkZGRkZDKSk7Cj4+Pj4+ICsgICAgICAgcmFkZW9u X3Jpbmdfd3JpdGUocmluZywgKGliLT5ncHVfYWRkciAmIDB4RkZGRkZGRkMpKTsKPj4+Pj4gICAg ICAgICAgIHJhZGVvbl9yaW5nX3dyaXRlKHJpbmcsIHVwcGVyXzMyX2JpdHMoaWItPmdwdV9hZGRy KSAmIDB4RkZGRik7Cj4+Pj4+ICAgICAgICAgICByYWRlb25fcmluZ193cml0ZShyaW5nLCBjb250 cm9sKTsKPj4+Pj4gICAgIH0KPj4+PgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJl ZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5m by9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from pegasos-out.vodafone.de ([80.84.1.38]:44341 "EHLO pegasos-out.vodafone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753369AbbLFTDm (ORCPT ); Sun, 6 Dec 2015 14:03:42 -0500 Subject: Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian To: Oded Gabbay References: <1449263345-4938-1-git-send-email-oded.gabbay@gmail.com> <5662BB2D.3040007@vodafone.de> <56648237.2070809@vodafone.de> Cc: Maling list - DRI developers , Alex Deucher , stable@vger.kernel.org From: =?UTF-8?Q?Christian_K=c3=b6nig?= Message-ID: <56648687.7040101@vodafone.de> Date: Sun, 6 Dec 2015 20:03:35 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: On 06.12.2015 20:00, Oded Gabbay wrote: > On Sun, Dec 6, 2015 at 8:45 PM, Christian König wrote: >> On 06.12.2015 08:29, Oded Gabbay wrote: >>> On Sat, Dec 5, 2015 at 12:23 PM, Christian König >>> wrote: >>>> Patch #1 & #2 are Reviewed-by: Christian König >>>> >>>> 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 . > Thanks! > >> It sound like you have hardware to test this combination, so could you try >> to get it working on for amdgpu as well? > Yes, I have a BE machine that I can test amdgpu with. > However, I only have a Bonaire card. Is it worth doing it for CIK ? > It's only for debug support, no ? Yeah, CIK support in amdgpu was only for debugging and bringup. But since it's only shared code you touch when it works with CIK it should work with VI as well and that's rather interesting to us. Regards, Christian. > >> 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 >>>>> 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); >>>>> } >>>>