Re: [Intel-gfx] [PATCH 1/4] drm/i915: Fix RGB color range property for PCH platforms

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

 



Hi

2013/1/16  <ville.syrjala@xxxxxxxxxxxxxxx>:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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_MODE_LIMITED_COLOR_RANGE private mode flag, and set
> it in the encoder mode_fixup if limited color range is requested.
> Set the the PIPECONF bit 13 based on the flag.
>
> 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.
>
> v2: Use mode private_flags instead of intel_encoder virtual functions
> v3: Moved the intel_dp color_range handling after bpc check to help
>     later patches

Things look correct. As you mention, the only problem seems to be the
Haswell. I could not find anything on the specs, so I think we should
send some emails and maybe consider removing the property on Haswell?

If-you-promise-to-find-a-solution-for-the-Haswell-case:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>

>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46800
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |    5 +++++
>  drivers/gpu/drm/i915/intel_dp.c      |    7 ++++++-
>  drivers/gpu/drm/i915/intel_drv.h     |    5 +++++
>  drivers/gpu/drm/i915/intel_hdmi.c    |    5 +++++
>  drivers/gpu/drm/i915/intel_sdvo.c    |    5 ++++-
>  6 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 36789bf..a2550c5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2652,6 +2652,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 c7313f8..f48f698 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5097,6 +5097,11 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
>         else
>                 val |= PIPECONF_PROGRESSIVE;
>
> +       if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE)
> +               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 cf25481..64824d0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -763,6 +763,10 @@ intel_dp_mode_fixup(struct drm_encoder *encoder,
>                 return false;
>
>         bpp = adjusted_mode->private_flags & INTEL_MODE_DP_FORCE_6BPC ? 18 : 24;
> +
> +       if (intel_dp->color_range)
> +               adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
> +
>         mode_rate = intel_dp_link_required(adjusted_mode->clock, bpp);
>
>         for (clock = 0; clock <= max_clock; clock++) {
> @@ -967,7 +971,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;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 54a034c..4df47be 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -109,6 +109,11 @@
>   * timings in the mode to prevent the crtc fixup from overwriting them.
>   * Currently only lvds needs that. */
>  #define INTEL_MODE_CRTC_TIMINGS_SET (0x20)
> +/*
> + * Set when limited 16-235 (as opposed to full 0-255) RGB color range is
> + * to be used.
> + */
> +#define INTEL_MODE_LIMITED_COLOR_RANGE (0x40)
>
>  static inline void
>  intel_mode_set_pixel_multiplier(struct drm_display_mode *mode,
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 6387f9b..f194d75 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -766,6 +766,11 @@ bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
>                            const struct drm_display_mode *mode,
>                            struct drm_display_mode *adjusted_mode)
>  {
> +       struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> +
> +       if (intel_hdmi->color_range)
> +               adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
> +
>         return true;
>  }
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 153377b..3b8491a 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1064,6 +1064,9 @@ static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
>         multiplier = intel_sdvo_get_pixel_multiplier(adjusted_mode);
>         intel_mode_set_pixel_multiplier(adjusted_mode, multiplier);
>
> +       if (intel_sdvo->color_range)
> +               adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
> +
>         return true;
>  }
>
> @@ -1153,7 +1156,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;
> --
> 1.7.8.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux