[v5,11/11] drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma/degamma lut values
diff mbox series

Message ID 1556989900-21972-12-git-send-email-swati2.sharma@intel.com
State New
Headers show
Series
  • drm/i915: adding state checker for gamma lut values
Related show

Commit Message

Swati Sharma May 4, 2019, 5:11 p.m. UTC
v3: Rebase
v4: -Renamed intel_compare_color_lut() to intel_color_lut_equal() [Jani]
    -Added the default label above the correct label [Jani]
    -Corrected smatch warn "variable dereferenced before check" [Dan Carpenter]
v5: -Added condition (!blob1 && !blob2) return true [Jani]
    -Called PIPE_CONF_CHECK_COLOR_LUT inside if (!adjust) [Jani]
    -Added #undef PIPE_CONF_CHECK_COLOR_LUT [Jani]

There are few things wrong in this patch:
[1] For chv bit precision is 14, on what basis it should be assigned?
[2] For glk and icl, degamma LUT values are not rounded off, there
should err=0 if using same function, how to make that exception?
[3] For glk, bit precision is 10 but gamma mode is 8BIT?

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c   | 54 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color.h   |  6 ++++
 drivers/gpu/drm/i915/intel_display.c | 12 ++++++++
 3 files changed, 72 insertions(+)

Comments

Ville Syrjälä May 6, 2019, 6:33 p.m. UTC | #1
On Sat, May 04, 2019 at 10:41:40PM +0530, Swati Sharma wrote:
> v3: Rebase
> v4: -Renamed intel_compare_color_lut() to intel_color_lut_equal() [Jani]
>     -Added the default label above the correct label [Jani]
>     -Corrected smatch warn "variable dereferenced before check" [Dan Carpenter]
> v5: -Added condition (!blob1 && !blob2) return true [Jani]
>     -Called PIPE_CONF_CHECK_COLOR_LUT inside if (!adjust) [Jani]
>     -Added #undef PIPE_CONF_CHECK_COLOR_LUT [Jani]
> 
> There are few things wrong in this patch:
> [1] For chv bit precision is 14, on what basis it should be assigned?

Like everything else it will more or less be a reverse of the
compute side. For CHV we need to look at cgm_mode, gamma_enable,
gamma_mode, and c8_planes. Hmm. We actually don't have readout for
c8_planes so I guess we'll have to make some kind of exception for
that one.

> [2] For glk and icl, degamma LUT values are not rounded off, there
> should err=0 if using same function, how to make that exception?

You mean the degamma? Just set precision==16? Maybe I'm not
understanding the question.

> [3] For glk, bit precision is 10 but gamma mode is 8BIT?

Not sure what you mean. glk gamma_mode works just like with
other ilk+ platforms (apart from not having the csc_mode
gamma vs. degamma control).

I suspect the most annoying case is ivb-bdw 10bit gamma mode
since we probably don't have the full readout to do the reverse
mapping of whether the LUT is used as gamma or degamma. But I
guess we'll just have to compare it against whichever one the
software state has.

> 
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c   | 54 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_color.h   |  6 ++++
>  drivers/gpu/drm/i915/intel_display.c | 12 ++++++++
>  3 files changed, 72 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 695b76d..73cb901 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -1630,6 +1630,60 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
>  		crtc_state->base.gamma_lut = ilk_read_gamma_lut(crtc_state);
>  }
>  
> +static inline bool err_check(struct drm_color_lut *sw_lut, struct drm_color_lut *hw_lut, u32 err)
> +{
> +	 return ((abs((long)hw_lut->red - sw_lut->red)) <= err) &&
> +	        ((abs((long)hw_lut->blue - sw_lut->blue)) <= err) &&
> +	        ((abs((long)hw_lut->green - sw_lut->green)) <= err);
> +}
> +
> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
> +			   struct drm_property_blob *blob2,
> +			   u32 gamma_mode)
> +{
> +	struct drm_color_lut *sw_lut, *hw_lut;
> +	int sw_lut_size, hw_lut_size, i;
> +	u32 bit_precision, err;
> +
> +	if (!blob1 && !blob2)
> +		return true;
> +
> +	if (!blob1 || !blob2)
> +		return false;
> +
> +	sw_lut_size = drm_color_lut_size(blob1);
> +	hw_lut_size = drm_color_lut_size(blob2);
> +
> +	if (sw_lut_size != hw_lut_size)
> +		return false;
> +
> +	switch(gamma_mode) {
> +	default:
> +	case GAMMA_MODE_MODE_8BIT:
> +		bit_precision = 8;
> +		break;
> +	case GAMMA_MODE_MODE_10BIT:
> +	case GAMMA_MODE_MODE_SPLIT:
> +		bit_precision = 10;
> +		break;
> +	case GAMMA_MODE_MODE_12BIT:
> +		bit_precision = 12;
> +		break;
> +	}
> +
> +	sw_lut = blob1->data;
> +	hw_lut = blob2->data;
> +
> +	err = 0xffff >> bit_precision;
> +
> +	for (i = 0; i < sw_lut_size; i++) {
> +		 if (!err_check(&hw_lut[i], &sw_lut[i], err))
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
>  void intel_color_init(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
> index fc53de9..b525c80 100644
> --- a/drivers/gpu/drm/i915/intel_color.h
> +++ b/drivers/gpu/drm/i915/intel_color.h
> @@ -6,13 +6,19 @@
>  #ifndef __INTEL_COLOR_H__
>  #define __INTEL_COLOR_H__
>  
> +#include <linux/types.h>
> +
>  struct intel_crtc_state;
>  struct intel_crtc;
> +struct drm_property_blob;
>  
>  void intel_color_init(struct intel_crtc *crtc);
>  int intel_color_check(struct intel_crtc_state *crtc_state);
>  void intel_color_commit(const struct intel_crtc_state *crtc_state);
>  void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
>  void intel_color_read_luts(struct intel_crtc_state *crtc_state);
> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
> +				 struct drm_property_blob *blob2,
> +				 u32 gamma_mode);
>  
>  #endif /* __INTEL_COLOR_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 791974b..a713171 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12287,6 +12287,14 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  	} \
>  } while (0)
>  
> +#define PIPE_CONF_CHECK_COLOR_LUT(name, gamma_mode) do { \
> +	if (!intel_color_lut_equal(current_config->name, pipe_config->name, gamma_mode)) { \
> +		pipe_config_err(adjust, __stringify(name), \
> +				"hw_state doesn't match sw_state\n"); \
> +		ret = false; \
> +	} \
> +} while (0)
> +
>  #define PIPE_CONF_QUIRK(quirk) \
>  	((current_config->quirks | pipe_config->quirks) & (quirk))
>  
> @@ -12376,6 +12384,9 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  			PIPE_CONF_CHECK_X(csc_mode);
>  		PIPE_CONF_CHECK_BOOL(gamma_enable);
>  		PIPE_CONF_CHECK_BOOL(csc_enable);
> +
> +		PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, pipe_config->gamma_mode);
> +		PIPE_CONF_CHECK_COLOR_LUT(base.degamma_lut, pipe_config->gamma_mode);
>  	}
>  
>  	PIPE_CONF_CHECK_BOOL(double_wide);
> @@ -12438,6 +12449,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  #undef PIPE_CONF_CHECK_FLAGS
>  #undef PIPE_CONF_CHECK_CLOCK_FUZZY
>  #undef PIPE_CONF_QUIRK
> +#undef PIPE_CONF_CHECK_COLOR_LUT
>  
>  	return ret;
>  }
> -- 
> 1.9.1
Swati Sharma May 6, 2019, 6:57 p.m. UTC | #2
On 07-May-19 12:03 AM, Ville Syrjälä wrote:

> On Sat, May 04, 2019 at 10:41:40PM +0530, Swati Sharma wrote:
>> v3: Rebase
>> v4: -Renamed intel_compare_color_lut() to intel_color_lut_equal() [Jani]
>>      -Added the default label above the correct label [Jani]
>>      -Corrected smatch warn "variable dereferenced before check" [Dan Carpenter]
>> v5: -Added condition (!blob1 && !blob2) return true [Jani]
>>      -Called PIPE_CONF_CHECK_COLOR_LUT inside if (!adjust) [Jani]
>>      -Added #undef PIPE_CONF_CHECK_COLOR_LUT [Jani]
>>
>> There are few things wrong in this patch:
>> [1] For chv bit precision is 14, on what basis it should be assigned?
> Like everything else it will more or less be a reverse of the
> compute side. For CHV we need to look at cgm_mode, gamma_enable,
> gamma_mode, and c8_planes. Hmm. We actually don't have readout for
> c8_planes so I guess we'll have to make some kind of exception for
> that one.

By this I meant was, since I am assigning bit_precision on the basis
of gamma_mode in the compare function like
+	case GAMMA_MODE_MODE_8BIT:
+		bit_precision = 8;
etc. We have 8BIT, 10BIT and 12BIT GAMMA_MODES only.How will I
assign 14BIT on the basis of GAMMA_MODES (or) is there some other
option to assign precision. Please see the comparison code below.

>
>> [2] For glk and icl, degamma LUT values are not rounded off, there
>> should err=0 if using same function, how to make that exception?
> You mean the degamma? Just set precision==16? Maybe I'm not
> understanding the question.

Again same query as above, if I set precision as 16..on what basis?
Which GAMMA_MODE? (or) default i should set as 16?

>
>> [3] For glk, bit precision is 10 but gamma mode is 8BIT?
> Not sure what you mean. glk gamma_mode works just like with
> other ilk+ platforms (apart from not having the csc_mode
> gamma vs. degamma control).

Sorry..my bad!

>
> I suspect the most annoying case is ivb-bdw 10bit gamma mode
> since we probably don't have the full readout to do the reverse
> mapping of whether the LUT is used as gamma or degamma. But I
> guess we'll just have to compare it against whichever one the
> software state has.

One last query, as there is difference in precision of gamma and
degamma..we have one comparison function..how will we get to know whether
blob received is degamma or gamma? Do we need to pass some kind of enum value
to know comparison needs to be done for gamma or degamma?

>
>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_color.c   | 54 ++++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_color.h   |  6 ++++
>>   drivers/gpu/drm/i915/intel_display.c | 12 ++++++++
>>   3 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>> index 695b76d..73cb901 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -1630,6 +1630,60 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
>>   		crtc_state->base.gamma_lut = ilk_read_gamma_lut(crtc_state);
>>   }
>>   
>> +static inline bool err_check(struct drm_color_lut *sw_lut, struct drm_color_lut *hw_lut, u32 err)
>> +{
>> +	 return ((abs((long)hw_lut->red - sw_lut->red)) <= err) &&
>> +	        ((abs((long)hw_lut->blue - sw_lut->blue)) <= err) &&
>> +	        ((abs((long)hw_lut->green - sw_lut->green)) <= err);
>> +}
>> +
>> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
>> +			   struct drm_property_blob *blob2,
>> +			   u32 gamma_mode)
>> +{
>> +	struct drm_color_lut *sw_lut, *hw_lut;
>> +	int sw_lut_size, hw_lut_size, i;
>> +	u32 bit_precision, err;
>> +
>> +	if (!blob1 && !blob2)
>> +		return true;
>> +
>> +	if (!blob1 || !blob2)
>> +		return false;
>> +
>> +	sw_lut_size = drm_color_lut_size(blob1);
>> +	hw_lut_size = drm_color_lut_size(blob2);
>> +
>> +	if (sw_lut_size != hw_lut_size)
>> +		return false;
>> +
>> +	switch(gamma_mode) {
>> +	default:
>> +	case GAMMA_MODE_MODE_8BIT:
>> +		bit_precision = 8;
>> +		break;
>> +	case GAMMA_MODE_MODE_10BIT:
>> +	case GAMMA_MODE_MODE_SPLIT:
>> +		bit_precision = 10;
>> +		break;
>> +	case GAMMA_MODE_MODE_12BIT:
>> +		bit_precision = 12;
>> +		break;
>> +	}
>> +
>> +	sw_lut = blob1->data;
>> +	hw_lut = blob2->data;
>> +
>> +	err = 0xffff >> bit_precision;
>> +
>> +	for (i = 0; i < sw_lut_size; i++) {
>> +		 if (!err_check(&hw_lut[i], &sw_lut[i], err))
>> +			return false;
>> +	}
>> +
>> +	return true;
>> +}
>> +
>>   void intel_color_init(struct intel_crtc *crtc)
>>   {
>>   	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
>> index fc53de9..b525c80 100644
>> --- a/drivers/gpu/drm/i915/intel_color.h
>> +++ b/drivers/gpu/drm/i915/intel_color.h
>> @@ -6,13 +6,19 @@
>>   #ifndef __INTEL_COLOR_H__
>>   #define __INTEL_COLOR_H__
>>   
>> +#include <linux/types.h>
>> +
>>   struct intel_crtc_state;
>>   struct intel_crtc;
>> +struct drm_property_blob;
>>   
>>   void intel_color_init(struct intel_crtc *crtc);
>>   int intel_color_check(struct intel_crtc_state *crtc_state);
>>   void intel_color_commit(const struct intel_crtc_state *crtc_state);
>>   void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
>>   void intel_color_read_luts(struct intel_crtc_state *crtc_state);
>> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
>> +				 struct drm_property_blob *blob2,
>> +				 u32 gamma_mode);
>>   
>>   #endif /* __INTEL_COLOR_H__ */
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 791974b..a713171 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -12287,6 +12287,14 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>>   	} \
>>   } while (0)
>>   
>> +#define PIPE_CONF_CHECK_COLOR_LUT(name, gamma_mode) do { \
>> +	if (!intel_color_lut_equal(current_config->name, pipe_config->name, gamma_mode)) { \
>> +		pipe_config_err(adjust, __stringify(name), \
>> +				"hw_state doesn't match sw_state\n"); \
>> +		ret = false; \
>> +	} \
>> +} while (0)
>> +
>>   #define PIPE_CONF_QUIRK(quirk) \
>>   	((current_config->quirks | pipe_config->quirks) & (quirk))
>>   
>> @@ -12376,6 +12384,9 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>>   			PIPE_CONF_CHECK_X(csc_mode);
>>   		PIPE_CONF_CHECK_BOOL(gamma_enable);
>>   		PIPE_CONF_CHECK_BOOL(csc_enable);
>> +
>> +		PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, pipe_config->gamma_mode);
>> +		PIPE_CONF_CHECK_COLOR_LUT(base.degamma_lut, pipe_config->gamma_mode);
>>   	}
>>   
>>   	PIPE_CONF_CHECK_BOOL(double_wide);
>> @@ -12438,6 +12449,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>>   #undef PIPE_CONF_CHECK_FLAGS
>>   #undef PIPE_CONF_CHECK_CLOCK_FUZZY
>>   #undef PIPE_CONF_QUIRK
>> +#undef PIPE_CONF_CHECK_COLOR_LUT
>>   
>>   	return ret;
>>   }
>> -- 
>> 1.9.1
Ville Syrjälä May 7, 2019, 10:53 a.m. UTC | #3
On Tue, May 07, 2019 at 12:27:07AM +0530, Sharma, Swati2 wrote:
> On 07-May-19 12:03 AM, Ville Syrjälä wrote:
> 
> > On Sat, May 04, 2019 at 10:41:40PM +0530, Swati Sharma wrote:
> >> v3: Rebase
> >> v4: -Renamed intel_compare_color_lut() to intel_color_lut_equal() [Jani]
> >>      -Added the default label above the correct label [Jani]
> >>      -Corrected smatch warn "variable dereferenced before check" [Dan Carpenter]
> >> v5: -Added condition (!blob1 && !blob2) return true [Jani]
> >>      -Called PIPE_CONF_CHECK_COLOR_LUT inside if (!adjust) [Jani]
> >>      -Added #undef PIPE_CONF_CHECK_COLOR_LUT [Jani]
> >>
> >> There are few things wrong in this patch:
> >> [1] For chv bit precision is 14, on what basis it should be assigned?
> > Like everything else it will more or less be a reverse of the
> > compute side. For CHV we need to look at cgm_mode, gamma_enable,
> > gamma_mode, and c8_planes. Hmm. We actually don't have readout for
> > c8_planes so I guess we'll have to make some kind of exception for
> > that one.
> 
> By this I meant was, since I am assigning bit_precision on the basis
> of gamma_mode in the compare function like
> +	case GAMMA_MODE_MODE_8BIT:
> +		bit_precision = 8;
> etc. We have 8BIT, 10BIT and 12BIT GAMMA_MODES only.How will I
> assign 14BIT on the basis of GAMMA_MODES (or) is there some other
> option to assign precision. Please see the comparison code below.

Yeah, gamma_mode by itself is not sufficient in some of the cases.
It's highly platform dependent how you figure out the precision.

> 
> >
> >> [2] For glk and icl, degamma LUT values are not rounded off, there
> >> should err=0 if using same function, how to make that exception?
> > You mean the degamma? Just set precision==16? Maybe I'm not
> > understanding the question.
> 
> Again same query as above, if I set precision as 16..on what basis?
> Which GAMMA_MODE? (or) default i should set as 16?

The glk+ degamma is always 16bpc, so you just hardcode it.

> 
> >
> >> [3] For glk, bit precision is 10 but gamma mode is 8BIT?
> > Not sure what you mean. glk gamma_mode works just like with
> > other ilk+ platforms (apart from not having the csc_mode
> > gamma vs. degamma control).
> 
> Sorry..my bad!
> 
> >
> > I suspect the most annoying case is ivb-bdw 10bit gamma mode
> > since we probably don't have the full readout to do the reverse
> > mapping of whether the LUT is used as gamma or degamma. But I
> > guess we'll just have to compare it against whichever one the
> > software state has.
> 
> One last query, as there is difference in precision of gamma and
> degamma..we have one comparison function..how will we get to know whether
> blob received is degamma or gamma? Do we need to pass some kind of enum value
> to know comparison needs to be done for gamma or degamma?

I think the comparison should just compare two blobs. Doesn't matter
what they are really.

> 
> >
> >> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_color.c   | 54 ++++++++++++++++++++++++++++++++++++
> >>   drivers/gpu/drm/i915/intel_color.h   |  6 ++++
> >>   drivers/gpu/drm/i915/intel_display.c | 12 ++++++++
> >>   3 files changed, 72 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> >> index 695b76d..73cb901 100644
> >> --- a/drivers/gpu/drm/i915/intel_color.c
> >> +++ b/drivers/gpu/drm/i915/intel_color.c
> >> @@ -1630,6 +1630,60 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
> >>   		crtc_state->base.gamma_lut = ilk_read_gamma_lut(crtc_state);
> >>   }
> >>   
> >> +static inline bool err_check(struct drm_color_lut *sw_lut, struct drm_color_lut *hw_lut, u32 err)
> >> +{
> >> +	 return ((abs((long)hw_lut->red - sw_lut->red)) <= err) &&
> >> +	        ((abs((long)hw_lut->blue - sw_lut->blue)) <= err) &&
> >> +	        ((abs((long)hw_lut->green - sw_lut->green)) <= err);
> >> +}
> >> +
> >> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
> >> +			   struct drm_property_blob *blob2,
> >> +			   u32 gamma_mode)
> >> +{
> >> +	struct drm_color_lut *sw_lut, *hw_lut;
> >> +	int sw_lut_size, hw_lut_size, i;
> >> +	u32 bit_precision, err;
> >> +
> >> +	if (!blob1 && !blob2)
> >> +		return true;
> >> +
> >> +	if (!blob1 || !blob2)
> >> +		return false;
> >> +
> >> +	sw_lut_size = drm_color_lut_size(blob1);
> >> +	hw_lut_size = drm_color_lut_size(blob2);
> >> +
> >> +	if (sw_lut_size != hw_lut_size)
> >> +		return false;
> >> +
> >> +	switch(gamma_mode) {
> >> +	default:
> >> +	case GAMMA_MODE_MODE_8BIT:
> >> +		bit_precision = 8;
> >> +		break;
> >> +	case GAMMA_MODE_MODE_10BIT:
> >> +	case GAMMA_MODE_MODE_SPLIT:
> >> +		bit_precision = 10;
> >> +		break;
> >> +	case GAMMA_MODE_MODE_12BIT:
> >> +		bit_precision = 12;
> >> +		break;
> >> +	}
> >> +
> >> +	sw_lut = blob1->data;
> >> +	hw_lut = blob2->data;
> >> +
> >> +	err = 0xffff >> bit_precision;
> >> +
> >> +	for (i = 0; i < sw_lut_size; i++) {
> >> +		 if (!err_check(&hw_lut[i], &sw_lut[i], err))
> >> +			return false;
> >> +	}
> >> +
> >> +	return true;
> >> +}
> >> +
> >>   void intel_color_init(struct intel_crtc *crtc)
> >>   {
> >>   	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >> diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
> >> index fc53de9..b525c80 100644
> >> --- a/drivers/gpu/drm/i915/intel_color.h
> >> +++ b/drivers/gpu/drm/i915/intel_color.h
> >> @@ -6,13 +6,19 @@
> >>   #ifndef __INTEL_COLOR_H__
> >>   #define __INTEL_COLOR_H__
> >>   
> >> +#include <linux/types.h>
> >> +
> >>   struct intel_crtc_state;
> >>   struct intel_crtc;
> >> +struct drm_property_blob;
> >>   
> >>   void intel_color_init(struct intel_crtc *crtc);
> >>   int intel_color_check(struct intel_crtc_state *crtc_state);
> >>   void intel_color_commit(const struct intel_crtc_state *crtc_state);
> >>   void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
> >>   void intel_color_read_luts(struct intel_crtc_state *crtc_state);
> >> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
> >> +				 struct drm_property_blob *blob2,
> >> +				 u32 gamma_mode);
> >>   
> >>   #endif /* __INTEL_COLOR_H__ */
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 791974b..a713171 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -12287,6 +12287,14 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
> >>   	} \
> >>   } while (0)
> >>   
> >> +#define PIPE_CONF_CHECK_COLOR_LUT(name, gamma_mode) do { \
> >> +	if (!intel_color_lut_equal(current_config->name, pipe_config->name, gamma_mode)) { \
> >> +		pipe_config_err(adjust, __stringify(name), \
> >> +				"hw_state doesn't match sw_state\n"); \
> >> +		ret = false; \
> >> +	} \
> >> +} while (0)
> >> +
> >>   #define PIPE_CONF_QUIRK(quirk) \
> >>   	((current_config->quirks | pipe_config->quirks) & (quirk))
> >>   
> >> @@ -12376,6 +12384,9 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
> >>   			PIPE_CONF_CHECK_X(csc_mode);
> >>   		PIPE_CONF_CHECK_BOOL(gamma_enable);
> >>   		PIPE_CONF_CHECK_BOOL(csc_enable);
> >> +
> >> +		PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, pipe_config->gamma_mode);
> >> +		PIPE_CONF_CHECK_COLOR_LUT(base.degamma_lut, pipe_config->gamma_mode);
> >>   	}
> >>   
> >>   	PIPE_CONF_CHECK_BOOL(double_wide);
> >> @@ -12438,6 +12449,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
> >>   #undef PIPE_CONF_CHECK_FLAGS
> >>   #undef PIPE_CONF_CHECK_CLOCK_FUZZY
> >>   #undef PIPE_CONF_QUIRK
> >> +#undef PIPE_CONF_CHECK_COLOR_LUT
> >>   
> >>   	return ret;
> >>   }
> >> -- 
> >> 1.9.1
> 
> -- 
> ~Swati Sharma
>

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 695b76d..73cb901 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1630,6 +1630,60 @@  static void ilk_read_luts(struct intel_crtc_state *crtc_state)
 		crtc_state->base.gamma_lut = ilk_read_gamma_lut(crtc_state);
 }
 
+static inline bool err_check(struct drm_color_lut *sw_lut, struct drm_color_lut *hw_lut, u32 err)
+{
+	 return ((abs((long)hw_lut->red - sw_lut->red)) <= err) &&
+	        ((abs((long)hw_lut->blue - sw_lut->blue)) <= err) &&
+	        ((abs((long)hw_lut->green - sw_lut->green)) <= err);
+}
+
+bool intel_color_lut_equal(struct drm_property_blob *blob1,
+			   struct drm_property_blob *blob2,
+			   u32 gamma_mode)
+{
+	struct drm_color_lut *sw_lut, *hw_lut;
+	int sw_lut_size, hw_lut_size, i;
+	u32 bit_precision, err;
+
+	if (!blob1 && !blob2)
+		return true;
+
+	if (!blob1 || !blob2)
+		return false;
+
+	sw_lut_size = drm_color_lut_size(blob1);
+	hw_lut_size = drm_color_lut_size(blob2);
+
+	if (sw_lut_size != hw_lut_size)
+		return false;
+
+	switch(gamma_mode) {
+	default:
+	case GAMMA_MODE_MODE_8BIT:
+		bit_precision = 8;
+		break;
+	case GAMMA_MODE_MODE_10BIT:
+	case GAMMA_MODE_MODE_SPLIT:
+		bit_precision = 10;
+		break;
+	case GAMMA_MODE_MODE_12BIT:
+		bit_precision = 12;
+		break;
+	}
+
+	sw_lut = blob1->data;
+	hw_lut = blob2->data;
+
+	err = 0xffff >> bit_precision;
+
+	for (i = 0; i < sw_lut_size; i++) {
+		 if (!err_check(&hw_lut[i], &sw_lut[i], err))
+			return false;
+	}
+
+	return true;
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
index fc53de9..b525c80 100644
--- a/drivers/gpu/drm/i915/intel_color.h
+++ b/drivers/gpu/drm/i915/intel_color.h
@@ -6,13 +6,19 @@ 
 #ifndef __INTEL_COLOR_H__
 #define __INTEL_COLOR_H__
 
+#include <linux/types.h>
+
 struct intel_crtc_state;
 struct intel_crtc;
+struct drm_property_blob;
 
 void intel_color_init(struct intel_crtc *crtc);
 int intel_color_check(struct intel_crtc_state *crtc_state);
 void intel_color_commit(const struct intel_crtc_state *crtc_state);
 void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
 void intel_color_read_luts(struct intel_crtc_state *crtc_state);
+bool intel_color_lut_equal(struct drm_property_blob *blob1,
+				 struct drm_property_blob *blob2,
+				 u32 gamma_mode);
 
 #endif /* __INTEL_COLOR_H__ */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 791974b..a713171 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12287,6 +12287,14 @@  static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 	} \
 } while (0)
 
+#define PIPE_CONF_CHECK_COLOR_LUT(name, gamma_mode) do { \
+	if (!intel_color_lut_equal(current_config->name, pipe_config->name, gamma_mode)) { \
+		pipe_config_err(adjust, __stringify(name), \
+				"hw_state doesn't match sw_state\n"); \
+		ret = false; \
+	} \
+} while (0)
+
 #define PIPE_CONF_QUIRK(quirk) \
 	((current_config->quirks | pipe_config->quirks) & (quirk))
 
@@ -12376,6 +12384,9 @@  static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 			PIPE_CONF_CHECK_X(csc_mode);
 		PIPE_CONF_CHECK_BOOL(gamma_enable);
 		PIPE_CONF_CHECK_BOOL(csc_enable);
+
+		PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, pipe_config->gamma_mode);
+		PIPE_CONF_CHECK_COLOR_LUT(base.degamma_lut, pipe_config->gamma_mode);
 	}
 
 	PIPE_CONF_CHECK_BOOL(double_wide);
@@ -12438,6 +12449,7 @@  static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 #undef PIPE_CONF_CHECK_FLAGS
 #undef PIPE_CONF_CHECK_CLOCK_FUZZY
 #undef PIPE_CONF_QUIRK
+#undef PIPE_CONF_CHECK_COLOR_LUT
 
 	return ret;
 }