Re: [PATCH RFC] DT support for omap4-iss
* Michael Allwright michael.allwri...@upb.de [150811 10:16]:
On 11 August 2015 at 13:16, Tony Lindgren t...@atomide.com wrote:
* Michael Allwright michael.allwri...@upb.de [150810 08:19]:
+
+/*
+We need a better solution for this
+*/
+#include ../arch/arm/mach-omap2/omap-pm.h
Please let's not do things like this, I end up having to deal with
all these eventually :(
+static void iss_set_constraints(struct iss_device *iss, bool enable)
+{
+if (!iss)
+return;
+
+/* FIXME: Look for something more precise as a good throughtput limit
*/
+omap_pm_set_min_bus_tput(iss-dev, OCP_INITIATOR_AGENT,
+ enable ? 80 : -1);
+}
+
+static struct iss_platform_data iss_dummy_pdata = {
+.set_constraints = iss_set_constraints,
+};
If this is one time setting, you could do it based on the
compatible string using arch/arm/mach-omap2/pdata-quirks.c.
If you need to toggle it, you could populate a function pointer
in pdata-quirks.c. Those are easy to fix once there is some Linux
generic API available :)
Regards,
Tony
Hi Tony,
Thanks for the suggestion, I'll move that function into
pdata-quirks.c. Please read on, I really need some help regarding the
following error, I lost 8-9 hours on this today.
[ 141.612609] omap4iss 5200.iss: CSI2: CSI2_96M_FCLK reset timeout!
This comes from the function: int omap4iss_csi2_reset(struct
iss_csi2_device *csi2) in iss_csi2.c. I have found that bit 29 in
REGISTER1 belonging to the CSI2A registers, isn't becoming high after
doing the reset on kernel 4.1.4. However it does come high in 3.17.
This bit is a flag indicating that the reset on the CSI2_96M_FCLK is
complete.
3.17
[ 43.399658] omap4-iss 5200.iss: REGISTER1 = 0x
[ 43.405456] omap4-iss 5200.iss: REGISTER1 = 0xe002e10e
4.1.4
[ 210.331909] omap4iss 5200.iss: REGISTER1 = 0x
[ 210.338470] omap4iss 5200.iss: REGISTER1 = 0xc002e10e
[ 210.342609] omap4iss 5200.iss: CSI2: CSI2_96M_FCLK reset timeout!
Note: the transition from 0x to 0xc002e10e would seem to
indicate that the operation completed, just not successfully...
I have spent the day sampling at different points in the code,
checking the contents of all the registers belonging to the ISS and
CSI PHY to conclude that there are no differences between the two
instances of the driver running on 3.17 and 4.1.4. Using the internal
__clk_is_enabled from clk-provider.h I also checked that the muxes
responsible for providing the clocks to the module were enabled
before, during and after the reset. I have also confirmed the
identical issue also occurs on a different board.
I suspect someone has broken something in the hwmods, or PRCM data
structures. Although I have not yet been able to find any relevant
differences in the source files that I have searched through.
Any suggestions regarding where I should continue to look for this
issue are welcome. Unfortunately if I can't get some support on this
soon, I will have to abandon working on this patch.
Sorry I'm a bit behind on emails.. If this is still an issue you
may be able to git bisect it with an additional camera patches. Of
course assuming your camera patches apply and work over a larger
set of kernel versions :) If it does not work, you might be able
to create a minimal test patch for git bisect that just tests the
CSI2_96M_FCLK on boot.
Of course it might make sense to test that with recent main kernel
revisions first before git bisect.
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] DT support for omap4-iss
Hi Michael,
Thank you for the patch.
On Monday 10 August 2015 17:16:30 Michael Allwright wrote:
Hi All,
The following PRELIMINARY patch adds DT support to the OMAP4 ISS. It
also fixes some problems a have found along the way. It is tightly
modelled after the omap3-isp media platform driver. This patch is a
work in progress as I would like feedback. It contains debugging
messages that need to be removed, as well as disgusting abuses of the
C language as required (i.e. clk_core_fake and clk_fake).
I'm working in the latest stable mainline which as far as this patch
is concerned is compatible with media tree master. I have had this
omap4-iss working on my hardware in the 3.17 kernel, however I'm
currently having the following issue in 4.1.4 stable when I start to
stream:
[ 141.612609] omap4iss 5200.iss: CSI2: CSI2_96M_FCLK reset timeout!
I've addressed this issue in a separate reply, I'll skip it here.
Any feedback regarding this issue would really be appreciated. After
resolving this issue, we still need to do a proper implementation
using syscon and also to find a solution regarding where to put the
iss_set_constraints function. I have to give up for the next couple of
weeks as I need to submit a conference paper, which I'm going to use
my 3.17 implementation for. The following is an example of how the ISS
would be instantiated in the top level device tree:
iss_csi21_pins: pinmux_iss_csi21_pins {
pinctrl-single,pins =
OMAP4_IOPAD(0x0a0, PIN_INPUT | MUX_MODE0)/*
csi21_dx0.csi21_dx0 */
OMAP4_IOPAD(0x0a2, PIN_INPUT | MUX_MODE0)/*
csi21_dy0.csi21_dy0 */
OMAP4_IOPAD(0x0a4, PIN_INPUT | MUX_MODE0)/*
csi21_dx1.csi21_dx1 */
OMAP4_IOPAD(0x0a6, PIN_INPUT | MUX_MODE0)/*
csi21_dy1.csi21_dy1 */
OMAP4_IOPAD(0x0a8, PIN_INPUT | MUX_MODE0)/*
csi21_dx2.csi21_dx2 */
OMAP4_IOPAD(0x0aa, PIN_INPUT | MUX_MODE0)/*
csi21_dy2.csi21_dy2 */
;
};
iss {
status = ok;
pinctrl-names = default;
pinctrl-0 = iss_csi21_pins;
ports {
port@0 {
reg = 0;
csi2a_ep: endpoint {
remote-endpoint = ov5640_1_cam_ep;
clock-lanes = 1;
data-lanes = 2;
crc = 0;
lane-polarities = 0 0;
};
};
};
};
and for the connected camera:
ov5640_1_camera: ov5640@3c {
compatible = omnivision,ov5640;
status = ok;
reg = 0x3c;
pwdn-gpios = ov5640_1_gpio 5 GPIO_ACTIVE_HIGH;
reset-gpios = ov5640_1_gpio 6 GPIO_ACTIVE_LOW;
avdd-supply = switch_ov5640_1_avdd;
dvdd-supply = switch_ov5640_1_dvdd;
clocks = ov5640_1_camera_clk;
port {
ov5640_1_cam_ep: endpoint {
clock-lanes = 0;
data-lanes = 1;
remote-endpoint = csi2a_ep;
};
};
};
From 919995491fb34cf7e2bd8a331c47e45cad677ce6 Mon Sep 17 00:00:00 2001
From: Michael Allwright allse...@gmail.com
Date: Mon, 10 Aug 2015 16:55:57 +0200
Subject: [PATCH] omap4-iss: Add device support (WIP)
---
arch/arm/boot/dts/omap4.dtsi| 33 +++
drivers/staging/media/omap4iss/iss.c| 419 ++---
drivers/staging/media/omap4iss/iss.h| 11 +
drivers/staging/media/omap4iss/iss_csi2.c | 4 +-
drivers/staging/media/omap4iss/iss_csiphy.c | 16 +-
drivers/staging/media/omap4iss/iss_video.c | 6 +-
include/media/omap4iss.h| 18 +-
I know this comment is usually not happily received, but you're missing the DT
bindings documentation :-)
7 files changed, 393 insertions(+), 114 deletions(-)
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index f884d6a..bd37437 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -923,6 +923,39 @@
status = disabled;
};
+iss: iss@5200 {
+compatible = ti,omap4-iss;
+reg = 0x5200 0x100, /* top */
+ 0x52001000 0x170, /* csi2_a_regs1 */
+ 0x52001170 0x020, /* camerarx_core1 */
+ 0x52001400 0x170, /* csi2_b_regs1 */
+ 0x52001570 0x020, /* camerarx_core2 */
+ 0x52002000 0x200, /* bte */
+ 0x5201 0x0a0, /* isp_sys1 */
+ 0x52010400 0x400, /* isp_resizer */
+ 0x52010800 0x800, /* isp_ipipe */
+ 0x52011000 0x200, /* isp_isif */
+ 0x52011200 0x080; /* isp_ipipeif */
+reg-names = top,
+csi2_a_regs1,
+camerarx_core1,
+csi2_b_regs1,
+camerarx_core2,
+bte,
+isp_sys1,
+isp_resizer,
+
Re: [PATCH RFC] DT support for omap4-iss
Hi Michael, On Thursday 20 August 2015 01:40:26 Laurent Pinchart wrote: Hi Michael, Thank you for the patch. I forgot to mention that your patch got corrupted by your mailer (tabs replaced by spaces for instance). Could you please fix that for the next version ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] DT support for omap4-iss
Hi Michael,
On Tuesday 11 August 2015 19:13:08 Michael Allwright wrote:
On 11 August 2015 at 13:16, Tony Lindgren t...@atomide.com wrote:
* Michael Allwright michael.allwri...@upb.de [150810 08:19]:
+
+/*
+We need a better solution for this
+*/
+#include ../arch/arm/mach-omap2/omap-pm.h
Please let's not do things like this, I end up having to deal with
all these eventually :(
+static void iss_set_constraints(struct iss_device *iss, bool enable)
+{
+if (!iss)
+return;
+
+/* FIXME: Look for something more precise as a good throughtput
limit */ +omap_pm_set_min_bus_tput(iss-dev, OCP_INITIATOR_AGENT,
+ enable ? 80 : -1);
+}
+
+static struct iss_platform_data iss_dummy_pdata = {
+.set_constraints = iss_set_constraints,
+};
If this is one time setting, you could do it based on the
compatible string using arch/arm/mach-omap2/pdata-quirks.c.
If you need to toggle it, you could populate a function pointer
in pdata-quirks.c. Those are easy to fix once there is some Linux
generic API available :)
Regards,
Tony
Hi Tony,
Thanks for the suggestion, I'll move that function into
pdata-quirks.c. Please read on, I really need some help regarding the
following error, I lost 8-9 hours on this today.
[ 141.612609] omap4iss 5200.iss: CSI2: CSI2_96M_FCLK reset timeout!
This comes from the function: int omap4iss_csi2_reset(struct
iss_csi2_device *csi2) in iss_csi2.c. I have found that bit 29 in
REGISTER1 belonging to the CSI2A registers, isn't becoming high after
doing the reset on kernel 4.1.4. However it does come high in 3.17.
This bit is a flag indicating that the reset on the CSI2_96M_FCLK is
complete.
3.17
[ 43.399658] omap4-iss 5200.iss: REGISTER1 = 0x
[ 43.405456] omap4-iss 5200.iss: REGISTER1 = 0xe002e10e
4.1.4
[ 210.331909] omap4iss 5200.iss: REGISTER1 = 0x
[ 210.338470] omap4iss 5200.iss: REGISTER1 = 0xc002e10e
[ 210.342609] omap4iss 5200.iss: CSI2: CSI2_96M_FCLK reset timeout!
Note: the transition from 0x to 0xc002e10e would seem to
indicate that the operation completed, just not successfully...
I have spent the day sampling at different points in the code,
checking the contents of all the registers belonging to the ISS and
CSI PHY to conclude that there are no differences between the two
instances of the driver running on 3.17 and 4.1.4. Using the internal
__clk_is_enabled from clk-provider.h I also checked that the muxes
responsible for providing the clocks to the module were enabled
before, during and after the reset. I have also confirmed the
identical issue also occurs on a different board.
I suspect someone has broken something in the hwmods, or PRCM data
structures. Although I have not yet been able to find any relevant
differences in the source files that I have searched through.
Any suggestions regarding where I should continue to look for this
issue are welcome. Unfortunately if I can't get some support on this
soon, I will have to abandon working on this patch.
How about using git bisect to find the root cause ?
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] DT support for omap4-iss
Hi Tony,
On Tuesday 11 August 2015 04:16:04 Tony Lindgren wrote:
* Michael Allwright michael.allwri...@upb.de [150810 08:19]:
+
+/*
+We need a better solution for this
+*/
+#include ../arch/arm/mach-omap2/omap-pm.h
Please let's not do things like this, I end up having to deal with
all these eventually :(
+static void iss_set_constraints(struct iss_device *iss, bool enable)
+{
+if (!iss)
+return;
+
+/* FIXME: Look for something more precise as a good throughtput limit
*/ +omap_pm_set_min_bus_tput(iss-dev, OCP_INITIATOR_AGENT,
+ enable ? 80 : -1);
+}
+
+static struct iss_platform_data iss_dummy_pdata = {
+.set_constraints = iss_set_constraints,
+};
If this is one time setting, you could do it based on the
compatible string using arch/arm/mach-omap2/pdata-quirks.c.
If you need to toggle it, you could populate a function pointer
in pdata-quirks.c. Those are easy to fix once there is some Linux
generic API available :)
Isn't this a good candidate for the PM QoS API ? Does OMAP4 implement it ?
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] DT support for omap4-iss
Hi Everyone, I'm thinking of using systemtap to create watchpoints on all memory regions of the ISS and associated PRCM registers to generate two log files with all memory accesses at any given point of time, one for 3.17 and one for 4.1.4. Does this sound like reasonable approach, or is this over the top / inefficient in your experience? All the best, Michael Allwright PhD Student Paderborn Institute for Advanced Studies in Computer Science and Engineering University of Paderborn Office-number 02-10 Zukunftsmeile 1 33102 Paderborn Germany -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] DT support for omap4-iss
* Michael Allwright michael.allwri...@upb.de [150810 08:19]:
+
+/*
+We need a better solution for this
+*/
+#include ../arch/arm/mach-omap2/omap-pm.h
Please let's not do things like this, I end up having to deal with
all these eventually :(
+static void iss_set_constraints(struct iss_device *iss, bool enable)
+{
+if (!iss)
+return;
+
+/* FIXME: Look for something more precise as a good throughtput limit */
+omap_pm_set_min_bus_tput(iss-dev, OCP_INITIATOR_AGENT,
+ enable ? 80 : -1);
+}
+
+static struct iss_platform_data iss_dummy_pdata = {
+.set_constraints = iss_set_constraints,
+};
If this is one time setting, you could do it based on the
compatible string using arch/arm/mach-omap2/pdata-quirks.c.
If you need to toggle it, you could populate a function pointer
in pdata-quirks.c. Those are easy to fix once there is some Linux
generic API available :)
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] DT support for omap4-iss
On 11 August 2015 at 13:16, Tony Lindgren t...@atomide.com wrote:
* Michael Allwright michael.allwri...@upb.de [150810 08:19]:
+
+/*
+We need a better solution for this
+*/
+#include ../arch/arm/mach-omap2/omap-pm.h
Please let's not do things like this, I end up having to deal with
all these eventually :(
+static void iss_set_constraints(struct iss_device *iss, bool enable)
+{
+if (!iss)
+return;
+
+/* FIXME: Look for something more precise as a good throughtput limit */
+omap_pm_set_min_bus_tput(iss-dev, OCP_INITIATOR_AGENT,
+ enable ? 80 : -1);
+}
+
+static struct iss_platform_data iss_dummy_pdata = {
+.set_constraints = iss_set_constraints,
+};
If this is one time setting, you could do it based on the
compatible string using arch/arm/mach-omap2/pdata-quirks.c.
If you need to toggle it, you could populate a function pointer
in pdata-quirks.c. Those are easy to fix once there is some Linux
generic API available :)
Regards,
Tony
Hi Tony,
Thanks for the suggestion, I'll move that function into
pdata-quirks.c. Please read on, I really need some help regarding the
following error, I lost 8-9 hours on this today.
[ 141.612609] omap4iss 5200.iss: CSI2: CSI2_96M_FCLK reset timeout!
This comes from the function: int omap4iss_csi2_reset(struct
iss_csi2_device *csi2) in iss_csi2.c. I have found that bit 29 in
REGISTER1 belonging to the CSI2A registers, isn't becoming high after
doing the reset on kernel 4.1.4. However it does come high in 3.17.
This bit is a flag indicating that the reset on the CSI2_96M_FCLK is
complete.
3.17
[ 43.399658] omap4-iss 5200.iss: REGISTER1 = 0x
[ 43.405456] omap4-iss 5200.iss: REGISTER1 = 0xe002e10e
4.1.4
[ 210.331909] omap4iss 5200.iss: REGISTER1 = 0x
[ 210.338470] omap4iss 5200.iss: REGISTER1 = 0xc002e10e
[ 210.342609] omap4iss 5200.iss: CSI2: CSI2_96M_FCLK reset timeout!
Note: the transition from 0x to 0xc002e10e would seem to
indicate that the operation completed, just not successfully...
I have spent the day sampling at different points in the code,
checking the contents of all the registers belonging to the ISS and
CSI PHY to conclude that there are no differences between the two
instances of the driver running on 3.17 and 4.1.4. Using the internal
__clk_is_enabled from clk-provider.h I also checked that the muxes
responsible for providing the clocks to the module were enabled
before, during and after the reset. I have also confirmed the
identical issue also occurs on a different board.
I suspect someone has broken something in the hwmods, or PRCM data
structures. Although I have not yet been able to find any relevant
differences in the source files that I have searched through.
Any suggestions regarding where I should continue to look for this
issue are welcome. Unfortunately if I can't get some support on this
soon, I will have to abandon working on this patch.
Michael Allwright
PhD Student
Paderborn Institute for Advanced Studies in Computer Science and Engineering
University of Paderborn
Office-number 02-10
Zukunftsmeile 1
33102 Paderborn
Germany
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
