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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E7980C7EE23 for ; Mon, 5 Jun 2023 08:07:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229549AbjFEIHY (ORCPT ); Mon, 5 Jun 2023 04:07:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51150 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230361AbjFEIHU (ORCPT ); Mon, 5 Jun 2023 04:07:20 -0400 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2D52AAF for ; Mon, 5 Jun 2023 01:07:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1685952439; x=1717488439; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=GtSfj1LgmRgrIEYm4Jj5FaJuaz6ZxcAyzCDjlEgus9k=; b=a3AptKbMOwEEL0PI7FdNt3ow6sSHEFqan6NszHmwSGEcR9c39Z3ULPtr 1SrLTaceznxG4X7jy7VYnEErUUcUy/+FdpzawZ3Pmy/W3L/bFx4byV3YU Cn4cOnBL/hWa07QlE6YYs/+cNoHCaQEYpZsc4bctDdPr1rl5wV1WysOgZ 08JJPdBT7OTapmg72Q3XfCpOxLWGsCxQgUNMUM3cOMXKJbsGntih2dA4n 0em/jZd6xGkulBFxJGFULWTi+cc67nBV9W7t2DjeWM5CBcxDmYE/F1z76 x7BTvKo1keWIoLWn2jPnDsU9erS+qSTR7VqfKl74mNWKlhIUbKzDe3tn3 g==; X-IronPort-AV: E=McAfee;i="6600,9927,10731"; a="422135967" X-IronPort-AV: E=Sophos;i="6.00,217,1681196400"; d="scan'208";a="422135967" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jun 2023 01:07:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10731"; a="778449199" X-IronPort-AV: E=Sophos;i="6.00,217,1681196400"; d="scan'208";a="778449199" Received: from turnipsi.fi.intel.com (HELO kekkonen.fi.intel.com) ([10.237.72.44]) by fmsmga004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jun 2023 01:07:00 -0700 Received: from kekkonen.localdomain (localhost [IPv6:::1]) by kekkonen.fi.intel.com (Postfix) with SMTP id 22C73120A13; Mon, 5 Jun 2023 11:06:58 +0300 (EEST) Date: Mon, 5 Jun 2023 08:06:58 +0000 From: Sakari Ailus To: Laurent Pinchart Cc: Tomi Valkeinen , linux-media@vger.kernel.org, bingbu.cao@intel.com, hongju.wang@intel.com Subject: Re: [RFC 2/7] media: v4l: subdev: Support INTERNAL_SOURCE pads in routing IOCTLs Message-ID: References: <20230505215257.60704-1-sakari.ailus@linux.intel.com> <20230505215257.60704-3-sakari.ailus@linux.intel.com> <691a9d35-218f-4eef-ddb0-5834f1e222c8@ideasonboard.com> <20230602094407.GF19463@pendragon.ideasonboard.com> <20230602094650.GG19463@pendragon.ideasonboard.com> <20230604142626.GF7754@pendragon.ideasonboard.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230604142626.GF7754@pendragon.ideasonboard.com> Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hi Laurent, On Sun, Jun 04, 2023 at 05:26:26PM +0300, Laurent Pinchart wrote: > Hi Sakari, > > On Fri, Jun 02, 2023 at 01:10:40PM +0000, Sakari Ailus wrote: > > On Fri, Jun 02, 2023 at 12:46:50PM +0300, Laurent Pinchart wrote: > > > On Fri, Jun 02, 2023 at 12:44:12PM +0300, Laurent Pinchart wrote: > > > > On Mon, May 08, 2023 at 03:24:10PM +0300, Sakari Ailus wrote: > > > > > On Mon, May 08, 2023 at 01:14:07PM +0300, Tomi Valkeinen wrote: > > > > > > On 06/05/2023 00:52, Sakari Ailus wrote: > > > > > > > Take the new INTERNAL_SOURCE pad flag into account in validating routing > > > > > > > IOCTL argument. Effectively this is a SINK pad in this respect. Due to the > > > > > > > union there's no need to check for the particular name. > > > > > > > > > > > > What union? The one you add in the next patch? > > > > > > > > > > Oops. I think we can reorder the patches. > > > > > > > > > > > As a concept the internal source pads sound good, and they work in practice > > > > > > in my tests. But this union is what grates me a bit. We have a flag, > > > > > > MEDIA_PAD_FL_INTERNAL_SOURCE, which tells which field of the union to use, > > > > > > and then we go and do not use the new union field. Well, and also the > > > > > > naming, as we normally have source and sink pads, but now we also have > > > > > > internal source pads, which are actually identical to sink pads... > > > > > > > > > > The union still should be used by the user space. We're testing flags here > > > > > and those flags are the same independently of the INTERNAL_SOURCE flag. > > > > > > > > > > I'm fine by not adding that union though, but for the user space I think > > > > > it's better we have it: explaining that the sink_pad has a different > > > > > meaning if that flag is set is harder than making it a union member. > > > > > > > > > > > I understand the idea and reasoning, but the two points above do confuse me > > > > > > and I'm sure would confuse others also. > > > > > > > > > > > > I wonder if it would be less or more confusing to simplify this by just > > > > > > adding a MEDIA_PAD_FL_INTERNAL, which could be applied to either a source or > > > > > > a sink pad, and would prevent linking to it. The flag would indicate that > > > > > > the stream from/to that pad is generated/consumed internally. (I don't know > > > > > > when you would need an internal pad to consume data, but... who knows, the > > > > > > need might pop up =). > > > > > > > > > > This is another option. But I envision there will be more compatibility > > > > > issues. Although... generally using such hardware requires knowledge of the > > > > > device, and we haven't obviously had any support for devices needing this > > > > > functionality in the tree. So in the end it might not matter much. > > > > > > > > > > > That would mean that an "internal sink pad" would generate a data stream, > > > > > > i.e. it's a "source", but I think a normal sink pad is almost the same > > > > > > anyway: when considering entity's internal routing, the normal sink pad is a > > > > > > "source", and the same logic would apply with the internal pads too. > > > > > > > > > > > > An internal sink pad that generates a stream is a bit more confusing than an > > > > > > internal source pad, but I think that confusion is less than the rest of the > > > > > > confusion in the code-side that comes with the internal source pads. > > > > > > > > > > I prefer having the possible sources of the confusion in the framework than > > > > > in the drivers. Therefore I think INTERNAL_SOURCE flag is a (slightly) > > > > > better option. > > > > > > > > > > I wonder what Llaurent thinks. > > > > > > > > I like the idea of a MEDIA_PAD_FL_INTERNAL flag. That's actually how I > > > > read patch 1/7, I didn't notice it used MEDIA_PAD_FL_INTERNAL*_SOURCE* > > > > :-) > > > > > > > > We could define MEDIA_PAD_FL_INTERNAL_SOURCE > > > > > > > > #define MEDIA_PAD_FL_INTERNAL_SOURCE (MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_INTERNAL) > > > > > > One option would be to find terms different from sink and source in this > > > case. It would generate less confusion to map (e.g., not a really good > > > name) MEDIA_PAD_FL_INTERNAL_PRODUCER to MEDIA_PAD_FL_SINK and to the > > > sink_pad field than using MEDIA_PAD_FL_INTERNAL_SOURCE. > > > > I don't think this helps as you'd still be accessing the sink pad related > > fields that are there for another purpose. > > > > Alternatively I'd have the (plain) INTERNAL flag and drop the union, > > without masking or adding compound flags. > > > > I can switch to that if you promise not to change your mind again. ;-) > > I'll do my best :-) Could you show the impact (if any) it would have on > drivers when accessing the routing fields ? I don't think there's much of an impact for the drivers. It's mainly affecting setting up pads for the entities. Tomi? -- Sakari Ailus