[Intel-gfx] [PATCH V2 3/4] drm/i915: Haswell HDMI audio enable
Wang, Xingchao
xingchao.wang at intel.com
Mon Aug 6 12:10:35 CEST 2012
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
More information about the Intel-gfx
mailing list