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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id E730CC4332F for ; Thu, 20 Jan 2022 08:45:17 +0000 (UTC) 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:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Content-ID:In-Reply-To: References:Message-ID:Date:Subject:To:From:Reply-To:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=vXlDbJ0nT+BEMsDODN46Vl666CisEnASmvLS2ru7+SQ=; b=32wdnZrGid6SpG 0nfl57+uTqRy9JL2HMsrwH6wXiUbdIS1MW9kiio19zomuEciDMe5mmy5iNzpknJ7tmzGmyTeDDpiN AtF6pGGQQ584qy1XcgrbBFWDo2ZUJyAUNp8kCs4b1rqBqKqr8PSHkfiVqHTlgqftt48QinDgA4t4q H9nmn6Pg47lnpLZWsO+abklk4Um9rJGWCiAldCYBF60iQsQMXp9yMgy5znc2zJyYSqXxakvLbyeBw Rf23QslfFlqmEjx1gkcdH71RpzPul60oVbxGQwABBbxPGw/lCGpfQyj7mQCCju16QG9CmiGnjZXon Hy3rnIuRjhY/fuJuVd0Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nAT2o-009sEX-8q; Thu, 20 Jan 2022 08:43:46 +0000 Received: from esa.microchip.iphmx.com ([68.232.153.233]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nAT2i-009sCN-Mm for linux-arm-kernel@lists.infradead.org; Thu, 20 Jan 2022 08:43:43 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1642668222; x=1674204222; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=ZWkaLAnnOmuAzALh8hl3hyko4H3RaZXyxqfI8iYSOMI=; b=RiMWvwRkKkTAYbQpc6xZGO7uUSeG2tGqT6cT5q3ozBbJAFC86iC6ufhP iw10QLg/f1jPKUL2VSuXfVLDRGda6qSF/ETiMssD8m/Kdu6rfU5jeYSTS 0UBhB9wujPo8BUc3BjfgQQTOn+Cbksyr1h5Y6P3HQ65pCBVwab+3R10QZ TlqoMKkQRJXP7aANynF9OcR7CZuHcRcYXBgcKC6i1gA1BgVpB+v2L+gzf 3SwCuVeyFykZ27OtB07VJAJ57O1RZLZpdtkiC327rOhsJuHMbbo+qV8zo ofF07V3sbDJU4yiqHpEbaQrfdtdaZYD4HT13/uAuTgUxUq5YHVQTcvMk/ Q==; IronPort-SDR: FImNPJeCZoLYsyykCXOqaWKrT0YkzHcHm8xR3ipQkeFjd1gtuZxJvwLQpKvUccdIW6qAMUigkf uVdq+jr82ZJeZUmYhYWxtNPnZb5lpxj7FaHQdIZUx5k7uAIpHIzGP/HldI9bXEdBaI1OF8/hno d9kZkYDZ//iWwhX0mk92x0xYazAAmdudUIsPO8rYyuZUV0NFrMSI2zOvWYDXkWM9KXSJI5Myq0 QPXG6YGb9vnrCrF6uI3lza6cpN3ypL/99z7WUX+6mV8rX5GPImNDEv7zLNP/md1cqOJRQSUBEF zeHyxFImmOv551/aNS8gU7m9 X-IronPort-AV: E=Sophos;i="5.88,301,1635231600"; d="scan'208";a="150280163" Received: from smtpout.microchip.com (HELO email.microchip.com) ([198.175.253.82]) by esa5.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 20 Jan 2022 01:43:40 -0700 Received: from chn-vm-ex01.mchp-main.com (10.10.85.143) by chn-vm-ex01.mchp-main.com (10.10.85.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.17; Thu, 20 Jan 2022 01:43:39 -0700 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (10.10.215.89) by email.microchip.com (10.10.87.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.17 via Frontend Transport; Thu, 20 Jan 2022 01:43:39 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UNJsW2rS0yuuYfkPygyIAiIEjkanu+QqzUwiDZHKO8J/l+hWuum17ywESdb7b9jZ7rV1Ouu3kBDQh/5b3Fnx6QyhP8LpWgB/+a4rsbjCjQkE4DxUyE/u8weqnQNMYGCdVcl7WzfleAzvQoclRJo82Tw+CoYQ+gC3CR+dUtqOeCsXI/tKF0F1LewExROiDg6rUqyu9BhJ4o2orcDb2oJzFnrx3DDm82ZwMwwbv2EQrdvwzlPn9AGThs4hmtFghonAhTWa97kMgzGTWHRGVdotkgdTvYnMmZWbfj3gaaUcec9tyhnZbNPFQ2TQfmy8AUzikEOHvY6HmrjpFVIlTh9Q3g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ZWkaLAnnOmuAzALh8hl3hyko4H3RaZXyxqfI8iYSOMI=; b=H/XlBRCqFm8N+EaKdeBLxNN+HG4PfpZDm3Ogczc1IPDZbGHKlmfvsXit1hZpuLdyDOBLecW/6V1KmXvD9VqcroQCacAy0eJdJLBSNQ21opL69E8tCODxb1a3Pq9VuRDs60ie+41w+J/yEdpVsHiA9gavn65kEXBnRMcD8R/p3/a5j1J5RyAN6jGc43bDnf3nDJ2Fs2x18iBVVsGOgEKMjqQmxarV8BKEwgto8/3PFnuqDh1MgMxhAf/tjEQGslCdK63l8LNownlyU8/lfO/lLfK5SF5k6I2dL9crlsgiOLIMo9mNccESn1LEnjmFqI5pNR+8XWdw7epWXQdGYE6J0A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microchiptechnology.onmicrosoft.com; s=selector2-microchiptechnology-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ZWkaLAnnOmuAzALh8hl3hyko4H3RaZXyxqfI8iYSOMI=; b=RR3+E2Z6MxJmCckiRszYOkaorAD/r4sI+njjARSZXgF61GSdF9iYaO2mfQ7Mky6A7Tt0U0hazcGx/ON/GjLralSWGP0hj8Hi6jMBubvnIy6nGnxd3Fmw4pCzU4iOsfA6rRTRcMNwmEW2yNtyUkXwIEpPbal2669BS/yMJ8Z5OV8= Received: from PH0PR11MB4920.namprd11.prod.outlook.com (2603:10b6:510:41::22) by SA2PR11MB4923.namprd11.prod.outlook.com (2603:10b6:806:fa::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4909.10; Thu, 20 Jan 2022 08:43:34 +0000 Received: from PH0PR11MB4920.namprd11.prod.outlook.com ([fe80::d7c:834f:4591:3c0c]) by PH0PR11MB4920.namprd11.prod.outlook.com ([fe80::d7c:834f:4591:3c0c%8]) with mapi id 15.20.4909.010; Thu, 20 Jan 2022 08:43:34 +0000 From: To: Subject: Re: [PATCH v3 23/23] media: atmel: atmel-isc: change format propagation to subdev into only verification Thread-Topic: [PATCH v3 23/23] media: atmel: atmel-isc: change format propagation to subdev into only verification Thread-Index: AQHX8Ck/tbX1q7nuLkqDEIOVxgi5pqxfS0aAgAyIAAA= Date: Thu, 20 Jan 2022 08:43:33 +0000 Message-ID: <9470258e-544c-ae65-2222-7a28467c7c99@microchip.com> References: <20211213134940.324266-1-eugen.hristev@microchip.com> <20211213134940.324266-24-eugen.hristev@microchip.com> <20220112092137.rzgmo436hjl2bdfv@uno.localdomain> In-Reply-To: <20220112092137.rzgmo436hjl2bdfv@uno.localdomain> Accept-Language: en-US, ro-RO Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: user-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=microchip.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: c41da79a-2baf-4be0-ec8a-08d9dbf0f19b x-ms-traffictypediagnostic: SA2PR11MB4923:EE_ x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:1443; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: IuZtEgpeNQ2Kqw/3p9miRRwKTmflwxAEVDPmkcPrb+KkP0/pl17A9jIBJeBJWjM9+4UrjOiSYDLStRmkoQPl8wDVh6CFWxYFPri5CLXSJsqB3tnTjxUH5lpyaT3c0pf6KjUpy/dC1XxUYY1gjXAK8JydDieCF1NqE5Vwqpb8V0oBerhIEz2+ANol5+sOTB0GyFBS3AO4UuD+iJdcykN2q+/aCN2m/5dtkZJdb0kFKZONgpojdlNiQNCtJ684nmrXsEXr8Ryo71Omtu/KXeX8RhloZ3+5y3Zm72mPK3zgr9+bfnSaaS26yAmFtZ9kq3KYrJlMM/fjdJCwR8IdHz67NPP7yTWBF444mH3W+4phz/ebE0lfrjQdDOKo3ECmsn5urDTDTNvoZNg2I2lobw27S7TijaRuVHumG2U2yLebOXCXV/MwtLz/0KmdR8wy/sm+BtZ/nLb+HW1Un23eQ9FCl1tCSbnSbSdBQ4BWPKShwPavKvDF0fE6OCsl0MvjlzogbqA6/WTOSAUX7IG6lTMNtxwEmPMDKCBnTcj/cK5InF7Jh2WsqgoWNIOJ7s7YzLtmHefkN/4ig7dYgB9LzA3HPPe60JsDarqMGXqOh+0eHP8QIUW7nNUm7JbfXUUi8Lab0NYHG/MtdokadTlo6wnyZ1ZzoBv0kONoF3h+bQn+YZAqyxRCeykDtG9E6v75147CTEByWqWcpoJgM7U4IVbcsyRyUmfxScLsDerQxpqSp7LpN1VdIZ6EnCRmH9jX+01OUuTkhWku1eBNl1FGOWz09A== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:PH0PR11MB4920.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(366004)(6916009)(6506007)(26005)(83380400001)(53546011)(66946007)(66446008)(6512007)(66476007)(66556008)(64756008)(186003)(8936002)(4326008)(38100700002)(122000001)(5660300002)(2616005)(508600001)(86362001)(2906002)(91956017)(76116006)(31696002)(31686004)(316002)(54906003)(107886003)(15650500001)(8676002)(30864003)(71200400001)(6486002)(38070700005)(36756003)(43740500002)(45980500001); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?utf-8?B?aStWVUxleXpHNnM0ZWIxc0R5SzZRRmhyMWs3VjhET01QUTlqeXNjMHBTaWdn?= =?utf-8?B?Z0c5dnRPejAzY2dDZyt1eGtjYXVlN25xMTJRTHY5NnB4T3pybnFTaUNPZ0tv?= =?utf-8?B?enJWZU01RWcxWjMyT3pZWmhWR1FCSDdCU0pMZUdDNnRQKzBaejRBa04yeTI1?= =?utf-8?B?UDk3NlFxL0NIVXZRb3g5YmhENnhWTnpQYW1sODRPMmdyUWNzKzBsMTVFNUdW?= =?utf-8?B?dXQrTW5RUTVNcHdYVnZBQkxhQmFrVFI5Y3dXdzFHbnZCYmY2K29RdW9CcGVI?= =?utf-8?B?TEJMWDduN3ZlU2xiWktwYUkzRG9RTlpCYmZhWjROcTN2UmhEVWUvaGZkbzBT?= =?utf-8?B?MVMzbjFiSldOZld6N1BFdDBEejRiMzlHNXhCLzg1OThJY0thckZtMEg4OGVH?= =?utf-8?B?SWZIUlpxbTMyUWpQc2QxMWVMalZmbDlOc0VUZVRpU3JYbVhxUkhrdWZPTFlZ?= =?utf-8?B?NUhWVjZLZCtYbU1Ja1g2dHdxV0JGQ0Z2ZEdOaUk0WEpyVDRhM2dSci9lRVBo?= =?utf-8?B?NVMzVEFLYVNscFRNdG9JbW0vNi9rU3Z1Z3VyMlRISndnQ3hwQW1QS1lRSTZt?= =?utf-8?B?c3huR2RTazJLRy9GYXc1M1YzSTdZckFWT2Fmazk2SUFYdkpaczBXMFdvRzZK?= =?utf-8?B?eXdaaGF5U0ZEYlZmcXdpa0FxaEg1N1hJZEliQ3JoZXVycVY4OGlJdTNuUFdV?= =?utf-8?B?L3BKNXdVeWIwVG1VSUNCYmhXd3hnMGVweUpVeWIwOUs1VnFIblZONEcwNHRw?= =?utf-8?B?M3p0Vk4rakhwTVMrV0J3MkdLTWdabTBGeUVRckFuOWZDVllpaVViclFmcVli?= =?utf-8?B?YU9YamlxTEJpN3BubGpIRmpralI0RGp6YVpKK1BqWFhRVk5mZ1IwVkxodUZP?= =?utf-8?B?N2pWOHFWM0tjZDV6MUZielBpREk5MWhzNXRnczBVa3VSRGJTV0R5eDd6c0xH?= =?utf-8?B?cDM0WE5GUmdUQTVRUEFpZVVDb3k0S2c0MXRZa0RPbHlzYmxSSHJOSFd2Y1d0?= =?utf-8?B?UG12YVZYUktFYis3TDB2UmRIYTZJNVpiUGR6ak5MNWFUVklYWlpEVVp1NFBo?= =?utf-8?B?TWFjYkhJdUR2Nkk1Y1RBdmZZL3NFQ1pybW11TzdoSnREUVlKR0Y1ZUdrOE00?= =?utf-8?B?UytvblhWY3g2MXdzZGhqUjBnRWVMUDRPQTY5dG9PM3RTaXNYWFczWCtLRWds?= =?utf-8?B?YWNlS3YwWUIrTlRnOUhFWWhkRTl4dEpLbURMYVdLK3psM2RScm1hYXNmT3NT?= =?utf-8?B?aXd0czhhQkREWHY3dGdwcEYzMDRLRVdTWW95dUVoSk15SG9nd0h1cDVxdFBQ?= =?utf-8?B?MlEwbmhlNWVobEE0Rmx0VU1lLy81bEwrZUFpYW1HZ090NEdMNzkxVExCOWFV?= =?utf-8?B?K2YxZU8rcjl0aEJCSDJIVm5YbTJwaUZwblMvcXdZQXp1ZHpaOXJCS1NDWG1Z?= =?utf-8?B?cFQyMUdsaUl6bUNFVERUT3JLb0NES1VmZ1M1cjV5eXNuZFZORUlqeGpoeDFa?= =?utf-8?B?ZDFkUEQ0UWswa3ZnS0ZFTVhndjgvYUZCcE9yWmpFLzk0WFE4UFFJK1RlSExZ?= =?utf-8?B?bldIQUtobHlYdkQrSnU1a3MrN3Z6cFRQQzRHcSt3ZzNtVVlxZy85b1BGM0xr?= =?utf-8?B?bm5scldKWE9Xd2xIdFNtS1RndlhxbVVHVWxxUHJHMjQwODRCUmdtcXMwNFdi?= =?utf-8?B?Yk1tYzEzdzNtcGNLV3hNbW5KYVBmeGkwL1VrRzVHL2E2aWlKUUtrVWp0MHFR?= =?utf-8?B?bWNqZk1jeDlkdlhnVUhsdlp2NGozUy9aV3VVZkRJaGxyeGZJUW5GN1Q0ZGpE?= =?utf-8?B?aHVPRmFEWjVuVWFkM29zNDViNTBIa09UZjdQZWJvWW9WTWp0RnpWSkNoZDd4?= =?utf-8?B?bjVwTzhZNmVSV2lxMTVadUxpdW5JcWNHVjh5M2lzWTQ4YVk1TnpnWTZLWkZu?= =?utf-8?B?S3hOWDY3VEJPNWxjT0JVbVZEMGtMT1JySmdLWFpYTnJmZzNnTDY0eVlLS0Mx?= =?utf-8?B?RDJoODZNUE9KR1hFcTZoN1BCR3V6T1U5NVFEd2pKcEswSWRTeFhjQ0JXNVdO?= =?utf-8?B?VWlyZ1JzVFZyY3BCMmUwdWtlVjM3OG1zSkdkVjFSaHdXajlmL09Yb2Evb2cz?= =?utf-8?B?aktvc0RFU2NlRFJnRStDMDBHaVFBMCtLaDlQNGMrM0dHWDFyd1ZlVWxYU2p1?= =?utf-8?B?NG5tVC9hVXI4T3hLMzlSejVZNUxWdXVWRTAzY0h3a2puWjd2Vy9LZzExQU5R?= =?utf-8?B?UVR2NktocXR2V29PdnFhSEFXL2RBPT0=?= Content-ID: MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4920.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: c41da79a-2baf-4be0-ec8a-08d9dbf0f19b X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Jan 2022 08:43:33.7423 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: zlPoj7hPhkHbwuZ7pHT1vtkwLhGa3RarbSY9Q7f/P+1mx3khkG6+t9Xq6J8TLr1N431OxtlrVccgfBJbEphCUX3ju0mhDJ8s1SQhDpznt7Y= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB4923 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220120_004340_907413_BC5B1E48 X-CRM114-Status: GOOD ( 28.74 ) 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: , Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, robh+dt@kernel.org, sakari.ailus@iki.fi, laurent.pinchart@ideasonboard.com, 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+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 1/12/22 11:21 AM, Jacopo Mondi wrote: > Hi Eugen Hi Jacopo, > > On Mon, Dec 13, 2021 at 03:49:40PM +0200, Eugen Hristev wrote: >> As a top MC video driver, the atmel-isc should not propagate the format to the >> subdevice. >> It should rather check at streamon() time if the subdev is properly configured >> with a compatible format. >> Removed the whole format finding logic, and reworked the format verification >> at streamon time, such that the ISC will return an error if the subdevice >> is not properly configured. >> With this being done, the module parameter 'sensor_prefered' makes no sense >> anymore. The ISC should not decide which format the sensor is using. The >> ISC should only cope with the situation and inform userspace if the streaming >> is possible in the current configuration. > > Sounds great! > >> >> Signed-off-by: Eugen Hristev >> --- >> Changes in v3: >> - clamp to maximum resolution once the frame size from the subdev is found >> >> drivers/media/platform/atmel/atmel-isc-base.c | 271 ++++++++---------- >> drivers/media/platform/atmel/atmel-isc.h | 1 + >> 2 files changed, 126 insertions(+), 146 deletions(-) >> >> diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c >> index 31c8e3029eee..00c8c9588a78 100644 >> --- a/drivers/media/platform/atmel/atmel-isc-base.c >> +++ b/drivers/media/platform/atmel/atmel-isc-base.c >> @@ -36,11 +36,6 @@ static unsigned int debug; >> module_param(debug, int, 0644); >> MODULE_PARM_DESC(debug, "debug level (0-2)"); >> >> -static unsigned int sensor_preferred = 1; >> -module_param(sensor_preferred, uint, 0644); >> -MODULE_PARM_DESC(sensor_preferred, >> - "Sensor is preferred to output the specified format (1-on 0-off), default 1"); >> - >> #define ISC_IS_FORMAT_RAW(mbus_code) \ >> (((mbus_code) & 0xf000) == 0x3000) >> >> @@ -532,7 +527,7 @@ static int isc_enum_fmt_vid_cap(struct file *file, void *priv, >> * convert it to any of the formats that we usually can with a >> * RAW sensor. Thus, do not advertise them. >> */ >> - if (!isc->config.sd_format || >> + if (isc->config.sd_format && > > Is this change intentional ? > >> !ISC_IS_FORMAT_RAW(isc->config.sd_format->mbus_code)) >> return -EINVAL; >> >> @@ -621,20 +616,30 @@ static int isc_try_validate_formats(struct isc_device *isc) >> break; >> default: >> /* any other different formats are not supported */ >> + v4l2_err(&isc->v4l2_dev, "Requested unsupported format.\n"); >> ret = -EINVAL; >> } >> v4l2_dbg(1, debug, &isc->v4l2_dev, >> "Format validation, requested rgb=%u, yuv=%u, grey=%u, bayer=%u\n", >> rgb, yuv, grey, bayer); > > Would it make sense to move this before the switch so that the > error messages, if any, appear later ? Actually, no, because the variables rgb, yuv, grey, bayer, are set according to what happens in the switch > >> >> - /* we cannot output RAW if we do not receive RAW */ >> - if ((bayer) && !ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code)) >> + if ((bayer) && >> + !ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code)) { >> + v4l2_err(&isc->v4l2_dev, "Cannot output RAW if we do not receive RAW.\n"); >> return -EINVAL; >> + } >> >> - /* we cannot output GREY if we do not receive RAW/GREY */ >> if (grey && !ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code) && >> - !ISC_IS_FORMAT_GREY(isc->try_config.sd_format->mbus_code)) >> + !ISC_IS_FORMAT_GREY(isc->try_config.sd_format->mbus_code)) { >> + v4l2_err(&isc->v4l2_dev, "Cannot output GREY if we do not receive RAW/GREY.\n"); >> return -EINVAL; >> + } >> + >> + if ((rgb || bayer || yuv) && >> + ISC_IS_FORMAT_GREY(isc->try_config.sd_format->mbus_code)) { >> + v4l2_err(&isc->v4l2_dev, "Cannot convert GREY to another format.\n"); >> + return -EINVAL; >> + } >> >> return ret; >> } >> @@ -862,7 +867,7 @@ static void isc_try_fse(struct isc_device *isc, >> * If we do not know yet which format the subdev is using, we cannot >> * do anything. >> */ >> - if (!isc->try_config.sd_format) >> + if (!isc->config.sd_format) >> return; >> >> fse.code = isc->try_config.sd_format->mbus_code; >> @@ -883,180 +888,141 @@ static void isc_try_fse(struct isc_device *isc, >> } >> } >> >> -static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f, >> - u32 *code) >> +static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f) >> { >> - int i; >> - struct isc_format *sd_fmt = NULL, *direct_fmt = NULL; >> struct v4l2_pix_format *pixfmt = &f->fmt.pix; >> - struct v4l2_subdev_pad_config pad_cfg = {}; >> - struct v4l2_subdev_state pad_state = { >> - .pads = &pad_cfg >> - }; >> - struct v4l2_subdev_format format = { >> - .which = V4L2_SUBDEV_FORMAT_TRY, >> - }; >> - u32 mbus_code; >> - int ret; >> - bool rlp_dma_direct_dump = false; >> + unsigned int i; >> >> if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) >> return -EINVAL; >> >> - /* Step 1: find a RAW format that is supported */ >> - for (i = 0; i < isc->num_user_formats; i++) { >> - if (ISC_IS_FORMAT_RAW(isc->user_formats[i]->mbus_code)) { >> - sd_fmt = isc->user_formats[i]; >> + isc->try_config.fourcc = isc->user_formats[0]->fourcc; >> + >> + /* find if the format requested is supported */ >> + for (i = 0; i < isc->controller_formats_size; i++) >> + if (isc->controller_formats[i].fourcc == pixfmt->pixelformat) { >> + isc->try_config.fourcc = pixfmt->pixelformat; >> break; >> } >> - } >> - /* Step 2: We can continue with this RAW format, or we can look >> - * for better: maybe sensor supports directly what we need. >> - */ >> - direct_fmt = find_format_by_fourcc(isc, pixfmt->pixelformat); >> - >> - /* Step 3: We have both. We decide given the module parameter which >> - * one to use. >> - */ >> - if (direct_fmt && sd_fmt && sensor_preferred) >> - sd_fmt = direct_fmt; >> - >> - /* Step 4: we do not have RAW but we have a direct format. Use it. */ >> - if (direct_fmt && !sd_fmt) >> - sd_fmt = direct_fmt; >> - >> - /* Step 5: if we are using a direct format, we need to package >> - * everything as 8 bit data and just dump it >> - */ >> - if (sd_fmt == direct_fmt) >> - rlp_dma_direct_dump = true; >> - >> - /* Step 6: We have no format. This can happen if the userspace >> - * requests some weird/invalid format. >> - * In this case, default to whatever we have >> - */ >> - if (!sd_fmt && !direct_fmt) { >> - sd_fmt = isc->user_formats[isc->num_user_formats - 1]; >> - v4l2_dbg(1, debug, &isc->v4l2_dev, >> - "Sensor not supporting %.4s, using %.4s\n", >> - (char *)&pixfmt->pixelformat, (char *)&sd_fmt->fourcc); >> - } >> - >> - if (!sd_fmt) { >> - ret = -EINVAL; >> - goto isc_try_fmt_err; >> - } >> - >> - /* Step 7: Print out what we decided for debugging */ >> - v4l2_dbg(1, debug, &isc->v4l2_dev, >> - "Preferring to have sensor using format %.4s\n", >> - (char *)&sd_fmt->fourcc); >> - >> - /* Step 8: at this moment we decided which format the subdev will use */ >> - isc->try_config.sd_format = sd_fmt; >> - >> - /* Limit to Atmel ISC hardware capabilities */ >> - if (pixfmt->width > isc->max_width) >> - pixfmt->width = isc->max_width; >> - if (pixfmt->height > isc->max_height) >> - pixfmt->height = isc->max_height; >> - >> - /* >> - * The mbus format is the one the subdev outputs. >> - * The pixels will be transferred in this format Sensor -> ISC >> - */ >> - mbus_code = sd_fmt->mbus_code; >> - >> - /* >> - * Validate formats. If the required format is not OK, default to raw. >> - */ >> - >> - isc->try_config.fourcc = pixfmt->pixelformat; >> - >> - if (isc_try_validate_formats(isc)) { >> - pixfmt->pixelformat = isc->try_config.fourcc = sd_fmt->fourcc; >> - /* Re-try to validate the new format */ >> - ret = isc_try_validate_formats(isc); >> - if (ret) >> - goto isc_try_fmt_err; >> - } >> - >> - ret = isc_try_configure_rlp_dma(isc, rlp_dma_direct_dump); >> - if (ret) >> - goto isc_try_fmt_err; >> - >> - ret = isc_try_configure_pipeline(isc); >> - if (ret) >> - goto isc_try_fmt_err; >> >> - /* Obtain frame sizes if possible to have crop requirements ready */ >> - isc_try_fse(isc, &pad_state); >> - >> - v4l2_fill_mbus_format(&format.format, pixfmt, mbus_code); >> - ret = v4l2_subdev_call(isc->current_subdev->sd, pad, set_fmt, >> - &pad_state, &format); >> - if (ret < 0) >> - goto isc_try_fmt_subdev_err; >> + /* If we did not find the requested format, we will fallback here */ >> + pixfmt->pixelformat = isc->try_config.fourcc; >> + pixfmt->colorspace = V4L2_COLORSPACE_SRGB; >> + pixfmt->field = V4L2_FIELD_NONE; >> >> - v4l2_fill_pix_format(pixfmt, &format.format); >> + isc_try_configure_rlp_dma(isc, false); >> >> /* Limit to Atmel ISC hardware capabilities */ >> - if (pixfmt->width > isc->max_width) >> - pixfmt->width = isc->max_width; >> - if (pixfmt->height > isc->max_height) >> - pixfmt->height = isc->max_height; >> + v4l_bound_align_image(&pixfmt->width, 16, isc->max_width, 0, >> + &pixfmt->height, 16, isc->max_height, 0, 0); >> >> pixfmt->field = V4L2_FIELD_NONE; >> pixfmt->bytesperline = (pixfmt->width * isc->try_config.bpp_v4l2) >> 3; >> pixfmt->sizeimage = ((pixfmt->width * isc->try_config.bpp) >> 3) * >> pixfmt->height; >> >> - if (code) >> - *code = mbus_code; >> + isc->try_fmt = *f; >> >> return 0; >> +} >> >> -isc_try_fmt_err: >> - v4l2_err(&isc->v4l2_dev, "Could not find any possible format for a working pipeline\n"); >> -isc_try_fmt_subdev_err: >> - memset(&isc->try_config, 0, sizeof(isc->try_config)); >> +static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f) >> +{ >> + isc_try_fmt(isc, f); >> >> - return ret; >> + /* make the try configuration active */ >> + isc->config = isc->try_config; >> + isc->fmt = isc->try_fmt; >> + >> + v4l2_dbg(1, debug, &isc->v4l2_dev, "ISC set_fmt to %.4s @%dx%d\n", >> + (char *)&f->fmt.pix.pixelformat, >> + f->fmt.pix.width, f->fmt.pix.height); >> + >> + return 0; >> } >> >> -static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f) >> +static int isc_validate(struct isc_device *isc) >> { >> + int ret; >> + int i; >> + struct isc_format *sd_fmt = NULL; >> + struct v4l2_pix_format *pixfmt = &isc->fmt.fmt.pix; >> struct v4l2_subdev_format format = { >> .which = V4L2_SUBDEV_FORMAT_ACTIVE, >> + .pad = isc->remote_pad, >> + }; >> + struct v4l2_subdev_pad_config pad_cfg = {}; >> + struct v4l2_subdev_state pad_state = { >> + .pads = &pad_cfg, >> }; >> - u32 mbus_code = 0; >> - int ret; >> >> - ret = isc_try_fmt(isc, f, &mbus_code); >> + /* Get current format from subdev */ >> + ret = v4l2_subdev_call(isc->current_subdev->sd, pad, get_fmt, NULL, >> + &format); > > Ah! Haven't you just said we don't care anymore about the subdev > format ? :) We don't ! but we can't stream anything if the sensor streams a format that we don't understand do we ? > > Kidding, I might got a bit lost in the logic, but if I look at your > above isc_try_validate_formats() it seems like an ideal candidate for > .link_validate() media_entity operation. > > Just to make sure we're on the same page, here's how it should ideally > look like: > - set format does care about subdev format. It only checks that the > format required from the user is one of the ISC supported one. Ie. > no v4l2_subdev_call() > > - as s_stream time your top driver calls media_pipeline_start() > > - media_pipeline_start() walks all the entities in the pipeline and > validates the format of connected pads. To validate formats the > __media_pipeline_start() functions calls link_validate() on each > entity. You should in your driver set Is this done automatically ? if this would be the case, where all links are correctly set, then, in theory, it would not be possible that some entities have a format set that is not supported by the other side > > static const struct media_entity_operations your_media_entity_ops = { > .link_validate = v4l2_subdev_link_validate, > }; > > if you want to use the default link validation procedure, or set the > callback to your custom validation function, which can behave more > or less like isc_try_validate_formats() Does this mean that I can ask the subdev for the format ? Or how can I obtain the format on the other side of the link ? link validate can do this for me ? Thanks for reviewing, Eugen > > Does it match your understanding too ? > > Thanks > j > > > >> if (ret) >> return ret; >> >> - v4l2_fill_mbus_format(&format.format, &f->fmt.pix, mbus_code); >> - ret = v4l2_subdev_call(isc->current_subdev->sd, pad, >> - set_fmt, NULL, &format); >> - if (ret < 0) >> - return ret; >> + /* Identify the subdev's format configuration */ >> + for (i = 0; i < isc->num_user_formats; i++) >> + if (isc->user_formats[i]->mbus_code == format.format.code) { >> + sd_fmt = isc->user_formats[i]; >> + break; >> + } >> + >> + /* Check if the format is not supported */ >> + if (!sd_fmt) { >> + v4l2_err(&isc->v4l2_dev, >> + "Current subdevice is streaming a media bus code that is not supported 0x%x\n", >> + format.format.code); >> + return -EPIPE; >> + } >> + >> + /* At this moment we know which format the subdev will use */ >> + isc->try_config.sd_format = sd_fmt; >> + >> + /* If the sensor is not RAW, we can only do a direct dump */ >> + if (!ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code)) >> + isc_try_configure_rlp_dma(isc, true); >> >> /* Limit to Atmel ISC hardware capabilities */ >> - if (f->fmt.pix.width > isc->max_width) >> - f->fmt.pix.width = isc->max_width; >> - if (f->fmt.pix.height > isc->max_height) >> - f->fmt.pix.height = isc->max_height; >> + v4l_bound_align_image(&format.format.width, 16, isc->max_width, 0, >> + &format.format.height, 16, isc->max_height, 0, 0); >> >> - isc->fmt = *f; >> + /* Check if the frame size is the same. Otherwise we may overflow */ >> + if (pixfmt->height != format.format.height || >> + pixfmt->width != format.format.width) { >> + v4l2_err(&isc->v4l2_dev, >> + "ISC not configured with the proper frame size: %dx%d\n", >> + format.format.width, format.format.height); >> + return -EPIPE; >> + } >> >> + v4l2_dbg(1, debug, &isc->v4l2_dev, >> + "Identified subdev using format %.4s with %dx%d %d bpp\n", >> + (char *)&sd_fmt->fourcc, pixfmt->width, pixfmt->height, >> + isc->try_config.bpp); >> + >> + /* Reset and restart AWB if the subdevice changed the format */ >> if (isc->try_config.sd_format && isc->config.sd_format && >> isc->try_config.sd_format != isc->config.sd_format) { >> isc->ctrls.hist_stat = HIST_INIT; >> isc_reset_awb_ctrls(isc); >> isc_update_v4l2_ctrls(isc); >> } >> - /* make the try configuration active */ >> + >> + /* Validate formats */ >> + ret = isc_try_validate_formats(isc); >> + if (ret) >> + return ret; >> + >> + /* Obtain frame sizes if possible to have crop requirements ready */ >> + isc_try_fse(isc, &pad_state); >> + >> + /* Configure ISC pipeline for the config */ >> + ret = isc_try_configure_pipeline(isc); >> + if (ret) >> + return ret; >> + >> isc->config = isc->try_config; >> >> v4l2_dbg(1, debug, &isc->v4l2_dev, "New ISC configuration in place\n"); >> @@ -1064,6 +1030,19 @@ static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f) >> return 0; >> } >> >> +static int isc_streamon(struct file *file, void *priv, enum v4l2_buf_type bt) >> +{ >> + struct isc_device *isc = video_drvdata(file); >> + int ret; >> + >> + ret = isc_validate(isc); >> + >> + if (ret) >> + return ret; >> + >> + return vb2_ioctl_streamon(file, priv, bt); >> +} >> + >> static int isc_s_fmt_vid_cap(struct file *file, void *priv, >> struct v4l2_format *f) >> { >> @@ -1080,7 +1059,7 @@ static int isc_try_fmt_vid_cap(struct file *file, void *priv, >> { >> struct isc_device *isc = video_drvdata(file); >> >> - return isc_try_fmt(isc, f, NULL); >> + return isc_try_fmt(isc, f); >> } >> >> static int isc_enum_input(struct file *file, void *priv, >> @@ -1176,7 +1155,7 @@ static const struct v4l2_ioctl_ops isc_ioctl_ops = { >> .vidioc_dqbuf = vb2_ioctl_dqbuf, >> .vidioc_create_bufs = vb2_ioctl_create_bufs, >> .vidioc_prepare_buf = vb2_ioctl_prepare_buf, >> - .vidioc_streamon = vb2_ioctl_streamon, >> + .vidioc_streamon = isc_streamon, >> .vidioc_streamoff = vb2_ioctl_streamoff, >> >> .vidioc_g_parm = isc_g_parm, >> @@ -1879,7 +1858,7 @@ static int isc_set_default_fmt(struct isc_device *isc) >> }; >> int ret; >> >> - ret = isc_try_fmt(isc, &f, NULL); >> + ret = isc_try_fmt(isc, &f); >> if (ret) >> return ret; >> >> diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h >> index c2cb805faff3..7081698adddd 100644 >> --- a/drivers/media/platform/atmel/atmel-isc.h >> +++ b/drivers/media/platform/atmel/atmel-isc.h >> @@ -297,6 +297,7 @@ struct isc_device { >> struct completion comp; >> >> struct v4l2_format fmt; >> + struct v4l2_format try_fmt; >> struct isc_format **user_formats; >> unsigned int num_user_formats; >> >> -- >> 2.25.1 >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel