[Intel-gfx] [PATCH] drm/i915: Fix RGB color range property for PCH platforms
Daniel Vetter
daniel at ffwll.ch
Wed Jan 9 19:49:07 CET 2013
On Wed, Jan 9, 2013 at 7:36 PM, <ville.syrjala at linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> The RGB color range select bit on the DP/SDVO/HDMI registers
> disappeared when PCH was introduced, and instead a new PIPECONF bit
> was added that performs the same function.
>
> Add a new intel_encoder function pointer to query the encoder whether
> limited or full range should be selected, and set the PIPECONF bit 13
> accordingly.
>
> Experimentation showed that simply toggling the bit while the pipe is
> active doesn't work. We need to restart the pipe, which luckily already
> happens.
>
> The DP/SDVO/HDMI bit 8 is marked MBZ in the docs, so avoid setting it,
> although it doesn't seem to do any harm in practice.
>
> TODO:
> - the PIPECONF bit too seems to have disappeared from HSW. Need a
> volunteer to test if it's just a documentation issue or if it's really
> gone. If the bit is gone and no easy replacement is found, then I suppose
> we may need to use the pipe CSC unit to perform the range compression.
> - move color_range to intel_encoder to avoid the silly func pointer?
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46800
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
Can't we just add another flag to mod->private_flags like we do for
6bpc dithering? I know that that's ugly, and we need to extend this
into a more generic pipe configuration struct, but we have way to many
bits&pieces where encoders want to control/influence pipe state like
that, so adding new virtual functions like this wont scale.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
> drivers/gpu/drm/i915/intel_dp.c | 11 ++++++++++-
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_hdmi.c | 8 ++++++++
> drivers/gpu/drm/i915/intel_sdvo.c | 10 +++++++++-
> 6 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3b039f4..a653b64 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2650,6 +2650,7 @@
> #define PIPECONF_INTERLACED_DBL_ILK (4 << 21) /* ilk/snb only */
> #define PIPECONF_PFIT_PF_INTERLACED_DBL_ILK (5 << 21) /* ilk/snb only */
> #define PIPECONF_CXSR_DOWNCLOCK (1<<16)
> +#define PIPECONF_COLOR_RANGE_SELECT (1 << 13)
> #define PIPECONF_BPC_MASK (0x7 << 5)
> #define PIPECONF_8BPC (0<<5)
> #define PIPECONF_10BPC (1<<5)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cf0c713..4023383 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5053,6 +5053,20 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
> return 120000;
> }
>
> +static bool intel_limited_color_range(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct intel_encoder *intel_encoder;
> + bool limited_color_range = false;
> +
> + for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
> + if (intel_encoder->limited_color_range)
> + limited_color_range |= intel_encoder->limited_color_range(intel_encoder);
> + }
> +
> + return limited_color_range;
> +}
> +
> static void ironlake_set_pipeconf(struct drm_crtc *crtc,
> struct drm_display_mode *adjusted_mode,
> bool dither)
> @@ -5093,6 +5107,11 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
> else
> val |= PIPECONF_PROGRESSIVE;
>
> + if (intel_limited_color_range(crtc))
> + val |= PIPECONF_COLOR_RANGE_SELECT;
> + else
> + val &= ~PIPECONF_COLOR_RANGE_SELECT;
> +
> I915_WRITE(PIPECONF(pipe), val);
> POSTING_READ(PIPECONF(pipe));
> }
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5f12eb2..14cb3d2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -967,7 +967,8 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
> else
> intel_dp->DP |= DP_PLL_FREQ_270MHZ;
> } else if (!HAS_PCH_CPT(dev) || is_cpu_edp(intel_dp)) {
> - intel_dp->DP |= intel_dp->color_range;
> + if (!HAS_PCH_SPLIT(dev))
> + intel_dp->DP |= intel_dp->color_range;
>
> if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
> intel_dp->DP |= DP_SYNC_HS_HIGH;
> @@ -1392,6 +1393,13 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static bool intel_dp_limited_color_range(struct intel_encoder *encoder)
> +{
> + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +
> + return intel_dp->color_range;
> +}
> +
> static void intel_disable_dp(struct intel_encoder *encoder)
> {
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> @@ -2913,6 +2921,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> intel_encoder->disable = intel_disable_dp;
> intel_encoder->post_disable = intel_post_disable_dp;
> intel_encoder->get_hw_state = intel_dp_get_hw_state;
> + intel_encoder->limited_color_range = intel_dp_limited_color_range;
>
> intel_dig_port->port = port;
> intel_dig_port->dp.output_reg = output_reg;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 54a034c..1f9de7c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -162,6 +162,7 @@ struct intel_encoder {
> * the encoder is active. If the encoder is enabled it also set the pipe
> * it is connected to in the pipe parameter. */
> bool (*get_hw_state)(struct intel_encoder *, enum pipe *pipe);
> + bool (*limited_color_range)(struct intel_encoder *);
> int crtc_mask;
> };
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 6387f9b..7ef617c 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -646,6 +646,13 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static bool intel_hdmi_limited_color_range(struct intel_encoder *encoder)
> +{
> + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> +
> + return intel_hdmi->color_range;
> +}
> +
> static void intel_enable_hdmi(struct intel_encoder *encoder)
> {
> struct drm_device *dev = encoder->base.dev;
> @@ -1062,6 +1069,7 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port port)
> intel_encoder->enable = intel_enable_hdmi;
> intel_encoder->disable = intel_disable_hdmi;
> intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
> + intel_encoder->limited_color_range = intel_hdmi_limited_color_range;
>
> intel_encoder->type = INTEL_OUTPUT_HDMI;
> intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 153377b..bfe855b 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1153,7 +1153,7 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
> /* The real mode polarity is set by the SDVO commands, using
> * struct intel_sdvo_dtd. */
> sdvox = SDVO_VSYNC_ACTIVE_HIGH | SDVO_HSYNC_ACTIVE_HIGH;
> - if (intel_sdvo->is_hdmi)
> + if (!HAS_PCH_SPLIT(dev) && intel_sdvo->is_hdmi)
> sdvox |= intel_sdvo->color_range;
> if (INTEL_INFO(dev)->gen < 5)
> sdvox |= SDVO_BORDER_ENABLE;
> @@ -1228,6 +1228,13 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static bool intel_sdvo_limited_color_range(struct intel_encoder *encoder)
> +{
> + struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base);
> +
> + return intel_sdvo->color_range;
> +}
> +
> static void intel_disable_sdvo(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -2746,6 +2753,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
> intel_encoder->disable = intel_disable_sdvo;
> intel_encoder->enable = intel_enable_sdvo;
> intel_encoder->get_hw_state = intel_sdvo_get_hw_state;
> + intel_encoder->limited_color_range = intel_sdvo_limited_color_range;
>
> /* In default case sdvo lvds is false */
> if (!intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps))
> --
> 1.7.8.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list