Date:   Wed, 26 Oct 2022 10:42:09 +0800
From:   Jammy Huang <jammy_huang@...eedtech.com>
To:     Nicolas Dufresne <nicolas.dufresne@...labora.com>,
        <eajames@...ux.ibm.com>, <mchehab@...nel.org>, <joel@....id.au>,
        <andrew@...id.au>, <hverkuil-cisco@...all.nl>,
        <laurent.pinchart@...asonboard.com>, <xavier.roumegue@....nxp.com>,
        <ezequiel@...guardiasur.com.ar>, <stanimir.varbanov@...aro.org>,
        <sakari.ailus@...ux.intel.com>, <ming.qian@....com>,
        <andrzej.p@...labora.com>, <linux-media@...r.kernel.org>,
        <openbmc@...ts.ozlabs.org>, <linux-arm-kernel@...ts.infradead.org>,
        <linux-aspeed@...ts.ozlabs.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v10 3/5] media: Documentation: aspeed-video: Add user
 documentation for the aspeed-video driver

Hi Nicolas,

Thanks for your comments.

On 2022/10/25 下午 09:18, Nicolas Dufresne wrote:
> Hi Jammy,
>
> thanks for the addition.
>
> Le mardi 25 octobre 2022 à 17:02 +0800, Jammy Huang a écrit :
>> Add user documentation for the aspeed-video driver.
>>
>> Signed-off-by: Jammy Huang <jammy_huang@...eedtech.com>
>> ---
>> v10:
>>    - new
>> ---
>>   .../media/drivers/aspeed-video.rst            | 61 +++++++++++++++++++
>>   .../userspace-api/media/drivers/index.rst     |  1 +
>>   2 files changed, 62 insertions(+)
>>   create mode 100644 Documentation/userspace-api/media/drivers/aspeed-video.rst
>>
>> diff --git a/Documentation/userspace-api/media/drivers/aspeed-video.rst b/Documentation/userspace-api/media/drivers/aspeed-video.rst
>> new file mode 100644
>> index 000000000000..798a2588b175
>> --- /dev/null
>> +++ b/Documentation/userspace-api/media/drivers/aspeed-video.rst
>> @@ -0,0 +1,61 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +.. include:: <isonum.txt>
>> +
>> +ASPEED video driver
>> +===================
>> +
>> +ASPEED Video Engine found on AST2400/2500/2600 SoC supports high performance
>> +video compressions with a wide range of video quality and compression ratio
>> +options. The adopted compressing algorithm is a modified JPEG algorithm.
>> +
>> +There are 2 types of compressions in this IP.
>> +
>> +* JPEG JFIF standard mode: for single frame and management compression
>> +* ASPEED proprietary mode: for multi-frame and differential compression.
>> +  Support 2-pass (high quality) video compression scheme (Patent pending by
>> +  ASPEED). Provide visually lossless video compression quality or to reduce
>> +  the network average loading under intranet KVM applications.
> I think some of the information disclosed in the following quote could be
> summarized. Notably the part about the extra buffers.
>
>     Aspeed JPEG Format requires an additional buffer, called bcd, to store
>     the information about which macro block in the new frame is different
>     from the previous one.
>     
>     To have bcd correctly working, we need to swap the buffers for src0/1 to
>     make src1 refer to previous frame and src0 to the coming new frame.
>     
> But before I push you this route, have you considered using a dedicated pixel
> format instead ? Here's my thinking, the output of the JPEG encoder is no longer
> "compatible" (or at least won't yield the expected images) if used with a normal
> JPEG decoder. By differentiating these two as dedicated formats, you will only need
> 1 vendor control, and you avoid the potential risk of software bugs mixing them up.
> Also note that there is other JPEG based vendor formats that exist in V4L2.
>
> Let me know what do you think ?

Yes, I also add a dedicated formats, V4L2_PIX_FMT_AJPG, in this series. 
In [PATCH v10 1/5]

media: v4l: Add definition for the Aspeed JPEG format, I add the 
description in pixfmt-reserved.rst.

After this series applied, the users can choose either of these two 
formats by VIDIOC_S_FMT as

per their preference.

>
> Nicolas
>
>> +
>> +More details on the ASPEED video hardware operations can be found in
>> +*chapter 6.2.16 KVM Video Driver* of SDK_User_Guide which available on
>> +AspeedTech-BMC/openbmc/releases.
>> +
>> +The ASPEED video driver implements the following driver-specific control:
>> +
>> +``V4L2_CID_ASPEED_HQ_MODE``
>> +-------------------------------
>> +    Enable/Disable ASPEED's High quality mode. This is a private control
>> +    that can be used to enable high quality for aspeed proprietary mode.
>> +
>> +.. flat-table::
>> +    :header-rows:  0
>> +    :stub-columns: 0
>> +    :widths:       1 4
>> +
>> +    * - ``(0)``
>> +      - ASPEED HQ mode is disabled.
>> +    * - ``(1)``
>> +      - ASPEED HQ mode is enabled.
>> +
>> +``V4L2_CID_ASPEED_HQ_JPEG_QUALITY``
>> +-------------------------------
>> +    Define the quality of ASPEED's High quality mode. This is a private control
>> +    that can be used to decide compression quality if High quality mode enabled
>> +    . Higher the value, better the quality and bigger the size.
>> +
>> +.. flat-table::
>> +    :header-rows:  0
>> +    :stub-columns: 0
>> +    :widths:       1 4
>> +
>> +    * - ``(1)``
>> +      - minimum
>> +    * - ``(12)``
>> +      - maximum
>> +    * - ``(1)``
>> +      - step
>> +    * - ``(1)``
>> +      - default
>> +
>> +**Copyright** |copy| 2022 ASPEED Technology Inc.
>> diff --git a/Documentation/userspace-api/media/drivers/index.rst b/Documentation/userspace-api/media/drivers/index.rst
>> index 32f82aed47d9..46a494e00b72 100644
>> --- a/Documentation/userspace-api/media/drivers/index.rst
>> +++ b/Documentation/userspace-api/media/drivers/index.rst
>> @@ -31,6 +31,7 @@ For more details see the file COPYING in the source distribution of Linux.
>>   	:maxdepth: 5
>>   	:numbered:
>>   
>> +	aspeed-video
>>   	ccs
>>   	cx2341x-uapi
>>   	dw100

-- 
Best Regards
Jammy

Powered by blists - more mailing lists