2024-06-14 17:13:08

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v3 3/4] drm/panel: jd9365da: Support for kd101ne3-40ti MIPI-DSI panel.

On Fri, Jun 14, 2024 at 10:55:09PM GMT, Zhaoxiong Lv wrote:
> The K&d kd101ne3-40ti is a 10.1" WXGA TFT-LCD panel, use
> jd9365da controller,which fits in nicely with the existing
> panel-jadard-jd9365da-h3 driver.Hence,we add a new compatible
> with panel specific config.
>
> Although they have the same control IC, the two panels are different,
> and the timing will be slightly different, so we added some variables
> in struct jadard_panel_desc to control the timing
>
> Signed-off-by: Zhaoxiong Lv <[email protected]>
> ---
>
> Chage since V3:
> - 1. Give up creating a new driver and re-add K&d kd101ne3-40ti
> - configuration to the panel-jadard-jd9365da-h3.c driver.
>
> V2:https://lore.kernel.org/all/20240601084528.22502-3-lvzhaoxiong@huaqin.corp-partner.google.com/
>
> Chage since V2:
>
> - 1. Use the new mipi_dsi_dcs_write_seq_multi() function.
> - 2. Modify Move mipi_dsi_dcs_set_display_off() and mipi_dsi_dcs_enter_sleep_mode() to disable(),
> - and drop kingdisplay_panel_enter_sleep_mode().
> - 3. If prepare fails, disable GPIO before regulators.
> - 4. This function drm_connector_set_panel_orientation() is no longer used. Delete it.
> - 5. Drop ".shutdown = kingdisplay_panel_shutdown".
>
> ---
> .../gpu/drm/panel/panel-jadard-jd9365da-h3.c | 284 ++++++++++++++++++
> 1 file changed, 284 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> index b39f01d7002e..f6e130567707 100644
> --- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> +++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> @@ -27,6 +27,15 @@ struct jadard_panel_desc {
> enum mipi_dsi_pixel_format format;
> int (*init)(struct jadard *jadard);
> u32 num_init_cmds;
> + bool lp11_before_reset;
> + bool power_off_vcioo_before_reset;
> + unsigned int vcioo_to_lp11_delay;
> + unsigned int lp11_to_reset_delay;
> + unsigned int exit_sleep_to_display_on_delay;
> + unsigned int display_on_delay;
> + unsigned int backlight_off_to_display_off_delay;
> + unsigned int display_off_to_enter_sleep_delay;
> + unsigned int enter_sleep_to_reset_down_delay;
> };
>
> struct jadard {
> @@ -57,10 +66,18 @@ static int jadard_enable(struct drm_panel *panel)
> if (err < 0)
> DRM_DEV_ERROR(dev, "failed to exit sleep mode ret = %d\n", err);
>
> + /* tSLPOUT >= 120ms */

The comments are probably applicable to your panel, but not to the other
panels.

> + if (jadard->desc->exit_sleep_to_display_on_delay)
> + msleep(jadard->desc->exit_sleep_to_display_on_delay);

Maybe extend mipi_dsi_msleep to skip slipping if delay is 0 and then use
_multi here too?

> +
> err = mipi_dsi_dcs_set_display_on(dsi);
> if (err < 0)
> DRM_DEV_ERROR(dev, "failed to set display on ret = %d\n", err);
>
> + /* tDISON >= 20ms */
> + if (jadard->desc->display_on_delay)
> + msleep(jadard->desc->display_on_delay);
> +
> return 0;
> }
>
> @@ -70,14 +87,26 @@ static int jadard_disable(struct drm_panel *panel)
> struct jadard *jadard = panel_to_jadard(panel);
> int ret;
>
> + /* tBLOFF:Backlight_to_0x28h >= 100ms */
> + if (jadard->desc->backlight_off_to_display_off_delay)
> + msleep(jadard->desc->backlight_off_to_display_off_delay);
> +
> ret = mipi_dsi_dcs_set_display_off(jadard->dsi);
> if (ret < 0)
> DRM_DEV_ERROR(dev, "failed to set display off: %d\n", ret);
>
> + /* tDISOFF >= 50ms */
> + if (jadard->desc->display_off_to_enter_sleep_delay)
> + msleep(jadard->desc->display_off_to_enter_sleep_delay);
> +
> ret = mipi_dsi_dcs_enter_sleep_mode(jadard->dsi);
> if (ret < 0)
> DRM_DEV_ERROR(dev, "failed to enter sleep mode: %d\n", ret);
>
> + /* tSLPIN >= 100ms */
> + if (jadard->desc->enter_sleep_to_reset_down_delay)
> + msleep(jadard->desc->enter_sleep_to_reset_down_delay);
> +
> return 0;
> }
>
> @@ -94,6 +123,21 @@ static int jadard_prepare(struct drm_panel *panel)
> if (ret)
> return ret;
>
> + /* tMIPI_ON >= 0ms */
> + if (jadard->desc->vcioo_to_lp11_delay)
> + msleep(jadard->desc->vcioo_to_lp11_delay);
> +
> + if (jadard->desc->lp11_before_reset) {
> + ret = mipi_dsi_dcs_nop(jadard->dsi);
> + if (ret < 0)
> + goto poweroff;
> +
> + usleep_range(1000, 2000);
> + }
> + /* tRPWIRES >= 5ms */
> + if (jadard->desc->lp11_to_reset_delay)
> + msleep(jadard->desc->lp11_to_reset_delay);
> +
> gpiod_set_value(jadard->reset, 1);
> msleep(5);
>
> @@ -125,6 +169,12 @@ static int jadard_unprepare(struct drm_panel *panel)
> gpiod_set_value(jadard->reset, 1);
> msleep(120);
>
> + if (jadard->desc->power_off_vcioo_before_reset) {
> + gpiod_set_value(jadard->reset, 0);

The implemented behaviour contradicts with the name of the flag. If the
flag is set, then reset is down before powering down the vccio supply.

> +
> + usleep_range(1000, 2000);
> + }
> +
> regulator_disable(jadard->vdd);
> regulator_disable(jadard->vccio);
>
> @@ -586,7 +636,237 @@ static const struct jadard_panel_desc cz101b4001_desc = {
> .lanes = 4,
> .format = MIPI_DSI_FMT_RGB888,
> .init = cz101b4001_init_cmds,
> +};
> +

[skipped]

> +
> +static const struct jadard_panel_desc kingdisplay_kd101ne3_40ti_desc = {
> + .mode = {
> + .clock = 70595,

Please change this to something like:
(800 + 30 + 30 + 30) * (1280 + 30 + 4 + 8) * 60 / 1000

> +
> + .hdisplay = 800,
> + .hsync_start = 800 + 30,
> + .hsync_end = 800 + 30 + 30,
> + .htotal = 800 + 30 + 30 + 30,
> +
> + .vdisplay = 1280,
> + .vsync_start = 1280 + 30,
> + .vsync_end = 1280 + 30 + 4,
> + .vtotal = 1280 + 30 + 4 + 8,
> +
> + .width_mm = 135,
> + .height_mm = 216,
> + .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> + },
> + .lanes = 4,
> + .format = MIPI_DSI_FMT_RGB888,
> + .init = kingdisplay_kd101ne3_init_cmds,
> + .lp11_before_reset = true,
> + .power_off_vcioo_before_reset = true,
> + .vcioo_to_lp11_delay = 5,
> + .lp11_to_reset_delay = 10,
> + .exit_sleep_to_display_on_delay = 120,
> + .display_on_delay = 20,
> + .backlight_off_to_display_off_delay = 100,
> + .display_off_to_enter_sleep_delay = 50,
> + .enter_sleep_to_reset_down_delay = 100,
> };
>
> static int jadard_dsi_probe(struct mipi_dsi_device *dsi)
> @@ -665,6 +945,10 @@ static const struct of_device_id jadard_of_match[] = {
> .compatible = "radxa,display-8hd-ad002",
> .data = &radxa_display_8hd_ad002_desc
> },
> + {
> + .compatible = "kingdisplay,kd101ne3-40ti",
> + .data = &kingdisplay_kd101ne3_40ti_desc
> + },

Keep it sorted, please.

> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, jadard_of_match);
> --
> 2.17.1
>

--
With best wishes
Dmitry