All of lore.kernel.org
 help / color / mirror / Atom feed
From: daniel@ffwll.ch (Daniel Vetter)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver
Date: Tue, 11 Jun 2013 09:33:56 +0200	[thread overview]
Message-ID: <20130611073356.GQ22870@phenom.ffwll.local> (raw)
In-Reply-To: <20130610223254.GC18614@n2100.arm.linux.org.uk>

On Mon, Jun 10, 2013 at 11:32:54PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 11, 2013 at 12:01:59AM +0200, Daniel Vetter wrote:
> > On Mon, Jun 10, 2013 at 9:59 PM, Rob Clark <robdclark@gmail.com> wrote:
> > > On Mon, Jun 10, 2013 at 1:06 PM, Russell King - ARM Linux
> > > <linux@arm.linux.org.uk> wrote:
> > >> On Mon, Jun 10, 2013 at 11:57:32AM -0400, Rob Clark wrote:
> > >>> On Sun, Jun 9, 2013 at 3:29 PM, Russell King
> > >>> <rmk+kernel@arm.linux.org.uk> wrote:
> > >>> > This patch adds support for the pair of LCD controllers on the Marvell
> > >>> > Armada 510 SoCs.  This driver supports:
> > >>> > - multiple contiguous scanout buffers for video and graphics
> > >>> > - shm backed cacheable buffer objects for X pixmaps for Vivante GPU
> > >>> >   acceleration
> > >>> > - dual lcd0 and lcd1 crt operation
> > >>> > - video overlay on each LCD crt
> > >>>
> > >>> Any particular reason for not exposing the overlays as drm_plane's?
> > >>> That is probably something that should change before merging the
> > >>> driver.
> > >>
> > >> Only through not understanding much of DRM when I started this.
> > >> Provided DRM planes can do everything that I'm already doing with
> > >> the overlay support, then yes.  Otherwise, I want to stick with this
> > >> which is modelled after the i915 overlay interface.
> > 
> > Oh i915 overlays aren't a good reason ;-) I've done those way back
> > when drm didn't have any plane infrastructure and pretty much as my
> > very contribution. So totally lacked any clue.
> > 
> > If I can scrap together a bit of time I want to port the legacy
> > overlay code over to drm planes (and remap the current ioctl interface
> > to the drm plane interface for backwards compat). But atm that's
> > crushed under a giant pile of other todo items.
> 
> Aren't we all :(  The amount of time I have to touch this has reduced
> substantially over the last couple of months, which is why there's been
> a few weeks between the RFC's for this.
> 
> The issue here is that with the overlay interface, all I have to do
> with one of these pass-by-phys-address things which the X server gets
> is to:
> 
> 1. associate the phys address with a gem object
> 2. pass it in via the overlay interface
> 
> With the DRM plane stuff, this gets more complicated:
> 
> 1. associate the phys address with a gem object
> 2. call another driver private ioctl to bind the gem object to a framebuffer
>    (there is no support for this in the generic ioctl API afaics) which
>    has to allocate and setup a framebuffer

drm_mode_addfb2 is the ioctl which packs up a bunch of driver objects
(lookup is done in the driver callback, so could in theory be something
else than gem) and wraps them up into a fb with a pile of metadata
(per-plane stride&offset, fourcc code, ...). drm_mode_addfb is the legacy
ioctl for simple packed rgb formats. The drm core has helpers to map
between the two kinds of metadata (as far as possible).

> 3. use setplane to update the plane
> 
> That all increases the expense of overlay, and adds further memory
> allocations to the path (and frees when the previous framebuffer is
> discarded.)
> 
> That all makes for higher load due to more CPU expense.

CPU load won't kill you for these, the problem was more that up to 3.8 the
locking was a bit b0rked and background probing (e.g. the output poll work
reading an edid) could block framebuffer creation and pageflips. That
should be all fixed no in 3.9 (with the execption of set_planes, for those
I'd like to use the ww mutexes ... but could also be fixed now with a few
hacks).

The addfb overhead should also be easy to amortize (if it really hurts
you) as long as you keep a queue of buffers around (like X/wayland do).
Then it's even better for planar since you avoid to do per-plane lookup
;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	"linux-arm-kernel@lists.infradead.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: Tue, 11 Jun 2013 09:33:56 +0200	[thread overview]
Message-ID: <20130611073356.GQ22870@phenom.ffwll.local> (raw)
In-Reply-To: <20130610223254.GC18614@n2100.arm.linux.org.uk>

On Mon, Jun 10, 2013 at 11:32:54PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 11, 2013 at 12:01:59AM +0200, Daniel Vetter wrote:
> > On Mon, Jun 10, 2013 at 9:59 PM, Rob Clark <robdclark@gmail.com> wrote:
> > > On Mon, Jun 10, 2013 at 1:06 PM, Russell King - ARM Linux
> > > <linux@arm.linux.org.uk> wrote:
> > >> On Mon, Jun 10, 2013 at 11:57:32AM -0400, Rob Clark wrote:
> > >>> On Sun, Jun 9, 2013 at 3:29 PM, Russell King
> > >>> <rmk+kernel@arm.linux.org.uk> wrote:
> > >>> > This patch adds support for the pair of LCD controllers on the Marvell
> > >>> > Armada 510 SoCs.  This driver supports:
> > >>> > - multiple contiguous scanout buffers for video and graphics
> > >>> > - shm backed cacheable buffer objects for X pixmaps for Vivante GPU
> > >>> >   acceleration
> > >>> > - dual lcd0 and lcd1 crt operation
> > >>> > - video overlay on each LCD crt
> > >>>
> > >>> Any particular reason for not exposing the overlays as drm_plane's?
> > >>> That is probably something that should change before merging the
> > >>> driver.
> > >>
> > >> Only through not understanding much of DRM when I started this.
> > >> Provided DRM planes can do everything that I'm already doing with
> > >> the overlay support, then yes.  Otherwise, I want to stick with this
> > >> which is modelled after the i915 overlay interface.
> > 
> > Oh i915 overlays aren't a good reason ;-) I've done those way back
> > when drm didn't have any plane infrastructure and pretty much as my
> > very contribution. So totally lacked any clue.
> > 
> > If I can scrap together a bit of time I want to port the legacy
> > overlay code over to drm planes (and remap the current ioctl interface
> > to the drm plane interface for backwards compat). But atm that's
> > crushed under a giant pile of other todo items.
> 
> Aren't we all :(  The amount of time I have to touch this has reduced
> substantially over the last couple of months, which is why there's been
> a few weeks between the RFC's for this.
> 
> The issue here is that with the overlay interface, all I have to do
> with one of these pass-by-phys-address things which the X server gets
> is to:
> 
> 1. associate the phys address with a gem object
> 2. pass it in via the overlay interface
> 
> With the DRM plane stuff, this gets more complicated:
> 
> 1. associate the phys address with a gem object
> 2. call another driver private ioctl to bind the gem object to a framebuffer
>    (there is no support for this in the generic ioctl API afaics) which
>    has to allocate and setup a framebuffer

drm_mode_addfb2 is the ioctl which packs up a bunch of driver objects
(lookup is done in the driver callback, so could in theory be something
else than gem) and wraps them up into a fb with a pile of metadata
(per-plane stride&offset, fourcc code, ...). drm_mode_addfb is the legacy
ioctl for simple packed rgb formats. The drm core has helpers to map
between the two kinds of metadata (as far as possible).

> 3. use setplane to update the plane
> 
> That all increases the expense of overlay, and adds further memory
> allocations to the path (and frees when the previous framebuffer is
> discarded.)
> 
> That all makes for higher load due to more CPU expense.

CPU load won't kill you for these, the problem was more that up to 3.8 the
locking was a bit b0rked and background probing (e.g. the output poll work
reading an edid) could block framebuffer creation and pageflips. That
should be all fixed no in 3.9 (with the execption of set_planes, for those
I'd like to use the ww mutexes ... but could also be fixed now with a few
hacks).

The addfb overhead should also be easy to amortize (if it really hurts
you) as long as you keep a queue of buffers around (like X/wayland do).
Then it's even better for planar since you avoid to do per-plane lookup
;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

  parent reply	other threads:[~2013-06-11  7:33 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
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 [this message]
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=20130611073356.GQ22870@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --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.