Hi Luca,

On 3/9/2026 7:25 PM, Damon Ding wrote:

Hi Luca,

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

quoted

Hello Damon,

On Tue Feb 10, 2026 at 8:12 AM CET, Damon Ding wrote:

quoted

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.

quoted

quoted

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

Signed-off-by: Damon Ding <redacted>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de> (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.

quoted

quoted

@@ -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.

My apologies for my somewhat irresponsible reply earlier!

When I was splitting this commit, I realized that moving 
devm_drm_bridge_add() to analogix_dp_probe() is actually not feasible – 
the type of the downstream bridge of the Analogix bridge can only be 
determined after the entire probe process is fully completed.

rockchip_dp_probe()/exynos_dp_probe() ->
   analogix_dp_finish_probe() ->
     analogix_dp_aux_done_probing() ->
       drm_of_find_panel_or_bridge() ->
         rockchip_dp_bind()/exynos_dp_bind() ->
           analogix_dp_bind()->
             devm_drm_bridge_add()

To avoid confusion, I’ve added some comments for this easily 
misunderstood logic to the commit message of v10, so the reasoning is 
clear for everyone.

quoted

quoted

+    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