| Message ID | 20240624-mxc-lcdif-bridge-attach-v1-1-37e8c5d5d934@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/lcdif: switch to DRM_BRIDGE_ATTACH_NO_CONNECTOR | expand |
Hi, Am Montag, 24. Juni 2024, 12:31:46 CEST schrieb Dmitry Baryshkov: > Existing in-kernel device trees use LCDIF with the dsim + adv7533, dsim > + tc358762 or with ldb + panel_bridge. All these combinations support > using DRM_BRIDGE_ATTACH_NO_CONNECTOR for bridge attachment. > > Change lcdif driver to use this flag when attaching the bridge and > create drm_bridge_connector afterwards. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > Note: compile-tested only. I gave it a try, but it doesn't work. Despite DSI output it also breaks HDMI output, where I at least some error messages: [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@32c00000/hdmi@32fd8000 to encoder None-37: -22 [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@32c00000/display-bridge@32fc4000 to encoder None-37: -22 imx-lcdif 32fc6000.display-controller: error -EINVAL: Failed to attach bridge for endpoint0 imx-lcdif 32fc6000.display-controller: error -EINVAL: Cannot connect bridge imx-lcdif 32fc6000.display-controller: probe with driver imx-lcdif failed with error -22 Best regards, Alexander > --- > drivers/gpu/drm/mxsfb/lcdif_drv.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c > index 0f895b8a99d6..1d5508449995 100644 > --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c > +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c > @@ -16,6 +16,7 @@ > > #include <drm/drm_atomic_helper.h> > #include <drm/drm_bridge.h> > +#include <drm/drm_bridge_connector.h> > #include <drm/drm_drv.h> > #include <drm/drm_encoder.h> > #include <drm/drm_fbdev_dma.h> > @@ -48,6 +49,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) > { > struct device *dev = lcdif->drm->dev; > struct device_node *ep; > + struct drm_connector *connector; > struct drm_bridge *bridge; > int ret; > > @@ -96,13 +98,23 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) > return ret; > } > > - ret = drm_bridge_attach(encoder, bridge, NULL, 0); > + ret = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR); > if (ret) { > of_node_put(ep); > return dev_err_probe(dev, ret, > "Failed to attach bridge for endpoint%u\n", > of_ep.id); > } > + > + connector = drm_bridge_connector_init(lcdif->drm, encoder); > + if (IS_ERR(connector)) { > + ret = PTR_ERR(connector); > + of_node_put(ep); > + > + return dev_err_probe(dev, ret, > + "Failed to create bridge connector for endpoint%u\n", > + of_ep.id); > + } > } > > return 0; > > --- > base-commit: f76698bd9a8ca01d3581236082d786e9a6b72bb7 > change-id: 20240624-mxc-lcdif-bridge-attach-60368807b2f9 > > Best regards, >
On Mon, 24 Jun 2024 at 14:32, Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > > Hi, > > Am Montag, 24. Juni 2024, 12:31:46 CEST schrieb Dmitry Baryshkov: > > Existing in-kernel device trees use LCDIF with the dsim + adv7533, dsim > > + tc358762 or with ldb + panel_bridge. All these combinations support > > using DRM_BRIDGE_ATTACH_NO_CONNECTOR for bridge attachment. > > > > Change lcdif driver to use this flag when attaching the bridge and > > create drm_bridge_connector afterwards. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > Note: compile-tested only. > > I gave it a try, but it doesn't work. Despite DSI output it also breaks > HDMI output, where I at least some error messages: > [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@32c00000/hdmi@32fd8000 to encoder None-37: -22 > [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@32c00000/display-bridge@32fc4000 to encoder None-37: -22 > imx-lcdif 32fc6000.display-controller: error -EINVAL: Failed to attach bridge for endpoint0 > imx-lcdif 32fc6000.display-controller: error -EINVAL: Cannot connect bridge > imx-lcdif 32fc6000.display-controller: probe with driver imx-lcdif failed with error -22 Could you please try it with the following change: diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c index 13bc570c5473..c87d3e55c00d 100644 --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c @@ -94,6 +94,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev) plat_data->phy_name = "SAMSUNG HDMI TX PHY"; plat_data->priv_data = hdmi; plat_data->phy_force_vendor = true; + plat_data->output_port = 1; hdmi->dw_hdmi = dw_hdmi_probe(pdev, plat_data); if (IS_ERR(hdmi->dw_hdmi)) > > Best regards, > Alexander > > > --- > > drivers/gpu/drm/mxsfb/lcdif_drv.c | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c > > index 0f895b8a99d6..1d5508449995 100644 > > --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c > > +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c > > @@ -16,6 +16,7 @@ > > > > #include <drm/drm_atomic_helper.h> > > #include <drm/drm_bridge.h> > > +#include <drm/drm_bridge_connector.h> > > #include <drm/drm_drv.h> > > #include <drm/drm_encoder.h> > > #include <drm/drm_fbdev_dma.h> > > @@ -48,6 +49,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) > > { > > struct device *dev = lcdif->drm->dev; > > struct device_node *ep; > > + struct drm_connector *connector; > > struct drm_bridge *bridge; > > int ret; > > > > @@ -96,13 +98,23 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) > > return ret; > > } > > > > - ret = drm_bridge_attach(encoder, bridge, NULL, 0); > > + ret = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR); > > if (ret) { > > of_node_put(ep); > > return dev_err_probe(dev, ret, > > "Failed to attach bridge for endpoint%u\n", > > of_ep.id); > > } > > + > > + connector = drm_bridge_connector_init(lcdif->drm, encoder); > > + if (IS_ERR(connector)) { > > + ret = PTR_ERR(connector); > > + of_node_put(ep); > > + > > + return dev_err_probe(dev, ret, > > + "Failed to create bridge connector for endpoint%u\n", > > + of_ep.id); > > + } > > } > > > > return 0; > > > > --- > > base-commit: f76698bd9a8ca01d3581236082d786e9a6b72bb7 > > change-id: 20240624-mxc-lcdif-bridge-attach-60368807b2f9 > > > > Best regards, > > > > > -- > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > Amtsgericht München, HRB 105018 > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > http://www.tq-group.com/ > >
Hi Dmitry, Am Montag, 24. Juni 2024, 13:49:03 CEST schrieb Dmitry Baryshkov: > On Mon, 24 Jun 2024 at 14:32, Alexander Stein > <alexander.stein@ew.tq-group.com> wrote: > > > > Hi, > > > > Am Montag, 24. Juni 2024, 12:31:46 CEST schrieb Dmitry Baryshkov: > > > Existing in-kernel device trees use LCDIF with the dsim + adv7533, dsim > > > + tc358762 or with ldb + panel_bridge. All these combinations support > > > using DRM_BRIDGE_ATTACH_NO_CONNECTOR for bridge attachment. > > > > > > Change lcdif driver to use this flag when attaching the bridge and > > > create drm_bridge_connector afterwards. > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > --- > > > Note: compile-tested only. > > > > I gave it a try, but it doesn't work. Despite DSI output it also breaks > > HDMI output, where I at least some error messages: > > [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@32c00000/hdmi@32fd8000 to encoder None-37: -22 > > [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@32c00000/display-bridge@32fc4000 to encoder None-37: -22 > > imx-lcdif 32fc6000.display-controller: error -EINVAL: Failed to attach bridge for endpoint0 > > imx-lcdif 32fc6000.display-controller: error -EINVAL: Cannot connect bridge > > imx-lcdif 32fc6000.display-controller: probe with driver imx-lcdif failed with error -22 > > Could you please try it with the following change: > > diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c > b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c > index 13bc570c5473..c87d3e55c00d 100644 > --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c > +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c > @@ -94,6 +94,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev) > plat_data->phy_name = "SAMSUNG HDMI TX PHY"; > plat_data->priv_data = hdmi; > plat_data->phy_force_vendor = true; > + plat_data->output_port = 1; > > hdmi->dw_hdmi = dw_hdmi_probe(pdev, plat_data); > if (IS_ERR(hdmi->dw_hdmi)) Okay, this does fix the HDMI probe errors. Nevertheless I get the errors: [ 13.429313] [drm] Initialized imx-lcdif 1.0.0 for 32e80000.display-controller on minor 1 [ 13.439116] imx-lcdif 32e80000.display-controller: [drm] Cannot find any crtc or sizes [ 13.448168] imx-lcdif 32e80000.display-controller: [drm] Cannot find any crtc or sizes [ 15.519737] [drm] Initialized imx-lcdif 1.0.0 for 32fc6000.display-controller on minor 2 [ 15.675672] imx-lcdif 32fc6000.display-controller: [drm] Cannot find any crtc or sizes just from the lcdif patch for both HDMI and DP. Best regards, Alexander > > > > > Best regards, > > Alexander > > > > > --- > > > drivers/gpu/drm/mxsfb/lcdif_drv.c | 14 +++++++++++++- > > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c > > > index 0f895b8a99d6..1d5508449995 100644 > > > --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c > > > +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c > > > @@ -16,6 +16,7 @@ > > > > > > #include <drm/drm_atomic_helper.h> > > > #include <drm/drm_bridge.h> > > > +#include <drm/drm_bridge_connector.h> > > > #include <drm/drm_drv.h> > > > #include <drm/drm_encoder.h> > > > #include <drm/drm_fbdev_dma.h> > > > @@ -48,6 +49,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) > > > { > > > struct device *dev = lcdif->drm->dev; > > > struct device_node *ep; > > > + struct drm_connector *connector; > > > struct drm_bridge *bridge; > > > int ret; > > > > > > @@ -96,13 +98,23 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) > > > return ret; > > > } > > > > > > - ret = drm_bridge_attach(encoder, bridge, NULL, 0); > > > + ret = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR); > > > if (ret) { > > > of_node_put(ep); > > > return dev_err_probe(dev, ret, > > > "Failed to attach bridge for endpoint%u\n", > > > of_ep.id); > > > } > > > + > > > + connector = drm_bridge_connector_init(lcdif->drm, encoder); > > > + if (IS_ERR(connector)) { > > > + ret = PTR_ERR(connector); > > > + of_node_put(ep); > > > + > > > + return dev_err_probe(dev, ret, > > > + "Failed to create bridge connector for endpoint%u\n", > > > + of_ep.id); > > > + } > > > } > > > > > > return 0; > > > > > > --- > > > base-commit: f76698bd9a8ca01d3581236082d786e9a6b72bb7 > > > change-id: 20240624-mxc-lcdif-bridge-attach-60368807b2f9 > > > > > > Best regards, > > > > > > > > > -- > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > > Amtsgericht München, HRB 105018 > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > > http://www.tq-group.com/ > > > > > > >
On Mon, 24 Jun 2024 at 15:03, Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > > Hi Dmitry, > > Am Montag, 24. Juni 2024, 13:49:03 CEST schrieb Dmitry Baryshkov: > > On Mon, 24 Jun 2024 at 14:32, Alexander Stein > > <alexander.stein@ew.tq-group.com> wrote: > > > > > > Hi, > > > > > > Am Montag, 24. Juni 2024, 12:31:46 CEST schrieb Dmitry Baryshkov: > > > > Existing in-kernel device trees use LCDIF with the dsim + adv7533, dsim > > > > + tc358762 or with ldb + panel_bridge. All these combinations support > > > > using DRM_BRIDGE_ATTACH_NO_CONNECTOR for bridge attachment. > > > > > > > > Change lcdif driver to use this flag when attaching the bridge and > > > > create drm_bridge_connector afterwards. > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > --- > > > > Note: compile-tested only. > > > > > > I gave it a try, but it doesn't work. Despite DSI output it also breaks > > > HDMI output, where I at least some error messages: > > > [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@32c00000/hdmi@32fd8000 to encoder None-37: -22 > > > [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@32c00000/display-bridge@32fc4000 to encoder None-37: -22 > > > imx-lcdif 32fc6000.display-controller: error -EINVAL: Failed to attach bridge for endpoint0 > > > imx-lcdif 32fc6000.display-controller: error -EINVAL: Cannot connect bridge > > > imx-lcdif 32fc6000.display-controller: probe with driver imx-lcdif failed with error -22 > > > > Could you please try it with the following change: > > > > diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c > > b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c > > index 13bc570c5473..c87d3e55c00d 100644 > > --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c > > +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c > > @@ -94,6 +94,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev) > > plat_data->phy_name = "SAMSUNG HDMI TX PHY"; > > plat_data->priv_data = hdmi; > > plat_data->phy_force_vendor = true; > > + plat_data->output_port = 1; > > > > hdmi->dw_hdmi = dw_hdmi_probe(pdev, plat_data); > > if (IS_ERR(hdmi->dw_hdmi)) > > Okay, this does fix the HDMI probe errors. Nevertheless I get the errors: > [ 13.429313] [drm] Initialized imx-lcdif 1.0.0 for 32e80000.display-controller on minor 1 > [ 13.439116] imx-lcdif 32e80000.display-controller: [drm] Cannot find any crtc or sizes > [ 13.448168] imx-lcdif 32e80000.display-controller: [drm] Cannot find any crtc or sizes > [ 15.519737] [drm] Initialized imx-lcdif 1.0.0 for 32fc6000.display-controller on minor 2 > [ 15.675672] imx-lcdif 32fc6000.display-controller: [drm] Cannot find any crtc or sizes > > just from the lcdif patch for both HDMI and DP. It seems, because I'm stupid and I missed drm_connector_attach_encoder(). Anyway, it looks like even if that fixes the issue, dw-hdmi needs more work to work with drm_bridge_connector without losing access to such features like YCbCr 420 and CEC. So thank you for the tests, but let's probably drop the patch / idea for now. > > Best regards, > Alexander > > > > > > > > > Best regards, > > > Alexander > > > > > > > --- > > > > drivers/gpu/drm/mxsfb/lcdif_drv.c | 14 +++++++++++++- > > > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c > > > > index 0f895b8a99d6..1d5508449995 100644 > > > > --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c > > > > +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c > > > > @@ -16,6 +16,7 @@ > > > > > > > > #include <drm/drm_atomic_helper.h> > > > > #include <drm/drm_bridge.h> > > > > +#include <drm/drm_bridge_connector.h> > > > > #include <drm/drm_drv.h> > > > > #include <drm/drm_encoder.h> > > > > #include <drm/drm_fbdev_dma.h> > > > > @@ -48,6 +49,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) > > > > { > > > > struct device *dev = lcdif->drm->dev; > > > > struct device_node *ep; > > > > + struct drm_connector *connector; > > > > struct drm_bridge *bridge; > > > > int ret; > > > > > > > > @@ -96,13 +98,23 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) > > > > return ret; > > > > } > > > > > > > > - ret = drm_bridge_attach(encoder, bridge, NULL, 0); > > > > + ret = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR); > > > > if (ret) { > > > > of_node_put(ep); > > > > return dev_err_probe(dev, ret, > > > > "Failed to attach bridge for endpoint%u\n", > > > > of_ep.id); > > > > } > > > > + > > > > + connector = drm_bridge_connector_init(lcdif->drm, encoder); > > > > + if (IS_ERR(connector)) { > > > > + ret = PTR_ERR(connector); > > > > + of_node_put(ep); > > > > + > > > > + return dev_err_probe(dev, ret, > > > > + "Failed to create bridge connector for endpoint%u\n", > > > > + of_ep.id); > > > > + } > > > > } > > > > > > > > return 0; > > > > > > > > --- > > > > base-commit: f76698bd9a8ca01d3581236082d786e9a6b72bb7 > > > > change-id: 20240624-mxc-lcdif-bridge-attach-60368807b2f9 > > > > > > > > Best regards, > > > > > > > > > > > > > -- > > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > > > Amtsgericht München, HRB 105018 > > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > > > http://www.tq-group.com/ > > > > > > > > > > > > > > > -- > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > Amtsgericht München, HRB 105018 > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > http://www.tq-group.com/ > >
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c index 0f895b8a99d6..1d5508449995 100644 --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c @@ -16,6 +16,7 @@ #include <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> +#include <drm/drm_bridge_connector.h> #include <drm/drm_drv.h> #include <drm/drm_encoder.h> #include <drm/drm_fbdev_dma.h> @@ -48,6 +49,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) { struct device *dev = lcdif->drm->dev; struct device_node *ep; + struct drm_connector *connector; struct drm_bridge *bridge; int ret; @@ -96,13 +98,23 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) return ret; } - ret = drm_bridge_attach(encoder, bridge, NULL, 0); + ret = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR); if (ret) { of_node_put(ep); return dev_err_probe(dev, ret, "Failed to attach bridge for endpoint%u\n", of_ep.id); } + + connector = drm_bridge_connector_init(lcdif->drm, encoder); + if (IS_ERR(connector)) { + ret = PTR_ERR(connector); + of_node_put(ep); + + return dev_err_probe(dev, ret, + "Failed to create bridge connector for endpoint%u\n", + of_ep.id); + } } return 0;
Existing in-kernel device trees use LCDIF with the dsim + adv7533, dsim + tc358762 or with ldb + panel_bridge. All these combinations support using DRM_BRIDGE_ATTACH_NO_CONNECTOR for bridge attachment. Change lcdif driver to use this flag when attaching the bridge and create drm_bridge_connector afterwards. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- Note: compile-tested only. --- drivers/gpu/drm/mxsfb/lcdif_drv.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) --- base-commit: f76698bd9a8ca01d3581236082d786e9a6b72bb7 change-id: 20240624-mxc-lcdif-bridge-attach-60368807b2f9 Best regards,