Re: [PATCH v2 2/3] drm/panel: ilitek-ili9805: Use dsi_multi in init
From: Doug Anderson
Date: Mon Jul 20 2026 - 16:10:40 EST
Hi,
On Sun, Jul 19, 2026 at 12:04 PM Michail Tatas <michail.tatas@xxxxxxxxx> wrote:
>
> Drop the struct ili9805_instr and the init table
> and use init functions.
>
> Signed-off-by: Michail Tatas <michail.tatas@xxxxxxxxx>
> ---
> drivers/gpu/drm/panel/panel-ilitek-ili9805.c | 156 +++++++++----------
> 1 file changed, 73 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9805.c b/drivers/gpu/drm/panel/panel-ilitek-ili9805.c
> index 7e9587afebbe..a70b9512746a 100644
> --- a/drivers/gpu/drm/panel/panel-ilitek-ili9805.c
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9805.c
> @@ -25,22 +25,9 @@
> #define ILI9805_SETEXTC_PARAMETER2 (0x98)
> #define ILI9805_SETEXTC_PARAMETER3 (0x05)
>
> -#define ILI9805_INSTR(_delay, ...) { \
> - .delay = (_delay), \
> - .len = sizeof((u8[]) {__VA_ARGS__}), \
> - .data = (u8[]){__VA_ARGS__} \
> - }
> -
> -struct ili9805_instr {
> - size_t len;
> - const u8 *data;
> - u32 delay;
> -};
> -
> struct ili9805_desc {
> const char *name;
> - const struct ili9805_instr *init;
> - const size_t init_length;
> + void (*init)(struct mipi_dsi_multi_context *ctx);
> const struct drm_display_mode *mode;
> u32 width_mm;
> u32 height_mm;
> @@ -56,65 +43,78 @@ struct ili9805 {
> struct gpio_desc *reset_gpio;
> };
>
> -static const struct ili9805_instr gpm1780a0_init[] = {
> - ILI9805_INSTR(100, ILI9805_EXTCMD_CMD_SET_ENABLE_REG, ILI9805_SETEXTC_PARAMETER1,
> - ILI9805_SETEXTC_PARAMETER2, ILI9805_SETEXTC_PARAMETER3),
> - ILI9805_INSTR(100, 0xFD, 0x0F, 0x10, 0x44, 0x00),
> - ILI9805_INSTR(0, 0xf8, 0x18, 0x02, 0x02, 0x18, 0x02, 0x02, 0x30, 0x00,
> - 0x00, 0x30, 0x00, 0x00, 0x30, 0x00, 0x00),
> - ILI9805_INSTR(0, 0xB8, 0x62),
> - ILI9805_INSTR(0, 0xF1, 0x00),
> - ILI9805_INSTR(0, 0xF2, 0x00, 0x58, 0x40),
> - ILI9805_INSTR(0, 0xF3, 0x60, 0x83, 0x04),
> - ILI9805_INSTR(0, 0xFC, 0x04, 0x0F, 0x01),
> - ILI9805_INSTR(0, 0xEB, 0x08, 0x0F),
> - ILI9805_INSTR(0, 0xe0, 0x00, 0x08, 0x0d, 0x0e, 0x0e, 0x0d, 0x0a, 0x08, 0x04,
> - 0x08, 0x0d, 0x0f, 0x0b, 0x1c, 0x14, 0x0a),
> - ILI9805_INSTR(0, 0xe1, 0x00, 0x08, 0x0d, 0x0e, 0x0e, 0x0d, 0x0a, 0x08, 0x04,
> - 0x08, 0x0d, 0x0f, 0x0b, 0x1c, 0x14, 0x0a),
> - ILI9805_INSTR(10, 0xc1, 0x13, 0x39, 0x19, 0x06),
> - ILI9805_INSTR(10, 0xc7, 0xe5),
> - ILI9805_INSTR(10, 0xB1, 0x00, 0x12, 0x14),
> - ILI9805_INSTR(10, 0xB4, 0x02),
> - ILI9805_INSTR(0, 0xBB, 0x14, 0x55),
> - ILI9805_INSTR(0, MIPI_DCS_SET_ADDRESS_MODE, 0x08),
> - ILI9805_INSTR(0, MIPI_DCS_SET_PIXEL_FORMAT, 0x77),
> - ILI9805_INSTR(0, 0x20),
> - ILI9805_INSTR(0, 0xB0, 0x01),
> - ILI9805_INSTR(0, 0xB6, 0x31, 0x00, 0xef),
> - ILI9805_INSTR(0, 0xDF, 0x23),
> - ILI9805_INSTR(0, 0xB9, 0x02, 0x00),
> -};
> +static void gpm1780a0_init(struct mipi_dsi_multi_context *ctx)
> +{
> + mipi_dsi_dcs_write_seq_multi(ctx, ILI9805_EXTCMD_CMD_SET_ENABLE_REG, ILI9805_SETEXTC_PARAMETER1,
> + ILI9805_SETEXTC_PARAMETER2, ILI9805_SETEXTC_PARAMETER3);
nit: the indentation of the 2nd line of your two-line
mipi_dsi_dcs_write_seq_multi() seems off. checkpatch.sh --strict
yells:
CHECK: Alignment should match open parenthesis
Strict mode also yells about a few lines that are over 100 characters.
I'm not quite as picky about that, but it would be nic eto fix too...
> + mipi_dsi_msleep(ctx, 100);
> + mipi_dsi_dcs_write_seq_multi(ctx, 0xFD, 0x0F, 0x10, 0x44, 0x00);
> + mipi_dsi_msleep(ctx, 100);
> + mipi_dsi_dcs_write_seq_multi(ctx, 0xf8, 0x18, 0x02, 0x02, 0x18, 0x02, 0x02, 0x30, 0x00,
> + 0x00, 0x30, 0x00, 0x00, 0x30, 0x00, 0x00);
Since you're touching these lines anyway, can you also fix these
functions to consistently use lower case hex instead of (mostly)
uppercase hex? AKA 0xfd rather than 0xFD.
Other than above nits, this looks great to me.
Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>