From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerome Glisse Subject: Re: [PATCH 14/15] drm/radeon: record what is next valid wptr for each ring v3 Date: Tue, 17 Jul 2012 10:17:44 -0400 Message-ID: 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" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-qc0-f181.google.com (mail-qc0-f181.google.com [209.85.216.181]) by gabe.freedesktop.org (Postfix) with ESMTP id E14139EB09 for ; Tue, 17 Jul 2012 07:17:44 -0700 (PDT) Received: by qcpx40 with SMTP id x40so315334qcp.26 for ; Tue, 17 Jul 2012 07:17: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 Tue, Jul 17, 2012 at 8:51 AM, Alex Deucher wrote: > 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_r= eg, >>>>>>>>> 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 w= ith >>>>>>> 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 >>> >>> 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 writebac= ks. >> > > 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 ;)) Cheers, Jerome