[PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver

[Rob Clark]

On Mon, Jun 10, 2013 at 4:08 PM, Russell King - ARM Linux
<linux at arm.linux.org.uk> wrote:
> On Mon, Jun 10, 2013 at 03:59:34PM -0400, Rob Clark wrote:
>> On Mon, Jun 10, 2013 at 1:06 PM, Russell King - ARM Linux
>> <linux at arm.linux.org.uk> wrote:
>> > ARMADA_GEM_CREATE_PHYS is to deal with creating a gem buffer object for
>> > a chunk of physical memory allocated by other means (eg, the Vmeta stuff.)
>> > This allows my X server to remain compatible with the XF86 Dove driver,
>> > which permits the passing of the physical address of the video frame to
>> > the X server (otherwise we're into rewriting a whole load more code than
>> > is really desirable - and I really don't have time to rewrite bits of
>> > gstreamer and other stuff.)
>>
>> ahh, gotcha.. and, ugg, that is even worse..
>>
>> I'm not hugely a fan of giving userspace a way to dma into arbitrary
>> phys addresses (ie. create_phys + put_image).  But I don't need to
>> explain what you already know ;-)
>>
>> Is there a pre-defined carveout pool that you can at least bounds
>> check against?  Otherwise put this ioctl inside a #if
>> CONFIG_CRAZY_SOC_VENDOR_HOLE_TO_DRIVE_A_TRUCK_THROUGH / #endif..
>
> This driver is _not_ about supporting the GPU natively as part of the DRM,
> but providing the foundations for:
>
> (a) a properly functional interface to HDMI TVs (fbdev doesn't hack that)
>     with hotplug
> (b) providing sufficient infrastructure to allow video playback to work.

sure, but even omitting the phys ioctls gives you (a), which seems
like it is useful on it's own.  And would, I expect, be pretty useful
for the etnaviv folks working on r/e the gpu.

> What this is not about is fixing the crappy userspace GPU libraries and
> video decoding so that it's "secure" - not only is that way beyond my
> abilities, it would also be a violation of the closed source license to
> reverse engineer them so that were possible.

yeah, once you add the vendor gpu/etc drivers, if they are also giving
you a way to pass a phys addr, then plugging the holes in the drm/kms
driver won't do any good.  But in a way that is some other
non-upstream driver's problem.

> It is something that continues to be a problem and I'm making no claims
> that I'm fixing that.  So no, I will not put such a config option around
> it for the simple reason that by doing so, turning such an option off
> renders userspace utterly useless and the driver might as well not exist.
>
> If that means you want to NACK the patch, then fine; I'll just maintain
> it out of tree.  I did the driver mostly for my own personal benefit to
> get the Cubox to the point where I can boot it with or without the TV
> connected and have it work.  But it would be a shame if that meant
> that others could not benefit from about 8 solid months of my work on
> this and have to put up with crappy a fbdev driver.

I would like to get at least some of the driver upstream.  I'd hate
for it to have to live completely out of tree.  I can think of a
couple possibilities.

1) the best would be, if there was some way for the drm driver to know
the upper/lower bounds of the carveout, then it could bounds-check
against this.  And then in worst case, userspace can just screw up
other "graphics" buffers

2) if #1 weren't possible, then maybe just keep the phys ioctls as a
small patch on top of upstream.  The at least out of the box you get
modesetting.  I had to do something like this w/ omapdrm for gluing it
together w/ sgx/pvr stuff.  I re-arranged the code a bit to group all
the non-upstream bits together to avoid merge/rebase conflicts.

BR,
-R