[PATCH v4 2/2] drm/panel: Add device_link from panel device to drm device

[Andrzej Hajda]

On 30.08.2018 14:32, Linus Walleij wrote:
> On Thu, Apr 26, 2018 at 10:07 AM Jyri Sarha <jsarha at ti.com> wrote:
>
>> Add device_link from panel device (supplier) to drm device (consumer)
>> when drm_panel_attach() is called. This patch should protect the
>> master drm driver if an attached panel driver unbinds while it is in
>> use. The device_link should make sure the drm device is unbound before
>> the panel driver becomes unavailable.
>>
>> The device_link is removed when drm_panel_detach() is called. The
>> drm_panel_detach() should be called by the consumer DRM driver, not the
>> panel driver, otherwise both drivers are racing to delete the same link.
>>
>> Signed-off-by: Jyri Sarha <jsarha at ti.com>
>> Reviewed-by: Eric Anholt <eric at anholt.net>
> Hi I have a problem with an in-development driver and this patch.
>
> Do not worry! It is no regression. I just need help to do things right
> now.
>
> My panel is a DSI device on a DSI master:
>
>                 mcde at a0350000 {
>                         status = "okay";
>
>                         dsi at a0351000 {
>                                 port {
>                                         dsi0_out: endpoint {
>                                                 remote-endpoint = <&panel_in>;
>                                         };
>                                 };
>
>                                 panel: display {
>                                         compatible = "samsung,s6d16d0";
>                                         reg = <0>;
>                                         /* VDD1 on the component */
>                                         power-supply = <&ab8500_ldo_aux1_reg>;
>                                         reset-gpios = <&gpio2 1
> GPIO_ACTIVE_LOW>;
>
>                                         port {
>                                                 panel_in: endpoint {
>
> remote-endpoint = <&dsi0_out>;
>                                                 };
>                                         };
>                                 };
>                         };
>                 };
>
> So in my DSI host driver .bind() I do:
>
>     drm_encoder_init(drm, encoder, &mcde_dsi_encoder_funcs,
>              DRM_MODE_ENCODER_DSI, NULL);
>     drm_encoder_helper_add(encoder, &mcde_dsi_encoder_helper_funcs);
>     ret = drm_connector_init(encoder->dev, connector,
>                  &mcde_dsi_connector_funcs,
>                  DRM_MODE_CONNECTOR_DSI);
>
>     drm_connector_helper_add(connector, &mcde_dsi_connector_helper_funcs);
>     drm_connector_attach_encoder(connector, encoder);
>     drm_connector_register(connector);
>
> This works fine. I managed to create the encoder and connector for this
> DSI. Then:
>
>     ret = drm_of_find_panel_or_bridge(dev->of_node,
>                       0, 0, &panel, &bridge);
>     if (panel) {
>         bridge = drm_panel_bridge_add(panel,
>                           DRM_MODE_CONNECTOR_DSI);
>         if (IS_ERR(bridge)) {
>             dev_err(dev, "error adding panel bridge\n");
>             return PTR_ERR(bridge);
>         }
>         dev_info(dev, "connected to panel\n");
>         d->panel = panel;
>     }
>     d->connector.status = connector_status_connected;
>     ret = drm_bridge_attach(encoder, bridge, NULL);
>
> And this is where it blows up: it fails in
> device_link_add(connector->dev->dev, panel->dev, 0);
> device_is_dependent(consumer, supplier) because the
> consumer (connector) is dependent on the supplier (bridge).
>
> This happens because the connector struct device is the
> same as the bridge struct device, I suppose.

I guess it is rather because the code tries to make circular dependency:
1. panel depends on dsi-host because it is MIPI-DSI child device.
2. dsi-host probably depends on drm parent device (connector->dev->dev)
- what drm driver do you use?
3. drm parent dev depends on panel: this patch adds this dependency.

If 2nd point is true it becomes circular dependency, but please verify it.

Regards
Andrzej

>
> Isn't that OK? Please help me to see what's wrong with
> my architecture here, and what I need to do :/ I tried to
> follow examples set by MSM and Exynos DSI.
>
> Yours,
> Linus Walleij
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel