Hi Jitao.
A few comments in the following.
Sam
On Sat, Oct 12, 2019 at 11:07:16AM +0800, Jitao Shi wrote:
> Auo,kd101n80-45na's connector is same as boe,tv101wum-nl6.
> The most codes can be reuse.
> So auo,kd101n80-45na and boe,tv101wum-nl6 use one driver file.
> Add the different parts in driver data.
>
> Signed-off-by: Jitao Shi <jitao.shi@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/panel/Kconfig | 6 +-
> .../gpu/drm/panel/panel-boe-tv101wum-nl6.c | 86 ++++++++++++++++---
> 2 files changed, 75 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index afcadb3585fb..0e887c978796 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -19,13 +19,13 @@ config DRM_PANEL_ARM_VERSATILE
> in the Versatile family syscon registers.
>
> config DRM_PANEL_BOE_TV101WUM_NL6
> - tristate "BOE TV101WUM 1200x1920 panel"
> + tristate "BOE TV101WUM and AUO KD101N80 45NA 1200x1920 panel"
> depends on OF
> depends on DRM_MIPI_DSI
> depends on BACKLIGHT_CLASS_DEVICE
> help
> - Say Y here if you want to support for BOE TV101WUM WUXGA PANEL
> - DSI Video Mode panel
> + Say Y here if you want to support for BOE TV101WUM and AUO KD101N80
> + 45NA WUXGA PANEL DSI Video Mode panel
>
> config DRM_PANEL_LVDS
> tristate "Generic LVDS panel driver"
> diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> index af68236ea0e8..e6457f87bc61 100644
> --- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> +++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> @@ -35,6 +35,7 @@ struct panel_desc {
> enum mipi_dsi_pixel_format format;
> const struct panel_init_cmd *init_cmds;
> unsigned int lanes;
> + bool discharge_on_disable;
> };
>
> struct boe_panel {
> @@ -372,6 +373,15 @@ static const struct panel_init_cmd boe_init_cmd[] = {
> {},
> };
>
> +static const struct panel_init_cmd auo_kd101n80_45na_init_cmd[] = {
> + _INIT_DELAY_CMD(24),
> + _INIT_DCS_CMD(0x11),
> + _INIT_DELAY_CMD(120),
> + _INIT_DCS_CMD(0x29),
> + _INIT_DELAY_CMD(120),
> + {},
> +};
> +
> static inline struct boe_panel *to_boe_panel(struct drm_panel *panel)
> {
> return container_of(panel, struct boe_panel, base);
> @@ -452,20 +462,30 @@ static int boe_panel_unprepare(struct drm_panel *panel)
> if (!boe->prepared)
> return 0;
>
> - ret = boe_panel_off(boe);
> - if (ret < 0) {
> - dev_err(panel->dev, "failed to set panel off: %d\n", ret);
> - return ret;
> + if (boe->desc->discharge_on_disable) {
> + msleep(150);
> + regulator_disable(boe->avee);
> + regulator_disable(boe->avdd);
> + usleep_range(5000, 7000);
> + gpiod_set_value(boe->enable_gpio, 0);
> + usleep_range(5000, 7000);
> + regulator_disable(boe->pp1800);
The panel is not put into sleep mode in this case.
(Done by boe_panel_off() - which is not the most descriptive name for
that function).
If this is on purpose then consider adding a comment.
> + } else {
> + ret = boe_panel_off(boe);
> + if (ret < 0) {
> + dev_err(panel->dev, "failed to set panel off: %d\n",
> + ret);
> + return ret;
> + }
> + msleep(150);
> + gpiod_set_value(boe->enable_gpio, 0);
> + usleep_range(500, 1000);
> + regulator_disable(boe->avee);
> + regulator_disable(boe->avdd);
> + usleep_range(5000, 7000);
> + regulator_disable(boe->pp1800);
> }
>
> - msleep(150);
> - gpiod_set_value(boe->enable_gpio, 0);
> - usleep_range(500, 1000);
> - regulator_disable(boe->avee);
> - regulator_disable(boe->avdd);
> - usleep_range(5000, 7000);
> - regulator_disable(boe->pp1800);
> -
> boe->prepared = false;
>
> return 0;
> @@ -495,10 +515,14 @@ static int boe_panel_prepare(struct drm_panel *panel)
> if (ret < 0)
> goto poweroffavdd;
>
> - msleep(100);
> + usleep_range(5000, 10000);
>
> gpiod_set_value(boe->enable_gpio, 1);
> - usleep_range(10000, 12000);
> + usleep_range(1000, 2000);
> + gpiod_set_value(boe->enable_gpio, 0);
> + usleep_range(1000, 2000);
> + gpiod_set_value(boe->enable_gpio, 1);
> + usleep_range(6000, 10000);
This looks like a geneeral change.
Consider pushing to the initial commit.
>
> ret = boe_panel_init(boe);
> if (ret < 0) {
> @@ -530,6 +554,8 @@ static int boe_panel_enable(struct drm_panel *panel)
> if (boe->enabled)
> return 0;
>
> + msleep(130);
> +
> ret = backlight_enable(boe->backlight);
> if (ret) {
> dev_err(panel->dev, "Failed to enable backlight %d\n",
> @@ -567,6 +593,35 @@ static const struct panel_desc boe_tv101wum_nl6_desc = {
> .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> MIPI_DSI_MODE_LPM,
> .init_cmds = boe_init_cmd,
> + .discharge_on_disable = false,
> +};
> +
> +static const struct drm_display_mode auo_kd101n80_45na_default_mode = {
> + .clock = 157000,
> + .hdisplay = 1200,
> + .hsync_start = 1200 + 80,
> + .hsync_end = 1200 + 80 + 24,
> + .htotal = 1200 + 80 + 24 + 36,
> + .vdisplay = 1920,
> + .vsync_start = 1920 + 16,
> + .vsync_end = 1920 + 16 + 4,
> + .vtotal = 1920 + 16 + 4 + 16,
> + .vrefresh = 60,
> +};
> +
> +static const struct panel_desc auo_kd101n80_45na_desc = {
> + .modes = &auo_kd101n80_45na_default_mode,
> + .bpc = 8,
> + .size = {
> + .width_mm = 135,
> + .height_mm = 216,
> + },
> + .lanes = 4,
> + .format = MIPI_DSI_FMT_RGB888,
> + .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> + MIPI_DSI_MODE_LPM,
> + .init_cmds = auo_kd101n80_45na_init_cmd,
> + .discharge_on_disable = true,
> };
>
> static int boe_panel_get_modes(struct drm_panel *panel)
> @@ -693,6 +748,9 @@ static const struct of_device_id boe_of_match[] = {
> { .compatible = "boe,tv101wum-nl6",
> .data = &boe_tv101wum_nl6_desc
> },
> + { .compatible = "auo,kd101n80-45na",
> + .data = &auo_kd101n80_45na_desc
> + },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, boe_of_match);
> --
> 2.21.0
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
