Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Michel Dänzer]

On 17/04/17 02:05 PM, Christopher James Halse Rogers wrote:
> On 10 April 2017 6:51:21 pm AEST, "Michel Dänzer"  wrote:
>> On 06/04/17 04:47 PM, Christopher James Halse Rogers wrote:
>>> On Wed, 5 Apr 2017 at 20:14 Lucas Stach >> > wrote:
>>>
 Drivers with separate VRAM and GTT are already doing a lot of migration
 behind the userspaces back. The only issue with dma-buf migration to
 VRAM is that you probably don't want to migrate the pages, but duplicate
 them in VRAM, doubling memory consumption with possible OOM. But then
 you could alloc the memory on addfb where you are able to return proper
 errors.
>>>
>>> Hm. So, on a first inspection, this looks like something I could
>>> actually cook up.
>>>
>>> Looking at amdgpu it seems like the thing to do would be to associate a
>>> shadow-bo in VRAM for the imported dma-buf in the addfb call, then
>>> pin-and-copy-to the shadow bo in the places where the bo is currently
>>> pinned.
>>>
>>> Is this approach likely to be acceptable?
>>
>> It would break e.g. with DRI2 flips, because they replace the screen
>> pixmap buffer with the buffer we're flipping to. If the app stops
>> flipping while such a shadow BO is being scanned out, later draws to
>> the screen pixmap won't become visible.
> 
> This shadow BO would only ever be used for imported dma-bufs. This
> would change the behaviour from “addfb fails” to “you get a shadow
> BO”. (And, pre-patch, from “addfb succeeds but you never see any new
> rendering”).
> 
> I don't think any DRI2 implementation hits this, because of it did it
> would already be broken.

You're right, I got it backwards. I guess this could work.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Christopher James Halse Rogers]

On 10 April 2017 6:51:21 pm AEST, "Michel Dänzer"  wrote:
>On 06/04/17 04:47 PM, Christopher James Halse Rogers wrote:
>> On Wed, 5 Apr 2017 at 20:14 Lucas Stach > > wrote:
>> 
>> Am Mittwoch, den 05.04.2017, 11:59 +0200 schrieb Daniel Vetter:
>> > On Wed, Apr 05, 2017 at 10:15:44AM +0200, Lucas Stach wrote:
>> > > Am Mittwoch, den 05.04.2017, 00:20 + schrieb Christopher
>> James Halse
>> > > Rogers:
>> > > >
>> > > >
>> > > > On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter
>> > wrote:
>> > > >
>> > > > On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach
>> > > > > > wrote:
>> > > > >> If I could guarantee that I'd only ever run on
>> > > > 4.13-or-later kernels
>> > > > >> (I think that's when the previous patches will
>> land?), then
>> > > > this would
>> > > > >> indeed be mostly unnecessary. It would save me a
>> bunch of
>> > > > addfb calls
>> > > > >> that would predictably fail, but they're cheap.
>> > > > >
>> > > > > I don't think we ever had caps for "things are
>> working now,
>> > > > as they are
>> > > > > supposed to". i915 wasn't properly synchronizing
>on
>> foreign
>> > > > fences for a
>> > > > > long time, yet we didn't gain a cap for "cross
>> device sync
>> > > > works now".
>> > > > >
>> > > > > If your distro use-case relies on those things
>> working it's
>> > > > probably
>> > > > > best to just backport the relevant fixes.
>> > > >
>> > > > The even better solution for this is to push the
>backports
>> > > > through
>> > > > upstream -stable kernels. This stuff here is simple
>enough
>> > > > that we can
>> > > > do it. Same could have been done for the fairly
>minimal
>> > > > fencing fixes
>> > > > for i915 (at least some of them, the ones in the
>> page-flip).
>> > > >
>> > > > Otherwise we'll end up with tons IM_NOT_BUGGY and
>> > > > IM_SLIGHTLY_LESS_BUGGY and
>> > > > IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
>> > > > flags where no one at all knows what they mean,
>usage
>> between
>> > > > different drivers and different userspace is
>entirely
>> > > > inconsistent and
>> > > > they just all add to the confusion. They're just
>bugs,
>> lets
>> > > > treat them
>> > > > like that.
>> > > >
>> > > >
>> > > > It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while
>the
>> relevant
>> > > > hardware allegedly supports it, nouveau/radeon/amdgpu don't
>do
>> scanout
>> > > > of GTT, so the lack of this cap indicates that there's no
>point in
>> > > > trying to call addfb2.
>> > > >
>> > > >
>> > > > But calling addfb2 and it failing is not expensive, so this
>is
>> rather
>> > > > niche.
>> > > >
>> > > >
>> > > > In practice I can just restrict attempting to scanout of
>imported
>> > > > buffers to i915, as that's the only driver that it'll work
>on.
>> By the
>> > > > time nouveau/radeon/amdgpu get patches to scanout of GTT
>the fixes
>> > > > should be old enough that I don't need to care about
>unfixed
>> kernels.
>> > > >
>> > > So given that most discreet hardware won't ever be able to
>> scanout from
>> > > GTT (latency and iso requirements will be hard to meet),
>can't
>> we just
>> > > fix the case of the broken prime sharing when migrating to
>VRAM?
>> > >
>> > > I'm thinking about attaching an exclusive fence to the
>dma-buf
>> when the
>> > > migration to VRAM happens, then when the GPU is done with the
>> buffer we
>> > > can either write back any changes to GTT, or just drop the
>fence
>> in case
>> > > the GPU didn't modify the buffer.
>> >
>> > We could, but someone needs to type the code for it. There's
>also the
>> > problem that you need to migrate back, and then doing all that
>behind
>> > userspaces back might not be the best idea.
>> 
>> Drivers with separate VRAM and GTT are already doing a lot of
>migration
>> behind the userspaces back. The only issue with dma-buf migration
>to
>> VRAM is that you probably don't want to migrate the pages, but
>duplicate
>> them in VRAM, doubling memory consumption with possible OOM. But
>then
>> you could alloc the memory on addfb where you are able to return
>proper
>> errors.
>> 
>> 
>> Hm. So, on a first inspection, this looks like something I could
>> actually 

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Alex Deucher]

On Mon, Apr 10, 2017 at 5:03 AM, Christian König
 wrote:
> Am 10.04.2017 um 10:52 schrieb Michel Dänzer:
>>
>> On 05/04/17 08:21 PM, Christian König wrote:
>>>
>>> Am 05.04.2017 um 13:13 schrieb Christopher James Halse Rogers:

 On Wed, Apr 5, 2017 at 8:14 PM Lucas Stach > wrote:

  Am Mittwoch, den 05.04.2017, 11:59 +0200 schrieb Daniel Vetter:
  > On Wed, Apr 05, 2017 at 10:15:44AM +0200, Lucas Stach wrote:
  > > Am Mittwoch, den 05.04.2017, 00:20 + schrieb Christopher
  James Halse
  > > Rogers:
  > > >
  > > >
  > > > On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter
  > wrote:
  > > >
  > > > On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach
  > > > > wrote:
  > > > >> If I could guarantee that I'd only ever run on
  > > > 4.13-or-later kernels
  > > > >> (I think that's when the previous patches will
  land?), then
  > > > this would
  > > > >> indeed be mostly unnecessary. It would save me a
  bunch of
  > > > addfb calls
  > > > >> that would predictably fail, but they're cheap.
  > > > >
  > > > > I don't think we ever had caps for "things are
  working now,
  > > > as they are
  > > > > supposed to". i915 wasn't properly synchronizing
  on foreign
  > > > fences for a
  > > > > long time, yet we didn't gain a cap for "cross
  device sync
  > > > works now".
  > > > >
  > > > > If your distro use-case relies on those things
  working it's
  > > > probably
  > > > > best to just backport the relevant fixes.
  > > >
  > > > The even better solution for this is to push the
  backports
  > > > through
  > > > upstream -stable kernels. This stuff here is simple
  enough
  > > > that we can
  > > > do it. Same could have been done for the fairly
 minimal
  > > > fencing fixes
  > > > for i915 (at least some of them, the ones in the
  page-flip).
  > > >
  > > > Otherwise we'll end up with tons IM_NOT_BUGGY and
  > > > IM_SLIGHTLY_LESS_BUGGY and
  > > > IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
  > > > flags where no one at all knows what they mean,
  usage between
  > > > different drivers and different userspace is entirely
  > > > inconsistent and
  > > > they just all add to the confusion. They're just
  bugs, lets
  > > > treat them
  > > > like that.
  > > >
  > > >
  > > > It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while the
  relevant
  > > > hardware allegedly supports it, nouveau/radeon/amdgpu don't
  do scanout
  > > > of GTT, so the lack of this cap indicates that there's no
  point in
  > > > trying to call addfb2.
  > > >
  > > >
  > > > But calling addfb2 and it failing is not expensive, so this
  is rather
  > > > niche.
  > > >
  > > >
  > > > In practice I can just restrict attempting to scanout of
  imported
  > > > buffers to i915, as that's the only driver that it'll work
  on. By the
  > > > time nouveau/radeon/amdgpu get patches to scanout of GTT the
  fixes
  > > > should be old enough that I don't need to care about unfixed
  kernels.
  > > >
  > > So given that most discreet hardware won't ever be able to
  scanout from
  > > GTT (latency and iso requirements will be hard to meet), can't
  we just
  > > fix the case of the broken prime sharing when migrating to
 VRAM?


 At least some nouveau and AMD devs seem to think that their hardware
 is capable of doing it. Shrug.
>>>
>>> Wow, wait a second. Recent AMD GPU can scanout from system memory,
>>> that's true.
>>
>> Even discrete GPUs, or only APUs?
>
>
> Good question. The crux is that you need to match certain bandwith and
> latency limitations or otherwise you get a flickering picture.
>
> If I understood the documentation correctly that should even work with dGPUs
> in theory, but nobody is testing/validating this.
>
> Long story short I wouldn't try it without feedback from the hardware/DC
> guys. They are the 

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Christian König]

Am 10.04.2017 um 10:52 schrieb Michel Dänzer:

On 05/04/17 08:21 PM, Christian König wrote:

Am 05.04.2017 um 13:13 schrieb Christopher James Halse Rogers:

On Wed, Apr 5, 2017 at 8:14 PM Lucas Stach > wrote:

 Am Mittwoch, den 05.04.2017, 11:59 +0200 schrieb Daniel Vetter:
 > On Wed, Apr 05, 2017 at 10:15:44AM +0200, Lucas Stach wrote:
 > > Am Mittwoch, den 05.04.2017, 00:20 + schrieb Christopher
 James Halse
 > > Rogers:
 > > >
 > > >
 > > > On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter
 > wrote:
 > > >
 > > > On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach
 > > > > wrote:
 > > > >> If I could guarantee that I'd only ever run on
 > > > 4.13-or-later kernels
 > > > >> (I think that's when the previous patches will
 land?), then
 > > > this would
 > > > >> indeed be mostly unnecessary. It would save me a
 bunch of
 > > > addfb calls
 > > > >> that would predictably fail, but they're cheap.
 > > > >
 > > > > I don't think we ever had caps for "things are
 working now,
 > > > as they are
 > > > > supposed to". i915 wasn't properly synchronizing
 on foreign
 > > > fences for a
 > > > > long time, yet we didn't gain a cap for "cross
 device sync
 > > > works now".
 > > > >
 > > > > If your distro use-case relies on those things
 working it's
 > > > probably
 > > > > best to just backport the relevant fixes.
 > > >
 > > > The even better solution for this is to push the
 backports
 > > > through
 > > > upstream -stable kernels. This stuff here is simple
 enough
 > > > that we can
 > > > do it. Same could have been done for the fairly minimal
 > > > fencing fixes
 > > > for i915 (at least some of them, the ones in the
 page-flip).
 > > >
 > > > Otherwise we'll end up with tons IM_NOT_BUGGY and
 > > > IM_SLIGHTLY_LESS_BUGGY and
 > > > IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
 > > > flags where no one at all knows what they mean,
 usage between
 > > > different drivers and different userspace is entirely
 > > > inconsistent and
 > > > they just all add to the confusion. They're just
 bugs, lets
 > > > treat them
 > > > like that.
 > > >
 > > >
 > > > It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while the
 relevant
 > > > hardware allegedly supports it, nouveau/radeon/amdgpu don't
 do scanout
 > > > of GTT, so the lack of this cap indicates that there's no
 point in
 > > > trying to call addfb2.
 > > >
 > > >
 > > > But calling addfb2 and it failing is not expensive, so this
 is rather
 > > > niche.
 > > >
 > > >
 > > > In practice I can just restrict attempting to scanout of
 imported
 > > > buffers to i915, as that's the only driver that it'll work
 on. By the
 > > > time nouveau/radeon/amdgpu get patches to scanout of GTT the
 fixes
 > > > should be old enough that I don't need to care about unfixed
 kernels.
 > > >
 > > So given that most discreet hardware won't ever be able to
 scanout from
 > > GTT (latency and iso requirements will be hard to meet), can't
 we just
 > > fix the case of the broken prime sharing when migrating to VRAM?


At least some nouveau and AMD devs seem to think that their hardware
is capable of doing it. Shrug.

Wow, wait a second. Recent AMD GPU can scanout from system memory,
that's true.

Even discrete GPUs, or only APUs?


Good question. The crux is that you need to match certain bandwith and 
latency limitations or otherwise you get a flickering picture.


If I understood the documentation correctly that should even work with 
dGPUs in theory, but nobody is testing/validating this.


Long story short I wouldn't try it without feedback from the hardware/DC 
guys. They are the designated experts for this.


Regards,
Christian.





But you need to met quite a bunch of special allocation requirements to
do this.

When we are talking about sharing between AMD GPUs, (e.g. both exporter
and importer are AMD hardware) than that might work.

But I think it's unrealistic that an imported BO (created by an external
driver stack) will ever meet those requirements.

Indeed. Also, none of the GPUs supported by the radeon driver support this.




___
dri-devel mailing list
dri-devel@lists.freedesktop.org

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Michel Dänzer]

On 05/04/17 08:21 PM, Christian König wrote:
> Am 05.04.2017 um 13:13 schrieb Christopher James Halse Rogers:
>> On Wed, Apr 5, 2017 at 8:14 PM Lucas Stach > > wrote:
>>
>> Am Mittwoch, den 05.04.2017, 11:59 +0200 schrieb Daniel Vetter:
>> > On Wed, Apr 05, 2017 at 10:15:44AM +0200, Lucas Stach wrote:
>> > > Am Mittwoch, den 05.04.2017, 00:20 + schrieb Christopher
>> James Halse
>> > > Rogers:
>> > > >
>> > > >
>> > > > On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter
>> > wrote:
>> > > >
>> > > > On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach
>> > > > > > wrote:
>> > > > >> If I could guarantee that I'd only ever run on
>> > > > 4.13-or-later kernels
>> > > > >> (I think that's when the previous patches will
>> land?), then
>> > > > this would
>> > > > >> indeed be mostly unnecessary. It would save me a
>> bunch of
>> > > > addfb calls
>> > > > >> that would predictably fail, but they're cheap.
>> > > > >
>> > > > > I don't think we ever had caps for "things are
>> working now,
>> > > > as they are
>> > > > > supposed to". i915 wasn't properly synchronizing
>> on foreign
>> > > > fences for a
>> > > > > long time, yet we didn't gain a cap for "cross
>> device sync
>> > > > works now".
>> > > > >
>> > > > > If your distro use-case relies on those things
>> working it's
>> > > > probably
>> > > > > best to just backport the relevant fixes.
>> > > >
>> > > > The even better solution for this is to push the
>> backports
>> > > > through
>> > > > upstream -stable kernels. This stuff here is simple
>> enough
>> > > > that we can
>> > > > do it. Same could have been done for the fairly minimal
>> > > > fencing fixes
>> > > > for i915 (at least some of them, the ones in the
>> page-flip).
>> > > >
>> > > > Otherwise we'll end up with tons IM_NOT_BUGGY and
>> > > > IM_SLIGHTLY_LESS_BUGGY and
>> > > > IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
>> > > > flags where no one at all knows what they mean,
>> usage between
>> > > > different drivers and different userspace is entirely
>> > > > inconsistent and
>> > > > they just all add to the confusion. They're just
>> bugs, lets
>> > > > treat them
>> > > > like that.
>> > > >
>> > > >
>> > > > It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while the
>> relevant
>> > > > hardware allegedly supports it, nouveau/radeon/amdgpu don't
>> do scanout
>> > > > of GTT, so the lack of this cap indicates that there's no
>> point in
>> > > > trying to call addfb2.
>> > > >
>> > > >
>> > > > But calling addfb2 and it failing is not expensive, so this
>> is rather
>> > > > niche.
>> > > >
>> > > >
>> > > > In practice I can just restrict attempting to scanout of
>> imported
>> > > > buffers to i915, as that's the only driver that it'll work
>> on. By the
>> > > > time nouveau/radeon/amdgpu get patches to scanout of GTT the
>> fixes
>> > > > should be old enough that I don't need to care about unfixed
>> kernels.
>> > > >
>> > > So given that most discreet hardware won't ever be able to
>> scanout from
>> > > GTT (latency and iso requirements will be hard to meet), can't
>> we just
>> > > fix the case of the broken prime sharing when migrating to VRAM?
>>
>>
>> At least some nouveau and AMD devs seem to think that their hardware
>> is capable of doing it. Shrug.
> 
> Wow, wait a second. Recent AMD GPU can scanout from system memory,
> that's true.

Even discrete GPUs, or only APUs?


> But you need to met quite a bunch of special allocation requirements to
> do this.
> 
> When we are talking about sharing between AMD GPUs, (e.g. both exporter
> and importer are AMD hardware) than that might work.
> 
> But I think it's unrealistic that an imported BO (created by an external
> driver stack) will ever meet those requirements.

Indeed. Also, none of the GPUs supported by the radeon driver support this.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Michel Dänzer]

On 06/04/17 04:47 PM, Christopher James Halse Rogers wrote:
> On Wed, 5 Apr 2017 at 20:14 Lucas Stach  > wrote:
> 
> Am Mittwoch, den 05.04.2017, 11:59 +0200 schrieb Daniel Vetter:
> > On Wed, Apr 05, 2017 at 10:15:44AM +0200, Lucas Stach wrote:
> > > Am Mittwoch, den 05.04.2017, 00:20 + schrieb Christopher
> James Halse
> > > Rogers:
> > > >
> > > >
> > > > On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter  > wrote:
> > > >
> > > > On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach
> > > >  > wrote:
> > > > >> If I could guarantee that I'd only ever run on
> > > > 4.13-or-later kernels
> > > > >> (I think that's when the previous patches will
> land?), then
> > > > this would
> > > > >> indeed be mostly unnecessary. It would save me a
> bunch of
> > > > addfb calls
> > > > >> that would predictably fail, but they're cheap.
> > > > >
> > > > > I don't think we ever had caps for "things are
> working now,
> > > > as they are
> > > > > supposed to". i915 wasn't properly synchronizing on
> foreign
> > > > fences for a
> > > > > long time, yet we didn't gain a cap for "cross
> device sync
> > > > works now".
> > > > >
> > > > > If your distro use-case relies on those things
> working it's
> > > > probably
> > > > > best to just backport the relevant fixes.
> > > >
> > > > The even better solution for this is to push the backports
> > > > through
> > > > upstream -stable kernels. This stuff here is simple enough
> > > > that we can
> > > > do it. Same could have been done for the fairly minimal
> > > > fencing fixes
> > > > for i915 (at least some of them, the ones in the
> page-flip).
> > > >
> > > > Otherwise we'll end up with tons IM_NOT_BUGGY and
> > > > IM_SLIGHTLY_LESS_BUGGY and
> > > > IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
> > > > flags where no one at all knows what they mean, usage
> between
> > > > different drivers and different userspace is entirely
> > > > inconsistent and
> > > > they just all add to the confusion. They're just bugs,
> lets
> > > > treat them
> > > > like that.
> > > >
> > > >
> > > > It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while the
> relevant
> > > > hardware allegedly supports it, nouveau/radeon/amdgpu don't do
> scanout
> > > > of GTT, so the lack of this cap indicates that there's no point in
> > > > trying to call addfb2.
> > > >
> > > >
> > > > But calling addfb2 and it failing is not expensive, so this is
> rather
> > > > niche.
> > > >
> > > >
> > > > In practice I can just restrict attempting to scanout of imported
> > > > buffers to i915, as that's the only driver that it'll work on.
> By the
> > > > time nouveau/radeon/amdgpu get patches to scanout of GTT the fixes
> > > > should be old enough that I don't need to care about unfixed
> kernels.
> > > >
> > > So given that most discreet hardware won't ever be able to
> scanout from
> > > GTT (latency and iso requirements will be hard to meet), can't
> we just
> > > fix the case of the broken prime sharing when migrating to VRAM?
> > >
> > > I'm thinking about attaching an exclusive fence to the dma-buf
> when the
> > > migration to VRAM happens, then when the GPU is done with the
> buffer we
> > > can either write back any changes to GTT, or just drop the fence
> in case
> > > the GPU didn't modify the buffer.
> >
> > We could, but someone needs to type the code for it. There's also the
> > problem that you need to migrate back, and then doing all that behind
> > userspaces back might not be the best idea.
> 
> Drivers with separate VRAM and GTT are already doing a lot of migration
> behind the userspaces back. The only issue with dma-buf migration to
> VRAM is that you probably don't want to migrate the pages, but duplicate
> them in VRAM, doubling memory consumption with possible OOM. But then
> you could alloc the memory on addfb where you are able to return proper
> errors.
> 
> 
> Hm. So, on a first inspection, this looks like something I could
> actually cook up.
> 
> Looking at amdgpu it seems like the thing to do would be to associate a
> shadow-bo in VRAM for the imported dma-buf in the addfb call, then
> pin-and-copy-to the shadow bo in the places where the bo is 

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Christopher James Halse Rogers]

On Wed, 5 Apr 2017 at 20:14 Lucas Stach  wrote:

> Am Mittwoch, den 05.04.2017, 11:59 +0200 schrieb Daniel Vetter:
> > On Wed, Apr 05, 2017 at 10:15:44AM +0200, Lucas Stach wrote:
> > > Am Mittwoch, den 05.04.2017, 00:20 + schrieb Christopher James
> Halse
> > > Rogers:
> > > >
> > > >
> > > > On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter 
> wrote:
> > > >
> > > > On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach
> > > >  wrote:
> > > > >> If I could guarantee that I'd only ever run on
> > > > 4.13-or-later kernels
> > > > >> (I think that's when the previous patches will land?),
> then
> > > > this would
> > > > >> indeed be mostly unnecessary. It would save me a bunch of
> > > > addfb calls
> > > > >> that would predictably fail, but they're cheap.
> > > > >
> > > > > I don't think we ever had caps for "things are working now,
> > > > as they are
> > > > > supposed to". i915 wasn't properly synchronizing on foreign
> > > > fences for a
> > > > > long time, yet we didn't gain a cap for "cross device sync
> > > > works now".
> > > > >
> > > > > If your distro use-case relies on those things working it's
> > > > probably
> > > > > best to just backport the relevant fixes.
> > > >
> > > > The even better solution for this is to push the backports
> > > > through
> > > > upstream -stable kernels. This stuff here is simple enough
> > > > that we can
> > > > do it. Same could have been done for the fairly minimal
> > > > fencing fixes
> > > > for i915 (at least some of them, the ones in the page-flip).
> > > >
> > > > Otherwise we'll end up with tons IM_NOT_BUGGY and
> > > > IM_SLIGHTLY_LESS_BUGGY and
> > > > IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
> > > > flags where no one at all knows what they mean, usage between
> > > > different drivers and different userspace is entirely
> > > > inconsistent and
> > > > they just all add to the confusion. They're just bugs, lets
> > > > treat them
> > > > like that.
> > > >
> > > >
> > > > It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while the relevant
> > > > hardware allegedly supports it, nouveau/radeon/amdgpu don't do
> scanout
> > > > of GTT, so the lack of this cap indicates that there's no point in
> > > > trying to call addfb2.
> > > >
> > > >
> > > > But calling addfb2 and it failing is not expensive, so this is rather
> > > > niche.
> > > >
> > > >
> > > > In practice I can just restrict attempting to scanout of imported
> > > > buffers to i915, as that's the only driver that it'll work on. By the
> > > > time nouveau/radeon/amdgpu get patches to scanout of GTT the fixes
> > > > should be old enough that I don't need to care about unfixed kernels.
> > > >
> > > So given that most discreet hardware won't ever be able to scanout from
> > > GTT (latency and iso requirements will be hard to meet), can't we just
> > > fix the case of the broken prime sharing when migrating to VRAM?
> > >
> > > I'm thinking about attaching an exclusive fence to the dma-buf when the
> > > migration to VRAM happens, then when the GPU is done with the buffer we
> > > can either write back any changes to GTT, or just drop the fence in
> case
> > > the GPU didn't modify the buffer.
> >
> > We could, but someone needs to type the code for it. There's also the
> > problem that you need to migrate back, and then doing all that behind
> > userspaces back might not be the best idea.
>
> Drivers with separate VRAM and GTT are already doing a lot of migration
> behind the userspaces back. The only issue with dma-buf migration to
> VRAM is that you probably don't want to migrate the pages, but duplicate
> them in VRAM, doubling memory consumption with possible OOM. But then
> you could alloc the memory on addfb where you are able to return proper
> errors.


Hm. So, on a first inspection, this looks like something I could actually
cook up.

Looking at amdgpu it seems like the thing to do would be to associate a
shadow-bo in VRAM for the imported dma-buf in the addfb call, then
pin-and-copy-to the shadow bo in the places where the bo is currently
pinned.

Is this approach likely to be acceptable?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Christian König]

Am 05.04.2017 um 13:13 schrieb Christopher James Halse Rogers:
On Wed, Apr 5, 2017 at 8:14 PM Lucas Stach > wrote:


Am Mittwoch, den 05.04.2017, 11:59 +0200 schrieb Daniel Vetter:
> On Wed, Apr 05, 2017 at 10:15:44AM +0200, Lucas Stach wrote:
> > Am Mittwoch, den 05.04.2017, 00:20 + schrieb Christopher
James Halse
> > Rogers:
> > >
> > >
> > > On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter
> wrote:
> > >
> > > On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach
> > > > wrote:
> > > >> If I could guarantee that I'd only ever run on
> > > 4.13-or-later kernels
> > > >> (I think that's when the previous patches will
land?), then
> > > this would
> > > >> indeed be mostly unnecessary. It would save me a
bunch of
> > > addfb calls
> > > >> that would predictably fail, but they're cheap.
> > > >
> > > > I don't think we ever had caps for "things are
working now,
> > > as they are
> > > > supposed to". i915 wasn't properly synchronizing
on foreign
> > > fences for a
> > > > long time, yet we didn't gain a cap for "cross
device sync
> > > works now".
> > > >
> > > > If your distro use-case relies on those things
working it's
> > > probably
> > > > best to just backport the relevant fixes.
> > >
> > > The even better solution for this is to push the
backports
> > > through
> > > upstream -stable kernels. This stuff here is simple
enough
> > > that we can
> > > do it. Same could have been done for the fairly minimal
> > > fencing fixes
> > > for i915 (at least some of them, the ones in the
page-flip).
> > >
> > > Otherwise we'll end up with tons IM_NOT_BUGGY and
> > > IM_SLIGHTLY_LESS_BUGGY and
> > >  IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
> > > flags where no one at all knows what they mean,
usage between
> > > different drivers and different userspace is entirely
> > > inconsistent and
> > > they just all add to the confusion. They're just
bugs, lets
> > > treat them
> > > like that.
> > >
> > >
> > > It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while the
relevant
> > > hardware allegedly supports it, nouveau/radeon/amdgpu don't
do scanout
> > > of GTT, so the lack of this cap indicates that there's no
point in
> > > trying to call addfb2.
> > >
> > >
> > > But calling addfb2 and it failing is not expensive, so this
is rather
> > > niche.
> > >
> > >
> > > In practice I can just restrict attempting to scanout of
imported
> > > buffers to i915, as that's the only driver that it'll work
on. By the
> > > time nouveau/radeon/amdgpu get patches to scanout of GTT the
fixes
> > > should be old enough that I don't need to care about unfixed
kernels.
> > >
> > So given that most discreet hardware won't ever be able to
scanout from
> > GTT (latency and iso requirements will be hard to meet), can't
we just
> > fix the case of the broken prime sharing when migrating to VRAM?


At least some nouveau and AMD devs seem to think that their hardware 
is capable of doing it. Shrug.


Wow, wait a second. Recent AMD GPU can scanout from system memory, 
that's true.


But you need to met quite a bunch of special allocation requirements to 
do this.


When we are talking about sharing between AMD GPUs, (e.g. both exporter 
and importer are AMD hardware) than that might work.


But I think it's unrealistic that an imported BO (created by an external 
driver stack) will ever meet those requirements.


Christian.



> >
> > I'm thinking about attaching an exclusive fence to the dma-buf
when the
> > migration to VRAM happens, then when the GPU is done with the
buffer we
> > can either write back any changes to GTT, or just drop the
fence in case
> > the GPU didn't modify the buffer.
>
> We could, but someone needs to type the code for it. There's
also the
> problem that you need to migrate back, and then doing all that
behind
> userspaces back might not be the best idea.

Drivers with separate VRAM and GTT are already doing a lot of
migration
behind the userspaces back. The only issue with dma-buf migration to
VRAM is that you probably don't want to migrate the pages, but
duplicate
them in VRAM, doubling memory consumption with possible OOM. But then
you could alloc the 

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Christopher James Halse Rogers]

On Wed, Apr 5, 2017 at 8:14 PM Lucas Stach  wrote:

> Am Mittwoch, den 05.04.2017, 11:59 +0200 schrieb Daniel Vetter:
> > On Wed, Apr 05, 2017 at 10:15:44AM +0200, Lucas Stach wrote:
> > > Am Mittwoch, den 05.04.2017, 00:20 + schrieb Christopher James
> Halse
> > > Rogers:
> > > >
> > > >
> > > > On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter 
> wrote:
> > > >
> > > > On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach
> > > >  wrote:
> > > > >> If I could guarantee that I'd only ever run on
> > > > 4.13-or-later kernels
> > > > >> (I think that's when the previous patches will land?),
> then
> > > > this would
> > > > >> indeed be mostly unnecessary. It would save me a bunch of
> > > > addfb calls
> > > > >> that would predictably fail, but they're cheap.
> > > > >
> > > > > I don't think we ever had caps for "things are working now,
> > > > as they are
> > > > > supposed to". i915 wasn't properly synchronizing on foreign
> > > > fences for a
> > > > > long time, yet we didn't gain a cap for "cross device sync
> > > > works now".
> > > > >
> > > > > If your distro use-case relies on those things working it's
> > > > probably
> > > > > best to just backport the relevant fixes.
> > > >
> > > > The even better solution for this is to push the backports
> > > > through
> > > > upstream -stable kernels. This stuff here is simple enough
> > > > that we can
> > > > do it. Same could have been done for the fairly minimal
> > > > fencing fixes
> > > > for i915 (at least some of them, the ones in the page-flip).
> > > >
> > > > Otherwise we'll end up with tons IM_NOT_BUGGY and
> > > > IM_SLIGHTLY_LESS_BUGGY and
> > > > IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
> > > > flags where no one at all knows what they mean, usage between
> > > > different drivers and different userspace is entirely
> > > > inconsistent and
> > > > they just all add to the confusion. They're just bugs, lets
> > > > treat them
> > > > like that.
> > > >
> > > >
> > > > It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while the relevant
> > > > hardware allegedly supports it, nouveau/radeon/amdgpu don't do
> scanout
> > > > of GTT, so the lack of this cap indicates that there's no point in
> > > > trying to call addfb2.
> > > >
> > > >
> > > > But calling addfb2 and it failing is not expensive, so this is rather
> > > > niche.
> > > >
> > > >
> > > > In practice I can just restrict attempting to scanout of imported
> > > > buffers to i915, as that's the only driver that it'll work on. By the
> > > > time nouveau/radeon/amdgpu get patches to scanout of GTT the fixes
> > > > should be old enough that I don't need to care about unfixed kernels.
> > > >
> > > So given that most discreet hardware won't ever be able to scanout from
> > > GTT (latency and iso requirements will be hard to meet), can't we just
> > > fix the case of the broken prime sharing when migrating to VRAM?
>

At least some nouveau and AMD devs seem to think that their hardware is
capable of doing it. Shrug.

> >
> > > I'm thinking about attaching an exclusive fence to the dma-buf when the
> > > migration to VRAM happens, then when the GPU is done with the buffer we
> > > can either write back any changes to GTT, or just drop the fence in
> case
> > > the GPU didn't modify the buffer.
> >
> > We could, but someone needs to type the code for it. There's also the
> > problem that you need to migrate back, and then doing all that behind
> > userspaces back might not be the best idea.
>
> Drivers with separate VRAM and GTT are already doing a lot of migration
> behind the userspaces back. The only issue with dma-buf migration to
> VRAM is that you probably don't want to migrate the pages, but duplicate
> them in VRAM, doubling memory consumption with possible OOM. But then
> you could alloc the memory on addfb where you are able to return proper
> errors.
>

I would *love* for the driver to copy the pages for me into VRAM for
scanout, rather than me having to spin up an EGL context and run the
trivial blitting shader across an EGLImage.

Are you offering to do it? :)

I'll still need to, for the short term, assume that only i915 can do this
without breaking, though.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Lucas Stach]

Am Mittwoch, den 05.04.2017, 11:59 +0200 schrieb Daniel Vetter:
> On Wed, Apr 05, 2017 at 10:15:44AM +0200, Lucas Stach wrote:
> > Am Mittwoch, den 05.04.2017, 00:20 + schrieb Christopher James Halse
> > Rogers:
> > > 
> > > 
> > > On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter  wrote:
> > > 
> > > On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach
> > >  wrote:
> > > >> If I could guarantee that I'd only ever run on
> > > 4.13-or-later kernels
> > > >> (I think that's when the previous patches will land?), then
> > > this would
> > > >> indeed be mostly unnecessary. It would save me a bunch of
> > > addfb calls
> > > >> that would predictably fail, but they're cheap.
> > > >
> > > > I don't think we ever had caps for "things are working now,
> > > as they are
> > > > supposed to". i915 wasn't properly synchronizing on foreign
> > > fences for a
> > > > long time, yet we didn't gain a cap for "cross device sync
> > > works now".
> > > >
> > > > If your distro use-case relies on those things working it's
> > > probably
> > > > best to just backport the relevant fixes.
> > > 
> > > The even better solution for this is to push the backports
> > > through
> > > upstream -stable kernels. This stuff here is simple enough
> > > that we can
> > > do it. Same could have been done for the fairly minimal
> > > fencing fixes
> > > for i915 (at least some of them, the ones in the page-flip).
> > > 
> > > Otherwise we'll end up with tons IM_NOT_BUGGY and
> > > IM_SLIGHTLY_LESS_BUGGY and
> > > IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
> > > flags where no one at all knows what they mean, usage between
> > > different drivers and different userspace is entirely
> > > inconsistent and
> > > they just all add to the confusion. They're just bugs, lets
> > > treat them
> > > like that.
> > > 
> > > 
> > > It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while the relevant
> > > hardware allegedly supports it, nouveau/radeon/amdgpu don't do scanout
> > > of GTT, so the lack of this cap indicates that there's no point in
> > > trying to call addfb2.
> > > 
> > > 
> > > But calling addfb2 and it failing is not expensive, so this is rather
> > > niche.
> > > 
> > > 
> > > In practice I can just restrict attempting to scanout of imported
> > > buffers to i915, as that's the only driver that it'll work on. By the
> > > time nouveau/radeon/amdgpu get patches to scanout of GTT the fixes
> > > should be old enough that I don't need to care about unfixed kernels.
> > > 
> > So given that most discreet hardware won't ever be able to scanout from
> > GTT (latency and iso requirements will be hard to meet), can't we just
> > fix the case of the broken prime sharing when migrating to VRAM?
> > 
> > I'm thinking about attaching an exclusive fence to the dma-buf when the
> > migration to VRAM happens, then when the GPU is done with the buffer we
> > can either write back any changes to GTT, or just drop the fence in case
> > the GPU didn't modify the buffer.
> 
> We could, but someone needs to type the code for it. There's also the
> problem that you need to migrate back, and then doing all that behind
> userspaces back might not be the best idea.

Drivers with separate VRAM and GTT are already doing a lot of migration
behind the userspaces back. The only issue with dma-buf migration to
VRAM is that you probably don't want to migrate the pages, but duplicate
them in VRAM, doubling memory consumption with possible OOM. But then
you could alloc the memory on addfb where you are able to return proper
errors.

I guess what I'm saying is that userspace really should check if addfb
for imported buffers works and base its decisions on that, not some
arbitrary "is this driver XY" or "does it have magic DRM_CAP".  This way
we can enable transparent VRAM migration (making addfb on them work) if
someone is interested in this, without further changes to the UAPI.

Regards,
Lucas

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

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Daniel Vetter]

On Wed, Apr 05, 2017 at 10:15:44AM +0200, Lucas Stach wrote:
> Am Mittwoch, den 05.04.2017, 00:20 + schrieb Christopher James Halse
> Rogers:
> > 
> > 
> > On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter  wrote:
> > 
> > On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach
> >  wrote:
> > >> If I could guarantee that I'd only ever run on
> > 4.13-or-later kernels
> > >> (I think that's when the previous patches will land?), then
> > this would
> > >> indeed be mostly unnecessary. It would save me a bunch of
> > addfb calls
> > >> that would predictably fail, but they're cheap.
> > >
> > > I don't think we ever had caps for "things are working now,
> > as they are
> > > supposed to". i915 wasn't properly synchronizing on foreign
> > fences for a
> > > long time, yet we didn't gain a cap for "cross device sync
> > works now".
> > >
> > > If your distro use-case relies on those things working it's
> > probably
> > > best to just backport the relevant fixes.
> > 
> > The even better solution for this is to push the backports
> > through
> > upstream -stable kernels. This stuff here is simple enough
> > that we can
> > do it. Same could have been done for the fairly minimal
> > fencing fixes
> > for i915 (at least some of them, the ones in the page-flip).
> > 
> > Otherwise we'll end up with tons IM_NOT_BUGGY and
> > IM_SLIGHTLY_LESS_BUGGY and
> > IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
> > flags where no one at all knows what they mean, usage between
> > different drivers and different userspace is entirely
> > inconsistent and
> > they just all add to the confusion. They're just bugs, lets
> > treat them
> > like that.
> > 
> > 
> > It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while the relevant
> > hardware allegedly supports it, nouveau/radeon/amdgpu don't do scanout
> > of GTT, so the lack of this cap indicates that there's no point in
> > trying to call addfb2.
> > 
> > 
> > But calling addfb2 and it failing is not expensive, so this is rather
> > niche.
> > 
> > 
> > In practice I can just restrict attempting to scanout of imported
> > buffers to i915, as that's the only driver that it'll work on. By the
> > time nouveau/radeon/amdgpu get patches to scanout of GTT the fixes
> > should be old enough that I don't need to care about unfixed kernels.
> > 
> So given that most discreet hardware won't ever be able to scanout from
> GTT (latency and iso requirements will be hard to meet), can't we just
> fix the case of the broken prime sharing when migrating to VRAM?
> 
> I'm thinking about attaching an exclusive fence to the dma-buf when the
> migration to VRAM happens, then when the GPU is done with the buffer we
> can either write back any changes to GTT, or just drop the fence in case
> the GPU didn't modify the buffer.

We could, but someone needs to type the code for it. There's also the
problem that you need to migrate back, and then doing all that behind
userspaces back might not be the best idea.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Lucas Stach]

Am Mittwoch, den 05.04.2017, 00:20 + schrieb Christopher James Halse
Rogers:
> 
> 
> On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter  wrote:
> 
> On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach
>  wrote:
> >> If I could guarantee that I'd only ever run on
> 4.13-or-later kernels
> >> (I think that's when the previous patches will land?), then
> this would
> >> indeed be mostly unnecessary. It would save me a bunch of
> addfb calls
> >> that would predictably fail, but they're cheap.
> >
> > I don't think we ever had caps for "things are working now,
> as they are
> > supposed to". i915 wasn't properly synchronizing on foreign
> fences for a
> > long time, yet we didn't gain a cap for "cross device sync
> works now".
> >
> > If your distro use-case relies on those things working it's
> probably
> > best to just backport the relevant fixes.
> 
> The even better solution for this is to push the backports
> through
> upstream -stable kernels. This stuff here is simple enough
> that we can
> do it. Same could have been done for the fairly minimal
> fencing fixes
> for i915 (at least some of them, the ones in the page-flip).
> 
> Otherwise we'll end up with tons IM_NOT_BUGGY and
> IM_SLIGHTLY_LESS_BUGGY and
> IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
> flags where no one at all knows what they mean, usage between
> different drivers and different userspace is entirely
> inconsistent and
> they just all add to the confusion. They're just bugs, lets
> treat them
> like that.
> 
> 
> It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while the relevant
> hardware allegedly supports it, nouveau/radeon/amdgpu don't do scanout
> of GTT, so the lack of this cap indicates that there's no point in
> trying to call addfb2.
> 
> 
> But calling addfb2 and it failing is not expensive, so this is rather
> niche.
> 
> 
> In practice I can just restrict attempting to scanout of imported
> buffers to i915, as that's the only driver that it'll work on. By the
> time nouveau/radeon/amdgpu get patches to scanout of GTT the fixes
> should be old enough that I don't need to care about unfixed kernels.
> 
So given that most discreet hardware won't ever be able to scanout from
GTT (latency and iso requirements will be hard to meet), can't we just
fix the case of the broken prime sharing when migrating to VRAM?

I'm thinking about attaching an exclusive fence to the dma-buf when the
migration to VRAM happens, then when the GPU is done with the buffer we
can either write back any changes to GTT, or just drop the fence in case
the GPU didn't modify the buffer.

Regards,
Lucas

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

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Christopher James Halse Rogers]

On Wed, Apr 5, 2017 at 4:27 PM, Daniel Vetter  wrote:
On Wed, Apr 05, 2017 at 12:20:46AM +, Christopher James Halse 
Rogers wrote:
 On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter  
wrote:


 > On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach 


 > wrote:
 > >> If I could guarantee that I'd only ever run on 4.13-or-later 
kernels
 > >> (I think that's when the previous patches will land?), then 
this would
 > >> indeed be mostly unnecessary. It would save me a bunch of 
addfb calls

 > >> that would predictably fail, but they're cheap.
 > >
 > > I don't think we ever had caps for "things are working now, as 
they are
 > > supposed to". i915 wasn't properly synchronizing on foreign 
fences for a
 > > long time, yet we didn't gain a cap for "cross device sync 
works now".

 > >
 > > If your distro use-case relies on those things working it's 
probably

 > > best to just backport the relevant fixes.
 >
 > The even better solution for this is to push the backports through
 > upstream -stable kernels. This stuff here is simple enough that 
we can
 > do it. Same could have been done for the fairly minimal fencing 
fixes

 > for i915 (at least some of them, the ones in the page-flip).
 >
 > Otherwise we'll end up with tons IM_NOT_BUGGY and
 > IM_SLIGHTLY_LESS_BUGGY and 
IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT

 > flags where no one at all knows what they mean, usage between
 > different drivers and different userspace is entirely 
inconsistent and
 > they just all add to the confusion. They're just bugs, lets treat 
them

 > like that.
 >

 It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while the 
relevant
 hardware allegedly supports it, nouveau/radeon/amdgpu don't do 
scanout of
 GTT, so the lack of this cap indicates that there's no point in 
trying to

 call addfb2.

 But calling addfb2 and it failing is not expensive, so this is 
rather niche.


 In practice I can just restrict attempting to scanout of imported 
buffers

 to i915, as that's the only driver that it'll work on. By the time
 nouveau/radeon/amdgpu get patches to scanout of GTT the fixes 
should be old

 enough that I don't need to care about unfixed kernels.


"I'll only run on i915" sounds like a rather risky assumption. You're 
sure
no one will install ubuntu on e.g. rasperry with Eric's shiny new 
pl111

driver?

I really think that if you want to rely on this, backporting the 
fixes is

perfectly fine.


Oh, no! I mean *check* that I'm running on i915, and only use the 
no-copy path if I am.


Backporting the patches would indeed be sensible. I guess I should have 
CC: stable'd at the time.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Daniel Vetter]

On Wed, Apr 05, 2017 at 12:20:46AM +, Christopher James Halse Rogers wrote:
> On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter  wrote:
> 
> > On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach 
> > wrote:
> > >> If I could guarantee that I'd only ever run on 4.13-or-later kernels
> > >> (I think that's when the previous patches will land?), then this would
> > >> indeed be mostly unnecessary. It would save me a bunch of addfb calls
> > >> that would predictably fail, but they're cheap.
> > >
> > > I don't think we ever had caps for "things are working now, as they are
> > > supposed to". i915 wasn't properly synchronizing on foreign fences for a
> > > long time, yet we didn't gain a cap for "cross device sync works now".
> > >
> > > If your distro use-case relies on those things working it's probably
> > > best to just backport the relevant fixes.
> >
> > The even better solution for this is to push the backports through
> > upstream -stable kernels. This stuff here is simple enough that we can
> > do it. Same could have been done for the fairly minimal fencing fixes
> > for i915 (at least some of them, the ones in the page-flip).
> >
> > Otherwise we'll end up with tons IM_NOT_BUGGY and
> > IM_SLIGHTLY_LESS_BUGGY and IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
> > flags where no one at all knows what they mean, usage between
> > different drivers and different userspace is entirely inconsistent and
> > they just all add to the confusion. They're just bugs, lets treat them
> > like that.
> >
> 
> It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while the relevant
> hardware allegedly supports it, nouveau/radeon/amdgpu don't do scanout of
> GTT, so the lack of this cap indicates that there's no point in trying to
> call addfb2.
> 
> But calling addfb2 and it failing is not expensive, so this is rather niche.
> 
> In practice I can just restrict attempting to scanout of imported buffers
> to i915, as that's the only driver that it'll work on. By the time
> nouveau/radeon/amdgpu get patches to scanout of GTT the fixes should be old
> enough that I don't need to care about unfixed kernels.

"I'll only run on i915" sounds like a rather risky assumption. You're sure
no one will install ubuntu on e.g. rasperry with Eric's shiny new pl111
driver?

I really think that if you want to rely on this, backporting the fixes is
perfectly fine.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Christopher James Halse Rogers]

On Tue, Apr 4, 2017 at 9:53 PM Daniel Vetter  wrote:

> On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach 
> wrote:
> >> If I could guarantee that I'd only ever run on 4.13-or-later kernels
> >> (I think that's when the previous patches will land?), then this would
> >> indeed be mostly unnecessary. It would save me a bunch of addfb calls
> >> that would predictably fail, but they're cheap.
> >
> > I don't think we ever had caps for "things are working now, as they are
> > supposed to". i915 wasn't properly synchronizing on foreign fences for a
> > long time, yet we didn't gain a cap for "cross device sync works now".
> >
> > If your distro use-case relies on those things working it's probably
> > best to just backport the relevant fixes.
>
> The even better solution for this is to push the backports through
> upstream -stable kernels. This stuff here is simple enough that we can
> do it. Same could have been done for the fairly minimal fencing fixes
> for i915 (at least some of them, the ones in the page-flip).
>
> Otherwise we'll end up with tons IM_NOT_BUGGY and
> IM_SLIGHTLY_LESS_BUGGY and IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
> flags where no one at all knows what they mean, usage between
> different drivers and different userspace is entirely inconsistent and
> they just all add to the confusion. They're just bugs, lets treat them
> like that.
>

It's not *quite* DRM_CAP_PRIME_SCANOUT_NOT_BUGGY - while the relevant
hardware allegedly supports it, nouveau/radeon/amdgpu don't do scanout of
GTT, so the lack of this cap indicates that there's no point in trying to
call addfb2.

But calling addfb2 and it failing is not expensive, so this is rather niche.

In practice I can just restrict attempting to scanout of imported buffers
to i915, as that's the only driver that it'll work on. By the time
nouveau/radeon/amdgpu get patches to scanout of GTT the fixes should be old
enough that I don't need to care about unfixed kernels.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Daniel Vetter]

On Tue, Apr 4, 2017 at 12:43 PM, Lucas Stach  wrote:
>> If I could guarantee that I'd only ever run on 4.13-or-later kernels
>> (I think that's when the previous patches will land?), then this would
>> indeed be mostly unnecessary. It would save me a bunch of addfb calls
>> that would predictably fail, but they're cheap.
>
> I don't think we ever had caps for "things are working now, as they are
> supposed to". i915 wasn't properly synchronizing on foreign fences for a
> long time, yet we didn't gain a cap for "cross device sync works now".
>
> If your distro use-case relies on those things working it's probably
> best to just backport the relevant fixes.

The even better solution for this is to push the backports through
upstream -stable kernels. This stuff here is simple enough that we can
do it. Same could have been done for the fairly minimal fencing fixes
for i915 (at least some of them, the ones in the page-flip).

Otherwise we'll end up with tons IM_NOT_BUGGY and
IM_SLIGHTLY_LESS_BUGGY and IM_NOT_BUGGY_EXCEPT_THIS_BOTCHED_BACKPORT
flags where no one at all knows what they mean, usage between
different drivers and different userspace is entirely inconsistent and
they just all add to the confusion. They're just bugs, lets treat them
like that.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Christian König]

Am 04.04.2017 um 13:32 schrieb Daniel Stone:

Hi,

On 4 April 2017 at 12:15, Christian König  wrote:

Am 04.04.2017 um 12:43 schrieb Daniel Stone:

Yeah, the ABI is that AddFB2 should fail hard on something which can
never be used as a framebuffer. The fact it didn't is a bug rather
than a behavioural change per se ...

I agree on that, but the problem Christopher tries to solve looks a bit
different from my perspective.

He wants to know if a driver can scan out from a foreign BO or if he needs
to copy the content.

The problem I see with this patch is that the kernel doesn't look like the
right place for this decision to make.

Any number of constraints exist there: 'can I use any completely
arbitrary dmabuf from anywhere?' doesn't express the full range.

Yes, completely agree.


AddFB can fail for other totally legitimate reasons, so to me it makes the
most sense to centralise the checks there.
Sorry, sounds like I wasn't clear on this. Removing the check in was 
*not* what I've wanted to suggest.


The kernel of course needs to check if what userspace requested makes 
sense and fail with a proper error message if it doesn't.


But the negotiation if two kernel drivers can work with each others data 
formats for each use case is not something I would want to push into the 
kernel. For this we simply have to many and to complex constraints on this.


In other words if you have an AMD APU with integrated graphics and an 
AMD dGPU it is likely that both can understand what the other puts into 
its buffer, even when it is completely AMD specific. The AMD user space 
drivers already have code to handle such cases.


But when we have a combination of Intel+AMD or Intel+NVidia we need way 
more negotiation between the two driver stacks then just the placement 
of buffers.


Regards,
Christian.



Cheers,
Daniel



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

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Daniel Stone]

Hi,

On 4 April 2017 at 12:15, Christian König  wrote:
> Am 04.04.2017 um 12:43 schrieb Daniel Stone:
>> Yeah, the ABI is that AddFB2 should fail hard on something which can
>> never be used as a framebuffer. The fact it didn't is a bug rather
>> than a behavioural change per se ...
>
> I agree on that, but the problem Christopher tries to solve looks a bit
> different from my perspective.
>
> He wants to know if a driver can scan out from a foreign BO or if he needs
> to copy the content.
>
> The problem I see with this patch is that the kernel doesn't look like the
> right place for this decision to make.

Any number of constraints exist there: 'can I use any completely
arbitrary dmabuf from anywhere?' doesn't express the full range. AddFB
can fail for other totally legitimate reasons, so to me it makes the
most sense to centralise the checks there.

Cheers,
Daniel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Christian König]

Am 04.04.2017 um 12:43 schrieb Daniel Stone:

Hi,

On 4 April 2017 at 11:27, Christopher James Halse Rogers
 wrote:

On 4 April 2017 6:31:12 pm AEST, Daniel Vetter  wrote:

This seems like an awful specific special case. Why can't you do the
entire dance upfront, i.e. import buffer, addfb2? None of that has any
visible effect, and it will also allow you to check runtime constraints
which can't be covered with this here.

No, because addfb2 doesn't (or, rather, didn't) actually check any runtime 
constraints.

The problem only appeared when the buffer is actually *used* in a modeset - 
otherwise I could do a (reasonably) cheap import/addfb/render on exporter/read 
out on importer dance to detect it.

To be clear - this is trying to solve the problem “how can I tell if it's safe 
to addfb/pageflip rather than do a GL copy to a GPU-local buffer and flip from 
that?”.

If I could guarantee that I'd only ever run on 4.13-or-later kernels (I think 
that's when the previous patches will land?), then this would indeed be mostly 
unnecessary. It would save me a bunch of addfb calls that would predictably 
fail, but they're cheap.

Yeah, the ABI is that AddFB2 should fail hard on something which can
never be used as a framebuffer. The fact it didn't is a bug rather
than a behavioural change per se ...


I agree on that, but the problem Christopher tries to solve looks a bit 
different from my perspective.


He wants to know if a driver can scan out from a foreign BO or if he 
needs to copy the content.


The problem I see with this patch is that the kernel doesn't look like 
the right place for this decision to make.


Regards,
Christian.



Cheers,
Daniel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel



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

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Daniel Stone]

Hi,

On 4 April 2017 at 11:27, Christopher James Halse Rogers
 wrote:
> On 4 April 2017 6:31:12 pm AEST, Daniel Vetter  wrote:
>>This seems like an awful specific special case. Why can't you do the
>>entire dance upfront, i.e. import buffer, addfb2? None of that has any
>>visible effect, and it will also allow you to check runtime constraints
>>which can't be covered with this here.
>
> No, because addfb2 doesn't (or, rather, didn't) actually check any runtime 
> constraints.
>
> The problem only appeared when the buffer is actually *used* in a modeset - 
> otherwise I could do a (reasonably) cheap import/addfb/render on 
> exporter/read out on importer dance to detect it.
>
> To be clear - this is trying to solve the problem “how can I tell if it's 
> safe to addfb/pageflip rather than do a GL copy to a GPU-local buffer and 
> flip from that?”.
>
> If I could guarantee that I'd only ever run on 4.13-or-later kernels (I think 
> that's when the previous patches will land?), then this would indeed be 
> mostly unnecessary. It would save me a bunch of addfb calls that would 
> predictably fail, but they're cheap.

Yeah, the ABI is that AddFB2 should fail hard on something which can
never be used as a framebuffer. The fact it didn't is a bug rather
than a behavioural change per se ...

Cheers,
Daniel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Lucas Stach]

Am Dienstag, den 04.04.2017, 20:27 +1000 schrieb Christopher James Halse
Rogers:
> On 4 April 2017 6:31:12 pm AEST, Daniel Vetter  wrote:
> >On Tue, Apr 04, 2017 at 06:13:20PM +1000, r...@ubuntu.com wrote:
> >> From: Christopher James Halse Rogers 
> >> 
> >> Until recently, on (at least) nouveau, radeon, and amdgpu attempting
> >to scanout of an
> >> imported dma-buf would silently result in the dma-buf sharing being
> >broken.
> >> 
> >> While the hardware is capable of scanning out of imported dma-bufs
> >(at least in some circumstances),
> >> these drivers do not currently implement it, so attempts to scan out
> >of such buffers will never succeed.
> >> 
> >> Add a userspace-visible drm capability and associated driver_feature
> >so that userspace can discover
> >> when scanning out of an imported dma-buf can work.
> >> 
> >> Signed-off-by: Christopher James Halse Rogers
> >
> >
> >This seems like an awful specific special case. Why can't you do the
> >entire dance upfront, i.e. import buffer, addfb2? None of that has any
> >visible effect, and it will also allow you to check runtime constraints
> >which can't be covered with this here.
> >-Daniel
> 
> No, because addfb2 doesn't (or, rather, didn't) actually check any
> runtime constraints.
> 
> The problem only appeared when the buffer is actually *used* in a
> modeset - otherwise I could do a (reasonably) cheap
> import/addfb/render on exporter/read out on importer dance to detect
> it.
> 
> To be clear - this is trying to solve the problem “how can I tell if
> it's safe to addfb/pageflip rather than do a GL copy to a GPU-local
> buffer and flip from that?”.
> 
> If I could guarantee that I'd only ever run on 4.13-or-later kernels
> (I think that's when the previous patches will land?), then this would
> indeed be mostly unnecessary. It would save me a bunch of addfb calls
> that would predictably fail, but they're cheap.

I don't think we ever had caps for "things are working now, as they are
supposed to". i915 wasn't properly synchronizing on foreign fences for a
long time, yet we didn't gain a cap for "cross device sync works now".

If your distro use-case relies on those things working it's probably
best to just backport the relevant fixes.

Regards,
Lucas

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

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Christopher James Halse Rogers]

On 4 April 2017 6:31:12 pm AEST, Daniel Vetter  wrote:
>On Tue, Apr 04, 2017 at 06:13:20PM +1000, r...@ubuntu.com wrote:
>> From: Christopher James Halse Rogers 
>> 
>> Until recently, on (at least) nouveau, radeon, and amdgpu attempting
>to scanout of an
>> imported dma-buf would silently result in the dma-buf sharing being
>broken.
>> 
>> While the hardware is capable of scanning out of imported dma-bufs
>(at least in some circumstances),
>> these drivers do not currently implement it, so attempts to scan out
>of such buffers will never succeed.
>> 
>> Add a userspace-visible drm capability and associated driver_feature
>so that userspace can discover
>> when scanning out of an imported dma-buf can work.
>> 
>> Signed-off-by: Christopher James Halse Rogers
>
>
>This seems like an awful specific special case. Why can't you do the
>entire dance upfront, i.e. import buffer, addfb2? None of that has any
>visible effect, and it will also allow you to check runtime constraints
>which can't be covered with this here.
>-Daniel

No, because addfb2 doesn't (or, rather, didn't) actually check any runtime 
constraints.

The problem only appeared when the buffer is actually *used* in a modeset - 
otherwise I could do a (reasonably) cheap import/addfb/render on exporter/read 
out on importer dance to detect it.

To be clear - this is trying to solve the problem “how can I tell if it's safe 
to addfb/pageflip rather than do a GL copy to a GPU-local buffer and flip from 
that?”.

If I could guarantee that I'd only ever run on 4.13-or-later kernels (I think 
that's when the previous patches will land?), then this would indeed be mostly 
unnecessary. It would save me a bunch of addfb calls that would predictably 
fail, but they're cheap.

>
>> ---
>>  drivers/gpu/drm/drm_ioctl.c |  3 +++
>>  include/drm/drm_drv.h   |  1 +
>>  include/uapi/drm/drm.h  | 21 +
>>  3 files changed, 25 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/drm_ioctl.c
>b/drivers/gpu/drm/drm_ioctl.c
>> index a7c61c23685a..79ccf638668e 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -285,6 +285,9 @@ static int drm_getcap(struct drm_device *dev,
>void *data, struct drm_file *file_
>>  case DRM_CAP_ADDFB2_MODIFIERS:
>>  req->value = dev->mode_config.allow_fb_modifiers;
>>  break;
>> +case DRM_CAP_PRIME_SCANOUT:
>> +req->value = drm_core_check_feature(dev, DRIVER_PRIME_SCANOUT);
>> +break;
>>  default:
>>  return -EINVAL;
>>  }
>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>> index 5699f42195fe..32cc0d956d7e 100644
>> --- a/include/drm/drm_drv.h
>> +++ b/include/drm/drm_drv.h
>> @@ -53,6 +53,7 @@ struct drm_mode_create_dumb;
>>  #define DRIVER_RENDER   0x8000
>>  #define DRIVER_ATOMIC   0x1
>>  #define DRIVER_KMS_LEGACY_CONTEXT   0x2
>> +#define DRIVER_PRIME_SCANOUT0x4
>>  
>>  /**
>>   * struct drm_driver - DRM driver structure
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index b2c52843bc70..c57213cdb702 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -647,6 +647,27 @@ struct drm_gem_open {
>>  #define DRM_CAP_CURSOR_HEIGHT   0x9
>>  #define DRM_CAP_ADDFB2_MODIFIERS0x10
>>  #define DRM_CAP_PAGE_FLIP_TARGET0x11
>> +/**
>> + * DRM_CAP_PRIME_SCANOUT
>> + *
>> + * The PRIME_SCANOUT capability returns whether the driver might be
>capable
>> + * of scanning out of an imported PRIME buffer.
>> + *
>> + * This does not guarantee that any imported buffer can be scanned
>out, as
>> + * there may be specific requirements that a given buffer might not
>satisfy.
>> + *
>> + * If this capability is false then imported PRIME buffers will
>definitely
>> + * never be suitable for scanout.
>> + *
>> + * Note: Prior to the introduction of this capability, bugs in
>drivers would
>> + * allow userspace to attempt to scan out of imported PRIME buffers.
>This would
>> + * work once, but move the buffer into an inconsistent state where
>rendering from
>> + * the exporting GPU would no longer be seen by the importing GPU.
>> + *
>> + * It is unsafe for driver-agnostic code to attempt to scan out of
>an imported
>> + * PRIME buffer in the absense of this capability.
>> + */
>> +#define DRM_CAP_PRIME_SCANOUT   0x12
>>  
>>  /** DRM_IOCTL_GET_CAP ioctl argument type */
>>  struct drm_get_cap {
>> -- 
>> 2.11.0
>> 
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Daniel Stone]

Hi,

On 4 April 2017 at 09:31, Daniel Vetter  wrote:
> On Tue, Apr 04, 2017 at 06:13:20PM +1000, r...@ubuntu.com wrote:
>> Until recently, on (at least) nouveau, radeon, and amdgpu attempting to 
>> scanout of an
>> imported dma-buf would silently result in the dma-buf sharing being broken.
>>
>> While the hardware is capable of scanning out of imported dma-bufs (at least 
>> in some circumstances),
>> these drivers do not currently implement it, so attempts to scan out of such 
>> buffers will never succeed.
>>
>> Add a userspace-visible drm capability and associated driver_feature so that 
>> userspace can discover
>> when scanning out of an imported dma-buf can work.
>>
>> Signed-off-by: Christopher James Halse Rogers 
>> 
>
> This seems like an awful specific special case. Why can't you do the
> entire dance upfront, i.e. import buffer, addfb2? None of that has any
> visible effect, and it will also allow you to check runtime constraints
> which can't be covered with this here.

Oh, that already happens. Chris sent out a series which fixes bugs for
PRIME import across a few drivers, mostly ones which need to pin
scanout buffers to dedicated memory. So this cap is basically
DRM_CAP_PRIME_SCANOUT_ISNT_BUGGY. FWIW, I'm fairly skeptical about
adding caps which essentially declare the absence of bugs like this.

Cheers,
Daniel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[Daniel Vetter]

On Tue, Apr 04, 2017 at 06:13:20PM +1000, r...@ubuntu.com wrote:
> From: Christopher James Halse Rogers 
> 
> Until recently, on (at least) nouveau, radeon, and amdgpu attempting to 
> scanout of an
> imported dma-buf would silently result in the dma-buf sharing being broken.
> 
> While the hardware is capable of scanning out of imported dma-bufs (at least 
> in some circumstances),
> these drivers do not currently implement it, so attempts to scan out of such 
> buffers will never succeed.
> 
> Add a userspace-visible drm capability and associated driver_feature so that 
> userspace can discover
> when scanning out of an imported dma-buf can work.
> 
> Signed-off-by: Christopher James Halse Rogers 
> 

This seems like an awful specific special case. Why can't you do the
entire dance upfront, i.e. import buffer, addfb2? None of that has any
visible effect, and it will also allow you to check runtime constraints
which can't be covered with this here.
-Daniel

> ---
>  drivers/gpu/drm/drm_ioctl.c |  3 +++
>  include/drm/drm_drv.h   |  1 +
>  include/uapi/drm/drm.h  | 21 +
>  3 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index a7c61c23685a..79ccf638668e 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -285,6 +285,9 @@ static int drm_getcap(struct drm_device *dev, void *data, 
> struct drm_file *file_
>   case DRM_CAP_ADDFB2_MODIFIERS:
>   req->value = dev->mode_config.allow_fb_modifiers;
>   break;
> + case DRM_CAP_PRIME_SCANOUT:
> + req->value = drm_core_check_feature(dev, DRIVER_PRIME_SCANOUT);
> + break;
>   default:
>   return -EINVAL;
>   }
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 5699f42195fe..32cc0d956d7e 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -53,6 +53,7 @@ struct drm_mode_create_dumb;
>  #define DRIVER_RENDER0x8000
>  #define DRIVER_ATOMIC0x1
>  #define DRIVER_KMS_LEGACY_CONTEXT0x2
> +#define DRIVER_PRIME_SCANOUT 0x4
>  
>  /**
>   * struct drm_driver - DRM driver structure
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index b2c52843bc70..c57213cdb702 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -647,6 +647,27 @@ struct drm_gem_open {
>  #define DRM_CAP_CURSOR_HEIGHT0x9
>  #define DRM_CAP_ADDFB2_MODIFIERS 0x10
>  #define DRM_CAP_PAGE_FLIP_TARGET 0x11
> +/**
> + * DRM_CAP_PRIME_SCANOUT
> + *
> + * The PRIME_SCANOUT capability returns whether the driver might be capable
> + * of scanning out of an imported PRIME buffer.
> + *
> + * This does not guarantee that any imported buffer can be scanned out, as
> + * there may be specific requirements that a given buffer might not satisfy.
> + *
> + * If this capability is false then imported PRIME buffers will definitely
> + * never be suitable for scanout.
> + *
> + * Note: Prior to the introduction of this capability, bugs in drivers would
> + * allow userspace to attempt to scan out of imported PRIME buffers. This 
> would
> + * work once, but move the buffer into an inconsistent state where rendering 
> from
> + * the exporting GPU would no longer be seen by the importing GPU.
> + *
> + * It is unsafe for driver-agnostic code to attempt to scan out of an 
> imported
> + * PRIME buffer in the absense of this capability.
> + */
> +#define DRM_CAP_PRIME_SCANOUT0x12
>  
>  /** DRM_IOCTL_GET_CAP ioctl argument type */
>  struct drm_get_cap {
> -- 
> 2.11.0
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm: Add DRM_CAP_PRIME_SCANOUT.

[raof]

From: Christopher James Halse Rogers 

Until recently, on (at least) nouveau, radeon, and amdgpu attempting to scanout 
of an
imported dma-buf would silently result in the dma-buf sharing being broken.

While the hardware is capable of scanning out of imported dma-bufs (at least in 
some circumstances),
these drivers do not currently implement it, so attempts to scan out of such 
buffers will never succeed.

Add a userspace-visible drm capability and associated driver_feature so that 
userspace can discover
when scanning out of an imported dma-buf can work.

Signed-off-by: Christopher James Halse Rogers 

---
 drivers/gpu/drm/drm_ioctl.c |  3 +++
 include/drm/drm_drv.h   |  1 +
 include/uapi/drm/drm.h  | 21 +
 3 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index a7c61c23685a..79ccf638668e 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -285,6 +285,9 @@ static int drm_getcap(struct drm_device *dev, void *data, 
struct drm_file *file_
case DRM_CAP_ADDFB2_MODIFIERS:
req->value = dev->mode_config.allow_fb_modifiers;
break;
+   case DRM_CAP_PRIME_SCANOUT:
+   req->value = drm_core_check_feature(dev, DRIVER_PRIME_SCANOUT);
+   break;
default:
return -EINVAL;
}
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 5699f42195fe..32cc0d956d7e 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -53,6 +53,7 @@ struct drm_mode_create_dumb;
 #define DRIVER_RENDER  0x8000
 #define DRIVER_ATOMIC  0x1
 #define DRIVER_KMS_LEGACY_CONTEXT  0x2
+#define DRIVER_PRIME_SCANOUT   0x4
 
 /**
  * struct drm_driver - DRM driver structure
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index b2c52843bc70..c57213cdb702 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -647,6 +647,27 @@ struct drm_gem_open {
 #define DRM_CAP_CURSOR_HEIGHT  0x9
 #define DRM_CAP_ADDFB2_MODIFIERS   0x10
 #define DRM_CAP_PAGE_FLIP_TARGET   0x11
+/**
+ * DRM_CAP_PRIME_SCANOUT
+ *
+ * The PRIME_SCANOUT capability returns whether the driver might be capable
+ * of scanning out of an imported PRIME buffer.
+ *
+ * This does not guarantee that any imported buffer can be scanned out, as
+ * there may be specific requirements that a given buffer might not satisfy.
+ *
+ * If this capability is false then imported PRIME buffers will definitely
+ * never be suitable for scanout.
+ *
+ * Note: Prior to the introduction of this capability, bugs in drivers would
+ * allow userspace to attempt to scan out of imported PRIME buffers. This would
+ * work once, but move the buffer into an inconsistent state where rendering 
from
+ * the exporting GPU would no longer be seen by the importing GPU.
+ *
+ * It is unsafe for driver-agnostic code to attempt to scan out of an imported
+ * PRIME buffer in the absense of this capability.
+ */
+#define DRM_CAP_PRIME_SCANOUT  0x12
 
 /** DRM_IOCTL_GET_CAP ioctl argument type */
 struct drm_get_cap {
-- 
2.11.0

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