Hi Hans,

On Wed, 2016-05-04 at 10:25 +0200, Hans Verkuil wrote:

On 05/04/2016 10:19 AM, tiffany lin wrote:

quoted

Hi Hans,

On Wed, 2016-05-04 at 09:32 +0200, Hans Verkuil wrote:

quoted

Hi Tiffany,

On 05/03/2016 12:11 PM, Tiffany Lin wrote:

quoted

==============
 Introduction
==============

The purpose of this series is to add the driver for video codec hw embedded in the Mediatek's MT8173 SoCs.
Mediatek Video Codec is able to handle video encoding of in a range of formats.

This patch series also include VPU driver. Mediatek Video Codec driver rely on VPU driver to load,
communicate with VPU.

Internally the driver uses videobuf2 framework and MTK IOMMU and MTK SMI both have been merged in v4.6-rc1.
This patch series need [PATCH v15 8/8] memory: mtk-smi: export mtk_smi_larb_get/put[1] to build as module

[1]http://lists.infradead.org/pipermail/linux-mediatek/2016-April/005173.html

==================
 Device interface
==================

In principle the driver bases on v4l2 memory-to-memory framework:
it provides a single video node and each opened file handle gets its own private context with separate
buffer queues. Each context consist of 2 buffer queues: OUTPUT (for source buffers, i.e. raw video
frames) and CAPTURE (for destination buffers, i.e. encoded video frames).

==============================
 VPU (Video Processor Unit)
==============================
The VPU driver for hw video codec embedded in Mediatek's MT8173 SOCs.
It is able to handle video decoding/encoding in a range of formats.
The driver provides with VPU firmware download, memory management and the communication interface between CPU and VPU.
For VPU initialization, it will create virtual memory for CPU access and physical address for VPU hw device access. 
When a decode/encode instance opens a device node, vpu driver will download vpu firmware to the device.
A decode/encode instant will decode/encode a frame using VPU interface to interrupt vpu to handle decoding/encoding jobs.

Please have a look at the code and comments will be very much appreciated.

Change in v10:
1. Fix smatch/sparse error message
2. Add depends on ARM || ARM64 and MTK_IOMMU in Kconfig

I don't like the ARM or ARM64 dependency since that makes COMPILE_TEST harder.

I removed it here and for the VPU and everything still compiles fine with
sparse and smatch. So I'll drop those dependencies.

Why is the MTK_IOMMU dependency needed? Just curious.

This is because we need MTK_IOMMU module to get dma continuous memory
for HW.

quoted

Note that sparse still complains about this:

media-git/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c:261:24: warning: incorrect type in assignment (different base types)

I think this code should be rewritten. Something like this would work
(if I understand the code correctly):

u32 tag = (bs_hdr_len << 5) | 0x10 | not_key;

ac_tag[0] = tag & 0xff;
ac_tag[1] = (tag >> 8) & 0xff;
ac_tag[2] = (tag >> 16) & 0xff;

It's perhaps a bit more verbose, but a lot easier to understand.

quoted

I'm accepting this driver but I'll remove the ARM || ARM64 dependency. Can
you post a follow-up patch for this sparse warning?

Got it. We will provide a patch to fix this.
I was curious why my sparse do not show this warning.
Is there any parameter I need to set except C=1?

I'm running it with these parameters: C=2 CF="-D__CHECK_ENDIAN__"

I suspect it is the CHECK_ENDIAN flag that does it.

Got it, thanks for the information.

best regards,
Tiffany

Regards,

	Hans