Re: [PATCH 0/3] tc358743: minor driver fixes
Hi Dave, On Fri, 2017-06-02 at 15:36 +0100, Dave Stevenson wrote: [...] > >> > Are you aware of the HDMI modes that the TC358743 driver has been used > >> > with? > >> > The comments mention 720P60 at 594MHz, but I have had to modify the > >> > fifo_level value from 16 to 110 to get VGA60 or 576P50 to work. (The > >> > value came out of Toshiba's spreadsheet for computing register > >> > settings). It increases the delay by 2.96usecs at 720P60 on 2 lanes, > >> > so not a huge change. > >> > Is it worth going to the effort of dynamically computing the delay, or > >> > is increasing the default acceptable? > >> > >> I see that the fifo_level value of 16 was supplied by Philipp Zabel, so > >> I have CC-ed him as I am not sure where those values came from. > > > > I've just chosen a small delay that worked reliably. For 4-lane 1080p60 > > and for 2-lane 720p60 at 594 Mbps lane speed, the Toshiba spreadsheet > > believes that it is ok to decrease the FIFO delay all the way down to 0 > > (it is not). I think it should be fine to delay transmission for a few > > microseconds unconditionally, I'll test this next week. > > Thanks Philipp. Were 1080p60 and 720p60 the only modes you were really > testing? Yes. Since 720p60 needs half the bandwidth of 1080p60, it was possible to just use the 1080p60 timings by switching from 4 to 2 lanes. > Did the 594Mbps lane speed come from a specific requirement somewhere? It is what the spreadsheed suggests for 4-lane 1080p60, I assume because it is nice and even to generate from the 27 MHz reference clock, and it fits the 547.28 Mbps/lane requirement (for YCbCr 4:2:2 CSI output) with a bit of headroom. > The standard Pi only has 2 CSI2 lanes exposed, and 1080P30 RGB3 just > tips over into needing 3 lanes with the current link frequency. > When I can find a bit more time I was thinking that an alternate link > frequency would allow us to squeeze it in to 2 lanes. Obviously the > timing values need to be checked carefully, but it should all work and > allow support of multiple link frequencies. See the FIXME comment in tc358743_probe_of, you could calculate and add the correct pdata for the raised link-frequency. > (My calcs say that 1080p50 UYVY can fit down 2 lanes, but that > requires more extensive register mods). With a lane rate of 911.25 Mbps or higher that should be possible, with all the CSI timings are relaxed a bit. regards Philipp
Re: [PATCH 0/3] tc358743: minor driver fixes
On 2 June 2017 at 15:13, Philipp Zabel wrote:
> On Fri, 2017-06-02 at 15:27 +0200, Hans Verkuil wrote:
>> On 06/02/17 15:03, Dave Stevenson wrote:
>> > Hi Hans.
>> >
>> > On 2 June 2017 at 13:35, Hans Verkuil wrote:
>> >> On 06/02/17 14:18, Dave Stevenson wrote:
>> >>> These 3 patches for TC358743 came out of trying to use the
>> >>> existing driver with a new Raspberry Pi CSI-2 receiver driver.
>> >>
>> >> Nice! Doing that has been on my todo list for ages but I never got
>> >> around to it. I have one of these and using the Raspberry Pi with
>> >> the tc358743 would allow me to add a CEC driver as well.
>> >
>> > It's been on my list for a while too! It's working, but just the final
>> > clean ups needed.
>> > I've got 1 v4l2-compliance failure still outstanding that needs
>> > digging into (subscribe_event), rebasing on top of the fwnode tree,
>> > and a couple of config things to tidy up. RFC hopefully next week.
>> > I'm testing with a demo board designed here at Pi Towers, but there
>> > are others successfully testing it using the auvidea.com B101 board.
>> >
>> > Are you aware of the HDMI modes that the TC358743 driver has been used
>> > with?
>> > The comments mention 720P60 at 594MHz, but I have had to modify the
>> > fifo_level value from 16 to 110 to get VGA60 or 576P50 to work. (The
>> > value came out of Toshiba's spreadsheet for computing register
>> > settings). It increases the delay by 2.96usecs at 720P60 on 2 lanes,
>> > so not a huge change.
>> > Is it worth going to the effort of dynamically computing the delay, or
>> > is increasing the default acceptable?
>>
>> I see that the fifo_level value of 16 was supplied by Philipp Zabel, so
>> I have CC-ed him as I am not sure where those values came from.
>
> I've just chosen a small delay that worked reliably. For 4-lane 1080p60
> and for 2-lane 720p60 at 594 Mbps lane speed, the Toshiba spreadsheet
> believes that it is ok to decrease the FIFO delay all the way down to 0
> (it is not). I think it should be fine to delay transmission for a few
> microseconds unconditionally, I'll test this next week.
Thanks Philipp. Were 1080p60 and 720p60 the only modes you were really testing?
Did the 594Mbps lane speed come from a specific requirement somewhere?
The standard Pi only has 2 CSI2 lanes exposed, and 1080P30 RGB3 just
tips over into needing 3 lanes with the current link frequency. When I
can find a bit more time I was thinking that an alternate link
frequency would allow us to squeeze it in to 2 lanes. Obviously the
timing values need to be checked carefully, but it should all work and
allow support of multiple link frequencies.
(My calcs say that 1080p50 UYVY can fit down 2 lanes, but that
requires more extensive register mods).
>> This driver is also used in a Cisco product, but we use platform_data for
>> that.
>> Here are our settings that we use for reference:
>>
>> static struct tc358743_platform_data tc358743_pdata = {
>> .refclk_hz = 2700,
>> .ddc5v_delay = DDC5V_DELAY_100_MS,
>> .fifo_level = 300,
>> .pll_prd = 4,
>> .pll_fbd = 122,
>> /* CSI */
>> .lineinitcnt = 0x1770,
>> .lptxtimecnt = 0x0005,
>> .tclk_headercnt = 0x1d04,
>> .ths_headercnt = 0x0505,
>> .twakeup = 0x4650,
>> .ths_trailcnt = 0x0004,
>> .hstxvregcnt = 0x0005,
>> /* HDMI PHY */
>> .hdmi_phy_auto_reset_tmds_detected = true,
>> .hdmi_phy_auto_reset_tmds_in_range = true,
>> .hdmi_phy_auto_reset_tmds_valid = true,
>> .hdmi_phy_auto_reset_hsync_out_of_range = true,
>> .hdmi_phy_auto_reset_vsync_out_of_range = true,
>> .hdmi_detection_delay = HDMI_MODE_DELAY_25_MS,
>> };
>>
>> I believe these are all calculated from the Toshiba spreadsheet.
>>
>> Frankly, I have no idea what they mean :-)
>>
>> I am fine with increasing the default if Philipp is OK as well. Since
>> Cisco uses a value of 300 I would expect that 16 is indeed too low.
>>
>> Regards,
>>
>> Hans
>>
>> >
>> >>> A couple of the subdevice API calls were not implemented or
>> >>> otherwise gave odd results. Those are fixed.
>> >>>
>> >>> The TC358743 interface board being used didn't have the IRQ
>> >>> line wired up to the SoC. "interrupts" is listed as being
>> >>> optional in the DT binding, but the driver didn't actually
>> >>> function if it wasn't provided.
>> >>>
>> >>> Dave Stevenson (3):
>> >>> [media] tc358743: Add enum_mbus_code
>> >>> [media] tc358743: Setup default mbus_fmt before registering
>> >>> [media] tc358743: Add support for platforms without IRQ line
>> >>
>> >> All looks good, I'll take this for 4.12.
>> >
>> > Thanks.
>> >
>> >> Regards,
>> >>
>> >> Hans
>> >>
>> >>>
>> >>> drivers/media/i2c/tc358743.c
Re: [PATCH 0/3] tc358743: minor driver fixes
On Fri, 2017-06-02 at 15:27 +0200, Hans Verkuil wrote:
> On 06/02/17 15:03, Dave Stevenson wrote:
> > Hi Hans.
> >
> > On 2 June 2017 at 13:35, Hans Verkuil wrote:
> >> On 06/02/17 14:18, Dave Stevenson wrote:
> >>> These 3 patches for TC358743 came out of trying to use the
> >>> existing driver with a new Raspberry Pi CSI-2 receiver driver.
> >>
> >> Nice! Doing that has been on my todo list for ages but I never got
> >> around to it. I have one of these and using the Raspberry Pi with
> >> the tc358743 would allow me to add a CEC driver as well.
> >
> > It's been on my list for a while too! It's working, but just the final
> > clean ups needed.
> > I've got 1 v4l2-compliance failure still outstanding that needs
> > digging into (subscribe_event), rebasing on top of the fwnode tree,
> > and a couple of config things to tidy up. RFC hopefully next week.
> > I'm testing with a demo board designed here at Pi Towers, but there
> > are others successfully testing it using the auvidea.com B101 board.
> >
> > Are you aware of the HDMI modes that the TC358743 driver has been used with?
> > The comments mention 720P60 at 594MHz, but I have had to modify the
> > fifo_level value from 16 to 110 to get VGA60 or 576P50 to work. (The
> > value came out of Toshiba's spreadsheet for computing register
> > settings). It increases the delay by 2.96usecs at 720P60 on 2 lanes,
> > so not a huge change.
> > Is it worth going to the effort of dynamically computing the delay, or
> > is increasing the default acceptable?
>
> I see that the fifo_level value of 16 was supplied by Philipp Zabel, so
> I have CC-ed him as I am not sure where those values came from.
I've just chosen a small delay that worked reliably. For 4-lane 1080p60
and for 2-lane 720p60 at 594 Mbps lane speed, the Toshiba spreadsheet
believes that it is ok to decrease the FIFO delay all the way down to 0
(it is not). I think it should be fine to delay transmission for a few
microseconds unconditionally, I'll test this next week.
> This driver is also used in a Cisco product, but we use platform_data for
> that.
> Here are our settings that we use for reference:
>
> static struct tc358743_platform_data tc358743_pdata = {
> .refclk_hz = 2700,
> .ddc5v_delay = DDC5V_DELAY_100_MS,
> .fifo_level = 300,
> .pll_prd = 4,
> .pll_fbd = 122,
> /* CSI */
> .lineinitcnt = 0x1770,
> .lptxtimecnt = 0x0005,
> .tclk_headercnt = 0x1d04,
> .ths_headercnt = 0x0505,
> .twakeup = 0x4650,
> .ths_trailcnt = 0x0004,
> .hstxvregcnt = 0x0005,
> /* HDMI PHY */
> .hdmi_phy_auto_reset_tmds_detected = true,
> .hdmi_phy_auto_reset_tmds_in_range = true,
> .hdmi_phy_auto_reset_tmds_valid = true,
> .hdmi_phy_auto_reset_hsync_out_of_range = true,
> .hdmi_phy_auto_reset_vsync_out_of_range = true,
> .hdmi_detection_delay = HDMI_MODE_DELAY_25_MS,
> };
>
> I believe these are all calculated from the Toshiba spreadsheet.
>
> Frankly, I have no idea what they mean :-)
>
> I am fine with increasing the default if Philipp is OK as well. Since
> Cisco uses a value of 300 I would expect that 16 is indeed too low.
>
> Regards,
>
> Hans
>
> >
> >>> A couple of the subdevice API calls were not implemented or
> >>> otherwise gave odd results. Those are fixed.
> >>>
> >>> The TC358743 interface board being used didn't have the IRQ
> >>> line wired up to the SoC. "interrupts" is listed as being
> >>> optional in the DT binding, but the driver didn't actually
> >>> function if it wasn't provided.
> >>>
> >>> Dave Stevenson (3):
> >>> [media] tc358743: Add enum_mbus_code
> >>> [media] tc358743: Setup default mbus_fmt before registering
> >>> [media] tc358743: Add support for platforms without IRQ line
> >>
> >> All looks good, I'll take this for 4.12.
> >
> > Thanks.
> >
> >> Regards,
> >>
> >> Hans
> >>
> >>>
> >>> drivers/media/i2c/tc358743.c | 59
> >>> +++-
> >>> 1 file changed, 58 insertions(+), 1 deletion(-)
> >>>
> >>
regards
Philipp
Re: [PATCH 0/3] tc358743: minor driver fixes
On 06/02/17 15:03, Dave Stevenson wrote:
> Hi Hans.
>
> On 2 June 2017 at 13:35, Hans Verkuil wrote:
>> On 06/02/17 14:18, Dave Stevenson wrote:
>>> These 3 patches for TC358743 came out of trying to use the
>>> existing driver with a new Raspberry Pi CSI-2 receiver driver.
>>
>> Nice! Doing that has been on my todo list for ages but I never got
>> around to it. I have one of these and using the Raspberry Pi with
>> the tc358743 would allow me to add a CEC driver as well.
>
> It's been on my list for a while too! It's working, but just the final
> clean ups needed.
> I've got 1 v4l2-compliance failure still outstanding that needs
> digging into (subscribe_event), rebasing on top of the fwnode tree,
> and a couple of config things to tidy up. RFC hopefully next week.
> I'm testing with a demo board designed here at Pi Towers, but there
> are others successfully testing it using the auvidea.com B101 board.
>
> Are you aware of the HDMI modes that the TC358743 driver has been used with?
> The comments mention 720P60 at 594MHz, but I have had to modify the
> fifo_level value from 16 to 110 to get VGA60 or 576P50 to work. (The
> value came out of Toshiba's spreadsheet for computing register
> settings). It increases the delay by 2.96usecs at 720P60 on 2 lanes,
> so not a huge change.
> Is it worth going to the effort of dynamically computing the delay, or
> is increasing the default acceptable?
I see that the fifo_level value of 16 was supplied by Philipp Zabel, so
I have CC-ed him as I am not sure where those values came from. This driver
is also used in a Cisco product, but we use platform_data for that. Here are
our settings that we use for reference:
static struct tc358743_platform_data tc358743_pdata = {
.refclk_hz = 2700,
.ddc5v_delay = DDC5V_DELAY_100_MS,
.fifo_level = 300,
.pll_prd = 4,
.pll_fbd = 122,
/* CSI */
.lineinitcnt = 0x1770,
.lptxtimecnt = 0x0005,
.tclk_headercnt = 0x1d04,
.ths_headercnt = 0x0505,
.twakeup = 0x4650,
.ths_trailcnt = 0x0004,
.hstxvregcnt = 0x0005,
/* HDMI PHY */
.hdmi_phy_auto_reset_tmds_detected = true,
.hdmi_phy_auto_reset_tmds_in_range = true,
.hdmi_phy_auto_reset_tmds_valid = true,
.hdmi_phy_auto_reset_hsync_out_of_range = true,
.hdmi_phy_auto_reset_vsync_out_of_range = true,
.hdmi_detection_delay = HDMI_MODE_DELAY_25_MS,
};
I believe these are all calculated from the Toshiba spreadsheet.
Frankly, I have no idea what they mean :-)
I am fine with increasing the default if Philipp is OK as well. Since
Cisco uses a value of 300 I would expect that 16 is indeed too low.
Regards,
Hans
>
>>> A couple of the subdevice API calls were not implemented or
>>> otherwise gave odd results. Those are fixed.
>>>
>>> The TC358743 interface board being used didn't have the IRQ
>>> line wired up to the SoC. "interrupts" is listed as being
>>> optional in the DT binding, but the driver didn't actually
>>> function if it wasn't provided.
>>>
>>> Dave Stevenson (3):
>>> [media] tc358743: Add enum_mbus_code
>>> [media] tc358743: Setup default mbus_fmt before registering
>>> [media] tc358743: Add support for platforms without IRQ line
>>
>> All looks good, I'll take this for 4.12.
>
> Thanks.
>
>> Regards,
>>
>> Hans
>>
>>>
>>> drivers/media/i2c/tc358743.c | 59
>>> +++-
>>> 1 file changed, 58 insertions(+), 1 deletion(-)
>>>
>>
Re: [PATCH 0/3] tc358743: minor driver fixes
Hi Hans. On 2 June 2017 at 13:35, Hans Verkuil wrote: > On 06/02/17 14:18, Dave Stevenson wrote: >> These 3 patches for TC358743 came out of trying to use the >> existing driver with a new Raspberry Pi CSI-2 receiver driver. > > Nice! Doing that has been on my todo list for ages but I never got > around to it. I have one of these and using the Raspberry Pi with > the tc358743 would allow me to add a CEC driver as well. It's been on my list for a while too! It's working, but just the final clean ups needed. I've got 1 v4l2-compliance failure still outstanding that needs digging into (subscribe_event), rebasing on top of the fwnode tree, and a couple of config things to tidy up. RFC hopefully next week. I'm testing with a demo board designed here at Pi Towers, but there are others successfully testing it using the auvidea.com B101 board. Are you aware of the HDMI modes that the TC358743 driver has been used with? The comments mention 720P60 at 594MHz, but I have had to modify the fifo_level value from 16 to 110 to get VGA60 or 576P50 to work. (The value came out of Toshiba's spreadsheet for computing register settings). It increases the delay by 2.96usecs at 720P60 on 2 lanes, so not a huge change. Is it worth going to the effort of dynamically computing the delay, or is increasing the default acceptable? >> A couple of the subdevice API calls were not implemented or >> otherwise gave odd results. Those are fixed. >> >> The TC358743 interface board being used didn't have the IRQ >> line wired up to the SoC. "interrupts" is listed as being >> optional in the DT binding, but the driver didn't actually >> function if it wasn't provided. >> >> Dave Stevenson (3): >> [media] tc358743: Add enum_mbus_code >> [media] tc358743: Setup default mbus_fmt before registering >> [media] tc358743: Add support for platforms without IRQ line > > All looks good, I'll take this for 4.12. Thanks. > Regards, > > Hans > >> >> drivers/media/i2c/tc358743.c | 59 >> +++- >> 1 file changed, 58 insertions(+), 1 deletion(-) >> >
Re: [PATCH 0/3] tc358743: minor driver fixes
On 06/02/17 14:18, Dave Stevenson wrote: > These 3 patches for TC358743 came out of trying to use the > existing driver with a new Raspberry Pi CSI-2 receiver driver. Nice! Doing that has been on my todo list for ages but I never got around to it. I have one of these and using the Raspberry Pi with the tc358743 would allow me to add a CEC driver as well. > A couple of the subdevice API calls were not implemented or > otherwise gave odd results. Those are fixed. > > The TC358743 interface board being used didn't have the IRQ > line wired up to the SoC. "interrupts" is listed as being > optional in the DT binding, but the driver didn't actually > function if it wasn't provided. > > Dave Stevenson (3): > [media] tc358743: Add enum_mbus_code > [media] tc358743: Setup default mbus_fmt before registering > [media] tc358743: Add support for platforms without IRQ line All looks good, I'll take this for 4.12. Regards, Hans > > drivers/media/i2c/tc358743.c | 59 > +++- > 1 file changed, 58 insertions(+), 1 deletion(-) >
[PATCH 0/3] tc358743: minor driver fixes
These 3 patches for TC358743 came out of trying to use the existing driver with a new Raspberry Pi CSI-2 receiver driver. A couple of the subdevice API calls were not implemented or otherwise gave odd results. Those are fixed. The TC358743 interface board being used didn't have the IRQ line wired up to the SoC. "interrupts" is listed as being optional in the DT binding, but the driver didn't actually function if it wasn't provided. Dave Stevenson (3): [media] tc358743: Add enum_mbus_code [media] tc358743: Setup default mbus_fmt before registering [media] tc358743: Add support for platforms without IRQ line drivers/media/i2c/tc358743.c | 59 +++- 1 file changed, 58 insertions(+), 1 deletion(-) -- 2.7.4
