[PATCH] drm/i915: Fix RGB color range property for PCH platforms

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux