On Thu, Jun 20, 2013 at 12:46:21PM +0200, Laurent Pinchart wrote:
Hi Thierry,
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
laurent.pinchart+renesas@ideasonboard.com
Documentation/DocBook/drm.tmpl | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
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 @@
<varlistentry> <term>DRIVER_HAVE_IRQ</term><term>DRIVER_IRQ_SHARED</term > <listitem><para>
DRIVER_HAVE_IRQ indicates whether the driver has an IRQhandler. 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 an IRQhandler + managed by the DRM Core. The core will support simple IRQ handler + installation when the flag is set. The installation process is + described in <xref linkend="drm-irq-registration"/>.</para> + <para>DRIVER_IRQ_SHARED indicates whether the device & handler +
support shared IRQs (note that this is required of PCI drivers).> </para></listitem> </varlistentry> <varlistentry>@@ -344,7 +345,8 @@ char *date;</synopsis>
The DRM core tries to facilitate IRQ handler registration and unregistration by providing <function>drm_irq_install</function> and <function>drm_irq_uninstall</function> functions. Those functions only
support a single interrupt per device.
support a single interrupt per device, devices that usemore than one + IRQs need to be handled manually.
Perhaps this should mention that if you handle IRQ installation manually you also need to manually set drm->irq_enabled = 1, as otherwise things like DRM_IOCTL_WAIT_VBLANK won't work properly.
That's only needed if DRIVER_HAVE_IRQ is set, otherwise the drm_wait_vblank() function skips the irq_enabled check.
Not quite. There's another check for dev->irq_enabled in the DRM_WAIT_ON() so it'll never actually block.
Indeed.
Arguably this could be solved by making the DRM_WAIT_ON() condition drop the !dev->irq_enabled in case DRIVER_HAVE_IRQ isn't set.
Or we could force drivers to set drm->irq_enabled, I'm fine with that as well. 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