Re: [PATCH v4 2/2] media: platform: Add Aspeed Video Engine driver

[Eddie James]

On 10/19/2018 03:26 PM, Eddie James wrote:
> On 10/12/2018 07:22 AM, Hans Verkuil wrote:
> > On 10/05/2018 09:57 PM, Eddie James wrote:
> > > The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
> > > can capture and compress video data from digital or analog sources. 
> > > With
> > > the Aspeed chip acting a service processor, the Video Engine can 
> > > capture
> > > the host processor graphics output.
> > >
> > > Add a V4L2 driver to capture video data and compress it to JPEG images.
> > > Make the video frames available through the V4L2 streaming interface.
> > >
> > > Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx>
> > > ---
> > > Â MAINTAINERSÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂ 8 +
> > > Â drivers/media/platform/KconfigÂÂÂÂÂÂÂ |ÂÂÂ 8 +
> > > Â drivers/media/platform/MakefileÂÂÂÂÂÂ |ÂÂÂ 1 +
> > > Â drivers/media/platform/aspeed-video.c | 1674 
> > > +++++++++++++++++++++++++++++++++
> > > Â 4 files changed, 1691 insertions(+)
> > > Â create mode 100644 drivers/media/platform/aspeed-video.c
> > <snip>
> >
> > > +static int aspeed_video_enum_input(struct file *file, void *fh,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct v4l2_input *inp)
> > > +{
> > > +ÂÂÂ if (inp->index)
> > > +ÂÂÂÂÂÂÂ return -EINVAL;
> > > +
> > > +ÂÂÂ strscpy(inp->name, "Host VGA capture", sizeof(inp->name));
> > > +ÂÂÂ inp->type = V4L2_INPUT_TYPE_CAMERA;
> > > +ÂÂÂ inp->capabilities = V4L2_IN_CAP_DV_TIMINGS;
> > > +ÂÂÂ inp->status = V4L2_IN_ST_NO_SIGNAL | V4L2_IN_ST_NO_SYNC;
> > This can't be right. If there is a valid signal, then status should 
> > be 0.
> > And ideally you can tell the difference between no signal and no sync
> > as well.
> >
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_get_input(struct file *file, void *fh, 
> > > unsigned int *i)
> > > +{
> > > +ÂÂÂ *i = 0;
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_set_input(struct file *file, void *fh, 
> > > unsigned int i)
> > > +{
> > > +ÂÂÂ if (i)
> > > +ÂÂÂÂÂÂÂ return -EINVAL;
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_get_parm(struct file *file, void *fh,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct v4l2_streamparm *a)
> > > +{
> > > +ÂÂÂ struct aspeed_video *video = video_drvdata(file);
> > > +
> > > +ÂÂÂ a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> > > +ÂÂÂ a->parm.capture.readbuffers = 3;
> > > +ÂÂÂ a->parm.capture.timeperframe.numerator = 1;
> > > +ÂÂÂ if (!video->frame_rate)
> > > +ÂÂÂÂÂÂÂ a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1;
> > > +ÂÂÂ else
> > > +ÂÂÂÂÂÂÂ a->parm.capture.timeperframe.denominator = video->frame_rate;
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_set_parm(struct file *file, void *fh,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct v4l2_streamparm *a)
> > > +{
> > > +ÂÂÂ unsigned int frame_rate = 0;
> > > +ÂÂÂ struct aspeed_video *video = video_drvdata(file);
> > > +
> > > +ÂÂÂ a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> > > +ÂÂÂ a->parm.capture.readbuffers = 3;
> > > +
> > > +ÂÂÂ if (a->parm.capture.timeperframe.numerator)
> > > +ÂÂÂÂÂÂÂ frame_rate = a->parm.capture.timeperframe.denominator /
> > > +ÂÂÂÂÂÂÂÂÂÂÂ a->parm.capture.timeperframe.numerator;
> > > +
> > > +ÂÂÂ if (!frame_rate || frame_rate > MAX_FRAME_RATE) {
> > > +ÂÂÂÂÂÂÂ frame_rate = 0;
> > > +
> > > +ÂÂÂÂÂÂÂ /*
> > > +ÂÂÂÂÂÂÂÂ * Set to max + 1 to differentiate between max and 0, which
> > > +ÂÂÂÂÂÂÂÂ * means "don't care".
> > But what does "don't care" mean in practice? It's still not clear to 
> > me how this
> > is supposed to work.
> >
> > > +ÂÂÂÂÂÂÂÂ */
> > > +ÂÂÂÂÂÂÂ a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1;
> > And regardless of anything else this timeperframe is out of the range 
> > that
> > aspeed_video_enum_frameintervals() returns.
> >
> > > + a->parm.capture.timeperframe.numerator = 1;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ if (video->frame_rate != frame_rate) {
> > > +ÂÂÂÂÂÂÂ video->frame_rate = frame_rate;
> > > +ÂÂÂÂÂÂÂ aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ FIELD_PREP(VE_CTRL_FRC, frame_rate));
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_enum_framesizes(struct file *file, void *fh,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct v4l2_frmsizeenum *fsize)
> > > +{
> > > +ÂÂÂ struct aspeed_video *video = video_drvdata(file);
> > > +
> > > +ÂÂÂ if (fsize->index)
> > > +ÂÂÂÂÂÂÂ return -EINVAL;
> > > +
> > > +ÂÂÂ if (fsize->pixel_format != V4L2_PIX_FMT_JPEG)
> > > +ÂÂÂÂÂÂÂ return -EINVAL;
> > > +
> > > +ÂÂÂ fsize->discrete.width = video->pix_fmt.width;
> > > +ÂÂÂ fsize->discrete.height = video->pix_fmt.height;
> > > +ÂÂÂ fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_enum_frameintervals(struct file *file, void 
> > > *fh,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct v4l2_frmivalenum *fival)
> > > +{
> > > +ÂÂÂ struct aspeed_video *video = video_drvdata(file);
> > > +
> > > +ÂÂÂ if (fival->index)
> > > +ÂÂÂÂÂÂÂ return -EINVAL;
> > > +
> > > +ÂÂÂ if (fival->width != video->width || fival->height != 
> > > video->height)
> > > +ÂÂÂÂÂÂÂ return -EINVAL;
> > > +
> > > +ÂÂÂ if (fival->pixel_format != V4L2_PIX_FMT_JPEG)
> > > +ÂÂÂÂÂÂÂ return -EINVAL;
> > > +
> > > +ÂÂÂ fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
> > > +
> > > +ÂÂÂ fival->stepwise.min.denominator = MAX_FRAME_RATE;
> > > +ÂÂÂ fival->stepwise.min.numerator = 1;
> > > +ÂÂÂ fival->stepwise.max.denominator = 1;
> > > +ÂÂÂ fival->stepwise.max.numerator = 1;
> > > +ÂÂÂ fival->stepwise.step = fival->stepwise.max;
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_set_dv_timings(struct file *file, void *fh,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct v4l2_dv_timings *timings)
> > > +{
> > > +ÂÂÂ struct aspeed_video *video = video_drvdata(file);
> > > +
> > If vb2_is_busy() returns true, then return -EBUSY here. It is not 
> > allowed to
> > set the timings while vb2 is busy.
>
> Buffer ioctls (Input 0):
> ÂÂÂ ÂÂÂ fail: v4l2-test-buffers.cpp(344): node->s_dv_timings(timings)
> ÂÂÂ ÂÂÂ fail: v4l2-test-buffers.cpp(452): testCanSetSameTimings(node)
> ÂÂÂ test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
>
> Streaming ioctls:
> ÂÂÂ test read/write: OK
> ÂÂÂ test blocking wait: OK
> ÂÂÂ ÂÂÂ fail: v4l2-test-buffers.cpp(344): node->s_dv_timings(timings)
> ÂÂÂ ÂÂÂ fail: v4l2-test-buffers.cpp(637): testCanSetSameTimings(node)
> ÂÂÂ ÂÂÂ fail: v4l2-test-buffers.cpp(952): captureBufs(node, q, m2m_q, 
> frame_count, false)
> ÂÂÂ test MMAP: FAIL
>
> Built from v4l-utils c36dbbdfa8b30b2badd4f893b59d0bd4f0bd12aa
>
> Adding this causes some of the v4l2-compliance streaming tests to 
> fail, and prevents my own application from being able to call 
> S_DV_TIMINGS after detecting a resolution change, despite calling 
> streamoff and unmapping all the buffers first.

Any thoughts on this Hans?

Thanks,
Eddie

> Thanks,
> Eddie
>
> > > +ÂÂÂ if (video->width != timings->bt.width ||
> > > +ÂÂÂÂÂÂÂ video->height != timings->bt.height)
> > > +ÂÂÂÂÂÂÂ return -EINVAL;
> > > +
> > > +ÂÂÂ video->pix_fmt.width = timings->bt.width;
> > > +ÂÂÂ video->pix_fmt.height = timings->bt.height;
> > > +ÂÂÂ video->pix_fmt.sizeimage = video->max_compressed_size;
> > > +ÂÂÂ video->timings.width = timings->bt.width;
> > > +ÂÂÂ video->timings.height = timings->bt.height;
> > > +
> > > +ÂÂÂ timings->type = V4L2_DV_BT_656_1120;
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_get_dv_timings(struct file *file, void *fh,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct v4l2_dv_timings *timings)
> > > +{
> > > +ÂÂÂ struct aspeed_video *video = video_drvdata(file);
> > > +
> > > +ÂÂÂ timings->type = V4L2_DV_BT_656_1120;
> > > +ÂÂÂ timings->bt = video->timings;
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_query_dv_timings(struct file *file, void *fh,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct v4l2_dv_timings *timings)
> > > +{
> > > +ÂÂÂ int rc;
> > > +ÂÂÂ struct aspeed_video *video = video_drvdata(file);
> > > +
> > > +ÂÂÂ if (file->f_flags & O_NONBLOCK) {
> > > +ÂÂÂÂÂÂÂ if (test_bit(VIDEO_RES_CHANGE, &video->flags))
> > > +ÂÂÂÂÂÂÂÂÂÂÂ return -EAGAIN;
> > > +ÂÂÂ } else {
> > > +ÂÂÂÂÂÂÂ rc = wait_event_interruptible(video->wait,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ !test_bit(VIDEO_RES_CHANGE,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &video->flags));
> > > +ÂÂÂÂÂÂÂ if (rc)
> > > +ÂÂÂÂÂÂÂÂÂÂÂ return -EINTR;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ timings->type = V4L2_DV_BT_656_1120;
> > > +ÂÂÂ timings->bt = video->timings;
> > > +ÂÂÂ timings->bt.width = video->width;
> > > +ÂÂÂ timings->bt.height = video->height;
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_enum_dv_timings(struct file *file, void *fh,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct v4l2_enum_dv_timings *timings)
> > > +{
> > > +ÂÂÂ if (timings->index)
> > > +ÂÂÂÂÂÂÂ return -EINVAL;
> > > +
> > > +ÂÂÂ return aspeed_video_get_dv_timings(file, fh, &timings->timings);
> > > +}
> > > +
> > > +static int aspeed_video_dv_timings_cap(struct file *file, void *fh,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct v4l2_dv_timings_cap *cap)
> > > +{
> > > +ÂÂÂ struct aspeed_video *video = video_drvdata(file);
> > > +
> > > +ÂÂÂ cap->type = V4L2_DV_BT_656_1120;
> > > +ÂÂÂ cap->bt.capabilities = V4L2_DV_BT_CAP_PROGRESSIVE;
> > > +ÂÂÂ cap->bt.min_width = video->width;
> > > +ÂÂÂ cap->bt.max_width = video->width;
> > > +ÂÂÂ cap->bt.min_height = video->height;
> > > +ÂÂÂ cap->bt.max_height = video->height;
> > This should return the capabilities of the aspeed. In this case I'd
> > guess that the max width/height is 1920x1080 (or perhaps 1200).
> >
> > The minimum is probably the VGA resolution.
> >
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_sub_event(struct v4l2_fh *fh,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ const struct v4l2_event_subscription *sub)
> > > +{
> > > +ÂÂÂ switch (sub->type) {
> > > +ÂÂÂ case V4L2_EVENT_SOURCE_CHANGE:
> > > +ÂÂÂÂÂÂÂ return v4l2_src_change_event_subscribe(fh, sub);
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ return v4l2_ctrl_subscribe_event(fh, sub);
> > > +}
> > > +
> > > +static const struct v4l2_ioctl_ops aspeed_video_ioctl_ops = {
> > > +ÂÂÂ .vidioc_querycap = aspeed_video_querycap,
> > > +
> > > +ÂÂÂ .vidioc_enum_fmt_vid_cap = aspeed_video_enum_format,
> > > +ÂÂÂ .vidioc_g_fmt_vid_cap = aspeed_video_get_format,
> > > +ÂÂÂ .vidioc_s_fmt_vid_cap = aspeed_video_get_format,
> > > +ÂÂÂ .vidioc_try_fmt_vid_cap = aspeed_video_get_format,
> > > +
> > > +ÂÂÂ .vidioc_reqbufs = vb2_ioctl_reqbufs,
> > > +ÂÂÂ .vidioc_querybuf = vb2_ioctl_querybuf,
> > > +ÂÂÂ .vidioc_qbuf = vb2_ioctl_qbuf,
> > > +ÂÂÂ .vidioc_dqbuf = vb2_ioctl_dqbuf,
> > > +ÂÂÂ .vidioc_create_bufs = vb2_ioctl_create_bufs,
> > > +ÂÂÂ .vidioc_prepare_buf = vb2_ioctl_prepare_buf,
> > > +ÂÂÂ .vidioc_streamon = vb2_ioctl_streamon,
> > > +ÂÂÂ .vidioc_streamoff = vb2_ioctl_streamoff,
> > > +
> > > +ÂÂÂ .vidioc_enum_input = aspeed_video_enum_input,
> > > +ÂÂÂ .vidioc_g_input = aspeed_video_get_input,
> > > +ÂÂÂ .vidioc_s_input = aspeed_video_set_input,
> > > +
> > > +ÂÂÂ .vidioc_g_parm = aspeed_video_get_parm,
> > > +ÂÂÂ .vidioc_s_parm = aspeed_video_set_parm,
> > > +ÂÂÂ .vidioc_enum_framesizes = aspeed_video_enum_framesizes,
> > > +ÂÂÂ .vidioc_enum_frameintervals = aspeed_video_enum_frameintervals,
> > > +
> > > +ÂÂÂ .vidioc_s_dv_timings = aspeed_video_set_dv_timings,
> > > +ÂÂÂ .vidioc_g_dv_timings = aspeed_video_get_dv_timings,
> > > +ÂÂÂ .vidioc_query_dv_timings = aspeed_video_query_dv_timings,
> > > +ÂÂÂ .vidioc_enum_dv_timings = aspeed_video_enum_dv_timings,
> > > +ÂÂÂ .vidioc_dv_timings_cap = aspeed_video_dv_timings_cap,
> > > +
> > > +ÂÂÂ .vidioc_subscribe_event = aspeed_video_sub_event,
> > > +ÂÂÂ .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> > > +};
> > > +
> > > +static void aspeed_video_update_jpeg_quality(struct aspeed_video 
> > > *video)
> > > +{
> > > +ÂÂÂ u32 comp_ctrl = FIELD_PREP(VE_COMP_CTRL_DCT_LUM, 
> > > video->jpeg_quality) |
> > > +ÂÂÂÂÂÂÂ FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10);
> > > +
> > > +ÂÂÂ aspeed_video_update(video, VE_COMP_CTRL,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ comp_ctrl);
> > > +}
> > > +
> > > +static void aspeed_video_update_subsampling(struct aspeed_video 
> > > *video)
> > > +{
> > > +ÂÂÂ if (video->jpeg.virt)
> > > +ÂÂÂÂÂÂÂ aspeed_video_init_jpeg_table(video->jpeg.virt, video->yuv420);
> > > +
> > > +ÂÂÂ if (video->yuv420)
> > > +ÂÂÂÂÂÂÂ aspeed_video_update(video, VE_SEQ_CTRL, 0, 
> > > VE_SEQ_CTRL_YUV420);
> > > +ÂÂÂ else
> > > +ÂÂÂÂÂÂÂ aspeed_video_update(video, VE_SEQ_CTRL, VE_SEQ_CTRL_YUV420, 
> > > 0);
> > > +}
> > > +
> > > +static int aspeed_video_set_ctrl(struct v4l2_ctrl *ctrl)
> > > +{
> > > +ÂÂÂ struct aspeed_video *video = container_of(ctrl->handler,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct aspeed_video,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ctrl_handler);
> > > +
> > > +ÂÂÂ switch (ctrl->id) {
> > > +ÂÂÂ case V4L2_CID_JPEG_COMPRESSION_QUALITY:
> > > +ÂÂÂÂÂÂÂ video->jpeg_quality = ctrl->val;
> > > +ÂÂÂÂÂÂÂ aspeed_video_update_jpeg_quality(video);
> > > +ÂÂÂÂÂÂÂ break;
> > > +ÂÂÂ case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
> > > +ÂÂÂÂÂÂÂ if (ctrl->val == V4L2_JPEG_CHROMA_SUBSAMPLING_420) {
> > > +ÂÂÂÂÂÂÂÂÂÂÂ video->yuv420 = true;
> > > +ÂÂÂÂÂÂÂÂÂÂÂ aspeed_video_update_subsampling(video);
> > > +ÂÂÂÂÂÂÂ } else {
> > > +ÂÂÂÂÂÂÂÂÂÂÂ video->yuv420 = false;
> > > +ÂÂÂÂÂÂÂÂÂÂÂ aspeed_video_update_subsampling(video);
> > > +ÂÂÂÂÂÂÂ }
> > > +ÂÂÂÂÂÂÂ break;
> > > +ÂÂÂ default:
> > > +ÂÂÂÂÂÂÂ return -EINVAL;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static const struct v4l2_ctrl_ops aspeed_video_ctrl_ops = {
> > > +ÂÂÂ .s_ctrl = aspeed_video_set_ctrl,
> > > +};
> > > +
> > > +static void aspeed_video_resolution_work(struct work_struct *work)
> > > +{
> > > +ÂÂÂ int rc;
> > > +ÂÂÂ struct delayed_work *dwork = to_delayed_work(work);
> > > +ÂÂÂ struct aspeed_video *video = container_of(dwork, struct 
> > > aspeed_video,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ res_work);
> > > +
> > > +ÂÂÂ /* No clients remaining after delay */
> > > +ÂÂÂ if (atomic_read(&video->clients) == 0)
> > > +ÂÂÂÂÂÂÂ goto done;
> > > +
> > > +ÂÂÂ aspeed_video_on(video);
> > > +
> > > +ÂÂÂ aspeed_video_init_regs(video);
> > > +
> > > +ÂÂÂ rc = aspeed_video_get_resolution(video);
> > > +ÂÂÂ if (rc)
> > > +ÂÂÂÂÂÂÂ dev_err(video->dev,
> > > +ÂÂÂÂÂÂÂÂÂÂÂ "resolution changed; couldn't get new resolution\n");
> > > +ÂÂÂ else if (test_bit(VIDEO_STREAMING, &video->flags))
> > > +ÂÂÂÂÂÂÂ aspeed_video_start_frame(video);
> > > +
> > > +ÂÂÂ if (video->width != video->pix_fmt.width ||
> > > +ÂÂÂÂÂÂÂ video->height != video->pix_fmt.height) {
> > > +ÂÂÂÂÂÂÂ static const struct v4l2_event ev = {
> > > +ÂÂÂÂÂÂÂÂÂÂÂ .type = V4L2_EVENT_SOURCE_CHANGE,
> > > +ÂÂÂÂÂÂÂÂÂÂÂ .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
> > > +ÂÂÂÂÂÂÂ };
> > > +
> > > +ÂÂÂÂÂÂÂ v4l2_event_queue(&video->vdev, &ev);
> > > +ÂÂÂ }
> > > +
> > > +done:
> > > +ÂÂÂ clear_bit(VIDEO_RES_CHANGE, &video->flags);
> > > +ÂÂÂ wake_up_interruptible_all(&video->wait);
> > > +}
> > > +
> > > +static int aspeed_video_open(struct file *file)
> > > +{
> > > +ÂÂÂ int rc;
> > > +ÂÂÂ struct aspeed_video *video = video_drvdata(file);
> > > +
> > > +ÂÂÂ mutex_lock(&video->video_lock);
> > > +
> > > +ÂÂÂ if (atomic_inc_return(&video->clients) == 1) {
> > > +ÂÂÂÂÂÂÂ rc = aspeed_video_start(video);
> > > +ÂÂÂÂÂÂÂ if (rc) {
> > > +ÂÂÂÂÂÂÂÂÂÂÂ dev_err(video->dev, "Failed to start video engine\n");
> > > +ÂÂÂÂÂÂÂÂÂÂÂ atomic_dec(&video->clients);
> > > +ÂÂÂÂÂÂÂÂÂÂÂ mutex_unlock(&video->video_lock);
> > > +ÂÂÂÂÂÂÂÂÂÂÂ return rc;
> > > +ÂÂÂÂÂÂÂ }
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ mutex_unlock(&video->video_lock);
> > > +
> > > +ÂÂÂ return v4l2_fh_open(file);
> > > +}
> > > +
> > > +static int aspeed_video_release(struct file *file)
> > > +{
> > > +ÂÂÂ int rc;
> > > +ÂÂÂ struct aspeed_video *video = video_drvdata(file);
> > > +
> > > +ÂÂÂ rc = vb2_fop_release(file);
> > > +
> > > +ÂÂÂ mutex_lock(&video->video_lock);
> > > +
> > > +ÂÂÂ if (atomic_dec_return(&video->clients) == 0)
> > > +ÂÂÂÂÂÂÂ aspeed_video_stop(video);
> > > +
> > > +ÂÂÂ mutex_unlock(&video->video_lock);
> > > +
> > > +ÂÂÂ return rc;
> > > +}
> > > +
> > > +static const struct v4l2_file_operations aspeed_video_v4l2_fops = {
> > > +ÂÂÂ .owner = THIS_MODULE,
> > > +ÂÂÂ .read = vb2_fop_read,
> > > +ÂÂÂ .poll = vb2_fop_poll,
> > > +ÂÂÂ .unlocked_ioctl = video_ioctl2,
> > > +ÂÂÂ .mmap = vb2_fop_mmap,
> > > +ÂÂÂ .open = aspeed_video_open,
> > > +ÂÂÂ .release = aspeed_video_release,
> > > +};
> > > +
> > > +static int aspeed_video_queue_setup(struct vb2_queue *q,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ unsigned int *num_buffers,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ unsigned int *num_planes,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ unsigned int sizes[],
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device *alloc_devs[])
> > > +{
> > > +ÂÂÂ struct aspeed_video *video = vb2_get_drv_priv(q);
> > > +
> > > +ÂÂÂ if (*num_planes) {
> > > +ÂÂÂÂÂÂÂ if (sizes[0] < video->max_compressed_size)
> > > +ÂÂÂÂÂÂÂÂÂÂÂ return -EINVAL;
> > > +
> > > +ÂÂÂÂÂÂÂ return 0;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ *num_planes = 1;
> > > +ÂÂÂ sizes[0] = video->max_compressed_size;
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_buf_prepare(struct vb2_buffer *vb)
> > > +{
> > > +ÂÂÂ struct aspeed_video *video = vb2_get_drv_priv(vb->vb2_queue);
> > > +
> > > +ÂÂÂ if (vb2_plane_size(vb, 0) < video->max_compressed_size)
> > > +ÂÂÂÂÂÂÂ return -EINVAL;
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_start_streaming(struct vb2_queue *q,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ unsigned int count)
> > > +{
> > > +ÂÂÂ int rc;
> > > +ÂÂÂ struct aspeed_video *video = vb2_get_drv_priv(q);
> > > +
> > > +ÂÂÂ rc = aspeed_video_start_frame(video);
> > > +ÂÂÂ if (rc) {
> > > +ÂÂÂÂÂÂÂ aspeed_video_bufs_done(video, VB2_BUF_STATE_QUEUED);
> > > +ÂÂÂÂÂÂÂ return rc;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ video->sequence = 0;
> > > +ÂÂÂ set_bit(VIDEO_STREAMING, &video->flags);
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static void aspeed_video_stop_streaming(struct vb2_queue *q)
> > > +{
> > > +ÂÂÂ int rc;
> > > +ÂÂÂ struct aspeed_video *video = vb2_get_drv_priv(q);
> > > +
> > > +ÂÂÂ clear_bit(VIDEO_STREAMING, &video->flags);
> > > +
> > > +ÂÂÂ rc = wait_event_timeout(video->wait,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ !test_bit(VIDEO_FRAME_INPRG, &video->flags),
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ STOP_TIMEOUT);
> > > +ÂÂÂ if (!rc) {
> > > +ÂÂÂÂÂÂÂ dev_err(video->dev, "Timed out when stopping streaming\n");
> > > +ÂÂÂÂÂÂÂ aspeed_video_stop(video);
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
> > > +}
> > > +
> > > +static void aspeed_video_buf_queue(struct vb2_buffer *vb)
> > > +{
> > > +ÂÂÂ struct aspeed_video *video = vb2_get_drv_priv(vb->vb2_queue);
> > > +ÂÂÂ struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> > > +ÂÂÂ struct aspeed_video_buffer *avb = to_aspeed_video_buffer(vbuf);
> > > +ÂÂÂ unsigned long flags;
> > > +
> > > +ÂÂÂ spin_lock_irqsave(&video->lock, flags);
> > > +ÂÂÂ list_add_tail(&avb->link, &video->buffers);
> > > +ÂÂÂ spin_unlock_irqrestore(&video->lock, flags);
> > > +}
> > > +
> > > +static const struct vb2_ops aspeed_video_vb2_ops = {
> > > +ÂÂÂ .queue_setup = aspeed_video_queue_setup,
> > > +ÂÂÂ .wait_prepare = vb2_ops_wait_prepare,
> > > +ÂÂÂ .wait_finish = vb2_ops_wait_finish,
> > > +ÂÂÂ .buf_prepare = aspeed_video_buf_prepare,
> > > +ÂÂÂ .start_streaming = aspeed_video_start_streaming,
> > > +ÂÂÂ .stop_streaming = aspeed_video_stop_streaming,
> > > +ÂÂÂ .buf_queue =Â aspeed_video_buf_queue,
> > > +};
> > > +
> > > +static int aspeed_video_setup_video(struct aspeed_video *video)
> > > +{
> > > +ÂÂÂ const u64 mask = ~(BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_444) |
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_420));
> > > +ÂÂÂ struct v4l2_device *v4l2_dev = &video->v4l2_dev;
> > > +ÂÂÂ struct vb2_queue *vbq = &video->queue;
> > > +ÂÂÂ struct video_device *vdev = &video->vdev;
> > > +ÂÂÂ int rc;
> > > +
> > > +ÂÂÂ video->pix_fmt.pixelformat = V4L2_PIX_FMT_JPEG;
> > > +ÂÂÂ video->pix_fmt.field = V4L2_FIELD_NONE;
> > > +ÂÂÂ video->pix_fmt.colorspace = V4L2_COLORSPACE_SRGB;
> > > +ÂÂÂ video->pix_fmt.quantization = V4L2_QUANTIZATION_FULL_RANGE;
> > > +
> > > +ÂÂÂ rc = v4l2_device_register(video->dev, v4l2_dev);
> > > +ÂÂÂ if (rc) {
> > > +ÂÂÂÂÂÂÂ dev_err(video->dev, "Failed to register v4l2 device\n");
> > > +ÂÂÂÂÂÂÂ return rc;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ v4l2_ctrl_handler_init(&video->ctrl_handler, 2);
> > > +ÂÂÂ v4l2_ctrl_new_std(&video->ctrl_handler, &aspeed_video_ctrl_ops,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂ V4L2_CID_JPEG_COMPRESSION_QUALITY, 0,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂ ASPEED_VIDEO_JPEG_NUM_QUALITIES - 1, 1, 0);
> > > +ÂÂÂ v4l2_ctrl_new_std_menu(&video->ctrl_handler, 
> > > &aspeed_video_ctrl_ops,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ V4L2_CID_JPEG_CHROMA_SUBSAMPLING,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ V4L2_JPEG_CHROMA_SUBSAMPLING_420, mask,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ V4L2_JPEG_CHROMA_SUBSAMPLING_444);
> > > +
> > > +ÂÂÂ if (video->ctrl_handler.error) {
> > > +ÂÂÂÂÂÂÂ v4l2_ctrl_handler_free(&video->ctrl_handler);
> > > +ÂÂÂÂÂÂÂ v4l2_device_unregister(v4l2_dev);
> > > +
> > > +ÂÂÂÂÂÂÂ dev_err(video->dev, "Failed to init controls: %d\n",
> > > +ÂÂÂÂÂÂÂÂÂÂÂ video->ctrl_handler.error);
> > > +ÂÂÂÂÂÂÂ return rc;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ v4l2_dev->ctrl_handler = &video->ctrl_handler;
> > > +
> > > +ÂÂÂ vbq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> > > +ÂÂÂ vbq->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
> > > +ÂÂÂ vbq->dev = v4l2_dev->dev;
> > > +ÂÂÂ vbq->lock = &video->video_lock;
> > > +ÂÂÂ vbq->ops = &aspeed_video_vb2_ops;
> > > +ÂÂÂ vbq->mem_ops = &vb2_dma_contig_memops;
> > > +ÂÂÂ vbq->drv_priv = video;
> > > +ÂÂÂ vbq->buf_struct_size = sizeof(struct aspeed_video_buffer);
> > > +ÂÂÂ vbq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> > > +ÂÂÂ vbq->min_buffers_needed = 3;
> > > +
> > > +ÂÂÂ rc = vb2_queue_init(vbq);
> > > +ÂÂÂ if (rc) {
> > > +ÂÂÂÂÂÂÂ v4l2_ctrl_handler_free(&video->ctrl_handler);
> > > +ÂÂÂÂÂÂÂ v4l2_device_unregister(v4l2_dev);
> > > +
> > > +ÂÂÂÂÂÂÂ dev_err(video->dev, "Failed to init vb2 queue\n");
> > > +ÂÂÂÂÂÂÂ return rc;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ vdev->queue = vbq;
> > > +ÂÂÂ vdev->fops = &aspeed_video_v4l2_fops;
> > > +ÂÂÂ vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
> > READWRITE doesn't work for JPEG since there is no clear end of a 
> > frame. Just drop
> > this and the read op in aspeed_video_v4l2_fops.
> >
> > > +ÂÂÂÂÂÂÂ V4L2_CAP_STREAMING;
> > > +ÂÂÂ vdev->v4l2_dev = v4l2_dev;
> > > +ÂÂÂ strscpy(vdev->name, DEVICE_NAME, sizeof(vdev->name));
> > > +ÂÂÂ vdev->vfl_type = VFL_TYPE_GRABBER;
> > > +ÂÂÂ vdev->vfl_dir = VFL_DIR_RX;
> > > +ÂÂÂ vdev->release = video_device_release_empty;
> > > +ÂÂÂ vdev->ioctl_ops = &aspeed_video_ioctl_ops;
> > > +ÂÂÂ vdev->lock = &video->video_lock;
> > > +
> > > +ÂÂÂ video_set_drvdata(vdev, video);
> > > +ÂÂÂ rc = video_register_device(vdev, VFL_TYPE_GRABBER, 0);
> > > +ÂÂÂ if (rc) {
> > > +ÂÂÂÂÂÂÂ vb2_queue_release(vbq);
> > > +ÂÂÂÂÂÂÂ v4l2_ctrl_handler_free(&video->ctrl_handler);
> > > +ÂÂÂÂÂÂÂ v4l2_device_unregister(v4l2_dev);
> > > +
> > > +ÂÂÂÂÂÂÂ dev_err(video->dev, "Failed to register video device\n");
> > > +ÂÂÂÂÂÂÂ return rc;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_init(struct aspeed_video *video)
> > > +{
> > > +ÂÂÂ int irq;
> > > +ÂÂÂ int rc;
> > > +ÂÂÂ struct device *dev = video->dev;
> > > +
> > > +ÂÂÂ irq = irq_of_parse_and_map(dev->of_node, 0);
> > > +ÂÂÂ if (!irq) {
> > > +ÂÂÂÂÂÂÂ dev_err(dev, "Unable to find IRQ\n");
> > > +ÂÂÂÂÂÂÂ return -ENODEV;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ rc = devm_request_irq(dev, irq, aspeed_video_irq, IRQF_SHARED,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ DEVICE_NAME, video);
> > > +ÂÂÂ if (rc < 0) {
> > > +ÂÂÂÂÂÂÂ dev_err(dev, "Unable to request IRQ %d\n", irq);
> > > +ÂÂÂÂÂÂÂ return rc;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ video->eclk = devm_clk_get(dev, "eclk");
> > > +ÂÂÂ if (IS_ERR(video->eclk)) {
> > > +ÂÂÂÂÂÂÂ dev_err(dev, "Unable to get ECLK\n");
> > > +ÂÂÂÂÂÂÂ return PTR_ERR(video->eclk);
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ video->vclk = devm_clk_get(dev, "vclk");
> > > +ÂÂÂ if (IS_ERR(video->vclk)) {
> > > +ÂÂÂÂÂÂÂ dev_err(dev, "Unable to get VCLK\n");
> > > +ÂÂÂÂÂÂÂ return PTR_ERR(video->vclk);
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ video->rst = devm_reset_control_get_exclusive(dev, NULL);
> > > +ÂÂÂ if (IS_ERR(video->rst)) {
> > > +ÂÂÂÂÂÂÂ dev_err(dev, "Unable to get VE reset\n");
> > > +ÂÂÂÂÂÂÂ return PTR_ERR(video->rst);
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ rc = of_reserved_mem_device_init(dev);
> > > +ÂÂÂ if (rc) {
> > > +ÂÂÂÂÂÂÂ dev_err(dev, "Unable to reserve memory\n");
> > > +ÂÂÂÂÂÂÂ return rc;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> > > +ÂÂÂ if (rc) {
> > > +ÂÂÂÂÂÂÂ dev_err(dev, "Failed to set DMA mask\n");
> > > +ÂÂÂÂÂÂÂ of_reserved_mem_device_release(dev);
> > > +ÂÂÂÂÂÂÂ return rc;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ if (!aspeed_video_alloc_buf(video, &video->jpeg,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ VE_JPEG_HEADER_SIZE)) {
> > > +ÂÂÂÂÂÂÂ dev_err(dev, "Failed to allocate DMA for JPEG header\n");
> > > +ÂÂÂÂÂÂÂ of_reserved_mem_device_release(dev);
> > > +ÂÂÂÂÂÂÂ return rc;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ aspeed_video_init_jpeg_table(video->jpeg.virt, video->yuv420);
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_probe(struct platform_device *pdev)
> > > +{
> > > +ÂÂÂ int rc;
> > > +ÂÂÂ struct resource *res;
> > > +ÂÂÂ struct aspeed_video *video = kzalloc(sizeof(*video), GFP_KERNEL);
> > > +
> > > +ÂÂÂ if (!video)
> > > +ÂÂÂÂÂÂÂ return -ENOMEM;
> > > +
> > > +ÂÂÂ video->frame_rate = 30;
> > > +ÂÂÂ video->dev = &pdev->dev;
> > > +ÂÂÂ mutex_init(&video->video_lock);
> > > +ÂÂÂ init_waitqueue_head(&video->wait);
> > > +ÂÂÂ INIT_DELAYED_WORK(&video->res_work, aspeed_video_resolution_work);
> > > +ÂÂÂ INIT_LIST_HEAD(&video->buffers);
> > > +
> > > +ÂÂÂ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > +
> > > +ÂÂÂ video->base = devm_ioremap_resource(video->dev, res);
> > > +
> > > +ÂÂÂ if (IS_ERR(video->base))
> > > +ÂÂÂÂÂÂÂ return PTR_ERR(video->base);
> > > +
> > > +ÂÂÂ rc = aspeed_video_init(video);
> > > +ÂÂÂ if (rc)
> > > +ÂÂÂÂÂÂÂ return rc;
> > > +
> > > +ÂÂÂ rc = aspeed_video_setup_video(video);
> > > +ÂÂÂ if (rc)
> > > +ÂÂÂÂÂÂÂ return rc;
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int aspeed_video_remove(struct platform_device *pdev)
> > > +{
> > > +ÂÂÂ struct device *dev = &pdev->dev;
> > > +ÂÂÂ struct v4l2_device *v4l2_dev = dev_get_drvdata(dev);
> > > +ÂÂÂ struct aspeed_video *video = to_aspeed_video(v4l2_dev);
> > > +
> > > +ÂÂÂ video_unregister_device(&video->vdev);
> > > +
> > > +ÂÂÂ vb2_queue_release(&video->queue);
> > > +
> > > +ÂÂÂ v4l2_ctrl_handler_free(&video->ctrl_handler);
> > > +
> > > +ÂÂÂ v4l2_device_unregister(v4l2_dev);
> > > +
> > > +ÂÂÂ dma_free_coherent(video->dev, VE_JPEG_HEADER_SIZE, 
> > > video->jpeg.virt,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂ video->jpeg.dma);
> > > +
> > > +ÂÂÂ of_reserved_mem_device_release(dev);
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static const struct of_device_id aspeed_video_of_match[] = {
> > > +ÂÂÂ { .compatible = "aspeed,ast2400-video-engine" },
> > > +ÂÂÂ { .compatible = "aspeed,ast2500-video-engine" },
> > > +ÂÂÂ {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, aspeed_video_of_match);
> > > +
> > > +static struct platform_driver aspeed_video_driver = {
> > > +ÂÂÂ .driver = {
> > > +ÂÂÂÂÂÂÂ .name = DEVICE_NAME,
> > > +ÂÂÂÂÂÂÂ .of_match_table = aspeed_video_of_match,
> > > +ÂÂÂ },
> > > +ÂÂÂ .probe = aspeed_video_probe,
> > > +ÂÂÂ .remove = aspeed_video_remove,
> > > +};
> > > +
> > > +module_platform_driver(aspeed_video_driver);
> > > +
> > > +MODULE_DESCRIPTION("ASPEED Video Engine Driver");
> > > +MODULE_AUTHOR("Eddie James");
> > > +MODULE_LICENSE("GPL v2");
> > Regards,
> >
> > ÂÂÂÂHans