On 30.08.2018 14:32, Linus Walleij wrote:
> On Thu, Apr 26, 2018 at 10:07 AM Jyri Sarha <jsarha@xxxxxx> 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@xxxxxx>
>> Reviewed-by: Eric Anholt <eric@xxxxxxxxxx>
> 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@a0350000 {
> status = "okay";
>
> dsi@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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
