Hey Daniel, I had v3 patches under review and will send them out later. > > > dev_priv->pipe_to_crtc_mapping[pipe]; > > > > > > + DRM_DEBUG_DRIVER("Enable transcoder %c\n", > pipe_name(pipe)); > > > > Do we really want this? > > ... and in kms code we use DRM_DEBUG_KMS. I'd removed this debug message in v3. > > > > /* PCH only available on I > > > > I know this is not part of your commit, but can't we just use the PIPE > > macro instead of this "var += i * 0x100" ? > > Yeah, and there are a few other places in the eld code that would welcome > some similar love. Renaming the temporary var i to tmp or reg would also look > a bit better. But such cleanups would need to happen in a separate patch. They are replaced with macro in v3 too. > > > > @@ -5135,6 +5191,8 @@ static void ironlake_write_eld(struct > drm_connector *connector, > > > i = I915_READ(aud_cntl_st); > > > i &= ~IBX_ELD_ADDRESS; > > > I915_WRITE(aud_cntl_st, i); > > > + i = (i >> 29) & 0x3; /* DIP_Port_Select, 0x1 = PortB > */ > > > + DRM_DEBUG_DRIVER("port num:%d\n", i); > > > > > > len = min_t(uint8_t, eld[2], 21); /* 84 bytes of hw ELD > buffer */ > > > DRM_DEBUG_DRIVER("ELD size %d\n", len); > > > > ironlake_write_eld looks scary... > > Imo the hsw code is sufficiently different to varant a haswell_write_eld. Hmm, I think I need write a separate patch to place the haswell_write_eld. Thanks --xingchao