Re: [RFC 1/5] media: hantro: Add support for i.MX8M Mini

From: Hans Verkuil
Date: Thu Nov 25 2021 - 10:45:33 EST

Next message: Russell King (Oracle): "Re: [PATCH v2] mdio: aspeed: Fix "Link is Down" issue"
Previous message: Alexandre TORGUE: "Re: [PATCH 0/2] tune the HS USB PHYs on stm32mp15 eval and disco boards"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 20/11/2021 17:03, Adam Ford wrote:
> On Sat, Nov 6, 2021 at 1:38 PM Adam Ford <aford173@xxxxxxxxx> wrote:
>>
>> The i.MX8M Mini has a similar implementation of the Hantro G1 and
>> G2 decoders, but the Mini uses the vpu-blk-ctrl for handling the
>> VPU resets through the power domain system. As such, there are
>> functions present in the 8MQ that are not applicable to the Mini
>> which requires the driver to have a different compatible flags.
>>
> + Nicolas Dufresne
>
> From what I can tell, the implementation between 8MQ and 8MM for the
> G2 is basically the same with the only be difference being the
> power-domain controller handling the vpu-blk-ctrl stuff in the Mini.
> It doesn't hang, so I don't think the power-domain is a problem, but
> the G2 throws some splat when v4l2-compliance is invoked.
>
> # v4l2-compliance -d1
> v4l2-compliance SHA: not available

That indicates that you did not compile v4l2-compliance from the git repo.

Please do that, that way I know you are using the latest compliance test.

git clone git://linuxtv.org/v4l-utils.git
cd v4l-utils
./bootstrap.sh
./configure
make
sudo make install

> , 64 bits, 64-bit time_t
>
> Compliance test for hantro-vpu device /dev/video1:
>
> Driver Info:
> Driver name : hantro-vpu
> Card type : nxp,imx8mm-vpu-g2-dec
> Bus info : platform: hantro-vpu
>
> Driver v[ 30.136983] ------------[ cut here ]------------
> [ 30.142038] WARNING: CPU: 3 PID: 534 at mm/page_alloc.c:5344
> __alloc_pages+0x5b4/0xbe0

Hmm:

if (unlikely(order >= MAX_ORDER)) {
WARN_ON_ONCE(!(gfp & __GFP_NOWARN));
return NULL;
}

video needs loads of memory, so if MAX_ORDER is too low it might hit this.

It's tied to kernel config option FORCE_MAX_ZONEORDER.

The hantro driver uses vb2_dma_contig_memops, so the memory has to be physically
contiguous. That does indeed typically require larger MAX_ORDER values.

Regards,

Hans

> [ 30.149971] Modules linked in: 8021q garp mrp stp llc af_alg
> caam_jr caamhash_desc caamalg_desc crypto_engine rng_core authenc
> libdes crct10dif_ce snd_soc_fsl_asoc_card snd_soc_imx_audmux
> snd_soc_simple_card_utils fsl_imx8_ddr_perf imx8m_ddrc hantro_vpu(C)
> v4l2_h264 brcmfmac v4l2_mem2mem videobuf2_vmalloc brcmutil
> videobuf2_dma_contig cfg80211 videobuf2_memops videobuf2_v4l2
> videobuf2_common snd_soc_wm8962 videodev hci_uart mc etnaviv gpu_sched
> btqca btbcm caam rtc_pcf85363 clk_bd718x7 at24 spi_imx spi_bitbang
> rtc_snvs error snvs_pwrkey snd_soc_fsl_sai imx_pcm_dma imx8mm_thermal
> imx_cpufreq_dt bluetooth ecdh_generic ecc rfkill fuse drm ipv6
> [ 30.206845] CPU: 3 PID: 534 Comm: v4l2-compliance Tainted: G
> C 5.16.0-rc1-00292-gb166e5dc4755-dirty #19
> [ 30.217544] Hardware name: Beacon EmbeddedWorks i.MX8M Mini
> Development Kit (DT)
> [ 30.224940] pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 30.231904] pc : __alloc_pages+0x5b4/0xbe0
> [ 30.236007] lr : __dma_direct_alloc_pages+0x17c/0x1e0
> [ 30.241065] sp : ffff80001265b810
> [ 30.244379] x29: ffff80001265b810 x28: 0000000000000034 x27: ffff00000b9ec220
> [ 30.251523] x26: ffff00000038d810 x25: 0000000000000000 x24: ffff00000038d810
> [ 30.258668] x23: ffff80001010c52c x22: ffff00000038d810 x21: 00000000ffffffff
> [ 30.265813] x20: 0000000000000cc1 x19: 0000000000000000 x18: 0000000000000000
> [ 30.272960] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> [ 30.280101] x14: 0000000000000001 x13: 0000000000000000 x12: 0000000000000000
> ersion : 5.16.0
> Capabilities [ 30.287247] x11: ffff80001231d000 x10:
> ffff00000b9ec33a x9 : ffff00000b9ec32a
> [ 30.297242] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000cc0
> [ 30.304385] x5 : 00000000bfffffff x4 : ffff00000a72f000 x3 : 0000000000000000
> [ 30.311529] x2 : 0000000000000000 x1 : 0000000000000001 x0 : 0000000000000cc1
> [ 30.318676] Call trace:
> [ 30.321124] __alloc_pages+0x5b4/0xbe0
> [ 30.324880] __dma_direct_alloc_pages+0x17c/0x1e0
> [ 30.329588] dma_direct_alloc+0x70/0x310
> [ 30.333514] dma_alloc_attrs+0x7c/0xe4
> [ 30.337266] hantro_hevc_get_ref_buf+0x15c/0x184 [hantro_vpu]
> [ 30.343038] hantro_g2_hevc_dec_run+0x3b8/0x1910 [hantro_vpu]
> [ 30.348805] device_run+0xac/0x110 [hantro_vpu]
> [ 30.353354] v4l2_m2m_try_run+0x84/0x134 [v4l2_mem2mem]
> [ 30.358603] v4l2_m2m_ioctl_streamon+0x84/0xa0 [v4l2_mem2mem]
> [ 30.364368] v4l_streamon+0x28/0x34 [videodev]
> [ 30.368874] __video_do_ioctl+0x178/0x3dc [videodev]
> [ 30.373889] video_usercopy+0x368/0x6dc [videodev]
> [ 30.378734] video_ioctl2+0x1c/0x30 [videodev]
> [ 30.383230] v4l2_ioctl+0x44/0x64 [videodev]
> [ 30.387554] __arm64_sys_ioctl+0xac/0xf0
> [ 30.391482] invoke_syscall+0x48/0x114
> [ 30.395238] el0_svc_common.constprop.0+0xd4/0xfc
> [ 30.399947] do_el0_svc+0x2c/0x94
> [ 30.403264] el0_svc+0x28/0x80
> [ 30.406327] el0t_64_sync_handler+0xa8/0x130
> [ 30.410603] el0t_64_sync+0x1a0/0x1a4
> [ 30.414272] ---[ end trace 878112336a378214 ]---
>
> I was hoping someone might have some suggestions on something to try..
>
> thanks,
>
> adam
>> Signed-off-by: Adam Ford <aford173@xxxxxxxxx>
>> ---
>> drivers/staging/media/hantro/hantro_drv.c | 2 +
>> drivers/staging/media/hantro/hantro_hw.h | 2 +
>> drivers/staging/media/hantro/imx8m_vpu_hw.c | 60 +++++++++++++++++++++
>> 3 files changed, 64 insertions(+)
>>
>> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
>> index fb82b9297a2b..2aa1c520be50 100644
>> --- a/drivers/staging/media/hantro/hantro_drv.c
>> +++ b/drivers/staging/media/hantro/hantro_drv.c
>> @@ -592,6 +592,8 @@ static const struct of_device_id of_hantro_match[] = {
>> { .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, },
>> #endif
>> #ifdef CONFIG_VIDEO_HANTRO_IMX8M
>> + { .compatible = "nxp,imx8mm-vpu", .data = &imx8mm_vpu_variant, },
>> + { .compatible = "nxp,imx8mm-vpu-g2", .data = &imx8mm_vpu_g2_variant },
>> { .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
>> { .compatible = "nxp,imx8mq-vpu-g2", .data = &imx8mq_vpu_g2_variant },
>> #endif
>> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
>> index 267a6d33a47b..ae7c3fff760c 100644
>> --- a/drivers/staging/media/hantro/hantro_hw.h
>> +++ b/drivers/staging/media/hantro/hantro_hw.h
>> @@ -211,6 +211,8 @@ enum hantro_enc_fmt {
>> ROCKCHIP_VPU_ENC_FMT_UYVY422 = 3,
>> };
>>
>> +extern const struct hantro_variant imx8mm_vpu_g2_variant;
>> +extern const struct hantro_variant imx8mm_vpu_variant;
>> extern const struct hantro_variant imx8mq_vpu_g2_variant;
>> extern const struct hantro_variant imx8mq_vpu_variant;
>> extern const struct hantro_variant px30_vpu_variant;
>> diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c
>> index ea919bfb9891..c819609d14d1 100644
>> --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
>> +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
>> @@ -242,6 +242,32 @@ static const struct hantro_codec_ops imx8mq_vpu_g2_codec_ops[] = {
>> },
>> };
>>
>> +static const struct hantro_codec_ops imx8mm_vpu_codec_ops[] = {
>> + [HANTRO_MODE_MPEG2_DEC] = {
>> + .run = hantro_g1_mpeg2_dec_run,
>> + .init = hantro_mpeg2_dec_init,
>> + .exit = hantro_mpeg2_dec_exit,
>> + },
>> + [HANTRO_MODE_VP8_DEC] = {
>> + .run = hantro_g1_vp8_dec_run,
>> + .init = hantro_vp8_dec_init,
>> + .exit = hantro_vp8_dec_exit,
>> + },
>> + [HANTRO_MODE_H264_DEC] = {
>> + .run = hantro_g1_h264_dec_run,
>> + .init = hantro_h264_dec_init,
>> + .exit = hantro_h264_dec_exit,
>> + },
>> +};
>> +
>> +static const struct hantro_codec_ops imx8mm_vpu_g2_codec_ops[] = {
>> + [HANTRO_MODE_HEVC_DEC] = {
>> + .run = hantro_g2_hevc_dec_run,
>> + .init = hantro_hevc_dec_init,
>> + .exit = hantro_hevc_dec_exit,
>> + },
>> +};
>> +
>> /*
>> * VPU variants.
>> */
>> @@ -257,6 +283,11 @@ static const struct hantro_irq imx8mq_g2_irqs[] = {
>> static const char * const imx8mq_clk_names[] = { "g1", "g2", "bus" };
>> static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" };
>>
>> +static const char * const imx8mm_g1_clk_names[] = { "g1", "bus" };
>> +static const char * const imx8mm_g1_reg_names[] = { "g1" };
>> +static const char * const imx8mm_g2_clk_names[] = { "g2", "bus" };
>> +static const char * const imx8mm_g2_reg_names[] = { "g2" };
>> +
>> const struct hantro_variant imx8mq_vpu_variant = {
>> .dec_fmts = imx8m_vpu_dec_fmts,
>> .num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
>> @@ -289,3 +320,32 @@ const struct hantro_variant imx8mq_vpu_g2_variant = {
>> .clk_names = imx8mq_clk_names,
>> .num_clocks = ARRAY_SIZE(imx8mq_clk_names),
>> };
>> +
>> +const struct hantro_variant imx8mm_vpu_variant = {
>> + .dec_fmts = imx8m_vpu_dec_fmts,
>> + .num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
>> + .postproc_fmts = imx8m_vpu_postproc_fmts,
>> + .num_postproc_fmts = ARRAY_SIZE(imx8m_vpu_postproc_fmts),
>> + .postproc_regs = &hantro_g1_postproc_regs,
>> + .codec = HANTRO_MPEG2_DECODER | HANTRO_VP8_DECODER |
>> + HANTRO_H264_DECODER,
>> + .codec_ops = imx8mm_vpu_codec_ops,
>> + .irqs = imx8mq_irqs,
>> + .num_irqs = ARRAY_SIZE(imx8mq_irqs),
>> + .clk_names = imx8mm_g1_clk_names,
>> + .num_clocks = ARRAY_SIZE(imx8mm_g1_clk_names),
>> + .reg_names = imx8mm_g1_reg_names,
>> + .num_regs = ARRAY_SIZE(imx8mm_g1_reg_names)
>> +};
>> +
>> +const struct hantro_variant imx8mm_vpu_g2_variant = {
>> + .dec_offset = 0x0,
>> + .dec_fmts = imx8m_vpu_g2_dec_fmts,
>> + .num_dec_fmts = ARRAY_SIZE(imx8m_vpu_g2_dec_fmts),
>> + .codec = HANTRO_HEVC_DECODER,
>> + .codec_ops = imx8mm_vpu_g2_codec_ops,
>> + .irqs = imx8mq_g2_irqs,
>> + .num_irqs = ARRAY_SIZE(imx8mq_g2_irqs),
>> + .clk_names = imx8mm_g2_clk_names,
>> + .num_clocks = ARRAY_SIZE(imx8mm_g2_reg_names),
>> +};
>> --
>> 2.32.0
>>

Next message: Russell King (Oracle): "Re: [PATCH v2] mdio: aspeed: Fix "Link is Down" issue"
Previous message: Alexandre TORGUE: "Re: [PATCH 0/2] tune the HS USB PHYs on stm32mp15 eval and disco boards"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]