[Intel-gfx] [PATCH v4.1 2/3] drm: Add CRTC background color property (v4)
Ville Syrjälä
ville.syrjala at linux.intel.com
Wed Jan 30 21:01:25 UTC 2019
On Wed, Jan 30, 2019 at 10:51:21AM -0800, Matt Roper wrote:
> Some display controllers can be programmed to present non-black colors
> for pixels not covered by any plane (or pixels covered by the
> transparent regions of higher planes). Compositors that want a UI with
> a solid color background can potentially save memory bandwidth by
> setting the CRTC background property and using smaller planes to display
> the rest of the content.
>
> To avoid confusion between different ways of encoding RGB data, we
> define a standard 64-bit format that should be used for this property's
> value. Helper functions and macros are provided to generate and dissect
> values in this standard format with varying component precision values.
>
> v2:
> - Swap internal representation's blue and red bits to make it easier
> to read if printed out. (Ville)
> - Document bgcolor property in drm_blend.c. (Sean Paul)
> - s/background_color/bgcolor/ for consistency between property name and
> value storage field. (Sean Paul)
> - Add a convenience function to attach property to a given crtc.
>
> v3:
> - Restructure ARGB component extraction macros to be easier to
> understand and enclose the parameters in () to avoid calculations
> if expressions are passed. (Sean Paul)
> - s/rgba/argb/ in helper function/macro names. Even though the idea is
> to not worry about the internal representation of the u64, it can
> still be confusing to look at code that uses 'rgba' terminology, but
> stores values with argb ordering. (Ville)
>
> v4:
> - Drop the bgcolor_changed flag. (Ville, Brian Starkey)
> - Clarify in kerneldoc that background color is expected to undergo the
> same pipe-level degamma/csc/gamma transformations that planes do.
> (Brian Starkey)
> - Update kerneldoc to indicate non-opaque colors are allowed, but are
> generally only useful in special cases such as when writeback
> connectors are used (Brian Starkey / Eric Anholt)
>
> Cc: dri-devel at lists.freedesktop.org
> Cc: wei.c.li at intel.com
> Cc: harish.krupo.kps at intel.com
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Sean Paul <sean at poorly.run>
> Cc: Brian Starkey <Brian.Starkey at arm.com>
> Cc: Eric Anholt <eric at anholt.net>
> Cc: Stéphane Marchesin <marcheu at chromium.org>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> Reviewed-by(v2): Sean Paul <sean at poorly.run>
> Reviewed-by: Brian Starkey <brian.starkey at arm.com>
> ---
> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++
> drivers/gpu/drm/drm_blend.c | 27 ++++++++++++++++++++++++---
> drivers/gpu/drm/drm_mode_config.c | 6 ++++++
> include/drm/drm_blend.h | 1 +
> include/drm/drm_crtc.h | 12 ++++++++++++
> include/drm/drm_mode_config.h | 5 +++++
> include/uapi/drm/drm_mode.h | 28 ++++++++++++++++++++++++++++
> 7 files changed, 80 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 9a1f41adfc67..d569e20e34e3 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -469,6 +469,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> return -EFAULT;
>
> set_out_fence_for_crtc(state->state, crtc, fence_ptr);
> + } else if (property == config->bgcolor_property) {
> + state->bgcolor = val;
> } else if (crtc->funcs->atomic_set_property) {
> return crtc->funcs->atomic_set_property(crtc, state, property, val);
> } else {
> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> else if (property == config->prop_out_fence_ptr)
> *val = 0;
> + else if (property == config->bgcolor_property)
> + *val = state->bgcolor;
> else if (crtc->funcs->atomic_get_property)
> return crtc->funcs->atomic_get_property(crtc, state, property, val);
> else
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index 0c78ca386cbe..d451ac9e1d6d 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -175,9 +175,22 @@
> * 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).
> + * The property extensions described above all apply to the plane. Drivers
> + * may also expose the following crtc property extension:
> + *
> + * BACKGROUND_COLOR:
> + * Background color is setup with drm_crtc_add_bgcolor_property(). It
> + * controls the ARGB color of a full-screen layer that exists below all
> + * planes. This color will be used for pixels not covered by any plane
> + * and may also be blended with plane contents as allowed by a plane's
> + * alpha values. The background color defaults to black, and is assumed
> + * to be black for drivers that do not expose this property. Although
> + * background color isn't a plane, it is assumed that the color provided
> + * here undergoes the same pipe-level degamma/CSC/gamma transformations
> + * that planes undergo. Note that the color value provided here includes
> + * an alpha channel...non-opaque background color values are allowed,
> + * but are generally only honored in special cases (e.g., when a memory
> + * writeback connector is in use).
What would the alpha<1.0 do in that case? Blend the writeback output
with what's already there?
> */
>
> /**
> @@ -593,3 +606,11 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane,
> return 0;
> }
> EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
> +
> +void drm_crtc_add_bgcolor_property(struct drm_crtc *crtc)
> +{
> + drm_object_attach_property(&crtc->base,
> + crtc->dev->mode_config.bgcolor_property,
> + drm_argb(16, 0xffff, 0, 0, 0));
if (crtc->state)
crtc->state->bg = default value; ?
> +}
> +EXPORT_SYMBOL(drm_crtc_add_bgcolor_property);
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 4a1c2023ccf0..8a7c346b3191 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -364,6 +364,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
> return -ENOMEM;
> dev->mode_config.modifiers_property = prop;
>
> + prop = drm_property_create_range(dev, 0, "BACKGROUND_COLOR",
> + 0, GENMASK_ULL(63, 0));
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.bgcolor_property = prop;
> +
> return 0;
> }
>
> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> index 88bdfec3bd88..9e2538dd7b9a 100644
> --- a/include/drm/drm_blend.h
> +++ b/include/drm/drm_blend.h
> @@ -58,4 +58,5 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
> struct drm_atomic_state *state);
> int drm_plane_create_blend_mode_property(struct drm_plane *plane,
> unsigned int supported_modes);
> +void drm_crtc_add_bgcolor_property(struct drm_crtc *crtc);
> #endif
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 85abd3fe9e83..dbe0b45d5da6 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -274,6 +274,18 @@ struct drm_crtc_state {
> */
> struct drm_property_blob *gamma_lut;
>
> + /**
> + * @bgcolor:
> + *
> + * RGB value representing the pipe's background color. The background
> + * color (aka "canvas color") of a pipe is the color that will be used
> + * for pixels not covered by a plane, or covered by transparent pixels
> + * of a plane. The value here should be built via drm_argb();
> + * individual color components can be extracted with desired precision
> + * via the DRM_ARGB_*() macros.
> + */
> + u64 bgcolor;
> +
> /**
> * @target_vblank:
> *
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 1e6cb885994d..0463d3f4ae59 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -836,6 +836,11 @@ struct drm_mode_config {
> */
> struct drm_property *writeback_out_fence_ptr_property;
>
> + /**
> + * @bgcolor_property: RGB background color for CRTC.
> + */
> + struct drm_property *bgcolor_property;
> +
> /* dumb ioctl parameters */
> uint32_t preferred_depth, prefer_shadow;
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index a439c2e67896..5f31e6a05bd9 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -907,6 +907,34 @@ struct drm_mode_rect {
> __s32 y2;
> };
>
> +/*
> + * Put ARGB values into a standard 64-bit representation that can be used
> + * for ioctl parameters, inter-driver commmunication, etc. If the component
> + * values being provided contain less than 16 bits of precision, they'll
> + * be shifted into the most significant bits.
> + */
> +static inline __u64
> +drm_argb(__u8 bpc, __u16 alpha, __u16 red, __u16 green, __u16 blue)
> +{
> + int msb_shift = 16 - bpc;
> +
> + return (__u64)alpha << msb_shift << 48 |
> + (__u64)red << msb_shift << 32 |
> + (__u64)green << msb_shift << 16 |
> + (__u64)blue << msb_shift;
> +}
> +
> +/*
> + * Extract the specified number of bits of a specific color component from a
> + * standard 64-bit ARGB value.
> + */
> +#define DRM_ARGB_COMP(c, shift, numbits) \
> + (__u16)(((c) & 0xFFFFull << (shift)) >> ((shift) + 16 - (numbits)))
> +#define DRM_ARGB_BLUE(c, numbits) DRM_ARGB_COMP(c, 0, numbits)
> +#define DRM_ARGB_GREEN(c, numbits) DRM_ARGB_COMP(c, 16, numbits)
> +#define DRM_ARGB_RED(c, numbits) DRM_ARGB_COMP(c, 32, numbits)
> +#define DRM_ARGB_ALPHA(c, numbits) DRM_ARGB_COMP(c, 48, numbits)
> +
> #if defined(__cplusplus)
> }
> #endif
> --
> 2.14.5
--
Ville Syrjälä
Intel
More information about the Intel-gfx
mailing list