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

[Sandor Yu]

> -----Original Message-----
> From: Adam Ford <aford173@xxxxxxxxx>
> Sent: 2024年1月30日 1:56
> To: Sandor Yu <sandor.yu@xxxxxxx>
> Cc: linux-pm@xxxxxxxxxxxxxxx; Jacky Bai <ping.bai@xxxxxxx>; Rob Herring
> <robh+dt@xxxxxxxxxx>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>;
> Shawn Guo <shawnguo@xxxxxxxxxx>; Sascha Hauer
> <s.hauer@xxxxxxxxxxxxxx>; Pengutronix Kernel Team
> <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>;
> dl-linux-imx <linux-imx@xxxxxxx>; Ulf Hansson <ulf.hansson@xxxxxxxxxx>;
> Lucas Stach <l.stach@xxxxxxxxxxxxxx>; devicetree@xxxxxxxxxxxxxxx;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Marek
> Vasut <marex@xxxxxxx>; Peng Fan (OSS) <peng.fan@xxxxxxxxxxx>
> 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@xxxxxxx> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Adam Ford <aford173@xxxxxxxxx>
> > > Sent: 2024年1月28日 2:20
> > > To: linux-pm@xxxxxxxxxxxxxxx
> > > Cc: Sandor Yu <sandor.yu@xxxxxxx>; Jacky Bai <ping.bai@xxxxxxx>; Rob
> > > Herring <robh+dt@xxxxxxxxxx>; Krzysztof Kozlowski
> > > <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley
> > > <conor+dt@xxxxxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>; Sascha
> Hauer
> > > <s.hauer@xxxxxxxxxxxxxx>; Pengutronix Kernel Team
> > > <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>;
> > > dl-linux-imx <linux-imx@xxxxxxx>; Ulf Hansson
> > > <ulf.hansson@xxxxxxxxxx>; Lucas Stach <l.stach@xxxxxxxxxxxxxx>;
> > > devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> > > linux-kernel@xxxxxxxxxxxxxxx; Marek Vasut <marex@xxxxxxx>; Peng Fan
> > > (OSS) <peng.fan@xxxxxxxxxxx>
> > > 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@xxxxxxxxx> 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@xxxxxxx>
> > > > Reviewed-by: Jacky Bai <ping.bai@xxxxxxx>
> > > > Signed-off-by: Adam Ford <aford173@xxxxxxxxx>
> > > > ---
> > > > 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
> > > >