[Intel-gfx] [PATCH] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl

[Eric Anholt]

Daniel Vetter <daniel at ffwll.ch> writes:

> On Mon, Apr 8, 2013 at 7:40 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>> On Mon, Apr 08, 2013 at 07:18:11PM +0200, Daniel Vetter wrote:
>>> On Tue, Feb 12, 2013 at 02:17:22PM +0000, Chris Wilson wrote:
>>> > By exporting the ability to map user address and inserting PTEs
>>> > representing their backing pages into the GTT, we can exploit UMA in order
>>> > to utilize normal application data as a texture source or even as a
>>> > render target (depending upon the capabilities of the chipset). This has
>>> > a number of uses, with zero-copy downloads to the GPU and efficient
>>> > readback making the intermixed streaming of CPU and GPU operations
>>> > fairly efficient. This ability has many widespread implications from
>>> > faster rendering of client-side software rasterisers (chromium),
>>> > mitigation of stalls due to read back (firefox) and to faster pipelining
>>> > of texture data (such as pixel buffer objects in GL or data blobs in CL).
>>> >
>>> > v2: Compile with CONFIG_MMU_NOTIFIER
>>> > v3: We can sleep while performing invalidate-range, which we can utilise
>>> > to drop our page references prior to the kernel manipulating the vma
>>> > (for either discard or cloning) and so protect normal users.
>>> > v4: Only run the invalidate notifier if the range intercepts the bo.
>>> > v5: Prevent userspace from attempting to GTT mmap non-page aligned buffers
>>> >
>>> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>>
>>> Quick bikeshed:
>>> - Still not really in favour of the in-page gtt offset handling ... I
>>>   still think that this is just a fancy mmap api, and it better reject
>>>   attempts to not map anything aligned to a full page outright.
>>
>> Strongly disagree.
>
> Ok, let's dig out the beaten arguments here ;-)
> - Imo the gtt_offset frobbery is a bit fragile, and getting this right
> in the face of ppgtt won't make it better. And yes the only reason we
> still have that field is that you've shot down any patch to remove it
> citing userptr here. So "it's here already" doesn't count ;-)

Agreed -- given that I need to look at byte offsets for alignment issues
on basically all my usages of memory, having my data have part of its
intra-page offset hidden in the kernel at creation time would be bad for
Mesa.

Access to data is controlled at a page level, so I think this kernel
interface should act at a page level.

>>> - I915_USERPTR_UNSYNCHRONIZED ... eeek. That means that despite everyone
>>>   having mmu notifiers enabled in their distro config, you make sure sna
>>>   doesn't hit it. Imo not enough testing coverage ;-) Or this there
>>>   another reason behind this than "mmu notifiers are too slow"?
>>>
>>>   Generally I'm a bit sloppy with going root-only for legacy X stuff (like
>>>   scanline waits), but this here looks very much generally useful. So not
>>>   exemption-material imo.
>>
>> Strongly disagree. Most of my machines do not have mmu-notifiers and
>> would still like to benefit from userptr and I see no reason why we need
>> to force mmu-notifiers.
>
> Note that I didn't shout against the mmu_notifier-less support
> (although I'm honestly not too happy about it), what I don't like is
> the override bit disabling the mmu_notifiers even if we have them.
> Since that will mean that the code won't be tested through SNA, and so
> has a good chance of being buggy. Once mesa comes around and uses it,
> it'll nicely blow up. And one of the reason Jesse is breathing down my
> neck to merge this is "other guys are interested in this at intel".

I hate root-only interfaces.  It helps lock in root-only X, and means
that X gets special treatment compared to the 3D driver.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20130408/106edf02/attachment.sig>