From: Jani Nikula <jani.nikula@intel.com>
To: Swati Sharma <swati2.sharma@intel.com>, intel-gfx@lists.freedesktop.org
Cc: daniel.vetter@ffwll.ch, ankit.k.nautiyal@intel.com
Subject: Re: [PATCH] Revert "drm/i915/color: Extract icl_read_luts()"
Date: Wed, 25 Sep 2019 11:40:25 +0300 [thread overview]
Message-ID: <878sqcu5ty.fsf@intel.com> (raw)
In-Reply-To: <20190924135820.11850-1-swati2.sharma@intel.com>
On Tue, 24 Sep 2019, Swati Sharma <swati2.sharma@intel.com> wrote:
> This reverts commit 84af7649188194a74cdd6437235a5e3c86108f0f.
>
> This is causing problems with the display, displays are all
> bright colors.
>
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
Pushed, thanks for the patch.
Now we do need to figure out how to do at least something useful and
non-regressing with icl/tgl readouts.
Could we add the legacy gamma checks first?
Also one other note inline below.
BR,
Jani.
> ---
> 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)
A patch adding just the register definitions could be merged as well.
>
> #define PREC_PAL_MULTI_SEG_INDEX(pipe) _MMIO_PIPE(pipe, \
> _PAL_PREC_MULTI_SEG_INDEX_A, \
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
prev parent reply other threads:[~2019-09-25 8:40 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
2019-09-25 6:04 ` ✓ Fi.CI.IGT: success for " Patchwork
2019-09-25 8:40 ` Jani Nikula [this message]
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=878sqcu5ty.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=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.