On Thu, May 25, 2017 at 12:46:29AM -0700, Stefan Agner wrote:
> On 2017-05-24 07:51, Daniel Vetter wrote:
> > Pull a (much shorter) overview into drm_irq.c, and instead put the
> > callback documentation into in-line comments in drm_drv.h.
>
> Looks good and just found all I needed to know to fix IRQ registration
> in fsl dcu.
>
> Reviewed-by: Stefan Agner <stefan@xxxxxxxx>
Can you pls ack/review the previous patch 7 too? I can't apply this one
here with the other one ...
Thanks, Daniel
>
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> > ---
> > Documentation/gpu/drm-internals.rst | 62 +++++--------------------------------
> > drivers/gpu/drm/drm_irq.c | 30 +++++++++++++++---
> > drivers/gpu/drm/drm_vblank.c | 3 ++
> > include/drm/drmP.h | 9 ++++--
> > include/drm/drm_drv.h | 33 ++++++++++++++++++--
> > 5 files changed, 74 insertions(+), 63 deletions(-)
> >
> > diff --git a/Documentation/gpu/drm-internals.rst
> > b/Documentation/gpu/drm-internals.rst
> > index d218dd29221a..82b406d3d377 100644
> > --- a/Documentation/gpu/drm-internals.rst
> > +++ b/Documentation/gpu/drm-internals.rst
> > @@ -149,60 +149,14 @@ Device Instance and Driver Handling
> > Driver Load
> > -----------
> >
> > -IRQ Registration
> > -~~~~~~~~~~~~~~~~
> > -
> > -The DRM core tries to facilitate IRQ handler registration and
> > -unregistration by providing :c:func:`drm_irq_install()` and
> > -:c:func:`drm_irq_uninstall()` functions. Those functions only
> > -support a single interrupt per device, devices that use more than one
> > -IRQs need to be handled manually.
> > -
> > -Managed IRQ Registration
> > -''''''''''''''''''''''''
> > -
> > -:c:func:`drm_irq_install()` starts by calling the irq_preinstall
> > -driver operation. The operation is optional and must make sure that the
> > -interrupt will not get fired by clearing all pending interrupt flags or
> > -disabling the interrupt.
> > -
> > -The passed-in IRQ will then be requested by a call to
> > -:c:func:`request_irq()`. If the DRIVER_IRQ_SHARED driver feature
> > -flag is set, a shared (IRQF_SHARED) IRQ handler will be requested.
> > -
> > -The IRQ handler function must be provided as the mandatory irq_handler
> > -driver operation. It will get passed directly to
> > -:c:func:`request_irq()` and thus has the same prototype as all IRQ
> > -handlers. It will get called with a pointer to the DRM device as the
> > -second argument.
> > -
> > -Finally the function calls the optional irq_postinstall driver
> > -operation. The operation usually enables interrupts (excluding the
> > -vblank interrupt, which is enabled separately), but drivers may choose
> > -to enable/disable interrupts at a different time.
> > -
> > -:c:func:`drm_irq_uninstall()` is similarly used to uninstall an
> > -IRQ handler. It starts by waking up all processes waiting on a vblank
> > -interrupt to make sure they don't hang, and then calls the optional
> > -irq_uninstall driver operation. The operation must disable all hardware
> > -interrupts. Finally the function frees the IRQ by calling
> > -:c:func:`free_irq()`.
> > -
> > -Manual IRQ Registration
> > -'''''''''''''''''''''''
> > -
> > -Drivers that require multiple interrupt handlers can't use the managed
> > -IRQ registration functions. In that case IRQs must be registered and
> > -unregistered manually (usually with the :c:func:`request_irq()` and
> > -:c:func:`free_irq()` functions, or their :c:func:`devm_request_irq()` and
> > -:c:func:`devm_free_irq()` equivalents).
> > -
> > -When manually registering IRQs, drivers must not set the
> > -DRIVER_HAVE_IRQ driver feature flag, and must not provide the
> > -irq_handler driver operation. They must set the :c:type:`struct
> > -drm_device <drm_device>` irq_enabled field to 1 upon
> > -registration of the IRQs, and clear it to 0 after unregistering the
> > -IRQs.
> > +IRQ Helper Library
> > +~~~~~~~~~~~~~~~~~~
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_irq.c
> > + :doc: irq helpers
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_irq.c
> > + :export:
> >
> > Memory Manager Initialization
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index 28d736c3fcb4..3b04c25100ae 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -62,19 +62,39 @@
> > #include "drm_internal.h"
> >
> > /**
> > + * DOC: irq helpers
> > + *
> > + * The DRM core provides very simple support helpers to enable IRQ
> > handling on a
> > + * device through the drm_irq_install() and drm_irq_uninstall() functions. This
> > + * only supports devices with a single interrupt on the main device stored in
> > + * &drm_device.dev and set as the device paramter in drm_dev_alloc().
> > + *
> > + * These IRQ helpers are strictly optional. Drivers which roll their own only
> > + * need to set &drm_device.irq_enabled to signal the DRM core that vblank
> > + * interrupts are working. Since these helpers don't automatically clean up the
> > + * requested interrupt like e.g. devm_request_irq() they're not really
> > + * recommended.
> > + */
> > +
> > +/**
> > * drm_irq_install - install IRQ handler
> > * @dev: DRM device
> > * @irq: IRQ number to install the handler for
> > *
> > * Initializes the IRQ related data. Installs the handler, calling the driver
> > - * irq_preinstall() and irq_postinstall() functions before and after the
> > - * installation.
> > + * &drm_driver.irq_preinstall and &drm_driver.irq_postinstall functions before
> > + * and after the installation.
> > *
> > * This is the simplified helper interface provided for drivers with no special
> > * needs. Drivers which need to install interrupt handlers for multiple
> > * interrupts must instead set &drm_device.irq_enabled to signal the DRM core
> > * that vblank interrupts are available.
> > *
> > + * @irq must match the interrupt number that would be passed to request_irq(),
> > + * if called directly instead of using this helper function.
> > + *
> > + * &drm_driver.irq_handler is called to handle the registered interrupt.
> > + *
> > * Returns:
> > * Zero on success or a negative error code on failure.
> > */
> > @@ -136,9 +156,9 @@ EXPORT_SYMBOL(drm_irq_install);
> > * drm_irq_uninstall - uninstall the IRQ handler
> > * @dev: DRM device
> > *
> > - * Calls the driver's irq_uninstall() function and unregisters the IRQ handler.
> > - * This should only be called by drivers which used drm_irq_install() to set up
> > - * their interrupt handler. Other drivers must only reset
> > + * Calls the driver's &drm_driver.irq_uninstall function and
> > unregisters the IRQ
> > + * handler. This should only be called by drivers which used drm_irq_install()
> > + * to set up their interrupt handler. Other drivers must only reset
> > * &drm_device.irq_enabled to false.
> > *
> > * Note that for kernel modesetting drivers it is a bug if this function fails.
> > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > index 630dc26379b7..463e4d81fb0d 100644
> > --- a/drivers/gpu/drm/drm_vblank.c
> > +++ b/drivers/gpu/drm/drm_vblank.c
> > @@ -363,6 +363,9 @@ static void vblank_disable_fn(unsigned long arg)
> > * @dev: DRM device
> > *
> > * This function cleans up any resources allocated in drm_vblank_init.
> > + *
> > + * Drivers which don't use drm_irq_install() need to set
> > &drm_device.irq_enabled
> > + * themselves, to signal to the DRM core that vblank interrupts are enabled.
> > */
> > void drm_vblank_cleanup(struct drm_device *dev)
> > {
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index 2e0b76cceb97..39df16af7a4a 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -377,8 +377,13 @@ struct drm_device {
> > int last_context; /**< Last current context */
> > /*@} */
> >
> > - /** \name VBLANK IRQ support */
> > - /*@{ */
> > + /**
> > + * @irq_enabled:
> > + *
> > + * Indicates that interrupt handling is enabled, specifically vblank
> > + * handling. Drivers which don't use drm_irq_install() need to set this
> > + * to true manually.
> > + */
> > bool irq_enabled;
> > int irq;
> >
> > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> > index ebb41688581b..f495eee01302 100644
> > --- a/include/drm/drm_drv.h
> > +++ b/include/drm/drm_drv.h
> > @@ -325,11 +325,40 @@ struct drm_driver {
> > struct timeval *vblank_time,
> > bool in_vblank_irq);
> >
> > - /* these have to be filled in */
> > -
> > + /**
> > + * @irq_handler:
> > + *
> > + * Interrupt handler called when using drm_irq_install(). Not used by
> > + * drivers which implement their own interrupt handling.
> > + */
> > irqreturn_t(*irq_handler) (int irq, void *arg);
> > +
> > + /**
> > + * @irq_preinstall:
> > + *
> > + * Optional callback used by drm_irq_install() which is called before
> > + * the interrupt handler is registered. This should be used to clear out
> > + * any pending interrupts (from e.g. firmware based drives) and reset
> > + * the interrupt handling registers.
> > + */
> > void (*irq_preinstall) (struct drm_device *dev);
> > +
> > + /**
> > + * @irq_postinstall:
> > + *
> > + * Optional callback used by drm_irq_install() which is called after
> > + * the interrupt handler is registered. This should be used to enable
> > + * interrupt generation in the hardware.
> > + */
> > int (*irq_postinstall) (struct drm_device *dev);
> > +
> > + /**
> > + * @irq_uninstall:
> > + *
> > + * Optional callback used by drm_irq_uninstall() which is called before
> > + * the interrupt handler is unregistered. This should be used to disable
> > + * interrupt generation in the hardware.
> > + */
> > void (*irq_uninstall) (struct drm_device *dev);
> >
> > /**
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
