linux-kernel - Re: [PATCH v10 3/5] media: Documentation: aspeed-video: Add user documentation for the aspeed-video driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <3c461dbe-7621-e4e6-c55e-6352119258f8@aspeedtech.com>
Date:   Thu, 27 Oct 2022 10:11:24 +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


On 2022/10/27 上午 03:02, Nicolas Dufresne wrote:
> Le mercredi 26 octobre 2022 à 10:42 +0800, Jammy Huang a écrit :
>> 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.
> Sorry about that, I have skipped too much. The approach seems fair then, can you
> state in the doc that these control applies to V4L2_PIX_FMT_AJPG in some way ?
> (just a little cross-reference can help). The confusion with normal JPEG is
> easy.
>
> thanks for you patience,
> Nicolas

Sure, I will add the words below in next patch.

"VIDIOC_S_FMT can be used to choose which format you want. V4L2_PIX_FMT_JPEG

stands for JPEG JFIF standard mode; V4L2_PIX_FMT_AJPG stands for ASPEED

proprietary mode."


Thanks for your help.

>>> 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