Support DRM bridges on NVIDIA Tegra
v4 cover Support DRM bridges on NVIDIA Tegra
Hello,
This small 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:
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.
- Please notice that the first "Support DRM bridges" patch was previously sent out as a standalone v1 change.
Dmitry Osipenko (3):
drm/tegra: output: Don't leak OF node on error drm/tegra: output: Support DRM bridges drm/tegra: output: rgb: Support LVDS encoder bridge
drivers/gpu/drm/tegra/drm.h | 2 ++ drivers/gpu/drm/tegra/output.c | 22 +++++++++---- drivers/gpu/drm/tegra/rgb.c | 58 ++++++++++++++++++++++++++++------ 3 files changed, 67 insertions(+), 15 deletions(-)
Signature · 1 line
2.26.0
PATCH v4 1/3 drm/tegra: output: Don't leak OF node on error
The OF node should be put before returning error in tegra_output_probe(), otherwise node's refcount will be leaked.
--- drivers/gpu/drm/tegra/output.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/gpu/drm/tegra/output.c +4 -5
@@ -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,
Signature · 1 line
2.26.0
PATCH v4 2/3 drm/tegra: output: Support DRM bridges
In reply to Dmitry OsipenkoNewer 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's DRM output.
--- drivers/gpu/drm/tegra/drm.h | 2 ++ drivers/gpu/drm/tegra/output.c | 13 ++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-)
drivers/gpu/drm/tegra/drm.h +2 -0
@@ -12,6 +12,7 @@#include <linux/of_gpio.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;
drivers/gpu/drm/tegra/output.c +12 -1
@@ -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>
@@ -92,13 +93,23 @@ static irqreturn_t hpd_irq(int irq, void *data)int tegra_output_probe (struct tegra_output *output) { -struct device_node *ddc, *panel; +struct device_node *ddc, *panel, *port;unsigned long flags;int err, size;if (!output->of_node) output->of_node = output->dev->of_node; + port =of_get_child_by_name (output->of_node,"port" ); +if (port) { + err =drm_of_find_panel_or_bridge (output->of_node,0 ,0 ,NULL , + &output->bridge); +of_node_put (port); + +if (err) +return err; + } + panel =of_parse_phandle (output->of_node,"nvidia,panel" ,0 );if (panel) { output->panel =of_drm_find_panel (panel);
Signature · 1 line
2.26.0
Re: [PATCH v4 2/3] drm/tegra: output: Support DRM bridges
Hi Dmitry,
Thank you for the patch.
Quoted 15 lines · Dmitry Osipenko
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's DRM output.
Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/gpu/drm/tegra/drm.h | 2 ++ drivers/gpu/drm/tegra/output.c | 13 ++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-)drivers/gpu/drm/tegra/drm.h +0
-0
drivers/gpu/drm/tegra/drm.h line 15
Quoted 4 lines
drivers/gpu/drm/tegra/drm.h +1
-0@@ -12,6 +12,7 @@#include <linux/of_gpio.h> #include <drm/drm_atomic.h> +#include <drm/drm_bridge.h>
You could add a forward declaration of struct drm_bridge instead, that can lower the compilation time a little bit.
Quoted 26 lines
drivers/gpu/drm/tegra/drm.h +1
-0#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;drivers/gpu/drm/tegra/output.c +1
-0@@ -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> @@ -92,13 +93,23 @@ static irqreturn_t hpd_irq(int irq, void *data)int tegra_output_probe (struct tegra_output *output)
drivers/gpu/drm/tegra/output.c static irqreturn_t hpd_irq(int irq, void *data) · line 103
Quoted 10 lines
drivers/gpu/drm/tegra/output.c +2
-1{ -struct device_node *ddc, *panel; +struct device_node *ddc, *panel, *port;unsigned long flags;int err, size;if (!output->of_node) output->of_node = output->dev->of_node; + port =of_get_child_by_name (output->of_node,"port" );
Do you need to check for the presence of a port node first ? Can you just check the return value of drm_of_find_panel_or_bridge(), and fall back to "nvidia,panel" if it returns -ENODEV ?
Quoted 12 lines
drivers/gpu/drm/tegra/output.c +9
-0+if (port) { + err =drm_of_find_panel_or_bridge (output->of_node,0 ,0 ,NULL , + &output->bridge); +of_node_put (port); + +if (err) +return err; + } + panel =of_parse_phandle (output->of_node,"nvidia,panel" ,0 );if (panel) { output->panel =of_drm_find_panel (panel);
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v4 2/3] drm/tegra: output: Support DRM bridges
Hello Laurent,
17.04.2020 22:30, Laurent Pinchart пишет:
...
Quoted 5 lines
Quoted 2 lines
#include <
drm/drm_atomic.h>
+#include <drm/drm_bridge.h>You could add a forward declaration of struct
drm_bridgeinstead, that can lower the compilation time a little bit.
This include is not only for the struct, but also for the drm_bridge_attach(). It looks to me that it should be nicer to keep the include here.
...
Quoted 5 lines
drivers/gpu/drm/tegra/output.c static irqreturn_t hpd_irq(int irq, void *data) · line 103
Quoted 1 line
drivers/gpu/drm/tegra/output.c +1
-0+ port =of_get_child_by_name (output->of_node,"port" );Do you need to check for the presence of a port node first ? Can you just check the return value of
drm_of_find_panel_or_bridge(), and fall back to "nvidia,panel" if it returns-ENODEV?
Without the check, the drm_of_find_panel_or_bridge() prints a very noisy error message about missing port node for every output that doesn't have a graph specified in a device-tree (HDMI, DSI and etc).
https://elixir.bootlin.com/linux/v5.7-rc1/source/drivers/of/property.c#L621
Re: [PATCH v4 2/3] drm/tegra: output: Support DRM bridges
Hi Dmitry,
Quoted 11 lines · Dmitry Osipenko
17.04.2020 22:30, Laurent Pinchart пишет:
...Quoted 5 lines
Quoted 2 lines
#include <
drm/drm_atomic.h>
+#include <drm/drm_bridge.h>You could add a forward declaration of struct
drm_bridgeinstead, that can lower the compilation time a little bit.This include is not only for the struct, but also for the
drm_bridge_attach(). It looks to me that it should be nicer to keep the include here.
drm_bridge_attach() is called from .c files. In the .h file you can use a forward declaration. It's entirely up to you, but as a general rule, I personally try to use forward structure declarations in .h files as much as possible.
Quoted 7 lines
...
Quoted 5 lines
drivers/gpu/drm/tegra/output.c static irqreturn_t hpd_irq(int irq, void *data) · line 103
Quoted 1 line
drivers/gpu/drm/tegra/output.c +1
-0+ port =of_get_child_by_name (output->of_node,"port" );Do you need to check for the presence of a port node first ? Can you just check the return value of
drm_of_find_panel_or_bridge(), and fall back to "nvidia,panel" if it returns-ENODEV?
Quoted 5 lines
Without the check, the
drm_of_find_panel_or_bridge()prints a very noisy error message about missing port node for every output that doesn't have a graph specified in a device-tree (HDMI, DSI and etc).https://elixir.bootlin.com/linux/v5.7-rc1/source/drivers/of/property.c#L621
Ah yes indeed. That's not very nice.
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v4 2/3] drm/tegra: output: Support DRM bridges
17.04.2020 23:31, Laurent Pinchart пишет:
Quoted 11 lines
Hi Dmitry,
Quoted 8 lines · Dmitry Osipenko
17.04.2020 22:30, Laurent Pinchart пишет:
...Quoted 5 lines
Quoted 2 lines
#include <
drm/drm_atomic.h>
+#include <drm/drm_bridge.h>You could add a forward declaration of struct
drm_bridgeinstead, that can lower the compilation time a little bit.
Quoted 8 lines
Quoted 3 lines
This include is not only for the struct, but also for the
drm_bridge_attach(). It looks to me that it should be nicer to keep the include here.
drm_bridge_attach()is called from .c files. In the .h file you can use a forward declaration. It's entirely up to you, but as a general rule, I personally try to use forward structure declarations in .h files as much as possible.
The current Tegra DRM code is a bit inconsistent in regards to having forward declarations, it doesn't have them more than have.
I'll add a forward declaration if there will be need to make a v5, ok?
Quoted 7 lines
Quoted 7 lines
...
Quoted 5 lines
drivers/gpu/drm/tegra/output.c static irqreturn_t hpd_irq(int irq, void *data) · line 103
Quoted 1 line
drivers/gpu/drm/tegra/output.c +1
-0+ port =of_get_child_by_name (output->of_node,"port" );Do you need to check for the presence of a port node first ? Can you just check the return value of
drm_of_find_panel_or_bridge(), and fall back to "nvidia,panel" if it returns-ENODEV?
Quoted 8 lines
Quoted 5 lines
Without the check, the
drm_of_find_panel_or_bridge()prints a very noisy error message about missing port node for every output that doesn't have a graph specified in a device-tree (HDMI, DSI and etc).https://elixir.bootlin.com/linux/v5.7-rc1/source/drivers/of/property.c#L621
Ah yes indeed. That's not very nice.
Please let me know if you'll have a better idea about how this could be handled.
Re: [PATCH v4 2/3] drm/tegra: output: Support DRM bridges
Hi Dmitry,
Quoted 19 lines · Dmitry Osipenko
17.04.2020 23:31, Laurent Pinchart пишет:
Quoted 9 lines · Dmitry Osipenko
Quoted 8 lines
17.04.2020 22:30, Laurent Pinchart пишет:
...Quoted 5 lines
Quoted 2 lines
#include <
drm/drm_atomic.h>
+#include <drm/drm_bridge.h>You could add a forward declaration of struct
drm_bridgeinstead, that can lower the compilation time a little bit.Quoted 8 lines
Quoted 3 lines
This include is not only for the struct, but also for the
drm_bridge_attach(). It looks to me that it should be nicer to keep the include here.
drm_bridge_attach()is called from .c files. In the .h file you can use a forward declaration. It's entirely up to you, but as a general rule, I personally try to use forward structure declarations in .h files as much as possible.
Quoted 4 lines
The current Tegra DRM code is a bit inconsistent in regards to having forward declarations, it doesn't have them more than have.
I'll add a forward declaration if there will be need to make a v5, ok?
It's up to you, you don't have to use a forward declaration if you don't want to, I was just pointing out what I think is a best practice rule :-)
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
...
Quoted 5 lines
drivers/gpu/drm/tegra/output.c static irqreturn_t hpd_irq(int irq, void *data) · line 103
Quoted 1 line
drivers/gpu/drm/tegra/output.c +1
-0+ port =of_get_child_by_name (output->of_node,"port" );Do you need to check for the presence of a port node first ? Can you just check the return value of
drm_of_find_panel_or_bridge(), and fall back to "nvidia,panel" if it returns-ENODEV?Without the check, the
drm_of_find_panel_or_bridge()prints a very noisy error message about missing port node for every output that doesn't have a graph specified in a device-tree (HDMI, DSI and etc).
Quoted 6 lines
Quoted 3 lines
Ah yes indeed. That's not very nice.
Please let me know if you'll have a better idea about how this could be handled.
It should be good enough as-is I think. You may however want to support both "port" and "ports", as even when there's a single port node, it could be put inside a ports node.
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v4 2/3] drm/tegra: output: Support DRM bridges
17.04.2020 23:58, Laurent Pinchart пишет:
Quoted 22 lines
Hi Dmitry,
Quoted 19 lines · Dmitry Osipenko
17.04.2020 23:31, Laurent Pinchart пишет:
Quoted 17 lines · Dmitry Osipenko
Quoted 11 lines
17.04.2020 22:30, Laurent Pinchart пишет:
...Quoted 5 lines
Quoted 2 lines
#include <
drm/drm_atomic.h>
+#include <drm/drm_bridge.h>You could add a forward declaration of struct
drm_bridgeinstead, that can lower the compilation time a little bit.This include is not only for the struct, but also for the
drm_bridge_attach(). It looks to me that it should be nicer to keep the include here.
drm_bridge_attach()is called from .c files. In the .h file you can use a forward declaration. It's entirely up to you, but as a general rule, I personally try to use forward structure declarations in .h files as much as possible.
Quoted 8 lines
Quoted 4 lines
The current Tegra DRM code is a bit inconsistent in regards to having forward declarations, it doesn't have them more than have.
I'll add a forward declaration if there will be need to make a v5, ok?
It's up to you, you don't have to use a forward declaration if you don't want to, I was just pointing out what I think is a best practice rule :-)
Alright, then I'll leave the include as-is in this patch since it should be better to keep the code consistent even if it's a bit less optimal than it could be, IMO.
We may return to cleaning up of driver includes later on.
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
Quoted 12 lines
...
Quoted 5 lines
drivers/gpu/drm/tegra/output.c static irqreturn_t hpd_irq(int irq, void *data) · line 103
Quoted 1 line
drivers/gpu/drm/tegra/output.c +1
-0+ port =of_get_child_by_name (output->of_node,"port" );Do you need to check for the presence of a port node first ? Can you just check the return value of
drm_of_find_panel_or_bridge(), and fall back to "nvidia,panel" if it returns-ENODEV?Without the check, the
drm_of_find_panel_or_bridge()prints a very noisy error message about missing port node for every output that doesn't have a graph specified in a device-tree (HDMI, DSI and etc).https://elixir.bootlin.com/linux/v5.7-rc1/source/drivers/of/property.c#L621
Quoted 9 lines
Quoted 4 lines
Quoted 1 line
Ah yes indeed. That's not very nice.
Please let me know if you'll have a better idea about how this could be handled.
It should be good enough as-is I think. You may however want to support both "port" and "ports", as even when there's a single port node, it could be put inside a ports node.
I'll make a v5 that will have additional patches for making drm_of_find_panel_or_bridge() to better handle that case.
While at it, I'll also add a patch that will wrap RGB panel into bridge.
Thank you very much for the reviews!
PATCH v4 3/3 drm/tegra: output: rgb: Support LVDS encoder bridge
In reply to Dmitry OsipenkoNewer 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.
--- drivers/gpu/drm/tegra/rgb.c | 58 +++++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 9 deletions(-)
drivers/gpu/drm/tegra/rgb.c +49 -9
@@ -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 ) {
Signature · 1 line
2.26.0
Re: [PATCH v4 3/3] drm/tegra: output: rgb: Support LVDS encoder bridge
Hi Dmitry,
Thank you for the patch.
Quoted 50 lines · Dmitry Osipenko
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.
Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/gpu/drm/tegra/rgb.c | 58 +++++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 9 deletions(-)drivers/gpu/drm/tegra/rgb.c +8
-9@@ -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
drivers/gpu/drm/tegra/rgb.c int tegra_dc_rgb_remove(struct tegra_dc *dc) · line 296
Quoted 10 lines
drivers/gpu/drm/tegra/rgb.c +10
-0+* 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.
As I tried to explain before, if you wrap the panel in a bridge with drm_panel_bridge_add() (or the devm_ variant), you will always have a bridge associated with the output, and will be able to remove your custom connector implementation. I thus recommend converting to drm_panel_bridge_add() either as part of this patch series, or just after it, to get full benefits.
With the assumption that this will be handled,
Quoted 34 lines
drivers/gpu/drm/tegra/rgb.c +31
-0+*/ +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 ) {
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v4 3/3] drm/tegra: output: rgb: Support LVDS encoder bridge
17.04.2020 22:24, Laurent Pinchart пишет:
...
Quoted 10 lines
As I tried to explain before, if you wrap the panel in a bridge with
drm_panel_bridge_add()(or thedevm_variant), you will always have a bridge associated with the output, and will be able to remove your custom connector implementation. I thus recommend converting todrm_panel_bridge_add()either as part of this patch series, or just after it, to get full benefits.With the assumption that this will be handled,
Thanks you very much!
Yes, I got yours point about wrapping panel into the bridge. But I don't think that it's worth the effort right now because each Tegra output has it's own implantation of the connector and it should be cleaner not to touch that code.
Secondly, I don't have hardware to test all available panel output types on Tegra and the benefits of messing with all that code are a bit dim to me.
I can make a patch to wrap the RGB panel into a bridge, but this should make code a bit inconsistent in regards to not having a common code path for the "legacy" nvidia,panel. So perhaps it's better to leave it all as-is for now.
Re: [PATCH v4 3/3] drm/tegra: output: rgb: Support LVDS encoder bridge
Quoted 20 lines · Dmitry Osipenko
17.04.2020 22:24, Laurent Pinchart пишет:
...Quoted 10 lines
As I tried to explain before, if you wrap the panel in a bridge with
drm_panel_bridge_add()(or thedevm_variant), you will always have a bridge associated with the output, and will be able to remove your custom connector implementation. I thus recommend converting todrm_panel_bridge_add()either as part of this patch series, or just after it, to get full benefits.With the assumption that this will be handled,
Thanks you very much!
Yes, I got yours point about wrapping panel into the bridge. But I don't think that it's worth the effort right now because each Tegra output has it's own implantation of the connector and it should be cleaner not to touch that code.
Quoted 7 lines
Secondly, I don't have hardware to test all available panel output types on Tegra and the benefits of messing with all that code are a bit dim to me.
I can make a patch to wrap the RGB panel into a bridge, but this should make code a bit inconsistent in regards to not having a common code path for the "legacy" nvidia,panel. So perhaps it's better to leave it all as-is for now.
I had a brief look at the code, converting the different output types one by one would be a better way forward than not doing anything at all in my opinion :-) Once you convert the first output it will also serve as an example on how to do it, and hopefully other developers with access to hardware could then do more conversions.
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v4 3/3] drm/tegra: output: rgb: Support LVDS encoder bridge
17.04.2020 23:34, Laurent Pinchart пишет:
Quoted 29 lines · Dmitry Osipenko
Quoted 20 lines
17.04.2020 22:24, Laurent Pinchart пишет:
...Quoted 10 lines
As I tried to explain before, if you wrap the panel in a bridge with
drm_panel_bridge_add()(or thedevm_variant), you will always have a bridge associated with the output, and will be able to remove your custom connector implementation. I thus recommend converting todrm_panel_bridge_add()either as part of this patch series, or just after it, to get full benefits.With the assumption that this will be handled,
Thanks you very much!
Yes, I got yours point about wrapping panel into the bridge. But I don't think that it's worth the effort right now because each Tegra output has it's own implantation of the connector and it should be cleaner not to touch that code.
Quoted 7 lines
Secondly, I don't have hardware to test all available panel output types on Tegra and the benefits of messing with all that code are a bit dim to me.
I can make a patch to wrap the RGB panel into a bridge, but this should make code a bit inconsistent in regards to not having a common code path for the "legacy" nvidia,panel. So perhaps it's better to leave it all as-is for now.
Quoted 6 lines
I had a brief look at the code, converting the different output types one by one would be a better way forward than not doing anything at all in my opinion :-) Once you convert the first output it will also serve as an example on how to do it, and hopefully other developers with access to hardware could then do more conversions.
Thierry, would you want to have RGB panel converted into a bridge? If yes, I'll make a v5 with that change.