Re: [PATCH 1/2] drm:fsl-dcu: rework codes to support of_graph dt binding for panel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Use "drm/fsl-dcu:" in the subject, that is what we commonly used.

On 2016-06-24 02:00, 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@xxxxxxx>
> ---
>  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    | 76 ++++++++++++++++++++--------
>  3 files changed, 57 insertions(+), 24 deletions(-)

I tested the patch with the old and new Syntax on a Colibri module,
seems to work fine. From a 210 lines patchset to a patchset which adds
barley 47 lines net... Nice work!

Please update the binding documentation with the new syntax, and mention
that fsl,panel property is deprecated.

Also add a patch which updates the existing device trees using
port/endpoints.


> 
> 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..57a030b 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> @@ -15,6 +15,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_panel.h>
> +#include <linux/of_graph.h>

Add that some lines above, where the other linux/ includes are.

>  
>  #include "fsl_dcu_drm_drv.h"
>  #include "fsl_tcon.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,

There is a superfluous space after fsl_dcu_drm_device.

> +				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,58 @@ 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 -EPROBE_DEFER;

I don't think we should defer probing here, if attaching a panel fails,
it is an actual error.

> +	}
> +
> +	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;
> +	struct device_node *parent = fsl_dev->dev->of_node;

Currently, fsl_dev->dev->of_node and fsl_dev->np are pointers to the
same of node. Use fsl_dev->np consistently in the code below.

> +
> +	/*This is for the backward compatibility*/

Add a space after /* and before */

> +	panel_node = of_parse_phandle(fsl_dev->np, "fsl,panel", 0);
> +	if (panel_node) {
> +		fsl_dev->connector.panel = of_drm_find_panel(panel_node);

Check if fsl_dev->connector.panel is null here, if it is null, return
EPROBE_DEFER (as we did before).

--
Stefan

> +		of_node_put(panel_node);
> +		fsl_dcu_attach_panel(fsl_dev, fsl_dev->connector.panel);
> +		return 0;
> +	}
> +
> +	ep_node = of_graph_get_next_endpoint(parent, 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;
> +}
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux