Received-SPF: pass (google.com: domain of linux-kernel+bounces-208438-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <485eef24-ddad-466a-a89f-f9f226801bb7@gmail.com>
In-Reply-To: <485eef24-ddad-466a-a89f-f9f226801bb7@gmail.com>
From: Doug Anderson <dianders@chromium.org>
Date: Mon, 10 Jun 2024 08:14:40 -0700
Message-ID: <CAD=FV=UZNQ8Beg7mZMNLh8FrenchGVt9SJAPdwHMZ8bPvo57mQ@mail.gmail.com>
Subject: Re: Subject: [PATCH] drm/panel : truly-nt35521: transition to
 mipi_dsi wrapped functions
To: Tejas Vipin <tejasvipin76@gmail.com>
Cc: neil.armstrong@linaro.org, quic_jesszhan@quicinc.com, 
	maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, 
	airlied@gmail.com, daniel@ffwll.ch, dri-devel@lists.freedesktop.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Sat, Jun 8, 2024 at 3:57=E2=80=AFAM Tejas Vipin <tejasvipin76@gmail.com>=
 wrote:
>
> Use functions introduced in 966e397e4f603 ("drm/mipi-dsi: Introduce
> mipi_dsi_*_write_seq_multi()") and f79d6d28d8fe
> ("drm/mipi-dsi: wrap more functions for streamline handling") for the
> sony tulip truly nt35521 panel.

Running "scripts/checkpatch.pl" will yell about the above. You're
supposed to write the word "commit" before the git hash. AKA:

Use functions introduced in commit 966e397e4f603  ("drm/mipi-dsi:
Introduce mipi_dsi_*_write_seq_multi()") and commit f79d6d28d8fe
("drm/mipi-dsi: wrap more functions for streamline handling") for
the...


> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> ---
>  .../panel/panel-sony-tulip-truly-nt35521.c    | 383 +++++++++---------
>  1 file changed, 183 insertions(+), 200 deletions(-)

The subject of your patch has an extra "Subject:" prefix. See:

https://lore.kernel.org/r/485eef24-ddad-466a-a89f-f9f226801bb7@gmail.com

...where you can see "Subject: Subject:". Maybe use "b4" or "patman"
to help you send your patch?


> diff --git a/drivers/gpu/drm/panel/panel-sony-tulip-truly-nt35521.c b/dri=
vers/gpu/drm/panel/panel-sony-tulip-truly-nt35521.c
> index 6d44970dccd9..13472c7c37f5 100644
> --- a/drivers/gpu/drm/panel/panel-sony-tulip-truly-nt35521.c
> +++ b/drivers/gpu/drm/panel/panel-sony-tulip-truly-nt35521.c
> @@ -44,248 +44,231 @@ static void truly_nt35521_reset(struct truly_nt3552=
1 *ctx)
>  static int truly_nt35521_on(struct truly_nt35521 *ctx)
>  {
>         struct mipi_dsi_device *dsi =3D ctx->dsi;
> -       struct device *dev =3D &dsi->dev;
> -       int ret;
>
>         dsi->mode_flags |=3D MIPI_DSI_MODE_LPM;
>
> -       mipi_dsi_generic_write_seq(dsi, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x0=
0);
> -       mipi_dsi_generic_write_seq(dsi, 0xff, 0xaa, 0x55, 0xa5, 0x80);
> -       mipi_dsi_generic_write_seq(dsi, 0x6f, 0x11, 0x00);
> -       mipi_dsi_generic_write_seq(dsi, 0xf7, 0x20, 0x00);
> -       mipi_dsi_generic_write_seq(dsi, 0x6f, 0x01);
> -       mipi_dsi_generic_write_seq(dsi, 0xb1, 0x21);
> -       mipi_dsi_generic_write_seq(dsi, 0xbd, 0x01, 0xa0, 0x10, 0x08, 0x0=
1);
> -       mipi_dsi_generic_write_seq(dsi, 0xb8, 0x01, 0x02, 0x0c, 0x02);
> -       mipi_dsi_generic_write_seq(dsi, 0xbb, 0x11, 0x11);
> -       mipi_dsi_generic_write_seq(dsi, 0xbc, 0x00, 0x00);
> -       mipi_dsi_generic_write_seq(dsi, 0xb6, 0x02);
> -       mipi_dsi_generic_write_seq(dsi, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x0=
1);
> -       mipi_dsi_generic_write_seq(dsi, 0xb0, 0x09, 0x09);
> -       mipi_dsi_generic_write_seq(dsi, 0xb1, 0x09, 0x09);
> -       mipi_dsi_generic_write_seq(dsi, 0xbc, 0x8c, 0x00);
> -       mipi_dsi_generic_write_seq(dsi, 0xbd, 0x8c, 0x00);
> -       mipi_dsi_generic_write_seq(dsi, 0xca, 0x00);
> -       mipi_dsi_generic_write_seq(dsi, 0xc0, 0x04);
> -       mipi_dsi_generic_write_seq(dsi, 0xbe, 0xb5);
> -       mipi_dsi_generic_write_seq(dsi, 0xb3, 0x35, 0x35);
> -       mipi_dsi_generic_write_seq(dsi, 0xb4, 0x25, 0x25);
> -       mipi_dsi_generic_write_seq(dsi, 0xb9, 0x43, 0x43);
> -       mipi_dsi_generic_write_seq(dsi, 0xba, 0x24, 0x24);
> -       mipi_dsi_generic_write_seq(dsi, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x0=
2);
> -       mipi_dsi_generic_write_seq(dsi, 0xee, 0x03);
> -       mipi_dsi_generic_write_seq(dsi, 0xb0,
> +       struct mipi_dsi_multi_context dsi_ctx =3D { .dsi =3D dsi };

Please move the variable declaration above the line "dsi->mode_flags
|=3D MIPI_DSI_MODE_LPM;"


> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52=
, 0x08, 0x00);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xff, 0xaa, 0x55, 0xa5=
, 0x80);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0x6f, 0x11, 0x00);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xf7, 0x20, 0x00);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0x6f, 0x01);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb1, 0x21);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xbd, 0x01, 0xa0, 0x10=
, 0x08, 0x01);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb8, 0x01, 0x02, 0x0c=
, 0x02);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xbb, 0x11, 0x11);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xbc, 0x00, 0x00);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb6, 0x02);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52=
, 0x08, 0x01);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x09, 0x09);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb1, 0x09, 0x09);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xbc, 0x8c, 0x00);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xbd, 0x8c, 0x00);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xca, 0x00);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xc0, 0x04);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xbe, 0xb5);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb3, 0x35, 0x35);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb4, 0x25, 0x25);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb9, 0x43, 0x43);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xba, 0x24, 0x24);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52=
, 0x08, 0x02);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xee, 0x03);
> +       mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0,
>                                    0x00, 0xb2, 0x00, 0xb3, 0x00, 0xb6, 0x=
00, 0xc3,
>                                    0x00, 0xce, 0x00, 0xe1, 0x00, 0xf3, 0x=
01, 0x11);

Please adjust the indentation for the followup lines to match. Right
now they line up properly with the old function name, but not the new
one.


>  static int truly_nt35521_off(struct truly_nt35521 *ctx)
>  {
>         struct mipi_dsi_device *dsi =3D ctx->dsi;
> -       struct device *dev =3D &dsi->dev;
> -       int ret;
>
>         dsi->mode_flags &=3D ~MIPI_DSI_MODE_LPM;
>
> -       ret =3D mipi_dsi_dcs_set_display_off(dsi);
> -       if (ret < 0) {
> -               dev_err(dev, "Failed to set display off: %d\n", ret);
> -               return ret;
> -       }
> -       msleep(50);
> +       struct mipi_dsi_multi_context dsi_ctx =3D { .dsi =3D dsi };

Please move the variable declaration above the line "dsi->mode_flags
&=3D ~MIPI_DSI_MODE_LPM;"

-Doug