diff mbox series

[2/3] pmdomain: imx8mp-blk-ctrl: imx8mp_blk: Add fdcc clock to hdmimix domain

Message ID 20240106223951.387067-2-aford173@gmail.com
State Superseded
Headers show
Series [1/3] dt-bindings: soc: imx: add fdcc clock to i.MX8MP hdmi blk ctrl | expand

Commit Message

Adam Ford Jan. 6, 2024, 10:39 p.m. UTC 
According to i.MX8MP RM and HDMI ADD, the fdcc clock is part of
hdmi rx verification IP that should not enable for HDMI TX.
But actually if the clock is disabled before HDMI/LCDIF probe,
LCDIF will not get pixel clock from HDMI PHY and print the error
logs:

[CRTC:39:crtc-2] vblank wait timed out
WARNING: CPU: 2 PID: 9 at drivers/gpu/drm/drm_atomic_helper.c:1634 drm_atomic_helper_wait_for_vblanks.part.0+0x23c/0x260

Add fdcc clock to LCDIF and HDMI TX power domains to fix the issue.

Signed-off-by: Sandor Yu <Sandor.yu@nxp.com>
Reviewed-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Adam Ford <aford173@gmail.com>
---
The original work was from Sandor on the NXP Down-stream kernel

Comments

Adam Ford Jan. 27, 2024, 6:19 p.m. UTC | #1 
On Sat, Jan 6, 2024 at 4:40 PM Adam Ford <aford173@gmail.com> wrote:
>
> According to i.MX8MP RM and HDMI ADD, the fdcc clock is part of
> hdmi rx verification IP that should not enable for HDMI TX.
> But actually if the clock is disabled before HDMI/LCDIF probe,
> LCDIF will not get pixel clock from HDMI PHY and print the error
> logs:
>
> [CRTC:39:crtc-2] vblank wait timed out
> WARNING: CPU: 2 PID: 9 at drivers/gpu/drm/drm_atomic_helper.c:1634 drm_atomic_helper_wait_for_vblanks.part.0+0x23c/0x260
>
> Add fdcc clock to LCDIF and HDMI TX power domains to fix the issue.

Peng (or anyone from NXP),

I borrowed this patch from the NXP down-stream kernel for two reasons:
 It's in NXP's branch to address an error & move the fdcc clock out of
the HDMI-tx driver due to questions/feedback that Lucas got on that
driver.

The FDCC clock isn't well documented, and it seems like it's necessary
for the HDMI-TX, but I'd like to make sure this is the proper
solution, and I haven't received any additional feedback.
Can someone from NXP confirm that really is the proper solution?

thank you,

adam

>
> Signed-off-by: Sandor Yu <Sandor.yu@nxp.com>
> Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
> The original work was from Sandor on the NXP Down-stream kernel
>
> diff --git a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> index e3203eb6a022..a56f7f92d091 100644
> --- a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> @@ -55,7 +55,7 @@ struct imx8mp_blk_ctrl_domain_data {
>         const char *gpc_name;
>  };
>
> -#define DOMAIN_MAX_CLKS 2
> +#define DOMAIN_MAX_CLKS 3
>  #define DOMAIN_MAX_PATHS 3
>
>  struct imx8mp_blk_ctrl_domain {
> @@ -457,8 +457,8 @@ static const struct imx8mp_blk_ctrl_domain_data imx8mp_hdmi_domain_data[] = {
>         },
>         [IMX8MP_HDMIBLK_PD_LCDIF] = {
>                 .name = "hdmiblk-lcdif",
> -               .clk_names = (const char *[]){ "axi", "apb" },
> -               .num_clks = 2,
> +               .clk_names = (const char *[]){ "axi", "apb", "fdcc" },
> +               .num_clks = 3,
>                 .gpc_name = "lcdif",
>                 .path_names = (const char *[]){"lcdif-hdmi"},
>                 .num_paths = 1,
> @@ -483,8 +483,8 @@ static const struct imx8mp_blk_ctrl_domain_data imx8mp_hdmi_domain_data[] = {
>         },
>         [IMX8MP_HDMIBLK_PD_HDMI_TX] = {
>                 .name = "hdmiblk-hdmi-tx",
> -               .clk_names = (const char *[]){ "apb", "ref_266m" },
> -               .num_clks = 2,
> +               .clk_names = (const char *[]){ "apb", "ref_266m", "fdcc" },
> +               .num_clks = 3,
>                 .gpc_name = "hdmi-tx",
>         },
>         [IMX8MP_HDMIBLK_PD_HDMI_TX_PHY] = {
> --
> 2.43.0
>
Ulf Hansson Feb. 1, 2024, 10:33 p.m. UTC | #2 
On Sat, 6 Jan 2024 at 23:40, Adam Ford <aford173@gmail.com> wrote:
>
> According to i.MX8MP RM and HDMI ADD, the fdcc clock is part of
> hdmi rx verification IP that should not enable for HDMI TX.
> But actually if the clock is disabled before HDMI/LCDIF probe,
> LCDIF will not get pixel clock from HDMI PHY and print the error
> logs:
>
> [CRTC:39:crtc-2] vblank wait timed out
> WARNING: CPU: 2 PID: 9 at drivers/gpu/drm/drm_atomic_helper.c:1634 drm_atomic_helper_wait_for_vblanks.part.0+0x23c/0x260
>
> Add fdcc clock to LCDIF and HDMI TX power domains to fix the issue.
>
> Signed-off-by: Sandor Yu <Sandor.yu@nxp.com>
> Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Just to let you know, this looks good to me and it seems like the NXP
people like this too. What I am waiting for is an ack on the DT patch,
then I am ready to queue this up.

Kind regards
Uffe

> ---
> The original work was from Sandor on the NXP Down-stream kernel
>
> diff --git a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> index e3203eb6a022..a56f7f92d091 100644
> --- a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> @@ -55,7 +55,7 @@ struct imx8mp_blk_ctrl_domain_data {
>         const char *gpc_name;
>  };
>
> -#define DOMAIN_MAX_CLKS 2
> +#define DOMAIN_MAX_CLKS 3
>  #define DOMAIN_MAX_PATHS 3
>
>  struct imx8mp_blk_ctrl_domain {
> @@ -457,8 +457,8 @@ static const struct imx8mp_blk_ctrl_domain_data imx8mp_hdmi_domain_data[] = {
>         },
>         [IMX8MP_HDMIBLK_PD_LCDIF] = {
>                 .name = "hdmiblk-lcdif",
> -               .clk_names = (const char *[]){ "axi", "apb" },
> -               .num_clks = 2,
> +               .clk_names = (const char *[]){ "axi", "apb", "fdcc" },
> +               .num_clks = 3,
>                 .gpc_name = "lcdif",
>                 .path_names = (const char *[]){"lcdif-hdmi"},
>                 .num_paths = 1,
> @@ -483,8 +483,8 @@ static const struct imx8mp_blk_ctrl_domain_data imx8mp_hdmi_domain_data[] = {
>         },
>         [IMX8MP_HDMIBLK_PD_HDMI_TX] = {
>                 .name = "hdmiblk-hdmi-tx",
> -               .clk_names = (const char *[]){ "apb", "ref_266m" },
> -               .num_clks = 2,
> +               .clk_names = (const char *[]){ "apb", "ref_266m", "fdcc" },
> +               .num_clks = 3,
>                 .gpc_name = "hdmi-tx",
>         },
>         [IMX8MP_HDMIBLK_PD_HDMI_TX_PHY] = {
> --
> 2.43.0
>
Adam Ford Feb. 2, 2024, 12:17 a.m. UTC | #3 
On Thu, Feb 1, 2024 at 4:33 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Sat, 6 Jan 2024 at 23:40, Adam Ford <aford173@gmail.com> wrote:
> >
> > According to i.MX8MP RM and HDMI ADD, the fdcc clock is part of
> > hdmi rx verification IP that should not enable for HDMI TX.
> > But actually if the clock is disabled before HDMI/LCDIF probe,
> > LCDIF will not get pixel clock from HDMI PHY and print the error
> > logs:
> >
> > [CRTC:39:crtc-2] vblank wait timed out
> > WARNING: CPU: 2 PID: 9 at drivers/gpu/drm/drm_atomic_helper.c:1634 drm_atomic_helper_wait_for_vblanks.part.0+0x23c/0x260
> >
> > Add fdcc clock to LCDIF and HDMI TX power domains to fix the issue.
> >
> > Signed-off-by: Sandor Yu <Sandor.yu@nxp.com>
> > Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Just to let you know, this looks good to me and it seems like the NXP
> people like this too. What I am waiting for is an ack on the DT patch,
> then I am ready to queue this up.

What about the bindings?  I'm assuming that Shawn would take the DT
through his IMX tree, but I am not sure if I need to resubmit the
bindings with a different commit message.

adam
>
> Kind regards
> Uffe
>
> > ---
> > The original work was from Sandor on the NXP Down-stream kernel
> >
> > diff --git a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> > index e3203eb6a022..a56f7f92d091 100644
> > --- a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> > +++ b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> > @@ -55,7 +55,7 @@ struct imx8mp_blk_ctrl_domain_data {
> >         const char *gpc_name;
> >  };
> >
> > -#define DOMAIN_MAX_CLKS 2
> > +#define DOMAIN_MAX_CLKS 3
> >  #define DOMAIN_MAX_PATHS 3
> >
> >  struct imx8mp_blk_ctrl_domain {
> > @@ -457,8 +457,8 @@ static const struct imx8mp_blk_ctrl_domain_data imx8mp_hdmi_domain_data[] = {
> >         },
> >         [IMX8MP_HDMIBLK_PD_LCDIF] = {
> >                 .name = "hdmiblk-lcdif",
> > -               .clk_names = (const char *[]){ "axi", "apb" },
> > -               .num_clks = 2,
> > +               .clk_names = (const char *[]){ "axi", "apb", "fdcc" },
> > +               .num_clks = 3,
> >                 .gpc_name = "lcdif",
> >                 .path_names = (const char *[]){"lcdif-hdmi"},
> >                 .num_paths = 1,
> > @@ -483,8 +483,8 @@ static const struct imx8mp_blk_ctrl_domain_data imx8mp_hdmi_domain_data[] = {
> >         },
> >         [IMX8MP_HDMIBLK_PD_HDMI_TX] = {
> >                 .name = "hdmiblk-hdmi-tx",
> > -               .clk_names = (const char *[]){ "apb", "ref_266m" },
> > -               .num_clks = 2,
> > +               .clk_names = (const char *[]){ "apb", "ref_266m", "fdcc" },
> > +               .num_clks = 3,
> >                 .gpc_name = "hdmi-tx",
> >         },
> >         [IMX8MP_HDMIBLK_PD_HDMI_TX_PHY] = {
> > --
> > 2.43.0
> >
Ulf Hansson Feb. 2, 2024, 11:55 a.m. UTC | #4 
On Fri, 2 Feb 2024 at 01:17, Adam Ford <aford173@gmail.com> wrote:
>
> On Thu, Feb 1, 2024 at 4:33 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Sat, 6 Jan 2024 at 23:40, Adam Ford <aford173@gmail.com> wrote:
> > >
> > > According to i.MX8MP RM and HDMI ADD, the fdcc clock is part of
> > > hdmi rx verification IP that should not enable for HDMI TX.
> > > But actually if the clock is disabled before HDMI/LCDIF probe,
> > > LCDIF will not get pixel clock from HDMI PHY and print the error
> > > logs:
> > >
> > > [CRTC:39:crtc-2] vblank wait timed out
> > > WARNING: CPU: 2 PID: 9 at drivers/gpu/drm/drm_atomic_helper.c:1634 drm_atomic_helper_wait_for_vblanks.part.0+0x23c/0x260
> > >
> > > Add fdcc clock to LCDIF and HDMI TX power domains to fix the issue.
> > >
> > > Signed-off-by: Sandor Yu <Sandor.yu@nxp.com>
> > > Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Just to let you know, this looks good to me and it seems like the NXP
> > people like this too. What I am waiting for is an ack on the DT patch,
> > then I am ready to queue this up.
>
> What about the bindings?  I'm assuming that Shawn would take the DT
> through his IMX tree, but I am not sure if I need to resubmit the
> bindings with a different commit message.

I am usually trying to help with patch1 and patch2 for pmdomain
related changes - and I am sharing new/updated DT bindings on an
immutable "dt" branch. Then Shawn can pull in that branch and apply
patch3 to his tree.

So, I need an ack on patch1 from some of the DT maintainers to go
ahead. Unless you want to manage this entirely through Shawn's tree,
that works too. Just let me know.

[...]

Kind regards
Uffe
diff mbox series

Patch

diff --git a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
index e3203eb6a022..a56f7f92d091 100644
--- a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
+++ b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
@@ -55,7 +55,7 @@  struct imx8mp_blk_ctrl_domain_data {
 	const char *gpc_name;
 };
 
-#define DOMAIN_MAX_CLKS 2
+#define DOMAIN_MAX_CLKS 3
 #define DOMAIN_MAX_PATHS 3
 
 struct imx8mp_blk_ctrl_domain {
@@ -457,8 +457,8 @@  static const struct imx8mp_blk_ctrl_domain_data imx8mp_hdmi_domain_data[] = {
 	},
 	[IMX8MP_HDMIBLK_PD_LCDIF] = {
 		.name = "hdmiblk-lcdif",
-		.clk_names = (const char *[]){ "axi", "apb" },
-		.num_clks = 2,
+		.clk_names = (const char *[]){ "axi", "apb", "fdcc" },
+		.num_clks = 3,
 		.gpc_name = "lcdif",
 		.path_names = (const char *[]){"lcdif-hdmi"},
 		.num_paths = 1,
@@ -483,8 +483,8 @@  static const struct imx8mp_blk_ctrl_domain_data imx8mp_hdmi_domain_data[] = {
 	},
 	[IMX8MP_HDMIBLK_PD_HDMI_TX] = {
 		.name = "hdmiblk-hdmi-tx",
-		.clk_names = (const char *[]){ "apb", "ref_266m" },
-		.num_clks = 2,
+		.clk_names = (const char *[]){ "apb", "ref_266m", "fdcc" },
+		.num_clks = 3,
 		.gpc_name = "hdmi-tx",
 	},
 	[IMX8MP_HDMIBLK_PD_HDMI_TX_PHY] = {