| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f31eef1eb99542e99486f3df84c90c304b7f2c9c.camel@pengutronix.de>
Date: Mon, 19 Jan 2026 11:08:01 +0100
From: Lucas Stach <l.stach@...gutronix.de>
To: Jacky Bai <ping.bai@....com>, Ulf Hansson <ulf.hansson@...aro.org>,
Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio Estevam
<festevam@...il.com>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Cc: imx@...ts.linux.dev, devicetree@...r.kernel.org,
linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/2] Fix the imx8mm gpu hang due to adb400 reset wrongly
Am Montag, dem 19.01.2026 um 16:53 +0800 schrieb Jacky Bai:
> Due to the HW limitation on i.MX8MM, the gpumix, gpu2d and
> gpu3d share the same reset domain, that means when gpu2d/3d
> go through the power off/on cycle, the gpu2d/3d reset will
> also reset the gpumix domain, The GPUMIX ADB400 port also be
> reset. But the ADB400 must be put into power down before reset
> it.
>
> currently, gpumix, gpu2d/3d power domain use the pm runtime_pm
> to handle these power domain dependency, but in some corner case,
> the gpumix power off will be skipped, then the ADB400 port will
> be in active while gpu2d/3d doing the reset. The GPUMIX the ADB400
> port will be reset wrongly, so lead to unpredictable bus behavior.
>
> To simplify these domain on/off order & dependency, refine the
> code to directly handle GPUMIX domain on/off along with the
> gpu2d/3d power on/off flow.
>
I don't like that we are adding quite a bit of code to this driver just
to work around a issue on a single domain on a single SoC.
Wouldn't it be much easier and produce much the same results if we just
move the ADB hsk bits from the gpumix to the gpu domain? This would
mean we don't trigger the handshake when the gpumix domain is powered
up/down, but I guess that should be fine.
Regards,
Lucas
> Signed-off-by: Jacky Bai <ping.bai@....com>
> ---
> drivers/pmdomain/imx/gpcv2.c | 100 ++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 84 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/pmdomain/imx/gpcv2.c b/drivers/pmdomain/imx/gpcv2.c
> index b7cea89140ee8923f32486eab953c0e1a36bf06d..3a13aa7f1888863048106c2432eec80c7364c462 100644
> --- a/drivers/pmdomain/imx/gpcv2.c
> +++ b/drivers/pmdomain/imx/gpcv2.c
> @@ -53,8 +53,7 @@
> #define IMX8MM_VPUG1_A53_DOMAIN BIT(13)
> #define IMX8MM_DISPMIX_A53_DOMAIN BIT(12)
> #define IMX8MM_VPUMIX_A53_DOMAIN BIT(10)
> -#define IMX8MM_GPUMIX_A53_DOMAIN BIT(9)
> -#define IMX8MM_GPU_A53_DOMAIN (BIT(8) | BIT(11))
> +#define IMX8MM_GPU_A53_DOMAIN (BIT(8) | BIT(9) | BIT(11))
> #define IMX8MM_DDR1_A53_DOMAIN BIT(7)
> #define IMX8MM_OTG2_A53_DOMAIN BIT(5)
> #define IMX8MM_OTG1_A53_DOMAIN BIT(4)
> @@ -292,6 +291,13 @@ struct imx_pgc_domain {
> u32 hskack;
> } bits;
>
> + const struct {
> + u32 pxx;
> + u32 hskreq;
> + u32 hskack;
> + unsigned long pgc;
> + } parent_bits;
> +
> const int voltage;
> const bool keep_clocks;
> struct device *dev;
> @@ -335,6 +341,30 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
> }
> }
>
> + /* Need to do special handling for parent domain like GPUMIX on i.MX8MM */
> + if (domain->parent_bits.pxx) {
> + /* request the domain to power up */
> + regmap_update_bits(domain->regmap, domain->regs->pup,
> + domain->parent_bits.pxx, domain->parent_bits.pxx);
> + /*
> + * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> + * for PUP_REQ/PDN_REQ bit to be cleared
> + */
> + ret = regmap_read_poll_timeout(domain->regmap,
> + domain->regs->pup, reg_val,
> + !(reg_val & domain->parent_bits.pxx),
> + 0, USEC_PER_MSEC);
> + if (ret)
> + dev_err(domain->dev, "failed to command parent PGC\n");
> +
> + /* disable power control */
> + for_each_set_bit(pgc, &domain->parent_bits.pgc, 32) {
> + regmap_clear_bits(domain->regmap, GPC_PGC_CTRL(pgc),
> + GPC_PGC_CTRL_PCR);
> + }
> +
> + }
> +
> reset_control_assert(domain->reset);
>
> /* Enable reset clocks for all devices in the domain */
> @@ -376,6 +406,11 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>
> reset_control_deassert(domain->reset);
>
> + /* request parent ADB400 to power up */
> + if (domain->parent_bits.hskreq)
> + regmap_update_bits(domain->regmap, domain->regs->hsk,
> + domain->parent_bits.hskreq, domain->parent_bits.hskreq);
> +
> /* request the ADB400 to power up */
> if (domain->bits.hskreq) {
> regmap_update_bits(domain->regmap, domain->regs->hsk,
> @@ -438,6 +473,21 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
> }
> }
>
> + /* request the Parent domain ADB400 to power down */
> + if (domain->parent_bits.hskreq) {
> + regmap_clear_bits(domain->regmap, domain->regs->hsk,
> + domain->parent_bits.hskreq);
> +
> + ret = regmap_read_poll_timeout(domain->regmap, domain->regs->hsk,
> + reg_val,
> + !(reg_val & domain->parent_bits.hskack),
> + 0, USEC_PER_MSEC);
> + if (ret) {
> + dev_err(domain->dev, "failed to power down parent ADB400\n");
> + goto out_clk_disable;
> + }
> + }
> +
> /* request the ADB400 to power down */
> if (domain->bits.hskreq) {
> regmap_clear_bits(domain->regmap, domain->regs->hsk,
> @@ -477,6 +527,30 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
> }
> }
>
> + if (domain->parent_bits.pxx) {
> + /* enable power control */
> + for_each_set_bit(pgc, &domain->parent_bits.pgc, 32) {
> + regmap_update_bits(domain->regmap, GPC_PGC_CTRL(pgc),
> + GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
> + }
> +
> + /* request the domain to power down */
> + regmap_update_bits(domain->regmap, domain->regs->pdn,
> + domain->parent_bits.pxx, domain->parent_bits.pxx);
> + /*
> + * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> + * for PUP_REQ/PDN_REQ bit to be cleared
> + */
> + ret = regmap_read_poll_timeout(domain->regmap,
> + domain->regs->pdn, reg_val,
> + !(reg_val & domain->parent_bits.pxx),
> + 0, USEC_PER_MSEC);
> + if (ret) {
> + dev_err(domain->dev, "failed to command Parent PGC\n");
> + goto out_clk_disable;
> + }
> + }
> +
> /* Disable reset clocks for all devices in the domain */
> clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
>
> @@ -787,20 +861,6 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
> .pgc = BIT(IMX8MM_PGC_OTG2),
> },
>
> - [IMX8MM_POWER_DOMAIN_GPUMIX] = {
> - .genpd = {
> - .name = "gpumix",
> - },
> - .bits = {
> - .pxx = IMX8MM_GPUMIX_SW_Pxx_REQ,
> - .map = IMX8MM_GPUMIX_A53_DOMAIN,
> - .hskreq = IMX8MM_GPUMIX_HSK_PWRDNREQN,
> - .hskack = IMX8MM_GPUMIX_HSK_PWRDNACKN,
> - },
> - .pgc = BIT(IMX8MM_PGC_GPUMIX),
> - .keep_clocks = true,
> - },
> -
> [IMX8MM_POWER_DOMAIN_GPU] = {
> .genpd = {
> .name = "gpu",
> @@ -811,6 +871,14 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
> .hskreq = IMX8MM_GPU_HSK_PWRDNREQN,
> .hskack = IMX8MM_GPU_HSK_PWRDNACKN,
> },
> +
> + .parent_bits = {
> + .pxx = IMX8MM_GPUMIX_SW_Pxx_REQ,
> + .hskreq = IMX8MM_GPUMIX_HSK_PWRDNREQN,
> + .hskack = IMX8MM_GPUMIX_HSK_PWRDNACKN,
> + .pgc = BIT(IMX8MM_PGC_GPUMIX),
> + },
> +
> .pgc = BIT(IMX8MM_PGC_GPU2D) | BIT(IMX8MM_PGC_GPU3D),
> },
>
>
Powered by blists - more mailing lists