All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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: link
Be 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.