| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20180814092858.GJ21634@phenom.ffwll.local>
Date: Tue, 14 Aug 2018 11:28:58 +0200
From: Daniel Vetter <daniel@...ll.ch>
To: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
Cc: Lowry Li <lowry.li@....com>, David Airlie <airlied@...ux.ie>,
Emil Velikov <emil.l.velikov@...il.com>,
Liviu Dudau <liviu.dudau@....com>,
"Linux-Kernel@...r. Kernel. Org" <linux-kernel@...r.kernel.org>,
ML dri-devel <dri-devel@...ts.freedesktop.org>,
Mali DP Maintainers <malidp@...s.arm.com>,
"Vetter, Daniel" <daniel.vetter@...el.com>, nd@....com
Subject: Re: [PATCH v3 1/2] drm: Add per-plane pixel blend mode property
On Tue, Aug 14, 2018 at 11:15:43AM +0200, Maarten Lankhorst wrote:
> Op 14-08-18 om 05:11 schreef Lowry Li:
> > On Mon, Aug 13, 2018 at 12:49:13PM +0200, Maarten Lankhorst wrote:
> >> Op 05-06-18 om 11:07 schreef Lowry Li:
> >>> On Mon, Jun 04, 2018 at 02:49:26PM +0100, Emil Velikov wrote:
> >>>> On 1 June 2018 at 13:41, Lowry Li <lowry.li@....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@....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.
> >>>>> + *
> >>>> 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@...labora.com>
> >>>>
> >>>> -Emil
> >>> Hi Emil,
> >>>
> >>> Thanks a lot for your review and will change that.
> >> Ping? Any updates?
> >>
> >> ~Maarten
> > Hi Maarten,
> >
> > So far the merge request on user space patch has been verified by John
> > Stultz. The comments from Sean has been fixed and now we are waiting the
> > reviewed-tag from Sean. Once it merged, we'll send a new kernel patch to
> > review.
> >
> Hey,
>
> In general the kernel patch should be merged first, because userspace cannot rely on the behavior until the next -rc1 that integrates it.
> However, in order to merge the kernel patch, a userspace proof of concept for validation is needed first. Since you're close to merging,
> I would say the userspace requirement is satisfied. It's safe to send the kernel patch. :)
Quick correction: Generally merged into -next is considered good enough.
-Daniel
>
> ~Maarten
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Powered by blists - more mailing lists