From patchwork Tue Apr 12 09:42:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 12810447 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1D22CC433F5 for ; Tue, 12 Apr 2022 10:45:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229895AbiDLKrN (ORCPT ); Tue, 12 Apr 2022 06:47:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54490 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1356952AbiDLKpq (ORCPT ); Tue, 12 Apr 2022 06:45:46 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6ABC55C849 for ; Tue, 12 Apr 2022 02:43:21 -0700 (PDT) Received: from deskari.lan (91-156-85-209.elisa-laajakaista.fi [91.156.85.209]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 19351494; Tue, 12 Apr 2022 11:43:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1649756599; bh=1I/lb2foFvhgtcFvLkOH7c5wTPWxLZSvgXE5f+z3D5k=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=eusrSKDdqsnzf5TuOpSMHFyYYGttk7sr8YYj0Iw3YZR3kb0B/PgJ82XkeMjSWVmvf WQHuQFfS4jRYHRjw1etq3CMfcO+dDgIFFT+L/riRSSnerni1IXy6svFKurE+/ZKkyN vHpoDyMKwfPGL0YqcTw/Xnoc1bTiw4YUhm/KpGyA= From: Tomi Valkeinen To: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com, Jacopo Mondi , Laurent Pinchart , niklas.soderlund+renesas@ragnatech.se, Mauro Carvalho Chehab , Hans Verkuil , Pratyush Yadav , satish.nagireddy@getcruise.com Cc: Tomi Valkeinen Subject: [PATCH v8 01/10] media: v4l2-subdev: fix #endif comments Date: Tue, 12 Apr 2022 12:42:40 +0300 Message-Id: <20220412094249.695754-2-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> References: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Add comments after #endifs to clarify their scope. Signed-off-by: Tomi Valkeinen --- include/media/v4l2-subdev.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 6c153b33bb04..a986fdd652e6 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1023,7 +1023,7 @@ v4l2_subdev_get_try_compose(struct v4l2_subdev *sd, return &state->pads[pad].try_compose; } -#endif +#endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ extern const struct v4l2_file_operations v4l2_subdev_fops; @@ -1204,4 +1204,4 @@ extern const struct v4l2_subdev_ops v4l2_subdev_call_wrappers; void v4l2_subdev_notify_event(struct v4l2_subdev *sd, const struct v4l2_event *ev); -#endif +#endif /* _V4L2_SUBDEV_H */ From patchwork Tue Apr 12 09:42:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 12810451 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 53525C43217 for ; Tue, 12 Apr 2022 10:46:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346287AbiDLKrs (ORCPT ); Tue, 12 Apr 2022 06:47:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54072 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1356954AbiDLKpq (ORCPT ); Tue, 12 Apr 2022 06:45:46 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1C42A5C854 for ; Tue, 12 Apr 2022 02:43:24 -0700 (PDT) Received: from deskari.lan (91-156-85-209.elisa-laajakaista.fi [91.156.85.209]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id ECC8C59D; Tue, 12 Apr 2022 11:43:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1649756600; bh=8/hMOYSDOUiBStvv5ckoI7AROItkWeotddm3pjO+4cM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=nQWDEb0r/PHsNzLnhEoPOFDqW1ONCU4lEYwM3D9wxF7geNk7xph1Jm1np3tBT4dtO 15i7pVs+CfXVd8DXv9muDJkJ4KggxVnwulLoF85Q4zrAEEwQvwWo1Mq7be8rw2nig2 NMBRLvbA7YhtrJ+tLOmHEc4zkzSBVgPtJXv865Q4= From: Tomi Valkeinen To: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com, Jacopo Mondi , Laurent Pinchart , niklas.soderlund+renesas@ragnatech.se, Mauro Carvalho Chehab , Hans Verkuil , Pratyush Yadav , satish.nagireddy@getcruise.com Cc: Tomi Valkeinen Subject: [PATCH v8 02/10] media: v4l2-subdev: drop extra #ifdef Date: Tue, 12 Apr 2022 12:42:41 +0300 Message-Id: <20220412094249.695754-3-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> References: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org subdev_open() is inside #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API, which depends on CONFIG_MEDIA_CONTROLLER, so there's no need for an extra Signed-off-by: Tomi Valkeinen --- drivers/media/v4l2-core/v4l2-subdev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 30eb50407db5..2f24ef75872b 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -63,7 +63,7 @@ static int subdev_open(struct file *file) v4l2_fh_init(&subdev_fh->vfh, vdev); v4l2_fh_add(&subdev_fh->vfh); file->private_data = &subdev_fh->vfh; -#if defined(CONFIG_MEDIA_CONTROLLER) + if (sd->v4l2_dev->mdev && sd->entity.graph_obj.mdev->dev) { struct module *owner; @@ -74,7 +74,6 @@ static int subdev_open(struct file *file) } subdev_fh->owner = owner; } -#endif if (sd->internal_ops && sd->internal_ops->open) { ret = sd->internal_ops->open(sd, subdev_fh); From patchwork Tue Apr 12 09:42:42 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 12810475 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E7C3EC433F5 for ; Tue, 12 Apr 2022 10:55:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1377323AbiDLK4g (ORCPT ); Tue, 12 Apr 2022 06:56:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53720 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1356957AbiDLKpr (ORCPT ); Tue, 12 Apr 2022 06:45:47 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A5A685C866 for ; Tue, 12 Apr 2022 02:43:24 -0700 (PDT) Received: from deskari.lan (91-156-85-209.elisa-laajakaista.fi [91.156.85.209]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D481F5A5; Tue, 12 Apr 2022 11:43:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1649756601; bh=hvlo3VIhJjNcsxH80uaudw4/VRETr0aO6rCocmqiobU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=pInSs0D+ql29mvLZAOrvQiOsDRzeFSNbpy1YaCPrNU0NoWHKkKmg17m2TLinq/FM6 q/VE87Y1jAis0EuMmGLbhWlhzqBpViITcRnLK7FRBsXL3/I9xicdel5Y5QY880tjK2 ZzKlMX/PojsEh86wXUo0YOCM+QNPg1H56j/f6oR8= From: Tomi Valkeinen To: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com, Jacopo Mondi , Laurent Pinchart , niklas.soderlund+renesas@ragnatech.se, Mauro Carvalho Chehab , Hans Verkuil , Pratyush Yadav , satish.nagireddy@getcruise.com Cc: Tomi Valkeinen , Jacopo Mondi Subject: [PATCH v8 03/10] media: subdev: rename subdev-state alloc & free Date: Tue, 12 Apr 2022 12:42:42 +0300 Message-Id: <20220412094249.695754-4-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> References: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org v4l2_subdev_alloc_state() and v4l2_subdev_free_state() are not supposed to be used by the drivers. However, we do have a few drivers that use those at the moment, so we need to expose these functions for the time being. Prefix the functions with __ to mark the functions as internal. At the same time, rename them to v4l2_subdev_state_alloc and v4l2_subdev_state_free to match the style used for other functions like video_device_alloc() and media_request_alloc(). Signed-off-by: Tomi Valkeinen Reviewed-by: Laurent Pinchart Reviewed-by: Hans Verkuil Reviewed-by: Jacopo Mondi --- .../media/platform/renesas/rcar-vin/rcar-v4l2.c | 8 ++++++-- drivers/media/platform/renesas/vsp1/vsp1_entity.c | 8 ++++++-- drivers/media/v4l2-core/v4l2-subdev.c | 12 ++++++------ drivers/staging/media/tegra-video/vi.c | 8 ++++++-- include/media/v4l2-subdev.h | 14 +++++++++----- 5 files changed, 33 insertions(+), 17 deletions(-) diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c index 2e60b9fce03b..da88f968c31a 100644 --- a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c @@ -263,7 +263,11 @@ static int rvin_try_format(struct rvin_dev *vin, u32 which, u32 width, height; int ret; - sd_state = v4l2_subdev_alloc_state(sd); + /* + * FIXME: Drop this call, drivers are not supposed to use + * __v4l2_subdev_state_alloc(). + */ + sd_state = __v4l2_subdev_state_alloc(sd); if (IS_ERR(sd_state)) return PTR_ERR(sd_state); @@ -299,7 +303,7 @@ static int rvin_try_format(struct rvin_dev *vin, u32 which, rvin_format_align(vin, pix); done: - v4l2_subdev_free_state(sd_state); + __v4l2_subdev_state_free(sd_state); return ret; } diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c index 823c15facd1b..c82b3fb7b89a 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c @@ -675,7 +675,11 @@ int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity *entity, * Allocate the pad configuration to store formats and selection * rectangles. */ - entity->config = v4l2_subdev_alloc_state(&entity->subdev); + /* + * FIXME: Drop this call, drivers are not supposed to use + * __v4l2_subdev_state_alloc(). + */ + entity->config = __v4l2_subdev_state_alloc(&entity->subdev); if (IS_ERR(entity->config)) { media_entity_cleanup(&entity->subdev.entity); return PTR_ERR(entity->config); @@ -690,6 +694,6 @@ void vsp1_entity_destroy(struct vsp1_entity *entity) entity->ops->destroy(entity); if (entity->subdev.ctrl_handler) v4l2_ctrl_handler_free(entity->subdev.ctrl_handler); - v4l2_subdev_free_state(entity->config); + __v4l2_subdev_state_free(entity->config); media_entity_cleanup(&entity->subdev.entity); } diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 2f24ef75872b..a1494999352b 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -28,7 +28,7 @@ static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd) { struct v4l2_subdev_state *state; - state = v4l2_subdev_alloc_state(sd); + state = __v4l2_subdev_state_alloc(sd); if (IS_ERR(state)) return PTR_ERR(state); @@ -39,7 +39,7 @@ static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd) static void subdev_fh_free(struct v4l2_subdev_fh *fh) { - v4l2_subdev_free_state(fh->state); + __v4l2_subdev_state_free(fh->state); fh->state = NULL; } @@ -861,7 +861,7 @@ int v4l2_subdev_link_validate(struct media_link *link) } EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate); -struct v4l2_subdev_state *v4l2_subdev_alloc_state(struct v4l2_subdev *sd) +struct v4l2_subdev_state *__v4l2_subdev_state_alloc(struct v4l2_subdev *sd) { struct v4l2_subdev_state *state; int ret; @@ -894,9 +894,9 @@ struct v4l2_subdev_state *v4l2_subdev_alloc_state(struct v4l2_subdev *sd) return ERR_PTR(ret); } -EXPORT_SYMBOL_GPL(v4l2_subdev_alloc_state); +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_alloc); -void v4l2_subdev_free_state(struct v4l2_subdev_state *state) +void __v4l2_subdev_state_free(struct v4l2_subdev_state *state) { if (!state) return; @@ -904,7 +904,7 @@ void v4l2_subdev_free_state(struct v4l2_subdev_state *state) kvfree(state->pads); kfree(state); } -EXPORT_SYMBOL_GPL(v4l2_subdev_free_state); +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_free); #endif /* CONFIG_MEDIA_CONTROLLER */ diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index d1f43f465c22..07d368f345cd 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -507,7 +507,11 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan, if (!subdev) return -ENODEV; - sd_state = v4l2_subdev_alloc_state(subdev); + /* + * FIXME: Drop this call, drivers are not supposed to use + * __v4l2_subdev_state_alloc(). + */ + sd_state = __v4l2_subdev_state_alloc(subdev); if (IS_ERR(sd_state)) return PTR_ERR(sd_state); /* @@ -558,7 +562,7 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan, v4l2_fill_pix_format(pix, &fmt.format); tegra_channel_fmt_align(chan, pix, fmtinfo->bpp); - v4l2_subdev_free_state(sd_state); + __v4l2_subdev_state_free(sd_state); return 0; } diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index a986fdd652e6..ac3bcc54ea07 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1122,20 +1122,24 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd, int v4l2_subdev_link_validate(struct media_link *link); /** - * v4l2_subdev_alloc_state - allocate v4l2_subdev_state + * __v4l2_subdev_state_alloc - allocate v4l2_subdev_state * * @sd: pointer to &struct v4l2_subdev for which the state is being allocated. * - * Must call v4l2_subdev_free_state() when state is no longer needed. + * Must call __v4l2_subdev_state_free() when state is no longer needed. + * + * Not to be called directly by the drivers. */ -struct v4l2_subdev_state *v4l2_subdev_alloc_state(struct v4l2_subdev *sd); +struct v4l2_subdev_state *__v4l2_subdev_state_alloc(struct v4l2_subdev *sd); /** - * v4l2_subdev_free_state - free a v4l2_subdev_state + * __v4l2_subdev_state_free - free a v4l2_subdev_state * * @state: v4l2_subdev_state to be freed. + * + * Not to be called directly by the drivers. */ -void v4l2_subdev_free_state(struct v4l2_subdev_state *state); +void __v4l2_subdev_state_free(struct v4l2_subdev_state *state); #endif /* CONFIG_MEDIA_CONTROLLER */ From patchwork Tue Apr 12 09:42:43 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 12810453 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E650BC4332F for ; Tue, 12 Apr 2022 10:46:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1352276AbiDLKsF (ORCPT ); Tue, 12 Apr 2022 06:48:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52226 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1356964AbiDLKpr (ORCPT ); Tue, 12 Apr 2022 06:45:47 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1A43A5F4EF for ; Tue, 12 Apr 2022 02:43:26 -0700 (PDT) Received: from deskari.lan (91-156-85-209.elisa-laajakaista.fi [91.156.85.209]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C6871741; Tue, 12 Apr 2022 11:43:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1649756602; bh=ycZ+9EvrhKxvhcLD7FGGkgrdRszuXNJkAc77xz5cgWQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vCSOiN42dOfDfol42BhfmM9qrq1fN7lrT4qrcPhjMTyObXOW/l/eljrerphCEW6B8 0aXzbBHpU8GnuqnmvSytbskNrG9tExcMRV+FD2oO8IBTfXNPJCSmp64Y2DCTB1ijKf 1ajR/25Df9qYAZCkJEScoRkkjcE1sLRJwnPy3C7M= From: Tomi Valkeinen To: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com, Jacopo Mondi , Laurent Pinchart , niklas.soderlund+renesas@ragnatech.se, Mauro Carvalho Chehab , Hans Verkuil , Pratyush Yadav , satish.nagireddy@getcruise.com Cc: Tomi Valkeinen Subject: [PATCH v8 04/10] media: subdev: add active state to struct v4l2_subdev Date: Tue, 12 Apr 2022 12:42:43 +0300 Message-Id: <20220412094249.695754-5-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> References: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Add a new 'active_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. Also add functions v4l2_subdev_init_finalize() and v4l2_subdev_cleanup(), which will allocate and free the active state. The functions are named in a generic way so that they can be also used for other subdev initialization work. Signed-off-by: Tomi Valkeinen Reviewed-by: Hans Verkuil Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- drivers/media/v4l2-core/v4l2-subdev.c | 21 ++++++++++ include/media/v4l2-subdev.h | 58 +++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index a1494999352b..5ac447b3038c 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -906,6 +906,27 @@ void __v4l2_subdev_state_free(struct v4l2_subdev_state *state) } EXPORT_SYMBOL_GPL(__v4l2_subdev_state_free); +int v4l2_subdev_init_finalize(struct v4l2_subdev *sd) +{ + struct v4l2_subdev_state *state; + + state = __v4l2_subdev_state_alloc(sd); + if (IS_ERR(state)) + return PTR_ERR(state); + + sd->active_state = state; + + return 0; +} +EXPORT_SYMBOL_GPL(v4l2_subdev_init_finalize); + +void v4l2_subdev_cleanup(struct v4l2_subdev *sd) +{ + __v4l2_subdev_state_free(sd->active_state); + sd->active_state = NULL; +} +EXPORT_SYMBOL_GPL(v4l2_subdev_cleanup); + #endif /* CONFIG_MEDIA_CONTROLLER */ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index ac3bcc54ea07..d0d0e4caf893 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -645,6 +645,9 @@ struct v4l2_subdev_ir_ops { * This structure only needs to be passed to the pad op if the 'which' field * of the main argument is set to %V4L2_SUBDEV_FORMAT_TRY. For * %V4L2_SUBDEV_FORMAT_ACTIVE it is safe to pass %NULL. + * + * Note: This struct is also used in active state, and the 'try' prefix is + * historical and to be removed. */ struct v4l2_subdev_pad_config { struct v4l2_mbus_framefmt try_fmt; @@ -885,6 +888,9 @@ struct v4l2_subdev_platform_data { * @subdev_notifier: A sub-device notifier implicitly registered for the sub- * device using v4l2_async_register_subdev_sensor(). * @pdata: common part of subdevice platform data + * @active_state: Active state for the subdev (NULL for subdevs tracking the + * state internally). Initialized by calling + * v4l2_subdev_init_finalize(). * * Each instance of a subdev driver should create this struct, either * stand-alone or embedded in a larger struct. @@ -916,6 +922,19 @@ struct v4l2_subdev { struct v4l2_async_notifier *notifier; struct v4l2_async_notifier *subdev_notifier; struct v4l2_subdev_platform_data *pdata; + + /* + * The fields below are private, and should only be accessed via + * appropriate functions. + */ + + /* + * TODO: active_state should most likely be changed from a pointer to an + * embedded field. For the time being it's kept as a pointer to more + * easily catch uses of active_state in the cases where the driver + * doesn't support it. + */ + struct v4l2_subdev_state *active_state; }; @@ -1141,6 +1160,45 @@ struct v4l2_subdev_state *__v4l2_subdev_state_alloc(struct v4l2_subdev *sd); */ void __v4l2_subdev_state_free(struct v4l2_subdev_state *state); +/** + * v4l2_subdev_init_finalize() - Finalizes the initialization of the subdevice + * @sd: The subdev + * + * This function finalizes the initialization of the subdev, including + * allocation of the active state for the subdev. + * + * This function must be called by the subdev drivers that use the centralized + * active state, after the subdev struct has been initialized and + * media_entity_pads_init() has been called, but before registering the + * subdev. + * + * The user must call v4l2_subdev_cleanup() when the subdev is being removed. + */ +int v4l2_subdev_init_finalize(struct v4l2_subdev *sd); + +/** + * v4l2_subdev_cleanup() - Releases the resources allocated by the subdevice + * @sd: The subdevice + * + * This function will release the resources allocated in + * v4l2_subdev_init_finalize. + */ +void v4l2_subdev_cleanup(struct v4l2_subdev *sd); + +/** + * v4l2_subdev_get_active_state() - Returns the active subdev state for + * subdevice + * @sd: The subdevice + * + * Returns the active state for the subdevice, or NULL if the subdev does not + * support active state. + */ +static inline struct v4l2_subdev_state * +v4l2_subdev_get_active_state(struct v4l2_subdev *sd) +{ + return sd->active_state; +} + #endif /* CONFIG_MEDIA_CONTROLLER */ /** From patchwork Tue Apr 12 09:42:44 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 12810450 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3AF72C4332F for ; Tue, 12 Apr 2022 10:46:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231406AbiDLKrf (ORCPT ); Tue, 12 Apr 2022 06:47:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51098 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1356974AbiDLKpr (ORCPT ); Tue, 12 Apr 2022 06:45:47 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C00545F4F5 for ; Tue, 12 Apr 2022 02:43:26 -0700 (PDT) Received: from deskari.lan (91-156-85-209.elisa-laajakaista.fi [91.156.85.209]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A3781BC0; Tue, 12 Apr 2022 11:43:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1649756603; bh=qaOo9fiqeiM+WyXXtsJlnoDbVc6Uq9lCpsCFmramfU8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ILs0fEOCL728Pyhym4wkcxycvf2Smtf3f7cSn/75jAJExLbkJQoy9ylkoJ03MMZro 4cexqwipBhOzsx9vkvXyRXlQFiaVibgZGaDwzVUioU+qmi+Z8L405Wv+s8XY+4UIKZ 4Dgolgu5pGUv8UIjkaxSRPA4Ct9KH0BDV8OwCZqA= From: Tomi Valkeinen To: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com, Jacopo Mondi , Laurent Pinchart , niklas.soderlund+renesas@ragnatech.se, Mauro Carvalho Chehab , Hans Verkuil , Pratyush Yadav , satish.nagireddy@getcruise.com Cc: Tomi Valkeinen Subject: [PATCH v8 05/10] media: subdev: rename v4l2_subdev_get_pad_* helpers Date: Tue, 12 Apr 2022 12:42:44 +0300 Message-Id: <20220412094249.695754-6-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> References: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org The subdev state is now used for both try and active cases. Rename rename v4l2_subdev_get_try_* helpers to v4l2_subdev_get_pad_*. Temporary wapper helper macros are added to keep the drivers using v4l2_subdev_get_try_* compiling. The next step is to change the uses in th drivers, and then drop the helpers. Signed-off-by: Tomi Valkeinen Reviewed-by: Hans Verkuil --- include/media/v4l2-subdev.h | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index d0d0e4caf893..eab8e4342613 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -989,7 +989,7 @@ struct v4l2_subdev_fh { #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) /** - * v4l2_subdev_get_try_format - ancillary routine to call + * v4l2_subdev_get_pad_format - ancillary routine to call * &struct v4l2_subdev_pad_config->try_fmt * * @sd: pointer to &struct v4l2_subdev @@ -997,7 +997,7 @@ struct v4l2_subdev_fh { * @pad: index of the pad in the &struct v4l2_subdev_state->pads array */ static inline struct v4l2_mbus_framefmt * -v4l2_subdev_get_try_format(struct v4l2_subdev *sd, +v4l2_subdev_get_pad_format(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, unsigned int pad) { @@ -1007,7 +1007,7 @@ v4l2_subdev_get_try_format(struct v4l2_subdev *sd, } /** - * v4l2_subdev_get_try_crop - ancillary routine to call + * v4l2_subdev_get_pad_crop - ancillary routine to call * &struct v4l2_subdev_pad_config->try_crop * * @sd: pointer to &struct v4l2_subdev @@ -1015,7 +1015,7 @@ v4l2_subdev_get_try_format(struct v4l2_subdev *sd, * @pad: index of the pad in the &struct v4l2_subdev_state->pads array. */ static inline struct v4l2_rect * -v4l2_subdev_get_try_crop(struct v4l2_subdev *sd, +v4l2_subdev_get_pad_crop(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, unsigned int pad) { @@ -1025,7 +1025,7 @@ v4l2_subdev_get_try_crop(struct v4l2_subdev *sd, } /** - * v4l2_subdev_get_try_compose - ancillary routine to call + * v4l2_subdev_get_pad_compose - ancillary routine to call * &struct v4l2_subdev_pad_config->try_compose * * @sd: pointer to &struct v4l2_subdev @@ -1033,7 +1033,7 @@ v4l2_subdev_get_try_crop(struct v4l2_subdev *sd, * @pad: index of the pad in the &struct v4l2_subdev_state->pads array. */ static inline struct v4l2_rect * -v4l2_subdev_get_try_compose(struct v4l2_subdev *sd, +v4l2_subdev_get_pad_compose(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, unsigned int pad) { @@ -1042,6 +1042,19 @@ v4l2_subdev_get_try_compose(struct v4l2_subdev *sd, return &state->pads[pad].try_compose; } +/* + * Temprary helpers until uses of v4l2_subdev_get_try_* functions have been + * renamed + */ +#define v4l2_subdev_get_try_format(sd, state, pad) \ + v4l2_subdev_get_pad_format(sd, state, pad) + +#define v4l2_subdev_get_try_crop(sd, state, pad) \ + v4l2_subdev_get_pad_crop(sd, state, pad) + +#define v4l2_subdev_get_try_compose(sd, state, pad) \ + v4l2_subdev_get_pad_compose(sd, state, pad) + #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ extern const struct v4l2_file_operations v4l2_subdev_fops; From patchwork Tue Apr 12 09:42:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 12810473 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 81C4CC433EF for ; Tue, 12 Apr 2022 10:54:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1377151AbiDLK4a (ORCPT ); Tue, 12 Apr 2022 06:56:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53744 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1356973AbiDLKpr (ORCPT ); Tue, 12 Apr 2022 06:45:47 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4D05414001 for ; Tue, 12 Apr 2022 02:43:28 -0700 (PDT) Received: from deskari.lan (91-156-85-209.elisa-laajakaista.fi [91.156.85.209]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 86D17BC1; Tue, 12 Apr 2022 11:43:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1649756604; bh=eM6+MXH7uj+01fhfvxgU1HnnGeUWmf8So4IZuC+CbqQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LWXollKOIEhjieWJ/KQLyg8t74l/uDA+fmj6ZpWpei3t8nvdDTfzn4arO1PYqFXGw u8gMYdiOkBarLE8OVaTIRAFHttR6aCUk8j8YKz8NEo/bFJAKs6/g++UQj1s04KVbpz AODDpsJxjVVEcCubz8FaQ48Am0LmBoSrjoeZIugk= From: Tomi Valkeinen To: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com, Jacopo Mondi , Laurent Pinchart , niklas.soderlund+renesas@ragnatech.se, Mauro Carvalho Chehab , Hans Verkuil , Pratyush Yadav , satish.nagireddy@getcruise.com Cc: Tomi Valkeinen Subject: [PATCH v8 06/10] media: subdev: pass also the active state to subdevs from ioctls Date: Tue, 12 Apr 2022 12:42:45 +0300 Message-Id: <20220412094249.695754-7-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> References: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org At the moment when a subdev op is called, the TRY subdev state (subdev_fh->state) is passed as a parameter even for the ACTIVE case, or alternatively a NULL can be passed for ACTIVE case. This used to make sense, as the ACTIVE state was handled internally by the subdev drivers. We now have a state for the ACTIVE case in a standard place, and can pass that also to the drivers. This patch changes the subdev ioctls to either pass the TRY or ACTIVE state to the subdev. Unfortunately many drivers call ops from other subdevs, and implicitly pass NULL as the state, so this is just a partial solution. A coccinelle spatch could perhaps be created which fixes the drivers' subdev calls. For all current upstream drivers this doesn't matter, as they do not expect to get a valid state for ACTIVE case. But future drivers which support multiplexed streaming and routing will depend on getting a state for both active and try cases. For new drivers we can mandate that the pipelines where the drivers are used need to pass the state properly, or preferably, not call such subdev ops at all. However, if an existing subdev driver is changed to support multiplexed streams, the driver has to consider cases where its ops will be called with NULL state. The problem can easily be solved by using the v4l2_subdev_lock_and_get_active_state() helper, introduced in a follow up patch. Another follow up patch adds wrappers for pad ops dealing with subdev state, which automate the use of v4l2_subdev_lock_and_get_active_state() for cases where the state is NULL. Signed-off-by: Tomi Valkeinen Reviewed-by: Hans Verkuil Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- drivers/media/v4l2-core/v4l2-subdev.c | 64 ++++++++++++++++++++++----- 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 5ac447b3038c..8f09b1175ef9 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -344,6 +344,44 @@ const struct v4l2_subdev_ops v4l2_subdev_call_wrappers = { EXPORT_SYMBOL(v4l2_subdev_call_wrappers); #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) + +static struct v4l2_subdev_state * +subdev_ioctl_get_state(struct v4l2_subdev *sd, struct v4l2_subdev_fh *subdev_fh, + unsigned int cmd, void *arg) +{ + u32 which; + + switch (cmd) { + default: + return NULL; + case VIDIOC_SUBDEV_G_FMT: + case VIDIOC_SUBDEV_S_FMT: + which = ((struct v4l2_subdev_format *)arg)->which; + break; + case VIDIOC_SUBDEV_G_CROP: + case VIDIOC_SUBDEV_S_CROP: + which = ((struct v4l2_subdev_crop *)arg)->which; + break; + case VIDIOC_SUBDEV_ENUM_MBUS_CODE: + which = ((struct v4l2_subdev_mbus_code_enum *)arg)->which; + break; + case VIDIOC_SUBDEV_ENUM_FRAME_SIZE: + which = ((struct v4l2_subdev_frame_size_enum *)arg)->which; + break; + case VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL: + which = ((struct v4l2_subdev_frame_interval_enum *)arg)->which; + break; + case VIDIOC_SUBDEV_G_SELECTION: + case VIDIOC_SUBDEV_S_SELECTION: + which = ((struct v4l2_subdev_selection *)arg)->which; + break; + } + + return which == V4L2_SUBDEV_FORMAT_TRY ? + subdev_fh->state : + v4l2_subdev_get_active_state(sd); +} + static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) { struct video_device *vdev = video_devdata(file); @@ -351,8 +389,11 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) struct v4l2_fh *vfh = file->private_data; struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh); bool ro_subdev = test_bit(V4L2_FL_SUBDEV_RO_DEVNODE, &vdev->flags); + struct v4l2_subdev_state *state; int rval; + state = subdev_ioctl_get_state(sd, subdev_fh, cmd, arg); + switch (cmd) { case VIDIOC_SUBDEV_QUERYCAP: { struct v4l2_subdev_capability *cap = arg; @@ -475,7 +516,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) memset(format->reserved, 0, sizeof(format->reserved)); memset(format->format.reserved, 0, sizeof(format->format.reserved)); - return v4l2_subdev_call(sd, pad, get_fmt, subdev_fh->state, format); + return v4l2_subdev_call(sd, pad, get_fmt, state, format); } case VIDIOC_SUBDEV_S_FMT: { @@ -486,7 +527,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) memset(format->reserved, 0, sizeof(format->reserved)); memset(format->format.reserved, 0, sizeof(format->format.reserved)); - return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->state, format); + return v4l2_subdev_call(sd, pad, set_fmt, state, format); } case VIDIOC_SUBDEV_G_CROP: { @@ -500,7 +541,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) sel.target = V4L2_SEL_TGT_CROP; rval = v4l2_subdev_call( - sd, pad, get_selection, subdev_fh->state, &sel); + sd, pad, get_selection, state, &sel); crop->rect = sel.r; @@ -522,7 +563,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) sel.r = crop->rect; rval = v4l2_subdev_call( - sd, pad, set_selection, subdev_fh->state, &sel); + sd, pad, set_selection, state, &sel); crop->rect = sel.r; @@ -533,7 +574,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) struct v4l2_subdev_mbus_code_enum *code = arg; memset(code->reserved, 0, sizeof(code->reserved)); - return v4l2_subdev_call(sd, pad, enum_mbus_code, subdev_fh->state, + return v4l2_subdev_call(sd, pad, enum_mbus_code, state, code); } @@ -541,7 +582,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) struct v4l2_subdev_frame_size_enum *fse = arg; memset(fse->reserved, 0, sizeof(fse->reserved)); - return v4l2_subdev_call(sd, pad, enum_frame_size, subdev_fh->state, + return v4l2_subdev_call(sd, pad, enum_frame_size, state, fse); } @@ -566,7 +607,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) struct v4l2_subdev_frame_interval_enum *fie = arg; memset(fie->reserved, 0, sizeof(fie->reserved)); - return v4l2_subdev_call(sd, pad, enum_frame_interval, subdev_fh->state, + return v4l2_subdev_call(sd, pad, enum_frame_interval, state, fie); } @@ -575,7 +616,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) memset(sel->reserved, 0, sizeof(sel->reserved)); return v4l2_subdev_call( - sd, pad, get_selection, subdev_fh->state, sel); + sd, pad, get_selection, state, sel); } case VIDIOC_SUBDEV_S_SELECTION: { @@ -586,7 +627,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) memset(sel->reserved, 0, sizeof(sel->reserved)); return v4l2_subdev_call( - sd, pad, set_selection, subdev_fh->state, sel); + sd, pad, set_selection, state, sel); } case VIDIOC_G_EDID: { @@ -820,10 +861,13 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad, if (is_media_entity_v4l2_subdev(pad->entity)) { struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(pad->entity); + struct v4l2_subdev_state *state; + + state = v4l2_subdev_get_active_state(sd); fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE; fmt->pad = pad->index; - return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt); + return v4l2_subdev_call(sd, pad, get_fmt, state, fmt); } WARN(pad->entity->function != MEDIA_ENT_F_IO_V4L, From patchwork Tue Apr 12 09:42:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 12810474 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C99A7C433F5 for ; Tue, 12 Apr 2022 10:54:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1356957AbiDLK4i (ORCPT ); Tue, 12 Apr 2022 06:56:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54164 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1356975AbiDLKpr (ORCPT ); Tue, 12 Apr 2022 06:45:47 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0F8CB5F279 for ; Tue, 12 Apr 2022 02:43:29 -0700 (PDT) Received: from deskari.lan (91-156-85-209.elisa-laajakaista.fi [91.156.85.209]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6D24FD02; Tue, 12 Apr 2022 11:43:24 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1649756605; bh=iEdOcx/9tf5trnHyobdsO0lXbhy46ZfHhzL8jeRRobU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WaSMVgHT8J63WCshpD78Wxi8uSz6A0bQ7/Ih/yJdrWB3mpHhhkqJyCQc46Xl+/UVI 0VyufwxyqKEakJlcQrVESyQc/mTg+0j6CCNcmamhl6RKROiSHs1A7nbrdqjCu81nVw lrQf52PX/XlpgpLNtuPTTLwy3k1LfhtNbKtyJl3g= From: Tomi Valkeinen To: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com, Jacopo Mondi , Laurent Pinchart , niklas.soderlund+renesas@ragnatech.se, Mauro Carvalho Chehab , Hans Verkuil , Pratyush Yadav , satish.nagireddy@getcruise.com Cc: Tomi Valkeinen Subject: [PATCH v8 07/10] media: subdev: add subdev state locking Date: Tue, 12 Apr 2022 12:42:46 +0300 Message-Id: <20220412094249.695754-8-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> References: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org The V4L2 subdevs have managed without centralized locking for the state (previously pad_config), as the try-state is supposedly safe (although I believe two TRY ioctls for the same fd would race), and the active-state, and its locking, is managed by the drivers internally. We now have active-state in a centralized position, and need locking. Strictly speaking the locking is only needed for new drivers that use the new state, as the current drivers continue behaving as they used to. However, active-state locking is complicated by the fact that currently the real active-state of a subdev is split into multiple parts: the new v4l2_subdev_state, subdev control state, and subdev's internal state. In the future all these three states should be combined into one state (the v4l2_subdev_state), and then a single lock for the state should be sufficient. But to solve the current split-state situation we need to share locks between the three states. This is accomplished by using the same lock management as the control handler does: we use a pointer to a mutex, allowing the driver to override the default mutex. Thus the driver can do e.g.: sd->state_lock = sd->ctrl_handler->lock; before calling v4l2_subdev_init_finalize(), resulting in sharing the same lock between the states and the controls. The locking model for active-state is such that any subdev op that gets the state as a parameter expects the state to be already locked by the caller, and expects the caller to release the lock. Signed-off-by: Tomi Valkeinen Reviewed-by: Laurent Pinchart Reviewed-by: Hans Verkuil --- .../platform/renesas/rcar-vin/rcar-v4l2.c | 3 +- .../media/platform/renesas/vsp1/vsp1_entity.c | 4 +- drivers/media/v4l2-core/v4l2-subdev.c | 63 ++++++--- drivers/staging/media/tegra-video/vi.c | 4 +- include/media/v4l2-subdev.h | 133 +++++++++++++++++- 5 files changed, 181 insertions(+), 26 deletions(-) diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c index da88f968c31a..3759f4619a77 100644 --- a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c @@ -255,6 +255,7 @@ static int rvin_try_format(struct rvin_dev *vin, u32 which, { struct v4l2_subdev *sd = vin_to_source(vin); struct v4l2_subdev_state *sd_state; + static struct lock_class_key key; struct v4l2_subdev_format format = { .which = which, .pad = vin->parallel.source_pad, @@ -267,7 +268,7 @@ static int rvin_try_format(struct rvin_dev *vin, u32 which, * FIXME: Drop this call, drivers are not supposed to use * __v4l2_subdev_state_alloc(). */ - sd_state = __v4l2_subdev_state_alloc(sd); + sd_state = __v4l2_subdev_state_alloc(sd, "rvin:state->lock", &key); if (IS_ERR(sd_state)) return PTR_ERR(sd_state); diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c index c82b3fb7b89a..a116a3362f9e 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c @@ -613,6 +613,7 @@ int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity *entity, const char *name, unsigned int num_pads, const struct v4l2_subdev_ops *ops, u32 function) { + static struct lock_class_key key; struct v4l2_subdev *subdev; unsigned int i; int ret; @@ -679,7 +680,8 @@ int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity *entity, * FIXME: Drop this call, drivers are not supposed to use * __v4l2_subdev_state_alloc(). */ - entity->config = __v4l2_subdev_state_alloc(&entity->subdev); + entity->config = __v4l2_subdev_state_alloc(&entity->subdev, + "vsp1:config->lock", &key); if (IS_ERR(entity->config)) { media_entity_cleanup(&entity->subdev.entity); return PTR_ERR(entity->config); diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 8f09b1175ef9..47d480786f03 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -27,8 +27,9 @@ static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd) { struct v4l2_subdev_state *state; + static struct lock_class_key key; - state = __v4l2_subdev_state_alloc(sd); + state = __v4l2_subdev_state_alloc(sd, "fh->state->lock", &key); if (IS_ERR(state)) return PTR_ERR(state); @@ -379,21 +380,18 @@ subdev_ioctl_get_state(struct v4l2_subdev *sd, struct v4l2_subdev_fh *subdev_fh, return which == V4L2_SUBDEV_FORMAT_TRY ? subdev_fh->state : - v4l2_subdev_get_active_state(sd); + v4l2_subdev_get_unlocked_active_state(sd); } -static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) +static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg, + struct v4l2_subdev_state *state) { struct video_device *vdev = video_devdata(file); struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev); struct v4l2_fh *vfh = file->private_data; - struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh); bool ro_subdev = test_bit(V4L2_FL_SUBDEV_RO_DEVNODE, &vdev->flags); - struct v4l2_subdev_state *state; int rval; - state = subdev_ioctl_get_state(sd, subdev_fh, cmd, arg); - switch (cmd) { case VIDIOC_SUBDEV_QUERYCAP: { struct v4l2_subdev_capability *cap = arg; @@ -706,8 +704,24 @@ static long subdev_do_ioctl_lock(struct file *file, unsigned int cmd, void *arg) if (lock && mutex_lock_interruptible(lock)) return -ERESTARTSYS; - if (video_is_registered(vdev)) - ret = subdev_do_ioctl(file, cmd, arg); + + if (video_is_registered(vdev)) { + struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev); + struct v4l2_fh *vfh = file->private_data; + struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh); + struct v4l2_subdev_state *state; + + state = subdev_ioctl_get_state(sd, subdev_fh, cmd, arg); + + if (state) + v4l2_subdev_lock_state(state); + + ret = subdev_do_ioctl(file, cmd, arg, state); + + if (state) + v4l2_subdev_unlock_state(state); + } + if (lock) mutex_unlock(lock); return ret; @@ -861,13 +875,10 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad, if (is_media_entity_v4l2_subdev(pad->entity)) { struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(pad->entity); - struct v4l2_subdev_state *state; - - state = v4l2_subdev_get_active_state(sd); fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE; fmt->pad = pad->index; - return v4l2_subdev_call(sd, pad, get_fmt, state, fmt); + return v4l2_subdev_call_state_active(sd, pad, get_fmt, fmt); } WARN(pad->entity->function != MEDIA_ENT_F_IO_V4L, @@ -905,7 +916,9 @@ int v4l2_subdev_link_validate(struct media_link *link) } EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate); -struct v4l2_subdev_state *__v4l2_subdev_state_alloc(struct v4l2_subdev *sd) +struct v4l2_subdev_state * +__v4l2_subdev_state_alloc(struct v4l2_subdev *sd, const char *lock_name, + struct lock_class_key *lock_key) { struct v4l2_subdev_state *state; int ret; @@ -914,6 +927,12 @@ struct v4l2_subdev_state *__v4l2_subdev_state_alloc(struct v4l2_subdev *sd) if (!state) return ERR_PTR(-ENOMEM); + __mutex_init(&state->_lock, lock_name, lock_key); + if (sd->state_lock) + state->lock = sd->state_lock; + else + state->lock = &state->_lock; + if (sd->entity.num_pads) { state->pads = kvmalloc_array(sd->entity.num_pads, sizeof(*state->pads), @@ -924,7 +943,14 @@ struct v4l2_subdev_state *__v4l2_subdev_state_alloc(struct v4l2_subdev *sd) } } + /* + * There can be no race at this point, but we lock the state anyway to + * satisfy lockdep checks. + */ + v4l2_subdev_lock_state(state); ret = v4l2_subdev_call(sd, pad, init_cfg, state); + v4l2_subdev_unlock_state(state); + if (ret < 0 && ret != -ENOIOCTLCMD) goto err; @@ -945,16 +971,19 @@ void __v4l2_subdev_state_free(struct v4l2_subdev_state *state) if (!state) return; + mutex_destroy(&state->_lock); + kvfree(state->pads); kfree(state); } EXPORT_SYMBOL_GPL(__v4l2_subdev_state_free); -int v4l2_subdev_init_finalize(struct v4l2_subdev *sd) +int __v4l2_subdev_init_finalize(struct v4l2_subdev *sd, const char *name, + struct lock_class_key *key) { struct v4l2_subdev_state *state; - state = __v4l2_subdev_state_alloc(sd); + state = __v4l2_subdev_state_alloc(sd, name, key); if (IS_ERR(state)) return PTR_ERR(state); @@ -962,7 +991,7 @@ int v4l2_subdev_init_finalize(struct v4l2_subdev *sd) return 0; } -EXPORT_SYMBOL_GPL(v4l2_subdev_init_finalize); +EXPORT_SYMBOL_GPL(__v4l2_subdev_init_finalize); void v4l2_subdev_cleanup(struct v4l2_subdev *sd) { diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index 07d368f345cd..8e184aa4c252 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -491,6 +491,7 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan, struct v4l2_pix_format *pix) { const struct tegra_video_format *fmtinfo; + static struct lock_class_key key; struct v4l2_subdev *subdev; struct v4l2_subdev_format fmt; struct v4l2_subdev_state *sd_state; @@ -511,7 +512,8 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan, * FIXME: Drop this call, drivers are not supposed to use * __v4l2_subdev_state_alloc(). */ - sd_state = __v4l2_subdev_state_alloc(subdev); + sd_state = __v4l2_subdev_state_alloc(subdev, "tegra:state->lock", + &key); if (IS_ERR(sd_state)) return PTR_ERR(sd_state); /* diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index eab8e4342613..f967c27e41c3 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -658,6 +658,8 @@ struct v4l2_subdev_pad_config { /** * struct v4l2_subdev_state - Used for storing subdev state information. * + * @_lock: default for 'lock' + * @lock: mutex for the state. May be replaced by the user. * @pads: &struct v4l2_subdev_pad_config array * * This structure only needs to be passed to the pad op if the 'which' field @@ -665,6 +667,9 @@ struct v4l2_subdev_pad_config { * %V4L2_SUBDEV_FORMAT_ACTIVE it is safe to pass %NULL. */ struct v4l2_subdev_state { + /* lock for the struct v4l2_subdev_state fields */ + struct mutex _lock; + struct mutex *lock; struct v4l2_subdev_pad_config *pads; }; @@ -888,6 +893,9 @@ struct v4l2_subdev_platform_data { * @subdev_notifier: A sub-device notifier implicitly registered for the sub- * device using v4l2_async_register_subdev_sensor(). * @pdata: common part of subdevice platform data + * @state_lock: A pointer to a lock used for all the subdev's states, set by the + * driver. This is optional. If NULL, each state instance will get + * a lock of its own. * @active_state: Active state for the subdev (NULL for subdevs tracking the * state internally). Initialized by calling * v4l2_subdev_init_finalize(). @@ -922,6 +930,7 @@ struct v4l2_subdev { struct v4l2_async_notifier *notifier; struct v4l2_async_notifier *subdev_notifier; struct v4l2_subdev_platform_data *pdata; + struct mutex *state_lock; /* * The fields below are private, and should only be accessed via @@ -1157,12 +1166,16 @@ int v4l2_subdev_link_validate(struct media_link *link); * __v4l2_subdev_state_alloc - allocate v4l2_subdev_state * * @sd: pointer to &struct v4l2_subdev for which the state is being allocated. + * @lock_name: name of the state lock + * @key: lock_class_key for the lock * * Must call __v4l2_subdev_state_free() when state is no longer needed. * * Not to be called directly by the drivers. */ -struct v4l2_subdev_state *__v4l2_subdev_state_alloc(struct v4l2_subdev *sd); +struct v4l2_subdev_state *__v4l2_subdev_state_alloc(struct v4l2_subdev *sd, + const char *lock_name, + struct lock_class_key *key); /** * __v4l2_subdev_state_free - free a v4l2_subdev_state @@ -1187,7 +1200,16 @@ void __v4l2_subdev_state_free(struct v4l2_subdev_state *state); * * The user must call v4l2_subdev_cleanup() when the subdev is being removed. */ -int v4l2_subdev_init_finalize(struct v4l2_subdev *sd); +#define v4l2_subdev_init_finalize(sd) \ + ({ \ + static struct lock_class_key __key; \ + const char *name = KBUILD_BASENAME \ + ":" __stringify(__LINE__) ":sd->active_state->lock"; \ + __v4l2_subdev_init_finalize(sd, name, &__key); \ + }) + +int __v4l2_subdev_init_finalize(struct v4l2_subdev *sd, const char *name, + struct lock_class_key *key); /** * v4l2_subdev_cleanup() - Releases the resources allocated by the subdevice @@ -1199,16 +1221,85 @@ int v4l2_subdev_init_finalize(struct v4l2_subdev *sd); void v4l2_subdev_cleanup(struct v4l2_subdev *sd); /** - * v4l2_subdev_get_active_state() - Returns the active subdev state for - * subdevice + * v4l2_subdev_lock_state() - Locks the subdev state + * @state: The subdevice state + * + * Locks the given subdev state. + * + * The state must be unlocked with v4l2_subdev_unlock_state() after use. + */ +static inline void v4l2_subdev_lock_state(struct v4l2_subdev_state *state) +{ + mutex_lock(state->lock); +} + +/** + * v4l2_subdev_unlock_state() - Unlocks the subdev state + * @state: The subdevice state + * + * Unlocks the given subdev state. + */ +static inline void v4l2_subdev_unlock_state(struct v4l2_subdev_state *state) +{ + mutex_unlock(state->lock); +} + +/** + * v4l2_subdev_get_unlocked_active_state() - Checks that the active subdev state + * is unlocked and returns it + * @sd: The subdevice + * + * Returns the active state for the subdevice, or NULL if the subdev does not + * support active state. If the state is not NULL, calls + * lockdep_assert_not_held() to issue a warning if the state is locked. + * + * This function is to be used e.g. when getting the active state for the sole + * purpose of passing it forward, without accessing the state fields. + */ +static inline struct v4l2_subdev_state * +v4l2_subdev_get_unlocked_active_state(struct v4l2_subdev *sd) +{ + if (sd->active_state) + lockdep_assert_not_held(sd->active_state->lock); + return sd->active_state; +} + +/** + * v4l2_subdev_get_locked_active_state() - Checks that the active subdev state + * is locked and returns it + * * @sd: The subdevice * * Returns the active state for the subdevice, or NULL if the subdev does not - * support active state. + * support active state. If the state is not NULL, calls lockdep_assert_held() + * to issue a warning if the state is not locked. + * + * This function is to be used when the caller knows that the active state is + * already locked. + */ +static inline struct v4l2_subdev_state * +v4l2_subdev_get_locked_active_state(struct v4l2_subdev *sd) +{ + if (sd->active_state) + lockdep_assert_held(sd->active_state->lock); + return sd->active_state; +} + +/** + * v4l2_subdev_lock_and_get_active_state() - Locks and returns the active subdev + * state for the subdevice + * @sd: The subdevice + * + * Returns the locked active state for the subdevice, or NULL if the subdev + * does not support active state. + * + * The state must be unlocked with v4l2_subdev_unlock_state() after use. */ static inline struct v4l2_subdev_state * -v4l2_subdev_get_active_state(struct v4l2_subdev *sd) +v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd) { + if (sd->active_state) + v4l2_subdev_lock_state(sd->active_state); return sd->active_state; } @@ -1255,6 +1346,36 @@ extern const struct v4l2_subdev_ops v4l2_subdev_call_wrappers; __result; \ }) +/** + * v4l2_subdev_call_state_active - call an operation of a v4l2_subdev which + * takes state as a parameter, passing the + * subdev its active state. + * + * @sd: pointer to the &struct v4l2_subdev + * @o: name of the element at &struct v4l2_subdev_ops that contains @f. + * Each element there groups a set of callbacks functions. + * @f: callback function to be called. + * The callback functions are defined in groups, according to + * each element at &struct v4l2_subdev_ops. + * @args: arguments for @f. + * + * This is similar to v4l2_subdev_call(), except that this version can only be + * used for ops that take a subdev state as a parameter. The macro will get the + * active state, lock it before calling the op and unlock it after the call. + */ +#define v4l2_subdev_call_state_active(sd, o, f, args...) \ + ({ \ + int __result; \ + struct v4l2_subdev_state *state; \ + state = v4l2_subdev_get_unlocked_active_state(sd); \ + if (state) \ + v4l2_subdev_lock_state(state); \ + __result = v4l2_subdev_call(sd, o, f, state, ##args); \ + if (state) \ + v4l2_subdev_unlock_state(state); \ + __result; \ + }) + /** * v4l2_subdev_has_op - Checks if a subdev defines a certain operation. * From patchwork Tue Apr 12 09:42:47 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 12810448 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E577CC4332F for ; Tue, 12 Apr 2022 10:45:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241736AbiDLKr1 (ORCPT ); Tue, 12 Apr 2022 06:47:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48080 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1356979AbiDLKpr (ORCPT ); Tue, 12 Apr 2022 06:45:47 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D394960060 for ; Tue, 12 Apr 2022 02:43:30 -0700 (PDT) Received: from deskari.lan (91-156-85-209.elisa-laajakaista.fi [91.156.85.209]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 4AF6AD1D; Tue, 12 Apr 2022 11:43:25 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1649756605; bh=Zb5BpviIUdydVk11HAVIV/HmMUyYO0huo6w31D17r0M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=QVTfw5Nn6Gs+3xhyP2Z5nsgZG5SGjC3izENcpQLua519rOoKperKFjnbnK5p4Ecy3 NAl2ncLMLZY6p6WEtSezy28bxue0mL8fPrrSWWJbrGF3e/44MrAhdYIJ1A7ZmfukTW o/2R3iuEwNfkW+XvY6hh4Qf7k1ay2oXEcY6IXpAg= From: Tomi Valkeinen To: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com, Jacopo Mondi , Laurent Pinchart , niklas.soderlund+renesas@ragnatech.se, Mauro Carvalho Chehab , Hans Verkuil , Pratyush Yadav , satish.nagireddy@getcruise.com Cc: Tomi Valkeinen Subject: [PATCH v8 08/10] media: subdev: add locking wrappers to subdev op wrappers Date: Tue, 12 Apr 2022 12:42:47 +0300 Message-Id: <20220412094249.695754-9-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> References: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org It is common that media drivers call subdev ops in source subdevs, and pass NULL as the state. This was the way to indicate that the callee should use the callee's private active state. E.g.: v4l2_subdev_call(priv->source_sd, pad, get_fmt, NULL, &sd_fmt); Now that we have a real subdev active state in the v4l2_subdev struct, we want the caller to pass a proper state (when available). And furthermore, the state should be locked. This would mean changing all the callers, which is the long term goal. To fix this issue in the short term, let's add an extra wrapper layer to all v4l2_subdev_call_pad_wrappers which deal with states. These wrappers handle the state == NULL case by using the locked active state instead (when available). Signed-off-by: Tomi Valkeinen Reviewed-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-subdev.c | 55 +++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 47d480786f03..afe916366dfe 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -318,14 +318,55 @@ static int call_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad, sd->ops->pad->get_mbus_config(sd, pad, config); } +#ifdef CONFIG_MEDIA_CONTROLLER +/* + * Create state-management wrapper for pad ops dealing with subdev state. The + * wrapper handles the case where the caller does not provide the called + * subdev's state. This should be removed when all the callers are fixed. + */ +#define DEFINE_STATE_WRAPPER(f, arg_type) \ + static int call_##f##_state(struct v4l2_subdev *sd, \ + struct v4l2_subdev_state *_state, \ + arg_type *arg) \ + { \ + struct v4l2_subdev_state *state = _state; \ + int ret; \ + if (!_state) \ + state = v4l2_subdev_lock_and_get_active_state(sd); \ + ret = call_##f(sd, state, arg); \ + if (!_state && state) \ + v4l2_subdev_unlock_state(state); \ + return ret; \ + } + +#else /* CONFIG_MEDIA_CONTROLLER */ + +#define DEFINE_STATE_WRAPPER(f, arg_type) \ + static int call_##f##_state(struct v4l2_subdev *sd, \ + struct v4l2_subdev_state *state, \ + arg_type *arg) \ + { \ + return call_##f(sd, state, arg); \ + } + +#endif /* CONFIG_MEDIA_CONTROLLER */ + +DEFINE_STATE_WRAPPER(get_fmt, struct v4l2_subdev_format); +DEFINE_STATE_WRAPPER(set_fmt, struct v4l2_subdev_format); +DEFINE_STATE_WRAPPER(enum_mbus_code, struct v4l2_subdev_mbus_code_enum); +DEFINE_STATE_WRAPPER(enum_frame_size, struct v4l2_subdev_frame_size_enum); +DEFINE_STATE_WRAPPER(enum_frame_interval, struct v4l2_subdev_frame_interval_enum); +DEFINE_STATE_WRAPPER(get_selection, struct v4l2_subdev_selection); +DEFINE_STATE_WRAPPER(set_selection, struct v4l2_subdev_selection); + static const struct v4l2_subdev_pad_ops v4l2_subdev_call_pad_wrappers = { - .get_fmt = call_get_fmt, - .set_fmt = call_set_fmt, - .enum_mbus_code = call_enum_mbus_code, - .enum_frame_size = call_enum_frame_size, - .enum_frame_interval = call_enum_frame_interval, - .get_selection = call_get_selection, - .set_selection = call_set_selection, + .get_fmt = call_get_fmt_state, + .set_fmt = call_set_fmt_state, + .enum_mbus_code = call_enum_mbus_code_state, + .enum_frame_size = call_enum_frame_size_state, + .enum_frame_interval = call_enum_frame_interval_state, + .get_selection = call_get_selection_state, + .set_selection = call_set_selection_state, .get_edid = call_get_edid, .set_edid = call_set_edid, .dv_timings_cap = call_dv_timings_cap, From patchwork Tue Apr 12 09:42:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 12810452 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DD7D6C433EF for ; Tue, 12 Apr 2022 10:46:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347441AbiDLKr4 (ORCPT ); Tue, 12 Apr 2022 06:47:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54400 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1356987AbiDLKps (ORCPT ); Tue, 12 Apr 2022 06:45:48 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D46E961A33 for ; Tue, 12 Apr 2022 02:43:31 -0700 (PDT) Received: from deskari.lan (91-156-85-209.elisa-laajakaista.fi [91.156.85.209]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2963FD2D; Tue, 12 Apr 2022 11:43:26 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1649756606; bh=x2tg7MnuMqPmfxqLudJzhlPnkEqHYBWNLKjCsUNgcHM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=iXsT3TWBNFqGWzKMBwGK+gmv+yC9TgSvgubkZB8I608mP+CHIinKvCBx1Ccn3FP5q 1WJSZJ7c4QSlbj7E+6Pbro6NkpGCc3Q2tiVC5BHh1fWzirxWb9mSImIM3XvyZHgyOL Phz2D524ORrWxO3/dVzPPnRuObw9pTg3khQzp1lY= From: Tomi Valkeinen To: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com, Jacopo Mondi , Laurent Pinchart , niklas.soderlund+renesas@ragnatech.se, Mauro Carvalho Chehab , Hans Verkuil , Pratyush Yadav , satish.nagireddy@getcruise.com Cc: Tomi Valkeinen Subject: [PATCH v8 09/10] media: subdev: add v4l2_subdev_get_fmt() helper function Date: Tue, 12 Apr 2022 12:42:48 +0300 Message-Id: <20220412094249.695754-10-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> References: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Add v4l2_subdev_get_fmt() helper function which implements v4l2_subdev_pad_ops.get_fmt using active state. Subdev drivers that support active state and do not need to do anything special in their get_fmt op can use this helper directly for v4l2_subdev_pad_ops.get_fmt. Signed-off-by: Tomi Valkeinen Reviewed-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-subdev.c | 22 ++++++++++++++++++++++ include/media/v4l2-subdev.h | 21 +++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index afe916366dfe..3da200bb23dd 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -1041,6 +1041,28 @@ void v4l2_subdev_cleanup(struct v4l2_subdev *sd) } EXPORT_SYMBOL_GPL(v4l2_subdev_cleanup); +#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) + +int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, + struct v4l2_subdev_format *format) +{ + struct v4l2_mbus_framefmt *fmt; + + if (format->pad >= sd->entity.num_pads) + return -EINVAL; + + fmt = v4l2_subdev_get_pad_format(sd, state, format->pad); + if (!fmt) + return -EINVAL; + + format->format = *fmt; + + return 0; +} +EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt); + +#endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ + #endif /* CONFIG_MEDIA_CONTROLLER */ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index f967c27e41c3..33a7201edb61 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1303,6 +1303,27 @@ v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd) return sd->active_state; } +#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) + +/** + * v4l2_subdev_get_fmt() - Fill format based on state + * @sd: subdevice + * @state: subdevice state + * @format: pointer to &struct v4l2_subdev_format + * + * Fill @format->format field based on the information in the @format struct. + * + * This function can be used by the subdev drivers which support active state to + * implement v4l2_subdev_pad_ops.get_fmt if the subdev driver does not need to + * do anything special in their get_fmt op. + * + * Returns 0 on success, error value otherwise. + */ +int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, + struct v4l2_subdev_format *format); + +#endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ + #endif /* CONFIG_MEDIA_CONTROLLER */ /** From patchwork Tue Apr 12 09:42:49 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 12810472 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2DD79C433F5 for ; Tue, 12 Apr 2022 10:54:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242922AbiDLKwb (ORCPT ); Tue, 12 Apr 2022 06:52:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53760 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1357010AbiDLKpt (ORCPT ); Tue, 12 Apr 2022 06:45:49 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3F33F62100 for ; Tue, 12 Apr 2022 02:43:33 -0700 (PDT) Received: from deskari.lan (91-156-85-209.elisa-laajakaista.fi [91.156.85.209]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 098E7D6E; Tue, 12 Apr 2022 11:43:26 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1649756607; bh=qg+wfCBvON+YzZspt4WVAWUBVPk3scWuwfLfXClVOn8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=J2JJ9EhyNttRB+4FLmA0oReCcw0KK+2IpDt4ETRcZ9ZE81a1svMWZH8qOQs2KnRCz etpeP2d+/ffjKpGxGA6p+YuFdb8HTcrp8EzpDKlCrvexHVlEng4r1EVR6I/aU9GcEH 19JTL2CLpTutKYdJy7ePOF0ijgzfYPU/2f8i1y/o= From: Tomi Valkeinen To: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com, Jacopo Mondi , Laurent Pinchart , niklas.soderlund+renesas@ragnatech.se, Mauro Carvalho Chehab , Hans Verkuil , Pratyush Yadav , satish.nagireddy@getcruise.com Cc: Tomi Valkeinen Subject: [PATCH v8 10/10] media: Documentation: add documentation about subdev state Date: Tue, 12 Apr 2022 12:42:49 +0300 Message-Id: <20220412094249.695754-11-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> References: <20220412094249.695754-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Add documentation about centrally managed subdev state. Signed-off-by: Tomi Valkeinen Reviewed-by: Jacopo Mondi Reviewed-by: Hans Verkuil Reviewed-by: Laurent Pinchart --- .../driver-api/media/v4l2-subdev.rst | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst index 08ea2673b19e..cf3b52bdbfb9 100644 --- a/Documentation/driver-api/media/v4l2-subdev.rst +++ b/Documentation/driver-api/media/v4l2-subdev.rst @@ -518,6 +518,75 @@ The :c:func:`v4l2_i2c_new_subdev` function will call :c:type:`i2c_board_info` structure using the ``client_type`` and the ``addr`` to fill it. +Centrally managed subdev active state +------------------------------------- + +Traditionally V4L2 subdev drivers maintained internal state for the active +device configuration. This is often implemented as e.g. an array of struct +v4l2_mbus_framefmt, one entry for each pad, and similarly for crop and compose +rectangles. + +In addition to the active configuration, each subdev file handle has an array of +struct v4l2_subdev_pad_config, managed by the V4L2 core, which contains the try +configuration. + +To simplify the subdev drivers the V4L2 subdev API now optionally supports a +centrally managed active configuration represented by +:c:type:`v4l2_subdev_state`. One instance of state, which contains the active +device configuration, is stored in the sub-device itself as part of +the :c:type:`v4l2_subdev` structure, while the core associates a try state to +each open file handle, to store the try configuration related to that file +handle. + +Sub-device drivers can opt-in and use state to manage their active configuration +by initializing the subdevice state with a call to v4l2_subdev_init_finalize() +before registering the sub-device. They must also call v4l2_subdev_cleanup() +to release all the allocated resources before unregistering the sub-device. +The core automatically allocates and initializes a state for each open file +handle to store the try configurations and frees it when closing the file +handle. + +V4L2 sub-device operations that use both the :ref:`ACTIVE and TRY formats +` receive the correct state to operate on through +the 'state' parameter. The state must be locked and unlocked by the +caller by calling :c:func:`v4l2_subdev_lock_state()` and +:c:func:`v4l2_subdev_unlock_state()`. The caller can do so by calling the subdev +operation through the :c:func:`v4l2_subdev_call_state_active()` macro. + +Operations that do not receive a state parameter implicitly operate on the +subdevice active state, which drivers can exclusively access by +calling :c:func:`v4l2_subdev_lock_and_get_active_state()`. The sub-device active +state must equally be released by calling :c:func:`v4l2_subdev_unlock_state()`. + +Drivers must never manually access the state stored in the :c:type:`v4l2_subdev` +or in the file handle without going through the designated helpers. + +While the V4L2 core passes the correct try or active state to the subdevice +operations, many existing device drivers pass a NULL state when calling +operations with :c:func:`v4l2_subdev_call()`. This legacy construct causes +issues with subdevice drivers that let the V4L2 core manage the active state, +as they expect to receive the appropriate state as a parameter. To help the +conversion of subdevice drivers to a managed active state without having to +convert all callers at the same time, an additional wrapper layer has been +added to v4l2_subdev_call(), which handles the NULL case by geting and locking +the callee's active state with :c:func:`v4l2_subdev_lock_and_get_active_state()`, +and unlocking the state after the call. + +The whole subdev state is in reality split into three parts: the +v4l2_subdev_state, subdev controls and subdev driver's internal state. In the +future these parts should be combined into a single state. For the time being +we need a way to handle the locking for these parts. This can be accomplished +by sharing a lock. The v4l2_ctrl_handler already supports this via its 'lock' +pointer and the same model is used with states. The driver can do the following +before calling v4l2_subdev_init_finalize(): + +.. code-block:: c + + sd->ctrl_handler->lock = &priv->mutex; + sd->state_lock = &priv->mutex; + +This shares the driver's private mutex between the controls and the states. + V4L2 sub-device functions and data structures ---------------------------------------------