Re: [PATCH 04/17] drm/msm/dpu: Add support for DPU 12.1 on Nord

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Adding `INTF_9` and `INTF_10` without corresponding entries in `enum dpu_hw_intr_reg` causes the `MDP_INTFn_INTR()` macro to silently alias with legacy DSI TEAR interrupts, leading to fatal unclocked hardware accesses.
- [High] Including inferred, unverified dummy register addresses and bus mapper IDs in an upstream hardware catalog.
--

commit bd1cb43210104c822d361350ae75f5b63854edc7
Author: Mahadevan P <mahadevan.p@oss.qualcomm.com>

drm/msm/dpu: Add support for DPU 12.1 on Nord

This commit adds the DPU catalog entry for the Qualcomm Nord SoC (MDSS 12.1),
a DP-only chip with 8 DSPP blocks, 2 writeback blocks, 8 interfaces and 2 DP
controllers. It extends the interface enum with INTF_9 and INTF_10 and adds
inferred hardware catalog entries marked with FIXME.

> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_12_1_nord.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_12_1_nord.h
> new file mode 100644
> index 0000000000000..8eb18752f3c4b
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_12_1_nord.h
[ ... ]
> +static const struct dpu_wb_cfg nord_wb[] = {
> +	{
> +		.name = "wb_1", .id = WB_1,
> +		.base = 0x64800, .len = 0x2c8, /* FIXME(REG_ADDR): inferred */
> +		.features = WB_SDM845_MASK,
> +		.format_list = wb2_formats_rgb_yuv,
> +		.num_formats = ARRAY_SIZE(wb2_formats_rgb_yuv),
> +		.xin_id = 7, /* FIXME(XIU_MAP): inferred */

[Severity: High]
Is it safe to include inferred register base addresses and bus mapper IDs that
haven't been verified against actual hardware?

When the driver probes on Nord, it will unconditionally map and access these
unverified addresses. If the inferred base address is incorrect, accessing
this region will likely hit invalid physical memory and cause an immediate
bus error.

[ ... ]
> +static const struct dpu_intf_cfg nord_intf[] = {
> +	{
[ ... ]
> +		.name = "intf_4", .id = INTF_4,
> +		.base = 0x38000, .len = 0x4bc, /* FIXME(REG_ADDR): new, stride +0x1000 */

[Severity: High]
Similarly here, will accessing this unverified INTF_4 base address cause a
system crash if the guessed stride is incorrect for this SoC?

[ ... ]
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> index 0e65bf5ddc4a6..7eccb4d2c65ef 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> @@ -226,6 +226,8 @@ enum dpu_intf {
>  	INTF_6,
>  	INTF_7,
>  	INTF_8,
> +	INTF_9,
> +	INTF_10,
>  	INTF_MAX
>  };

[Severity: High]
Does adding INTF_9 and INTF_10 without updating enum dpu_hw_intr_reg cause
an interrupt mapping collision?

Looking at MDP_INTFn_INTR in dpu_hw_interrupts.h:

#define MDP_INTFn_INTR(intf)    (MDP_INTF0_INTR + (intf - INTF_0))

When this maps INTF_9 (index 10) and INTF_10 (index 11), they evaluate to
offsets 12 and 13 respectively. In enum dpu_hw_intr_reg, offsets 12 and 13
are already mapped to MDP_INTF1_TEAR_INTR and MDP_INTF2_TEAR_INTR.

During driver probe, dpu_hw_intr_init() uses this macro to populate the
irq_mask. Since Nord is DP-only and explicitly lacks the INTF_1 and INTF_2
hardware blocks, will dpu_clear_irqs() iterating over this mask attempt to
write to unmapped TEAR registers and cause an unrecoverable bus fault?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-nord-drv-dt-v1-0-e25c073cf1b6@oss.qualcomm.com?part=4

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy