From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: implement ibx_hpd_irq_setup Date: Wed, 27 Mar 2013 16:42:01 +0100 Message-ID: <20130327154201.GC2228@phenom.ffwll.local> References: <20130327140316.GA4469@intel.com> <1364396101-32056-1-git-send-email-daniel.vetter@ffwll.ch> <20130327150554.GD4469@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ee0-f53.google.com (mail-ee0-f53.google.com [74.125.83.53]) by gabe.freedesktop.org (Postfix) with ESMTP id CB01CE6270 for ; Wed, 27 Mar 2013 08:39:12 -0700 (PDT) Received: by mail-ee0-f53.google.com with SMTP id c13so1595282eek.26 for ; Wed, 27 Mar 2013 08:39:11 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130327150554.GD4469@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: Egbert Eich , Daniel Vetter , Intel Graphics Development , Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Wed, Mar 27, 2013 at 05:05:54PM +0200, Ville Syrj=E4l=E4 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 > > 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 > > 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=E4l=E4 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 > > 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 > > Cc: Jesse Barnes > > Cc: Paulo Zanoni > > Cc: Ville Syrj=E4l=E4 > > Signed-off-by: Daniel Vetter > = > Makes sense to me. > = > Reviewed-by: Ville Syrj=E4l=E4 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