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 >