Hi Luca,
On 3/9/2026 7:51 PM, Damon Ding wrote:
Hi Luca,
On 3/3/2026 5:54 PM, Luca Ceresoli wrote:
Hello Damon,
On Tue Feb 10, 2026 at 8:12 AM CET, Damon Ding wrote:
Since the panel/bridge should logically be positioned behind the
Analogix bridge in the display pipeline, it makes sense to handle
the panel/bridge parsing on the Analogix side. Therefore, we add
a new API analogix_dp_finish_probe(), which combines the panel/bridge
parsing with component addition, to do it.
Signed-off-by: Damon Ding <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
Tested-by: Marek Szyprowski <[email protected]>
Tested-by: Heiko Stuebner <[email protected]> (on rk3588)
...
@@ -1581,6 +1583,52 @@ struct drm_dp_aux *analogix_dp_get_aux(struct
analogix_dp_device *dp)
}
EXPORT_SYMBOL_GPL(analogix_dp_get_aux);
+static int analogix_dp_aux_done_probing(struct drm_dp_aux *aux)
+{
+ struct analogix_dp_device *dp = to_dp(aux);
+ struct analogix_dp_plat_data *plat_data = dp->plat_data;
+ int port = plat_data->dev_type == EXYNOS_DP ? 0 : 1;
+ int ret;
+
+ /*
+ * If drm_of_find_panel_or_bridge() returns -ENODEV, there may
be no valid panel
+ * or bridge nodes. The driver should go on for the driver-free
bridge or the DP
+ * mode applications.
+ */
+ ret = drm_of_find_panel_or_bridge(dp->dev->of_node, port, 0,
+ &plat_data->panel, &plat_data->next_bridge);
+ if (ret && ret != -ENODEV)
+ return ret;
+
+ return component_add(dp->dev, plat_data->ops);
+}
+
+int analogix_dp_finish_probe(struct analogix_dp_device *dp)
+{
+ int ret;
+
+ ret = devm_of_dp_aux_populate_bus(&dp->aux,
analogix_dp_aux_done_probing);
+ if (ret) {
+ /*
+ * If devm_of_dp_aux_populate_bus() returns -ENODEV, the
done_probing() will
+ * not be called because there are no EP devices. Then the
callback function
+ * analogix_dp_aux_done_probing() will be called directly in
order to support
+ * the other valid DT configurations.
+ *
+ * NOTE: The devm_of_dp_aux_populate_bus() is allowed to
return -EPROBE_DEFER.
Uhm, if it is allowed to return -EPROBE_DEFER...
+ */
+ if (ret != -ENODEV) {
+ dev_err(dp->dev, "failed to populate aux bus\n");
+ return ret;
+ }
...then you shouldn't dev_err() when -EPROBE_DEFER is returned.
Either use dev_err_probe() (which would also simplify your code) or check
for if (ret != -ENODEV && ret != -EPROBE_DEFER).
Will fix in v10.
+
+ return analogix_dp_aux_done_probing(&dp->aux);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(analogix_dp_finish_probe);
+
MODULE_AUTHOR("Jingoo Han <[email protected]>");
MODULE_DESCRIPTION("Analogix DP Core Driver");
MODULE_LICENSE("GPL v2");
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/
analogix_dp.h
index 3428ffff24c5..bae969dec63a 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -30,6 +30,7 @@ struct analogix_dp_plat_data {
struct drm_bridge *next_bridge;
struct drm_encoder *encoder;
struct drm_connector *connector;
+ const struct component_ops *ops;
Is adding a new stored field a good idea? Can it be instead passed as an
argument to analogix_dp_finish_probe()?
Note I don't have a strong opinion here, just the added struct field
seems
overkill for being used just once.
I agree this is a better approach, since the &component_ops is only used
during probing.
Sorry for another shamefully incorrect reply!
The &dp_aux_ep_device_with_data.done_probing only supports passing the
&drm_dp_aux parameter, meaning we can't directly pass &component_ops.
I'll also add extra context to the commit msg – similar to what I did
for v9 patch 9/15 – so everyone understands why this change was made and
avoids confusion.
@@ -49,5 +50,6 @@ int analogix_dp_stop_crc(struct drm_connector
*connector);
struct analogix_dp_plat_data *analogix_dp_aux_to_plat_data(struct
drm_dp_aux *aux);
struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp);
+int analogix_dp_finish_probe(struct analogix_dp_device *dp);
#endif /* _ANALOGIX_DP_H_ */
Best regards,
Damon
