On 21.08.2018 07:27, Inki Dae wrote:
>
> 2018년 08월 20일 18:00에 Andrzej Hajda 이(가) 쓴 글:
>> On 17.08.2018 03:56, Inki Dae wrote:
>>> 2018년 08월 13일 20:17에 Andrzej Hajda 이(가) 쓴 글:
>>>> On 07.08.2018 10:53, Inki Dae wrote:
>>>>> 2018년 07월 26일 00:46에 Andrzej Hajda 이(가) 쓴 글:
>>>>>> From: Maciej Purski <m.pur...@samsung.com>
>>>>>>
>>>>>> The current implementation assumes that the only possible peripheral
>>>>>> device for DSIM is a panel. Using an output bridge child device
>>>>>> should also be possible.
>>>>>>
>>>>>> If an output bridge is available, don't create a new connector.
>>>>>> Instead, call drm_bridge_attach() and set encoder's bridge to NULL
>>>>>> in order to avoid an out bridge from being visible by the framework, as
>>>>>> the DSI bus needs control on enabling its child output bridge.
>>>>>>
>>>>>> Such sequence is required by Toshiba TC358764 bridge, which is a DSI
>>>>>> peripheral bridge device.
>>>>>>
>>>>>> changed in v5:
>>>>>> - detach bridge in mipi_dsi detach callback
>>>>>>
>>>>>> Signed-off-by: Maciej Purski <m.pur...@samsung.com>
>>>>>> [ a.ha...@samsung.com: v5 ]
>>>>>> Signed-off-by: Andrzej Hajda <a.ha...@samsung.com>
>>>>>> ---
>>>>>> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 50 ++++++++++++++++---------
>>>>>> 1 file changed, 32 insertions(+), 18 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>>>> b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>>>> index 351403f9d245..f5f51f584fa0 100644
>>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>>>> @@ -255,6 +255,7 @@ struct exynos_dsi {
>>>>>> struct mipi_dsi_host dsi_host;
>>>>>> struct drm_connector connector;
>>>>>> struct drm_panel *panel;
>>>>>> + struct drm_bridge *out_bridge;
>>>>>> struct device *dev;
>>>>>>
>>>>>> void __iomem *reg_base;
>>>>>> @@ -1499,7 +1500,30 @@ static int exynos_dsi_host_attach(struct
>>>>>> mipi_dsi_host *host,
>>>>>> struct mipi_dsi_device *device)
>>>>>> {
>>>>>> struct exynos_dsi *dsi = host_to_dsi(host);
>>>>>> - struct drm_device *drm = dsi->connector.dev;
>>>>>> + struct drm_encoder *encoder = &dsi->encoder;
>>>>>> + struct drm_device *drm = encoder->dev;
>>>>>> + struct drm_bridge *out_bridge;
>>>>>> +
>>>>>> + out_bridge = of_drm_find_bridge(device->dev.of_node);
>>>>> Is there any reason to find out_bridge without considering device tree
>>>>> graph binding?
>>>> If the sink is a child MIPI-DSI device, the bindings can be omitted, as
>>>> in case of all DSI panels in Exynos platforms.
>>>> In case bindings are not present you cannot use graph functions.
>>> In other words, this means that this patch doesn't allow to use the device
>>> tree graph binding.
>>> I.e., if other people wrote the graph things in his board dts file for the
>>> use of bridge device then with this patch he cannot use the bride device.
>>>
>>> So I think it would be better to allow to use both of them, as a child
>>> device and graph binding.
>>>
>>> How about trying to bind the graph things using drm_of_find_panel_or_bridge
>>> funtion first and then for child device way?
>> It could be done this way, but it should be done for panels and for
>> bridges, and it should be rather generic helper, not Exynos specific. So
> As the function name says, drm_of_find_panel_or_bridge function will return
> panel, bridge or both of them according to given arguments.
>
>
>> it is something which require additional investigation, discussion and
>> separate patchset.
> So I think we wouldn't need additional discussion at all becuase we don't
> touch panel but add only bridge device as output, and the use of this
> function for bridge looks more generic way.
But drm_of_find_panel_or_bridge is only for looking for non-dsi devices,
or more specifically for looking for devices connected in DTS via
DT-graph. This is not case of panels and bridges used in Exynos boards.
So this function is currently useless with our boards. Maybe some day we
will have bridge/panel controlled via I2C and then it will become
useful, but for now it serves for nothing.
> Of course, as a separate patch, we could consider using this function for
> panel device later.
As you said drm_of_find_panel_or_bridge looks for panel and/or bridge,
and it was created to merge similar code in panel and bridge lookup,
using it only for bridges and not for panels seems against it's purpose.
>
>> On the other side this patch is enough to correctly handle all DSI
>> bridges in existing Exynos boards.
>>
>>> And I'm not clear about what kinds of devices could be a child device of
>>> DSI, which isn't required for the graph binding.
>>> Is there any document I can refer to?
>> DSI devices (peripherals) should be described as child nodes of DSI host
>> node in DT bindings, it is described in [1]. And you can find all dsi
>> panels in exynos based boards are modeled this way.
>> And the graph documentation [2] says that graphs should be used for
>> connections that "can not be inferred from device tree parent-child
>> relationships", it was emphasised multiple times by Rob in discussions
>> regarding bindings of DSI devices.
> Hm... I don't see why Rob introduced drm_of_find_panel_or_bridge fucntion[1]
> if DSI devices should be described as child nodes.
> And some DSI drivers of other ARM DRM - vc4, mediatek, and kirin - use only
> drm_of_find_panel_or_bridge funtion.
Because this function was created for i2c/spi controlled bridges/panels
and probably these boards use only such devices.
Regards
Andrzej
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
