On Wed, 22 May 2024 at 21:38, Abhinav Kumar <quic_abhin...@quicinc.com> wrote:
>
>
>
> On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote:
> > Command mode panels provide TE signal back to the DSI host to signal
> > that the frame display has completed and update of the image will not
> > cause tearing. Usually it is connected to the first GPIO with the
> > mdp_vsync function, which is the default. In such case the property can
> > be skipped.
> >
>
> This is a good addition overall. Some comments below.
>
> > Signed-off-by: Dmitry Baryshkov <dmitry.barysh...@linaro.org>
> > ---
> >   .../bindings/display/msm/dsi-controller-main.yaml        | 16 
> > ++++++++++++++++
> >   1 file changed, 16 insertions(+)
> >
> > diff --git 
> > a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml 
> > b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > index 1fa28e976559..c1771c69b247 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > @@ -162,6 +162,21 @@ properties:
> >                   items:
> >                     enum: [ 0, 1, 2, 3 ]
> >
> > +              qcom,te-source:
> > +                $ref: /schemas/types.yaml#/definitions/string
> > +                description:
> > +                  Specifies the source of vsync signal from the panel used 
> > for
> > +                  tearing elimination. The default is mdp_gpio0.
>
> panel --> command mode panel?
>
> > +                enum:
> > +                  - mdp_gpio0
> > +                  - mdp_gpio1
> > +                  - mdp_gpio2
>
> are gpio0, gpio1 and gpio2 referring to the vsync_p, vsync_s and vsync_e
> sources?

No idea, unfortunately. They are gpioN or just mdp_vsync all over the
place. For the reference, in case of the SDM845 and Pixel3 the signal
is routed through SoC GPIO12.

> In that case wouldnt it be better to name it like that?
>
> > +                  - timer0
> > +                  - timer1
> > +                  - timer2
> > +                  - timer3
> > +                  - timer4
> > +
>
> These are indicating watchdog timer sources right?

Yes.

>
> >       required:
> >         - port@0
> >         - port@1
> > @@ -452,6 +467,7 @@ examples:
> >                             dsi0_out: endpoint {
> >                                      remote-endpoint = <&sn65dsi86_in>;
> >                                      data-lanes = <0 1 2 3>;
> > +                                   qcom,te-source = "mdp_gpio2";
>
> I have a basic doubt on this. Should te-source should be in the input
> port or the output one for the controller? Because TE is an input to the
> DSI. And if the source is watchdog timer then it aligns even more as a
> property of the input endpoint.

I don't really want to split this. Both data-lanes and te-source are
properties of the link between the DSI and panel. You can not really
say which side has which property.

> >                             };
> >                     };
> >              };
> >

-- 
With best wishes
Dmitry

Reply via email to