Re: [Intel-gfx] [PATCH 07/13] drm/i915: Move the DSB->mmio fallback into the LUT code
> -Original Message-
> From: Intel-gfx On Behalf Of Ville
> Syrjala
> Sent: Wednesday, November 23, 2022 8:57 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 07/13] drm/i915: Move the DSB->mmio fallback into
> the
> LUT code
>
> From: Ville Syrjälä
>
> The use of DSB has to be done differently on a case by case basis.
> So no way this kind of blind mmio fallback in the guts of the DSB code will
> work
> properly. Move it at least one level up into the LUT loading code. Not sure
> if this is
> the way we want do the DSB vs. mmio handling in the end, but at least it's a
> bit
> closer than what we had before.
Looks Good to me.
Reviewed-by: Uma Shankar
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/display/intel_color.c | 94 ++
> drivers/gpu/drm/i915/display/intel_dsb.c | 18 +
> 2 files changed, 62 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index bd7e781d9d07..5a4f794e1d08 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -836,6 +836,28 @@ static void i965_load_luts(const struct intel_crtc_state
> *crtc_state)
> }
> }
>
> +static void ilk_lut_write(const struct intel_crtc_state *crtc_state,
> + i915_reg_t reg, u32 val)
> +{
> + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +
> + if (crtc_state->dsb)
> + intel_dsb_reg_write(crtc_state, reg, val);
> + else
> + intel_de_write_fw(i915, reg, val);
> +}
> +
> +static void ilk_lut_write_indexed(const struct intel_crtc_state *crtc_state,
> + i915_reg_t reg, u32 val)
> +{
> + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +
> + if (crtc_state->dsb)
> + intel_dsb_indexed_reg_write(crtc_state, reg, val);
> + else
> + intel_de_write_fw(i915, reg, val);
> +}
> +
> static void ilk_load_lut_8(struct intel_crtc *crtc,
> const struct drm_property_blob *blob) { @@ -958,9
> +980,9 @@ static void ivb_load_lut_ext_max(const struct intel_crtc_state
> *crtc_state)
> enum pipe pipe = crtc->pipe;
>
> /* Program the max register to clamp values > 1.0. */
> - intel_dsb_reg_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
> - intel_dsb_reg_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
> - intel_dsb_reg_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
> + ilk_lut_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
> + ilk_lut_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
> + ilk_lut_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
> }
>
> static void glk_load_lut_ext2_max(const struct intel_crtc_state *crtc_state)
> @@ -
> 969,9 +991,9 @@ static void glk_load_lut_ext2_max(const struct
> intel_crtc_state
> *crtc_state)
> enum pipe pipe = crtc->pipe;
>
> /* Program the max register to clamp values > 1.0. */
> - intel_dsb_reg_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 0), 1 << 16);
> - intel_dsb_reg_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 1), 1 << 16);
> - intel_dsb_reg_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 2), 1 << 16);
> + ilk_lut_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 0), 1 << 16);
> + ilk_lut_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 1), 1 << 16);
> + ilk_lut_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 2), 1 << 16);
> }
>
> static void ivb_load_luts(const struct intel_crtc_state *crtc_state) @@
> -1118,9
> +1140,9 @@ ivb_load_lut_max(const struct intel_crtc_state *crtc_state,
> enum pipe pipe = crtc->pipe;
>
> /* FIXME LUT entries are 16 bit only, so we can prog 0x max */
> - intel_dsb_reg_write(crtc_state, PREC_PAL_GC_MAX(pipe, 0), color->red);
> - intel_dsb_reg_write(crtc_state, PREC_PAL_GC_MAX(pipe, 1), color->green);
> - intel_dsb_reg_write(crtc_state, PREC_PAL_GC_MAX(pipe, 2), color->blue);
> + ilk_lut_write(crtc_state, PREC_PAL_GC_MAX(pipe, 0), color->red);
> + ilk_lut_write(crtc_state, PREC_PAL_GC_MAX(pipe, 1), color->green);
> + ilk_lut_write(crtc_state, PREC_PAL_GC_MAX(pipe, 2), color->blue);
> }
>
> static void
> @@ -1139,23 +1161,23 @@ icl_program_gamma_superfine_segment(const struct
> intel_crtc_state *crtc_state)
>* 9 entries, corresponding to values 0, 1/(8 * 128 * 256),
Re: [Intel-gfx] [PATCH 07/13] drm/i915: Move the DSB->mmio fallback into the LUT code
Makes sense to me.
Reviewed-by: Ankit Nautiyal
On 11/23/2022 8:56 PM, Ville Syrjala wrote:
From: Ville Syrjälä
The use of DSB has to be done differently on a case by case basis.
So no way this kind of blind mmio fallback in the guts of the DSB
code will work properly. Move it at least one level up into the
LUT loading code. Not sure if this is the way we want do the
DSB vs. mmio handling in the end, but at least it's a bit
closer than what we had before.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_color.c | 94 ++
drivers/gpu/drm/i915/display/intel_dsb.c | 18 +
2 files changed, 62 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c
b/drivers/gpu/drm/i915/display/intel_color.c
index bd7e781d9d07..5a4f794e1d08 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -836,6 +836,28 @@ static void i965_load_luts(const struct intel_crtc_state
*crtc_state)
}
}
+static void ilk_lut_write(const struct intel_crtc_state *crtc_state,
+ i915_reg_t reg, u32 val)
+{
+ struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+
+ if (crtc_state->dsb)
+ intel_dsb_reg_write(crtc_state, reg, val);
+ else
+ intel_de_write_fw(i915, reg, val);
+}
+
+static void ilk_lut_write_indexed(const struct intel_crtc_state *crtc_state,
+ i915_reg_t reg, u32 val)
+{
+ struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+
+ if (crtc_state->dsb)
+ intel_dsb_indexed_reg_write(crtc_state, reg, val);
+ else
+ intel_de_write_fw(i915, reg, val);
+}
+
static void ilk_load_lut_8(struct intel_crtc *crtc,
const struct drm_property_blob *blob)
{
@@ -958,9 +980,9 @@ static void ivb_load_lut_ext_max(const struct
intel_crtc_state *crtc_state)
enum pipe pipe = crtc->pipe;
/* Program the max register to clamp values > 1.0. */
- intel_dsb_reg_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
- intel_dsb_reg_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
- intel_dsb_reg_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
+ ilk_lut_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
+ ilk_lut_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
+ ilk_lut_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
}
static void glk_load_lut_ext2_max(const struct intel_crtc_state *crtc_state)
@@ -969,9 +991,9 @@ static void glk_load_lut_ext2_max(const struct
intel_crtc_state *crtc_state)
enum pipe pipe = crtc->pipe;
/* Program the max register to clamp values > 1.0. */
- intel_dsb_reg_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 0), 1 << 16);
- intel_dsb_reg_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 1), 1 << 16);
- intel_dsb_reg_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 2), 1 << 16);
+ ilk_lut_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 0), 1 << 16);
+ ilk_lut_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 1), 1 << 16);
+ ilk_lut_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 2), 1 << 16);
}
static void ivb_load_luts(const struct intel_crtc_state *crtc_state)
@@ -1118,9 +1140,9 @@ ivb_load_lut_max(const struct intel_crtc_state
*crtc_state,
enum pipe pipe = crtc->pipe;
/* FIXME LUT entries are 16 bit only, so we can prog 0x max */
- intel_dsb_reg_write(crtc_state, PREC_PAL_GC_MAX(pipe, 0), color->red);
- intel_dsb_reg_write(crtc_state, PREC_PAL_GC_MAX(pipe, 1), color->green);
- intel_dsb_reg_write(crtc_state, PREC_PAL_GC_MAX(pipe, 2), color->blue);
+ ilk_lut_write(crtc_state, PREC_PAL_GC_MAX(pipe, 0), color->red);
+ ilk_lut_write(crtc_state, PREC_PAL_GC_MAX(pipe, 1), color->green);
+ ilk_lut_write(crtc_state, PREC_PAL_GC_MAX(pipe, 2), color->blue);
}
static void
@@ -1139,23 +1161,23 @@ icl_program_gamma_superfine_segment(const struct
intel_crtc_state *crtc_state)
* 9 entries, corresponding to values 0, 1/(8 * 128 * 256),
* 2/(8 * 128 * 256) ... 8/(8 * 128 * 256).
*/
- intel_dsb_reg_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
- PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
- intel_dsb_reg_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
- PAL_PREC_AUTO_INCREMENT |
- PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
+ ilk_lut_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
+ PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
+ ilk_lut_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
+ PAL_PREC_AUTO_INCREMENT |
+ PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
for (i = 0; i < 9; i++) {
const struct drm_color_l
[Intel-gfx] [PATCH 07/13] drm/i915: Move the DSB->mmio fallback into the LUT code
From: Ville Syrjälä
The use of DSB has to be done differently on a case by case basis.
So no way this kind of blind mmio fallback in the guts of the DSB
code will work properly. Move it at least one level up into the
LUT loading code. Not sure if this is the way we want do the
DSB vs. mmio handling in the end, but at least it's a bit
closer than what we had before.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_color.c | 94 ++
drivers/gpu/drm/i915/display/intel_dsb.c | 18 +
2 files changed, 62 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c
b/drivers/gpu/drm/i915/display/intel_color.c
index bd7e781d9d07..5a4f794e1d08 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -836,6 +836,28 @@ static void i965_load_luts(const struct intel_crtc_state
*crtc_state)
}
}
+static void ilk_lut_write(const struct intel_crtc_state *crtc_state,
+ i915_reg_t reg, u32 val)
+{
+ struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+
+ if (crtc_state->dsb)
+ intel_dsb_reg_write(crtc_state, reg, val);
+ else
+ intel_de_write_fw(i915, reg, val);
+}
+
+static void ilk_lut_write_indexed(const struct intel_crtc_state *crtc_state,
+ i915_reg_t reg, u32 val)
+{
+ struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+
+ if (crtc_state->dsb)
+ intel_dsb_indexed_reg_write(crtc_state, reg, val);
+ else
+ intel_de_write_fw(i915, reg, val);
+}
+
static void ilk_load_lut_8(struct intel_crtc *crtc,
const struct drm_property_blob *blob)
{
@@ -958,9 +980,9 @@ static void ivb_load_lut_ext_max(const struct
intel_crtc_state *crtc_state)
enum pipe pipe = crtc->pipe;
/* Program the max register to clamp values > 1.0. */
- intel_dsb_reg_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
- intel_dsb_reg_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
- intel_dsb_reg_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
+ ilk_lut_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
+ ilk_lut_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
+ ilk_lut_write(crtc_state, PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
}
static void glk_load_lut_ext2_max(const struct intel_crtc_state *crtc_state)
@@ -969,9 +991,9 @@ static void glk_load_lut_ext2_max(const struct
intel_crtc_state *crtc_state)
enum pipe pipe = crtc->pipe;
/* Program the max register to clamp values > 1.0. */
- intel_dsb_reg_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 0), 1 << 16);
- intel_dsb_reg_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 1), 1 << 16);
- intel_dsb_reg_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 2), 1 << 16);
+ ilk_lut_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 0), 1 << 16);
+ ilk_lut_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 1), 1 << 16);
+ ilk_lut_write(crtc_state, PREC_PAL_EXT2_GC_MAX(pipe, 2), 1 << 16);
}
static void ivb_load_luts(const struct intel_crtc_state *crtc_state)
@@ -1118,9 +1140,9 @@ ivb_load_lut_max(const struct intel_crtc_state
*crtc_state,
enum pipe pipe = crtc->pipe;
/* FIXME LUT entries are 16 bit only, so we can prog 0x max */
- intel_dsb_reg_write(crtc_state, PREC_PAL_GC_MAX(pipe, 0), color->red);
- intel_dsb_reg_write(crtc_state, PREC_PAL_GC_MAX(pipe, 1), color->green);
- intel_dsb_reg_write(crtc_state, PREC_PAL_GC_MAX(pipe, 2), color->blue);
+ ilk_lut_write(crtc_state, PREC_PAL_GC_MAX(pipe, 0), color->red);
+ ilk_lut_write(crtc_state, PREC_PAL_GC_MAX(pipe, 1), color->green);
+ ilk_lut_write(crtc_state, PREC_PAL_GC_MAX(pipe, 2), color->blue);
}
static void
@@ -1139,23 +1161,23 @@ icl_program_gamma_superfine_segment(const struct
intel_crtc_state *crtc_state)
* 9 entries, corresponding to values 0, 1/(8 * 128 * 256),
* 2/(8 * 128 * 256) ... 8/(8 * 128 * 256).
*/
- intel_dsb_reg_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
- PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
- intel_dsb_reg_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
- PAL_PREC_AUTO_INCREMENT |
- PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
+ ilk_lut_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
+ PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
+ ilk_lut_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
+ PAL_PREC_AUTO_INCREMENT |
+ PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
for (i = 0; i < 9; i++) {
const struct drm_color_lut *entry = &lut[i];
- intel_dsb_indexed_reg_write(crtc_state,
PREC_PAL_MULTI_SEG_DATA(pi
