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ä <
[email protected]> 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 <[email protected]>
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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to