| Message ID | 20180530132755.GA27705@e107564-lin.cambridge.arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, May 30, 2018 at 02:27:55PM +0100, Brian Starkey wrote: > Hi Lowry, > > On Wed, May 30, 2018 at 07:23:53PM +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. > > > >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> > > With or without the kerneldoc tweaks I've suggested below, this patch > is: > > Reviewed-by: Brian Starkey <brian.starkey@arm.com> > Hi Brian, Thanks for the patch and comments. Will update a new patchset with this. Regards, Lowry > >--- > >drivers/gpu/drm/drm_atomic.c | 4 ++ > >drivers/gpu/drm/drm_atomic_helper.c | 1 + > >drivers/gpu/drm/drm_blend.c | 110 ++++++++++++++++++++++++++++++++++++ > >include/drm/drm_blend.h | 6 ++ > >include/drm/drm_plane.h | 6 ++ > >5 files changed, 127 insertions(+) > > > >diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > >index 7d25c42..467f8de 100644 > >--- a/drivers/gpu/drm/drm_atomic.c > >+++ b/drivers/gpu/drm/drm_atomic.c > >@@ -783,6 +783,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, > > state->src_w = val; > > } else if (property == config->prop_src_h) { > > state->src_h = val; > >+ } else if (property == plane->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; > >@@ -848,6 +850,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, > > *val = state->src_w; > > } else if (property == config->prop_src_h) { > > *val = state->src_h; > >+ } 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 c356545..ddbd0d2 100644 > >--- a/drivers/gpu/drm/drm_atomic_helper.c > >+++ b/drivers/gpu/drm/drm_atomic_helper.c > >@@ -3484,6 +3484,7 @@ void drm_atomic_helper_plane_reset(struct drm_plane *plane) > > if (plane->state) { > > plane->state->plane = plane; > > plane->state->rotation = DRM_MODE_ROTATE_0; > >+ 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 5a81e1b..4ac45da 100644 > >--- a/drivers/gpu/drm/drm_blend.c > >+++ b/drivers/gpu/drm/drm_blend.c > >@@ -100,6 +100,41 @@ > > * 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(note that the fg.rgb or bg.rgb notation > >+ * means each of the R, G or B channels for the foreground and background > >+ * colors, respectively): > >+ * > >+ * "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 > >+ * > >+ * fg.rgb: Each of the RGB component values from the plane's pixel > >+ * fg.alpha: Alpha component value from the plane's pixel > >+ * bg.rgb: Each of the RGB component values from the background > >+ * plane_alpha: Plane alpha value set by the plane alpha property (if > >+ * applicable). > >+ * > >+ * This property has no effect on formats with no pixel alpha, as fg.alpha > >+ * is assumed to be 1.0. If the plane does not expose the "alpha" property, > >+ * then plane_alpha is assumed to be 1.0, otherwise, it is the value of the > >+ * "alpha" property. > >+ * > > I did a little fiddling to make this render nicely in the > documentation output. If you like it, you can apply the hunk below: > > -- >8 -- > > diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c > index 714b0602127d..7b5c3307d4c1 100644 > --- a/drivers/gpu/drm/drm_blend.c > +++ b/drivers/gpu/drm/drm_blend.c > @@ -113,34 +113,45 @@ > * how the pixels from the current plane are composited with the > * background. > * > - * Three alpha blending equations(note that the fg.rgb or bg.rgb notation > - * means each of the R, G or B channels for the foreground and background > - * colors, respectively): > - * > - * "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 > - * > - * fg.rgb: Each of the RGB component values from the plane's pixel > - * fg.alpha: Alpha component value from the plane's pixel > - * bg.rgb: Each of the RGB component values from the background > - * plane_alpha: Plane alpha value set by the plane alpha property (if > - * applicable). > - * > - * This property has no effect on formats with no pixel alpha, as fg.alpha > - * is assumed to be 1.0. If the plane does not expose the "alpha" property, > - * then plane_alpha is assumed to be 1.0, otherwise, it is the value of the > - * "alpha" property. > + * 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 > > -- >8 -- > > > * 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). > >@@ -409,3 +444,78 @@ 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) > >+ * > >+ * 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. > > Similar thing here - if you put the descriptions (after the ":") on a > new line, and indent them one more level, then it renders much more > nicely: > > * "None": > * Blend formula that ignores the pixel alpha. > > Thanks, > -Brian > > >+ * > >+ * 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 1760602..2966c0d 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; > >@@ -49,4 +53,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 f7bf4a4..447ebe7 100644 > >--- a/include/drm/drm_plane.h > >+++ b/include/drm/drm_plane.h > >@@ -43,6 +43,8 @@ > > * plane (in 16.16) > > * @src_w: width of visible portion of plane (in 16.16) > > * @src_h: height of visible portion of plane (in 16.16) > >+ * @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 > >@@ -106,6 +108,8 @@ struct drm_plane_state { > > uint32_t src_x, src_y; > > uint32_t src_h, src_w; > > > >+ uint16_t pixel_blend_mode; > >+ > > /* Plane rotation */ > > unsigned int rotation; > > > >@@ -498,6 +502,7 @@ enum drm_plane_type { > > * @type: type of plane (overlay, primary, cursor) > > * @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 { > >@@ -573,6 +578,7 @@ struct drm_plane { > > > > struct drm_property *zpos_property; > > struct drm_property *rotation_property; > >+ struct drm_property *blend_mode_property; > > > > /** > > * @color_encoding_property: > >-- > >1.9.1 > >
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index 714b0602127d..7b5c3307d4c1 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -113,34 +113,45 @@ * how the pixels from the current plane are composited with the * background. * - * Three alpha blending equations(note that the fg.rgb or bg.rgb notation - * means each of the R, G or B channels for the foreground and background - * colors, respectively): - * - * "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 - * - * fg.rgb: Each of the RGB component values from the plane's pixel - * fg.alpha: Alpha component value from the plane's pixel - * bg.rgb: Each of the RGB component values from the background - * plane_alpha: Plane alpha value set by the plane alpha property (if - * applicable). - * - * This property has no effect on formats with no pixel alpha, as fg.alpha - * is assumed to be 1.0. If the plane does not expose the "alpha" property, - * then plane_alpha is assumed to be 1.0, otherwise, it is the value of the - * "alpha" property. + * 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