Re: [PATCH 11/17] drm/spacemit: add Saturn DPU DRM device driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing IS_ERR() check for 'match' causes an ERR_PTR dereference inside component_master_add_with_match().
- [Medium] The atomic commit tail stalls the commit pipeline by waiting for flip completion before signaling hardware done.
--

--- Patch [11]: [PATCH 11/17] drm/spacemit: add Saturn DPU DRM device driver ---
commit e12174b9614adeccb5a06f03b5944837004e96f4
Author: Cody Kang <codykang.hk@gmail.com>

drm/spacemit: add Saturn DPU DRM device driver

This commit adds the DRM device for the SpacemiT K3 Saturn display processing
unit. The SoC has two DPU instances, each driving one CRTC that scans out a
single primary plane in linear RGB, feeding a downstream DP/eDP controller
over an OF-graph endpoint.

> diff --git a/drivers/gpu/drm/spacemit/spacemit_drm.c b/drivers/gpu/drm/spacemit/spacemit_drm.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/gpu/drm/spacemit/spacemit_drm.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2025-2026 SpacemiT Co., Ltd.
> + *
> + */

[ ... ]

> +static void spacemit_drm_atomic_commit_tail(struct drm_atomic_commit *old_state)
> +{
> +	struct drm_device *dev = old_state->dev;
> +
> +	drm_atomic_helper_commit_modeset_disables(dev, old_state);
> +
> +	drm_atomic_helper_commit_modeset_enables(dev, old_state);
> +
> +	drm_atomic_helper_commit_planes(dev, old_state,
> +					DRM_PLANE_COMMIT_ACTIVE_ONLY);
> +
> +	drm_atomic_helper_wait_for_flip_done(dev, old_state);
> +
> +	drm_atomic_helper_commit_hw_done(old_state);

[Severity: Medium]
Does calling drm_atomic_helper_wait_for_flip_done() before
drm_atomic_helper_commit_hw_done() stall the commit pipeline?

By waiting for the flip completion before signaling hardware done, the CPU work
for the next frame (such as state preparation and buffer pinning) is blocked
until the hardware flip completes.

Should commit_hw_done() be called before wait_for_flip_done() to avoid
degrading atomic commit performance?

> +
> +	drm_atomic_helper_cleanup_planes(dev, old_state);
> +}

[ ... ]

> +int spacemit_drm_of_component_probe(struct device *dev)
> +{
> +	struct device_node *ports, *port, *ep;
> +	struct device_node *remote;
> +	struct component_match *match = NULL;

[ ... ]

> +	for_each_child_of_node(ports, port) {
> +		if (!of_node_name_eq(port, "port"))
> +			continue;
> +		for_each_child_of_node(port, ep) {
> +			if (!of_node_name_eq(ep, "endpoint"))
> +				continue;
> +			remote = of_graph_get_remote_port_parent(ep);
> +			if (!remote)
> +				continue;
> +			if (of_device_is_available(remote))
> +				drm_of_component_match_add(dev, &match,
> +							   compare_of, remote);
> +			of_node_put(remote);
> +		}
> +	}
> +	of_node_put(ports);
> +
> +	if (!match) {
> +		dev_err(dev, "no downstream component available\n");
> +		return -ENODEV;
> +	}
> +
> +	return component_master_add_with_match(dev, &spacemit_drm_master_ops,
> +					       match);

[Severity: High]
Can match be an ERR_PTR here instead of just NULL?

If memory allocation fails inside drm_of_component_match_add(), it can set
the match pointer to an ERR_PTR. Since the code only checks if (!match), an
error pointer would bypass this check and be passed directly to
component_master_add_with_match().

Looking at component_master_add_with_match(), it immediately accesses the
pointer:

drivers/base/component.c:component_master_add_with_match() {
    ...
	/* Reallocate the match array for its true size */
	ret = component_match_realloc(match, match->num);
    ...
}

Should this code check IS_ERR(match) to prevent a potential kernel panic
under memory pressure?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-k3-display-v1-0-f22a10a52867@gmail.com?part=11

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