Re: [Intel-gfx] [PATCH 05/13] drm/i915: Standardize auto-increment LUT load procedure
> -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 05/13] drm/i915: Standardize auto-increment LUT
> load
> procedure
>
> From: Ville Syrjälä
>
> Various gamma units on various platforms have some problems loading the LUT
> index and auto-increment bit at the same time. We have to do this in two
> steps. The
> first known case was the glk degamma LUT, but at least ADL has another known
> case.
>
> We're not going to suffer too badly from a couple of extra register writes
> here, so
> let's just standardize on this practice for all auto-increment LUT
> loads/reads. This
> way we never have to worry about this specific issue again. And for good
> measure
> always reset the index back to zero at the end (we already did this in a few
> places).
Looks Good to me.
Reviewed-by: Uma Shankar
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/display/intel_color.c | 19 ++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index c960c2aaf328..bd7e781d9d07 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -934,6 +934,8 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
> int i, lut_size = drm_color_lut_size(blob);
> enum pipe pipe = crtc->pipe;
>
> + intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> + prec_index);
> intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> PAL_PREC_AUTO_INCREMENT |
> prec_index);
> @@ -1138,7 +1140,10 @@ icl_program_gamma_superfine_segment(const struct
> intel_crtc_state *crtc_state)
>* 2/(8 * 128 * 256) ... 8/(8 * 128 * 256).
>*/
> intel_dsb_reg_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
> - PAL_PREC_AUTO_INCREMENT);
> + 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));
>
> for (i = 0; i < 9; i++) {
> const struct drm_color_lut *entry = [i]; @@ -1148,6 +1153,9
> @@ icl_program_gamma_superfine_segment(const struct intel_crtc_state
> *crtc_state)
> intel_dsb_indexed_reg_write(crtc_state,
> PREC_PAL_MULTI_SEG_DATA(pipe),
> ilk_lut_12p4_udw(entry));
> }
> +
> + intel_dsb_reg_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
> + PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
> }
>
> static void
> @@ -1170,6 +1178,8 @@ icl_program_gamma_multi_segment(const struct
> intel_crtc_state *crtc_state)
>* PAL_PREC_INDEX[0] and PAL_PREC_INDEX[1] map to seg2[1],
>* seg2[0] being unused by the hardware.
>*/
> + intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
> + PAL_PREC_INDEX_VALUE(0));
> intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
> PAL_PREC_AUTO_INCREMENT |
> PAL_PREC_INDEX_VALUE(0));
> @@ -1202,6 +1212,9 @@ icl_program_gamma_multi_segment(const struct
> intel_crtc_state *crtc_state)
> ilk_lut_12p4_udw(entry));
> }
>
> + intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
> + PAL_PREC_INDEX_VALUE(0));
> +
> /* The last entry in the LUT is to be programmed in GCMAX */
> entry = [256 * 8 * 128];
> ivb_load_lut_max(crtc_state, entry);
> @@ -2819,6 +2832,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);
> intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> PAL_PREC_AUTO_INCREMENT |
> prec_index);
> @@ -2947,6 +2962,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_MULTI_SEG_INDEX_VALUE(0));
> intel_de_write_fw(i915, PREC_PAL_MULTI_SEG_INDEX(pipe),
> PAL_PREC_MULTI_SEG_AUTO_INCREMENT |
> PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
> --
> 2.37.4
Re: [Intel-gfx] [PATCH 05/13] drm/i915: Standardize auto-increment LUT load procedure
LGTM.
Reviewed-by: Ankit Nautiyal
On 11/23/2022 8:56 PM, Ville Syrjala wrote:
From: Ville Syrjälä
Various gamma units on various platforms have some problems loading
the LUT index and auto-increment bit at the same time. We have to
do this in two steps. The first known case was the glk degamma LUT,
but at least ADL has another known case.
We're not going to suffer too badly from a couple of extra register
writes here, so let's just standardize on this practice for all
auto-increment LUT loads/reads. This way we never have to worry about
this specific issue again. And for good measure always reset the
index back to zero at the end (we already did this in a few places).
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_color.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c
b/drivers/gpu/drm/i915/display/intel_color.c
index c960c2aaf328..bd7e781d9d07 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -934,6 +934,8 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
int i, lut_size = drm_color_lut_size(blob);
enum pipe pipe = crtc->pipe;
+ intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
+ prec_index);
intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
PAL_PREC_AUTO_INCREMENT |
prec_index);
@@ -1138,7 +1140,10 @@ icl_program_gamma_superfine_segment(const struct
intel_crtc_state *crtc_state)
* 2/(8 * 128 * 256) ... 8/(8 * 128 * 256).
*/
intel_dsb_reg_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
- PAL_PREC_AUTO_INCREMENT);
+ 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));
for (i = 0; i < 9; i++) {
const struct drm_color_lut *entry = [i];
@@ -1148,6 +1153,9 @@ icl_program_gamma_superfine_segment(const struct
intel_crtc_state *crtc_state)
intel_dsb_indexed_reg_write(crtc_state,
PREC_PAL_MULTI_SEG_DATA(pipe),
ilk_lut_12p4_udw(entry));
}
+
+ intel_dsb_reg_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
+ PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
}
static void
@@ -1170,6 +1178,8 @@ icl_program_gamma_multi_segment(const struct
intel_crtc_state *crtc_state)
* PAL_PREC_INDEX[0] and PAL_PREC_INDEX[1] map to seg2[1],
* seg2[0] being unused by the hardware.
*/
+ intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
+ PAL_PREC_INDEX_VALUE(0));
intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
PAL_PREC_AUTO_INCREMENT |
PAL_PREC_INDEX_VALUE(0));
@@ -1202,6 +1212,9 @@ icl_program_gamma_multi_segment(const struct
intel_crtc_state *crtc_state)
ilk_lut_12p4_udw(entry));
}
+ intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
+ PAL_PREC_INDEX_VALUE(0));
+
/* The last entry in the LUT is to be programmed in GCMAX */
entry = [256 * 8 * 128];
ivb_load_lut_max(crtc_state, entry);
@@ -2819,6 +2832,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);
intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
PAL_PREC_AUTO_INCREMENT |
prec_index);
@@ -2947,6 +2962,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_MULTI_SEG_INDEX_VALUE(0));
intel_de_write_fw(i915, PREC_PAL_MULTI_SEG_INDEX(pipe),
PAL_PREC_MULTI_SEG_AUTO_INCREMENT |
PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
[Intel-gfx] [PATCH 05/13] drm/i915: Standardize auto-increment LUT load procedure
From: Ville Syrjälä
Various gamma units on various platforms have some problems loading
the LUT index and auto-increment bit at the same time. We have to
do this in two steps. The first known case was the glk degamma LUT,
but at least ADL has another known case.
We're not going to suffer too badly from a couple of extra register
writes here, so let's just standardize on this practice for all
auto-increment LUT loads/reads. This way we never have to worry about
this specific issue again. And for good measure always reset the
index back to zero at the end (we already did this in a few places).
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_color.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c
b/drivers/gpu/drm/i915/display/intel_color.c
index c960c2aaf328..bd7e781d9d07 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -934,6 +934,8 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
int i, lut_size = drm_color_lut_size(blob);
enum pipe pipe = crtc->pipe;
+ intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
+ prec_index);
intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
PAL_PREC_AUTO_INCREMENT |
prec_index);
@@ -1138,7 +1140,10 @@ icl_program_gamma_superfine_segment(const struct
intel_crtc_state *crtc_state)
* 2/(8 * 128 * 256) ... 8/(8 * 128 * 256).
*/
intel_dsb_reg_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
- PAL_PREC_AUTO_INCREMENT);
+ 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));
for (i = 0; i < 9; i++) {
const struct drm_color_lut *entry = [i];
@@ -1148,6 +1153,9 @@ icl_program_gamma_superfine_segment(const struct
intel_crtc_state *crtc_state)
intel_dsb_indexed_reg_write(crtc_state,
PREC_PAL_MULTI_SEG_DATA(pipe),
ilk_lut_12p4_udw(entry));
}
+
+ intel_dsb_reg_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
+ PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
}
static void
@@ -1170,6 +1178,8 @@ icl_program_gamma_multi_segment(const struct
intel_crtc_state *crtc_state)
* PAL_PREC_INDEX[0] and PAL_PREC_INDEX[1] map to seg2[1],
* seg2[0] being unused by the hardware.
*/
+ intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
+ PAL_PREC_INDEX_VALUE(0));
intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
PAL_PREC_AUTO_INCREMENT |
PAL_PREC_INDEX_VALUE(0));
@@ -1202,6 +1212,9 @@ icl_program_gamma_multi_segment(const struct
intel_crtc_state *crtc_state)
ilk_lut_12p4_udw(entry));
}
+ intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
+ PAL_PREC_INDEX_VALUE(0));
+
/* The last entry in the LUT is to be programmed in GCMAX */
entry = [256 * 8 * 128];
ivb_load_lut_max(crtc_state, entry);
@@ -2819,6 +2832,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);
intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
PAL_PREC_AUTO_INCREMENT |
prec_index);
@@ -2947,6 +2962,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_MULTI_SEG_INDEX_VALUE(0));
intel_de_write_fw(i915, PREC_PAL_MULTI_SEG_INDEX(pipe),
PAL_PREC_MULTI_SEG_AUTO_INCREMENT |
PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
--
2.37.4
