On Sat, Jun 07, 2014 at 02:56:10PM -0700, Steve Longerbeam wrote:
> Adds the following new IPU units:
> 
> - Camera Sensor Interface (csi)
> - Sensor Multi FIFO Controller (smfc)
> - Image Converter (ic)
> - Image Rotator (irt)
> 
> Signed-off-by: Steve Longerbeam <steve_longerb...@mentor.com>
> ---
>  drivers/staging/imx-drm/ipu-v3/Makefile     |    3 +-
>  drivers/staging/imx-drm/ipu-v3/ipu-common.c |   67 ++-
>  drivers/staging/imx-drm/ipu-v3/ipu-csi.c    |  821 ++++++++++++++++++++++++++
>  drivers/staging/imx-drm/ipu-v3/ipu-ic.c     |  835 
> +++++++++++++++++++++++++++
>  drivers/staging/imx-drm/ipu-v3/ipu-irt.c    |  103 ++++
>  drivers/staging/imx-drm/ipu-v3/ipu-prv.h    |   24 +
>  drivers/staging/imx-drm/ipu-v3/ipu-smfc.c   |  348 +++++++++++
>  include/linux/platform_data/imx-ipu-v3.h    |  151 +++++
>  8 files changed, 2350 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/staging/imx-drm/ipu-v3/ipu-csi.c
>  create mode 100644 drivers/staging/imx-drm/ipu-v3/ipu-ic.c
>  create mode 100644 drivers/staging/imx-drm/ipu-v3/ipu-irt.c
>  create mode 100644 drivers/staging/imx-drm/ipu-v3/ipu-smfc.c
> 
> diff --git a/drivers/staging/imx-drm/ipu-v3/Makefile 
> b/drivers/staging/imx-drm/ipu-v3/Makefile
> index 28ed72e..79c0c88 100644
> --- a/drivers/staging/imx-drm/ipu-v3/Makefile
> +++ b/drivers/staging/imx-drm/ipu-v3/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_DRM_IMX_IPUV3_CORE) += imx-ipu-v3.o
>  
> -imx-ipu-v3-objs := ipu-common.o ipu-dc.o ipu-di.o ipu-dp.o ipu-dmfc.o
> +imx-ipu-v3-objs := ipu-common.o ipu-csi.o ipu-dc.o ipu-di.o \
> +     ipu-dp.o ipu-dmfc.o ipu-ic.o ipu-irt.o ipu-smfc.o
> diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-common.c 
> b/drivers/staging/imx-drm/ipu-v3/ipu-common.c
> index 1c606b5..3d7e28d 100644
> --- a/drivers/staging/imx-drm/ipu-v3/ipu-common.c
> +++ b/drivers/staging/imx-drm/ipu-v3/ipu-common.c
> @@ -834,6 +834,10 @@ struct ipu_devtype {
>       unsigned long cpmem_ofs;
>       unsigned long srm_ofs;
>       unsigned long tpm_ofs;
> +     unsigned long csi0_ofs;
> +     unsigned long csi1_ofs;
> +     unsigned long smfc_ofs;
> +     unsigned long ic_ofs;
>       unsigned long disp0_ofs;
>       unsigned long disp1_ofs;
>       unsigned long dc_tmpl_ofs;
> @@ -873,8 +877,12 @@ static struct ipu_devtype ipu_type_imx6q = {
>       .cpmem_ofs = 0x00300000,
>       .srm_ofs = 0x00340000,
>       .tpm_ofs = 0x00360000,
> +     .csi0_ofs = 0x00230000,
> +     .csi1_ofs = 0x00238000,
>       .disp0_ofs = 0x00240000,
>       .disp1_ofs = 0x00248000,
> +     .smfc_ofs =  0x00250000,
> +     .ic_ofs = 0x00220000,
>       .dc_tmpl_ofs = 0x00380000,
>       .vdi_ofs = 0x00268000,
>       .type = IPUV3H,
> @@ -915,8 +923,44 @@ static int ipu_submodules_init(struct ipu_soc *ipu,
>       struct device *dev = &pdev->dev;
>       const struct ipu_devtype *devtype = ipu->devtype;
>  
> +     ret = ipu_csi_init(ipu, dev, 0, ipu_base + devtype->csi0_ofs,
> +                        IPU_CONF_CSI0_EN, ipu_clk);
> +     if (ret) {
> +             unit = "csi0";
> +             goto err_csi_0;
> +     }

Please be nice to other SoCs. You set csi0_ofs for i.MX6, but not for
i.MX5. For i.MX5 you silently initialize the CSI with bogus register
offsets. Either test for _ofs == 0 before initializing it or add the
correct values for i.MX51/53 (preferred).

> +int ipu_csi_set_src(struct ipu_csi *csi, u32 vc, bool select_mipi_csi2)
> +{
> +     struct ipu_soc *ipu = csi->ipu;
> +     int ipu_id = ipu_get_num(ipu);
> +     u32 val, bit;
> +
> +     /*
> +      * Set the muxes that choose between mipi-csi2 and parallel inputs
> +      * to the CSI's.
> +      */
> +     if (csi->ipu->ipu_type == IPUV3HDL) {
> +             /*
> +              * On D/L, the mux is in GPR13. The TRM is unclear,
> +              * but it appears the mux allows selecting the MIPI
> +              * CSI-2 virtual channel number to route to the CSIs.
> +              */
> +             bit = GPR13_IPUV3HDL_CSI_MUX_CTL_SHIFT + csi->id * 3;
> +             val = select_mipi_csi2 ? vc << bit : 4 << bit;
> +             regmap_update_bits(ipu->gp_reg, IOMUXC_GPR13,
> +                                0x7 << bit, val);
> +     } else if (csi->ipu->ipu_type == IPUV3H) {
> +             /*
> +              * For Q/D, the mux only exists on IPU0-CSI0 and IPU1-CSI1,
> +              * and the routed virtual channel numbers are fixed at 0 and
> +              * 3 respectively.
> +              */
> +             if ((ipu_id == 0 && csi->id == 0) ||
> +                 (ipu_id == 1 && csi->id == 1)) {
> +                     bit = GPR1_IPU_CSI_MUX_CTL_SHIFT + csi->ipu->id;
> +                     val = select_mipi_csi2 ? 0 : 1 << bit;
> +                     regmap_update_bits(ipu->gp_reg, IOMUXC_GPR1,
> +                                        0x1 << bit, val);
> +             }
> +     } else {
> +             dev_err(csi->ipu->dev,
> +                     "ERROR: %s: unsupported CPU version!\n",
> +                     __func__);
> +             return -EINVAL;
> +     }
> +
> +     /*
> +      * there is another mux in the IPU config register that
> +      * must be set as well
> +      */
> +     ipu_set_csi_src_mux(ipu, csi->id, select_mipi_csi2);

This is not a property of the IPU but how the IPU is integrated into the
SoC. I'm unsure this should be integrated like this, it sounds more like
a job for mediactrl.

> +#include <linux/types.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <linux/spinlock.h>
> +#include <linux/videodev2.h>
> +#include <linux/bitrev.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>

Please clean up the include list. There is nothing used from
clk-provider.h for example.

> +static int init_csc(struct ipu_ic *ic,
> +                 enum ipu_color_space in_format,
> +                 enum ipu_color_space out_format,
> +                 int csc_index);
> +static int calc_resize_coeffs(struct ipu_ic *ic,
> +                           u32 in_size, u32 out_size,
> +                           u32 *resize_coeff,
> +                           u32 *downsize_coeff);

Please reorder functions to get rid of static function declarations.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
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

Reply via email to