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 <[email protected]>
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>;
--
2.43.0
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 <[email protected]>
Reviewed-by: Jacky Bai <[email protected]>
Signed-off-by: Adam Ford <[email protected]>
---
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
From: Lucas Stach <[email protected]>
This adds the PGC and HDMI blk-ctrl nodes providing power control for
HDMI subsystem peripherals.
Signed-off-by: Lucas Stach <[email protected]>
Signed-off-by: Adam Ford <[email protected]>
---
V2: Added the fdcc to hdmi_blk_ctrl per NXP's downstream kernel guidance
I (Adam) tried to help move this along, so I took Lucas' patch and
attempted to apply fixes based on feedback. I don't have
all the history, so apologies for that.
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 76c73daf546b..d695c80e710c 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -836,6 +836,23 @@ pgc_mediamix: power-domain@10 {
<&clk IMX8MP_CLK_MEDIA_APB_ROOT>;
};
+ pgc_hdmimix: power-domains@14 {
+ #power-domain-cells = <0>;
+ reg = <IMX8MP_POWER_DOMAIN_HDMIMIX>;
+ clocks = <&clk IMX8MP_CLK_HDMI_ROOT>,
+ <&clk IMX8MP_CLK_HDMI_APB>;
+ assigned-clocks = <&clk IMX8MP_CLK_HDMI_AXI>,
+ <&clk IMX8MP_CLK_HDMI_APB>;
+ assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>,
+ <&clk IMX8MP_SYS_PLL1_133M>;
+ assigned-clock-rates = <500000000>, <133000000>;
+ };
+
+ pgc_hdmi_phy: power-domains@15 {
+ #power-domain-cells = <0>;
+ reg = <IMX8MP_POWER_DOMAIN_HDMI_PHY>;
+ };
+
pgc_mipi_phy2: power-domain@16 {
#power-domain-cells = <0>;
reg = <IMX8MP_POWER_DOMAIN_MIPI_PHY2>;
@@ -1361,6 +1378,25 @@ eqos: ethernet@30bf0000 {
intf_mode = <&gpr 0x4>;
status = "disabled";
};
+
+ hdmi_blk_ctrl: blk-ctrl@32fc0000 {
+ compatible = "fsl,imx8mp-hdmi-blk-ctrl", "syscon";
+ reg = <0x32fc0000 0x23c>;
+ clocks = <&clk IMX8MP_CLK_HDMI_APB>,
+ <&clk IMX8MP_CLK_HDMI_ROOT>,
+ <&clk IMX8MP_CLK_HDMI_REF_266M>,
+ <&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>;
+ power-domain-names = "bus", "irqsteer", "lcdif",
+ "pai", "pvi", "trng",
+ "hdmi-tx", "hdmi-tx-phy";
+ #power-domain-cells = <1>;
+ };
};
aips5: bus@30c00000 {
--
2.43.0
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
<[email protected]> 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 <[email protected]> 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 <[email protected]>
> Reviewed-by: Jacky Bai <[email protected]>
> Signed-off-by: Adam Ford <[email protected]>
> ---
> 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 <[email protected]>
> Sent: 2024年1月28日 2:20
> To: [email protected]
> Cc: Sandor Yu <[email protected]>; Jacky Bai <[email protected]>; Rob
> Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>;
> Shawn Guo <[email protected]>; Sascha Hauer
> <[email protected]>; Pengutronix Kernel Team
> <[email protected]>; Fabio Estevam <[email protected]>;
> dl-linux-imx <[email protected]>; Ulf Hansson <[email protected]>;
> Lucas Stach <[email protected]>; [email protected];
> [email protected]; [email protected]; Marek
> Vasut <[email protected]>; Peng Fan (OSS) <[email protected]>
> 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 <[email protected]> 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 <[email protected]>
> > Reviewed-by: Jacky Bai <[email protected]>
> > Signed-off-by: Adam Ford <[email protected]>
> > ---
> > 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 <[email protected]> wrote:
>
>
>
> > -----Original Message-----
> > From: Adam Ford <[email protected]>
> > Sent: 2024年1月28日 2:20
> > To: [email protected]
> > Cc: Sandor Yu <[email protected]>; Jacky Bai <[email protected]>; Rob
> > Herring <[email protected]>; Krzysztof Kozlowski
> > <[email protected]>; Conor Dooley <[email protected]>;
> > Shawn Guo <[email protected]>; Sascha Hauer
> > <[email protected]>; Pengutronix Kernel Team
> > <[email protected]>; Fabio Estevam <[email protected]>;
> > dl-linux-imx <[email protected]>; Ulf Hansson <[email protected]>;
> > Lucas Stach <[email protected]>; [email protected];
> > [email protected]; [email protected]; Marek
> > Vasut <[email protected]>; Peng Fan (OSS) <[email protected]>
> > 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 <[email protected]> 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 <[email protected]>
> > > Reviewed-by: Jacky Bai <[email protected]>
> > > Signed-off-by: Adam Ford <[email protected]>
> > > ---
> > > 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 <[email protected]>
> Sent: 2024年1月30日 1:56
> To: Sandor Yu <[email protected]>
> Cc: [email protected]; Jacky Bai <[email protected]>; Rob Herring
> <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>;
> Shawn Guo <[email protected]>; Sascha Hauer
> <[email protected]>; Pengutronix Kernel Team
> <[email protected]>; Fabio Estevam <[email protected]>;
> dl-linux-imx <[email protected]>; Ulf Hansson <[email protected]>;
> Lucas Stach <[email protected]>; [email protected];
> [email protected]; [email protected]; Marek
> Vasut <[email protected]>; Peng Fan (OSS) <[email protected]>
> 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 <[email protected]> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Adam Ford <[email protected]>
> > > Sent: 2024年1月28日 2:20
> > > To: [email protected]
> > > Cc: Sandor Yu <[email protected]>; Jacky Bai <[email protected]>; Rob
> > > Herring <[email protected]>; Krzysztof Kozlowski
> > > <[email protected]>; Conor Dooley
> > > <[email protected]>; Shawn Guo <[email protected]>; Sascha
> Hauer
> > > <[email protected]>; Pengutronix Kernel Team
> > > <[email protected]>; Fabio Estevam <[email protected]>;
> > > dl-linux-imx <[email protected]>; Ulf Hansson
> > > <[email protected]>; Lucas Stach <[email protected]>;
> > > [email protected]; [email protected];
> > > [email protected]; Marek Vasut <[email protected]>; Peng Fan
> > > (OSS) <[email protected]>
> > > 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 <[email protected]> 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 <[email protected]>
> > > > Reviewed-by: Jacky Bai <[email protected]>
> > > > Signed-off-by: Adam Ford <[email protected]>
> > > > ---
> > > > 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 <[email protected]> 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 <[email protected]>
> Reviewed-by: Jacky Bai <[email protected]>
> Signed-off-by: Adam Ford <[email protected]>
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 <[email protected]> wrote:
>
> On Sat, 6 Jan 2024 at 23:40, Adam Ford <[email protected]> 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 <[email protected]>
> > Reviewed-by: Jacky Bai <[email protected]>
> > Signed-off-by: Adam Ford <[email protected]>
>
> 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 <[email protected]> wrote:
>
> On Thu, Feb 1, 2024 at 4:33 PM Ulf Hansson <[email protected]> wrote:
> >
> > On Sat, 6 Jan 2024 at 23:40, Adam Ford <[email protected]> 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 <[email protected]>
> > > Reviewed-by: Jacky Bai <[email protected]>
> > > Signed-off-by: Adam Ford <[email protected]>
> >
> > 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