Thanks for your comments, I tried to implement a "flag driven version" below.
On Fri, Nov 11, 2016 at 2:57 PM, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
On Fri, Nov 11, 2016 at 09:53:35AM +0100, Peter Frühberger wrote:
> Hi,
>
> I was implementing this suggestion today and I think that will confuse
> users and also devs maintaining that code. Out of the following reason:
> compress_color_range can be true or false, it does not reference a mode,
> but an additional setting that only influences color clamping / scaling.
>
> What do we expect if someone runs in Full Range and has
> compress_color_range set to true? Also what do we expect if someone runs in
> Limited Range and additionally set this flag to true? In some cases it
> would do nothing and was not transparent.
I didn't mean you should add a new property for this. Just an internal
flag.
[...]
In my opinion that makes maintainability much harder. Looking forward to your comments.
From f3bccf1611108247add0d85e8faed5430971cd71 Mon Sep 17 00:00:00 2001 From: fritsch <peter.fruehberger@xxxxxxxxx> Date: Sat, 16 Sep 2017 13:54:06 +0200 Subject: [PATCH] i915: Implement uncompressed Video Range output --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_color.c | 12 ++++++++---- drivers/gpu/drm/i915/intel_display.c | 6 ++++-- drivers/gpu/drm/i915/intel_dp.c | 3 ++- drivers/gpu/drm/i915/intel_drv.h | 11 +++++++++-- drivers/gpu/drm/i915/intel_hdmi.c | 16 +++++++++++++--- drivers/gpu/drm/i915/intel_modes.c | 1 + 7 files changed, 38 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5aecbf795b55..70bd525317c8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -4230,6 +4230,7 @@ __raw_write(64, q) #define INTEL_BROADCAST_RGB_AUTO 0 #define INTEL_BROADCAST_RGB_FULL 1 #define INTEL_BROADCAST_RGB_LIMITED 2 +#define INTEL_BROADCAST_RGB_VIDEO 3 static inline i915_reg_t i915_vgacntrl_reg(struct drm_i915_private *dev_priv) { diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index ff9ecd211abb..b72477a43ea4 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -149,7 +149,8 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state) (struct drm_color_ctm *)crtc_state->ctm->data; uint64_t input[9] = { 0, }; - if (intel_crtc_state->limited_color_range) { + if (intel_crtc_state->limited_color_range && + intel_crtc_state->compress_range) { ctm_mult_by_limited(input, ctm->matrix); } else { for (i = 0; i < ARRAY_SIZE(input); i++) @@ -201,7 +202,8 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state) * into consideration. */ for (i = 0; i < 3; i++) { - if (intel_crtc_state->limited_color_range) + if (intel_crtc_state->limited_color_range && + intel_crtc_state->compress_range) coeffs[i * 3 + i] = I9XX_CSC_COEFF_LIMITED_RANGE; else @@ -225,7 +227,8 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state) if (INTEL_GEN(dev_priv) > 6) { uint16_t postoff = 0; - if (intel_crtc_state->limited_color_range) + if (intel_crtc_state->limited_color_range && + intel_crtc_state->compress_range) postoff = (16 * (1 << 12) / 255) & 0x1fff; I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff); @@ -236,7 +239,8 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state) } else { uint32_t mode = CSC_MODE_YUV_TO_RGB; - if (intel_crtc_state->limited_color_range) + if (intel_crtc_state->limited_color_range && + intel_crtc_state->compress_range) mode |= CSC_BLACK_SCREEN_OFFSET; I915_WRITE(PIPE_CSC_MODE(pipe), mode); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8599e425abb1..4de43eb12f0e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7247,7 +7247,8 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc) pipeconf |= PIPECONF_PROGRESSIVE; if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) && - intel_crtc->config->limited_color_range) + intel_crtc->config->limited_color_range && + intel_crtc->config->compress_range) pipeconf |= PIPECONF_COLOR_RANGE_SELECT; I915_WRITE(PIPECONF(intel_crtc->pipe), pipeconf); @@ -8177,7 +8178,8 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc) else val |= PIPECONF_PROGRESSIVE; - if (intel_crtc->config->limited_color_range) + if (intel_crtc->config->limited_color_range && + intel_crtc->config->compress_range) val |= PIPECONF_COLOR_RANGE_SELECT; I915_WRITE(PIPECONF(pipe), val); diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 887953c0f495..437dd0465be3 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1926,7 +1926,8 @@ static void intel_dp_prepare(struct intel_encoder *encoder, trans_dp &= ~TRANS_DP_ENH_FRAMING; I915_WRITE(TRANS_DP_CTL(crtc->pipe), trans_dp); } else { - if (IS_G4X(dev_priv) && pipe_config->limited_color_range) + if (IS_G4X(dev_priv) && pipe_config->limited_color_range && + pipe_config->compress_range) intel_dp->DP |= DP_COLOR_RANGE_16_235; if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 307807672896..35602a1ed471 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -649,11 +649,18 @@ struct intel_crtc_state { enum transcoder cpu_transcoder; /* - * Use reduced/limited/broadcast rbg range, compressing from the full - * range fed into the crtcs. + * Use reduced/limited/broadcast rbg range. */ bool limited_color_range; + /* + * + * Compress the input range when doing limited/broadcast rgb range + * output. This is the default for Limited 16:235 Output mode. + * + */ + bool compress_range; + /* Bitmask of encoder types (enum intel_output_type) * driven by the pipe. */ diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index e6f8f30ce7bd..4963903f177e 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -880,7 +880,8 @@ static void intel_hdmi_prepare(struct intel_encoder *encoder, intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); hdmi_val = SDVO_ENCODING_HDMI; - if (!HAS_PCH_SPLIT(dev_priv) && crtc_state->limited_color_range) + if (!HAS_PCH_SPLIT(dev_priv) && crtc_state->limited_color_range + && crtc_state->compress_range) hdmi_val |= HDMI_COLOR_RANGE_16_235; if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC) hdmi_val |= SDVO_VSYNC_ACTIVE_HIGH; @@ -1419,9 +1420,18 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, pipe_config->has_hdmi_sink && drm_default_rgb_quant_range(adjusted_mode) == HDMI_QUANTIZATION_RANGE_LIMITED; + pipe_config->compress_range = pipe_config->limited_color_range; + } else if (intel_conn_state->broadcast_rgb + == INTEL_BROADCAST_RGB_LIMITED) { + pipe_config->limited_color_range = true; + pipe_config->compress_range = true; + } else if (intel_conn_state->broadcast_rgb + == INTEL_BROADCAST_RGB_FULL) { + pipe_config->limited_color_range = false; + pipe_config->compress_range = false; } else { - pipe_config->limited_color_range = - intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED; + pipe_config->limited_color_range = true; + pipe_config->compress_range = false; } if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) { diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c index 951e834dd274..d817558410eb 100644 --- a/drivers/gpu/drm/i915/intel_modes.c +++ b/drivers/gpu/drm/i915/intel_modes.c @@ -102,6 +102,7 @@ static const struct drm_prop_enum_list broadcast_rgb_names[] = { { INTEL_BROADCAST_RGB_AUTO, "Automatic" }, { INTEL_BROADCAST_RGB_FULL, "Full" }, { INTEL_BROADCAST_RGB_LIMITED, "Limited 16:235" }, + { INTEL_BROADCAST_RGB_VIDEO, "Video 16:235 pass-through" }, }; void -- 2.11.0
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
