[PATCH v3 4/4] drm: require each CRTC to have a unique primary plane

[PATCH v3 4/4] drm: require each CRTC to have a unique primary plane

[Simon Ser]

User-space expects to be able to pick a primary plane for each CRTC
exposed by the driver. Make sure this assumption holds in
drm_mode_config_validate.

Use the legacy drm_crtc.primary field to check this, because it's
simpler and we require drivers to set it anyways. Accumulate a set of
primary planes which are already used for a CRTC in a bitmask. Error out
if a primary plane is re-used.

v2: new patch

v3:
- Use u64 instead of __u64 (Jani)
- Use `unsigned int` instead of `unsigned` (Jani)

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
---
 drivers/gpu/drm/drm_mode_config.c | 21 +++++++++++++++++++++
 drivers/gpu/drm/drm_plane.c       |  6 ++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index fbe680035129..c5cf5624c106 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -626,6 +626,9 @@ void drm_mode_config_validate(struct drm_device *dev)
 {
 	struct drm_encoder *encoder;
 	struct drm_crtc *crtc;
+	struct drm_plane *plane;
+	u64 primary_with_crtc = 0, cursor_with_crtc = 0;
+	unsigned int num_primary = 0;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return;
@@ -647,12 +650,30 @@ void drm_mode_config_validate(struct drm_device *dev)
 			     "Bogus primary plane possible_crtcs: [PLANE:%d:%s] must be compatible with [CRTC:%d:%s]\n",
 			     crtc->primary->base.id, crtc->primary->name,
 			     crtc->base.id, crtc->name);
+			WARN(primary_with_crtc & BIT(crtc->primary->index),
+			     "Primary plane [PLANE:%d:%s] used for two CRTCs",
+			     crtc->primary->base.id, crtc->primary->name);
+			primary_with_crtc |= BIT(crtc->primary->index);
 		}
 		if (crtc->cursor) {
 			WARN(!(crtc->cursor->possible_crtcs & BIT(crtc->index)),
 			     "Bogus cursor plane possible_crtcs: [PLANE:%d:%s] must be compatible with [CRTC:%d:%s]\n",
 			     crtc->cursor->base.id, crtc->cursor->name,
 			     crtc->base.id, crtc->name);
+			WARN(cursor_with_crtc & BIT(crtc->cursor->index),
+			     "Primary plane [PLANE:%d:%s] used for two CRTCs",
+			     crtc->cursor->base.id, crtc->cursor->name);
+			cursor_with_crtc |= BIT(crtc->cursor->index);
 		}
 	}
+
+	drm_for_each_plane(plane, dev) {
+		if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
+			num_primary++;
+		}
+	}
+
+	WARN(num_primary != dev->mode_config.num_crtc,
+	     "Must have as many primary planes as there are CRTCs, but have %u primary planes and %u CRTCs",
+	     num_primary, dev->mode_config.num_crtc);
 }
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 49b0a8b9ac02..a1f4510efa83 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -54,6 +54,12 @@
  * enum drm_plane_type). A plane can be compatible with multiple CRTCs, see
  * &drm_plane.possible_crtcs.
  *
+ * Each CRTC must have a unique primary plane userspace can attach to enable
+ * the CRTC. In other words, userspace must be able to attach a different
+ * primary plane to each CRTC at the same time. Primary planes can still be
+ * compatible with multiple CRTCs. There must be exactly as many primary planes
+ * as there are CRTCs.
+ *
  * Legacy uAPI doesn't expose the primary and cursor planes directly. DRM core
  * relies on the driver to set the primary and optionally the cursor plane used
  * for legacy IOCTLs. This is done by calling drm_crtc_init_with_planes(). All
-- 
2.29.2


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

Re: [PATCH v3 4/4] drm: require each CRTC to have a unique primary plane

[Daniel Vetter]

On Fri, Dec 11, 2020 at 2:08 PM Simon Ser <contact@emersion.fr> wrote:
>
> User-space expects to be able to pick a primary plane for each CRTC
> exposed by the driver. Make sure this assumption holds in
> drm_mode_config_validate.
>
> Use the legacy drm_crtc.primary field to check this, because it's
> simpler and we require drivers to set it anyways. Accumulate a set of
> primary planes which are already used for a CRTC in a bitmask. Error out
> if a primary plane is re-used.
>
> v2: new patch
>
> v3:
> - Use u64 instead of __u64 (Jani)
> - Use `unsigned int` instead of `unsigned` (Jani)
>
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>

Yeah makes sense to also check this.

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

> ---
>  drivers/gpu/drm/drm_mode_config.c | 21 +++++++++++++++++++++
>  drivers/gpu/drm/drm_plane.c       |  6 ++++++
>  2 files changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index fbe680035129..c5cf5624c106 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -626,6 +626,9 @@ void drm_mode_config_validate(struct drm_device *dev)
>  {
>         struct drm_encoder *encoder;
>         struct drm_crtc *crtc;
> +       struct drm_plane *plane;
> +       u64 primary_with_crtc = 0, cursor_with_crtc = 0;
> +       unsigned int num_primary = 0;
>
>         if (!drm_core_check_feature(dev, DRIVER_MODESET))
>                 return;
> @@ -647,12 +650,30 @@ void drm_mode_config_validate(struct drm_device *dev)
>                              "Bogus primary plane possible_crtcs: [PLANE:%d:%s] must be compatible with [CRTC:%d:%s]\n",
>                              crtc->primary->base.id, crtc->primary->name,
>                              crtc->base.id, crtc->name);
> +                       WARN(primary_with_crtc & BIT(crtc->primary->index),
> +                            "Primary plane [PLANE:%d:%s] used for two CRTCs",
> +                            crtc->primary->base.id, crtc->primary->name);
> +                       primary_with_crtc |= BIT(crtc->primary->index);
>                 }
>                 if (crtc->cursor) {
>                         WARN(!(crtc->cursor->possible_crtcs & BIT(crtc->index)),
>                              "Bogus cursor plane possible_crtcs: [PLANE:%d:%s] must be compatible with [CRTC:%d:%s]\n",
>                              crtc->cursor->base.id, crtc->cursor->name,
>                              crtc->base.id, crtc->name);
> +                       WARN(cursor_with_crtc & BIT(crtc->cursor->index),
> +                            "Primary plane [PLANE:%d:%s] used for two CRTCs",
> +                            crtc->cursor->base.id, crtc->cursor->name);
> +                       cursor_with_crtc |= BIT(crtc->cursor->index);
>                 }
>         }
> +
> +       drm_for_each_plane(plane, dev) {
> +               if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> +                       num_primary++;
> +               }
> +       }
> +
> +       WARN(num_primary != dev->mode_config.num_crtc,
> +            "Must have as many primary planes as there are CRTCs, but have %u primary planes and %u CRTCs",
> +            num_primary, dev->mode_config.num_crtc);
>  }
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 49b0a8b9ac02..a1f4510efa83 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -54,6 +54,12 @@
>   * enum drm_plane_type). A plane can be compatible with multiple CRTCs, see
>   * &drm_plane.possible_crtcs.
>   *
> + * Each CRTC must have a unique primary plane userspace can attach to enable
> + * the CRTC. In other words, userspace must be able to attach a different
> + * primary plane to each CRTC at the same time. Primary planes can still be
> + * compatible with multiple CRTCs. There must be exactly as many primary planes
> + * as there are CRTCs.
> + *
>   * Legacy uAPI doesn't expose the primary and cursor planes directly. DRM core
>   * relies on the driver to set the primary and optionally the cursor plane used
>   * for legacy IOCTLs. This is done by calling drm_crtc_init_with_planes(). All
> --
> 2.29.2
>
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 4/4] drm: require each CRTC to have a unique primary plane

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 3283 bytes --]

On Fri, 11 Dec 2020 13:08:26 +0000
Simon Ser <contact@emersion.fr> wrote:

> User-space expects to be able to pick a primary plane for each CRTC
> exposed by the driver. Make sure this assumption holds in
> drm_mode_config_validate.
> 
> Use the legacy drm_crtc.primary field to check this, because it's
> simpler and we require drivers to set it anyways. Accumulate a set of
> primary planes which are already used for a CRTC in a bitmask. Error out
> if a primary plane is re-used.
> 
> v2: new patch
> 
> v3:
> - Use u64 instead of __u64 (Jani)
> - Use `unsigned int` instead of `unsigned` (Jani)
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_mode_config.c | 21 +++++++++++++++++++++
>  drivers/gpu/drm/drm_plane.c       |  6 ++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index fbe680035129..c5cf5624c106 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c

...

> +
> +	drm_for_each_plane(plane, dev) {
> +		if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> +			num_primary++;
> +		}
> +	}
> +
> +	WARN(num_primary != dev->mode_config.num_crtc,
> +	     "Must have as many primary planes as there are CRTCs, but have %u primary planes and %u CRTCs",
> +	     num_primary, dev->mode_config.num_crtc);
>  }
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 49b0a8b9ac02..a1f4510efa83 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -54,6 +54,12 @@
>   * enum drm_plane_type). A plane can be compatible with multiple CRTCs, see
>   * &drm_plane.possible_crtcs.
>   *
> + * Each CRTC must have a unique primary plane userspace can attach to enable
> + * the CRTC. In other words, userspace must be able to attach a different
> + * primary plane to each CRTC at the same time. Primary planes can still be
> + * compatible with multiple CRTCs. There must be exactly as many primary planes
> + * as there are CRTCs.
> + *
>   * Legacy uAPI doesn't expose the primary and cursor planes directly. DRM core
>   * relies on the driver to set the primary and optionally the cursor plane used
>   * for legacy IOCTLs. This is done by calling drm_crtc_init_with_planes(). All

Hi,

is there a reason why one cannot have more primary planes than CRTCs in
existence?

Daniel implied that in <20201209003637.GK401619@phenom.ffwll.local>,
but I didn't get the reason for it yet.

E.g. if all your planes are interchangeable in the sense that you can
turn on a CRTC with any one of them, would one not then expose all the
planes as "Primary"?

If the planes have other differences, like supported formats or
scaling, then marking them all "Primary" would let userspace know that
it can pick any plane with the suitable properties and expect to turn
on the CRTC with it.

Or does marking a plane as "Primary" imply something else too, like
"cannot scale"? I think Weston does make this assumption in an attempt
to hit fewer causes for failure.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v3 4/4] drm: require each CRTC to have a unique primary plane

[Simon Ser]

On Friday, December 11th, 2020 at 2:50 PM, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> is there a reason why one cannot have more primary planes than CRTCs in
> existence?
>
> Daniel implied that in <20201209003637.GK401619@phenom.ffwll.local>,
> but I didn't get the reason for it yet.
>
> E.g. if all your planes are interchangeable in the sense that you can
> turn on a CRTC with any one of them, would one not then expose all the
> planes as "Primary"?

I'm thinking of primary as a hint for simple user-space: "you can likely
light up a CRTC if you attach this plane and don't do anything crazy".
For anything more complicated, user-space uses atomic commits and can
completely ignore whether a plane is primary, cursor or overlay.

> If the planes have other differences, like supported formats or
> scaling, then marking them all "Primary" would let userspace know that
> it can pick any plane with the suitable properties and expect to turn
> on the CRTC with it.

That's interesting, but I'd bet no user-space does that. If new user-space
wants to, it's better to rely on test-only commits instead.

> Or does marking a plane as "Primary" imply something else too, like
> "cannot scale"? I think Weston does make this assumption in an attempt
> to hit fewer causes for failure.

No, AFAIK "Primary" doesn't imply something else, e.g. on amdgpu you can do
scaling on the primary plane.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 4/4] drm: require each CRTC to have a unique primary plane

[Ville Syrjälä]

On Fri, Dec 11, 2020 at 01:08:26PM +0000, Simon Ser wrote:
> User-space expects to be able to pick a primary plane for each CRTC
> exposed by the driver. Make sure this assumption holds in
> drm_mode_config_validate.
> 
> Use the legacy drm_crtc.primary field to check this, because it's
> simpler and we require drivers to set it anyways. Accumulate a set of
> primary planes which are already used for a CRTC in a bitmask. Error out
> if a primary plane is re-used.
> 
> v2: new patch
> 
> v3:
> - Use u64 instead of __u64 (Jani)
> - Use `unsigned int` instead of `unsigned` (Jani)
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_mode_config.c | 21 +++++++++++++++++++++
>  drivers/gpu/drm/drm_plane.c       |  6 ++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index fbe680035129..c5cf5624c106 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -626,6 +626,9 @@ void drm_mode_config_validate(struct drm_device *dev)
>  {
>  	struct drm_encoder *encoder;
>  	struct drm_crtc *crtc;
> +	struct drm_plane *plane;
> +	u64 primary_with_crtc = 0, cursor_with_crtc = 0;

u32

> +	unsigned int num_primary = 0;
>  
>  	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>  		return;
> @@ -647,12 +650,30 @@ void drm_mode_config_validate(struct drm_device *dev)
>  			     "Bogus primary plane possible_crtcs: [PLANE:%d:%s] must be compatible with [CRTC:%d:%s]\n",
>  			     crtc->primary->base.id, crtc->primary->name,
>  			     crtc->base.id, crtc->name);
> +			WARN(primary_with_crtc & BIT(crtc->primary->index),

drm_plane_mask() all over

> +			     "Primary plane [PLANE:%d:%s] used for two CRTCs",

s/two/multiple/ ?

> +			     crtc->primary->base.id, crtc->primary->name);
> +			primary_with_crtc |= BIT(crtc->primary->index);
>  		}
>  		if (crtc->cursor) {
>  			WARN(!(crtc->cursor->possible_crtcs & BIT(crtc->index)),
>  			     "Bogus cursor plane possible_crtcs: [PLANE:%d:%s] must be compatible with [CRTC:%d:%s]\n",
>  			     crtc->cursor->base.id, crtc->cursor->name,
>  			     crtc->base.id, crtc->name);
> +			WARN(cursor_with_crtc & BIT(crtc->cursor->index),
> +			     "Primary plane [PLANE:%d:%s] used for two CRTCs",

s/Primary/Cursor/

> +			     crtc->cursor->base.id, crtc->cursor->name);
> +			cursor_with_crtc |= BIT(crtc->cursor->index);
>  		}
>  	}
> +
> +	drm_for_each_plane(plane, dev) {
> +		if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> +			num_primary++;
> +		}
> +	}
> +
> +	WARN(num_primary != dev->mode_config.num_crtc,
> +	     "Must have as many primary planes as there are CRTCs, but have %u primary planes and %u CRTCs",
> +	     num_primary, dev->mode_config.num_crtc);
>  }
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 49b0a8b9ac02..a1f4510efa83 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -54,6 +54,12 @@
>   * enum drm_plane_type). A plane can be compatible with multiple CRTCs, see
>   * &drm_plane.possible_crtcs.
>   *
> + * Each CRTC must have a unique primary plane userspace can attach to enable
> + * the CRTC. In other words, userspace must be able to attach a different
> + * primary plane to each CRTC at the same time. Primary planes can still be
> + * compatible with multiple CRTCs. There must be exactly as many primary planes
> + * as there are CRTCs.
> + *
>   * Legacy uAPI doesn't expose the primary and cursor planes directly. DRM core
>   * relies on the driver to set the primary and optionally the cursor plane used
>   * for legacy IOCTLs. This is done by calling drm_crtc_init_with_planes(). All
> -- 
> 2.29.2
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 4/4] drm: require each CRTC to have a unique primary plane

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 2336 bytes --]

On Fri, 11 Dec 2020 14:39:35 +0000
Simon Ser <contact@emersion.fr> wrote:

> On Friday, December 11th, 2020 at 2:50 PM, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> 
> > is there a reason why one cannot have more primary planes than CRTCs in
> > existence?
> >
> > Daniel implied that in <20201209003637.GK401619@phenom.ffwll.local>,
> > but I didn't get the reason for it yet.
> >
> > E.g. if all your planes are interchangeable in the sense that you can
> > turn on a CRTC with any one of them, would one not then expose all the
> > planes as "Primary"?  
> 
> I'm thinking of primary as a hint for simple user-space: "you can likely
> light up a CRTC if you attach this plane and don't do anything crazy".
> For anything more complicated, user-space uses atomic commits and can
> completely ignore whether a plane is primary, cursor or overlay.

That's a nice reason, do we have those written down anywhere?

- plane type "Primary" is a hint to userspace that using this plane
  alone on a CRTC has the highest probability of being able to turn on
  the CRTC

- plane types are just a hint to userspace, userspace can and *should*
  use atomic test_only commits to discover more ways of making use of
  the planes (note: if this applies to cursor planes, it will invalidate
  some "optimizations" that virtual hardware drivers like vmwgfx(?)
  might do by having the cursor plane position controller directly from
  the host rather than looped through the guest)

> > If the planes have other differences, like supported formats or
> > scaling, then marking them all "Primary" would let userspace know that
> > it can pick any plane with the suitable properties and expect to turn
> > on the CRTC with it.  
> 
> That's interesting, but I'd bet no user-space does that. If new user-space
> wants to, it's better to rely on test-only commits instead.

Ok. So plane types are not a good reason to prune a compositor's testing
matrix to avoid testing some combinations.

> > Or does marking a plane as "Primary" imply something else too, like
> > "cannot scale"? I think Weston does make this assumption in an attempt
> > to hit fewer causes for failure.  
> 
> No, AFAIK "Primary" doesn't imply something else, e.g. on amdgpu you can do
> scaling on the primary plane.

Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v3 4/4] drm: require each CRTC to have a unique primary plane

[Simon Ser]

On Monday, December 14th, 2020 at 9:41 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Fri, 11 Dec 2020 14:39:35 +0000
> Simon Ser <contact@emersion.fr> wrote:
>
> > On Friday, December 11th, 2020 at 2:50 PM, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> >
> > > is there a reason why one cannot have more primary planes than CRTCs in
> > > existence?
> > >
> > > Daniel implied that in <20201209003637.GK401619@phenom.ffwll.local>,
> > > but I didn't get the reason for it yet.
> > >
> > > E.g. if all your planes are interchangeable in the sense that you can
> > > turn on a CRTC with any one of them, would one not then expose all the
> > > planes as "Primary"?
> >
> > I'm thinking of primary as a hint for simple user-space: "you can likely
> > light up a CRTC if you attach this plane and don't do anything crazy".
> > For anything more complicated, user-space uses atomic commits and can
> > completely ignore whether a plane is primary, cursor or overlay.
>
> That's a nice reason, do we have those written down anywhere?

Doesn't seem like it. The docs for enum drm_plane_type incorrectly say that the
a plane of type "Primary" will be used for legacy IOCTLs. Also, there are no
docs for the "type" property at all. I'm not even sure where to document it, as
there's no section for plane properties.

> - plane type "Primary" is a hint to userspace that using this plane
>   alone on a CRTC has the highest probability of being able to turn on
>   the CRTC
>
> - plane types are just a hint to userspace, userspace can and *should*
>   use atomic test_only commits to discover more ways of making use of
>   the planes (note: if this applies to cursor planes, it will invalidate
>   some "optimizations" that virtual hardware drivers like vmwgfx(?)
>   might do by having the cursor plane position controller directly from
>   the host rather than looped through the guest)

Yes. In an old thread, I suggested having a DRM cap that needs to be enabled
to allow the driver to de-synchronize a cursor plane's CRTC_X/Y.

> > > If the planes have other differences, like supported formats or
> > > scaling, then marking them all "Primary" would let userspace know that
> > > it can pick any plane with the suitable properties and expect to turn
> > > on the CRTC with it.
> >
> > That's interesting, but I'd bet no user-space does that. If new user-space
> > wants to, it's better to rely on test-only commits instead.
>
> Ok. So plane types are not a good reason to prune a compositor's testing
> matrix to avoid testing some combinations.

They are a hint, so in this sense they do help pruning the testing matrix. For
instance it would be impossible for user-space to figure out the right cursor
buffer parameters without DRM_CAP_CURSOR_{WIDTH,HEIGHT}. I also think there's
an undocumented assumption that the cursor buffer must be allocated with a
LINEAR layout when the driver doesn't support modifiers.

However, for this particular case I don't think the hint would be helpful.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel