Hi Thierry,
On Tue, Apr 22, 2014 at 2:57 PM, Thierry Reding
<thierry.reding at gmail.com> 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 at samsung.com>
>> Signed-off-by: Ajay Kumar <ajaykumar.rs at samsung.com>
>> ---
>> 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
