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@xxxxxxxxxxxxxxxxxxxxx > Cc: wei.c.li@xxxxxxxxx > Cc: harish.krupo.kps@xxxxxxxxx > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Cc: Sean Paul <sean@xxxxxxxxxx> > Cc: Brian Starkey <Brian.Starkey@xxxxxxx> > Cc: Eric Anholt <eric@xxxxxxxxxx> > Cc: Stéphane Marchesin <marcheu@xxxxxxxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > Reviewed-by(v2): Sean Paul <sean@xxxxxxxxxx> > Reviewed-by: Brian Starkey <brian.starkey@xxxxxxx> > --- > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx