This series introduces pipe level color management through a set of properties attached to the CRTC. It also provides an implementation for some Intel platforms.
This series is based of a previous set of patches by Shashank Sharma.
Cheers,
Lionel
Lionel Landwerlin (5): drm/i915: Extract out gamma table and CSC to their own file drm/i915: Do not read GAMMA_MODE register drm: introduce pipe color correction properties drm/i915: Implement color management on bdw/skl/bxt/kbl drm/i915: Implement color management on chv
Documentation/DocBook/gpu.tmpl | 65 +++- drivers/gpu/drm/drm_atomic.c | 86 +++++- drivers/gpu/drm/drm_atomic_helper.c | 103 +++++++ drivers/gpu/drm/drm_crtc.c | 35 +++ drivers/gpu/drm/drm_crtc_helper.c | 33 +++ drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.c | 27 +- drivers/gpu/drm/i915/i915_drv.h | 8 + drivers/gpu/drm/i915/i915_reg.h | 53 ++++ drivers/gpu/drm/i915/intel_color.c | 556 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_display.c | 183 ++---------- drivers/gpu/drm/i915/intel_drv.h | 12 + drivers/gpu/drm/i915/intel_fbdev.c | 8 + include/drm/drm_atomic_helper.h | 3 + include/drm/drm_crtc.h | 46 ++- include/drm/drm_crtc_helper.h | 3 + include/uapi/drm/drm_mode.h | 15 + 17 files changed, 1072 insertions(+), 165 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_color.c
-- 2.7.0
The moves a couple of functions programming the gamma LUT and CSC units into their own file.
On generations prior to Haswell there is only a gamma LUT. From haswell on there is also a new enhanced color correction unit that isn't used yet. This is why we need to set the GAMMA_MODE register, either we're using the legacy 8bits LUT or enhanced LUTs (of 10 or 12bits).
The CSC unit is only available from Haswell on.
We also need to make a special case for CherryView which is recognized as a gen 8 but doesn't have the same enhanced color correction unit from Haswell on.
v2: Fix access to GAMMA_MODE register on older generations than Haswell (from Matt Roper's comments)
Signed-off-by: Lionel Landwerlin lionel.g.landwerlin@intel.com Reviewed-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/intel_color.c | 190 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_display.c | 163 +++--------------------------- drivers/gpu/drm/i915/intel_drv.h | 10 ++ 5 files changed, 215 insertions(+), 151 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_color.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 0851de07..0516300 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -55,6 +55,7 @@ i915-y += intel_audio.o \ intel_atomic.o \ intel_atomic_plane.o \ intel_bios.o \ + intel_color.o \ intel_display.o \ intel_fbc.o \ intel_fifo_underrun.o \ diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9e76bfc..8cb4418 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -662,6 +662,8 @@ struct drm_i915_display_funcs { /* render clock increase/decrease */ /* display clock increase/decrease */ /* pll clock increase/decrease */ + + void (*load_luts)(struct drm_crtc *crtc); };
enum forcewake_domain_id { diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c new file mode 100644 index 0000000..5e0b997 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_color.c @@ -0,0 +1,190 @@ +/* + * Copyright © 2016 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + */ + +#include "intel_drv.h" + +/* + * Set up the pipe CSC unit. + * + * Currently only full range RGB to limited range RGB conversion + * is supported, but eventually this should handle various + * RGB<->YCbCr scenarios as well. + */ +static void i9xx_load_csc_matrix(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + int pipe = intel_crtc->pipe; + uint16_t coeff = 0x7800; /* 1.0 */ + + /* + * TODO: Check what kind of values actually come out of the pipe + * with these coeff/postoff values and adjust to get the best + * accuracy. Perhaps we even need to take the bpc value into + * consideration. + */ + + if (intel_crtc->config->limited_color_range) + coeff = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */ + + I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeff << 16); + I915_WRITE(PIPE_CSC_COEFF_BY(pipe), 0); + + I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeff); + I915_WRITE(PIPE_CSC_COEFF_BU(pipe), 0); + + I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), 0); + I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeff << 16); + + I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0); + I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0); + I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0); + + if (INTEL_INFO(dev)->gen > 6) { + uint16_t postoff = 0; + + if (intel_crtc->config->limited_color_range) + postoff = (16 * (1 << 12) / 255) & 0x1fff; + + I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff); + I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff); + I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff); + + I915_WRITE(PIPE_CSC_MODE(pipe), 0); + } else { + uint32_t mode = CSC_MODE_YUV_TO_RGB; + + if (intel_crtc->config->limited_color_range) + mode |= CSC_BLACK_SCREEN_OFFSET; + + I915_WRITE(PIPE_CSC_MODE(pipe), mode); + } +} + +void intel_color_set_csc(struct drm_crtc *crtc) +{ + i9xx_load_csc_matrix(crtc); +} + +/* Loads the palette/gamma unit for the CRTC with the prepared values. */ +static void i9xx_load_luts(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + enum pipe pipe = intel_crtc->pipe; + int i; + + if (HAS_GMCH_DISPLAY(dev)) { + if (intel_crtc->config->has_dsi_encoder) + assert_dsi_pll_enabled(dev_priv); + else + assert_pll_enabled(dev_priv, pipe); + } + + for (i = 0; i < 256; i++) { + uint32_t word = (intel_crtc->lut_r[i] << 16) | + (intel_crtc->lut_g[i] << 8) | + intel_crtc->lut_b[i]; + if (HAS_GMCH_DISPLAY(dev)) + I915_WRITE(PALETTE(pipe, i), word); + else + I915_WRITE(LGC_PALETTE(pipe, i), word); + } +} + +/* Loads the legacy palette/gamma unit for the CRTC on Haswell+. */ +static void haswell_load_luts(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + bool reenable_ips = false; + + /* + * Workaround : Do not read or write the pipe palette/gamma data while + * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled. + */ + if (IS_HASWELL(dev) && intel_crtc->config->ips_enabled && + ((I915_READ(GAMMA_MODE(intel_crtc->pipe)) & GAMMA_MODE_MODE_MASK) == + GAMMA_MODE_MODE_SPLIT)) { + hsw_disable_ips(intel_crtc); + reenable_ips = true; + } + I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT); + + i9xx_load_luts(crtc); + + if (reenable_ips) + hsw_enable_ips(intel_crtc); +} + +void intel_color_load_luts(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + /* The clocks have to be on to load the palette. */ + if (!crtc->state->active) + return; + + dev_priv->display.load_luts(crtc); +} + +void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, + u16 *blue, uint32_t start, uint32_t size) +{ + int end = (start + size > 256) ? 256 : start + size, i; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + + for (i = start; i < end; i++) { + intel_crtc->lut_r[i] = red[i] >> 8; + intel_crtc->lut_g[i] = green[i] >> 8; + intel_crtc->lut_b[i] = blue[i] >> 8; + } + + intel_color_load_luts(crtc); +} + +void intel_color_init(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + int i; + + drm_mode_crtc_set_gamma_size(crtc, 256); + for (i = 0; i < 256; i++) { + intel_crtc->lut_r[i] = i; + intel_crtc->lut_g[i] = i; + intel_crtc->lut_b[i] = i; + } + + if (IS_HASWELL(dev) || + (INTEL_INFO(dev)->gen >= 8 && !IS_CHERRYVIEW(dev))) + dev_priv->display.load_luts = haswell_load_luts; + else + dev_priv->display.load_luts = i9xx_load_luts; +} diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index deee560..acbb1d9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -101,7 +101,6 @@ static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc, struct intel_link_m_n *m2_n2); static void ironlake_set_pipeconf(struct drm_crtc *crtc); static void haswell_set_pipeconf(struct drm_crtc *crtc); -static void intel_set_pipe_csc(struct drm_crtc *crtc); static void vlv_prepare_pll(struct intel_crtc *crtc, const struct intel_crtc_state *pipe_config); static void chv_prepare_pll(struct intel_crtc *crtc, @@ -1165,7 +1164,7 @@ void assert_pll(struct drm_i915_private *dev_priv, }
/* XXX: the dsi pll is shared between MIPI DSI ports */ -static void assert_dsi_pll(struct drm_i915_private *dev_priv, bool state) +void assert_dsi_pll(struct drm_i915_private *dev_priv, bool state) { u32 val; bool cur_state; @@ -1179,8 +1178,6 @@ static void assert_dsi_pll(struct drm_i915_private *dev_priv, bool state) "DSI PLL state assertion failure (expected %s, current %s)\n", onoff(state), onoff(cur_state)); } -#define assert_dsi_pll_enabled(d) assert_dsi_pll(d, true) -#define assert_dsi_pll_disabled(d) assert_dsi_pll(d, false)
struct intel_shared_dpll * intel_crtc_to_shared_dpll(struct intel_crtc *crtc) @@ -3315,7 +3312,7 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, pipe_config->pipe_src_w, pipe_config->pipe_src_h);
if (HAS_DDI(dev)) - intel_set_pipe_csc(&crtc->base); + intel_color_set_csc(&crtc->base);
/* * Update pipe size and adjust fitter if needed: the reason for this is @@ -4626,55 +4623,6 @@ void hsw_disable_ips(struct intel_crtc *crtc) intel_wait_for_vblank(dev, crtc->pipe); }
-/** Loads the palette/gamma unit for the CRTC with the prepared values */ -static void intel_crtc_load_lut(struct drm_crtc *crtc) -{ - struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum pipe pipe = intel_crtc->pipe; - int i; - bool reenable_ips = false; - - /* The clocks have to be on to load the palette. */ - if (!crtc->state->active) - return; - - if (HAS_GMCH_DISPLAY(dev_priv->dev)) { - if (intel_crtc->config->has_dsi_encoder) - assert_dsi_pll_enabled(dev_priv); - else - assert_pll_enabled(dev_priv, pipe); - } - - /* Workaround : Do not read or write the pipe palette/gamma data while - * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled. - */ - if (IS_HASWELL(dev) && intel_crtc->config->ips_enabled && - ((I915_READ(GAMMA_MODE(pipe)) & GAMMA_MODE_MODE_MASK) == - GAMMA_MODE_MODE_SPLIT)) { - hsw_disable_ips(intel_crtc); - reenable_ips = true; - } - - for (i = 0; i < 256; i++) { - i915_reg_t palreg; - - if (HAS_GMCH_DISPLAY(dev)) - palreg = PALETTE(pipe, i); - else - palreg = LGC_PALETTE(pipe, i); - - I915_WRITE(palreg, - (intel_crtc->lut_r[i] << 16) | - (intel_crtc->lut_g[i] << 8) | - intel_crtc->lut_b[i]); - } - - if (reenable_ips) - hsw_enable_ips(intel_crtc); -} - static void intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc) { if (intel_crtc->overlay) { @@ -4923,7 +4871,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) * On ILK+ LUT must be loaded before the pipe is running but with * clocks enabled */ - intel_crtc_load_lut(crtc); + intel_color_load_luts(crtc);
intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); @@ -4989,7 +4937,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
haswell_set_pipeconf(crtc);
- intel_set_pipe_csc(crtc); + intel_color_set_csc(crtc);
intel_crtc->active = true;
@@ -5018,7 +4966,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) * On ILK+ LUT must be loaded before the pipe is running but with * clocks enabled */ - intel_crtc_load_lut(crtc); + intel_color_load_luts(crtc);
intel_ddi_set_pipe_settings(crtc); if (!intel_crtc->config->has_dsi_encoder) @@ -6229,7 +6177,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
i9xx_pfit_enable(intel_crtc);
- intel_crtc_load_lut(crtc); + intel_color_load_luts(crtc);
intel_enable_pipe(intel_crtc);
@@ -6282,7 +6230,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
i9xx_pfit_enable(intel_crtc);
- intel_crtc_load_lut(crtc); + intel_color_load_luts(crtc);
intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); @@ -8737,70 +8685,6 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc) POSTING_READ(PIPECONF(pipe)); }
-/* - * Set up the pipe CSC unit. - * - * Currently only full range RGB to limited range RGB conversion - * is supported, but eventually this should handle various - * RGB<->YCbCr scenarios as well. - */ -static void intel_set_pipe_csc(struct drm_crtc *crtc) -{ - struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int pipe = intel_crtc->pipe; - uint16_t coeff = 0x7800; /* 1.0 */ - - /* - * TODO: Check what kind of values actually come out of the pipe - * with these coeff/postoff values and adjust to get the best - * accuracy. Perhaps we even need to take the bpc value into - * consideration. - */ - - if (intel_crtc->config->limited_color_range) - coeff = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */ - - /* - * GY/GU and RY/RU should be the other way around according - * to BSpec, but reality doesn't agree. Just set them up in - * a way that results in the correct picture. - */ - I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeff << 16); - I915_WRITE(PIPE_CSC_COEFF_BY(pipe), 0); - - I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeff); - I915_WRITE(PIPE_CSC_COEFF_BU(pipe), 0); - - I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), 0); - I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeff << 16); - - I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0); - I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0); - I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0); - - if (INTEL_INFO(dev)->gen > 6) { - uint16_t postoff = 0; - - if (intel_crtc->config->limited_color_range) - postoff = (16 * (1 << 12) / 255) & 0x1fff; - - I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff); - I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff); - I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff); - - I915_WRITE(PIPE_CSC_MODE(pipe), 0); - } else { - uint32_t mode = CSC_MODE_YUV_TO_RGB; - - if (intel_crtc->config->limited_color_range) - mode |= CSC_BLACK_SCREEN_OFFSET; - - I915_WRITE(PIPE_CSC_MODE(pipe), mode); - } -} - static void haswell_set_pipeconf(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -8823,9 +8707,6 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc) I915_WRITE(PIPECONF(cpu_transcoder), val); POSTING_READ(PIPECONF(cpu_transcoder));
- I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT); - POSTING_READ(GAMMA_MODE(intel_crtc->pipe)); - if (IS_BROADWELL(dev) || INTEL_INFO(dev)->gen >= 9) { val = 0;
@@ -10238,21 +10119,6 @@ static bool cursor_size_ok(struct drm_device *dev, return true; }
-static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, - u16 *blue, uint32_t start, uint32_t size) -{ - int end = (start + size > 256) ? 256 : start + size, i; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - - for (i = start; i < end; i++) { - intel_crtc->lut_r[i] = red[i] >> 8; - intel_crtc->lut_g[i] = green[i] >> 8; - intel_crtc->lut_b[i] = blue[i] >> 8; - } - - intel_crtc_load_lut(crtc); -} - /* VESA 640x480x72Hz mode to set on the pipe */ static struct drm_display_mode load_detect_mode = { DRM_MODE("640x480", DRM_MODE_TYPE_DEFAULT, 31500, 640, 664, @@ -12001,7 +11867,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
static const struct drm_crtc_helper_funcs intel_helper_funcs = { .mode_set_base_atomic = intel_pipe_set_base_atomic, - .load_lut = intel_crtc_load_lut, + .load_lut = intel_color_load_luts, .atomic_begin = intel_begin_crtc_commit, .atomic_flush = intel_finish_crtc_commit, .atomic_check = intel_crtc_atomic_check, @@ -13621,7 +13487,7 @@ out: #undef for_each_intel_crtc_masked
static const struct drm_crtc_funcs intel_crtc_funcs = { - .gamma_set = intel_crtc_gamma_set, + .gamma_set = intel_color_legacy_gamma_set, .set_config = drm_atomic_helper_set_config, .destroy = intel_crtc_destroy, .page_flip = intel_crtc_page_flip, @@ -14228,7 +14094,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) struct intel_crtc_state *crtc_state = NULL; struct drm_plane *primary = NULL; struct drm_plane *cursor = NULL; - int i, ret; + int ret;
intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL); if (intel_crtc == NULL) @@ -14264,13 +14130,6 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) if (ret) goto fail;
- drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256); - for (i = 0; i < 256; i++) { - intel_crtc->lut_r[i] = i; - intel_crtc->lut_g[i] = i; - intel_crtc->lut_b[i] = i; - } - /* * On gen2/3 only plane A can do fbc, but the panel fitter and lvds port * is hooked to pipe B. Hence we want plane A feeding pipe B. @@ -14295,6 +14154,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
+ intel_color_init(&intel_crtc->base); + WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe); return;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 4852049..7532f61 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1166,6 +1166,9 @@ void assert_pll(struct drm_i915_private *dev_priv, enum pipe pipe, bool state); #define assert_pll_enabled(d, p) assert_pll(d, p, true) #define assert_pll_disabled(d, p) assert_pll(d, p, false) +void assert_dsi_pll(struct drm_i915_private *dev_priv, bool state); +#define assert_dsi_pll_enabled(d) assert_dsi_pll(d, true) +#define assert_dsi_pll_disabled(d) assert_dsi_pll(d, false) void assert_fdi_rx_pll(struct drm_i915_private *dev_priv, enum pipe pipe, bool state); #define assert_fdi_rx_pll_enabled(d, p) assert_fdi_rx_pll(d, p, true) @@ -1618,4 +1621,11 @@ void intel_plane_destroy_state(struct drm_plane *plane, struct drm_plane_state *state); extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
+/* intel_color.c */ +void intel_color_init(struct drm_crtc *crtc); +void intel_color_set_csc(struct drm_crtc *crtc); +void intel_color_load_luts(struct drm_crtc *crtc); +void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, + u16 *blue, uint32_t start, uint32_t size); + #endif /* __INTEL_DRV_H__ */
Implement Daniel Stone's recommendation to not read registers to infer the hardware's state.
v2: Read GAMMA_MODE register value at init (Matt Roper's comment)
v3: Read GAMMA_MODE register in intel_modeset_readout_hw_state along with other registers (Matt Roper's comment).
Signed-off-by: Lionel Landwerlin lionel.g.landwerlin@intel.com --- drivers/gpu/drm/i915/intel_color.c | 12 ++++++++---- drivers/gpu/drm/i915/intel_display.c | 2 ++ drivers/gpu/drm/i915/intel_drv.h | 3 +++ 3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 5e0b997..16657eb 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -121,6 +121,8 @@ static void haswell_load_luts(struct drm_crtc *crtc) struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc_state *intel_crtc_state = + to_intel_crtc_state(crtc->state); bool reenable_ips = false;
/* @@ -128,11 +130,12 @@ static void haswell_load_luts(struct drm_crtc *crtc) * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled. */ if (IS_HASWELL(dev) && intel_crtc->config->ips_enabled && - ((I915_READ(GAMMA_MODE(intel_crtc->pipe)) & GAMMA_MODE_MODE_MASK) == - GAMMA_MODE_MODE_SPLIT)) { + (intel_crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)) { hsw_disable_ips(intel_crtc); reenable_ips = true; } + + intel_crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT; I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
i9xx_load_luts(crtc); @@ -183,8 +186,9 @@ void intel_color_init(struct drm_crtc *crtc) }
if (IS_HASWELL(dev) || - (INTEL_INFO(dev)->gen >= 8 && !IS_CHERRYVIEW(dev))) + (INTEL_INFO(dev)->gen >= 8 && !IS_CHERRYVIEW(dev))) { dev_priv->display.load_luts = haswell_load_luts; - else + } else { dev_priv->display.load_luts = i9xx_load_luts; + } } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index acbb1d9..19f8284 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9891,6 +9891,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
intel_get_pipe_timings(crtc, pipe_config);
+ pipe_config->gamma_mode = I915_READ(GAMMA_MODE(crtc->pipe)); + if (INTEL_INFO(dev)->gen >= 9) { skl_init_scalers(dev, crtc, pipe_config); } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 7532f61..8c48131 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -517,6 +517,9 @@ struct intel_crtc_state { struct skl_pipe_wm skl; } optimal; } wm; + + /* Gamma mode programmed on the pipe */ + uint32_t gamma_mode; };
struct vlv_wm_state {
On Thu, Feb 25, 2016 at 10:58:46AM +0000, Lionel Landwerlin wrote:
Implement Daniel Stone's recommendation to not read registers to infer the hardware's state.
v2: Read GAMMA_MODE register value at init (Matt Roper's comment)
v3: Read GAMMA_MODE register in intel_modeset_readout_hw_state along with other registers (Matt Roper's comment).
Signed-off-by: Lionel Landwerlin lionel.g.landwerlin@intel.com
drivers/gpu/drm/i915/intel_color.c | 12 ++++++++---- drivers/gpu/drm/i915/intel_display.c | 2 ++ drivers/gpu/drm/i915/intel_drv.h | 3 +++ 3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 5e0b997..16657eb 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -121,6 +121,8 @@ static void haswell_load_luts(struct drm_crtc *crtc) struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *intel_crtc_state =
to_intel_crtc_state(crtc->state);bool reenable_ips = false;
/*
@@ -128,11 +130,12 @@ static void haswell_load_luts(struct drm_crtc *crtc) * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled. */ if (IS_HASWELL(dev) && intel_crtc->config->ips_enabled &&
((I915_READ(GAMMA_MODE(intel_crtc->pipe)) & GAMMA_MODE_MODE_MASK) ==GAMMA_MODE_MODE_SPLIT)) {
(intel_crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)) {hsw_disable_ips(intel_crtc); reenable_ips = true; }
intel_crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT; I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
i9xx_load_luts(crtc);
@@ -183,8 +186,9 @@ void intel_color_init(struct drm_crtc *crtc) }
if (IS_HASWELL(dev) ||
(INTEL_INFO(dev)->gen >= 8 && !IS_CHERRYVIEW(dev)))
(INTEL_INFO(dev)->gen >= 8 && !IS_CHERRYVIEW(dev))) {
- else
- } else { dev_priv->display.load_luts = i9xx_load_luts;
- }
} diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index acbb1d9..19f8284 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9891,6 +9891,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
intel_get_pipe_timings(crtc, pipe_config);
- pipe_config->gamma_mode = I915_READ(GAMMA_MODE(crtc->pipe));
I think you want to mask this with GAMMA_MODE_MODE_MASK since there may be stuff in the higher bits.
With that change made, you can consider this patch
Reviewed-by: Matt Roper matthew.d.roper@intel.com
- if (INTEL_INFO(dev)->gen >= 9) { skl_init_scalers(dev, crtc, pipe_config); }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 7532f61..8c48131 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -517,6 +517,9 @@ struct intel_crtc_state { struct skl_pipe_wm skl; } optimal; } wm;
- /* Gamma mode programmed on the pipe */
- uint32_t gamma_mode;
};
struct vlv_wm_state {
2.7.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Patch based on a previous series by Shashank Sharma.
This introduces optional properties to enable color correction at the pipe level. It relies on 3 transformations applied to every pixels displayed. First a lookup into a degamma table, then a multiplication of the rgb components by a 3x3 matrix and finally another lookup into a gamma table.
The following properties can be added to a pipe : - DEGAMMA_LUT : blob containing degamma LUT - DEGAMMA_LUT_SIZE : number of elements in DEGAMMA_LUT - CTM : transformation matrix applied after the degamma LUT - GAMMA_LUT : blob containing gamma LUT - GAMMA_LUT_SIZE : number of elements in GAMMA_LUT
DEGAMMA_LUT_SIZE and GAMMA_LUT_SIZE are read only properties, set by the driver to tell userspace applications what sizes should be the lookup tables in DEGAMMA_LUT and GAMMA_LUT.
A helper is also provided so legacy gamma correction is redirected through these new properties.
v2: Register LUT size properties as range
v3: Fix round in drm_color_lut_get_value() helper More docs on how degamma/gamma properties are used
v4: Update contributors
v5: Rename CTM_MATRIX property to CTM (Doh!) Add legacy gamma_set atomic helper Describe CTM/LUT acronyms in the kernel doc
Signed-off-by: Shashank Sharma shashank.sharma@intel.com Signed-off-by: Lionel Landwerlin lionel.g.landwerlin@intel.com Signed-off-by: Kumar, Kiran S kiran.s.kumar@intel.com Signed-off-by: Kausal Malladi kausalmalladi@gmail.com Reviewed-by: Matt Roper matthew.d.roper@intel.com --- Documentation/DocBook/gpu.tmpl | 59 ++++++++++++++++++++- drivers/gpu/drm/drm_atomic.c | 86 +++++++++++++++++++++++++++++- drivers/gpu/drm/drm_atomic_helper.c | 103 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_crtc.c | 35 ++++++++++++ drivers/gpu/drm/drm_crtc_helper.c | 33 ++++++++++++ include/drm/drm_atomic_helper.h | 3 ++ include/drm/drm_crtc.h | 46 +++++++++++++++- include/drm/drm_crtc_helper.h | 3 ++ include/uapi/drm/drm_mode.h | 15 ++++++ 9 files changed, 378 insertions(+), 5 deletions(-)
diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index fe6b36a..1692c4d 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -1816,7 +1816,7 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >Description/Restrictions</td> </tr> <tr> - <td rowspan="37" valign="top" >DRM</td> + <td rowspan="42" valign="top" >DRM</td> <td valign="top" >Generic</td> <td valign="top" >“rotation”</td> <td valign="top" >BITMASK</td> @@ -2068,7 +2068,7 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >property to suggest an Y offset for a connector</td> </tr> <tr> - <td rowspan="3" valign="top" >Optional</td> + <td rowspan="8" valign="top" >Optional</td> <td valign="top" >“scaling mode”</td> <td valign="top" >ENUM</td> <td valign="top" >{ "None", "Full", "Center", "Full aspect" }</td> @@ -2092,6 +2092,61 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >TBD</td> </tr> <tr> + <td valign="top" >“DEGAMMA_LUT”</td> + <td valign="top" >BLOB</td> + <td valign="top" >0</td> + <td valign="top" >CRTC</td> + <td valign="top" >DRM property to set the degamma lookup table + (LUT) mapping pixel data from the framebuffer before it is + given to the transformation matrix. The data is an interpreted + as an array of struct drm_color_lut elements. Hardware might + choose not to use the full precision of the LUT elements nor + use all the elements of the LUT (for example the hardware + might choose to interpolate between LUT[0] and LUT[4]). </td> + </tr> + <tr> + <td valign="top" >“DEGAMMA_LUT_SIZE”</td> + <td valign="top" >RANGE | IMMUTABLE</td> + <td valign="top" >Min=0, Max=UINT_MAX</td> + <td valign="top" >CRTC</td> + <td valign="top" >DRM property to gives the size of the lookup + table to be set on the DEGAMMA_LUT property (the size depends + on the underlying hardware).</td> + </tr> + <tr> + <td valign="top" >“CTM”</td> + <td valign="top" >BLOB</td> + <td valign="top" >0</td> + <td valign="top" >CRTC</td> + <td valign="top" >DRM property to set the current + transformation matrix (CTM) apply to pixel data after the + lookup through the degamma LUT and before the lookup through + the gamma LUT. The data is an interpreted as a struct + drm_color_ctm.</td> + </tr> + <tr> + <td valign="top" >“GAMMA_LUT”</td> + <td valign="top" >BLOB</td> + <td valign="top" >0</td> + <td valign="top" >CRTC</td> + <td valign="top" >DRM property to set the gamma lookup table + (LUT) mapping pixel data after to the transformation matrix to + data sent to the connector. The data is an interpreted as an + array of struct drm_color_lut elements. Hardware might choose + not to use the full precision of the LUT elements nor use all + the elements of the LUT (for example the hardware might choose + to interpolate between LUT[0] and LUT[4]).</td> + </tr> + <tr> + <td valign="top" >“GAMMA_LUT_SIZE”</td> + <td valign="top" >RANGE | IMMUTABLE</td> + <td valign="top" >Min=0, Max=UINT_MAX</td> + <td valign="top" >CRTC</td> + <td valign="top" >DRM property to gives the size of the lookup + table to be set on the GAMMA_LUT property (the size depends on + the underlying hardware).</td> + </tr> + <tr> <td rowspan="20" valign="top" >i915</td> <td rowspan="2" valign="top" >Generic</td> <td valign="top" >"Broadcast RGB"</td> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 092620c..b767a4f 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -28,6 +28,7 @@
#include <drm/drmP.h> #include <drm/drm_atomic.h> +#include <drm/drm_mode.h> #include <drm/drm_plane_helper.h>
/** @@ -376,6 +377,57 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state, EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
/** + * drm_atomic_replace_property_blob - replace a blob property + * @blob: a pointer to the member blob to be replaced + * @new_blob: the new blob to replace with + * @expected_size: the expected size of the new blob + * @replaced: whether the blob has been replaced + * + * RETURNS: + * Zero on success, error code on failure + */ +static int +drm_atomic_replace_property_blob(struct drm_property_blob **blob, + struct drm_property_blob *new_blob, + bool *replaced) +{ + struct drm_property_blob *old_blob = *blob; + + if (old_blob == new_blob) + return 0; + + if (old_blob) + drm_property_unreference_blob(old_blob); + if (new_blob) + drm_property_reference_blob(new_blob); + *blob = new_blob; + *replaced = true; + + return 0; +} + +static int +drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc, + struct drm_property_blob **blob, + uint64_t blob_id, + ssize_t expected_size, + bool *replaced) +{ + struct drm_device *dev = crtc->dev; + struct drm_property_blob *new_blob = NULL; + + if (blob_id != 0) { + new_blob = drm_property_lookup_blob(dev, blob_id); + if (new_blob == NULL) + return -EINVAL; + if (expected_size > 0 && expected_size != new_blob->length) + return -EINVAL; + } + + return drm_atomic_replace_property_blob(blob, new_blob, replaced); +} + +/** * drm_atomic_crtc_set_property - set property on CRTC * @crtc: the drm CRTC to set a property on * @state: the state object to update with the new property value @@ -397,6 +449,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, { struct drm_device *dev = crtc->dev; struct drm_mode_config *config = &dev->mode_config; + bool replaced = false; int ret;
if (property == config->prop_active) @@ -407,8 +460,31 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, ret = drm_atomic_set_mode_prop_for_crtc(state, mode); drm_property_unreference_blob(mode); return ret; - } - else if (crtc->funcs->atomic_set_property) + } else if (property == config->degamma_lut_property) { + ret = drm_atomic_replace_property_blob_from_id(crtc, + &state->degamma_lut, + val, + -1, + &replaced); + state->color_mgmt_changed = replaced; + return ret; + } else if (property == config->ctm_property) { + ret = drm_atomic_replace_property_blob_from_id(crtc, + &state->ctm, + val, + sizeof(struct drm_color_ctm), + &replaced); + state->color_mgmt_changed = replaced; + return ret; + } else if (property == config->gamma_lut_property) { + ret = drm_atomic_replace_property_blob_from_id(crtc, + &state->gamma_lut, + val, + -1, + &replaced); + state->color_mgmt_changed = replaced; + return ret; + } else if (crtc->funcs->atomic_set_property) return crtc->funcs->atomic_set_property(crtc, state, property, val); else return -EINVAL; @@ -444,6 +520,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, *val = state->active; else if (property == config->prop_mode_id) *val = (state->mode_blob) ? state->mode_blob->base.id : 0; + else if (property == config->degamma_lut_property) + *val = (state->degamma_lut) ? state->degamma_lut->base.id : 0; + else if (property == config->ctm_property) + *val = (state->ctm) ? state->ctm->base.id : 0; + else if (property == config->gamma_lut_property) + *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0; else if (crtc->funcs->atomic_get_property) return crtc->funcs->atomic_get_property(crtc, state, property, val); else diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 4da4f2a..7ab8040 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2513,10 +2513,17 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
if (state->mode_blob) drm_property_reference_blob(state->mode_blob); + if (state->degamma_lut) + drm_property_reference_blob(state->degamma_lut); + if (state->ctm) + drm_property_reference_blob(state->ctm); + if (state->gamma_lut) + drm_property_reference_blob(state->gamma_lut); state->mode_changed = false; state->active_changed = false; state->planes_changed = false; state->connectors_changed = false; + state->color_mgmt_changed = false; state->event = NULL; } EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state); @@ -2557,6 +2564,9 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc, struct drm_crtc_state *state) { drm_property_unreference_blob(state->mode_blob); + drm_property_unreference_blob(state->degamma_lut); + drm_property_unreference_blob(state->ctm); + drm_property_unreference_blob(state->gamma_lut); } EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
@@ -2870,3 +2880,96 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector, kfree(state); } EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state); + +/** + * drm_atomic_helper_legacy_gamma_set - set the legacy gamma correction table + * @crtc: CRTC object + * @red: red correction table + * @green: green correction table + * @blue: green correction table + * @start: + * @size: size of the tables + * + * Implements support for legacy gamma correction table for drivers + * that support color management through the DEGAMMA_LUT/GAMMA_LUT + * properties. + */ +void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, + u16 *red, u16 *green, u16 *blue, + uint32_t start, uint32_t size) +{ + struct drm_device *dev = crtc->dev; + struct drm_mode_config *config = &dev->mode_config; + struct drm_atomic_state *state; + struct drm_crtc_state *crtc_state; + struct drm_property_blob *blob = NULL; + struct drm_color_lut *blob_data; + int i, ret = 0; + + state = drm_atomic_state_alloc(crtc->dev); + if (!state) + return; + + blob = drm_property_create_blob(dev, + sizeof(struct drm_color_lut) * size, + NULL); + + state->acquire_ctx = crtc->dev->mode_config.acquire_ctx; +retry: + crtc_state = drm_atomic_get_crtc_state(state, crtc); + if (IS_ERR(crtc_state)) { + ret = PTR_ERR(crtc_state); + goto fail; + } + + /* Reset DEGAMMA_LUT and CTM properties. */ + ret = drm_atomic_crtc_set_property(crtc, crtc_state, + config->degamma_lut_property, 0); + if (ret) + goto fail; + ret = drm_atomic_crtc_set_property(crtc, crtc_state, + config->ctm_property, 0); + if (ret) + goto fail; + + /* Set GAMMA_LUT with legacy values. */ + if (blob == NULL) { + ret = -ENOMEM; + goto fail; + } + + blob_data = (struct drm_color_lut *) blob->data; + for (i = 0; i < size; i++) { + blob_data[i].red = red[i]; + blob_data[i].green = green[i]; + blob_data[i].blue = blue[i]; + } + + ret = drm_atomic_crtc_set_property(crtc, crtc_state, + config->gamma_lut_property, blob->base.id); + if (ret) + goto fail; + + ret = drm_atomic_commit(state); + if (ret != 0) + goto fail; + + drm_property_unreference_blob(blob); + + /* Driver takes ownership of state on successful commit. */ + return; +fail: + if (ret == -EDEADLK) + goto backoff; + + drm_atomic_state_free(state); + drm_property_unreference_blob(blob); + + return; +backoff: + drm_atomic_state_clear(state); + drm_atomic_legacy_backoff(state); + + goto retry; +} +EXPORT_SYMBOL(drm_atomic_helper_legacy_gamma_set); diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 8451400..50066d1 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1554,6 +1554,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.prop_mode_id = prop;
+ prop = drm_property_create(dev, + DRM_MODE_PROP_BLOB, + "DEGAMMA_LUT", 0); + if (!prop) + return -ENOMEM; + dev->mode_config.degamma_lut_property = prop; + + prop = drm_property_create_range(dev, + DRM_MODE_PROP_IMMUTABLE, + "DEGAMMA_LUT_SIZE", 0, UINT_MAX); + if (!prop) + return -ENOMEM; + dev->mode_config.degamma_lut_size_property = prop; + + prop = drm_property_create(dev, + DRM_MODE_PROP_BLOB, + "CTM", 0); + if (!prop) + return -ENOMEM; + dev->mode_config.ctm_property = prop; + + prop = drm_property_create(dev, + DRM_MODE_PROP_BLOB, + "GAMMA_LUT", 0); + if (!prop) + return -ENOMEM; + dev->mode_config.gamma_lut_property = prop; + + prop = drm_property_create_range(dev, + DRM_MODE_PROP_IMMUTABLE, + "GAMMA_LUT_SIZE", 0, UINT_MAX); + if (!prop) + return -ENOMEM; + dev->mode_config.gamma_lut_size_property = prop; + return 0; }
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 7539eea..79555d2 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -1075,3 +1075,36 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, return drm_plane_helper_commit(plane, plane_state, old_fb); } EXPORT_SYMBOL(drm_helper_crtc_mode_set_base); + +/** + * drm_helper_crtc_enable_color_mgmt - enable color management properties + * @crtc: DRM CRTC + * @degamma_lut_size: the size of the degamma lut (before CSC) + * @gamma_lut_size: the size of the gamma lut (after CSC) + * + * This function lets the driver enable the color correction properties on a + * CRTC. This includes 3 degamma, csc and gamma properties that userspace can + * set and 2 size properties to inform the userspace of the lut sizes. + */ +void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc, + int degamma_lut_size, + int gamma_lut_size) +{ + struct drm_device *dev = crtc->dev; + struct drm_mode_config *config = &dev->mode_config; + + drm_object_attach_property(&crtc->base, + config->degamma_lut_property, 0); + drm_object_attach_property(&crtc->base, + config->ctm_property, 0); + drm_object_attach_property(&crtc->base, + config->gamma_lut_property, 0); + + drm_object_attach_property(&crtc->base, + config->degamma_lut_size_property, + degamma_lut_size); + drm_object_attach_property(&crtc->base, + config->gamma_lut_size_property, + gamma_lut_size); +} +EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt); diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index fe5efad..9054598c 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -146,6 +146,9 @@ __drm_atomic_helper_connector_destroy_state(struct drm_connector *connector, struct drm_connector_state *state); void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector, struct drm_connector_state *state); +void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, + u16 *red, u16 *green, u16 *blue, + uint32_t start, uint32_t size);
/** * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 7fad193..f618803 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -305,6 +305,8 @@ struct drm_plane_helper_funcs; * @mode_changed: crtc_state->mode or crtc_state->enable has been changed * @active_changed: crtc_state->active has been toggled. * @connectors_changed: connectors to this crtc have been updated + * @color_mgmt_changed: color management properties have changed (degamma or + * gamma LUT or CSC matrix) * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors * @encoder_mask: bitmask of (1 << drm_encoder_index(encoder)) of attached encoders @@ -312,6 +314,11 @@ struct drm_plane_helper_funcs; * update to ensure framebuffer cleanup isn't done too early * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings * @mode: current mode timings + * @degamma_lut: Lookup table for converting framebuffer pixel data + * before apply the conversion matrix + * @ctm: Transformation matrix + * @gamma_lut: Lookup table for converting pixel data after the + * conversion matrix * @event: optional pointer to a DRM event to signal upon completion of the * state update * @state: backpointer to global drm_atomic_state @@ -333,6 +340,7 @@ struct drm_crtc_state { bool mode_changed : 1; bool active_changed : 1; bool connectors_changed : 1; + bool color_mgmt_changed : 1;
/* attached planes bitmask: * WARNING: transitional helpers do not maintain plane_mask so @@ -355,6 +363,11 @@ struct drm_crtc_state { /* blob property to expose current mode to atomic userspace */ struct drm_property_blob *mode_blob;
+ /* blob property to expose color management to userspace */ + struct drm_property_blob *degamma_lut; + struct drm_property_blob *ctm; + struct drm_property_blob *gamma_lut; + struct drm_pending_vblank_event *event;
struct drm_atomic_state *state; @@ -757,7 +770,7 @@ struct drm_crtc { int x, y; const struct drm_crtc_funcs *funcs;
- /* CRTC gamma size for reporting to userspace */ + /* Legacy FB CRTC gamma size for reporting to userspace */ uint32_t gamma_size; uint16_t *gamma_store;
@@ -2026,6 +2039,15 @@ struct drm_mode_config_funcs { * @property_blob_list: list of all the blob property objects * @blob_lock: mutex for blob property allocation and management * @*_property: core property tracking + * @degamma_lut_property: LUT used to convert the framebuffer's colors to linear + * gamma + * @degamma_lut_size_property: size of the degamma LUT as supported by the + * driver (read-only) + * @ctm_property: Matrix used to convert colors after the lookup in the + * degamma LUT + * @gamma_lut_property: LUT used to convert the colors, after the CSC matrix, to + * the gamma space of the connected screen (read-only) + * @gamma_lut_size_property: size of the gamma LUT as supported by the driver * @preferred_depth: preferred RBG pixel depth, used by fb helpers * @prefer_shadow: hint to userspace to prefer shadow-fb rendering * @async_page_flip: does this device support async flips on the primary plane? @@ -2128,6 +2150,13 @@ struct drm_mode_config { struct drm_property *aspect_ratio_property; struct drm_property *dirty_info_property;
+ /* Optional color correction properties */ + struct drm_property *degamma_lut_property; + struct drm_property *degamma_lut_size_property; + struct drm_property *ctm_property; + struct drm_property *gamma_lut_property; + struct drm_property *gamma_lut_size_property; + /* properties for virtual machine layout */ struct drm_property *suggested_x_property; struct drm_property *suggested_y_property; @@ -2554,6 +2583,21 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev, return mo ? obj_to_property(mo) : NULL; }
+/* + * Extract a degamma/gamma LUT value provided by user and round it to the + * precision supported by the hardware. + */ +static inline uint32_t drm_color_lut_extract(uint32_t user_input, + uint32_t bit_precision) +{ + uint32_t val = user_input + (1 << (16 - bit_precision - 1)); + uint32_t max = 0xffff >> (16 - bit_precision); + + val >>= 16 - bit_precision; + + return clamp_val(val, 0, max); +} + /* Plane list iterator for legacy (overlay only) planes. */ #define drm_for_each_legacy_plane(plane, dev) \ list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \ diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h index 4b37afa..97fa894 100644 --- a/include/drm/drm_crtc_helper.h +++ b/include/drm/drm_crtc_helper.h @@ -48,6 +48,9 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode, int x, int y, struct drm_framebuffer *old_fb); +extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc, + int degamma_lut_size, + int gamma_lut_size); extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc); extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 50adb46..c021743 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -487,6 +487,21 @@ struct drm_mode_crtc_lut { __u64 blue; };
+struct drm_color_ctm { + /* Conversion matrix in S31.32 format. */ + __s64 matrix[9]; +}; + +struct drm_color_lut { + /* + * Data is U0.16 fixed point format. + */ + __u16 red; + __u16 green; + __u16 blue; + __u16 reserved; +}; + #define DRM_MODE_PAGE_FLIP_EVENT 0x01 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 #define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)
Hi Lionel,
A bunch of suggestions - feel free to take or ignore them :-)
On 25 February 2016 at 10:58, Lionel Landwerlin lionel.g.landwerlin@intel.com wrote:
Patch based on a previous series by Shashank Sharma.
This introduces optional properties to enable color correction at the pipe level. It relies on 3 transformations applied to every pixels displayed. First a lookup into a degamma table, then a multiplication of the rgb components by a 3x3 matrix and finally another lookup into a gamma table.
The following properties can be added to a pipe :
- DEGAMMA_LUT : blob containing degamma LUT
- DEGAMMA_LUT_SIZE : number of elements in DEGAMMA_LUT
- CTM : transformation matrix applied after the degamma LUT
- GAMMA_LUT : blob containing gamma LUT
- GAMMA_LUT_SIZE : number of elements in GAMMA_LUT
DEGAMMA_LUT_SIZE and GAMMA_LUT_SIZE are read only properties, set by the driver to tell userspace applications what sizes should be the lookup tables in DEGAMMA_LUT and GAMMA_LUT.
A helper is also provided so legacy gamma correction is redirected through these new properties.
v2: Register LUT size properties as range
v3: Fix round in drm_color_lut_get_value() helper More docs on how degamma/gamma properties are used
v4: Update contributors
v5: Rename CTM_MATRIX property to CTM (Doh!) Add legacy gamma_set atomic helper Describe CTM/LUT acronyms in the kernel doc
Signed-off-by: Shashank Sharma shashank.sharma@intel.com Signed-off-by: Lionel Landwerlin lionel.g.landwerlin@intel.com Signed-off-by: Kumar, Kiran S kiran.s.kumar@intel.com Signed-off-by: Kausal Malladi kausalmalladi@gmail.com
The above should be kept in the order of which people worked on them.
Reviewed-by: Matt Roper matthew.d.roper@intel.com
--- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c
@@ -376,6 +377,57 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state, EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
/**
- drm_atomic_replace_property_blob - replace a blob property
- @blob: a pointer to the member blob to be replaced
- @new_blob: the new blob to replace with
- @expected_size: the expected size of the new blob
- @replaced: whether the blob has been replaced
- RETURNS:
- Zero on success, error code on failure
- */
+static int +drm_atomic_replace_property_blob(struct drm_property_blob **blob,
struct drm_property_blob *new_blob,bool *replaced)
"Replaced" here and though the rest of the patch is used as "changed". Worth naming it that way ?
+{
struct drm_property_blob *old_blob = *blob;if (old_blob == new_blob)return 0;if (old_blob)drm_property_unreference_blob(old_blob);if (new_blob)drm_property_reference_blob(new_blob);*blob = new_blob;*replaced = true;return 0;
The function always succeeds - drop the return value ?
+}
+static int +drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
struct drm_property_blob **blob,uint64_t blob_id,ssize_t expected_size,bool *replaced)+{
struct drm_device *dev = crtc->dev;struct drm_property_blob *new_blob = NULL;if (blob_id != 0) {new_blob = drm_property_lookup_blob(dev, blob_id);if (new_blob == NULL)return -EINVAL;if (expected_size > 0 && expected_size != new_blob->length)return -EINVAL;}
Having a look at drm_atomic_set_mode_prop_for_crtc() I think I can spot a bug - it shouldn't drop/unref the old blob in case of an error. A case you handle nicely here. Perhaps it's worth using the drm_atomic_replace_property_blob() in there ?
@@ -397,6 +449,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, { struct drm_device *dev = crtc->dev; struct drm_mode_config *config = &dev->mode_config;
bool replaced = false; int ret; if (property == config->prop_active)@@ -407,8 +460,31 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, ret = drm_atomic_set_mode_prop_for_crtc(state, mode); drm_property_unreference_blob(mode); return ret;
}else if (crtc->funcs->atomic_set_property)
} else if (property == config->degamma_lut_property) {ret = drm_atomic_replace_property_blob_from_id(crtc,&state->degamma_lut,val,-1,&replaced);state->color_mgmt_changed = replaced;return ret;
} else if (property == config->gamma_lut_property) {ret = drm_atomic_replace_property_blob_from_id(crtc,&state->gamma_lut,val,-1,
Wondering if these "-1" shouldn't be derived/replaced with the contents of the respective _size properly ?
@@ -444,6 +520,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, *val = state->active; else if (property == config->prop_mode_id) *val = (state->mode_blob) ? state->mode_blob->base.id : 0;
else if (property == config->degamma_lut_property)*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;else if (property == config->ctm_property)*val = (state->ctm) ? state->ctm->base.id : 0;else if (property == config->gamma_lut_property)*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0; else if (crtc->funcs->atomic_get_property) return crtc->funcs->atomic_get_property(crtc, state, property, val); elsediff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 4da4f2a..7ab8040 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2557,6 +2564,9 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc, struct drm_crtc_state *state) { drm_property_unreference_blob(state->mode_blob);
drm_property_unreference_blob(state->degamma_lut);drm_property_unreference_blob(state->ctm);drm_property_unreference_blob(state->gamma_lut);
Might want to keep the dtor in reverse order comparing to the ctor - duplicate_state()
@@ -2870,3 +2880,96 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector, kfree(state); } EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
+/**
- drm_atomic_helper_legacy_gamma_set - set the legacy gamma correction table
- @crtc: CRTC object
- @red: red correction table
- @green: green correction table
- @blue: green correction table
- @start:
- @size: size of the tables
- Implements support for legacy gamma correction table for drivers
- that support color management through the DEGAMMA_LUT/GAMMA_LUT
- properties.
- */
+void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
u16 *red, u16 *green, u16 *blue,uint32_t start, uint32_t size)+{
struct drm_device *dev = crtc->dev;struct drm_mode_config *config = &dev->mode_config;struct drm_atomic_state *state;struct drm_crtc_state *crtc_state;struct drm_property_blob *blob = NULL;struct drm_color_lut *blob_data;int i, ret = 0;state = drm_atomic_state_alloc(crtc->dev);if (!state)return;blob = drm_property_create_blob(dev,sizeof(struct drm_color_lut) * size,NULL);
To keep the bringup/teardown simpler (and complete): Move create_blob() before to state_alloc() and null check blob immediately. One would need to add unref_blob() when state_alloc() fails.
state->acquire_ctx = crtc->dev->mode_config.acquire_ctx;+retry:
crtc_state = drm_atomic_get_crtc_state(state, crtc);if (IS_ERR(crtc_state)) {ret = PTR_ERR(crtc_state);goto fail;}/* Reset DEGAMMA_LUT and CTM properties. */ret = drm_atomic_crtc_set_property(crtc, crtc_state,config->degamma_lut_property, 0);if (ret)goto fail;
Add new blank line please.
ret = drm_atomic_crtc_set_property(crtc, crtc_state,config->ctm_property, 0);if (ret)goto fail;/* Set GAMMA_LUT with legacy values. */if (blob == NULL) {ret = -ENOMEM;goto fail;}blob_data = (struct drm_color_lut *) blob->data;for (i = 0; i < size; i++) {blob_data[i].red = red[i];blob_data[i].green = green[i];blob_data[i].blue = blue[i];}
Move this loop after create_blob()
ret = drm_atomic_crtc_set_property(crtc, crtc_state,config->gamma_lut_property, blob->base.id);if (ret)goto fail;ret = drm_atomic_commit(state);if (ret != 0)
Please check in a consistent way. Currently we have ret != 0 vs ret and foo == NULL vs !foo.
goto fail;drm_property_unreference_blob(blob);/* Driver takes ownership of state on successful commit. */
Move the comment before unreference_blob(), so that it's closer to atomic_commit() ?
--- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1554,6 +1554,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.prop_mode_id = prop;
prop = drm_property_create(dev,DRM_MODE_PROP_BLOB,"DEGAMMA_LUT", 0);
Just wondering - don't we want this and the remaining properties to be atomic only ? I doubt we have userspace that [will be updated to] handle these, yet lacks atomic.
--- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -1075,3 +1075,36 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, return drm_plane_helper_commit(plane, plane_state, old_fb); } EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
+/**
- drm_helper_crtc_enable_color_mgmt - enable color management properties
- @crtc: DRM CRTC
- @degamma_lut_size: the size of the degamma lut (before CSC)
- @gamma_lut_size: the size of the gamma lut (after CSC)
- This function lets the driver enable the color correction properties on a
- CRTC. This includes 3 degamma, csc and gamma properties that userspace can
- set and 2 size properties to inform the userspace of the lut sizes.
- */
+void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
int degamma_lut_size,int gamma_lut_size)+{
struct drm_device *dev = crtc->dev;struct drm_mode_config *config = &dev->mode_config;drm_object_attach_property(&crtc->base,config->degamma_lut_property, 0);drm_object_attach_property(&crtc->base,config->ctm_property, 0);drm_object_attach_property(&crtc->base,config->gamma_lut_property, 0);drm_object_attach_property(&crtc->base,config->degamma_lut_size_property,degamma_lut_size);drm_object_attach_property(&crtc->base,config->gamma_lut_size_property,gamma_lut_size);
Wondering if we cannot have these listed like elsewhere in the patch. I.e. have the _size property just after its respective counterpart.
Regards, Emil
On Fri, Feb 26, 2016 at 12:36:12AM +0000, Emil Velikov wrote: ...
--- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1554,6 +1554,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.prop_mode_id = prop;
prop = drm_property_create(dev,DRM_MODE_PROP_BLOB,"DEGAMMA_LUT", 0);Just wondering - don't we want this and the remaining properties to be atomic only ? I doubt we have userspace that [will be updated to] handle these, yet lacks atomic.
I asked this on a previous version of the series as well since I thought I remembered Daniel Vetter indicating that the goal was to have new capabilities going forward should require atomic (even if the properties could still technically work okay via the legacy property interface). Daniel Stone felt it was probably fine to still allow it via legacy though:
https://lists.freedesktop.org/archives/intel-gfx/2016-January/086120.html
I personally don't have a strong feeling either way, but we should probably just make sure everyone is on the same page. If we decide as a community that we *do* want the atomic requirement going forward, maybe we can add a note about that to the kerneldoc or something so we remember in the future.
Matt
On 26/02/16 00:36, Emil Velikov wrote:
Hi Lionel,
A bunch of suggestions - feel free to take or ignore them :-)
On 25 February 2016 at 10:58, Lionel Landwerlin lionel.g.landwerlin@intel.com wrote:
Patch based on a previous series by Shashank Sharma.
This introduces optional properties to enable color correction at the pipe level. It relies on 3 transformations applied to every pixels displayed. First a lookup into a degamma table, then a multiplication of the rgb components by a 3x3 matrix and finally another lookup into a gamma table.
The following properties can be added to a pipe :
- DEGAMMA_LUT : blob containing degamma LUT
- DEGAMMA_LUT_SIZE : number of elements in DEGAMMA_LUT
- CTM : transformation matrix applied after the degamma LUT
- GAMMA_LUT : blob containing gamma LUT
- GAMMA_LUT_SIZE : number of elements in GAMMA_LUT
DEGAMMA_LUT_SIZE and GAMMA_LUT_SIZE are read only properties, set by the driver to tell userspace applications what sizes should be the lookup tables in DEGAMMA_LUT and GAMMA_LUT.
A helper is also provided so legacy gamma correction is redirected through these new properties.
v2: Register LUT size properties as range
v3: Fix round in drm_color_lut_get_value() helper More docs on how degamma/gamma properties are used
v4: Update contributors
v5: Rename CTM_MATRIX property to CTM (Doh!) Add legacy gamma_set atomic helper Describe CTM/LUT acronyms in the kernel doc
Signed-off-by: Shashank Sharma shashank.sharma@intel.com Signed-off-by: Lionel Landwerlin lionel.g.landwerlin@intel.com Signed-off-by: Kumar, Kiran S kiran.s.kumar@intel.com Signed-off-by: Kausal Malladi kausalmalladi@gmail.com
The above should be kept in the order of which people worked on them.
Reviewed-by: Matt Roper matthew.d.roper@intel.com --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -376,6 +377,57 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state, EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
/**
- drm_atomic_replace_property_blob - replace a blob property
- @blob: a pointer to the member blob to be replaced
- @new_blob: the new blob to replace with
- @expected_size: the expected size of the new blob
- @replaced: whether the blob has been replaced
- RETURNS:
- Zero on success, error code on failure
- */
+static int +drm_atomic_replace_property_blob(struct drm_property_blob **blob,
struct drm_property_blob *new_blob,bool *replaced)"Replaced" here and though the rest of the patch is used as "changed". Worth naming it that way ?
I think the former describes the action, the later the state.
+{
struct drm_property_blob *old_blob = *blob;if (old_blob == new_blob)return 0;if (old_blob)drm_property_unreference_blob(old_blob);if (new_blob)drm_property_reference_blob(new_blob);*blob = new_blob;*replaced = true;return 0;The function always succeeds - drop the return value ?
Well spotted, dropping.
+}
+static int +drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
struct drm_property_blob **blob,uint64_t blob_id,ssize_t expected_size,bool *replaced)+{
struct drm_device *dev = crtc->dev;struct drm_property_blob *new_blob = NULL;if (blob_id != 0) {new_blob = drm_property_lookup_blob(dev, blob_id);if (new_blob == NULL)return -EINVAL;if (expected_size > 0 && expected_size != new_blob->length)return -EINVAL;}Having a look at drm_atomic_set_mode_prop_for_crtc() I think I can spot a bug - it shouldn't drop/unref the old blob in case of an error. A case you handle nicely here. Perhaps it's worth using the drm_atomic_replace_property_blob() in there ?
I'm not sure it matters as the drm_crtc_state you're set properties on will be discarded if there is an error. The current drm_crtc_state that has been applied onto the hardware should be untouched.
@@ -397,6 +449,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, { struct drm_device *dev = crtc->dev; struct drm_mode_config *config = &dev->mode_config;
bool replaced = false; int ret; if (property == config->prop_active)@@ -407,8 +460,31 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, ret = drm_atomic_set_mode_prop_for_crtc(state, mode); drm_property_unreference_blob(mode); return ret;
}else if (crtc->funcs->atomic_set_property)
} else if (property == config->degamma_lut_property) {ret = drm_atomic_replace_property_blob_from_id(crtc,&state->degamma_lut,val,-1,&replaced);state->color_mgmt_changed = replaced;return ret;} else if (property == config->gamma_lut_property) {ret = drm_atomic_replace_property_blob_from_id(crtc,&state->gamma_lut,val,-1,Wondering if these "-1" shouldn't be derived/replaced with the contents of the respective _size properly ?
This is because we accept more than one size of degamma/gamma LUT (legacy -> 256 elements, new LUT -> (de)gamma_lut_size elements). It's up for the driver the check the size and raise an error in its atomic_check() vfunc.
@@ -444,6 +520,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, *val = state->active; else if (property == config->prop_mode_id) *val = (state->mode_blob) ? state->mode_blob->base.id : 0;
else if (property == config->degamma_lut_property)*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;else if (property == config->ctm_property)*val = (state->ctm) ? state->ctm->base.id : 0;else if (property == config->gamma_lut_property)*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0; else if (crtc->funcs->atomic_get_property) return crtc->funcs->atomic_get_property(crtc, state, property, val); elsediff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 4da4f2a..7ab8040 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2557,6 +2564,9 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc, struct drm_crtc_state *state) { drm_property_unreference_blob(state->mode_blob);
drm_property_unreference_blob(state->degamma_lut);drm_property_unreference_blob(state->ctm);drm_property_unreference_blob(state->gamma_lut);Might want to keep the dtor in reverse order comparing to the ctor - duplicate_state()
@@ -2870,3 +2880,96 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector, kfree(state); } EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
+/**
- drm_atomic_helper_legacy_gamma_set - set the legacy gamma correction table
- @crtc: CRTC object
- @red: red correction table
- @green: green correction table
- @blue: green correction table
- @start:
- @size: size of the tables
- Implements support for legacy gamma correction table for drivers
- that support color management through the DEGAMMA_LUT/GAMMA_LUT
- properties.
- */
+void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
u16 *red, u16 *green, u16 *blue,uint32_t start, uint32_t size)+{
struct drm_device *dev = crtc->dev;struct drm_mode_config *config = &dev->mode_config;struct drm_atomic_state *state;struct drm_crtc_state *crtc_state;struct drm_property_blob *blob = NULL;struct drm_color_lut *blob_data;int i, ret = 0;state = drm_atomic_state_alloc(crtc->dev);if (!state)return;blob = drm_property_create_blob(dev,sizeof(struct drm_color_lut) * size,NULL);To keep the bringup/teardown simpler (and complete): Move create_blob() before to state_alloc() and null check blob immediately. One would need to add unref_blob() when state_alloc() fails.
Moving the if (blob == NULL) right after the blob allocation to make it simpler.
What about completeness? Is there something inherently wrong here?
state->acquire_ctx = crtc->dev->mode_config.acquire_ctx;+retry:
crtc_state = drm_atomic_get_crtc_state(state, crtc);if (IS_ERR(crtc_state)) {ret = PTR_ERR(crtc_state);goto fail;}/* Reset DEGAMMA_LUT and CTM properties. */ret = drm_atomic_crtc_set_property(crtc, crtc_state,config->degamma_lut_property, 0);if (ret)goto fail;Add new blank line please.
Sure.
ret = drm_atomic_crtc_set_property(crtc, crtc_state,config->ctm_property, 0);if (ret)goto fail;/* Set GAMMA_LUT with legacy values. */if (blob == NULL) {ret = -ENOMEM;goto fail;}blob_data = (struct drm_color_lut *) blob->data;for (i = 0; i < size; i++) {blob_data[i].red = red[i];blob_data[i].green = green[i];blob_data[i].blue = blue[i];}Move this loop after create_blob()
Thanks, indeed no need to refill it in case of retry.
ret = drm_atomic_crtc_set_property(crtc, crtc_state,config->gamma_lut_property, blob->base.id);if (ret)goto fail;ret = drm_atomic_commit(state);if (ret != 0)Please check in a consistent way. Currently we have ret != 0 vs ret and foo == NULL vs !foo.
Sure.
goto fail;drm_property_unreference_blob(blob);/* Driver takes ownership of state on successful commit. */Move the comment before unreference_blob(), so that it's closer to atomic_commit() ?
Sure.
--- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1554,6 +1554,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.prop_mode_id = prop;
prop = drm_property_create(dev,DRM_MODE_PROP_BLOB,"DEGAMMA_LUT", 0);Just wondering - don't we want this and the remaining properties to be atomic only ? I doubt we have userspace that [will be updated to] handle these, yet lacks atomic.
This was pointed out by Matt already. Here is Daniel Stone's response : https://lists.freedesktop.org/archives/intel-gfx/2016-January/086120.html
I think it's fine to have these properties not atomic because it's not really something you update very often (maybe just when starting your UI). That's actually how we would like to use them in ChromiumOS as a first step, until eventually ChromiumOS switches to atomic.
--- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -1075,3 +1075,36 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, return drm_plane_helper_commit(plane, plane_state, old_fb); } EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
+/**
- drm_helper_crtc_enable_color_mgmt - enable color management properties
- @crtc: DRM CRTC
- @degamma_lut_size: the size of the degamma lut (before CSC)
- @gamma_lut_size: the size of the gamma lut (after CSC)
- This function lets the driver enable the color correction properties on a
- CRTC. This includes 3 degamma, csc and gamma properties that userspace can
- set and 2 size properties to inform the userspace of the lut sizes.
- */
+void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
int degamma_lut_size,int gamma_lut_size)+{
struct drm_device *dev = crtc->dev;struct drm_mode_config *config = &dev->mode_config;drm_object_attach_property(&crtc->base,config->degamma_lut_property, 0);drm_object_attach_property(&crtc->base,config->ctm_property, 0);drm_object_attach_property(&crtc->base,config->gamma_lut_property, 0);drm_object_attach_property(&crtc->base,config->degamma_lut_size_property,degamma_lut_size);drm_object_attach_property(&crtc->base,config->gamma_lut_size_property,gamma_lut_size);Wondering if we cannot have these listed like elsewhere in the patch. I.e. have the _size property just after its respective counterpart.
Regards, Emil
On 26 February 2016 at 15:43, Lionel Landwerlin lionel.g.landwerlin@intel.com wrote:
On 26/02/16 00:36, Emil Velikov wrote:
Hi Lionel,
A bunch of suggestions - feel free to take or ignore them :-)
On 25 February 2016 at 10:58, Lionel Landwerlin lionel.g.landwerlin@intel.com wrote:
I'm not sure it matters as the drm_crtc_state you're set properties on will be discarded if there is an error. The current drm_crtc_state that has been applied onto the hardware should be untouched.
That's the thing - the current drm_crts_state (mode_blob) is being discarded, as opposed to the newly setup one. Although I could have misunderstood something.
This is because we accept more than one size of degamma/gamma LUT (legacy -> 256 elements, new LUT -> (de)gamma_lut_size elements). It's up for the driver the check the size and raise an error in its atomic_check() vfunc.
Ahh yes the legacy vs atomic difference.
Moving the if (blob == NULL) right after the blob allocation to make it simpler.
What about completeness? Is there something inherently wrong here?
The suggestion to reorder things is mostly to keep the setup/teardown order in reverse order - create_blob, create_state and drop_state, drop_blob. As you've noticed, I'm kind of a sucker for those :-) There are no inter-dependencies that would require it here so it's not required.
state->acquire_ctx = crtc->dev->mode_config.acquire_ctx;+retry:
crtc_state = drm_atomic_get_crtc_state(state, crtc);if (IS_ERR(crtc_state)) {ret = PTR_ERR(crtc_state);goto fail;}/* Reset DEGAMMA_LUT and CTM properties. */ret = drm_atomic_crtc_set_property(crtc, crtc_state,config->degamma_lut_property, 0);if (ret)goto fail;Add new blank line please.
Sure.
ret = drm_atomic_crtc_set_property(crtc, crtc_state,config->ctm_property, 0);if (ret)goto fail;/* Set GAMMA_LUT with legacy values. */if (blob == NULL) {ret = -ENOMEM;goto fail;}blob_data = (struct drm_color_lut *) blob->data;for (i = 0; i < size; i++) {blob_data[i].red = red[i];blob_data[i].green = green[i];blob_data[i].blue = blue[i];}Move this loop after create_blob()
Thanks, indeed no need to refill it in case of retry.
ret = drm_atomic_crtc_set_property(crtc, crtc_state,config->gamma_lut_property, blob->base.id);if (ret)goto fail;ret = drm_atomic_commit(state);if (ret != 0)Please check in a consistent way. Currently we have ret != 0 vs ret and foo == NULL vs !foo.
Sure.
goto fail;drm_property_unreference_blob(blob);/* Driver takes ownership of state on successful commit. */Move the comment before unreference_blob(), so that it's closer to atomic_commit() ?
Sure.
--- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1554,6 +1554,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.prop_mode_id = prop;
prop = drm_property_create(dev,DRM_MODE_PROP_BLOB,"DEGAMMA_LUT", 0);Just wondering - don't we want this and the remaining properties to be atomic only ? I doubt we have userspace that [will be updated to] handle these, yet lacks atomic.
This was pointed out by Matt already. Here is Daniel Stone's response : https://lists.freedesktop.org/archives/intel-gfx/2016-January/086120.html
I think it's fine to have these properties not atomic because it's not really something you update very often (maybe just when starting your UI). That's actually how we would like to use them in ChromiumOS as a first step, until eventually ChromiumOS switches to atomic.
It wasn't a question of "can it be used" but more of "would it make sense to not switch to atomics once we're here". As is, userspace will need to have two slightly different code paths. Put the question "how to deal with if compositor crashes and (re)applying gamma/etc. multiple times" by Daniel Vettel, on top of it all and things get extra messy. In both usespace in kernel.
My line of thought is - if there is a high demand for non-atomic(legacy) degamma/etc. one can easily add it. On the other hand, once it lands one cannot remove the code from the kernel, ever.
It's up-to you guys really. Just thought I mentioned it.
-Emil
Patch based on a previous series by Shashank Sharma.
v2: Do not read GAMMA_MODE register to figure what mode we're in
v3: Program PREC_PAL_GC_MAX to clamp pixel values > 1.0
Add documentation on how the Broadcast RGB property is affected by CTM
v4: Update contributors
v5: Refactor degamma/gamma LUTs load into a single function
v6: Fix missing intel_crtc variable (bisect issue)
v7: Fix & simplify limited range matrix multiplication
Signed-off-by: Shashank Sharma shashank.sharma@intel.com Signed-off-by: Lionel Landwerlin lionel.g.landwerlin@intel.com Signed-off-by: Kumar, Kiran S kiran.s.kumar@intel.com Signed-off-by: Kausal Malladi kausalmalladi@gmail.com --- Documentation/DocBook/gpu.tmpl | 6 +- drivers/gpu/drm/i915/i915_drv.c | 24 ++- drivers/gpu/drm/i915/i915_drv.h | 6 + drivers/gpu/drm/i915/i915_reg.h | 22 +++ drivers/gpu/drm/i915/intel_color.c | 341 ++++++++++++++++++++++++++++++----- drivers/gpu/drm/i915/intel_display.c | 22 ++- drivers/gpu/drm/i915/intel_drv.h | 3 +- drivers/gpu/drm/i915/intel_fbdev.c | 8 + 8 files changed, 369 insertions(+), 63 deletions(-)
diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index 1692c4d..430e99b 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -2153,7 +2153,11 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >ENUM</td> <td valign="top" >{ "Automatic", "Full", "Limited 16:235" }</td> <td valign="top" >Connector</td> - <td valign="top" >TBD</td> + <td valign="top" >When this property is set to Limited 16:235 + and CTM is set, the hardware will be programmed with the + result of the multiplication of CTM by the limited range + matrix to ensure the pixels normaly in the range 0..1.0 are + remapped to the range 16/255..235/255.</td> </tr> <tr> <td valign="top" >“audio”</td> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 20e8200..3807b73 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -66,6 +66,9 @@ static struct drm_driver driver; #define IVB_CURSOR_OFFSETS \ .cursor_offsets = { CURSOR_A_OFFSET, IVB_CURSOR_B_OFFSET, IVB_CURSOR_C_OFFSET }
+#define BDW_COLORS \ + .color = { .degamma_lut_size = 512, .gamma_lut_size = 512 } + static const struct intel_device_info intel_i830_info = { .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2, .has_overlay = 1, .overlay_needs_physical = 1, @@ -288,24 +291,28 @@ static const struct intel_device_info intel_haswell_m_info = { .is_mobile = 1, };
+#define BDW_FEATURES \ + HSW_FEATURES, \ + BDW_COLORS + static const struct intel_device_info intel_broadwell_d_info = { - HSW_FEATURES, + BDW_FEATURES, .gen = 8, };
static const struct intel_device_info intel_broadwell_m_info = { - HSW_FEATURES, + BDW_FEATURES, .gen = 8, .is_mobile = 1, };
static const struct intel_device_info intel_broadwell_gt3d_info = { - HSW_FEATURES, + BDW_FEATURES, .gen = 8, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, };
static const struct intel_device_info intel_broadwell_gt3m_info = { - HSW_FEATURES, + BDW_FEATURES, .gen = 8, .is_mobile = 1, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, }; @@ -321,13 +328,13 @@ static const struct intel_device_info intel_cherryview_info = { };
static const struct intel_device_info intel_skylake_info = { - HSW_FEATURES, + BDW_FEATURES, .is_skylake = 1, .gen = 9, };
static const struct intel_device_info intel_skylake_gt3_info = { - HSW_FEATURES, + BDW_FEATURES, .is_skylake = 1, .gen = 9, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, @@ -345,17 +352,18 @@ static const struct intel_device_info intel_broxton_info = { .has_fbc = 1, GEN_DEFAULT_PIPEOFFSETS, IVB_CURSOR_OFFSETS, + BDW_COLORS, };
static const struct intel_device_info intel_kabylake_info = { - HSW_FEATURES, + BDW_FEATURES, .is_preliminary = 1, .is_kabylake = 1, .gen = 9, };
static const struct intel_device_info intel_kabylake_gt3_info = { - HSW_FEATURES, + BDW_FEATURES, .is_preliminary = 1, .is_kabylake = 1, .gen = 9, diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8cb4418..e71dc9a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -663,6 +663,7 @@ struct drm_i915_display_funcs { /* display clock increase/decrease */ /* pll clock increase/decrease */
+ void (*load_csc_matrix)(struct drm_crtc *crtc); void (*load_luts)(struct drm_crtc *crtc); };
@@ -812,6 +813,11 @@ struct intel_device_info { u8 has_slice_pg:1; u8 has_subslice_pg:1; u8 has_eu_pg:1; + + struct color_luts { + u16 degamma_lut_size; + u16 gamma_lut_size; + } color; };
#undef DEFINE_FLAG diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index f76cbf3..76a9e49 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7651,6 +7651,28 @@ enum skl_disp_power_wells { #define PIPE_CSC_POSTOFF_ME(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) #define PIPE_CSC_POSTOFF_LO(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO)
+/* pipe degamma/gamma LUTs on IVB+ */ +#define _PAL_PREC_INDEX_A 0x4A400 +#define _PAL_PREC_INDEX_B 0x4AC00 +#define _PAL_PREC_INDEX_C 0x4B400 +#define PAL_PREC_10_12_BIT (0 << 31) +#define PAL_PREC_SPLIT_MODE (1 << 31) +#define PAL_PREC_AUTO_INCREMENT (1 << 15) +#define _PAL_PREC_DATA_A 0x4A404 +#define _PAL_PREC_DATA_B 0x4AC04 +#define _PAL_PREC_DATA_C 0x4B404 +#define _PAL_PREC_GC_MAX_A 0x4A410 +#define _PAL_PREC_GC_MAX_B 0x4AC10 +#define _PAL_PREC_GC_MAX_C 0x4B410 +#define _PAL_PREC_EXT_GC_MAX_A 0x4A420 +#define _PAL_PREC_EXT_GC_MAX_B 0x4AC20 +#define _PAL_PREC_EXT_GC_MAX_C 0x4B420 + +#define PREC_PAL_INDEX(pipe) _MMIO_PIPE(pipe, _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B) +#define PREC_PAL_DATA(pipe) _MMIO_PIPE(pipe, _PAL_PREC_DATA_A, _PAL_PREC_DATA_B) +#define PREC_PAL_GC_MAX(pipe, i) _MMIO(_PIPE(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4) +#define PREC_PAL_EXT_GC_MAX(pipe, i) _MMIO(_PIPE(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4) + /* MIPI DSI registers */
#define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */ diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 16657eb..a6dc2af 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -24,39 +24,155 @@
#include "intel_drv.h"
+#define CTM_COEFF_SIGN (1ULL << 63) + +#define CTM_COEFF_1_0 (1ULL << 32) +#define CTM_COEFF_2_0 (CTM_COEFF_1_0 << 1) +#define CTM_COEFF_4_0 (CTM_COEFF_2_0 << 1) +#define CTM_COEFF_0_5 (CTM_COEFF_1_0 >> 1) +#define CTM_COEFF_0_25 (CTM_COEFF_0_5 >> 1) +#define CTM_COEFF_0_125 (CTM_COEFF_0_25 >> 1) + +#define CTM_COEFF_LIMITED_RANGE ((235ULL - 16ULL) * CTM_COEFF_1_0 / 255) + +#define CTM_COEFF_NEGATIVE(coeff) (((coeff) & CTM_COEFF_SIGN) != 0) +#define CTM_COEFF_ABS(coeff) ((coeff) & (CTM_COEFF_SIGN - 1)) + +#define LEGACY_LUT_LENGTH (sizeof(struct drm_color_lut) * 256) + /* - * Set up the pipe CSC unit. + * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point + * format). This macro takes the coefficient we want transformed and the + * number of fractional bits. * - * Currently only full range RGB to limited range RGB conversion - * is supported, but eventually this should handle various - * RGB<->YCbCr scenarios as well. + * We only have a 9 bits precision window which slides depending on the value + * of the CTM coefficient and we write the value from bit 3. We also round the + * value. */ +#define I9XX_CSC_COEFF_FP(coeff, fbits) \ + (clamp_val(((coeff) >> (32 - (fbits) - 3)) + 4, 0, 0xfff) & 0xff8) + +#define I9XX_CSC_COEFF_LIMITED_RANGE \ + I9XX_CSC_COEFF_FP(CTM_COEFF_LIMITED_RANGE, 9) +#define I9XX_CSC_COEFF_1_0 \ + ((7 << 12) | I9XX_CSC_COEFF_FP(CTM_COEFF_1_0, 8)) + +static bool crtc_state_is_legacy(struct drm_crtc_state *state) +{ + return !state->degamma_lut && + !state->ctm && + state->gamma_lut && + state->gamma_lut->length == LEGACY_LUT_LENGTH; +} + +/* + * When using limited range, multiply the matrix given by userspace by + * the matrix that we would use for the limited range. We do the + * multiplication in U2.30 format. + */ +static void ctm_mult_by_limited(uint64_t *result, int64_t *input) +{ + int i; + + for (i = 0; i < 9; i++) + result[i] = 0; + + for (i = 0; i < 3; i++) { + int64_t user_coeff = input[i * 3 + i]; + uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2; + uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff), + 0, + CTM_COEFF_4_0 - 1) >> 2; + + result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27; + if (CTM_COEFF_NEGATIVE(user_coeff)) + result[i * 3 + i] |= CTM_COEFF_SIGN; + } +} + +/* Set up the pipe CSC unit. */ static void i9xx_load_csc_matrix(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; + struct drm_crtc_state *crtc_state = crtc->state; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int pipe = intel_crtc->pipe; - uint16_t coeff = 0x7800; /* 1.0 */ + int i, pipe = intel_crtc->pipe; + uint16_t coeffs[9] = { 0, };
- /* - * TODO: Check what kind of values actually come out of the pipe - * with these coeff/postoff values and adjust to get the best - * accuracy. Perhaps we even need to take the bpc value into - * consideration. - */ + if (crtc_state->ctm) { + struct drm_color_ctm *ctm = + (struct drm_color_ctm *)crtc_state->ctm->data; + uint64_t input[9] = { 0, };
- if (intel_crtc->config->limited_color_range) - coeff = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */ + if (intel_crtc->config->limited_color_range) + ctm_mult_by_limited(input, ctm->matrix); + else { + for (i = 0; i < ARRAY_SIZE(input); i++) + input[i] = ctm->matrix[i]; + } + + /* + * Convert fixed point S31.32 input to format supported by the + * hardware. + */ + for (i = 0; i < ARRAY_SIZE(coeffs); i++) { + uint64_t abs_coeff = ((1ULL << 63) - 1) & input[i]; + + /* + * Clamp input value to min/max supported by + * hardware. + */ + abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_4_0 - 1); + + /* sign bit */ + if (CTM_COEFF_NEGATIVE(input[i])) + coeffs[i] |= 1 << 15; + + if (abs_coeff < CTM_COEFF_0_125) + coeffs[i] |= (3 << 12) | + I9XX_CSC_COEFF_FP(abs_coeff, 12); + else if (abs_coeff < CTM_COEFF_0_25) + coeffs[i] |= (2 << 12) | + I9XX_CSC_COEFF_FP(abs_coeff, 11); + else if (abs_coeff < CTM_COEFF_0_5) + coeffs[i] |= (1 << 12) | + I9XX_CSC_COEFF_FP(abs_coeff, 10); + else if (abs_coeff < CTM_COEFF_1_0) + coeffs[i] |= I9XX_CSC_COEFF_FP(abs_coeff, 9); + else if (abs_coeff < CTM_COEFF_2_0) + coeffs[i] |= (7 << 12) | + I9XX_CSC_COEFF_FP(abs_coeff, 8); + else + coeffs[i] |= (6 << 12) | + I9XX_CSC_COEFF_FP(abs_coeff, 7); + } + } else { + /* + * Load an identify matrix if no coefficients are provided. + * + * TODO: Check what kind of values actually come out of the + * pipe with these coeff/postoff values and adjust to get the + * best accuracy. Perhaps we even need to take the bpc value + * into consideration. + */ + for (i = 0; i < 3; i++) { + if (intel_crtc->config->limited_color_range) + coeffs[i * 3 + i] = + I9XX_CSC_COEFF_LIMITED_RANGE; + else + coeffs[i * 3 + i] = I9XX_CSC_COEFF_1_0; + } + }
- I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeff << 16); - I915_WRITE(PIPE_CSC_COEFF_BY(pipe), 0); + I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeffs[0] << 16 | coeffs[1]); + I915_WRITE(PIPE_CSC_COEFF_BY(pipe), coeffs[2] << 16);
- I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeff); - I915_WRITE(PIPE_CSC_COEFF_BU(pipe), 0); + I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeffs[3] << 16 | coeffs[4]); + I915_WRITE(PIPE_CSC_COEFF_BU(pipe), coeffs[5] << 16);
- I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), 0); - I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeff << 16); + I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), coeffs[6] << 16 | coeffs[7]); + I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeffs[8] << 16);
I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0); I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0); @@ -85,13 +201,18 @@ static void i9xx_load_csc_matrix(struct drm_crtc *crtc)
void intel_color_set_csc(struct drm_crtc *crtc) { - i9xx_load_csc_matrix(crtc); + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + if (dev_priv->display.load_csc_matrix) + dev_priv->display.load_csc_matrix(crtc); }
-/* Loads the palette/gamma unit for the CRTC with the prepared values. */ +/* Loads the legacy palette/gamma unit for the CRTC. */ static void i9xx_load_luts(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; + struct drm_crtc_state *state = crtc->state; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); enum pipe pipe = intel_crtc->pipe; @@ -104,18 +225,33 @@ static void i9xx_load_luts(struct drm_crtc *crtc) assert_pll_enabled(dev_priv, pipe); }
- for (i = 0; i < 256; i++) { - uint32_t word = (intel_crtc->lut_r[i] << 16) | - (intel_crtc->lut_g[i] << 8) | - intel_crtc->lut_b[i]; - if (HAS_GMCH_DISPLAY(dev)) - I915_WRITE(PALETTE(pipe, i), word); - else - I915_WRITE(LGC_PALETTE(pipe, i), word); + if (state->gamma_lut) { + struct drm_color_lut *lut = + (struct drm_color_lut *) state->gamma_lut->data; + for (i = 0; i < 256; i++) { + uint32_t word = + (drm_color_lut_extract(lut[i].red, 8) << 16) | + (drm_color_lut_extract(lut[i].green, 8) << 8) | + drm_color_lut_extract(lut[i].blue, 8); + + if (HAS_GMCH_DISPLAY(dev)) + I915_WRITE(PALETTE(pipe, i), word); + else + I915_WRITE(LGC_PALETTE(pipe, i), word); + } + } else { + for (i = 0; i < 256; i++) { + uint32_t word = (i << 16) | (i << 8) | i; + + if (HAS_GMCH_DISPLAY(dev)) + I915_WRITE(PALETTE(pipe, i), word); + else + I915_WRITE(LGC_PALETTE(pipe, i), word); + } } }
-/* Loads the legacy palette/gamma unit for the CRTC on Haswell+. */ +/* Loads the legacy palette/gamma unit for the CRTC on Haswell. */ static void haswell_load_luts(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -144,6 +280,89 @@ static void haswell_load_luts(struct drm_crtc *crtc) hsw_enable_ips(intel_crtc); }
+/* Loads the palette/gamma unit for the CRTC on Broadwell+. */ +static void broadwell_load_luts(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_crtc_state *state = crtc->state; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc_state *intel_state = to_intel_crtc_state(state); + enum pipe pipe = to_intel_crtc(crtc)->pipe; + uint32_t i, lut_size = INTEL_INFO(dev)->color.degamma_lut_size; + + if (crtc_state_is_legacy(state)) { + haswell_load_luts(crtc); + return; + } + + I915_WRITE(PREC_PAL_INDEX(pipe), + PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT); + + if (state->degamma_lut) { + struct drm_color_lut *lut = + (struct drm_color_lut *) state->degamma_lut->data; + + for (i = 0; i < lut_size; i++) { + uint32_t word = + drm_color_lut_extract(lut[i].red, 10) << 20 | + drm_color_lut_extract(lut[i].green, 10) << 10 | + drm_color_lut_extract(lut[i].blue, 10); + + I915_WRITE(PREC_PAL_DATA(pipe), word); + } + } else { + for (i = 0; i < lut_size; i++) { + uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1); + + I915_WRITE(PREC_PAL_DATA(pipe), + (v << 20) | (v << 10) | v); + } + } + + if (state->gamma_lut) { + struct drm_color_lut *lut = + (struct drm_color_lut *) state->gamma_lut->data; + + for (i = 0; i < lut_size; i++) { + uint32_t word = + (drm_color_lut_extract(lut[i].red, 10) << 20) | + (drm_color_lut_extract(lut[i].green, 10) << 10) | + drm_color_lut_extract(lut[i].blue, 10); + + I915_WRITE(PREC_PAL_DATA(pipe), word); + } + + /* Program the max register to clamp values > 1.0. */ + I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), + drm_color_lut_extract(lut[i].red, 16)); + I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), + drm_color_lut_extract(lut[i].green, 16)); + I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), + drm_color_lut_extract(lut[i].blue, 16)); + } else { + for (i = 0; i < lut_size; i++) { + uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1); + + I915_WRITE(PREC_PAL_DATA(pipe), + (v << 20) | (v << 10) | v); + } + + I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16) - 1); + I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16) - 1); + I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16) - 1); + } + + intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT; + I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT); + POSTING_READ(GAMMA_MODE(pipe)); + + /* + * Reset the index, otherwise it prevents the legacy palette to be + * written properly. + */ + I915_WRITE(PREC_PAL_INDEX(pipe), 0); +} + void intel_color_load_luts(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -156,39 +375,61 @@ void intel_color_load_luts(struct drm_crtc *crtc) dev_priv->display.load_luts(crtc); }
-void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, - u16 *blue, uint32_t start, uint32_t size) +int intel_color_check(struct drm_crtc *crtc, + struct drm_crtc_state *crtc_state) { - int end = (start + size > 256) ? 256 : start + size, i; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct drm_device *dev = crtc->dev; + size_t gamma_length, degamma_length;
- for (i = start; i < end; i++) { - intel_crtc->lut_r[i] = red[i] >> 8; - intel_crtc->lut_g[i] = green[i] >> 8; - intel_crtc->lut_b[i] = blue[i] >> 8; - } + degamma_length = INTEL_INFO(dev)->color.degamma_lut_size * + sizeof(struct drm_color_lut); + gamma_length = INTEL_INFO(dev)->color.gamma_lut_size * + sizeof(struct drm_color_lut); + + /* + * We allow both degamma & gamma luts at the right size or + * NULL. + */ + if ((!crtc_state->degamma_lut || + crtc_state->degamma_lut->length == degamma_length) && + (!crtc_state->gamma_lut || + crtc_state->gamma_lut->length == gamma_length)) + return 0; + + /* + * We also allow no degamma lut and a gamma lut at the legacy + * size (256 entries). + */ + if (!crtc_state->degamma_lut && + crtc_state->gamma_lut && + crtc_state->gamma_lut->length == LEGACY_LUT_LENGTH) + return 0;
- intel_color_load_luts(crtc); + return -EINVAL; }
void intel_color_init(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int i;
drm_mode_crtc_set_gamma_size(crtc, 256); - for (i = 0; i < 256; i++) { - intel_crtc->lut_r[i] = i; - intel_crtc->lut_g[i] = i; - intel_crtc->lut_b[i] = i; - }
- if (IS_HASWELL(dev) || - (INTEL_INFO(dev)->gen >= 8 && !IS_CHERRYVIEW(dev))) { + if (IS_HASWELL(dev)) { + dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix; dev_priv->display.load_luts = haswell_load_luts; + } else if (IS_BROADWELL(dev) || IS_SKYLAKE(dev) || + IS_BROXTON(dev) || IS_KABYLAKE(dev)) { + dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix; + dev_priv->display.load_luts = broadwell_load_luts; } else { dev_priv->display.load_luts = i9xx_load_luts; } + + /* Enable color management support when we have degamma & gamma LUTs. */ + if (INTEL_INFO(dev)->color.degamma_lut_size != 0 && + INTEL_INFO(dev)->color.gamma_lut_size != 0) + drm_helper_crtc_enable_color_mgmt(crtc, + INTEL_INFO(dev)->color.degamma_lut_size, + INTEL_INFO(dev)->color.gamma_lut_size); } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 19f8284..5a9acdd 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11848,6 +11848,12 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, return ret; }
+ if (crtc_state->color_mgmt_changed) { + ret = intel_color_check(crtc, crtc_state); + if (ret) + return ret; + } + ret = 0; if (dev_priv->display.compute_pipe_wm) { ret = dev_priv->display.compute_pipe_wm(intel_crtc, state); @@ -11869,7 +11875,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
static const struct drm_crtc_helper_funcs intel_helper_funcs = { .mode_set_base_atomic = intel_pipe_set_base_atomic, - .load_lut = intel_color_load_luts, .atomic_begin = intel_begin_crtc_commit, .atomic_flush = intel_finish_crtc_commit, .atomic_check = intel_crtc_atomic_check, @@ -13400,6 +13405,18 @@ static int intel_atomic_commit(struct drm_device *dev, hw_check = true; }
+ if (!modeset && + crtc->state->active && + crtc->state->color_mgmt_changed) { + /* + * Only update color management when not doing + * a modeset as this will be done by + * crtc_enable already. + */ + intel_color_set_csc(crtc); + intel_color_load_luts(crtc); + } + if (!modeset) intel_pre_plane_update(to_intel_crtc_state(crtc_state));
@@ -13489,8 +13506,9 @@ out: #undef for_each_intel_crtc_masked
static const struct drm_crtc_funcs intel_crtc_funcs = { - .gamma_set = intel_color_legacy_gamma_set, + .gamma_set = drm_atomic_helper_legacy_gamma_set, .set_config = drm_atomic_helper_set_config, + .set_property = drm_atomic_helper_crtc_set_property, .destroy = intel_crtc_destroy, .page_flip = intel_crtc_page_flip, .atomic_duplicate_state = intel_crtc_duplicate_state, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 8c48131..baa6adb 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1626,9 +1626,8 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
/* intel_color.c */ void intel_color_init(struct drm_crtc *crtc); +int intel_color_check(struct drm_crtc *crtc, struct drm_crtc_state *state); void intel_color_set_csc(struct drm_crtc *crtc); void intel_color_load_luts(struct drm_crtc *crtc); -void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, - u16 *blue, uint32_t start, uint32_t size);
#endif /* __INTEL_DRV_H__ */ diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 97a91e6..777f98c 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -379,6 +379,7 @@ retry: struct drm_connector *connector; struct drm_encoder *encoder; struct drm_fb_helper_crtc *new_crtc; + struct intel_crtc *intel_crtc;
fb_conn = fb_helper->connector_info[i]; connector = fb_conn->connector; @@ -420,6 +421,13 @@ retry:
num_connectors_enabled++;
+ intel_crtc = to_intel_crtc(connector->state->crtc); + for (j = 0; j < 256; j++) { + intel_crtc->lut_r[j] = j; + intel_crtc->lut_g[j] = j; + intel_crtc->lut_b[j] = j; + } + new_crtc = intel_fb_helper_crtc(fb_helper, connector->state->crtc);
/*
Patch based on a previous series by Shashank Sharma.
v2: Update contributors
v3: Refactor degamma/gamma LUTs load into a single function
v4: Remove unused variable
Signed-off-by: Shashank Sharma shashank.sharma@intel.com Signed-off-by: Lionel Landwerlin lionel.g.landwerlin@intel.com Signed-off-by: Kumar, Kiran S kiran.s.kumar@intel.com Signed-off-by: Kausal Malladi kausalmalladi@gmail.com --- drivers/gpu/drm/i915/i915_drv.c | 3 + drivers/gpu/drm/i915/i915_reg.h | 31 +++++++++ drivers/gpu/drm/i915/intel_color.c | 133 +++++++++++++++++++++++++++++++++++-- 3 files changed, 161 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 3807b73..8a2aaa7 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -68,6 +68,8 @@ static struct drm_driver driver;
#define BDW_COLORS \ .color = { .degamma_lut_size = 512, .gamma_lut_size = 512 } +#define CHV_COLORS \ + .color = { .degamma_lut_size = 65, .gamma_lut_size = 257 }
static const struct intel_device_info intel_i830_info = { .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2, @@ -325,6 +327,7 @@ static const struct intel_device_info intel_cherryview_info = { .display_mmio_offset = VLV_DISPLAY_BASE, GEN_CHV_PIPEOFFSETS, CURSOR_OFFSETS, + CHV_COLORS, };
static const struct intel_device_info intel_skylake_info = { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 76a9e49..ea599a9 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7673,6 +7673,37 @@ enum skl_disp_power_wells { #define PREC_PAL_GC_MAX(pipe, i) _MMIO(_PIPE(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4) #define PREC_PAL_EXT_GC_MAX(pipe, i) _MMIO(_PIPE(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4)
+/* pipe CSC & degamma/gamma LUTs on CHV */ +#define _CGM_PIPE_A_CSC_COEFF01 (VLV_DISPLAY_BASE + 0x67900) +#define _CGM_PIPE_A_CSC_COEFF23 (VLV_DISPLAY_BASE + 0x67904) +#define _CGM_PIPE_A_CSC_COEFF45 (VLV_DISPLAY_BASE + 0x67908) +#define _CGM_PIPE_A_CSC_COEFF67 (VLV_DISPLAY_BASE + 0x6790C) +#define _CGM_PIPE_A_CSC_COEFF8 (VLV_DISPLAY_BASE + 0x67910) +#define _CGM_PIPE_A_DEGAMMA (VLV_DISPLAY_BASE + 0x66000) +#define _CGM_PIPE_A_GAMMA (VLV_DISPLAY_BASE + 0x67000) +#define _CGM_PIPE_A_MODE (VLV_DISPLAY_BASE + 0x67A00) +#define CGM_PIPE_MODE_GAMMA (1 << 2) +#define CGM_PIPE_MODE_CSC (1 << 1) +#define CGM_PIPE_MODE_DEGAMMA (1 << 0) + +#define _CGM_PIPE_B_CSC_COEFF01 (VLV_DISPLAY_BASE + 0x69900) +#define _CGM_PIPE_B_CSC_COEFF23 (VLV_DISPLAY_BASE + 0x69904) +#define _CGM_PIPE_B_CSC_COEFF45 (VLV_DISPLAY_BASE + 0x69908) +#define _CGM_PIPE_B_CSC_COEFF67 (VLV_DISPLAY_BASE + 0x6990C) +#define _CGM_PIPE_B_CSC_COEFF8 (VLV_DISPLAY_BASE + 0x69910) +#define _CGM_PIPE_B_DEGAMMA (VLV_DISPLAY_BASE + 0x68000) +#define _CGM_PIPE_B_GAMMA (VLV_DISPLAY_BASE + 0x69000) +#define _CGM_PIPE_B_MODE (VLV_DISPLAY_BASE + 0x69A00) + +#define CGM_PIPE_CSC_COEFF01(pipe) _MMIO_PIPE(pipe, _CGM_PIPE_A_CSC_COEFF01, _CGM_PIPE_B_CSC_COEFF01) +#define CGM_PIPE_CSC_COEFF23(pipe) _MMIO_PIPE(pipe, _CGM_PIPE_A_CSC_COEFF23, _CGM_PIPE_B_CSC_COEFF23) +#define CGM_PIPE_CSC_COEFF45(pipe) _MMIO_PIPE(pipe, _CGM_PIPE_A_CSC_COEFF45, _CGM_PIPE_B_CSC_COEFF45) +#define CGM_PIPE_CSC_COEFF67(pipe) _MMIO_PIPE(pipe, _CGM_PIPE_A_CSC_COEFF67, _CGM_PIPE_B_CSC_COEFF67) +#define CGM_PIPE_CSC_COEFF8(pipe) _MMIO_PIPE(pipe, _CGM_PIPE_A_CSC_COEFF8, _CGM_PIPE_B_CSC_COEFF8) +#define CGM_PIPE_DEGAMMA(pipe, i, w) _MMIO(_PIPE(pipe, _CGM_PIPE_A_DEGAMMA, _CGM_PIPE_B_DEGAMMA) + (i) * 8 + (w) * 4) +#define CGM_PIPE_GAMMA(pipe, i, w) _MMIO(_PIPE(pipe, _CGM_PIPE_A_GAMMA, _CGM_PIPE_B_GAMMA) + (i) * 8 + (w) * 4) +#define CGM_PIPE_MODE(pipe) _MMIO_PIPE(pipe, _CGM_PIPE_A_MODE, _CGM_PIPE_B_MODE) + /* MIPI DSI registers */
#define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */ diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index a6dc2af..9ad37e5 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -29,6 +29,7 @@ #define CTM_COEFF_1_0 (1ULL << 32) #define CTM_COEFF_2_0 (CTM_COEFF_1_0 << 1) #define CTM_COEFF_4_0 (CTM_COEFF_2_0 << 1) +#define CTM_COEFF_8_0 (CTM_COEFF_4_0 << 1) #define CTM_COEFF_0_5 (CTM_COEFF_1_0 >> 1) #define CTM_COEFF_0_25 (CTM_COEFF_0_5 >> 1) #define CTM_COEFF_0_125 (CTM_COEFF_0_25 >> 1) @@ -199,6 +200,58 @@ static void i9xx_load_csc_matrix(struct drm_crtc *crtc) } }
+/* + * Set up the pipe CSC unit on CherryView. + */ +static void cherryview_load_csc_matrix(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_crtc_state *state = crtc->state; + struct drm_i915_private *dev_priv = dev->dev_private; + int pipe = to_intel_crtc(crtc)->pipe; + uint32_t mode; + + if (state->ctm) { + struct drm_color_ctm *ctm = + (struct drm_color_ctm *) state->ctm->data; + uint16_t coeffs[9] = { 0, }; + int i; + + for (i = 0; i < ARRAY_SIZE(coeffs); i++) { + uint64_t abs_coeff = + ((1ULL << 63) - 1) & ctm->matrix[i]; + + /* Round coefficient. */ + abs_coeff += 1 << (32 - 13); + /* Clamp to hardware limits. */ + abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_8_0 - 1); + + /* Write coefficients in S3.12 format. */ + if (ctm->matrix[i] & (1ULL << 63)) + coeffs[i] = 1 << 15; + coeffs[i] |= ((abs_coeff >> 32) & 7) << 12; + coeffs[i] |= (abs_coeff >> 20) & 0xfff; + } + + I915_WRITE(CGM_PIPE_CSC_COEFF01(pipe), + coeffs[1] << 16 | coeffs[0]); + I915_WRITE(CGM_PIPE_CSC_COEFF23(pipe), + coeffs[3] << 16 | coeffs[2]); + I915_WRITE(CGM_PIPE_CSC_COEFF45(pipe), + coeffs[5] << 16 | coeffs[4]); + I915_WRITE(CGM_PIPE_CSC_COEFF67(pipe), + coeffs[7] << 16 | coeffs[6]); + I915_WRITE(CGM_PIPE_CSC_COEFF8(pipe), coeffs[8]); + } + + mode = (state->ctm ? CGM_PIPE_MODE_CSC : 0); + if (!crtc_state_is_legacy(state)) { + mode |= (state->degamma_lut ? CGM_PIPE_MODE_DEGAMMA : 0) | + (state->gamma_lut ? CGM_PIPE_MODE_GAMMA : 0); + } + I915_WRITE(CGM_PIPE_MODE(pipe), mode); +} + void intel_color_set_csc(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -209,10 +262,10 @@ void intel_color_set_csc(struct drm_crtc *crtc) }
/* Loads the legacy palette/gamma unit for the CRTC. */ -static void i9xx_load_luts(struct drm_crtc *crtc) +static void i9xx_load_luts_internal(struct drm_crtc *crtc, + struct drm_property_blob *blob) { struct drm_device *dev = crtc->dev; - struct drm_crtc_state *state = crtc->state; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); enum pipe pipe = intel_crtc->pipe; @@ -225,9 +278,8 @@ static void i9xx_load_luts(struct drm_crtc *crtc) assert_pll_enabled(dev_priv, pipe); }
- if (state->gamma_lut) { - struct drm_color_lut *lut = - (struct drm_color_lut *) state->gamma_lut->data; + if (blob) { + struct drm_color_lut *lut = (struct drm_color_lut *) blob->data; for (i = 0; i < 256; i++) { uint32_t word = (drm_color_lut_extract(lut[i].red, 8) << 16) | @@ -251,6 +303,11 @@ static void i9xx_load_luts(struct drm_crtc *crtc) } }
+static void i9xx_load_luts(struct drm_crtc *crtc) +{ + i9xx_load_luts_internal(crtc, crtc->state->gamma_lut); +} + /* Loads the legacy palette/gamma unit for the CRTC on Haswell. */ static void haswell_load_luts(struct drm_crtc *crtc) { @@ -363,6 +420,67 @@ static void broadwell_load_luts(struct drm_crtc *crtc) I915_WRITE(PREC_PAL_INDEX(pipe), 0); }
+/* Loads the palette/gamma unit for the CRTC on CherryView. */ +static void cherryview_load_luts(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_crtc_state *state = crtc->state; + enum pipe pipe = to_intel_crtc(crtc)->pipe; + struct drm_color_lut *lut; + uint32_t i, lut_size; + uint32_t word0, word1; + + if (crtc_state_is_legacy(state)) { + /* Turn off degamma/gamma on CGM block. */ + I915_WRITE(CGM_PIPE_MODE(pipe), + (state->ctm ? CGM_PIPE_MODE_CSC : 0)); + i9xx_load_luts_internal(crtc, state->gamma_lut); + return; + } + + if (state->degamma_lut) { + lut = (struct drm_color_lut *) state->degamma_lut->data; + lut_size = INTEL_INFO(dev)->color.degamma_lut_size; + for (i = 0; i < lut_size; i++) { + /* Write LUT in U0.14 format. */ + word0 = + (drm_color_lut_extract(lut[i].green, 14) << 16) | + drm_color_lut_extract(lut[i].blue, 14); + word1 = drm_color_lut_extract(lut[i].red, 14); + + I915_WRITE(CGM_PIPE_DEGAMMA(pipe, i, 0), word0); + I915_WRITE(CGM_PIPE_DEGAMMA(pipe, i, 1), word1); + } + } + + if (state->gamma_lut) { + lut = (struct drm_color_lut *) state->gamma_lut->data; + lut_size = INTEL_INFO(dev)->color.gamma_lut_size; + for (i = 0; i < lut_size; i++) { + /* Write LUT in U0.10 format. */ + word0 = + (drm_color_lut_extract(lut[i].green, 10) << 16) | + drm_color_lut_extract(lut[i].blue, 10); + word1 = drm_color_lut_extract(lut[i].red, 10); + + I915_WRITE(CGM_PIPE_GAMMA(pipe, i, 0), word0); + I915_WRITE(CGM_PIPE_GAMMA(pipe, i, 1), word1); + } + } + + I915_WRITE(CGM_PIPE_MODE(pipe), + (state->ctm ? CGM_PIPE_MODE_CSC : 0) | + (state->degamma_lut ? CGM_PIPE_MODE_DEGAMMA : 0) | + (state->gamma_lut ? CGM_PIPE_MODE_GAMMA : 0)); + + /* + * Also program a linear LUT in the legacy block (behind the + * CGM block). + */ + i9xx_load_luts_internal(crtc, NULL); +} + void intel_color_load_luts(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -415,7 +533,10 @@ void intel_color_init(struct drm_crtc *crtc)
drm_mode_crtc_set_gamma_size(crtc, 256);
- if (IS_HASWELL(dev)) { + if (IS_CHERRYVIEW(dev)) { + dev_priv->display.load_csc_matrix = cherryview_load_csc_matrix; + dev_priv->display.load_luts = cherryview_load_luts; + } else if (IS_HASWELL(dev)) { dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix; dev_priv->display.load_luts = haswell_load_luts; } else if (IS_BROADWELL(dev) || IS_SKYLAKE(dev) ||
dri-devel@lists.freedesktop.org
-
Emil Velikov -
Lionel Landwerlin -
Matt Roper