* [PATCH v2 0/2] OMAP 3 CSI-2 configuration @ 2012-09-26 21:50 Sakari Ailus 2012-09-26 21:50 ` [PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration Sakari Ailus 2012-09-26 21:50 ` [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data Sakari Ailus 0 siblings, 2 replies; 15+ messages in thread From: Sakari Ailus @ 2012-09-26 21:50 UTC (permalink / raw) To: paul, laurent.pinchart, linux-media, linux-omap Hi Paul and Laurent, This is an update to an old patchset for CSI-2 configuration for OMAP 3430 and 3630, and the corresponding patch to the ISP driver. Both have been tested on the 3630 only so far. Additional patches for the N9(50) camera support that mostly aren't yet upstreamable, are available in rm696-016/002-devel branch here: <URL:https://git.gitorious.org/omap3camera/mainline.git> Kind regards, -- Sakari Ailus e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration 2012-09-26 21:50 [PATCH v2 0/2] OMAP 3 CSI-2 configuration Sakari Ailus @ 2012-09-26 21:50 ` Sakari Ailus 2012-09-27 9:20 ` Laurent Pinchart 2012-10-09 20:50 ` Kevin Hilman 2012-09-26 21:50 ` [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data Sakari Ailus 1 sibling, 2 replies; 15+ messages in thread From: Sakari Ailus @ 2012-09-26 21:50 UTC (permalink / raw) To: paul, laurent.pinchart, linux-media, linux-omap The OMAP 3630 has configuration how the ISP CSI-2 PHY pins are connected to the actual CSI-2 receivers outside the ISP itself. Allow changing this configuration from the ISP driver. Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> --- arch/arm/mach-omap2/control.c | 86 ++++++++++++++++++++++++++++ arch/arm/mach-omap2/control.h | 15 +++++ arch/arm/mach-omap2/include/mach/control.h | 13 ++++ 3 files changed, 114 insertions(+), 0 deletions(-) create mode 100644 arch/arm/mach-omap2/include/mach/control.h diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c index 3223b81..11bb900 100644 --- a/arch/arm/mach-omap2/control.c +++ b/arch/arm/mach-omap2/control.c @@ -12,9 +12,12 @@ */ #undef DEBUG +#include <linux/export.h> #include <linux/kernel.h> #include <linux/io.h> +#include <mach/control.h> + #include <plat/hardware.h> #include <plat/sdrc.h> @@ -607,4 +610,87 @@ int omap3_ctrl_save_padconf(void) return 0; } +static int omap3630_ctrl_csi2_phy_cfg(u32 phy, u32 flags) +{ + u32 cam_phy_ctrl = + omap_ctrl_readl(OMAP3630_CONTROL_CAMERA_PHY_CTRL); + u32 shift, mode; + + switch (phy) { + case OMAP3_CTRL_CSI2_PHY1_CCP2B: + cam_phy_ctrl &= ~OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2; + shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT; + break; + case OMAP3_CTRL_CSI2_PHY1_CSI2C: + shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT; + mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY; + break; + case OMAP3_CTRL_CSI2_PHY2_CCP2B: + cam_phy_ctrl |= OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2; + shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT; + break; + case OMAP3_CTRL_CSI2_PHY2_CSI2A: + shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT; + mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY; + break; + default: + pr_warn("bad phy %d\n", phy); + return -EINVAL; + } + + /* Select data/clock or data/strobe mode for CCP2 */ + switch (phy) { + case OMAP3_CTRL_CSI2_PHY1_CCP2B: + case OMAP3_CTRL_CSI2_PHY2_CCP2B: + if (flags & OMAP3_CTRL_CSI2_CFG_CCP2_DATA_STROBE) + mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_STROBE; + else + mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_CLOCK; + break; + } + + cam_phy_ctrl &= ~(OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_MASK << shift); + cam_phy_ctrl |= mode << shift; + + omap_ctrl_writel(cam_phy_ctrl, + OMAP3630_CONTROL_CAMERA_PHY_CTRL); + + return 0; +} + +static int omap3430_ctrl_csi2_phy_cfg(u32 phy, bool on, u32 flags) +{ + uint32_t csirxfe = OMAP343X_CONTROL_CSIRXFE_PWRDNZ + | OMAP343X_CONTROL_CSIRXFE_RESET; + + /* Nothing to configure here. */ + if (phy == OMAP3_CTRL_CSI2_PHY2_CSI2A) + return 0; + + if (phy != OMAP3_CTRL_CSI2_PHY1_CCP2B) + return -EINVAL; + + if (!on) { + omap_ctrl_writel(0, OMAP343X_CONTROL_CSIRXFE); + return 0; + } + + if (flags & OMAP3_CTRL_CSI2_CFG_CCP2_DATA_STROBE) + csirxfe |= OMAP343X_CONTROL_CSIRXFE_SELFORM; + + omap_ctrl_writel(csirxfe, OMAP343X_CONTROL_CSIRXFE); + + return 0; +} + +int omap3_ctrl_csi2_phy_cfg(u32 phy, bool on, u32 flags) +{ + if (cpu_is_omap3630() && on) + return omap3630_ctrl_csi2_phy_cfg(phy, flags); + if (cpu_is_omap3430()) + return omap3430_ctrl_csi2_phy_cfg(phy, on, flags); + return 0; +} +EXPORT_SYMBOL_GPL(omap3_ctrl_csi2_phy_cfg); + #endif /* CONFIG_ARCH_OMAP3 && CONFIG_PM */ diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h index b8cdc85..7b2ee5d 100644 --- a/arch/arm/mach-omap2/control.h +++ b/arch/arm/mach-omap2/control.h @@ -132,6 +132,11 @@ #define OMAP343X_CONTROL_MEM_DFTRW1 (OMAP2_CONTROL_GENERAL + 0x000c) #define OMAP343X_CONTROL_DEVCONF1 (OMAP2_CONTROL_GENERAL + 0x0068) #define OMAP343X_CONTROL_CSIRXFE (OMAP2_CONTROL_GENERAL + 0x006c) +#define OMAP343X_CONTROL_CSIRXFE_CSIB_INV (1 << 7) +#define OMAP343X_CONTROL_CSIRXFE_RESENABLE (1 << 8) +#define OMAP343X_CONTROL_CSIRXFE_SELFORM (1 << 10) +#define OMAP343X_CONTROL_CSIRXFE_PWRDNZ (1 << 12) +#define OMAP343X_CONTROL_CSIRXFE_RESET (1 << 13) #define OMAP343X_CONTROL_SEC_STATUS (OMAP2_CONTROL_GENERAL + 0x0070) #define OMAP343X_CONTROL_SEC_ERR_STATUS (OMAP2_CONTROL_GENERAL + 0x0074) #define OMAP343X_CONTROL_SEC_ERR_STATUS_DEBUG (OMAP2_CONTROL_GENERAL + 0x0078) @@ -189,6 +194,16 @@ #define OMAP3630_CONTROL_FUSE_OPP50_VDD2 (OMAP2_CONTROL_GENERAL + 0x0128) #define OMAP3630_CONTROL_FUSE_OPP100_VDD2 (OMAP2_CONTROL_GENERAL + 0x012C) #define OMAP3630_CONTROL_CAMERA_PHY_CTRL (OMAP2_CONTROL_GENERAL + 0x02f0) +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT 2 +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT 0 +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY 0x0 +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_STROBE 0x1 +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_CLOCK 0x2 +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_GPI 0x3 +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_MASK 0x3 +/* CCP2B: set to receive data from PHY2 instead of PHY1 */ +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2 (1 << 4) + /* OMAP44xx control efuse offsets */ #define OMAP44XX_CONTROL_FUSE_IVA_OPP50 0x22C diff --git a/arch/arm/mach-omap2/include/mach/control.h b/arch/arm/mach-omap2/include/mach/control.h new file mode 100644 index 0000000..afd0bed --- /dev/null +++ b/arch/arm/mach-omap2/include/mach/control.h @@ -0,0 +1,13 @@ +#ifndef __MACH_CONTROL_H__ +#define __MACH_CONTROL_H__ + +#define OMAP3_CTRL_CSI2_PHY2_CSI2A 0 +#define OMAP3_CTRL_CSI2_PHY2_CCP2B 1 +#define OMAP3_CTRL_CSI2_PHY1_CCP2B 2 +#define OMAP3_CTRL_CSI2_PHY1_CSI2C 3 + +#define OMAP3_CTRL_CSI2_CFG_CCP2_DATA_STROBE (1 << 0) + +int omap3_ctrl_csi2_phy_cfg(u32 phy, bool on, u32 flags); + +#endif /* __MACH_CONTROL_H__ */ -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration 2012-09-26 21:50 ` [PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration Sakari Ailus @ 2012-09-27 9:20 ` Laurent Pinchart 2012-09-27 20:08 ` Sakari Ailus 2012-10-09 20:50 ` Kevin Hilman 1 sibling, 1 reply; 15+ messages in thread From: Laurent Pinchart @ 2012-09-27 9:20 UTC (permalink / raw) To: Sakari Ailus; +Cc: paul, linux-media, linux-omap Hi Sakari, Thanks for the patch. On Thursday 27 September 2012 00:50:35 Sakari Ailus wrote: > The OMAP 3630 has configuration how the ISP CSI-2 PHY pins are connected to > the actual CSI-2 receivers outside the ISP itself. Allow changing this > configuration from the ISP driver. > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> Just one small comment below, otherwise Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > arch/arm/mach-omap2/control.c | 86 +++++++++++++++++++++++++ > arch/arm/mach-omap2/control.h | 15 +++++ > arch/arm/mach-omap2/include/mach/control.h | 13 ++++ > 3 files changed, 114 insertions(+), 0 deletions(-) > create mode 100644 arch/arm/mach-omap2/include/mach/control.h > > diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c > index 3223b81..11bb900 100644 > --- a/arch/arm/mach-omap2/control.c > +++ b/arch/arm/mach-omap2/control.c > @@ -12,9 +12,12 @@ > */ > #undef DEBUG > > +#include <linux/export.h> > #include <linux/kernel.h> > #include <linux/io.h> > > +#include <mach/control.h> > + > #include <plat/hardware.h> > #include <plat/sdrc.h> > > @@ -607,4 +610,87 @@ int omap3_ctrl_save_padconf(void) > return 0; > } > > +static int omap3630_ctrl_csi2_phy_cfg(u32 phy, u32 flags) > +{ > + u32 cam_phy_ctrl = > + omap_ctrl_readl(OMAP3630_CONTROL_CAMERA_PHY_CTRL); > + u32 shift, mode; > + > + switch (phy) { > + case OMAP3_CTRL_CSI2_PHY1_CCP2B: > + cam_phy_ctrl &= ~OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2; > + shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT; > + break; > + case OMAP3_CTRL_CSI2_PHY1_CSI2C: > + shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT; > + mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY; > + break; > + case OMAP3_CTRL_CSI2_PHY2_CCP2B: > + cam_phy_ctrl |= OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2; > + shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT; > + break; > + case OMAP3_CTRL_CSI2_PHY2_CSI2A: > + shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT; > + mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY; > + break; > + default: > + pr_warn("bad phy %d\n", phy); > + return -EINVAL; > + } > + > + /* Select data/clock or data/strobe mode for CCP2 */ > + switch (phy) { > + case OMAP3_CTRL_CSI2_PHY1_CCP2B: > + case OMAP3_CTRL_CSI2_PHY2_CCP2B: > + if (flags & OMAP3_CTRL_CSI2_CFG_CCP2_DATA_STROBE) > + mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_STROBE; > + else > + mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_CLOCK; > + break; > + } > + > + cam_phy_ctrl &= ~(OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_MASK << shift); > + cam_phy_ctrl |= mode << shift; > + > + omap_ctrl_writel(cam_phy_ctrl, > + OMAP3630_CONTROL_CAMERA_PHY_CTRL); This can fit on one line. > + > + return 0; > +} > + > +static int omap3430_ctrl_csi2_phy_cfg(u32 phy, bool on, u32 flags) > +{ > + uint32_t csirxfe = OMAP343X_CONTROL_CSIRXFE_PWRDNZ > + | OMAP343X_CONTROL_CSIRXFE_RESET; > + > + /* Nothing to configure here. */ > + if (phy == OMAP3_CTRL_CSI2_PHY2_CSI2A) > + return 0; > + > + if (phy != OMAP3_CTRL_CSI2_PHY1_CCP2B) > + return -EINVAL; > + > + if (!on) { > + omap_ctrl_writel(0, OMAP343X_CONTROL_CSIRXFE); > + return 0; > + } > + > + if (flags & OMAP3_CTRL_CSI2_CFG_CCP2_DATA_STROBE) > + csirxfe |= OMAP343X_CONTROL_CSIRXFE_SELFORM; > + > + omap_ctrl_writel(csirxfe, OMAP343X_CONTROL_CSIRXFE); > + > + return 0; > +} > + > +int omap3_ctrl_csi2_phy_cfg(u32 phy, bool on, u32 flags) > +{ > + if (cpu_is_omap3630() && on) > + return omap3630_ctrl_csi2_phy_cfg(phy, flags); > + if (cpu_is_omap3430()) > + return omap3430_ctrl_csi2_phy_cfg(phy, on, flags); > + return 0; > +} > +EXPORT_SYMBOL_GPL(omap3_ctrl_csi2_phy_cfg); > + > #endif /* CONFIG_ARCH_OMAP3 && CONFIG_PM */ > diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h > index b8cdc85..7b2ee5d 100644 > --- a/arch/arm/mach-omap2/control.h > +++ b/arch/arm/mach-omap2/control.h > @@ -132,6 +132,11 @@ > #define OMAP343X_CONTROL_MEM_DFTRW1 (OMAP2_CONTROL_GENERAL + 0x000c) > #define OMAP343X_CONTROL_DEVCONF1 (OMAP2_CONTROL_GENERAL + 0x0068) > #define OMAP343X_CONTROL_CSIRXFE (OMAP2_CONTROL_GENERAL + 0x006c) > +#define OMAP343X_CONTROL_CSIRXFE_CSIB_INV (1 << 7) > +#define OMAP343X_CONTROL_CSIRXFE_RESENABLE (1 << 8) > +#define OMAP343X_CONTROL_CSIRXFE_SELFORM (1 << 10) > +#define OMAP343X_CONTROL_CSIRXFE_PWRDNZ (1 << 12) > +#define OMAP343X_CONTROL_CSIRXFE_RESET (1 << 13) > #define OMAP343X_CONTROL_SEC_STATUS (OMAP2_CONTROL_GENERAL + 0x0070) > #define OMAP343X_CONTROL_SEC_ERR_STATUS (OMAP2_CONTROL_GENERAL + 0x0074) > #define OMAP343X_CONTROL_SEC_ERR_STATUS_DEBUG (OMAP2_CONTROL_GENERAL + > 0x0078) -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration 2012-09-27 9:20 ` Laurent Pinchart @ 2012-09-27 20:08 ` Sakari Ailus 0 siblings, 0 replies; 15+ messages in thread From: Sakari Ailus @ 2012-09-27 20:08 UTC (permalink / raw) To: Laurent Pinchart; +Cc: paul, linux-media, linux-omap Hi Laurent, Thanks for the review! Laurent Pinchart wrote: > Hi Sakari, > > Thanks for the patch. > > On Thursday 27 September 2012 00:50:35 Sakari Ailus wrote: >> The OMAP 3630 has configuration how the ISP CSI-2 PHY pins are connected to >> the actual CSI-2 receivers outside the ISP itself. Allow changing this >> configuration from the ISP driver. >> >> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> > > Just one small comment below, otherwise > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> --- >> arch/arm/mach-omap2/control.c | 86 +++++++++++++++++++++++++ >> arch/arm/mach-omap2/control.h | 15 +++++ >> arch/arm/mach-omap2/include/mach/control.h | 13 ++++ >> 3 files changed, 114 insertions(+), 0 deletions(-) >> create mode 100644 arch/arm/mach-omap2/include/mach/control.h >> >> diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c >> index 3223b81..11bb900 100644 >> --- a/arch/arm/mach-omap2/control.c >> +++ b/arch/arm/mach-omap2/control.c ... >> + cam_phy_ctrl |= mode << shift; >> + >> + omap_ctrl_writel(cam_phy_ctrl, >> + OMAP3630_CONTROL_CAMERA_PHY_CTRL); > > This can fit on one line. I'll fix that for the next version. I noticed there were a few too long lines, too; I've broken them up where it makes sense, and removed a useless break statement. Cheers, -- Sakari Ailus sakari.ailus@iki.fi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration 2012-09-26 21:50 ` [PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration Sakari Ailus 2012-09-27 9:20 ` Laurent Pinchart @ 2012-10-09 20:50 ` Kevin Hilman 2012-10-09 22:33 ` Sakari Ailus 1 sibling, 1 reply; 15+ messages in thread From: Kevin Hilman @ 2012-10-09 20:50 UTC (permalink / raw) To: Sakari Ailus; +Cc: paul, laurent.pinchart, linux-media, linux-omap Hi Sakari, Sakari Ailus <sakari.ailus@iki.fi> writes: > The OMAP 3630 has configuration how the ISP CSI-2 PHY pins are connected to > the actual CSI-2 receivers outside the ISP itself. Allow changing this > configuration from the ISP driver. > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> These control module registers (CSIRXFE, CAMERA_PHY_CTRL) are in the CORE powerdomain, so they will be lost during off-mode transitions. So, I suspect you'll also want to add them to the save/restore functions in control.c in order for this to work across off-mode transitions. Kevin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration 2012-10-09 20:50 ` Kevin Hilman @ 2012-10-09 22:33 ` Sakari Ailus 2012-10-10 0:22 ` Kevin Hilman 0 siblings, 1 reply; 15+ messages in thread From: Sakari Ailus @ 2012-10-09 22:33 UTC (permalink / raw) To: Kevin Hilman; +Cc: paul, laurent.pinchart, linux-media, linux-omap Hi Kevin, Thanks for the comments! On Tue, Oct 09, 2012 at 01:50:04PM -0700, Kevin Hilman wrote: > Hi Sakari, > > Sakari Ailus <sakari.ailus@iki.fi> writes: > > > The OMAP 3630 has configuration how the ISP CSI-2 PHY pins are connected to > > the actual CSI-2 receivers outside the ISP itself. Allow changing this > > configuration from the ISP driver. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> > > These control module registers (CSIRXFE, CAMERA_PHY_CTRL) are in the > CORE powerdomain, so they will be lost during off-mode transitions. So, > I suspect you'll also want to add them to the save/restore functions in > control.c in order for this to work across off-mode transitions. I've got another patch that implements this in the ISP driver instead. <URL:http://www.spinics.net/lists/linux-media/msg54781.html> The ISP also can't wake up the MPU from the off mode, so I don't think losing the register contents is necessarily an issue. The registers will be written to a new value whenever streaming is started. Perhaps adding a note about that would be worthwhile. Regards, -- Sakari Ailus e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration 2012-10-09 22:33 ` Sakari Ailus @ 2012-10-10 0:22 ` Kevin Hilman 0 siblings, 0 replies; 15+ messages in thread From: Kevin Hilman @ 2012-10-10 0:22 UTC (permalink / raw) To: Sakari Ailus; +Cc: paul, laurent.pinchart, linux-media, linux-omap Sakari Ailus <sakari.ailus@iki.fi> writes: > Hi Kevin, > > Thanks for the comments! > > On Tue, Oct 09, 2012 at 01:50:04PM -0700, Kevin Hilman wrote: >> Hi Sakari, >> >> Sakari Ailus <sakari.ailus@iki.fi> writes: >> >> > The OMAP 3630 has configuration how the ISP CSI-2 PHY pins are connected to >> > the actual CSI-2 receivers outside the ISP itself. Allow changing this >> > configuration from the ISP driver. >> > >> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> >> >> These control module registers (CSIRXFE, CAMERA_PHY_CTRL) are in the >> CORE powerdomain, so they will be lost during off-mode transitions. So, >> I suspect you'll also want to add them to the save/restore functions in >> control.c in order for this to work across off-mode transitions. > > I've got another patch that implements this in the ISP driver instead. > > <URL:http://www.spinics.net/lists/linux-media/msg54781.html> Oops, sorry. I should've looked at v3. > The ISP also can't wake up the MPU from the off mode, so I don't think > losing the register contents is necessarily an issue. The registers will be > written to a new value whenever streaming is started. Perhaps adding a note > about that would be worthwhile. Yes, I suggest mentioning that these register are not saved/restored and why would be helpful. Kevin ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data 2012-09-26 21:50 [PATCH v2 0/2] OMAP 3 CSI-2 configuration Sakari Ailus 2012-09-26 21:50 ` [PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration Sakari Ailus @ 2012-09-26 21:50 ` Sakari Ailus 2012-09-26 22:00 ` Tony Lindgren 2012-09-27 9:51 ` [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data Laurent Pinchart 1 sibling, 2 replies; 15+ messages in thread From: Sakari Ailus @ 2012-09-26 21:50 UTC (permalink / raw) To: paul, laurent.pinchart, linux-media, linux-omap Configure CSI-2 phy based on platform data in the ISP driver. For that, the new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same was configured from the board code. This patch is dependent on "omap3: Provide means for changing CSI2 PHY configuration". Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/platform/omap3isp/isp.h | 3 - drivers/media/platform/omap3isp/ispcsiphy.c | 161 +++++++++++++++------------ drivers/media/platform/omap3isp/ispcsiphy.h | 10 -- 3 files changed, 90 insertions(+), 84 deletions(-) diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h index 8be7487..a2f992c 100644 --- a/drivers/media/platform/omap3isp/isp.h +++ b/drivers/media/platform/omap3isp/isp.h @@ -127,9 +127,6 @@ struct isp_reg { struct isp_platform_callback { u32 (*set_xclk)(struct isp_device *isp, u32 xclk, u8 xclksel); - int (*csiphy_config)(struct isp_csiphy *phy, - struct isp_csiphy_dphy_cfg *dphy, - struct isp_csiphy_lanes_cfg *lanes); }; /* diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c index 348f67e..1d16e66 100644 --- a/drivers/media/platform/omap3isp/ispcsiphy.c +++ b/drivers/media/platform/omap3isp/ispcsiphy.c @@ -28,41 +28,13 @@ #include <linux/device.h> #include <linux/regulator/consumer.h> +#include <mach/control.h> + #include "isp.h" #include "ispreg.h" #include "ispcsiphy.h" /* - * csiphy_lanes_config - Configuration of CSIPHY lanes. - * - * Updates HW configuration. - * Called with phy->mutex taken. - */ -static void csiphy_lanes_config(struct isp_csiphy *phy) -{ - unsigned int i; - u32 reg; - - reg = isp_reg_readl(phy->isp, phy->cfg_regs, ISPCSI2_PHY_CFG); - - for (i = 0; i < phy->num_data_lanes; i++) { - reg &= ~(ISPCSI2_PHY_CFG_DATA_POL_MASK(i + 1) | - ISPCSI2_PHY_CFG_DATA_POSITION_MASK(i + 1)); - reg |= (phy->lanes.data[i].pol << - ISPCSI2_PHY_CFG_DATA_POL_SHIFT(i + 1)); - reg |= (phy->lanes.data[i].pos << - ISPCSI2_PHY_CFG_DATA_POSITION_SHIFT(i + 1)); - } - - reg &= ~(ISPCSI2_PHY_CFG_CLOCK_POL_MASK | - ISPCSI2_PHY_CFG_CLOCK_POSITION_MASK); - reg |= phy->lanes.clk.pol << ISPCSI2_PHY_CFG_CLOCK_POL_SHIFT; - reg |= phy->lanes.clk.pos << ISPCSI2_PHY_CFG_CLOCK_POSITION_SHIFT; - - isp_reg_writel(phy->isp, reg, phy->cfg_regs, ISPCSI2_PHY_CFG); -} - -/* * csiphy_power_autoswitch_enable * @enable: Sets or clears the autoswitch function enable flag. */ @@ -107,46 +79,32 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32 power) } /* - * csiphy_dphy_config - Configure CSI2 D-PHY parameters. - * - * Called with phy->mutex taken. + * TCLK values are OK at their reset values */ -static void csiphy_dphy_config(struct isp_csiphy *phy) -{ - u32 reg; - - /* Set up ISPCSIPHY_REG0 */ - reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG0); - - reg &= ~(ISPCSIPHY_REG0_THS_TERM_MASK | - ISPCSIPHY_REG0_THS_SETTLE_MASK); - reg |= phy->dphy.ths_term << ISPCSIPHY_REG0_THS_TERM_SHIFT; - reg |= phy->dphy.ths_settle << ISPCSIPHY_REG0_THS_SETTLE_SHIFT; - - isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG0); - - /* Set up ISPCSIPHY_REG1 */ - reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG1); - - reg &= ~(ISPCSIPHY_REG1_TCLK_TERM_MASK | - ISPCSIPHY_REG1_TCLK_MISS_MASK | - ISPCSIPHY_REG1_TCLK_SETTLE_MASK); - reg |= phy->dphy.tclk_term << ISPCSIPHY_REG1_TCLK_TERM_SHIFT; - reg |= phy->dphy.tclk_miss << ISPCSIPHY_REG1_TCLK_MISS_SHIFT; - reg |= phy->dphy.tclk_settle << ISPCSIPHY_REG1_TCLK_SETTLE_SHIFT; - - isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG1); -} +#define TCLK_TERM 0 +#define TCLK_MISS 1 +#define TCLK_SETTLE 14 -static int csiphy_config(struct isp_csiphy *phy, - struct isp_csiphy_dphy_cfg *dphy, - struct isp_csiphy_lanes_cfg *lanes) +static int omap3isp_csiphy_config(struct isp_csiphy *phy) { + struct isp_csi2_device *csi2 = phy->csi2; + struct isp_pipeline *pipe = to_isp_pipeline(&csi2->subdev.entity); + struct isp_v4l2_subdevs_group *subdevs = pipe->external->host_priv; + struct isp_csiphy_lanes_cfg *lanes; + int csi2_ddrclk_khz; unsigned int used_lanes = 0; unsigned int i; + unsigned int phy_num; + u32 reg; + + if (subdevs->interface == ISP_INTERFACE_CCP2B_PHY1 + || subdevs->interface == ISP_INTERFACE_CCP2B_PHY2) + lanes = &subdevs->bus.ccp2.lanecfg; + else + lanes = &subdevs->bus.csi2.lanecfg; /* Clock and data lanes verification */ - for (i = 0; i < phy->num_data_lanes; i++) { + for (i = 0; i < csi2->phy->num_data_lanes; i++) { if (lanes->data[i].pol > 1 || lanes->data[i].pos > 3) return -EINVAL; @@ -162,10 +120,72 @@ static int csiphy_config(struct isp_csiphy *phy, if (lanes->clk.pos == 0 || used_lanes & (1 << lanes->clk.pos)) return -EINVAL; - mutex_lock(&phy->mutex); - phy->dphy = *dphy; - phy->lanes = *lanes; - mutex_unlock(&phy->mutex); + switch (subdevs->interface) { + case ISP_INTERFACE_CSI2A_PHY2: + phy_num = OMAP3_CTRL_CSI2_PHY2_CSI2A; + break; + case ISP_INTERFACE_CSI2C_PHY1: + phy_num = OMAP3_CTRL_CSI2_PHY1_CSI2C; + break; + case ISP_INTERFACE_CCP2B_PHY1: + phy_num = OMAP3_CTRL_CSI2_PHY1_CCP2B; + break; + case ISP_INTERFACE_CCP2B_PHY2: + phy_num = OMAP3_CTRL_CSI2_PHY2_CCP2B; + break; + default: + return -EINVAL; + } + + omap3_ctrl_csi2_phy_cfg(phy_num, true, 0); + + /* DPHY timing configuration */ + /* CSI-2 is DDR and we only count used lanes. */ + csi2_ddrclk_khz = pipe->external_rate / 1000 + / (2 * hweight32(used_lanes)) * pipe->external_width; + + reg = isp_reg_readl(csi2->isp, csi2->phy->phy_regs, ISPCSIPHY_REG0); + + reg &= ~(ISPCSIPHY_REG0_THS_TERM_MASK | + ISPCSIPHY_REG0_THS_SETTLE_MASK); + /* THS_TERM: Programmed value = ceil(12.5 ns/DDRClk period) - 1. */ + reg |= (DIV_ROUND_UP(25 * csi2_ddrclk_khz, 2000000) - 1) + << ISPCSIPHY_REG0_THS_TERM_SHIFT; + /* THS_SETTLE: Programmed value = ceil(90 ns/DDRClk period) + 3. */ + reg |= (DIV_ROUND_UP(90 * csi2_ddrclk_khz, 1000000) + 3) + << ISPCSIPHY_REG0_THS_SETTLE_SHIFT; + + isp_reg_writel(csi2->isp, reg, csi2->phy->phy_regs, ISPCSIPHY_REG0); + + reg = isp_reg_readl(csi2->isp, csi2->phy->phy_regs, ISPCSIPHY_REG1); + + reg &= ~(ISPCSIPHY_REG1_TCLK_TERM_MASK | + ISPCSIPHY_REG1_TCLK_MISS_MASK | + ISPCSIPHY_REG1_TCLK_SETTLE_MASK); + reg |= TCLK_TERM << ISPCSIPHY_REG1_TCLK_TERM_SHIFT; + reg |= TCLK_MISS << ISPCSIPHY_REG1_TCLK_MISS_SHIFT; + reg |= TCLK_SETTLE << ISPCSIPHY_REG1_TCLK_SETTLE_SHIFT; + + isp_reg_writel(csi2->isp, reg, csi2->phy->phy_regs, ISPCSIPHY_REG1); + + /* DPHY lane configuration */ + reg = isp_reg_readl(csi2->isp, csi2->phy->cfg_regs, ISPCSI2_PHY_CFG); + + for (i = 0; i < csi2->phy->num_data_lanes; i++) { + reg &= ~(ISPCSI2_PHY_CFG_DATA_POL_MASK(i + 1) | + ISPCSI2_PHY_CFG_DATA_POSITION_MASK(i + 1)); + reg |= (lanes->data[i].pol << + ISPCSI2_PHY_CFG_DATA_POL_SHIFT(i + 1)); + reg |= (lanes->data[i].pos << + ISPCSI2_PHY_CFG_DATA_POSITION_SHIFT(i + 1)); + } + + reg &= ~(ISPCSI2_PHY_CFG_CLOCK_POL_MASK | + ISPCSI2_PHY_CFG_CLOCK_POSITION_MASK); + reg |= lanes->clk.pol << ISPCSI2_PHY_CFG_CLOCK_POL_SHIFT; + reg |= lanes->clk.pos << ISPCSI2_PHY_CFG_CLOCK_POSITION_SHIFT; + + isp_reg_writel(csi2->isp, reg, csi2->phy->cfg_regs, ISPCSI2_PHY_CFG); return 0; } @@ -190,8 +210,9 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy) if (rval < 0) goto done; - csiphy_dphy_config(phy); - csiphy_lanes_config(phy); + rval = omap3isp_csiphy_config(phy); + if (rval < 0) + goto done; rval = csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_ON); if (rval) { @@ -227,8 +248,6 @@ int omap3isp_csiphy_init(struct isp_device *isp) struct isp_csiphy *phy1 = &isp->isp_csiphy1; struct isp_csiphy *phy2 = &isp->isp_csiphy2; - isp->platform_cb.csiphy_config = csiphy_config; - phy2->isp = isp; phy2->csi2 = &isp->isp_csi2a; phy2->num_data_lanes = ISP_CSIPHY2_NUM_DATA_LANES; diff --git a/drivers/media/platform/omap3isp/ispcsiphy.h b/drivers/media/platform/omap3isp/ispcsiphy.h index e93a661..14551fd 100644 --- a/drivers/media/platform/omap3isp/ispcsiphy.h +++ b/drivers/media/platform/omap3isp/ispcsiphy.h @@ -32,14 +32,6 @@ struct isp_csi2_device; struct regulator; -struct isp_csiphy_dphy_cfg { - u8 ths_term; - u8 ths_settle; - u8 tclk_term; - unsigned tclk_miss:1; - u8 tclk_settle; -}; - struct isp_csiphy { struct isp_device *isp; struct mutex mutex; /* serialize csiphy configuration */ @@ -52,8 +44,6 @@ struct isp_csiphy { unsigned int phy_regs; u8 num_data_lanes; /* number of CSI2 Data Lanes supported */ - struct isp_csiphy_lanes_cfg lanes; - struct isp_csiphy_dphy_cfg dphy; }; int omap3isp_csiphy_acquire(struct isp_csiphy *phy); -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data 2012-09-26 21:50 ` [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data Sakari Ailus @ 2012-09-26 22:00 ` Tony Lindgren 2012-09-27 9:52 ` Laurent Pinchart 2012-09-27 14:38 ` [PATCH] omap3isp: Replace cpu_is_omap3630() with ISP revision check Laurent Pinchart 2012-09-27 9:51 ` [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data Laurent Pinchart 1 sibling, 2 replies; 15+ messages in thread From: Tony Lindgren @ 2012-09-26 22:00 UTC (permalink / raw) To: Sakari Ailus; +Cc: paul, laurent.pinchart, linux-media, linux-omap Moi Sakari * Sakari Ailus <sakari.ailus@iki.fi> [120926 14:51]: > Configure CSI-2 phy based on platform data in the ISP driver. For that, the > new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same > was configured from the board code. > > This patch is dependent on "omap3: Provide means for changing CSI2 PHY > configuration". Can you please do one more patch to get rid of the last remaining cpu_is_omapxxxx check in drivers/media/platform/omap3isp/isp.c? That data should come from platform_data (or device tree) as we going to make cpu_is_omap privat to mach-omap2. Regards, Tony ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data 2012-09-26 22:00 ` Tony Lindgren @ 2012-09-27 9:52 ` Laurent Pinchart 2012-09-28 0:00 ` Tony Lindgren 2012-09-27 14:38 ` [PATCH] omap3isp: Replace cpu_is_omap3630() with ISP revision check Laurent Pinchart 1 sibling, 1 reply; 15+ messages in thread From: Laurent Pinchart @ 2012-09-27 9:52 UTC (permalink / raw) To: Tony Lindgren; +Cc: Sakari Ailus, paul, linux-media, linux-omap Hi Tony, On Wednesday 26 September 2012 15:00:19 Tony Lindgren wrote: > Moi Sakari > > * Sakari Ailus <sakari.ailus@iki.fi> [120926 14:51]: > > Configure CSI-2 phy based on platform data in the ISP driver. For that, > > the > > new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same > > was configured from the board code. > > > > This patch is dependent on "omap3: Provide means for changing CSI2 PHY > > configuration". > > Can you please do one more patch to get rid of the last remaining > cpu_is_omapxxxx check in drivers/media/platform/omap3isp/isp.c? I'm working on that, I'll submit a patch. > That data should come from platform_data (or device tree) as > we going to make cpu_is_omap privat to mach-omap2. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data 2012-09-27 9:52 ` Laurent Pinchart @ 2012-09-28 0:00 ` Tony Lindgren 0 siblings, 0 replies; 15+ messages in thread From: Tony Lindgren @ 2012-09-28 0:00 UTC (permalink / raw) To: Laurent Pinchart; +Cc: Sakari Ailus, paul, linux-media, linux-omap * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [120927 02:52]: > Hi Tony, > > On Wednesday 26 September 2012 15:00:19 Tony Lindgren wrote: > > Moi Sakari > > > > * Sakari Ailus <sakari.ailus@iki.fi> [120926 14:51]: > > > Configure CSI-2 phy based on platform data in the ISP driver. For that, > > > the > > > new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same > > > was configured from the board code. > > > > > > This patch is dependent on "omap3: Provide means for changing CSI2 PHY > > > configuration". > > > > Can you please do one more patch to get rid of the last remaining > > cpu_is_omapxxxx check in drivers/media/platform/omap3isp/isp.c? > > I'm working on that, I'll submit a patch. Thanks acked it. Regards, Tony ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] omap3isp: Replace cpu_is_omap3630() with ISP revision check 2012-09-26 22:00 ` Tony Lindgren 2012-09-27 9:52 ` Laurent Pinchart @ 2012-09-27 14:38 ` Laurent Pinchart 2012-09-27 23:59 ` Tony Lindgren 1 sibling, 1 reply; 15+ messages in thread From: Laurent Pinchart @ 2012-09-27 14:38 UTC (permalink / raw) To: linux-media; +Cc: linux-omap, Sakari Ailus, Tony Lindgren, paul Drivers must not rely on cpu_is_omap* macros (they will soon become private). Use the ISP revision instead to identify the hardware. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/platform/omap3isp/isp.c | 25 ++++++++++++++++--------- 1 files changed, 16 insertions(+), 9 deletions(-) diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index d7aa513..6034dca 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -1345,10 +1345,7 @@ static int isp_enable_clocks(struct isp_device *isp) * has to be twice of what is set on OMAP3430 to get * the required value for cam_mclk */ - if (cpu_is_omap3630()) - divisor = 1; - else - divisor = 2; + divisor = isp->revision == ISP_REVISION_15_0 ? 1 : 2; r = clk_enable(isp->clock[ISP_CLK_CAM_ICK]); if (r) { @@ -2093,7 +2090,11 @@ static int __devinit isp_probe(struct platform_device *pdev) isp->isp_csiphy1.vdd = regulator_get(&pdev->dev, "VDD_CSIPHY1"); isp->isp_csiphy2.vdd = regulator_get(&pdev->dev, "VDD_CSIPHY2"); - /* Clocks */ + /* Clocks + * + * The ISP clock tree is revision-dependent. We thus need to enable ICLK + * manually to read the revision before calling __omap3isp_get(). + */ ret = isp_map_mem_resource(pdev, isp, OMAP3_ISP_IOMEM_MAIN); if (ret < 0) goto error; @@ -2102,6 +2103,16 @@ static int __devinit isp_probe(struct platform_device *pdev) if (ret < 0) goto error; + ret = clk_enable(isp->clock[ISP_CLK_CAM_ICK]); + if (ret < 0) + goto error; + + isp->revision = isp_reg_readl(isp, OMAP3_ISP_IOMEM_MAIN, ISP_REVISION); + dev_info(isp->dev, "Revision %d.%d found\n", + (isp->revision & 0xf0) >> 4, isp->revision & 0x0f); + + clk_disable(isp->clock[ISP_CLK_CAM_ICK]); + if (__omap3isp_get(isp, false) == NULL) { ret = -ENODEV; goto error; @@ -2112,10 +2123,6 @@ static int __devinit isp_probe(struct platform_device *pdev) goto error_isp; /* Memory resources */ - isp->revision = isp_reg_readl(isp, OMAP3_ISP_IOMEM_MAIN, ISP_REVISION); - dev_info(isp->dev, "Revision %d.%d found\n", - (isp->revision & 0xf0) >> 4, isp->revision & 0x0f); - for (m = 0; m < ARRAY_SIZE(isp_res_maps); m++) if (isp->revision == isp_res_maps[m].isp_rev) break; -- Regards, Laurent Pinchart ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] omap3isp: Replace cpu_is_omap3630() with ISP revision check 2012-09-27 14:38 ` [PATCH] omap3isp: Replace cpu_is_omap3630() with ISP revision check Laurent Pinchart @ 2012-09-27 23:59 ` Tony Lindgren 0 siblings, 0 replies; 15+ messages in thread From: Tony Lindgren @ 2012-09-27 23:59 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media, linux-omap, Sakari Ailus, paul * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [120927 07:38]: > Drivers must not rely on cpu_is_omap* macros (they will soon become > private). Use the ISP revision instead to identify the hardware. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Great, thanks for fixing this: Acked-by: Tony Lindgren <tony@atomide.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data 2012-09-26 21:50 ` [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data Sakari Ailus 2012-09-26 22:00 ` Tony Lindgren @ 2012-09-27 9:51 ` Laurent Pinchart 2012-09-27 20:19 ` Sakari Ailus 1 sibling, 1 reply; 15+ messages in thread From: Laurent Pinchart @ 2012-09-27 9:51 UTC (permalink / raw) To: Sakari Ailus; +Cc: paul, linux-media, linux-omap Hi Sakari, Thanks for the patch. On Thursday 27 September 2012 00:50:36 Sakari Ailus wrote: > Configure CSI-2 phy based on platform data in the ISP driver. For that, the > new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same > was configured from the board code. > > This patch is dependent on "omap3: Provide means for changing CSI2 PHY > configuration". > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/platform/omap3isp/isp.h | 3 - > drivers/media/platform/omap3isp/ispcsiphy.c | 161 +++++++++++++----------- > drivers/media/platform/omap3isp/ispcsiphy.h | 10 -- > 3 files changed, 90 insertions(+), 84 deletions(-) > > diff --git a/drivers/media/platform/omap3isp/isp.h > b/drivers/media/platform/omap3isp/isp.h index 8be7487..a2f992c 100644 > --- a/drivers/media/platform/omap3isp/isp.h > +++ b/drivers/media/platform/omap3isp/isp.h > @@ -127,9 +127,6 @@ struct isp_reg { > > struct isp_platform_callback { > u32 (*set_xclk)(struct isp_device *isp, u32 xclk, u8 xclksel); > - int (*csiphy_config)(struct isp_csiphy *phy, > - struct isp_csiphy_dphy_cfg *dphy, > - struct isp_csiphy_lanes_cfg *lanes); > }; > > /* > diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c > b/drivers/media/platform/omap3isp/ispcsiphy.c index 348f67e..1d16e66 100644 > --- a/drivers/media/platform/omap3isp/ispcsiphy.c > +++ b/drivers/media/platform/omap3isp/ispcsiphy.c > @@ -28,41 +28,13 @@ > #include <linux/device.h> > #include <linux/regulator/consumer.h> > > +#include <mach/control.h> > + > #include "isp.h" > #include "ispreg.h" > #include "ispcsiphy.h" > > /* > - * csiphy_lanes_config - Configuration of CSIPHY lanes. > - * > - * Updates HW configuration. > - * Called with phy->mutex taken. > - */ > -static void csiphy_lanes_config(struct isp_csiphy *phy) > -{ > - unsigned int i; > - u32 reg; > - > - reg = isp_reg_readl(phy->isp, phy->cfg_regs, ISPCSI2_PHY_CFG); > - > - for (i = 0; i < phy->num_data_lanes; i++) { > - reg &= ~(ISPCSI2_PHY_CFG_DATA_POL_MASK(i + 1) | > - ISPCSI2_PHY_CFG_DATA_POSITION_MASK(i + 1)); > - reg |= (phy->lanes.data[i].pol << > - ISPCSI2_PHY_CFG_DATA_POL_SHIFT(i + 1)); > - reg |= (phy->lanes.data[i].pos << > - ISPCSI2_PHY_CFG_DATA_POSITION_SHIFT(i + 1)); > - } > - > - reg &= ~(ISPCSI2_PHY_CFG_CLOCK_POL_MASK | > - ISPCSI2_PHY_CFG_CLOCK_POSITION_MASK); > - reg |= phy->lanes.clk.pol << ISPCSI2_PHY_CFG_CLOCK_POL_SHIFT; > - reg |= phy->lanes.clk.pos << ISPCSI2_PHY_CFG_CLOCK_POSITION_SHIFT; > - > - isp_reg_writel(phy->isp, reg, phy->cfg_regs, ISPCSI2_PHY_CFG); > -} > - > -/* > * csiphy_power_autoswitch_enable > * @enable: Sets or clears the autoswitch function enable flag. > */ > @@ -107,46 +79,32 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32 > power) } > > /* > - * csiphy_dphy_config - Configure CSI2 D-PHY parameters. > - * > - * Called with phy->mutex taken. > + * TCLK values are OK at their reset values > */ > -static void csiphy_dphy_config(struct isp_csiphy *phy) > -{ > - u32 reg; > - > - /* Set up ISPCSIPHY_REG0 */ > - reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG0); > - > - reg &= ~(ISPCSIPHY_REG0_THS_TERM_MASK | > - ISPCSIPHY_REG0_THS_SETTLE_MASK); > - reg |= phy->dphy.ths_term << ISPCSIPHY_REG0_THS_TERM_SHIFT; > - reg |= phy->dphy.ths_settle << ISPCSIPHY_REG0_THS_SETTLE_SHIFT; > - > - isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG0); > - > - /* Set up ISPCSIPHY_REG1 */ > - reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG1); > - > - reg &= ~(ISPCSIPHY_REG1_TCLK_TERM_MASK | > - ISPCSIPHY_REG1_TCLK_MISS_MASK | > - ISPCSIPHY_REG1_TCLK_SETTLE_MASK); > - reg |= phy->dphy.tclk_term << ISPCSIPHY_REG1_TCLK_TERM_SHIFT; > - reg |= phy->dphy.tclk_miss << ISPCSIPHY_REG1_TCLK_MISS_SHIFT; > - reg |= phy->dphy.tclk_settle << ISPCSIPHY_REG1_TCLK_SETTLE_SHIFT; > - > - isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG1); > -} > +#define TCLK_TERM 0 > +#define TCLK_MISS 1 > +#define TCLK_SETTLE 14 > > -static int csiphy_config(struct isp_csiphy *phy, > - struct isp_csiphy_dphy_cfg *dphy, > - struct isp_csiphy_lanes_cfg *lanes) > +static int omap3isp_csiphy_config(struct isp_csiphy *phy) > { > + struct isp_csi2_device *csi2 = phy->csi2; > + struct isp_pipeline *pipe = to_isp_pipeline(&csi2->subdev.entity); > + struct isp_v4l2_subdevs_group *subdevs = pipe->external->host_priv; > + struct isp_csiphy_lanes_cfg *lanes; > + int csi2_ddrclk_khz; > unsigned int used_lanes = 0; > unsigned int i; > + unsigned int phy_num; > + u32 reg; > + > + if (subdevs->interface == ISP_INTERFACE_CCP2B_PHY1 > + || subdevs->interface == ISP_INTERFACE_CCP2B_PHY2) > + lanes = &subdevs->bus.ccp2.lanecfg; > + else > + lanes = &subdevs->bus.csi2.lanecfg; > > /* Clock and data lanes verification */ > - for (i = 0; i < phy->num_data_lanes; i++) { > + for (i = 0; i < csi2->phy->num_data_lanes; i++) { > if (lanes->data[i].pol > 1 || lanes->data[i].pos > 3) > return -EINVAL; > > @@ -162,10 +120,72 @@ static int csiphy_config(struct isp_csiphy *phy, > if (lanes->clk.pos == 0 || used_lanes & (1 << lanes->clk.pos)) > return -EINVAL; > > - mutex_lock(&phy->mutex); > - phy->dphy = *dphy; > - phy->lanes = *lanes; > - mutex_unlock(&phy->mutex); > + switch (subdevs->interface) { > + case ISP_INTERFACE_CSI2A_PHY2: > + phy_num = OMAP3_CTRL_CSI2_PHY2_CSI2A; > + break; > + case ISP_INTERFACE_CSI2C_PHY1: > + phy_num = OMAP3_CTRL_CSI2_PHY1_CSI2C; > + break; > + case ISP_INTERFACE_CCP2B_PHY1: > + phy_num = OMAP3_CTRL_CSI2_PHY1_CCP2B; > + break; > + case ISP_INTERFACE_CCP2B_PHY2: > + phy_num = OMAP3_CTRL_CSI2_PHY2_CCP2B; > + break; > + default: > + return -EINVAL; > + } > + > + omap3_ctrl_csi2_phy_cfg(phy_num, true, 0); > + > + /* DPHY timing configuration */ > + /* CSI-2 is DDR and we only count used lanes. */ > + csi2_ddrclk_khz = pipe->external_rate / 1000 > + / (2 * hweight32(used_lanes)) * pipe->external_width; Board code previously used op_sys_clk_freq_hz / 1000 / (2 * hweight32(used_lanes)). Looking at the SMIA++ PLL code, pipe->external_rate is equal to op_sys_clk_freq_hz / bits_per_pixel * lane_op_clock_ratio. Both values are identical if lane_op_clock_ratio is set to 1, which is the case if the SMIAPP_QUIRK_FLAG_OP_PIX_CLOCK_PER_LANE quirk is not set. Have you verified that the new CSI2 DDR clock frequency calculation is correct when the quirk is set ? > + reg = isp_reg_readl(csi2->isp, csi2->phy->phy_regs, ISPCSIPHY_REG0); Isn't csi2->phy == phy ? You could then just write reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG0); and similarly below. > + reg &= ~(ISPCSIPHY_REG0_THS_TERM_MASK | > + ISPCSIPHY_REG0_THS_SETTLE_MASK); > + /* THS_TERM: Programmed value = ceil(12.5 ns/DDRClk period) - 1. */ > + reg |= (DIV_ROUND_UP(25 * csi2_ddrclk_khz, 2000000) - 1) > + << ISPCSIPHY_REG0_THS_TERM_SHIFT; > + /* THS_SETTLE: Programmed value = ceil(90 ns/DDRClk period) + 3. */ > + reg |= (DIV_ROUND_UP(90 * csi2_ddrclk_khz, 1000000) + 3) > + << ISPCSIPHY_REG0_THS_SETTLE_SHIFT; > + > + isp_reg_writel(csi2->isp, reg, csi2->phy->phy_regs, ISPCSIPHY_REG0); > + > + reg = isp_reg_readl(csi2->isp, csi2->phy->phy_regs, ISPCSIPHY_REG1); > + > + reg &= ~(ISPCSIPHY_REG1_TCLK_TERM_MASK | > + ISPCSIPHY_REG1_TCLK_MISS_MASK | > + ISPCSIPHY_REG1_TCLK_SETTLE_MASK); > + reg |= TCLK_TERM << ISPCSIPHY_REG1_TCLK_TERM_SHIFT; > + reg |= TCLK_MISS << ISPCSIPHY_REG1_TCLK_MISS_SHIFT; > + reg |= TCLK_SETTLE << ISPCSIPHY_REG1_TCLK_SETTLE_SHIFT; > + > + isp_reg_writel(csi2->isp, reg, csi2->phy->phy_regs, ISPCSIPHY_REG1); > + > + /* DPHY lane configuration */ > + reg = isp_reg_readl(csi2->isp, csi2->phy->cfg_regs, ISPCSI2_PHY_CFG); > + > + for (i = 0; i < csi2->phy->num_data_lanes; i++) { > + reg &= ~(ISPCSI2_PHY_CFG_DATA_POL_MASK(i + 1) | > + ISPCSI2_PHY_CFG_DATA_POSITION_MASK(i + 1)); > + reg |= (lanes->data[i].pol << > + ISPCSI2_PHY_CFG_DATA_POL_SHIFT(i + 1)); > + reg |= (lanes->data[i].pos << > + ISPCSI2_PHY_CFG_DATA_POSITION_SHIFT(i + 1)); > + } > + > + reg &= ~(ISPCSI2_PHY_CFG_CLOCK_POL_MASK | > + ISPCSI2_PHY_CFG_CLOCK_POSITION_MASK); > + reg |= lanes->clk.pol << ISPCSI2_PHY_CFG_CLOCK_POL_SHIFT; > + reg |= lanes->clk.pos << ISPCSI2_PHY_CFG_CLOCK_POSITION_SHIFT; > + > + isp_reg_writel(csi2->isp, reg, csi2->phy->cfg_regs, ISPCSI2_PHY_CFG); > > return 0; > } > @@ -190,8 +210,9 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy) > if (rval < 0) > goto done; > > - csiphy_dphy_config(phy); > - csiphy_lanes_config(phy); > + rval = omap3isp_csiphy_config(phy); > + if (rval < 0) > + goto done; > > rval = csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_ON); > if (rval) { > @@ -227,8 +248,6 @@ int omap3isp_csiphy_init(struct isp_device *isp) > struct isp_csiphy *phy1 = &isp->isp_csiphy1; > struct isp_csiphy *phy2 = &isp->isp_csiphy2; > > - isp->platform_cb.csiphy_config = csiphy_config; > - > phy2->isp = isp; > phy2->csi2 = &isp->isp_csi2a; > phy2->num_data_lanes = ISP_CSIPHY2_NUM_DATA_LANES; > diff --git a/drivers/media/platform/omap3isp/ispcsiphy.h > b/drivers/media/platform/omap3isp/ispcsiphy.h index e93a661..14551fd 100644 > --- a/drivers/media/platform/omap3isp/ispcsiphy.h > +++ b/drivers/media/platform/omap3isp/ispcsiphy.h > @@ -32,14 +32,6 @@ > struct isp_csi2_device; > struct regulator; > > -struct isp_csiphy_dphy_cfg { > - u8 ths_term; > - u8 ths_settle; > - u8 tclk_term; > - unsigned tclk_miss:1; > - u8 tclk_settle; > -}; > - > struct isp_csiphy { > struct isp_device *isp; > struct mutex mutex; /* serialize csiphy configuration */ > @@ -52,8 +44,6 @@ struct isp_csiphy { > unsigned int phy_regs; > > u8 num_data_lanes; /* number of CSI2 Data Lanes supported */ > - struct isp_csiphy_lanes_cfg lanes; > - struct isp_csiphy_dphy_cfg dphy; > }; > > int omap3isp_csiphy_acquire(struct isp_csiphy *phy); -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data 2012-09-27 9:51 ` [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data Laurent Pinchart @ 2012-09-27 20:19 ` Sakari Ailus 0 siblings, 0 replies; 15+ messages in thread From: Sakari Ailus @ 2012-09-27 20:19 UTC (permalink / raw) To: Laurent Pinchart; +Cc: paul, linux-media, linux-omap Hi Laurent, Thanks for the review. Laurent Pinchart wrote: > On Thursday 27 September 2012 00:50:36 Sakari Ailus wrote: >> Configure CSI-2 phy based on platform data in the ISP driver. For that, the >> new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same >> was configured from the board code. >> >> This patch is dependent on "omap3: Provide means for changing CSI2 PHY >> configuration". >> >> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> >> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> --- >> drivers/media/platform/omap3isp/isp.h | 3 - >> drivers/media/platform/omap3isp/ispcsiphy.c | 161 +++++++++++++----------- >> drivers/media/platform/omap3isp/ispcsiphy.h | 10 -- >> 3 files changed, 90 insertions(+), 84 deletions(-) >> >> diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c >> b/drivers/media/platform/omap3isp/ispcsiphy.c index 348f67e..1d16e66 100644 >> --- a/drivers/media/platform/omap3isp/ispcsiphy.c >> +++ b/drivers/media/platform/omap3isp/ispcsiphy.c ... >> @@ -162,10 +120,72 @@ static int csiphy_config(struct isp_csiphy *phy, >> if (lanes->clk.pos == 0 || used_lanes & (1 << lanes->clk.pos)) >> return -EINVAL; >> >> - mutex_lock(&phy->mutex); >> - phy->dphy = *dphy; >> - phy->lanes = *lanes; >> - mutex_unlock(&phy->mutex); >> + switch (subdevs->interface) { >> + case ISP_INTERFACE_CSI2A_PHY2: >> + phy_num = OMAP3_CTRL_CSI2_PHY2_CSI2A; >> + break; >> + case ISP_INTERFACE_CSI2C_PHY1: >> + phy_num = OMAP3_CTRL_CSI2_PHY1_CSI2C; >> + break; >> + case ISP_INTERFACE_CCP2B_PHY1: >> + phy_num = OMAP3_CTRL_CSI2_PHY1_CCP2B; >> + break; >> + case ISP_INTERFACE_CCP2B_PHY2: >> + phy_num = OMAP3_CTRL_CSI2_PHY2_CCP2B; >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + omap3_ctrl_csi2_phy_cfg(phy_num, true, 0); >> + >> + /* DPHY timing configuration */ >> + /* CSI-2 is DDR and we only count used lanes. */ >> + csi2_ddrclk_khz = pipe->external_rate / 1000 >> + / (2 * hweight32(used_lanes)) * pipe->external_width; > > Board code previously used op_sys_clk_freq_hz / 1000 / (2 * > hweight32(used_lanes)). Looking at the SMIA++ PLL code, pipe->external_rate is > equal to op_sys_clk_freq_hz / bits_per_pixel * lane_op_clock_ratio. Both > values are identical if lane_op_clock_ratio is set to 1, which is the case if > the SMIAPP_QUIRK_FLAG_OP_PIX_CLOCK_PER_LANE quirk is not set. Have you > verified that the new CSI2 DDR clock frequency calculation is correct when the > quirk is set ? Good point. The presence of that quirk flag means that the bit rate (or clock) is per-lane, and not all lanes together as it should be. This is why the value is multiplied by the number of lanes. It should have no visibility outside the SMIA++ driver itself; if it does, then it's a bug in the SMIA++ driver. >> + reg = isp_reg_readl(csi2->isp, csi2->phy->phy_regs, ISPCSIPHY_REG0); > > Isn't csi2->phy == phy ? You could then just write > > reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG0); > > and similarly below. Fixed. Cheers, -- Sakari Ailus sakari.ailus@iki.fi ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2012-10-10 0:22 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-09-26 21:50 [PATCH v2 0/2] OMAP 3 CSI-2 configuration Sakari Ailus 2012-09-26 21:50 ` [PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration Sakari Ailus 2012-09-27 9:20 ` Laurent Pinchart 2012-09-27 20:08 ` Sakari Ailus 2012-10-09 20:50 ` Kevin Hilman 2012-10-09 22:33 ` Sakari Ailus 2012-10-10 0:22 ` Kevin Hilman 2012-09-26 21:50 ` [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data Sakari Ailus 2012-09-26 22:00 ` Tony Lindgren 2012-09-27 9:52 ` Laurent Pinchart 2012-09-28 0:00 ` Tony Lindgren 2012-09-27 14:38 ` [PATCH] omap3isp: Replace cpu_is_omap3630() with ISP revision check Laurent Pinchart 2012-09-27 23:59 ` Tony Lindgren 2012-09-27 9:51 ` [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data Laurent Pinchart 2012-09-27 20:19 ` Sakari Ailus
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.