Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Medium] Missing polling configuration for dynamic hotplug detection. Pre-existing issues: - [High] Missing -EPROBE_DEFER handling undermines the intended fix under normal probe ordering. -- --- Patch [3]: [PATCH 3/4] drm/imx: imx-tve: probe DDC before claiming the connector is connected --- commit 496cfa19c398d86d2f77e8a07cbe96a51f14ca0a Author: Dmitry Baryshkov <[email protected]> drm/imx: imx-tve: probe DDC before claiming the connector is connected This commit adds a detect_ctx hook to the TVE connector using the DDC bus to detect whether a monitor is attached. It aims to fix a CRTC timeout issue caused by unconditionally lighting up the VGA output when nothing is connected. > diff --git a/drivers/gpu/drm/imx/ipuv3/imx-tve.c > b/drivers/gpu/drm/imx/ipuv3/imx-tve.c > index e861b8b9d8fa8..11ac69561eb81 100644 > --- a/drivers/gpu/drm/imx/ipuv3/imx-tve.c > +++ b/drivers/gpu/drm/imx/ipuv3/imx-tve.c > @@ -322,6 +322,7 @@ static const struct drm_connector_funcs > imx_tve_connector_funcs = { > static const struct drm_connector_helper_funcs > imx_tve_connector_helper_funcs = { > .get_modes = imx_tve_connector_get_modes, > .mode_valid = imx_tve_connector_mode_valid, > + .detect_ctx = drm_connector_helper_detect_from_ddc, [Severity: Medium] Since this adds dynamic detection through the detect_ctx callback, shouldn't we also configure the connector polling flags in imx_tve_bind()? It looks like the connector is initialized here: imx_tve_bind() { ... ret = drm_connector_init_with_ddc(drm, connector, &imx_tve_connector_funcs, DRM_MODE_CONNECTOR_VGA, tve->ddc); ... } Without setting connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT, the DRM core's periodic poll worker will ignore this connector. Will this prevent dynamic detection if a user plugs in a monitor after boot? [Severity: High] This is a pre-existing issue, but could probe ordering undermine this fix? The patch relies on tve->ddc being initialized to accurately report the connector status. However, imx_tve_probe() does not currently handle probe deferral: imx_tve_probe() { ... tve->ddc = of_find_i2c_adapter_by_node(ddc_node); of_node_put(ddc_node); if (tve->ddc) { ... } If the I2C driver hasn't probed yet, tve->ddc will be left as NULL instead of returning -EPROBE_DEFER. If detect_ctx is later called with a NULL DDC adapter, drm_connector_helper_detect_from_ddc() will return connector_status_unknown. Because the DRM core treats unknown as connected when fallback modes are involved, does this mean the CRTC timeout bug will reoccur whenever the I2C driver probes after imx-tve? > }; > > static const struct drm_encoder_helper_funcs imx_tve_encoder_helper_funcs = { -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
