diff mbox series

[v1,5/7] staging: media: starfive: Add ISP raw video device

Message ID 20240306093334.9321-6-changhuang.liang@starfivetech.com (mailing list archive)
State New
Headers show
Series Add ISP RAW for StarFive | expand

Commit Message

Changhuang Liang March 6, 2024, 9:33 a.m. UTC 
Add raw video device to capture raw data from ISP.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 .../staging/media/starfive/camss/stf-camss.c  | 19 ++++++
 .../media/starfive/camss/stf-capture.c        | 58 ++++++++++++++++++-
 .../staging/media/starfive/camss/stf-video.h  |  1 +
 3 files changed, 77 insertions(+), 1 deletion(-)

Comments

Dan Carpenter March 6, 2024, 2:26 p.m. UTC | #1 
I wasn't able to get this patch to apply.  I tried applying the patch
mentioned in the cover letter first but it didn't help...  It's not
your fault, but it made reviewing the rest hard so I might have made
some mistakes.

On Wed, Mar 06, 2024 at 01:33:32AM -0800, Changhuang Liang wrote:
> Add raw video device to capture raw data from ISP.
> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  .../staging/media/starfive/camss/stf-camss.c  | 19 ++++++
>  .../media/starfive/camss/stf-capture.c        | 58 ++++++++++++++++++-
>  .../staging/media/starfive/camss/stf-video.h  |  1 +
>  3 files changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/starfive/camss/stf-camss.c b/drivers/staging/media/starfive/camss/stf-camss.c
> index 81fc39f20615..90ac8b67c76e 100644
> --- a/drivers/staging/media/starfive/camss/stf-camss.c
> +++ b/drivers/staging/media/starfive/camss/stf-camss.c
> @@ -126,6 +126,7 @@ static int stfcamss_of_parse_ports(struct stfcamss *stfcamss)
>  static int stfcamss_register_devs(struct stfcamss *stfcamss)
>  {
>  	struct stf_capture *cap_yuv = &stfcamss->captures[STF_CAPTURE_YUV];
> +	struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
>  	struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
>  	int ret;
>  
> @@ -150,8 +151,18 @@ static int stfcamss_register_devs(struct stfcamss *stfcamss)
>  
>  	cap_yuv->video.source_subdev = &isp_dev->subdev;
>  
> +	ret = media_create_pad_link(&isp_dev->subdev.entity, STF_ISP_PAD_SRC_RAW,
> +				    &cap_raw->video.vdev.entity, 0, 0);
> +	if (ret)
> +		goto err_rm_links0;
> +
> +	cap_raw->video.source_subdev = &isp_dev->subdev;
> +
>  	return ret;
>  
> +err_rm_links0:
> +	media_entity_remove_links(&isp_dev->subdev.entity);

I don't think this line is correct.  I think we only need to
remove &cap_yuv->video.vdev.entity.

> +	media_entity_remove_links(&cap_yuv->video.vdev.entity);
>  err_cap_unregister:
>  	stf_capture_unregister(stfcamss);
>  err_isp_unregister:
> @@ -162,6 +173,14 @@ static int stfcamss_register_devs(struct stfcamss *stfcamss)
>  
>  static void stfcamss_unregister_devs(struct stfcamss *stfcamss)
>  {
> +	struct stf_capture *cap_yuv = &stfcamss->captures[STF_CAPTURE_YUV];
> +	struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
> +	struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
> +
> +	media_entity_remove_links(&isp_dev->subdev.entity);

I think this line should be deleted.

> +	media_entity_remove_links(&cap_raw->video.vdev.entity);
> +	media_entity_remove_links(&cap_yuv->video.vdev.entity);

I think this "&cap_yuv" should be submitted by itself as a bugfix patch.

> +
>  	stf_isp_unregister(&stfcamss->isp_dev);
>  	stf_capture_unregister(stfcamss);
>  }

regards,
dan carpenter
Changhuang Liang March 7, 2024, 2:13 a.m. UTC | #2 
Hi, Dan

[...]
> >
> > +err_rm_links0:
> > +	media_entity_remove_links(&isp_dev->subdev.entity);
> 
> I don't think this line is correct.  I think we only need to remove
> &cap_yuv->video.vdev.entity.
> 

The instance I refer to needs to clear both the source entity and the sink entity. See
https://elixir.bootlin.com/linux/v6.8-rc7/source/drivers/media/platform/verisilicon/hantro_drv.c#L855

> > +	media_entity_remove_links(&cap_yuv->video.vdev.entity);
> >  err_cap_unregister:
> >  	stf_capture_unregister(stfcamss);
> >  err_isp_unregister:
> > @@ -162,6 +173,14 @@ static int stfcamss_register_devs(struct stfcamss
> > *stfcamss)
> >
> >  static void stfcamss_unregister_devs(struct stfcamss *stfcamss)  {
> > +	struct stf_capture *cap_yuv =
> &stfcamss->captures[STF_CAPTURE_YUV];
> > +	struct stf_capture *cap_raw =
> &stfcamss->captures[STF_CAPTURE_RAW];
> > +	struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
> > +
> > +	media_entity_remove_links(&isp_dev->subdev.entity);
> 
> I think this line should be deleted.
> 
> > +	media_entity_remove_links(&cap_raw->video.vdev.entity);
> > +	media_entity_remove_links(&cap_yuv->video.vdev.entity);
> 
> I think this "&cap_yuv" should be submitted by itself as a bugfix patch.
> 

Yes. 

Regards
Changhuang
Dan Carpenter March 7, 2024, 7:55 a.m. UTC | #3 
On Thu, Mar 07, 2024 at 02:13:50AM +0000, Changhuang Liang wrote:
> Hi, Dan
> 
> [...]
> > >
> > > +err_rm_links0:
> > > +	media_entity_remove_links(&isp_dev->subdev.entity);
> > 
> > I don't think this line is correct.  I think we only need to remove
> > &cap_yuv->video.vdev.entity.
> > 
> 
> The instance I refer to needs to clear both the source entity and the sink entity. See
> https://elixir.bootlin.com/linux/v6.8-rc7/source/drivers/media/platform/verisilicon/hantro_drv.c#L855
> 

Oh yeah.  It's the same in v4l2_m2m_register_media_controller().

regards,
dan carpenter
diff mbox series

Patch

diff --git a/drivers/staging/media/starfive/camss/stf-camss.c b/drivers/staging/media/starfive/camss/stf-camss.c
index 81fc39f20615..90ac8b67c76e 100644
--- a/drivers/staging/media/starfive/camss/stf-camss.c
+++ b/drivers/staging/media/starfive/camss/stf-camss.c
@@ -126,6 +126,7 @@  static int stfcamss_of_parse_ports(struct stfcamss *stfcamss)
 static int stfcamss_register_devs(struct stfcamss *stfcamss)
 {
 	struct stf_capture *cap_yuv = &stfcamss->captures[STF_CAPTURE_YUV];
+	struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
 	struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
 	int ret;
 
@@ -150,8 +151,18 @@  static int stfcamss_register_devs(struct stfcamss *stfcamss)
 
 	cap_yuv->video.source_subdev = &isp_dev->subdev;
 
+	ret = media_create_pad_link(&isp_dev->subdev.entity, STF_ISP_PAD_SRC_RAW,
+				    &cap_raw->video.vdev.entity, 0, 0);
+	if (ret)
+		goto err_rm_links0;
+
+	cap_raw->video.source_subdev = &isp_dev->subdev;
+
 	return ret;
 
+err_rm_links0:
+	media_entity_remove_links(&isp_dev->subdev.entity);
+	media_entity_remove_links(&cap_yuv->video.vdev.entity);
 err_cap_unregister:
 	stf_capture_unregister(stfcamss);
 err_isp_unregister:
@@ -162,6 +173,14 @@  static int stfcamss_register_devs(struct stfcamss *stfcamss)
 
 static void stfcamss_unregister_devs(struct stfcamss *stfcamss)
 {
+	struct stf_capture *cap_yuv = &stfcamss->captures[STF_CAPTURE_YUV];
+	struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
+	struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
+
+	media_entity_remove_links(&isp_dev->subdev.entity);
+	media_entity_remove_links(&cap_raw->video.vdev.entity);
+	media_entity_remove_links(&cap_yuv->video.vdev.entity);
+
 	stf_isp_unregister(&stfcamss->isp_dev);
 	stf_capture_unregister(stfcamss);
 }
diff --git a/drivers/staging/media/starfive/camss/stf-capture.c b/drivers/staging/media/starfive/camss/stf-capture.c
index 5c91126d5132..a5f10ec57782 100644
--- a/drivers/staging/media/starfive/camss/stf-capture.c
+++ b/drivers/staging/media/starfive/camss/stf-capture.c
@@ -12,6 +12,7 @@ 
 static const char * const stf_cap_names[] = {
 	"capture_dump",
 	"capture_yuv",
+	"capture_raw",
 };
 
 static const struct stfcamss_format_info stf_wr_fmts[] = {
@@ -55,6 +56,37 @@  static const struct stfcamss_format_info stf_isp_fmts[] = {
 	},
 };
 
+static const struct stfcamss_format_info stf_isp_raw_fmts[] = {
+	{
+		.code = MEDIA_BUS_FMT_SRGGB12_1X12,
+		.pixelformat = V4L2_PIX_FMT_SRGGB12,
+		.planes = 1,
+		.vsub = { 1 },
+		.bpp = 12,
+	},
+	{
+		.code = MEDIA_BUS_FMT_SGRBG12_1X12,
+		.pixelformat = V4L2_PIX_FMT_SGRBG12,
+		.planes = 1,
+		.vsub = { 1 },
+		.bpp = 12,
+	},
+	{
+		.code = MEDIA_BUS_FMT_SGBRG12_1X12,
+		.pixelformat = V4L2_PIX_FMT_SGBRG12,
+		.planes = 1,
+		.vsub = { 1 },
+		.bpp = 12,
+	},
+	{
+		.code = MEDIA_BUS_FMT_SBGGR12_1X12,
+		.pixelformat = V4L2_PIX_FMT_SBGGR12,
+		.planes = 1,
+		.vsub = { 1 },
+		.bpp = 12,
+	},
+};
+
 static inline struct stf_capture *to_stf_capture(struct stfcamss_video *video)
 {
 	return container_of(video, struct stf_capture, video);
@@ -73,6 +105,11 @@  static void stf_set_yuv_addr(struct stfcamss *stfcamss,
 	stf_isp_reg_write(stfcamss, ISP_REG_UV_PLANE_START_ADDR, uv_addr);
 }
 
+static void stf_set_raw_addr(struct stfcamss *stfcamss, dma_addr_t raw_addr)
+{
+	stf_isp_reg_write(stfcamss, ISP_REG_DUMP_CFG_0, raw_addr);
+}
+
 static void stf_init_addrs(struct stfcamss_video *video)
 {
 	struct stf_capture *cap = to_stf_capture(video);
@@ -91,6 +128,8 @@  static void stf_init_addrs(struct stfcamss_video *video)
 		stf_set_dump_addr(video->stfcamss, addr0);
 	else if (cap->type == STF_CAPTURE_YUV)
 		stf_set_yuv_addr(video->stfcamss, addr0, addr1);
+	else
+		stf_set_raw_addr(video->stfcamss, addr0);
 }
 
 static struct stfcamss_buffer *stf_buf_get_pending(struct stf_v_buf *output)
@@ -250,7 +289,6 @@  static void stf_capture_init(struct stfcamss *stfcamss, struct stf_capture *cap)
 
 	cap->video.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	cap->video.stfcamss = stfcamss;
-	cap->video.bpl_alignment = 16 * 8;
 
 	if (cap->type == STF_CAPTURE_DUMP) {
 		cap->video.formats = stf_wr_fmts;
@@ -260,6 +298,10 @@  static void stf_capture_init(struct stfcamss *stfcamss, struct stf_capture *cap)
 		cap->video.formats = stf_isp_fmts;
 		cap->video.nformats = ARRAY_SIZE(stf_isp_fmts);
 		cap->video.bpl_alignment = 1;
+	} else {
+		cap->video.formats = stf_isp_raw_fmts;
+		cap->video.nformats = ARRAY_SIZE(stf_isp_raw_fmts);
+		cap->video.bpl_alignment = STFCAMSS_FRAME_WIDTH_ALIGN_128;
 	}
 }
 
@@ -441,6 +483,8 @@  static void stf_change_buffer(struct stf_v_buf *output)
 			stf_set_dump_addr(stfcamss, new_addr[0]);
 		else if (cap->type == STF_CAPTURE_YUV)
 			stf_set_yuv_addr(stfcamss, new_addr[0], new_addr[1]);
+		else
+			stf_set_raw_addr(stfcamss, new_addr[0]);
 
 		stf_buf_add_ready(output, ready_buf);
 	}
@@ -468,6 +512,7 @@  irqreturn_t stf_wr_irq_handler(int irq, void *priv)
 irqreturn_t stf_isp_irq_handler(int irq, void *priv)
 {
 	struct stfcamss *stfcamss = priv;
+	struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
 	struct stf_capture *cap = &stfcamss->captures[STF_CAPTURE_YUV];
 	u32 status;
 
@@ -476,6 +521,9 @@  irqreturn_t stf_isp_irq_handler(int irq, void *priv)
 		if (status & ISPC_ENUO)
 			stf_buf_done(&cap->buffers);
 
+		if (status & ISPC_CSI)
+			stf_buf_done(&cap_raw->buffers);
+
 		stf_isp_reg_write(stfcamss, ISP_REG_ISP_CTRL_0,
 				  (status & ~ISPC_INT_ALL_MASK) |
 				  ISPC_ISP | ISPC_CSI | ISPC_SC);
@@ -487,14 +535,20 @@  irqreturn_t stf_isp_irq_handler(int irq, void *priv)
 irqreturn_t stf_line_irq_handler(int irq, void *priv)
 {
 	struct stfcamss *stfcamss = priv;
+	struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
 	struct stf_capture *cap = &stfcamss->captures[STF_CAPTURE_YUV];
 	u32 status;
+	u32 value;
 
 	status = stf_isp_reg_read(stfcamss, ISP_REG_ISP_CTRL_0);
 	if (status & ISPC_LINE) {
 		if (atomic_dec_if_positive(&cap->buffers.frame_skip) < 0) {
 			if ((status & ISPC_ENUO))
 				stf_change_buffer(&cap->buffers);
+
+			value = stf_isp_reg_read(stfcamss, ISP_REG_CSI_MODULE_CFG);
+			if (value & CSI_DUMP_EN)
+				stf_change_buffer(&cap_raw->buffers);
 		}
 
 		stf_isp_reg_set_bit(stfcamss, ISP_REG_CSIINTS,
@@ -571,9 +625,11 @@  void stf_capture_unregister(struct stfcamss *stfcamss)
 {
 	struct stf_capture *cap_dump = &stfcamss->captures[STF_CAPTURE_DUMP];
 	struct stf_capture *cap_yuv = &stfcamss->captures[STF_CAPTURE_YUV];
+	struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
 
 	stf_capture_unregister_one(cap_dump);
 	stf_capture_unregister_one(cap_yuv);
+	stf_capture_unregister_one(cap_raw);
 }
 
 int stf_capture_register(struct stfcamss *stfcamss,
diff --git a/drivers/staging/media/starfive/camss/stf-video.h b/drivers/staging/media/starfive/camss/stf-video.h
index 90c73c0dee89..cad861aae31c 100644
--- a/drivers/staging/media/starfive/camss/stf-video.h
+++ b/drivers/staging/media/starfive/camss/stf-video.h
@@ -37,6 +37,7 @@  enum stf_v_line_id {
 enum stf_capture_type {
 	STF_CAPTURE_DUMP = 0,
 	STF_CAPTURE_YUV,
+	STF_CAPTURE_RAW,
 	STF_CAPTURE_NUM,
 };