linux-kernel - Re: [PATCH v5 3/5] drm/panel: panel-jadard-jd9365da-h3: use wrapped MIPI DCS functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <zvkl2wyqp3iem4ln4qkbhgvxafsfn5wkkmqwhufabm2gqs3eqw@vmqs3lx72ekk>
Date: Mon, 24 Jun 2024 18:27:41 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Zhaoxiong Lv <lvzhaoxiong@...qin.corp-partner.google.com>
Cc: dmitry.torokhov@...il.com, robh@...nel.org, 
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org, jikos@...nel.org, 
	benjamin.tissoires@...hat.co, dianders@...gle.com, hsinyi@...gle.com, jagan@...eble.ai, 
	neil.armstrong@...aro.org, quic_jesszhan@...cinc.com, dri-devel@...ts.freedesktop.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 3/5] drm/panel: panel-jadard-jd9365da-h3: use wrapped
 MIPI DCS functions

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@...qin.corp-partner.google.com>
> ---
>  .../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