[Intel-gfx] [RFC v1 7/7] drm/i915: Add multi segment gamma for icl

[Shankar, Uma]

>-----Original Message-----
>From: Roper, Matthew D
>Sent: Friday, March 22, 2019 3:34 AM
>To: Shankar, Uma <uma.shankar at intel.com>
>Cc: intel-gfx at lists.freedesktop.org; Lankhorst, Maarten
><maarten.lankhorst at intel.com>; Syrjala, Ville <ville.syrjala at intel.com>; Sharma,
>Shashank <shashank.sharma at intel.com>
>Subject: Re: [RFC v1 7/7] drm/i915: Add multi segment gamma for icl
>
>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 at 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 at 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?

Yeah, it will/can-be re-used for future platforms with no/some slight changes. I will modify this
to align to our overall feature enabling plans in driver.

>
>> +		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)?

Sure, I can move it to platform device info structures and make it transparent here.

Regards,
Uma Shankar
>
>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