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,
> };
