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@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.
>> 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. :)
~Maarten
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
