Re: [Intel-gfx] [PATCH 06/12] drm/i915: Utilize crtc_state->csc on chv

2023-04-06 Thread Nautiyal, Ankit K



On 3/29/2023 7:19 PM, Ville Syrjala wrote:

From: Ville Syrjälä 

Store the chv cgm csc matrix in the crtc state as well. We
shall store it in the same place where we store the ilk+
pipe csc matrix (as opposed to the output csc matrix).

Signed-off-by: Ville Syrjälä 
---
  drivers/gpu/drm/i915/display/intel_color.c | 35 +-
  1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
b/drivers/gpu/drm/i915/display/intel_color.c
index a6f7eba59e12..7e8820583942 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -431,10 +431,10 @@ static void icl_load_csc_matrix(const struct 
intel_crtc_state *crtc_state)
icl_update_output_csc(crtc, _state->output_csc);
  }
  
-static void chv_cgm_csc_convert_ctm(struct intel_csc_matrix *csc,

-   const struct drm_property_blob *blob)
+static void chv_cgm_csc_convert_ctm(const struct intel_crtc_state *crtc_state,
+   struct intel_csc_matrix *csc)
  {
-   const struct drm_color_ctm *ctm = blob->data;
+   const struct drm_color_ctm *ctm = crtc_state->hw.ctm->data;
int i;
  
  	for (i = 0; i < 9; i++) {

@@ -457,24 +457,30 @@ static void chv_cgm_csc_convert_ctm(struct 
intel_csc_matrix *csc,
  }
  
  static void chv_load_cgm_csc(struct intel_crtc *crtc,

-const struct drm_property_blob *blob)
+const struct intel_csc_matrix *csc)
  {
struct drm_i915_private *i915 = to_i915(crtc->base.dev);
enum pipe pipe = crtc->pipe;
-   struct intel_csc_matrix tmp;
-
-   chv_cgm_csc_convert_ctm(, blob);
  
  	intel_de_write_fw(i915, CGM_PIPE_CSC_COEFF01(pipe),

- tmp.coeff[1] << 16 | tmp.coeff[0]);
+ csc->coeff[1] << 16 | csc->coeff[0]);
intel_de_write_fw(i915, CGM_PIPE_CSC_COEFF23(pipe),
- tmp.coeff[3] << 16 | tmp.coeff[2]);
+ csc->coeff[3] << 16 | csc->coeff[2]);
intel_de_write_fw(i915, CGM_PIPE_CSC_COEFF45(pipe),
- tmp.coeff[5] << 16 | tmp.coeff[4]);
+ csc->coeff[5] << 16 | csc->coeff[4]);
intel_de_write_fw(i915, CGM_PIPE_CSC_COEFF67(pipe),
- tmp.coeff[7] << 16 | tmp.coeff[6]);
+ csc->coeff[7] << 16 | csc->coeff[6]);
intel_de_write_fw(i915, CGM_PIPE_CSC_COEFF8(pipe),
- tmp.coeff[8]);
+ csc->coeff[8]);
+}
+
+static void chv_assign_csc(struct intel_crtc_state *crtc_state)
+{
+   if (crtc_state->hw.ctm) {
+   chv_cgm_csc_convert_ctm(crtc_state, _state->csc);
+   } else {
+   intel_csc_clear(_state->csc);
+   }


It does make sense to have a block for csc and another for output_csc, 
analogous to the load part.


We are anyway adding drm_Warn later, so braces can be dropped here and 
introduced in patch adding warn_ons


Otherwise the Patch looks good to me.

Reviewed-by: Ankit Nautiyal 



  }
  
  /* convert hw value with given bit_precision to lut property val */

@@ -1442,10 +1448,9 @@ static void chv_load_luts(const struct intel_crtc_state 
*crtc_state)
struct drm_i915_private *i915 = to_i915(crtc->base.dev);
const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
-   const struct drm_property_blob *ctm = crtc_state->hw.ctm;
  
  	if (crtc_state->cgm_mode & CGM_PIPE_MODE_CSC)

-   chv_load_cgm_csc(crtc, ctm);
+   chv_load_cgm_csc(crtc, _state->csc);
  
  	if (crtc_state->cgm_mode & CGM_PIPE_MODE_DEGAMMA)

chv_load_cgm_degamma(crtc, pre_csc_lut);
@@ -1872,6 +1877,8 @@ static int chv_color_check(struct intel_crtc_state 
*crtc_state)
  
  	intel_assign_luts(crtc_state);
  
+	chv_assign_csc(crtc_state);

+
crtc_state->preload_luts = chv_can_preload_luts(crtc_state);
  
  	return 0;


[Intel-gfx] [PATCH 06/12] drm/i915: Utilize crtc_state->csc on chv

2023-03-29 Thread Ville Syrjala
From: Ville Syrjälä 

Store the chv cgm csc matrix in the crtc state as well. We
shall store it in the same place where we store the ilk+
pipe csc matrix (as opposed to the output csc matrix).

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/intel_color.c | 35 +-
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
b/drivers/gpu/drm/i915/display/intel_color.c
index a6f7eba59e12..7e8820583942 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -431,10 +431,10 @@ static void icl_load_csc_matrix(const struct 
intel_crtc_state *crtc_state)
icl_update_output_csc(crtc, _state->output_csc);
 }
 
-static void chv_cgm_csc_convert_ctm(struct intel_csc_matrix *csc,
-   const struct drm_property_blob *blob)
+static void chv_cgm_csc_convert_ctm(const struct intel_crtc_state *crtc_state,
+   struct intel_csc_matrix *csc)
 {
-   const struct drm_color_ctm *ctm = blob->data;
+   const struct drm_color_ctm *ctm = crtc_state->hw.ctm->data;
int i;
 
for (i = 0; i < 9; i++) {
@@ -457,24 +457,30 @@ static void chv_cgm_csc_convert_ctm(struct 
intel_csc_matrix *csc,
 }
 
 static void chv_load_cgm_csc(struct intel_crtc *crtc,
-const struct drm_property_blob *blob)
+const struct intel_csc_matrix *csc)
 {
struct drm_i915_private *i915 = to_i915(crtc->base.dev);
enum pipe pipe = crtc->pipe;
-   struct intel_csc_matrix tmp;
-
-   chv_cgm_csc_convert_ctm(, blob);
 
intel_de_write_fw(i915, CGM_PIPE_CSC_COEFF01(pipe),
- tmp.coeff[1] << 16 | tmp.coeff[0]);
+ csc->coeff[1] << 16 | csc->coeff[0]);
intel_de_write_fw(i915, CGM_PIPE_CSC_COEFF23(pipe),
- tmp.coeff[3] << 16 | tmp.coeff[2]);
+ csc->coeff[3] << 16 | csc->coeff[2]);
intel_de_write_fw(i915, CGM_PIPE_CSC_COEFF45(pipe),
- tmp.coeff[5] << 16 | tmp.coeff[4]);
+ csc->coeff[5] << 16 | csc->coeff[4]);
intel_de_write_fw(i915, CGM_PIPE_CSC_COEFF67(pipe),
- tmp.coeff[7] << 16 | tmp.coeff[6]);
+ csc->coeff[7] << 16 | csc->coeff[6]);
intel_de_write_fw(i915, CGM_PIPE_CSC_COEFF8(pipe),
- tmp.coeff[8]);
+ csc->coeff[8]);
+}
+
+static void chv_assign_csc(struct intel_crtc_state *crtc_state)
+{
+   if (crtc_state->hw.ctm) {
+   chv_cgm_csc_convert_ctm(crtc_state, _state->csc);
+   } else {
+   intel_csc_clear(_state->csc);
+   }
 }
 
 /* convert hw value with given bit_precision to lut property val */
@@ -1442,10 +1448,9 @@ static void chv_load_luts(const struct intel_crtc_state 
*crtc_state)
struct drm_i915_private *i915 = to_i915(crtc->base.dev);
const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
-   const struct drm_property_blob *ctm = crtc_state->hw.ctm;
 
if (crtc_state->cgm_mode & CGM_PIPE_MODE_CSC)
-   chv_load_cgm_csc(crtc, ctm);
+   chv_load_cgm_csc(crtc, _state->csc);
 
if (crtc_state->cgm_mode & CGM_PIPE_MODE_DEGAMMA)
chv_load_cgm_degamma(crtc, pre_csc_lut);
@@ -1872,6 +1877,8 @@ static int chv_color_check(struct intel_crtc_state 
*crtc_state)
 
intel_assign_luts(crtc_state);
 
+   chv_assign_csc(crtc_state);
+
crtc_state->preload_luts = chv_can_preload_luts(crtc_state);
 
return 0;
-- 
2.39.2