From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Christian_K=F6nig?= Subject: Re: [PATCH 14/15] drm/radeon: record what is next valid wptr for each ring v3 Date: Tue, 17 Jul 2012 10:49:39 +0200 Message-ID: <50052723.2090900@vodafone.de> References: <1342109574-8107-1-git-send-email-deathsimple@vodafone.de> <1342109574-8107-15-git-send-email-deathsimple@vodafone.de> <4FFFE5B2.6020800@vodafone.de> <500026AF.7030503@vodafone.de> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from outgoing.email.vodafone.de (outgoing.email.vodafone.de [139.7.28.128]) by gabe.freedesktop.org (Postfix) with ESMTP id E94B0A09A5 for ; Tue, 17 Jul 2012 01:49:44 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Alex Deucher Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 17.07.2012 01:13, Alex Deucher wrote: > On Fri, Jul 13, 2012 at 9:57 AM, Alex Deucher wro= te: >> On Fri, Jul 13, 2012 at 9:46 AM, Christian K=F6nig >> wrote: >>> On 13.07.2012 14:27, Alex Deucher wrote: >>>> On Fri, Jul 13, 2012 at 5:09 AM, Christian K=F6nig >>>> wrote: >>>>> On 12.07.2012 18:36, Alex Deucher wrote: >>>>>> On Thu, Jul 12, 2012 at 12:12 PM, Christian K=F6nig >>>>>> 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 >>>>>>> Signed-off-by: Christian K=F6nig >>>>>>> --- >>>>>>> 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 =3D 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 me= rging >>> now? >> Other than that, it looks good to me. For the series: >> >> Reviewed-by: Alex Deucher > 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 >