* [PATCH 1/9] media: staging/imx: get CSI bus type from nearest upstream entity
2017-10-28 20:36 [PATCH 0/9] media: imx: Add better OF graph support Steve Longerbeam
@ 2017-10-28 20:36 ` Steve Longerbeam
2017-10-28 20:36 ` [PATCH 2/9] media: staging/imx: remove static media link arrays Steve Longerbeam
` (8 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Steve Longerbeam @ 2017-10-28 20:36 UTC (permalink / raw)
To: Philipp Zabel, Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: linux-media, devel, linux-kernel, Russell King, Steve Longerbeam
The imx-media driver currently supports a device tree graph of
limited complexity. This patch is a first step in allowing imx-media
to work with more general OF graphs.
The CSI subdevice assumes the originating upstream subdevice (the
"sensor") is connected directly to either the CSI mux or the MIPI
CSI-2 receiver. But for more complex graphs, the sensor can be distant,
with possible bridge entities in between. Thus the sensor's bus type
could be quite different from what is entering the CSI. For example
a distant sensor could have a parallel interface, but the stream
entering the i.MX is MIPI CSI-2.
To remove this assumption, get the entering bus config from the entity
that is directly upstream from either the CSI mux, or the CSI-2 receiver.
If the CSI-2 receiver is not in the enabled pipeline, the bus type to the
CSI is parallel, otherwise the CSI is receiving MIPI CSI-2.
Note that we can't use the direct upstream source connected to CSI
(which is either the CSI mux or the CSI-2 receiver) to determine
bus type. The bus entering the CSI from the CSI-2 receiver is a 32-bit
parallel bus containing the demultiplexed MIPI CSI-2 virtual channels.
But the CSI and its IDMAC channels must be configured based on whether
it is receiving data from the CSI-2 receiver or from the CSI mux's
parallel interface pins.
The function csi_get_upstream_endpoint() is used to find this
endpoint. It makes use of a new utility function
imx_media_find_upstream_pad(), that if given a grp_id of 0, will
return the closest upstream pad from start_entity.
With these changes, imx_media_find_sensor() is no longer used and
is removed. As a result there is also no longer a need to identify
any sensor or set the sensor subdev's group id as a method to search
for it. So IMX_MEDIA_GRP_ID_SENSOR is removed. Also the video-mux group
id IMX_MEDIA_GRP_ID_VIDMUX was never used so that is removed as well.
The remaining IMX_MEDIA_GRP_ID_* definitions are entities internal
to the i.MX.
Another use of imx_media_find_sensor() in the CSI was to call the
sensor's g_skip_frames op to determine if a delay was needed before
enabling the CSI at stream on. If necessary this will have to be
re-addressed at a later time.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/staging/media/imx/imx-media-csi.c | 188 ++++++++++++++++------------
drivers/staging/media/imx/imx-media-dev.c | 12 --
drivers/staging/media/imx/imx-media-of.c | 21 ----
drivers/staging/media/imx/imx-media-utils.c | 63 +++++-----
drivers/staging/media/imx/imx-media.h | 27 ++--
5 files changed, 150 insertions(+), 161 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 6d85611..c45b235 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -13,6 +13,7 @@
#include <linux/gcd.h>
#include <linux/interrupt.h>
#include <linux/module.h>
+#include <linux/of_graph.h>
#include <linux/pinctrl/consumer.h>
#include <linux/platform_device.h>
#include <media/v4l2-ctrls.h>
@@ -99,8 +100,8 @@ struct csi_priv {
/* the mipi virtual channel number at link validate */
int vc_num;
- /* the attached sensor at stream on */
- struct imx_media_subdev *sensor;
+ /* the upstream endpoint CSI is receiving from */
+ struct v4l2_fwnode_endpoint upstream_ep;
spinlock_t irqlock; /* protect eof_irq handler */
struct timer_list eof_timeout_timer;
@@ -120,6 +121,71 @@ static inline struct csi_priv *sd_to_dev(struct v4l2_subdev *sdev)
return container_of(sdev, struct csi_priv, sd);
}
+static inline bool is_parallel_16bit_bus(struct v4l2_fwnode_endpoint *ep)
+{
+ return ep->bus_type != V4L2_MBUS_CSI2 &&
+ ep->bus.parallel.bus_width >= 16;
+}
+
+/*
+ * Parses the fwnode endpoint from the source pad of the entity
+ * connected to this CSI. This will either be the entity directly
+ * upstream from the CSI-2 receiver, or directly upstream from the
+ * video mux. The endpoint is needed to determine the bus type and
+ * bus config coming into the CSI.
+ */
+static int csi_get_upstream_endpoint(struct csi_priv *priv,
+ struct v4l2_fwnode_endpoint *ep)
+{
+ struct device_node *endpoint, *port;
+ struct imx_media_subdev *imxsd;
+ struct media_entity *src;
+ struct v4l2_subdev *sd;
+ struct media_pad *pad;
+
+ if (!priv->src_sd)
+ return -EPIPE;
+
+ src = &priv->src_sd->entity;
+
+ if (src->function == MEDIA_ENT_F_VID_MUX) {
+ /*
+ * CSI is connected directly to video mux, skip up to
+ * CSI-2 receiver if it is in the path, otherwise stay
+ * with video mux.
+ */
+ imxsd = imx_media_find_upstream_subdev(priv->md, src,
+ IMX_MEDIA_GRP_ID_CSI2);
+ if (!IS_ERR(imxsd))
+ src = &imxsd->sd->entity;
+ }
+
+ /* get source pad of entity directly upstream from src */
+ pad = imx_media_find_upstream_pad(priv->md, src, 0);
+ if (IS_ERR(pad))
+ return PTR_ERR(pad);
+
+ sd = media_entity_to_v4l2_subdev(pad->entity);
+
+ /*
+ * NOTE: this assumes an OF-graph port id is the same as a
+ * media pad index.
+ */
+ port = of_graph_get_port_by_id(sd->dev->of_node, pad->index);
+ if (!port)
+ return -ENODEV;
+
+ endpoint = of_get_next_child(port, NULL);
+ of_node_put(port);
+ if (!endpoint)
+ return -ENODEV;
+
+ v4l2_fwnode_endpoint_parse(of_fwnode_handle(endpoint), ep);
+ of_node_put(endpoint);
+
+ return 0;
+}
+
static void csi_idmac_put_ipu_resources(struct csi_priv *priv)
{
if (priv->idmac_ch)
@@ -306,7 +372,6 @@ static void csi_idmac_unsetup_vb2_buf(struct csi_priv *priv,
static int csi_idmac_setup_channel(struct csi_priv *priv)
{
struct imx_media_video_dev *vdev = priv->vdev;
- struct v4l2_fwnode_endpoint *sensor_ep;
struct v4l2_mbus_framefmt *infmt;
struct ipu_image image;
u32 passthrough_bits;
@@ -316,7 +381,6 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
int ret;
infmt = &priv->format_mbus[CSI_SINK_PAD];
- sensor_ep = &priv->sensor->sensor_ep;
ipu_cpmem_zero(priv->idmac_ch);
@@ -334,7 +398,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
* Check for conditions that require the IPU to handle the
* data internally as generic data, aka passthrough mode:
* - raw bayer formats
- * - the sensor bus is 16-bit parallel
+ * - the CSI is receiving from a 16-bit parallel bus
*/
switch (image.pix.pixelformat) {
case V4L2_PIX_FMT_SBGGR8:
@@ -358,8 +422,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
burst_size = (image.pix.width & 0x3f) ?
((image.pix.width & 0x1f) ?
((image.pix.width & 0xf) ? 8 : 16) : 32) : 64;
- passthrough = (sensor_ep->bus_type != V4L2_MBUS_CSI2 &&
- sensor_ep->bus.parallel.bus_width >= 16);
+ passthrough = is_parallel_16bit_bus(&priv->upstream_ep);
passthrough_bits = 16;
/* Skip writing U and V components to odd rows */
ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
@@ -368,14 +431,12 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
case V4L2_PIX_FMT_UYVY:
burst_size = (image.pix.width & 0x1f) ?
((image.pix.width & 0xf) ? 8 : 16) : 32;
- passthrough = (sensor_ep->bus_type != V4L2_MBUS_CSI2 &&
- sensor_ep->bus.parallel.bus_width >= 16);
+ passthrough = is_parallel_16bit_bus(&priv->upstream_ep);
passthrough_bits = 16;
break;
default:
burst_size = (image.pix.width & 0xf) ? 8 : 16;
- passthrough = (sensor_ep->bus_type != V4L2_MBUS_CSI2 &&
- sensor_ep->bus.parallel.bus_width >= 16);
+ passthrough = is_parallel_16bit_bus(&priv->upstream_ep);
passthrough_bits = 16;
break;
}
@@ -572,22 +633,20 @@ static void csi_idmac_stop(struct csi_priv *priv)
static int csi_setup(struct csi_priv *priv)
{
struct v4l2_mbus_framefmt *infmt, *outfmt;
- struct v4l2_mbus_config sensor_mbus_cfg;
- struct v4l2_fwnode_endpoint *sensor_ep;
+ struct v4l2_mbus_config mbus_cfg;
struct v4l2_mbus_framefmt if_fmt;
infmt = &priv->format_mbus[CSI_SINK_PAD];
outfmt = &priv->format_mbus[priv->active_output_pad];
- sensor_ep = &priv->sensor->sensor_ep;
- /* compose mbus_config from sensor endpoint */
- sensor_mbus_cfg.type = sensor_ep->bus_type;
- sensor_mbus_cfg.flags = (sensor_ep->bus_type == V4L2_MBUS_CSI2) ?
- sensor_ep->bus.mipi_csi2.flags :
- sensor_ep->bus.parallel.flags;
+ /* compose mbus_config from the upstream endpoint */
+ mbus_cfg.type = priv->upstream_ep.bus_type;
+ mbus_cfg.flags = (priv->upstream_ep.bus_type == V4L2_MBUS_CSI2) ?
+ priv->upstream_ep.bus.mipi_csi2.flags :
+ priv->upstream_ep.bus.parallel.flags;
/*
- * we need to pass input sensor frame to CSI interface, but
+ * we need to pass input frame to CSI interface, but
* with translated field type from output format
*/
if_fmt = *infmt;
@@ -599,7 +658,7 @@ static int csi_setup(struct csi_priv *priv)
priv->crop.width == 2 * priv->compose.width,
priv->crop.height == 2 * priv->compose.height);
- ipu_csi_init_interface(priv->csi, &sensor_mbus_cfg, &if_fmt);
+ ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
ipu_csi_set_dest(priv->csi, priv->dest);
@@ -615,35 +674,11 @@ static int csi_setup(struct csi_priv *priv)
static int csi_start(struct csi_priv *priv)
{
struct v4l2_fract *output_fi, *input_fi;
- u32 bad_frames = 0;
int ret;
- if (!priv->sensor) {
- v4l2_err(&priv->sd, "no sensor attached\n");
- return -EINVAL;
- }
-
output_fi = &priv->frame_interval[priv->active_output_pad];
input_fi = &priv->frame_interval[CSI_SINK_PAD];
- ret = v4l2_subdev_call(priv->sensor->sd, sensor,
- g_skip_frames, &bad_frames);
- if (!ret && bad_frames) {
- u32 delay_usec;
-
- /*
- * This sensor has bad frames when it is turned on,
- * add a delay to avoid them before enabling the CSI
- * hardware. Especially for sensors with a bt.656 interface,
- * any shifts in the SAV/EAV sync codes will cause the CSI
- * to lose vert/horiz sync.
- */
- delay_usec = DIV_ROUND_UP_ULL(
- (u64)USEC_PER_SEC * input_fi->numerator * bad_frames,
- input_fi->denominator);
- usleep_range(delay_usec, delay_usec + 1000);
- }
-
if (priv->dest == IPU_CSI_DEST_IDMAC) {
ret = csi_idmac_start(priv);
if (ret)
@@ -975,9 +1010,8 @@ static int csi_link_validate(struct v4l2_subdev *sd,
struct v4l2_subdev_format *sink_fmt)
{
struct csi_priv *priv = v4l2_get_subdevdata(sd);
- struct v4l2_fwnode_endpoint *sensor_ep;
+ struct v4l2_fwnode_endpoint upstream_ep;
const struct imx_media_pixfmt *incc;
- struct imx_media_subdev *sensor;
bool is_csi2;
int ret;
@@ -986,22 +1020,20 @@ static int csi_link_validate(struct v4l2_subdev *sd,
if (ret)
return ret;
- sensor = __imx_media_find_sensor(priv->md, &priv->sd.entity);
- if (IS_ERR(sensor)) {
- v4l2_err(&priv->sd, "no sensor attached\n");
- return PTR_ERR(priv->sensor);
+ ret = csi_get_upstream_endpoint(priv, &upstream_ep);
+ if (ret) {
+ v4l2_err(&priv->sd, "failed to find upstream endpoint\n");
+ return ret;
}
mutex_lock(&priv->lock);
- priv->sensor = sensor;
- sensor_ep = &priv->sensor->sensor_ep;
- is_csi2 = (sensor_ep->bus_type == V4L2_MBUS_CSI2);
+ priv->upstream_ep = upstream_ep;
+ is_csi2 = (upstream_ep.bus_type == V4L2_MBUS_CSI2);
incc = priv->cc[CSI_SINK_PAD];
if (priv->dest != IPU_CSI_DEST_IDMAC &&
- (incc->bayer || (!is_csi2 &&
- sensor_ep->bus.parallel.bus_width >= 16))) {
+ (incc->bayer || is_parallel_16bit_bus(&upstream_ep))) {
v4l2_err(&priv->sd,
"bayer/16-bit parallel buses must go to IDMAC pad\n");
ret = -EINVAL;
@@ -1071,12 +1103,8 @@ static void csi_try_crop(struct csi_priv *priv,
struct v4l2_rect *crop,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_mbus_framefmt *infmt,
- struct imx_media_subdev *sensor)
+ struct v4l2_fwnode_endpoint *upstream_ep)
{
- struct v4l2_fwnode_endpoint *sensor_ep;
-
- sensor_ep = &sensor->sensor_ep;
-
crop->width = min_t(__u32, infmt->width, crop->width);
if (crop->left + crop->width > infmt->width)
crop->left = infmt->width - crop->width;
@@ -1090,7 +1118,7 @@ static void csi_try_crop(struct csi_priv *priv,
* sync, so fix it to NTSC/PAL active lines. NTSC contains
* 2 extra lines of active video that need to be cropped.
*/
- if (sensor_ep->bus_type == V4L2_MBUS_BT656 &&
+ if (upstream_ep->bus_type == V4L2_MBUS_BT656 &&
(V4L2_FIELD_HAS_BOTH(infmt->field) ||
infmt->field == V4L2_FIELD_ALTERNATE)) {
crop->height = infmt->height;
@@ -1240,7 +1268,7 @@ static int csi_get_fmt(struct v4l2_subdev *sd,
}
static void csi_try_fmt(struct csi_priv *priv,
- struct imx_media_subdev *sensor,
+ struct v4l2_fwnode_endpoint *upstream_ep,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *sdformat,
struct v4l2_rect *crop,
@@ -1308,7 +1336,7 @@ static void csi_try_fmt(struct csi_priv *priv,
crop->top = 0;
crop->width = sdformat->format.width;
crop->height = sdformat->format.height;
- csi_try_crop(priv, crop, cfg, &sdformat->format, sensor);
+ csi_try_crop(priv, crop, cfg, &sdformat->format, upstream_ep);
compose->left = 0;
compose->top = 0;
compose->width = crop->width;
@@ -1337,20 +1365,20 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
{
struct csi_priv *priv = v4l2_get_subdevdata(sd);
struct imx_media_video_dev *vdev = priv->vdev;
+ struct v4l2_fwnode_endpoint upstream_ep;
const struct imx_media_pixfmt *cc;
- struct imx_media_subdev *sensor;
struct v4l2_pix_format vdev_fmt;
struct v4l2_mbus_framefmt *fmt;
struct v4l2_rect *crop, *compose;
- int ret = 0;
+ int ret;
if (sdformat->pad >= CSI_NUM_PADS)
return -EINVAL;
- sensor = imx_media_find_sensor(priv->md, &priv->sd.entity);
- if (IS_ERR(sensor)) {
- v4l2_err(&priv->sd, "no sensor attached\n");
- return PTR_ERR(sensor);
+ ret = csi_get_upstream_endpoint(priv, &upstream_ep);
+ if (ret) {
+ v4l2_err(&priv->sd, "failed to find upstream endpoint\n");
+ return ret;
}
mutex_lock(&priv->lock);
@@ -1363,7 +1391,7 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
crop = __csi_get_crop(priv, cfg, sdformat->which);
compose = __csi_get_compose(priv, cfg, sdformat->which);
- csi_try_fmt(priv, sensor, cfg, sdformat, crop, compose, &cc);
+ csi_try_fmt(priv, &upstream_ep, cfg, sdformat, crop, compose, &cc);
fmt = __csi_get_fmt(priv, cfg, sdformat->pad, sdformat->which);
*fmt = sdformat->format;
@@ -1380,8 +1408,8 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
format.pad = pad;
format.which = sdformat->which;
format.format = sdformat->format;
- csi_try_fmt(priv, sensor, cfg, &format, NULL, compose,
- &outcc);
+ csi_try_fmt(priv, &upstream_ep, cfg, &format,
+ NULL, compose, &outcc);
outfmt = __csi_get_fmt(priv, cfg, pad, sdformat->which);
*outfmt = format.format;
@@ -1476,18 +1504,18 @@ static int csi_set_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_selection *sel)
{
struct csi_priv *priv = v4l2_get_subdevdata(sd);
+ struct v4l2_fwnode_endpoint upstream_ep;
struct v4l2_mbus_framefmt *infmt;
struct v4l2_rect *crop, *compose;
- struct imx_media_subdev *sensor;
- int pad, ret = 0;
+ int pad, ret;
if (sel->pad != CSI_SINK_PAD)
return -EINVAL;
- sensor = imx_media_find_sensor(priv->md, &priv->sd.entity);
- if (IS_ERR(sensor)) {
- v4l2_err(&priv->sd, "no sensor attached\n");
- return PTR_ERR(sensor);
+ ret = csi_get_upstream_endpoint(priv, &upstream_ep);
+ if (ret) {
+ v4l2_err(&priv->sd, "failed to find upstream endpoint\n");
+ return ret;
}
mutex_lock(&priv->lock);
@@ -1515,7 +1543,7 @@ static int csi_set_selection(struct v4l2_subdev *sd,
goto out;
}
- csi_try_crop(priv, &sel->r, cfg, infmt, sensor);
+ csi_try_crop(priv, &sel->r, cfg, infmt, &upstream_ep);
*crop = sel->r;
diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
index b55e5eb..701f8c9 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -222,18 +222,6 @@ static int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
ret = imx_media_get_ipu(imxmd, sd);
if (ret)
goto out_unlock;
- } else if (sd->entity.function == MEDIA_ENT_F_VID_MUX) {
- /* this is a video mux */
- sd->grp_id = IMX_MEDIA_GRP_ID_VIDMUX;
- } else if (imxsd->num_sink_pads == 0) {
- /*
- * this is an original source of video frames, it
- * could be a camera sensor, an analog decoder, or
- * a bridge device (HDMI -> MIPI CSI-2 for example).
- * This group ID is used to locate the entity that
- * is the original source of video in a pipeline.
- */
- sd->grp_id = IMX_MEDIA_GRP_ID_SENSOR;
}
/* attach the subdev */
diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
index 12df09f..883ad85 100644
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -34,20 +34,6 @@ static int of_add_pad_link(struct imx_media_dev *imxmd,
local_pad, remote_pad);
}
-static void of_parse_sensor(struct imx_media_dev *imxmd,
- struct imx_media_subdev *sensor,
- struct device_node *sensor_np)
-{
- struct device_node *endpoint;
-
- endpoint = of_graph_get_next_endpoint(sensor_np, NULL);
- if (endpoint) {
- v4l2_fwnode_endpoint_parse(of_fwnode_handle(endpoint),
- &sensor->sensor_ep);
- of_node_put(endpoint);
- }
-}
-
static int of_get_port_count(const struct device_node *np)
{
struct device_node *ports, *child;
@@ -172,13 +158,6 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np,
__func__, sd_np->name, num_pads,
imxsd->num_sink_pads, imxsd->num_src_pads);
- /*
- * With no sink, this subdev node is the original source
- * of video, parse it's media bus for use by the pipeline.
- */
- if (imxsd->num_sink_pads == 0)
- of_parse_sensor(imxmd, imxsd, sd_np);
-
for (i = 0; i < num_pads; i++) {
struct device_node *epnode = NULL, *port, *remote_np;
struct imx_media_subdev *remote_imxsd;
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 5952387..51d7e18 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -730,10 +730,11 @@ int imx_media_add_video_device(struct imx_media_dev *imxmd,
EXPORT_SYMBOL_GPL(imx_media_add_video_device);
/*
- * Search upstream or downstream for a subdevice in the current pipeline
+ * Search upstream/downstream for a subdevice in the current pipeline
* with given grp_id, starting from start_entity. Returns the subdev's
- * source/sink pad that it was reached from. Must be called with
- * mdev->graph_mutex held.
+ * source/sink pad that it was reached from. If grp_id is zero, just
+ * returns the nearest source/sink pad to start_entity. Must be called
+ * with mdev->graph_mutex held.
*/
static struct media_pad *
find_pipeline_pad(struct imx_media_dev *imxmd,
@@ -756,11 +757,16 @@ find_pipeline_pad(struct imx_media_dev *imxmd,
if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
continue;
- sd = media_entity_to_v4l2_subdev(pad->entity);
- if (sd->grp_id & grp_id)
- return pad;
+ if (grp_id != 0) {
+ sd = media_entity_to_v4l2_subdev(pad->entity);
+ if (sd->grp_id & grp_id)
+ return pad;
- return find_pipeline_pad(imxmd, pad->entity, grp_id, upstream);
+ return find_pipeline_pad(imxmd, pad->entity,
+ grp_id, upstream);
+ } else {
+ return pad;
+ }
}
return NULL;
@@ -789,7 +795,6 @@ find_upstream_subdev(struct imx_media_dev *imxmd,
return pad ? media_entity_to_v4l2_subdev(pad->entity) : NULL;
}
-
/*
* Find the upstream mipi-csi2 virtual channel reached from the given
* start entity in the current pipeline.
@@ -814,6 +819,25 @@ int imx_media_find_mipi_csi2_channel(struct imx_media_dev *imxmd,
EXPORT_SYMBOL_GPL(imx_media_find_mipi_csi2_channel);
/*
+ * Find a source pad reached upstream from the given start entity in
+ * the current pipeline. Must be called with mdev->graph_mutex held.
+ */
+struct media_pad *
+imx_media_find_upstream_pad(struct imx_media_dev *imxmd,
+ struct media_entity *start_entity,
+ u32 grp_id)
+{
+ struct media_pad *pad;
+
+ pad = find_pipeline_pad(imxmd, start_entity, grp_id, true);
+ if (!pad)
+ return ERR_PTR(-ENODEV);
+
+ return pad;
+}
+EXPORT_SYMBOL_GPL(imx_media_find_upstream_pad);
+
+/*
* Find a subdev reached upstream from the given start entity in
* the current pipeline.
* Must be called with mdev->graph_mutex held.
@@ -833,29 +857,6 @@ imx_media_find_upstream_subdev(struct imx_media_dev *imxmd,
}
EXPORT_SYMBOL_GPL(imx_media_find_upstream_subdev);
-struct imx_media_subdev *
-__imx_media_find_sensor(struct imx_media_dev *imxmd,
- struct media_entity *start_entity)
-{
- return imx_media_find_upstream_subdev(imxmd, start_entity,
- IMX_MEDIA_GRP_ID_SENSOR);
-}
-EXPORT_SYMBOL_GPL(__imx_media_find_sensor);
-
-struct imx_media_subdev *
-imx_media_find_sensor(struct imx_media_dev *imxmd,
- struct media_entity *start_entity)
-{
- struct imx_media_subdev *sensor;
-
- mutex_lock(&imxmd->md.graph_mutex);
- sensor = __imx_media_find_sensor(imxmd, start_entity);
- mutex_unlock(&imxmd->md.graph_mutex);
-
- return sensor;
-}
-EXPORT_SYMBOL_GPL(imx_media_find_sensor);
-
/*
* Turn current pipeline streaming on/off starting from entity.
*/
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index d409170..17422f9 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -158,9 +158,6 @@ struct imx_media_subdev {
struct platform_device *pdev;
/* the devname is needed for async devname match */
char devname[32];
-
- /* if this is a sensor */
- struct v4l2_fwnode_endpoint sensor_ep;
};
struct imx_media_dev {
@@ -251,16 +248,14 @@ int imx_media_add_video_device(struct imx_media_dev *imxmd,
struct imx_media_video_dev *vdev);
int imx_media_find_mipi_csi2_channel(struct imx_media_dev *imxmd,
struct media_entity *start_entity);
+struct media_pad *
+imx_media_find_upstream_pad(struct imx_media_dev *imxmd,
+ struct media_entity *start_entity,
+ u32 grp_id);
struct imx_media_subdev *
imx_media_find_upstream_subdev(struct imx_media_dev *imxmd,
struct media_entity *start_entity,
u32 grp_id);
-struct imx_media_subdev *
-__imx_media_find_sensor(struct imx_media_dev *imxmd,
- struct media_entity *start_entity);
-struct imx_media_subdev *
-imx_media_find_sensor(struct imx_media_dev *imxmd,
- struct media_entity *start_entity);
struct imx_media_dma_buf {
void *virt;
@@ -310,16 +305,14 @@ void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
void imx_media_capture_device_error(struct imx_media_video_dev *vdev);
/* subdev group ids */
-#define IMX_MEDIA_GRP_ID_SENSOR (1 << 8)
-#define IMX_MEDIA_GRP_ID_VIDMUX (1 << 9)
-#define IMX_MEDIA_GRP_ID_CSI2 (1 << 10)
-#define IMX_MEDIA_GRP_ID_CSI_BIT 11
+#define IMX_MEDIA_GRP_ID_CSI2 (1 << 8)
+#define IMX_MEDIA_GRP_ID_CSI_BIT 9
#define IMX_MEDIA_GRP_ID_CSI (0x3 << IMX_MEDIA_GRP_ID_CSI_BIT)
#define IMX_MEDIA_GRP_ID_CSI0 (1 << IMX_MEDIA_GRP_ID_CSI_BIT)
#define IMX_MEDIA_GRP_ID_CSI1 (2 << IMX_MEDIA_GRP_ID_CSI_BIT)
-#define IMX_MEDIA_GRP_ID_VDIC (1 << 13)
-#define IMX_MEDIA_GRP_ID_IC_PRP (1 << 14)
-#define IMX_MEDIA_GRP_ID_IC_PRPENC (1 << 15)
-#define IMX_MEDIA_GRP_ID_IC_PRPVF (1 << 16)
+#define IMX_MEDIA_GRP_ID_VDIC (1 << 11)
+#define IMX_MEDIA_GRP_ID_IC_PRP (1 << 12)
+#define IMX_MEDIA_GRP_ID_IC_PRPENC (1 << 13)
+#define IMX_MEDIA_GRP_ID_IC_PRPVF (1 << 14)
#endif
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 2/9] media: staging/imx: remove static media link arrays
2017-10-28 20:36 [PATCH 0/9] media: imx: Add better OF graph support Steve Longerbeam
2017-10-28 20:36 ` [PATCH 1/9] media: staging/imx: get CSI bus type from nearest upstream entity Steve Longerbeam
@ 2017-10-28 20:36 ` Steve Longerbeam
2017-10-28 20:36 ` [PATCH 3/9] media: staging/imx: of: allow for recursing downstream Steve Longerbeam
` (7 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Steve Longerbeam @ 2017-10-28 20:36 UTC (permalink / raw)
To: Philipp Zabel, Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: linux-media, devel, linux-kernel, Russell King, Steve Longerbeam
Remove the static list of media links that were formed at probe time.
These links can instead be created after all registered async subdevices
have been bound in imx_media_probe_complete().
The media links between subdevices that exist in the device tree, can
be created post-async completion by using v4l2_fwnode_parse_link() for
each endpoint node of that subdevice. Note this approach assumes
device-tree ports are equivalent to media pads (pad index equals
port id), and that device-tree endpoints are equivalent to media
links between pads.
Because links are no longer parsed by imx_media_of_parse(), its sole
function is now only to add subdevices that it encounters by walking
the OF graph to the async list, so the function has been renamed
imx_media_add_of_subdevs().
Similarly, the media links between the IPU-internal subdevice pads (the
CSI source pads, and all pads between the vdic, ic-prp, ic-prpenc, and
ic-prpvf subdevices), can be created post-async completion by looping
through the subdevice's media pads and using the const internal_subdev
table.
Because links are no longer parsed by imx_media_add_internal_subdevs(),
this function no longer needs an array of CSI subdevs to form links
from.
In summary, the following functions, which were used to form a list
of media links at probe time, are removed:
imx_media_add_pad_link()
add_internal_links()
of_add_pad_link()
replaced by these functions, called at probe time, which only populate
the async subdev list:
imx_media_add_of_subdevs()
imx_media_add_internal_subdevs()
and these functions, called at async completion, which create the
media links:
imx_media_create_of_links()
imx_media_create_csi_of_links()
imx_media_create_internal_links()
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/staging/media/imx/imx-media-dev.c | 130 +++----------
drivers/staging/media/imx/imx-media-internal-sd.c | 219 +++++++++++++---------
drivers/staging/media/imx/imx-media-of.c | 189 ++++++++++++-------
drivers/staging/media/imx/imx-media.h | 39 +---
4 files changed, 281 insertions(+), 296 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
index 701f8c9..f63808f 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -11,6 +11,7 @@
#include <linux/delay.h>
#include <linux/fs.h>
#include <linux/module.h>
+#include <linux/of_graph.h>
#include <linux/of_platform.h>
#include <linux/pinctrl/consumer.h>
#include <linux/platform_device.h>
@@ -128,50 +129,6 @@ imx_media_add_async_subdev(struct imx_media_dev *imxmd,
}
/*
- * Adds an imx-media link to a subdev pad's link list. This is called
- * during driver load when forming the links between subdevs.
- *
- * @pad: the local pad
- * @remote_node: the device node of the remote subdev
- * @remote_devname: the device name of the remote subdev
- * @local_pad: local pad index
- * @remote_pad: remote pad index
- */
-int imx_media_add_pad_link(struct imx_media_dev *imxmd,
- struct imx_media_pad *pad,
- struct device_node *remote_node,
- const char *remote_devname,
- int local_pad, int remote_pad)
-{
- struct imx_media_link *link;
- int link_idx, ret = 0;
-
- mutex_lock(&imxmd->mutex);
-
- link_idx = pad->num_links;
- if (link_idx >= IMX_MEDIA_MAX_LINKS) {
- dev_err(imxmd->md.dev, "%s: too many links!\n", __func__);
- ret = -ENOSPC;
- goto out;
- }
-
- link = &pad->link[link_idx];
-
- link->remote_sd_node = remote_node;
- if (remote_devname)
- strncpy(link->remote_devname, remote_devname,
- sizeof(link->remote_devname));
-
- link->local_pad = local_pad;
- link->remote_pad = remote_pad;
-
- pad->num_links++;
-out:
- mutex_unlock(&imxmd->mutex);
- return ret;
-}
-
-/*
* get IPU from this CSI and add it to the list of IPUs
* the media driver will control.
*/
@@ -240,76 +197,38 @@ static int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
}
/*
- * Create a single source->sink media link given a subdev and a single
- * link from one of its source pads. Called after all subdevs have
- * registered.
- */
-static int imx_media_create_link(struct imx_media_dev *imxmd,
- struct imx_media_subdev *src,
- struct imx_media_link *link)
-{
- struct imx_media_subdev *sink;
- u16 source_pad, sink_pad;
- int ret;
-
- sink = imx_media_find_async_subdev(imxmd, link->remote_sd_node,
- link->remote_devname);
- if (!sink) {
- v4l2_warn(&imxmd->v4l2_dev, "%s: no sink for %s:%d\n",
- __func__, src->sd->name, link->local_pad);
- return 0;
- }
-
- source_pad = link->local_pad;
- sink_pad = link->remote_pad;
-
- v4l2_info(&imxmd->v4l2_dev, "%s: %s:%d -> %s:%d\n", __func__,
- src->sd->name, source_pad, sink->sd->name, sink_pad);
-
- ret = media_create_pad_link(&src->sd->entity, source_pad,
- &sink->sd->entity, sink_pad, 0);
- if (ret)
- v4l2_err(&imxmd->v4l2_dev,
- "create_pad_link failed: %d\n", ret);
-
- return ret;
-}
-
-/*
- * create the media links from all imx-media pads and their links.
+ * create the media links from all pads and their links.
* Called after all subdevs have registered.
*/
static int imx_media_create_links(struct imx_media_dev *imxmd)
{
struct imx_media_subdev *imxsd;
- struct imx_media_link *link;
- struct imx_media_pad *pad;
- int num_pads, i, j, k;
- int ret = 0;
+ struct v4l2_subdev *sd;
+ int i, ret;
for (i = 0; i < imxmd->num_subdevs; i++) {
imxsd = &imxmd->subdev[i];
- num_pads = imxsd->num_sink_pads + imxsd->num_src_pads;
-
- for (j = 0; j < num_pads; j++) {
- pad = &imxsd->pad[j];
-
- /* only create the source->sink links */
- if (!(pad->pad.flags & MEDIA_PAD_FL_SOURCE))
- continue;
-
- for (k = 0; k < pad->num_links; k++) {
- link = &pad->link[k];
+ sd = imxsd->sd;
- ret = imx_media_create_link(imxmd, imxsd, link);
- if (ret)
- goto out;
- }
+ if (((sd->grp_id & IMX_MEDIA_GRP_ID_CSI) || imxsd->pdev)) {
+ /* this is an internal subdev or a CSI */
+ ret = imx_media_create_internal_links(imxmd, imxsd);
+ if (ret)
+ return ret;
+ /*
+ * the CSIs straddle between the external and the IPU
+ * internal entities, so create the external links
+ * to the CSI sink pads.
+ */
+ if (sd->grp_id & IMX_MEDIA_GRP_ID_CSI)
+ imx_media_create_csi_of_links(imxmd, imxsd);
+ } else {
+ /* this is an external fwnode subdev */
+ imx_media_create_of_links(imxmd, imxsd);
}
}
-out:
- return ret;
+ return 0;
}
/*
@@ -542,7 +461,6 @@ static int imx_media_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct device_node *node = dev->of_node;
- struct imx_media_subdev *csi[4] = {0};
struct imx_media_dev *imxmd;
int ret;
@@ -573,14 +491,14 @@ static int imx_media_probe(struct platform_device *pdev)
dev_set_drvdata(imxmd->v4l2_dev.dev, imxmd);
- ret = imx_media_of_parse(imxmd, &csi, node);
+ ret = imx_media_add_of_subdevs(imxmd, node);
if (ret) {
v4l2_err(&imxmd->v4l2_dev,
- "imx_media_of_parse failed with %d\n", ret);
+ "add_of_subdevs failed with %d\n", ret);
goto unreg_dev;
}
- ret = imx_media_add_internal_subdevs(imxmd, csi);
+ ret = imx_media_add_internal_subdevs(imxmd);
if (ret) {
v4l2_err(&imxmd->v4l2_dev,
"add_internal_subdevs failed with %d\n", ret);
diff --git a/drivers/staging/media/imx/imx-media-internal-sd.c b/drivers/staging/media/imx/imx-media-internal-sd.c
index cdfbf40..3e60df5 100644
--- a/drivers/staging/media/imx/imx-media-internal-sd.c
+++ b/drivers/staging/media/imx/imx-media-internal-sd.c
@@ -60,14 +60,19 @@ static const struct internal_subdev_id {
},
};
+struct internal_subdev;
+
struct internal_link {
- const struct internal_subdev_id *remote_id;
+ const struct internal_subdev *remote;
+ int local_pad;
int remote_pad;
};
+/* max links per internal-sd pad */
+#define MAX_INTERNAL_LINKS 8
+
struct internal_pad {
- bool devnode; /* does this pad link to a device node */
- struct internal_link link[IMX_MEDIA_MAX_LINKS];
+ struct internal_link link[MAX_INTERNAL_LINKS];
};
static const struct internal_subdev {
@@ -75,7 +80,7 @@ static const struct internal_subdev {
struct internal_pad pad[IMX_MEDIA_MAX_PADS];
int num_sink_pads;
int num_src_pads;
-} internal_subdev[num_isd] = {
+} int_subdev[num_isd] = {
[isd_csi0] = {
.id = &isd_id[isd_csi0],
.num_sink_pads = CSI_NUM_SINK_PADS,
@@ -83,17 +88,16 @@ static const struct internal_subdev {
.pad[CSI_SRC_PAD_DIRECT] = {
.link = {
{
- .remote_id = &isd_id[isd_ic_prp],
+ .local_pad = CSI_SRC_PAD_DIRECT,
+ .remote = &int_subdev[isd_ic_prp],
.remote_pad = PRP_SINK_PAD,
}, {
- .remote_id = &isd_id[isd_vdic],
+ .local_pad = CSI_SRC_PAD_DIRECT,
+ .remote = &int_subdev[isd_vdic],
.remote_pad = VDIC_SINK_PAD_DIRECT,
},
},
},
- .pad[CSI_SRC_PAD_IDMAC] = {
- .devnode = true,
- },
},
[isd_csi1] = {
@@ -103,30 +107,27 @@ static const struct internal_subdev {
.pad[CSI_SRC_PAD_DIRECT] = {
.link = {
{
- .remote_id = &isd_id[isd_ic_prp],
+ .local_pad = CSI_SRC_PAD_DIRECT,
+ .remote = &int_subdev[isd_ic_prp],
.remote_pad = PRP_SINK_PAD,
}, {
- .remote_id = &isd_id[isd_vdic],
+ .local_pad = CSI_SRC_PAD_DIRECT,
+ .remote = &int_subdev[isd_vdic],
.remote_pad = VDIC_SINK_PAD_DIRECT,
},
},
},
- .pad[CSI_SRC_PAD_IDMAC] = {
- .devnode = true,
- },
},
[isd_vdic] = {
.id = &isd_id[isd_vdic],
.num_sink_pads = VDIC_NUM_SINK_PADS,
.num_src_pads = VDIC_NUM_SRC_PADS,
- .pad[VDIC_SINK_PAD_IDMAC] = {
- .devnode = true,
- },
.pad[VDIC_SRC_PAD_DIRECT] = {
.link = {
{
- .remote_id = &isd_id[isd_ic_prp],
+ .local_pad = VDIC_SRC_PAD_DIRECT,
+ .remote = &int_subdev[isd_ic_prp],
.remote_pad = PRP_SINK_PAD,
},
},
@@ -140,7 +141,8 @@ static const struct internal_subdev {
.pad[PRP_SRC_PAD_PRPENC] = {
.link = {
{
- .remote_id = &isd_id[isd_ic_prpenc],
+ .local_pad = PRP_SRC_PAD_PRPENC,
+ .remote = &int_subdev[isd_ic_prpenc],
.remote_pad = 0,
},
},
@@ -148,7 +150,8 @@ static const struct internal_subdev {
.pad[PRP_SRC_PAD_PRPVF] = {
.link = {
{
- .remote_id = &isd_id[isd_ic_prpvf],
+ .local_pad = PRP_SRC_PAD_PRPVF,
+ .remote = &int_subdev[isd_ic_prpvf],
.remote_pad = 0,
},
},
@@ -159,68 +162,114 @@ static const struct internal_subdev {
.id = &isd_id[isd_ic_prpenc],
.num_sink_pads = PRPENCVF_NUM_SINK_PADS,
.num_src_pads = PRPENCVF_NUM_SRC_PADS,
- .pad[PRPENCVF_SRC_PAD] = {
- .devnode = true,
- },
},
[isd_ic_prpvf] = {
.id = &isd_id[isd_ic_prpvf],
.num_sink_pads = PRPENCVF_NUM_SINK_PADS,
.num_src_pads = PRPENCVF_NUM_SRC_PADS,
- .pad[PRPENCVF_SRC_PAD] = {
- .devnode = true,
- },
},
};
-/* form a device name given a group id and ipu id */
-static inline void isd_id_to_devname(char *devname, int sz,
- const struct internal_subdev_id *id,
- int ipu_id)
+/* form a device name given an internal subdev and ipu id */
+static inline void isd_to_devname(char *devname, int sz,
+ const struct internal_subdev *isd,
+ int ipu_id)
{
- int pdev_id = ipu_id * num_isd + id->index;
+ int pdev_id = ipu_id * num_isd + isd->id->index;
- snprintf(devname, sz, "%s.%d", id->name, pdev_id);
+ snprintf(devname, sz, "%s.%d", isd->id->name, pdev_id);
}
-/* adds the links from given internal subdev */
-static int add_internal_links(struct imx_media_dev *imxmd,
- const struct internal_subdev *isd,
- struct imx_media_subdev *imxsd,
- int ipu_id)
+static const struct internal_subdev *find_intsd_by_grp_id(u32 grp_id)
{
- int i, num_pads, ret;
+ enum isd_enum i;
+
+ for (i = 0; i < num_isd; i++) {
+ const struct internal_subdev *isd = &int_subdev[i];
- num_pads = isd->num_sink_pads + isd->num_src_pads;
+ if (isd->id->grp_id == grp_id)
+ return isd;
+ }
- for (i = 0; i < num_pads; i++) {
- const struct internal_pad *intpad = &isd->pad[i];
- struct imx_media_pad *pad = &imxsd->pad[i];
- int j;
+ return NULL;
+}
- /* init the pad flags for this internal subdev */
- pad->pad.flags = (i < isd->num_sink_pads) ?
- MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
- /* export devnode pad flag to the subdevs */
- pad->devnode = intpad->devnode;
+static struct imx_media_subdev *find_sink(struct imx_media_dev *imxmd,
+ struct imx_media_subdev *src,
+ const struct internal_link *link)
+{
+ char sink_devname[32];
+ int ipu_id;
+
+ /*
+ * retrieve IPU id from subdev name, note: can't get this from
+ * struct imx_media_internal_sd_platformdata because if src is
+ * a CSI, it has different struct ipu_client_platformdata which
+ * does not contain IPU id.
+ */
+ if (sscanf(src->sd->name, "ipu%d", &ipu_id) != 1)
+ return NULL;
+
+ isd_to_devname(sink_devname, sizeof(sink_devname),
+ link->remote, ipu_id - 1);
+
+ return imx_media_find_async_subdev(imxmd, NULL, sink_devname);
+}
- for (j = 0; ; j++) {
- const struct internal_link *link;
- char remote_devname[32];
+static int create_ipu_internal_link(struct imx_media_dev *imxmd,
+ struct imx_media_subdev *src,
+ const struct internal_link *link)
+{
+ struct imx_media_subdev *sink;
+ int ret;
+
+ sink = find_sink(imxmd, src, link);
+ if (!sink)
+ return -ENODEV;
+ v4l2_info(&imxmd->v4l2_dev, "%s:%d -> %s:%d\n",
+ src->sd->name, link->local_pad,
+ sink->sd->name, link->remote_pad);
+
+ ret = media_create_pad_link(&src->sd->entity, link->local_pad,
+ &sink->sd->entity, link->remote_pad, 0);
+ if (ret)
+ v4l2_err(&imxmd->v4l2_dev,
+ "create_pad_link failed: %d\n", ret);
+
+ return ret;
+}
+
+int imx_media_create_internal_links(struct imx_media_dev *imxmd,
+ struct imx_media_subdev *imxsd)
+{
+ struct v4l2_subdev *sd = imxsd->sd;
+ const struct internal_subdev *intsd;
+ const struct internal_pad *intpad;
+ const struct internal_link *link;
+ struct media_pad *pad;
+ int i, j, ret;
+
+ intsd = find_intsd_by_grp_id(imxsd->sd->grp_id);
+ if (!intsd)
+ return -ENODEV;
+
+ /* create the source->sink links */
+ for (i = 0; i < sd->entity.num_pads; i++) {
+ intpad = &intsd->pad[i];
+ pad = &sd->entity.pads[i];
+
+ if (!(pad->flags & MEDIA_PAD_FL_SOURCE))
+ continue;
+
+ for (j = 0; ; j++) {
link = &intpad->link[j];
- if (!link->remote_id)
+ if (!link->remote)
break;
- isd_id_to_devname(remote_devname,
- sizeof(remote_devname),
- link->remote_id, ipu_id);
-
- ret = imx_media_add_pad_link(imxmd, pad,
- NULL, remote_devname,
- i, link->remote_pad);
+ ret = create_ipu_internal_link(imxmd, imxsd, link);
if (ret)
return ret;
}
@@ -230,10 +279,9 @@ static int add_internal_links(struct imx_media_dev *imxmd,
}
/* register an internal subdev as a platform device */
-static struct imx_media_subdev *
-add_internal_subdev(struct imx_media_dev *imxmd,
- const struct internal_subdev *isd,
- int ipu_id)
+static int add_internal_subdev(struct imx_media_dev *imxmd,
+ const struct internal_subdev *isd,
+ int ipu_id)
{
struct imx_media_internal_sd_platformdata pdata;
struct platform_device_info pdevinfo = {0};
@@ -258,73 +306,58 @@ add_internal_subdev(struct imx_media_dev *imxmd,
pdev = platform_device_register_full(&pdevinfo);
if (IS_ERR(pdev))
- return ERR_CAST(pdev);
+ return PTR_ERR(pdev);
imxsd = imx_media_add_async_subdev(imxmd, NULL, pdev);
if (IS_ERR(imxsd))
- return imxsd;
+ return PTR_ERR(imxsd);
imxsd->num_sink_pads = isd->num_sink_pads;
imxsd->num_src_pads = isd->num_src_pads;
- return imxsd;
+ return 0;
}
/* adds the internal subdevs in one ipu */
-static int add_ipu_internal_subdevs(struct imx_media_dev *imxmd,
- struct imx_media_subdev *csi0,
- struct imx_media_subdev *csi1,
- int ipu_id)
+static int add_ipu_internal_subdevs(struct imx_media_dev *imxmd, int ipu_id)
{
enum isd_enum i;
- int ret;
for (i = 0; i < num_isd; i++) {
- const struct internal_subdev *isd = &internal_subdev[i];
- struct imx_media_subdev *imxsd;
+ const struct internal_subdev *isd = &int_subdev[i];
+ int ret;
/*
* the CSIs are represented in the device-tree, so those
- * devices are added already, and are added to the async
- * subdev list by of_parse_subdev(), so we are given those
- * subdevs as csi0 and csi1.
+ * devices are already added to the async subdev list by
+ * of_parse_subdev().
*/
switch (isd->id->grp_id) {
case IMX_MEDIA_GRP_ID_CSI0:
- imxsd = csi0;
- break;
case IMX_MEDIA_GRP_ID_CSI1:
- imxsd = csi1;
+ ret = 0;
break;
default:
- imxsd = add_internal_subdev(imxmd, isd, ipu_id);
+ ret = add_internal_subdev(imxmd, isd, ipu_id);
break;
}
- if (IS_ERR(imxsd))
- return PTR_ERR(imxsd);
-
- /* add the links from this subdev */
- if (imxsd) {
- ret = add_internal_links(imxmd, isd, imxsd, ipu_id);
- if (ret)
- return ret;
- }
+ if (ret)
+ return ret;
}
return 0;
}
-int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd,
- struct imx_media_subdev *csi[4])
+int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd)
{
int ret;
- ret = add_ipu_internal_subdevs(imxmd, csi[0], csi[1], 0);
+ ret = add_ipu_internal_subdevs(imxmd, 0);
if (ret)
goto remove;
- ret = add_ipu_internal_subdevs(imxmd, csi[2], csi[3], 1);
+ ret = add_ipu_internal_subdevs(imxmd, 1);
if (ret)
goto remove;
diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
index 883ad85..d35c99e 100644
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -20,20 +20,6 @@
#include <video/imx-ipu-v3.h>
#include "imx-media.h"
-static int of_add_pad_link(struct imx_media_dev *imxmd,
- struct imx_media_pad *pad,
- struct device_node *local_sd_node,
- struct device_node *remote_sd_node,
- int local_pad, int remote_pad)
-{
- dev_dbg(imxmd->md.dev, "%s: adding %s:%d -> %s:%d\n", __func__,
- local_sd_node->name, local_pad,
- remote_sd_node->name, remote_pad);
-
- return imx_media_add_pad_link(imxmd, pad, remote_sd_node, NULL,
- local_pad, remote_pad);
-}
-
static int of_get_port_count(const struct device_node *np)
{
struct device_node *ports, *child;
@@ -53,12 +39,10 @@ static int of_get_port_count(const struct device_node *np)
}
/*
- * find the remote device node and remote port id (remote pad #)
- * given local endpoint node
+ * find the remote device node given local endpoint node
*/
-static void of_get_remote_pad(struct device_node *epnode,
- struct device_node **remote_node,
- int *remote_pad)
+static void of_get_remote(struct device_node *epnode,
+ struct device_node **remote_node)
{
struct device_node *rp, *rpp;
struct device_node *remote;
@@ -69,12 +53,9 @@ static void of_get_remote_pad(struct device_node *epnode,
if (of_device_is_compatible(rpp, "fsl,imx6q-ipu")) {
/* the remote is one of the CSI ports */
remote = rp;
- *remote_pad = 0;
of_node_put(rpp);
} else {
remote = rpp;
- if (of_property_read_u32(rp, "reg", remote_pad))
- *remote_pad = 0;
of_node_put(rp);
}
@@ -88,7 +69,7 @@ static void of_get_remote_pad(struct device_node *epnode,
static int
of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np,
- bool is_csi_port, struct imx_media_subdev **subdev)
+ bool is_csi_port)
{
struct imx_media_subdev *imxsd;
int i, num_pads, ret;
@@ -96,7 +77,6 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np,
if (!of_device_is_available(sd_np)) {
dev_dbg(imxmd->md.dev, "%s: %s not enabled\n", __func__,
sd_np->name);
- *subdev = NULL;
/* unavailable is not an error */
return 0;
}
@@ -107,14 +87,12 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np,
if (ret) {
if (ret == -EEXIST) {
/* already added, everything is fine */
- *subdev = NULL;
return 0;
}
/* other error, can't continue */
return ret;
}
- *subdev = imxsd;
if (is_csi_port) {
/*
@@ -160,14 +138,6 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np,
for (i = 0; i < num_pads; i++) {
struct device_node *epnode = NULL, *port, *remote_np;
- struct imx_media_subdev *remote_imxsd;
- struct imx_media_pad *pad;
- int remote_pad;
-
- /* init this pad */
- pad = &imxsd->pad[i];
- pad->pad.flags = (i < imxsd->num_sink_pads) ?
- MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
if (is_csi_port)
port = (i < imxsd->num_sink_pads) ? sd_np : NULL;
@@ -177,19 +147,13 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np,
continue;
for_each_child_of_node(port, epnode) {
- of_get_remote_pad(epnode, &remote_np, &remote_pad);
+ of_get_remote(epnode, &remote_np);
if (!remote_np)
continue;
- ret = of_add_pad_link(imxmd, pad, sd_np, remote_np,
- i, remote_pad);
- if (ret)
- break;
-
if (i < imxsd->num_sink_pads) {
/* follow sink endpoints upstream */
- ret = of_parse_subdev(imxmd, remote_np,
- false, &remote_imxsd);
+ ret = of_parse_subdev(imxmd, remote_np, false);
if (ret)
break;
}
@@ -209,13 +173,10 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np,
return ret;
}
-int imx_media_of_parse(struct imx_media_dev *imxmd,
- struct imx_media_subdev *(*csi)[4],
- struct device_node *np)
+int imx_media_add_of_subdevs(struct imx_media_dev *imxmd,
+ struct device_node *np)
{
- struct imx_media_subdev *lcsi;
struct device_node *csi_np;
- u32 ipu_id, csi_id;
int i, ret;
for (i = 0; ; i++) {
@@ -223,33 +184,125 @@ int imx_media_of_parse(struct imx_media_dev *imxmd,
if (!csi_np)
break;
- ret = of_parse_subdev(imxmd, csi_np, true, &lcsi);
+ ret = of_parse_subdev(imxmd, csi_np, true);
+ of_node_put(csi_np);
if (ret)
- goto err_put;
+ return ret;
+ }
- ret = of_property_read_u32(csi_np, "reg", &csi_id);
- if (ret) {
- dev_err(imxmd->md.dev,
- "%s: csi port missing reg property!\n",
- __func__);
- goto err_put;
- }
+ return 0;
+}
- ipu_id = of_alias_get_id(csi_np->parent, "ipu");
- of_node_put(csi_np);
+/*
+ * Create a single media link to/from imxsd using a fwnode link.
+ *
+ * NOTE: this function assumes an OF port node is equivalent to
+ * a media pad (port id equal to media pad index), and that an
+ * OF endpoint node is equivalent to a media link.
+ */
+static int create_of_link(struct imx_media_dev *imxmd,
+ struct imx_media_subdev *imxsd,
+ struct v4l2_fwnode_link *link)
+{
+ struct v4l2_subdev *sd = imxsd->sd;
+ struct imx_media_subdev *remote;
+ struct v4l2_subdev *src, *sink;
+ int src_pad, sink_pad;
- if (ipu_id > 1 || csi_id > 1) {
- dev_err(imxmd->md.dev,
- "%s: invalid ipu/csi id (%u/%u)\n",
- __func__, ipu_id, csi_id);
- return -EINVAL;
- }
+ if (link->local_port >= sd->entity.num_pads)
+ return -EINVAL;
+
+ remote = imx_media_find_async_subdev(imxmd,
+ to_of_node(link->remote_node),
+ NULL);
+ if (!remote)
+ return 0;
- (*csi)[ipu_id * 2 + csi_id] = lcsi;
+ if (sd->entity.pads[link->local_port].flags & MEDIA_PAD_FL_SINK) {
+ src = remote->sd;
+ src_pad = link->remote_port;
+ sink = sd;
+ sink_pad = link->local_port;
+ } else {
+ src = sd;
+ src_pad = link->local_port;
+ sink = remote->sd;
+ sink_pad = link->remote_port;
+ }
+
+ /* make sure link doesn't already exist before creating */
+ if (media_entity_find_link(&src->entity.pads[src_pad],
+ &sink->entity.pads[sink_pad]))
+ return 0;
+
+ v4l2_info(sd->v4l2_dev, "%s:%d -> %s:%d\n",
+ src->name, src_pad, sink->name, sink_pad);
+
+ return media_create_pad_link(&src->entity, src_pad,
+ &sink->entity, sink_pad, 0);
+}
+
+/*
+ * Create media links to/from imxsd using its device-tree endpoints.
+ */
+int imx_media_create_of_links(struct imx_media_dev *imxmd,
+ struct imx_media_subdev *imxsd)
+{
+ struct v4l2_subdev *sd = imxsd->sd;
+ struct v4l2_fwnode_link link;
+ struct device_node *ep;
+ int ret;
+
+ for_each_endpoint_of_node(sd->dev->of_node, ep) {
+ ret = v4l2_fwnode_parse_link(of_fwnode_handle(ep), &link);
+ if (ret)
+ continue;
+
+ ret = create_of_link(imxmd, imxsd, &link);
+ v4l2_fwnode_put_link(&link);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+/*
+ * Create media links to the given CSI subdevice's sink pads,
+ * using its device-tree endpoints.
+ */
+int imx_media_create_csi_of_links(struct imx_media_dev *imxmd,
+ struct imx_media_subdev *csi)
+{
+ struct device_node *csi_np = csi->sd->dev->of_node;
+ struct fwnode_handle *fwnode, *csi_ep;
+ struct v4l2_fwnode_link link;
+ struct device_node *ep;
+ int ret;
+
+ link.local_node = of_fwnode_handle(csi_np);
+ link.local_port = CSI_SINK_PAD;
+
+ for_each_child_of_node(csi_np, ep) {
+ csi_ep = of_fwnode_handle(ep);
+
+ fwnode = fwnode_graph_get_remote_endpoint(csi_ep);
+ if (!fwnode)
+ continue;
+
+ fwnode = fwnode_get_parent(fwnode);
+ fwnode_property_read_u32(fwnode, "reg", &link.remote_port);
+ fwnode = fwnode_get_next_parent(fwnode);
+ if (is_of_node(fwnode) &&
+ of_node_cmp(to_of_node(fwnode)->name, "ports") == 0)
+ fwnode = fwnode_get_next_parent(fwnode);
+ link.remote_node = fwnode;
+
+ ret = create_of_link(imxmd, csi, &link);
+ fwnode_handle_put(link.remote_node);
+ if (ret)
+ return ret;
}
return 0;
-err_put:
- of_node_put(csi_np);
- return ret;
}
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 17422f9..6c0e443 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -35,8 +35,6 @@
#define IMX_MEDIA_MAX_SUBDEVS 32
/* max pads per subdev */
#define IMX_MEDIA_MAX_PADS 16
-/* max links per pad */
-#define IMX_MEDIA_MAX_LINKS 8
/*
* Pad definitions for the subdevs with multiple source or
@@ -119,19 +117,7 @@ static inline struct imx_media_buffer *to_imx_media_vb(struct vb2_buffer *vb)
return container_of(vbuf, struct imx_media_buffer, vbuf);
}
-struct imx_media_link {
- struct device_node *remote_sd_node;
- char remote_devname[32];
- int local_pad;
- int remote_pad;
-};
-
struct imx_media_pad {
- struct media_pad pad;
- struct imx_media_link link[IMX_MEDIA_MAX_LINKS];
- bool devnode; /* does this pad link to a device node */
- int num_links;
-
/*
* list of video devices that can be reached from this pad,
* list is only valid for source pads.
@@ -154,7 +140,7 @@ struct imx_media_subdev {
int num_sink_pads;
int num_src_pads;
- /* the platform device if this is an internal subdev */
+ /* the platform device if this is an IPU-internal subdev */
struct platform_device *pdev;
/* the devname is needed for async devname match */
char devname[32];
@@ -225,17 +211,13 @@ struct imx_media_subdev *
imx_media_add_async_subdev(struct imx_media_dev *imxmd,
struct device_node *np,
struct platform_device *pdev);
-int imx_media_add_pad_link(struct imx_media_dev *imxmd,
- struct imx_media_pad *pad,
- struct device_node *remote_node,
- const char *remote_devname,
- int local_pad, int remote_pad);
void imx_media_grp_id_to_sd_name(char *sd_name, int sz,
u32 grp_id, int ipu_id);
-int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd,
- struct imx_media_subdev *csi[4]);
+int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd);
+int imx_media_create_internal_links(struct imx_media_dev *imxmd,
+ struct imx_media_subdev *imxsd);
void imx_media_remove_internal_subdevs(struct imx_media_dev *imxmd);
struct imx_media_subdev *
@@ -284,13 +266,12 @@ struct imx_media_fim *imx_media_fim_init(struct v4l2_subdev *sd);
void imx_media_fim_free(struct imx_media_fim *fim);
/* imx-media-of.c */
-struct imx_media_subdev *
-imx_media_of_find_subdev(struct imx_media_dev *imxmd,
- struct device_node *np,
- const char *name);
-int imx_media_of_parse(struct imx_media_dev *dev,
- struct imx_media_subdev *(*csi)[4],
- struct device_node *np);
+int imx_media_add_of_subdevs(struct imx_media_dev *dev,
+ struct device_node *np);
+int imx_media_create_of_links(struct imx_media_dev *imxmd,
+ struct imx_media_subdev *imxsd);
+int imx_media_create_csi_of_links(struct imx_media_dev *imxmd,
+ struct imx_media_subdev *csi);
/* imx-media-capture.c */
struct imx_media_video_dev *
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 3/9] media: staging/imx: of: allow for recursing downstream
2017-10-28 20:36 [PATCH 0/9] media: imx: Add better OF graph support Steve Longerbeam
2017-10-28 20:36 ` [PATCH 1/9] media: staging/imx: get CSI bus type from nearest upstream entity Steve Longerbeam
2017-10-28 20:36 ` [PATCH 2/9] media: staging/imx: remove static media link arrays Steve Longerbeam
@ 2017-10-28 20:36 ` Steve Longerbeam
2017-10-28 20:36 ` [PATCH 4/9] media: staging/imx: remove devname string from imx_media_subdev Steve Longerbeam
` (6 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Steve Longerbeam @ 2017-10-28 20:36 UTC (permalink / raw)
To: Philipp Zabel, Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: linux-media, devel, linux-kernel, Russell King, Steve Longerbeam
Calling of_parse_subdev() recursively to a downstream path that has
already been followed is ok, it just means that call will return
immediately since the subdevice was already added to the async list.
With that there is no need to determine whether a subdevice's port
is a sink or source, so 'num_{sink|src}_pads' is no longer used and
is removed.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/staging/media/imx/imx-media-internal-sd.c | 17 -----
drivers/staging/media/imx/imx-media-of.c | 78 ++++++-----------------
drivers/staging/media/imx/imx-media.h | 14 ----
3 files changed, 21 insertions(+), 88 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-internal-sd.c b/drivers/staging/media/imx/imx-media-internal-sd.c
index 3e60df5..53f2383 100644
--- a/drivers/staging/media/imx/imx-media-internal-sd.c
+++ b/drivers/staging/media/imx/imx-media-internal-sd.c
@@ -78,13 +78,9 @@ struct internal_pad {
static const struct internal_subdev {
const struct internal_subdev_id *id;
struct internal_pad pad[IMX_MEDIA_MAX_PADS];
- int num_sink_pads;
- int num_src_pads;
} int_subdev[num_isd] = {
[isd_csi0] = {
.id = &isd_id[isd_csi0],
- .num_sink_pads = CSI_NUM_SINK_PADS,
- .num_src_pads = CSI_NUM_SRC_PADS,
.pad[CSI_SRC_PAD_DIRECT] = {
.link = {
{
@@ -102,8 +98,6 @@ static const struct internal_subdev {
[isd_csi1] = {
.id = &isd_id[isd_csi1],
- .num_sink_pads = CSI_NUM_SINK_PADS,
- .num_src_pads = CSI_NUM_SRC_PADS,
.pad[CSI_SRC_PAD_DIRECT] = {
.link = {
{
@@ -121,8 +115,6 @@ static const struct internal_subdev {
[isd_vdic] = {
.id = &isd_id[isd_vdic],
- .num_sink_pads = VDIC_NUM_SINK_PADS,
- .num_src_pads = VDIC_NUM_SRC_PADS,
.pad[VDIC_SRC_PAD_DIRECT] = {
.link = {
{
@@ -136,8 +128,6 @@ static const struct internal_subdev {
[isd_ic_prp] = {
.id = &isd_id[isd_ic_prp],
- .num_sink_pads = PRP_NUM_SINK_PADS,
- .num_src_pads = PRP_NUM_SRC_PADS,
.pad[PRP_SRC_PAD_PRPENC] = {
.link = {
{
@@ -160,14 +150,10 @@ static const struct internal_subdev {
[isd_ic_prpenc] = {
.id = &isd_id[isd_ic_prpenc],
- .num_sink_pads = PRPENCVF_NUM_SINK_PADS,
- .num_src_pads = PRPENCVF_NUM_SRC_PADS,
},
[isd_ic_prpvf] = {
.id = &isd_id[isd_ic_prpvf],
- .num_sink_pads = PRPENCVF_NUM_SINK_PADS,
- .num_src_pads = PRPENCVF_NUM_SRC_PADS,
},
};
@@ -312,9 +298,6 @@ static int add_internal_subdev(struct imx_media_dev *imxmd,
if (IS_ERR(imxsd))
return PTR_ERR(imxsd);
- imxsd->num_sink_pads = isd->num_sink_pads;
- imxsd->num_src_pads = isd->num_src_pads;
-
return 0;
}
diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
index d35c99e..a085e52 100644
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -41,11 +41,12 @@ static int of_get_port_count(const struct device_node *np)
/*
* find the remote device node given local endpoint node
*/
-static void of_get_remote(struct device_node *epnode,
+static bool of_get_remote(struct device_node *epnode,
struct device_node **remote_node)
{
struct device_node *rp, *rpp;
struct device_node *remote;
+ bool is_csi_port;
rp = of_graph_get_remote_port(epnode);
rpp = of_graph_get_remote_port_parent(epnode);
@@ -54,9 +55,11 @@ static void of_get_remote(struct device_node *epnode,
/* the remote is one of the CSI ports */
remote = rp;
of_node_put(rpp);
+ is_csi_port = true;
} else {
remote = rpp;
of_node_put(rp);
+ is_csi_port = false;
}
if (!of_device_is_available(remote)) {
@@ -65,6 +68,8 @@ static void of_get_remote(struct device_node *epnode,
} else {
*remote_node = remote;
}
+
+ return is_csi_port;
}
static int
@@ -72,7 +77,7 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np,
bool is_csi_port)
{
struct imx_media_subdev *imxsd;
- int i, num_pads, ret;
+ int i, num_ports, ret;
if (!of_device_is_available(sd_np)) {
dev_dbg(imxmd->md.dev, "%s: %s not enabled\n", __func__,
@@ -94,77 +99,36 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np,
return ret;
}
- if (is_csi_port) {
- /*
- * the ipu-csi has one sink port and two source ports.
- * The source ports are not represented in the device tree,
- * but are described by the internal pads and links later.
- */
- num_pads = CSI_NUM_PADS;
- imxsd->num_sink_pads = CSI_NUM_SINK_PADS;
- } else if (of_device_is_compatible(sd_np, "fsl,imx6-mipi-csi2")) {
- num_pads = of_get_port_count(sd_np);
- /* the mipi csi2 receiver has only one sink port */
- imxsd->num_sink_pads = 1;
- } else if (of_device_is_compatible(sd_np, "video-mux")) {
- num_pads = of_get_port_count(sd_np);
- /* for the video mux, all but the last port are sinks */
- imxsd->num_sink_pads = num_pads - 1;
- } else {
- num_pads = of_get_port_count(sd_np);
- if (num_pads != 1) {
- /* confused, but no reason to give up here */
- dev_warn(imxmd->md.dev,
- "%s: unknown device %s with %d ports\n",
- __func__, sd_np->name, num_pads);
- return 0;
- }
+ /*
+ * the ipu-csi has one sink port. The source pads are not
+ * represented in the device tree by port nodes, but are
+ * described by the internal pads and links later.
+ */
+ num_ports = is_csi_port ? 1 : of_get_port_count(sd_np);
- /*
- * we got to this node from this single source port,
- * there are no sink pads.
- */
- imxsd->num_sink_pads = 0;
- }
-
- if (imxsd->num_sink_pads >= num_pads)
- return -EINVAL;
-
- imxsd->num_src_pads = num_pads - imxsd->num_sink_pads;
-
- dev_dbg(imxmd->md.dev, "%s: %s has %d pads (%d sink, %d src)\n",
- __func__, sd_np->name, num_pads,
- imxsd->num_sink_pads, imxsd->num_src_pads);
-
- for (i = 0; i < num_pads; i++) {
+ for (i = 0; i < num_ports; i++) {
struct device_node *epnode = NULL, *port, *remote_np;
- if (is_csi_port)
- port = (i < imxsd->num_sink_pads) ? sd_np : NULL;
- else
- port = of_graph_get_port_by_id(sd_np, i);
+ port = is_csi_port ? sd_np : of_graph_get_port_by_id(sd_np, i);
if (!port)
continue;
for_each_child_of_node(port, epnode) {
- of_get_remote(epnode, &remote_np);
+ bool remote_is_csi;
+
+ remote_is_csi = of_get_remote(epnode, &remote_np);
if (!remote_np)
continue;
- if (i < imxsd->num_sink_pads) {
- /* follow sink endpoints upstream */
- ret = of_parse_subdev(imxmd, remote_np, false);
- if (ret)
- break;
- }
-
+ ret = of_parse_subdev(imxmd, remote_np, remote_is_csi);
of_node_put(remote_np);
+ if (ret)
+ break;
}
if (port != sd_np)
of_node_put(port);
if (ret) {
- of_node_put(remote_np);
of_node_put(epnode);
break;
}
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 6c0e443..925b46b 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -49,9 +49,6 @@ enum {
CSI_NUM_PADS,
};
-#define CSI_NUM_SINK_PADS 1
-#define CSI_NUM_SRC_PADS 2
-
/* ipu_vdic */
enum {
VDIC_SINK_PAD_DIRECT = 0,
@@ -60,9 +57,6 @@ enum {
VDIC_NUM_PADS,
};
-#define VDIC_NUM_SINK_PADS 2
-#define VDIC_NUM_SRC_PADS 1
-
/* ipu_ic_prp */
enum {
PRP_SINK_PAD = 0,
@@ -71,9 +65,6 @@ enum {
PRP_NUM_PADS,
};
-#define PRP_NUM_SINK_PADS 1
-#define PRP_NUM_SRC_PADS 2
-
/* ipu_ic_prpencvf */
enum {
PRPENCVF_SINK_PAD = 0,
@@ -81,9 +72,6 @@ enum {
PRPENCVF_NUM_PADS,
};
-#define PRPENCVF_NUM_SINK_PADS 1
-#define PRPENCVF_NUM_SRC_PADS 1
-
/* How long to wait for EOF interrupts in the buffer-capture subdevs */
#define IMX_MEDIA_EOF_TIMEOUT 1000
@@ -137,8 +125,6 @@ struct imx_media_subdev {
struct v4l2_subdev *sd; /* set when bound */
struct imx_media_pad pad[IMX_MEDIA_MAX_PADS];
- int num_sink_pads;
- int num_src_pads;
/* the platform device if this is an IPU-internal subdev */
struct platform_device *pdev;
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 4/9] media: staging/imx: remove devname string from imx_media_subdev
2017-10-28 20:36 [PATCH 0/9] media: imx: Add better OF graph support Steve Longerbeam
` (2 preceding siblings ...)
2017-10-28 20:36 ` [PATCH 3/9] media: staging/imx: of: allow for recursing downstream Steve Longerbeam
@ 2017-10-28 20:36 ` Steve Longerbeam
2017-10-28 20:36 ` [PATCH 5/9] media: staging/imx: pass fwnode handle to find/add async subdev Steve Longerbeam
` (5 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Steve Longerbeam @ 2017-10-28 20:36 UTC (permalink / raw)
To: Philipp Zabel, Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: linux-media, devel, linux-kernel, Russell King, Steve Longerbeam
A separate string for the device name, for DEVNAME async match, was
never needed. Just assign the asd device name to the passed platform
device name pointer in imx_media_add_async_subdev().
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/staging/media/imx/imx-media-dev.c | 3 +--
drivers/staging/media/imx/imx-media.h | 2 --
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
index f63808f..b948ce8 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -112,8 +112,7 @@ imx_media_add_async_subdev(struct imx_media_dev *imxmd,
asd->match.fwnode.fwnode = of_fwnode_handle(np);
} else {
asd->match_type = V4L2_ASYNC_MATCH_DEVNAME;
- strncpy(imxsd->devname, devname, sizeof(imxsd->devname));
- asd->match.device_name.name = imxsd->devname;
+ asd->match.device_name.name = devname;
imxsd->pdev = pdev;
}
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 925b46b..299d4df 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -128,8 +128,6 @@ struct imx_media_subdev {
/* the platform device if this is an IPU-internal subdev */
struct platform_device *pdev;
- /* the devname is needed for async devname match */
- char devname[32];
};
struct imx_media_dev {
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 5/9] media: staging/imx: pass fwnode handle to find/add async subdev
2017-10-28 20:36 [PATCH 0/9] media: imx: Add better OF graph support Steve Longerbeam
` (3 preceding siblings ...)
2017-10-28 20:36 ` [PATCH 4/9] media: staging/imx: remove devname string from imx_media_subdev Steve Longerbeam
@ 2017-10-28 20:36 ` Steve Longerbeam
2017-10-28 20:36 ` [PATCH 6/9] media: staging/imx: remove static subdev arrays Steve Longerbeam
` (4 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Steve Longerbeam @ 2017-10-28 20:36 UTC (permalink / raw)
To: Philipp Zabel, Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: linux-media, devel, linux-kernel, Russell King, Steve Longerbeam
Pass the subdev's fwnode_handle to imx_media_find_async_subdev() and
imx_media_add_async_subdev(), instead of a device_node.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/staging/media/imx/imx-media-dev.c | 20 ++++++++++----------
drivers/staging/media/imx/imx-media-of.c | 7 +++----
drivers/staging/media/imx/imx-media.h | 4 ++--
3 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
index b948ce8..eba8560 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -33,15 +33,14 @@ static inline struct imx_media_dev *notifier2dev(struct v4l2_async_notifier *n)
}
/*
- * Find a subdev by device node or device name. This is called during
+ * Find a subdev by fwnode or device name. This is called during
* driver load to form the async subdev list and bind them.
*/
struct imx_media_subdev *
imx_media_find_async_subdev(struct imx_media_dev *imxmd,
- struct device_node *np,
+ struct fwnode_handle *fwnode,
const char *devname)
{
- struct fwnode_handle *fwnode = np ? of_fwnode_handle(np) : NULL;
struct imx_media_subdev *imxsd;
int i;
@@ -67,7 +66,7 @@ imx_media_find_async_subdev(struct imx_media_dev *imxmd,
/*
- * Adds a subdev to the async subdev list. If np is non-NULL, adds
+ * Adds a subdev to the async subdev list. If fwnode is non-NULL, adds
* the async as a V4L2_ASYNC_MATCH_FWNODE match type, otherwise as
* a V4L2_ASYNC_MATCH_DEVNAME match type using the dev_name of the
* given platform_device. This is called during driver load when
@@ -75,9 +74,10 @@ imx_media_find_async_subdev(struct imx_media_dev *imxmd,
*/
struct imx_media_subdev *
imx_media_add_async_subdev(struct imx_media_dev *imxmd,
- struct device_node *np,
+ struct fwnode_handle *fwnode,
struct platform_device *pdev)
{
+ struct device_node *np = to_of_node(fwnode);
struct imx_media_subdev *imxsd;
struct v4l2_async_subdev *asd;
const char *devname = NULL;
@@ -89,7 +89,7 @@ imx_media_add_async_subdev(struct imx_media_dev *imxmd,
devname = dev_name(&pdev->dev);
/* return -EEXIST if this subdev already added */
- if (imx_media_find_async_subdev(imxmd, np, devname)) {
+ if (imx_media_find_async_subdev(imxmd, fwnode, devname)) {
dev_dbg(imxmd->md.dev, "%s: already added %s\n",
__func__, np ? np->name : devname);
imxsd = ERR_PTR(-EEXIST);
@@ -107,9 +107,9 @@ imx_media_add_async_subdev(struct imx_media_dev *imxmd,
imxsd = &imxmd->subdev[sd_idx];
asd = &imxsd->asd;
- if (np) {
+ if (fwnode) {
asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
- asd->match.fwnode.fwnode = of_fwnode_handle(np);
+ asd->match.fwnode.fwnode = fwnode;
} else {
asd->match_type = V4L2_ASYNC_MATCH_DEVNAME;
asd->match.device_name.name = devname;
@@ -162,13 +162,13 @@ static int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
struct v4l2_async_subdev *asd)
{
struct imx_media_dev *imxmd = notifier2dev(notifier);
- struct device_node *np = to_of_node(sd->fwnode);
struct imx_media_subdev *imxsd;
int ret = 0;
mutex_lock(&imxmd->mutex);
- imxsd = imx_media_find_async_subdev(imxmd, np, dev_name(sd->dev));
+ imxsd = imx_media_find_async_subdev(imxmd, sd->fwnode,
+ dev_name(sd->dev));
if (!imxsd) {
ret = -EINVAL;
goto out;
diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
index a085e52..eb7a7f2 100644
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -87,7 +87,8 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np,
}
/* register this subdev with async notifier */
- imxsd = imx_media_add_async_subdev(imxmd, sd_np, NULL);
+ imxsd = imx_media_add_async_subdev(imxmd, of_fwnode_handle(sd_np),
+ NULL);
ret = PTR_ERR_OR_ZERO(imxsd);
if (ret) {
if (ret == -EEXIST) {
@@ -176,9 +177,7 @@ static int create_of_link(struct imx_media_dev *imxmd,
if (link->local_port >= sd->entity.num_pads)
return -EINVAL;
- remote = imx_media_find_async_subdev(imxmd,
- to_of_node(link->remote_node),
- NULL);
+ remote = imx_media_find_async_subdev(imxmd, link->remote_node, NULL);
if (!remote)
return 0;
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 299d4df..b208af41 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -189,11 +189,11 @@ int imx_media_ipu_image_to_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
struct imx_media_subdev *
imx_media_find_async_subdev(struct imx_media_dev *imxmd,
- struct device_node *np,
+ struct fwnode_handle *fwnode,
const char *devname);
struct imx_media_subdev *
imx_media_add_async_subdev(struct imx_media_dev *imxmd,
- struct device_node *np,
+ struct fwnode_handle *fwnode,
struct platform_device *pdev);
void imx_media_grp_id_to_sd_name(char *sd_name, int sz,
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 6/9] media: staging/imx: remove static subdev arrays
2017-10-28 20:36 [PATCH 0/9] media: imx: Add better OF graph support Steve Longerbeam
` (4 preceding siblings ...)
2017-10-28 20:36 ` [PATCH 5/9] media: staging/imx: pass fwnode handle to find/add async subdev Steve Longerbeam
@ 2017-10-28 20:36 ` Steve Longerbeam
2017-10-28 20:36 ` [PATCH 7/9] media: staging/imx: convert static vdev lists to list_head Steve Longerbeam
` (3 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Steve Longerbeam @ 2017-10-28 20:36 UTC (permalink / raw)
To: Philipp Zabel, Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: linux-media, devel, linux-kernel, Russell King, Steve Longerbeam
For more complex OF graphs, there will be more async subdevices
registered. Remove the static subdev[IMX_MEDIA_MAX_SUBDEVS] array,
so that imx-media places no limits on the number of async subdevs
that can be added and registered.
There were two uses for 'struct imx_media_subdev'. First was to act
as the async subdev list to be passed to v4l2_async_notifier_register().
Second was to aid in inheriting subdev controls to the capture devices,
and this is done by creating a list of capture devices that can be reached
from a subdev's source pad. So 'struct imx_media_subdev' also contained
a static array of 'struct imx_media_pad' for placing the capture device
lists at each pad.
'struct imx_media_subdev' has been completely removed. Instead, at async
completion, allocate an array of 'struct imx_media_pad' and attach it to
the subdev's host_priv pointer, in order to support subdev controls
inheritance.
Likewise, remove static async_ptrs[IMX_MEDIA_MAX_SUBDEVS] array.
Instead, allocate a 'struct imx_media_async_subdev' when forming
the async list, and add it to an asd_list list_head in
imx_media_add_async_subdev(). At async completion, allocate the
asd pointer list and pull the asd's off asd_list for
v4l2_async_notifier_register().
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/staging/media/imx/imx-ic-prp.c | 4 +-
drivers/staging/media/imx/imx-media-csi.c | 9 +-
drivers/staging/media/imx/imx-media-dev.c | 216 ++++++++++++----------
drivers/staging/media/imx/imx-media-internal-sd.c | 51 +++--
drivers/staging/media/imx/imx-media-of.c | 31 ++--
drivers/staging/media/imx/imx-media-utils.c | 43 ++---
drivers/staging/media/imx/imx-media.h | 81 ++++----
7 files changed, 218 insertions(+), 217 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prp.c b/drivers/staging/media/imx/imx-ic-prp.c
index 9e41987..c6d7e80 100644
--- a/drivers/staging/media/imx/imx-ic-prp.c
+++ b/drivers/staging/media/imx/imx-ic-prp.c
@@ -300,7 +300,7 @@ static int prp_link_validate(struct v4l2_subdev *sd,
{
struct imx_ic_priv *ic_priv = v4l2_get_subdevdata(sd);
struct prp_priv *priv = ic_priv->prp_priv;
- struct imx_media_subdev *csi;
+ struct v4l2_subdev *csi;
int ret;
ret = v4l2_subdev_link_validate_default(sd, link,
@@ -333,7 +333,7 @@ static int prp_link_validate(struct v4l2_subdev *sd,
}
if (csi) {
- switch (csi->sd->grp_id) {
+ switch (csi->grp_id) {
case IMX_MEDIA_GRP_ID_CSI0:
priv->csi_id = 0;
break;
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index c45b235..bce7ca7 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -138,7 +138,6 @@ static int csi_get_upstream_endpoint(struct csi_priv *priv,
struct v4l2_fwnode_endpoint *ep)
{
struct device_node *endpoint, *port;
- struct imx_media_subdev *imxsd;
struct media_entity *src;
struct v4l2_subdev *sd;
struct media_pad *pad;
@@ -154,10 +153,10 @@ static int csi_get_upstream_endpoint(struct csi_priv *priv,
* CSI-2 receiver if it is in the path, otherwise stay
* with video mux.
*/
- imxsd = imx_media_find_upstream_subdev(priv->md, src,
- IMX_MEDIA_GRP_ID_CSI2);
- if (!IS_ERR(imxsd))
- src = &imxsd->sd->entity;
+ sd = imx_media_find_upstream_subdev(priv->md, src,
+ IMX_MEDIA_GRP_ID_CSI2);
+ if (!IS_ERR(sd))
+ src = &sd->entity;
}
/* get source pad of entity directly upstream from src */
diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
index eba8560..40c7cfd 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -33,28 +33,28 @@ static inline struct imx_media_dev *notifier2dev(struct v4l2_async_notifier *n)
}
/*
- * Find a subdev by fwnode or device name. This is called during
+ * Find an asd by fwnode or device name. This is called during
* driver load to form the async subdev list and bind them.
*/
-struct imx_media_subdev *
-imx_media_find_async_subdev(struct imx_media_dev *imxmd,
- struct fwnode_handle *fwnode,
- const char *devname)
+static struct v4l2_async_subdev *
+find_async_subdev(struct imx_media_dev *imxmd,
+ struct fwnode_handle *fwnode,
+ const char *devname)
{
- struct imx_media_subdev *imxsd;
- int i;
+ struct imx_media_async_subdev *imxasd;
+ struct v4l2_async_subdev *asd;
- for (i = 0; i < imxmd->subdev_notifier.num_subdevs; i++) {
- imxsd = &imxmd->subdev[i];
- switch (imxsd->asd.match_type) {
+ list_for_each_entry(imxasd, &imxmd->asd_list, list) {
+ asd = &imxasd->asd;
+ switch (asd->match_type) {
case V4L2_ASYNC_MATCH_FWNODE:
- if (fwnode && imxsd->asd.match.fwnode.fwnode == fwnode)
- return imxsd;
+ if (fwnode && asd->match.fwnode.fwnode == fwnode)
+ return asd;
break;
case V4L2_ASYNC_MATCH_DEVNAME:
- if (devname &&
- !strcmp(imxsd->asd.match.device_name.name, devname))
- return imxsd;
+ if (devname && !strcmp(asd->match.device_name.name,
+ devname))
+ return asd;
break;
default:
break;
@@ -72,51 +72,47 @@ imx_media_find_async_subdev(struct imx_media_dev *imxmd,
* given platform_device. This is called during driver load when
* forming the async subdev list.
*/
-struct imx_media_subdev *
-imx_media_add_async_subdev(struct imx_media_dev *imxmd,
- struct fwnode_handle *fwnode,
- struct platform_device *pdev)
+int imx_media_add_async_subdev(struct imx_media_dev *imxmd,
+ struct fwnode_handle *fwnode,
+ struct platform_device *pdev)
{
struct device_node *np = to_of_node(fwnode);
- struct imx_media_subdev *imxsd;
+ struct imx_media_async_subdev *imxasd;
struct v4l2_async_subdev *asd;
const char *devname = NULL;
- int sd_idx;
+ int ret = 0;
mutex_lock(&imxmd->mutex);
if (pdev)
devname = dev_name(&pdev->dev);
- /* return -EEXIST if this subdev already added */
- if (imx_media_find_async_subdev(imxmd, fwnode, devname)) {
+ /* return -EEXIST if this asd already added */
+ if (find_async_subdev(imxmd, fwnode, devname)) {
dev_dbg(imxmd->md.dev, "%s: already added %s\n",
__func__, np ? np->name : devname);
- imxsd = ERR_PTR(-EEXIST);
+ ret = -EEXIST;
goto out;
}
- sd_idx = imxmd->subdev_notifier.num_subdevs;
- if (sd_idx >= IMX_MEDIA_MAX_SUBDEVS) {
- dev_err(imxmd->md.dev, "%s: too many subdevs! can't add %s\n",
- __func__, np ? np->name : devname);
- imxsd = ERR_PTR(-ENOSPC);
+ imxasd = devm_kzalloc(imxmd->md.dev, sizeof(*imxasd), GFP_KERNEL);
+ if (!imxasd) {
+ ret = -ENOMEM;
goto out;
}
+ asd = &imxasd->asd;
- imxsd = &imxmd->subdev[sd_idx];
-
- asd = &imxsd->asd;
if (fwnode) {
asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
asd->match.fwnode.fwnode = fwnode;
} else {
asd->match_type = V4L2_ASYNC_MATCH_DEVNAME;
asd->match.device_name.name = devname;
- imxsd->pdev = pdev;
+ imxasd->pdev = pdev;
}
- imxmd->async_ptrs[sd_idx] = asd;
+ list_add_tail(&imxasd->list, &imxmd->asd_list);
+
imxmd->subdev_notifier.num_subdevs++;
dev_dbg(imxmd->md.dev, "%s: added %s, match type %s\n",
@@ -124,7 +120,7 @@ imx_media_add_async_subdev(struct imx_media_dev *imxmd,
out:
mutex_unlock(&imxmd->mutex);
- return imxsd;
+ return ret;
}
/*
@@ -162,56 +158,48 @@ static int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
struct v4l2_async_subdev *asd)
{
struct imx_media_dev *imxmd = notifier2dev(notifier);
- struct imx_media_subdev *imxsd;
int ret = 0;
mutex_lock(&imxmd->mutex);
- imxsd = imx_media_find_async_subdev(imxmd, sd->fwnode,
- dev_name(sd->dev));
- if (!imxsd) {
- ret = -EINVAL;
- goto out;
- }
-
if (sd->grp_id & IMX_MEDIA_GRP_ID_CSI) {
ret = imx_media_get_ipu(imxmd, sd);
if (ret)
- goto out_unlock;
+ goto out;
}
- /* attach the subdev */
- imxsd->sd = sd;
+ v4l2_info(&imxmd->v4l2_dev, "subdev %s bound\n", sd->name);
out:
- if (ret)
- v4l2_warn(&imxmd->v4l2_dev,
- "Received unknown subdev %s\n", sd->name);
- else
- v4l2_info(&imxmd->v4l2_dev,
- "Registered subdev %s\n", sd->name);
-
-out_unlock:
mutex_unlock(&imxmd->mutex);
return ret;
}
/*
- * create the media links from all pads and their links.
- * Called after all subdevs have registered.
+ * create the media links for all subdevs that registered async.
+ * Called after all async subdevs have bound.
*/
-static int imx_media_create_links(struct imx_media_dev *imxmd)
+static int imx_media_create_links(struct v4l2_async_notifier *notifier)
{
- struct imx_media_subdev *imxsd;
+ struct imx_media_dev *imxmd = notifier2dev(notifier);
struct v4l2_subdev *sd;
- int i, ret;
-
- for (i = 0; i < imxmd->num_subdevs; i++) {
- imxsd = &imxmd->subdev[i];
- sd = imxsd->sd;
+ int ret;
- if (((sd->grp_id & IMX_MEDIA_GRP_ID_CSI) || imxsd->pdev)) {
- /* this is an internal subdev or a CSI */
- ret = imx_media_create_internal_links(imxmd, imxsd);
+ /*
+ * Only links are created between subdevices that are known
+ * to the async notifier. If there are other non-async subdevices,
+ * they were created internally by some subdevice (smiapp is one
+ * example). In those cases it is expected the subdevice is
+ * responsible for creating those internal links.
+ */
+ list_for_each_entry(sd, ¬ifier->done, async_list) {
+ switch (sd->grp_id) {
+ case IMX_MEDIA_GRP_ID_VDIC:
+ case IMX_MEDIA_GRP_ID_IC_PRP:
+ case IMX_MEDIA_GRP_ID_IC_PRPENC:
+ case IMX_MEDIA_GRP_ID_IC_PRPVF:
+ case IMX_MEDIA_GRP_ID_CSI0:
+ case IMX_MEDIA_GRP_ID_CSI1:
+ ret = imx_media_create_internal_links(imxmd, sd);
if (ret)
return ret;
/*
@@ -220,10 +208,12 @@ static int imx_media_create_links(struct imx_media_dev *imxmd)
* to the CSI sink pads.
*/
if (sd->grp_id & IMX_MEDIA_GRP_ID_CSI)
- imx_media_create_csi_of_links(imxmd, imxsd);
- } else {
+ imx_media_create_csi_of_links(imxmd, sd);
+ break;
+ default:
/* this is an external fwnode subdev */
- imx_media_create_of_links(imxmd, imxsd);
+ imx_media_create_of_links(imxmd, sd);
+ break;
}
}
@@ -239,7 +229,6 @@ static int imx_media_add_vdev_to_pad(struct imx_media_dev *imxmd,
struct media_pad *srcpad)
{
struct media_entity *entity = srcpad->entity;
- struct imx_media_subdev *imxsd;
struct imx_media_pad *imxpad;
struct media_link *link;
struct v4l2_subdev *sd;
@@ -250,11 +239,18 @@ static int imx_media_add_vdev_to_pad(struct imx_media_dev *imxmd,
return 0;
sd = media_entity_to_v4l2_subdev(entity);
- imxsd = imx_media_find_subdev_by_sd(imxmd, sd);
- if (IS_ERR(imxsd))
- return PTR_ERR(imxsd);
- imxpad = &imxsd->pad[srcpad->index];
+ imxpad = to_imx_media_pad(sd, srcpad->index);
+ if (!imxpad) {
+ v4l2_warn(&imxmd->v4l2_dev, "%s:%u has no vdev list!\n",
+ entity->name, srcpad->index);
+ /*
+ * shouldn't happen, but no reason to fail driver load,
+ * just skip this entity.
+ */
+ return 0;
+ }
+
vdev_idx = imxpad->num_vdevs;
/* just return if we've been here before */
@@ -293,6 +289,27 @@ static int imx_media_add_vdev_to_pad(struct imx_media_dev *imxmd,
return 0;
}
+static int imx_media_alloc_pad_vdev_lists(struct imx_media_dev *imxmd)
+{
+ struct imx_media_pad *imxpads;
+ struct media_entity *entity;
+ struct v4l2_subdev *sd;
+
+ list_for_each_entry(sd, &imxmd->v4l2_dev.subdevs, list) {
+ entity = &sd->entity;
+ imxpads = devm_kzalloc(imxmd->md.dev,
+ entity->num_pads * sizeof(*imxpads),
+ GFP_KERNEL);
+ if (!imxpads)
+ return -ENOMEM;
+
+ /* attach imxpads to the subdev's host private pointer */
+ sd->host_priv = imxpads;
+ }
+
+ return 0;
+}
+
/* form the vdev lists in all imx-media source pads */
static int imx_media_create_pad_vdev_lists(struct imx_media_dev *imxmd)
{
@@ -300,6 +317,10 @@ static int imx_media_create_pad_vdev_lists(struct imx_media_dev *imxmd)
struct media_link *link;
int i, ret;
+ ret = imx_media_alloc_pad_vdev_lists(imxmd);
+ if (ret)
+ return ret;
+
for (i = 0; i < imxmd->num_vdevs; i++) {
vdev = imxmd->vdev[i];
link = list_first_entry(&vdev->vfd->entity.links,
@@ -316,20 +337,11 @@ static int imx_media_create_pad_vdev_lists(struct imx_media_dev *imxmd)
static int imx_media_probe_complete(struct v4l2_async_notifier *notifier)
{
struct imx_media_dev *imxmd = notifier2dev(notifier);
- int i, ret;
+ int ret;
mutex_lock(&imxmd->mutex);
- /* make sure all subdevs were bound */
- for (i = 0; i < imxmd->num_subdevs; i++) {
- if (!imxmd->subdev[i].sd) {
- v4l2_err(&imxmd->v4l2_dev, "unbound subdev!\n");
- ret = -ENODEV;
- goto unlock;
- }
- }
-
- ret = imx_media_create_links(imxmd);
+ ret = imx_media_create_links(notifier);
if (ret)
goto unlock;
@@ -393,7 +405,6 @@ static int imx_media_link_notify(struct media_link *link, u32 flags,
unsigned int notification)
{
struct media_entity *source = link->source->entity;
- struct imx_media_subdev *imxsd;
struct imx_media_pad *imxpad;
struct imx_media_dev *imxmd;
struct video_device *vfd;
@@ -413,10 +424,11 @@ static int imx_media_link_notify(struct media_link *link, u32 flags,
imxmd = dev_get_drvdata(sd->v4l2_dev->dev);
- imxsd = imx_media_find_subdev_by_sd(imxmd, sd);
- if (IS_ERR(imxsd))
- return PTR_ERR(imxsd);
- imxpad = &imxsd->pad[pad_idx];
+ imxpad = to_imx_media_pad(sd, pad_idx);
+ if (!imxpad) {
+ /* shouldn't happen, but no reason to fail link setup */
+ return 0;
+ }
/*
* Before disabling a link, reset controls for all video
@@ -460,8 +472,10 @@ static int imx_media_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct device_node *node = dev->of_node;
+ struct imx_media_async_subdev *imxasd;
+ struct v4l2_async_subdev **subdevs;
struct imx_media_dev *imxmd;
- int ret;
+ int num_subdevs, i, ret;
imxmd = devm_kzalloc(dev, sizeof(*imxmd), GFP_KERNEL);
if (!imxmd)
@@ -490,6 +504,8 @@ static int imx_media_probe(struct platform_device *pdev)
dev_set_drvdata(imxmd->v4l2_dev.dev, imxmd);
+ INIT_LIST_HEAD(&imxmd->asd_list);
+
ret = imx_media_add_of_subdevs(imxmd, node);
if (ret) {
v4l2_err(&imxmd->v4l2_dev,
@@ -504,15 +520,27 @@ static int imx_media_probe(struct platform_device *pdev)
goto unreg_dev;
}
+ num_subdevs = imxmd->subdev_notifier.num_subdevs;
+
/* no subdevs? just bail */
- imxmd->num_subdevs = imxmd->subdev_notifier.num_subdevs;
- if (imxmd->num_subdevs == 0) {
+ if (num_subdevs == 0) {
ret = -ENODEV;
goto unreg_dev;
}
+ subdevs = devm_kzalloc(imxmd->md.dev, sizeof(*subdevs) * num_subdevs,
+ GFP_KERNEL);
+ if (!subdevs) {
+ ret = -ENOMEM;
+ goto unreg_dev;
+ }
+
+ i = 0;
+ list_for_each_entry(imxasd, &imxmd->asd_list, list)
+ subdevs[i++] = &imxasd->asd;
+
/* prepare the async subdev notifier and register it */
- imxmd->subdev_notifier.subdevs = imxmd->async_ptrs;
+ imxmd->subdev_notifier.subdevs = subdevs;
imxmd->subdev_notifier.bound = imx_media_subdev_bound;
imxmd->subdev_notifier.complete = imx_media_probe_complete;
ret = v4l2_async_notifier_register(&imxmd->v4l2_dev,
diff --git a/drivers/staging/media/imx/imx-media-internal-sd.c b/drivers/staging/media/imx/imx-media-internal-sd.c
index 53f2383..70833fe 100644
--- a/drivers/staging/media/imx/imx-media-internal-sd.c
+++ b/drivers/staging/media/imx/imx-media-internal-sd.c
@@ -68,6 +68,8 @@ struct internal_link {
int remote_pad;
};
+/* max pads per internal-sd */
+#define MAX_INTERNAL_PADS 8
/* max links per internal-sd pad */
#define MAX_INTERNAL_LINKS 8
@@ -77,7 +79,7 @@ struct internal_pad {
static const struct internal_subdev {
const struct internal_subdev_id *id;
- struct internal_pad pad[IMX_MEDIA_MAX_PADS];
+ struct internal_pad pad[MAX_INTERNAL_PADS];
} int_subdev[num_isd] = {
[isd_csi0] = {
.id = &isd_id[isd_csi0],
@@ -181,9 +183,9 @@ static const struct internal_subdev *find_intsd_by_grp_id(u32 grp_id)
return NULL;
}
-static struct imx_media_subdev *find_sink(struct imx_media_dev *imxmd,
- struct imx_media_subdev *src,
- const struct internal_link *link)
+static struct v4l2_subdev *find_sink(struct imx_media_dev *imxmd,
+ struct v4l2_subdev *src,
+ const struct internal_link *link)
{
char sink_devname[32];
int ipu_id;
@@ -194,20 +196,20 @@ static struct imx_media_subdev *find_sink(struct imx_media_dev *imxmd,
* a CSI, it has different struct ipu_client_platformdata which
* does not contain IPU id.
*/
- if (sscanf(src->sd->name, "ipu%d", &ipu_id) != 1)
+ if (sscanf(src->name, "ipu%d", &ipu_id) != 1)
return NULL;
isd_to_devname(sink_devname, sizeof(sink_devname),
link->remote, ipu_id - 1);
- return imx_media_find_async_subdev(imxmd, NULL, sink_devname);
+ return imx_media_find_subdev_by_devname(imxmd, sink_devname);
}
static int create_ipu_internal_link(struct imx_media_dev *imxmd,
- struct imx_media_subdev *src,
+ struct v4l2_subdev *src,
const struct internal_link *link)
{
- struct imx_media_subdev *sink;
+ struct v4l2_subdev *sink;
int ret;
sink = find_sink(imxmd, src, link);
@@ -215,11 +217,11 @@ static int create_ipu_internal_link(struct imx_media_dev *imxmd,
return -ENODEV;
v4l2_info(&imxmd->v4l2_dev, "%s:%d -> %s:%d\n",
- src->sd->name, link->local_pad,
- sink->sd->name, link->remote_pad);
+ src->name, link->local_pad,
+ sink->name, link->remote_pad);
- ret = media_create_pad_link(&src->sd->entity, link->local_pad,
- &sink->sd->entity, link->remote_pad, 0);
+ ret = media_create_pad_link(&src->entity, link->local_pad,
+ &sink->entity, link->remote_pad, 0);
if (ret)
v4l2_err(&imxmd->v4l2_dev,
"create_pad_link failed: %d\n", ret);
@@ -228,16 +230,15 @@ static int create_ipu_internal_link(struct imx_media_dev *imxmd,
}
int imx_media_create_internal_links(struct imx_media_dev *imxmd,
- struct imx_media_subdev *imxsd)
+ struct v4l2_subdev *sd)
{
- struct v4l2_subdev *sd = imxsd->sd;
const struct internal_subdev *intsd;
const struct internal_pad *intpad;
const struct internal_link *link;
struct media_pad *pad;
int i, j, ret;
- intsd = find_intsd_by_grp_id(imxsd->sd->grp_id);
+ intsd = find_intsd_by_grp_id(sd->grp_id);
if (!intsd)
return -ENODEV;
@@ -255,7 +256,7 @@ int imx_media_create_internal_links(struct imx_media_dev *imxmd,
if (!link->remote)
break;
- ret = create_ipu_internal_link(imxmd, imxsd, link);
+ ret = create_ipu_internal_link(imxmd, sd, link);
if (ret)
return ret;
}
@@ -271,7 +272,6 @@ static int add_internal_subdev(struct imx_media_dev *imxmd,
{
struct imx_media_internal_sd_platformdata pdata;
struct platform_device_info pdevinfo = {0};
- struct imx_media_subdev *imxsd;
struct platform_device *pdev;
pdata.grp_id = isd->id->grp_id;
@@ -294,11 +294,7 @@ static int add_internal_subdev(struct imx_media_dev *imxmd,
if (IS_ERR(pdev))
return PTR_ERR(pdev);
- imxsd = imx_media_add_async_subdev(imxmd, NULL, pdev);
- if (IS_ERR(imxsd))
- return PTR_ERR(imxsd);
-
- return 0;
+ return imx_media_add_async_subdev(imxmd, NULL, pdev);
}
/* adds the internal subdevs in one ipu */
@@ -353,13 +349,12 @@ int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd)
void imx_media_remove_internal_subdevs(struct imx_media_dev *imxmd)
{
- struct imx_media_subdev *imxsd;
- int i;
+ struct imx_media_async_subdev *imxasd;
- for (i = 0; i < imxmd->subdev_notifier.num_subdevs; i++) {
- imxsd = &imxmd->subdev[i];
- if (!imxsd->pdev)
+ list_for_each_entry(imxasd, &imxmd->asd_list, list) {
+ if (!imxasd->pdev)
continue;
- platform_device_unregister(imxsd->pdev);
+
+ platform_device_unregister(imxasd->pdev);
}
}
diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
index eb7a7f2..acde372 100644
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -76,7 +76,6 @@ static int
of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np,
bool is_csi_port)
{
- struct imx_media_subdev *imxsd;
int i, num_ports, ret;
if (!of_device_is_available(sd_np)) {
@@ -87,9 +86,8 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np,
}
/* register this subdev with async notifier */
- imxsd = imx_media_add_async_subdev(imxmd, of_fwnode_handle(sd_np),
- NULL);
- ret = PTR_ERR_OR_ZERO(imxsd);
+ ret = imx_media_add_async_subdev(imxmd, of_fwnode_handle(sd_np),
+ NULL);
if (ret) {
if (ret == -EEXIST) {
/* already added, everything is fine */
@@ -159,37 +157,35 @@ int imx_media_add_of_subdevs(struct imx_media_dev *imxmd,
}
/*
- * Create a single media link to/from imxsd using a fwnode link.
+ * Create a single media link to/from sd using a fwnode link.
*
* NOTE: this function assumes an OF port node is equivalent to
* a media pad (port id equal to media pad index), and that an
* OF endpoint node is equivalent to a media link.
*/
static int create_of_link(struct imx_media_dev *imxmd,
- struct imx_media_subdev *imxsd,
+ struct v4l2_subdev *sd,
struct v4l2_fwnode_link *link)
{
- struct v4l2_subdev *sd = imxsd->sd;
- struct imx_media_subdev *remote;
- struct v4l2_subdev *src, *sink;
+ struct v4l2_subdev *remote, *src, *sink;
int src_pad, sink_pad;
if (link->local_port >= sd->entity.num_pads)
return -EINVAL;
- remote = imx_media_find_async_subdev(imxmd, link->remote_node, NULL);
+ remote = imx_media_find_subdev_by_fwnode(imxmd, link->remote_node);
if (!remote)
return 0;
if (sd->entity.pads[link->local_port].flags & MEDIA_PAD_FL_SINK) {
- src = remote->sd;
+ src = remote;
src_pad = link->remote_port;
sink = sd;
sink_pad = link->local_port;
} else {
src = sd;
src_pad = link->local_port;
- sink = remote->sd;
+ sink = remote;
sink_pad = link->remote_port;
}
@@ -206,12 +202,11 @@ static int create_of_link(struct imx_media_dev *imxmd,
}
/*
- * Create media links to/from imxsd using its device-tree endpoints.
+ * Create media links to/from sd using its device-tree endpoints.
*/
int imx_media_create_of_links(struct imx_media_dev *imxmd,
- struct imx_media_subdev *imxsd)
+ struct v4l2_subdev *sd)
{
- struct v4l2_subdev *sd = imxsd->sd;
struct v4l2_fwnode_link link;
struct device_node *ep;
int ret;
@@ -221,7 +216,7 @@ int imx_media_create_of_links(struct imx_media_dev *imxmd,
if (ret)
continue;
- ret = create_of_link(imxmd, imxsd, &link);
+ ret = create_of_link(imxmd, sd, &link);
v4l2_fwnode_put_link(&link);
if (ret)
return ret;
@@ -235,9 +230,9 @@ int imx_media_create_of_links(struct imx_media_dev *imxmd,
* using its device-tree endpoints.
*/
int imx_media_create_csi_of_links(struct imx_media_dev *imxmd,
- struct imx_media_subdev *csi)
+ struct v4l2_subdev *csi)
{
- struct device_node *csi_np = csi->sd->dev->of_node;
+ struct device_node *csi_np = csi->dev->of_node;
struct fwnode_handle *fwnode, *csi_ep;
struct v4l2_fwnode_link link;
struct device_node *ep;
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 51d7e18..e143a88 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -668,38 +668,35 @@ void imx_media_grp_id_to_sd_name(char *sd_name, int sz, u32 grp_id, int ipu_id)
}
EXPORT_SYMBOL_GPL(imx_media_grp_id_to_sd_name);
-struct imx_media_subdev *
-imx_media_find_subdev_by_sd(struct imx_media_dev *imxmd,
- struct v4l2_subdev *sd)
+struct v4l2_subdev *
+imx_media_find_subdev_by_fwnode(struct imx_media_dev *imxmd,
+ struct fwnode_handle *fwnode)
{
- struct imx_media_subdev *imxsd;
- int i;
+ struct v4l2_subdev *sd;
- for (i = 0; i < imxmd->num_subdevs; i++) {
- imxsd = &imxmd->subdev[i];
- if (sd == imxsd->sd)
- return imxsd;
+ list_for_each_entry(sd, &imxmd->v4l2_dev.subdevs, list) {
+ if (sd->fwnode == fwnode)
+ return sd;
}
- return ERR_PTR(-ENODEV);
+ return NULL;
}
-EXPORT_SYMBOL_GPL(imx_media_find_subdev_by_sd);
+EXPORT_SYMBOL_GPL(imx_media_find_subdev_by_fwnode);
-struct imx_media_subdev *
-imx_media_find_subdev_by_id(struct imx_media_dev *imxmd, u32 grp_id)
+struct v4l2_subdev *
+imx_media_find_subdev_by_devname(struct imx_media_dev *imxmd,
+ const char *devname)
{
- struct imx_media_subdev *imxsd;
- int i;
+ struct v4l2_subdev *sd;
- for (i = 0; i < imxmd->num_subdevs; i++) {
- imxsd = &imxmd->subdev[i];
- if (imxsd->sd && imxsd->sd->grp_id == grp_id)
- return imxsd;
+ list_for_each_entry(sd, &imxmd->v4l2_dev.subdevs, list) {
+ if (!strcmp(devname, dev_name(sd->dev)))
+ return sd;
}
- return ERR_PTR(-ENODEV);
+ return NULL;
}
-EXPORT_SYMBOL_GPL(imx_media_find_subdev_by_id);
+EXPORT_SYMBOL_GPL(imx_media_find_subdev_by_devname);
/*
* Adds a video device to the master video device list. This is called by
@@ -842,7 +839,7 @@ EXPORT_SYMBOL_GPL(imx_media_find_upstream_pad);
* the current pipeline.
* Must be called with mdev->graph_mutex held.
*/
-struct imx_media_subdev *
+struct v4l2_subdev *
imx_media_find_upstream_subdev(struct imx_media_dev *imxmd,
struct media_entity *start_entity,
u32 grp_id)
@@ -853,7 +850,7 @@ imx_media_find_upstream_subdev(struct imx_media_dev *imxmd,
if (!sd)
return ERR_PTR(-ENODEV);
- return imx_media_find_subdev_by_sd(imxmd, sd);
+ return sd;
}
EXPORT_SYMBOL_GPL(imx_media_find_upstream_subdev);
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index b208af41..042de92 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -11,6 +11,7 @@
#ifndef _IMX_MEDIA_H
#define _IMX_MEDIA_H
+#include <linux/platform_device.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
#include <media/v4l2-fwnode.h>
@@ -18,23 +19,8 @@
#include <media/videobuf2-dma-contig.h>
#include <video/imx-ipu-v3.h>
-/*
- * This is somewhat arbitrary, but we need at least:
- * - 4 video devices per IPU
- * - 3 IC subdevs per IPU
- * - 1 VDIC subdev per IPU
- * - 2 CSI subdevs per IPU
- * - 1 mipi-csi2 receiver subdev
- * - 2 video-mux subdevs
- * - 2 camera sensor subdevs per IPU (1 parallel, 1 mipi-csi2)
- *
- */
/* max video devices */
#define IMX_MEDIA_MAX_VDEVS 8
-/* max subdevices */
-#define IMX_MEDIA_MAX_SUBDEVS 32
-/* max pads per subdev */
-#define IMX_MEDIA_MAX_PADS 16
/*
* Pad definitions for the subdevs with multiple source or
@@ -105,6 +91,7 @@ static inline struct imx_media_buffer *to_imx_media_vb(struct vb2_buffer *vb)
return container_of(vbuf, struct imx_media_buffer, vbuf);
}
+/* to support control inheritance to video devices */
struct imx_media_pad {
/*
* list of video devices that can be reached from this pad,
@@ -114,22 +101,34 @@ struct imx_media_pad {
int num_vdevs;
};
+static inline struct imx_media_pad *
+to_imx_media_pad(struct v4l2_subdev *sd, int pad_index)
+{
+ struct imx_media_pad *imxpads = sd->host_priv;
+
+ return imxpads ? &imxpads[pad_index] : NULL;
+}
+
struct imx_media_internal_sd_platformdata {
char sd_name[V4L2_SUBDEV_NAME_SIZE];
u32 grp_id;
int ipu_id;
};
-struct imx_media_subdev {
- struct v4l2_async_subdev asd;
- struct v4l2_subdev *sd; /* set when bound */
-
- struct imx_media_pad pad[IMX_MEDIA_MAX_PADS];
- /* the platform device if this is an IPU-internal subdev */
+struct imx_media_async_subdev {
+ struct v4l2_async_subdev asd;
+ /* the platform device of IPU-internal subdevs */
struct platform_device *pdev;
+ struct list_head list;
};
+static inline struct imx_media_async_subdev *
+to_imx_media_asd(struct v4l2_async_subdev *asd)
+{
+ return container_of(asd, struct imx_media_async_subdev, asd);
+}
+
struct imx_media_dev {
struct media_device md;
struct v4l2_device v4l2_dev;
@@ -139,10 +138,6 @@ struct imx_media_dev {
struct mutex mutex; /* protect elements below */
- /* master subdevice list */
- struct imx_media_subdev subdev[IMX_MEDIA_MAX_SUBDEVS];
- int num_subdevs;
-
/* master video device list */
struct imx_media_video_dev *vdev[IMX_MEDIA_MAX_VDEVS];
int num_vdevs;
@@ -151,7 +146,7 @@ struct imx_media_dev {
struct ipu_soc *ipu[2];
/* for async subdev registration */
- struct v4l2_async_subdev *async_ptrs[IMX_MEDIA_MAX_SUBDEVS];
+ struct list_head asd_list;
struct v4l2_async_notifier subdev_notifier;
};
@@ -172,7 +167,6 @@ int imx_media_enum_mbus_format(u32 *code, u32 index, enum codespace_sel cs_sel,
const struct imx_media_pixfmt *
imx_media_find_ipu_format(u32 code, enum codespace_sel cs_sel);
int imx_media_enum_ipu_format(u32 *code, u32 index, enum codespace_sel cs_sel);
-
int imx_media_init_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
u32 width, u32 height, u32 code, u32 field,
const struct imx_media_pixfmt **cc);
@@ -186,30 +180,23 @@ int imx_media_mbus_fmt_to_ipu_image(struct ipu_image *image,
struct v4l2_mbus_framefmt *mbus);
int imx_media_ipu_image_to_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
struct ipu_image *image);
-
-struct imx_media_subdev *
-imx_media_find_async_subdev(struct imx_media_dev *imxmd,
- struct fwnode_handle *fwnode,
- const char *devname);
-struct imx_media_subdev *
-imx_media_add_async_subdev(struct imx_media_dev *imxmd,
- struct fwnode_handle *fwnode,
- struct platform_device *pdev);
-
+int imx_media_add_async_subdev(struct imx_media_dev *imxmd,
+ struct fwnode_handle *fwnode,
+ struct platform_device *pdev);
void imx_media_grp_id_to_sd_name(char *sd_name, int sz,
u32 grp_id, int ipu_id);
int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd);
int imx_media_create_internal_links(struct imx_media_dev *imxmd,
- struct imx_media_subdev *imxsd);
+ struct v4l2_subdev *sd);
void imx_media_remove_internal_subdevs(struct imx_media_dev *imxmd);
-struct imx_media_subdev *
-imx_media_find_subdev_by_sd(struct imx_media_dev *imxmd,
- struct v4l2_subdev *sd);
-struct imx_media_subdev *
-imx_media_find_subdev_by_id(struct imx_media_dev *imxmd,
- u32 grp_id);
+struct v4l2_subdev *
+imx_media_find_subdev_by_fwnode(struct imx_media_dev *imxmd,
+ struct fwnode_handle *fwnode);
+struct v4l2_subdev *
+imx_media_find_subdev_by_devname(struct imx_media_dev *imxmd,
+ const char *devname);
int imx_media_add_video_device(struct imx_media_dev *imxmd,
struct imx_media_video_dev *vdev);
int imx_media_find_mipi_csi2_channel(struct imx_media_dev *imxmd,
@@ -218,7 +205,7 @@ struct media_pad *
imx_media_find_upstream_pad(struct imx_media_dev *imxmd,
struct media_entity *start_entity,
u32 grp_id);
-struct imx_media_subdev *
+struct v4l2_subdev *
imx_media_find_upstream_subdev(struct imx_media_dev *imxmd,
struct media_entity *start_entity,
u32 grp_id);
@@ -253,9 +240,9 @@ void imx_media_fim_free(struct imx_media_fim *fim);
int imx_media_add_of_subdevs(struct imx_media_dev *dev,
struct device_node *np);
int imx_media_create_of_links(struct imx_media_dev *imxmd,
- struct imx_media_subdev *imxsd);
+ struct v4l2_subdev *sd);
int imx_media_create_csi_of_links(struct imx_media_dev *imxmd,
- struct imx_media_subdev *csi);
+ struct v4l2_subdev *csi);
/* imx-media-capture.c */
struct imx_media_video_dev *
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 7/9] media: staging/imx: convert static vdev lists to list_head
2017-10-28 20:36 [PATCH 0/9] media: imx: Add better OF graph support Steve Longerbeam
` (5 preceding siblings ...)
2017-10-28 20:36 ` [PATCH 6/9] media: staging/imx: remove static subdev arrays Steve Longerbeam
@ 2017-10-28 20:36 ` Steve Longerbeam
2017-10-28 20:36 ` [PATCH 8/9] media: staging/imx: reorder function prototypes Steve Longerbeam
` (2 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Steve Longerbeam @ 2017-10-28 20:36 UTC (permalink / raw)
To: Philipp Zabel, Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: linux-media, devel, linux-kernel, Russell King, Steve Longerbeam
Although not technically necessary because imx-media has only a
maximum of 8 video devices, and once setup the video device lists
are static, in anticipation of moving control ineritance to
v4l2-core, make the vdev lists more generic by converting to
dynamic list_head's.
After doing that, 'struct imx_media_pad' is now just a list_head
of video devices reachable from a pad. Allocate an array of list_head's,
one list_head for each pad, and attach that array to sd->host_priv.
An entry in the pad lists is of type 'struct imx_media_pad_vdev', and
points to a video device from the master list.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/staging/media/imx/imx-media-capture.c | 2 +
drivers/staging/media/imx/imx-media-dev.c | 77 +++++++++++++++------------
drivers/staging/media/imx/imx-media-utils.c | 16 +-----
drivers/staging/media/imx/imx-media.h | 39 +++++++-------
4 files changed, 68 insertions(+), 66 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index ea145ba..def3df2 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -754,6 +754,8 @@ imx_media_capture_device_init(struct v4l2_subdev *src_sd, int pad)
vfd->queue = &priv->q;
priv->vdev.vfd = vfd;
+ INIT_LIST_HEAD(&priv->vdev.list);
+
video_set_drvdata(vfd, priv);
v4l2_ctrl_handler_init(&priv->ctrl_hdlr, 0);
diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
index 40c7cfd..b6acddb 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -229,10 +229,11 @@ static int imx_media_add_vdev_to_pad(struct imx_media_dev *imxmd,
struct media_pad *srcpad)
{
struct media_entity *entity = srcpad->entity;
- struct imx_media_pad *imxpad;
+ struct imx_media_pad_vdev *pad_vdev;
+ struct list_head *pad_vdev_list;
struct media_link *link;
struct v4l2_subdev *sd;
- int i, vdev_idx, ret;
+ int i, ret;
/* skip this entity if not a v4l2_subdev */
if (!is_media_entity_v4l2_subdev(entity))
@@ -240,8 +241,8 @@ static int imx_media_add_vdev_to_pad(struct imx_media_dev *imxmd,
sd = media_entity_to_v4l2_subdev(entity);
- imxpad = to_imx_media_pad(sd, srcpad->index);
- if (!imxpad) {
+ pad_vdev_list = to_pad_vdev_list(sd, srcpad->index);
+ if (!pad_vdev_list) {
v4l2_warn(&imxmd->v4l2_dev, "%s:%u has no vdev list!\n",
entity->name, srcpad->index);
/*
@@ -251,23 +252,22 @@ static int imx_media_add_vdev_to_pad(struct imx_media_dev *imxmd,
return 0;
}
- vdev_idx = imxpad->num_vdevs;
-
/* just return if we've been here before */
- for (i = 0; i < vdev_idx; i++)
- if (vdev == imxpad->vdev[i])
+ list_for_each_entry(pad_vdev, pad_vdev_list, list) {
+ if (pad_vdev->vdev == vdev)
return 0;
-
- if (vdev_idx >= IMX_MEDIA_MAX_VDEVS) {
- dev_err(imxmd->md.dev, "can't add %s to pad %s:%u\n",
- vdev->vfd->entity.name, entity->name, srcpad->index);
- return -ENOSPC;
}
dev_dbg(imxmd->md.dev, "adding %s to pad %s:%u\n",
vdev->vfd->entity.name, entity->name, srcpad->index);
- imxpad->vdev[vdev_idx] = vdev;
- imxpad->num_vdevs++;
+
+ pad_vdev = devm_kzalloc(imxmd->md.dev, sizeof(*pad_vdev), GFP_KERNEL);
+ if (!pad_vdev)
+ return -ENOMEM;
+
+ /* attach this vdev to this pad */
+ pad_vdev->vdev = vdev;
+ list_add_tail(&pad_vdev->list, pad_vdev_list);
/* move upstream from this entity's sink pads */
for (i = 0; i < entity->num_pads; i++) {
@@ -289,22 +289,32 @@ static int imx_media_add_vdev_to_pad(struct imx_media_dev *imxmd,
return 0;
}
+/*
+ * For every subdevice, allocate an array of list_head's, one list_head
+ * for each pad, to hold the list of video devices reachable from that
+ * pad.
+ */
static int imx_media_alloc_pad_vdev_lists(struct imx_media_dev *imxmd)
{
- struct imx_media_pad *imxpads;
+ struct list_head *vdev_lists;
struct media_entity *entity;
struct v4l2_subdev *sd;
+ int i;
list_for_each_entry(sd, &imxmd->v4l2_dev.subdevs, list) {
entity = &sd->entity;
- imxpads = devm_kzalloc(imxmd->md.dev,
- entity->num_pads * sizeof(*imxpads),
- GFP_KERNEL);
- if (!imxpads)
+ vdev_lists = devm_kzalloc(
+ imxmd->md.dev,
+ entity->num_pads * sizeof(*vdev_lists),
+ GFP_KERNEL);
+ if (!vdev_lists)
return -ENOMEM;
- /* attach imxpads to the subdev's host private pointer */
- sd->host_priv = imxpads;
+ /* attach to the subdev's host private pointer */
+ sd->host_priv = vdev_lists;
+
+ for (i = 0; i < entity->num_pads; i++)
+ INIT_LIST_HEAD(to_pad_vdev_list(sd, i));
}
return 0;
@@ -315,14 +325,13 @@ static int imx_media_create_pad_vdev_lists(struct imx_media_dev *imxmd)
{
struct imx_media_video_dev *vdev;
struct media_link *link;
- int i, ret;
+ int ret;
ret = imx_media_alloc_pad_vdev_lists(imxmd);
if (ret)
return ret;
- for (i = 0; i < imxmd->num_vdevs; i++) {
- vdev = imxmd->vdev[i];
+ list_for_each_entry(vdev, &imxmd->vdev_list, list) {
link = list_first_entry(&vdev->vfd->entity.links,
struct media_link, list);
ret = imx_media_add_vdev_to_pad(imxmd, vdev, link->source);
@@ -405,11 +414,12 @@ static int imx_media_link_notify(struct media_link *link, u32 flags,
unsigned int notification)
{
struct media_entity *source = link->source->entity;
- struct imx_media_pad *imxpad;
+ struct imx_media_pad_vdev *pad_vdev;
+ struct list_head *pad_vdev_list;
struct imx_media_dev *imxmd;
struct video_device *vfd;
struct v4l2_subdev *sd;
- int i, pad_idx, ret;
+ int pad_idx, ret;
ret = v4l2_pipeline_link_notify(link, flags, notification);
if (ret)
@@ -424,8 +434,8 @@ static int imx_media_link_notify(struct media_link *link, u32 flags,
imxmd = dev_get_drvdata(sd->v4l2_dev->dev);
- imxpad = to_imx_media_pad(sd, pad_idx);
- if (!imxpad) {
+ pad_vdev_list = to_pad_vdev_list(sd, pad_idx);
+ if (!pad_vdev_list) {
/* shouldn't happen, but no reason to fail link setup */
return 0;
}
@@ -439,8 +449,8 @@ static int imx_media_link_notify(struct media_link *link, u32 flags,
*/
if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH &&
!(flags & MEDIA_LNK_FL_ENABLED)) {
- for (i = 0; i < imxpad->num_vdevs; i++) {
- vfd = imxpad->vdev[i]->vfd;
+ list_for_each_entry(pad_vdev, pad_vdev_list, list) {
+ vfd = pad_vdev->vdev->vfd;
dev_dbg(imxmd->md.dev,
"reset controls for %s\n",
vfd->entity.name);
@@ -449,8 +459,8 @@ static int imx_media_link_notify(struct media_link *link, u32 flags,
}
} else if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH &&
(link->flags & MEDIA_LNK_FL_ENABLED)) {
- for (i = 0; i < imxpad->num_vdevs; i++) {
- vfd = imxpad->vdev[i]->vfd;
+ list_for_each_entry(pad_vdev, pad_vdev_list, list) {
+ vfd = pad_vdev->vdev->vfd;
dev_dbg(imxmd->md.dev,
"refresh controls for %s\n",
vfd->entity.name);
@@ -505,6 +515,7 @@ static int imx_media_probe(struct platform_device *pdev)
dev_set_drvdata(imxmd->v4l2_dev.dev, imxmd);
INIT_LIST_HEAD(&imxmd->asd_list);
+ INIT_LIST_HEAD(&imxmd->vdev_list);
ret = imx_media_add_of_subdevs(imxmd, node);
if (ret) {
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index e143a88..13dafa7 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -705,24 +705,12 @@ EXPORT_SYMBOL_GPL(imx_media_find_subdev_by_devname);
int imx_media_add_video_device(struct imx_media_dev *imxmd,
struct imx_media_video_dev *vdev)
{
- int vdev_idx, ret = 0;
-
mutex_lock(&imxmd->mutex);
- vdev_idx = imxmd->num_vdevs;
- if (vdev_idx >= IMX_MEDIA_MAX_VDEVS) {
- dev_err(imxmd->md.dev,
- "%s: too many video devices! can't add %s\n",
- __func__, vdev->vfd->name);
- ret = -ENOSPC;
- goto out;
- }
+ list_add_tail(&vdev->list, &imxmd->vdev_list);
- imxmd->vdev[vdev_idx] = vdev;
- imxmd->num_vdevs++;
-out:
mutex_unlock(&imxmd->mutex);
- return ret;
+ return 0;
}
EXPORT_SYMBOL_GPL(imx_media_add_video_device);
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 042de92..79d7958 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -19,9 +19,6 @@
#include <media/videobuf2-dma-contig.h>
#include <video/imx-ipu-v3.h>
-/* max video devices */
-#define IMX_MEDIA_MAX_VDEVS 8
-
/*
* Pad definitions for the subdevs with multiple source or
* sink pads
@@ -82,6 +79,9 @@ struct imx_media_video_dev {
/* the user format */
struct v4l2_format fmt;
const struct imx_media_pixfmt *cc;
+
+ /* links this vdev to master list */
+ struct list_head list;
};
static inline struct imx_media_buffer *to_imx_media_vb(struct vb2_buffer *vb)
@@ -91,24 +91,26 @@ static inline struct imx_media_buffer *to_imx_media_vb(struct vb2_buffer *vb)
return container_of(vbuf, struct imx_media_buffer, vbuf);
}
-/* to support control inheritance to video devices */
-struct imx_media_pad {
- /*
- * list of video devices that can be reached from this pad,
- * list is only valid for source pads.
- */
- struct imx_media_video_dev *vdev[IMX_MEDIA_MAX_VDEVS];
- int num_vdevs;
-};
-
-static inline struct imx_media_pad *
-to_imx_media_pad(struct v4l2_subdev *sd, int pad_index)
+/*
+ * to support control inheritance to video devices, this
+ * retrieves a pad's list_head of video devices that can
+ * be reached from the pad. Note that only the lists in
+ * source pads get populated, sink pads have empty lists.
+ */
+static inline struct list_head *
+to_pad_vdev_list(struct v4l2_subdev *sd, int pad_index)
{
- struct imx_media_pad *imxpads = sd->host_priv;
+ struct list_head *vdev_list = sd->host_priv;
- return imxpads ? &imxpads[pad_index] : NULL;
+ return vdev_list ? &vdev_list[pad_index] : NULL;
}
+/* an entry in a pad's video device list */
+struct imx_media_pad_vdev {
+ struct imx_media_video_dev *vdev;
+ struct list_head list;
+};
+
struct imx_media_internal_sd_platformdata {
char sd_name[V4L2_SUBDEV_NAME_SIZE];
u32 grp_id;
@@ -139,8 +141,7 @@ struct imx_media_dev {
struct mutex mutex; /* protect elements below */
/* master video device list */
- struct imx_media_video_dev *vdev[IMX_MEDIA_MAX_VDEVS];
- int num_vdevs;
+ struct list_head vdev_list;
/* IPUs this media driver control, valid after subdevs bound */
struct ipu_soc *ipu[2];
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 8/9] media: staging/imx: reorder function prototypes
2017-10-28 20:36 [PATCH 0/9] media: imx: Add better OF graph support Steve Longerbeam
` (6 preceding siblings ...)
2017-10-28 20:36 ` [PATCH 7/9] media: staging/imx: convert static vdev lists to list_head Steve Longerbeam
@ 2017-10-28 20:36 ` Steve Longerbeam
2017-10-28 20:36 ` [PATCH 9/9] media: staging/imx: update TODO Steve Longerbeam
2017-12-04 13:44 ` [PATCH 0/9] media: imx: Add better OF graph support Hans Verkuil
9 siblings, 0 replies; 13+ messages in thread
From: Steve Longerbeam @ 2017-10-28 20:36 UTC (permalink / raw)
To: Philipp Zabel, Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: linux-media, devel, linux-kernel, Russell King, Steve Longerbeam
Re-order some of the function prototypes in imx-media.h to
group them correctly by source file. No functional changes.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/staging/media/imx/imx-media.h | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 79d7958..39b09fc 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -157,6 +157,7 @@ enum codespace_sel {
CS_SEL_ANY,
};
+/* imx-media-utils.c */
const struct imx_media_pixfmt *
imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel, bool allow_bayer);
int imx_media_enum_format(u32 *fourcc, u32 index, enum codespace_sel cs_sel);
@@ -181,17 +182,8 @@ int imx_media_mbus_fmt_to_ipu_image(struct ipu_image *image,
struct v4l2_mbus_framefmt *mbus);
int imx_media_ipu_image_to_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
struct ipu_image *image);
-int imx_media_add_async_subdev(struct imx_media_dev *imxmd,
- struct fwnode_handle *fwnode,
- struct platform_device *pdev);
void imx_media_grp_id_to_sd_name(char *sd_name, int sz,
u32 grp_id, int ipu_id);
-
-int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd);
-int imx_media_create_internal_links(struct imx_media_dev *imxmd,
- struct v4l2_subdev *sd);
-void imx_media_remove_internal_subdevs(struct imx_media_dev *imxmd);
-
struct v4l2_subdev *
imx_media_find_subdev_by_fwnode(struct imx_media_dev *imxmd,
struct fwnode_handle *fwnode);
@@ -227,6 +219,11 @@ int imx_media_pipeline_set_stream(struct imx_media_dev *imxmd,
struct media_entity *entity,
bool on);
+/* imx-media-dev.c */
+int imx_media_add_async_subdev(struct imx_media_dev *imxmd,
+ struct fwnode_handle *fwnode,
+ struct platform_device *pdev);
+
/* imx-media-fim.c */
struct imx_media_fim;
void imx_media_fim_eof_monitor(struct imx_media_fim *fim, struct timespec *ts);
@@ -237,6 +234,12 @@ int imx_media_fim_add_controls(struct imx_media_fim *fim);
struct imx_media_fim *imx_media_fim_init(struct v4l2_subdev *sd);
void imx_media_fim_free(struct imx_media_fim *fim);
+/* imx-media-internal-sd.c */
+int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd);
+int imx_media_create_internal_links(struct imx_media_dev *imxmd,
+ struct v4l2_subdev *sd);
+void imx_media_remove_internal_subdevs(struct imx_media_dev *imxmd);
+
/* imx-media-of.c */
int imx_media_add_of_subdevs(struct imx_media_dev *dev,
struct device_node *np);
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 9/9] media: staging/imx: update TODO
2017-10-28 20:36 [PATCH 0/9] media: imx: Add better OF graph support Steve Longerbeam
` (7 preceding siblings ...)
2017-10-28 20:36 ` [PATCH 8/9] media: staging/imx: reorder function prototypes Steve Longerbeam
@ 2017-10-28 20:36 ` Steve Longerbeam
2017-12-04 13:44 ` [PATCH 0/9] media: imx: Add better OF graph support Hans Verkuil
9 siblings, 0 replies; 13+ messages in thread
From: Steve Longerbeam @ 2017-10-28 20:36 UTC (permalink / raw)
To: Philipp Zabel, Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: linux-media, devel, linux-kernel, Russell King, Steve Longerbeam
Update TODO file:
- Remove TODO info about the OV564x driver, while this still needs
to be done (add a OV5642 driver or merge with OV5640 driver), it
is not relevant here.
- Update TODO about methods for retrieving CSI bus config.
- Add some TODO's about OF graph parsing restrictions.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/staging/media/imx/TODO | 63 ++++++++++++++++++++++++++++++++++--------
1 file changed, 51 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/media/imx/TODO b/drivers/staging/media/imx/TODO
index 0bee313..9eb7326 100644
--- a/drivers/staging/media/imx/TODO
+++ b/drivers/staging/media/imx/TODO
@@ -1,19 +1,14 @@
-- Clean up and move the ov5642 subdev driver to drivers/media/i2c, or
- merge support for OV5642 into drivers/media/i2c/ov5640.c, and create
- the binding docs for it.
-
- The Frame Interval Monitor could be exported to v4l2-core for
general use.
-- At driver load time, the device-tree node that is the original source
- (the "sensor"), is parsed to record its media bus configuration, and
- this info is required in imx-media-csi.c to setup the CSI.
- Laurent Pinchart argues that instead the CSI subdev should call its
- neighbor's g_mbus_config op (which should be propagated if necessary)
- to get this info. However Hans Verkuil is planning to remove the
- g_mbus_config op. For now this driver uses the parsed DT mbus config
- method until this issue is resolved.
+- The CSI subdevice parses its nearest upstream neighbor's device-tree
+ bus config in order to setup the CSI. Laurent Pinchart argues that
+ instead the CSI subdev should call its neighbor's g_mbus_config op
+ (which should be propagated if necessary) to get this info. However
+ Hans Verkuil is planning to remove the g_mbus_config op. For now this
+ driver uses the parsed DT bus config method until this issue is
+ resolved.
- This media driver supports inheriting V4L2 controls to the
video capture devices, from the subdevices in the capture device's
@@ -21,3 +16,47 @@
link_notify callback when the pipeline is modified. It should be
decided whether this feature is useful enough to make it generally
available by exporting to v4l2-core.
+
+- The OF graph is walked at probe time to form the list of fwnodes to
+ be passed to v4l2_async_notifier_register(), starting from the IPU
+ CSI ports. And after all async subdevices have been bound,
+ v4l2_fwnode_parse_link() is used to form the media links between
+ the entities discovered by walking the OF graph.
+
+ While this approach allows support for arbitrary OF graphs, there
+ are some assumptions for this to work:
+
+ 1. All port parent nodes reachable in the graph from the IPU CSI
+ ports bind to V4L2 async subdevice drivers.
+
+ If a device has mixed-use ports such as video plus audio, the
+ endpoints from the audio ports are followed to devices that must
+ bind to V4L2 subdevice drivers, and not for example, to an ALSA
+ driver or a non-V4L2 media driver. If the device were bound to
+ such a driver, imx-media would never get an async completion
+ notification because the device fwnode was added to the async
+ list, but the driver does not interface with the V4L2 async
+ framework.
+
+ 2. Every port reachable in the graph is treated as a media pad,
+ owned by the V4L2 subdevice that is bound to the port's parent.
+
+ This presents problems for devices that don't make this port = pad
+ assumption. Examples are SMIAPP compatible cameras which define only
+ a single output port node, but which define multiple pads owned
+ by multiple subdevices (pixel-array, binner, scaler). Or video
+ decoders (entity function MEDIA_ENT_F_ATV_DECODER), which also define
+ only a single output port node, but define multiple pads for video,
+ VBI, and audio out.
+
+ A workaround at present is to set the port reg properties to
+ correspond to the media pad index that the port represents. A
+ possible long-term solution is to implement a subdev API that
+ maps a port id to a media pad index.
+
+ 3. Every endpoint of a port reachable in the graph is treated as
+ a media link, between V4L2 subdevices that are bound to the
+ port parents of the local and remote endpoints.
+
+ Which means a port must not contain mixed-use endpoints, they
+ must all refer to media links between V4L2 subdevices.
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 0/9] media: imx: Add better OF graph support
2017-10-28 20:36 [PATCH 0/9] media: imx: Add better OF graph support Steve Longerbeam
` (8 preceding siblings ...)
2017-10-28 20:36 ` [PATCH 9/9] media: staging/imx: update TODO Steve Longerbeam
@ 2017-12-04 13:44 ` Hans Verkuil
2017-12-07 23:23 ` Steve Longerbeam
9 siblings, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2017-12-04 13:44 UTC (permalink / raw)
To: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
Greg Kroah-Hartman
Cc: linux-media, devel, linux-kernel, Russell King, Steve Longerbeam
Hi Steve,
On 10/28/2017 10:36 PM, Steve Longerbeam wrote:
> This is a set of patches that improve support for more complex OF
> graphs. Currently the imx-media driver only supports a single device
> with a single port connected directly to either the CSI muxes or the
> MIPI CSI-2 receiver input ports. There can't be a multi-port device in
> between. This patch set removes those limitations.
>
> For an example taken from automotive, a camera sensor or decoder could
> be literally a remote device accessible over a FPD-III link, via TI
> DS90Ux9xx deserializer/serializer pairs. This patch set would support
> such OF graphs.
>
> There are still some assumptions and restrictions, regarding the equivalence
> of device-tree ports, port parents, and endpoints to media pads, entities,
> and links that have been enumerated in the TODO file.
Before I merge this patch series I wanted to know if Sakari's async work
that has now been merged (see https://www.spinics.net/lists/linux-media/msg124082.html)
affects this patch series.
It still applies cleanly, but I wondered if the subnotifier improvements
would simplify this driver.
Of course, any such simplification can also be done after this series has
been applied, but I don't know what your thoughts are on this.
Regards,
Hans
> This patch set supersedes the following patches submitted earlier:
>
> "[PATCH v2] media: staging/imx: do not return error in link_notify for unknown sources"
> "[PATCH RFC] media: staging/imx: fix complete handler"
>
> Tested by: Steve Longerbeam <steve_longerbeam@mentor.com>
> on SabreLite with the OV5640
>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> on Nitrogen6X with the TC358743.
>
> Tested-by: Russell King <rmk+kernel@armlinux.org.uk>
> with the IMX219
>
>
> Steve Longerbeam (9):
> media: staging/imx: get CSI bus type from nearest upstream entity
> media: staging/imx: remove static media link arrays
> media: staging/imx: of: allow for recursing downstream
> media: staging/imx: remove devname string from imx_media_subdev
> media: staging/imx: pass fwnode handle to find/add async subdev
> media: staging/imx: remove static subdev arrays
> media: staging/imx: convert static vdev lists to list_head
> media: staging/imx: reorder function prototypes
> media: staging/imx: update TODO
>
> drivers/staging/media/imx/TODO | 63 +++-
> drivers/staging/media/imx/imx-ic-prp.c | 4 +-
> drivers/staging/media/imx/imx-media-capture.c | 2 +
> drivers/staging/media/imx/imx-media-csi.c | 187 +++++-----
> drivers/staging/media/imx/imx-media-dev.c | 400 ++++++++++------------
> drivers/staging/media/imx/imx-media-internal-sd.c | 253 +++++++-------
> drivers/staging/media/imx/imx-media-of.c | 278 ++++++++-------
> drivers/staging/media/imx/imx-media-utils.c | 122 +++----
> drivers/staging/media/imx/imx-media.h | 187 ++++------
> 9 files changed, 722 insertions(+), 774 deletions(-)
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 0/9] media: imx: Add better OF graph support
2017-12-04 13:44 ` [PATCH 0/9] media: imx: Add better OF graph support Hans Verkuil
@ 2017-12-07 23:23 ` Steve Longerbeam
2017-12-08 1:05 ` Steve Longerbeam
0 siblings, 1 reply; 13+ messages in thread
From: Steve Longerbeam @ 2017-12-07 23:23 UTC (permalink / raw)
To: Hans Verkuil, Philipp Zabel, Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: linux-media, devel, linux-kernel, Russell King, Steve Longerbeam
Hi Hans,
On 12/04/2017 05:44 AM, Hans Verkuil wrote:
> Hi Steve,
>
> On 10/28/2017 10:36 PM, Steve Longerbeam wrote:
>> This is a set of patches that improve support for more complex OF
>> graphs. Currently the imx-media driver only supports a single device
>> with a single port connected directly to either the CSI muxes or the
>> MIPI CSI-2 receiver input ports. There can't be a multi-port device in
>> between. This patch set removes those limitations.
>>
>> For an example taken from automotive, a camera sensor or decoder could
>> be literally a remote device accessible over a FPD-III link, via TI
>> DS90Ux9xx deserializer/serializer pairs. This patch set would support
>> such OF graphs.
>>
>> There are still some assumptions and restrictions, regarding the equivalence
>> of device-tree ports, port parents, and endpoints to media pads, entities,
>> and links that have been enumerated in the TODO file.
> Before I merge this patch series I wanted to know if Sakari's async work
> that has now been merged (see https://www.spinics.net/lists/linux-media/msg124082.html)
> affects this patch series.
>
> It still applies cleanly, but I wondered if the subnotifier improvements
> would simplify this driver.
Indeed it does, it negates the need to recursively walk the OF graph to
discover
and register the fwnodes with the async notifier, as imx-media is
currently doing.
Of course it does require adding some code to the sub-devices, to call
v4l2_async_notifier_parse_fwnode_endpoints_by_port() to register remote
fwnodes attached to the sub-device's ports, and then
v4l2_async_subdev_notifier_register() to register a sub-notifier.
>
> Of course, any such simplification can also be done after this series has
> been applied, but I don't know what your thoughts are on this.
I do prefer the sub-notifier approach to discovering fwnodes, so yes I would
like to switch to this.
Since it is a distinct change (using sub-notifiers instead of recursive
graph
walk), I would prefer to get this series applied first, and then switch to
sub-notifiers as distinct patches afterwards.
Let me submit a v2 of this series first however. There are some minor
changes I
would like to make so that the up-coming sub-notifier patches are cleaner.
Steve
>
>> This patch set supersedes the following patches submitted earlier:
>>
>> "[PATCH v2] media: staging/imx: do not return error in link_notify for unknown sources"
>> "[PATCH RFC] media: staging/imx: fix complete handler"
>>
>> Tested by: Steve Longerbeam <steve_longerbeam@mentor.com>
>> on SabreLite with the OV5640
>>
>> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
>> on Nitrogen6X with the TC358743.
>>
>> Tested-by: Russell King <rmk+kernel@armlinux.org.uk>
>> with the IMX219
>>
>>
>> Steve Longerbeam (9):
>> media: staging/imx: get CSI bus type from nearest upstream entity
>> media: staging/imx: remove static media link arrays
>> media: staging/imx: of: allow for recursing downstream
>> media: staging/imx: remove devname string from imx_media_subdev
>> media: staging/imx: pass fwnode handle to find/add async subdev
>> media: staging/imx: remove static subdev arrays
>> media: staging/imx: convert static vdev lists to list_head
>> media: staging/imx: reorder function prototypes
>> media: staging/imx: update TODO
>>
>> drivers/staging/media/imx/TODO | 63 +++-
>> drivers/staging/media/imx/imx-ic-prp.c | 4 +-
>> drivers/staging/media/imx/imx-media-capture.c | 2 +
>> drivers/staging/media/imx/imx-media-csi.c | 187 +++++-----
>> drivers/staging/media/imx/imx-media-dev.c | 400 ++++++++++------------
>> drivers/staging/media/imx/imx-media-internal-sd.c | 253 +++++++-------
>> drivers/staging/media/imx/imx-media-of.c | 278 ++++++++-------
>> drivers/staging/media/imx/imx-media-utils.c | 122 +++----
>> drivers/staging/media/imx/imx-media.h | 187 ++++------
>> 9 files changed, 722 insertions(+), 774 deletions(-)
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/9] media: imx: Add better OF graph support
2017-12-07 23:23 ` Steve Longerbeam
@ 2017-12-08 1:05 ` Steve Longerbeam
0 siblings, 0 replies; 13+ messages in thread
From: Steve Longerbeam @ 2017-12-08 1:05 UTC (permalink / raw)
To: Hans Verkuil, Philipp Zabel, Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: linux-media, devel, linux-kernel, Russell King, Steve Longerbeam
On 12/07/2017 03:23 PM, Steve Longerbeam wrote:
> Hi Hans,
>
>
> On 12/04/2017 05:44 AM, Hans Verkuil wrote:
>
> <snip>
>>
>> Of course, any such simplification can also be done after this series
>> has
>> been applied, but I don't know what your thoughts are on this.
>
> I do prefer the sub-notifier approach to discovering fwnodes, so yes I
> would
> like to switch to this.
>
> Since it is a distinct change (using sub-notifiers instead of
> recursive graph
> walk), I would prefer to get this series applied first, and then
> switch to
> sub-notifiers as distinct patches afterwards.
>
> Let me submit a v2 of this series first however. There are some minor
> changes I
> would like to make so that the up-coming sub-notifier patches are
> cleaner.
>
Hans,
Never mind, the sub-notifier patches should be fairly clean. I won't
submit a
v2 of this series unless it is requested by reviewers.
Steve
^ permalink raw reply [flat|nested] 13+ messages in thread