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