All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: DRI Development <dri-devel@lists.freedesktop.org>
Cc: Daniel Stone <daniels@collabora.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Daniel Vetter <daniel.vetter@intel.com>
Subject: [PATCH] drm/atomic: refuse changing CRTC for planes directly
Date: Wed, 26 Aug 2015 21:49:42 +0200	[thread overview]
Message-ID: <1440618582-19845-1-git-send-email-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <1440603683-15822-1-git-send-email-daniel.vetter@ffwll.ch>

Very strictly speaking this is possible if you have special hw and
genlocked CRTCs. In general switching a plane between two active CRTC
just won't work so well and is probably not tested at all. Just forbid
it.

I've put this into the core since right now no helper or driver copes
with it, no userspace has code for it and no one asks for it. Yes
there's piles of corner-cases where this would be possible to do this
like:
- switch from inactive crtc to active crtc
- swithc from active crtc to inactive crtc
- genlocked display
- invisible plane (to do whatever)
- idle plane hw due to dsi cmd mode/psr
- whatever
but looking at details it's not that easy to implement this correctly.
Hence just put it into the core and add a comment, since the only
userspace we have right now for atomic (weston) doesn't want to use
direct plane switching either.

v2: don't bother with complexity and just outright disallow plane
switching without the intermediate OFF state. Simplifies drivers, we
don't have any hw that could do it anyway and current atomic userspace
(weston) works like this already anyway.

v3: Bikeshed function name (Ville) and add comment (Rob).

v4: Also bikeshed commit message (Rob).

Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Stone <daniels@collabora.com>
Acked-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

---

Imo this is bikeshedded enough, so either someone takes this or
someone else can mangle this patch more.
-Daniel
---
 drivers/gpu/drm/drm_atomic.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 1066e4b658cf..40ddd6aa100f 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -663,6 +663,27 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
 	return 0;
 }
 
+static bool
+plane_switching_crtc(struct drm_atomic_state *state,
+		     struct drm_plane *plane,
+		     struct drm_plane_state *plane_state)
+{
+	struct drm_crtc_state *crtc_state, *curr_crtc_state;
+
+	if (!plane->state->crtc || !plane_state->crtc)
+		return false;
+
+	if (plane->state->crtc == plane_state->crtc)
+		return false;
+
+	/* This could be refined, but currently there's no helper or driver code
+	 * to implement direct switching of active planes nor userspace to take
+	 * advantage of more direct plane switching without the intermediate
+	 * full OFF state.
+	 */
+	return true;
+}
+
 /**
  * drm_atomic_plane_check - check plane state
  * @plane: plane to check
@@ -734,6 +755,12 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
 		return -ENOSPC;
 	}
 
+	if (plane_switching_crtc(state->state, plane, state)) {
+		DRM_DEBUG_ATOMIC("[PLANE:%d] switching CRTC directly\n",
+				 plane->base.id);
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2015-08-26 19:49 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-29 12:01 [PATCH 1/3] drm/atomic-helper: Add option to update planes only on active crtc Daniel Vetter
2015-07-29 12:01 ` [PATCH 2/3] drm/rcar: Only update planes " Daniel Vetter
2015-07-29 12:01 ` [PATCH 3/3] drm/atomic: refuse changing CRTC for planes while active Daniel Vetter
2015-07-30 10:37   ` shuang.he
2015-08-26 15:41   ` [PATCH] drm/atomic: refuse changing CRTC for planes directly Daniel Vetter
2015-08-26 15:53     ` [Intel-gfx] " Ville Syrjälä
2015-08-26 16:33       ` Daniel Stone
2015-08-26 16:03     ` Rob Clark
2015-08-26 16:07       ` Rob Clark
2015-08-26 16:30         ` Ville Syrjälä
2015-08-26 17:38           ` Rob Clark
2015-08-26 17:53             ` Ville Syrjälä
2015-08-26 17:58               ` Rob Clark
2015-08-26 19:49     ` Daniel Vetter [this message]
2015-08-26 21:51       ` [Intel-gfx] " Rob Clark
2015-08-27  7:45         ` Daniel Vetter
2015-08-27  6:08       ` Maarten Lankhorst
2015-08-31  2:12       ` shuang.he
2015-08-30 21:46     ` shuang.he
2015-07-30 16:28 ` [PATCH 1/3] drm/atomic-helper: Add option to update planes only on active crtc Maarten Lankhorst
2015-08-26 14:02 ` [PATCH] " Daniel Vetter
2015-09-08 10:02   ` Daniel Vetter
2015-09-08 11:10     ` Thierry Reding
2015-09-08 11:50       ` Daniel Vetter
2015-09-08 11:27     ` Laurent Pinchart

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=1440618582-19845-1-git-send-email-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=daniels@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.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.