* [PATCH 01/24] drm/bridge: allow optionally specifying an .owner device
2018-04-26 22:31 ` [PATCH 00/24] device link, bridge supplier <-> drm device Peter Rosin
@ 2018-04-26 22:31 ` Peter Rosin
2018-04-26 22:31 ` [PATCH 17/24] drm/exynos: mic: provide an .owner device for the bridge Peter Rosin
` (6 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2018-04-26 22:31 UTC (permalink / raw)
To: linux-arm-kernel
Bridge drivers can now (temporarily, in a transition phase) select if
they want to provide a full owner or keep just providing an of_node.
By providing a full owner device, the bridge drivers no longer need
to provide an of_node since that node is available via the owner
device.
When all bridge drivers provide an owner device, that will become
mandatory and the .of_node member will be removed.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/drm_bridge.c | 3 ++-
include/drm/drm_bridge.h | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 1638bfe9627c..67147673fdeb 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -365,7 +365,8 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np)
mutex_lock(&bridge_lock);
list_for_each_entry(bridge, &bridge_list, list) {
- if (bridge->of_node == np) {
+ if ((bridge->owner && bridge->owner->of_node == np) ||
+ bridge->of_node == np) {
mutex_unlock(&bridge_lock);
return bridge;
}
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 3270fec46979..c28a75ad0ae2 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -254,6 +254,7 @@ struct drm_bridge_timings {
/**
* struct drm_bridge - central DRM bridge control structure
+ * @owner: device that owns the bridge
* @dev: DRM device this bridge belongs to
* @encoder: encoder to which this bridge is connected
* @next: the next bridge in the encoder chain
@@ -265,6 +266,7 @@ struct drm_bridge_timings {
* @driver_private: pointer to the bridge driver's internal context
*/
struct drm_bridge {
+ struct device *owner;
struct drm_device *dev;
struct drm_encoder *encoder;
struct drm_bridge *next;
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 17/24] drm/exynos: mic: provide an .owner device for the bridge
2018-04-26 22:31 ` [PATCH 00/24] device link, bridge supplier <-> drm device Peter Rosin
2018-04-26 22:31 ` [PATCH 01/24] drm/bridge: allow optionally specifying an .owner device Peter Rosin
@ 2018-04-26 22:31 ` Peter Rosin
2018-04-26 22:31 ` [PATCH 18/24] drm/mediatek: hdmi: " Peter Rosin
` (5 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2018-04-26 22:31 UTC (permalink / raw)
To: linux-arm-kernel
The .of_node member is going away.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/exynos/exynos_drm_mic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c
index 2174814273e2..453716cb4d3d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
@@ -417,8 +417,8 @@ static int exynos_mic_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, mic);
+ mic->bridge.owner = dev;
mic->bridge.funcs = &mic_bridge_funcs;
- mic->bridge.of_node = dev->of_node;
drm_bridge_add(&mic->bridge);
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 18/24] drm/mediatek: hdmi: provide an .owner device for the bridge
2018-04-26 22:31 ` [PATCH 00/24] device link, bridge supplier <-> drm device Peter Rosin
2018-04-26 22:31 ` [PATCH 01/24] drm/bridge: allow optionally specifying an .owner device Peter Rosin
2018-04-26 22:31 ` [PATCH 17/24] drm/exynos: mic: provide an .owner device for the bridge Peter Rosin
@ 2018-04-26 22:31 ` Peter Rosin
2018-04-26 22:31 ` [PATCH 22/24] drm/bridge: remove the .of_node member Peter Rosin
` (4 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2018-04-26 22:31 UTC (permalink / raw)
To: linux-arm-kernel
The .of_node member is going away.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 59a11026dceb..b4a7908e0cc6 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1694,8 +1694,8 @@ static int mtk_drm_hdmi_probe(struct platform_device *pdev)
mtk_hdmi_register_audio_driver(dev);
+ hdmi->bridge.owner = &pdev->dev;
hdmi->bridge.funcs = &mtk_hdmi_bridge_funcs;
- hdmi->bridge.of_node = pdev->dev.of_node;
drm_bridge_add(&hdmi->bridge);
ret = mtk_hdmi_clk_enable_audio(hdmi);
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 22/24] drm/bridge: remove the .of_node member
2018-04-26 22:31 ` [PATCH 00/24] device link, bridge supplier <-> drm device Peter Rosin
` (2 preceding siblings ...)
2018-04-26 22:31 ` [PATCH 18/24] drm/mediatek: hdmi: " Peter Rosin
@ 2018-04-26 22:31 ` Peter Rosin
2018-04-28 8:09 ` kbuild test robot
2018-04-26 22:31 ` [PATCH 23/24] drm/bridge: require the .owner to be filled in on drm_bridge_attach Peter Rosin
` (3 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2018-04-26 22:31 UTC (permalink / raw)
To: linux-arm-kernel
It is unused.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/drm_bridge.c | 3 +--
include/drm/drm_bridge.h | 4 ----
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 67147673fdeb..9f023bd84d56 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -365,8 +365,7 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np)
mutex_lock(&bridge_lock);
list_for_each_entry(bridge, &bridge_list, list) {
- if ((bridge->owner && bridge->owner->of_node == np) ||
- bridge->of_node == np) {
+ if (bridge->owner->of_node == np) {
mutex_unlock(&bridge_lock);
return bridge;
}
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index c28a75ad0ae2..3bc659f3e7d2 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -258,7 +258,6 @@ struct drm_bridge_timings {
* @dev: DRM device this bridge belongs to
* @encoder: encoder to which this bridge is connected
* @next: the next bridge in the encoder chain
- * @of_node: device node pointer to the bridge
* @list: to keep track of all added bridges
* @timings: the timing specification for the bridge, if any (may
* be NULL)
@@ -270,9 +269,6 @@ struct drm_bridge {
struct drm_device *dev;
struct drm_encoder *encoder;
struct drm_bridge *next;
-#ifdef CONFIG_OF
- struct device_node *of_node;
-#endif
struct list_head list;
const struct drm_bridge_timings *timings;
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 22/24] drm/bridge: remove the .of_node member
2018-04-26 22:31 ` [PATCH 22/24] drm/bridge: remove the .of_node member Peter Rosin
@ 2018-04-28 8:09 ` kbuild test robot
2018-04-30 11:14 ` Peter Rosin
0 siblings, 1 reply; 19+ messages in thread
From: kbuild test robot @ 2018-04-28 8:09 UTC (permalink / raw)
To: linux-arm-kernel
Hi Peter,
I love your patch! Yet something to improve:
[auto build test ERROR on v4.17-rc2]
[also build test ERROR on next-20180426]
[cannot apply to drm/drm-next robclark/msm-next drm-exynos/exynos-drm/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Peter-Rosin/device-link-bridge-supplier-drm-device/20180428-135229
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
drivers/gpu//drm/rockchip/rockchip_lvds.c: In function 'rockchip_lvds_bind':
>> drivers/gpu//drm/rockchip/rockchip_lvds.c:381:24: error: 'struct drm_bridge' has no member named 'of_node'
remote = lvds->bridge->of_node;
^~
vim +381 drivers/gpu//drm/rockchip/rockchip_lvds.c
34cc0aa25 Sandy Huang 2017-09-02 340
34cc0aa25 Sandy Huang 2017-09-02 341 static int rockchip_lvds_bind(struct device *dev, struct device *master,
34cc0aa25 Sandy Huang 2017-09-02 342 void *data)
34cc0aa25 Sandy Huang 2017-09-02 343 {
34cc0aa25 Sandy Huang 2017-09-02 344 struct rockchip_lvds *lvds = dev_get_drvdata(dev);
34cc0aa25 Sandy Huang 2017-09-02 345 struct drm_device *drm_dev = data;
34cc0aa25 Sandy Huang 2017-09-02 346 struct drm_encoder *encoder;
34cc0aa25 Sandy Huang 2017-09-02 347 struct drm_connector *connector;
34cc0aa25 Sandy Huang 2017-09-02 348 struct device_node *remote = NULL;
34cc0aa25 Sandy Huang 2017-09-02 349 struct device_node *port, *endpoint;
6bf2e0324 Sean Paul 2017-09-20 350 int ret = 0, child_count = 0;
34cc0aa25 Sandy Huang 2017-09-02 351 const char *name;
34cc0aa25 Sandy Huang 2017-09-02 352 u32 endpoint_id;
34cc0aa25 Sandy Huang 2017-09-02 353
34cc0aa25 Sandy Huang 2017-09-02 354 lvds->drm_dev = drm_dev;
34cc0aa25 Sandy Huang 2017-09-02 355 port = of_graph_get_port_by_id(dev->of_node, 1);
34cc0aa25 Sandy Huang 2017-09-02 356 if (!port) {
34cc0aa25 Sandy Huang 2017-09-02 357 DRM_DEV_ERROR(dev,
34cc0aa25 Sandy Huang 2017-09-02 358 "can't found port point, please init lvds panel port!\n");
34cc0aa25 Sandy Huang 2017-09-02 359 return -EINVAL;
34cc0aa25 Sandy Huang 2017-09-02 360 }
34cc0aa25 Sandy Huang 2017-09-02 361 for_each_child_of_node(port, endpoint) {
6bf2e0324 Sean Paul 2017-09-20 362 child_count++;
34cc0aa25 Sandy Huang 2017-09-02 363 of_property_read_u32(endpoint, "reg", &endpoint_id);
34cc0aa25 Sandy Huang 2017-09-02 364 ret = drm_of_find_panel_or_bridge(dev->of_node, 1, endpoint_id,
34cc0aa25 Sandy Huang 2017-09-02 365 &lvds->panel, &lvds->bridge);
34cc0aa25 Sandy Huang 2017-09-02 366 if (!ret)
34cc0aa25 Sandy Huang 2017-09-02 367 break;
34cc0aa25 Sandy Huang 2017-09-02 368 }
6bf2e0324 Sean Paul 2017-09-20 369 if (!child_count) {
6bf2e0324 Sean Paul 2017-09-20 370 DRM_DEV_ERROR(dev, "lvds port does not have any children\n");
6bf2e0324 Sean Paul 2017-09-20 371 ret = -EINVAL;
6bf2e0324 Sean Paul 2017-09-20 372 goto err_put_port;
6bf2e0324 Sean Paul 2017-09-20 373 } else if (ret) {
34cc0aa25 Sandy Huang 2017-09-02 374 DRM_DEV_ERROR(dev, "failed to find panel and bridge node\n");
34cc0aa25 Sandy Huang 2017-09-02 375 ret = -EPROBE_DEFER;
34cc0aa25 Sandy Huang 2017-09-02 376 goto err_put_port;
34cc0aa25 Sandy Huang 2017-09-02 377 }
34cc0aa25 Sandy Huang 2017-09-02 378 if (lvds->panel)
34cc0aa25 Sandy Huang 2017-09-02 379 remote = lvds->panel->dev->of_node;
34cc0aa25 Sandy Huang 2017-09-02 380 else
34cc0aa25 Sandy Huang 2017-09-02 @381 remote = lvds->bridge->of_node;
34cc0aa25 Sandy Huang 2017-09-02 382 if (of_property_read_string(dev->of_node, "rockchip,output", &name))
34cc0aa25 Sandy Huang 2017-09-02 383 /* default set it as output rgb */
34cc0aa25 Sandy Huang 2017-09-02 384 lvds->output = DISPLAY_OUTPUT_RGB;
34cc0aa25 Sandy Huang 2017-09-02 385 else
34cc0aa25 Sandy Huang 2017-09-02 386 lvds->output = lvds_name_to_output(name);
34cc0aa25 Sandy Huang 2017-09-02 387
34cc0aa25 Sandy Huang 2017-09-02 388 if (lvds->output < 0) {
34cc0aa25 Sandy Huang 2017-09-02 389 DRM_DEV_ERROR(dev, "invalid output type [%s]\n", name);
34cc0aa25 Sandy Huang 2017-09-02 390 ret = lvds->output;
34cc0aa25 Sandy Huang 2017-09-02 391 goto err_put_remote;
34cc0aa25 Sandy Huang 2017-09-02 392 }
34cc0aa25 Sandy Huang 2017-09-02 393
34cc0aa25 Sandy Huang 2017-09-02 394 if (of_property_read_string(remote, "data-mapping", &name))
34cc0aa25 Sandy Huang 2017-09-02 395 /* default set it as format vesa 18 */
34cc0aa25 Sandy Huang 2017-09-02 396 lvds->format = LVDS_VESA_18;
34cc0aa25 Sandy Huang 2017-09-02 397 else
34cc0aa25 Sandy Huang 2017-09-02 398 lvds->format = lvds_name_to_format(name);
34cc0aa25 Sandy Huang 2017-09-02 399
34cc0aa25 Sandy Huang 2017-09-02 400 if (lvds->format < 0) {
34cc0aa25 Sandy Huang 2017-09-02 401 DRM_DEV_ERROR(dev, "invalid data-mapping format [%s]\n", name);
34cc0aa25 Sandy Huang 2017-09-02 402 ret = lvds->format;
34cc0aa25 Sandy Huang 2017-09-02 403 goto err_put_remote;
34cc0aa25 Sandy Huang 2017-09-02 404 }
34cc0aa25 Sandy Huang 2017-09-02 405
34cc0aa25 Sandy Huang 2017-09-02 406 encoder = &lvds->encoder;
34cc0aa25 Sandy Huang 2017-09-02 407 encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
34cc0aa25 Sandy Huang 2017-09-02 408 dev->of_node);
34cc0aa25 Sandy Huang 2017-09-02 409
34cc0aa25 Sandy Huang 2017-09-02 410 ret = drm_encoder_init(drm_dev, encoder, &rockchip_lvds_encoder_funcs,
34cc0aa25 Sandy Huang 2017-09-02 411 DRM_MODE_ENCODER_LVDS, NULL);
34cc0aa25 Sandy Huang 2017-09-02 412 if (ret < 0) {
34cc0aa25 Sandy Huang 2017-09-02 413 DRM_DEV_ERROR(drm_dev->dev,
34cc0aa25 Sandy Huang 2017-09-02 414 "failed to initialize encoder: %d\n", ret);
34cc0aa25 Sandy Huang 2017-09-02 415 goto err_put_remote;
34cc0aa25 Sandy Huang 2017-09-02 416 }
34cc0aa25 Sandy Huang 2017-09-02 417
34cc0aa25 Sandy Huang 2017-09-02 418 drm_encoder_helper_add(encoder, &rockchip_lvds_encoder_helper_funcs);
34cc0aa25 Sandy Huang 2017-09-02 419
34cc0aa25 Sandy Huang 2017-09-02 420 if (lvds->panel) {
34cc0aa25 Sandy Huang 2017-09-02 421 connector = &lvds->connector;
34cc0aa25 Sandy Huang 2017-09-02 422 connector->dpms = DRM_MODE_DPMS_OFF;
34cc0aa25 Sandy Huang 2017-09-02 423 ret = drm_connector_init(drm_dev, connector,
34cc0aa25 Sandy Huang 2017-09-02 424 &rockchip_lvds_connector_funcs,
34cc0aa25 Sandy Huang 2017-09-02 425 DRM_MODE_CONNECTOR_LVDS);
34cc0aa25 Sandy Huang 2017-09-02 426 if (ret < 0) {
34cc0aa25 Sandy Huang 2017-09-02 427 DRM_DEV_ERROR(drm_dev->dev,
34cc0aa25 Sandy Huang 2017-09-02 428 "failed to initialize connector: %d\n", ret);
34cc0aa25 Sandy Huang 2017-09-02 429 goto err_free_encoder;
34cc0aa25 Sandy Huang 2017-09-02 430 }
34cc0aa25 Sandy Huang 2017-09-02 431
34cc0aa25 Sandy Huang 2017-09-02 432 drm_connector_helper_add(connector,
34cc0aa25 Sandy Huang 2017-09-02 433 &rockchip_lvds_connector_helper_funcs);
34cc0aa25 Sandy Huang 2017-09-02 434
34cc0aa25 Sandy Huang 2017-09-02 435 ret = drm_mode_connector_attach_encoder(connector, encoder);
34cc0aa25 Sandy Huang 2017-09-02 436 if (ret < 0) {
34cc0aa25 Sandy Huang 2017-09-02 437 DRM_DEV_ERROR(drm_dev->dev,
34cc0aa25 Sandy Huang 2017-09-02 438 "failed to attach encoder: %d\n", ret);
34cc0aa25 Sandy Huang 2017-09-02 439 goto err_free_connector;
34cc0aa25 Sandy Huang 2017-09-02 440 }
34cc0aa25 Sandy Huang 2017-09-02 441
34cc0aa25 Sandy Huang 2017-09-02 442 ret = drm_panel_attach(lvds->panel, connector);
34cc0aa25 Sandy Huang 2017-09-02 443 if (ret < 0) {
34cc0aa25 Sandy Huang 2017-09-02 444 DRM_DEV_ERROR(drm_dev->dev,
34cc0aa25 Sandy Huang 2017-09-02 445 "failed to attach panel: %d\n", ret);
34cc0aa25 Sandy Huang 2017-09-02 446 goto err_free_connector;
34cc0aa25 Sandy Huang 2017-09-02 447 }
34cc0aa25 Sandy Huang 2017-09-02 448 } else {
34cc0aa25 Sandy Huang 2017-09-02 449 lvds->bridge->encoder = encoder;
34cc0aa25 Sandy Huang 2017-09-02 450 ret = drm_bridge_attach(encoder, lvds->bridge, NULL);
34cc0aa25 Sandy Huang 2017-09-02 451 if (ret) {
34cc0aa25 Sandy Huang 2017-09-02 452 DRM_DEV_ERROR(drm_dev->dev,
34cc0aa25 Sandy Huang 2017-09-02 453 "failed to attach bridge: %d\n", ret);
34cc0aa25 Sandy Huang 2017-09-02 454 goto err_free_encoder;
34cc0aa25 Sandy Huang 2017-09-02 455 }
34cc0aa25 Sandy Huang 2017-09-02 456 encoder->bridge = lvds->bridge;
34cc0aa25 Sandy Huang 2017-09-02 457 }
34cc0aa25 Sandy Huang 2017-09-02 458
34cc0aa25 Sandy Huang 2017-09-02 459 pm_runtime_enable(dev);
34cc0aa25 Sandy Huang 2017-09-02 460 of_node_put(remote);
34cc0aa25 Sandy Huang 2017-09-02 461 of_node_put(port);
34cc0aa25 Sandy Huang 2017-09-02 462
34cc0aa25 Sandy Huang 2017-09-02 463 return 0;
34cc0aa25 Sandy Huang 2017-09-02 464
34cc0aa25 Sandy Huang 2017-09-02 465 err_free_connector:
34cc0aa25 Sandy Huang 2017-09-02 466 drm_connector_cleanup(connector);
34cc0aa25 Sandy Huang 2017-09-02 467 err_free_encoder:
34cc0aa25 Sandy Huang 2017-09-02 468 drm_encoder_cleanup(encoder);
34cc0aa25 Sandy Huang 2017-09-02 469 err_put_remote:
34cc0aa25 Sandy Huang 2017-09-02 470 of_node_put(remote);
34cc0aa25 Sandy Huang 2017-09-02 471 err_put_port:
34cc0aa25 Sandy Huang 2017-09-02 472 of_node_put(port);
34cc0aa25 Sandy Huang 2017-09-02 473
34cc0aa25 Sandy Huang 2017-09-02 474 return ret;
34cc0aa25 Sandy Huang 2017-09-02 475 }
34cc0aa25 Sandy Huang 2017-09-02 476
:::::: The code at line 381 was first introduced by commit
:::::: 34cc0aa2545603560c79aaea3340d8ff3a71bd10 drm/rockchip: Add support for Rockchip Soc LVDS
:::::: TO: Sandy Huang <hjc@rock-chips.com>
:::::: CC: Mark Yao <mark.yao@rock-chips.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 65228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180428/285eb5d4/attachment-0001.gz>
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 22/24] drm/bridge: remove the .of_node member
2018-04-28 8:09 ` kbuild test robot
@ 2018-04-30 11:14 ` Peter Rosin
0 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2018-04-30 11:14 UTC (permalink / raw)
To: linux-arm-kernel
On 2018-04-28 10:09, kbuild test robot wrote:
> Hi Peter,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on v4.17-rc2]
> [also build test ERROR on next-20180426]
> [cannot apply to drm/drm-next robclark/msm-next drm-exynos/exynos-drm/for-next]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Peter-Rosin/device-link-bridge-supplier-drm-device/20180428-135229
> config: arm-allmodconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm
>
> All errors (new ones prefixed by >>):
>
> drivers/gpu//drm/rockchip/rockchip_lvds.c: In function 'rockchip_lvds_bind':
>>> drivers/gpu//drm/rockchip/rockchip_lvds.c:381:24: error: 'struct drm_bridge' has no member named 'of_node'
> remote = lvds->bridge->of_node;
> ^~
>
> vim +381 drivers/gpu//drm/rockchip/rockchip_lvds.c
Ugh.
So, patch 1/24 needs to be amended with this
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index e67f4ea28c0e..3f33034b3f58 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -377,8 +377,10 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
}
if (lvds->panel)
remote = lvds->panel->dev->of_node;
- else
+ else if (lvds->bridge->of_node)
remote = lvds->bridge->of_node;
+ else
+ remote = lvds->bridge->owner->of_node;
if (of_property_read_string(dev->of_node, "rockchip,output", &name))
/* default set it as output rgb */
lvds->output = DISPLAY_OUTPUT_RGB;
and patch 22/24 with this
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 3f33034b3f58..8c82fa647536 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -377,8 +377,6 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
}
if (lvds->panel)
remote = lvds->panel->dev->of_node;
- else if (lvds->bridge->of_node)
- remote = lvds->bridge->of_node;
else
remote = lvds->bridge->owner->of_node;
if (of_property_read_string(dev->of_node, "rockchip,output", &name))
But that is of course just a stop-gap. The real fix is to adapt to the
"drm: bridge: Add support for static image formats?" series from Jacopo.
But that's orthogonal.
Cheers,
Peter
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 23/24] drm/bridge: require the .owner to be filled in on drm_bridge_attach
2018-04-26 22:31 ` [PATCH 00/24] device link, bridge supplier <-> drm device Peter Rosin
` (3 preceding siblings ...)
2018-04-26 22:31 ` [PATCH 22/24] drm/bridge: remove the .of_node member Peter Rosin
@ 2018-04-26 22:31 ` Peter Rosin
2018-04-30 15:24 ` Daniel Vetter
2018-04-26 22:31 ` [PATCH 24/24] drm/bridge: establish a link between the bridge supplier and consumer Peter Rosin
` (2 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2018-04-26 22:31 UTC (permalink / raw)
To: linux-arm-kernel
The .owner will be handy to have around.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/drm_bridge.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 9f023bd84d56..a038da696802 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -115,6 +115,9 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
if (!encoder || !bridge)
return -EINVAL;
+ if (WARN_ON(!bridge->owner))
+ return -EINVAL;
+
if (previous && (!previous->dev || previous->encoder != encoder))
return -EINVAL;
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 23/24] drm/bridge: require the .owner to be filled in on drm_bridge_attach
2018-04-26 22:31 ` [PATCH 23/24] drm/bridge: require the .owner to be filled in on drm_bridge_attach Peter Rosin
@ 2018-04-30 15:24 ` Daniel Vetter
2018-04-30 20:32 ` Peter Rosin
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2018-04-30 15:24 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 27, 2018 at 12:31:38AM +0200, Peter Rosin wrote:
> The .owner will be handy to have around.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
> drivers/gpu/drm/drm_bridge.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 9f023bd84d56..a038da696802 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -115,6 +115,9 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> if (!encoder || !bridge)
> return -EINVAL;
>
> + if (WARN_ON(!bridge->owner))
> + return -EINVAL;
I think conceptually this is checked at the wrong place, and I think also misnamed
a bit. The ->owner is essentially the struct device (and its associated
driver) that provides the drm_bridge. As such it should be filled out
already at drm_bridge_add() time, and I think the check should be in
there. For driver-internal bridges it might make sense to also check this
here, not sure. Or just require all bridges get added.
Wrt the name, I think we should call this pdev or something. ->owner
usually means the module owner. I think in other subsystems ->dev is used,
but in drm we use ->dev for the drm_device pointer, so totally different
thing. pdev = physical device is the best I came up with. Better
suggestions very much welcome.
-Daniel
> +
> if (previous && (!previous->dev || previous->encoder != encoder))
> return -EINVAL;
>
> --
> 2.11.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 23/24] drm/bridge: require the .owner to be filled in on drm_bridge_attach
2018-04-30 15:24 ` Daniel Vetter
@ 2018-04-30 20:32 ` Peter Rosin
0 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2018-04-30 20:32 UTC (permalink / raw)
To: linux-arm-kernel
On 2018-04-30 17:24, Daniel Vetter wrote:
> On Fri, Apr 27, 2018 at 12:31:38AM +0200, Peter Rosin wrote:
>> The .owner will be handy to have around.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>> drivers/gpu/drm/drm_bridge.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
>> index 9f023bd84d56..a038da696802 100644
>> --- a/drivers/gpu/drm/drm_bridge.c
>> +++ b/drivers/gpu/drm/drm_bridge.c
>> @@ -115,6 +115,9 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
>> if (!encoder || !bridge)
>> return -EINVAL;
>>
>> + if (WARN_ON(!bridge->owner))
>> + return -EINVAL;
>
> I think conceptually this is checked at the wrong place, and I think also misnamed
> a bit. The ->owner is essentially the struct device (and its associated
> driver) that provides the drm_bridge. As such it should be filled out
> already at drm_bridge_add() time, and I think the check should be in
> there. For driver-internal bridges it might make sense to also check this
> here, not sure. Or just require all bridges get added.
The reason for the position is that while I originally had the WARN in
drm_bridge_add, I found that quite a few bridges never call drm_bridge_add.
So I moved it. Other options are to start requiring all bridge suppliers to
call drm_bridge_add or to have the WARN in both function. Too me, it would
make sense to require all bridge suppliers to call drm_bridge_add, as that
enables other init stuff later, when needed. But that is a hairy patch to
get right, and is probably best left as a separate series.
> Wrt the name, I think we should call this pdev or something. ->owner
> usually means the module owner. I think in other subsystems ->dev is used,
> but in drm we use ->dev for the drm_device pointer, so totally different
> thing. pdev = physical device is the best I came up with. Better
> suggestions very much welcome.
pdev is about as problematic as owner. To me it reads "platform device".
And dev for a drm_device is also somewhat problematic, and I think that
drm would have been better, but dev for drm_device is probably quite
common. But one way to go is to rename the current dev to drm, so that
dev is freed up for the owner/supplier device. But that is a tedious
patch to write (I don't do the cocci thing).
Other suggestions I can think of: odev for owner device, sdev for supplier
device or just plain supplier.
Cheers,
Peter
> -Daniel
>
>> +
>> if (previous && (!previous->dev || previous->encoder != encoder))
>> return -EINVAL;
>>
>> --
>> 2.11.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 24/24] drm/bridge: establish a link between the bridge supplier and consumer
2018-04-26 22:31 ` [PATCH 00/24] device link, bridge supplier <-> drm device Peter Rosin
` (4 preceding siblings ...)
2018-04-26 22:31 ` [PATCH 23/24] drm/bridge: require the .owner to be filled in on drm_bridge_attach Peter Rosin
@ 2018-04-26 22:31 ` Peter Rosin
2018-04-30 15:32 ` Daniel Vetter
[not found] ` <1881012.XPHlTWYXH6@avalon>
2018-04-27 7:11 ` Andrzej Hajda
7 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2018-04-26 22:31 UTC (permalink / raw)
To: linux-arm-kernel
If the bridge supplier is unbound, this will bring the bridge consumer
down along with the bridge. Thus, there will no longer linger any
dangling pointers from the bridge consumer (the drm_device) to some
non-existent bridge supplier.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/drm_bridge.c | 18 ++++++++++++++++++
include/drm/drm_bridge.h | 2 ++
2 files changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index a038da696802..f0c79043ec43 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -26,6 +26,7 @@
#include <linux/mutex.h>
#include <drm/drm_bridge.h>
+#include <drm/drm_device.h>
#include <drm/drm_encoder.h>
#include "drm_crtc_internal.h"
@@ -124,12 +125,25 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
if (bridge->dev)
return -EBUSY;
+ if (encoder->dev->dev != bridge->owner) {
+ bridge->link = device_link_add(encoder->dev->dev,
+ bridge->owner, 0);
+ if (!bridge->link) {
+ dev_err(bridge->owner, "failed to link bridge to %s\n",
+ dev_name(encoder->dev->dev));
+ return -EINVAL;
+ }
+ }
+
bridge->dev = encoder->dev;
bridge->encoder = encoder;
if (bridge->funcs->attach) {
ret = bridge->funcs->attach(bridge);
if (ret < 0) {
+ if (bridge->link)
+ device_link_del(bridge->link);
+ bridge->link = NULL;
bridge->dev = NULL;
bridge->encoder = NULL;
return ret;
@@ -156,6 +170,10 @@ void drm_bridge_detach(struct drm_bridge *bridge)
if (bridge->funcs->detach)
bridge->funcs->detach(bridge);
+ if (bridge->link)
+ device_link_del(bridge->link);
+ bridge->link = NULL;
+
bridge->dev = NULL;
}
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 3bc659f3e7d2..9a386559a41a 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -261,6 +261,7 @@ struct drm_bridge_timings {
* @list: to keep track of all added bridges
* @timings: the timing specification for the bridge, if any (may
* be NULL)
+ * @link: drm consumer <-> bridge supplier
* @funcs: control functions
* @driver_private: pointer to the bridge driver's internal context
*/
@@ -271,6 +272,7 @@ struct drm_bridge {
struct drm_bridge *next;
struct list_head list;
const struct drm_bridge_timings *timings;
+ struct device_link *link;
const struct drm_bridge_funcs *funcs;
void *driver_private;
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 24/24] drm/bridge: establish a link between the bridge supplier and consumer
2018-04-26 22:31 ` [PATCH 24/24] drm/bridge: establish a link between the bridge supplier and consumer Peter Rosin
@ 2018-04-30 15:32 ` Daniel Vetter
2018-04-30 21:12 ` Peter Rosin
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2018-04-30 15:32 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 27, 2018 at 12:31:39AM +0200, Peter Rosin wrote:
> If the bridge supplier is unbound, this will bring the bridge consumer
> down along with the bridge. Thus, there will no longer linger any
> dangling pointers from the bridge consumer (the drm_device) to some
> non-existent bridge supplier.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
Minus the ->owner bikeshed I brought up in the previous patch I agree with
this approach as the best way to move forward for now.
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
One small suggestion below, for merging I'd say pls get Jyri's
review/tested-by too, since you're both working on the same problem it
seems.
Aside: Do you want commit rights to drm-misc to be able to push work like
this?
Cheers, Daniel
> ---
> drivers/gpu/drm/drm_bridge.c | 18 ++++++++++++++++++
> include/drm/drm_bridge.h | 2 ++
> 2 files changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index a038da696802..f0c79043ec43 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -26,6 +26,7 @@
> #include <linux/mutex.h>
>
> #include <drm/drm_bridge.h>
> +#include <drm/drm_device.h>
> #include <drm/drm_encoder.h>
>
> #include "drm_crtc_internal.h"
> @@ -124,12 +125,25 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> if (bridge->dev)
> return -EBUSY;
>
> + if (encoder->dev->dev != bridge->owner) {
You might end up with a NULL encoder->dev->dev. Perhaps check that and
bail with a WARN_ON?
> + bridge->link = device_link_add(encoder->dev->dev,
> + bridge->owner, 0);
> + if (!bridge->link) {
> + dev_err(bridge->owner, "failed to link bridge to %s\n",
> + dev_name(encoder->dev->dev));
> + return -EINVAL;
> + }
> + }
> +
> bridge->dev = encoder->dev;
> bridge->encoder = encoder;
>
> if (bridge->funcs->attach) {
> ret = bridge->funcs->attach(bridge);
> if (ret < 0) {
> + if (bridge->link)
> + device_link_del(bridge->link);
> + bridge->link = NULL;
> bridge->dev = NULL;
> bridge->encoder = NULL;
> return ret;
> @@ -156,6 +170,10 @@ void drm_bridge_detach(struct drm_bridge *bridge)
> if (bridge->funcs->detach)
> bridge->funcs->detach(bridge);
>
> + if (bridge->link)
> + device_link_del(bridge->link);
> + bridge->link = NULL;
> +
> bridge->dev = NULL;
> }
>
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 3bc659f3e7d2..9a386559a41a 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -261,6 +261,7 @@ struct drm_bridge_timings {
> * @list: to keep track of all added bridges
> * @timings: the timing specification for the bridge, if any (may
> * be NULL)
> + * @link: drm consumer <-> bridge supplier
> * @funcs: control functions
> * @driver_private: pointer to the bridge driver's internal context
> */
> @@ -271,6 +272,7 @@ struct drm_bridge {
> struct drm_bridge *next;
> struct list_head list;
> const struct drm_bridge_timings *timings;
> + struct device_link *link;
>
> const struct drm_bridge_funcs *funcs;
> void *driver_private;
> --
> 2.11.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 24/24] drm/bridge: establish a link between the bridge supplier and consumer
2018-04-30 15:32 ` Daniel Vetter
@ 2018-04-30 21:12 ` Peter Rosin
0 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2018-04-30 21:12 UTC (permalink / raw)
To: linux-arm-kernel
On 2018-04-30 17:32, Daniel Vetter wrote:
> On Fri, Apr 27, 2018 at 12:31:39AM +0200, Peter Rosin wrote:
>> If the bridge supplier is unbound, this will bring the bridge consumer
>> down along with the bridge. Thus, there will no longer linger any
>> dangling pointers from the bridge consumer (the drm_device) to some
>> non-existent bridge supplier.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>
> Minus the ->owner bikeshed I brought up in the previous patch I agree with
> this approach as the best way to move forward for now.
>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Thanks, let's see if Laurent is also on-board...
> One small suggestion below, for merging I'd say pls get Jyri's
> review/tested-by too, since you're both working on the same problem it
> seems.
Yes, I too would be very happy to see a tested-by from someone.
> Aside: Do you want commit rights to drm-misc to be able to push work like
> this?
If that makes life easier, sure.
Cheers,
Peter
^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <1881012.XPHlTWYXH6@avalon>]
* [PATCH 00/24] device link, bridge supplier <-> drm device
[not found] ` <1881012.XPHlTWYXH6@avalon>
@ 2018-04-26 23:09 ` Peter Rosin
[not found] ` <3483427.EaMxDYJt8U@avalon>
2018-04-30 15:18 ` Daniel Vetter
1 sibling, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2018-04-26 23:09 UTC (permalink / raw)
To: linux-arm-kernel
On 2018-04-27 00:40, Laurent Pinchart wrote:
> Hi Peter,
>
> Thank you for the patches.
>
> On Friday, 27 April 2018 01:31:15 EEST Peter Rosin wrote:
>> Hi!
>>
>> It was noted by Russel King [1] that bridges (not using components)
>> might disappear unexpectedly if the owner of the bridge was unbound.
>> Jyri Sarha had previously noted the same thing with panels [2]. Jyri
>> came up with using device links to resolve the panel issue, which
>> was also my (independent) reaction to the note from Russel.
>>
>> This series builds up to the addition of that link in the last
>> patch, but in my opinion the other 23 patches do have merit on their
>> own.
>>
>> The last patch needs testing, while the others look trivial. That
>> said, I might have missed some subtlety.
>
> I don't think this is the way to go. We have a real lifetime management
> problem with bridges, and device links are just trying to hide the problem
> under the carpet. They will further corner us by making a real fix much more
> difficult to implement. I'll try to comment further in the next few days on
> what I think a better solution would be, but in a nutshell I believe that
> drm_bridge objects need to be refcounted, with a .release() operation to free
> the bridge resources when the reference count drops to zero. This shouldn't be
> difficult to implement and I'm willing to help.
Ok, sp 24/24 is dead, and maybe 23/24 too. But how do you feel about dropping
.of_node in favour of .owner? That gets rid of ugly #ifdefs...
I also have the nagging feeling that .driver_private serves very little real
purpose if there is a .owner so that you can do
dev_get_drvdata(bridge->owner)
for the cases where the container_of macro is not appropriate.
Cheers,
Peter
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 00/24] device link, bridge supplier <-> drm device
[not found] ` <1881012.XPHlTWYXH6@avalon>
2018-04-26 23:09 ` [PATCH 00/24] device link, bridge supplier <-> drm device Peter Rosin
@ 2018-04-30 15:18 ` Daniel Vetter
1 sibling, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2018-04-30 15:18 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 27, 2018 at 01:40:21AM +0300, Laurent Pinchart wrote:
> Hi Peter,
>
> Thank you for the patches.
>
> On Friday, 27 April 2018 01:31:15 EEST Peter Rosin wrote:
> > Hi!
> >
> > It was noted by Russel King [1] that bridges (not using components)
> > might disappear unexpectedly if the owner of the bridge was unbound.
> > Jyri Sarha had previously noted the same thing with panels [2]. Jyri
> > came up with using device links to resolve the panel issue, which
> > was also my (independent) reaction to the note from Russel.
> >
> > This series builds up to the addition of that link in the last
> > patch, but in my opinion the other 23 patches do have merit on their
> > own.
> >
> > The last patch needs testing, while the others look trivial. That
> > said, I might have missed some subtlety.
>
> I don't think this is the way to go. We have a real lifetime management
> problem with bridges, and device links are just trying to hide the problem
> under the carpet. They will further corner us by making a real fix much more
> difficult to implement. I'll try to comment further in the next few days on
> what I think a better solution would be, but in a nutshell I believe that
> drm_bridge objects need to be refcounted, with a .release() operation to free
> the bridge resources when the reference count drops to zero. This shouldn't be
> difficult to implement and I'm willing to help.
I agree that refcounting is the proper fix if bridges can be
hot-unplugged, independently of the overall drm_device.
And yes retro-shoehorning refcounting is "fun", but it's also not
impossible. I've done it a few times by now in drm.
Otoh, refcounting has a pretty serious cost - we're still blowing up
drm_framebuffer refcounting in corner cases at shocking regularity, and
that's something that's been refcounted for years by now. As long as we
don't have a clearly defined need to make bridges hotpluggable then I
think not paying the hefty bill just yet is the correct technical
decision. This patch series here looks like a reasonable way to tie the
lifetimes together a bit more closely and should fix the bugs we have
right now.
Cheers, Daniel
PS: Yes there's cases where you can hotplug display blocks on a SoC. To my
knowledge they're all covered by hotplugging the entire drm_device (plus
all the statically associated bits like drm_bridge/crtc/plane/encoder).
And we're already working on fixing up drm_device to at least make it
possible to write a correct hot-unpluggable driver in theory. We'll see
whether anyone manages to pull that off in practice :-)
>
> > [1] https://lkml.org/lkml/2018/4/23/769
> > [2] https://www.spinics.net/lists/dri-devel/msg174275.html
> >
> > Peter Rosin (24):
> > drm/bridge: allow optionally specifying an .owner device
> > drm/bridge: adv7511: provide an .owner device
> > drm/bridge/analogix: core: specify the .owner of the bridge
> > drm/bridge: analogix-anx78xx: provide an .owner device
> > drm/bridge: vga-dac: provide an .owner device
> > drm/bridge: lvds-encoder: provide an .owner device
> > drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: provide an .owner device
> > drm/bridge: nxp-ptn3460: provide an .owner device
> > drm/bridge: panel: provide an .owner device
> > drm/bridge: ps8622: provide an .owner device
> > drm/bridge: sii902x: provide an .owner device
> > drm/bridge: sii9234: provide an .owner device
> > drm/bridge: sii8620: provide an .owner device
> > drm/bridge: synopsys: provide an .owner device for the bridges
> > drm/bridge: tc358767: provide an .owner device
> > drm/bridge: ti-tfp410: provide an .owner device
> > drm/exynos: mic: provide an .owner device for the bridge
> > drm/mediatek: hdmi: provide an .owner device for the bridge
> > drm/msm: specify the .owner of the bridges
> > drm/rcar-du: lvds: provide an .owner device for the bridge
> > drm/sti: provide an .owner device for the bridges
> > drm/bridge: remove the .of_node member
> > drm/bridge: require the .owner to be filled in on drm_bridge_attach
> > drm/bridge: establish a link between the bridge supplier and consumer
> >
> > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
> > drivers/gpu/drm/bridge/analogix-anx78xx.c | 5 +----
> > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 +
> > drivers/gpu/drm/bridge/dumb-vga-dac.c | 2 +-
> > drivers/gpu/drm/bridge/lvds-encoder.c | 2 +-
> > .../drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c | 2 +-
> > drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +-
> > drivers/gpu/drm/bridge/panel.c | 4 +---
> > drivers/gpu/drm/bridge/parade-ps8622.c | 2 +-
> > drivers/gpu/drm/bridge/sii902x.c | 2 +-
> > drivers/gpu/drm/bridge/sii9234.c | 2 +-
> > drivers/gpu/drm/bridge/sil-sii8620.c | 2 +-
> > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 +---
> > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 4 +---
> > drivers/gpu/drm/bridge/tc358767.c | 2 +-
> > drivers/gpu/drm/bridge/ti-tfp410.c | 2 +-
> > drivers/gpu/drm/drm_bridge.c | 23 ++++++++++++++++++-
> > drivers/gpu/drm/exynos/exynos_drm_mic.c | 2 +-
> > drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> > drivers/gpu/drm/msm/dsi/dsi_manager.c | 1 +
> > drivers/gpu/drm/msm/edp/edp_bridge.c | 1 +
> > drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 1 +
> > drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +-
> > drivers/gpu/drm/sti/sti_dvo.c | 2 +-
> > drivers/gpu/drm/sti/sti_hda.c | 1 +
> > drivers/gpu/drm/sti/sti_hdmi.c | 1 +
> > include/drm/drm_bridge.h | 8 ++++----
> > 27 files changed, 51 insertions(+), 33 deletions(-)
>
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 00/24] device link, bridge supplier <-> drm device
2018-04-26 22:31 ` [PATCH 00/24] device link, bridge supplier <-> drm device Peter Rosin
` (6 preceding siblings ...)
[not found] ` <1881012.XPHlTWYXH6@avalon>
@ 2018-04-27 7:11 ` Andrzej Hajda
2018-04-27 7:37 ` Peter Rosin
7 siblings, 1 reply; 19+ messages in thread
From: Andrzej Hajda @ 2018-04-27 7:11 UTC (permalink / raw)
To: linux-arm-kernel
Hi Peter,
On 27.04.2018 00:31, Peter Rosin wrote:
> Hi!
>
> It was noted by Russel King [1] that bridges (not using components)
> might disappear unexpectedly if the owner of the bridge was unbound.
> Jyri Sarha had previously noted the same thing with panels [2]. Jyri
> came up with using device links to resolve the panel issue, which
> was also my (independent) reaction to the note from Russel.
>
> This series builds up to the addition of that link in the last
> patch, but in my opinion the other 23 patches do have merit on their
> own.
>
> The last patch needs testing, while the others look trivial. That
> said, I might have missed some subtlety.
of_node is used as an identifier of the bridge in the kernel. If you
replace it with device pointer there will be potential problem with
devices having two or more bridges, how do you differentiate bridges if
the owner is the same? If I remember correctly current bridge code does
not allow to have multiple bridges in one device, but that should be
quite easy to fix if necessary. After this change it will become more
difficult.
Anyway I remember discussion that in DT world bridge should be
identified rather by of_graph port node, not by parent node as it is
now. If you want to translate this relation to device owner, you should
add also port number to have full identification of the bridge, ie pair
(owner, port_number) would be equivalent of port node.
Regards
Andrzej
>
> Cheers,
> Peter
>
> [1] https://lkml.org/lkml/2018/4/23/769
> [2] https://www.spinics.net/lists/dri-devel/msg174275.html
>
> Peter Rosin (24):
> drm/bridge: allow optionally specifying an .owner device
> drm/bridge: adv7511: provide an .owner device
> drm/bridge/analogix: core: specify the .owner of the bridge
> drm/bridge: analogix-anx78xx: provide an .owner device
> drm/bridge: vga-dac: provide an .owner device
> drm/bridge: lvds-encoder: provide an .owner device
> drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: provide an .owner device
> drm/bridge: nxp-ptn3460: provide an .owner device
> drm/bridge: panel: provide an .owner device
> drm/bridge: ps8622: provide an .owner device
> drm/bridge: sii902x: provide an .owner device
> drm/bridge: sii9234: provide an .owner device
> drm/bridge: sii8620: provide an .owner device
> drm/bridge: synopsys: provide an .owner device for the bridges
> drm/bridge: tc358767: provide an .owner device
> drm/bridge: ti-tfp410: provide an .owner device
> drm/exynos: mic: provide an .owner device for the bridge
> drm/mediatek: hdmi: provide an .owner device for the bridge
> drm/msm: specify the .owner of the bridges
> drm/rcar-du: lvds: provide an .owner device for the bridge
> drm/sti: provide an .owner device for the bridges
> drm/bridge: remove the .of_node member
> drm/bridge: require the .owner to be filled in on drm_bridge_attach
> drm/bridge: establish a link between the bridge supplier and consumer
>
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
> drivers/gpu/drm/bridge/analogix-anx78xx.c | 5 +----
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 +
> drivers/gpu/drm/bridge/dumb-vga-dac.c | 2 +-
> drivers/gpu/drm/bridge/lvds-encoder.c | 2 +-
> .../drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c | 2 +-
> drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +-
> drivers/gpu/drm/bridge/panel.c | 4 +---
> drivers/gpu/drm/bridge/parade-ps8622.c | 2 +-
> drivers/gpu/drm/bridge/sii902x.c | 2 +-
> drivers/gpu/drm/bridge/sii9234.c | 2 +-
> drivers/gpu/drm/bridge/sil-sii8620.c | 2 +-
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 +---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 4 +---
> drivers/gpu/drm/bridge/tc358767.c | 2 +-
> drivers/gpu/drm/bridge/ti-tfp410.c | 2 +-
> drivers/gpu/drm/drm_bridge.c | 23 +++++++++++++++++++++-
> drivers/gpu/drm/exynos/exynos_drm_mic.c | 2 +-
> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> drivers/gpu/drm/msm/dsi/dsi_manager.c | 1 +
> drivers/gpu/drm/msm/edp/edp_bridge.c | 1 +
> drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 1 +
> drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +-
> drivers/gpu/drm/sti/sti_dvo.c | 2 +-
> drivers/gpu/drm/sti/sti_hda.c | 1 +
> drivers/gpu/drm/sti/sti_hdmi.c | 1 +
> include/drm/drm_bridge.h | 8 ++++----
> 27 files changed, 51 insertions(+), 33 deletions(-)
>
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 00/24] device link, bridge supplier <-> drm device
2018-04-27 7:11 ` Andrzej Hajda
@ 2018-04-27 7:37 ` Peter Rosin
2018-04-27 7:51 ` Peter Rosin
0 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2018-04-27 7:37 UTC (permalink / raw)
To: linux-arm-kernel
On 2018-04-27 09:11, Andrzej Hajda wrote:
> Hi Peter,
>
> On 27.04.2018 00:31, Peter Rosin wrote:
>> Hi!
>>
>> It was noted by Russel King [1] that bridges (not using components)
>> might disappear unexpectedly if the owner of the bridge was unbound.
>> Jyri Sarha had previously noted the same thing with panels [2]. Jyri
>> came up with using device links to resolve the panel issue, which
>> was also my (independent) reaction to the note from Russel.
>>
>> This series builds up to the addition of that link in the last
>> patch, but in my opinion the other 23 patches do have merit on their
>> own.
>>
>> The last patch needs testing, while the others look trivial. That
>> said, I might have missed some subtlety.
>
> of_node is used as an identifier of the bridge in the kernel. If you
> replace it with device pointer there will be potential problem with
> devices having two or more bridges, how do you differentiate bridges if
> the owner is the same? If I remember correctly current bridge code does
> not allow to have multiple bridges in one device, but that should be
> quite easy to fix if necessary. After this change it will become more
> difficult.
I don't see how it will be more difficult?
> Anyway I remember discussion that in DT world bridge should be
> identified rather by of_graph port node, not by parent node as it is
> now. If you want to translate this relation to device owner, you should
> add also port number to have full identification of the bridge, ie pair
> (owner, port_number) would be equivalent of port node.
You even state the trivial solution here, just add the port/endpoint ID
when/if it is needed. So, what is the significant difference?
Cheers,
Peter
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 00/24] device link, bridge supplier <-> drm device
2018-04-27 7:37 ` Peter Rosin
@ 2018-04-27 7:51 ` Peter Rosin
0 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2018-04-27 7:51 UTC (permalink / raw)
To: linux-arm-kernel
On 2018-04-27 09:37, Peter Rosin wrote:
> On 2018-04-27 09:11, Andrzej Hajda wrote:
>> Hi Peter,
>>
>> On 27.04.2018 00:31, Peter Rosin wrote:
>>> Hi!
>>>
>>> It was noted by Russel King [1] that bridges (not using components)
>>> might disappear unexpectedly if the owner of the bridge was unbound.
>>> Jyri Sarha had previously noted the same thing with panels [2]. Jyri
>>> came up with using device links to resolve the panel issue, which
>>> was also my (independent) reaction to the note from Russel.
>>>
>>> This series builds up to the addition of that link in the last
>>> patch, but in my opinion the other 23 patches do have merit on their
>>> own.
>>>
>>> The last patch needs testing, while the others look trivial. That
>>> said, I might have missed some subtlety.
>>
>> of_node is used as an identifier of the bridge in the kernel. If you
>> replace it with device pointer there will be potential problem with
>> devices having two or more bridges, how do you differentiate bridges if
>> the owner is the same? If I remember correctly current bridge code does
>> not allow to have multiple bridges in one device, but that should be
>> quite easy to fix if necessary. After this change it will become more
>> difficult.
>
> I don't see how it will be more difficult?
>
>> Anyway I remember discussion that in DT world bridge should be
>> identified rather by of_graph port node, not by parent node as it is
>> now. If you want to translate this relation to device owner, you should
>> add also port number to have full identification of the bridge, ie pair
>> (owner, port_number) would be equivalent of port node.
>
> You even state the trivial solution here, just add the port/endpoint ID
> when/if it is needed. So, what is the significant difference?
Or, since this is apparently a rare requirement, you could make the owners
that do need it fix it themselves. E.g. by embedding the struct drm_bridge
in another struct that contains the needed ID, and use container_of to get
to that containing struct with the ID.
Cheers,
Peter
^ permalink raw reply [flat|nested] 19+ messages in thread