On Fri, 08 Mar 2013 15:33:32 +0200 Jani Nikula <jani.nikula at linux.intel.com> wrote: > > + intel_dpio_write(dev_priv, 0x8438, 0x00760018); > > + intel_dpio_write(dev_priv, 0x845c, 0x00400888); > > + > > + intel_dpio_write(dev_priv, 0x8400, 0x10080); > > + intel_dpio_write(dev_priv, 0x8404, 0x00600060); > > + } > > +} > > Dunno, it feels a bit funny that you add loads of #defines for the dpio > stuff, and then use magic numbers here. > > Also, some of the regs are per-pipe, which is probably all right given > the intel_pipe_has_type() checks, but perhaps it would be more > self-explanatory if the pipe number was used anyway. > > All in all, just a /* XXX: clear these up */ would be good too. I know, it's ugly. I'm having to make up names for some of these since we don't have real docs for them, just cspec info. Agree that they could be nicer, but w/o a real programming guide they won't map back to anything useful anyway, so the numbers may actually be better. I took your other comments into account already from your last review, I'll post updated patches today. -- Jesse Barnes, Intel Open Source Technology Center
