RE: [v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

2022-01-12 Thread Rajeev Nandan
Hi Dmitry, Rob,

Thanks for the review.
> 
> On Mon, Jan 10, 2022 at 05:06:03PM +0300, Dmitry Baryshkov wrote:
> > On Mon, 10 Jan 2022 at 15:56, Rajeev Nandan
>  wrote:
> > >
> > > In most cases, the default values of DSI PHY tuning registers should
> > > be sufficient as they are fully optimized. However, in some cases
> > > where extreme board parasitics cause the eye shape to degrade, the
> > > override bits can be used to improve the signal quality.
> > >
> > > The general guidelines for DSI PHY tuning include:
> > > - High and moderate data rates may benefit from the drive strength and
> > >   drive level tuning.
> > > - Drive strength tuning will affect the output impedance and may be used
> > >   for matching optimization.
> > > - Drive level tuning will affect the output levels without affecting the
> > >   impedance.
> > >
> > > The clock and data lanes have a calibration circuitry feature. The
> > > drive strength tuning can be done by adjusting rescode offset for
> > > hstop/hsbot, and the drive level tuning can be done by adjusting the
> > > LDO output level for the HSTX drive.
> > >
> > > Signed-off-by: Rajeev Nandan 
> > > ---
> > >
> > > Changes in v2:
> > >  - More details in the commit text (Stephen Boyd)
> > >  - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
> > >  - Do not take values that are going to be unused (Dmitry Baryshkov)
> > >
> > >  .../bindings/display/msm/dsi-phy-10nm.yaml | 33
> ++
> > >  1 file changed, 33 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > > b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > > index 4399715..d0eb8f6 100644
> > > ---
> > > a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > > +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-
> 10nm.yam
> > > +++ l
> > > @@ -35,6 +35,35 @@ properties:
> > >Connected to DSI0_MIPI_DSI_PLL_VDDA0P9 pin for sc7180 target and
> > >connected to VDDA_MIPI_DSI_0_PLL_0P9 pin for sdm845 target
> >
> > Generic note:
> > I think these properties should be prefixed with "qcom," prefix.
Sure, I will update in next version along with removing encoding for 
phy-drive-ldo-level property.

> >
> > >
> > > +  phy-rescode-offset-top:
> > > +$ref: /schemas/types.yaml#/definitions/uint8-array
> > > +minItems: 5
> > > +maxItems: 5
> > > +description:
> > > +  Integer array of offset for pull-up legs rescode for all five 
> > > lanes.
> > > +  To offset the drive strength from the calibrated value in an 
> > > increasing
> > > +  or decreasing manner, use 6 bit two’s complement values.
> >
> > dtc should support negative values, google hints that <(-2)> should work.
This format is working.
> 
> Yes, but the schema checks don't check negative values correctly yet. So you
> can use 'int8-array', but just don't use negative values in the examples. I'm
> working on changes that will fix this issue.
> 
> What does 6-bit mean? 0x3f is negative? Just sign extend the values and
> specify the valid range instead:
> 
> minimum: -32
> maximum: 31
Yes, Rob. 0x3f is negative value (-1) in 6-bit two's complement.
I will implement your suggestion in the next patch version.

> 
> Rob

Thanks,
Rajeev


Re: [v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

2022-01-10 Thread Rob Herring
On Mon, Jan 10, 2022 at 05:06:03PM +0300, Dmitry Baryshkov wrote:
> On Mon, 10 Jan 2022 at 15:56, Rajeev Nandan  wrote:
> >
> > In most cases, the default values of DSI PHY tuning registers should be
> > sufficient as they are fully optimized. However, in some cases where
> > extreme board parasitics cause the eye shape to degrade, the override
> > bits can be used to improve the signal quality.
> >
> > The general guidelines for DSI PHY tuning include:
> > - High and moderate data rates may benefit from the drive strength and
> >   drive level tuning.
> > - Drive strength tuning will affect the output impedance and may be used
> >   for matching optimization.
> > - Drive level tuning will affect the output levels without affecting the
> >   impedance.
> >
> > The clock and data lanes have a calibration circuitry feature. The drive
> > strength tuning can be done by adjusting rescode offset for hstop/hsbot,
> > and the drive level tuning can be done by adjusting the LDO output level
> > for the HSTX drive.
> >
> > Signed-off-by: Rajeev Nandan 
> > ---
> >
> > Changes in v2:
> >  - More details in the commit text (Stephen Boyd)
> >  - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
> >  - Do not take values that are going to be unused (Dmitry Baryshkov)
> >
> >  .../bindings/display/msm/dsi-phy-10nm.yaml | 33 
> > ++
> >  1 file changed, 33 insertions(+)
> >
> > diff --git 
> > a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml 
> > b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > index 4399715..d0eb8f6 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > @@ -35,6 +35,35 @@ properties:
> >Connected to DSI0_MIPI_DSI_PLL_VDDA0P9 pin for sc7180 target and
> >connected to VDDA_MIPI_DSI_0_PLL_0P9 pin for sdm845 target
> 
> Generic note:
> I think these properties should be prefixed with "qcom," prefix.
> 
> >
> > +  phy-rescode-offset-top:
> > +$ref: /schemas/types.yaml#/definitions/uint8-array
> > +minItems: 5
> > +maxItems: 5
> > +description:
> > +  Integer array of offset for pull-up legs rescode for all five lanes.
> > +  To offset the drive strength from the calibrated value in an 
> > increasing
> > +  or decreasing manner, use 6 bit two’s complement values.
> 
> dtc should support negative values, google hints that <(-2)> should work.

Yes, but the schema checks don't check negative values correctly yet. So 
you can use 'int8-array', but just don't use negative values in the 
examples. I'm working on changes that will fix this issue.

What does 6-bit mean? 0x3f is negative? Just sign extend the values and 
specify the valid range instead:

minimum: -32
maximum: 31

Rob


Re: [v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

2022-01-10 Thread Rob Herring
On Mon, 10 Jan 2022 18:25:35 +0530, Rajeev Nandan wrote:
> In most cases, the default values of DSI PHY tuning registers should be
> sufficient as they are fully optimized. However, in some cases where
> extreme board parasitics cause the eye shape to degrade, the override
> bits can be used to improve the signal quality.
> 
> The general guidelines for DSI PHY tuning include:
> - High and moderate data rates may benefit from the drive strength and
>   drive level tuning.
> - Drive strength tuning will affect the output impedance and may be used
>   for matching optimization.
> - Drive level tuning will affect the output levels without affecting the
>   impedance.
> 
> The clock and data lanes have a calibration circuitry feature. The drive
> strength tuning can be done by adjusting rescode offset for hstop/hsbot,
> and the drive level tuning can be done by adjusting the LDO output level
> for the HSTX drive.
> 
> Signed-off-by: Rajeev Nandan 
> ---
> 
> Changes in v2:
>  - More details in the commit text (Stephen Boyd)
>  - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
>  - Do not take values that are going to be unused (Dmitry Baryshkov)
> 
>  .../bindings/display/msm/dsi-phy-10nm.yaml | 33 
> ++
>  1 file changed, 33 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml:63:54: 
[error] syntax error: mapping values are not allowed here (syntax)

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml:  mapping 
values are not allowed in this context
  in "", line 63, column 54
make[1]: *** Deleting file 
'Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.example.dts'
Traceback (most recent call last):
  File "/usr/local/bin/dt-extract-example", line 46, in 
binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
  File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 434, 
in load
return constructor.get_single_data()
  File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", 
line 119, in get_single_data
node = self.composer.get_single_node()
  File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
  File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in 
_ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in 
_ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 891, in 
_ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
ruamel.yaml.scanner.ScannerError: mapping values are not allowed in this context
  in "", line 63, column 54
make[1]: *** [Documentation/devicetree/bindings/Makefile:25: 
Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml:
 ignoring, error parsing file
make: *** [Makefile:1413: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1577891

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.



Re: [v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

2022-01-10 Thread Dmitry Baryshkov
On Mon, 10 Jan 2022 at 15:56, Rajeev Nandan  wrote:
>
> In most cases, the default values of DSI PHY tuning registers should be
> sufficient as they are fully optimized. However, in some cases where
> extreme board parasitics cause the eye shape to degrade, the override
> bits can be used to improve the signal quality.
>
> The general guidelines for DSI PHY tuning include:
> - High and moderate data rates may benefit from the drive strength and
>   drive level tuning.
> - Drive strength tuning will affect the output impedance and may be used
>   for matching optimization.
> - Drive level tuning will affect the output levels without affecting the
>   impedance.
>
> The clock and data lanes have a calibration circuitry feature. The drive
> strength tuning can be done by adjusting rescode offset for hstop/hsbot,
> and the drive level tuning can be done by adjusting the LDO output level
> for the HSTX drive.
>
> Signed-off-by: Rajeev Nandan 
> ---
>
> Changes in v2:
>  - More details in the commit text (Stephen Boyd)
>  - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
>  - Do not take values that are going to be unused (Dmitry Baryshkov)
>
>  .../bindings/display/msm/dsi-phy-10nm.yaml | 33 
> ++
>  1 file changed, 33 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml 
> b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> index 4399715..d0eb8f6 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> @@ -35,6 +35,35 @@ properties:
>Connected to DSI0_MIPI_DSI_PLL_VDDA0P9 pin for sc7180 target and
>connected to VDDA_MIPI_DSI_0_PLL_0P9 pin for sdm845 target

Generic note:
I think these properties should be prefixed with "qcom," prefix.

>
> +  phy-rescode-offset-top:
> +$ref: /schemas/types.yaml#/definitions/uint8-array
> +minItems: 5
> +maxItems: 5
> +description:
> +  Integer array of offset for pull-up legs rescode for all five lanes.
> +  To offset the drive strength from the calibrated value in an increasing
> +  or decreasing manner, use 6 bit two’s complement values.

dtc should support negative values, google hints that <(-2)> should work.

> +
> +  phy-rescode-offset-bot:
> +$ref: /schemas/types.yaml#/definitions/uint8-array
> +minItems: 5
> +maxItems: 5
> +description:
> +  Integer array of offset for pull-down legs rescode for all five lanes.
> +  To offset the drive strength from the calibrated value in an increasing
> +  or decreasing manner, use 6 bit two’s complement values.
> +
> +  phy-drive-ldo-level:
> +$ref: /schemas/types.yaml#/definitions/uint8
> +minimum: 0
> +maximum: 7
> +description:
> +  The PHY LDO has an amplitude tuning feature to adjust the LDO output
> +  for the HSTX drive. To offset the drive level from the default value,
> +  supported levels are with the following mapping:
> +  0 = 375mV, 1 = 400mV, 2 = 425mV, 3 = 450mV, 4 = 475mV, 5 = 500mV,
> +  6 = 500mV, 7 = 500mV

No encoding please. Specify the values in the dts and convert them
into the register values in the driver.

> +
>  required:
>- compatible
>- reg
> @@ -64,5 +93,9 @@ examples:
>   clocks = < DISP_CC_MDSS_AHB_CLK>,
>< RPMH_CXO_CLK>;
>   clock-names = "iface", "ref";
> +
> + phy-resocde-offset-top = /bits/ 8 <0x0 0x0 0x0 0x0 0x0>;
> + phy-rescode-offset-bot = /bits/ 8 <0x0 0x0 0x0 0x0 0x0>;
> + phy-drive-ldo-level = /bits/ 8 <1>;

--
With best wishes
Dmitry