From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Christian_K=F6nig?= Subject: Re: Include request for reset-rework branch v4 Date: Thu, 03 May 2012 19:49:00 +0200 Message-ID: <4FA2C50C.6020902@vodafone.de> References: <1335964285-2625-1-git-send-email-deathsimple@vodafone.de> <4FA23F99.7050207@vodafone.de> <4FA2B4DB.4070204@vodafone.de> <4FA2C048.50808@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 C64E39E74C for ; Thu, 3 May 2012 10:49:02 -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 03.05.2012 19:34, Jerome Glisse wrote: > On Thu, May 3, 2012 at 1:28 PM, Christian K=F6nig wrote: >> On 03.05.2012 19:20, Alex Deucher wrote: >>> 2012/5/3 Jerome Glisse: >>>> On Thu, May 3, 2012 at 12:39 PM, Christian K=F6nig >>>> wrote: >>>>> On 03.05.2012 18:32, Jerome Glisse wrote: >>>>>> On Thu, May 3, 2012 at 4:19 AM, Christian >>>>>> K=F6nig >>>>>> wrote: >>>>>>> On 02.05.2012 18:01, Jerome Glisse wrote: >>>>>>>> On Wed, May 2, 2012 at 9:11 AM, Christian >>>>>>>> K=F6nig >>>>>>>> wrote: >>>>>>>>> Hi Dave, >>>>>>>>> >>>>>>>>> there still seems to be the need for some further discussion about >>>>>>>>> the >>>>>>>>> SA >>>>>>>>> code, >>>>>>>>> so I again split that out of the patchset and tested the result a >>>>>>>>> bit. >>>>>>>>> >>>>>>>>> Most of the stuff still works fine without those offending change= s, >>>>>>>>> so >>>>>>>>> to >>>>>>>>> avoid >>>>>>>>> mailing around unrelated and already reviewed patches, I request = the >>>>>>>>> include >>>>>>>>> the following 17 patches into drm-next. >>>>>>>>> >>>>>>>>> If you prefer to merge they are also available from >>>>>>>>> git://people.freedesktop.org/~deathsimple/linux branch reset-rewo= rk. >>>>>>>>> >>>>>>>>> Cheers, >>>>>>>>> Christian. >>>>>>>>> >>>>>>>> I am ok with this 17 patchset, i just sent 3 patch on top of those= 17 >>>>>>>> that >>>>>>>> bring back some other of the previous cleanup. >>>>>>> At least for now those three are NAK, cause I just realized we need= to >>>>>>> put >>>>>>> those on top of a more sophisticated fence implementation. >>>>>>> >>>>>>> Your idea of not using a list, but 64 bit sequences instead actually >>>>>>> sounds >>>>>>> quite nifty to me. Going to hack something together in the next cou= ple >>>>>>> of >>>>>>> hours. >>>>>>> >>>>>>> Christian. >>>>>> Btw you said that you are having issue when using multiple ring, it >>>>>> comes to my attention that you never sync with the GFX ring (unless >>>>>> asked by userspace) that's wrong, before scheduling on another ring >>>>>> than GFX index you need to emit semaphore to make the ring wait for >>>>>> the last emited fence on the GFX ring because of ttm. What might >>>>>> happen is that ttm scheduled bo move on the GFX ring and that the ri= ng >>>>>> you work on start using those bo at there soon to be GPU offset while >>>>>> the bo data is not there yet. >>>>> Yeah, already stumbled over that but IIRC Alex already solved that >>>>> issue.. >>>>> >>>>> >>>>> We set the sync_obj to the fence of the move operation, so when there= is >>>>> a >>>>> move operation in progress we sync to it correctly (at least I hope s= o). >>>>> >>>>> Christian. >>>>> >>>> Looking at code doesn't seems ok, yes you use the fence from the move >>>> operation but you only emit wait if userspace ask for it, that's >>>> wrong. >>>> >>>> if (!(p->relocs[i].flags& RADEON_RELOC_DONT_SYNC)) { >>>> >>> The default is to sync all the rings. We only skip the sync if the >>> _DONT_ sync flag is set. >> Yeah, but the _DONT_ sync flag is just an optimization, and we only want= to >> not sync to things userspace has submitted. >> >> E.g. userspace tells us that two jobs can happily run on the same bo e= ven >> if they are both writing to it.... >> >> So Jerome is completely right, userspace doesn't expect that TTM is jump= ing >> in between and moving the bo around, that is indeed a bug and another th= ing >> for the todo list. >> >> Christian. > Well until userspace can tell kernel on which fence it wants to wait i > believe this flags become obsolete and can be remove, i am pretty no > release userspace code ever used that flags. Agree, AFAIK it currently isn't used anyway. Christian.