Re: [PATCH v2 00/17] Rework frame descriptors
Sakari Ailus <[email protected]>
| Newsgroups | org.kernel.vger.linux-media |
|---|---|
| Organization | Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo |
| Message-ID | <[email protected]> |
Hi Mattijs, Tomi, On Fri, Aug 14, 2026 at 11:26:23AM +0200, Mattijs Korpershoek wrote: > Hi Tomi, > > On Fri, Aug 14, 2026 at 11:21, Tomi Valkeinen <[email protected]> wrote: > > > Hi, > > > > On 14/08/2026 11:17, Mattijs Korpershoek wrote: > >> Hi Sakari, > >> > >> Thank you for the series. > >> > >> On Mon, May 18, 2026 at 19:43, Sakari Ailus <[email protected]> wrote: > >> > >>> Hi folks, > >>> > >>> This smallish set makes frame descriptors dynamically allocated and > >>> implements a single-entry frame descriptor based on the device's format, > >>> using a new helper called v4l2_subdev_get_frame_desc(). All drivers that > >>> do not obtain their frame descriptor from upstream are converted. The > >>> helper also obtains a frame descriptor for the desired type (parallel or > >>> CSI-2) and checks there's at least one entry there. These checks are > >>> removed from drivers that currently perform them. (Some drivers also check > >>> there's exactly a single frame descriptor entry but I think in most cases > >>> this check could be loosened. That could be done after this set.) > >>> > >>> On callee side these patches introduce no changes as the number of > >>> pre-allocated memory for 8 frame descriptors remains as-is. The > >>> get_frame_desc() pad op can return more than 8 frame descriptors by > >>> setting the num_entries to the desired number and returning -ENOSPC. > >>> > >>> If people prefer using cleanup.h / __free() to release the dynamically > >>> allocated array (I think I'd almost require that), I'll merge the > >>> now-separate __v4l2_subdev_get_frame_desc() into > >>> v4l2_subdev_get_frame_desc(). > >>> > >>> More formats can be added to df-to-mbus conversion as needed. These are > >>> meant to be initial formats that are enough for typical raw sensors (and > >>> one RGB format, too). > >> > >> I've tried this out on a AM69-SK with the Arducam FPD V3Link[1] using > >> the following device tree overlays: > >> ti/k3-am68-sk-v3link-fusion.dtbo ti/k3-v3link-imx219-0-0.dtbo > >> > >> See TI's documentation about this [2] > >> > >> I (naively) assumed that this series would replace Tomi's patch [3], but > >> it did not. I see the following in dmesg: > >> > >> [ 286.686574] cdns-csi2rx 4504000.csi-bridge: collect_streams: "cdns_csi2rx.4504000.csi-bridge":1: found 0x1 enabled 0x0 > >> [ 286.686754] ds90ub953 7-0044: Failed to get frame desc from remote subdev imx219 10-0010 > >> [ 286.700147] ds90ub960 7-0030: Failed to get source frame desc for pad 0 > >> [ 286.712679] j721e-csi2rx 4500000.ticsi2rx: enable streams "ds90ub960 7-0030":4/0x1 > >> [ 286.712684] ds90ub960 7-0030: collect_streams: "ds90ub960 7-0030":4: found 0x1 enabled 0x0 > >> [ 286.712690] ds90ub953 7-0044: Failed to get frame desc from remote subdev imx219 10-0010 > >> [ 286.725884] j721e-csi2rx 4500000.ticsi2rx: enable streams 4:0x1 failed: -515 > >> > >> Here is my camera topology: > >> https://paste.debian.net/hidden/7f56f635 > >> > >> I also made the following patch to attempt to convert over j721e-csi2rx: > >> https://paste.debian.net/hidden/314f9c32 > >> > >> Is this series indeed aimed to replace all sensor-specific > >> implementations of .get_frame_desc() or are patches such as the one send > >> from Tomi [3] still useful? > > I don't remember the details anymore, but probably related to my comment > > in this thread: > > > > "It also looks like you only modified platform drivers. Did you check > > the i2c drivers? Some call get_frame_desc().". So I think ub953 is > > missing the conversion to v4l2_subdev_get_frame_desc(). > > Thanks for the hint. > > ub953 and ub960 (which I both use) indirectly call .get_frame_desc() via > v4l2_subdev_get_frame_desc_passthrough(). > > So maybe v4l2_subdev_get_frame_desc_passthrough() needs an update as > well in this series. Using v4l2_subdev_call() is still ok as such but it won't be able to return more routes than it used to. I've made some changes since which I have pushed to my frame-desc branch in my linuxtv.org (and FDo) trees but I'm not sure if these address the issue. -- Regards, Sakari Ailus