* [PATCH] drm/i915: Fix RGB color range property for PCH platforms
@ 2013-01-09 18:36 ville.syrjala
2013-01-09 18:49 ` Daniel Vetter
0 siblings, 1 reply; 3+ messages in thread
From: ville.syrjala @ 2013-01-09 18:36 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@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@linux.intel.com>
---
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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Fix RGB color range property for PCH platforms
2013-01-09 18:36 [PATCH] drm/i915: Fix RGB color range property for PCH platforms ville.syrjala
@ 2013-01-09 18:49 ` Daniel Vetter
2013-01-10 11:10 ` Ville Syrjälä
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Vetter @ 2013-01-09 18:49 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Wed, Jan 9, 2013 at 7:36 PM, <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@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@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@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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Fix RGB color range property for PCH platforms
2013-01-09 18:49 ` Daniel Vetter
@ 2013-01-10 11:10 ` Ville Syrjälä
0 siblings, 0 replies; 3+ messages in thread
From: Ville Syrjälä @ 2013-01-10 11:10 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Wed, Jan 09, 2013 at 07:49:07PM +0100, Daniel Vetter wrote:
> On Wed, Jan 9, 2013 at 7:36 PM, <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@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@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.
Right. private_flags seems like a decent way to handle this.
v2 coming up soon.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-01-10 11:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-09 18:36 [PATCH] drm/i915: Fix RGB color range property for PCH platforms ville.syrjala
2013-01-09 18:49 ` Daniel Vetter
2013-01-10 11:10 ` Ville Syrjälä
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.