Re: [PATCH 2/7] [media] vimc: sen: Integrate the tpg on the sensor

Laurent Pinchart
2015-08-13T20:28:19Z

Hi Helen,

Thank you for the patch.

On Thursday 06 August 2015 17:26:09 Helen Fornazier wrote:
> Initialize the test pattern generator on the sensor
> Generate a colored bar image instead of a grey one
>
> Signed-off-by: Helen Fornazier <helen.fornazier@gmail.com>
> ---
> drivers/media/platform/vimc/Kconfig | 1 +
> drivers/media/platform/vimc/vimc-sensor.c | 44 ++++++++++++++++++++++++++--
> 2 files changed, 43 insertions(+), 2 deletions(-)

[snip]

> diff --git a/drivers/media/platform/vimc/vimc-sensor.c
> b/drivers/media/platform/vimc/vimc-sensor.c index d613792..a2879ad 100644
> --- a/drivers/media/platform/vimc/vimc-sensor.c
> +++ b/drivers/media/platform/vimc/vimc-sensor.c
> @@ -16,15 +16,19 @@
> */
>
> #include <linux/freezer.h>
> +#include <media/tpg.h>
> #include <linux/vmalloc.h>
> #include <linux/v4l2-mediabus.h>

media/tpg.h should have been inserted here.

> #include <media/v4l2-subdev.h>
>
> #include "vimc-sensor.h"

[snip]

> +static void vimc_sen_tpg_s_format(struct vimc_sen_device *vsen)
> +{
> + const struct vimc_pix_map *vpix;
> +
> + vpix = vimc_pix_map_by_code(vsen->mbus_format.code);
> + /* This should never be NULL, as we won't allow any format other then
> + * the ones in the vimc_pix_map_list table */
> + BUG_ON(!vpix);

BUG_ON() is quite harsh, it will stop the machine. If the condition can never
happen then you can remove the check. If you're worried it might happen I'd
use a WARN_ON() and return.

> + tpg_s_bytesperline(&vsen->tpg, 0,
> + vsen->mbus_format.width * vpix->bpp);
> + tpg_s_buf_height(&vsen->tpg, vsen->mbus_format.height);
> + tpg_s_fourcc(&vsen->tpg, vpix->pixelformat);
> + /* TODO: check why the tpg_s_field need this third argument if
> + * it is already receiving the field */
> + tpg_s_field(&vsen->tpg, vsen->mbus_format.field,
> + vsen->mbus_format.field == V4L2_FIELD_ALTERNATE);
> + tpg_s_colorspace(&vsen->tpg, vsen->mbus_format.colorspace);
> + tpg_s_ycbcr_enc(&vsen->tpg, vsen->mbus_format.ycbcr_enc);
> + tpg_s_quantization(&vsen->tpg, vsen->mbus_format.quantization);
> + tpg_s_xfer_func(&vsen->tpg, vsen->mbus_format.xfer_func);
> +}

--
Regards,

Laurent Pinchart