drm/i915: Never set limited_color_range=true for YCbCr output

crtc_state->limited_color_range only applies to RGB output but
we're currently setting it even for YCbCr output. That will
lead to conflicting MSA and PIPECONF settings which can mess
up the image. Let's make sure limited_color_range stays unset
with YCbCr output.

Also WARN if we end up with such a bogus combination when
programming the MSA MISC bits as it's impossible to even
indicate quantization rangle for YCbCr via MSA MISC. YCbCr
output is simply assumed to be limited range always. Note
that VSC SDP does provide a mechanism for full range YCbCr,
so in the future we may want to rethink how we compute/store
this state.

And for good measure we add the same WARN to the HDMI path.

v2: s/==/!=/ in the HDMI WARN

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190718164523.11738-1-ville.syrjala@linux.intel.com
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index fb6cb6aca188e768cdd7a295b23e0d258199dec4..0c0da9f6c2e8872aaf5a0a6946a3ec109fe0f581 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1706,9 +1706,6 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
 
        temp = TRANS_MSA_SYNC_CLK;
 
-       if (crtc_state->limited_color_range)
-               temp |= TRANS_MSA_CEA_RANGE;
-
        switch (crtc_state->pipe_bpp) {
        case 18:
                temp |= TRANS_MSA_6_BPC;
@@ -1727,6 +1724,13 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
                break;
        }
 
+       /* nonsense combination */
+       WARN_ON(crtc_state->limited_color_range &&
+               crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB);
+
+       if (crtc_state->limited_color_range)
+               temp |= TRANS_MSA_CEA_RANGE;
+
        /*
         * As per DP 1.2 spec section 2.3.4.3 while sending
         * YCBCR 444 signals we should program MSA MISC1/0 fields with

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 0c7755e915d919cc90aa2d0fe7755735c2fb2cab..5cce9459e93c00539f0ba9662fd84b2b397b77c3 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2150,6 +2150,16 @@ bool intel_dp_limited_color_range(const struct intel_crtc_state *crtc_state,
        const struct drm_display_mode *adjusted_mode =
                &crtc_state->base.adjusted_mode;
 
+       /*
+        * Our YCbCr output is always limited range.
+        * crtc_state->limited_color_range only applies to RGB,
+        * and it must never be set for YCbCr or we risk setting
+        * some conflicting bits in PIPECONF which will mess up
+        * the colors on the monitor.
+        */
+       if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB)
+               return false;
+
        if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) {
                /*
                 * See:

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b781e0d1962ef7f16a5b6c939401f530fef1113a..5ab73331d35df75b3fbedeef9560f86a2ce5f7c8 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -724,6 +724,10 @@ intel_hdmi_compute_avi_infoframe(struct intel_encoder *encoder,
 
        drm_hdmi_avi_infoframe_colorspace(frame, conn_state);
 
+       /* nonsense combination */
+       WARN_ON(crtc_state->limited_color_range &&
+               crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB);
+
        if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_RGB) {
                drm_hdmi_avi_infoframe_quant_range(frame, connector,
                                                   adjusted_mode,
@@ -2373,6 +2377,16 @@ static bool intel_hdmi_limited_color_range(const struct intel_crtc_state *crtc_s
        const struct drm_display_mode *adjusted_mode =
                &crtc_state->base.adjusted_mode;
 
+       /*
+        * Our YCbCr output is always limited range.
+        * crtc_state->limited_color_range only applies to RGB,
+        * and it must never be set for YCbCr or we risk setting
+        * some conflicting bits in PIPECONF which will mess up
+        * the colors on the monitor.
+        */
+       if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB)
+               return false;
+
        if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) {
                /* See CEA-861-E - 5.1 Default Encoding Parameters */
                return crtc_state->has_hdmi_sink &&
@@ -2406,9 +2420,6 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
        if (pipe_config->has_hdmi_sink)
                pipe_config->has_infoframe = true;
 
-       pipe_config->limited_color_range =
-               intel_hdmi_limited_color_range(pipe_config, conn_state);
-
        if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
                pipe_config->pixel_multiplier = 2;
 
@@ -2419,6 +2430,9 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
                }
        }
 
+       pipe_config->limited_color_range =
+               intel_hdmi_limited_color_range(pipe_config, conn_state);
+
        if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
                pipe_config->has_pch_encoder = true;