In tc358746_link_validate() an attempt is made to get the state lock of the
subdev, but since this is already held by the calling function
v4l2_subdev_link_validate(), this leads to a deadlock.
Another problem is that this function queries the link frequency of the
source device. Since some image sensors use the lock of the v4l2 control
handler as a state lock, which is already held at this point, a deadlock
can also occur here.
Move the calculation of the FIFO size from tc358746_link_validate() to
tc358746_apply_misc_config() to avoid the deadlocks mentioned.
Signed-off-by: Matthias Fend <matthias.fend@xxxxxxxxx>
---
drivers/media/i2c/tc358746.c | 191 +++++++++++++++++------------------
1 file changed, 90 insertions(+), 101 deletions(-)
diff --git a/drivers/media/i2c/tc358746.c b/drivers/media/i2c/tc358746.c
index 31586f8e4be4..8c169860b972 100644
--- a/drivers/media/i2c/tc358746.c
+++ b/drivers/media/i2c/tc358746.c
@@ -161,10 +161,6 @@ struct tc358746 {
u16 pll_pre_div;
u16 pll_mul;
-#define TC358746_VB_MAX_SIZE (511 * 32)
-#define TC358746_VB_DEFAULT_SIZE (1 * 32)
- unsigned int vb_size; /* Video buffer size in bits */
-
struct phy_configure_opts_mipi_dphy dphy_cfg;
};
@@ -415,6 +411,70 @@ tc358746_apply_pll_config(struct tc358746 *tc358746)
return tc358746_set_bits(tc358746, PLLCTL1_REG, CKEN);
}
+#define TC358746_VB_PRECISION 10
+#define TC358746_VB_MAX_SIZE (511 * 32)
+#define TC358746_VB_DEFAULT_SIZE (1 * 32)
+
+static int tc358746_calc_vb_size(struct tc358746 *tc358746,
+ s64 source_link_freq,
+ const struct v4l2_mbus_framefmt *mbusfmt,
+ const struct tc358746_format *fmt)
+{
+ unsigned long csi_bitrate, source_bitrate;
+ unsigned int fifo_sz, tmp, n;
+ int vb_size; /* Video buffer size in bits */
+
+ source_bitrate = source_link_freq * fmt->bus_width;
+
+ csi_bitrate = tc358746->dphy_cfg.lanes * tc358746->pll_rate;
+
+ dev_dbg(tc358746->sd.dev,
+ "Fifo settings params: source-bitrate:%lu csi-bitrate:%lu",
+ source_bitrate, csi_bitrate);
+
+ /* Avoid possible FIFO overflows */
+ if (csi_bitrate < source_bitrate)
+ return -EINVAL;
+
+ /* Best case */
+ if (csi_bitrate == source_bitrate) {
+ fifo_sz = TC358746_VB_DEFAULT_SIZE;
+ vb_size = TC358746_VB_DEFAULT_SIZE;
+ } else {
+ /*
+ * Avoid possible FIFO underflow in case of
+ * csi_bitrate > source_bitrate. For such case the chip has a internal
+ * fifo which can be used to delay the line output.
+ *
+ * Fifo size calculation (excluding precision):
+ *
+ * fifo-sz, image-width - in bits
+ * sbr - source_bitrate in bits/s
+ * csir - csi_bitrate in bits/s
+ *
+ * image-width / csir >= (image-width - fifo-sz) / sbr
+ * image-width * sbr / csir >= image-width - fifo-sz
+ * fifo-sz >= image-width - image-width * sbr / csir; with n = csir/sbr
+ * fifo-sz >= image-width - image-width / n
+ */
+ source_bitrate /= TC358746_VB_PRECISION;
+ n = csi_bitrate / source_bitrate;
+ tmp = (mbusfmt->width * TC358746_VB_PRECISION) / n;
+ fifo_sz = mbusfmt->width - tmp;
+ fifo_sz *= fmt->bpp;
+ vb_size = round_up(fifo_sz, 32);
+ }
+
+ dev_dbg(tc358746->sd.dev,
+ "Found FIFO size[bits]:%u -> aligned to size[bits]:%u\n",
+ fifo_sz, vb_size);
+
+ if (vb_size > TC358746_VB_MAX_SIZE)
+ return -EINVAL;
+
+ return vb_size;
+}
+
static int tc358746_apply_misc_config(struct tc358746 *tc358746)
{
const struct v4l2_mbus_framefmt *mbusfmt;
@@ -422,6 +482,9 @@ static int tc358746_apply_misc_config(struct tc358746 *tc358746)
struct v4l2_subdev_state *sink_state;
const struct tc358746_format *fmt;
struct device *dev = sd->dev;
+ struct media_pad *source_pad;
+ s64 source_link_freq;
+ int vb_size;
u32 val;
int err;
@@ -430,6 +493,21 @@ static int tc358746_apply_misc_config(struct tc358746 *tc358746)
mbusfmt = v4l2_subdev_state_get_format(sink_state, TC358746_SINK);
fmt = tc358746_get_format_by_code(TC358746_SINK, mbusfmt->code);
+ source_pad = media_entity_remote_source_pad_unique(&sd->entity);
+ if (IS_ERR(source_pad)) {
+ dev_err(dev, "Failed to get source pad of %s\n", sd->name);
+ err = PTR_ERR(source_pad);
+ goto out;
+ }
+ source_link_freq = v4l2_get_link_freq(source_pad, 0, 0);
+ if (source_link_freq <= 0) {
+ dev_err(dev,
+ "Failed to query or invalid source link frequency\n");
+ /* Return -EINVAL in case of source_link_freq is 0 */
+ err = source_link_freq ?: -EINVAL;
+ goto out;
+ }
+
/* Self defined CSI user data type id's are not supported yet */
val = PDFMT(fmt->pdformat);
dev_dbg(dev, "DATAFMT: 0x%x\n", val);
@@ -443,7 +521,13 @@ static int tc358746_apply_misc_config(struct tc358746 *tc358746)
if (err)
goto out;
- val = tc358746->vb_size / 32;
+ vb_size = tc358746_calc_vb_size(tc358746, source_link_freq, mbusfmt, fmt);
+ if (vb_size < 0) {
+ err = vb_size;
+ goto out;
+ }
+
+ val = vb_size / 32;
dev_dbg(dev, "FIFOCTL: %u (0x%x)\n", val, val);
err = tc358746_write(tc358746, FIFOCTL_REG, val);
if (err)
@@ -882,99 +966,6 @@ static unsigned long tc358746_find_pll_settings(struct tc358746 *tc358746,
return best_freq;
}
-#define TC358746_PRECISION 10
-
-static int
-tc358746_link_validate(struct v4l2_subdev *sd, struct media_link *link,
- struct v4l2_subdev_format *source_fmt,
- struct v4l2_subdev_format *sink_fmt)
-{
- struct tc358746 *tc358746 = to_tc358746(sd);
- unsigned long csi_bitrate, source_bitrate;
- struct v4l2_subdev_state *sink_state;
- struct v4l2_mbus_framefmt *mbusfmt;
- const struct tc358746_format *fmt;
- unsigned int fifo_sz, tmp, n;
- struct v4l2_subdev *source;
- struct media_pad *src_pad;
- s64 source_link_freq;
- int err;
-
- err = v4l2_subdev_link_validate_default(sd, link, source_fmt, sink_fmt);
- if (err)
- return err;
-
- sink_state = v4l2_subdev_lock_and_get_active_state(sd);
- mbusfmt = v4l2_subdev_state_get_format(sink_state, TC358746_SINK);
-
- /* Check the FIFO settings */
- fmt = tc358746_get_format_by_code(TC358746_SINK, mbusfmt->code);
-
- source = media_entity_to_v4l2_subdev(link->source->entity);
- src_pad = &source->entity.pads[source_fmt->pad];
- source_link_freq = v4l2_get_link_freq(src_pad, 0, 0);
- if (source_link_freq <= 0) {
- dev_err(tc358746->sd.dev,
- "Failed to query or invalid source link frequency\n");
- v4l2_subdev_unlock_state(sink_state);
- /* Return -EINVAL in case of source_link_freq is 0 */
- return source_link_freq ? : -EINVAL;
- }
- source_bitrate = source_link_freq * fmt->bus_width;
-
- csi_bitrate = tc358746->dphy_cfg.lanes * tc358746->pll_rate;
-
- dev_dbg(tc358746->sd.dev,
- "Fifo settings params: source-bitrate:%lu csi-bitrate:%lu",
- source_bitrate, csi_bitrate);
-
- /* Avoid possible FIFO overflows */
- if (csi_bitrate < source_bitrate) {
- v4l2_subdev_unlock_state(sink_state);
- return -EINVAL;
- }
-
- /* Best case */
- if (csi_bitrate == source_bitrate) {
- fifo_sz = TC358746_VB_DEFAULT_SIZE;
- tc358746->vb_size = TC358746_VB_DEFAULT_SIZE;
- goto out;
- }
-
- /*
- * Avoid possible FIFO underflow in case of
- * csi_bitrate > source_bitrate. For such case the chip has a internal
- * fifo which can be used to delay the line output.
- *
- * Fifo size calculation (excluding precision):
- *
- * fifo-sz, image-width - in bits
- * sbr - source_bitrate in bits/s
- * csir - csi_bitrate in bits/s
- *
- * image-width / csir >= (image-width - fifo-sz) / sbr
- * image-width * sbr / csir >= image-width - fifo-sz
- * fifo-sz >= image-width - image-width * sbr / csir; with n = csir/sbr
- * fifo-sz >= image-width - image-width / n
- */
-
- source_bitrate /= TC358746_PRECISION;
- n = csi_bitrate / source_bitrate;
- tmp = (mbusfmt->width * TC358746_PRECISION) / n;
- fifo_sz = mbusfmt->width - tmp;
- fifo_sz *= fmt->bpp;
- tc358746->vb_size = round_up(fifo_sz, 32);
-
-out:
- dev_dbg(tc358746->sd.dev,
- "Found FIFO size[bits]:%u -> aligned to size[bits]:%u\n",
- fifo_sz, tc358746->vb_size);
-
- v4l2_subdev_unlock_state(sink_state);
-
- return tc358746->vb_size > TC358746_VB_MAX_SIZE ? -EINVAL : 0;
-}
-
static int tc358746_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad,
struct v4l2_mbus_config *config)
{
@@ -1042,7 +1033,7 @@ static const struct v4l2_subdev_pad_ops tc358746_pad_ops = {
.enum_mbus_code = tc358746_enum_mbus_code,
.set_fmt = tc358746_set_fmt,
.get_fmt = v4l2_subdev_get_fmt,
- .link_validate = tc358746_link_validate,
+ .link_validate = v4l2_subdev_link_validate_default,
.get_mbus_config = tc358746_get_mbus_config,
};
@@ -1354,8 +1345,6 @@ tc358746_init_output_port(struct tc358746 *tc358746, unsigned long refclk)
if (err)
goto err;
- tc358746->vb_size = TC358746_VB_DEFAULT_SIZE;
-
return 0;
err:
--
2.34.1
