Thank you again for testing this driver!
I am sorry I haven't had time to prepare a second RFC for this driver,
but I will try to do that as soon as possible.
On 02/17/2015 09:50 AM, Philipp Zabel wrote:
Hi Mats,
Am Montag, den 15.12.2014, 19:21 +0100 schrieb matrandg@xxxxxxxxx:
[...]
+static void tc358743_set_pll(struct v4l2_subdev *sd)
+{
+ struct tc358743_state *state = to_state(sd);
+ struct tc358743_platform_data *pdata = &state->pdata;
+ u16 pllctl0 = i2c_rd16(sd, PLLCTL0);
+ u16 pllctl0_new = SET_PLL_PRD(pdata->pll_prd) |
+ SET_PLL_FBD(pdata->pll_fbd);
+
+ v4l2_dbg(2, debug, sd, "%s:\n", __func__);
+
+ /* Only rewrite when needed, since rewriting triggers another format
+ * change event. */
+ if (pllctl0 != pllctl0_new) {
+ u32 hsck = (pdata->refclk_hz * pdata->pll_prd) / pdata->pll_fbd;
This is the wrong way around. refclk_hz is divided by pll_prd to get the
PLL input clock. The PLL then multiplies by pll_fbd. Example:
refclk_hz = 27000000, pll_prd = 4, pll_fbd = 88
--> hsck = refclk_hz / pll_prd * pll_fbd = 594 MHz, pll_frs should be 0.
Yes, you are right, and there was a bug in SET_PLL_FRS() as well, so the
bits where always set to zero. I will fix that!
[...]
+static unsigned tc358743_num_csi_lanes_needed(struct v4l2_subdev *sd)
+{
+ struct tc358743_state *state = to_state(sd);
+ struct v4l2_bt_timings *bt = &state->timings.bt;
+ u32 bits_pr_pixel =
+ (state->mbus_fmt_code == MEDIA_BUS_FMT_UYVY8_1X16) ? 16 : 24;
+ u32 bps = bt->width * bt->height * fps(bt) * bits_pr_pixel;
I think this calculation should include the blanking intervals.
As far as I understand is only the active video from the HDMI interface
transferred on the CSI interface, so I think this calculation is
correct. We transfer 1080p60 video on four lanes with 823.5 Mbps/lane,
which would not have been possible if the blanking should have been
transferred as well ((2200 * 1125 * 60 * 24) bps / 823.5 Mbps/lane =
4.33 lanes.
+static void tc358743_set_csi(struct v4l2_subdev *sd)
+{
+ struct tc358743_state *state = to_state(sd);
+ struct tc358743_platform_data *pdata = &state->pdata;
+ unsigned lanes = tc358743_num_csi_lanes_needed(sd);
+
+ v4l2_dbg(3, debug, sd, "%s:\n", __func__);
+
+ tc358743_reset(sd, MASK_CTXRST);
+
+ if (lanes < 1)
+ i2c_wr32(sd, CLW_CNTRL, MASK_CLW_LANEDISABLE);
+ if (lanes < 1)
+ i2c_wr32(sd, D0W_CNTRL, MASK_D0W_LANEDISABLE);
+ if (lanes < 2)
+ i2c_wr32(sd, D1W_CNTRL, MASK_D1W_LANEDISABLE);
+ if (lanes < 3)
+ i2c_wr32(sd, D2W_CNTRL, MASK_D2W_LANEDISABLE);
+ if (lanes < 4)
+ i2c_wr32(sd, D3W_CNTRL, MASK_D3W_LANEDISABLE);
+
+ i2c_wr32(sd, LINEINITCNT, pdata->lineinitcnt);
+ i2c_wr32(sd, LPTXTIMECNT, pdata->lptxtimecnt);
+ i2c_wr32(sd, TCLK_HEADERCNT, pdata->tclk_headercnt);
+ i2c_wr32(sd, THS_HEADERCNT, pdata->ths_headercnt);
+ i2c_wr32(sd, TWAKEUP, pdata->twakeup);
+ i2c_wr32(sd, THS_TRAILCNT, pdata->ths_trailcnt);
+ i2c_wr32(sd, HSTXVREGCNT, pdata->hstxvregcnt);
+
+ i2c_wr32(sd, HSTXVREGEN,
+ ((lanes > 0) ? MASK_CLM_HSTXVREGEN : 0x0) |
+ ((lanes > 0) ? MASK_D0M_HSTXVREGEN : 0x0) |
+ ((lanes > 1) ? MASK_D1M_HSTXVREGEN : 0x0) |
+ ((lanes > 2) ? MASK_D2M_HSTXVREGEN : 0x0) |
+ ((lanes > 3) ? MASK_D3M_HSTXVREGEN : 0x0));
+
+ i2c_wr32(sd, TXOPTIONCNTRL, MASK_CONTCLKMODE);
Since anything below can't be undone without pulling CTXRST, I propose
to split tc358743_set_csi into tc358743_set_csi (above) and
tc358743_start_csi (below).
To make this driver work with the Synopsys DesignWare MIPI CSI-2 Host
Controller, there needs to be a time when the lanes are in stop state
first, so the host can synchronize. I'd then like to call start_csi in
s_stream only.
With help from Toshiba we have now implemented start and stop of the CSI
interface without pulling CTXRST. You can see our solution in the next
RFC, and I would appreciate if you could test if that works fine for you
as well!
Regards,
Mats Randgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
