On Tue, Jul 23, 2013 at 07:33:40PM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > Hi > > This patch series is based on emails I sent a few days ago, with subject > "proposals/questions about the IRQ registers". The biggest goal of the series is > to have an unified way to initialize/reset IRQ regsiters. After this series, > when you add new interrupts to our driver you won't need to think about doing > the correct reads, writes and posting_reads, all you'll need to worry about is > to call the macros everyone else calls. So now either all the registers are > initialized correctly or all the registers are initialized wrongly. > > The first 10 patches do all the rework on i915+. The changes should be > incremental and any non-trivial changes are on separate patches, so bisecting > regressions should be very easy. It is worth mentioning that patches 2, 6, 9 and > 10 touch VLV-specific code, and I don't have a way to test this code. > > On my intial patches the i8xx code was changed along with the i915+ code, but > Ben suggested I shouldn't be touching i8xx code, so patches 11-15 do the i8xx > work. I don't know if we really want these patches, but they're here anyway, so > feel free to give them a NACK, I won't complain. In case we decide to merge the > patches, git bisect should easily spot regressions. > > Patches tested on Haswell (constant use for 2 weeks), SandyBridge (a few hours > of usage) and Atom (only booted and then suspend/resumed once). 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. 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? 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. Cheers, Daniel > > Thanks, > Paulo > > Paulo Zanoni (15): > drm/i915: add INTEL_IRQ_REG_RESET > drm/i915: change how VLV_IIR is reset > drm/i915: port i965_irq_uninstall go INTEL_IRQ_REG_RESET > drm/i915: really clear the IIR registers > drm/i915: add INTEL_IRQ_REG_INIT > drm/i915: use INTEL_IRQ_REG_INIT on VLV too > drm/i915: reset the IIR registers at preinstall > drm/i915: WARN if IIR is not zero at irq_postinstall > drm/i915: remove additional zerogin of VLV_IIR at postinstall > drm/i915: remove extra clearing of GTIIR from VLV irq preinstall > drm/i915: add INTEL_IRQ_REG_RESET16 > drm/i915: really clear the IIR registers on i8xx > drm/i915: add INTEL_IRQ_REG_INIT16 > drm/i915: reset the i8xx IIR registers at preinstall > drm/i915: WARN if IIR is not zero at i8xx irq_postinstall > > drivers/gpu/drm/i915/i915_irq.c | 144 +++++++++++++++++----------------------- > 1 file changed, 62 insertions(+), 82 deletions(-) > > -- > 1.8.1.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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
