RE: [Intel-gfx] [PATCH] drm/i915: Remove unused IRQ chip data of HDMI LPE audio

["Chen, Augustine" <augustine.chen@intel.com>]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, December 12, 2017 5:45 PM
> To: Chen, Augustine <augustine.chen@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Anand, Jerome
> <jerome.anand@intel.com>; Thomas Gleixner <tglx@linutronix.de>; intel-
> gfx@lists.freedesktop.org; alsa-devel@alsa-project.org; Bossart, Pierre-louis
> <pierre-louis.bossart@intel.com>; Ingo Molnar <mingo@redhat.com>; H. Peter
> Anvin <hpa@zytor.com>; Jiang Liu <jiang.liu@linux.intel.com>; Juergen Gross
> <jgross@suse.com>; Dou Liyang <douly.fnst@cn.fujitsu.com>; linux-
> kernel@vger.kernel.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Remove unused IRQ chip data of
> HDMI LPE audio
> 
> On Tue, 12 Dec 2017 10:26:08 +0100,
> Chen, Augustine wrote:
> > > > > > > That *looks* more correct to me based on a cursory glance at
> > > > > > > the
> > > > > > > x86 code, but I didn't trawl very deeply.
> > > > > >
> > > > > > The x86 core cares not at all about interrupt chips which are
> > > > > > created in a driver and not connected to an actual
> > > > > > apic/ioapic/msi interrupt. This interrupt chip is its own thing as we have
> others in GPIO etc.
> > > > > >
> > > > > > > > In the case of not enabling CONFIG_CPUMASK_OFFSTACK, this
> > > > > > > > would cause kernel panic while doing CPU hotplug.
> > > > > >
> > > > > > And why so? Surely not because you set irq_chip_data. That's
> > > > > > really no explanation at all.
> > > > > >
> > > > >
> > > > > Ideally, I feel there needs to be an irq domain for mapping the
> > > > > irq numbers
> > > with hwirq.
> > > > > It is not created as part of the hdmi lpe audio bridge.
> > > > > Since the logic to mask/unmask lpe audio interrupts is removed,
> > > > > there is no need of the Chip data or creation of the domain now.
> > > >
> > > > There is no need right now. But there might be a need in the future.
> > > > LPE audio isn't even the only piece of hardware whose irq goes
> > > > through the i915 display engine (there's also the ISP and VED). So
> > > > I would much prefer a proper solution instead of sweeping the
> > > > problem under the rug.
> > >
> > > IMO, the primary question is whether the usage of irq chip without
> > > irq domain is valid or not.  If an irq domain is mandatory, that's
> > > the thing to be fixed in i915 side.
> > In terms of functionality, the interrupt and hdmi audio work fine
> > without irq domain according to the validation.
> 
> Sure, otherwise the patch never landed to mainline :)
> 
> > And besides, there
> > are other drivers with similar implementation which doesn't set chip
> > data at all.
> 
> Yes, dropping the chip data should be OK in the driver, but the only question is
> whether it is the right fix.
> 
> Did you check whether the issue happens with 4.15-rc, too?  This was never
> answered in bugzilla.  If I understand correctly, the code triggering Oops has
> been changed largely there and it should prune the non-legacy irqs.
> 
I tested it with 4.15-RC1 today and couldn't observed the same symptom. And yes, the code running into invalid pointer was modified dramatically.
Feels like it's available to keep this driver intact.


> 
> Takashi