Re: [PATCH v3, 00/10] Enable two H264 encoder core on MT8195

[Irui Wang <irui.wang@mediatek.com>]

Dear Angelo,

Thanks for your comments.
On Fri, 2022-03-18 at 15:04 +0100, AngeloGioacchino Del Regno wrote:
> Il 17/03/22 09:22, Irui Wang ha scritto:
> > MT8195 has two H264 encoder cores, they have their own power-
> > domains,
> > clocks, interrupts, register base. The two H264 encoder cores can
> > work
> > together to achieve higher performance, it's a core mode called
> > frame-racing, one core has 4K@30fps performance, two cores can
> > achieve
> > 4K@60fps.
> > The two encoder core encoding process looks like this:
> > 
> >      VENC Core0: frm#0....frm#2....frm#4....
> >      VENC Core1: ..frm#1....frm#3....frm#5....
> > 
> > This series of patches are used to enable the two H264 encoder
> > cores,
> > encoding process will be changed:
> > As-Is: Synchronous
> > V4L2_VIDIOC_QBUF#0 --> device_run(triger encoder) --> wait encoder
> > IRQ -->
> > encoding done with result --> job_finish
> > V4l2_VIDIOC_QBUF#1 --> device_run(triger encoder) --> wait encoder
> > IRQ -->
> > encoding done with result --> job_finish
> > ...
> > 
> > To-Be: Asynchronous
> > V4L2_VIDIOC_QBUF#0 --> device_run(triger encoder) --> job_finish
> > ..V4l2_VIDIOC_QBUF#1 --> device_run(triger encoder) --> job_finish
> > (venc core0 may encode done here, done the encoding result to
> > client)
> > V4L2_VIDIOC_QBUF#2 --> device_run(triger encoder) --> job_finish.
> > 
> > There is no "wait encoder IRQ" synchronous call during frame-racing 
> > mode
> > encoding process, it can full use the two encoder cores to achieve
> > higher
> > performance.
> > 
> 
> Hello Irui,
> like v1 and v2, also this version of the series will crash the kernel
> when
> trying to encode.

As mentioned before, dual core encoding need more encoding working
buffers in patch-8:
media: mtk-vcodec: Add more extra processing for venc_multi_core mode
a new structure, struct venc_multi_core_vsi defined for it,

encoding processing like this: 
kernel --> ipi message --> scp firmware(userspace), vsi is a share
memory to transfer data between kernel and user, so, the definition of
vsi must be the same in kernel and userspace. otherwise, errors will be
occurred, for example, kernel crash.

When we enable dual core encoding in this patch, the
venc_multi_core_vsi used in kernel, but scp(userspace) still use the
older one vsi(struct venc_h264_vsi), the crash happened.

Maybe we would find out how to resolve the limitations, many thanks for
your review.
> 
> Before pushing the next series, you shall ensure that it is not
> crashing
> the entire kernel, hence, please test your code with gstreamer.
> 
> 
> Please use the following pipeline to test:
> 
> gst-launch-1.0 videotestsrc num-buffers=100 ! v4l2h264enc ! queue !
> h264parse ! 
> qtmux ! filesink location=test.mov
> 
> 
> Regards,
> Angelo

Thanks
Best Regards