[Intel-gfx] [PATCH 2/6] drm: introduce color correction properties
Matt Roper
matthew.d.roper at intel.com
Fri Jan 22 13:20:44 PST 2016
On Fri, Jan 22, 2016 at 12:12:08PM +0000, Lionel Landwerlin wrote:
> v2: Register LUT size properties as range
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
Probably should have noticed/commented on this on your previous
iteration, but should we also restrict these new properties to be
atomic-only? I thought there was a consensus a while back that new
functionality would only be exposed for atomic-ready userspace. That
would mean also adding DRM_MODE_PROP_ATOMIC to your flags at creation.
Sorry for not noticing this when I commented before!
Also, you probably want to add a Cc: dri-devel to this patch since it
deals with the DRM core and isn't i915-specific.
Matt
> ---
> Documentation/DocBook/gpu.tmpl | 50 +++++++++++++++++-
> drivers/gpu/drm/drm_atomic.c | 102 +++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/drm_atomic_helper.c | 10 ++++
> drivers/gpu/drm/drm_crtc.c | 35 +++++++++++++
> drivers/gpu/drm/drm_crtc_helper.c | 33 ++++++++++++
> include/drm/drm_crtc.h | 43 ++++++++++++++-
> include/drm/drm_crtc_helper.h | 3 ++
> include/uapi/drm/drm_mode.h | 15 ++++++
> 8 files changed, 287 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index 351e801..c17ac29 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -2068,7 +2068,7 @@ void intel_crt_init(struct drm_device *dev)
> <td valign="top" >property to suggest an Y offset for a connector</td>
> </tr>
> <tr>
> - <td rowspan="3" valign="top" >Optional</td>
> + <td rowspan="8" valign="top" >Optional</td>
> <td valign="top" >“scaling mode”</td>
> <td valign="top" >ENUM</td>
> <td valign="top" >{ "None", "Full", "Center", "Full aspect" }</td>
> @@ -2092,6 +2092,54 @@ void intel_crt_init(struct drm_device *dev)
> <td valign="top" >TBD</td>
> </tr>
> <tr>
> + <td valign="top" >“DEGAMMA_LUT”</td>
> + <td valign="top" >BLOB</td>
> + <td valign="top" >0</td>
> + <td valign="top" >CRTC</td>
> + <td valign="top" >DRM property to set the degamma LUT mapping
> + pixel data from the framebuffer before it is given to the
> + transformation matrix. The data is an interpreted as an array
> + of struct drm_color_lut elements.</td>
> + </tr>
> + <tr>
> + <td valign="top" >“DEGAMMA_LUT_SIZE”</td>
> + <td valign="top" >RANGE | IMMUTABLE</td>
> + <td valign="top" >Min=0, Max=UINT_MAX</td>
> + <td valign="top" >CRTC</td>
> + <td valign="top" >DRM property to gives the size of the LUT to
> + be set on the DEGAMMA_LUT property (the size depends on the
> + underlying hardware).</td>
> + </tr>
> + <tr>
> + <td valign="top" >“CTM_MATRIX”</td>
> + <td valign="top" >BLOB</td>
> + <td valign="top" >0</td>
> + <td valign="top" >CRTC</td>
> + <td valign="top" >DRM property to set the transformation
> + matrix apply to pixel data after the lookup through the
> + degamma LUT and before the lookup through the gamma LUT. The
> + data is an interpreted as a struct drm_color_ctm.</td>
> + </tr>
> + <tr>
> + <td valign="top" >“GAMMA_LUT”</td>
> + <td valign="top" >BLOB</td>
> + <td valign="top" >0</td>
> + <td valign="top" >CRTC</td>
> + <td valign="top" >DRM property to set the gamma LUT mapping
> + pixel data after to the transformation matrix to data sent to
> + the connector. The data is an interpreted as an array
> + of struct drm_color_lut elements.</td>
> + </tr>
> + <tr>
> + <td valign="top" >“GAMMA_LUT_SIZE”</td>
> + <td valign="top" >RANGE | IMMUTABLE</td>
> + <td valign="top" >Min=0, Max=UINT_MAX</td>
> + <td valign="top" >CRTC</td>
> + <td valign="top" >DRM property to gives the size of the LUT to
> + be set on the GAMMA_LUT property (the size depends on the
> + underlying hardware).</td>
> + </tr>
> + <tr>
> <td rowspan="20" valign="top" >i915</td>
> <td rowspan="2" valign="top" >Generic</td>
> <td valign="top" >"Broadcast RGB"</td>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3f74193..5287416 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -28,6 +28,7 @@
>
> #include <drm/drmP.h>
> #include <drm/drm_atomic.h>
> +#include <drm/drm_mode.h>
> #include <drm/drm_plane_helper.h>
>
> /**
> @@ -388,6 +389,58 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
> EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
>
> /**
> + * drm_atomic_replace_property_blob - replace a blob property
> + * @blob: a pointer to the member blob to be replaced
> + * @new_blob: the new blob to replace with
> + * @expected_size: the expected size of the new blob
> + * @replaced: whether the blob has been replaced
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +static int
> +drm_atomic_replace_property_blob(struct drm_property_blob **blob,
> + struct drm_property_blob *new_blob,
> + size_t expected_size,
> + bool *replaced)
> +{
> + struct drm_property_blob *old_blob = *blob;
> +
> + if (old_blob == new_blob)
> + return 0;
> +
> + if (new_blob != NULL && new_blob->length != expected_size)
> + return -EINVAL;
> +
> + if (old_blob != NULL)
> + drm_property_unreference_blob(old_blob);
> + *blob = new_blob;
> + *replaced = true;
> +
> + return 0;
> +}
> +
> +static int
> +drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
> + struct drm_property_blob **blob,
> + uint64_t blob_id,
> + size_t expected_size,
> + bool *replaced)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_property_blob *new_blob = NULL;
> +
> + if (blob_id != 0) {
> + new_blob = drm_property_lookup_blob(dev, blob_id);
> + if (new_blob == NULL)
> + return -EINVAL;
> + }
> +
> + return drm_atomic_replace_property_blob(blob, new_blob,
> + expected_size, replaced);
> +}
> +
> +/**
> * 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 +472,47 @@ 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->degamma_lut_property) {
> + bool replaced = false;
> + uint64_t lut_size = 0;
> +
> + ret = drm_object_property_get_value(&crtc->base,
> + config->degamma_lut_size_property,
> + &lut_size);
> + if (ret == 0)
> + ret = drm_atomic_replace_property_blob_from_id(crtc,
> + &state->degamma_lut,
> + val,
> + lut_size * sizeof(struct drm_color_lut),
> + &replaced);
> + state->color_mgmt_changed = replaced;
> + return ret;
> + } else if (property == config->ctm_matrix_property) {
> + bool replaced = false;
> +
> + ret = drm_atomic_replace_property_blob_from_id(crtc,
> + &state->ctm_matrix,
> + val,
> + sizeof(struct drm_color_ctm),
> + &replaced);
> + state->color_mgmt_changed = replaced;
> + return ret;
> + } else if (property == config->gamma_lut_property) {
> + bool replaced = false;
> + uint64_t lut_size = 0;
> +
> + ret = drm_object_property_get_value(&crtc->base,
> + config->gamma_lut_size_property,
> + &lut_size);
> + if (ret == 0)
> + ret = drm_atomic_replace_property_blob_from_id(crtc,
> + &state->gamma_lut,
> + val,
> + lut_size * sizeof(struct drm_color_lut),
> + &replaced);
> + state->color_mgmt_changed = replaced;
> + return ret;
> + } else if (crtc->funcs->atomic_set_property)
> return crtc->funcs->atomic_set_property(crtc, state, property, val);
> else
> return -EINVAL;
> @@ -456,6 +548,12 @@ 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->degamma_lut_property)
> + *val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
> + else if (property == config->ctm_matrix_property)
> + *val = (state->ctm_matrix) ? state->ctm_matrix->base.id : 0;
> + else if (property == config->gamma_lut_property)
> + *val = (state->gamma_lut) ? state->gamma_lut->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 f0c3984..e4a5755 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2454,10 +2454,17 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>
> if (state->mode_blob)
> drm_property_reference_blob(state->mode_blob);
> + if (state->degamma_lut)
> + drm_property_reference_blob(state->degamma_lut);
> + if (state->ctm_matrix)
> + drm_property_reference_blob(state->ctm_matrix);
> + if (state->gamma_lut)
> + drm_property_reference_blob(state->gamma_lut);
> state->mode_changed = false;
> state->active_changed = false;
> state->planes_changed = false;
> state->connectors_changed = false;
> + state->color_mgmt_changed = false;
> state->event = NULL;
> }
> EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
> @@ -2498,6 +2505,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->degamma_lut);
> + drm_property_unreference_blob(state->ctm_matrix);
> + drm_property_unreference_blob(state->gamma_lut);
> }
> 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 d40bab2..b1aa38a 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1542,6 +1542,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
> return -ENOMEM;
> dev->mode_config.prop_mode_id = prop;
>
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_BLOB,
> + "DEGAMMA_LUT", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.degamma_lut_property = prop;
> +
> + prop = drm_property_create_range(dev,
> + DRM_MODE_PROP_IMMUTABLE,
> + "DEGAMMA_LUT_SIZE", 0, UINT_MAX);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.degamma_lut_size_property = prop;
> +
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_BLOB,
> + "CTM_MATRIX", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.ctm_matrix_property = prop;
> +
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_BLOB,
> + "GAMMA_LUT", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.gamma_lut_property = prop;
> +
> + prop = drm_property_create_range(dev,
> + DRM_MODE_PROP_IMMUTABLE,
> + "GAMMA_LUT_SIZE", 0, UINT_MAX);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.gamma_lut_size_property = prop;
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 5d4bc64..b9b1929 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -1064,3 +1064,36 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
> return drm_plane_helper_commit(plane, plane_state, old_fb);
> }
> EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
> +
> +/**
> + * drm_helper_crtc_enable_color_mgmt - enable color management properties
> + * @crtc: DRM CRTC
> + * @degamma_lut_size: the size of the degamma lut (before CSC)
> + * @gamma_lut_size: the size of the gamma lut (after CSC)
> + *
> + * This function lets the driver enable the color correction properties on a
> + * CRTC. This includes 3 degamma, csc and gamma properties that userspace can
> + * set and 2 size properties to inform the userspace of the lut sizes.
> + */
> +void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> + int degamma_lut_size,
> + int gamma_lut_size)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + drm_object_attach_property(&crtc->base,
> + config->degamma_lut_property, 0);
> + drm_object_attach_property(&crtc->base,
> + config->ctm_matrix_property, 0);
> + drm_object_attach_property(&crtc->base,
> + config->gamma_lut_property, 0);
> +
> + drm_object_attach_property(&crtc->base,
> + config->degamma_lut_size_property,
> + degamma_lut_size);
> + drm_object_attach_property(&crtc->base,
> + config->gamma_lut_size_property,
> + gamma_lut_size);
> +}
> +EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c65a212..967d646 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -305,12 +305,19 @@ 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_mgmt_changed: color management properties have changed (degamma or
> + * gamma LUT or CSC matrix)
> * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
> * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors
> * @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
> + * @degamma_lut: Lookup table for converting framebuffer pixel data
> + * before apply the conversion matrix
> + * @ctm_matrix: Transformation matrix
> + * @gamma_lut: Lookup table for converting pixel data after the
> + * conversion matrix
> * @event: optional pointer to a DRM event to signal upon completion of the
> * state update
> * @state: backpointer to global drm_atomic_state
> @@ -332,6 +339,7 @@ struct drm_crtc_state {
> bool mode_changed : 1;
> bool active_changed : 1;
> bool connectors_changed : 1;
> + bool color_mgmt_changed : 1;
>
> /* attached planes bitmask:
> * WARNING: transitional helpers do not maintain plane_mask so
> @@ -353,6 +361,11 @@ struct drm_crtc_state {
> /* blob property to expose current mode to atomic userspace */
> struct drm_property_blob *mode_blob;
>
> + /* blob property to expose color management to userspace */
> + struct drm_property_blob *degamma_lut;
> + struct drm_property_blob *ctm_matrix;
> + struct drm_property_blob *gamma_lut;
> +
> struct drm_pending_vblank_event *event;
>
> struct drm_atomic_state *state;
> @@ -755,7 +768,7 @@ struct drm_crtc {
> int x, y;
> const struct drm_crtc_funcs *funcs;
>
> - /* CRTC gamma size for reporting to userspace */
> + /* Legacy FB CRTC gamma size for reporting to userspace */
> uint32_t gamma_size;
> uint16_t *gamma_store;
>
> @@ -2023,6 +2036,15 @@ 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
> + * @degamma_lut_property: LUT used to convert the framebuffer's colors to linear
> + * gamma
> + * @degamma_lut_size_property: size of the degamma LUT as supported by the
> + * driver (read-only)
> + * @ctm_matrix_property: Matrix used to convert colors after the lookup in the
> + * degamma LUT
> + * @gamma_lut_property: LUT used to convert the colors, after the CSC matrix, to
> + * the gamma space of the connected screen (read-only)
> + * @gamma_lut_size_property: size of the gamma LUT as supported by the driver
> * @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 +2146,13 @@ struct drm_mode_config {
> struct drm_property *aspect_ratio_property;
> struct drm_property *dirty_info_property;
>
> + /* Optional color correction properties */
> + struct drm_property *degamma_lut_property;
> + struct drm_property *degamma_lut_size_property;
> + struct drm_property *ctm_matrix_property;
> + struct drm_property *gamma_lut_property;
> + struct drm_property *gamma_lut_size_property;
> +
> /* properties for virtual machine layout */
> struct drm_property *suggested_x_property;
> struct drm_property *suggested_y_property;
> @@ -2530,6 +2559,18 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
> return mo ? obj_to_property(mo) : NULL;
> }
>
> +/*
> + * Extract a degamma/gamma LUT value provided by user and round it to the
> + * precision supported by the hardware.
> + */
> +static inline uint32_t drm_color_lut_get_value(uint32_t user_input,
> + uint32_t bit_precision)
> +{
> + uint32_t val = (user_input << 1) + (1 << (16 - bit_precision));
> +
> + return val >> (16 - bit_precision + 1);
> +}
> +
> /* Plane list iterator for legacy (overlay only) planes. */
> #define drm_for_each_legacy_plane(plane, dev) \
> list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index 4b37afa..97fa894 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -48,6 +48,9 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
> struct drm_display_mode *mode,
> int x, int y,
> struct drm_framebuffer *old_fb);
> +extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> + int degamma_lut_size,
> + int gamma_lut_size);
> extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
> extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 50adb46..c021743 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -487,6 +487,21 @@ struct drm_mode_crtc_lut {
> __u64 blue;
> };
>
> +struct drm_color_ctm {
> + /* Conversion matrix in S31.32 format. */
> + __s64 matrix[9];
> +};
> +
> +struct drm_color_lut {
> + /*
> + * Data is U0.16 fixed point format.
> + */
> + __u16 red;
> + __u16 green;
> + __u16 blue;
> + __u16 reserved;
> +};
> +
> #define DRM_MODE_PAGE_FLIP_EVENT 0x01
> #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
> #define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)
> --
> 2.6.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
More information about the Intel-gfx
mailing list