linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] media: staging: rkisp1: document rkisp1-common.h
@ 2020-08-06 10:34 Dafna Hirschfeld
  2020-08-06 10:34 ` [PATCH v3 1/5] media: staging: rkisp1: don't define vaddr field in rkisp1_buffer as an array Dafna Hirschfeld
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Dafna Hirschfeld @ 2020-08-06 10:34 UTC (permalink / raw)
  To: linux-media, laurent.pinchart
  Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel,
	dafna3, sakari.ailus, mchehab, tfiga

This patchset extends the documentation of rkisp1-common.h, together with some cleanup patches.
It adds a line description of every struct and every field, and also
a line description of every lock.

change since v2:

first 3 patches were already accepted and are not in this version.

patch 5 from v2 was removed

in patch 6 - add document field 'yuv_seq' in rkisp1_isp_mbus_info

Dafna Hirschfeld (5):
  media: staging: rkisp1: don't define vaddr field in rkisp1_buffer as
    an array
  media: staging: rkisp1: unify (un)register functions to have the same
    parameters
  media: staging: rkisp1: remove declaration of unimplemented function
    'rkisp1_params_isr_handler'
  media: staging: rkisp1: group declaration of similar functions
    together
  media: staging: rkisp1: improve documentation of rkisp1-common.h

 drivers/staging/media/rkisp1/TODO            |   1 -
 drivers/staging/media/rkisp1/rkisp1-common.h | 265 ++++++++++++++-----
 drivers/staging/media/rkisp1/rkisp1-dev.c    |  15 +-
 drivers/staging/media/rkisp1/rkisp1-isp.c    |   5 +-
 drivers/staging/media/rkisp1/rkisp1-params.c |  14 +-
 drivers/staging/media/rkisp1/rkisp1-stats.c  |  14 +-
 6 files changed, 226 insertions(+), 88 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/5] media: staging: rkisp1: don't define vaddr field in rkisp1_buffer as an array
  2020-08-06 10:34 [PATCH v3 0/5] media: staging: rkisp1: document rkisp1-common.h Dafna Hirschfeld
@ 2020-08-06 10:34 ` Dafna Hirschfeld
  2020-08-06 10:34 ` [PATCH v3 2/5] media: staging: rkisp1: unify (un)register functions to have the same parameters Dafna Hirschfeld
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Dafna Hirschfeld @ 2020-08-06 10:34 UTC (permalink / raw)
  To: linux-media, laurent.pinchart
  Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel,
	dafna3, sakari.ailus, mchehab, tfiga

The field vaddr in rkisp1_buffer struct is used only by the
rkisp1-stats and rkisp1-params entities and they both use only
vaddr[0] so there is no need to define this field as an array.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
---
 drivers/staging/media/rkisp1/rkisp1-common.h | 2 +-
 drivers/staging/media/rkisp1/rkisp1-params.c | 4 ++--
 drivers/staging/media/rkisp1/rkisp1-stats.c  | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
index 3dc51d703f73..e54793d13c3d 100644
--- a/drivers/staging/media/rkisp1/rkisp1-common.h
+++ b/drivers/staging/media/rkisp1/rkisp1-common.h
@@ -127,7 +127,7 @@ struct rkisp1_buffer {
 	struct list_head queue;
 	union {
 		u32 buff_addr[VIDEO_MAX_PLANES];
-		void *vaddr[VIDEO_MAX_PLANES];
+		void *vaddr;
 	};
 };
 
diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
index 797e79de659c..2ab25062cde6 100644
--- a/drivers/staging/media/rkisp1/rkisp1-params.c
+++ b/drivers/staging/media/rkisp1/rkisp1-params.c
@@ -1215,7 +1215,7 @@ void rkisp1_params_isr(struct rkisp1_device *rkisp1, u32 isp_mis)
 	if (!cur_buf)
 		return;
 
-	new_params = (struct rkisp1_params_cfg *)(cur_buf->vaddr[0]);
+	new_params = (struct rkisp1_params_cfg *)(cur_buf->vaddr);
 
 	if (isp_mis & RKISP1_CIF_ISP_FRAME) {
 		u32 isp_ctrl;
@@ -1463,7 +1463,7 @@ static void rkisp1_params_vb2_buf_queue(struct vb2_buffer *vb)
 		return;
 	}
 
-	params_buf->vaddr[0] = vb2_plane_vaddr(vb, 0);
+	params_buf->vaddr = vb2_plane_vaddr(vb, 0);
 	spin_lock_irqsave(&params->config_lock, flags);
 	list_add_tail(&params_buf->queue, &params->params);
 	spin_unlock_irqrestore(&params->config_lock, flags);
diff --git a/drivers/staging/media/rkisp1/rkisp1-stats.c b/drivers/staging/media/rkisp1/rkisp1-stats.c
index 87e4104d20dd..a67c233b8641 100644
--- a/drivers/staging/media/rkisp1/rkisp1-stats.c
+++ b/drivers/staging/media/rkisp1/rkisp1-stats.c
@@ -116,7 +116,7 @@ static void rkisp1_stats_vb2_buf_queue(struct vb2_buffer *vb)
 	struct vb2_queue *vq = vb->vb2_queue;
 	struct rkisp1_stats *stats_dev = vq->drv_priv;
 
-	stats_buf->vaddr[0] = vb2_plane_vaddr(vb, 0);
+	stats_buf->vaddr = vb2_plane_vaddr(vb, 0);
 
 	spin_lock_irq(&stats_dev->lock);
 	list_add_tail(&stats_buf->queue, &stats_dev->stat);
@@ -322,7 +322,7 @@ rkisp1_stats_send_measurement(struct rkisp1_stats *stats, u32 isp_ris)
 		return;
 
 	cur_stat_buf =
-		(struct rkisp1_stat_buffer *)(cur_buf->vaddr[0]);
+		(struct rkisp1_stat_buffer *)(cur_buf->vaddr);
 
 	if (isp_ris & RKISP1_CIF_ISP_AWB_DONE)
 		rkisp1_stats_get_awb_meas(stats, cur_stat_buf);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/5] media: staging: rkisp1: unify (un)register functions to have the same parameters
  2020-08-06 10:34 [PATCH v3 0/5] media: staging: rkisp1: document rkisp1-common.h Dafna Hirschfeld
  2020-08-06 10:34 ` [PATCH v3 1/5] media: staging: rkisp1: don't define vaddr field in rkisp1_buffer as an array Dafna Hirschfeld
@ 2020-08-06 10:34 ` Dafna Hirschfeld
  2020-08-06 10:34 ` [PATCH v3 3/5] media: staging: rkisp1: remove declaration of unimplemented function 'rkisp1_params_isr_handler' Dafna Hirschfeld
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Dafna Hirschfeld @ 2020-08-06 10:34 UTC (permalink / raw)
  To: linux-media, laurent.pinchart
  Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel,
	dafna3, sakari.ailus, mchehab, tfiga

The different register/unregister functions receive
different parameters. This patch unify them so they all receive just
'struct *rkisp1_device' as parameter.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
---
 drivers/staging/media/rkisp1/rkisp1-common.h | 15 +++++----------
 drivers/staging/media/rkisp1/rkisp1-dev.c    | 15 +++++++--------
 drivers/staging/media/rkisp1/rkisp1-isp.c    |  5 ++---
 drivers/staging/media/rkisp1/rkisp1-params.c | 10 +++++-----
 drivers/staging/media/rkisp1/rkisp1-stats.c  | 10 +++++-----
 5 files changed, 24 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
index e54793d13c3d..7d414f89d79b 100644
--- a/drivers/staging/media/rkisp1/rkisp1-common.h
+++ b/drivers/staging/media/rkisp1/rkisp1-common.h
@@ -303,8 +303,7 @@ void rkisp1_sd_adjust_crop_rect(struct v4l2_rect *crop,
 void rkisp1_sd_adjust_crop(struct v4l2_rect *crop,
 			   const struct v4l2_mbus_framefmt *bounds);
 
-int rkisp1_isp_register(struct rkisp1_device *rkisp1,
-			struct v4l2_device *v4l2_dev);
+int rkisp1_isp_register(struct rkisp1_device *rkisp1);
 void rkisp1_isp_unregister(struct rkisp1_device *rkisp1);
 
 const struct rkisp1_isp_mbus_info *rkisp1_isp_mbus_info_get(u32 mbus_code);
@@ -321,19 +320,15 @@ void rkisp1_capture_devs_unregister(struct rkisp1_device *rkisp1);
 int rkisp1_resizer_devs_register(struct rkisp1_device *rkisp1);
 void rkisp1_resizer_devs_unregister(struct rkisp1_device *rkisp1);
 
-int rkisp1_stats_register(struct rkisp1_stats *stats,
-			  struct v4l2_device *v4l2_dev,
-			  struct rkisp1_device *rkisp1);
-void rkisp1_stats_unregister(struct rkisp1_stats *stats);
+int rkisp1_stats_register(struct rkisp1_device *rkisp1);
+void rkisp1_stats_unregister(struct rkisp1_device *rkisp1);
 
 void rkisp1_params_configure(struct rkisp1_params *params,
 			     enum rkisp1_fmt_raw_pat_type bayer_pat,
 			     enum v4l2_quantization quantization);
 void rkisp1_params_disable(struct rkisp1_params *params);
-int rkisp1_params_register(struct rkisp1_params *params,
-			   struct v4l2_device *v4l2_dev,
-			   struct rkisp1_device *rkisp1);
-void rkisp1_params_unregister(struct rkisp1_params *params);
+int rkisp1_params_register(struct rkisp1_device *rkisp1);
+void rkisp1_params_unregister(struct rkisp1_device *rkisp1);
 
 void rkisp1_params_isr_handler(struct rkisp1_device *rkisp1, u32 isp_mis);
 
diff --git a/drivers/staging/media/rkisp1/rkisp1-dev.c b/drivers/staging/media/rkisp1/rkisp1-dev.c
index a0eb8f08708b..d85ac10e5494 100644
--- a/drivers/staging/media/rkisp1/rkisp1-dev.c
+++ b/drivers/staging/media/rkisp1/rkisp1-dev.c
@@ -345,7 +345,7 @@ static int rkisp1_entities_register(struct rkisp1_device *rkisp1)
 {
 	int ret;
 
-	ret = rkisp1_isp_register(rkisp1, &rkisp1->v4l2_dev);
+	ret = rkisp1_isp_register(rkisp1);
 	if (ret)
 		return ret;
 
@@ -357,12 +357,11 @@ static int rkisp1_entities_register(struct rkisp1_device *rkisp1)
 	if (ret)
 		goto err_unreg_resizer_devs;
 
-	ret = rkisp1_stats_register(&rkisp1->stats, &rkisp1->v4l2_dev, rkisp1);
+	ret = rkisp1_stats_register(rkisp1);
 	if (ret)
 		goto err_unreg_capture_devs;
 
-	ret = rkisp1_params_register(&rkisp1->params,
-				     &rkisp1->v4l2_dev, rkisp1);
+	ret = rkisp1_params_register(rkisp1);
 	if (ret)
 		goto err_unreg_stats;
 
@@ -375,9 +374,9 @@ static int rkisp1_entities_register(struct rkisp1_device *rkisp1)
 
 	return 0;
 err_unreg_params:
-	rkisp1_params_unregister(&rkisp1->params);
+	rkisp1_params_unregister(rkisp1);
 err_unreg_stats:
-	rkisp1_stats_unregister(&rkisp1->stats);
+	rkisp1_stats_unregister(rkisp1);
 err_unreg_capture_devs:
 	rkisp1_capture_devs_unregister(rkisp1);
 err_unreg_resizer_devs:
@@ -551,8 +550,8 @@ static int rkisp1_remove(struct platform_device *pdev)
 	v4l2_async_notifier_unregister(&rkisp1->notifier);
 	v4l2_async_notifier_cleanup(&rkisp1->notifier);
 
-	rkisp1_params_unregister(&rkisp1->params);
-	rkisp1_stats_unregister(&rkisp1->stats);
+	rkisp1_params_unregister(rkisp1);
+	rkisp1_stats_unregister(rkisp1);
 	rkisp1_capture_devs_unregister(rkisp1);
 	rkisp1_resizer_devs_unregister(rkisp1);
 	rkisp1_isp_unregister(rkisp1);
diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
index 6ec1e9816e9f..7b40f53fde9a 100644
--- a/drivers/staging/media/rkisp1/rkisp1-isp.c
+++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
@@ -989,8 +989,7 @@ static const struct v4l2_subdev_ops rkisp1_isp_ops = {
 	.pad = &rkisp1_isp_pad_ops,
 };
 
-int rkisp1_isp_register(struct rkisp1_device *rkisp1,
-			struct v4l2_device *v4l2_dev)
+int rkisp1_isp_register(struct rkisp1_device *rkisp1)
 {
 	struct rkisp1_isp *isp = &rkisp1->isp;
 	struct media_pad *pads = isp->pads;
@@ -1018,7 +1017,7 @@ int rkisp1_isp_register(struct rkisp1_device *rkisp1,
 	if (ret)
 		return ret;
 
-	ret = v4l2_device_register_subdev(v4l2_dev, sd);
+	ret = v4l2_device_register_subdev(&rkisp1->v4l2_dev, sd);
 	if (ret) {
 		dev_err(rkisp1->dev, "Failed to register isp subdev\n");
 		goto err_cleanup_media_entity;
diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
index 2ab25062cde6..c3c0ab5a5713 100644
--- a/drivers/staging/media/rkisp1/rkisp1-params.c
+++ b/drivers/staging/media/rkisp1/rkisp1-params.c
@@ -1570,10 +1570,9 @@ static void rkisp1_init_params(struct rkisp1_params *params)
 		sizeof(struct rkisp1_params_cfg);
 }
 
-int rkisp1_params_register(struct rkisp1_params *params,
-			   struct v4l2_device *v4l2_dev,
-			   struct rkisp1_device *rkisp1)
+int rkisp1_params_register(struct rkisp1_device *rkisp1)
 {
+	struct rkisp1_params *params = &rkisp1->params;
 	struct rkisp1_vdev_node *node = &params->vnode;
 	struct video_device *vdev = &node->vdev;
 	int ret;
@@ -1593,7 +1592,7 @@ int rkisp1_params_register(struct rkisp1_params *params,
 	 * to protect all fops and v4l2 ioctls.
 	 */
 	vdev->lock = &node->vlock;
-	vdev->v4l2_dev = v4l2_dev;
+	vdev->v4l2_dev = &rkisp1->v4l2_dev;
 	vdev->queue = &node->buf_queue;
 	vdev->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_META_OUTPUT;
 	vdev->vfl_dir = VFL_DIR_TX;
@@ -1619,8 +1618,9 @@ int rkisp1_params_register(struct rkisp1_params *params,
 	return ret;
 }
 
-void rkisp1_params_unregister(struct rkisp1_params *params)
+void rkisp1_params_unregister(struct rkisp1_device *rkisp1)
 {
+	struct rkisp1_params *params = &rkisp1->params;
 	struct rkisp1_vdev_node *node = &params->vnode;
 	struct video_device *vdev = &node->vdev;
 
diff --git a/drivers/staging/media/rkisp1/rkisp1-stats.c b/drivers/staging/media/rkisp1/rkisp1-stats.c
index a67c233b8641..f5dbd134ee24 100644
--- a/drivers/staging/media/rkisp1/rkisp1-stats.c
+++ b/drivers/staging/media/rkisp1/rkisp1-stats.c
@@ -375,10 +375,9 @@ static void rkisp1_init_stats(struct rkisp1_stats *stats)
 		sizeof(struct rkisp1_stat_buffer);
 }
 
-int rkisp1_stats_register(struct rkisp1_stats *stats,
-			  struct v4l2_device *v4l2_dev,
-			  struct rkisp1_device *rkisp1)
+int rkisp1_stats_register(struct rkisp1_device *rkisp1)
 {
+	struct rkisp1_stats *stats = &rkisp1->stats;
 	struct rkisp1_vdev_node *node = &stats->vnode;
 	struct video_device *vdev = &node->vdev;
 	int ret;
@@ -395,7 +394,7 @@ int rkisp1_stats_register(struct rkisp1_stats *stats,
 	vdev->fops = &rkisp1_stats_fops;
 	vdev->release = video_device_release_empty;
 	vdev->lock = &node->vlock;
-	vdev->v4l2_dev = v4l2_dev;
+	vdev->v4l2_dev = &rkisp1->v4l2_dev;
 	vdev->queue = &node->buf_queue;
 	vdev->device_caps = V4L2_CAP_META_CAPTURE | V4L2_CAP_STREAMING;
 	vdev->vfl_dir =  VFL_DIR_RX;
@@ -425,8 +424,9 @@ int rkisp1_stats_register(struct rkisp1_stats *stats,
 	return ret;
 }
 
-void rkisp1_stats_unregister(struct rkisp1_stats *stats)
+void rkisp1_stats_unregister(struct rkisp1_device *rkisp1)
 {
+	struct rkisp1_stats *stats = &rkisp1->stats;
 	struct rkisp1_vdev_node *node = &stats->vnode;
 	struct video_device *vdev = &node->vdev;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 3/5] media: staging: rkisp1: remove declaration of unimplemented function 'rkisp1_params_isr_handler'
  2020-08-06 10:34 [PATCH v3 0/5] media: staging: rkisp1: document rkisp1-common.h Dafna Hirschfeld
  2020-08-06 10:34 ` [PATCH v3 1/5] media: staging: rkisp1: don't define vaddr field in rkisp1_buffer as an array Dafna Hirschfeld
  2020-08-06 10:34 ` [PATCH v3 2/5] media: staging: rkisp1: unify (un)register functions to have the same parameters Dafna Hirschfeld
@ 2020-08-06 10:34 ` Dafna Hirschfeld
  2020-08-06 10:34 ` [PATCH v3 4/5] media: staging: rkisp1: group declaration of similar functions together Dafna Hirschfeld
  2020-08-06 10:34 ` [PATCH v3 5/5] media: staging: rkisp1: improve documentation of rkisp1-common.h Dafna Hirschfeld
  4 siblings, 0 replies; 6+ messages in thread
From: Dafna Hirschfeld @ 2020-08-06 10:34 UTC (permalink / raw)
  To: linux-media, laurent.pinchart
  Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel,
	dafna3, sakari.ailus, mchehab, tfiga

The file rkisp1-common.h has a function declarion
'rkisp1_params_isr_handler' that is not implemented.
Remove it.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
---
 drivers/staging/media/rkisp1/rkisp1-common.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
index 7d414f89d79b..6b8fb9909369 100644
--- a/drivers/staging/media/rkisp1/rkisp1-common.h
+++ b/drivers/staging/media/rkisp1/rkisp1-common.h
@@ -330,6 +330,4 @@ void rkisp1_params_disable(struct rkisp1_params *params);
 int rkisp1_params_register(struct rkisp1_device *rkisp1);
 void rkisp1_params_unregister(struct rkisp1_device *rkisp1);
 
-void rkisp1_params_isr_handler(struct rkisp1_device *rkisp1, u32 isp_mis);
-
 #endif /* _RKISP1_COMMON_H */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 4/5] media: staging: rkisp1: group declaration of similar functions together
  2020-08-06 10:34 [PATCH v3 0/5] media: staging: rkisp1: document rkisp1-common.h Dafna Hirschfeld
                   ` (2 preceding siblings ...)
  2020-08-06 10:34 ` [PATCH v3 3/5] media: staging: rkisp1: remove declaration of unimplemented function 'rkisp1_params_isr_handler' Dafna Hirschfeld
@ 2020-08-06 10:34 ` Dafna Hirschfeld
  2020-08-06 10:34 ` [PATCH v3 5/5] media: staging: rkisp1: improve documentation of rkisp1-common.h Dafna Hirschfeld
  4 siblings, 0 replies; 6+ messages in thread
From: Dafna Hirschfeld @ 2020-08-06 10:34 UTC (permalink / raw)
  To: linux-media, laurent.pinchart
  Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel,
	dafna3, sakari.ailus, mchehab, tfiga

In file rkisp1-common.h, group declaration of register/unregister
functions together and group other functions together to make
the code easier to read.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
---
 drivers/staging/media/rkisp1/rkisp1-common.h | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
index 6b8fb9909369..44acebcc7881 100644
--- a/drivers/staging/media/rkisp1/rkisp1-common.h
+++ b/drivers/staging/media/rkisp1/rkisp1-common.h
@@ -303,11 +303,13 @@ void rkisp1_sd_adjust_crop_rect(struct v4l2_rect *crop,
 void rkisp1_sd_adjust_crop(struct v4l2_rect *crop,
 			   const struct v4l2_mbus_framefmt *bounds);
 
-int rkisp1_isp_register(struct rkisp1_device *rkisp1);
-void rkisp1_isp_unregister(struct rkisp1_device *rkisp1);
-
 const struct rkisp1_isp_mbus_info *rkisp1_isp_mbus_info_get(u32 mbus_code);
 
+void rkisp1_params_configure(struct rkisp1_params *params,
+			     enum rkisp1_fmt_raw_pat_type bayer_pat,
+			     enum v4l2_quantization quantization);
+void rkisp1_params_disable(struct rkisp1_params *params);
+
 void rkisp1_isp_isr(struct rkisp1_device *rkisp1);
 void rkisp1_mipi_isr(struct rkisp1_device *rkisp1);
 void rkisp1_capture_isr(struct rkisp1_device *rkisp1);
@@ -317,16 +319,15 @@ void rkisp1_params_isr(struct rkisp1_device *rkisp1, u32 isp_mis);
 int rkisp1_capture_devs_register(struct rkisp1_device *rkisp1);
 void rkisp1_capture_devs_unregister(struct rkisp1_device *rkisp1);
 
+int rkisp1_isp_register(struct rkisp1_device *rkisp1);
+void rkisp1_isp_unregister(struct rkisp1_device *rkisp1);
+
 int rkisp1_resizer_devs_register(struct rkisp1_device *rkisp1);
 void rkisp1_resizer_devs_unregister(struct rkisp1_device *rkisp1);
 
 int rkisp1_stats_register(struct rkisp1_device *rkisp1);
 void rkisp1_stats_unregister(struct rkisp1_device *rkisp1);
 
-void rkisp1_params_configure(struct rkisp1_params *params,
-			     enum rkisp1_fmt_raw_pat_type bayer_pat,
-			     enum v4l2_quantization quantization);
-void rkisp1_params_disable(struct rkisp1_params *params);
 int rkisp1_params_register(struct rkisp1_device *rkisp1);
 void rkisp1_params_unregister(struct rkisp1_device *rkisp1);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 5/5] media: staging: rkisp1: improve documentation of rkisp1-common.h
  2020-08-06 10:34 [PATCH v3 0/5] media: staging: rkisp1: document rkisp1-common.h Dafna Hirschfeld
                   ` (3 preceding siblings ...)
  2020-08-06 10:34 ` [PATCH v3 4/5] media: staging: rkisp1: group declaration of similar functions together Dafna Hirschfeld
@ 2020-08-06 10:34 ` Dafna Hirschfeld
  4 siblings, 0 replies; 6+ messages in thread
From: Dafna Hirschfeld @ 2020-08-06 10:34 UTC (permalink / raw)
  To: linux-media, laurent.pinchart
  Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel,
	dafna3, sakari.ailus, mchehab, tfiga

Add more detailed documentation of the structs and functions
in rkisp1-common.h

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
---
 drivers/staging/media/rkisp1/TODO            |   1 -
 drivers/staging/media/rkisp1/rkisp1-common.h | 233 +++++++++++++++----
 2 files changed, 190 insertions(+), 44 deletions(-)

diff --git a/drivers/staging/media/rkisp1/TODO b/drivers/staging/media/rkisp1/TODO
index bdb1b8f73556..f0c90d1c86a8 100644
--- a/drivers/staging/media/rkisp1/TODO
+++ b/drivers/staging/media/rkisp1/TODO
@@ -2,7 +2,6 @@
 * Fix checkpatch errors.
 * Review and comment every lock
 * Handle quantization
-* Document rkisp1-common.h
 * streaming paths (mainpath and selfpath) check if the other path is streaming
 in several places of the code, review this, specially that it doesn't seem it
 supports streaming from both paths at the same time.
diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
index 44acebcc7881..e6b60244c356 100644
--- a/drivers/staging/media/rkisp1/rkisp1-common.h
+++ b/drivers/staging/media/rkisp1/rkisp1-common.h
@@ -22,9 +22,14 @@
 #include "rkisp1-regs.h"
 #include "uapi/rkisp1-config.h"
 
+/*
+ * flags on the 'direction' field in struct 'rkisp1_isp_mbus_info' that indicate
+ * on which pad the media bus format is supported
+ */
 #define RKISP1_ISP_SD_SRC BIT(0)
 #define RKISP1_ISP_SD_SINK BIT(1)
 
+/* min and max values for the widths and heights of the entities */
 #define RKISP1_ISP_MAX_WIDTH		4032
 #define RKISP1_ISP_MAX_HEIGHT		3024
 #define RKISP1_ISP_MIN_WIDTH		32
@@ -37,29 +42,36 @@
 #define RKISP1_RSZ_SRC_MIN_WIDTH		32
 #define RKISP1_RSZ_SRC_MIN_HEIGHT		16
 
+/* the default width and height of all the entities */
 #define RKISP1_DEFAULT_WIDTH		800
 #define RKISP1_DEFAULT_HEIGHT		600
 
 #define RKISP1_DRIVER_NAME	"rkisp1"
 #define RKISP1_BUS_INFO		"platform:" RKISP1_DRIVER_NAME
 
+/* maximum number of clocks */
 #define RKISP1_MAX_BUS_CLK	8
 
+/* a bitmask of the ready stats */
 #define RKISP1_STATS_MEAS_MASK		(RKISP1_CIF_ISP_AWB_DONE |	\
 					 RKISP1_CIF_ISP_AFM_FIN |	\
 					 RKISP1_CIF_ISP_EXP_END |	\
 					 RKISP1_CIF_ISP_HIST_MEASURE_RDY)
+
+/* enum for the resizer pads */
 enum rkisp1_rsz_pad {
 	RKISP1_RSZ_PAD_SINK,
 	RKISP1_RSZ_PAD_SRC,
 	RKISP1_RSZ_PAD_MAX
 };
 
+/* enum for the capture id */
 enum rkisp1_stream_id {
 	RKISP1_MAINPATH,
 	RKISP1_SELFPATH,
 };
 
+/* bayer patterns */
 enum rkisp1_fmt_raw_pat_type {
 	RKISP1_RAW_RGGB = 0,
 	RKISP1_RAW_GRBG,
@@ -67,6 +79,7 @@ enum rkisp1_fmt_raw_pat_type {
 	RKISP1_RAW_BGGR,
 };
 
+/* enum for the isp pads */
 enum rkisp1_isp_pad {
 	RKISP1_ISP_PAD_SINK_VIDEO,
 	RKISP1_ISP_PAD_SINK_PARAMS,
@@ -76,8 +89,16 @@ enum rkisp1_isp_pad {
 };
 
 /*
- * struct rkisp1_sensor_async - Sensor information
- * @mbus: media bus configuration
+ * struct rkisp1_sensor_async - A container for the v4l2_async_subdev to add to the notifier
+ *				of the v4l2-async API
+ *
+ * @asd:		async_subdev variable for the sensor
+ * @lanes:		number of lanes
+ * @mbus_type:		type of bus (currently only CSI2 is supported)
+ * @mbus_flags:		media bus (V4L2_MBUS_*) flags
+ * @sd:			a pointer to v4l2_subdev struct of the sensor
+ * @pixel_rate_ctrl:	pixel rate of the sensor, used to initialize the phy
+ * @dphy:		a pointer to the phy
  */
 struct rkisp1_sensor_async {
 	struct v4l2_async_subdev asd;
@@ -90,19 +111,17 @@ struct rkisp1_sensor_async {
 };
 
 /*
- * struct rkisp1_isp - ISP sub-device
- *
- * See Cropping regions of ISP in rkisp1.c for details
- * @sink_frm: input size, don't have to be equal to sensor size
- * @sink_fmt: input format
- * @sink_crop: crop for sink pad
- * @src_fmt: output format
- * @src_crop: output size
- * @ops_lock: ops serialization
+ * struct rkisp1_isp - ISP subdev entity
  *
- * @is_dphy_errctrl_disabled : if dphy errctrl is disabled (avoid endless interrupt)
- * @frame_sequence: used to synchronize frame_id between video devices.
- * @quantization: output quantization
+ * @sd:				v4l2_subdev variable
+ * @rkisp1:			pointer to rkisp1_device
+ * @pads:			media pads
+ * @pad_cfg:			pads configurations
+ * @sink_fmt:			input format
+ * @src_fmt:			output format
+ * @ops_lock:			ops serialization
+ * @is_dphy_errctrl_disabled:	if dphy errctrl is disabled (avoid endless interrupt)
+ * @frame_sequence:		used to synchronize frame_id between video devices.
  */
 struct rkisp1_isp {
 	struct v4l2_subdev sd;
@@ -110,11 +129,19 @@ struct rkisp1_isp {
 	struct v4l2_subdev_pad_config pad_cfg[RKISP1_ISP_PAD_MAX];
 	const struct rkisp1_isp_mbus_info *sink_fmt;
 	const struct rkisp1_isp_mbus_info *src_fmt;
-	struct mutex ops_lock;
+	struct mutex ops_lock; /* serialize the subdevice ops */
 	bool is_dphy_errctrl_disabled;
 	atomic_t frame_sequence;
 };
 
+/*
+ * struct rkisp1_vdev_node - Container for the video nodes: params, stats, mainpath, selfpath
+ *
+ * @buf_queue:	queue of buffers
+ * @vlock:	lock of the video node
+ * @vdev:	video node
+ * @pad:	media pad
+ */
 struct rkisp1_vdev_node {
 	struct vb2_queue buf_queue;
 	struct mutex vlock; /* ioctl serialization mutex */
@@ -122,6 +149,15 @@ struct rkisp1_vdev_node {
 	struct media_pad pad;
 };
 
+/*
+ * struct rkisp1_buffer - A container for the vb2 buffers used by the video devices:
+ *			  params, stats, mainpath, selfpath
+ *
+ * @vb:		vb2 buffer
+ * @queue:	entry of the buffer in the queue
+ * @buff_addr:	dma addresses of each plane, used only by the capture devices: selfpath, mainpath
+ * @vaddr:	virtual address for buffers used by params and stats devices
+ */
 struct rkisp1_buffer {
 	struct vb2_v4l2_buffer vb;
 	struct list_head queue;
@@ -131,6 +167,14 @@ struct rkisp1_buffer {
 	};
 };
 
+/*
+ * struct rkisp1_dummy_buffer - A buffer to write the next frame to in case
+ *				there are no vb2 buffers available.
+ *
+ * @vaddr:	return value of call to dma_alloc_attrs.
+ * @dma_addr:	dma address of the buffer.
+ * @size:	size of the buffer.
+ */
 struct rkisp1_dummy_buffer {
 	void *vaddr;
 	dma_addr_t dma_addr;
@@ -142,17 +186,29 @@ struct rkisp1_device;
 /*
  * struct rkisp1_capture - ISP capture video device
  *
- * @pix.fmt: buffer format
- * @pix.info: pixel information
- * @pix.cfg: pixel configuration
- *
- * @buf.lock: lock to protect buf_queue
- * @buf.queue: queued buffer list
- * @buf.dummy: dummy space to store dropped data
+ * @vnode:	  video node
+ * @rkisp1:	  pointer to rkisp1_device
+ * @id:		  id of the capture, one of RKISP1_SELFPATH, RKISP1_MAINPATH
+ * @ops:	  list of callbacks to configure the capture device.
+ * @config:	  a pointer to the list of registers to configure the capture format.
+ * @is_streaming: device is streaming
+ * @is_stopping:  stop_streaming callback was called and the device is in the process of
+ *		  stopping the streaming.
+ * @done:	  when stop_streaming callback is called, the device waits for the next irq
+ *		  handler to stop the streaming by waiting on the 'done' wait queue.
+ *		  If the irq handler is not called, the stream is stopped by the callback
+ *		  after timeout.
+ * @sp_y_stride:  the selfpath allows to configure a y stride that is longer than the image width.
+ * @buf.lock:	  lock to protect buf.queue
+ * @buf.queue:	  queued buffer list
+ * @buf.dummy:	  dummy space to store dropped data
  *
- * rkisp1 use shadowsock registers, so it need two buffer at a time
- * @buf.curr: the buffer used for current frame
- * @buf.next: the buffer used for next frame
+ * rkisp1 uses shadow registers, so it needs two buffers at a time
+ * @buf.curr:	  the buffer used for current frame
+ * @buf.next:	  the buffer used for next frame
+ * @pix.cfg:	  pixel configuration
+ * @pix.info:	  a pointer to the v4l2_format_info of the pixel format
+ * @pix.fmt:	  buffer format
  */
 struct rkisp1_capture {
 	struct rkisp1_vdev_node vnode;
@@ -182,14 +238,18 @@ struct rkisp1_capture {
 /*
  * struct rkisp1_stats - ISP Statistics device
  *
- * @lock: locks the buffer list 'stat' and 'is_streaming'
- * @stat: stats buffer list
+ * @vnode:	  video node
+ * @rkisp1:	  pointer to the rkisp1 device
+ * @lock:	  locks the buffer list 'stat' and 'is_streaming'
+ * @stat:	  queue of rkisp1_buffer
+ * @vdev_fmt:	  v4l2_format of the metadata format
+ * @is_streaming: device is streaming
  */
 struct rkisp1_stats {
 	struct rkisp1_vdev_node vnode;
 	struct rkisp1_device *rkisp1;
 
-	spinlock_t lock; /* locks 'is_streaming', and 'stats' */
+	spinlock_t lock; /* locks the buffers list 'stats' and 'is_streaming' */
 	struct list_head stat;
 	struct v4l2_format vdev_fmt;
 	bool is_streaming;
@@ -198,14 +258,22 @@ struct rkisp1_stats {
 /*
  * struct rkisp1_params - ISP input parameters device
  *
- * @cur_params: Current ISP parameters
- * @is_first_params: the first params should take effect immediately
+ * @vnode:		video node
+ * @rkisp1:		pointer to the rkisp1 device
+ * @config_lock:	locks the buffer list 'params' and 'is_streaming'
+ * @params:		queue of rkisp1_buffer
+ * @cur_params:		the first params values from userspace
+ * @vdev_fmt:		v4l2_format of the metadata format
+ * @is_streaming:	device is streaming
+ * @is_first_params:	the first params should take effect immediately
+ * @quantization:	the quantization configured on the isp's src pad
+ * @raw_type:		the bayer pattern on the isp video sink pad
  */
 struct rkisp1_params {
 	struct rkisp1_vdev_node vnode;
 	struct rkisp1_device *rkisp1;
 
-	spinlock_t config_lock;
+	spinlock_t config_lock; /* locks the buffers list 'params' and 'is_streaming' */
 	struct list_head params;
 	struct rkisp1_params_cfg cur_params;
 	struct v4l2_format vdev_fmt;
@@ -216,6 +284,18 @@ struct rkisp1_params {
 	enum rkisp1_fmt_raw_pat_type raw_type;
 };
 
+/*
+ * struct rkisp1_resizer - Resizer subdev
+ *
+ * @sd:	       v4l2_subdev variable
+ * @id:	       id of the resizer, one of RKISP1_SELFPATH, RKISP1_MAINPATH
+ * @rkisp1:    pointer to the rkisp1 device
+ * @pads:      media pads
+ * @pad_cfg:   configurations for the pads
+ * @config:    the set of registers to configure the resizer
+ * @pixel_enc: pixel encoding of the resizer
+ * @ops_lock:  a lock for the subdev ops
+ */
 struct rkisp1_resizer {
 	struct v4l2_subdev sd;
 	enum rkisp1_stream_id id;
@@ -224,9 +304,26 @@ struct rkisp1_resizer {
 	struct v4l2_subdev_pad_config pad_cfg[RKISP1_RSZ_PAD_MAX];
 	const struct rkisp1_rsz_config *config;
 	enum v4l2_pixel_encoding pixel_enc;
-	struct mutex ops_lock;
+	struct mutex ops_lock; /* serialize the subdevice ops */
 };
 
+/*
+ * struct rkisp1_debug - Values to be exposed on debugfs.
+ *			 The parameters are counters of the number of times the
+ *			 event occurred since the driver was loaded.
+ *
+ * @data_loss:			  loss of data occurred within a line, processing failure
+ * @outform_size_error:		  size error is generated in outmux submodule
+ * @img_stabilization_size_error: size error is generated in image stabilization submodule
+ * @inform_size_err:		  size error is generated in inform submodule
+ * @mipi_error:			  mipi error occurred
+ * @stats_error:		  writing to the 'Interrupt clear register' did not clear
+ *				  it in the register 'Masked interrupt status'
+ * @stop_timeout:		  upon stream stop, the capture waits 1 second for the isr to stop
+ *				  the stream. This param is incremented in case of timeout.
+ * @frame_drop:			  a frame was ready but the buffer queue was empty so the frame
+ *				  was not sent to userspace
+ */
 struct rkisp1_debug {
 	struct dentry *debugfs_dir;
 	unsigned long data_loss;
@@ -241,13 +338,24 @@ struct rkisp1_debug {
 
 /*
  * struct rkisp1_device - ISP platform device
- * @base_addr: base register address
+ *
+ * @base_addr:	   base register address
+ * @irq:	   the irq number
+ * @dev:	   a pointer to the struct device
+ * @clk_size:	   number of clocks
+ * @clks:	   array of clocks
+ * @v4l2_dev:	   v4l2_device variable
+ * @media_dev:	   media_device variable
+ * @notifier:	   a notifier to register on the v4l2-async API to be notified on the sensor
  * @active_sensor: sensor in-use, set when streaming on
- * @isp: ISP sub-device
- * @rkisp1_capture: capture video device
- * @stats: ISP statistics output device
- * @params: ISP input parameters device
- * @stream_lock: lock to serialize start/stop streaming in capture devices.
+ * @isp:	   ISP sub-device
+ * @resizer_devs:  resizer sub-devices
+ * @capture_devs:  capture devices
+ * @stats:	   ISP statistics metadata capture device
+ * @params:	   ISP parameters metadata output device
+ * @pipe:	   media pipeline
+ * @stream_lock:   serializes {start/stop}_streaming callbacks between the capture devices.
+ * @debug:	   debug params to be exposed on debugfs
  */
 struct rkisp1_device {
 	void __iomem *base_addr;
@@ -265,16 +373,21 @@ struct rkisp1_device {
 	struct rkisp1_stats stats;
 	struct rkisp1_params params;
 	struct media_pipeline pipe;
-	struct mutex stream_lock;
+	struct mutex stream_lock; /* serialize {start/stop}_streaming cb between capture devices */
 	struct rkisp1_debug debug;
 };
 
 /*
- * struct rkisp1_isp_mbus_info - ISP pad format info
+ * struct rkisp1_isp_mbus_info - ISP media bus info, Translates media bus code to hardware
+ *				 format values
  *
- * Translate mbus_code to hardware format values
- *
- * @bus_width: used for parallel
+ * @mbus_code: media bus code
+ * @pixel_enc: pixel encoding
+ * @mipi_dt:   mipi data type
+ * @yuv_seq:   the order of the Y, Cb, Cr values
+ * @bus_width: bus width
+ * @bayer_pat: bayer pattern
+ * @direction: a bitmask of the flags indicating on which pad the format is supported on
  */
 struct rkisp1_isp_mbus_info {
 	u32 mbus_code;
@@ -297,25 +410,59 @@ static inline u32 rkisp1_read(struct rkisp1_device *rkisp1, unsigned int addr)
 	return readl(rkisp1->base_addr + addr);
 }
 
+/*
+ * rkisp1_sd_adjust_crop_rect - adjust a rectangle to fit into another rectangle.
+ *
+ * @crop:   rectangle to adjust.
+ * @bounds: rectangle used as bounds.
+ */
 void rkisp1_sd_adjust_crop_rect(struct v4l2_rect *crop,
 				const struct v4l2_rect *bounds);
 
+/*
+ * rkisp1_sd_adjust_crop - adjust a rectangle to fit into media bus format
+ *
+ * @crop:   rectangle to adjust.
+ * @bounds: media bus format used as bounds.
+ */
 void rkisp1_sd_adjust_crop(struct v4l2_rect *crop,
 			   const struct v4l2_mbus_framefmt *bounds);
 
+/*
+ * rkisp1_isp_mbus_info - get the isp info of the media bus code
+ *
+ * @mbus_code: the media bus code
+ */
 const struct rkisp1_isp_mbus_info *rkisp1_isp_mbus_info_get(u32 mbus_code);
 
+/* rkisp1_params_configure - configure the params when stream starts.
+ *			     This function is called by the isp entity upon stream starts.
+ *			     The function applies the initial configuration of the parameters.
+ *
+ * @params:	  pointer to rkisp1_params.
+ * @bayer_pat:	  the bayer pattern on the isp video sink pad
+ * @quantization: the quantization configured on the isp's src pad
+ */
 void rkisp1_params_configure(struct rkisp1_params *params,
 			     enum rkisp1_fmt_raw_pat_type bayer_pat,
 			     enum v4l2_quantization quantization);
+
+/* rkisp1_params_disable - disable all parameters.
+ *			   This function is called by the isp entity upon stream start
+ *			   when capturing bayer format.
+ *
+ * @params: pointer to rkisp1_params.
+ */
 void rkisp1_params_disable(struct rkisp1_params *params);
 
+/* irq handlers */
 void rkisp1_isp_isr(struct rkisp1_device *rkisp1);
 void rkisp1_mipi_isr(struct rkisp1_device *rkisp1);
 void rkisp1_capture_isr(struct rkisp1_device *rkisp1);
 void rkisp1_stats_isr(struct rkisp1_stats *stats, u32 isp_ris);
 void rkisp1_params_isr(struct rkisp1_device *rkisp1, u32 isp_mis);
 
+/* register/unregisters functions of the entities */
 int rkisp1_capture_devs_register(struct rkisp1_device *rkisp1);
 void rkisp1_capture_devs_unregister(struct rkisp1_device *rkisp1);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-08-06 11:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-06 10:34 [PATCH v3 0/5] media: staging: rkisp1: document rkisp1-common.h Dafna Hirschfeld
2020-08-06 10:34 ` [PATCH v3 1/5] media: staging: rkisp1: don't define vaddr field in rkisp1_buffer as an array Dafna Hirschfeld
2020-08-06 10:34 ` [PATCH v3 2/5] media: staging: rkisp1: unify (un)register functions to have the same parameters Dafna Hirschfeld
2020-08-06 10:34 ` [PATCH v3 3/5] media: staging: rkisp1: remove declaration of unimplemented function 'rkisp1_params_isr_handler' Dafna Hirschfeld
2020-08-06 10:34 ` [PATCH v3 4/5] media: staging: rkisp1: group declaration of similar functions together Dafna Hirschfeld
2020-08-06 10:34 ` [PATCH v3 5/5] media: staging: rkisp1: improve documentation of rkisp1-common.h Dafna Hirschfeld

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).