[PATCH v3 0/3] media: atmel-isi: rework on the clock part and add runtime pm support

[PATCH v3 0/3] media: atmel-isi: rework on the clock part and add runtime pm support

[Josh Wu]

This patch series fix the peripheral clock code and enable runtime pm
support.
Also it clean up the code which is for the compatiblity of mck.

Changes in v3:
- remove useless definition: ISI_DEFAULT_MCLK_FREQ

Changes in v2:
- this file is new added.
- merged v1 two patch into one.
- use runtime_pm_put() instead of runtime_pm_put_sync()
- enable peripheral clock before access ISI registers.
- totally remove clock_start()/clock_stop() as they are optional.

Josh Wu (3):
  media: atmel-isi: remove the useless code which disable isi
  media: atmel-isi: add runtime pm support
  media: atmel-isi: remove mck back compatiable code as it's not need

 drivers/media/platform/soc_camera/atmel-isi.c | 102 ++++++++++++--------------
 1 file changed, 45 insertions(+), 57 deletions(-)

-- 
1.9.1

[PATCH v3 1/3] media: atmel-isi: remove the useless code which disable isi

[Josh Wu]

To program ISI control register, the pixel clock should be enabled.
So without pixel clock (from sensor) enabled, disable ISI controller is
not make sense. So this patch remove those code.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---

Changes in v3: None
Changes in v2:
- this file is new added.

 drivers/media/platform/soc_camera/atmel-isi.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index c125b1d..31254b4 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -131,8 +131,6 @@ static int configure_geometry(struct atmel_isi *isi, u32 width,
 		return -EINVAL;
 	}
 
-	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-
 	cfg2 = isi_readl(isi, ISI_CFG2);
 	/* Set YCC swap mode */
 	cfg2 &= ~ISI_CFG2_YCC_SWAP_MODE_MASK;
@@ -843,7 +841,6 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 
 	cfg1 |= ISI_CFG1_THMASK_BEATS_16;
 
-	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
 	isi_writel(isi, ISI_CFG1, cfg1);
 
 	return 0;
@@ -1022,8 +1019,6 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	if (isi->pdata.data_width_flags & ISI_DATAWIDTH_10)
 		isi->width_flags |= 1 << 9;
 
-	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-
 	irq = platform_get_irq(pdev, 0);
 	if (IS_ERR_VALUE(irq)) {
 		ret = irq;
-- 
1.9.1

[PATCH v3 2/3] media: atmel-isi: add runtime pm support

[Josh Wu]

The runtime pm resume/suspend will enable/disable pclk (ISI peripheral
clock).
And we need to call runtime_pm_get_sync()/runtime_pm_put() when we need
access ISI registers.

In the meantime, as clock_start()/clock_stop() is used to control the
mclk not ISI peripheral clock. So move this to start[stop]_streaming()
function.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---

Changes in v3: None
Changes in v2:
- merged v1 two patch into one.
- use runtime_pm_put() instead of runtime_pm_put_sync()
- enable peripheral clock before access ISI registers.

 drivers/media/platform/soc_camera/atmel-isi.c | 51 +++++++++++++++++++++++----
 1 file changed, 45 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index 31254b4..2b05f89 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -20,6 +20,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 
 #include <media/atmel-isi.h>
@@ -384,6 +385,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 	struct atmel_isi *isi = ici->priv;
 	int ret;
 
+	pm_runtime_get_sync(ici->v4l2_dev.dev);
+
 	/* Reset ISI */
 	ret = atmel_isi_wait_status(isi, WAIT_ISI_RESET);
 	if (ret < 0) {
@@ -441,6 +444,8 @@ static void stop_streaming(struct vb2_queue *vq)
 	ret = atmel_isi_wait_status(isi, WAIT_ISI_DISABLE);
 	if (ret < 0)
 		dev_err(icd->parent, "Disable ISI timed out\n");
+
+	pm_runtime_put(ici->v4l2_dev.dev);
 }
 
 static struct vb2_ops isi_video_qops = {
@@ -509,7 +514,13 @@ static int isi_camera_set_fmt(struct soc_camera_device *icd,
 	if (mf.code != xlate->code)
 		return -EINVAL;
 
+	/* Enable PM and peripheral clock before operate isi registers */
+	pm_runtime_get_sync(ici->v4l2_dev.dev);
+
 	ret = configure_geometry(isi, pix->width, pix->height, xlate->code);
+
+	pm_runtime_put(ici->v4l2_dev.dev);
+
 	if (ret < 0)
 		return ret;
 
@@ -722,14 +733,9 @@ static int isi_camera_clock_start(struct soc_camera_host *ici)
 	struct atmel_isi *isi = ici->priv;
 	int ret;
 
-	ret = clk_prepare_enable(isi->pclk);
-	if (ret)
-		return ret;
-
 	if (!IS_ERR(isi->mck)) {
 		ret = clk_prepare_enable(isi->mck);
 		if (ret) {
-			clk_disable_unprepare(isi->pclk);
 			return ret;
 		}
 	}
@@ -744,7 +750,6 @@ static void isi_camera_clock_stop(struct soc_camera_host *ici)
 
 	if (!IS_ERR(isi->mck))
 		clk_disable_unprepare(isi->mck);
-	clk_disable_unprepare(isi->pclk);
 }
 
 static unsigned int isi_camera_poll(struct file *file, poll_table *pt)
@@ -841,8 +846,13 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 
 	cfg1 |= ISI_CFG1_THMASK_BEATS_16;
 
+	/* Enable PM and peripheral clock before operate isi registers */
+	pm_runtime_get_sync(ici->v4l2_dev.dev);
+
 	isi_writel(isi, ISI_CFG1, cfg1);
 
+	pm_runtime_put(ici->v4l2_dev.dev);
+
 	return 0;
 }
 
@@ -1039,6 +1049,9 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	soc_host->v4l2_dev.dev	= &pdev->dev;
 	soc_host->nr		= pdev->id;
 
+	pm_suspend_ignore_children(&pdev->dev, true);
+	pm_runtime_enable(&pdev->dev);
+
 	if (isi->pdata.asd_sizes) {
 		soc_host->asd = isi->pdata.asd;
 		soc_host->asd_sizes = isi->pdata.asd_sizes;
@@ -1052,6 +1065,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	return 0;
 
 err_register_soc_camera_host:
+	pm_runtime_disable(&pdev->dev);
 err_req_irq:
 err_ioremap:
 	vb2_dma_contig_cleanup_ctx(isi->alloc_ctx);
@@ -1064,6 +1078,30 @@ err_alloc_ctx:
 	return ret;
 }
 
+static int atmel_isi_runtime_suspend(struct device *dev)
+{
+	struct soc_camera_host *soc_host = to_soc_camera_host(dev);
+	struct atmel_isi *isi = container_of(soc_host,
+					struct atmel_isi, soc_host);
+
+	clk_disable_unprepare(isi->pclk);
+
+	return 0;
+}
+static int atmel_isi_runtime_resume(struct device *dev)
+{
+	struct soc_camera_host *soc_host = to_soc_camera_host(dev);
+	struct atmel_isi *isi = container_of(soc_host,
+					struct atmel_isi, soc_host);
+
+	return clk_prepare_enable(isi->pclk);
+}
+
+static const struct dev_pm_ops atmel_isi_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(atmel_isi_runtime_suspend,
+				atmel_isi_runtime_resume, NULL)
+};
+
 static const struct of_device_id atmel_isi_of_match[] = {
 	{ .compatible = "atmel,at91sam9g45-isi" },
 	{ }
@@ -1075,6 +1113,7 @@ static struct platform_driver atmel_isi_driver = {
 	.driver		= {
 		.name = "atmel_isi",
 		.of_match_table = of_match_ptr(atmel_isi_of_match),
+		.pm	= &atmel_isi_dev_pm_ops,
 	},
 };
 
-- 
1.9.1

[PATCH v3 3/3] media: atmel-isi: remove mck back compatiable code as it's not need

[Josh Wu]

The master clock should handled by sensor itself.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---

Changes in v3:
- remove useless definition: ISI_DEFAULT_MCLK_FREQ

Changes in v2:
- totally remove clock_start()/clock_stop() as they are optional.

 drivers/media/platform/soc_camera/atmel-isi.c | 46 ---------------------------
 1 file changed, 46 deletions(-)

diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index 2b05f89..d4989d1 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -35,7 +35,6 @@
 #define VID_LIMIT_BYTES			(16 * 1024 * 1024)
 #define MIN_FRAME_RATE			15
 #define FRAME_INTERVAL_MILLI_SEC	(1000 / MIN_FRAME_RATE)
-#define ISI_DEFAULT_MCLK_FREQ		25000000
 
 /* Frame buffer descriptor */
 struct fbd {
@@ -83,8 +82,6 @@ struct atmel_isi {
 	struct completion		complete;
 	/* ISI peripherial clock */
 	struct clk			*pclk;
-	/* ISI_MCK, feed to camera sensor to generate pixel clock */
-	struct clk			*mck;
 	unsigned int			irq;
 
 	struct isi_platform_data	pdata;
@@ -727,31 +724,6 @@ static void isi_camera_remove_device(struct soc_camera_device *icd)
 		 icd->devnum);
 }
 
-/* Called with .host_lock held */
-static int isi_camera_clock_start(struct soc_camera_host *ici)
-{
-	struct atmel_isi *isi = ici->priv;
-	int ret;
-
-	if (!IS_ERR(isi->mck)) {
-		ret = clk_prepare_enable(isi->mck);
-		if (ret) {
-			return ret;
-		}
-	}
-
-	return 0;
-}
-
-/* Called with .host_lock held */
-static void isi_camera_clock_stop(struct soc_camera_host *ici)
-{
-	struct atmel_isi *isi = ici->priv;
-
-	if (!IS_ERR(isi->mck))
-		clk_disable_unprepare(isi->mck);
-}
-
 static unsigned int isi_camera_poll(struct file *file, poll_table *pt)
 {
 	struct soc_camera_device *icd = file->private_data;
@@ -865,8 +837,6 @@ static struct soc_camera_host_ops isi_soc_camera_host_ops = {
 	.owner		= THIS_MODULE,
 	.add		= isi_camera_add_device,
 	.remove		= isi_camera_remove_device,
-	.clock_start	= isi_camera_clock_start,
-	.clock_stop	= isi_camera_clock_stop,
 	.set_fmt	= isi_camera_set_fmt,
 	.try_fmt	= isi_camera_try_fmt,
 	.get_formats	= isi_camera_get_formats,
@@ -904,7 +874,6 @@ static int atmel_isi_probe_dt(struct atmel_isi *isi,
 
 	/* Default settings for ISI */
 	isi->pdata.full_mode = 1;
-	isi->pdata.mck_hz = ISI_DEFAULT_MCLK_FREQ;
 	isi->pdata.frate = ISI_CFG1_FRATE_CAPTURE_ALL;
 
 	np = of_graph_get_next_endpoint(np, NULL);
@@ -980,21 +949,6 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&isi->video_buffer_list);
 	INIT_LIST_HEAD(&isi->dma_desc_head);
 
-	/* ISI_MCK is the sensor master clock. It should be handled by the
-	 * sensor driver directly, as the ISI has no use for that clock. Make
-	 * the clock optional here while platforms transition to the correct
-	 * model.
-	 */
-	isi->mck = devm_clk_get(dev, "isi_mck");
-	if (!IS_ERR(isi->mck)) {
-		/* Set ISI_MCK's frequency, it should be faster than pixel
-		 * clock.
-		 */
-		ret = clk_set_rate(isi->mck, isi->pdata.mck_hz);
-		if (ret < 0)
-			return ret;
-	}
-
 	isi->p_fb_descriptors = dma_alloc_coherent(&pdev->dev,
 				sizeof(struct fbd) * MAX_BUFFER_NUM,
 				&isi->fb_descriptors_phys,
-- 
1.9.1

Re: [PATCH v3 2/3] media: atmel-isi: add runtime pm support

[Josh Wu]

On 5/6/2015 6:25 PM, Josh Wu wrote:
> The runtime pm resume/suspend will enable/disable pclk (ISI peripheral
> clock).
> And we need to call runtime_pm_get_sync()/runtime_pm_put() when we need
> access ISI registers.
>
> In the meantime, as clock_start()/clock_stop() is used to control the
> mclk not ISI peripheral clock. So move this to start[stop]_streaming()
> function.
>
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> ---
>
> Changes in v3: None
> Changes in v2:
> - merged v1 two patch into one.
> - use runtime_pm_put() instead of runtime_pm_put_sync()
> - enable peripheral clock before access ISI registers.
>
>   drivers/media/platform/soc_camera/atmel-isi.c | 51 +++++++++++++++++++++++----
>   1 file changed, 45 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
> index 31254b4..2b05f89 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> @@ -20,6 +20,7 @@
>   #include <linux/kernel.h>
>   #include <linux/module.h>
>   #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>   #include <linux/slab.h>
>   
>   #include <media/atmel-isi.h>
> @@ -384,6 +385,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
>   	struct atmel_isi *isi = ici->priv;
>   	int ret;
>   
> +	pm_runtime_get_sync(ici->v4l2_dev.dev);
> +
>   	/* Reset ISI */
>   	ret = atmel_isi_wait_status(isi, WAIT_ISI_RESET);
>   	if (ret < 0) {
> @@ -441,6 +444,8 @@ static void stop_streaming(struct vb2_queue *vq)
>   	ret = atmel_isi_wait_status(isi, WAIT_ISI_DISABLE);
>   	if (ret < 0)
>   		dev_err(icd->parent, "Disable ISI timed out\n");
> +
> +	pm_runtime_put(ici->v4l2_dev.dev);
>   }
>   
>   static struct vb2_ops isi_video_qops = {
> @@ -509,7 +514,13 @@ static int isi_camera_set_fmt(struct soc_camera_device *icd,
>   	if (mf.code != xlate->code)
>   		return -EINVAL;
>   
> +	/* Enable PM and peripheral clock before operate isi registers */
> +	pm_runtime_get_sync(ici->v4l2_dev.dev);
> +
>   	ret = configure_geometry(isi, pix->width, pix->height, xlate->code);
> +
> +	pm_runtime_put(ici->v4l2_dev.dev);
> +
>   	if (ret < 0)
>   		return ret;
>   
> @@ -722,14 +733,9 @@ static int isi_camera_clock_start(struct soc_camera_host *ici)
>   	struct atmel_isi *isi = ici->priv;
>   	int ret;
>   
> -	ret = clk_prepare_enable(isi->pclk);
> -	if (ret)
> -		return ret;
> -
>   	if (!IS_ERR(isi->mck)) {
>   		ret = clk_prepare_enable(isi->mck);
>   		if (ret) {
> -			clk_disable_unprepare(isi->pclk);
>   			return ret;
>   		}
>   	}
> @@ -744,7 +750,6 @@ static void isi_camera_clock_stop(struct soc_camera_host *ici)
>   
>   	if (!IS_ERR(isi->mck))
>   		clk_disable_unprepare(isi->mck);
> -	clk_disable_unprepare(isi->pclk);
>   }
>   
>   static unsigned int isi_camera_poll(struct file *file, poll_table *pt)
> @@ -841,8 +846,13 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
>   
>   	cfg1 |= ISI_CFG1_THMASK_BEATS_16;
>   
> +	/* Enable PM and peripheral clock before operate isi registers */
> +	pm_runtime_get_sync(ici->v4l2_dev.dev);
> +
>   	isi_writel(isi, ISI_CFG1, cfg1);
>   
> +	pm_runtime_put(ici->v4l2_dev.dev);
> +
>   	return 0;
>   }
>   
> @@ -1039,6 +1049,9 @@ static int atmel_isi_probe(struct platform_device *pdev)
>   	soc_host->v4l2_dev.dev	= &pdev->dev;
>   	soc_host->nr		= pdev->id;
>   
> +	pm_suspend_ignore_children(&pdev->dev, true);
> +	pm_runtime_enable(&pdev->dev);
> +
>   	if (isi->pdata.asd_sizes) {
>   		soc_host->asd = isi->pdata.asd;
>   		soc_host->asd_sizes = isi->pdata.asd_sizes;
> @@ -1052,6 +1065,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
>   	return 0;
>   
>   err_register_soc_camera_host:
> +	pm_runtime_disable(&pdev->dev);

This function is missed in atmel_isi_remove(). I'll fix it.

Best Regards,
Josh Wu
>   err_req_irq:
>   err_ioremap:
>   	vb2_dma_contig_cleanup_ctx(isi->alloc_ctx);
> @@ -1064,6 +1078,30 @@ err_alloc_ctx:
>   	return ret;
>   }
>   
> +static int atmel_isi_runtime_suspend(struct device *dev)
> +{
> +	struct soc_camera_host *soc_host = to_soc_camera_host(dev);
> +	struct atmel_isi *isi = container_of(soc_host,
> +					struct atmel_isi, soc_host);
> +
> +	clk_disable_unprepare(isi->pclk);
> +
> +	return 0;
> +}
> +static int atmel_isi_runtime_resume(struct device *dev)
> +{
> +	struct soc_camera_host *soc_host = to_soc_camera_host(dev);
> +	struct atmel_isi *isi = container_of(soc_host,
> +					struct atmel_isi, soc_host);
> +
> +	return clk_prepare_enable(isi->pclk);
> +}
> +
> +static const struct dev_pm_ops atmel_isi_dev_pm_ops = {
> +	SET_RUNTIME_PM_OPS(atmel_isi_runtime_suspend,
> +				atmel_isi_runtime_resume, NULL)
> +};
> +
>   static const struct of_device_id atmel_isi_of_match[] = {
>   	{ .compatible = "atmel,at91sam9g45-isi" },
>   	{ }
> @@ -1075,6 +1113,7 @@ static struct platform_driver atmel_isi_driver = {
>   	.driver		= {
>   		.name = "atmel_isi",
>   		.of_match_table = of_match_ptr(atmel_isi_of_match),
> +		.pm	= &atmel_isi_dev_pm_ops,
>   	},
>   };
>