diff mbox series

[v7,10/15] drm/rockchip: analogix_dp: Add support to get panel from the DP AUX bus

Message ID 20250224081325.96724-11-damon.ding@rock-chips.com (mailing list archive)
State New
Headers show
Series Add eDP support for RK3588 | expand

Commit Message

Damon Ding Feb. 24, 2025, 8:13 a.m. UTC 
Move drm_of_find_panel_or_bridge() a little later and combine it with
component_add() into a new function rockchip_dp_link_panel(). The function
will serve as done_probing() callback of devm_of_dp_aux_populate_bus(),
aiding to support for obtaining the eDP panel via the DP AUX bus.

If failed to get the panel from the DP AUX bus, it will then try the other
way to get panel information through the platform bus.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

---

Changes in v4:
- Use done_probing() to call drm_of_find_panel_or_bridge() and
  component_add() when getting panel from the DP AUX bus

Changes in v5:
- Use the functions exported by the Analogix side to get the pointers of
  struct analogix_dp_plat_data and struct drm_dp_aux.
- Use dev_err() instead of drm_err() in rockchip_dp_poweron().

Changes in v6:
- Keep drm_err() in rockchip_dp_poweron()
- Pass 'dp' in drm_...() rather than 'dp->drm_dev'

Changes in v7:
- Include the drm_dp_aux_bus.h for devm_of_dp_aux_populate_bus()
- Use dev_err_probe() for the return value check of
  devm_of_dp_aux_populate_bus()
- Select DRM_DISPLAY_DP_AUX_BUS if ROCKCHIP_ANALOGIX_DP
- Restore the error check for drm_of_find_panel_or_bridge() which was
  removed by mistake
---
 drivers/gpu/drm/rockchip/Kconfig              |  1 +
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 37 ++++++++++++++-----
 2 files changed, 29 insertions(+), 9 deletions(-)

Comments

Doug Anderson Feb. 25, 2025, 1:42 a.m. UTC | #1 
Hi,

On Mon, Feb 24, 2025 at 12:14 AM Damon Ding <damon.ding@rock-chips.com> wrote:
>
> @@ -392,11 +393,27 @@ static const struct component_ops rockchip_dp_component_ops = {
>         .unbind = rockchip_dp_unbind,
>  };
>
> +static int rockchip_dp_link_panel(struct drm_dp_aux *aux)
> +{
> +       struct analogix_dp_plat_data *plat_data = analogix_dp_aux_to_plat_data(aux);
> +       struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
> +       int ret;
> +
> +       ret = drm_of_find_panel_or_bridge(dp->dev->of_node, 1, 0, &plat_data->panel, NULL);
> +       if (ret && ret != -ENODEV)
> +               return ret;

Can you explain why you treat -ENODEV as a non-error case here? Maybe
this is for the non-eDP case (AKA the DP case) where there's no
further panels or bridges? Maybe a comment would be helpful to remind
us?


> +       ret = component_add(dp->dev, &rockchip_dp_component_ops);
> +       if (ret)
> +               return ret;
> +
> +       return ret;

nit: the above could just be:

return component_add(dp->dev, &rockchip_dp_component_ops);


> @@ -448,9 +460,16 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>         if (IS_ERR(dp->adp))
>                 return PTR_ERR(dp->adp);
>
> -       ret = component_add(dev, &rockchip_dp_component_ops);
> -       if (ret)
> -               return ret;
> +       ret = devm_of_dp_aux_populate_bus(analogix_dp_get_aux(dp->adp), rockchip_dp_link_panel);
> +       if (ret) {
> +               if (ret != -ENODEV)
> +                       return dev_err_probe(dp->dev, ret,
> +                                            "failed to populate aux bus : %d\n", ret);

IIRC this -ENODEV case is for old legacy panels that aren't listed
under the aux bus in the device tree. Maybe a comment would be helpful
to remind us?

nit: don't need the %d in your error message. dev_err_probe() already
prints the error code.


> +               ret = rockchip_dp_link_panel(analogix_dp_get_aux(dp->adp));
> +               if (ret)
> +                       return ret;
> +       }
>
>         return 0;

You can get rid of a few of your return cases by just returning "ret" here.


-Doug
diff mbox series

Patch

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 26c4410b2407..f9d7776a859a 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -8,6 +8,7 @@  config DRM_ROCKCHIP
 	select DRM_PANEL
 	select VIDEOMODE_HELPERS
 	select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
+	select DRM_DISPLAY_DP_AUX_BUS if ROCKCHIP_ANALOGIX_DP
 	select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
 	select DRM_DW_HDMI_QP if ROCKCHIP_DW_HDMI_QP
 	select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index a8265a1bf9ff..bbfcc59081f6 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -21,6 +21,7 @@ 
 #include <video/of_videomode.h>
 #include <video/videomode.h>
 
+#include <drm/display/drm_dp_aux_bus.h>
 #include <drm/display/drm_dp_helper.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -392,11 +393,27 @@  static const struct component_ops rockchip_dp_component_ops = {
 	.unbind = rockchip_dp_unbind,
 };
 
+static int rockchip_dp_link_panel(struct drm_dp_aux *aux)
+{
+	struct analogix_dp_plat_data *plat_data = analogix_dp_aux_to_plat_data(aux);
+	struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
+	int ret;
+
+	ret = drm_of_find_panel_or_bridge(dp->dev->of_node, 1, 0, &plat_data->panel, NULL);
+	if (ret && ret != -ENODEV)
+		return ret;
+
+	ret = component_add(dp->dev, &rockchip_dp_component_ops);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
 static int rockchip_dp_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	const struct rockchip_dp_chip_data *dp_data;
-	struct drm_panel *panel = NULL;
 	struct rockchip_dp_device *dp;
 	struct resource *res;
 	int i;
@@ -406,10 +423,6 @@  static int rockchip_dp_probe(struct platform_device *pdev)
 	if (!dp_data)
 		return -ENODEV;
 
-	ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL);
-	if (ret < 0 && ret != -ENODEV)
-		return ret;
-
 	dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
 	if (!dp)
 		return -ENOMEM;
@@ -432,7 +445,6 @@  static int rockchip_dp_probe(struct platform_device *pdev)
 
 	dp->dev = dev;
 	dp->adp = ERR_PTR(-ENODEV);
-	dp->plat_data.panel = panel;
 	dp->plat_data.dev_type = dp->data->chip_type;
 	dp->plat_data.power_on = rockchip_dp_poweron;
 	dp->plat_data.power_off = rockchip_dp_powerdown;
@@ -448,9 +460,16 @@  static int rockchip_dp_probe(struct platform_device *pdev)
 	if (IS_ERR(dp->adp))
 		return PTR_ERR(dp->adp);
 
-	ret = component_add(dev, &rockchip_dp_component_ops);
-	if (ret)
-		return ret;
+	ret = devm_of_dp_aux_populate_bus(analogix_dp_get_aux(dp->adp), rockchip_dp_link_panel);
+	if (ret) {
+		if (ret != -ENODEV)
+			return dev_err_probe(dp->dev, ret,
+					     "failed to populate aux bus : %d\n", ret);
+
+		ret = rockchip_dp_link_panel(analogix_dp_get_aux(dp->adp));
+		if (ret)
+			return ret;
+	}
 
 	return 0;
 }