[Intel-gfx] [PATCH 08/10] drm/i915: Extract ilk_color_check()

[Matt Roper]

On Mon, Mar 18, 2019 at 06:13:15PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> With everything else moved out of the way only ilk+
> remains using _intel_color_check(). Streamline the logic
> into ilk_color_check().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c | 70 ++++++++++++------------------
>  1 file changed, 27 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 6507e5f79c06..8c5c66f5d100 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -871,6 +871,32 @@ static int chv_color_check(struct intel_crtc_state *crtc_state)
>  	return 0;
>  }
>  
> +static int ilk_color_check(struct intel_crtc_state *crtc_state)

Since it confused me at first, you might want to add a comment on this
function that we don't expose color management properties to userspace
on any platforms that use this function.  So the only gamma we can wind
up with is legacy (via ioctl) and the only CSC usage possible is our own
internal limited range conversion.

> +{
> +	int ret;
> +
> +	ret = check_luts(crtc_state);
> +	if (ret)
> +		return ret;

Do we need to call this?  There's never a degamma LUT for these
platforms, and I think the only gamma LUT we'll ever encounter comes
from the legacy gamma ioctl which has already checked that it's a valid
legacy table.


Matt

> +
> +	crtc_state->gamma_enable =
> +		crtc_state->base.gamma_lut &&
> +		!crtc_state->c8_planes;
> +
> +	crtc_state->csc_enable =
> +		ilk_csc_limited_range(crtc_state);
> +
> +	crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
> +
> +	crtc_state->csc_mode = 0;
> +
> +	ret = intel_color_add_affected_planes(crtc_state);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>  static u32 bdw_gamma_mode(const struct intel_crtc_state *crtc_state)
>  {
>  	if (!crtc_state->gamma_enable ||
> @@ -995,48 +1021,6 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
>  	return 0;
>  }
>  
> -static int _intel_color_check(struct intel_crtc_state *crtc_state)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> -	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
> -	const struct drm_property_blob *degamma_lut = crtc_state->base.degamma_lut;
> -	bool limited_color_range = false;
> -	int ret;
> -
> -	ret = check_luts(crtc_state);
> -	if (ret)
> -		return ret;
> -
> -	crtc_state->gamma_enable = (gamma_lut || degamma_lut) &&
> -		!crtc_state->c8_planes;
> -
> -	if (INTEL_GEN(dev_priv) >= 9 ||
> -	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
> -		limited_color_range = crtc_state->limited_color_range;
> -
> -	crtc_state->csc_enable =
> -		crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
> -		crtc_state->base.ctm || limited_color_range;
> -
> -	ret = intel_color_add_affected_planes(crtc_state);
> -	if (ret)
> -		return ret;
> -
> -	crtc_state->csc_mode = 0;
> -
> -	if (!crtc_state->gamma_enable ||
> -	    crtc_state_is_legacy_gamma(crtc_state))
> -		crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
> -	else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> -		crtc_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
> -	else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> -		crtc_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
> -	else
> -		crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
> -
> -	return 0;
> -}
> -
>  void intel_color_init(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -1077,7 +1061,7 @@ void intel_color_init(struct intel_crtc *crtc)
>  		else if (INTEL_GEN(dev_priv) >= 8)
>  			dev_priv->display.color_check = bdw_color_check;
>  		else
> -			dev_priv->display.color_check = _intel_color_check;
> +			dev_priv->display.color_check = ilk_color_check;
>  	}
>  
>  	/* Enable color management support when we have degamma & gamma LUTs. */
> -- 
> 2.19.2
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795