Re: [PATCH v4 00/11] [media]: vimc: Virtual Media Control VPU's

2017-06-19 Thread Hans Verkuil

Hi Helen,

On 06/13/2017 09:35 PM, Helen Koike wrote:

This patch series improves the current video processing units in vimc
(by adding more controls to the sensor and capture node, allowing the
user to configure different frame formats) and also adds a debayer
and a scaler node.
The debayer transforms the bayer format image received in its sink pad
to a bayer format by averaging the pixels within a mean window.
The scaler only scales up the image for now.

This patch series is based on media/master and it is available at:
https://github.com/helen-fornazier/opw-staging/tree/z/sent/vimc/vpu/v4

In this version I removed the commit
[media] vimc: Optimize frame generation through the pipe
There was a bug when multiple links going to the same sink was enabled,
I'll rework on it and re-send it in a different patch series


I ran the v4 patch series through my sparse/smatch test and found a few 
warnings:

sparse:

drivers/media/platform/vimc/vimc-debayer.c:376:30: warning: symbol 
'vimc_deb_video_ops' was not declared. Should it be static?
drivers/media/platform/vimc/vimc-scaler.c:270:30: warning: symbol 
'vimc_sca_video_ops' was not declared. Should it be static?
drivers/media/platform/vimc/vimc-sensor.c:285:30: warning: symbol 
'vimc_sen_video_ops' was not declared. Should it be static?

smatch:

drivers/media/platform/vimc/vimc-core.c:271 vimc_add_subdevs() warn: always true 
condition '(--i >= 0) => (0-u32max >= 0)'
drivers/media/platform/vimc/vimc-core.c:271 vimc_add_subdevs() warn: always true 
condition '(--i >= 0) => (0-u32max >= 0)'

Can you take a look?

Regards,

Hans


[PATCH v4 00/11] [media]: vimc: Virtual Media Control VPU's

2017-06-13 Thread Helen Koike
This patch series improves the current video processing units in vimc
(by adding more controls to the sensor and capture node, allowing the
user to configure different frame formats) and also adds a debayer
and a scaler node.
The debayer transforms the bayer format image received in its sink pad
to a bayer format by averaging the pixels within a mean window.
The scaler only scales up the image for now.

This patch series is based on media/master and it is available at:
https://github.com/helen-fornazier/opw-staging/tree/z/sent/vimc/vpu/v4

In this version I removed the commit
[media] vimc: Optimize frame generation through the pipe
There was a bug when multiple links going to the same sink was enabled,
I'll rework on it and re-send it in a different patch series

Changes in v4:
[media] vimc: common: Add vimc_link_validate
- remove vimc_fmt_pix_to_mbus(), replaced by
v4l2_fill_mbus_format()
- remove EXPORT_SYMBOL(vimc_link_validate), not necessary in
this patch, moved to submodules patch
- Fix multi-line comment style
- If colorspace is set to DEFAULT, then assume all the other
colorimetry parameters are also DEFAULT
[media] vimc: common: Add vimc_colorimetry_clamp
- this is a new patch in the series
[media] vimc: sen: Support several image formats
- use vimc_colorimetry_clamp macro
- replace V4L2_COLORSPACE_SRGB by V4L2_COLORSPACE_DEFAULT in the
default format struct
[media] vimc: cap: Support several image formats
- add vimc_colorimetry_clamp macro
- replace V4L2_COLORSPACE_SRGB by V4L2_COLORSPACE_DEFAULT in the
default format struct
[media] vimc: Subdevices as modules
- Rebase without [media] vimc: Optimize frame generation the through
pipe
- s/EXPORT_SYMBOL/EXPORT_SYMBOL_GPL
- add struct vimc_platform_data to pass the entity's name to the
sudmodule
- Fix comment about vimc-input (remove vimc-output comment)
[media] vimc: deb: Add debayer filter
- Rebase without [media] vimc: Optimize frame generation through
pipe
- use vimc_colorimetry_clamp
- replace V4L2_COLORSPACE_SRGB by V4L2_COLORSPACE_DEFAULT in the
default format struct
- use struct vimc_platform_data to retrieve the entity's name
[media] vimc: sca: Add scaler
- use vimc_colorimetry_clamp
- replace V4L2_COLORSPACE_SRGB by V4L2_COLORSPACE_DEFAULT in the
default format struct
- use struct vimc_platform_data to retrieve the entity's name

Changes in v3:
[media] vimc: sen: Integrate the tpg on the sensor
- Declare frame_size as a local variable
- Set tpg frame format before starting kthread
- s_stream(sd, 1): return 0 if stream is already enabled
- s_stream(sd, 0): return 0 if stream is already disabled
- s_stream: propagate error from kthread_stop
- coding style when calling tpg_s_bytesperline
- s/vimc_thread_sen/vimc_sen_tpg_thread
- fix multiline comment
- remove V4L2_FIELD_ALTERNATE from tpg_s_field
- remove V4L2_STD_PAL from tpg_fill_plane_buffer
[media] vimc: Move common code from the core
- This is a new patch in the series
[media] vimc: common: Add vimc_ent_sd_* helper
- add it in vimc-common.c instead in vimc-core.c
- fix vimc_ent_sd_register, use function parameter to assign
sd->entity.function instead of using a fixed value
- rename commit to add the "common" tag
[media] vimc: Add vimc_pipeline_s_stream in the core
- add it in vimc-common instead of vimc-core
- rename commit with "common" tag
[media] vimc: common: Add vimc_link_validate
- this is a new patch in the series
[media] vimc: sen: Support several image formats
- remove support for V4L2_FIELD_ALTERNATE (left as TODO for now)
- clamp image size to an even dimension for height and width
- set default values for colorimetry using _DEFAULT macro
- reset all values of colorimetry to _DEFAULT if user tries to
set an invalid colorspace
[media] vimc: cap: Support several image formats
- use *_DEFAULT macros for colorimetry in the default format
- clamp height and width of the image by an even value
- is user try to set colorspace to an invalid format, set all
colorimetry parameters to _DEFAULT
- remove V4L2_FMT_FLAG_COMPRESSED from vimc_cap_enum_fmt_vid_cap
- remove V4L2_BUF_TYPE_VIDEO_CAPTURE from vimc_cap_enum_fmt_vid_cap
- increase step_width and step_height to 2 instead of 1
- remove link validate function, use the one in vimc-common.c
[media] vimc: Subdevices as modules
- This is a new patch in the series
[media] vimc: deb: Add debayer filter
- Declare frame_size as a local variable
- s_stream(sd, 1): return 0 if stream is already enabled
- s_stream(sd, 0):