| Message ID | 20181010235051.16693-2-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | CRTC background color | expand |
On Wed, Oct 10, 2018 at 04:50:50PM -0700, 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. > > Cc: dri-devel@lists.freedesktop.org > Cc: wei.c.li@intel.com > Cc: harish.krupo.kps@intel.com > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/drm_atomic_state_helper.c | 1 + > drivers/gpu/drm/drm_atomic_uapi.c | 5 +++++ > drivers/gpu/drm/drm_mode_config.c | 6 ++++++ > include/drm/drm_crtc.h | 17 +++++++++++++++++ > include/drm/drm_mode_config.h | 5 +++++ > include/uapi/drm/drm_mode.h | 26 ++++++++++++++++++++++++++ > 6 files changed, 60 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c > index 3ba996069d69..2f8c55668089 100644 > --- a/drivers/gpu/drm/drm_atomic_state_helper.c > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c > @@ -101,6 +101,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc, > state->planes_changed = false; > state->connectors_changed = false; > state->color_mgmt_changed = false; > + state->bgcolor_changed = false; > state->zpos_changed = false; > state->commit = NULL; > state->event = NULL; > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > index d5b7f315098c..399f0ead5370 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -467,6 +467,9 @@ 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->background_color = val; > + state->bgcolor_changed = true; > } else if (crtc->funcs->atomic_set_property) { > return crtc->funcs->atomic_set_property(crtc, state, property, val); > } else { > @@ -499,6 +502,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->background_color; > 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_mode_config.c b/drivers/gpu/drm/drm_mode_config.c > index ee80788f2c40..75e376755176 100644 > --- a/drivers/gpu/drm/drm_mode_config.c > +++ b/drivers/gpu/drm/drm_mode_config.c > @@ -352,6 +352,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_crtc.h b/include/drm/drm_crtc.h > index b21437bc95bf..ddfdad9ccecb 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -168,6 +168,11 @@ struct drm_crtc_state { > * drivers to steer the atomic commit control flow. > */ > bool color_mgmt_changed : 1; > + /** > + * @bgcolor_changed: Background color value has changed. Used by > + * drivers to steer the atomic commit control flow. > + */ > + bool bgcolor_changed : 1; > > /** > * @no_vblank: > @@ -274,6 +279,18 @@ struct drm_crtc_state { > */ > struct drm_property_blob *gamma_lut; > > + /** > + * @background_color: > + * > + * 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_rgba(); > + * individual color components can be extracted with desired precision > + * via the DRM_RGBA_*() macros. > + */ > + u64 background_color; > + > /** > * @target_vblank: > * > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > index 928e4172a0bb..c3f57a9e5b31 100644 > --- a/include/drm/drm_mode_config.h > +++ b/include/drm/drm_mode_config.h > @@ -807,6 +807,11 @@ struct drm_mode_config { > */ > struct drm_property *writeback_out_fence_ptr_property; > > + /** > + * @bgcolor_property: RGBA background color for CRTC. > + */ > + struct drm_property *bgcolor_property; > + > /* dumb ioctl parameters */ > uint32_t preferred_depth, prefer_shadow; > bool quirk_addfb_prefer_xbgr_30bpp; > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index d3e0fe31efc5..66e2e2c2630e 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -888,6 +888,32 @@ struct drm_mode_revoke_lease { > __u32 lessee_id; > }; > > +/* > + * Put RGBA 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_rgba(__u8 bpc, __u16 red, __u16 green, __u16 blue, __u16 alpha) > +{ > + int msb_shift = 16 - bpc; > + > + return (__u64)alpha << msb_shift << 48 | > + (__u64)blue << msb_shift << 32 | > + (__u64)green << msb_shift << 16 | > + (__u64)red << msb_shift; I would still call that abgr in drm terminology (eg. when compared to the fourccs). And I'd still prefer the argb order as it's what our hardware expects anyway :) Would make me less confused when looking at a 64bit hex color value at least. > +} > + > +/* > + * Extract the specified number of bits of a specific color component from a > + * standard 64-bit RGBA value. > + */ > +#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull) >> (16-numbits)) > +#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits)) > +#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits)) > +#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits)) > + > #if defined(__cplusplus) > } > #endif > -- > 2.14.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Oct 10, 2018 at 04:50:50PM -0700, 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. > > Cc: dri-devel@lists.freedesktop.org > Cc: wei.c.li@intel.com > Cc: harish.krupo.kps@intel.com > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/drm_atomic_state_helper.c | 1 + > drivers/gpu/drm/drm_atomic_uapi.c | 5 +++++ > drivers/gpu/drm/drm_mode_config.c | 6 ++++++ > include/drm/drm_crtc.h | 17 +++++++++++++++++ > include/drm/drm_mode_config.h | 5 +++++ > include/uapi/drm/drm_mode.h | 26 ++++++++++++++++++++++++++ > 6 files changed, 60 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c > index 3ba996069d69..2f8c55668089 100644 > --- a/drivers/gpu/drm/drm_atomic_state_helper.c > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c > @@ -101,6 +101,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc, > state->planes_changed = false; > state->connectors_changed = false; > state->color_mgmt_changed = false; > + state->bgcolor_changed = false; > state->zpos_changed = false; > state->commit = NULL; > state->event = NULL; > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > index d5b7f315098c..399f0ead5370 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -467,6 +467,9 @@ 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->background_color = val; > + state->bgcolor_changed = true; > } else if (crtc->funcs->atomic_set_property) { > return crtc->funcs->atomic_set_property(crtc, state, property, val); > } else { > @@ -499,6 +502,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->background_color; > 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_mode_config.c b/drivers/gpu/drm/drm_mode_config.c > index ee80788f2c40..75e376755176 100644 > --- a/drivers/gpu/drm/drm_mode_config.c > +++ b/drivers/gpu/drm/drm_mode_config.c > @@ -352,6 +352,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", This property should be documented somewhere? Maybe under "Color Management Properties", since they apply to crtc? > + 0, GENMASK_ULL(63, 0)); > + if (!prop) > + return -ENOMEM; > + dev->mode_config.bgcolor_property = prop; > + > return 0; > } > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index b21437bc95bf..ddfdad9ccecb 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -168,6 +168,11 @@ struct drm_crtc_state { > * drivers to steer the atomic commit control flow. > */ > bool color_mgmt_changed : 1; > + /** > + * @bgcolor_changed: Background color value has changed. Used by > + * drivers to steer the atomic commit control flow. > + */ > + bool bgcolor_changed : 1; > > /** > * @no_vblank: > @@ -274,6 +279,18 @@ struct drm_crtc_state { > */ > struct drm_property_blob *gamma_lut; > > + /** > + * @background_color: > + * > + * 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_rgba(); > + * individual color components can be extracted with desired precision > + * via the DRM_RGBA_*() macros. > + */ > + u64 background_color; nit: fwiw, I find it's more useful keep the property name consistent throughout the codebase for easier grepping. So either expand the others from bgcolor to background_color, or vice versa. > + > /** > * @target_vblank: > * > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > index 928e4172a0bb..c3f57a9e5b31 100644 > --- a/include/drm/drm_mode_config.h > +++ b/include/drm/drm_mode_config.h > @@ -807,6 +807,11 @@ struct drm_mode_config { > */ > struct drm_property *writeback_out_fence_ptr_property; > > + /** > + * @bgcolor_property: RGBA background color for CRTC. > + */ > + struct drm_property *bgcolor_property; > + > /* dumb ioctl parameters */ > uint32_t preferred_depth, prefer_shadow; > bool quirk_addfb_prefer_xbgr_30bpp; > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index d3e0fe31efc5..66e2e2c2630e 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -888,6 +888,32 @@ struct drm_mode_revoke_lease { > __u32 lessee_id; > }; > > +/* > + * Put RGBA 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_rgba(__u8 bpc, __u16 red, __u16 green, __u16 blue, __u16 alpha) > +{ > + int msb_shift = 16 - bpc; > + > + return (__u64)alpha << msb_shift << 48 | > + (__u64)blue << msb_shift << 32 | > + (__u64)green << msb_shift << 16 | > + (__u64)red << msb_shift; > +} > + > +/* > + * Extract the specified number of bits of a specific color component from a > + * standard 64-bit RGBA value. > + */ > +#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull) >> (16-numbits)) > +#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits)) > +#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits)) > +#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits)) > + > #if defined(__cplusplus) > } > #endif > -- > 2.14.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index 3ba996069d69..2f8c55668089 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -101,6 +101,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc, state->planes_changed = false; state->connectors_changed = false; state->color_mgmt_changed = false; + state->bgcolor_changed = false; state->zpos_changed = false; state->commit = NULL; state->event = NULL; diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index d5b7f315098c..399f0ead5370 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -467,6 +467,9 @@ 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->background_color = val; + state->bgcolor_changed = true; } else if (crtc->funcs->atomic_set_property) { return crtc->funcs->atomic_set_property(crtc, state, property, val); } else { @@ -499,6 +502,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->background_color; 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_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index ee80788f2c40..75e376755176 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -352,6 +352,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_crtc.h b/include/drm/drm_crtc.h index b21437bc95bf..ddfdad9ccecb 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -168,6 +168,11 @@ struct drm_crtc_state { * drivers to steer the atomic commit control flow. */ bool color_mgmt_changed : 1; + /** + * @bgcolor_changed: Background color value has changed. Used by + * drivers to steer the atomic commit control flow. + */ + bool bgcolor_changed : 1; /** * @no_vblank: @@ -274,6 +279,18 @@ struct drm_crtc_state { */ struct drm_property_blob *gamma_lut; + /** + * @background_color: + * + * 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_rgba(); + * individual color components can be extracted with desired precision + * via the DRM_RGBA_*() macros. + */ + u64 background_color; + /** * @target_vblank: * diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 928e4172a0bb..c3f57a9e5b31 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -807,6 +807,11 @@ struct drm_mode_config { */ struct drm_property *writeback_out_fence_ptr_property; + /** + * @bgcolor_property: RGBA background color for CRTC. + */ + struct drm_property *bgcolor_property; + /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow; bool quirk_addfb_prefer_xbgr_30bpp; diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index d3e0fe31efc5..66e2e2c2630e 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -888,6 +888,32 @@ struct drm_mode_revoke_lease { __u32 lessee_id; }; +/* + * Put RGBA 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_rgba(__u8 bpc, __u16 red, __u16 green, __u16 blue, __u16 alpha) +{ + int msb_shift = 16 - bpc; + + return (__u64)alpha << msb_shift << 48 | + (__u64)blue << msb_shift << 32 | + (__u64)green << msb_shift << 16 | + (__u64)red << msb_shift; +} + +/* + * Extract the specified number of bits of a specific color component from a + * standard 64-bit RGBA value. + */ +#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull) >> (16-numbits)) +#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits)) +#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits)) +#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits)) + #if defined(__cplusplus) } #endif
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. Cc: dri-devel@lists.freedesktop.org Cc: wei.c.li@intel.com Cc: harish.krupo.kps@intel.com Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/drm_atomic_state_helper.c | 1 + drivers/gpu/drm/drm_atomic_uapi.c | 5 +++++ drivers/gpu/drm/drm_mode_config.c | 6 ++++++ include/drm/drm_crtc.h | 17 +++++++++++++++++ include/drm/drm_mode_config.h | 5 +++++ include/uapi/drm/drm_mode.h | 26 ++++++++++++++++++++++++++ 6 files changed, 60 insertions(+)