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

PATCH Superseded Shubhashree Dhar 2 participants 2 messages Nov 14, 2019

Subsystems (5)

DRM drivers DRM misc Arm Qualcomm mailing list DRM driver for Qualcomm display hardware +1 more subsystems

lore

Share & tools

lore

Shubhashree Dhar

author

2019-11-14 05:48 UTC #0 lore raw

PATCH 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

Signed-off-by Shubhashree Dhar <[email protected]>

---
 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 +
 drivers/gpu/drm/panel/panel-visionox-rm69299.c    | 478 ++++++++++++++++++++++
 5 files changed, 500 insertions(+), 5 deletions(-)
 create mode 100644 drivers/gpu/drm/panel/panel-visionox-rm69299.c

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;
 };
 
 /**

drivers/gpu/drm/panel/panel-visionox-rm69299.c new file +478 -0

478 lines not on this page · raw message

Signature · 6 lines

1.9.1

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

Jordan Crouse

2019-11-14 17:39 UTC #1 lore raw

Quoted 42 lines · Shubhashree Dhar

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
Signed-off-by Shubhashree Dhar <[email protected]>
---
 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 +
 drivers/gpu/drm/panel/panel-visionox-rm69299.c    | 478 ++++++++++++++++++++++
 5 files changed, 500 insertions(+), 5 deletions(-)
 create mode 100644 drivers/gpu/drm/panel/panel-visionox-rm69299.c
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 +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
  */
 struct dpu_mdss_cfg {
 	u32 hwversion;

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h struct dpu_mdss_cfg { · line 689

Quoted 5 lines

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +2 -0

@@ -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));

This is a very round about way of declaring an unsigned long. Do you ever expect to have more than a long's worth of interrupt bits? If not, then just an unsigned long mdss_irqs would be the preferred less macro-y way of doing this.

Quoted 8 lines

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +0 -0
 };
 
 struct dpu_mdss_hw_cfg_handler {
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +0 -0
index 8bfa7d0..2a3634c 100644

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr *intr, · line 805

Quoted 7 lines

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +2 -1

@@ -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))

This last one will always be true since end_idx is always 32 bigger than start_idx. If you add the start_idx check you no longer need the end_idx check.

Quoted 31 lines

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +10 -4

 			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();

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c static void dpu_hw_intr_get_interrupt_statuses(struct dpu_hw_intr *intr) · line 1001

Quoted 4 lines

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +1 -0

@@ -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++) {
+

This extra blank line isn't strictly needed if you don't want it.

Quoted 18 lines

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +4 -0

+		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];
 
 	return intr;

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h +0 -0

index 4edcf40..fc9c986 100644

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h struct dpu_hw_intr { · line 190

Quoted 4 lines

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h +1 -0

@@ -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;

This plus the chunk above would imply that, no, you never expect more than a long's worth of interrupt bits.

drivers/gpu/drm/panel/panel-visionox-rm69299.c

Quoted 9 lines

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h +0 -0
 	spin_lock_init(&intr->irq_lock);
 };
 
/**
drivers/gpu/drm/panel/panel-visionox-rm69299.c new file +0 -0
new file mode 100644
index 00000000..1bbd40d

Do you mean this file to be in this patch? The commit log doesn't make mention of it and it doesn't seem to be related.

<snip>

Jordan

Signature · 6 lines

The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel