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 16:37:18 +0200 Message-ID: <5005789E.4090908@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> <50052723.2090900@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 B43949E88F for ; Tue, 17 Jul 2012 07:37:21 -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: Jerome Glisse Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 17.07.2012 16:17, Jerome Glisse wrote: > On Tue, Jul 17, 2012 at 8:51 AM, Alex Deucher wro= te: >> On Tue, Jul 17, 2012 at 4:49 AM, Christian K=F6nig >> wrote: >>> On 17.07.2012 01:13, Alex Deucher wrote: >>>> On Fri, Jul 13, 2012 at 9:57 AM, Alex Deucher >>>> wrote: >>>>> 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 regist= er >>>>>>>> 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 wor= k, >>>>>>> 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 >>>> 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 work= s as >>> expected. Just remember the trouble we had with AGP and scratch writeba= cks. >>> >> 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 >