dt-bindings: display: panel: mipi-dbi-spi: Make width-mm/height-mm mandatory
Subsystems (6)
RFC PATCH 1/3 dt-bindings: display: panel: mipi-dbi-spi: Make width-mm/height-mm mandatory
Make the width-mm/height-mm panel properties mandatory to correctly report the panel dimensions to the OS.
--- .../devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml | 2 ++ 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml +2 -0
@@ -83,6 +83,8 @@ properties: required: - compatible - reg + - width-mm + - height-mm - panel-timing unevaluatedProperties: false
Signature · 1 line
2.35.1
RFC PATCH 2/3 drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()
Make the width-mm/height-mm panel properties mandatory in of_get_drm_panel_display_mode(), print error message and return -ve in case these DT properties are not present. This is needed to correctly report panel dimensions.
--- drivers/gpu/drm/drm_modes.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/gpu/drm/drm_modes.c +6 -2
@@ -761,12 +761,16 @@ int of_get_drm_panel_display_mode(struct device_node *np,drm_bus_flags_from_videomode (&vm, bus_flags); ret =of_property_read_u32 (np,"width-mm" , &width_mm); -if (ret && ret != -EINVAL ) +if (ret <0 ) { +pr_err ("%pOF: invalid or missing width-mm DT property\n" , np);return ret; + } ret =of_property_read_u32 (np,"height-mm" , &height_mm); -if (ret && ret != -EINVAL ) +if (ret <0 ) { +pr_err ("%pOF: invalid or missing height-mm DT property\n" , np);return ret; + } dmode->width_mm = width_mm; dmode->height_mm = height_mm;
Signature · 1 line
2.35.1
Laurent Pinchart
maintainerRe: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()
Hi Marek,
Thank you for the patch.
Quoted 4 lines · Marek Vasut
Make the width-mm/height-mm panel properties mandatory in
of_get_drm_panel_display_mode(), print error message and return -ve in case these DT properties are not present. This is needed to correctly report panel dimensions.
Can we guarantee this won't cause a regression ?
Quoted 39 lines
Signed-off-by Marek Vasut <[email protected]>Cc Christoph Niedermaier <[email protected]>Cc Daniel Vetter <[email protected]>Cc Dmitry Osipenko <[email protected]>Cc Laurent Pinchart <[email protected]>Cc Noralf Trønnes <[email protected]>Cc Rob Herring <[email protected]>Cc Robert Foss <[email protected]>Cc Sam Ravnborg <[email protected]>Cc Thomas Zimmermann <[email protected]>--- drivers/gpu/drm/drm_modes.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)drivers/gpu/drm/drm_modes.c +6
-2@@ -761,12 +761,16 @@ int of_get_drm_panel_display_mode(struct device_node *np,drm_bus_flags_from_videomode (&vm, bus_flags); ret =of_property_read_u32 (np,"width-mm" , &width_mm); -if (ret && ret != -EINVAL ) +if (ret <0 ) { +pr_err ("%pOF: invalid or missing width-mm DT property\n" , np);return ret; + } ret =of_property_read_u32 (np,"height-mm" , &height_mm); -if (ret && ret != -EINVAL ) +if (ret <0 ) { +pr_err ("%pOF: invalid or missing height-mm DT property\n" , np);return ret; + } dmode->width_mm = width_mm; dmode->height_mm = height_mm;
Signature · 3 lines
Regards, Laurent Pinchart
Re: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()
On 4/1/22 20:46, Laurent Pinchart wrote:
Hi,
Quoted 7 lines · Marek Vasut
Quoted 4 lines
Make the width-mm/height-mm panel properties mandatory in
of_get_drm_panel_display_mode(), print error message and return -ve in case these DT properties are not present. This is needed to correctly report panel dimensions.Can we guarantee this won't cause a regression ?
For the upstream DTs, I think we can.
For downstream DTs, we cannot know.
Laurent Pinchart
maintainerRe: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()
Quoted 11 lines · Marek Vasut
Quoted 7 lines · Laurent Pinchart
Quoted 4 lines · Marek Vasut
Make the width-mm/height-mm panel properties mandatory in
of_get_drm_panel_display_mode(), print error message and return -ve in case these DT properties are not present. This is needed to correctly report panel dimensions.Can we guarantee this won't cause a regression ?
For the upstream DTs, I think we can.
For downstream DTs, we cannot know.
Are there users of this function whose DT bindings don't require the width-mm and height-mm properties ?
Signature · 3 lines
Regards, Laurent Pinchart
Re: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()
Quoted 8 lines · Laurent Pinchart
Quoted 7 lines · Marek Vasut
Quoted 6 lines · Laurent Pinchart
Quoted 4 lines · Marek Vasut
Make the width-mm/height-mm panel properties mandatory in
of_get_drm_panel_display_mode(), print error message and return -ve in case these DT properties are not present. This is needed to correctly report panel dimensions.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Can we guarantee this won't cause a regression ?
For the upstream DTs, I think we can.
For downstream DTs, we cannot know.Are there users of this function whose DT bindings don't require the width-mm and height-mm properties ?
There is literally one user of this function upstream:drivers/gpu/drm/tiny/panel-mipi-dbi.c
Re: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()
Den 02.04.2022 06.28, skrev Marek Vasut:
Quoted 11 lines · Laurent Pinchart
Quoted 10 lines · Marek Vasut
Quoted 9 lines · Laurent Pinchart
Quoted 7 lines · Marek Vasut
Quoted 4 lines
Make the width-mm/height-mm panel properties mandatory in
of_get_drm_panel_display_mode(), print error message and return -ve in case these DT properties are not present. This is needed to correctly report panel dimensions.Can we guarantee this won't cause a regression ?
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
For the upstream DTs, I think we can.
For downstream DTs, we cannot know.Are there users of this function whose DT bindings don't require the width-mm and height-mm properties ?
There is literally one user of this function upstream:
drivers/gpu/drm/tiny/panel-mipi-dbi.c
Yes, the function was added for that driver since it was so generic in nature. What about adding an argument to of_get_drm_panel_display_mode() that tells if the properties are mandatory or not?
Noralf.
Re: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()
Quoted 20 lines · Noralf Trønnes
Den 02.04.2022 06.28, skrev Marek Vasut:
Quoted 17 lines · Laurent Pinchart
Quoted 15 lines · Marek Vasut
Quoted 11 lines · Laurent Pinchart
Quoted 7 lines · Marek Vasut
Quoted 4 lines
Make the width-mm/height-mm panel properties mandatory in
of_get_drm_panel_display_mode(), print error message and return -ve in case these DT properties are not present. This is needed to correctly report panel dimensions.Can we guarantee this won't cause a regression ?
For the upstream DTs, I think we can.
For downstream DTs, we cannot know.Are there users of this function whose DT bindings don't require the width-mm and height-mm properties ?
Quoted 6 lines
Quoted 2 lines
There is literally one user of this function upstream:
drivers/gpu/drm/tiny/panel-mipi-dbi.cYes, the function was added for that driver since it was so generic in nature. What about adding an argument to
of_get_drm_panel_display_mode()that tells if the properties are mandatory or not?
Sure, we can do that, but maybe the question here is even bigger than this series.
Should every panel set mandatory width_mm/height_mm so e.g. the user space can infer DPI from it and set up scaling accordingly, or should width_mm/height_mm be optional ?
I think width_mm/height_mm should be mandatory for all panels.
Thoughts ?
Re: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()
Den 02.04.2022 18.39, skrev Marek Vasut:
Quoted 31 lines · Noralf Trønnes
Quoted 20 lines
Den 02.04.2022 06.28, skrev Marek Vasut:
Quoted 17 lines · Laurent Pinchart
Quoted 15 lines · Marek Vasut
Quoted 11 lines · Laurent Pinchart
Quoted 7 lines · Marek Vasut
Quoted 4 lines
Make the width-mm/height-mm panel properties mandatory in
of_get_drm_panel_display_mode(), print error message and return -ve in case these DT properties are not present. This is needed to correctly report panel dimensions.Can we guarantee this won't cause a regression ?
For the upstream DTs, I think we can.
For downstream DTs, we cannot know.Are there users of this function whose DT bindings don't require the width-mm and height-mm properties ?
Quoted 6 lines
Quoted 2 lines
There is literally one user of this function upstream:
drivers/gpu/drm/tiny/panel-mipi-dbi.cYes, the function was added for that driver since it was so generic in nature. What about adding an argument to
of_get_drm_panel_display_mode()that tells if the properties are mandatory or not?Sure, we can do that, but maybe the question here is even bigger than this series.
Quoted 7 lines
Should every panel set mandatory width_mm/height_mm so e.g. the user space can infer DPI from it and set up scaling accordingly, or should width_mm/height_mm be optional ?
I think width_mm/height_mm should be mandatory for all panels.
Thoughts ?
If this had come up during the review of the driver I would have no problem making it mandatory. It makes sense for DPI. Maybe it's possible to get around the ABI break by getting in a change through -fixes before 5.18 is released? I'm fine with that.
Noralf.
Re: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()
Quoted 38 lines · Noralf Trønnes
Den 02.04.2022 18.39, skrev Marek Vasut:
Quoted 35 lines · Noralf Trønnes
Quoted 20 lines
Den 02.04.2022 06.28, skrev Marek Vasut:
Quoted 17 lines · Laurent Pinchart
Quoted 15 lines · Marek Vasut
Quoted 11 lines · Laurent Pinchart
Quoted 7 lines · Marek Vasut
Quoted 4 lines
Make the width-mm/height-mm panel properties mandatory in
of_get_drm_panel_display_mode(), print error message and return -ve in case these DT properties are not present. This is needed to correctly report panel dimensions.Can we guarantee this won't cause a regression ?
For the upstream DTs, I think we can.
For downstream DTs, we cannot know.Are there users of this function whose DT bindings don't require the width-mm and height-mm properties ?
Quoted 6 lines
Quoted 2 lines
There is literally one user of this function upstream:
drivers/gpu/drm/tiny/panel-mipi-dbi.cYes, the function was added for that driver since it was so generic in nature. What about adding an argument to
of_get_drm_panel_display_mode()that tells if the properties are mandatory or not?Sure, we can do that, but maybe the question here is even bigger than this series.
Should every panel set mandatory width_mm/height_mm so e.g. the user space can infer DPI from it and set up scaling accordingly, or should width_mm/height_mm be optional ?
Quoted 8 lines
Quoted 3 lines
I think width_mm/height_mm should be mandatory for all panels.
Thoughts ?
If this had come up during the review of the driver I would have no problem making it mandatory. It makes sense for DPI. Maybe it's possible to get around the ABI break by getting in a change through -fixes before 5.18 is released? I'm fine with that.
Well that's awesome, the dbi-spi.yaml didn't land in any kernel release yet, so we still have a chance to fix it ? Rob ?
Re: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()
Den 02.04.2022 19.55, skrev Marek Vasut:
Quoted 44 lines · Noralf Trønnes
Quoted 43 lines
Den 02.04.2022 18.39, skrev Marek Vasut:
Quoted 39 lines · Noralf Trønnes
Quoted 20 lines
Den 02.04.2022 06.28, skrev Marek Vasut:
Quoted 17 lines · Laurent Pinchart
Quoted 15 lines · Marek Vasut
Quoted 11 lines · Laurent Pinchart
Quoted 7 lines · Marek Vasut
Quoted 4 lines
Make the width-mm/height-mm panel properties mandatory in
of_get_drm_panel_display_mode(), print error message and return -ve in case these DT properties are not present. This is needed to correctly report panel dimensions.Can we guarantee this won't cause a regression ?
For the upstream DTs, I think we can.
For downstream DTs, we cannot know.Are there users of this function whose DT bindings don't require the width-mm and height-mm properties ?
Quoted 7 lines
Quoted 2 lines
There is literally one user of this function upstream:
drivers/gpu/drm/tiny/panel-mipi-dbi.cYes, the function was added for that driver since it was so generic in nature. What about adding an argument to
of_get_drm_panel_display_mode()
that tells if the properties are mandatory or not?Sure, we can do that, but maybe the question here is even bigger than this series.
Should every panel set mandatory width_mm/height_mm so e.g. the user space can infer DPI from it and set up scaling accordingly, or should width_mm/height_mm be optional ?
I think width_mm/height_mm should be mandatory for all panels.
Thoughts ?
Quoted 7 lines
Quoted 4 lines
If this had come up during the review of the driver I would have no problem making it mandatory. It makes sense for DPI. Maybe it's possible to get around the ABI break by getting in a change through -fixes before 5.18 is released? I'm fine with that.
Well that's awesome, the
dbi-spi.yamldidn't land in any kernel release yet, so we still have a chance to fix it ?
It entered this merge window.
Rob Herring
maintainerRe: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()
In reply to Marek VasutQuoted 43 lines · Marek Vasut
Quoted 42 lines · Noralf Trønnes
Den 02.04.2022 18.39, skrev Marek Vasut:
Quoted 38 lines · Noralf Trønnes
Quoted 20 lines
Den 02.04.2022 06.28, skrev Marek Vasut:
Quoted 17 lines · Laurent Pinchart
Quoted 15 lines · Marek Vasut
Quoted 11 lines · Laurent Pinchart
Quoted 7 lines · Marek Vasut
Quoted 4 lines
Make the width-mm/height-mm panel properties mandatory in
of_get_drm_panel_display_mode(), print error message and return -ve in case these DT properties are not present. This is needed to correctly report panel dimensions.Can we guarantee this won't cause a regression ?
For the upstream DTs, I think we can.
For downstream DTs, we cannot know.Are there users of this function whose DT bindings don't require the width-mm and height-mm properties ?
Quoted 6 lines
Quoted 2 lines
There is literally one user of this function upstream:
drivers/gpu/drm/tiny/panel-mipi-dbi.cYes, the function was added for that driver since it was so generic in nature. What about adding an argument to
of_get_drm_panel_display_mode()that tells if the properties are mandatory or not?Sure, we can do that, but maybe the question here is even bigger than this series.
Should every panel set mandatory width_mm/height_mm so e.g. the user space can infer DPI from it and set up scaling accordingly, or should width_mm/height_mm be optional ?
I think width_mm/height_mm should be mandatory for all panels.
Thoughts ?
Quoted 7 lines
Quoted 4 lines
If this had come up during the review of the driver I would have no problem making it mandatory. It makes sense for DPI. Maybe it's possible to get around the ABI break by getting in a change through -fixes before 5.18 is released? I'm fine with that.
Well that's awesome, the
dbi-spi.yamldidn't land in any kernel release yet, so we still have a chance to fix it ? Rob ?
Yes, it can be fixed. And the binding, not the kernel, is the place to enforce it being mandatory IMO.
Rob
Re: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()
Quoted 49 lines · Rob Herring
Quoted 48 lines · Marek Vasut
Quoted 46 lines · Noralf Trønnes
Den 02.04.2022 18.39, skrev Marek Vasut:
Quoted 31 lines · Noralf Trønnes
Quoted 20 lines
Den 02.04.2022 06.28, skrev Marek Vasut:
Quoted 17 lines · Laurent Pinchart
Quoted 15 lines · Marek Vasut
Quoted 11 lines · Laurent Pinchart
Quoted 7 lines · Marek Vasut
Quoted 4 lines
Make the width-mm/height-mm panel properties mandatory in
of_get_drm_panel_display_mode(), print error message and return -ve in case these DT properties are not present. This is needed to correctly report panel dimensions.Can we guarantee this won't cause a regression ?
For the upstream DTs, I think we can.
For downstream DTs, we cannot know.Are there users of this function whose DT bindings don't require the width-mm and height-mm properties ?
Quoted 6 lines
Quoted 2 lines
There is literally one user of this function upstream:
drivers/gpu/drm/tiny/panel-mipi-dbi.cYes, the function was added for that driver since it was so generic in nature. What about adding an argument to
of_get_drm_panel_display_mode()that tells if the properties are mandatory or not?Sure, we can do that, but maybe the question here is even bigger than this series.
Quoted 7 lines
Should every panel set mandatory width_mm/height_mm so e.g. the user space can infer DPI from it and set up scaling accordingly, or should width_mm/height_mm be optional ?
I think width_mm/height_mm should be mandatory for all panels.
Thoughts ?
If this had come up during the review of the driver I would have no problem making it mandatory. It makes sense for DPI. Maybe it's possible to get around the ABI break by getting in a change through -fixes before 5.18 is released? I'm fine with that.
Quoted 5 lines
Quoted 2 lines
Well that's awesome, the
dbi-spi.yamldidn't land in any kernel release yet, so we still have a chance to fix it ? Rob ?Yes, it can be fixed. And the binding, not the kernel, is the place to enforce it being mandatory IMO.
All right, I sent 1/3 separately with Fixes: tag, so it can be applied .
RFC PATCH 3/3 drm/panel: lvds: Drop duplicate width-mm/height-mm check
In reply to Marek VasutThe of_get_drm_panel_display_mode() now does check for presence of width-mm/height-mm DT properties, drop the duplicate check here.
--- drivers/gpu/drm/panel/panel-lvds.c | 12 ------------ 1 file changed, 12 deletions(-)
drivers/gpu/drm/panel/panel-lvds.c +0 -12
@@ -128,18 +128,6 @@ static int panel_lvds_parse_dt(struct panel_lvds *lvds)return ret; } -if (lvds->dmode.width_mm ==0 ) { -dev_err (lvds->dev,"%pOF: invalid or missing %s DT property\n" , - np,"width-mm" ); -return -ENODEV ; - } - -if (lvds->dmode.height_mm ==0 ) { -dev_err (lvds->dev,"%pOF: invalid or missing %s DT property\n" , - np,"height-mm" ); -return -ENODEV ; - } -of_property_read_string (np,"label" , &lvds->label); ret =drm_of_lvds_get_data_mapping (np);
Signature · 1 line
2.35.1
Re: [RFC][PATCH 3/3] drm/panel: lvds: Drop duplicate width-mm/height-mm check
Hi Marek,
Thank you for the patch.
Quoted 15 lines · Marek Vasut
The
of_get_drm_panel_display_mode()now does check for presence of width-mm/height-mm DT properties, drop the duplicate check here.Signed-off-by Marek Vasut <[email protected]>Cc Christoph Niedermaier <[email protected]>Cc Daniel Vetter <[email protected]>Cc Dmitry Osipenko <[email protected]>Cc Laurent Pinchart <[email protected]>Cc Noralf Trønnes <[email protected]>Cc Rob Herring <[email protected]>Cc Robert Foss <[email protected]>Cc Sam Ravnborg <[email protected]>Cc Thomas Zimmermann <[email protected]>
Quoted 1 line
assuming patches 1/3 and 2/3 get accepted.
Quoted 27 lines
--- drivers/gpu/drm/panel/panel-lvds.c | 12 ------------ 1 file changed, 12 deletions(-)drivers/gpu/drm/panel/panel-lvds.c +0
-12@@ -128,18 +128,6 @@ static int panel_lvds_parse_dt(struct panel_lvds *lvds)return ret; } -if (lvds->dmode.width_mm ==0 ) { -dev_err (lvds->dev,"%pOF: invalid or missing %s DT property\n" , - np,"width-mm" ); -return -ENODEV ; - } - -if (lvds->dmode.height_mm ==0 ) { -dev_err (lvds->dev,"%pOF: invalid or missing %s DT property\n" , - np,"height-mm" ); -return -ENODEV ; - } -of_property_read_string (np,"label" , &lvds->label); ret =drm_of_lvds_get_data_mapping (np);
Signature · 3 lines
Regards, Laurent Pinchart
Quoted 3 lines · Marek Vasut
Make the width-mm/height-mm panel properties mandatory to correctly report the panel dimensions to the OS.
Adding required properties is an ABI break.
Quoted 33 lines
Signed-off-by Marek Vasut <[email protected]>Cc Christoph Niedermaier <[email protected]>Cc Daniel Vetter <[email protected]>Cc Dmitry Osipenko <[email protected]>Cc Laurent Pinchart <[email protected]>Cc Noralf Trønnes <[email protected]>Cc Rob Herring <[email protected]>Cc Robert Foss <[email protected]>Cc Sam Ravnborg <[email protected]>Cc Thomas Zimmermann <[email protected]>--- .../devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml | 2 ++ 1 file changed, 2 insertions(+)Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml +2
-1@@ -83,6 +83,8 @@ properties: required: - compatible - reg + - width-mm + - height-mm - panel-timing unevaluatedProperties: false --2.35.1
Laurent Pinchart
maintainerQuoted 6 lines · Rob Herring
Quoted 3 lines · Marek Vasut
Make the width-mm/height-mm panel properties mandatory to correctly report the panel dimensions to the OS.
Adding required properties is an ABI break.
Only if drivers don't provide backward compatibility.
There's still the issue of validation warnings of course. We can ensure that DT sources in the kernel are updated. For out-of-tree DT, I'll argue that a validation warning is a good thing, it will notify that an update is needed.
Quoted 29 lines
Quoted 29 lines
Signed-off-by Marek Vasut <[email protected]>Cc Christoph Niedermaier <[email protected]>Cc Daniel Vetter <[email protected]>Cc Dmitry Osipenko <[email protected]>Cc Laurent Pinchart <[email protected]>Cc Noralf Trønnes <[email protected]>Cc Rob Herring <[email protected]>Cc Robert Foss <[email protected]>Cc Sam Ravnborg <[email protected]>Cc Thomas Zimmermann <[email protected]>--- .../devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml | 2 ++ 1 file changed, 2 insertions(+)Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml +2
-0@@ -83,6 +83,8 @@ properties: required: - compatible - reg + - width-mm + - height-mm - panel-timing unevaluatedProperties: false
Signature · 3 lines
Regards, Laurent Pinchart
Quoted 6 lines · Rob Herring
Quoted 3 lines · Marek Vasut
Make the width-mm/height-mm panel properties mandatory to correctly report the panel dimensions to the OS.
Adding required properties is an ABI break.
Per discussion with Noralf in 2/3, these bindings entered in 5.18 merge window, so maybe there is still chance to fix them before rc1/rc2 ? I.e. they were not shipped in any kernel release yet.