All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: "Saarinen, Jani" <jani.saarinen@intel.com>,
	"Sharma, Swati2" <swati2.sharma@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Cc: "daniel.vetter@ffwll.ch" <daniel.vetter@ffwll.ch>,
	"Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>
Subject: Re: [PATCH] Revert "drm/i915/color: Extract icl_read_luts()"
Date: Wed, 25 Sep 2019 11:41:21 +0300	[thread overview]
Message-ID: <875zlgu5se.fsf@intel.com> (raw)
In-Reply-To: <43D4F724E12AB6478FC1572B3FBE89D07692E53B@IRSMSX106.ger.corp.intel.com>

On Wed, 25 Sep 2019, "Saarinen, Jani" <jani.saarinen@intel.com> wrote:
> Hi, 
>
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Swati
>> Sharma
>> Sent: tiistai 24. syyskuuta 2019 16.58
>> To: intel-gfx@lists.freedesktop.org
>> Cc: Nikula, Jani <jani.nikula@intel.com>; daniel.vetter@ffwll.ch; Nautiyal, Ankit K
>> <ankit.k.nautiyal@intel.com>
>> Subject: [Intel-gfx] [PATCH] Revert "drm/i915/color: Extract icl_read_luts()"
>> 
>> This reverts commit 84af7649188194a74cdd6437235a5e3c86108f0f.
>> 
>> This is causing problems with the display, displays are all bright colors.
>
> Tested-by: Jani Saarinen <jani.saarinen@intel.com>
> Bugzilla:  https://bugs.freedesktop.org/show_bug.cgi?id=111809

Apologies, missed these before pushing. :(

BR,
Jani.

>
>> 
>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_color.c | 126 +++------------------
>>  drivers/gpu/drm/i915/i915_reg.h            |   6 -
>>  2 files changed, 15 insertions(+), 117 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>> b/drivers/gpu/drm/i915/display/intel_color.c
>> index 402151128e1f..9ab34902663e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>> @@ -1420,9 +1420,6 @@ static int icl_color_check(struct intel_crtc_state
>> *crtc_state)
>> 
>>  static int i9xx_gamma_precision(const struct intel_crtc_state *crtc_state)  {
>> -	if (!crtc_state->gamma_enable)
>> -		return 0;
>> -
>>  	switch (crtc_state->gamma_mode) {
>>  	case GAMMA_MODE_MODE_8BIT:
>>  		return 8;
>> @@ -1436,9 +1433,6 @@ static int i9xx_gamma_precision(const struct
>> intel_crtc_state *crtc_state)
>> 
>>  static int ilk_gamma_precision(const struct intel_crtc_state *crtc_state)  {
>> -	if (!crtc_state->gamma_enable)
>> -		return 0;
>> -
>>  	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
>>  		return 0;
>> 
>> @@ -1455,9 +1449,6 @@ static int ilk_gamma_precision(const struct
>> intel_crtc_state *crtc_state)
>> 
>>  static int chv_gamma_precision(const struct intel_crtc_state *crtc_state)  {
>> -	if (!crtc_state->gamma_enable)
>> -		return 0;
>> -
>>  	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
>>  		return 10;
>>  	else
>> @@ -1466,9 +1457,6 @@ static int chv_gamma_precision(const struct
>> intel_crtc_state *crtc_state)
>> 
>>  static int glk_gamma_precision(const struct intel_crtc_state *crtc_state)  {
>> -	if (!crtc_state->gamma_enable)
>> -		return 0;
>> -
>>  	switch (crtc_state->gamma_mode) {
>>  	case GAMMA_MODE_MODE_8BIT:
>>  		return 8;
>> @@ -1480,39 +1468,21 @@ static int glk_gamma_precision(const struct
>> intel_crtc_state *crtc_state)
>>  	}
>>  }
>> 
>> -static int icl_gamma_precision(const struct intel_crtc_state *crtc_state) -{
>> -	if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
>> -		return 0;
>> -
>> -	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
>> -	case GAMMA_MODE_MODE_8BIT:
>> -		return 8;
>> -	case GAMMA_MODE_MODE_10BIT:
>> -		return 10;
>> -	case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
>> -		return 16;
>> -	default:
>> -		MISSING_CASE(crtc_state->gamma_mode);
>> -		return 0;
>> -	}
>> -
>> -}
>> -
>>  int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_state)
>> {
>>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> 
>> +	if (!crtc_state->gamma_enable)
>> +		return 0;
>> +
>>  	if (HAS_GMCH(dev_priv)) {
>>  		if (IS_CHERRYVIEW(dev_priv))
>>  			return
>> chv_gamma_precision(crtc_state);
>>  		else
>>  			return
>> i9xx_gamma_precision(crtc_state);
>>  	} else {
>> -		if (INTEL_GEN(dev_priv) >= 11)
>> -			return
>> icl_gamma_precision(crtc_state);
>> -		else if (IS_CANNONLAKE(dev_priv) ||
>> IS_GEMINILAKE(dev_priv))
>> +		if (IS_CANNONLAKE(dev_priv) ||
>> IS_GEMINILAKE(dev_priv))
>>  			return
>> glk_gamma_precision(crtc_state);
>>  		else if (IS_IRONLAKE(dev_priv))
>>  			return
>> ilk_gamma_precision(crtc_state); @@ -1543,20 +1513,6 @@ static bool
>> intel_color_lut_entry_equal(struct drm_color_lut *lut1,
>>  	return true;
>>  }
>> 
>> -static bool intel_color_lut_entry_multi_equal(struct drm_color_lut *lut1,
>> -					      struct
>> drm_color_lut *lut2,
>> -					      int
>> lut_size, u32 err)
>> -{
>> -	int i;
>> -
>> -	for (i = 0; i < 9; i++) {
>> -		if (!err_check(&lut1[i], &lut2[i], err))
>> -			return false;
>> -	}
>> -
>> -	return true;
>> -}
>> -
>>  bool intel_color_lut_equal(struct drm_property_blob *blob1,
>>  			   struct drm_property_blob *blob2,
>>  			   u32 gamma_mode, u32 bit_precision)
>> @@ -1575,8 +1531,16 @@ bool intel_color_lut_equal(struct drm_property_blob
>> *blob1,
>>  	lut_size2 = drm_color_lut_size(blob2);
>> 
>>  	/* check sw and hw lut size */
>> -	if (lut_size1 != lut_size2)
>> -		return false;
>> +	switch (gamma_mode) {
>> +	case GAMMA_MODE_MODE_8BIT:
>> +	case GAMMA_MODE_MODE_10BIT:
>> +		if (lut_size1 != lut_size2)
>> +			return false;
>> +		break;
>> +	default:
>> +		MISSING_CASE(gamma_mode);
>> +			return false;
>> +	}
>> 
>>  	lut1 = blob1->data;
>>  	lut2 = blob2->data;
>> @@ -1584,18 +1548,13 @@ bool intel_color_lut_equal(struct drm_property_blob
>> *blob1,
>>  	err = 0xffff >> bit_precision;
>> 
>>  	/* check sw and hw lut entry to be equal */
>> -	switch (gamma_mode & GAMMA_MODE_MODE_MASK) {
>> +	switch (gamma_mode) {
>>  	case GAMMA_MODE_MODE_8BIT:
>>  	case GAMMA_MODE_MODE_10BIT:
>>  		if (!intel_color_lut_entry_equal(lut1, lut2,
>> 
>> 	 lut_size2, err))
>>  			return false;
>>  		break;
>> -	case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
>> -		if (!intel_color_lut_entry_multi_equal(lut1, lut2,
>> -
>> 	       lut_size2, err))
>> -			return false;
>> -		break;
>>  	default:
>>  		MISSING_CASE(gamma_mode);
>>  			return false;
>> @@ -1835,60 +1794,6 @@ static void glk_read_luts(struct intel_crtc_state
>> *crtc_state)
>>  		crtc_state->base.gamma_lut =
>> glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));  }
>> 
>> -static struct drm_property_blob *
>> -icl_read_lut_multi_segment(const struct intel_crtc_state *crtc_state) -{
>> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> -	int lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
>> -	enum pipe pipe = crtc->pipe;
>> -	struct drm_property_blob *blob;
>> -	struct drm_color_lut *blob_data;
>> -	u32 i, val1, val2;
>> -
>> -	blob = drm_property_create_blob(&dev_priv->drm,
>> -
>> 	sizeof(struct drm_color_lut) * lut_size,
>> -					NULL);
>> -	if (IS_ERR(blob))
>> -		return NULL;
>> -
>> -	blob_data = blob->data;
>> -
>> -	I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe),
>> PAL_PREC_AUTO_INCREMENT);
>> -
>> -	for (i = 0; i < 9; i++) {
>> -		val1 =
>> I915_READ(PREC_PAL_MULTI_SEG_DATA(pipe));
>> -		val2 =
>> I915_READ(PREC_PAL_MULTI_SEG_DATA(pipe));
>> -
>> -		blob_data[i].red =
>> REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_UDW_MASK, val2) << 6 |
>> -
>> REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_LDW_MASK, val1);
>> -		blob_data[i].green =
>> REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_UDW_MASK, val2) << 6 |
>> -
>> REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_LDW_MASK, val1);
>> -		blob_data[i].blue =
>> REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_UDW_MASK, val2) << 6 |
>> -
>> REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_LDW_MASK, val1);
>> -	}
>> -
>> -	/*
>> -	 * FIXME readouts from PAL_PREC_DATA register aren't giving correct
>> values
>> -	 * in the case of fine and coarse segments. Restricting readouts only
>> for
>> -	 * super fine segment as of now.
>> -	 */
>> -
>> -	return blob;
>> -}
>> -
>> -static void icl_read_luts(struct intel_crtc_state *crtc_state) -{
>> -	if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
>> -	    GAMMA_MODE_MODE_8BIT)
>> -		crtc_state->base.gamma_lut =
>> i9xx_read_lut_8(crtc_state);
>> -	else if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK)
>> ==
>> -		 GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED)
>> -		crtc_state->base.gamma_lut =
>> icl_read_lut_multi_segment(crtc_state);
>> -	else
>> -		crtc_state->base.gamma_lut =
>> glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
>> -}
>> -
>>  void intel_color_init(struct intel_crtc *crtc)  {
>>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -
>> 1932,7 +1837,6 @@ void intel_color_init(struct intel_crtc *crtc)
>> 
>>  		if (INTEL_GEN(dev_priv) >= 11) {
>>  			dev_priv->display.load_luts =
>> icl_load_luts;
>> -			dev_priv->display.read_luts =
>> icl_read_luts;
>>  		} else if (IS_CANNONLAKE(dev_priv) ||
>> IS_GEMINILAKE(dev_priv)) {
>>  			dev_priv->display.load_luts =
>> glk_load_luts;
>>  			dev_priv->display.read_luts =
>> glk_read_luts; diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h index a69c19aae5bb..661cbe4c933a 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -10574,12 +10574,6 @@ enum skl_power_gate {
>> 
>>  #define _PAL_PREC_MULTI_SEG_DATA_A	0x4A40C
>>  #define _PAL_PREC_MULTI_SEG_DATA_B	0x4AC0C
>> -#define  PAL_PREC_MULTI_SEG_RED_LDW_MASK   REG_GENMASK(29, 24)
>> -#define  PAL_PREC_MULTI_SEG_RED_UDW_MASK   REG_GENMASK(29, 20)
>> -#define  PAL_PREC_MULTI_SEG_GREEN_LDW_MASK REG_GENMASK(19, 14) -
>> #define  PAL_PREC_MULTI_SEG_GREEN_UDW_MASK REG_GENMASK(19, 10) -
>> #define  PAL_PREC_MULTI_SEG_BLUE_LDW_MASK  REG_GENMASK(9, 4) -#define
>> PAL_PREC_MULTI_SEG_BLUE_UDW_MASK  REG_GENMASK(9, 0)
>> 
>>  #define PREC_PAL_MULTI_SEG_INDEX(pipe)	_MMIO_PIPE(pipe, \
>> 
>> 	_PAL_PREC_MULTI_SEG_INDEX_A, \
>> --
>> 2.23.0
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2019-09-25  8:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-24 13:58 [PATCH] Revert "drm/i915/color: Extract icl_read_luts()" Swati Sharma
2019-09-24 15:52 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-09-25  5:49 ` [PATCH] " Saarinen, Jani
2019-09-25  8:41   ` Jani Nikula [this message]
2019-09-25  6:04 ` ✓ Fi.CI.IGT: success for " Patchwork
2019-09-25  8:40 ` [PATCH] " Jani Nikula

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=875zlgu5se.fsf@intel.com \
    --to=jani.nikula@intel.com \
    --cc=ankit.k.nautiyal@intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.saarinen@intel.com \
    --cc=swati2.sharma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.