All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com,
	Jacopo Mondi <jacopo+renesas@jmondi.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	niklas.soderlund+renesas@ragnatech.se,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	Pratyush Yadav <p.yadav@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Subject: [PATCH 5/6] media: subdev: Add v4l2_subdev_lock_and_return_state()
Date: Fri, 17 Dec 2021 13:18:35 +0200	[thread overview]
Message-ID: <20211217111836.225013-6-tomi.valkeinen@ideasonboard.com> (raw)
In-Reply-To: <20211217111836.225013-1-tomi.valkeinen@ideasonboard.com>

All suitable subdev ops are now passed either the TRY or the ACTIVE
state by the v4l2 core. However, other subdev drivers can still call the
ops passing NULL as the state, implying the active case.

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_return_state() helper, introduced here.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/media/v4l2-subdev.h | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 80cd0ffb60d1..4bc3cc04cc6e 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -8,6 +8,7 @@
 #ifndef _V4L2_SUBDEV_H
 #define _V4L2_SUBDEV_H
 
+#include "linux/dev_printk.h"
 #include <linux/types.h>
 #include <linux/v4l2-subdev.h>
 #include <media/media-entity.h>
@@ -1321,4 +1322,38 @@ void v4l2_subdev_lock_state(struct v4l2_subdev_state *state);
  */
 void v4l2_subdev_unlock_state(struct v4l2_subdev_state *state);
 
+/**
+ * v4l2_subdev_lock_and_return_state() - Gets locked try or active subdev state
+ * @sd: subdevice
+ * @state: subdevice state as passed to the subdev op
+ *
+ * Due to legacy reasons, when subdev drivers call ops in other subdevs they use
+ * NULL as the state parameter, as subdevs always used to have their active
+ * state stored privately.
+ *
+ * However, newer state-aware subdev drivers, which store their active state in
+ * a common place, subdev->active_state, expect to always get a proper state as
+ * a parameter.
+ *
+ * These state-aware drivers can use v4l2_subdev_lock_and_return_state() instead
+ * of v4l2_subdev_lock_state(). v4l2_subdev_lock_and_return_state() solves the
+ * issue by using subdev->state in case the passed state is NULL.
+ *
+ * This is a temporary helper function, and should be removed when we can ensure
+ * that all drivers pass proper state when calling other subdevs.
+ */
+static inline struct v4l2_subdev_state *
+v4l2_subdev_lock_and_return_state(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *state)
+{
+	if (!state)
+		dev_notice_once(sd->dev, "source subdev should be ported to new state management\n");
+
+	state = state ? state : sd->active_state;
+
+	v4l2_subdev_lock_state(state);
+
+	return state;
+}
+
 #endif
-- 
2.25.1


  parent reply	other threads:[~2021-12-17 11:19 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-17 11:18 [PATCH 0/6] v4l: subdev active state Tomi Valkeinen
2021-12-17 11:18 ` [PATCH 1/6] media: subdev: rename subdev-state alloc & free Tomi Valkeinen
2021-12-17 11:18 ` [PATCH 2/6] media: subdev: add active state to struct v4l2_subdev Tomi Valkeinen
2021-12-17 12:07   ` Jacopo Mondi
2021-12-17 13:19     ` Tomi Valkeinen
2021-12-17 11:18 ` [PATCH 3/6] media: subdev: pass also the active state to subdevs from ioctls Tomi Valkeinen
2021-12-17 11:18 ` [PATCH 4/6] media: subdev: add subdev state locking Tomi Valkeinen
2021-12-17 11:18 ` Tomi Valkeinen [this message]
2021-12-17 11:49   ` [PATCH 5/6] media: subdev: Add v4l2_subdev_lock_and_return_state() Jacopo Mondi
2021-12-17 11:18 ` [PATCH 6/6] media: Documentation: add documentation about subdev state Tomi Valkeinen
2021-12-17 11:57   ` Hans Verkuil
2021-12-17 12:05   ` Jacopo Mondi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211217111836.225013-6-tomi.valkeinen@ideasonboard.com \
    --to=tomi.valkeinen@ideasonboard.com \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=jacopo+renesas@jmondi.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --cc=p.yadav@ti.com \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.