[PATCH] drm: DON'T require each CRTC to have a unique primary plane

[PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

The ingenic-drm driver has two mutually exclusive primary planes
already; so the fact that a CRTC must have one and only one primary
plane is an invalid assumption.

Fixes: 96962e3de725 ("drm: require each CRTC to have a unique primary plane")
Cc: <stable@vger.kernel.org> # 5.11
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/drm_mode_config.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 37b4b9f0e468..d86621c41047 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -626,9 +626,7 @@ void drm_mode_config_validate(struct drm_device *dev)
 {
 	struct drm_encoder *encoder;
 	struct drm_crtc *crtc;
-	struct drm_plane *plane;
 	u32 primary_with_crtc = 0, cursor_with_crtc = 0;
-	unsigned int num_primary = 0;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return;
@@ -676,13 +674,4 @@ void drm_mode_config_validate(struct drm_device *dev)
 			cursor_with_crtc |= drm_plane_mask(crtc->cursor);
 		}
 	}
-
-	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);
 }
-- 
2.30.2

[PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

The ingenic-drm driver has two mutually exclusive primary planes
already; so the fact that a CRTC must have one and only one primary
plane is an invalid assumption.

Fixes: 96962e3de725 ("drm: require each CRTC to have a unique primary plane")
Cc: <stable@vger.kernel.org> # 5.11
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/drm_mode_config.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 37b4b9f0e468..d86621c41047 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -626,9 +626,7 @@ void drm_mode_config_validate(struct drm_device *dev)
 {
 	struct drm_encoder *encoder;
 	struct drm_crtc *crtc;
-	struct drm_plane *plane;
 	u32 primary_with_crtc = 0, cursor_with_crtc = 0;
-	unsigned int num_primary = 0;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return;
@@ -676,13 +674,4 @@ void drm_mode_config_validate(struct drm_device *dev)
 			cursor_with_crtc |= drm_plane_mask(crtc->cursor);
 		}
 	}
-
-	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);
 }
-- 
2.30.2

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

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Simon Ser]

On Saturday, March 27th, 2021 at 12:22 PM, Paul Cercueil <paul@crapouillou.net> wrote:

> The ingenic-drm driver has two mutually exclusive primary planes
> already; so the fact that a CRTC must have one and only one primary
> plane is an invalid assumption.

Why does this driver expose two primary planes, if it only has a single
CRTC?

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Simon Ser]

On Saturday, March 27th, 2021 at 12:22 PM, Paul Cercueil <paul@crapouillou.net> wrote:

> The ingenic-drm driver has two mutually exclusive primary planes
> already; so the fact that a CRTC must have one and only one primary
> plane is an invalid assumption.

Why does this driver expose two primary planes, if it only has a single
CRTC?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

It has two mutually exclusive background planes (same Z level) + one 
overlay plane.

-Paul


Le sam. 27 mars 2021 à 11:24, Simon Ser <contact@emersion.fr> a écrit 
:
> On Saturday, March 27th, 2021 at 12:22 PM, Paul Cercueil 
> <paul@crapouillou.net> wrote:
> 
>>  The ingenic-drm driver has two mutually exclusive primary planes
>>  already; so the fact that a CRTC must have one and only one primary
>>  plane is an invalid assumption.
> 
> Why does this driver expose two primary planes, if it only has a 
> single
> CRTC?

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

It has two mutually exclusive background planes (same Z level) + one 
overlay plane.

-Paul


Le sam. 27 mars 2021 à 11:24, Simon Ser <contact@emersion.fr> a écrit 
:
> On Saturday, March 27th, 2021 at 12:22 PM, Paul Cercueil 
> <paul@crapouillou.net> wrote:
> 
>>  The ingenic-drm driver has two mutually exclusive primary planes
>>  already; so the fact that a CRTC must have one and only one primary
>>  plane is an invalid assumption.
> 
> Why does this driver expose two primary planes, if it only has a 
> single
> CRTC?


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

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 972 bytes --]

On Sat, 27 Mar 2021 11:26:26 +0000
Paul Cercueil <paul@crapouillou.net> wrote:

> It has two mutually exclusive background planes (same Z level) + one 
> overlay plane.

What's the difference between the two background planes?

How will generic userspace know to pick the "right" one?


Thanks,
pq

> Le sam. 27 mars 2021 à 11:24, Simon Ser <contact@emersion.fr> a écrit 
> :
> > On Saturday, March 27th, 2021 at 12:22 PM, Paul Cercueil 
> > <paul@crapouillou.net> wrote:
> >   
> >>  The ingenic-drm driver has two mutually exclusive primary planes
> >>  already; so the fact that a CRTC must have one and only one primary
> >>  plane is an invalid assumption.  
> > 
> > Why does this driver expose two primary planes, if it only has a 
> > single
> > CRTC?  
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Pekka Paalanen]

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

On Sat, 27 Mar 2021 11:26:26 +0000
Paul Cercueil <paul@crapouillou.net> wrote:

> It has two mutually exclusive background planes (same Z level) + one 
> overlay plane.

What's the difference between the two background planes?

How will generic userspace know to pick the "right" one?


Thanks,
pq

> Le sam. 27 mars 2021 à 11:24, Simon Ser <contact@emersion.fr> a écrit 
> :
> > On Saturday, March 27th, 2021 at 12:22 PM, Paul Cercueil 
> > <paul@crapouillou.net> wrote:
> >   
> >>  The ingenic-drm driver has two mutually exclusive primary planes
> >>  already; so the fact that a CRTC must have one and only one primary
> >>  plane is an invalid assumption.  
> > 
> > Why does this driver expose two primary planes, if it only has a 
> > single
> > CRTC?  
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


[-- 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] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

Hi,

Le lun. 29 mars 2021 à 11:15, Pekka Paalanen <ppaalanen@gmail.com> a 
écrit :
> On Sat, 27 Mar 2021 11:26:26 +0000
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>>  It has two mutually exclusive background planes (same Z level) + one
>>  overlay plane.
> 
> What's the difference between the two background planes?
> 
> How will generic userspace know to pick the "right" one?

First primary plane cannot scale, supports RGB and C8. Second primary 
plane goes through the IPU, and as such can scale and convert pixel 
formats; it supports RGB, non-planar YUV, and multi-planar YUV.

Right now the userspace apps we have will simply pick the first one 
that fits the bill.

Cheers,
-Paul

>>  Le sam. 27 mars 2021 à 11:24, Simon Ser <contact@emersion.fr> a 
>> écrit
>>  :
>>  > On Saturday, March 27th, 2021 at 12:22 PM, Paul Cercueil
>>  > <paul@crapouillou.net> wrote:
>>  >
>>  >>  The ingenic-drm driver has two mutually exclusive primary planes
>>  >>  already; so the fact that a CRTC must have one and only one 
>> primary
>>  >>  plane is an invalid assumption.
>>  >
>>  > Why does this driver expose two primary planes, if it only has a
>>  > single
>>  > CRTC?
>> 
>> 
>>  _______________________________________________
>>  dri-devel mailing list
>>  dri-devel@lists.freedesktop.org
>>  https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

Hi,

Le lun. 29 mars 2021 à 11:15, Pekka Paalanen <ppaalanen@gmail.com> a 
écrit :
> On Sat, 27 Mar 2021 11:26:26 +0000
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>>  It has two mutually exclusive background planes (same Z level) + one
>>  overlay plane.
> 
> What's the difference between the two background planes?
> 
> How will generic userspace know to pick the "right" one?

First primary plane cannot scale, supports RGB and C8. Second primary 
plane goes through the IPU, and as such can scale and convert pixel 
formats; it supports RGB, non-planar YUV, and multi-planar YUV.

Right now the userspace apps we have will simply pick the first one 
that fits the bill.

Cheers,
-Paul

>>  Le sam. 27 mars 2021 à 11:24, Simon Ser <contact@emersion.fr> a 
>> écrit
>>  :
>>  > On Saturday, March 27th, 2021 at 12:22 PM, Paul Cercueil
>>  > <paul@crapouillou.net> wrote:
>>  >
>>  >>  The ingenic-drm driver has two mutually exclusive primary planes
>>  >>  already; so the fact that a CRTC must have one and only one 
>> primary
>>  >>  plane is an invalid assumption.
>>  >
>>  > Why does this driver expose two primary planes, if it only has a
>>  > single
>>  > CRTC?
>> 
>> 
>>  _______________________________________________
>>  dri-devel mailing list
>>  dri-devel@lists.freedesktop.org
>>  https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 


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

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

On Sat, Mar 27, 2021 at 11:22:14AM +0000, Paul Cercueil wrote:
> The ingenic-drm driver has two mutually exclusive primary planes
> already; so the fact that a CRTC must have one and only one primary
> plane is an invalid assumption.

I mean, no? It's been documented for a while that a CRTC should only
have a single primary, so I'd say that the invalid assumption was that
it was possible to have multiple primary planes for a CRTC.

Since it looks like you have two mutually exclusive planes, just expose
one and be done with it?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Maxime Ripard]

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

On Sat, Mar 27, 2021 at 11:22:14AM +0000, Paul Cercueil wrote:
> The ingenic-drm driver has two mutually exclusive primary planes
> already; so the fact that a CRTC must have one and only one primary
> plane is an invalid assumption.

I mean, no? It's been documented for a while that a CRTC should only
have a single primary, so I'd say that the invalid assumption was that
it was possible to have multiple primary planes for a CRTC.

Since it looks like you have two mutually exclusive planes, just expose
one and be done with it?

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 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] drm: DON'T require each CRTC to have a unique primary plane

[Simon Ser]

On Monday, March 29th, 2021 at 4:07 PM, Maxime Ripard <maxime@cerno.tech> wrote:

> Since it looks like you have two mutually exclusive planes, just expose
> one and be done with it?

You can expose the other as an overlay. Clever user-space will be able
to figure out that the more advanced plane can be used if the primary
plane is disabled.

But yeah, I don't think exposing two primary planes makes sense. The
"primary" bit is just there for legacy user-space, it's a hint that
it's the best plane to light up for fullscreen content. It has no other
significance than that, and in particular it doesn't mean that it's
incompatible with other primary planes.

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Simon Ser]

On Monday, March 29th, 2021 at 4:07 PM, Maxime Ripard <maxime@cerno.tech> wrote:

> Since it looks like you have two mutually exclusive planes, just expose
> one and be done with it?

You can expose the other as an overlay. Clever user-space will be able
to figure out that the more advanced plane can be used if the primary
plane is disabled.

But yeah, I don't think exposing two primary planes makes sense. The
"primary" bit is just there for legacy user-space, it's a hint that
it's the best plane to light up for fullscreen content. It has no other
significance than that, and in particular it doesn't mean that it's
incompatible with other primary planes.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

On Mon, 29 Mar 2021 12:41:00 +0100
Paul Cercueil <paul@crapouillou.net> wrote:

> Hi,
> 
> Le lun. 29 mars 2021 à 11:15, Pekka Paalanen <ppaalanen@gmail.com> a 
> écrit :
> > On Sat, 27 Mar 2021 11:26:26 +0000
> > Paul Cercueil <paul@crapouillou.net> wrote:
> >   
> >>  It has two mutually exclusive background planes (same Z level) + one
> >>  overlay plane.  
> > 
> > What's the difference between the two background planes?
> > 
> > How will generic userspace know to pick the "right" one?  
> 
> First primary plane cannot scale, supports RGB and C8. Second primary 
> plane goes through the IPU, and as such can scale and convert pixel 
> formats; it supports RGB, non-planar YUV, and multi-planar YUV.
> 
> Right now the userspace apps we have will simply pick the first one 
> that fits the bill.

What would be the downside of exposing just one "virtual" primary
plane, and then have the driver pick one of the two hardware planes as
appropriate per modeset?


Thanks,
pq

> >>  Le sam. 27 mars 2021 à 11:24, Simon Ser <contact@emersion.fr> a 
> >> écrit
> >>  :  
> >>  > On Saturday, March 27th, 2021 at 12:22 PM, Paul Cercueil
> >>  > <paul@crapouillou.net> wrote:
> >>  >  
> >>  >>  The ingenic-drm driver has two mutually exclusive primary planes
> >>  >>  already; so the fact that a CRTC must have one and only one   
> >> primary  
> >>  >>  plane is an invalid assumption.  
> >>  >
> >>  > Why does this driver expose two primary planes, if it only has a
> >>  > single
> >>  > CRTC?  
> >> 
> >> 
> >>  _______________________________________________
> >>  dri-devel mailing list
> >>  dri-devel@lists.freedesktop.org
> >>  https://lists.freedesktop.org/mailman/listinfo/dri-devel  
> >   
> 
> 


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

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Pekka Paalanen]

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

On Mon, 29 Mar 2021 12:41:00 +0100
Paul Cercueil <paul@crapouillou.net> wrote:

> Hi,
> 
> Le lun. 29 mars 2021 à 11:15, Pekka Paalanen <ppaalanen@gmail.com> a 
> écrit :
> > On Sat, 27 Mar 2021 11:26:26 +0000
> > Paul Cercueil <paul@crapouillou.net> wrote:
> >   
> >>  It has two mutually exclusive background planes (same Z level) + one
> >>  overlay plane.  
> > 
> > What's the difference between the two background planes?
> > 
> > How will generic userspace know to pick the "right" one?  
> 
> First primary plane cannot scale, supports RGB and C8. Second primary 
> plane goes through the IPU, and as such can scale and convert pixel 
> formats; it supports RGB, non-planar YUV, and multi-planar YUV.
> 
> Right now the userspace apps we have will simply pick the first one 
> that fits the bill.

What would be the downside of exposing just one "virtual" primary
plane, and then have the driver pick one of the two hardware planes as
appropriate per modeset?


Thanks,
pq

> >>  Le sam. 27 mars 2021 à 11:24, Simon Ser <contact@emersion.fr> a 
> >> écrit
> >>  :  
> >>  > On Saturday, March 27th, 2021 at 12:22 PM, Paul Cercueil
> >>  > <paul@crapouillou.net> wrote:
> >>  >  
> >>  >>  The ingenic-drm driver has two mutually exclusive primary planes
> >>  >>  already; so the fact that a CRTC must have one and only one   
> >> primary  
> >>  >>  plane is an invalid assumption.  
> >>  >
> >>  > Why does this driver expose two primary planes, if it only has a
> >>  > single
> >>  > CRTC?  
> >> 
> >> 
> >>  _______________________________________________
> >>  dri-devel mailing list
> >>  dri-devel@lists.freedesktop.org
> >>  https://lists.freedesktop.org/mailman/listinfo/dri-devel  
> >   
> 
> 


[-- 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] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

Hi Maxime,

Le lun. 29 mars 2021 à 16:07, Maxime Ripard <maxime@cerno.tech> a 
écrit :
> On Sat, Mar 27, 2021 at 11:22:14AM +0000, Paul Cercueil wrote:
>>  The ingenic-drm driver has two mutually exclusive primary planes
>>  already; so the fact that a CRTC must have one and only one primary
>>  plane is an invalid assumption.
> 
> I mean, no? It's been documented for a while that a CRTC should only
> have a single primary, so I'd say that the invalid assumption was that
> it was possible to have multiple primary planes for a CRTC.

Documented where?

I did read the doc of "enum drm_plane_type" in <drm/drm_plane.h>, and 
the DRM_PLANE_TYPE_PRIMARY describes my two planes, so I went with that.

-Paul

> Since it looks like you have two mutually exclusive planes, just 
> expose
> one and be done with it?
> 
> Maxime

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

Hi Maxime,

Le lun. 29 mars 2021 à 16:07, Maxime Ripard <maxime@cerno.tech> a 
écrit :
> On Sat, Mar 27, 2021 at 11:22:14AM +0000, Paul Cercueil wrote:
>>  The ingenic-drm driver has two mutually exclusive primary planes
>>  already; so the fact that a CRTC must have one and only one primary
>>  plane is an invalid assumption.
> 
> I mean, no? It's been documented for a while that a CRTC should only
> have a single primary, so I'd say that the invalid assumption was that
> it was possible to have multiple primary planes for a CRTC.

Documented where?

I did read the doc of "enum drm_plane_type" in <drm/drm_plane.h>, and 
the DRM_PLANE_TYPE_PRIMARY describes my two planes, so I went with that.

-Paul

> Since it looks like you have two mutually exclusive planes, just 
> expose
> one and be done with it?
> 
> Maxime


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

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

Le lun. 29 mars 2021 à 17:35, Pekka Paalanen <ppaalanen@gmail.com> a 
écrit :
> On Mon, 29 Mar 2021 12:41:00 +0100
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>>  Hi,
>> 
>>  Le lun. 29 mars 2021 à 11:15, Pekka Paalanen <ppaalanen@gmail.com> 
>> a
>>  écrit :
>>  > On Sat, 27 Mar 2021 11:26:26 +0000
>>  > Paul Cercueil <paul@crapouillou.net> wrote:
>>  >
>>  >>  It has two mutually exclusive background planes (same Z level) 
>> + one
>>  >>  overlay plane.
>>  >
>>  > What's the difference between the two background planes?
>>  >
>>  > How will generic userspace know to pick the "right" one?
>> 
>>  First primary plane cannot scale, supports RGB and C8. Second 
>> primary
>>  plane goes through the IPU, and as such can scale and convert pixel
>>  formats; it supports RGB, non-planar YUV, and multi-planar YUV.
>> 
>>  Right now the userspace apps we have will simply pick the first one
>>  that fits the bill.
> 
> What would be the downside of exposing just one "virtual" primary
> plane, and then have the driver pick one of the two hardware planes as
> appropriate per modeset?

The IPU plane is in a different driver, so all the callbacks are 
different. That sounds like it would be a mess.

-Paul

> Thanks,
> pq
> 
>>  >>  Le sam. 27 mars 2021 à 11:24, Simon Ser <contact@emersion.fr> a
>>  >> écrit
>>  >>  :
>>  >>  > On Saturday, March 27th, 2021 at 12:22 PM, Paul Cercueil
>>  >>  > <paul@crapouillou.net> wrote:
>>  >>  >
>>  >>  >>  The ingenic-drm driver has two mutually exclusive primary 
>> planes
>>  >>  >>  already; so the fact that a CRTC must have one and only one
>>  >> primary
>>  >>  >>  plane is an invalid assumption.
>>  >>  >
>>  >>  > Why does this driver expose two primary planes, if it only 
>> has a
>>  >>  > single
>>  >>  > CRTC?
>>  >>
>>  >>
>>  >>  _______________________________________________
>>  >>  dri-devel mailing list
>>  >>  dri-devel@lists.freedesktop.org
>>  >>  https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>  >
>> 
>> 
> 

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

Le lun. 29 mars 2021 à 17:35, Pekka Paalanen <ppaalanen@gmail.com> a 
écrit :
> On Mon, 29 Mar 2021 12:41:00 +0100
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>>  Hi,
>> 
>>  Le lun. 29 mars 2021 à 11:15, Pekka Paalanen <ppaalanen@gmail.com> 
>> a
>>  écrit :
>>  > On Sat, 27 Mar 2021 11:26:26 +0000
>>  > Paul Cercueil <paul@crapouillou.net> wrote:
>>  >
>>  >>  It has two mutually exclusive background planes (same Z level) 
>> + one
>>  >>  overlay plane.
>>  >
>>  > What's the difference between the two background planes?
>>  >
>>  > How will generic userspace know to pick the "right" one?
>> 
>>  First primary plane cannot scale, supports RGB and C8. Second 
>> primary
>>  plane goes through the IPU, and as such can scale and convert pixel
>>  formats; it supports RGB, non-planar YUV, and multi-planar YUV.
>> 
>>  Right now the userspace apps we have will simply pick the first one
>>  that fits the bill.
> 
> What would be the downside of exposing just one "virtual" primary
> plane, and then have the driver pick one of the two hardware planes as
> appropriate per modeset?

The IPU plane is in a different driver, so all the callbacks are 
different. That sounds like it would be a mess.

-Paul

> Thanks,
> pq
> 
>>  >>  Le sam. 27 mars 2021 à 11:24, Simon Ser <contact@emersion.fr> a
>>  >> écrit
>>  >>  :
>>  >>  > On Saturday, March 27th, 2021 at 12:22 PM, Paul Cercueil
>>  >>  > <paul@crapouillou.net> wrote:
>>  >>  >
>>  >>  >>  The ingenic-drm driver has two mutually exclusive primary 
>> planes
>>  >>  >>  already; so the fact that a CRTC must have one and only one
>>  >> primary
>>  >>  >>  plane is an invalid assumption.
>>  >>  >
>>  >>  > Why does this driver expose two primary planes, if it only 
>> has a
>>  >>  > single
>>  >>  > CRTC?
>>  >>
>>  >>
>>  >>  _______________________________________________
>>  >>  dri-devel mailing list
>>  >>  dri-devel@lists.freedesktop.org
>>  >>  https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>  >
>> 
>> 
> 


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

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

Hi Simon,

Le lun. 29 mars 2021 à 14:11, Simon Ser <contact@emersion.fr> a écrit 
:
> On Monday, March 29th, 2021 at 4:07 PM, Maxime Ripard 
> <maxime@cerno.tech> wrote:
> 
>>  Since it looks like you have two mutually exclusive planes, just 
>> expose
>>  one and be done with it?
> 
> You can expose the other as an overlay. Clever user-space will be able
> to figure out that the more advanced plane can be used if the primary
> plane is disabled.
> 
> But yeah, I don't think exposing two primary planes makes sense. The
> "primary" bit is just there for legacy user-space, it's a hint that
> it's the best plane to light up for fullscreen content. It has no 
> other
> significance than that, and in particular it doesn't mean that it's
> incompatible with other primary planes.

Yes, from what I understood when writing the driver, there is not much 
of a difference with primary vs. overlay planes when dealing with the 
atomic DRM API, which I used exclusively.

Making the second plane an overlay would break the ABI, which is never 
something I'm happy to do; but I'd prefer to do it now than later.

I still have concerns about the user-space being "clever" enough to 
know it can disable the primary plane. Can e.g. wlroots handle that?

Cheers,
-Paul

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

Hi Simon,

Le lun. 29 mars 2021 à 14:11, Simon Ser <contact@emersion.fr> a écrit 
:
> On Monday, March 29th, 2021 at 4:07 PM, Maxime Ripard 
> <maxime@cerno.tech> wrote:
> 
>>  Since it looks like you have two mutually exclusive planes, just 
>> expose
>>  one and be done with it?
> 
> You can expose the other as an overlay. Clever user-space will be able
> to figure out that the more advanced plane can be used if the primary
> plane is disabled.
> 
> But yeah, I don't think exposing two primary planes makes sense. The
> "primary" bit is just there for legacy user-space, it's a hint that
> it's the best plane to light up for fullscreen content. It has no 
> other
> significance than that, and in particular it doesn't mean that it's
> incompatible with other primary planes.

Yes, from what I understood when writing the driver, there is not much 
of a difference with primary vs. overlay planes when dealing with the 
atomic DRM API, which I used exclusively.

Making the second plane an overlay would break the ABI, which is never 
something I'm happy to do; but I'd prefer to do it now than later.

I still have concerns about the user-space being "clever" enough to 
know it can disable the primary plane. Can e.g. wlroots handle that?

Cheers,
-Paul


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

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 979 bytes --]

On Mon, Mar 29, 2021 at 04:15:28PM +0100, Paul Cercueil wrote:
> Hi Maxime,
> 
> Le lun. 29 mars 2021 à 16:07, Maxime Ripard <maxime@cerno.tech> a écrit :
> > On Sat, Mar 27, 2021 at 11:22:14AM +0000, Paul Cercueil wrote:
> > >  The ingenic-drm driver has two mutually exclusive primary planes
> > >  already; so the fact that a CRTC must have one and only one primary
> > >  plane is an invalid assumption.
> > 
> > I mean, no? It's been documented for a while that a CRTC should only
> > have a single primary, so I'd say that the invalid assumption was that
> > it was possible to have multiple primary planes for a CRTC.
> 
> Documented where?
> 
> I did read the doc of "enum drm_plane_type" in <drm/drm_plane.h>, and the
> DRM_PLANE_TYPE_PRIMARY describes my two planes, so I went with that.

At least since 4.9, this was in the documentation generated for DRM:
https://elixir.bootlin.com/linux/v4.9.263/source/drivers/gpu/drm/drm_plane.c#L43

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Maxime Ripard]

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

On Mon, Mar 29, 2021 at 04:15:28PM +0100, Paul Cercueil wrote:
> Hi Maxime,
> 
> Le lun. 29 mars 2021 à 16:07, Maxime Ripard <maxime@cerno.tech> a écrit :
> > On Sat, Mar 27, 2021 at 11:22:14AM +0000, Paul Cercueil wrote:
> > >  The ingenic-drm driver has two mutually exclusive primary planes
> > >  already; so the fact that a CRTC must have one and only one primary
> > >  plane is an invalid assumption.
> > 
> > I mean, no? It's been documented for a while that a CRTC should only
> > have a single primary, so I'd say that the invalid assumption was that
> > it was possible to have multiple primary planes for a CRTC.
> 
> Documented where?
> 
> I did read the doc of "enum drm_plane_type" in <drm/drm_plane.h>, and the
> DRM_PLANE_TYPE_PRIMARY describes my two planes, so I went with that.

At least since 4.9, this was in the documentation generated for DRM:
https://elixir.bootlin.com/linux/v4.9.263/source/drivers/gpu/drm/drm_plane.c#L43

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 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] drm: DON'T require each CRTC to have a unique primary plane

[Simon Ser]

On Monday, March 29th, 2021 at 5:32 PM, Paul Cercueil <paul@crapouillou.net> wrote:

> Making the second plane an overlay would break the ABI, which is never
> something I'm happy to do; but I'd prefer to do it now than later.

Yeah, I wonder if some user-space depends on this behavior somehow?

> I still have concerns about the user-space being "clever" enough to
> know it can disable the primary plane. Can e.g. wlroots handle that?

wlroots will always pick the first primary plane, and will never use
overlays. The plan is to use libliftoff [1] to make use of overlay
planes. libliftoff should already support the scenario you describe.

I think Weston supports that too.

[1]: https://github.com/emersion/libliftoff

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Simon Ser]

On Monday, March 29th, 2021 at 5:32 PM, Paul Cercueil <paul@crapouillou.net> wrote:

> Making the second plane an overlay would break the ABI, which is never
> something I'm happy to do; but I'd prefer to do it now than later.

Yeah, I wonder if some user-space depends on this behavior somehow?

> I still have concerns about the user-space being "clever" enough to
> know it can disable the primary plane. Can e.g. wlroots handle that?

wlroots will always pick the first primary plane, and will never use
overlays. The plan is to use libliftoff [1] to make use of overlay
planes. libliftoff should already support the scenario you describe.

I think Weston supports that too.

[1]: https://github.com/emersion/libliftoff
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

Le lun. 29 mars 2021 à 17:35, Maxime Ripard <maxime@cerno.tech> a 
écrit :
> On Mon, Mar 29, 2021 at 04:15:28PM +0100, Paul Cercueil wrote:
>>  Hi Maxime,
>> 
>>  Le lun. 29 mars 2021 à 16:07, Maxime Ripard <maxime@cerno.tech> a 
>> écrit :
>>  > On Sat, Mar 27, 2021 at 11:22:14AM +0000, Paul Cercueil wrote:
>>  > >  The ingenic-drm driver has two mutually exclusive primary 
>> planes
>>  > >  already; so the fact that a CRTC must have one and only one 
>> primary
>>  > >  plane is an invalid assumption.
>>  >
>>  > I mean, no? It's been documented for a while that a CRTC should 
>> only
>>  > have a single primary, so I'd say that the invalid assumption was 
>> that
>>  > it was possible to have multiple primary planes for a CRTC.
>> 
>>  Documented where?
>> 
>>  I did read the doc of "enum drm_plane_type" in <drm/drm_plane.h>, 
>> and the
>>  DRM_PLANE_TYPE_PRIMARY describes my two planes, so I went with that.
> 
> At least since 4.9, this was in the documentation generated for DRM:
> https://elixir.bootlin.com/linux/v4.9.263/source/drivers/gpu/drm/drm_plane.c#L43

Ok, I read that as "all drivers should provide AT LEAST one primary 
plane per CRTC", and not as "all drivers should provide ONE AND ONLY 
ONE primary plane per CRTC". My bad.

-Paul

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

Le lun. 29 mars 2021 à 17:35, Maxime Ripard <maxime@cerno.tech> a 
écrit :
> On Mon, Mar 29, 2021 at 04:15:28PM +0100, Paul Cercueil wrote:
>>  Hi Maxime,
>> 
>>  Le lun. 29 mars 2021 à 16:07, Maxime Ripard <maxime@cerno.tech> a 
>> écrit :
>>  > On Sat, Mar 27, 2021 at 11:22:14AM +0000, Paul Cercueil wrote:
>>  > >  The ingenic-drm driver has two mutually exclusive primary 
>> planes
>>  > >  already; so the fact that a CRTC must have one and only one 
>> primary
>>  > >  plane is an invalid assumption.
>>  >
>>  > I mean, no? It's been documented for a while that a CRTC should 
>> only
>>  > have a single primary, so I'd say that the invalid assumption was 
>> that
>>  > it was possible to have multiple primary planes for a CRTC.
>> 
>>  Documented where?
>> 
>>  I did read the doc of "enum drm_plane_type" in <drm/drm_plane.h>, 
>> and the
>>  DRM_PLANE_TYPE_PRIMARY describes my two planes, so I went with that.
> 
> At least since 4.9, this was in the documentation generated for DRM:
> https://elixir.bootlin.com/linux/v4.9.263/source/drivers/gpu/drm/drm_plane.c#L43

Ok, I read that as "all drivers should provide AT LEAST one primary 
plane per CRTC", and not as "all drivers should provide ONE AND ONLY 
ONE primary plane per CRTC". My bad.

-Paul


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

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Simon Ser]

On Monday, March 29th, 2021 at 5:39 PM, Paul Cercueil <paul@crapouillou.net> wrote:

> Ok, I read that as "all drivers should provide AT LEAST one primary
> plane per CRTC", and not as "all drivers should provide ONE AND ONLY
> ONE primary plane per CRTC". My bad.

Yeah, it's a little complicated to document, because it's possible for
a primary plane to be compatible with multiple CRTCs… We tried to
improve this [1] recently.

[1]: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#plane-abstraction

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Simon Ser]

On Monday, March 29th, 2021 at 5:39 PM, Paul Cercueil <paul@crapouillou.net> wrote:

> Ok, I read that as "all drivers should provide AT LEAST one primary
> plane per CRTC", and not as "all drivers should provide ONE AND ONLY
> ONE primary plane per CRTC". My bad.

Yeah, it's a little complicated to document, because it's possible for
a primary plane to be compatible with multiple CRTCs… We tried to
improve this [1] recently.

[1]: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#plane-abstraction
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

Le lun. 29 mars 2021 à 15:42, Simon Ser <contact@emersion.fr> a écrit 
:
> On Monday, March 29th, 2021 at 5:39 PM, Paul Cercueil 
> <paul@crapouillou.net> wrote:
> 
>>  Ok, I read that as "all drivers should provide AT LEAST one primary
>>  plane per CRTC", and not as "all drivers should provide ONE AND ONLY
>>  ONE primary plane per CRTC". My bad.
> 
> Yeah, it's a little complicated to document, because it's possible for
> a primary plane to be compatible with multiple CRTCs… We tried to
> improve this [1] recently.
> 
> [1]: 
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#plane-abstraction

Ok, that is definitely much clearer :)

-Paul

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Paul Cercueil]

Le lun. 29 mars 2021 à 15:42, Simon Ser <contact@emersion.fr> a écrit 
:
> On Monday, March 29th, 2021 at 5:39 PM, Paul Cercueil 
> <paul@crapouillou.net> wrote:
> 
>>  Ok, I read that as "all drivers should provide AT LEAST one primary
>>  plane per CRTC", and not as "all drivers should provide ONE AND ONLY
>>  ONE primary plane per CRTC". My bad.
> 
> Yeah, it's a little complicated to document, because it's possible for
> a primary plane to be compatible with multiple CRTCs… We tried to
> improve this [1] recently.
> 
> [1]: 
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#plane-abstraction

Ok, that is definitely much clearer :)

-Paul


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

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]

On Mon, 29 Mar 2021 15:36:27 +0000
Simon Ser <contact@emersion.fr> wrote:

> On Monday, March 29th, 2021 at 5:32 PM, Paul Cercueil <paul@crapouillou.net> wrote:
> 
> > Making the second plane an overlay would break the ABI, which is never
> > something I'm happy to do; but I'd prefer to do it now than later.  
> 
> Yeah, I wonder if some user-space depends on this behavior somehow?
> 
> > I still have concerns about the user-space being "clever" enough to
> > know it can disable the primary plane. Can e.g. wlroots handle that?  
> 
> wlroots will always pick the first primary plane, and will never use
> overlays. The plan is to use libliftoff [1] to make use of overlay
> planes. libliftoff should already support the scenario you describe.
> 
> I think Weston supports that too.

Weston supports overlays, but I don't think it will try without "the"
primary plane, IIRC. I'd need to verify.

I'm not quite sure what Weston would do with multiple primary planes.
It probably picks one for a CRTC ahead of time, and then sticks to it,
always using it.

But if Weston never worked with a driver to begin with, it also can't
regress, so you're safe.


Thanks,
pq

> 
> [1]: https://github.com/emersion/libliftoff
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

[Pekka Paalanen]

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

On Mon, 29 Mar 2021 15:36:27 +0000
Simon Ser <contact@emersion.fr> wrote:

> On Monday, March 29th, 2021 at 5:32 PM, Paul Cercueil <paul@crapouillou.net> wrote:
> 
> > Making the second plane an overlay would break the ABI, which is never
> > something I'm happy to do; but I'd prefer to do it now than later.  
> 
> Yeah, I wonder if some user-space depends on this behavior somehow?
> 
> > I still have concerns about the user-space being "clever" enough to
> > know it can disable the primary plane. Can e.g. wlroots handle that?  
> 
> wlroots will always pick the first primary plane, and will never use
> overlays. The plan is to use libliftoff [1] to make use of overlay
> planes. libliftoff should already support the scenario you describe.
> 
> I think Weston supports that too.

Weston supports overlays, but I don't think it will try without "the"
primary plane, IIRC. I'd need to verify.

I'm not quite sure what Weston would do with multiple primary planes.
It probably picks one for a CRTC ahead of time, and then sticks to it,
always using it.

But if Weston never worked with a driver to begin with, it also can't
regress, so you're safe.


Thanks,
pq

> 
> [1]: https://github.com/emersion/libliftoff
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


[-- 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