[PATCH v2 1/2] drm: add legacy support for using degamma for gamma

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Dec 08, 2020 at 03:57:58PM +0200, Tomi Valkeinen wrote:
> We currently have drm_atomic_helper_legacy_gamma_set() helper which can
> be used to handle legacy gamma-set ioctl.
> drm_atomic_helper_legacy_gamma_set() sets GAMMA_LUT, and clears
> CTM and DEGAMMA_LUT. This works fine on HW where we have either:
> 
> degamma -> ctm -> gamma -> out
> 
> or
> 
> ctm -> gamma -> out
> 
> However, if the HW has gamma table before ctm, the atomic property
> should be DEGAMMA_LUT, and thus we have:
> 
> degamma -> ctm -> out
> 
> This is fine for userspace which sets gamma table using the properties,
> as the userspace can check for the existence of gamma & degamma, but the
> legacy gamma-set ioctl does not work.
> 
> This patch fixes the issue by changing
> drm_atomic_helper_legacy_gamma_set() so that GAMMA_LUT will be used if
> it exists, and DEGAMMA_LUT will be used as a fallback.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 15 ++++++++++++---
>  drivers/gpu/drm/drm_color_mgmt.c    |  4 ++++
>  drivers/gpu/drm/drm_fb_helper.c     |  8 ++++++--
>  include/drm/drm_crtc.h              |  3 +++
>  4 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ba1507036f26..117b186fe646 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3512,6 +3512,10 @@ EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
>   * that support color management through the DEGAMMA_LUT/GAMMA_LUT
>   * properties. See drm_crtc_enable_color_mgmt() and the containing chapter for
>   * how the atomic color management and gamma tables work.
> + *
> + * This function uses the GAMMA_LUT or DEGAMMA_LUT property for the gamma table.
> + * GAMMA_LUT property is used if it exists, and DEGAMMA_LUT property is used as
> + * a fallback.
>   */
>  int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
>  				       u16 *red, u16 *green, u16 *blue,
> @@ -3526,6 +3530,9 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
>  	int i, ret = 0;
>  	bool replaced;
>  
> +	if (!crtc->has_gamma_prop && !crtc->has_degamma_prop)
> +		return -ENODEV;
> +
>  	state = drm_atomic_state_alloc(crtc->dev);
>  	if (!state)
>  		return -ENOMEM;
> @@ -3554,10 +3561,12 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
>  		goto fail;
>  	}
>  
> -	/* Reset DEGAMMA_LUT and CTM properties. */
> -	replaced  = drm_property_replace_blob(&crtc_state->degamma_lut, NULL);
> +	/* Set GAMMA/DEGAMMA_LUT and reset DEGAMMA/GAMMA_LUT and CTM */
> +	replaced  = drm_property_replace_blob(&crtc_state->degamma_lut,
> +					      crtc->has_gamma_prop ? NULL : blob);
>  	replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
> -	replaced |= drm_property_replace_blob(&crtc_state->gamma_lut, blob);
> +	replaced |= drm_property_replace_blob(&crtc_state->gamma_lut,
> +					      crtc->has_gamma_prop ? blob : NULL);
>  	crtc_state->color_mgmt_changed |= replaced;
>  
>  	ret = drm_atomic_commit(state);
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 3bcabc2f6e0e..956e59d5f6a7 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -176,6 +176,8 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  					   degamma_lut_size);
>  	}
>  
> +	crtc->has_degamma_prop = !!degamma_lut_size;
> +
>  	if (has_ctm)
>  		drm_object_attach_property(&crtc->base,
>  					   config->ctm_property, 0);
> @@ -187,6 +189,8 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  					   config->gamma_lut_size_property,
>  					   gamma_lut_size);
>  	}
> +
> +	crtc->has_gamma_prop = !!gamma_lut_size;
>  }
>  EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
>  
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 25edf670867c..b0906ef97617 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1001,6 +1001,9 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
>  	drm_client_for_each_modeset(modeset, &fb_helper->client) {
>  		crtc = modeset->crtc;
>  
> +		if (!crtc->has_gamma_prop && !crtc->has_degamma_prop)
> +			continue;
> +
>  		if (!gamma_lut)
>  			gamma_lut = setcmap_new_gamma_lut(crtc, cmap);
>  		if (IS_ERR(gamma_lut)) {
> @@ -1015,11 +1018,12 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
>  			goto out_state;
>  		}
>  
> +		/* Set GAMMA/DEGAMMA_LUT and reset DEGAMMA/GAMMA_LUT and CTM */
>  		replaced  = drm_property_replace_blob(&crtc_state->degamma_lut,
> -						      NULL);
> +						      crtc->has_gamma_prop ? NULL : gamma_lut);
>  		replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
>  		replaced |= drm_property_replace_blob(&crtc_state->gamma_lut,
> -						      gamma_lut);
> +						      crtc->has_gamma_prop ? gamma_lut : NULL);
>  		crtc_state->color_mgmt_changed |= replaced;
>  	}
>  
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index ba839e5e357d..4d9e217e5040 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1084,6 +1084,9 @@ struct drm_crtc {
>  	 */
>  	uint16_t *gamma_store;
>  
> +	bool has_gamma_prop : 1;
> +	bool has_degamma_prop : 1;
> +
>  	/** @helper_private: mid-layer private data */
>  	const struct drm_crtc_helper_funcs *helper_private;
>  

-- 
Regards,

Laurent Pinchart