Re: [PATCH v2 1/6] drm/msm/dpu: don't set DPU_INTF_TE globally

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

 



On 23/05/2023 00:56, Marijn Suijten wrote:
Title suggestion: s/globally/on non-TE/DSI (INTF) blocks

On 2023-05-23 00:45:22, Dmitry Baryshkov wrote:
Using BIT(DPU_INTF_TE) in INTF_SC7180_MASK (and by extension in
INTF_SC7280_MASK) results in this bit (and corrsponding operations)
being enabled for all interfaces, even the ones which do not have TE
block. Move this bit setting to INTF_DSI_TE(), so that it is only
enabled for those INTF blocks which have TE support.

Fixes: 152c1d430992 ("drm/msm/dpu: Add TEAR-READ-pointer interrupt to INTF block")
Reviewed-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>

We've always been setting flags globally but I guess it makes sense to
not only restrict this flag to DPU >= 5.0.0 but also just the few
hardware blocks that actually have these in their *enlarged* register
space (and have the interrupt).

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

---
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 3 +--
  1 file changed, 1 insertion(+), 2 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 1dee5ba2b312..162141cb5c83 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -101,7 +101,6 @@
#define INTF_SC7180_MASK \
  	(BIT(DPU_INTF_INPUT_CTRL) | \
-	 BIT(DPU_INTF_TE) | \
  	 BIT(DPU_INTF_STATUS_SUPPORTED) | \
  	 BIT(DPU_DATA_HCTL_EN))
@@ -544,7 +543,7 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
  	{\
  	.name = _name, .id = _id, \
  	.base = _base, .len = _len, \
-	.features = _features, \
+	.features = _features | BIT(DPU_INTF_TE), \

Now that we're more broadly switching to this pattern, should we do the
same for PP_BLK() with and without TE block?  That way we can also
forcefully initialize intr_rdptr=-1 similar to what I did for
intr_tear_rd_ptr in INTF_BLK() (vs INTF_BLK_DSI_TE) here, instead of
having the -1's floating around the catalog when I added them in commit
7952f5180eb3e ("drm/msm/dpu: Remove intr_rdptr from DPU >= 5.0.0
pingpong config").

If we are going to expand the macros, then hiding -1 probably doesn't make sense as it will reappear soon.

Probably it makes sense to do another thing (which would play better with the expanded macros): increase IRQ indices by 1, making 'NO IRQ' equal to 0 instead of -1. This way all non-existing interrupts can be omitted during macros expansion. WDYT?


- Marijn

  	.type = _type, \
  	.controller_id = _ctrl_id, \
  	.prog_fetch_lines_worst_case = _progfetch, \
--
2.39.2


--
With best wishes
Dmitry




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux