| Message ID | 20241018064813.2750016-6-victor.liu@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm: lcdif: Use drm_bridge_connector | expand |
Hi Liu Ying, Thanks for the patch. > -----Original Message----- > From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On Behalf Of Liu Ying > Sent: Friday, October 18, 2024 7:48 AM > Subject: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector > > Initialize a connector by calling drm_bridge_connector_init() for each encoder so that down stream > bridge drivers don't need to create connectors any more. > > Signed-off-by: Liu Ying <victor.liu@nxp.com> > --- > drivers/gpu/drm/mxsfb/Kconfig | 1 + > drivers/gpu/drm/mxsfb/lcdif_drv.c | 17 ++++++++++++++++- > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig index > 264e74f45554..06c95e556380 100644 > --- a/drivers/gpu/drm/mxsfb/Kconfig > +++ b/drivers/gpu/drm/mxsfb/Kconfig > @@ -27,6 +27,7 @@ config DRM_IMX_LCDIF > depends on DRM && OF > depends on COMMON_CLK > depends on ARCH_MXC || COMPILE_TEST > + select DRM_BRIDGE_CONNECTOR > select DRM_CLIENT_SELECTION > select DRM_MXS > select DRM_KMS_HELPER > diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c > index 58ccad9c425d..d4521da6675e 100644 > --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c > +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c > @@ -16,7 +16,9 @@ > > #include <drm/drm_atomic_helper.h> > #include <drm/drm_bridge.h> > +#include <drm/drm_bridge_connector.h> > #include <drm/drm_client_setup.h> > +#include <drm/drm_connector.h> > #include <drm/drm_drv.h> > #include <drm/drm_encoder.h> > #include <drm/drm_fbdev_dma.h> > @@ -56,6 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) > struct device_node *remote; > struct of_endpoint of_ep; > struct drm_encoder *encoder; > + struct drm_connector *connector; > > remote = of_graph_get_remote_port_parent(ep); > if (!of_device_is_available(remote)) { @@ -97,13 +100,25 @@ 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); > + dev_err(dev, "Failed to initialize bridge connector: %d\n", > + ret); > + of_node_put(ep); > + return ret; Maybe same error path style like above?? of_node_put(ep); return dev_err_probe(dev, PTR_ERR(connector),"Failed to initialize bridge connector"); Cheers, Biju > + } > + > + drm_connector_attach_encoder(connector, encoder); > } > > return 0; > -- > 2.34.1 >
Hi, Am Freitag, 18. Oktober 2024, 08:48:13 CEST schrieb Liu Ying: > Initialize a connector by calling drm_bridge_connector_init() for > each encoder so that down stream bridge drivers don't need to create > connectors any more. > > Signed-off-by: Liu Ying <victor.liu@nxp.com> I've tested this using HDMI output. HPD and CEC is still working. Also output using a DSI->DP bridge (tc358767) also works. My platform: imx8mp-tqma8mpql-mba8mpxl.dts Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> > --- > drivers/gpu/drm/mxsfb/Kconfig | 1 + > drivers/gpu/drm/mxsfb/lcdif_drv.c | 17 ++++++++++++++++- > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig > index 264e74f45554..06c95e556380 100644 > --- a/drivers/gpu/drm/mxsfb/Kconfig > +++ b/drivers/gpu/drm/mxsfb/Kconfig > @@ -27,6 +27,7 @@ config DRM_IMX_LCDIF > depends on DRM && OF > depends on COMMON_CLK > depends on ARCH_MXC || COMPILE_TEST > + select DRM_BRIDGE_CONNECTOR > select DRM_CLIENT_SELECTION > select DRM_MXS > select DRM_KMS_HELPER > diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c > index 58ccad9c425d..d4521da6675e 100644 > --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c > +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c > @@ -16,7 +16,9 @@ > > #include <drm/drm_atomic_helper.h> > #include <drm/drm_bridge.h> > +#include <drm/drm_bridge_connector.h> > #include <drm/drm_client_setup.h> > +#include <drm/drm_connector.h> > #include <drm/drm_drv.h> > #include <drm/drm_encoder.h> > #include <drm/drm_fbdev_dma.h> > @@ -56,6 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) > struct device_node *remote; > struct of_endpoint of_ep; > struct drm_encoder *encoder; > + struct drm_connector *connector; > > remote = of_graph_get_remote_port_parent(ep); > if (!of_device_is_available(remote)) { > @@ -97,13 +100,25 @@ 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); > + dev_err(dev, "Failed to initialize bridge connector: %d\n", > + ret); > + of_node_put(ep); > + return ret; > + } > + > + drm_connector_attach_encoder(connector, encoder); > } > > return 0; >
On 10/18/2024, Biju Das wrote: > Hi Liu Ying, Hi Biju, > > Thanks for the patch. Thanks for your review. > >> -----Original Message----- >> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On Behalf Of Liu Ying >> Sent: Friday, October 18, 2024 7:48 AM >> Subject: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector >> >> Initialize a connector by calling drm_bridge_connector_init() for each encoder so that down stream >> bridge drivers don't need to create connectors any more. >> >> Signed-off-by: Liu Ying <victor.liu@nxp.com> >> --- >> drivers/gpu/drm/mxsfb/Kconfig | 1 + >> drivers/gpu/drm/mxsfb/lcdif_drv.c | 17 ++++++++++++++++- >> 2 files changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig index >> 264e74f45554..06c95e556380 100644 >> --- a/drivers/gpu/drm/mxsfb/Kconfig >> +++ b/drivers/gpu/drm/mxsfb/Kconfig >> @@ -27,6 +27,7 @@ config DRM_IMX_LCDIF >> depends on DRM && OF >> depends on COMMON_CLK >> depends on ARCH_MXC || COMPILE_TEST >> + select DRM_BRIDGE_CONNECTOR >> select DRM_CLIENT_SELECTION >> select DRM_MXS >> select DRM_KMS_HELPER >> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c >> index 58ccad9c425d..d4521da6675e 100644 >> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c >> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c >> @@ -16,7 +16,9 @@ >> >> #include <drm/drm_atomic_helper.h> >> #include <drm/drm_bridge.h> >> +#include <drm/drm_bridge_connector.h> >> #include <drm/drm_client_setup.h> >> +#include <drm/drm_connector.h> >> #include <drm/drm_drv.h> >> #include <drm/drm_encoder.h> >> #include <drm/drm_fbdev_dma.h> >> @@ -56,6 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) >> struct device_node *remote; >> struct of_endpoint of_ep; >> struct drm_encoder *encoder; >> + struct drm_connector *connector; >> >> remote = of_graph_get_remote_port_parent(ep); >> if (!of_device_is_available(remote)) { @@ -97,13 +100,25 @@ 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); >> + dev_err(dev, "Failed to initialize bridge connector: %d\n", >> + ret); >> + of_node_put(ep); >> + return ret; > > Maybe same error path style like above?? > of_node_put(ep); > return dev_err_probe(dev, PTR_ERR(connector),"Failed to initialize bridge connector"); I thought about that and decided not to use dev_err_probe() because I don't think drm_bridge_connector_init() may return -EPROBE_DEFER, no? > > Cheers, > Biju > >> + } >> + >> + drm_connector_attach_encoder(connector, encoder); >> } >> >> return 0; >> -- >> 2.34.1 >> >
Hi Liu Ying, > -----Original Message----- > From: Liu Ying <victor.liu@nxp.com> > Sent: Friday, October 18, 2024 8:38 AM > To: Biju Das <biju.das.jz@bp.renesas.com>; devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dri-devel@lists.freedesktop.org > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; shawnguo@kernel.org; > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com; andrzej.hajda@intel.com; > neil.armstrong@linaro.org; rfoss@kernel.org; laurent.pinchart <laurent.pinchart@ideasonboard.com>; > jonas@kwiboo.se; jernej.skrabec@gmail.com; maarten.lankhorst@linux.intel.com; mripard@kernel.org; > tzimmermann@suse.de; airlied@gmail.com; simona@ffwll.ch; marex@denx.de; stefan@agner.ch; > dmitry.baryshkov@linaro.org > Subject: Re: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector > > On 10/18/2024, Biju Das wrote: > > Hi Liu Ying, > > Hi Biju, > > > > > Thanks for the patch. > > Thanks for your review. > > > > >> -----Original Message----- > >> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> > >> On Behalf Of Liu Ying > >> Sent: Friday, October 18, 2024 7:48 AM > >> Subject: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector > >> > >> Initialize a connector by calling drm_bridge_connector_init() for > >> each encoder so that down stream bridge drivers don't need to create connectors any more. > >> > >> Signed-off-by: Liu Ying <victor.liu@nxp.com> > >> --- > >> drivers/gpu/drm/mxsfb/Kconfig | 1 + > >> drivers/gpu/drm/mxsfb/lcdif_drv.c | 17 ++++++++++++++++- > >> 2 files changed, 17 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/mxsfb/Kconfig > >> b/drivers/gpu/drm/mxsfb/Kconfig index > >> 264e74f45554..06c95e556380 100644 > >> --- a/drivers/gpu/drm/mxsfb/Kconfig > >> +++ b/drivers/gpu/drm/mxsfb/Kconfig > >> @@ -27,6 +27,7 @@ config DRM_IMX_LCDIF > >> depends on DRM && OF > >> depends on COMMON_CLK > >> depends on ARCH_MXC || COMPILE_TEST > >> + select DRM_BRIDGE_CONNECTOR > >> select DRM_CLIENT_SELECTION > >> select DRM_MXS > >> select DRM_KMS_HELPER > >> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c > >> b/drivers/gpu/drm/mxsfb/lcdif_drv.c > >> index 58ccad9c425d..d4521da6675e 100644 > >> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c > >> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c > >> @@ -16,7 +16,9 @@ > >> > >> #include <drm/drm_atomic_helper.h> > >> #include <drm/drm_bridge.h> > >> +#include <drm/drm_bridge_connector.h> > >> #include <drm/drm_client_setup.h> > >> +#include <drm/drm_connector.h> > >> #include <drm/drm_drv.h> > >> #include <drm/drm_encoder.h> > >> #include <drm/drm_fbdev_dma.h> > >> @@ -56,6 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) > >> struct device_node *remote; > >> struct of_endpoint of_ep; > >> struct drm_encoder *encoder; > >> + struct drm_connector *connector; > >> > >> remote = of_graph_get_remote_port_parent(ep); > >> if (!of_device_is_available(remote)) { @@ -97,13 +100,25 @@ 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); > >> + dev_err(dev, "Failed to initialize bridge connector: %d\n", > >> + ret); > >> + of_node_put(ep); > >> + return ret; > > > > Maybe same error path style like above?? > > of_node_put(ep); > > return dev_err_probe(dev, PTR_ERR(connector),"Failed to initialize > > bridge connector"); > > I thought about that and decided not to use dev_err_probe() because I don't think > drm_bridge_connector_init() may return -EPROBE_DEFER, no? Yes, you are correct. Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> Cheers, Biju > > > > > Cheers, > > Biju > > > >> + } > >> + > >> + drm_connector_attach_encoder(connector, encoder); > >> } > >> > >> return 0; > >> -- > >> 2.34.1 > >> > > > > -- > Regards, > Liu Ying
diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig index 264e74f45554..06c95e556380 100644 --- a/drivers/gpu/drm/mxsfb/Kconfig +++ b/drivers/gpu/drm/mxsfb/Kconfig @@ -27,6 +27,7 @@ config DRM_IMX_LCDIF depends on DRM && OF depends on COMMON_CLK depends on ARCH_MXC || COMPILE_TEST + select DRM_BRIDGE_CONNECTOR select DRM_CLIENT_SELECTION select DRM_MXS select DRM_KMS_HELPER diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c index 58ccad9c425d..d4521da6675e 100644 --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c @@ -16,7 +16,9 @@ #include <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> +#include <drm/drm_bridge_connector.h> #include <drm/drm_client_setup.h> +#include <drm/drm_connector.h> #include <drm/drm_drv.h> #include <drm/drm_encoder.h> #include <drm/drm_fbdev_dma.h> @@ -56,6 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif) struct device_node *remote; struct of_endpoint of_ep; struct drm_encoder *encoder; + struct drm_connector *connector; remote = of_graph_get_remote_port_parent(ep); if (!of_device_is_available(remote)) { @@ -97,13 +100,25 @@ 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); + dev_err(dev, "Failed to initialize bridge connector: %d\n", + ret); + of_node_put(ep); + return ret; + } + + drm_connector_attach_encoder(connector, encoder); } return 0;
Initialize a connector by calling drm_bridge_connector_init() for each encoder so that down stream bridge drivers don't need to create connectors any more. Signed-off-by: Liu Ying <victor.liu@nxp.com> --- drivers/gpu/drm/mxsfb/Kconfig | 1 + drivers/gpu/drm/mxsfb/lcdif_drv.c | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-)