Re: [RFC v1 7/7] drm/i915: Add multi segment gamma for icl

[Matt Roper <matthew.d.roper@intel.com>]

On Tue, Mar 19, 2019 at 02:00:18PM +0530, Uma Shankar wrote:
> Added support for ICL platform multi segment gamma
> capabilties and attached the property, exposing the
> same to userspace.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 7733c256..1e9f784 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -1011,6 +1011,8 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
>  void intel_color_init(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	struct multi_segment_gamma_lut multi_segment_lut;
> +
>  
>  	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
>  
> @@ -1049,6 +1051,24 @@ void intel_color_init(struct intel_crtc *crtc)
>  
>  	intel_attach_gamma_mode_property(crtc);
>  
> -	if (INTEL_GEN(dev_priv) >= 11)
> +	if (IS_ICELAKE(dev_priv)) {

Is it intentional to limit this specifically to Icelake?  This is
basically the opposite of the type of generalization that we've been
moving toward with patches like

        commit 2dd24a9c2c8d767a976da37d59680f09b9d111ab
        Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
        Date:   Fri Mar 8 13:42:58 2019 -0800

            drm/i915/gen11+: First assume next platforms will inherit stuff

Also, we already support another gen11 platform (or will once the
mailing list patches land) in the form of EHL; is there any reason that
this shouldn't apply to it?

> +		multi_segment_lut.segment_cnt = 3;
> +		multi_segment_lut.precision_bits = 16;
> +		multi_segment_lut.segment_lut_cnt_ptr = kzalloc(3 * sizeof(int),
> +								GFP_KERNEL);
> +		if (!multi_segment_lut.segment_lut_cnt_ptr)
> +			return;
> +		multi_segment_lut.segment_lut_cnt_ptr[0] = 9;
> +		multi_segment_lut.segment_lut_cnt_ptr[1] = 256;
> +		multi_segment_lut.segment_lut_cnt_ptr[2] = 256;

On this patch and the previous one we have a few magic numbers
representing how the segmented gamma is laid out.  Can we move stuff
like this into the device info structure and then just setup the
appropriate userspace properties on any platform that has the device
info fields filled in (i.e., the same way we do the basic color
management properties)?


Matt

> +
>  		intel_attach_multi_segment_gamma_mode_property(crtc);
> +
> +		drm_property_replace_global_blob(crtc->base.dev,
> +						 &crtc->config->multi_segment_gamma_lut,
> +						 sizeof(struct multi_segment_gamma_lut),
> +						 &multi_segment_lut,
> +						 &crtc->base.base,
> +						 dev_priv->multi_segment_gamma_mode_property);
> +	}
>  }
> -- 
> 1.9.1
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx