On 2021-06-17 15:20, Dmitry Baryshkov wrote:

With dpu_core_irq being the wrapper around dpu_hw_interrupts, there is little sense in having them separate. Squash them together to remove another layer of abstraction (hw_intr ops).

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

Overall, I think this is a reasonable cleanup, Reviewed-by: Abhinav Kumar abhinavk@codeaurora.org

---

drivers/gpu/drm/msm/Makefile | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c | 256 ----------------- .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 269 ++++++++++++++---- .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 87 ------ 4 files changed, 214 insertions(+), 399 deletions(-) delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index 2c00aa70b708..a5245e8d0f14 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -51,7 +51,6 @@ msm-y := \ disp/mdp5/mdp5_mixer.o \ disp/mdp5/mdp5_plane.o \ disp/mdp5/mdp5_smp.o \

• disp/dpu1/dpu_core_irq.o \ disp/dpu1/dpu_core_perf.o \ disp/dpu1/dpu_crtc.o \ disp/dpu1/dpu_encoder.o \

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c deleted file mode 100644 index d2457490930b..000000000000 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c +++ /dev/null @@ -1,256 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.

• */

-#define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__

-#include <linux/debugfs.h> -#include <linux/irqdomain.h> -#include <linux/irq.h> -#include <linux/kthread.h>

-#include "dpu_core_irq.h" -#include "dpu_trace.h"

-/**

• • dpu_core_irq_callback_handler - dispatch core interrupts
• • @arg: private data of callback handler
• • @irq_idx: interrupt index
• */

-static void dpu_core_irq_callback_handler(void *arg, int irq_idx) -{

• struct dpu_kms *dpu_kms = arg;
• struct dpu_irq *irq_obj = &dpu_kms->irq_obj;
• struct dpu_irq_callback *cb;
• VERB("irq_idx=%d\n", irq_idx);
• if (list_empty(&irq_obj->irq_cb_tbl[irq_idx]))

• DRM_ERROR("no registered cb, idx:%d\n", irq_idx);
  

• atomic_inc(&irq_obj->irq_counts[irq_idx]);
• /*

• * Perform registered function callback
  

• */
  

• list_for_each_entry(cb, &irq_obj->irq_cb_tbl[irq_idx], list)

• if (cb->func)
  

• 	cb->func(cb->arg, irq_idx);
  

-}

-u32 dpu_core_irq_read(struct dpu_kms *dpu_kms, int irq_idx, bool clear) -{

• if (!dpu_kms->hw_intr ||

• 	!dpu_kms->hw_intr->ops.get_interrupt_status)
  

• return 0;
  

• if (irq_idx < 0) {

• DPU_ERROR("[%pS] invalid irq_idx=%d\n",
  

• 		__builtin_return_address(0), irq_idx);
  

• return 0;
  

• }
• return dpu_kms->hw_intr->ops.get_interrupt_status(dpu_kms->hw_intr,

• 	irq_idx, clear);
  

-}

-int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx,

• struct dpu_irq_callback *register_irq_cb)
  

-{

• unsigned long irq_flags;
• if (!dpu_kms->irq_obj.irq_cb_tbl) {

• DPU_ERROR("invalid params\n");
  

• return -EINVAL;
  

• }
• if (!register_irq_cb || !register_irq_cb->func) {

• DPU_ERROR("invalid irq_cb:%d func:%d\n",
  

• 		register_irq_cb != NULL,
  

• 		register_irq_cb ?
  

• 			register_irq_cb->func != NULL : -1);
  

• return -EINVAL;
  

• }
• if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->total_irqs) {

• DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx);
  

• return -EINVAL;
  

• }
• VERB("[%pS] irq_idx=%d\n", __builtin_return_address(0), irq_idx);
• irq_flags = dpu_kms->hw_intr->ops.lock(dpu_kms->hw_intr);
• trace_dpu_core_irq_register_callback(irq_idx, register_irq_cb);
• list_del_init(&register_irq_cb->list);
• list_add_tail(&register_irq_cb->list,

• 	&dpu_kms->irq_obj.irq_cb_tbl[irq_idx]);
  

• if (list_is_first(&register_irq_cb->list,

• 	&dpu_kms->irq_obj.irq_cb_tbl[irq_idx])) {
  

• int ret = dpu_kms->hw_intr->ops.enable_irq_locked(
  

• 		dpu_kms->hw_intr,
  

• 		irq_idx);
  

• if (ret)
  

• 	DPU_ERROR("Fail to enable IRQ for irq_idx:%d\n",
  

• 			irq_idx);
  

• }
• dpu_kms->hw_intr->ops.unlock(dpu_kms->hw_intr, irq_flags);
• return 0;

-}

-int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx,

• struct dpu_irq_callback *register_irq_cb)
  

-{

• unsigned long irq_flags;
• if (!dpu_kms->irq_obj.irq_cb_tbl) {

• DPU_ERROR("invalid params\n");
  

• return -EINVAL;
  

• }
• if (!register_irq_cb || !register_irq_cb->func) {

• DPU_ERROR("invalid irq_cb:%d func:%d\n",
  

• 		register_irq_cb != NULL,
  

• 		register_irq_cb ?
  

• 			register_irq_cb->func != NULL : -1);
  

• return -EINVAL;
  

• }
• if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->total_irqs) {

• DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx);
  

• return -EINVAL;
  

• }
• VERB("[%pS] irq_idx=%d\n", __builtin_return_address(0), irq_idx);
• irq_flags = dpu_kms->hw_intr->ops.lock(dpu_kms->hw_intr);
• trace_dpu_core_irq_unregister_callback(irq_idx, register_irq_cb);
• list_del_init(&register_irq_cb->list);
• /* empty callback list but interrupt is still enabled */
• if (list_empty(&dpu_kms->irq_obj.irq_cb_tbl[irq_idx])) {

• int ret = dpu_kms->hw_intr->ops.disable_irq_locked(
  

• 		dpu_kms->hw_intr,
  

• 		irq_idx);
  

• if (ret)
  

• 	DPU_ERROR("Fail to disable IRQ for irq_idx:%d\n",
  

• 			irq_idx);
  

• VERB("irq_idx=%d ret=%d\n", irq_idx, ret);
  

• }
• dpu_kms->hw_intr->ops.unlock(dpu_kms->hw_intr, irq_flags);
• return 0;

-}

-static void dpu_clear_all_irqs(struct dpu_kms *dpu_kms) -{

• if (!dpu_kms->hw_intr || !dpu_kms->hw_intr->ops.clear_all_irqs)

• return;
  

• dpu_kms->hw_intr->ops.clear_all_irqs(dpu_kms->hw_intr);

-}

-static void dpu_disable_all_irqs(struct dpu_kms *dpu_kms) -{

• if (!dpu_kms->hw_intr || !dpu_kms->hw_intr->ops.disable_all_irqs)

• return;
  

• dpu_kms->hw_intr->ops.disable_all_irqs(dpu_kms->hw_intr);

-}

-#ifdef CONFIG_DEBUG_FS -static int dpu_debugfs_core_irq_show(struct seq_file *s, void *v) -{

• struct dpu_kms *dpu_kms = s->private;
• struct dpu_irq *irq_obj = &dpu_kms->irq_obj;
• struct dpu_irq_callback *cb;
• unsigned long irq_flags;
• int i, irq_count, cb_count;
• if (WARN_ON(!irq_obj->irq_cb_tbl))

• return 0;
  

• for (i = 0; i < irq_obj->total_irqs; i++) {

• irq_flags = dpu_kms->hw_intr->ops.lock(dpu_kms->hw_intr);
  

• cb_count = 0;
  

• irq_count = atomic_read(&irq_obj->irq_counts[i]);
  

• list_for_each_entry(cb, &irq_obj->irq_cb_tbl[i], list)
  

• 	cb_count++;
  

• dpu_kms->hw_intr->ops.unlock(dpu_kms->hw_intr, irq_flags);
  

• if (irq_count || cb_count)
  

• 	seq_printf(s, "idx:%d irq:%d cb:%d\n",
  

• 			i, irq_count, cb_count);
  

• }
• return 0;

-}

-DEFINE_SHOW_ATTRIBUTE(dpu_debugfs_core_irq);

-void dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms,

• struct dentry *parent)
  

-{

• debugfs_create_file("core_irq", 0600, parent, dpu_kms,

• &dpu_debugfs_core_irq_fops);
  

-} -#endif

-void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms) -{

• int i;
• pm_runtime_get_sync(&dpu_kms->pdev->dev);
• dpu_clear_all_irqs(dpu_kms);
• dpu_disable_all_irqs(dpu_kms);
• pm_runtime_put_sync(&dpu_kms->pdev->dev);
• /* Create irq callbacks for all possible irq_idx */
• dpu_kms->irq_obj.total_irqs = dpu_kms->hw_intr->total_irqs;
• dpu_kms->irq_obj.irq_cb_tbl = kcalloc(dpu_kms->irq_obj.total_irqs,

• 	sizeof(struct list_head), GFP_KERNEL);
  

• dpu_kms->irq_obj.irq_counts = kcalloc(dpu_kms->irq_obj.total_irqs,

• 	sizeof(atomic_t), GFP_KERNEL);
  

• for (i = 0; i < dpu_kms->irq_obj.total_irqs; i++) {

• INIT_LIST_HEAD(&dpu_kms->irq_obj.irq_cb_tbl[i]);
  

• atomic_set(&dpu_kms->irq_obj.irq_counts[i], 0);
  

• }

-}

-void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms) -{

• int i;
• pm_runtime_get_sync(&dpu_kms->pdev->dev);
• for (i = 0; i < dpu_kms->irq_obj.total_irqs; i++)

• if (!list_empty(&dpu_kms->irq_obj.irq_cb_tbl[i]))
  

• 	DPU_ERROR("irq_idx=%d still enabled/registered\n", i);
  

• dpu_clear_all_irqs(dpu_kms);
• dpu_disable_all_irqs(dpu_kms);
• pm_runtime_put_sync(&dpu_kms->pdev->dev);
• kfree(dpu_kms->irq_obj.irq_cb_tbl);
• kfree(dpu_kms->irq_obj.irq_counts);
• dpu_kms->irq_obj.irq_cb_tbl = NULL;
• dpu_kms->irq_obj.irq_counts = NULL;
• dpu_kms->irq_obj.total_irqs = 0;

-}

-irqreturn_t dpu_core_irq(struct dpu_kms *dpu_kms) -{

• /*

• * Dispatch to HW driver to handle interrupt lookup that is being
  

• * fired. When matching interrupt is located, HW driver will call to
  

• * dpu_core_irq_callback_handler with the irq_idx from the lookup 
  

table.

• * dpu_core_irq_callback_handler will perform the registered function
  

• * callback, and do the interrupt status clearing once the registered
  

• * callback is finished.
  

• * Function will also clear the interrupt status after reading.
  

• */
  

• dpu_kms->hw_intr->ops.dispatch_irqs(

• 	dpu_kms->hw_intr,
  

• 	dpu_core_irq_callback_handler,
  

• 	dpu_kms);
  

• return IRQ_HANDLED;

-} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index 2e816f232e85..2437b0c7c073 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -3,12 +3,15 @@ */

#include <linux/bitops.h> +#include <linux/debugfs.h> #include <linux/slab.h>

+#include "dpu_core_irq.h" #include "dpu_kms.h" #include "dpu_hw_interrupts.h" #include "dpu_hw_util.h" #include "dpu_hw_mdss.h" +#include "dpu_trace.h"

/**

• Register offsets in MDSS register file for the interrupt registers

@@ -132,10 +135,34 @@ static void dpu_hw_intr_clear_intr_status_nolock(struct dpu_hw_intr *intr, wmb(); }

-static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr *intr,

• void (*cbfunc)(void *, int),
  

• void *arg)
  

+/**

• • dpu_core_irq_callback_handler - dispatch core interrupts
• • @arg: private data of callback handler
• • @irq_idx: interrupt index
• */

+static void dpu_core_irq_callback_handler(struct dpu_kms *dpu_kms, int irq_idx) {

• struct dpu_irq *irq_obj = &dpu_kms->irq_obj;
• struct dpu_irq_callback *cb;
• VERB("irq_idx=%d\n", irq_idx);
• if (list_empty(&irq_obj->irq_cb_tbl[irq_idx]))

• DRM_ERROR("no registered cb, idx:%d\n", irq_idx);
  

• atomic_inc(&irq_obj->irq_counts[irq_idx]);
• /*

• * Perform registered function callback
  

• */
  

• list_for_each_entry(cb, &irq_obj->irq_cb_tbl[irq_idx], list)

• if (cb->func)
  

• 	cb->func(cb->arg, irq_idx);
  

+}

+irqreturn_t dpu_core_irq(struct dpu_kms *dpu_kms) +{

• struct dpu_hw_intr *intr = dpu_kms->hw_intr; int reg_idx; int irq_idx; u32 irq_status;

@@ -144,13 +171,8 @@ static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr *intr, unsigned long irq_flags;

if (!intr)

• return;
  

• return IRQ_NONE;
  
  

• /*

• * The dispatcher will save the IRQ status before calling here.
  

• * Now need to go through each IRQ status and find matching
  

• * irq lookup index.
  

• */
  

  spin_lock_irqsave(&intr->irq_lock, irq_flags); for (reg_idx = 0; reg_idx < ARRAY_SIZE(dpu_intr_set); reg_idx++) { if (!test_bit(reg_idx, &intr->irq_mask))

@@ -178,15 +200,8 @@ static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr *intr, */ while ((bit = ffs(irq_status)) != 0) { irq_idx = DPU_IRQ_IDX(reg_idx, bit - 1);

• 	/*
  

• 	 * Once a match on irq mask, perform a callback
  

• 	 * to the given cbfunc. cbfunc will take care
  

• 	 * the interrupt status clearing. If cbfunc is
  

• 	 * not provided, then the interrupt clearing
  

• 	 * is here.
  

• 	 */
  

• 	if (cbfunc)
  

• 		cbfunc(arg, irq_idx);
  

• 	dpu_core_irq_callback_handler(dpu_kms, irq_idx);
  
  dpu_hw_intr_clear_intr_status_nolock(intr, irq_idx);
  
  

@@ -203,6 +218,8 @@ static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr *intr, wmb();

spin_unlock_irqrestore(&intr->irq_lock, irq_flags);

• return IRQ_HANDLED;

}

static int dpu_hw_intr_enable_irq_locked(struct dpu_hw_intr *intr, int irq_idx) @@ -303,12 +320,13 @@ static int dpu_hw_intr_disable_irq_locked(struct dpu_hw_intr *intr, int irq_idx) return 0; }

-static int dpu_hw_intr_clear_irqs(struct dpu_hw_intr *intr) +static void dpu_clear_irqs(struct dpu_kms *dpu_kms) {

• struct dpu_hw_intr *intr = dpu_kms->hw_intr; int i;

  if (!intr)

• return -EINVAL;
  

• return;
  
  

  for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) { if (test_bit(i, &intr->irq_mask))

@@ -318,16 +336,15 @@ static int dpu_hw_intr_clear_irqs(struct dpu_hw_intr *intr)

/* ensure register writes go through */ wmb();

• return 0;

}

-static int dpu_hw_intr_disable_irqs(struct dpu_hw_intr *intr) +static void dpu_disable_all_irqs(struct dpu_kms *dpu_kms) {

• struct dpu_hw_intr *intr = dpu_kms->hw_intr; int i;

  if (!intr)

• return -EINVAL;
  

• return;
  
  

  for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) { if (test_bit(i, &intr->irq_mask))

@@ -337,13 +354,11 @@ static int dpu_hw_intr_disable_irqs(struct dpu_hw_intr *intr)

/* ensure register writes go through */ wmb();

• return 0;

}

-static u32 dpu_hw_intr_get_interrupt_status(struct dpu_hw_intr *intr,

• int irq_idx, bool clear)
  

+u32 dpu_core_irq_read(struct dpu_kms *dpu_kms, int irq_idx, bool clear) {

• struct dpu_hw_intr *intr = dpu_kms->hw_intr; int reg_idx; unsigned long irq_flags; u32 intr_status;

@@ -351,6 +366,12 @@ static u32 dpu_hw_intr_get_interrupt_status(struct dpu_hw_intr *intr, if (!intr) return 0;

• if (irq_idx < 0) {

• DPU_ERROR("[%pS] invalid irq_idx=%d\n",
  

• 		__builtin_return_address(0), irq_idx);
  

• return 0;
  

• }
• if (irq_idx < 0 || irq_idx >= intr->total_irqs) { pr_err("invalid IRQ index: [%d]\n", irq_idx); return 0;

@@ -374,32 +395,6 @@ static u32 dpu_hw_intr_get_interrupt_status(struct dpu_hw_intr *intr, return intr_status; }

-static unsigned long dpu_hw_intr_lock(struct dpu_hw_intr *intr) -{

• unsigned long irq_flags;
• spin_lock_irqsave(&intr->irq_lock, irq_flags);
• return irq_flags;

-}

-static void dpu_hw_intr_unlock(struct dpu_hw_intr *intr, unsigned long irq_flags) -{

• spin_unlock_irqrestore(&intr->irq_lock, irq_flags);

-}

-static void __setup_intr_ops(struct dpu_hw_intr_ops *ops) -{

• ops->enable_irq_locked = dpu_hw_intr_enable_irq_locked;
• ops->disable_irq_locked = dpu_hw_intr_disable_irq_locked;
• ops->dispatch_irqs = dpu_hw_intr_dispatch_irq;
• ops->clear_all_irqs = dpu_hw_intr_clear_irqs;
• ops->disable_all_irqs = dpu_hw_intr_disable_irqs;
• ops->get_interrupt_status = dpu_hw_intr_get_interrupt_status;
• ops->lock = dpu_hw_intr_lock;
• ops->unlock = dpu_hw_intr_unlock;

-}

static void __intr_offset(struct dpu_mdss_cfg *m, void __iomem *addr, struct dpu_hw_blk_reg_map *hw) { @@ -421,7 +416,6 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr, return ERR_PTR(-ENOMEM);

__intr_offset(m, addr, &intr->hw);

• __setup_intr_ops(&intr->ops);

  intr->total_irqs = ARRAY_SIZE(dpu_intr_set) * 32;

@@ -447,3 +441,168 @@ void dpu_hw_intr_destroy(struct dpu_hw_intr *intr) } }

+int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx,

• struct dpu_irq_callback *register_irq_cb)
  

+{

• unsigned long irq_flags;
• if (!dpu_kms->irq_obj.irq_cb_tbl) {

• DPU_ERROR("invalid params\n");
  

• return -EINVAL;
  

• }
• if (!register_irq_cb || !register_irq_cb->func) {

• DPU_ERROR("invalid irq_cb:%d func:%d\n",
  

• 		register_irq_cb != NULL,
  

• 		register_irq_cb ?
  

• 			register_irq_cb->func != NULL : -1);
  

• return -EINVAL;
  

• }
• if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->total_irqs) {

• DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx);
  

• return -EINVAL;
  

• }
• VERB("[%pS] irq_idx=%d\n", __builtin_return_address(0), irq_idx);
• spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags);
• trace_dpu_core_irq_register_callback(irq_idx, register_irq_cb);
• list_del_init(&register_irq_cb->list);
• list_add_tail(&register_irq_cb->list,

• 	&dpu_kms->irq_obj.irq_cb_tbl[irq_idx]);
  

• if (list_is_first(&register_irq_cb->list,

• 	&dpu_kms->irq_obj.irq_cb_tbl[irq_idx])) {
  

• int ret = dpu_hw_intr_enable_irq_locked(
  

• 		dpu_kms->hw_intr,
  

• 		irq_idx);
  

• if (ret)
  

• 	DPU_ERROR("Fail to enable IRQ for irq_idx:%d\n",
  

• 			irq_idx);
  

• }
• spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
• return 0;

+}

+int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx,

• struct dpu_irq_callback *register_irq_cb)
  

+{

• unsigned long irq_flags;
• if (!dpu_kms->irq_obj.irq_cb_tbl) {

• DPU_ERROR("invalid params\n");
  

• return -EINVAL;
  

• }
• if (!register_irq_cb || !register_irq_cb->func) {

• DPU_ERROR("invalid irq_cb:%d func:%d\n",
  

• 		register_irq_cb != NULL,
  

• 		register_irq_cb ?
  

• 			register_irq_cb->func != NULL : -1);
  

• return -EINVAL;
  

• }
• if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->total_irqs) {

• DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx);
  

• return -EINVAL;
  

• }
• VERB("[%pS] irq_idx=%d\n", __builtin_return_address(0), irq_idx);
• spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags);
• trace_dpu_core_irq_unregister_callback(irq_idx, register_irq_cb);
• list_del_init(&register_irq_cb->list);
• /* empty callback list but interrupt is still enabled */
• if (list_empty(&dpu_kms->irq_obj.irq_cb_tbl[irq_idx])) {

• int ret = dpu_hw_intr_disable_irq_locked(
  

• 		dpu_kms->hw_intr,
  

• 		irq_idx);
  

• if (ret)
  

• 	DPU_ERROR("Fail to disable IRQ for irq_idx:%d\n",
  

• 			irq_idx);
  

• VERB("irq_idx=%d ret=%d\n", irq_idx, ret);
  

• }
• spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
• return 0;

+}

+#ifdef CONFIG_DEBUG_FS +static int dpu_debugfs_core_irq_show(struct seq_file *s, void *v) +{

• struct dpu_kms *dpu_kms = s->private;
• struct dpu_irq *irq_obj = &dpu_kms->irq_obj;
• struct dpu_irq_callback *cb;
• unsigned long irq_flags;
• int i, irq_count, cb_count;
• if (WARN_ON(!irq_obj->irq_cb_tbl))

• return 0;
  

• for (i = 0; i < irq_obj->total_irqs; i++) {

• spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags);
  

• cb_count = 0;
  

• irq_count = atomic_read(&irq_obj->irq_counts[i]);
  

• list_for_each_entry(cb, &irq_obj->irq_cb_tbl[i], list)
  

• 	cb_count++;
  

• spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
  

• if (irq_count || cb_count)
  

• 	seq_printf(s, "idx:%d irq:%d cb:%d\n",
  

• 			i, irq_count, cb_count);
  

• }
• return 0;

+}

+DEFINE_SHOW_ATTRIBUTE(dpu_debugfs_core_irq);

+void dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms,

• struct dentry *parent)
  

+{

• debugfs_create_file("core_irq", 0600, parent, dpu_kms,

• &dpu_debugfs_core_irq_fops);
  

+} +#endif

+void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms) +{

• int i;
• pm_runtime_get_sync(&dpu_kms->pdev->dev);
• dpu_clear_irqs(dpu_kms);
• dpu_disable_all_irqs(dpu_kms);
• pm_runtime_put_sync(&dpu_kms->pdev->dev);
• /* Create irq callbacks for all possible irq_idx */
• dpu_kms->irq_obj.total_irqs = dpu_kms->hw_intr->total_irqs;
• dpu_kms->irq_obj.irq_cb_tbl = kcalloc(dpu_kms->irq_obj.total_irqs,

• 	sizeof(struct list_head), GFP_KERNEL);
  

• dpu_kms->irq_obj.irq_counts = kcalloc(dpu_kms->irq_obj.total_irqs,

• 	sizeof(atomic_t), GFP_KERNEL);
  

• for (i = 0; i < dpu_kms->irq_obj.total_irqs; i++) {

• INIT_LIST_HEAD(&dpu_kms->irq_obj.irq_cb_tbl[i]);
  

• atomic_set(&dpu_kms->irq_obj.irq_counts[i], 0);
  

• }

+}

+void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms) +{

• int i;
• pm_runtime_get_sync(&dpu_kms->pdev->dev);
• for (i = 0; i < dpu_kms->irq_obj.total_irqs; i++)

• if (!list_empty(&dpu_kms->irq_obj.irq_cb_tbl[i]))
  

• 	DPU_ERROR("irq_idx=%d still enabled/registered\n", i);
  

• dpu_clear_irqs(dpu_kms);
• dpu_disable_all_irqs(dpu_kms);
• pm_runtime_put_sync(&dpu_kms->pdev->dev);
• kfree(dpu_kms->irq_obj.irq_cb_tbl);
• kfree(dpu_kms->irq_obj.irq_counts);
• dpu_kms->irq_obj.irq_cb_tbl = NULL;
• dpu_kms->irq_obj.irq_counts = NULL;
• dpu_kms->irq_obj.total_irqs = 0;

+} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h index ac83c1159815..99ab01f54aa6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h @@ -32,92 +32,6 @@ enum dpu_hw_intr_reg {

#define DPU_IRQ_IDX(reg_idx, offset) (reg_idx * 32 + offset)

-struct dpu_hw_intr;

-/**

• • Interrupt operations.
• */

-struct dpu_hw_intr_ops {

• /**

• * enable_irq - Enable IRQ based on lookup IRQ index
  

• * @intr:	HW interrupt handle
  

• * @irq_idx:	Lookup irq index return from irq_idx_lookup
  

• * @return:	0 for success, otherwise failure
  

• */
  

• int (*enable_irq_locked)(

• 	struct dpu_hw_intr *intr,
  

• 	int irq_idx);
  

• /**

• * disable_irq - Disable IRQ based on lookup IRQ index
  

• * @intr:	HW interrupt handle
  

• * @irq_idx:	Lookup irq index return from irq_idx_lookup
  

• * @return:	0 for success, otherwise failure
  

• */
  

• int (*disable_irq_locked)(

• 	struct dpu_hw_intr *intr,
  

• 	int irq_idx);
  

• /**

• * clear_all_irqs - Clears all the interrupts (i.e. acknowledges
  

• *                  any asserted IRQs). Useful during reset.
  

• * @intr:	HW interrupt handle
  

• * @return:	0 for success, otherwise failure
  

• */
  

• int (*clear_all_irqs)(

• 	struct dpu_hw_intr *intr);
  

• /**

• * disable_all_irqs - Disables all the interrupts. Useful during 
  

reset.

• * @intr:	HW interrupt handle
  

• * @return:	0 for success, otherwise failure
  

• */
  

• int (*disable_all_irqs)(

• 	struct dpu_hw_intr *intr);
  

• /**

• * dispatch_irqs - IRQ dispatcher will call the given callback
  

• *                 function when a matching interrupt status bit is
  

• *                 found in the irq mapping table.
  

• * @intr:	HW interrupt handle
  

• * @cbfunc:	Callback function pointer
  

• * @arg:	Argument to pass back during callback
  

• */
  

• void (*dispatch_irqs)(

• 	struct dpu_hw_intr *intr,
  

• 	void (*cbfunc)(void *arg, int irq_idx),
  

• 	void *arg);
  

• /**

• * get_interrupt_status - Gets HW interrupt status, and clear if set,
  

• *                        based on given lookup IRQ index.
  

• * @intr:	HW interrupt handle
  

• * @irq_idx:	Lookup irq index return from irq_idx_lookup
  

• * @clear:	True to clear irq after read
  

• */
  

• u32 (*get_interrupt_status)(

• 	struct dpu_hw_intr *intr,
  

• 	int irq_idx,
  

• 	bool clear);
  

• /**

• * lock - take the IRQ lock
  

• * @intr:	HW interrupt handle
  

• * @return:	irq_flags for the taken spinlock
  

• */
  

• unsigned long (*lock)(

• 	struct dpu_hw_intr *intr);
  

• /**

• * unlock - take the IRQ lock
  

• * @intr:	HW interrupt handle
  

• * @irq_flags:  the irq_flags returned from lock
  

• */
  

• void (*unlock)(

• 	struct dpu_hw_intr *intr, unsigned long irq_flags);
  

-};

/**

• struct dpu_hw_intr: hw interrupts handling data structure
• @hw: virtual address mapping

@@ -129,7 +43,6 @@ struct dpu_hw_intr_ops { */ struct dpu_hw_intr { struct dpu_hw_blk_reg_map hw;

• struct dpu_hw_intr_ops ops; u32 *cache_irq_mask; u32 *save_irq_status; u32 total_irqs;

On 2021-06-17 15:20, Dmitry Baryshkov wrote:

We already clear the IRQ status register before processing IRQs, so do not clear the register again. Especially do not clear the IRQ status _after_ processing the IRQ as this way we can loose the event.

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

Reviewed-by: Abhinav Kumar abhinavk@codeaurora.org

---

.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 17 ----------------- 1 file changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index 2437b0c7c073..28e9b0d448db 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -120,21 +120,6 @@ static const struct dpu_intr_reg dpu_intr_set[] = { #define DPU_IRQ_REG(irq_idx) (irq_idx / 32) #define DPU_IRQ_MASK(irq_idx) (BIT(irq_idx % 32))

-static void dpu_hw_intr_clear_intr_status_nolock(struct dpu_hw_intr *intr,

• int irq_idx)
  

-{

• int reg_idx;
• if (!intr)

• return;
  

• reg_idx = DPU_IRQ_REG(irq_idx);
• DPU_REG_WRITE(&intr->hw, dpu_intr_set[reg_idx].clr_off,

DPU_IRQ_MASK(irq_idx));

• /* ensure register writes go through */
• wmb();

-}

/**

• dpu_core_irq_callback_handler - dispatch core interrupts
• @arg: private data of callback handler

@@ -203,8 +188,6 @@ irqreturn_t dpu_core_irq(struct dpu_kms *dpu_kms)

	dpu_core_irq_callback_handler(dpu_kms, irq_idx);

• 	dpu_hw_intr_clear_intr_status_nolock(intr, irq_idx);
  

• /*
   * When callback finish, clear the irq_status
   * with the matching mask. Once irq_status
  

On 2021-06-17 15:20, Dmitry Baryshkov wrote:

As dpu_core_irq was merged into dpu_hw_intr, merge data structures too, removing the need for a separate data structure.

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

Reviewed-by: Abhinav Kumar abhinavk@codeaurora.org

---

.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 51 +++++++++---------- .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 5 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 13 ----- 3 files changed, 28 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index 28e9b0d448db..d2b6dca487e3 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -127,20 +127,19 @@ static const struct dpu_intr_reg dpu_intr_set[] = { */ static void dpu_core_irq_callback_handler(struct dpu_kms *dpu_kms, int irq_idx) {

• struct dpu_irq *irq_obj = &dpu_kms->irq_obj; struct dpu_irq_callback *cb;

  VERB("irq_idx=%d\n", irq_idx);

• if (list_empty(&irq_obj->irq_cb_tbl[irq_idx]))

• if (list_empty(&dpu_kms->hw_intr->irq_cb_tbl[irq_idx])) DRM_ERROR("no registered cb, idx:%d\n", irq_idx);

• atomic_inc(&irq_obj->irq_counts[irq_idx]);

• atomic_inc(&dpu_kms->hw_intr->irq_counts[irq_idx]);

  /*

  • Perform registered function callback

  */

• list_for_each_entry(cb, &irq_obj->irq_cb_tbl[irq_idx], list)

• list_for_each_entry(cb, &dpu_kms->hw_intr->irq_cb_tbl[irq_idx], list) if (cb->func) cb->func(cb->arg, irq_idx);

} @@ -420,6 +419,10 @@ void dpu_hw_intr_destroy(struct dpu_hw_intr *intr) { if (intr) { kfree(intr->cache_irq_mask);

• kfree(intr->irq_cb_tbl);
  

• kfree(intr->irq_counts);
  

• kfree(intr); }

} @@ -429,7 +432,7 @@ int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx, { unsigned long irq_flags;

• if (!dpu_kms->irq_obj.irq_cb_tbl) {

• if (!dpu_kms->hw_intr->irq_cb_tbl) { DPU_ERROR("invalid params\n"); return -EINVAL; }

@@ -453,9 +456,9 @@ int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx, trace_dpu_core_irq_register_callback(irq_idx, register_irq_cb); list_del_init(&register_irq_cb->list); list_add_tail(&register_irq_cb->list,

• 	&dpu_kms->irq_obj.irq_cb_tbl[irq_idx]);
  

• 	&dpu_kms->hw_intr->irq_cb_tbl[irq_idx]);
  

  if (list_is_first(&register_irq_cb->list,

• 	&dpu_kms->irq_obj.irq_cb_tbl[irq_idx])) {
  

• 	&dpu_kms->hw_intr->irq_cb_tbl[irq_idx])) {
  

  int ret = dpu_hw_intr_enable_irq_locked( dpu_kms->hw_intr, irq_idx);

@@ -473,7 +476,7 @@ int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx, { unsigned long irq_flags;

• if (!dpu_kms->irq_obj.irq_cb_tbl) {

• if (!dpu_kms->hw_intr->irq_cb_tbl) { DPU_ERROR("invalid params\n"); return -EINVAL; }

@@ -497,7 +500,7 @@ int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx, trace_dpu_core_irq_unregister_callback(irq_idx, register_irq_cb); list_del_init(&register_irq_cb->list); /* empty callback list but interrupt is still enabled */

• if (list_empty(&dpu_kms->irq_obj.irq_cb_tbl[irq_idx])) {

• if (list_empty(&dpu_kms->hw_intr->irq_cb_tbl[irq_idx])) { int ret = dpu_hw_intr_disable_irq_locked( dpu_kms->hw_intr, irq_idx);

@@ -515,19 +518,18 @@ int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx, static int dpu_debugfs_core_irq_show(struct seq_file *s, void *v) { struct dpu_kms *dpu_kms = s->private;

• struct dpu_irq *irq_obj = &dpu_kms->irq_obj; struct dpu_irq_callback *cb; unsigned long irq_flags; int i, irq_count, cb_count;

• if (WARN_ON(!irq_obj->irq_cb_tbl))

• if (WARN_ON(!dpu_kms->hw_intr->irq_cb_tbl)) return 0;

• for (i = 0; i < irq_obj->total_irqs; i++) {

• for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) { spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags); cb_count = 0;

• irq_count = atomic_read(&irq_obj->irq_counts[i]);
  

• list_for_each_entry(cb, &irq_obj->irq_cb_tbl[i], list)
  

• irq_count = atomic_read(&dpu_kms->hw_intr->irq_counts[i]);
  

• list_for_each_entry(cb, &dpu_kms->hw_intr->irq_cb_tbl[i], list)
  cb_count++;
  

  spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);

@@ -559,14 +561,13 @@ void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms) pm_runtime_put_sync(&dpu_kms->pdev->dev);

/* Create irq callbacks for all possible irq_idx */

• dpu_kms->irq_obj.total_irqs = dpu_kms->hw_intr->total_irqs;
• dpu_kms->irq_obj.irq_cb_tbl = kcalloc(dpu_kms->irq_obj.total_irqs,

• dpu_kms->hw_intr->irq_cb_tbl = kcalloc(dpu_kms->hw_intr->total_irqs, sizeof(struct list_head), GFP_KERNEL);

• dpu_kms->irq_obj.irq_counts = kcalloc(dpu_kms->irq_obj.total_irqs,

• dpu_kms->hw_intr->irq_counts = kcalloc(dpu_kms->hw_intr->total_irqs, sizeof(atomic_t), GFP_KERNEL);

• for (i = 0; i < dpu_kms->irq_obj.total_irqs; i++) {

• INIT_LIST_HEAD(&dpu_kms->irq_obj.irq_cb_tbl[i]);
  

• atomic_set(&dpu_kms->irq_obj.irq_counts[i], 0);
  

• for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) {

• INIT_LIST_HEAD(&dpu_kms->hw_intr->irq_cb_tbl[i]);
  

• atomic_set(&dpu_kms->hw_intr->irq_counts[i], 0);
  

  }

}

@@ -575,17 +576,11 @@ void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms) int i;

pm_runtime_get_sync(&dpu_kms->pdev->dev);

• for (i = 0; i < dpu_kms->irq_obj.total_irqs; i++)

• if (!list_empty(&dpu_kms->irq_obj.irq_cb_tbl[i]))
  

• for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++)

• if (!list_empty(&dpu_kms->hw_intr->irq_cb_tbl[i]))
  DPU_ERROR("irq_idx=%d still enabled/registered\n", i);
  
  

  dpu_clear_irqs(dpu_kms); dpu_disable_all_irqs(dpu_kms); pm_runtime_put_sync(&dpu_kms->pdev->dev);

• kfree(dpu_kms->irq_obj.irq_cb_tbl);
• kfree(dpu_kms->irq_obj.irq_counts);
• dpu_kms->irq_obj.irq_cb_tbl = NULL;
• dpu_kms->irq_obj.irq_counts = NULL;
• dpu_kms->irq_obj.total_irqs = 0;

} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h index 99ab01f54aa6..d50e78c9f148 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h @@ -40,6 +40,8 @@ enum dpu_hw_intr_reg {

• @save_irq_status: array of IRQ status reg storage created during

init

• @total_irqs: total number of irq_idx mapped in the hw_interrupts
• @irq_lock: spinlock for accessing IRQ resources

• • @irq_cb_tbl: array of IRQ callbacks lists
• • @irq_counts: array of IRQ counts
  */

struct dpu_hw_intr { struct dpu_hw_blk_reg_map hw; @@ -48,6 +50,9 @@ struct dpu_hw_intr { u32 total_irqs; spinlock_t irq_lock; unsigned long irq_mask;

• struct list_head *irq_cb_tbl;
• atomic_t *irq_counts;

};

/** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 323a6bce9e64..775bcbda860f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -78,18 +78,6 @@ struct dpu_irq_callback { void *arg; };

-/**

• • struct dpu_irq: IRQ structure contains callback registration info
• • @total_irq: total number of irq_idx obtained from HW interrupts

mapping

• • @irq_cb_tbl: array of IRQ callbacks setting
• • @debugfs_file: debugfs file for irq statistics
• */

-struct dpu_irq {

• u32 total_irqs;
• struct list_head *irq_cb_tbl;
• atomic_t *irq_counts;

-};

struct dpu_kms { struct msm_kms base; struct drm_device *dev; @@ -104,7 +92,6 @@ struct dpu_kms { struct regulator *venus;

struct dpu_hw_intr *hw_intr;

• struct dpu_irq irq_obj;

  struct dpu_core_perf perf;

On 2021-06-17 15:20, Dmitry Baryshkov wrote:

DPU interrupts code allows multiple callbacks per interrut. In reality

/interrupt

none of the interrupts is shared between blocks (and will probably never be). Drop support for registering multiple callbacks per interrupt to simplify interrupt handling code.

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

I need to check on why we had this design originally and we still do. the idx with which we are registering today can generate only one hw interrupt. But i am not sure if something for planned for future use. Will update in a day or two.

meanwhile some comments and questions below.

---

drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h | 18 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 6 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 2 +- .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 10 +- .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 6 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 144 +++++++----------- .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 12 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 12 -- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 +- 9 files changed, 86 insertions(+), 134 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h index 90ae6c9ccc95..44ab97fb2964 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h @@ -46,10 +46,8 @@ u32 dpu_core_irq_read(

•                         interrupt
  

• @dpu_kms: DPU handle
• @irq_idx: irq index

• • @irq_cb: IRQ callback structure, containing callback function

• • 	and argument. Passing NULL for irq_cb will unregister
    

• • 	the callback for the given irq_idx
    

• • 	This must exist until un-registration.
    

• • @irq_cb: IRQ callback funcion.
• • @irq_arg: IRQ callback argument.
  • @return: 0 for success registering callback, otherwise failure
  • This function supports registration of multiple callbacks for each

interrupt. @@ -57,17 +55,16 @@ u32 dpu_core_irq_read( int dpu_core_irq_register_callback( struct dpu_kms *dpu_kms, int irq_idx,

• struct dpu_irq_callback *irq_cb);
  

• void (*irq_cb)(void *arg, int irq_idx),
  

• void *irq_arg);
  
  

/**

• dpu_core_irq_unregister_callback - For unregistering callback

function on IRQ

•                         interrupt
  

• @dpu_kms: DPU handle
• @irq_idx: irq index

• • @irq_cb: IRQ callback structure, containing callback function

• • 	and argument. Passing NULL for irq_cb will unregister
    

• • 	the callback for the given irq_idx
    

• • 	This must match with registration.
    

• • @irq_cb: IRQ callback funcion.

/function this typo is there in multiple places

• • @irq_arg: IRQ callback argument.
  • @return: 0 for success registering callback, otherwise failure
  • This function supports registration of multiple callbacks for each

interrupt. @@ -75,7 +72,8 @@ int dpu_core_irq_register_callback( int dpu_core_irq_unregister_callback( struct dpu_kms *dpu_kms, int irq_idx,

• struct dpu_irq_callback *irq_cb);
  

• void (*irq_cb)(void *arg, int irq_idx),
  

• void *irq_arg);
  
  

/**

• dpu_debugfs_core_irq_init - register core irq debugfs

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 1c04b7cce43e..d3557b0f4db9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -310,7 +310,7 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc, phys_enc->hw_pp->idx - PINGPONG_0, atomic_read(wait_info->atomic_cnt)); local_irq_save(flags);

• 	irq->cb.func(phys_enc, irq->irq_idx);
  

• 	irq->func(phys_enc, irq->irq_idx);
  local_irq_restore(flags);
  ret = 0;
  

  } else {

@@ -352,7 +352,7 @@ int dpu_encoder_helper_register_irq(struct dpu_encoder_phys *phys_enc, }

ret = dpu_core_irq_register_callback(phys_enc->dpu_kms, irq->irq_idx,

• 	&irq->cb);
  

• 	irq->func, phys_enc);
  

  if (ret) { DPU_ERROR_PHYS(phys_enc, "failed to register IRQ callback for %s\n",

@@ -384,7 +384,7 @@ int dpu_encoder_helper_unregister_irq(struct dpu_encoder_phys *phys_enc, }

ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms, irq->irq_idx,

• 	&irq->cb);
  

• 	irq->func, phys_enc);
  

  if (ret) { DRM_ERROR("unreg cb fail id=%u, intr=%d, irq=%d ret=%d", DRMID(phys_enc->parent), intr_idx,

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h index e7270eb6b84b..80d87871fd94 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -174,7 +174,7 @@ struct dpu_encoder_irq { const char *name; enum dpu_intr_idx intr_idx; int irq_idx;

• struct dpu_irq_callback cb;

• void (*func)(void *arg, int irq_idx);

};

/** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index aa01698d6b25..f921a5c99456 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -782,30 +782,28 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( phys_enc->enable_state = DPU_ENC_DISABLED; for (i = 0; i < INTR_IDX_MAX; i++) { irq = &phys_enc->irq[i];

• INIT_LIST_HEAD(&irq->cb.list);
  

  irq->irq_idx = -EINVAL;

• irq->cb.arg = phys_enc;
  

  }

  irq = &phys_enc->irq[INTR_IDX_CTL_START]; irq->name = "ctl_start"; irq->intr_idx = INTR_IDX_CTL_START;

• irq->cb.func = dpu_encoder_phys_cmd_ctl_start_irq;

• irq->func = dpu_encoder_phys_cmd_ctl_start_irq;

  irq = &phys_enc->irq[INTR_IDX_PINGPONG]; irq->name = "pp_done"; irq->intr_idx = INTR_IDX_PINGPONG;

• irq->cb.func = dpu_encoder_phys_cmd_pp_tx_done_irq;

• irq->func = dpu_encoder_phys_cmd_pp_tx_done_irq;

  irq = &phys_enc->irq[INTR_IDX_RDPTR]; irq->name = "pp_rd_ptr"; irq->intr_idx = INTR_IDX_RDPTR;

• irq->cb.func = dpu_encoder_phys_cmd_pp_rd_ptr_irq;

• irq->func = dpu_encoder_phys_cmd_pp_rd_ptr_irq;

  irq = &phys_enc->irq[INTR_IDX_UNDERRUN]; irq->name = "underrun"; irq->intr_idx = INTR_IDX_UNDERRUN;

• irq->cb.func = dpu_encoder_phys_cmd_underrun_irq;

• irq->func = dpu_encoder_phys_cmd_underrun_irq;

  atomic_set(&phys_enc->vblank_refcount, 0); atomic_set(&phys_enc->pending_kickoff_cnt, 0);

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index 185379b18572..437af231d6a4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -725,20 +725,18 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init( phys_enc->enc_spinlock = p->enc_spinlock; for (i = 0; i < INTR_IDX_MAX; i++) { irq = &phys_enc->irq[i];

• INIT_LIST_HEAD(&irq->cb.list);
  

  irq->irq_idx = -EINVAL;

• irq->cb.arg = phys_enc;
  

  }

  irq = &phys_enc->irq[INTR_IDX_VSYNC]; irq->name = "vsync_irq"; irq->intr_idx = INTR_IDX_VSYNC;

• irq->cb.func = dpu_encoder_phys_vid_vblank_irq;

• irq->func = dpu_encoder_phys_vid_vblank_irq;

  irq = &phys_enc->irq[INTR_IDX_UNDERRUN]; irq->name = "underrun"; irq->intr_idx = INTR_IDX_UNDERRUN;

• irq->cb.func = dpu_encoder_phys_vid_underrun_irq;

• irq->func = dpu_encoder_phys_vid_underrun_irq;

  atomic_set(&phys_enc->vblank_refcount, 0); atomic_set(&phys_enc->pending_kickoff_cnt, 0);

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index d2b6dca487e3..7062e7f0e860 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -127,21 +127,17 @@ static const struct dpu_intr_reg dpu_intr_set[] = { */ static void dpu_core_irq_callback_handler(struct dpu_kms *dpu_kms, int irq_idx) {

• struct dpu_irq_callback *cb;

• VERB("irq_idx=%d\n", irq_idx);

• if (list_empty(&dpu_kms->hw_intr->irq_cb_tbl[irq_idx]))

• if (!dpu_kms->hw_intr->irq_tbl[irq_idx].cb) DRM_ERROR("no registered cb, idx:%d\n", irq_idx);

• atomic_inc(&dpu_kms->hw_intr->irq_counts[irq_idx]);

• atomic_inc(&dpu_kms->hw_intr->irq_tbl[irq_idx].count);

  /*

  • Perform registered function callback

  */

• list_for_each_entry(cb, &dpu_kms->hw_intr->irq_cb_tbl[irq_idx], list)

• if (cb->func)
  

• 	cb->func(cb->arg, irq_idx);
  

• dpu_kms->hw_intr->irq_tbl[irq_idx].cb(dpu_kms->hw_intr->irq_tbl[irq_idx].arg,

irq_idx); }

irqreturn_t dpu_core_irq(struct dpu_kms *dpu_kms) @@ -389,24 +385,18 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr, struct dpu_mdss_cfg *m) { struct dpu_hw_intr *intr;

• int nirq = MDP_INTR_MAX * 32;

  if (!addr || !m) return ERR_PTR(-EINVAL);

• intr = kzalloc(sizeof(*intr), GFP_KERNEL);

• intr = kzalloc(struct_size(intr, irq_tbl, nirq), GFP_KERNEL); if (!intr) return ERR_PTR(-ENOMEM);

  __intr_offset(m, addr, &intr->hw);

• intr->total_irqs = ARRAY_SIZE(dpu_intr_set) * 32;
• intr->cache_irq_mask = kcalloc(ARRAY_SIZE(dpu_intr_set), sizeof(u32),

• 	GFP_KERNEL);
  

• if (intr->cache_irq_mask == NULL) {

• kfree(intr);
  

• return ERR_PTR(-ENOMEM);
  

• }

• intr->total_irqs = nirq;

  intr->irq_mask = m->mdss_irqs;

@@ -417,31 +407,19 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr,

void dpu_hw_intr_destroy(struct dpu_hw_intr *intr) {

• if (intr) {

• kfree(intr->cache_irq_mask);
  

• kfree(intr->irq_cb_tbl);
  

• kfree(intr->irq_counts);
  

• if (intr) kfree(intr);

• }

}

int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx,

• struct dpu_irq_callback *register_irq_cb)
  

• void (*irq_cb)(void *arg, int irq_idx),
  

• void *irq_arg)
  

{ unsigned long irq_flags;

• int ret;

• if (!dpu_kms->hw_intr->irq_cb_tbl) {

• DPU_ERROR("invalid params\n");
  

• return -EINVAL;
  

• }
• if (!register_irq_cb || !register_irq_cb->func) {

• DPU_ERROR("invalid irq_cb:%d func:%d\n",
  

• 		register_irq_cb != NULL,
  

• 		register_irq_cb ?
  

• 			register_irq_cb->func != NULL : -1);
  

• if (!irq_cb) {

• DPU_ERROR("invalid ird_idx:%d irq_cb:%ps\n", irq_idx, irq_cb);
  

  return -EINVAL; }

@@ -453,39 +431,37 @@ int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx, VERB("[%pS] irq_idx=%d\n", __builtin_return_address(0), irq_idx);

spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags);

• trace_dpu_core_irq_register_callback(irq_idx, register_irq_cb);
• list_del_init(&register_irq_cb->list);
• list_add_tail(&register_irq_cb->list,

• 	&dpu_kms->hw_intr->irq_cb_tbl[irq_idx]);
  

• if (list_is_first(&register_irq_cb->list,

• 	&dpu_kms->hw_intr->irq_cb_tbl[irq_idx])) {
  

• int ret = dpu_hw_intr_enable_irq_locked(
  

• if (dpu_kms->hw_intr->irq_tbl[irq_idx].cb) {

• spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
  

• return -EBUSY;
  

• }

Why is this check present? Ideally there should be only one callback registered.

• trace_dpu_core_irq_register_callback(irq_idx, irq_cb);
• dpu_kms->hw_intr->irq_tbl[irq_idx].arg = irq_arg;
• dpu_kms->hw_intr->irq_tbl[irq_idx].cb = irq_cb;
• ret = dpu_hw_intr_enable_irq_locked( dpu_kms->hw_intr, irq_idx);

• if (ret)
  

• 	DPU_ERROR("Fail to enable IRQ for irq_idx:%d\n",
  

• if (ret)

• DPU_ERROR("Fail to enable IRQ for irq_idx:%d\n",
  		irq_idx);
  

• } spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);

  return 0;

}

int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx,

• struct dpu_irq_callback *register_irq_cb)
  

• void (*irq_cb)(void *arg, int irq_idx),
  

• void *irq_arg)
  

{ unsigned long irq_flags;

• int ret;

• if (!dpu_kms->hw_intr->irq_cb_tbl) {

• DPU_ERROR("invalid params\n");
  

• return -EINVAL;
  

• }
• if (!register_irq_cb || !register_irq_cb->func) {

• DPU_ERROR("invalid irq_cb:%d func:%d\n",
  

• 		register_irq_cb != NULL,
  

• 		register_irq_cb ?
  

• 			register_irq_cb->func != NULL : -1);
  

• if (!irq_cb) {

• DPU_ERROR("invalid ird_idx:%d irq_cb:%ps\n", irq_idx, irq_cb);
  

  return -EINVAL; }

@@ -497,18 +473,23 @@ int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx, VERB("[%pS] irq_idx=%d\n", __builtin_return_address(0), irq_idx);

spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags);

• trace_dpu_core_irq_unregister_callback(irq_idx, register_irq_cb);
• list_del_init(&register_irq_cb->list);
• /* empty callback list but interrupt is still enabled */
• if (list_empty(&dpu_kms->hw_intr->irq_cb_tbl[irq_idx])) {

• int ret = dpu_hw_intr_disable_irq_locked(
  

• 		dpu_kms->hw_intr,
  

• 		irq_idx);
  

• if (ret)
  

• 	DPU_ERROR("Fail to disable IRQ for irq_idx:%d\n",
  

• 			irq_idx);
  

• VERB("irq_idx=%d ret=%d\n", irq_idx, ret);
  

• trace_dpu_core_irq_unregister_callback(irq_idx, irq_cb);
• if (WARN_ON(dpu_kms->hw_intr->irq_tbl[irq_idx].cb != irq_cb ||

•     dpu_kms->hw_intr->irq_tbl[irq_idx].arg != irq_arg)) {
  

• spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
  

• return -EINVAL;
  

  }

Why do we need this check too? Any condition where this wont match?

• ret = dpu_hw_intr_disable_irq_locked(dpu_kms->hw_intr, irq_idx);

• if (ret)

• DPU_ERROR("Fail to disable IRQ for irq_idx:%d: %d\n",
  

• 			irq_idx, ret);
  

• dpu_kms->hw_intr->irq_tbl[irq_idx].cb = NULL;

• dpu_kms->hw_intr->irq_tbl[irq_idx].arg = NULL;

• spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);

  return 0;

@@ -518,24 +499,18 @@ int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx, static int dpu_debugfs_core_irq_show(struct seq_file *s, void *v) { struct dpu_kms *dpu_kms = s->private;

• struct dpu_irq_callback *cb; unsigned long irq_flags;
• int i, irq_count, cb_count;
• if (WARN_ON(!dpu_kms->hw_intr->irq_cb_tbl))

• return 0;
  

• int i, irq_count;

• void *cb;

  for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) { spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags);

• cb_count = 0;
  

• irq_count = atomic_read(&dpu_kms->hw_intr->irq_counts[i]);
  

• list_for_each_entry(cb, &dpu_kms->hw_intr->irq_cb_tbl[i], list)
  

• 	cb_count++;
  

• irq_count = atomic_read(&dpu_kms->hw_intr->irq_tbl[i].count);
  

• cb = dpu_kms->hw_intr->irq_tbl[i].cb;
  

  spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);

• if (irq_count || cb_count)
  

• 	seq_printf(s, "idx:%d irq:%d cb:%d\n",
  

• 			i, irq_count, cb_count);
  

• if (irq_count || cb)
  

• 	seq_printf(s, "idx:%d irq:%d cb:%ps\n", i, irq_count, cb);
  

  }

  return 0;

@@ -560,15 +535,8 @@ void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms) dpu_disable_all_irqs(dpu_kms); pm_runtime_put_sync(&dpu_kms->pdev->dev);

• /* Create irq callbacks for all possible irq_idx */
• dpu_kms->hw_intr->irq_cb_tbl = kcalloc(dpu_kms->hw_intr->total_irqs,

• 	sizeof(struct list_head), GFP_KERNEL);
  

• dpu_kms->hw_intr->irq_counts = kcalloc(dpu_kms->hw_intr->total_irqs,

• 	sizeof(atomic_t), GFP_KERNEL);
  

• for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) {

• INIT_LIST_HEAD(&dpu_kms->hw_intr->irq_cb_tbl[i]);
  

• atomic_set(&dpu_kms->hw_intr->irq_counts[i], 0);
  

• }

• for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++)

• atomic_set(&dpu_kms->hw_intr->irq_tbl[i].count, 0);
  

}

void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms) @@ -577,7 +545,7 @@ void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms)

pm_runtime_get_sync(&dpu_kms->pdev->dev); for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++)

• if (!list_empty(&dpu_kms->hw_intr->irq_cb_tbl[i]))
  

• if (dpu_kms->hw_intr->irq_tbl[i].cb)
  DPU_ERROR("irq_idx=%d still enabled/registered\n", i);
  
  

  dpu_clear_irqs(dpu_kms);

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h index d50e78c9f148..9855e176b173 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h @@ -40,19 +40,21 @@ enum dpu_hw_intr_reg {

• @save_irq_status: array of IRQ status reg storage created during

init

• @total_irqs: total number of irq_idx mapped in the hw_interrupts
• @irq_lock: spinlock for accessing IRQ resources

• • @irq_cb_tbl: array of IRQ callbacks lists
• • @irq_counts: array of IRQ counts

• • @irq_cb_tbl: array of IRQ callbacks
  */

struct dpu_hw_intr { struct dpu_hw_blk_reg_map hw;

• u32 *cache_irq_mask;

• u32 cache_irq_mask[MDP_INTR_MAX]; u32 *save_irq_status; u32 total_irqs; spinlock_t irq_lock; unsigned long irq_mask;

• struct list_head *irq_cb_tbl;
• atomic_t *irq_counts;

• struct {

• void (*cb)(void *arg, int irq_idx);
  

• void *arg;
  

• atomic_t count;
  

• } irq_tbl[];

};

/** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 775bcbda860f..d24dcd852d2e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -66,18 +66,6 @@

#define DPU_NAME_SIZE 12

-/*

• • struct dpu_irq_callback - IRQ callback handlers
• • @list: list to callback
• • @func: intr handler
• • @arg: argument for the handler
• */

-struct dpu_irq_callback {

• struct list_head list;
• void (*func)(void *arg, int irq_idx);
• void *arg;

-};

struct dpu_kms { struct msm_kms base; struct drm_device *dev; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h index 37bba57675a8..1e2619556f93 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h @@ -876,25 +876,25 @@ TRACE_EVENT(dpu_pp_connect_ext_te, );

DECLARE_EVENT_CLASS(dpu_core_irq_callback_template,

• TP_PROTO(int irq_idx, struct dpu_irq_callback *callback),

• TP_PROTO(int irq_idx, void *callback), TP_ARGS(irq_idx, callback), TP_STRUCT__entry( __field( int, irq_idx )

• __field(	struct dpu_irq_callback *,	callback)
  

• __field(	void *,				callback)
  

  ), TP_fast_assign( __entry->irq_idx = irq_idx; __entry->callback = callback; ),

• TP_printk("irq_idx:%d callback:%pK", __entry->irq_idx,

• TP_printk("irq_idx:%d callback:%ps", __entry->irq_idx, __entry->callback)

); DEFINE_EVENT(dpu_core_irq_callback_template, dpu_core_irq_register_callback,

• TP_PROTO(int irq_idx, struct dpu_irq_callback *callback),

• TP_PROTO(int irq_idx, void *callback), TP_ARGS(irq_idx, callback)

); DEFINE_EVENT(dpu_core_irq_callback_template, dpu_core_irq_unregister_callback,

• TP_PROTO(int irq_idx, struct dpu_irq_callback *callback),

• TP_PROTO(int irq_idx, void *callback), TP_ARGS(irq_idx, callback)

);

On 19/01/2022 03:10, abhinavk@codeaurora.org wrote:

<replying from my older email as i couldnt find this thread on my new one>

On 2021-08-17 20:30, abhinavk@codeaurora.org wrote:

...

On 2021-06-17 15:20, Dmitry Baryshkov wrote:

...

DPU interrupts code allows multiple callbacks per interrut. In reality

/interrupt

...

none of the interrupts is shared between blocks (and will probably never be). Drop support for registering multiple callbacks per interrupt to simplify interrupt handling code.

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

I need to check on why we had this design originally and we still do. the idx with which we are registering today can generate only one hw interrupt. But i am not sure if something for planned for future use. Will update in a day or two.

meanwhile some comments and questions below.

I did check internally on the original design of this and yes the plan was for other sub-modules to register for callbacks and that callback goes into the callback list.

For example, lets say for some reason some other modules like the DSI want to register for the VSYNC interrupt, it can register for a callback and that will get added to the callback list.

But, this never got used that way and even now there is only one callback per interrupt. We dont have a concrete use-case where we will need this in the future but like many other things, we cannot tell for certain.

Since there is no concrete use-case where we might need this, if you would like to go ahead with this, please do.

Once you address the other comments on this, I can ack this.

Thanks for the info. I'll respin this on top of current msm-next + https://patchwork.freedesktop.org/patch/464350/

...

...

---

drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h  |  18 +--  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   |   6 +-  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |   2 +-  .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  |  10 +-  .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  |   6 +-  .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 144 +++++++-----------  .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h |  12 +-  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h       |  12 --  drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h     |  10 +-  9 files changed, 86 insertions(+), 134 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h index 90ae6c9ccc95..44ab97fb2964 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h @@ -46,10 +46,8 @@ u32 dpu_core_irq_read(   *                             interrupt   * @dpu_kms:        DPU handle   * @irq_idx:        irq index

• • @irq_cb:        IRQ callback structure, containing callback function
• *            and argument. Passing NULL for irq_cb will unregister
• *            the callback for the given irq_idx
• *            This must exist until un-registration.

• • @irq_cb:        IRQ callback funcion.
• • @irq_arg:        IRQ callback argument.

* @return:        0 for success registering callback, otherwise failure   *   * This function supports registration of multiple callbacks for each interrupt. @@ -57,17 +55,16 @@ u32 dpu_core_irq_read(  int dpu_core_irq_register_callback(          struct dpu_kms *dpu_kms,          int irq_idx, -        struct dpu_irq_callback *irq_cb); +        void (*irq_cb)(void *arg, int irq_idx), +        void *irq_arg);

/**   * dpu_core_irq_unregister_callback - For unregistering callback function on IRQ   *                             interrupt   * @dpu_kms:        DPU handle   * @irq_idx:        irq index

• • @irq_cb:        IRQ callback structure, containing callback function
• *            and argument. Passing NULL for irq_cb will unregister
• *            the callback for the given irq_idx
• *            This must match with registration.

• • @irq_cb:        IRQ callback funcion.

/function this typo is there in multiple places

...

• • @irq_arg:        IRQ callback argument.

* @return:        0 for success registering callback, otherwise failure   *   * This function supports registration of multiple callbacks for each interrupt. @@ -75,7 +72,8 @@ int dpu_core_irq_register_callback(  int dpu_core_irq_unregister_callback(          struct dpu_kms *dpu_kms,          int irq_idx, -        struct dpu_irq_callback *irq_cb); +        void (*irq_cb)(void *arg, int irq_idx), +        void *irq_arg);

/**   * dpu_debugfs_core_irq_init - register core irq debugfs diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 1c04b7cce43e..d3557b0f4db9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -310,7 +310,7 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,                        phys_enc->hw_pp->idx - PINGPONG_0,                        atomic_read(wait_info->atomic_cnt));              local_irq_save(flags); -            irq->cb.func(phys_enc, irq->irq_idx); +            irq->func(phys_enc, irq->irq_idx);              local_irq_restore(flags);              ret = 0;          } else { @@ -352,7 +352,7 @@ int dpu_encoder_helper_register_irq(struct dpu_encoder_phys *phys_enc,      }

ret = dpu_core_irq_register_callback(phys_enc->dpu_kms, irq->irq_idx, -            &irq->cb); +            irq->func, phys_enc);      if (ret) {          DPU_ERROR_PHYS(phys_enc,              "failed to register IRQ callback for %s\n", @@ -384,7 +384,7 @@ int dpu_encoder_helper_unregister_irq(struct dpu_encoder_phys *phys_enc,      }

ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms, irq->irq_idx, -            &irq->cb); +            irq->func, phys_enc);      if (ret) {          DRM_ERROR("unreg cb fail id=%u, intr=%d, irq=%d ret=%d",                DRMID(phys_enc->parent), intr_idx, diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h index e7270eb6b84b..80d87871fd94 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -174,7 +174,7 @@ struct dpu_encoder_irq {      const char *name;      enum dpu_intr_idx intr_idx;      int irq_idx; -    struct dpu_irq_callback cb; +    void (*func)(void *arg, int irq_idx);  };

/** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index aa01698d6b25..f921a5c99456 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -782,30 +782,28 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(      phys_enc->enable_state = DPU_ENC_DISABLED;      for (i = 0; i < INTR_IDX_MAX; i++) {          irq = &phys_enc->irq[i]; -        INIT_LIST_HEAD(&irq->cb.list);          irq->irq_idx = -EINVAL; -        irq->cb.arg = phys_enc;      }

irq = &phys_enc->irq[INTR_IDX_CTL_START];      irq->name = "ctl_start";      irq->intr_idx = INTR_IDX_CTL_START; -    irq->cb.func = dpu_encoder_phys_cmd_ctl_start_irq; +    irq->func = dpu_encoder_phys_cmd_ctl_start_irq;

irq = &phys_enc->irq[INTR_IDX_PINGPONG];      irq->name = "pp_done";      irq->intr_idx = INTR_IDX_PINGPONG; -    irq->cb.func = dpu_encoder_phys_cmd_pp_tx_done_irq; +    irq->func = dpu_encoder_phys_cmd_pp_tx_done_irq;

irq = &phys_enc->irq[INTR_IDX_RDPTR];      irq->name = "pp_rd_ptr";      irq->intr_idx = INTR_IDX_RDPTR; -    irq->cb.func = dpu_encoder_phys_cmd_pp_rd_ptr_irq; +    irq->func = dpu_encoder_phys_cmd_pp_rd_ptr_irq;

irq = &phys_enc->irq[INTR_IDX_UNDERRUN];      irq->name = "underrun";      irq->intr_idx = INTR_IDX_UNDERRUN; -    irq->cb.func = dpu_encoder_phys_cmd_underrun_irq; +    irq->func = dpu_encoder_phys_cmd_underrun_irq;

atomic_set(&phys_enc->vblank_refcount, 0);      atomic_set(&phys_enc->pending_kickoff_cnt, 0); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index 185379b18572..437af231d6a4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -725,20 +725,18 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(      phys_enc->enc_spinlock = p->enc_spinlock;      for (i = 0; i < INTR_IDX_MAX; i++) {          irq = &phys_enc->irq[i]; -        INIT_LIST_HEAD(&irq->cb.list);          irq->irq_idx = -EINVAL; -        irq->cb.arg = phys_enc;      }

irq = &phys_enc->irq[INTR_IDX_VSYNC];      irq->name = "vsync_irq";      irq->intr_idx = INTR_IDX_VSYNC; -    irq->cb.func = dpu_encoder_phys_vid_vblank_irq; +    irq->func = dpu_encoder_phys_vid_vblank_irq;

irq = &phys_enc->irq[INTR_IDX_UNDERRUN];      irq->name = "underrun";      irq->intr_idx = INTR_IDX_UNDERRUN; -    irq->cb.func = dpu_encoder_phys_vid_underrun_irq; +    irq->func = dpu_encoder_phys_vid_underrun_irq;

atomic_set(&phys_enc->vblank_refcount, 0);      atomic_set(&phys_enc->pending_kickoff_cnt, 0); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index d2b6dca487e3..7062e7f0e860 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -127,21 +127,17 @@ static const struct dpu_intr_reg dpu_intr_set[] = {   */  static void dpu_core_irq_callback_handler(struct dpu_kms *dpu_kms, int irq_idx)  { -    struct dpu_irq_callback *cb;

VERB("irq_idx=%d\n", irq_idx);

-    if (list_empty(&dpu_kms->hw_intr->irq_cb_tbl[irq_idx])) +    if (!dpu_kms->hw_intr->irq_tbl[irq_idx].cb)          DRM_ERROR("no registered cb, idx:%d\n", irq_idx);

-    atomic_inc(&dpu_kms->hw_intr->irq_counts[irq_idx]); +    atomic_inc(&dpu_kms->hw_intr->irq_tbl[irq_idx].count);

/*       * Perform registered function callback       */ -    list_for_each_entry(cb, &dpu_kms->hw_intr->irq_cb_tbl[irq_idx], list) -        if (cb->func) -            cb->func(cb->arg, irq_idx);

dpu_kms->hw_intr->irq_tbl[irq_idx].cb(dpu_kms->hw_intr->irq_tbl[irq_idx].arg,

irq_idx);  }

irqreturn_t dpu_core_irq(struct dpu_kms *dpu_kms) @@ -389,24 +385,18 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr,          struct dpu_mdss_cfg *m)  {      struct dpu_hw_intr *intr; +    int nirq = MDP_INTR_MAX * 32;

if (!addr || !m)          return ERR_PTR(-EINVAL);

-    intr = kzalloc(sizeof(*intr), GFP_KERNEL); +    intr = kzalloc(struct_size(intr, irq_tbl, nirq), GFP_KERNEL);      if (!intr)          return ERR_PTR(-ENOMEM);

__intr_offset(m, addr, &intr->hw);

-    intr->total_irqs = ARRAY_SIZE(dpu_intr_set) * 32;

-    intr->cache_irq_mask = kcalloc(ARRAY_SIZE(dpu_intr_set), sizeof(u32), -            GFP_KERNEL); -    if (intr->cache_irq_mask == NULL) { -        kfree(intr); -        return ERR_PTR(-ENOMEM); -    } +    intr->total_irqs = nirq;

intr->irq_mask = m->mdss_irqs;

@@ -417,31 +407,19 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr,

void dpu_hw_intr_destroy(struct dpu_hw_intr *intr)  { -    if (intr) { -        kfree(intr->cache_irq_mask);

-        kfree(intr->irq_cb_tbl); -        kfree(intr->irq_counts);

+    if (intr)          kfree(intr); -    }  }

int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx, -        struct dpu_irq_callback *register_irq_cb) +        void (*irq_cb)(void *arg, int irq_idx), +        void *irq_arg)  {      unsigned long irq_flags; +    int ret;

-    if (!dpu_kms->hw_intr->irq_cb_tbl) { -        DPU_ERROR("invalid params\n"); -        return -EINVAL; -    }

-    if (!register_irq_cb || !register_irq_cb->func) { -        DPU_ERROR("invalid irq_cb:%d func:%d\n", -                register_irq_cb != NULL, -                register_irq_cb ? -                    register_irq_cb->func != NULL : -1); +    if (!irq_cb) { +        DPU_ERROR("invalid ird_idx:%d irq_cb:%ps\n", irq_idx, irq_cb);          return -EINVAL;      }

@@ -453,39 +431,37 @@ int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx,      VERB("[%pS] irq_idx=%d\n", __builtin_return_address(0), irq_idx);

spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags); -    trace_dpu_core_irq_register_callback(irq_idx, register_irq_cb); -    list_del_init(&register_irq_cb->list); -    list_add_tail(&register_irq_cb->list, -            &dpu_kms->hw_intr->irq_cb_tbl[irq_idx]); -    if (list_is_first(&register_irq_cb->list, -            &dpu_kms->hw_intr->irq_cb_tbl[irq_idx])) { -        int ret = dpu_hw_intr_enable_irq_locked(

+    if (dpu_kms->hw_intr->irq_tbl[irq_idx].cb) { +        spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);

+        return -EBUSY; +    }

Why is this check present? Ideally there should be only one callback registered.

There should be one callback registered. However I'd prefer to be safe than sorry and to warn if somebody attempts to register a second callback here.

...

...

+    trace_dpu_core_irq_register_callback(irq_idx, irq_cb); +    dpu_kms->hw_intr->irq_tbl[irq_idx].arg = irq_arg; +    dpu_kms->hw_intr->irq_tbl[irq_idx].cb = irq_cb;

+    ret = dpu_hw_intr_enable_irq_locked(                  dpu_kms->hw_intr,                  irq_idx); -        if (ret) -            DPU_ERROR("Fail to enable IRQ for irq_idx:%d\n", +    if (ret) +        DPU_ERROR("Fail to enable IRQ for irq_idx:%d\n",                      irq_idx); -    }      spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);

return 0;  }

int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx, -        struct dpu_irq_callback *register_irq_cb) +        void (*irq_cb)(void *arg, int irq_idx), +        void *irq_arg)  {      unsigned long irq_flags; +    int ret;

-    if (!dpu_kms->hw_intr->irq_cb_tbl) { -        DPU_ERROR("invalid params\n"); -        return -EINVAL; -    }

-    if (!register_irq_cb || !register_irq_cb->func) { -        DPU_ERROR("invalid irq_cb:%d func:%d\n", -                register_irq_cb != NULL, -                register_irq_cb ? -                    register_irq_cb->func != NULL : -1); +    if (!irq_cb) { +        DPU_ERROR("invalid ird_idx:%d irq_cb:%ps\n", irq_idx, irq_cb);          return -EINVAL;      }

@@ -497,18 +473,23 @@ int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx,      VERB("[%pS] irq_idx=%d\n", __builtin_return_address(0), irq_idx);

spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags); -    trace_dpu_core_irq_unregister_callback(irq_idx, register_irq_cb); -    list_del_init(&register_irq_cb->list); -    /* empty callback list but interrupt is still enabled */ -    if (list_empty(&dpu_kms->hw_intr->irq_cb_tbl[irq_idx])) { -        int ret = dpu_hw_intr_disable_irq_locked( -                dpu_kms->hw_intr, -                irq_idx); -        if (ret) -            DPU_ERROR("Fail to disable IRQ for irq_idx:%d\n", -                    irq_idx); -        VERB("irq_idx=%d ret=%d\n", irq_idx, ret); +    trace_dpu_core_irq_unregister_callback(irq_idx, irq_cb);

+    if (WARN_ON(dpu_kms->hw_intr->irq_tbl[irq_idx].cb != irq_cb || +            dpu_kms->hw_intr->irq_tbl[irq_idx].arg != irq_arg)) { +        spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);

+        return -EINVAL;      }

Why do we need this check too? Any condition where this wont match?

No, we can drop this.

...

...

+    ret = dpu_hw_intr_disable_irq_locked(dpu_kms->hw_intr, irq_idx); +    if (ret) +        DPU_ERROR("Fail to disable IRQ for irq_idx:%d: %d\n", +                    irq_idx, ret);

+    dpu_kms->hw_intr->irq_tbl[irq_idx].cb = NULL; +    dpu_kms->hw_intr->irq_tbl[irq_idx].arg = NULL;

spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);

return 0; @@ -518,24 +499,18 @@ int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx,  static int dpu_debugfs_core_irq_show(struct seq_file *s, void *v)  {      struct dpu_kms *dpu_kms = s->private; -    struct dpu_irq_callback *cb;      unsigned long irq_flags; -    int i, irq_count, cb_count;

-    if (WARN_ON(!dpu_kms->hw_intr->irq_cb_tbl)) -        return 0; +    int i, irq_count; +    void *cb;

for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) {          spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags); -        cb_count = 0; -        irq_count = atomic_read(&dpu_kms->hw_intr->irq_counts[i]); -        list_for_each_entry(cb, &dpu_kms->hw_intr->irq_cb_tbl[i], list) -            cb_count++; +        irq_count = atomic_read(&dpu_kms->hw_intr->irq_tbl[i].count); +        cb = dpu_kms->hw_intr->irq_tbl[i].cb;          spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);

-        if (irq_count || cb_count) -            seq_printf(s, "idx:%d irq:%d cb:%d\n", -                    i, irq_count, cb_count); +        if (irq_count || cb) +            seq_printf(s, "idx:%d irq:%d cb:%ps\n", i, irq_count, cb);      }

return 0; @@ -560,15 +535,8 @@ void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms)      dpu_disable_all_irqs(dpu_kms);      pm_runtime_put_sync(&dpu_kms->pdev->dev);

-    /* Create irq callbacks for all possible irq_idx */ -    dpu_kms->hw_intr->irq_cb_tbl = kcalloc(dpu_kms->hw_intr->total_irqs, -            sizeof(struct list_head), GFP_KERNEL); -    dpu_kms->hw_intr->irq_counts = kcalloc(dpu_kms->hw_intr->total_irqs, -            sizeof(atomic_t), GFP_KERNEL); -    for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) { -        INIT_LIST_HEAD(&dpu_kms->hw_intr->irq_cb_tbl[i]); -        atomic_set(&dpu_kms->hw_intr->irq_counts[i], 0); -    } +    for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) +        atomic_set(&dpu_kms->hw_intr->irq_tbl[i].count, 0);  }

void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms) @@ -577,7 +545,7 @@ void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms)

pm_runtime_get_sync(&dpu_kms->pdev->dev);      for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) -        if (!list_empty(&dpu_kms->hw_intr->irq_cb_tbl[i])) +        if (dpu_kms->hw_intr->irq_tbl[i].cb)              DPU_ERROR("irq_idx=%d still enabled/registered\n", i);

dpu_clear_irqs(dpu_kms); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h index d50e78c9f148..9855e176b173 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h @@ -40,19 +40,21 @@ enum dpu_hw_intr_reg {   * @save_irq_status:  array of IRQ status reg storage created during init   * @total_irqs: total number of irq_idx mapped in the hw_interrupts   * @irq_lock:         spinlock for accessing IRQ resources

• • @irq_cb_tbl:       array of IRQ callbacks lists
• • @irq_counts:       array of IRQ counts

• • @irq_cb_tbl:       array of IRQ callbacks

*/  struct dpu_hw_intr {      struct dpu_hw_blk_reg_map hw; -    u32 *cache_irq_mask; +    u32 cache_irq_mask[MDP_INTR_MAX];      u32 *save_irq_status;      u32 total_irqs;      spinlock_t irq_lock;      unsigned long irq_mask;

-    struct list_head *irq_cb_tbl; -    atomic_t *irq_counts; +    struct { +        void (*cb)(void *arg, int irq_idx); +        void *arg; +        atomic_t count; +    } irq_tbl[];  };

/** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 775bcbda860f..d24dcd852d2e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -66,18 +66,6 @@

#define DPU_NAME_SIZE  12

-/*

• • struct dpu_irq_callback - IRQ callback handlers
• • @list: list to callback
• • @func: intr handler
• • @arg: argument for the handler
• */

-struct dpu_irq_callback { -    struct list_head list; -    void (*func)(void *arg, int irq_idx); -    void *arg; -};

struct dpu_kms {      struct msm_kms base;      struct drm_device *dev; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h index 37bba57675a8..1e2619556f93 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h @@ -876,25 +876,25 @@ TRACE_EVENT(dpu_pp_connect_ext_te,  );

DECLARE_EVENT_CLASS(dpu_core_irq_callback_template, -    TP_PROTO(int irq_idx, struct dpu_irq_callback *callback), +    TP_PROTO(int irq_idx, void *callback),      TP_ARGS(irq_idx, callback),      TP_STRUCT__entry(          __field(    int,                irq_idx    ) -        __field(    struct dpu_irq_callback *,    callback) +        __field(    void *,                callback)      ),      TP_fast_assign(          __entry->irq_idx = irq_idx;          __entry->callback = callback;      ), -    TP_printk("irq_idx:%d callback:%pK", __entry->irq_idx, +    TP_printk("irq_idx:%d callback:%ps", __entry->irq_idx,            __entry->callback)  );  DEFINE_EVENT(dpu_core_irq_callback_template, dpu_core_irq_register_callback, -    TP_PROTO(int irq_idx, struct dpu_irq_callback *callback), +    TP_PROTO(int irq_idx, void *callback),      TP_ARGS(irq_idx, callback)  );  DEFINE_EVENT(dpu_core_irq_callback_template, dpu_core_irq_unregister_callback, -    TP_PROTO(int irq_idx, struct dpu_irq_callback *callback), +    TP_PROTO(int irq_idx, void *callback),      TP_ARGS(irq_idx, callback)  );

On 2021-06-17 15:20, Dmitry Baryshkov wrote:

Remove extra dpu_irq_* wrappers from dpu_kms.c, merge them directly into dpu_core_irq_* functions.

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

Reviewed-by: Abhinav Kumar abhinavk@codeaurora.org

---

drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h | 12 ++++----- .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 9 ++++--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 27 +++---------------- 3 files changed, 15 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h index 44ab97fb2964..afc8cd546368 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h @@ -10,24 +10,24 @@

/**

• dpu_core_irq_preinstall - perform pre-installation of core IRQ

handler

• • @dpu_kms: DPU handle

• • @kms: MSM KMS handle
  • @return: none
  */

-void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms); +void dpu_core_irq_preinstall(struct msm_kms *kms);

/**

• dpu_core_irq_uninstall - uninstall core IRQ handler

• • @dpu_kms: DPU handle

• • @kms: MSM KMS handle
  • @return: none
  */

-void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms); +void dpu_core_irq_uninstall(struct msm_kms *kms);

/**

• dpu_core_irq - core IRQ handler

• • @dpu_kms: DPU handle

• • @kms: MSM KMS handle
  • @return: interrupt handling status
  */

-irqreturn_t dpu_core_irq(struct dpu_kms *dpu_kms); +irqreturn_t dpu_core_irq(struct msm_kms *kms);

/**

• dpu_core_irq_read - IRQ helper function for reading IRQ status

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index 7062e7f0e860..9b74cfdf5355 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -140,8 +140,9 @@ static void dpu_core_irq_callback_handler(struct dpu_kms *dpu_kms, int irq_idx)

dpu_kms->hw_intr->irq_tbl[irq_idx].cb(dpu_kms->hw_intr->irq_tbl[irq_idx].arg, irq_idx); }

-irqreturn_t dpu_core_irq(struct dpu_kms *dpu_kms) +irqreturn_t dpu_core_irq(struct msm_kms *kms) {

• struct dpu_kms *dpu_kms = to_dpu_kms(kms); struct dpu_hw_intr *intr = dpu_kms->hw_intr; int reg_idx; int irq_idx;

@@ -526,8 +527,9 @@ void dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms, } #endif

-void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms) +void dpu_core_irq_preinstall(struct msm_kms *kms) {

• struct dpu_kms *dpu_kms = to_dpu_kms(kms); int i;

  pm_runtime_get_sync(&dpu_kms->pdev->dev);

@@ -539,8 +541,9 @@ void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms) atomic_set(&dpu_kms->hw_intr->irq_tbl[i].count, 0); }

-void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms) +void dpu_core_irq_uninstall(struct msm_kms *kms) {

• struct dpu_kms *dpu_kms = to_dpu_kms(kms); int i;

  pm_runtime_get_sync(&dpu_kms->pdev->dev);

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 1d3a4f395e74..c1706205a514 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -761,20 +761,6 @@ static void _dpu_kms_set_encoder_mode(struct msm_kms *kms, encoder->base.id, rc); }

-static irqreturn_t dpu_irq(struct msm_kms *kms) -{

• struct dpu_kms *dpu_kms = to_dpu_kms(kms);
• return dpu_core_irq(dpu_kms);

-}

-static void dpu_irq_preinstall(struct msm_kms *kms) -{

• struct dpu_kms *dpu_kms = to_dpu_kms(kms);
• dpu_core_irq_preinstall(dpu_kms);

-}

static int dpu_irq_postinstall(struct msm_kms *kms) { struct msm_drm_private *priv; @@ -792,13 +778,6 @@ static int dpu_irq_postinstall(struct msm_kms *kms) return 0; }

-static void dpu_irq_uninstall(struct msm_kms *kms) -{

• struct dpu_kms *dpu_kms = to_dpu_kms(kms);
• dpu_core_irq_uninstall(dpu_kms);

-}

static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_kms *kms) { int i; @@ -846,10 +825,10 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_k

static const struct msm_kms_funcs kms_funcs = { .hw_init = dpu_kms_hw_init,

• .irq_preinstall = dpu_irq_preinstall,

• .irq_preinstall = dpu_core_irq_preinstall, .irq_postinstall = dpu_irq_postinstall,

• .irq_uninstall = dpu_irq_uninstall,
• .irq = dpu_irq,

• .irq_uninstall = dpu_core_irq_uninstall,
• .irq = dpu_core_irq, .enable_commit = dpu_kms_enable_commit, .disable_commit = dpu_kms_disable_commit, .vsync_time = dpu_kms_vsync_time,

On 2021-06-17 15:20, Dmitry Baryshkov wrote:

Get rid of dpu_encoder_helper_register_irq/unregister_irq helpers, call dpu_core_register/unregister_callback directly, without surrounding them with helpers.

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

Reviewed-by: Abhinav Kumar abhinavk@codeaurora.org

---

drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 64 ------------------- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 18 ------ .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 39 +++++++---- .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 21 ++++-- .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 4 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 29 +++------ 6 files changed, 56 insertions(+), 119 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index d3557b0f4db9..3d8864df8605 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -333,70 +333,6 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc, return ret; }

-int dpu_encoder_helper_register_irq(struct dpu_encoder_phys *phys_enc,

• enum dpu_intr_idx intr_idx)
  

-{

• struct dpu_encoder_irq *irq;
• int ret = 0;
• if (intr_idx >= INTR_IDX_MAX) {

• DPU_ERROR("invalid params\n");
  

• return -EINVAL;
  

• }
• irq = &phys_enc->irq[intr_idx];
• if (irq->irq_idx < 0) {

• DPU_ERROR_PHYS(phys_enc,
  

• 	"invalid IRQ index:%d\n", irq->irq_idx);
  

• return -EINVAL;
  

• }
• ret = dpu_core_irq_register_callback(phys_enc->dpu_kms, irq->irq_idx,

• 	irq->func, phys_enc);
  

• if (ret) {

• DPU_ERROR_PHYS(phys_enc,
  

• 	"failed to register IRQ callback for %s\n",
  

• 	irq->name);
  

• irq->irq_idx = -EINVAL;
  

• return ret;
  

• }
• trace_dpu_enc_irq_register_success(DRMID(phys_enc->parent), intr_idx,

• 		irq->irq_idx);
  

• return ret;

-}

-int dpu_encoder_helper_unregister_irq(struct dpu_encoder_phys *phys_enc,

• enum dpu_intr_idx intr_idx)
  

-{

• struct dpu_encoder_irq *irq;
• int ret;
• irq = &phys_enc->irq[intr_idx];
• /* silently skip irqs that weren't registered */
• if (irq->irq_idx < 0) {

• DRM_ERROR("duplicate unregister id=%u, intr=%d, irq=%d",
  

• 	  DRMID(phys_enc->parent), intr_idx,
  

• 	  irq->irq_idx);
  

• return 0;
  

• }
• ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms,

irq->irq_idx,

• 	irq->func, phys_enc);
  

• if (ret) {

• DRM_ERROR("unreg cb fail id=%u, intr=%d, irq=%d ret=%d",
  

• 	  DRMID(phys_enc->parent), intr_idx,
  

• 	  irq->irq_idx, ret);
  

• }
• trace_dpu_enc_irq_unregister_success(DRMID(phys_enc->parent),

intr_idx,

• 			     irq->irq_idx);
  

• return 0;

-}

int dpu_encoder_get_frame_count(struct drm_encoder *drm_enc) { struct dpu_encoder_virt *dpu_enc; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h index 80d87871fd94..ff2218155b44 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -364,22 +364,4 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc, enum dpu_intr_idx intr_idx, struct dpu_encoder_wait_info *wait_info);

-/**

• • dpu_encoder_helper_register_irq - register and enable an irq
• • @phys_enc: Pointer to physical encoder structure
• • @intr_idx: encoder interrupt index
• • @Return: 0 or -ERROR
• */

-int dpu_encoder_helper_register_irq(struct dpu_encoder_phys *phys_enc,

• enum dpu_intr_idx intr_idx);
  

-/**

• • dpu_encoder_helper_unregister_irq - unregister and disable an irq
• • @phys_enc: Pointer to physical encoder structure
• • @intr_idx: encoder interrupt index
• • @Return: 0 or -ERROR
• */

-int dpu_encoder_helper_unregister_irq(struct dpu_encoder_phys *phys_enc,

• enum dpu_intr_idx intr_idx);
  

#endif /* __dpu_encoder_phys_H__ */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index f921a5c99456..4bfeac821f51 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -211,7 +211,9 @@ static int _dpu_encoder_phys_cmd_handle_ppdone_timeout( cmd_enc->pp_timeout_report_cnt, atomic_read(&phys_enc->pending_kickoff_cnt)); msm_disp_snapshot_state(drm_enc->dev);

• dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_RDPTR);
  

• dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
  

• 		phys_enc->irq[INTR_IDX_RDPTR].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_RDPTR].func, phys_enc);
  

  }

  atomic_add_unless(&phys_enc->pending_kickoff_cnt, -1, 0);

@@ -277,10 +279,13 @@ static int dpu_encoder_phys_cmd_control_vblank_irq( enable ? "true" : "false", refcount);

if (enable && atomic_inc_return(&phys_enc->vblank_refcount) == 1)

• ret = dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_RDPTR);
  

• ret = dpu_core_irq_register_callback(phys_enc->dpu_kms,
  

• 		phys_enc->irq[INTR_IDX_RDPTR].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_RDPTR].func, phys_enc);
  

  else if (!enable && atomic_dec_return(&phys_enc->vblank_refcount) ==

• ret = dpu_encoder_helper_unregister_irq(phys_enc,
  

• 		INTR_IDX_RDPTR);
  

• ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
  

• 		phys_enc->irq[INTR_IDX_RDPTR].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_RDPTR].func, phys_enc);
  
  

end: if (ret) { @@ -301,21 +306,31 @@ static void dpu_encoder_phys_cmd_irq_control(struct dpu_encoder_phys *phys_enc, enable, atomic_read(&phys_enc->vblank_refcount));

if (enable) {

• dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_PINGPONG);
  

• dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_UNDERRUN);
  

• dpu_core_irq_register_callback(phys_enc->dpu_kms,
  

• 		phys_enc->irq[INTR_IDX_PINGPONG].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_PINGPONG].func, phys_enc);
  

• dpu_core_irq_register_callback(phys_enc->dpu_kms,
  

• 		phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
  

  dpu_encoder_phys_cmd_control_vblank_irq(phys_enc, true);

  if (dpu_encoder_phys_cmd_is_master(phys_enc))

• 	dpu_encoder_helper_register_irq(phys_enc,
  

• 			INTR_IDX_CTL_START);
  

• 	dpu_core_irq_register_callback(phys_enc->dpu_kms,
  

• 			phys_enc->irq[INTR_IDX_CTL_START].irq_idx,
  

• 			phys_enc->irq[INTR_IDX_CTL_START].func, phys_enc);
  

  } else { if (dpu_encoder_phys_cmd_is_master(phys_enc))

• 	dpu_encoder_helper_unregister_irq(phys_enc,
  

• 			INTR_IDX_CTL_START);
  

• 	dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
  

• 			phys_enc->irq[INTR_IDX_CTL_START].irq_idx,
  

• 			phys_enc->irq[INTR_IDX_CTL_START].func, phys_enc);
  
  

• dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_UNDERRUN);
  

• dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
  

• 		phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
  

  dpu_encoder_phys_cmd_control_vblank_irq(phys_enc, false);

• dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_PINGPONG);
  

• dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
  

• 		phys_enc->irq[INTR_IDX_PINGPONG].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_PINGPONG].func, phys_enc);
  

  }

}

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index 437af231d6a4..7f605287a377 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -404,10 +404,13 @@ static int dpu_encoder_phys_vid_control_vblank_irq( atomic_read(&phys_enc->vblank_refcount));

if (enable && atomic_inc_return(&phys_enc->vblank_refcount) == 1)

• ret = dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_VSYNC);
  

• ret = dpu_core_irq_register_callback(phys_enc->dpu_kms,
  

• 		phys_enc->irq[INTR_IDX_VSYNC].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_VSYNC].func, phys_enc);
  

  else if (!enable && atomic_dec_return(&phys_enc->vblank_refcount) ==

• ret = dpu_encoder_helper_unregister_irq(phys_enc,
  

• 		INTR_IDX_VSYNC);
  

• ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
  

• 		phys_enc->irq[INTR_IDX_VSYNC].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_VSYNC].func, phys_enc);
  
  

end: if (ret) { @@ -539,7 +542,9 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff( DPU_ERROR_VIDENC(phys_enc, "ctl %d reset failure: %d\n", ctl->idx, rc); msm_disp_snapshot_state(drm_enc->dev);

• dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_VSYNC);
  

• dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
  

• 		phys_enc->irq[INTR_IDX_VSYNC].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_VSYNC].func, phys_enc);
  

  }

}

@@ -628,10 +633,14 @@ static void dpu_encoder_phys_vid_irq_control(struct dpu_encoder_phys *phys_enc, if (WARN_ON(ret)) return;

• dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_UNDERRUN);
  

• dpu_core_irq_register_callback(phys_enc->dpu_kms,
  

• 		phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
  

  } else { dpu_encoder_phys_vid_control_vblank_irq(phys_enc, false);

• dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_UNDERRUN);
  

• dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
  

• 		phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
  

  }

}

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index 9b74cfdf5355..557ad831e651 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -451,6 +451,8 @@ int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx, irq_idx); spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);

• trace_dpu_irq_register_success(irq_idx);
• return 0;

}

@@ -493,6 +495,8 @@ int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx,

spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);

• trace_dpu_irq_unregister_success(irq_idx);
• return 0;

}

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h index 1e2619556f93..58b7b8654543 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h @@ -167,33 +167,24 @@ TRACE_EVENT(dpu_perf_crtc_update, __entry->update_clk) );

-DECLARE_EVENT_CLASS(dpu_enc_irq_template,

• TP_PROTO(uint32_t drm_id, enum dpu_intr_idx intr_idx,

•  int irq_idx),
  

• TP_ARGS(drm_id, intr_idx, irq_idx),

+DECLARE_EVENT_CLASS(dpu_irq_template,

• TP_PROTO(int irq_idx),
• TP_ARGS(irq_idx), TP_STRUCT__entry(

• __field(	uint32_t,		drm_id		)
  

• __field(	enum dpu_intr_idx,	intr_idx	)
  

  __field( int, irq_idx ) ), TP_fast_assign(

• __entry->drm_id = drm_id;
  

• __entry->intr_idx = intr_idx;
  

  __entry->irq_idx = irq_idx; ),
• TP_printk("id=%u, intr=%d, irq=%d",

•   __entry->drm_id, __entry->intr_idx,
  

•   __entry->irq_idx)
  

• TP_printk("irq=%d", __entry->irq_idx)

); -DEFINE_EVENT(dpu_enc_irq_template, dpu_enc_irq_register_success,

• TP_PROTO(uint32_t drm_id, enum dpu_intr_idx intr_idx,

•  int irq_idx),
  

• TP_ARGS(drm_id, intr_idx, irq_idx)

+DEFINE_EVENT(dpu_irq_template, dpu_irq_register_success,

• TP_PROTO(int irq_idx),
• TP_ARGS(irq_idx)

); -DEFINE_EVENT(dpu_enc_irq_template, dpu_enc_irq_unregister_success,

• TP_PROTO(uint32_t drm_id, enum dpu_intr_idx intr_idx,

•  int irq_idx),
  

• TP_ARGS(drm_id, intr_idx, irq_idx)

+DEFINE_EVENT(dpu_irq_template, dpu_irq_unregister_success,

• TP_PROTO(int irq_idx),
• TP_ARGS(irq_idx)

);

TRACE_EVENT(dpu_enc_irq_wait_success,

On 2021-06-17 15:20, Dmitry Baryshkov wrote:

Drop the wrapping structures and the enum used to index those structures in dpu_kms. Instead of them use IRQ indices and callback functions directly.

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

Is this change really needed because I think the enum based approach is good. Even in the diff stats, the LOC reduced is not that significant. Having one dpu_encoder_irq struct with the information contained seems reasonable to me.

Can you please explain any redundancy or benefits of this approach that i am missing?

---

drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 47 +++++----- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 48 +++------- .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 94 +++++++------------ .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 53 ++++------- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 12 +-- 5 files changed, 92 insertions(+), 162 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 3d8864df8605..55ae3ede5846 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -241,11 +241,11 @@ static void _dpu_encoder_setup_dither(struct dpu_hw_pingpong *hw_pp, unsigned bp }

void dpu_encoder_helper_report_irq_timeout(struct dpu_encoder_phys *phys_enc,

• enum dpu_intr_idx intr_idx)
  

• int irq_idx)
  

{ DRM_ERROR("irq timeout id=%u, intf=%d, pp=%d, intr=%d\n", DRMID(phys_enc->parent), phys_enc->intf_idx - INTF_0,

•   phys_enc->hw_pp->idx - PINGPONG_0, intr_idx);
  

•   phys_enc->hw_pp->idx - PINGPONG_0, irq_idx);
  
  

  if (phys_enc->parent_ops->handle_frame_done) phys_enc->parent_ops->handle_frame_done(

@@ -257,75 +257,70 @@ static int dpu_encoder_helper_wait_event_timeout(int32_t drm_id, u32 irq_idx, struct dpu_encoder_wait_info *info);

int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,

• enum dpu_intr_idx intr_idx,
  

• int irq_idx, void (*irq_cb)(void *, int),
  

  struct dpu_encoder_wait_info *wait_info)

{

• struct dpu_encoder_irq *irq; u32 irq_status; int ret;

• if (!wait_info || intr_idx >= INTR_IDX_MAX) {

• if (!wait_info || irq_idx < 0) { DPU_ERROR("invalid params\n"); return -EINVAL; }

• irq = &phys_enc->irq[intr_idx];

  /* note: do master / slave checking outside */

  /* return EWOULDBLOCK since we know the wait isn't necessary */ if (phys_enc->enable_state == DPU_ENC_DISABLED) {

• DRM_ERROR("encoder is disabled id=%u, intr=%d, irq=%d",
  

• 	  DRMID(phys_enc->parent), intr_idx,
  

• 	  irq->irq_idx);
  

• DRM_ERROR("encoder is disabled id=%u, irq=%d",
  

• 	  DRMID(phys_enc->parent), irq_idx);
  

  return -EWOULDBLOCK; }

• if (irq->irq_idx < 0) {

• DRM_DEBUG_KMS("skip irq wait id=%u, intr=%d, irq=%s",
  

• 	      DRMID(phys_enc->parent), intr_idx,
  

• 	      irq->name);
  

• if (irq_idx < 0) {

• DRM_DEBUG_KMS("skip irq wait id=%u", DRMID(phys_enc->parent));
  

  return 0; }

• DRM_DEBUG_KMS("id=%u, intr=%d, irq=%d, pp=%d, pending_cnt=%d",

•       DRMID(phys_enc->parent), intr_idx,
  

•       irq->irq_idx, phys_enc->hw_pp->idx - PINGPONG_0,
  

• DRM_DEBUG_KMS("id=%u, irq=%d, pp=%d, pending_cnt=%d",

•       DRMID(phys_enc->parent),
  

•       irq_idx, phys_enc->hw_pp->idx - PINGPONG_0,
   atomic_read(wait_info->atomic_cnt));
  
  

  ret = dpu_encoder_helper_wait_event_timeout( DRMID(phys_enc->parent),

• 	irq->irq_idx,
  

• 	irq_idx,
  wait_info);
  
  

  if (ret <= 0) { irq_status = dpu_core_irq_read(phys_enc->dpu_kms,

• 		irq->irq_idx, true);
  

• 		irq_idx, true);
  

  if (irq_status) { unsigned long flags;

• 	DRM_DEBUG_KMS("irq not triggered id=%u, intr=%d, "
  

• 	DRM_DEBUG_KMS("irq not triggered id=%u, "
  	      "irq=%d, pp=%d, atomic_cnt=%d",
  

• 		      DRMID(phys_enc->parent), intr_idx,
  

• 		      irq->irq_idx,
  

• 		      DRMID(phys_enc->parent),
  

• 		      irq_idx,
  	      phys_enc->hw_pp->idx - PINGPONG_0,
  	      atomic_read(wait_info->atomic_cnt));
  local_irq_save(flags);
  

• 	irq->func(phys_enc, irq->irq_idx);
  

• 	irq_cb(phys_enc, irq_idx);
  local_irq_restore(flags);
  ret = 0;
  

  } else { ret = -ETIMEDOUT;

• 	DRM_DEBUG_KMS("irq timeout id=%u, intr=%d, "
  

• 	DRM_DEBUG_KMS("irq timeout id=%u, "
  	      "irq=%d, pp=%d, atomic_cnt=%d",
  

• 		      DRMID(phys_enc->parent), intr_idx,
  

• 		      irq->irq_idx,
  

• 		      DRMID(phys_enc->parent),
  

• 		      irq_idx,
  	      phys_enc->hw_pp->idx - PINGPONG_0,
  	      atomic_read(wait_info->atomic_cnt));
  

  } } else { ret = 0; trace_dpu_enc_irq_wait_success(DRMID(phys_enc->parent),

• 	intr_idx, irq->irq_idx,
  

• 	irq_idx,
  phys_enc->hw_pp->idx - PINGPONG_0,
  atomic_read(wait_info->atomic_cnt));
  

  }

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h index ff2218155b44..983a92d152cd 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -146,37 +146,6 @@ struct dpu_encoder_phys_ops { int (*get_frame_count)(struct dpu_encoder_phys *phys); };

-/**

• • enum dpu_intr_idx - dpu encoder interrupt index
• • @INTR_IDX_VSYNC: Vsync interrupt for video mode panel
• • @INTR_IDX_PINGPONG: Pingpong done unterrupt for cmd mode panel
• • @INTR_IDX_UNDERRUN: Underrun unterrupt for video and cmd mode panel
• • @INTR_IDX_RDPTR: Readpointer done unterrupt for cmd mode panel
• */

-enum dpu_intr_idx {

• INTR_IDX_VSYNC,
• INTR_IDX_PINGPONG,
• INTR_IDX_UNDERRUN,
• INTR_IDX_CTL_START,
• INTR_IDX_RDPTR,
• INTR_IDX_MAX,

-};

-/**

• • dpu_encoder_irq - tracking structure for interrupts
• • @name: string name of interrupt
• • @intr_idx: Encoder interrupt enumeration
• • @irq_idx: IRQ interface lookup index from DPU IRQ framework

• • 	will be -EINVAL if IRQ is not registered
    

• • @irq_cb: interrupt callback
• */

-struct dpu_encoder_irq {

• const char *name;
• enum dpu_intr_idx intr_idx;
• int irq_idx;
• void (*func)(void *arg, int irq_idx);

-};

/**

• struct dpu_encoder_phys - physical encoder that drives a single

INTF block

• tied to a specific panel / sub-panel. Abstract type, sub-classed by

@@ -231,7 +200,13 @@ struct dpu_encoder_phys { atomic_t pending_ctlstart_cnt; atomic_t pending_kickoff_cnt; wait_queue_head_t pending_kickoff_wq;

• struct dpu_encoder_irq irq[INTR_IDX_MAX];

• int intf_underrun_irq;
• int vblank_irq;
• /* for CMD only */
• int ctl_start_irq;
• int pp_done_irq;

};

static inline int dpu_encoder_phys_inc_pending(struct dpu_encoder_phys *phys) @@ -347,21 +322,22 @@ void dpu_encoder_helper_split_config(

• dpu_encoder_helper_report_irq_timeout - utility to report error

that irq has

• timed out, including reporting frame error event to crtc and debug

dump

• @phys_enc: Pointer to physical encoder structure

• • @intr_idx: Failing interrupt index

• • @irq_idx: Failing interrupt index
  */

void dpu_encoder_helper_report_irq_timeout(struct dpu_encoder_phys *phys_enc,

• enum dpu_intr_idx intr_idx);
  

• int irq_idx);
  
  

/**

• dpu_encoder_helper_wait_for_irq - utility to wait on an irq.
• note: will call dpu_encoder_helper_wait_for_irq on timeout
• @phys_enc: Pointer to physical encoder structure

• • @intr_idx: encoder interrupt index

• • @irq_idx: encoder interrupt index
• • @irq_cb: encoder interrupt callback
  • @wait_info: wait info struct
  • @Return: 0 or -ERROR
  */

int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,

• enum dpu_intr_idx intr_idx,
  

• int irq_idx, void (*irq_cb)(void *, int),
  

  struct dpu_encoder_wait_info *wait_info);

#endif /* __dpu_encoder_phys_H__ */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 4bfeac821f51..122364a4ef54 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -151,7 +151,6 @@ static void dpu_encoder_phys_cmd_mode_set( { struct dpu_encoder_phys_cmd *cmd_enc = to_dpu_encoder_phys_cmd(phys_enc);

• struct dpu_encoder_irq *irq;

  if (!mode || !adj_mode) { DPU_ERROR("invalid args\n");

@@ -161,17 +160,10 @@ static void dpu_encoder_phys_cmd_mode_set( DPU_DEBUG_CMDENC(cmd_enc, "caching mode:\n"); drm_mode_debug_printmodeline(adj_mode);

• irq = &phys_enc->irq[INTR_IDX_CTL_START];
• irq->irq_idx = phys_enc->hw_ctl->caps->intr_start;
• irq = &phys_enc->irq[INTR_IDX_PINGPONG];
• irq->irq_idx = phys_enc->hw_pp->caps->intr_done;
• irq = &phys_enc->irq[INTR_IDX_RDPTR];
• irq->irq_idx = phys_enc->hw_pp->caps->intr_rdptr;
• irq = &phys_enc->irq[INTR_IDX_UNDERRUN];
• irq->irq_idx = phys_enc->hw_intf->cap->intr_underrun;

• phys_enc->ctl_start_irq = phys_enc->hw_ctl->caps->intr_start;
• phys_enc->pp_done_irq = phys_enc->hw_pp->caps->intr_done;
• phys_enc->vblank_irq = phys_enc->hw_pp->caps->intr_rdptr;
• phys_enc->intf_underrun_irq = phys_enc->hw_intf->cap->intr_underrun;

}

static int _dpu_encoder_phys_cmd_handle_ppdone_timeout( @@ -212,8 +204,8 @@ static int _dpu_encoder_phys_cmd_handle_ppdone_timeout( atomic_read(&phys_enc->pending_kickoff_cnt)); msm_disp_snapshot_state(drm_enc->dev); dpu_core_irq_unregister_callback(phys_enc->dpu_kms,

• 		phys_enc->irq[INTR_IDX_RDPTR].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_RDPTR].func, phys_enc);
  

• 		phys_enc->vblank_irq,
  

• 		dpu_encoder_phys_cmd_pp_rd_ptr_irq, phys_enc);
  

  }

  atomic_add_unless(&phys_enc->pending_kickoff_cnt, -1, 0);

@@ -240,7 +232,9 @@ static int _dpu_encoder_phys_cmd_wait_for_idle( wait_info.atomic_cnt = &phys_enc->pending_kickoff_cnt; wait_info.timeout_ms = KICKOFF_TIMEOUT_MS;

• ret = dpu_encoder_helper_wait_for_irq(phys_enc, INTR_IDX_PINGPONG,

• ret = dpu_encoder_helper_wait_for_irq(phys_enc,

• 	phys_enc->pp_done_irq,
  

• 	dpu_encoder_phys_cmd_pp_tx_done_irq,
  &wait_info);
  

  if (ret == -ETIMEDOUT) _dpu_encoder_phys_cmd_handle_ppdone_timeout(phys_enc);

@@ -280,12 +274,12 @@ static int dpu_encoder_phys_cmd_control_vblank_irq(

if (enable && atomic_inc_return(&phys_enc->vblank_refcount) == 1) ret = dpu_core_irq_register_callback(phys_enc->dpu_kms,

• 		phys_enc->irq[INTR_IDX_RDPTR].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_RDPTR].func, phys_enc);
  

• 		phys_enc->vblank_irq,
  

• 		dpu_encoder_phys_cmd_pp_rd_ptr_irq, phys_enc);
  

  else if (!enable && atomic_dec_return(&phys_enc->vblank_refcount) ==

0. ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms,

• 		phys_enc->irq[INTR_IDX_RDPTR].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_RDPTR].func, phys_enc);
  

• 		phys_enc->vblank_irq,
  

• 		dpu_encoder_phys_cmd_pp_rd_ptr_irq, phys_enc);
  
  

end: if (ret) { @@ -307,30 +301,30 @@ static void dpu_encoder_phys_cmd_irq_control(struct dpu_encoder_phys *phys_enc,

if (enable) { dpu_core_irq_register_callback(phys_enc->dpu_kms,

• 		phys_enc->irq[INTR_IDX_PINGPONG].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_PINGPONG].func, phys_enc);
  

• 		phys_enc->pp_done_irq,
  

• 		dpu_encoder_phys_cmd_pp_tx_done_irq, phys_enc);
  

  dpu_core_irq_register_callback(phys_enc->dpu_kms,

• 		phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
  

• 		phys_enc->intf_underrun_irq,
  

• 		dpu_encoder_phys_cmd_underrun_irq, phys_enc);
  

  dpu_encoder_phys_cmd_control_vblank_irq(phys_enc, true);

  if (dpu_encoder_phys_cmd_is_master(phys_enc)) dpu_core_irq_register_callback(phys_enc->dpu_kms,

• 			phys_enc->irq[INTR_IDX_CTL_START].irq_idx,
  

• 			phys_enc->irq[INTR_IDX_CTL_START].func, phys_enc);
  

• 			phys_enc->ctl_start_irq,
  

• 			dpu_encoder_phys_cmd_ctl_start_irq, phys_enc);
  

  } else { if (dpu_encoder_phys_cmd_is_master(phys_enc)) dpu_core_irq_unregister_callback(phys_enc->dpu_kms,

• 			phys_enc->irq[INTR_IDX_CTL_START].irq_idx,
  

• 			phys_enc->irq[INTR_IDX_CTL_START].func, phys_enc);
  

• 			phys_enc->ctl_start_irq,
  

• 			dpu_encoder_phys_cmd_ctl_start_irq, phys_enc);
  
  

  dpu_core_irq_unregister_callback(phys_enc->dpu_kms,

• 		phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
  

• 		phys_enc->intf_underrun_irq,
  

• 		dpu_encoder_phys_cmd_underrun_irq, phys_enc);
  

  dpu_encoder_phys_cmd_control_vblank_irq(phys_enc, false); dpu_core_irq_unregister_callback(phys_enc->dpu_kms,

• 		phys_enc->irq[INTR_IDX_PINGPONG].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_PINGPONG].func, phys_enc);
  

• 		phys_enc->pp_done_irq,
  

• 		dpu_encoder_phys_cmd_pp_tx_done_irq, phys_enc);
  

  }

}

@@ -664,7 +658,9 @@ static int _dpu_encoder_phys_cmd_wait_for_ctl_start( wait_info.atomic_cnt = &phys_enc->pending_ctlstart_cnt; wait_info.timeout_ms = KICKOFF_TIMEOUT_MS;

• ret = dpu_encoder_helper_wait_for_irq(phys_enc, INTR_IDX_CTL_START,

• ret = dpu_encoder_helper_wait_for_irq(phys_enc,

• 	phys_enc->ctl_start_irq,
  

• 	dpu_encoder_phys_cmd_ctl_start_irq,
  &wait_info);
  

  if (ret == -ETIMEDOUT) { DPU_ERROR_CMDENC(cmd_enc, "ctl start interrupt wait failed\n");

@@ -719,7 +715,9 @@ static int dpu_encoder_phys_cmd_wait_for_vblank(

atomic_inc(&cmd_enc->pending_vblank_cnt);

• rc = dpu_encoder_helper_wait_for_irq(phys_enc, INTR_IDX_RDPTR,

• rc = dpu_encoder_helper_wait_for_irq(phys_enc,

• 	phys_enc->vblank_irq,
  

• 	dpu_encoder_phys_cmd_pp_rd_ptr_irq,
  &wait_info);
  
  

  return rc;

@@ -771,8 +769,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( { struct dpu_encoder_phys *phys_enc = NULL; struct dpu_encoder_phys_cmd *cmd_enc = NULL;

• struct dpu_encoder_irq *irq;
• int i, ret = 0;

• int ret = 0;

  DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0);

@@ -795,30 +792,11 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( phys_enc->enc_spinlock = p->enc_spinlock; cmd_enc->stream_sel = 0; phys_enc->enable_state = DPU_ENC_DISABLED;

• for (i = 0; i < INTR_IDX_MAX; i++) {

• irq = &phys_enc->irq[i];
  

• irq->irq_idx = -EINVAL;
  

• }

• irq = &phys_enc->irq[INTR_IDX_CTL_START];

• irq->name = "ctl_start";

• irq->intr_idx = INTR_IDX_CTL_START;

• irq->func = dpu_encoder_phys_cmd_ctl_start_irq;

• irq = &phys_enc->irq[INTR_IDX_PINGPONG];

• irq->name = "pp_done";

• irq->intr_idx = INTR_IDX_PINGPONG;

• irq->func = dpu_encoder_phys_cmd_pp_tx_done_irq;

• irq = &phys_enc->irq[INTR_IDX_RDPTR];

• irq->name = "pp_rd_ptr";

• irq->intr_idx = INTR_IDX_RDPTR;

• irq->func = dpu_encoder_phys_cmd_pp_rd_ptr_irq;

• irq = &phys_enc->irq[INTR_IDX_UNDERRUN];

• irq->name = "underrun";

• irq->intr_idx = INTR_IDX_UNDERRUN;

• irq->func = dpu_encoder_phys_cmd_underrun_irq;

• phys_enc->ctl_start_irq = -EINVAL;

• phys_enc->pp_done_irq = -EINVAL;

• phys_enc->vblank_irq = -EINVAL;

• phys_enc->intf_underrun_irq = -EINVAL;

  atomic_set(&phys_enc->vblank_refcount, 0); atomic_set(&phys_enc->pending_kickoff_cnt, 0);

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index 7f605287a377..19f728e63d7d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -366,19 +366,14 @@ static void dpu_encoder_phys_vid_mode_set( struct drm_display_mode *mode, struct drm_display_mode *adj_mode) {

• struct dpu_encoder_irq *irq;

• if (adj_mode) { phys_enc->cached_mode = *adj_mode; drm_mode_debug_printmodeline(adj_mode); DPU_DEBUG_VIDENC(phys_enc, "caching mode:\n"); }

• irq = &phys_enc->irq[INTR_IDX_VSYNC];

• irq->irq_idx = phys_enc->hw_intf->cap->intr_vsync;

• irq = &phys_enc->irq[INTR_IDX_UNDERRUN];

• irq->irq_idx = phys_enc->hw_intf->cap->intr_underrun;

• phys_enc->vblank_irq = phys_enc->hw_intf->cap->intr_vsync;
• phys_enc->intf_underrun_irq = phys_enc->hw_intf->cap->intr_underrun;

}

static int dpu_encoder_phys_vid_control_vblank_irq( @@ -405,12 +400,12 @@ static int dpu_encoder_phys_vid_control_vblank_irq(

if (enable && atomic_inc_return(&phys_enc->vblank_refcount) == 1) ret = dpu_core_irq_register_callback(phys_enc->dpu_kms,

• 		phys_enc->irq[INTR_IDX_VSYNC].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_VSYNC].func, phys_enc);
  

• 		phys_enc->vblank_irq,
  

• 		dpu_encoder_phys_vid_vblank_irq, phys_enc);
  

  else if (!enable && atomic_dec_return(&phys_enc->vblank_refcount) ==

0. ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms,

• 		phys_enc->irq[INTR_IDX_VSYNC].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_VSYNC].func, phys_enc);
  

• 		phys_enc->vblank_irq,
  

• 		dpu_encoder_phys_vid_vblank_irq, phys_enc);
  
  

end: if (ret) { @@ -490,11 +485,13 @@ static int dpu_encoder_phys_vid_wait_for_vblank( }

/* Wait for kickoff to complete */

• ret = dpu_encoder_helper_wait_for_irq(phys_enc, INTR_IDX_VSYNC,

• ret = dpu_encoder_helper_wait_for_irq(phys_enc,

• 	phys_enc->vblank_irq,
  

• 	dpu_encoder_phys_vid_vblank_irq,
  &wait_info);
  
  

  if (ret == -ETIMEDOUT) {

• dpu_encoder_helper_report_irq_timeout(phys_enc, INTR_IDX_VSYNC);
  

• dpu_encoder_helper_report_irq_timeout(phys_enc, 
  

phys_enc->vblank_irq); }

return ret; @@ -543,8 +540,8 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff( ctl->idx, rc); msm_disp_snapshot_state(drm_enc->dev); dpu_core_irq_unregister_callback(phys_enc->dpu_kms,

• 		phys_enc->irq[INTR_IDX_VSYNC].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_VSYNC].func, phys_enc);
  

• 		phys_enc->vblank_irq,
  

• 		dpu_encoder_phys_vid_vblank_irq, phys_enc);
  

  }

}

@@ -634,13 +631,13 @@ static void dpu_encoder_phys_vid_irq_control(struct dpu_encoder_phys *phys_enc, return;

dpu_core_irq_register_callback(phys_enc->dpu_kms,

• 		phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
  

• 		phys_enc->intf_underrun_irq,
  

• 		dpu_encoder_phys_vid_underrun_irq, phys_enc);
  

  } else { dpu_encoder_phys_vid_control_vblank_irq(phys_enc, false); dpu_core_irq_unregister_callback(phys_enc->dpu_kms,

• 		phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
  

• 		phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
  

• 		phys_enc->intf_underrun_irq,
  

• 		dpu_encoder_phys_vid_underrun_irq, phys_enc);
  

  }

}

@@ -706,8 +703,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init( struct dpu_enc_phys_init_params *p) { struct dpu_encoder_phys *phys_enc = NULL;

• struct dpu_encoder_irq *irq;
• int i, ret = 0;

• int ret = 0;

  if (!p) { ret = -EINVAL;

@@ -732,20 +728,9 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init( phys_enc->split_role = p->split_role; phys_enc->intf_mode = INTF_MODE_VIDEO; phys_enc->enc_spinlock = p->enc_spinlock;

• for (i = 0; i < INTR_IDX_MAX; i++) {

• irq = &phys_enc->irq[i];
  

• irq->irq_idx = -EINVAL;
  

• }

• irq = &phys_enc->irq[INTR_IDX_VSYNC];

• irq->name = "vsync_irq";

• irq->intr_idx = INTR_IDX_VSYNC;

• irq->func = dpu_encoder_phys_vid_vblank_irq;

• irq = &phys_enc->irq[INTR_IDX_UNDERRUN];

• irq->name = "underrun";

• irq->intr_idx = INTR_IDX_UNDERRUN;

• irq->func = dpu_encoder_phys_vid_underrun_irq;

• phys_enc->vblank_irq = -EINVAL;

• phys_enc->intf_underrun_irq = -EINVAL;

  atomic_set(&phys_enc->vblank_refcount, 0); atomic_set(&phys_enc->pending_kickoff_cnt, 0);

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h index 58b7b8654543..648124e8ea2f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h @@ -188,26 +188,22 @@ DEFINE_EVENT(dpu_irq_template, dpu_irq_unregister_success, );

TRACE_EVENT(dpu_enc_irq_wait_success,

• TP_PROTO(uint32_t drm_id, enum dpu_intr_idx intr_idx,

•  int irq_idx, enum dpu_pingpong pp_idx, int atomic_cnt),
  

• TP_ARGS(drm_id, intr_idx, irq_idx, pp_idx, atomic_cnt),

• TP_PROTO(uint32_t drm_id, int irq_idx, enum dpu_pingpong pp_idx, int

atomic_cnt),

• TP_ARGS(drm_id, irq_idx, pp_idx, atomic_cnt), TP_STRUCT__entry( __field( uint32_t, drm_id )

• __field(	enum dpu_intr_idx,	intr_idx	)
  

  __field( int, irq_idx ) __field( enum dpu_pingpong, pp_idx ) __field( int, atomic_cnt ) ), TP_fast_assign( __entry->drm_id = drm_id;

• __entry->intr_idx = intr_idx;
  

  __entry->irq_idx = irq_idx; __entry->pp_idx = pp_idx; __entry->atomic_cnt = atomic_cnt; ),
• TP_printk("id=%u, intr=%d, irq=%d, pp=%d, atomic_cnt=%d",

•   __entry->drm_id, __entry->intr_idx,
  

•   __entry->irq_idx, __entry->pp_idx, __entry->atomic_cnt)
  

• TP_printk("id=%u, irq=%d, pp=%d, atomic_cnt=%d",

•   __entry->drm_id, __entry->irq_idx, __entry->pp_idx, 
  

__entry->atomic_cnt) );

DECLARE_EVENT_CLASS(dpu_drm_obj_template,