[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