Thanks for your advice. Maybe my patch log fail to _clearly_ show that there are _two_ differences about irq control interface between gen4 and gen5
> -----Original Message----- > From: Keith Packard [mailto:[email protected]] > Sent: Friday, May 13, 2011 11:08 PM > To: Feng, Boqun; [email protected] > Cc: [email protected] > Subject: Re: [Intel-gfx] [PATCH] drm/i915: fix user irq miss in BSD ring on > g4x > > On Tue, 3 May 2011 12:42:24 +0800, "Feng, Boqun" <[email protected]> > wrote: > > On g4x, user interrupt in BSD ring is missed. > > g4x and ironlake share the same bsd_ring, but their interrupt control > > interfaces are different. On g4x i915_enable_irq and i915_disable_irq ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1st, on gen5, we use ironlake_enable_irq and ironlake_disable_irq > > are used to enable/disable irq,and user interrupt flag in BSD ring on > > g4x is I915_BSD_USER_INTERRUPT. ~~~~~~~~~~~~~~~~~~~~~~~ 2nd, on gen5, we use GT_BSD_USER_INTERRUPT > > The ring_get_irq and ring_put_irq use ironlake style interrupt control > > interface. So rather than use them, expand their code and add an if-else > > statement about the device version. > > Please don't open-code ring_get_irq here. I'd suggest a cleaner fix > would be to either just conditionally call ring_get_irq, or to stick the > BSD interrupt value in dev_priv where you can get it: > > if (IS_G4X(dev)) > ring_get_irq(ring, I915_BSD_USER_INTERRUPT); > else > ring_get_irq(ring, GT_BSD_USER_INTERRUPT); > So, we can't simply conditionally call ring_get_irq, we need to add a conditionally call in ring_get_irq, too: if(is_G4X(dev)) i915_enable_irq(ring, flag); else ironlake_enable_irq(ring, flag); That's why I want to open-code ring_get_irq. > or > ring_get_irq(ring, dev_priv->bsd_user_interrupt); > > -- > [email protected] -- Feng, Boqun _______________________________________________ stable mailing list [email protected] http://linux.kernel.org/mailman/listinfo/stable
