drm/exynos: mixer: Fix enabling of the runtime power management

Message ID 20200514100812.17043-1-m.szyprowski@samsung.com
State Accepted
Commit fda022143f6f00fc4c3c296175b5e315c7c12710
Headers show
Series
  • drm/exynos: mixer: Fix enabling of the runtime power management
Related show

Commit Message

Marek Szyprowski May 14, 2020, 10:08 a.m.
Runtime power management is essential for the Exynos Mixer driver
operation. It should be enabled before adding its DRM component, because
in some cases (when deferred probe takes place due to the IOMMU
availability) the DRM driver might be initialized directly from the
Mixer's component_add() call, what results in starting the driver
operation without enabling the runtime power management.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

---
 drivers/gpu/drm/exynos/exynos_mixer.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Comments

Inki Dae May 18, 2020, 5:12 a.m. | #1
Hi Marek,

20. 5. 14. 오후 7:08에 Marek Szyprowski 이(가) 쓴 글:
> Runtime power management is essential for the Exynos Mixer driver
> operation. It should be enabled before adding its DRM component, because
> in some cases (when deferred probe takes place due to the IOMMU
> availability) the DRM driver might be initialized directly from the
> Mixer's component_add() call, what results in starting the driver
> operation without enabling the runtime power management.

Seems better to change call order of mixer_remove function like below because you changed the one of probe function.
static int mixer_remove(struct platform_device *pdev)
{
	component_del(&pdev->dev, &mixer_component_ops);

	pm_runtime_disable(&pdev->dev);
}

It's a trivial thing and it would be no problem as-is - we don't touch HW in unbind - so picked it up.

Thanks,
Inki Dae

> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index ffbf4a950f69..829d2ce7560d 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -1200,9 +1200,11 @@ static int mixer_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, ctx);
>  
> +	pm_runtime_enable(dev);
> +
>  	ret = component_add(&pdev->dev, &mixer_component_ops);
> -	if (!ret)
> -		pm_runtime_enable(dev);
> +	if (ret)
> +		pm_runtime_disable(dev);
>  
>  	return ret;
>  }
>

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index ffbf4a950f69..829d2ce7560d 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1200,9 +1200,11 @@  static int mixer_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, ctx);
 
+	pm_runtime_enable(dev);
+
 	ret = component_add(&pdev->dev, &mixer_component_ops);
-	if (!ret)
-		pm_runtime_enable(dev);
+	if (ret)
+		pm_runtime_disable(dev);
 
 	return ret;
 }