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

2019-03-22 Thread Shankar, Uma


>-Original Message-
>From: Roper, Matthew D
>Sent: Friday, March 22, 2019 3:34 AM
>To: Shankar, Uma 
>Cc: intel-gfx@lists.freedesktop.org; Lankhorst, Maarten
>; Syrjala, Ville ; 
>Sharma,
>Shashank 
>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 
>> ---
>>  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 
>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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2019-03-21 Thread Matt Roper
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 
> ---
>  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 
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

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

2019-03-19 Thread Uma Shankar
Added support for ICL platform multi segment gamma
capabilties and attached the property, exposing the
same to userspace.

Signed-off-by: Uma Shankar 
---
 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)) {
+   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;
+
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

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx