On Mon, Oct 23, 2023 at 04:29:04PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Mon, Oct 23, 2023 at 03:33:03PM +0300, Sakari Ailus wrote:
> > There are two sets of functions that return information from sub-device
> > state, one for stream-unaware users and another for stream-aware users.
> > Add support for stream-aware functions to return format, crop and compose
> > information from pad-based array that are functionally equivalent to the
> > old, stream-unaware ones.
> >
> > Also check state is non-NULL, in order to guard against old drivers
> > potentially calling this with NULL state for active formats or selection
> > rectangles.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> > drivers/media/v4l2-core/v4l2-subdev.c | 39 +++++++++++++++++++++++++++
> > 1 file changed, 39 insertions(+)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > index ee4fe8f33a41..955ee9a6c91f 100644
> > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > @@ -1684,6 +1684,19 @@ v4l2_subdev_state_get_stream_format(struct v4l2_subdev_state *state,
> > struct v4l2_subdev_stream_configs *stream_configs;
> > unsigned int i;
> >
> > + if (WARN_ON(!state))
> > + return NULL;
> > +
> > + if (state->pads) {
> > + if (stream)
> > + return NULL;
> > +
> > + if (WARN_ON(pad >= state->sd->entity.num_pads))
There's no sd field in struct v4l2_subdev_state in the linux media
master branch, no mention of dependencies in the cover letter, and no
specified base.
Please generate patch series with --base.
> > + pad = 0;
> > +
> > + return &state->pads[pad].try_fmt;
> > + }
> > +
> > lockdep_assert_held(state->lock);
>
> Can we move towards proper locking for all callers ?
>
> >
> > stream_configs = &state->stream_configs;
> > @@ -1705,6 +1718,19 @@ v4l2_subdev_state_get_stream_crop(struct v4l2_subdev_state *state,
> > struct v4l2_subdev_stream_configs *stream_configs;
> > unsigned int i;
> >
> > + if (WARN_ON(!state))
> > + return NULL;
> > +
> > + if (state->pads) {
> > + if (stream)
> > + return NULL;
> > +
> > + if (WARN_ON(pad >= state->sd->entity.num_pads))
> > + pad = 0;
> > +
> > + return &state->pads[pad].try_crop;
> > + }
> > +
> > lockdep_assert_held(state->lock);
> >
> > stream_configs = &state->stream_configs;
> > @@ -1726,6 +1752,19 @@ v4l2_subdev_state_get_stream_compose(struct v4l2_subdev_state *state,
> > struct v4l2_subdev_stream_configs *stream_configs;
> > unsigned int i;
> >
> > + if (WARN_ON(!state))
> > + return NULL;
> > +
> > + if (state->pads) {
> > + if (stream)
> > + return NULL;
> > +
> > + if (WARN_ON(pad >= state->sd->entity.num_pads))
> > + pad = 0;
> > +
> > + return &state->pads[pad].try_compose;
> > + }
> > +
> > lockdep_assert_held(state->lock);
> >
> > stream_configs = &state->stream_configs;
--
Regards,
Laurent Pinchart
