Add generic panel-dsi to panel-simple

PATCH In review Johan Adolfsson 3 participants 11 messages Apr 18, 2024 – May 13, 2024

Subsystems (4)

Device tree bindings DRM drivers DRM misc DRM panel drivers

lore

Share & tools

lore

Johan Adolfsson

author

2024-04-18 14:01 UTC #0 lore raw

cover Add generic panel-dsi to panel-simple

Adds a generic panel-dsi panel similar to panel-dpi to panel-simple. Allows override of timings, flags and lanes using devicetree.

Signed-off-by Johan Adolfsson <[email protected]>

---
Johan Adolfsson (2):
drm/panel: panel-simple: Add generic panel-dsi driver
dt-bindings: panel-simple-dsi: Add generic panel-dsi

 .../bindings/display/panel/panel-simple-dsi.yaml   |  2 +
 drivers/gpu/drm/panel/panel-simple.c               | 76 +++++++++++++++++++++-
 2 files changed, 75 insertions(+), 3 deletions(-)

---
base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
change-id: 20240308-foo-fix-5ea945a14e2b

Best regards,

Signature · 1 line

Johan Adolfsson <[email protected]>

Johan Adolfsson

author

2024-04-18 14:01 UTC #1 lore raw

PATCH 1/2 drm/panel: panel-simple: Add generic panel-dsi driver

Add generic panel-dsi panel, similar to panel-dpi that can have it's timing, lanes and flags overridden by devicetree.
Add some dev_err() and dev_warn() calls.

Signed-off-by Johan Adolfsson <[email protected]>

---
 drivers/gpu/drm/panel/panel-simple.c | 76 ++++++++++++++++++++++++++++++++++--
 1 file changed, 73 insertions(+), 3 deletions(-)

drivers/gpu/drm/panel/panel-simple.c +73 -3

index d493ee735c73..3b812fb9ae9e 100644
@@ -455,7 +455,9 @@ static const struct drm_panel_funcs panel_simple_funcs = {
 	.get_timings = panel_simple_get_timings,
 };
 
-static struct panel_desc panel_dpi;
+static struct panel_desc panel_dpi = {
+	.connector_type = DRM_MODE_CONNECTOR_DPI
+};
 
 static int panel_dpi_probe(struct device *dev,
 			   struct panel_simple *panel)

@@ -471,6 +473,8 @@ static int panel_dpi_probe(struct device *dev,
 	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
 	if (!desc)
 		return -ENOMEM;
+	if (panel->desc)
+		*desc = *panel->desc;
 
 	timing = devm_kzalloc(dev, sizeof(*timing), GFP_KERNEL);
 	if (!timing)

@@ -503,6 +507,20 @@ static int panel_dpi_probe(struct device *dev,
 	return 0;
 }
 
+static int panel_dsi_probe(struct device *dev,
+			   struct panel_simple *panel)
+{
+	int ret;
+
+	ret = panel_dpi_probe(dev, panel);
+	if (panel->desc) {
+		struct panel_desc *desc = (struct panel_desc *)panel->desc;
+
+		desc->connector_type = DRM_MODE_CONNECTOR_DSI;
+	}
+	return ret;
+}
+
 #define PANEL_SIMPLE_BOUNDS_CHECK(to_check, bounds, field) \
 	(to_check->field.typ >= bounds->field.min && \
 	 to_check->field.typ <= bounds->field.max)

@@ -533,11 +551,16 @@ static void panel_simple_parse_panel_timing_node(struct device *dev,
 		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vactive) ||
 		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vfront_porch) ||
 		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vback_porch) ||
-		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vsync_len))
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vsync_len)) {
+			dev_warn(dev, "timing bounds failed\n");
 			continue;
+		}
 
-		if (ot->flags != dt->flags)
+		/* Allow mismatch in flags for last entry */
+		if (i + 1 < panel->desc->num_timings && ot->flags != dt->flags) {
+			dev_warn(dev, "flags mismatch: ot %x vs dt %x\n", ot->flags, dt->flags);
 			continue;
+		}
 
 		videomode_from_timing(ot, &vm);
 		drm_display_mode_from_videomode(&vm, &panel->override_mode);

@@ -4752,6 +4775,39 @@ struct panel_desc_dsi {
 	unsigned int lanes;
 };
 
+static const struct display_timing generic_800x480_timing = {
+	.pixelclock = { 16400000, 864 * 490 * 60, (1920 + 512) * (1920 + 512) * 60 },
+	.hactive = { 400, 800, 1920 },
+	.hfront_porch = { 2, 16, 512 },
+	.hback_porch = { 2, 16, 512 },
+	.hsync_len = { 1, 10, 40 },
+	.vactive = { 400, 480, 1920 },
+	.vfront_porch = { 2, 5, 512 },
+	.vback_porch = { 2, 5, 512 },
+	.vsync_len = { 1, 10, 20 },
+	.flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
+		 DISPLAY_FLAGS_SYNC_POSEDGE
+};
+
+static struct panel_desc_dsi panel_dsi = {
+	.desc = {
+		.timings = &generic_800x480_timing,
+		.num_timings = 1,
+		.bpc = 8,
+		.size = {
+			.width = 152,
+			.height = 91,
+		},
+		.connector_type = DRM_MODE_CONNECTOR_DSI,
+	 },
+	.flags = MIPI_DSI_MODE_VIDEO |
+		 MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
+		 MIPI_DSI_MODE_VIDEO_HSE | /* Needed to generate Hsync End Short package */
+		 MIPI_DSI_MODE_VIDEO_NO_HBP,
+	.format = MIPI_DSI_FMT_RGB888,
+	.lanes = 2,
+};
+
 static const struct drm_display_mode auo_b080uan01_mode = {
 	.clock = 154500,
 	.hdisplay = 1200,

@@ -4976,6 +5032,9 @@ static const struct of_device_id dsi_of_match[] = {
 	}, {
 		.compatible = "osddisplays,osd101t2045-53ts",
 		.data = &osd101t2045_53ts
+	}, {
+		.compatible = "panel-dsi",
+		.data = &panel_dsi,
 	}, {
 		/* sentinel */
 	}

@@ -4992,17 +5051,28 @@ static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi)
 		return -ENODEV;
 
 	err = panel_simple_probe(&dsi->dev, &desc->desc);
+	if (err)
+		dev_err(&dsi->dev, "%s: err %i\n", __func__, err);
+
+	if (desc == &panel_dsi) {
+		struct panel_simple *panel = mipi_dsi_get_drvdata(dsi);
+		/* Handle the generic panel-dsi binding */
+		err = panel_dsi_probe(&dsi->dev, panel);
+	}
+
 	if (err < 0)
 		return err;
 
 	dsi->mode_flags = desc->flags;
 	dsi->format = desc->format;
 	dsi->lanes = desc->lanes;
+	of_property_read_u32(dsi->dev.of_node, "lanes", &dsi->lanes);
 
 	err = mipi_dsi_attach(dsi);
 	if (err) {
 		struct panel_simple *panel = mipi_dsi_get_drvdata(dsi);
 
+		dev_err(&dsi->dev, "probe attach err: %i", err);
 		drm_panel_remove(&panel->base);
 	}

Signature · 1 line

2.30.2

Krzysztof Kozlowski

2024-04-18 23:06 UTC #2 lore raw

Re: [PATCH 1/2] drm/panel: panel-simple: Add generic panel-dsi driver

Quoted 4 lines · Johan Adolfsson

Add generic panel-dsi panel, similar to panel-dpi that can have it's timing, lanes and flags overridden by devicetree.
Add some dev_err() and dev_warn() calls.

...

drivers/gpu/drm/panel/panel-simple.c static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi) · line 5055

Quoted 8 lines

drivers/gpu/drm/panel/panel-simple.c +2 -0

 		/* sentinel */
 	}

@@ -4992,17 +5051,28 @@ static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi)
 		return -ENODEV;
 
 	err = panel_simple_probe(&dsi->dev, &desc->desc);
+	if (err)
+		dev_err(&dsi->dev, "%s: err %i\n", __func__, err);

This looks like debugging code.

Quoted 4 lines

drivers/gpu/drm/panel/panel-simple.c +4 -0

+
+	if (desc == &panel_dsi) {
+		struct panel_simple *panel = mipi_dsi_get_drvdata(dsi);
+		/* Handle the generic panel-dsi binding */

drivers/gpu/drm/panel/panel-simple.c static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi) · line 5069

Quoted 10 lines

drivers/gpu/drm/panel/panel-simple.c +4 -0

+		err = panel_dsi_probe(&dsi->dev, panel);
+	}
+
 	if (err < 0)
 		return err;
 
 	dsi->mode_flags = desc->flags;
 	dsi->format = desc->format;
 	dsi->lanes = desc->lanes;
+	of_property_read_u32(dsi->dev.of_node, "lanes", &dsi->lanes);

Is this defined in the binding?

drivers/gpu/drm/panel/panel-simple.c static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi) · line 5075

Quoted 6 lines

drivers/gpu/drm/panel/panel-simple.c +1 -0

 	err = mipi_dsi_attach(dsi);
 	if (err) {
 		struct panel_simple *panel = mipi_dsi_get_drvdata(dsi);
 
+		dev_err(&dsi->dev, "probe attach err: %i", err);

Do not introduce unrelated code changes.

Best regards,
Krzysztof

Johan Adolfsson

author

2024-04-22 14:05 UTC #3 lore raw

RE: [PATCH 1/2] drm/panel: panel-simple: Add generic panel-dsi driver

Hi,
Sorry for delayed response, still investigating why these mails didn't reach my inbox as expected..

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: den 19 april 2024 01:06
To: Johan Adolfsson <[email protected]>; Neil Armstrong <[email protected]>; Jessica Zhang <[email protected]>; Sam Ravnborg <[email protected]>; Maarten Lankhorst <[email protected]>; Maxime Ripard <[email protected]>; Thomas Zimmermann <[email protected]>; David Airlie <[email protected]>; Daniel Vetter <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Thierry Reding <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; kernel <[email protected]>
Subject: Re: [PATCH 1/2] drm/panel: panel-simple: Add generic panel-dsi driver

Quoted 4 lines · Johan Adolfsson

Add generic panel-dsi panel, similar to panel-dpi that can have it's timing, lanes and flags overridden by devicetree.
Add some dev_err() and dev_warn() calls.

...

Quoted 10 lines

drivers/gpu/drm/panel/panel-simple.c static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi) · line 5055
Quoted 8 lines
drivers/gpu/drm/panel/panel-simple.c +2 -0
 		/* sentinel */
 	}
@@ -4992,17 +5051,28 @@ static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi)
 		return -ENODEV;
 
 	err = panel_simple_probe(&dsi->dev, &desc->desc);
+	if (err)
+		dev_err(&dsi->dev, "%s: err %i\n", __func__, err);
This looks like debugging code.

I added it since you don't really get any good hints on where things fails if they do it. Debugging code or not depends on the definition I guess - it helps the user track down a faulty devicetree, or as in the case below mismatch with the DSI driver.

...

Quoted 5 lines

drivers/gpu/drm/panel/panel-simple.c static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi) · line 5069
Quoted 3 lines
drivers/gpu/drm/panel/panel-simple.c +1 -0
 	dsi->format = desc->format;
 	dsi->lanes = desc->lanes;
+	of_property_read_u32(dsi->dev.of_node, "lanes", &dsi->lanes);
Is this defined in the binding?

Apparently not which I assumed. Other bindings mentions dsi-lanes, but I guess "num-dsi-lanes" would be more correct.

Quoted 7 lines

drivers/gpu/drm/panel/panel-simple.c static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi) · line 5075
Quoted 5 lines
drivers/gpu/drm/panel/panel-simple.c +1 -0
 	err = mipi_dsi_attach(dsi);
 	if (err) {
 		struct panel_simple *panel = mipi_dsi_get_drvdata(dsi);
 
+		dev_err(&dsi->dev, "probe attach err: %i", err);
Do not introduce unrelated code changes.

As before, it helps the user who has a messed up devicetree find out, since we now gets some more configurability using devicetree. Would it be acceptable as a separate commit, or should I simply skip this?

Quoted 2 lines

Best regards,
Krzysztof

Thanks!

Best regards
/Johan

Dmitry Baryshkov

2024-04-22 19:43 UTC #4 lore raw

Re: [PATCH 1/2] drm/panel: panel-simple: Add generic panel-dsi driver

Quoted 32 lines · Johan Adolfsson

Hi,
Sorry for delayed response, still investigating why these mails didn't reach my inbox as expected..
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: den 19 april 2024 01:06
To: Johan Adolfsson <[email protected]>; Neil Armstrong <[email protected]>; Jessica Zhang <[email protected]>; Sam Ravnborg <[email protected]>; Maarten Lankhorst <[email protected]>; Maxime Ripard <[email protected]>; Thomas Zimmermann <[email protected]>; David Airlie <[email protected]>; Daniel Vetter <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Thierry Reding <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; kernel <[email protected]>
Subject: Re: [PATCH 1/2] drm/panel: panel-simple: Add generic panel-dsi driver
Quoted 4 lines · Johan Adolfsson
Add generic panel-dsi panel, similar to panel-dpi that can have it's timing, lanes and flags overridden by devicetree.
Add some dev_err() and dev_warn() calls.
...
Quoted 10 lines
drivers/gpu/drm/panel/panel-simple.c static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi) · line 5055
Quoted 8 lines
drivers/gpu/drm/panel/panel-simple.c +2 -0
 		/* sentinel */
 	}
@@ -4992,17 +5051,28 @@ static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi)
 		return -ENODEV;
 
 	err = panel_simple_probe(&dsi->dev, &desc->desc);
+	if (err)
+		dev_err(&dsi->dev, "%s: err %i\n", __func__, err);
This looks like debugging code.
I added it since you don't really get any good hints on where things fails if they do it. Debugging code or not depends on the definition I guess - it helps the user track down a faulty devicetree, or as in the case below mismatch with the DSI driver.

Quoted 8 lines

...
Quoted 5 lines
drivers/gpu/drm/panel/panel-simple.c static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi) · line 5069
Quoted 3 lines
drivers/gpu/drm/panel/panel-simple.c +1 -0
 	dsi->format = desc->format;
 	dsi->lanes = desc->lanes;
+	of_property_read_u32(dsi->dev.of_node, "lanes", &dsi->lanes);
Is this defined in the binding?
Apparently not which I assumed. Other bindings mentions dsi-lanes, but I guess "num-dsi-lanes" would be more correct.

Please use drm_of_get_data_lanes_count() and corresponding property from the bindings.

Quoted 22 lines

Quoted 8 lines
drivers/gpu/drm/panel/panel-simple.c static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi) · line 5075
Quoted 6 lines
drivers/gpu/drm/panel/panel-simple.c +1 -0
 	err = mipi_dsi_attach(dsi);
 	if (err) {
 		struct panel_simple *panel = mipi_dsi_get_drvdata(dsi);
 
+		dev_err(&dsi->dev, "probe attach err: %i", err);
Do not introduce unrelated code changes.
As before, it helps the user who has a messed up devicetree find out, since we now gets some more configurability using devicetree. Would it be acceptable as a separate commit, or should I simply skip this?
Quoted 2 lines
Best regards,
Krzysztof
Thanks!
Best regards
/Johan

Signature · 2 lines

With best wishes
Dmitry

Johan Adolfsson

author

2024-04-18 14:01 UTC #5 lore raw

PATCH 2/2 dt-bindings: panel-simple-dsi: Add generic panel-dsi

In reply to Johan Adolfsson

panel-dsi is similar to panel-dpi with overridable timings

Signed-off-by Johan Adolfsson <[email protected]>

---
 Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++
 1 file changed, 2 insertions(+)

Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml +2 -0

index f9160d7bac3c..4c9dd4f66a8f 100644
@@ -56,6 +56,8 @@ properties:
       - samsung,sofef00
         # Shangai Top Display Optoelectronics 7" TL070WSH30 1024x600 TFT LCD panel
       - tdo,tl070wsh30
+        # Generic dsi panel with timing overridable
+      - panel-dsi
 
   reg:
     maxItems: 1

Signature · 1 line

2.30.2

Krzysztof Kozlowski

2024-04-18 23:04 UTC #6 lore raw

Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi

Quoted 1 line · Johan Adolfsson

panel-dsi is similar to panel-dpi with overridable timings

???

Quoted 11 lines

Signed-off-by Johan Adolfsson <[email protected]>
---
 Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml +0 -0
index f9160d7bac3c..4c9dd4f66a8f 100644

Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml properties: · line 60

Quoted 5 lines

Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml +2 -0

@@ -56,6 +56,8 @@ properties:
       - samsung,sofef00
         # Shangai Top Display Optoelectronics 7" TL070WSH30 1024x600 TFT LCD panel
       - tdo,tl070wsh30
+        # Generic dsi panel with timing overridable
+      - panel-dsi

? Devices are not generic. This is very confusing and commit msg does not help me. Compatibles *must* be specific, see writing-bindings.

Best regards,
Krzysztof

Johan Adolfsson

author

2024-04-22 14:34 UTC #7 lore raw

RE: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: den 19 april 2024 01:05
To: Johan Adolfsson <[email protected]>; Neil Armstrong <[email protected]>; Jessica Zhang <[email protected]>; Sam Ravnborg <[email protected]>; Maarten Lankhorst <[email protected]>; Maxime Ripard <[email protected]>; Thomas Zimmermann <[email protected]>; David Airlie <[email protected]>; Daniel Vetter <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Thierry Reding <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; kernel <[email protected]>
Subject: Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi

Quoted 4 lines · Johan Adolfsson

Quoted 1 line
panel-dsi is similar to panel-dpi with overridable timings
???

I guess a more correct description would be
"panel-dsi is a fallback in a similar way that panel-dpi is in panel-dpi.yaml .."?

...

Quoted 4 lines

Quoted 4 lines
a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yam l
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi +++ .yaml

Quoted 8 lines

Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml properties: · line 60
Quoted 6 lines
Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml +2 -0
@@ -56,6 +56,8 @@ properties:
       - samsung,sofef00
         # Shangai Top Display Optoelectronics 7" TL070WSH30 1024x600 TFT LCD panel
       - tdo,tl070wsh30
+        # Generic dsi panel with timing overridable
+      - panel-dsi
? Devices are not generic. This is very confusing and commit msg does not help me. Compatibles *must* be specific, see writing-bindings.

I guess I see this a generic fallback, that handles any dsi panel as long as the correct timing etc is specified in devictree, similar to what panel-dpi is in panel-dpi.yaml Maybe phrasing it similar to panel-dpi.yaml is better?

Quoted 2 lines

Best regards,
Krzysztof

Best regards
/Johan

Krzysztof Kozlowski

2024-04-22 20:56 UTC #8 lore raw

Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi

Quoted 9 lines · Johan Adolfsson

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: den 19 april 2024 01:05
To: Johan Adolfsson <[email protected]>; Neil Armstrong <[email protected]>; Jessica Zhang <[email protected]>; Sam Ravnborg <[email protected]>; Maarten Lankhorst <[email protected]>; Maxime Ripard <[email protected]>; Thomas Zimmermann <[email protected]>; David Airlie <[email protected]>; Daniel Vetter <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Thierry Reding <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; kernel <[email protected]>
Subject: Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi

Quoted 7 lines

Quoted 4 lines · Johan Adolfsson
Quoted 1 line
panel-dsi is similar to panel-dpi with overridable timings
???
I guess a more correct description would be
"panel-dsi is a fallback in a similar way that panel-dpi is in panel-dpi.yaml .."?

I meant how does it explain the hardware.

Quoted 15 lines

...
Quoted 4 lines
Quoted 4 lines
a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yam l
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi +++ .yaml
Quoted 8 lines
Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml properties: · line 60
Quoted 6 lines
Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml +2 -0
@@ -56,6 +56,8 @@ properties:
       - samsung,sofef00
         # Shangai Top Display Optoelectronics 7" TL070WSH30 1024x600 TFT LCD panel
       - tdo,tl070wsh30
+        # Generic dsi panel with timing overridable
+      - panel-dsi
? Devices are not generic. This is very confusing and commit msg does not help me. Compatibles *must* be specific, see writing-bindings.

Quoted 2 lines

I guess I see this a generic fallback, that handles any dsi panel as long as the correct timing etc is specified in devictree, similar to what panel-dpi is in panel-dpi.yaml Maybe phrasing it similar to panel-dpi.yaml is better?

But panel-dpi is quite different. You always have two compatibles. I don't see two compatibles here.

Best regards,
Krzysztof

Johan Adolfsson

author

2024-05-13 09:34 UTC #9 lore raw

RE: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi

Quoted 16 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: den 22 april 2024 22:57
To: Johan Adolfsson <[email protected]>; Neil Armstrong <[email protected]>; Jessica Zhang <[email protected]>; Sam Ravnborg <[email protected]>; Maarten Lankhorst <[email protected]>; Maxime Ripard <[email protected]>; Thomas Zimmermann <[email protected]>; David Airlie <[email protected]>; Daniel Vetter <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Thierry Reding <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; kernel <[email protected]>
Subject: Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi>
Quoted 6 lines · Johan Adolfsson
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: den 19 april 2024 01:05
To: Johan Adolfsson <[email protected]>; Neil Armstrong

Quoted 4 lines

Quoted 4 lines
Subject: Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi
Quoted 1 line · Johan Adolfsson

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
Quoted 1 line
panel-dsi is similar to panel-dpi with overridable timings
???
I guess a more correct description would be "panel-dsi is a fallback in a similar way that panel-dpi is in panel-dpi.yaml .."?
I meant how does it explain the hardware.

In our case it's actually an FPGA that takes MIPI DSI input and can support multiple different kind of displays, and what I want to achieve is to avoid specifying a lot of different panel timings in the driver and have that in devicetree instead.
I appreciate any suggestions on how to phrase that in an acceptable manner.

Quoted 16 lines

Quoted 16 lines
...
Quoted 14 lines
Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml properties: · line 60
Quoted 12 lines
a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.y am
l
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple-d +++ si
+++ .yaml
Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml +2 -0
@@ -56,6 +56,8 @@ properties:
       - samsung,sofef00
         # Shangai Top Display Optoelectronics 7" TL070WSH30 1024x600 TFT LCD panel
       - tdo,tl070wsh30
+        # Generic dsi panel with timing overridable
+      - panel-dsi
? Devices are not generic. This is very confusing and commit msg does not help me. Compatibles *must* be specific, see writing-bindings.

Quoted 4 lines

Quoted 2 lines
I guess I see this a generic fallback, that handles any dsi panel as
long as the correct timing etc is specified in devictree, similar to what panel-dpi is in panel-dpi.yaml Maybe phrasing it similar to panel-dpi.yaml is better?
But panel-dpi is quite different. You always have two compatibles. I don't see two compatibles here.

It doesn't really need two compatibles (in practice at least), the new generic panel-dsi could be enough - I do believe that devices can be generic, but I admit that I'm not really "fluent" in devicetree and binding descriptions.

Quoted 2 lines

Best regards,
Krzysztof

Best regards
/Johan

Krzysztof Kozlowski

2024-05-13 09:38 UTC #10 lore raw

Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi

Quoted 50 lines · Johan Adolfsson

Quoted 16 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: den 22 april 2024 22:57
To: Johan Adolfsson <[email protected]>; Neil Armstrong <[email protected]>; Jessica Zhang <[email protected]>; Sam Ravnborg <[email protected]>; Maarten Lankhorst <[email protected]>; Maxime Ripard <[email protected]>; Thomas Zimmermann <[email protected]>; David Airlie <[email protected]>; Daniel Vetter <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Thierry Reding <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; kernel <[email protected]>
Subject: Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi>
Quoted 6 lines · Johan Adolfsson
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: den 19 april 2024 01:05
To: Johan Adolfsson <[email protected]>; Neil Armstrong
..
Quoted 4 lines
Quoted 4 lines
Subject: Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi
Quoted 1 line · Johan Adolfsson
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
panel-dsi is similar to panel-dpi with overridable timings
???
I guess a more correct description would be "panel-dsi is a fallback in a similar way that panel-dpi is in panel-dpi.yaml .."?
I meant how does it explain the hardware.
In our case it's actually an FPGA that takes MIPI DSI input and can support multiple different kind of displays, and what I want to achieve is to avoid specifying a lot of different panel timings in the driver and have that in devicetree instead.
I appreciate any suggestions on how to phrase that in an acceptable manner.
Quoted 16 lines
Quoted 16 lines
...
Quoted 14 lines
Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml properties: · line 60
Quoted 12 lines
a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.y am
l
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple-d +++ si
+++ .yaml
Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml +2 -0
@@ -56,6 +56,8 @@ properties:
       - samsung,sofef00
         # Shangai Top Display Optoelectronics 7" TL070WSH30 1024x600 TFT LCD panel
       - tdo,tl070wsh30
+        # Generic dsi panel with timing overridable
+      - panel-dsi
? Devices are not generic. This is very confusing and commit msg does not help me. Compatibles *must* be specific, see writing-bindings.

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
I guess I see this a generic fallback, that handles any dsi panel as
long as the correct timing etc is specified in devictree, similar to what panel-dpi is in panel-dpi.yaml Maybe phrasing it similar to panel-dpi.yaml is better?
But panel-dpi is quite different. You always have two compatibles. I don't see two compatibles here.
It doesn't really need two compatibles (in practice at least), the new generic panel-dsi could be enough - I do believe that devices can be generic, but I admit that I'm not really "fluent" in devicetree and binding descriptions.

I guess the first comment brought here a bit of naming issue... The bindings are not "generic".

Best regards,
Krzysztof