On Thu, Jan 26, 2017 at 01:24:24PM +0200, Ander Conselvan de Oliveira wrote:
> The gamma tables in Geminilake were changed. There is no split-gamma
> mode. Instead, there is a dedicated degamma table that is enabled
> whenever pipe CSC is enabled.
>
> The dedicated gamma table has 16 bit precision but doesn't support
> separate channels. Since that doesn't match the per-channel format of
> the degamma LUT property, for now only a linear table is loaded and the
> property ignored.
>
> v2: Remove empty line. (Ville)
> Reuse broadwell code. (Ville)
>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Signed-off-by: Ander Conselvan de Oliveira
> <ander.conselvan.de.olive...@intel.com>
> ---
> drivers/gpu/drm/i915/i915_pci.c | 1 +
> drivers/gpu/drm/i915/i915_reg.h | 14 +++++++++
> drivers/gpu/drm/i915/intel_color.c | 60
> +++++++++++++++++++++++++++++++++++++-
> 3 files changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index ecb487b..df2051b 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -403,6 +403,7 @@ static const struct intel_device_info
> intel_geminilake_info = {
> .platform = INTEL_GEMINILAKE,
> .is_alpha_support = 1,
> .ddb_size = 1024,
> + .color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 }
> };
>
> static const struct intel_device_info intel_kabylake_info = {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 06bbe55..e029691 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8181,12 +8181,26 @@ enum {
> #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 _PAL_PREC_EXT2_GC_MAX_A 0x4A430
> +#define _PAL_PREC_EXT2_GC_MAX_B 0x4AC30
> +#define _PAL_PREC_EXT2_GC_MAX_C 0x4B430
>
> #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)
>
> +#define _PRE_CSC_GAMC_INDEX_A 0x4A484
> +#define _PRE_CSC_GAMC_INDEX_B 0x4AC84
> +#define _PRE_CSC_GAMC_INDEX_C 0x4B484
> +#define PRE_CSC_GAMC_AUTO_INCREMENT (1 << 10)
> +#define _PRE_CSC_GAMC_DATA_A 0x4A488
> +#define _PRE_CSC_GAMC_DATA_B 0x4AC88
> +#define _PRE_CSC_GAMC_DATA_C 0x4B488
> +
> +#define PRE_CSC_GAMC_INDEX(pipe) _MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A,
> _PRE_CSC_GAMC_INDEX_B)
> +#define PRE_CSC_GAMC_DATA(pipe) _MMIO_PIPE(pipe,
> _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
> +
> /* 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)
> diff --git a/drivers/gpu/drm/i915/intel_color.c
> b/drivers/gpu/drm/i915/intel_color.c
> index 82a3bc9..2125aa3 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -380,7 +380,9 @@ static void bdw_load_gamma_lut(struct drm_crtc_state
> *state, u32 offset)
> WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
>
> I915_WRITE(PREC_PAL_INDEX(pipe),
> - PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | offset);
> + (offset ? PAL_PREC_SPLIT_MODE : 0) |
> + PAL_PREC_AUTO_INCREMENT |
> + offset);
This confused me for a bit. I was thinking we're using this to write the
deamma part for the split gamma case as well, which would end up
disabling the split gamma mode when doing that. But that's not actually
what's happening since you had another function to write the degamma
half.
>
> if (state->gamma_lut) {
> struct drm_color_lut *lut =
> @@ -443,6 +445,59 @@ static void broadwell_load_luts(struct drm_crtc_state
> *state)
> I915_WRITE(PREC_PAL_INDEX(pipe), 0);
> }
>
> +static void glk_load_degamma_lut(struct drm_crtc_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
> + enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
> + const uint32_t lut_size = 33;
> + uint32_t i;
> +
> + /*
> + * When setting the auto-increment bit, the hardware seems to
> + * ignore the index bits, so we need to reset it to index 0
> + * separately.
> + */
Interesting. Do we know if the same problem might be present in other
gamma tables?
> + I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
> + I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT);
> +
> + /*
> + * FIXME: The pipe degamma table in geminilake doesn't support
> + * different values per channel, so this just loads a linear table.
> + */
> + for (i = 0; i < lut_size; i++) {
> + uint32_t v = (i * ((1 << 16) - 1)) / (lut_size - 1);
> +
> + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
> + }
> +
> + /* Clamp values > 1.0. */
> + while (i++ < 35)
> + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16) - 1);
> +}
> +
> +static void glk_load_luts(struct drm_crtc_state *state)
> +{
> + struct drm_crtc *crtc = state->crtc;
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
> + enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +
> + if (crtc_state_is_legacy(state)) {
> + haswell_load_luts(state);
> + return;
> + }
> +
> + glk_load_degamma_lut(state);
> + bdw_load_gamma_lut(state, 0);
> +
> + intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
> + I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
> + POSTING_READ(GAMMA_MODE(pipe));
> +
> + I915_WRITE(PIPE_CSC_MODE(pipe), 0);
Why are we writing the CSC_MODE register here?
> +}
> +
> /* Loads the palette/gamma unit for the CRTC on CherryView. */
> static void cherryview_load_luts(struct drm_crtc_state *state)
> {
> @@ -561,6 +616,9 @@ void intel_color_init(struct drm_crtc *crtc)
> IS_BROXTON(dev_priv)) {
> dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
> dev_priv->display.load_luts = broadwell_load_luts;
> + } else if (IS_GEMINILAKE(dev_priv)) {
> + dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
> + dev_priv->display.load_luts = glk_load_luts;
> } else {
> dev_priv->display.load_luts = i9xx_load_luts;
> }
> --
> 2.5.5
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
