Received-SPF: pass (google.com: domain of linux-kernel+bounces-43249-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240106223951.387067-1-aford173@gmail.com> <20240106223951.387067-2-aford173@gmail.com>
 <CAHCN7x+=etco+xEELdf5AyR07sR6c9tTFtsA3K5Dy99qhAZQMg@mail.gmail.com> <DB9PR04MB9452DB580ACF844505F7AA2CF47E2@DB9PR04MB9452.eurprd04.prod.outlook.com>
In-Reply-To: <DB9PR04MB9452DB580ACF844505F7AA2CF47E2@DB9PR04MB9452.eurprd04.prod.outlook.com>
From: Adam Ford <aford173@gmail.com>
Date: Mon, 29 Jan 2024 11:55:37 -0600
Message-ID: <CAHCN7xLwSyGsyo5XigJroB8RftJNXeRDb8o+a-VRSFGSs5NW8A@mail.gmail.com>
Subject: Re: [EXT] Re: [PATCH 2/3] pmdomain: imx8mp-blk-ctrl: imx8mp_blk: Add
 fdcc clock to hdmimix domain
To: Sandor Yu <sandor.yu@nxp.com>
Cc: "linux-pm@vger.kernel.org" <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" <devicetree@vger.kernel.org>, 
	"linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Marek Vasut <marex@denx.de>, 
	"Peng Fan (OSS)" <peng.fan@oss.nxp.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sun, Jan 28, 2024 at 7:39=E2=80=AFPM Sandor Yu <sandor.yu@nxp.com> wrote=
:
>
>
>
> > -----Original Message-----
> > From: Adam Ford <aford173@gmail.com>
> > Sent: 2024=E5=B9=B41=E6=9C=8828=E6=97=A5 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; Mar=
ek
> > Vasut <marex@denx.de>; Peng Fan (OSS) <peng.fan@oss.nxp.com>
> > Subject: [EXT] Re: [PATCH 2/3] pmdomain: imx8mp-blk-ctrl: imx8mp_blk: A=
dd
> > fdcc clock to hdmimix domain
> >
> > Caution: This is an external email. Please take care when clicking link=
s or
> > opening attachments. When in doubt, report the message using the 'Repor=
t
> > this email' button
> >
> >
> > On Sat, Jan 6, 2024 at 4:40=E2=80=AFPM Adam Ford <aford173@gmail.com> w=
rote:
> > >
> > > 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 h=
aven'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 intern=
al use only for future NXP development IP.
> It should not be exposed to customer in document but unfortunately it hav=
e 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 h=
aven=E2=80=99t received any valuable feedback.
> Once design team confirmed, I think the document will update to add the f=
dcc 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[] =3D {
> > >         },
> > >         [IMX8MP_HDMIBLK_PD_LCDIF] =3D {
> > >                 .name =3D "hdmiblk-lcdif",
> > > -               .clk_names =3D (const char *[]){ "axi", "apb" },
> > > -               .num_clks =3D 2,
> > > +               .clk_names =3D (const char *[]){ "axi", "apb", "fdcc"=
 },
> > > +               .num_clks =3D 3,
> > >                 .gpc_name =3D "lcdif",
> > >                 .path_names =3D (const char *[]){"lcdif-hdmi"},
> > >                 .num_paths =3D 1,
> > > @@ -483,8 +483,8 @@ static const struct imx8mp_blk_ctrl_domain_data
> > imx8mp_hdmi_domain_data[] =3D {
> > >         },
> > >         [IMX8MP_HDMIBLK_PD_HDMI_TX] =3D {
> > >                 .name =3D "hdmiblk-hdmi-tx",
> > > -               .clk_names =3D (const char *[]){ "apb", "ref_266m" },
> > > -               .num_clks =3D 2,
> > > +               .clk_names =3D (const char *[]){ "apb", "ref_266m",
> > "fdcc" },
> > > +               .num_clks =3D 3,
> > >                 .gpc_name =3D "hdmi-tx",
> > >         },
> > >         [IMX8MP_HDMIBLK_PD_HDMI_TX_PHY] =3D {
> > > --
> > > 2.43.0
> > >