On Mon, Jun 24, 2024 at 10:19:24PM GMT, Zhaoxiong Lv wrote:
> Remove conditional code and always use mipi_dsi_dcs_*multi() wrappers to
> simplify driver's init/enable/exit code.
>
> Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> ---
> .../gpu/drm/panel/panel-jadard-jd9365da-h3.c | 793 +++++++++---------
> 1 file changed, 390 insertions(+), 403 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> index a9c483a7b3fa..e836260338bf 100644
> --- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> +++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> @@ -19,17 +19,13 @@
> #include <linux/of.h>
> #include <linux/regulator/consumer.h>
>
> -#define JD9365DA_INIT_CMD_LEN 2
> -
> -struct jadard_init_cmd {
> - u8 data[JD9365DA_INIT_CMD_LEN];
> -};
> +struct jadard;
>
> struct jadard_panel_desc {
> const struct drm_display_mode mode;
> unsigned int lanes;
> enum mipi_dsi_pixel_format format;
> - const struct jadard_init_cmd *init_cmds;
> + int (*init)(struct jadard *jadard);
> u32 num_init_cmds;
> };
>
> @@ -50,46 +46,33 @@ static inline struct jadard *panel_to_jadard(struct drm_panel *panel)
>
> static int jadard_enable(struct drm_panel *panel)
> {
> - struct device *dev = panel->dev;
> struct jadard *jadard = panel_to_jadard(panel);
> - struct mipi_dsi_device *dsi = jadard->dsi;
> - int err;
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = jadard->dsi };
>
> msleep(120);
>
> - err = mipi_dsi_dcs_exit_sleep_mode(dsi);
> - if (err < 0)
> - DRM_DEV_ERROR(dev, "failed to exit sleep mode ret = %d\n", err);
> + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
>
> - err = mipi_dsi_dcs_set_display_on(dsi);
> - if (err < 0)
> - DRM_DEV_ERROR(dev, "failed to set display on ret = %d\n", err);
> + mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
>
> - return 0;
> + return dsi_ctx.accum_err;
> }
>
> static int jadard_disable(struct drm_panel *panel)
> {
> - struct device *dev = panel->dev;
> struct jadard *jadard = panel_to_jadard(panel);
> - int ret;
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = jadard->dsi };
>
> - ret = mipi_dsi_dcs_set_display_off(jadard->dsi);
> - if (ret < 0)
> - DRM_DEV_ERROR(dev, "failed to set display off: %d\n", ret);
> + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
>
> - ret = mipi_dsi_dcs_enter_sleep_mode(jadard->dsi);
> - if (ret < 0)
> - DRM_DEV_ERROR(dev, "failed to enter sleep mode: %d\n", ret);
> + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>
> - return 0;
> + return dsi_ctx.accum_err;
> }
>
> static int jadard_prepare(struct drm_panel *panel)
> {
> struct jadard *jadard = panel_to_jadard(panel);
> - const struct jadard_panel_desc *desc = jadard->desc;
> - unsigned int i;
> int ret;
>
> ret = regulator_enable(jadard->vccio);
> @@ -109,13 +92,9 @@ static int jadard_prepare(struct drm_panel *panel)
> gpiod_set_value(jadard->reset, 1);
> msleep(130);
>
> - for (i = 0; i < desc->num_init_cmds; i++) {
> - const struct jadard_init_cmd *cmd = &desc->init_cmds[i];
> -
> - ret = mipi_dsi_dcs_write_buffer(dsi, cmd->data, JD9365DA_INIT_CMD_LEN);
This function usesd mipi_dsi_dcs_write_buffer()...
> - if (ret < 0)
> - return ret;
> - }
> + ret = jadard->desc->init(jadard);
> + if (ret)
> + return ret;
>
> return 0;
[...]
> +static int radxa_display_8hd_ad002_init_cmds(struct jadard *jadard)
> +{
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = jadard->dsi };
> +
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xE0, 0x00);
... while your code uses mipi_dsi_dcs_write_seq_multi(), which
internally calls mipi_dsi_generic_write_multi(). These two function use
different packet types to send the payload. To be conservatite, please
use mipi_dsi_dcs_write_buffer_multi().
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xE1, 0x93);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xE2, 0x65);
--
With best wishes
Dmitry
