On Fri, Apr 12, 2019 at 03:50:57PM +0530, Uma Shankar wrote:
> From: Ville Syrjälä
>
> 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ä
> Signed-off-by: Uma Shankar
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,
> );
> 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 = >mode_config;
> +
> + if (!config->gamma_mode_property)
> + return;
> +
> + drm_object_attach_property(>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 = >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 = >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;
