Re: [PATCH v2 3/4] drm/msm/dpu: add helper to get IRQ-related data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

On 27/07/2023 22:41, Marijn Suijten wrote:

On 2023-07-27 22:34:59, Dmitry Baryshkov wrote:

On 27/07/2023 22:29, Marijn Suijten wrote:

On 2023-07-27 18:04:54, Dmitry Baryshkov wrote:

In preparation to reworking the IRQ indices, move irq_tbl access to
separate helper.


I am not seeing the advantage of the helper, but making every function
look up dpu_kms->hw_intr->irq_tbl[irq_idx] only once and storing that in
a local dpu_hw_intr_entry pointer is much tidier.


There was a bonus point when I tried to do a irq_idx-1 in the next
patch. But since that code has gone, maybe I can drop this patch too.


Don't drop the whole patch though.  While maybe not necessary, having
the lookup only once is much easier to follow.


Then it's easier to keep it as is.



- Marijn


Maybe I expected it to do extra mutations to irq_idx in 4/4?


Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>


Reviewed-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>


---
   .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 48 +++++++++++++------
   .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 12 +++--
   2 files changed, 41 insertions(+), 19 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 eaae7f11f57f..ede7161ae904 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -199,6 +199,12 @@ static const struct dpu_intr_reg dpu_intr_set_7xxx[] = {
#define DPU_IRQ_MASK(irq_idx) (BIT(DPU_IRQ_OFFSET(irq_idx))) +static inline struct dpu_hw_intr_entry *dpu_core_irq_get_entry(struct dpu_kms *dpu_kms, 
+							       int irq_idx)
+{
+	return &dpu_kms->hw_intr->irq_tbl[irq_idx];
+}
+
   /**
    * dpu_core_irq_callback_handler - dispatch core interrupts
    * @dpu_kms:		Pointer to DPU's KMS structure
@@ -206,17 +212,19 @@ static const struct dpu_intr_reg dpu_intr_set_7xxx[] = {
    */
   static void dpu_core_irq_callback_handler(struct dpu_kms *dpu_kms, int irq_idx)
   {
+	struct dpu_hw_intr_entry *irq_entry = dpu_core_irq_get_entry(dpu_kms, irq_idx);
+
   	VERB("irq_idx=%d\n", irq_idx);
- if (!dpu_kms->hw_intr->irq_tbl[irq_idx].cb) 
+	if (!irq_entry->cb)
   		DRM_ERROR("no registered cb, idx:%d\n", irq_idx);
- atomic_inc(&dpu_kms->hw_intr->irq_tbl[irq_idx].count); 
+	atomic_inc(&irq_entry->count);
/* 
   	 * Perform registered function callback
   	 */
-	dpu_kms->hw_intr->irq_tbl[irq_idx].cb(dpu_kms->hw_intr->irq_tbl[irq_idx].arg);
+	irq_entry->cb(irq_entry->arg);
   }
irqreturn_t dpu_core_irq(struct msm_kms *kms) 
@@ -509,6 +517,7 @@ int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx,
   		void (*irq_cb)(void *arg),
   		void *irq_arg)
   {
+	struct dpu_hw_intr_entry *irq_entry;
   	unsigned long irq_flags;
   	int ret;
@@ -526,15 +535,16 @@ int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx, spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags); - if (unlikely(WARN_ON(dpu_kms->hw_intr->irq_tbl[irq_idx].cb))) { 
+	irq_entry = dpu_core_irq_get_entry(dpu_kms, irq_idx);
+	if (unlikely(WARN_ON(irq_entry->cb))) {
   		spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
return -EBUSY; 
   	}
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;
+	irq_entry->arg = irq_arg;
+	irq_entry->cb = irq_cb;
ret = dpu_hw_intr_enable_irq_locked( 
   				dpu_kms->hw_intr,
@@ -551,6 +561,7 @@ int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx,
int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx) 
   {
+	struct dpu_hw_intr_entry *irq_entry;
   	unsigned long irq_flags;
   	int ret;
@@ -569,8 +580,9 @@ int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx) 
   		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;
+	irq_entry = dpu_core_irq_get_entry(dpu_kms, irq_idx);
+	irq_entry->cb = NULL;
+	irq_entry->arg = NULL;
spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags); @@ -583,14 +595,16 @@ 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_hw_intr_entry *irq_entry;
   	unsigned long irq_flags;
   	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);
-		irq_count = atomic_read(&dpu_kms->hw_intr->irq_tbl[i].count);
-		cb = dpu_kms->hw_intr->irq_tbl[i].cb;
+		irq_entry = dpu_core_irq_get_entry(dpu_kms, i);
+		irq_count = atomic_read(&irq_entry->count);
+		cb = irq_entry->cb;
   		spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
if (irq_count || cb) 
@@ -613,6 +627,7 @@ void dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms,
   void dpu_core_irq_preinstall(struct msm_kms *kms)
   {
   	struct dpu_kms *dpu_kms = to_dpu_kms(kms);
+	struct dpu_hw_intr_entry *irq_entry;
   	int i;
pm_runtime_get_sync(&dpu_kms->pdev->dev); 
@@ -620,22 +635,27 @@ void dpu_core_irq_preinstall(struct msm_kms *kms)
   	dpu_disable_all_irqs(dpu_kms);
   	pm_runtime_put_sync(&dpu_kms->pdev->dev);
- for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) 
-		atomic_set(&dpu_kms->hw_intr->irq_tbl[i].count, 0);
+	for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) {
+		irq_entry = dpu_core_irq_get_entry(dpu_kms, i);
+		atomic_set(&irq_entry->count, 0);
+	}
   }
void dpu_core_irq_uninstall(struct msm_kms *kms) 
   {
   	struct dpu_kms *dpu_kms = to_dpu_kms(kms);
+	struct dpu_hw_intr_entry *irq_entry;
   	int i;
if (!dpu_kms->hw_intr) 
   		return;
pm_runtime_get_sync(&dpu_kms->pdev->dev); 
-	for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++)
-		if (dpu_kms->hw_intr->irq_tbl[i].cb)
+	for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) {
+		irq_entry = dpu_core_irq_get_entry(dpu_kms, i);
+		if (irq_entry->cb)
   			DPU_ERROR("irq_idx=%d still enabled/registered\n", i);
+	}
dpu_clear_irqs(dpu_kms); 
   	dpu_disable_all_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 3a988a4e4f33..59bde8ab50c8 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,12 @@ enum dpu_hw_intr_reg {
   #define DPU_IRQ_REG(irq_idx)		((irq_idx) / 32)
   #define DPU_IRQ_OFFSET(irq_idx)		((irq_idx) % 32)
+struct dpu_hw_intr_entry { 
+	void (*cb)(void *arg);
+	void *arg;
+	atomic_t count;
+};
+
   /**
    * struct dpu_hw_intr: hw interrupts handling data structure
    * @hw:               virtual address mapping
@@ -59,11 +65,7 @@ struct dpu_hw_intr {
   	unsigned long irq_mask;
   	const struct dpu_intr_reg *intr_set;
- struct { 
-		void (*cb)(void *arg);
-		void *arg;
-		atomic_t count;
-	} irq_tbl[];
+	struct dpu_hw_intr_entry irq_tbl[];
   };
/** 
--
2.39.2



--
With best wishes
Dmitry



--
With best wishes
Dmitry
[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux