| Message ID | 1467106325-33861-1-git-send-email-meng.yi@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Stefan, Those two patches actually can be merged independently. Do you have some more comments? Best Regards, Meng > -----Original Message----- > From: Meng Yi [mailto:meng.yi@nxp.com] > Sent: Tuesday, June 28, 2016 5:32 PM > To: stefan@agner.ch > Cc: airlied@linux.ie; emil.l.velikov@gmail.com; jianwei.wang.chn@gmail.com; > alexander.stein@systec-electronic.com; dri-devel@lists.freedesktop.org; > Meng Yi <meng.yi@nxp.com> > Subject: [PATCH v2 1/2] drm/fsl-dcu: rework codes to support of_graph dt > binding for panel > > This patch rework the output code to add of_graph dt binding support for > panel device and also keeps the backward compatibility > > Signed-off-by: Meng Yi <meng.yi@nxp.com> > --- > Changes in V2: > -fix some coding style issue > -add fsl_dev->connector.panel check > -use fsl_dev->np and drop fsl_dev->dev->of_node -return 'ret' when > fsl_dcu_attach_panel failed > --- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 2 +- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h | 3 +- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 79 ++++++++++++++++++++-- > ------ > 3 files changed, 60 insertions(+), 24 deletions(-) > > 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..b48ffa7 100644 > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c > @@ -43,7 +43,7 @@ int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device > *fsl_dev) > if (ret) > goto fail_encoder; > > - ret = fsl_dcu_drm_connector_create(fsl_dev, &fsl_dev->encoder); > + ret = fsl_dcu_create_outputs(fsl_dev); > if (ret) > goto fail_connector; > > 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..5a7b88e 100644 > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h > @@ -25,9 +25,8 @@ to_fsl_dcu_connector(struct drm_connector *con) > : NULL; > } > > -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_create_outputs(struct fsl_dcu_drm_device *fsl_dev); > > #endif /* __FSL_DCU_DRM_CONNECTOR_H__ */ diff --git > a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl- > dcu/fsl_dcu_drm_rgb.c > index 98c998d..b23cc58 100644 > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c > @@ -10,6 +10,7 @@ > */ > > #include <linux/backlight.h> > +#include <linux/of_graph.h> > > #include <drm/drmP.h> > #include <drm/drm_atomic_helper.h> > @@ -141,15 +142,15 @@ static const struct drm_connector_helper_funcs > connector_helper_funcs = { > .mode_valid = fsl_dcu_drm_connector_mode_valid, }; > > -int fsl_dcu_drm_connector_create(struct fsl_dcu_drm_device *fsl_dev, > - struct drm_encoder *encoder) > +static int fsl_dcu_attach_panel(struct fsl_dcu_drm_device *fsl_dev, > + struct drm_panel *panel) > { > + struct drm_encoder *encoder = &fsl_dev->encoder; > struct drm_connector *connector = &fsl_dev->connector.base; > struct drm_mode_config *mode_config = &fsl_dev->drm- > >mode_config; > - struct device_node *panel_node; > int ret; > > - fsl_dev->connector.encoder = encoder; > + fsl_dev->connector.encoder = &fsl_dev->encoder; > > ret = drm_connector_init(fsl_dev->drm, connector, > &fsl_dcu_drm_connector_funcs, > @@ -170,21 +171,7 @@ int fsl_dcu_drm_connector_create(struct > fsl_dcu_drm_device *fsl_dev, > mode_config->dpms_property, > DRM_MODE_DPMS_OFF); > > - panel_node = of_parse_phandle(fsl_dev->np, "fsl,panel", 0); > - if (!panel_node) { > - dev_err(fsl_dev->dev, "fsl,panel property not found\n"); > - ret = -ENODEV; > - goto err_sysfs; > - } > - > - fsl_dev->connector.panel = of_drm_find_panel(panel_node); > - if (!fsl_dev->connector.panel) { > - ret = -EPROBE_DEFER; > - goto err_panel; > - } > - of_node_put(panel_node); > - > - ret = drm_panel_attach(fsl_dev->connector.panel, connector); > + ret = drm_panel_attach(panel, connector); > if (ret) { > dev_err(fsl_dev->dev, "failed to attach panel\n"); > goto err_sysfs; > @@ -192,11 +179,61 @@ int fsl_dcu_drm_connector_create(struct > fsl_dcu_drm_device *fsl_dev, > > return 0; > > -err_panel: > - of_node_put(panel_node); > err_sysfs: > drm_connector_unregister(connector); > err_cleanup: > drm_connector_cleanup(connector); > return ret; > } > + > +static int fsl_dcu_attach_endpoint(struct fsl_dcu_drm_device *fsl_dev, > + const struct of_endpoint *ep) > +{ > + struct device_node *np; > + int ret; > + > + np = of_graph_get_remote_port_parent(ep->local_node); > + > + fsl_dev->connector.panel = of_drm_find_panel(np); > + if (fsl_dev->connector.panel) { > + of_node_put(np); > + ret = fsl_dcu_attach_panel(fsl_dev, fsl_dev->connector.panel); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +int fsl_dcu_create_outputs(struct fsl_dcu_drm_device *fsl_dev) { > + struct of_endpoint ep; > + struct device_node *ep_node, *panel_node; > + int ret; > + > + /* This is for the backward compatibility */ > + panel_node = of_parse_phandle(fsl_dev->np, "fsl,panel", 0); > + if (panel_node) { > + fsl_dev->connector.panel = of_drm_find_panel(panel_node); > + of_node_put(panel_node); > + if (!fsl_dev->connector.panel) > + return -EPROBE_DEFER; > + ret = fsl_dcu_attach_panel(fsl_dev, fsl_dev->connector.panel); > + if (ret) > + return ret; > + return 0; > + } > + > + ep_node = of_graph_get_next_endpoint(fsl_dev->np, NULL); > + if (!ep_node) > + return -ENODEV; > + > + ret = of_graph_parse_endpoint(ep_node, &ep); > + if (ret) { > + of_node_put(ep_node); > + return -ENODEV; > + } > + fsl_dcu_attach_endpoint(fsl_dev, &ep); > + > + return 0; > +} > -- > 2.1.0.27.g96db324
On 2016-06-28 02:32, Meng Yi wrote: > This patch rework the output code to add of_graph dt binding support > for panel device and also keeps the backward compatibility > > Signed-off-by: Meng Yi <meng.yi@nxp.com> > --- > Changes in V2: > -fix some coding style issue > -add fsl_dev->connector.panel check > -use fsl_dev->np and drop fsl_dev->dev->of_node > -return 'ret' when fsl_dcu_attach_panel failed > --- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 2 +- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h | 3 +- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 79 ++++++++++++++++++++-------- > 3 files changed, 60 insertions(+), 24 deletions(-) > > 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..b48ffa7 100644 > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c > @@ -43,7 +43,7 @@ int fsl_dcu_drm_modeset_init(struct > fsl_dcu_drm_device *fsl_dev) > if (ret) > goto fail_encoder; > > - ret = fsl_dcu_drm_connector_create(fsl_dev, &fsl_dev->encoder); > + ret = fsl_dcu_create_outputs(fsl_dev); > if (ret) > goto fail_connector; > > 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..5a7b88e 100644 > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h > @@ -25,9 +25,8 @@ to_fsl_dcu_connector(struct drm_connector *con) > : NULL; > } > > -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_create_outputs(struct fsl_dcu_drm_device *fsl_dev); > > #endif /* __FSL_DCU_DRM_CONNECTOR_H__ */ > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c > b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c > index 98c998d..b23cc58 100644 > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c > @@ -10,6 +10,7 @@ > */ > > #include <linux/backlight.h> > +#include <linux/of_graph.h> > > #include <drm/drmP.h> > #include <drm/drm_atomic_helper.h> > @@ -141,15 +142,15 @@ static const struct drm_connector_helper_funcs > connector_helper_funcs = { > .mode_valid = fsl_dcu_drm_connector_mode_valid, > }; > > -int fsl_dcu_drm_connector_create(struct fsl_dcu_drm_device *fsl_dev, > - struct drm_encoder *encoder) > +static int fsl_dcu_attach_panel(struct fsl_dcu_drm_device *fsl_dev, > + struct drm_panel *panel) Align the second line with the beginning of the arguments of the first line (using tabs first, and if necessary spaces). > { > + struct drm_encoder *encoder = &fsl_dev->encoder; > struct drm_connector *connector = &fsl_dev->connector.base; > struct drm_mode_config *mode_config = &fsl_dev->drm->mode_config; > - struct device_node *panel_node; > int ret; > > - fsl_dev->connector.encoder = encoder; > + fsl_dev->connector.encoder = &fsl_dev->encoder; You don't need to change this line since you created a local variable above. > > ret = drm_connector_init(fsl_dev->drm, connector, > &fsl_dcu_drm_connector_funcs, > @@ -170,21 +171,7 @@ int fsl_dcu_drm_connector_create(struct > fsl_dcu_drm_device *fsl_dev, > mode_config->dpms_property, > DRM_MODE_DPMS_OFF); > > - panel_node = of_parse_phandle(fsl_dev->np, "fsl,panel", 0); > - if (!panel_node) { > - dev_err(fsl_dev->dev, "fsl,panel property not found\n"); > - ret = -ENODEV; > - goto err_sysfs; > - } > - > - fsl_dev->connector.panel = of_drm_find_panel(panel_node); > - if (!fsl_dev->connector.panel) { > - ret = -EPROBE_DEFER; > - goto err_panel; > - } > - of_node_put(panel_node); > - > - ret = drm_panel_attach(fsl_dev->connector.panel, connector); > + ret = drm_panel_attach(panel, connector); > if (ret) { > dev_err(fsl_dev->dev, "failed to attach panel\n"); > goto err_sysfs; > @@ -192,11 +179,61 @@ int fsl_dcu_drm_connector_create(struct > fsl_dcu_drm_device *fsl_dev, > > return 0; > > -err_panel: > - of_node_put(panel_node); > err_sysfs: > drm_connector_unregister(connector); > err_cleanup: > drm_connector_cleanup(connector); > return ret; > } > + > +static int fsl_dcu_attach_endpoint(struct fsl_dcu_drm_device *fsl_dev, > + const struct of_endpoint *ep) > +{ > + struct device_node *np; > + int ret; > + > + np = of_graph_get_remote_port_parent(ep->local_node); > + > + fsl_dev->connector.panel = of_drm_find_panel(np); > + if (fsl_dev->connector.panel) { > + of_node_put(np); After using np we need to put np unconditionally since we don't use it (yet). Moved that line one line up. > + ret = fsl_dcu_attach_panel(fsl_dev, fsl_dev->connector.panel); > + if (ret) > + return ret; You should return whatever fsl_dcu_attach_panel returns here and... > + } > + > + return 0; ... return -ENODEV so that the driver doesn't load when no panel has been found. > +} > + > +int fsl_dcu_create_outputs(struct fsl_dcu_drm_device *fsl_dev) > +{ > + struct of_endpoint ep; > + struct device_node *ep_node, *panel_node; > + int ret; > + > + /* This is for the backward compatibility */ Remove "the" > + panel_node = of_parse_phandle(fsl_dev->np, "fsl,panel", 0); > + if (panel_node) { > + fsl_dev->connector.panel = of_drm_find_panel(panel_node); > + of_node_put(panel_node); > + if (!fsl_dev->connector.panel) > + return -EPROBE_DEFER; > + ret = fsl_dcu_attach_panel(fsl_dev, fsl_dev->connector.panel); > + if (ret) > + return ret; > + return 0; Same here, just return whatever fsl_dcu_attach_panel returns. > + } > + > + ep_node = of_graph_get_next_endpoint(fsl_dev->np, NULL); > + if (!ep_node) > + return -ENODEV; > + > + ret = of_graph_parse_endpoint(ep_node, &ep); > + if (ret) { > + of_node_put(ep_node); > + return -ENODEV; > + } > + fsl_dcu_attach_endpoint(fsl_dev, &ep); Directly return the return value of fsl_dcu_attach_endpoint here. -- Stefan > + > + return 0; > +}
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..b48ffa7 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c @@ -43,7 +43,7 @@ int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev) if (ret) goto fail_encoder; - ret = fsl_dcu_drm_connector_create(fsl_dev, &fsl_dev->encoder); + ret = fsl_dcu_create_outputs(fsl_dev); if (ret) goto fail_connector; 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..5a7b88e 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h @@ -25,9 +25,8 @@ to_fsl_dcu_connector(struct drm_connector *con) : NULL; } -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_create_outputs(struct fsl_dcu_drm_device *fsl_dev); #endif /* __FSL_DCU_DRM_CONNECTOR_H__ */ diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c index 98c998d..b23cc58 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c @@ -10,6 +10,7 @@ */ #include <linux/backlight.h> +#include <linux/of_graph.h> #include <drm/drmP.h> #include <drm/drm_atomic_helper.h> @@ -141,15 +142,15 @@ static const struct drm_connector_helper_funcs connector_helper_funcs = { .mode_valid = fsl_dcu_drm_connector_mode_valid, }; -int fsl_dcu_drm_connector_create(struct fsl_dcu_drm_device *fsl_dev, - struct drm_encoder *encoder) +static int fsl_dcu_attach_panel(struct fsl_dcu_drm_device *fsl_dev, + struct drm_panel *panel) { + struct drm_encoder *encoder = &fsl_dev->encoder; struct drm_connector *connector = &fsl_dev->connector.base; struct drm_mode_config *mode_config = &fsl_dev->drm->mode_config; - struct device_node *panel_node; int ret; - fsl_dev->connector.encoder = encoder; + fsl_dev->connector.encoder = &fsl_dev->encoder; ret = drm_connector_init(fsl_dev->drm, connector, &fsl_dcu_drm_connector_funcs, @@ -170,21 +171,7 @@ int fsl_dcu_drm_connector_create(struct fsl_dcu_drm_device *fsl_dev, mode_config->dpms_property, DRM_MODE_DPMS_OFF); - panel_node = of_parse_phandle(fsl_dev->np, "fsl,panel", 0); - if (!panel_node) { - dev_err(fsl_dev->dev, "fsl,panel property not found\n"); - ret = -ENODEV; - goto err_sysfs; - } - - fsl_dev->connector.panel = of_drm_find_panel(panel_node); - if (!fsl_dev->connector.panel) { - ret = -EPROBE_DEFER; - goto err_panel; - } - of_node_put(panel_node); - - ret = drm_panel_attach(fsl_dev->connector.panel, connector); + ret = drm_panel_attach(panel, connector); if (ret) { dev_err(fsl_dev->dev, "failed to attach panel\n"); goto err_sysfs; @@ -192,11 +179,61 @@ int fsl_dcu_drm_connector_create(struct fsl_dcu_drm_device *fsl_dev, return 0; -err_panel: - of_node_put(panel_node); err_sysfs: drm_connector_unregister(connector); err_cleanup: drm_connector_cleanup(connector); return ret; } + +static int fsl_dcu_attach_endpoint(struct fsl_dcu_drm_device *fsl_dev, + const struct of_endpoint *ep) +{ + struct device_node *np; + int ret; + + np = of_graph_get_remote_port_parent(ep->local_node); + + fsl_dev->connector.panel = of_drm_find_panel(np); + if (fsl_dev->connector.panel) { + of_node_put(np); + ret = fsl_dcu_attach_panel(fsl_dev, fsl_dev->connector.panel); + if (ret) + return ret; + } + + return 0; +} + +int fsl_dcu_create_outputs(struct fsl_dcu_drm_device *fsl_dev) +{ + struct of_endpoint ep; + struct device_node *ep_node, *panel_node; + int ret; + + /* This is for the backward compatibility */ + panel_node = of_parse_phandle(fsl_dev->np, "fsl,panel", 0); + if (panel_node) { + fsl_dev->connector.panel = of_drm_find_panel(panel_node); + of_node_put(panel_node); + if (!fsl_dev->connector.panel) + return -EPROBE_DEFER; + ret = fsl_dcu_attach_panel(fsl_dev, fsl_dev->connector.panel); + if (ret) + return ret; + return 0; + } + + ep_node = of_graph_get_next_endpoint(fsl_dev->np, NULL); + if (!ep_node) + return -ENODEV; + + ret = of_graph_parse_endpoint(ep_node, &ep); + if (ret) { + of_node_put(ep_node); + return -ENODEV; + } + fsl_dcu_attach_endpoint(fsl_dev, &ep); + + return 0; +}
This patch rework the output code to add of_graph dt binding support for panel device and also keeps the backward compatibility Signed-off-by: Meng Yi <meng.yi@nxp.com> --- Changes in V2: -fix some coding style issue -add fsl_dev->connector.panel check -use fsl_dev->np and drop fsl_dev->dev->of_node -return 'ret' when fsl_dcu_attach_panel failed --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h | 3 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 79 ++++++++++++++++++++-------- 3 files changed, 60 insertions(+), 24 deletions(-)