media: tc358746: fix locking issue

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@emfend.at>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>

diff --git a/drivers/media/i2c/tc358746.c b/drivers/media/i2c/tc358746.c
index f7d7bfb42dab32b33fab318e08d0ed4ade5345c2..143aa1359aba51c2a36e8800e7c1149698268ca1 100644 (file)
--- 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;
 };
 
@@ -440,6 +436,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;
@@ -447,6 +507,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;
 
@@ -455,6 +518,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);
@@ -468,7 +546,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)
@@ -902,99 +986,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)
 {
@@ -1062,7 +1053,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,
 };
 
@@ -1374,8 +1365,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: