Re: [PATCH v6 10/13] drm/imx: dc-ed: Drop initial source selection

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unconditional dereference of potentially NULL component pointers during pixel engine runtime resume.
--

commit 0e56cfa8da82fa905cb37a791e64c203dd9bf7e5
Author: Liu Ying <victor.liu@nxp.com>

drm/imx: dc-ed: Drop initial source selection

It's unnecessary to set initial ExtDst source selection because KMS
driver would do that when doing atomic commits.

> diff --git a/drivers/gpu/drm/imx/dc/dc-ed.c b/drivers/gpu/drm/imx/dc/dc-ed.c
> index b8ab41271bb40..803b3e891a1e6 100644
> --- a/drivers/gpu/drm/imx/dc/dc-ed.c
> +++ b/drivers/gpu/drm/imx/dc/dc-ed.c
> @@ -202,7 +202,6 @@ static inline void dc_ed_disable_gamma_apply(struct dc_ed *ed)
>  
>  void dc_ed_init(struct dc_ed *ed)
>  {
> -	dc_ed_pec_src_sel(ed, LINK_ID_NONE);
>  	dc_ed_pec_enable_shden(ed);

[Severity: High]
This is a pre-existing issue, but does this function dereference the ed pointer
without checking if it's NULL?

Looking at dc_pe_runtime_resume() in drivers/gpu/drm/imx/dc/dc-pe.c, it
unconditionally loops over fixed-size arrays:

    for (i = 0; i < ARRAY_SIZE(pe->ed_safe); i++)
        dc_ed_init(pe->ed_safe[i]);

    for (i = 0; i < ARRAY_SIZE(pe->ed_cont); i++)
        dc_ed_init(pe->ed_cont[i]);

    for (i = 0; i < ARRAY_SIZE(pe->fu_disp); i++)
        pe->fu_disp[i]->ops.init(pe->fu_disp[i]);

If any sub-component of the display pipeline (such as extdst, constframe, or fu)
is marked as status = "disabled" in the device tree, the component master in
dc-drv.c uses for_each_available_child_of_node() and only binds the available
components.

The pointers corresponding to disabled components remain NULL. When
dc_pe_runtime_resume() executes, it passes these NULL pointers to initialization
functions like dc_ed_init(), which immediately dereferences them when calling
dc_ed_pec_enable_shden(). It also directly dereferences them when calling
pe->fu_disp[i]->ops.init().

Disabling unused display pipes in the device tree is a standard configuration
practice for generic SoCs, but doing so appears to cause a kernel panic here
during device runtime resume.

>  	dc_ed_pec_poweron(ed);
>  	dc_ed_pec_sync_mode_single(ed);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-imx8-dc-prefetch-v6-0-26643e3f6cb7@nxp.com?part=10