On Wed, Jul 24, 2013 at 3:10 PM, Paulo Zanoni <przanoni@xxxxxxxxx> wrote: >> My first question was why you didn't use name##IMR, name##IER, name##IIR >> in the macros, since that would match what we have in -internal and imo >> would look more natural. But then I've noticed that for IIR, IER & IMR we >> don't have a prefix ... I still think the I in the name looks a bit funny >> though (e.g. GTI). Maybe we should add an I915_ prefix then we could >> remove the I. > > Yeah, you guessed the reason. If we use name##IMR then we could > replace "INTEL_IRQ_REG_INIT(I, false, enable_mask, > dev_priv->irq_mask);" with "INTEL_IRQ_REG_INIT(, false, enable_mask, > dev_priv->irq_mask);", but I guess your suggestion of renaming is > probably better. The problem with renaming to I915_IIR is that the > registers also exists on i8xx, so I'd vote for another name > (GEN2_IIR?). Hm, I've have thought that an empty macro param would trip up the cpp. But for sure it looks funny. I guess we can do the renaming later on, once a bit of -internal has landed. >> The other thing I'm unclear about is the do_iir flag, which in the end >> seems to clear IIR bits in uninstall but not in preinstall. Iirc Ben's >> proposal was to do the double IIR clear in preinstall, and I tend to agree >> with him. Can you please elaborate on this? > > Please look at the final result of the series, we remove the flag on > later patches (you just made me notice I forgot to remove it on the > reg_init functions, but we don't really use it anymore). At the end of > the series we do the double-IIR clear at both preinstall and > uninstall. I wanted to keep one functional change at each patch. Yeah, killing do_iir would be good at the end of this series, maybe together with the kerneldoc patch? >> Maybe one patch on top to add some kerneldoc to the new magic macros would >> be good so that we can explain once what the thinking between the sequence >> is. > > Will do. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx