Re: [PATCH v2 1/4] drm/bridge: Add fwnode based helpers to get the next bridge

[Sui Jingfeng]

Hi,


On 2024/3/8 03:37, Dmitry Baryshkov wrote:
> On Thu, 7 Mar 2024 at 21:20, Sui Jingfeng <sui.jingfeng@xxxxxxxxx> wrote:
> > Hi,
> >
> >
> > On 2024/3/8 02:43, Dmitry Baryshkov wrote:
> > > > +
> > > >    MODULE_AUTHOR("Ajay Kumar<ajaykumar.rs@xxxxxxxxxxx>");
> > > >    MODULE_DESCRIPTION("DRM bridge infrastructure");
> > > >    MODULE_LICENSE("GPL and additional rights");
> > > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > > > index 3606e1a7f965..d4c95afdd662 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;
> > > In my opinion, if you are adding fwnode, we can drop of_node
> > > completely. There is no need to keep both of them.
> >
> > But the 'struct device' have both of them contained, we should *follow the core*, right?
> > They are two major firmware subsystems (DT and ACPT), both are great and large, right?
> > Personally, I think the drm_bridge should embeds 'struct device', after all, drm bridge
> > are mainly stand for display bridges device. And also to reflect what you said: "to
> > reflect the hardware perfectly" and remove some boilerplate.
> struct device contains both because it is at the root of the hierarchy
> and it should support both API. drm_bridge is a consumer, so it's fine
> to have just one.

What I really means is that
if one day a 'struct device *dev' is embedded into struct drm_bridge,
we only need to improve(modify) the implement ofdrm_bridge_set_node().
This is *key point*. Currently, they(of_node and fwnode) point to the
same thing, it is also fine.

For the various drm bridge drivers implementations, the 'struct drm_bridge'
is also belong to the driver core category. drm bridges are also play the
producer role.

> > I think I'm not good enough to do such a big refactor, sorry. I believe that Maxime
> > and Laurent are the advanced programmers who is good enough to do such things, maybe
> > you can ask them for help?
> Well, you picked up the task ;-)


Well, my subject(title) is "*Allow* to use fwnode based API to get drm bridges".
not "Replace 'OF' with fwnode", My task is to provide an alternative solution
for the possible users. And to help improve code sharing and improve code reuse.
And remove some boilerplate. Others things beyond that is not being taken by
anybody. Thanks.


> But really, there is nothing so hard about it:
> - Change of_node to fw_node, apply an automatic patch changing this in
> bridge drivers.
> - Make drm_of_bridge functions convert passed of_node and comp
>
> After this we can start cleaning up bridge drivers to use fw_node API
> natively as you did in your patches 2-4.


Yes, it's not so hard. But I'm a little busy due to other downstream developing
tasks. Sorry, very sorry!

During the talk with you, I observed that you are very good at fwnode domain.
Are you willing to help the community to do something? For example, currently
the modern drm bridge framework is corrupted by legacy implement, is it possible
for us to migrate them to modern? Instead of rotting there? such as the lontium-lt9611uxc.c
which create a drm connector manually, not modernized yet and it's DT dependent.
So, there are a lot things to do.



Best Regards,
Sui.