[PATCH] drm/gem: Add new flink_to ioctl

[Kristian Høgsberg]

On Thu, Jul 8, 2010 at 12:49 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> On Thu, 08 Jul 2010 17:37:20 +0100
> Chris Wilson <chris at chris-wilson.co.uk> wrote:
>
>> On Thu, 8 Jul 2010 12:14:28 -0400, Kristian Høgsberg <krh at bitplanet.net> wrote:
>> > On Thu, Jul 8, 2010 at 11:59 AM, Keith Packard <keithp at keithp.com> wrote:
>> > > On Thu,  8 Jul 2010 11:23:25 -0400, Kristian Høgsberg <krh at bitplanet.net> wrote:
>> > >
>> > >>  - a mechanism to attach a binary blob to an flink_to buffer name.
>> > >>    open_with_data returns the data.  Userspace (typically libdrm)
>> > >>    decides the layout and versioning of the blob and the contents
>> > >>    will be chipset specific.  it's an opaque blob to the kernel,
>> > >>    which doesn't need to know about stride and formats etc.
>> > >
>> > > Arbitrary binary blobs considered harmful? Even if the kernel doesn't
>> > > need to know all of this data, having it in an explicit (versioned)
>> > > format will protect applications from randomly mis-interpreting the data.
>> >
>> > I talked with ickle about that and whether or not to include a
>> > version+format u32 for the data in the ioctl args.  He convinced me
>> > that the kernel didn't need to know about the layout of the blob and
>> > that requiring by convention that the first u32 of the blob is the
>> > version+format u32 would suffice.  I can go either way on this, but I
>> > guess I have a small preference for making it part of the ioctl args
>> > as you suggest.
>>
>> I am not going to argue with someone who has been tackling the issue of
>> protocol extensions for 25 years... ;-)
>>
>> My argument was based around that the current system is designed as a
>> directory of opaque objects and so the extended attributes should be
>> kept opaque to the kernel as well and left open to interpretation by
>> userland. What I am most unclear about is under which circumstances is
>> this backchannel communication preferable to passing the extra information
>> over the IPC that needs to be performed anyway in order to open a surface.
>
> That's the part I had trouble with as well.  Passing the blob through
> the kernel saves a little IPC but also seems unnecessary, and so rubs
> against my kernel minimalist side...

There's nothing in the use cases I have in mind that strictly requires
passing buffers around as integer IDs.  We could standardize a
serialization mechanism in libdrm that would give you a blob that
contains the bo name and the meta data, and you would then send that
over the protocol when you need to share a buffer.  Attaching a blob
to the name instead and passing just the uint32_t name around makes
life a little easier though, in the EGL API, in the protocol code.  It
is IPC-ish, I guess, but in the same sense as xattr's are, for
example.

The other point is that we already do this to some extent, with the
object size (returned by open) and the tiling and swizzle parameters
(communicated through the set and get ioctls).  They're are somewhat
special in that the kernel also needs to know these values, but we do
use the kernel to communicate these values between processes.  There's
no reason gem_open needs to return size in the ioctl args and the
intel get_tiling ioctl is only required because we don't pass the
tiling meta data over dri2 protocol (it's a chipset specific thing
after all).  The flink_to blob approach combines the convenience of
getting the data at gem_open time and the option to pass free form
chipset specific meta data.

In the work on the EGL extension, the other Khronos members have
indicated that sharing buffers by passing an integer could work for
their implementations too, and that's what the draft standard
currently requires.  I could try to get that changed to a
size+bytearray couple, but then the "pass the blob" API makes it all
the way into user APIs.  We could implement the integer ID in
userspace by creating a file in a shared directory by the name of the
integer handle and serialize meta data into that.  I'm not fond of
either approach, but they're possible.

Kristian