[Intel-gfx] [PATCH 1/2] drm: Add CRTC background color property
Sean Paul
sean at poorly.run
Wed Nov 7 16:14:31 UTC 2018
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 at lists.freedesktop.org
> Cc: wei.c.li at intel.com
> Cc: harish.krupo.kps at intel.com
> Signed-off-by: Matt Roper <matthew.d.roper at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Sean Paul, Software Engineer, Google / Chromium OS
More information about the Intel-gfx
mailing list