Re: [PATCH v2 2/3] media: verisilicon: add WebP decoding support

[Hugues FRUCHET <hugues.fruchet@foss.st.com>]

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@foss.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...

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

> 
>>   	{
>>   		.fourcc = V4L2_PIX_FMT_YUYV,
>>   		.codec_mode = HANTRO_MODE_NONE,
> 

BR,
Hugues.

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip