Support DRM bridges on NVIDIA Tegra
v3 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:
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 (2):
drm/tegra: output: Don't leak OF node on error drm/tegra: output: rgb: Support LVDS encoder bridge
drivers/gpu/drm/tegra/drm.h | 2 ++ drivers/gpu/drm/tegra/output.c | 19 ++++++++++++++----- drivers/gpu/drm/tegra/rgb.c | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 5 deletions(-)
Signature · 1 line
2.26.0
PATCH v3 1/2 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
Re: [PATCH v3 1/2] drm/tegra: output: Don't leak OF node on error
Hi Dmitry,
Thank you for the patch.
Quoted 4 lines · Dmitry Osipenko
The OF node should be put before returning error in
tegra_output_probe(), otherwise node's refcount will be leaked.Signed-off-by Dmitry Osipenko <[email protected]>
Quoted 37 lines
--- 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 · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v3 1/2] drm/tegra: output: Don't leak OF node on error
16.04.2020 20:27, Laurent Pinchart пишет:
Quoted 11 lines
Hi Dmitry,
Thank you for the patch.
Quoted 4 lines · Dmitry Osipenko
The OF node should be put before returning error in
tegra_output_probe(), otherwise node's refcount will be leaked.Signed-off-by Dmitry Osipenko <[email protected]>
Hello Laurent,
That is the fastest kernel review I ever got, thank you :)
Re: [PATCH v3 1/2] drm/tegra: output: Don't leak OF node on error
Hi Dmitry,
Quoted 5 lines · Dmitry Osipenko
16.04.2020 20:27, Laurent Pinchart пишет:
Quoted 4 lines · Dmitry Osipenko
Quoted 3 lines
The OF node should be put before returning error in
tegra_output_probe(), otherwise node's refcount will be leaked.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Dmitry Osipenko <[email protected]>Hello Laurent,
That is the fastest kernel review I ever got, thank you :)
I think I'm also guilty for the slowest kernel reviews ever, so maybe in the end it will even out :-)
Signature · 3 lines
Regards, Laurent Pinchart
PATCH v3 2/2 drm/tegra: output: rgb: Support LVDS encoder bridge
In reply to Dmitry OsipenkoNewer Tegra device-trees will specify a video output graph that involves LVDS encoder bridge, This patch adds support for the LVDS encoder bridge to the RGB output, allowing us to model display hardware properly.
--- drivers/gpu/drm/tegra/drm.h | 2 ++ drivers/gpu/drm/tegra/output.c | 10 ++++++++++ drivers/gpu/drm/tegra/rgb.c | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+)
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 +10 -0
@@ -180,6 +180,16 @@ int tegra_output_init(struct drm_device *drm, struct tegra_output *output)int connector_type;int err; +if (output->bridge) { + err =drm_bridge_attach (&output->encoder, output->bridge, +NULL ,DRM_BRIDGE_ATTACH_NO_CONNECTOR ); +if (err) { +dev_err (output->dev,"cannot connect bridge: %d\n" , + err); +return err; + } + } +if (output->panel) { err =drm_panel_attach (output->panel, &output->connector);if (err <0 )
drivers/gpu/drm/tegra/rgb.c +34 -0
@@ -5,6 +5,7 @@*/ #include <linux/clk.h> +#include <linux/of_graph.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_panel.h>
@@ -210,6 +211,7 @@ static const struct drm_encoder_helper_funcs tegra_rgb_encoder_helper_funcs = {int tegra_dc_rgb_probe (struct tegra_dc *dc) { +const unsigned int encoder_port =0 , panel_port =1 ;struct device_node *np;struct tegra_rgb *rgb;int err;
@@ -226,6 +228,38 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc) rgb->output.of_node = np; rgb->dc = dc; +/* +* Tegra devices that have LVDS panel utilize LVDS-encoder bridge +* for converting 24/18 RGB data-lanes into 8 lanes. Encoder usually +* have a powerdown control which needs to be enabled in order to +* transfer 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 may utilize output->encoder->panel graph. +* +* For older device-trees we fall back to use nvidia,panel phandle. +*/ + np =of_graph_get_remote_node (rgb->output.of_node, encoder_port,-1 ); +if (np) { + rgb->output.bridge =of_drm_find_bridge (np); +of_node_put (np); + +if (!rgb->output.bridge) +return -EPROBE_DEFER ; + + np =of_graph_get_remote_node (rgb->output.bridge->of_node, + panel_port,-1 ); +if (np) { + rgb->output.panel =of_drm_find_panel (np); +of_node_put (np); + +if (IS_ERR (rgb->output.panel)) +return PTR_ERR (rgb->output.panel); + } + } + err =tegra_output_probe (&rgb->output);if (err <0 )return err;
Signature · 1 line
2.26.0
Re: [PATCH v3 2/2] drm/tegra: output: rgb: Support LVDS encoder bridge
Hi Dmitry,
Thank you for the patch.
Quoted 36 lines · Dmitry Osipenko
Newer Tegra device-trees will specify a video output graph that involves LVDS encoder bridge, This patch adds support for the LVDS encoder bridge to the RGB output, allowing us to model display hardware properly.
Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/gpu/drm/tegra/drm.h | 2 ++ drivers/gpu/drm/tegra/output.c | 10 ++++++++++ drivers/gpu/drm/tegra/rgb.c | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+)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 +0
-0
drivers/gpu/drm/tegra/output.c int tegra_output_init(struct drm_device *drm, struct tegra_output *output) · line 185
Quoted 6 lines
drivers/gpu/drm/tegra/output.c +3
-0@@ -180,6 +180,16 @@ int tegra_output_init(struct drm_device *drm, struct tegra_output *output)int connector_type;int err; +if (output->bridge) { + err =drm_bridge_attach (&output->encoder, output->bridge, +NULL ,DRM_BRIDGE_ATTACH_NO_CONNECTOR );
Using DRM_BRIDGE_ATTACH_NO_CONNECTOR is definitely the way to go, but please be aware that it will require creating a connector and an encoder manually (which I think this driver already does), and using the bridge operations to implement the connector operations. I highly recommend using the DRM bridge connector helper for this purpose.
drivers/gpu/drm/tegra/output.c int tegra_output_init(struct drm_device *drm, struct tegra_output *output) · line 193
Quoted 8 lines
drivers/gpu/drm/tegra/output.c +7
-0+if (err) { +dev_err (output->dev,"cannot connect bridge: %d\n" , + err); +return err; + } + } +if (output->panel) {
May I also recommend switching to the DRM panel bridge helper ? It will simplify the code.
Quoted 39 lines
drivers/gpu/drm/tegra/output.c +0
-0err =drm_panel_attach (output->panel, &output->connector);if (err <0 )drivers/gpu/drm/tegra/rgb.c +15
-0@@ -5,6 +5,7 @@*/ #include <linux/clk.h> +#include <linux/of_graph.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_panel.h> @@ -210,6 +211,7 @@ static const struct drm_encoder_helper_funcs tegra_rgb_encoder_helper_funcs = {int tegra_dc_rgb_probe (struct tegra_dc *dc) { +const unsigned int encoder_port =0 , panel_port =1 ;struct device_node *np;struct tegra_rgb *rgb;int err;@@ -226,6 +228,38 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc) rgb->output.of_node = np; rgb->dc = dc; +/* +* Tegra devices that have LVDS panel utilize LVDS-encoder bridge +* for converting 24/18 RGB data-lanes into 8 lanes. Encoder usually +* have a powerdown control which needs to be enabled in order to +* transfer 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 may utilize output->encoder->panel graph. +* +* For older device-trees we fall back to use nvidia,panel phandle. +*/
drivers/gpu/drm/tegra/rgb.c int tegra_dc_rgb_probe(struct tegra_dc *dc) · line 253
Quoted 10 lines
drivers/gpu/drm/tegra/rgb.c +10
-0+ np =of_graph_get_remote_node (rgb->output.of_node, encoder_port,-1 ); +if (np) { + rgb->output.bridge =of_drm_find_bridge (np); +of_node_put (np); + +if (!rgb->output.bridge) +return -EPROBE_DEFER ; + + np =of_graph_get_remote_node (rgb->output.bridge->of_node, + panel_port,-1 );
This shouldn't be needed, the LVDS codec bridge driver should already get the panel and handle it internally. You only need to handle panels in this driver when they're connected directly to the RGB input.
Quoted 12 lines
drivers/gpu/drm/tegra/rgb.c +9
-0+if (np) { + rgb->output.panel =of_drm_find_panel (np); +of_node_put (np); + +if (IS_ERR (rgb->output.panel)) +return PTR_ERR (rgb->output.panel); + } + } + err =tegra_output_probe (&rgb->output);if (err <0 )return err;
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v3 2/2] drm/tegra: output: rgb: Support LVDS encoder bridge
16.04.2020 20:41, Laurent Pinchart пишет:
...
Quoted 9 lines
drivers/gpu/drm/tegra/output.c int tegra_output_init(struct drm_device *drm, struct tegra_output *output) · line 185
Quoted 3 lines
drivers/gpu/drm/tegra/output.c +3
-0+if (output->bridge) { + err =drm_bridge_attach (&output->encoder, output->bridge, +NULL ,DRM_BRIDGE_ATTACH_NO_CONNECTOR );Using
DRM_BRIDGE_ATTACH_NO_CONNECTORis definitely the way to go, but please be aware that it will require creating a connector and an encoder manually (which I think this driver already does), and using the bridge operations to implement the connector operations. I highly recommend using the DRM bridge connector helper for this purpose.
Okay, I missed that there is a DRM bridge connector helper. Thank you very much for the suggestion, I'll switch to the bridge helper in v4.
Quoted 11 lines
drivers/gpu/drm/tegra/output.c int tegra_output_init(struct drm_device *drm, struct tegra_output *output) · line 193
Quoted 8 lines
drivers/gpu/drm/tegra/output.c +7
-0+if (err) { +dev_err (output->dev,"cannot connect bridge: %d\n" , + err); +return err; + } + } +if (output->panel) {May I also recommend switching to the DRM panel bridge helper ? It will simplify the code.
Could you please clarify what is the "DRM panel bridge helper"?
I think we won't need any additional helpers after switching to the bridge connector helper, no?
...
Quoted 6 lines
drivers/gpu/drm/tegra/rgb.c int tegra_dc_rgb_probe(struct tegra_dc *dc) · line 253
Quoted 2 lines
drivers/gpu/drm/tegra/rgb.c +2
-0+ np =of_graph_get_remote_node (rgb->output.bridge->of_node, + panel_port,-1 );This shouldn't be needed, the LVDS codec bridge driver should already get the panel and handle it internally. You only need to handle panels in this driver when they're connected directly to the RGB input.
Indeed, it won't be needed if we will use the bridge connector helper. Thank you very much for the clarifications!
Re: [PATCH v3 2/2] drm/tegra: output: rgb: Support LVDS encoder bridge
16.04.2020 21:52, Dmitry Osipenko пишет:
...
Quoted 7 lines
Quoted 2 lines
May I also recommend switching to the DRM panel bridge helper ? It will simplify the code.
Could you please clarify what is the "DRM panel bridge helper"?
I think we won't need any additional helpers after switching to the bridge connector helper, no?
Actually, I now see that the panel needs to be manually attached to the connector.
Still it's not apparent to me how to get panel out of the bridge. It looks like there is no such "panel helper" for the bridge API or I just can't find it.
Re: [PATCH v3 2/2] drm/tegra: output: rgb: Support LVDS encoder bridge
Hi Dimitry.
Quoted 10 lines · Dmitry Osipenko
16.04.2020 21:52, Dmitry Osipenko пишет:
...Quoted 7 lines
Quoted 2 lines
May I also recommend switching to the DRM panel bridge helper ? It will simplify the code.
Could you please clarify what is the "DRM panel bridge helper"?
I think we won't need any additional helpers after switching to the bridge connector helper, no?
Quoted 6 lines
Actually, I now see that the panel needs to be manually attached to the connector.
Still it's not apparent to me how to get panel out of the bridge. It looks like there is no such "panel helper" for the bridge API or I just can't find it.
Take a look in bridge/panel.c
I think devm_drm_panel_bridge_add() is what you are after.
Sam
Re: [PATCH v3 2/2] drm/tegra: output: rgb: Support LVDS encoder bridge
In reply to Dmitry OsipenkoHi Dmitry,
Quoted 5 lines · Dmitry Osipenko
16.04.2020 21:52, Dmitry Osipenko пишет:
...Quoted 3 lines
Quoted 2 lines
May I also recommend switching to the DRM panel bridge helper ? It will simplify the code.
Quoted 7 lines
Quoted 4 lines
Could you please clarify what is the "DRM panel bridge helper"?
I think we won't need any additional helpers after switching to the bridge connector helper, no?
Actually, I now see that the panel needs to be manually attached to the connector.
The DRM panel bridge helper creates a bridge from a panel (with devm_drm_panel_bridge_add()). You can then attach that bridge to the chain, like any other bridge, and the enable/disable operations will be called automatically without any need to call the panel enable/disable manually as done currently.
Quoted 3 lines
Still it's not apparent to me how to get panel out of the bridge. It looks like there is no such "panel helper" for the bridge API or I just can't find it.
You don't need to get a panel out of the bridge. You should get the bridge as done today, and wrap it in a bridge with devm_drm_panel_bridge_add().
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v3 2/2] drm/tegra: output: rgb: Support LVDS encoder bridge
16.04.2020 23:50, Laurent Pinchart пишет:
Quoted 22 lines
Hi Dmitry,
Quoted 5 lines · Dmitry Osipenko
16.04.2020 21:52, Dmitry Osipenko пишет:
...Quoted 3 lines
Quoted 2 lines
May I also recommend switching to the DRM panel bridge helper ? It will simplify the code.
Quoted 7 lines
Quoted 4 lines
Could you please clarify what is the "DRM panel bridge helper"?
I think we won't need any additional helpers after switching to the bridge connector helper, no?
Actually, I now see that the panel needs to be manually attached to the connector.
The DRM panel bridge helper creates a bridge from a panel (with
devm_drm_panel_bridge_add()). You can then attach that bridge to the chain, like any other bridge, and the enable/disable operations will be called automatically without any need to call the panel enable/disable manually as done currently.
Quoted 6 lines
Quoted 3 lines
Still it's not apparent to me how to get panel out of the bridge. It looks like there is no such "panel helper" for the bridge API or I just can't find it.
You don't need to get a panel out of the bridge. You should get the bridge as done today,
You mean "get the panel", correct?
Quoted 3 lines
and wrap it in a bridge with
devm_drm_panel_bridge_add().
The lvds-codec already wraps panel into the bridge using devm_drm_panel_bridge_add() and chains it for us, please see lvds_codec_probe() / lvds_codec_attach().
Does it mean that lvds-codec is not supporting the new model?
Everything works nicely using the old model, where bridge creates connector and attaches panel to it for us.
[I'm still not sure what is the point to use the new model in a case of a simple chain of bridges]
Using the new model, the connector isn't created by the bridge, so I need to duplicate that creation effort in the driver. Once the bridge connector is manually created, I need to attach panel to this connector, but panel is reachable only via the remote bridge (which wraps the panel).
driver connector -> LVDS bridge -> panel bridge -> panel
Re: [PATCH v3 2/2] drm/tegra: output: rgb: Support LVDS encoder bridge
Hi Dmitry,
Quoted 21 lines · Dmitry Osipenko
16.04.2020 23:50, Laurent Pinchart пишет:
Quoted 20 lines · Dmitry Osipenko
Quoted 5 lines
16.04.2020 21:52, Dmitry Osipenko пишет:
...Quoted 3 lines
Quoted 2 lines
May I also recommend switching to the DRM panel bridge helper ? It will simplify the code.
Quoted 7 lines
Quoted 4 lines
Could you please clarify what is the "DRM panel bridge helper"?
I think we won't need any additional helpers after switching to the bridge connector helper, no?
Actually, I now see that the panel needs to be manually attached to the connector.
The DRM panel bridge helper creates a bridge from a panel (with
devm_drm_panel_bridge_add()). You can then attach that bridge to the chain, like any other bridge, and the enable/disable operations will be called automatically without any need to call the panel enable/disable manually as done currently.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Still it's not apparent to me how to get panel out of the bridge. It looks like there is no such "panel helper" for the bridge API or I just can't find it.
You don't need to get a panel out of the bridge. You should get the bridge as done today,
You mean "get the panel", correct?
Yes, sorry.
Quoted 9 lines
Quoted 3 lines
and wrap it in a bridge with
devm_drm_panel_bridge_add().The lvds-codec already wraps panel into the bridge using
devm_drm_panel_bridge_add()and chains it for us, please seelvds_codec_probe()/lvds_codec_attach().Does it mean that lvds-codec is not supporting the new model?
No, that *is* the new model :-) As I think I've commented during review, when you have an LVDS encoder and a panel, you don't need to handle the panel at all. When you have an RGB panel connected directly to the RGB output, you need to wrap it in a bridge, exactly the same way as lvds-codec does for its panel.
Quoted 6 lines
Everything works nicely using the old model, where bridge creates connector and attaches panel to it for us.
[I'm still not sure what is the point to use the new model in a case of a simple chain of bridges]
Quoted 6 lines
Using the new model, the connector isn't created by the bridge, so I need to duplicate that creation effort in the driver. Once the bridge connector is manually created, I need to attach panel to this connector, but panel is reachable only via the remote bridge (which wraps the panel).
driver connector -> LVDS bridge -> panel bridge -> panel
With the new model,
1. The display driver and the bridge drivers need to get hold of the bridge directly connected to their output (for instance with of_drm_find_panel()). If the output is connected to a panel, they need to wrap that panel in a bridge (with devm_drm_panel_bridge_add()). I plan, in the future, to make creation of panel bridges automatic, so drivers won't have to care.
2. The display driver needs to create a dummy drm_encoder for each of its outputs (for instance with drm_simple_encoder_init()).
3. The display driver needs to create a drm_connector for each of its outputs, and implement connector operations by delegating them to the bridges in the pipeline. Unless there's a good reason not to do so, this should be done with drm_bridge_connector_init().
That's it. Every driver then focusses on its own needs, bridge drivers handle only the device they're associated with, and the DRM core and DRM bridge connector helper will handle all the rest.
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v3 2/2] drm/tegra: output: rgb: Support LVDS encoder bridge
17.04.2020 00:39, Laurent Pinchart пишет:
Quoted 76 lines
Hi Dmitry,
Quoted 21 lines · Dmitry Osipenko
16.04.2020 23:50, Laurent Pinchart пишет:
Quoted 20 lines · Dmitry Osipenko
Quoted 5 lines
16.04.2020 21:52, Dmitry Osipenko пишет:
...Quoted 3 lines
Quoted 2 lines
May I also recommend switching to the DRM panel bridge helper ? It will simplify the code.
Quoted 7 lines
Quoted 4 lines
Could you please clarify what is the "DRM panel bridge helper"?
I think we won't need any additional helpers after switching to the bridge connector helper, no?
Actually, I now see that the panel needs to be manually attached to the connector.
The DRM panel bridge helper creates a bridge from a panel (with
devm_drm_panel_bridge_add()). You can then attach that bridge to the chain, like any other bridge, and the enable/disable operations will be called automatically without any need to call the panel enable/disable manually as done currently.Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Still it's not apparent to me how to get panel out of the bridge. It looks like there is no such "panel helper" for the bridge API or I just can't find it.
You don't need to get a panel out of the bridge. You should get the bridge as done today,
You mean "get the panel", correct?
Yes, sorry.
Quoted 9 lines
Quoted 3 lines
and wrap it in a bridge with
devm_drm_panel_bridge_add().The lvds-codec already wraps panel into the bridge using
devm_drm_panel_bridge_add()and chains it for us, please seelvds_codec_probe()/lvds_codec_attach().Does it mean that lvds-codec is not supporting the new model?
No, that *is* the new model :-) As I think I've commented during review, when you have an LVDS encoder and a panel, you don't need to handle the panel at all. When you have an RGB panel connected directly to the RGB output, you need to wrap it in a bridge, exactly the same way as lvds-codec does for its panel.
Quoted 6 lines
Everything works nicely using the old model, where bridge creates connector and attaches panel to it for us.
[I'm still not sure what is the point to use the new model in a case of a simple chain of bridges]
Quoted 6 lines
Using the new model, the connector isn't created by the bridge, so I need to duplicate that creation effort in the driver. Once the bridge connector is manually created, I need to attach panel to this connector, but panel is reachable only via the remote bridge (which wraps the panel).
driver connector -> LVDS bridge -> panel bridge -> panel
With the new model,
1. The display driver and the bridge drivers need to get hold of the bridge directly connected to their output (for instance with
of_drm_find_panel()). If the output is connected to a panel, they need to wrap that panel in a bridge (withdevm_drm_panel_bridge_add()). I plan, in the future, to make creation of panel bridges automatic, so drivers won't have to care.2. The display driver needs to create a dummy
drm_encoderfor each of its outputs (for instance withdrm_simple_encoder_init()).
Quoted 9 lines
3. The display driver needs to create a
drm_connectorfor each of its outputs, and implement connector operations by delegating them to the bridges in the pipeline. Unless there's a good reason not to do so, this should be done withdrm_bridge_connector_init().That's it. Every driver then focusses on its own needs, bridge drivers handle only the device they're associated with, and the DRM core and DRM bridge connector helper will handle all the rest.
Thank you very much for the clarification again! :)
Now I realized what was the missing part.. in my case display panel is mounted upside-down and display output needs to be rotated by 180°. I have a local patch (hack) that adds orientation-property support to the panel-lvds driver and previously the panel's orientation was assigned to the connector using drm_panel_attach(), but now this attachment doesn't happen and I found that panel_lvds_get_modes() missed drm_connector_set_panel_orientation() in my patch. Everything is okay once the panel_lvds_get_modes() is corrected. I'll prepare the v4.