On Mon, May 28, 2012 at 04:43:01PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>
> This function is supposed to be used at mode set time, so prevent
> against future mistakes by adding a WARN().
>
> Problems when calling it when the HDMI port is enabled:
> - It may disable sending DIPs (on IVB and older), and the register
> specification says we shouldn't disable DIPs with the HDMI port
> enabled.
> - It does not wait for the VSync because the pipe is disabled at
> mode set.
>
> The second condition can be fixed with some additional code, but the
> first one is not so easy... Ideally, code changing the infoframes
> outside of the mode set should wait for the VSync and then directly
> call the write_infoframe functions.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
Can't we extract this into a little assert_hdmi_port_disabled(dev_priv,
reg) like we have with e.g. assert_pch_hdmi_disabled? Difference would be
that we don't check for the pipe here ... Imo that would make the code
slight less verbose and the intention behind the check a bit clearer.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_hdmi.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index f61af21..6fda359 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -334,6 +334,9 @@ static void g4x_set_infoframes(struct drm_encoder *encoder,
> u32 val = I915_READ(reg);
> u32 port;
>
> + WARN(I915_READ(intel_hdmi->hdmi_reg) & SDVO_ENABLE,
> + "Calling set_infoframes with HDMI port enabled");
> +
> /* If the registers were not initialized yet, they might be zeroes,
> * which means we're selecting the AVI DIP and we're setting its
> * frequency to once. This seems to really confuse the HW and make
> @@ -395,6 +398,9 @@ static void ibx_set_infoframes(struct drm_encoder *encoder,
> u32 val = I915_READ(reg);
> u32 port;
>
> + WARN(I915_READ(intel_hdmi->hdmi_reg) & SDVO_ENABLE,
> + "Calling set_infoframes with HDMI port enabled");
> +
> /* See the big comment in g4x_set_infoframes() */
> val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
>
> @@ -451,6 +457,9 @@ static void cpt_set_infoframes(struct drm_encoder *encoder,
> u32 reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
> u32 val = I915_READ(reg);
>
> + WARN(I915_READ(intel_hdmi->hdmi_reg) & SDVO_ENABLE,
> + "Calling set_infoframes with HDMI port enabled");
> +
> /* See the big comment in g4x_set_infoframes() */
> val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
>
> @@ -484,6 +493,9 @@ static void vlv_set_infoframes(struct drm_encoder *encoder,
> u32 reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
> u32 val = I915_READ(reg);
>
> + WARN(I915_READ(intel_hdmi->hdmi_reg) & SDVO_ENABLE,
> + "Calling set_infoframes with HDMI port enabled");
> +
> /* See the big comment in g4x_set_infoframes() */
> val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
>
> @@ -516,6 +528,9 @@ static void hsw_set_infoframes(struct drm_encoder *encoder,
> u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe);
> u32 val = I915_READ(reg);
>
> + WARN(I915_READ(intel_hdmi->hdmi_reg) & DDI_BUF_CTL_ENABLE,
> + "Calling set_infoframes with HDMI port enabled");
> +
> if (!intel_hdmi->has_hdmi_sink) {
> I915_WRITE(reg, 0);
> POSTING_READ(reg);
> --
> 1.7.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48
