Re: [PATCH] drm/i915/gen4: Extra CRT hotplug paranoia

From: Andrew Lutomirski <luto@mit.edu>
To: Adam Jackson <ajax@redhat.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/gen4: Extra CRT hotplug paranoia
Date: Tue, 25 May 2010 10:57:31 -0400	[thread overview]
Message-ID: <AANLkTilduFC2_96CibQG2ZX702MuSJiu-kP0V0lw3Hjh@mail.gmail.com> (raw)
In-Reply-To: <1274798792.25350.20913.camel@atropine.boston.devel.redhat.com>

On Tue, May 25, 2010 at 10:46 AM, Adam Jackson <ajax@redhat.com> wrote:
> On Mon, 2010-05-24 at 21:46 -0400, Andrew Lutomirski wrote:
>> On Mon, May 24, 2010 at 4:46 PM, Adam Jackson <ajax@redhat.com> wrote:
>> > Disable the CRT plug interrupt while doing the force cycle, explicitly
>> > clear any CRT interrupt we may have generated, and restore when done.
>> > Should mitigate interrupt storms from hotplug detection.
>> >
>>
>> Is there any locking in here to protect PORT_HOTPLUG_EN?  I'm asking
>> because I *still* can't use mainline kernels reliably without
>> commenting out digital output initialization, and that's one place
>> where a bug might be lurking.
>
> At least in d-i-n, PORT_HOTPLUG_EN is only ever written to from ->detect
> in the connector vtable, and from IRQ setup.  I don't have a firm
> understanding of the locking around either path, but I'd be rather
> surprised if it was racy in any practical sense, since neither of those
> should get called from interrupt context and you typically only have one
> app doing setup.

->detect seems to be called from status_show in drm_sysfs.c, which
makes its way into intel_crt_detect_hotplug, which plays with
PORT_HOTPLUG_EN without any locking.  If another detect function (or
the same one, even) is called at the same time, PORT_HOTPLUG_EN can be
left in a bad state.

There should probably be a mutex protecting PORT_HOTPLUG_EN.

--Andy

>
> - ajax
>