Skip to contentSkip to conversation

msm: disp: dpu1: add support to access hw irqs regs depending on revision

PATCH Superseded Shubhashree Dhar 3 participants 4 messages Nov 14, 2019 – Nov 19, 2019 1 message not in the archive

Subsystems (3)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 msm: disp: dpu1: add support to access hw irqs regs depending on revision

Current code assumes that all the irqs registers offsets can be accessed in all the hw revisions; this is not the case for some targets that should not access some of the irq registers. This change adds the support to selectively remove the irqs that are not supported in some of the hw revisions.

Change-Id I6052b8237b703a1a9edd53893e04f7bd72223da1
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    |  1 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |  3 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 22 +++++++++++++++++-----
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h |  1 +
 4 files changed, 22 insertions(+), 5 deletions(-)
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +1 -0
index 04c8c44..357e15b 100644
@@ -421,6 +421,7 @@ static void sdm845_cfg_init(struct dpu_mdss_cfg *dpu_cfg)
 		.reg_dma_count = 1,
 		.dma_cfg = sdm845_regdma,
 		.perf = sdm845_perf_data,
+		.mdss_irqs[0] = 0x3ff,
 	};
 }
 
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +3 -0
index ec76b868..def8a3f 100644
@@ -646,6 +646,7 @@ struct dpu_perf_cfg {
  * @dma_formats        Supported formats for dma pipe
  * @cursor_formats     Supported formats for cursor pipe
  * @vig_formats        Supported formats for vig pipe
+ * @mdss_irqs          Bitmap with the irqs supported by the target
  */
 struct dpu_mdss_cfg {
 	u32 hwversion;
@@ -684,6 +685,8 @@ struct dpu_mdss_cfg {
 	struct dpu_format_extended *dma_formats;
 	struct dpu_format_extended *cursor_formats;
 	struct dpu_format_extended *vig_formats;
+
+	DECLARE_BITMAP(mdss_irqs, BITS_PER_BYTE * sizeof(long));
 };
 
 struct dpu_mdss_hw_cfg_handler {
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +17 -5
index 8bfa7d0..2a3634c 100644
@@ -800,7 +800,8 @@ static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr *intr,
 		start_idx = reg_idx * 32;
 		end_idx = start_idx + 32;
 
-		if (start_idx >= ARRAY_SIZE(dpu_irq_map) ||
+		if (!test_bit(reg_idx, &intr->irq_mask) ||
+			start_idx >= ARRAY_SIZE(dpu_irq_map) ||
 				end_idx > ARRAY_SIZE(dpu_irq_map))
 			continue;
 
@@ -955,8 +956,11 @@ static int dpu_hw_intr_clear_irqs(struct dpu_hw_intr *intr)
 	if (!intr)
 		return -EINVAL;
 
-	for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++)
-		DPU_REG_WRITE(&intr->hw, dpu_intr_set[i].clr_off, 0xffffffff);
+	for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) {
+		if(test_bit(i, &intr->irq_mask))
+			DPU_REG_WRITE(&intr->hw,
+					dpu_intr_set[i].clr_off, 0xffffffff);
+	}
 
 	/* ensure register writes go through */
 	wmb();
@@ -971,8 +975,11 @@ static int dpu_hw_intr_disable_irqs(struct dpu_hw_intr *intr)
 	if (!intr)
 		return -EINVAL;
 
-	for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++)
-		DPU_REG_WRITE(&intr->hw, dpu_intr_set[i].en_off, 0x00000000);
+	for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) {
+		if(test_bit(i, &intr->irq_mask))
+			DPU_REG_WRITE(&intr->hw,
+					dpu_intr_set[i].en_off, 0x00000000);
+	}
 
 	/* ensure register writes go through */
 	wmb();
@@ -991,6 +998,10 @@ static void dpu_hw_intr_get_interrupt_statuses(struct dpu_hw_intr *intr)
 
 	spin_lock_irqsave(&intr->irq_lock, irq_flags);
 	for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) {
+
+		if(!test_bit(i, &intr->irq_mask))
+			continue;
+
 		/* Read interrupt status */
 		intr->save_irq_status[i] = DPU_REG_READ(&intr->hw,
 				dpu_intr_set[i].status_off);
@@ -1115,6 +1126,7 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr,
 		return ERR_PTR(-ENOMEM);
 	}
 
+	intr->irq_mask = m->mdss_irqs[0];
 	spin_lock_init(&intr->irq_lock);
 
 	return intr;
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h +1 -0
index 4edcf40..fc9c986 100644
@@ -187,6 +187,7 @@ struct dpu_hw_intr {
 	u32 *save_irq_status;
 	u32 irq_idx_tbl_size;
 	spinlock_t irq_lock;
+	unsigned long irq_mask;
 };
 
 /**
Signature · 1 line
1.9.1
#1 lore raw

Quoting Shubhashree Dhar (2019-11-13 21:56:16)

Quoted 3 lines

Current code assumes that all the irqs registers offsets can be accessed in all the hw revisions; this is not the case for some targets that should not access some of the irq registers.

What happens if we read the irq registers that we "should not access"? Does the system reset? It would be easier to make those registers return 0 when read indicating no interrupt and ignore writes so that everything keeps working without having to skip registers.

Quoted 4 lines

This change adds the support to selectively remove the irqs that are not supported in some of the hw revisions.

Change-Id: I6052b8237b703a1a9edd53893e04f7bd72223da1

Please remove these before sending upstream.

Quoted 13 lines
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    |  1 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |  3 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 22 +++++++++++++++++-----
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h |  1 +
 4 files changed, 22 insertions(+), 5 deletions(-)
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +0 -0
index ec76b868..def8a3f 100644

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h struct dpu_perf_cfg { · line 649

Quoted 4 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +1 -0
@@ -646,6 +646,7 @@ struct dpu_perf_cfg {
  * @dma_formats        Supported formats for dma pipe
  * @cursor_formats     Supported formats for cursor pipe
  * @vig_formats        Supported formats for vig pipe
+ * @mdss_irqs          Bitmap with the irqs supported by the target

Hmm pretty sure there needs to be a colon so that kernel-doc can match this but maybe I'm wrong.

Quoted 3 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +0 -0
  */
 struct dpu_mdss_cfg {
        u32 hwversion;
#2 lore raw
Quoted 10 lines · Stephen Boyd

Quoting Shubhashree Dhar (2019-11-13 21:56:16)

Quoted 3 lines

Current code assumes that all the irqs registers offsets can be accessed in all the hw revisions; this is not the case for some targets that should not access some of the irq registers.

What happens if we read the irq registers that we "should not access"? Does the system reset? It would be easier to make those registers return 0 when read indicating no interrupt and ignore writes so that everything keeps working without having to skip registers.

In some of the hw revisions, the whole hw block is absent and trying to access those registers causes system panic(bus noc error).

Quoted 32 lines
Quoted 4 lines

This change adds the support to selectively remove the irqs that are not supported in some of the hw revisions.

Change-Id: I6052b8237b703a1a9edd53893e04f7bd72223da1

Please remove these before sending upstream.

Quoted 14 lines
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    |  1 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |  3 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 22 
+++++++++++++++++-----
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h |  1 +
 4 files changed, 22 insertions(+), 5 deletions(-)
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +0 -0
index ec76b868..def8a3f 100644

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h struct dpu_perf_cfg { · line 649

Quoted 4 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +1 -0
@@ -646,6 +646,7 @@ struct dpu_perf_cfg {
  * @dma_formats        Supported formats for dma pipe
  * @cursor_formats     Supported formats for cursor pipe
  * @vig_formats        Supported formats for vig pipe
+ * @mdss_irqs          Bitmap with the irqs supported by the target

Hmm pretty sure there needs to be a colon so that kernel-doc can match this but maybe I'm wrong.

Quoted 3 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +0 -0
  */
 struct dpu_mdss_cfg {
        u32 hwversion;

A message that is not in the archive (sent off-list or not yet received): 5dcd8f05.1c69fb81.bdd4.2b0a-ATjtLOhZ0NVl57MIdRCFDg@public.gmane.org

#4 lore raw
Quoted 10 lines · Stephen Boyd

Quoting Shubhashree Dhar (2019-11-13 21:56:16)

Quoted 3 lines

Current code assumes that all the irqs registers offsets can be accessed in all the hw revisions; this is not the case for some targets that should not access some of the irq registers.

What happens if we read the irq registers that we "should not access"? Does the system reset? It would be easier to make those registers return 0 when read indicating no interrupt and ignore writes so that everything keeps working without having to skip registers.

In some of the hw revisions, the whole hw block is absent and trying to access those registers causes system panic(bus noc error).

Quoted 16 lines
Quoted 4 lines

This change adds the support to selectively remove the irqs that are not supported in some of the hw revisions.

Change-Id: I6052b8237b703a1a9edd53893e04f7bd72223da1

Please remove these before sending upstream.

Quoted 9 lines
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    |  1 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |  3 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 22 
+++++++++++++++++-----
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h |  1 +
 4 files changed, 22 insertions(+), 5 deletions(-)
Quoted 16 lines

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h struct dpu_perf_cfg { · line 649

Quoted 9 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +1 -0
index ec76b868..def8a3f 100644
@@ -646,6 +646,7 @@ struct dpu_perf_cfg {
  * @dma_formats        Supported formats for dma pipe
  * @cursor_formats     Supported formats for cursor pipe
  * @vig_formats        Supported formats for vig pipe
+ * @mdss_irqs          Bitmap with the irqs supported by the target

Hmm pretty sure there needs to be a colon so that kernel-doc can match this but maybe I'm wrong.

Quoted 3 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +0 -0
  */
 struct dpu_mdss_cfg {
        u32 hwversion;

_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno