Hi Luca,

On 3/3/2026 5:42 PM, Luca Ceresoli wrote:
Hello Damon,

On Tue Feb 10, 2026 at 8:12 AM CET, Damon Ding wrote:
Apply drm_bridge_connector helper for Analogix DP driver.

The following changes have been made:
- Apply drm_bridge_connector helper to get rid of &drm_connector_funcs
   and &drm_connector_helper_funcs.
- Remove unnecessary parameter struct drm_connector* for callback
   &analogix_dp_plat_data.attach.
- Remove &analogix_dp_device.connector.
- Convert analogix_dp_atomic_check()/analogix_dp_detect() to
   &drm_bridge_funcs.atomic_check()/&drm_bridge_funcs.detect().
- Split analogix_dp_get_modes() into &drm_bridge_funcs.get_modes() and
   &drm_bridge_funcs.edid_read().
- Set flag DRM_BRIDGE_ATTACH_NO_CONNECTOR for bridge attachment while
   binding. Meanwhile, make DRM_BRIDGE_ATTACH_NO_CONNECTOR unsuppported
                              ^

Do you mean "!DRM_BRIDGE_ATTACH_NO_CONNECTOR" here (i.e. missing '!')?

Also, unsuppported -> unsupported (typo)


Will fix in v10.

   in analogix_dp_bridge_attach().
- Set &drm_bridge.ops according to different cases.

Signed-off-by: Damon Ding <[email protected]>
Tested-by: Marek Szyprowski <[email protected]>
Tested-by: Heiko Stuebner <[email protected]> (on rk3588)

I had a quick look, looks good overall, for the moment I have only a
question, see below.

I aim at reviewing this patch in depth, but it's not an easy one to
digest. Would it be feasible to split it in smaller logical steps? If it
is, please do, it would be very helpful for reviewing.

Yes, this commit will be split into several smaller ones in v10.


@@ -1532,6 +1481,7 @@ EXPORT_SYMBOL_GPL(analogix_dp_resume);

  int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device 
*drm_dev)
  {
+       struct drm_bridge *bridge = &dp->bridge;
        int ret;

        dp->drm_dev = drm_dev;
@@ -1545,7 +1495,18 @@ int analogix_dp_bind(struct analogix_dp_device *dp, 
struct drm_device *drm_dev)
                return ret;
        }

-       ret = drm_bridge_attach(dp->encoder, &dp->bridge, NULL, 0);
+       if (dp->plat_data->panel)
+               bridge->ops = DRM_BRIDGE_OP_MODES | DRM_BRIDGE_OP_DETECT;
+       else
+               bridge->ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_DETECT;
+
+       bridge->of_node = dp->dev->of_node;
+       bridge->type = DRM_MODE_CONNECTOR_eDP;
+       ret = devm_drm_bridge_add(dp->dev, &dp->bridge);

Can devm_drm_bridge_add() be added to analogix_dp_probe() instead?

Will do in v10.


+       if (ret)
+               goto err_unregister_aux;
+
+       ret = drm_bridge_attach(dp->encoder, bridge, NULL, 
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
        if (ret) {
                DRM_ERROR("failed to create bridge (%d)\n", ret);
                goto err_unregister_aux;

Best regards,
Damon

Reply via email to