Re: [PATCH v5 00/10] add aspeed-jpeg support for aspeed-video

Jammy Huang
2021-12-02T01:32:24Z

Hi Hans,

Thanks for your review and suggestions.

On 2021/11/26 下午 05:51, Hans Verkuil wrote:
> On 26/11/2021 09:17, Jammy Huang wrote:
>> Hi Hans,
>>
>> Please refer to Page 132, 'Video stream data format – ASPEED mode compression' of
>> https://github.com/AspeedTech-BMC/openbmc/releases/download/v07.02/SDK_User_Guide_v07.02.pdf
>>
>> Is above information enough??
> Hmm, close. It would need some additional information, possibly documented in the source
> code:
>
> - What is the Frame Header? (the doc just says that it is different, but not what it actually is)
> - What is the EOI code? Is it equal to 'FEND'? (not quite clear to me)
>
> I think the documentation looks sufficient once those two items are addressed.
Information you mentioned will be included in next revision,
"SDK_User_Guide_v07.03.pdf".
The releases will be updated to
https://github.com/AspeedTech-BMC/openbmc/releases
>> Or maybe we can postpone this series until the source code for this format ready.
>> I think I can accomplish it by the end of this year.
> That's quite soon, so perhaps that would be the best approach. That way you also
> know exactly what information is missing from the SDK User Guide, and if that is
> documented, then this format can be used in the kernel.
>
> But several of the patches in the series are independent of the new jpeg format,
> so it might be useful to get those in first.

Understood. For the patches below, I will resend except for patch
5/6/8/10 which are closely
relative to aspeed jpeg format.

1. media: aspeed: move err-handling together to the bottom
2. media: aspeed: use v4l2_info/v4l2_warn/v4l2_dbg for log
3. media: aspeed: add more debug log messages
4. media: aspeed: refactor to gather format/compress settings
5. media: v4l: Add definition for the Aspeed JPEG format
6. media: v4l2-ctrls: Reserve controls for ASPEED
7. media: aspeed: use model-data
8. media: aspeed: Support aspeed mode to reduce compressed data
9. media: aspeed: add comments and macro
10.media: aspeed: Extend debug message

>
> Regards,
>
> Hans
>
>> On 2021/11/25 下午 10:31, Hans Verkuil wrote:
>>> Hi Jammy,
>>>
>>> I suggest posting a v3 that moves the patches that deal with the
>>> Aspeed JPEG format to the end of the series. That way I can easily
>>> merge the patches up to that point, and the Aspeed JPEG support can
>>> be handled separately.
>>>
>>> I'm not sure if it can be merged without that format being documented,
>>> either in pixfmt-reserved.rst, by implementing support for it in
>>> libv4lconvert from v4l-utils, or by providing a URL to some other
>>> publicly available source code. We really don't like adding pixel
>>> formats that cannot be interpreted by someone without access to the
>>> datasheets.
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>> On 18/11/2021 08:40, Jammy Huang wrote:
>>>> The aim of this series is to add aspeed-jpeg support for aspeed-video
>>>> driver.
>>>>
>>>> To achieve this major goal some refactors are included.
>>>>
>>>> In the last, debugfs information is also updated per this change.
>>>>
>>>> Changes in v5:
>>>> - Use model data to tell different soc
>>>>
>>>> Changes in v4:
>>>> - Add definition for the Aspeed JPEG format
>>>> - Reserve controls for ASPEED
>>>> - Use s_fmt to update format rather than new control
>>>> - Update aspeed hq quality range, 1 ~ 12
>>>>
>>>>
>>>> Jammy Huang (10):
>>>> media: aspeed: move err-handling together to the bottom
>>>> media: aspeed: use v4l2_info/v4l2_warn/v4l2_dbg for log
>>>> media: aspeed: add more debug log messages
>>>> media: aspeed: refactor to gather format/compress settings
>>>> media: v4l: Add definition for the Aspeed JPEG format
>>>> media: v4l2-ctrls: Reserve controls for ASPEED
>>>> media: aspeed: use model-data
>>>> media: aspeed: Support aspeed mode to reduce compressed data
>>>> media: aspeed: add comments and macro
>>>> media: aspeed: Extend debug message
>>>>
>>>> .../media/uapi/v4l/pixfmt-reserved.rst | 12 +
>>>> drivers/media/platform/aspeed-video.c | 534 ++++++++++++++----
>>>> drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
>>>> include/uapi/linux/aspeed-video.h | 15 +
>>>> include/uapi/linux/v4l2-controls.h | 5 +
>>>> include/uapi/linux/videodev2.h | 1 +
>>>> 6 files changed, 467 insertions(+), 101 deletions(-)
>>>> create mode 100644 include/uapi/linux/aspeed-video.h
>>>>
--
Best Regards
Jammy