On Wed, 22 May 2024 at 21:38, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> 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.baryshkov@xxxxxxxxxx>
> > ---
> > .../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
