All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Marcus Lorentzon <marcus.xm.lorentzon@stericsson.com>
Cc: "hoegsberg@gmail.com" <hoegsberg@gmail.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: Re: [RFC PATCH] drm: Add plane event
Date: Wed, 18 Apr 2012 17:27:57 +0200	[thread overview]
Message-ID: <20120418152757.GD20469@phenom.ffwll.local> (raw)
In-Reply-To: <4F8ED977.9090706@stericsson.com>

On Wed, Apr 18, 2012 at 05:10:47PM +0200, Marcus Lorentzon wrote:
> On 04/18/2012 04:36 PM, Daniel Vetter wrote:
> >Last time around I've discussed with people we've ended up with 2 new
> >ioctls:
> >
> >- atomic modeset, to configure the output state on more than one crtc at
> >   the same time. This is useful to get pll assignement, memory bandwidth
> >   constrains and similar stuff right. This ioctl is synchronous. A
> >   testmode can be used to inquire the driver whether the proposed mode
> >   actually works. This could be used for gui interfaces to automatically
> >   grey out unsupportable configurations, e.g. if you have 3 crtc but on 2
> >   pll and 2 modes need to have the same pixelclocks to drive all 3 crtcs.
> >
> >- an atomic pageflip. This one would be like the current pageflip ioclt,
> >   i.e. run asynchronously and deliver a drm event upont completion. The
> >   idea is that compositors can use this to make flicker-free compositition
> >   with drm planes possible. I think we want drivers to be able to indicate
> >   which crtc properties they can switch with this ioctl, e.g. I expect
> >   some yuv->rbg color space properties might not work. All the changes
> >   should be applied on the same vblank, obviously.
> Why an atomic pagefilp? How is this different from an atomic modeset
> where only fbs change? Can't drm frmwrk "optimize" this like SETCRTC
> does today with base/full modeset helpers?

The important difference is that the pageflip should happen vsynced on one
single crtc. So it can't do anything which takes longer than a vsync
(usually you need to wait a frame for the clocks to stabilize before
switching on the next stage in the output pipeline), so any output
configuration changes are pretty much out of the window. We also want
pageflip to run asynchronously and return immediately after emitting all
relevant commands. That's not really important for modeset.

So yeah, you could smash all this into one giant ioctl, but I think the
split is useful and would simplify things quite a bit on the
implementation side. Otherwise you need to add funny queries so that
userspace can figure out which modeset ops run asynchronous, which can be
vblank synced. And it needs to tell the kernel for which it wants an
event. Especially when you have multiple crtc and want to drive all of
them with a compositor, this can get funny.

Also, I'm toying around with ideas to split up the big modeset lock such
that operations that only touch the crtc (like pageflip, plane changes and
cursor changes) do not take the big modeset lock but only a per-crtc
mutex. This way a compositor running on crtc A could run without hiccups
while we do a modeset operation on crtc B, or while a hotplug detection is
reading back the edid from a unused connector (which can easily take
longer than a few frames). We have tons of bug reports from users that
complain that every 10s their cursor stalls (due to hotplug detect).

If you smash everything into one ioctl, I imagine you have plenty of fun
implementing all this. Which is why I prefer to split this up.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

  reply	other threads:[~2012-04-18 15:26 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-18  4:31 [RFC PATCH] drm: Add plane event Joonyoung Shim
2012-04-18  8:46 ` Daniel Vetter
2012-04-18 10:11   ` Joonyoung Shim
2012-04-18 12:25     ` Rob Clark
2012-04-18 14:04       ` Marcus Lorentzon
2012-04-18 14:26         ` Ville Syrjälä
2012-04-18 14:36           ` Daniel Vetter
2012-04-18 15:10             ` Marcus Lorentzon
2012-04-18 15:27               ` Daniel Vetter [this message]
2012-04-18 15:55                 ` Marcus Lorentzon
2012-04-18 16:04                   ` Jesse Barnes
2012-04-18 19:08                   ` Daniel Vetter
2012-04-18 16:06                 ` Ville Syrjälä
2012-04-18 16:26                   ` Marcus Lorentzon
2012-04-18 18:19                   ` Ville Syrjälä
2012-04-19  8:10                     ` Daniel Vetter
2012-04-18 15:43               ` Luc Verhaegen
2012-04-18 16:00                 ` Marcus Lorentzon
2012-04-19  1:24             ` Kristian Høgsberg
2012-04-19  8:05               ` Daniel Vetter
2012-04-18 14:58           ` Marcus Lorentzon
2012-04-18 15:57             ` Jesse Barnes
2012-04-18 16:47 ` Luc Verhaegen

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=20120418152757.GD20469@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hoegsberg@gmail.com \
    --cc=marcus.xm.lorentzon@stericsson.com \
    /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.