sailus: Should the return value of v4l2_ctrl_new_fwnode_properties() be checked or only the ctrl_handler->error be checked and is sufficient ? v4l2_ctrl_new_fwnode_properties() can however return -EINVAL too... but should it block probing the device on v4l2m2m: how come the job is finished after the buffers are submitted to the mmal firmware (for the bcm2835-codec driver); shouldn't the job only be finished after the mmal callbacks confirm that it's actually finished? also: how is device_run called? is it only called if there is input data? it seems that it is called until all of the src/dst buffers are used? and so i don't get why there are mmal submissions with no src buffer, and only dst buffer? i feel like i should prevent submission if there is only src or only dst? in bcm2!35 the device_run always ends and has finished the job, but should i return without finishing the job so it would not call it constantly if there is no input yet? also; on the first callback there is usually a format_change command, but i do notice that it exits before doing a vb2_buf_done() ? that seems wrong too? nvm the last one, it seems during format_change the mmal output callback actually gives a "buffer" that is not a buffer in a queue at all... uajain: Checking for handler's error is enough IMO. alien_lappy: hi, nice to see you are still looking into this upstreaming work Regarding the jobs and stateful drivers, the implementation varies lot between drivers Samsung MFC simply does not use that framework, Venus keeps the job active from the first buffer till the EOS, and wave5 (just to give few examples) binds the job to the period of time the firmware is active The last is mostly the exception, since normally you want to make use of the firmware processing queues, and not wait for each operations to finish before submitting new ones So I think brcm method is as valid as any other The pm management becomes less obvious though For when device_run is called, by default it's called when there is a available buffer on both queues, but stateful decoders need to start running without the capture queue, so they always have some special case, they may use buffered mode to skip the available checked. This is needed when you have a ring buffer notably. If you end up with device_run being called too often, you may want to implement the job_ready callback to refine it Please note this callback is called with the job spinlock, which creates complications if you have data protected by a mutex ndufresne: thx for the info, while fixing kernel segfault, I ended up having an issue that the output queue (20 buffers) don't seem to be handled nice, and after 20 buffers are processed the whole thing stops working, as if there is no free buffer anymore, there is a job_ready override, so i think i need to see that ndufresne: so, after your email, I have identified 21 issues that need looking after, and I already have a fulltime job, so i can't actually put much time into this, tbh: at this point, I think I could progress way faster if I could be fulltime on this, instead of my current job So device_run does a input buffer and output buffer submission to fw and there is a callback, and in the callback the buf_done() is called, but the job finish is at the end of the device_run (which means the callbacks and thus done() function is called later than the job_finish... I thought the job_finish would be followed by the user-space using the buffers to actually display it (decoding), but how does that work if the buf_done() is called later? or, is buf_done() on the output buffers (without job consideration) used to push the data to the user space to display it? alien_lappy: I believe the last remark is correct, it's the buf_done calls that drive the userspace with stateful codec ndufresne: thx