From: robdclark@gmail.com (Rob Clark) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver Date: Mon, 10 Jun 2013 17:01:18 -0400 [thread overview] Message-ID: <CAF6AEGsfzvdiGXqAu56rc-G_ZhhE_vnNbC6kpBmRtd-8Sh_uOQ@mail.gmail.com> (raw) In-Reply-To: <20130610200839.GY18614@n2100.arm.linux.org.uk> On Mon, Jun 10, 2013 at 4:08 PM, Russell King - ARM Linux <linux@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@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
WARNING: multiple messages have this Message-ID (diff)
From: Rob Clark <robdclark@gmail.com> To: Russell King - ARM Linux <linux@arm.linux.org.uk> Cc: Jason Cooper <jason@lakedaemon.net>, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> Subject: Re: [PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver Date: Mon, 10 Jun 2013 17:01:18 -0400 [thread overview] Message-ID: <CAF6AEGsfzvdiGXqAu56rc-G_ZhhE_vnNbC6kpBmRtd-8Sh_uOQ@mail.gmail.com> (raw) In-Reply-To: <20130610200839.GY18614@n2100.arm.linux.org.uk> On Mon, Jun 10, 2013 at 4:08 PM, Russell King - ARM Linux <linux@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@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
next prev parent reply other threads:[~2013-06-10 21:01 UTC|newest] Thread overview: 120+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-06-09 19:06 [RFC v2 0/8] rmk's Dove DRM/TDA19988 Cubox driver Russell King - ARM Linux 2013-06-09 19:06 ` Russell King - ARM Linux 2013-06-09 19:29 ` [PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver Russell King 2013-06-09 19:32 ` Russell King 2013-06-10 11:10 ` Sebastian Hesselbarth 2013-06-10 11:10 ` Sebastian Hesselbarth 2013-06-10 21:48 ` Russell King - ARM Linux 2013-06-10 21:48 ` Russell King - ARM Linux 2013-06-10 21:56 ` Sebastian Hesselbarth 2013-06-10 21:56 ` Sebastian Hesselbarth 2013-06-10 15:57 ` Rob Clark 2013-06-10 15:57 ` Rob Clark 2013-06-10 17:06 ` Russell King - ARM Linux 2013-06-10 17:06 ` Russell King - ARM Linux 2013-06-10 19:59 ` Rob Clark 2013-06-10 19:59 ` Rob Clark 2013-06-10 20:08 ` Russell King - ARM Linux 2013-06-10 20:08 ` Russell King - ARM Linux 2013-06-10 21:01 ` Rob Clark [this message] 2013-06-10 21:01 ` Rob Clark 2013-06-10 21:15 ` Russell King - ARM Linux 2013-06-10 21:15 ` Russell King - ARM Linux 2013-06-10 22:49 ` Rob Clark 2013-06-10 22:49 ` Rob Clark 2013-06-10 22:56 ` Russell King - ARM Linux 2013-06-10 22:56 ` Russell King - ARM Linux 2013-06-10 23:17 ` Rob Clark 2013-06-10 23:17 ` Rob Clark 2013-06-10 23:24 ` Dave Airlie 2013-06-10 23:24 ` Dave Airlie 2013-06-10 23:35 ` Rob Clark 2013-06-10 23:35 ` Rob Clark 2013-06-10 23:36 ` Russell King - ARM Linux 2013-06-10 23:36 ` Russell King - ARM Linux 2013-06-10 23:48 ` Dave Airlie 2013-06-10 23:48 ` Dave Airlie 2013-06-10 23:56 ` Russell King - ARM Linux 2013-06-10 23:56 ` Russell King - ARM Linux 2013-06-12 13:48 ` Russell King - ARM Linux 2013-06-12 13:48 ` Russell King - ARM Linux 2013-06-12 13:56 ` Rob Clark 2013-06-12 13:56 ` Rob Clark 2013-06-12 16:49 ` Russell King - ARM Linux 2013-06-12 16:49 ` Russell King - ARM Linux 2013-06-12 17:05 ` Russell King - ARM Linux 2013-06-12 17:05 ` Russell King - ARM Linux 2013-06-12 19:40 ` Russell King - ARM Linux 2013-06-12 19:40 ` Russell King - ARM Linux 2013-06-12 23:00 ` Russell King - ARM Linux 2013-06-12 23:00 ` Russell King - ARM Linux 2013-06-13 0:17 ` Rob Clark 2013-06-13 0:17 ` Rob Clark 2013-06-13 11:19 ` Russell King - ARM Linux 2013-06-13 11:19 ` Russell King - ARM Linux 2013-06-13 11:50 ` Russell King - ARM Linux 2013-06-13 11:50 ` Russell King - ARM Linux 2013-06-13 13:03 ` Russell King - ARM Linux 2013-06-13 13:03 ` Russell King - ARM Linux 2013-06-14 14:23 ` Daniel Vetter 2013-06-14 14:23 ` Daniel Vetter 2013-06-14 14:42 ` Russell King - ARM Linux 2013-06-14 14:42 ` Russell King - ARM Linux 2013-06-14 19:50 ` Daniel Vetter 2013-06-14 19:50 ` Daniel Vetter 2013-06-14 22:15 ` Russell King - ARM Linux 2013-06-14 22:15 ` Russell King - ARM Linux 2013-06-14 22:36 ` Daniel Vetter 2013-06-14 22:36 ` Daniel Vetter 2013-06-14 13:53 ` Daniel Vetter 2013-06-14 13:53 ` Daniel Vetter 2013-06-14 14:27 ` Russell King - ARM Linux 2013-06-14 14:27 ` Russell King - ARM Linux 2013-06-13 12:52 ` Rob Clark 2013-06-13 12:52 ` Rob Clark 2013-06-13 12:58 ` Daniel Vetter 2013-06-13 12:58 ` Daniel Vetter 2013-06-12 20:04 ` Rob Clark 2013-06-12 20:04 ` Rob Clark 2013-06-10 23:38 ` Russell King - ARM Linux 2013-06-10 23:38 ` Russell King - ARM Linux 2013-06-10 23:49 ` Rob Clark 2013-06-10 23:49 ` Rob Clark 2013-06-10 22:01 ` Daniel Vetter 2013-06-10 22:01 ` Daniel Vetter 2013-06-10 22:32 ` Russell King - ARM Linux 2013-06-10 22:32 ` Russell King - ARM Linux 2013-06-10 23:12 ` Rob Clark 2013-06-10 23:12 ` Rob Clark 2013-06-11 7:33 ` Daniel Vetter 2013-06-11 7:33 ` Daniel Vetter 2013-06-11 8:08 ` Ville Syrjälä 2013-06-11 8:08 ` Ville Syrjälä 2013-06-10 21:38 ` Russell King - ARM Linux 2013-06-10 21:38 ` Russell King - ARM Linux 2013-06-09 19:30 ` [PATCH RFC 3/8] drm/i2c: nxp-tda998x: fix EDID reading on TDA19988 devices Russell King 2013-06-09 19:30 ` Russell King 2013-06-09 19:31 ` [PATCH RFC 4/8] drm/i2c: nxp-tda998x: ensure VIP output mux is properly set Russell King 2013-06-09 19:31 ` Russell King 2013-06-09 19:32 ` [PATCH RFC 5/8] drm/i2c: nxp-tda998x: fix npix/nline programming Russell King 2013-06-09 19:32 ` Russell King 2013-06-09 20:02 ` Sebastian Hesselbarth 2013-06-09 20:02 ` Sebastian Hesselbarth 2013-06-09 19:34 ` [PATCH RFC 6/8] drm/i2c: nxp-tda998x: prepare for video input configuration Russell King 2013-06-09 19:34 ` Russell King 2013-06-09 19:35 ` [PATCH RFC 7/8] drm/i2c: nxp-tda998x: add video and audio " Russell King 2013-06-09 19:35 ` Russell King 2013-06-09 19:36 ` [PATCH RFC 8/8] DRM: Armada: add support for drm tda19988 driver Russell King 2013-06-09 19:36 ` Russell King 2013-06-09 19:43 ` [RFC v2 0/8] rmk's Dove DRM/TDA19988 Cubox driver Russell King - ARM Linux 2013-06-09 19:43 ` Russell King - ARM Linux 2013-06-10 22:47 ` [RFC v3 0/4] " Russell King - ARM Linux 2013-06-10 22:47 ` Russell King - ARM Linux 2013-06-10 22:48 ` [PATCH RFC v3 1/4] DRM: Armada: Add Armada DRM driver Russell King 2013-06-10 22:51 ` Russell King 2013-06-10 22:49 ` [PATCH RFC v3 2/4] DRM: Armada: Add support for hardware cursors Russell King 2013-06-10 22:49 ` Russell King 2013-06-10 22:50 ` [PATCH RFC v3 3/4] DRM: Armada: convert Armada hardware cursor support to RGB+transparency Russell King 2013-06-10 22:50 ` Russell King 2013-06-10 22:51 ` [PATCH RFC v3 4/4] DRM: Armada: convert hardware cursor support to 64x32 or 32x64 ARGB Russell King 2013-06-10 22:51 ` Russell King
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAF6AEGsfzvdiGXqAu56rc-G_ZhhE_vnNbC6kpBmRtd-8Sh_uOQ@mail.gmail.com \ --to=robdclark@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.