[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACvgo511gXZfu1kPPQLs4oguCxYUmpcTEioZFk58=522KPHhNQ@mail.gmail.com>
Date: Wed, 14 Mar 2018 12:16:41 +0000
From: Emil Velikov <emil.l.velikov@...il.com>
To: Lin Huang <hl@...k-chips.com>
Cc: Thierry Reding <thierry.reding@...il.com>,
Brian Norris <briannorris@...omium.org>,
Rob Herring <robh+dt@...nel.org>,
Sean Paul <seanpaul@...omium.org>,
David Airlie <airlied@...ux.ie>,
"Linux-Kernel@...r. Kernel. Org" <linux-kernel@...r.kernel.org>,
ML dri-devel <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v4 2/3] drm/panel: support Innolux P097PFG panel
On 14 March 2018 at 09:12, Lin Huang <hl@...k-chips.com> wrote:
> Support Innolux P097PFG 9.7" 1536x2048 TFT LCD panel, it reuse
> the Innolux P079ZCA panel driver.
>
> Change-Id: I97923aa3735f707332681691b0231c9421b427d0
> Signed-off-by: Lin Huang <hl@...k-chips.com>
> ---
> Changes in v2:
> - None
> Changes in v3:
> - None
> Changes in v4:
> - download panel initial code
>
> drivers/gpu/drm/panel/panel-innolux-p079zca.c | 192 +++++++++++++++++++++++++-
> 1 file changed, 187 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> index 2075a9d..883b279 100644
> --- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> +++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> @@ -20,6 +20,15 @@
>
> #include <video/mipi_display.h>
>
> +struct panel_init_cmd {
> + int len;
> + const char *data;
> +};
> +
> +#define _INIT_CMD(...) { \
> + .len = sizeof((char[]){__VA_ARGS__}), \
> + .data = (char[]){__VA_ARGS__} }
> +
> struct panel_desc {
> const struct drm_display_mode *modes;
> unsigned int bpc;
> @@ -30,6 +39,7 @@ struct panel_desc {
>
> unsigned long flags;
> enum mipi_dsi_pixel_format format;
> + const struct panel_init_cmd *init_cmds;
> unsigned int lanes;
> };
>
> @@ -88,9 +98,12 @@ static int innolux_panel_unprepare(struct drm_panel *panel)
> return err;
> }
>
> + /* p097pfg: t15 */
> + msleep(100);
> +
Based on the comment this is not needed for p079zca, yet still executed.
> gpiod_set_value_cansleep(innolux->enable_gpio, 0);
>
> - /* T8: 80ms - 1000ms */
> + /* p079zca: t8*/
> msleep(80);
>
And this is seem the opposite - zca only, yet executed on pfg.
One way to to avoid these and use the appropriate sleep throughout is
to store have the numbers in the respective panel_desc entries.
> regulator_disable(innolux->avee);
> @@ -124,13 +137,43 @@ static int innolux_panel_prepare(struct drm_panel *panel)
> if (err < 0)
> goto disable_avdd;
>
> - /* T2: 15ms - 1000ms */
> - usleep_range(15000, 16000);
> + /* p079zca: t2 (20ms), p097pfg: t4 (15ms) */
> + usleep_range(20000, 21000);
>
> gpiod_set_value_cansleep(innolux->enable_gpio, 1);
>
> - /* T4: 15ms - 1000ms */
> - usleep_range(15000, 16000);
> + /* p079zca: t4, p097pfg: t5 */
> + usleep_range(20000, 21000);
> +
> + if (innolux->desc->init_cmds) {
> + const struct panel_init_cmd *cmds =
> + innolux->desc->init_cmds;
> + int i;
> +
> + for (i = 0; cmds[i].len != 0; i++) {
> + const struct panel_init_cmd *cmd = &cmds[i];
> +
> + err = mipi_dsi_generic_write(innolux->link, cmd->data,
> + cmd->len);
> + if (err < 0) {
> + dev_err(panel->dev,
> + "failed to write command %d\n", i);
> + goto poweroff;
> + }
> +
> + /*
> + * Included by random guessing, because without this
> + * (or at least, some delay), the panel sometimes
> + * didn't appear to pick up the command sequence.
> + */
This seems a bit hackish. Adding a reference to a bugreport/mailing
list discussion would be a nice move.
It'll save you/next person a lot of time searching for the specifics
of the problem.
> + err = mipi_dsi_dcs_nop(innolux->link);
> + if (err < 0) {
> + dev_err(panel->dev,
> + "failed to send DCS nop: %d\n", err);
> + goto poweroff;
> + }
> + }
> + }
>
> err = mipi_dsi_dcs_exit_sleep_mode(innolux->link);
> if (err < 0) {
> @@ -213,6 +256,142 @@ static const struct panel_desc innolux_p079zca_panel_desc = {
> .lanes = 4,
> };
>
> +static const struct drm_display_mode innolux_p097pfg_mode = {
> + .clock = 229000,
> + .hdisplay = 1536,
> + .hsync_start = 1536 + 100,
> + .hsync_end = 1536 + 100 + 24,
> + .htotal = 1536 + 100 + 24 + 100,
> + .vdisplay = 2048,
> + .vsync_start = 2048 + 100,
> + .vsync_end = 2048 + 100 + 2,
> + .vtotal = 2048 + 100 + 2 + 18,
> + .vrefresh = 60,
> +};
> +
> +static const struct panel_init_cmd innolux_p097pfg_init_cmds[] = {
A lot of magic numbers :-(
Please reference the source of these - say XX spec. vY.
We could get a vY+1, which makes the nop workaround obsolete.
HTH
Emil
Powered by blists - more mailing lists