All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/7] drm/i915: Clean up DP/HDMI limited color range handling
Date: Thu, 13 Aug 2015 15:01:02 +0300	[thread overview]
Message-ID: <20150813120102.GP5176@intel.com> (raw)
In-Reply-To: <55CC3658.40308@intel.com>

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2015-08-13 12:01 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-06 12:09 [PATCH 0/7] drm/i915: Move DP link parameters out from intel_dp ville.syrjala
2015-07-06 12:10 ` [PATCH 1/7] drm/i915: Clean up DP/HDMI limited color range handling ville.syrjala
2015-08-13  6:16   ` Sivakumar Thulasimani
2015-08-13 12:01     ` Ville Syrjälä [this message]
2015-07-06 12:10 ` [PATCH 2/7] drm/i915: Don't use link_bw for PLL setup ville.syrjala
2015-07-07  8:05   ` Sivakumar Thulasimani
2015-08-11 17:21   ` [PATCH v2 " ville.syrjala
2015-07-06 12:10 ` [PATCH 3/7] drm/i915: Don't pass clock to DDI PLL select functions ville.syrjala
2015-07-07  8:07   ` Sivakumar Thulasimani
2015-07-06 12:10 ` [PATCH 4/7] drm/i915: Avoid confusion between DP and TRANS_DP_CTL in DP .get_config() ville.syrjala
2015-07-07  8:14   ` Sivakumar Thulasimani
2015-07-06 12:10 ` [PATCH 5/7] drm/i915: Move intel_dp->lane_count into pipe_config ville.syrjala
2015-07-06 13:39   ` [PATCH v2 " ville.syrjala
2015-08-13  7:00     ` Sivakumar Thulasimani
2015-07-06 12:10 ` [PATCH 6/7] drm/i915: Don't use link_bw to select between TP1 and TP3 ville.syrjala
2015-07-07  8:18   ` Sivakumar Thulasimani
2015-07-06 12:10 ` [PATCH 7/7] drm/i915: Kill intel_dp->{link_bw, rate_select} ville.syrjala
2015-07-07  8:46   ` Sivakumar Thulasimani
2015-07-10 23:13   ` shuang.he
2015-08-12 16:04 ` [PATCH 0/7] drm/i915: Move DP link parameters out from intel_dp Ville Syrjälä
2015-08-14  8:23   ` Daniel Vetter
2015-08-17  8:42 ` Maarten Lankhorst
2015-08-17 12:03   ` Ville Syrjälä
2015-08-25 12:57     ` Daniel Vetter
2015-08-25 14:11       ` Ville Syrjälä

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150813120102.GP5176@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=sivakumar.thulasimani@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.