On Wed, Mar 27, 2013 at 05:05:54PM +0200, Ville Syrj?l? wrote: > On Wed, Mar 27, 2013 at 03:55:01PM +0100, Daniel Vetter wrote: > > This fixes a regression introduced in > > > > commit e5868a318d1ae28f760f77bb91ce5deb751733fd > > Author: Egbert Eich <eich at suse.de> > > Date: Thu Feb 28 04:17:12 2013 -0500 > > > > DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in encode > > > > Due to the irq setup rework in 3.9, see > > > > commit 20afbda209d708be66944907966486d0c1331cb8 > > Author: Daniel Vetter <daniel.vetter at ffwll.ch> > > Date: Tue Dec 11 14:05:07 2012 +0100 > > > > drm/i915: Fixup hpd irq register setup ordering > > > > Egbert Eich's hpd rework blows up on pch-split platforms - it walks > > the encoder list before that has been set up completely. The new init > > sequence is: > > > > 1. irq enabling > > 2. modeset init > > 3. hpd setup > > > > We need to move around the ibx setup a bit to fix this. > > > > Ville Syrj?l? pointed out in his review that we can't touch SDEIER > > after the interrupt handler is set up, since that'll race with Paulo > > Zanoni's PCH interrupt race fix: > > > > commit 44498aea293b37af1d463acd9658cdce1ecdf427 > > Author: Paulo Zanoni <paulo.r.zanoni at intel.com> > > Date: Fri Feb 22 17:05:28 2013 -0300 > > > > drm/i915: also disable south interrupts when handling them > > > > We fix that by unconditionally enabling all interrupts in SDEIER, but > > masking them as-needed in SDEIMR. Since only the single-threaded > > setup/teardown (or suspend/resume) code touches that, no further > > locking is required. > > > > While at it also simplify the mask handling - we start out with all > > interrupts cleared in the postinstall hook, and never enable a hpd > > interrupt before hpd_irq_setup is called. > > > > And finally, for consistency rename the ibx hpd setup function to > > ibx_hpd_irq_setup. > > > > v2: Fix race around SDEIER writes (Ville). > > > > v3: Remove the superflous posting read for SDEIER, spotted by Ville. > > > > Ville also wondered whether we shouldn't clear SDEIIR, since now > > SDE interrupts are enabled before we have an irq handler installed. > > But the master interrupt control bit in DEIER is still cleared, so we > > should be fine. > > > > Cc: Egbert Eich <eich at suse.de> > > Cc: Jesse Barnes <jbarnes at virtuousgeek.org> > > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com> > > Cc: Ville Syrj?l? <ville.syrjala at linux.intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch> > > Makes sense to me. > > Reviewed-by: Ville Syrj?l? <ville.syrjala at linux.intel.com> Queued for -next, thanks for the patch. I've just pushed down the patch a bit to keep the bisect regression window small, but not squashed - keeping a record of our discussion seemed beneficial. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
