Merge tag 'next-media-20240826' of git://git.kernel.org/pub/scm/linux/kernel/git/pinchartl/linux.git
Improvements to link validation in media graph.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://patchwork.linuxtv.org/project/linux-media/patch/20240826141040.GC11033@pendragon.ideasonboard.com/
diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index f3a5cba..28e56f6 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -902,8 +902,11 @@ static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)
return 0;
}
-static int isc_validate(struct isc_device *isc)
+static int isc_link_validate(struct media_link *link)
{
+ struct video_device *vdev =
+ media_entity_to_video_device(link->sink->entity);
+ struct isc_device *isc = video_get_drvdata(vdev);
int ret;
int i;
struct isc_format *sd_fmt = NULL;
@@ -1906,20 +1909,6 @@ int microchip_isc_pipeline_init(struct isc_device *isc)
}
EXPORT_SYMBOL_GPL(microchip_isc_pipeline_init);
-static int isc_link_validate(struct media_link *link)
-{
- struct video_device *vdev =
- media_entity_to_video_device(link->sink->entity);
- struct isc_device *isc = video_get_drvdata(vdev);
- int ret;
-
- ret = v4l2_subdev_link_validate(link);
- if (ret)
- return ret;
-
- return isc_validate(isc);
-}
-
static const struct media_entity_operations isc_entity_operations = {
.link_validate = isc_link_validate,
};
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index fdb46ec..e728f9f 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -1082,6 +1082,27 @@ static const struct v4l2_file_operations vsp1_video_fops = {
};
/* -----------------------------------------------------------------------------
+ * Media entity operations
+ */
+
+static int vsp1_video_link_validate(struct media_link *link)
+{
+ /*
+ * Ideally, link validation should be implemented here instead of
+ * calling vsp1_video_verify_format() in vsp1_video_streamon()
+ * manually. That would however break userspace that start one video
+ * device before configures formats on other video devices in the
+ * pipeline. This operation is just a no-op to silence the warnings
+ * from v4l2_subdev_link_validate().
+ */
+ return 0;
+}
+
+static const struct media_entity_operations vsp1_video_media_ops = {
+ .link_validate = vsp1_video_link_validate,
+};
+
+/* -----------------------------------------------------------------------------
* Suspend and Resume
*/
@@ -1215,6 +1236,7 @@ struct vsp1_video *vsp1_video_create(struct vsp1_device *vsp1,
/* ... and the video node... */
video->video.v4l2_dev = &video->vsp1->v4l2_dev;
+ video->video.entity.ops = &vsp1_video_media_ops;
video->video.fops = &vsp1_video_fops;
snprintf(video->video.name, sizeof(video->video.name), "%s %s",
rwpf->entity.subdev.name, direction);
diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
index 097a3a0..d07e980 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
@@ -35,7 +35,18 @@ struct sun4i_csi_traits {
bool has_isp;
};
+static int sun4i_csi_video_link_validate(struct media_link *link)
+{
+ dev_warn_once(link->graph_obj.mdev->dev,
+ "Driver bug: link validation not implemented\n");
+ return 0;
+}
+
static const struct media_entity_operations sun4i_csi_video_entity_ops = {
+ .link_validate = sun4i_csi_video_link_validate,
+};
+
+static const struct media_entity_operations sun4i_csi_subdev_entity_ops = {
.link_validate = v4l2_subdev_link_validate,
};
@@ -214,6 +225,7 @@ static int sun4i_csi_probe(struct platform_device *pdev)
subdev->internal_ops = &sun4i_csi_subdev_internal_ops;
subdev->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
subdev->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+ subdev->entity.ops = &sun4i_csi_subdev_entity_ops;
subdev->owner = THIS_MODULE;
snprintf(subdev->name, sizeof(subdev->name), "sun4i-csi-0");
v4l2_set_subdevdata(subdev, csi);
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 7c5812d..3a4ba08 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1443,16 +1443,53 @@ int v4l2_subdev_link_validate(struct media_link *link)
bool states_locked;
int ret;
- if (!is_media_entity_v4l2_subdev(link->sink->entity) ||
- !is_media_entity_v4l2_subdev(link->source->entity)) {
- pr_warn_once("%s of link '%s':%u->'%s':%u is not a V4L2 sub-device, driver bug!\n",
- !is_media_entity_v4l2_subdev(link->sink->entity) ?
- "sink" : "source",
- link->source->entity->name, link->source->index,
- link->sink->entity->name, link->sink->index);
- return 0;
+ /*
+ * Links are validated in the context of the sink entity. Usage of this
+ * helper on a sink that is not a subdev is a clear driver bug.
+ */
+ if (WARN_ON_ONCE(!is_media_entity_v4l2_subdev(link->sink->entity)))
+ return -EINVAL;
+
+ /*
+ * If the source is a video device, delegate link validation to it. This
+ * allows usage of this helper for subdev connected to a video output
+ * device, provided that the driver implement the video output device's
+ * .link_validate() operation.
+ */
+ if (is_media_entity_v4l2_video_device(link->source->entity)) {
+ struct media_entity *source = link->source->entity;
+
+ if (!source->ops || !source->ops->link_validate) {
+ /*
+ * Many existing drivers do not implement the required
+ * .link_validate() operation for their video devices.
+ * Print a warning to get the drivers fixed, and return
+ * 0 to avoid breaking userspace. This should
+ * eventually be turned into a WARN_ON() when all
+ * drivers will have been fixed.
+ */
+ pr_warn_once("video device '%s' does not implement .link_validate(), driver bug!\n",
+ source->name);
+ return 0;
+ }
+
+ /*
+ * Avoid infinite loops in case a video device incorrectly uses
+ * this helper function as its .link_validate() handler.
+ */
+ if (WARN_ON(source->ops->link_validate == v4l2_subdev_link_validate))
+ return -EINVAL;
+
+ return source->ops->link_validate(link);
}
+ /*
+ * If the source is still not a subdev, usage of this helper is a clear
+ * driver bug.
+ */
+ if (WARN_ON(!is_media_entity_v4l2_subdev(link->source->entity)))
+ return -EINVAL;
+
sink_sd = media_entity_to_v4l2_subdev(link->sink->entity);
source_sd = media_entity_to_v4l2_subdev(link->source->entity);
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index bd235d32..8daa092 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1250,6 +1250,12 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd,
* calls v4l2_subdev_link_validate_default() to ensure that
* width, height and the media bus pixel code are equal on both
* source and sink of the link.
+ *
+ * The function can be used as a drop-in &media_entity_ops.link_validate
+ * implementation for v4l2_subdev instances. It supports all links between
+ * subdevs, as well as links between subdevs and video devices, provided that
+ * the video devices also implement their &media_entity_ops.link_validate
+ * operation.
*/
int v4l2_subdev_link_validate(struct media_link *link);
