Message ID | 20231008031909.32146-1-yunfei.dong@mediatek.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:a888:0:b0:403:3b70:6f57 with SMTP id x8csp1196813vqo; Sat, 7 Oct 2023 20:19:58 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFr1IGuUbmKG7tdMhYxIynScrzGChGzpEK4fHUPsF1wDj0HNyhX6nzUWkB9cUIKuJ4B0xc7 X-Received: by 2002:a17:90a:dd43:b0:273:83ac:5eb9 with SMTP id u3-20020a17090add4300b0027383ac5eb9mr12700631pjv.4.1696735197824; Sat, 07 Oct 2023 20:19:57 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1696735197; cv=none; d=google.com; s=arc-20160816; b=Xd8m0cPrKZng7hlZDpOshqqNIr+5BICdXlUZHHZ9qWwboLay3IHrI9LujJHKeT5e9Z IHTMtbnIZcYTqvubgmaIbC8CkPpFdxFLeQnK1IPutmVLvIZzuIVc4f8H2UE7cy5Ms1Dj Lcr/eKFu7Yknq3qPRUXhdYiXQtjabiiNPfGwrY027FkAfC4wAukFKwN6lxn4SlgcM9jG lhEHuqGIxD/SWwA5s/pdPVV/iihNyzEOUqpldE7iVzj27PlyDhy6rfwTW7niF3qGt6Uu zYBz6a+pt34RJikAIpO+KHUtuBBuwo9if3MLY5PPWExUmeijk1J/GcfeU5fUYmn2SImh ZHBQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=2lncwbw67fIZafoqWE9u36La+aHXLXeJrzy1kQnUnk4=; fh=VLyy4Tm0zlQcwgCu1UJZYj3DB0sUORgG/e4QjPwt6FM=; b=yinvpmH6/wVDjDohlmmkP/xPAsRXgTFnMvl3BMDJucEjmC4kOvlSfDLwhejAUYEhEi +4EFEaAIxQ5X3HUshKu/EQAmcl83ieDnXWkRiZNNd5jmT7oj9hybtuKuYvgoh9Hz3b1G mJPE6xZORy61ZedTq4XhrN5HJ6UJ5uRk34X3k0fJw1RKE9uVlqyRKSpvuuf3buupgrzj vT0VzkY1dwol+Ti8EWl7Z8MENpnecJwduszQz/v2BTkT/0V6bgzsqezTSYSQIH04wTVA 5SmcoH+I3NJ6eDGq2V6kK62xHKrIIb0bYM8hClDU7bpfNKihatkuTgGmzmyWU7nXJDCn 9Rjg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@mediatek.com header.s=dk header.b=PjAVUJJu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=mediatek.com Received: from lipwig.vger.email (lipwig.vger.email. [2620:137:e000::3:3]) by mx.google.com with ESMTPS id lr17-20020a17090b4b9100b0026ce877b4cbsi9463999pjb.151.2023.10.07.20.19.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 07 Oct 2023 20:19:57 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3; Authentication-Results: mx.google.com; dkim=pass header.i=@mediatek.com header.s=dk header.b=PjAVUJJu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=mediatek.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id 8CE2980545A2; Sat, 7 Oct 2023 20:19:55 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344309AbjJHDT3 (ORCPT <rfc822;pusanteemu@gmail.com> + 17 others); Sat, 7 Oct 2023 23:19:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50080 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234215AbjJHDT0 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sat, 7 Oct 2023 23:19:26 -0400 Received: from mailgw01.mediatek.com (unknown [60.244.123.138]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 27758BD; Sat, 7 Oct 2023 20:19:19 -0700 (PDT) X-UUID: 75bb6f30658911eea33bb35ae8d461a2-20231008 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=Content-Type:Content-Transfer-Encoding:MIME-Version:Message-ID:Date:Subject:CC:To:From; bh=2lncwbw67fIZafoqWE9u36La+aHXLXeJrzy1kQnUnk4=; b=PjAVUJJuSHqHzRTKGPA5OXNAW2UMD/ShP7lruTxpWR84/Y7K5Nbff6s5UWPEiJaggLjKylCKpU4T8hXIcfSiSoLNDmWRODH4VzBlUeoos4aT6J30/7+hCFNquUFmtB5gaRhLcBjZcJ2a5Xq5kyzI9Sq2STdEZcL9FzzMlSfUBnA=; X-CID-P-RULE: Release_Ham X-CID-O-INFO: VERSION:1.1.32,REQID:6c9a4b80-1fa8-48ca-b766-41ba7fce0e95,IP:0,U RL:0,TC:0,Content:0,EDM:0,RT:0,SF:0,FILE:0,BULK:0,RULE:Release_Ham,ACTION: release,TS:0 X-CID-META: VersionHash:5f78ec9,CLOUDID:1f1fb3bf-14cc-44ca-b657-2d2783296e72,B ulkID:nil,BulkQuantity:0,Recheck:0,SF:102,TC:nil,Content:0,EDM:-3,IP:nil,U RL:0,File:nil,Bulk:nil,QS:nil,BEC:nil,COL:0,OSI:0,OSA:0,AV:0,LES:1,SPR:NO, DKR:0,DKP:0,BRR:0,BRE:0 X-CID-BVR: 0,NGT X-CID-BAS: 0,NGT,0,_ X-CID-FACTOR: TF_CID_SPAM_SNR X-UUID: 75bb6f30658911eea33bb35ae8d461a2-20231008 Received: from mtkmbs11n1.mediatek.inc [(172.21.101.185)] by mailgw01.mediatek.com (envelope-from <yunfei.dong@mediatek.com>) (Generic MTA with TLSv1.2 ECDHE-RSA-AES256-GCM-SHA384 256/256) with ESMTP id 1277965314; Sun, 08 Oct 2023 11:19:15 +0800 Received: from mtkmbs13n2.mediatek.inc (172.21.101.108) by mtkmbs10n2.mediatek.inc (172.21.101.183) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.26; Sun, 8 Oct 2023 11:19:13 +0800 Received: from mhfsdcap04.gcn.mediatek.inc (10.17.3.154) by mtkmbs13n2.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.2.1118.26 via Frontend Transport; Sun, 8 Oct 2023 11:19:12 +0800 From: Yunfei Dong <yunfei.dong@mediatek.com> To: =?utf-8?q?N=C3=ADcolas_F_=2E_R_=2E_A_=2E_Prado?= <nfraprado@collabora.com>, Nicolas Dufresne <nicolas.dufresne@collabora.com>, Hans Verkuil <hverkuil-cisco@xs4all.nl>, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, Benjamin Gaignard <benjamin.gaignard@collabora.com>, Nathan Hebert <nhebert@chromium.org> CC: Chen-Yu Tsai <wenst@chromium.org>, Hsin-Yi Wang <hsinyi@chromium.org>, Fritz Koenig <frkoenig@chromium.org>, Daniel Vetter <daniel@ffwll.ch>, "Steve Cho" <stevecho@chromium.org>, Yunfei Dong <yunfei.dong@mediatek.com>, <linux-media@vger.kernel.org>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-mediatek@lists.infradead.org>, <Project_Global_Chrome_Upstream_Group@mediatek.com> Subject: [PATCH] media: mediatek: vcodec: using encoder device to alloc/free encoder memory Date: Sun, 8 Oct 2023 11:19:09 +0800 Message-ID: <20231008031909.32146-1-yunfei.dong@mediatek.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-TM-AS-Product-Ver: SMEX-14.0.0.3152-9.1.1006-23728.005 X-TM-AS-Result: No-10--9.270600-8.000000 X-TMASE-MatchedRID: EvLd3cwGQhEtJMbDWD8p3hWCVBr+Ay98uoYFb0nRiqOCsBeCv8CM/Xf3 4BoKFzcHTg/jfa6b1OczYorSrzJ8m2MAzi+7d0chF6z9HGHKwNuIrmqDVyayv8lgi/vLS272INi phQlaWRN2ZsQ86ifK0L+R4Fk68cCxoXHg+KoW2Ubil2r2x2PwtYfsPVs/8Vw6Ydn5x3tXIpcota sKsNUdSeLzNWBegCW2xl8lw85EaVQLbigRnpKlKWxlRJiH4397GKedVHUXrT7KIf/D+oFxIU+rs 1+BI9BmqizkboPGhHnzIgp4i3lt2A== X-TM-AS-User-Approved-Sender: No X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--9.270600-8.000000 X-TMASE-Version: SMEX-14.0.0.3152-9.1.1006-23728.005 X-TM-SNTS-SMTP: 115D3AFDCB6F7F683D4B83FD3D6673B22C9D90B17C7C1E4CDDFDB2404A119D962000:8 X-MTK: N X-Spam-Status: No, score=2.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Sat, 07 Oct 2023 20:19:55 -0700 (PDT) X-Spam-Level: ** X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1779155807035194944 X-GMAIL-MSGID: 1779155807035194944 |
Series |
media: mediatek: vcodec: using encoder device to alloc/free encoder memory
|
|
Commit Message
Yunfei Dong (董云飞)
Oct. 8, 2023, 3:19 a.m. UTC
Need to use encoder device to allocate/free encoder memory when calling
mtk_vcodec_mem_alloc/mtk_vcodec_mem_free, or leading to below crash log
when test encoder with decoder device.
pc : dma_alloc_attrs+0x44/0xf4
lr : mtk_vcodec_mem_alloc+0x50/0xa4 [mtk_vcodec_common]
sp : ffffffc0209f3990
x29: ffffffc0209f39a0 x28: ffffff8024102a18 x27: 0000000000000000
x26: 0000000000000000 x25: ffffffc00c06e2d8 x24: 0000000000000001
x23: 0000000000000cc0 x22: 0000000000000010 x21: 0000000000000800
x20: ffffff8024102a18 x19: 0000000000000000 x18: 0000000000000000
x17: 0000000000000009 x16: ffffffe389736a98 x15: 0000000000000078
x14: ffffffe389704434 x13: 0000000000000007 x12: ffffffe38a2b2560
x11: 0000000000000800 x10: 0000000000000004 x9 : ffffffe331f07484
x8 : 5400e9aef2395000 x7 : 0000000000000000 x6 : 000000000000003f
x5 : 0000000000000001 x4 : 0000000000000000 x3 : 0000000000000cc0
x2 : ffffff8024102a18 x1 : 0000000000000800 x0 : 0000000000000010
Call trace:
dma_alloc_attrs+0x44/0xf4
mtk_vcodec_mem_alloc+0x50/0xa4 [mtk_vcodec_common 2819d3d601f3cd06c1f2213ac1b9995134441421]
h264_enc_set_param+0x27c/0x378 [mtk_vcodec_enc 772cc3d26c254e8cf54079451ef8d930d2eb4404]
venc_if_set_param+0x4c/0x7c [mtk_vcodec_enc 772cc3d26c254e8cf54079451ef8d930d2eb4404]
vb2ops_venc_start_streaming+0x1bc/0x328 [mtk_vcodec_enc 772cc3d26c254e8cf54079451ef8d930d2eb4404]
vb2_start_streaming+0x64/0x12c
vb2_core_streamon+0x114/0x158
vb2_streamon+0x38/0x60
v4l2_m2m_streamon+0x48/0x88
v4l2_m2m_ioctl_streamon+0x20/0x2c
v4l_streamon+0x2c/0x38
__video_do_ioctl+0x2c4/0x3dc
video_usercopy+0x404/0x934
video_ioctl2+0x20/0x2c
v4l2_ioctl+0x54/0x64
v4l2_compat_ioctl32+0x90/0xa34
__arm64_compat_sys_ioctl+0x128/0x13c
invoke_syscall+0x4c/0x108
el0_svc_common+0x98/0x104
do_el0_svc_compat+0x28/0x34
el0_svc_compat+0x2c/0x74
el0t_32_sync_handler+0xa8/0xcc
el0t_32_sync+0x194/0x198
Code: aa0003f6 aa0203f4 aa0103f5 f900
'Fixes: 01abf5fbb081c ("media: mediatek: vcodec: separate struct 'mtk_vcodec_ctx'")'
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
.../mediatek/vcodec/common/mtk_vcodec_util.c | 66 ++++++++++++++++++-
1 file changed, 64 insertions(+), 2 deletions(-)
Comments
Il 08/10/23 05:19, Yunfei Dong ha scritto: > Need to use encoder device to allocate/free encoder memory when calling > mtk_vcodec_mem_alloc/mtk_vcodec_mem_free, or leading to below crash log > when test encoder with decoder device. > > pc : dma_alloc_attrs+0x44/0xf4 > lr : mtk_vcodec_mem_alloc+0x50/0xa4 [mtk_vcodec_common] > sp : ffffffc0209f3990 > x29: ffffffc0209f39a0 x28: ffffff8024102a18 x27: 0000000000000000 > x26: 0000000000000000 x25: ffffffc00c06e2d8 x24: 0000000000000001 > x23: 0000000000000cc0 x22: 0000000000000010 x21: 0000000000000800 > x20: ffffff8024102a18 x19: 0000000000000000 x18: 0000000000000000 > x17: 0000000000000009 x16: ffffffe389736a98 x15: 0000000000000078 > x14: ffffffe389704434 x13: 0000000000000007 x12: ffffffe38a2b2560 > x11: 0000000000000800 x10: 0000000000000004 x9 : ffffffe331f07484 > x8 : 5400e9aef2395000 x7 : 0000000000000000 x6 : 000000000000003f > x5 : 0000000000000001 x4 : 0000000000000000 x3 : 0000000000000cc0 > x2 : ffffff8024102a18 x1 : 0000000000000800 x0 : 0000000000000010 > Call trace: > dma_alloc_attrs+0x44/0xf4 > mtk_vcodec_mem_alloc+0x50/0xa4 [mtk_vcodec_common 2819d3d601f3cd06c1f2213ac1b9995134441421] > h264_enc_set_param+0x27c/0x378 [mtk_vcodec_enc 772cc3d26c254e8cf54079451ef8d930d2eb4404] > venc_if_set_param+0x4c/0x7c [mtk_vcodec_enc 772cc3d26c254e8cf54079451ef8d930d2eb4404] > vb2ops_venc_start_streaming+0x1bc/0x328 [mtk_vcodec_enc 772cc3d26c254e8cf54079451ef8d930d2eb4404] > vb2_start_streaming+0x64/0x12c > vb2_core_streamon+0x114/0x158 > vb2_streamon+0x38/0x60 > v4l2_m2m_streamon+0x48/0x88 > v4l2_m2m_ioctl_streamon+0x20/0x2c > v4l_streamon+0x2c/0x38 > __video_do_ioctl+0x2c4/0x3dc > video_usercopy+0x404/0x934 > video_ioctl2+0x20/0x2c > v4l2_ioctl+0x54/0x64 > v4l2_compat_ioctl32+0x90/0xa34 > __arm64_compat_sys_ioctl+0x128/0x13c > invoke_syscall+0x4c/0x108 > el0_svc_common+0x98/0x104 > do_el0_svc_compat+0x28/0x34 > el0_svc_compat+0x2c/0x74 > el0t_32_sync_handler+0xa8/0xcc > el0t_32_sync+0x194/0x198 > Code: aa0003f6 aa0203f4 aa0103f5 f900 > > 'Fixes: 01abf5fbb081c ("media: mediatek: vcodec: separate struct 'mtk_vcodec_ctx'")' > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > --- > .../mediatek/vcodec/common/mtk_vcodec_util.c | 66 ++++++++++++++++++- > 1 file changed, 64 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c > index 908602031fd0..62bb7290c56d 100644 > --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c > +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c > @@ -45,7 +45,7 @@ int mtk_vcodec_write_vdecsys(struct mtk_vcodec_dec_ctx *ctx, unsigned int reg, > } > EXPORT_SYMBOL(mtk_vcodec_write_vdecsys); > > -int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem) > +static int mtk_vcodec_mem_dec_alloc(void *priv, struct mtk_vcodec_mem *mem) > { > unsigned long size = mem->size; > struct mtk_vcodec_dec_ctx *ctx = priv; > @@ -64,9 +64,39 @@ int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem) > > return 0; > } > + > +static int mtk_vcodec_mem_enc_alloc(void *priv, struct mtk_vcodec_mem *mem) > +{ > + unsigned long size = mem->size; > + struct mtk_vcodec_enc_ctx *ctx = priv; > + struct device *dev = &ctx->dev->plat_dev->dev; > + > + mem->va = dma_alloc_coherent(dev, size, &mem->dma_addr, GFP_KERNEL); > + if (!mem->va) { > + mtk_v4l2_venc_err(ctx, "%s dma_alloc size=%ld failed!", dev_name(dev), size); > + return -ENOMEM; > + } > + > + mtk_v4l2_venc_dbg(3, ctx, "[%d] - va = %p", ctx->id, mem->va); > + mtk_v4l2_venc_dbg(3, ctx, "[%d] - dma = 0x%lx", ctx->id, > + (unsigned long)mem->dma_addr); > + mtk_v4l2_venc_dbg(3, ctx, "[%d] size = 0x%lx", ctx->id, size); > + > + return 0; > +} > + > +int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem) > +{ > + enum mtk_instance_type inst_type = *((unsigned int *)priv); I agree in that the first member of both mtk_vcodec_{enc,dec}_ctx is this enum mtk_instance_type, but no, you're not passing that as priv: you're actually passing one of the two structures, and this would add up complexity in human readability, and would also open the road for mistakes. You should at this point pass the instance type to the function. int mtk_vcodec_mem_alloc(void *priv, enum mtk_instance_type /* or int, but I prefer enum */ inst_type, struct mtk_vcodec_mem *mem) ...and then, since the only difference between the two `alloc` function is just only about debugging logs, you could commonize the allocation part... an idea: struct platform_device *pdev; unsigned long size; int id; if (inst_type == MTK_INST_DECODER) { struct mtk_vcodec_enc_ctx *enc_ctx = priv; pdev = enc_ctx->dev->plat_dev; id = enc_ctx->id; } else { struct mtk_vcodec_dec_ctx *dec_ctx = priv; pdev = dec_ctx->dev->plat_dev; id = dec_ctx->id; } mem->va = dma_alloc_coherent(&pdev->dev, etc.....) if (!mem->va) { mtk_v4l2_err(pdev, .....); } mtk_v4l2_debug(&pdev->dev, 3, "[%d] - va = %p", id, mem->va); ...you wouldn't even need one function for enc_alloc and one for dec_alloc, as the logic would be pretty much commonized like this. > + > + if (inst_type == MTK_INST_ENCODER) > + return mtk_vcodec_mem_enc_alloc(priv, mem); > + else > + return mtk_vcodec_mem_dec_alloc(priv, mem); > +} > EXPORT_SYMBOL(mtk_vcodec_mem_alloc); > > -void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem) > +static void mtk_vcodec_mem_dec_free(void *priv, struct mtk_vcodec_mem *mem) > { > unsigned long size = mem->size; > struct mtk_vcodec_dec_ctx *ctx = priv; > @@ -87,6 +117,38 @@ void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem) > mem->dma_addr = 0; > mem->size = 0; > } > + > +static void mtk_vcodec_mem_enc_free(void *priv, struct mtk_vcodec_mem *mem) > +{ > + unsigned long size = mem->size; > + struct mtk_vcodec_enc_ctx *ctx = priv; > + struct device *dev = &ctx->dev->plat_dev->dev; > + > + if (!mem->va) { > + mtk_v4l2_venc_err(ctx, "%s dma_free size=%ld failed!", dev_name(dev), size); > + return; > + } > + > + mtk_v4l2_venc_dbg(3, ctx, "[%d] - va = %p", ctx->id, mem->va); > + mtk_v4l2_venc_dbg(3, ctx, "[%d] - dma = 0x%lx", ctx->id, > + (unsigned long)mem->dma_addr); > + mtk_v4l2_venc_dbg(3, ctx, "[%d] size = 0x%lx", ctx->id, size); > + > + dma_free_coherent(dev, size, mem->va, mem->dma_addr); > + mem->va = NULL; > + mem->dma_addr = 0; > + mem->size = 0; > +} > + > +void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem) > +{ > + enum mtk_instance_type inst_type = *((unsigned int *)priv); > + > + if (inst_type == MTK_INST_ENCODER) > + mtk_vcodec_mem_enc_free(priv, mem); > + else > + mtk_vcodec_mem_dec_free(priv, mem); same comments apply to the xxxx_free() function as well. You're in the "common" scope here, so you wouldn't be forced to use the enc/dec specific logging functions, as you can use the more generic mtk_v4l2_{debug,err}() and mtk_vcodec_{debug,err}() functions here, which are *perfect* for the common code. Cheers, Angelo
Hi AngeloGioacchino, Thanks for your advice. On Mon, 2023-10-09 at 14:15 +0200, AngeloGioacchino Del Regno wrote: > Il 08/10/23 05:19, Yunfei Dong ha scritto: > > Need to use encoder device to allocate/free encoder memory when > > calling > > mtk_vcodec_mem_alloc/mtk_vcodec_mem_free, or leading to below crash > > log > > when test encoder with decoder device. > > > > pc : dma_alloc_attrs+0x44/0xf4 > > lr : mtk_vcodec_mem_alloc+0x50/0xa4 [mtk_vcodec_common] > > sp : ffffffc0209f3990 > > x29: ffffffc0209f39a0 x28: ffffff8024102a18 x27: 0000000000000000 > > x26: 0000000000000000 x25: ffffffc00c06e2d8 x24: 0000000000000001 > > x23: 0000000000000cc0 x22: 0000000000000010 x21: 0000000000000800 > > x20: ffffff8024102a18 x19: 0000000000000000 x18: 0000000000000000 > > x17: 0000000000000009 x16: ffffffe389736a98 x15: 0000000000000078 > > x14: ffffffe389704434 x13: 0000000000000007 x12: ffffffe38a2b2560 > > x11: 0000000000000800 x10: 0000000000000004 x9 : ffffffe331f07484 > > x8 : 5400e9aef2395000 x7 : 0000000000000000 x6 : 000000000000003f > > x5 : 0000000000000001 x4 : 0000000000000000 x3 : 0000000000000cc0 > > x2 : ffffff8024102a18 x1 : 0000000000000800 x0 : 0000000000000010 > > Call trace: > > dma_alloc_attrs+0x44/0xf4 > > mtk_vcodec_mem_alloc+0x50/0xa4 [mtk_vcodec_common > > 2819d3d601f3cd06c1f2213ac1b9995134441421] > > h264_enc_set_param+0x27c/0x378 [mtk_vcodec_enc > > 772cc3d26c254e8cf54079451ef8d930d2eb4404] > > venc_if_set_param+0x4c/0x7c [mtk_vcodec_enc > > 772cc3d26c254e8cf54079451ef8d930d2eb4404] > > vb2ops_venc_start_streaming+0x1bc/0x328 [mtk_vcodec_enc > > 772cc3d26c254e8cf54079451ef8d930d2eb4404] > > vb2_start_streaming+0x64/0x12c > > vb2_core_streamon+0x114/0x158 > > vb2_streamon+0x38/0x60 > > v4l2_m2m_streamon+0x48/0x88 > > v4l2_m2m_ioctl_streamon+0x20/0x2c > > v4l_streamon+0x2c/0x38 > > __video_do_ioctl+0x2c4/0x3dc > > video_usercopy+0x404/0x934 > > video_ioctl2+0x20/0x2c > > v4l2_ioctl+0x54/0x64 > > v4l2_compat_ioctl32+0x90/0xa34 > > __arm64_compat_sys_ioctl+0x128/0x13c > > invoke_syscall+0x4c/0x108 > > el0_svc_common+0x98/0x104 > > do_el0_svc_compat+0x28/0x34 > > el0_svc_compat+0x2c/0x74 > > el0t_32_sync_handler+0xa8/0xcc > > el0t_32_sync+0x194/0x198 > > Code: aa0003f6 aa0203f4 aa0103f5 f900 > > > > 'Fixes: 01abf5fbb081c ("media: mediatek: vcodec: separate struct > > 'mtk_vcodec_ctx'")' > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > --- > > .../mediatek/vcodec/common/mtk_vcodec_util.c | 66 > > ++++++++++++++++++- > > 1 file changed, 64 insertions(+), 2 deletions(-) > > > > diff --git > > a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c > > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c > > index 908602031fd0..62bb7290c56d 100644 > > --- > > a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c > > +++ > > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c > > @@ -45,7 +45,7 @@ int mtk_vcodec_write_vdecsys(struct > > mtk_vcodec_dec_ctx *ctx, unsigned int reg, > > } > > EXPORT_SYMBOL(mtk_vcodec_write_vdecsys); > > > > -int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem) > > +static int mtk_vcodec_mem_dec_alloc(void *priv, struct > > mtk_vcodec_mem *mem) > > { > > unsigned long size = mem->size; > > struct mtk_vcodec_dec_ctx *ctx = priv; > > @@ -64,9 +64,39 @@ int mtk_vcodec_mem_alloc(void *priv, struct > > mtk_vcodec_mem *mem) > > > > return 0; > > } > > + > > +static int mtk_vcodec_mem_enc_alloc(void *priv, struct > > mtk_vcodec_mem *mem) > > +{ > > + unsigned long size = mem->size; > > + struct mtk_vcodec_enc_ctx *ctx = priv; > > + struct device *dev = &ctx->dev->plat_dev->dev; > > + > > + mem->va = dma_alloc_coherent(dev, size, &mem->dma_addr, > > GFP_KERNEL); > > + if (!mem->va) { > > + mtk_v4l2_venc_err(ctx, "%s dma_alloc size=%ld failed!", > > dev_name(dev), size); > > + return -ENOMEM; > > + } > > + > > + mtk_v4l2_venc_dbg(3, ctx, "[%d] - va = %p", ctx->id, mem- > > >va); > > + mtk_v4l2_venc_dbg(3, ctx, "[%d] - dma = 0x%lx", ctx->id, > > + (unsigned long)mem->dma_addr); > > + mtk_v4l2_venc_dbg(3, ctx, "[%d] size = 0x%lx", ctx->id, > > size); > > + > > + return 0; > > +} > > + > > +int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem) > > +{ > > + enum mtk_instance_type inst_type = *((unsigned int *)priv); > > I agree in that the first member of both mtk_vcodec_{enc,dec}_ctx is > this > enum mtk_instance_type, but no, you're not passing that as priv: > you're actually > passing one of the two structures, and this would add up complexity > in human > readability, and would also open the road for mistakes. > > You should at this point pass the instance type to the function. > > int mtk_vcodec_mem_alloc(void *priv, enum mtk_instance_type /* or > int, but I prefer > enum */ inst_type, struct mtk_vcodec_mem *mem) > > ...and then, since the only difference between the two `alloc` > function is just > only about debugging logs, you could commonize the allocation part... > an idea: > > struct platform_device *pdev; > unsigned long size; > int id; > > Will rewrite allocate and free memory interface in patch v2. Best Regards, Yunfei Dong > if (inst_type == MTK_INST_DECODER) { > struct mtk_vcodec_enc_ctx *enc_ctx = priv; > pdev = enc_ctx->dev->plat_dev; > id = enc_ctx->id; > } else { > struct mtk_vcodec_dec_ctx *dec_ctx = priv; > pdev = dec_ctx->dev->plat_dev; > id = dec_ctx->id; > } > > mem->va = dma_alloc_coherent(&pdev->dev, etc.....) > if (!mem->va) { > mtk_v4l2_err(pdev, .....); > } > > mtk_v4l2_debug(&pdev->dev, 3, "[%d] - va = %p", id, mem->va); > > ...you wouldn't even need one function for enc_alloc and one for > dec_alloc, as > the logic would be pretty much commonized like this. > > > + > > + if (inst_type == MTK_INST_ENCODER) > > + return mtk_vcodec_mem_enc_alloc(priv, mem); > > + else > > + return mtk_vcodec_mem_dec_alloc(priv, mem); > > +} > > EXPORT_SYMBOL(mtk_vcodec_mem_alloc); > > > > -void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem) > > +static void mtk_vcodec_mem_dec_free(void *priv, struct > > mtk_vcodec_mem *mem) > > { > > unsigned long size = mem->size; > > struct mtk_vcodec_dec_ctx *ctx = priv; > > @@ -87,6 +117,38 @@ void mtk_vcodec_mem_free(void *priv, struct > > mtk_vcodec_mem *mem) > > mem->dma_addr = 0; > > mem->size = 0; > > } > > + > > +static void mtk_vcodec_mem_enc_free(void *priv, struct > > mtk_vcodec_mem *mem) > > +{ > > + unsigned long size = mem->size; > > + struct mtk_vcodec_enc_ctx *ctx = priv; > > + struct device *dev = &ctx->dev->plat_dev->dev; > > + > > + if (!mem->va) { > > + mtk_v4l2_venc_err(ctx, "%s dma_free size=%ld failed!", > > dev_name(dev), size); > > + return; > > + } > > + > > + mtk_v4l2_venc_dbg(3, ctx, "[%d] - va = %p", ctx->id, mem- > > >va); > > + mtk_v4l2_venc_dbg(3, ctx, "[%d] - dma = 0x%lx", ctx->id, > > + (unsigned long)mem->dma_addr); > > + mtk_v4l2_venc_dbg(3, ctx, "[%d] size = 0x%lx", ctx->id, > > size); > > + > > + dma_free_coherent(dev, size, mem->va, mem->dma_addr); > > + mem->va = NULL; > > + mem->dma_addr = 0; > > + mem->size = 0; > > +} > > + > > +void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem) > > +{ > > + enum mtk_instance_type inst_type = *((unsigned int *)priv); > > + > > + if (inst_type == MTK_INST_ENCODER) > > + mtk_vcodec_mem_enc_free(priv, mem); > > + else > > + mtk_vcodec_mem_dec_free(priv, mem); > > same comments apply to the xxxx_free() function as well. > You're in the "common" scope here, so you wouldn't be forced to use > the enc/dec > specific logging functions, as you can use the more generic > mtk_v4l2_{debug,err}() > and mtk_vcodec_{debug,err}() functions here, which are *perfect* for > the common > code. > > Cheers, > Angelo > >
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c index 908602031fd0..62bb7290c56d 100644 --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c @@ -45,7 +45,7 @@ int mtk_vcodec_write_vdecsys(struct mtk_vcodec_dec_ctx *ctx, unsigned int reg, } EXPORT_SYMBOL(mtk_vcodec_write_vdecsys); -int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem) +static int mtk_vcodec_mem_dec_alloc(void *priv, struct mtk_vcodec_mem *mem) { unsigned long size = mem->size; struct mtk_vcodec_dec_ctx *ctx = priv; @@ -64,9 +64,39 @@ int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem) return 0; } + +static int mtk_vcodec_mem_enc_alloc(void *priv, struct mtk_vcodec_mem *mem) +{ + unsigned long size = mem->size; + struct mtk_vcodec_enc_ctx *ctx = priv; + struct device *dev = &ctx->dev->plat_dev->dev; + + mem->va = dma_alloc_coherent(dev, size, &mem->dma_addr, GFP_KERNEL); + if (!mem->va) { + mtk_v4l2_venc_err(ctx, "%s dma_alloc size=%ld failed!", dev_name(dev), size); + return -ENOMEM; + } + + mtk_v4l2_venc_dbg(3, ctx, "[%d] - va = %p", ctx->id, mem->va); + mtk_v4l2_venc_dbg(3, ctx, "[%d] - dma = 0x%lx", ctx->id, + (unsigned long)mem->dma_addr); + mtk_v4l2_venc_dbg(3, ctx, "[%d] size = 0x%lx", ctx->id, size); + + return 0; +} + +int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem) +{ + enum mtk_instance_type inst_type = *((unsigned int *)priv); + + if (inst_type == MTK_INST_ENCODER) + return mtk_vcodec_mem_enc_alloc(priv, mem); + else + return mtk_vcodec_mem_dec_alloc(priv, mem); +} EXPORT_SYMBOL(mtk_vcodec_mem_alloc); -void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem) +static void mtk_vcodec_mem_dec_free(void *priv, struct mtk_vcodec_mem *mem) { unsigned long size = mem->size; struct mtk_vcodec_dec_ctx *ctx = priv; @@ -87,6 +117,38 @@ void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem) mem->dma_addr = 0; mem->size = 0; } + +static void mtk_vcodec_mem_enc_free(void *priv, struct mtk_vcodec_mem *mem) +{ + unsigned long size = mem->size; + struct mtk_vcodec_enc_ctx *ctx = priv; + struct device *dev = &ctx->dev->plat_dev->dev; + + if (!mem->va) { + mtk_v4l2_venc_err(ctx, "%s dma_free size=%ld failed!", dev_name(dev), size); + return; + } + + mtk_v4l2_venc_dbg(3, ctx, "[%d] - va = %p", ctx->id, mem->va); + mtk_v4l2_venc_dbg(3, ctx, "[%d] - dma = 0x%lx", ctx->id, + (unsigned long)mem->dma_addr); + mtk_v4l2_venc_dbg(3, ctx, "[%d] size = 0x%lx", ctx->id, size); + + dma_free_coherent(dev, size, mem->va, mem->dma_addr); + mem->va = NULL; + mem->dma_addr = 0; + mem->size = 0; +} + +void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem) +{ + enum mtk_instance_type inst_type = *((unsigned int *)priv); + + if (inst_type == MTK_INST_ENCODER) + mtk_vcodec_mem_enc_free(priv, mem); + else + mtk_vcodec_mem_dec_free(priv, mem); +} EXPORT_SYMBOL(mtk_vcodec_mem_free); void *mtk_vcodec_get_hw_dev(struct mtk_vcodec_dec_dev *dev, int hw_idx)