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....@samsung.com>
>> Signed-off-by: Shuah Khan <shua...@osg.samsung.com>
>> Signed-off-by: Inki Dae <inki....@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

Reply via email to