[PATCH v4 0/5] drm/blend: Support generic plane-wide alpha

[PATCH v4 0/5] drm/blend: Support generic plane-wide alpha

[Maxime Ripard]

Hi,

This serie aims at enhancing the support for plane-wide alpha in the
drivers that are implementing it at the moment, by turning it into a
generic property and converting the drivers (rcar-du and atmel-hclcdc). It
also introduces support for it in the sun4i driver.

Let me know what you think,
Maxime

Changes from v3:
  - Rebased on current drm-misc-next

  - Made the alpha property a 16 bits property, and have the drivers
    drop the lowest 8 bits
  - Removed the csv documentation, and documented it in the doc instead

Changes from v2:
  - Rebased on current drm-misc-next
  - Removed the patches already applied
  - Split the patch implementing the automatic pipe assignment in two

Changes from v1:
  - Document the behaviour on concurrent usage of the alpha property and an
    alpha component in the format
  - Allowed for higher alpha values
  - Moved the alpha value from a helper to the struct drm_format_info
  - Collected tags
  - Rebased on current drm-misc-next

Maxime Ripard (5):
  drm/blend: Add a generic alpha property
  drm/atmel-hclcdc: Convert to the new generic alpha property
  drm/rcar-du: Convert to the new generic alpha property
  drm/sun4i: Add support for plane alpha
  drm/docs: Remove the rcar alpha from the csv file

 Documentation/gpu/kms-properties.csv            |  1 +-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h    | 13 +---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 89 ++----------------
 drivers/gpu/drm/drm_atomic.c                    |  4 +-
 drivers/gpu/drm/drm_atomic_helper.c             |  4 +-
 drivers/gpu/drm/drm_blend.c                     | 38 ++++++++-
 drivers/gpu/drm/rcar-du/rcar_du_drv.h           |  1 +-
 drivers/gpu/drm/rcar-du/rcar_du_kms.c           |  5 +-
 drivers/gpu/drm/rcar-du/rcar_du_plane.c         | 15 +--
 drivers/gpu/drm/rcar-du/rcar_du_plane.h         |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c           | 42 +--------
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h           |  2 +-
 drivers/gpu/drm/sun4i/sun4i_backend.c           | 16 ++-
 drivers/gpu/drm/sun4i/sun4i_backend.h           |  3 +-
 drivers/gpu/drm/sun4i/sun4i_layer.c             |  2 +-
 include/drm/drm_blend.h                         |  3 +-
 include/drm/drm_plane.h                         |  6 +-
 17 files changed, 96 insertions(+), 150 deletions(-)

base-commit: 9c936b12f15019b38edb5f8bae77bb5b0046d1b7
-- 
git-series 0.9.1

[PATCH v4 1/5] drm/blend: Add a generic alpha property

[Maxime Ripard]

Some drivers duplicate the logic to create a property to store a per-plane
alpha.

This is especially useful if we ever want to support extra protocols for
Wayland like:
https://lists.freedesktop.org/archives/wayland-devel/2017-August/034741.html

Let's create a helper in order to move that to the core.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/drm_atomic.c        |  4 +++-
 drivers/gpu/drm/drm_atomic_helper.c |  4 +++-
 drivers/gpu/drm/drm_blend.c         | 38 ++++++++++++++++++++++++++++++-
 include/drm/drm_blend.h             |  3 ++-
 include/drm/drm_plane.h             |  6 +++++-
 5 files changed, 55 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 34b7d420e555..39204c88d2c5 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -753,6 +753,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 		state->src_w = val;
 	} else if (property == config->prop_src_h) {
 		state->src_h = val;
+	} else if (property == plane->alpha_property) {
+		state->alpha = val;
 	} else if (property == plane->rotation_property) {
 		if (!is_power_of_2(val & DRM_MODE_ROTATE_MASK))
 			return -EINVAL;
@@ -818,6 +820,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
 		*val = state->src_w;
 	} else if (property == config->prop_src_h) {
 		*val = state->src_h;
+	} else if (property == plane->alpha_property) {
+		*val = state->alpha;
 	} else if (property == plane->rotation_property) {
 		*val = state->rotation;
 	} else if (property == plane->zpos_property) {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 00c78c1c9681..ac4c3f18a0b1 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3484,6 +3484,10 @@ void drm_atomic_helper_plane_reset(struct drm_plane *plane)
 	if (plane->state) {
 		plane->state->plane = plane;
 		plane->state->rotation = DRM_MODE_ROTATE_0;
+
+		/* Reset the alpha value to fully opaque if it matters */
+		if (plane->alpha_property)
+			plane->state->alpha = plane->alpha_property->values[1];
 	}
 }
 EXPORT_SYMBOL(drm_atomic_helper_plane_reset);
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 5a81e1b4c076..05eda2d57c77 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -88,6 +88,12 @@
  * On top of this basic transformation additional properties can be exposed by
  * the driver:
  *
+ * alpha:
+ * 	Alpha is setup with drm_plane_create_alpha_property(). It controls the
+ * 	plane-wide opacity, from transparent (0) to opaque (0xffff). It can be
+ * 	combined with pixel alpha.
+ * 	The alpha value is represented as premultiplied alpha.
+ *
  * rotation:
  *	Rotation is set up with drm_plane_create_rotation_property(). It adds a
  *	rotation and reflection step between the source and destination rectangles.
@@ -106,6 +112,38 @@
  */
 
 /**
+ * drm_plane_create_alpha_property - create a new alpha property
+ * @plane: drm plane
+ *
+ * This function creates a generic, mutable, alpha property and enables support
+ * for it in the DRM core. It is attached to @plane.
+ *
+ * The alpha property will be allowed to be within the bounds of 0
+ * (transparent) to 0xffff (opaque).
+ *
+ * Returns:
+ * 0 on success, negative error code on failure.
+ */
+int drm_plane_create_alpha_property(struct drm_plane *plane)
+{
+	struct drm_property *prop;
+
+	prop = drm_property_create_range(plane->dev, 0, "alpha",
+					 0, DRM_BLEND_ALPHA_OPAQUE);
+	if (!prop)
+		return -ENOMEM;
+
+	drm_object_attach_property(&plane->base, prop, DRM_BLEND_ALPHA_OPAQUE);
+	plane->alpha_property = prop;
+
+	if (plane->state)
+		plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_alpha_property);
+
+/**
  * drm_plane_create_rotation_property - create a new rotation property
  * @plane: drm plane
  * @rotation: initial value of the rotation property
diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
index 17606026590b..330c561c4c11 100644
--- a/include/drm/drm_blend.h
+++ b/include/drm/drm_blend.h
@@ -36,6 +36,9 @@ static inline bool drm_rotation_90_or_270(unsigned int rotation)
 	return rotation & (DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_270);
 }
 
+#define DRM_BLEND_ALPHA_OPAQUE		0xffff
+
+int drm_plane_create_alpha_property(struct drm_plane *plane);
 int drm_plane_create_rotation_property(struct drm_plane *plane,
 				       unsigned int rotation,
 				       unsigned int supported_rotations);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index f7bf4a48b1c3..8af573615c5f 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -43,6 +43,7 @@ struct drm_modeset_acquire_ctx;
  *	plane (in 16.16)
  * @src_w: width of visible portion of plane (in 16.16)
  * @src_h: height of visible portion of plane (in 16.16)
+ * @alpha: opacity of the plane
  * @rotation: rotation of the plane
  * @zpos: priority of the given plane on crtc (optional)
  *	Note that multiple active planes on the same crtc can have an identical
@@ -106,6 +107,9 @@ struct drm_plane_state {
 	uint32_t src_x, src_y;
 	uint32_t src_h, src_w;
 
+	/* Plane opacity */
+	u16 alpha;
+
 	/* Plane rotation */
 	unsigned int rotation;
 
@@ -496,6 +500,7 @@ enum drm_plane_type {
  * @funcs: helper functions
  * @properties: property tracking for this plane
  * @type: type of plane (overlay, primary, cursor)
+ * @alpha_property: alpha property for this plane
  * @zpos_property: zpos property for this plane
  * @rotation_property: rotation property for this plane
  * @helper_private: mid-layer private data
@@ -571,6 +576,7 @@ struct drm_plane {
 	 */
 	struct drm_plane_state *state;
 
+	struct drm_property *alpha_property;
 	struct drm_property *zpos_property;
 	struct drm_property *rotation_property;
 
-- 
git-series 0.9.1

[PATCH v4 1/5] drm/blend: Add a generic alpha property

[Eric Anholt]

Maxime Ripard <maxime.ripard@bootlin.com> writes:

> Some drivers duplicate the logic to create a property to store a per-plane
> alpha.
>
> This is especially useful if we ever want to support extra protocols for
> Wayland like:
> https://lists.freedesktop.org/archives/wayland-devel/2017-August/034741.html
>
> Let's create a helper in order to move that to the core.

> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index 5a81e1b4c076..05eda2d57c77 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -88,6 +88,12 @@
>   * On top of this basic transformation additional properties can be exposed by
>   * the driver:
>   *
> + * alpha:
> + * 	Alpha is setup with drm_plane_create_alpha_property(). It controls the

s/setup/set up/

> + * 	plane-wide opacity, from transparent (0) to opaque (0xffff). It can be
> + * 	combined with pixel alpha.
> + * 	The alpha value is represented as premultiplied alpha.

I don't think this premultiplied comment makes any sense.  What are you
saying it's premultiplied with?  Maybe you mean that the output pixels
will have both their color and alpha channels multiplied by this alpha?
I'd just drop it.

Reviewed-by: Eric Anholt <eric@anholt.net>

We should probably do this property for vc4 at some point, too.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180330/249a4627/attachment.sig>

[PATCH v4 1/5] drm/blend: Add a generic alpha property

[Paul Kocialkowski]

Hi,

On Fri, 2018-03-30 at 13:37 -0700, Eric Anholt wrote:
> Maxime Ripard <maxime.ripard@bootlin.com> writes:
> 
> > Some drivers duplicate the logic to create a property to store a
> > per-plane
> > alpha.
> > 
> > This is especially useful if we ever want to support extra protocols
> > for
> > Wayland like:
> > https://lists.freedesktop.org/archives/wayland-devel/2017-August/034
> > 741.html
> > 
> > Let's create a helper in order to move that to the core.
> > diff --git a/drivers/gpu/drm/drm_blend.c
> > b/drivers/gpu/drm/drm_blend.c
> > index 5a81e1b4c076..05eda2d57c77 100644
> > --- a/drivers/gpu/drm/drm_blend.c
> > +++ b/drivers/gpu/drm/drm_blend.c
> > @@ -88,6 +88,12 @@
> >   * On top of this basic transformation additional properties can be
> > exposed by
> >   * the driver:
> >   *
> > + * alpha:
> > + * 	Alpha is setup with drm_plane_create_alpha_property().
> > It controls the
> 
> s/setup/set up/
> 
> > + * 	plane-wide opacity, from transparent (0) to opaque
> > (0xffff). It can be
> > + * 	combined with pixel alpha.
> > + * 	The alpha value is represented as premultiplied alpha.
> 
> I don't think this premultiplied comment makes any sense.  What are
> you saying it's premultiplied with?  Maybe you mean that the output
> pixels will have both their color and alpha channels multiplied by
> this alpha?
> I'd just drop it.

I disagree here: since there are multiple ways to blend the pixel alpha
value and the plane-wide alpha value, I think it's important to clearly
specify which blending equation DRM expects here. Otherwise, the plane-
wide alpha value just doesn't have a specified meaning and driver
implementations can't figure whether the hardware uses the same equation
or not and whether adaptation to this coefficient is needed.

The equations for premultiplied alpha blending are at:
https://en.wikipedia.org/wiki/Alpha_compositing#Alpha_blending

What do you think?

> Reviewed-by: Eric Anholt <eric@anholt.net>
> 
> We should probably do this property for vc4 at some point, too.

-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180404/f1f7aa6e/attachment-0001.sig>

[PATCH v4 1/5] drm/blend: Add a generic alpha property

[Laurent Pinchart]

Hi Paul,

On Wednesday, 4 April 2018 12:15:04 EEST Paul Kocialkowski wrote:
> On Fri, 2018-03-30 at 13:37 -0700, Eric Anholt wrote:
> > Maxime Ripard <maxime.ripard@bootlin.com> writes:
> >> Some drivers duplicate the logic to create a property to store a
> >> per-plane alpha.
> >> 
> >> This is especially useful if we ever want to support extra protocols
> >> for Wayland like:
> >> https://lists.freedesktop.org/archives/wayland-devel/2017-August/034
> >> 741.html
> >> 
> >> Let's create a helper in order to move that to the core.
> >> diff --git a/drivers/gpu/drm/drm_blend.c
> >> b/drivers/gpu/drm/drm_blend.c
> >> index 5a81e1b4c076..05eda2d57c77 100644
> >> --- a/drivers/gpu/drm/drm_blend.c
> >> +++ b/drivers/gpu/drm/drm_blend.c
> >> @@ -88,6 +88,12 @@
> >>   * On top of this basic transformation additional properties can be
> >> exposed by
> >>   * the driver:
> >>   *
> >> + * alpha:
> >> + * 	Alpha is setup with drm_plane_create_alpha_property().
> >> It controls the
> > 
> > s/setup/set up/
> > 
> >> + * 	plane-wide opacity, from transparent (0) to opaque
> >> (0xffff). It can be
> >> + * 	combined with pixel alpha.
> >> + * 	The alpha value is represented as premultiplied alpha.
> > 
> > I don't think this premultiplied comment makes any sense.  What are
> > you saying it's premultiplied with?  Maybe you mean that the output
> > pixels will have both their color and alpha channels multiplied by
> > this alpha? I'd just drop it.
> 
> I disagree here: since there are multiple ways to blend the pixel alpha
> value and the plane-wide alpha value, I think it's important to clearly
> specify which blending equation DRM expects here. Otherwise, the plane-
> wide alpha value just doesn't have a specified meaning and driver
> implementations can't figure whether the hardware uses the same equation
> or not and whether adaptation to this coefficient is needed.
> 
> The equations for premultiplied alpha blending are at:
> https://en.wikipedia.org/wiki/Alpha_compositing#Alpha_blending
> 
> What do you think?

I think it's important to be clear in the documentation, but I also don't 
think "premultiplied alpha" makes sense in this context. It's not the alpha 
value that is premultiplied, it is the RGB data. As correctly explained in the 
link you have provided, to compute the output RGB values the src and dst RGB 
values have to be multiplied by the src and dst alpha respectively. To 
simplify the hardware computation, RGB values can be premultiplied in a the 
framebuffer. This is useful when you deal with textures that have an alpha 
component (you can premultiply them offline first) or with multi-pass alpha 
blending where you can skip the division by outA in the intermediate steps. 
However, for plane global alpha, I fail to see a use case that would feed the 
device with a XRGB framebuffer premultiplied with a fixed global alpha value.

To summarize this, I'm fine specifying how alpha premultiplication is handled 
in relationship with the generic alpha property, but I think Maxime's patch 
was wrong, and the RGB values should not be premultiplied.

> > Reviewed-by: Eric Anholt <eric@anholt.net>
> > 
> > We should probably do this property for vc4 at some point, too.

-- 
Regards,

Laurent Pinchart

[PATCH v4 1/5] drm/blend: Add a generic alpha property

[Eric Anholt]

Paul Kocialkowski <paul.kocialkowski@bootlin.com> writes:

> [ Unknown signature status ]
> Hi,
>
> On Fri, 2018-03-30 at 13:37 -0700, Eric Anholt wrote:
>> Maxime Ripard <maxime.ripard@bootlin.com> writes:
>> 
>> > Some drivers duplicate the logic to create a property to store a
>> > per-plane
>> > alpha.
>> > 
>> > This is especially useful if we ever want to support extra protocols
>> > for
>> > Wayland like:
>> > https://lists.freedesktop.org/archives/wayland-devel/2017-August/034
>> > 741.html
>> > 
>> > Let's create a helper in order to move that to the core.
>> > diff --git a/drivers/gpu/drm/drm_blend.c
>> > b/drivers/gpu/drm/drm_blend.c
>> > index 5a81e1b4c076..05eda2d57c77 100644
>> > --- a/drivers/gpu/drm/drm_blend.c
>> > +++ b/drivers/gpu/drm/drm_blend.c
>> > @@ -88,6 +88,12 @@
>> >   * On top of this basic transformation additional properties can be
>> > exposed by
>> >   * the driver:
>> >   *
>> > + * alpha:
>> > + * 	Alpha is setup with drm_plane_create_alpha_property().
>> > It controls the
>> 
>> s/setup/set up/
>> 
>> > + * 	plane-wide opacity, from transparent (0) to opaque
>> > (0xffff). It can be
>> > + * 	combined with pixel alpha.
>> > + * 	The alpha value is represented as premultiplied alpha.
>> 
>> I don't think this premultiplied comment makes any sense.  What are
>> you saying it's premultiplied with?  Maybe you mean that the output
>> pixels will have both their color and alpha channels multiplied by
>> this alpha?
>> I'd just drop it.
>
> I disagree here: since there are multiple ways to blend the pixel alpha
> value and the plane-wide alpha value, I think it's important to clearly
> specify which blending equation DRM expects here. Otherwise, the plane-
> wide alpha value just doesn't have a specified meaning and driver
> implementations can't figure whether the hardware uses the same equation
> or not and whether adaptation to this coefficient is needed.
>
> The equations for premultiplied alpha blending are at:
> https://en.wikipedia.org/wiki/Alpha_compositing#Alpha_blending
>
> What do you think?

"Premultiplied alpha" only has a meaning for an RGBA value, where RGB
have been already multiplied by their own A.  In this case, you're
saying that an alpha-only value is "premultiplied", but with what?  What
would an alternative to it being "premultiplied" do instead?  That's
what I'm confused about.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180404/506231ea/attachment.sig>

[PATCH v4 1/5] drm/blend: Add a generic alpha property

[Paul Kocialkowski]

Hi,

On Tue, 2018-03-13 at 21:54 +0100, Maxime Ripard wrote:
> Some drivers duplicate the logic to create a property to store a per-
> plane alpha.
> 
> This is especially useful if we ever want to support extra protocols
> for
> Wayland like:
> https://lists.freedesktop.org/archives/wayland-devel/2017-August/03474
> 1.html
> 
> Let's create a helper in order to move that to the core.

See one comment below about the alpha blending equation.
Otherwise, this is:

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/drm_atomic.c        |  4 +++-
>  drivers/gpu/drm/drm_atomic_helper.c |  4 +++-
>  drivers/gpu/drm/drm_blend.c         | 38
> ++++++++++++++++++++++++++++++-
>  include/drm/drm_blend.h             |  3 ++-
>  include/drm/drm_plane.h             |  6 +++++-
>  5 files changed, 55 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c
> b/drivers/gpu/drm/drm_atomic.c
> index 34b7d420e555..39204c88d2c5 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -753,6 +753,8 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
>  		state->src_w = val;
>  	} else if (property == config->prop_src_h) {
>  		state->src_h = val;
> +	} else if (property == plane->alpha_property) {
> +		state->alpha = val;
>  	} else if (property == plane->rotation_property) {
>  		if (!is_power_of_2(val & DRM_MODE_ROTATE_MASK))
>  			return -EINVAL;
> @@ -818,6 +820,8 @@ drm_atomic_plane_get_property(struct drm_plane
> *plane,
>  		*val = state->src_w;
>  	} else if (property == config->prop_src_h) {
>  		*val = state->src_h;
> +	} else if (property == plane->alpha_property) {
> +		*val = state->alpha;
>  	} else if (property == plane->rotation_property) {
>  		*val = state->rotation;
>  	} else if (property == plane->zpos_property) {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 00c78c1c9681..ac4c3f18a0b1 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3484,6 +3484,10 @@ void drm_atomic_helper_plane_reset(struct
> drm_plane *plane)
>  	if (plane->state) {
>  		plane->state->plane = plane;
>  		plane->state->rotation = DRM_MODE_ROTATE_0;
> +
> +		/* Reset the alpha value to fully opaque if it
> matters */
> +		if (plane->alpha_property)
> +			plane->state->alpha = plane->alpha_property-
> >values[1];
>  	}
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_plane_reset);
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index 5a81e1b4c076..05eda2d57c77 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -88,6 +88,12 @@
>   * On top of this basic transformation additional properties can be
> exposed by
>   * the driver:
>   *
> + * alpha:
> + * 	Alpha is setup with drm_plane_create_alpha_property(). It
> controls the
> + * 	plane-wide opacity, from transparent (0) to opaque
> (0xffff). It can be
> + * 	combined with pixel alpha.
> + * 	The alpha value is represented as premultiplied alpha.

As Eric highlighted already, it's hard to grasp what "premultiplied
alpha" is about. From what I can see, the Wayland proposal supports the
following blending equations:
* none: no alpha
* opaque: alpha between 1 (opaque) and 0 (transparent)
* premultiplied: alpha between 1 and 1 - (pixel alpha)
* straight: alpha between (pixel alpha) and 1 - (pixel alpha)

The comment seems to imply that the value should always be specified to
DRM as premultiplied. I think it would be good to explain what this
entails in the comment and in other relevant places (e.g. DRM
documentation).

Also, I'm not sure this is the best fit for all the hardware out there,
so to deal with the possible variety of cases, we could:
* Have a way for the driver to expose what blending equations are
supported (maybe another DRM property?)
* Keep the DRM property implicitly (although with clear documentation
about it) tied to one specific equation (e.g. premultiplied) and have
the drivers to the adaptation on the coefficient if needed to fit what
the hardware needs.

> + *
>   * rotation:
>   *	Rotation is set up with
> drm_plane_create_rotation_property(). It adds a
>   *	rotation and reflection step between the source and
> destination rectangles.
> @@ -106,6 +112,38 @@
>   */
>  
>  /**
> + * drm_plane_create_alpha_property - create a new alpha property
> + * @plane: drm plane
> + *
> + * This function creates a generic, mutable, alpha property and
> enables support
> + * for it in the DRM core. It is attached to @plane.
> + *
> + * The alpha property will be allowed to be within the bounds of 0
> + * (transparent) to 0xffff (opaque).
> + *
> + * Returns:
> + * 0 on success, negative error code on failure.
> + */
> +int drm_plane_create_alpha_property(struct drm_plane *plane)
> +{
> +	struct drm_property *prop;
> +
> +	prop = drm_property_create_range(plane->dev, 0, "alpha",
> +					 0, DRM_BLEND_ALPHA_OPAQUE);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	drm_object_attach_property(&plane->base, prop,
> DRM_BLEND_ALPHA_OPAQUE);
> +	plane->alpha_property = prop;
> +
> +	if (plane->state)
> +		plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_create_alpha_property);
> +
> +/**
>   * drm_plane_create_rotation_property - create a new rotation
> property
>   * @plane: drm plane
>   * @rotation: initial value of the rotation property
> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> index 17606026590b..330c561c4c11 100644
> --- a/include/drm/drm_blend.h
> +++ b/include/drm/drm_blend.h
> @@ -36,6 +36,9 @@ static inline bool drm_rotation_90_or_270(unsigned
> int rotation)
>  	return rotation & (DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_270);
>  }
>  
> +#define DRM_BLEND_ALPHA_OPAQUE		0xffff
> +
> +int drm_plane_create_alpha_property(struct drm_plane *plane);
>  int drm_plane_create_rotation_property(struct drm_plane *plane,
>  				       unsigned int rotation,
>  				       unsigned int
> supported_rotations);
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index f7bf4a48b1c3..8af573615c5f 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -43,6 +43,7 @@ struct drm_modeset_acquire_ctx;
>   *	plane (in 16.16)
>   * @src_w: width of visible portion of plane (in 16.16)
>   * @src_h: height of visible portion of plane (in 16.16)
> + * @alpha: opacity of the plane
>   * @rotation: rotation of the plane
>   * @zpos: priority of the given plane on crtc (optional)
>   *	Note that multiple active planes on the same crtc can have
> an identical
> @@ -106,6 +107,9 @@ struct drm_plane_state {
>  	uint32_t src_x, src_y;
>  	uint32_t src_h, src_w;
>  
> +	/* Plane opacity */
> +	u16 alpha;
> +
>  	/* Plane rotation */
>  	unsigned int rotation;
>  
> @@ -496,6 +500,7 @@ enum drm_plane_type {
>   * @funcs: helper functions
>   * @properties: property tracking for this plane
>   * @type: type of plane (overlay, primary, cursor)
> + * @alpha_property: alpha property for this plane
>   * @zpos_property: zpos property for this plane
>   * @rotation_property: rotation property for this plane
>   * @helper_private: mid-layer private data
> @@ -571,6 +576,7 @@ struct drm_plane {
>  	 */
>  	struct drm_plane_state *state;
>  
> +	struct drm_property *alpha_property;
>  	struct drm_property *zpos_property;
>  	struct drm_property *rotation_property;
>  
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180404/0513bf64/attachment.sig>

[PATCH v4 2/5] drm/atmel-hclcdc: Convert to the new generic alpha property

[Maxime Ripard]

Now that we have support for per-plane alpha in the core, let's use it.

Acked-by: Boris Brezillon <boris.brezillon@bootlin.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h    | 13 +---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 89 ++----------------
 2 files changed, 14 insertions(+), 88 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index ab32d5b268d2..60c937f42114 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -299,7 +299,6 @@ struct atmel_hlcdc_layer {
 struct atmel_hlcdc_plane {
 	struct drm_plane base;
 	struct atmel_hlcdc_layer layer;
-	struct atmel_hlcdc_plane_properties *properties;
 };
 
 static inline struct atmel_hlcdc_plane *
@@ -346,18 +345,6 @@ struct atmel_hlcdc_dc_desc {
 };
 
 /**
- * Atmel HLCDC Plane properties.
- *
- * This structure stores plane property definitions.
- *
- * @alpha: alpha blending (or transparency) property
- * @rotation: rotation property
- */
-struct atmel_hlcdc_plane_properties {
-	struct drm_property *alpha;
-};
-
-/**
  * Atmel HLCDC Display Controller.
  *
  * @desc: HLCDC Display Controller description
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index e18800ed7cd1..73c875db45f4 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -31,7 +31,6 @@
  * @src_y: y buffer position
  * @src_w: buffer width
  * @src_h: buffer height
- * @alpha: alpha blending of the plane
  * @disc_x: x discard position
  * @disc_y: y discard position
  * @disc_w: discard width
@@ -54,8 +53,6 @@ struct atmel_hlcdc_plane_state {
 	uint32_t src_w;
 	uint32_t src_h;
 
-	u8 alpha;
-
 	int disc_x;
 	int disc_y;
 	int disc_w;
@@ -385,7 +382,7 @@ atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
 			cfg |= ATMEL_HLCDC_LAYER_LAEN;
 		else
 			cfg |= ATMEL_HLCDC_LAYER_GAEN |
-			       ATMEL_HLCDC_LAYER_GA(state->alpha);
+			       ATMEL_HLCDC_LAYER_GA(state->base.alpha >> 8);
 	}
 
 	if (state->disc_h && state->disc_w)
@@ -553,7 +550,7 @@ atmel_hlcdc_plane_prepare_disc_area(struct drm_crtc_state *c_state)
 
 		if (!ovl_s->fb ||
 		    ovl_s->fb->format->has_alpha ||
-		    ovl_state->alpha != 255)
+		    ovl_s->alpha != DRM_BLEND_ALPHA_OPAQUE)
 			continue;
 
 		/* TODO: implement a smarter hidden area detection */
@@ -829,51 +826,18 @@ static void atmel_hlcdc_plane_destroy(struct drm_plane *p)
 	drm_plane_cleanup(p);
 }
 
-static int atmel_hlcdc_plane_atomic_set_property(struct drm_plane *p,
-						 struct drm_plane_state *s,
-						 struct drm_property *property,
-						 uint64_t val)
-{
-	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
-	struct atmel_hlcdc_plane_properties *props = plane->properties;
-	struct atmel_hlcdc_plane_state *state =
-			drm_plane_state_to_atmel_hlcdc_plane_state(s);
-
-	if (property == props->alpha)
-		state->alpha = val;
-	else
-		return -EINVAL;
-
-	return 0;
-}
-
-static int atmel_hlcdc_plane_atomic_get_property(struct drm_plane *p,
-					const struct drm_plane_state *s,
-					struct drm_property *property,
-					uint64_t *val)
-{
-	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
-	struct atmel_hlcdc_plane_properties *props = plane->properties;
-	const struct atmel_hlcdc_plane_state *state =
-		container_of(s, const struct atmel_hlcdc_plane_state, base);
-
-	if (property == props->alpha)
-		*val = state->alpha;
-	else
-		return -EINVAL;
-
-	return 0;
-}
-
-static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
-				struct atmel_hlcdc_plane_properties *props)
+static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
 {
 	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
 
 	if (desc->type == ATMEL_HLCDC_OVERLAY_LAYER ||
-	    desc->type == ATMEL_HLCDC_CURSOR_LAYER)
-		drm_object_attach_property(&plane->base.base,
-					   props->alpha, 255);
+	    desc->type == ATMEL_HLCDC_CURSOR_LAYER) {
+		int ret;
+
+		ret = drm_plane_create_alpha_property(&plane->base);
+		if (ret)
+			return ret;
+	}
 
 	if (desc->layout.xstride && desc->layout.pstride) {
 		int ret;
@@ -988,8 +952,8 @@ static void atmel_hlcdc_plane_reset(struct drm_plane *p)
 			return;
 		}
 
-		state->alpha = 255;
 		p->state = &state->base;
+		p->state->alpha = DRM_BLEND_ALPHA_OPAQUE;
 		p->state->plane = p;
 	}
 }
@@ -1042,13 +1006,10 @@ static const struct drm_plane_funcs layer_plane_funcs = {
 	.reset = atmel_hlcdc_plane_reset,
 	.atomic_duplicate_state = atmel_hlcdc_plane_atomic_duplicate_state,
 	.atomic_destroy_state = atmel_hlcdc_plane_atomic_destroy_state,
-	.atomic_set_property = atmel_hlcdc_plane_atomic_set_property,
-	.atomic_get_property = atmel_hlcdc_plane_atomic_get_property,
 };
 
 static int atmel_hlcdc_plane_create(struct drm_device *dev,
-				    const struct atmel_hlcdc_layer_desc *desc,
-				    struct atmel_hlcdc_plane_properties *props)
+				    const struct atmel_hlcdc_layer_desc *desc)
 {
 	struct atmel_hlcdc_dc *dc = dev->dev_private;
 	struct atmel_hlcdc_plane *plane;
@@ -1060,7 +1021,6 @@ static int atmel_hlcdc_plane_create(struct drm_device *dev,
 		return -ENOMEM;
 
 	atmel_hlcdc_layer_init(&plane->layer, desc, dc->hlcdc->regmap);
-	plane->properties = props;
 
 	if (desc->type == ATMEL_HLCDC_BASE_LAYER)
 		type = DRM_PLANE_TYPE_PRIMARY;
@@ -1081,7 +1041,7 @@ static int atmel_hlcdc_plane_create(struct drm_device *dev,
 			     &atmel_hlcdc_layer_plane_helper_funcs);
 
 	/* Set default property values*/
-	ret = atmel_hlcdc_plane_init_properties(plane, props);
+	ret = atmel_hlcdc_plane_init_properties(plane);
 	if (ret)
 		return ret;
 
@@ -1090,34 +1050,13 @@ static int atmel_hlcdc_plane_create(struct drm_device *dev,
 	return 0;
 }
 
-static struct atmel_hlcdc_plane_properties *
-atmel_hlcdc_plane_create_properties(struct drm_device *dev)
-{
-	struct atmel_hlcdc_plane_properties *props;
-
-	props = devm_kzalloc(dev->dev, sizeof(*props), GFP_KERNEL);
-	if (!props)
-		return ERR_PTR(-ENOMEM);
-
-	props->alpha = drm_property_create_range(dev, 0, "alpha", 0, 255);
-	if (!props->alpha)
-		return ERR_PTR(-ENOMEM);
-
-	return props;
-}
-
 int atmel_hlcdc_create_planes(struct drm_device *dev)
 {
 	struct atmel_hlcdc_dc *dc = dev->dev_private;
-	struct atmel_hlcdc_plane_properties *props;
 	const struct atmel_hlcdc_layer_desc *descs = dc->desc->layers;
 	int nlayers = dc->desc->nlayers;
 	int i, ret;
 
-	props = atmel_hlcdc_plane_create_properties(dev);
-	if (IS_ERR(props))
-		return PTR_ERR(props);
-
 	dc->dscrpool = dmam_pool_create("atmel-hlcdc-dscr", dev->dev,
 				sizeof(struct atmel_hlcdc_dma_channel_dscr),
 				sizeof(u64), 0);
@@ -1130,7 +1069,7 @@ int atmel_hlcdc_create_planes(struct drm_device *dev)
 		    descs[i].type != ATMEL_HLCDC_CURSOR_LAYER)
 			continue;
 
-		ret = atmel_hlcdc_plane_create(dev, &descs[i], props);
+		ret = atmel_hlcdc_plane_create(dev, &descs[i]);
 		if (ret)
 			return ret;
 	}
-- 
git-series 0.9.1

[PATCH v4 3/5] drm/rcar-du: Convert to the new generic alpha property

[Maxime Ripard]

Now that we have support for per-plane alpha in the core, let's use it.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  1 +-
 drivers/gpu/drm/rcar-du/rcar_du_kms.c   |  5 +---
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 15 +++------
 drivers/gpu/drm/rcar-du/rcar_du_plane.h |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c   | 42 ++------------------------
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h   |  2 +-
 6 files changed, 9 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index f400fde65a0c..dad87d449ef7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -90,7 +90,6 @@ struct rcar_du_device {
 	struct rcar_du_vsp vsps[RCAR_DU_MAX_VSPS];
 
 	struct {
-		struct drm_property *alpha;
 		struct drm_property *colorkey;
 	} props;
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 566d1a948c8f..e1b5a7b460cc 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -417,11 +417,6 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu)
 
 static int rcar_du_properties_init(struct rcar_du_device *rcdu)
 {
-	rcdu->props.alpha =
-		drm_property_create_range(rcdu->ddev, 0, "alpha", 0, 255);
-	if (rcdu->props.alpha == NULL)
-		return -ENOMEM;
-
 	/*
 	 * The color key is expressed as an RGB888 triplet stored in a 32-bit
 	 * integer in XRGB8888 format. Bit 24 is used as a flag to disable (0)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index 68556bd9dad2..c20f7ed48c8d 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -423,7 +423,7 @@ static void rcar_du_plane_setup_mode(struct rcar_du_group *rgrp,
 		rcar_du_plane_write(rgrp, index, PnALPHAR, PnALPHAR_ABIT_0);
 	else
 		rcar_du_plane_write(rgrp, index, PnALPHAR,
-				    PnALPHAR_ABIT_X | state->alpha);
+				    PnALPHAR_ABIT_X | state->state.alpha >> 8);
 
 	pnmr = PnMR_BM_MD | state->format->pnmr;
 
@@ -692,11 +692,11 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
 
 	state->hwindex = -1;
 	state->source = RCAR_DU_PLANE_MEMORY;
-	state->alpha = 255;
 	state->colorkey = RCAR_DU_COLORKEY_NONE;
 	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
 
 	plane->state = &state->state;
+	plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE;
 	plane->state->plane = plane;
 }
 
@@ -708,9 +708,7 @@ static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
 	struct rcar_du_plane_state *rstate = to_rcar_plane_state(state);
 	struct rcar_du_device *rcdu = to_rcar_plane(plane)->group->dev;
 
-	if (property == rcdu->props.alpha)
-		rstate->alpha = val;
-	else if (property == rcdu->props.colorkey)
+	if (property == rcdu->props.colorkey)
 		rstate->colorkey = val;
 	else
 		return -EINVAL;
@@ -726,9 +724,7 @@ static int rcar_du_plane_atomic_get_property(struct drm_plane *plane,
 		container_of(state, const struct rcar_du_plane_state, state);
 	struct rcar_du_device *rcdu = to_rcar_plane(plane)->group->dev;
 
-	if (property == rcdu->props.alpha)
-		*val = rstate->alpha;
-	else if (property == rcdu->props.colorkey)
+	if (property == rcdu->props.colorkey)
 		*val = rstate->colorkey;
 	else
 		return -EINVAL;
@@ -797,10 +793,9 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
 			continue;
 
 		drm_object_attach_property(&plane->plane.base,
-					   rcdu->props.alpha, 255);
-		drm_object_attach_property(&plane->plane.base,
 					   rcdu->props.colorkey,
 					   RCAR_DU_COLORKEY_NONE);
+		drm_plane_create_alpha_property(&plane->plane);
 		drm_plane_create_zpos_property(&plane->plane, 1, 1, 7);
 	}
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.h b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
index 890321b4665d..5c19c69e4691 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
@@ -50,7 +50,6 @@ static inline struct rcar_du_plane *to_rcar_plane(struct drm_plane *plane)
  * @state: base DRM plane state
  * @format: information about the pixel format used by the plane
  * @hwindex: 0-based hardware plane index, -1 means unused
- * @alpha: value of the plane alpha property
  * @colorkey: value of the plane colorkey property
  */
 struct rcar_du_plane_state {
@@ -60,7 +59,6 @@ struct rcar_du_plane_state {
 	int hwindex;
 	enum rcar_du_plane_source source;
 
-	unsigned int alpha;
 	unsigned int colorkey;
 };
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 2c260c33840b..b3bec0125696 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -54,6 +54,7 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
 	};
 	struct rcar_du_plane_state state = {
 		.state = {
+			.alpha = DRM_BLEND_ALPHA_OPAQUE,
 			.crtc = &crtc->crtc,
 			.dst.x1 = 0,
 			.dst.y1 = 0,
@@ -67,7 +68,6 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
 		},
 		.format = rcar_du_format_info(DRM_FORMAT_ARGB8888),
 		.source = RCAR_DU_PLANE_VSPD1,
-		.alpha = 255,
 		.colorkey = 0,
 	};
 
@@ -173,7 +173,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
 	struct vsp1_du_atomic_config cfg = {
 		.pixelformat = 0,
 		.pitch = fb->pitches[0],
-		.alpha = state->alpha,
+		.alpha = state->state.alpha >> 8,
 		.zpos = state->state.zpos,
 	};
 	unsigned int i;
@@ -335,44 +335,13 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane)
 	if (state == NULL)
 		return;
 
-	state->alpha = 255;
+	state->state.alpha = DRM_BLEND_ALPHA_OPAQUE;
 	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
 
 	plane->state = &state->state;
 	plane->state->plane = plane;
 }
 
-static int rcar_du_vsp_plane_atomic_set_property(struct drm_plane *plane,
-	struct drm_plane_state *state, struct drm_property *property,
-	uint64_t val)
-{
-	struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
-	struct rcar_du_device *rcdu = to_rcar_vsp_plane(plane)->vsp->dev;
-
-	if (property == rcdu->props.alpha)
-		rstate->alpha = val;
-	else
-		return -EINVAL;
-
-	return 0;
-}
-
-static int rcar_du_vsp_plane_atomic_get_property(struct drm_plane *plane,
-	const struct drm_plane_state *state, struct drm_property *property,
-	uint64_t *val)
-{
-	const struct rcar_du_vsp_plane_state *rstate =
-		container_of(state, const struct rcar_du_vsp_plane_state, state);
-	struct rcar_du_device *rcdu = to_rcar_vsp_plane(plane)->vsp->dev;
-
-	if (property == rcdu->props.alpha)
-		*val = rstate->alpha;
-	else
-		return -EINVAL;
-
-	return 0;
-}
-
 static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
@@ -380,8 +349,6 @@ static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
 	.destroy = drm_plane_cleanup,
 	.atomic_duplicate_state = rcar_du_vsp_plane_atomic_duplicate_state,
 	.atomic_destroy_state = rcar_du_vsp_plane_atomic_destroy_state,
-	.atomic_set_property = rcar_du_vsp_plane_atomic_set_property,
-	.atomic_get_property = rcar_du_vsp_plane_atomic_get_property,
 };
 
 int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np,
@@ -438,8 +405,7 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np,
 		if (type == DRM_PLANE_TYPE_PRIMARY)
 			continue;
 
-		drm_object_attach_property(&plane->plane.base,
-					   rcdu->props.alpha, 255);
+		drm_plane_create_alpha_property(&plane->plane);
 		drm_plane_create_zpos_property(&plane->plane, 1, 1,
 					       vsp->num_planes - 1);
 	}
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
index f876c512163c..8b19914761e4 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
@@ -44,7 +44,6 @@ static inline struct rcar_du_vsp_plane *to_rcar_vsp_plane(struct drm_plane *p)
  * @state: base DRM plane state
  * @format: information about the pixel format used by the plane
  * @sg_tables: scatter-gather tables for the frame buffer memory
- * @alpha: value of the plane alpha property
  * @zpos: value of the plane zpos property
  */
 struct rcar_du_vsp_plane_state {
@@ -53,7 +52,6 @@ struct rcar_du_vsp_plane_state {
 	const struct rcar_du_format_info *format;
 	struct sg_table sg_tables[3];
 
-	unsigned int alpha;
 	unsigned int zpos;
 };
 
-- 
git-series 0.9.1

[PATCH v4 4/5] drm/sun4i: Add support for plane alpha

[Maxime Ripard]

Our backend supports a per-plane alpha property. Support it through our new
helper.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 16 +++++++++++++---
 drivers/gpu/drm/sun4i/sun4i_backend.h |  3 +++
 drivers/gpu/drm/sun4i/sun4i_layer.c   |  2 ++
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 092ade4ff6a5..98cd4a8a93ed 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -186,6 +186,15 @@ int sun4i_backend_update_layer_formats(struct sun4i_backend *backend,
 	DRM_DEBUG_DRIVER("Switching display backend interlaced mode %s\n",
 			 interlaced ? "on" : "off");
 
+	val = SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA(state->alpha >> 8);
+	if (state->alpha != DRM_BLEND_ALPHA_OPAQUE)
+		val |= SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_EN;
+	regmap_update_bits(backend->engine.regs,
+			   SUN4I_BACKEND_ATTCTL_REG0(layer),
+			   SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_MASK |
+			   SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_EN,
+			   val);
+
 	ret = sun4i_backend_drm_format_to_layer(fb->format->format, &val);
 	if (ret) {
 		DRM_DEBUG_DRIVER("Invalid format\n");
@@ -359,7 +368,7 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
 		DRM_DEBUG_DRIVER("Plane FB format is %s\n",
 				 drm_get_format_name(fb->format->format,
 						     &format_name));
-		if (fb->format->has_alpha)
+		if (fb->format->has_alpha || (plane_state->alpha != DRM_BLEND_ALPHA_OPAQUE))
 			num_alpha_planes++;
 
 		DRM_DEBUG_DRIVER("Plane zpos is %d\n",
@@ -412,7 +421,8 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
 	}
 
 	/* We can't have an alpha plane at the lowest position */
-	if (plane_states[0]->fb->format->has_alpha)
+	if (plane_states[0]->fb->format->has_alpha ||
+	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE))
 		return -EINVAL;
 
 	for (i = 1; i < num_planes; i++) {
@@ -424,7 +434,7 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
 		 * The only alpha position is the lowest plane of the
 		 * second pipe.
 		 */
-		if (fb->format->has_alpha)
+		if (fb->format->has_alpha || (p_state->alpha != DRM_BLEND_ALPHA_OPAQUE))
 			current_pipe++;
 
 		s_state->pipe = current_pipe;
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
index 52e77591186a..03294d5dd1a2 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
@@ -68,11 +68,14 @@
 #define SUN4I_BACKEND_CKMIN_REG			0x884
 #define SUN4I_BACKEND_CKCFG_REG			0x888
 #define SUN4I_BACKEND_ATTCTL_REG0(l)		(0x890 + (0x4 * (l)))
+#define SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_MASK	GENMASK(31, 24)
+#define SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA(x)		((x) << 24)
 #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL_MASK	BIT(15)
 #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(x)		((x) << 15)
 #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL_MASK	GENMASK(11, 10)
 #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL(x)			((x) << 10)
 #define SUN4I_BACKEND_ATTCTL_REG0_LAY_VDOEN		BIT(1)
+#define SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_EN	BIT(0)
 
 #define SUN4I_BACKEND_ATTCTL_REG1(l)		(0x8a0 + (0x4 * (l)))
 #define SUN4I_BACKEND_ATTCTL_REG1_LAY_HSCAFCT		GENMASK(15, 14)
diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 33ad377569ec..cf7857b8ac5c 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -37,6 +37,7 @@ static void sun4i_backend_layer_reset(struct drm_plane *plane)
 	if (state) {
 		plane->state = &state->state;
 		plane->state->plane = plane;
+		plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE;
 		plane->state->zpos = layer->id;
 	}
 }
@@ -163,6 +164,7 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 			     &sun4i_backend_layer_helper_funcs);
 	layer->backend = backend;
 
+	drm_plane_create_alpha_property(&layer->plane);
 	drm_plane_create_zpos_property(&layer->plane, 0, 0,
 				       SUN4I_BACKEND_NUM_LAYERS - 1);
 
-- 
git-series 0.9.1

[PATCH v4 4/5] drm/sun4i: Add support for plane alpha

[Paul Kocialkowski]

Hi,

On Tue, 2018-03-13 at 21:54 +0100, Maxime Ripard wrote:
> Our backend supports a per-plane alpha property. Support it through
> our new
> helper.

See one comment below. Otherwise, this is:
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/sun4i/sun4i_backend.c | 16 +++++++++++++---
>  drivers/gpu/drm/sun4i/sun4i_backend.h |  3 +++
>  drivers/gpu/drm/sun4i/sun4i_layer.c   |  2 ++
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c
> b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index 092ade4ff6a5..98cd4a8a93ed 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -186,6 +186,15 @@ int sun4i_backend_update_layer_formats(struct
> sun4i_backend *backend,
>  	DRM_DEBUG_DRIVER("Switching display backend interlaced mode
> %s\n",
>  			 interlaced ? "on" : "off");
>  
> +	val = SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA(state->alpha >>
> 8);

Have you checked that the alpha value expected by the hardware does
match the "premultiplied" alpha blemding equation?

> +	if (state->alpha != DRM_BLEND_ALPHA_OPAQUE)
> +		val |= SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_EN;

Maybe insert a newline for improved readability here?

> +	regmap_update_bits(backend->engine.regs,
> +			   SUN4I_BACKEND_ATTCTL_REG0(layer),
> +			   SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_MAS
> K |
> +			   SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_EN,
> +			   val);
> +
>  	ret = sun4i_backend_drm_format_to_layer(fb->format->format,
> &val);
>  	if (ret) {
>  		DRM_DEBUG_DRIVER("Invalid format\n");
> @@ -359,7 +368,7 @@ static int sun4i_backend_atomic_check(struct
> sunxi_engine *engine,
>  		DRM_DEBUG_DRIVER("Plane FB format is %s\n",
>  				 drm_get_format_name(fb->format-
> >format,
>  						     &format_name));
> -		if (fb->format->has_alpha)
> +		if (fb->format->has_alpha || (plane_state->alpha !=
> DRM_BLEND_ALPHA_OPAQUE))
>  			num_alpha_planes++;
>  
>  		DRM_DEBUG_DRIVER("Plane zpos is %d\n",
> @@ -412,7 +421,8 @@ static int sun4i_backend_atomic_check(struct
> sunxi_engine *engine,
>  	}
>  
>  	/* We can't have an alpha plane at the lowest position */
> -	if (plane_states[0]->fb->format->has_alpha)
> +	if (plane_states[0]->fb->format->has_alpha ||
> +	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE))
>  		return -EINVAL;
>  
>  	for (i = 1; i < num_planes; i++) {
> @@ -424,7 +434,7 @@ static int sun4i_backend_atomic_check(struct
> sunxi_engine *engine,
>  		 * The only alpha position is the lowest plane of the
>  		 * second pipe.
>  		 */
> -		if (fb->format->has_alpha)
> +		if (fb->format->has_alpha || (p_state->alpha !=
> DRM_BLEND_ALPHA_OPAQUE))
>  			current_pipe++;
>  
>  		s_state->pipe = current_pipe;
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h
> b/drivers/gpu/drm/sun4i/sun4i_backend.h
> index 52e77591186a..03294d5dd1a2 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> @@ -68,11 +68,14 @@
>  #define SUN4I_BACKEND_CKMIN_REG			0x884
>  #define SUN4I_BACKEND_CKCFG_REG			0x888
>  #define SUN4I_BACKEND_ATTCTL_REG0(l)		(0x890 + (0x4 *
> (l)))
> +#define SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_MASK	GENMASK(31
> , 24)
> +#define SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA(x)		((x)
> << 24)
>  #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL_MASK	BIT(15)
>  #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(x)		((x)
> << 15)
>  #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL_MASK	GENMASK(11,
> 10)
>  #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL(x)			
> ((x) << 10)
>  #define SUN4I_BACKEND_ATTCTL_REG0_LAY_VDOEN		BIT(1)
> +#define SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_EN	BIT(0)
>  
>  #define SUN4I_BACKEND_ATTCTL_REG1(l)		(0x8a0 + (0x4 *
> (l)))
>  #define SUN4I_BACKEND_ATTCTL_REG1_LAY_HSCAFCT		GENMASK(
> 15, 14)
> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c
> b/drivers/gpu/drm/sun4i/sun4i_layer.c
> index 33ad377569ec..cf7857b8ac5c 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
> @@ -37,6 +37,7 @@ static void sun4i_backend_layer_reset(struct
> drm_plane *plane)
>  	if (state) {
>  		plane->state = &state->state;
>  		plane->state->plane = plane;
> +		plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE;
>  		plane->state->zpos = layer->id;
>  	}
>  }
> @@ -163,6 +164,7 @@ static struct sun4i_layer
> *sun4i_layer_init_one(struct drm_device *drm,
>  			     &sun4i_backend_layer_helper_funcs);
>  	layer->backend = backend;
>  
> +	drm_plane_create_alpha_property(&layer->plane);
>  	drm_plane_create_zpos_property(&layer->plane, 0, 0,
>  				       SUN4I_BACKEND_NUM_LAYERS - 1);
>  
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180404/3540741d/attachment.sig>

[PATCH v4 5/5] drm/docs: Remove the rcar alpha from the csv file

[Maxime Ripard]

Now that we moved the rcar-du DRM driver has been switched to the generic
alpha property, remove the former property documentation from the
deperecated CSV file.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 Documentation/gpu/kms-properties.csv | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/gpu/kms-properties.csv b/Documentation/gpu/kms-properties.csv
index 6b28b014cb7d..07ed22ea3bd6 100644
--- a/Documentation/gpu/kms-properties.csv
+++ b/Documentation/gpu/kms-properties.csv
@@ -98,5 +98,4 @@ radeon,DVI-I,?coherent?,RANGE,"Min=0, Max=1",Connector,TBD
 ,,"""underscan vborder""",RANGE,"Min=0, Max=128",Connector,TBD
 ,Audio,?audio?,ENUM,"{ ""off"", ""on"", ""auto"" }",Connector,TBD
 ,FMT Dithering,?dither?,ENUM,"{ ""off"", ""on"" }",Connector,TBD
-rcar-du,Generic,"""alpha""",RANGE,"Min=0, Max=255",Plane,TBD
 ,,"""colorkey""",RANGE,"Min=0, Max=0x01ffffff",Plane,TBD
-- 
git-series 0.9.1

[PATCH v4 0/5] drm/blend: Support generic plane-wide alpha

[Maxime Ripard]

On Tue, Mar 13, 2018 at 09:54:20PM +0100, Maxime Ripard wrote:
> This serie aims at enhancing the support for plane-wide alpha in the
> drivers that are implementing it at the moment, by turning it into a
> generic property and converting the drivers (rcar-du and atmel-hclcdc). It
> also introduces support for it in the sun4i driver.

Ping?

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180326/9fa67e7c/attachment.sig>