Re: [PATCH 2/2] drm: adv7511: Add support for i2c_new_secondary_device

[Kieran Bingham]

On 22/01/18 13:00, Lars-Peter Clausen wrote:
> On 01/22/2018 01:50 PM, Kieran Bingham wrote:
>> The ADV7511 has four 256-byte maps that can be accessed via the main I²C
>> ports. Each map has it own I²C address and acts as a standard slave
>> device on the I²C bus.
>>
>> Allow a device tree node to override the default addresses so that
>> address conflicts with other devices on the same bus may be resolved at
>> the board description level.
>>
>> Signed-off-by: Kieran Bingham 
> 
> I've been working on the same thing, but you've beat me to it! Patch looks
> mostly OK, but I think you are missing this piece:
> 
> https://github.com/analogdevicesinc/linux/commit/ba9b57507cb78724a606eb24104e22fea942437d#diff-2cf1828c644e351adefabe9509410400L553

Ah yes - Thanks, - you're correct - I had missed that bit.

Added locally for a v2.
--
Kieran

>> ---
>>  .../bindings/display/bridge/adi,adv7511.txt| 10 +-
>>  drivers/gpu/drm/bridge/adv7511/adv7511.h   |  4 +++
>>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   | 36 
>> ++
>>  3 files changed, 37 insertions(+), 13 deletions(-)
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt 
>> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> index 0047b1394c70..f6bb9f6d3f48 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> @@ -70,6 +70,9 @@ Optional properties:
>>rather than generate its own timings for HDMI output.
>>  - clocks: from common clock binding: reference to the CEC clock.
>>  - clock-names: from common clock binding: must be "cec".
>> +- reg-names : Names of maps with programmable addresses.
>> +It can contain any map needing a non-default address.
>> +Possible maps names are : "main", "edid", "cec", "packet"
>>  
>>  Required nodes:
>>  
>> @@ -88,7 +91,12 @@ Example
>>  
>>  adv7511w: hdmi@39 {
>>  compatible = "adi,adv7511w";
>> -reg = <39>;
>> +/*
>> + * The EDID page will be accessible on address 0x66 on the i2c
>> + * bus. All other maps continue to use their default addresses.
>> + */
>> +reg = <0x39 0x66>;
>> +reg-names = "main", "edid";
>>  interrupt-parent = <>;
>>  interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
>>  clocks = <_clock>;
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h 
>> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> index d034b2cb5eee..7d81ce3808e0 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> @@ -53,8 +53,10 @@
>>  #define ADV7511_REG_POWER   0x41
>>  #define ADV7511_REG_STATUS  0x42
>>  #define ADV7511_REG_EDID_I2C_ADDR   0x43
>> +#define ADV7511_REG_EDID_I2C_ADDR_DEFAULT   0x3f
>>  #define ADV7511_REG_PACKET_ENABLE1  0x44
>>  #define ADV7511_REG_PACKET_I2C_ADDR 0x45
>> +#define ADV7511_REG_PACKET_I2C_ADDR_DEFAULT 0x38
>>  #define ADV7511_REG_DSD_ENABLE  0x46
>>  #define ADV7511_REG_VIDEO_INPUT_CFG20x48
>>  #define ADV7511_REG_INFOFRAME_UPDATE0x4a
>> @@ -89,6 +91,7 @@
>>  #define ADV7511_REG_TMDS_CLOCK_INV  0xde
>>  #define ADV7511_REG_ARC_CTRL0xdf
>>  #define ADV7511_REG_CEC_I2C_ADDR0xe1
>> +#define ADV7511_REG_CEC_I2C_ADDR_DEFAULT0x3c
>>  #define ADV7511_REG_CEC_CTRL0xe2
>>  #define ADV7511_REG_CHIP_ID_HIGH0xf5
>>  #define ADV7511_REG_CHIP_ID_LOW 0xf6
>> @@ -322,6 +325,7 @@ struct adv7511 {
>>  struct i2c_client *i2c_main;
>>  struct i2c_client *i2c_edid;
>>  struct i2c_client *i2c_cec;
>> +struct i2c_client *i2c_packet;
>>  
>>  struct regmap *regmap;
>>  struct regmap *regmap_cec;
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> index efa29db5fc2b..7ec33837752b 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> @@ -969,8 +969,8 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv)
>>  {
>>  int ret;
>>  
>> -adv->i2c_cec = i2c_new_dummy(adv->i2c_main->adapter,
>> - adv->i2c_main->addr - 1);
>> +adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec",
>> +ADV7511_REG_CEC_I2C_ADDR_DEFAULT);
>>  if (!adv->i2c_cec)
>>  return -ENOMEM;
>>  i2c_set_clientdata(adv->i2c_cec, adv);
>> @@ -1082,8 +1082,6 @@ static int adv7511_probe(struct i2c_client *i2c, const 
>> struct i2c_device_id *id)
>>  struct adv7511_link_config link_config;
>>  struct adv7511 *adv7511;
>>  struct device *dev = >dev;
>> -unsigned int 

Re: [Patch v6 12/12] Documention: v4l: Documentation for HEVC CIDs

[Smitha T Murthy]

On Mon, 2018-01-22 at 13:15 +0100, Hans Verkuil wrote:
> On 08/12/17 10:08, Smitha T Murthy wrote:
> > Added V4l2 controls for HEVC encoder
> > 
> > Signed-off-by: Smitha T Murthy 
> > ---
> >  Documentation/media/uapi/v4l/extended-controls.rst | 395 
> > +
> >  1 file changed, 395 insertions(+)
> > 
> > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst 
> > b/Documentation/media/uapi/v4l/extended-controls.rst
> > index a3e81c1..3c92763 100644
> > --- a/Documentation/media/uapi/v4l/extended-controls.rst
> > +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> > @@ -1960,6 +1960,401 @@ enum v4l2_vp8_golden_frame_sel -
> >  1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
> >  
> >  
> > +High Efficiency Video Coding (HEVC/H.265) Control Reference
> > +---
> > +
> > +The HEVC/H.265 controls include controls for encoding parameters of 
> > HEVC/H.265
> > +video codec.
> > +
> > +
> > +.. _hevc-control-id:
> > +
> > +HEVC/H.265 Control IDs
> > +^^
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (integer)``
> > +Minimum quantization parameter for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP (integer)``
> > +Maximum quantization parameter for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP (integer)``
> > +Quantization parameter for an I frame for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP (integer)``
> > +Quantization parameter for a P frame for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_QP (integer)``
> > +Quantization parameter for a B frame for HEVC.
> 
> I assume these values all have to be in the range MIN_QP to MAX_QP?
> If so, then this should be documented I think.
> 
Yes they should be in the limit. I will mention the range in next
version.

> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_QP (boolean)``
> > +HIERARCHICAL_QP allows host to specify the quantization parameter 
> > values
> 
> host -> the host
ok I will change it.
> 
> > +for each temporal layer through HIERARCHICAL_QP_LAYER. This is valid 
> > only
> > +if HIERARCHICAL_CODING_LAYER is greater than 1. Setting the control 
> > value
> > +to 1 enables setting of the QP values for the layers.
> > +
> > +.. _v4l2-hevc-hier-coding-type:
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_TYPE``
> > +(enum)
> > +
> > +enum v4l2_mpeg_video_hevc_hier_coding_type -
> > +Selects the hierarchical coding type for encoding. Possible values are:
> > +
> > +.. raw:: latex
> > +
> > +\begin{adjustbox}{width=\columnwidth}
> > +
> > +.. tabularcolumns:: |p{11.0cm}|p{10.0cm}|
> > +
> > +.. flat-table::
> > +:header-rows:  0
> > +:stub-columns: 0
> > +
> > +* - ``V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_B``
> > +  - Use the B frame for hierarchical coding.
> > +* - ``V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_P``
> > +  - Use the P frame for hierarchical coding.
> > +
> > +.. raw:: latex
> > +
> > +\end{adjustbox}
> > +
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER (integer)``
> > +Selects the hierarchical coding layer. In normal encoding
> > +(non-hierarchial coding), it should be zero. Possible values are 0 ~ 6.
> 
> Use - instead of ~. Or just say: [0, 6]
> 
ok I will change it.
> > +0 indicates HIERARCHICAL CODING LAYER 0, 1 indicates HIERARCHICAL 
> > CODING
> > +LAYER 1 and so on.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L0_QP (integer)``
> > +Indicates quantization parameter for hierarchical coding layer 0.
> > +For HEVC it can have a value of 0-51.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L1_QP (integer)``
> > +Indicates quantization parameter for hierarchical coding layer 1.
> > +For HEVC it can have a value of 0-51.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L2_QP (integer)``
> > +Indicates quantization parameter for hierarchical coding layer 2.
> > +For HEVC it can have a value of 0-51.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L3_QP (integer)``
> > +Indicates quantization parameter for hierarchical coding layer 3.
> > +For HEVC it can have a value of 0-51.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L4_QP (integer)``
> > +Indicates quantization parameter for hierarchical coding layer 4.
> > +For HEVC it can have a value of 0-51.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L5_QP (integer)``
> > +Indicates quantization parameter for hierarchical coding layer 5.
> > +For HEVC it can have a value of 0-51.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_QP (integer)``
> > +Indicates quantization parameter for hierarchical coding layer 6.
> > +For HEVC it can have a value of 0-51.
> 
> How do these controls relate to MIN_QP and MAX_QP?
> 
Here each layer will adhere to MAX_QP and MIN_QP set.
> > +
> > +.. _v4l2-hevc-profile:
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_PROFILE``
> > 

Re: [Patch v6 00/12] Add MFC v10.10 support

[Smitha T Murthy]

On Mon, 2018-01-22 at 13:18 +0100, Hans Verkuil wrote:
> Hi Smitha,
> 
> Thank you for this v6 series!
> 
> You can add my:
> 
> Acked-by: Hans Verkuil 
> 
> to patches 1-9 and 11. See my review for patches 10 and 12. The comments
> are minor, so I hope I can Ack v7 once it's posted and this can be merged
> for 4.17.
> 
> Regards,
> 
>   Hans
> 
Thank you so much for the review.
I will update the same by this week and post.

Regards,
Smitha
> On 08/12/17 10:08, Smitha T Murthy wrote:
> > This patch series adds MFC v10.10 support. MFC v10.10 is used in some
> > of Exynos7 variants.
> > 
> > This adds support for following:
> > 
> > * Add support for HEVC encoder and decoder
> > * Add support for VP9 decoder
> > * Update Documentation for control id definitions
> > * Update computation of min scratch buffer size requirement for V8 onwards
> > 
> > Changes since v5:
> >  - Addressed review comments by Kamil Debski .
> >  - Addressed review comments by 
> >Stanimir Varbanov .
> >  - Addressed review comments by Hans Verkuil .
> >  - Rebased on latest git://linuxtv.org/snawrocki/samsung.git
> >for-v4.15/media/next.
> >  - Applied r-o-b from Andrzej, Stanimir on respective patches.
> >  - Applied acked-by from Kamil, Hans on respective patches.
> > 
> > Smitha T Murthy (12):
> >   [media] s5p-mfc: Rename IS_MFCV8 macro
> >   [media] s5p-mfc: Adding initial support for MFC v10.10
> >   [media] s5p-mfc: Use min scratch buffer size as provided by F/W
> >   [media] s5p-mfc: Support MFCv10.10 buffer requirements
> >   [media] videodev2.h: Add v4l2 definition for HEVC
> >   [media] v4l2-ioctl: add HEVC format description
> >   Documentation: v4l: Documentation for HEVC v4l2 definition
> >   [media] s5p-mfc: Add support for HEVC decoder
> >   [media] s5p-mfc: Add VP9 decoder support
> >   [media] v4l2: Add v4l2 control IDs for HEVC encoder
> >   [media] s5p-mfc: Add support for HEVC encoder
> >   Documention: v4l: Documentation for HEVC CIDs
> > 
> >  .../devicetree/bindings/media/s5p-mfc.txt  |   1 +
> >  Documentation/media/uapi/v4l/extended-controls.rst | 395 +++
> >  Documentation/media/uapi/v4l/pixfmt-compressed.rst |   5 +
> >  drivers/media/platform/s5p-mfc/regs-mfc-v10.h  |  88 
> >  drivers/media/platform/s5p-mfc/regs-mfc-v8.h   |   2 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc.c   |  28 ++
> >  drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c|   9 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_common.h|  68 ++-
> >  drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c  |   6 +-
> >  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c   |  48 +-
> >  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c   | 555 
> > -
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr.h   |  14 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c| 397 +--
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h|  15 +
> >  drivers/media/v4l2-core/v4l2-ctrls.c   | 118 +
> >  drivers/media/v4l2-core/v4l2-ioctl.c   |   1 +
> >  include/uapi/linux/v4l2-controls.h |  92 +++-
> >  include/uapi/linux/videodev2.h |   1 +
> >  18 files changed, 1765 insertions(+), 78 deletions(-)
> >  create mode 100644 drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > 
> 
> 
> 

Re: [Patch v6 10/12] [media] v4l2: Add v4l2 control IDs for HEVC encoder

[Smitha T Murthy]

On Mon, 2018-01-22 at 12:08 +0100, Hans Verkuil wrote:
> On 08/12/17 10:08, Smitha T Murthy wrote:
> > Add v4l2 controls for HEVC encoder
> > 
> > Signed-off-by: Smitha T Murthy 
> > Reviewed-by: Andrzej Hajda 
> > ---
> >  drivers/media/v4l2-core/v4l2-ctrls.c | 118 
> > +++
> >  include/uapi/linux/v4l2-controls.h   |  92 ++-
> >  2 files changed, 209 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> > b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index 4e53a86..3f98318 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -480,6 +480,56 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> > NULL,
> > };
> >  
> > +   static const char * const hevc_profile[] = {
> > +   "Main",
> > +   "Main Still Picture",
> > +   NULL,
> > +   };
> > +   static const char * const hevc_level[] = {
> > +   "1",
> > +   "2",
> > +   "2.1",
> > +   "3",
> > +   "3.1",
> > +   "4",
> > +   "4.1",
> > +   "5",
> > +   "5.1",
> > +   "5.2",
> > +   "6",
> > +   "6.1",
> > +   "6.2",
> > +   NULL,
> > +   };
> > +   static const char * const hevc_hierarchial_coding_type[] = {
> > +   "B",
> > +   "P",
> > +   NULL,
> > +   };
> > +   static const char * const hevc_refresh_type[] = {
> > +   "None",
> > +   "CRA",
> > +   "IDR",
> > +   NULL,
> > +   };
> > +   static const char * const hevc_size_of_length_field[] = {
> > +   "0",
> > +   "1",
> > +   "2",
> > +   "4",
> > +   NULL,
> > +   };
> > +   static const char * const hevc_tier_flag[] = {
> > +   "Main",
> > +   "High",
> > +   NULL,
> > +   };
> > +   static const char * const hevc_loop_filter_mode[] = {
> > +   "Disabled",
> > +   "Enabled",
> > +   "Disabled at slice boundary",
> > +   "NULL",
> > +   };
> >  
> > switch (id) {
> > case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
> > @@ -575,6 +625,20 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> > return dv_it_content_type;
> > case V4L2_CID_DETECT_MD_MODE:
> > return detect_md_mode;
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
> > +   return hevc_profile;
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
> > +   return hevc_level;
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_TYPE:
> > +   return hevc_hierarchial_coding_type;
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_REFRESH_TYPE:
> > +   return hevc_refresh_type;
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_SIZE_OF_LENGTH_FIELD:
> > +   return hevc_size_of_length_field;
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_TIER_FLAG:
> > +   return hevc_tier_flag;
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_LOOP_FILTER_MODE:
> > +   return hevc_loop_filter_mode;
> >  
> > default:
> > return NULL;
> > @@ -776,6 +840,53 @@ const char *v4l2_ctrl_get_name(u32 id)
> > case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP:return "VPX 
> > P-Frame QP Value";
> > case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:   return "VPX 
> > Profile";
> >  
> > +   /* HEVC controls */
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:   return "HEVC 
> > I-Frame QP Value";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP:   return "HEVC 
> > P-Frame QP Value";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_QP:   return "HEVC 
> > B-Frame QP Value";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP:   return "HEVC 
> > Minimum QP Value";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP:   return "HEVC 
> > Maximum QP Value";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:  return "HEVC 
> > Profile";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:return "HEVC 
> > Level";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_TIER_FLAG:return "HEVC 
> > Tier Flag";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_FRAME_RATE_RESOLUTION:return "HEVC 
> > Frame Rate Resolution";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_MAX_PARTITION_DEPTH:  return "HEVC 
> > Maximum Coding Unit Depth";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_REFRESH_TYPE: return "HEVC 
> > Refresh Type";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_CONST_INTRA_PRED: return "HEVC 
> > Constant Intra Prediction";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_LOSSLESS_CU:  return "HEVC 
> > Lossless Encoding";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_WAVEFRONT:return "HEVC 
> > Wavefront";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_LOOP_FILTER_MODE: return "HEVC 
> > Loop Filter";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_HIER_QP:

cron job: media_tree daily build: ERRORS

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Tue Jan 23 05:00:16 CET 2018
media-tree git hash:e3ee691dbf24096ea51b3200946b11d68ce75361
media_build git hash:   2bd1f1623fbadfdc1026712b3d55141ba164c403
v4l-utils git hash: 7eadec47ff4db4a032612349c362f3337b29d485
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0-3911-g6f737e1f
smatch version: v0.5.0-3911-g6f737e1f
host hardware:  x86_64
host os:4.14.0-364

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.8-i686: ERRORS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.16.7-i686: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.18.7-i686: ERRORS
linux-3.19-i686: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.1.33-i686: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.4.22-i686: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.8-i686: ERRORS
linux-4.9.26-i686: ERRORS
linux-4.10.14-i686: WARNINGS
linux-4.11-i686: WARNINGS
linux-4.12.1-i686: WARNINGS
linux-4.13-i686: WARNINGS
linux-4.14-i686: WARNINGS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.7-x86_64: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.7-x86_64: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.33-x86_64: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.22-x86_64: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9.26-x86_64: ERRORS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: WARNINGS
linux-4.14-x86_64: WARNINGS
apps: WARNINGS
spec-git: OK
smatch: OK

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html

[PATCH v4] media: imx258: Add imx258 camera sensor driver

[Andy Yeh]

Add a V4L2 sub-device driver for the Sony IMX258 image sensor.
This is a camera sensor using the I2C bus for control and the
CSI-2 bus for data.

Signed-off-by: Andy Yeh 
---
- v2->v3
-- Update the streaming function to remove SW_STANDBY in the beginning.
-- Adjust the delay time from 1ms to 12ms before set stream-on register.
- v3->v4
-- fix the sd.entity to make code be compiled on the mainline kernel.
-- " - imx258->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;"
-- " + imx258->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;"

 MAINTAINERS|7 +
 drivers/media/i2c/Kconfig  |   11 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/imx258.c | 1148 
 4 files changed, 1167 insertions(+)
 create mode 100644 drivers/media/i2c/imx258.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d76af75..806aa46 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12640,6 +12640,13 @@ S: Maintained
 F: drivers/ssb/
 F: include/linux/ssb/
 
+SONY IMX258 SENSOR DRIVER
+M: Sakari Ailus 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/imx258.c
+
 SONY IMX274 SENSOR DRIVER
 M: Leon Luo 
 L: linux-media@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cb5d7ff..cabde37 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -555,6 +555,17 @@ config VIDEO_APTINA_PLL
 config VIDEO_SMIAPP_PLL
tristate
 
+config VIDEO_IMX258
+   tristate "Sony IMX258 sensor support"
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the Sony
+ IMX258 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called imx258.
+
 config VIDEO_IMX274
tristate "Sony IMX274 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 548a9ef..cf1e0f1 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
 obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
 obj-$(CONFIG_VIDEO_TC358743)   += tc358743.o
+obj-$(CONFIG_VIDEO_IMX258) += imx258.o
 obj-$(CONFIG_VIDEO_IMX274) += imx274.o
 
 obj-$(CONFIG_SDR_MAX2175) += max2175.o
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
new file mode 100644
index 000..a7e58bd2
--- /dev/null
+++ b/drivers/media/i2c/imx258.c
@@ -0,0 +1,1148 @@
+// Copyright (C) 2018 Intel Corporation
+// SPDX-License-Identifier: GPL-2.0
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define IMX258_REG_VALUE_08BIT 1
+#define IMX258_REG_VALUE_16BIT 2
+#define IMX258_REG_VALUE_24BIT 3
+
+#define IMX258_REG_MODE_SELECT 0x0100
+#define IMX258_MODE_STANDBY0x00
+#define IMX258_MODE_STREAMING  0x01
+
+/* Chip ID */
+#define IMX258_REG_CHIP_ID 0x0016
+#define IMX258_CHIP_ID 0x0258
+
+/* V_TIMING internal */
+#define IMX258_REG_VTS 0x0340
+#define IMX258_VTS_30FPS   0x0c98
+#define IMX258_VTS_MAX 0x7fff
+
+/* HBLANK control - read only */
+#define IMX258_PPL_650MHZ  5352
+#define IMX258_PPL_325MHZ  2676
+
+/* Exposure control */
+#define IMX258_REG_EXPOSURE0x0202
+#define IMX258_EXPOSURE_MIN4
+#define IMX258_EXPOSURE_STEP   1
+#define IMX258_EXPOSURE_DEFAULT0x640
+
+/* Analog gain control */
+#define IMX258_REG_ANALOG_GAIN 0x0204
+#define IMX258_ANA_GAIN_MIN0
+#define IMX258_ANA_GAIN_MAX0x1fff
+#define IMX258_ANA_GAIN_STEP   1
+#define IMX258_ANA_GAIN_DEFAULT0x0
+
+/* Orientation */
+#define REG_MIRROR_FLIP_CONTROL0x0101
+#define REG_CONFIG_MIRROR_FLIP 0x03
+
+struct imx258_reg {
+   u16 address;
+   u8 val;
+};
+
+struct imx258_reg_list {
+   u32 num_of_regs;
+   const struct imx258_reg *regs;
+};
+
+/* Link frequency config */
+struct imx258_link_freq_config {
+   u32 pixels_per_line;
+
+   /* PLL registers for this link frequency */
+   struct imx258_reg_list reg_list;
+};
+
+/* Mode : resolution and related config */
+struct imx258_mode {
+   /* Frame width */
+   u32 width;
+   /* Frame height */
+   u32 height;
+
+   /* V-timing */
+   u32 vts_def;
+   u32 vts_min;
+
+   /* Index of Link frequency config to be used */
+   u32 link_freq_index;
+   /* Default register values */
+   struct imx258_reg_list reg_list;
+};
+
+/* 4208x3118 needs 1300Mbps/lane, 4 lanes */
+static 

[PATCH v3 1/1] imx258: Fix sparse warnings

[Sakari Ailus]

Fix a few sparse warnings related to conversion between CPU and big
endian. Also simplify the code in the process.

Signed-off-by: Sakari Ailus 
---
since v2:

- Count loop downwards, not up.

 drivers/media/i2c/imx258.c | 23 +--
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index a7e58bd23de7..213429cca8b5 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -440,10 +440,10 @@ static int imx258_read_reg(struct imx258 *imx258, u16 
reg, u32 len, u32 *val)
 {
struct i2c_client *client = v4l2_get_subdevdata(>sd);
struct i2c_msg msgs[2];
+   __be16 reg_addr_be = cpu_to_be16(reg);
+   __be32 data_be = 0;
u8 *data_be_p;
int ret;
-   u32 data_be = 0;
-   u16 reg_addr_be = cpu_to_be16(reg);
 
if (len > 4)
return -EINVAL;
@@ -474,24 +474,19 @@ static int imx258_read_reg(struct imx258 *imx258, u16 
reg, u32 len, u32 *val)
 static int imx258_write_reg(struct imx258 *imx258, u16 reg, u32 len, u32 val)
 {
struct i2c_client *client = v4l2_get_subdevdata(>sd);
-   int buf_i, val_i;
-   u8 buf[6], *val_p;
+   u8 __buf[6], *buf = __buf;
+   int i;
 
if (len > 4)
return -EINVAL;
 
-   buf[0] = reg >> 8;
-   buf[1] = reg & 0xff;
+   *buf++ = reg >> 8;
+   *buf++ = reg & 0xff;
 
-   val = cpu_to_be32(val);
-   val_p = (u8 *)
-   buf_i = 2;
-   val_i = 4 - len;
+   for (i = len - 1; i >= 0; i--)
+   *buf++ = (u8)(val >> (i << 3));
 
-   while (val_i < 4)
-   buf[buf_i++] = val_p[val_i++];
-
-   if (i2c_master_send(client, buf, len + 2) != len + 2)
+   if (i2c_master_send(client, __buf, len + 2) != len + 2)
return -EIO;
 
return 0;
-- 
2.11.0

Re: [PATCH v2 1/1] imx258: Fix sparse warnings

[Sakari Ailus]

Hi Andy,

On Mon, Jan 22, 2018 at 03:19:00PM +, Yeh, Andy wrote:
> Hi Sakari,
> 
> I made a minor fix. I2C write function works after the change.  Please kindly 
> review soon then I would submit v5. 
> 
>   *buf++ = reg >> 8;
>   *buf++ = reg & 0xff;
> 
> - for (i = len - 1; i >= 0; i++)
> + for (i = len - 1; i >= 0; i--)

Oops... it was untested all along. Thanks! I'll send v3.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [PATCH] [media] s3c-camif: array underflow in __camif_subdev_try_format()

[Sylwester Nawrocki]

On 01/22/2018 11:37 AM, Dan Carpenter wrote:
> The while loop is a post op, "while (i-- >= 0)" so the last iteration
> will read camif_mbus_formats[-1] and then the loop will exit with "i"
> set to -2 and so we do: "mf->code = camif_mbus_formats[-2];".
> 
> I've changed it to a pre-op, I've added a check to ensure we found the
> right format and I've removed the "mf->code = camif_mbus_formats[i];"
> because that is a no-op anyway.
> 
> Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series 
> camera interface")
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
> b/drivers/media/platform/s3c-camif/camif-capture.c
> index 437395a61065..012f4b389c55 100644
> --- a/drivers/media/platform/s3c-camif/camif-capture.c
> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> @@ -1261,11 +1261,11 @@ static void __camif_subdev_try_format(struct 
> camif_dev *camif,
>   /* FIXME: constraints against codec or preview path ? */
>   pix_lim = >vp_pix_limits[VP_CODEC];
>   
> - while (i-- >= 0)
> + while (--i >= 0)
>   if (camif_mbus_formats[i] == mf->code)
>   break;
> -
> - mf->code = camif_mbus_formats[i];
> + if (i < 0)
> + return;

Thanks for the patch Dan. mf->width needs to be aligned by this try_format
function so we shouldn't return here. Also it needs to be ensured mf->code 
is set to one of the supported values when this function returns. Sorry,
the current code really doesn't give a clue what was intended.

There is already queued a patch from Arnd [1] addressing the issues you 
have found.
 
>   if (pad == CAMIF_SD_PAD_SINK) {
>   v4l_bound_align_image(>width, 8, CAMIF_MAX_PIX_WIDTH,
> 

[1] https://patchwork.linuxtv.org/patch/46508

--
Regards,
Sylwester

Darlehen

[defina]

Benötigen Sie Privat- oder Geschäftskredite ohne Stress und schnelle 
Zustimmung? Wenn ja, kontaktieren Sie uns bitte alexgr...@gmail.com

[PATCH v2 3/5] media: dvb-frontends/stv0910: report FEC 1/4 and 1/3 in get_frontend()

[Daniel Scheller]

From: Daniel Scheller 

The first two missing FECs in the modcod2fec fe_code_rate table were 1/4
and 1/3. Add them as they're now defined by the API.

Signed-off-by: Daniel Scheller 
---
 drivers/media/dvb-frontends/stv0910.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/stv0910.c 
b/drivers/media/dvb-frontends/stv0910.c
index a2f7c0c1587f..de132a85e537 100644
--- a/drivers/media/dvb-frontends/stv0910.c
+++ b/drivers/media/dvb-frontends/stv0910.c
@@ -1562,7 +1562,7 @@ static int get_frontend(struct dvb_frontend *fe,
APSK_32,
};
const enum fe_code_rate modcod2fec[0x20] = {
-   FEC_NONE, FEC_NONE, FEC_NONE, FEC_2_5,
+   FEC_NONE, FEC_1_4, FEC_1_3, FEC_2_5,
FEC_1_2, FEC_3_5, FEC_2_3, FEC_3_4,
FEC_4_5, FEC_5_6, FEC_8_9, FEC_9_10,
FEC_3_5, FEC_2_3, FEC_3_4, FEC_5_6,
-- 
2.13.6

[PATCH v2 5/5] media: dvb-frontends/stv0910: report active delsys in get_frontend()

[Daniel Scheller]

From: Daniel Scheller 

Report the active delivery system based on the receive_mode state of the
demodulator.

Suggested-by: Richard Scobie 
Signed-off-by: Daniel Scheller 
---
 drivers/media/dvb-frontends/stv0910.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/dvb-frontends/stv0910.c 
b/drivers/media/dvb-frontends/stv0910.c
index 7ab014cec56c..6e6a70ad7354 100644
--- a/drivers/media/dvb-frontends/stv0910.c
+++ b/drivers/media/dvb-frontends/stv0910.c
@@ -1580,6 +1580,7 @@ static int get_frontend(struct dvb_frontend *fe,
p->modulation = modcod2mod[mc];
p->fec_inner = modcod2fec[mc];
p->rolloff = ro2ro[state->fe_rolloff];
+   p->delivery_system = SYS_DVBS2;
} else if (state->receive_mode == RCVMODE_DVBS) {
read_reg(state, RSTV0910_P2_VITCURPUN + state->regoff, );
switch (tmp & 0x1F) {
@@ -1603,6 +1604,7 @@ static int get_frontend(struct dvb_frontend *fe,
break;
}
p->rolloff = ROLLOFF_35;
+   p->delivery_system = SYS_DVBS;
}
 
if (state->receive_mode != RCVMODE_NONE) {
-- 
2.13.6

[PATCH v2 4/5] media: dvb-frontends/stv0910: report S2 rolloff in get_frontend()

[Daniel Scheller]

From: Daniel Scheller 

Report the currently used  S2 rolloff factor in get_frontend(). For
cosmetic reasons, also change all feroll_off occurences to fe_rolloff.

Signed-off-by: Daniel Scheller 
---
 drivers/media/dvb-frontends/stv0910.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-frontends/stv0910.c 
b/drivers/media/dvb-frontends/stv0910.c
index de132a85e537..7ab014cec56c 100644
--- a/drivers/media/dvb-frontends/stv0910.c
+++ b/drivers/media/dvb-frontends/stv0910.c
@@ -113,7 +113,7 @@ struct stv {
enum fe_stv0910_mod_cod  mod_cod;
enum dvbs2_fectype   fectype;
u32  pilots;
-   enum fe_stv0910_roll_off feroll_off;
+   enum fe_stv0910_roll_off fe_rolloff;
 
int   is_standard_broadcast;
int   is_vcm;
@@ -541,7 +541,7 @@ static int get_signal_parameters(struct stv *state)
}
state->is_vcm = 0;
state->is_standard_broadcast = 1;
-   state->feroll_off = FE_SAT_35;
+   state->fe_rolloff = FE_SAT_35;
}
return 0;
 }
@@ -1300,14 +1300,14 @@ static int manage_matype_info(struct stv *state)
 
read_regs(state, RSTV0910_P2_MATSTR1 + state->regoff,
  bbheader, 2);
-   state->feroll_off =
+   state->fe_rolloff =
(enum fe_stv0910_roll_off)(bbheader[0] & 0x03);
state->is_vcm = (bbheader[0] & 0x10) == 0;
state->is_standard_broadcast = (bbheader[0] & 0xFC) == 0xF0;
} else if (state->receive_mode == RCVMODE_DVBS) {
state->is_vcm = 0;
state->is_standard_broadcast = 1;
-   state->feroll_off = FE_SAT_35;
+   state->fe_rolloff = FE_SAT_35;
}
return 0;
 }
@@ -1571,11 +1571,15 @@ static int get_frontend(struct dvb_frontend *fe,
FEC_3_4, FEC_4_5, FEC_5_6, FEC_8_9,
FEC_9_10
};
+   const enum fe_rolloff ro2ro[4] = {
+   ROLLOFF_35, ROLLOFF_25, ROLLOFF_20, ROLLOFF_15,
+   };
read_reg(state, RSTV0910_P2_DMDMODCOD + state->regoff, );
mc = ((tmp & 0x7c) >> 2);
p->pilot = (tmp & 0x01) ? PILOT_ON : PILOT_OFF;
p->modulation = modcod2mod[mc];
p->fec_inner = modcod2fec[mc];
+   p->rolloff = ro2ro[state->fe_rolloff];
} else if (state->receive_mode == RCVMODE_DVBS) {
read_reg(state, RSTV0910_P2_VITCURPUN + state->regoff, );
switch (tmp & 0x1F) {
-- 
2.13.6

[PATCH v2 2/5] media: dvb_frontend: add DVB-S2X rolloff factors

[Daniel Scheller]

From: Daniel Scheller 

Add 15%, 10% and 5% DVB-S2X rolloff factors. Also fix roloff typos.

Signed-off-by: Daniel Scheller 
---
 Documentation/media/frontend.h.rst.exceptions |  3 +++
 drivers/media/dvb-core/dvb_frontend.c |  9 +
 include/uapi/linux/dvb/frontend.h | 16 +++-
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/Documentation/media/frontend.h.rst.exceptions 
b/Documentation/media/frontend.h.rst.exceptions
index ae1148be0a39..c1643ce93426 100644
--- a/Documentation/media/frontend.h.rst.exceptions
+++ b/Documentation/media/frontend.h.rst.exceptions
@@ -167,6 +167,9 @@ ignore symbol ROLLOFF_35
 ignore symbol ROLLOFF_20
 ignore symbol ROLLOFF_25
 ignore symbol ROLLOFF_AUTO
+ignore symbol ROLLOFF_15
+ignore symbol ROLLOFF_10
+ignore symbol ROLLOFF_5
 
 ignore symbol INVERSION_ON
 ignore symbol INVERSION_OFF
diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index 722b86a43497..e5105c1783b8 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2183,6 +2183,15 @@ static int dtv_set_frontend(struct dvb_frontend *fe)
break;
case SYS_DVBS2:
switch (c->rolloff) {
+   case ROLLOFF_5:
+   rolloff = 105;
+   break;
+   case ROLLOFF_10:
+   rolloff = 110;
+   break;
+   case ROLLOFF_15:
+   rolloff = 115;
+   break;
case ROLLOFF_20:
rolloff = 120;
break;
diff --git a/include/uapi/linux/dvb/frontend.h 
b/include/uapi/linux/dvb/frontend.h
index 227268a657cd..8bf1c63627a2 100644
--- a/include/uapi/linux/dvb/frontend.h
+++ b/include/uapi/linux/dvb/frontend.h
@@ -580,20 +580,26 @@ enum fe_pilot {
 
 /**
  * enum fe_rolloff - Rolloff factor
- * @ROLLOFF_35:Roloff factor: α=35%
- * @ROLLOFF_20:Roloff factor: α=20%
- * @ROLLOFF_25:Roloff factor: α=25%
- * @ROLLOFF_AUTO:  Auto-detect the roloff factor.
+ * @ROLLOFF_35:Rolloff factor: α=35%
+ * @ROLLOFF_20:Rolloff factor: α=20%
+ * @ROLLOFF_25:Rolloff factor: α=25%
+ * @ROLLOFF_AUTO:  Auto-detect the rolloff factor.
+ * @ROLLOFF_15:Rolloff factor: α=15%
+ * @ROLLOFF_10:Rolloff factor: α=10%
+ * @ROLLOFF_5: Rolloff factor: α=5%
  *
  * .. note:
  *
- *Roloff factor of 35% is implied on DVB-S. On DVB-S2, it is default.
+ *Rolloff factor of 35% is implied on DVB-S. On DVB-S2, it is default.
  */
 enum fe_rolloff {
ROLLOFF_35,
ROLLOFF_20,
ROLLOFF_25,
ROLLOFF_AUTO,
+   ROLLOFF_15,
+   ROLLOFF_10,
+   ROLLOFF_5,
 };
 
 /**
-- 
2.13.6

[PATCH v2 1/5] media: dvb_frontend: add FEC modes, S2X modulations and 64K transmission

[Daniel Scheller]

From: Daniel Scheller 

Add 1/4 and 1/3 FEC ratios, 64/128/256-APSK S2X modulations and 64K
transmission mode. Update relevant doc items aswell.

Signed-off-by: Daniel Scheller 
---
 Documentation/media/frontend.h.rst.exceptions |  6 ++
 include/uapi/linux/dvb/frontend.h | 13 +
 2 files changed, 19 insertions(+)

diff --git a/Documentation/media/frontend.h.rst.exceptions 
b/Documentation/media/frontend.h.rst.exceptions
index f7c4df620a52..ae1148be0a39 100644
--- a/Documentation/media/frontend.h.rst.exceptions
+++ b/Documentation/media/frontend.h.rst.exceptions
@@ -84,6 +84,9 @@ ignore symbol APSK_16
 ignore symbol APSK_32
 ignore symbol DQPSK
 ignore symbol QAM_4_NR
+ignore symbol APSK_64
+ignore symbol APSK_128
+ignore symbol APSK_256
 
 ignore symbol SEC_VOLTAGE_13
 ignore symbol SEC_VOLTAGE_18
@@ -117,6 +120,8 @@ ignore symbol FEC_AUTO
 ignore symbol FEC_3_5
 ignore symbol FEC_9_10
 ignore symbol FEC_2_5
+ignore symbol FEC_1_4
+ignore symbol FEC_1_3
 
 ignore symbol TRANSMISSION_MODE_AUTO
 ignore symbol TRANSMISSION_MODE_1K
@@ -129,6 +134,7 @@ ignore symbol TRANSMISSION_MODE_C1
 ignore symbol TRANSMISSION_MODE_C3780
 ignore symbol TRANSMISSION_MODE_2K
 ignore symbol TRANSMISSION_MODE_8K
+ignore symbol TRANSMISSION_MODE_64K
 
 ignore symbol GUARD_INTERVAL_AUTO
 ignore symbol GUARD_INTERVAL_1_128
diff --git a/include/uapi/linux/dvb/frontend.h 
b/include/uapi/linux/dvb/frontend.h
index 4f9b4551c534..227268a657cd 100644
--- a/include/uapi/linux/dvb/frontend.h
+++ b/include/uapi/linux/dvb/frontend.h
@@ -296,6 +296,8 @@ enum fe_spectral_inversion {
  * @FEC_3_5:  Forward Error Correction Code 3/5
  * @FEC_9_10: Forward Error Correction Code 9/10
  * @FEC_2_5:  Forward Error Correction Code 2/5
+ * @FEC_1_4:  Forward Error Correction Code 1/4
+ * @FEC_1_3:  Forward Error Correction Code 1/3
  *
  * Please note that not all FEC types are supported by a given standard.
  */
@@ -313,6 +315,8 @@ enum fe_code_rate {
FEC_3_5,
FEC_9_10,
FEC_2_5,
+   FEC_1_4,
+   FEC_1_3,
 };
 
 /**
@@ -331,6 +335,9 @@ enum fe_code_rate {
  * @APSK_32:   32-APSK modulation
  * @DQPSK: DQPSK modulation
  * @QAM_4_NR:  4-QAM-NR modulation
+ * @APSK_64:   64-APSK modulation
+ * @APSK_128:  128-APSK modulation
+ * @APSK_256:  256-APSK modulation
  *
  * Please note that not all modulations are supported by a given standard.
  *
@@ -350,6 +357,9 @@ enum fe_modulation {
APSK_32,
DQPSK,
QAM_4_NR,
+   APSK_64,
+   APSK_128,
+   APSK_256,
 };
 
 /**
@@ -374,6 +384,8 @@ enum fe_modulation {
  * Single Carrier (C=1) transmission mode (DTMB only)
  * @TRANSMISSION_MODE_C3780:
  * Multi Carrier (C=3780) transmission mode (DTMB only)
+ * @TRANSMISSION_MODE_64K:
+ * Transmission mode 64K
  *
  * Please note that not all transmission modes are supported by a given
  * standard.
@@ -388,6 +400,7 @@ enum fe_transmit_mode {
TRANSMISSION_MODE_32K,
TRANSMISSION_MODE_C1,
TRANSMISSION_MODE_C3780,
+   TRANSMISSION_MODE_64K,
 };
 
 /**
-- 
2.13.6

[PATCH v2 0/5] Add FEC rates, S2X params and 64K transmission

[Daniel Scheller]

From: Daniel Scheller 

dddvb brings a few additional FEC rates (1/4 and 1/3), 64/128/256APSK
modulations and more rolloff factors (DVB-S2X), and 64K transmission mode
(DVB-T2). These rather trivial patches bring them to mainline, and puts
these missing bits into the stv0910's get_frontend() callback (FEC 1/4
and 1/3 are handled throughout the rest of the demod driver already). In
addition (as suggestion from Richard Scobie), the stv0910 driver now
reports it's active delivery system.

Changes from v1 to v2:
- DVB-S2X rolloff factors and reporting
- report of the active delivery system in stv0910:get_frontend()

Daniel Scheller (5):
  media: dvb_frontend: add FEC modes, S2X modulations and 64K
transmission
  media: dvb_frontend: add DVB-S2X rolloff factors
  media: dvb-frontends/stv0910: report FEC 1/4 and 1/3 in get_frontend()
  media: dvb-frontends/stv0910: report S2 rolloff in get_frontend()
  media: dvb-frontends/stv0910: report active delsys in get_frontend()

 Documentation/media/frontend.h.rst.exceptions |  9 +
 drivers/media/dvb-core/dvb_frontend.c |  9 +
 drivers/media/dvb-frontends/stv0910.c | 16 ++-
 include/uapi/linux/dvb/frontend.h | 29 ++-
 4 files changed, 53 insertions(+), 10 deletions(-)

-- 
2.13.6

[PATCH v2] media: dw9807: Add dw9807 vcm driver

[Andy Yeh]

From: Alan Chiang 

DW9807 is a 10 bit DAC from Dongwoon, designed for linear
control of voice coil motor.

This driver creates a V4L2 subdevice and
provides control to set the desired focus.

Signed-off-by: Andy Yeh 
Signed-off-by: Sakari Ailus 
---
v2: changed author

 .../bindings/media/i2c/dongwoon,dw9807.txt |   9 +
 MAINTAINERS|   7 +
 drivers/media/i2c/Kconfig  |  10 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/dw9807.c | 387 +
 5 files changed, 414 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
 create mode 100644 drivers/media/i2c/dw9807.c

diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt 
b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
new file mode 100644
index 000..1771cd0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
@@ -0,0 +1,9 @@
+Dngwoon Anatech DW9807 voice coil lens driver
+
+DW9807 is a 10-bit DAC with current sink capability. It is intended for
+controlling voice coil lenses.
+
+Mandatory properties:
+
+- compatible: "dongwoon,dw9807"
+- reg: I2C slave address
diff --git a/MAINTAINERS b/MAINTAINERS
index 9c9db44..32b536b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4377,6 +4377,13 @@ T:   git git://linuxtv.org/media_tree.git
 S: Maintained
 F: drivers/media/i2c/dw9714.c
 
+DONGWOON DW9807 LENS VOICE COIL DRIVER
+M: Sakari Ailus 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/dw9807.c
+
 DOUBLETALK DRIVER
 M: "James R. Van Zandt" 
 L: blinux-l...@redhat.com
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cabde37..bcd4bf1 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -325,6 +325,16 @@ config VIDEO_DW9714
  capability. This is designed for linear control of
  voice coil motors, controlled via I2C serial interface.
 
+config VIDEO_DW9807
+   tristate "DW9807 lens voice coil support"
+   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on VIDEO_V4L2_SUBDEV_API
+   ---help---
+ This is a driver for the DW9807 camera lens voice coil.
+ DW9807 is a 10 bit DAC with 100mA output current sink
+ capability. This is designed for linear control of
+ voice coil motors, controlled via I2C serial interface.
+
 config VIDEO_SAA7110
tristate "Philips SAA7110 video decoder"
depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index cf1e0f1..4bf7d00 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
 obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
 obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
 obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
+obj-$(CONFIG_VIDEO_DW9807)  += dw9807.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
 obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o
diff --git a/drivers/media/i2c/dw9807.c b/drivers/media/i2c/dw9807.c
new file mode 100644
index 000..f068771
--- /dev/null
+++ b/drivers/media/i2c/dw9807.c
@@ -0,0 +1,387 @@
+/*
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DW9807_NAME"dw9807"
+#define DW9807_MAX_FOCUS_POS   1023
+/*
+ * This sets the minimum granularity for the focus positions.
+ * A value of 1 gives maximum accuracy for a desired focus position
+ */
+#define DW9807_FOCUS_STEPS 1
+/*
+ * This acts as the minimum granularity of lens movement.
+ * Keep this value power of 2, so the control steps can be
+ * uniformly adjusted for gradual lens movement, with desired
+ * number of control steps.
+ */
+#define DW9807_CTRL_STEPS  16
+#define DW9807_CTRL_DELAY_US   1000
+
+/*
+ * DW9807 separates two registers to control the VCM position.
+ * One for MSB value, another is LSB value.
+ */
+#define DW9807_CTL_ADDR 0x02
+#define DW9807_MSB_ADDR 0x03
+#define DW9807_LSB_ADDR 0x04
+#define DW9807_STATUS_ADDR 0x05
+#define DW9807_MODE_ADDR 0x06
+#define DW9807_RESONANCE_ADDR 0x07
+
+#define MAX_RETRY 10
+
+/* dw9807 device structure */

[PATCH] media: dw9807: Add dw9807 vcm driver

[Andy Yeh]

DW9807 is a 10 bit DAC from Dongwoon, designed for linear
control of voice coil motor.

This driver creates a V4L2 subdevice and
provides control to set the desired focus.

Signed-off-by: Andy Yeh 
Signed-off-by: Sakari Ailus 
---
 .../bindings/media/i2c/dongwoon,dw9807.txt |   9 +
 MAINTAINERS|   7 +
 drivers/media/i2c/Kconfig  |  10 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/dw9807.c | 387 +
 5 files changed, 414 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
 create mode 100644 drivers/media/i2c/dw9807.c

diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt 
b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
new file mode 100644
index 000..1771cd0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
@@ -0,0 +1,9 @@
+Dngwoon Anatech DW9807 voice coil lens driver
+
+DW9807 is a 10-bit DAC with current sink capability. It is intended for
+controlling voice coil lenses.
+
+Mandatory properties:
+
+- compatible: "dongwoon,dw9807"
+- reg: I2C slave address
diff --git a/MAINTAINERS b/MAINTAINERS
index 9c9db44..32b536b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4377,6 +4377,13 @@ T:   git git://linuxtv.org/media_tree.git
 S: Maintained
 F: drivers/media/i2c/dw9714.c
 
+DONGWOON DW9807 LENS VOICE COIL DRIVER
+M: Sakari Ailus 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/dw9807.c
+
 DOUBLETALK DRIVER
 M: "James R. Van Zandt" 
 L: blinux-l...@redhat.com
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cabde37..bcd4bf1 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -325,6 +325,16 @@ config VIDEO_DW9714
  capability. This is designed for linear control of
  voice coil motors, controlled via I2C serial interface.
 
+config VIDEO_DW9807
+   tristate "DW9807 lens voice coil support"
+   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on VIDEO_V4L2_SUBDEV_API
+   ---help---
+ This is a driver for the DW9807 camera lens voice coil.
+ DW9807 is a 10 bit DAC with 100mA output current sink
+ capability. This is designed for linear control of
+ voice coil motors, controlled via I2C serial interface.
+
 config VIDEO_SAA7110
tristate "Philips SAA7110 video decoder"
depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index cf1e0f1..4bf7d00 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
 obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
 obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
 obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
+obj-$(CONFIG_VIDEO_DW9807)  += dw9807.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
 obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o
diff --git a/drivers/media/i2c/dw9807.c b/drivers/media/i2c/dw9807.c
new file mode 100644
index 000..f068771
--- /dev/null
+++ b/drivers/media/i2c/dw9807.c
@@ -0,0 +1,387 @@
+/*
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DW9807_NAME"dw9807"
+#define DW9807_MAX_FOCUS_POS   1023
+/*
+ * This sets the minimum granularity for the focus positions.
+ * A value of 1 gives maximum accuracy for a desired focus position
+ */
+#define DW9807_FOCUS_STEPS 1
+/*
+ * This acts as the minimum granularity of lens movement.
+ * Keep this value power of 2, so the control steps can be
+ * uniformly adjusted for gradual lens movement, with desired
+ * number of control steps.
+ */
+#define DW9807_CTRL_STEPS  16
+#define DW9807_CTRL_DELAY_US   1000
+
+/*
+ * DW9807 separates two registers to control the VCM position.
+ * One for MSB value, another is LSB value.
+ */
+#define DW9807_CTL_ADDR 0x02
+#define DW9807_MSB_ADDR 0x03
+#define DW9807_LSB_ADDR 0x04
+#define DW9807_STATUS_ADDR 0x05
+#define DW9807_MODE_ADDR 0x06
+#define DW9807_RESONANCE_ADDR 0x07
+
+#define MAX_RETRY 10
+
+/* dw9807 device structure */
+struct dw9807_device {
+   struct i2c_client *client;
+ 

Re: [PATCH] media: ov5640: add JPEG support

[Hugues FRUCHET]

Hi Sakari,
Thanks for reviewing,

On 01/22/2018 01:52 PM, Sakari Ailus wrote:
> Hi Hugues,
> 
> On Mon, Jan 22, 2018 at 11:46:36AM +0100, Hugues Fruchet wrote:
>> Add YUV422 encoded JPEG support.
>>
>> Signed-off-by: Hugues Fruchet 
>> ---
>>   drivers/media/i2c/ov5640.c | 82 
>> --
>>   1 file changed, 80 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
>> index e2dd352..db9aeeb 100644
>> --- a/drivers/media/i2c/ov5640.c
>> +++ b/drivers/media/i2c/ov5640.c
>> @@ -18,6 +18,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>>   #include 
>>   #include 
>>   #include 
>> @@ -34,6 +35,10 @@
>>   
>>   #define OV5640_DEFAULT_SLAVE_ID 0x3c
>>   
>> +#define OV5640_JPEG_SIZE_MAX (5 * SZ_1M)
>> +
>> +#define OV5640_REG_SYS_RESET02  0x3002
>> +#define OV5640_REG_SYS_CLOCK_ENABLE02   0x3006
>>   #define OV5640_REG_SYS_CTRL0   0x3008
>>   #define OV5640_REG_CHIP_ID 0x300a
>>   #define OV5640_REG_IO_MIPI_CTRL00  0x300e
>> @@ -114,6 +119,7 @@ struct ov5640_pixfmt {
>>   };
>>   
>>   static const struct ov5640_pixfmt ov5640_formats[] = {
>> +{ MEDIA_BUS_FMT_JPEG_1X8, V4L2_COLORSPACE_JPEG, },
>>  { MEDIA_BUS_FMT_UYVY8_2X8, V4L2_COLORSPACE_SRGB, },
>>  { MEDIA_BUS_FMT_YUYV8_2X8, V4L2_COLORSPACE_SRGB, },
>>  { MEDIA_BUS_FMT_RGB565_2X8_LE, V4L2_COLORSPACE_SRGB, },
>> @@ -220,6 +226,8 @@ struct ov5640_dev {
>>   
>>  bool pending_mode_change;
>>  bool streaming;
>> +
>> +unsigned int jpeg_size;
>>   };
>>   
>>   static inline struct ov5640_dev *to_ov5640_dev(struct v4l2_subdev *sd)
>> @@ -1910,11 +1918,51 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
>>  return ret;
>>   }
>>   
>> +static int ov5640_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>> + struct v4l2_mbus_frame_desc *fd)
>> +{
>> +struct ov5640_dev *sensor = to_ov5640_dev(sd);
>> +
>> +if (pad != 0 || !fd)
>> +return -EINVAL;
>> +
>> +mutex_lock(>lock);
>> +fd->entry[0].length = sensor->jpeg_size;
>> +fd->entry[0].pixelcode = MEDIA_BUS_FMT_JPEG_1X8;
> 
> This doesn't need to be serialised i.e. can be moved below where the flags
> are assigned.
> 

It was for "sensor->jpeg_size" protection, should be:

mutex_lock(>lock);
fd->entry[0].length = sensor->jpeg_size;
mutex_unlock(>lock);


>> +mutex_unlock(>lock);
>> +
>> +fd->entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
>> +fd->num_entries = 1;
>> +
>> +return 0;
>> +}
>> +
>> +static int ov5640_set_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>> + struct v4l2_mbus_frame_desc *fd)
>> +{
>> +struct ov5640_dev *sensor = to_ov5640_dev(sd);
>> +
>> +if (pad != 0 || !fd)
>> +return -EINVAL;
>> +
>> +fd->entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
>> +fd->num_entries = 1;
>> +fd->entry[0].length = clamp_t(u32, fd->entry[0].length,
>> +  sensor->fmt.width * sensor->fmt.height,
>> +  OV5640_JPEG_SIZE_MAX);
> 
> Access to sensor->fmt.width and .height needs to be serialised; acquire
> mutex first?

Yes, should be:

mutex_lock(>lock);
fd->entry[0].length = clamp_t(u32, fd->entry[0].length,
  sensor->fmt.width * sensor->fmt.height,
  OV5640_JPEG_SIZE_MAX);
sensor->jpeg_size = fd->entry[0].length;
mutex_unlock(>lock);

> 
>> +mutex_lock(>lock);
>> +sensor->jpeg_size = fd->entry[0].length;
>> +mutex_unlock(>lock);
>> +
>> +return 0;
>> +}
>> +
>>   static int ov5640_set_framefmt(struct ov5640_dev *sensor,
>> struct v4l2_mbus_framefmt *format)
>>   {
>>  int ret = 0;
>>  bool is_rgb = false;
>> +bool is_jpeg = false;
>>  u8 val;
>>   
>>  switch (format->code) {
>> @@ -1936,6 +1984,11 @@ static int ov5640_set_framefmt(struct ov5640_dev 
>> *sensor,
>>  val = 0x61;
>>  is_rgb = true;
>>  break;
>> +case MEDIA_BUS_FMT_JPEG_1X8:
>> +/* YUV422, YUYV */
>> +val = 0x30;
>> +is_jpeg = true;
>> +break;
>>  default:
>>  return -EINVAL;
>>  }
>> @@ -1946,8 +1999,31 @@ static int ov5640_set_framefmt(struct ov5640_dev 
>> *sensor,
>>  return ret;
>>   
>>  /* FORMAT MUX CONTROL: ISP YUV or RGB */
>> -return ov5640_write_reg(sensor, OV5640_REG_ISP_FORMAT_MUX_CTRL,
>> -is_rgb ? 0x01 : 0x00);
>> +ret = ov5640_write_reg(sensor, OV5640_REG_ISP_FORMAT_MUX_CTRL,
>> +   is_rgb ? 0x01 : 0x00);
>> +if (ret)
>> +return ret;
>> +
>> +if (is_jpeg) {
>> +/* Enable jpeg */
>> +ret = ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
>> + BIT(5), 

[PATCH] media: imx: add 8-bit grayscale support

[Philipp Zabel]

The IPUv3 code has 8-bit grayscale capture support.
Enable imx-media to use it.

Signed-off-by: Philipp Zabel 
---
This patch depends on https://patchwork.kernel.org/patch/10178777/
to work, otherwise STREAMON will fail with -EINVAL.
---
 drivers/staging/media/imx/imx-media-csi.c   | 1 +
 drivers/staging/media/imx/imx-media-utils.c | 8 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index eb7be5093a9d5..e280ba31262a8 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -400,6 +400,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
case V4L2_PIX_FMT_SGBRG8:
case V4L2_PIX_FMT_SGRBG8:
case V4L2_PIX_FMT_SRGGB8:
+   case V4L2_PIX_FMT_GREY:
burst_size = 16;
passthrough = true;
passthrough_bits = 8;
diff --git a/drivers/staging/media/imx/imx-media-utils.c 
b/drivers/staging/media/imx/imx-media-utils.c
index 13dafa77a2eba..5f61eecb81f1e 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -93,7 +93,7 @@ static const struct imx_media_pixfmt rgb_formats[] = {
.bpp= 32,
.ipufmt = true,
},
-   /*** raw bayer formats start here ***/
+   /*** raw bayer and grayscale formats start here ***/
{
.fourcc = V4L2_PIX_FMT_SBGGR8,
.codes  = {MEDIA_BUS_FMT_SBGGR8_1X8},
@@ -162,6 +162,12 @@ static const struct imx_media_pixfmt rgb_formats[] = {
.cs = IPUV3_COLORSPACE_RGB,
.bpp= 16,
.bayer  = true,
+   }, {
+   .fourcc = V4L2_PIX_FMT_GREY,
+   .codes = {MEDIA_BUS_FMT_Y8_1X8},
+   .cs = IPUV3_COLORSPACE_RGB,
+   .bpp= 8,
+   .bayer  = true,
},
/***
 * non-mbus RGB formats start here. NOTE! when adding non-mbus
-- 
2.11.0

[PATCH] media: imx258: Add imx258 camera sensor driver

[Andy Yeh]

Add a V4L2 sub-device driver for the Sony IMX258 image sensor.
This is a camera sensor using the I2C bus for control and the
CSI-2 bus for data.

Signed-off-by: Andy Yeh 
Signed-off-by: Sakari Ailus 

---
v3:
-- Update the streaming function to remove SW_STANDBY in the beginning.
-- Adjust the delay time from 1ms to 12ms before set stream-on register.
v4:
-- fix the sd.entity to make code be compiled on the mainline kernel.
-- " - imx258->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;"
-- " + imx258->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;"
v5:
-- Fix a few sparse warnings related to conversion between CPU and big
-- endian. Also simplify the code in the process.

 MAINTAINERS|7 +
 drivers/media/i2c/Kconfig  |   11 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/imx258.c | 1144 
 4 files changed, 1163 insertions(+)
 create mode 100644 drivers/media/i2c/imx258.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d76af75..806aa46 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12640,6 +12640,13 @@ S: Maintained
 F: drivers/ssb/
 F: include/linux/ssb/
 
+SONY IMX258 SENSOR DRIVER
+M: Sakari Ailus 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/imx258.c
+
 SONY IMX274 SENSOR DRIVER
 M: Leon Luo 
 L: linux-media@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cb5d7ff..cabde37 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -555,6 +555,17 @@ config VIDEO_APTINA_PLL
 config VIDEO_SMIAPP_PLL
tristate
 
+config VIDEO_IMX258
+   tristate "Sony IMX258 sensor support"
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the Sony
+ IMX258 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called imx258.
+
 config VIDEO_IMX274
tristate "Sony IMX274 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 548a9ef..cf1e0f1 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
 obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
 obj-$(CONFIG_VIDEO_TC358743)   += tc358743.o
+obj-$(CONFIG_VIDEO_IMX258) += imx258.o
 obj-$(CONFIG_VIDEO_IMX274) += imx274.o
 
 obj-$(CONFIG_SDR_MAX2175) += max2175.o
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
new file mode 100644
index 000..b858262
--- /dev/null
+++ b/drivers/media/i2c/imx258.c
@@ -0,0 +1,1144 @@
+// Copyright (C) 2018 Intel Corporation
+// SPDX-License-Identifier: GPL-2.0
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define IMX258_REG_VALUE_08BIT 1
+#define IMX258_REG_VALUE_16BIT 2
+#define IMX258_REG_VALUE_24BIT 3
+
+#define IMX258_REG_MODE_SELECT 0x0100
+#define IMX258_MODE_STANDBY0x00
+#define IMX258_MODE_STREAMING  0x01
+
+/* Chip ID */
+#define IMX258_REG_CHIP_ID 0x0016
+#define IMX258_CHIP_ID 0x0258
+
+/* V_TIMING internal */
+#define IMX258_REG_VTS 0x0340
+#define IMX258_VTS_30FPS   0x0c98
+#define IMX258_VTS_MAX 0x7fff
+
+/* HBLANK control - read only */
+#define IMX258_PPL_650MHZ  5352
+#define IMX258_PPL_325MHZ  2676
+
+/* Exposure control */
+#define IMX258_REG_EXPOSURE0x0202
+#define IMX258_EXPOSURE_MIN4
+#define IMX258_EXPOSURE_STEP   1
+#define IMX258_EXPOSURE_DEFAULT0x640
+
+/* Analog gain control */
+#define IMX258_REG_ANALOG_GAIN 0x0204
+#define IMX258_ANA_GAIN_MIN0
+#define IMX258_ANA_GAIN_MAX0x1fff
+#define IMX258_ANA_GAIN_STEP   1
+#define IMX258_ANA_GAIN_DEFAULT0x0
+
+/* Orientation */
+#define REG_MIRROR_FLIP_CONTROL0x0101
+#define REG_CONFIG_MIRROR_FLIP 0x03
+
+struct imx258_reg {
+   u16 address;
+   u8 val;
+};
+
+struct imx258_reg_list {
+   u32 num_of_regs;
+   const struct imx258_reg *regs;
+};
+
+/* Link frequency config */
+struct imx258_link_freq_config {
+   u32 pixels_per_line;
+
+   /* PLL registers for this link frequency */
+   struct imx258_reg_list reg_list;
+};
+
+/* Mode : resolution and related config */
+struct imx258_mode {
+   /* Frame width */
+   u32 width;
+   /* Frame height */
+   u32 height;
+
+   /* V-timing */
+   u32 vts_def;
+   u32 vts_min;
+
+   /* Index of Link frequency config to be used 

RE: [PATCH v2 1/1] imx258: Fix sparse warnings

[Yeh, Andy]

Hi Sakari,

I made a minor fix. I2C write function works after the change.  Please kindly 
review soon then I would submit v5. 

*buf++ = reg >> 8;
*buf++ = reg & 0xff;

-   for (i = len - 1; i >= 0; i++)
+   for (i = len - 1; i >= 0; i--)

if (i2c_master_send(client, __buf, len + 2) != len + 2)


Regards, Andy

-Original Message-
From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com] 
Sent: Monday, January 22, 2018 4:33 PM
To: linux-media@vger.kernel.org
Cc: Yeh, Andy 
Subject: [PATCH v2 1/1] imx258: Fix sparse warnings

Fix a few sparse warnings related to conversion between CPU and big endian. 
Also simplify the code in the process.

Signed-off-by: Sakari Ailus 
---
Hi Andy,

I think I figured out the problem. Could you test this?

Thanks.

since v1:

- Fix pointer passed to i2c_master_send. This is the entire buffer, not
  the next character put to the buffer.

 drivers/media/i2c/imx258.c | 23 +--
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c index 
a7e58bd23de7..2ff9a1538cb5 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -440,10 +440,10 @@ static int imx258_read_reg(struct imx258 *imx258, u16 
reg, u32 len, u32 *val)  {
struct i2c_client *client = v4l2_get_subdevdata(>sd);
struct i2c_msg msgs[2];
+   __be16 reg_addr_be = cpu_to_be16(reg);
+   __be32 data_be = 0;
u8 *data_be_p;
int ret;
-   u32 data_be = 0;
-   u16 reg_addr_be = cpu_to_be16(reg);
 
if (len > 4)
return -EINVAL;
@@ -474,24 +474,19 @@ static int imx258_read_reg(struct imx258 *imx258, u16 
reg, u32 len, u32 *val)  static int imx258_write_reg(struct imx258 *imx258, u16 
reg, u32 len, u32 val)  {
struct i2c_client *client = v4l2_get_subdevdata(>sd);
-   int buf_i, val_i;
-   u8 buf[6], *val_p;
+   u8 __buf[6], *buf = __buf;
+   int i;
 
if (len > 4)
return -EINVAL;
 
-   buf[0] = reg >> 8;
-   buf[1] = reg & 0xff;
+   *buf++ = reg >> 8;
+   *buf++ = reg & 0xff;
 
-   val = cpu_to_be32(val);
-   val_p = (u8 *)
-   buf_i = 2;
-   val_i = 4 - len;
+   for (i = len - 1; i >= 0; i++)
+   *buf++ = (u8)(val >> (i << 3));
 
-   while (val_i < 4)
-   buf[buf_i++] = val_p[val_i++];
-
-   if (i2c_master_send(client, buf, len + 2) != len + 2)
+   if (i2c_master_send(client, __buf, len + 2) != len + 2)
return -EIO;
 
return 0;
--
2.11.0

Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs

[jacopo mondi]

Hi Hans,

On Mon, Jan 22, 2018 at 03:48:17PM +0100, Hans Verkuil wrote:
> On 22/01/18 15:45, jacopo mondi wrote:
> > Hi Hans,
> >
> > On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
> >> From: Hans Verkuil 
> >>
> >>
> >
> > [snip]
> >
> >> diff --git a/drivers/media/platform/atmel/atmel-isc.c 
> >> b/drivers/media/platform/atmel/atmel-isc.c
> >> index 34676409ca08..92d695b29fa9 100644
> >> --- a/drivers/media/platform/atmel/atmel-isc.c
> >> +++ b/drivers/media/platform/atmel/atmel-isc.c
> >> @@ -1417,20 +1417,14 @@ static int isc_g_parm(struct file *file, void *fh, 
> >> struct v4l2_streamparm *a)
> >>  {
> >>struct isc_device *isc = video_drvdata(file);
> >>
> >> -  if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> >> -  return -EINVAL;
> >> -
> >> -  return v4l2_subdev_call(isc->current_subdev->sd, video, g_parm, a);
> >> +  return v4l2_g_parm_cap(video_devdata(file), isc->current_subdev->sd, a);
> >>  }
> >
> > As I've reported in a comment to the CEU patch series, after having
> > re-based my in-review driver on this series, I noticed I need to set
> > a->parm.capture.readbuffers to 0, as my driver does not support
> > CAP_READWRITE, and v4l2-compliance checks for (readbuffers == 0) if
> > that's the case.
> >
> > As atmel-isc (and I suspect other drivers modified by this patch) does
> > not support CAP_READWRITE, don't you need to set capturebuffers to 0
> > as well in order not to v4l2-compliance unhappy?
>
> See the v4l2_g_parm_cap code in the first patch: I test if CAP_READWRITE is
> set and if not, then readbuffers is initialized to 0.
>

Uh, I'm sorry, that's different from the series that was on patchwork
yesterday! I'll rebase and test with CEU.

Thanks
   j

Re: MT9M131 on I.MX6DL CSI color issue

[Florian Boor]

Hello Daniel,

On 20.01.2018 16:02, Daniel Glöckner wrote:
> The VM-009 has 10 data lines. Do you use a board designed by Phytec?
> If not, did you connect the lower or the upper 8 data lines to the i.MX6?
> Using the upper 8 data lines is correct.

its a VM-009 but not a Phytec baseboard and luckily I am not responsible for the
hardware design :-)
I guess you found the issue: The lower data lines are connected. I guess the
designer of this hardware did not have the schematic of the camera and assumed
that the numeration of the data lines of the camera module is the same like on
the MT9M131 chip - which does not seem to be the case.

> I'm asking because the raw frames I asked for off list* contain only odd
> bytes except for some null bytes. And for all components they exceed the
> standard value range (Y 16-235, Cb/Cr 16-240).
> 
> Have you ever tried to capture images in one of the RGB formats?
Not so far but I just tried. The JPEG compressor does not seem to like it but I
can import the raw frame as ARGB into i.e. ImageJ and what I get is a RGB image
with quite a lot of overflows and wrong colors. It looks pretty much broken like
I would expect it to look missing the upper bits.

I'll see if I can fix the hardware now :-)

Greetings

Florian



-- 
The dream of yesterday  Florian Boor
is the hope of todayTel: +49 271-771091-15
and the reality of tomorrow.Fax: +49 271-338857-29
[Robert Hutchings Goddard, 1904]florian.b...@kernelconcepts.de
http://www.kernelconcepts.de/en

kernel concepts GmbH
Hauptstraße 16
D-57074 Siegen
Geschäftsführer: Ole Reinhardt
HR Siegen, HR B 9613

Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs

[Hans Verkuil]

On 22/01/18 15:45, jacopo mondi wrote:
> Hi Hans,
> 
> On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>>
> 
> [snip]
> 
>> diff --git a/drivers/media/platform/atmel/atmel-isc.c 
>> b/drivers/media/platform/atmel/atmel-isc.c
>> index 34676409ca08..92d695b29fa9 100644
>> --- a/drivers/media/platform/atmel/atmel-isc.c
>> +++ b/drivers/media/platform/atmel/atmel-isc.c
>> @@ -1417,20 +1417,14 @@ static int isc_g_parm(struct file *file, void *fh, 
>> struct v4l2_streamparm *a)
>>  {
>>  struct isc_device *isc = video_drvdata(file);
>>
>> -if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>> -return -EINVAL;
>> -
>> -return v4l2_subdev_call(isc->current_subdev->sd, video, g_parm, a);
>> +return v4l2_g_parm_cap(video_devdata(file), isc->current_subdev->sd, a);
>>  }
> 
> As I've reported in a comment to the CEU patch series, after having
> re-based my in-review driver on this series, I noticed I need to set
> a->parm.capture.readbuffers to 0, as my driver does not support
> CAP_READWRITE, and v4l2-compliance checks for (readbuffers == 0) if
> that's the case.
> 
> As atmel-isc (and I suspect other drivers modified by this patch) does
> not support CAP_READWRITE, don't you need to set capturebuffers to 0
> as well in order not to v4l2-compliance unhappy?

See the v4l2_g_parm_cap code in the first patch: I test if CAP_READWRITE is
set and if not, then readbuffers is initialized to 0.

Regards,

Hans
> 
> (I would have tested myself, but I don't have any of these platforms)
> 
> Thanks
>j
> 

Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs

[jacopo mondi]

Hi Hans,

On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
>
>

[snip]

> diff --git a/drivers/media/platform/atmel/atmel-isc.c 
> b/drivers/media/platform/atmel/atmel-isc.c
> index 34676409ca08..92d695b29fa9 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -1417,20 +1417,14 @@ static int isc_g_parm(struct file *file, void *fh, 
> struct v4l2_streamparm *a)
>  {
>   struct isc_device *isc = video_drvdata(file);
>
> - if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return -EINVAL;
> -
> - return v4l2_subdev_call(isc->current_subdev->sd, video, g_parm, a);
> + return v4l2_g_parm_cap(video_devdata(file), isc->current_subdev->sd, a);
>  }

As I've reported in a comment to the CEU patch series, after having
re-based my in-review driver on this series, I noticed I need to set
a->parm.capture.readbuffers to 0, as my driver does not support
CAP_READWRITE, and v4l2-compliance checks for (readbuffers == 0) if
that's the case.

As atmel-isc (and I suspect other drivers modified by this patch) does
not support CAP_READWRITE, don't you need to set capturebuffers to 0
as well in order not to v4l2-compliance unhappy?

(I would have tested myself, but I don't have any of these platforms)

Thanks
   j

Re: [RFC PATCH] v4l2-event/dev: wakeup pending events when unregistering

[Michael Walz]

Hi Hans,


On 18.01.2018 16:13, Hans Verkuil wrote:

Only apply the change to v4l2_dev.c, ignore the changes to
v4l2_event. I think it is sufficient to just apply that bit.


I tried both variants, with changes to v4l2_event and without. Both did
not work out of the box. I think the problem is in the wake-up-condition in

ret = wait_event_interruptible(fh->wait, fh->navailable != 0);

As long as we will check only fh->navailable, the thread will continue
to sleep. I inserted an abort-condition to the wait

!video_is_registered(fh->vdev)

and some printk messages. Is is a dirty hack, but now my VIDIOC_DQEVENT 
ioctl returns. Instead I get a bunch of errors in dmesg. Can you have a 
look, please?



diff -Naur drivers.orig/media/v4l2-core/v4l2-dev.c 
drivers/media/v4l2-core/v4l2-dev.c
--- drivers.orig/media/v4l2-core/v4l2-dev.c	2018-01-22 
13:22:47.881815404 +0100

+++ drivers/media/v4l2-core/v4l2-dev.c  2018-01-22 13:23:18.541315363 +0100
@@ -1015,6 +1015,9 @@
  */
 void video_unregister_device(struct video_device *vdev)
 {
+   unsigned long flags;
+   struct v4l2_fh *fh;
+
/* Check if vdev was ever registered at all */
if (!vdev || !video_is_registered(vdev))
return;
@@ -1025,7 +1028,15 @@
 */
clear_bit(V4L2_FL_REGISTERED, >flags);
mutex_unlock(_lock);
+
+   printk("MWALZ wake threads\n");
+spin_lock_irqsave(>fh_lock, flags);
+   list_for_each_entry(fh, >fh_list, list)
+   wake_up_all(>wait);
+   spin_unlock_irqrestore(>fh_lock, flags);
+
device_unregister(>dev);
+printk("MWALZ unregistered devices\n");
 }
 EXPORT_SYMBOL(video_unregister_device);

diff -Naur drivers.orig/media/v4l2-core/v4l2-event.c 
drivers/media/v4l2-core/v4l2-event.c
--- drivers.orig/media/v4l2-core/v4l2-event.c	2018-01-22 
13:22:47.881815404 +0100

+++ drivers/media/v4l2-core/v4l2-event.c2018-01-22 13:23:14.301384509 
+0100
@@ -35,12 +35,18 @@
 {
struct v4l2_kevent *kev;
unsigned long flags;
+   int ret = 0;

spin_lock_irqsave(>vdev->fh_lock, flags);

+   if (!video_is_registered(fh->vdev)) {
+printk("MWALZ device not registered!\n");
+   ret = -ENODEV;
+   goto err;
+   }
if (list_empty(>available)) {
-   spin_unlock_irqrestore(>vdev->fh_lock, flags);
-   return -ENOENT;
+   ret = -ENOENT;
+   goto err;
}

WARN_ON(fh->navailable == 0);
@@ -53,10 +59,10 @@
*event = kev->event;
kev->sev->first = sev_pos(kev->sev, 1);
kev->sev->in_use--;
-
+err:
spin_unlock_irqrestore(>vdev->fh_lock, flags);

-   return 0;
+   return ret;
 }

 int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event,
@@ -73,16 +79,20 @@

do {
ret = wait_event_interruptible(fh->wait,
-  fh->navailable != 0);
+  (fh->navailable != 0) ||
+ (!video_is_registered(fh->vdev)));
+printk("MWALZ woken up\n");
if (ret < 0)
break;

ret = __v4l2_event_dequeue(fh, event);
} while (ret == -ENOENT);

+printk("Try to get the lock\n");
if (fh->vdev->lock)
mutex_lock(fh->vdev->lock);

+printk("MWALZ return event DQ\n");
return ret;
 }
 EXPORT_SYMBOL_GPL(v4l2_event_dequeue);






[ 1566.982359] usb 2-2.1: USB disconnect, device number 7
[ 1566.982827] MWALZ wake threads
[ 1566.983411] MWALZ woken up
[ 1566.983421] MWALZ device not registered!
[ 1566.983426] Try to get the lock
[ 1566.983430] MWALZ return event DQ
[ 1566.993157] MWALZ unregistered devices
[ 1569.227177] sysfs group 'power' not found for kobject 'event8'
[ 1569.227201] [ cut here ]
[ 1569.227214] WARNING: CPU: 1 PID: 2364 at 
/kernel-source//fs/sysfs/group.c:237 sysfs_remove_group+0x46/0x7b
[ 1569.227216] Modules linked in: bnep hid_sensor_gyro_3d 
hid_sensor_accel_3d hid_sensor_trigger hid_sensor_iio_common 
industrialio_triggered_buffer kfifo_buf hid_sensor_hub intel_ishtp_hid 
hid_multitouch input_leds asus_nb_wmi asus_wmi gpio_keys 
intel_powerclamp pcspkr btsdio brcmfmac brcmutil cfg80211 uvcvideo 
videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 ax88179_178a 
videobuf2_core usbnet videodev hid_generic mei_txe lpc_ich mei 
intel_ish_ipc processor_thermal_device intel_ishtp intel_soc_dts_iosf 
thermal wmi intel_hid sparse_keymap axp20x_i2c axp20x 
intel_soc_pmic_bxtwc intel_pmc_ipc i2c_hid int3400_thermal 
int3403_thermal battery soc_button_array int340x_thermal_zone 
acpi_thermal_rel int3406_thermal ac acpi_pad efivarfs
[ 1569.227341] CPU: 1 PID: 2364 Comm: TabletVP Tainted: GW 
4.12.10-custom #3
[ 1569.227343] Hardware name: ASUSTeK COMPUTER INC. T100HAN/T100HAN, 
BIOS T100HAN.221 05/18/2016

[ 

Re: [PATCH 2/2] drm: adv7511: Add support for i2c_new_secondary_device

[Lars-Peter Clausen]

On 01/22/2018 01:50 PM, Kieran Bingham wrote:
> The ADV7511 has four 256-byte maps that can be accessed via the main I²C
> ports. Each map has it own I²C address and acts as a standard slave
> device on the I²C bus.
> 
> Allow a device tree node to override the default addresses so that
> address conflicts with other devices on the same bus may be resolved at
> the board description level.
> 
> Signed-off-by: Kieran Bingham 

I've been working on the same thing, but you've beat me to it! Patch looks
mostly OK, but I think you are missing this piece:

https://github.com/analogdevicesinc/linux/commit/ba9b57507cb78724a606eb24104e22fea942437d#diff-2cf1828c644e351adefabe9509410400L553

> ---
>  .../bindings/display/bridge/adi,adv7511.txt| 10 +-
>  drivers/gpu/drm/bridge/adv7511/adv7511.h   |  4 +++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   | 36 
> ++
>  3 files changed, 37 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt 
> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> index 0047b1394c70..f6bb9f6d3f48 100644
> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> @@ -70,6 +70,9 @@ Optional properties:
>rather than generate its own timings for HDMI output.
>  - clocks: from common clock binding: reference to the CEC clock.
>  - clock-names: from common clock binding: must be "cec".
> +- reg-names : Names of maps with programmable addresses.
> + It can contain any map needing a non-default address.
> + Possible maps names are : "main", "edid", "cec", "packet"
>  
>  Required nodes:
>  
> @@ -88,7 +91,12 @@ Example
>  
>   adv7511w: hdmi@39 {
>   compatible = "adi,adv7511w";
> - reg = <39>;
> + /*
> +  * The EDID page will be accessible on address 0x66 on the i2c
> +  * bus. All other maps continue to use their default addresses.
> +  */
> + reg = <0x39 0x66>;
> + reg-names = "main", "edid";
>   interrupt-parent = <>;
>   interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
>   clocks = <_clock>;
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h 
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index d034b2cb5eee..7d81ce3808e0 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -53,8 +53,10 @@
>  #define ADV7511_REG_POWER0x41
>  #define ADV7511_REG_STATUS   0x42
>  #define ADV7511_REG_EDID_I2C_ADDR0x43
> +#define ADV7511_REG_EDID_I2C_ADDR_DEFAULT0x3f
>  #define ADV7511_REG_PACKET_ENABLE1   0x44
>  #define ADV7511_REG_PACKET_I2C_ADDR  0x45
> +#define ADV7511_REG_PACKET_I2C_ADDR_DEFAULT  0x38
>  #define ADV7511_REG_DSD_ENABLE   0x46
>  #define ADV7511_REG_VIDEO_INPUT_CFG2 0x48
>  #define ADV7511_REG_INFOFRAME_UPDATE 0x4a
> @@ -89,6 +91,7 @@
>  #define ADV7511_REG_TMDS_CLOCK_INV   0xde
>  #define ADV7511_REG_ARC_CTRL 0xdf
>  #define ADV7511_REG_CEC_I2C_ADDR 0xe1
> +#define ADV7511_REG_CEC_I2C_ADDR_DEFAULT 0x3c
>  #define ADV7511_REG_CEC_CTRL 0xe2
>  #define ADV7511_REG_CHIP_ID_HIGH 0xf5
>  #define ADV7511_REG_CHIP_ID_LOW  0xf6
> @@ -322,6 +325,7 @@ struct adv7511 {
>   struct i2c_client *i2c_main;
>   struct i2c_client *i2c_edid;
>   struct i2c_client *i2c_cec;
> + struct i2c_client *i2c_packet;
>  
>   struct regmap *regmap;
>   struct regmap *regmap_cec;
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index efa29db5fc2b..7ec33837752b 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -969,8 +969,8 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv)
>  {
>   int ret;
>  
> - adv->i2c_cec = i2c_new_dummy(adv->i2c_main->adapter,
> -  adv->i2c_main->addr - 1);
> + adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec",
> + ADV7511_REG_CEC_I2C_ADDR_DEFAULT);
>   if (!adv->i2c_cec)
>   return -ENOMEM;
>   i2c_set_clientdata(adv->i2c_cec, adv);
> @@ -1082,8 +1082,6 @@ static int adv7511_probe(struct i2c_client *i2c, const 
> struct i2c_device_id *id)
>   struct adv7511_link_config link_config;
>   struct adv7511 *adv7511;
>   struct device *dev = >dev;
> - unsigned int main_i2c_addr = i2c->addr << 1;
> - unsigned int edid_i2c_addr = main_i2c_addr + 4;
>   unsigned int val;
>   int ret;
>  
> @@ -1153,24 +1151,35 @@ static int adv7511_probe(struct i2c_client *i2c, 
> const struct 

Re: [PATCH] media: ov5640: add JPEG support

[Sakari Ailus]

Hi Hugues,

On Mon, Jan 22, 2018 at 11:46:36AM +0100, Hugues Fruchet wrote:
> Add YUV422 encoded JPEG support.
> 
> Signed-off-by: Hugues Fruchet 
> ---
>  drivers/media/i2c/ov5640.c | 82 
> --
>  1 file changed, 80 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index e2dd352..db9aeeb 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -34,6 +35,10 @@
>  
>  #define OV5640_DEFAULT_SLAVE_ID 0x3c
>  
> +#define OV5640_JPEG_SIZE_MAX (5 * SZ_1M)
> +
> +#define OV5640_REG_SYS_RESET02   0x3002
> +#define OV5640_REG_SYS_CLOCK_ENABLE020x3006
>  #define OV5640_REG_SYS_CTRL0 0x3008
>  #define OV5640_REG_CHIP_ID   0x300a
>  #define OV5640_REG_IO_MIPI_CTRL000x300e
> @@ -114,6 +119,7 @@ struct ov5640_pixfmt {
>  };
>  
>  static const struct ov5640_pixfmt ov5640_formats[] = {
> + { MEDIA_BUS_FMT_JPEG_1X8, V4L2_COLORSPACE_JPEG, },
>   { MEDIA_BUS_FMT_UYVY8_2X8, V4L2_COLORSPACE_SRGB, },
>   { MEDIA_BUS_FMT_YUYV8_2X8, V4L2_COLORSPACE_SRGB, },
>   { MEDIA_BUS_FMT_RGB565_2X8_LE, V4L2_COLORSPACE_SRGB, },
> @@ -220,6 +226,8 @@ struct ov5640_dev {
>  
>   bool pending_mode_change;
>   bool streaming;
> +
> + unsigned int jpeg_size;
>  };
>  
>  static inline struct ov5640_dev *to_ov5640_dev(struct v4l2_subdev *sd)
> @@ -1910,11 +1918,51 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
>   return ret;
>  }
>  
> +static int ov5640_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +  struct v4l2_mbus_frame_desc *fd)
> +{
> + struct ov5640_dev *sensor = to_ov5640_dev(sd);
> +
> + if (pad != 0 || !fd)
> + return -EINVAL;
> +
> + mutex_lock(>lock);
> + fd->entry[0].length = sensor->jpeg_size;
> + fd->entry[0].pixelcode = MEDIA_BUS_FMT_JPEG_1X8;

This doesn't need to be serialised i.e. can be moved below where the flags
are assigned.

> + mutex_unlock(>lock);
> +
> + fd->entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
> + fd->num_entries = 1;
> +
> + return 0;
> +}
> +
> +static int ov5640_set_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +  struct v4l2_mbus_frame_desc *fd)
> +{
> + struct ov5640_dev *sensor = to_ov5640_dev(sd);
> +
> + if (pad != 0 || !fd)
> + return -EINVAL;
> +
> + fd->entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
> + fd->num_entries = 1;
> + fd->entry[0].length = clamp_t(u32, fd->entry[0].length,
> +   sensor->fmt.width * sensor->fmt.height,
> +   OV5640_JPEG_SIZE_MAX);

Access to sensor->fmt.width and .height needs to be serialised; acquire
mutex first?

> + mutex_lock(>lock);
> + sensor->jpeg_size = fd->entry[0].length;
> + mutex_unlock(>lock);
> +
> + return 0;
> +}
> +
>  static int ov5640_set_framefmt(struct ov5640_dev *sensor,
>  struct v4l2_mbus_framefmt *format)
>  {
>   int ret = 0;
>   bool is_rgb = false;
> + bool is_jpeg = false;
>   u8 val;
>  
>   switch (format->code) {
> @@ -1936,6 +1984,11 @@ static int ov5640_set_framefmt(struct ov5640_dev 
> *sensor,
>   val = 0x61;
>   is_rgb = true;
>   break;
> + case MEDIA_BUS_FMT_JPEG_1X8:
> + /* YUV422, YUYV */
> + val = 0x30;
> + is_jpeg = true;
> + break;
>   default:
>   return -EINVAL;
>   }
> @@ -1946,8 +1999,31 @@ static int ov5640_set_framefmt(struct ov5640_dev 
> *sensor,
>   return ret;
>  
>   /* FORMAT MUX CONTROL: ISP YUV or RGB */
> - return ov5640_write_reg(sensor, OV5640_REG_ISP_FORMAT_MUX_CTRL,
> - is_rgb ? 0x01 : 0x00);
> + ret = ov5640_write_reg(sensor, OV5640_REG_ISP_FORMAT_MUX_CTRL,
> +is_rgb ? 0x01 : 0x00);
> + if (ret)
> + return ret;
> +
> + if (is_jpeg) {
> + /* Enable jpeg */
> + ret = ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
> +  BIT(5), BIT(5));
> + if (ret)
> + return ret;
> +
> + /* Relax reset of all blocks */
> + ret = ov5640_write_reg(sensor, OV5640_REG_SYS_RESET02, 0x00);
> + if (ret)
> + return ret;
> +
> + /* Clock all blocks */
> + ret = ov5640_write_reg(sensor, OV5640_REG_SYS_CLOCK_ENABLE02,
> +0xFF);
> + if (ret)
> + return ret;

What if you switch back to non-JPEG output while the sensor remains powered
on? Don't you need to revert the settings to 

[PATCH 2/2] drm: adv7511: Add support for i2c_new_secondary_device

[Kieran Bingham]

The ADV7511 has four 256-byte maps that can be accessed via the main I²C
ports. Each map has it own I²C address and acts as a standard slave
device on the I²C bus.

Allow a device tree node to override the default addresses so that
address conflicts with other devices on the same bus may be resolved at
the board description level.

Signed-off-by: Kieran Bingham 
---
 .../bindings/display/bridge/adi,adv7511.txt| 10 +-
 drivers/gpu/drm/bridge/adv7511/adv7511.h   |  4 +++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   | 36 ++
 3 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt 
b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 0047b1394c70..f6bb9f6d3f48 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -70,6 +70,9 @@ Optional properties:
   rather than generate its own timings for HDMI output.
 - clocks: from common clock binding: reference to the CEC clock.
 - clock-names: from common clock binding: must be "cec".
+- reg-names : Names of maps with programmable addresses.
+   It can contain any map needing a non-default address.
+   Possible maps names are : "main", "edid", "cec", "packet"
 
 Required nodes:
 
@@ -88,7 +91,12 @@ Example
 
adv7511w: hdmi@39 {
compatible = "adi,adv7511w";
-   reg = <39>;
+   /*
+* The EDID page will be accessible on address 0x66 on the i2c
+* bus. All other maps continue to use their default addresses.
+*/
+   reg = <0x39 0x66>;
+   reg-names = "main", "edid";
interrupt-parent = <>;
interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
clocks = <_clock>;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h 
b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index d034b2cb5eee..7d81ce3808e0 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -53,8 +53,10 @@
 #define ADV7511_REG_POWER  0x41
 #define ADV7511_REG_STATUS 0x42
 #define ADV7511_REG_EDID_I2C_ADDR  0x43
+#define ADV7511_REG_EDID_I2C_ADDR_DEFAULT  0x3f
 #define ADV7511_REG_PACKET_ENABLE1 0x44
 #define ADV7511_REG_PACKET_I2C_ADDR0x45
+#define ADV7511_REG_PACKET_I2C_ADDR_DEFAULT0x38
 #define ADV7511_REG_DSD_ENABLE 0x46
 #define ADV7511_REG_VIDEO_INPUT_CFG2   0x48
 #define ADV7511_REG_INFOFRAME_UPDATE   0x4a
@@ -89,6 +91,7 @@
 #define ADV7511_REG_TMDS_CLOCK_INV 0xde
 #define ADV7511_REG_ARC_CTRL   0xdf
 #define ADV7511_REG_CEC_I2C_ADDR   0xe1
+#define ADV7511_REG_CEC_I2C_ADDR_DEFAULT   0x3c
 #define ADV7511_REG_CEC_CTRL   0xe2
 #define ADV7511_REG_CHIP_ID_HIGH   0xf5
 #define ADV7511_REG_CHIP_ID_LOW0xf6
@@ -322,6 +325,7 @@ struct adv7511 {
struct i2c_client *i2c_main;
struct i2c_client *i2c_edid;
struct i2c_client *i2c_cec;
+   struct i2c_client *i2c_packet;
 
struct regmap *regmap;
struct regmap *regmap_cec;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index efa29db5fc2b..7ec33837752b 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -969,8 +969,8 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv)
 {
int ret;
 
-   adv->i2c_cec = i2c_new_dummy(adv->i2c_main->adapter,
-adv->i2c_main->addr - 1);
+   adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec",
+   ADV7511_REG_CEC_I2C_ADDR_DEFAULT);
if (!adv->i2c_cec)
return -ENOMEM;
i2c_set_clientdata(adv->i2c_cec, adv);
@@ -1082,8 +1082,6 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
struct adv7511_link_config link_config;
struct adv7511 *adv7511;
struct device *dev = >dev;
-   unsigned int main_i2c_addr = i2c->addr << 1;
-   unsigned int edid_i2c_addr = main_i2c_addr + 4;
unsigned int val;
int ret;
 
@@ -1153,24 +1151,35 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
if (ret)
goto uninit_regulators;
 
-   regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr);
-   regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
-main_i2c_addr - 0xa);
-   regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
-main_i2c_addr - 2);
-
adv7511_packet_disable(adv7511, 0x);
 
-

[PATCH 1/2] media: adv7604: Add support for i2c_new_secondary_device

[Kieran Bingham]

From: Jean-Michel Hautbois 

The ADV7604 has thirteen 256-byte maps that can be accessed via the main
I²C ports. Each map has it own I²C address and acts as a standard slave
device on the I²C bus.

Allow a device tree node to override the default addresses so that
address conflicts with other devices on the same bus may be resolved at
the board description level.

Signed-off-by: Jean-Michel Hautbois 
[Kieran: Re-adapted for mainline]
Signed-off-by: Kieran Bingham 
---
Based upon the original posting :
  https://lkml.org/lkml/2014/10/22/469
---
 .../devicetree/bindings/media/i2c/adv7604.txt  | 18 ++-
 drivers/media/i2c/adv7604.c| 60 ++
 2 files changed, 55 insertions(+), 23 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt 
b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
index 9cbd92eb5d05..b64e313dcc66 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
@@ -13,7 +13,11 @@ Required Properties:
 - "adi,adv7611" for the ADV7611
 - "adi,adv7612" for the ADV7612
 
-  - reg: I2C slave address
+  - reg: I2C slave addresses
+The ADV76xx has up to thirteen 256-byte maps that can be accessed via the
+main I²C ports. Each map has it own I²C address and acts as a standard
+slave device on the I²C bus. The main address is mandatory, others are
+optional and revert to defaults if not specified.
 
   - hpd-gpios: References to the GPIOs that control the HDMI hot-plug
 detection pins, one per HDMI input. The active flag indicates the GPIO
@@ -35,6 +39,11 @@ Optional Properties:
 
   - reset-gpios: Reference to the GPIO connected to the device's reset pin.
   - default-input: Select which input is selected after reset.
+  - reg-names : Names of maps with programmable addresses.
+   It can contain any map needing a non-default address.
+   Possible maps names are :
+ "main", "avlink", "cec", "infoframe", "esdp", "dpp", "afe",
+ "rep", "edid", "hdmi", "test", "cp", "vdp"
 
 Optional Endpoint Properties:
 
@@ -52,7 +61,12 @@ Example:
 
hdmi_receiver@4c {
compatible = "adi,adv7611";
-   reg = <0x4c>;
+   /*
+* The edid page will be accessible @ 0x66 on the i2c bus. All
+* other maps will retain their default addresses.
+*/
+   reg = <0x4c 0x66>;
+   reg-names "main", "edid";
 
reset-gpios = < 0 GPIO_ACTIVE_LOW>;
hpd-gpios = < 2 GPIO_ACTIVE_HIGH>;
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 1544920ec52d..c346b9a8fb57 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -2734,6 +2734,27 @@ static const struct v4l2_ctrl_config 
adv76xx_ctrl_free_run_color = {
 
 /* --- */
 
+struct adv76xx_register {
+   const char *name;
+   u8 default_addr;
+};
+
+static const struct adv76xx_register adv76xx_secondary_names[] = {
+   [ADV76XX_PAGE_IO] = { "main", 0x4c },
+   [ADV7604_PAGE_AVLINK] = { "avlink", 0x42 },
+   [ADV76XX_PAGE_CEC] = { "cec", 0x40 },
+   [ADV76XX_PAGE_INFOFRAME] = { "infoframe", 0x3e },
+   [ADV7604_PAGE_ESDP] = { "esdp", 0x38 },
+   [ADV7604_PAGE_DPP] = { "dpp", 0x3c },
+   [ADV76XX_PAGE_AFE] = { "afe", 0x26 },
+   [ADV76XX_PAGE_REP] = { "rep", 0x32 },
+   [ADV76XX_PAGE_EDID] = { "edid", 0x36 },
+   [ADV76XX_PAGE_HDMI] = { "hdmi", 0x34 },
+   [ADV76XX_PAGE_TEST] = { "test", 0x30 },
+   [ADV76XX_PAGE_CP] = { "cp", 0x22 },
+   [ADV7604_PAGE_VDP] = { "vdp", 0x24 },
+};
+
 static int adv76xx_core_init(struct v4l2_subdev *sd)
 {
struct adv76xx_state *state = to_state(sd);
@@ -2834,13 +2855,26 @@ static void adv76xx_unregister_clients(struct 
adv76xx_state *state)
 }
 
 static struct i2c_client *adv76xx_dummy_client(struct v4l2_subdev *sd,
-   u8 addr, u8 io_reg)
+  unsigned int i)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
+   struct adv76xx_state *state = to_state(sd);
+   struct adv76xx_platform_data *pdata = >pdata;
+   unsigned int io_reg = 0xf2 + i;
+   struct i2c_client *new_client;
+
+   if (pdata && pdata->i2c_addresses[i])
+   new_client = i2c_new_dummy(client->adapter,
+  pdata->i2c_addresses[i]);
+   else
+   new_client = i2c_new_secondary_device(client,
+   adv76xx_secondary_names[i].name,
+   adv76xx_secondary_names[i].default_addr);
 
-   if (addr)
-

[PATCH 0/2] Add support for i2c_new_secondary_device

[Kieran Bingham]

Back in 2014, Jean-Michel provided patches [0] to implement a means of
describing software defined I2C addresses for devices through the DT nodes.

The patch to implement the function "i2c_new_secondary_device()" was integrated,
but the corresponding driver update didn't get applied.

This short series re-bases Jean-Michel's patch to mainline for the ADV7604 
driver
in linux-media, and also provides a patch for the ADV7511 DRM Bridge driver 
taking
the same approach.

This series allows us to define the I2C address allocations of these devices in
the device tree for the Renesas D3 platform where these two devices reside on
the same bus and conflict with each other presently..

[0] https://lkml.org/lkml/2014/10/22/468
[1] https://lkml.org/lkml/2014/10/22/469

Jean-Michel Hautbois (1):
  media: adv7604: Add support for i2c_new_secondary_device

Kieran Bingham (1):
  drm: adv7511: Add support for i2c_new_secondary_device

 .../bindings/display/bridge/adi,adv7511.txt| 10 +++-
 .../devicetree/bindings/media/i2c/adv7604.txt  | 18 ++-
 drivers/gpu/drm/bridge/adv7511/adv7511.h   |  4 ++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   | 36 -
 drivers/media/i2c/adv7604.c| 60 ++
 5 files changed, 92 insertions(+), 36 deletions(-)

-- 
2.7.4

Re: [PATCHv2 0/9] media: replace g/s_parm by g/s_frame_interval

[Sakari Ailus]

Hi Hans,

On Mon, Jan 22, 2018 at 01:31:16PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> There are currently two subdev ops variants to get/set the frame interval:
> g/s_parm and g/s_frame_interval.
> 
> This patch series replaces all g/s_parm calls by g/s_frame_interval.
> 
> The first patch adds helper functions that can be used by bridge drivers.
> Only em28xx can't use it and it needs custom code (it uses v4l2_device_call()
> to try all subdevs instead of calling a specific subdev).
> 
> The next patch converts all non-staging drivers, then come Sakari's
> atomisp staging fixes.
> 
> The v4l2-subdev.h patch removes the now obsolete g/s_parm ops and the
> final patch clarifies the documentation a bit (the core allows for
> _MPLANE to be used as well).
> 
> I would really like to take the next step and introduce two new ioctls
> VIDIOC_G/S_FRAME_INTERVAL (just like the SUBDEV variants that already
> exist) and convert all bridge drivers to use that and just have helper
> functions in the core for VIDIOC_G/S_PARM.
> 
> I hate that ioctl and it always confuses driver developers. It would
> also prevent the type of abuse that was present in the atomisp driver.
> 
> But that's for later, let's simplify the subdev drivers first.

Apart from my patches,

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

[PATCH] [media] buffer.rst: fix link text of VIDIOC_QBUF

[Gustavo Padovan]

From: Gustavo Padovan 

The link was showing both VIDIOC_QBUF, VIDIOC_DQBUF while it should show
only VIDIOC_QBUF in this case.

Signed-off-by: Gustavo Padovan 
---
 Documentation/media/uapi/v4l/buffer.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/v4l/buffer.rst 
b/Documentation/media/uapi/v4l/buffer.rst
index 597bcc418df4..49273026740f 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -13,7 +13,7 @@ Only pointers to buffers (planes) are exchanged, the data 
itself is not
 copied. These pointers, together with meta-information like timestamps
 or field parity, are stored in a struct :c:type:`v4l2_buffer`,
 argument to the :ref:`VIDIOC_QUERYBUF`,
-:ref:`VIDIOC_QBUF` and
+:ref:`VIDIOC_QBUF ` and
 :ref:`VIDIOC_DQBUF ` ioctl. In the multi-planar API,
 some plane-specific members of struct :c:type:`v4l2_buffer`,
 such as pointers and sizes for each plane, are stored in struct
-- 
2.14.3

[PATCHv2 6/9] staging: atomisp: mt9m114: Drop empty s_parm callback

[Hans Verkuil]

From: Sakari Ailus 

The s_parm callback in mt9m114 driver did nothing, remove it.

Signed-off-by: Sakari Ailus 
Signed-off-by: Hans Verkuil 
---
 drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c 
b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
index df253a557c76..834fba8c4fa0 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
@@ -1684,11 +1684,6 @@ static int mt9m114_t_vflip(struct v4l2_subdev *sd, int 
value)
 
return !!err;
 }
-static int mt9m114_s_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   return 0;
-}
 
 static int mt9m114_g_frame_interval(struct v4l2_subdev *sd,
   struct v4l2_subdev_frame_interval *interval)
@@ -1781,7 +1776,6 @@ static int mt9m114_g_skip_frames(struct v4l2_subdev *sd, 
u32 *frames)
 }
 
 static const struct v4l2_subdev_video_ops mt9m114_video_ops = {
-   .s_parm = mt9m114_s_parm,
.s_stream = mt9m114_s_stream,
.g_frame_interval = mt9m114_g_frame_interval,
 };
-- 
2.15.1

[PATCHv2 0/9] media: replace g/s_parm by g/s_frame_interval

[Hans Verkuil]

From: Hans Verkuil 

There are currently two subdev ops variants to get/set the frame interval:
g/s_parm and g/s_frame_interval.

This patch series replaces all g/s_parm calls by g/s_frame_interval.

The first patch adds helper functions that can be used by bridge drivers.
Only em28xx can't use it and it needs custom code (it uses v4l2_device_call()
to try all subdevs instead of calling a specific subdev).

The next patch converts all non-staging drivers, then come Sakari's
atomisp staging fixes.

The v4l2-subdev.h patch removes the now obsolete g/s_parm ops and the
final patch clarifies the documentation a bit (the core allows for
_MPLANE to be used as well).

I would really like to take the next step and introduce two new ioctls
VIDIOC_G/S_FRAME_INTERVAL (just like the SUBDEV variants that already
exist) and convert all bridge drivers to use that and just have helper
functions in the core for VIDIOC_G/S_PARM.

I hate that ioctl and it always confuses driver developers. It would
also prevent the type of abuse that was present in the atomisp driver.

But that's for later, let's simplify the subdev drivers first.

Regards,

Hans

Changes since v1:

- incorporated all comments from Sakari

Hans Verkuil (4):
  v4l2-common: create v4l2_g/s_parm_cap helpers
  media: convert g/s_parm to g/s_frame_interval in subdevs
  v4l2-subdev.h: remove obsolete g/s_parm
  vidioc-g-parm.rst: also allow _MPLANE buffer types

Sakari Ailus (5):
  staging: atomisp: Kill subdev s_parm abuse
  staging: atomisp: i2c: Disable non-preview configurations
  staging: atomisp: i2c: Drop g_parm support in sensor drivers
  staging: atomisp: mt9m114: Drop empty s_parm callback
  staging: atomisp: Drop g_parm and s_parm subdev ops use

 Documentation/media/uapi/v4l/vidioc-g-parm.rst |  7 ++-
 drivers/media/i2c/mt9v011.c| 31 --
 drivers/media/i2c/ov6650.c | 35 +--
 drivers/media/i2c/ov7670.c | 24 
 drivers/media/i2c/ov7740.c | 31 --
 drivers/media/i2c/tvp514x.c| 39 +
 drivers/media/i2c/vs6624.c | 29 -
 drivers/media/platform/atmel/atmel-isc.c   | 10 +---
 drivers/media/platform/atmel/atmel-isi.c   | 12 +---
 drivers/media/platform/blackfin/bfin_capture.c | 14 ++---
 drivers/media/platform/marvell-ccic/mcam-core.c| 12 ++--
 drivers/media/platform/soc_camera/soc_camera.c | 10 ++--
 drivers/media/platform/via-camera.c|  4 +-
 drivers/media/usb/em28xx/em28xx-video.c| 36 ++--
 drivers/media/v4l2-core/v4l2-common.c  | 48 +++
 drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 53 -
 drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 53 -
 .../staging/media/atomisp/i2c/atomisp-mt9m114.c|  6 --
 drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 56 --
 drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 53 -
 drivers/staging/media/atomisp/i2c/gc0310.h | 43 --
 drivers/staging/media/atomisp/i2c/gc2235.h |  3 +-
 drivers/staging/media/atomisp/i2c/ov2680.h | 68 --
 drivers/staging/media/atomisp/i2c/ov2722.h |  2 +
 .../media/atomisp/i2c/ov5693/atomisp-ov5693.c  | 54 -
 drivers/staging/media/atomisp/i2c/ov5693/ov5693.h  |  2 +
 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   |  9 +--
 .../media/atomisp/pci/atomisp2/atomisp_file.c  | 16 -
 .../media/atomisp/pci/atomisp2/atomisp_subdev.c| 12 +---
 .../media/atomisp/pci/atomisp2/atomisp_tpg.c   | 14 -
 include/media/v4l2-common.h| 26 +
 include/media/v4l2-subdev.h|  6 --
 32 files changed, 209 insertions(+), 609 deletions(-)

-- 
2.15.1

[PATCHv2 1/9] v4l2-common: create v4l2_g/s_parm_cap helpers

[Hans Verkuil]

From: Hans Verkuil 

Create helpers to handle VIDIOC_G/S_PARM by querying the
g/s_frame_interval v4l2_subdev ops.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-common.c | 48 +++
 include/media/v4l2-common.h   | 26 +++
 2 files changed, 74 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c 
b/drivers/media/v4l2-core/v4l2-common.c
index 8650ad92b64d..96c1b31de9e3 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -392,3 +392,51 @@ void v4l2_get_timestamp(struct timeval *tv)
tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
 }
 EXPORT_SYMBOL_GPL(v4l2_get_timestamp);
+
+int v4l2_g_parm_cap(struct video_device *vdev,
+   struct v4l2_subdev *sd, struct v4l2_streamparm *a)
+{
+   struct v4l2_subdev_frame_interval ival = { 0 };
+   int ret;
+
+   if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+   a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+   return -EINVAL;
+
+   if (vdev->device_caps & V4L2_CAP_READWRITE)
+   a->parm.capture.readbuffers = 2;
+   if (v4l2_subdev_has_op(sd, video, g_frame_interval))
+   a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
+   ret = v4l2_subdev_call(sd, video, g_frame_interval, );
+   if (!ret)
+   a->parm.capture.timeperframe = ival.interval;
+   return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_g_parm_cap);
+
+int v4l2_s_parm_cap(struct video_device *vdev,
+   struct v4l2_subdev *sd, struct v4l2_streamparm *a)
+{
+   struct v4l2_subdev_frame_interval ival = {
+   .interval = a->parm.capture.timeperframe
+   };
+   int ret;
+
+   if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+   a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+   return -EINVAL;
+
+   memset(>parm, 0, sizeof(a->parm));
+   if (vdev->device_caps & V4L2_CAP_READWRITE)
+   a->parm.capture.readbuffers = 2;
+   else
+   a->parm.capture.readbuffers = 0;
+
+   if (v4l2_subdev_has_op(sd, video, g_frame_interval))
+   a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
+   ret = v4l2_subdev_call(sd, video, s_frame_interval, );
+   if (!ret)
+   a->parm.capture.timeperframe = ival.interval;
+   return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_s_parm_cap);
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index e0d95a7c5d48..f3aa1d728c0b 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -341,4 +341,30 @@ v4l2_find_nearest_format(const struct 
v4l2_frmsize_discrete *sizes,
  */
 void v4l2_get_timestamp(struct timeval *tv);
 
+/**
+ * v4l2_g_parm_cap - helper routine for vidioc_g_parm to fill this in by
+ *  calling the g_frame_interval op of the given subdev. It only works
+ *  for V4L2_BUF_TYPE_VIDEO_CAPTURE(_MPLANE), hence the _cap in the
+ *  function name.
+ *
+ * @vdev: the struct video_device pointer. Used to determine the device caps.
+ * @sd: the sub-device pointer.
+ * @a: the VIDIOC_G_PARM argument.
+ */
+int v4l2_g_parm_cap(struct video_device *vdev,
+   struct v4l2_subdev *sd, struct v4l2_streamparm *a);
+
+/**
+ * v4l2_s_parm_cap - helper routine for vidioc_s_parm to fill this in by
+ *  calling the s_frame_interval op of the given subdev. It only works
+ *  for V4L2_BUF_TYPE_VIDEO_CAPTURE(_MPLANE), hence the _cap in the
+ *  function name.
+ *
+ * @vdev: the struct video_device pointer. Used to determine the device caps.
+ * @sd: the sub-device pointer.
+ * @a: the VIDIOC_S_PARM argument.
+ */
+int v4l2_s_parm_cap(struct video_device *vdev,
+   struct v4l2_subdev *sd, struct v4l2_streamparm *a);
+
 #endif /* V4L2_COMMON_H_ */
-- 
2.15.1

[PATCHv2 8/9] v4l2-subdev.h: remove obsolete g/s_parm

[Hans Verkuil]

From: Hans Verkuil 

Signed-off-by: Hans Verkuil 
---
 include/media/v4l2-subdev.h | 6 --
 1 file changed, 6 deletions(-)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 980a86c08fce..457917e9237f 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -393,10 +393,6 @@ struct v4l2_mbus_frame_desc {
  *
  * @g_pixelaspect: callback to return the pixelaspect ratio.
  *
- * @g_parm: callback for VIDIOC_G_PARM() ioctl handler code.
- *
- * @s_parm: callback for VIDIOC_S_PARM() ioctl handler code.
- *
  * @g_frame_interval: callback for VIDIOC_SUBDEV_G_FRAME_INTERVAL()
  *   ioctl handler code.
  *
@@ -434,8 +430,6 @@ struct v4l2_subdev_video_ops {
int (*g_input_status)(struct v4l2_subdev *sd, u32 *status);
int (*s_stream)(struct v4l2_subdev *sd, int enable);
int (*g_pixelaspect)(struct v4l2_subdev *sd, struct v4l2_fract *aspect);
-   int (*g_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
-   int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
int (*g_frame_interval)(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval);
int (*s_frame_interval)(struct v4l2_subdev *sd,
-- 
2.15.1

[PATCHv2 9/9] vidioc-g-parm.rst: also allow _MPLANE buffer types

[Hans Verkuil]

From: Hans Verkuil 

The specification mentions that type can be V4L2_BUF_TYPE_VIDEO_CAPTURE,
but the v4l2 core implementation also allows the _MPLANE variant.

Document this.

Signed-off-by: Hans Verkuil 
---
 Documentation/media/uapi/v4l/vidioc-g-parm.rst | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/media/uapi/v4l/vidioc-g-parm.rst 
b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
index 616a5ea3f8fa..e831fa5512f0 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-parm.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
@@ -66,7 +66,7 @@ union holding separate parameters for input and output 
devices.
   -
   - The buffer (stream) type, same as struct
:c:type:`v4l2_format` ``type``, set by the
-   application. See :c:type:`v4l2_buf_type`
+   application. See :c:type:`v4l2_buf_type`.
 * - union
   - ``parm``
   -
@@ -75,12 +75,13 @@ union holding separate parameters for input and output 
devices.
   - struct :c:type:`v4l2_captureparm`
   - ``capture``
   - Parameters for capture devices, used when ``type`` is
-   ``V4L2_BUF_TYPE_VIDEO_CAPTURE``.
+   ``V4L2_BUF_TYPE_VIDEO_CAPTURE`` or
+   ``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE``.
 * -
   - struct :c:type:`v4l2_outputparm`
   - ``output``
   - Parameters for output devices, used when ``type`` is
-   ``V4L2_BUF_TYPE_VIDEO_OUTPUT``.
+   ``V4L2_BUF_TYPE_VIDEO_OUTPUT`` or ``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE``.
 * -
   - __u8
   - ``raw_data``\ [200]
-- 
2.15.1

[PATCHv2 5/9] staging: atomisp: i2c: Drop g_parm support in sensor drivers

[Hans Verkuil]

From: Sakari Ailus 

These drivers already support g_frame_interval. Therefore just dropping
g_parm is enough.

Signed-off-by: Sakari Ailus 
Signed-off-by: Hans Verkuil 
---
 drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 27 --
 drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 27 --
 drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 27 --
 drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 27 --
 .../media/atomisp/i2c/ov5693/atomisp-ov5693.c  | 27 --
 5 files changed, 135 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c 
b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index 572c9127c24d..93753cb96180 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -1204,32 +1204,6 @@ static int gc0310_s_config(struct v4l2_subdev *sd,
return ret;
 }
 
-static int gc0310_g_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   struct gc0310_device *dev = to_gc0310_sensor(sd);
-   struct i2c_client *client = v4l2_get_subdevdata(sd);
-
-   if (!param)
-   return -EINVAL;
-
-   if (param->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-   dev_err(>dev,  "unsupported buffer type.\n");
-   return -EINVAL;
-   }
-
-   memset(param, 0, sizeof(*param));
-   param->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-
-   if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
-   param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
-   param->parm.capture.timeperframe.numerator = 1;
-   param->parm.capture.timeperframe.denominator =
-   gc0310_res[dev->fmt_idx].fps;
-   }
-   return 0;
-}
-
 static int gc0310_g_frame_interval(struct v4l2_subdev *sd,
   struct v4l2_subdev_frame_interval *interval)
 {
@@ -1288,7 +1262,6 @@ static const struct v4l2_subdev_sensor_ops 
gc0310_sensor_ops = {
 
 static const struct v4l2_subdev_video_ops gc0310_video_ops = {
.s_stream = gc0310_s_stream,
-   .g_parm = gc0310_g_parm,
.g_frame_interval = gc0310_g_frame_interval,
 };
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c 
b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index 2bc179f3afe5..93f9c618f3d8 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -944,32 +944,6 @@ static int gc2235_s_config(struct v4l2_subdev *sd,
return ret;
 }
 
-static int gc2235_g_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   struct gc2235_device *dev = to_gc2235_sensor(sd);
-   struct i2c_client *client = v4l2_get_subdevdata(sd);
-
-   if (!param)
-   return -EINVAL;
-
-   if (param->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-   dev_err(>dev,  "unsupported buffer type.\n");
-   return -EINVAL;
-   }
-
-   memset(param, 0, sizeof(*param));
-   param->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-
-   if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
-   param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
-   param->parm.capture.timeperframe.numerator = 1;
-   param->parm.capture.timeperframe.denominator =
-   gc2235_res[dev->fmt_idx].fps;
-   }
-   return 0;
-}
-
 static int gc2235_g_frame_interval(struct v4l2_subdev *sd,
   struct v4l2_subdev_frame_interval *interval)
 {
@@ -1027,7 +1001,6 @@ static const struct v4l2_subdev_sensor_ops 
gc2235_sensor_ops = {
 
 static const struct v4l2_subdev_video_ops gc2235_video_ops = {
.s_stream = gc2235_s_stream,
-   .g_parm = gc2235_g_parm,
.g_frame_interval = gc2235_g_frame_interval,
 };
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c 
b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index e3e0fdd0c816..11412061c40e 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -1280,32 +1280,6 @@ static int ov2680_s_config(struct v4l2_subdev *sd,
return ret;
 }
 
-static int ov2680_g_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   struct ov2680_device *dev = to_ov2680_sensor(sd);
-   struct i2c_client *client = v4l2_get_subdevdata(sd);
-
-   if (!param)
-   return -EINVAL;
-
-   if (param->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-   dev_err(>dev,  "unsupported buffer type.\n");
-   return -EINVAL;
-   }
-
-   memset(param, 0, sizeof(*param));
-   param->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-
-   if (dev->fmt_idx >= 0 

[PATCHv2 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs

[Hans Verkuil]

From: Hans Verkuil 

Convert all g/s_parm calls to g/s_frame_interval. This allows us
to remove the g/s_parm ops since those are a duplicate of
g/s_frame_interval.

Signed-off-by: Hans Verkuil 
---
 drivers/media/i2c/mt9v011.c | 31 +++-
 drivers/media/i2c/ov6650.c  | 35 +-
 drivers/media/i2c/ov7670.c  | 24 +++
 drivers/media/i2c/ov7740.c  | 31 +++-
 drivers/media/i2c/tvp514x.c | 39 +
 drivers/media/i2c/vs6624.c  | 29 +++---
 drivers/media/platform/atmel/atmel-isc.c| 10 ++-
 drivers/media/platform/atmel/atmel-isi.c| 12 ++--
 drivers/media/platform/blackfin/bfin_capture.c  | 14 +++--
 drivers/media/platform/marvell-ccic/mcam-core.c | 12 
 drivers/media/platform/soc_camera/soc_camera.c  | 10 ---
 drivers/media/platform/via-camera.c |  4 +--
 drivers/media/usb/em28xx/em28xx-video.c | 36 +++
 13 files changed, 122 insertions(+), 165 deletions(-)

diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
index 5e29064fae91..3e23c5b0de1f 100644
--- a/drivers/media/i2c/mt9v011.c
+++ b/drivers/media/i2c/mt9v011.c
@@ -364,33 +364,24 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
return 0;
 }
 
-static int mt9v011_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm 
*parms)
+static int mt9v011_g_frame_interval(struct v4l2_subdev *sd,
+   struct v4l2_subdev_frame_interval *ival)
 {
-   struct v4l2_captureparm *cp = >parm.capture;
-
-   if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
-   return -EINVAL;
-
-   memset(cp, 0, sizeof(struct v4l2_captureparm));
-   cp->capability = V4L2_CAP_TIMEPERFRAME;
+   memset(ival->reserved, 0, sizeof(ival->reserved));
calc_fps(sd,
->timeperframe.numerator,
->timeperframe.denominator);
+>interval.numerator,
+>interval.denominator);
 
return 0;
 }
 
-static int mt9v011_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm 
*parms)
+static int mt9v011_s_frame_interval(struct v4l2_subdev *sd,
+   struct v4l2_subdev_frame_interval *ival)
 {
-   struct v4l2_captureparm *cp = >parm.capture;
-   struct v4l2_fract *tpf = >timeperframe;
+   struct v4l2_fract *tpf = >interval;
u16 speed;
 
-   if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
-   return -EINVAL;
-   if (cp->extendedmode != 0)
-   return -EINVAL;
-
+   memset(ival->reserved, 0, sizeof(ival->reserved));
speed = calc_speed(sd, tpf->numerator, tpf->denominator);
 
mt9v011_write(sd, R0A_MT9V011_CLK_SPEED, speed);
@@ -469,8 +460,8 @@ static const struct v4l2_subdev_core_ops mt9v011_core_ops = 
{
 };
 
 static const struct v4l2_subdev_video_ops mt9v011_video_ops = {
-   .g_parm = mt9v011_g_parm,
-   .s_parm = mt9v011_s_parm,
+   .g_frame_interval = mt9v011_g_frame_interval,
+   .s_frame_interval = mt9v011_s_frame_interval,
 };
 
 static const struct v4l2_subdev_pad_ops mt9v011_pad_ops = {
diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
index 8975d16b2b24..3f962dae7534 100644
--- a/drivers/media/i2c/ov6650.c
+++ b/drivers/media/i2c/ov6650.c
@@ -201,7 +201,7 @@ struct ov6650 {
struct v4l2_rectrect;   /* sensor cropping window */
unsigned long   pclk_limit; /* from host */
unsigned long   pclk_max;   /* from resolution and format */
-   struct v4l2_fract   tpf;/* as requested with s_parm */
+   struct v4l2_fract   tpf;/* as requested with 
s_frame_interval */
u32 code;
enum v4l2_colorspacecolorspace;
 };
@@ -723,42 +723,33 @@ static int ov6650_enum_mbus_code(struct v4l2_subdev *sd,
return 0;
 }
 
-static int ov6650_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
+static int ov6650_g_frame_interval(struct v4l2_subdev *sd,
+  struct v4l2_subdev_frame_interval *ival)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov6650 *priv = to_ov6650(client);
-   struct v4l2_captureparm *cp = >parm.capture;
 
-   if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
-   return -EINVAL;
-
-   memset(cp, 0, sizeof(*cp));
-   cp->capability = V4L2_CAP_TIMEPERFRAME;
-   cp->timeperframe.numerator = GET_CLKRC_DIV(to_clkrc(>tpf,
+   memset(ival->reserved, 0, sizeof(ival->reserved));
+   ival->interval.numerator = GET_CLKRC_DIV(to_clkrc(>tpf,
priv->pclk_limit, priv->pclk_max));
-   cp->timeperframe.denominator = FRAME_RATE_MAX;
+   

[PATCHv2 7/9] staging: atomisp: Drop g_parm and s_parm subdev ops use

[Hans Verkuil]

From: Sakari Ailus 

The s_parm and g_parm did nothing. Remove them.

Signed-off-by: Sakari Ailus 
Signed-off-by: Hans Verkuil 
---
 .../staging/media/atomisp/pci/atomisp2/atomisp_file.c| 16 
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c | 14 --
 2 files changed, 30 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_file.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_file.c
index 377ec2a9fa6d..c6d96987561d 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_file.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_file.c
@@ -77,20 +77,6 @@ static int file_input_s_stream(struct v4l2_subdev *sd, int 
enable)
return 0;
 }
 
-static int file_input_g_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   /*to fake*/
-   return 0;
-}
-
-static int file_input_s_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   /*to fake*/
-   return 0;
-}
-
 static int file_input_get_fmt(struct v4l2_subdev *sd,
  struct v4l2_subdev_pad_config *cfg,
  struct v4l2_subdev_format *format)
@@ -166,8 +152,6 @@ static int file_input_enum_frame_ival(struct v4l2_subdev 
*sd,
 
 static const struct v4l2_subdev_video_ops file_input_video_ops = {
.s_stream = file_input_s_stream,
-   .g_parm = file_input_g_parm,
-   .s_parm = file_input_s_parm,
 };
 
 static const struct v4l2_subdev_core_ops file_input_core_ops = {
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
index b71cc7bcdbab..adc900272f6f 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
@@ -27,18 +27,6 @@ static int tpg_s_stream(struct v4l2_subdev *sd, int enable)
return 0;
 }
 
-static int tpg_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *param)
-{
-   /*to fake*/
-   return 0;
-}
-
-static int tpg_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *param)
-{
-   /*to fake*/
-   return 0;
-}
-
 static int tpg_get_fmt(struct v4l2_subdev *sd,
   struct v4l2_subdev_pad_config *cfg,
   struct v4l2_subdev_format *format)
@@ -101,8 +89,6 @@ static int tpg_enum_frame_ival(struct v4l2_subdev *sd,
 
 static const struct v4l2_subdev_video_ops tpg_video_ops = {
.s_stream = tpg_s_stream,
-   .g_parm = tpg_g_parm,
-   .s_parm = tpg_s_parm,
 };
 
 static const struct v4l2_subdev_core_ops tpg_core_ops = {
-- 
2.15.1

[PATCHv2 3/9] staging: atomisp: Kill subdev s_parm abuse

[Hans Verkuil]

From: Sakari Ailus 

Remove sensor driver's interface that made use of use case specific
knowledge of platform capabilities.

Signed-off-by: Sakari Ailus 
Signed-off-by: Hans Verkuil 
---
 drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 26 -
 drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 26 -
 drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 29 -
 drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 26 -
 drivers/staging/media/atomisp/i2c/gc0310.h | 43 --
 drivers/staging/media/atomisp/i2c/gc2235.h |  1 -
 drivers/staging/media/atomisp/i2c/ov2680.h | 68 --
 .../media/atomisp/i2c/ov5693/atomisp-ov5693.c  | 27 -
 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   |  9 +--
 .../media/atomisp/pci/atomisp2/atomisp_subdev.c| 12 +---
 10 files changed, 3 insertions(+), 264 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c 
b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index 61b7598469eb..572c9127c24d 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -1224,37 +1224,12 @@ static int gc0310_g_parm(struct v4l2_subdev *sd,
if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
param->parm.capture.timeperframe.numerator = 1;
-   param->parm.capture.capturemode = dev->run_mode;
param->parm.capture.timeperframe.denominator =
gc0310_res[dev->fmt_idx].fps;
}
return 0;
 }
 
-static int gc0310_s_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   struct gc0310_device *dev = to_gc0310_sensor(sd);
-   dev->run_mode = param->parm.capture.capturemode;
-
-   mutex_lock(>input_lock);
-   switch (dev->run_mode) {
-   case CI_MODE_VIDEO:
-   gc0310_res = gc0310_res_video;
-   N_RES = N_RES_VIDEO;
-   break;
-   case CI_MODE_STILL_CAPTURE:
-   gc0310_res = gc0310_res_still;
-   N_RES = N_RES_STILL;
-   break;
-   default:
-   gc0310_res = gc0310_res_preview;
-   N_RES = N_RES_PREVIEW;
-   }
-   mutex_unlock(>input_lock);
-   return 0;
-}
-
 static int gc0310_g_frame_interval(struct v4l2_subdev *sd,
   struct v4l2_subdev_frame_interval *interval)
 {
@@ -1314,7 +1289,6 @@ static const struct v4l2_subdev_sensor_ops 
gc0310_sensor_ops = {
 static const struct v4l2_subdev_video_ops gc0310_video_ops = {
.s_stream = gc0310_s_stream,
.g_parm = gc0310_g_parm,
-   .s_parm = gc0310_s_parm,
.g_frame_interval = gc0310_g_frame_interval,
 };
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c 
b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index d8de46da64ae..2bc179f3afe5 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -964,37 +964,12 @@ static int gc2235_g_parm(struct v4l2_subdev *sd,
if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
param->parm.capture.timeperframe.numerator = 1;
-   param->parm.capture.capturemode = dev->run_mode;
param->parm.capture.timeperframe.denominator =
gc2235_res[dev->fmt_idx].fps;
}
return 0;
 }
 
-static int gc2235_s_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   struct gc2235_device *dev = to_gc2235_sensor(sd);
-   dev->run_mode = param->parm.capture.capturemode;
-
-   mutex_lock(>input_lock);
-   switch (dev->run_mode) {
-   case CI_MODE_VIDEO:
-   gc2235_res = gc2235_res_video;
-   N_RES = N_RES_VIDEO;
-   break;
-   case CI_MODE_STILL_CAPTURE:
-   gc2235_res = gc2235_res_still;
-   N_RES = N_RES_STILL;
-   break;
-   default:
-   gc2235_res = gc2235_res_preview;
-   N_RES = N_RES_PREVIEW;
-   }
-   mutex_unlock(>input_lock);
-   return 0;
-}
-
 static int gc2235_g_frame_interval(struct v4l2_subdev *sd,
   struct v4l2_subdev_frame_interval *interval)
 {
@@ -1053,7 +1028,6 @@ static const struct v4l2_subdev_sensor_ops 
gc2235_sensor_ops = {
 static const struct v4l2_subdev_video_ops gc2235_video_ops = {
.s_stream = gc2235_s_stream,
.g_parm = gc2235_g_parm,
-   .s_parm = gc2235_s_parm,
.g_frame_interval = gc2235_g_frame_interval,
 };
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c 

[PATCHv2 4/9] staging: atomisp: i2c: Disable non-preview configurations

[Hans Verkuil]

From: Sakari Ailus 

Disable configurations for non-preview modes until configuration selection
is improved.

Signed-off-by: Sakari Ailus 
Signed-off-by: Hans Verkuil 
---
 drivers/staging/media/atomisp/i2c/gc2235.h| 2 ++
 drivers/staging/media/atomisp/i2c/ov2722.h| 2 ++
 drivers/staging/media/atomisp/i2c/ov5693/ov5693.h | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/drivers/staging/media/atomisp/i2c/gc2235.h 
b/drivers/staging/media/atomisp/i2c/gc2235.h
index 45a54fea5466..817c0068c1d3 100644
--- a/drivers/staging/media/atomisp/i2c/gc2235.h
+++ b/drivers/staging/media/atomisp/i2c/gc2235.h
@@ -574,6 +574,7 @@ static struct gc2235_resolution gc2235_res_preview[] = {
 };
 #define N_RES_PREVIEW (ARRAY_SIZE(gc2235_res_preview))
 
+#if 0 /* Disable non-previes configurations for now */
 static struct gc2235_resolution gc2235_res_still[] = {
{
.desc = "gc2235_1600_900_30fps",
@@ -658,6 +659,7 @@ static struct gc2235_resolution gc2235_res_video[] = {
 
 };
 #define N_RES_VIDEO (ARRAY_SIZE(gc2235_res_video))
+#endif
 
 static struct gc2235_resolution *gc2235_res = gc2235_res_preview;
 static unsigned long N_RES = N_RES_PREVIEW;
diff --git a/drivers/staging/media/atomisp/i2c/ov2722.h 
b/drivers/staging/media/atomisp/i2c/ov2722.h
index d8a973d71699..f133439adfd5 100644
--- a/drivers/staging/media/atomisp/i2c/ov2722.h
+++ b/drivers/staging/media/atomisp/i2c/ov2722.h
@@ -1148,6 +1148,7 @@ struct ov2722_resolution ov2722_res_preview[] = {
 };
 #define N_RES_PREVIEW (ARRAY_SIZE(ov2722_res_preview))
 
+#if 0 /* Disable non-previes configurations for now */
 struct ov2722_resolution ov2722_res_still[] = {
{
.desc = "ov2722_480P_30fps",
@@ -1250,6 +1251,7 @@ struct ov2722_resolution ov2722_res_video[] = {
},
 };
 #define N_RES_VIDEO (ARRAY_SIZE(ov2722_res_video))
+#endif
 
 static struct ov2722_resolution *ov2722_res = ov2722_res_preview;
 static unsigned long N_RES = N_RES_PREVIEW;
diff --git a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h 
b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h
index 68cfcb4a6c3c..15a33dcd2d59 100644
--- a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h
+++ b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h
@@ -1147,6 +1147,7 @@ struct ov5693_resolution ov5693_res_preview[] = {
 };
 #define N_RES_PREVIEW (ARRAY_SIZE(ov5693_res_preview))
 
+#if 0 /* Disable non-previes configurations for now */
 struct ov5693_resolution ov5693_res_still[] = {
{
.desc = "ov5693_736x496_30fps",
@@ -1364,6 +1365,7 @@ struct ov5693_resolution ov5693_res_video[] = {
},
 };
 #define N_RES_VIDEO (ARRAY_SIZE(ov5693_res_video))
+#endif
 
 static struct ov5693_resolution *ov5693_res = ov5693_res_preview;
 static unsigned long N_RES = N_RES_PREVIEW;
-- 
2.15.1

Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs

[Hans Verkuil]

On 22/01/18 11:59, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Jan 22, 2018 at 11:33:28AM +0100, Hans Verkuil wrote:
>> On 22/01/18 11:26, Sakari Ailus wrote:
>>> Hi Hans,
>>>
>>> On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
 From: Hans Verkuil 

 Convert all g/s_parm calls to g/s_frame_interval. This allows us
 to remove the g/s_parm ops since those are a duplicate of
 g/s_frame_interval.

 Signed-off-by: Hans Verkuil 
 ---
  drivers/media/i2c/mt9v011.c | 29 +--
  drivers/media/i2c/ov6650.c  | 33 
 ++
  drivers/media/i2c/ov7670.c  | 26 +
  drivers/media/i2c/ov7740.c  | 29 ---
  drivers/media/i2c/tvp514x.c | 37 
 +++--
  drivers/media/i2c/vs6624.c  | 29 ++-
  drivers/media/platform/atmel/atmel-isc.c| 10 ++-
  drivers/media/platform/atmel/atmel-isi.c| 12 ++--
  drivers/media/platform/blackfin/bfin_capture.c  | 14 +++---
  drivers/media/platform/marvell-ccic/mcam-core.c | 12 
  drivers/media/platform/soc_camera/soc_camera.c  | 10 ---
  drivers/media/platform/via-camera.c |  4 +--
  drivers/media/usb/em28xx/em28xx-video.c | 36 
 
  13 files changed, 137 insertions(+), 144 deletions(-)

 diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
 index 5e29064fae91..0e0bcc8b67ca 100644
 --- a/drivers/media/i2c/mt9v011.c
 +++ b/drivers/media/i2c/mt9v011.c
 @@ -364,33 +364,30 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
return 0;
  }
  
 -static int mt9v011_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm 
 *parms)
 +static int mt9v011_g_frame_interval(struct v4l2_subdev *sd,
 +  struct v4l2_subdev_frame_interval *ival)
  {
 -  struct v4l2_captureparm *cp = >parm.capture;
 -
 -  if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 +  if (ival->pad)
return -EINVAL;
>>>
>>> The pad number checks are already present in v4l2-subdev.c. Do you think
>>> we'll need them in drivers as well?
>>>
>>> It's true that another driver could mis-use this interface. In that case
>>> I'd introduce a wrapper to the op rather than introduce the check in every
>>> driver.
>>
>> I'm not that keen on introducing wrappers for an op. I wouldn't actually know
>> how to implement that cleanly. Since the pad check is subdev driver specific,
>> and the overhead of a wrapper is almost certainly higher than just doing this
>> check I feel it is OK to do this.
> 
> For a majority of drivers, the check is that the pad must be a valid
> pad in an entity. The case where the overhead could matter (it's just a
> single if) is when called through an IOCTL from the user space. And the
> subdevices' IOCTL handler already performs this check. The wrapper would
> simply extend the check to other drivers calling the op.
> 
> It's easy to miss such checks in drivers. We've had quite a few such cases
> in the past. Performing the check universally would make the framework more
> robust.
> 
> What comes to this patchset, I'd omit the pad number check as other drivers
> don't do such checks either --- unless the check is more strict than what
> the interface allows.

I'll remove the check.

Hans

> 
>>
>>>
  
 -  memset(cp, 0, sizeof(struct v4l2_captureparm));
 -  cp->capability = V4L2_CAP_TIMEPERFRAME;
 +  memset(ival->reserved, 0, sizeof(ival->reserved));
calc_fps(sd,
 -   >timeperframe.numerator,
 -   >timeperframe.denominator);
 +   >interval.numerator,
 +   >interval.denominator);
  
return 0;
  }
>>
>> Regards,
>>
>>  Hans
>>
> 

Re: [Patch v6 00/12] Add MFC v10.10 support

[Hans Verkuil]

Hi Smitha,

Thank you for this v6 series!

You can add my:

Acked-by: Hans Verkuil 

to patches 1-9 and 11. See my review for patches 10 and 12. The comments
are minor, so I hope I can Ack v7 once it's posted and this can be merged
for 4.17.

Regards,

Hans

On 08/12/17 10:08, Smitha T Murthy wrote:
> This patch series adds MFC v10.10 support. MFC v10.10 is used in some
> of Exynos7 variants.
> 
> This adds support for following:
> 
> * Add support for HEVC encoder and decoder
> * Add support for VP9 decoder
> * Update Documentation for control id definitions
> * Update computation of min scratch buffer size requirement for V8 onwards
> 
> Changes since v5:
>  - Addressed review comments by Kamil Debski .
>  - Addressed review comments by 
>Stanimir Varbanov .
>  - Addressed review comments by Hans Verkuil .
>  - Rebased on latest git://linuxtv.org/snawrocki/samsung.git
>for-v4.15/media/next.
>  - Applied r-o-b from Andrzej, Stanimir on respective patches.
>  - Applied acked-by from Kamil, Hans on respective patches.
> 
> Smitha T Murthy (12):
>   [media] s5p-mfc: Rename IS_MFCV8 macro
>   [media] s5p-mfc: Adding initial support for MFC v10.10
>   [media] s5p-mfc: Use min scratch buffer size as provided by F/W
>   [media] s5p-mfc: Support MFCv10.10 buffer requirements
>   [media] videodev2.h: Add v4l2 definition for HEVC
>   [media] v4l2-ioctl: add HEVC format description
>   Documentation: v4l: Documentation for HEVC v4l2 definition
>   [media] s5p-mfc: Add support for HEVC decoder
>   [media] s5p-mfc: Add VP9 decoder support
>   [media] v4l2: Add v4l2 control IDs for HEVC encoder
>   [media] s5p-mfc: Add support for HEVC encoder
>   Documention: v4l: Documentation for HEVC CIDs
> 
>  .../devicetree/bindings/media/s5p-mfc.txt  |   1 +
>  Documentation/media/uapi/v4l/extended-controls.rst | 395 +++
>  Documentation/media/uapi/v4l/pixfmt-compressed.rst |   5 +
>  drivers/media/platform/s5p-mfc/regs-mfc-v10.h  |  88 
>  drivers/media/platform/s5p-mfc/regs-mfc-v8.h   |   2 +
>  drivers/media/platform/s5p-mfc/s5p_mfc.c   |  28 ++
>  drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c|   9 +
>  drivers/media/platform/s5p-mfc/s5p_mfc_common.h|  68 ++-
>  drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c  |   6 +-
>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c   |  48 +-
>  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c   | 555 
> -
>  drivers/media/platform/s5p-mfc/s5p_mfc_opr.h   |  14 +
>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c| 397 +--
>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h|  15 +
>  drivers/media/v4l2-core/v4l2-ctrls.c   | 118 +
>  drivers/media/v4l2-core/v4l2-ioctl.c   |   1 +
>  include/uapi/linux/v4l2-controls.h |  92 +++-
>  include/uapi/linux/videodev2.h |   1 +
>  18 files changed, 1765 insertions(+), 78 deletions(-)
>  create mode 100644 drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> 

Re: [Patch v6 12/12] Documention: v4l: Documentation for HEVC CIDs

[Hans Verkuil]

On 08/12/17 10:08, Smitha T Murthy wrote:
> Added V4l2 controls for HEVC encoder
> 
> Signed-off-by: Smitha T Murthy 
> ---
>  Documentation/media/uapi/v4l/extended-controls.rst | 395 
> +
>  1 file changed, 395 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst 
> b/Documentation/media/uapi/v4l/extended-controls.rst
> index a3e81c1..3c92763 100644
> --- a/Documentation/media/uapi/v4l/extended-controls.rst
> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> @@ -1960,6 +1960,401 @@ enum v4l2_vp8_golden_frame_sel -
>  1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
>  
>  
> +High Efficiency Video Coding (HEVC/H.265) Control Reference
> +---
> +
> +The HEVC/H.265 controls include controls for encoding parameters of 
> HEVC/H.265
> +video codec.
> +
> +
> +.. _hevc-control-id:
> +
> +HEVC/H.265 Control IDs
> +^^
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (integer)``
> +Minimum quantization parameter for HEVC.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP (integer)``
> +Maximum quantization parameter for HEVC.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP (integer)``
> +Quantization parameter for an I frame for HEVC.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP (integer)``
> +Quantization parameter for a P frame for HEVC.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_QP (integer)``
> +Quantization parameter for a B frame for HEVC.

I assume these values all have to be in the range MIN_QP to MAX_QP?
If so, then this should be documented I think.

> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_QP (boolean)``
> +HIERARCHICAL_QP allows host to specify the quantization parameter values

host -> the host

> +for each temporal layer through HIERARCHICAL_QP_LAYER. This is valid only
> +if HIERARCHICAL_CODING_LAYER is greater than 1. Setting the control value
> +to 1 enables setting of the QP values for the layers.
> +
> +.. _v4l2-hevc-hier-coding-type:
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_TYPE``
> +(enum)
> +
> +enum v4l2_mpeg_video_hevc_hier_coding_type -
> +Selects the hierarchical coding type for encoding. Possible values are:
> +
> +.. raw:: latex
> +
> +\begin{adjustbox}{width=\columnwidth}
> +
> +.. tabularcolumns:: |p{11.0cm}|p{10.0cm}|
> +
> +.. flat-table::
> +:header-rows:  0
> +:stub-columns: 0
> +
> +* - ``V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_B``
> +  - Use the B frame for hierarchical coding.
> +* - ``V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_P``
> +  - Use the P frame for hierarchical coding.
> +
> +.. raw:: latex
> +
> +\end{adjustbox}
> +
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER (integer)``
> +Selects the hierarchical coding layer. In normal encoding
> +(non-hierarchial coding), it should be zero. Possible values are 0 ~ 6.

Use - instead of ~. Or just say: [0, 6]

> +0 indicates HIERARCHICAL CODING LAYER 0, 1 indicates HIERARCHICAL CODING
> +LAYER 1 and so on.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L0_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer 0.
> +For HEVC it can have a value of 0-51.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L1_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer 1.
> +For HEVC it can have a value of 0-51.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L2_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer 2.
> +For HEVC it can have a value of 0-51.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L3_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer 3.
> +For HEVC it can have a value of 0-51.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L4_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer 4.
> +For HEVC it can have a value of 0-51.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L5_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer 5.
> +For HEVC it can have a value of 0-51.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer 6.
> +For HEVC it can have a value of 0-51.

How do these controls relate to MIN_QP and MAX_QP?

> +
> +.. _v4l2-hevc-profile:
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_PROFILE``
> +(enum)
> +
> +enum v4l2_mpeg_video_hevc_profile -
> +Select the desired profile for HEVC encoder.
> +
> +.. raw:: latex
> +
> +\begin{adjustbox}{width=\columnwidth}
> +
> +.. tabularcolumns:: |p{11.0cm}|p{10.0cm}|
> +
> +.. flat-table::
> +:header-rows:  0
> +:stub-columns: 0
> +
> +* - ``V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN``
> +  - Main profile.
> +* - ``V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN10``
> +  - Main 10 profile.
> +* - 

Re: [Patch v6 10/12] [media] v4l2: Add v4l2 control IDs for HEVC encoder

[Hans Verkuil]

On 08/12/17 10:08, Smitha T Murthy wrote:
> Add v4l2 controls for HEVC encoder
> 
> Signed-off-by: Smitha T Murthy 
> Reviewed-by: Andrzej Hajda 
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 118 
> +++
>  include/uapi/linux/v4l2-controls.h   |  92 ++-
>  2 files changed, 209 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 4e53a86..3f98318 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -480,6 +480,56 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>   NULL,
>   };
>  
> + static const char * const hevc_profile[] = {
> + "Main",
> + "Main Still Picture",
> + NULL,
> + };
> + static const char * const hevc_level[] = {
> + "1",
> + "2",
> + "2.1",
> + "3",
> + "3.1",
> + "4",
> + "4.1",
> + "5",
> + "5.1",
> + "5.2",
> + "6",
> + "6.1",
> + "6.2",
> + NULL,
> + };
> + static const char * const hevc_hierarchial_coding_type[] = {
> + "B",
> + "P",
> + NULL,
> + };
> + static const char * const hevc_refresh_type[] = {
> + "None",
> + "CRA",
> + "IDR",
> + NULL,
> + };
> + static const char * const hevc_size_of_length_field[] = {
> + "0",
> + "1",
> + "2",
> + "4",
> + NULL,
> + };
> + static const char * const hevc_tier_flag[] = {
> + "Main",
> + "High",
> + NULL,
> + };
> + static const char * const hevc_loop_filter_mode[] = {
> + "Disabled",
> + "Enabled",
> + "Disabled at slice boundary",
> + "NULL",
> + };
>  
>   switch (id) {
>   case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
> @@ -575,6 +625,20 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>   return dv_it_content_type;
>   case V4L2_CID_DETECT_MD_MODE:
>   return detect_md_mode;
> + case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
> + return hevc_profile;
> + case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
> + return hevc_level;
> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_TYPE:
> + return hevc_hierarchial_coding_type;
> + case V4L2_CID_MPEG_VIDEO_HEVC_REFRESH_TYPE:
> + return hevc_refresh_type;
> + case V4L2_CID_MPEG_VIDEO_HEVC_SIZE_OF_LENGTH_FIELD:
> + return hevc_size_of_length_field;
> + case V4L2_CID_MPEG_VIDEO_HEVC_TIER_FLAG:
> + return hevc_tier_flag;
> + case V4L2_CID_MPEG_VIDEO_HEVC_LOOP_FILTER_MODE:
> + return hevc_loop_filter_mode;
>  
>   default:
>   return NULL;
> @@ -776,6 +840,53 @@ const char *v4l2_ctrl_get_name(u32 id)
>   case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP:return "VPX 
> P-Frame QP Value";
>   case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:   return "VPX 
> Profile";
>  
> + /* HEVC controls */
> + case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:   return "HEVC 
> I-Frame QP Value";
> + case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP:   return "HEVC 
> P-Frame QP Value";
> + case V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_QP:   return "HEVC 
> B-Frame QP Value";
> + case V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP:   return "HEVC 
> Minimum QP Value";
> + case V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP:   return "HEVC 
> Maximum QP Value";
> + case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:  return "HEVC 
> Profile";
> + case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:return "HEVC 
> Level";
> + case V4L2_CID_MPEG_VIDEO_HEVC_TIER_FLAG:return "HEVC 
> Tier Flag";
> + case V4L2_CID_MPEG_VIDEO_HEVC_FRAME_RATE_RESOLUTION:return "HEVC 
> Frame Rate Resolution";
> + case V4L2_CID_MPEG_VIDEO_HEVC_MAX_PARTITION_DEPTH:  return "HEVC 
> Maximum Coding Unit Depth";
> + case V4L2_CID_MPEG_VIDEO_HEVC_REFRESH_TYPE: return "HEVC 
> Refresh Type";
> + case V4L2_CID_MPEG_VIDEO_HEVC_CONST_INTRA_PRED: return "HEVC 
> Constant Intra Prediction";
> + case V4L2_CID_MPEG_VIDEO_HEVC_LOSSLESS_CU:  return "HEVC 
> Lossless Encoding";
> + case V4L2_CID_MPEG_VIDEO_HEVC_WAVEFRONT:return "HEVC 
> Wavefront";
> + case V4L2_CID_MPEG_VIDEO_HEVC_LOOP_FILTER_MODE: return "HEVC 
> Loop Filter";
> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_QP:  return "HEVC QP 
> Values";
> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_TYPE: return "HEVC 
> Hierarchical Coding 

Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs

[Sakari Ailus]

Hi Hans,

On Mon, Jan 22, 2018 at 11:33:28AM +0100, Hans Verkuil wrote:
> On 22/01/18 11:26, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
> >> From: Hans Verkuil 
> >>
> >> Convert all g/s_parm calls to g/s_frame_interval. This allows us
> >> to remove the g/s_parm ops since those are a duplicate of
> >> g/s_frame_interval.
> >>
> >> Signed-off-by: Hans Verkuil 
> >> ---
> >>  drivers/media/i2c/mt9v011.c | 29 +--
> >>  drivers/media/i2c/ov6650.c  | 33 
> >> ++
> >>  drivers/media/i2c/ov7670.c  | 26 +
> >>  drivers/media/i2c/ov7740.c  | 29 ---
> >>  drivers/media/i2c/tvp514x.c | 37 
> >> +++--
> >>  drivers/media/i2c/vs6624.c  | 29 ++-
> >>  drivers/media/platform/atmel/atmel-isc.c| 10 ++-
> >>  drivers/media/platform/atmel/atmel-isi.c| 12 ++--
> >>  drivers/media/platform/blackfin/bfin_capture.c  | 14 +++---
> >>  drivers/media/platform/marvell-ccic/mcam-core.c | 12 
> >>  drivers/media/platform/soc_camera/soc_camera.c  | 10 ---
> >>  drivers/media/platform/via-camera.c |  4 +--
> >>  drivers/media/usb/em28xx/em28xx-video.c | 36 
> >> 
> >>  13 files changed, 137 insertions(+), 144 deletions(-)
> >>
> >> diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
> >> index 5e29064fae91..0e0bcc8b67ca 100644
> >> --- a/drivers/media/i2c/mt9v011.c
> >> +++ b/drivers/media/i2c/mt9v011.c
> >> @@ -364,33 +364,30 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
> >>return 0;
> >>  }
> >>  
> >> -static int mt9v011_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm 
> >> *parms)
> >> +static int mt9v011_g_frame_interval(struct v4l2_subdev *sd,
> >> +  struct v4l2_subdev_frame_interval *ival)
> >>  {
> >> -  struct v4l2_captureparm *cp = >parm.capture;
> >> -
> >> -  if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> >> +  if (ival->pad)
> >>return -EINVAL;
> > 
> > The pad number checks are already present in v4l2-subdev.c. Do you think
> > we'll need them in drivers as well?
> > 
> > It's true that another driver could mis-use this interface. In that case
> > I'd introduce a wrapper to the op rather than introduce the check in every
> > driver.
> 
> I'm not that keen on introducing wrappers for an op. I wouldn't actually know
> how to implement that cleanly. Since the pad check is subdev driver specific,
> and the overhead of a wrapper is almost certainly higher than just doing this
> check I feel it is OK to do this.

For a majority of drivers, the check is that the pad must be a valid
pad in an entity. The case where the overhead could matter (it's just a
single if) is when called through an IOCTL from the user space. And the
subdevices' IOCTL handler already performs this check. The wrapper would
simply extend the check to other drivers calling the op.

It's easy to miss such checks in drivers. We've had quite a few such cases
in the past. Performing the check universally would make the framework more
robust.

What comes to this patchset, I'd omit the pad number check as other drivers
don't do such checks either --- unless the check is more strict than what
the interface allows.

> 
> > 
> >>  
> >> -  memset(cp, 0, sizeof(struct v4l2_captureparm));
> >> -  cp->capability = V4L2_CAP_TIMEPERFRAME;
> >> +  memset(ival->reserved, 0, sizeof(ival->reserved));
> >>calc_fps(sd,
> >> -   >timeperframe.numerator,
> >> -   >timeperframe.denominator);
> >> +   >interval.numerator,
> >> +   >interval.denominator);
> >>  
> >>return 0;
> >>  }
> 
> Regards,
> 
>   Hans
> 

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

[PATCH] media: ov5640: add JPEG support

[Hugues Fruchet]

Add YUV422 encoded JPEG support.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/i2c/ov5640.c | 82 --
 1 file changed, 80 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index e2dd352..db9aeeb 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -34,6 +35,10 @@
 
 #define OV5640_DEFAULT_SLAVE_ID 0x3c
 
+#define OV5640_JPEG_SIZE_MAX (5 * SZ_1M)
+
+#define OV5640_REG_SYS_RESET02 0x3002
+#define OV5640_REG_SYS_CLOCK_ENABLE02  0x3006
 #define OV5640_REG_SYS_CTRL0   0x3008
 #define OV5640_REG_CHIP_ID 0x300a
 #define OV5640_REG_IO_MIPI_CTRL00  0x300e
@@ -114,6 +119,7 @@ struct ov5640_pixfmt {
 };
 
 static const struct ov5640_pixfmt ov5640_formats[] = {
+   { MEDIA_BUS_FMT_JPEG_1X8, V4L2_COLORSPACE_JPEG, },
{ MEDIA_BUS_FMT_UYVY8_2X8, V4L2_COLORSPACE_SRGB, },
{ MEDIA_BUS_FMT_YUYV8_2X8, V4L2_COLORSPACE_SRGB, },
{ MEDIA_BUS_FMT_RGB565_2X8_LE, V4L2_COLORSPACE_SRGB, },
@@ -220,6 +226,8 @@ struct ov5640_dev {
 
bool pending_mode_change;
bool streaming;
+
+   unsigned int jpeg_size;
 };
 
 static inline struct ov5640_dev *to_ov5640_dev(struct v4l2_subdev *sd)
@@ -1910,11 +1918,51 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
return ret;
 }
 
+static int ov5640_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+struct v4l2_mbus_frame_desc *fd)
+{
+   struct ov5640_dev *sensor = to_ov5640_dev(sd);
+
+   if (pad != 0 || !fd)
+   return -EINVAL;
+
+   mutex_lock(>lock);
+   fd->entry[0].length = sensor->jpeg_size;
+   fd->entry[0].pixelcode = MEDIA_BUS_FMT_JPEG_1X8;
+   mutex_unlock(>lock);
+
+   fd->entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
+   fd->num_entries = 1;
+
+   return 0;
+}
+
+static int ov5640_set_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+struct v4l2_mbus_frame_desc *fd)
+{
+   struct ov5640_dev *sensor = to_ov5640_dev(sd);
+
+   if (pad != 0 || !fd)
+   return -EINVAL;
+
+   fd->entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
+   fd->num_entries = 1;
+   fd->entry[0].length = clamp_t(u32, fd->entry[0].length,
+ sensor->fmt.width * sensor->fmt.height,
+ OV5640_JPEG_SIZE_MAX);
+   mutex_lock(>lock);
+   sensor->jpeg_size = fd->entry[0].length;
+   mutex_unlock(>lock);
+
+   return 0;
+}
+
 static int ov5640_set_framefmt(struct ov5640_dev *sensor,
   struct v4l2_mbus_framefmt *format)
 {
int ret = 0;
bool is_rgb = false;
+   bool is_jpeg = false;
u8 val;
 
switch (format->code) {
@@ -1936,6 +1984,11 @@ static int ov5640_set_framefmt(struct ov5640_dev *sensor,
val = 0x61;
is_rgb = true;
break;
+   case MEDIA_BUS_FMT_JPEG_1X8:
+   /* YUV422, YUYV */
+   val = 0x30;
+   is_jpeg = true;
+   break;
default:
return -EINVAL;
}
@@ -1946,8 +1999,31 @@ static int ov5640_set_framefmt(struct ov5640_dev *sensor,
return ret;
 
/* FORMAT MUX CONTROL: ISP YUV or RGB */
-   return ov5640_write_reg(sensor, OV5640_REG_ISP_FORMAT_MUX_CTRL,
-   is_rgb ? 0x01 : 0x00);
+   ret = ov5640_write_reg(sensor, OV5640_REG_ISP_FORMAT_MUX_CTRL,
+  is_rgb ? 0x01 : 0x00);
+   if (ret)
+   return ret;
+
+   if (is_jpeg) {
+   /* Enable jpeg */
+   ret = ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
+BIT(5), BIT(5));
+   if (ret)
+   return ret;
+
+   /* Relax reset of all blocks */
+   ret = ov5640_write_reg(sensor, OV5640_REG_SYS_RESET02, 0x00);
+   if (ret)
+   return ret;
+
+   /* Clock all blocks */
+   ret = ov5640_write_reg(sensor, OV5640_REG_SYS_CLOCK_ENABLE02,
+  0xFF);
+   if (ret)
+   return ret;
+   }
+
+   return ret;
 }
 
 /*
@@ -2391,6 +2467,8 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int 
enable)
.set_fmt = ov5640_set_fmt,
.enum_frame_size = ov5640_enum_frame_size,
.enum_frame_interval = ov5640_enum_frame_interval,
+   .get_frame_desc = ov5640_get_frame_desc,
+   .set_frame_desc = ov5640_set_frame_desc,
 };
 
 static const struct v4l2_subdev_ops ov5640_subdev_ops = {
-- 
1.9.1

Re: [PATCH 9/9] vidioc-g-parm.rst: also allow _MPLANE buffer types

[Hans Verkuil]

On 22/01/18 11:26, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Jan 22, 2018 at 11:18:57AM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> The specification mentions that type can be V4L2_BUF_TYPE_VIDEO_CAPTURE,
>> but the v4l2 core implementation also allows the _MPLANE variant.
>>
>> Document this.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  Documentation/media/uapi/v4l/vidioc-g-parm.rst | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/media/uapi/v4l/vidioc-g-parm.rst 
>> b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
>> index 616a5ea3f8fa..a941526cfeb4 100644
>> --- a/Documentation/media/uapi/v4l/vidioc-g-parm.rst
>> +++ b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
>> @@ -66,7 +66,7 @@ union holding separate parameters for input and output 
>> devices.
>>-
>>- The buffer (stream) type, same as struct
>>  :c:type:`v4l2_format` ``type``, set by the
>> -application. See :c:type:`v4l2_buf_type`
>> +application. See :c:type:`v4l2_buf_type`.
>>  * - union
>>- ``parm``
>>-
>> @@ -75,12 +75,12 @@ union holding separate parameters for input and output 
>> devices.
>>- struct :c:type:`v4l2_captureparm`
>>- ``capture``
>>- Parameters for capture devices, used when ``type`` is
>> -``V4L2_BUF_TYPE_VIDEO_CAPTURE``.
>   ``V4L2_BUF_TYPE_VIDEO_CAPTURE`` or 
> ``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE``.
> 
> Wrap this one? It's over 80...

OK, done.

Hans

> 
>>  * -
>>- struct :c:type:`v4l2_outputparm`
>>- ``output``
>>- Parameters for output devices, used when ``type`` is
>> -``V4L2_BUF_TYPE_VIDEO_OUTPUT``.
>> +``V4L2_BUF_TYPE_VIDEO_OUTPUT`` or ``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE``.
>>  * -
>>- __u8
>>- ``raw_data``\ [200]
> 

[PATCH] [media] s3c-camif: array underflow in __camif_subdev_try_format()

[Dan Carpenter]

The while loop is a post op, "while (i-- >= 0)" so the last iteration
will read camif_mbus_formats[-1] and then the loop will exit with "i"
set to -2 and so we do: "mf->code = camif_mbus_formats[-2];".

I've changed it to a pre-op, I've added a check to ensure we found the
right format and I've removed the "mf->code = camif_mbus_formats[i];"
because that is a no-op anyway.

Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series 
camera interface")
Signed-off-by: Dan Carpenter 

diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
b/drivers/media/platform/s3c-camif/camif-capture.c
index 437395a61065..012f4b389c55 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -1261,11 +1261,11 @@ static void __camif_subdev_try_format(struct camif_dev 
*camif,
/* FIXME: constraints against codec or preview path ? */
pix_lim = >vp_pix_limits[VP_CODEC];
 
-   while (i-- >= 0)
+   while (--i >= 0)
if (camif_mbus_formats[i] == mf->code)
break;
-
-   mf->code = camif_mbus_formats[i];
+   if (i < 0)
+   return;
 
if (pad == CAMIF_SD_PAD_SINK) {
v4l_bound_align_image(>width, 8, CAMIF_MAX_PIX_WIDTH,

Re: [PATCH 1/9] v4l2-common: create v4l2_g/s_parm_cap helpers

[Hans Verkuil]

On 22/01/18 11:28, Sakari Ailus wrote:
> On Mon, Jan 22, 2018 at 11:18:49AM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Create helpers to handle VIDIOC_G/S_PARM by querying the
>> g/s_frame_interval v4l2_subdev ops.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/v4l2-core/v4l2-common.c | 49 
>> +++
>>  include/media/v4l2-common.h   | 26 +++
>>  2 files changed, 75 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-common.c 
>> b/drivers/media/v4l2-core/v4l2-common.c
>> index 8650ad92b64d..4e371ae4aed7 100644
>> --- a/drivers/media/v4l2-core/v4l2-common.c
>> +++ b/drivers/media/v4l2-core/v4l2-common.c
>> @@ -392,3 +392,52 @@ void v4l2_get_timestamp(struct timeval *tv)
>>  tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>>  }
>>  EXPORT_SYMBOL_GPL(v4l2_get_timestamp);
>> +
>> +int v4l2_g_parm_cap(struct video_device *vdev,
>> +struct v4l2_subdev *sd, struct v4l2_streamparm *a)
>> +{
>> +struct v4l2_subdev_frame_interval ival = { 0 };
>> +int ret;
>> +
>> +if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
>> +a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
>> +return -EINVAL;
>> +
>> +if (vdev->device_caps & V4L2_CAP_READWRITE)
>> +a->parm.capture.readbuffers = 2;
>> +if (v4l2_subdev_has_op(sd, video, g_frame_interval))
>> +a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
>> +ret = v4l2_subdev_call(sd, video, g_frame_interval, );
>> +if (!ret)
>> +a->parm.capture.timeperframe = ival.interval;
>> +return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_g_parm_cap);
>> +
>> +int v4l2_s_parm_cap(struct video_device *vdev,
>> +struct v4l2_subdev *sd, struct v4l2_streamparm *a)
>> +{
>> +struct v4l2_subdev_frame_interval ival = {
>> +0,
>> +a->parm.capture.timeperframe
> 
> I'd explicitly specify which members are assigned here. It looks cleaner
> and does not depend on field order in the struct definition. It won't be
> changed as it's part of uAPI but then again people take examples from
> existing code and could do this where it's not safe.

True. I'll change this.

Regards,

Hans

Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs

[Hans Verkuil]

On 22/01/18 11:26, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Convert all g/s_parm calls to g/s_frame_interval. This allows us
>> to remove the g/s_parm ops since those are a duplicate of
>> g/s_frame_interval.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/i2c/mt9v011.c | 29 +--
>>  drivers/media/i2c/ov6650.c  | 33 ++
>>  drivers/media/i2c/ov7670.c  | 26 +
>>  drivers/media/i2c/ov7740.c  | 29 ---
>>  drivers/media/i2c/tvp514x.c | 37 
>> +++--
>>  drivers/media/i2c/vs6624.c  | 29 ++-
>>  drivers/media/platform/atmel/atmel-isc.c| 10 ++-
>>  drivers/media/platform/atmel/atmel-isi.c| 12 ++--
>>  drivers/media/platform/blackfin/bfin_capture.c  | 14 +++---
>>  drivers/media/platform/marvell-ccic/mcam-core.c | 12 
>>  drivers/media/platform/soc_camera/soc_camera.c  | 10 ---
>>  drivers/media/platform/via-camera.c |  4 +--
>>  drivers/media/usb/em28xx/em28xx-video.c | 36 
>> 
>>  13 files changed, 137 insertions(+), 144 deletions(-)
>>
>> diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
>> index 5e29064fae91..0e0bcc8b67ca 100644
>> --- a/drivers/media/i2c/mt9v011.c
>> +++ b/drivers/media/i2c/mt9v011.c
>> @@ -364,33 +364,30 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
>>  return 0;
>>  }
>>  
>> -static int mt9v011_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm 
>> *parms)
>> +static int mt9v011_g_frame_interval(struct v4l2_subdev *sd,
>> +struct v4l2_subdev_frame_interval *ival)
>>  {
>> -struct v4l2_captureparm *cp = >parm.capture;
>> -
>> -if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>> +if (ival->pad)
>>  return -EINVAL;
> 
> The pad number checks are already present in v4l2-subdev.c. Do you think
> we'll need them in drivers as well?
> 
> It's true that another driver could mis-use this interface. In that case
> I'd introduce a wrapper to the op rather than introduce the check in every
> driver.

I'm not that keen on introducing wrappers for an op. I wouldn't actually know
how to implement that cleanly. Since the pad check is subdev driver specific,
and the overhead of a wrapper is almost certainly higher than just doing this
check I feel it is OK to do this.

> 
>>  
>> -memset(cp, 0, sizeof(struct v4l2_captureparm));
>> -cp->capability = V4L2_CAP_TIMEPERFRAME;
>> +memset(ival->reserved, 0, sizeof(ival->reserved));
>>  calc_fps(sd,
>> - >timeperframe.numerator,
>> - >timeperframe.denominator);
>> + >interval.numerator,
>> + >interval.denominator);
>>  
>>  return 0;
>>  }

Regards,

Hans

Re: [PATCH 1/9] v4l2-common: create v4l2_g/s_parm_cap helpers

[Sakari Ailus]

On Mon, Jan 22, 2018 at 11:18:49AM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Create helpers to handle VIDIOC_G/S_PARM by querying the
> g/s_frame_interval v4l2_subdev ops.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 49 
> +++
>  include/media/v4l2-common.h   | 26 +++
>  2 files changed, 75 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c 
> b/drivers/media/v4l2-core/v4l2-common.c
> index 8650ad92b64d..4e371ae4aed7 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -392,3 +392,52 @@ void v4l2_get_timestamp(struct timeval *tv)
>   tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_get_timestamp);
> +
> +int v4l2_g_parm_cap(struct video_device *vdev,
> + struct v4l2_subdev *sd, struct v4l2_streamparm *a)
> +{
> + struct v4l2_subdev_frame_interval ival = { 0 };
> + int ret;
> +
> + if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> + a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> + return -EINVAL;
> +
> + if (vdev->device_caps & V4L2_CAP_READWRITE)
> + a->parm.capture.readbuffers = 2;
> + if (v4l2_subdev_has_op(sd, video, g_frame_interval))
> + a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> + ret = v4l2_subdev_call(sd, video, g_frame_interval, );
> + if (!ret)
> + a->parm.capture.timeperframe = ival.interval;
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_g_parm_cap);
> +
> +int v4l2_s_parm_cap(struct video_device *vdev,
> + struct v4l2_subdev *sd, struct v4l2_streamparm *a)
> +{
> + struct v4l2_subdev_frame_interval ival = {
> + 0,
> + a->parm.capture.timeperframe

I'd explicitly specify which members are assigned here. It looks cleaner
and does not depend on field order in the struct definition. It won't be
changed as it's part of uAPI but then again people take examples from
existing code and could do this where it's not safe.

> + };
> + int ret;
> +
> + if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> + a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> + return -EINVAL;
> +
> + memset(>parm, 0, sizeof(a->parm));
> + if (vdev->device_caps & V4L2_CAP_READWRITE)
> + a->parm.capture.readbuffers = 2;
> + else
> + a->parm.capture.readbuffers = 0;
> +
> + if (v4l2_subdev_has_op(sd, video, g_frame_interval))
> + a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> + ret = v4l2_subdev_call(sd, video, s_frame_interval, );
> + if (!ret)
> + a->parm.capture.timeperframe = ival.interval;
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_s_parm_cap);
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index e0d95a7c5d48..f3aa1d728c0b 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -341,4 +341,30 @@ v4l2_find_nearest_format(const struct 
> v4l2_frmsize_discrete *sizes,
>   */
>  void v4l2_get_timestamp(struct timeval *tv);
>  
> +/**
> + * v4l2_g_parm_cap - helper routine for vidioc_g_parm to fill this in by
> + *  calling the g_frame_interval op of the given subdev. It only works
> + *  for V4L2_BUF_TYPE_VIDEO_CAPTURE(_MPLANE), hence the _cap in the
> + *  function name.
> + *
> + * @vdev: the struct video_device pointer. Used to determine the device caps.
> + * @sd: the sub-device pointer.
> + * @a: the VIDIOC_G_PARM argument.
> + */
> +int v4l2_g_parm_cap(struct video_device *vdev,
> + struct v4l2_subdev *sd, struct v4l2_streamparm *a);
> +
> +/**
> + * v4l2_s_parm_cap - helper routine for vidioc_s_parm to fill this in by
> + *  calling the s_frame_interval op of the given subdev. It only works
> + *  for V4L2_BUF_TYPE_VIDEO_CAPTURE(_MPLANE), hence the _cap in the
> + *  function name.
> + *
> + * @vdev: the struct video_device pointer. Used to determine the device caps.
> + * @sd: the sub-device pointer.
> + * @a: the VIDIOC_S_PARM argument.
> + */
> +int v4l2_s_parm_cap(struct video_device *vdev,
> + struct v4l2_subdev *sd, struct v4l2_streamparm *a);
> +
>  #endif /* V4L2_COMMON_H_ */

-- 
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com

Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs

[Sakari Ailus]

Hi Hans,

On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Convert all g/s_parm calls to g/s_frame_interval. This allows us
> to remove the g/s_parm ops since those are a duplicate of
> g/s_frame_interval.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/i2c/mt9v011.c | 29 +--
>  drivers/media/i2c/ov6650.c  | 33 ++
>  drivers/media/i2c/ov7670.c  | 26 +
>  drivers/media/i2c/ov7740.c  | 29 ---
>  drivers/media/i2c/tvp514x.c | 37 
> +++--
>  drivers/media/i2c/vs6624.c  | 29 ++-
>  drivers/media/platform/atmel/atmel-isc.c| 10 ++-
>  drivers/media/platform/atmel/atmel-isi.c| 12 ++--
>  drivers/media/platform/blackfin/bfin_capture.c  | 14 +++---
>  drivers/media/platform/marvell-ccic/mcam-core.c | 12 
>  drivers/media/platform/soc_camera/soc_camera.c  | 10 ---
>  drivers/media/platform/via-camera.c |  4 +--
>  drivers/media/usb/em28xx/em28xx-video.c | 36 
>  13 files changed, 137 insertions(+), 144 deletions(-)
> 
> diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
> index 5e29064fae91..0e0bcc8b67ca 100644
> --- a/drivers/media/i2c/mt9v011.c
> +++ b/drivers/media/i2c/mt9v011.c
> @@ -364,33 +364,30 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
>   return 0;
>  }
>  
> -static int mt9v011_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm 
> *parms)
> +static int mt9v011_g_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
>  {
> - struct v4l2_captureparm *cp = >parm.capture;
> -
> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + if (ival->pad)
>   return -EINVAL;

The pad number checks are already present in v4l2-subdev.c. Do you think
we'll need them in drivers as well?

It's true that another driver could mis-use this interface. In that case
I'd introduce a wrapper to the op rather than introduce the check in every
driver.

>  
> - memset(cp, 0, sizeof(struct v4l2_captureparm));
> - cp->capability = V4L2_CAP_TIMEPERFRAME;
> + memset(ival->reserved, 0, sizeof(ival->reserved));
>   calc_fps(sd,
> -  >timeperframe.numerator,
> -  >timeperframe.denominator);
> +  >interval.numerator,
> +  >interval.denominator);
>  
>   return 0;
>  }
>  
> -static int mt9v011_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm 
> *parms)
> +static int mt9v011_s_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
>  {
> - struct v4l2_captureparm *cp = >parm.capture;
> - struct v4l2_fract *tpf = >timeperframe;
> + struct v4l2_fract *tpf = >interval;
>   u16 speed;
>  
> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return -EINVAL;
> - if (cp->extendedmode != 0)
> + if (ival->pad)
>   return -EINVAL;
>  
> + memset(ival->reserved, 0, sizeof(ival->reserved));
>   speed = calc_speed(sd, tpf->numerator, tpf->denominator);
>  
>   mt9v011_write(sd, R0A_MT9V011_CLK_SPEED, speed);
> @@ -469,8 +466,8 @@ static const struct v4l2_subdev_core_ops mt9v011_core_ops 
> = {
>  };
>  
>  static const struct v4l2_subdev_video_ops mt9v011_video_ops = {
> - .g_parm = mt9v011_g_parm,
> - .s_parm = mt9v011_s_parm,
> + .g_frame_interval = mt9v011_g_frame_interval,
> + .s_frame_interval = mt9v011_s_frame_interval,
>  };
>  
>  static const struct v4l2_subdev_pad_ops mt9v011_pad_ops = {
> diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
> index 8975d16b2b24..86fb75d3df5b 100644
> --- a/drivers/media/i2c/ov6650.c
> +++ b/drivers/media/i2c/ov6650.c
> @@ -201,7 +201,7 @@ struct ov6650 {
>   struct v4l2_rectrect;   /* sensor cropping window */
>   unsigned long   pclk_limit; /* from host */
>   unsigned long   pclk_max;   /* from resolution and format */
> - struct v4l2_fract   tpf;/* as requested with s_parm */
> + struct v4l2_fract   tpf;/* as requested with 
> s_frame_interval */
>   u32 code;
>   enum v4l2_colorspacecolorspace;
>  };
> @@ -723,42 +723,39 @@ static int ov6650_enum_mbus_code(struct v4l2_subdev *sd,
>   return 0;
>  }
>  
> -static int ov6650_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm 
> *parms)
> +static int ov6650_g_frame_interval(struct v4l2_subdev *sd,
> +struct v4l2_subdev_frame_interval *ival)
>  {
>   struct i2c_client *client = v4l2_get_subdevdata(sd);
>   struct ov6650 *priv = to_ov6650(client);
> - struct 

Re: [PATCH 9/9] vidioc-g-parm.rst: also allow _MPLANE buffer types

[Sakari Ailus]

Hi Hans,

On Mon, Jan 22, 2018 at 11:18:57AM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> The specification mentions that type can be V4L2_BUF_TYPE_VIDEO_CAPTURE,
> but the v4l2 core implementation also allows the _MPLANE variant.
> 
> Document this.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  Documentation/media/uapi/v4l/vidioc-g-parm.rst | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-g-parm.rst 
> b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
> index 616a5ea3f8fa..a941526cfeb4 100644
> --- a/Documentation/media/uapi/v4l/vidioc-g-parm.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
> @@ -66,7 +66,7 @@ union holding separate parameters for input and output 
> devices.
>-
>- The buffer (stream) type, same as struct
>   :c:type:`v4l2_format` ``type``, set by the
> - application. See :c:type:`v4l2_buf_type`
> + application. See :c:type:`v4l2_buf_type`.
>  * - union
>- ``parm``
>-
> @@ -75,12 +75,12 @@ union holding separate parameters for input and output 
> devices.
>- struct :c:type:`v4l2_captureparm`
>- ``capture``
>- Parameters for capture devices, used when ``type`` is
> - ``V4L2_BUF_TYPE_VIDEO_CAPTURE``.
``V4L2_BUF_TYPE_VIDEO_CAPTURE`` or 
``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE``.

Wrap this one? It's over 80...

>  * -
>- struct :c:type:`v4l2_outputparm`
>- ``output``
>- Parameters for output devices, used when ``type`` is
> - ``V4L2_BUF_TYPE_VIDEO_OUTPUT``.
> + ``V4L2_BUF_TYPE_VIDEO_OUTPUT`` or ``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE``.
>  * -
>- __u8
>- ``raw_data``\ [200]

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

[PATCH 5/9] staging: atomisp: i2c: Drop g_parm support in sensor drivers

[Hans Verkuil]

From: Sakari Ailus 

These drivers already support g_frame_interval. Therefore just dropping
g_parm is enough.

Signed-off-by: Sakari Ailus 
Signed-off-by: Hans Verkuil 
---
 drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 27 --
 drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 27 --
 drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 27 --
 drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 27 --
 .../media/atomisp/i2c/ov5693/atomisp-ov5693.c  | 27 --
 5 files changed, 135 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c 
b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index 572c9127c24d..93753cb96180 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -1204,32 +1204,6 @@ static int gc0310_s_config(struct v4l2_subdev *sd,
return ret;
 }
 
-static int gc0310_g_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   struct gc0310_device *dev = to_gc0310_sensor(sd);
-   struct i2c_client *client = v4l2_get_subdevdata(sd);
-
-   if (!param)
-   return -EINVAL;
-
-   if (param->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-   dev_err(>dev,  "unsupported buffer type.\n");
-   return -EINVAL;
-   }
-
-   memset(param, 0, sizeof(*param));
-   param->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-
-   if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
-   param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
-   param->parm.capture.timeperframe.numerator = 1;
-   param->parm.capture.timeperframe.denominator =
-   gc0310_res[dev->fmt_idx].fps;
-   }
-   return 0;
-}
-
 static int gc0310_g_frame_interval(struct v4l2_subdev *sd,
   struct v4l2_subdev_frame_interval *interval)
 {
@@ -1288,7 +1262,6 @@ static const struct v4l2_subdev_sensor_ops 
gc0310_sensor_ops = {
 
 static const struct v4l2_subdev_video_ops gc0310_video_ops = {
.s_stream = gc0310_s_stream,
-   .g_parm = gc0310_g_parm,
.g_frame_interval = gc0310_g_frame_interval,
 };
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c 
b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index 2bc179f3afe5..93f9c618f3d8 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -944,32 +944,6 @@ static int gc2235_s_config(struct v4l2_subdev *sd,
return ret;
 }
 
-static int gc2235_g_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   struct gc2235_device *dev = to_gc2235_sensor(sd);
-   struct i2c_client *client = v4l2_get_subdevdata(sd);
-
-   if (!param)
-   return -EINVAL;
-
-   if (param->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-   dev_err(>dev,  "unsupported buffer type.\n");
-   return -EINVAL;
-   }
-
-   memset(param, 0, sizeof(*param));
-   param->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-
-   if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
-   param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
-   param->parm.capture.timeperframe.numerator = 1;
-   param->parm.capture.timeperframe.denominator =
-   gc2235_res[dev->fmt_idx].fps;
-   }
-   return 0;
-}
-
 static int gc2235_g_frame_interval(struct v4l2_subdev *sd,
   struct v4l2_subdev_frame_interval *interval)
 {
@@ -1027,7 +1001,6 @@ static const struct v4l2_subdev_sensor_ops 
gc2235_sensor_ops = {
 
 static const struct v4l2_subdev_video_ops gc2235_video_ops = {
.s_stream = gc2235_s_stream,
-   .g_parm = gc2235_g_parm,
.g_frame_interval = gc2235_g_frame_interval,
 };
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c 
b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index e3e0fdd0c816..11412061c40e 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -1280,32 +1280,6 @@ static int ov2680_s_config(struct v4l2_subdev *sd,
return ret;
 }
 
-static int ov2680_g_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   struct ov2680_device *dev = to_ov2680_sensor(sd);
-   struct i2c_client *client = v4l2_get_subdevdata(sd);
-
-   if (!param)
-   return -EINVAL;
-
-   if (param->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-   dev_err(>dev,  "unsupported buffer type.\n");
-   return -EINVAL;
-   }
-
-   memset(param, 0, sizeof(*param));
-   param->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-
-   if (dev->fmt_idx >= 0 

[PATCH 9/9] vidioc-g-parm.rst: also allow _MPLANE buffer types

[Hans Verkuil]

From: Hans Verkuil 

The specification mentions that type can be V4L2_BUF_TYPE_VIDEO_CAPTURE,
but the v4l2 core implementation also allows the _MPLANE variant.

Document this.

Signed-off-by: Hans Verkuil 
---
 Documentation/media/uapi/v4l/vidioc-g-parm.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/media/uapi/v4l/vidioc-g-parm.rst 
b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
index 616a5ea3f8fa..a941526cfeb4 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-parm.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
@@ -66,7 +66,7 @@ union holding separate parameters for input and output 
devices.
   -
   - The buffer (stream) type, same as struct
:c:type:`v4l2_format` ``type``, set by the
-   application. See :c:type:`v4l2_buf_type`
+   application. See :c:type:`v4l2_buf_type`.
 * - union
   - ``parm``
   -
@@ -75,12 +75,12 @@ union holding separate parameters for input and output 
devices.
   - struct :c:type:`v4l2_captureparm`
   - ``capture``
   - Parameters for capture devices, used when ``type`` is
-   ``V4L2_BUF_TYPE_VIDEO_CAPTURE``.
+   ``V4L2_BUF_TYPE_VIDEO_CAPTURE`` or 
``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE``.
 * -
   - struct :c:type:`v4l2_outputparm`
   - ``output``
   - Parameters for output devices, used when ``type`` is
-   ``V4L2_BUF_TYPE_VIDEO_OUTPUT``.
+   ``V4L2_BUF_TYPE_VIDEO_OUTPUT`` or ``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE``.
 * -
   - __u8
   - ``raw_data``\ [200]
-- 
2.15.1

[PATCH 6/9] staging: atomisp: mt9m114: Drop empty s_parm callback

[Hans Verkuil]

From: Sakari Ailus 

The s_parm callback in mt9m114 driver did nothing, remove it.

Signed-off-by: Sakari Ailus 
Signed-off-by: Hans Verkuil 
---
 drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c 
b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
index df253a557c76..834fba8c4fa0 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
@@ -1684,11 +1684,6 @@ static int mt9m114_t_vflip(struct v4l2_subdev *sd, int 
value)
 
return !!err;
 }
-static int mt9m114_s_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   return 0;
-}
 
 static int mt9m114_g_frame_interval(struct v4l2_subdev *sd,
   struct v4l2_subdev_frame_interval *interval)
@@ -1781,7 +1776,6 @@ static int mt9m114_g_skip_frames(struct v4l2_subdev *sd, 
u32 *frames)
 }
 
 static const struct v4l2_subdev_video_ops mt9m114_video_ops = {
-   .s_parm = mt9m114_s_parm,
.s_stream = mt9m114_s_stream,
.g_frame_interval = mt9m114_g_frame_interval,
 };
-- 
2.15.1

[PATCH 8/9] v4l2-subdev.h: remove obsolete g/s_parm

[Hans Verkuil]

From: Hans Verkuil 

Signed-off-by: Hans Verkuil 
---
 include/media/v4l2-subdev.h | 6 --
 1 file changed, 6 deletions(-)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 980a86c08fce..457917e9237f 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -393,10 +393,6 @@ struct v4l2_mbus_frame_desc {
  *
  * @g_pixelaspect: callback to return the pixelaspect ratio.
  *
- * @g_parm: callback for VIDIOC_G_PARM() ioctl handler code.
- *
- * @s_parm: callback for VIDIOC_S_PARM() ioctl handler code.
- *
  * @g_frame_interval: callback for VIDIOC_SUBDEV_G_FRAME_INTERVAL()
  *   ioctl handler code.
  *
@@ -434,8 +430,6 @@ struct v4l2_subdev_video_ops {
int (*g_input_status)(struct v4l2_subdev *sd, u32 *status);
int (*s_stream)(struct v4l2_subdev *sd, int enable);
int (*g_pixelaspect)(struct v4l2_subdev *sd, struct v4l2_fract *aspect);
-   int (*g_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
-   int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
int (*g_frame_interval)(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval);
int (*s_frame_interval)(struct v4l2_subdev *sd,
-- 
2.15.1

[PATCH 7/9] staging: atomisp: Drop g_parm and s_parm subdev ops use

[Hans Verkuil]

From: Sakari Ailus 

The s_parm and g_parm did nothing. Remove them.

Signed-off-by: Sakari Ailus 
Signed-off-by: Hans Verkuil 
---
 .../staging/media/atomisp/pci/atomisp2/atomisp_file.c| 16 
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c | 14 --
 2 files changed, 30 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_file.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_file.c
index 377ec2a9fa6d..c6d96987561d 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_file.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_file.c
@@ -77,20 +77,6 @@ static int file_input_s_stream(struct v4l2_subdev *sd, int 
enable)
return 0;
 }
 
-static int file_input_g_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   /*to fake*/
-   return 0;
-}
-
-static int file_input_s_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   /*to fake*/
-   return 0;
-}
-
 static int file_input_get_fmt(struct v4l2_subdev *sd,
  struct v4l2_subdev_pad_config *cfg,
  struct v4l2_subdev_format *format)
@@ -166,8 +152,6 @@ static int file_input_enum_frame_ival(struct v4l2_subdev 
*sd,
 
 static const struct v4l2_subdev_video_ops file_input_video_ops = {
.s_stream = file_input_s_stream,
-   .g_parm = file_input_g_parm,
-   .s_parm = file_input_s_parm,
 };
 
 static const struct v4l2_subdev_core_ops file_input_core_ops = {
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
index b71cc7bcdbab..adc900272f6f 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
@@ -27,18 +27,6 @@ static int tpg_s_stream(struct v4l2_subdev *sd, int enable)
return 0;
 }
 
-static int tpg_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *param)
-{
-   /*to fake*/
-   return 0;
-}
-
-static int tpg_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *param)
-{
-   /*to fake*/
-   return 0;
-}
-
 static int tpg_get_fmt(struct v4l2_subdev *sd,
   struct v4l2_subdev_pad_config *cfg,
   struct v4l2_subdev_format *format)
@@ -101,8 +89,6 @@ static int tpg_enum_frame_ival(struct v4l2_subdev *sd,
 
 static const struct v4l2_subdev_video_ops tpg_video_ops = {
.s_stream = tpg_s_stream,
-   .g_parm = tpg_g_parm,
-   .s_parm = tpg_s_parm,
 };
 
 static const struct v4l2_subdev_core_ops tpg_core_ops = {
-- 
2.15.1

[PATCH 3/9] staging: atomisp: Kill subdev s_parm abuse

[Hans Verkuil]

From: Sakari Ailus 

Remove sensor driver's interface that made use of use case specific
knowledge of platform capabilities.

Signed-off-by: Sakari Ailus 
Signed-off-by: Hans Verkuil 
---
 drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 26 -
 drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 26 -
 drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 29 -
 drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 26 -
 drivers/staging/media/atomisp/i2c/gc0310.h | 43 --
 drivers/staging/media/atomisp/i2c/gc2235.h |  1 -
 drivers/staging/media/atomisp/i2c/ov2680.h | 68 --
 .../media/atomisp/i2c/ov5693/atomisp-ov5693.c  | 27 -
 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   |  9 +--
 .../media/atomisp/pci/atomisp2/atomisp_subdev.c| 12 +---
 10 files changed, 3 insertions(+), 264 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c 
b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index 61b7598469eb..572c9127c24d 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -1224,37 +1224,12 @@ static int gc0310_g_parm(struct v4l2_subdev *sd,
if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
param->parm.capture.timeperframe.numerator = 1;
-   param->parm.capture.capturemode = dev->run_mode;
param->parm.capture.timeperframe.denominator =
gc0310_res[dev->fmt_idx].fps;
}
return 0;
 }
 
-static int gc0310_s_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   struct gc0310_device *dev = to_gc0310_sensor(sd);
-   dev->run_mode = param->parm.capture.capturemode;
-
-   mutex_lock(>input_lock);
-   switch (dev->run_mode) {
-   case CI_MODE_VIDEO:
-   gc0310_res = gc0310_res_video;
-   N_RES = N_RES_VIDEO;
-   break;
-   case CI_MODE_STILL_CAPTURE:
-   gc0310_res = gc0310_res_still;
-   N_RES = N_RES_STILL;
-   break;
-   default:
-   gc0310_res = gc0310_res_preview;
-   N_RES = N_RES_PREVIEW;
-   }
-   mutex_unlock(>input_lock);
-   return 0;
-}
-
 static int gc0310_g_frame_interval(struct v4l2_subdev *sd,
   struct v4l2_subdev_frame_interval *interval)
 {
@@ -1314,7 +1289,6 @@ static const struct v4l2_subdev_sensor_ops 
gc0310_sensor_ops = {
 static const struct v4l2_subdev_video_ops gc0310_video_ops = {
.s_stream = gc0310_s_stream,
.g_parm = gc0310_g_parm,
-   .s_parm = gc0310_s_parm,
.g_frame_interval = gc0310_g_frame_interval,
 };
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c 
b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index d8de46da64ae..2bc179f3afe5 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -964,37 +964,12 @@ static int gc2235_g_parm(struct v4l2_subdev *sd,
if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
param->parm.capture.timeperframe.numerator = 1;
-   param->parm.capture.capturemode = dev->run_mode;
param->parm.capture.timeperframe.denominator =
gc2235_res[dev->fmt_idx].fps;
}
return 0;
 }
 
-static int gc2235_s_parm(struct v4l2_subdev *sd,
-   struct v4l2_streamparm *param)
-{
-   struct gc2235_device *dev = to_gc2235_sensor(sd);
-   dev->run_mode = param->parm.capture.capturemode;
-
-   mutex_lock(>input_lock);
-   switch (dev->run_mode) {
-   case CI_MODE_VIDEO:
-   gc2235_res = gc2235_res_video;
-   N_RES = N_RES_VIDEO;
-   break;
-   case CI_MODE_STILL_CAPTURE:
-   gc2235_res = gc2235_res_still;
-   N_RES = N_RES_STILL;
-   break;
-   default:
-   gc2235_res = gc2235_res_preview;
-   N_RES = N_RES_PREVIEW;
-   }
-   mutex_unlock(>input_lock);
-   return 0;
-}
-
 static int gc2235_g_frame_interval(struct v4l2_subdev *sd,
   struct v4l2_subdev_frame_interval *interval)
 {
@@ -1053,7 +1028,6 @@ static const struct v4l2_subdev_sensor_ops 
gc2235_sensor_ops = {
 static const struct v4l2_subdev_video_ops gc2235_video_ops = {
.s_stream = gc2235_s_stream,
.g_parm = gc2235_g_parm,
-   .s_parm = gc2235_s_parm,
.g_frame_interval = gc2235_g_frame_interval,
 };
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c 

[PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs

[Hans Verkuil]

From: Hans Verkuil 

Convert all g/s_parm calls to g/s_frame_interval. This allows us
to remove the g/s_parm ops since those are a duplicate of
g/s_frame_interval.

Signed-off-by: Hans Verkuil 
---
 drivers/media/i2c/mt9v011.c | 29 +--
 drivers/media/i2c/ov6650.c  | 33 ++
 drivers/media/i2c/ov7670.c  | 26 +
 drivers/media/i2c/ov7740.c  | 29 ---
 drivers/media/i2c/tvp514x.c | 37 +++--
 drivers/media/i2c/vs6624.c  | 29 ++-
 drivers/media/platform/atmel/atmel-isc.c| 10 ++-
 drivers/media/platform/atmel/atmel-isi.c| 12 ++--
 drivers/media/platform/blackfin/bfin_capture.c  | 14 +++---
 drivers/media/platform/marvell-ccic/mcam-core.c | 12 
 drivers/media/platform/soc_camera/soc_camera.c  | 10 ---
 drivers/media/platform/via-camera.c |  4 +--
 drivers/media/usb/em28xx/em28xx-video.c | 36 
 13 files changed, 137 insertions(+), 144 deletions(-)

diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
index 5e29064fae91..0e0bcc8b67ca 100644
--- a/drivers/media/i2c/mt9v011.c
+++ b/drivers/media/i2c/mt9v011.c
@@ -364,33 +364,30 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
return 0;
 }
 
-static int mt9v011_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm 
*parms)
+static int mt9v011_g_frame_interval(struct v4l2_subdev *sd,
+   struct v4l2_subdev_frame_interval *ival)
 {
-   struct v4l2_captureparm *cp = >parm.capture;
-
-   if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+   if (ival->pad)
return -EINVAL;
 
-   memset(cp, 0, sizeof(struct v4l2_captureparm));
-   cp->capability = V4L2_CAP_TIMEPERFRAME;
+   memset(ival->reserved, 0, sizeof(ival->reserved));
calc_fps(sd,
->timeperframe.numerator,
->timeperframe.denominator);
+>interval.numerator,
+>interval.denominator);
 
return 0;
 }
 
-static int mt9v011_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm 
*parms)
+static int mt9v011_s_frame_interval(struct v4l2_subdev *sd,
+   struct v4l2_subdev_frame_interval *ival)
 {
-   struct v4l2_captureparm *cp = >parm.capture;
-   struct v4l2_fract *tpf = >timeperframe;
+   struct v4l2_fract *tpf = >interval;
u16 speed;
 
-   if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
-   return -EINVAL;
-   if (cp->extendedmode != 0)
+   if (ival->pad)
return -EINVAL;
 
+   memset(ival->reserved, 0, sizeof(ival->reserved));
speed = calc_speed(sd, tpf->numerator, tpf->denominator);
 
mt9v011_write(sd, R0A_MT9V011_CLK_SPEED, speed);
@@ -469,8 +466,8 @@ static const struct v4l2_subdev_core_ops mt9v011_core_ops = 
{
 };
 
 static const struct v4l2_subdev_video_ops mt9v011_video_ops = {
-   .g_parm = mt9v011_g_parm,
-   .s_parm = mt9v011_s_parm,
+   .g_frame_interval = mt9v011_g_frame_interval,
+   .s_frame_interval = mt9v011_s_frame_interval,
 };
 
 static const struct v4l2_subdev_pad_ops mt9v011_pad_ops = {
diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
index 8975d16b2b24..86fb75d3df5b 100644
--- a/drivers/media/i2c/ov6650.c
+++ b/drivers/media/i2c/ov6650.c
@@ -201,7 +201,7 @@ struct ov6650 {
struct v4l2_rectrect;   /* sensor cropping window */
unsigned long   pclk_limit; /* from host */
unsigned long   pclk_max;   /* from resolution and format */
-   struct v4l2_fract   tpf;/* as requested with s_parm */
+   struct v4l2_fract   tpf;/* as requested with 
s_frame_interval */
u32 code;
enum v4l2_colorspacecolorspace;
 };
@@ -723,42 +723,39 @@ static int ov6650_enum_mbus_code(struct v4l2_subdev *sd,
return 0;
 }
 
-static int ov6650_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
+static int ov6650_g_frame_interval(struct v4l2_subdev *sd,
+  struct v4l2_subdev_frame_interval *ival)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov6650 *priv = to_ov6650(client);
-   struct v4l2_captureparm *cp = >parm.capture;
 
-   if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+   if (ival->pad)
return -EINVAL;
 
-   memset(cp, 0, sizeof(*cp));
-   cp->capability = V4L2_CAP_TIMEPERFRAME;
-   cp->timeperframe.numerator = GET_CLKRC_DIV(to_clkrc(>tpf,
+   memset(ival->reserved, 0, sizeof(ival->reserved));
+   ival->interval.numerator = GET_CLKRC_DIV(to_clkrc(>tpf,
priv->pclk_limit, priv->pclk_max));

[PATCH 1/9] v4l2-common: create v4l2_g/s_parm_cap helpers

[Hans Verkuil]

From: Hans Verkuil 

Create helpers to handle VIDIOC_G/S_PARM by querying the
g/s_frame_interval v4l2_subdev ops.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-common.c | 49 +++
 include/media/v4l2-common.h   | 26 +++
 2 files changed, 75 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c 
b/drivers/media/v4l2-core/v4l2-common.c
index 8650ad92b64d..4e371ae4aed7 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -392,3 +392,52 @@ void v4l2_get_timestamp(struct timeval *tv)
tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
 }
 EXPORT_SYMBOL_GPL(v4l2_get_timestamp);
+
+int v4l2_g_parm_cap(struct video_device *vdev,
+   struct v4l2_subdev *sd, struct v4l2_streamparm *a)
+{
+   struct v4l2_subdev_frame_interval ival = { 0 };
+   int ret;
+
+   if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+   a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+   return -EINVAL;
+
+   if (vdev->device_caps & V4L2_CAP_READWRITE)
+   a->parm.capture.readbuffers = 2;
+   if (v4l2_subdev_has_op(sd, video, g_frame_interval))
+   a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
+   ret = v4l2_subdev_call(sd, video, g_frame_interval, );
+   if (!ret)
+   a->parm.capture.timeperframe = ival.interval;
+   return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_g_parm_cap);
+
+int v4l2_s_parm_cap(struct video_device *vdev,
+   struct v4l2_subdev *sd, struct v4l2_streamparm *a)
+{
+   struct v4l2_subdev_frame_interval ival = {
+   0,
+   a->parm.capture.timeperframe
+   };
+   int ret;
+
+   if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+   a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+   return -EINVAL;
+
+   memset(>parm, 0, sizeof(a->parm));
+   if (vdev->device_caps & V4L2_CAP_READWRITE)
+   a->parm.capture.readbuffers = 2;
+   else
+   a->parm.capture.readbuffers = 0;
+
+   if (v4l2_subdev_has_op(sd, video, g_frame_interval))
+   a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
+   ret = v4l2_subdev_call(sd, video, s_frame_interval, );
+   if (!ret)
+   a->parm.capture.timeperframe = ival.interval;
+   return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_s_parm_cap);
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index e0d95a7c5d48..f3aa1d728c0b 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -341,4 +341,30 @@ v4l2_find_nearest_format(const struct 
v4l2_frmsize_discrete *sizes,
  */
 void v4l2_get_timestamp(struct timeval *tv);
 
+/**
+ * v4l2_g_parm_cap - helper routine for vidioc_g_parm to fill this in by
+ *  calling the g_frame_interval op of the given subdev. It only works
+ *  for V4L2_BUF_TYPE_VIDEO_CAPTURE(_MPLANE), hence the _cap in the
+ *  function name.
+ *
+ * @vdev: the struct video_device pointer. Used to determine the device caps.
+ * @sd: the sub-device pointer.
+ * @a: the VIDIOC_G_PARM argument.
+ */
+int v4l2_g_parm_cap(struct video_device *vdev,
+   struct v4l2_subdev *sd, struct v4l2_streamparm *a);
+
+/**
+ * v4l2_s_parm_cap - helper routine for vidioc_s_parm to fill this in by
+ *  calling the s_frame_interval op of the given subdev. It only works
+ *  for V4L2_BUF_TYPE_VIDEO_CAPTURE(_MPLANE), hence the _cap in the
+ *  function name.
+ *
+ * @vdev: the struct video_device pointer. Used to determine the device caps.
+ * @sd: the sub-device pointer.
+ * @a: the VIDIOC_S_PARM argument.
+ */
+int v4l2_s_parm_cap(struct video_device *vdev,
+   struct v4l2_subdev *sd, struct v4l2_streamparm *a);
+
 #endif /* V4L2_COMMON_H_ */
-- 
2.15.1

[PATCH 4/9] staging: atomisp: i2c: Disable non-preview configurations

[Hans Verkuil]

From: Sakari Ailus 

Disable configurations for non-preview modes until configuration selection
is improved.

Signed-off-by: Sakari Ailus 
Signed-off-by: Hans Verkuil 
---
 drivers/staging/media/atomisp/i2c/gc2235.h| 2 ++
 drivers/staging/media/atomisp/i2c/ov2722.h| 2 ++
 drivers/staging/media/atomisp/i2c/ov5693/ov5693.h | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/drivers/staging/media/atomisp/i2c/gc2235.h 
b/drivers/staging/media/atomisp/i2c/gc2235.h
index 45a54fea5466..817c0068c1d3 100644
--- a/drivers/staging/media/atomisp/i2c/gc2235.h
+++ b/drivers/staging/media/atomisp/i2c/gc2235.h
@@ -574,6 +574,7 @@ static struct gc2235_resolution gc2235_res_preview[] = {
 };
 #define N_RES_PREVIEW (ARRAY_SIZE(gc2235_res_preview))
 
+#if 0 /* Disable non-previes configurations for now */
 static struct gc2235_resolution gc2235_res_still[] = {
{
.desc = "gc2235_1600_900_30fps",
@@ -658,6 +659,7 @@ static struct gc2235_resolution gc2235_res_video[] = {
 
 };
 #define N_RES_VIDEO (ARRAY_SIZE(gc2235_res_video))
+#endif
 
 static struct gc2235_resolution *gc2235_res = gc2235_res_preview;
 static unsigned long N_RES = N_RES_PREVIEW;
diff --git a/drivers/staging/media/atomisp/i2c/ov2722.h 
b/drivers/staging/media/atomisp/i2c/ov2722.h
index d8a973d71699..f133439adfd5 100644
--- a/drivers/staging/media/atomisp/i2c/ov2722.h
+++ b/drivers/staging/media/atomisp/i2c/ov2722.h
@@ -1148,6 +1148,7 @@ struct ov2722_resolution ov2722_res_preview[] = {
 };
 #define N_RES_PREVIEW (ARRAY_SIZE(ov2722_res_preview))
 
+#if 0 /* Disable non-previes configurations for now */
 struct ov2722_resolution ov2722_res_still[] = {
{
.desc = "ov2722_480P_30fps",
@@ -1250,6 +1251,7 @@ struct ov2722_resolution ov2722_res_video[] = {
},
 };
 #define N_RES_VIDEO (ARRAY_SIZE(ov2722_res_video))
+#endif
 
 static struct ov2722_resolution *ov2722_res = ov2722_res_preview;
 static unsigned long N_RES = N_RES_PREVIEW;
diff --git a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h 
b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h
index 68cfcb4a6c3c..15a33dcd2d59 100644
--- a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h
+++ b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h
@@ -1147,6 +1147,7 @@ struct ov5693_resolution ov5693_res_preview[] = {
 };
 #define N_RES_PREVIEW (ARRAY_SIZE(ov5693_res_preview))
 
+#if 0 /* Disable non-previes configurations for now */
 struct ov5693_resolution ov5693_res_still[] = {
{
.desc = "ov5693_736x496_30fps",
@@ -1364,6 +1365,7 @@ struct ov5693_resolution ov5693_res_video[] = {
},
 };
 #define N_RES_VIDEO (ARRAY_SIZE(ov5693_res_video))
+#endif
 
 static struct ov5693_resolution *ov5693_res = ov5693_res_preview;
 static unsigned long N_RES = N_RES_PREVIEW;
-- 
2.15.1

[PATCH 0/9] media: replace g/s_parm by g/s_frame_interval

[Hans Verkuil]

From: Hans Verkuil 

There are currently two subdev ops variants to get/set the frame interval:
g/s_parm and g/s_frame_interval.

This patch series replaces all g/s_parm calls by g/s_frame_interval.

The first patch adds helper functions that can be used by bridge drivers.
Only em28xx can't use it and it needs custom code (it uses v4l2_device_call()
to try all subdevs instead of calling a specific subdev).

The next patch converts all non-staging drivers, then come Sakari's
atomisp staging fixes.

The v4l2-subdev.h patch removes the now obsolete g/s_parm ops and the
final patch clarifies the documentation a bit (the core allows for
_MPLANE to be used as well).

I would really like to take the next step and introduce two new ioctls
VIDIOC_G/S_FRAME_INTERVAL (just like the SUBDEV variants that already
exist) and convert all bridge drivers to use that and just have helper
functions in the core for VIDIOC_G/S_PARM.

I hate that ioctl and it always confuses driver developers. It would
also prevent the type of abuse that was present in the atomisp driver.

But that's for later, let's simplify the subdev drivers first.

Regards,

Hans

Hans Verkuil (4):
  v4l2-common: create v4l2_g/s_parm_cap helpers
  media: convert g/s_parm to g/s_frame_interval in subdevs
  v4l2-subdev.h: remove obsolete g/s_parm
  vidioc-g-parm.rst: also allow _MPLANE buffer types

Sakari Ailus (5):
  staging: atomisp: Kill subdev s_parm abuse
  staging: atomisp: i2c: Disable non-preview configurations
  staging: atomisp: i2c: Drop g_parm support in sensor drivers
  staging: atomisp: mt9m114: Drop empty s_parm callback
  staging: atomisp: Drop g_parm and s_parm subdev ops use

 Documentation/media/uapi/v4l/vidioc-g-parm.rst |  6 +-
 drivers/media/i2c/mt9v011.c| 29 +
 drivers/media/i2c/ov6650.c | 33 +--
 drivers/media/i2c/ov7670.c | 26 +
 drivers/media/i2c/ov7740.c | 29 -
 drivers/media/i2c/tvp514x.c| 37 +---
 drivers/media/i2c/vs6624.c | 29 +
 drivers/media/platform/atmel/atmel-isc.c   | 10 +---
 drivers/media/platform/atmel/atmel-isi.c   | 12 +---
 drivers/media/platform/blackfin/bfin_capture.c | 14 ++---
 drivers/media/platform/marvell-ccic/mcam-core.c| 12 ++--
 drivers/media/platform/soc_camera/soc_camera.c | 10 ++--
 drivers/media/platform/via-camera.c|  4 +-
 drivers/media/usb/em28xx/em28xx-video.c| 36 ++--
 drivers/media/v4l2-core/v4l2-common.c  | 49 
 drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 53 -
 drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 53 -
 .../staging/media/atomisp/i2c/atomisp-mt9m114.c|  6 --
 drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 56 --
 drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 53 -
 drivers/staging/media/atomisp/i2c/gc0310.h | 43 --
 drivers/staging/media/atomisp/i2c/gc2235.h |  3 +-
 drivers/staging/media/atomisp/i2c/ov2680.h | 68 --
 drivers/staging/media/atomisp/i2c/ov2722.h |  2 +
 .../media/atomisp/i2c/ov5693/atomisp-ov5693.c  | 54 -
 drivers/staging/media/atomisp/i2c/ov5693/ov5693.h  |  2 +
 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   |  9 +--
 .../media/atomisp/pci/atomisp2/atomisp_file.c  | 16 -
 .../media/atomisp/pci/atomisp2/atomisp_subdev.c| 12 +---
 .../media/atomisp/pci/atomisp2/atomisp_tpg.c   | 14 -
 include/media/v4l2-common.h| 26 +
 include/media/v4l2-subdev.h|  6 --
 32 files changed, 224 insertions(+), 588 deletions(-)

-- 
2.15.1

[PATCH] media: ov5640: fix spurious streamon failures

[Hugues Fruchet]

Time to time, stream is failing with a strange positive error.
Error code is returned erroneously by ov5640_set_ctrl_exposure()
due to ov5640_get_vts() return value wrongly treated as error.
Fix this by forcing ret to 0 after ov5640_get_vts() success call,
in order that ret is set to success for rest of code sequence.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/i2c/ov5640.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index f017742..e2dd352 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2057,6 +2057,7 @@ static int ov5640_set_ctrl_exposure(struct ov5640_dev 
*sensor, int exp)
if (ret < 0)
return ret;
max_exp += ret;
+   ret = 0;
 
if (ctrls->exposure->val < max_exp)
ret = ov5640_set_exposure(sensor, ctrls->exposure->val);
-- 
1.9.1

[PATCH] media: ov5640: fix spurious streamon failures

[Hugues Fruchet]

Time to time, stream on is failing with a strange positive error.
Error code is returned erroneously by ov5640_set_ctrl_exposure()
due to ov5640_get_vts() return value wrongly treated as error.
Fix this by forcing ret to 0 after ov5640_get_vts() success call,
in order that ret is set to success for rest of code sequence.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/i2c/ov5640.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index f017742..e2dd352 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2057,6 +2057,7 @@ static int ov5640_set_ctrl_exposure(struct ov5640_dev 
*sensor, int exp)
if (ret < 0)
return ret;
max_exp += ret;
+   ret = 0;
 
if (ctrls->exposure->val < max_exp)
ret = ov5640_set_exposure(sensor, ctrls->exposure->val);
-- 
1.9.1

Re: [PATCH v6 0/9] Renesas Capture Engine Unit (CEU) V4L2 driver

[Hans Verkuil]

On 21/01/18 18:49, jacopo mondi wrote:
> Hi Hans,
> 
> On Fri, Jan 19, 2018 at 12:26:09PM +0100, Hans Verkuil wrote:
>> Hi Jacopo,
>>
>> On 01/16/18 22:44, Jacopo Mondi wrote:
>>> Hello,
>>>new version of CEU after Hans' review.
>>>
>>> Added his Acked-by to most patches and closed review comments.
>>> Running v4l2-compliance, I noticed a new failure introduced by the way I now
>>> calculate the plane sizes in set/try_fmt.
>>>
>>> This is the function used to update per-plane bytesperline and sizeimage:
>>>
>>> static void ceu_update_plane_sizes(struct v4l2_plane_pix_format *plane,
>>>unsigned int bpl, unsigned int szimage)
>>> {
>>> if (plane->bytesperline < bpl)
>>> plane->bytesperline = bpl;
>>> if (plane->sizeimage < szimage)
>>> plane->sizeimage = szimage;
>>> }
>>>
>>> I'm seeing a failure as v4l2-compliance requires buffers with both 
>>> bytesperline
>>> and sizeimage set to MAX_INT . Hans, is this expected from v4l2-compliance?
>>> How should I handle this, if that has to be handled by the single drivers?
>>
>> I commented on this in my review of patch 3/9.
> 
> Fixed thank you.
> 
>>
>>>
>>> Apart from that, here it is the output of v4l2-compliance, with the last 
>>> tests
>>> failing due to the above stated reason, and two errors in try/set format 
>>> due to
>>> the fact the driver is not setting ycbcr encoding after it receives an 
>>> invalid
>>
>> Which driver? The CEU driver or the sensor driver? I don't actually see where
>> it fails.
>>
> 
> Here it is:
> 
> fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff
> fail: v4l2-test-formats.cpp(451): 
> testColorspace(pix_mp.pixelformat, pix_mp.colorspace, pix_mp.ycbcr_enc, 
> pix_mp.quantization)
> fail: v4l2-test-formats.cpp(736): Video Capture Multiplanar 
> is valid, but TRY_FMT failed to return a format
> test VIDIOC_TRY_FMT: FAIL
> fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff
> fail: v4l2-test-formats.cpp(451): 
> testColorspace(pix_mp.pixelformat, pix_mp.colorspace, pix_mp.ycbcr_enc, 
> pix_mp.quantization)
> fail: v4l2-test-formats.cpp(996): Video Capture Multiplanar 
> is valid, but no S_FMT was implemented
> test VIDIOC_S_FMT: FAIL

Sorry, I was perhaps confusing. I meant that I couldn't see where in the code
ycbcr_enc was not overwritten by 0 (which should have happened). You will need
to debug a bit, I think. It could be a bug in the ceu driver or the sensor 
driver.

> 
> 
>>> format. I would set those, but I'm not sure what it the correct value and 
>>> not
>>> all mainline drivers do that.
>>
>> In any case, the default for ycbcr_enc, xfer_func and quantization is 0.
>>
> 
> Thanks again. I do expect to be the sensor driver to set ycbcr_enc and
> quantization, but from a very trivial grep on media/i2c/ I see only a
> few drivers taking care of them (adv7511 and adv7842). What about the
> others? I assume v4l2-compliance would not fail on them as it does on
> ov7670, but I don't see where ycbr_enc (and others) are managed.

In most cases these values are initialized to 0 (either a memset or by
initializing the struct), which is typically all you need for sensor
drivers. HDMI drivers are more complicated which is why you see explicit
handling of these fields only there.

> 
> Overall, with this addressed, the other issue I mentioned on patch
> [3/9] on readbuffers clarified and frameinterval handled for ov722x, I
> hope we're done with this series. Thanks again your continued effort
> in reviews and guidance.

My pleasure!

Regards,

Hans

Re: [PATCH v6 3/9] v4l: platform: Add Renesas CEU driver

[Hans Verkuil]

On 21/01/18 18:29, jacopo mondi wrote:
> Hi Hans,
> 
> On Sun, Jan 21, 2018 at 11:23:12AM +0100, Hans Verkuil wrote:
>> On 21/01/18 11:21, Hans Verkuil wrote:
>>> On 21/01/18 10:53, jacopo mondi wrote:
 Hi Hans,

 On Fri, Jan 19, 2018 at 12:20:19PM +0100, Hans Verkuil wrote:
> static int ov7670_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm 
> *parms)
> {
> struct v4l2_captureparm *cp = >parm.capture;
> struct ov7670_info *info = to_state(sd);
>
> if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> return -EINVAL;
>
> And parms->type is CAPTURE_MPLANE. Just drop this test from the ov7670 
> driver
> in the g/s_parm functions. It shouldn't test for that since a subdev 
> driver
> knows nothing about buffer types.
>

 I will drop that test in an additional patch part of next iteration of 
 this series,
>>>
>>> Replace g/s_parm by g/s_frame_interval. Consider g/s_parm for subdev 
>>> drivers as
>>> deprecated (I'm working on a patch series to replace all g/s_parm uses by
>>> g/s_frame_interval).
>>
>> Take a look here:
>>
>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=parm
>>
>> You probably want to use the patch 'v4l2-common: add g/s_parm helper 
>> functions'
>> for the new ceu driver in your patch series. Feel free to add it.
> 
> Thanks, I have now re-based my series on top of your 'parm' branch,
> and now I have silenced those errors on bad frame interval.
> 
> CEU g/s_parm now look like this:
> 
> static int ceu_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> {
>   struct ceu_device *ceudev = video_drvdata(file);
>   int ret;
> 
>   ret = v4l2_g_parm(V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> ceudev->sd->v4l2_sd, a);
>   if (ret)
>   return ret;
> 
>   a->parm.capture.readbuffers = 0;
> 
>   return 0;
> }
> 
> Very similar to what you've done on other platform drivers in this
> commit:
> https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=parm=a58956ef45cebaa5ce43a5f740fe04517b24a853
> 
> I have a question though (please bear with me a little more :)
> I had to manually set a->parm.capture.readbuffers to 0 to silence the 
> following
> error in v4l2_compliance (which I have now updated to the most recent
> remote HEAD):
> 
>  fail: v4l2-test-formats.cpp(1114): cap->readbuffers
> test VIDIOC_G/S_PARM: FAIL
> 
>   fail_on_test(cap->readbuffers > VIDEO_MAX_FRAME);
>   if (!(node->g_caps() & V4L2_CAP_READWRITE))
>   fail_on_test(cap->readbuffers);
>   else if (node->g_caps() & V4L2_CAP_STREAMING)
>   fail_on_test(!cap->readbuffers);
> 
> CEU does not support CAP_READWRITE, as it seems atmel-isc/isi do not, so
> v4l2-compliance wants to have readbuffers set to 0. I wonder why in
> the previously mentioned commit you didn't have to set readbuffers
> explicitly to 0 for atmel-isc/isi as I had to for CEU. Will v4l2-compliance
> fail if run on atmel-isc/isi with your commit, or am I missing something?

I've reworked the g/s_parm helper functions so they will now check for
the READWRITE capability and set readbuffers accordingly. I'll post a new
version later today.

Thanks for testing this, I missed that corner case.

Regards,

Hans

Re: atomisp and g/s_parm

[Sakari Ailus]

Hi Hans,

On Mon, Jan 22, 2018 at 10:19:13AM +0100, Hans Verkuil wrote:
> On 21/01/18 23:48, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Sun, Jan 21, 2018 at 11:46:46AM +0100, Hans Verkuil wrote:
> >> Hi Sakari,
> >>
> >> I looked a bit closer at how atomisp uses g/s_parm. They abuse the 
> >> capturemode field
> >> to select video/preview/still modes on the sensor, which actually changes 
> >> the list
> >> of supported resolutions.
> >>
> >> The following files use this:
> >>
> >> i2c/atomisp-gc0310.c
> >> i2c/atomisp-gc2235.c
> >> i2c/atomisp-ov2680.c
> >> i2c/atomisp-ov2722.c
> >> i2c/ov5693/atomisp-ov5693.c
> >> pci/atomisp2/atomisp_file.c
> >> pci/atomisp2/atomisp_tpg.c
> >>
> >> The last two have a dummy g/s_parm implementation, so are easy to fix.
> >> The gc0310 and 0v2680 have identical resolution lists for all three modes, 
> >> so
> >> the capturemode can just be ignored and these two drivers can be 
> >> simplified.
> >>
> >> Looking at the higher level code it turns out that this atomisp driver 
> >> appears
> >> to be in the middle of a conversion from using s_parm to a 
> >> V4L2_CID_RUN_MODE
> >> control. If the control is present, then that will be used to set the mode,
> >> otherwise it falls back to s_parm.
> >>
> >> So the best solution would be if Intel can convert the remaining drivers 
> >> from
> >> using s_parm to the new control and then drop all code that uses s_parm to 
> >> do
> >> this, so g/s_parm is then only used to get/set the framerate.
> >>
> >> Is this something you or a colleague can take on?
> > 
> > I've stabbed the atomisp sensor drivers enough to remove the s_parm and
> > g_parm usage there. This effectively removes the s_parm abuse, as there was
> > nothing else it was being used for.
> > 
> > The patches are here; there are no changes to your patches in the branch
> > you pointed me to:
> > 
> > 
> > 
> > I've split dropping support for certain modes in the drivers into separate
> > patch; it's easy to bring them back by just reverting the patch ("staging:
> > atomisp: i2c: Disable non-preview configurations") or removing the ifdefs.
> > I don't object merging this with the previous patch either.
> > 
> > What comes to the run mode control --- this logic should have always
> > resided in user space; that control (and s_parm hack) is basically getting
> > around lack of support for MC / V4L2 sub-device interface in the driver. So
> > that control isn't the right solution either going forward.
> > 
> > Cc Andy and Alan.
> > 
> 
> Looks good. Just one note: in atomisp_ioctl.c the atomisp_g_parm function 
> still
> abuses this API (setting capturemode) but more importantly, it never calls
> g_frame_interval. The atomisp_s_parm function *does* call s_frame_interval.
> 
> So this is inconsistent. However, this was always there, so it's not something
> that was introduced by these changes.

There could be some value in bringing the sensor drivers as such out of
staging, too; so implementing g_frame_interval is a good thing.

If you're fine with the additional patches, feel free to send to the list.

-- 
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com

Re: atomisp and g/s_parm

[Hans Verkuil]

On 21/01/18 23:48, Sakari Ailus wrote:
> Hi Hans,
> 
> On Sun, Jan 21, 2018 at 11:46:46AM +0100, Hans Verkuil wrote:
>> Hi Sakari,
>>
>> I looked a bit closer at how atomisp uses g/s_parm. They abuse the 
>> capturemode field
>> to select video/preview/still modes on the sensor, which actually changes 
>> the list
>> of supported resolutions.
>>
>> The following files use this:
>>
>> i2c/atomisp-gc0310.c
>> i2c/atomisp-gc2235.c
>> i2c/atomisp-ov2680.c
>> i2c/atomisp-ov2722.c
>> i2c/ov5693/atomisp-ov5693.c
>> pci/atomisp2/atomisp_file.c
>> pci/atomisp2/atomisp_tpg.c
>>
>> The last two have a dummy g/s_parm implementation, so are easy to fix.
>> The gc0310 and 0v2680 have identical resolution lists for all three modes, so
>> the capturemode can just be ignored and these two drivers can be simplified.
>>
>> Looking at the higher level code it turns out that this atomisp driver 
>> appears
>> to be in the middle of a conversion from using s_parm to a V4L2_CID_RUN_MODE
>> control. If the control is present, then that will be used to set the mode,
>> otherwise it falls back to s_parm.
>>
>> So the best solution would be if Intel can convert the remaining drivers from
>> using s_parm to the new control and then drop all code that uses s_parm to do
>> this, so g/s_parm is then only used to get/set the framerate.
>>
>> Is this something you or a colleague can take on?
> 
> I've stabbed the atomisp sensor drivers enough to remove the s_parm and
> g_parm usage there. This effectively removes the s_parm abuse, as there was
> nothing else it was being used for.
> 
> The patches are here; there are no changes to your patches in the branch
> you pointed me to:
> 
> 
> 
> I've split dropping support for certain modes in the drivers into separate
> patch; it's easy to bring them back by just reverting the patch ("staging:
> atomisp: i2c: Disable non-preview configurations") or removing the ifdefs.
> I don't object merging this with the previous patch either.
> 
> What comes to the run mode control --- this logic should have always
> resided in user space; that control (and s_parm hack) is basically getting
> around lack of support for MC / V4L2 sub-device interface in the driver. So
> that control isn't the right solution either going forward.
> 
> Cc Andy and Alan.
> 

Looks good. Just one note: in atomisp_ioctl.c the atomisp_g_parm function still
abuses this API (setting capturemode) but more importantly, it never calls
g_frame_interval. The atomisp_s_parm function *does* call s_frame_interval.

So this is inconsistent. However, this was always there, so it's not something
that was introduced by these changes.

Regards,

Hans

Re: [PATCH v6 3/9] v4l: platform: Add Renesas CEU driver

[Hans Verkuil]

On 22/01/18 01:52, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Friday, 19 January 2018 14:25:39 EET Hans Verkuil wrote:
>> On 01/19/18 13:20, Laurent Pinchart wrote:
>>> On Friday, 19 January 2018 13:20:19 EET Hans Verkuil wrote:
 On 01/16/18 22:44, Jacopo Mondi wrote:
> Add driver for Renesas Capture Engine Unit (CEU).
>
> The CEU interface supports capturing 'data' (YUV422) and 'images'
> (NV[12|21|16|61]).
>
> This driver aims to replace the soc_camera-based sh_mobile_ceu one.
>
> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> platform GR-Peach.
>
> Tested with ov7725 camera sensor on SH4 platform Migo-R.
>
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 
> ---
>
>  drivers/media/platform/Kconfig   |9 +
>  drivers/media/platform/Makefile  |1 +
>  drivers/media/platform/renesas-ceu.c | 1659 +++
>  3 files changed, 1669 insertions(+)
>  create mode 100644 drivers/media/platform/renesas-ceu.c
>>>
>>> [snip]
>>>
> diff --git a/drivers/media/platform/renesas-ceu.c
> b/drivers/media/platform/renesas-ceu.c new file mode 100644
> index 000..1b8f0ef
> --- /dev/null
> +++ b/drivers/media/platform/renesas-ceu.c
>>>
>>> [snip]
>>>
> +static void ceu_update_plane_sizes(struct v4l2_plane_pix_format *plane,
> +unsigned int bpl, unsigned int szimage)
> +{
> + if (plane->bytesperline < bpl)
> + plane->bytesperline = bpl;
> + if (plane->sizeimage < szimage)
> + plane->sizeimage = szimage;

 As mentioned in your cover letter, you do need to check for invalid
 bytesperline values. The v4l2-compliance test is to see what happens
 when userspace gives insane values, so yes, drivers need to be able
 to handle that.
>>>
>>> What limit would you set, what is an acceptable large value versus an
>>> invalid large value ? I think we should have rules for this at the API
>>> level (or at least, if not part of the API, rules that are consistent
>>> across drivers).
>>
>> I would expect this to be the max of what the hardware can support. If
>> that's really high, then this can be, say, 4 times the width.
>>
>> Note that there are very few drivers that can handle a user-specified
>> stride.
> 
> But that's no reason not to handle it here if the hardware permits, right ? 
> :-)

Certainly. But it is the reason why it's hard to find example code in
existing drivers.

 plane->sizeimage is set by the driver, so drop the 'if' before the
 assignment.
>>>
>>> I don't think that's correct. Userspace should be able to control padding
>>> lines at the end of the image, the same way it controls padding pixels at
>>> the end of lines.
>>
>> If userspace wants larger buffers, then it should use VIDIOC_CREATE_BUFS.
>>
>> sizeimage is exclusively set by the driver, applications rely on that.
> 
> The API documentation is pretty confusing about this.
> 
> In pixfmt-v4l2.rst, the field in the v4l2_pix_format structure is documented 
> as
> 
>   - Size in bytes of the buffer to hold a complete image, set by the
> driver. Usually this is ``bytesperline`` times ``height``. When
> the image consists of variable length compressed data this is the
> maximum number of bytes required to hold an image.
> 
> Then in pixfmt-v4l2-mplane.rst, the field in the v4l2_plane_pix_format 
> structure is documented as
> 
>   - Maximum size in bytes required for image data in this plane.

This should contain the same text as in pixfmt-v4l2.rst.

> 
> Finally, in vidioc-create-bufs.rst, we have
> 
> The buffers created by this ioctl will have as minimum size the size
> defined by the ``format.pix.sizeimage`` field (or the corresponding
> fields for other format types). Usually if the ``format.pix.sizeimage``
> field is less than the minimum required for the given format, then an
> error will be returned since drivers will typically not allow this. If
> it is larger, then the value will be used as-is. In other words, the
> driver may reject the requested size, but if it is accepted the driver
> will use it unchanged.
> 
> The VIDIOC_CREATE_BUFS documentation contradicts the v4l2_pix_format 
> documentation, as the multiplane case doesn't state anything about who sets 
> the sizeimage field. We should clarify the documentation.

The pixfmt-v4l2-mplane.rst is the one that's incomplete. I noticed the same
thing when I looked at it.

> 
> +}
>>>
>>> [snip]
>>>
> +static const struct v4l2_ioctl_ops ceu_ioctl_ops = {
> + .vidioc_querycap= ceu_querycap,
> +
> + .vidioc_enum_fmt_vid_cap_mplane = ceu_enum_fmt_vid_cap,
> + .vidioc_try_fmt_vid_cap_mplane  = ceu_try_fmt_vid_cap,
> + .vidioc_s_fmt_vid_cap_mplane= ceu_s_fmt_vid_cap,
> + 

Re: [PATCH v4] media: imx258: Add imx258 camera sensor driver

[Sakari Ailus]

On Mon, Jan 22, 2018 at 10:37:43AM +0200, Sakari Ailus wrote:
> Hi Andy,
> 
> On Mon, Jan 22, 2018 at 08:03:23AM +, Yeh, Andy wrote:
> > Hi Sakari, Tomasz,
> > 
> > As below discussion that other drivers are with this pattern, I would 
> > prefer to defer to address the concern in later discussion with you and 
> > owners of other sensors.
> > 
> > Thanks a lot.
> 
> I thought of taking a look into the problem area and one sensor driver
> which doesn't appear to have the problem in this respect is imx258. This is
> because the v4l2_ctrl_handler_setup() isn't called in a runtime PM
> callback, but through V4L2 sub-dev s_stream callback instead. The runtime
> PM transition has already taken place by then. This isn't entirely optimal
> but works. Other sensor drivers will still need to be fixed.

And by a quick check we seem to have no sensor drivers doing this after
all... the v4l2_ctrl_handler_setup is called in the s_stream callback.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [PATCH v4] media: imx258: Add imx258 camera sensor driver

[Sakari Ailus]

Hi Andy,

On Mon, Jan 22, 2018 at 08:03:23AM +, Yeh, Andy wrote:
> Hi Sakari, Tomasz,
> 
> As below discussion that other drivers are with this pattern, I would prefer 
> to defer to address the concern in later discussion with you and owners of 
> other sensors.
> 
> Thanks a lot.

I thought of taking a look into the problem area and one sensor driver
which doesn't appear to have the problem in this respect is imx258. This is
because the v4l2_ctrl_handler_setup() isn't called in a runtime PM
callback, but through V4L2 sub-dev s_stream callback instead. The runtime
PM transition has already taken place by then. This isn't entirely optimal
but works. Other sensor drivers will still need to be fixed.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [PATCH v4] media: imx258: Add imx258 camera sensor driver

[Sakari Ailus]

On Mon, Jan 22, 2018 at 08:03:23AM +, Yeh, Andy wrote:
> Hi Sakari, Tomasz,
> 
> As below discussion that other drivers are with this pattern, I would
> prefer to defer to address the concern in later discussion with you and
> owners of other sensors.
> 
> Thanks a lot.

Works for me.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

[PATCH v2 1/1] imx258: Fix sparse warnings

[Sakari Ailus]

Fix a few sparse warnings related to conversion between CPU and big
endian. Also simplify the code in the process.

Signed-off-by: Sakari Ailus 
---
Hi Andy,

I think I figured out the problem. Could you test this?

Thanks.

since v1:

- Fix pointer passed to i2c_master_send. This is the entire buffer, not
  the next character put to the buffer.

 drivers/media/i2c/imx258.c | 23 +--
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index a7e58bd23de7..2ff9a1538cb5 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -440,10 +440,10 @@ static int imx258_read_reg(struct imx258 *imx258, u16 
reg, u32 len, u32 *val)
 {
struct i2c_client *client = v4l2_get_subdevdata(>sd);
struct i2c_msg msgs[2];
+   __be16 reg_addr_be = cpu_to_be16(reg);
+   __be32 data_be = 0;
u8 *data_be_p;
int ret;
-   u32 data_be = 0;
-   u16 reg_addr_be = cpu_to_be16(reg);
 
if (len > 4)
return -EINVAL;
@@ -474,24 +474,19 @@ static int imx258_read_reg(struct imx258 *imx258, u16 
reg, u32 len, u32 *val)
 static int imx258_write_reg(struct imx258 *imx258, u16 reg, u32 len, u32 val)
 {
struct i2c_client *client = v4l2_get_subdevdata(>sd);
-   int buf_i, val_i;
-   u8 buf[6], *val_p;
+   u8 __buf[6], *buf = __buf;
+   int i;
 
if (len > 4)
return -EINVAL;
 
-   buf[0] = reg >> 8;
-   buf[1] = reg & 0xff;
+   *buf++ = reg >> 8;
+   *buf++ = reg & 0xff;
 
-   val = cpu_to_be32(val);
-   val_p = (u8 *)
-   buf_i = 2;
-   val_i = 4 - len;
+   for (i = len - 1; i >= 0; i++)
+   *buf++ = (u8)(val >> (i << 3));
 
-   while (val_i < 4)
-   buf[buf_i++] = val_p[val_i++];
-
-   if (i2c_master_send(client, buf, len + 2) != len + 2)
+   if (i2c_master_send(client, __buf, len + 2) != len + 2)
return -EIO;
 
return 0;
-- 
2.11.0

RE: [PATCH 1/1] imx258: Fix sparse warnings

[Yeh, Andy]

Hi Sakari,

I verified the patch multiple times but the code cannot work. 


Regards, Andy

-Original Message-
From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com] 
Sent: Friday, January 19, 2018 5:23 PM
To: linux-media@vger.kernel.org
Cc: Yeh, Andy 
Subject: [PATCH 1/1] imx258: Fix sparse warnings

Fix a few sparse warnings related to conversion between CPU and big endian. 
Also simplify the code in the process.

Signed-off-by: Sakari Ailus 
---
Hi Andy,

There were a few issues Sparse found in the imx258 driver. Could you test the 
patch, please?

 drivers/media/i2c/imx258.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c index 
a7e58bd23de7..b73c25ae8725 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -440,10 +440,10 @@ static int imx258_read_reg(struct imx258 *imx258, u16 
reg, u32 len, u32 *val)  {
struct i2c_client *client = v4l2_get_subdevdata(>sd);
struct i2c_msg msgs[2];
+   __be16 reg_addr_be = cpu_to_be16(reg);
+   __be32 data_be = 0;
u8 *data_be_p;
int ret;
-   u32 data_be = 0;
-   u16 reg_addr_be = cpu_to_be16(reg);
 
if (len > 4)
return -EINVAL;
@@ -474,22 +474,17 @@ static int imx258_read_reg(struct imx258 *imx258, u16 
reg, u32 len, u32 *val)  static int imx258_write_reg(struct imx258 *imx258, u16 
reg, u32 len, u32 val)  {
struct i2c_client *client = v4l2_get_subdevdata(>sd);
-   int buf_i, val_i;
-   u8 buf[6], *val_p;
+   u8 __buf[6], *buf = __buf;
+   int i;
 
if (len > 4)
return -EINVAL;
 
-   buf[0] = reg >> 8;
-   buf[1] = reg & 0xff;
+   *buf++ = reg >> 8;
+   *buf++ = reg & 0xff;
 
-   val = cpu_to_be32(val);
-   val_p = (u8 *)
-   buf_i = 2;
-   val_i = 4 - len;
-
-   while (val_i < 4)
-   buf[buf_i++] = val_p[val_i++];
+   for (i = len - 1; i >= 0; i++)
+   *buf++ = (u8)(val >> (i << 3));
 
if (i2c_master_send(client, buf, len + 2) != len + 2)
return -EIO;
--
2.11.0



sakari_new.diff
Description: sakari_new.diff

RE: [PATCH v4] media: imx258: Add imx258 camera sensor driver

[Yeh, Andy]

Hi Sakari, Tomasz,

As below discussion that other drivers are with this pattern, I would prefer to 
defer to address the concern in later discussion with you and owners of other 
sensors.

Thanks a lot.

Regards, Andy

-Original Message-
From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com] 
Sent: Friday, January 19, 2018 5:18 PM
To: Yeh, Andy 
Cc: Tomasz Figa ; Linux Media Mailing List 

Subject: Re: [PATCH v4] media: imx258: Add imx258 camera sensor driver

Hi Andy,

On Fri, Jan 19, 2018 at 07:29:46AM +, Yeh, Andy wrote:
> Thanks Tomasz,
> 
> Agree with your point, if so, we could just change as below with a simple 
> check of streaming flag.
> And for Sakari, do you agree with Tomasz's comment?
> 
> Kindly review and I would send v5 with the change.
> 
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c 
> index a7e58bd2..cf1c5ee 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -561,10 +561,13 @@ static int imx258_set_ctrl(struct v4l2_ctrl 
> *ctrl)
> 
> /*
>  * Applying V4L2 control value only happens
> -* when power is up for qstreaming
> +* when streaming flag is on
>  */
> -   if (pm_runtime_get_if_in_use(>dev) <= 0)
> +   if (imx258->streaming == 0)

This doesn't address the problem yet. I think we'll need one more field in the 
device specific struct to convey this to the driver. Please see the smiapp 
driver, and its use of "active" field.

It's a little different implementation, you could well put the check here 
rather than the function performing the writes.

This isn't a severe issue though, in practice it'll be unlikely to be noticed 
as it hasn't been noticed in some other drivers that use the same pattern. IMO 
this could be addressed later on, possibly together with other drivers with 
similar issues.

> return 0;
> 
> switch (ctrl->id) {
> case V4L2_CID_ANALOGUE_GAIN:
> @@ -590,8 +593,6 @@ static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
> break;
> }
> 
> -   pm_runtime_put(>dev);
> -
> return ret;
>  }

--
Kind regards,

Sakari Ailus
sakari.ai...@linux.intel.com