Re: [PATCH v4 1/9] drm/bridge: Allow using fwnode API to get the next bridge

Dmitry Baryshkov
2024-04-22T19:51:45Z

On Tue, Apr 23, 2024 at 03:18:55AM +0800, Sui Jingfeng wrote:
> Currently, the various display bridge drivers rely on OF infrastructures
> to works very well, yet there are platforms and/or devices absence of 'OF'
> support. Such as virtual display drivers, USB display apapters and ACPI
> based systems etc.
>
> Add fwnode based helpers to fill the niche, this allows part of the display
> bridge drivers to work across systems. As the fwnode API has wider coverage
> than DT counterpart and the fwnode graphs are compatible with the OF graph,
> so the provided helpers can be used on all systems in theory. Assumed that
> the system has valid fwnode graphs established before drm bridge drivers
> are probed, and there has fwnode assigned to involved drm bridge instance.
>
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
> drivers/gpu/drm/drm_bridge.c | 74 ++++++++++++++++++++++++++++++++++++
> include/drm/drm_bridge.h | 16 ++++++++
> 2 files changed, 90 insertions(+)
>

[skipped]

> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 4baca0d9107b..a3f5d12a308c 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -26,6 +26,7 @@
> #include <linux/ctype.h>
> #include <linux/list.h>
> #include <linux/mutex.h>
> +#include <linux/of.h>
>
> #include <drm/drm_atomic.h>
> #include <drm/drm_encoder.h>
> @@ -721,6 +722,8 @@ struct drm_bridge {
> struct list_head chain_node;
> /** @of_node: device node pointer to the bridge */
> struct device_node *of_node;
> + /** @fwnode: fwnode pointer to the bridge */
> + struct fwnode_handle *fwnode;

My comment is still the same: plese replace of_node with fwnode. It is
more intrusive, however it will lower the possible confusion if the
driver sets both of_node and fwnode. Also it will remove the necessity
for helpers like drm_bridge_set_node().

> /** @list: to keep track of all added bridges */
> struct list_head list;
> /**
> @@ -788,6 +791,13 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> struct drm_bridge *previous,
> enum drm_bridge_attach_flags flags);
>
> +static inline void
> +drm_bridge_set_node(struct drm_bridge *bridge, struct fwnode_handle *fwnode)
> +{
> + bridge->fwnode = fwnode;
> + bridge->of_node = to_of_node(fwnode);
> +}
> +
> #ifdef CONFIG_OF
> struct drm_bridge *of_drm_find_bridge(struct device_node *np);
> #else
> @@ -797,6 +807,12 @@ static inline struct drm_bridge *of_drm_find_bridge(struct device_node *np)
> }
> #endif
>
> +struct drm_bridge *
> +drm_bridge_find_by_fwnode(struct fwnode_handle *fwnode);
> +
> +struct drm_bridge *
> +drm_bridge_find_next_bridge_by_fwnode(struct fwnode_handle *fwnode, u32 port);
> +
> /**
> * drm_bridge_get_next_bridge() - Get the next bridge in the chain
> * @bridge: bridge object
> --
> 2.34.1
>

--
With best wishes
Dmitry