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