2012/5/3 Christian König <deathsimple@xxxxxxxxxxx>: > On 03.05.2012 19:20, Alex Deucher wrote: >> >> 2012/5/3 Jerome Glisse<j.glisse@xxxxxxxxx>: >>> >>> On Thu, May 3, 2012 at 12:39 PM, Christian König >>> <deathsimple@xxxxxxxxxxx> wrote: >>>> >>>> On 03.05.2012 18:32, Jerome Glisse wrote: >>>>> >>>>> On Thu, May 3, 2012 at 4:19 AM, Christian >>>>> König<deathsimple@xxxxxxxxxxx> >>>>> wrote: >>>>>> >>>>>> On 02.05.2012 18:01, Jerome Glisse wrote: >>>>>>> >>>>>>> On Wed, May 2, 2012 at 9:11 AM, Christian >>>>>>> König<deathsimple@xxxxxxxxxxx> >>>>>>> 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 changes, >>>>>>>> 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-rework. >>>>>>>> >>>>>>>> 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 couple >>>>>> 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 ring >>>>> 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 so). >>>> >>>> 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 even > if they are both writing to it.... > > So Jerome is completely right, userspace doesn't expect that TTM is jumping > in between and moving the bo around, that is indeed a bug and another thing > for the todo list. Ah, yes, that makes sense. I misinterpreted the original comment. Alex _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel