Hi Laurent, On 2023/8/2 18:38, Laurent Pinchart wrote: > Hi Jack, > > On Wed, Aug 02, 2023 at 05:58:26PM +0800, Jack Zhu wrote: >> On 2023/7/28 4:49, Laurent Pinchart wrote: >> > On Mon, Jun 19, 2023 at 07:28:38PM +0800, Jack Zhu wrote: >> >> Add Video In Controller driver for StarFive Camera Subsystem. >> > >> > I haven't reviewed this patch in details, as I have a high-level >> > question: why do you need VIN subdevs ? They don't seem to represent any >> > particular piece of hardware, their input and output formats are always >> > identical, and they're not used to configure registers. The contents of >> > this patch seems to belong to the video device, I think you can drop the >> > VIN subdevs. >> >> The VIN module corresponds to a hardware module, which is mainly responsible >> for data routing and partial interrupt management (when the image data does >> not pass through the isp but directly goes to the ddr), the relevant registers >> need to be configured. > > That's fine, but I don't think you need a subdev for it. As far as I > understand, the VIn modules are (more or less) DMA engines. You can just > model them as video devices, connected directly to the CSI-2 RX and ISP > source pads. > The VIN hardware can also route input data, it can decide whether DVP sensor or MIPI sensor is used as input data. > Does the "vin0_wr" have the ability to capture raw data from the DVP > interface as well, or only from the CSI-2 RX ? > Yes, the "vin0_wr" has the ability to capture raw data from the DVP interface. >> >> Signed-off-by: Jack Zhu <jack.zhu@xxxxxxxxxxxxxxxx> >> >> --- >> >> .../media/platform/starfive/camss/Makefile | 4 +- >> >> .../media/platform/starfive/camss/stf_camss.c | 42 +- >> >> .../media/platform/starfive/camss/stf_camss.h | 2 + >> >> .../media/platform/starfive/camss/stf_vin.c | 1069 +++++++++++++++++ >> >> .../media/platform/starfive/camss/stf_vin.h | 173 +++ >> >> .../platform/starfive/camss/stf_vin_hw_ops.c | 192 +++ >> >> 6 files changed, 1478 insertions(+), 4 deletions(-) >> >> create mode 100644 drivers/media/platform/starfive/camss/stf_vin.c >> >> create mode 100644 drivers/media/platform/starfive/camss/stf_vin.h >> >> create mode 100644 drivers/media/platform/starfive/camss/stf_vin_hw_ops.c >> >> >> >> diff --git a/drivers/media/platform/starfive/camss/Makefile b/drivers/media/platform/starfive/camss/Makefile >> >> index cdf57e8c9546..ef574e01ca47 100644 >> >> --- a/drivers/media/platform/starfive/camss/Makefile >> >> +++ b/drivers/media/platform/starfive/camss/Makefile >> >> @@ -7,6 +7,8 @@ starfive-camss-objs += \ >> >> stf_camss.o \ >> >> stf_isp.o \ >> >> stf_isp_hw_ops.o \ >> >> - stf_video.o >> >> + stf_video.o \ >> >> + stf_vin.o \ >> >> + stf_vin_hw_ops.o >> >> >> >> obj-$(CONFIG_VIDEO_STARFIVE_CAMSS) += starfive-camss.o >> >> diff --git a/drivers/media/platform/starfive/camss/stf_camss.c b/drivers/media/platform/starfive/camss/stf_camss.c >> >> index 6f56b45f57db..834ea63eb833 100644 >> >> --- a/drivers/media/platform/starfive/camss/stf_camss.c >> >> +++ b/drivers/media/platform/starfive/camss/stf_camss.c >> >> @@ -131,27 +131,61 @@ static int stfcamss_init_subdevices(struct stfcamss *stfcamss) >> >> return ret; >> >> } >> >> Regards, Jack Zhu
