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=-2.4 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,UNPARSEABLE_RELAY,USER_AGENT_SANE_2 autolearn=no 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 9C5D0C47254 for ; Sat, 9 May 2020 10:29:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6A567214D8 for ; Sat, 9 May 2020 10:29:40 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=mediatek.com header.i=@mediatek.com header.b="EH6bU/iv" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728152AbgEIK3j (ORCPT ); Sat, 9 May 2020 06:29:39 -0400 Received: from mailgw02.mediatek.com ([210.61.82.184]:5023 "EHLO mailgw02.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1726951AbgEIK3j (ORCPT ); Sat, 9 May 2020 06:29:39 -0400 X-UUID: fb7970c2f449403a90bdfbd90fd4b0a0-20200509 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=epJDxfn56pm1KYyJJsHHZHZ67ESMx3iBBECMEcHoO2Q=; b=EH6bU/ivwKNlZGtx3QmkhDPM1Ncw2sUxTOIC6t26jrHHE46HsNwumNC5aVmLPeLp8WX0TNQiGMOJTgU7aIGy4upzWOLIbefjAEw2rmbtDcIAkUeVllVLTV+yqGoLy+QaKwDN1EA8tspEhwgakDJbGlIoohnSn9DV2BfnEaLv630=; X-UUID: fb7970c2f449403a90bdfbd90fd4b0a0-20200509 Received: from mtkcas10.mediatek.inc [(172.21.101.39)] by mailgw02.mediatek.com (envelope-from ) (Cellopoint E-mail Firewall v4.1.10 Build 0809 with TLS) with ESMTP id 571192746; Sat, 09 May 2020 18:29:28 +0800 Received: from MTKCAS36.mediatek.inc (172.27.4.186) by mtkmbs07n1.mediatek.inc (172.21.101.16) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Sat, 9 May 2020 18:29:25 +0800 Received: from [10.17.3.153] (10.17.3.153) by MTKCAS36.mediatek.inc (172.27.4.170) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Sat, 9 May 2020 18:29:24 +0800 Message-ID: <1589020095.24163.150.camel@mhfsdcap03> Subject: Re: [PATCH v7 11/11] media: platform: Add jpeg dec/enc feature From: Xia Jiang To: Tomasz Figa CC: Hans Verkuil , Mauro Carvalho Chehab , Rob Herring , "Matthias Brugger" , Rick Chang , , , , , , Marek Szyprowski , Date: Sat, 9 May 2020 18:28:15 +0800 In-Reply-To: <20200501173712.GB218308@chromium.org> References: <20200303123446.20095-1-xia.jiang@mediatek.com> <20200303123446.20095-12-xia.jiang@mediatek.com> <20200306112337.GA163286@chromium.org> <1587009795.24163.87.camel@mhfsdcap03> <20200501173712.GB218308@chromium.org> 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 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org T24gRnJpLCAyMDIwLTA1LTAxIGF0IDE3OjM3ICswMDAwLCBUb21hc3ogRmlnYSB3cm90ZToNCj4g SGkgWGlhLA0KPiANCj4gT24gVGh1LCBBcHIgMTYsIDIwMjAgYXQgMTI6MDM6MTVQTSArMDgwMCwg WGlhIEppYW5nIHdyb3RlOg0KPiA+IE9uIEZyaSwgMjAyMC0wMy0wNiBhdCAyMDoyMyArMDkwMCwg VG9tYXN6IEZpZ2Egd3JvdGU6DQo+ID4gPiBIaSBYaWEsDQo+ID4gPiANCj4gPiA+IE9uIFR1ZSwg TWFyIDAzLCAyMDIwIGF0IDA4OjM0OjQ2UE0gKzA4MDAsIFhpYSBKaWFuZyB3cm90ZToNCj4gPiA+ ID4gQWRkIG10ayBqcGVnIGVuY29kZSB2NGwyIGRyaXZlciBiYXNlZCBvbiBqcGVnIGRlY29kZSwg YmVjYXVzZSB0aGF0IGpwZWcNCj4gPiA+ID4gZGVjb2RlIGFuZCBlbmNvZGUgaGF2ZSBncmVhdCBz aW1pbGFyaXRpZXMgd2l0aCBmdW5jdGlvbiBvcGVyYXRpb24uDQo+ID4gPiANCj4gPiA+IFRoYW5r IHlvdSBmb3IgdGhlIHBhdGNoLiBQbGVhc2Ugc2VlIG15IGNvbW1lbnRzIGlubGluZS4NCj4gPiAN Cj4gPiBEZWFyIFRvbWFzeiwNCj4gPiANCj4gPiBUaGFuayB5b3UgZm9yIHlvdXIgcmVwbHkuIEkg aGF2ZSBmb2xsb3dlZCB5b3VyIGFkdmljZSBhbmQgc3VibWl0ZWQgdjgNCj4gPiB2ZXJzaW9uIHBh dGNoLg0KPiA+IA0KPiA+IFBsZWFzZSBjaGVjayBteSByZXBseSBiZWxvdy4NCkRlYXIgVG9tYXN6 LA0KSSBoYXZlIHNvbWUgY29uZnVzZSBhYm91dCB5b3VyIGFkdmljZSwgcGxlYXNlIGNoZWNrIG15 IHJlcGx5IGJlbG93Lg0KPiBbc25pcF0NCj4gPiA+IA0KPiA+ID4gPiAgDQo+ID4gPiA+IC0Jc3dp dGNoIChzLT50YXJnZXQpIHsNCj4gPiA+ID4gLQljYXNlIFY0TDJfU0VMX1RHVF9DT01QT1NFOg0K PiA+ID4gPiAtCQlzLT5yLmxlZnQgPSAwOw0KPiA+ID4gPiAtCQlzLT5yLnRvcCA9IDA7DQo+ID4g PiA+IC0JCWN0eC0+b3V0X3EudyA9IHMtPnIud2lkdGg7DQo+ID4gPiA+IC0JCWN0eC0+b3V0X3Eu aCA9IHMtPnIuaGVpZ2h0Ow0KPiA+ID4gPiAtCQlicmVhazsNCj4gPiA+ID4gLQlkZWZhdWx0Og0K PiA+ID4gPiAtCQlyZXR1cm4gLUVJTlZBTDsNCj4gPiA+ID4gKwkJc3dpdGNoIChzLT50YXJnZXQp IHsNCj4gPiA+ID4gKwkJY2FzZSBWNEwyX1NFTF9UR1RfQ1JPUDoNCj4gPiA+ID4gKwkJCXMtPnIu bGVmdCA9IDA7DQo+ID4gPiA+ICsJCQlzLT5yLnRvcCA9IDA7DQo+ID4gPiA+ICsJCQljdHgtPm91 dF9xLncgPSBzLT5yLndpZHRoOw0KPiA+ID4gPiArCQkJY3R4LT5vdXRfcS5oID0gcy0+ci5oZWln aHQ7DQo+ID4gPiANCj4gPiA+IFdoYXQgaGFwcGVucyBpZiB0aGUgdXNlcnNwYWNlIHByb3ZpZGVz IGEgdmFsdWUgYmlnZ2VyIHRoYW4gY3VycmVudCBmb3JtYXQ/DQo+ID4gd2UgbmVlZCBnZXQgdGhl IG1pbiB2YWx1ZSBvZiB1c2Vyc3BhY2UgdmFsdWUgYW5kIGN1cnJlbnQgdmFsdWUsY2hhbmdlZA0K PiA+IGl0IGxpa2UgdGhpczoNCj4gPiBjdHgtPm91dF9xLncgPSBtaW4ocy0+ci53aWR0aCwgY3R4 LT5vdXRfcS53KTsNCj4gPiBjdHgtPm91dF9xLmggPSBtaW4ocy0+ci5oZWlnaHQsY3R4LT5vdXRf cS5oKTsNCj4gDQo+IFNpbmNlIGN0eC0+b3V0X3EgaXMgbW9kaWZpZWQgYnkgdGhpcyBmdW5jdGlv biwgd291bGRuJ3QgdGhhdCBjYXVzZQ0KPiBwcm9ibGVtcyBpZiBTX1NFTEVDVElPTiB3YXMgY2Fs bGVkIHR3byB0aW1lcywgZmlyc3Qgd2l0aCBhIHNtYWxsZXINCj4gcmVjdGFuZ2xlIGFuZCB0aGVu IHdpdGggYSBiaWdnZXIgb25lPyBXZSBzaG91bGQgc3RvcmUgdGhlIGFjdGl2ZSBjcm9wDQo+IGFu ZCBmb3JtYXQgc2VwYXJhdGVseSBhbmQgdXNlIHRoZSBsYXR0ZXIgZm9yIG1pbigpLg0KQWRkIGEg bWVtYmVyIHZhcmlhYmxlKHN0cnVjdCB2NGwyX3JlY3QpIGluIG91dF9xIHN0cnVjdHVyZSBmb3Ig c3RvcmluZw0KdGhlIGFjdGl2ZSBjcm9wLCBsaWtlIHRoaXM6DQpzLT5yLndpZHRoID0gIG1pbihz LT5yLndpZHRoLCBjdHgtPm91dF9xLncpOw0Kcy0+ci5oZWlnaHQgPSBtaW4ocy0+ci5oZWlnaHQs Y3R4LT5vdXRfcS5oKTsNCmN0eC0+b3V0X3EucmVjdC53aWR0aCA9IHMtPnIud2lkdGg7DQpjdHgt Pm91dF9xLnJlY3QuaGVpZ2h0ID0gIHMtPnIuaGVpZ2h0Ow0KSXMgdGhhdCBvaz8NCj4gDQo+IFtz bmlwXQ0KPiA+ID4gPiAgDQo+ID4gPiA+ICAJd2hpbGUgKCh2YiA9IG10a19qcGVnX2J1Zl9yZW1v dmUoY3R4LCBxLT50eXBlKSkpDQo+ID4gPiA+ICAJCXY0bDJfbTJtX2J1Zl9kb25lKHZiLCBWQjJf QlVGX1NUQVRFX0VSUk9SKTsNCj4gPiA+ID4gQEAgLTc3Miw2ICsxMDExLDQ1IEBAIHN0YXRpYyBp bnQgbXRrX2pwZWdfc2V0X2RlY19kc3Qoc3RydWN0IG10a19qcGVnX2N0eCAqY3R4LA0KPiA+ID4g PiAgCXJldHVybiAwOw0KPiA+ID4gPiAgfQ0KPiA+ID4gPiAgDQo+ID4gPiA+ICtzdGF0aWMgdm9p ZCBtdGtfanBlZ19zZXRfZW5jX2RzdChzdHJ1Y3QgbXRrX2pwZWdfY3R4ICpjdHgsIHZvaWQgX19p b21lbSAqYmFzZSwNCj4gPiA+ID4gKwkJCQkgc3RydWN0IHZiMl9idWZmZXIgKmRzdF9idWYsDQo+ ID4gPiA+ICsJCQkJIHN0cnVjdCBtdGtfanBlZ19lbmNfYnMgKmJzKQ0KPiA+ID4gPiArew0KPiA+ ID4gPiArCWJzLT5kbWFfYWRkciA9IHZiMl9kbWFfY29udGlnX3BsYW5lX2RtYV9hZGRyKGRzdF9i dWYsIDApOw0KPiA+ID4gPiArCWJzLT5kbWFfYWRkcl9vZmZzZXQgPSBjdHgtPmVuYWJsZV9leGlm ID8gTVRLX0pQRUdfREVGQVVMVF9FWElGX1NJWkUgOiAwOw0KPiA+ID4gDQo+ID4gPiBDb3VsZCB5 b3UgZXhwbGFpbiB3aGF0IGlzIHRoZSBtZWFuaW5nIG9mIHRoZSBkbWFfYWRkcl9vZmZzZXQgYW5k IHdoZXJlIHRoZQ0KPiA+ID4gZGVmYXVsdCBFWElGIHNpemUgY29tZXMgZnJvbT8gQWxzbywgaG93 IGlzIHRoZSBlbmNvZGVyIG91dHB1dCBhZmZlY3RlZCBieQ0KPiA+ID4gdGhlIGVuYWJsZV9leGlm IGZsYWc/DQo+ID4gSWYgZW5hYmxlZCB0aGUgZXhpZiBtb2RlLCB0aGUgcmVhbCBvdXRwdXQgd2ls bCBiZSBmaWxsZWQgYXQgdGhlIGxvY2FpdG9uDQo+ID4gb2YgZHN0X2FkZHIrIGRtYV9hZGRyX29m ZnNldChleGlmIHNpemUpLlRoZSBkbWFfYWRkcl9vZmZzZXQgd2lsbCBiZQ0KPiA+IGZpbGxlZCBi eSB0aGUgYXBwbGljYXRpb24uDQo+ID4gVGhlIGRlZmF1bHQgZXhpZiBzaXplIGlzIHNldHRlZCBh cyBjb25zdGFudCB2YWx1ZSA2NGsgYWNjb3JkaW5nIHRvIHRoZQ0KPiA+IHNwZWMuKEV4aWYgbWV0 YWRhdGEgYXJlIHJlc3RyaWN0ZWQgaW4gc2l6ZSB0byA2NGtCIGluIEpQRUcgaW1hZ2VzDQo+ID4g YmVjYXVzZSBhY2NvcmRpbmcgdG8gdGhlIHNwZWNpZmljYXRpb24gdGhpcyBpbmZvcm1hdGlvbiBt dXN0IGJlDQo+ID4gY29udGFpbmVkIHdpdGhpbiBhIHNpZ25lZCBKUEVHIEFQUDEgc2VnbWVudCkN Cj4gDQo+IE9rYXksIHRoYW5rcy4gVGhlbiBpdCBzb3VuZHMgbGlrZSBNVEtfSlBFR19NQVhfRVhJ Rl9TSVpFIGNvdWxkIGJlIGEgbW9yZQ0KPiBhcHByb3ByaWF0ZSBuYW1lLg0KPiANCj4gW3NuaXBd DQo+ID4gPiA+ICt9DQo+ID4gPiA+ICsNCj4gPiA+ID4gIHN0YXRpYyB2b2lkIG10a19qcGVnX2Rl dmljZV9ydW4odm9pZCAqcHJpdikNCj4gPiA+ID4gIHsNCj4gPiA+ID4gIAlzdHJ1Y3QgbXRrX2pw ZWdfY3R4ICpjdHggPSBwcml2Ow0KPiA+ID4gPiBAQCAtNzgyLDYgKzEwNjAsOCBAQCBzdGF0aWMg dm9pZCBtdGtfanBlZ19kZXZpY2VfcnVuKHZvaWQgKnByaXYpDQo+ID4gPiA+ICAJc3RydWN0IG10 a19qcGVnX3NyY19idWYgKmpwZWdfc3JjX2J1ZjsNCj4gPiA+ID4gIAlzdHJ1Y3QgbXRrX2pwZWdf YnMgYnM7DQo+ID4gPiA+ICAJc3RydWN0IG10a19qcGVnX2ZiIGZiOw0KPiA+ID4gPiArCXN0cnVj dCBtdGtfanBlZ19lbmNfYnMgZW5jX2JzOw0KPiA+ID4gPiArCXN0cnVjdCBtdGtfanBlZ19lbmNf ZmIgZW5jX2ZiOw0KPiA+ID4gPiAgCWludCBpOw0KPiA+ID4gPiAgDQo+ID4gPiA+ICAJc3JjX2J1 ZiA9IHY0bDJfbTJtX25leHRfc3JjX2J1ZihjdHgtPmZoLm0ybV9jdHgpOw0KPiA+ID4gPiBAQCAt NzkyLDMwICsxMDcyLDQ3IEBAIHN0YXRpYyB2b2lkIG10a19qcGVnX2RldmljZV9ydW4odm9pZCAq cHJpdikNCj4gPiA+ID4gIAkJZm9yIChpID0gMDsgaSA8IGRzdF9idWYtPnZiMl9idWYubnVtX3Bs YW5lczsgaSsrKQ0KPiA+ID4gPiAgCQkJdmIyX3NldF9wbGFuZV9wYXlsb2FkKCZkc3RfYnVmLT52 YjJfYnVmLCBpLCAwKTsNCj4gPiA+ID4gIAkJYnVmX3N0YXRlID0gVkIyX0JVRl9TVEFURV9ET05F Ow0KPiA+ID4gDQo+ID4gPiBBYm91dCBleGlzdGluZyBjb2RlLCBidXQgd2UgbWF5IHdhbnQgdG8g ZXhwbGFpbiB0aGlzLg0KPiA+ID4gV2hhdCBpcyB0aGlzIGxhc3QgZnJhbWUgaGFuZGxpbmcgYWJv dmUgZm9yPw0KPiA+IGlmIHRoZSB1c2VyIGdpdmVzIHVzIGEgZW1wdHkgYnVmZmVyKG1lYW5zIGl0 IGlzIHRoZSBsYXN0IGZyYW1lKSx0aGUNCj4gPiBkcml2ZXIgd2lsbCBub3QgZW5jb2RlIGFuZCBk b25lIHRoZSBidWZmZXIgdG8gdGhlIHVzZXIuDQo+ID4NCj4gDQo+IEFuIGVtcHR5IGJ1ZmZlciBp cyBub3QgYSB2YWxpZCB3YXkgb2Ygc2lnbmFsaW5nIGEgbGFzdCBmcmFtZSBpbiBWNEwyLiBJbg0K PiBnZW5lcmFsLCBJJ20gbm90IHN1cmUgdGhlcmUgaXMgc3VjaCBhIHRoaW5nIGluIEpQRUcsIGJl Y2F1c2UgYWxsIGZyYW1lcw0KPiBhcmUgc2VwYXJhdGUgZnJvbSBlYWNoIG90aGVyIGFuZCB3ZSBh bHdheXMgZXhwZWN0IDEgaW5wdXQgYnVmZmVyIGFuZCAxDQo+IG91dHB1dCBidWZmZXIgZm9yIG9u ZSBmcmFtZS4gV2UgbWlnaHQgd2FudCB0byByZW1vdmUgdGhlIHNwZWNpYWwNCj4gaGFuZGxpbmcg aW4gYSBmb2xsb3cgdXAgcGF0Y2guDQpIb3cgZG9lcyBhcHBsaWNhdGlvbiB0byBlbmQganBlZyBv cGVyYXRpb24gaW4gbW90aW9uIGpwZWcgaWYgd2UgcmVtb3ZlDQp0aGlzPyBJIHRyeWVkIHRvIGVu ZCB3aXRoIHRoZSBjb25kaXRpb24gdGhhdCB0aGUgaW5wdXQgbnVtYmVyIGVxdWFscw0Kb3V0cHV0 IG51bWJlciBpbiBVVCwgYW5kIGlzIG9rLg0KPiANCj4gPiA+ID4gLQkJZ290byBkZWNfZW5kOw0K PiA+ID4gPiArCQlnb3RvIGRldmljZV9ydW5fZW5kOw0KPiA+ID4gPiAgCX0NCj4gPiA+ID4gIA0K PiA+ID4gPiAtCWlmIChtdGtfanBlZ19jaGVja19yZXNvbHV0aW9uX2NoYW5nZShjdHgsICZqcGVn X3NyY19idWYtPmRlY19wYXJhbSkpIHsNCj4gPiA+ID4gLQkJbXRrX2pwZWdfcXVldWVfc3JjX2No Z19ldmVudChjdHgpOw0KPiA+ID4gPiAtCQljdHgtPnN0YXRlID0gTVRLX0pQRUdfU09VUkNFX0NI QU5HRTsNCj4gPiA+ID4gLQkJdjRsMl9tMm1fam9iX2ZpbmlzaChqcGVnLT5tMm1fZGV2LCBjdHgt PmZoLm0ybV9jdHgpOw0KPiA+ID4gPiAtCQlyZXR1cm47DQo+ID4gPiA+IC0JfQ0KPiA+ID4gPiAr CWlmIChqcGVnLT5tb2RlID09IE1US19KUEVHX0VOQykgew0KPiA+ID4gPiArCQlzcGluX2xvY2tf aXJxc2F2ZSgmanBlZy0+aHdfbG9jaywgZmxhZ3MpOw0KPiA+ID4gPiArCQltdGtfanBlZ19lbmNf cmVzZXQoanBlZy0+cmVnX2Jhc2UpOw0KPiA+ID4gDQo+ID4gPiBXaHkgZG8gd2UgbmVlZCB0byBy ZXNldCBldmVyeSBmcmFtZT8NCj4gPiBXZSBkbyB0aGlzIG9wZXJhdGlvbiBpcyB0byBlbnN1cmUg dGhhdCBhbGwgcmVnaXN0ZXJzIGFyZSBjbGVhcmVkLg0KPiA+IEl0J3Mgc2FmZXIgZnJvbSB0aGUg aGFyZHdhcmUgcG9pbnQgb2Ygdmlldy4NCj4gDQo+IFdvdWxkbid0IHRoaXMgb25seSB3YXN0ZSBw b3dlcj8gSWYgd2UgcmVzZXQgdGhlIGhhcmR3YXJlIGFmdGVyIHBvd2VyaW5nDQo+IHVwLCB0aGUg b25seSByZWdpc3RlcnMgdGhhdCBjb3VsZCBjaGFuZ2Ugd291bGQgYmUgY2hhbmdlZCBieSB0aGUg ZHJpdmVyDQo+IGl0c2VsZi4gVGhlIGRyaXZlciBzaG91bGQgcHJvZ3JhbSBhbGwgcmVnaXN0ZXJz IHByb3Blcmx5IHdoZW4gc3RhcnRpbmcNCj4gbmV4dCBmcmFtZSBhbnl3YXksIHNvIHN1Y2ggYSBy ZXNldCBzaG91bGRuJ3QgYmUgbmVjZXNzYXJ5Lg0KSSBjb25maXJtZWQgd2l0aCBoYXJkd2FyZSBk ZXNpZ25lciBhZ2FpbiB0aGF0IHdlIG5lZWQgdG8gcmVzZXQgZXZlcnkNCmZyYW1lLiBJZiB3ZSBk byBub3QgZG8gbGlrZSB0aGlzLCB1bmV4cGVjdGVkIG1pc3Rha2VzIG1heSBvY2N1ci4NCj4gDQo+ ID4gPiANCj4gPiA+ID4gKw0KPiA+ID4gPiArCQltdGtfanBlZ19zZXRfZW5jX2RzdChjdHgsIGpw ZWctPnJlZ19iYXNlLCAmZHN0X2J1Zi0+dmIyX2J1ZiwNCj4gPiA+ID4gKwkJCQkgICAgICZlbmNf YnMpOw0KPiA+ID4gPiArCQltdGtfanBlZ19zZXRfZW5jX3NyYyhjdHgsIGpwZWctPnJlZ19iYXNl LCAmc3JjX2J1Zi0+dmIyX2J1ZiwNCj4gPiA+ID4gKwkJCQkgICAgICZlbmNfZmIpOw0KPiA+ID4g PiArCQltdGtfanBlZ19lbmNfc2V0X2N0cmxfY2ZnKGpwZWctPnJlZ19iYXNlLCBjdHgtPmVuYWJs ZV9leGlmLA0KPiA+ID4gPiArCQkJCQkgIGN0eC0+ZW5jX3F1YWxpdHksDQo+ID4gPiA+ICsJCQkJ CSAgY3R4LT5yZXN0YXJ0X2ludGVydmFsKTsNCj4gPiA+ID4gKw0KPiA+ID4gPiArCQltdGtfanBl Z19lbmNfc3RhcnQoanBlZy0+cmVnX2Jhc2UpOw0KPiA+ID4gPiArCX0gZWxzZSB7DQo+ID4gPiA+ ICsJCWlmIChtdGtfanBlZ19jaGVja19yZXNvbHV0aW9uX2NoYW5nZQ0KPiA+ID4gPiArCQkJKGN0 eCwgJmpwZWdfc3JjX2J1Zi0+ZGVjX3BhcmFtKSkgew0KPiA+ID4gPiArCQkJbXRrX2pwZWdfcXVl dWVfc3JjX2NoZ19ldmVudChjdHgpOw0KPiA+ID4gPiArCQkJY3R4LT5zdGF0ZSA9IE1US19KUEVH X1NPVVJDRV9DSEFOR0U7DQo+ID4gPiA+ICsJCQl2NGwyX20ybV9qb2JfZmluaXNoKGpwZWctPm0y bV9kZXYsIGN0eC0+ZmgubTJtX2N0eCk7DQo+ID4gPiANCj4gPiA+IFRoaXMgaXMgYSBiaXQgc3Ry YW5nZS4gUmVzb2x1dGlvbiBjaGFuZ2Ugc2hvdWxkIGJlIHNpZ25hbGVkIHdoZW4gdGhlDQo+ID4g PiBoYXJkd2FyZSBhdHRlbXB0ZWQgdG8gZGVjb2RlIGEgZnJhbWUgYW5kIGRldGVjdGVkIGEgZGlm ZmVyZW50IHJlc29sdXRpb24NCj4gPiA+IHRoYW4gY3VycmVudC4gSXQgc2hvdWxkbid0IGJlIG5l Y2Vzc2FyeSBmb3IgdGhlIHVzZXJzcGFjZSB0byBxdWV1ZSBhIHBhaXINCj4gPiA+IG9mIGJ1ZmZl cnMgdG8gc2lnbmFsIGl0LCBhcyB3aXRoIHRoZSBjdXJyZW50IGNvZGUuDQo+ID4gSWYgdGhlIHRo ZSByZXNvbHV0aW9uIGlzIGJpZ2dlciB0aGFuIGN1cnJlbnQsIHRoZSBjdXJyZW50IGJ1ZmZlciB3 aWxsDQo+ID4gbm90IGJlIGVub3VnaCBmb3IgdGhlIGNoYW5nZWQgcmVzb2x1dGlvbi5TaG91bGRu J3QgaXQgdGVsbCB0aGUgdXNlcnNwYWNlDQo+ID4gdG8gcXVldWUgbmV3IGJ1ZmZlciBhbmQgc3Ry ZWFtIG9uIGFnYWluPw0KPiANCj4gVGhlIFY0TDIgZGVjb2RlIGZsb3cgaXMgYXMgZm9sbG93czoN Cj4gIC0gYXBwbGljYXRpb24gY29uZmlndXJlcyBhbmQgc3RhcnRzIG9ubHkgdGhlIE9VVFBVVCBx dWV1ZSwNCj4gIC0gYXBwbGljYXRpb24gcXVldWVzIGFuIE9VVFBVVCBidWZmZXIgd2l0aCBhIGZy YW1lIHdvcnRoIG9mIGJpdHN0cmVhbSwNCj4gIC0gZGVjb2RlciBwYXJzZXMgdGhlIGJpdHN0cmVh bSBoZWFkZXJzLCBkZXRlY3RzIENBUFRVUkUgZm9ybWF0IGFuZA0KPiAgICBzaWduYWxzIHRoZSBz b3VyY2UgY2hhbmdlIGV2ZW50LA0KPiAgLSBhcHBsaWNhdGlvbiByZWFkcyBDQVBUVVJFIGZvcm1h dCBhbmQgY29uZmlndXJlcyBhbmQgc3RhcnRzIHRoZQ0KPiAgICBDQVBUVVJFIHF1ZXVlLA0KPiAg LSBhcHBsaWNhdGlvbiBxdWV1ZXMgYSBDQVBUVVJFIGJ1ZmZlciwNCj4gIC0gZGVjb2RlciBkZWNv ZGVzIHRoZSBpbWFnZSB0byB0aGUgcXVldWVkIGJ1ZmZlci4NCj4gDQo+IEluIGNhc2Ugb2Ygc3Vi c2VxdWVudCAoZHluYW1pYykgcmVzb2x1dGlvbiBjaGFuZ2U6DQo+ICAtIGFwcGxpY2F0aW9uIHF1 ZXVlcyBhbiBPVVRQVVQgYnVmZmVyIGFuZCBhIENBUFRVUkUgYnVmZmVyLA0KPiAgLSBkZWNvZGVy IHBhcnNlcyB0aGUgYml0c3RyZWFtLCBub3RpY2VzIHJlc29sdXRpb24gY2hhbmdlLCB1cGRhdGVz DQo+ICAgIENBUFRVUkUgZm9ybWF0IGFuZCBzaWduYWxzIHRoZSBzb3VyY2UgY2hhbmdlIGV2ZW50 LCByZWZ1c2luZyB0bw0KPiAgICBjb250aW51ZSB0aGUgZGVjb2RpbmcgdW50aWwgdGhlIGFwcGxp Y2F0aW9uIGFja25vd2xlZGdlcyBpdCwNCj4gIC0gYXBwbGljYXRpb24gZWl0aGVyIHJlYWxsb2Nh dGVzIGl0cyBDQVBUVVJFIGJ1ZmZlcnMgb3IgY29uZmlybXMgdGhhdA0KPiAgICB0aGUgZXhpc3Rp bmcgYnVmZmVycyBhcmUgZmluZSBhbmQgYWNrbm93bGVkZ2VzIHJlc29sdXRpb24gY2hhbmdlLA0K PiAgLSBkZWNvZGluZyBjb250aW51ZXMuDQo+IA0KPiBGb3IgbW9yZSBkZXRhaWxzLCBwbGVhc2Ug Y2hlY2sgdGhlIGludGVyZmFjZSBzcGVjaWZpY2F0aW9uOg0KPiBodHRwczovL3d3dy5rZXJuZWwu b3JnL2RvYy9odG1sL2xhdGVzdC9tZWRpYS91YXBpL3Y0bC9kZXYtZGVjb2Rlci5odG1sDQo+IA0K SSB0cnllZCB0byBtb3ZlIHRoaXMgb3BlcmF0aW9uIGZyb20gZGV2aWNlX3J1bigpIHRvDQptdGtf anBlZ19kZWNfYnVmX3F1ZXVlKCksYnV0IGhhdmUgYSBwcm9ibGVtIGluIG1vdGlvbiBqcGVnLkZv ciBleGFtcGxlLEkNCnF1ZXVlZCB0aHJlZSBidWZmZXJzIGNvbnRpbnVvdXNseSx0aGUgdGhpcmQg YnVmZmVyIGhhcyByZXNvbHV0aW9uDQpjaGFuZ2UoYmlnZ2VyIHRoYW4gdGhlIHNlY29uZCBidWZm ZXIpLGJ1dCB0aGUgY2FwdHVyZSBidWZmZXIgdXNlZCBpbg0KZGV2aWNlIHJ1biBkaWRuJ3QgY2hh bmdlZC4NCkhvdyBkbyB3ZSBoYW5kbGUgdGhpcyBjYXNlPw0KPiBbc25pcF0NCj4gPiA+ID4gLQly ZXQgPSB2aWRlb19yZWdpc3Rlcl9kZXZpY2UoanBlZy0+ZGVjX3ZkZXYsIFZGTF9UWVBFX0dSQUJC RVIsIDMpOw0KPiA+ID4gPiArCXJldCA9IHZpZGVvX3JlZ2lzdGVyX2RldmljZShqcGVnLT52ZmRf anBlZywgVkZMX1RZUEVfR1JBQkJFUiwgLTEpOw0KPiA+ID4gDQo+ID4gPiBGWUkgdGhlIHR5cGUg Y2hhbmdlZCB0byBWRkxfVFlQRV9WSURFTyByZWNlbnRseS4NCj4gPiBJIGNoYW5nZWQgVkZMX1RZ UEVfR1JBQkJFUiB0byBWRkxfVFlQRV9WSURFTyxidXQgYnVpbGRlZCBmYWlsLg0KPiANCj4gV2hh dCBrZXJuZWwgdmVyc2lvbiBhcmUgeW91IGJ1aWxkaW5nIHdpdGg/DQpJIGJ1aWxkIGl0IHdpdGgg dGhlIGxhdGVzdCBrZXJuZWwgNS43LGJ1dCBidWlsZWQgZmFpbCBhZ2Fpbi4NCj4gDQo+ID4gPiA+ ICAJaWYgKHJldCkgew0KPiA+ID4gPiAgCQl2NGwyX2VycigmanBlZy0+djRsMl9kZXYsICJGYWls ZWQgdG8gcmVnaXN0ZXIgdmlkZW8gZGV2aWNlXG4iKTsNCj4gPiA+ID4gLQkJZ290byBlcnJfZGVj X3ZkZXZfcmVnaXN0ZXI7DQo+ID4gPiA+ICsJCWdvdG8gZXJyX3ZmZF9qcGVnX3JlZ2lzdGVyOw0K PiA+ID4gPiAgCX0NCj4gPiA+ID4gIA0KPiA+ID4gPiAtCXZpZGVvX3NldF9kcnZkYXRhKGpwZWct PmRlY192ZGV2LCBqcGVnKTsNCj4gPiA+ID4gKwl2aWRlb19zZXRfZHJ2ZGF0YShqcGVnLT52ZmRf anBlZywganBlZyk7DQo+ID4gPiA+ICAJdjRsMl9pbmZvKCZqcGVnLT52NGwyX2RldiwNCj4gPiA+ ID4gLQkJICAiZGVjb2RlciBkZXZpY2UgcmVnaXN0ZXJlZCBhcyAvZGV2L3ZpZGVvJWQgKCVkLCVk KVxuIiwNCj4gPiA+ID4gLQkJICBqcGVnLT5kZWNfdmRldi0+bnVtLCBWSURFT19NQUpPUiwganBl Zy0+ZGVjX3ZkZXYtPm1pbm9yKTsNCj4gPiA+ID4gKwkJICAianBlZyBkZXZpY2UgJWQgcmVnaXN0 ZXJlZCBhcyAvZGV2L3ZpZGVvJWQgKCVkLCVkKVxuIiwNCj4gPiA+IA0KPiA+ID4gSGVyZSBpdCB3 b3VsZCBiZSBhY3R1YWxseSB1c2VmdWwgdG8gc3BlY2lhbCBjYXNlIHRoZSBlbmNvZGVyIGFuZCBk ZWNvZGVyLA0KPiA+ID4gYmVjYXVzZSBpdCB3b3VsZCBiZSBlYXNpZXIgZm9yIHRoZSB1c2VyIHRv IGtub3cgd2hpY2ggZGV2aWNlIGlzIHdoaWNoLg0KPiA+ID4gDQo+IA0KPiBKdXN0IG1ha2luZyBz dXJlIHRoaXMgd2Fzbid0IG92ZXJsb29rZWQuDQo+IA0KPiBbc25pcF0NCj4gPiA+ID4gKw0KPiA+ ID4gPiArdm9pZCBtdGtfanBlZ19lbmNfcmVzZXQodm9pZCBfX2lvbWVtICpiYXNlKQ0KPiA+ID4g PiArew0KPiA+ID4gPiArCXdyaXRlbCgweDAwLCBiYXNlICsgSlBFR19FTkNfUlNUQik7DQo+ID4g PiA+ICsJd3JpdGVsKEpQRUdfRU5DX1JFU0VUX0JJVCwgYmFzZSArIEpQRUdfRU5DX1JTVEIpOw0K PiA+ID4gPiArCXdyaXRlbCgweDAwLCBiYXNlICsgSlBFR19FTkNfQ09ERUNfU0VMKTsNCj4gPiA+ ID4gK30NCj4gPiA+ID4gKw0KPiA+ID4gPiArdTMyIG10a19qcGVnX2VuY19nZXRfaW50X3N0YXR1 cyh2b2lkIF9faW9tZW0gKmJhc2UpDQo+ID4gPiA+ICt7DQo+ID4gPiA+ICsJdTMyIHJldDsNCj4g PiA+ID4gKw0KPiA+ID4gPiArCXJldCA9IHJlYWRsKGJhc2UgKyBKUEVHX0VOQ19JTlRfU1RTKSAm DQo+ID4gPiA+ICsJCSAgICBKUEVHX0VOQ19JTlRfU1RBVFVTX01BU0tfQUxMSVJROw0KPiA+ID4g PiArCWlmIChyZXQpDQo+ID4gPiA+ICsJCXdyaXRlbCgwLCBiYXNlICsgSlBFR19FTkNfSU5UX1NU Uyk7DQo+ID4gPiA+ICsNCj4gPiA+ID4gKwlyZXR1cm4gcmV0Ow0KPiA+ID4gPiArfQ0KPiA+ID4g DQo+ID4gPiBEb2VzIGl0IG1ha2Ugc2Vuc2UgdG8gaGF2ZSBhIGZ1bmN0aW9uIGZvciB3aGF0IGlz IGVzc2VudGlhbGx5IGp1c3QgMiBsaW5lcz8NCj4gPiA+IEFsc28sIHRoZSBuYW1lIGlzIG1pc2xl YWRpbmcsIGFzIHRoZSBmdW5jdGlvbiBub3Qgb25seSBnZXRzIGJ1dCBhbHNvDQo+ID4gPiBjbGVh cnMuDQo+ID4gTWFrZSBhbGwgaHcgcmVnaXN0ZXIgc2V0dGluZyBpbiBtdGtfanBlZ19lbmNfaHcu YyBpcyBvbmUgcGFydCBvZiBjdXJyZW50DQo+ID4gYXJjaGl0ZWN0dXJlLg0KPiA+IEkgaGF2ZSBj aGFuZ2VkIHRoZSBmdW5jdGlvbiBuYW1lIHRvDQo+ID4gbXRrX2pwZWdfZW5jX2dldF9hbmRfY2xl YXJfaW50X3N0YXR1cy4NCj4gDQo+IEFzIEkgbWVudGlvbmVkIGJlZm9yZSwgdGhpcyBkcml2ZXIg bmVlZHMgYSBiaWcgY2xlYW4gdXAgYW5kIHRoYXQncyB3aHkgSQ0KPiBzdWdnZXN0ZWQgc3RhcnRp bmcgb3ZlciB3aXRoIGEgbmV3IG9uZSBmb3IgdGhlIEpQRUcgZW5jb2RlciBwYXJ0LiBTaW5jZQ0K PiB3ZSBkZWNpZGVkIHRvIGV4dGVuZCB0aGlzIG9uZSBpbiB0aGUgZW5kLCB3b3VsZCB5b3UgYmUg YWJsZSB0byBpbXByb3ZlDQo+IHRoaXMgYXNwZWN0IGFzIHdlbGw/IFRoYW5rcy4NCj4gDQo+IEJl c3QgcmVnYXJkcywNCj4gVG9tYXN6DQoNCg== 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=-2.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,UNPARSEABLE_RELAY,USER_AGENT_SANE_2 autolearn=no 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 50888C54E49 for ; Sat, 9 May 2020 10:29:45 +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 1478A214D8 for ; Sat, 9 May 2020 10:29:45 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="qumhoUAR"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=mediatek.com header.i=@mediatek.com header.b="EH6bU/iv" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1478A214D8 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.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date: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=l+/CnSTVOI1tXSCNo9cf8xegyU48dBrPtYdyIPifqb4=; b=qumhoUAR4lYkJY 3Ykwe3usZZvs4bHgAzIY5tDCQF5AjVoWXHK4cFoxa4sftFdKnacORXCviGg4Pp2904ZuoZYeKWYTl QClEKu48FAydkxahtnjVf4IMT3WEJRokq3VqF5M+2F2sRBsh/xtWNy6CE1Vq00X+TTDhkWWxAdqiV xnG0lYURDw6oLXxmYhvYLCNGpYdbbKT2pgXqrpjDx92r1QpKaX1SW0a5oIfkcpkPlf4UPFzgBu7ni ztSrkcZw1+XCMKiSuGFcT9fe1t29hjsyyl9CekPMuON7V0bFj53kP/P+MkQaHgR2bJKTW2va9S4LL FzQ1RHnaz+fM7LmUTcRw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jXMjg-0003Cx-Jm; Sat, 09 May 2020 10:29:36 +0000 Received: from mailgw01.mediatek.com ([216.200.240.184]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jXMjd-0003C7-JU; Sat, 09 May 2020 10:29:35 +0000 X-UUID: 6b7c0b6dd98e4998a2cf0f8dca7e42ab-20200509 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=epJDxfn56pm1KYyJJsHHZHZ67ESMx3iBBECMEcHoO2Q=; b=EH6bU/ivwKNlZGtx3QmkhDPM1Ncw2sUxTOIC6t26jrHHE46HsNwumNC5aVmLPeLp8WX0TNQiGMOJTgU7aIGy4upzWOLIbefjAEw2rmbtDcIAkUeVllVLTV+yqGoLy+QaKwDN1EA8tspEhwgakDJbGlIoohnSn9DV2BfnEaLv630=; X-UUID: 6b7c0b6dd98e4998a2cf0f8dca7e42ab-20200509 Received: from mtkcas68.mediatek.inc [(172.29.94.19)] by mailgw01.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLS) with ESMTP id 2064341429; Sat, 09 May 2020 02:29:21 -0800 Received: from mtkmbs07n1.mediatek.inc (172.21.101.16) by MTKMBS62DR.mediatek.inc (172.29.94.18) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Sat, 9 May 2020 03:29:25 -0700 Received: from MTKCAS36.mediatek.inc (172.27.4.186) by mtkmbs07n1.mediatek.inc (172.21.101.16) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Sat, 9 May 2020 18:29:25 +0800 Received: from [10.17.3.153] (10.17.3.153) by MTKCAS36.mediatek.inc (172.27.4.170) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Sat, 9 May 2020 18:29:24 +0800 Message-ID: <1589020095.24163.150.camel@mhfsdcap03> Subject: Re: [PATCH v7 11/11] media: platform: Add jpeg dec/enc feature From: Xia Jiang To: Tomasz Figa Date: Sat, 9 May 2020 18:28:15 +0800 In-Reply-To: <20200501173712.GB218308@chromium.org> References: <20200303123446.20095-1-xia.jiang@mediatek.com> <20200303123446.20095-12-xia.jiang@mediatek.com> <20200306112337.GA163286@chromium.org> <1587009795.24163.87.camel@mhfsdcap03> <20200501173712.GB218308@chromium.org> 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-20200509_032933_665058_6760A873 X-CRM114-Status: GOOD ( 35.64 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, srv_heupstream@mediatek.com, Rick Chang , linux-kernel@vger.kernel.org, Mauro Carvalho Chehab , Rob Herring , Matthias Brugger , Hans Verkuil , linux-mediatek@lists.infradead.org, Marek Szyprowski , linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org 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 On Fri, 2020-05-01 at 17:37 +0000, Tomasz Figa wrote: > Hi Xia, > > On Thu, Apr 16, 2020 at 12:03:15PM +0800, Xia Jiang wrote: > > On Fri, 2020-03-06 at 20:23 +0900, Tomasz Figa wrote: > > > Hi Xia, > > > > > > On Tue, Mar 03, 2020 at 08:34:46PM +0800, Xia Jiang wrote: > > > > Add mtk jpeg encode v4l2 driver based on jpeg decode, because that jpeg > > > > decode and encode have great similarities with function operation. > > > > > > Thank you for the patch. Please see my comments inline. > > > > Dear Tomasz, > > > > Thank you for your reply. I have followed your advice and submited v8 > > version patch. > > > > Please check my reply below. Dear Tomasz, I have some confuse about your advice, please check my reply below. > [snip] > > > > > > > > > > > - switch (s->target) { > > > > - case V4L2_SEL_TGT_COMPOSE: > > > > - s->r.left = 0; > > > > - s->r.top = 0; > > > > - ctx->out_q.w = s->r.width; > > > > - ctx->out_q.h = s->r.height; > > > > - break; > > > > - default: > > > > - return -EINVAL; > > > > + switch (s->target) { > > > > + case V4L2_SEL_TGT_CROP: > > > > + s->r.left = 0; > > > > + s->r.top = 0; > > > > + ctx->out_q.w = s->r.width; > > > > + ctx->out_q.h = s->r.height; > > > > > > What happens if the userspace provides a value bigger than current format? > > we need get the min value of userspace value and current value,changed > > it like this: > > ctx->out_q.w = min(s->r.width, ctx->out_q.w); > > ctx->out_q.h = min(s->r.height,ctx->out_q.h); > > Since ctx->out_q is modified by this function, wouldn't that cause > problems if S_SELECTION was called two times, first with a smaller > rectangle and then with a bigger one? We should store the active crop > and format separately and use the latter for min(). Add a member variable(struct v4l2_rect) in out_q structure for storing the active crop, like this: s->r.width = min(s->r.width, ctx->out_q.w); s->r.height = min(s->r.height,ctx->out_q.h); ctx->out_q.rect.width = s->r.width; ctx->out_q.rect.height = s->r.height; Is that ok? > > [snip] > > > > > > > > while ((vb = mtk_jpeg_buf_remove(ctx, q->type))) > > > > v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR); > > > > @@ -772,6 +1011,45 @@ static int mtk_jpeg_set_dec_dst(struct mtk_jpeg_ctx *ctx, > > > > return 0; > > > > } > > > > > > > > +static void mtk_jpeg_set_enc_dst(struct mtk_jpeg_ctx *ctx, void __iomem *base, > > > > + struct vb2_buffer *dst_buf, > > > > + struct mtk_jpeg_enc_bs *bs) > > > > +{ > > > > + bs->dma_addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0); > > > > + bs->dma_addr_offset = ctx->enable_exif ? MTK_JPEG_DEFAULT_EXIF_SIZE : 0; > > > > > > Could you explain what is the meaning of the dma_addr_offset and where the > > > default EXIF size comes from? Also, how is the encoder output affected by > > > the enable_exif flag? > > If enabled the exif mode, the real output will be filled at the locaiton > > of dst_addr+ dma_addr_offset(exif size).The dma_addr_offset will be > > filled by the application. > > The default exif size is setted as constant value 64k according to the > > spec.(Exif metadata are restricted in size to 64kB in JPEG images > > because according to the specification this information must be > > contained within a signed JPEG APP1 segment) > > Okay, thanks. Then it sounds like MTK_JPEG_MAX_EXIF_SIZE could be a more > appropriate name. > > [snip] > > > > +} > > > > + > > > > static void mtk_jpeg_device_run(void *priv) > > > > { > > > > struct mtk_jpeg_ctx *ctx = priv; > > > > @@ -782,6 +1060,8 @@ static void mtk_jpeg_device_run(void *priv) > > > > struct mtk_jpeg_src_buf *jpeg_src_buf; > > > > struct mtk_jpeg_bs bs; > > > > struct mtk_jpeg_fb fb; > > > > + struct mtk_jpeg_enc_bs enc_bs; > > > > + struct mtk_jpeg_enc_fb enc_fb; > > > > int i; > > > > > > > > src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx); > > > > @@ -792,30 +1072,47 @@ static void mtk_jpeg_device_run(void *priv) > > > > for (i = 0; i < dst_buf->vb2_buf.num_planes; i++) > > > > vb2_set_plane_payload(&dst_buf->vb2_buf, i, 0); > > > > buf_state = VB2_BUF_STATE_DONE; > > > > > > About existing code, but we may want to explain this. > > > What is this last frame handling above for? > > if the user gives us a empty buffer(means it is the last frame),the > > driver will not encode and done the buffer to the user. > > > > An empty buffer is not a valid way of signaling a last frame in V4L2. In > general, I'm not sure there is such a thing in JPEG, because all frames > are separate from each other and we always expect 1 input buffer and 1 > output buffer for one frame. We might want to remove the special > handling in a follow up patch. How does application to end jpeg operation in motion jpeg if we remove this? I tryed to end with the condition that the input number equals output number in UT, and is ok. > > > > > - goto dec_end; > > > > + goto device_run_end; > > > > } > > > > > > > > - if (mtk_jpeg_check_resolution_change(ctx, &jpeg_src_buf->dec_param)) { > > > > - mtk_jpeg_queue_src_chg_event(ctx); > > > > - ctx->state = MTK_JPEG_SOURCE_CHANGE; > > > > - v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx); > > > > - return; > > > > - } > > > > + if (jpeg->mode == MTK_JPEG_ENC) { > > > > + spin_lock_irqsave(&jpeg->hw_lock, flags); > > > > + mtk_jpeg_enc_reset(jpeg->reg_base); > > > > > > Why do we need to reset every frame? > > We do this operation is to ensure that all registers are cleared. > > It's safer from the hardware point of view. > > Wouldn't this only waste power? If we reset the hardware after powering > up, the only registers that could change would be changed by the driver > itself. The driver should program all registers properly when starting > next frame anyway, so such a reset shouldn't be necessary. I confirmed with hardware designer again that we need to reset every frame. If we do not do like this, unexpected mistakes may occur. > > > > > > > > + > > > > + mtk_jpeg_set_enc_dst(ctx, jpeg->reg_base, &dst_buf->vb2_buf, > > > > + &enc_bs); > > > > + mtk_jpeg_set_enc_src(ctx, jpeg->reg_base, &src_buf->vb2_buf, > > > > + &enc_fb); > > > > + mtk_jpeg_enc_set_ctrl_cfg(jpeg->reg_base, ctx->enable_exif, > > > > + ctx->enc_quality, > > > > + ctx->restart_interval); > > > > + > > > > + mtk_jpeg_enc_start(jpeg->reg_base); > > > > + } else { > > > > + if (mtk_jpeg_check_resolution_change > > > > + (ctx, &jpeg_src_buf->dec_param)) { > > > > + mtk_jpeg_queue_src_chg_event(ctx); > > > > + ctx->state = MTK_JPEG_SOURCE_CHANGE; > > > > + v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx); > > > > > > This is a bit strange. Resolution change should be signaled when the > > > hardware attempted to decode a frame and detected a different resolution > > > than current. It shouldn't be necessary for the userspace to queue a pair > > > of buffers to signal it, as with the current code. > > If the the resolution is bigger than current, the current buffer will > > not be enough for the changed resolution.Shouldn't it tell the userspace > > to queue new buffer and stream on again? > > The V4L2 decode flow is as follows: > - application configures and starts only the OUTPUT queue, > - application queues an OUTPUT buffer with a frame worth of bitstream, > - decoder parses the bitstream headers, detects CAPTURE format and > signals the source change event, > - application reads CAPTURE format and configures and starts the > CAPTURE queue, > - application queues a CAPTURE buffer, > - decoder decodes the image to the queued buffer. > > In case of subsequent (dynamic) resolution change: > - application queues an OUTPUT buffer and a CAPTURE buffer, > - decoder parses the bitstream, notices resolution change, updates > CAPTURE format and signals the source change event, refusing to > continue the decoding until the application acknowledges it, > - application either reallocates its CAPTURE buffers or confirms that > the existing buffers are fine and acknowledges resolution change, > - decoding continues. > > For more details, please check the interface specification: > https://www.kernel.org/doc/html/latest/media/uapi/v4l/dev-decoder.html > I tryed to move this operation from device_run() to mtk_jpeg_dec_buf_queue(),but have a problem in motion jpeg.For example,I queued three buffers continuously,the third buffer has resolution change(bigger than the second buffer),but the capture buffer used in device run didn't changed. How do we handle this case? > [snip] > > > > - ret = video_register_device(jpeg->dec_vdev, VFL_TYPE_GRABBER, 3); > > > > + ret = video_register_device(jpeg->vfd_jpeg, VFL_TYPE_GRABBER, -1); > > > > > > FYI the type changed to VFL_TYPE_VIDEO recently. > > I changed VFL_TYPE_GRABBER to VFL_TYPE_VIDEO,but builded fail. > > What kernel version are you building with? I build it with the latest kernel 5.7,but builed fail again. > > > > > if (ret) { > > > > v4l2_err(&jpeg->v4l2_dev, "Failed to register video device\n"); > > > > - goto err_dec_vdev_register; > > > > + goto err_vfd_jpeg_register; > > > > } > > > > > > > > - video_set_drvdata(jpeg->dec_vdev, jpeg); > > > > + video_set_drvdata(jpeg->vfd_jpeg, jpeg); > > > > v4l2_info(&jpeg->v4l2_dev, > > > > - "decoder device registered as /dev/video%d (%d,%d)\n", > > > > - jpeg->dec_vdev->num, VIDEO_MAJOR, jpeg->dec_vdev->minor); > > > > + "jpeg device %d registered as /dev/video%d (%d,%d)\n", > > > > > > Here it would be actually useful to special case the encoder and decoder, > > > because it would be easier for the user to know which device is which. > > > > > Just making sure this wasn't overlooked. > > [snip] > > > > + > > > > +void mtk_jpeg_enc_reset(void __iomem *base) > > > > +{ > > > > + writel(0x00, base + JPEG_ENC_RSTB); > > > > + writel(JPEG_ENC_RESET_BIT, base + JPEG_ENC_RSTB); > > > > + writel(0x00, base + JPEG_ENC_CODEC_SEL); > > > > +} > > > > + > > > > +u32 mtk_jpeg_enc_get_int_status(void __iomem *base) > > > > +{ > > > > + u32 ret; > > > > + > > > > + ret = readl(base + JPEG_ENC_INT_STS) & > > > > + JPEG_ENC_INT_STATUS_MASK_ALLIRQ; > > > > + if (ret) > > > > + writel(0, base + JPEG_ENC_INT_STS); > > > > + > > > > + return ret; > > > > +} > > > > > > Does it make sense to have a function for what is essentially just 2 lines? > > > Also, the name is misleading, as the function not only gets but also > > > clears. > > Make all hw register setting in mtk_jpeg_enc_hw.c is one part of current > > architecture. > > I have changed the function name to > > mtk_jpeg_enc_get_and_clear_int_status. > > As I mentioned before, this driver needs a big clean up and that's why I > suggested starting over with a new one for the JPEG encoder part. Since > we decided to extend this one in the end, would you be able to improve > this aspect as well? Thanks. > > Best regards, > Tomasz _______________________________________________ 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=-2.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,UNPARSEABLE_RELAY,USER_AGENT_SANE_2 autolearn=no 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 1A947C28CBC for ; Sat, 9 May 2020 10:29:38 +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 D8CC024954 for ; Sat, 9 May 2020 10:29:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="F5mehd/p"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=mediatek.com header.i=@mediatek.com header.b="EH6bU/iv" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D8CC024954 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+infradead-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.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date: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=rtkRlK9RNY4vx8+26MZ0zUIvn3DusMSQ9AVGJR0L/tw=; b=F5mehd/pdIUhNI JRpY8gZz5uHnoeN6JlfrMdFw23oFWMg6396gTutrqTee6tL8ED0qK3RFsfFJNDsJjeLiqsVZUfbgK 5cIVbVBp1mfpHtroEjS91+P/tz3Wbvi9nD1W/j1xG36LAJzgzGEyGjDSKZ/gCAZqRU60m36lT1uz2 Lj0DnAocAEbel95Tlnf5n+4Q1yuyrTQ9BM68xJ3fC4cITT4rmL+QzaLr1HR6AOhmHTSla+zEFcC8g TtzGABSrhbxBplZYhi9/vbhP0AQ0lAu+4JTNPNMF5RcOY2vbBOxKOmBLxeuiJ/PfG6s+72sRZ5TiB a8R/MP2iOyzRwMv4GDtg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jXMjh-0003Dg-EY; Sat, 09 May 2020 10:29:37 +0000 Received: from mailgw01.mediatek.com ([216.200.240.184]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jXMjd-0003C7-JU; Sat, 09 May 2020 10:29:35 +0000 X-UUID: 6b7c0b6dd98e4998a2cf0f8dca7e42ab-20200509 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=epJDxfn56pm1KYyJJsHHZHZ67ESMx3iBBECMEcHoO2Q=; b=EH6bU/ivwKNlZGtx3QmkhDPM1Ncw2sUxTOIC6t26jrHHE46HsNwumNC5aVmLPeLp8WX0TNQiGMOJTgU7aIGy4upzWOLIbefjAEw2rmbtDcIAkUeVllVLTV+yqGoLy+QaKwDN1EA8tspEhwgakDJbGlIoohnSn9DV2BfnEaLv630=; X-UUID: 6b7c0b6dd98e4998a2cf0f8dca7e42ab-20200509 Received: from mtkcas68.mediatek.inc [(172.29.94.19)] by mailgw01.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLS) with ESMTP id 2064341429; Sat, 09 May 2020 02:29:21 -0800 Received: from mtkmbs07n1.mediatek.inc (172.21.101.16) by MTKMBS62DR.mediatek.inc (172.29.94.18) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Sat, 9 May 2020 03:29:25 -0700 Received: from MTKCAS36.mediatek.inc (172.27.4.186) by mtkmbs07n1.mediatek.inc (172.21.101.16) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Sat, 9 May 2020 18:29:25 +0800 Received: from [10.17.3.153] (10.17.3.153) by MTKCAS36.mediatek.inc (172.27.4.170) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Sat, 9 May 2020 18:29:24 +0800 Message-ID: <1589020095.24163.150.camel@mhfsdcap03> Subject: Re: [PATCH v7 11/11] media: platform: Add jpeg dec/enc feature From: Xia Jiang To: Tomasz Figa Date: Sat, 9 May 2020 18:28:15 +0800 In-Reply-To: <20200501173712.GB218308@chromium.org> References: <20200303123446.20095-1-xia.jiang@mediatek.com> <20200303123446.20095-12-xia.jiang@mediatek.com> <20200306112337.GA163286@chromium.org> <1587009795.24163.87.camel@mhfsdcap03> <20200501173712.GB218308@chromium.org> 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-20200509_032933_665058_6760A873 X-CRM114-Status: GOOD ( 35.64 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, srv_heupstream@mediatek.com, Rick Chang , linux-kernel@vger.kernel.org, Mauro Carvalho Chehab , Rob Herring , Matthias Brugger , Hans Verkuil , linux-mediatek@lists.infradead.org, Marek Szyprowski , linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 2020-05-01 at 17:37 +0000, Tomasz Figa wrote: > Hi Xia, > > On Thu, Apr 16, 2020 at 12:03:15PM +0800, Xia Jiang wrote: > > On Fri, 2020-03-06 at 20:23 +0900, Tomasz Figa wrote: > > > Hi Xia, > > > > > > On Tue, Mar 03, 2020 at 08:34:46PM +0800, Xia Jiang wrote: > > > > Add mtk jpeg encode v4l2 driver based on jpeg decode, because that jpeg > > > > decode and encode have great similarities with function operation. > > > > > > Thank you for the patch. Please see my comments inline. > > > > Dear Tomasz, > > > > Thank you for your reply. I have followed your advice and submited v8 > > version patch. > > > > Please check my reply below. Dear Tomasz, I have some confuse about your advice, please check my reply below. > [snip] > > > > > > > > > > > - switch (s->target) { > > > > - case V4L2_SEL_TGT_COMPOSE: > > > > - s->r.left = 0; > > > > - s->r.top = 0; > > > > - ctx->out_q.w = s->r.width; > > > > - ctx->out_q.h = s->r.height; > > > > - break; > > > > - default: > > > > - return -EINVAL; > > > > + switch (s->target) { > > > > + case V4L2_SEL_TGT_CROP: > > > > + s->r.left = 0; > > > > + s->r.top = 0; > > > > + ctx->out_q.w = s->r.width; > > > > + ctx->out_q.h = s->r.height; > > > > > > What happens if the userspace provides a value bigger than current format? > > we need get the min value of userspace value and current value,changed > > it like this: > > ctx->out_q.w = min(s->r.width, ctx->out_q.w); > > ctx->out_q.h = min(s->r.height,ctx->out_q.h); > > Since ctx->out_q is modified by this function, wouldn't that cause > problems if S_SELECTION was called two times, first with a smaller > rectangle and then with a bigger one? We should store the active crop > and format separately and use the latter for min(). Add a member variable(struct v4l2_rect) in out_q structure for storing the active crop, like this: s->r.width = min(s->r.width, ctx->out_q.w); s->r.height = min(s->r.height,ctx->out_q.h); ctx->out_q.rect.width = s->r.width; ctx->out_q.rect.height = s->r.height; Is that ok? > > [snip] > > > > > > > > while ((vb = mtk_jpeg_buf_remove(ctx, q->type))) > > > > v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR); > > > > @@ -772,6 +1011,45 @@ static int mtk_jpeg_set_dec_dst(struct mtk_jpeg_ctx *ctx, > > > > return 0; > > > > } > > > > > > > > +static void mtk_jpeg_set_enc_dst(struct mtk_jpeg_ctx *ctx, void __iomem *base, > > > > + struct vb2_buffer *dst_buf, > > > > + struct mtk_jpeg_enc_bs *bs) > > > > +{ > > > > + bs->dma_addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0); > > > > + bs->dma_addr_offset = ctx->enable_exif ? MTK_JPEG_DEFAULT_EXIF_SIZE : 0; > > > > > > Could you explain what is the meaning of the dma_addr_offset and where the > > > default EXIF size comes from? Also, how is the encoder output affected by > > > the enable_exif flag? > > If enabled the exif mode, the real output will be filled at the locaiton > > of dst_addr+ dma_addr_offset(exif size).The dma_addr_offset will be > > filled by the application. > > The default exif size is setted as constant value 64k according to the > > spec.(Exif metadata are restricted in size to 64kB in JPEG images > > because according to the specification this information must be > > contained within a signed JPEG APP1 segment) > > Okay, thanks. Then it sounds like MTK_JPEG_MAX_EXIF_SIZE could be a more > appropriate name. > > [snip] > > > > +} > > > > + > > > > static void mtk_jpeg_device_run(void *priv) > > > > { > > > > struct mtk_jpeg_ctx *ctx = priv; > > > > @@ -782,6 +1060,8 @@ static void mtk_jpeg_device_run(void *priv) > > > > struct mtk_jpeg_src_buf *jpeg_src_buf; > > > > struct mtk_jpeg_bs bs; > > > > struct mtk_jpeg_fb fb; > > > > + struct mtk_jpeg_enc_bs enc_bs; > > > > + struct mtk_jpeg_enc_fb enc_fb; > > > > int i; > > > > > > > > src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx); > > > > @@ -792,30 +1072,47 @@ static void mtk_jpeg_device_run(void *priv) > > > > for (i = 0; i < dst_buf->vb2_buf.num_planes; i++) > > > > vb2_set_plane_payload(&dst_buf->vb2_buf, i, 0); > > > > buf_state = VB2_BUF_STATE_DONE; > > > > > > About existing code, but we may want to explain this. > > > What is this last frame handling above for? > > if the user gives us a empty buffer(means it is the last frame),the > > driver will not encode and done the buffer to the user. > > > > An empty buffer is not a valid way of signaling a last frame in V4L2. In > general, I'm not sure there is such a thing in JPEG, because all frames > are separate from each other and we always expect 1 input buffer and 1 > output buffer for one frame. We might want to remove the special > handling in a follow up patch. How does application to end jpeg operation in motion jpeg if we remove this? I tryed to end with the condition that the input number equals output number in UT, and is ok. > > > > > - goto dec_end; > > > > + goto device_run_end; > > > > } > > > > > > > > - if (mtk_jpeg_check_resolution_change(ctx, &jpeg_src_buf->dec_param)) { > > > > - mtk_jpeg_queue_src_chg_event(ctx); > > > > - ctx->state = MTK_JPEG_SOURCE_CHANGE; > > > > - v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx); > > > > - return; > > > > - } > > > > + if (jpeg->mode == MTK_JPEG_ENC) { > > > > + spin_lock_irqsave(&jpeg->hw_lock, flags); > > > > + mtk_jpeg_enc_reset(jpeg->reg_base); > > > > > > Why do we need to reset every frame? > > We do this operation is to ensure that all registers are cleared. > > It's safer from the hardware point of view. > > Wouldn't this only waste power? If we reset the hardware after powering > up, the only registers that could change would be changed by the driver > itself. The driver should program all registers properly when starting > next frame anyway, so such a reset shouldn't be necessary. I confirmed with hardware designer again that we need to reset every frame. If we do not do like this, unexpected mistakes may occur. > > > > > > > > + > > > > + mtk_jpeg_set_enc_dst(ctx, jpeg->reg_base, &dst_buf->vb2_buf, > > > > + &enc_bs); > > > > + mtk_jpeg_set_enc_src(ctx, jpeg->reg_base, &src_buf->vb2_buf, > > > > + &enc_fb); > > > > + mtk_jpeg_enc_set_ctrl_cfg(jpeg->reg_base, ctx->enable_exif, > > > > + ctx->enc_quality, > > > > + ctx->restart_interval); > > > > + > > > > + mtk_jpeg_enc_start(jpeg->reg_base); > > > > + } else { > > > > + if (mtk_jpeg_check_resolution_change > > > > + (ctx, &jpeg_src_buf->dec_param)) { > > > > + mtk_jpeg_queue_src_chg_event(ctx); > > > > + ctx->state = MTK_JPEG_SOURCE_CHANGE; > > > > + v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx); > > > > > > This is a bit strange. Resolution change should be signaled when the > > > hardware attempted to decode a frame and detected a different resolution > > > than current. It shouldn't be necessary for the userspace to queue a pair > > > of buffers to signal it, as with the current code. > > If the the resolution is bigger than current, the current buffer will > > not be enough for the changed resolution.Shouldn't it tell the userspace > > to queue new buffer and stream on again? > > The V4L2 decode flow is as follows: > - application configures and starts only the OUTPUT queue, > - application queues an OUTPUT buffer with a frame worth of bitstream, > - decoder parses the bitstream headers, detects CAPTURE format and > signals the source change event, > - application reads CAPTURE format and configures and starts the > CAPTURE queue, > - application queues a CAPTURE buffer, > - decoder decodes the image to the queued buffer. > > In case of subsequent (dynamic) resolution change: > - application queues an OUTPUT buffer and a CAPTURE buffer, > - decoder parses the bitstream, notices resolution change, updates > CAPTURE format and signals the source change event, refusing to > continue the decoding until the application acknowledges it, > - application either reallocates its CAPTURE buffers or confirms that > the existing buffers are fine and acknowledges resolution change, > - decoding continues. > > For more details, please check the interface specification: > https://www.kernel.org/doc/html/latest/media/uapi/v4l/dev-decoder.html > I tryed to move this operation from device_run() to mtk_jpeg_dec_buf_queue(),but have a problem in motion jpeg.For example,I queued three buffers continuously,the third buffer has resolution change(bigger than the second buffer),but the capture buffer used in device run didn't changed. How do we handle this case? > [snip] > > > > - ret = video_register_device(jpeg->dec_vdev, VFL_TYPE_GRABBER, 3); > > > > + ret = video_register_device(jpeg->vfd_jpeg, VFL_TYPE_GRABBER, -1); > > > > > > FYI the type changed to VFL_TYPE_VIDEO recently. > > I changed VFL_TYPE_GRABBER to VFL_TYPE_VIDEO,but builded fail. > > What kernel version are you building with? I build it with the latest kernel 5.7,but builed fail again. > > > > > if (ret) { > > > > v4l2_err(&jpeg->v4l2_dev, "Failed to register video device\n"); > > > > - goto err_dec_vdev_register; > > > > + goto err_vfd_jpeg_register; > > > > } > > > > > > > > - video_set_drvdata(jpeg->dec_vdev, jpeg); > > > > + video_set_drvdata(jpeg->vfd_jpeg, jpeg); > > > > v4l2_info(&jpeg->v4l2_dev, > > > > - "decoder device registered as /dev/video%d (%d,%d)\n", > > > > - jpeg->dec_vdev->num, VIDEO_MAJOR, jpeg->dec_vdev->minor); > > > > + "jpeg device %d registered as /dev/video%d (%d,%d)\n", > > > > > > Here it would be actually useful to special case the encoder and decoder, > > > because it would be easier for the user to know which device is which. > > > > > Just making sure this wasn't overlooked. > > [snip] > > > > + > > > > +void mtk_jpeg_enc_reset(void __iomem *base) > > > > +{ > > > > + writel(0x00, base + JPEG_ENC_RSTB); > > > > + writel(JPEG_ENC_RESET_BIT, base + JPEG_ENC_RSTB); > > > > + writel(0x00, base + JPEG_ENC_CODEC_SEL); > > > > +} > > > > + > > > > +u32 mtk_jpeg_enc_get_int_status(void __iomem *base) > > > > +{ > > > > + u32 ret; > > > > + > > > > + ret = readl(base + JPEG_ENC_INT_STS) & > > > > + JPEG_ENC_INT_STATUS_MASK_ALLIRQ; > > > > + if (ret) > > > > + writel(0, base + JPEG_ENC_INT_STS); > > > > + > > > > + return ret; > > > > +} > > > > > > Does it make sense to have a function for what is essentially just 2 lines? > > > Also, the name is misleading, as the function not only gets but also > > > clears. > > Make all hw register setting in mtk_jpeg_enc_hw.c is one part of current > > architecture. > > I have changed the function name to > > mtk_jpeg_enc_get_and_clear_int_status. > > As I mentioned before, this driver needs a big clean up and that's why I > suggested starting over with a new one for the JPEG encoder part. Since > we decided to extend this one in the end, would you be able to improve > this aspect as well? Thanks. > > Best regards, > Tomasz _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel