mirror of
https://github.com/torvalds/linux.git
synced 2025-04-12 06:49:52 +00:00
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>
This commit is contained in:
Matthias Fend
Hans Verkuil
parent
78d7265e2e
commit
7d8fa0ee43
@ -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:
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user