Hi Paul,
Thank you for the patch.
On Mon, Sep 09, 2024 at 05:48:28PM +0200, Paul Elder wrote:
> The current order of locking between the driver mutex and the v4l2
> subdev state lock causes a circuluar locking dependency when trying to
> set up a link. Fix this.
>
> Signed-off-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
This being said, I think we should deprecate the video-mux driver and
implement a driver that uses the V4L2 subdev internal routing API
instead of basing the routing configuration on link setup. Any opinion ?
> ---
> drivers/media/platform/video-mux.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c
> index 31e9e92e723eb..f43849db56800 100644
> --- a/drivers/media/platform/video-mux.c
> +++ b/drivers/media/platform/video-mux.c
> @@ -52,6 +52,7 @@ static int video_mux_link_setup(struct media_entity *entity,
> const struct media_pad *remote, u32 flags)
> {
> struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> + struct v4l2_subdev_state *sd_state;
> struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
> u16 source_pad = entity->num_pads - 1;
> int ret = 0;
> @@ -67,10 +68,10 @@ static int video_mux_link_setup(struct media_entity *entity,
> remote->entity->name, remote->index, local->entity->name,
> local->index, flags & MEDIA_LNK_FL_ENABLED);
>
> + sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> mutex_lock(&vmux->lock);
>
> if (flags & MEDIA_LNK_FL_ENABLED) {
> - struct v4l2_subdev_state *sd_state;
> struct v4l2_mbus_framefmt *source_mbusformat;
>
> if (vmux->active == local->index)
> @@ -88,12 +89,10 @@ static int video_mux_link_setup(struct media_entity *entity,
> vmux->active = local->index;
>
> /* Propagate the active format to the source */
> - sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> source_mbusformat = v4l2_subdev_state_get_format(sd_state,
> source_pad);
> *source_mbusformat = *v4l2_subdev_state_get_format(sd_state,
> vmux->active);
> - v4l2_subdev_unlock_state(sd_state);
> } else {
> if (vmux->active != local->index)
> goto out;
> @@ -105,6 +104,7 @@ static int video_mux_link_setup(struct media_entity *entity,
>
> out:
> mutex_unlock(&vmux->lock);
> + v4l2_subdev_unlock_state(sd_state);
> return ret;
> }
>
--
Regards,
Laurent Pinchart
