| Message ID | 20241219080604.1423600-9-damon.ding@rock-chips.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add eDP support for RK3588 | expand |
On Thu, Dec 19, 2024 at 04:05:57PM +0800, Damon Ding wrote: > The rockchip_dp_of_panel_on_aux_bus() helps to check whether the DT > configurations related to the DP AUX bus are correct or not. > > If failed to get the panel from the platform bus, it is good to try > the DP AUX bus. Then, the probing process will continue until it enters > the analogix_dp_bind(), where devm_of_dp_aux_populate_bus() is called > after &analogix_dp_device.aux has been initialized. No. devm_of_dp_aux_populate_bus() should be called before bind(). And bind should only be called from the done_probing() callback. The reason is very simple: the panel driver might be built as a module and might be not available when the analogix driver is being probed. Also, please invert the logic of the commit message (and the driver). The platform bus should be a fallback if there is no AUX bus panel, not other way around. > > Signed-off-by: Damon Ding <damon.ding@rock-chips.com> > --- > .../gpu/drm/rockchip/analogix_dp-rockchip.c | 24 +++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > index ba5263f85ee2..60c902abf40b 100644 > --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > @@ -317,6 +317,24 @@ static const struct drm_encoder_helper_funcs rockchip_dp_encoder_helper_funcs = > .atomic_check = rockchip_dp_drm_encoder_atomic_check, > }; > > +static bool rockchip_dp_of_panel_on_aux_bus(const struct device_node *np) > +{ > + struct device_node *bus_node, *panel_node; > + > + bus_node = of_get_child_by_name(np, "aux-bus"); > + if (!bus_node) > + return false; > + > + panel_node = of_get_child_by_name(bus_node, "panel"); > + of_node_put(bus_node); > + if (!panel_node) > + return false; > + > + of_node_put(panel_node); > + > + return true; > +} > + > static int rockchip_dp_of_probe(struct rockchip_dp_device *dp) > { > struct device *dev = dp->dev; > @@ -435,8 +453,10 @@ static int rockchip_dp_probe(struct platform_device *pdev) > return -ENODEV; > > ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL); > - if (ret < 0) > - return ret; > + if (ret < 0) { > + if (!rockchip_dp_of_panel_on_aux_bus(dev->of_node)) > + return ret; > + } > > dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL); > if (!dp) > -- > 2.34.1 >
Hi Dmitry, On 2024/12/20 8:16, Dmitry Baryshkov wrote: > On Thu, Dec 19, 2024 at 04:05:57PM +0800, Damon Ding wrote: >> The rockchip_dp_of_panel_on_aux_bus() helps to check whether the DT >> configurations related to the DP AUX bus are correct or not. >> >> If failed to get the panel from the platform bus, it is good to try >> the DP AUX bus. Then, the probing process will continue until it enters >> the analogix_dp_bind(), where devm_of_dp_aux_populate_bus() is called >> after &analogix_dp_device.aux has been initialized. > > No. devm_of_dp_aux_populate_bus() should be called before bind(). And > bind should only be called from the done_probing() callback. The reason > is very simple: the panel driver might be built as a module and might be > not available when the analogix driver is being probed. > > Also, please invert the logic of the commit message (and the driver). > The platform bus should be a fallback if there is no AUX bus panel, not > other way around. > I have tried the logic as you recommanded, and it is really a good way. I will fix this in the next version. >> >> Signed-off-by: Damon Ding <damon.ding@rock-chips.com> >> --- >> .../gpu/drm/rockchip/analogix_dp-rockchip.c | 24 +++++++++++++++++-- >> 1 file changed, 22 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c >> index ba5263f85ee2..60c902abf40b 100644 >> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c >> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c >> @@ -317,6 +317,24 @@ static const struct drm_encoder_helper_funcs rockchip_dp_encoder_helper_funcs = >> .atomic_check = rockchip_dp_drm_encoder_atomic_check, >> }; >> >> +static bool rockchip_dp_of_panel_on_aux_bus(const struct device_node *np) >> +{ >> + struct device_node *bus_node, *panel_node; >> + >> + bus_node = of_get_child_by_name(np, "aux-bus"); >> + if (!bus_node) >> + return false; >> + >> + panel_node = of_get_child_by_name(bus_node, "panel"); >> + of_node_put(bus_node); >> + if (!panel_node) >> + return false; >> + >> + of_node_put(panel_node); >> + >> + return true; >> +} >> + >> static int rockchip_dp_of_probe(struct rockchip_dp_device *dp) >> { >> struct device *dev = dp->dev; >> @@ -435,8 +453,10 @@ static int rockchip_dp_probe(struct platform_device *pdev) >> return -ENODEV; >> >> ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL); >> - if (ret < 0) >> - return ret; >> + if (ret < 0) { >> + if (!rockchip_dp_of_panel_on_aux_bus(dev->of_node)) >> + return ret; >> + } >> >> dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL); >> if (!dp) >> -- >> 2.34.1 >> > Best regards, Damon
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index ba5263f85ee2..60c902abf40b 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -317,6 +317,24 @@ static const struct drm_encoder_helper_funcs rockchip_dp_encoder_helper_funcs = .atomic_check = rockchip_dp_drm_encoder_atomic_check, }; +static bool rockchip_dp_of_panel_on_aux_bus(const struct device_node *np) +{ + struct device_node *bus_node, *panel_node; + + bus_node = of_get_child_by_name(np, "aux-bus"); + if (!bus_node) + return false; + + panel_node = of_get_child_by_name(bus_node, "panel"); + of_node_put(bus_node); + if (!panel_node) + return false; + + of_node_put(panel_node); + + return true; +} + static int rockchip_dp_of_probe(struct rockchip_dp_device *dp) { struct device *dev = dp->dev; @@ -435,8 +453,10 @@ static int rockchip_dp_probe(struct platform_device *pdev) return -ENODEV; ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL); - if (ret < 0) - return ret; + if (ret < 0) { + if (!rockchip_dp_of_panel_on_aux_bus(dev->of_node)) + return ret; + } dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL); if (!dp)
The rockchip_dp_of_panel_on_aux_bus() helps to check whether the DT configurations related to the DP AUX bus are correct or not. If failed to get the panel from the platform bus, it is good to try the DP AUX bus. Then, the probing process will continue until it enters the analogix_dp_bind(), where devm_of_dp_aux_populate_bus() is called after &analogix_dp_device.aux has been initialized. Signed-off-by: Damon Ding <damon.ding@rock-chips.com> --- .../gpu/drm/rockchip/analogix_dp-rockchip.c | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)