From: "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
<intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH 04/13] drm/i915: Clean up various indexed LUT registers
Date: Thu, 1 Dec 2022 11:15:55 +0530 [thread overview]
Message-ID: <23780f01-ab31-71e0-23fc-258b80e138ef@intel.com> (raw)
In-Reply-To: <20221123152638.20622-5-ville.syrjala@linux.intel.com>
On 11/23/2022 8:56 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Use REG_BIT() & co. for the LUT index registers, and also
> use the REG_FIELD_PREP() stuff a bit more consistently when
> generating the values for said registers.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_color.c | 46 +++++++++++++++-------
> drivers/gpu/drm/i915/i915_reg.h | 18 +++++----
> 2 files changed, 41 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 956b221860e6..c960c2aaf328 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -910,7 +910,8 @@ static void ivb_load_lut_10(struct intel_crtc *crtc,
> enum pipe pipe = crtc->pipe;
>
> for (i = 0; i < lut_size; i++) {
> - intel_de_write_fw(i915, PREC_PAL_INDEX(pipe), prec_index++);
> + intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> + prec_index + i);
> intel_de_write_fw(i915, PREC_PAL_DATA(pipe),
> ilk_lut_10(&lut[i]));
> }
> @@ -919,7 +920,8 @@ static void ivb_load_lut_10(struct intel_crtc *crtc,
> * Reset the index, otherwise it prevents the legacy palette to be
> * written properly.
> */
> - intel_de_write_fw(i915, PREC_PAL_INDEX(pipe), 0);
> + intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> + PAL_PREC_INDEX_VALUE(0));
> }
>
> /* On BDW+ the index auto increment mode actually works */
> @@ -933,7 +935,8 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
> enum pipe pipe = crtc->pipe;
>
> intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> - prec_index | PAL_PREC_AUTO_INCREMENT);
> + PAL_PREC_AUTO_INCREMENT |
> + prec_index);
>
> for (i = 0; i < lut_size; i++)
> intel_de_write_fw(i915, PREC_PAL_DATA(pipe),
> @@ -943,7 +946,8 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
> * Reset the index, otherwise it prevents the legacy palette to be
> * written properly.
> */
> - intel_de_write_fw(i915, PREC_PAL_INDEX(pipe), 0);
> + intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> + PAL_PREC_INDEX_VALUE(0));
> }
>
> static void ivb_load_lut_ext_max(const struct intel_crtc_state *crtc_state)
> @@ -1049,9 +1053,11 @@ static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state,
> * ignore the index bits, so we need to reset it to index 0
> * separately.
> */
> - intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe), 0);
> intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe),
> - PRE_CSC_GAMC_AUTO_INCREMENT);
> + PRE_CSC_GAMC_INDEX_VALUE(0));
> + intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe),
> + PRE_CSC_GAMC_AUTO_INCREMENT |
> + PRE_CSC_GAMC_INDEX_VALUE(0));
>
> for (i = 0; i < lut_size; i++) {
> /*
> @@ -1165,7 +1171,9 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
> * seg2[0] being unused by the hardware.
> */
> intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
> - PAL_PREC_AUTO_INCREMENT);
> + PAL_PREC_AUTO_INCREMENT |
> + PAL_PREC_INDEX_VALUE(0));
> +
> for (i = 1; i < 257; i++) {
> entry = &lut[i * 8];
> intel_dsb_indexed_reg_write(crtc_state, PREC_PAL_DATA(pipe),
> @@ -2756,7 +2764,8 @@ static struct drm_property_blob *ivb_read_lut_10(struct intel_crtc *crtc,
> ilk_lut_10_pack(&lut[i], val);
> }
>
> - intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe), 0);
> + intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe),
> + PAL_PREC_INDEX_VALUE(0));
>
> return blob;
> }
> @@ -2811,7 +2820,8 @@ static struct drm_property_blob *bdw_read_lut_10(struct intel_crtc *crtc,
> lut = blob->data;
>
> intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> - prec_index | PAL_PREC_AUTO_INCREMENT);
> + PAL_PREC_AUTO_INCREMENT |
> + prec_index);
>
> for (i = 0; i < lut_size; i++) {
> u32 val = intel_de_read_fw(i915, PREC_PAL_DATA(pipe));
> @@ -2819,7 +2829,8 @@ static struct drm_property_blob *bdw_read_lut_10(struct intel_crtc *crtc,
> ilk_lut_10_pack(&lut[i], val);
> }
>
> - intel_de_write_fw(i915, PREC_PAL_INDEX(pipe), 0);
> + intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> + PAL_PREC_INDEX_VALUE(0));
>
> return blob;
> }
> @@ -2876,9 +2887,11 @@ static struct drm_property_blob *glk_read_degamma_lut(struct intel_crtc *crtc)
> * ignore the index bits, so we need to reset it to index 0
> * separately.
> */
> - intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
> - PRE_CSC_GAMC_AUTO_INCREMENT);
> + PRE_CSC_GAMC_INDEX_VALUE(0));
> + intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
> + PRE_CSC_GAMC_AUTO_INCREMENT |
> + PRE_CSC_GAMC_INDEX_VALUE(0));
>
> for (i = 0; i < lut_size; i++) {
> u32 val = intel_de_read_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe));
> @@ -2888,7 +2901,8 @@ static struct drm_property_blob *glk_read_degamma_lut(struct intel_crtc *crtc)
> lut[i].blue = val;
> }
>
> - intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> + intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
> + PRE_CSC_GAMC_INDEX_VALUE(0));
>
> return blob;
> }
> @@ -2934,7 +2948,8 @@ icl_read_lut_multi_segment(struct intel_crtc *crtc)
> lut = blob->data;
>
> intel_de_write_fw(i915, PREC_PAL_MULTI_SEG_INDEX(pipe),
> - PAL_PREC_AUTO_INCREMENT);
> + PAL_PREC_MULTI_SEG_AUTO_INCREMENT |
> + PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
>
> for (i = 0; i < 9; i++) {
> u32 ldw = intel_de_read_fw(i915, PREC_PAL_MULTI_SEG_DATA(pipe));
> @@ -2943,7 +2958,8 @@ icl_read_lut_multi_segment(struct intel_crtc *crtc)
> ilk_lut_12p4_pack(&lut[i], ldw, udw);
> }
>
> - intel_de_write_fw(i915, PREC_PAL_MULTI_SEG_INDEX(pipe), 0);
> + intel_de_write_fw(i915, PREC_PAL_MULTI_SEG_INDEX(pipe),
> + PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
>
> /*
> * FIXME readouts from PAL_PREC_DATA register aren't giving
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 80ac50d80af4..22fb9fd78483 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7531,11 +7531,10 @@ enum skl_power_gate {
> #define _PAL_PREC_INDEX_A 0x4A400
> #define _PAL_PREC_INDEX_B 0x4AC00
> #define _PAL_PREC_INDEX_C 0x4B400
> -#define PAL_PREC_10_12_BIT (0 << 31)
> -#define PAL_PREC_SPLIT_MODE (1 << 31)
> -#define PAL_PREC_AUTO_INCREMENT (1 << 15)
> -#define PAL_PREC_INDEX_VALUE_MASK (0x3ff << 0)
> -#define PAL_PREC_INDEX_VALUE(x) ((x) << 0)
> +#define PAL_PREC_SPLIT_MODE REG_BIT(31)
> +#define PAL_PREC_AUTO_INCREMENT REG_BIT(15)
> +#define PAL_PREC_INDEX_VALUE_MASK REG_GENMASK(9, 0)
> +#define PAL_PREC_INDEX_VALUE(x) REG_FIELD_PREP(PAL_PREC_INDEX_VALUE_MASK, (x))
> #define _PAL_PREC_DATA_A 0x4A404
> #define _PAL_PREC_DATA_B 0x4AC04
> #define _PAL_PREC_DATA_C 0x4B404
> @@ -7559,7 +7558,9 @@ enum skl_power_gate {
> #define _PRE_CSC_GAMC_INDEX_A 0x4A484
> #define _PRE_CSC_GAMC_INDEX_B 0x4AC84
> #define _PRE_CSC_GAMC_INDEX_C 0x4B484
> -#define PRE_CSC_GAMC_AUTO_INCREMENT (1 << 10)
> +#define PRE_CSC_GAMC_AUTO_INCREMENT REG_BIT(10)
> +#define PRE_CSC_GAMC_INDEX_VALUE_MASK REG_GENMASK(7, 0)
PRE_CSC_GAMC_INDEX_VALUE_MASK till TGL seem to be using bits 0:5. For
ADL+ this seem to be 0:7 though. Would it make sense to use separate masks?
Regards,
Ankit
> +#define PRE_CSC_GAMC_INDEX_VALUE(x) REG_FIELD_PREP(PRE_CSC_GAMC_INDEX_VALUE_MASK, (x))
> #define _PRE_CSC_GAMC_DATA_A 0x4A488
> #define _PRE_CSC_GAMC_DATA_B 0x4AC88
> #define _PRE_CSC_GAMC_DATA_C 0x4B488
> @@ -7570,8 +7571,9 @@ enum skl_power_gate {
> /* ICL Multi segmented gamma */
> #define _PAL_PREC_MULTI_SEG_INDEX_A 0x4A408
> #define _PAL_PREC_MULTI_SEG_INDEX_B 0x4AC08
> -#define PAL_PREC_MULTI_SEGMENT_AUTO_INCREMENT REG_BIT(15)
> -#define PAL_PREC_MULTI_SEGMENT_INDEX_VALUE_MASK REG_GENMASK(4, 0)
> +#define PAL_PREC_MULTI_SEG_AUTO_INCREMENT REG_BIT(15)
> +#define PAL_PREC_MULTI_SEG_INDEX_VALUE_MASK REG_GENMASK(4, 0)
> +#define PAL_PREC_MULTI_SEG_INDEX_VALUE(x) REG_FIELD_PREP(PAL_PREC_MULTI_SEG_INDEX_VALUE_MASK, (x))
>
> #define _PAL_PREC_MULTI_SEG_DATA_A 0x4A40C
> #define _PAL_PREC_MULTI_SEG_DATA_B 0x4AC0C
next prev parent reply other threads:[~2022-12-01 5:46 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-23 15:26 [Intel-gfx] [PATCH 00/13] drm/i915: Gamma/DSB prep work Ville Syrjala
2022-11-23 15:26 ` [Intel-gfx] [PATCH 01/13] drm/i915: Shorten GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED a bit Ville Syrjala
2022-12-01 5:40 ` Nautiyal, Ankit K
2022-12-07 5:28 ` Shankar, Uma
2022-11-23 15:26 ` [Intel-gfx] [PATCH 02/13] drm/i915: Clean up GAMMA_MODE defines Ville Syrjala
2022-12-01 5:42 ` Nautiyal, Ankit K
2022-11-23 15:26 ` [Intel-gfx] [PATCH 03/13] drm/i915: Define skl+ palette anti-collision bit Ville Syrjala
2022-12-07 5:49 ` Shankar, Uma
2022-11-23 15:26 ` [Intel-gfx] [PATCH 04/13] drm/i915: Clean up various indexed LUT registers Ville Syrjala
2022-12-01 5:45 ` Nautiyal, Ankit K [this message]
2022-12-07 8:45 ` Shankar, Uma
2022-12-07 10:22 ` Shankar, Uma
2022-11-23 15:26 ` [Intel-gfx] [PATCH 05/13] drm/i915: Standardize auto-increment LUT load procedure Ville Syrjala
2022-12-01 5:48 ` Nautiyal, Ankit K
2022-12-07 9:06 ` Shankar, Uma
2022-11-23 15:26 ` [Intel-gfx] [PATCH 06/13] drm/i915: Document LUT "max" register precision Ville Syrjala
2022-12-01 5:53 ` Nautiyal, Ankit K
2022-12-07 9:07 ` Shankar, Uma
2022-11-23 15:26 ` [Intel-gfx] [PATCH 07/13] drm/i915: Move the DSB->mmio fallback into the LUT code Ville Syrjala
2022-12-01 6:05 ` Nautiyal, Ankit K
2022-12-07 9:15 ` Shankar, Uma
2022-11-23 15:26 ` [Intel-gfx] [PATCH 08/13] drm/i915: Move the DSB setup/cleaup into the color code Ville Syrjala
2022-12-07 9:29 ` Shankar, Uma
2022-11-23 15:26 ` [Intel-gfx] [PATCH 09/13] drm/i915: Make DSB lower level Ville Syrjala
2022-12-01 6:21 ` Nautiyal, Ankit K
2022-12-07 9:44 ` Shankar, Uma
2022-11-23 15:26 ` [Intel-gfx] [PATCH 10/13] drm/i915: Disable DSB usage specifically for LUTs Ville Syrjala
2022-12-01 6:24 ` Nautiyal, Ankit K
2022-12-07 9:51 ` Shankar, Uma
2022-11-23 15:26 ` [Intel-gfx] [PATCH 11/13] Revert "drm/i915: Disable DSB usage for now" Ville Syrjala
2022-12-01 6:26 ` Nautiyal, Ankit K
2022-12-07 9:53 ` Shankar, Uma
2022-11-23 15:26 ` [Intel-gfx] [PATCH 12/13] drm/i915: Use ilk_lut_write*() for all ilk+ gamma modes Ville Syrjala
2022-12-07 10:18 ` Shankar, Uma
2022-11-23 15:26 ` [Intel-gfx] [PATCH 13/13] drm/i915: Do state check for color management changes Ville Syrjala
2022-11-23 18:24 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Gamma/DSB prep work Patchwork
2022-11-23 18:24 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-11-23 18:24 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2022-11-23 18:43 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
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=23780f01-ab31-71e0-23fc-258b80e138ef@intel.com \
--to=ankit.k.nautiyal@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox