Hi,
This serie aims at adding the support for pixel blend modes represent the alpha blending equation selection in the driver. It also introduces to use it in the malidp driver.
Let me know what you think, Lowry
Changes for v3: - 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 - Refines the comments of drm/mali-dp patchset - Rebased on drm-misc-next and fixed the confilcts with plane alpha patch
Changes for v2: - 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 - Introduces to use it in the malidp driver, which depends on the plane alpha patch
Changes from v1: - v1 is just the core changes to request for commments - 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 - Rebased on current drm-next
Lowry Li (2): drm: Add per-plane pixel blend mode property drm/mali-dp: Implement plane alpha and pixel blend on malidp
drivers/gpu/drm/arm/malidp_planes.c | 76 +++++++++++++--------- drivers/gpu/drm/drm_atomic.c | 4 ++ drivers/gpu/drm/drm_atomic_helper.c | 1 + drivers/gpu/drm/drm_blend.c | 126 ++++++++++++++++++++++++++++++++++++ include/drm/drm_blend.h | 6 ++ include/drm/drm_plane.h | 5 ++ 6 files changed, 186 insertions(+), 32 deletions(-)
Pixel blend modes represent the alpha blending equation selection, describing how the pixels from the current plane are composited with the background.
Add 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.
Signed-off-by: Lowry Li lowry.li@arm.com --- drivers/gpu/drm/drm_atomic.c | 4 ++ drivers/gpu/drm/drm_atomic_helper.c | 1 + drivers/gpu/drm/drm_blend.c | 126 ++++++++++++++++++++++++++++++++++++ include/drm/drm_blend.h | 6 ++ include/drm/drm_plane.h | 5 ++ 5 files changed, 142 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 07fef42..1d18389 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -785,6 +785,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)) return -EINVAL; @@ -852,6 +854,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 130da51..7f5d463 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3515,6 +3515,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) plane->state->alpha = plane->alpha_property->values[1]; + plane->state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI; } } EXPORT_SYMBOL(drm_atomic_helper_plane_reset); diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index a16a74d..ac6f19c 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:: + * + * out.rgb = plane_alpha * fg.alpha * fg.rgb + + * (1 - (plane_alpha * fg.alpha)) * bg.rgb + * + * Using the following symbols: + * + * ``fg.rgb``: + * Each of the RGB component values from the plane's pixel + * ``fg.alpha``: + * Alpha component value from the plane's pixel. If the plane's + * pixel format has no alpha component, then this is assumed to be + * 1.0. In these cases, this property has no effect, as all three + * equations become equivalent. + * ``bg.rgb``: + * Each of the RGB component values from the background + * ``plane_alpha``: + * Plane alpha value set by the plane "alpha" property. If the + * plane does not expose the "alpha" property, then this is + * assumed to be 1.0 + * * Note that all the property extensions described here apply either to the * plane or the CRTC (e.g. for the background color, which currently is not * exposed and assumed to be black). @@ -448,3 +494,83 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, return 0; } EXPORT_SYMBOL(drm_atomic_normalize_zpos); + +/** + * drm_plane_create_blend_mode_property - create a new blend mode property + * @plane: drm plane + * @supported_modes: bitmask of supported modes, must include + * BIT(DRM_MODE_BLEND_PREMULTI). Current DRM assumption is + * that alpha is premultiplied, and old userspace can break if + * the property defaults to coverage. + * + * This creates a new property describing the blend mode. + * + * The property exposed to userspace is an enumeration property (see + * drm_property_create_enum()) called "pixel blend mode" and has the + * following enumeration values: + * + * "None": + * Blend formula that ignores the pixel alpha. + * + * "Pre-multiplied": + * Blend formula that assumes the pixel color values have been already + * pre-multiplied with the alpha channel values. + * + * "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. + * + * RETURNS: + * Zero for success or -errno + */ +int drm_plane_create_blend_mode_property(struct drm_plane *plane, + unsigned int supported_modes) +{ + struct drm_device *dev = plane->dev; + struct drm_property *prop; + static const struct drm_prop_enum_list props[] = { + { DRM_MODE_BLEND_PIXEL_NONE, "None" }, + { DRM_MODE_BLEND_PREMULTI, "Pre-multiplied" }, + { DRM_MODE_BLEND_COVERAGE, "Coverage" }, + }; + unsigned int valid_mode_mask = BIT(DRM_MODE_BLEND_PIXEL_NONE) | + BIT(DRM_MODE_BLEND_PREMULTI) | + BIT(DRM_MODE_BLEND_COVERAGE); + int i, j = 0; + + if (WARN_ON((supported_modes & ~valid_mode_mask) || + ((supported_modes & BIT(DRM_MODE_BLEND_PREMULTI)) == 0))) + return -EINVAL; + + prop = drm_property_create(dev, DRM_MODE_PROP_ENUM, + "pixel blend mode", + hweight32(supported_modes)); + if (!prop) + return -ENOMEM; + + for (i = 0; i < ARRAY_SIZE(props); i++) { + int ret; + + if (!(BIT(props[i].type) & supported_modes)) + continue; + + ret = drm_property_add_enum(prop, j++, props[i].type, + props[i].name); + + if (ret) { + drm_property_destroy(dev, prop); + + return ret; + } + } + + drm_object_attach_property(&plane->base, prop, DRM_MODE_BLEND_PREMULTI); + plane->blend_mode_property = prop; + + if (plane->state) + plane->state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI; + + return 0; +} +EXPORT_SYMBOL(drm_plane_create_blend_mode_property); diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h index 330c561..28c5076 100644 --- a/include/drm/drm_blend.h +++ b/include/drm/drm_blend.h @@ -27,6 +27,10 @@ #include <linux/ctype.h> #include <drm/drm_mode.h>
+#define DRM_MODE_BLEND_PIXEL_NONE 0 +#define DRM_MODE_BLEND_PREMULTI 1 +#define DRM_MODE_BLEND_COVERAGE 2 + struct drm_device; struct drm_atomic_state; struct drm_plane; @@ -52,4 +56,6 @@ int drm_plane_create_zpos_immutable_property(struct drm_plane *plane, unsigned int zpos); int drm_atomic_normalize_zpos(struct drm_device *dev, struct drm_atomic_state *state); +int drm_plane_create_blend_mode_property(struct drm_plane *plane, + unsigned int supported_modes); #endif diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index 14b1607..6e2c045 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -44,6 +44,8 @@ * @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 + * @pixel_blend_mode: how the plane's framebuffer alpha channel is used when + * blending with the background colour. * @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 @@ -116,6 +118,7 @@ struct drm_plane_state {
/* Plane opacity */ u16 alpha; + uint16_t pixel_blend_mode;
/* Plane rotation */ unsigned int rotation; @@ -513,6 +516,7 @@ enum drm_plane_type { * @alpha_property: alpha property for this plane * @zpos_property: zpos property for this plane * @rotation_property: rotation property for this plane + * @blend_mode_property: blend mode property for this plane * @helper_private: mid-layer private data */ struct drm_plane { @@ -589,6 +593,7 @@ struct drm_plane { struct drm_property *alpha_property; struct drm_property *zpos_property; struct drm_property *rotation_property; + struct drm_property *blend_mode_property;
/** * @color_encoding_property:
On 1 June 2018 at 13:41, Lowry Li lowry.li@arm.com wrote:
Pixel blend modes represent the alpha blending equation selection, describing how the pixels from the current plane are composited with the background.
Add 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.
Signed-off-by: Lowry Li lowry.li@arm.com
drivers/gpu/drm/drm_atomic.c | 4 ++ drivers/gpu/drm/drm_atomic_helper.c | 1 + drivers/gpu/drm/drm_blend.c | 126 ++++++++++++++++++++++++++++++++++++ include/drm/drm_blend.h | 6 ++ include/drm/drm_plane.h | 5 ++ 5 files changed, 142 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 07fef42..1d18389 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -785,6 +785,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)) return -EINVAL;@@ -852,6 +854,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 130da51..7f5d463 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3515,6 +3515,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) plane->state->alpha = plane->alpha_property->values[1];
plane->state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI; }} EXPORT_SYMBOL(drm_atomic_helper_plane_reset); diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index a16a74d..ac6f19c 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 cursorplane, 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::
out.rgb = plane_alpha * fg.alpha * fg.rgb +
(1 - (plane_alpha * fg.alpha)) * bg.rgb
Using the following symbols:
``fg.rgb``:
Each of the RGB component values from the plane's pixel
``fg.alpha``:
Alpha component value from the plane's pixel. If the plane's
pixel format has no alpha component, then this is assumed to be
1.0. In these cases, this property has no effect, as all three
equations become equivalent.
``bg.rgb``:
Each of the RGB component values from the background
``plane_alpha``:
Plane alpha value set by the plane "alpha" property. If the
plane does not expose the "alpha" property, then this is
assumed to be 1.0
- Note that all the property extensions described here apply either to the
- plane or the CRTC (e.g. for the background color, which currently is not
- exposed and assumed to be black).
@@ -448,3 +494,83 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, return 0; } EXPORT_SYMBOL(drm_atomic_normalize_zpos);
+/**
- drm_plane_create_blend_mode_property - create a new blend mode property
- @plane: drm plane
- @supported_modes: bitmask of supported modes, must include
BIT(DRM_MODE_BLEND_PREMULTI). Current DRM assumption is
that alpha is premultiplied, and old userspace can break if
the property defaults to coverage.
Thanks for the explanation Lowry. Sigh, old userspace :-\
One pedantic suggestion s/defaults to coverage/defaults to anything else/ Since defaulting to "none" or any future blend mode is also a no-go.
I wouldn't bother resending solely for ^^. Perhaps squash locally before merging?
With that, the patch is Reviewed-by: Emil Velikov emil.velikov@collabora.com
-Emil
On Mon, Jun 04, 2018 at 02:49:26PM +0100, Emil Velikov wrote:
On 1 June 2018 at 13:41, Lowry Li lowry.li@arm.com wrote:
Pixel blend modes represent the alpha blending equation selection, describing how the pixels from the current plane are composited with the background.
Add 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.
Signed-off-by: Lowry Li lowry.li@arm.com
drivers/gpu/drm/drm_atomic.c | 4 ++ drivers/gpu/drm/drm_atomic_helper.c | 1 + drivers/gpu/drm/drm_blend.c | 126 ++++++++++++++++++++++++++++++++++++ include/drm/drm_blend.h | 6 ++ include/drm/drm_plane.h | 5 ++ 5 files changed, 142 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 07fef42..1d18389 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -785,6 +785,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)) return -EINVAL;@@ -852,6 +854,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 130da51..7f5d463 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3515,6 +3515,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) plane->state->alpha = plane->alpha_property->values[1];
plane->state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI; }} EXPORT_SYMBOL(drm_atomic_helper_plane_reset); diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index a16a74d..ac6f19c 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 cursorplane, 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::
out.rgb = plane_alpha * fg.alpha * fg.rgb +
(1 - (plane_alpha * fg.alpha)) * bg.rgb
Using the following symbols:
``fg.rgb``:
Each of the RGB component values from the plane's pixel
``fg.alpha``:
Alpha component value from the plane's pixel. If the plane's
pixel format has no alpha component, then this is assumed to be
1.0. In these cases, this property has no effect, as all three
equations become equivalent.
``bg.rgb``:
Each of the RGB component values from the background
``plane_alpha``:
Plane alpha value set by the plane "alpha" property. If the
plane does not expose the "alpha" property, then this is
assumed to be 1.0
- Note that all the property extensions described here apply either to the
- plane or the CRTC (e.g. for the background color, which currently is not
- exposed and assumed to be black).
@@ -448,3 +494,83 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, return 0; } EXPORT_SYMBOL(drm_atomic_normalize_zpos);
+/**
- drm_plane_create_blend_mode_property - create a new blend mode property
- @plane: drm plane
- @supported_modes: bitmask of supported modes, must include
BIT(DRM_MODE_BLEND_PREMULTI). Current DRM assumption is
that alpha is premultiplied, and old userspace can break if
the property defaults to coverage.
Thanks for the explanation Lowry. Sigh, old userspace :-\
One pedantic suggestion s/defaults to coverage/defaults to anything else/ Since defaulting to "none" or any future blend mode is also a no-go.
I wouldn't bother resending solely for ^^. Perhaps squash locally before merging?
With that, the patch is Reviewed-by: Emil Velikov emil.velikov@collabora.com
-Emil
Hi Emil,
Thanks a lot for your review and will change that.
On Mon, Jun 04, 2018 at 02:49:26PM +0100, Emil Velikov wrote:
On 1 June 2018 at 13:41, Lowry Li lowry.li@arm.com wrote:
Pixel blend modes represent the alpha blending equation selection, describing how the pixels from the current plane are composited with the background.
Add 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.
Signed-off-by: Lowry Li lowry.li@arm.com
drivers/gpu/drm/drm_atomic.c | 4 ++ drivers/gpu/drm/drm_atomic_helper.c | 1 + drivers/gpu/drm/drm_blend.c | 126 ++++++++++++++++++++++++++++++++++++ include/drm/drm_blend.h | 6 ++ include/drm/drm_plane.h | 5 ++ 5 files changed, 142 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 07fef42..1d18389 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -785,6 +785,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)) return -EINVAL;@@ -852,6 +854,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 130da51..7f5d463 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3515,6 +3515,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) plane->state->alpha = plane->alpha_property->values[1];
plane->state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI; }} EXPORT_SYMBOL(drm_atomic_helper_plane_reset); diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index a16a74d..ac6f19c 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 cursorplane, 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::
out.rgb = plane_alpha * fg.alpha * fg.rgb +
(1 - (plane_alpha * fg.alpha)) * bg.rgb
Using the following symbols:
``fg.rgb``:
Each of the RGB component values from the plane's pixel
``fg.alpha``:
Alpha component value from the plane's pixel. If the plane's
pixel format has no alpha component, then this is assumed to be
1.0. In these cases, this property has no effect, as all three
equations become equivalent.
``bg.rgb``:
Each of the RGB component values from the background
``plane_alpha``:
Plane alpha value set by the plane "alpha" property. If the
plane does not expose the "alpha" property, then this is
assumed to be 1.0
- Note that all the property extensions described here apply either to the
- plane or the CRTC (e.g. for the background color, which currently is not
- exposed and assumed to be black).
@@ -448,3 +494,83 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, return 0; } EXPORT_SYMBOL(drm_atomic_normalize_zpos);
+/**
- drm_plane_create_blend_mode_property - create a new blend mode property
- @plane: drm plane
- @supported_modes: bitmask of supported modes, must include
BIT(DRM_MODE_BLEND_PREMULTI). Current DRM assumption is
that alpha is premultiplied, and old userspace can break if
the property defaults to coverage.
Thanks for the explanation Lowry. Sigh, old userspace :-\
One pedantic suggestion s/defaults to coverage/defaults to anything else/ Since defaulting to "none" or any future blend mode is also a no-go.
I wouldn't bother resending solely for ^^. Perhaps squash locally before merging?
With that, the patch is Reviewed-by: Emil Velikov emil.velikov@collabora.com
-Emil
Hi Emil,
The comments has been changed as you request in the v4 patchset. Since it also did some refine in other places (please refer to the cover letter), please let me know if I can have your reviewed-tag :)
Thanks a lot for your time.
Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com --- lib/igt_kms.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++ lib/igt_kms.h | 16 ++++++++++++++- 2 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c index cb382c893c6c..9ac7ce73542a 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -2893,6 +2893,37 @@ uint64_t igt_plane_get_prop(igt_plane_t *plane, enum igt_atomic_plane_properties plane->drm_plane->plane_id, plane->props[prop]); }
+static uint64_t igt_mode_object_get_prop_enum_value(int drm_fd, uint32_t id, const char *str) +{ + drmModePropertyPtr prop = drmModeGetProperty(drm_fd, id); + int i; + + igt_assert(id); + igt_assert(prop); + + for (i = 0; i < prop->count_enums; i++) + if (!strcmp(str, prop->enums[i].name)) { + uint64_t ret = prop->enums[i].value; + drmModeFreeProperty(prop); + return ret; + } + + igt_assert_f(0, "Could not find property value for %s\n", str); + return 0; +} + +void igt_plane_set_prop_enum(igt_plane_t *plane, + enum igt_atomic_plane_properties prop, + const char *val) +{ + igt_display_t *display = plane->display; + uint64_t uval = + igt_mode_object_get_prop_enum_value(display->drm_fd, + plane->props[prop], val); + + igt_plane_set_prop_value(plane, prop, uval); +} + /** * igt_plane_replace_prop_blob: * @plane: plane to set property on. @@ -2944,6 +2975,18 @@ uint64_t igt_output_get_prop(igt_output_t *output, enum igt_atomic_connector_pro output->id, output->props[prop]); }
+void igt_output_set_prop_enum(igt_output_t *output, + enum igt_atomic_connector_properties prop, + const char *val) +{ + igt_display_t *display = output->display; + uint64_t uval = + igt_mode_object_get_prop_enum_value(display->drm_fd, + output->props[prop], val); + + igt_output_set_prop_value(output, prop, uval); +} + /** * igt_output_replace_prop_blob: * @output: output to set property on. @@ -2995,6 +3038,19 @@ uint64_t igt_pipe_obj_get_prop(igt_pipe_t *pipe, enum igt_atomic_crtc_properties pipe->crtc_id, pipe->props[prop]); }
+void igt_pipe_obj_set_prop_enum(igt_pipe_t *pipe_obj, + enum igt_atomic_crtc_properties prop, + const char *val) +{ + igt_display_t *display = pipe_obj->display; + uint64_t uval = + igt_mode_object_get_prop_enum_value(display->drm_fd, + pipe_obj->props[prop], val); + + igt_pipe_obj_set_prop_value(pipe_obj, prop, uval); +} + + /** * igt_pipe_obj_replace_prop_blob: * @pipe: pipe to set property on. diff --git a/lib/igt_kms.h b/lib/igt_kms.h index 8990a6fd6d12..b55881885b11 100644 --- a/lib/igt_kms.h +++ b/lib/igt_kms.h @@ -569,6 +569,10 @@ uint64_t igt_plane_get_prop(igt_plane_t *plane, enum igt_atomic_plane_properties igt_plane_set_prop_changed(plane, prop); \ } while (0)
+extern void igt_plane_set_prop_enum(igt_plane_t *plane, + enum igt_atomic_plane_properties prop, + const char *val); + extern void igt_plane_replace_prop_blob(igt_plane_t *plane, enum igt_atomic_plane_properties prop, const void *ptr, size_t length); @@ -604,10 +608,13 @@ uint64_t igt_output_get_prop(igt_output_t *output, enum igt_atomic_connector_pro igt_output_set_prop_changed(output, prop); \ } while (0)
+extern void igt_output_set_prop_enum(igt_output_t *output, + enum igt_atomic_connector_properties prop, + const char *val); + extern void igt_output_replace_prop_blob(igt_output_t *output, enum igt_atomic_connector_properties prop, const void *ptr, size_t length); - /** * igt_pipe_obj_has_prop: * @pipe: Pipe to check. @@ -688,6 +695,13 @@ igt_pipe_has_prop(igt_display_t *display, enum pipe pipe, #define igt_pipe_set_prop_value(display, pipe, prop, value) \ igt_pipe_obj_set_prop_value(&(display)->pipes[(pipe)], prop, value)
+extern void igt_pipe_obj_set_prop_enum(igt_pipe_t *pipe, + enum igt_atomic_crtc_properties prop, + const char *val); + +#define igt_pipe_set_prop_enum(display, pipe, prop, val) \ + igt_pipe_obj_set_prop_enum(&(display)->pipes[(pipe)], prop, val) + extern void igt_pipe_obj_replace_prop_blob(igt_pipe_t *pipe, enum igt_atomic_crtc_properties prop, const void *ptr, size_t length);
Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com --- lib/igt_kms.c | 2 + lib/igt_kms.h | 2 + tests/Makefile.sources | 1 + tests/kms_plane_alpha_blend.c | 561 ++++++++++++++++++++++++++++++++++ tests/meson.build | 1 + 5 files changed, 567 insertions(+) create mode 100644 tests/kms_plane_alpha_blend.c
diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 9ac7ce73542a..f348f1c71e5e 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -173,6 +173,8 @@ const char * const igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = { [IGT_PLANE_TYPE] = "type", [IGT_PLANE_ROTATION] = "rotation", [IGT_PLANE_IN_FORMATS] = "IN_FORMATS", + [IGT_PLANE_PIXEL_BLEND_MODE] = "pixel blend mode", + [IGT_PLANE_ALPHA] = "alpha", };
const char * const igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = { diff --git a/lib/igt_kms.h b/lib/igt_kms.h index b55881885b11..762eafb2e47e 100644 --- a/lib/igt_kms.h +++ b/lib/igt_kms.h @@ -264,6 +264,8 @@ enum igt_atomic_plane_properties { IGT_PLANE_TYPE, IGT_PLANE_ROTATION, IGT_PLANE_IN_FORMATS, + IGT_PLANE_PIXEL_BLEND_MODE, + IGT_PLANE_ALPHA, IGT_NUM_PLANE_PROPS };
diff --git a/tests/Makefile.sources b/tests/Makefile.sources index 8197d59417b3..b607189dc37a 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -200,6 +200,7 @@ TESTS_progs = \ kms_pipe_b_c_ivb \ kms_pipe_crc_basic \ kms_plane \ + kms_plane_alpha_blend \ kms_plane_lowres \ kms_plane_multiple \ kms_plane_scaling \ diff --git a/tests/kms_plane_alpha_blend.c b/tests/kms_plane_alpha_blend.c new file mode 100644 index 000000000000..8ce9c460e9c2 --- /dev/null +++ b/tests/kms_plane_alpha_blend.c @@ -0,0 +1,561 @@ +/* + * Copyright © 2018 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ + +#include "igt.h" +#include <math.h> + +IGT_TEST_DESCRIPTION("Test plane alpha and blending mode properties"); + +typedef struct { + int gfx_fd; + igt_display_t display; + struct igt_fb xrgb_fb, argb_fb_0, argb_fb_cov_0, argb_fb_7e, argb_fb_cov_7e, argb_fb_fc, argb_fb_cov_fc, argb_fb_100, black_fb, gray_fb; + igt_crc_t ref_crc; + igt_pipe_crc_t *pipe_crc; +} data_t; + +static void __draw_gradient(struct igt_fb *fb, int w, int h, double a, cairo_t *cr) +{ + cairo_pattern_t *pat; + + pat = cairo_pattern_create_linear(0, 0, w, h); + cairo_pattern_add_color_stop_rgba(pat, 0.00, 0.00, 0.00, 0.00, 1.); + cairo_pattern_add_color_stop_rgba(pat, 0.25, 1.00, 1.00, 0.00, 1.); + cairo_pattern_add_color_stop_rgba(pat, 0.50, 0.00, 1.00, 1.00, 1.); + cairo_pattern_add_color_stop_rgba(pat, 0.75, 1.00, 0.00, 1.00, 1.); + cairo_pattern_add_color_stop_rgba(pat, 1.00, 1.00, 1.00, 1.00, 1.); + + cairo_rectangle(cr, 0, 0, w, h); + cairo_set_source(cr, pat); + cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE); + cairo_paint_with_alpha(cr, a); + cairo_pattern_destroy(pat); +} + +static void draw_gradient(struct igt_fb *fb, int w, int h, double a) +{ + cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb); + + __draw_gradient(fb, w, h, a, cr); + + igt_put_cairo_ctx(fb->fd, fb, cr); +} + +static void draw_gradient_coverage(struct igt_fb *fb, int w, int h, uint8_t a) +{ + cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb); + uint8_t *data = cairo_image_surface_get_data(fb->cairo_surface); + uint32_t stride = fb->stride; + int i; + + __draw_gradient(fb, w, h, 1., cr); + + for (; h--; data += stride) + for (i = 0; i < w; i++) + data[i * 4 + 3] = a; + + igt_put_cairo_ctx(fb->fd, fb, cr); +} + +static void draw_squares(struct igt_fb *fb, int w, int h, double a) +{ + cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb); + + igt_paint_color_alpha(cr, 0, 0, w / 2, h / 2, 1., 0., 0., a); + igt_paint_color_alpha(cr, w / 2, 0, w / 2, h / 2, 0., 1., 0., a); + igt_paint_color_alpha(cr, 0, h / 2, w / 2, h / 2, 0., 0., 1., a); + igt_paint_color_alpha(cr, w / 2, h / 2, w / 4, h / 2, 1., 1., 1., a); + igt_paint_color_alpha(cr, 3 * w / 4, h / 2, w / 4, h / 2, 0., 0., 0., a); + + igt_put_cairo_ctx(fb->fd, fb, cr); +} + +static void draw_squares_coverage(struct igt_fb *fb, int w, int h, uint8_t as) +{ + cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb); + int i, j; + uint32_t *data = (void *)cairo_image_surface_get_data(fb->cairo_surface); + uint32_t stride = fb->stride / 4; + uint32_t a = as << 24; + + for (j = 0; j < h / 2; j++) { + for (i = 0; i < w / 2; i++) + data[j * stride + i] = a | 0xff0000; + + for (; i < w; i++) + data[j * stride + i] = a | 0xff00; + } + + for (j = h / 2; j < h; j++) { + for (i = 0; i < w / 2; i++) + data[j * stride + i] = a | 0xff; + + for (; i < 3 * w / 4; i++) + data[j * stride + i] = a | 0xffffff; + + for (; i < w; i++) + data[j * stride + i] = a; + } + + igt_put_cairo_ctx(fb->fd, fb, cr); +} + +static void reset_alpha(igt_display_t *display, enum pipe pipe) +{ + igt_plane_t *plane; + + for_each_plane_on_pipe(display, pipe, plane) { + if (igt_plane_has_prop(plane, IGT_PLANE_ALPHA)) + igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0xffff); + + if (igt_plane_has_prop(plane, IGT_PLANE_PIXEL_BLEND_MODE)) + igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_BLEND_MODE, "Pre-multiplied"); + } +} + +static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe) +{ + drmModeModeInfo *mode; + igt_display_t *display = &data->display; + int w, h; + igt_plane_t *primary = igt_pipe_get_plane_type(&display->pipes[pipe], DRM_PLANE_TYPE_PRIMARY); + + igt_display_reset(display); + igt_output_set_pipe(output, pipe); + + /* create the pipe_crc object for this pipe */ + igt_pipe_crc_free(data->pipe_crc); + data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO); + + mode = igt_output_get_mode(output); + w = mode->hdisplay; + h = mode->vdisplay; + + /* recreate all fbs if incompatible */ + if (data->xrgb_fb.width != w || data->xrgb_fb.height != h) { + cairo_t *cr; + + igt_remove_fb(data->gfx_fd, &data->xrgb_fb); + igt_remove_fb(data->gfx_fd, &data->argb_fb_0); + igt_remove_fb(data->gfx_fd, &data->argb_fb_cov_0); + igt_remove_fb(data->gfx_fd, &data->argb_fb_7e); + igt_remove_fb(data->gfx_fd, &data->argb_fb_fc); + igt_remove_fb(data->gfx_fd, &data->argb_fb_cov_7e); + igt_remove_fb(data->gfx_fd, &data->argb_fb_cov_fc); + igt_remove_fb(data->gfx_fd, &data->argb_fb_100); + igt_remove_fb(data->gfx_fd, &data->black_fb); + igt_remove_fb(data->gfx_fd, &data->gray_fb); + + igt_create_fb(data->gfx_fd, w, h, + DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, + &data->xrgb_fb); + draw_gradient(&data->xrgb_fb, w, h, 1.); + + igt_create_fb(data->gfx_fd, w, h, + DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE, + &data->argb_fb_cov_0); + draw_gradient_coverage(&data->argb_fb_cov_0, w, h, 0); + + igt_create_fb(data->gfx_fd, w, h, + DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE, + &data->argb_fb_0); + + cr = igt_get_cairo_ctx(data->gfx_fd, &data->argb_fb_0); + igt_paint_color_alpha(cr, 0, 0, w, h, 0., 0., 0., 1.0); + igt_put_cairo_ctx(data->gfx_fd, &data->argb_fb_0, cr); + + igt_create_fb(data->gfx_fd, w, h, + DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE, + &data->argb_fb_7e); + draw_squares(&data->argb_fb_7e, w, h, 126. / 255.); + + igt_create_fb(data->gfx_fd, w, h, + DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE, + &data->argb_fb_cov_7e); + draw_squares_coverage(&data->argb_fb_cov_7e, w, h, 0x7e); + + igt_create_fb(data->gfx_fd, w, h, + DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE, + &data->argb_fb_fc); + draw_squares(&data->argb_fb_fc, w, h, 252. / 255.); + + igt_create_fb(data->gfx_fd, w, h, + DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE, + &data->argb_fb_cov_fc); + draw_squares_coverage(&data->argb_fb_cov_fc, w, h, 0xfc); + + igt_create_fb(data->gfx_fd, w, h, + DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE, + &data->argb_fb_100); + draw_gradient(&data->argb_fb_100, w, h, 1.); + + igt_create_fb(data->gfx_fd, w, h, + DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, + &data->black_fb); + + igt_create_color_fb(data->gfx_fd, w, h, + DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, + .5, .5, .5, &data->gray_fb); + } + + igt_plane_set_fb(primary, &data->black_fb); + /* reset alpha property to default */ + reset_alpha(display, pipe); +} + +static void basic_alpha(data_t *data, enum pipe pipe, igt_plane_t *plane) +{ + igt_display_t *display = &data->display; + igt_crc_t ref_crc, crc; + int i; + + /* Testcase 1: alpha = 0.0, plane should be transparant. */ + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_start(data->pipe_crc); + igt_pipe_crc_get_single(data->pipe_crc, &ref_crc); + + igt_plane_set_fb(plane, &data->argb_fb_0); + + /* transparant fb should be transparant, no matter what.. */ + for (i = 7; i < 256; i += 8) { + igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, i | (i << 8)); + igt_display_commit2(display, COMMIT_ATOMIC); + + igt_pipe_crc_drain(data->pipe_crc); + igt_pipe_crc_get_single(data->pipe_crc, &crc); + igt_assert_crc_equal(&ref_crc, &crc); + } + + /* And test alpha = 0, should give same CRC, but doesn't on some i915 platforms. */ + igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0); + igt_display_commit2(display, COMMIT_ATOMIC); + + igt_pipe_crc_drain(data->pipe_crc); + igt_pipe_crc_get_single(data->pipe_crc, &crc); + igt_pipe_crc_stop(data->pipe_crc); + igt_assert_crc_equal(&ref_crc, &crc); +} + +static void argb_opaque(data_t *data, enum pipe pipe, igt_plane_t *plane) +{ + igt_display_t *display = &data->display; + igt_crc_t ref_crc, crc; + + /* alpha = 1.0, plane should be fully opaque, test with an opaque fb */ + igt_plane_set_fb(plane, &data->xrgb_fb); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc); + + igt_plane_set_fb(plane, &data->argb_fb_100); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &crc); + + igt_assert_crc_equal(&ref_crc, &crc); +} + +static void argb_transparant(data_t *data, enum pipe pipe, igt_plane_t *plane) +{ + igt_display_t *display = &data->display; + igt_crc_t ref_crc, crc; + + /* alpha = 1.0, plane should be fully opaque, test with a transparant fb */ + igt_plane_set_fb(plane, NULL); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc); + + igt_plane_set_fb(plane, &data->argb_fb_0); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &crc); + + igt_assert_crc_equal(&ref_crc, &crc); +} + +static void constant_alpha_min(data_t *data, enum pipe pipe, igt_plane_t *plane) +{ + igt_display_t *display = &data->display; + igt_crc_t ref_crc, crc; + + igt_plane_set_fb(plane, NULL); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc); + + igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_BLEND_MODE, "None"); + igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0); + igt_plane_set_fb(plane, &data->argb_fb_100); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &crc); + igt_assert_crc_equal(&ref_crc, &crc); + + igt_plane_set_fb(plane, &data->argb_fb_0); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &crc); + igt_assert_crc_equal(&ref_crc, &crc); +} + +static void constant_alpha_mid(data_t *data, enum pipe pipe, igt_plane_t *plane) +{ + igt_display_t *display = &data->display; + igt_crc_t ref_crc, crc; + + if (plane->type != DRM_PLANE_TYPE_PRIMARY) + igt_plane_set_fb(igt_pipe_get_plane_type(&display->pipes[pipe], DRM_PLANE_TYPE_PRIMARY), &data->gray_fb); + + igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_BLEND_MODE, "None"); + igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0x7fff); + igt_plane_set_fb(plane, &data->xrgb_fb); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc); + + igt_plane_set_fb(plane, &data->argb_fb_cov_0); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &crc); + igt_assert_crc_equal(&ref_crc, &crc); + + igt_plane_set_fb(plane, &data->argb_fb_100); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &crc); + igt_assert_crc_equal(&ref_crc, &crc); +} + +static void constant_alpha_max(data_t *data, enum pipe pipe, igt_plane_t *plane) +{ + igt_display_t *display = &data->display; + igt_crc_t ref_crc, crc; + + if (plane->type != DRM_PLANE_TYPE_PRIMARY) + igt_plane_set_fb(igt_pipe_get_plane_type(&display->pipes[pipe], DRM_PLANE_TYPE_PRIMARY), &data->gray_fb); + + igt_plane_set_fb(plane, &data->argb_fb_100); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc); + + igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_BLEND_MODE, "None"); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &crc); + igt_assert_crc_equal(&ref_crc, &crc); + + igt_plane_set_fb(plane, &data->argb_fb_cov_0); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &crc); + igt_assert_crc_equal(&ref_crc, &crc); + + igt_plane_set_fb(plane, &data->xrgb_fb); + igt_display_commit2(display, COMMIT_ATOMIC); + igt_pipe_crc_collect_crc(data->pipe_crc, &crc); + igt_assert_crc_equal(&ref_crc, &crc); + + igt_plane_set_fb(plane, NULL); +} + +static void alpha_7efc(data_t *data, enum pipe pipe, igt_plane_t *plane) +{ + igt_display_t *display = &data->display; + igt_crc_t ref_crc = {}, crc = {}; + int i; + + if (plane->type != DRM_PLANE_TYPE_PRIMARY) + igt_plane_set_fb(igt_pipe_get_plane_type(&display->pipes[pipe], DRM_PLANE_TYPE_PRIMARY), &data->gray_fb); + + igt_pipe_crc_start(data->pipe_crc); + + /* for coverage, plane alpha and fb alpha should be swappable, so swap fb and alpha */ + for (i = 0; i < 256; i += 8) { + igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, ((i/2) << 8) | (i/2)); + igt_plane_set_fb(plane, &data->argb_fb_fc); + igt_display_commit2(display, COMMIT_ATOMIC); + + igt_pipe_crc_drain(data->pipe_crc); + igt_pipe_crc_get_single(data->pipe_crc, &ref_crc); + + igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, (i << 8) | i); + igt_plane_set_fb(plane, &data->argb_fb_7e); + igt_display_commit2(display, COMMIT_ATOMIC); + + igt_pipe_crc_drain(data->pipe_crc); + igt_pipe_crc_get_single(data->pipe_crc, &crc); + igt_assert_crc_equal(&ref_crc, &crc); + } +} + +static void coverage_7efc(data_t *data, enum pipe pipe, igt_plane_t *plane) +{ + igt_display_t *display = &data->display; + igt_crc_t ref_crc = {}, crc = {}; + int i; + + igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_BLEND_MODE, "Coverage"); + igt_pipe_crc_start(data->pipe_crc); + + /* for coverage, plane alpha and fb alpha should be swappable, so swap fb and alpha */ + for (i = 0; i < 256; i += 8) { + igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, ((i/2) << 8) | (i/2)); + igt_plane_set_fb(plane, &data->argb_fb_cov_fc); + igt_display_commit2(display, COMMIT_ATOMIC); + + igt_pipe_crc_drain(data->pipe_crc); + igt_pipe_crc_get_single(data->pipe_crc, &ref_crc); + + igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, (i << 8) | i); + igt_plane_set_fb(plane, &data->argb_fb_cov_7e); + igt_display_commit2(display, COMMIT_ATOMIC); + + igt_pipe_crc_drain(data->pipe_crc); + igt_pipe_crc_get_single(data->pipe_crc, &crc); + igt_assert_crc_equal(&ref_crc, &crc); + } +} + +static void coverage_premult_constant(data_t *data, enum pipe pipe, igt_plane_t *plane) +{ + igt_display_t *display = &data->display; + igt_crc_t ref_crc = {}, crc = {}; + + igt_pipe_crc_start(data->pipe_crc); + + /* Set a background color on the primary fb for testing */ + if (plane->type != DRM_PLANE_TYPE_PRIMARY) + igt_plane_set_fb(igt_pipe_get_plane_type(&display->pipes[pipe], DRM_PLANE_TYPE_PRIMARY), &data->gray_fb); + + igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_BLEND_MODE, "Coverage"); + igt_plane_set_fb(plane, &data->argb_fb_cov_7e); + igt_display_commit2(display, COMMIT_ATOMIC); + + igt_pipe_crc_drain(data->pipe_crc); + igt_pipe_crc_get_single(data->pipe_crc, &ref_crc); + + igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_BLEND_MODE, "Pre-multiplied"); + igt_plane_set_fb(plane, &data->argb_fb_7e); + igt_display_commit2(display, COMMIT_ATOMIC); + + igt_pipe_crc_drain(data->pipe_crc); + igt_pipe_crc_get_single(data->pipe_crc, &crc); + igt_assert_crc_equal(&ref_crc, &crc); + + igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_BLEND_MODE, "None"); + igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0x7e7e); + igt_plane_set_fb(plane, &data->argb_fb_cov_7e); + igt_display_commit2(display, COMMIT_ATOMIC); + + igt_pipe_crc_drain(data->pipe_crc); + igt_pipe_crc_get_single(data->pipe_crc, &crc); + igt_assert_crc_equal(&ref_crc, &crc); +} + +static void run_test_on_pipe_planes(data_t *data, enum pipe pipe, bool blend, + void(*test)(data_t *, enum pipe, igt_plane_t *)) +{ + igt_display_t *display = &data->display; + igt_output_t *output = igt_get_single_output_for_pipe(display, pipe); + igt_plane_t *plane; + bool found = false; + + for_each_plane_on_pipe(display, pipe, plane) { + if (!igt_plane_has_prop(plane, IGT_PLANE_ALPHA)) + continue; + + if (blend && !igt_plane_has_prop(plane, IGT_PLANE_PIXEL_BLEND_MODE)) + continue; + + prepare_crtc(data, output, pipe); + + /* reset plane alpha properties between each plane */ + reset_alpha(display, pipe); + + found = true; + igt_info("Testing plane %u\n", plane->index); + test(data, pipe, plane); + igt_plane_set_fb(plane, NULL); + } + + igt_require_f(found, "No planes with %s property found\n", blend ? "pixel blending mode" : "alpha"); +} + +static void run_subtests(data_t *data, enum pipe pipe) +{ + igt_fixture { + bool found = false; + igt_plane_t *plane; + + igt_display_require_output_on_pipe(&data->display, pipe); + for_each_plane_on_pipe(&data->display, pipe, plane) { + if (!igt_plane_has_prop(plane, IGT_PLANE_ALPHA)) + continue; + + found = true; + break; + } + + igt_require_f(found, "Found no plane on pipe %s with alpha blending supported\n", + kmstest_pipe_name(pipe)); + } + + igt_subtest_f("pipe-%s-alpha-basic", kmstest_pipe_name(pipe)) + run_test_on_pipe_planes(data, pipe, false, basic_alpha); + + igt_subtest_f("pipe-%s-alpha-7efc", kmstest_pipe_name(pipe)) + run_test_on_pipe_planes(data, pipe, false, alpha_7efc); + + igt_subtest_f("pipe-%s-coverage-7efc", kmstest_pipe_name(pipe)) + run_test_on_pipe_planes(data, pipe, true, coverage_7efc); + + igt_subtest_f("pipe-%s-coverage-vs-premult-vs-constant", kmstest_pipe_name(pipe)) + run_test_on_pipe_planes(data, pipe, true, coverage_premult_constant); + + igt_subtest_f("pipe-%s-alpha-transparant-fb", kmstest_pipe_name(pipe)) + run_test_on_pipe_planes(data, pipe, false, argb_transparant); + + igt_subtest_f("pipe-%s-alpha-opaque-fb", kmstest_pipe_name(pipe)) + run_test_on_pipe_planes(data, pipe, false, argb_opaque); + + igt_subtest_f("pipe-%s-constant-alpha-min", kmstest_pipe_name(pipe)) + run_test_on_pipe_planes(data, pipe, true, constant_alpha_min); + + igt_subtest_f("pipe-%s-constant-alpha-mid", kmstest_pipe_name(pipe)) + run_test_on_pipe_planes(data, pipe, true, constant_alpha_mid); + + igt_subtest_f("pipe-%s-constant-alpha-max", kmstest_pipe_name(pipe)) + run_test_on_pipe_planes(data, pipe, true, constant_alpha_max); +} + +igt_main +{ + data_t data = {}; + enum pipe pipe; + + igt_fixture { + igt_skip_on_simulation(); + + data.gfx_fd = drm_open_driver(DRIVER_ANY); + igt_require_pipe_crc(data.gfx_fd); + igt_display_init(&data.display, data.gfx_fd); + igt_require(data.display.is_atomic); + } + + for_each_pipe_static(pipe) + igt_subtest_group + run_subtests(&data, pipe); + + igt_fixture + igt_display_fini(&data.display); +} diff --git a/tests/meson.build b/tests/meson.build index 826f3c4678d0..4b2fc20bf8fe 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -176,6 +176,7 @@ test_progs = [ 'kms_pipe_b_c_ivb', 'kms_pipe_crc_basic', 'kms_plane', + 'kms_plane_alpha_blend', 'kms_plane_lowres', 'kms_plane_multiple', 'kms_plane_scaling',
Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 2 + drivers/gpu/drm/i915/intel_display.c | 67 ++++++++++++++++++++-------- drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_fbc.c | 4 ++ drivers/gpu/drm/i915/intel_sprite.c | 24 +++++++++- 5 files changed, 79 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 327829cc61f8..23f0cb0fad0e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6425,8 +6425,10 @@ enum { #define _PLANE_KEYVAL_2_A 0x70294 #define _PLANE_KEYMSK_1_A 0x70198 #define _PLANE_KEYMSK_2_A 0x70298 +#define PLANE_KEYMSK_ALPHA_ENABLE (1 << 31) #define _PLANE_KEYMAX_1_A 0x701a0 #define _PLANE_KEYMAX_2_A 0x702a0 +#define PLANE_KEYMAX_ALPHA_SHIFT 24 #define _PLANE_AUX_DIST_1_A 0x701c0 #define _PLANE_AUX_DIST_2_A 0x702c0 #define _PLANE_AUX_OFFSET_1_A 0x701c4 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d48256f89a50..8ddff09c3110 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3170,6 +3170,21 @@ int skl_check_plane_surface(const struct intel_crtc_state *crtc_state, return -EINVAL; }
+ /* HW only has 8 bits pixel precision, disable plane if invisible */ + if (!(plane_state->base.alpha >> 8)) { + plane_state->base.visible = false; + return 0; + } + + if (plane_state->base.alpha < 0xff00) { + plane_state->flags |= PLANE_ALPHA_ENABLED; + + /* FBC cannot be enabled with per pixel alpha */ + if (plane_state->base.pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE && + fb->format->has_alpha) + plane_state->flags |= PLANE_ALPHA_NO_FBC; + } + if (!plane_state->base.visible) return 0;
@@ -3496,30 +3511,39 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format) return 0; }
-/* - * XXX: For ARBG/ABGR formats we default to expecting scanout buffers - * to be already pre-multiplied. We need to add a knob (or a different - * DRM_FORMAT) for user-space to configure that. - */ -static u32 skl_plane_ctl_alpha(uint32_t pixel_format) +static u32 skl_plane_ctl_alpha(const struct intel_plane_state *plane_state) { - switch (pixel_format) { - case DRM_FORMAT_ABGR8888: - case DRM_FORMAT_ARGB8888: + if (!plane_state->base.fb->format->has_alpha) + return PLANE_CTL_ALPHA_DISABLE; + + switch (plane_state->base.pixel_blend_mode) { + case DRM_MODE_BLEND_PIXEL_NONE: + return PLANE_CTL_ALPHA_DISABLE; + case DRM_MODE_BLEND_PREMULTI: return PLANE_CTL_ALPHA_SW_PREMULTIPLY; + case DRM_MODE_BLEND_COVERAGE: + return PLANE_CTL_ALPHA_HW_PREMULTIPLY; default: - return PLANE_CTL_ALPHA_DISABLE; + MISSING_CASE(plane_state->base.pixel_blend_mode); + return 0; } }
-static u32 glk_plane_color_ctl_alpha(uint32_t pixel_format) +static u32 glk_plane_color_ctl_alpha(const struct intel_plane_state *plane_state) { - switch (pixel_format) { - case DRM_FORMAT_ABGR8888: - case DRM_FORMAT_ARGB8888: + if (!plane_state->base.fb->format->has_alpha) + return PLANE_COLOR_ALPHA_DISABLE; + + switch (plane_state->base.pixel_blend_mode) { + case DRM_MODE_BLEND_PIXEL_NONE: + return PLANE_COLOR_ALPHA_DISABLE; + case DRM_MODE_BLEND_PREMULTI: return PLANE_COLOR_ALPHA_SW_PREMULTIPLY; + case DRM_MODE_BLEND_COVERAGE: + return PLANE_COLOR_ALPHA_HW_PREMULTIPLY; default: - return PLANE_COLOR_ALPHA_DISABLE; + MISSING_CASE(plane_state->base.pixel_blend_mode); + return 0; } }
@@ -3595,7 +3619,7 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state, plane_ctl = PLANE_CTL_ENABLE;
if (INTEL_GEN(dev_priv) < 10 && !IS_GEMINILAKE(dev_priv)) { - plane_ctl |= skl_plane_ctl_alpha(fb->format->format); + plane_ctl |= skl_plane_ctl_alpha(plane_state); plane_ctl |= PLANE_CTL_PIPE_GAMMA_ENABLE | PLANE_CTL_PIPE_CSC_ENABLE | @@ -3637,7 +3661,7 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, plane_color_ctl |= PLANE_COLOR_PIPE_CSC_ENABLE; } plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE; - plane_color_ctl |= glk_plane_color_ctl_alpha(fb->format->format); + plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
if (intel_format_is_yuv(fb->format->format)) { if (fb->format->format == DRM_FORMAT_NV12) { @@ -13623,7 +13647,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) DRM_MODE_ROTATE_0, supported_rotations);
- if (INTEL_GEN(dev_priv) >= 9) + if (INTEL_GEN(dev_priv) >= 9) { drm_plane_create_color_properties(&primary->base, BIT(DRM_COLOR_YCBCR_BT601) | BIT(DRM_COLOR_YCBCR_BT709), @@ -13632,6 +13656,13 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
+ drm_plane_create_alpha_property(&primary->base); + drm_plane_create_blend_mode_property(&primary->base, + BIT(DRM_MODE_BLEND_PIXEL_NONE) | + BIT(DRM_MODE_BLEND_PREMULTI) | + BIT(DRM_MODE_BLEND_COVERAGE)); + } + drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
return primary; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 2dc01be0c7cc..675dbe927a06 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -500,6 +500,8 @@ struct intel_plane_state { struct i915_vma *vma; unsigned long flags; #define PLANE_HAS_FENCE BIT(0) +#define PLANE_ALPHA_ENABLED BIT(1) +#define PLANE_ALPHA_NO_FBC BIT(2)
struct { u32 offset; diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 1f2f41e67dcd..20a2dba78cad 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -732,6 +732,10 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) fbc->no_fbc_reason = "framebuffer not tiled or fenced"; return false; } + if (cache->flags & PLANE_ALPHA_NO_FBC) { + fbc->no_fbc_reason = "per pixel alpha blending enabled"; + return false; + } if (INTEL_GEN(dev_priv) <= 4 && !IS_G4X(dev_priv) && cache->plane.rotation != DRM_MODE_ROTATE_0) { fbc->no_fbc_reason = "rotation unsupported"; diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0b1bd5de5192..4fcc7ce09a9c 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -254,6 +254,7 @@ skl_update_plane(struct intel_plane *plane, uint32_t y = plane_state->main.y; uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16; uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16; + u32 keymsk = 0, keymax = 0;
/* Sizes are 0 based */ src_w--; @@ -267,10 +268,20 @@ skl_update_plane(struct intel_plane *plane,
if (key->flags) { I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value); - I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), key->max_value); - I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), key->channel_mask); + + keymax = key->max_value & 0xffffff; + keymsk |= key->channel_mask & 0x3ffffff; }
+ keymax |= (plane_state->base.alpha >> 8) << PLANE_KEYMAX_ALPHA_SHIFT; + + if (plane_state->flags & PLANE_ALPHA_ENABLED || + plane_state->base.fb->format->has_alpha) + keymsk |= PLANE_KEYMSK_ALPHA_ENABLE; + + I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax); + I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), keymsk); + I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (y << 16) | x); I915_WRITE_FW(PLANE_STRIDE(pipe, plane_id), stride); I915_WRITE_FW(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w); @@ -1525,6 +1536,15 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv, DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
+ if (INTEL_GEN(dev_priv) >= 9) { + drm_plane_create_alpha_property(&intel_plane->base); + + drm_plane_create_blend_mode_property(&intel_plane->base, + BIT(DRM_MODE_BLEND_PIXEL_NONE) | + BIT(DRM_MODE_BLEND_PREMULTI) | + BIT(DRM_MODE_BLEND_COVERAGE)); + } + drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
return intel_plane;
On Mon, Jun 04, 2018 at 03:50:13PM +0200, Maarten Lankhorst wrote:
Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
drivers/gpu/drm/i915/i915_reg.h | 2 + drivers/gpu/drm/i915/intel_display.c | 67 ++++++++++++++++++++-------- drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_fbc.c | 4 ++ drivers/gpu/drm/i915/intel_sprite.c | 24 +++++++++- 5 files changed, 79 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 327829cc61f8..23f0cb0fad0e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6425,8 +6425,10 @@ enum { #define _PLANE_KEYVAL_2_A 0x70294 #define _PLANE_KEYMSK_1_A 0x70198 #define _PLANE_KEYMSK_2_A 0x70298 +#define PLANE_KEYMSK_ALPHA_ENABLE (1 << 31) #define _PLANE_KEYMAX_1_A 0x701a0 #define _PLANE_KEYMAX_2_A 0x702a0 +#define PLANE_KEYMAX_ALPHA_SHIFT 24
PLANE_KEYMAX_ALPHA(x)
#define _PLANE_AUX_DIST_1_A 0x701c0 #define _PLANE_AUX_DIST_2_A 0x702c0 #define _PLANE_AUX_OFFSET_1_A 0x701c4 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d48256f89a50..8ddff09c3110 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3170,6 +3170,21 @@ int skl_check_plane_surface(const struct intel_crtc_state *crtc_state, return -EINVAL; }
- /* HW only has 8 bits pixel precision, disable plane if invisible */
- if (!(plane_state->base.alpha >> 8)) {
plane_state->base.visible = false;return 0;- }
- if (plane_state->base.alpha < 0xff00) {
plane_state->flags |= PLANE_ALPHA_ENABLED;/* FBC cannot be enabled with per pixel alpha */if (plane_state->base.pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE &&fb->format->has_alpha)plane_state->flags |= PLANE_ALPHA_NO_FBC;
Why is this fbc per-pixel alpha check inside the constant alpha if block?
Also I'm not convinced by these plane state flags. They're not consistent with how we do everything else, so I would drop them.
- }
- if (!plane_state->base.visible) return 0;
@@ -3496,30 +3511,39 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format) return 0; }
-/*
- XXX: For ARBG/ABGR formats we default to expecting scanout buffers
- to be already pre-multiplied. We need to add a knob (or a different
- DRM_FORMAT) for user-space to configure that.
- */
-static u32 skl_plane_ctl_alpha(uint32_t pixel_format) +static u32 skl_plane_ctl_alpha(const struct intel_plane_state *plane_state) {
- switch (pixel_format) {
- case DRM_FORMAT_ABGR8888:
- case DRM_FORMAT_ARGB8888:
- if (!plane_state->base.fb->format->has_alpha)
return PLANE_CTL_ALPHA_DISABLE;- switch (plane_state->base.pixel_blend_mode) {
- case DRM_MODE_BLEND_PIXEL_NONE:
return PLANE_CTL_ALPHA_DISABLE;- case DRM_MODE_BLEND_PREMULTI: return PLANE_CTL_ALPHA_SW_PREMULTIPLY;
- case DRM_MODE_BLEND_COVERAGE:
return PLANE_CTL_ALPHA_HW_PREMULTIPLY;
return PLANE_CTL_ALPHA_DISABLE;
MISSING_CASE(plane_state->base.pixel_blend_mode);return 0;}
-static u32 glk_plane_color_ctl_alpha(uint32_t pixel_format) +static u32 glk_plane_color_ctl_alpha(const struct intel_plane_state *plane_state) {
- switch (pixel_format) {
- case DRM_FORMAT_ABGR8888:
- case DRM_FORMAT_ARGB8888:
- if (!plane_state->base.fb->format->has_alpha)
return PLANE_COLOR_ALPHA_DISABLE;- switch (plane_state->base.pixel_blend_mode) {
- case DRM_MODE_BLEND_PIXEL_NONE:
return PLANE_COLOR_ALPHA_DISABLE;- case DRM_MODE_BLEND_PREMULTI: return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
- case DRM_MODE_BLEND_COVERAGE:
return PLANE_COLOR_ALPHA_HW_PREMULTIPLY;
return PLANE_COLOR_ALPHA_DISABLE;
MISSING_CASE(plane_state->base.pixel_blend_mode);return 0;}
@@ -3595,7 +3619,7 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state, plane_ctl = PLANE_CTL_ENABLE;
if (INTEL_GEN(dev_priv) < 10 && !IS_GEMINILAKE(dev_priv)) {
plane_ctl |= skl_plane_ctl_alpha(fb->format->format);
plane_ctl |= skl_plane_ctl_alpha(plane_state);@@ -3637,7 +3661,7 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, plane_color_ctl |= PLANE_COLOR_PIPE_CSC_ENABLE; } plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
- plane_color_ctl |= glk_plane_color_ctl_alpha(fb->format->format);
plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
if (intel_format_is_yuv(fb->format->format)) { if (fb->format->format == DRM_FORMAT_NV12) {
@@ -13623,7 +13647,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) DRM_MODE_ROTATE_0, supported_rotations);
- if (INTEL_GEN(dev_priv) >= 9)
- if (INTEL_GEN(dev_priv) >= 9) { drm_plane_create_color_properties(&primary->base, BIT(DRM_COLOR_YCBCR_BT601) | BIT(DRM_COLOR_YCBCR_BT709),
@@ -13632,6 +13656,13 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
drm_plane_create_alpha_property(&primary->base);drm_plane_create_blend_mode_property(&primary->base,BIT(DRM_MODE_BLEND_PIXEL_NONE) |BIT(DRM_MODE_BLEND_PREMULTI) |BIT(DRM_MODE_BLEND_COVERAGE));}
drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
return primary;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 2dc01be0c7cc..675dbe927a06 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -500,6 +500,8 @@ struct intel_plane_state { struct i915_vma *vma; unsigned long flags; #define PLANE_HAS_FENCE BIT(0) +#define PLANE_ALPHA_ENABLED BIT(1) +#define PLANE_ALPHA_NO_FBC BIT(2)
struct { u32 offset; diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 1f2f41e67dcd..20a2dba78cad 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -732,6 +732,10 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) fbc->no_fbc_reason = "framebuffer not tiled or fenced"; return false; }
- if (cache->flags & PLANE_ALPHA_NO_FBC) {
fbc->no_fbc_reason = "per pixel alpha blending enabled";return false;- } if (INTEL_GEN(dev_priv) <= 4 && !IS_G4X(dev_priv) && cache->plane.rotation != DRM_MODE_ROTATE_0) { fbc->no_fbc_reason = "rotation unsupported";
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0b1bd5de5192..4fcc7ce09a9c 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -254,6 +254,7 @@ skl_update_plane(struct intel_plane *plane, uint32_t y = plane_state->main.y; uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16; uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
u32 keymsk = 0, keymax = 0;
/* Sizes are 0 based */ src_w--;
@@ -267,10 +268,20 @@ skl_update_plane(struct intel_plane *plane,
if (key->flags) { I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), key->max_value);I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), key->channel_mask);
keymax = key->max_value & 0xffffff;keymsk |= key->channel_mask & 0x3ffffff;
What's up with |= vs. = here?
}
- keymax |= (plane_state->base.alpha >> 8) << PLANE_KEYMAX_ALPHA_SHIFT;
- if (plane_state->flags & PLANE_ALPHA_ENABLED ||
plane_state->base.fb->format->has_alpha)keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
Why are we checking the format has_alpha here? Isn't this about the constant alpha?
- I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax);
- I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), keymsk);
- I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (y << 16) | x); I915_WRITE_FW(PLANE_STRIDE(pipe, plane_id), stride); I915_WRITE_FW(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w);
@@ -1525,6 +1536,15 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv, DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
if (INTEL_GEN(dev_priv) >= 9) {
drm_plane_create_alpha_property(&intel_plane->base);drm_plane_create_blend_mode_property(&intel_plane->base,BIT(DRM_MODE_BLEND_PIXEL_NONE) |BIT(DRM_MODE_BLEND_PREMULTI) |BIT(DRM_MODE_BLEND_COVERAGE));}
drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
return intel_plane;
-- 2.17.1
igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev
Op 04-06-18 om 16:29 schreef Ville Syrjälä:
On Mon, Jun 04, 2018 at 03:50:13PM +0200, Maarten Lankhorst wrote:
Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
drivers/gpu/drm/i915/i915_reg.h | 2 + drivers/gpu/drm/i915/intel_display.c | 67 ++++++++++++++++++++-------- drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_fbc.c | 4 ++ drivers/gpu/drm/i915/intel_sprite.c | 24 +++++++++- 5 files changed, 79 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 327829cc61f8..23f0cb0fad0e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6425,8 +6425,10 @@ enum { #define _PLANE_KEYVAL_2_A 0x70294 #define _PLANE_KEYMSK_1_A 0x70198 #define _PLANE_KEYMSK_2_A 0x70298 +#define PLANE_KEYMSK_ALPHA_ENABLE (1 << 31) #define _PLANE_KEYMAX_1_A 0x701a0 #define _PLANE_KEYMAX_2_A 0x702a0 +#define PLANE_KEYMAX_ALPHA_SHIFT 24
PLANE_KEYMAX_ALPHA(x)
#define _PLANE_AUX_DIST_1_A 0x701c0 #define _PLANE_AUX_DIST_2_A 0x702c0 #define _PLANE_AUX_OFFSET_1_A 0x701c4 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d48256f89a50..8ddff09c3110 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3170,6 +3170,21 @@ int skl_check_plane_surface(const struct intel_crtc_state *crtc_state, return -EINVAL; }
- /* HW only has 8 bits pixel precision, disable plane if invisible */
- if (!(plane_state->base.alpha >> 8)) {
plane_state->base.visible = false;return 0;- }
- if (plane_state->base.alpha < 0xff00) {
plane_state->flags |= PLANE_ALPHA_ENABLED;/* FBC cannot be enabled with per pixel alpha */if (plane_state->base.pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE &&fb->format->has_alpha)plane_state->flags |= PLANE_ALPHA_NO_FBC;Why is this fbc per-pixel alpha check inside the constant alpha if block?
Also I'm not convinced by these plane state flags. They're not consistent with how we do everything else, so I would drop them.
The only reason I'm using it is because we already have PLANE_HAS_FENCE there. Because the FBC code already copies flags, it makes sense to re-use it rather than adding another field.
- }
- if (!plane_state->base.visible) return 0;
@@ -3496,30 +3511,39 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format) return 0; }
-/*
- XXX: For ARBG/ABGR formats we default to expecting scanout buffers
- to be already pre-multiplied. We need to add a knob (or a different
- DRM_FORMAT) for user-space to configure that.
- */
-static u32 skl_plane_ctl_alpha(uint32_t pixel_format) +static u32 skl_plane_ctl_alpha(const struct intel_plane_state *plane_state) {
- switch (pixel_format) {
- case DRM_FORMAT_ABGR8888:
- case DRM_FORMAT_ARGB8888:
- if (!plane_state->base.fb->format->has_alpha)
return PLANE_CTL_ALPHA_DISABLE;- switch (plane_state->base.pixel_blend_mode) {
- case DRM_MODE_BLEND_PIXEL_NONE:
return PLANE_CTL_ALPHA_DISABLE;- case DRM_MODE_BLEND_PREMULTI: return PLANE_CTL_ALPHA_SW_PREMULTIPLY;
- case DRM_MODE_BLEND_COVERAGE:
return PLANE_CTL_ALPHA_HW_PREMULTIPLY;
return PLANE_CTL_ALPHA_DISABLE;
MISSING_CASE(plane_state->base.pixel_blend_mode);return 0;}
-static u32 glk_plane_color_ctl_alpha(uint32_t pixel_format) +static u32 glk_plane_color_ctl_alpha(const struct intel_plane_state *plane_state) {
- switch (pixel_format) {
- case DRM_FORMAT_ABGR8888:
- case DRM_FORMAT_ARGB8888:
- if (!plane_state->base.fb->format->has_alpha)
return PLANE_COLOR_ALPHA_DISABLE;- switch (plane_state->base.pixel_blend_mode) {
- case DRM_MODE_BLEND_PIXEL_NONE:
return PLANE_COLOR_ALPHA_DISABLE;- case DRM_MODE_BLEND_PREMULTI: return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
- case DRM_MODE_BLEND_COVERAGE:
return PLANE_COLOR_ALPHA_HW_PREMULTIPLY;
return PLANE_COLOR_ALPHA_DISABLE;
MISSING_CASE(plane_state->base.pixel_blend_mode);return 0;}
@@ -3595,7 +3619,7 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state, plane_ctl = PLANE_CTL_ENABLE;
if (INTEL_GEN(dev_priv) < 10 && !IS_GEMINILAKE(dev_priv)) {
plane_ctl |= skl_plane_ctl_alpha(fb->format->format);
plane_ctl |= skl_plane_ctl_alpha(plane_state);@@ -3637,7 +3661,7 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, plane_color_ctl |= PLANE_COLOR_PIPE_CSC_ENABLE; } plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
- plane_color_ctl |= glk_plane_color_ctl_alpha(fb->format->format);
plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
if (intel_format_is_yuv(fb->format->format)) { if (fb->format->format == DRM_FORMAT_NV12) {
@@ -13623,7 +13647,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) DRM_MODE_ROTATE_0, supported_rotations);
- if (INTEL_GEN(dev_priv) >= 9)
- if (INTEL_GEN(dev_priv) >= 9) { drm_plane_create_color_properties(&primary->base, BIT(DRM_COLOR_YCBCR_BT601) | BIT(DRM_COLOR_YCBCR_BT709),
@@ -13632,6 +13656,13 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
drm_plane_create_alpha_property(&primary->base);drm_plane_create_blend_mode_property(&primary->base,BIT(DRM_MODE_BLEND_PIXEL_NONE) |BIT(DRM_MODE_BLEND_PREMULTI) |BIT(DRM_MODE_BLEND_COVERAGE));}
drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
return primary;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 2dc01be0c7cc..675dbe927a06 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -500,6 +500,8 @@ struct intel_plane_state { struct i915_vma *vma; unsigned long flags; #define PLANE_HAS_FENCE BIT(0) +#define PLANE_ALPHA_ENABLED BIT(1) +#define PLANE_ALPHA_NO_FBC BIT(2)
struct { u32 offset; diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 1f2f41e67dcd..20a2dba78cad 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -732,6 +732,10 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) fbc->no_fbc_reason = "framebuffer not tiled or fenced"; return false; }
- if (cache->flags & PLANE_ALPHA_NO_FBC) {
fbc->no_fbc_reason = "per pixel alpha blending enabled";return false;- } if (INTEL_GEN(dev_priv) <= 4 && !IS_G4X(dev_priv) && cache->plane.rotation != DRM_MODE_ROTATE_0) { fbc->no_fbc_reason = "rotation unsupported";
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0b1bd5de5192..4fcc7ce09a9c 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -254,6 +254,7 @@ skl_update_plane(struct intel_plane *plane, uint32_t y = plane_state->main.y; uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16; uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
u32 keymsk = 0, keymax = 0;
/* Sizes are 0 based */ src_w--;
@@ -267,10 +268,20 @@ skl_update_plane(struct intel_plane *plane,
if (key->flags) { I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), key->max_value);I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), key->channel_mask);
keymax = key->max_value & 0xffffff;keymsk |= key->channel_mask & 0x3ffffff;What's up with |= vs. = here?
Well I guess it can be just |= for both.
}
- keymax |= (plane_state->base.alpha >> 8) << PLANE_KEYMAX_ALPHA_SHIFT;
- if (plane_state->flags & PLANE_ALPHA_ENABLED ||
plane_state->base.fb->format->has_alpha)keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;Why are we checking the format has_alpha here? Isn't this about the constant alpha?
Hm looks wrong, must be just the initial check.
- I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax);
- I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), keymsk);
- I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (y << 16) | x); I915_WRITE_FW(PLANE_STRIDE(pipe, plane_id), stride); I915_WRITE_FW(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w);
@@ -1525,6 +1536,15 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv, DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
if (INTEL_GEN(dev_priv) >= 9) {
drm_plane_create_alpha_property(&intel_plane->base);drm_plane_create_blend_mode_property(&intel_plane->base,BIT(DRM_MODE_BLEND_PIXEL_NONE) |BIT(DRM_MODE_BLEND_PREMULTI) |BIT(DRM_MODE_BLEND_COVERAGE));}
drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
return intel_plane;
-- 2.17.1
igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev
On Mon, Jun 04, 2018 at 06:27:20PM +0200, Maarten Lankhorst wrote:
Op 04-06-18 om 16:29 schreef Ville Syrjälä:
On Mon, Jun 04, 2018 at 03:50:13PM +0200, Maarten Lankhorst wrote:
Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
drivers/gpu/drm/i915/i915_reg.h | 2 + drivers/gpu/drm/i915/intel_display.c | 67 ++++++++++++++++++++-------- drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_fbc.c | 4 ++ drivers/gpu/drm/i915/intel_sprite.c | 24 +++++++++- 5 files changed, 79 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 327829cc61f8..23f0cb0fad0e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6425,8 +6425,10 @@ enum { #define _PLANE_KEYVAL_2_A 0x70294 #define _PLANE_KEYMSK_1_A 0x70198 #define _PLANE_KEYMSK_2_A 0x70298 +#define PLANE_KEYMSK_ALPHA_ENABLE (1 << 31) #define _PLANE_KEYMAX_1_A 0x701a0 #define _PLANE_KEYMAX_2_A 0x702a0 +#define PLANE_KEYMAX_ALPHA_SHIFT 24
PLANE_KEYMAX_ALPHA(x)
#define _PLANE_AUX_DIST_1_A 0x701c0 #define _PLANE_AUX_DIST_2_A 0x702c0 #define _PLANE_AUX_OFFSET_1_A 0x701c4 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d48256f89a50..8ddff09c3110 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3170,6 +3170,21 @@ int skl_check_plane_surface(const struct intel_crtc_state *crtc_state, return -EINVAL; }
- /* HW only has 8 bits pixel precision, disable plane if invisible */
- if (!(plane_state->base.alpha >> 8)) {
plane_state->base.visible = false;return 0;- }
- if (plane_state->base.alpha < 0xff00) {
plane_state->flags |= PLANE_ALPHA_ENABLED;/* FBC cannot be enabled with per pixel alpha */if (plane_state->base.pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE &&fb->format->has_alpha)plane_state->flags |= PLANE_ALPHA_NO_FBC;Why is this fbc per-pixel alpha check inside the constant alpha if block?
Also I'm not convinced by these plane state flags. They're not consistent with how we do everything else, so I would drop them.
The only reason I'm using it is because we already have PLANE_HAS_FENCE there. Because the FBC code already copies flags, it makes sense to re-use it rather than adding another field.
The only reason for the has_fence is because we don't have that information elsewhere. I'd rather not duplicate things that are easily available in the state already.
Also, the fbc thing needs to be rewritten anyway. There is no reason to copy over all these things into the cache, and instead we should just look at the plane state during the plane atomic check and decide if fbc can or can't be used based on the new state.
- }
- if (!plane_state->base.visible) return 0;
@@ -3496,30 +3511,39 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format) return 0; }
-/*
- XXX: For ARBG/ABGR formats we default to expecting scanout buffers
- to be already pre-multiplied. We need to add a knob (or a different
- DRM_FORMAT) for user-space to configure that.
- */
-static u32 skl_plane_ctl_alpha(uint32_t pixel_format) +static u32 skl_plane_ctl_alpha(const struct intel_plane_state *plane_state) {
- switch (pixel_format) {
- case DRM_FORMAT_ABGR8888:
- case DRM_FORMAT_ARGB8888:
- if (!plane_state->base.fb->format->has_alpha)
return PLANE_CTL_ALPHA_DISABLE;- switch (plane_state->base.pixel_blend_mode) {
- case DRM_MODE_BLEND_PIXEL_NONE:
return PLANE_CTL_ALPHA_DISABLE;- case DRM_MODE_BLEND_PREMULTI: return PLANE_CTL_ALPHA_SW_PREMULTIPLY;
- case DRM_MODE_BLEND_COVERAGE:
return PLANE_CTL_ALPHA_HW_PREMULTIPLY;
return PLANE_CTL_ALPHA_DISABLE;
MISSING_CASE(plane_state->base.pixel_blend_mode);return 0;}
-static u32 glk_plane_color_ctl_alpha(uint32_t pixel_format) +static u32 glk_plane_color_ctl_alpha(const struct intel_plane_state *plane_state) {
- switch (pixel_format) {
- case DRM_FORMAT_ABGR8888:
- case DRM_FORMAT_ARGB8888:
- if (!plane_state->base.fb->format->has_alpha)
return PLANE_COLOR_ALPHA_DISABLE;- switch (plane_state->base.pixel_blend_mode) {
- case DRM_MODE_BLEND_PIXEL_NONE:
return PLANE_COLOR_ALPHA_DISABLE;- case DRM_MODE_BLEND_PREMULTI: return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
- case DRM_MODE_BLEND_COVERAGE:
return PLANE_COLOR_ALPHA_HW_PREMULTIPLY;
return PLANE_COLOR_ALPHA_DISABLE;
MISSING_CASE(plane_state->base.pixel_blend_mode);return 0;}
@@ -3595,7 +3619,7 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state, plane_ctl = PLANE_CTL_ENABLE;
if (INTEL_GEN(dev_priv) < 10 && !IS_GEMINILAKE(dev_priv)) {
plane_ctl |= skl_plane_ctl_alpha(fb->format->format);
plane_ctl |= skl_plane_ctl_alpha(plane_state);@@ -3637,7 +3661,7 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, plane_color_ctl |= PLANE_COLOR_PIPE_CSC_ENABLE; } plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
- plane_color_ctl |= glk_plane_color_ctl_alpha(fb->format->format);
plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
if (intel_format_is_yuv(fb->format->format)) { if (fb->format->format == DRM_FORMAT_NV12) {
@@ -13623,7 +13647,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) DRM_MODE_ROTATE_0, supported_rotations);
- if (INTEL_GEN(dev_priv) >= 9)
- if (INTEL_GEN(dev_priv) >= 9) { drm_plane_create_color_properties(&primary->base, BIT(DRM_COLOR_YCBCR_BT601) | BIT(DRM_COLOR_YCBCR_BT709),
@@ -13632,6 +13656,13 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
drm_plane_create_alpha_property(&primary->base);drm_plane_create_blend_mode_property(&primary->base,BIT(DRM_MODE_BLEND_PIXEL_NONE) |BIT(DRM_MODE_BLEND_PREMULTI) |BIT(DRM_MODE_BLEND_COVERAGE));}
drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
return primary;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 2dc01be0c7cc..675dbe927a06 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -500,6 +500,8 @@ struct intel_plane_state { struct i915_vma *vma; unsigned long flags; #define PLANE_HAS_FENCE BIT(0) +#define PLANE_ALPHA_ENABLED BIT(1) +#define PLANE_ALPHA_NO_FBC BIT(2)
struct { u32 offset; diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 1f2f41e67dcd..20a2dba78cad 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -732,6 +732,10 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) fbc->no_fbc_reason = "framebuffer not tiled or fenced"; return false; }
- if (cache->flags & PLANE_ALPHA_NO_FBC) {
fbc->no_fbc_reason = "per pixel alpha blending enabled";return false;- } if (INTEL_GEN(dev_priv) <= 4 && !IS_G4X(dev_priv) && cache->plane.rotation != DRM_MODE_ROTATE_0) { fbc->no_fbc_reason = "rotation unsupported";
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0b1bd5de5192..4fcc7ce09a9c 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -254,6 +254,7 @@ skl_update_plane(struct intel_plane *plane, uint32_t y = plane_state->main.y; uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16; uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
u32 keymsk = 0, keymax = 0;
/* Sizes are 0 based */ src_w--;
@@ -267,10 +268,20 @@ skl_update_plane(struct intel_plane *plane,
if (key->flags) { I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), key->max_value);I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), key->channel_mask);
keymax = key->max_value & 0xffffff;keymsk |= key->channel_mask & 0x3ffffff;What's up with |= vs. = here?
Well I guess it can be just |= for both.
}
- keymax |= (plane_state->base.alpha >> 8) << PLANE_KEYMAX_ALPHA_SHIFT;
- if (plane_state->flags & PLANE_ALPHA_ENABLED ||
plane_state->base.fb->format->has_alpha)keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;Why are we checking the format has_alpha here? Isn't this about the constant alpha?
Hm looks wrong, must be just the initial check.
- I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax);
- I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), keymsk);
- I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (y << 16) | x); I915_WRITE_FW(PLANE_STRIDE(pipe, plane_id), stride); I915_WRITE_FW(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w);
@@ -1525,6 +1536,15 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv, DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
if (INTEL_GEN(dev_priv) >= 9) {
drm_plane_create_alpha_property(&intel_plane->base);drm_plane_create_blend_mode_property(&intel_plane->base,BIT(DRM_MODE_BLEND_PIXEL_NONE) |BIT(DRM_MODE_BLEND_PREMULTI) |BIT(DRM_MODE_BLEND_COVERAGE));}
drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
return intel_plane;
-- 2.17.1
igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev
1. Check the pixel blending mode and plane alpha value when do the plane_check. Mali DP supports blending the current plane with the background either based on the pixel alpha blending mode or by using the layer's alpha value, but not both at the same time. If both case, plane_check will return failed.
2. Set the HW when doing plane_update accordingly. If plane alpha is the 0xffff, set the pixel blending bits accordingly. If not we'd set ALPHA bit as zero and layer alpha value.
Signed-off-by: Lowry Li lowry.li@arm.com --- drivers/gpu/drm/arm/malidp_planes.c | 76 +++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c index 7a44897..daa3f4f 100644 --- a/drivers/gpu/drm/arm/malidp_planes.c +++ b/drivers/gpu/drm/arm/malidp_planes.c @@ -35,6 +35,7 @@ #define LAYER_COMP_MASK (0x3 << 12) #define LAYER_COMP_PIXEL (0x3 << 12) #define LAYER_COMP_PLANE (0x2 << 12) +#define LAYER_PMUL_ENABLE (0x1 << 14) #define LAYER_ALPHA_OFFSET (16) #define LAYER_ALPHA_MASK (0xff) #define LAYER_ALPHA(x) (((x) & LAYER_ALPHA_MASK) << LAYER_ALPHA_OFFSET) @@ -182,6 +183,7 @@ static int malidp_de_plane_check(struct drm_plane *plane, struct malidp_plane_state *ms = to_malidp_plane_state(state); bool rotated = state->rotation & MALIDP_ROTATED_MASK; struct drm_framebuffer *fb; + u16 pixel_alpha = state->pixel_blend_mode; int i, ret;
if (!state->crtc || !state->fb) @@ -244,6 +246,11 @@ static int malidp_de_plane_check(struct drm_plane *plane, ms->rotmem_size = val; }
+ /* HW can't support plane + pixel blending */ + if ((state->alpha != DRM_BLEND_ALPHA_OPAQUE) && + (pixel_alpha != DRM_MODE_BLEND_PIXEL_NONE)) + return -EINVAL; + return 0; }
@@ -325,31 +332,33 @@ static void malidp_de_plane_update(struct drm_plane *plane, { struct malidp_plane *mp; struct malidp_plane_state *ms = to_malidp_plane_state(plane->state); + struct drm_plane_state *state = plane->state; + u16 pixel_alpha = state->pixel_blend_mode; + u8 plane_alpha = state->alpha >> 8; u32 src_w, src_h, dest_w, dest_h, val; int i; - bool format_has_alpha = plane->state->fb->format->has_alpha;
mp = to_malidp_plane(plane);
/* convert src values from Q16 fixed point to integer */ - src_w = plane->state->src_w >> 16; - src_h = plane->state->src_h >> 16; - dest_w = plane->state->crtc_w; - dest_h = plane->state->crtc_h; + src_w = state->src_w >> 16; + src_h = state->src_h >> 16; + dest_w = state->crtc_w; + dest_h = state->crtc_h;
malidp_hw_write(mp->hwdev, ms->format, mp->layer->base);
for (i = 0; i < ms->n_planes; i++) { /* calculate the offset for the layer's plane registers */ u16 ptr = mp->layer->ptr + (i << 4); - dma_addr_t fb_addr = drm_fb_cma_get_gem_addr(plane->state->fb, - plane->state, i); + dma_addr_t fb_addr = drm_fb_cma_get_gem_addr(state->fb, + state, i);
malidp_hw_write(mp->hwdev, lower_32_bits(fb_addr), ptr); malidp_hw_write(mp->hwdev, upper_32_bits(fb_addr), ptr + 4); } malidp_de_set_plane_pitches(mp, ms->n_planes, - plane->state->fb->pitches); + state->fb->pitches);
if ((plane->state->color_encoding != old_state->color_encoding) || (plane->state->color_range != old_state->color_range)) @@ -362,8 +371,8 @@ static void malidp_de_plane_update(struct drm_plane *plane, malidp_hw_write(mp->hwdev, LAYER_H_VAL(dest_w) | LAYER_V_VAL(dest_h), mp->layer->base + MALIDP_LAYER_COMP_SIZE);
- malidp_hw_write(mp->hwdev, LAYER_H_VAL(plane->state->crtc_x) | - LAYER_V_VAL(plane->state->crtc_y), + malidp_hw_write(mp->hwdev, LAYER_H_VAL(state->crtc_x) | + LAYER_V_VAL(state->crtc_y), mp->layer->base + MALIDP_LAYER_OFFSET);
if (mp->layer->id == DE_SMART) @@ -376,38 +385,35 @@ static void malidp_de_plane_update(struct drm_plane *plane, val &= ~LAYER_ROT_MASK;
/* setup the rotation and axis flip bits */ - if (plane->state->rotation & DRM_MODE_ROTATE_MASK) + if (state->rotation & DRM_MODE_ROTATE_MASK) val |= ilog2(plane->state->rotation & DRM_MODE_ROTATE_MASK) << LAYER_ROT_OFFSET; - if (plane->state->rotation & DRM_MODE_REFLECT_X) + if (state->rotation & DRM_MODE_REFLECT_X) val |= LAYER_H_FLIP; - if (plane->state->rotation & DRM_MODE_REFLECT_Y) + if (state->rotation & DRM_MODE_REFLECT_Y) val |= LAYER_V_FLIP;
- val &= ~LAYER_COMP_MASK; - if (format_has_alpha) { - - /* - * always enable pixel alpha blending until we have a way - * to change blend modes - */ - val |= LAYER_COMP_PIXEL; - } else { - - /* - * do not enable pixel alpha blending as the color channel - * does not have any alpha information - */ - val |= LAYER_COMP_PLANE; - - /* Set layer alpha coefficient to 0xff ie fully opaque */ + val &= ~(LAYER_COMP_MASK | LAYER_PMUL_ENABLE); + + if (state->alpha != DRM_BLEND_ALPHA_OPAQUE) { + val |= LAYER_COMP_PLANE | LAYER_ALPHA(plane_alpha); + } else if (state->fb->format->has_alpha) { + /* We only care about blend mode if the format has alpha */ + switch (pixel_alpha) { + case DRM_MODE_BLEND_PREMULTI: + val |= LAYER_COMP_PIXEL | LAYER_PMUL_ENABLE; + break; + case DRM_MODE_BLEND_COVERAGE: + val |= LAYER_COMP_PIXEL; + break; + } val |= LAYER_ALPHA(0xff); }
val &= ~LAYER_FLOWCFG(LAYER_FLOWCFG_MASK); - if (plane->state->crtc) { + if (state->crtc) { struct malidp_crtc_state *m = - to_malidp_crtc_state(plane->state->crtc->state); + to_malidp_crtc_state(state->crtc->state);
if (m->scaler_config.scale_enable && m->scaler_config.plane_src_id == mp->layer->id) @@ -446,6 +452,9 @@ int malidp_de_planes_init(struct drm_device *drm) unsigned long crtcs = 1 << drm->mode_config.num_crtc; unsigned long flags = DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_180 | DRM_MODE_ROTATE_270 | DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y; + unsigned int blend_caps = BIT(DRM_MODE_BLEND_PIXEL_NONE) | + BIT(DRM_MODE_BLEND_PREMULTI) | + BIT(DRM_MODE_BLEND_COVERAGE); u32 *formats; int ret, i, j, n;
@@ -498,6 +507,9 @@ int malidp_de_planes_init(struct drm_device *drm) malidp_hw_write(malidp->dev, MALIDP_ALPHA_LUT, plane->layer->base + MALIDP_LAYER_COMPOSE);
+ drm_plane_create_alpha_property(&plane->base); + drm_plane_create_blend_mode_property(&plane->base, blend_caps); + /* Attach the YUV->RGB property only to video layers */ if (id & (DE_VIDEO1 | DE_VIDEO2)) { /* default encoding for YUV->RGB is BT601 NARROW */
dri-devel@lists.freedesktop.org
-
Emil Velikov -
Lowry Li -
Maarten Lankhorst -
Ville Syrjälä