On Thu, Aug 13, 2015 at 11:46:56AM +0530, Sivakumar Thulasimani wrote: > sdvo is still using color_range name in it's functions. would be good to > rename that as well along with dp & hdmi done here. Doh. I forgot about sdvo completely. I'll take a look to make sure it conforms to the same style. Thanks for spotting it. > > otherwise changes are fine > Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@xxxxxxxxx> > > On Monday 06 July 2015 05:40 PM, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > Currently we treat intel_{dp,hdmi}->color_range as partly user > > controller value (via the property) but we also change it during > > .compute_config() when using the "Automatic" mode. That is a bit > > confusing, so let's just change things so that we store the user > > property values in intel_dp, and only change what's stored in > > pipe_config during .compute_config(). > > > > There should be no functional change. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 25 ++++++++++++------------- > > drivers/gpu/drm/i915/intel_drv.h | 4 ++-- > > drivers/gpu/drm/i915/intel_hdmi.c | 26 ++++++++++++-------------- > > 3 files changed, 26 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index fcc64e5..decefa1 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -1455,15 +1455,13 @@ found: > > * CEA-861-E - 5.1 Default Encoding Parameters > > * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry > > */ > > - if (bpp != 18 && drm_match_cea_mode(adjusted_mode) > 1) > > - intel_dp->color_range = DP_COLOR_RANGE_16_235; > > - else > > - intel_dp->color_range = 0; > > + pipe_config->limited_color_range = > > + bpp != 18 && drm_match_cea_mode(adjusted_mode) > 1; > > + } else { > > + pipe_config->limited_color_range = > > + intel_dp->limited_color_range; > > } > > > > - if (intel_dp->color_range) > > - pipe_config->limited_color_range = true; > > - > > intel_dp->lane_count = lane_count; > > > > if (intel_dp->num_sink_rates) { > > @@ -1605,8 +1603,9 @@ static void intel_dp_prepare(struct intel_encoder *encoder) > > trans_dp &= ~TRANS_DP_ENH_FRAMING; > > I915_WRITE(TRANS_DP_CTL(crtc->pipe), trans_dp); > > } else { > > - if (!HAS_PCH_SPLIT(dev) && !IS_VALLEYVIEW(dev)) > > - intel_dp->DP |= intel_dp->color_range; > > + if (!HAS_PCH_SPLIT(dev) && !IS_VALLEYVIEW(dev) && > > + crtc->config->limited_color_range) > > + intel_dp->DP |= DP_COLOR_RANGE_16_235; > > > > if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC) > > intel_dp->DP |= DP_SYNC_HS_HIGH; > > @@ -4663,7 +4662,7 @@ intel_dp_set_property(struct drm_connector *connector, > > > > if (property == dev_priv->broadcast_rgb_property) { > > bool old_auto = intel_dp->color_range_auto; > > - uint32_t old_range = intel_dp->color_range; > > + bool old_range = intel_dp->limited_color_range; > > > > switch (val) { > > case INTEL_BROADCAST_RGB_AUTO: > > @@ -4671,18 +4670,18 @@ intel_dp_set_property(struct drm_connector *connector, > > break; > > case INTEL_BROADCAST_RGB_FULL: > > intel_dp->color_range_auto = false; > > - intel_dp->color_range = 0; > > + intel_dp->limited_color_range = false; > > break; > > case INTEL_BROADCAST_RGB_LIMITED: > > intel_dp->color_range_auto = false; > > - intel_dp->color_range = DP_COLOR_RANGE_16_235; > > + intel_dp->limited_color_range = true; > > break; > > default: > > return -EINVAL; > > } > > > > if (old_auto == intel_dp->color_range_auto && > > - old_range == intel_dp->color_range) > > + old_range == intel_dp->limited_color_range) > > return 0; > > > > goto done; > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 3f0a890..983a7a7 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -669,7 +669,7 @@ struct cxsr_latency { > > struct intel_hdmi { > > u32 hdmi_reg; > > int ddc_bus; > > - uint32_t color_range; > > + bool limited_color_range; > > bool color_range_auto; > > bool has_hdmi_sink; > > bool has_audio; > > @@ -714,7 +714,7 @@ struct intel_dp { > > uint32_t DP; > > bool has_audio; > > enum hdmi_force_audio force_audio; > > - uint32_t color_range; > > + bool limited_color_range; > > bool color_range_auto; > > uint8_t link_bw; > > uint8_t rate_select; > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > > index c7e912b..ba845f7 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -848,8 +848,8 @@ static void intel_hdmi_prepare(struct intel_encoder *encoder) > > u32 hdmi_val; > > > > hdmi_val = SDVO_ENCODING_HDMI; > > - if (!HAS_PCH_SPLIT(dev)) > > - hdmi_val |= intel_hdmi->color_range; > > + if (!HAS_PCH_SPLIT(dev) && crtc->config->limited_color_range) > > + hdmi_val |= HDMI_COLOR_RANGE_16_235; > > if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC) > > hdmi_val |= SDVO_VSYNC_ACTIVE_HIGH; > > if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC) > > @@ -1257,11 +1257,12 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, > > > > if (intel_hdmi->color_range_auto) { > > /* See CEA-861-E - 5.1 Default Encoding Parameters */ > > - if (pipe_config->has_hdmi_sink && > > - drm_match_cea_mode(adjusted_mode) > 1) > > - intel_hdmi->color_range = HDMI_COLOR_RANGE_16_235; > > - else > > - intel_hdmi->color_range = 0; > > + pipe_config->limited_color_range = > > + pipe_config->has_hdmi_sink && > > + drm_match_cea_mode(adjusted_mode) > 1; > > + } else { > > + pipe_config->limited_color_range = > > + intel_hdmi->limited_color_range; > > } > > > > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) { > > @@ -1270,9 +1271,6 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, > > clock_12bpc *= 2; > > } > > > > - if (intel_hdmi->color_range) > > - pipe_config->limited_color_range = true; > > - > > if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev)) > > pipe_config->has_pch_encoder = true; > > > > @@ -1467,7 +1465,7 @@ intel_hdmi_set_property(struct drm_connector *connector, > > > > if (property == dev_priv->broadcast_rgb_property) { > > bool old_auto = intel_hdmi->color_range_auto; > > - uint32_t old_range = intel_hdmi->color_range; > > + bool old_range = intel_hdmi->limited_color_range; > > > > switch (val) { > > case INTEL_BROADCAST_RGB_AUTO: > > @@ -1475,18 +1473,18 @@ intel_hdmi_set_property(struct drm_connector *connector, > > break; > > case INTEL_BROADCAST_RGB_FULL: > > intel_hdmi->color_range_auto = false; > > - intel_hdmi->color_range = 0; > > + intel_hdmi->limited_color_range = false; > > break; > > case INTEL_BROADCAST_RGB_LIMITED: > > intel_hdmi->color_range_auto = false; > > - intel_hdmi->color_range = HDMI_COLOR_RANGE_16_235; > > + intel_hdmi->limited_color_range = true; > > break; > > default: > > return -EINVAL; > > } > > > > if (old_auto == intel_hdmi->color_range_auto && > > - old_range == intel_hdmi->color_range) > > + old_range == intel_hdmi->limited_color_range) > > return 0; > > > > goto done; > > -- > regards, > Sivakumar Thulasimani -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx