[PATCH v5 2/9] drm/exynos: move connector creation to attach callback
Andrzej Hajda
a.hajda at samsung.com
Mon Aug 20 09:00:13 UTC 2018
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.purski at 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.purski at samsung.com>
>>>> [ a.hajda at samsung.com: v5 ]
>>>> Signed-off-by: Andrzej Hajda <a.hajda at 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
it is something which require additional investigation, discussion and
separate patchset.
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.
[1]: Documentation/devicetree/bindings/display/mipi-dsi-bus.txt
[2]: Documentation/devicetree/bindings/graph.txt
Regards
Andrzej
>
> Thanks,
> Inki Dae
>
>>> Then, you embedded bridge device node in dsi device node[1] but the binding document below doesn't mention about this,
>>> Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt
>> But this document says about child nodes, and Toshiba bridge is MIPI-DSI
>> child [1].
>>
>> Regards
>> Andrzej
>>
>>> [1] https://lkml.org/lkml/2018/6/19/237
>>>
>>> Thanks,
>>> Inki Dae
>>>
>>>> + if (out_bridge) {
>>>> + drm_bridge_attach(encoder, out_bridge, NULL);
>>>> + dsi->out_bridge = out_bridge;
>>>> + encoder->bridge = NULL;
>>>> + } else {
>>>> + int ret = exynos_dsi_create_connector(encoder);
>>>> +
>>>> + if (ret) {
>>>> + DRM_ERROR("failed to create connector ret = %d\n", ret);
>>>> + drm_encoder_cleanup(encoder);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + dsi->panel = of_drm_find_panel(device->dev.of_node);
>>>> + if (dsi->panel) {
>>>> + drm_panel_attach(dsi->panel, &dsi->connector);
>>>> + dsi->connector.status = connector_status_connected;
>>>> + }
>>>> + }
>>>>
>>>> /*
>>>> * This is a temporary solution and should be made by more generic way.
>>>> @@ -1518,11 +1542,6 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>>>> dsi->lanes = device->lanes;
>>>> dsi->format = device->format;
>>>> dsi->mode_flags = device->mode_flags;
>>>> - dsi->panel = of_drm_find_panel(device->dev.of_node);
>>>> - if (dsi->panel) {
>>>> - drm_panel_attach(dsi->panel, &dsi->connector);
>>>> - dsi->connector.status = connector_status_connected;
>>>> - }
>>>> exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode =
>>>> !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO);
>>>>
>>>> @@ -1538,19 +1557,21 @@ static int exynos_dsi_host_detach(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;
>>>> -
>>>> - mutex_lock(&drm->mode_config.mutex);
>>>> + struct drm_device *drm = dsi->encoder.dev;
>>>>
>>>> if (dsi->panel) {
>>>> + mutex_lock(&drm->mode_config.mutex);
>>>> exynos_dsi_disable(&dsi->encoder);
>>>> drm_panel_detach(dsi->panel);
>>>> dsi->panel = NULL;
>>>> dsi->connector.status = connector_status_disconnected;
>>>> + mutex_unlock(&drm->mode_config.mutex);
>>>> + } else {
>>>> + if (dsi->out_bridge->funcs->detach)
>>>> + dsi->out_bridge->funcs->detach(dsi->out_bridge);
>>>> + dsi->out_bridge = NULL;
>>>> }
>>>>
>>>> - mutex_unlock(&drm->mode_config.mutex);
>>>> -
>>>> if (drm->mode_config.poll_enabled)
>>>> drm_kms_helper_hotplug_event(drm);
>>>>
>>>> @@ -1654,13 +1675,6 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>>>> if (ret < 0)
>>>> return ret;
>>>>
>>>> - ret = exynos_dsi_create_connector(encoder);
>>>> - if (ret) {
>>>> - DRM_ERROR("failed to create connector ret = %d\n", ret);
>>>> - drm_encoder_cleanup(encoder);
>>>> - return ret;
>>>> - }
>>>> -
>>>> if (dsi->in_bridge_node) {
>>>> in_bridge = of_drm_find_bridge(dsi->in_bridge_node);
>>>> if (in_bridge)
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>>> the body of a message to majordomo at vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>
>>
>
More information about the dri-devel
mailing list