Hi,
On 5/15/24 17:39, Jani Nikula wrote:
On Tue, 14 May 2024, Sui Jingfeng <sui.jingfeng@xxxxxxxxx> wrote:
The pointer of 'struct device' can also be used as a key to search drm
bridge instance from the global bridge list, traditionally, fwnode and
'OF' based APIs requires the system has decent fwnode/OF Graph support.
While the drm_find_bridge_by_dev() function introduced in this series
don't has such a restriction. It only require you has a pointer to the
backing device. Hence, it may suitable for some small and/or limited
display subsystems.
Also add the drm_bridge_add_with_dev() as a helper, which automatically
set the .of_node field of drm_bridge instances if you call it. But it
suitable for simple bridge drivers which one device backing one drm_bridge
instance.
Signed-off-by: Sui Jingfeng <sui.jingfeng@xxxxxxxxx>
---
drivers/gpu/drm/drm_bridge.c | 39 ++++++++++++++++++++++++++++++++++++
include/drm/drm_bridge.h | 5 +++++
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 584d109330ab..1928d9d0dd3c 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -213,6 +213,23 @@ void drm_bridge_add(struct drm_bridge *bridge)
}
EXPORT_SYMBOL(drm_bridge_add);
+/**
+ * drm_bridge_add_with_dev - add the given bridge to the global bridge list
+ *
+ * @bridge: bridge control structure
+ * @dev: pointer to the kernel device that this bridge is backed.
+ */
+void drm_bridge_add_with_dev(struct drm_bridge *bridge, struct device *dev)
+{
+ if (dev) {
+ bridge->kdev = dev;
+ bridge->of_node = dev->of_node;
+ }
+
+ drm_bridge_add(bridge);
+}
+EXPORT_SYMBOL_GPL(drm_bridge_add_with_dev);
I don't actually have an opinion on whether the dev parameter is useful
or not.
But please don't add a drm_bridge_add_with_dev() and then convert more
than half the drm_bridge_add() users to that. Please just add a struct
device *dev parameter to drm_bridge_add(), and pass NULL if it's not
relevant.
To be honest, previously, I'm just do it exactly same as the way you
told me here. But I'm exhausted and finally give up.
Because this is again need me to modify *all* callers of
drm_bridge_add(), not only those bridges in drm/bridge/, but also
bridge instances in various KMS drivers.
However, their some exceptions just don't fit!
For example, the imx/imx8qxp-pixel-combiner.c just don't fit our
simple model. Our helper function assume that one device backing
one drm_bridge instance (1 to 1). Yet, that driver backing two or
more bridges with one platform device (1 to 2, 1 to 3, ..., ).
Hence, the imx/imx8qxp-pixel-combiner.c just can't use
drm_bridge_add_with_dev().
The aux_hpd_bridge.c is also bad, it store the of_node of struct device
at the .platform_data member of the struct device.
BR,
Jani.
+
static void drm_bridge_remove_void(void *bridge)
{
drm_bridge_remove(bridge);
@@ -1334,6 +1351,27 @@ void drm_bridge_hpd_notify(struct drm_bridge *bridge,
}
EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify);
+struct drm_bridge *drm_find_bridge_by_dev(struct device *kdev)
+{
+ struct drm_bridge *bridge;
+
+ if (!kdev)
+ return NULL;
+
+ mutex_lock(&bridge_lock);
+
+ list_for_each_entry(bridge, &bridge_list, list) {
+ if (bridge->kdev == kdev) {
+ mutex_unlock(&bridge_lock);
+ return bridge;
+ }
+ }
+
+ mutex_unlock(&bridge_lock);
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(drm_find_bridge_by_dev);
+
#ifdef CONFIG_OF
/**
* of_drm_find_bridge - find the bridge corresponding to the device node in
@@ -1361,6 +1399,7 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np)
return NULL;
}
EXPORT_SYMBOL(of_drm_find_bridge);
+
#endif
MODULE_AUTHOR("Ajay Kumar <ajaykumar.rs@xxxxxxxxxxx>");
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4baca0d9107b..70d8393bbd9c 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -715,6 +715,8 @@ struct drm_bridge {
struct drm_private_obj base;
/** @dev: DRM device this bridge belongs to */
struct drm_device *dev;
+ /** @kdev: pointer to the kernel device backing this bridge */
+ struct device *kdev;
/** @encoder: encoder to which this bridge is connected */
struct drm_encoder *encoder;
/** @chain_node: used to form a bridge chain */
@@ -782,12 +784,15 @@ drm_priv_to_bridge(struct drm_private_obj *priv)
}
void drm_bridge_add(struct drm_bridge *bridge);
+void drm_bridge_add_with_dev(struct drm_bridge *bridge, struct device *dev);
int devm_drm_bridge_add(struct device *dev, struct drm_bridge *bridge);
void drm_bridge_remove(struct drm_bridge *bridge);
int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
struct drm_bridge *previous,
enum drm_bridge_attach_flags flags);
+struct drm_bridge *drm_find_bridge_by_dev(struct device *kdev);
+
#ifdef CONFIG_OF
struct drm_bridge *of_drm_find_bridge(struct device_node *np);
#else
--
Best regards
Sui
