| Message ID | 20240106223951.387067-1-aford173@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/3] dt-bindings: soc: imx: add fdcc clock to i.MX8MP hdmi blk ctrl | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 06/01/2024 23:39, Adam Ford wrote: > Per guidance from the NXP downstream kernel, if the clock is > disabled before HDMI/LCDIF probe, LCDIF will not get pixel > clock from HDMI PHY and throw an error. Fix this by adding > the fdcc clock to the hdmi_blk_ctrl. > Adding a required clock is ABI break and commit msg is not clear whether this was actually necessary or how did it worked so far... Best regards, Krzysztof
On Sun, Jan 7, 2024 at 4:53 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 06/01/2024 23:39, Adam Ford wrote: > > Per guidance from the NXP downstream kernel, if the clock is > > disabled before HDMI/LCDIF probe, LCDIF will not get pixel > > clock from HDMI PHY and throw an error. Fix this by adding > > the fdcc clock to the hdmi_blk_ctrl. > > > > Adding a required clock is ABI break and commit msg is not clear whether > this was actually necessary or how did it worked so far... As of right now, this power domain isn't enabled in the device tree. This power domain is necessary for the HDMI driver which is split across several drivers which Lucas attempted to push but got some feedback. One of those items in question is the enabling of FDCC during the HDMI Tx probe. The NXP documentation is vague on what this clock is and who uses it. When I did my investigation into how NXP used it, it turned out they didn't use it in the HDMI TX driver, because they moved it to the power domain and referenced it from both the LCDIF and the HDMI TX. NXP's downstream commit didn't get into a lot of detail on how to reproduce the error, but it sounded like some sort of order of operations issue between the LCDIF and HDMI TX. Moving this to the power domain appeared to solve the issue for them, and it seemed like it would have also resolved the concern about its use in the HDMI TX driver that was submitted on the mailing list. In the subsequent patch, I listed the error that NXP described, but I can re-do this commit message to make it more clear if you let me know what you're wanting to see. adam > > Best regards, > Krzysztof >
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 >
> -----Original Message----- > From: Adam Ford <aford173@gmail.com> > Sent: 2024年1月28日 2:20 > To: linux-pm@vger.kernel.org > Cc: Sandor Yu <sandor.yu@nxp.com>; Jacky Bai <ping.bai@nxp.com>; Rob > Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Pengutronix Kernel Team > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; > dl-linux-imx <linux-imx@nxp.com>; Ulf Hansson <ulf.hansson@linaro.org>; > Lucas Stach <l.stach@pengutronix.de>; devicetree@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Marek > Vasut <marex@denx.de>; Peng Fan (OSS) <peng.fan@oss.nxp.com> > 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 <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 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 <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 > >
On Sun, Jan 28, 2024 at 7:39 PM Sandor Yu <sandor.yu@nxp.com> wrote: > > > > > -----Original Message----- > > From: Adam Ford <aford173@gmail.com> > > Sent: 2024年1月28日 2:20 > > To: linux-pm@vger.kernel.org > > Cc: Sandor Yu <sandor.yu@nxp.com>; Jacky Bai <ping.bai@nxp.com>; Rob > > Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > > Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > > <s.hauer@pengutronix.de>; Pengutronix Kernel Team > > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; > > dl-linux-imx <linux-imx@nxp.com>; Ulf Hansson <ulf.hansson@linaro.org>; > > Lucas Stach <l.stach@pengutronix.de>; devicetree@vger.kernel.org; > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Marek > > Vasut <marex@denx.de>; Peng Fan (OSS) <peng.fan@oss.nxp.com> > > 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 <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 > > 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 <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 > > >
> -----Original Message----- > From: Adam Ford <aford173@gmail.com> > Sent: 2024年1月30日 1:56 > To: Sandor Yu <sandor.yu@nxp.com> > Cc: linux-pm@vger.kernel.org; Jacky Bai <ping.bai@nxp.com>; Rob Herring > <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Pengutronix Kernel Team > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; > dl-linux-imx <linux-imx@nxp.com>; Ulf Hansson <ulf.hansson@linaro.org>; > Lucas Stach <l.stach@pengutronix.de>; devicetree@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Marek > Vasut <marex@denx.de>; Peng Fan (OSS) <peng.fan@oss.nxp.com> > 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 <sandor.yu@nxp.com> wrote: > > > > > > > > > -----Original Message----- > > > From: Adam Ford <aford173@gmail.com> > > > Sent: 2024年1月28日 2:20 > > > To: linux-pm@vger.kernel.org > > > Cc: Sandor Yu <sandor.yu@nxp.com>; Jacky Bai <ping.bai@nxp.com>; Rob > > > Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > > > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley > > > <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha > Hauer > > > <s.hauer@pengutronix.de>; Pengutronix Kernel Team > > > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; > > > dl-linux-imx <linux-imx@nxp.com>; Ulf Hansson > > > <ulf.hansson@linaro.org>; Lucas Stach <l.stach@pengutronix.de>; > > > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > > > linux-kernel@vger.kernel.org; Marek Vasut <marex@denx.de>; Peng Fan > > > (OSS) <peng.fan@oss.nxp.com> > > > 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 <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 > > > > 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 <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 > > > >
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 >
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 > >
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 --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml index 1be4ce2a45e8..741b5d8da4bb 100644 --- a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml @@ -42,8 +42,8 @@ properties: - const: hdmi-tx-phy clocks: - minItems: 4 - maxItems: 4 + minItems: 5 + maxItems: 5 clock-names: items: @@ -51,6 +51,7 @@ properties: - const: axi - const: ref_266m - const: ref_24m + - const: fdcc interconnects: maxItems: 3 @@ -82,8 +83,9 @@ examples: clocks = <&clk IMX8MP_CLK_HDMI_APB>, <&clk IMX8MP_CLK_HDMI_ROOT>, <&clk IMX8MP_CLK_HDMI_REF_266M>, - <&clk IMX8MP_CLK_HDMI_24M>; - clock-names = "apb", "axi", "ref_266m", "ref_24m"; + <&clk IMX8MP_CLK_HDMI_24M>, + <&clk IMX8MP_CLK_HDMI_FDCC_TST>; + clock-names = "apb", "axi", "ref_266m", "ref_24m", "fdcc"; power-domains = <&pgc_hdmimix>, <&pgc_hdmimix>, <&pgc_hdmimix>, <&pgc_hdmimix>, <&pgc_hdmimix>, <&pgc_hdmimix>, <&pgc_hdmimix>, <&pgc_hdmi_phy>;
Per guidance from the NXP downstream kernel, if the clock is disabled before HDMI/LCDIF probe, LCDIF will not get pixel clock from HDMI PHY and throw an error. Fix this by adding the fdcc clock to the hdmi_blk_ctrl. Signed-off-by: Adam Ford <aford173@gmail.com>