From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: dri-devel@lists.freedesktop.org,
linux-samsung-soc@vger.kernel.org,
Inki Dae <inki.dae@samsung.com>, Daniel Vetter <daniel@ffwll.ch>,
Joonyoung Shim <jy0922.shim@samsung.com>,
Seung-Woo Kim <sw0312.kim@samsung.com>,
Andrzej Hajda <a.hajda@samsung.com>,
Krzysztof Kozlowski <k.kozlowski@samsung.com>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Tobias Jakobi <tjakobi@math.uni-bielefeld.de>,
Gustavo Padovan <gustavo@padovan.org>,
Benjamin Gaignard <benjamin.gaignard@linaro.org>,
vincent.abriou@st.com, fabien.dessenne@st.com
Subject: Re: [PATCH v5 4/5] drm: add generic blending related properties
Date: Mon, 29 Feb 2016 17:23:12 +0200 [thread overview]
Message-ID: <20160229152312.GY15993@intel.com> (raw)
In-Reply-To: <1453905883-6807-5-git-send-email-m.szyprowski@samsung.com>
On Wed, Jan 27, 2016 at 03:44:42PM +0100, Marek Szyprowski wrote:
> This patch adds code and documentation for the following blending
> related properties: 'alpha', 'blending' and 'alpha_premult'.
>
> 'alpha' property defines plane's transparency used for some blending
> modes.
>
> 'alpha_premult' property defines if RGB pixel data in the framebuffer
> contains values pre-multiplied by its alpha value or full range RGB
> values (0-255), which needs to be multiplied by pixel alpha value for
> blending operation.
>
> 'blending' property selects blending mode - by selecting one of the
> predefined values, user can select a pair of factors, which will be
> used in the generic blending equation. For more information, see
> the kerneldoc for the added code.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> Documentation/DocBook/gpu.tmpl | 47 +++++++++++++++++-
> drivers/gpu/drm/drm_atomic.c | 12 +++++
> drivers/gpu/drm/drm_blend.c | 110 +++++++++++++++++++++++++++++++++++++++++
> include/drm/drm_crtc.h | 15 ++++++
> include/uapi/drm/drm_mode.h | 101 +++++++++++++++++++++++++++++++++++++
> 5 files changed, 283 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index ae7d913adf60..e5197368e68f 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -1110,6 +1110,22 @@ int max_width, max_height;</synopsis>
> </sect2>
> </sect1>
>
> + <!-- Internals: plane blending -->
> +
> + <sect1 id="drm-plane-blending">
> + <title>Plane blending</title>
> +
> + <sect2>
> + <title>Plane Blending Overview</title>
> +!Pinclude/uapi/drm/drm_mode.h Generic plane blending operation
> + </sect2>
> + <sect2>
> + <title>Blending Function Reference</title>
> +!Iinclude/uapi/drm/drm_mode.h
> +!Edrivers/gpu/drm/drm_blend.c
> + </sect2>
> + </sect1>
> +
> <!-- Internals: kms initialization and cleanup -->
>
> <sect1 id="drm-kms-init">
> @@ -1816,7 +1832,7 @@ void intel_crt_init(struct drm_device *dev)
> <td valign="top" >Description/Restrictions</td>
> </tr>
> <tr>
> - <td rowspan="38" valign="top" >DRM</td>
> + <td rowspan="41" valign="top" >DRM</td>
> <td valign="top" >Generic</td>
> <td valign="top" >“rotation”</td>
> <td valign="top" >BITMASK</td>
> @@ -2068,7 +2084,7 @@ void intel_crt_init(struct drm_device *dev)
> <td valign="top" >property to suggest an Y offset for a connector</td>
> </tr>
> <tr>
> - <td rowspan="4" valign="top" >Optional</td>
> + <td rowspan="7" valign="top" >Optional</td>
> <td valign="top" >“scaling mode”</td>
> <td valign="top" >ENUM</td>
> <td valign="top" >{ "None", "Full", "Center", "Full aspect" }</td>
> @@ -2102,6 +2118,33 @@ void intel_crt_init(struct drm_device *dev)
> planes' order. Exact value range is driver dependent.</td>
> </tr>
> <tr>
> + <td valign="top" > "alpha" </td>
> + <td valign="top" >RANGE</td>
> + <td valign="top" >Min=0, Max= driver dependent</td>
> + <td valign="top" >Plane</td>
> + <td valign="top" >Plane's alpha value (transparency) for blending operation. Used in some blending modes. </td>
> + </tr>
> + <tr>
> + <td valign="top" > "alpha_premult" </td>
> + <td valign="top" >BOOL</td>
> + <td valign="top" >Min=0, Max=1</td>
> + <td valign="top" >Plane</td>
> + <td valign="top" >Indicates the range of the RGB data of the framebuffer attached to the given plane.
> + When enabled, RGB values fits the range from 0 to pixel's alpha value. When disabled, RGB
> + values are from 0 to 255 range and during blending operations they will be multiplied by
> + the pixel's alpha value first before computing result of blending equations. Value of this
> + property is used in some blending modes and only when user attaches framebuffer with pixel
> + format, which contains non-binary alpha channel.</td>
I have a hard to figure out what that is saying. The fact that it goes
on talking about ranges of the RGB values is IMO just confusing.
I'm pretty sure I've suggested it before, but I would just make this
some kind of 'alpha_premultiply' which when enabled will perform the
premultiplication, and when disabled it won't.
Description could IMO be something as simple as
"Premultiply the source RGB values with the source alpha before blending?"
> + </tr>
> + <tr>
> + <td valign="top" > "blending" </td>
> + <td valign="top" >ENUM</td>
> + <td valign="top" >{ "disabled", "pixel-alpha", "const-alpha" "pixel-const-alpha" }</td>
> + <td valign="top" >Plane</td>
> + <td valign="top" >Selects algorithm used in plane blending operation. For more information, see
> + DRM_BLEND_* symbolic constant documentation. </td>
> + </tr>
> + <tr>
> <td rowspan="20" valign="top" >i915</td>
> <td rowspan="2" valign="top" >Generic</td>
> <td valign="top" >"Broadcast RGB"</td>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index a19201efb7d1..a85da144fd95 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -632,6 +632,12 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
> state->rotation = val;
> } else if (property == config->zpos_property) {
> state->zpos = val;
> + } else if (property == config->alpha_property) {
> + state->alpha = val;
> + } else if (property == config->alpha_premult_property) {
> + state->alpha_premult = val;
> + } else if (property == config->blending_property) {
> + state->blending = val;
> } else if (plane->funcs->atomic_set_property) {
> return plane->funcs->atomic_set_property(plane, state,
> property, val);
> @@ -690,6 +696,12 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> *val = state->rotation;
> } else if (property == config->zpos_property) {
> *val = state->zpos;
> + } else if (property == config->alpha_property) {
> + *val = state->alpha;
> + } else if (property == config->alpha_premult_property) {
> + *val = state->alpha_premult;
> + } else if (property == config->blending_property) {
> + *val = state->blending;
> } else if (plane->funcs->atomic_get_property) {
> return plane->funcs->atomic_get_property(plane, state, property, val);
> } else {
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index cdcb647c8ed9..eb7996a848df 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -99,6 +99,116 @@ int drm_mode_create_zpos_immutable_property(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_mode_create_zpos_immutable_property);
>
> +/**
> + * drm_mode_create_blending_property - create generic blending property
> + * @dev: DRM device
> + * @supported_blendings: array of supported blending modes
> + * @supported_blendings_count: size of @supported_blendings array
> + *
> + * This function initializes generic blending property to selected subset
> + * of supported blending modes. Drivers can then attach this property to
> + * planes to let userspace to configure different blending modes. For more
> + * information on blending modes, see DRM_BLEND_* defines.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_mode_create_blending_property(struct drm_device *dev,
> + unsigned int *supported_blendings,
> + unsigned int supported_blendings_count)
> +{
> + struct drm_property *prop;
> + static const struct drm_prop_enum_list all_values[] = {
> + { DRM_BLEND_DISABLED, "disabled" },
> + { DRM_BLEND_PIXEL_ALPHA, "pixel-alpha" },
> + { DRM_BLEND_CONST_ALPHA, "const-alpha" },
> + { DRM_BLEND_PIXEL_CONST_ALPHA, "pixel-const-alpha" },
> + };
Why do we need these names for the modes? Can't we just use the real
GLesque things? They would at least tell you waht it's going to instead
of having to guess. Also these are clearly missing needed stuff
(eg. src=1.0 dst=1.0-src alpha)
> + struct drm_prop_enum_list *values;
> + int i, j;
> +
> + values = kmalloc(supported_blendings_count * sizeof(*values),
> + GFP_KERNEL);
> + if (!values)
> + return -ENOMEM;
> +
> + for (i = 0; i < supported_blendings_count; i++) {
> + for (j = 0; j < ARRAY_SIZE(all_values); j++) {
> + if (all_values[j].type == supported_blendings[i]) {
> + values[i].type = supported_blendings[i];
> + values[i].name = all_values[j].name;
> + break;
> + }
> + }
> + }
> +
> + prop = drm_property_create_enum(dev, 0, "blending", values,
> + supported_blendings_count);
> + kfree(values);
> +
> + if (!prop)
> + return -ENOMEM;
> +
> + dev->mode_config.blending_property = prop;
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_mode_create_blending_property);
> +
> +/**
> + * drm_mode_create_alpha_property - create plane alpha property
> + * @dev: DRM device
> + * @max: maximal possible value of alpha property
> + *
> + * This function initializes generic plane's alpha property. It's value is used
> + * for blending operations, depending on selected blending mode. For more
> + * information, see DRM_BLEND_* modes and its documentation. Maximum alpha value
> + * is determined by the driver.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_mode_create_alpha_property(struct drm_device *dev, unsigned int max)
> +{
> + struct drm_property *prop;
> +
> + prop = drm_property_create_range(dev, 0, "alpha", 0, max);
> + if (!prop)
> + return -ENOMEM;
> +
> + dev->mode_config.alpha_property = prop;
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_mode_create_alpha_property);
> +
> +/**
> + * drm_mode_create_alpha_premult_property - create muttable zpos property
> + * @dev: DRM device
> + *
> + * This function initializes generic plane's alpha pre-multiplication property.
> + * This property indicates the range of the RGB data of the framebuffer attached
> + * to the given plane. When enabled, RGB values fits the range from 0 to pixel's
> + * alpha value. When disabled, RGB values are from 0 to 255 range and during
> + * blending operations they will be multiplied by the pixel's alpha value first
> + * before computing result of blending equations. Value of this property is used
> + * only when user attaches framebuffer with pixel format, which contains
> + * non-binary alpha channel (for example: DRM_FORMAT_ARGB8888).
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_mode_create_alpha_premult_property(struct drm_device *dev)
> +{
> + struct drm_property *prop;
> +
> + prop = drm_property_create_bool(dev, 0, "alpha_premult");
> + if (!prop)
> + return -ENOMEM;
> +
> + dev->mode_config.alpha_premult_property = prop;
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_mode_create_alpha_premult_property);
> +
> static int drm_atomic_state_zpos_cmp(const void *a, const void *b)
> {
> const struct drm_plane_state *sa = *(struct drm_plane_state **)a;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 702f4f28f580..47ae79483a19 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1276,6 +1276,11 @@ struct drm_plane_state {
> unsigned int zpos;
> unsigned int normalized_zpos;
>
> + /* Plane blending */
> + unsigned int blending;
> + unsigned int alpha;
> + bool alpha_premult;
> +
> struct drm_atomic_state *state;
> };
>
> @@ -2098,6 +2103,9 @@ struct drm_mode_config {
> struct drm_property *rotation_property;
> struct drm_property *zpos_property;
> struct drm_property *zpos_immutable_property;
> + struct drm_property *blending_property;
> + struct drm_property *alpha_property;
> + struct drm_property *alpha_premult_property;
> struct drm_property *prop_src_x;
> struct drm_property *prop_src_y;
> struct drm_property *prop_src_w;
> @@ -2505,6 +2513,13 @@ extern int drm_mode_create_zpos_immutable_property(struct drm_device *dev,
> unsigned int min,
> unsigned int max);
>
> +extern int drm_mode_create_blending_property(struct drm_device *dev,
> + unsigned int *supported_blendings,
> + unsigned int supported_blendings_count);
> +extern int drm_mode_create_alpha_property(struct drm_device *dev,
> + unsigned int max);
> +extern int drm_mode_create_alpha_premult_property(struct drm_device *dev);
> +
> /* Helpers */
>
> static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 50adb46204c2..6da7076a3856 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -596,4 +596,105 @@ struct drm_mode_destroy_blob {
> __u32 blob_id;
> };
>
> +/**
> + * DOC: Generic plane blending operation
> + *
> + * Planes attached to one CRTC are blended by the hardware to produce the final
> + * result on the display. The order of planes during blending operation is
> + * determined by 'zpos' property. When order of plane for blending operation is
> + * determined, then framebuffers of all planes are placed on the display area
> + * according to the configured position and target size.
> + *
> + * Then one of the selected blending procedure is applied to each pixel on the
> + * display area to compute the final result of the blending. The blending
> + * procedure is similar to the well known glBlendFunc() API.
> + *
> + * The generic equation for blending is:
> + * outputRGB = sF * sRGB + dF * dRGB
> + *
> + * @sRGB: are RGB values of blended plane
> + *
> + * @dRGB: are RGB values of plane's background (result of previous blending or
> + * RGB pixel values of deeper planes).
> + *
> + * @sF and @dF: are one of DRM_FACTOR_* symbolic constants.
> + *
> + * Blending mode then defined as DRM_BLEND(sF, dF), which selects respective
> + * factors for the above equation. For more information, see drm_blend_mode
> + * enum.
> + */
> +
> +/**
> + * enum drm_blend_factor - factors for defining plane blending formula
> + * @DRM_FACTOR_ZERO: constant zero
> + * @DRM_FACTOR_ONE: constant one
> + * @DRM_FACTOR_SRC_ALPHA: pixel alpha value
> + * @DRM_FACTOR_ONE_MINUS_SRC_ALPHA: 1 - DRM_FACTOR_SRC_ALPHA
> + * @DRM_FACTOR_CONST_ALPHA: constant alpha (plane's property)
> + * @DRM_FACTOR_ONE_MINUS_CONST_ALPHA: 1 - DRM_FACTOR_CONST_ALPHA
> + * @DRM_FACTOR_SRC_CONST_ALPHA: pixel alpha value multiplied by plane's
> + * constant alpha
> + * @DRM_FACTOR_ONE_MINUS_SRC_CONST_ALPHA: 1 - DRM_FACTOR_SRC_CONST_ALPHA
> + *
> + * Values of this enum are used to define plane blending formula. Two factors
> + * have to be selected - one for source plane and one for destination
> + * (background).
> + */
> +enum drm_blend_factor {
> + DRM_FACTOR_ZERO,
> + DRM_FACTOR_ONE,
> + DRM_FACTOR_SRC_ALPHA,
> + DRM_FACTOR_ONE_MINUS_SRC_ALPHA,
> + DRM_FACTOR_CONST_ALPHA,
> + DRM_FACTOR_ONE_MINUS_CONST_ALPHA,
> + DRM_FACTOR_SRC_CONST_ALPHA,
> + DRM_FACTOR_ONE_MINUS_SRC_CONST_ALPHA,
> +};
> +
> +#define DRM_BLEND(s, d) ((d << 16) | (s))
> +
> +/**
> + * enum drm_blend_mode - predefined blending modes
> + * @DRM_BLEND_S_ONE_D_ZERO: no transparency; per-pixel and plane's alpha is
> + * ignored regardless of the selected pixel format.
> + * @DRM_BLEND_S_SRC_ALPHA_D_ONE_MINUS_SRC_ALPHA: blending with per-pixel alpha;
> + * plane's alpha is ignored, aplies only when pixel format defines alpha
> + * channel, otherwise same as @DRM_BLEND_DISABLED and
> + * @DRM_BLEND_S_ONE_D_ZERO.
> + * @DRM_BLEND_S_CONST_ALPHA_D_ONE_MINUS_CONST_ALPHA: blending with constant
> + * alpha; per-pixel alpha is ignored regardless of the selected pixel
> + * format.
> + * @DRM_BLEND_S_SRC_CONST_ALPHA_D_ONE_MINUS_SRC_CONST_ALPHA: blending with both
> + * per-pixel and plane's alpha; aplies only when pixel format defines alpha
> + * channel, otherwise same as @DRM_BLEND_CONST_ALPHA and
> + * @DRM_BLEND_S_CONST_ALPHA_D_ONE_MINUS_CONST_ALPHA
> + * @DRM_BLEND_DISABLED: same as @DRM_BLEND_S_ONE_D_ZERO
> + * @DRM_BLEND_PIXEL_ALPHA: same as @DRM_BLEND_S_SRC_ALPHA_D_ONE_MINUS_SRC_ALPHA,
> + * @DRM_BLEND_CONST_ALPHA: same as
> + * @DRM_BLEND_S_CONST_ALPHA_D_ONE_MINUS_CONST_ALPHA,
> + * @DRM_BLEND_PIXEL_CONST_ALPHA: same as
> + * @DRM_BLEND_S_SRC_CONST_ALPHA_D_ONE_MINUS_SRC_CONST_ALPHA,
> + *
> + * Values of this enum can be set to 'blend' plane's property. The actual
> + * value of each blending mode consists of two drm_blend_factor values
> + * encoded on lower 16 bits for source plane and higher 16 bits for destiantion
> + * (background).
> + */
> +enum drm_blend_mode {
> + DRM_BLEND_S_ONE_D_ZERO = DRM_BLEND(DRM_FACTOR_ONE, DRM_FACTOR_ZERO),
> + DRM_BLEND_S_SRC_ALPHA_D_ONE_MINUS_SRC_ALPHA =
> + DRM_BLEND(DRM_FACTOR_SRC_ALPHA,
> + DRM_FACTOR_ONE_MINUS_SRC_ALPHA),
> + DRM_BLEND_S_CONST_ALPHA_D_ONE_MINUS_CONST_ALPHA =
> + DRM_BLEND(DRM_FACTOR_CONST_ALPHA,
> + DRM_FACTOR_ONE_MINUS_CONST_ALPHA),
> + DRM_BLEND_S_SRC_CONST_ALPHA_D_ONE_MINUS_SRC_CONST_ALPHA =
> + DRM_BLEND(DRM_FACTOR_SRC_CONST_ALPHA,
> + DRM_FACTOR_ONE_MINUS_SRC_CONST_ALPHA),
> + DRM_BLEND_DISABLED = DRM_BLEND_S_ONE_D_ZERO,
> + DRM_BLEND_PIXEL_ALPHA = DRM_BLEND_S_SRC_ALPHA_D_ONE_MINUS_SRC_ALPHA,
> + DRM_BLEND_CONST_ALPHA = DRM_BLEND_S_CONST_ALPHA_D_ONE_MINUS_CONST_ALPHA,
> + DRM_BLEND_PIXEL_CONST_ALPHA = DRM_BLEND_S_SRC_CONST_ALPHA_D_ONE_MINUS_SRC_CONST_ALPHA,
> +};
> +
> #endif
> --
> 1.9.2
--
Ville Syrjälä
Intel OTC
next prev parent reply other threads:[~2016-02-29 15:23 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-27 14:44 [PATCH v5 0/5] drm/exynos: introduce generic zpos and blending properties Marek Szyprowski
2016-01-27 14:44 ` [PATCH v5 1/5] drm: add generic zpos property Marek Szyprowski
2016-02-29 15:12 ` Ville Syrjälä
2016-03-23 8:46 ` Benjamin Gaignard
2016-04-01 12:48 ` Benjamin Gaignard
2016-05-09 8:42 ` Benjamin Gaignard
2016-05-09 9:05 ` Daniel Vetter
2016-05-09 10:21 ` Tobias Jakobi
2016-01-27 14:44 ` [PATCH v5 2/5] drm/exynos: use generic code for managing zpos plane property Marek Szyprowski
2016-02-28 23:27 ` Inki Dae
2016-01-27 14:44 ` [PATCH v5 3/5] drm: simplify initialization of rotation property Marek Szyprowski
2016-02-29 15:06 ` Daniel Vetter
2016-02-29 15:09 ` Daniel Vetter
2016-02-29 15:17 ` Daniel Vetter
2016-02-29 15:13 ` Ville Syrjälä
2016-01-27 14:44 ` [PATCH v5 4/5] drm: add generic blending related properties Marek Szyprowski
2016-02-29 15:23 ` Ville Syrjälä [this message]
2016-01-27 14:44 ` [PATCH v5 5/5] drm/exynos: add support for blending properties Marek Szyprowski
2016-02-28 23:31 ` Inki Dae
2016-02-23 9:10 ` [PATCH v5 0/5] drm/exynos: introduce generic zpos and " Benjamin Gaignard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160229152312.GY15993@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=a.hajda@samsung.com \
--cc=b.zolnierkie@samsung.com \
--cc=benjamin.gaignard@linaro.org \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=fabien.dessenne@st.com \
--cc=gustavo@padovan.org \
--cc=inki.dae@samsung.com \
--cc=jy0922.shim@samsung.com \
--cc=k.kozlowski@samsung.com \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=sw0312.kim@samsung.com \
--cc=tjakobi@math.uni-bielefeld.de \
--cc=vincent.abriou@st.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.