[Intel-gfx] [PATCH 1/6] drm: Create Color Management DRM properties
Daniel Vetter
daniel at ffwll.ch
Mon Dec 21 04:43:18 PST 2015
On Thu, Dec 17, 2015 at 06:57:53PM +0000, Lionel Landwerlin wrote:
> From: Shashank Sharma <shashank.sharma at intel.com>
>
> Color Management is an extension to DRM framework to allow hardware color
> correction and enhancement capabilities.
>
> DRM color manager supports these color properties:
> 1. "ctm": Color transformation matrix property, where a
> color transformation matrix of 9 correction values gets
> applied as correction.
> 2. "palette_before_ctm": for corrections which get applied
> beore color transformation matrix correction.
> 3. "palette_after_ctm": for corrections which get applied
> after color transformation matrix correction.
>
> These color correction capabilities may differ per platform, supporting
> various different no. of correction coefficients. So DRM color manager
> support few properties using which a user space can query the platform's
> capability, and prepare color correction accordingly.
> These query properties are:
> 1. cm_coeff_after_ctm_property
> 2. cm_coeff_before_ctm_property
> (CTM is fix to 9 coefficients across industry)
>
> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
> Signed-off-by: Kausal Malladi <kausalmalladi at gmail.com>
> ---
> drivers/gpu/drm/drm_atomic.c | 67 +++++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/drm_atomic_helper.c | 9 +++++
> drivers/gpu/drm/drm_crtc.c | 32 ++++++++++++++++++
> include/drm/drm_crtc.h | 24 +++++++++++++
> include/uapi/drm/drm.h | 30 +++++++++++++++++
> 5 files changed, 160 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 6a21e5c..ebdb98d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -388,6 +388,38 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
> EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
>
> /**
> + * drm_atomic_crtc_set_blob - find and set a blob
> + * @state_blob: reference pointer to the color blob in the crtc_state
> + * @blob_id: blob_id coming from set_property() call
> + *
> + * Set a color correction blob (originating from a set blob property) on the
> + * desired CRTC state. This function will take reference of the blob property
> + * in the CRTC state, finds the blob based on blob_id (which comes from
> + * set_property call) and set the blob at the proper place.
> + *
> + * RETURNS:
> + * Zero on success, error code on failure.
> + */
> +static int drm_atomic_crtc_set_blob(struct drm_device *dev,
> + struct drm_property_blob **state_blob, uint32_t blob_id)
I think we need to at least split this into a ctm and gamma table variant
to be able to have a size check for the passed-in blob:
- ctm must exactly match the size we expect
- gamma table must be an integer multiple of the drm_r32g32b32 struct.
Also we should probably have a little helper to compute the size of the
gamma lut in entries (just the size/sizeof(drm_r32g32b32) division
really).
The other bits that's missing here is a drm core function to attach these
properties to planes/crtc. We don't want every driver to roll their own
implementation, since it'll lead to fun divergence between different
implementations. Also, that main function could be used to place the ABI
rules someplace nice (like how generic userspace is supposed to interact
with color manager props).
-Daniel
> +{
> + struct drm_property_blob *blob;
> +
> + blob = drm_property_lookup_blob(dev, blob_id);
> + if (!blob) {
> + DRM_DEBUG_KMS("Invalid Blob ID\n");
> + return -EINVAL;
> + }
> +
> + if (*state_blob)
> + drm_property_unreference_blob(*state_blob);
> +
> + /* Attach the blob to be committed in state */
> + *state_blob = blob;
> + return 0;
> +}
> +
> +/**
> * drm_atomic_crtc_set_property - set property on CRTC
> * @crtc: the drm CRTC to set a property on
> * @state: the state object to update with the new property value
> @@ -419,8 +451,31 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
> drm_property_unreference_blob(mode);
> return ret;
> - }
> - else if (crtc->funcs->atomic_set_property)
> + } else if (property == config->cm_palette_after_ctm_property) {
> + ret = drm_atomic_crtc_set_blob(dev,
> + &state->palette_after_ctm_blob, val);
> + if (ret)
> + DRM_DEBUG_KMS("Failed to load blob palette_after_ctm\n");
> + else
> + state->color_correction_changed = true;
> + return ret;
> + } else if (property == config->cm_palette_before_ctm_property) {
> + ret = drm_atomic_crtc_set_blob(dev,
> + &state->palette_before_ctm_blob, val);
> + if (ret)
> + DRM_DEBUG_KMS("Failed to load blob palette_before_ctm\n");
> + else
> + state->color_correction_changed = true;
> + return ret;
> + } else if (property == config->cm_ctm_property) {
> + ret = drm_atomic_crtc_set_blob(dev,
> + &state->ctm_blob, val);
> + if (ret)
> + DRM_DEBUG_KMS("Failed to load blob ctm\n");
> + else
> + state->color_correction_changed = true;
> + return ret;
> + } else if (crtc->funcs->atomic_set_property)
> return crtc->funcs->atomic_set_property(crtc, state, property, val);
> else
> return -EINVAL;
> @@ -456,6 +511,14 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> *val = state->active;
> else if (property == config->prop_mode_id)
> *val = (state->mode_blob) ? state->mode_blob->base.id : 0;
> + else if (property == config->cm_palette_after_ctm_property)
> + *val = (state->palette_after_ctm_blob) ?
> + state->palette_after_ctm_blob->base.id : 0;
> + else if (property == config->cm_palette_before_ctm_property)
> + *val = (state->palette_before_ctm_blob) ?
> + state->palette_before_ctm_blob->base.id : 0;
> + else if (property == config->cm_ctm_property)
> + *val = (state->ctm_blob) ? state->ctm_blob->base.id : 0;
> 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_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 268d37f..bd325da 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2448,6 +2448,12 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>
> if (state->mode_blob)
> drm_property_reference_blob(state->mode_blob);
> + if (state->ctm_blob)
> + drm_property_reference_blob(state->ctm_blob);
> + if (state->palette_after_ctm_blob)
> + drm_property_reference_blob(state->palette_after_ctm_blob);
> + if (state->palette_before_ctm_blob)
> + drm_property_reference_blob(state->palette_before_ctm_blob);
> state->mode_changed = false;
> state->active_changed = false;
> state->planes_changed = false;
> @@ -2492,6 +2498,9 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
> struct drm_crtc_state *state)
> {
> drm_property_unreference_blob(state->mode_blob);
> + drm_property_unreference_blob(state->ctm_blob);
> + drm_property_unreference_blob(state->palette_after_ctm_blob);
> + drm_property_unreference_blob(state->palette_before_ctm_blob);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 62fa95f..2e02d0f 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1542,6 +1542,38 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
> return -ENOMEM;
> dev->mode_config.prop_mode_id = prop;
>
> + /* Color Management properties */
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_BLOB, "PALETTE_AFTER_CTM", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.cm_palette_after_ctm_property = prop;
> +
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_BLOB, "PALETTE_BEFORE_CTM", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.cm_palette_before_ctm_property = prop;
> +
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_BLOB, "CTM", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.cm_ctm_property = prop;
> +
> + /* DRM properties to query color capabilities */
> + prop = drm_property_create(dev, DRM_MODE_PROP_IMMUTABLE,
> + "COEFFICIENTS_BEFORE_CTM", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.cm_coeff_before_ctm_property = prop;
> +
> + prop = drm_property_create(dev, DRM_MODE_PROP_IMMUTABLE,
> + "COEFFICIENTS_AFTER_CTM", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.cm_coeff_after_ctm_property = prop;
> +
> return 0;
> }
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 3b040b3..8326765 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -305,11 +305,15 @@ struct drm_plane_helper_funcs;
> * @mode_changed: crtc_state->mode or crtc_state->enable has been changed
> * @active_changed: crtc_state->active has been toggled.
> * @connectors_changed: connectors to this crtc have been updated
> + * @color_correction_changed: color correction blob in this crtc got updated
> * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
> * @last_vblank_count: for helpers and drivers to capture the vblank of the
> * update to ensure framebuffer cleanup isn't done too early
> * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
> * @mode: current mode timings
> + * @palette_before_ctm_blob: blob for color corrections to be applied after CTM
> + * @palette_after_ctm_blob: blob for color corrections to be applied before CTM
> + * @ctm_blob: blob for CTM color correction
> * @event: optional pointer to a DRM event to signal upon completion of the
> * state update
> * @state: backpointer to global drm_atomic_state
> @@ -331,6 +335,7 @@ struct drm_crtc_state {
> bool mode_changed : 1;
> bool active_changed : 1;
> bool connectors_changed : 1;
> + bool color_correction_changed : 1;
>
> /* attached planes bitmask:
> * WARNING: transitional helpers do not maintain plane_mask so
> @@ -350,6 +355,11 @@ struct drm_crtc_state {
> /* blob property to expose current mode to atomic userspace */
> struct drm_property_blob *mode_blob;
>
> + /* Color management blobs */
> + struct drm_property_blob *palette_before_ctm_blob;
> + struct drm_property_blob *palette_after_ctm_blob;
> + struct drm_property_blob *ctm_blob;
> +
> struct drm_pending_vblank_event *event;
>
> struct drm_atomic_state *state;
> @@ -2019,6 +2029,11 @@ struct drm_mode_config_funcs {
> * @property_blob_list: list of all the blob property objects
> * @blob_lock: mutex for blob property allocation and management
> * @*_property: core property tracking
> + * @cm_palette_before_ctm_property: color corrections before CTM block
> + * @cm_palette_after_ctm_property: color corrections after CTM block
> + * @cm_ctm_property: color transformation matrix correction
> + * @cm_coeff_before_ctm_property: query no of correction coeffi before CTM
> + * @cm_coeff_after_ctm_property: query no of correction coeffi after CTM
> * @preferred_depth: preferred RBG pixel depth, used by fb helpers
> * @prefer_shadow: hint to userspace to prefer shadow-fb rendering
> * @async_page_flip: does this device support async flips on the primary plane?
> @@ -2124,6 +2139,15 @@ struct drm_mode_config {
> struct drm_property *suggested_x_property;
> struct drm_property *suggested_y_property;
>
> + /* Color correction properties */
> + struct drm_property *cm_palette_before_ctm_property;
> + struct drm_property *cm_palette_after_ctm_property;
> + struct drm_property *cm_ctm_property;
> +
> + /* Color correction query */
> + struct drm_property *cm_coeff_before_ctm_property;
> + struct drm_property *cm_coeff_after_ctm_property;
> +
> /* dumb ioctl parameters */
> uint32_t preferred_depth, prefer_shadow;
>
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index b4e92eb..24e4520 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -830,6 +830,36 @@ struct drm_event_vblank {
> __u32 reserved;
> };
>
> +struct drm_r32g32b32 {
> + /*
> + * Data is in U8.24 fixed point format.
> + * All platforms support values within [0, 1.0] range,
> + * for Red, Green and Blue colors.
> + */
> + __u32 r32;
> + __u32 g32;
> + __u32 b32;
> + __u32 reserved;
> +};
> +
> +struct drm_palette {
> + /*
> + * Starting of palette LUT in R32G32B32 format.
> + * Each of RGB value is in U8.24 fixed point format.
> + */
> + struct drm_r32g32b32 lut[0];
> +};
> +
> +struct drm_ctm {
> + /*
> + * Each value is in S31.32 format.
> + * This is 3x3 matrix in row major format.
> + * Integer part will be clipped to nearest
> + * max/min boundary as supported by the HW platform.
> + */
> + __s64 ctm_coeff[9];
> +};
> +
> /* typedef area */
> #ifndef __KERNEL__
> typedef struct drm_clip_rect drm_clip_rect_t;
> --
> 2.6.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list