From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH] drm: Improve manual IRQ installation documentation Date: Thu, 20 Jun 2013 13:00:36 +0200 Message-ID: <20130620110034.GA28718@manwe> References: <1371643245-8325-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com> <13858203.X70ZrUFadi@avalon> <20130620104026.GA28308@manwe> <4413102.6AHlGAblh3@avalon> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1185180255==" Return-path: Received: from mail-bk0-f47.google.com (mail-bk0-f47.google.com [209.85.214.47]) by gabe.freedesktop.org (Postfix) with ESMTP id A6196E639C for ; Thu, 20 Jun 2013 04:00:39 -0700 (PDT) Received: by mail-bk0-f47.google.com with SMTP id jg1so2787140bkc.20 for ; Thu, 20 Jun 2013 04:00:38 -0700 (PDT) In-Reply-To: <4413102.6AHlGAblh3@avalon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Laurent Pinchart Cc: Laurent Pinchart , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1185180255== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="gKMricLos+KVdGMg" Content-Disposition: inline --gKMricLos+KVdGMg Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jun 20, 2013 at 12:46:21PM +0200, Laurent Pinchart wrote: > Hi Thierry, >=20 > On Thursday 20 June 2013 12:40:26 Thierry Reding wrote: > > On Thu, Jun 20, 2013 at 12:17:25PM +0200, Laurent Pinchart wrote: > > > On Thursday 20 June 2013 12:10:47 Thierry Reding wrote: > > > > On Wed, Jun 19, 2013 at 02:00:45PM +0200, Laurent Pinchart wrote: > > > > > Signed-off-by: Laurent Pinchart > > > > > > > > > > --- > > > > >=20 > > > > > Documentation/DocBook/drm.tmpl | 14 ++++++++------ > > > > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > >=20 > > > > > diff --git a/Documentation/DocBook/drm.tmpl > > > > > b/Documentation/DocBook/drm.tmpl index f9df3b8..738b727 100644 > > > > > --- a/Documentation/DocBook/drm.tmpl > > > > > +++ b/Documentation/DocBook/drm.tmpl > > > > > @@ -186,11 +186,12 @@ > > > > >=20 > > > > > > > > > > =20 > > > > > DRIVER_HAVE_IRQDRIVER_IRQ_SHARED<= /term > > > > > > > > > > > > > > > >=20 > > > > > - DRIVER_HAVE_IRQ indicates whether the driver has a= n IRQ > > > > > handler. The - DRM core will automatically register = an > > > > > interrupt handler when the - flag is set. > > > > > DRIVER_IRQ_SHARED > > > > > indicates whether the device & - handler support > > > > > shared > > > > > IRQs (note that this is required of PCI - drivers). > > > > > + DRIVER_HAVE_IRQ indicates whether the driver has a= n IRQ > > > > > handler + managed by the DRM Core. The core will sup= port > > > > > simple IRQ handler + installation when the flag is s= et. > > > > > The > > > > > installation process is + described in > > > > linkend=3D"drm-irq-registration"/>. + > > > > > DRIVER_IRQ_SHARED indicates whether the device & handle= r + > > > > >=20 > > > > > support shared IRQs (note that this is required of PCI=20 > > > > > drivers).> > > > > > =20 > > > > > > > > > > =20 > > > > > > > > > > > > > > >=20 > > > > > @@ -344,7 +345,8 @@ char *date; > > > > >=20 > > > > > The DRM core tries to facilitate IRQ handler registrat= ion > > > > > and > > > > > unregistration by providing > > > > > drm_irq_install and > > > > > drm_irq_uninstall functions. Those > > > > > functions only > > > > >=20 > > > > > - support a single interrupt per device. > > > > > + support a single interrupt per device, devices that use > > > > > more > > > > > than one + IRQs need to be handled manually. > > > >=20 > > > > Perhaps this should mention that if you handle IRQ installation man= ually > > > > you also need to manually set drm->irq_enabled =3D 1, as otherwise = things > > > > like DRM_IOCTL_WAIT_VBLANK won't work properly. > > >=20 > > > That's only needed if DRIVER_HAVE_IRQ is set, otherwise the > > > drm_wait_vblank() function skips the irq_enabled check. > >=20 > > Not quite. There's another check for dev->irq_enabled in the > > DRM_WAIT_ON() so it'll never actually block. >=20 > Indeed. >=20 > > Arguably this could be solved by making the DRM_WAIT_ON() condition dro= p the > > !dev->irq_enabled in case DRIVER_HAVE_IRQ isn't set. >=20 > Or we could force drivers to set drm->irq_enabled, I'm fine with that as = well.=20 > I can fix the documentation if that would be the preferred solution. Thinking about it some more, the latter seems more appropriate. Consider some driver that doesn't set DRIVER_HAVE_IRQ but also doesn't initialize interrupts manually. If so it'll cause DRM_IOCTL_WAIT_VBLANK to block indefinitely. On the other hand perhaps the check at the beginning of drm_wait_vblank should be improved. Maybe make it something like this: if (drm_core_check_feature(dev, DRIVER_HAVE_IRQ)) if (!drm_dev_to_irq(dev)) return -EINVAL; if (!dev->irq_enabled) return -EINVAL; Thierry --gKMricLos+KVdGMg Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.20 (GNU/Linux) iQIcBAEBAgAGBQJRwuDSAAoJEN0jrNd/PrOhX5IP/A6RYxMWZzsZ5yfl8qEQcLdY dDEVDFUlQq+fJdAeawXhNVx2wrJk2gdqtS+SvllY+UfXVlIU9v+0B6atJ85LoVKd pMfa2r66A/r4FbcCmyoz5RSq8fwgCv+T39wFmJI/tj3Y9mBwLGkjApaeTFfE5Vj+ 8UVGKHa4E/21aMXqtmhES/Wnp284khC5Pg1Yu3uyahaW/dQWQAOHfTnMOihBdjLY XufYCHIimcIweYSV6nFWCWjhxp/sOGQVn/r7HuA5c9NKe4mrqW2M1q1cPfJPvSfP 8Dvr0HNcrTPzc1RE8574sPYC13OJO+fTm/HLURDHOupcd8g2vqEme43KBmKmstoU KLA4ZCcMnZSN1+LVzax+puHToLYJpeNva9ABTvVWJqqwU5fhVq2sjaoiSgAcRVCf ECwtDbBN2enU6hFhVw9qJAQMex7egjc/dDhY6FuHIZBnz35/yPhoNgjCwCGBnh3P PfkFG8v9KTFV7uAwondRNkShrO8i3lZe5wWsa/AbhT03Th/Lz1RZ6Bs1fI1RyiY0 sFdlBtra/iHsoiJxeq7cGUAvlbSRDIRkDSFcZUm8MbjRZL3LjNDP7IjyaUIulnh4 gzIxqku9gjJY5UyG5nQZbhq80uXmm1/iNezr92Pvi7nUcXxWoYHbcb9fEbhTlXvx HVY6IRG2fwbZ4rTKt8Nw =eHRv -----END PGP SIGNATURE----- --gKMricLos+KVdGMg-- --===============1185180255== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============1185180255==--