Hi, thank you all for your review
On Fri, Aug 14, 2015 at 9:15 AM, Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
> On 08/06/2015 10:26 PM, Helen Fornazier wrote:
>> Initialize the test pattern generator on the sensor
>> Generate a colored bar image instead of a grey one
>
> You don't want to put the tpg in every sensor and have all the blocks in
> between process the video. This is all virtual, so all that is necessary
> is to put the tpg in every DMA engine (video node) but have the subdevs
> modify the tpg setting when you start the pipeline.
>
> So the source would set the width/height to the sensor resolution, and it
> will initialize the crop/compose rectangles. Every other entity in the
> pipeline will continue modifying according to what they do. E.g. a scaler
> will just change the compose rectangle.
>
> When you start streaming the tpg will generate the image based on all those
> settings as if all the entities would actually do the work.
>
> Of course, this assumes the processing the entities do map to what the tpg
> can do, but that's true for vimc.
>
> An additional advantage is that the entities can use a wide range of
> mediabus formats since the tpg can generate basically anything. Implementing
> multiplanar is similarly easy. This would be much harder if you had to write
> the image processing code for the entities since you'd either have to support
> lots of different formats (impractical) or limit yourself to just a few.
>
>>
>> Signed-off-by: Helen Fornazier <helen.fornazier@xxxxxxxxx>
>> ---
>> drivers/media/platform/vimc/Kconfig | 1 +
>> drivers/media/platform/vimc/vimc-sensor.c | 44 +++++++++++++++++++++++++++++--
>> 2 files changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/vimc/Kconfig b/drivers/media/platform/vimc/Kconfig
>> index 81279f4..7cf7e84 100644
>> --- a/drivers/media/platform/vimc/Kconfig
>> +++ b/drivers/media/platform/vimc/Kconfig
>> @@ -1,6 +1,7 @@
>> config VIDEO_VIMC
>> tristate "Virtual Media Controller Driver (VIMC)"
>> select VIDEO_V4L2_SUBDEV_API
>> + select VIDEO_TPG
>> default n
>> ---help---
>> Skeleton driver for Virtual Media Controller
>> 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>
>> #include <media/v4l2-subdev.h>
>>
>> #include "vimc-sensor.h"
>>
>> +#define VIMC_SEN_FRAME_MAX_WIDTH 4096
>> +
>> struct vimc_sen_device {
>> struct vimc_ent_device ved;
>> struct v4l2_subdev sd;
>> + struct tpg_data tpg;
>> struct v4l2_device *v4l2_dev;
>> struct device *dev;
>> struct task_struct *kthread_sen;
>> @@ -87,6 +91,29 @@ static int vimc_sen_get_fmt(struct v4l2_subdev *sd,
>> return 0;
>> }
>>
>> +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);
>> +
>> + 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);
>
> Actually the second argument argument cannot be FIELD_ALTERNATE. If it
> is field ALTERNATE, then the second argument must be either FIELD_TOP or
> FIELD_BOTTOM: i.e. it tells the generator which field comes first in the
> FIELD_ALTERNATE case.
>
> And in case you are wondering: it's always FIELD_TOP except for 60 Hz SDTV
> formats where it is FIELD_BOTTOM.
I am not really familiar to SDTV, but it seems to me to be a different
structure API. Thus can I put FIELD_TOP directly in the second
argument?
tpg_s_field(&vsen->tpg, V4L2_FIELD_TOP,
vsen->mbus_format.field == V4L2_FIELD_ALTERNATE);
Or is there a way to check if the format is SDTV? I didn't find it
defined on V4L2_PIX_FMT_
>
>> + 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);
>> +}
>> +
>> static const struct v4l2_subdev_pad_ops vimc_sen_pad_ops = {
>> .enum_mbus_code = vimc_sen_enum_mbus_code,
>> .enum_frame_size = vimc_sen_enum_frame_size,
>> @@ -112,7 +139,7 @@ static int vimc_thread_sen(void *data)
>> if (kthread_should_stop())
>> break;
>>
>> - memset(vsen->frame, 100, vsen->frame_size);
>> + tpg_fill_plane_buffer(&vsen->tpg, V4L2_STD_PAL, 0, vsen->frame);
>>
>> /* Send the frame to all source pads */
>> for (i = 0; i < vsen->sd.entity.num_pads; i++)
>> @@ -192,6 +219,7 @@ static void vimc_sen_destroy(struct vimc_ent_device *ved)
>> struct vimc_sen_device *vsen = container_of(ved,
>> struct vimc_sen_device, ved);
>>
>> + tpg_free(&vsen->tpg);
>> media_entity_cleanup(ved->ent);
>> v4l2_device_unregister_subdev(&vsen->sd);
>> kfree(vsen);
>> @@ -242,6 +270,16 @@ struct vimc_ent_device *vimc_sen_create(struct v4l2_device *v4l2_dev,
>> vsen->mbus_format.quantization = V4L2_QUANTIZATION_FULL_RANGE;
>> vsen->mbus_format.xfer_func = V4L2_XFER_FUNC_SRGB;
>>
>> + /* Initialize the test pattern generator */
>> + tpg_init(&vsen->tpg, vsen->mbus_format.width,
>> + vsen->mbus_format.height);
>> + ret = tpg_alloc(&vsen->tpg, VIMC_SEN_FRAME_MAX_WIDTH);
>> + if (ret)
>> + goto err_clean_m_ent;
>> +
>> + /* Configure the tpg */
>> + vimc_sen_tpg_s_format(vsen);
>> +
>> /* Fill the vimc_ent_device struct */
>> vsen->ved.destroy = vimc_sen_destroy;
>> vsen->ved.ent = &vsen->sd.entity;
>> @@ -261,11 +299,13 @@ struct vimc_ent_device *vimc_sen_create(struct v4l2_device *v4l2_dev,
>> if (ret) {
>> dev_err(vsen->dev,
>> "subdev register failed (err=%d)\n", ret);
>> - goto err_clean_m_ent;
>> + goto err_free_tpg;
>> }
>>
>> return &vsen->ved;
>>
>> +err_free_tpg:
>> + tpg_free(&vsen->tpg);
>> err_clean_m_ent:
>> media_entity_cleanup(&vsen->sd.entity);
>> err_clean_pads:
>>
>
> Regards,
>
> Hans
--
Helen Fornazier
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
