| Message ID | 1358356365-23191-5-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi 2013/1/16 <ville.syrjala@linux.intel.com>: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > The AVI infoframe is able to inform the display whether the source is > sending full or limited range RGB data. > > As per CEA-861 we must first check whether the display reports the > quantization range as selectable, and if so we can set the approriate > bits in the AVI inforframe. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_drv.h | 3 +++ > drivers/gpu/drm/i915/intel_hdmi.c | 11 +++++++++++ > drivers/gpu/drm/i915/intel_sdvo.c | 16 ++++++++++++++-- > 3 files changed, 28 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 1a698c6..c5251d9 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -289,6 +289,8 @@ struct cxsr_latency { > #define DIP_LEN_AVI 13 > #define DIP_AVI_PR_1 0 > #define DIP_AVI_PR_2 1 > +#define DIP_AVI_Q0 0x4 > +#define DIP_AVI_Q1 0x8 <bikeshedding optional="true"> I'd define more descriptive names here... How about the following? #define DIP_AVI_QUANTIZATION_DEFAULT (0 << 2) #define DIP_AVI_QUANTIZATION_LIMITED (1 << 2) #define DIP_AVI_QUANTIZATION_FULL (2 << 2) <bikeshedding optional="true"> Everything else looks fine. With or without that: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Also, these things kinda conflict with Thierry's patches, but I know you're aware because you've reviewed some of them :) > > #define DIP_TYPE_SPD 0x83 > #define DIP_VERSION_SPD 0x1 > @@ -347,6 +349,7 @@ struct intel_hdmi { > bool has_hdmi_sink; > bool has_audio; > enum hdmi_force_audio force_audio; > + bool rgb_quant_range_selectable; > void (*write_infoframe)(struct drm_encoder *encoder, > struct dip_infoframe *frame); > void (*set_infoframes)(struct drm_encoder *encoder, > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index 58b072e..270d7ee 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -331,6 +331,7 @@ static void intel_set_infoframe(struct drm_encoder *encoder, > static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder, > struct drm_display_mode *adjusted_mode) > { > + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); > struct dip_infoframe avi_if = { > .type = DIP_TYPE_AVI, > .ver = DIP_VERSION_AVI, > @@ -340,6 +341,13 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder, > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) > avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2; > > + if (intel_hdmi->rgb_quant_range_selectable) { > + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) > + avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q0; > + else > + avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q1; > + } > + > avi_if.body.avi.VIC = drm_mode_cea_vic(adjusted_mode); > > intel_set_infoframe(encoder, &avi_if); > @@ -825,6 +833,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) > > intel_hdmi->has_hdmi_sink = false; > intel_hdmi->has_audio = false; > + intel_hdmi->rgb_quant_range_selectable = false; > edid = drm_get_edid(connector, > intel_gmbus_get_adapter(dev_priv, > intel_hdmi->ddc_bus)); > @@ -836,6 +845,8 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) > intel_hdmi->has_hdmi_sink = > drm_detect_hdmi_monitor(edid); > intel_hdmi->has_audio = drm_detect_monitor_audio(edid); > + intel_hdmi->rgb_quant_range_selectable = > + drm_rgb_quant_range_selectable(edid); > } > kfree(edid); > } > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c > index b422109..af93999 100644 > --- a/drivers/gpu/drm/i915/intel_sdvo.c > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > @@ -126,6 +126,7 @@ struct intel_sdvo { > bool is_hdmi; > bool has_hdmi_monitor; > bool has_hdmi_audio; > + bool rgb_quant_range_selectable; > > /** > * This is set if we detect output of sdvo device as LVDS and > @@ -947,7 +948,8 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo, > &tx_rate, 1); > } > > -static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo) > +static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo, > + const struct drm_display_mode *adjusted_mode) > { > struct dip_infoframe avi_if = { > .type = DIP_TYPE_AVI, > @@ -956,6 +958,13 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo) > }; > uint8_t sdvo_data[4 + sizeof(avi_if.body.avi)]; > > + if (intel_sdvo->rgb_quant_range_selectable) { > + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) > + avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q0; > + else > + avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q1; > + } > + > intel_dip_infoframe_csum(&avi_if); > > /* sdvo spec says that the ecc is handled by the hw, and it looks like > @@ -1134,7 +1143,7 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder, > intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_HDMI); > intel_sdvo_set_colorimetry(intel_sdvo, > SDVO_COLORIMETRY_RGB256); > - intel_sdvo_set_avi_infoframe(intel_sdvo); > + intel_sdvo_set_avi_infoframe(intel_sdvo, adjusted_mode); > } else > intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_DVI); > > @@ -1526,6 +1535,8 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector) > if (intel_sdvo->is_hdmi) { > intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid); > intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid); > + intel_sdvo->rgb_quant_range_selectable = > + drm_rgb_quant_range_selectable(edid); > } > } else > status = connector_status_disconnected; > @@ -1577,6 +1588,7 @@ intel_sdvo_detect(struct drm_connector *connector, bool force) > > intel_sdvo->has_hdmi_monitor = false; > intel_sdvo->has_hdmi_audio = false; > + intel_sdvo->rgb_quant_range_selectable = false; > > if ((intel_sdvo_connector->output_flag & response) == 0) > ret = connector_status_disconnected; > -- > 1.7.8.6 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jan 17, 2013 at 10:16:46AM -0200, Paulo Zanoni wrote: > Hi > > 2013/1/16 <ville.syrjala@linux.intel.com>: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > The AVI infoframe is able to inform the display whether the source is > > sending full or limited range RGB data. > > > > As per CEA-861 we must first check whether the display reports the > > quantization range as selectable, and if so we can set the approriate > > bits in the AVI inforframe. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_drv.h | 3 +++ > > drivers/gpu/drm/i915/intel_hdmi.c | 11 +++++++++++ > > drivers/gpu/drm/i915/intel_sdvo.c | 16 ++++++++++++++-- > > 3 files changed, 28 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 1a698c6..c5251d9 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -289,6 +289,8 @@ struct cxsr_latency { > > #define DIP_LEN_AVI 13 > > #define DIP_AVI_PR_1 0 > > #define DIP_AVI_PR_2 1 > > +#define DIP_AVI_Q0 0x4 > > +#define DIP_AVI_Q1 0x8 > > <bikeshedding optional="true"> > > I'd define more descriptive names here... How about the following? > #define DIP_AVI_QUANTIZATION_DEFAULT (0 << 2) > #define DIP_AVI_QUANTIZATION_LIMITED (1 << 2) > #define DIP_AVI_QUANTIZATION_FULL (2 << 2) Sure, that seems like sensible idea. I think I'll want the fact that these only refer to RGB to be included in the name though. So something like this probably: DIP_AVI_RGB_QUANT_RANGE_DEFAULT DIP_AVI_RGB_QUANT_RANGE_LIMITED DIP_AVI_RGB_QUANT_RANGE_FULL > <bikeshedding optional="true"> > > Everything else looks fine. > > With or without that: > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Also, these things kinda conflict with Thierry's patches, but I know > you're aware because you've reviewed some of them :) Yeah. BTW are we (as in i915 folks) mostly OK with those? > > #define DIP_TYPE_SPD 0x83 > > #define DIP_VERSION_SPD 0x1 > > @@ -347,6 +349,7 @@ struct intel_hdmi { > > bool has_hdmi_sink; > > bool has_audio; > > enum hdmi_force_audio force_audio; > > + bool rgb_quant_range_selectable; > > void (*write_infoframe)(struct drm_encoder *encoder, > > struct dip_infoframe *frame); > > void (*set_infoframes)(struct drm_encoder *encoder, > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > > index 58b072e..270d7ee 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -331,6 +331,7 @@ static void intel_set_infoframe(struct drm_encoder *encoder, > > static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder, > > struct drm_display_mode *adjusted_mode) > > { > > + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); > > struct dip_infoframe avi_if = { > > .type = DIP_TYPE_AVI, > > .ver = DIP_VERSION_AVI, > > @@ -340,6 +341,13 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder, > > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) > > avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2; > > > > + if (intel_hdmi->rgb_quant_range_selectable) { > > + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) > > + avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q0; > > + else > > + avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q1; > > + } > > + > > avi_if.body.avi.VIC = drm_mode_cea_vic(adjusted_mode); > > > > intel_set_infoframe(encoder, &avi_if); > > @@ -825,6 +833,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) > > > > intel_hdmi->has_hdmi_sink = false; > > intel_hdmi->has_audio = false; > > + intel_hdmi->rgb_quant_range_selectable = false; > > edid = drm_get_edid(connector, > > intel_gmbus_get_adapter(dev_priv, > > intel_hdmi->ddc_bus)); > > @@ -836,6 +845,8 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) > > intel_hdmi->has_hdmi_sink = > > drm_detect_hdmi_monitor(edid); > > intel_hdmi->has_audio = drm_detect_monitor_audio(edid); > > + intel_hdmi->rgb_quant_range_selectable = > > + drm_rgb_quant_range_selectable(edid); > > } > > kfree(edid); > > } > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c > > index b422109..af93999 100644 > > --- a/drivers/gpu/drm/i915/intel_sdvo.c > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > > @@ -126,6 +126,7 @@ struct intel_sdvo { > > bool is_hdmi; > > bool has_hdmi_monitor; > > bool has_hdmi_audio; > > + bool rgb_quant_range_selectable; > > > > /** > > * This is set if we detect output of sdvo device as LVDS and > > @@ -947,7 +948,8 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo, > > &tx_rate, 1); > > } > > > > -static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo) > > +static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo, > > + const struct drm_display_mode *adjusted_mode) > > { > > struct dip_infoframe avi_if = { > > .type = DIP_TYPE_AVI, > > @@ -956,6 +958,13 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo) > > }; > > uint8_t sdvo_data[4 + sizeof(avi_if.body.avi)]; > > > > + if (intel_sdvo->rgb_quant_range_selectable) { > > + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) > > + avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q0; > > + else > > + avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q1; > > + } > > + > > intel_dip_infoframe_csum(&avi_if); > > > > /* sdvo spec says that the ecc is handled by the hw, and it looks like > > @@ -1134,7 +1143,7 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder, > > intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_HDMI); > > intel_sdvo_set_colorimetry(intel_sdvo, > > SDVO_COLORIMETRY_RGB256); > > - intel_sdvo_set_avi_infoframe(intel_sdvo); > > + intel_sdvo_set_avi_infoframe(intel_sdvo, adjusted_mode); > > } else > > intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_DVI); > > > > @@ -1526,6 +1535,8 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector) > > if (intel_sdvo->is_hdmi) { > > intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid); > > intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid); > > + intel_sdvo->rgb_quant_range_selectable = > > + drm_rgb_quant_range_selectable(edid); > > } > > } else > > status = connector_status_disconnected; > > @@ -1577,6 +1588,7 @@ intel_sdvo_detect(struct drm_connector *connector, bool force) > > > > intel_sdvo->has_hdmi_monitor = false; > > intel_sdvo->has_hdmi_audio = false; > > + intel_sdvo->rgb_quant_range_selectable = false; > > > > if ((intel_sdvo_connector->output_flag & response) == 0) > > ret = connector_status_disconnected; > > -- > > 1.7.8.6 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > Paulo Zanoni
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1a698c6..c5251d9 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -289,6 +289,8 @@ struct cxsr_latency { #define DIP_LEN_AVI 13 #define DIP_AVI_PR_1 0 #define DIP_AVI_PR_2 1 +#define DIP_AVI_Q0 0x4 +#define DIP_AVI_Q1 0x8 #define DIP_TYPE_SPD 0x83 #define DIP_VERSION_SPD 0x1 @@ -347,6 +349,7 @@ struct intel_hdmi { bool has_hdmi_sink; bool has_audio; enum hdmi_force_audio force_audio; + bool rgb_quant_range_selectable; void (*write_infoframe)(struct drm_encoder *encoder, struct dip_infoframe *frame); void (*set_infoframes)(struct drm_encoder *encoder, diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 58b072e..270d7ee 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -331,6 +331,7 @@ static void intel_set_infoframe(struct drm_encoder *encoder, static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder, struct drm_display_mode *adjusted_mode) { + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); struct dip_infoframe avi_if = { .type = DIP_TYPE_AVI, .ver = DIP_VERSION_AVI, @@ -340,6 +341,13 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder, if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2; + if (intel_hdmi->rgb_quant_range_selectable) { + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) + avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q0; + else + avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q1; + } + avi_if.body.avi.VIC = drm_mode_cea_vic(adjusted_mode); intel_set_infoframe(encoder, &avi_if); @@ -825,6 +833,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) intel_hdmi->has_hdmi_sink = false; intel_hdmi->has_audio = false; + intel_hdmi->rgb_quant_range_selectable = false; edid = drm_get_edid(connector, intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus)); @@ -836,6 +845,8 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid); intel_hdmi->has_audio = drm_detect_monitor_audio(edid); + intel_hdmi->rgb_quant_range_selectable = + drm_rgb_quant_range_selectable(edid); } kfree(edid); } diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index b422109..af93999 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -126,6 +126,7 @@ struct intel_sdvo { bool is_hdmi; bool has_hdmi_monitor; bool has_hdmi_audio; + bool rgb_quant_range_selectable; /** * This is set if we detect output of sdvo device as LVDS and @@ -947,7 +948,8 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo, &tx_rate, 1); } -static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo) +static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo, + const struct drm_display_mode *adjusted_mode) { struct dip_infoframe avi_if = { .type = DIP_TYPE_AVI, @@ -956,6 +958,13 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo) }; uint8_t sdvo_data[4 + sizeof(avi_if.body.avi)]; + if (intel_sdvo->rgb_quant_range_selectable) { + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) + avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q0; + else + avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q1; + } + intel_dip_infoframe_csum(&avi_if); /* sdvo spec says that the ecc is handled by the hw, and it looks like @@ -1134,7 +1143,7 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder, intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_HDMI); intel_sdvo_set_colorimetry(intel_sdvo, SDVO_COLORIMETRY_RGB256); - intel_sdvo_set_avi_infoframe(intel_sdvo); + intel_sdvo_set_avi_infoframe(intel_sdvo, adjusted_mode); } else intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_DVI); @@ -1526,6 +1535,8 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector) if (intel_sdvo->is_hdmi) { intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid); intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid); + intel_sdvo->rgb_quant_range_selectable = + drm_rgb_quant_range_selectable(edid); } } else status = connector_status_disconnected; @@ -1577,6 +1588,7 @@ intel_sdvo_detect(struct drm_connector *connector, bool force) intel_sdvo->has_hdmi_monitor = false; intel_sdvo->has_hdmi_audio = false; + intel_sdvo->rgb_quant_range_selectable = false; if ((intel_sdvo_connector->output_flag & response) == 0) ret = connector_status_disconnected;