This patch creates another Encoder for HDMI port, and linking the Encoder to appropriate DRM bridge. And this Encoder using same CRTC with RGB-LCD. For RGB-LCD and HDMI using the same hardware connection to DCU, RGB-LCD panel should be unplugged when using the HDMI connection.
Signed-off-by: Alison Wang alison.wang@nxp.com Signed-off-by: Xiubo Li lixiubo@cmss.chinamobile.com Signed-off-by: Jianwei Wang jianwei.wang.chn@gmail.com Signed-off-by: Meng Yi meng.yi@nxp.com --- Changes in V2: -remove unused headers inclusion -remove module declarations -fix error handling coding style -drop moulde parameters and auto detect HDMI connection relying on deviece tree -modified comment lines --- drivers/gpu/drm/fsl-dcu/Makefile | 1 + drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_hdmi.c | 189 +++++++++++++++++++++++++++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 16 +++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h | 4 + 4 files changed, 210 insertions(+) create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_hdmi.c
diff --git a/drivers/gpu/drm/fsl-dcu/Makefile b/drivers/gpu/drm/fsl-dcu/Makefile index b35a292..12e2245 100644 --- a/drivers/gpu/drm/fsl-dcu/Makefile +++ b/drivers/gpu/drm/fsl-dcu/Makefile @@ -1,6 +1,7 @@ fsl-dcu-drm-y := fsl_dcu_drm_drv.o \ fsl_dcu_drm_kms.o \ fsl_dcu_drm_rgb.o \ + fsl_dcu_drm_hdmi.o \ fsl_dcu_drm_plane.o \ fsl_dcu_drm_crtc.o \ fsl_dcu_drm_fbdev.o \ diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_hdmi.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_hdmi.c new file mode 100644 index 0000000..f567534 --- /dev/null +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_hdmi.c @@ -0,0 +1,189 @@ +/* + * Copyright 2016 NXP Semiconductor, Inc. + * + * NXP DCU drm device driver + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include <linux/errno.h> +#include <linux/kernel.h> +#include <linux/of_graph.h> + +#include <drm/drmP.h> +#include <drm/drm_crtc_helper.h> + +#include "fsl_dcu_drm_drv.h" +#include "fsl_dcu_drm_output.h" +#include "fsl_tcon.h" + +static void +fsl_dcu_drm_hdmienc_mode_set(struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + /*TODO*/ +} + +static int +fsl_dcu_drm_hdmienc_atomic_check(struct drm_encoder *encoder, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + return 0; +} + +static void +fsl_dcu_drm_hdmienc_disable(struct drm_encoder *encoder) +{ + struct drm_device *dev = encoder->dev; + struct fsl_dcu_drm_device *fsl_dev = dev->dev_private; + + if (fsl_dev->tcon) + fsl_tcon_bypass_disable(fsl_dev->tcon); +} + +static void +fsl_dcu_drm_hdmienc_enable(struct drm_encoder *encoder) +{ + struct drm_device *dev = encoder->dev; + struct fsl_dcu_drm_device *fsl_dev = dev->dev_private; + + if (fsl_dev->tcon) + fsl_tcon_bypass_enable(fsl_dev->tcon); +} + +static const struct +drm_encoder_helper_funcs encoder_helper_funcs = { + .atomic_check = fsl_dcu_drm_hdmienc_atomic_check, + .disable = fsl_dcu_drm_hdmienc_disable, + .enable = fsl_dcu_drm_hdmienc_enable, + .mode_set = fsl_dcu_drm_hdmienc_mode_set, +}; + +static void +fsl_dcu_drm_hdmienc_destroy(struct drm_encoder *encoder) +{ + drm_encoder_cleanup(encoder); +} + +static const struct +drm_encoder_funcs encoder_funcs = { + .destroy = fsl_dcu_drm_hdmienc_destroy, +}; + +int fsl_dcu_drm_hdmienc_create(struct fsl_dcu_drm_device *fsl_dev, + struct drm_crtc *crtc) +{ + struct drm_encoder *encoder; + int ret; + + encoder = devm_kzalloc(fsl_dev->dev, + sizeof(struct drm_encoder), GFP_KERNEL); + if (!encoder) + return -ENOMEM; + + encoder->possible_crtcs = 1; + ret = drm_encoder_init(fsl_dev->drm, encoder, &encoder_funcs, + DRM_MODE_ENCODER_TMDS, NULL); + if (ret) + goto fail_encoder; + + drm_encoder_helper_add(encoder, &encoder_helper_funcs); + encoder->crtc = crtc; + + return 0; + +fail_encoder: + devm_kfree(fsl_dev->dev, encoder); + return ret; +} + +static struct +drm_encoder *fsl_dcu_drm_hdmi_find_encoder(struct drm_device *dev) +{ + struct drm_encoder *encoder; + + list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { + if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS) + return encoder; + } + + return NULL; +} + +static struct +device_node *detect_hdmi_connection(struct fsl_dcu_drm_device *fsl_dev) +{ + struct device_node *remote_port; + struct of_endpoint *ep; + struct device_node *ep_node; + int ret; + struct device_node *parent = fsl_dev->dev->of_node; + + ep = devm_kzalloc(fsl_dev->dev, + sizeof(struct of_endpoint), GFP_KERNEL); + if (!ep) + return NULL; + + ep_node = devm_kzalloc(fsl_dev->dev, + sizeof(struct device_node), GFP_KERNEL); + if (!ep_node) + return NULL; + + ep_node = of_graph_get_next_endpoint(parent, NULL); + if (!ep_node) + goto error; + + ret = of_graph_parse_endpoint(ep_node, ep); + if (ret) { + of_node_put(ep_node); + goto error; + } + + remote_port = of_graph_get_remote_port_parent(ep->local_node); + if (!remote_port) + goto error; + + return remote_port; +error: + devm_kfree(ep); + devm_kfree(ep_node); + return NULL; +} + +int fsl_dcu_drm_hdmienc_attach_bridge(struct fsl_dcu_drm_device *fsl_dev) +{ + struct drm_device *drm_dev = fsl_dev->drm; + struct drm_encoder *encoder; + struct drm_bridge *bridge; + struct device_node *remote_port; + int ret; + + remote_port = detect_hdmi_connection(fsl_dev); + if (!remote_port) + return -ENODEV; + + bridge = of_drm_find_bridge(remote_port); + if (!bridge) + return -ENODEV; + + encoder = fsl_dcu_drm_hdmi_find_encoder(drm_dev); + if (!encoder) + return -ENODEV; + + encoder->bridge = bridge; + bridge->encoder = encoder; + + ret = drm_bridge_attach(drm_dev, bridge); + if (ret) + goto error; + + return 0; +error: + encoder->funcs->destroy(encoder); + return ret; +} diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c index c564ec6..658bc92 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c @@ -17,10 +17,18 @@ #include "fsl_dcu_drm_crtc.h" #include "fsl_dcu_drm_drv.h"
+static void fsl_dcu_drm_output_poll_changed(struct drm_device *dev) +{ + struct fsl_dcu_drm_device *fsl_dev = dev->dev_private; + + drm_fbdev_cma_hotplug_event(fsl_dev->fbdev); +} + static const struct drm_mode_config_funcs fsl_dcu_drm_mode_config_funcs = { .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit, .fb_create = drm_fb_cma_create, + .output_poll_changed = fsl_dcu_drm_output_poll_changed, };
int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev) @@ -47,6 +55,14 @@ int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev) if (ret) goto fail_connector;
+ ret = fsl_dcu_drm_hdmienc_create(fsl_dev, &fsl_dev->crtc); + if (ret) + DRM_ERROR("Failed to create HDMI encoder\n"); + + ret = fsl_dcu_drm_hdmienc_attach_bridge(fsl_dev); + if (ret) + DRM_ERROR("Failed to attach HDMI bridge\n"); + drm_mode_config_reset(fsl_dev->drm); drm_kms_helper_poll_init(fsl_dev->drm);
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h index 7093109..8915b07 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h @@ -29,5 +29,9 @@ int fsl_dcu_drm_connector_create(struct fsl_dcu_drm_device *fsl_dev, struct drm_encoder *encoder); int fsl_dcu_drm_encoder_create(struct fsl_dcu_drm_device *fsl_dev, struct drm_crtc *crtc); +int fsl_dcu_drm_hdmienc_attach_bridge(struct fsl_dcu_drm_device *fsl_dev); +int fsl_dcu_drm_hdmienc_create(struct fsl_dcu_drm_device *fsl_dev, + struct drm_crtc *crtc); +
#endif /* __FSL_DCU_DRM_CONNECTOR_H__ */
Add port node to DCU node using "OF graph" discription.
Signed-off-by: Alison Wang b18965@freescale.com Signed-off-by: Xiubo Li lixiubo@cmss.chinamobile.com Signed-off-by: Jianwei Wang jianwei.wang.chn@gmail.com Signed-off-by: Meng Yi meng.yi@nxp.com --- Documentation/devicetree/bindings/display/fsl,dcu.txt | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/fsl,dcu.txt b/Documentation/devicetree/bindings/display/fsl,dcu.txt index ae55cde..c973d94 100644 --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt @@ -17,6 +17,10 @@ Required properties: Optional properties: - fsl,tcon: The phandle to the timing controller node.
+Optional nodes: + +- Video port 0 for the HDMI output + Examples: dcu: dcu@2ce0000 { compatible = "fsl,ls1021a-dcu"; @@ -26,4 +30,9 @@ dcu: dcu@2ce0000 { big-endian; fsl,panel = <&panel>; fsl,tcon = <&tcon>; + port@0 { + dcu_out_hdmi: endpoint { + remote-endpoint = <&sii9022a_out>; + }; + }; };
On Tuesday 14 June 2016 17:20:36, Meng Yi wrote:
This patch creates another Encoder for HDMI port, and linking the Encoder to appropriate DRM bridge. And this Encoder using same CRTC with RGB-LCD. For RGB-LCD and HDMI using the same hardware connection to DCU, RGB-LCD panel should be unplugged when using the HDMI connection.
Signed-off-by: Alison Wang alison.wang@nxp.com Signed-off-by: Xiubo Li lixiubo@cmss.chinamobile.com Signed-off-by: Jianwei Wang jianwei.wang.chn@gmail.com Signed-off-by: Meng Yi meng.yi@nxp.com
Changes in V2: -remove unused headers inclusion -remove module declarations -fix error handling coding style -drop moulde parameters and auto detect HDMI connection relying on deviece tree -modified comment lines
drivers/gpu/drm/fsl-dcu/Makefile | 1 + drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_hdmi.c | 189 +++++++++++++++++++++++++++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 16 +++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h | 4 + 4 files changed, 210 insertions(+) create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_hdmi.c [...] +static struct +device_node *detect_hdmi_connection(struct fsl_dcu_drm_device *fsl_dev) +{
- struct device_node *remote_port;
- struct of_endpoint *ep;
- struct device_node *ep_node;
- int ret;
- struct device_node *parent = fsl_dev->dev->of_node;
- ep = devm_kzalloc(fsl_dev->dev,
sizeof(struct of_endpoint), GFP_KERNEL);- if (!ep)
return NULL;- ep_node = devm_kzalloc(fsl_dev->dev,
sizeof(struct device_node), GFP_KERNEL);- if (!ep_node)
return NULL;- ep_node = of_graph_get_next_endpoint(parent, NULL);
^^^^^^^ You overwrite the pointer to previously allocated memory.
- if (!ep_node)
goto error;- ret = of_graph_parse_endpoint(ep_node, ep);
- if (ret) {
of_node_put(ep_node);goto error;- }
- remote_port = of_graph_get_remote_port_parent(ep->local_node);
- if (!remote_port)
goto error;- return remote_port;
+error:
- devm_kfree(ep);
- devm_kfree(ep_node);
- return NULL;
+}
So, unless you hit the error label the memory allocated using devm_kzalloc stays around until the device is removed. I don't know DRM internals much, but can load (within drm_driver) be called multiple times during device lifetime? This would allocate memory each time it is called. Why not allocate 'ep' on stack while ep_node don't need allocation at all, no?
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c index c564ec6..658bc92 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c @@ -17,10 +17,18 @@ #include "fsl_dcu_drm_crtc.h" #include "fsl_dcu_drm_drv.h"
+static void fsl_dcu_drm_output_poll_changed(struct drm_device *dev) +{
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
- drm_fbdev_cma_hotplug_event(fsl_dev->fbdev);
+}
static const struct drm_mode_config_funcs fsl_dcu_drm_mode_config_funcs = { .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit, .fb_create = drm_fb_cma_create,
- .output_poll_changed = fsl_dcu_drm_output_poll_changed,
};
int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev) @@ -47,6 +55,14 @@ int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev) if (ret) goto fail_connector;
- ret = fsl_dcu_drm_hdmienc_create(fsl_dev, &fsl_dev->crtc);
- if (ret)
DRM_ERROR("Failed to create HDMI encoder\n");- ret = fsl_dcu_drm_hdmienc_attach_bridge(fsl_dev);
- if (ret)
DRM_ERROR("Failed to attach HDMI bridge\n");
Are those really errors? What about setups without any HDMI bridge at all? I would conside it an error if a bridge is given in device-tree but detecting or whatsoever fails for some reason.
Best regards, Alexander
Hi Alexander,
Thanks for your comments!
On Tuesday, June 14, 2016 5:54 PM, Alexander wrote:
+static struct +device_node *detect_hdmi_connection(struct fsl_dcu_drm_device +*fsl_dev) {
- struct device_node *remote_port;
- struct of_endpoint *ep;
- struct device_node *ep_node;
- int ret;
- struct device_node *parent = fsl_dev->dev->of_node;
- ep = devm_kzalloc(fsl_dev->dev,
sizeof(struct of_endpoint), GFP_KERNEL);- if (!ep)
return NULL;- ep_node = devm_kzalloc(fsl_dev->dev,
sizeof(struct device_node), GFP_KERNEL);- if (!ep_node)
return NULL;- ep_node = of_graph_get_next_endpoint(parent, NULL);
^^^^^^^You overwrite the pointer to previously allocated memory.
Woops! I will fix that.
- if (!ep_node)
goto error;- ret = of_graph_parse_endpoint(ep_node, ep);
- if (ret) {
of_node_put(ep_node);goto error;- }
- remote_port = of_graph_get_remote_port_parent(ep->local_node);
- if (!remote_port)
goto error;- return remote_port;
+error:
- devm_kfree(ep);
- devm_kfree(ep_node);
- return NULL;
+}
So, unless you hit the error label the memory allocated using devm_kzalloc stays around until the device is removed. I don't know DRM internals much, but can load (within drm_driver) be called multiple times during device lifetime?
I think load called once every drm_device registered.
This would allocate memory each time it is called. Why not allocate 'ep' on stack while ep_node don't need allocation at all, no?
Yep, allocate 'ep' on stack is better, and error label can be removed.
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c index c564ec6..658bc92 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c @@ -17,10 +17,18 @@ #include "fsl_dcu_drm_crtc.h" #include "fsl_dcu_drm_drv.h"
+static void fsl_dcu_drm_output_poll_changed(struct drm_device *dev) {
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
- drm_fbdev_cma_hotplug_event(fsl_dev->fbdev);
+}
static const struct drm_mode_config_funcs fsl_dcu_drm_mode_config_funcs = { .atomic_check =
drm_atomic_helper_check,
.atomic_commit = drm_atomic_helper_commit, .fb_create = drm_fb_cma_create,
- .output_poll_changed = fsl_dcu_drm_output_poll_changed,
};
int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev) @@ -47,6 +55,14 @@ int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev) if (ret) goto fail_connector;
- ret = fsl_dcu_drm_hdmienc_create(fsl_dev, &fsl_dev->crtc);
- if (ret)
DRM_ERROR("Failed to create HDMI encoder\n");- ret = fsl_dcu_drm_hdmienc_attach_bridge(fsl_dev);
- if (ret)
DRM_ERROR("Failed to attach HDMI bridge\n");Are those really errors? What about setups without any HDMI bridge at all? I
If device-tree is given, it just print error message, and HDMI don't display, that all. RGB-LCD can display normally.
If device-tree is not given, attach bridge will return when detecting HDMI connection. I think this should also be checked within " hdmienc_create ", or encoder will be allocated anyway. Will fix that next version.
would conside it an error if a bridge is given in device-tree but detecting or whatsoever fails for some reason.
Yep, I just think that two function's return value should be handled, so put the error message here. Maybe return value check should be dropped. What do you think?
Best Regards, Meng
On 2016-06-14 02:20, Meng Yi wrote:
This patch creates another Encoder for HDMI port, and linking the Encoder to appropriate DRM bridge. And this Encoder using same CRTC with RGB-LCD. For RGB-LCD and HDMI using the same hardware connection to DCU, RGB-LCD panel should be unplugged when using the HDMI connection.
Signed-off-by: Alison Wang alison.wang@nxp.com Signed-off-by: Xiubo Li lixiubo@cmss.chinamobile.com Signed-off-by: Jianwei Wang jianwei.wang.chn@gmail.com Signed-off-by: Meng Yi meng.yi@nxp.com
Changes in V2: -remove unused headers inclusion -remove module declarations -fix error handling coding style -drop moulde parameters and auto detect HDMI connection relying on deviece tree
-modified comment lines
drivers/gpu/drm/fsl-dcu/Makefile | 1 + drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_hdmi.c | 189 +++++++++++++++++++++++++++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 16 +++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h | 4 + 4 files changed, 210 insertions(+) create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_hdmi.c
Hi Meng,
I took a bit more time to understand what it is all about with this patchset and the DRM bridge infrastructure in general.
I agree with Boris Brezillions comment of the first version of this patchset: You "only" want to add DRM bridge support here, it doesn't has anything to do with HDMI. So there should not be a file called fsl_dcu_drm_hdmi.c for instance....
In fact, you should fold the DRM bridge code into fsl_dcu_drm_rgb.c. Have a look at Boris patch which added such support to the Atmel HLDCD driver: https://lists.freedesktop.org/archives/dri-devel/2016-January/098050.html
However, the HLDCD driver already supported the endpoint syntax for panels at that time, which is not the case for the DCU driver.
Panel support with endpoint DT syntax seems to be more generic and is a easier first step towards DRM bridge support. Hence I suggest to take this two steps:
1. Extend the current code to support panels through endpoint syntax (of_graph..) 2. Extend the endpoint support to also support DRM bridges
During 1 you should be careful to not remove the old fsl,panel support (backward compatibility). But it shouldn't be a big deal, just check if the fsl,panel property is there, if it is, do not parse the endpoints. Such backward compatibility has been implemented here: https://patchwork.kernel.org/patch/7706481/
-- Stefan
diff --git a/drivers/gpu/drm/fsl-dcu/Makefile b/drivers/gpu/drm/fsl-dcu/Makefile index b35a292..12e2245 100644 --- a/drivers/gpu/drm/fsl-dcu/Makefile +++ b/drivers/gpu/drm/fsl-dcu/Makefile @@ -1,6 +1,7 @@ fsl-dcu-drm-y := fsl_dcu_drm_drv.o \ fsl_dcu_drm_kms.o \ fsl_dcu_drm_rgb.o \
fsl_dcu_drm_hdmi.o \diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_hdmi.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_hdmi.c new file mode 100644 index 0000000..f567534 --- /dev/null +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_hdmi.c @@ -0,0 +1,189 @@ +/*
- Copyright 2016 NXP Semiconductor, Inc.
- NXP DCU drm device driver
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- */
+#include <linux/errno.h> +#include <linux/kernel.h> +#include <linux/of_graph.h>
+#include <drm/drmP.h> +#include <drm/drm_crtc_helper.h>
+#include "fsl_dcu_drm_drv.h" +#include "fsl_dcu_drm_output.h" +#include "fsl_tcon.h"
+static void +fsl_dcu_drm_hdmienc_mode_set(struct drm_encoder *encoder,
struct drm_display_mode *mode,struct drm_display_mode *adjusted_mode)+{
- /*TODO*/
+}
+static int +fsl_dcu_drm_hdmienc_atomic_check(struct drm_encoder *encoder,
struct drm_crtc_state *crtc_state,struct drm_connector_state *conn_state)+{
- return 0;
+}
+static void +fsl_dcu_drm_hdmienc_disable(struct drm_encoder *encoder) +{
- struct drm_device *dev = encoder->dev;
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
- if (fsl_dev->tcon)
fsl_tcon_bypass_disable(fsl_dev->tcon);+}
+static void +fsl_dcu_drm_hdmienc_enable(struct drm_encoder *encoder) +{
- struct drm_device *dev = encoder->dev;
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
- if (fsl_dev->tcon)
fsl_tcon_bypass_enable(fsl_dev->tcon);+}
+static const struct +drm_encoder_helper_funcs encoder_helper_funcs = {
- .atomic_check = fsl_dcu_drm_hdmienc_atomic_check,
- .disable = fsl_dcu_drm_hdmienc_disable,
- .enable = fsl_dcu_drm_hdmienc_enable,
- .mode_set = fsl_dcu_drm_hdmienc_mode_set,
+};
+static void +fsl_dcu_drm_hdmienc_destroy(struct drm_encoder *encoder) +{
- drm_encoder_cleanup(encoder);
+}
+static const struct +drm_encoder_funcs encoder_funcs = {
- .destroy = fsl_dcu_drm_hdmienc_destroy,
+};
+int fsl_dcu_drm_hdmienc_create(struct fsl_dcu_drm_device *fsl_dev,
struct drm_crtc *crtc)+{
- struct drm_encoder *encoder;
- int ret;
- encoder = devm_kzalloc(fsl_dev->dev,
sizeof(struct drm_encoder), GFP_KERNEL);- if (!encoder)
return -ENOMEM;- encoder->possible_crtcs = 1;
- ret = drm_encoder_init(fsl_dev->drm, encoder, &encoder_funcs,
DRM_MODE_ENCODER_TMDS, NULL);- if (ret)
goto fail_encoder;- drm_encoder_helper_add(encoder, &encoder_helper_funcs);
- encoder->crtc = crtc;
- return 0;
+fail_encoder:
- devm_kfree(fsl_dev->dev, encoder);
- return ret;
+}
+static struct +drm_encoder *fsl_dcu_drm_hdmi_find_encoder(struct drm_device *dev) +{
- struct drm_encoder *encoder;
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS)return encoder;- }
- return NULL;
+}
+static struct +device_node *detect_hdmi_connection(struct fsl_dcu_drm_device *fsl_dev) +{
- struct device_node *remote_port;
- struct of_endpoint *ep;
- struct device_node *ep_node;
- int ret;
- struct device_node *parent = fsl_dev->dev->of_node;
- ep = devm_kzalloc(fsl_dev->dev,
sizeof(struct of_endpoint), GFP_KERNEL);- if (!ep)
return NULL;- ep_node = devm_kzalloc(fsl_dev->dev,
sizeof(struct device_node), GFP_KERNEL);- if (!ep_node)
return NULL;- ep_node = of_graph_get_next_endpoint(parent, NULL);
- if (!ep_node)
goto error;- ret = of_graph_parse_endpoint(ep_node, ep);
- if (ret) {
of_node_put(ep_node);goto error;- }
- remote_port = of_graph_get_remote_port_parent(ep->local_node);
- if (!remote_port)
goto error;- return remote_port;
+error:
- devm_kfree(ep);
- devm_kfree(ep_node);
- return NULL;
+}
+int fsl_dcu_drm_hdmienc_attach_bridge(struct fsl_dcu_drm_device *fsl_dev) +{
- struct drm_device *drm_dev = fsl_dev->drm;
- struct drm_encoder *encoder;
- struct drm_bridge *bridge;
- struct device_node *remote_port;
- int ret;
- remote_port = detect_hdmi_connection(fsl_dev);
- if (!remote_port)
return -ENODEV;- bridge = of_drm_find_bridge(remote_port);
- if (!bridge)
return -ENODEV;- encoder = fsl_dcu_drm_hdmi_find_encoder(drm_dev);
- if (!encoder)
return -ENODEV;- encoder->bridge = bridge;
- bridge->encoder = encoder;
- ret = drm_bridge_attach(drm_dev, bridge);
- if (ret)
goto error;- return 0;
+error:
- encoder->funcs->destroy(encoder);
- return ret;
+} diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c index c564ec6..658bc92 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c @@ -17,10 +17,18 @@ #include "fsl_dcu_drm_crtc.h" #include "fsl_dcu_drm_drv.h"
+static void fsl_dcu_drm_output_poll_changed(struct drm_device *dev) +{
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
- drm_fbdev_cma_hotplug_event(fsl_dev->fbdev);
+}
static const struct drm_mode_config_funcs fsl_dcu_drm_mode_config_funcs = { .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit, .fb_create = drm_fb_cma_create,
- .output_poll_changed = fsl_dcu_drm_output_poll_changed,
};
int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev) @@ -47,6 +55,14 @@ int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev) if (ret) goto fail_connector;
- ret = fsl_dcu_drm_hdmienc_create(fsl_dev, &fsl_dev->crtc);
- if (ret)
DRM_ERROR("Failed to create HDMI encoder\n");- ret = fsl_dcu_drm_hdmienc_attach_bridge(fsl_dev);
- if (ret)
DRM_ERROR("Failed to attach HDMI bridge\n");- drm_mode_config_reset(fsl_dev->drm); drm_kms_helper_poll_init(fsl_dev->drm);
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h index 7093109..8915b07 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h @@ -29,5 +29,9 @@ int fsl_dcu_drm_connector_create(struct fsl_dcu_drm_device *fsl_dev, struct drm_encoder *encoder); int fsl_dcu_drm_encoder_create(struct fsl_dcu_drm_device *fsl_dev, struct drm_crtc *crtc); +int fsl_dcu_drm_hdmienc_attach_bridge(struct fsl_dcu_drm_device *fsl_dev); +int fsl_dcu_drm_hdmienc_create(struct fsl_dcu_drm_device *fsl_dev,
struct drm_crtc *crtc);#endif /* __FSL_DCU_DRM_CONNECTOR_H__ */
Hi Stefan,
Thank for your reply.
I took a bit more time to understand what it is all about with this patchset and the DRM bridge infrastructure in general.
I agree with Boris Brezillions comment of the first version of this patchset: You "only" want to add DRM bridge support here, it doesn't has anything to do with HDMI. So there should not be a file called fsl_dcu_drm_hdmi.c for instance....
In fact, you should fold the DRM bridge code into fsl_dcu_drm_rgb.c. Have a look at Boris patch which added such support to the Atmel HLDCD driver: https://lists.freedesktop.org/archives/dri-devel/2016-January/098050.html
However, the HLDCD driver already supported the endpoint syntax for panels at that time, which is not the case for the DCU driver.
Panel support with endpoint DT syntax seems to be more generic and is a easier first step towards DRM bridge support. Hence I suggest to take this two steps:
- Extend the current code to support panels through endpoint syntax
(of_graph..) 2. Extend the endpoint support to also support DRM bridges
During 1 you should be careful to not remove the old fsl,panel support (backward compatibility). But it shouldn't be a big deal, just check if the fsl,panel property is there, if it is, do not parse the endpoints. Such backward compatibility has been implemented here: https://patchwork.kernel.org/patch/7706481/
That's more reasonable. Thank much for your information and guidance! I will working on that and send the patch later.
Best Regards, Meng Yi
dri-devel@lists.freedesktop.org
-
Alexander Stein -
Meng Yi -
Stefan Agner