Re: [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Neil Armstrong]

On 15/08/2022 02:15, Dmitry Osipenko wrote:
> 08.08.2022 12:51, Neil Armstrong пишет:
> > On 08/08/2022 11:15, Neil Armstrong wrote:
> > > Hi Dmitry,
> > >
> > > On 31/07/2022 22:07, Dmitry Osipenko wrote:
> > > > 13.01.2022 17:43, Neil Armstrong пишет:
> > > > > This adds support for DRM_BRIDGE_ATTACH_NO_CONNECTOR by adding the
> > > > > bridge get_edid() and detect() callbacks after refactoring the
> > > > > connector
> > > > > get_modes() and connector_detect() callbacks.
> > > > >
> > > > > In order to keep the bridge working, extra code in get_modes() has been
> > > > > moved to more logical places.
> > > > >
> > > > > Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
> > > > > ---
> > > > >    drivers/gpu/drm/bridge/sii902x.c | 129
> > > > > ++++++++++++++++++++++++-------
> > >
> > > 1 file changed, 99 insertions(+), 30 deletions(-)
> > > > > diff --git a/drivers/gpu/drm/bridge/sii902x.c
> > > > > b/drivers/gpu/drm/bridge/sii902x.c
> > > > > index 89558e581530..65549fbfdc87 100644
> > > > > --- a/drivers/gpu/drm/bridge/sii902x.c
> > > > > +++ b/drivers/gpu/drm/bridge/sii902x.c
> >
> > [...]
> >
> > > > >        }
> > > > > +    endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> > > > > +    if (endpoint) {
> > > > > +        struct device_node *remote =
> > > > > of_graph_get_remote_port_parent(endpoint);
> > > > > +
> > > > > +        of_node_put(endpoint);
> > > > > +        if (!remote) {
> > > > > +            dev_err(dev, "Endpoint in port@1 unconnected\n");
> > > > > +            return -ENODEV;
> > > > > +        }
> > > > > +
> > > > > +        if (!of_device_is_available(remote)) {
> > > > > +            dev_err(dev, "port@1 remote device is disabled\n");
> > > > > +            of_node_put(remote);
> > > > > +            return -ENODEV;
> > > > > +        }
> > > > > +
> > > > > +        sii902x->next_bridge = of_drm_find_bridge(remote);
> > > > > +        of_node_put(remote);
> > > > > +        if (!sii902x->next_bridge)
> > > > > +            return -EPROBE_DEFER;
> > > >
> > > > Hi,
> > > >
> > > > This patch broke ARM/QEMU vexpress display because of_drm_find_bridge()
> > > > always fail with -EPROBE_DEFER. Reverting this patch returns display
> > > > back. Please fix or revert, thanks in advance.
> > >
> > > Can you share a QEMU cmdline to reproduce ?
> >
> > Actually the vexpress DT has multiple input ports instead of a single
> > input port at @0
> > and an output port at @1 like documented in the bindings:
> >
> > vexpress-v2m.dtsi#L303-L307:
> > ports {
> >      #address-cells = <1>;
> >      #size-cells = <0>;
> >
> >      /*
> >       * Both the core tile and the motherboard routes their output
> >       * pads to this transmitter. The motherboard system controller
> >       * can select one of them as input using a mux register in
> >       * "arm,vexpress-muxfpga". The Vexpress with the CA9 core tile is
> >       * the only platform with this specific set-up.
> >       */
> >      port@0 {
> >          reg = <0>;
> >          dvi_bridge_in_ct: endpoint {
> >              remote-endpoint = <&clcd_pads_ct>;
> >          };
> >      };
> >      port@1 {
> >          reg = <1>;
> >          dvi_bridge_in_mb: endpoint {
> >              remote-endpoint = <&clcd_pads_mb>;
> >          };
> >      };
> > };
> >
> > bindings:
> >    ports:
> >      $ref: /schemas/graph.yaml#/properties/ports
> >
> >      properties:
> >        port@0:
> >          $ref: /schemas/graph.yaml#/properties/port
> >          description: Parallel RGB input port
> >
> >        port@1:
> >          $ref: /schemas/graph.yaml#/properties/port
> >          description: HDMI output port
> >
> >        port@3:
> >          $ref: /schemas/graph.yaml#/properties/port
> >          description: Sound input port
> >
> > The patch is conform to the bindings, the DT was working but is actually
> > not valid.
>
> I haven't looked closely at how to fix this properly, but if we can fix
> it using of_machine_is_compatible("arm,vexpress") workaround in the
> driver, then it will be good enough at least as a temporal fix, IMO.

If other maintainers are ok with that, it can be temporary fix until the DT gets fixed.

Neil