dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Stone <daniels@collabora.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Daniel Vetter" <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH] drm/atomic: refuse changing CRTC for planes directly
Date: Wed, 26 Aug 2015 17:33:16 +0100	[thread overview]
Message-ID: <1440606796.8123.16.camel@collabora.com> (raw)
In-Reply-To: <20150826155304.GU5176@intel.com>

On Wed, 2015-08-26 at 18:53 +0300, Ville Syrjälä wrote:
> On Wed, Aug 26, 2015 at 05:41:23PM +0200, Daniel Vetter wrote:
> > 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 I really couldn't come up with a
> > case where we don't want to enforce that. But if that ever happens
> > it
> > would be easy to move this check into helpers.
> > 
> > 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.
> > 
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Daniel Stone <daniels@collabora.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/drm_atomic.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c
> > b/drivers/gpu/drm/drm_atomic.c
> > index 434915448ea0..f27aae3fa765 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -663,6 +663,22 @@ drm_atomic_plane_get_property(struct
> > drm_plane *plane,
> >  	return 0;
> >  }
> >  
> > +static bool
> > +plane_switching(struct drm_atomic_state *state,
> > +		struct drm_plane *plane,
> > +		struct drm_plane_state *plane_state)
> 
> plane_switching_crtc()?

With Ville's shed colour:
Acked-by: Daniel Stone <daniels@collabora.com>

As this won't work in the general case, I'd prefer to see a hint to
userspace that two CRTCs are genlocked before we enable it and people
start to rely on it.

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

  reply	other threads:[~2015-08-26 16:33 UTC|newest]

Thread overview: 22+ 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-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 [this message]
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
2015-08-26 21:51       ` [Intel-gfx] " Rob Clark
2015-08-27  7:45         ` Daniel Vetter
2015-08-27  6:08       ` Maarten Lankhorst
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=1440606796.8123.16.camel@collabora.com \
    --to=daniels@collabora.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=ville.syrjala@linux.intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox