Re: [RFC PATCH 1/4] drm/etnaviv: Prevent IRQ triggering at probe time on i.MX8MM

From: Lucas Stach <l.stach@pengutronix.de>
To: Schrempf Frieder <frieder.schrempf@kontron.de>,
	Adam Ford <aford173@gmail.com>, Anson Huang <Anson.Huang@nxp.com>,
	Christian Gmeiner <christian.gmeiner@gmail.com>,
	Daniel Baluta <daniel.baluta@nxp.com>,
	Fabio Estevam <festevam@gmail.com>,
	Leonard Crestez <leonard.crestez@nxp.com>,
	Li Jun <jun.li@nxp.com>, NXP Linux Team <linux-imx@nxp.com>,
	Peng Fan <peng.fan@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Russell King <linux+etnaviv@armlinux.org.uk>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Shawn Guo <shawnguo@kernel.org>,
	"S.j. Wang" <shengjiu.wang@nxp.com>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"etnaviv@lists.freedesktop.org" <etnaviv@lists.freedesktop.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH 1/4] drm/etnaviv: Prevent IRQ triggering at probe time on i.MX8MM
Date: Thu, 30 Apr 2020 16:32:27 +0200	[thread overview]
Message-ID: <5e1f804c4c27927d10b2283747c1cae6606abe7c.camel@pengutronix.de> (raw)
In-Reply-To: <20200430124602.14463-2-frieder.schrempf@kontron.de>

Hi Frieder,

Am Donnerstag, den 30.04.2020, 12:46 +0000 schrieb Schrempf Frieder:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> On i.MX8MM there is an interrupt getting triggered immediately after
> requesting the IRQ, which leads to a stall as the handler accesses
> the GPU registers whithout the clock being enabled.
> 
> Enabling the clocks briefly seems to clear the IRQ state, so we do
> this before requesting the IRQ.

This is most likely caused by improper power-up sequencing. Normally
the GPC will trigger a hardware reset of the modules inside a power
domain when the domain is powered on. This requires the clocks to be
running at this point, as those resets are synchronous, so need clock
pulses to propagate through the hardware.

From what I see the i.MX8MM is still missing the power domain
controller integration, but I'm pretty confident that this problem
should be solved in the power domain code, instead of the GPU driver.

Regards,
Lucas

> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 29 ++++++++++++++++++++-----
> --
>  1 file changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index a31eeff2b297..23877c1f150a 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1775,13 +1775,6 @@ static int etnaviv_gpu_platform_probe(struct
> platform_device *pdev)
>  		return gpu->irq;
>  	}
>  
> -	err = devm_request_irq(&pdev->dev, gpu->irq, irq_handler, 0,
> -			       dev_name(gpu->dev), gpu);
> -	if (err) {
> -		dev_err(dev, "failed to request IRQ%u: %d\n", gpu->irq, 
> err);
> -		return err;
> -	}
> -
>  	/* Get Clocks: */
>  	gpu->clk_reg = devm_clk_get(&pdev->dev, "reg");
>  	DBG("clk_reg: %p", gpu->clk_reg);
> @@ -1805,6 +1798,28 @@ static int etnaviv_gpu_platform_probe(struct
> platform_device *pdev)
>  		gpu->clk_shader = NULL;
>  	gpu->base_rate_shader = clk_get_rate(gpu->clk_shader);
>  
> +	/*
> +	 * On i.MX8MM there is an interrupt getting triggered
> immediately
> +	 * after requesting the IRQ, which leads to a stall as the
> handler
> +	 * accesses the GPU registers whithout the clock being enabled.
> +	 * Enabling the clocks briefly seems to clear the IRQ state, so
> we do
> +	 * this here before requesting the IRQ.
> +	 */
> +	err = etnaviv_gpu_clk_enable(gpu);
> +	if (err)
> +		return err;
> +
> +	err = etnaviv_gpu_clk_disable(gpu);
> +	if (err)
> +		return err;
> +
> +	err = devm_request_irq(&pdev->dev, gpu->irq, irq_handler, 0,
> +			       dev_name(gpu->dev), gpu);
> +	if (err) {
> +		dev_err(dev, "failed to request IRQ%u: %d\n", gpu->irq, 
> err);
> +		return err;
> +	}
> +
>  	/* TODO: figure out max mapped size */
>  	dev_set_drvdata(dev, gpu);
>  