Hi Yong,
Please see my comments inline.
On Tue, Jun 6, 2017 at 5:39 AM, Yong Zhi wrote:
> Functions to load and install imgu FW blobs
>
> Signed-off-by: Yong Zhi
> ---
> drivers/media/pci/intel/ipu3/ipu3-abi.h| 1572
>
> drivers/media/pci/intel/ipu3/ipu3-css-fw.c | 272 +
> drivers/media/pci/intel/ipu3/ipu3-css-fw.h | 215
> drivers/media/pci/intel/ipu3/ipu3-css.h| 54 +
> 4 files changed, 2113 insertions(+)
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-abi.h
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-fw.c
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-fw.h
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css.h
[snip]
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-css-fw.c
> b/drivers/media/pci/intel/ipu3/ipu3-css-fw.c
> new file mode 100644
> index 000..55edbb8
> --- /dev/null
> +++ b/drivers/media/pci/intel/ipu3/ipu3-css-fw.c
> @@ -0,0 +1,272 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include
Shouldn't need this.
> +#include
> +#include
> +#include
> +
> +#include "ipu3-css.h"
> +#include "ipu3-css-fw.h"
> +
> +static void ipu3_css_fw_show_binary(struct device *dev,
> + struct imgu_fw_info *bi, const char *name)
> +{
> + int i;
> +
> + dev_dbg(dev, "found firmware binary type %i size %i name %s\n",
> + bi->type, bi->blob.size, name);
> + if (bi->type != IMGU_FW_ISP_FIRMWARE)
> + return;
> +
> + dev_dbg(dev, "id %i mode %i bds 0x%x veceven %i/%i out_pins %i\n",
> + bi->info.isp.sp.id, bi->info.isp.sp.pipeline.mode,
> + bi->info.isp.sp.bds.supported_bds_factors,
> + bi->info.isp.sp.enable.vf_veceven,
> + bi->info.isp.sp.vf_dec.is_variable,
> + bi->info.isp.num_output_pins);
> +
> + dev_dbg(dev, "input (%i,%i)-(%i,%i) formats %s%s%s\n",
> + bi->info.isp.sp.input.min_width,
> + bi->info.isp.sp.input.min_height,
> + bi->info.isp.sp.input.max_width,
> + bi->info.isp.sp.input.max_height,
> + bi->info.isp.sp.enable.input_yuv ? "yuv420 " : "",
> + bi->info.isp.sp.enable.input_feeder ||
> + bi->info.isp.sp.enable.input_raw ? "raw8 raw10 " : "",
> + bi->info.isp.sp.enable.input_raw ? "raw12" : "");
> +
> + dev_dbg(dev, "internal (%i,%i)\n",
> + bi->info.isp.sp.internal.max_width,
> + bi->info.isp.sp.internal.max_height);
> +
> + dev_dbg(dev, "output (%i,%i)-(%i,%i) formats",
> + bi->info.isp.sp.output.min_width,
> + bi->info.isp.sp.output.min_height,
> + bi->info.isp.sp.output.max_width,
> + bi->info.isp.sp.output.max_height);
> + for (i = 0; i < bi->info.isp.num_output_formats; i++)
> + dev_dbg(dev, " %i", bi->info.isp.output_formats[i]);
> + dev_dbg(dev, " vf");
> + for (i = 0; i < bi->info.isp.num_vf_formats; i++)
> + dev_dbg(dev, " %i", bi->info.isp.vf_formats[i]);
When the function is called, neither num_output_formats nor
num_vf_formats is validated. It will cause an out of bound read here
if it isn't correct.
> + dev_dbg(dev, "\n");
> +}
> +
> +const int ipu3_css_fw_obgrid_size(const struct imgu_fw_info *bi)
> +{
> + unsigned int stripes = bi->info.isp.sp.iterator.num_stripes;
> + unsigned int width, height, obgrid_size;
> +
> + width = ALIGN(DIV_ROUND_UP(bi->info.isp.sp.internal.max_width,
> + IMGU_OBGRID_TILE_SIZE * 2) + 1, IPU3_UAPI_ISP_VEC_ELEMS / 4);
> + height = DIV_ROUND_UP(bi->info.isp.sp.internal.max_height,
> + IMGU_OBGRID_TILE_SIZE * 2) + 1;
> + obgrid_size = PAGE_ALIGN(width * height *
> + sizeof(struct ipu3_uapi_obgrid_param)) * stripes;
> +
> + return obgrid_size;
> +}
> +
> +void *ipu3_css_fw_pipeline_params(struct ipu3_css *css,
> + enum imgu_abi_param_class c, enum imgu_abi_memories m,
> + struct imgu_fw_isp_parameter *par, size_t par_size,
> + void *binary_params)
> +{
> + struct imgu_fw_info *bi =
> &css->fwp->binary_header[css->current_binary];
> +
> + if (par->offset + par->size >
> + bi->info.isp.sp.mem_initializers.params[c][m].size)
> + return NULL;
> +
> + if (par->size != par_size)
> + pr_warn(