[RFC][PATCH 1/3] dt-bindings: display: panel: mipi-dbi-spi: Make width-mm/height-mm mandatory

[RFC][PATCH 1/3] dt-bindings: display: panel: mipi-dbi-spi: Make width-mm/height-mm mandatory

[Marek Vasut]

Make the width-mm/height-mm panel properties mandatory
to correctly report the panel dimensions to the OS.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dmitry Osipenko <digetx@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Robert Foss <robert.foss@linaro.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
To: dri-devel@lists.freedesktop.org
---
 .../devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml   | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
index f29789994b18..c2df8d28aaf5 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
@@ -83,6 +83,8 @@ properties:
 required:
   - compatible
   - reg
+  - width-mm
+  - height-mm
   - panel-timing
 
 unevaluatedProperties: false
-- 
2.35.1

[RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()

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

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dmitry Osipenko <digetx@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Robert Foss <robert.foss@linaro.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
To: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_modes.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 3f819c7a021b..45dc2d5c3ea6 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -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;
-- 
2.35.1

Re: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()

[Laurent Pinchart]

Hi Marek,

Thank you for the patch.

On Fri, Apr 01, 2022 at 06:37:54PM +0200, Marek Vasut wrote:
> 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 ?

> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dmitry Osipenko <digetx@gmail.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Robert Foss <robert.foss@linaro.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> To: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_modes.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 3f819c7a021b..45dc2d5c3ea6 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -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;

-- 
Regards,

Laurent Pinchart

Re: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()

[Marek Vasut]

On 4/1/22 20:46, Laurent Pinchart wrote:

Hi,

> On Fri, Apr 01, 2022 at 06:37:54PM +0200, Marek Vasut wrote:
>> 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.

Re: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()

[Laurent Pinchart]

On Fri, Apr 01, 2022 at 10:36:24PM +0200, Marek Vasut wrote:
> On 4/1/22 20:46, Laurent Pinchart wrote:
> > On Fri, Apr 01, 2022 at 06:37:54PM +0200, Marek Vasut wrote:
> >> 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 ?

-- 
Regards,

Laurent Pinchart

Re: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()

[Marek Vasut]

On 4/2/22 05:19, Laurent Pinchart wrote:
> On Fri, Apr 01, 2022 at 10:36:24PM +0200, Marek Vasut wrote:
>> On 4/1/22 20:46, Laurent Pinchart wrote:
>>> On Fri, Apr 01, 2022 at 06:37:54PM +0200, Marek Vasut wrote:
>>>> 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 ?

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

[Noralf Trønnes]

Den 02.04.2022 06.28, skrev Marek Vasut:
> On 4/2/22 05:19, Laurent Pinchart wrote:
>> On Fri, Apr 01, 2022 at 10:36:24PM +0200, Marek Vasut wrote:
>>> On 4/1/22 20:46, Laurent Pinchart wrote:
>>>> On Fri, Apr 01, 2022 at 06:37:54PM +0200, Marek Vasut wrote:
>>>>> 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 ?
> 
> 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()

[Marek Vasut]

On 4/2/22 09:45, Noralf Trønnes wrote:
> 
> 
> Den 02.04.2022 06.28, skrev Marek Vasut:
>> On 4/2/22 05:19, Laurent Pinchart wrote:
>>> On Fri, Apr 01, 2022 at 10:36:24PM +0200, Marek Vasut wrote:
>>>> On 4/1/22 20:46, Laurent Pinchart wrote:
>>>>> On Fri, Apr 01, 2022 at 06:37:54PM +0200, Marek Vasut wrote:
>>>>>> 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 ?
>>
>> 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?

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

[Noralf Trønnes]

Den 02.04.2022 18.39, skrev Marek Vasut:
> On 4/2/22 09:45, Noralf Trønnes wrote:
>>
>>
>> Den 02.04.2022 06.28, skrev Marek Vasut:
>>> On 4/2/22 05:19, Laurent Pinchart wrote:
>>>> On Fri, Apr 01, 2022 at 10:36:24PM +0200, Marek Vasut wrote:
>>>>> On 4/1/22 20:46, Laurent Pinchart wrote:
>>>>>> On Fri, Apr 01, 2022 at 06:37:54PM +0200, Marek Vasut wrote:
>>>>>>> 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 ?
>>>
>>> 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?
> 
> 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 ?

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

[Marek Vasut]

On 4/2/22 19:08, Noralf Trønnes wrote:
> 
> 
> Den 02.04.2022 18.39, skrev Marek Vasut:
>> On 4/2/22 09:45, Noralf Trønnes wrote:
>>>
>>>
>>> Den 02.04.2022 06.28, skrev Marek Vasut:
>>>> On 4/2/22 05:19, Laurent Pinchart wrote:
>>>>> On Fri, Apr 01, 2022 at 10:36:24PM +0200, Marek Vasut wrote:
>>>>>> On 4/1/22 20:46, Laurent Pinchart wrote:
>>>>>>> On Fri, Apr 01, 2022 at 06:37:54PM +0200, Marek Vasut wrote:
>>>>>>>> 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 ?
>>>>
>>>> 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?
>>
>> 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 ?
> 
> 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()

[Noralf Trønnes]

Den 02.04.2022 19.55, skrev Marek Vasut:
> On 4/2/22 19:08, Noralf Trønnes wrote:
>>
>>
>> Den 02.04.2022 18.39, skrev Marek Vasut:
>>> On 4/2/22 09:45, Noralf Trønnes wrote:
>>>>
>>>>
>>>> Den 02.04.2022 06.28, skrev Marek Vasut:
>>>>> On 4/2/22 05:19, Laurent Pinchart wrote:
>>>>>> On Fri, Apr 01, 2022 at 10:36:24PM +0200, Marek Vasut wrote:
>>>>>>> On 4/1/22 20:46, Laurent Pinchart wrote:
>>>>>>>> On Fri, Apr 01, 2022 at 06:37:54PM +0200, Marek Vasut wrote:
>>>>>>>>> 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 ?
>>>>>
>>>>> 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?
>>>
>>> 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 ?
>>
>> 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 ?

It entered this merge window.

Re: [RFC][PATCH 2/3] drm/modes: Make width-mm/height-mm mandatory in of_get_drm_panel_display_mode()

[Rob Herring]

On Sat, Apr 02, 2022 at 07:55:59PM +0200, Marek Vasut wrote:
> On 4/2/22 19:08, Noralf Trønnes wrote:
> > 
> > 
> > Den 02.04.2022 18.39, skrev Marek Vasut:
> > > On 4/2/22 09:45, Noralf Trønnes wrote:
> > > > 
> > > > 
> > > > Den 02.04.2022 06.28, skrev Marek Vasut:
> > > > > On 4/2/22 05:19, Laurent Pinchart wrote:
> > > > > > On Fri, Apr 01, 2022 at 10:36:24PM +0200, Marek Vasut wrote:
> > > > > > > On 4/1/22 20:46, Laurent Pinchart wrote:
> > > > > > > > On Fri, Apr 01, 2022 at 06:37:54PM +0200, Marek Vasut wrote:
> > > > > > > > > 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 ?
> > > > > 
> > > > > 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?
> > > 
> > > 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 ?
> > 
> > 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 ?

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

[Marek Vasut]

On 4/4/22 18:01, Rob Herring wrote:
> On Sat, Apr 02, 2022 at 07:55:59PM +0200, Marek Vasut wrote:
>> On 4/2/22 19:08, Noralf Trønnes wrote:
>>>
>>>
>>> Den 02.04.2022 18.39, skrev Marek Vasut:
>>>> On 4/2/22 09:45, Noralf Trønnes wrote:
>>>>>
>>>>>
>>>>> Den 02.04.2022 06.28, skrev Marek Vasut:
>>>>>> On 4/2/22 05:19, Laurent Pinchart wrote:
>>>>>>> On Fri, Apr 01, 2022 at 10:36:24PM +0200, Marek Vasut wrote:
>>>>>>>> On 4/1/22 20:46, Laurent Pinchart wrote:
>>>>>>>>> On Fri, Apr 01, 2022 at 06:37:54PM +0200, Marek Vasut wrote:
>>>>>>>>>> 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 ?
>>>>>>
>>>>>> 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?
>>>>
>>>> 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 ?
>>>
>>> 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 ?
> 
> 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

[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 <marex@denx.de>
Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dmitry Osipenko <digetx@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Robert Foss <robert.foss@linaro.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
To: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/panel/panel-lvds.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-lvds.c b/drivers/gpu/drm/panel/panel-lvds.c
index eca067e78579..f11252fb00fe 100644
--- a/drivers/gpu/drm/panel/panel-lvds.c
+++ b/drivers/gpu/drm/panel/panel-lvds.c
@@ -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);
-- 
2.35.1

Re: [RFC][PATCH 3/3] drm/panel: lvds: Drop duplicate width-mm/height-mm check

[Laurent Pinchart]

Hi Marek,

Thank you for the patch.

On Fri, Apr 01, 2022 at 06:37:55PM +0200, Marek Vasut wrote:
> 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 <marex@denx.de>
> Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dmitry Osipenko <digetx@gmail.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Robert Foss <robert.foss@linaro.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> To: dri-devel@lists.freedesktop.org

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

assuming patches 1/3 and 2/3 get accepted.

> ---
>  drivers/gpu/drm/panel/panel-lvds.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-lvds.c b/drivers/gpu/drm/panel/panel-lvds.c
> index eca067e78579..f11252fb00fe 100644
> --- a/drivers/gpu/drm/panel/panel-lvds.c
> +++ b/drivers/gpu/drm/panel/panel-lvds.c
> @@ -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);

-- 
Regards,

Laurent Pinchart

Re: [RFC][PATCH 1/3] dt-bindings: display: panel: mipi-dbi-spi: Make width-mm/height-mm mandatory

[Rob Herring]

On Fri, Apr 1, 2022 at 11:38 AM Marek Vasut <marex@denx.de> wrote:
>
> 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.

>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dmitry Osipenko <digetx@gmail.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Robert Foss <robert.foss@linaro.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> To: dri-devel@lists.freedesktop.org
> ---
>  .../devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml   | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> index f29789994b18..c2df8d28aaf5 100644
> --- a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> @@ -83,6 +83,8 @@ properties:
>  required:
>    - compatible
>    - reg
> +  - width-mm
> +  - height-mm
>    - panel-timing
>
>  unevaluatedProperties: false
> --
> 2.35.1
>

Re: [RFC][PATCH 1/3] dt-bindings: display: panel: mipi-dbi-spi: Make width-mm/height-mm mandatory

[Laurent Pinchart]

On Fri, Apr 01, 2022 at 01:36:53PM -0500, Rob Herring wrote:
> On Fri, Apr 1, 2022 at 11:38 AM Marek Vasut <marex@denx.de> wrote:
> >
> > 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.

> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Dmitry Osipenko <digetx@gmail.com>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Noralf Trønnes <noralf@tronnes.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Robert Foss <robert.foss@linaro.org>
> > Cc: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: devicetree@vger.kernel.org
> > To: dri-devel@lists.freedesktop.org
> > ---
> >  .../devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml   | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> > index f29789994b18..c2df8d28aaf5 100644
> > --- a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> > @@ -83,6 +83,8 @@ properties:
> >  required:
> >    - compatible
> >    - reg
> > +  - width-mm
> > +  - height-mm
> >    - panel-timing
> >
> >  unevaluatedProperties: false

-- 
Regards,

Laurent Pinchart

Re: [RFC][PATCH 1/3] dt-bindings: display: panel: mipi-dbi-spi: Make width-mm/height-mm mandatory

[Marek Vasut]

On 4/1/22 20:36, Rob Herring wrote:
> On Fri, Apr 1, 2022 at 11:38 AM Marek Vasut <marex@denx.de> wrote:
>>
>> 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.