<div dir="ltr">Thanks for your comments, I tried to implement a "flag driven version" below.<div class="gmail_extra"><br><div class="gmail_quote">On Fri, Nov 11, 2016 at 2:57 PM, Ville Syrjälä <span dir="ltr"><<a href="mailto:ville.syrjala@linux.intel.com" target="_blank">ville.syrjala@linux.intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">On Fri, Nov 11, 2016 at 09:53:35AM +0100, Peter Frühberger wrote:<br>
> Hi,<br>
><br>
> I was implementing this suggestion today and I think that will confuse<br>
> users and also devs maintaining that code. Out of the following reason:<br>
> compress_color_range can be true or false, it does not reference a mode,<br>
> but an additional setting that only influences color clamping / scaling.<br>
><br>
> What do we expect if someone runs in Full Range and has<br>
> compress_color_range set to true? Also what do we expect if someone runs in<br>
> Limited Range and additionally set this flag to true? In some cases it<br>
> would do nothing and was not transparent.<br>
<br>
</span>I didn't mean you should add a new property for this. Just an internal<br>
flag.<br>
<div class="gmail-HOEnZb"><div class="gmail-h5">[...]</div></div></blockquote><div><br></div><div>In my opinion that makes maintainability much harder. Looking forward to your comments.</div><div> </div><pre style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap">From f3bccf1611108247add0d85e8faed5430971cd71 Mon Sep 17 00:00:00 2001
From: fritsch <<a href="mailto:peter.fruehberger@gmail.com">peter.fruehberger@gmail.com</a>>
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
</pre><div> <br></div></div></div></div>