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

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 <[e-mail]>
Reviewed-by: Jacky Bai <[e-mail]>
Signed-off-by: Adam Ford <[e-mail]>
---
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 <[e-mail]> 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 <[e-mail]>
> Reviewed-by: Jacky Bai <[e-mail]>
> Signed-off-by: Adam Ford <[e-mail]>
> ---
> 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
>

Sandor Yu Jan. 29, 2024, 1:38 a.m. UTC | #2

> -----Original Message-----
> From: Adam Ford <[e-mail]>
> Sent: 2024年1月28日 2:20
> To: [e-mail]
> Cc: Sandor Yu <[e-mail]>; Jacky Bai <[e-mail]>; Rob
> Herring <[e-mail]>; Krzysztof Kozlowski
> <[e-mail]>; Conor Dooley <[e-mail]>;
> Shawn Guo <[e-mail]>; Sascha Hauer
> <[e-mail]>; Pengutronix Kernel Team
> <[e-mail]>; Fabio Estevam <[e-mail]>;
> dl-linux-imx <[e-mail]>; Ulf Hansson <[e-mail]>;
> Lucas Stach <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; Marek
> Vasut <[e-mail]>; Peng Fan (OSS) <[e-mail]>
> Subject: [EXT] Re: [PATCH 2/3] pmdomain: imx8mp-blk-ctrl: imx8mp_blk: Add
> fdcc clock to hdmimix domain
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> On Sat, Jan 6, 2024 at 4:40 PM Adam Ford <[e-mail]> 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

Hi Adam,

In NXP internal document, the clock HDMI_FDCC_TST_CLK_ROOT was for internal use only for future NXP development IP.
It should not be exposed to customer in document but unfortunately it have to be enabled for HDMITX.

I submitted a request ticket to the design team several months ago, 
Generally, tickets of this didn't get the priority in design team and I haven’t received any valuable feedback.
Once design team confirmed, I think the document will update to add the fdcc clock.

B.R
Sandor

> 
> >
> > Signed-off-by: Sandor Yu <[e-mail]>
> > Reviewed-by: Jacky Bai <[e-mail]>
> > Signed-off-by: Adam Ford <[e-mail]>
> > ---
> > 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 Jan. 29, 2024, 5:55 p.m. UTC | #3

On Sun, Jan 28, 2024 at 7:39 PM Sandor Yu <[e-mail]> wrote:
>
>
>
> > -----Original Message-----
> > From: Adam Ford <[e-mail]>
> > Sent: 2024年1月28日 2:20
> > To: [e-mail]
> > Cc: Sandor Yu <[e-mail]>; Jacky Bai <[e-mail]>; Rob
> > Herring <[e-mail]>; Krzysztof Kozlowski
> > <[e-mail]>; Conor Dooley <[e-mail]>;
> > Shawn Guo <[e-mail]>; Sascha Hauer
> > <[e-mail]>; Pengutronix Kernel Team
> > <[e-mail]>; Fabio Estevam <[e-mail]>;
> > dl-linux-imx <[e-mail]>; Ulf Hansson <[e-mail]>;
> > Lucas Stach <[e-mail]>; [e-mail];
> > [e-mail]; [e-mail]; Marek
> > Vasut <[e-mail]>; Peng Fan (OSS) <[e-mail]>
> > Subject: [EXT] Re: [PATCH 2/3] pmdomain: imx8mp-blk-ctrl: imx8mp_blk: Add
> > fdcc clock to hdmimix domain
> >
> > Caution: This is an external email. Please take care when clicking links or
> > opening attachments. When in doubt, report the message using the 'Report
> > this email' button
> >
> >
> > On Sat, Jan 6, 2024 at 4:40 PM Adam Ford <[e-mail]> 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
>
> Hi Adam,
>

Sandor,

> In NXP internal document, the clock HDMI_FDCC_TST_CLK_ROOT was for internal use only for future NXP development IP.
> It should not be exposed to customer in document but unfortunately it have to be enabled for HDMITX.
>
> I submitted a request ticket to the design team several months ago,
> Generally, tickets of this didn't get the priority in design team and I haven’t received any valuable feedback.
> Once design team confirmed, I think the document will update to add the fdcc clock.

Thank you for your response.   Do you have any objections to having
the FDCC clock added to the power domain driver?  I know there are
several of us who would really like to see the HDMI-TX driver applied,
and I think this patch gets us one step closer.

thanks,

adam
>
> B.R
> Sandor
>
> >
> > >
> > > Signed-off-by: Sandor Yu <[e-mail]>
> > > Reviewed-by: Jacky Bai <[e-mail]>
> > > Signed-off-by: Adam Ford <[e-mail]>
> > > ---
> > > 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
> > >

Sandor Yu Jan. 30, 2024, 2:28 a.m. UTC | #4

> -----Original Message-----
> From: Adam Ford <[e-mail]>
> Sent: 2024年1月30日 1:56
> To: Sandor Yu <[e-mail]>
> Cc: [e-mail]; Jacky Bai <[e-mail]>; Rob Herring
> <[e-mail]>; Krzysztof Kozlowski
> <[e-mail]>; Conor Dooley <[e-mail]>;
> Shawn Guo <[e-mail]>; Sascha Hauer
> <[e-mail]>; Pengutronix Kernel Team
> <[e-mail]>; Fabio Estevam <[e-mail]>;
> dl-linux-imx <[e-mail]>; Ulf Hansson <[e-mail]>;
> Lucas Stach <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; Marek
> Vasut <[e-mail]>; Peng Fan (OSS) <[e-mail]>
> Subject: Re: [EXT] Re: [PATCH 2/3] pmdomain: imx8mp-blk-ctrl: imx8mp_blk:
> Add fdcc clock to hdmimix domain
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> On Sun, Jan 28, 2024 at 7:39 PM Sandor Yu <[e-mail]> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Adam Ford <[e-mail]>
> > > Sent: 2024年1月28日 2:20
> > > To: [e-mail]
> > > Cc: Sandor Yu <[e-mail]>; Jacky Bai <[e-mail]>; Rob
> > > Herring <[e-mail]>; Krzysztof Kozlowski
> > > <[e-mail]>; Conor Dooley
> > > <[e-mail]>; Shawn Guo <[e-mail]>; Sascha
> Hauer
> > > <[e-mail]>; Pengutronix Kernel Team
> > > <[e-mail]>; Fabio Estevam <[e-mail]>;
> > > dl-linux-imx <[e-mail]>; Ulf Hansson
> > > <[e-mail]>; Lucas Stach <[e-mail]>;
> > > [e-mail]; [e-mail];
> > > [e-mail]; Marek Vasut <[e-mail]>; Peng Fan
> > > (OSS) <[e-mail]>
> > > Subject: [EXT] Re: [PATCH 2/3] pmdomain: imx8mp-blk-ctrl:
> > > imx8mp_blk: Add fdcc clock to hdmimix domain
> > >
> > > Caution: This is an external email. Please take care when clicking
> > > links or opening attachments. When in doubt, report the message
> > > using the 'Report this email' button
> > >
> > >
> > > On Sat, Jan 6, 2024 at 4:40 PM Adam Ford <[e-mail]> 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
> >
> > Hi Adam,
> >
> 
> Sandor,
> 
> > In NXP internal document, the clock HDMI_FDCC_TST_CLK_ROOT was for
> internal use only for future NXP development IP.
> > It should not be exposed to customer in document but unfortunately it have
> to be enabled for HDMITX.
> >
> > I submitted a request ticket to the design team several months ago,
> > Generally, tickets of this didn't get the priority in design team and I haven’t
> received any valuable feedback.
> > Once design team confirmed, I think the document will update to add the
> fdcc clock.
> 

Hi Adam,

> Thank you for your response.   Do you have any objections to having
> the FDCC clock added to the power domain driver?  I know there are several
> of us who would really like to see the HDMI-TX driver applied, and I think this
> patch gets us one step closer.
> 
As I mentioned above, the FDCC clock is not for HDMITX in desgin, but it is part of HDMI domain that needed by HDMITX.
So I think it is reasonable added it to the power domain driver.

B.R
Sandor

> thanks,
> 
> adam
> >
> > B.R
> > Sandor
> >
> > >
> > > >
> > > > Signed-off-by: Sandor Yu <[e-mail]>
> > > > Reviewed-by: Jacky Bai <[e-mail]>
> > > > Signed-off-by: Adam Ford <[e-mail]>
> > > > ---
> > > > 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 | #5

On Sat, 6 Jan 2024 at 23:40, Adam Ford <[e-mail]> 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 <[e-mail]>
> Reviewed-by: Jacky Bai <[e-mail]>
> Signed-off-by: Adam Ford <[e-mail]>

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 | #6

On Thu, Feb 1, 2024 at 4:33 PM Ulf Hansson <[e-mail]> wrote:
>
> On Sat, 6 Jan 2024 at 23:40, Adam Ford <[e-mail]> 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 <[e-mail]>
> > Reviewed-by: Jacky Bai <[e-mail]>
> > Signed-off-by: Adam Ford <[e-mail]>
>
> 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 | #7

On Fri, 2 Feb 2024 at 01:17, Adam Ford <[e-mail]> wrote:
>
> On Thu, Feb 1, 2024 at 4:33 PM Ulf Hansson <[e-mail]> wrote:
> >
> > On Sat, 6 Jan 2024 at 23:40, Adam Ford <[e-mail]> 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 <[e-mail]>
> > > Reviewed-by: Jacky Bai <[e-mail]>
> > > Signed-off-by: Adam Ford <[e-mail]>
> >
> > 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

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] = {