Hi, On 15/09/2021 12:44, Jacopo Mondi wrote:
Hi Tomi, On Mon, Aug 30, 2021 at 02:00:42PM +0300, Tomi Valkeinen wrote:Add a new 'state' field to struct v4l2_subdev to which we can store the active state of a subdev. This will place the subdev configuration into a known place, allowing us to use the state directly from the v4l2 framework, thus simplifying the drivers. We also add v4l2_subdev_alloc_state() and v4l2_subdev_free_state(), which need to be used by the drivers that support subdev state in struct v4l2_subdev. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx> --- drivers/media/v4l2-core/v4l2-subdev.c | 21 ++++++++++++++++ include/media/v4l2-subdev.h | 36 +++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 26a34a8e3d37..e1a794f69815 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -943,3 +943,24 @@ void v4l2_subdev_notify_event(struct v4l2_subdev *sd, v4l2_subdev_notify(sd, V4L2_DEVICE_NOTIFY_EVENT, (void *)ev); } EXPORT_SYMBOL_GPL(v4l2_subdev_notify_event); + +int v4l2_subdev_alloc_state(struct v4l2_subdev *sd) +{ + struct v4l2_subdev_state *state; + + state = v4l2_alloc_subdev_state(sd);So, I think this is one source of confusion about init_cfg. v4l2_alloc_subdev_state() calls init_cfg() and 'state-aware' driver are now supposed to allocate their state by calling v4l2_subdev_alloc_state(), in the same way as the core does for the file-handle ones. This will lead to init_cfg to be called for the 'active' (ie owned by the subdev) state, and then you need to add context to the state (by adding a 'which' field) to know what state you're dealing with. According to the init_cfg() documentation * @init_cfg: initialize the pad config to default values the op has to be called in order to initialize the per-file-handle context, not the active one.
I have missed updating the documentation there =).
I would rather just embed 'struct v4l2_subdev_state' in 'struct v4l2_subdev', have the core going through the
Why would embedding the state change anything?
'v4l2_subdev_alloc_state()' to initialize the per-fh state, but have drivers initialize their own state at probe time. If they need for some reason to access their 'active' state at init_cfg() time, they caan fish it from their subdev. If I'm not mistaken this will remove the need to have a which filed in the state, as I think the 'context' should be inferred from the 'which' argument embedded in the ops-specific structures, and not held in the state itself.
I'll comment on these in the next reply, as your second mail covered the same topics.
Tomi
