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 <[email protected]> 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/[email protected]?part=1
