Hello,
This series adds initial support for the DRM bridges to NVIDIA Tegra DRM driver. This is required by newer device-trees where we model the LVDS encoder bridge properly.
Changelog:
v9: - Dropped the of-graph/drm-of patches from this series because they are now factored out into a standalone series [1].
[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=186813
- The "drm/panel-simple: Add missing connector type for some panels" patch of v8 was already applied.
v8: - The new of_graph_get_local_port() helper is replaced with the of_graph_presents(), which simply checks the graph presence in a given DT node. Thank to Laurent Pinchart for the suggestion!
- The of_graph_get_local_port() is still there, but now it isn't a public function anymore. In the review to v7 Laurent Pinchart suggested that the function's doc-comments and name could be improved and I implemented these suggestions in v8.
- A day ago I discovered that devm_drm_panel_bridge_add() requires panel to have connector type to be properly set, otherwise function rejects panels with the incomplete description. So, I checked what LVDS panels are used on Tegra and fixed the missing connector types in this new patch:
drm/panel-simple: Add missing connector type for some panels
v7: - Removed the obscure unused structs (which GCC doesn't detect, but CLANG does) in the patch "Wrap directly-connected panel into DRM bridge", which was reported by kernel test robot for v6.
v6: - Added r-b and acks from Rob Herring and Sam Ravnborg.
- Rebased on a recent linux-next, patches now apply without fuzz.
v5: - Added new patches that make drm_of_find_panel_or_bridge() more usable if graph isn't defined in a device-tree:
of_graph: add of_graph_get_local_port() drm/of: Make drm_of_find_panel_or_bridge() to check graph's presence
- Updated "Support DRM bridges" patch to use drm_of_find_panel_or_bridge() directly and added WARN_ON(output->panel || output->bridge) sanity-check.
- Added new "Wrap directly-connected panel into DRM bridge" patch, as was suggested by Laurent Pinchart.
v4: - Following review comments that were made by Laurent Pinchart to the v3, we now create and use the "bridge connector".
v3: - Following recommendation from Sam Ravnborg, the new bridge attachment model is now being used, i.e. we ask bridge to *not* create a connector using the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.
- The bridge is now created only for the RGB (LVDS) output, and only when necessary. For now we don't need bridges for HDMI or DSI outputs.
- I noticed that we're leaking OF node in the panel's error code path, this is fixed now by the new patch "Don't leak OF node on error".
v2: - Added the new "rgb: Don't register connector if bridge is used" patch, which hides the unused connector provided by the Tegra DRM driver when bridge is used, since bridge provides its own connector to us.
Dmitry Osipenko (4): drm/tegra: output: Don't leak OF node on error drm/tegra: output: Support DRM bridges drm/tegra: output: rgb: Support LVDS encoder bridge drm/tegra: output: rgb: Wrap directly-connected panel into DRM bridge
drivers/gpu/drm/tegra/drm.h | 2 + drivers/gpu/drm/tegra/output.c | 21 +++++-- drivers/gpu/drm/tegra/rgb.c | 102 +++++++++++++++++---------------- 3 files changed, 72 insertions(+), 53 deletions(-)
The OF node should be put before returning error in tegra_output_probe(), otherwise node's refcount will be leaked.
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Reviewed-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Dmitry Osipenko digetx@gmail.com --- drivers/gpu/drm/tegra/output.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c index e36e5e7c2f69..a6a711d54e88 100644 --- a/drivers/gpu/drm/tegra/output.c +++ b/drivers/gpu/drm/tegra/output.c @@ -102,10 +102,10 @@ int tegra_output_probe(struct tegra_output *output) panel = of_parse_phandle(output->of_node, "nvidia,panel", 0); if (panel) { output->panel = of_drm_find_panel(panel); + of_node_put(panel); + if (IS_ERR(output->panel)) return PTR_ERR(output->panel); - - of_node_put(panel); }
output->edid = of_get_property(output->of_node, "nvidia,edid", &size); @@ -113,13 +113,12 @@ int tegra_output_probe(struct tegra_output *output) ddc = of_parse_phandle(output->of_node, "nvidia,ddc-i2c-bus", 0); if (ddc) { output->ddc = of_find_i2c_adapter_by_node(ddc); + of_node_put(ddc); + if (!output->ddc) { err = -EPROBE_DEFER; - of_node_put(ddc); return err; } - - of_node_put(ddc); }
output->hpd_gpio = devm_gpiod_get_from_of_node(output->dev,
Newer Tegra device-trees will specify a video output graph which involves a bridge. This patch adds initial support for the DRM bridges to the Tegra DRM output.
Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Dmitry Osipenko digetx@gmail.com --- drivers/gpu/drm/tegra/drm.h | 2 ++ drivers/gpu/drm/tegra/output.c | 12 ++++++++++++ 2 files changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h index b25443255be6..f38de08e0c95 100644 --- a/drivers/gpu/drm/tegra/drm.h +++ b/drivers/gpu/drm/tegra/drm.h @@ -12,6 +12,7 @@ #include <linux/gpio/consumer.h>
#include <drm/drm_atomic.h> +#include <drm/drm_bridge.h> #include <drm/drm_edid.h> #include <drm/drm_encoder.h> #include <drm/drm_fb_helper.h> @@ -116,6 +117,7 @@ struct tegra_output { struct device_node *of_node; struct device *dev;
+ struct drm_bridge *bridge; struct drm_panel *panel; struct i2c_adapter *ddc; const struct edid *edid; diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c index a6a711d54e88..ccd1421f1b24 100644 --- a/drivers/gpu/drm/tegra/output.c +++ b/drivers/gpu/drm/tegra/output.c @@ -5,6 +5,7 @@ */
#include <drm/drm_atomic_helper.h> +#include <drm/drm_of.h> #include <drm/drm_panel.h> #include <drm/drm_simple_kms_helper.h>
@@ -99,8 +100,19 @@ int tegra_output_probe(struct tegra_output *output) if (!output->of_node) output->of_node = output->dev->of_node;
+ err = drm_of_find_panel_or_bridge(output->of_node, -1, -1, + &output->panel, &output->bridge); + if (err && err != -ENODEV) + return err; + panel = of_parse_phandle(output->of_node, "nvidia,panel", 0); if (panel) { + /* + * Don't mix nvidia,panel phandle with the graph in a + * device-tree. + */ + WARN_ON(output->panel || output->bridge); + output->panel = of_drm_find_panel(panel); of_node_put(panel);
Newer Tegra device-trees will specify a video output graph, which involves LVDS encoder bridge. This patch adds support for the LVDS encoder bridge to the RGB output, allowing us to model the display hardware properly.
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Dmitry Osipenko digetx@gmail.com --- drivers/gpu/drm/tegra/rgb.c | 58 +++++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c index 0562a7eb793f..9a7024ec96bc 100644 --- a/drivers/gpu/drm/tegra/rgb.c +++ b/drivers/gpu/drm/tegra/rgb.c @@ -7,6 +7,7 @@ #include <linux/clk.h>
#include <drm/drm_atomic_helper.h> +#include <drm/drm_bridge_connector.h> #include <drm/drm_panel.h> #include <drm/drm_simple_kms_helper.h>
@@ -267,24 +268,63 @@ int tegra_dc_rgb_remove(struct tegra_dc *dc) int tegra_dc_rgb_init(struct drm_device *drm, struct tegra_dc *dc) { struct tegra_output *output = dc->rgb; + struct drm_connector *connector; int err;
if (!dc->rgb) return -ENODEV;
- drm_connector_init(drm, &output->connector, &tegra_rgb_connector_funcs, - DRM_MODE_CONNECTOR_LVDS); - drm_connector_helper_add(&output->connector, - &tegra_rgb_connector_helper_funcs); - output->connector.dpms = DRM_MODE_DPMS_OFF; - drm_simple_encoder_init(drm, &output->encoder, DRM_MODE_ENCODER_LVDS); drm_encoder_helper_add(&output->encoder, &tegra_rgb_encoder_helper_funcs);
- drm_connector_attach_encoder(&output->connector, - &output->encoder); - drm_connector_register(&output->connector); + /* + * Tegra devices that have LVDS panel utilize LVDS encoder bridge + * for converting up to 28 LCD LVTTL lanes into 5/4 LVDS lanes that + * go to display panel's receiver. + * + * Encoder usually have a power-down control which needs to be enabled + * in order to transmit data to the panel. Historically devices that + * use an older device-tree version didn't model the bridge, assuming + * that encoder is turned ON by default, while today's DRM allows us + * to model LVDS encoder properly. + * + * Newer device-trees utilize LVDS encoder bridge, which provides + * us with a connector and handles the display panel. + * + * For older device-trees we fall back to our own connector and use + * nvidia,panel phandle. + */ + if (output->bridge) { + err = drm_bridge_attach(&output->encoder, output->bridge, + NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR); + if (err) { + dev_err(output->dev, "failed to attach bridge: %d\n", + err); + return err; + } + + connector = drm_bridge_connector_init(drm, &output->encoder); + if (IS_ERR(connector)) { + dev_err(output->dev, + "failed to initialize bridge connector: %pe\n", + connector); + return PTR_ERR(connector); + } + + drm_connector_attach_encoder(connector, &output->encoder); + } else { + drm_connector_init(drm, &output->connector, + &tegra_rgb_connector_funcs, + DRM_MODE_CONNECTOR_LVDS); + drm_connector_helper_add(&output->connector, + &tegra_rgb_connector_helper_funcs); + output->connector.dpms = DRM_MODE_DPMS_OFF; + + drm_connector_attach_encoder(&output->connector, + &output->encoder); + drm_connector_register(&output->connector); + }
err = tegra_output_init(drm, output); if (err < 0) {
Currently Tegra DRM driver manually manages display panel, but this management could be moved out into DRM core if we'll wrap panel into DRM bridge. This patch wraps RGB panel into a DRM bridge and removes manual handling of the panel from the RGB output code.
Suggested-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Dmitry Osipenko digetx@gmail.com --- drivers/gpu/drm/tegra/rgb.c | 70 ++++++++++--------------------------- 1 file changed, 18 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c index 9a7024ec96bc..4142a56ca764 100644 --- a/drivers/gpu/drm/tegra/rgb.c +++ b/drivers/gpu/drm/tegra/rgb.c @@ -8,7 +8,6 @@
#include <drm/drm_atomic_helper.h> #include <drm/drm_bridge_connector.h> -#include <drm/drm_panel.h> #include <drm/drm_simple_kms_helper.h>
#include "drm.h" @@ -86,45 +85,13 @@ static void tegra_dc_write_regs(struct tegra_dc *dc, tegra_dc_writel(dc, table[i].value, table[i].offset); }
-static const struct drm_connector_funcs tegra_rgb_connector_funcs = { - .reset = drm_atomic_helper_connector_reset, - .detect = tegra_output_connector_detect, - .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = tegra_output_connector_destroy, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, -}; - -static enum drm_mode_status -tegra_rgb_connector_mode_valid(struct drm_connector *connector, - struct drm_display_mode *mode) -{ - /* - * FIXME: For now, always assume that the mode is okay. There are - * unresolved issues with clk_round_rate(), which doesn't always - * reliably report whether a frequency can be set or not. - */ - return MODE_OK; -} - -static const struct drm_connector_helper_funcs tegra_rgb_connector_helper_funcs = { - .get_modes = tegra_output_connector_get_modes, - .mode_valid = tegra_rgb_connector_mode_valid, -}; - static void tegra_rgb_encoder_disable(struct drm_encoder *encoder) { struct tegra_output *output = encoder_to_output(encoder); struct tegra_rgb *rgb = to_rgb(output);
- if (output->panel) - drm_panel_disable(output->panel); - tegra_dc_write_regs(rgb->dc, rgb_disable, ARRAY_SIZE(rgb_disable)); tegra_dc_commit(rgb->dc); - - if (output->panel) - drm_panel_unprepare(output->panel); }
static void tegra_rgb_encoder_enable(struct drm_encoder *encoder) @@ -133,9 +100,6 @@ static void tegra_rgb_encoder_enable(struct drm_encoder *encoder) struct tegra_rgb *rgb = to_rgb(output); u32 value;
- if (output->panel) - drm_panel_prepare(output->panel); - tegra_dc_write_regs(rgb->dc, rgb_enable, ARRAY_SIZE(rgb_enable));
value = DE_SELECT_ACTIVE | DE_CONTROL_NORMAL; @@ -157,9 +121,6 @@ static void tegra_rgb_encoder_enable(struct drm_encoder *encoder) tegra_dc_writel(rgb->dc, value, DC_DISP_SHIFT_CLOCK_OPTIONS);
tegra_dc_commit(rgb->dc); - - if (output->panel) - drm_panel_enable(output->panel); }
static int @@ -278,6 +239,23 @@ int tegra_dc_rgb_init(struct drm_device *drm, struct tegra_dc *dc) drm_encoder_helper_add(&output->encoder, &tegra_rgb_encoder_helper_funcs);
+ /* + * Wrap directly-connected panel into DRM bridge in order to let + * DRM core to handle panel for us. + */ + if (output->panel) { + output->bridge = devm_drm_panel_bridge_add(output->dev, + output->panel); + if (IS_ERR(output->bridge)) { + dev_err(output->dev, + "failed to wrap panel into bridge: %pe\n", + output->bridge); + return PTR_ERR(output->bridge); + } + + output->panel = NULL; + } + /* * Tegra devices that have LVDS panel utilize LVDS encoder bridge * for converting up to 28 LCD LVTTL lanes into 5/4 LVDS lanes that @@ -292,8 +270,7 @@ int tegra_dc_rgb_init(struct drm_device *drm, struct tegra_dc *dc) * Newer device-trees utilize LVDS encoder bridge, which provides * us with a connector and handles the display panel. * - * For older device-trees we fall back to our own connector and use - * nvidia,panel phandle. + * For older device-trees we wrapped panel into the panel-bridge. */ if (output->bridge) { err = drm_bridge_attach(&output->encoder, output->bridge, @@ -313,17 +290,6 @@ int tegra_dc_rgb_init(struct drm_device *drm, struct tegra_dc *dc) }
drm_connector_attach_encoder(connector, &output->encoder); - } else { - drm_connector_init(drm, &output->connector, - &tegra_rgb_connector_funcs, - DRM_MODE_CONNECTOR_LVDS); - drm_connector_helper_add(&output->connector, - &tegra_rgb_connector_helper_funcs); - output->connector.dpms = DRM_MODE_DPMS_OFF; - - drm_connector_attach_encoder(&output->connector, - &output->encoder); - drm_connector_register(&output->connector); }
err = tegra_output_init(drm, output);
dri-devel@lists.freedesktop.org
-
Dmitry Osipenko