[PATCH 1/2] drm/i2c/tda998x: Switch to atomic operations

[PATCH 1/2] drm/i2c/tda998x: Switch to atomic operations

[Fabio Estevam]

Use the atomic version of the enable/disable operations to continue the
transition to the atomic API, started with the introduction of
.atomic_get_input_bus_fmts(). This will be needed to access the mode
from the atomic state.

Based on Laurent's commit a6ea7d268a63("drm: bridge: ti-sn65dsi83:
Switch to atomic operations").

Tested on a imx6sx-udoo-neo board.

Suggested-by: Marek Vasut <marex@denx.de>
Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index b7ec6c374fbd..adaa985af87e 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1395,7 +1395,8 @@ static enum drm_mode_status tda998x_bridge_mode_valid(struct drm_bridge *bridge,
 	return MODE_OK;
 }
 
-static void tda998x_bridge_enable(struct drm_bridge *bridge)
+static void tda998x_bridge_atomic_enable(struct drm_bridge *bridge,
+					  struct drm_bridge_state *old_bridge_state)
 {
 	struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
 
@@ -1413,7 +1414,8 @@ static void tda998x_bridge_enable(struct drm_bridge *bridge)
 	}
 }
 
-static void tda998x_bridge_disable(struct drm_bridge *bridge)
+static void tda998x_bridge_atomic_disable(struct drm_bridge *bridge,
+					   struct drm_bridge_state *old_bridge_state)
 {
 	struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
 
@@ -1680,9 +1682,9 @@ static const struct drm_bridge_funcs tda998x_bridge_funcs = {
 	.attach = tda998x_bridge_attach,
 	.detach = tda998x_bridge_detach,
 	.mode_valid = tda998x_bridge_mode_valid,
-	.disable = tda998x_bridge_disable,
+	.atomic_disable = tda998x_bridge_atomic_disable,
 	.mode_set = tda998x_bridge_mode_set,
-	.enable = tda998x_bridge_enable,
+	.atomic_enable = tda998x_bridge_atomic_enable,
 };
 
 /* I2C driver functions */
-- 
2.25.1

[PATCH 2/2] drm/i2c/tda998x: Implement atomic_get_input_bus_fmts

[Fabio Estevam]

Implement the .atomic_get_input_bus_fmts callback to let the bridge
indicate the pixel format it requires on the parallel bus to the LCDIF.

Based on Marek's commit db8b7ca5b232 ("drm/bridge: ti-sn65dsi83: Replace
connector format patching with atomic_get_input_bus_fmts").

Tested on a imx6sx-udoo-neo board.

Suggested-by: Marek Vasut <marex@denx.de>
Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index adaa985af87e..d987481e97c1 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1678,6 +1678,31 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge,
 	mutex_unlock(&priv->audio_mutex);
 }
 
+#define MAX_INPUT_SEL_FORMATS  1
+
+static u32 *
+tda998x_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+				   struct drm_bridge_state *bridge_state,
+				   struct drm_crtc_state *crtc_state,
+				   struct drm_connector_state *conn_state,
+				   u32 output_fmt,
+				   unsigned int *num_input_fmts)
+{
+	u32 *input_fmts;
+
+	*num_input_fmts = 0;
+
+	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
+			     GFP_KERNEL);
+	if (!input_fmts)
+		return NULL;
+
+	input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+	*num_input_fmts = 1;
+
+	return input_fmts;
+}
+
 static const struct drm_bridge_funcs tda998x_bridge_funcs = {
 	.attach = tda998x_bridge_attach,
 	.detach = tda998x_bridge_detach,
@@ -1685,6 +1710,10 @@ static const struct drm_bridge_funcs tda998x_bridge_funcs = {
 	.atomic_disable = tda998x_bridge_atomic_disable,
 	.mode_set = tda998x_bridge_mode_set,
 	.atomic_enable = tda998x_bridge_atomic_enable,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_reset = drm_atomic_helper_bridge_reset,
+	.atomic_get_input_bus_fmts = tda998x_atomic_get_input_bus_fmts,
 };
 
 /* I2C driver functions */
-- 
2.25.1

Re: [PATCH 2/2] drm/i2c/tda998x: Implement atomic_get_input_bus_fmts

[Laurent Pinchart]

Hi Fabio,

Thank you for the patch.

On Thu, Dec 30, 2021 at 10:39:30PM -0300, Fabio Estevam wrote:
> Implement the .atomic_get_input_bus_fmts callback to let the bridge
> indicate the pixel format it requires on the parallel bus to the LCDIF.
> 
> Based on Marek's commit db8b7ca5b232 ("drm/bridge: ti-sn65dsi83: Replace
> connector format patching with atomic_get_input_bus_fmts").
> 
> Tested on a imx6sx-udoo-neo board.
> 
> Suggested-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
>  drivers/gpu/drm/i2c/tda998x_drv.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index adaa985af87e..d987481e97c1 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -1678,6 +1678,31 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge,
>  	mutex_unlock(&priv->audio_mutex);
>  }
>  
> +#define MAX_INPUT_SEL_FORMATS  1
> +
> +static u32 *
> +tda998x_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> +				   struct drm_bridge_state *bridge_state,
> +				   struct drm_crtc_state *crtc_state,
> +				   struct drm_connector_state *conn_state,
> +				   u32 output_fmt,
> +				   unsigned int *num_input_fmts)
> +{
> +	u32 *input_fmts;
> +
> +	*num_input_fmts = 0;
> +
> +	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
> +			     GFP_KERNEL);
> +	if (!input_fmts)
> +		return NULL;
> +
> +	input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> +	*num_input_fmts = 1;
> +
> +	return input_fmts;
> +}
> +
>  static const struct drm_bridge_funcs tda998x_bridge_funcs = {
>  	.attach = tda998x_bridge_attach,
>  	.detach = tda998x_bridge_detach,
> @@ -1685,6 +1710,10 @@ static const struct drm_bridge_funcs tda998x_bridge_funcs = {
>  	.atomic_disable = tda998x_bridge_atomic_disable,
>  	.mode_set = tda998x_bridge_mode_set,
>  	.atomic_enable = tda998x_bridge_atomic_enable,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_reset = drm_atomic_helper_bridge_reset,

I think this should go to 1/2.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	.atomic_get_input_bus_fmts = tda998x_atomic_get_input_bus_fmts,
>  };
>  
>  /* I2C driver functions */

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm/i2c/tda998x: Switch to atomic operations

[Laurent Pinchart]

Hi Fabio,

Thank you for the patch.

On Thu, Dec 30, 2021 at 10:39:29PM -0300, Fabio Estevam wrote:
> Use the atomic version of the enable/disable operations to continue the
> transition to the atomic API, started with the introduction of
> .atomic_get_input_bus_fmts(). This will be needed to access the mode
> from the atomic state.
> 
> Based on Laurent's commit a6ea7d268a63("drm: bridge: ti-sn65dsi83:
> Switch to atomic operations").
> 
> Tested on a imx6sx-udoo-neo board.
> 
> Suggested-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

With the comment from 2/2 taken into account,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/i2c/tda998x_drv.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index b7ec6c374fbd..adaa985af87e 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -1395,7 +1395,8 @@ static enum drm_mode_status tda998x_bridge_mode_valid(struct drm_bridge *bridge,
>  	return MODE_OK;
>  }
>  
> -static void tda998x_bridge_enable(struct drm_bridge *bridge)
> +static void tda998x_bridge_atomic_enable(struct drm_bridge *bridge,
> +					  struct drm_bridge_state *old_bridge_state)
>  {
>  	struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
>  
> @@ -1413,7 +1414,8 @@ static void tda998x_bridge_enable(struct drm_bridge *bridge)
>  	}
>  }
>  
> -static void tda998x_bridge_disable(struct drm_bridge *bridge)
> +static void tda998x_bridge_atomic_disable(struct drm_bridge *bridge,
> +					   struct drm_bridge_state *old_bridge_state)
>  {
>  	struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
>  
> @@ -1680,9 +1682,9 @@ static const struct drm_bridge_funcs tda998x_bridge_funcs = {
>  	.attach = tda998x_bridge_attach,
>  	.detach = tda998x_bridge_detach,
>  	.mode_valid = tda998x_bridge_mode_valid,
> -	.disable = tda998x_bridge_disable,
> +	.atomic_disable = tda998x_bridge_atomic_disable,
>  	.mode_set = tda998x_bridge_mode_set,
> -	.enable = tda998x_bridge_enable,
> +	.atomic_enable = tda998x_bridge_atomic_enable,
>  };
>  
>  /* I2C driver functions */

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm/i2c/tda998x: Switch to atomic operations

[Fabio Estevam]

Hi Laurent,

On Mon, Jan 3, 2022 at 8:48 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:

> With the comment from 2/2 taken into account,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks for the review. I addressed your feedback and sent v2.

I noticed a problem when removing/inserting the HDMI cable.

If I boot the board with the HDMI cable connected, then after
removal/insertion of the HDMI cable, the following
kernel warning is observed:

# [   23.201080] ------------[ cut here ]------------
[   23.207275] WARNING: CPU: 0 PID: 56 at
drivers/gpu/drm/drm_atomic_helper.c:1514
drm_atomic_helper_wait_for_vblanks.part.0+0x27c/0x294
[   23.221469] [CRTC:35:crtc-0] vblank wait timed out
[   23.226448] Modules linked in:
[   23.230255] CPU: 0 PID: 56 Comm: kworker/0:3 Not tainted
5.15.12-00003-g27f29fb60028 #94
[   23.238508] Hardware name: Freescale i.MX6 SoloX (Device Tree)
[   23.244457] Workqueue: events output_poll_execute
[   23.249377] [<c01118f8>] (unwind_backtrace) from [<c010bde0>]
(show_stack+0x10/0x14)
[   23.257316] [<c010bde0>] (show_stack) from [<c0e8d38c>]
(dump_stack_lvl+0x58/0x70)
[   23.265059] [<c0e8d38c>] (dump_stack_lvl) from [<c0125e2c>]
(__warn+0xd8/0x114)
[   23.272533] [<c0125e2c>] (__warn) from [<c0e87f94>]
(warn_slowpath_fmt+0x90/0xc4)
[   23.280166] [<c0e87f94>] (warn_slowpath_fmt) from [<c0707c98>]
(drm_atomic_helper_wait_for_vblanks.part.0+0x27c/0x294)
[   23.291054] [<c0707c98>]
(drm_atomic_helper_wait_for_vblanks.part.0) from [<c07092cc>]
(drm_atomic_helper_commit_tail_rpm+0x5c/0x6c)
[   23.303150] [<c07092cc>] (drm_atomic_helper_commit_tail_rpm) from
[<c0709e44>] (commit_tail+0x9c/0x190)
[   23.312717] [<c0709e44>] (commit_tail) from [<c070a098>]
(drm_atomic_helper_commit+0x158/0x18c)
[   23.321588] [<c070a098>] (drm_atomic_helper_commit) from
[<c073f4e4>] (drm_client_modeset_commit_atomic+0x238/0x284)
[   23.332314] [<c073f4e4>] (drm_client_modeset_commit_atomic) from
[<c073f600>] (drm_client_modeset_commit_locked+0x60/0x1cc)
[   23.343615] [<c073f600>] (drm_client_modeset_commit_locked) from
[<c073f790>] (drm_client_modeset_commit+0x24/0x40)
[   23.354218] [<c073f790>] (drm_client_modeset_commit) from
[<c071050c>] (__drm_fb_helper_restore_fbdev_mode_unlocked+0x9c/0xc8)
[   23.365803] [<c071050c>]
(__drm_fb_helper_restore_fbdev_mode_unlocked) from [<c07105a0>]
(drm_fb_helper_set_par+0x38/0x68)
[   23.377015] [<c07105a0>] (drm_fb_helper_set_par) from [<c0710674>]
(drm_fb_helper_hotplug_event.part.0+0xa4/0xc0)
[   23.387443] [<c0710674>] (drm_fb_helper_hotplug_event.part.0) from
[<c073ea80>] (drm_client_dev_hotplug+0x6c/0xb4)
[   23.397959] [<c073ea80>] (drm_client_dev_hotplug) from [<c06fc5e0>]
(output_poll_execute+0x200/0x21c)
[   23.407346] [<c06fc5e0>] (output_poll_execute) from [<c0147f54>]
(process_one_work+0x298/0x7cc)
[   23.416224] [<c0147f54>] (process_one_work) from [<c01484b8>]
(worker_thread+0x30/0x50c)
[   23.424479] [<c01484b8>] (worker_thread) from [<c015138c>]
(kthread+0x154/0x17c)
[   23.432039] [<c015138c>] (kthread) from [<c010011c>]
(ret_from_fork+0x14/0x38)
[   23.439413] Exception stack(0xc42a1fb0 to 0xc42a1ff8)
[   23.444588] 1fa0:                                     00000000
00000000 00000000 00000000
[   23.452888] 1fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[   23.461182] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[   23.468734] irq event stamp: 43775
[   23.472305] hardirqs last  enabled at (43783): [<c0197408>]
__up_console_sem+0x50/0x60
[   23.480785] hardirqs last disabled at (43792): [<c01973f4>]
__up_console_sem+0x3c/0x60
[   23.489224] softirqs last  enabled at (43774): [<c010150c>]
__do_softirq+0x2ec/0x5a4
[   23.497163] softirqs last disabled at (43747): [<c012efdc>]
irq_exit+0x18c/0x210
[   23.505106] ---[ end trace 86572327287ca501 ]---

I haven't managed to fix this yet, but if you have any suggestions,
please let me know.

Thanks,

Fabio Estevam

Re: [PATCH 1/2] drm/i2c/tda998x: Switch to atomic operations

[Tommaso Merciai]

On Mon, Jan 03, 2022 at 09:35:14AM -0300, Fabio Estevam wrote:
> Hi Laurent,
> 
> On Mon, Jan 3, 2022 at 8:48 AM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> 
> > With the comment from 2/2 taken into account,
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Thanks for the review. I addressed your feedback and sent v2.
> 
> I noticed a problem when removing/inserting the HDMI cable.
> 
> If I boot the board with the HDMI cable connected, then after
> removal/insertion of the HDMI cable, the following
> kernel warning is observed:
> 
> # [   23.201080] ------------[ cut here ]------------
> [   23.207275] WARNING: CPU: 0 PID: 56 at
> drivers/gpu/drm/drm_atomic_helper.c:1514
> drm_atomic_helper_wait_for_vblanks.part.0+0x27c/0x294
> [   23.221469] [CRTC:35:crtc-0] vblank wait timed out
> [   23.226448] Modules linked in:
> [   23.230255] CPU: 0 PID: 56 Comm: kworker/0:3 Not tainted
> 5.15.12-00003-g27f29fb60028 #94
> [   23.238508] Hardware name: Freescale i.MX6 SoloX (Device Tree)
> [   23.244457] Workqueue: events output_poll_execute
> [   23.249377] [<c01118f8>] (unwind_backtrace) from [<c010bde0>]
> (show_stack+0x10/0x14)
> [   23.257316] [<c010bde0>] (show_stack) from [<c0e8d38c>]
> (dump_stack_lvl+0x58/0x70)
> [   23.265059] [<c0e8d38c>] (dump_stack_lvl) from [<c0125e2c>]
> (__warn+0xd8/0x114)
> [   23.272533] [<c0125e2c>] (__warn) from [<c0e87f94>]
> (warn_slowpath_fmt+0x90/0xc4)
> [   23.280166] [<c0e87f94>] (warn_slowpath_fmt) from [<c0707c98>]
> (drm_atomic_helper_wait_for_vblanks.part.0+0x27c/0x294)
> [   23.291054] [<c0707c98>]
> (drm_atomic_helper_wait_for_vblanks.part.0) from [<c07092cc>]
> (drm_atomic_helper_commit_tail_rpm+0x5c/0x6c)
> [   23.303150] [<c07092cc>] (drm_atomic_helper_commit_tail_rpm) from
> [<c0709e44>] (commit_tail+0x9c/0x190)
> [   23.312717] [<c0709e44>] (commit_tail) from [<c070a098>]
> (drm_atomic_helper_commit+0x158/0x18c)
> [   23.321588] [<c070a098>] (drm_atomic_helper_commit) from
> [<c073f4e4>] (drm_client_modeset_commit_atomic+0x238/0x284)
> [   23.332314] [<c073f4e4>] (drm_client_modeset_commit_atomic) from
> [<c073f600>] (drm_client_modeset_commit_locked+0x60/0x1cc)
> [   23.343615] [<c073f600>] (drm_client_modeset_commit_locked) from
> [<c073f790>] (drm_client_modeset_commit+0x24/0x40)
> [   23.354218] [<c073f790>] (drm_client_modeset_commit) from
> [<c071050c>] (__drm_fb_helper_restore_fbdev_mode_unlocked+0x9c/0xc8)
> [   23.365803] [<c071050c>]
> (__drm_fb_helper_restore_fbdev_mode_unlocked) from [<c07105a0>]
> (drm_fb_helper_set_par+0x38/0x68)
> [   23.377015] [<c07105a0>] (drm_fb_helper_set_par) from [<c0710674>]
> (drm_fb_helper_hotplug_event.part.0+0xa4/0xc0)
> [   23.387443] [<c0710674>] (drm_fb_helper_hotplug_event.part.0) from
> [<c073ea80>] (drm_client_dev_hotplug+0x6c/0xb4)
> [   23.397959] [<c073ea80>] (drm_client_dev_hotplug) from [<c06fc5e0>]
> (output_poll_execute+0x200/0x21c)
> [   23.407346] [<c06fc5e0>] (output_poll_execute) from [<c0147f54>]
> (process_one_work+0x298/0x7cc)
> [   23.416224] [<c0147f54>] (process_one_work) from [<c01484b8>]
> (worker_thread+0x30/0x50c)
> [   23.424479] [<c01484b8>] (worker_thread) from [<c015138c>]
> (kthread+0x154/0x17c)
> [   23.432039] [<c015138c>] (kthread) from [<c010011c>]
> (ret_from_fork+0x14/0x38)
> [   23.439413] Exception stack(0xc42a1fb0 to 0xc42a1ff8)
> [   23.444588] 1fa0:                                     00000000
> 00000000 00000000 00000000
> [   23.452888] 1fc0: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [   23.461182] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [   23.468734] irq event stamp: 43775
> [   23.472305] hardirqs last  enabled at (43783): [<c0197408>]
> __up_console_sem+0x50/0x60
> [   23.480785] hardirqs last disabled at (43792): [<c01973f4>]
> __up_console_sem+0x3c/0x60
> [   23.489224] softirqs last  enabled at (43774): [<c010150c>]
> __do_softirq+0x2ec/0x5a4
> [   23.497163] softirqs last disabled at (43747): [<c012efdc>]
> irq_exit+0x18c/0x210
> [   23.505106] ---[ end trace 86572327287ca501 ]---
> 
> I haven't managed to fix this yet, but if you have any suggestions,
> please let me know.
> 
> Thanks,
> 
> Fabio Estevam

Hi Fabio,
If you need some test let me know. Whitch filesystem are you using?
In the next days I will investigate on this issue.
Let me know.

Thanks,
Tommaso

Re: [PATCH 1/2] drm/i2c/tda998x: Switch to atomic operations

[Fabio Estevam]

Hi Tommaso,

On Sat, Jan 8, 2022 at 4:17 PM Tommaso Merciai <tomm.merciai@gmail.com> wrote:

> Hi Fabio,
> If you need some test let me know. Whitch filesystem are you using?

I am using a rootfs generated by Buildroot.

The issue I see seems to be hotplug-related.

cat /sys/class/drm/card1-HDMI-A-1/status

not always match with the real state of the HDMI cable.

> In the next days I will investigate on this issue.
> Let me know.

Thanks

Re: [PATCH 1/2] drm/i2c/tda998x: Switch to atomic operations

[Tommaso Merciai]

On Sun, Jan 09, 2022 at 03:44:07PM -0300, Fabio Estevam wrote:
> Hi Tommaso,
> 
> On Sat, Jan 8, 2022 at 4:17 PM Tommaso Merciai <tomm.merciai@gmail.com> wrote:
> 
> > Hi Fabio,
> > If you need some test let me know. Whitch filesystem are you using?
> 
> I am using a rootfs generated by Buildroot.
> 
> The issue I see seems to be hotplug-related.
> 
> cat /sys/class/drm/card1-HDMI-A-1/status
> 
> not always match with the real state of the HDMI cable.
> 
> > In the next days I will investigate on this issue.
> > Let me know.
> 
> Thanks

Hi Fabio,
Got it, I'll try to reproduce the issue on my side and let you know.

Thanks,
Tommaso

Re: [PATCH 1/2] drm/i2c/tda998x: Switch to atomic operations

[Tommaso Merciai]

On Tue, Jan 11, 2022 at 09:21:51PM +0100, Tommaso Merciai wrote:
> On Sun, Jan 09, 2022 at 03:44:07PM -0300, Fabio Estevam wrote:
> > Hi Tommaso,
> > 
> > On Sat, Jan 8, 2022 at 4:17 PM Tommaso Merciai <tomm.merciai@gmail.com> wrote:
> > 
> > > Hi Fabio,
> > > If you need some test let me know. Whitch filesystem are you using?
> > 
> > I am using a rootfs generated by Buildroot.
> > 
> > The issue I see seems to be hotplug-related.
> > 
> > cat /sys/class/drm/card1-HDMI-A-1/status
> > 
> > not always match with the real state of the HDMI cable.
> > 
> > > In the next days I will investigate on this issue.
> > > Let me know.
> > 
> > Thanks
> 
> Hi Fabio,
> Got it, I'll try to reproduce the issue on my side and let you know.
> 
> Thanks,
> Tommaso

Hi Fabio,
I'm working on bring up urt,umsh-8596md-20t lvds kit panel, but after enable
following node I get the following error:

+       backlight_display: backlight-display {
+               compatible = "pwm-backlight";
+               pwms = <&pwm4 0 5000000>;
+               brightness-levels = <0 4 8 16 32 64 128 255>;
+               default-brightness-level = <32>;
+       };
+
        leds {
                compatible = "gpio-leds";

@@ -70,6 +77,17 @@ reg_wlan: regulator-wlan {
                startup-delay-us = <70000>;
                enable-active-high;
        };
+
+       panel {
+               compatible = "urt,umsh-8596md-20t";
+               backlight = <&backlight_display>;
+
+               port {
+                       panel_in: endpoint {
+                               remote-endpoint = <&display_out>;
+                       };
+               };
+       };
 };

 &fec1 {
@@ -196,6 +214,12 @@ pinctrl_bt_reg: btreggrp {
                        <MX6SX_PAD_KEY_ROW2__GPIO2_IO_17        0x15059>;
        };

+       pinctrl_pwm4: pwm4grp-1 {
+               fsl,pins = <
+                       MX6SX_PAD_SD1_DATA1__PWM4_OUT 0x110b0
+               >;
+       };
+
        pinctrl_enet1: enet1grp {
                fsl,pins =
                        <MX6SX_PAD_ENET1_CRS__GPIO2_IO_1        0xa0b1>,
@@ -316,6 +340,40 @@ pinctrl_usdhc3: usdhc3grp {
                        <MX6SX_PAD_SD3_CLK__USDHC3_CLK          0x10069>,
                        <MX6SX_PAD_CSI_MCLK__OSC32K_32K_OUT     0x10059>;
        };
+
+       pinctrl_lcd: lcdgrp {
+                       fsl,pins = <
+                               MX6SX_PAD_LCD1_DATA00__LCDIF1_DATA_0 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA01__LCDIF1_DATA_1 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA02__LCDIF1_DATA_2 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA03__LCDIF1_DATA_3 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA04__LCDIF1_DATA_4 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA05__LCDIF1_DATA_5 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA06__LCDIF1_DATA_6 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA07__LCDIF1_DATA_7 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA08__LCDIF1_DATA_8 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA09__LCDIF1_DATA_9 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA10__LCDIF1_DATA_10 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA11__LCDIF1_DATA_11 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA12__LCDIF1_DATA_12 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA13__LCDIF1_DATA_13 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA14__LCDIF1_DATA_14 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA15__LCDIF1_DATA_15 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA16__LCDIF1_DATA_16 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA17__LCDIF1_DATA_17 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA18__LCDIF1_DATA_18 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA19__LCDIF1_DATA_19 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA20__LCDIF1_DATA_20 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA21__LCDIF1_DATA_21 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA22__LCDIF1_DATA_22 0x4001b0b0
+                               MX6SX_PAD_LCD1_DATA23__LCDIF1_DATA_23 0x4001b0b0
+                               MX6SX_PAD_LCD1_CLK__LCDIF1_CLK  0x4001b0b0
+                               MX6SX_PAD_LCD1_ENABLE__LCDIF1_ENABLE 0x4001b0b0
+                               MX6SX_PAD_LCD1_VSYNC__LCDIF1_VSYNC 0x4001b0b0
+                               MX6SX_PAD_LCD1_HSYNC__LCDIF1_HSYNC 0x4001b0b0
+                               MX6SX_PAD_LCD1_RESET__GPIO3_IO_27 0x4001b0b0
+                       >;
+               };
 };

 &uart1 {
@@ -408,3 +466,22 @@ wlcore: wlcore@2 {
                tcxo-clock-frequency = <26000000>;
        };
 };
+
+&lcdif1 {
+       pinctrl-names = "default";
+       pinctrl-0 = <&pinctrl_lcd>;
+       status = "okay";
+
+       port {
+               display_out: endpoint {
+                       remote-endpoint = <&panel_in>;
+               };
+       };
+};
+
+&pwm4 {
+       #pwm-cells = <2>;
+       pinctrl-names = "default";
+       pinctrl-0 = <&pinctrl_pwm4>;
+       status = "okay";
+};


Error:

imx6qpdlsolox login: [   49.624735] ------------[ cut here ]------------
[   49.625348] WARNING: CPU: 0 PID: 324 at drivers/gpu/drm/drm_atomic_helper.c:1529 drm_atomic_helper_wait_for_vblanks.part.0+0x278/0x290
[   49.625527] [CRTC:35:crtc-0] vblank wait timed out
[   49.625590] Modules linked in: imx_sdma
[   49.625755] CPU: 0 PID: 324 Comm: systemd-logind Not tainted 5.16.0-08197-gfeb7a43de5ef-dirty #62
[   49.625829] Hardware name: Freescale i.MX6 SoloX (Device Tree)
[   49.625883]  unwind_backtrace from show_stack+0x10/0x14
[   49.625998]  show_stack from dump_stack_lvl+0x58/0x70
[   49.626093]  dump_stack_lvl from __warn+0xd4/0x154
[   49.626180]  __warn from warn_slowpath_fmt+0x74/0xa8
[   49.626286]  warn_slowpath_fmt from drm_atomic_helper_wait_for_vblanks.part.0+0x278/0x290
[   49.626395]  drm_atomic_helper_wait_for_vblanks.part.0 from drm_atomic_helper_commit_tail_rpm+0x7c/0x8c
[   49.626498]  drm_atomic_helper_commit_tail_rpm from commit_tail+0x9c/0x184
[   49.626591]  commit_tail from drm_atomic_helper_commit+0x158/0x18c
[   49.626680]  drm_atomic_helper_commit from drm_client_modeset_commit_atomic+0x238/0x284
[   49.626784]  drm_client_modeset_commit_atomic from drm_client_modeset_commit_locked+0x60/0x1cc
[   49.626868]  drm_client_modeset_commit_locked from drm_fb_helper_pan_display+0x94/0x1c4
[   49.626970]  drm_fb_helper_pan_display from fb_pan_display+0x9c/0x114
[   49.627051]  fb_pan_display from bit_update_start+0x14/0x30
[   49.627146]  bit_update_start from fbcon_switch+0x2d4/0x45c
[   49.627236]  fbcon_switch from redraw_screen+0xd4/0x248
[   49.627338]  redraw_screen from fbcon_blank+0x1b0/0x258
[   49.627424]  fbcon_blank from do_unblank_screen+0x98/0x170
[   49.627509]  do_unblank_screen from vt_ioctl+0x960/0x1d78
[   49.627623]  vt_ioctl from tty_ioctl+0x3e0/0xe18
[   49.627709]  tty_ioctl from sys_ioctl+0x5a8/0xec4
[   49.627796]  sys_ioctl from ret_fast_syscall+0x0/0x1c
[   49.627874] Exception stack(0xc4767fa8 to 0xc4767ff0)
[   49.627943] 7fa0:                   021e1600 fffffffb 0000000a 00004b3a 00000000 00000134
[   49.628010] 7fc0: 021e1600 fffffffb 0000000a 00000036 be8479d8 00000000 00000000 021d43b8
[   49.628061] 7fe0: b6f0f978 be847914 b6e41d75 b6cbfc18
[   49.628114] irq event stamp: 52041
[   49.628170] hardirqs last  enabled at (52047): [<c019b5d0>] vprintk_emit+0x2dc/0x310
[   49.628285] hardirqs last disabled at (52052): [<c019b57c>] vprintk_emit+0x288/0x310
[   49.628377] softirqs last  enabled at (51264): [<c0101518>] __do_softirq+0x2f8/0x5b0
[   49.628466] softirqs last disabled at (51231): [<c012eda8>] __irq_exit_rcu+0x16c/0x1e0
[   49.628573] ---[ end trace 1572e4d782184e68 ]---

imx6qpdlsolox login: root
[   60.016074] mxsfb 2220000.lcdif: [drm] *ERROR* flip_done timed out
[   60.016455] mxsfb 2220000.lcdif: [drm] *ERROR* [CRTC:35:crtc-0] commit wait timed out
[   70.256588] mxsfb 2220000.lcdif: [drm] *ERROR* flip_done timed out
[   70.256695] mxsfb 2220000.lcdif: [drm] *ERROR* [CONNECTOR:37:DPI-1] commit wait timed out
[   80.495783] mxsfb 2220000.lcdif: [drm] *ERROR* flip_done timed out
[   80.495889] mxsfb 2220000.lcdif: [drm] *ERROR* [PLANE:31:plane-0] commit wait timed out
[   80.606207] ------------[ cut here ]------------
[   80.606257] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/drm_atomic_helper.c:1529 drm_atomic_helper_wait_for_vblanks.part.0+0x278/0x290
[   80.606350] [CRTC:35:crtc-0] vblank wait timed out
[   80.606386] Modules linked in: imx_sdma
[   80.606484] CPU: 0 PID: 1 Comm: systemd Tainted: G        W         5.16.0-08197-gfeb7a43de5ef-dirty #62
[   80.606520] Hardware name: Freescale i.MX6 SoloX (Device Tree)
[   80.606551]  unwind_backtrace from show_stack+0x10/0x14
[   80.606614]  show_stack from dump_stack_lvl+0x58/0x70
[   80.606665]  dump_stack_lvl from __warn+0xd4/0x154
[   80.606710]  __warn from warn_slowpath_fmt+0x74/0xa8
[   80.606769]  warn_slowpath_fmt from drm_atomic_helper_wait_for_vblanks.part.0+0x278/0x290
[   80.606829]  drm_atomic_helper_wait_for_vblanks.part.0 from drm_atomic_helper_commit_tail_rpm+0x7c/0x8c
[   80.606884]  drm_atomic_helper_commit_tail_rpm from commit_tail+0x9c/0x184
[   80.606933]  commit_tail from drm_atomic_helper_commit+0x158/0x18c
[   80.606979]  drm_atomic_helper_commit from drm_client_modeset_commit_atomic+0x238/0x284
[   80.607034]  drm_client_modeset_commit_atomic from drm_client_modeset_commit_locked+0x60/0x1cc
[   80.607078]  drm_client_modeset_commit_locked from drm_fb_helper_pan_display+0x94/0x1c4
[   80.607131]  drm_fb_helper_pan_display from fb_pan_display+0x9c/0x114
[   80.607176]  fb_pan_display from bit_update_start+0x14/0x30
[   80.607225]  bit_update_start from fbcon_switch+0x2d4/0x45c
[   80.607270]  fbcon_switch from csi_J+0x278/0x298
[   80.607323]  csi_J from do_con_write+0x1f10/0x20e8
[   80.607371]  do_con_write from con_write+0xc/0x20
[   80.607419]  con_write from n_tty_write+0x1e4/0x4cc
[   80.607467]  n_tty_write from file_tty_write.constprop.0+0x148/0x2c4
[   80.607527]  file_tty_write.constprop.0 from vfs_write+0x2ec/0x57c
[   80.607594]  vfs_write from ksys_write+0x60/0xec
[   80.607643]  ksys_write from ret_fast_syscall+0x0/0x1c
[   80.607688] Exception stack(0xc40d5fa8 to 0xc40d5ff0)
[   80.607724] 5fa0:                   0000000a b6f62404 00000017 b6f62404 0000000a 00000000
[   80.607758] 5fc0: 0000000a b6f62404 b6aa1a60 00000004 00000004 00989cc8 0096f040 00000000
[   80.607786] 5fe0: 00000004 be81b9a0 b6d371ff b6d39096
[   80.607816] irq event stamp: 629899
[   80.607850] hardirqs last  enabled at (629905): [<c019b5d0>] vprintk_emit+0x2dc/0x310
[   80.607913] hardirqs last disabled at (629910): [<c019b57c>] vprintk_emit+0x288/0x310
[   80.607964] softirqs last  enabled at (629482): [<c0101518>] __do_softirq+0x2f8/0x5b0
[   80.608015] softirqs last disabled at (629431): [<c012eda8>] __irq_exit_rcu+0x16c/0x1e0
[   80.608077] ---[ end trace 1572e4d782184e69 ]---

I'm working using NXP fs, from 5.10.72 2.2.0 release.
Any suggestion? Maybe some problem on lcdif node?
I take as reference imx6sx-sdb.dtsi

Let me know.

Thanks,
Tommaso

Re: [PATCH 1/2] drm/i2c/tda998x: Switch to atomic operations

[Fabio Estevam]

Hi Tommaso,

On Sat, Jan 15, 2022 at 8:23 PM Tommaso Merciai <tomm.merciai@gmail.com> wrote:

> Hi Fabio,
> I'm working on bring up urt,umsh-8596md-20t lvds kit panel, but after enable
> following node I get the following error:

I assume you are trying to connect an external panel via connector CN3.

This connector is for LVDS panel, not parallel.

The eLCDIF parallel interface is connected to the TDA19988.

Anyway, this is a different topic. My goal here is to fix the kernel
warning when using the TDA19988 HDMI output.

Thanks

Re: [PATCH 1/2] drm/i2c/tda998x: Switch to atomic operations

[Tommaso Merciai]

On Sun, Jan 16, 2022 at 10:27:19AM -0300, Fabio Estevam wrote:
> Hi Tommaso,
> 
> On Sat, Jan 15, 2022 at 8:23 PM Tommaso Merciai <tomm.merciai@gmail.com> wrote:
> 
> > Hi Fabio,
> > I'm working on bring up urt,umsh-8596md-20t lvds kit panel, but after enable
> > following node I get the following error:
> 
> I assume you are trying to connect an external panel via connector CN3.

Hi Fabio,
Right.

> 
> This connector is for LVDS panel, not parallel.

Yep, for that I try also urt,umsh-8596md-19t without success.

> 
> The eLCDIF parallel interface is connected to the TDA19988.

Thanks for the info.
> 
> Anyway, this is a different topic. My goal here is to fix the kernel
> warning when using the TDA19988 HDMI output.

My bad sorry, I'm looking for a second HDMI Display for try to reproduce
the issue on my side. Meanwhile I'm working on lvds.

Thanks,
Tommaso

> 
> Thanks