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 18:49:06 -0400 [thread overview] Message-ID: <CAF6AEGu-e+cZW-1F6L+epE11oonFucreEY_KH+awRx7j5pScpA@mail.gmail.com> (raw) In-Reply-To: <20130610211516.GZ18614@n2100.arm.linux.org.uk> On Mon, Jun 10, 2013 at 5:15 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Mon, Jun 10, 2013 at 05:01:18PM -0400, Rob Clark wrote: >> 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 > > The bounds check does what? Check that the buffer object's physical > address is within another driver's range? Fine, but all that it's > doing is preventing it being associated with a buffer object which can > _only_ be used for _read_ access by the LCD controller. There is no > write access to the GEM objects by anything that this DRM driver talks > to. I was assuming there was a global carveout pool, so it would be just one upper/lower check. But from what you said and looking at the allocation code in your driver, I guess this is not actually the case. > So all it'll do is prevent you passing rogue addresses to the LCD > controller for scanout. > > And how do we get that information into the driver from other random > drivers? Hack in random inter-driver specific APIs to do it? Come > up with yet another different way to try and identify whether a > particular resource is a block of reserved memory for this driver's > usage as a linear region or one of the others? How. Really, please > think about the problem. > > The benefit vs the complexity involved really isn't worth it. yeah, that suggestion was based on a wrong assumption about how the phys contig buffers are allocated and passed. So disregard it. I guess in the short term, the best I can think is keep those phys ioctls as a small patch on top of the upstream driver. It is ok to leave place-holder ioctl #'s in the upstream driver so that the ioctl #'s don't shift when you rebase. And then try to get the vendor to add support for dmabuf so that the patch on top of upstream can eventually be dropped. Maybe someone else has a better suggestion, but I don't think we can merge those phys ioctls upstream, and I'd really hate to 'throw the baby out with the bathwater' in this case and not at least get the modesetting part of the driver merged. 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 18:49:06 -0400 [thread overview] Message-ID: <CAF6AEGu-e+cZW-1F6L+epE11oonFucreEY_KH+awRx7j5pScpA@mail.gmail.com> (raw) In-Reply-To: <20130610211516.GZ18614@n2100.arm.linux.org.uk> On Mon, Jun 10, 2013 at 5:15 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Mon, Jun 10, 2013 at 05:01:18PM -0400, Rob Clark wrote: >> 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 > > The bounds check does what? Check that the buffer object's physical > address is within another driver's range? Fine, but all that it's > doing is preventing it being associated with a buffer object which can > _only_ be used for _read_ access by the LCD controller. There is no > write access to the GEM objects by anything that this DRM driver talks > to. I was assuming there was a global carveout pool, so it would be just one upper/lower check. But from what you said and looking at the allocation code in your driver, I guess this is not actually the case. > So all it'll do is prevent you passing rogue addresses to the LCD > controller for scanout. > > And how do we get that information into the driver from other random > drivers? Hack in random inter-driver specific APIs to do it? Come > up with yet another different way to try and identify whether a > particular resource is a block of reserved memory for this driver's > usage as a linear region or one of the others? How. Really, please > think about the problem. > > The benefit vs the complexity involved really isn't worth it. yeah, that suggestion was based on a wrong assumption about how the phys contig buffers are allocated and passed. So disregard it. I guess in the short term, the best I can think is keep those phys ioctls as a small patch on top of the upstream driver. It is ok to leave place-holder ioctl #'s in the upstream driver so that the ioctl #'s don't shift when you rebase. And then try to get the vendor to add support for dmabuf so that the patch on top of upstream can eventually be dropped. Maybe someone else has a better suggestion, but I don't think we can merge those phys ioctls upstream, and I'd really hate to 'throw the baby out with the bathwater' in this case and not at least get the modesetting part of the driver merged. BR, -R
next prev parent reply other threads:[~2013-06-10 22:49 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 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 [this message] 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=CAF6AEGu-e+cZW-1F6L+epE11oonFucreEY_KH+awRx7j5pScpA@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.