| Message ID | 20230915-wave5_v12_on_media_master-v12-0-92fc66cd685d@collabora.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Wave5 codec driver | expand |
On 21/09/2023 21:11, Nicolas Dufresne wrote: > Le mercredi 20 septembre 2023 à 17:13 +0200, Hans Verkuil a écrit : >> On 15/09/2023 23:11, Sebastian Fricke wrote: >>> From: Nas Chung <nas.chung@chipsnmedia.com> >>> >>> Add the decoder and encoder implementing the v4l2 >>> API. This patch also adds the Makefile and the VIDEO_WAVE_VPU config >>> >>> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com> >>> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> >>> Signed-off-by: Robert Beckett <bob.beckett@collabora.com> >>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> >>> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> >>> --- >>> drivers/media/platform/chips-media/Kconfig | 1 + >>> drivers/media/platform/chips-media/Makefile | 1 + >>> drivers/media/platform/chips-media/wave5/Kconfig | 12 + >>> drivers/media/platform/chips-media/wave5/Makefile | 10 + >>> .../platform/chips-media/wave5/wave5-helper.c | 196 ++ >>> .../platform/chips-media/wave5/wave5-helper.h | 30 + >>> .../platform/chips-media/wave5/wave5-vpu-dec.c | 1965 ++++++++++++++++++++ >>> .../platform/chips-media/wave5/wave5-vpu-enc.c | 1825 ++++++++++++++++++ >>> .../media/platform/chips-media/wave5/wave5-vpu.c | 331 ++++ >>> .../media/platform/chips-media/wave5/wave5-vpu.h | 83 + >>> 10 files changed, 4454 insertions(+) >>> <snip> >>> +static int wave5_vpu_dec_set_eos_on_firmware(struct vpu_instance *inst) >>> +{ >>> + int ret; >>> + >>> + ret = wave5_vpu_dec_update_bitstream_buffer(inst, 0); >>> + if (ret) { >>> + dev_err(inst->dev->dev, >>> + "Setting EOS for the bitstream, fail: %d\n", ret); >> >> Is this an error due to a driver problem, or because a bad bitstream is >> fed from userspace? In the first case, dev_err would be right, in the >> second dev_dbg would be more appropriate. Bad userspace input should not >> spam the kernel log in general. > > Its the first. To set the EOS flag, a command is sent to the firmware. That > command may never return (timeout) or may report an error. For this specific > command, if that happens we are likely facing firmware of driver problem (or > both). OK, I'd add that as a comment here as this is unexpected behavior. > >> >>> + return ret; >>> + } >>> + return 0; >>> +} <snip> >>> +static int wave5_vpu_dec_create_bufs(struct file *file, void *priv, >>> + struct v4l2_create_buffers *create) >>> +{ >>> + struct v4l2_format *f = &create->format; >>> + >>> + if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) >>> + return -ENOTTY; >> >> Huh? Why is this needed? > > Minimally a comment should be added. The why is that we support CREATE_BUF for > OUTPUT queue (bitstream) but not for CAPTURE queues. This is simply not > supported by Wave5 firmware. Do you have any suggestion how this asymmetry can > be implemented better ? Certainly not with ENOTTY: the ioctl exists, it is just not supported for CAPTURE queues. How about -EPERM? And document this error as well in the VIDIOC_CREATE_BUFS documentation. And you want a dev_dbg here too. So I would propose that EPERM is returned if CREATE_BUFS is only supported for for one of the two queues of an M2M device. > >> >>> + >>> + return v4l2_m2m_ioctl_create_bufs(file, priv, create); >>> +} <snip> >>> +static int wave5_vpu_dec_queue_setup(struct vb2_queue *q, unsigned int *num_buffers, >>> + unsigned int *num_planes, unsigned int sizes[], >>> + struct device *alloc_devs[]) >>> +{ >>> + struct vpu_instance *inst = vb2_get_drv_priv(q); >>> + struct v4l2_pix_format_mplane inst_format = >>> + (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) ? inst->src_fmt : inst->dst_fmt; >>> + unsigned int i; >>> + >>> + dev_dbg(inst->dev->dev, "%s: num_buffers: %u | num_planes: %u | type: %u\n", __func__, >>> + *num_buffers, *num_planes, q->type); >>> + >>> + /* the CREATE_BUFS case */ >>> + if (*num_planes) { >>> + if (inst_format.num_planes != *num_planes) >>> + return -EINVAL; >>> + >>> + for (i = 0; i < *num_planes; i++) { >>> + if (sizes[i] < inst_format.plane_fmt[i].sizeimage) >>> + return -EINVAL; >>> + } >>> + /* the REQBUFS case */ >>> + } else { >>> + *num_planes = inst_format.num_planes; >>> + >>> + if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { >>> + sizes[0] = inst_format.plane_fmt[0].sizeimage; >>> + dev_dbg(inst->dev->dev, "%s: size[0]: %u\n", __func__, sizes[0]); >>> + } else if (*num_planes == 1) { >>> + if (inst->output_format == FORMAT_422) >>> + sizes[0] = inst_format.width * inst_format.height * 2; >>> + else >>> + sizes[0] = inst_format.width * inst_format.height * 3 / 2; >>> + dev_dbg(inst->dev->dev, "%s: size[0]: %u\n", __func__, sizes[0]); >>> + } else if (*num_planes == 2) { >>> + sizes[0] = inst_format.width * inst_format.height; >>> + if (inst->output_format == FORMAT_422) >>> + sizes[1] = inst_format.width * inst_format.height; >>> + else >>> + sizes[1] = inst_format.width * inst_format.height / 2; >>> + dev_dbg(inst->dev->dev, "%s: size[0]: %u | size[1]: %u\n", >>> + __func__, sizes[0], sizes[1]); >>> + } else if (*num_planes == 3) { >>> + sizes[0] = inst_format.width * inst_format.height; >>> + if (inst->output_format == FORMAT_422) { >>> + sizes[1] = inst_format.width * inst_format.height / 2; >>> + sizes[2] = inst_format.width * inst_format.height / 2; >>> + } else { >>> + sizes[1] = inst_format.width * inst_format.height / 4; >>> + sizes[2] = inst_format.width * inst_format.height / 4; >>> + } >>> + dev_dbg(inst->dev->dev, "%s: size[0]: %u | size[1]: %u | size[2]: %u\n", >>> + __func__, sizes[0], sizes[1], sizes[2]); >>> + } >>> + } >>> + >>> + if (inst->state == VPU_INST_STATE_INIT_SEQ && >>> + q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { >>> + if (*num_buffers > inst->dst_buf_count && >>> + *num_buffers < WAVE5_MAX_FBS) >>> + inst->dst_buf_count = *num_buffers; >> >> In the create_bufs case, *num_buffers is the number of buffers you are >> adding to the already existing buffers. Frankly, the logic here is >> dubious. I'm not sure what the intent is. Why do you need to keep track >> of the buf count at all when the vb2_queue already does that? > > This needs to be cleaned up. CREATE_BUFS case is not supported for capture, and > so there is no such weirdo case second calls for that queue at least. Meanwhile, > inst->dst_buf_count is used a MIN_BUFFERS_FOR_CAPTURE initially, and the number > of allocated buffer later. I think it would be better to simply store the min, > and use the queue to track the number of allocated buffers. > > In this diver, the reference frame are stored separately, and compressed. The > capture queue contains the display frame. There is a gap when comes time to > transfer timestamp, and for this reason we had to keep the two fbc_count equal. > We classified this as hardware issue and moved on. > > I think the dst_buf_count can be dropped now and the "*num_buffers > inst- >> dst_buf_count" not longer make any sense. > >> >> WAVE5_MAX_FBS == 32, which is VIDEO_MAX_FRAMES. You can just drop the check >> against WAVE5_MAX_FBS since the core ensures already it will never allocate >> more than VIDEO_MAX_FRAMES. >> >> I'm not sure why WAVE5_MAX_FBS is defined at all, when it is just equal to >> VIDEO_MAX_FRAMES. Perhaps it can be replaced everywhere with VIDEO_MAX_FRAMES? > > That is more challenging changes, since VIDEO_MAX_FRAMES is a software > limitation, but WAVE5_MAX_FBS is a hardware limitation. Buffer index only have 4 > bits on this hardware. And the marking of frame being used for display is using > a 32bit flag. Considering there is effort to lift that software limitation, it > seems ill advised to completely stop ensuring this HW limit is respected. If there are only 4 bits for the buffer index, shouldn't WAVE5_MAX_FBS be 16? Or did you mean '5 bits'? Assuming that you meant '5 bits', then that makes WAVE5_MAX_FBS identical to VIDEO_MAX_FRAMES, but that is just luck, really. In any case, you should document at the place where WAVE5_MAX_FBS is defined that this is a hardware limitation, and not a random software limit. I also think that the DELETE_BUFS series should allow drivers to set max_num_buffers to a value less than 32 (currently the requirement is that it is at least 32). I saw a few more drivers that limit the number of buffers, usually based on the format (and so the buffer size) and some HW memory limitation. It might be interesting to allow drivers to change max_num_buffers on the fly (provided no buffers are allocated, of course). Limit checking is really something that vb2 should do, and not the driver. > > I'm open for suggestions. > >> >>> + >>> + *num_buffers = inst->dst_buf_count; >>> + } >>> + >>> + return 0; >>> +} Regards, Hans
Le vendredi 22 septembre 2023 à 09:33 +0200, Hans Verkuil a écrit : > On 21/09/2023 21:11, Nicolas Dufresne wrote: > > Le mercredi 20 septembre 2023 à 17:13 +0200, Hans Verkuil a écrit : > > > On 15/09/2023 23:11, Sebastian Fricke wrote: > > > > From: Nas Chung <nas.chung@chipsnmedia.com> > > > > > > > > Add the decoder and encoder implementing the v4l2 > > > > API. This patch also adds the Makefile and the VIDEO_WAVE_VPU config > > > > > > > > Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com> > > > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > Signed-off-by: Robert Beckett <bob.beckett@collabora.com> > > > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> > > > > Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> > > > > --- > > > > drivers/media/platform/chips-media/Kconfig | 1 + > > > > drivers/media/platform/chips-media/Makefile | 1 + > > > > drivers/media/platform/chips-media/wave5/Kconfig | 12 + > > > > drivers/media/platform/chips-media/wave5/Makefile | 10 + > > > > .../platform/chips-media/wave5/wave5-helper.c | 196 ++ > > > > .../platform/chips-media/wave5/wave5-helper.h | 30 + > > > > .../platform/chips-media/wave5/wave5-vpu-dec.c | 1965 ++++++++++++++++++++ > > > > .../platform/chips-media/wave5/wave5-vpu-enc.c | 1825 ++++++++++++++++++ > > > > .../media/platform/chips-media/wave5/wave5-vpu.c | 331 ++++ > > > > .../media/platform/chips-media/wave5/wave5-vpu.h | 83 + > > > > 10 files changed, 4454 insertions(+) > > > > > > <snip> > > > > > +static int wave5_vpu_dec_set_eos_on_firmware(struct vpu_instance *inst) > > > > +{ > > > > + int ret; > > > > + > > > > + ret = wave5_vpu_dec_update_bitstream_buffer(inst, 0); > > > > + if (ret) { > > > > + dev_err(inst->dev->dev, > > > > + "Setting EOS for the bitstream, fail: %d\n", ret); > > > > > > Is this an error due to a driver problem, or because a bad bitstream is > > > fed from userspace? In the first case, dev_err would be right, in the > > > second dev_dbg would be more appropriate. Bad userspace input should not > > > spam the kernel log in general. > > > > Its the first. To set the EOS flag, a command is sent to the firmware. That > > command may never return (timeout) or may report an error. For this specific > > command, if that happens we are likely facing firmware of driver problem (or > > both). > > OK, I'd add that as a comment here as this is unexpected behavior. > > > > > > > > > > + return ret; > > > > + } > > > > + return 0; > > > > +} > > <snip> > > > > > +static int wave5_vpu_dec_create_bufs(struct file *file, void *priv, > > > > + struct v4l2_create_buffers *create) > > > > +{ > > > > + struct v4l2_format *f = &create->format; > > > > + > > > > + if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) > > > > + return -ENOTTY; > > > > > > Huh? Why is this needed? > > > > Minimally a comment should be added. The why is that we support CREATE_BUF for > > OUTPUT queue (bitstream) but not for CAPTURE queues. This is simply not > > supported by Wave5 firmware. Do you have any suggestion how this asymmetry can > > be implemented better ? > > Certainly not with ENOTTY: the ioctl exists, it is just not supported for > CAPTURE queues. > > How about -EPERM? And document this error as well in the VIDIOC_CREATE_BUFS > documentation. And you want a dev_dbg here too. The suggestion cannot be used since there is documentation for that one already, and it does not match "unsupported". "Permission denied. Can be returned if the device needs write permission, or some special capabilities is needed (e. g. root)" What about using the most logical error code, which name is actually obvious, like ENOTSUP ? #define ENOTSUPP 524 /* Operation is not supported */ > > So I would propose that EPERM is returned if CREATE_BUFS is only supported > for for one of the two queues of an M2M device. Note that userspace does not care of the difference between an ioctl not being implemented at all or not being implement for one queue. GStreamer have been testing with both queue type for couple of years now. Adding this distinction is just leaking an implementation details to userspace. I'm fine to just do what you'd like, just stating the obvious that while it may look logical inside the kernel, its a bit of a non-sense for our users. regards, Nicolas > > > > > > > > > > + > > > > + return v4l2_m2m_ioctl_create_bufs(file, priv, create); > > > > +} > > <snip> > > > > > +static int wave5_vpu_dec_queue_setup(struct vb2_queue *q, unsigned int *num_buffers, > > > > + unsigned int *num_planes, unsigned int sizes[], > > > > + struct device *alloc_devs[]) > > > > +{ > > > > + struct vpu_instance *inst = vb2_get_drv_priv(q); > > > > + struct v4l2_pix_format_mplane inst_format = > > > > + (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) ? inst->src_fmt : inst->dst_fmt; > > > > + unsigned int i; > > > > + > > > > + dev_dbg(inst->dev->dev, "%s: num_buffers: %u | num_planes: %u | type: %u\n", __func__, > > > > + *num_buffers, *num_planes, q->type); > > > > + > > > > + /* the CREATE_BUFS case */ > > > > + if (*num_planes) { > > > > + if (inst_format.num_planes != *num_planes) > > > > + return -EINVAL; > > > > + > > > > + for (i = 0; i < *num_planes; i++) { > > > > + if (sizes[i] < inst_format.plane_fmt[i].sizeimage) > > > > + return -EINVAL; > > > > + } > > > > + /* the REQBUFS case */ > > > > + } else { > > > > + *num_planes = inst_format.num_planes; > > > > + > > > > + if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { > > > > + sizes[0] = inst_format.plane_fmt[0].sizeimage; > > > > + dev_dbg(inst->dev->dev, "%s: size[0]: %u\n", __func__, sizes[0]); > > > > + } else if (*num_planes == 1) { > > > > + if (inst->output_format == FORMAT_422) > > > > + sizes[0] = inst_format.width * inst_format.height * 2; > > > > + else > > > > + sizes[0] = inst_format.width * inst_format.height * 3 / 2; > > > > + dev_dbg(inst->dev->dev, "%s: size[0]: %u\n", __func__, sizes[0]); > > > > + } else if (*num_planes == 2) { > > > > + sizes[0] = inst_format.width * inst_format.height; > > > > + if (inst->output_format == FORMAT_422) > > > > + sizes[1] = inst_format.width * inst_format.height; > > > > + else > > > > + sizes[1] = inst_format.width * inst_format.height / 2; > > > > + dev_dbg(inst->dev->dev, "%s: size[0]: %u | size[1]: %u\n", > > > > + __func__, sizes[0], sizes[1]); > > > > + } else if (*num_planes == 3) { > > > > + sizes[0] = inst_format.width * inst_format.height; > > > > + if (inst->output_format == FORMAT_422) { > > > > + sizes[1] = inst_format.width * inst_format.height / 2; > > > > + sizes[2] = inst_format.width * inst_format.height / 2; > > > > + } else { > > > > + sizes[1] = inst_format.width * inst_format.height / 4; > > > > + sizes[2] = inst_format.width * inst_format.height / 4; > > > > + } > > > > + dev_dbg(inst->dev->dev, "%s: size[0]: %u | size[1]: %u | size[2]: %u\n", > > > > + __func__, sizes[0], sizes[1], sizes[2]); > > > > + } > > > > + } > > > > + > > > > + if (inst->state == VPU_INST_STATE_INIT_SEQ && > > > > + q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { > > > > + if (*num_buffers > inst->dst_buf_count && > > > > + *num_buffers < WAVE5_MAX_FBS) > > > > + inst->dst_buf_count = *num_buffers; > > > > > > In the create_bufs case, *num_buffers is the number of buffers you are > > > adding to the already existing buffers. Frankly, the logic here is > > > dubious. I'm not sure what the intent is. Why do you need to keep track > > > of the buf count at all when the vb2_queue already does that? > > > > This needs to be cleaned up. CREATE_BUFS case is not supported for capture, and > > so there is no such weirdo case second calls for that queue at least. Meanwhile, > > inst->dst_buf_count is used a MIN_BUFFERS_FOR_CAPTURE initially, and the number > > of allocated buffer later. I think it would be better to simply store the min, > > and use the queue to track the number of allocated buffers. > > > > In this diver, the reference frame are stored separately, and compressed. The > > capture queue contains the display frame. There is a gap when comes time to > > transfer timestamp, and for this reason we had to keep the two fbc_count equal. > > We classified this as hardware issue and moved on. > > > > I think the dst_buf_count can be dropped now and the "*num_buffers > inst- > > > dst_buf_count" not longer make any sense. > > > > > > > > WAVE5_MAX_FBS == 32, which is VIDEO_MAX_FRAMES. You can just drop the check > > > against WAVE5_MAX_FBS since the core ensures already it will never allocate > > > more than VIDEO_MAX_FRAMES. > > > > > > I'm not sure why WAVE5_MAX_FBS is defined at all, when it is just equal to > > > VIDEO_MAX_FRAMES. Perhaps it can be replaced everywhere with VIDEO_MAX_FRAMES? > > > > That is more challenging changes, since VIDEO_MAX_FRAMES is a software > > limitation, but WAVE5_MAX_FBS is a hardware limitation. Buffer index only have 4 > > bits on this hardware. And the marking of frame being used for display is using > > a 32bit flag. Considering there is effort to lift that software limitation, it > > seems ill advised to completely stop ensuring this HW limit is respected. > > If there are only 4 bits for the buffer index, shouldn't WAVE5_MAX_FBS be 16? Or > did you mean '5 bits'? Assuming that you meant '5 bits', then that makes > WAVE5_MAX_FBS identical to VIDEO_MAX_FRAMES, but that is just luck, really. > > In any case, you should document at the place where WAVE5_MAX_FBS is defined that > this is a hardware limitation, and not a random software limit. > > I also think that the DELETE_BUFS series should allow drivers to set max_num_buffers > to a value less than 32 (currently the requirement is that it is at least 32). > > I saw a few more drivers that limit the number of buffers, usually based on the > format (and so the buffer size) and some HW memory limitation. It might be interesting > to allow drivers to change max_num_buffers on the fly (provided no buffers are > allocated, of course). Limit checking is really something that vb2 should do, and > not the driver. > > > > > I'm open for suggestions. > > > > > > > > > + > > > > + *num_buffers = inst->dst_buf_count; > > > > + } > > > > + > > > > + return 0; > > > > +} > > Regards, > > Hans
On 27/09/2023 01:29, Nicolas Dufresne wrote: > Le vendredi 22 septembre 2023 à 09:33 +0200, Hans Verkuil a écrit : >> On 21/09/2023 21:11, Nicolas Dufresne wrote: >>> Le mercredi 20 septembre 2023 à 17:13 +0200, Hans Verkuil a écrit : >>>> On 15/09/2023 23:11, Sebastian Fricke wrote: >>>>> From: Nas Chung <nas.chung@chipsnmedia.com> >>>>> >>>>> Add the decoder and encoder implementing the v4l2 >>>>> API. This patch also adds the Makefile and the VIDEO_WAVE_VPU config >>>>> >>>>> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com> >>>>> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> >>>>> Signed-off-by: Robert Beckett <bob.beckett@collabora.com> >>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> >>>>> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> >>>>> --- >>>>> drivers/media/platform/chips-media/Kconfig | 1 + >>>>> drivers/media/platform/chips-media/Makefile | 1 + >>>>> drivers/media/platform/chips-media/wave5/Kconfig | 12 + >>>>> drivers/media/platform/chips-media/wave5/Makefile | 10 + >>>>> .../platform/chips-media/wave5/wave5-helper.c | 196 ++ >>>>> .../platform/chips-media/wave5/wave5-helper.h | 30 + >>>>> .../platform/chips-media/wave5/wave5-vpu-dec.c | 1965 ++++++++++++++++++++ >>>>> .../platform/chips-media/wave5/wave5-vpu-enc.c | 1825 ++++++++++++++++++ >>>>> .../media/platform/chips-media/wave5/wave5-vpu.c | 331 ++++ >>>>> .../media/platform/chips-media/wave5/wave5-vpu.h | 83 + >>>>> 10 files changed, 4454 insertions(+) >>>>> >> >> <snip> >> >>>>> +static int wave5_vpu_dec_set_eos_on_firmware(struct vpu_instance *inst) >>>>> +{ >>>>> + int ret; >>>>> + >>>>> + ret = wave5_vpu_dec_update_bitstream_buffer(inst, 0); >>>>> + if (ret) { >>>>> + dev_err(inst->dev->dev, >>>>> + "Setting EOS for the bitstream, fail: %d\n", ret); >>>> >>>> Is this an error due to a driver problem, or because a bad bitstream is >>>> fed from userspace? In the first case, dev_err would be right, in the >>>> second dev_dbg would be more appropriate. Bad userspace input should not >>>> spam the kernel log in general. >>> >>> Its the first. To set the EOS flag, a command is sent to the firmware. That >>> command may never return (timeout) or may report an error. For this specific >>> command, if that happens we are likely facing firmware of driver problem (or >>> both). >> >> OK, I'd add that as a comment here as this is unexpected behavior. >> >>> >>>> >>>>> + return ret; >>>>> + } >>>>> + return 0; >>>>> +} >> >> <snip> >> >>>>> +static int wave5_vpu_dec_create_bufs(struct file *file, void *priv, >>>>> + struct v4l2_create_buffers *create) >>>>> +{ >>>>> + struct v4l2_format *f = &create->format; >>>>> + >>>>> + if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) >>>>> + return -ENOTTY; >>>> >>>> Huh? Why is this needed? >>> >>> Minimally a comment should be added. The why is that we support CREATE_BUF for >>> OUTPUT queue (bitstream) but not for CAPTURE queues. This is simply not >>> supported by Wave5 firmware. Do you have any suggestion how this asymmetry can >>> be implemented better ? >> >> Certainly not with ENOTTY: the ioctl exists, it is just not supported for >> CAPTURE queues. >> >> How about -EPERM? And document this error as well in the VIDIOC_CREATE_BUFS >> documentation. And you want a dev_dbg here too. > > The suggestion cannot be used since there is documentation for that one already, > and it does not match "unsupported". > > "Permission denied. Can be returned if the device needs write permission, or > some special capabilities is needed (e. g. root)" > > What about using the most logical error code, which name is actually obvious, > like ENOTSUP ? > > #define ENOTSUPP 524 /* Operation is not supported */ > Let's go with EOPNOTSUPP. That seems to be the more commonly used error code in drivers. >> >> So I would propose that EPERM is returned if CREATE_BUFS is only supported >> for for one of the two queues of an M2M device. > > Note that userspace does not care of the difference between an ioctl not being > implemented at all or not being implement for one queue. GStreamer have been > testing with both queue type for couple of years now. Adding this distinction is > just leaking an implementation details to userspace. I'm fine to just do what > you'd like, just stating the obvious that while it may look logical inside the > kernel, its a bit of a non-sense for our users. I don't agree with that. If an ioctl returns ENOTTY, then userspace can be certain that that ioctl is not implemented for the given file descriptor. That's not the case here: it is implemented, the operation is just not supported for one of the queues. Regards, Hans
On Wed, Sep 27, 2023 at 09:19:46AM +0200, Hans Verkuil wrote: > On 27/09/2023 01:29, Nicolas Dufresne wrote: > > Le vendredi 22 septembre 2023 à 09:33 +0200, Hans Verkuil a écrit : > >> On 21/09/2023 21:11, Nicolas Dufresne wrote: > >>> Le mercredi 20 septembre 2023 à 17:13 +0200, Hans Verkuil a écrit : > >>>> On 15/09/2023 23:11, Sebastian Fricke wrote: > >>>>> From: Nas Chung <nas.chung@chipsnmedia.com> > >>>>> > >>>>> Add the decoder and encoder implementing the v4l2 > >>>>> API. This patch also adds the Makefile and the VIDEO_WAVE_VPU config > >>>>> > >>>>> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com> > >>>>> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > >>>>> Signed-off-by: Robert Beckett <bob.beckett@collabora.com> > >>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> > >>>>> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> > >>>>> --- > >>>>> drivers/media/platform/chips-media/Kconfig | 1 + > >>>>> drivers/media/platform/chips-media/Makefile | 1 + > >>>>> drivers/media/platform/chips-media/wave5/Kconfig | 12 + > >>>>> drivers/media/platform/chips-media/wave5/Makefile | 10 + > >>>>> .../platform/chips-media/wave5/wave5-helper.c | 196 ++ > >>>>> .../platform/chips-media/wave5/wave5-helper.h | 30 + > >>>>> .../platform/chips-media/wave5/wave5-vpu-dec.c | 1965 ++++++++++++++++++++ > >>>>> .../platform/chips-media/wave5/wave5-vpu-enc.c | 1825 ++++++++++++++++++ > >>>>> .../media/platform/chips-media/wave5/wave5-vpu.c | 331 ++++ > >>>>> .../media/platform/chips-media/wave5/wave5-vpu.h | 83 + > >>>>> 10 files changed, 4454 insertions(+) > >>>>> > >> > >> <snip> > >> > >>>>> +static int wave5_vpu_dec_set_eos_on_firmware(struct vpu_instance *inst) > >>>>> +{ > >>>>> + int ret; > >>>>> + > >>>>> + ret = wave5_vpu_dec_update_bitstream_buffer(inst, 0); > >>>>> + if (ret) { > >>>>> + dev_err(inst->dev->dev, > >>>>> + "Setting EOS for the bitstream, fail: %d\n", ret); > >>>> > >>>> Is this an error due to a driver problem, or because a bad bitstream is > >>>> fed from userspace? In the first case, dev_err would be right, in the > >>>> second dev_dbg would be more appropriate. Bad userspace input should not > >>>> spam the kernel log in general. > >>> > >>> Its the first. To set the EOS flag, a command is sent to the firmware. That > >>> command may never return (timeout) or may report an error. For this specific > >>> command, if that happens we are likely facing firmware of driver problem (or > >>> both). > >> > >> OK, I'd add that as a comment here as this is unexpected behavior. > >> > >>> > >>>> > >>>>> + return ret; > >>>>> + } > >>>>> + return 0; > >>>>> +} > >> > >> <snip> > >> > >>>>> +static int wave5_vpu_dec_create_bufs(struct file *file, void *priv, > >>>>> + struct v4l2_create_buffers *create) > >>>>> +{ > >>>>> + struct v4l2_format *f = &create->format; > >>>>> + > >>>>> + if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) > >>>>> + return -ENOTTY; > >>>> > >>>> Huh? Why is this needed? > >>> > >>> Minimally a comment should be added. The why is that we support CREATE_BUF for > >>> OUTPUT queue (bitstream) but not for CAPTURE queues. This is simply not > >>> supported by Wave5 firmware. Do you have any suggestion how this asymmetry can > >>> be implemented better ? > >> > >> Certainly not with ENOTTY: the ioctl exists, it is just not supported for > >> CAPTURE queues. > >> > >> How about -EPERM? And document this error as well in the VIDIOC_CREATE_BUFS > >> documentation. And you want a dev_dbg here too. > > > > The suggestion cannot be used since there is documentation for that one already, > > and it does not match "unsupported". > > > > "Permission denied. Can be returned if the device needs write permission, or > > some special capabilities is needed (e. g. root)" > > > > What about using the most logical error code, which name is actually obvious, > > like ENOTSUP ? > > > > #define ENOTSUPP 524 /* Operation is not supported */ > > > > Let's go with EOPNOTSUPP. That seems to be the more commonly used error > code in drivers. Hi Hans, Sorry to belabour this issue but when I change the return value to EOPNOTSUPP, it now causes v4l2-compliance to fail because v4l2-test-buffers.cpp expects ENOTTY if CREATE_BUFS is not supported. We didn't get this warning before because there was a typo in the buffer check and it was only checking for single-planar buffers. How would you prefer to handle this? The options seem like keep ENOTTY in this driver or patch v4l2-compliance to warn if it also receives EOPNOTSUPP? > > >> > >> So I would propose that EPERM is returned if CREATE_BUFS is only supported > >> for for one of the two queues of an M2M device. > > > > Note that userspace does not care of the difference between an ioctl not being > > implemented at all or not being implement for one queue. GStreamer have been > > testing with both queue type for couple of years now. Adding this distinction is > > just leaking an implementation details to userspace. I'm fine to just do what > > you'd like, just stating the obvious that while it may look logical inside the > > kernel, its a bit of a non-sense for our users. > > I don't agree with that. If an ioctl returns ENOTTY, then userspace can be certain > that that ioctl is not implemented for the given file descriptor. That's not the case > here: it is implemented, the operation is just not supported for one of the queues. > > Regards, > > Hans
Hi Deb, On 03/10/2023 01:51, Deborah Brouwer wrote: > On Wed, Sep 27, 2023 at 09:19:46AM +0200, Hans Verkuil wrote: >> On 27/09/2023 01:29, Nicolas Dufresne wrote: >>> Le vendredi 22 septembre 2023 à 09:33 +0200, Hans Verkuil a écrit : >>>> On 21/09/2023 21:11, Nicolas Dufresne wrote: >>>>> Le mercredi 20 septembre 2023 à 17:13 +0200, Hans Verkuil a écrit : >>>>>> On 15/09/2023 23:11, Sebastian Fricke wrote: >>>>>>> From: Nas Chung <nas.chung@chipsnmedia.com> >>>>>>> >>>>>>> Add the decoder and encoder implementing the v4l2 >>>>>>> API. This patch also adds the Makefile and the VIDEO_WAVE_VPU config >>>>>>> >>>>>>> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com> >>>>>>> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> >>>>>>> Signed-off-by: Robert Beckett <bob.beckett@collabora.com> >>>>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> >>>>>>> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> >>>>>>> --- >>>>>>> drivers/media/platform/chips-media/Kconfig | 1 + >>>>>>> drivers/media/platform/chips-media/Makefile | 1 + >>>>>>> drivers/media/platform/chips-media/wave5/Kconfig | 12 + >>>>>>> drivers/media/platform/chips-media/wave5/Makefile | 10 + >>>>>>> .../platform/chips-media/wave5/wave5-helper.c | 196 ++ >>>>>>> .../platform/chips-media/wave5/wave5-helper.h | 30 + >>>>>>> .../platform/chips-media/wave5/wave5-vpu-dec.c | 1965 ++++++++++++++++++++ >>>>>>> .../platform/chips-media/wave5/wave5-vpu-enc.c | 1825 ++++++++++++++++++ >>>>>>> .../media/platform/chips-media/wave5/wave5-vpu.c | 331 ++++ >>>>>>> .../media/platform/chips-media/wave5/wave5-vpu.h | 83 + >>>>>>> 10 files changed, 4454 insertions(+) >>>>>>> >>>> >>>> <snip> >>>> >>>>>>> +static int wave5_vpu_dec_set_eos_on_firmware(struct vpu_instance *inst) >>>>>>> +{ >>>>>>> + int ret; >>>>>>> + >>>>>>> + ret = wave5_vpu_dec_update_bitstream_buffer(inst, 0); >>>>>>> + if (ret) { >>>>>>> + dev_err(inst->dev->dev, >>>>>>> + "Setting EOS for the bitstream, fail: %d\n", ret); >>>>>> >>>>>> Is this an error due to a driver problem, or because a bad bitstream is >>>>>> fed from userspace? In the first case, dev_err would be right, in the >>>>>> second dev_dbg would be more appropriate. Bad userspace input should not >>>>>> spam the kernel log in general. >>>>> >>>>> Its the first. To set the EOS flag, a command is sent to the firmware. That >>>>> command may never return (timeout) or may report an error. For this specific >>>>> command, if that happens we are likely facing firmware of driver problem (or >>>>> both). >>>> >>>> OK, I'd add that as a comment here as this is unexpected behavior. >>>> >>>>> >>>>>> >>>>>>> + return ret; >>>>>>> + } >>>>>>> + return 0; >>>>>>> +} >>>> >>>> <snip> >>>> >>>>>>> +static int wave5_vpu_dec_create_bufs(struct file *file, void *priv, >>>>>>> + struct v4l2_create_buffers *create) >>>>>>> +{ >>>>>>> + struct v4l2_format *f = &create->format; >>>>>>> + >>>>>>> + if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) >>>>>>> + return -ENOTTY; >>>>>> >>>>>> Huh? Why is this needed? >>>>> >>>>> Minimally a comment should be added. The why is that we support CREATE_BUF for >>>>> OUTPUT queue (bitstream) but not for CAPTURE queues. This is simply not >>>>> supported by Wave5 firmware. Do you have any suggestion how this asymmetry can >>>>> be implemented better ? >>>> >>>> Certainly not with ENOTTY: the ioctl exists, it is just not supported for >>>> CAPTURE queues. >>>> >>>> How about -EPERM? And document this error as well in the VIDIOC_CREATE_BUFS >>>> documentation. And you want a dev_dbg here too. >>> >>> The suggestion cannot be used since there is documentation for that one already, >>> and it does not match "unsupported". >>> >>> "Permission denied. Can be returned if the device needs write permission, or >>> some special capabilities is needed (e. g. root)" >>> >>> What about using the most logical error code, which name is actually obvious, >>> like ENOTSUP ? >>> >>> #define ENOTSUPP 524 /* Operation is not supported */ >>> >> >> Let's go with EOPNOTSUPP. That seems to be the more commonly used error >> code in drivers. > > Hi Hans, > > Sorry to belabour this issue but when I change the return value > to EOPNOTSUPP, it now causes v4l2-compliance to fail because > v4l2-test-buffers.cpp expects ENOTTY if CREATE_BUFS is not supported. > > We didn't get this warning before because there was a typo in the > buffer check and it was only checking for single-planar buffers. > > How would you prefer to handle this? The options seem like > keep ENOTTY in this driver or > patch v4l2-compliance to warn if it also receives EOPNOTSUPP? You patch v4l2-compliance. It makes sense: we're making a uAPI modification, so that implies changes to v4l2-compliance. So v4l2-compliance needs to understand EOPNOTSUPP for CREATE_BUFS: if it is returned it has to check that it is used correctly: so there has to be at least one buffer type for which CREATE_BUFS actually works. In other words, v4l2-compliance must check that EOPNOTSUPP isn't used as a replacement for ENOTTY. This can be done in testReqBufs(). Regards, Hans
The Wave5 codec driver is a stateful encoder/decoder. It is found on the J721S2 SoC, JH7100 SoC, ssd202d SoC. Etc. But current test report is based on J721S2 SoC and pre-silicon FPGA. The driver currently supports V4L2_PIX_FMT_HEVC, V4L2_PIX_FMT_H264 for both encoding and decoding. This driver has so far been tested on J721S2 EVM board and pre-silicon FPGA. Testing on J721S2 EVM board shows it working fine both decoder and encoder. The driver is successfully working with gstreamer v4l2 good-plugin without any modification. We have based the tree on top of the latest https://git.linuxtv.org/media_tree.git repository. M2M framework modification: =========================== (Patch 1 & 2) Added a set of changes to enable ignoring the streaming state of one or more queues while checking whether a new job can be inserted into the ready queue. The use-case we encountered for this is a stateful decoder/encoder chip with a tight connection to a firmware. On that firmware the bitstream is first analyzed and the firmware produces output indicating the requirements for the output of the decode (e.g. the buffers of the CAPTURE queue). We want to be able to perform this action within the M2M job workflow in order to rely on the concurrency safety provided by the M2M jobs. v4l2-compliance results: ======================== v4l2-compliance 1.25.0-5092, 64 bits, 64-bit time_t Total for wave5-dec device /dev/video0: 45, Succeeded: 45, Failed: 0, Warnings: 0 Total for wave5-enc device /dev/video1: 45, Succeeded: 45, Failed: 0, Warnings: 0 Fluster test results: ===================== Running test suite JCT-VC-HEVC_V1 with decoder GStreamer-H.265-V4L2-Gst1.0 Using 1 parallel job(s) Ran 133/147 tests successfully in 95.898 secs (1 test fails because of a missing frame and slight corruption, 2 tests fail because of sizes which are incompatible with the IP, 11 tests fail because of unsupported 10 bit format) Running test suite JVT-AVC_V1 with decoder GStreamer-H.264-V4L2-Gst1.0 Using 1 parallel job(s) Ran 78/135 tests successfully in 57.198 secs (57 fail because the hardware is unable to decode MBAFF / FMO / Field / Extended profile streams.) Encoder testing: ================ Among other tests the driver is able to produce valid output with the following test: `gst-launch-1.0 videotestsrc num-buffers=300 ! v4l2h264enc ! h264parse ! qtmux ! filesink location=test.mp4` Changes since v11: ================== * Major rework of the decoder - Fix concurrency issues by moving the commands that invoke actions on the firmware into the device_run function from M2M, effectively causing M2M to take care of the concurrency via its job queue. - In order to do that device_run needs to be able to run before both queues are ready, as a sequence needs to be initialized on the firmware to get the required buffer count & communicate the result back to userspace, thus we added a routine to allow jobs to be queued in M2M with only one of two queues being started. (See: "Add ignore_streaming flag to M2M" series) - Add support for more output formats (YUV422P, YUV422M, NV16, NV16M, NV61, NV61M) - Add proper state switch function to verify state switches - Simplify the queue_setup function and move the decoder opening to STREAMON as request in the review of V11 - Enable handling dynamic resolution change and seeking - Remove thumbnail mode * Similar reworks on the encoder - Move encoding into device_run and encoder opening + sequence intialization to STREAMON, this change simplifies the encoder as it previously was able to run multiple encode jobs simultaneously * Remove unused configurations and support for untested hardware - Remove the ability to configure the endianess of memory writes as only a single hardware can be tested so far - Remove the ability to configure the bitstream_mode, as the driver is currently hardcoded for the INTERRUPT mode - Remove support for all CODECS, that were not tested (everything besides H264/HEVC encoder + decoder) - The encoder currently contains a lot of configurable parameter, which are hard-coded in the `wave5_set_enc_openparam` function, remove all parameters which aren't currently specified in that configuration - Remove unused rotation and mirroring options from the decoder * Add FLUSH_FIRMWARE firmware command * Refactoring * Add wrappers for frequently used routines or to make the code more descriptive - Wrapper for firmware command calling `send_firmware_command` - Wrappers to intialize the sequence and to set up the framebuffers in the decoder and encoder - Using more general kernel functions and macros in various places * Add Macros for constant values and bit shifts * Fix typos and improve comments * Fix bug with M2M instance stored in the driver instance, multiple simultaneous instances would overwrite the M2M handler of each other. Use a M2M handler per device instead to avoid overwriting. * Adjust TRACE_PATH in the coda directory as highlighted in the review of V11 * Applied requested changes from review to the devicetree bindings file (the bindings check didn't return any warnings or errors) Changes since v10: ================== * Remove structure member from the encoder and decoder output info structs, that have assigned values from the registers but aren't used in the driver, add comments to describe the register values in the register definitions * Fix issue with decoding videos with a dimension where the height is not a multiple of 16 (270, 360, 540, 1024 etc.) * Fix incorrect variable format identifiers in printks * Use debug logs in loops to avoid flooding the message log * Use the swap() function instead of manual swapping of two values * Add extended controls for the encoder * Fix control flow issue while handling bitstream buffers, where an error while writing the source buffer into the hardware ring buffer would result in skipping the problematic buffer, which in turn causes a reordering of source buffers * Use the rectangle format as described by the hardware, the hardware uses for rectangles like the display rectangle 4 offsets (top, bottom, left, right), which depict the offset from the respective edge. Use this format instead of implicitly converting the bottom and right attributes to width and height attributes. * Return an error upon reading the sequence header while STREAMON * Squash the VDI and the VPUAPI layer commits as they had circular dependencies changes since v9: ================= * Move from staging to the media directory * Move coda driver to sub-directory * Fixes: * Use platform_get_irq instead of platform_get_resource to fetch the IRQ * General cleanups: * Add missing error messages to error conditions * Improve messages/variable names/comments, align parameter names across the driver * Use macros instead of magic numbers in multiple occassions * Reduce code duplication in multiple places * Fix whitespace, newline and tab alignment issues * Remove unused struct fields & commented out code * Convert signed integers to unsigned if signed is not necessary * Convert int/unsigned int to s32/u32, when the variable is assigned to the return of a register read or provided as a parameter for a register write (and vice versa) * Fix incorrect bitwise operators where logical operators are appropriate * Multiple smaller changes * Generalization: * Add new helper file providing generalized routines for vpu-dec & vpu-enc * Generalize luma & chroma table size calculation and stride calculation * Resource cleanup and error handling: * Add error handling to all calls with ignored return codes * Handle DMA resource cleanup properly * Fix insufficient instance cleanup while opening dec/enc Changes since v8: ================= * add 'wave5' to DEV_NAME * update to support Multi-stream * update to support loop test/dynamic resolution change * remove unnecessary memset, g_volatile, old version option Changes since v7: ================= * update v4l2-compliance test report * fix build error on linux-kernel 5.18.0-rc4 Changes since v6: ================= * update TODO file * get sram info from device tree Changes since v5: ================= * support NV12/NV21 pixelformat for encoder and decoder * handle adnormal exit and EOS Changes since v4: ================= * refactor functions in wave5-hw and fix bug reported by Daniel Palmer * rename functions and variables to better names * change variable types such as replacing s32 with u32 and int with bool * as appropriate Changes since v3: ================= * Fixing all issues commented by Dan Carpenter * Change file names to have wave5- prefix * In wave5_vpu_probe, enable the clocks before reading registers, as * commented from Daniel Palmer * Add more to the TODO list, Changes since v2: ================= Main fixes includes: * change the yaml and dirver code to support up to 4 clks (instead of * one) * fix Kconfig format * remove unneeded cast, * change var types * change var names, func names * checkpatch fixes Changes since v1: ================= Fix changes due to comments from Ezequiel and Dan Carpenter. Main fixes inclueds: * move all files to one dir 'wave5' * replace private error codes with standard error codes * fix extra spaces * various checkpatch fixes * replace private 'DPRINTK' macro with standard 'dev_err/dbg ..' * fix error handling * add more possible fixes to the TODO file To: Mauro Carvalho Chehab <mchehab@kernel.org> To: Hans Verkuil <hverkuil@xs4all.nl> To: Philipp Zabel <p.zabel@pengutronix.de> To: Shawn Guo <shawnguo@kernel.org> To: Sascha Hauer <s.hauer@pengutronix.de> To: Pengutronix Kernel Team <kernel@pengutronix.de> To: Fabio Estevam <festevam@gmail.com> To: NXP Linux Team <linux-imx@nxp.com> To: Jackson Lee <jackson.lee@chipsnmedia.com> To: Rob Herring <robh+dt@kernel.org> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> To: Conor Dooley <conor+dt@kernel.org> Cc: linux-media@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: devicetree@vger.kernel.org Cc: Nicolas Dufresne <nicolas.dufresne@collabora.com> Cc: Robert Beckett <bob.beckett@collabora.com> Cc: Nas Chung <nas.chung@chipsnmedia.com> CC: kernel@collabora.com Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com> --- Nas Chung (2): media: chips-media: wave5: Add vpuapi layer media: chips-media: wave5: Add the v4l2 layer Robert Beckett (2): dt-bindings: media: wave5: add yaml devicetree bindings media: chips-media: wave5: Add wave5 driver to maintainers file Sebastian Fricke (3): media: v4l2: Add ignore_streaming flag media: v4l2: Allow M2M job queuing w/o streaming CAP queue media: platform: chips-media: Move Coda to separate folder .../devicetree/bindings/media/cnm,wave5.yaml | 66 + MAINTAINERS | 10 +- drivers/media/platform/chips-media/Kconfig | 18 +- drivers/media/platform/chips-media/Makefile | 6 +- drivers/media/platform/chips-media/coda/Kconfig | 18 + drivers/media/platform/chips-media/coda/Makefile | 6 + .../platform/chips-media/{ => coda}/coda-bit.c | 0 .../platform/chips-media/{ => coda}/coda-common.c | 0 .../platform/chips-media/{ => coda}/coda-gdi.c | 0 .../platform/chips-media/{ => coda}/coda-h264.c | 0 .../platform/chips-media/{ => coda}/coda-jpeg.c | 0 .../platform/chips-media/{ => coda}/coda-mpeg2.c | 0 .../platform/chips-media/{ => coda}/coda-mpeg4.c | 0 .../media/platform/chips-media/{ => coda}/coda.h | 0 .../platform/chips-media/{ => coda}/coda_regs.h | 0 .../platform/chips-media/{ => coda}/imx-vdoa.c | 0 .../platform/chips-media/{ => coda}/imx-vdoa.h | 0 .../media/platform/chips-media/{ => coda}/trace.h | 2 +- drivers/media/platform/chips-media/wave5/Kconfig | 12 + drivers/media/platform/chips-media/wave5/Makefile | 10 + .../platform/chips-media/wave5/wave5-helper.c | 196 ++ .../platform/chips-media/wave5/wave5-helper.h | 30 + .../media/platform/chips-media/wave5/wave5-hw.c | 2553 ++++++++++++++++++++ .../platform/chips-media/wave5/wave5-regdefine.h | 732 ++++++ .../media/platform/chips-media/wave5/wave5-vdi.c | 208 ++ .../media/platform/chips-media/wave5/wave5-vdi.h | 35 + .../platform/chips-media/wave5/wave5-vpu-dec.c | 1965 +++++++++++++++ .../platform/chips-media/wave5/wave5-vpu-enc.c | 1825 ++++++++++++++ .../media/platform/chips-media/wave5/wave5-vpu.c | 331 +++ .../media/platform/chips-media/wave5/wave5-vpu.h | 83 + .../platform/chips-media/wave5/wave5-vpuapi.c | 958 ++++++++ .../platform/chips-media/wave5/wave5-vpuapi.h | 874 +++++++ .../platform/chips-media/wave5/wave5-vpuconfig.h | 77 + .../platform/chips-media/wave5/wave5-vpuerror.h | 292 +++ drivers/media/platform/chips-media/wave5/wave5.h | 116 + drivers/media/v4l2-core/v4l2-mem2mem.c | 9 +- include/media/v4l2-mem2mem.h | 17 + 37 files changed, 10424 insertions(+), 25 deletions(-) --- base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d change-id: 20230915-wave5_v12_on_media_master-ac30ff2c4c00 Best regards,