Re: [Intel-gfx] [PATCH 07/12] drm/i915: Sprinke a few sanity check WARNS during csc assignment

2023-04-06 Thread Nautiyal, Ankit K 

LGTM.

Reviewed-by: Ankit Nautiyal 

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

From: Ville Syrjälä 

Make sure the csc enable bit(s) match the way we're about to
fill the csc matrices.

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

diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
b/drivers/gpu/drm/i915/display/intel_color.c
index 7e8820583942..2988c91d8ff6 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -373,10 +373,16 @@ static void ilk_assign_csc(struct intel_crtc_state 
*crtc_state)
bool limited_color_range = ilk_csc_limited_range(crtc_state);
  
  	if (crtc_state->hw.ctm) {

+   drm_WARN_ON(&i915->drm, !crtc_state->csc_enable);
+
ilk_csc_convert_ctm(crtc_state, &crtc_state->csc, 
limited_color_range);
} else if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) {
+   drm_WARN_ON(&i915->drm, !crtc_state->csc_enable);
+
ilk_csc_copy(i915, &crtc_state->csc, 
&ilk_csc_matrix_rgb_to_ycbcr);
} else if (limited_color_range) {
+   drm_WARN_ON(&i915->drm, !crtc_state->csc_enable);
+
ilk_csc_copy(i915, &crtc_state->csc, 
&ilk_csc_matrix_limited_range);
} else if (crtc_state->csc_enable) {
/*
@@ -406,16 +412,26 @@ static void icl_assign_csc(struct intel_crtc_state 
*crtc_state)
struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
  
  	if (crtc_state->hw.ctm) {

+   drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & ICL_CSC_ENABLE) 
== 0);
+
ilk_csc_convert_ctm(crtc_state, &crtc_state->csc, false);
} else {
+   drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & ICL_CSC_ENABLE) 
!= 0);
+
intel_csc_clear(&crtc_state->csc);
}
  
  	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) {

+   drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & 
ICL_OUTPUT_CSC_ENABLE) == 0);
+
ilk_csc_copy(i915, &crtc_state->output_csc, 
&ilk_csc_matrix_rgb_to_ycbcr);
} else if (crtc_state->limited_color_range) {
+   drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & 
ICL_OUTPUT_CSC_ENABLE) == 0);
+
ilk_csc_copy(i915, &crtc_state->output_csc, 
&ilk_csc_matrix_limited_range);
} else {
+   drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & 
ICL_OUTPUT_CSC_ENABLE) != 0);
+
intel_csc_clear(&crtc_state->output_csc);
}
  }
@@ -476,9 +492,15 @@ static void chv_load_cgm_csc(struct intel_crtc *crtc,
  
  static void chv_assign_csc(struct intel_crtc_state *crtc_state)

  {
+   struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+
if (crtc_state->hw.ctm) {
+   drm_WARN_ON(&i915->drm, (crtc_state->cgm_mode & 
CGM_PIPE_MODE_CSC) == 0);
+
chv_cgm_csc_convert_ctm(crtc_state, &crtc_state->csc);
} else {
+   drm_WARN_ON(&i915->drm, (crtc_state->cgm_mode & 
CGM_PIPE_MODE_CSC) != 0);
+
intel_csc_clear(&crtc_state->csc);
}
  }

[Intel-gfx] [PATCH 07/12] drm/i915: Sprinke a few sanity check WARNS during csc assignment

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

Make sure the csc enable bit(s) match the way we're about to
fill the csc matrices.

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

diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
b/drivers/gpu/drm/i915/display/intel_color.c
index 7e8820583942..2988c91d8ff6 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -373,10 +373,16 @@ static void ilk_assign_csc(struct intel_crtc_state 
*crtc_state)
bool limited_color_range = ilk_csc_limited_range(crtc_state);
 
if (crtc_state->hw.ctm) {
+   drm_WARN_ON(&i915->drm, !crtc_state->csc_enable);
+
ilk_csc_convert_ctm(crtc_state, &crtc_state->csc, 
limited_color_range);
} else if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) {
+   drm_WARN_ON(&i915->drm, !crtc_state->csc_enable);
+
ilk_csc_copy(i915, &crtc_state->csc, 
&ilk_csc_matrix_rgb_to_ycbcr);
} else if (limited_color_range) {
+   drm_WARN_ON(&i915->drm, !crtc_state->csc_enable);
+
ilk_csc_copy(i915, &crtc_state->csc, 
&ilk_csc_matrix_limited_range);
} else if (crtc_state->csc_enable) {
/*
@@ -406,16 +412,26 @@ static void icl_assign_csc(struct intel_crtc_state 
*crtc_state)
struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
 
if (crtc_state->hw.ctm) {
+   drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & ICL_CSC_ENABLE) 
== 0);
+
ilk_csc_convert_ctm(crtc_state, &crtc_state->csc, false);
} else {
+   drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & ICL_CSC_ENABLE) 
!= 0);
+
intel_csc_clear(&crtc_state->csc);
}
 
if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) {
+   drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & 
ICL_OUTPUT_CSC_ENABLE) == 0);
+
ilk_csc_copy(i915, &crtc_state->output_csc, 
&ilk_csc_matrix_rgb_to_ycbcr);
} else if (crtc_state->limited_color_range) {
+   drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & 
ICL_OUTPUT_CSC_ENABLE) == 0);
+
ilk_csc_copy(i915, &crtc_state->output_csc, 
&ilk_csc_matrix_limited_range);
} else {
+   drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & 
ICL_OUTPUT_CSC_ENABLE) != 0);
+
intel_csc_clear(&crtc_state->output_csc);
}
 }
@@ -476,9 +492,15 @@ static void chv_load_cgm_csc(struct intel_crtc *crtc,
 
 static void chv_assign_csc(struct intel_crtc_state *crtc_state)
 {
+   struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+
if (crtc_state->hw.ctm) {
+   drm_WARN_ON(&i915->drm, (crtc_state->cgm_mode & 
CGM_PIPE_MODE_CSC) == 0);
+
chv_cgm_csc_convert_ctm(crtc_state, &crtc_state->csc);
} else {
+   drm_WARN_ON(&i915->drm, (crtc_state->cgm_mode & 
CGM_PIPE_MODE_CSC) != 0);
+
intel_csc_clear(&crtc_state->csc);
}
 }
-- 
2.39.2