Re: [RFC v2 PATCH v3 10/14] drm/panel: add S6E3FA0 driver

Laurent Pinchart
2014-04-21T22:59:53Z

Hi YoungJun,

Thank you for the patch.

On Monday 21 April 2014 21:28:37 YoungJun Cho wrote:
> This patch adds MIPI-DSI command mode based S6E3FA0 AMOLED LCD Panel driver.
>
> Changelog v2:
> - Declares delay, size properties in probe routine instead of DT
> Changelog v3:
> - Moves CPU timings relevant properties from FIMD DT
> (commented by Laurent Pinchart, Andrzej Hajda)
>
> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> Acked-by: Inki Dae <inki.dae@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> drivers/gpu/drm/panel/Kconfig | 7 +
> drivers/gpu/drm/panel/Makefile | 1 +
> drivers/gpu/drm/panel/panel-s6e3fa0.c | 569 ++++++++++++++++++++++++++++++
> 3 files changed, 577 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-s6e3fa0.c

[snip]

> diff --git a/drivers/gpu/drm/panel/panel-s6e3fa0.c
> b/drivers/gpu/drm/panel/panel-s6e3fa0.c new file mode 100644
> index 0000000..1282678
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-s6e3fa0.c
> @@ -0,0 +1,569 @@

[snip]

> +static int s6e3fa0_get_modes(struct drm_panel *panel)
> +{
> + struct drm_connector *connector = panel->connector;
> + struct s6e3fa0 *ctx = panel_to_s6e3fa0(panel);
> + struct drm_display_mode *mode;
> +
> + mode = drm_mode_create(connector->dev);
> + if (!mode) {
> + DRM_ERROR("failed to create a new display mode\n");
> + return 0;
> + }
> +
> + drm_display_mode_from_videomode(&ctx->vm, mode);
> + mode->width_mm = ctx->width_mm;
> + mode->height_mm = ctx->height_mm;
> + connector->display_info.width_mm = mode->width_mm;
> + connector->display_info.height_mm = mode->height_mm;
> +
> + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> + mode->private = (void *)&ctx->cpu_timings;

Wouldn't it make sense to create a new get_interface_params (or similar)
operation for drm_panel to query interface configuration parameters instead of
shoving it in the mode private field ?

> + drm_mode_probed_add(connector, mode);
> +
> + return 1;
> +}

[snip]

--
Regards,

Laurent Pinchart