From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.2 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED, USER_AGENT_SANE_2 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2BE88C11F6B for ; Mon, 12 Jul 2021 08:08:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 12358613E6 for ; Mon, 12 Jul 2021 08:08:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232077AbhGLILS (ORCPT ); Mon, 12 Jul 2021 04:11:18 -0400 Received: from mailgw01.mediatek.com ([60.244.123.138]:55226 "EHLO mailgw01.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1346430AbhGLHau (ORCPT ); Mon, 12 Jul 2021 03:30:50 -0400 X-UUID: ad7cb11227124d7e999b49f5d9e630b8-20210712 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=Content-Transfer-Encoding:MIME-Version:Content-Type:References:In-Reply-To:Date:CC:To:From:Subject:Message-ID; bh=RaHR82D17SfxYYxbMS4VegTIpCJoSq06AsIDCLpUfYU=; b=HJVBmBfrR1ybFuWzNlz0CloXWENT2m2CUD410UaVGwjGERcmGetrtC2bLranNJrSoObX5cl9NXLpWnICndcybmpfygR3QmPX6Tgz31kTzFBFDqi99sMzvHIFd36Fx02B/Tbc/DonLAzzhlO+UQy5PgdXKDPV7GtR3ZAJmv/8Td4=; X-UUID: ad7cb11227124d7e999b49f5d9e630b8-20210712 Received: from mtkcas11.mediatek.inc [(172.21.101.40)] by mailgw01.mediatek.com (envelope-from ) (Generic MTA with TLSv1.2 ECDHE-RSA-AES256-SHA384 256/256) with ESMTP id 1845177520; Mon, 12 Jul 2021 15:27:58 +0800 Received: from mtkcas07.mediatek.inc (172.21.101.84) by mtkmbs07n1.mediatek.inc (172.21.101.16) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Mon, 12 Jul 2021 15:27:56 +0800 Received: from [10.17.3.153] (10.17.3.153) by mtkcas07.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Mon, 12 Jul 2021 15:27:55 +0800 Message-ID: <1626074875.7221.15.camel@mhfsdcap03> Subject: Re: [PATCH v1, 07/14] media: mtk-vcodec: Add msg queue feature for lat and core architecture From: mtk12024 To: Tzung-Bi Shih CC: Alexandre Courbot , Hans Verkuil , Tzung-Bi Shih , "Tiffany Lin" , Andrew-CT Chen , Mauro Carvalho Chehab , Rob Herring , Matthias Brugger , Tomasz Figa , Hsin-Yi Wang , Fritz Koenig , Irui Wang , , , , , , , Date: Mon, 12 Jul 2021 15:27:55 +0800 In-Reply-To: References: <20210707062157.21176-1-yunfei.dong@mediatek.com> <20210707062157.21176-8-yunfei.dong@mediatek.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4-0ubuntu2 MIME-Version: 1.0 X-MTK: N Content-Transfer-Encoding: base64 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org SGkgVHp1bmctQmksDQoNClRoYW5rcyBmb3IgeW91ciBkZXRhaWwgZmVlZGJhY2suDQpJIGFkZCB0 aGUgZGVzY3JpcHRpb24gYWNjb3JkaW5nIHRvIHlvdXIgZWFjaCBjb21tZW50cy4NCg0KT24gRnJp LCAyMDIxLTA3LTA5IGF0IDE3OjM5ICswODAwLCBUenVuZy1CaSBTaGloIHdyb3RlOg0KPiBPbiBX ZWQsIEp1bCA3LCAyMDIxIGF0IDI6MjIgUE0gWXVuZmVpIERvbmcgPHl1bmZlaS5kb25nQG1lZGlh dGVrLmNvbT4gd3JvdGU6DQo+ID4gQEAgLTQ2NCw2ICs0NjksMTEgQEAgc3RydWN0IG10a192Y29k ZWNfZW5jX3BkYXRhIHsNCj4gPiAgICogY29tcF9kZXY6IGNvbXBvbmVudCBoYXJkd2FyZSBkZXZp Y2UNCj4gPiAgICogY29tcG9uZW50X25vZGU6IGNvbXBvbmVudCBub2RlDQo+ID4gICAqIGNvbXBf aWR4OiBjb21wb25lbnQgaW5kZXgNCj4gPiArICoNCj4gPiArICogY29yZV9yZWFkOiBXYWl0IHF1 ZXVlIHVzZWQgdG8gc2lnbmFsaXplIHdoZW4gY29yZSBnZXQgdXNlZnVsIGxhdCBidWZmZXINCj4g PiArICogY29yZV9xdWV1ZTogTGlzdCBvZiBWNEwyIGxhdF9idWYNCj4gVG8gYmUgbmVhdCwgcmVw bGFjZSAiV2FpdCIgdG8gIndhaXQiIGFuZCAiTGlzdCIgdG8gImxpc3QiLg0KV2lsbCBmaXguDQo+ ID4gK2ludCB2ZGVjX21zZ19xdWV1ZV9pbml0KA0KPiA+ICsgICAgICAgc3RydWN0IG10a192Y29k ZWNfY3R4ICpjdHgsDQo+ID4gKyAgICAgICBzdHJ1Y3QgdmRlY19tc2dfcXVldWUgKm1zZ19xdWV1 ZSwNCj4gPiArICAgICAgIGNvcmVfZGVjb2RlX2NiX3QgY29yZV9kZWNvZGUsDQo+ID4gKyAgICAg ICBpbnQgcHJpdmF0ZV9zaXplKQ0KPiA+ICt7DQo+ID4gKyAgICAgICBzdHJ1Y3QgdmRlY19sYXRf YnVmICpsYXRfYnVmOw0KPiA+ICsgICAgICAgaW50IGksIGVycjsNCj4gPiArDQo+ID4gKyAgICAg ICBpbml0X3dhaXRxdWV1ZV9oZWFkKCZtc2dfcXVldWUtPmxhdF9yZWFkKTsNCj4gPiArICAgICAg IElOSVRfTElTVF9IRUFEKCZtc2dfcXVldWUtPmxhdF9xdWV1ZSk7DQo+ID4gKyAgICAgICBzcGlu X2xvY2tfaW5pdCgmbXNnX3F1ZXVlLT5sYXRfbG9jayk7DQo+ID4gKyAgICAgICBtc2dfcXVldWUt Pm51bV9sYXQgPSAwOw0KPiA+ICsNCj4gPiArICAgICAgIG1zZ19xdWV1ZS0+d2RtYV9hZGRyLnNp emUgPSB2ZGVfbXNnX3F1ZXVlX2dldF90cmFuc19zaXplKA0KPiA+ICsgICAgICAgICAgICAgICBj dHgtPnBpY2luZm8uYnVmX3csIGN0eC0+cGljaW5mby5idWZfaCk7DQo+ID4gKw0KPiA+ICsgICAg ICAgZXJyID0gbXRrX3Zjb2RlY19tZW1fYWxsb2MoY3R4LCAmbXNnX3F1ZXVlLT53ZG1hX2FkZHIp Ow0KPiA+ICsgICAgICAgaWYgKGVycikgew0KPiA+ICsgICAgICAgICAgICAgICBtdGtfdjRsMl9l cnIoImZhaWxlZCB0byBhbGxvY2F0ZSB3ZG1hX2FkZHIgYnVmIik7DQo+ID4gKyAgICAgICAgICAg ICAgIHJldHVybiAtRU5PTUVNOw0KPiA+ICsgICAgICAgfQ0KPiA+ICsgICAgICAgbXNnX3F1ZXVl LT53ZG1hX3JwdHJfYWRkciA9IG1zZ19xdWV1ZS0+d2RtYV9hZGRyLmRtYV9hZGRyOw0KPiA+ICsg ICAgICAgbXNnX3F1ZXVlLT53ZG1hX3dwdHJfYWRkciA9IG1zZ19xdWV1ZS0+d2RtYV9hZGRyLmRt YV9hZGRyOw0KPiA+ICsNCj4gPiArICAgICAgIGZvciAoaSA9IDA7IGkgPCBOVU1fQlVGRkVSX0NP VU5UOyBpKyspIHsNCj4gPiArICAgICAgICAgICAgICAgbGF0X2J1ZiA9ICZtc2dfcXVldWUtPmxh dF9idWZbaV07DQo+ID4gKw0KPiA+ICsgICAgICAgICAgICAgICBsYXRfYnVmLT53ZG1hX2Vycl9h ZGRyLnNpemUgPSBWREVDX0VSUl9NQVBfU1pfQVZDOw0KPiA+ICsgICAgICAgICAgICAgICBlcnIg PSBtdGtfdmNvZGVjX21lbV9hbGxvYyhjdHgsICZsYXRfYnVmLT53ZG1hX2Vycl9hZGRyKTsNCj4g PiArICAgICAgICAgICAgICAgaWYgKGVycikgew0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAg IG10a192NGwyX2VycigiZmFpbGVkIHRvIGFsbG9jYXRlIHdkbWFfZXJyX2FkZHIgYnVmWyVkXSIs IGkpOw0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgIHJldHVybiAtRU5PTUVNOw0KPiA+ICsg ICAgICAgICAgICAgICB9DQo+ID4gKw0KPiA+ICsgICAgICAgICAgICAgICBsYXRfYnVmLT5zbGlj ZV9iY19hZGRyLnNpemUgPSBWREVDX0xBVF9TTElDRV9IRUFERVJfU1o7DQo+ID4gKyAgICAgICAg ICAgICAgIGVyciA9IG10a192Y29kZWNfbWVtX2FsbG9jKGN0eCwgJmxhdF9idWYtPnNsaWNlX2Jj X2FkZHIpOw0KPiA+ICsgICAgICAgICAgICAgICBpZiAoZXJyKSB7DQo+ID4gKyAgICAgICAgICAg ICAgICAgICAgICAgbXRrX3Y0bDJfZXJyKCJmYWlsZWQgdG8gYWxsb2NhdGUgd2RtYV9hZGRyIGJ1 ZlslZF0iLCBpKTsNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICByZXR1cm4gLUVOT01FTTsN Cj4gPiArICAgICAgICAgICAgICAgfQ0KPiA+ICsNCj4gPiArICAgICAgICAgICAgICAgbGF0X2J1 Zi0+cHJpdmF0ZV9kYXRhID0ga3phbGxvYyhwcml2YXRlX3NpemUsIEdGUF9LRVJORUwpOw0KPiA+ ICsgICAgICAgICAgICAgICBpZiAoIWxhdF9idWYtPnByaXZhdGVfZGF0YSkgew0KPiA+ICsgICAg ICAgICAgICAgICAgICAgICAgIG10a192NGwyX2VycigiZmFpbGVkIHRvIGFsbG9jYXRlIHByaXZh dGVfZGF0YVslZF0iLCBpKTsNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICByZXR1cm4gLUVO T01FTTsNCj4gPiArICAgICAgICAgICAgICAgfQ0KPiA+ICsNCj4gPiArICAgICAgICAgICAgICAg bGF0X2J1Zi0+Y3R4ID0gY3R4Ow0KPiA+ICsgICAgICAgICAgICAgICBsYXRfYnVmLT5jb3JlX2Rl Y29kZSA9IGNvcmVfZGVjb2RlOw0KPiA+ICsgICAgICAgICAgICAgICB2ZGVjX21zZ19xdWV1ZV9i dWZfdG9fbGF0KGxhdF9idWYpOw0KPiA+ICsgICAgICAgfQ0KPiBEb2Vzbid0IGl0IG5lZWQgdG8g Y2FsbCBtdGtfdmNvZGVjX21lbV9mcmVlKCkgYW5kIGtmcmVlKCkgZm9yIGFueSBmYWlsdXJlIHBh dGhzPw0KV2hlbiBhbGxvY2F0ZSBtZW1vcnkgZmFpbCwgd2lsbCBjYWxsIGRlaW5pdCBmdW5jdGlv biBhdXRvLCB0aGVuIGZyZWUgYWxsIGFsbG9jYXRlZCBtZW1vcnkuDQo+ID4gK3N0cnVjdCB2ZGVj X2xhdF9idWYgKnZkZWNfbXNnX3F1ZXVlX2dldF9jb3JlX2J1ZigNCj4gPiArICAgICAgIHN0cnVj dCBtdGtfdmNvZGVjX2RldiAqZGV2KQ0KPiA+ICt7DQo+ID4gKyAgICAgICBzdHJ1Y3QgdmRlY19s YXRfYnVmICpidWY7DQo+ID4gKyAgICAgICBpbnQgcmV0Ow0KPiA+ICsNCj4gPiArICAgICAgIHNw aW5fbG9jaygmZGV2LT5jb3JlX2xvY2spOw0KPiA+ICsgICAgICAgaWYgKGxpc3RfZW1wdHkoJmRl di0+Y29yZV9xdWV1ZSkpIHsNCj4gPiArICAgICAgICAgICAgICAgbXRrX3Y0bDJfZGVidWcoMywg ImNvcmUgcXVldWUgaXMgTlVMTCwgbnVtX2NvcmUgPSAlZCIsIGRldi0+bnVtX2NvcmUpOw0KPiA+ ICsgICAgICAgICAgICAgICBzcGluX3VubG9jaygmZGV2LT5jb3JlX2xvY2spOw0KPiA+ICsgICAg ICAgICAgICAgICByZXQgPSB3YWl0X2V2ZW50X2ZyZWV6YWJsZShkZXYtPmNvcmVfcmVhZCwNCj4g PiArICAgICAgICAgICAgICAgICAgICAgICAhbGlzdF9lbXB0eSgmZGV2LT5jb3JlX3F1ZXVlKSk7 DQo+ID4gKyAgICAgICAgICAgICAgIGlmIChyZXQpDQo+ID4gKyAgICAgICAgICAgICAgICAgICAg ICAgcmV0dXJuIE5VTEw7DQo+IFNob3VsZCBiZSAhcmV0Pw0KQWNjb3JkaW5nIHRoZSBkZWZpbmlk dGlvbiwgd2hlbiBjb25kaXRpb24gaXMgdHJ1ZSwgcmV0dXJuIHZhbHVlIGlzIDAuDQojZGVmaW5l IHdhaXRfZXZlbnRfZnJlZXphYmxlKHdxX2hlYWQsIGNvbmRpdGlvbikJCQkJXA0KKHsJCQkJCQkJ CQkJXA0KCWludCBfX3JldCA9IDA7CQkJCQkJCQlcDQoJbWlnaHRfc2xlZXAoKTsJCQkJCQkJCVwN CglpZiAoIShjb25kaXRpb24pKQkJCQkJCQlcDQoJCV9fcmV0ID0gX193YWl0X2V2ZW50X2ZyZWV6 YWJsZSh3cV9oZWFkLCBjb25kaXRpb24pOwkJXA0KCV9fcmV0OwkJCQkJCQkJCVwNCn0pIA0KPiA+ ICt2b2lkIHZkZWNfbXNnX3F1ZXVlX2J1Zl90b19jb3JlKHN0cnVjdCBtdGtfdmNvZGVjX2RldiAq ZGV2LA0KPiA+ICsgICAgICAgc3RydWN0IHZkZWNfbGF0X2J1ZiAqYnVmKQ0KPiA+ICt7DQo+ID4g KyAgICAgICBzcGluX2xvY2soJmRldi0+Y29yZV9sb2NrKTsNCj4gPiArICAgICAgIGxpc3RfYWRk X3RhaWwoJmJ1Zi0+Y29yZV9saXN0LCAmZGV2LT5jb3JlX3F1ZXVlKTsNCj4gPiArICAgICAgIGRl di0+bnVtX2NvcmUrKzsNCj4gPiArICAgICAgIHdha2VfdXBfYWxsKCZkZXYtPmNvcmVfcmVhZCk7 DQo+ID4gKyAgICAgICBtdGtfdjRsMl9kZWJ1ZygzLCAicXVldSBidWYgYWRkcjogKDB4JXApIiwg YnVmKTsNCj4gVHlwby4NCj4gDQo+ID4gK2Jvb2wgdmRlY19tc2dfcXVldWVfd2FpdF9sYXRfYnVm X2Z1bGwoc3RydWN0IHZkZWNfbXNnX3F1ZXVlICptc2dfcXVldWUpDQo+ID4gK3sNCj4gPiArICAg ICAgIGxvbmcgdGltZW91dF9qaWZmOw0KPiA+ICsgICAgICAgaW50IHJldCwgaTsNCj4gPiArDQo+ ID4gKyAgICAgICBmb3IgKGkgPSAwOyBpIDwgTlVNX0JVRkZFUl9DT1VOVCArIDI7IGkrKykgew0K PiA+ICsgICAgICAgICAgICAgIHRpbWVvdXRfamlmZiA9IG1zZWNzX3RvX2ppZmZpZXMoMTAwMCk7 DQo+ID4gKyAgICAgICAgICAgICAgcmV0ID0gd2FpdF9ldmVudF90aW1lb3V0KG1zZ19xdWV1ZS0+ bGF0X3JlYWQsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgbXNnX3F1ZXVlLT5udW1fbGF0ID09 IE5VTV9CVUZGRVJfQ09VTlQsIHRpbWVvdXRfamlmZik7DQo+ID4gKyAgICAgICAgICAgICAgaWYg KHJldCkgew0KPiA+ICsgICAgICAgICAgICAgICAgICAgICBtdGtfdjRsMl9kZWJ1ZygzLCAic3Vj Y2VzcyB0byBnZXQgbGF0IGJ1ZjogJWQiLA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAg ICAgbXNnX3F1ZXVlLT5udW1fbGF0KTsNCj4gPiArICAgICAgICAgICAgICAgICAgICAgcmV0dXJu IHRydWU7DQo+ID4gKyAgICAgICAgICAgICAgfQ0KPiA+ICsgICAgICAgfQ0KPiBXaHkgZG9lcyBp dCBuZWVkIHRoZSBsb29wPyAgaSBpcyB1bnVzZWQuDQpDb3JlIG1heWJlIGRlY29kZSB0aW1lb3V0 LCBuZWVkIHRvIHdhaXQgYWxsIGNvcmUgYnVmZmVyIHByb2Nlc3MNCmNvbXBsZXRlbHkuDQo+ID4g K3ZvaWQgdmRlY19tc2dfcXVldWVfZGVpbml0KA0KPiA+ICsgICAgICAgc3RydWN0IG10a192Y29k ZWNfY3R4ICpjdHgsDQo+ID4gKyAgICAgICBzdHJ1Y3QgdmRlY19tc2dfcXVldWUgKm1zZ19xdWV1 ZSkNCj4gPiArew0KPiA+ICsgICAgICAgc3RydWN0IHZkZWNfbGF0X2J1ZiAqbGF0X2J1ZjsNCj4g PiArICAgICAgIHN0cnVjdCBtdGtfdmNvZGVjX21lbSAqbWVtOw0KPiA+ICsgICAgICAgaW50IGk7 DQo+ID4gKw0KPiA+ICsgICAgICAgbWVtID0gJm1zZ19xdWV1ZS0+d2RtYV9hZGRyOw0KPiA+ICsg ICAgICAgaWYgKG1lbS0+dmEpDQo+ID4gKyAgICAgICAgICAgICAgIG10a192Y29kZWNfbWVtX2Zy ZWUoY3R4LCBtZW0pOw0KPiA+ICsgICAgICAgZm9yIChpID0gMDsgaSA8IE5VTV9CVUZGRVJfQ09V TlQ7IGkrKykgew0KPiA+ICsgICAgICAgICAgICAgICBsYXRfYnVmID0gJm1zZ19xdWV1ZS0+bGF0 X2J1ZltpXTsNCj4gPiArDQo+ID4gKyAgICAgICAgICAgICAgIG1lbSA9ICZsYXRfYnVmLT53ZG1h X2Vycl9hZGRyOw0KPiA+ICsgICAgICAgICAgICAgICBpZiAobWVtLT52YSkNCj4gPiArICAgICAg ICAgICAgICAgICAgICAgICBtdGtfdmNvZGVjX21lbV9mcmVlKGN0eCwgbWVtKTsNCj4gPiArDQo+ ID4gKyAgICAgICAgICAgICAgIG1lbSA9ICZsYXRfYnVmLT5zbGljZV9iY19hZGRyOw0KPiA+ICsg ICAgICAgICAgICAgICBpZiAobWVtLT52YSkNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICBt dGtfdmNvZGVjX21lbV9mcmVlKGN0eCwgbWVtKTsNCj4gPiArDQo+ID4gKyAgICAgICAgICAgICAg IGlmIChsYXRfYnVmLT5wcml2YXRlX2RhdGEpDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAg a2ZyZWUobGF0X2J1Zi0+cHJpdmF0ZV9kYXRhKTsNCj4gPiArICAgICAgIH0NCj4gPiArDQo+ID4g KyAgICAgICBtc2dfcXVldWUtPmluaXRfZG9uZSA9IGZhbHNlOw0KPiBIYXZlIG5vIGlkZWEgd2hh dCBpbml0X2RvbmUgZG9lcyBpbiB0aGUgY29kZS4gIEl0IGlzIG5vdCBpbmNsdWRlZCBpbg0KPiBh bnkgYnJhbmNoIGNvbmRpdGlvbi4NCldoZW4gY2FsbCB2ZGVjX21zZ19xdWV1ZV9pbml0IHdpbGwg c2V0IHRoaXMgcGFyYW1ldGVyIHRvIHRydWUuDQo+ID4gKy8qKg0KPiA+ICsgKiB2ZGVjX21zZ19x dWV1ZV9pbml0IC0gaW5pdCBsYXQgYnVmZmVyIGluZm9ybWF0aW9uLg0KPiA+ICsgKiBAY3R4OiB2 NGwyIGN0eA0KPiA+ICsgKiBAbXNnX3F1ZXVlOiB1c2VkIHRvIHN0b3JlIHRoZSBsYXQgYnVmZmVy IGluZm9ybWF0aW9uDQo+ID4gKyAqIEBjb3JlX2RlY29kZTogY29yZSBkZWNvZGUgY2FsbGJhY2sg Zm9yIGVhY2ggY29kZWMNCj4gPiArICogQHByaXZhdGVfc2l6ZTogdGhlIHByaXZhdGUgZGF0YSBz aXplIHVzZWQgdG8gc2hhcmUgd2l0aCBjb3JlDQo+ID4gKyAqLw0KPiA+ICtpbnQgdmRlY19tc2df cXVldWVfaW5pdCgNCj4gPiArICAgICAgIHN0cnVjdCBtdGtfdmNvZGVjX2N0eCAqY3R4LA0KPiA+ ICsgICAgICAgc3RydWN0IHZkZWNfbXNnX3F1ZXVlICptc2dfcXVldWUsDQo+ID4gKyAgICAgICBj b3JlX2RlY29kZV9jYl90IGNvcmVfZGVjb2RlLA0KPiA+ICsgICAgICAgaW50IHByaXZhdGVfc2l6 ZSk7DQo+IFdvdWxkIHByZWZlciB0byBoYXZlICptc2dfcXVldWUgYXMgdGhlIGZpcnN0IGFyZ3Vt ZW50IChhbHNvIGFwcGxpZXMgdG8NCj4gYWxsIG9wZXJhdG9ycyBvZiB2ZGVjX21zZ19xdWV1ZSku DQpDYW4gZml4Lg0KPiA+ICsvKioNCj4gPiArICogdmRlY19tc2dfcXVldWVfZ2V0X2NvcmVfYnVm IC0gZ2V0IHVzZWQgY29yZSBidWZmZXIgZm9yIGxhdCBkZWNvZGUuDQo+ID4gKyAqIEBkZXY6IG10 ayB2Y29kZWMgZGV2aWNlDQo+ID4gKyAqLw0KPiA+ICtzdHJ1Y3QgdmRlY19sYXRfYnVmICp2ZGVj X21zZ19xdWV1ZV9nZXRfY29yZV9idWYoDQo+ID4gKyAgICAgICBzdHJ1Y3QgbXRrX3Zjb2RlY19k ZXYgKmRldik7DQo+IFRoaXMgaXMgd2VpcmQ6IHZkZWNfbXNnX3F1ZXVlJ3Mgb3BlcmF0b3IgYnV0 IG1hbmlwdWxhdGluZyBtdGtfdmNvZGVjX2Rldj8NCnZkZWNfbXNnX3F1ZXVlIGlzIHVzZWQgdG8g c2hhcmUgbWVzc2FnZSBiZXR3ZWVuIGxhdCBhbmQgY29yZSwgZm9yIGVhY2gNCmluc3RhbmNlIGhh cyBpdHMgbGF0IG1zZyBxdWV1ZSBsaXN0LCBidXQgYWxsIGluc3RhbmNlIHNoYXJlIG9uZSBjb3Jl IG1zZw0KcXVldWUgbGlzdC4gV2hlbiB0cnkgdG8gZ2V0IGNvcmUgYnVmZmVyIG5lZWQgdG8gZ2V0 IGl0IGZyb20gY29yZSBxdWV1ZQ0KbGlzdC4gVGhlbiBxdWV1ZSBpdCB0byBsYXQgcXVldWUgbGlz dCB3aGVuIGNvcmUgZGVjb2RlIGRvbmUuDQo+ID4gKw0KPiA+ICsvKioNCj4gPiArICogdmRlY19t c2dfcXVldWVfYnVmX3RvX2NvcmUgLSBxdWV1ZSBidWYgdG8gdGhlIGNvcmUgZm9yIGNvcmUgZGVj b2RlLg0KPiA+ICsgKiBAZGV2OiBtdGsgdmNvZGVjIGRldmljZQ0KPiA+ICsgKiBAYnVmOiBjdXJy ZW50IGxhdCBidWZmZXINCj4gPiArICovDQo+ID4gK3ZvaWQgdmRlY19tc2dfcXVldWVfYnVmX3Rv X2NvcmUoc3RydWN0IG10a192Y29kZWNfZGV2ICpkZXYsDQo+ID4gKyAgICAgICBzdHJ1Y3QgdmRl Y19sYXRfYnVmICpidWYpOw0KPiBBbHNvIHdlaXJkLg0KPiANCj4gPiArLyoqDQo+ID4gKyAqIHZk ZWNfbXNnX3F1ZXVlX2J1Zl90b19sYXQgLSBxdWV1ZSBidWYgdG8gbGF0IGZvciBsYXQgZGVjb2Rl Lg0KPiA+ICsgKiBAYnVmOiBjdXJyZW50IGxhdCBidWZmZXINCj4gPiArICovDQo+ID4gK3ZvaWQg dmRlY19tc2dfcXVldWVfYnVmX3RvX2xhdChzdHJ1Y3QgdmRlY19sYXRfYnVmICpidWYpOw0KPiBJ dCBzaG91bGQgYXQgbGVhc3QgYWNjZXB0IGEgc3RydWN0IHZkZWNfbXNnX3F1ZXVlIGFyZ3VtZW50 IChvciB3aGljaA0KPiBtc2cgcXVldWUgc2hvdWxkIHRoZSBidWYgcHV0IGludG8/KS4NCkFsbCBi dWZmZXIgaXMgc3RydWN0IHZkZWNfbGF0X2J1ZiwgdXNlZCB0byBzaGFyZSBpbmZvIGJldHdlZW4g bGF0IGFuZCBjb3JlIHF1ZXVlIGxpc3QuDQo+ID4gKy8qKg0KPiA+ICsgKiB2ZGVjX21zZ19xdWV1 ZV91cGRhdGVfdWJlX3JwdHIgLSB1c2VkIHRvIHVwZGF0YSB0aGUgdWJlIHJlYWQgcG9pbnQuDQo+ IFR5cG8uDQo+IA0KPiA+ICsvKioNCj4gPiArICogdmRlY19tc2dfcXVldWVfdXBkYXRlX3ViZV93 cHRyIC0gdXNlZCB0byB1cGRhdGEgdGhlIHViZSB3cml0ZSBwb2ludC4NCj4gVHlwby4NCj4gDQo+ ID4gKy8qKg0KPiA+ICsgKiB2ZGVjX21zZ19xdWV1ZV9kZWluaXQgLSBkZWluaXQgbGF0IGJ1ZmZl ciBpbmZvcm1hdGlvbi4NCj4gPiArICogQGN0eDogdjRsMiBjdHgNCj4gPiArICogQG1zZ19xdWV1 ZTogdXNlZCB0byBzdG9yZSB0aGUgbGF0IGJ1ZmZlciBpbmZvcm1hdGlvbg0KPiA+ICsgKi8NCj4g PiArdm9pZCB2ZGVjX21zZ19xdWV1ZV9kZWluaXQoDQo+ID4gKyAgICAgICBzdHJ1Y3QgbXRrX3Zj b2RlY19jdHggKmN0eCwNCj4gPiArICAgICAgIHN0cnVjdCB2ZGVjX21zZ19xdWV1ZSAqbXNnX3F1 ZXVlKTsNCj4gV291bGQgcHJlZmVyIHRvIGhhdmUgKm1zZ19xdWV1ZSBhcyB0aGUgZmlyc3QgYXJn dW1lbnQuDQpZZXMsIGNhbiBmaXguDQo+IA0KPiBUaGUgcG9zaXRpb24gb2Ygc3RydWN0IHZkZWNf bXNnX3F1ZXVlIGlzIHdlaXJkLiAgSXQgbG9va3MgbGlrZSB0aGUgbXNnDQo+IHF1ZXVlIGlzIG9u bHkgZm9yIHN0cnVjdCB2ZGVjX2xhdF9idWYuICBJZiBzbywgd291bGQgdmRlY19tc2dfcXVldWUg YmUNCj4gYmV0dGVyIHRvIGNhbGwgdmRlY19sYXRfcXVldWUgb3Igc29tZXRoaW5nIHNpbWlsYXI/ DQo+IA0KPiBJdCBzaG91bGRuJ3QgdG91Y2ggdGhlIGNvcmUgcXVldWUgaW4gbXRrX3Zjb2RlY19k ZXYgYW55d2F5LiAgSXMgaXQNCj4gcG9zc2libGUgdG8gZ2VuZXJhbGl6ZSB0aGUgcXVldWUtcmVs YXRlZCBjb2RlIGZvciBib3RoIGxhdCBhbmQgY29yZQ0KPiBxdWV1ZXM/DQpMYXQgcXVldWUgbGlz dCBpcyBzZXBhcmF0ZWx5IGZvciBlYWNoIGluc3RhbmNlLCBidXQgb25seSBoYXMgb25lIGNvcmUN CnF1ZXVlIGxpc3QuDQoNCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED, USER_AGENT_SANE_2 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 21A85C07E99 for ; Mon, 12 Jul 2021 07:28:21 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CEF5C60230 for ; Mon, 12 Jul 2021 07:28:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CEF5C60230 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=mediatek.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:CC:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=eEIHFmTioQk8uNOBszkZczoKYym3MiHRKbE1koaIWS8=; b=t1zzKei9IsigVy EZDleKD4K2M8bZWRs0Jr98SokPU7qHPlbQ+a5+I6RCYLaiDapLHp2E3CiMLS4g9UMOtA25JyZHGPJ TL7hBaRYKRFeAVOr4pkR3flSkrDXMwI3Yfx2LRMgSkLiZWTummU2WQ5OowppRo+ZCWngIclcoDn0W gZtbTRfClOVEAtv/btxgV9weVaa8ghxCpRbnvInTb/zCzvgWWjlF3zTVuDiT0xl3hcZMTdVGjd7in Eg7OsWzkSXa/RbN7HBLuuHLB3UI3JGcxmPsZdL1fOBv19Vr+2m1hIeOquKty+UQz/rb9E+IT6oxeO L9fdMyrftxFkCMeiTfSg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m2qML-006LDm-GH; Mon, 12 Jul 2021 07:28:09 +0000 Received: from mailgw01.mediatek.com ([216.200.240.184]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1m2qMI-006LBv-PR; Mon, 12 Jul 2021 07:28:08 +0000 X-UUID: 110591268a634a76b668bd4f73f61441-20210712 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=Content-Transfer-Encoding:MIME-Version:Content-Type:References:In-Reply-To:Date:CC:To:From:Subject:Message-ID; bh=RaHR82D17SfxYYxbMS4VegTIpCJoSq06AsIDCLpUfYU=; b=OJ++dcyktadjb9gMSdjrbHgWMwtj5tl6Niat8K1goIZ++yGT3QsWXxzDuSSCiPlYz22xkVZvF39RRVfL67MGPhH0oKyMckft8Vjosl6VXRO8cxBoaKu2mwQJktRx4mU75gJy4ytblCh3w8l3OC87t44rULenK0VBOzMWR67SMaw=; X-UUID: 110591268a634a76b668bd4f73f61441-20210712 Received: from mtkcas67.mediatek.inc [(172.29.193.45)] by mailgw01.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLSv1.2 ECDHE-RSA-AES256-SHA384 256/256) with ESMTP id 754523131; Mon, 12 Jul 2021 00:28:00 -0700 Received: from mtkmbs07n1.mediatek.inc (172.21.101.16) by MTKMBS62N2.mediatek.inc (172.29.193.42) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Mon, 12 Jul 2021 00:27:58 -0700 Received: from mtkcas07.mediatek.inc (172.21.101.84) by mtkmbs07n1.mediatek.inc (172.21.101.16) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Mon, 12 Jul 2021 15:27:56 +0800 Received: from [10.17.3.153] (10.17.3.153) by mtkcas07.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Mon, 12 Jul 2021 15:27:55 +0800 Message-ID: <1626074875.7221.15.camel@mhfsdcap03> Subject: Re: [PATCH v1, 07/14] media: mtk-vcodec: Add msg queue feature for lat and core architecture From: mtk12024 To: Tzung-Bi Shih CC: Alexandre Courbot , Hans Verkuil , Tzung-Bi Shih , "Tiffany Lin" , Andrew-CT Chen , Mauro Carvalho Chehab , Rob Herring , Matthias Brugger , Tomasz Figa , Hsin-Yi Wang , "Fritz Koenig" , Irui Wang , , , , , , , Date: Mon, 12 Jul 2021 15:27:55 +0800 In-Reply-To: References: <20210707062157.21176-1-yunfei.dong@mediatek.com> <20210707062157.21176-8-yunfei.dong@mediatek.com> X-Mailer: Evolution 3.10.4-0ubuntu2 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210712_002806_864089_EE2B4B3A X-CRM114-Status: GOOD ( 31.73 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org Hi Tzung-Bi, Thanks for your detail feedback. I add the description according to your each comments. On Fri, 2021-07-09 at 17:39 +0800, Tzung-Bi Shih wrote: > On Wed, Jul 7, 2021 at 2:22 PM Yunfei Dong wrote: > > @@ -464,6 +469,11 @@ struct mtk_vcodec_enc_pdata { > > * comp_dev: component hardware device > > * component_node: component node > > * comp_idx: component index > > + * > > + * core_read: Wait queue used to signalize when core get useful lat buffer > > + * core_queue: List of V4L2 lat_buf > To be neat, replace "Wait" to "wait" and "List" to "list". Will fix. > > +int vdec_msg_queue_init( > > + struct mtk_vcodec_ctx *ctx, > > + struct vdec_msg_queue *msg_queue, > > + core_decode_cb_t core_decode, > > + int private_size) > > +{ > > + struct vdec_lat_buf *lat_buf; > > + int i, err; > > + > > + init_waitqueue_head(&msg_queue->lat_read); > > + INIT_LIST_HEAD(&msg_queue->lat_queue); > > + spin_lock_init(&msg_queue->lat_lock); > > + msg_queue->num_lat = 0; > > + > > + msg_queue->wdma_addr.size = vde_msg_queue_get_trans_size( > > + ctx->picinfo.buf_w, ctx->picinfo.buf_h); > > + > > + err = mtk_vcodec_mem_alloc(ctx, &msg_queue->wdma_addr); > > + if (err) { > > + mtk_v4l2_err("failed to allocate wdma_addr buf"); > > + return -ENOMEM; > > + } > > + msg_queue->wdma_rptr_addr = msg_queue->wdma_addr.dma_addr; > > + msg_queue->wdma_wptr_addr = msg_queue->wdma_addr.dma_addr; > > + > > + for (i = 0; i < NUM_BUFFER_COUNT; i++) { > > + lat_buf = &msg_queue->lat_buf[i]; > > + > > + lat_buf->wdma_err_addr.size = VDEC_ERR_MAP_SZ_AVC; > > + err = mtk_vcodec_mem_alloc(ctx, &lat_buf->wdma_err_addr); > > + if (err) { > > + mtk_v4l2_err("failed to allocate wdma_err_addr buf[%d]", i); > > + return -ENOMEM; > > + } > > + > > + lat_buf->slice_bc_addr.size = VDEC_LAT_SLICE_HEADER_SZ; > > + err = mtk_vcodec_mem_alloc(ctx, &lat_buf->slice_bc_addr); > > + if (err) { > > + mtk_v4l2_err("failed to allocate wdma_addr buf[%d]", i); > > + return -ENOMEM; > > + } > > + > > + lat_buf->private_data = kzalloc(private_size, GFP_KERNEL); > > + if (!lat_buf->private_data) { > > + mtk_v4l2_err("failed to allocate private_data[%d]", i); > > + return -ENOMEM; > > + } > > + > > + lat_buf->ctx = ctx; > > + lat_buf->core_decode = core_decode; > > + vdec_msg_queue_buf_to_lat(lat_buf); > > + } > Doesn't it need to call mtk_vcodec_mem_free() and kfree() for any failure paths? When allocate memory fail, will call deinit function auto, then free all allocated memory. > > +struct vdec_lat_buf *vdec_msg_queue_get_core_buf( > > + struct mtk_vcodec_dev *dev) > > +{ > > + struct vdec_lat_buf *buf; > > + int ret; > > + > > + spin_lock(&dev->core_lock); > > + if (list_empty(&dev->core_queue)) { > > + mtk_v4l2_debug(3, "core queue is NULL, num_core = %d", dev->num_core); > > + spin_unlock(&dev->core_lock); > > + ret = wait_event_freezable(dev->core_read, > > + !list_empty(&dev->core_queue)); > > + if (ret) > > + return NULL; > Should be !ret? According the definidtion, when condition is true, return value is 0. #define wait_event_freezable(wq_head, condition) \ ({ \ int __ret = 0; \ might_sleep(); \ if (!(condition)) \ __ret = __wait_event_freezable(wq_head, condition); \ __ret; \ }) > > +void vdec_msg_queue_buf_to_core(struct mtk_vcodec_dev *dev, > > + struct vdec_lat_buf *buf) > > +{ > > + spin_lock(&dev->core_lock); > > + list_add_tail(&buf->core_list, &dev->core_queue); > > + dev->num_core++; > > + wake_up_all(&dev->core_read); > > + mtk_v4l2_debug(3, "queu buf addr: (0x%p)", buf); > Typo. > > > +bool vdec_msg_queue_wait_lat_buf_full(struct vdec_msg_queue *msg_queue) > > +{ > > + long timeout_jiff; > > + int ret, i; > > + > > + for (i = 0; i < NUM_BUFFER_COUNT + 2; i++) { > > + timeout_jiff = msecs_to_jiffies(1000); > > + ret = wait_event_timeout(msg_queue->lat_read, > > + msg_queue->num_lat == NUM_BUFFER_COUNT, timeout_jiff); > > + if (ret) { > > + mtk_v4l2_debug(3, "success to get lat buf: %d", > > + msg_queue->num_lat); > > + return true; > > + } > > + } > Why does it need the loop? i is unused. Core maybe decode timeout, need to wait all core buffer process completely. > > +void vdec_msg_queue_deinit( > > + struct mtk_vcodec_ctx *ctx, > > + struct vdec_msg_queue *msg_queue) > > +{ > > + struct vdec_lat_buf *lat_buf; > > + struct mtk_vcodec_mem *mem; > > + int i; > > + > > + mem = &msg_queue->wdma_addr; > > + if (mem->va) > > + mtk_vcodec_mem_free(ctx, mem); > > + for (i = 0; i < NUM_BUFFER_COUNT; i++) { > > + lat_buf = &msg_queue->lat_buf[i]; > > + > > + mem = &lat_buf->wdma_err_addr; > > + if (mem->va) > > + mtk_vcodec_mem_free(ctx, mem); > > + > > + mem = &lat_buf->slice_bc_addr; > > + if (mem->va) > > + mtk_vcodec_mem_free(ctx, mem); > > + > > + if (lat_buf->private_data) > > + kfree(lat_buf->private_data); > > + } > > + > > + msg_queue->init_done = false; > Have no idea what init_done does in the code. It is not included in > any branch condition. When call vdec_msg_queue_init will set this parameter to true. > > +/** > > + * vdec_msg_queue_init - init lat buffer information. > > + * @ctx: v4l2 ctx > > + * @msg_queue: used to store the lat buffer information > > + * @core_decode: core decode callback for each codec > > + * @private_size: the private data size used to share with core > > + */ > > +int vdec_msg_queue_init( > > + struct mtk_vcodec_ctx *ctx, > > + struct vdec_msg_queue *msg_queue, > > + core_decode_cb_t core_decode, > > + int private_size); > Would prefer to have *msg_queue as the first argument (also applies to > all operators of vdec_msg_queue). Can fix. > > +/** > > + * vdec_msg_queue_get_core_buf - get used core buffer for lat decode. > > + * @dev: mtk vcodec device > > + */ > > +struct vdec_lat_buf *vdec_msg_queue_get_core_buf( > > + struct mtk_vcodec_dev *dev); > This is weird: vdec_msg_queue's operator but manipulating mtk_vcodec_dev? vdec_msg_queue is used to share message between lat and core, for each instance has its lat msg queue list, but all instance share one core msg queue list. When try to get core buffer need to get it from core queue list. Then queue it to lat queue list when core decode done. > > + > > +/** > > + * vdec_msg_queue_buf_to_core - queue buf to the core for core decode. > > + * @dev: mtk vcodec device > > + * @buf: current lat buffer > > + */ > > +void vdec_msg_queue_buf_to_core(struct mtk_vcodec_dev *dev, > > + struct vdec_lat_buf *buf); > Also weird. > > > +/** > > + * vdec_msg_queue_buf_to_lat - queue buf to lat for lat decode. > > + * @buf: current lat buffer > > + */ > > +void vdec_msg_queue_buf_to_lat(struct vdec_lat_buf *buf); > It should at least accept a struct vdec_msg_queue argument (or which > msg queue should the buf put into?). All buffer is struct vdec_lat_buf, used to share info between lat and core queue list. > > +/** > > + * vdec_msg_queue_update_ube_rptr - used to updata the ube read point. > Typo. > > > +/** > > + * vdec_msg_queue_update_ube_wptr - used to updata the ube write point. > Typo. > > > +/** > > + * vdec_msg_queue_deinit - deinit lat buffer information. > > + * @ctx: v4l2 ctx > > + * @msg_queue: used to store the lat buffer information > > + */ > > +void vdec_msg_queue_deinit( > > + struct mtk_vcodec_ctx *ctx, > > + struct vdec_msg_queue *msg_queue); > Would prefer to have *msg_queue as the first argument. Yes, can fix. > > The position of struct vdec_msg_queue is weird. It looks like the msg > queue is only for struct vdec_lat_buf. If so, would vdec_msg_queue be > better to call vdec_lat_queue or something similar? > > It shouldn't touch the core queue in mtk_vcodec_dev anyway. Is it > possible to generalize the queue-related code for both lat and core > queues? Lat queue list is separately for each instance, but only has one core queue list. _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED, USER_AGENT_SANE_2 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 16819C07E9E for ; Mon, 12 Jul 2021 07:29:56 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CFD256115A for ; Mon, 12 Jul 2021 07:29:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CFD256115A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=mediatek.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:CC:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=5AIPXQeNo2/HHvMCoF6NLodsvB4zc2fvPSo3BfGN3MM=; b=BkMEfutkHrpTEr Mwj7izq+keC8j5b/3Bhx83FxgidAKCx/ert355DJJhJvjiUb4SN4eCNyDuBraSWh1w7wSz9uakOmU r3mjlpGmD2++ICkIUL+ZLdrc8Na3fW9Nv39KoM9zv7RFoKqpbcHBJ4cDoub8N4/nlEfIYYuUTjxSW IkxpOrbgJWdnrNA8LIulsZBGCGKfnwgrx4YLmgTIPkXH3r3Hri09UPDz93OArxVhZttoTh7scnfcj FlFKnMj5IWF9nl9lnlxOJnoklwBtjaixEjIi9ttDXUpTVv863mum1FPnZjRU8n98jlaXnoY0vCzeD EuIFetXM8WKSOQAOfPwA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m2qMN-006LE0-NE; Mon, 12 Jul 2021 07:28:11 +0000 Received: from mailgw01.mediatek.com ([216.200.240.184]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1m2qMI-006LBv-PR; Mon, 12 Jul 2021 07:28:08 +0000 X-UUID: 110591268a634a76b668bd4f73f61441-20210712 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=Content-Transfer-Encoding:MIME-Version:Content-Type:References:In-Reply-To:Date:CC:To:From:Subject:Message-ID; bh=RaHR82D17SfxYYxbMS4VegTIpCJoSq06AsIDCLpUfYU=; b=OJ++dcyktadjb9gMSdjrbHgWMwtj5tl6Niat8K1goIZ++yGT3QsWXxzDuSSCiPlYz22xkVZvF39RRVfL67MGPhH0oKyMckft8Vjosl6VXRO8cxBoaKu2mwQJktRx4mU75gJy4ytblCh3w8l3OC87t44rULenK0VBOzMWR67SMaw=; X-UUID: 110591268a634a76b668bd4f73f61441-20210712 Received: from mtkcas67.mediatek.inc [(172.29.193.45)] by mailgw01.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLSv1.2 ECDHE-RSA-AES256-SHA384 256/256) with ESMTP id 754523131; Mon, 12 Jul 2021 00:28:00 -0700 Received: from mtkmbs07n1.mediatek.inc (172.21.101.16) by MTKMBS62N2.mediatek.inc (172.29.193.42) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Mon, 12 Jul 2021 00:27:58 -0700 Received: from mtkcas07.mediatek.inc (172.21.101.84) by mtkmbs07n1.mediatek.inc (172.21.101.16) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Mon, 12 Jul 2021 15:27:56 +0800 Received: from [10.17.3.153] (10.17.3.153) by mtkcas07.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Mon, 12 Jul 2021 15:27:55 +0800 Message-ID: <1626074875.7221.15.camel@mhfsdcap03> Subject: Re: [PATCH v1, 07/14] media: mtk-vcodec: Add msg queue feature for lat and core architecture From: mtk12024 To: Tzung-Bi Shih CC: Alexandre Courbot , Hans Verkuil , Tzung-Bi Shih , "Tiffany Lin" , Andrew-CT Chen , Mauro Carvalho Chehab , Rob Herring , Matthias Brugger , Tomasz Figa , Hsin-Yi Wang , "Fritz Koenig" , Irui Wang , , , , , , , Date: Mon, 12 Jul 2021 15:27:55 +0800 In-Reply-To: References: <20210707062157.21176-1-yunfei.dong@mediatek.com> <20210707062157.21176-8-yunfei.dong@mediatek.com> X-Mailer: Evolution 3.10.4-0ubuntu2 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210712_002806_864089_EE2B4B3A X-CRM114-Status: GOOD ( 31.73 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Tzung-Bi, Thanks for your detail feedback. I add the description according to your each comments. On Fri, 2021-07-09 at 17:39 +0800, Tzung-Bi Shih wrote: > On Wed, Jul 7, 2021 at 2:22 PM Yunfei Dong wrote: > > @@ -464,6 +469,11 @@ struct mtk_vcodec_enc_pdata { > > * comp_dev: component hardware device > > * component_node: component node > > * comp_idx: component index > > + * > > + * core_read: Wait queue used to signalize when core get useful lat buffer > > + * core_queue: List of V4L2 lat_buf > To be neat, replace "Wait" to "wait" and "List" to "list". Will fix. > > +int vdec_msg_queue_init( > > + struct mtk_vcodec_ctx *ctx, > > + struct vdec_msg_queue *msg_queue, > > + core_decode_cb_t core_decode, > > + int private_size) > > +{ > > + struct vdec_lat_buf *lat_buf; > > + int i, err; > > + > > + init_waitqueue_head(&msg_queue->lat_read); > > + INIT_LIST_HEAD(&msg_queue->lat_queue); > > + spin_lock_init(&msg_queue->lat_lock); > > + msg_queue->num_lat = 0; > > + > > + msg_queue->wdma_addr.size = vde_msg_queue_get_trans_size( > > + ctx->picinfo.buf_w, ctx->picinfo.buf_h); > > + > > + err = mtk_vcodec_mem_alloc(ctx, &msg_queue->wdma_addr); > > + if (err) { > > + mtk_v4l2_err("failed to allocate wdma_addr buf"); > > + return -ENOMEM; > > + } > > + msg_queue->wdma_rptr_addr = msg_queue->wdma_addr.dma_addr; > > + msg_queue->wdma_wptr_addr = msg_queue->wdma_addr.dma_addr; > > + > > + for (i = 0; i < NUM_BUFFER_COUNT; i++) { > > + lat_buf = &msg_queue->lat_buf[i]; > > + > > + lat_buf->wdma_err_addr.size = VDEC_ERR_MAP_SZ_AVC; > > + err = mtk_vcodec_mem_alloc(ctx, &lat_buf->wdma_err_addr); > > + if (err) { > > + mtk_v4l2_err("failed to allocate wdma_err_addr buf[%d]", i); > > + return -ENOMEM; > > + } > > + > > + lat_buf->slice_bc_addr.size = VDEC_LAT_SLICE_HEADER_SZ; > > + err = mtk_vcodec_mem_alloc(ctx, &lat_buf->slice_bc_addr); > > + if (err) { > > + mtk_v4l2_err("failed to allocate wdma_addr buf[%d]", i); > > + return -ENOMEM; > > + } > > + > > + lat_buf->private_data = kzalloc(private_size, GFP_KERNEL); > > + if (!lat_buf->private_data) { > > + mtk_v4l2_err("failed to allocate private_data[%d]", i); > > + return -ENOMEM; > > + } > > + > > + lat_buf->ctx = ctx; > > + lat_buf->core_decode = core_decode; > > + vdec_msg_queue_buf_to_lat(lat_buf); > > + } > Doesn't it need to call mtk_vcodec_mem_free() and kfree() for any failure paths? When allocate memory fail, will call deinit function auto, then free all allocated memory. > > +struct vdec_lat_buf *vdec_msg_queue_get_core_buf( > > + struct mtk_vcodec_dev *dev) > > +{ > > + struct vdec_lat_buf *buf; > > + int ret; > > + > > + spin_lock(&dev->core_lock); > > + if (list_empty(&dev->core_queue)) { > > + mtk_v4l2_debug(3, "core queue is NULL, num_core = %d", dev->num_core); > > + spin_unlock(&dev->core_lock); > > + ret = wait_event_freezable(dev->core_read, > > + !list_empty(&dev->core_queue)); > > + if (ret) > > + return NULL; > Should be !ret? According the definidtion, when condition is true, return value is 0. #define wait_event_freezable(wq_head, condition) \ ({ \ int __ret = 0; \ might_sleep(); \ if (!(condition)) \ __ret = __wait_event_freezable(wq_head, condition); \ __ret; \ }) > > +void vdec_msg_queue_buf_to_core(struct mtk_vcodec_dev *dev, > > + struct vdec_lat_buf *buf) > > +{ > > + spin_lock(&dev->core_lock); > > + list_add_tail(&buf->core_list, &dev->core_queue); > > + dev->num_core++; > > + wake_up_all(&dev->core_read); > > + mtk_v4l2_debug(3, "queu buf addr: (0x%p)", buf); > Typo. > > > +bool vdec_msg_queue_wait_lat_buf_full(struct vdec_msg_queue *msg_queue) > > +{ > > + long timeout_jiff; > > + int ret, i; > > + > > + for (i = 0; i < NUM_BUFFER_COUNT + 2; i++) { > > + timeout_jiff = msecs_to_jiffies(1000); > > + ret = wait_event_timeout(msg_queue->lat_read, > > + msg_queue->num_lat == NUM_BUFFER_COUNT, timeout_jiff); > > + if (ret) { > > + mtk_v4l2_debug(3, "success to get lat buf: %d", > > + msg_queue->num_lat); > > + return true; > > + } > > + } > Why does it need the loop? i is unused. Core maybe decode timeout, need to wait all core buffer process completely. > > +void vdec_msg_queue_deinit( > > + struct mtk_vcodec_ctx *ctx, > > + struct vdec_msg_queue *msg_queue) > > +{ > > + struct vdec_lat_buf *lat_buf; > > + struct mtk_vcodec_mem *mem; > > + int i; > > + > > + mem = &msg_queue->wdma_addr; > > + if (mem->va) > > + mtk_vcodec_mem_free(ctx, mem); > > + for (i = 0; i < NUM_BUFFER_COUNT; i++) { > > + lat_buf = &msg_queue->lat_buf[i]; > > + > > + mem = &lat_buf->wdma_err_addr; > > + if (mem->va) > > + mtk_vcodec_mem_free(ctx, mem); > > + > > + mem = &lat_buf->slice_bc_addr; > > + if (mem->va) > > + mtk_vcodec_mem_free(ctx, mem); > > + > > + if (lat_buf->private_data) > > + kfree(lat_buf->private_data); > > + } > > + > > + msg_queue->init_done = false; > Have no idea what init_done does in the code. It is not included in > any branch condition. When call vdec_msg_queue_init will set this parameter to true. > > +/** > > + * vdec_msg_queue_init - init lat buffer information. > > + * @ctx: v4l2 ctx > > + * @msg_queue: used to store the lat buffer information > > + * @core_decode: core decode callback for each codec > > + * @private_size: the private data size used to share with core > > + */ > > +int vdec_msg_queue_init( > > + struct mtk_vcodec_ctx *ctx, > > + struct vdec_msg_queue *msg_queue, > > + core_decode_cb_t core_decode, > > + int private_size); > Would prefer to have *msg_queue as the first argument (also applies to > all operators of vdec_msg_queue). Can fix. > > +/** > > + * vdec_msg_queue_get_core_buf - get used core buffer for lat decode. > > + * @dev: mtk vcodec device > > + */ > > +struct vdec_lat_buf *vdec_msg_queue_get_core_buf( > > + struct mtk_vcodec_dev *dev); > This is weird: vdec_msg_queue's operator but manipulating mtk_vcodec_dev? vdec_msg_queue is used to share message between lat and core, for each instance has its lat msg queue list, but all instance share one core msg queue list. When try to get core buffer need to get it from core queue list. Then queue it to lat queue list when core decode done. > > + > > +/** > > + * vdec_msg_queue_buf_to_core - queue buf to the core for core decode. > > + * @dev: mtk vcodec device > > + * @buf: current lat buffer > > + */ > > +void vdec_msg_queue_buf_to_core(struct mtk_vcodec_dev *dev, > > + struct vdec_lat_buf *buf); > Also weird. > > > +/** > > + * vdec_msg_queue_buf_to_lat - queue buf to lat for lat decode. > > + * @buf: current lat buffer > > + */ > > +void vdec_msg_queue_buf_to_lat(struct vdec_lat_buf *buf); > It should at least accept a struct vdec_msg_queue argument (or which > msg queue should the buf put into?). All buffer is struct vdec_lat_buf, used to share info between lat and core queue list. > > +/** > > + * vdec_msg_queue_update_ube_rptr - used to updata the ube read point. > Typo. > > > +/** > > + * vdec_msg_queue_update_ube_wptr - used to updata the ube write point. > Typo. > > > +/** > > + * vdec_msg_queue_deinit - deinit lat buffer information. > > + * @ctx: v4l2 ctx > > + * @msg_queue: used to store the lat buffer information > > + */ > > +void vdec_msg_queue_deinit( > > + struct mtk_vcodec_ctx *ctx, > > + struct vdec_msg_queue *msg_queue); > Would prefer to have *msg_queue as the first argument. Yes, can fix. > > The position of struct vdec_msg_queue is weird. It looks like the msg > queue is only for struct vdec_lat_buf. If so, would vdec_msg_queue be > better to call vdec_lat_queue or something similar? > > It shouldn't touch the core queue in mtk_vcodec_dev anyway. Is it > possible to generalize the queue-related code for both lat and core > queues? Lat queue list is separately for each instance, but only has one core queue list. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel