[v3,7/7] drm: Allow drivers to leave encoder->possible_crtcs==0
diff mbox series

Message ID 20200211162208.16224-8-ville.syrjala@linux.intel.com
State New
Headers show
Series
  • drm: Try to fix encoder possible_clones/crtc
Related show

Commit Message

Ville Syrjälä Feb. 11, 2020, 4:22 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Let's simplify life of driver by allowing them to leave
encoder->possible_crtcs unset if they have no restrictions
in crtc<->encoder linkage. We'll just populate possible_crtcs
with the full crtc mask when registering the encoder so that
userspace doesn't have to deal with drivers not populating
this correctly.

Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
We might not actually need/want this, but included it here for
future reference if that assumption turns out to be wrong.
---
 drivers/gpu/drm/drm_mode_config.c | 15 ++++++++++++++-
 include/drm/drm_encoder.h         |  4 ++++
 2 files changed, 18 insertions(+), 1 deletion(-)

Comments

Daniel Vetter Feb. 11, 2020, 5:05 p.m. UTC | #1
On Tue, Feb 11, 2020 at 06:22:08PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Let's simplify life of driver by allowing them to leave
> encoder->possible_crtcs unset if they have no restrictions
> in crtc<->encoder linkage. We'll just populate possible_crtcs
> with the full crtc mask when registering the encoder so that
> userspace doesn't have to deal with drivers not populating
> this correctly.
> 
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> We might not actually need/want this, but included it here for
> future reference if that assumption turns out to be wrong.

I think this one is most definitely needed. _Lots_ of drivers get this
toally wrong and just leave the value blank. It's encoded as official
fallback in most userspace compositors.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_mode_config.c | 15 ++++++++++++++-
>  include/drm/drm_encoder.h         |  4 ++++
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 4c1b350ddb95..ce18c3dd0bde 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -592,6 +592,17 @@ static u32 full_crtc_mask(struct drm_device *dev)
>  	return crtc_mask;
>  }
>  
> +/*
> + * Make life easy for drivers by allowing them to leave
> + * possible_crtcs unset if there are not crtc<->encoder
> + * restrictions.
> + */
> +static void fixup_encoder_possible_crtcs(struct drm_encoder *encoder)
> +{
> +	if (encoder->possible_crtcs == 0)
> +		encoder->possible_crtcs = full_crtc_mask(encoder->dev);
> +}
> +
>  static void validate_encoder_possible_crtcs(struct drm_encoder *encoder)
>  {
>  	u32 crtc_mask = full_crtc_mask(encoder->dev);
> @@ -608,8 +619,10 @@ void drm_mode_config_validate(struct drm_device *dev)
>  {
>  	struct drm_encoder *encoder;
>  
> -	drm_for_each_encoder(encoder, dev)
> +	drm_for_each_encoder(encoder, dev) {
>  		fixup_encoder_possible_clones(encoder);
> +		fixup_encoder_possible_crtcs(encoder);
> +	}
>  
>  	drm_for_each_encoder(encoder, dev) {
>  		validate_encoder_possible_clones(encoder);
> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> index b236269f41ac..bd033c5618bf 100644
> --- a/include/drm/drm_encoder.h
> +++ b/include/drm/drm_encoder.h
> @@ -142,6 +142,10 @@ struct drm_encoder {
>  	 * the bits for all &drm_crtc objects this encoder can be connected to
>  	 * before calling drm_dev_register().
>  	 *
> +	 * As an exception to the above rule if any crtc can be connected to
> +	 * the encoder the driver can leave @possible_crtcs set to 0. The core
> +	 * will automagically fix this up by setting the bit for every crtc.
> +	 *
>  	 * You will get a WARN if you get this wrong in the driver.
>  	 *
>  	 * Note that since CRTC objects can't be hotplugged the assigned indices
> -- 
> 2.24.1
>
Ville Syrjälä Feb. 11, 2020, 5:14 p.m. UTC | #2
On Tue, Feb 11, 2020 at 06:05:45PM +0100, Daniel Vetter wrote:
> On Tue, Feb 11, 2020 at 06:22:08PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Let's simplify life of driver by allowing them to leave
> > encoder->possible_crtcs unset if they have no restrictions
> > in crtc<->encoder linkage. We'll just populate possible_crtcs
> > with the full crtc mask when registering the encoder so that
> > userspace doesn't have to deal with drivers not populating
> > this correctly.
> > 
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > We might not actually need/want this, but included it here for
> > future reference if that assumption turns out to be wrong.
> 
> I think this one is most definitely needed. _Lots_ of drivers get this
> toally wrong and just leave the value blank. It's encoded as official
> fallback in most userspace compositors.

OK. It's been a while since I dug around so can't really remmber how
this was being handled. I'll reorder before pushing.

> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > ---
> >  drivers/gpu/drm/drm_mode_config.c | 15 ++++++++++++++-
> >  include/drm/drm_encoder.h         |  4 ++++
> >  2 files changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> > index 4c1b350ddb95..ce18c3dd0bde 100644
> > --- a/drivers/gpu/drm/drm_mode_config.c
> > +++ b/drivers/gpu/drm/drm_mode_config.c
> > @@ -592,6 +592,17 @@ static u32 full_crtc_mask(struct drm_device *dev)
> >  	return crtc_mask;
> >  }
> >  
> > +/*
> > + * Make life easy for drivers by allowing them to leave
> > + * possible_crtcs unset if there are not crtc<->encoder
> > + * restrictions.
> > + */
> > +static void fixup_encoder_possible_crtcs(struct drm_encoder *encoder)
> > +{
> > +	if (encoder->possible_crtcs == 0)
> > +		encoder->possible_crtcs = full_crtc_mask(encoder->dev);
> > +}
> > +
> >  static void validate_encoder_possible_crtcs(struct drm_encoder *encoder)
> >  {
> >  	u32 crtc_mask = full_crtc_mask(encoder->dev);
> > @@ -608,8 +619,10 @@ void drm_mode_config_validate(struct drm_device *dev)
> >  {
> >  	struct drm_encoder *encoder;
> >  
> > -	drm_for_each_encoder(encoder, dev)
> > +	drm_for_each_encoder(encoder, dev) {
> >  		fixup_encoder_possible_clones(encoder);
> > +		fixup_encoder_possible_crtcs(encoder);
> > +	}
> >  
> >  	drm_for_each_encoder(encoder, dev) {
> >  		validate_encoder_possible_clones(encoder);
> > diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> > index b236269f41ac..bd033c5618bf 100644
> > --- a/include/drm/drm_encoder.h
> > +++ b/include/drm/drm_encoder.h
> > @@ -142,6 +142,10 @@ struct drm_encoder {
> >  	 * the bits for all &drm_crtc objects this encoder can be connected to
> >  	 * before calling drm_dev_register().
> >  	 *
> > +	 * As an exception to the above rule if any crtc can be connected to
> > +	 * the encoder the driver can leave @possible_crtcs set to 0. The core
> > +	 * will automagically fix this up by setting the bit for every crtc.
> > +	 *
> >  	 * You will get a WARN if you get this wrong in the driver.
> >  	 *
> >  	 * Note that since CRTC objects can't be hotplugged the assigned indices
> > -- 
> > 2.24.1
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Daniel Vetter Feb. 12, 2020, 9:07 a.m. UTC | #3
On Tue, Feb 11, 2020 at 07:14:51PM +0200, Ville Syrjälä wrote:
> On Tue, Feb 11, 2020 at 06:05:45PM +0100, Daniel Vetter wrote:
> > On Tue, Feb 11, 2020 at 06:22:08PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Let's simplify life of driver by allowing them to leave
> > > encoder->possible_crtcs unset if they have no restrictions
> > > in crtc<->encoder linkage. We'll just populate possible_crtcs
> > > with the full crtc mask when registering the encoder so that
> > > userspace doesn't have to deal with drivers not populating
> > > this correctly.
> > > 
> > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > We might not actually need/want this, but included it here for
> > > future reference if that assumption turns out to be wrong.
> > 
> > I think this one is most definitely needed. _Lots_ of drivers get this
> > toally wrong and just leave the value blank. It's encoded as official
> > fallback in most userspace compositors.
> 
> OK. It's been a while since I dug around so can't really remmber how
> this was being handled. I'll reorder before pushing.

Hm otoh having "works with all crtcs" as default is a bit dangerous,
whereas the "cannot be cloned" default for possible_clones is perfectly
safe.

So now I'm kinda not sure whether this is a bright idea, and we shouldn't
just eat the cost of fixing up all the various WARNING backtraces your
previous patch triggers. I've done a full review and the following look
suspect:

- tegara/sor.c Strangely it's the only one, the other output drivers do
  seem to set the possible_crtcs mask to something useful.

Everything else seems to be fine. I'd say let's drop this patch, and I'm
adding Thierry to shed some light on what's going on in tegra/sor.c.
-Daniel

> 
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > > ---
> > >  drivers/gpu/drm/drm_mode_config.c | 15 ++++++++++++++-
> > >  include/drm/drm_encoder.h         |  4 ++++
> > >  2 files changed, 18 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> > > index 4c1b350ddb95..ce18c3dd0bde 100644
> > > --- a/drivers/gpu/drm/drm_mode_config.c
> > > +++ b/drivers/gpu/drm/drm_mode_config.c
> > > @@ -592,6 +592,17 @@ static u32 full_crtc_mask(struct drm_device *dev)
> > >  	return crtc_mask;
> > >  }
> > >  
> > > +/*
> > > + * Make life easy for drivers by allowing them to leave
> > > + * possible_crtcs unset if there are not crtc<->encoder
> > > + * restrictions.
> > > + */
> > > +static void fixup_encoder_possible_crtcs(struct drm_encoder *encoder)
> > > +{
> > > +	if (encoder->possible_crtcs == 0)
> > > +		encoder->possible_crtcs = full_crtc_mask(encoder->dev);
> > > +}
> > > +
> > >  static void validate_encoder_possible_crtcs(struct drm_encoder *encoder)
> > >  {
> > >  	u32 crtc_mask = full_crtc_mask(encoder->dev);
> > > @@ -608,8 +619,10 @@ void drm_mode_config_validate(struct drm_device *dev)
> > >  {
> > >  	struct drm_encoder *encoder;
> > >  
> > > -	drm_for_each_encoder(encoder, dev)
> > > +	drm_for_each_encoder(encoder, dev) {
> > >  		fixup_encoder_possible_clones(encoder);
> > > +		fixup_encoder_possible_crtcs(encoder);
> > > +	}
> > >  
> > >  	drm_for_each_encoder(encoder, dev) {
> > >  		validate_encoder_possible_clones(encoder);
> > > diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> > > index b236269f41ac..bd033c5618bf 100644
> > > --- a/include/drm/drm_encoder.h
> > > +++ b/include/drm/drm_encoder.h
> > > @@ -142,6 +142,10 @@ struct drm_encoder {
> > >  	 * the bits for all &drm_crtc objects this encoder can be connected to
> > >  	 * before calling drm_dev_register().
> > >  	 *
> > > +	 * As an exception to the above rule if any crtc can be connected to
> > > +	 * the encoder the driver can leave @possible_crtcs set to 0. The core
> > > +	 * will automagically fix this up by setting the bit for every crtc.
> > > +	 *
> > >  	 * You will get a WARN if you get this wrong in the driver.
> > >  	 *
> > >  	 * Note that since CRTC objects can't be hotplugged the assigned indices
> > > -- 
> > > 2.24.1
> > > 
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Ville Syrjälä
> Intel
Daniel Vetter Feb. 12, 2020, 9:08 a.m. UTC | #4
On Wed, Feb 12, 2020 at 10:07:55AM +0100, Daniel Vetter wrote:
> On Tue, Feb 11, 2020 at 07:14:51PM +0200, Ville Syrjälä wrote:
> > On Tue, Feb 11, 2020 at 06:05:45PM +0100, Daniel Vetter wrote:
> > > On Tue, Feb 11, 2020 at 06:22:08PM +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Let's simplify life of driver by allowing them to leave
> > > > encoder->possible_crtcs unset if they have no restrictions
> > > > in crtc<->encoder linkage. We'll just populate possible_crtcs
> > > > with the full crtc mask when registering the encoder so that
> > > > userspace doesn't have to deal with drivers not populating
> > > > this correctly.
> > > > 
> > > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > > We might not actually need/want this, but included it here for
> > > > future reference if that assumption turns out to be wrong.
> > > 
> > > I think this one is most definitely needed. _Lots_ of drivers get this
> > > toally wrong and just leave the value blank. It's encoded as official
> > > fallback in most userspace compositors.
> > 
> > OK. It's been a while since I dug around so can't really remmber how
> > this was being handled. I'll reorder before pushing.
> 
> Hm otoh having "works with all crtcs" as default is a bit dangerous,
> whereas the "cannot be cloned" default for possible_clones is perfectly
> safe.
> 
> So now I'm kinda not sure whether this is a bright idea, and we shouldn't
> just eat the cost of fixing up all the various WARNING backtraces your
> previous patch triggers. I've done a full review and the following look
> suspect:
> 
> - tegara/sor.c Strangely it's the only one, the other output drivers do
>   seem to set the possible_crtcs mask to something useful.

Strike that, it sets it using tegra_output_find_possible_crtcs().

I think everything is good and we really don't need this patch here to fix
up possible_crtcs.
-Daniel

> 
> Everything else seems to be fine. I'd say let's drop this patch, and I'm
> adding Thierry to shed some light on what's going on in tegra/sor.c.
> -Daniel
> 
> > 
> > > 
> > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > 
> > > > ---
> > > >  drivers/gpu/drm/drm_mode_config.c | 15 ++++++++++++++-
> > > >  include/drm/drm_encoder.h         |  4 ++++
> > > >  2 files changed, 18 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> > > > index 4c1b350ddb95..ce18c3dd0bde 100644
> > > > --- a/drivers/gpu/drm/drm_mode_config.c
> > > > +++ b/drivers/gpu/drm/drm_mode_config.c
> > > > @@ -592,6 +592,17 @@ static u32 full_crtc_mask(struct drm_device *dev)
> > > >  	return crtc_mask;
> > > >  }
> > > >  
> > > > +/*
> > > > + * Make life easy for drivers by allowing them to leave
> > > > + * possible_crtcs unset if there are not crtc<->encoder
> > > > + * restrictions.
> > > > + */
> > > > +static void fixup_encoder_possible_crtcs(struct drm_encoder *encoder)
> > > > +{
> > > > +	if (encoder->possible_crtcs == 0)
> > > > +		encoder->possible_crtcs = full_crtc_mask(encoder->dev);
> > > > +}
> > > > +
> > > >  static void validate_encoder_possible_crtcs(struct drm_encoder *encoder)
> > > >  {
> > > >  	u32 crtc_mask = full_crtc_mask(encoder->dev);
> > > > @@ -608,8 +619,10 @@ void drm_mode_config_validate(struct drm_device *dev)
> > > >  {
> > > >  	struct drm_encoder *encoder;
> > > >  
> > > > -	drm_for_each_encoder(encoder, dev)
> > > > +	drm_for_each_encoder(encoder, dev) {
> > > >  		fixup_encoder_possible_clones(encoder);
> > > > +		fixup_encoder_possible_crtcs(encoder);
> > > > +	}
> > > >  
> > > >  	drm_for_each_encoder(encoder, dev) {
> > > >  		validate_encoder_possible_clones(encoder);
> > > > diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> > > > index b236269f41ac..bd033c5618bf 100644
> > > > --- a/include/drm/drm_encoder.h
> > > > +++ b/include/drm/drm_encoder.h
> > > > @@ -142,6 +142,10 @@ struct drm_encoder {
> > > >  	 * the bits for all &drm_crtc objects this encoder can be connected to
> > > >  	 * before calling drm_dev_register().
> > > >  	 *
> > > > +	 * As an exception to the above rule if any crtc can be connected to
> > > > +	 * the encoder the driver can leave @possible_crtcs set to 0. The core
> > > > +	 * will automagically fix this up by setting the bit for every crtc.
> > > > +	 *
> > > >  	 * You will get a WARN if you get this wrong in the driver.
> > > >  	 *
> > > >  	 * Note that since CRTC objects can't be hotplugged the assigned indices
> > > > -- 
> > > > 2.24.1
> > > > 
> > > 
> > > -- 
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Ville Syrjälä March 18, 2020, 4:44 p.m. UTC | #5
On Wed, Feb 12, 2020 at 10:08:49AM +0100, Daniel Vetter wrote:
> On Wed, Feb 12, 2020 at 10:07:55AM +0100, Daniel Vetter wrote:
> > On Tue, Feb 11, 2020 at 07:14:51PM +0200, Ville Syrjälä wrote:
> > > On Tue, Feb 11, 2020 at 06:05:45PM +0100, Daniel Vetter wrote:
> > > > On Tue, Feb 11, 2020 at 06:22:08PM +0200, Ville Syrjala wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > Let's simplify life of driver by allowing them to leave
> > > > > encoder->possible_crtcs unset if they have no restrictions
> > > > > in crtc<->encoder linkage. We'll just populate possible_crtcs
> > > > > with the full crtc mask when registering the encoder so that
> > > > > userspace doesn't have to deal with drivers not populating
> > > > > this correctly.
> > > > > 
> > > > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > ---
> > > > > We might not actually need/want this, but included it here for
> > > > > future reference if that assumption turns out to be wrong.
> > > > 
> > > > I think this one is most definitely needed. _Lots_ of drivers get this
> > > > toally wrong and just leave the value blank. It's encoded as official
> > > > fallback in most userspace compositors.
> > > 
> > > OK. It's been a while since I dug around so can't really remmber how
> > > this was being handled. I'll reorder before pushing.
> > 
> > Hm otoh having "works with all crtcs" as default is a bit dangerous,
> > whereas the "cannot be cloned" default for possible_clones is perfectly
> > safe.
> > 
> > So now I'm kinda not sure whether this is a bright idea, and we shouldn't
> > just eat the cost of fixing up all the various WARNING backtraces your
> > previous patch triggers. I've done a full review and the following look
> > suspect:
> > 
> > - tegara/sor.c Strangely it's the only one, the other output drivers do
> >   seem to set the possible_crtcs mask to something useful.
> 
> Strike that, it sets it using tegra_output_find_possible_crtcs().
> 
> I think everything is good and we really don't need this patch here to fix
> up possible_crtcs.

Finally pushed the other patches from the series to drm-misc-next.
Thanks for the reviews.

Should the new possible_{crtcs,clones} WARNs start to trigger for
anyone despite our best efforts, please holler and I'll look into
what needs fixing.

Patch
diff mbox series

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 4c1b350ddb95..ce18c3dd0bde 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -592,6 +592,17 @@  static u32 full_crtc_mask(struct drm_device *dev)
 	return crtc_mask;
 }
 
+/*
+ * Make life easy for drivers by allowing them to leave
+ * possible_crtcs unset if there are not crtc<->encoder
+ * restrictions.
+ */
+static void fixup_encoder_possible_crtcs(struct drm_encoder *encoder)
+{
+	if (encoder->possible_crtcs == 0)
+		encoder->possible_crtcs = full_crtc_mask(encoder->dev);
+}
+
 static void validate_encoder_possible_crtcs(struct drm_encoder *encoder)
 {
 	u32 crtc_mask = full_crtc_mask(encoder->dev);
@@ -608,8 +619,10 @@  void drm_mode_config_validate(struct drm_device *dev)
 {
 	struct drm_encoder *encoder;
 
-	drm_for_each_encoder(encoder, dev)
+	drm_for_each_encoder(encoder, dev) {
 		fixup_encoder_possible_clones(encoder);
+		fixup_encoder_possible_crtcs(encoder);
+	}
 
 	drm_for_each_encoder(encoder, dev) {
 		validate_encoder_possible_clones(encoder);
diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index b236269f41ac..bd033c5618bf 100644
--- a/include/drm/drm_encoder.h
+++ b/include/drm/drm_encoder.h
@@ -142,6 +142,10 @@  struct drm_encoder {
 	 * the bits for all &drm_crtc objects this encoder can be connected to
 	 * before calling drm_dev_register().
 	 *
+	 * As an exception to the above rule if any crtc can be connected to
+	 * the encoder the driver can leave @possible_crtcs set to 0. The core
+	 * will automagically fix this up by setting the bit for every crtc.
+	 *
 	 * You will get a WARN if you get this wrong in the driver.
 	 *
 	 * Note that since CRTC objects can't be hotplugged the assigned indices