Re: [RFC PATCH 5/7] drm/msm/dpu: Document and enable TEAR interrupts on DSI interfaces

Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> · Mon, 13 Feb 2023 19:09:32 -0800

On 2/13/2023 1:46 PM, Dmitry Baryshkov wrote:
On 13/02/2023 21:37, Jessica Zhang wrote:


On 12/31/2022 1:50 PM, Marijn Suijten wrote:
All SoCs since DPU 5.0.0 (and seemingly up until and including 6.0.0,
but excluding 7.x.x) have the tear interrupt and control registers moved
out of the PINGPONG block and into the INTF block.  Wire up the
necessary interrupts and IRQ masks on all supported hardware.

Hi Marijn,

Thanks for the patch.

I saw that in your commit msg, you mentioned that 7.x doesn't have 
tearcheck in the INTF block -- can you double check that this is correct?

I'm working on SM8350 (DPU v7) and I'm seeing that it does have 
tearcheck in INTF block.

I confirm, according to the vendor drivers INTF TE should be used for 
all DPU >= 5.0, including 7.x and 8.x

However I think I know what Marijn meant here. For 5.x and 6.x these 
IRQs are handled at the address MDSS + 0x6e800 / + 0x6e900 (which means 
offset here should 0x6d800 and 0x6d900) for INTF_1 and INTF_2. Since DPU 
7.x these IRQ registers were moved close to the main INTF block (0x36800 
and 0x37800 = INTF + 0x800).


Got it, then the commit text should remove "control" and just say tear 
interrupt registers. It got a bit confusing.

We will add the 7xxx intf tear check support on top of this series.



Signed-off-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
---
  .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    | 78 +++++++++++--------
  .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |  6 +-
  .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 12 +++
  .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h |  2 +
  drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h      |  3 +
  5 files changed, 68 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 1cfe94494135..b9b9b5b0b615 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -86,6 +86,15 @@
  #define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN)
+#define IRQ_MSM8998_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
+             BIT(MDP_SSPP_TOP0_INTR2) | \
+             BIT(MDP_SSPP_TOP0_HIST_INTR) | \
+             BIT(MDP_INTF0_INTR) | \
+             BIT(MDP_INTF1_INTR) | \
+             BIT(MDP_INTF2_INTR) | \
+             BIT(MDP_INTF3_INTR) | \
+             BIT(MDP_INTF4_INTR))
+
  #define IRQ_SDM845_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
               BIT(MDP_SSPP_TOP0_INTR2) | \
               BIT(MDP_SSPP_TOP0_HIST_INTR) | \
@@ -100,13 +109,15 @@
  #define IRQ_QCM2290_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
               BIT(MDP_SSPP_TOP0_INTR2) | \
               BIT(MDP_SSPP_TOP0_HIST_INTR) | \
-             BIT(MDP_INTF1_INTR))
+             BIT(MDP_INTF1_INTR) | \
+             BIT(MDP_INTF1_TEAR_INTR))
  #define IRQ_SC7180_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
               BIT(MDP_SSPP_TOP0_INTR2) | \
               BIT(MDP_SSPP_TOP0_HIST_INTR) | \
               BIT(MDP_INTF0_INTR) | \
-             BIT(MDP_INTF1_INTR))
+             BIT(MDP_INTF1_INTR) | \
+             BIT(MDP_INTF1_TEAR_INTR))
  #define IRQ_SC7280_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
               BIT(MDP_SSPP_TOP0_INTR2) | \
@@ -120,7 +131,9 @@
               BIT(MDP_SSPP_TOP0_HIST_INTR) | \
               BIT(MDP_INTF0_INTR) | \
               BIT(MDP_INTF1_INTR) | \
+             BIT(MDP_INTF1_TEAR_INTR) | \
               BIT(MDP_INTF2_INTR) | \
+             BIT(MDP_INTF2_TEAR_INTR) | \
               BIT(MDP_INTF3_INTR) | \
               BIT(MDP_INTF4_INTR))
@@ -129,7 +142,9 @@
                BIT(MDP_SSPP_TOP0_HIST_INTR) | \
                BIT(MDP_INTF0_INTR) | \
                BIT(MDP_INTF1_INTR) | \
+              BIT(MDP_INTF1_TEAR_INTR) | \
                BIT(MDP_INTF2_INTR) | \
+              BIT(MDP_INTF2_TEAR_INTR) | \
                BIT(MDP_INTF3_INTR) | \
                BIT(MDP_INTF4_INTR) | \
                BIT(MDP_INTF5_INTR) | \
@@ -1300,63 +1315,64 @@ static struct dpu_dsc_cfg sdm845_dsc[] = {
  /*************************************************************
   * INTF sub blocks config
   *************************************************************/
-#define INTF_BLK(_name, _id, _base, _type, _ctrl_id, _progfetch, 
_features, _reg, _underrun_bit, _vsync_bit) \
+#define INTF_BLK(_name, _id, _base, _len, _type, _ctrl_id, 
_progfetch, _features, _reg, _underrun_bit, _vsync_bit, _tear_reg, 
_tear_rd_ptr_bit) \
      {\
      .name = _name, .id = _id, \
-    .base = _base, .len = 0x280, \
+    .base = _base, .len = _len, \
      .features = _features, \
      .type = _type, \
      .controller_id = _ctrl_id, \
      .prog_fetch_lines_worst_case = _progfetch, \
      .intr_underrun = DPU_IRQ_IDX(_reg, _underrun_bit), \
      .intr_vsync = DPU_IRQ_IDX(_reg, _vsync_bit), \
+    .intr_tear_rd_ptr = DPU_IRQ_IDX(_tear_reg, _tear_rd_ptr_bit), \
      }
  static const struct dpu_intf_cfg msm8998_intf[] = {
-    INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 25, 
INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
-    INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 25, 
INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
-    INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 25, 
INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29),
-    INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_HDMI, 0, 25, 
INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31),
+    INTF_BLK("intf_0", INTF_0, 0x6A000, 0x268, INTF_DP, 0, 25, 
INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1),

Just wondering, how were the lengths calculated for the INTF blocks? 
The values in general seem a little off to me.

For example, I'm looking downstream and it seems to me that the length 
for the INTF_0 on MSM8998 should be 0x280. Similarly for SC7280, I'm 
seeing that length for INTF + tearcheck should be 0x2c4.

We have discussed INTF lengths in [1]. The current understanding of the 
block lengths can be found at [2]. Please comment there if any of the 
fixed lengths sounds incorrect to you.

[1] https://patchwork.freedesktop.org/patch/522187/
[2] https://patchwork.freedesktop.org/patch/522227/

[skipped the rest]


Please correct my understanding here, it was agreed to fix intf blocks 
to 0x2c4 here https://patchwork.freedesktop.org/patch/522227/ but I dont 
see this was merged?

It was agreed to first land INTF_TE and then add the higher addresses 
but I dont see such a change, am i missing something?






