* [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian @ 2015-12-04 21:09 ` Oded Gabbay 0 siblings, 0 replies; 19+ messages in thread From: Oded Gabbay @ 2015-12-04 21:09 UTC (permalink / raw) To: dri-devel, alexdeucher; +Cc: stable 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); } -- 2.5.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian @ 2015-12-04 21:09 ` Oded Gabbay 0 siblings, 0 replies; 19+ messages in thread From: Oded Gabbay @ 2015-12-04 21:09 UTC (permalink / raw) To: dri-devel, alexdeucher; +Cc: stable 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); } -- 2.5.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/3] radeon: Fix VCE ring test for Big-Endian systems 2015-12-04 21:09 ` Oded Gabbay (?) @ 2015-12-04 21:09 ` Oded Gabbay 2015-12-07 7:51 ` Michel Dänzer -1 siblings, 1 reply; 19+ messages in thread From: Oded Gabbay @ 2015-12-04 21:09 UTC (permalink / raw) To: dri-devel, alexdeucher; +Cc: stable This patch fixes the VCE ring test when running on Big-Endian machines. Every write to the ring needs to be translated to little-endian. Signed-off-by: Oded Gabbay <oded.gabbay@gmail.com> Cc: stable@vger.kernel.org --- drivers/gpu/drm/radeon/radeon_vce.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_vce.c b/drivers/gpu/drm/radeon/radeon_vce.c index 574f62b..86f57e4 100644 --- a/drivers/gpu/drm/radeon/radeon_vce.c +++ b/drivers/gpu/drm/radeon/radeon_vce.c @@ -699,12 +699,12 @@ bool radeon_vce_semaphore_emit(struct radeon_device *rdev, { uint64_t addr = semaphore->gpu_addr; - radeon_ring_write(ring, VCE_CMD_SEMAPHORE); - radeon_ring_write(ring, (addr >> 3) & 0x000FFFFF); - radeon_ring_write(ring, (addr >> 23) & 0x000FFFFF); - radeon_ring_write(ring, 0x01003000 | (emit_wait ? 1 : 0)); + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_SEMAPHORE)); + radeon_ring_write(ring, cpu_to_le32((addr >> 3) & 0x000FFFFF)); + radeon_ring_write(ring, cpu_to_le32((addr >> 23) & 0x000FFFFF)); + radeon_ring_write(ring, cpu_to_le32(0x01003000 | (emit_wait ? 1 : 0))); if (!emit_wait) - radeon_ring_write(ring, VCE_CMD_END); + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END)); return true; } @@ -719,10 +719,10 @@ bool radeon_vce_semaphore_emit(struct radeon_device *rdev, void radeon_vce_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib) { struct radeon_ring *ring = &rdev->ring[ib->ring]; - radeon_ring_write(ring, VCE_CMD_IB); - radeon_ring_write(ring, ib->gpu_addr); - radeon_ring_write(ring, upper_32_bits(ib->gpu_addr)); - radeon_ring_write(ring, ib->length_dw); + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_IB)); + radeon_ring_write(ring, cpu_to_le32(ib->gpu_addr)); + radeon_ring_write(ring, cpu_to_le32(upper_32_bits(ib->gpu_addr))); + radeon_ring_write(ring, cpu_to_le32(ib->length_dw)); } /** @@ -738,12 +738,12 @@ void radeon_vce_fence_emit(struct radeon_device *rdev, struct radeon_ring *ring = &rdev->ring[fence->ring]; uint64_t addr = rdev->fence_drv[fence->ring].gpu_addr; - radeon_ring_write(ring, VCE_CMD_FENCE); - radeon_ring_write(ring, addr); - radeon_ring_write(ring, upper_32_bits(addr)); - radeon_ring_write(ring, fence->seq); - radeon_ring_write(ring, VCE_CMD_TRAP); - radeon_ring_write(ring, VCE_CMD_END); + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_FENCE)); + radeon_ring_write(ring, cpu_to_le32(addr)); + radeon_ring_write(ring, cpu_to_le32(upper_32_bits(addr))); + radeon_ring_write(ring, cpu_to_le32(fence->seq)); + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_TRAP)); + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END)); } /** @@ -765,7 +765,7 @@ int radeon_vce_ring_test(struct radeon_device *rdev, struct radeon_ring *ring) ring->idx, r); return r; } - radeon_ring_write(ring, VCE_CMD_END); + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END)); radeon_ring_unlock_commit(rdev, ring, false); for (i = 0; i < rdev->usec_timeout; i++) { -- 2.5.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] radeon: Fix VCE ring test for Big-Endian systems 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 0 siblings, 1 reply; 19+ messages in thread From: Michel Dänzer @ 2015-12-07 7:51 UTC (permalink / raw) To: Oded Gabbay; +Cc: dri-devel On 05.12.2015 06:09, Oded Gabbay wrote: > This patch fixes the VCE ring test when running on Big-Endian machines. > Every write to the ring needs to be translated to little-endian. > > Signed-off-by: Oded Gabbay <oded.gabbay@gmail.com> > Cc: stable@vger.kernel.org > --- > drivers/gpu/drm/radeon/radeon_vce.c | 32 ++++++++++++++++---------------- > 1 file changed, 16 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_vce.c b/drivers/gpu/drm/radeon/radeon_vce.c > index 574f62b..86f57e4 100644 > --- a/drivers/gpu/drm/radeon/radeon_vce.c > +++ b/drivers/gpu/drm/radeon/radeon_vce.c > @@ -699,12 +699,12 @@ bool radeon_vce_semaphore_emit(struct radeon_device *rdev, > { > uint64_t addr = semaphore->gpu_addr; > > - radeon_ring_write(ring, VCE_CMD_SEMAPHORE); > - radeon_ring_write(ring, (addr >> 3) & 0x000FFFFF); > - radeon_ring_write(ring, (addr >> 23) & 0x000FFFFF); > - radeon_ring_write(ring, 0x01003000 | (emit_wait ? 1 : 0)); > + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_SEMAPHORE)); > + radeon_ring_write(ring, cpu_to_le32((addr >> 3) & 0x000FFFFF)); > + radeon_ring_write(ring, cpu_to_le32((addr >> 23) & 0x000FFFFF)); > + radeon_ring_write(ring, cpu_to_le32(0x01003000 | (emit_wait ? 1 : 0))); > if (!emit_wait) > - radeon_ring_write(ring, VCE_CMD_END); > + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END)); > > return true; > } > @@ -719,10 +719,10 @@ bool radeon_vce_semaphore_emit(struct radeon_device *rdev, > void radeon_vce_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib) > { > struct radeon_ring *ring = &rdev->ring[ib->ring]; > - radeon_ring_write(ring, VCE_CMD_IB); > - radeon_ring_write(ring, ib->gpu_addr); > - radeon_ring_write(ring, upper_32_bits(ib->gpu_addr)); > - radeon_ring_write(ring, ib->length_dw); > + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_IB)); > + radeon_ring_write(ring, cpu_to_le32(ib->gpu_addr)); > + radeon_ring_write(ring, cpu_to_le32(upper_32_bits(ib->gpu_addr))); > + radeon_ring_write(ring, cpu_to_le32(ib->length_dw)); > } > > /** > @@ -738,12 +738,12 @@ void radeon_vce_fence_emit(struct radeon_device *rdev, > struct radeon_ring *ring = &rdev->ring[fence->ring]; > uint64_t addr = rdev->fence_drv[fence->ring].gpu_addr; > > - radeon_ring_write(ring, VCE_CMD_FENCE); > - radeon_ring_write(ring, addr); > - radeon_ring_write(ring, upper_32_bits(addr)); > - radeon_ring_write(ring, fence->seq); > - radeon_ring_write(ring, VCE_CMD_TRAP); > - radeon_ring_write(ring, VCE_CMD_END); > + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_FENCE)); > + radeon_ring_write(ring, cpu_to_le32(addr)); > + radeon_ring_write(ring, cpu_to_le32(upper_32_bits(addr))); > + radeon_ring_write(ring, cpu_to_le32(fence->seq)); > + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_TRAP)); > + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END)); > } > > /** > @@ -765,7 +765,7 @@ int radeon_vce_ring_test(struct radeon_device *rdev, struct radeon_ring *ring) > ring->idx, r); > return r; > } > - radeon_ring_write(ring, VCE_CMD_END); > + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END)); > radeon_ring_unlock_commit(rdev, ring, false); > > for (i = 0; i < rdev->usec_timeout; i++) { > A new helper function such as static inline void radeon_ring_write_le(struct radeon_ring *ring, uint32_t v) { radeon_ring_write(ring, cpu_to_le32(v)); } might be nice for this. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] radeon: Fix VCE ring test for Big-Endian systems 2015-12-07 7:51 ` Michel Dänzer @ 2015-12-07 17:49 ` Oded Gabbay 2015-12-08 3:00 ` Michel Dänzer 0 siblings, 1 reply; 19+ messages in thread From: Oded Gabbay @ 2015-12-07 17:49 UTC (permalink / raw) To: Michel Dänzer; +Cc: Maling list - DRI developers On Mon, Dec 7, 2015 at 9:51 AM, Michel Dänzer <michel@daenzer.net> wrote: > On 05.12.2015 06:09, Oded Gabbay wrote: >> This patch fixes the VCE ring test when running on Big-Endian machines. >> Every write to the ring needs to be translated to little-endian. >> >> Signed-off-by: Oded Gabbay <oded.gabbay@gmail.com> >> Cc: stable@vger.kernel.org >> --- >> drivers/gpu/drm/radeon/radeon_vce.c | 32 ++++++++++++++++---------------- >> 1 file changed, 16 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/radeon_vce.c b/drivers/gpu/drm/radeon/radeon_vce.c >> index 574f62b..86f57e4 100644 >> --- a/drivers/gpu/drm/radeon/radeon_vce.c >> +++ b/drivers/gpu/drm/radeon/radeon_vce.c >> @@ -699,12 +699,12 @@ bool radeon_vce_semaphore_emit(struct radeon_device *rdev, >> { >> uint64_t addr = semaphore->gpu_addr; >> >> - radeon_ring_write(ring, VCE_CMD_SEMAPHORE); >> - radeon_ring_write(ring, (addr >> 3) & 0x000FFFFF); >> - radeon_ring_write(ring, (addr >> 23) & 0x000FFFFF); >> - radeon_ring_write(ring, 0x01003000 | (emit_wait ? 1 : 0)); >> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_SEMAPHORE)); >> + radeon_ring_write(ring, cpu_to_le32((addr >> 3) & 0x000FFFFF)); >> + radeon_ring_write(ring, cpu_to_le32((addr >> 23) & 0x000FFFFF)); >> + radeon_ring_write(ring, cpu_to_le32(0x01003000 | (emit_wait ? 1 : 0))); >> if (!emit_wait) >> - radeon_ring_write(ring, VCE_CMD_END); >> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END)); >> >> return true; >> } >> @@ -719,10 +719,10 @@ bool radeon_vce_semaphore_emit(struct radeon_device *rdev, >> void radeon_vce_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib) >> { >> struct radeon_ring *ring = &rdev->ring[ib->ring]; >> - radeon_ring_write(ring, VCE_CMD_IB); >> - radeon_ring_write(ring, ib->gpu_addr); >> - radeon_ring_write(ring, upper_32_bits(ib->gpu_addr)); >> - radeon_ring_write(ring, ib->length_dw); >> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_IB)); >> + radeon_ring_write(ring, cpu_to_le32(ib->gpu_addr)); >> + radeon_ring_write(ring, cpu_to_le32(upper_32_bits(ib->gpu_addr))); >> + radeon_ring_write(ring, cpu_to_le32(ib->length_dw)); >> } >> >> /** >> @@ -738,12 +738,12 @@ void radeon_vce_fence_emit(struct radeon_device *rdev, >> struct radeon_ring *ring = &rdev->ring[fence->ring]; >> uint64_t addr = rdev->fence_drv[fence->ring].gpu_addr; >> >> - radeon_ring_write(ring, VCE_CMD_FENCE); >> - radeon_ring_write(ring, addr); >> - radeon_ring_write(ring, upper_32_bits(addr)); >> - radeon_ring_write(ring, fence->seq); >> - radeon_ring_write(ring, VCE_CMD_TRAP); >> - radeon_ring_write(ring, VCE_CMD_END); >> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_FENCE)); >> + radeon_ring_write(ring, cpu_to_le32(addr)); >> + radeon_ring_write(ring, cpu_to_le32(upper_32_bits(addr))); >> + radeon_ring_write(ring, cpu_to_le32(fence->seq)); >> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_TRAP)); >> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END)); >> } >> >> /** >> @@ -765,7 +765,7 @@ int radeon_vce_ring_test(struct radeon_device *rdev, struct radeon_ring *ring) >> ring->idx, r); >> return r; >> } >> - radeon_ring_write(ring, VCE_CMD_END); >> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END)); >> radeon_ring_unlock_commit(rdev, ring, false); >> >> for (i = 0; i < rdev->usec_timeout; i++) { >> > > A new helper function such as > > static inline void radeon_ring_write_le(struct radeon_ring *ring, uint32_t v) > { > radeon_ring_write(ring, cpu_to_le32(v)); > } > > might be nice for this. > > > -- > Earthling Michel Dänzer | http://www.amd.com > Libre software enthusiast | Mesa and X developer IMHO, I don't think this gives any benefit. You would just need to replace every: radeon_ring_write(ring, cpu_to_le32(SOME_DEFINE)); with radeon_ring_write_le(ring, SOME_DEFINE); So no reduce in code size. Also, if you change it in my code, I think you need to change it in the entire driver for consistency. What's even more important, is that when I look at the above, it seems to me this change even makes the code *less* clear as you now need to go into radeon_ring_write_le to actually understand how the value is written. Oded _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] radeon: Fix VCE ring test for Big-Endian systems 2015-12-07 17:49 ` Oded Gabbay @ 2015-12-08 3:00 ` Michel Dänzer 2015-12-09 10:20 ` Rafał Miłecki 0 siblings, 1 reply; 19+ messages in thread From: Michel Dänzer @ 2015-12-08 3:00 UTC (permalink / raw) To: Oded Gabbay; +Cc: Maling list - DRI developers On 08.12.2015 02:49, Oded Gabbay wrote: > On Mon, Dec 7, 2015 at 9:51 AM, Michel Dänzer <michel@daenzer.net> wrote: >> On 05.12.2015 06:09, Oded Gabbay wrote: >>> >>> @@ -765,7 +765,7 @@ int radeon_vce_ring_test(struct radeon_device *rdev, struct radeon_ring *ring) >>> ring->idx, r); >>> return r; >>> } >>> - radeon_ring_write(ring, VCE_CMD_END); >>> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END)); >>> radeon_ring_unlock_commit(rdev, ring, false); >>> >>> for (i = 0; i < rdev->usec_timeout; i++) { >>> >> >> A new helper function such as >> >> static inline void radeon_ring_write_le(struct radeon_ring *ring, uint32_t v) >> { >> radeon_ring_write(ring, cpu_to_le32(v)); >> } >> >> might be nice for this. > > IMHO, I don't think this gives any benefit. > You would just need to replace every: > > radeon_ring_write(ring, cpu_to_le32(SOME_DEFINE)); > > with > > radeon_ring_write_le(ring, SOME_DEFINE); > > So no reduce in code size. Also, if you change it in my code, I think > you need to change it in the entire driver for consistency. > > What's even more important, is that when I look at the above, it seems > to me this change even makes the code *less* clear as you now need to > go into radeon_ring_write_le to actually understand how the value is > written. Sprinkling cpu_to_le32 all over the place just seems a bit ugly to me, but I don't feel strongly about it. I.e. I'm fine with the patch as is, it was just a suggestion. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] radeon: Fix VCE ring test for Big-Endian systems 2015-12-08 3:00 ` Michel Dänzer @ 2015-12-09 10:20 ` Rafał Miłecki 0 siblings, 0 replies; 19+ messages in thread From: Rafał Miłecki @ 2015-12-09 10:20 UTC (permalink / raw) To: Michel Dänzer; +Cc: Maling list - DRI developers On 8 December 2015 at 04:00, Michel Dänzer <michel@daenzer.net> wrote: > On 08.12.2015 02:49, Oded Gabbay wrote: >> On Mon, Dec 7, 2015 at 9:51 AM, Michel Dänzer <michel@daenzer.net> wrote: >>> On 05.12.2015 06:09, Oded Gabbay wrote: >>>> >>>> @@ -765,7 +765,7 @@ int radeon_vce_ring_test(struct radeon_device *rdev, struct radeon_ring *ring) >>>> ring->idx, r); >>>> return r; >>>> } >>>> - radeon_ring_write(ring, VCE_CMD_END); >>>> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END)); >>>> radeon_ring_unlock_commit(rdev, ring, false); >>>> >>>> for (i = 0; i < rdev->usec_timeout; i++) { >>>> >>> >>> A new helper function such as >>> >>> static inline void radeon_ring_write_le(struct radeon_ring *ring, uint32_t v) >>> { >>> radeon_ring_write(ring, cpu_to_le32(v)); >>> } >>> >>> might be nice for this. >> >> IMHO, I don't think this gives any benefit. >> You would just need to replace every: >> >> radeon_ring_write(ring, cpu_to_le32(SOME_DEFINE)); >> >> with >> >> radeon_ring_write_le(ring, SOME_DEFINE); >> >> So no reduce in code size. Also, if you change it in my code, I think >> you need to change it in the entire driver for consistency. >> >> What's even more important, is that when I look at the above, it seems >> to me this change even makes the code *less* clear as you now need to >> go into radeon_ring_write_le to actually understand how the value is >> written. > > Sprinkling cpu_to_le32 all over the place just seems a bit ugly to me, > but I don't feel strongly about it. I.e. I'm fine with the patch as is, > it was just a suggestion. I agree, having cpu_to_le32 repeated over and over makes code messy, harder to read, harder to fit 80 chars (which is already often violated in radeon code). radeon_ring_write_le sounds self explanatory. -- Rafał _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/3] radeon: Fix VCE IB test on Big-Endian systems 2015-12-04 21:09 ` Oded Gabbay (?) (?) @ 2015-12-04 21:09 ` Oded Gabbay -1 siblings, 0 replies; 19+ messages in thread From: Oded Gabbay @ 2015-12-04 21:09 UTC (permalink / raw) To: dri-devel, alexdeucher; +Cc: stable This patch makes the VCE IB test pass on Big-Endian systems. It converts to little-endian the contents of the VCE message. Signed-off-by: Oded Gabbay <oded.gabbay@gmail.com> Cc: stable@vger.kernel.org --- drivers/gpu/drm/radeon/radeon_vce.c | 68 ++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_vce.c b/drivers/gpu/drm/radeon/radeon_vce.c index 86f57e4..7eb1ae7 100644 --- a/drivers/gpu/drm/radeon/radeon_vce.c +++ b/drivers/gpu/drm/radeon/radeon_vce.c @@ -361,31 +361,31 @@ int radeon_vce_get_create_msg(struct radeon_device *rdev, int ring, /* stitch together an VCE create msg */ ib.length_dw = 0; - ib.ptr[ib.length_dw++] = 0x0000000c; /* len */ - ib.ptr[ib.length_dw++] = 0x00000001; /* session cmd */ - ib.ptr[ib.length_dw++] = handle; - - ib.ptr[ib.length_dw++] = 0x00000030; /* len */ - ib.ptr[ib.length_dw++] = 0x01000001; /* create cmd */ - ib.ptr[ib.length_dw++] = 0x00000000; - ib.ptr[ib.length_dw++] = 0x00000042; - ib.ptr[ib.length_dw++] = 0x0000000a; - ib.ptr[ib.length_dw++] = 0x00000001; - ib.ptr[ib.length_dw++] = 0x00000080; - ib.ptr[ib.length_dw++] = 0x00000060; - ib.ptr[ib.length_dw++] = 0x00000100; - ib.ptr[ib.length_dw++] = 0x00000100; - ib.ptr[ib.length_dw++] = 0x0000000c; - ib.ptr[ib.length_dw++] = 0x00000000; - - ib.ptr[ib.length_dw++] = 0x00000014; /* len */ - ib.ptr[ib.length_dw++] = 0x05000005; /* feedback buffer */ - ib.ptr[ib.length_dw++] = upper_32_bits(dummy); - ib.ptr[ib.length_dw++] = dummy; - ib.ptr[ib.length_dw++] = 0x00000001; + ib.ptr[ib.length_dw++] = cpu_to_le32(0x0000000c); /* len */ + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000001); /* session cmd */ + ib.ptr[ib.length_dw++] = cpu_to_le32(handle); + + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000030); /* len */ + ib.ptr[ib.length_dw++] = cpu_to_le32(0x01000001); /* create cmd */ + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000000); + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000042); + ib.ptr[ib.length_dw++] = cpu_to_le32(0x0000000a); + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000001); + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000080); + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000060); + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000100); + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000100); + ib.ptr[ib.length_dw++] = cpu_to_le32(0x0000000c); + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000000); + + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000014); /* len */ + ib.ptr[ib.length_dw++] = cpu_to_le32(0x05000005); /* feedback buffer */ + ib.ptr[ib.length_dw++] = cpu_to_le32(upper_32_bits(dummy)); + ib.ptr[ib.length_dw++] = cpu_to_le32(dummy); + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000001); for (i = ib.length_dw; i < ib_size_dw; ++i) - ib.ptr[i] = 0x0; + ib.ptr[i] = cpu_to_le32(0x0); r = radeon_ib_schedule(rdev, &ib, NULL, false); if (r) { @@ -428,21 +428,21 @@ int radeon_vce_get_destroy_msg(struct radeon_device *rdev, int ring, /* stitch together an VCE destroy msg */ ib.length_dw = 0; - ib.ptr[ib.length_dw++] = 0x0000000c; /* len */ - ib.ptr[ib.length_dw++] = 0x00000001; /* session cmd */ - ib.ptr[ib.length_dw++] = handle; + ib.ptr[ib.length_dw++] = cpu_to_le32(0x0000000c); /* len */ + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000001); /* session cmd */ + ib.ptr[ib.length_dw++] = cpu_to_le32(handle); - ib.ptr[ib.length_dw++] = 0x00000014; /* len */ - ib.ptr[ib.length_dw++] = 0x05000005; /* feedback buffer */ - ib.ptr[ib.length_dw++] = upper_32_bits(dummy); - ib.ptr[ib.length_dw++] = dummy; - ib.ptr[ib.length_dw++] = 0x00000001; + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000014); /* len */ + ib.ptr[ib.length_dw++] = cpu_to_le32(0x05000005); /* feedback buffer */ + ib.ptr[ib.length_dw++] = cpu_to_le32(upper_32_bits(dummy)); + ib.ptr[ib.length_dw++] = cpu_to_le32(dummy); + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000001); - ib.ptr[ib.length_dw++] = 0x00000008; /* len */ - ib.ptr[ib.length_dw++] = 0x02000001; /* destroy cmd */ + ib.ptr[ib.length_dw++] = cpu_to_le32(0x00000008); /* len */ + ib.ptr[ib.length_dw++] = cpu_to_le32(0x02000001); /* destroy cmd */ for (i = ib.length_dw; i < ib_size_dw; ++i) - ib.ptr[i] = 0x0; + ib.ptr[i] = cpu_to_le32(0x0); r = radeon_ib_schedule(rdev, &ib, NULL, false); if (r) { -- 2.5.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian 2015-12-04 21:09 ` Oded Gabbay ` (2 preceding siblings ...) (?) @ 2015-12-05 10:23 ` Christian König 2015-12-06 7:29 ` Oded Gabbay -1 siblings, 1 reply; 19+ messages in thread From: Christian König @ 2015-12-05 10:23 UTC (permalink / raw) To: Oded Gabbay, dri-devel, alexdeucher; +Cc: stable 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. 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); Alternatively a helper function adding DW to an IB with swapping could do it as well. 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); > } ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian 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 0 siblings, 0 replies; 19+ messages in thread From: Oded Gabbay @ 2015-12-06 7:29 UTC (permalink / raw) To: Christian König; +Cc: stable, Maling list - DRI developers 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. > 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian @ 2015-12-06 7:29 ` Oded Gabbay 0 siblings, 0 replies; 19+ messages in thread From: Oded Gabbay @ 2015-12-06 7:29 UTC (permalink / raw) To: Christian König; +Cc: Maling list - DRI developers, Alex Deucher, stable 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. > 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); >> } > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian 2015-12-06 7:29 ` Oded Gabbay @ 2015-12-06 18:45 ` Christian König -1 siblings, 0 replies; 19+ messages in thread From: Christian König @ 2015-12-06 18:45 UTC (permalink / raw) To: Oded Gabbay; +Cc: stable, Maling list - DRI developers 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian @ 2015-12-06 18:45 ` Christian König 0 siblings, 0 replies; 19+ messages in thread From: Christian König @ 2015-12-06 18:45 UTC (permalink / raw) To: Oded Gabbay; +Cc: Maling list - DRI developers, Alex Deucher, stable 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); >>> } >> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian 2015-12-06 18:45 ` Christian König (?) @ 2015-12-06 19:00 ` Oded Gabbay 2015-12-06 19:03 ` Christian König -1 siblings, 1 reply; 19+ messages in thread From: Oded Gabbay @ 2015-12-06 19:00 UTC (permalink / raw) To: Christian König; +Cc: Maling list - DRI developers, Alex Deucher, stable On Sun, Dec 6, 2015 at 8:45 PM, Christian König <deathsimple@vodafone.de> wrote: > 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>. 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 ? > > 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); >>>> } >>> >>> > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian 2015-12-06 19:00 ` Oded Gabbay @ 2015-12-06 19:03 ` Christian König 0 siblings, 0 replies; 19+ messages in thread From: Christian König @ 2015-12-06 19:03 UTC (permalink / raw) To: Oded Gabbay; +Cc: stable, Maling list - DRI developers On 06.12.2015 20:00, Oded Gabbay wrote: > On Sun, Dec 6, 2015 at 8:45 PM, Christian König <deathsimple@vodafone.de> wrote: >> 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>. > 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 <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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian @ 2015-12-06 19:03 ` Christian König 0 siblings, 0 replies; 19+ messages in thread From: Christian König @ 2015-12-06 19:03 UTC (permalink / raw) To: Oded Gabbay; +Cc: Maling list - DRI developers, Alex Deucher, stable On 06.12.2015 20:00, Oded Gabbay wrote: > On Sun, Dec 6, 2015 at 8:45 PM, Christian König <deathsimple@vodafone.de> wrote: >> 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>. > 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 <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); >>>>> } >>>> ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian 2015-12-06 19:03 ` Christian König (?) @ 2015-12-06 19:05 ` Oded Gabbay -1 siblings, 0 replies; 19+ messages in thread From: Oded Gabbay @ 2015-12-06 19:05 UTC (permalink / raw) To: Christian König; +Cc: Maling list - DRI developers, Alex Deucher, stable OK, I'll take a look Oded > 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian 2015-12-06 18:45 ` Christian König @ 2015-12-09 5:25 ` Alex Deucher -1 siblings, 0 replies; 19+ messages in thread From: Alex Deucher @ 2015-12-09 5:25 UTC (permalink / raw) To: Christian König; +Cc: for 3.8, Maling list - DRI developers On Sun, Dec 6, 2015 at 1:45 PM, Christian König <deathsimple@vodafone.de> wrote: > 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>. Applied the series. Thanks! Alex > > 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] radeon/cik: Fix GFX IB test on Big-Endian @ 2015-12-09 5:25 ` Alex Deucher 0 siblings, 0 replies; 19+ messages in thread From: Alex Deucher @ 2015-12-09 5:25 UTC (permalink / raw) To: Christian König; +Cc: Oded Gabbay, Maling list - DRI developers, for 3.8 On Sun, Dec 6, 2015 at 1:45 PM, Christian König <deathsimple@vodafone.de> wrote: > 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>. Applied the series. Thanks! Alex > > 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); >>>> } >>> >>> > ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-12-09 10:20 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 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
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.