[Intel-gfx] [v3 1/7] drm: Add gamma mode property
Daniel Vetter
daniel at ffwll.ch
Tue Apr 16 07:28:43 UTC 2019
On Fri, Apr 12, 2019 at 03:50:57PM +0530, Uma Shankar wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Add a gamma mode property to enable various kind of
> gamma modes supported by platforms like: Interpolated, Split,
> Multi Segmented etc. Userspace can get this property and
> should be able to get the platform capabilties wrt various
> gamma modes possible and the possible ranges.
>
> It can select one of the modes exposed as blob_id as an
> enum and set the respective mode.
>
> It can then create the LUT and send it to driver using
> already available GAMMA_LUT property as blob.
>
> v2: Addressed Sam Ravnborg's review comments. Implemented
> gamma mode with just one property and renamed the current
> one to GAMMA_MODE property as recommended by Ville.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
Please also extend the CTM property docs, see
https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#color-management-properties
And especially how GAMMA_MODE interacts with everything else we have
already. I think the current comments don't really explain well how this
is supposed to be used.
Also, since this is quite a complicated data structure, can't we do at
least some basic validation in the core code?
-Daniel
> ---
> drivers/gpu/drm/drm_atomic_uapi.c | 5 +++
> drivers/gpu/drm/drm_color_mgmt.c | 77 +++++++++++++++++++++++++++++++++++++++
> include/drm/drm_color_mgmt.h | 8 ++++
> include/drm/drm_crtc.h | 7 ++++
> include/drm/drm_mode_config.h | 6 +++
> include/uapi/drm/drm_mode.h | 38 +++++++++++++++++++
> 6 files changed, 141 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index ea797d4..d85e0c9 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -459,6 +459,9 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> &replaced);
> state->color_mgmt_changed |= replaced;
> return ret;
> + } else if (property == config->gamma_mode_property) {
> + state->gamma_mode = val;
> + state->color_mgmt_changed |= replaced;
> } else if (property == config->prop_out_fence_ptr) {
> s32 __user *fence_ptr = u64_to_user_ptr(val);
>
> @@ -495,6 +498,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> *val = (state->mode_blob) ? state->mode_blob->base.id : 0;
> else if (property == config->prop_vrr_enabled)
> *val = state->vrr_enabled;
> + else if (property == config->gamma_mode_property)
> + *val = state->gamma_mode;
> else if (property == config->degamma_lut_property)
> *val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
> else if (property == config->ctm_property)
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index d5d34d0..4d6792d 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -176,6 +176,83 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> }
> EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
>
> +void drm_crtc_attach_gamma_mode_property(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + if (!config->gamma_mode_property)
> + return;
> +
> + drm_object_attach_property(&crtc->base,
> + config->gamma_mode_property, 0);
> +}
> +EXPORT_SYMBOL(drm_crtc_attach_gamma_mode_property);
> +
> +int drm_color_create_gamma_mode_property(struct drm_device *dev,
> + int num_values)
> +{
> + struct drm_mode_config *config = &dev->mode_config;
> + struct drm_property *prop;
> +
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_ENUM,
> + "GAMMA_MODE", num_values);
> + if (!prop)
> + return -ENOMEM;
> +
> + config->gamma_mode_property = prop;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_color_create_gamma_mode_property);
> +
> +int drm_color_add_gamma_mode_range(struct drm_device *dev,
> + const char *name,
> + const struct drm_color_lut_range *ranges,
> + size_t length)
> +{
> + struct drm_mode_config *config = &dev->mode_config;
> + struct drm_property_blob *blob;
> + struct drm_property *prop;
> + int num_ranges = length / sizeof(ranges[0]);
> + int i, ret, num_types_0;
> +
> + if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0))
> + return -EINVAL;
> +
> + num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA |
> + DRM_MODE_LUT_DEGAMMA));
> + if (num_types_0 == 0)
> + return -EINVAL;
> +
> + for (i = 1; i < num_ranges; i++) {
> + int num_types = hweight8(ranges[i].flags & (DRM_MODE_LUT_GAMMA |
> + DRM_MODE_LUT_DEGAMMA));
> +
> + /* either all ranges have DEGAMMA|GAMMA or none have it */
> + if (num_types_0 != num_types)
> + return -EINVAL;
> + }
> +
> + prop = config->gamma_mode_property;
> + if (!prop)
> + return -EINVAL;
> +
> + blob = drm_property_create_blob(dev, length, ranges);
> + if (IS_ERR(blob))
> + return PTR_ERR(blob);
> +
> + ret = drm_property_add_enum(prop, blob->base.id, name);
> + if (ret) {
> + drm_property_blob_put(blob);
> + return ret;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_color_add_gamma_mode_range);
> +
> /**
> * drm_mode_crtc_set_gamma_size - set the gamma table size
> * @crtc: CRTC to set the gamma table size for
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index d1c662d..f18e9b8 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -51,6 +51,14 @@ static inline int drm_color_lut_size(const struct drm_property_blob *blob)
> return blob->length / sizeof(struct drm_color_lut);
> }
>
> +int drm_color_create_gamma_mode_property(struct drm_device *dev,
> + int num_values);
> +void drm_crtc_attach_gamma_mode_property(struct drm_crtc *crtc);
> +int drm_color_add_gamma_mode_range(struct drm_device *dev,
> + const char *name,
> + const struct drm_color_lut_range *ranges,
> + size_t length);
> +
> enum drm_color_encoding {
> DRM_COLOR_YCBCR_BT601,
> DRM_COLOR_YCBCR_BT709,
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 58ad983..f2e60bd 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -249,6 +249,13 @@ struct drm_crtc_state {
> struct drm_property_blob *mode_blob;
>
> /**
> + * @gamma_mode: This is a blob_id and exposes the platform capabilties
> + * wrt to various gamma modes and the respective lut ranges. This also
> + * helps user select a gamma mode amongst the supported ones.
> + */
> + u32 gamma_mode;
> +
> + /**
> * @degamma_lut:
> *
> * Lookup table for converting framebuffer pixel data before apply the
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 7f60e8e..8f961c5b 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -761,6 +761,12 @@ struct drm_mode_config {
> */
> struct drm_property *content_type_property;
> /**
> + * @gamma_mode_property: Optional CRTC property to enumerate and
> + * select the mode of the crtc gamma/degmama LUTs. This also exposes
> + * the lut ranges of the various supported gamma modes to userspace.
> + */
> + struct drm_property *gamma_mode_property;
> + /**
> * @degamma_lut_property: Optional CRTC property to set the LUT used to
> * convert the framebuffer's colors to linear gamma.
> */
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 83cd163..e70b7f8 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -630,6 +630,44 @@ struct drm_color_lut {
> __u16 reserved;
> };
>
> +/*
> + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT
> + * can be used for either purpose, but not simultaneously. To expose
> + * modes that support gamma and degamma simultaneously the gamma mode
> + * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA
> + * ranges.
> + */
> +/* LUT is for gamma (after CTM) */
> +#define DRM_MODE_LUT_GAMMA BIT(0)
> +/* LUT is for degamma (before CTM) */
> +#define DRM_MODE_LUT_DEGAMMA BIT(1)
> +/* linearly interpolate between the points */
> +#define DRM_MODE_LUT_INTERPOLATE BIT(2)
> +/*
> + * the last value of the previous range is the
> + * first value of the current range.
> + */
> +#define DRM_MODE_LUT_REUSE_LAST BIT(3)
> +/* the curve must be non-decreasing */
> +#define DRM_MODE_LUT_NON_DECREASING BIT(4)
> +/* the curve is reflected across origin for negative inputs */
> +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5)
> +/* the same curve (red) is used for blue and green channels as well */
> +#define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
> +
> +struct drm_color_lut_range {
> + /* DRM_MODE_LUT_* */
> + __u32 flags;
> + /* number of points on the curve */
> + __u16 count;
> + /* input/output bits per component */
> + __u8 input_bpc, output_bpc;
> + /* input start/end values */
> + __s32 start, end;
> + /* output min/max values */
> + __s32 min, max;
> +};
> +
> #define DRM_MODE_PAGE_FLIP_EVENT 0x01
> #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
> #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list