[Letux-kernel] [PATCH pre-upstream RFC] media: omap3isp: Fix high idle current

H. Nikolaus Schaller hns at goldelico.com
Sat Jan 5 11:33:23 CET 2019


Hi Andreas,

> Am 05.01.2019 um 10:17 schrieb Andreas Kemnade <andreas at kemnade.info>:
> 
> On the GTA04, current consumption rose by about 30mA when the omap3_isp
> module was loaded and the v4l device was not accessed and even no
> camera attached.
> Module removal fixed it again. Slowing down the removal process reveals
> that nothing that calling isp_detach_iommu() is required to have low
> current. So isp_attach/detach_iommu() to moved to the get()/put()
> functions.

To me there are one or two words missing to make the later sentences understandable.

Maybe deepl.com en -> de and back to en?

Beim GTA04 stieg die Stromaufnahme um ca. 30mA, wenn das omap3_isp-Modul geladen wurde und auf das v4l-Gerät nicht zugegriffen und auch keine Kamera angeschlossen wurde.
Die Entfernung des Moduls hat es wieder behoben. Die Verlangsamung des Entfernungsprozesses zeigt, dass nichts, was den Aufruf von isp_detach_iommu() erfordert, um einen niedrigen Strom zu haben. Also isp_attach/detach_iommu(), um zu den Funktionen get()/put() zu wechseln.

Übersetzt mit www.DeepL.com/Translator

and back to -> en

With the GTA04 the current consumption increased by about 30mA if the omap3_isp module was loaded and the v4l device was not accessed and no camera was connected.
The removal of the module fixed it again. The slowing down of the removal process shows that nothing that requires isp_detach_iommu() to be called to have a low current. So isp_attach/detach_iommu() to switch to the get()/put() functions.

Translated with www.DeepL.com/Translator

Impressively good how it even works with () etc.

> 
> Signed-off-by: Andreas Kemnade <andreas at kemnade.info>
> ---
> As I am not completely sure about current state of our camera stuff,
> I am hesitating a bit here. I am doing now research of the actual state
> of the camera in the letux kernel. Last time I tried, it was not soo stable.
> But at least it fixes a problem in devices without camera.

Yes, that is fine. I'll take it for letux-4.21-rc1. Should I also add to
4.20.1?

The status is that the camera driver seems to work - but only once.
I was able to get images through mplayer but when stopping and restarting
I only got select timeouts and a green screen.

So something is bad either in the camera module driver close() or in
some isp function.

> 
> drivers/media/platform/omap3isp/isp.c | 28 +++++++++++++++-------------
> 1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
> index 13f2828d880d..b837ca5604ad 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -88,6 +88,10 @@ static void isp_save_ctx(struct isp_device *isp);
> 
> static void isp_restore_ctx(struct isp_device *isp);
> 
> +static int isp_attach_iommu(struct isp_device *isp);
> +
> +static void isp_detach_iommu(struct isp_device *isp);
> +
> static const struct isp_res_mapping isp_res_maps[] = {
> 	{
> 		.isp_rev = ISP_REVISION_2_0,
> @@ -1407,6 +1411,14 @@ static struct isp_device *__omap3isp_get(struct isp_device *isp, bool irq)
> 		__isp = NULL;
> 		goto out;
> 	}
> +	/* IOMMU */
> +	if (isp_attach_iommu(isp) < 0) {
> +		dev_err(isp->dev, "unable to attach to IOMMU\n");
> +		isp_disable_clocks(isp);
> +		__isp = NULL;
> +		goto out;
> +	}
> +
> 
> 	/* We don't want to restore context before saving it! */
> 	if (isp->has_context)
> @@ -1453,6 +1465,7 @@ static void __omap3isp_put(struct isp_device *isp, bool save_ctx)
> 		if (!media_entity_enum_empty(&isp->crashed) ||
> 		    isp->stop_failure)
> 			isp_reset(isp);
> +		isp_detach_iommu(isp);
> 		isp_disable_clocks(isp);
> 	}
> 	mutex_unlock(&isp->isp_mutex);
> @@ -1999,10 +2012,6 @@ static int isp_remove(struct platform_device *pdev)
> 	isp_cleanup_modules(isp);
> 	isp_xclk_cleanup(isp);
> 
> -	__omap3isp_get(isp, false);
> -	isp_detach_iommu(isp);
> -	__omap3isp_put(isp, false);
> -
> 	media_entity_enum_cleanup(&isp->crashed);
> 	v4l2_async_notifier_cleanup(&isp->notifier);
> 
> @@ -2313,19 +2322,12 @@ static int isp_probe(struct platform_device *pdev)
> 	isp->mmio_hist_base_phys =
> 		mem->start + isp_res_maps[m].offset[OMAP3_ISP_IOMEM_HIST];
> 
> -	/* IOMMU */
> -	ret = isp_attach_iommu(isp);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "unable to attach to IOMMU\n");
> -		goto error_isp;
> -	}
> -
> 	/* Interrupt */
> 	ret = platform_get_irq(pdev, 0);
> 	if (ret <= 0) {
> 		dev_err(isp->dev, "No IRQ resource\n");
> 		ret = -ENODEV;
> -		goto error_iommu;
> +		goto error_isp;
> 	}
> 	isp->irq_num = ret;
> 
> @@ -2339,7 +2341,7 @@ static int isp_probe(struct platform_device *pdev)
> 	/* Entities */
> 	ret = isp_initialize_modules(isp);
> 	if (ret < 0)
> -		goto error_iommu;
> +		goto error_isp;
> 
> 	ret = isp_register_entities(isp);
> 	if (ret < 0)
> -- 
> 2.11.0
> 

BR and thanks,
Nikolaus



More information about the Letux-kernel mailing list