[PATCH] drm/amdgpu: update documentation for amdgpu_irq.c

Christian König ckoenig.leichtzumerken at gmail.com
Fri Jun 8 07:58:36 UTC 2018


Am 07.06.2018 um 23:46 schrieb Slava Abramov:
> Add/update function level documentation and add reference to amdgpu_irq.c
> in amdgpu.rst
>
> Signed-off-by: Slava Abramov <slava.abramov at amd.com>
> ---
>   Documentation/gpu/amdgpu.rst            |   9 +++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 102 ++++++++++++++++++++++----------
>   2 files changed, 80 insertions(+), 31 deletions(-)
>
> diff --git a/Documentation/gpu/amdgpu.rst b/Documentation/gpu/amdgpu.rst
> index a4852f9a6141..63b0bc3c30d1 100644
> --- a/Documentation/gpu/amdgpu.rst
> +++ b/Documentation/gpu/amdgpu.rst
> @@ -44,3 +44,12 @@ MMU Notifier
>   
>   .. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>      :internal:
> +
> +Interrupt Handling
> +------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> +   :doc: Interrupt Handling
> +
> +.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> +   :internal:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> index 3a5ca462abf0..07a2642de634 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> @@ -25,6 +25,11 @@
>    *          Alex Deucher
>    *          Jerome Glisse
>    */
> +
> +/**
> + * DOC: Interrupt Handling
> + */
> +

Some introduction text would be nice to have here, but only optional.

Apart from that looks good to me on first glance, but I really only 
skimmed over it.

Patch is Acked-by: Christian König <christian.koenig at amd.com>.

Regards,
Christian.

>   #include <linux/irq.h>
>   #include <drm/drmP.h>
>   #include <drm/drm_crtc_helper.h>
> @@ -44,7 +49,7 @@
>   #define AMDGPU_WAIT_IDLE_TIMEOUT 200
>   
>   /*
> - * Handle hotplug events outside the interrupt handler proper.
> + * XXX: handle hotplug events outside the interrupt handler proper.
>    */
>   /**
>    * amdgpu_hotplug_work_func - display hotplug work handler
> @@ -91,7 +96,13 @@ static void amdgpu_irq_reset_work_func(struct work_struct *work)
>   		amdgpu_device_gpu_recover(adev, NULL, false);
>   }
>   
> -/* Disable *all* interrupts */
> +/**
> + * amdgpu_irq_disable_all - disable *all* interrupts
> + *
> + * @adev: amdgpu device pointer
> + *
> + * Disable all types of interrupts from all sources.
> + */
>   void amdgpu_irq_disable_all(struct amdgpu_device *adev)
>   {
>   	unsigned long irqflags;
> @@ -125,9 +136,10 @@ void amdgpu_irq_disable_all(struct amdgpu_device *adev)
>   /**
>    * amdgpu_irq_handler - irq handler
>    *
> - * @int irq, void *arg: args
> + * @irq: irq number (**not used**)
> + * @arg: pointer to drm device
>    *
> - * This is the irq handler for the amdgpu driver (all asics).
> + * Irq handler for amdgpu driver (all asics).
>    */
>   irqreturn_t amdgpu_irq_handler(int irq, void *arg)
>   {
> @@ -142,14 +154,12 @@ irqreturn_t amdgpu_irq_handler(int irq, void *arg)
>   }
>   
>   /**
> - * amdgpu_msi_ok - asic specific msi checks
> + * amdgpu_msi_ok - check whether MSI functionality is enabled
>    *
> - * @adev: amdgpu device pointer
> + * @adev: amdgpu device pointer (**not used**)
>    *
> - * Handles asic specific MSI checks to determine if
> - * MSIs should be enabled on a particular chip (all asics).
> - * Returns true if MSIs should be enabled, false if MSIs
> - * should not be enabled.
> + * Checks whether MSI functionality has been disabled via module parameter.
> + * Returns true if MSIs are allowed to be enabled or false otherwise
>    */
>   static bool amdgpu_msi_ok(struct amdgpu_device *adev)
>   {
> @@ -163,12 +173,13 @@ static bool amdgpu_msi_ok(struct amdgpu_device *adev)
>   }
>   
>   /**
> - * amdgpu_irq_init - init driver interrupt info
> + * amdgpu_irq_init - initialize driver interrupt info
>    *
>    * @adev: amdgpu device pointer
>    *
> - * Sets up the work irq handlers, vblank init, MSIs, etc. (all asics).
> - * Returns 0 for success, error for failure.
> + * Sets up work functions for hotplug and reset interrupts, enables MSI
> + * functionality, initializes vblank, hotplug and reset interrupt handling.
> + * Returns 0 on success, error code on failure.
>    */
>   int amdgpu_irq_init(struct amdgpu_device *adev)
>   {
> @@ -176,7 +187,7 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
>   
>   	spin_lock_init(&adev->irq.lock);
>   
> -	/* enable msi */
> +	/* enable msi if not disabled by module parameter */
>   	adev->irq.msi_enabled = false;
>   
>   	if (amdgpu_msi_ok(adev)) {
> @@ -224,7 +235,9 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
>    *
>    * @adev: amdgpu device pointer
>    *
> - * Tears down the work irq handlers, vblank handlers, MSIs, etc. (all asics).
> + * Tears down work functions for hotplug and reset interrupts, disables MSI
> + * functionality, shuts down vblank, hotplug and reset interrupt handling,
> + * turns off interrupts from all sources (all asics).
>    */
>   void amdgpu_irq_fini(struct amdgpu_device *adev)
>   {
> @@ -267,9 +280,12 @@ void amdgpu_irq_fini(struct amdgpu_device *adev)
>    * amdgpu_irq_add_id - register irq source
>    *
>    * @adev: amdgpu device pointer
> - * @src_id: source id for this source
> - * @source: irq source
> + * @client_id: client id
> + * @src_id: source id
> + * @source: irq source pointer
>    *
> + * Registers irq source on a client.
> + * Returns 0 on success or error code otherwise.
>    */
>   int amdgpu_irq_add_id(struct amdgpu_device *adev,
>   		      unsigned client_id, unsigned src_id,
> @@ -315,9 +331,9 @@ int amdgpu_irq_add_id(struct amdgpu_device *adev,
>    * amdgpu_irq_dispatch - dispatch irq to IP blocks
>    *
>    * @adev: amdgpu device pointer
> - * @entry: interrupt vector
> + * @entry: interrupt vector pointer
>    *
> - * Dispatches the irq to the different IP blocks
> + * Dispatches irq to IP blocks.
>    */
>   void amdgpu_irq_dispatch(struct amdgpu_device *adev,
>   			 struct amdgpu_iv_entry *entry)
> @@ -364,10 +380,10 @@ void amdgpu_irq_dispatch(struct amdgpu_device *adev,
>    * amdgpu_irq_update - update hw interrupt state
>    *
>    * @adev: amdgpu device pointer
> - * @src: interrupt src you want to enable
> - * @type: type of interrupt you want to update
> + * @src: interrupt source pointer
> + * @type: type of interrupt
>    *
> - * Updates the interrupt state for a specific src (all asics).
> + * Updates interrupt state for the specific source (all asics).
>    */
>   int amdgpu_irq_update(struct amdgpu_device *adev,
>   			     struct amdgpu_irq_src *src, unsigned type)
> @@ -390,6 +406,14 @@ int amdgpu_irq_update(struct amdgpu_device *adev,
>   	return r;
>   }
>   
> +/**
> + * amdgpu_irq_gpu_reset_resume_helper - update interrupt states on all sources
> + *
> + * @adev: amdgpu device pointer
> + *
> + * Updates state of all types of interrupts on all sources on resume after
> + * reset.
> + */
>   void amdgpu_irq_gpu_reset_resume_helper(struct amdgpu_device *adev)
>   {
>   	int i, j, k;
> @@ -413,10 +437,11 @@ void amdgpu_irq_gpu_reset_resume_helper(struct amdgpu_device *adev)
>    * amdgpu_irq_get - enable interrupt
>    *
>    * @adev: amdgpu device pointer
> - * @src: interrupt src you want to enable
> - * @type: type of interrupt you want to enable
> + * @src: interrupt source pointer
> + * @type: type of interrupt
>    *
> - * Enables the interrupt type for a specific src (all asics).
> + * Enables specified type of interrupt on the specified source (all asics).
> + * Returns 0 on success or error code otherwise.
>    */
>   int amdgpu_irq_get(struct amdgpu_device *adev, struct amdgpu_irq_src *src,
>   		   unsigned type)
> @@ -440,10 +465,11 @@ int amdgpu_irq_get(struct amdgpu_device *adev, struct amdgpu_irq_src *src,
>    * amdgpu_irq_put - disable interrupt
>    *
>    * @adev: amdgpu device pointer
> - * @src: interrupt src you want to disable
> - * @type: type of interrupt you want to disable
> + * @src: interrupt source pointer
> + * @type: type of interrupt
>    *
> - * Disables the interrupt type for a specific src (all asics).
> + * Enables specified type of interrupt on the specified source (all asics).
> + * Returns 0 on success or error code otherwise.
>    */
>   int amdgpu_irq_put(struct amdgpu_device *adev, struct amdgpu_irq_src *src,
>   		   unsigned type)
> @@ -467,9 +493,11 @@ int amdgpu_irq_put(struct amdgpu_device *adev, struct amdgpu_irq_src *src,
>    * amdgpu_irq_enabled - test if irq is enabled or not
>    *
>    * @adev: amdgpu device pointer
> - * @idx: interrupt src you want to test
> + * @src: interrupt source pointer
> + * @type: type of interrupt
>    *
> - * Tests if the given interrupt source is enabled or not
> + * Tests whether the given type of interrupt is enabled on the given source.
> + * Returns true is enabled or false otherwise.
>    */
>   bool amdgpu_irq_enabled(struct amdgpu_device *adev, struct amdgpu_irq_src *src,
>   			unsigned type)
> @@ -486,7 +514,7 @@ bool amdgpu_irq_enabled(struct amdgpu_device *adev, struct amdgpu_irq_src *src,
>   	return !!atomic_read(&src->enabled_types[type]);
>   }
>   
> -/* gen irq */
> +/* XXX: generic irq handling */
>   static void amdgpu_irq_mask(struct irq_data *irqd)
>   {
>   	/* XXX */
> @@ -497,12 +525,23 @@ static void amdgpu_irq_unmask(struct irq_data *irqd)
>   	/* XXX */
>   }
>   
> +/* amdgpu hardware interrupt chip descriptor */
>   static struct irq_chip amdgpu_irq_chip = {
>   	.name = "amdgpu-ih",
>   	.irq_mask = amdgpu_irq_mask,
>   	.irq_unmask = amdgpu_irq_unmask,
>   };
>   
> +/**
> + * amdgpu_irqdomain_map - create mapping between virtual and hw irq numbers
> + *
> + * @d: amdgpu irq domain pointer (**not used**)
> + * @irq: virtual irq number
> + * @hwirq: hw irq number
> + *
> + * Current implementation assigns simple interrupt handler to the given virtual
> + * irq. **This implementation seems to be either obsolete or not finished**.
> + */
>   static int amdgpu_irqdomain_map(struct irq_domain *d,
>   				unsigned int irq, irq_hw_number_t hwirq)
>   {
> @@ -514,6 +553,7 @@ static int amdgpu_irqdomain_map(struct irq_domain *d,
>   	return 0;
>   }
>   
> +/* Implementation of methods for amdgpu irq domain */
>   static const struct irq_domain_ops amdgpu_hw_irqdomain_ops = {
>   	.map = amdgpu_irqdomain_map,
>   };



More information about the amd-gfx mailing list