| 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
| ||
|
Message-ID: <41f0b8e4-d6aa-4a42-8633-9ea77873d275@foss.st.com>
Date: Thu, 21 Nov 2024 15:28:56 +0100
From: Hugues FRUCHET <hugues.fruchet@...s.st.com>
To: Nicolas Dufresne <nicolas.dufresne@...labora.com>,
Mauro Carvalho Chehab
<mchehab@...nel.org>,
Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
Philipp Zabel <p.zabel@...gutronix.de>,
Hans Verkuil
<hverkuil-cisco@...all.nl>,
Fritz Koenig <frkoenig@...omium.org>,
Sebastian
Fricke <sebastian.fricke@...labora.com>,
Daniel Almeida
<daniel.almeida@...labora.com>,
Andrzej Pietrasiewicz
<andrzej.p@...labora.com>,
Benjamin Gaignard
<benjamin.gaignard@...labora.com>,
<linux-media@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-rockchip@...ts.infradead.org>,
<linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [PATCH v2 2/3] media: verisilicon: add WebP decoding support
Hi Nicolas,
On 11/21/24 15:20, Nicolas Dufresne wrote:
> Hi Hugues,
>
> Le jeudi 21 novembre 2024 à 11:07 +0100, Hugues FRUCHET a écrit :
>> Hi Nicolas,
>>
>> On 11/20/24 15:25, Nicolas Dufresne wrote:
>>> Le mercredi 20 novembre 2024 à 12:01 +0100, Hugues Fruchet a écrit :
>>>> Add WebP picture decoding support to VP8 stateless decoder.
>>>>
>>>> Signed-off-by: Hugues Fruchet <hugues.fruchet@...s.st.com>
>>>> ---
>>>> .../media/platform/verisilicon/hantro_g1_regs.h | 1 +
>>>> .../platform/verisilicon/hantro_g1_vp8_dec.c | 14 ++++++++++++++
>>>> .../media/platform/verisilicon/hantro_v4l2.c | 2 ++
>>>> .../platform/verisilicon/stm32mp25_vpu_hw.c | 17 +++++++++++++++--
>>>> 4 files changed, 32 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/verisilicon/hantro_g1_regs.h b/drivers/media/platform/verisilicon/hantro_g1_regs.h
>>>> index c623b3b0be18..e7d4db788e57 100644
>>>> --- a/drivers/media/platform/verisilicon/hantro_g1_regs.h
>>>> +++ b/drivers/media/platform/verisilicon/hantro_g1_regs.h
>>>> @@ -232,6 +232,7 @@
>>>> #define G1_REG_DEC_CTRL7_DCT7_START_BIT(x) (((x) & 0x3f) << 0)
>>>> #define G1_REG_ADDR_STR 0x030
>>>> #define G1_REG_ADDR_DST 0x034
>>>> +#define G1_REG_ADDR_DST_CHROMA 0x038
>>>> #define G1_REG_ADDR_REF(i) (0x038 + ((i) * 0x4))
>>>> #define G1_REG_ADDR_REF_FIELD_E BIT(1)
>>>> #define G1_REG_ADDR_REF_TOPC_E BIT(0)
>>>> diff --git a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
>>>> index 851eb67f19f5..c83ee6f5edc8 100644
>>>> --- a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
>>>> +++ b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
>>>> @@ -307,6 +307,12 @@ static void cfg_parts(struct hantro_ctx *ctx,
>>>> G1_REG_DEC_CTRL3_STREAM_LEN(dct_part_total_len),
>>>> G1_REG_DEC_CTRL3);
>>>>
>>>> + if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
>>>> + vdpu_write_relaxed(vpu,
>>>> + G1_REG_DEC_CTRL3_STREAM_LEN_EXT
>>>> + (dct_part_total_len >> 24),
>>>> + G1_REG_DEC_CTRL3);
>>>> +
>>>> /* DCT partitions base address */
>>>> for (i = 0; i < hdr->num_dct_parts; i++) {
>>>> u32 byte_offset = dct_part_offset + dct_size_part_size + count;
>>>> @@ -427,6 +433,12 @@ static void cfg_buffers(struct hantro_ctx *ctx,
>>>>
>>>> dst_dma = hantro_get_dec_buf_addr(ctx, &vb2_dst->vb2_buf);
>>>> vdpu_write_relaxed(vpu, dst_dma, G1_REG_ADDR_DST);
>>>> +
>>>> + if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
>>>> + vdpu_write_relaxed(vpu, dst_dma +
>>>> + ctx->dst_fmt.plane_fmt[0].bytesperline *
>>>> + ctx->dst_fmt.height,
>>>> + G1_REG_ADDR_DST_CHROMA);
>>>> }
>>>>
>>>> int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
>>>> @@ -471,6 +483,8 @@ int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
>>>> reg |= G1_REG_DEC_CTRL0_SKIP_MODE;
>>>> if (hdr->lf.level == 0)
>>>> reg |= G1_REG_DEC_CTRL0_FILTERING_DIS;
>>>> + if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
>>>> + reg |= G1_REG_DEC_CTRL0_WEBP_E;
>>>> vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0);
>>>>
>>>> /* Frame dimensions */
>>>> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
>>>> index 2513adfbd825..7075b2ba1ec2 100644
>>>> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
>>>> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
>>>> @@ -470,6 +470,7 @@ hantro_update_requires_request(struct hantro_ctx *ctx, u32 fourcc)
>>>> break;
>>>> case V4L2_PIX_FMT_MPEG2_SLICE:
>>>> case V4L2_PIX_FMT_VP8_FRAME:
>>>> + case V4L2_PIX_FMT_WEBP_FRAME:
>>>> case V4L2_PIX_FMT_H264_SLICE:
>>>> case V4L2_PIX_FMT_HEVC_SLICE:
>>>> case V4L2_PIX_FMT_VP9_FRAME:
>>>> @@ -492,6 +493,7 @@ hantro_update_requires_hold_capture_buf(struct hantro_ctx *ctx, u32 fourcc)
>>>> case V4L2_PIX_FMT_JPEG:
>>>> case V4L2_PIX_FMT_MPEG2_SLICE:
>>>> case V4L2_PIX_FMT_VP8_FRAME:
>>>> + case V4L2_PIX_FMT_WEBP_FRAME:
>>>> case V4L2_PIX_FMT_HEVC_SLICE:
>>>> case V4L2_PIX_FMT_VP9_FRAME:
>>>> vq->subsystem_flags &= ~(VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF);
>>>> diff --git a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
>>>> index 833821120b20..48d6912c3bab 100644
>>>> --- a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
>>>> +++ b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
>>>> @@ -22,10 +22,10 @@ static const struct hantro_fmt stm32mp25_vdec_fmts[] = {
>>>> .codec_mode = HANTRO_MODE_NONE,
>>>> .frmsize = {
>>>> .min_width = FMT_MIN_WIDTH,
>>>> - .max_width = FMT_FHD_WIDTH,
>>>> + .max_width = FMT_4K_WIDTH,
>>>> .step_width = MB_DIM,
>>>> .min_height = FMT_MIN_HEIGHT,
>>>> - .max_height = FMT_FHD_HEIGHT,
>>>> + .max_height = FMT_4K_HEIGHT,
>>>
>>> I'm a little surprised of this change, since this is modifying VP8_FRAME, while
>>> we should instead introduce WEBP_FRAME.
>>
>> This is the resolution of the YUV output of decoder, not the WebP input,
>> and because of lack of post-processor, the output is not scaled, so can
>> go up to 4K with WebP.
>> Before WebP introduction, the maximum output resolution was FHD for all
>> codecs. Now WebP allows up to 4K but FHD constraint remains for
>> H264/VP8. I don't see real problems because VP8/H264 compressed inputs
>> are well limited to FHD and only WebP allows 4K...
>
> Good point. Would you mind adding a justification for this change within the
> commit message in v3 ?
v3 already sent but I'll note that for the future v4.
Could you test & ack the 4K support on your platform having
post-processor support ?
>
>>
>>>
>>>> .step_height = MB_DIM,
>>>> },
>>>> },
>>>> @@ -68,6 +68,19 @@ static const struct hantro_fmt stm32mp25_venc_fmts[] = {
>>>> .codec_mode = HANTRO_MODE_NONE,
>>>> .enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420SP,
>>>> },
>>>> + {
>>>> + .fourcc = V4L2_PIX_FMT_WEBP_FRAME,
>>>> + .codec_mode = HANTRO_MODE_VP8_DEC,
>>>> + .max_depth = 2,
>>>> + .frmsize = {
>>>> + .min_width = FMT_MIN_WIDTH,
>>>> + .max_width = FMT_4K_WIDTH,
>>>> + .step_width = MB_DIM,
>>>> + .min_height = FMT_MIN_HEIGHT,
>>>> + .max_height = FMT_4K_HEIGHT,
>>>> + .step_height = MB_DIM,
>>>> + },
>>>> + },
>>>
>>> This is venc_fmt (encoder), this shouldn't be there.
>>
>> All apologizes for this rebase issue, it is of course part of
>> stm32mp25_vdec_fmts.
>
> Ack, let's get this right in v3 :-D
>
>>
>>>
>>>> {
>>>> .fourcc = V4L2_PIX_FMT_YUYV,
>>>> .codec_mode = HANTRO_MODE_NONE,
>>>
>>
>> BR,
>> Hugues.
>
Powered by blists - more mailing lists