Re: [PATCH 1/2] drm/exynos: dsi: move of_drm_find_bridge call into probe

[Inki Dae <inki.dae@samsung.com>]

2017년 07월 03일 19:34에 Andrzej Hajda 이(가) 쓴 글:
> On 03.07.2017 09:27, Inki Dae wrote:
>> This patch moves of_drm_find_bridge call into probe.
>>
>> It doesn't need to call of_drm_find_bridge function every time
>> bind callback is called. It's enough to call this funcation
>> at probe one time.
>>
>> Suggested-by: Inki Dae <inki.dae@samsung.com>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>> Signed-off-by: Inki Dae <inki.dae@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 23 ++++++++++++-----------
>>  1 file changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> index b6a46d9..2412b23 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> @@ -1661,7 +1661,6 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>>  	struct drm_encoder *encoder = dev_get_drvdata(dev);
>>  	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
>>  	struct drm_device *drm_dev = data;
>> -	struct drm_bridge *bridge;
>>  	int ret;
>>  
>>  	ret = exynos_drm_crtc_get_pipe_from_type(drm_dev,
>> @@ -1685,12 +1684,6 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>>  		return ret;
>>  	}
>>  
>> -	if (dsi->bridge_node) {
>> -		bridge = of_drm_find_bridge(dsi->bridge_node);
>> -		if (bridge)
>> -			drm_bridge_attach(encoder, bridge, NULL);
>> -	}
>> -
>>  	return mipi_dsi_host_register(&dsi->dsi_host);
>>  }
>>  
>> @@ -1798,6 +1791,18 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>>  
>>  	platform_set_drvdata(pdev, &dsi->encoder);
>>  
>> +	if (dsi->bridge_node) {
>> +		struct drm_bridge *bridge;
>> +
>> +		bridge = of_drm_find_bridge(dsi->bridge_node);
>> +		if (!bridge)
>> +			return -EPROBE_DEFER;
>> +
>> +		of_node_put(dsi->bridge_node);
>> +		drm_bridge_attach(&dsi->encoder, bridge, NULL);
>> +	}
>> +
>> +
> 
> One of benefits of componentized drivers is that they do not need to use
> probe deferall to wait for other components. There is guarantee that in
> bind callback all components are already probed.
> This patch looks like step back - it reintroduces probe deferall despite
> of existence of better mechanism.

Agree. This patch avoids of_drm_find_bridge function is called repeately and also this makes probe to be deferred.
I will skip this patch.

> Another issue is that now drm_bridge_attach is called before drm device
> creation, and before encoder is registered, even if it works for now, it
> does not seem proper, and it can beat us later.
> For sure bridge->dev is unitialized, and it can cause warnings.
> 
> If you want to put bridge code together it should be put rather into
> bind callback.

Oops, sorry. as I commented[1] at the original patch from Shuah, drm_bridge_attach should be keepped in bind callback.
This is my mistake.

[1] https://patchwork.kernel.org/patch/9808497/


Thanks,
Inki Dae

> 
> Regards
> Andrzej
> 
>>  	pm_runtime_enable(dev);
>>  
>>  	return component_add(dev, &exynos_dsi_component_ops);
>> @@ -1805,10 +1810,6 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>>  
>>  static int exynos_dsi_remove(struct platform_device *pdev)
>>  {
>> -	struct exynos_dsi *dsi = platform_get_drvdata(pdev);
>> -
>> -	of_node_put(dsi->bridge_node);
>> -
>>  	pm_runtime_disable(&pdev->dev);
>>  
>>  	component_del(&pdev->dev, &exynos_dsi_component_ops);
> 
> 
> 
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel