[PATCH 1/3] drm/tegra: sor: Suspend on clock registration failure

[PATCH 1/3] drm/tegra: sor: Suspend on clock registration failure

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Make sure the SOR module is suspenden after we fail to register the SOR
pad output clock.

Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/tegra/sor.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 41d24949478e..96cd89bb2e82 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -3922,15 +3922,16 @@ static int tegra_sor_probe(struct platform_device *pdev)
 	if (!sor->clk_pad) {
 		char *name;
 
-		err = host1x_client_resume(&sor->client);
-		if (err < 0) {
-			dev_err(sor->dev, "failed to resume: %d\n", err);
+		name = devm_kasprintf(sor->dev, GFP_KERNEL, "sor%u_pad_clkout",
+				      sor->index);
+		if (!name) {
+			err = -ENOMEM;
 			goto remove;
 		}
 
-		name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "sor%u_pad_clkout", sor->index);
-		if (!name) {
-			err = -ENOMEM;
+		err = host1x_client_resume(&sor->client);
+		if (err < 0) {
+			dev_err(sor->dev, "failed to resume: %d\n", err);
 			goto remove;
 		}
 
-- 
2.24.1

[PATCH 2/3] drm/tegra: sor: Disable runtime PM on probe failure

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

If the driver fails to probe, make sure to disable runtime PM again.

While at it, make the cleanup code in ->remove() symmetric.

Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/tegra/sor.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 96cd89bb2e82..aa4e1695b537 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -3926,13 +3926,13 @@ static int tegra_sor_probe(struct platform_device *pdev)
 				      sor->index);
 		if (!name) {
 			err = -ENOMEM;
-			goto remove;
+			goto rpm_disable;
 		}
 
 		err = host1x_client_resume(&sor->client);
 		if (err < 0) {
 			dev_err(sor->dev, "failed to resume: %d\n", err);
-			goto remove;
+			goto rpm_disable;
 		}
 
 		sor->clk_pad = tegra_clk_sor_pad_register(sor, name);
@@ -3943,7 +3943,7 @@ static int tegra_sor_probe(struct platform_device *pdev)
 		err = PTR_ERR(sor->clk_pad);
 		dev_err(&pdev->dev, "failed to register SOR pad clock: %d\n",
 			err);
-		goto remove;
+		goto rpm_disable;
 	}
 
 	INIT_LIST_HEAD(&sor->client.list);
@@ -3954,11 +3954,13 @@ static int tegra_sor_probe(struct platform_device *pdev)
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to register host1x client: %d\n",
 			err);
-		goto remove;
+		goto rpm_disable;
 	}
 
 	return 0;
 
+rpm_disable:
+	pm_runtime_disable(&pdev->dev);
 remove:
 	if (sor->ops && sor->ops->remove)
 		sor->ops->remove(sor);
@@ -3972,8 +3974,6 @@ static int tegra_sor_remove(struct platform_device *pdev)
 	struct tegra_sor *sor = platform_get_drvdata(pdev);
 	int err;
 
-	pm_runtime_disable(&pdev->dev);
-
 	err = host1x_client_unregister(&sor->client);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to unregister host1x client: %d\n",
@@ -3981,6 +3981,8 @@ static int tegra_sor_remove(struct platform_device *pdev)
 		return err;
 	}
 
+	pm_runtime_disable(&pdev->dev);
+
 	if (sor->ops && sor->ops->remove) {
 		err = sor->ops->remove(sor);
 		if (err < 0)
-- 
2.24.1

[PATCH 3/3] drm/tegra: sor: Initialize runtime PM before use

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Commit fd67e9c6ed5a ("drm/tegra: Do not implement runtime PM") replaced
the generic runtime PM usage by a host1x bus-specific implementation in
order to work around some assumptions baked into runtime PM that are in
conflict with the requirements in the Tegra DRM driver.

Unfortunately the new runtime PM callbacks are not setup yet at the time
when the SOR driver first needs to resume the device to register the SOR
pad clock, and accesses to register will cause the system to hang.

Note that this only happens on Tegra124 and Tegra210 because those are
the only SoCs where the SOR pad clock is registered from the SOR driver.
Later generations use a SOR pad clock provided by the BPMP.

Fix this by moving the registration of the SOR pad clock after the
host1x client has been registered. That's somewhat suboptimal because
this could potentially, though it's very unlikely, cause the Tegra DRM
to be probed if the SOR happens to be the last subdevice to register,
only to be immediately removed again if the SOR pad output clock fails
to register. That's just a minor annoyance, though, and doesn't justify
implementing a workaround.

Fixes: fd67e9c6ed5a ("drm/tegra: Do not implement runtime PM")
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/tegra/sor.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index aa4e1695b537..81226a4953c1 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -3915,6 +3915,17 @@ static int tegra_sor_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, sor);
 	pm_runtime_enable(&pdev->dev);
 
+	INIT_LIST_HEAD(&sor->client.list);
+	sor->client.ops = &sor_client_ops;
+	sor->client.dev = &pdev->dev;
+
+	err = host1x_client_register(&sor->client);
+	if (err < 0) {
+		dev_err(&pdev->dev, "failed to register host1x client: %d\n",
+			err);
+		goto rpm_disable;
+	}
+
 	/*
 	 * On Tegra210 and earlier, provide our own implementation for the
 	 * pad output clock.
@@ -3926,13 +3937,13 @@ static int tegra_sor_probe(struct platform_device *pdev)
 				      sor->index);
 		if (!name) {
 			err = -ENOMEM;
-			goto rpm_disable;
+			goto unregister;
 		}
 
 		err = host1x_client_resume(&sor->client);
 		if (err < 0) {
 			dev_err(sor->dev, "failed to resume: %d\n", err);
-			goto rpm_disable;
+			goto unregister;
 		}
 
 		sor->clk_pad = tegra_clk_sor_pad_register(sor, name);
@@ -3941,24 +3952,15 @@ static int tegra_sor_probe(struct platform_device *pdev)
 
 	if (IS_ERR(sor->clk_pad)) {
 		err = PTR_ERR(sor->clk_pad);
-		dev_err(&pdev->dev, "failed to register SOR pad clock: %d\n",
+		dev_err(sor->dev, "failed to register SOR pad clock: %d\n",
 			err);
-		goto rpm_disable;
-	}
-
-	INIT_LIST_HEAD(&sor->client.list);
-	sor->client.ops = &sor_client_ops;
-	sor->client.dev = &pdev->dev;
-
-	err = host1x_client_register(&sor->client);
-	if (err < 0) {
-		dev_err(&pdev->dev, "failed to register host1x client: %d\n",
-			err);
-		goto rpm_disable;
+		goto unregister;
 	}
 
 	return 0;
 
+unregister:
+	host1x_client_unregister(&sor->client);
 rpm_disable:
 	pm_runtime_disable(&pdev->dev);
 remove:
-- 
2.24.1

Re: [PATCH 1/3] drm/tegra: sor: Suspend on clock registration failure

[Jon Hunter]

On 31/01/2020 16:59, Thierry Reding wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Make sure the SOR module is suspenden after we fail to register the SOR
> pad output clock.

s/suspenden/suspended/

> 
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/gpu/drm/tegra/sor.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index 41d24949478e..96cd89bb2e82 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -3922,15 +3922,16 @@ static int tegra_sor_probe(struct platform_device *pdev)
>  	if (!sor->clk_pad) {
>  		char *name;
>  
> -		err = host1x_client_resume(&sor->client);
> -		if (err < 0) {
> -			dev_err(sor->dev, "failed to resume: %d\n", err);
> +		name = devm_kasprintf(sor->dev, GFP_KERNEL, "sor%u_pad_clkout",
> +				      sor->index);
> +		if (!name) {
> +			err = -ENOMEM;
>  			goto remove;
>  		}
>  
> -		name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "sor%u_pad_clkout", sor->index);
> -		if (!name) {
> -			err = -ENOMEM;
> +		err = host1x_client_resume(&sor->client);
> +		if (err < 0) {
> +			dev_err(sor->dev, "failed to resume: %d\n", err);
>  			goto remove;
>  		}
>  
> 

Otherwise ...

Reviewed-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Tested-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Cheers
Jon

-- 
nvpublic

Re: [PATCH 2/3] drm/tegra: sor: Disable runtime PM on probe failure

[Jon Hunter]

On 31/01/2020 16:59, Thierry Reding wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> If the driver fails to probe, make sure to disable runtime PM again.
> 
> While at it, make the cleanup code in ->remove() symmetric.
> 
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/gpu/drm/tegra/sor.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index 96cd89bb2e82..aa4e1695b537 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -3926,13 +3926,13 @@ static int tegra_sor_probe(struct platform_device *pdev)
>  				      sor->index);
>  		if (!name) {
>  			err = -ENOMEM;
> -			goto remove;
> +			goto rpm_disable;
>  		}
>  
>  		err = host1x_client_resume(&sor->client);
>  		if (err < 0) {
>  			dev_err(sor->dev, "failed to resume: %d\n", err);
> -			goto remove;
> +			goto rpm_disable;
>  		}
>  
>  		sor->clk_pad = tegra_clk_sor_pad_register(sor, name);
> @@ -3943,7 +3943,7 @@ static int tegra_sor_probe(struct platform_device *pdev)
>  		err = PTR_ERR(sor->clk_pad);
>  		dev_err(&pdev->dev, "failed to register SOR pad clock: %d\n",
>  			err);
> -		goto remove;
> +		goto rpm_disable;
>  	}
>  
>  	INIT_LIST_HEAD(&sor->client.list);
> @@ -3954,11 +3954,13 @@ static int tegra_sor_probe(struct platform_device *pdev)
>  	if (err < 0) {
>  		dev_err(&pdev->dev, "failed to register host1x client: %d\n",
>  			err);
> -		goto remove;
> +		goto rpm_disable;
>  	}
>  
>  	return 0;
>  
> +rpm_disable:
> +	pm_runtime_disable(&pdev->dev);
>  remove:
>  	if (sor->ops && sor->ops->remove)
>  		sor->ops->remove(sor);
> @@ -3972,8 +3974,6 @@ static int tegra_sor_remove(struct platform_device *pdev)
>  	struct tegra_sor *sor = platform_get_drvdata(pdev);
>  	int err;
>  
> -	pm_runtime_disable(&pdev->dev);
> -
>  	err = host1x_client_unregister(&sor->client);
>  	if (err < 0) {
>  		dev_err(&pdev->dev, "failed to unregister host1x client: %d\n",
> @@ -3981,6 +3981,8 @@ static int tegra_sor_remove(struct platform_device *pdev)
>  		return err;
>  	}
>  
> +	pm_runtime_disable(&pdev->dev);
> +
>  	if (sor->ops && sor->ops->remove) {
>  		err = sor->ops->remove(sor);
>  		if (err < 0)
> 

Reviewed-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Tested-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Cheers
Jon

-- 
nvpublic

Re: [PATCH 3/3] drm/tegra: sor: Initialize runtime PM before use

[Jon Hunter]

On 31/01/2020 16:59, Thierry Reding wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Commit fd67e9c6ed5a ("drm/tegra: Do not implement runtime PM") replaced
> the generic runtime PM usage by a host1x bus-specific implementation in
> order to work around some assumptions baked into runtime PM that are in
> conflict with the requirements in the Tegra DRM driver.
> 
> Unfortunately the new runtime PM callbacks are not setup yet at the time
> when the SOR driver first needs to resume the device to register the SOR
> pad clock, and accesses to register will cause the system to hang.
> 
> Note that this only happens on Tegra124 and Tegra210 because those are
> the only SoCs where the SOR pad clock is registered from the SOR driver.
> Later generations use a SOR pad clock provided by the BPMP.
> 
> Fix this by moving the registration of the SOR pad clock after the
> host1x client has been registered. That's somewhat suboptimal because
> this could potentially, though it's very unlikely, cause the Tegra DRM
> to be probed if the SOR happens to be the last subdevice to register,
> only to be immediately removed again if the SOR pad output clock fails
> to register. That's just a minor annoyance, though, and doesn't justify
> implementing a workaround.
> 
> Fixes: fd67e9c6ed5a ("drm/tegra: Do not implement runtime PM")
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/gpu/drm/tegra/sor.c | 32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index aa4e1695b537..81226a4953c1 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -3915,6 +3915,17 @@ static int tegra_sor_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, sor);
>  	pm_runtime_enable(&pdev->dev);
>  
> +	INIT_LIST_HEAD(&sor->client.list);
> +	sor->client.ops = &sor_client_ops;
> +	sor->client.dev = &pdev->dev;
> +
> +	err = host1x_client_register(&sor->client);
> +	if (err < 0) {
> +		dev_err(&pdev->dev, "failed to register host1x client: %d\n",
> +			err);
> +		goto rpm_disable;
> +	}
> +
>  	/*
>  	 * On Tegra210 and earlier, provide our own implementation for the
>  	 * pad output clock.
> @@ -3926,13 +3937,13 @@ static int tegra_sor_probe(struct platform_device *pdev)
>  				      sor->index);
>  		if (!name) {
>  			err = -ENOMEM;
> -			goto rpm_disable;
> +			goto unregister;
>  		}
>  
>  		err = host1x_client_resume(&sor->client);
>  		if (err < 0) {
>  			dev_err(sor->dev, "failed to resume: %d\n", err);
> -			goto rpm_disable;
> +			goto unregister;
>  		}
>  
>  		sor->clk_pad = tegra_clk_sor_pad_register(sor, name);
> @@ -3941,24 +3952,15 @@ static int tegra_sor_probe(struct platform_device *pdev)
>  
>  	if (IS_ERR(sor->clk_pad)) {
>  		err = PTR_ERR(sor->clk_pad);
> -		dev_err(&pdev->dev, "failed to register SOR pad clock: %d\n",
> +		dev_err(sor->dev, "failed to register SOR pad clock: %d\n",
>  			err);
> -		goto rpm_disable;
> -	}
> -
> -	INIT_LIST_HEAD(&sor->client.list);
> -	sor->client.ops = &sor_client_ops;
> -	sor->client.dev = &pdev->dev;
> -
> -	err = host1x_client_register(&sor->client);
> -	if (err < 0) {
> -		dev_err(&pdev->dev, "failed to register host1x client: %d\n",
> -			err);
> -		goto rpm_disable;
> +		goto unregister;
>  	}
>  
>  	return 0;
>  
> +unregister:
> +	host1x_client_unregister(&sor->client);
>  rpm_disable:
>  	pm_runtime_disable(&pdev->dev);
>  remove:
> 

Reviewed-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Tested-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Cheers
Jon

-- 
nvpublic