| Message ID | 20240122163220.110788-3-sui.jingfeng@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: Allow using fwnode API to get the next bridge | expand |
On Tue, Jan 23, 2024 at 12:32:17AM +0800, Sui Jingfeng wrote: > Which is intended to be used on non-DT environment, where the simple-bridge > platform device is created by either the display controller driver side or > platform firmware subsystem. Could you give an example of a platform where you intend to use this ? > To avoid duplication and to keep consistent, > we choose to reuse the OF match tables. Because the potentional user may > not has a of_node attached, nor a ACPI match id. If this is the case, > a software node string property can be provide to fill the niche. Shouldn't non-DT, non-ACPI platforms use swnodes ? > Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev> > --- > drivers/gpu/drm/bridge/simple-bridge.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/simple-bridge.c b/drivers/gpu/drm/bridge/simple-bridge.c > index cbe8e778d7c7..595f672745b9 100644 > --- a/drivers/gpu/drm/bridge/simple-bridge.c > +++ b/drivers/gpu/drm/bridge/simple-bridge.c > @@ -166,6 +166,24 @@ static const struct drm_bridge_funcs simple_bridge_bridge_funcs = { > .disable = simple_bridge_disable, > }; > > +static const void *simple_bridge_get_match_data(const struct device *dev) > +{ > + const struct of_device_id *matches = dev->driver->of_match_table; > + > + /* Try to get the match data by software node */ > + while (matches) { > + if (!matches->compatible[0]) > + break; > + > + if (device_is_compatible(dev, matches->compatible)) > + return matches->data; > + > + matches++; > + } > + > + return NULL; > +} > + > static int simple_bridge_probe(struct platform_device *pdev) > { > struct simple_bridge *sbridge; > @@ -176,7 +194,10 @@ static int simple_bridge_probe(struct platform_device *pdev) > return -ENOMEM; > platform_set_drvdata(pdev, sbridge); > > - sbridge->info = of_device_get_match_data(&pdev->dev); > + if (pdev->dev.of_node) > + sbridge->info = of_device_get_match_data(&pdev->dev); > + else > + sbridge->info = simple_bridge_get_match_data(&pdev->dev); > > /* Get the next bridge in the pipeline. */ > remote = of_graph_get_remote_node(pdev->dev.of_node, 1, -1); > @@ -309,3 +330,4 @@ module_platform_driver(simple_bridge_driver); > MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>"); > MODULE_DESCRIPTION("Simple DRM bridge driver"); > MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:simple-bridge"); This is an unrelated change.
Hi, On 2024/1/23 09:21, Laurent Pinchart wrote: > On Tue, Jan 23, 2024 at 12:32:17AM +0800, Sui Jingfeng wrote: >> Which is intended to be used on non-DT environment, where the simple-bridge >> platform device is created by either the display controller driver side or >> platform firmware subsystem. > Could you give an example of a platform where you intend to use this ? For example: 1) USB based display adapter, such as FL2000DX[1] which use the it66121 HDMI transmitter to convert the RGB888 to HDMI. 2) Simple 2D PCIe display controller, such as SM750(EMPV-1201) which using sii9022 HDMI transmitter to convert the RGB888 to HDMI. 3) Some FPGA PCIe Board (sil9136) 4) Be able to run unit test of drm bridges on X86. [1] https://github.com/FrescoLogic/FL2000
Hi, On 2024/1/23 09:21, Laurent Pinchart wrote: >> static int simple_bridge_probe(struct platform_device *pdev) >> { >> struct simple_bridge *sbridge; >> @@ -176,7 +194,10 @@ static int simple_bridge_probe(struct platform_device *pdev) >> return -ENOMEM; >> platform_set_drvdata(pdev, sbridge); >> >> - sbridge->info = of_device_get_match_data(&pdev->dev); >> + if (pdev->dev.of_node) >> + sbridge->info = of_device_get_match_data(&pdev->dev); >> + else >> + sbridge->info = simple_bridge_get_match_data(&pdev->dev); >> >> /* Get the next bridge in the pipeline. */ >> remote = of_graph_get_remote_node(pdev->dev.of_node, 1, -1); >> @@ -309,3 +330,4 @@ module_platform_driver(simple_bridge_driver); >> MODULE_AUTHOR("Maxime Ripard<maxime.ripard@free-electrons.com>"); >> MODULE_DESCRIPTION("Simple DRM bridge driver"); >> MODULE_LICENSE("GPL"); >> +MODULE_ALIAS("platform:simple-bridge"); > This is an unrelated change. Otherwise, this driver will not be probed when compiled as module on non-DT environment.
On Tue, Jan 23, 2024 at 04:20:04PM +0800, Sui Jingfeng wrote: > On 2024/1/23 09:21, Laurent Pinchart wrote: > > On Tue, Jan 23, 2024 at 12:32:17AM +0800, Sui Jingfeng wrote: > >> Which is intended to be used on non-DT environment, where the simple-bridge > >> platform device is created by either the display controller driver side or > >> platform firmware subsystem. > > > > Could you give an example of a platform where you intend to use this ? > > For example: > > 1) USB based display adapter, such as FL2000DX[1] which use > the it66121 HDMI transmitter to convert the RGB888 to HDMI. > > 2) Simple 2D PCIe display controller, such as SM750(EMPV-1201) > which using sii9022 HDMI transmitter to convert the RGB888 > to HDMI. > > 3) Some FPGA PCIe Board (sil9136) > > 4) Be able to run unit test of drm bridges on X86. Thank you, those are useful examples. It would be nice to capture at least some of them (first instance the first two) to the commit message. > [1] https://github.com/FrescoLogic/FL2000
On Tue, Jan 23, 2024 at 12:32:17AM +0800, Sui Jingfeng wrote: > Which is intended to be used on non-DT environment, where the simple-bridge > platform device is created by either the display controller driver side or > platform firmware subsystem. To avoid duplication and to keep consistent, > we choose to reuse the OF match tables. Because the potentional user may > not has a of_node attached, nor a ACPI match id. If this is the case, > a software node string property can be provide to fill the niche. ... > - sbridge->info = of_device_get_match_data(&pdev->dev); > + if (pdev->dev.of_node) > + sbridge->info = of_device_get_match_data(&pdev->dev); > + else > + sbridge->info = simple_bridge_get_match_data(&pdev->dev); This is wrong. Just use device_get_match_data() instead of of_ counter part. The rest, if required, has to be addressed elsewhere. So, formal NAK for the changes like above.
diff --git a/drivers/gpu/drm/bridge/simple-bridge.c b/drivers/gpu/drm/bridge/simple-bridge.c index cbe8e778d7c7..595f672745b9 100644 --- a/drivers/gpu/drm/bridge/simple-bridge.c +++ b/drivers/gpu/drm/bridge/simple-bridge.c @@ -166,6 +166,24 @@ static const struct drm_bridge_funcs simple_bridge_bridge_funcs = { .disable = simple_bridge_disable, }; +static const void *simple_bridge_get_match_data(const struct device *dev) +{ + const struct of_device_id *matches = dev->driver->of_match_table; + + /* Try to get the match data by software node */ + while (matches) { + if (!matches->compatible[0]) + break; + + if (device_is_compatible(dev, matches->compatible)) + return matches->data; + + matches++; + } + + return NULL; +} + static int simple_bridge_probe(struct platform_device *pdev) { struct simple_bridge *sbridge; @@ -176,7 +194,10 @@ static int simple_bridge_probe(struct platform_device *pdev) return -ENOMEM; platform_set_drvdata(pdev, sbridge); - sbridge->info = of_device_get_match_data(&pdev->dev); + if (pdev->dev.of_node) + sbridge->info = of_device_get_match_data(&pdev->dev); + else + sbridge->info = simple_bridge_get_match_data(&pdev->dev); /* Get the next bridge in the pipeline. */ remote = of_graph_get_remote_node(pdev->dev.of_node, 1, -1); @@ -309,3 +330,4 @@ module_platform_driver(simple_bridge_driver); MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>"); MODULE_DESCRIPTION("Simple DRM bridge driver"); MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:simple-bridge");
Which is intended to be used on non-DT environment, where the simple-bridge platform device is created by either the display controller driver side or platform firmware subsystem. To avoid duplication and to keep consistent, we choose to reuse the OF match tables. Because the potentional user may not has a of_node attached, nor a ACPI match id. If this is the case, a software node string property can be provide to fill the niche. Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev> --- drivers/gpu/drm/bridge/simple-bridge.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)