Re: [PATCH V2 9/9] drm/exynos: Add ps8622 lvds bridge discovery to DP driver

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1800 bytes --]

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@samsung.com>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@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.

> +		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.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]