Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1516885 > unrolled thread

[PATCH v7 0/3] drm: add explict fencing

Back to article view | Back to linux.kernel

Contents

  [PATCH v7 0/3] drm: add explict fencing  Gustavo Padovan <gustavo@padovan.org> - 2016-11-08 08:00 +0100
    Re: [PATCH v7 0/3] drm: add explict fencing Chris Wilson <chris@chris-wilson.co.uk> - 2016-11-08 12:00 +0100
      Re: [PATCH v7 0/3] drm: add explict fencing Daniel Vetter <daniel@ffwll.ch> - 2016-11-08 12:40 +0100
        Re: [PATCH v7 0/3] drm: add explict fencing Chris Wilson <chris@chris-wilson.co.uk> - 2016-11-08 12:50 +0100
          Re: [PATCH v7 0/3] drm: add explict fencing Daniel Vetter <daniel@ffwll.ch> - 2016-11-08 13:50 +0100
            Re: [PATCH v7 0/3] drm: add explict fencing Chris Wilson <chris@chris-wilson.co.uk> - 2016-11-08 14:00 +0100
          Re: [PATCH v7 0/3] drm: add explict fencing Daniel Vetter <daniel@ffwll.ch> - 2016-11-08 14:00 +0100
    Re: [PATCH v7 0/3] drm: add explict fencing Daniel Vetter <daniel@ffwll.ch> - 2016-11-08 14:20 +0100

#1516885 — [PATCH v7 0/3] drm: add explict fencing

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Hi,

This is yet another version of the DRM fences patches. Please refer
to the cover letter[1] in a previous version to check for more details.

In v7 we now have split most of the out_fences code into
prepare_crtc_signaling() and unprepare_crtc_signaling() with improved error
handling. More details on the v7 changes are embedded in each commit's
message.

Robert Foss managed to port Android's drm_hwcomposer to the new HWC2 API and
added support to fences. Current patches can be seen here:

https://git.collabora.com/cgit/user/robertfoss/drm_hwcomposer.git/log/?h=hwc2_fence_v1

He managed to run AOSP on top of padovan/fences kernel branch with full fence
support on qemu/virgl and msm db410c. That means we already have a working
open source userspace using the explicit fencing implementation.

Also i-g-t testing are available at:

https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/log/

Please review!

Gustavo

[1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1253822.html

---
Gustavo Padovan (3):
  drm/fence: add in-fences support
  drm/fence: add fence timeline to drm_crtc
  drm/fence: add out-fences support

 drivers/gpu/drm/Kconfig             |   1 +
 drivers/gpu/drm/drm_atomic.c        | 247 ++++++++++++++++++++++++++++++------
 drivers/gpu/drm/drm_atomic_helper.c |   3 +
 drivers/gpu/drm/drm_crtc.c          |  45 +++++++
 drivers/gpu/drm/drm_plane.c         |   1 +
 include/drm/drm_atomic.h            |   1 +
 include/drm/drm_crtc.h              |  55 ++++++++
 7 files changed, 311 insertions(+), 42 deletions(-)

-- 
2.5.5

[toc] | [next] | [standalone]

#1517070

On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Hi,
> 
> This is yet another version of the DRM fences patches. Please refer
> to the cover letter[1] in a previous version to check for more details.

Explicit fencing is not a superset of the implicit fences. The driver
may be using implicit fences (on a reservation object) to serialise
asynchronous operations wrt to each other (such as dispatching threads
to flush cpu caches to memory, manipulating page tables and the like
before the flip).  Since the user doesn't know about these operations,
they are not included in the explicit fence they provide, at which point
we can't trust their fence to the exclusion of the implicit fences...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[toc] | [prev] | [next] | [standalone]

#1517098

On Tue, Nov 08, 2016 at 10:35:08AM +0000, Chris Wilson wrote:
> On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > 
> > Hi,
> > 
> > This is yet another version of the DRM fences patches. Please refer
> > to the cover letter[1] in a previous version to check for more details.
> 
> Explicit fencing is not a superset of the implicit fences. The driver
> may be using implicit fences (on a reservation object) to serialise
> asynchronous operations wrt to each other (such as dispatching threads
> to flush cpu caches to memory, manipulating page tables and the like
> before the flip).  Since the user doesn't know about these operations,
> they are not included in the explicit fence they provide, at which point
> we can't trust their fence to the exclusion of the implicit fences...

My thoughts are that in atomic_check drivers just fill in the fence from
the reservation_object (i.e. the uapi implicit fencing part). If there's
any additional work that's queued up in ->prepare_fb then I guess the
driver needs to track that internally, but _only_ for kernel-internally
queued work.

The reason for that is that with explicit fencing we want to allow
userspace to overwrite any existing implicit fences that might hang
around.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1517107

On Tue, Nov 08, 2016 at 12:32:56PM +0100, Daniel Vetter wrote:
> On Tue, Nov 08, 2016 at 10:35:08AM +0000, Chris Wilson wrote:
> > On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote:
> > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > > 
> > > Hi,
> > > 
> > > This is yet another version of the DRM fences patches. Please refer
> > > to the cover letter[1] in a previous version to check for more details.
> > 
> > Explicit fencing is not a superset of the implicit fences. The driver
> > may be using implicit fences (on a reservation object) to serialise
> > asynchronous operations wrt to each other (such as dispatching threads
> > to flush cpu caches to memory, manipulating page tables and the like
> > before the flip).  Since the user doesn't know about these operations,
> > they are not included in the explicit fence they provide, at which point
> > we can't trust their fence to the exclusion of the implicit fences...
> 
> My thoughts are that in atomic_check drivers just fill in the fence from
> the reservation_object (i.e. the uapi implicit fencing part). If there's
> any additional work that's queued up in ->prepare_fb then I guess the
> driver needs to track that internally, but _only_ for kernel-internally
> queued work.

That's not a trivial task to work out which of the fence contexts within
the reservation object are required and which are to be replaced by the
explicit fence, esp. when you have to consider external fences.
 
> The reason for that is that with explicit fencing we want to allow
> userspace to overwrite any existing implicit fences that might hang
> around.

I'm just suggesting the danger of that when userspace doesn't know
everything and the current interfaces do not allow for userspace to know,
we only tell userspace about its own action (more or less).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[toc] | [prev] | [next] | [standalone]

#1517140

On Tue, Nov 08, 2016 at 11:45:51AM +0000, Chris Wilson wrote:
> On Tue, Nov 08, 2016 at 12:32:56PM +0100, Daniel Vetter wrote:
> > On Tue, Nov 08, 2016 at 10:35:08AM +0000, Chris Wilson wrote:
> > > On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote:
> > > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > > > 
> > > > Hi,
> > > > 
> > > > This is yet another version of the DRM fences patches. Please refer
> > > > to the cover letter[1] in a previous version to check for more details.
> > > 
> > > Explicit fencing is not a superset of the implicit fences. The driver
> > > may be using implicit fences (on a reservation object) to serialise
> > > asynchronous operations wrt to each other (such as dispatching threads
> > > to flush cpu caches to memory, manipulating page tables and the like
> > > before the flip).  Since the user doesn't know about these operations,
> > > they are not included in the explicit fence they provide, at which point
> > > we can't trust their fence to the exclusion of the implicit fences...
> > 
> > My thoughts are that in atomic_check drivers just fill in the fence from
> > the reservation_object (i.e. the uapi implicit fencing part). If there's
> > any additional work that's queued up in ->prepare_fb then I guess the
> > driver needs to track that internally, but _only_ for kernel-internally
> > queued work.
> 
> That's not a trivial task to work out which of the fence contexts within
> the reservation object are required and which are to be replaced by the
> explicit fence, esp. when you have to consider external fences.

Hm, what kind of async kernel tasks are you thinking off? Atm I don't know
of anyone who does e.g. clflush through the gpu. And ttm bo placement
moves for display should be explicit enough that drivers will deal with
them correctly. At least that seems to have been the conclusion from the
long amdgpu thread.

> > The reason for that is that with explicit fencing we want to allow
> > userspace to overwrite any existing implicit fences that might hang
> > around.
> 
> I'm just suggesting the danger of that when userspace doesn't know
> everything and the current interfaces do not allow for userspace to know,
> we only tell userspace about its own action (more or less).

tools for fools, but yes userspace is expected to get this 100% right (for
any userspace-issued cs at least), and eat the fallout if it doesn't.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1517144

On Tue, Nov 08, 2016 at 01:43:40PM +0100, Daniel Vetter wrote:
> On Tue, Nov 08, 2016 at 11:45:51AM +0000, Chris Wilson wrote:
> > On Tue, Nov 08, 2016 at 12:32:56PM +0100, Daniel Vetter wrote:
> > > On Tue, Nov 08, 2016 at 10:35:08AM +0000, Chris Wilson wrote:
> > > > On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote:
> > > > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > This is yet another version of the DRM fences patches. Please refer
> > > > > to the cover letter[1] in a previous version to check for more details.
> > > > 
> > > > Explicit fencing is not a superset of the implicit fences. The driver
> > > > may be using implicit fences (on a reservation object) to serialise
> > > > asynchronous operations wrt to each other (such as dispatching threads
> > > > to flush cpu caches to memory, manipulating page tables and the like
> > > > before the flip).  Since the user doesn't know about these operations,
> > > > they are not included in the explicit fence they provide, at which point
> > > > we can't trust their fence to the exclusion of the implicit fences...
> > > 
> > > My thoughts are that in atomic_check drivers just fill in the fence from
> > > the reservation_object (i.e. the uapi implicit fencing part). If there's
> > > any additional work that's queued up in ->prepare_fb then I guess the
> > > driver needs to track that internally, but _only_ for kernel-internally
> > > queued work.
> > 
> > That's not a trivial task to work out which of the fence contexts within
> > the reservation object are required and which are to be replaced by the
> > explicit fence, esp. when you have to consider external fences.
> 
> Hm, what kind of async kernel tasks are you thinking off? Atm I don't know
> of anyone who does e.g. clflush through the gpu. And ttm bo placement
> moves for display should be explicit enough that drivers will deal with
> them correctly. At least that seems to have been the conclusion from the
> long amdgpu thread.

Now that we (i915) serialise on an reservation_object (obj->resv), we
have floated ideas to use that to serialise async tasks (such as
offloading the 100ms clflush to a (cpu) worker, a gpu task would pose a
similar problem with a fence inserted that is not exposed to userspace).
Also tempted to look at using async tasks + fences to do GTT updates
but that is not a common pain point at the moment, and cases where it
is the GTT thrashing itself is the issue.

So how does i915 deal with ttm bo fences?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[toc] | [prev] | [next] | [standalone]

#1517145

On Tue, Nov 08, 2016 at 01:44:34PM +0100, Christian König wrote:
> Am 08.11.2016 um 12:45 schrieb Chris Wilson:
> > On Tue, Nov 08, 2016 at 12:32:56PM +0100, Daniel Vetter wrote:
> > > On Tue, Nov 08, 2016 at 10:35:08AM +0000, Chris Wilson wrote:
> > > > On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote:
> > > > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > This is yet another version of the DRM fences patches. Please refer
> > > > > to the cover letter[1] in a previous version to check for more details.
> > > > Explicit fencing is not a superset of the implicit fences. The driver
> > > > may be using implicit fences (on a reservation object) to serialise
> > > > asynchronous operations wrt to each other (such as dispatching threads
> > > > to flush cpu caches to memory, manipulating page tables and the like
> > > > before the flip).  Since the user doesn't know about these operations,
> > > > they are not included in the explicit fence they provide, at which point
> > > > we can't trust their fence to the exclusion of the implicit fences...
> > > My thoughts are that in atomic_check drivers just fill in the fence from
> > > the reservation_object (i.e. the uapi implicit fencing part). If there's
> > > any additional work that's queued up in ->prepare_fb then I guess the
> > > driver needs to track that internally, but _only_ for kernel-internally
> > > queued work.
> > That's not a trivial task to work out which of the fence contexts within
> > the reservation object are required and which are to be replaced by the
> > explicit fence, esp. when you have to consider external fences.
> > > The reason for that is that with explicit fencing we want to allow
> > > userspace to overwrite any existing implicit fences that might hang
> > > around.
> > I'm just suggesting the danger of that when userspace doesn't know
> > everything and the current interfaces do not allow for userspace to know,
> > we only tell userspace about its own action (more or less).
> 
> It's even worse than that. See the kernel can for example swap out objects
> any time it wants.
> 
> Userspace doesn't know about such operations and so can't provide them as
> explicit fence.
> 
> Same is true for example in situations where one userspace process doesn't
> know about operations another process does. E.g. for backward compatibility
> with DRI2/3 for example.
> 
> So we will always have a mixture of implicit fences and explicit fences.
> 
> The approach we used for amdgpu is that we implicit wait for all fences
> which the initiator of an operation can't know about (e.g. from another
> process or kernel internally) and explicitly wait for all additional fences
> provided by the initiator or an operation.

On android userspace is also supposed to know about explicit fences from
other users, i.e. shared buffers. So there you shouldn't wait for implicit
fences from other processes, only for kernel-internal stuff.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1517160

On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Hi,
> 
> This is yet another version of the DRM fences patches. Please refer
> to the cover letter[1] in a previous version to check for more details.
> 
> In v7 we now have split most of the out_fences code into
> prepare_crtc_signaling() and unprepare_crtc_signaling() with improved error
> handling. More details on the v7 changes are embedded in each commit's
> message.
> 
> Robert Foss managed to port Android's drm_hwcomposer to the new HWC2 API and
> added support to fences. Current patches can be seen here:
> 
> https://git.collabora.com/cgit/user/robertfoss/drm_hwcomposer.git/log/?h=hwc2_fence_v1
> 
> He managed to run AOSP on top of padovan/fences kernel branch with full fence
> support on qemu/virgl and msm db410c. That means we already have a working
> open source userspace using the explicit fencing implementation.
> 
> Also i-g-t testing are available at:
> 
> https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/log/
> 
> Please review!

I think we're getting there. Found some issues with patch 3 still. Besides
that I think we need:
- r-b/ack from Sean Paul, as an ack that he's happy with what this means
  for drm_hwcomposer (and that the hwc patches look ok, too).
- acks/t-b from everyone who's run this, the more the better. This is a
  big uabi extension, making the effort by everyone explicit is important.
- ack from Brian that he can use the out-fence stuff for his writeback
  support. Probably need to add a for_each_connector loop to
  prepare/complete_signalling (and drop the crtc_ in there, but Brian can
  do that).

Cheers, Daniel

> 
> Gustavo
> 
> [1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1253822.html
> 
> ---
> Gustavo Padovan (3):
>   drm/fence: add in-fences support
>   drm/fence: add fence timeline to drm_crtc
>   drm/fence: add out-fences support
> 
>  drivers/gpu/drm/Kconfig             |   1 +
>  drivers/gpu/drm/drm_atomic.c        | 247 ++++++++++++++++++++++++++++++------
>  drivers/gpu/drm/drm_atomic_helper.c |   3 +
>  drivers/gpu/drm/drm_crtc.c          |  45 +++++++
>  drivers/gpu/drm/drm_plane.c         |   1 +
>  include/drm/drm_atomic.h            |   1 +
>  include/drm/drm_crtc.h              |  55 ++++++++
>  7 files changed, 311 insertions(+), 42 deletions(-)
> 
> -- 
> 2.5.5
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web