Re: [PATCH 1/2] drm/amdgpu: cleanup amdgpu_ih_process a bit more

2019-01-31 Thread Kuehling, Felix 
On 2019-01-24 7:52 a.m., Christian König wrote:
> Remove the callback and call the dispatcher directly.
>
> Signed-off-by: Christian König 

This patch is Reviewed-by: Felix Kuehling 


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c  |  6 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h  |  4 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 48 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h |  2 +-
>   4 files changed, 21 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
> index d0a5db777b6d..1c50be3ab8a9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
> @@ -140,9 +140,7 @@ void amdgpu_ih_ring_fini(struct amdgpu_device *adev, 
> struct amdgpu_ih_ring *ih)
>* Interrupt hander (VI), walk the IH ring.
>* Returns irq process return code.
>*/
> -int amdgpu_ih_process(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih,
> -   void (*callback)(struct amdgpu_device *adev,
> -struct amdgpu_ih_ring *ih))
> +int amdgpu_ih_process(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih)
>   {
>   u32 wptr;
>   
> @@ -162,7 +160,7 @@ int amdgpu_ih_process(struct amdgpu_device *adev, struct 
> amdgpu_ih_ring *ih,
>   rmb();
>   
>   while (ih->rptr != wptr) {
> - callback(adev, ih);
> + amdgpu_irq_dispatch(adev, ih);
>   ih->rptr &= ih->ptr_mask;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
> index 1ccb1831382a..113a1ba13d4a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
> @@ -69,8 +69,6 @@ struct amdgpu_ih_funcs {
>   int amdgpu_ih_ring_init(struct amdgpu_device *adev, struct amdgpu_ih_ring 
> *ih,
>   unsigned ring_size, bool use_bus_addr);
>   void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct amdgpu_ih_ring 
> *ih);
> -int amdgpu_ih_process(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih,
> -   void (*callback)(struct amdgpu_device *adev,
> -struct amdgpu_ih_ring *ih));
> +int amdgpu_ih_process(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih);
>   
>   #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> index 8bfb3dab46f7..af4c3b1af322 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> @@ -130,29 +130,6 @@ void amdgpu_irq_disable_all(struct amdgpu_device *adev)
>   spin_unlock_irqrestore(>irq.lock, irqflags);
>   }
>   
> -/**
> - * amdgpu_irq_callback - callback from the IH ring
> - *
> - * @adev: amdgpu device pointer
> - * @ih: amdgpu ih ring
> - *
> - * Callback from IH ring processing to handle the entry at the current 
> position
> - * and advance the read pointer.
> - */
> -static void amdgpu_irq_callback(struct amdgpu_device *adev,
> - struct amdgpu_ih_ring *ih)
> -{
> - u32 ring_index = ih->rptr >> 2;
> - struct amdgpu_iv_entry entry;
> -
> - entry.iv_entry = (const uint32_t *)>ring[ring_index];
> - amdgpu_ih_decode_iv(adev, );
> -
> - trace_amdgpu_iv(ih - >irq.ih, );
> -
> - amdgpu_irq_dispatch(adev, );
> -}
> -
>   /**
>* amdgpu_irq_handler - IRQ handler
>*
> @@ -170,7 +147,7 @@ irqreturn_t amdgpu_irq_handler(int irq, void *arg)
>   struct amdgpu_device *adev = dev->dev_private;
>   irqreturn_t ret;
>   
> - ret = amdgpu_ih_process(adev, >irq.ih, amdgpu_irq_callback);
> + ret = amdgpu_ih_process(adev, >irq.ih);
>   if (ret == IRQ_HANDLED)
>   pm_runtime_mark_last_busy(dev->dev);
>   return ret;
> @@ -188,7 +165,7 @@ static void amdgpu_irq_handle_ih1(struct work_struct 
> *work)
>   struct amdgpu_device *adev = container_of(work, struct amdgpu_device,
> irq.ih1_work);
>   
> - amdgpu_ih_process(adev, >irq.ih1, amdgpu_irq_callback);
> + amdgpu_ih_process(adev, >irq.ih1);
>   }
>   
>   /**
> @@ -203,7 +180,7 @@ static void amdgpu_irq_handle_ih2(struct work_struct 
> *work)
>   struct amdgpu_device *adev = container_of(work, struct amdgpu_device,
> irq.ih2_work);
>   
> - amdgpu_ih_process(adev, >irq.ih2, amdgpu_irq_callback);
> + amdgpu_ih_process(adev, >irq.ih2);
>   }
>   
>   /**
> @@ -394,14 +371,23 @@ int amdgpu_irq_add_id(struct amdgpu_device *adev,
>* Dispatches IRQ to IP blocks.
>*/
>   void amdgpu_irq_dispatch(struct amdgpu_device *adev,
> -  struct amdgpu_iv_entry *entry)
> +  struct amdgpu_ih_ring *ih)
>   {
> - unsigned client_id = entry->client_id;
> - unsigned src_id = entry->src_id;
> + u32 ring_index = ih->rptr >> 2;
> + struct

[PATCH 1/2] drm/amdgpu: cleanup amdgpu_ih_process a bit more

2019-01-24 Thread Christian König 
Remove the callback and call the dispatcher directly.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c  |  6 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h  |  4 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 48 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h |  2 +-
 4 files changed, 21 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
index d0a5db777b6d..1c50be3ab8a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
@@ -140,9 +140,7 @@ void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct 
amdgpu_ih_ring *ih)
  * Interrupt hander (VI), walk the IH ring.
  * Returns irq process return code.
  */
-int amdgpu_ih_process(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih,
- void (*callback)(struct amdgpu_device *adev,
-  struct amdgpu_ih_ring *ih))
+int amdgpu_ih_process(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih)
 {
u32 wptr;
 
@@ -162,7 +160,7 @@ int amdgpu_ih_process(struct amdgpu_device *adev, struct 
amdgpu_ih_ring *ih,
rmb();
 
while (ih->rptr != wptr) {
-   callback(adev, ih);
+   amdgpu_irq_dispatch(adev, ih);
ih->rptr &= ih->ptr_mask;
}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
index 1ccb1831382a..113a1ba13d4a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
@@ -69,8 +69,6 @@ struct amdgpu_ih_funcs {
 int amdgpu_ih_ring_init(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih,
unsigned ring_size, bool use_bus_addr);
 void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct amdgpu_ih_ring 
*ih);
-int amdgpu_ih_process(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih,
- void (*callback)(struct amdgpu_device *adev,
-  struct amdgpu_ih_ring *ih));
+int amdgpu_ih_process(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih);
 
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 8bfb3dab46f7..af4c3b1af322 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -130,29 +130,6 @@ void amdgpu_irq_disable_all(struct amdgpu_device *adev)
spin_unlock_irqrestore(>irq.lock, irqflags);
 }
 
-/**
- * amdgpu_irq_callback - callback from the IH ring
- *
- * @adev: amdgpu device pointer
- * @ih: amdgpu ih ring
- *
- * Callback from IH ring processing to handle the entry at the current position
- * and advance the read pointer.
- */
-static void amdgpu_irq_callback(struct amdgpu_device *adev,
-   struct amdgpu_ih_ring *ih)
-{
-   u32 ring_index = ih->rptr >> 2;
-   struct amdgpu_iv_entry entry;
-
-   entry.iv_entry = (const uint32_t *)>ring[ring_index];
-   amdgpu_ih_decode_iv(adev, );
-
-   trace_amdgpu_iv(ih - >irq.ih, );
-
-   amdgpu_irq_dispatch(adev, );
-}
-
 /**
  * amdgpu_irq_handler - IRQ handler
  *
@@ -170,7 +147,7 @@ irqreturn_t amdgpu_irq_handler(int irq, void *arg)
struct amdgpu_device *adev = dev->dev_private;
irqreturn_t ret;
 
-   ret = amdgpu_ih_process(adev, >irq.ih, amdgpu_irq_callback);
+   ret = amdgpu_ih_process(adev, >irq.ih);
if (ret == IRQ_HANDLED)
pm_runtime_mark_last_busy(dev->dev);
return ret;
@@ -188,7 +165,7 @@ static void amdgpu_irq_handle_ih1(struct work_struct *work)
struct amdgpu_device *adev = container_of(work, struct amdgpu_device,
  irq.ih1_work);
 
-   amdgpu_ih_process(adev, >irq.ih1, amdgpu_irq_callback);
+   amdgpu_ih_process(adev, >irq.ih1);
 }
 
 /**
@@ -203,7 +180,7 @@ static void amdgpu_irq_handle_ih2(struct work_struct *work)
struct amdgpu_device *adev = container_of(work, struct amdgpu_device,
  irq.ih2_work);
 
-   amdgpu_ih_process(adev, >irq.ih2, amdgpu_irq_callback);
+   amdgpu_ih_process(adev, >irq.ih2);
 }
 
 /**
@@ -394,14 +371,23 @@ int amdgpu_irq_add_id(struct amdgpu_device *adev,
  * Dispatches IRQ to IP blocks.
  */
 void amdgpu_irq_dispatch(struct amdgpu_device *adev,
-struct amdgpu_iv_entry *entry)
+struct amdgpu_ih_ring *ih)
 {
-   unsigned client_id = entry->client_id;
-   unsigned src_id = entry->src_id;
+   u32 ring_index = ih->rptr >> 2;
+   struct amdgpu_iv_entry entry;
+   unsigned client_id, src_id;
struct amdgpu_irq_src *src;
bool handled = false;
int r;
 
+   entry.iv_entry = (const uint32_t *)>ring[ring_index];
+   amdgpu_ih_decode_iv(adev, );
+
+   trace_amdgpu_iv(ih - >irq.ih, );
+
+   client_id =