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

[Andrzej Hajda]

On 23.05.2018 12:46, Peter Rosin wrote:
> On 2018-05-23 11:39, Andrzej Hajda wrote:
>
> *snip*
>
>> Panels are managed by dsi host only if dsi host implements it, and it is
>> up to dsi host author, it is not mandatory. The only case I am aware of
>> is exynos-dsi. I guess most developers are not aware of the problem, or
>> they do not care, or ... whatever. This patch shows it very clearly.
> *snip*
>
>> With such approach you do not protect dsi hosts without dynamic panel
>> management support. As I said earlier: it is matter of panel consumers
>> (for example dsi host), ie if it implements dynamic panel management, it
>> should attach panel without creating device_links, as it take care of
>> disappearing panels. If it does not perform dynamic management it should
>> rely on default behavior - links should be created, to protect drm
>> device from using dangling pointers.
> So, you're saying that the only place that needs to be patched to call
> drm_panel_attach_without_link is exynos_dsi_host_attach()?

It should be at least there, but I suppose only there.

>
> What about the dw-mipi-dsi driver in rockchip/dw-mipi-dsi.c? It also
> calls drm_panel_attach/drm_panel_detach in its dw_mipi_dsi_host_attach
> and dw_mipi_dsi_host_detach functions.

dw_mipi_dsi_host_detach calls only drm_panel_detach, dangling pointer
will stay in dsi->panel, and nothing prevents from using it. I guess it
is just buggy code.

>
> Similar thing in bridge/synopsis/dw-mipi-dsi.c, but it calls the
> wrappers drm_panel_bridge_add/remove to create an implicit bridge
> that in turn handles the panel. So, do we need a to add a
> drm_panel_bridge_add_without_link too?

Again looks like a buggy code, before panel removal it should be turned
off, it should be performed under lock,....

>
> What about tegra/dsi.c? It also calls drm_panel_attach in its
> tegra_dsi_host_attach function? But that one doesn't detach the
> panel in its tegra_dsi_host_detach. Is that a bug?

Also looks like a bug, but this is dual-channel device so I am not so sure.

Regards
Andrzej