Re: [PATCH] media: stm32: dcmi: Switch to __v4l2_subdev_state_alloc()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux