[PATCH 14/15] drm/radeon: record what is next valid wptr for each ring v3

[Christian König]

On 17.07.2012 16:17, Jerome Glisse wrote:
> On Tue, Jul 17, 2012 at 8:51 AM, Alex Deucher <alexdeucher at gmail.com> wrote:
>> On Tue, Jul 17, 2012 at 4:49 AM, Christian König
>> <deathsimple at vodafone.de> wrote:
>>> 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.
>>>
>> Ok, I'll put a new patch on top when we need it.
>>
>> Alex
> My first version used memory write and i think we should forget about
> AGP this will never gonna happen again (if i were in the mob i would
> say that we made them an offer they could not refuse ;))
LOL, yeah that's somewhat true. Well it just looked simpler to me to use 
a register instead of memory.

Feel free to use a memory write for the CP if the need really arise, but 
keep in mind that we still have rings which can't do so.

Christian.

> Cheers,
> Jerome
>