On Mon, Jun 20, 2022 at 12:44:02PM +0300, Tomi Valkeinen wrote: > On 19/06/2022 02:16, Laurent Pinchart wrote: > > Hi Marek, > > > > CC'ing Tomi to get his opinion. > > > > On Sun, Jun 19, 2022 at 12:24:42AM +0200, Marek Vasut wrote: > >> Any local subdev state should be allocated and free'd using > >> __v4l2_subdev_state_alloc()/__v4l2_subdev_state_free(), which > >> takes care of calling .init_cfg() subdev op. Without this, > >> subdev internal state might be uninitialized by the time > >> any other subdev op is called. > > Does this fix a bug you have? Wasn't this broken even before the active > state, as init_cfg was not called? > > In any case, I think we have to do something like this, as the source > subdev might depend on a valid subdev state. > > It's not very nice to have the drivers using __v4l2_subdev_state_alloc, > though. But if non-MC drivers are not going away, You know my opinion on this :-) We shouldn't have any new user of __v4l2_subdev_state_alloc() in new drivers, but fixing issues in existing drivers is a valid use case. I'd like if the dcmi driver could be converted to be MC-centric, but that will likely not happen. > and if they are going > to be calling ops in other subdevs with V4L2_SUBDEV_FORMAT_TRY, they > need to pass a valid subdev state... > > I don't see a better way right away, so I think this is fine. > > Do we need a v4l2_subdev_call_state_try(), similar to > v4l2_subdev_call_state_active()? It would handle allocating the state, > calling the op and freeing the state. I could imagine the user trying multiple operations on the same state, but maybe a single call is a common enough use case to implement a dedicated helper ? -- Regards, Laurent Pinchart
