All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gustavo Padovan <gustavo@padovan.org>
To: Daniel Stone <daniel@fooishbar.org>
Cc: "Daniel Stone" <daniels@collabora.com>,
	"Arve Hjønnevåg" <arve@android.com>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	"Riley Andrews" <riandrews@android.com>,
	"Gustavo Padovan" <gustavo.padovan@collabora.co.uk>,
	"John Harrison" <John.C.Harrison@intel.com>
Subject: Re: [RFC v2 5/8] drm/fence: add in-fences support
Date: Thu, 28 Apr 2016 11:36:44 -0300	[thread overview]
Message-ID: <20160428143644.GA3496@joana> (raw)
In-Reply-To: <CAPj87rOfhDr14xFCH1A+BM3u_EfS66cmSTXcs32PUJrhH8nfLg@mail.gmail.com>

2016-04-27 Daniel Stone <daniel@fooishbar.org>:

> Hi,
> 
> On 26 April 2016 at 21:48, Greg Hackmann <ghackmann@google.com> wrote:
> > On 04/26/2016 01:05 PM, Daniel Vetter wrote:
> >> On Tue, Apr 26, 2016 at 09:55:06PM +0300, Ville Syrjälä wrote:
> >>> What are they doing that can't stuff the fences into an array
> >>> instead of props?
> >>
> >> The hw composer interface is one in-fence per plane. That's really the
> >> major reason why the kernel interface is built to match. And I really
> >> don't think we should diverge just because we have a slight different
> >> color preference ;-)
> >
> > The relationship between layers and fences is only fuzzy and indirect
> > though.  The relationship is really between the buffer you're displaying on
> > that layer, and the fence representing the work done to render into that
> > buffer.  SurfaceFlinger just happens to bundle them together inside the same
> > struct hwc_layer_1 as an API convenience.
> 
> Right, and when using implicit fencing, this comes as a plane
> property, by virtue of plane -> fb -> buffer -> fence.
> 
> > Which is kind of splitting hairs as long as you have a 1-to-1 relationship
> > between layers and DRM planes.  But that's not always the case.
> 
> Can you please elaborate?
> 
> > A (per-CRTC?) array of fences would be more flexible.  And even in the cases
> > where you could make a 1-to-1 mapping between planes and fences, it's not
> > that much more work for userspace to assemble those fences into an array
> > anyway.
> 
> As Ville says, I don't want to go down the path of scheduling CRTC
> updates separately, because that breaks MST pretty badly. If you don't
> want your updates to display atomically, then don't schedule them
> atomically ... ? That's the only reason I can see for making fencing
> per-CRTC, rather than just a pile of unassociated fences appended to
> the request. Per-CRTC fences also forces userspace to merge fences
> before submission when using multiple planes per CRTC, which is pretty
> punitive.
> 
> I think having it semantically attached to the plane is a little bit
> nicer for tracing (why was this request delayed? -> a fence -> which
> buffer was that fence for?) at a glance. Also the 'pile of appended
> fences' model is a bit awkward for more generic userspace, which
> creates a libdrm request and builds it (add a plane, try it out, wind
> back) incrementally. Using properties makes that really easy, but
> without properties, we'd have to add separate codepaths - and thus
> separate ABI, which complicates distribution - to libdrm to account
> for a separate plane array which shares a cursor with the properties.
> So for that reason if none other, I'd really prefer not to go down
> that route.

I also agree to have it as FENCE_FD prop on the plane. Summarizing the
arguments on this thread, they are:

 - implicit fences also needs one fence per plane/fb, so it will be good to     
   match with that.                                                             
 - requires userspace to always merge fences                                    
 - can use standard plane properties, making kernel and userspace life easier,  
   an array brings more work to build the atomic request plus extra checkings   
   on the kernel.                                                               
 - do not need to changes to drivers                                            
 - better for tracing, can identify the buffer/fence promptly

	Gustavo
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Gustavo Padovan <gustavo@padovan.org>
To: Daniel Stone <daniel@fooishbar.org>
Cc: "Greg Hackmann" <ghackmann@google.com>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Gustavo Padovan" <gustavo.padovan@collabora.co.uk>,
	"Daniel Stone" <daniels@collabora.com>,
	"Riley Andrews" <riandrews@android.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Arve Hjønnevåg" <arve@android.com>,
	"John Harrison" <John.C.Harrison@intel.com>
Subject: Re: [RFC v2 5/8] drm/fence: add in-fences support
Date: Thu, 28 Apr 2016 11:36:44 -0300	[thread overview]
Message-ID: <20160428143644.GA3496@joana> (raw)
In-Reply-To: <CAPj87rOfhDr14xFCH1A+BM3u_EfS66cmSTXcs32PUJrhH8nfLg@mail.gmail.com>

2016-04-27 Daniel Stone <daniel@fooishbar.org>:

> Hi,
> 
> On 26 April 2016 at 21:48, Greg Hackmann <ghackmann@google.com> wrote:
> > On 04/26/2016 01:05 PM, Daniel Vetter wrote:
> >> On Tue, Apr 26, 2016 at 09:55:06PM +0300, Ville Syrjälä wrote:
> >>> What are they doing that can't stuff the fences into an array
> >>> instead of props?
> >>
> >> The hw composer interface is one in-fence per plane. That's really the
> >> major reason why the kernel interface is built to match. And I really
> >> don't think we should diverge just because we have a slight different
> >> color preference ;-)
> >
> > The relationship between layers and fences is only fuzzy and indirect
> > though.  The relationship is really between the buffer you're displaying on
> > that layer, and the fence representing the work done to render into that
> > buffer.  SurfaceFlinger just happens to bundle them together inside the same
> > struct hwc_layer_1 as an API convenience.
> 
> Right, and when using implicit fencing, this comes as a plane
> property, by virtue of plane -> fb -> buffer -> fence.
> 
> > Which is kind of splitting hairs as long as you have a 1-to-1 relationship
> > between layers and DRM planes.  But that's not always the case.
> 
> Can you please elaborate?
> 
> > A (per-CRTC?) array of fences would be more flexible.  And even in the cases
> > where you could make a 1-to-1 mapping between planes and fences, it's not
> > that much more work for userspace to assemble those fences into an array
> > anyway.
> 
> As Ville says, I don't want to go down the path of scheduling CRTC
> updates separately, because that breaks MST pretty badly. If you don't
> want your updates to display atomically, then don't schedule them
> atomically ... ? That's the only reason I can see for making fencing
> per-CRTC, rather than just a pile of unassociated fences appended to
> the request. Per-CRTC fences also forces userspace to merge fences
> before submission when using multiple planes per CRTC, which is pretty
> punitive.
> 
> I think having it semantically attached to the plane is a little bit
> nicer for tracing (why was this request delayed? -> a fence -> which
> buffer was that fence for?) at a glance. Also the 'pile of appended
> fences' model is a bit awkward for more generic userspace, which
> creates a libdrm request and builds it (add a plane, try it out, wind
> back) incrementally. Using properties makes that really easy, but
> without properties, we'd have to add separate codepaths - and thus
> separate ABI, which complicates distribution - to libdrm to account
> for a separate plane array which shares a cursor with the properties.
> So for that reason if none other, I'd really prefer not to go down
> that route.

I also agree to have it as FENCE_FD prop on the plane. Summarizing the
arguments on this thread, they are:

 - implicit fences also needs one fence per plane/fb, so it will be good to     
   match with that.                                                             
 - requires userspace to always merge fences                                    
 - can use standard plane properties, making kernel and userspace life easier,  
   an array brings more work to build the atomic request plus extra checkings   
   on the kernel.                                                               
 - do not need to changes to drivers                                            
 - better for tracing, can identify the buffer/fence promptly

	Gustavo

  reply	other threads:[~2016-04-28 14:36 UTC|newest]

Thread overview: 90+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-25 22:33 [RFC v2 0/8] drm: explicit fencing support Gustavo Padovan
2016-04-25 22:33 ` Gustavo Padovan
2016-04-25 22:33 ` [RFC v2 1/8] dma-buf/fence: add fence_collection fences Gustavo Padovan
2016-04-25 22:33   ` Gustavo Padovan
2016-04-26 14:41   ` Daniel Vetter
2016-04-26 14:41     ` Daniel Vetter
2016-04-26 15:02     ` Gustavo Padovan
2016-04-27  6:36       ` Daniel Vetter
2016-04-27  6:36         ` Daniel Vetter
2016-04-26 15:09   ` Chris Wilson
2016-04-26 15:09     ` Chris Wilson
2016-04-28 14:47     ` Gustavo Padovan
2016-04-28 14:47       ` Gustavo Padovan
2016-04-25 22:33 ` [RFC v2 2/8] Documentation: add fence-collection to kernel DocBook Gustavo Padovan
2016-04-25 22:33   ` Gustavo Padovan
2016-04-25 22:33 ` [RFC v2 3/8] dma-buf/sync_file: add sync_file_fences_get() Gustavo Padovan
2016-04-25 22:33   ` Gustavo Padovan
2016-04-25 22:33 ` [RFC v2 4/8] drm/fence: allow fence waiting to be interrupted by userspace Gustavo Padovan
2016-04-25 22:33   ` Gustavo Padovan
2016-04-25 22:33 ` [RFC v2 5/8] drm/fence: add in-fences support Gustavo Padovan
2016-04-25 22:33   ` Gustavo Padovan
2016-04-26 10:10   ` Ville Syrjälä
2016-04-26 10:10     ` Ville Syrjälä
2016-04-26 14:14     ` Gustavo Padovan
2016-04-26 14:36       ` Daniel Vetter
2016-04-26 14:36         ` Daniel Vetter
2016-04-26 16:26         ` Ville Syrjälä
2016-04-26 16:26           ` Ville Syrjälä
2016-04-26 17:20           ` Daniel Vetter
2016-04-26 17:20             ` Daniel Vetter
2016-04-26 17:40             ` Ville Syrjälä
2016-04-26 18:23               ` Daniel Vetter
2016-04-26 18:23                 ` Daniel Vetter
2016-04-26 18:55                 ` Ville Syrjälä
2016-04-26 18:55                   ` Ville Syrjälä
2016-04-26 20:05                   ` Daniel Vetter
2016-04-26 20:48                     ` Greg Hackmann
2016-04-26 20:48                       ` Greg Hackmann
2016-04-27  6:39                       ` Daniel Vetter
2016-04-27  6:39                         ` Daniel Vetter
2016-04-28 21:28                         ` Rob Clark
2016-04-28 21:28                           ` Rob Clark
2016-04-29  7:48                           ` Daniel Stone
2016-04-29  7:48                             ` Daniel Stone
2016-04-29 22:23                             ` Rob Clark
2016-04-29 22:23                               ` Rob Clark
2016-07-12 21:14                               ` Dominik Behr
2016-07-12 21:21                                 ` Gustavo Padovan
2016-04-29 21:14                         ` Greg Hackmann
2016-04-29 21:14                           ` Greg Hackmann
2016-04-27  6:57                       ` Daniel Stone
2016-04-27  6:57                         ` Daniel Stone
2016-04-28 14:36                         ` Gustavo Padovan [this message]
2016-04-28 14:36                           ` Gustavo Padovan
2016-04-28 14:38                           ` Daniel Vetter
2016-04-28 14:38                             ` Daniel Vetter
2016-04-28 16:56                           ` Ville Syrjälä
2016-04-28 16:56                             ` Ville Syrjälä
2016-04-28 17:43                             ` Daniel Vetter
2016-04-28 17:43                               ` Daniel Vetter
2016-04-28 17:51                               ` Ville Syrjälä
2016-04-28 17:51                                 ` Ville Syrjälä
2016-04-28 17:55                                 ` Gustavo Padovan
2016-04-28 18:02                                   ` Daniel Vetter
2016-04-28 18:02                                     ` Daniel Vetter
2016-04-28 18:17                             ` Ville Syrjälä
2016-04-28 18:17                               ` Ville Syrjälä
2016-04-28 20:40                               ` Daniel Vetter
2016-04-28 20:40                                 ` Daniel Vetter
2016-04-26 16:25       ` Ville Syrjälä
2016-04-25 22:33 ` [RFC v2 6/8] drm/fence: add fence to drm_pending_event Gustavo Padovan
2016-04-25 22:33   ` Gustavo Padovan
2016-04-25 22:33 ` [RFC v2 7/8] drm/fence: add fence timeline to drm_crtc Gustavo Padovan
2016-04-25 22:33   ` Gustavo Padovan
2016-04-26 10:12   ` Ville Syrjälä
2016-04-26 10:12     ` Ville Syrjälä
2016-04-26 14:23     ` Gustavo Padovan
2016-04-26 16:34       ` Ville Syrjälä
2016-04-26 16:34         ` Ville Syrjälä
2016-04-27  8:23   ` Daniel Stone
2016-04-27  8:23     ` Daniel Stone
2016-04-25 22:33 ` [RFC v2 8/8] drm/fence: add out-fences support Gustavo Padovan
2016-04-25 22:33   ` Gustavo Padovan
2016-04-26 14:53   ` Daniel Vetter
2016-04-26 14:53     ` Daniel Vetter
2016-04-28 15:23     ` Gustavo Padovan
2016-04-28 15:23       ` Gustavo Padovan
2016-04-25 23:21 ` [RFC v2 0/8] drm: explicit fencing support Mike Lothian
2016-04-26  6:30   ` Daniel Vetter
2016-04-26  6:30     ` Daniel Vetter

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=20160428143644.GA3496@joana \
    --to=gustavo@padovan.org \
    --cc=John.C.Harrison@intel.com \
    --cc=arve@android.com \
    --cc=daniel@fooishbar.org \
    --cc=daniels@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gustavo.padovan@collabora.co.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=riandrews@android.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.