Re: [PATCH] GPU: drm: meson inline drm_simple_encoder_init()

[sashiko-bot@kernel.org]

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

New issues:
- [High] Incorrect inlining of `drm_simple_encoder_init()` omits the `funcs` parameter, passing 4 arguments to a function expecting at least 5, resulting in a severe type mismatch and a missing `drm_encoder_funcs` structure.

Pre-existing issues:
- [High] Failure to remove the DRM bridge on error paths leaves a dangling pointer in the global bridge list and permanently leaks the bridge container memory.
--

commit cff971d423a1058d9ded86a03517245fb14f47c1
Author: Liam Zuiderhoek <zuiderhoekl@gmail.com>

GPU: drm: meson inline drm_simple_encoder_init()

This patch replaces the drm_simple_encoder_init() call in the meson HDMI
encoder with a direct call to drm_encoder_init() as part of the DRM subsystem
TODO list.

> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> index 55c0601df3c62..48ca6874d4a01 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> @@ -407,8 +407,8 @@ int meson_encoder_hdmi_probe(struct meson_drm *priv)
>  	meson_encoder_hdmi->priv = priv;
>  
>  	/* Encoder */
> -	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
> -				      DRM_MODE_ENCODER_TMDS);
> +	ret = drm_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
> +			       DRM_MODE_ENCODER_TMDS, NULL);

[Severity: High]
Does this pass the correct number of arguments to drm_encoder_init()?

Looking at drm_encoder_init() in drm_encoder.c, it expects at least five
arguments:
drm_encoder_init(dev, encoder, funcs, encoder_type, name, ...)

With only four arguments provided here in meson_encoder_hdmi_probe(), this
appears to pass the integer DRM_MODE_ENCODER_TMDS into the const struct
drm_encoder_funcs *funcs parameter, and NULL into the encoder_type parameter,
omitting the name parameter entirely.

Will this cause a build failure, or a panic when funcs->destroy is later
dereferenced?

>  	if (ret) {
>  		dev_err_probe(priv->dev, ret, "Failed to init HDMI encoder\n");
>  		goto err_put_node;

[Severity: High]
This isn't a bug introduced by this patch, but does this error path need to
call drm_bridge_remove() to prevent a memory leak?

Earlier in meson_encoder_hdmi_probe(), the bridge is added to the global
bridge list:

	drm_bridge_add(&meson_encoder_hdmi->bridge);

If drm_encoder_init() fails and takes this goto err_put_node path, the
bridge remains in the global bridge list with an elevated refcount because
drm_bridge_remove() is not called.

Could this leave an orphaned bridge that permanently breaks the display
pipeline on subsequent probe retries?

>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622180832.28143-1-zuiderhoekl@gmail.com?part=1