Hi Dmitry, On Fri, Apr 17, 2020 at 11:52:11PM +0300, Dmitry Osipenko wrote: > 17.04.2020 23:31, Laurent Pinchart пишет: > > On Fri, Apr 17, 2020 at 10:41:59PM +0300, Dmitry Osipenko wrote: > >> 17.04.2020 22:30, Laurent Pinchart пишет: > >> ... > >>>> #include <drm/drm_atomic.h> > >>>> +#include <drm/drm_bridge.h> > >>> > >>> You could add a forward declaration of struct drm_bridge instead, that > >>> can lower the compilation time a little bit. > >> > >> This include is not only for the struct, but also for the > >> drm_bridge_attach(). It looks to me that it should be nicer to keep the > >> include here. > > > > drm_bridge_attach() is called from .c files. In the .h file you can use > > a forward declaration. It's entirely up to you, but as a general rule, I > > personally try to use forward structure declarations in .h files as much > > as possible. > > The current Tegra DRM code is a bit inconsistent in regards to having > forward declarations, it doesn't have them more than have. > > I'll add a forward declaration if there will be need to make a v5, ok? It's up to you, you don't have to use a forward declaration if you don't want to, I was just pointing out what I think is a best practice rule :-) > >> ... > >>>> + port = of_get_child_by_name(output->of_node, "port"); > >>> > >>> Do you need to check for the presence of a port node first ? Can you > >>> just check the return value of drm_of_find_panel_or_bridge(), and fall > >>> back to "nvidia,panel" if it returns -ENODEV ? > >> > >> Without the check, the drm_of_find_panel_or_bridge() prints a very noisy > >> error message about missing port node for every output that doesn't have > >> a graph specified in a device-tree (HDMI, DSI and etc). > >> > >> https://elixir.bootlin.com/linux/v5.7-rc1/source/drivers/of/property.c#L621 > > > > Ah yes indeed. That's not very nice. > > Please let me know if you'll have a better idea about how this could be > handled. It should be good enough as-is I think. You may however want to support both "port" and "ports", as even when there's a single port node, it could be put inside a ports node. -- Regards, Laurent Pinchart
