Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753531AbcDZSXz (ORCPT ); Tue, 26 Apr 2016 14:23:55 -0400 Received: from mail-wm0-f51.google.com ([74.125.82.51]:37049 "EHLO mail-wm0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753382AbcDZSXw (ORCPT ); Tue, 26 Apr 2016 14:23:52 -0400 Date: Tue, 26 Apr 2016 20:23:46 +0200 From: Daniel Vetter To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: Gustavo Padovan , Gustavo Padovan , Daniel Stone , Riley Andrews , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Arve =?iso-8859-1?B?SGr4bm5lduVn?= , John Harrison Subject: Re: [RFC v2 5/8] drm/fence: add in-fences support Message-ID: <20160426182346.GC2558@phenom.ffwll.local> Mail-Followup-To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , Gustavo Padovan , Gustavo Padovan , Daniel Stone , Riley Andrews , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Arve =?iso-8859-1?B?SGr4bm5lduVn?= , John Harrison References: <1461623608-29538-1-git-send-email-gustavo@padovan.org> <1461623608-29538-6-git-send-email-gustavo@padovan.org> <20160426101050.GN4329@intel.com> <20160426141422.GG7857@joana> <20160426143635.GW8291@phenom.ffwll.local> <20160426162621.GU4329@intel.com> <20160426172049.GB2558@phenom.ffwll.local> <20160426174045.GC4329@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20160426174045.GC4329@intel.com> X-Operating-System: Linux phenom 4.6.0-rc5+ User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3344 Lines: 71 On Tue, Apr 26, 2016 at 08:40:45PM +0300, Ville Syrj?l? wrote: > On Tue, Apr 26, 2016 at 07:20:49PM +0200, Daniel Vetter wrote: > > On Tue, Apr 26, 2016 at 07:26:21PM +0300, Ville Syrj?l? wrote: > > > On Tue, Apr 26, 2016 at 04:36:36PM +0200, Daniel Vetter wrote: > > > > On Tue, Apr 26, 2016 at 11:14:22AM -0300, Gustavo Padovan wrote: > > > > > 2016-04-26 Ville Syrj?l? : > > > > > > > > > > > On Mon, Apr 25, 2016 at 07:33:25PM -0300, Gustavo Padovan wrote: > > > > > > > From: Gustavo Padovan > > > > > > > > > > > > > > There is now a new property called FENCE_FD attached to every plane > > > > > > > state that receives the sync_file fd from userspace via the atomic commit > > > > > > > IOCTL. > > > > > > > > > > > > I still don't like this property abuse. Also with atomic, all passed > > > > > > fences must be waited upon before anything is done, so attaching them > > > > > > to planes seems like it might just give people the wrong idea. > > > > > > > > > > I'm actually fine with this as property, but another solutions is use > > > > > an array of {plane, fence_fd} and extend drm_atomic_ioctl args just like > > > > > we have done for out fences. However the FENCE_FD property is easier to > > > > > handle in userspace than the array. Any other idea? > > > > > > > > Imo FENCE_FD is perfectly fine. But what's the concern around giving > > > > people the wrong idea with attaching fences to planes? For nonblocking > > > > commits we need to store them somewhere for the worker, drm_plane_state > > > > seems like an as good place as any other. > > > > > > It gives the impression that each plane might flip as soon as its fence > > > signals. > > > > That wouldn't be atomic. Not sure how someone could come up with that > > idea. > > What else would it mean? It's attached to a specific plane, so why would > it affect other planes? > > > I mean we could move FENCE_FD to the crtc (fence fds can be merged), > > but that's just a needless difference to what hwc expects. I think > > aligning with the only real-world users in this case here makes sense. > > Well it doesn't belong on the crtc either. I would just stick in the > ioctl as a separate thing, then it's clear it's related to the whole > operation rather than any kms object. We want it per-crtc I'd say, so that you could flip each crtc individually. But really the reason for per-plane is hw composer from Android. I don't see any point in designing an api that's needlessly different from what the main user expects (even if it may be silly). The other bit is that for implicit syncing you need one fence per fb/plane anyway, so this also fits nicely on the driver side I think. > > Plus docs in case someone has funny ideas. > > Weren't you just quoting rusty's API manifesto recently? ;) I quote it all the time. http://sweng.the-davies.net/Home/rustys-api-design-manifesto I think current interface is scoring pretty high since as part of Gustavo's work there's no also a new atomic helper which will get the waiting right. There's still the problem that neither for drm_event nor the fence do we have anything idiot-proof. So for that the solution is testcases (which are also happening). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch