| Message ID | 1436184606-18729-2-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. otherwise changes are fine Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> On Monday 06 July 2015 05:40 PM, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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@linux.intel.com> > --- > 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;
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@intel.com> > > On Monday 06 July 2015 05:40 PM, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > 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@linux.intel.com> > > --- > > 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
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;