On 17.07.2012 01:13, Alex Deucher wrote:
> On Fri, Jul 13, 2012 at 9:57 AM, Alex Deucher <alexdeucher at gmail.com>
> wrote:
>> On Fri, Jul 13, 2012 at 9:46 AM, Christian K?nig
>> <deathsimple at vodafone.de> wrote:
>>> On 13.07.2012 14:27, Alex Deucher wrote:
>>>> On Fri, Jul 13, 2012 at 5:09 AM, Christian K?nig
>>>> <deathsimple at vodafone.de> wrote:
>>>>> On 12.07.2012 18:36, Alex Deucher wrote:
>>>>>> On Thu, Jul 12, 2012 at 12:12 PM, Christian K?nig
>>>>>> <deathsimple at vodafone.de> wrote:
>>>>>>> Before emitting any indirect buffer, emit the offset of the next
>>>>>>> valid ring content if any. This allow code that want to resume
>>>>>>> ring to resume ring right after ib that caused GPU lockup.
>>>>>>>
>>>>>>> v2: use scratch registers instead of storing it into memory
>>>>>>> v3: skip over the surface sync for ni and si as well
>>>>>>>
>>>>>>> Signed-off-by: Jerome Glisse <jglisse at redhat.com>
>>>>>>> Signed-off-by: Christian K?nig <deathsimple at vodafone.de>
>>>>>>> ---
>>>>>>> drivers/gpu/drm/radeon/evergreen.c | 8 +++++++-
>>>>>>> drivers/gpu/drm/radeon/ni.c | 11 ++++++++++-
>>>>>>> drivers/gpu/drm/radeon/r600.c | 18 ++++++++++++++++--
>>>>>>> drivers/gpu/drm/radeon/radeon.h | 1 +
>>>>>>> drivers/gpu/drm/radeon/radeon_ring.c | 4 ++++
>>>>>>> drivers/gpu/drm/radeon/rv770.c | 4 +++-
>>>>>>> drivers/gpu/drm/radeon/si.c | 22 +++++++++++++++++++---
>>>>>>> 7 files changed, 60 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/radeon/evergreen.c
>>>>>>> b/drivers/gpu/drm/radeon/evergreen.c
>>>>>>> index f39b900..40de347 100644
>>>>>>> --- a/drivers/gpu/drm/radeon/evergreen.c
>>>>>>> +++ b/drivers/gpu/drm/radeon/evergreen.c
>>>>>>> @@ -1368,7 +1368,13 @@ void evergreen_ring_ib_execute(struct
>>>>>>> radeon_device *rdev, struct radeon_ib *ib)
>>>>>>> /* set to DX10/11 mode */
>>>>>>> radeon_ring_write(ring, PACKET3(PACKET3_MODE_CONTROL, 0));
>>>>>>> radeon_ring_write(ring, 1);
>>>>>>> - /* FIXME: implement */
>>>>>>> +
>>>>>>> + if (ring->rptr_save_reg) {
>>>>>>> + uint32_t next_rptr = ring->wptr + 2 + 4;
>>>>>>> + radeon_ring_write(ring, PACKET0(ring->rptr_save_reg,
>>>>>>> 0));
>>>>>>> + radeon_ring_write(ring, next_rptr);
>>>>>>> + }
>>>>>> On r600 and newer please use SET_CONFIG_REG rather than Packet0.
>>>>> Why? Please note that it's on purpose that this doesn't interfere with
>>>>> the
>>>>> top/bottom of pipe handling and the draw commands, e.g. the register
>>>>> write
>>>>> isn't associated with drawing but instead just marks the beginning of
>>>>> parsing the IB.
>>>> Packet0's are have been semi-deprecated since r600. They still work,
>>>> but the CP guys recommend using the appropriate packet3 whenever
>>>> possible.
>>> Ok, that makes sense.
>>>
>>> Any further comments on the patchset, or can I send that to Dave for merging
>>> now?
>> Other than that, it looks good to me. For the series:
>>
>> Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> Thinking about this more, we should probably support a memory
> locations as well in case there are rings that can't write to
> registers and since most things now use memory (fences, etc.), I'm not
> sure we'll always have scratch regs to use.
The number of scratch registers could get a bit tight if we really get
so much rings with the next hw generation, but I thing that this should
do it for now.
We can always extend it in the future to also support a memory location,
but then we also make sure that writing to that memory location really
works as expected. Just remember the trouble we had with AGP and scratch
writebacks.
Christian.
>
> Alex
>
