Re: [PATCH 03/14] drm/panel-sitronix-st7703: Drop custom DSI write macros

2023-01-02 Thread Sam Ravnborg 
Hi Javier,

On Wed, Dec 28, 2022 at 02:47:46AM +0100, Javier Martinez Canillas wrote:
> There are macros for these already in the  header, use
> that instead and delete the custom DSI write macros defined in the driver.
> 
> Signed-off-by: Javier Martinez Canillas 
> ---
> 
>  drivers/gpu/drm/panel/panel-sitronix-st7703.c | 83 ---
>  1 file changed, 33 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7703.c 
> b/drivers/gpu/drm/panel/panel-sitronix-st7703.c
> index 86a472b01360..3e6655c2727e 100644
> --- a/drivers/gpu/drm/panel/panel-sitronix-st7703.c
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7703.c
> @@ -73,14 +73,6 @@ static inline struct st7703 *panel_to_st7703(struct 
> drm_panel *panel)
>   return container_of(panel, struct st7703, panel);
>  }
>  
> -#define dsi_generic_write_seq(dsi, seq...) do {  
> \
> - static const u8 d[] = { seq };  \
> - int ret;\
> - ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));\
> - if (ret < 0)\
> - return ret; \
> - } while (0)
> -
>  static int jh057n_init_sequence(struct st7703 *ctx)
>  {
>   struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> @@ -90,27 +82,27 @@ static int jh057n_init_sequence(struct st7703 *ctx)
>* resemble the ST7703 but the number of parameters often don't match
>* so it's likely a clone.
>*/
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETEXTC,
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETEXTC,
> 0xF1, 0x12, 0x83);
Fix indent here, and in similar places below.
With that fixed:
Reviewed-by: Sam Ravnborg 

> - dsi_generic_write_seq(dsi, ST7703_CMD_SETRGBIF,
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETRGBIF,
> 0x10, 0x10, 0x05, 0x05, 0x03, 0xFF, 0x00, 0x00,
> 0x00, 0x00);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETSCR,
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETSCR,
> 0x73, 0x73, 0x50, 0x50, 0x00, 0x00, 0x08, 0x70,
> 0x00);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETVDC, 0x4E);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETPANEL, 0x0B);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETCYC, 0x80);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETDISP, 0xF0, 0x12, 0x30);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETEQ,
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETVDC, 0x4E);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETPANEL, 0x0B);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETCYC, 0x80);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETDISP, 0xF0, 0x12, 0x30);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETEQ,
> 0x07, 0x07, 0x0B, 0x0B, 0x03, 0x0B, 0x00, 0x00,
> 0x00, 0x00, 0xFF, 0x00, 0xC0, 0x10);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETBGP, 0x08, 0x08);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETBGP, 0x08, 0x08);
>   msleep(20);
>  
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETVCOM, 0x3F, 0x3F);
> - dsi_generic_write_seq(dsi, ST7703_CMD_UNKNOWN_BF, 0x02, 0x11, 0x00);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP1,
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETVCOM, 0x3F, 0x3F);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_UNKNOWN_BF, 0x02, 0x11, 
> 0x00);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP1,
> 0x82, 0x10, 0x06, 0x05, 0x9E, 0x0A, 0xA5, 0x12,
> 0x31, 0x23, 0x37, 0x83, 0x04, 0xBC, 0x27, 0x38,
> 0x0C, 0x00, 0x03, 0x00, 0x00, 0x00, 0x0C, 0x00,
> @@ -119,7 +111,7 @@ static int jh057n_init_sequence(struct st7703 *ctx)
> 0x64, 0x20, 0x88, 0x88, 0x88, 0x88, 0x88, 0x88,
> 0x02, 0x88, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP2,
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP2,
> 0x02, 0x21, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x02, 0x46, 0x02, 0x88,
> 0x88, 0x88, 0x88, 0x88, 0x88, 0x64, 0x88, 0x13,
> @@ -128,7 +120,7 @@ static int jh057n_init_sequence(struct st7703 *ctx)
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x30, 0x0A,
> 0xA5, 0x00, 0x00, 0x00, 0x00);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETGAMMA,
> +

Re: [PATCH 03/14] drm/panel-sitronix-st7703: Drop custom DSI write macros

2022-12-29 Thread Javier Martinez Canillas 
Hello Guido,

On 12/28/22 13:01, Guido Günther wrote:
> Hi Javier,
> Could you please also cc maintainers on the actual macro addition since
> it's hard to review without seeing what the code gets changed to
> (especially when there's multiple revisions). I assume
>

Sure, I will do it if post another revision. Although the changes are quite
trivial and all the drivers define basically the same macro so no functional
changes are expected.
 
>
> https://lore.kernel.org/dri-devel/20221228014757.3170486-2-javi...@redhat.com/
> 
> is the right one?

Correct.

> Cheers,
>  -- Guido
> 
-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 03/14] drm/panel-sitronix-st7703: Drop custom DSI write macros

2022-12-28 Thread Guido Günther 
Hi Javier,
Could you please also cc maintainers on the actual macro addition since
it's hard to review without seeing what the code gets changed to
(especially when there's multiple revisions). I assume

   
https://lore.kernel.org/dri-devel/20221228014757.3170486-2-javi...@redhat.com/

is the right one?
Cheers,
 -- Guido

On Wed, Dec 28, 2022 at 02:47:46AM +0100, Javier Martinez Canillas wrote:
> There are macros for these already in the  header, use
> that instead and delete the custom DSI write macros defined in the driver.
> 
> Signed-off-by: Javier Martinez Canillas 
> ---
> 
>  drivers/gpu/drm/panel/panel-sitronix-st7703.c | 83 ---
>  1 file changed, 33 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7703.c 
> b/drivers/gpu/drm/panel/panel-sitronix-st7703.c
> index 86a472b01360..3e6655c2727e 100644
> --- a/drivers/gpu/drm/panel/panel-sitronix-st7703.c
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7703.c
> @@ -73,14 +73,6 @@ static inline struct st7703 *panel_to_st7703(struct 
> drm_panel *panel)
>   return container_of(panel, struct st7703, panel);
>  }
>  
> -#define dsi_generic_write_seq(dsi, seq...) do {  
> \
> - static const u8 d[] = { seq };  \
> - int ret;\
> - ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));\
> - if (ret < 0)\
> - return ret; \
> - } while (0)
> -
>  static int jh057n_init_sequence(struct st7703 *ctx)
>  {
>   struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> @@ -90,27 +82,27 @@ static int jh057n_init_sequence(struct st7703 *ctx)
>* resemble the ST7703 but the number of parameters often don't match
>* so it's likely a clone.
>*/
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETEXTC,
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETEXTC,
> 0xF1, 0x12, 0x83);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETRGBIF,
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETRGBIF,
> 0x10, 0x10, 0x05, 0x05, 0x03, 0xFF, 0x00, 0x00,
> 0x00, 0x00);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETSCR,
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETSCR,
> 0x73, 0x73, 0x50, 0x50, 0x00, 0x00, 0x08, 0x70,
> 0x00);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETVDC, 0x4E);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETPANEL, 0x0B);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETCYC, 0x80);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETDISP, 0xF0, 0x12, 0x30);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETEQ,
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETVDC, 0x4E);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETPANEL, 0x0B);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETCYC, 0x80);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETDISP, 0xF0, 0x12, 0x30);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETEQ,
> 0x07, 0x07, 0x0B, 0x0B, 0x03, 0x0B, 0x00, 0x00,
> 0x00, 0x00, 0xFF, 0x00, 0xC0, 0x10);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETBGP, 0x08, 0x08);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETBGP, 0x08, 0x08);
>   msleep(20);
>  
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETVCOM, 0x3F, 0x3F);
> - dsi_generic_write_seq(dsi, ST7703_CMD_UNKNOWN_BF, 0x02, 0x11, 0x00);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP1,
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETVCOM, 0x3F, 0x3F);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_UNKNOWN_BF, 0x02, 0x11, 
> 0x00);
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP1,
> 0x82, 0x10, 0x06, 0x05, 0x9E, 0x0A, 0xA5, 0x12,
> 0x31, 0x23, 0x37, 0x83, 0x04, 0xBC, 0x27, 0x38,
> 0x0C, 0x00, 0x03, 0x00, 0x00, 0x00, 0x0C, 0x00,
> @@ -119,7 +111,7 @@ static int jh057n_init_sequence(struct st7703 *ctx)
> 0x64, 0x20, 0x88, 0x88, 0x88, 0x88, 0x88, 0x88,
> 0x02, 0x88, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00);
> - dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP2,
> + mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP2,
> 0x02, 0x21, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x02, 0x46, 0x02, 0x88,
> 0x88, 0x88, 0x88, 0x88, 0x88, 0x64, 0x88, 0x13,
> @@ -128,7 +120,7 @@ static int jh057n_init_sequence(struct st7703 *ctx)
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>

[PATCH 03/14] drm/panel-sitronix-st7703: Drop custom DSI write macros

2022-12-27 Thread Javier Martinez Canillas 
There are macros for these already in the  header, use
that instead and delete the custom DSI write macros defined in the driver.

Signed-off-by: Javier Martinez Canillas 
---

 drivers/gpu/drm/panel/panel-sitronix-st7703.c | 83 ---
 1 file changed, 33 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7703.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7703.c
index 86a472b01360..3e6655c2727e 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7703.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7703.c
@@ -73,14 +73,6 @@ static inline struct st7703 *panel_to_st7703(struct 
drm_panel *panel)
return container_of(panel, struct st7703, panel);
 }
 
-#define dsi_generic_write_seq(dsi, seq...) do {
\
-   static const u8 d[] = { seq };  \
-   int ret;\
-   ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));\
-   if (ret < 0)\
-   return ret; \
-   } while (0)
-
 static int jh057n_init_sequence(struct st7703 *ctx)
 {
struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
@@ -90,27 +82,27 @@ static int jh057n_init_sequence(struct st7703 *ctx)
 * resemble the ST7703 but the number of parameters often don't match
 * so it's likely a clone.
 */
-   dsi_generic_write_seq(dsi, ST7703_CMD_SETEXTC,
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETEXTC,
  0xF1, 0x12, 0x83);
-   dsi_generic_write_seq(dsi, ST7703_CMD_SETRGBIF,
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETRGBIF,
  0x10, 0x10, 0x05, 0x05, 0x03, 0xFF, 0x00, 0x00,
  0x00, 0x00);
-   dsi_generic_write_seq(dsi, ST7703_CMD_SETSCR,
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETSCR,
  0x73, 0x73, 0x50, 0x50, 0x00, 0x00, 0x08, 0x70,
  0x00);
-   dsi_generic_write_seq(dsi, ST7703_CMD_SETVDC, 0x4E);
-   dsi_generic_write_seq(dsi, ST7703_CMD_SETPANEL, 0x0B);
-   dsi_generic_write_seq(dsi, ST7703_CMD_SETCYC, 0x80);
-   dsi_generic_write_seq(dsi, ST7703_CMD_SETDISP, 0xF0, 0x12, 0x30);
-   dsi_generic_write_seq(dsi, ST7703_CMD_SETEQ,
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETVDC, 0x4E);
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETPANEL, 0x0B);
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETCYC, 0x80);
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETDISP, 0xF0, 0x12, 0x30);
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETEQ,
  0x07, 0x07, 0x0B, 0x0B, 0x03, 0x0B, 0x00, 0x00,
  0x00, 0x00, 0xFF, 0x00, 0xC0, 0x10);
-   dsi_generic_write_seq(dsi, ST7703_CMD_SETBGP, 0x08, 0x08);
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETBGP, 0x08, 0x08);
msleep(20);
 
-   dsi_generic_write_seq(dsi, ST7703_CMD_SETVCOM, 0x3F, 0x3F);
-   dsi_generic_write_seq(dsi, ST7703_CMD_UNKNOWN_BF, 0x02, 0x11, 0x00);
-   dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP1,
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETVCOM, 0x3F, 0x3F);
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_UNKNOWN_BF, 0x02, 0x11, 
0x00);
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP1,
  0x82, 0x10, 0x06, 0x05, 0x9E, 0x0A, 0xA5, 0x12,
  0x31, 0x23, 0x37, 0x83, 0x04, 0xBC, 0x27, 0x38,
  0x0C, 0x00, 0x03, 0x00, 0x00, 0x00, 0x0C, 0x00,
@@ -119,7 +111,7 @@ static int jh057n_init_sequence(struct st7703 *ctx)
  0x64, 0x20, 0x88, 0x88, 0x88, 0x88, 0x88, 0x88,
  0x02, 0x88, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00);
-   dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP2,
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP2,
  0x02, 0x21, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  0x00, 0x00, 0x00, 0x00, 0x02, 0x46, 0x02, 0x88,
  0x88, 0x88, 0x88, 0x88, 0x88, 0x64, 0x88, 0x13,
@@ -128,7 +120,7 @@ static int jh057n_init_sequence(struct st7703 *ctx)
  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x30, 0x0A,
  0xA5, 0x00, 0x00, 0x00, 0x00);
-   dsi_generic_write_seq(dsi, ST7703_CMD_SETGAMMA,
+   mipi_dsi_generic_write_seq(dsi, ST7703_CMD_SETGAMMA,
  0x00, 0x09, 0x0E, 0x29, 0x2D, 0x3C, 0x41, 0x37,
  0x07, 0x0B, 0x0D, 0x10, 0x11, 0x0F, 0x10, 0x11,
  0x18, 0x00, 0x09,