[PATCH v2] drm/imx: Match imx-ipuv3-crtc components using device node in platform data

[PATCH v2] drm/imx: Match imx-ipuv3-crtc components using device node in platform data

[Philipp Zabel]

The component master driver imx-drm-core matches component devices using
their of_node. Since commit 950b410dd1ab ("gpu: ipu-v3: Fix imx-ipuv3-crtc
module autoloading"), the imx-ipuv3-crtc dev->of_node is not set during
probing. Before that, of_node was set and caused an of: modalias to be
used instead of the platform: modalias, which broke module autoloading.

On the other hand, if dev->of_node is not set yet when the imx-ipuv3-crtc
probe function calls component_add, component matching in imx-drm-core
fails. While dev->of_node will be set once the next component tries to
bring up the component master, imx-drm-core component binding will never
succeed if one of the crtc devices is probed last.

Add of_node to the component platform data and match against the
pdata->of_node instead of dev->of_node in imx-drm-core to work around
this problem.

Cc: <stable@vger.kernel.org> # 4.4.x
Fixes: 950b410dd1ab ("gpu: ipu-v3: Fix imx-ipuv3-crtc module autoloading")
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v1:
 - Also set the crtc->port to the of_node from platform data
---
 drivers/gpu/drm/imx/imx-drm-core.c | 8 ++++++++
 drivers/gpu/drm/imx/ipuv3-crtc.c   | 2 +-
 drivers/gpu/ipu-v3/ipu-common.c    | 5 +++--
 include/video/imx-ipu-v3.h         | 2 ++
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index e26dcde..05229b9 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -25,6 +25,7 @@
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_of.h>
+#include <video/imx-ipu-v3.h>
 
 #include "imx-drm.h"
 
@@ -444,6 +445,13 @@ static int compare_of(struct device *dev, void *data)
 {
 	struct device_node *np = data;
 
+	/* Special case for DI, dev->of_node may not be set yet */
+	if (strcmp(dev->driver->name, "imx-ipuv3-crtc") == 0) {
+		struct ipu_client_platformdata *pdata = dev->platform_data;
+
+		return pdata->of_node == np;
+	}
+
 	/* Special case for LDB, one device for two channels */
 	if (of_node_cmp(np->name, "lvds-channel") == 0) {
 		np = of_get_parent(np);
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index dee8e8b..b2c30b8 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -473,7 +473,7 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
 
 	ret = imx_drm_add_crtc(drm, &ipu_crtc->base, &ipu_crtc->imx_crtc,
 			&ipu_crtc->plane[0]->base, &ipu_crtc_helper_funcs,
-			ipu_crtc->dev->of_node);
+			pdata->of_node);
 	if (ret) {
 		dev_err(ipu_crtc->dev, "adding crtc failed with %d.\n", ret);
 		goto err_put_resources;
diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
index abb98c7..99dcacf 100644
--- a/drivers/gpu/ipu-v3/ipu-common.c
+++ b/drivers/gpu/ipu-v3/ipu-common.c
@@ -997,7 +997,7 @@ struct ipu_platform_reg {
 };
 
 /* These must be in the order of the corresponding device tree port nodes */
-static const struct ipu_platform_reg client_reg[] = {
+static struct ipu_platform_reg client_reg[] = {
 	{
 		.pdata = {
 			.csi = 0,
@@ -1048,7 +1048,7 @@ static int ipu_add_client_devices(struct ipu_soc *ipu, unsigned long ipu_base)
 	mutex_unlock(&ipu_client_id_mutex);
 
 	for (i = 0; i < ARRAY_SIZE(client_reg); i++) {
-		const struct ipu_platform_reg *reg = &client_reg[i];
+		struct ipu_platform_reg *reg = &client_reg[i];
 		struct platform_device *pdev;
 		struct device_node *of_node;
 
@@ -1070,6 +1070,7 @@ static int ipu_add_client_devices(struct ipu_soc *ipu, unsigned long ipu_base)
 
 		pdev->dev.parent = dev;
 
+		reg->pdata.of_node = of_node;
 		ret = platform_device_add_data(pdev, &reg->pdata,
 					       sizeof(reg->pdata));
 		if (!ret)
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index ad66589..3a2a794 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -16,6 +16,7 @@
 #include <linux/videodev2.h>
 #include <linux/bitmap.h>
 #include <linux/fb.h>
+#include <linux/of.h>
 #include <media/v4l2-mediabus.h>
 #include <video/videomode.h>
 
@@ -345,6 +346,7 @@ struct ipu_client_platformdata {
 	int dc;
 	int dp;
 	int dma[2];
+	struct device_node *of_node;
 };
 
 #endif /* __DRM_IPU_H__ */
-- 
2.8.0.rc3

Re: [PATCH v2] drm/imx: Match imx-ipuv3-crtc components using device node in platform data

[Fabio Estevam]

On Wed, May 18, 2016 at 9:56 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> The component master driver imx-drm-core matches component devices using
> their of_node. Since commit 950b410dd1ab ("gpu: ipu-v3: Fix imx-ipuv3-crtc
> module autoloading"), the imx-ipuv3-crtc dev->of_node is not set during
> probing. Before that, of_node was set and caused an of: modalias to be
> used instead of the platform: modalias, which broke module autoloading.
>
> On the other hand, if dev->of_node is not set yet when the imx-ipuv3-crtc
> probe function calls component_add, component matching in imx-drm-core
> fails. While dev->of_node will be set once the next component tries to
> bring up the component master, imx-drm-core component binding will never
> succeed if one of the crtc devices is probed last.
>
> Add of_node to the component platform data and match against the
> pdata->of_node instead of dev->of_node in imx-drm-core to work around
> this problem.
>
> Cc: <stable@vger.kernel.org> # 4.4.x
> Fixes: 950b410dd1ab ("gpu: ipu-v3: Fix imx-ipuv3-crtc module autoloading")
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

Thanks, Philipp. With this patch HDMI is functional again:

Tested-by: Fabio Estevam <fabio.estevam@nxp.com>

Re: [PATCH v2] drm/imx: Match imx-ipuv3-crtc components using device node in platform data

[Lothar Waßmann]

Hi,

On Wed, 18 May 2016 14:56:40 +0200 Philipp Zabel wrote:
> The component master driver imx-drm-core matches component devices using
> their of_node. Since commit 950b410dd1ab ("gpu: ipu-v3: Fix imx-ipuv3-crtc
> module autoloading"), the imx-ipuv3-crtc dev->of_node is not set during
> probing. Before that, of_node was set and caused an of: modalias to be
> used instead of the platform: modalias, which broke module autoloading.
> 
> On the other hand, if dev->of_node is not set yet when the imx-ipuv3-crtc
> probe function calls component_add, component matching in imx-drm-core
> fails. While dev->of_node will be set once the next component tries to
> bring up the component master, imx-drm-core component binding will never
> succeed if one of the crtc devices is probed last.
> 
> Add of_node to the component platform data and match against the
> pdata->of_node instead of dev->of_node in imx-drm-core to work around
> this problem.
> 
> Cc: <stable@vger.kernel.org> # 4.4.x
> Fixes: 950b410dd1ab ("gpu: ipu-v3: Fix imx-ipuv3-crtc module autoloading")
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> Changes since v1:
>  - Also set the crtc->port to the of_node from platform data
> ---
>  drivers/gpu/drm/imx/imx-drm-core.c | 8 ++++++++
>  drivers/gpu/drm/imx/ipuv3-crtc.c   | 2 +-
>  drivers/gpu/ipu-v3/ipu-common.c    | 5 +++--
>  include/video/imx-ipu-v3.h         | 2 ++
>  4 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> index e26dcde..05229b9 100644
> --- a/drivers/gpu/drm/imx/imx-drm-core.c
> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> @@ -25,6 +25,7 @@
>  #include <drm/drm_fb_cma_helper.h>
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drm_of.h>
> +#include <video/imx-ipu-v3.h>
>  
>  #include "imx-drm.h"
>  
> @@ -444,6 +445,13 @@ static int compare_of(struct device *dev, void *data)
>  {
>  	struct device_node *np = data;
>  
> +	/* Special case for DI, dev->of_node may not be set yet */
> +	if (strcmp(dev->driver->name, "imx-ipuv3-crtc") == 0) {
> +		struct ipu_client_platformdata *pdata = dev->platform_data;
> +
> +		return pdata->of_node == np;
> +	}
> +
>  	/* Special case for LDB, one device for two channels */
>  	if (of_node_cmp(np->name, "lvds-channel") == 0) {
>  		np = of_get_parent(np);
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index dee8e8b..b2c30b8 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -473,7 +473,7 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
>  
>  	ret = imx_drm_add_crtc(drm, &ipu_crtc->base, &ipu_crtc->imx_crtc,
>  			&ipu_crtc->plane[0]->base, &ipu_crtc_helper_funcs,
> -			ipu_crtc->dev->of_node);
> +			pdata->of_node);
>  	if (ret) {
>  		dev_err(ipu_crtc->dev, "adding crtc failed with %d.\n", ret);
>  		goto err_put_resources;
> diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
> index abb98c7..99dcacf 100644
> --- a/drivers/gpu/ipu-v3/ipu-common.c
> +++ b/drivers/gpu/ipu-v3/ipu-common.c
> @@ -997,7 +997,7 @@ struct ipu_platform_reg {
>  };
>  
>  /* These must be in the order of the corresponding device tree port nodes */
> -static const struct ipu_platform_reg client_reg[] = {
> +static struct ipu_platform_reg client_reg[] = {
>  	{
>  		.pdata = {
>  			.csi = 0,
> @@ -1048,7 +1048,7 @@ static int ipu_add_client_devices(struct ipu_soc *ipu, unsigned long ipu_base)
>  	mutex_unlock(&ipu_client_id_mutex);
>  
>  	for (i = 0; i < ARRAY_SIZE(client_reg); i++) {
> -		const struct ipu_platform_reg *reg = &client_reg[i];
> +		struct ipu_platform_reg *reg = &client_reg[i];
>  		struct platform_device *pdev;
>  		struct device_node *of_node;
>  
> @@ -1070,6 +1070,7 @@ static int ipu_add_client_devices(struct ipu_soc *ipu, unsigned long ipu_base)
>  
>  		pdev->dev.parent = dev;
>  
> +		reg->pdata.of_node = of_node;
>  		ret = platform_device_add_data(pdev, &reg->pdata,
>  					       sizeof(reg->pdata));
>  		if (!ret)
> diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
> index ad66589..3a2a794 100644
> --- a/include/video/imx-ipu-v3.h
> +++ b/include/video/imx-ipu-v3.h
> @@ -16,6 +16,7 @@
>  #include <linux/videodev2.h>
>  #include <linux/bitmap.h>
>  #include <linux/fb.h>
> +#include <linux/of.h>
>  #include <media/v4l2-mediabus.h>
>  #include <video/videomode.h>
>  
> @@ -345,6 +346,7 @@ struct ipu_client_platformdata {
>  	int dc;
>  	int dp;
>  	int dma[2];
> +	struct device_node *of_node;
>  };
>  
>  #endif /* __DRM_IPU_H__ */
>
This seems to be OK. I tested this patch with parallel-display and
imx-ldb modular and built-in.


Lothar Waßmann

Re: [v2] drm/imx: Match imx-ipuv3-crtc components using device node in platform data

[Heiko Schocher]

Hello Philipp,

Am 18.05.2016 um 14:56 schrieb Philipp Zabel:
> The component master driver imx-drm-core matches component devices using
> their of_node. Since commit 950b410dd1ab ("gpu: ipu-v3: Fix imx-ipuv3-crtc
> module autoloading"), the imx-ipuv3-crtc dev->of_node is not set during
> probing. Before that, of_node was set and caused an of: modalias to be
> used instead of the platform: modalias, which broke module autoloading.
>
> On the other hand, if dev->of_node is not set yet when the imx-ipuv3-crtc
> probe function calls component_add, component matching in imx-drm-core
> fails. While dev->of_node will be set once the next component tries to
> bring up the component master, imx-drm-core component binding will never
> succeed if one of the crtc devices is probed last.
>
> Add of_node to the component platform data and match against the
> pdata->of_node instead of dev->of_node in imx-drm-core to work around
> this problem.
>
> Cc: <stable@vger.kernel.org> # 4.4.x
> Fixes: 950b410dd1ab ("gpu: ipu-v3: Fix imx-ipuv3-crtc module autoloading")
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> Changes since v1:
>   - Also set the crtc->port to the of_node from platform data
> ---
>   drivers/gpu/drm/imx/imx-drm-core.c | 8 ++++++++
>   drivers/gpu/drm/imx/ipuv3-crtc.c   | 2 +-
>   drivers/gpu/ipu-v3/ipu-common.c    | 5 +++--
>   include/video/imx-ipu-v3.h         | 2 ++
>   4 files changed, 14 insertions(+), 3 deletions(-)

Thanks! This fixes the aristainetos2 board, see log:
http://xeidos.ddns.net/buildbot/builders/ari_lx/builds/0/steps/shell/logs/tbotlog

so:

Tested-by: Heiko Schocher <hs@denx.de>

bye,
Heiko

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Re: [PATCH v2] drm/imx: Match imx-ipuv3-crtc components using device node in platform data

[Chris Ruehl]

Hi,

On Wednesday, May 18, 2016 08:56 PM, Philipp Zabel wrote:
> The component master driver imx-drm-core matches component devices using
> their of_node. Since commit 950b410dd1ab ("gpu: ipu-v3: Fix imx-ipuv3-crtc
> module autoloading"), the imx-ipuv3-crtc dev->of_node is not set during
> probing. Before that, of_node was set and caused an of: modalias to be
> used instead of the platform: modalias, which broke module autoloading.
>
> On the other hand, if dev->of_node is not set yet when the imx-ipuv3-crtc
> probe function calls component_add, component matching in imx-drm-core
> fails. While dev->of_node will be set once the next component tries to
> bring up the component master, imx-drm-core component binding will never
> succeed if one of the crtc devices is probed last.
>
> Add of_node to the component platform data and match against the
> pdata->of_node instead of dev->of_node in imx-drm-core to work around
> this problem.
>
> Cc: <stable@vger.kernel.org> # 4.4.x
> Fixes: 950b410dd1ab ("gpu: ipu-v3: Fix imx-ipuv3-crtc module autoloading")
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> Changes since v1:
>   - Also set the crtc->port to the of_node from platform data
> ---
>   drivers/gpu/drm/imx/imx-drm-core.c | 8 ++++++++
>   drivers/gpu/drm/imx/ipuv3-crtc.c   | 2 +-
>   drivers/gpu/ipu-v3/ipu-common.c    | 5 +++--
>   include/video/imx-ipu-v3.h         | 2 ++
>   4 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> index e26dcde..05229b9 100644
> --- a/drivers/gpu/drm/imx/imx-drm-core.c
> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> @@ -25,6 +25,7 @@
>   #include <drm/drm_fb_cma_helper.h>
>   #include <drm/drm_plane_helper.h>
>   #include <drm/drm_of.h>
> +#include <video/imx-ipu-v3.h>
>
>   #include "imx-drm.h"
>
> @@ -444,6 +445,13 @@ static int compare_of(struct device *dev, void *data)
>   {
>   	struct device_node *np = data;
>
> +	/* Special case for DI, dev->of_node may not be set yet */
> +	if (strcmp(dev->driver->name, "imx-ipuv3-crtc") == 0) {
> +		struct ipu_client_platformdata *pdata = dev->platform_data;
> +
> +		return pdata->of_node == np;
> +	}
> +
>   	/* Special case for LDB, one device for two channels */
>   	if (of_node_cmp(np->name, "lvds-channel") == 0) {
>   		np = of_get_parent(np);
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index dee8e8b..b2c30b8 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -473,7 +473,7 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
>
>   	ret = imx_drm_add_crtc(drm, &ipu_crtc->base, &ipu_crtc->imx_crtc,
>   			&ipu_crtc->plane[0]->base, &ipu_crtc_helper_funcs,
> -			ipu_crtc->dev->of_node);
> +			pdata->of_node);
>   	if (ret) {
>   		dev_err(ipu_crtc->dev, "adding crtc failed with %d.\n", ret);
>   		goto err_put_resources;
> diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
> index abb98c7..99dcacf 100644
> --- a/drivers/gpu/ipu-v3/ipu-common.c
> +++ b/drivers/gpu/ipu-v3/ipu-common.c
> @@ -997,7 +997,7 @@ struct ipu_platform_reg {
>   };
>
>   /* These must be in the order of the corresponding device tree port nodes */
> -static const struct ipu_platform_reg client_reg[] = {
> +static struct ipu_platform_reg client_reg[] = {
>   	{
>   		.pdata = {
>   			.csi = 0,
> @@ -1048,7 +1048,7 @@ static int ipu_add_client_devices(struct ipu_soc *ipu, unsigned long ipu_base)
>   	mutex_unlock(&ipu_client_id_mutex);
>
>   	for (i = 0; i < ARRAY_SIZE(client_reg); i++) {
> -		const struct ipu_platform_reg *reg = &client_reg[i];
> +		struct ipu_platform_reg *reg = &client_reg[i];
>   		struct platform_device *pdev;
>   		struct device_node *of_node;
>
> @@ -1070,6 +1070,7 @@ static int ipu_add_client_devices(struct ipu_soc *ipu, unsigned long ipu_base)
>
>   		pdev->dev.parent = dev;
>
> +		reg->pdata.of_node = of_node;
>   		ret = platform_device_add_data(pdev, &reg->pdata,
>   					       sizeof(reg->pdata));
>   		if (!ret)
> diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
> index ad66589..3a2a794 100644
> --- a/include/video/imx-ipu-v3.h
> +++ b/include/video/imx-ipu-v3.h
> @@ -16,6 +16,7 @@
>   #include <linux/videodev2.h>
>   #include <linux/bitmap.h>
>   #include <linux/fb.h>
> +#include <linux/of.h>
>   #include <media/v4l2-mediabus.h>
>   #include <video/videomode.h>
>
> @@ -345,6 +346,7 @@ struct ipu_client_platformdata {
>   	int dc;
>   	int dp;
>   	int dma[2];
> +	struct device_node *of_node;
>   };
>
>   #endif /* __DRM_IPU_H__ */
>

Tested successfully against kernel 4.6

root@gtsir-a0fd:~# dmesg | grep hdmi
[    2.288698] dwhdmi-imx 120000.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1
[    2.307519] imx-drm display-subsystem: bound 120000.hdmi (ops dw_hdmi_imx_ops)

Tested-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>


Thank you!


-- 
GTSYS Limited RFID Technology
9/F, Unit E, R07, Kwai Shing Industrial Building Phase 2,
42-46 Tai Lin Pai Road, Kwai Chung, N.T., Hong Kong
Tel (852) 9079 9521

Disclaimer: http://www.gtsys.com.hk/email/classified.html