[Intel-gfx] [PATCH v4.1 2/3] drm: Add CRTC background color property (v4)
Ville Syrjälä
ville.syrjala at linux.intel.com
Thu Jan 31 12:10:26 UTC 2019
On Wed, Jan 30, 2019 at 06:11:16PM -0800, Matt Roper wrote:
> On Wed, Jan 30, 2019 at 11:01:25PM +0200, Ville Syrjälä wrote:
> > 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?
>
> I think it's more for cases where your hardware's pipe writes back
> actual ARGB data to memory, including an alpha component, rather than
> just RGB or XRGB. You could then take that ARGB writeback buffer and
> pass it through some other form of useful compositing if the bgcolor
> pixels have a non-opaque alpha.
I guess someone should specify the blend equation for the alpha
channel as well. I presume it should just be 'da = sa + (1-sa)*da'
regardless of the plane pre-multiply setting?
>
>
> Matt
>
> >
> > > */
> > >
> > > /**
> > > @@ -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
>
> --
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
--
Ville Syrjälä
Intel
More information about the Intel-gfx
mailing list