Re: [PATCH v5 1/2] drm: Add per-plane pixel blend mode property

2018-08-23 Thread Sean Paul
On Thu, Aug 23, 2018 at 04:30:19PM +0800, Lowry Li wrote:
> Pixel blend modes represent the alpha blending equation
> selection, describing how the pixels from the current
> plane are composited with the background.
> 
> Adds a pixel_blend_mode to drm_plane_state and a
> blend_mode_property to drm_plane, and related support
> functions.
> 
> Defines three blend modes in drm_blend.h.
> 
> Changes since v1:
>  - Moves the blending equation into the DOC comment
>  - Refines the comments of drm_plane_create_blend_mode_property to not
>enumerate the #defines, but instead the string values
>  - Uses fg.* instead of pixel.* and plane_alpha instead of plane.alpha
> Changes since v2:
>  - Refines the comments of drm_plane_create_blend_mode_property:
>   1) Puts the descriptions (after the ":") on a new line
>   2) Adds explaining why @supported_modes need PREMUL as default
> Changes since v3:
>  - Refines drm_plane_create_blend_mode_property(). drm_property_add_enum()
>can calculate the index itself just fine, so no point in having the
>caller pass it in.
>  - Since the current DRM assumption is that alpha is premultiplied
>as default, define DRM_MODE_BLEND_PREMULTI as 0 will be better.
>  - Refines some comments.
> Changes since v4:
>  - Adds comments in drm_blend.h.
>  - Removes setting default value in drm_plane_create_blend_mode_property()
>as it is already in __drm_atomic_helper_plane_reset().
>  - Fixes to use state->pixel_blend_mode instead of using
>plane->state->pixel_blend_mode in reset function.
>  - Rebases on drm-misc-next.
> 
> Reviewed-by: Liviu Dudau 

Reviewed-by: Sean Paul 

I've also Reviewed the userspace component in drm_hwc here:
https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/merge_requests/16#note_24690

Thanks,

Sean

> Signed-off-by: Lowry Li 
> ---
>  drivers/gpu/drm/drm_atomic.c|   4 ++
>  drivers/gpu/drm/drm_atomic_helper.c |   1 +
>  drivers/gpu/drm/drm_blend.c | 123 
> 
>  include/drm/drm_blend.h |   6 ++
>  include/drm/drm_plane.h |   9 +++
>  5 files changed, 143 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3eb061e..d0478ab 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -895,6 +895,8 @@ static int drm_atomic_plane_set_property(struct drm_plane 
> *plane,
>   state->src_h = val;
>   } else if (property == plane->alpha_property) {
>   state->alpha = val;
> + } else if (property == plane->blend_mode_property) {
> + state->pixel_blend_mode = val;
>   } else if (property == plane->rotation_property) {
>   if (!is_power_of_2(val & DRM_MODE_ROTATE_MASK)) {
>   DRM_DEBUG_ATOMIC("[PLANE:%d:%s] bad rotation bitmask: 
> 0x%llx\n",
> @@ -968,6 +970,8 @@ static int drm_atomic_plane_set_property(struct drm_plane 
> *plane,
>   *val = state->src_h;
>   } else if (property == plane->alpha_property) {
>   *val = state->alpha;
> + } else if (property == plane->blend_mode_property) {
> + *val = state->pixel_blend_mode;
>   } 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 6dd5036..284a5d2 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3569,6 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane 
> *plane,
>   /* Reset the alpha value to fully opaque if it matters */
>   if (plane->alpha_property)
>   state->alpha = plane->alpha_property->values[1];
> + state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
>  
>   plane->state = state;
>  }
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index a16a74d..402b62d 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -107,6 +107,52 @@
>   *   planes. Without this property the primary plane is always below the 
> cursor
>   *   plane, and ordering between all other planes is undefined.
>   *
> + * pixel blend mode:
> + *   Pixel blend mode is set up with drm_plane_create_blend_mode_property().
> + *   It adds a blend mode for alpha blending equation selection, describing
> + *   how the pixels from the current plane are composited with the
> + *   background.
> + *
> + *Three alpha blending equations are defined:
> + *
> + *"None":
> + *Blend formula that ignores the pixel alpha::
> + *
> + *out.rgb = plane_alpha * fg.rgb +
> + *(1 - plane_alpha) * bg.rgb
> + *
> + *"Pre-multiplied":
> + *Blend formula that assumes the pixel color values
> + *have been already pre-multiplied with the alpha

[PATCH v5 1/2] drm: Add per-plane pixel blend mode property

2018-08-23 Thread Lowry Li
Pixel blend modes represent the alpha blending equation
selection, describing how the pixels from the current
plane are composited with the background.

Adds a pixel_blend_mode to drm_plane_state and a
blend_mode_property to drm_plane, and related support
functions.

Defines three blend modes in drm_blend.h.

Changes since v1:
 - Moves the blending equation into the DOC comment
 - Refines the comments of drm_plane_create_blend_mode_property to not
   enumerate the #defines, but instead the string values
 - Uses fg.* instead of pixel.* and plane_alpha instead of plane.alpha
Changes since v2:
 - Refines the comments of drm_plane_create_blend_mode_property:
  1) Puts the descriptions (after the ":") on a new line
  2) Adds explaining why @supported_modes need PREMUL as default
Changes since v3:
 - Refines drm_plane_create_blend_mode_property(). drm_property_add_enum()
   can calculate the index itself just fine, so no point in having the
   caller pass it in.
 - Since the current DRM assumption is that alpha is premultiplied
   as default, define DRM_MODE_BLEND_PREMULTI as 0 will be better.
 - Refines some comments.
Changes since v4:
 - Adds comments in drm_blend.h.
 - Removes setting default value in drm_plane_create_blend_mode_property()
   as it is already in __drm_atomic_helper_plane_reset().
 - Fixes to use state->pixel_blend_mode instead of using
   plane->state->pixel_blend_mode in reset function.
 - Rebases on drm-misc-next.

Reviewed-by: Liviu Dudau 
Signed-off-by: Lowry Li 
---
 drivers/gpu/drm/drm_atomic.c|   4 ++
 drivers/gpu/drm/drm_atomic_helper.c |   1 +
 drivers/gpu/drm/drm_blend.c | 123 
 include/drm/drm_blend.h |   6 ++
 include/drm/drm_plane.h |   9 +++
 5 files changed, 143 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3eb061e..d0478ab 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -895,6 +895,8 @@ static int drm_atomic_plane_set_property(struct drm_plane 
*plane,
state->src_h = val;
} else if (property == plane->alpha_property) {
state->alpha = val;
+   } else if (property == plane->blend_mode_property) {
+   state->pixel_blend_mode = val;
} else if (property == plane->rotation_property) {
if (!is_power_of_2(val & DRM_MODE_ROTATE_MASK)) {
DRM_DEBUG_ATOMIC("[PLANE:%d:%s] bad rotation bitmask: 
0x%llx\n",
@@ -968,6 +970,8 @@ static int drm_atomic_plane_set_property(struct drm_plane 
*plane,
*val = state->src_h;
} else if (property == plane->alpha_property) {
*val = state->alpha;
+   } else if (property == plane->blend_mode_property) {
+   *val = state->pixel_blend_mode;
} 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 6dd5036..284a5d2 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3569,6 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane 
*plane,
/* Reset the alpha value to fully opaque if it matters */
if (plane->alpha_property)
state->alpha = plane->alpha_property->values[1];
+   state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
 
plane->state = state;
 }
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index a16a74d..402b62d 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -107,6 +107,52 @@
  * planes. Without this property the primary plane is always below the 
cursor
  * plane, and ordering between all other planes is undefined.
  *
+ * pixel blend mode:
+ * Pixel blend mode is set up with drm_plane_create_blend_mode_property().
+ * It adds a blend mode for alpha blending equation selection, describing
+ * how the pixels from the current plane are composited with the
+ * background.
+ *
+ *  Three alpha blending equations are defined:
+ *
+ *  "None":
+ *  Blend formula that ignores the pixel alpha::
+ *
+ *  out.rgb = plane_alpha * fg.rgb +
+ *  (1 - plane_alpha) * bg.rgb
+ *
+ *  "Pre-multiplied":
+ *  Blend formula that assumes the pixel color values
+ *  have been already pre-multiplied with the alpha
+ *  channel values::
+ *
+ *  out.rgb = plane_alpha * fg.rgb +
+ *  (1 - (plane_alpha * fg.alpha)) * bg.rgb
+ *
+ *  "Coverage":
+ *  Blend formula that assumes the pixel color values have not
+ *  been pre-multiplied and will do so when blending them to the
+ *  background color values::
+ *
+ *