[PATCH 0/2] Add generic panel-dsi to panel-simple

[PATCH 0/2] Add generic panel-dsi to panel-simple

[Johan Adolfsson]

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 <johan.adolfsson@axis.com>
---
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,
-- 
Johan Adolfsson <johan.adolfsson@axis.com>

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

[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.

Signed-off-by: Johan Adolfsson <johan.adolfsson@axis.com>
---
 drivers/gpu/drm/panel/panel-simple.c | 76 ++++++++++++++++++++++++++++++++++--
 1 file changed, 73 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index d493ee735c73..3b812fb9ae9e 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -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);
 	}
 

-- 
2.30.2

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

[Krzysztof Kozlowski]

On 18/04/2024 16:01, Johan Adolfsson wrote:
> 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.
> 

...

>  		/* 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.

> +
> +	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);

Is this defined in the binding?

>  
>  	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

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

[Johan Adolfsson]

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

-----Original Message-----
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 
Sent: den 19 april 2024 01:06
To: Johan Adolfsson <Johan.Adolfsson@axis.com>; Neil Armstrong <neil.armstrong@linaro.org>; Jessica Zhang <quic_jesszhan@quicinc.com>; Sam Ravnborg <sam@ravnborg.org>; Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David Airlie <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Thierry Reding <thierry.reding@gmail.com>
Cc: dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; kernel <kernel@axis.com>
Subject: Re: [PATCH 1/2] drm/panel: panel-simple: Add generic panel-dsi driver

On 18/04/2024 16:01, Johan Adolfsson wrote:
> 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.
> 

...

>>  		/* 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.

...
>>  	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.

>>  	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?


>Best regards,
>Krzysztof

Thanks!

Best regards
/Johan

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

[Dmitry Baryshkov]

On Mon, Apr 22, 2024 at 02:05:01PM +0000, Johan Adolfsson wrote:
> Hi,
> Sorry for delayed response, still investigating why these mails didn't reach my inbox as expected..
> 
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 
> Sent: den 19 april 2024 01:06
> To: Johan Adolfsson <Johan.Adolfsson@axis.com>; Neil Armstrong <neil.armstrong@linaro.org>; Jessica Zhang <quic_jesszhan@quicinc.com>; Sam Ravnborg <sam@ravnborg.org>; Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David Airlie <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Thierry Reding <thierry.reding@gmail.com>
> Cc: dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; kernel <kernel@axis.com>
> Subject: Re: [PATCH 1/2] drm/panel: panel-simple: Add generic panel-dsi driver
> 
> On 18/04/2024 16:01, Johan Adolfsson wrote:
> > 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.
> > 
> 
> ...
> 
> >>  		/* 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.
> 
> ...
> >>  	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.
> 
> >>  	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?
> 
> 
> >Best regards,
> >Krzysztof
> 
> Thanks!
> 
> Best regards
> /Johan
> 
> 

-- 
With best wishes
Dmitry

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

[Johan Adolfsson]

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

Signed-off-by: Johan Adolfsson <johan.adolfsson@axis.com>
---
 Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
index f9160d7bac3c..4c9dd4f66a8f 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
@@ -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

-- 
2.30.2

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

[Krzysztof Kozlowski]

On 18/04/2024 16:01, Johan Adolfsson wrote:
> panel-dsi is similar to panel-dpi with overridable timings

???

> 
> Signed-off-by: Johan Adolfsson <johan.adolfsson@axis.com>
> ---
>  Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
> index f9160d7bac3c..4c9dd4f66a8f 100644
> --- a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
> @@ -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

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

[Johan Adolfsson]

-----Original Message-----
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 
Sent: den 19 april 2024 01:05
To: Johan Adolfsson <Johan.Adolfsson@axis.com>; Neil Armstrong <neil.armstrong@linaro.org>; Jessica Zhang <quic_jesszhan@quicinc.com>; Sam Ravnborg <sam@ravnborg.org>; Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David Airlie <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Thierry Reding <thierry.reding@gmail.com>
Cc: dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; kernel <kernel@axis.com>
Subject: Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi

>On 18/04/2024 16:01, Johan Adolfsson wrote:
>> 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 .."?

...
>> a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yam
>> l
>> +++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi
>> +++ .yaml
>> @@ -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?

>Best regards,
>Krzysztof

Best regards
/Johan

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

[Krzysztof Kozlowski]

On 22/04/2024 16:34, Johan Adolfsson wrote:
> 
> 
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 
> Sent: den 19 april 2024 01:05
> To: Johan Adolfsson <Johan.Adolfsson@axis.com>; Neil Armstrong <neil.armstrong@linaro.org>; Jessica Zhang <quic_jesszhan@quicinc.com>; Sam Ravnborg <sam@ravnborg.org>; Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David Airlie <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Thierry Reding <thierry.reding@gmail.com>
> Cc: dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; kernel <kernel@axis.com>
> Subject: Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi
> 
>> On 18/04/2024 16:01, Johan Adolfsson wrote:
>>> 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.

> 
> ...
>>> a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yam
>>> l
>>> +++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi
>>> +++ .yaml
>>> @@ -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?

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

Best regards,
Krzysztof

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

[Johan Adolfsson]

>-----Original Message-----
>From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 
>Sent: den 22 april 2024 22:57
>To: Johan Adolfsson <Johan.Adolfsson@axis.com>; Neil Armstrong <neil.armstrong@linaro.org>; Jessica Zhang <quic_jesszhan@quicinc.com>; Sam Ravnborg <sam@ravnborg.org>; Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David Airlie <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Thierry Reding <thierry.reding@gmail.com>
>Cc: dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; kernel <kernel@axis.com>
>Subject: Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi>
>
>On 22/04/2024 16:34, Johan Adolfsson wrote:
>> 
>> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: den 19 april 2024 01:05
>> To: Johan Adolfsson <Johan.Adolfsson@axis.com>; Neil Armstrong 
..
>> Subject: Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic 
>> panel-dsi
>> 
>>> On 18/04/2024 16:01, Johan Adolfsson wrote:
>>>> 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.

>> ...
>>>> a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.y
>>>> am
>>>> l
>>>> +++ b/Documentation/devicetree/bindings/display/panel/panel-simple-d
>>>> +++ si
>>>> +++ .yaml
>>>> @@ -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?
>
>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.

>Best regards,
>Krzysztof

Best regards
/Johan

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

[Krzysztof Kozlowski]

On 13/05/2024 11:34, Johan Adolfsson wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 
>> Sent: den 22 april 2024 22:57
>> To: Johan Adolfsson <Johan.Adolfsson@axis.com>; Neil Armstrong <neil.armstrong@linaro.org>; Jessica Zhang <quic_jesszhan@quicinc.com>; Sam Ravnborg <sam@ravnborg.org>; Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David Airlie <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Thierry Reding <thierry.reding@gmail.com>
>> Cc: dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; kernel <kernel@axis.com>
>> Subject: Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic panel-dsi>
>>
>> On 22/04/2024 16:34, Johan Adolfsson wrote:
>>>
>>>
>>> -----Original Message-----
>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> Sent: den 19 april 2024 01:05
>>> To: Johan Adolfsson <Johan.Adolfsson@axis.com>; Neil Armstrong 
> ..
>>> Subject: Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: Add generic 
>>> panel-dsi
>>>
>>>> On 18/04/2024 16:01, Johan Adolfsson wrote:
>>>>> 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.
> 
>>> ...
>>>>> a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.y
>>>>> am
>>>>> l
>>>>> +++ b/Documentation/devicetree/bindings/display/panel/panel-simple-d
>>>>> +++ si
>>>>> +++ .yaml
>>>>> @@ -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?
>>
>> 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