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 E14A6C433F5 for ; Fri, 11 Feb 2022 10:44:45 +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=j2LDrMe0T0FlzEPToim99VGFiGWWYLgJbjIO93F4ITw=; b=S/V19fR759PXME dn87Cu17JwSvp2F9XPNPHWmd18hf5sFkNCoy7w/2If4Q76+rrPjBCdG1GUSa/ZOeFCtjLBe1M13UW fBg4mrywF33NrH6DNkCq7UXdlrZc/hWrD49rzYjbGwLevy7LoZC5aJ5hh53B4FEOI/KLGECZD2EEb SVd4CKoXe8tT3vGhjGyr57DfIACBoZFHBv1bl2kjJEKMGCfWbYFD4RM5w4HV9m0v1oIUY5v9fdtpx iCcz6g1II+X0Wn2LZRrVxi9qiY/XVlWDHaWh/zB3TXwtOnxmWp5Tj9YWec3jpBYkfySQsPC9Ajw// ge8bRMa9Be+3UveKHHvg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nITNs-006dam-D5; Fri, 11 Feb 2022 10:42:42 +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 1nITKO-006bw4-MM for linux-arm-kernel@lists.infradead.org; Fri, 11 Feb 2022 10:39:05 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1644575941; x=1676111941; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=ZOINPi4u1MX89B6sUU9lJSXIUX7wpb6WeFpFx7yV1AE=; b=nMZk7i50mjE4XoFhrCvw0wmA6oOX+KZ+rMGPKH1GCQ7nFHkTdsOJ99Eg nHqlFl7TOvOKF+VNi/jlVy5F3C9qQreZIeTo74pnQCxmdBEQb+O+NHyJ4 r17RGOYwRwBM27O+7T4bNWlRaLuHj3ws/1Ebh4MjzTsFjAN6K7n6l5FfF wLavoINAyJsk4qGu/uTITYShHfU7sz7ssbP/5dXkYEPk9LkzAtZCTFajH 2KufkGRICGTPZQLj9uQdOVXK5jAn0Rpn7K9YABKjtKXNDqPafEY1oiyFu 4wc/RR+T2+A0sOITRsKqQE4T6Fkdij+ZuAW+OeyAOprXYEWLR4PmcY/Nx A==; IronPort-SDR: uzG0oXxx1vQKVKW0pqiZDwnxImEGa4lwLpAuDaCDuOvrTyX/DcWYR3dJT0yCjh0KaVT6DFve5U 2JJyN+NUvh+YcrwyO3AFjokQd+Qb+fffp0jnxizes213z9b4QiMzQYFZkIqx/jMtXV3Oc4ouQX pV0bFHvPd2uGdUhyVOuuF0399qV6/FeE0E7ded4czcTPa5KAaVLo1x8d0GOqqkVmGG1JaLNrNM hmmFkzcL9umXJQ3X5CtFnjSgtS+6CpbHrlQZxIToAfXLdeNo1mOH7wpVSs4thrLitUIE/DEc7h MTJOFrNOfvHwWxSqH0Nax2my X-IronPort-AV: E=Sophos;i="5.88,360,1635231600"; d="scan'208";a="152745265" Received: from smtpout.microchip.com (HELO email.microchip.com) ([198.175.253.82]) by esa5.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 11 Feb 2022 03:38:59 -0700 Received: from chn-vm-ex03.mchp-main.com (10.10.85.151) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.17; Fri, 11 Feb 2022 03:38:58 -0700 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (10.10.215.89) by email.microchip.com (10.10.87.152) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.17 via Frontend Transport; Fri, 11 Feb 2022 03:38:58 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NOjbc4YmMDEiZrYm4gkaISHvQtsJJHOegBAWR5BfvKn6YeYR02B3UjKeQB9wHxBkR4+JrF+2EGi8T5LhyXLDaPXPvlSXdVKXRoFRm92PhbhntMzqdGHfda6A/dzUyxi+Wc39aY9zR4t0WJhXUvK3FmsgreCnugabPvqyETn11nkoA9/8i+a15jTS5vP5mv0zPrY8Pyeju7DolgTqsaAbAKzkJEfrv46GDW0GfHTpBUsyPmp6XLVm09UMx6fDwVL6SrBHIv9CfT42sEt1wXNyVuF357AZRAP00NPbTs/y2tzEkAOFbDh3G0c+wCxi/HExHgSJkrYtvpUjdNAdSabrZQ== 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=ZOINPi4u1MX89B6sUU9lJSXIUX7wpb6WeFpFx7yV1AE=; b=N2SpeN+lKiWFchMtfEQqMMGIdwohe+ss0MpfMJk1uJ6nbRw7cfWDrG8d7ZO2AqlKmv9dP2kuQ3/wGdHj5SmFB7ICXfzS7uEvcn/qRm/ayQGo7hrSEh3wcRukGUjRNcsN9gszT/Ked/53t+kF2znEXG8Eq/hQQNgUgtw37sASM6TTw/EtG64xuciULLpvlq3Y6bFSO2qMQsXK7Xxb9Cz9DbsT+han8JFb4ojk+14kxKfmKiIClFZJX8qrhBcjjZ2ZMlZjkVKSawjl8+DysVMowmzbmqCi0zkB5mOA5l2yMlDAfhnrW3myChMSifmw5sN+gVhFdZeOZ9EwwH3Ch9FXyw== 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=ZOINPi4u1MX89B6sUU9lJSXIUX7wpb6WeFpFx7yV1AE=; b=foeRZd/z/SS2AyEs6iuB1It8s5Aw72ho+ZBojEfZveJCb2HRo94oH2qcxS62U5oSydZZCg12/HG/TN9POujb5COikkIC9ekut9YakDQTJSZHuQYCyjSFcw6PSmdZu3ioQHkJnEhTLHx+k8hGBfdRTTkRunNteMZIwybmEv0UJ4c= Received: from BL1PR11MB5384.namprd11.prod.outlook.com (2603:10b6:208:311::14) by BN8PR11MB3635.namprd11.prod.outlook.com (2603:10b6:408:86::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4951.14; Fri, 11 Feb 2022 10:38:56 +0000 Received: from BL1PR11MB5384.namprd11.prod.outlook.com ([fe80::11a5:42e0:3f3d:fcdc]) by BL1PR11MB5384.namprd11.prod.outlook.com ([fe80::11a5:42e0:3f3d:fcdc%9]) with mapi id 15.20.4975.011; Fri, 11 Feb 2022 10:38:56 +0000 From: To: Subject: Re: [PATCH v4 07/11] media: atmel: atmel-isc: change format propagation to subdev into only verification Thread-Topic: [PATCH v4 07/11] media: atmel: atmel-isc: change format propagation to subdev into only verification Thread-Index: AQHYDskk/iS7/SebFEGaKQlBt8SkBayNPoaAgAELBYA= Date: Fri, 11 Feb 2022 10:38:56 +0000 Message-ID: References: <20220121131416.603972-1-eugen.hristev@microchip.com> <20220121131416.603972-8-eugen.hristev@microchip.com> <20220210184309.5ifihhnb4qmahdy5@uno.localdomain> In-Reply-To: <20220210184309.5ifihhnb4qmahdy5@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: 02b5e087-2488-42c2-d380-08d9ed4ab4ae x-ms-traffictypediagnostic: BN8PR11MB3635:EE_ x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:901; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: LqltieaHUDXVcmGIjBMD4HnKBU52/I1FEvtxrVMAeIP08dJRXPQUJeq/zzCoc0pO5AdEp9TuCB8z+tsyEsxc1qGGOMz0NU6YDk7geTvaCGjc6ELR6TDre37rbv/53LxrFEnzXyHpRlN9zdPmQiuHguG+Sd5N4U2AEk2sIGqXowGADSecspY+cM+uyFZ0tuNzFAGDq8jF2XAnquj1eRpJFVPi7DTjP81913cFxC5GCf/GaOcbPiNQ1XHSpPSnm56/JKh2F4BUDukblir8g5TOAN7pdQ4/wPBfvgAvPenOSCZKOsYrLPsw6WWu/Qm2irhAvyTjs4jK40pXMLJVVnTpilbuWumkSkKL1Xqa9h4PJFwBArIiTMtFhwl3cq1CzQrMXEbgd4nm5GoJtUDuu3yClOLtHyCnmqiPk6/pBHPpR/xlDizsXw5uu1cDMkPhxzUbCbu+5E+7lvpRsEOTTZAI159HYmt3syfdKIHqXXvNSXVBiHeIgMADyNGY6Fp1tzEhrYFXhxi2jqk6bpM1/rNjK0ZuMtjN9j2RbtD62cUCYk2uYgBXxCWNTbz88iIowW2CV5F7nO5TQtcZsTWrWWmOpBgKKV1hErOtC+99kXwc8GjRJisGPnS7RA+EEewlR1b+ytURVschW9GYNrHyvW/Z9diWMBCC4nuGe4BBpXFjFZF10e/y1iwV1C8HINz0RYmxAJa+aAJd6v39jg4lpKYe6KR8whSJYX/uKJCN8AIccFF2JAtKwpCaEg0MlO/7w2CYBOHIjtNew1y/iGzni/2BJQ== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BL1PR11MB5384.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230001)(366004)(6916009)(2616005)(122000001)(31686004)(30864003)(508600001)(2906002)(36756003)(15650500001)(38070700005)(6486002)(316002)(83380400001)(54906003)(66556008)(7416002)(71200400001)(5660300002)(186003)(76116006)(91956017)(66476007)(26005)(38100700002)(4326008)(66446008)(6512007)(31696002)(8676002)(64756008)(66946007)(86362001)(8936002)(53546011)(6506007)(43740500002)(45980500001)(579004)(559001); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?utf-8?B?dnE5N051REllUldYdmNVejAxY2kxMVlBbzlDUG9rc0Z5Q3Vzb2tJVmRLeU1M?= =?utf-8?B?TFhnSTQ3dHIyeFlOeW5Uc2p3aG9DaUJZb1hudDIrL1ErNHc5KzhlMCtIbG1U?= =?utf-8?B?cStuam56Y2RDTXRyL25yTEZnOWh3VnQzZHVISTFtUXlpR25KdzBtczZ0RTJu?= =?utf-8?B?U3JqQWpzUUNhbDR1MkFtaGZwZkNYL2dTQ3JzNUpqNEljRnhhWjA0RFVRVitE?= =?utf-8?B?Ym44Q2krbS9VeDMzdkd4NEU2RGxwL1IrVm1aVFdQTko5MlNPZjZhVFBXOWxv?= =?utf-8?B?aDBJMUxIa3hRZCtxcGRXNXA3MmszSThkVkh3WTlxeGlTYjZuV3dMTDVOcEJ5?= =?utf-8?B?N2Y3Yko5dUxLYm1qd0pyemlacXpwd1p4UXFZOUsweXRoSGpMZlJ0dXg4OW5r?= =?utf-8?B?b0tiNE5JSlRWbVF4MFNxRGJ6bkFiWDlWZ2FqNXZYMmh2UmpWRmpZWUlSeE41?= =?utf-8?B?MHVqa242RDRZVVVPc0V4TGJ2TlJOV3pHOUZ4Sy9QcEdtTDF3RnMxMDVrTXNV?= =?utf-8?B?TGxva1JkN0pwd3FJbjBJdU92UDVkbFk3VmZMSExjeDIzUTJ4WGgvM0pQYXdL?= =?utf-8?B?TEY2WnJIY2lrUzBCK3dqNExJTWxDcTFsUGhIbjRwZEdRMG1vZCtXOGhrSllk?= =?utf-8?B?NU1WeWJsSUVFcWo2QTc1cjZpNjlOQ0pEVnVpLzJLL0dtWGxzNXE2M1oyT3dy?= =?utf-8?B?SC8rVjVYZTdQMXlhanhyY3RDK1RtOE52WXZXK0QzSjJoblVpYkVkNVVyWjV4?= =?utf-8?B?WDVUSzVYODd2UU1kRjRwN2l4VndFbVVuaDYyRmdOWExyY3IrczNRbnNNa25U?= =?utf-8?B?YzE2SjRDdU1jR25mQ3MwMXJhb2M2S3BBWTFUSHV1N2V1TGJCbVRIdHI1ZDlr?= =?utf-8?B?RU9DSCtyUloxTEFWU1prY2RRNmpmOFMvV1FRZHNZTkxmOFg1Q050L1Fuckpy?= =?utf-8?B?aXQvRXdQN3BoK0RqK1V1RXdkUjRPSmhhRE5yUThHTHV2d2lkRnhWc0l3SnhJ?= =?utf-8?B?WDRXZk81dFNROXhHbGtacHNQVEJUUGVXSGd1ZmVTOGJXQUJWaWdQSDhYK3pq?= =?utf-8?B?TXpOWkMvMVFpQlcwR2RiK01wR1gzSFllaXZtckdocXE4V0plRTliY3BXd2gy?= =?utf-8?B?Z1BuTU9RLzZwMzBUV3Y2S2dsWkNlNnJLSmZvQUUwWGI0MHpvNEl3WEkzYkJr?= =?utf-8?B?S2FFOFpYNnhxN25hTGN0OUV4NXU2SFpiZHgvQVhlUlRuTFFOcVpIUTRPSUdF?= =?utf-8?B?S1p2cTEzYnRPUWRjTU84RXg1eENzSHJKaVNtWHNoUDVOSmp1VkVqa2tLcUs2?= =?utf-8?B?YWkxbXhUQ3VWQWJnUktGVzlrOHFobWlIKzNtTGZVUWtoOFlubHlwOERVbGw2?= =?utf-8?B?V05tamZJVlVLK09UZ1NVdDBDNFpVNWxNa2Z0Mm50eTZQRllZMWpZc2pRQkZ5?= =?utf-8?B?QW5BZjcyMDQ5VE1uWnFhMmVERTdwVUMrbE1yb1RFcjA2cW1TUVdablFTc3B1?= =?utf-8?B?b3VrbDJ5bXJrRTJJaHdvNlZxR3FKOWFWR1BwUnpwUFY3QldjYWVkTWtsOE9a?= =?utf-8?B?QkpVUTBha3N1YXlwR2EyeTV5eUNzcUdkWkJQMW9WOHBFM3g0SklYMmQwNnpE?= =?utf-8?B?ZzVXQy9uV1Q5TUQ1K01VS0p2QklDcEdPQUxNNy9vcnFDd0s3MHNqUThWOUF0?= =?utf-8?B?NHBtODVNZjJlOEJPQ2RVQ1VLNE1rdDdpTExnanhvN2tkbCs2dHdSSEJ5SExr?= =?utf-8?B?bm9FeE95cndXeGdVcUpESFVZUVc3bXJ0UjdlZTZKOU5BZVpCOGVmL0l3TVVl?= =?utf-8?B?dXNhbXgrRklLT2lnQzY5bGRISFA2WERmL0FOZGMvdXR4cUVzblZMTVRJSG5w?= =?utf-8?B?RXJ6NzE0Q2p3VTJnM2lFam9zcmdLUWlOTkRpYXc3YkhVK2UyRFltMEVJSU9i?= =?utf-8?B?a1ROakw5Y1pURktVZmhwTzQydWQ4SDFWWlZaQklodEcrOEJQd0VrYlFxOTQy?= =?utf-8?B?VXRxTDBMMllOaFdrd01GeTJ5QlBhRXh3VzQxbFdSNW9VckJnK1ZYUEQ1eUhj?= =?utf-8?B?aXY5S2t0WWNvaUpWNmtqaUl4cTFRRjRXQW00VUJsM1RSSzREM043bldSS0dQ?= =?utf-8?B?dG5RUXdqU0pEb3lsaDlQWmlOaC9tRllGTlpFZ2hRN0drR0VOTkhJSWxSQWpi?= =?utf-8?B?NTBlOE1saUc1dWFXVHdvRVFYbnhaTWlTNzFFSFRWU0grUWFrRU4rVkdiRERs?= =?utf-8?B?Q0RHZ1NqaGpBNXova1ByTzRsb1BnPT0=?= Content-ID: MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5384.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 02b5e087-2488-42c2-d380-08d9ed4ab4ae X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Feb 2022 10:38:56.3569 (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: LDiyzxm2CYquNNtH7Pu/cPH+U7SnWdpf4SlLaogDhygImoWDYMMvAJ4bycoxxNmEuMp1tm3sRf0nAizmJGX0+IssqVRl9WA8o/Vjxed30QI= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR11MB3635 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220211_023900_878046_43BB3E88 X-CRM114-Status: GOOD ( 27.71 ) 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, jacopo+renesas@jmondi.org, laurent.pinchart@ideasonboard.com, hverkuil-cisco@xs4all.nl, sakari.ailus@iki.fi, 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 2/10/22 8:43 PM, Jacopo Mondi wrote: > Hi Eugen > > On Fri, Jan 21, 2022 at 03:14:12PM +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 start_streaming() time if the subdev is properly >> configured with a compatible format. >> Removed the whole format finding logic, and reworked the format verification >> at start_streaming time, such that the ISC will return an error if the subdevice >> is not properly configured. To achieve this, media_pipeline_start >> is called and a link_validate callback is created to check the formats. >> With this being done, the module parameter 'sensor_preferred' 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. > > This sounds right! > >> The redesign of the format propagation has also risen the question of the >> enumfmt callback. If enumfmt is called with an mbus_code, the enumfmt handler >> should only return the formats that are supported for this mbus_code. >> To make it more easy to understand the formats, changed the report order >> to report first the native formats, and after that the formats that the ISC >> can convert to. >> >> Signed-off-by: Eugen Hristev >> --- >> Changes in v4: >> - moved validation code into link_validate and used media_pipeline_start >> - merged this patch with the enum_fmt patch which was previously in v3 of >> the series >> >> Changes in v3: >> - clamp to maximum resolution once the frame size from the subdev is found >> >> drivers/media/platform/atmel/atmel-isc-base.c | 335 ++++++++++-------- >> .../media/platform/atmel/atmel-isc-scaler.c | 5 + >> drivers/media/platform/atmel/atmel-isc.h | 3 + >> 3 files changed, 191 insertions(+), 152 deletions(-) >> >> diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c >> index f7a88399bd54..31c79313aadc 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) >> >> @@ -337,6 +332,10 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count) >> unsigned long flags; >> int ret; >> >> + ret = media_pipeline_start(&isc->video_dev.entity, &isc->mpipe); >> + if (ret) >> + goto err_pipeline_start; >> + >> /* Enable stream on the sub device */ >> ret = v4l2_subdev_call(isc->current_subdev->sd, video, s_stream, 1); >> if (ret && ret != -ENOIOCTLCMD) { >> @@ -384,6 +383,9 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count) >> v4l2_subdev_call(isc->current_subdev->sd, video, s_stream, 0); >> >> err_start_stream: >> + media_pipeline_stop(&isc->video_dev.entity); >> + >> +err_pipeline_start: >> spin_lock_irqsave(&isc->dma_queue_lock, flags); >> list_for_each_entry(buf, &isc->dma_queue, list) >> vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED); >> @@ -420,6 +422,9 @@ static void isc_stop_streaming(struct vb2_queue *vq) >> if (ret && ret != -ENOIOCTLCMD) >> v4l2_err(&isc->v4l2_dev, "stream off failed in subdev\n"); >> >> + /* Stop media pipeline */ >> + media_pipeline_stop(&isc->video_dev.entity); >> + >> /* Release all active buffers */ >> spin_lock_irqsave(&isc->dma_queue_lock, flags); >> if (unlikely(isc->cur_frm)) { >> @@ -496,21 +501,56 @@ static int isc_enum_fmt_vid_cap(struct file *file, void *priv, >> u32 index = f->index; >> u32 i, supported_index; >> >> - if (index < isc->controller_formats_size) { >> - f->pixelformat = isc->controller_formats[index].fourcc; >> - return 0; >> + supported_index = 0; >> + >> + for (i = 0; i < isc->formats_list_size; i++) { >> + if (!isc->formats_list[i].sd_support) >> + continue; > > Sorry to ask again about sd_support, but if the ISC formats > enumeration is not constrained to a specific mbus_code, shouldn't all > the formats be enumerated regardless if they're supported from the > sensor or not ? Yes. this slipped a little. I have to rework a bit the format reporting to user space. > >> + /* >> + * If specific mbus_code is requested, provide only >> + * supported formats with this mbus code >> + */ >> + if (f->mbus_code && f->mbus_code != >> + isc->formats_list[i].mbus_code) > > nit: for clarity > if (f->mbus_code && > f->mbus_code != isc->formats_list[i].mbus_code) > >> + continue; >> + if (supported_index == index) { >> + f->pixelformat = isc->formats_list[i].fourcc; >> + return 0; >> + } >> + supported_index++; >> } >> >> - index -= isc->controller_formats_size; >> + /* >> + * If the sensor does not support this mbus_code whatsoever, >> + * there is no reason to advertise any of our output formats >> + */ >> + if (supported_index == 0) >> + return -EINVAL; >> >> + /* >> + * If the sensor uses a format that is not raw, then we cannot >> + * 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 && >> + !ISC_IS_FORMAT_RAW(isc->config.sd_format->mbus_code)) >> + return -EINVAL; >> + >> + /* >> + * Iterate again through the formats that we can convert to. >> + * However, to avoid duplicates, skip the formats that >> + * the sensor already supports directly >> + */ >> + index -= supported_index; >> supported_index = 0; >> >> - for (i = 0; i < isc->formats_list_size; i++) { >> - if (!ISC_IS_FORMAT_RAW(isc->formats_list[i].mbus_code) || >> - !isc->formats_list[i].sd_support) >> + for (i = 0; i < isc->controller_formats_size; i++) { >> + /* if this format is already supported by sensor, skip it */ >> + if (find_format_by_fourcc(isc, isc->controller_formats[i].fourcc)) >> continue; >> if (supported_index == index) { >> - f->pixelformat = isc->formats_list[i].fourcc; >> + f->pixelformat = >> + isc->controller_formats[i].fourcc; > > I trust this last part to work correctly, as it's not 100% clear to me :) > I think I will remove this and just report all the formats to userspace. Let libcamera handle this ;) >> return 0; >> } >> supported_index++; >> @@ -581,20 +621,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); >> >> - /* 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) && > > just 'bayer' ? not sure what you mean ? Have to check if we can output raw bayer , and we can only do that if we receive raw bayer from the sensor. > >> + !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; >> } >> @@ -822,7 +872,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; >> @@ -843,180 +893,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; > > Seems pixfmt->field is set twice. > > Could you move the assignment of all pixfmt to a single block ? okay > >> >> - 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); >> 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); >> + >> + /* 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; >> + } >> >> - isc->fmt = *f; >> + 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"); >> @@ -1040,7 +1051,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, >> @@ -1841,7 +1852,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; >> >> @@ -2015,6 +2026,24 @@ int isc_pipeline_init(struct isc_device *isc) >> } >> EXPORT_SYMBOL_GPL(isc_pipeline_init); >> >> +int isc_link_validate(struct media_link *link) >> +{ >> + struct video_device *vdev = >> + media_entity_to_video_device(link->sink->entity); >> + struct isc_device *isc = video_get_drvdata(vdev); >> + int ret; >> + >> + ret = v4l2_subdev_link_validate(link); >> + if (ret) >> + return ret; > > Doesn't v4l2_subdev_link_validate() call > v4l2_subdev_link_validate_default() if you don't define a .link_validate() > pad operation ? v4l2_subdev_link_validate_default() compares format > and sizes of both ends to the links and want them to be identical, > something that defeat the purpose of your custom isc_validate() ? I think so. However if I replace the validate call with my custom one, I thought that the old default should be called as well. Meaning, that maybe my custom validation does certain isc-related checks, while the v4l2_subdev_link_validate does exactly what you said. So, by defining a custom link_validate, do I lose the checks that are done by the v4l2_subdev_link_validate ? That's what I wanted to avoid. Chip in my checks, but keep the v4l2 standard validation in place. Does that make sense ? > Thanks > j > >> + >> + return isc_validate(isc); >> +} >> + >> +static const struct media_entity_operations isc_entity_operations = { >> + .link_validate = isc_link_validate, >> +}; >> + > }; >> int isc_mc_init(struct isc_device *isc, u32 ver) >> { >> const struct of_device_id *match; >> @@ -2022,6 +2051,8 @@ int isc_mc_init(struct isc_device *isc, u32 ver) >> >> isc->video_dev.entity.function = MEDIA_ENT_F_IO_V4L; >> isc->video_dev.entity.flags = MEDIA_ENT_FL_DEFAULT; >> + isc->video_dev.entity.ops = &isc_entity_operations; >> + >> isc->pads[ISC_PAD_SINK].flags = MEDIA_PAD_FL_SINK; >> >> ret = media_entity_pads_init(&isc->video_dev.entity, ISC_PADS_NUM, >> diff --git a/drivers/media/platform/atmel/atmel-isc-scaler.c b/drivers/media/platform/atmel/atmel-isc-scaler.c >> index ec95c9665883..93375a61aea6 100644 >> --- a/drivers/media/platform/atmel/atmel-isc-scaler.c >> +++ b/drivers/media/platform/atmel/atmel-isc-scaler.c >> @@ -155,6 +155,10 @@ static const struct v4l2_subdev_pad_ops isc_scaler_pad_ops = { >> .init_cfg = isc_scaler_init_cfg, >> }; >> >> +static const struct media_entity_operations isc_scaler_entity_ops = { >> + .link_validate = v4l2_subdev_link_validate, >> +}; >> + >> static const struct v4l2_subdev_ops xisc_scaler_subdev_ops = { >> .pad = &isc_scaler_pad_ops, >> }; >> @@ -172,6 +176,7 @@ int isc_scaler_init(struct isc_device *isc) >> >> isc->scaler_sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; >> isc->scaler_sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER; >> + isc->scaler_sd.entity.ops = &isc_scaler_entity_ops; >> isc->scaler_pads[ISC_SCALER_PAD_SINK].flags = MEDIA_PAD_FL_SINK; >> isc->scaler_pads[ISC_SCALER_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE; >> >> diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h >> index 04ea5e340808..e36cf9d3942a 100644 >> --- a/drivers/media/platform/atmel/atmel-isc.h >> +++ b/drivers/media/platform/atmel/atmel-isc.h >> @@ -270,6 +270,7 @@ enum isc_scaler_pads { >> * @formats_list_size: size of formats_list array >> * @pads: media controller pads for isc video entity >> * @mdev: media device that is registered by the isc >> + * @mpipe: media device pipeline used by the isc >> * @remote_pad: remote pad on the connected subdevice >> * @scaler_sd: subdevice for the scaler that isc registers >> * @scaler_pads: media controller pads for the scaler subdevice >> @@ -295,6 +296,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; >> >> @@ -366,6 +368,7 @@ struct isc_device { >> struct { >> struct media_pad pads[ISC_PADS_NUM]; >> struct media_device mdev; >> + struct media_pipeline mpipe; >> >> u32 remote_pad; >> }; >> -- >> 2.25.1 >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel