On Tue, Jul 17, 2012 at 5:50 AM, Christian König
<deathsimple@xxxxxxxxxxx> wrote:
> On 13.07.2012 16:17, Tom Stellard wrote:
>>
>> On Fri, Jul 13, 2012 at 04:08:15PM +0200, Christian König wrote:
>>>
>>> Const IBs are executed on the CE not the CP, so we can't
>>> fence them in the normal way.
>>>
>>> So submit them directly before the IB instead, just as
>>> the documentation says.
>>>
>>> Signed-off-by: Christian König <deathsimple@xxxxxxxxxxx>
>>> ---
>>> drivers/gpu/drm/radeon/r100.c | 2 +-
>>> drivers/gpu/drm/radeon/r600.c | 2 +-
>>> drivers/gpu/drm/radeon/radeon.h | 3 ++-
>>> drivers/gpu/drm/radeon/radeon_cs.c | 25 +++++++++++--------------
>>> drivers/gpu/drm/radeon/radeon_ring.c | 10 +++++++++-
>>> 5 files changed, 24 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/r100.c
>>> b/drivers/gpu/drm/radeon/r100.c
>>> index e0f5ae8..4ee5a74 100644
>>> --- a/drivers/gpu/drm/radeon/r100.c
>>> +++ b/drivers/gpu/drm/radeon/r100.c
>>> @@ -3693,7 +3693,7 @@ int r100_ib_test(struct radeon_device *rdev, struct
>>> radeon_ring *ring)
>>> ib.ptr[6] = PACKET2(0);
>>> ib.ptr[7] = PACKET2(0);
>>> ib.length_dw = 8;
>>> - r = radeon_ib_schedule(rdev, &ib);
>>> + r = radeon_ib_schedule(rdev, &ib, NULL);
>>> if (r) {
>>> radeon_scratch_free(rdev, scratch);
>>> radeon_ib_free(rdev, &ib);
>>> diff --git a/drivers/gpu/drm/radeon/r600.c
>>> b/drivers/gpu/drm/radeon/r600.c
>>> index 3156d25..c2e5069 100644
>>> --- a/drivers/gpu/drm/radeon/r600.c
>>> +++ b/drivers/gpu/drm/radeon/r600.c
>>> @@ -2619,7 +2619,7 @@ int r600_ib_test(struct radeon_device *rdev, struct
>>> radeon_ring *ring)
>>> ib.ptr[1] = ((scratch - PACKET3_SET_CONFIG_REG_OFFSET) >> 2);
>>> ib.ptr[2] = 0xDEADBEEF;
>>> ib.length_dw = 3;
>>> - r = radeon_ib_schedule(rdev, &ib);
>>> + r = radeon_ib_schedule(rdev, &ib, NULL);
>>> if (r) {
>>> radeon_scratch_free(rdev, scratch);
>>> radeon_ib_free(rdev, &ib);
>>> diff --git a/drivers/gpu/drm/radeon/radeon.h
>>> b/drivers/gpu/drm/radeon/radeon.h
>>> index 2cb355b..2d7f06c 100644
>>> --- a/drivers/gpu/drm/radeon/radeon.h
>>> +++ b/drivers/gpu/drm/radeon/radeon.h
>>> @@ -751,7 +751,8 @@ struct si_rlc {
>>> int radeon_ib_get(struct radeon_device *rdev, int ring,
>>> struct radeon_ib *ib, unsigned size);
>>> void radeon_ib_free(struct radeon_device *rdev, struct radeon_ib *ib);
>>> -int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib
>>> *ib);
>>> +int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib,
>>> + struct radeon_ib *const_ib);
>>> int radeon_ib_pool_init(struct radeon_device *rdev);
>>> void radeon_ib_pool_fini(struct radeon_device *rdev);
>>> int radeon_ib_ring_tests(struct radeon_device *rdev);
>>> diff --git a/drivers/gpu/drm/radeon/radeon_cs.c
>>> b/drivers/gpu/drm/radeon/radeon_cs.c
>>> index 553da67..d0be5d5 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_cs.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_cs.c
>>> @@ -354,7 +354,7 @@ static int radeon_cs_ib_chunk(struct radeon_device
>>> *rdev,
>>> }
>>> radeon_cs_sync_rings(parser);
>>> parser->ib.vm_id = 0;
>>> - r = radeon_ib_schedule(rdev, &parser->ib);
>>> + r = radeon_ib_schedule(rdev, &parser->ib, NULL);
>>> if (r) {
>>> DRM_ERROR("Failed to schedule IB !\n");
>>> }
>>> @@ -452,25 +452,22 @@ static int radeon_cs_ib_vm_chunk(struct
>>> radeon_device *rdev,
>>> }
>>> radeon_cs_sync_rings(parser);
>>> + parser->ib.vm_id = vm->id;
>>> + /* ib pool is bind at 0 in virtual address space,
>>> + * so gpu_addr is the offset inside the pool bo
>>> + */
>>> + parser->ib.gpu_addr = parser->ib.sa_bo->soffset;
>>> +
>>> if ((rdev->family >= CHIP_TAHITI) &&
>>> (parser->chunk_const_ib_idx != -1)) {
>>> parser->const_ib.vm_id = vm->id;
>>> - /* ib pool is bind at 0 in virtual address space to
>>> gpu_addr is the
>>> - * offset inside the pool bo
>>> - */
>>> + /* same reason as above */
>>> parser->const_ib.gpu_addr =
>>> parser->const_ib.sa_bo->soffset;
>>> - r = radeon_ib_schedule(rdev, &parser->const_ib);
>>> - if (r)
>>> - goto out;
>>> + r = radeon_ib_schedule(rdev, &parser->ib,
>>> &parser->const_ib);
>>> + } else {
>>> + r = radeon_ib_schedule(rdev, &parser->ib, NULL);
>>> }
>>> - parser->ib.vm_id = vm->id;
>>> - /* ib pool is bind at 0 in virtual address space to gpu_addr is
>>> the
>>> - * offset inside the pool bo
>>> - */
>>> - parser->ib.gpu_addr = parser->ib.sa_bo->soffset;
>>> - parser->ib.is_const_ib = false;
>>> - r = radeon_ib_schedule(rdev, &parser->ib);
>>> out:
>>> if (!r) {
>>> if (vm->fence) {
>>> diff --git a/drivers/gpu/drm/radeon/radeon_ring.c
>>> b/drivers/gpu/drm/radeon/radeon_ring.c
>>> index 75cbe46..c48c354 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_ring.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_ring.c
>>> @@ -74,7 +74,8 @@ void radeon_ib_free(struct radeon_device *rdev, struct
>>> radeon_ib *ib)
>>> radeon_fence_unref(&ib->fence);
>>> }
>>>
>>> -int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib)
>>> +int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib,
>>> + struct radeon_ib *const_ib)
>>
>> Since you are modifying an uncommented function, comments should be
>> added, per the new documentation rules.
>>
>> I don't mean to be picky, but there is no point having documentation
>> rules if they aren't enforced.
>
> Oh no, please be picky about it. Otherwise I wouldn't learn it.
>
> For this particular change Alex already had appropriate documentation in the
> pipeline and I wanted to rather change them instead of adding documentation
> in this patch.
>
> Christian.
Still my earlier remark matter, i would rather not see cross comment
reference it's confusing especially if code move. I rather see the
same comment 2 times.
Cheers,
Jerome
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
