Hi,
On 16/09/2021 16:07, Jacopo Mondi wrote:
Also note that operations like s_stream do not take a state as
parameter. The driver has to fetch it from the subdev anyway
(this in reply to the idea of having the active state as parameter vs
retrieving it from the subdev if ACTIVE)
While porting the R-Car drivers on top of this series I found myself
in the need to (in the s_stream call chain)
static int rcsi2_start_receiver(struct rcar_csi2 *priv)
{
const struct v4l2_subdev_state *state = priv->subdev.state;
const struct v4l2_subdev_stream_configs *configs = &state->stream_configs;
...
/*
* Configure field handling inspecting the formats of the
* single sink pad streams.
*/
for (i = 0; i < configs->num_configs; ++i) {
const struct v4l2_subdev_stream_config *config = configs->configs;
if (config->pad != RCAR_CSI2_SINK)
continue;
if (config->fmt.field != V4L2_FIELD_ALTERNATE)
continue;
fld |= FLD_DET_SEL(1);
fld |= FLD_FLD_EN(config->stream);
/* PAL vs NTSC. */
if (config->fmt.height == 240)
fld |= FLD_FLD_NUM(0);
else
fld |= FLD_FLD_NUM(1);
}
...
}
Am I doing it wrong, or is this a case for the subdev to have to
directly access sd->state ?
In s_stream path you should:
state = v4l2_subdev_lock_active_state(sd);
<do the work with the state>
v4l2_subdev_unlock_state(state);
If you already have the state, e.g. in set_fmt:
state = v4l2_subdev_validate_and_lock_state(sd, state);
<do the work with the state>
v4l2_subdev_unlock_state(state);
Accessing the stream_configs directly is fine but not that nice. I did
think about some helpers, perhaps for_each_stream_config(), but I didn't
add that as I didn't have the need.
There's v4l2_state_get_stream_format() which can be used in many cases,
but we probably need something else if you need to iterate over all the
configs.
Tomi
