On Wed, 30 May 2012 12:31:57 +0200, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> Bspec Vol 3, Part 3, Section 3.8.1.1, bit 30:
>
> "[DevIBX] Writing to this bit only takes effect when port is enabled.
> Due to hardware issue it is required that this bit be cleared when port
> is disabled. To clear this bit software must temporarily enable this
> port on transcoder A."
>
> Unfortunately the public Bspec misses totally out on the same language
> for HDMIB. Internal Bspec also mentions that one of the bad
> side-effects is that DPx can fail to light up on transcoder A if HDMIx
> is disabled but using transcoder B.
>
> I've found this while reviewing Bsepc. We already implement the same
> workaround for the DP ports.
>
> Also replace a magic 1 with PIPE_B I've found while looking through the
> code.
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_hdmi.c | 29 ++++++++++++++++++++++++++++-
> 1 files changed, 28 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 4c6f141..4ebdcf1 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -382,7 +382,7 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
>
> if (HAS_PCH_CPT(dev))
> sdvox |= PORT_TRANS_SEL_CPT(intel_crtc->pipe);
> - else if (intel_crtc->pipe == 1)
> + else if (intel_crtc->pipe == PIPE_B)
> sdvox |= SDVO_PIPE_B_SELECT;
>
> I915_WRITE(intel_hdmi->sdvox_reg, sdvox);
> @@ -405,6 +405,33 @@ static void intel_hdmi_dpms(struct drm_encoder *encoder, int mode)
>
> temp = I915_READ(intel_hdmi->sdvox_reg);
>
> + /* HW workaround for IBX, we need to move the port to transcoder A
> + * before disabling it. */
> + if (HAS_PCH_IBX(dev)) {
> + struct drm_crtc *crtc = encoder->crtc;
> + struct intel_crtc *intel_crtc;
> +
> + if (crtc)
> + intel_crtc = to_intel_crtc(crtc);
> + else
> + intel_crtc = NULL;
If you extracted pipe = crtc ? to_intel_crtc(crtc)->pipe : -1; then that
would make the following code less ugly.
> +
> + if (mode != DRM_MODE_DPMS_ON && (temp & SDVO_PIPE_B_SELECT)) {
Split this into a nested if:
if (mode != DPMS_MODE_DPMS_ON) {
if (temp & SDVO_PIPE_B_SELECT) {
// w/a
}
} else {
// restore
}
> + temp &= ~SDVO_PIPE_B_SELECT;
> + I915_WRITE(intel_hdmi->sdvox_reg, temp);
> + POSTING_READ(intel_hdmi->sdvox_reg);
Should we not worry about the HW w/a requiring this to be written twice?
static void intel_hdmi_write_sdvox(intel_hdmi, u32 val)
{
struct drm_i915_private *dev_priv = intel_hdmi->base.dev->dev_private;
I915_WRITE(intel_hdmi->sdvox_reg, val);
POSTING_READ(intel_hdmi->sdvo_reg);
/* HW workaround, need to write this twice for issues that may result
* in first write getting masked.
*/
if (dev_priv->info.has_pch_split) {
I915_WRITE(intel_hdmi->sdvox_reg, val);
POSTING_READ(intel_hdmi->sdvox_reg);
}
}
> +
> + if (intel_crtc)
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + else
> + msleep(50);
> + } else if (mode == DRM_MODE_DPMS_ON){
> + /* Restore the transcoder select bit. */
> + if (intel_crtc->pipe == PIPE_B)
> + enable_bits |= SDVO_PIPE_B_SELECT;
> + }
> + }
> +
> /* HW workaround, need to toggle enable bit off and on for 12bpc, but
> * we do this anyway which shows more stable in testing.
> */
--
Chris Wilson, Intel Open Source Technology Centre
