Hi Thierry, On Tue, Apr 22, 2014 at 2:57 PM, Thierry Reding <thierry.reding@xxxxxxxxx> wrote: > On Tue, Apr 22, 2014 at 04:09:18AM +0530, Ajay Kumar wrote: >> This patch adds ps8622 lvds bridge discovery code to the dp driver. >> >> Signed-off-by: Rahul Sharma <Rahul.Sharma@xxxxxxxxxxx> >> Signed-off-by: Ajay Kumar <ajaykumar.rs@xxxxxxxxxxx> >> --- >> Changes since V1: >> Pushing V1 for this as V2 because this patch holds good in this series. >> >> drivers/gpu/drm/exynos/exynos_dp_core.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c >> index 4853f31..0006412 100644 >> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c >> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c >> @@ -30,6 +30,7 @@ >> #include <drm/drm_crtc_helper.h> >> #include <drm/drm_panel.h> >> #include <drm/bridge/ptn3460.h> >> +#include <drm/bridge/ps8622.h> >> >> #include "exynos_drm_drv.h" >> #include "exynos_dp_core.h" >> @@ -999,7 +1000,15 @@ static int exynos_drm_attach_lcd_bridge(struct drm_device *dev, >> panel); >> if (!ret) >> return 1; >> + } else if (find_bridge("parade,ps8625", &bridge)) { > > So this is where the inspiration for the of_find_compatible_node() in > the earlier patch came from. > I shall use phandle for that as suggested by you for previous patches. >> + ret = ps8622_init(dev, encoder, bridge.client, bridge.node, >> + panel); > > Long-term I don't think this is going to scale very well. In my opinion > it would be much more useful to have the bridge driver initialize what > it can and then have the DRM driver call a generic initialization > function to bind it to the DRM device or encoder. Otherwise we have to > keep knowledge about the type of bridge in each driver that uses it, > whereas the goal (I think) would be to create a proper abstraction so > that the DRM driver doesn't have to know that kind of detail. Well, having a generic initialization function makes more sense when we have multiple platforms supporting the 2 available bridge chips. Then we can let the bridge chip initialize first, and then call a drm_bridge search and bind function to search the bridge_list to find the bridge which has already got registered. But, this again poses a challenge that the bridge chip driver should be probed before the corresponding drm_encoder is trying to bind the bridge with drm_device/encoder. So, I think the current way of explicitly calling bridgeXXX_init is fine, at least till multiple platforms start keeping track of all possible bridges they support, inside their respective drivers. Thanks and regards, Ajay Kumar _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel