| Message ID | 20190130185122.10322-3-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | CRTC background color | expand |
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@lists.freedesktop.org > Cc: wei.c.li@intel.com > Cc: harish.krupo.kps@intel.com > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Sean Paul <sean@poorly.run> > Cc: Brian Starkey <Brian.Starkey@arm.com> > Cc: Eric Anholt <eric@anholt.net> > Cc: Stéphane Marchesin <marcheu@chromium.org> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > Reviewed-by(v2): Sean Paul <sean@poorly.run> > Reviewed-by: Brian Starkey <brian.starkey@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
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@lists.freedesktop.org > > Cc: wei.c.li@intel.com > > Cc: harish.krupo.kps@intel.com > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Sean Paul <sean@poorly.run> > > Cc: Brian Starkey <Brian.Starkey@arm.com> > > Cc: Eric Anholt <eric@anholt.net> > > Cc: Stéphane Marchesin <marcheu@chromium.org> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > Reviewed-by(v2): Sean Paul <sean@poorly.run> > > Reviewed-by: Brian Starkey <brian.starkey@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. 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
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@lists.freedesktop.org > > > Cc: wei.c.li@intel.com > > > Cc: harish.krupo.kps@intel.com > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Sean Paul <sean@poorly.run> > > > Cc: Brian Starkey <Brian.Starkey@arm.com> > > > Cc: Eric Anholt <eric@anholt.net> > > > Cc: Stéphane Marchesin <marcheu@chromium.org> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > > Reviewed-by(v2): Sean Paul <sean@poorly.run> > > > Reviewed-by: Brian Starkey <brian.starkey@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
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). */ /** @@ -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)); +} +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