On Mon, Jun 04, 2018 at 02:49:26PM +0100, Emil Velikov wrote:
> On 1 June 2018 at 13:41, Lowry Li <lowry.li@xxxxxxx> 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@xxxxxxx>
> > ---
> > 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.
> > + *
>
> 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@xxxxxxxxxxxxx>
>
> -Emil
Hi Emil,
Thanks a lot for your review and will change that.
--
Regards,
Lowry
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
