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,
--
Johan Adolfsson <[email protected]>
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(+)
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
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(-)
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
On 18/04/2024 16:01, Johan Adolfsson wrote:
> 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(+)
>
> 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
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
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
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
-----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
>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
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 <[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
>
> 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
On 22/04/2024 16:34, Johan Adolfsson wrote:
>
>
> -----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
>
>> 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