> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
> ville.syrj...@linux.intel.com
> Sent: Tuesday, May 05, 2015 7:06 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v2 6/9] drm/i915: Disable all infoframes when
> turning off the HDMI port
>
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
>
> 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.syrj...@linux.intel.com>
> ---
> 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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
