dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jerome Glisse <j.glisse@gmail.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: dri-devel <dri-devel@lists.freedesktop.org>,
	Ben Skeggs <bskeggs@redhat.com>
Subject: Re: Fence, timeline and android sync points
Date: Fri, 15 Aug 2014 10:53:45 -0400	[thread overview]
Message-ID: <20140815145344.GB1910@gmail.com> (raw)
In-Reply-To: <20140815080739.GN10500@phenom.ffwll.local>

On Fri, Aug 15, 2014 at 10:07:39AM +0200, Daniel Vetter wrote:
> On Thu, Aug 14, 2014 at 07:03:44PM -0400, Jerome Glisse wrote:
> > On Thu, Aug 14, 2014 at 11:23:01PM +0200, Daniel Vetter wrote:
> > > On Thu, Aug 14, 2014 at 9:15 PM, Jerome Glisse <j.glisse@gmail.com> wrote:
> > > > Cost 1 uint32 per buffer and simple if without locking to check status of
> > > > a buffer.
> > > 
> > > Yeah well except it doesn't and that's why we switch to full blown
> > > fence objects internally instead of smacking seqno values all over the
> > > place. At least in i915 because I have seen what this "simple"
> > > solution looks like if you throw all the complexities into the bin and
> > > mix it well.
> > 
> > I am kind of curious on what can go wrong here ? Unless you have thousands
> > of different hw block inside your gpu all of them with different cmd queue
> > then i do not see any issue.
> 
> See my other reply, but because we'll schedule software contexts here and
> we need to do it with implicit fencing because we can't just break the
> existing world. The below is pretty much the design we currently have.

Software need the object to be binded to the gpu gart on intel ? That's bad.
But even with such thing, you can do it as a secondary lock only for user
space.

> -Daniel
> 
> > 
> > Note that this global seqno i am talking is only useful for bind/unbind of
> > buffer in ideal world of explicit sync, not for synchronization btw command
> > buffer. So pseudo code would be :
> > 
> > // if buffer_can_unbind(buf, dev) return true then buffer is no longer in
> > // use and can be unbind. if false you can wait on the device wait queue.
> > bool buffer_can_unbind(buf, dev)
> > {
> >   // Is the last gseqno associated with buffer is smaller than current
> >   // smallest signaled seqno ?
> >   if (buf->gseqno <= dev->gseqno_cmin)
> >     return true;
> >   hw_update_gseqno_min(dev);
> >   if (buf->gseqno <= dev->gseqno_cmin)
> >     return true;
> >   for_each_hw_block(hwblock, dev) {
> >     // If that hwblock has not signaled a gseqno bigger than the
> >     // buffer one's and also has work scheduled that might be using
> >     // the buffer (ie the last scheduled gseqno is greater than
> >     // buffer gseqno). If that's true than buffer might still be
> >     // in use so assume the worst.
> >     if (hwblock->gseqno < buf->gseqno &&
> >         hwblock->gseqno_last_scheduled >= buf->gseqno)
> >       return false;
> >     // This means either this block is already past the buf->gseqno
> >     // or that this block have nothing in its pipeline that will ever
> >     // use buf.
> >     // As an extra optimization one might add a hwblock mask to buf
> >     // and unmask wait for that hwblock so further wait will not wait
> >     // on this block as we know for sure it's not involve.
> >   }
> >   // Well none of the hwblock is still using that buffer.
> >   return true;
> > }
> > 
> > hw_update_gseqno_min(dev)
> > {
> >    unsigned nmin = -1;
> > 
> >    for_each_hw_block(hwblock, dev) {
> >      nmin = min(nmin, hwblock->gseqno);
> >    }
> >    // atomic exchange and compage set new min only if it's bigger then
> >    // current min
> >    atomic_cmp_xchg(dev->gseqno_cmin, nmin)
> > }
> > 
> > 
> > So this is how it looks in my mind, each hwblock write to there dedicated
> > >gseqno and for each hwblock you store the last gseqno that was scheduled.
> > There is no need for any lock and this is outside any other sync mecanism.
> > 
> > For hw with preemption, i assume you have then have a hwcontext, well you
> > use same code except that now you have a gseqno per context which means
> > you need to store a seqno per context per buffer. With some trickery this
> > might be avoided especialy if hw can do atomic cmp_xchg.
> > 
> > Cheers,
> > Jérôme
> > 
> > 
> > > -Daniel
> > > -- 
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

  reply	other threads:[~2014-08-15 14:53 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-12 22:13 Fence, timeline and android sync points Jerome Glisse
2014-08-13  1:23 ` Jerome Glisse
2014-08-13  7:59 ` Christian König
2014-08-13 13:41   ` Jerome Glisse
2014-08-13 14:08     ` Christian König
2014-08-13 15:56       ` Jerome Glisse
2014-08-13  8:28 ` Daniel Vetter
2014-08-13 13:36   ` Jerome Glisse
2014-08-13 15:54     ` Daniel Vetter
2014-08-13 17:07       ` Jerome Glisse
2014-08-14  9:08         ` Daniel Vetter
2014-08-14 14:23           ` Jerome Glisse
2014-08-14 15:55             ` Daniel Vetter
2014-08-14 18:18               ` Jerome Glisse
2014-08-14 18:47                 ` Daniel Vetter
2014-08-14 19:15                   ` Jerome Glisse
2014-08-14 19:40                     ` Maarten Lankhorst
2014-08-14 19:56                       ` Jerome Glisse
2014-08-14 21:20                         ` Daniel Vetter
2014-08-14 21:23                     ` Daniel Vetter
2014-08-14 23:03                       ` Jerome Glisse
2014-08-15  8:07                         ` Daniel Vetter
2014-08-15 14:53                           ` Jerome Glisse [this message]
2014-08-14 21:30                     ` Daniel Vetter
2014-08-15  6:54                     ` Thomas Hellstrom
2014-08-15 14:52                       ` Jerome Glisse
2014-08-16  7:01                         ` Thomas Hellstrom
2014-08-16 15:30                           ` Jerome Glisse
2014-08-14  9:15         ` Maarten Lankhorst
2014-08-14 11:53           ` Christian König
2014-08-14 12:37             ` Maarten Lankhorst
2014-08-14 14:31               ` Christian König
2014-08-14 14:09           ` Jerome Glisse
2014-08-14 13:16         ` Rob Clark
2014-08-14 14:12           ` Jerome Glisse
2014-08-14 15:58             ` Daniel Vetter
2014-08-14 18:26               ` Jerome Glisse
2014-08-14 19:16                 ` Maarten Lankhorst
2014-08-14 22:11             ` Rob Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140815145344.GB1910@gmail.com \
    --to=j.glisse@gmail.com \
    --cc=bskeggs@redhat.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox