Re: [PATCH v2 6/9] drm/i915: Disable all infoframes when turning off the HDMI port

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of
> ville.syrjala@xxxxxxxxxxxxxxx
> Sent: Tuesday, May 05, 2015 7:06 AM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject:  [PATCH v2 6/9] drm/i915: Disable all infoframes when
> turning off the HDMI port
> 
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> Currently we just disable the GCP infoframe when turning off the port.
> That means if the same transcoder is used on a DP port next, we might
> end up pushing infoframes over DP, which isn't intended. Just disable

Wonder how it is working. May be it is ok, or never hit using a previously 
used transcoder for HDMI port for DP.

By the way, you mean end up pushing "other" infoframes over DP?

> all the infoframes when turning off the port.
> 
> Also protect against two ports stomping on each other on g4x due to
> the single video DIP instance. Now only the first port to enable
> gets to send infoframes.

So is, 2nd port doesn't gets to send infoframes, the expected behavior 
when two ports trying with a single DIP?

Seems like a corner case to test thoroughly. Is this path exercised in your testing?

> 
> v2: Rebase
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 85 ++++++++++++++++++---------------------
>  1 file changed, 40 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 766bdb1..03b4759 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -514,7 +514,13 @@ static void g4x_set_infoframes(struct drm_encoder
> *encoder,
>  	if (!enable) {
>  		if (!(val & VIDEO_DIP_ENABLE))
>  			return;
> -		val &= ~VIDEO_DIP_ENABLE;
> +		if (port != (val & VIDEO_DIP_PORT_MASK)) {
> +			DRM_DEBUG_KMS("video DIP still enabled on port
> %c\n",
> +				      (val & VIDEO_DIP_PORT_MASK) >> 29);
> +			return;
> +		}
> +		val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI |
> +			 VIDEO_DIP_ENABLE_VENDOR |
> VIDEO_DIP_ENABLE_SPD);
>  		I915_WRITE(reg, val);
>  		POSTING_READ(reg);
>  		return;
> @@ -522,16 +528,17 @@ static void g4x_set_infoframes(struct drm_encoder
> *encoder,
> 
>  	if (port != (val & VIDEO_DIP_PORT_MASK)) {
>  		if (val & VIDEO_DIP_ENABLE) {
> -			val &= ~VIDEO_DIP_ENABLE;
> -			I915_WRITE(reg, val);
> -			POSTING_READ(reg);
> +			DRM_DEBUG_KMS("video DIP already enabled on port
> %c\n",
> +				      (val & VIDEO_DIP_PORT_MASK) >> 29);
> +			return;
>  		}
>  		val &= ~VIDEO_DIP_PORT_MASK;
>  		val |= port;
>  	}
> 
>  	val |= VIDEO_DIP_ENABLE;
> -	val &= ~VIDEO_DIP_ENABLE_VENDOR;
> +	val &= ~(VIDEO_DIP_ENABLE_AVI |
> +		 VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_SPD);
> 
>  	I915_WRITE(reg, val);
>  	POSTING_READ(reg);
> @@ -632,23 +639,6 @@ static bool intel_hdmi_set_gcp_infoframe(struct
> drm_encoder *encoder)
>  	return val != 0;
>  }
> 
> -static void intel_disable_gcp_infoframe(struct intel_crtc *crtc)
> -{
> -	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> -	u32 reg;
> -
> -	if (HAS_DDI(dev_priv))
> -		reg = HSW_TVIDEO_DIP_CTL(crtc->config->cpu_transcoder);
> -	else if (IS_VALLEYVIEW(dev_priv))
> -		reg = VLV_TVIDEO_DIP_CTL(crtc->pipe);
> -	else if (HAS_PCH_SPLIT(dev_priv->dev))
> -		reg = TVIDEO_DIP_CTL(crtc->pipe);
> -	else
> -		return;
> -
> -	I915_WRITE(reg, I915_READ(reg) & ~VIDEO_DIP_ENABLE_GCP);
> -}
> -
>  static void ibx_set_infoframes(struct drm_encoder *encoder,
>  			       bool enable,
>  			       struct drm_display_mode *adjusted_mode)
> @@ -669,25 +659,26 @@ static void ibx_set_infoframes(struct drm_encoder
> *encoder,
>  	if (!enable) {
>  		if (!(val & VIDEO_DIP_ENABLE))
>  			return;
> -		val &= ~VIDEO_DIP_ENABLE;
> +		val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI |
> +			 VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
> +			 VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP);
>  		I915_WRITE(reg, val);
>  		POSTING_READ(reg);
>  		return;
>  	}
> 
>  	if (port != (val & VIDEO_DIP_PORT_MASK)) {
> -		if (val & VIDEO_DIP_ENABLE) {
> -			val &= ~VIDEO_DIP_ENABLE;
> -			I915_WRITE(reg, val);
> -			POSTING_READ(reg);
> -		}
> +		WARN(val & VIDEO_DIP_ENABLE,
> +		     "DIP already enabled on port %c\n",
> +		     (val & VIDEO_DIP_PORT_MASK) >> 29);
>  		val &= ~VIDEO_DIP_PORT_MASK;
>  		val |= port;
>  	}
> 
>  	val |= VIDEO_DIP_ENABLE;
> -	val &= ~(VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
> -		 VIDEO_DIP_ENABLE_GCP);
> +	val &= ~(VIDEO_DIP_ENABLE_AVI |
> +		 VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
> +		 VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP);
> 
>  	if (intel_hdmi_set_gcp_infoframe(encoder))
>  		val |= VIDEO_DIP_ENABLE_GCP;
> @@ -718,7 +709,9 @@ static void cpt_set_infoframes(struct drm_encoder
> *encoder,
>  	if (!enable) {
>  		if (!(val & VIDEO_DIP_ENABLE))
>  			return;
> -		val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI);
> +		val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI |
> +			 VIDEO_DIP_ENABLE_VENDOR |
> VIDEO_DIP_ENABLE_GAMUT |
> +			 VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP);
>  		I915_WRITE(reg, val);
>  		POSTING_READ(reg);
>  		return;
> @@ -727,7 +720,7 @@ static void cpt_set_infoframes(struct drm_encoder
> *encoder,
>  	/* Set both together, unset both together: see the spec. */
>  	val |= VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI;
>  	val &= ~(VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
> -		 VIDEO_DIP_ENABLE_GCP);
> +		 VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP);
> 
>  	if (intel_hdmi_set_gcp_infoframe(encoder))
>  		val |= VIDEO_DIP_ENABLE_GCP;
> @@ -760,25 +753,26 @@ static void vlv_set_infoframes(struct drm_encoder
> *encoder,
>  	if (!enable) {
>  		if (!(val & VIDEO_DIP_ENABLE))
>  			return;
> -		val &= ~VIDEO_DIP_ENABLE;
> +		val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI |
> +			 VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
> +			 VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP);
>  		I915_WRITE(reg, val);
>  		POSTING_READ(reg);
>  		return;
>  	}
> 
>  	if (port != (val & VIDEO_DIP_PORT_MASK)) {
> -		if (val & VIDEO_DIP_ENABLE) {
> -			val &= ~VIDEO_DIP_ENABLE;
> -			I915_WRITE(reg, val);
> -			POSTING_READ(reg);
> -		}
> +		WARN(val & VIDEO_DIP_ENABLE,
> +		     "DIP already enabled on port %c\n",
> +		     (val & VIDEO_DIP_PORT_MASK) >> 29);
>  		val &= ~VIDEO_DIP_PORT_MASK;
>  		val |= port;
>  	}
> 
>  	val |= VIDEO_DIP_ENABLE;
> -	val &= ~(VIDEO_DIP_ENABLE_AVI | VIDEO_DIP_ENABLE_VENDOR |
> -		 VIDEO_DIP_ENABLE_GAMUT | VIDEO_DIP_ENABLE_GCP);
> +	val &= ~(VIDEO_DIP_ENABLE_AVI |
> +		 VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
> +		 VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP);
> 
>  	if (intel_hdmi_set_gcp_infoframe(encoder))
>  		val |= VIDEO_DIP_ENABLE_GCP;
> @@ -803,15 +797,16 @@ static void hsw_set_infoframes(struct drm_encoder
> *encoder,
> 
>  	assert_hdmi_port_disabled(intel_hdmi);
> 
> +	val &= ~(VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
> +		 VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
> +		 VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW);
> +
>  	if (!enable) {
> -		I915_WRITE(reg, 0);
> +		I915_WRITE(reg, val);
>  		POSTING_READ(reg);
>  		return;
>  	}
> 
> -	val &= ~(VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_GCP_HSW |
> -		 VIDEO_DIP_ENABLE_VS_HSW | VIDEO_DIP_ENABLE_GMP_HSW);
> -
>  	if (intel_hdmi_set_gcp_infoframe(encoder))
>  		val |= VIDEO_DIP_ENABLE_GCP_HSW;
> 
> @@ -1133,7 +1128,7 @@ static void intel_disable_hdmi(struct intel_encoder
> *encoder)
>  	if (IS_CHERRYVIEW(dev))
>  		chv_powergate_phy_lanes(encoder, 0xf);
> 
> -	intel_disable_gcp_infoframe(to_intel_crtc(encoder->base.crtc));
> +	intel_hdmi->set_infoframes(&encoder->base, false, NULL);
>  }
> 
>  static int hdmi_portclock_limit(struct intel_hdmi *hdmi, bool respect_dvi_limit)
> --
> 2.0.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux