Re: [PATCH V2 4/9] drm/exynos: add exynos_dp_panel driver registration to drm driver

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

[-- Attachment #1.1: Type: text/plain, Size: 2081 bytes --]

On Tue, Apr 22, 2014 at 04:09:13AM +0530, Ajay Kumar wrote:
> Register exynos_dp_panel before the list of exynos crtcs and
> connectors are probed.
> 
> This is needed because exynos_dp_panel should be registered to
> the drm_panel list via panel-exynos-dp probe, i.e much before
> exynos_dp_bind calls of_drm_find_panel().
> 
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> ---
> Changes since V1:
> 	Added platform_driver_unregister(&exynos_dp_panel_driver) to
> 	exynos_drm_platform_remove as per Jingoo Han's correction
> 
>  drivers/gpu/drm/exynos/exynos_drm_drv.c |   15 +++++++++++++++
>  drivers/gpu/drm/exynos/exynos_drm_drv.h |    1 +
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 1d653f8..2db7f67 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -530,12 +530,23 @@ static int exynos_drm_platform_probe(struct platform_device *pdev)
>  		goto err_unregister_ipp_drv;
>  #endif
>  
> +#ifdef CONFIG_DRM_PANEL_EXYNOS_DP
> +	ret = platform_driver_register(&exynos_dp_panel_driver);
> +	if (ret < 0)
> +		goto err_unregister_dp_panel;
> +#endif

No, this is not how you're supposed to use DRM panel drivers. The idea
is that you write a standalone driver for a given panel.

What you do here has a number of problems. For one it's a driver that's
tightly coupled to Exynos SoCs. But if I have a different SoC that uses
the same panel I want to be able to use the same driver, and not have to
rewrite the driver for my SoC.

Another problem is that you're assuming here that the driver is built in
and it will break if you try to build either Exynos DRM or the panel
driver as a module. This is perhaps nothing you care about right now,
but eventually people will want to ship a single kernel that can run on
a number of SoCs. But if we keep adding things like this, that kernel
will keep growing in size until it no longer fits in any kind of memory.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel