[<prev] [next>] [day] [month] [year] [list]
Message-ID: <325674b0-d100-4e38-3963-0e458e6dec80@linux.intel.com>
Date: Mon, 13 Aug 2018 12:49:13 +0200
From: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
To: Lowry Li <lowry.li@....com>,
Emil Velikov <emil.l.velikov@...il.com>
Cc: Liviu Dudau <liviu.dudau@....com>,
Gustavo Padovan <gustavo@...ovan.org>,
"Vetter, Daniel" <daniel.vetter@...el.com>,
Sean Paul <seanpaul@...omium.org>,
David Airlie <airlied@...ux.ie>,
Ville Syrjälä <ville.syrjala@...ux.intel.com>,
ML dri-devel <dri-devel@...ts.freedesktop.org>,
"Linux-Kernel@...r. Kernel. Org" <linux-kernel@...r.kernel.org>,
Brian Starkey <brian.starkey@....com>,
Mali DP Maintainers <malidp@...s.arm.com>, nd@....com
Subject: Re: [PATCH v3 1/2] drm: Add per-plane pixel blend mode property
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
Powered by blists - more mailing lists