On 14/12/2021 09:42, Jacopo Mondi wrote:
Hi Tomi,
On Tue, Nov 30, 2021 at 04:15:02PM +0200, Tomi Valkeinen wrote:
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.
I would use 'active' instead of ACTIVE
Hmm, yes, the naming I use varies, I guess =). I often use capitalized
ACTIVE/TRY to make them stand out, and not get mixed with normal words.
With active/try-state it's clear, I believe, especially if I have a dash
there.
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.
Add a mutex to the struct v4l2_subdev_state, along with a few helper
functions for locking/unlocking.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
---
drivers/media/platform/rcar-vin/rcar-v4l2.c | 3 +-
drivers/media/platform/vsp1/vsp1_entity.c | 4 +-
drivers/media/v4l2-core/v4l2-subdev.c | 38 +++++++++++++---
drivers/staging/media/tegra-video/vi.c | 4 +-
include/media/v4l2-subdev.h | 50 ++++++++++++++++++++-
5 files changed, 89 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index ba1d16ab1651..e6bd94d63e4f 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -244,6 +244,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,
@@ -252,7 +253,7 @@ static int rvin_try_format(struct rvin_dev *vin, u32 which,
u32 width, height;
int ret;
- sd_state = __v4l2_subdev_state_alloc(sd);
+ sd_state = __v4l2_subdev_state_alloc(sd, "rvin:state->lock", &key);
Is key needed in the callers ? can it be moved to
__v4l2_subdev_state_alloc() ?
These drivers shouldn't really be using the functions, so I didn't even
try to make it look neat.
Tomi
