On Sat, 2015-10-10 at 00:59 +0530, Shashank Sharma wrote:
BDW/SKL/BXT support Color Space Conversion (CSC) using a 3x3 matrix that needs to be programmed into respective CSC registers.
This patch does the following:
- Adds the core function to program CSC correction values for BDW/SKL/BXT platform
- Adds CSC correction macros/defines
Signed-off-by: Shashank Sharma shashank.sharma@intel.com Signed-off-by: Kausal Malladi kausalmalladi@gmail.com Signed-off-by: Kumar, Kiran S kiran.s.kumar@intel.com
drivers/gpu/drm/i915/i915_reg.h | 7 ++ drivers/gpu/drm/i915/intel_color_manager.c | 114 ++++++++++++++++++++++++++++- drivers/gpu/drm/i915/intel_color_manager.h | 12 ++- 3 files changed, 129 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ed50f75..0e9d252 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8085,4 +8085,11 @@ enum skl_disp_power_wells { (_PIPE3(pipe, PAL_PREC_GCMAX_A, PAL_PREC_GCMAX_B, PAL_PREC_GCMAX_C))
+/* BDW CSC correction */ +#define CSC_COEFF_A 0x49010 +#define CSC_COEFF_B 0x49110 +#define CSC_COEFF_C 0x49210 +#define _PIPE_CSC_COEFF(pipe) \
- (_PIPE3(pipe, CSC_COEFF_A, CSC_COEFF_B, CSC_COEFF_C))
#endif /* _I915_REG_H_ */ diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c index e659382..0a6c00c 100644 --- a/drivers/gpu/drm/i915/intel_color_manager.c +++ b/drivers/gpu/drm/i915/intel_color_manager.c @@ -330,11 +330,119 @@ static int bdw_set_degamma(struct drm_device *dev, return 0; }
-static s16 chv_prepare_csc_coeff(s64 csc_value) +static uint32_t bdw_prepare_csc_coeff(int64_t coeff) +{
- uint32_t reg_val, ls_bit_pos, exponent_bits, sign_bit = 0;
- int32_t mantissa;
- uint64_t abs_coeff;
- coeff = min_t(int64_t, coeff, BDW_CSC_COEFF_MAX_VAL);
- coeff = max_t(int64_t, coeff, BDW_CSC_COEFF_MIN_VAL);
- abs_coeff = abs(coeff);
- if (abs_coeff < (BDW_CSC_COEFF_UNITY_VAL >> 3)) {
/* abs_coeff < 0.125 */exponent_bits = 3;ls_bit_pos = 19;- } else if (abs_coeff >= (BDW_CSC_COEFF_UNITY_VAL >> 3) &&
abs_coeff < (BDW_CSC_COEFF_UNITY_VAL >> 2)) {/* abs_coeff >= 0.125 && val < 0.25 */exponent_bits = 2;ls_bit_pos = 20;- } else if (abs_coeff >= (BDW_CSC_COEFF_UNITY_VAL >> 2)
&& abs_coeff < (BDW_CSC_COEFF_UNITY_VAL >> 1)) {/* abs_coeff >= 0.25 && val < 0.5 */exponent_bits = 1;ls_bit_pos = 21;- } else if (abs_coeff >= (BDW_CSC_COEFF_UNITY_VAL >> 1)
&& abs_coeff < BDW_CSC_COEFF_UNITY_VAL) {/* abs_coeff >= 0.5 && val < 1.0 */exponent_bits = 0;ls_bit_pos = 22;- } else if (abs_coeff >= BDW_CSC_COEFF_UNITY_VAL &&
abs_coeff < (BDW_CSC_COEFF_UNITY_VAL << 1)) {/* abs_coeff >= 1.0 && val < 2.0 */exponent_bits = 7;ls_bit_pos = 23;- } else {
/* abs_coeff >= 2.0 && val < 4.0 */exponent_bits = 6;ls_bit_pos = 24;- }
- mantissa = GET_BITS_ROUNDOFF(abs_coeff, ls_bit_pos,
CSC_MAX_VALS);
Is GET_BITS_ROUNDOFF safe for negative values?
- if (coeff < 0) {
sign_bit = 1;mantissa = -mantissa;mantissa &= ((1 << CSC_MAX_VALS) - 1);
Oops. It looks like you are reusing the macro/define for the number of coefficients for the size of the mantissa. Also you defined a macro for this very purpose in your other patch.
- }
- reg_val = 0;
- SET_BITS(reg_val, exponent_bits, 12, 3);
- SET_BITS(reg_val, mantissa, 3, 9);
- SET_BITS(reg_val, sign_bit, 15, 1);
- DRM_DEBUG_DRIVER("CSC: reg_val=0x%x\n", reg_val);
This debug message is superfluous as you can easily read register values out with i-g-t.
- return reg_val;
+}
+int bdw_set_csc(struct drm_device *dev, struct drm_property_blob *blob,
struct drm_crtc *crtc)+{
- enum pipe pipe;
- enum plane plane;
- int i, j, temp;
- int word = 0;
- u32 reg, plane_ctl, mode;
- struct drm_ctm *csc_data;
- struct drm_i915_private *dev_priv = dev->dev_private;
- if (WARN_ON(!blob))
return -EINVAL;- if (blob->length != sizeof(struct drm_ctm)) {
DRM_ERROR("Invalid length of data received\n");return -EINVAL;- }
- csc_data = (struct drm_ctm *)blob->data;
- pipe = to_intel_crtc(crtc)->pipe;
- plane = to_intel_crtc(crtc)->plane;
- plane_ctl = I915_READ(PLANE_CTL(pipe, plane));
- plane_ctl |= PLANE_CTL_PIPE_CSC_ENABLE;
- I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
- reg = _PIPE_CSC_COEFF(pipe);
- /* Write csc coeff to csc regs */
- for (i = 0, j = 0; i < CSC_MAX_VALS; i++) {
if ((i % 3) == 0) {temp = bdw_prepare_csc_coeff(csc_data->ctm_coeff[i]);
SET_BITS(word, temp, 16, 16);i++;temp = bdw_prepare_csc_coeff(csc_data->ctm_coeff[i]);
SET_BITS(word, temp, 0, 16);} else {temp = bdw_prepare_csc_coeff(csc_data->ctm_coeff[i]);
SET_BITS(word, temp, 16, 16);}I915_WRITE(reg + j, word);j = j + 4;}
Although this loops looks like it should work, i'm not convinced of it's merits over programming the 5 registers directly. That's a style question for the more experienced kernel folk.
- /* Enable CSC functionality */
- mode = I915_READ(PIPE_CSC_MODE(pipe));
- mode |= CSC_POSITION_BEFORE_GAMMA;
- I915_WRITE(PIPE_CSC_MODE(pipe), mode);
The specs indicate that this value is ignored in split gamma mode, but at the same time say that this register arms the CSC registers above. A comment to that affect might would be helpful.
- DRM_DEBUG_DRIVER("CSC enabled on Pipe %c\n",
pipe_name(pipe));
- return 0;
+}
+static s32 chv_prepare_csc_coeff(s64 csc_value) { s32 csc_int_value; u32 csc_fract_value;
- s16 csc_s3_12_format;
- s32 csc_s3_12_format;
Why is this change in this patch?
if (csc_value >= 0) { csc_value += CHV_CSC_FRACT_ROUNDOFF; @@ -650,6 +758,8 @@ void intel_color_manager_crtc_commit(struct drm_device *dev, /* CSC correction */ if (IS_CHERRYVIEW(dev)) ret = chv_set_csc(dev, blob, crtc);
else if (IS_BROADWELL(dev) || IS_GEN9(dev))ret = bdw_set_csc(dev, blob, crtc);if (ret) DRM_ERROR("set CSC correction failed\n");
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h index e0c486e..853c73c 100644 --- a/drivers/gpu/drm/i915/intel_color_manager.h +++ b/drivers/gpu/drm/i915/intel_color_manager.h @@ -90,7 +90,15 @@ #define CGM_CSC_EN (1 << 1) #define CGM_DEGAMMA_EN (1 << 0)
-/* Gamma on BDW */ +/* BDW CSC */ +/* 1.0000000 in S31.32 format */ +#define BDW_CSC_COEFF_UNITY_VAL 0x100000000 +/* 3.9921875 in S31.32 format */ +#define BDW_CSC_COEFF_MAX_VAL 0x3FE000000 +/*-4.000000 in S31.32 format */ +#define BDW_CSC_COEFF_MIN_VAL 0xFFFFFFFC00000000
+/* BDW Gamma */
Another change that has nothing to do with CSC.
#define BDW_SPLITGAMMA_MAX_VALS 512 #define BDW_8BIT_GAMMA_MAX_VALS 256 #define BDW_10BIT_GAMMA_MAX_VALS 1024 @@ -99,5 +107,5 @@ #define BDW_INDEX_AUTO_INCREMENT (1 << 15) #define BDW_INDEX_SPLIT_MODE (1 << 31)
-/* Degamma on BDW */ +/* BDW Degamma*/
Ditto.
#define BDW_DEGAMMA_MAX_VALS 512
Rob