[RFC PATCH 0/8] Add FS035VG158 panel

[RFC PATCH 0/8] Add FS035VG158 panel

[John Watts]

Hello there,

I have recently done some work to get a new panel running on the kernel.

I am completely new to this kind of work so I don't know how to split my
patches up, especially as I did some light refactoring and fixing on the way.
These changes affect these existing LCD panel but should work.

I'm also not sure if this device tree yaml should be merged with the existing
leadtek,ltk035c5444t device tree yaml.

checkpatch has also warned me about updating MAINTAINERS for the device tree
documentation but I'm not sure if that's relevant here as I have put my name
in the documentation itself.

Thanks for your time,
John.

John Watts (8):
  drm/panel: nv3052c: Document known register names
  drm/panel: nv3052c: Add SPI device IDs
  drm/panel: nv3052c: Sleep for 150ms after reset
  drm/panel: nv3052c: Wait before entering sleep mode
  drm/panel: nv3052c: Allow specifying registers per panel
  drm/panel: nv3052c: Add Fascontek FS035VG158 LCD display
  dt-bindings: vendor-prefixes: Add fascontek
  dt-bindings: display: panel: add Fascontek FS035VG158 panel

 .../display/panel/fascontek,fs035vg158.yaml   |  60 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 .../gpu/drm/panel/panel-newvision-nv3052c.c   | 521 +++++++++++++-----
 3 files changed, 444 insertions(+), 139 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/fascontek,fs035vg158.yaml

-- 
2.42.0

[RFC PATCH 1/8] drm/panel: nv3052c: Document known register names

[John Watts]

Many of these registers have a known name in the public datasheet.
Document them as comments for reference.

Signed-off-by: John Watts <contact@jookia.org>
---
 .../gpu/drm/panel/panel-newvision-nv3052c.c   | 261 +++++++++---------
 1 file changed, 132 insertions(+), 129 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
index 71e57de6d8b2..589431523ce7 100644
--- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
+++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
@@ -42,9 +42,9 @@ struct nv3052c_reg {
 };
 
 static const struct nv3052c_reg nv3052c_panel_regs[] = {
-	{ 0xff, 0x30 },
-	{ 0xff, 0x52 },
-	{ 0xff, 0x01 },
+	// EXTC Command set enable, select page 1
+	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x01 },
+	// Mostly unknown registers
 	{ 0xe3, 0x00 },
 	{ 0x40, 0x00 },
 	{ 0x03, 0x40 },
@@ -62,15 +62,15 @@ static const struct nv3052c_reg nv3052c_panel_regs[] = {
 	{ 0x25, 0x06 },
 	{ 0x26, 0x14 },
 	{ 0x27, 0x14 },
-	{ 0x38, 0xcc },
-	{ 0x39, 0xd7 },
-	{ 0x3a, 0x4a },
+	{ 0x38, 0xcc }, // VCOM_ADJ1
+	{ 0x39, 0xd7 }, // VCOM_ADJ2
+	{ 0x3a, 0x4a }, // VCOM_ADJ3
 	{ 0x28, 0x40 },
 	{ 0x29, 0x01 },
 	{ 0x2a, 0xdf },
 	{ 0x49, 0x3c },
-	{ 0x91, 0x77 },
-	{ 0x92, 0x77 },
+	{ 0x91, 0x77 }, // EXTPW_CTRL2
+	{ 0x92, 0x77 }, // EXTPW_CTRL3
 	{ 0xa0, 0x55 },
 	{ 0xa1, 0x50 },
 	{ 0xa4, 0x9c },
@@ -94,123 +94,126 @@ static const struct nv3052c_reg nv3052c_panel_regs[] = {
 	{ 0xb8, 0x26 },
 	{ 0xf0, 0x00 },
 	{ 0xf6, 0xc0 },
-	{ 0xff, 0x30 },
-	{ 0xff, 0x52 },
-	{ 0xff, 0x02 },
-	{ 0xb0, 0x0b },
-	{ 0xb1, 0x16 },
-	{ 0xb2, 0x17 },
-	{ 0xb3, 0x2c },
-	{ 0xb4, 0x32 },
-	{ 0xb5, 0x3b },
-	{ 0xb6, 0x29 },
-	{ 0xb7, 0x40 },
-	{ 0xb8, 0x0d },
-	{ 0xb9, 0x05 },
-	{ 0xba, 0x12 },
-	{ 0xbb, 0x10 },
-	{ 0xbc, 0x12 },
-	{ 0xbd, 0x15 },
-	{ 0xbe, 0x19 },
-	{ 0xbf, 0x0e },
-	{ 0xc0, 0x16 },
-	{ 0xc1, 0x0a },
-	{ 0xd0, 0x0c },
-	{ 0xd1, 0x17 },
-	{ 0xd2, 0x14 },
-	{ 0xd3, 0x2e },
-	{ 0xd4, 0x32 },
-	{ 0xd5, 0x3c },
-	{ 0xd6, 0x22 },
-	{ 0xd7, 0x3d },
-	{ 0xd8, 0x0d },
-	{ 0xd9, 0x07 },
-	{ 0xda, 0x13 },
-	{ 0xdb, 0x13 },
-	{ 0xdc, 0x11 },
-	{ 0xdd, 0x15 },
-	{ 0xde, 0x19 },
-	{ 0xdf, 0x10 },
-	{ 0xe0, 0x17 },
-	{ 0xe1, 0x0a },
-	{ 0xff, 0x30 },
-	{ 0xff, 0x52 },
-	{ 0xff, 0x03 },
-	{ 0x00, 0x2a },
-	{ 0x01, 0x2a },
-	{ 0x02, 0x2a },
-	{ 0x03, 0x2a },
-	{ 0x04, 0x61 },
-	{ 0x05, 0x80 },
-	{ 0x06, 0xc7 },
-	{ 0x07, 0x01 },
-	{ 0x08, 0x03 },
-	{ 0x09, 0x04 },
-	{ 0x70, 0x22 },
-	{ 0x71, 0x80 },
-	{ 0x30, 0x2a },
-	{ 0x31, 0x2a },
-	{ 0x32, 0x2a },
-	{ 0x33, 0x2a },
-	{ 0x34, 0x61 },
-	{ 0x35, 0xc5 },
-	{ 0x36, 0x80 },
-	{ 0x37, 0x23 },
-	{ 0x40, 0x03 },
-	{ 0x41, 0x04 },
-	{ 0x42, 0x05 },
-	{ 0x43, 0x06 },
-	{ 0x44, 0x11 },
-	{ 0x45, 0xe8 },
-	{ 0x46, 0xe9 },
-	{ 0x47, 0x11 },
-	{ 0x48, 0xea },
-	{ 0x49, 0xeb },
-	{ 0x50, 0x07 },
-	{ 0x51, 0x08 },
-	{ 0x52, 0x09 },
-	{ 0x53, 0x0a },
-	{ 0x54, 0x11 },
-	{ 0x55, 0xec },
-	{ 0x56, 0xed },
-	{ 0x57, 0x11 },
-	{ 0x58, 0xef },
-	{ 0x59, 0xf0 },
-	{ 0xb1, 0x01 },
-	{ 0xb4, 0x15 },
-	{ 0xb5, 0x16 },
-	{ 0xb6, 0x09 },
-	{ 0xb7, 0x0f },
-	{ 0xb8, 0x0d },
-	{ 0xb9, 0x0b },
-	{ 0xba, 0x00 },
-	{ 0xc7, 0x02 },
-	{ 0xca, 0x17 },
-	{ 0xcb, 0x18 },
-	{ 0xcc, 0x0a },
-	{ 0xcd, 0x10 },
-	{ 0xce, 0x0e },
-	{ 0xcf, 0x0c },
-	{ 0xd0, 0x00 },
-	{ 0x81, 0x00 },
-	{ 0x84, 0x15 },
-	{ 0x85, 0x16 },
-	{ 0x86, 0x10 },
-	{ 0x87, 0x0a },
-	{ 0x88, 0x0c },
-	{ 0x89, 0x0e },
-	{ 0x8a, 0x02 },
-	{ 0x97, 0x00 },
-	{ 0x9a, 0x17 },
-	{ 0x9b, 0x18 },
-	{ 0x9c, 0x0f },
-	{ 0x9d, 0x09 },
-	{ 0x9e, 0x0b },
-	{ 0x9f, 0x0d },
-	{ 0xa0, 0x01 },
-	{ 0xff, 0x30 },
-	{ 0xff, 0x52 },
-	{ 0xff, 0x02 },
+	// EXTC Command set enable, select page 2
+	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x02 },
+	// Set gray scale voltage to adjust gamma
+	{ 0xb0, 0x0b }, // PGAMVR0
+	{ 0xb1, 0x16 }, // PGAMVR1
+	{ 0xb2, 0x17 }, // PGAMVR2
+	{ 0xb3, 0x2c }, // PGAMVR3
+	{ 0xb4, 0x32 }, // PGAMVR4
+	{ 0xb5, 0x3b }, // PGAMVR5
+	{ 0xb6, 0x29 }, // PGAMPR0
+	{ 0xb7, 0x40 }, // PGAMPR1
+	{ 0xb8, 0x0d }, // PGAMPK0
+	{ 0xb9, 0x05 }, // PGAMPK1
+	{ 0xba, 0x12 }, // PGAMPK2
+	{ 0xbb, 0x10 }, // PGAMPK3
+	{ 0xbc, 0x12 }, // PGAMPK4
+	{ 0xbd, 0x15 }, // PGAMPK5
+	{ 0xbe, 0x19 }, // PGAMPK6
+	{ 0xbf, 0x0e }, // PGAMPK7
+	{ 0xc0, 0x16 }, // PGAMPK8
+	{ 0xc1, 0x0a }, // PGAMPK9
+	// Set gray scale voltage to adjust gamma
+	{ 0xd0, 0x0c }, // NGAMVR0
+	{ 0xd1, 0x17 }, // NGAMVR0
+	{ 0xd2, 0x14 }, // NGAMVR1
+	{ 0xd3, 0x2e }, // NGAMVR2
+	{ 0xd4, 0x32 }, // NGAMVR3
+	{ 0xd5, 0x3c }, // NGAMVR4
+	{ 0xd6, 0x22 }, // NGAMPR0
+	{ 0xd7, 0x3d }, // NGAMPR1
+	{ 0xd8, 0x0d }, // NGAMPK0
+	{ 0xd9, 0x07 }, // NGAMPK1
+	{ 0xda, 0x13 }, // NGAMPK2
+	{ 0xdb, 0x13 }, // NGAMPK3
+	{ 0xdc, 0x11 }, // NGAMPK4
+	{ 0xdd, 0x15 }, // NGAMPK5
+	{ 0xde, 0x19 }, // NGAMPK6
+	{ 0xdf, 0x10 }, // NGAMPK7
+	{ 0xe0, 0x17 }, // NGAMPK8
+	{ 0xe1, 0x0a }, // NGAMPK9
+	// EXTC Command set enable, select page 3
+	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x03 },
+	// Set various timing settings
+	{ 0x00, 0x2a }, // GIP_VST_1
+	{ 0x01, 0x2a }, // GIP_VST_2
+	{ 0x02, 0x2a }, // GIP_VST_3
+	{ 0x03, 0x2a }, // GIP_VST_4
+	{ 0x04, 0x61 }, // GIP_VST_5
+	{ 0x05, 0x80 }, // GIP_VST_6
+	{ 0x06, 0xc7 }, // GIP_VST_7
+	{ 0x07, 0x01 }, // GIP_VST_8
+	{ 0x08, 0x03 }, // GIP_VST_9
+	{ 0x09, 0x04 }, // GIP_VST_10
+	{ 0x70, 0x22 }, // GIP_ECLK1
+	{ 0x71, 0x80 }, // GIP_ECLK2
+	{ 0x30, 0x2a }, // GIP_CLK_1
+	{ 0x31, 0x2a }, // GIP_CLK_2
+	{ 0x32, 0x2a }, // GIP_CLK_3
+	{ 0x33, 0x2a }, // GIP_CLK_4
+	{ 0x34, 0x61 }, // GIP_CLK_5
+	{ 0x35, 0xc5 }, // GIP_CLK_6
+	{ 0x36, 0x80 }, // GIP_CLK_7
+	{ 0x37, 0x23 }, // GIP_CLK_8
+	{ 0x40, 0x03 }, // GIP_CLKA_1
+	{ 0x41, 0x04 }, // GIP_CLKA_2
+	{ 0x42, 0x05 }, // GIP_CLKA_3
+	{ 0x43, 0x06 }, // GIP_CLKA_4
+	{ 0x44, 0x11 }, // GIP_CLKA_5
+	{ 0x45, 0xe8 }, // GIP_CLKA_6
+	{ 0x46, 0xe9 }, // GIP_CLKA_7
+	{ 0x47, 0x11 }, // GIP_CLKA_8
+	{ 0x48, 0xea }, // GIP_CLKA_9
+	{ 0x49, 0xeb }, // GIP_CLKA_10
+	{ 0x50, 0x07 }, // GIP_CLKB_1
+	{ 0x51, 0x08 }, // GIP_CLKB_2
+	{ 0x52, 0x09 }, // GIP_CLKB_3
+	{ 0x53, 0x0a }, // GIP_CLKB_4
+	{ 0x54, 0x11 }, // GIP_CLKB_5
+	{ 0x55, 0xec }, // GIP_CLKB_6
+	{ 0x56, 0xed }, // GIP_CLKB_7
+	{ 0x57, 0x11 }, // GIP_CLKB_8
+	{ 0x58, 0xef }, // GIP_CLKB_9
+	{ 0x59, 0xf0 }, // GIP_CLKB_10
+	// Map internal GOA signals to GOA output pad
+	{ 0xb1, 0x01 }, // PANELD2U2
+	{ 0xb4, 0x15 }, // PANELD2U5
+	{ 0xb5, 0x16 }, // PANELD2U6
+	{ 0xb6, 0x09 }, // PANELD2U7
+	{ 0xb7, 0x0f }, // PANELD2U8
+	{ 0xb8, 0x0d }, // PANELD2U9
+	{ 0xb9, 0x0b }, // PANELD2U10
+	{ 0xba, 0x00 }, // PANELD2U11
+	{ 0xc7, 0x02 }, // PANELD2U24
+	{ 0xca, 0x17 }, // PANELD2U27
+	{ 0xcb, 0x18 }, // PANELD2U28
+	{ 0xcc, 0x0a }, // PANELD2U29
+	{ 0xcd, 0x10 }, // PANELD2U30
+	{ 0xce, 0x0e }, // PANELD2U31
+	{ 0xcf, 0x0c }, // PANELD2U32
+	{ 0xd0, 0x00 }, // PANELD2U33
+	// Map internal GOA signals to GOA output pad
+	{ 0x81, 0x00 }, // PANELU2D2
+	{ 0x84, 0x15 }, // PANELU2D5
+	{ 0x85, 0x16 }, // PANELU2D6
+	{ 0x86, 0x10 }, // PANELU2D7
+	{ 0x87, 0x0a }, // PANELU2D8
+	{ 0x88, 0x0c }, // PANELU2D9
+	{ 0x89, 0x0e }, // PANELU2D10
+	{ 0x8a, 0x02 }, // PANELU2D11
+	{ 0x97, 0x00 }, // PANELU2D24
+	{ 0x9a, 0x17 }, // PANELU2D27
+	{ 0x9b, 0x18 }, // PANELU2D28
+	{ 0x9c, 0x0f }, // PANELU2D29
+	{ 0x9d, 0x09 }, // PANELU2D30
+	{ 0x9e, 0x0b }, // PANELU2D31
+	{ 0x9f, 0x0d }, // PANELU2D32
+	{ 0xa0, 0x01 }, // PANELU2D33
+	// EXTC Command set enable, select page 2
+	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x02 },
+	// Unknown registers
 	{ 0x01, 0x01 },
 	{ 0x02, 0xda },
 	{ 0x03, 0xba },
@@ -227,10 +230,10 @@ static const struct nv3052c_reg nv3052c_panel_regs[] = {
 	{ 0x0e, 0x48 },
 	{ 0x0f, 0x38 },
 	{ 0x10, 0x2b },
-	{ 0xff, 0x30 },
-	{ 0xff, 0x52 },
-	{ 0xff, 0x00 },
-	{ 0x36, 0x0a },
+	// EXTC Command set enable, select page 0
+	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x00 },
+	// Display Access Control
+	{ 0x36, 0x0a }, // bgr = 1, ss = 1, gs = 0
 };
 
 static inline struct nv3052c *to_nv3052c(struct drm_panel *panel)
-- 
2.42.0

Re: [RFC PATCH 1/8] drm/panel: nv3052c: Document known register names

[Jessica Zhang]

On 9/11/2023 2:01 AM, John Watts wrote:
> Many of these registers have a known name in the public datasheet.
> Document them as comments for reference.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>   .../gpu/drm/panel/panel-newvision-nv3052c.c   | 261 +++++++++---------
>   1 file changed, 132 insertions(+), 129 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> index 71e57de6d8b2..589431523ce7 100644
> --- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> +++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> @@ -42,9 +42,9 @@ struct nv3052c_reg {
>   };
>   
>   static const struct nv3052c_reg nv3052c_panel_regs[] = {
> -	{ 0xff, 0x30 },
> -	{ 0xff, 0x52 },
> -	{ 0xff, 0x01 },
> +	// EXTC Command set enable, select page 1
> +	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x01 },
> +	// Mostly unknown registers

Hi John,

Just curious, what do you mean by these registers being mostly unknown?

I do see them specified in the online specs -- some even seem to map to 
existing MIPI_DCS_* enums (ex. 0x01 to MIPI_DCS_SOFT_RESET, and 0x04 to 
MIPI_DCS_GET_DISPLAY_ID).

Thanks,

Jessica Zhang

>   	{ 0xe3, 0x00 },
>   	{ 0x40, 0x00 },
>   	{ 0x03, 0x40 },
> @@ -62,15 +62,15 @@ static const struct nv3052c_reg nv3052c_panel_regs[] = {
>   	{ 0x25, 0x06 },
>   	{ 0x26, 0x14 },
>   	{ 0x27, 0x14 },
> -	{ 0x38, 0xcc },
> -	{ 0x39, 0xd7 },
> -	{ 0x3a, 0x4a },
> +	{ 0x38, 0xcc }, // VCOM_ADJ1
> +	{ 0x39, 0xd7 }, // VCOM_ADJ2
> +	{ 0x3a, 0x4a }, // VCOM_ADJ3
>   	{ 0x28, 0x40 },
>   	{ 0x29, 0x01 },
>   	{ 0x2a, 0xdf },
>   	{ 0x49, 0x3c },
> -	{ 0x91, 0x77 },
> -	{ 0x92, 0x77 },
> +	{ 0x91, 0x77 }, // EXTPW_CTRL2
> +	{ 0x92, 0x77 }, // EXTPW_CTRL3
>   	{ 0xa0, 0x55 },
>   	{ 0xa1, 0x50 },
>   	{ 0xa4, 0x9c },
> @@ -94,123 +94,126 @@ static const struct nv3052c_reg nv3052c_panel_regs[] = {
>   	{ 0xb8, 0x26 },
>   	{ 0xf0, 0x00 },
>   	{ 0xf6, 0xc0 },
> -	{ 0xff, 0x30 },
> -	{ 0xff, 0x52 },
> -	{ 0xff, 0x02 },
> -	{ 0xb0, 0x0b },
> -	{ 0xb1, 0x16 },
> -	{ 0xb2, 0x17 },
> -	{ 0xb3, 0x2c },
> -	{ 0xb4, 0x32 },
> -	{ 0xb5, 0x3b },
> -	{ 0xb6, 0x29 },
> -	{ 0xb7, 0x40 },
> -	{ 0xb8, 0x0d },
> -	{ 0xb9, 0x05 },
> -	{ 0xba, 0x12 },
> -	{ 0xbb, 0x10 },
> -	{ 0xbc, 0x12 },
> -	{ 0xbd, 0x15 },
> -	{ 0xbe, 0x19 },
> -	{ 0xbf, 0x0e },
> -	{ 0xc0, 0x16 },
> -	{ 0xc1, 0x0a },
> -	{ 0xd0, 0x0c },
> -	{ 0xd1, 0x17 },
> -	{ 0xd2, 0x14 },
> -	{ 0xd3, 0x2e },
> -	{ 0xd4, 0x32 },
> -	{ 0xd5, 0x3c },
> -	{ 0xd6, 0x22 },
> -	{ 0xd7, 0x3d },
> -	{ 0xd8, 0x0d },
> -	{ 0xd9, 0x07 },
> -	{ 0xda, 0x13 },
> -	{ 0xdb, 0x13 },
> -	{ 0xdc, 0x11 },
> -	{ 0xdd, 0x15 },
> -	{ 0xde, 0x19 },
> -	{ 0xdf, 0x10 },
> -	{ 0xe0, 0x17 },
> -	{ 0xe1, 0x0a },
> -	{ 0xff, 0x30 },
> -	{ 0xff, 0x52 },
> -	{ 0xff, 0x03 },
> -	{ 0x00, 0x2a },
> -	{ 0x01, 0x2a },
> -	{ 0x02, 0x2a },
> -	{ 0x03, 0x2a },
> -	{ 0x04, 0x61 },
> -	{ 0x05, 0x80 },
> -	{ 0x06, 0xc7 },
> -	{ 0x07, 0x01 },
> -	{ 0x08, 0x03 },
> -	{ 0x09, 0x04 },
> -	{ 0x70, 0x22 },
> -	{ 0x71, 0x80 },
> -	{ 0x30, 0x2a },
> -	{ 0x31, 0x2a },
> -	{ 0x32, 0x2a },
> -	{ 0x33, 0x2a },
> -	{ 0x34, 0x61 },
> -	{ 0x35, 0xc5 },
> -	{ 0x36, 0x80 },
> -	{ 0x37, 0x23 },
> -	{ 0x40, 0x03 },
> -	{ 0x41, 0x04 },
> -	{ 0x42, 0x05 },
> -	{ 0x43, 0x06 },
> -	{ 0x44, 0x11 },
> -	{ 0x45, 0xe8 },
> -	{ 0x46, 0xe9 },
> -	{ 0x47, 0x11 },
> -	{ 0x48, 0xea },
> -	{ 0x49, 0xeb },
> -	{ 0x50, 0x07 },
> -	{ 0x51, 0x08 },
> -	{ 0x52, 0x09 },
> -	{ 0x53, 0x0a },
> -	{ 0x54, 0x11 },
> -	{ 0x55, 0xec },
> -	{ 0x56, 0xed },
> -	{ 0x57, 0x11 },
> -	{ 0x58, 0xef },
> -	{ 0x59, 0xf0 },
> -	{ 0xb1, 0x01 },
> -	{ 0xb4, 0x15 },
> -	{ 0xb5, 0x16 },
> -	{ 0xb6, 0x09 },
> -	{ 0xb7, 0x0f },
> -	{ 0xb8, 0x0d },
> -	{ 0xb9, 0x0b },
> -	{ 0xba, 0x00 },
> -	{ 0xc7, 0x02 },
> -	{ 0xca, 0x17 },
> -	{ 0xcb, 0x18 },
> -	{ 0xcc, 0x0a },
> -	{ 0xcd, 0x10 },
> -	{ 0xce, 0x0e },
> -	{ 0xcf, 0x0c },
> -	{ 0xd0, 0x00 },
> -	{ 0x81, 0x00 },
> -	{ 0x84, 0x15 },
> -	{ 0x85, 0x16 },
> -	{ 0x86, 0x10 },
> -	{ 0x87, 0x0a },
> -	{ 0x88, 0x0c },
> -	{ 0x89, 0x0e },
> -	{ 0x8a, 0x02 },
> -	{ 0x97, 0x00 },
> -	{ 0x9a, 0x17 },
> -	{ 0x9b, 0x18 },
> -	{ 0x9c, 0x0f },
> -	{ 0x9d, 0x09 },
> -	{ 0x9e, 0x0b },
> -	{ 0x9f, 0x0d },
> -	{ 0xa0, 0x01 },
> -	{ 0xff, 0x30 },
> -	{ 0xff, 0x52 },
> -	{ 0xff, 0x02 },
> +	// EXTC Command set enable, select page 2
> +	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x02 },
> +	// Set gray scale voltage to adjust gamma
> +	{ 0xb0, 0x0b }, // PGAMVR0
> +	{ 0xb1, 0x16 }, // PGAMVR1
> +	{ 0xb2, 0x17 }, // PGAMVR2
> +	{ 0xb3, 0x2c }, // PGAMVR3
> +	{ 0xb4, 0x32 }, // PGAMVR4
> +	{ 0xb5, 0x3b }, // PGAMVR5
> +	{ 0xb6, 0x29 }, // PGAMPR0
> +	{ 0xb7, 0x40 }, // PGAMPR1
> +	{ 0xb8, 0x0d }, // PGAMPK0
> +	{ 0xb9, 0x05 }, // PGAMPK1
> +	{ 0xba, 0x12 }, // PGAMPK2
> +	{ 0xbb, 0x10 }, // PGAMPK3
> +	{ 0xbc, 0x12 }, // PGAMPK4
> +	{ 0xbd, 0x15 }, // PGAMPK5
> +	{ 0xbe, 0x19 }, // PGAMPK6
> +	{ 0xbf, 0x0e }, // PGAMPK7
> +	{ 0xc0, 0x16 }, // PGAMPK8
> +	{ 0xc1, 0x0a }, // PGAMPK9
> +	// Set gray scale voltage to adjust gamma
> +	{ 0xd0, 0x0c }, // NGAMVR0
> +	{ 0xd1, 0x17 }, // NGAMVR0
> +	{ 0xd2, 0x14 }, // NGAMVR1
> +	{ 0xd3, 0x2e }, // NGAMVR2
> +	{ 0xd4, 0x32 }, // NGAMVR3
> +	{ 0xd5, 0x3c }, // NGAMVR4
> +	{ 0xd6, 0x22 }, // NGAMPR0
> +	{ 0xd7, 0x3d }, // NGAMPR1
> +	{ 0xd8, 0x0d }, // NGAMPK0
> +	{ 0xd9, 0x07 }, // NGAMPK1
> +	{ 0xda, 0x13 }, // NGAMPK2
> +	{ 0xdb, 0x13 }, // NGAMPK3
> +	{ 0xdc, 0x11 }, // NGAMPK4
> +	{ 0xdd, 0x15 }, // NGAMPK5
> +	{ 0xde, 0x19 }, // NGAMPK6
> +	{ 0xdf, 0x10 }, // NGAMPK7
> +	{ 0xe0, 0x17 }, // NGAMPK8
> +	{ 0xe1, 0x0a }, // NGAMPK9
> +	// EXTC Command set enable, select page 3
> +	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x03 },
> +	// Set various timing settings
> +	{ 0x00, 0x2a }, // GIP_VST_1
> +	{ 0x01, 0x2a }, // GIP_VST_2
> +	{ 0x02, 0x2a }, // GIP_VST_3
> +	{ 0x03, 0x2a }, // GIP_VST_4
> +	{ 0x04, 0x61 }, // GIP_VST_5
> +	{ 0x05, 0x80 }, // GIP_VST_6
> +	{ 0x06, 0xc7 }, // GIP_VST_7
> +	{ 0x07, 0x01 }, // GIP_VST_8
> +	{ 0x08, 0x03 }, // GIP_VST_9
> +	{ 0x09, 0x04 }, // GIP_VST_10
> +	{ 0x70, 0x22 }, // GIP_ECLK1
> +	{ 0x71, 0x80 }, // GIP_ECLK2
> +	{ 0x30, 0x2a }, // GIP_CLK_1
> +	{ 0x31, 0x2a }, // GIP_CLK_2
> +	{ 0x32, 0x2a }, // GIP_CLK_3
> +	{ 0x33, 0x2a }, // GIP_CLK_4
> +	{ 0x34, 0x61 }, // GIP_CLK_5
> +	{ 0x35, 0xc5 }, // GIP_CLK_6
> +	{ 0x36, 0x80 }, // GIP_CLK_7
> +	{ 0x37, 0x23 }, // GIP_CLK_8
> +	{ 0x40, 0x03 }, // GIP_CLKA_1
> +	{ 0x41, 0x04 }, // GIP_CLKA_2
> +	{ 0x42, 0x05 }, // GIP_CLKA_3
> +	{ 0x43, 0x06 }, // GIP_CLKA_4
> +	{ 0x44, 0x11 }, // GIP_CLKA_5
> +	{ 0x45, 0xe8 }, // GIP_CLKA_6
> +	{ 0x46, 0xe9 }, // GIP_CLKA_7
> +	{ 0x47, 0x11 }, // GIP_CLKA_8
> +	{ 0x48, 0xea }, // GIP_CLKA_9
> +	{ 0x49, 0xeb }, // GIP_CLKA_10
> +	{ 0x50, 0x07 }, // GIP_CLKB_1
> +	{ 0x51, 0x08 }, // GIP_CLKB_2
> +	{ 0x52, 0x09 }, // GIP_CLKB_3
> +	{ 0x53, 0x0a }, // GIP_CLKB_4
> +	{ 0x54, 0x11 }, // GIP_CLKB_5
> +	{ 0x55, 0xec }, // GIP_CLKB_6
> +	{ 0x56, 0xed }, // GIP_CLKB_7
> +	{ 0x57, 0x11 }, // GIP_CLKB_8
> +	{ 0x58, 0xef }, // GIP_CLKB_9
> +	{ 0x59, 0xf0 }, // GIP_CLKB_10
> +	// Map internal GOA signals to GOA output pad
> +	{ 0xb1, 0x01 }, // PANELD2U2
> +	{ 0xb4, 0x15 }, // PANELD2U5
> +	{ 0xb5, 0x16 }, // PANELD2U6
> +	{ 0xb6, 0x09 }, // PANELD2U7
> +	{ 0xb7, 0x0f }, // PANELD2U8
> +	{ 0xb8, 0x0d }, // PANELD2U9
> +	{ 0xb9, 0x0b }, // PANELD2U10
> +	{ 0xba, 0x00 }, // PANELD2U11
> +	{ 0xc7, 0x02 }, // PANELD2U24
> +	{ 0xca, 0x17 }, // PANELD2U27
> +	{ 0xcb, 0x18 }, // PANELD2U28
> +	{ 0xcc, 0x0a }, // PANELD2U29
> +	{ 0xcd, 0x10 }, // PANELD2U30
> +	{ 0xce, 0x0e }, // PANELD2U31
> +	{ 0xcf, 0x0c }, // PANELD2U32
> +	{ 0xd0, 0x00 }, // PANELD2U33
> +	// Map internal GOA signals to GOA output pad
> +	{ 0x81, 0x00 }, // PANELU2D2
> +	{ 0x84, 0x15 }, // PANELU2D5
> +	{ 0x85, 0x16 }, // PANELU2D6
> +	{ 0x86, 0x10 }, // PANELU2D7
> +	{ 0x87, 0x0a }, // PANELU2D8
> +	{ 0x88, 0x0c }, // PANELU2D9
> +	{ 0x89, 0x0e }, // PANELU2D10
> +	{ 0x8a, 0x02 }, // PANELU2D11
> +	{ 0x97, 0x00 }, // PANELU2D24
> +	{ 0x9a, 0x17 }, // PANELU2D27
> +	{ 0x9b, 0x18 }, // PANELU2D28
> +	{ 0x9c, 0x0f }, // PANELU2D29
> +	{ 0x9d, 0x09 }, // PANELU2D30
> +	{ 0x9e, 0x0b }, // PANELU2D31
> +	{ 0x9f, 0x0d }, // PANELU2D32
> +	{ 0xa0, 0x01 }, // PANELU2D33
> +	// EXTC Command set enable, select page 2
> +	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x02 },
> +	// Unknown registers
>   	{ 0x01, 0x01 },
>   	{ 0x02, 0xda },
>   	{ 0x03, 0xba },
> @@ -227,10 +230,10 @@ static const struct nv3052c_reg nv3052c_panel_regs[] = {
>   	{ 0x0e, 0x48 },
>   	{ 0x0f, 0x38 },
>   	{ 0x10, 0x2b },
> -	{ 0xff, 0x30 },
> -	{ 0xff, 0x52 },
> -	{ 0xff, 0x00 },
> -	{ 0x36, 0x0a },
> +	// EXTC Command set enable, select page 0
> +	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x00 },
> +	// Display Access Control
> +	{ 0x36, 0x0a }, // bgr = 1, ss = 1, gs = 0
>   };
>   
>   static inline struct nv3052c *to_nv3052c(struct drm_panel *panel)
> -- 
> 2.42.0
> 

Re: [RFC PATCH 1/8] drm/panel: nv3052c: Document known register names

[John Watts]

On Wed, Sep 13, 2023 at 02:43:43PM -0700, Jessica Zhang wrote:
> Hi John,
> 
> Just curious, what do you mean by these registers being mostly unknown?
> 
> I do see them specified in the online specs -- some even seem to map to
> existing MIPI_DCS_* enums (ex. 0x01 to MIPI_DCS_SOFT_RESET, and 0x04 to
> MIPI_DCS_GET_DISPLAY_ID).
> 
> Thanks,
> 
> Jessica Zhang

Hi Jessica,

Unfortunately these registers are not MIPI ones, but on a separate page of
registers. So page 2 register 1 isn't MIPI_DCS_SOFT_RESET, that is page 0
register 1.

John.

Re: [RFC PATCH 1/8] drm/panel: nv3052c: Document known register names

[Jessica Zhang]

On 9/13/2023 9:12 PM, John Watts wrote:
> On Wed, Sep 13, 2023 at 02:43:43PM -0700, Jessica Zhang wrote:
>> Hi John,
>>
>> Just curious, what do you mean by these registers being mostly unknown?
>>
>> I do see them specified in the online specs -- some even seem to map to
>> existing MIPI_DCS_* enums (ex. 0x01 to MIPI_DCS_SOFT_RESET, and 0x04 to
>> MIPI_DCS_GET_DISPLAY_ID).
>>
>> Thanks,
>>
>> Jessica Zhang
> 
> Hi Jessica,
> 
> Unfortunately these registers are not MIPI ones, but on a separate page of
> registers. So page 2 register 1 isn't MIPI_DCS_SOFT_RESET, that is page 0
> register 1.

Got it -- thanks for the explanation.

In that case,

Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>

Thanks,

Jessica Zhang

> 
> John.

[RFC PATCH 2/8] drm/panel: nv3052c: Add SPI device IDs

[John Watts]

SPI drivers needs their own list of compatible device IDs in order
for automatic module loading to work. Add those for this driver.

Signed-off-by: John Watts <contact@jookia.org>
---
 drivers/gpu/drm/panel/panel-newvision-nv3052c.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
index 589431523ce7..90dea21f9856 100644
--- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
+++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
@@ -465,6 +465,12 @@ static const struct nv3052c_panel_info ltk035c5444t_panel_info = {
 	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
 };
 
+static const struct spi_device_id nv3052c_ids[] = {
+	{ "ltk035c5444t", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(spi, nv3052c_ids);
+
 static const struct of_device_id nv3052c_of_match[] = {
 	{ .compatible = "leadtek,ltk035c5444t", .data = &ltk035c5444t_panel_info },
 	{ /* sentinel */ }
@@ -476,6 +482,7 @@ static struct spi_driver nv3052c_driver = {
 		.name = "nv3052c",
 		.of_match_table = nv3052c_of_match,
 	},
+	.id_table = nv3052c_ids,
 	.probe = nv3052c_probe,
 	.remove = nv3052c_remove,
 };
-- 
2.42.0

Re: [RFC PATCH 2/8] drm/panel: nv3052c: Add SPI device IDs

[Jessica Zhang]

On 9/11/2023 2:02 AM, John Watts wrote:
> SPI drivers needs their own list of compatible device IDs in order
> for automatic module loading to work. Add those for this driver.

Hi John,

Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>

Thanks,

Jessica Zhang

> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>   drivers/gpu/drm/panel/panel-newvision-nv3052c.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> index 589431523ce7..90dea21f9856 100644
> --- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> +++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> @@ -465,6 +465,12 @@ static const struct nv3052c_panel_info ltk035c5444t_panel_info = {
>   	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
>   };
>   
> +static const struct spi_device_id nv3052c_ids[] = {
> +	{ "ltk035c5444t", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(spi, nv3052c_ids);
> +
>   static const struct of_device_id nv3052c_of_match[] = {
>   	{ .compatible = "leadtek,ltk035c5444t", .data = &ltk035c5444t_panel_info },
>   	{ /* sentinel */ }
> @@ -476,6 +482,7 @@ static struct spi_driver nv3052c_driver = {
>   		.name = "nv3052c",
>   		.of_match_table = nv3052c_of_match,
>   	},
> +	.id_table = nv3052c_ids,
>   	.probe = nv3052c_probe,
>   	.remove = nv3052c_remove,
>   };
> -- 
> 2.42.0
> 

[RFC PATCH 3/8] drm/panel: nv3052c: Sleep for 150ms after reset

[John Watts]

The current code waits after resets for 5 to 20 milliseconds.
This is appropriate when resetting a sleeping panel, but an awake panel
requires at least 120ms of waiting.

Sleep for 150ms so the panel always completes it reset properly.

Signed-off-by: John Watts <contact@jookia.org>
---
 drivers/gpu/drm/panel/panel-newvision-nv3052c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
index 90dea21f9856..2526b123b1f5 100644
--- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
+++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
@@ -258,7 +258,7 @@ static int nv3052c_prepare(struct drm_panel *panel)
 	gpiod_set_value_cansleep(priv->reset_gpio, 1);
 	usleep_range(10, 1000);
 	gpiod_set_value_cansleep(priv->reset_gpio, 0);
-	usleep_range(5000, 20000);
+	msleep(150);
 
 	for (i = 0; i < ARRAY_SIZE(nv3052c_panel_regs); i++) {
 		err = mipi_dbi_command(dbi, nv3052c_panel_regs[i].cmd,
-- 
2.42.0

[RFC PATCH 4/8] drm/panel: nv3052c: Wait before entering sleep mode

[John Watts]

The panel needs us to wait 120ms between exiting and entering sleep.
Guarantee that by always waiting 150ms before entering sleep mode.

Signed-off-by: John Watts <contact@jookia.org>
---
 drivers/gpu/drm/panel/panel-newvision-nv3052c.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
index 2526b123b1f5..307335d0f1fc 100644
--- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
+++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
@@ -289,6 +289,9 @@ static int nv3052c_unprepare(struct drm_panel *panel)
 	struct mipi_dbi *dbi = &priv->dbi;
 	int err;
 
+	/* Wait 150ms in case we just exited sleep mode */
+	msleep(150);
+
 	err = mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
 	if (err)
 		dev_err(priv->dev, "Unable to enter sleep mode: %d\n", err);
-- 
2.42.0

[RFC PATCH 5/8] drm/panel: nv3052c: Allow specifying registers per panel

[John Watts]

Panel initialization registers are per-display and not tied to the
controller itself. Different panels will specify their own registers.
Attach the sequences to the panel info struct so future panels
can specify their own sequences.

Signed-off-by: John Watts <contact@jookia.org>
---
 .../gpu/drm/panel/panel-newvision-nv3052c.c   | 25 ++++++++++++-------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
index 307335d0f1fc..b2ad9b3a5eb7 100644
--- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
+++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
@@ -20,11 +20,18 @@
 #include <drm/drm_modes.h>
 #include <drm/drm_panel.h>
 
+struct nv3052c_reg {
+	u8 cmd;
+	u8 val;
+};
+
 struct nv3052c_panel_info {
 	const struct drm_display_mode *display_modes;
 	unsigned int num_modes;
 	u16 width_mm, height_mm;
 	u32 bus_format, bus_flags;
+	const struct nv3052c_reg *panel_regs;
+	int panel_regs_len;
 };
 
 struct nv3052c {
@@ -36,12 +43,7 @@ struct nv3052c {
 	struct gpio_desc *reset_gpio;
 };
 
-struct nv3052c_reg {
-	u8 cmd;
-	u8 val;
-};
-
-static const struct nv3052c_reg nv3052c_panel_regs[] = {
+static const struct nv3052c_reg ltk035c5444t_panel_regs[] = {
 	// EXTC Command set enable, select page 1
 	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x01 },
 	// Mostly unknown registers
@@ -244,6 +246,7 @@ static inline struct nv3052c *to_nv3052c(struct drm_panel *panel)
 static int nv3052c_prepare(struct drm_panel *panel)
 {
 	struct nv3052c *priv = to_nv3052c(panel);
+	const struct nv3052c_reg *panel_regs = priv->panel_info->panel_regs;
 	struct mipi_dbi *dbi = &priv->dbi;
 	unsigned int i;
 	int err;
@@ -260,9 +263,11 @@ static int nv3052c_prepare(struct drm_panel *panel)
 	gpiod_set_value_cansleep(priv->reset_gpio, 0);
 	msleep(150);
 
-	for (i = 0; i < ARRAY_SIZE(nv3052c_panel_regs); i++) {
-		err = mipi_dbi_command(dbi, nv3052c_panel_regs[i].cmd,
-				       nv3052c_panel_regs[i].val);
+	int panel_regs_len = priv->panel_info->panel_regs_len;
+
+	for (i = 0; i < panel_regs_len; i++) {
+		err = mipi_dbi_command(dbi, panel_regs[i].cmd,
+				       panel_regs[i].val);
 
 		if (err) {
 			dev_err(priv->dev, "Unable to set register: %d\n", err);
@@ -466,6 +471,8 @@ static const struct nv3052c_panel_info ltk035c5444t_panel_info = {
 	.height_mm = 64,
 	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
 	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
+	.panel_regs = ltk035c5444t_panel_regs,
+	.panel_regs_len = ARRAY_SIZE(ltk035c5444t_panel_regs),
 };
 
 static const struct spi_device_id nv3052c_ids[] = {
-- 
2.42.0

Re: [RFC PATCH 5/8] drm/panel: nv3052c: Allow specifying registers per panel

[Jessica Zhang]

On 9/11/2023 2:02 AM, John Watts wrote:
> Panel initialization registers are per-display and not tied to the
> controller itself. Different panels will specify their own registers.
> Attach the sequences to the panel info struct so future panels
> can specify their own sequences.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>   .../gpu/drm/panel/panel-newvision-nv3052c.c   | 25 ++++++++++++-------
>   1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> index 307335d0f1fc..b2ad9b3a5eb7 100644
> --- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> +++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> @@ -20,11 +20,18 @@
>   #include <drm/drm_modes.h>
>   #include <drm/drm_panel.h>
>   
> +struct nv3052c_reg {
> +	u8 cmd;
> +	u8 val;
> +};
> +
>   struct nv3052c_panel_info {
>   	const struct drm_display_mode *display_modes;
>   	unsigned int num_modes;
>   	u16 width_mm, height_mm;
>   	u32 bus_format, bus_flags;
> +	const struct nv3052c_reg *panel_regs;
> +	int panel_regs_len;

Hi John,

Having a separate panel_regs_len field seems a bit unnecessary to me.

Looks like it's only being called in the panel prepare() and I don't 
seen any reason why we shouldn't just call the ARRAY_SIZE() macro there.

Thanks,

Jessica Zhang

>   };
>   
>   struct nv3052c {
> @@ -36,12 +43,7 @@ struct nv3052c {
>   	struct gpio_desc *reset_gpio;
>   };
>   
> -struct nv3052c_reg {
> -	u8 cmd;
> -	u8 val;
> -};
> -
> -static const struct nv3052c_reg nv3052c_panel_regs[] = {
> +static const struct nv3052c_reg ltk035c5444t_panel_regs[] = {
>   	// EXTC Command set enable, select page 1
>   	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x01 },
>   	// Mostly unknown registers
> @@ -244,6 +246,7 @@ static inline struct nv3052c *to_nv3052c(struct drm_panel *panel)
>   static int nv3052c_prepare(struct drm_panel *panel)
>   {
>   	struct nv3052c *priv = to_nv3052c(panel);
> +	const struct nv3052c_reg *panel_regs = priv->panel_info->panel_regs;
>   	struct mipi_dbi *dbi = &priv->dbi;
>   	unsigned int i;
>   	int err;
> @@ -260,9 +263,11 @@ static int nv3052c_prepare(struct drm_panel *panel)
>   	gpiod_set_value_cansleep(priv->reset_gpio, 0);
>   	msleep(150);
>   
> -	for (i = 0; i < ARRAY_SIZE(nv3052c_panel_regs); i++) {
> -		err = mipi_dbi_command(dbi, nv3052c_panel_regs[i].cmd,
> -				       nv3052c_panel_regs[i].val);
> +	int panel_regs_len = priv->panel_info->panel_regs_len;
> +
> +	for (i = 0; i < panel_regs_len; i++) {
> +		err = mipi_dbi_command(dbi, panel_regs[i].cmd,
> +				       panel_regs[i].val);
>   
>   		if (err) {
>   			dev_err(priv->dev, "Unable to set register: %d\n", err);
> @@ -466,6 +471,8 @@ static const struct nv3052c_panel_info ltk035c5444t_panel_info = {
>   	.height_mm = 64,
>   	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
>   	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> +	.panel_regs = ltk035c5444t_panel_regs,
> +	.panel_regs_len = ARRAY_SIZE(ltk035c5444t_panel_regs),
>   };
>   
>   static const struct spi_device_id nv3052c_ids[] = {
> -- 
> 2.42.0
> 

Re: [RFC PATCH 5/8] drm/panel: nv3052c: Allow specifying registers per panel

[John Watts]

On Wed, Sep 13, 2023 at 02:34:38PM -0700, Jessica Zhang wrote:
> Hi John,
> 
> Having a separate panel_regs_len field seems a bit unnecessary to me.
> 
> Looks like it's only being called in the panel prepare() and I don't seen
> any reason why we shouldn't just call the ARRAY_SIZE() macro there.

Can you call ARRAY_SIZE on an an array pointer?

> 
> Thanks,
> 
> Jessica Zhang

John.

Re: [RFC PATCH 5/8] drm/panel: nv3052c: Allow specifying registers per panel

[Jessica Zhang]

On 9/13/2023 9:09 PM, John Watts wrote:
> On Wed, Sep 13, 2023 at 02:34:38PM -0700, Jessica Zhang wrote:
>> Hi John,
>>
>> Having a separate panel_regs_len field seems a bit unnecessary to me.
>>
>> Looks like it's only being called in the panel prepare() and I don't seen
>> any reason why we shouldn't just call the ARRAY_SIZE() macro there.
> 
> Can you call ARRAY_SIZE on an an array pointer?

Ah, I'd missed the array pointer declaration. This field is fine then.

Thanks,

Jessica Zhang

> 
>>
>> Thanks,
>>
>> Jessica Zhang
> 
> John.

Re: [RFC PATCH 5/8] drm/panel: nv3052c: Allow specifying registers per panel

[Jessica Zhang]

On 9/11/2023 2:02 AM, John Watts wrote:
> Panel initialization registers are per-display and not tied to the
> controller itself. Different panels will specify their own registers.
> Attach the sequences to the panel info struct so future panels
> can specify their own sequences.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>   .../gpu/drm/panel/panel-newvision-nv3052c.c   | 25 ++++++++++++-------
>   1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> index 307335d0f1fc..b2ad9b3a5eb7 100644
> --- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> +++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> @@ -20,11 +20,18 @@
>   #include <drm/drm_modes.h>
>   #include <drm/drm_panel.h>
>   
> +struct nv3052c_reg {
> +	u8 cmd;
> +	u8 val;
> +};
> +
>   struct nv3052c_panel_info {
>   	const struct drm_display_mode *display_modes;
>   	unsigned int num_modes;
>   	u16 width_mm, height_mm;
>   	u32 bus_format, bus_flags;
> +	const struct nv3052c_reg *panel_regs;
> +	int panel_regs_len;
>   };
>   
>   struct nv3052c {
> @@ -36,12 +43,7 @@ struct nv3052c {
>   	struct gpio_desc *reset_gpio;
>   };
>   
> -struct nv3052c_reg {
> -	u8 cmd;
> -	u8 val;
> -};
> -
> -static const struct nv3052c_reg nv3052c_panel_regs[] = {
> +static const struct nv3052c_reg ltk035c5444t_panel_regs[] = {
>   	// EXTC Command set enable, select page 1
>   	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x01 },
>   	// Mostly unknown registers
> @@ -244,6 +246,7 @@ static inline struct nv3052c *to_nv3052c(struct drm_panel *panel)
>   static int nv3052c_prepare(struct drm_panel *panel)
>   {
>   	struct nv3052c *priv = to_nv3052c(panel);
> +	const struct nv3052c_reg *panel_regs = priv->panel_info->panel_regs;
>   	struct mipi_dbi *dbi = &priv->dbi;
>   	unsigned int i;
>   	int err;
> @@ -260,9 +263,11 @@ static int nv3052c_prepare(struct drm_panel *panel)
>   	gpiod_set_value_cansleep(priv->reset_gpio, 0);
>   	msleep(150);
>   
> -	for (i = 0; i < ARRAY_SIZE(nv3052c_panel_regs); i++) {
> -		err = mipi_dbi_command(dbi, nv3052c_panel_regs[i].cmd,
> -				       nv3052c_panel_regs[i].val);
> +	int panel_regs_len = priv->panel_info->panel_regs_len;

Hi John,

Sorry for not catching this earlier -- can you move the declaration of 
panel_regs_len to the top of the function? Otherwise this throws a 
compiler warning.

With that change,

Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>

Thanks,

Jessica Zhang

> +
> +	for (i = 0; i < panel_regs_len; i++) {
> +		err = mipi_dbi_command(dbi, panel_regs[i].cmd,
> +				       panel_regs[i].val);
>   
>   		if (err) {
>   			dev_err(priv->dev, "Unable to set register: %d\n", err);
> @@ -466,6 +471,8 @@ static const struct nv3052c_panel_info ltk035c5444t_panel_info = {
>   	.height_mm = 64,
>   	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
>   	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> +	.panel_regs = ltk035c5444t_panel_regs,
> +	.panel_regs_len = ARRAY_SIZE(ltk035c5444t_panel_regs),
>   };
>   
>   static const struct spi_device_id nv3052c_ids[] = {
> -- 
> 2.42.0
> 

[RFC PATCH 6/8] drm/panel: nv3052c: Add Fascontek FS035VG158 LCD display

[John Watts]

This display is extremely similar to the LTK035C5444T, but still has
some minor variations in panel initialization.

Signed-off-by: John Watts <contact@jookia.org>
---
 .../gpu/drm/panel/panel-newvision-nv3052c.c   | 223 ++++++++++++++++++
 1 file changed, 223 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
index b2ad9b3a5eb7..3b1ec34491f1 100644
--- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
+++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
@@ -238,6 +238,201 @@ static const struct nv3052c_reg ltk035c5444t_panel_regs[] = {
 	{ 0x36, 0x0a }, // bgr = 1, ss = 1, gs = 0
 };
 
+static const struct nv3052c_reg fs035vg158_panel_regs[] = {
+	// EXTC Command set enable, select page 1
+	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x01 },
+	// Mostly unknown registers
+	{ 0xe3, 0x00 },
+	{ 0x40, 0x00 },
+	{ 0x03, 0x40 },
+	{ 0x04, 0x00 },
+	{ 0x05, 0x03 },
+	{ 0x08, 0x00 },
+	{ 0x09, 0x07 },
+	{ 0x0a, 0x01 },
+	{ 0x0b, 0x32 },
+	{ 0x0c, 0x32 },
+	{ 0x0d, 0x0b },
+	{ 0x0e, 0x00 },
+	{ 0x23, 0x20 }, // RGB interface control: DE MODE PCLK-N
+	{ 0x24, 0x0c },
+	{ 0x25, 0x06 },
+	{ 0x26, 0x14 },
+	{ 0x27, 0x14 },
+	{ 0x38, 0x9c }, //VCOM_ADJ1, different to ltk035c5444t
+	{ 0x39, 0xa7 }, //VCOM_ADJ2, different to ltk035c5444t
+	{ 0x3a, 0x50 }, //VCOM_ADJ3, different to ltk035c5444t
+	{ 0x28, 0x40 },
+	{ 0x29, 0x01 },
+	{ 0x2a, 0xdf },
+	{ 0x49, 0x3c },
+	{ 0x91, 0x57 }, //EXTPW_CTRL2, different to ltk035c5444t
+	{ 0x92, 0x57 }, //EXTPW_CTRL3, different to ltk035c5444t
+	{ 0xa0, 0x55 },
+	{ 0xa1, 0x50 },
+	{ 0xa4, 0x9c },
+	{ 0xa7, 0x02 },
+	{ 0xa8, 0x01 },
+	{ 0xa9, 0x01 },
+	{ 0xaa, 0xfc },
+	{ 0xab, 0x28 },
+	{ 0xac, 0x06 },
+	{ 0xad, 0x06 },
+	{ 0xae, 0x06 },
+	{ 0xaf, 0x03 },
+	{ 0xb0, 0x08 },
+	{ 0xb1, 0x26 },
+	{ 0xb2, 0x28 },
+	{ 0xb3, 0x28 },
+	{ 0xb4, 0x03 }, // Unknown, different to ltk035c5444
+	{ 0xb5, 0x08 },
+	{ 0xb6, 0x26 },
+	{ 0xb7, 0x08 },
+	{ 0xb8, 0x26 },
+	{ 0xf0, 0x00 },
+	{ 0xf6, 0xc0 },
+	// EXTC Command set enable, select page 0
+	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x02 },
+	// Set gray scale voltage to adjust gamma
+	{ 0xb0, 0x0b }, // PGAMVR0
+	{ 0xb1, 0x16 }, // PGAMVR1
+	{ 0xb2, 0x17 }, // PGAMVR2
+	{ 0xb3, 0x2c }, // PGAMVR3
+	{ 0xb4, 0x32 }, // PGAMVR4
+	{ 0xb5, 0x3b }, // PGAMVR5
+	{ 0xb6, 0x29 }, // PGAMPR0
+	{ 0xb7, 0x40 }, // PGAMPR1
+	{ 0xb8, 0x0d }, // PGAMPK0
+	{ 0xb9, 0x05 }, // PGAMPK1
+	{ 0xba, 0x12 }, // PGAMPK2
+	{ 0xbb, 0x10 }, // PGAMPK3
+	{ 0xbc, 0x12 }, // PGAMPK4
+	{ 0xbd, 0x15 }, // PGAMPK5
+	{ 0xbe, 0x19 }, // PGAMPK6
+	{ 0xbf, 0x0e }, // PGAMPK7
+	{ 0xc0, 0x16 }, // PGAMPK8
+	{ 0xc1, 0x0a }, // PGAMPK9
+	// Set gray scale voltage to adjust gamma
+	{ 0xd0, 0x0c }, // NGAMVR0
+	{ 0xd1, 0x17 }, // NGAMVR0
+	{ 0xd2, 0x14 }, // NGAMVR1
+	{ 0xd3, 0x2e }, // NGAMVR2
+	{ 0xd4, 0x32 }, // NGAMVR3
+	{ 0xd5, 0x3c }, // NGAMVR4
+	{ 0xd6, 0x22 }, // NGAMPR0
+	{ 0xd7, 0x3d }, // NGAMPR1
+	{ 0xd8, 0x0d }, // NGAMPK0
+	{ 0xd9, 0x07 }, // NGAMPK1
+	{ 0xda, 0x13 }, // NGAMPK2
+	{ 0xdb, 0x13 }, // NGAMPK3
+	{ 0xdc, 0x11 }, // NGAMPK4
+	{ 0xdd, 0x15 }, // NGAMPK5
+	{ 0xde, 0x19 }, // NGAMPK6
+	{ 0xdf, 0x10 }, // NGAMPK7
+	{ 0xe0, 0x17 }, // NGAMPK8
+	{ 0xe1, 0x0a }, // NGAMPK9
+	// EXTC Command set enable, select page 3
+	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x03 },
+	// Set various timing settings
+	{ 0x00, 0x2a }, // GIP_VST_1
+	{ 0x01, 0x2a }, // GIP_VST_2
+	{ 0x02, 0x2a }, // GIP_VST_3
+	{ 0x03, 0x2a }, // GIP_VST_4
+	{ 0x04, 0x61 }, // GIP_VST_5
+	{ 0x05, 0x80 }, // GIP_VST_6
+	{ 0x06, 0xc7 }, // GIP_VST_7
+	{ 0x07, 0x01 }, // GIP_VST_8
+	{ 0x08, 0x03 }, // GIP_VST_9
+	{ 0x09, 0x04 }, // GIP_VST_10
+	{ 0x70, 0x22 }, // GIP_ECLK1
+	{ 0x71, 0x80 }, // GIP_ECLK2
+	{ 0x30, 0x2a }, // GIP_CLK_1
+	{ 0x31, 0x2a }, // GIP_CLK_2
+	{ 0x32, 0x2a }, // GIP_CLK_3
+	{ 0x33, 0x2a }, // GIP_CLK_4
+	{ 0x34, 0x61 }, // GIP_CLK_5
+	{ 0x35, 0xc5 }, // GIP_CLK_6
+	{ 0x36, 0x80 }, // GIP_CLK_7
+	{ 0x37, 0x23 }, // GIP_CLK_8
+	{ 0x40, 0x03 }, // GIP_CLKA_1
+	{ 0x41, 0x04 }, // GIP_CLKA_2
+	{ 0x42, 0x05 }, // GIP_CLKA_3
+	{ 0x43, 0x06 }, // GIP_CLKA_4
+	{ 0x44, 0x11 }, // GIP_CLKA_5
+	{ 0x45, 0xe8 }, // GIP_CLKA_6
+	{ 0x46, 0xe9 }, // GIP_CLKA_7
+	{ 0x47, 0x11 }, // GIP_CLKA_8
+	{ 0x48, 0xea }, // GIP_CLKA_9
+	{ 0x49, 0xeb }, // GIP_CLKA_10
+	{ 0x50, 0x07 }, // GIP_CLKB_1
+	{ 0x51, 0x08 }, // GIP_CLKB_2
+	{ 0x52, 0x09 }, // GIP_CLKB_3
+	{ 0x53, 0x0a }, // GIP_CLKB_4
+	{ 0x54, 0x11 }, // GIP_CLKB_5
+	{ 0x55, 0xec }, // GIP_CLKB_6
+	{ 0x56, 0xed }, // GIP_CLKB_7
+	{ 0x57, 0x11 }, // GIP_CLKB_8
+	{ 0x58, 0xef }, // GIP_CLKB_9
+	{ 0x59, 0xf0 }, // GIP_CLKB_10
+	// Map internal GOA signals to GOA output pad
+	{ 0xb1, 0x01 }, // PANELD2U2
+	{ 0xb4, 0x15 }, // PANELD2U5
+	{ 0xb5, 0x16 }, // PANELD2U6
+	{ 0xb6, 0x09 }, // PANELD2U7
+	{ 0xb7, 0x0f }, // PANELD2U8
+	{ 0xb8, 0x0d }, // PANELD2U9
+	{ 0xb9, 0x0b }, // PANELD2U10
+	{ 0xba, 0x00 }, // PANELD2U11
+	{ 0xc7, 0x02 }, // PANELD2U24
+	{ 0xca, 0x17 }, // PANELD2U27
+	{ 0xcb, 0x18 }, // PANELD2U28
+	{ 0xcc, 0x0a }, // PANELD2U29
+	{ 0xcd, 0x10 }, // PANELD2U30
+	{ 0xce, 0x0e }, // PANELD2U31
+	{ 0xcf, 0x0c }, // PANELD2U32
+	{ 0xd0, 0x00 }, // PANELD2U33
+	// Map internal GOA signals to GOA output pad
+	{ 0x81, 0x00 }, // PANELU2D2
+	{ 0x84, 0x15 }, // PANELU2D5
+	{ 0x85, 0x16 }, // PANELU2D6
+	{ 0x86, 0x10 }, // PANELU2D7
+	{ 0x87, 0x0a }, // PANELU2D8
+	{ 0x88, 0x0c }, // PANELU2D9
+	{ 0x89, 0x0e }, // PANELU2D10
+	{ 0x8a, 0x02 }, // PANELU2D11
+	{ 0x97, 0x00 }, // PANELU2D24
+	{ 0x9a, 0x17 }, // PANELU2D27
+	{ 0x9b, 0x18 }, // PANELU2D28
+	{ 0x9c, 0x0f }, // PANELU2D29
+	{ 0x9d, 0x09 }, // PANELU2D30
+	{ 0x9e, 0x0b }, // PANELU2D31
+	{ 0x9f, 0x0d }, // PANELU2D32
+	{ 0xa0, 0x01 }, // PANELU2D33
+	// EXTC Command set enable, select page 2
+	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x02 },
+	// Unknown registers
+	{ 0x01, 0x01 },
+	{ 0x02, 0xda },
+	{ 0x03, 0xba },
+	{ 0x04, 0xa8 },
+	{ 0x05, 0x9a },
+	{ 0x06, 0x70 },
+	{ 0x07, 0xff },
+	{ 0x08, 0x91 },
+	{ 0x09, 0x90 },
+	{ 0x0a, 0xff },
+	{ 0x0b, 0x8f },
+	{ 0x0c, 0x60 },
+	{ 0x0d, 0x58 },
+	{ 0x0e, 0x48 },
+	{ 0x0f, 0x38 },
+	{ 0x10, 0x2b },
+	// EXTC Command set enable, select page 0
+	{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x00 },
+	// Display Access Control
+	{ 0x36, 0x0a }, // bgr = 1, ss = 1, gs = 0
+};
+
 static inline struct nv3052c *to_nv3052c(struct drm_panel *panel)
 {
 	return container_of(panel, struct nv3052c, panel);
@@ -464,6 +659,21 @@ static const struct drm_display_mode ltk035c5444t_modes[] = {
 	},
 };
 
+static const struct drm_display_mode fs035vg158_modes[] = {
+	{ /* 60 Hz */
+		.clock = 21000,
+		.hdisplay = 640,
+		.hsync_start = 640 + 34,
+		.hsync_end = 640 + 34 + 4,
+		.htotal = 640 + 34 + 4 + 20,
+		.vdisplay = 480,
+		.vsync_start = 480 + 12,
+		.vsync_end = 480 + 12 + 4,
+		.vtotal = 480 + 12 + 4 + 6,
+		.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+	},
+};
+
 static const struct nv3052c_panel_info ltk035c5444t_panel_info = {
 	.display_modes = ltk035c5444t_modes,
 	.num_modes = ARRAY_SIZE(ltk035c5444t_modes),
@@ -475,14 +685,27 @@ static const struct nv3052c_panel_info ltk035c5444t_panel_info = {
 	.panel_regs_len = ARRAY_SIZE(ltk035c5444t_panel_regs),
 };
 
+static const struct nv3052c_panel_info fs035vg158_panel_info = {
+	.display_modes = fs035vg158_modes,
+	.num_modes = ARRAY_SIZE(fs035vg158_modes),
+	.width_mm = 70,
+	.height_mm = 53,
+	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
+	.panel_regs = fs035vg158_panel_regs,
+	.panel_regs_len = ARRAY_SIZE(fs035vg158_panel_regs),
+};
+
 static const struct spi_device_id nv3052c_ids[] = {
 	{ "ltk035c5444t", },
+	{ "fs035vg158", },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(spi, nv3052c_ids);
 
 static const struct of_device_id nv3052c_of_match[] = {
 	{ .compatible = "leadtek,ltk035c5444t", .data = &ltk035c5444t_panel_info },
+	{ .compatible = "fascontek,fs035vg158", .data = &fs035vg158_panel_info },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, nv3052c_of_match);
-- 
2.42.0

[RFC PATCH 7/8] dt-bindings: vendor-prefixes: Add fascontek

[John Watts]

Fascontek manufactures LCD panels such as the FS035VG158.

Signed-off-by: John Watts <contact@jookia.org>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 573578db9509..69befb76b6ce 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -468,6 +468,8 @@ patternProperties:
     description: Fairphone B.V.
   "^faraday,.*":
     description: Faraday Technology Corporation
+  "^fascontek,.*":
+    description: Fascontek
   "^fastrax,.*":
     description: Fastrax Oy
   "^fcs,.*":
-- 
2.42.0

Re: [RFC PATCH 7/8] dt-bindings: vendor-prefixes: Add fascontek

[Krzysztof Kozlowski]

On 11/09/2023 11:02, John Watts wrote:
> Fascontek manufactures LCD panels such as the FS035VG158.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


---

This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

Best regards,
Krzysztof

[RFC PATCH 8/8] dt-bindings: display: panel: add Fascontek FS035VG158 panel

[John Watts]

This is a small 3.5" 640x480 IPS LCD panel.

Signed-off-by: John Watts <contact@jookia.org>
---
 .../display/panel/fascontek,fs035vg158.yaml   | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/fascontek,fs035vg158.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/fascontek,fs035vg158.yaml b/Documentation/devicetree/bindings/display/panel/fascontek,fs035vg158.yaml
new file mode 100644
index 000000000000..00d43ef8a33d
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/fascontek,fs035vg158.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/fascontek,fs035vg158.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Fascontek FS035VG158 3.5" (640x480 pixels) 24-bit IPS LCD panel
+
+maintainers:
+  - John Watts <contact@jookia.org>
+
+allOf:
+  - $ref: panel-common.yaml#
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    const: fascontek,fs035vg158
+
+  backlight: true
+  port: true
+  power-supply: true
+  reg: true
+  reset-gpios: true
+
+  spi-3wire: true
+
+required:
+  - compatible
+  - power-supply
+  - reset-gpios
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        panel@0 {
+            compatible = "fascontek,fs035vg158";
+            reg = <0>;
+
+            spi-3wire;
+            spi-max-frequency = <3125000>;
+
+            reset-gpios = <&gpe 2 GPIO_ACTIVE_LOW>;
+
+            backlight = <&backlight>;
+            power-supply = <&vcc>;
+
+            port {
+                panel_input: endpoint {
+                    remote-endpoint = <&panel_output>;
+                };
+            };
+        };
+    };
-- 
2.42.0

Re: [RFC PATCH 8/8] dt-bindings: display: panel: add Fascontek FS035VG158 panel

[Krzysztof Kozlowski]

On 11/09/2023 11:02, John Watts wrote:
> This is a small 3.5" 640x480 IPS LCD panel.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---

Thank you for your patch. There is something to discuss/improve.

> +maintainers:
> +  - John Watts <contact@jookia.org>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: fascontek,fs035vg158
> +
> +  backlight: true
> +  port: true
> +  power-supply: true
> +  reg: true
> +  reset-gpios: true

Why do you need all these 5? They are allowed by panel-common.

> +
> +  spi-3wire: true
> +
> +required:
> +  - compatible

Missing reg. Probably also port.


> +  - power-supply
> +  - reset-gpios
> +


Best regards,
Krzysztof

Re: [RFC PATCH 8/8] dt-bindings: display: panel: add Fascontek FS035VG158 panel

[John Watts]

Hello again,

On Mon, Sep 11, 2023 at 11:41:12AM +0200, Krzysztof Kozlowski wrote:
> > +maintainers:
> > +  - John Watts <contact@jookia.org>
> > +
> > +allOf:
> > +  - $ref: panel-common.yaml#
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: fascontek,fs035vg158
> > +
> > +  backlight: true
> > +  port: true
> > +  power-supply: true
> > +  reg: true
> > +  reset-gpios: true
> 
> Why do you need all these 5? They are allowed by panel-common.
> 
> > +
> > +  spi-3wire: true
> > +
> > +required:
> > +  - compatible
> 
> Missing reg. Probably also port.
> 
> 
> > +  - power-supply
> > +  - reset-gpios
> > +

I have just copy pasted the other panel's yaml, so these issues apply
there too. Should I fix that panel's yaml up first then re-copy?

> 
> 
> Best regards,
> Krzysztof
> 

John.

Re: [RFC PATCH 8/8] dt-bindings: display: panel: add Fascontek FS035VG158 panel

[Krzysztof Kozlowski]

On 11/09/2023 11:49, John Watts wrote:
> Hello again,
> 
> On Mon, Sep 11, 2023 at 11:41:12AM +0200, Krzysztof Kozlowski wrote:
>>> +maintainers:
>>> +  - John Watts <contact@jookia.org>
>>> +
>>> +allOf:
>>> +  - $ref: panel-common.yaml#
>>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: fascontek,fs035vg158
>>> +
>>> +  backlight: true
>>> +  port: true
>>> +  power-supply: true
>>> +  reg: true
>>> +  reset-gpios: true
>>
>> Why do you need all these 5? They are allowed by panel-common.
>>
>>> +
>>> +  spi-3wire: true
>>> +
>>> +required:
>>> +  - compatible
>>
>> Missing reg. Probably also port.
>>
>>
>>> +  - power-supply
>>> +  - reset-gpios
>>> +
> 
> I have just copy pasted the other panel's yaml, so these issues apply
> there too. Should I fix that panel's yaml up first then re-copy?

If the other panel has exactly the same case, then yes, you can do like
this. But it depends on the bindings - to which ones do you refer as
your tmeplate?

Best regards,
Krzysztof

Re: [RFC PATCH 8/8] dt-bindings: display: panel: add Fascontek FS035VG158 panel

[John Watts]

On Mon, Sep 11, 2023 at 01:49:39PM +0200, Krzysztof Kozlowski wrote:
> If the other panel has exactly the same case, then yes, you can do like
> this. But it depends on the bindings - to which ones do you refer as
> your tmeplate?

Documentation/devicetree/bindings/display/panel/leadtek,ltk035c5444t.yaml

> 
> Best regards,
> Krzysztof

John.

Re: [RFC PATCH 8/8] dt-bindings: display: panel: add Fascontek FS035VG158 panel

[Krzysztof Kozlowski]

On 11/09/2023 18:47, John Watts wrote:
> On Mon, Sep 11, 2023 at 01:49:39PM +0200, Krzysztof Kozlowski wrote:
>> If the other panel has exactly the same case, then yes, you can do like
>> this. But it depends on the bindings - to which ones do you refer as
>> your tmeplate?
> 
> Documentation/devicetree/bindings/display/panel/leadtek,ltk035c5444t.yaml

The file is indeed serving as poor example.

Best regards,
Krzysztof

Re: [RFC PATCH 8/8] dt-bindings: display: panel: add Fascontek FS035VG158 panel

[John Watts]

On Tue, Sep 12, 2023 at 08:55:31AM +0200, Krzysztof Kozlowski wrote:
> On 11/09/2023 18:47, John Watts wrote:
> > On Mon, Sep 11, 2023 at 01:49:39PM +0200, Krzysztof Kozlowski wrote:
> >> If the other panel has exactly the same case, then yes, you can do like
> >> this. But it depends on the bindings - to which ones do you refer as
> >> your tmeplate?
> > 
> > Documentation/devicetree/bindings/display/panel/leadtek,ltk035c5444t.yaml
> 
> The file is indeed serving as poor example.

I'm happy to fix it up according to your response and include it in v2 of the RFC.
Should this be split in to two RFCs- one for cleanup, one for the new panel?

> 
> Best regards,
> Krzysztof
> 

John.

Re: [RFC PATCH 8/8] dt-bindings: display: panel: add Fascontek FS035VG158 panel

[John Watts]

On Mon, Sep 11, 2023 at 11:41:12AM +0200, Krzysztof Kozlowski wrote:
> Missing reg. Probably also port.

Hello again,

I've been working on v2 of this series and done some initial cleanup.

Right now it looks a bit like this:

> allOf:
>   - $ref: panel-common.yaml#
>   - $ref: /schemas/spi/spi-peripheral-props.yaml#
> 
> properties:
>   compatible:
>     const: fascontek,fs035vg158
> 
>   spi-3wire: true
> 
> required:
>   - compatible
>   - reg
>   - port
>   - power-supply
>   - reset-gpios

Does this seem correct?
- reg is required by spi-controller
- port is listed in panel-common
- power-supply is listed in panel-common

I'm guessing that the required section just lists the minimal
properties needed for this specific device tree yaml?
There's nothing implied by allOf?

> Best regards,
> Krzysztof
> 

John Watts.

Re: [RFC PATCH 8/8] dt-bindings: display: panel: add Fascontek FS035VG158 panel

[Krzysztof Kozlowski]

On 17/09/2023 22:24, John Watts wrote:
> On Mon, Sep 11, 2023 at 11:41:12AM +0200, Krzysztof Kozlowski wrote:
>> Missing reg. Probably also port.
> 
> Hello again,
> 
> I've been working on v2 of this series and done some initial cleanup.
> 
> Right now it looks a bit like this:
> 
>> allOf:
>>   - $ref: panel-common.yaml#
>>   - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>
>> properties:
>>   compatible:
>>     const: fascontek,fs035vg158
>>
>>   spi-3wire: true
>>
>> required:
>>   - compatible
>>   - reg
>>   - port
>>   - power-supply
>>   - reset-gpios
> 
> Does this seem correct?
> - reg is required by spi-controller
> - port is listed in panel-common
> - power-supply is listed in panel-common
> 
> I'm guessing that the required section just lists the minimal
> properties needed for this specific device tree yaml?
> There's nothing implied by allOf?

Ah, then it is fine.

Best regards,
Krzysztof