| Message ID | 12cd62cc9ea0f79d0f399c76bfa715125a0816ce.1479142062.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2016-11-14 17:54 GMT+01:00 Jyri Sarha <jsarha@ti.com>: > Adds drm bride support for attaching drm bridge drivers to tilcdc. The > decision whether a video port leads to an external encoder or bridge > is made simply based on remote device's compatible string. The code > has been tested with BeagleBone-Black with and without BeagleBone > DVI-D Cape Rev A3 using ti-tfp410 driver. > > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- Hi Jyri, thanks a lot for doing this. One issue I see with this patch is that tilcdc doesn't seem to support deferred probe correctly (if modules are built-in). The following happens on my setup: The dump-vga-dac module is loaded first, but the i2c0 is not ready yet - probe returns EPROBE_DEFER and it's propagated to tilcdc probe. [drm] Initialized dumb-vga-dac vga_bridge: Couldn't retrieve i2c bus Then the i2c bus is initialized and dump-vga-dac probe succeeds, but the second probe of tilcdc gives me: [drm:drm_debugfs_init] *ERROR* Cannot create /sys/kernel/debug/dri/64 [drm:drm_minor_register] *ERROR* DRM: Failed to initialize /sys/kernel/debug/dri. tilcdc: probe of da8xx_lcdc.0 failed with error -1 I was able to work around this issue by loading modules in correct order. I then tried testing the patch with a da850-lcdk, but I don't get anything on the display (no signal), even though the LCDC seems to work fine (modetest and dmesg messages work just like when using the tilcdc panel). Also: I see the EDID info is correctly retrieved from the display. Could you take a look at my DT[1] and see if you find it correct? Best regards, Bartosz Golaszewski [1] http://pastebin.com/dfUX7PyL
On 11/15/16 19:36, Bartosz Golaszewski wrote: > 2016-11-14 17:54 GMT+01:00 Jyri Sarha <jsarha@ti.com>: >> Adds drm bride support for attaching drm bridge drivers to tilcdc. The >> decision whether a video port leads to an external encoder or bridge >> is made simply based on remote device's compatible string. The code >> has been tested with BeagleBone-Black with and without BeagleBone >> DVI-D Cape Rev A3 using ti-tfp410 driver. >> >> Signed-off-by: Jyri Sarha <jsarha@ti.com> >> --- > > Hi Jyri, > > thanks a lot for doing this. > > One issue I see with this patch is that tilcdc doesn't seem to support > deferred probe correctly (if modules are built-in). The following > happens on my setup: > > The dump-vga-dac module is loaded first, but the i2c0 is not ready yet > - probe returns EPROBE_DEFER and it's propagated to tilcdc probe. > > [drm] Initialized > dumb-vga-dac vga_bridge: Couldn't retrieve i2c bus > > Then the i2c bus is initialized and dump-vga-dac probe succeeds, but > the second probe of tilcdc gives me: > > [drm:drm_debugfs_init] *ERROR* Cannot create /sys/kernel/debug/dri/64 > [drm:drm_minor_register] *ERROR* DRM: Failed to initialize > /sys/kernel/debug/dri. > tilcdc: probe of da8xx_lcdc.0 failed with error -1 > > I was able to work around this issue by loading modules in correct order. > Did you have any conflicts when applying my patch? I have done quite a few changes lately and especially the initialization sequence and back off from deferred probe may get broken easily broken if the source base is not correct. I try to come up with a pull-request candidate branch soon (hopefully tomorrow) for you to test. > I then tried testing the patch with a da850-lcdk, but I don't get > anything on the display (no signal), even though the LCDC seems to > work fine (modetest and dmesg messages work just like when using the > tilcdc panel). Also: I see the EDID info is correctly retrieved from > the display. > > Could you take a look at my DT[1] and see if you find it correct? > It is hard to follow the dts diff, but if it probes and tilcdc is able to read EDID modes, there should not be anything more to it. Cheers, Jyri > Best regards, > Bartosz Golaszewski > > [1] http://pastebin.com/dfUX7PyL >
2016-11-15 21:46 GMT+01:00 Jyri Sarha <jsarha@ti.com>: > On 11/15/16 19:36, Bartosz Golaszewski wrote: >> 2016-11-14 17:54 GMT+01:00 Jyri Sarha <jsarha@ti.com>: >>> Adds drm bride support for attaching drm bridge drivers to tilcdc. The >>> decision whether a video port leads to an external encoder or bridge >>> is made simply based on remote device's compatible string. The code >>> has been tested with BeagleBone-Black with and without BeagleBone >>> DVI-D Cape Rev A3 using ti-tfp410 driver. >>> >>> Signed-off-by: Jyri Sarha <jsarha@ti.com> >>> --- >> >> Hi Jyri, >> >> thanks a lot for doing this. >> >> One issue I see with this patch is that tilcdc doesn't seem to support >> deferred probe correctly (if modules are built-in). The following >> happens on my setup: >> >> The dump-vga-dac module is loaded first, but the i2c0 is not ready yet >> - probe returns EPROBE_DEFER and it's propagated to tilcdc probe. >> >> [drm] Initialized >> dumb-vga-dac vga_bridge: Couldn't retrieve i2c bus >> >> Then the i2c bus is initialized and dump-vga-dac probe succeeds, but >> the second probe of tilcdc gives me: >> >> [drm:drm_debugfs_init] *ERROR* Cannot create /sys/kernel/debug/dri/64 >> [drm:drm_minor_register] *ERROR* DRM: Failed to initialize >> /sys/kernel/debug/dri. >> tilcdc: probe of da8xx_lcdc.0 failed with error -1 >> >> I was able to work around this issue by loading modules in correct order. >> > > Did you have any conflicts when applying my patch? I have done quite a > few changes lately and especially the initialization sequence and back > off from deferred probe may get broken easily broken if the source base > is not correct. I try to come up with a pull-request candidate branch > soon (hopefully tomorrow) for you to test. > I only had some trivial conflicts, but you're right, maybe I'm missing some parts. It would be great to have a branch for testing - I could then rebase my follow-up work on tilcdc against it. >> I then tried testing the patch with a da850-lcdk, but I don't get >> anything on the display (no signal), even though the LCDC seems to >> work fine (modetest and dmesg messages work just like when using the >> tilcdc panel). Also: I see the EDID info is correctly retrieved from >> the display. >> >> Could you take a look at my DT[1] and see if you find it correct? >> > > It is hard to follow the dts diff, but if it probes and tilcdc is able > to read EDID modes, there should not be anything more to it. > Yes, this is what I thought too. Let me know when you'll have the testing branch ready. Best regards, Bartosz Golaszewski
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index dcc9621..ec22576 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -384,9 +384,14 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) ret = tilcdc_add_external_encoders(ddev); if (ret < 0) goto init_failed; + } else { + ret = tilcdc_attach_remote_device(ddev); + if (ret) + goto init_failed; } - if ((priv->num_encoders == 0) || (priv->num_connectors == 0)) { + if (!priv->remote_encoder && + ((priv->num_encoders == 0) || (priv->num_connectors == 0))) { dev_err(dev, "no encoders/connectors found\n"); ret = -ENXIO; goto init_failed; diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h index d31fe5d..283ff28 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h @@ -90,6 +90,8 @@ struct tilcdc_drm_private { struct drm_connector *connectors[8]; const struct drm_connector_helper_funcs *connector_funcs[8]; + struct drm_encoder *remote_encoder; + bool is_registered; bool is_componentized; }; diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c index 06a4c58..e1576ba 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c @@ -28,6 +28,18 @@ .raster_order = 0, }; +static const struct tilcdc_panel_info panel_info_default = { + .ac_bias = 255, + .ac_bias_intrpt = 0, + .dma_burst_sz = 16, + .bpp = 16, + .fdd = 0x80, + .tft_alt_mode = 0, + .sync_edge = 0, + .sync_ctrl = 1, + .raster_order = 0, +}; + static int tilcdc_external_mode_valid(struct drm_connector *connector, struct drm_display_mode *mode) { @@ -130,6 +142,101 @@ void tilcdc_remove_external_encoders(struct drm_device *dev) priv->connector_funcs[i]); } +static const struct drm_encoder_funcs tilcdc_remote_encoder_funcs = { + .destroy = drm_encoder_cleanup, +}; + +static +int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge) +{ + struct tilcdc_drm_private *priv = ddev->dev_private; + int ret; + + priv->remote_encoder->possible_crtcs = BIT(0); + priv->remote_encoder->bridge = bridge; + bridge->encoder = priv->remote_encoder; + + ret = drm_bridge_attach(ddev, bridge); + if (ret) { + dev_err(ddev->dev, "drm_bridge_attach() failed %d\n", ret); + return ret; + } + + tilcdc_crtc_set_panel_info(priv->crtc, &panel_info_default); + + return 0; +} + +static int tilcdc_node_has_port(struct device_node *dev_node) +{ + struct device_node *node; + + node = of_get_child_by_name(dev_node, "ports"); + if (!node) + node = of_get_child_by_name(dev_node, "port"); + if (!node) + return 0; + of_node_put(node); + + return 1; +} + +static +struct device_node *tilcdc_get_remote_node(struct device_node *node) +{ + struct device_node *ep; + struct device_node *parent; + + if (!tilcdc_node_has_port(node)) + return NULL; + + ep = of_graph_get_next_endpoint(node, NULL); + if (!ep) + return NULL; + + parent = of_graph_get_remote_port_parent(ep); + of_node_put(ep); + + return parent; +} + +int tilcdc_attach_remote_device(struct drm_device *ddev) +{ + struct tilcdc_drm_private *priv = ddev->dev_private; + struct device_node *remote_node; + struct drm_bridge *bridge; + int ret; + + remote_node = tilcdc_get_remote_node(ddev->dev->of_node); + if (!remote_node) + return 0; + + bridge = of_drm_find_bridge(remote_node); + of_node_put(remote_node); + if (!bridge) + return -EPROBE_DEFER; + + priv->remote_encoder = devm_kzalloc(ddev->dev, + sizeof(*priv->remote_encoder), + GFP_KERNEL); + if (!priv->remote_encoder) + return -ENOMEM; + + ret = drm_encoder_init(ddev, priv->remote_encoder, + &tilcdc_remote_encoder_funcs, + DRM_MODE_ENCODER_NONE, NULL); + if (ret) { + dev_err(ddev->dev, "drm_encoder_init() failed %d\n", ret); + return ret; + } + + ret = tilcdc_attach_bridge(ddev, bridge); + if (ret) + drm_encoder_cleanup(priv->remote_encoder); + + return ret; +} + static int dev_match_of(struct device *dev, void *data) { return dev->of_node == data; @@ -141,16 +248,10 @@ int tilcdc_get_external_components(struct device *dev, struct device_node *node; struct device_node *ep = NULL; int count = 0; + int ret = 0; - /* Avoid error print by of_graph_get_next_endpoint() if there - * is no ports present. - */ - node = of_get_child_by_name(dev->of_node, "ports"); - if (!node) - node = of_get_child_by_name(dev->of_node, "port"); - if (!node) + if (!tilcdc_node_has_port(dev->of_node)) return 0; - of_node_put(node); while ((ep = of_graph_get_next_endpoint(dev->of_node, ep))) { node = of_graph_get_remote_port_parent(ep); @@ -160,17 +261,20 @@ int tilcdc_get_external_components(struct device *dev, } dev_dbg(dev, "Subdevice node '%s' found\n", node->name); - if (match) - drm_of_component_match_add(dev, match, dev_match_of, - node); - of_node_put(node); - count++; - } - if (count > 1) { - dev_err(dev, "Only one external encoder is supported\n"); - return -EINVAL; + if (of_device_is_compatible(node, "nxp,tda998x")) { + if (match) + drm_of_component_match_add(dev, match, + dev_match_of, node); + ret = 1; + } + + of_node_put(node); + if (count++ > 1) { + dev_err(dev, "Only one port is supported\n"); + return -EINVAL; + } } - return count; + return ret; } diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.h b/drivers/gpu/drm/tilcdc/tilcdc_external.h index c700e0c..a27c365 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_external.h +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.h @@ -22,4 +22,5 @@ void tilcdc_remove_external_encoders(struct drm_device *dev); int tilcdc_get_external_components(struct device *dev, struct component_match **match); +int tilcdc_attach_remote_device(struct drm_device *ddev); #endif /* __TILCDC_SLAVE_H__ */
Adds drm bride support for attaching drm bridge drivers to tilcdc. The decision whether a video port leads to an external encoder or bridge is made simply based on remote device's compatible string. The code has been tested with BeagleBone-Black with and without BeagleBone DVI-D Cape Rev A3 using ti-tfp410 driver. Signed-off-by: Jyri Sarha <jsarha@ti.com> --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 7 +- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 + drivers/gpu/drm/tilcdc/tilcdc_external.c | 140 +++++++++++++++++++++++++++---- drivers/gpu/drm/tilcdc/tilcdc_external.h | 1 + 4 files changed, 131 insertions(+), 19 deletions(-)