Hi Helen; Sorry for late reply. Please check my feedback & questions below. On Tue, 2020-04-14 at 09:25 -0300, Helen Koike wrote: > On 4/8/20 11:05 PM, Jungo Lin wrote: > > Hi Helen: > > > > Thanks for your comments. > > > > On Tue, 2020-03-31 at 12:34 -0300, Helen Koike wrote: > >> Hello Jungo, > >> > >> I was taking a look at this patch (thanks for the work), > >> I didn't look in deep details, but I have some comments, please see > >> below. I hope it helps. > >> > >> On 12/19/19 3:49 AM, Jungo Lin wrote: > >>> This patch adds the Mediatek ISP P1 HW control device driver. > >>> It handles the ISP HW configuration, provides interrupt handling and > >>> initializes the V4L2 device nodes and other V4L2 functions. Moreover, > >>> implement standard V4L2 video driver that utilizes V4L2 and media > >>> framework APIs. It supports one media device, one sub-device and > >>> several video devices during initialization. Moreover, it also connects > >>> with sensor and seninf drivers with V4L2 async APIs. Communicate with > >>> co-process via SCP communication to compose ISP registers in the > >>> firmware. > >>> > >>> (The current metadata interface used in meta input and partial > >>> meta nodes is only a temporary solution to kick off the driver > >>> development and is not ready to be reviewed yet.) > >>> > >>> Signed-off-by: Jungo Lin <jungo.lin@xxxxxxxxxxxx> > >>> Signed-off-by: Tomasz Figa <tfiga@xxxxxxxxxxxx> > >>> Signed-off-by: Pi-Hsun Shih <pihsun@xxxxxxxxxxxx> > >>> --- > >>> Changes from v6: > >>> - Revise help description for VIDEO_MEDIATEK_ISP_PASS1 > >>> - Apply SCP v21 change in P1 driver by Pi-Hsun Shih > >>> - Correct auto suspend timer value for suspend/resume issue > >>> - Increase IPI guard timer to 1 second to avoid false alarm command timeout event > >>> - Fix KE due to no sen-inf sub-device > >>> --- > >>> drivers/media/platform/mtk-isp/Kconfig | 20 + > >>> .../media/platform/mtk-isp/isp_50/Makefile | 3 + > >>> .../platform/mtk-isp/isp_50/cam/Makefile | 6 + > >>> .../platform/mtk-isp/isp_50/cam/mtk_cam-hw.c | 636 +++++ > >>> .../platform/mtk-isp/isp_50/cam/mtk_cam-hw.h | 64 + > >>> .../platform/mtk-isp/isp_50/cam/mtk_cam-ipi.h | 222 ++ > >>> .../mtk-isp/isp_50/cam/mtk_cam-regs.h | 95 + > >>> .../platform/mtk-isp/isp_50/cam/mtk_cam.c | 2087 +++++++++++++++++ > >> > >> I think I would split this file a bit, to separate which code is being used for the subdevice, which for > >> capture, which for metadata, and what is being used to deal with requests. > >> > >> It would make it easier to review imho. > >> > > > > For file structure design, it was reviewed in the previous patch > > serials. > > e.g. > > https://patchwork.kernel.org/patch/10938137/ > > If you think it is better, I will modify it. > > Right, I saw a suggestion to merge two files there. > > I'm not sure what others think, but I'm used to see a separation per entity, or at least separate subdevices > from video devices, it is easier to see which v4l2 functions is being called per entity IMHO. > So it reflects a bit the topology. > But it is also up to you to see if it improves organization or not, it is just a suggestion. > Ok, got your point. We will discuss how to do internally. [snip] > >>> +isp_composer_hw_init(p1_dev); > >>> + > >>> +p1_dev->enqueued_frame_seq_no = 0; > >>> +p1_dev->dequeued_frame_seq_no = 0; > >>> +p1_dev->composed_frame_seq_no = 0; > >>> +p1_dev->sof_count = 0; > >>> + > >>> +dev_dbg(dev, "%s done\n", __func__); > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +int mtk_isp_hw_release(struct mtk_cam_dev *cam) > >>> +{ > >>> +struct device *dev = cam->dev; > >>> +struct mtk_isp_p1_device *p1_dev = dev_get_drvdata(dev); > >>> + > >>> +isp_composer_hw_deinit(p1_dev); > >>> +pm_runtime_mark_last_busy(dev); > >>> +pm_runtime_put_autosuspend(dev); > >>> +rproc_shutdown(p1_dev->rproc_handle); > >>> + > >>> +dev_dbg(dev, "%s done\n", __func__); > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +void mtk_isp_req_enqueue(struct mtk_cam_dev *cam, > >>> + struct mtk_cam_dev_request *req) > >>> +{ > >>> +struct mtk_isp_p1_device *p1_dev = dev_get_drvdata(cam->dev); > >>> + > >>> +/* Accumulated frame sequence number */ > >>> +req->frame_params.frame_seq_no = ++p1_dev->enqueued_frame_seq_no; > >>> + > >>> +INIT_WORK(&req->frame_work, isp_tx_frame_worker); > >>> +queue_work(p1_dev->composer_wq, &req->frame_work); > >>> +dev_dbg(cam->dev, "enqueue fd:%s frame_seq_no:%d job cnt:%d\n", > >>> +req->req.debug_str, req->frame_params.frame_seq_no, > >>> +cam->running_job_count); > >>> +} > >>> + > >>> +static void isp_irq_handle_sof(struct mtk_isp_p1_device *p1_dev, > >>> + unsigned int dequeued_frame_seq_no) > >>> +{ > >>> +dma_addr_t base_addr = p1_dev->composer_iova; > >>> +struct device *dev = p1_dev->dev; > >>> +struct mtk_cam_dev_request *req; > >>> +int composed_frame_seq_no = p1_dev->composed_frame_seq_no; > >>> +unsigned int addr_offset; > >>> + > >>> +/* Send V4L2_EVENT_FRAME_SYNC event */ > >>> +mtk_cam_dev_event_frame_sync(&p1_dev->cam_dev, dequeued_frame_seq_no); > >>> + > >>> +p1_dev->sof_count += 1; > >>> +/* Save frame information */ > >>> +p1_dev->dequeued_frame_seq_no = dequeued_frame_seq_no; > >>> + > >>> +req = mtk_cam_dev_get_req(&p1_dev->cam_dev, dequeued_frame_seq_no); > >>> +if (req) > >>> +req->timestamp = ktime_get_boottime_ns(); > >>> + > >>> +/* Update CQ base address if needed */ > >>> +if (composed_frame_seq_no <= dequeued_frame_seq_no) { > >>> +dev_dbg(dev, > >>> +"SOF_INT_ST, no update, cq_num:%d, frame_seq:%d\n", > >>> +composed_frame_seq_no, dequeued_frame_seq_no); > >>> +return; > >>> +} > >>> +addr_offset = MTK_ISP_CQ_ADDRESS_OFFSET * > >>> +(dequeued_frame_seq_no % MTK_ISP_CQ_BUFFER_COUNT); > >>> +writel(base_addr + addr_offset, p1_dev->regs + REG_CQ_THR0_BASEADDR); > >>> +dev_dbg(dev, > >>> +"SOF_INT_ST, update next, cq_num:%d, frame_seq:%d cq_addr:0x%x\n", > >>> +composed_frame_seq_no, dequeued_frame_seq_no, addr_offset); > >>> +} > >>> + > >>> +static void isp_irq_handle_dma_err(struct mtk_isp_p1_device *p1_dev) > >>> +{ > >>> +u32 val; > >>> + > >>> +dev_err(p1_dev->dev, > >>> +"IMGO:0x%x, RRZO:0x%x, AAO=0x%x, AFO=0x%x, LMVO=0x%x\n", > >>> +readl(p1_dev->regs + REG_IMGO_ERR_STAT), > >>> +readl(p1_dev->regs + REG_RRZO_ERR_STAT), > >>> +readl(p1_dev->regs + REG_AAO_ERR_STAT), > >>> +readl(p1_dev->regs + REG_AFO_ERR_STAT), > >>> +readl(p1_dev->regs + REG_LMVO_ERR_STAT)); > >>> +dev_err(p1_dev->dev, > >>> +"LCSO=0x%x, PSO=0x%x, FLKO=0x%x, BPCI:0x%x, LSCI=0x%x\n", > >>> +readl(p1_dev->regs + REG_LCSO_ERR_STAT), > >>> +readl(p1_dev->regs + REG_PSO_ERR_STAT), > >>> +readl(p1_dev->regs + REG_FLKO_ERR_STAT), > >>> +readl(p1_dev->regs + REG_BPCI_ERR_STAT), > >>> +readl(p1_dev->regs + REG_LSCI_ERR_STAT)); > >> > >> I think if would be better to transfor those into dev_dbg and add a counter > >> in debugfs. > >> > > > > These error messages are important for debugging. > > I suggest to keep in dev_err. > > I mean, these messages are usefull for debug (as you mentioned yourself), but for an > end user not so much, since end users won't know the meaning of those values. > > For end users a "dma failure" message would be enough, then advanced users can enable > debug messages to see more. OK. Got your point. > > > > Moreover, could you give more information about debug counter? > > I don't get your point. > > Do you suggest to accumulate the total count of DMA errors? > > > Yes, you could have a debugfs entry with error counters like: > > cat /debugfs/mtk_isp/dma_err > 8 > > So it is easier if this error happens very frequent or not. > In the rkisp1 case we added: > > /debugfs/rkisp1/data_loss > /debugfs/rkisp1/pic_size_error > /debugfs/rkisp1/mipi_error > /debugfs/rkisp1/stats_error > /debugfs/rkisp1/mp_stop_timeout > /debugfs/rkisp1/sp_stop_timeout > /debugfs/rkisp1/mp_frame_drop > /debugfs/rkisp1/sp_frame_drop > > Also, these error are non fatal, userspace can continue to work (in a way) when they happen, > so the idea was not to flood the logs with messages that end users don't care much, if they are frequent. > > But I'm not sure if this applies well or if it is useful to you case (please don't take my suggestions blindly). > Ok, we will follow your suggestion. [snip] > >>> +return; > >>> + > >>> +dev_dbg(cam->dev, "job done request:%s frame_seq:%d state:%d\n", > >>> +req->req.debug_str, req->frame_params.frame_seq_no, state); > >>> + > >>> +list_for_each_entry_safe(obj, obj_prev, &req->req.objects, list) { > >>> +struct vb2_buffer *vb; > >>> +struct mtk_cam_dev_buffer *buf; > >>> +struct mtk_cam_video_device *node; > >>> + > >>> +if (!vb2_request_object_is_buffer(obj)) > >>> +continue; > >>> +vb = container_of(obj, struct vb2_buffer, req_obj); > >>> +buf = mtk_cam_vb2_buf_to_dev_buf(vb); > >>> +node = mtk_cam_vbq_to_vdev(vb->vb2_queue); > >>> +spin_lock_irqsave(&node->buf_list_lock, flags); > >>> +list_del(&buf->list); > >>> +spin_unlock_irqrestore(&node->buf_list_lock, flags); > >>> +buf->vbb.sequence = req->frame_params.frame_seq_no; > >>> +if (V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) > >>> +vb->timestamp = ts_eof; > >>> +else > >>> +vb->timestamp = req->timestamp; > >>> +vb2_buffer_done(&buf->vbb.vb2_buf, state); > >>> +} > >>> +} > >>> + > >>> +struct mtk_cam_dev_request *mtk_cam_dev_get_req(struct mtk_cam_dev *cam, > >>> +unsigned int frame_seq_no) > >>> +{ > >>> +struct mtk_cam_dev_request *req, *req_prev; > >>> +unsigned long flags; > >>> + > >>> +spin_lock_irqsave(&cam->running_job_lock, flags); > >>> +list_for_each_entry_safe(req, req_prev, &cam->running_job_list, list) { > >>> +dev_dbg(cam->dev, "frame_seq:%d, get frame_seq:%d\n", > >>> +req->frame_params.frame_seq_no, frame_seq_no); > >>> + > >>> +/* Match by the en-queued request number */ > >>> +if (req->frame_params.frame_seq_no == frame_seq_no) { > >>> +spin_unlock_irqrestore(&cam->running_job_lock, flags); > >>> +return req; > >>> +} > >>> +} > >>> +spin_unlock_irqrestore(&cam->running_job_lock, flags); > >>> + > >>> +return NULL; > >>> +} > >>> + > >>> +void mtk_cam_dev_dequeue_req_frame(struct mtk_cam_dev *cam, > >>> + unsigned int frame_seq_no) > >>> +{ > >>> +struct mtk_cam_dev_request *req, *req_prev; > >>> +unsigned long flags; > >>> + > >>> +spin_lock_irqsave(&cam->running_job_lock, flags); > >>> +list_for_each_entry_safe(req, req_prev, &cam->running_job_list, list) { > >>> +dev_dbg(cam->dev, "frame_seq:%d, de-queue frame_seq:%d\n", > >>> +req->frame_params.frame_seq_no, frame_seq_no); > >>> + > >>> +/* Match by the en-queued request number */ > >>> +if (req->frame_params.frame_seq_no == frame_seq_no) { > >>> +cam->running_job_count--; > >>> +/* Pass to user space */ > >>> +mtk_cam_dev_job_done(cam, req, VB2_BUF_STATE_DONE); > >>> +list_del(&req->list); > >>> +break; > >>> +} else if (req->frame_params.frame_seq_no < frame_seq_no) { > >>> +cam->running_job_count--; > >>> +/* Pass to user space for frame drop */ > >>> +mtk_cam_dev_job_done(cam, req, VB2_BUF_STATE_ERROR); > >>> +dev_warn(cam->dev, "frame_seq:%d drop\n", > >>> + req->frame_params.frame_seq_no); > >> > >> maybe a counter in debugfs instead of the warning. > >> > > > > Do you mean to add counter to accumulate the total count of drop frames? > > please see my comment above. > Ok, add this in next patch. > > Could we add this and also keep this warning message? > > Userspace would still continue to work when this happens, not sure if it is worthy > adding a warn, I would move it to dev_dbg() instead IMHO. > Ok, revise in next patch. [snip] > >>> + > >>> +static void cal_image_pix_mp(struct mtk_cam_dev *cam, unsigned int node_id, > >>> + struct v4l2_pix_format_mplane *mp) > >>> +{ > >>> +unsigned int bpl, ppl; > >> > >> bytes per line and pixels per line right? > >> > > > > Yes. > > > >>> +unsigned int pixel_bits = get_pixel_bits(mp->pixelformat); > >> > >> wouldn't be easier a get_pixel_bytes() function instead of bits? > >> > > > > Sorry. I didn't get the point. > > The unit of return value is bits, not bytes. > > Do you suggest move bpl & ppl calculation into get_pixel_bits() and > > rename to get_pixel_bytes()? > > Never mind, I misread it. > Ok, we will skip this. [snip] > >>> +unsigned int enabled_dma_ports = cam->enabled_dmas; > >>> +int ret; > >>> + > >>> +/* Get sensor format configuration */ > >>> +sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE; > >>> +ret = v4l2_subdev_call(cam->sensor, pad, get_fmt, NULL, &sd_fmt); > >>> +if (ret) { > >>> +dev_dbg(dev, "sensor g_fmt failed:%d\n", ret); > >>> +return ret; > >>> +} > >>> +sd_width = sd_fmt.format.width; > >>> +sd_height = sd_fmt.format.height; > >>> +sd_code = sd_fmt.format.code; > >>> +dev_dbg(dev, "sd fmt w*h=%d*%d, code=0x%x\n", sd_width, sd_height, > >>> +sd_code); > >> > >> If V4L2_SUBDEV_FL_HAS_DEVNODE is used, then format shouldn't propagate from one node to the other, > >> it should be configured from userspace. > >> > > > > Could you explain why? > > Moreover, how does configuration from user space? > > IIUC there are two ways to configure the topology, see Hans comment on https://lkml.org/lkml/2020/2/6/305 > > If you use v4l2_device_register_subdev_nodes(), it exposes a /dev/v4l-subdevX file to userspace > in all subdevices you have the flag V4L2_SUBDEV_FL_HAS_DEVNODE (and you have it in the isp node). > > Which means that if the sensor implements VIDIOC_SUBDEV_S_FMT, part of the subdevices in the topology > can be configured by userspace and part can't (which iirc should't be done in the media API). > > Do you need to use v4l2_device_register_subdev_nodes() ? > > Also, Jacopo's patchset introduces a v4l2_device_register_ro_subdev_nodes() fuction: > https://patchwork.kernel.org/cover/11463183/ > > which would be more appropriated if you don't want userspace to configure the whole pipeline. > The purpose of P1 sub-device is to provide V4L2 event subscribe with V4L2_EVENT_FRAME_SYNC event for user space. It is the same implementation as blow link. https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/omap3isp/ispccdc.c#L1853 As you suggest, we may use v4l2_device_register_ro_subdev_nodes() more precisely. [snip] > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static void mtk_cam_vb2_buf_queue(struct vb2_buffer *vb) > >>> +{ > >>> +struct mtk_cam_dev *cam = vb2_get_drv_priv(vb->vb2_queue); > >>> +struct mtk_cam_dev_buffer *buf = mtk_cam_vb2_buf_to_dev_buf(vb); > >>> +struct mtk_cam_dev_request *req = mtk_cam_req_to_dev_req(vb->request); > >>> +struct mtk_cam_video_device *node = mtk_cam_vbq_to_vdev(vb->vb2_queue); > >>> +struct device *dev = cam->dev; > >>> +unsigned long flags; > >>> + > >>> +dev_dbg(dev, "%s: node:%d fd:%d idx:%d\n", __func__, > >>> +node->id, buf->vbb.request_fd, buf->vbb.vb2_buf.index); > >>> + > >>> +/* added the buffer into the tracking list */ > >>> +spin_lock_irqsave(&node->buf_list_lock, flags); > >>> +list_add_tail(&buf->list, &node->buf_list); > >>> +spin_unlock_irqrestore(&node->buf_list_lock, flags); > >>> + > >>> +/* update buffer internal address */ > >>> +req->frame_params.dma_bufs[buf->node_id].iova = buf->daddr; > >>> +req->frame_params.dma_bufs[buf->node_id].scp_addr = buf->scp_addr; > >> > >> isn't it an issue if userspace queue two buffers for the same video device in the same request? > >> > >> vb2_request_queue(req) will call all the .buf_queue() callbacks, and only the last buffer in the list > >> will be at req->frame_params.dma_bufs[buf->node_id], no? > >> > >> Also, what happens if a request doesn't contain buffers for all node_ids ? Will it put data in the previous programmed > >> buffer? > >> > >> Please, let me know if these questions doesn't make sense, I'm not that familiar with the request API internals. > >> > > > > 1. yes, it is a issue if userspace queues two buffers for the same video > > device with the same request FD. > > > > 2. All buffers which are belonged different to different video devices > > in the request list will be updated to req->frame_params.dma_bufs by > > buf->node_id. > > > > 3. It is not allowed for userspace to queue partial buffers for all > > enabled video devices. If it happens, it may trigger DMA errors for this > > request. > > So I guess you should implement a custom .req_validate() to enforce userspace follows this. > For case 1, it is handled in the vb2_queue_or_prepare_buf. https://elixir.bootlin.com/linux/latest/source/drivers/media/common/videobuf2/videobuf2-v4l2.c#L453 For case 3, I need to correct my previous answer. This behavior should be OK for outputted DMA. We have frame buffer controller for each outputted DMAs. So it means the tuning buffer node is mandatory for each request, other nodes are optional. We will implement this in .req_validate to check. > > > >>> +} > >>> + > >>> +static int mtk_cam_vb2_buf_init(struct vb2_buffer *vb) > >>> +{ > >>> +struct mtk_cam_video_device *node = mtk_cam_vbq_to_vdev(vb->vb2_queue); > >>> +struct mtk_cam_dev *cam = vb2_get_drv_priv(vb->vb2_queue); > >>> +struct device *dev = cam->dev; > >>> +struct mtk_cam_dev_buffer *buf; > >>> +dma_addr_t addr; > >>> + > >>> +buf = mtk_cam_vb2_buf_to_dev_buf(vb); > >>> +buf->node_id = node->id; > >>> +buf->daddr = vb2_dma_contig_plane_dma_addr(vb, 0); > >>> +buf->scp_addr = 0; > >>> + > >>> +/* SCP address is only valid for meta input buffer */ > >>> +if (!node->desc.smem_alloc) > >>> +return 0; > >>> + > >>> +buf = mtk_cam_vb2_buf_to_dev_buf(vb); > >>> +/* Use coherent address to get iova address */ > >>> +addr = dma_map_resource(dev, buf->daddr, vb->planes[0].length, > >>> +DMA_BIDIRECTIONAL, DMA_ATTR_SKIP_CPU_SYNC);> +if (dma_mapping_error(dev, addr)) { > >>> +dev_err(dev, "failed to map meta addr:%pad\n", &buf->daddr); > >>> +return -EFAULT; > >>> +} > >>> +buf->scp_addr = buf->daddr; > >>> +buf->daddr = addr; > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static int mtk_cam_vb2_buf_prepare(struct vb2_buffer *vb) > >>> +{ > >>> +struct mtk_cam_video_device *node = mtk_cam_vbq_to_vdev(vb->vb2_queue); > >>> +struct mtk_cam_dev *cam = vb2_get_drv_priv(vb->vb2_queue); > >>> +struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb); > >>> +const struct v4l2_format *fmt = &node->vdev_fmt; > >>> +unsigned int size; > >>> + > >>> +if (vb->vb2_queue->type == V4L2_BUF_TYPE_META_OUTPUT || > >>> + vb->vb2_queue->type == V4L2_BUF_TYPE_META_CAPTURE) > >>> +size = fmt->fmt.meta.buffersize; > >>> +else > >>> +size = fmt->fmt.pix_mp.plane_fmt[0].sizeimage; > >>> + > >>> +if (vb2_plane_size(vb, 0) < size) { > >>> +dev_dbg(cam->dev, "plane size is too small:%lu<%u\n", > >>> +vb2_plane_size(vb, 0), size); > >>> +return -EINVAL; > >>> +} > >>> + > >>> +if (V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) { > >>> +if (vb2_get_plane_payload(vb, 0) != size) { > >>> +dev_dbg(cam->dev, "plane payload is mismatch:%lu:%u\n", > >>> +vb2_get_plane_payload(vb, 0), size); > >>> +return -EINVAL; > >>> +} > >>> +return 0; > >>> +} > >>> + > >>> +v4l2_buf->field = V4L2_FIELD_NONE; > >>> +vb2_set_plane_payload(vb, 0, size); > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static void mtk_cam_vb2_buf_cleanup(struct vb2_buffer *vb) > >>> +{ > >>> +struct mtk_cam_video_device *node = mtk_cam_vbq_to_vdev(vb->vb2_queue); > >>> +struct mtk_cam_dev *cam = vb2_get_drv_priv(vb->vb2_queue); > >>> +struct mtk_cam_dev_buffer *buf; > >>> +struct device *dev = cam->dev; > >>> + > >>> +if (!node->desc.smem_alloc) > >>> +return; > >>> + > >>> +buf = mtk_cam_vb2_buf_to_dev_buf(vb); > >>> +dma_unmap_page_attrs(dev, buf->daddr, > >>> + vb->planes[0].length, > >>> + DMA_BIDIRECTIONAL, > >>> + DMA_ATTR_SKIP_CPU_SYNC); > >>> +} > >>> + > >>> +static void mtk_cam_vb2_request_complete(struct vb2_buffer *vb) > >>> +{ > >>> +struct mtk_cam_dev *cam = vb2_get_drv_priv(vb->vb2_queue); > >>> + > >>> +dev_dbg(cam->dev, "%s\n", __func__); > >>> +} > >>> + > >>> +static int mtk_cam_vb2_queue_setup(struct vb2_queue *vq, > >>> + unsigned int *num_buffers, > >>> + unsigned int *num_planes, > >>> + unsigned int sizes[], > >>> + struct device *alloc_devs[]) > >>> +{ > >>> +struct mtk_cam_video_device *node = mtk_cam_vbq_to_vdev(vq); > >>> +unsigned int max_buffer_count = node->desc.max_buf_count; > >>> +const struct v4l2_format *fmt = &node->vdev_fmt; > >>> +unsigned int size; > >>> + > >>> +/* Check the limitation of buffer size */ > >>> +if (max_buffer_count) > >>> +*num_buffers = clamp_val(*num_buffers, 1, max_buffer_count); > >>> + > >>> +if (node->desc.smem_alloc) > >>> +vq->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING; > >>> + > >>> +if (vq->type == V4L2_BUF_TYPE_META_OUTPUT || > >>> + vq->type == V4L2_BUF_TYPE_META_CAPTURE) > >>> +size = fmt->fmt.meta.buffersize; > >>> +else > >>> +size = fmt->fmt.pix_mp.plane_fmt[0].sizeimage; > >>> + > >>> +/* Add for q.create_bufs with fmt.g_sizeimage(p) / 2 test */ > >>> +if (*num_planes) { > >>> +if (sizes[0] < size || *num_planes != 1) > >>> +return -EINVAL; > >>> +} else { > >>> +*num_planes = 1; > >>> +sizes[0] = size; > >>> +} > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static void mtk_cam_vb2_return_all_buffers(struct mtk_cam_dev *cam, > >>> + struct mtk_cam_video_device *node, > >>> + enum vb2_buffer_state state) > >>> +{ > >>> +struct mtk_cam_dev_buffer *buf, *buf_prev; > >>> +unsigned long flags; > >>> + > >>> +spin_lock_irqsave(&node->buf_list_lock, flags); > >>> +list_for_each_entry_safe(buf, buf_prev, &node->buf_list, list) { > >>> +list_del(&buf->list); > >>> +vb2_buffer_done(&buf->vbb.vb2_buf, state); > >>> +} > >>> +spin_unlock_irqrestore(&node->buf_list_lock, flags); > >>> +} > >>> + > >>> +static int mtk_cam_vb2_start_streaming(struct vb2_queue *vq, > >>> + unsigned int count) > >>> +{ > >>> +struct mtk_cam_dev *cam = vb2_get_drv_priv(vq); > >>> +struct mtk_cam_video_device *node = mtk_cam_vbq_to_vdev(vq); > >>> +struct device *dev = cam->dev; > >>> +int ret; > >>> + > >>> +if (!node->enabled) { > >>> +dev_err(dev, "Node:%d is not enabled\n", node->id); > >>> +ret = -ENOLINK; > >>> +goto fail_ret_buf; > >>> +} > >>> + > >>> +mutex_lock(&cam->op_lock); > >>> +/* Start streaming of the whole pipeline now*/ > >>> +if (!cam->pipeline.streaming_count) { > >> > >> No need for this check, vb2 won't call .start_streaming() twice without stop_streaming() in between. > >> > > > > The check is designed to start the media pipeline when we start > > streaming on the first node. You could refer the detail in below link. > > > > https://patchwork.kernel.org/patch/10985819/ > > right, ok, this is when enabling streaming from multiple nodes. > > media_pipeline_start() is usually called for every stream that starts. > > So cam->pipeline.streaming_count can reflect the number of streams enabled. > > So maybe you don't need cam->stream_count. > Ok, revise in next patch. > > > > > >>> +ret = media_pipeline_start(&node->vdev.entity, &cam->pipeline); > >>> +if (ret) { > >>> +dev_err(dev, "failed to start pipeline:%d\n", ret); > >>> +goto fail_unlock; > >>> +} > >>> +mtk_cam_dev_init_stream(cam); > >>> +ret = mtk_isp_hw_init(cam); > > Would it make sense to move this to s_stream in the ISP's subdevice ? > No, we like to initialize our ISP firmware here when the first video node is streaming on. It is too late to initialize when all video nodes are streaming-on. > >>> +if (ret) { > >>> +dev_err(dev, "failed to init HW:%d\n", ret); > >>> +goto fail_stop_pipeline; > >>> +} > >>> +} > >>> + > >>> +/* Media links are fixed after media_pipeline_start */ > >>> +cam->stream_count++; > >>> +dev_dbg(dev, "%s: count info:%d:%d\n", __func__, cam->stream_count, > >>> +cam->enabled_count); > >>> +if (cam->stream_count < cam->enabled_count) { > > I'm also wondering, since you need to wait for all the enabled video devices > to start streaming, shouldn't this be done inside a request? So you can enable > all of them at once? > > Also, like this you wouldn't need to check enabled links to query for enabled video > nodes, you can just enable the ones in the request. > > make sense? > Sorry, I didn't get your point about this comment. Which request could we handle this logic? Do you mean move this logic into mtk_cam_req_queue function? > >>> +mutex_unlock(&cam->op_lock); > >>> +return 0; > >>> +} > >>> + > >>> +/* Stream on sub-devices node */ > >>> +ret = v4l2_subdev_call(&cam->subdev, video, s_stream, 1); > >>> +if (ret) > >>> +goto fail_no_stream; > >>> +mutex_unlock(&cam->op_lock); > >>> + > >>> +return 0; > >>> + > >>> +fail_no_stream: > >>> +cam->stream_count--; > >>> +fail_stop_pipeline: > >>> +if (cam->stream_count == 0) > >>> +media_pipeline_stop(&node->vdev.entity); > >>> +fail_unlock: > >>> +mutex_unlock(&cam->op_lock); > >>> +fail_ret_buf: > >>> +mtk_cam_vb2_return_all_buffers(cam, node, VB2_BUF_STATE_QUEUED); > >>> + > >>> +return ret; > >>> +} > >>> + > >>> +static void mtk_cam_vb2_stop_streaming(struct vb2_queue *vq) > >>> +{ > >>> +struct mtk_cam_dev *cam = vb2_get_drv_priv(vq); > >>> +struct mtk_cam_video_device *node = mtk_cam_vbq_to_vdev(vq); > >>> +struct device *dev = cam->dev; > >>> + > >>> +mutex_lock(&cam->op_lock); > >>> +dev_dbg(dev, "%s node:%d count info:%d\n", __func__, node->id, > >>> +cam->stream_count); > >>> +/* Check the first node to stream-off */ > >>> +if (cam->stream_count == cam->enabled_count) > >>> +v4l2_subdev_call(&cam->subdev, video, s_stream, 0); > >>> + > >>> +mtk_cam_vb2_return_all_buffers(cam, node, VB2_BUF_STATE_ERROR); > >>> +cam->stream_count--; > >>> +if (cam->stream_count) { > >>> +mutex_unlock(&cam->op_lock); > >>> +return; > >>> +} > >>> +mutex_unlock(&cam->op_lock); > >>> + > >>> +mtk_cam_dev_req_cleanup(cam); > >>> +media_pipeline_stop(&node->vdev.entity); > >>> +} > >>> + > >>> +static int mtk_cam_vidioc_querycap(struct file *file, void *fh, > >>> + struct v4l2_capability *cap) > >>> +{ > >>> +struct mtk_cam_dev *cam = video_drvdata(file); > >>> + > >>> +strscpy(cap->driver, dev_driver_string(cam->dev), sizeof(cap->driver)); > >>> +strscpy(cap->card, dev_driver_string(cam->dev), sizeof(cap->card)); > >>> +snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s", > >>> + dev_name(cam->dev)); > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static int mtk_cam_vidioc_enum_fmt(struct file *file, void *fh, > >>> + struct v4l2_fmtdesc *f) > >>> +{ > >>> +struct mtk_cam_video_device *node = file_to_mtk_cam_node(file); > >>> + > >>> +if (f->index >= node->desc.num_fmts) > >>> +return -EINVAL; > >>> + > >>> +/* f->description is filled in v4l_fill_fmtdesc function */ > >>> +f->pixelformat = node->desc.fmts[f->index].fmt.pix_mp.pixelformat; > >>> +f->flags = 0; > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static int mtk_cam_vidioc_g_fmt(struct file *file, void *fh, > >>> +struct v4l2_format *f) > >>> +{ > >>> +struct mtk_cam_video_device *node = file_to_mtk_cam_node(file); > >>> + > >>> +f->fmt = node->vdev_fmt.fmt; > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static int mtk_cam_vidioc_try_fmt(struct file *file, void *fh, > >>> + struct v4l2_format *f) > >>> +{ > >>> +struct mtk_cam_dev *cam = video_drvdata(file); > >>> +struct mtk_cam_video_device *node = file_to_mtk_cam_node(file); > >>> +struct device *dev = cam->dev; > >>> +const struct v4l2_format *dev_fmt; > >>> +struct v4l2_format try_fmt; > >>> + > >>> +memset(&try_fmt, 0, sizeof(try_fmt)); > >>> +try_fmt.type = f->type; > >>> + > >>> +/* Validate pixelformat */ > >>> +dev_fmt = mtk_cam_dev_find_fmt(&node->desc, f->fmt.pix_mp.pixelformat); > >>> +if (!dev_fmt) { > >>> +dev_dbg(dev, "unknown fmt:%d\n", f->fmt.pix_mp.pixelformat); > >>> +dev_fmt = &node->desc.fmts[node->desc.default_fmt_idx]; > >>> +} > >>> +try_fmt.fmt.pix_mp.pixelformat = dev_fmt->fmt.pix_mp.pixelformat; > >>> + > >>> +/* Validate image width & height range */ > >>> +try_fmt.fmt.pix_mp.width = clamp_val(f->fmt.pix_mp.width, > >>> + IMG_MIN_WIDTH, IMG_MAX_WIDTH); > >>> +try_fmt.fmt.pix_mp.height = clamp_val(f->fmt.pix_mp.height, > >>> + IMG_MIN_HEIGHT, IMG_MAX_HEIGHT); > >>> +/* 4 bytes alignment for width */ > >>> +try_fmt.fmt.pix_mp.width = ALIGN(try_fmt.fmt.pix_mp.width, 4); > >>> + > >>> +/* Only support one plane */ > >>> +try_fmt.fmt.pix_mp.num_planes = 1; > >>> + > >>> +/* bytesperline & sizeimage calculation */ > >>> +cal_image_pix_mp(cam, node->id, &try_fmt.fmt.pix_mp); > >>> + > >>> +/* Constant format fields */ > >>> +try_fmt.fmt.pix_mp.colorspace = V4L2_COLORSPACE_SRGB; > >>> +try_fmt.fmt.pix_mp.field = V4L2_FIELD_NONE; > >>> +try_fmt.fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > >>> +try_fmt.fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT; > >>> +try_fmt.fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_SRGB; > >>> + > >>> +*f = try_fmt; > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static int mtk_cam_vidioc_s_fmt(struct file *file, void *fh, > >>> +struct v4l2_format *f) > >>> +{ > >>> +struct mtk_cam_dev *cam = video_drvdata(file); > >>> +struct mtk_cam_video_device *node = file_to_mtk_cam_node(file); > >>> + > >>> +if (vb2_is_busy(node->vdev.queue)) { > >>> +dev_dbg(cam->dev, "%s: queue is busy\n", __func__); > >>> +return -EBUSY; > >>> +} > >>> + > >>> +/* Get the valid format */ > >>> +mtk_cam_vidioc_try_fmt(file, fh, f); > >>> +/* Configure to video device */ > >>> +node->vdev_fmt = *f; > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static int mtk_cam_vidioc_enum_framesizes(struct file *filp, void *priv, > >>> + struct v4l2_frmsizeenum *sizes) > >>> +{ > >>> +struct mtk_cam_video_device *node = file_to_mtk_cam_node(filp); > >>> +const struct v4l2_format *dev_fmt; > >>> + > >>> +dev_fmt = mtk_cam_dev_find_fmt(&node->desc, sizes->pixel_format); > >>> +if (!dev_fmt || sizes->index) > >>> +return -EINVAL; > >>> + > >>> +sizes->type = node->desc.frmsizes->type; > >>> +memcpy(&sizes->stepwise, &node->desc.frmsizes->stepwise, > >>> + sizeof(sizes->stepwise)); > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static int mtk_cam_vidioc_meta_enum_fmt(struct file *file, void *fh, > >>> +struct v4l2_fmtdesc *f) > >>> +{ > >>> +struct mtk_cam_video_device *node = file_to_mtk_cam_node(file); > >>> + > >>> +if (f->index) > >>> +return -EINVAL; > >>> + > >>> +/* f->description is filled in v4l_fill_fmtdesc function */ > >>> +f->pixelformat = node->vdev_fmt.fmt.meta.dataformat; > >>> +f->flags = 0; > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static int mtk_cam_vidioc_g_meta_fmt(struct file *file, void *fh, > >>> + struct v4l2_format *f) > >>> +{ > >>> +struct mtk_cam_video_device *node = file_to_mtk_cam_node(file); > >>> + > >>> +f->fmt.meta.dataformat = node->vdev_fmt.fmt.meta.dataformat; > >>> +f->fmt.meta.buffersize = node->vdev_fmt.fmt.meta.buffersize; > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static const struct v4l2_subdev_core_ops mtk_cam_subdev_core_ops = { > >>> +.subscribe_event = mtk_cam_sd_subscribe_event, > >>> +.unsubscribe_event = v4l2_event_subdev_unsubscribe, > >>> +}; > >>> + > >>> +static const struct v4l2_subdev_video_ops mtk_cam_subdev_video_ops = { > >>> +.s_stream = mtk_cam_sd_s_stream, > >>> +}; > >>> + > >>> +static const struct v4l2_subdev_ops mtk_cam_subdev_ops = { > >>> +.core = &mtk_cam_subdev_core_ops, > >>> +.video = &mtk_cam_subdev_video_ops, > >>> +}; > >> > >> hmm, since this subdevice is exposed with V4L2_SUBDEV_FL_HAS_DEVNODE, > >> I wonder if pad ops shouldn't be implemented too (to be verified). > >> > > > > Ok, I will investigate this. > > > >>> + > >>> +static const struct media_entity_operations mtk_cam_media_entity_ops = { > >>> +.link_setup = mtk_cam_media_link_setup, > >>> +.link_validate = v4l2_subdev_link_validate, > >>> +}; > >>> + > >>> +static const struct vb2_ops mtk_cam_vb2_ops = { > >>> +.queue_setup = mtk_cam_vb2_queue_setup, > >>> +.wait_prepare = vb2_ops_wait_prepare, > >>> +.wait_finish = vb2_ops_wait_finish, > >>> +.buf_init = mtk_cam_vb2_buf_init, > >>> +.buf_prepare = mtk_cam_vb2_buf_prepare, > >>> +.start_streaming = mtk_cam_vb2_start_streaming, > >>> +.stop_streaming = mtk_cam_vb2_stop_streaming, > >>> +.buf_queue = mtk_cam_vb2_buf_queue, > >>> +.buf_cleanup = mtk_cam_vb2_buf_cleanup, > >>> +.buf_request_complete = mtk_cam_vb2_request_complete, > >>> +};> + > >>> +static const struct v4l2_file_operations mtk_cam_v4l2_fops = { > >>> +.unlocked_ioctl = video_ioctl2, > >>> +.open = v4l2_fh_open, > >>> +.release = vb2_fop_release, > >>> +.poll = vb2_fop_poll, > >>> +.mmap = vb2_fop_mmap, > >>> +#ifdef CONFIG_COMPAT > >>> +.compat_ioctl32 = v4l2_compat_ioctl32, > >>> +#endif > >>> +}; > >>> + > >>> +static const struct media_device_ops mtk_cam_media_ops = { > >>> +.req_alloc = mtk_cam_req_alloc, > >>> +.req_free = mtk_cam_req_free, > >>> +.req_validate = vb2_request_validate, > >>> +.req_queue = mtk_cam_req_queue, > >>> +}; > >>> + > >>> +static int mtk_cam_media_register(struct mtk_cam_dev *cam, > >>> + struct media_device *media_dev) > >>> +{ > >>> +/* Reserved MTK_CAM_CIO_PAD_SINK + 1 pads to use */ > >>> +unsigned int num_pads = MTK_CAM_CIO_PAD_SINK + 1; > >>> +struct device *dev = cam->dev; > >>> +int i, ret; > >>> + > >>> +media_dev->dev = cam->dev; > >>> +strscpy(media_dev->model, dev_driver_string(dev), > >>> +sizeof(media_dev->model)); > >>> +snprintf(media_dev->bus_info, sizeof(media_dev->bus_info), > >>> + "platform:%s", dev_name(dev)); > >>> +media_dev->hw_revision = 0; > >>> +media_device_init(media_dev); > >>> +media_dev->ops = &mtk_cam_media_ops; > >>> + > >>> +ret = media_device_register(media_dev); > >>> +if (ret) { > >>> +dev_err(dev, "failed to register media device:%d\n", ret); > >>> +return ret; > >>> +} > >>> + > >>> +/* Initialize subdev pads */ > >>> +cam->subdev_pads = devm_kcalloc(dev, num_pads, > >>> +sizeof(*cam->subdev_pads), > >>> +GFP_KERNEL); > >>> +if (!cam->subdev_pads) { > >>> +dev_err(dev, "failed to allocate subdev_pads\n"); > >>> +ret = -ENOMEM; > >>> +goto fail_media_unreg; > >>> +} > >>> + > >>> +ret = media_entity_pads_init(&cam->subdev.entity, num_pads, > >>> + cam->subdev_pads); > >>> +if (ret) { > >>> +dev_err(dev, "failed to initialize media pads:%d\n", ret); > >>> +goto fail_media_unreg; > >>> +} > >>> + > >>> +/* Initialize all pads with MEDIA_PAD_FL_SOURCE */ > >>> +for (i = 0; i < num_pads; i++) > >>> +cam->subdev_pads[i].flags = MEDIA_PAD_FL_SOURCE; > >>> + > >>> +/* Customize the last one pad as CIO sink pad. */ > >>> +cam->subdev_pads[MTK_CAM_CIO_PAD_SINK].flags = MEDIA_PAD_FL_SINK; > >>> + > >>> +return 0; > >>> + > >>> +fail_media_unreg: > >>> +media_device_unregister(&cam->media_dev); > >>> +media_device_cleanup(&cam->media_dev); > >>> + > >>> +return ret; > >>> +} > >>> + > >>> +static int > >>> +mtk_cam_video_register_device(struct mtk_cam_dev *cam, > >>> + struct mtk_cam_video_device *node) > >>> +{ > >>> +struct device *dev = cam->dev; > >>> +struct video_device *vdev = &node->vdev; > >>> +struct vb2_queue *vbq = &node->vbq; > >>> +unsigned int output = V4L2_TYPE_IS_OUTPUT(node->desc.buf_type); > >>> +unsigned int link_flags = node->desc.link_flags; > >>> +int ret; > >>> + > >>> +/* Initialize mtk_cam_video_device */ > >>> +if (link_flags & MEDIA_LNK_FL_IMMUTABLE) > >>> +node->enabled = true; > >>> +else > >>> +node->enabled = false; > >>> +mtk_cam_dev_load_default_fmt(cam, &node->desc, &node->vdev_fmt); > >>> + > >>> +cam->subdev_pads[node->id].flags = output ? > >>> +MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE; > >>> + > >>> +/* Initialize media entities */ > >>> +ret = media_entity_pads_init(&vdev->entity, 1, &node->vdev_pad); > >>> +if (ret) { > >>> +dev_err(dev, "failed to initialize media pad:%d\n", ret); > >>> +return ret; > >>> +} > >>> +node->vdev_pad.flags = output ? MEDIA_PAD_FL_SOURCE : MEDIA_PAD_FL_SINK; > >>> + > >>> +/* Initialize vbq */ > >>> +vbq->type = node->desc.buf_type; > >>> +if (vbq->type == V4L2_BUF_TYPE_META_OUTPUT) > >>> +vbq->io_modes = VB2_MMAP; > >>> +else > >>> +vbq->io_modes = VB2_MMAP | VB2_DMABUF; > >>> + > >>> +if (node->desc.smem_alloc) { > >>> +vbq->bidirectional = 1; > >>> +vbq->dev = cam->smem_dev; > >>> +} else { > >>> +vbq->dev = dev; > >>> +} > >>> +vbq->ops = &mtk_cam_vb2_ops; > >>> +vbq->mem_ops = &vb2_dma_contig_memops; > >>> +vbq->buf_struct_size = sizeof(struct mtk_cam_dev_buffer); > >>> +vbq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_BOOTIME; > >>> +if (output) > >>> +vbq->timestamp_flags |= V4L2_BUF_FLAG_TSTAMP_SRC_EOF; > >>> +else > >>> +vbq->timestamp_flags |= V4L2_BUF_FLAG_TSTAMP_SRC_SOE; > >>> +/* No minimum buffers limitation */ > >>> +vbq->min_buffers_needed = 0; > >>> +vbq->drv_priv = cam; > >>> +vbq->lock = &node->vdev_lock; > >>> +vbq->supports_requests = true; > >>> +vbq->requires_requests = true; > >>> + > >>> +ret = vb2_queue_init(vbq); > >>> +if (ret) { > >>> +dev_err(dev, "failed to init. vb2 queue:%d\n", ret); > >>> +goto fail_media_clean; > >>> +} > >>> + > >>> +/* Initialize vdev */ > >>> +snprintf(vdev->name, sizeof(vdev->name), "%s %s", > >>> + dev_driver_string(dev), node->desc.name); > >>> +/* set cap/type/ioctl_ops of the video device */ > >>> +vdev->device_caps = node->desc.cap | V4L2_CAP_STREAMING; > >>> +vdev->ioctl_ops = node->desc.ioctl_ops; > >>> +vdev->fops = &mtk_cam_v4l2_fops; > >>> +vdev->release = video_device_release_empty; > >>> +vdev->lock = &node->vdev_lock; > >>> +vdev->v4l2_dev = &cam->v4l2_dev; > >>> +vdev->queue = &node->vbq; > >>> +vdev->vfl_dir = output ? VFL_DIR_TX : VFL_DIR_RX; > >>> +vdev->entity.function = MEDIA_ENT_F_IO_V4L; > >>> +vdev->entity.ops = NULL; > >>> +video_set_drvdata(vdev, cam); > >>> +dev_dbg(dev, "registered vdev:%d:%s\n", node->id, vdev->name); > >>> + > >>> +/* Initialize miscellaneous variables */ > >>> +mutex_init(&node->vdev_lock); > >>> +INIT_LIST_HEAD(&node->buf_list); > >>> +spin_lock_init(&node->buf_list_lock); > >>> + > >>> +ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1); > >>> +if (ret) { > >>> +dev_err(dev, "failed to register vde:%d\n", ret); > >>> +goto fail_vb2_rel; > >>> +} > >>> + > >>> +/* Create link between video node and the subdev pad */ > >>> +if (output) { > >>> +ret = media_create_pad_link(&vdev->entity, 0, > >>> + &cam->subdev.entity, > >>> + node->id, link_flags); > >>> +} else { > >>> +ret = media_create_pad_link(&cam->subdev.entity, > >>> + node->id, &vdev->entity, 0, > >>> + link_flags); > >>> +} > >> > >> No need for the curly braces. > >> > > > > Revised in next patch. > > > >>> +if (ret) > >>> +goto fail_vdev_ureg; > >>> + > >>> +return 0; > >>> + > >>> +fail_vdev_ureg: > >>> +video_unregister_device(vdev); > >>> +fail_vb2_rel: > >>> +mutex_destroy(&node->vdev_lock); > >>> +vb2_queue_release(vbq); > >>> +fail_media_clean: > >>> +media_entity_cleanup(&vdev->entity); > >>> + > >>> +return ret; > >>> +} > >>> + > >>> +static void > >>> +mtk_cam_video_unregister_device(struct mtk_cam_video_device *node) > >>> +{ > >>> +video_unregister_device(&node->vdev); > >>> +vb2_queue_release(&node->vbq); > >>> +media_entity_cleanup(&node->vdev.entity); > >>> +mutex_destroy(&node->vdev_lock); > >>> +} > >>> + > >>> +static int mtk_cam_v4l2_register(struct mtk_cam_dev *cam) > >>> +{ > >>> +struct device *dev = cam->dev; > >>> +int i, ret; > >>> + > >>> +/* Set up media device & pads */ > >>> +ret = mtk_cam_media_register(cam, &cam->media_dev); > >>> +if (ret) > >>> +return ret; > >>> +dev_info(dev, "Registered media%d\n", cam->media_dev.devnode->minor); > >>> + > >>> +/* Set up v4l2 device */ > >>> +cam->v4l2_dev.mdev = &cam->media_dev; > >>> +ret = v4l2_device_register(dev, &cam->v4l2_dev); > >>> +if (ret) { > >>> +dev_err(dev, "failed to register V4L2 device:%d\n", ret); > >>> +goto fail_media_unreg; > >>> +} > >>> +dev_info(dev, "Registered %s\n", cam->v4l2_dev.name); > >>> + > >>> +/* Initialize subdev */ > >>> +v4l2_subdev_init(&cam->subdev, &mtk_cam_subdev_ops); > >>> +cam->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER; > >>> +cam->subdev.entity.ops = &mtk_cam_media_entity_ops; > >>> +cam->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE | > >>> +V4L2_SUBDEV_FL_HAS_EVENTS; > >>> +snprintf(cam->subdev.name, sizeof(cam->subdev.name), > >>> + "%s", dev_driver_string(dev)); > >>> +v4l2_set_subdevdata(&cam->subdev, cam); > >>> + > >>> +ret = v4l2_device_register_subdev(&cam->v4l2_dev, &cam->subdev); > >>> +if (ret) { > >>> +dev_err(dev, "failed to initialize subdev:%d\n", ret); > >>> +goto fail_clean_media_entiy; > >>> +} > >>> +dev_dbg(dev, "registered %s\n", cam->subdev.name); > >>> + > >>> +/* Create video nodes and links */ > >>> +for (i = 0; i < MTK_CAM_P1_TOTAL_NODES; i++) { > >>> +struct mtk_cam_video_device *node = &cam->vdev_nodes[i]; > >>> + > >>> +node->id = node->desc.id; > >>> +ret = mtk_cam_video_register_device(cam, node); > >>> +if (ret) > >>> +goto fail_vdev_unreg; > >>> +} > >>> +vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); > >>> + > >>> +return 0; > >>> + > >>> +fail_vdev_unreg: > >>> +for (i--; i >= 0; i--) > >>> +mtk_cam_video_unregister_device(&cam->vdev_nodes[i]); > >>> +fail_clean_media_entiy: > >>> +media_entity_cleanup(&cam->subdev.entity); > >>> +v4l2_device_unregister(&cam->v4l2_dev); > >>> +fail_media_unreg: > >>> +media_device_unregister(&cam->media_dev); > >>> +media_device_cleanup(&cam->media_dev); > >>> + > >>> +return ret; > >>> +} > >>> + > >>> +static int mtk_cam_v4l2_unregister(struct mtk_cam_dev *cam) > >>> +{ > >>> +int i; > >>> + > >>> +for (i = 0; i < MTK_CAM_P1_TOTAL_NODES; i++) > >>> +mtk_cam_video_unregister_device(&cam->vdev_nodes[i]); > >>> + > >>> +vb2_dma_contig_clear_max_seg_size(cam->dev); > >>> +v4l2_device_unregister_subdev(&cam->subdev); > >>> +v4l2_device_unregister(&cam->v4l2_dev); > >>> +media_entity_cleanup(&cam->subdev.entity); > >>> +media_device_unregister(&cam->media_dev); > >>> +media_device_cleanup(&cam->media_dev); > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static int mtk_cam_dev_notifier_bound(struct v4l2_async_notifier *notifier, > >>> + struct v4l2_subdev *sd, > >>> + struct v4l2_async_subdev *asd) > >>> +{ > >>> +struct mtk_cam_dev *cam = > >>> +container_of(notifier, struct mtk_cam_dev, notifier); > >>> + > >>> +if (!(sd->entity.function & MEDIA_ENT_F_VID_IF_BRIDGE)) { > >>> +dev_dbg(cam->dev, "no MEDIA_ENT_F_VID_IF_BRIDGE function\n"); > >>> +return -ENODEV; > >>> +} > >>> + > >>> +cam->seninf = sd; > >>> +dev_dbg(cam->dev, "%s is bound\n", sd->entity.name); > >>> + > >>> +return 0; > >>> +} > >>> + > >>> +static void mtk_cam_dev_notifier_unbind(struct v4l2_async_notifier *notifier, > >>> +struct v4l2_subdev *sd, > >>> +struct v4l2_async_subdev *asd) > >>> +{ > >>> +struct mtk_cam_dev *cam = > >>> +container_of(notifier, struct mtk_cam_dev, notifier); > >>> + > >>> +cam->seninf = NULL; > >>> +dev_dbg(cam->dev, "%s is unbound\n", sd->entity.name); > >>> +} > >>> + > >>> +static int mtk_cam_dev_notifier_complete(struct v4l2_async_notifier *notifier) > >>> +{ > >>> +struct mtk_cam_dev *cam = > >>> +container_of(notifier, struct mtk_cam_dev, notifier); > >>> +struct device *dev = cam->dev; > >>> +int ret; > >>> + > >>> +if (!cam->seninf) { > >>> +dev_err(dev, "No seninf subdev\n"); > >>> +return -ENODEV; > >>> +} > >>> + > >>> +ret = media_create_pad_link(&cam->seninf->entity, MTK_CAM_CIO_PAD_SRC, > >>> + &cam->subdev.entity, MTK_CAM_CIO_PAD_SINK, > >>> + MEDIA_LNK_FL_IMMUTABLE | > >>> + MEDIA_LNK_FL_ENABLED); > >>> +if (ret) { > >>> +dev_err(dev, "failed to create pad link %s %s err:%d\n", > >>> +cam->seninf->entity.name, cam->subdev.entity.name, > >>> +ret); > >>> +return ret; > >>> +} > >>> + > >>> +ret = v4l2_device_register_subdev_nodes(&cam->v4l2_dev); > >>> +if (ret) { > >>> +dev_err(dev, "failed to initialize subdev nodes:%d\n", ret); > >>> +return ret; > >>> +} > >>> + > >>> +return ret; > >>> +} > >>> + > >>> +static const struct v4l2_async_notifier_operations mtk_cam_v4l2_async_ops = { > >>> +.bound = mtk_cam_dev_notifier_bound, > >>> +.unbind = mtk_cam_dev_notifier_unbind, > >>> +.complete = mtk_cam_dev_notifier_complete, > >>> +}; > >>> + > >>> +static int mtk_cam_v4l2_async_register(struct mtk_cam_dev *cam) > >>> +{ > >>> +struct device *dev = cam->dev; > >>> +int ret; > >>> + > >>> +v4l2_async_notifier_init(&cam->notifier); > >>> +ret = v4l2_async_notifier_parse_fwnode_endpoints(dev, > >>> +&cam->notifier, sizeof(struct v4l2_async_subdev), NULL); > >> > >> It seems we shouldn't be using this function, please see comments at https://patchwork.kernel.org/patch/11066527/ > >> > >> Regards, > >> Helen > >> > > > > Ok, we will investigate how to do. > > > >>> +if (ret) { > >>> +dev_err(dev, "failed to parse fwnode endpoints:%d\n", ret); > >>> +return ret; > >>> +} > >>> + > >>> +cam->notifier.ops = &mtk_cam_v4l2_async_ops; > >>> +dev_dbg(dev, "mtk_cam v4l2_async_notifier_register\n"); > >>> +ret = v4l2_async_notifier_register(&cam->v4l2_dev, &cam->notifier); > >>> +if (ret) { > >>> +dev_err(dev, "failed to register async notifier : %d\n", ret); > >>> +v4l2_async_notifier_cleanup(&cam->notifier); > >>> +} > >>> + > >>> +return ret; > >>> +} > >>> + > >>> +static void mtk_cam_v4l2_async_unregister(struct mtk_cam_dev *cam) > >>> +{ > >>> +v4l2_async_notifier_unregister(&cam->notifier); > >>> +v4l2_async_notifier_cleanup(&cam->notifier); > >>> +} > >>> + > >>> +static const struct v4l2_ioctl_ops mtk_cam_v4l2_vcap_ioctl_ops = { > >>> +.vidioc_querycap = mtk_cam_vidioc_querycap, > >>> +.vidioc_enum_framesizes = mtk_cam_vidioc_enum_framesizes, > >>> +.vidioc_enum_fmt_vid_cap = mtk_cam_vidioc_enum_fmt, > >>> +.vidioc_g_fmt_vid_cap_mplane = mtk_cam_vidioc_g_fmt, > >>> +.vidioc_s_fmt_vid_cap_mplane = mtk_cam_vidioc_s_fmt, > >>> +.vidioc_try_fmt_vid_cap_mplane = mtk_cam_vidioc_try_fmt, > >>> +.vidioc_reqbufs = vb2_ioctl_reqbufs, > >>> +.vidioc_create_bufs = vb2_ioctl_create_bufs, > >>> +.vidioc_prepare_buf = vb2_ioctl_prepare_buf, > >>> +.vidioc_querybuf = vb2_ioctl_querybuf, > >>> +.vidioc_qbuf = vb2_ioctl_qbuf, > >>> +.vidioc_dqbuf = vb2_ioctl_dqbuf, > >>> +.vidioc_streamon = vb2_ioctl_streamon, > >>> +.vidioc_streamoff = vb2_ioctl_streamoff, > >>> +.vidioc_expbuf = vb2_ioctl_expbuf, > >>> +.vidioc_subscribe_event = v4l2_ctrl_subscribe_event, > >>> +.vidioc_unsubscribe_event = v4l2_event_unsubscribe, > >>> +}; > >>> + > >>> +static const struct v4l2_ioctl_ops mtk_cam_v4l2_meta_cap_ioctl_ops = { > >>> +.vidioc_querycap = mtk_cam_vidioc_querycap, > >>> +.vidioc_enum_fmt_meta_cap = mtk_cam_vidioc_meta_enum_fmt, > >>> +.vidioc_g_fmt_meta_cap = mtk_cam_vidioc_g_meta_fmt, > >>> +.vidioc_s_fmt_meta_cap = mtk_cam_vidioc_g_meta_fmt, > >>> +.vidioc_try_fmt_meta_cap = mtk_cam_vidioc_g_meta_fmt, > >>> +.vidioc_reqbufs = vb2_ioctl_reqbufs, > >>> +.vidioc_create_bufs = vb2_ioctl_create_bufs, > >>> +.vidioc_prepare_buf = vb2_ioctl_prepare_buf, > >>> +.vidioc_querybuf = vb2_ioctl_querybuf, > >>> +.vidioc_qbuf = vb2_ioctl_qbuf, > >>> +.vidioc_dqbuf = vb2_ioctl_dqbuf, > >>> +.vidioc_streamon = vb2_ioctl_streamon, > >>> +.vidioc_streamoff = vb2_ioctl_streamoff, > >>> +.vidioc_expbuf = vb2_ioctl_expbuf, > >>> +}; > >>> + > >>> +static const struct v4l2_ioctl_ops mtk_cam_v4l2_meta_out_ioctl_ops = { > >>> +.vidioc_querycap = mtk_cam_vidioc_querycap, > >>> +.vidioc_enum_fmt_meta_out = mtk_cam_vidioc_meta_enum_fmt, > >>> +.vidioc_g_fmt_meta_out = mtk_cam_vidioc_g_meta_fmt, > >>> +.vidioc_s_fmt_meta_out = mtk_cam_vidioc_g_meta_fmt, > >>> +.vidioc_try_fmt_meta_out = mtk_cam_vidioc_g_meta_fmt, > >>> +.vidioc_reqbufs = vb2_ioctl_reqbufs, > >>> +.vidioc_create_bufs = vb2_ioctl_create_bufs, > >>> +.vidioc_prepare_buf = vb2_ioctl_prepare_buf, > >>> +.vidioc_querybuf = vb2_ioctl_querybuf, > >>> +.vidioc_qbuf = vb2_ioctl_qbuf, > >>> +.vidioc_dqbuf = vb2_ioctl_dqbuf, > >>> +.vidioc_streamon = vb2_ioctl_streamon, > >>> +.vidioc_streamoff = vb2_ioctl_streamoff, > >>> +.vidioc_expbuf = vb2_ioctl_expbuf, > >>> +};> + > >>> +static const struct v4l2_format meta_fmts[] = { > >>> +{ > >>> +.fmt.meta = { > >>> +.dataformat = V4L2_META_FMT_MTISP_PARAMS, > >>> +.buffersize = 512 * SZ_1K, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.meta = { > >>> +.dataformat = V4L2_META_FMT_MTISP_3A, > >>> +.buffersize = 1200 * SZ_1K, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.meta = { > >>> +.dataformat = V4L2_META_FMT_MTISP_AF, > >>> +.buffersize = 640 * SZ_1K, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.meta = { > >>> +.dataformat = V4L2_META_FMT_MTISP_LCS, > >>> +.buffersize = 288 * SZ_1K, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.meta = { > >>> +.dataformat = V4L2_META_FMT_MTISP_LMV, > >>> +.buffersize = 256, > >>> +}, > >>> +}, > >>> +}; > >>> + > >>> +static const struct v4l2_format stream_out_fmts[] = { > >>> +/* This is a default image format */ > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SBGGR10, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SBGGR8, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SBGGR12, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SBGGR14, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGBRG8, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGBRG10, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGBRG12, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGBRG14, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGRBG8, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGRBG10, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGRBG12, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGRBG14, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SRGGB8, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SRGGB10, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SRGGB12, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SRGGB14, > >>> +}, > >>> +}, > >>> +}; > >>> + > >>> +static const struct v4l2_format bin_out_fmts[] = { > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SBGGR8F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SBGGR10F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SBGGR12F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SBGGR14F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGBRG8F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGBRG10F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGBRG12F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGBRG14F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGRBG8F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGRBG10F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGRBG12F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SGRBG14F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SRGGB8F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SRGGB10F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SRGGB12F, > >>> +}, > >>> +}, > >>> +{ > >>> +.fmt.pix_mp = { > >>> +.width = IMG_MAX_WIDTH, > >>> +.height = IMG_MAX_HEIGHT, > >>> +.pixelformat = V4L2_PIX_FMT_MTISP_SRGGB14F, > >>> +}, > >>> +}, > >>> +}; > >>> + > >>> +static const struct > >>> +mtk_cam_dev_node_desc output_queues[] = { > >>> +{ > >>> +.id = MTK_CAM_P1_META_IN_0, > >>> +.name = "meta input", > >>> +.cap = V4L2_CAP_META_OUTPUT, > >>> +.buf_type = V4L2_BUF_TYPE_META_OUTPUT, > >>> +.link_flags = 0, > >>> +.image = false, > >>> +.smem_alloc = true, > >>> +.fmts = meta_fmts, > >>> +.default_fmt_idx = 0, > >>> +.max_buf_count = 10, > >>> +.ioctl_ops = &mtk_cam_v4l2_meta_out_ioctl_ops, > >>> +}, > >>> +}; > >>> + > >>> +static const struct > >>> +mtk_cam_dev_node_desc capture_queues[] = { > >>> +{ > >>> +.id = MTK_CAM_P1_MAIN_STREAM_OUT, > >>> +.name = "main stream", > >>> +.cap = V4L2_CAP_VIDEO_CAPTURE_MPLANE, > >>> +.buf_type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, > >>> +.link_flags = MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED, > >>> +.image = true, > >>> +.smem_alloc = false, > >>> +.dma_port = R_IMGO, > >>> +.fmts = stream_out_fmts, > >>> +.num_fmts = ARRAY_SIZE(stream_out_fmts), > >>> +.default_fmt_idx = 0, > >>> +.ioctl_ops = &mtk_cam_v4l2_vcap_ioctl_ops, > >>> +.frmsizes = &(struct v4l2_frmsizeenum) { > >>> +.index = 0, > >>> +.type = V4L2_FRMSIZE_TYPE_CONTINUOUS, > >>> +.stepwise = { > >>> +.max_width = IMG_MAX_WIDTH, > >>> +.min_width = IMG_MIN_WIDTH, > >>> +.max_height = IMG_MAX_HEIGHT, > >>> +.min_height = IMG_MIN_HEIGHT, > >>> +.step_height = 1, > >>> +.step_width = 1, > >>> +}, > >>> +}, > >>> +}, > >>> +{ > >>> +.id = MTK_CAM_P1_PACKED_BIN_OUT, > >>> +.name = "packed out", > >>> +.cap = V4L2_CAP_VIDEO_CAPTURE_MPLANE, > >>> +.buf_type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, > >>> +.link_flags = 0, > >>> +.image = true, > >>> +.smem_alloc = false, > >>> +.dma_port = R_RRZO, > >>> +.fmts = bin_out_fmts, > >>> +.num_fmts = ARRAY_SIZE(bin_out_fmts), > >>> +.default_fmt_idx = 0, > >>> +.ioctl_ops = &mtk_cam_v4l2_vcap_ioctl_ops, > >>> +.frmsizes = &(struct v4l2_frmsizeenum) { > >>> +.index = 0, > >>> +.type = V4L2_FRMSIZE_TYPE_CONTINUOUS, > >>> +.stepwise = { > >>> +.max_width = IMG_MAX_WIDTH, > >>> +.min_width = IMG_MIN_WIDTH, > >>> +.max_height = IMG_MAX_HEIGHT, > >>> +.min_height = IMG_MIN_HEIGHT, > >>> +.step_height = 1, > >>> +.step_width = 1, > >>> +}, > >>> +}, > >>> +}, > >>> +{ > >>> +.id = MTK_CAM_P1_META_OUT_0, > >>> +.name = "partial meta 0", > >>> +.cap = V4L2_CAP_META_CAPTURE, > >>> +.buf_type = V4L2_BUF_TYPE_META_CAPTURE, > >>> +.link_flags = 0, > >>> +.image = false, > >>> +.smem_alloc = false, > >>> +.dma_port = R_AAO | R_FLKO | R_PSO, > >>> +.fmts = meta_fmts, > >>> +.default_fmt_idx = 1, > >>> +.max_buf_count = 5, > >>> +.ioctl_ops = &mtk_cam_v4l2_meta_cap_ioctl_ops, > >>> +}, > >>> +{ > >>> +.id = MTK_CAM_P1_META_OUT_1, > >>> +.name = "partial meta 1", > >>> +.cap = V4L2_CAP_META_CAPTURE, > >>> +.buf_type = V4L2_BUF_TYPE_META_CAPTURE, > >>> +.link_flags = 0, > >>> +.image = false, > >>> +.smem_alloc = false, > >>> +.dma_port = R_AFO, > >>> +.fmts = meta_fmts, > >>> +.default_fmt_idx = 2, > >>> +.max_buf_count = 5, > >>> +.ioctl_ops = &mtk_cam_v4l2_meta_cap_ioctl_ops, > >>> +}, > >>> +{ > >>> +.id = MTK_CAM_P1_META_OUT_2, > >>> +.name = "partial meta 2", > >>> +.cap = V4L2_CAP_META_CAPTURE, > >>> +.buf_type = V4L2_BUF_TYPE_META_CAPTURE, > >>> +.link_flags = 0, > >>> +.image = false, > >>> +.smem_alloc = false, > >>> +.dma_port = R_LCSO, > >>> +.fmts = meta_fmts, > >>> +.default_fmt_idx = 3, > >>> +.max_buf_count = 10, > >>> +.ioctl_ops = &mtk_cam_v4l2_meta_cap_ioctl_ops, > >>> +}, > >>> +{ > >>> +.id = MTK_CAM_P1_META_OUT_3, > >>> +.name = "partial meta 3", > >>> +.cap = V4L2_CAP_META_CAPTURE, > >>> +.buf_type = V4L2_BUF_TYPE_META_CAPTURE, > >>> +.link_flags = 0, > >>> +.image = false, > >>> +.smem_alloc = false, > >>> +.dma_port = R_LMVO, > >>> +.fmts = meta_fmts, > >>> +.default_fmt_idx = 4, > >>> +.max_buf_count = 10, > >>> +.ioctl_ops = &mtk_cam_v4l2_meta_cap_ioctl_ops, > >>> +}, > >>> +}; > >>> + > >>> +/* The helper to configure the device context */ > >>> +static void mtk_cam_dev_queue_setup(struct mtk_cam_dev *cam) > >>> +{ > >>> +unsigned int node_idx; > >>> +int i; > >>> + > >>> +node_idx = 0; > >>> +/* Setup the output queue */ > >>> +for (i = 0; i < ARRAY_SIZE(output_queues); i++) > >>> +cam->vdev_nodes[node_idx++].desc = output_queues[i]; > >>> + > >>> +/* Setup the capture queue */ > >>> +for (i = 0; i < ARRAY_SIZE(capture_queues); i++) > >>> +cam->vdev_nodes[node_idx++].desc = capture_queues[i]; > >>> +} > >>> + > >>> +int mtk_cam_dev_init(struct platform_device *pdev, > >>> + struct mtk_cam_dev *cam) > >>> +{ > >>> +int ret; > >>> + > >>> +cam->dev = &pdev->dev; > >>> +mtk_cam_dev_queue_setup(cam); > >>> + > >>> +spin_lock_init(&cam->pending_job_lock); > >>> +spin_lock_init(&cam->running_job_lock); > >>> +INIT_LIST_HEAD(&cam->pending_job_list); > >>> +INIT_LIST_HEAD(&cam->running_job_list); > >>> +mutex_init(&cam->op_lock); > >>> + > >>> +/* v4l2 sub-device registration */ > >>> +ret = mtk_cam_v4l2_register(cam); > >>> +if (ret) > >>> +return ret; > >>> + > >>> +ret = mtk_cam_v4l2_async_register(cam); > >>> +if (ret) > >>> +goto fail_v4l2_unreg; > >>> + > >>> +return 0; > >>> + > >>> +fail_v4l2_unreg: > >>> +mutex_destroy(&cam->op_lock); > >>> +mtk_cam_v4l2_unregister(cam); > >>> + > >>> +return ret; > >>> +} > >>> + > >>> +void mtk_cam_dev_cleanup(struct mtk_cam_dev *cam) > >>> +{ > >>> +mtk_cam_v4l2_async_unregister(cam); > >>> +mtk_cam_v4l2_unregister(cam); > >>> +mutex_destroy(&cam->op_lock); > >>> +} > >>> + > >>> diff --git a/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.h b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.h > >>> new file mode 100644 > >>> index 000000000000..0a340a1e65ea > >>> --- /dev/null > >>> +++ b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.h > >>> @@ -0,0 +1,244 @@ > >>> +/* SPDX-License-Identifier: GPL-2.0 */ > >>> +/* > >>> + * Copyright (c) 2019 MediaTek Inc. > >>> + */ > >>> + > >>> +#ifndef __MTK_CAM_H__ > >>> +#define __MTK_CAM_H__ > >>> + > >>> +#include <linux/device.h> > >>> +#include <linux/types.h> > >>> +#include <linux/platform_device.h> > >>> +#include <linux/spinlock.h> > >>> +#include <linux/videodev2.h> > >>> +#include <media/v4l2-device.h> > >>> +#include <media/v4l2-ctrls.h> > >>> +#include <media/v4l2-subdev.h> > >>> +#include <media/videobuf2-core.h> > >>> +#include <media/videobuf2-v4l2.h> > >>> + > >>> +#include "mtk_cam-ipi.h" > >>> + > >>> +#define IMG_MAX_WIDTH5376 > >>> +#define IMG_MAX_HEIGHT4032 > >>> +#define IMG_MIN_WIDTH80 > >>> +#define IMG_MIN_HEIGHT60 > >>> + > >>> +/* > >>> + * ID enum value for struct mtk_cam_dev_node_desc:id > >>> + * or mtk_cam_video_device:id > >>> + */ > >>> +enum { > >>> +MTK_CAM_P1_META_IN_0 = 0, > >>> +MTK_CAM_P1_MAIN_STREAM_OUT, > >>> +MTK_CAM_P1_PACKED_BIN_OUT, > >>> +MTK_CAM_P1_META_OUT_0, > >>> +MTK_CAM_P1_META_OUT_1, > >>> +MTK_CAM_P1_META_OUT_2, > >>> +MTK_CAM_P1_META_OUT_3, > >>> +MTK_CAM_P1_TOTAL_NODES > >>> +}; > >>> + > >>> +/* Supported image format list */ > >>> +#define MTK_CAM_IMG_FMT_UNKNOWN0x0000 > >>> +#define MTK_CAM_IMG_FMT_BAYER80x2200 > >>> +#define MTK_CAM_IMG_FMT_BAYER100x2201 > >>> +#define MTK_CAM_IMG_FMT_BAYER120x2202 > >>> +#define MTK_CAM_IMG_FMT_BAYER140x2203 > >>> +#define MTK_CAM_IMG_FMT_FG_BAYER80x2204 > >>> +#define MTK_CAM_IMG_FMT_FG_BAYER100x2205 > >>> +#define MTK_CAM_IMG_FMT_FG_BAYER120x2206 > >>> +#define MTK_CAM_IMG_FMT_FG_BAYER140x2207 > >>> + > >>> +/* Supported bayer pixel order */ > >>> +#define MTK_CAM_RAW_PXL_ID_B0 > >>> +#define MTK_CAM_RAW_PXL_ID_GB1 > >>> +#define MTK_CAM_RAW_PXL_ID_GR2 > >>> +#define MTK_CAM_RAW_PXL_ID_R3 > >>> +#define MTK_CAM_RAW_PXL_ID_UNKNOWN4 > >>> + > >>> +/* > >>> + * struct mtk_p1_frame_param - MTK ISP P1 driver frame parameters. > >>> + * > >>> + * @frame_seq_no: The frame sequence of frame in driver layer. > >>> + * @dma_bufs: The DMA buffer address information of enabled DMA nodes. > >>> + * > >>> + */ > >>> +struct mtk_p1_frame_param { > >>> +unsigned int frame_seq_no; > >>> +struct dma_buffer dma_bufs[MTK_CAM_P1_TOTAL_NODES]; > >>> +} __packed; > >>> + > >>> +/* > >>> + * struct mtk_cam_dev_request - MTK camera device request. > >>> + * > >>> + * @req: Embedded struct media request. > >>> + * @frame_params: The frame info. & address info. of enabled DMA nodes. > >>> + * @frame_work: work queue entry for frame transmission to SCP. > >>> + * @list: List entry of the object for @struct mtk_cam_dev: > >>> + * pending_job_list or running_job_list. > >>> + * @timestamp: Start of frame timestamp in ns > >>> + * > >>> + */ > >>> +struct mtk_cam_dev_request { > >>> +struct media_request req; > >>> +struct mtk_p1_frame_param frame_params; > >>> +struct work_struct frame_work; > >>> +struct list_head list; > >>> +u64 timestamp; > >>> +}; > >>> + > >>> +/* > >>> + * struct mtk_cam_dev_buffer - MTK camera device buffer. > >>> + * > >>> + * @vbb: Embedded struct vb2_v4l2_buffer. > >>> + * @list: List entry of the object for @struct mtk_cam_video_device: > >>> + * buf_list. > >>> + * @daddr: The DMA address of this buffer. > >>> + * @scp_addr: The SCP address of this buffer which > >>> + * is only supported for meta input node. > >>> + * @node_id: The vidoe node id which this buffer belongs to. > >>> + * > >>> + */ > >>> +struct mtk_cam_dev_buffer { > >>> +struct vb2_v4l2_buffer vbb; > >>> +struct list_head list; > >>> +/* Intenal part */ > >>> +dma_addr_t daddr; > >>> +dma_addr_t scp_addr; > >>> +unsigned int node_id; > >>> +}; > >>> + > >>> +/* > >>> + * struct mtk_cam_dev_node_desc - MTK camera device node descriptor > >>> + * > >>> + * @id: id of the node > >>> + * @name: name of the node > >>> + * @cap: supported V4L2 capabilities > >>> + * @buf_type: supported V4L2 buffer type > >>> + * @dma_port: the dma ports associated to the node > >>> + * @link_flags: default media link flags > >>> + * @smem_alloc: using the smem_dev as alloc device or not > >>> + * @image: true for image node, false for meta node > >>> + * @num_fmts: the number of supported node formats > >>> + * @default_fmt_idx: default format of this node > >>> + * @max_buf_count: maximum VB2 buffer count > >>> + * @ioctl_ops: mapped to v4l2_ioctl_ops > >>> + * @fmts: supported format > >>> + * @frmsizes: supported V4L2 frame size number > >>> + * > >>> + */ > >>> +struct mtk_cam_dev_node_desc { > >>> +u8 id; > >>> +const char *name; > >>> +u32 cap; > >>> +u32 buf_type; > >>> +u32 dma_port; > >>> +u32 link_flags; > >>> +u8 smem_alloc:1; > >>> +u8 image:1; > >>> +u8 num_fmts; > >>> +u8 default_fmt_idx; > >>> +u8 max_buf_count; > >>> +const struct v4l2_ioctl_ops *ioctl_ops; > >>> +const struct v4l2_format *fmts; > >>> +const struct v4l2_frmsizeenum *frmsizes; > >>> +}; > >>> + > >>> +/* > >>> + * struct mtk_cam_video_device - Mediatek video device structure > >>> + * > >>> + * @id: Id for index of mtk_cam_dev:vdev_nodes array > >>> + * @enabled: Indicate the video device is enabled or not > >>> + * @desc: The node description of video device > >>> + * @vdev_fmt: The V4L2 format of video device > >>> + * @vdev_pad: The media pad graph object of video device > >>> + * @vbq: A videobuf queue of video device > >>> + * @vdev: The video device instance > >>> + * @vdev_lock: Serializes vb2 queue and video device operations > >>> + * @buf_list: List for enqueue buffers > >>> + * @buf_list_lock: Lock used to protect buffer list. > >>> + * > >>> + */ > >>> +struct mtk_cam_video_device { > >>> +unsigned int id; > >>> +unsigned int enabled; > >>> +struct mtk_cam_dev_node_desc desc; > >>> +struct v4l2_format vdev_fmt; > >>> +struct media_pad vdev_pad; > >>> +struct vb2_queue vbq; > >>> +struct video_device vdev; > >>> +/* Serializes vb2 queue and video device operations */ > >>> +struct mutex vdev_lock; > >>> +struct list_head buf_list; > >>> +/* Lock used to protect buffer list */ > >>> +spinlock_t buf_list_lock; > >>> +}; > >>> + > >>> +/* > >>> + * struct mtk_cam_dev - Mediatek camera device structure. > >>> + * > >>> + * @dev: Pointer to device. > >>> + * @smem_pdev: Pointer to shared memory device. > >>> + * @pipeline: Media pipeline information. > >>> + * @media_dev: Media device instance. > >>> + * @subdev: The V4L2 sub-device instance. > >>> + * @v4l2_dev: The V4L2 device driver instance. > >>> + * @notifier: The v4l2_device notifier data. > >>> + * @subdev_pads: Pointer to the number of media pads of this sub-device. > >>> + * @vdev_nodes: The array list of mtk_cam_video_device nodes. > >>> + * @seninf: Pointer to the seninf sub-device. > >>> + * @sensor: Pointer to the active sensor V4L2 sub-device when streaming on. > >>> + * @streaming: Indicate the overall streaming status is on or off. > >>> + * @enabled_dmas: The enabled dma port information when streaming on. > >>> + * @enabled_count: Number of enabled video nodes > >>> + * @stream_count: Number of streaming video nodes > >>> + * @running_job_count: Nunber of running jobs in the HW driver. > >>> + * @pending_job_list: List to keep the media requests before en-queue into > >>> + * HW driver. > >>> + * @pending_job_lock: Protect the pending_job_list data & running_job_count. > >>> + * @running_job_list: List to keep the media requests after en-queue into > >>> + * HW driver. > >>> + * @running_job_lock: Protect the running_job_list data. > >>> + * @op_lock: Serializes driver's VB2 callback operations. > >>> + * > >>> + */ > >>> +struct mtk_cam_dev { > >>> +struct device *dev; > >>> +struct device *smem_dev; > >>> +struct media_pipeline pipeline; > >>> +struct media_device media_dev; > >>> +struct v4l2_subdev subdev; > >>> +struct v4l2_device v4l2_dev; > >>> +struct v4l2_async_notifier notifier; > >>> +struct media_pad *subdev_pads; > >>> +struct mtk_cam_video_device vdev_nodes[MTK_CAM_P1_TOTAL_NODES]; > >>> +struct v4l2_subdev *seninf; > >>> +struct v4l2_subdev *sensor; > >>> +unsigned int streaming; > >>> +unsigned int enabled_dmas; > >>> +unsigned int enabled_count; > >>> +unsigned int stream_count; > >>> +unsigned int running_job_count; > >>> +struct list_head pending_job_list; > >>> +/* Protect the pending_job_list data */ > >>> +spinlock_t pending_job_lock; > >>> +struct list_head running_job_list; > >>> +/* Protect the running_job_list data & running_job_count */ > >>> +spinlock_t running_job_lock; > >>> +/* Serializes driver's VB2 callback operations */ > >>> +struct mutex op_lock; > >>> +}; > >>> + > >>> +int mtk_cam_dev_init(struct platform_device *pdev, > >>> + struct mtk_cam_dev *cam_dev); > >>> +void mtk_cam_dev_cleanup(struct mtk_cam_dev *cam_dev); > >>> +void mtk_cam_dev_req_try_queue(struct mtk_cam_dev *cam_dev); > >>> +void mtk_cam_dev_dequeue_req_frame(struct mtk_cam_dev *cam_dev, > >>> + unsigned int frame_seq_no); > >>> +void mtk_cam_dev_event_frame_sync(struct mtk_cam_dev *cam_dev, > >>> + unsigned int frame_seq_no); > >>> +struct mtk_cam_dev_request *mtk_cam_dev_get_req(struct mtk_cam_dev *cam, > >>> +unsigned int frame_seq_no); > >>> + > >>> +#endif /* __MTK_CAM_H__ */ > >>> > >> > >> _______________________________________________ > >> Linux-mediatek mailing list > >> Linux-mediatek@xxxxxxxxxxxxxxxxxxx > >> http://lists.infradead.org/mailman/listinfo/linux-mediatek > > > > Regards, > Helen Thanks for your comment. Best regards, Jungo
