[PATCH] dt-bindings: display: bridge: ti,sn65dsi83: add burst-mode-disabled

[PATCH] dt-bindings: display: bridge: ti,sn65dsi83: add burst-mode-disabled

[stefano.radaelli21]

From: Stefano Radaelli <stefano.radaelli21@gmail.com>

It allows to disable Burst video mode

Co-developed-by: Noah J. Rosa <noahj.rosa@gmail.com>
Signed-off-by: Noah J. Rosa <noahj.rosa@gmail.com>
Signed-off-by: Stefano Radaelli <stefano.radaelli21@gmail.com>
---
 .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml       | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
index 48a97bb3e2e0..eb9c8b6b6813 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
@@ -35,6 +35,9 @@ properties:
   vcc-supply:
     description: A 1.8V power supply (see regulator/regulator.yaml).
 
+  burst-mode-disabled:
+    description: Set Video Mode in Non-Burst Mode
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
-- 
2.34.1

Re: [PATCH] dt-bindings: display: bridge: ti,sn65dsi83: add burst-mode-disabled

[Marek Vasut]

On 7/8/24 5:18 PM, stefano.radaelli21@gmail.com wrote:
> From: Stefano Radaelli <stefano.radaelli21@gmail.com>
> 
> It allows to disable Burst video mode

Why would you want to disable burst mode ? This is the energy efficient 
and recommended mode, why disable it ? Details please ?

Re: [PATCH] dt-bindings: display: bridge: ti,sn65dsi83: add burst-mode-disabled

[Krzysztof Kozlowski]

On 08/07/2024 17:18, stefano.radaelli21@gmail.com wrote:
> From: Stefano Radaelli <stefano.radaelli21@gmail.com>
> 
> It allows to disable Burst video mode
> 
> Co-developed-by: Noah J. Rosa <noahj.rosa@gmail.com>
> Signed-off-by: Noah J. Rosa <noahj.rosa@gmail.com>
> Signed-off-by: Stefano Radaelli <stefano.radaelli21@gmail.com>
> ---
>  .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml       | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> index 48a97bb3e2e0..eb9c8b6b6813 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> @@ -35,6 +35,9 @@ properties:
>    vcc-supply:
>      description: A 1.8V power supply (see regulator/regulator.yaml).
>  
> +  burst-mode-disabled:

You described the desired Linux feature or behavior, not the actual
hardware. The bindings are about the latter, so instead you need to
rephrase the property and its description to match actual hardware
capabilities/features/configuration etc.

Also: missing type, vendor prefix and never tested.

It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: display: bridge: ti,sn65dsi83: add burst-mode-disabled

[Rob Herring (Arm)]

On Mon, 08 Jul 2024 17:18:56 +0200, stefano.radaelli21@gmail.com wrote:
> From: Stefano Radaelli <stefano.radaelli21@gmail.com>
> 
> It allows to disable Burst video mode
> 
> Co-developed-by: Noah J. Rosa <noahj.rosa@gmail.com>
> Signed-off-by: Noah J. Rosa <noahj.rosa@gmail.com>
> Signed-off-by: Stefano Radaelli <stefano.radaelli21@gmail.com>
> ---
>  .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml       | 3 +++
>  1 file changed, 3 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml: burst-mode-disabled: missing type definition

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240708151857.40538-1-stefano.radaelli21@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH] dt-bindings: display: bridge: ti,sn65dsi83: add burst-mode-disabled

[Marek Vasut]

On 7/9/24 2:45 PM, Stefano Radaelli wrote:
> Hello everyone,

Hi,

> Thank you a lot for your prompt feedbacks.
> I'm really sorry for all the mistakes, it is the first time that I try to
> submit a patch and i thought I followed the guideline but clearly that was
> not the case.
> 
>   @Marek Vasut <marex@denx.de> About your question to why disabling
> burst-mode:
> - I agree with you that Burst Mode is the preferred way to send data. For
> that reason I created the new flag in a way that, if not used in dts, burst
> mode remains active by default.
>    However, I decide to introduced this property because I have noticed that
> some dual-channel panels work better in non-burst mode (even if less
> efficient), and since the sn65dsi84 datasheet allows this setting, I
> thought to give this opportunity to users.
>    What do you think about it?

Are there any further details, which panels behave this way ? Does your 
DSI host generate correct HS clock, ones which the DSI84 expects to 
receive on the DSI side ?

Such link mode properties would have to be generic properties placed in 
some dsi-client.yaml file in any case, such properties are not specific 
to this DSI8x bridge.

Re: [PATCH] dt-bindings: display: bridge: ti,sn65dsi83: add burst-mode-disabled

[Marek Vasut]

On 7/9/24 4:44 PM, Stefano Radaelli wrote:
> Hi Marek,

Hi,

> Actually this property is specific also to DSI8x bridge, as you can see
> from the screenshot below taken from official datasheet:
> 
> [image: image.png]
> 
> And it's the sn65dsi8x driver that tells MIPI driver which flags to use
> during attachment.

There are other bridges and panels which support both DSI burst and 
sync-pulse/sync-events modes, so a property which selects the mode is 
generic, not specific to this particular bridge . The bridge driver 
could parse such generic property, although it would be better if the 
core code parsed it instead.

> So, for example, this bridge can work also for MIPI interfaces which don't
> support burst-mode.
> Also, as a value-added benefit, I found non-burst mode better for some
> 1920x1200 LVDS panels I'm testing (Of course with more energy consumption).
> That's why I though it could be useful have this option, since SN65DSI8x
> supports both modes.

Can you share which panel model this is ?

Re: [PATCH] dt-bindings: display: bridge: ti,sn65dsi83: add burst-mode-disabled

[Marek Vasut]

On 7/9/24 7:30 PM, Stefano Radaelli wrote:
> Okay, I get it.
> 
> So if you think this mode shouldn't be implemented within this driver, we
> can close the thread.
> Just for information, this driver has been implemented from the work done
> by Compulab (as it says in the driver's initial comments), and they do not
> put the burst mode by default, not even giving the possibility to activate
> it by dts:
> https://github.com/compulab-yokneam/imx8-android/blob/master/o8/vendor/nxp-opensource/kernel_imx/0055-sn65dsi83-Add-ti-sn65dsi83-dsi-to-lvds-bridge-driver.patch

This is not the mainline Linux driver.

> The panels that I've had these problems with are some of JuTouch's
> 1920x1200, for example JT101TM015 , and I solved it by giving the option to
> remove this mode.
> I have also heard from other colleagues who have had the same problem on
> some dual-channel displays.

Does that problem happen with the aforementioned driver or the mainline 
Linux driver ?