| Message ID | 20240708151857.40538-1-stefano.radaelli21@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: display: bridge: ti, sn65dsi83: add burst-mode-disabled | expand |
Hello everyone, 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? After reading better the documentation, I understood that I have to send a Patch Series that includes binding documentation, the implementation and a cover letter. Is that correct? Should I start a new thread or should I continue this one? Thank you very much for your support and your patience, Best regards, Stefano Il giorno lun 8 lug 2024 alle ore 18:47 Rob Herring (Arm) <robh@kernel.org> ha scritto: > > 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. > >
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.
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 ?
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 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. Thank you, Stefano Il Mar 9 Lug 2024, 17:00 Marek Vasut <marex@denx.de> ha scritto: > 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 ? >
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 ?
Yes this is not the mainline driver but it is the one from which the mainline driver is taken. Yes this problem occurs with this mainline driver. Stefano Il Mar 9 Lug 2024, 23:12 Marek Vasut <marex@denx.de> ha scritto: > 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 ? >
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