Re: [PATCH] media:dvb-frontends:Return if Max devices are added in dvb_pll_attach().

[Vandana BN]

On 19/07/19 7:11 AM, Akihiro TSUKADA wrote:
>> Will it be better, if dvb_pll_devcount is decremented in dvb_pll_release(),  
>> instead of removing module params?
> But you cannot know deterministically which device corrensponds to
> what "id" (when you have multiple dvb_pll devices),
> since "id" is dependent on the history of register and unregister
> of dvb_pll devices.
dvb_pll_release() frees  fe->tuner_priv, and priv->nr is dvb_pll_devcount, but, 
decrementing  count will only tell array has a free slot, and now if that free 
slot needs to be used it will have to either maintain free index list to be 
consumed next or convert array id to a list.
> So I wonder about the benefit / practical usecase of "id" parameter.

Ok, I will remove the module parameters and send a patch.

Thanks,

Vandana.

>
> --
> Akihiro

Re: [PATCH] media/i2c: don't return ENOTTY if SUBDEV_API is not set

[Lad, Prabhakar]

Hi Hans,

Thank you for the patch.

On Wed, Jul 17, 2019 at 10:24 AM Hans Verkuil  wrote:
>
> If CONFIG_VIDEO_V4L2_SUBDEV_API is not set, then it is still possible
> to call set_fmt for V4L2_SUBDEV_FORMAT_TRY, the result is just not
> stored. So return 0 instead of -ENOTTY.
>
> Calling get_fmt with V4L2_SUBDEV_FORMAT_TRY should return -EINVAL
> instead of -ENOTTY, after all the get_fmt functionality is still
> present, just not supported for TRY.
>
> Signed-off-by: Hans Verkuil 
> ---
> This was fixed for the ov7670 (https://patchwork.linuxtv.org/patch/57584/) 
> when
> working on the via-camera driver, but the same pattern is found in other 
> drivers,
> and those are fixed in this patch.
> ---
>  drivers/media/i2c/mt9m111.c | 2 +-
>  drivers/media/i2c/ov2640.c  | 2 +-
>  drivers/media/i2c/ov2659.c  | 4 +---

for ov2659 changes,

Acked-by: Lad, Prabhakar 

Cheers,
--Prabhakar Lad

>  drivers/media/i2c/ov2680.c  | 5 +
>  drivers/media/i2c/ov5695.c  | 5 +
>  drivers/media/i2c/ov7740.c  | 8 ++--
>  6 files changed, 7 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> index d10fe3712036..d4864d155f0b 100644
> --- a/drivers/media/i2c/mt9m111.c
> +++ b/drivers/media/i2c/mt9m111.c
> @@ -536,7 +536,7 @@ static int mt9m111_get_fmt(struct v4l2_subdev *sd,
> format->format = *mf;
> return 0;
>  #else
> -   return -ENOTTY;
> +   return -EINVAL;
>  #endif
> }
>
> diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
> index 30e7e6b2b293..39474b287bb2 100644
> --- a/drivers/media/i2c/ov2640.c
> +++ b/drivers/media/i2c/ov2640.c
> @@ -932,7 +932,7 @@ static int ov2640_get_fmt(struct v4l2_subdev *sd,
> format->format = *mf;
> return 0;
>  #else
> -   return -ENOTTY;
> +   return -EINVAL;
>  #endif
> }
>
> diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
> index 5ed2413eac8a..a71277e361ff 100644
> --- a/drivers/media/i2c/ov2659.c
> +++ b/drivers/media/i2c/ov2659.c
> @@ -1055,7 +1055,7 @@ static int ov2659_get_fmt(struct v4l2_subdev *sd,
> mutex_unlock(&ov2659->lock);
> return 0;
>  #else
> -   return -ENOTTY;
> +   return -EINVAL;
>  #endif
> }
>
> @@ -1131,8 +1131,6 @@ static int ov2659_set_fmt(struct v4l2_subdev *sd,
>  #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
> mf = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
> *mf = fmt->format;
> -#else
> -   ret = -ENOTTY;
>  #endif
> } else {
> s64 val;
> diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
> index b10bcfabaeeb..164f983c1814 100644
> --- a/drivers/media/i2c/ov2680.c
> +++ b/drivers/media/i2c/ov2680.c
> @@ -675,7 +675,7 @@ static int ov2680_get_fmt(struct v4l2_subdev *sd,
>  #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
> fmt = v4l2_subdev_get_try_format(&sensor->sd, cfg, 
> format->pad);
>  #else
> -   ret = -ENOTTY;
> +   ret = -EINVAL;
>  #endif
> } else {
> fmt = &sensor->fmt;
> @@ -723,10 +723,7 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd,
>  #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
> try_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
> format->format = *try_fmt;
> -#else
> -   ret = -ENOTTY;
>  #endif
> -
> goto unlock;
> }
>
> diff --git a/drivers/media/i2c/ov5695.c b/drivers/media/i2c/ov5695.c
> index e65a94353175..34b7046d9702 100644
> --- a/drivers/media/i2c/ov5695.c
> +++ b/drivers/media/i2c/ov5695.c
> @@ -823,9 +823,6 @@ static int ov5695_set_fmt(struct v4l2_subdev *sd,
> if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
>  #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
> *v4l2_subdev_get_try_format(sd, cfg, fmt->pad) = fmt->format;
> -#else
> -   mutex_unlock(&ov5695->mutex);
> -   return -ENOTTY;
>  #endif
> } else {
> ov5695->cur_mode = mode;
> @@ -856,7 +853,7 @@ static int ov5695_get_fmt(struct v4l2_subdev *sd,
> fmt->format = *v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
>  #else
> mutex_unlock(&ov5695->mutex);
> -   return -ENOTTY;
> +   return -EINVAL;
>  #endif
> } else {
> fmt->format.width = mode->width;
> diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
> index 70bb870b1d08..a2f8f19bca7c 100644
> --- a/drivers/media/i2c/ov7740.c
> +++ b/drivers/media/i2c/ov7740.c
> @@ -827,13 +827,9 @@ static int ov7740_set_fmt(struct v4l2_subdev *sd,
>  #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
> mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
> *mbus_fmt = format->format;
> -
> +#endif
> mutex_unlock(&ov7740->mutex);
> return 0;
> -#else
> - 

Re: [PATCH] media/i2c: don't return ENOTTY if SUBDEV_API is not set

[Rui Miguel Silva]

Hi Hans,
On Wed 17 Jul 2019 at 10:24, Hans Verkuil wrote:
> If CONFIG_VIDEO_V4L2_SUBDEV_API is not set, then it is still possible
> to call set_fmt for V4L2_SUBDEV_FORMAT_TRY, the result is just not
> stored. So return 0 instead of -ENOTTY.
>
> Calling get_fmt with V4L2_SUBDEV_FORMAT_TRY should return -EINVAL
> instead of -ENOTTY, after all the get_fmt functionality is still
> present, just not supported for TRY.
>
> Signed-off-by: Hans Verkuil 
> ---
> This was fixed for the ov7670 (https://patchwork.linuxtv.org/patch/57584/) 
> when
> working on the via-camera driver, but the same pattern is found in other 
> drivers,
> and those are fixed in this patch.
> ---
>  drivers/media/i2c/mt9m111.c | 2 +-
>  drivers/media/i2c/ov2640.c  | 2 +-
>  drivers/media/i2c/ov2659.c  | 4 +---
>  drivers/media/i2c/ov2680.c  | 5 +
>  drivers/media/i2c/ov5695.c  | 5 +
>  drivers/media/i2c/ov7740.c  | 8 ++--
>  6 files changed, 7 insertions(+), 19 deletions(-)
>

for the ov2680:
Acked-by: Rui Miguel Silva 

---
Cheers,
Rui


> diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> index d10fe3712036..d4864d155f0b 100644
> --- a/drivers/media/i2c/mt9m111.c
> +++ b/drivers/media/i2c/mt9m111.c
> @@ -536,7 +536,7 @@ static int mt9m111_get_fmt(struct v4l2_subdev *sd,
>   format->format = *mf;
>   return 0;
>  #else
> - return -ENOTTY;
> + return -EINVAL;
>  #endif
>   }
>
> diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
> index 30e7e6b2b293..39474b287bb2 100644
> --- a/drivers/media/i2c/ov2640.c
> +++ b/drivers/media/i2c/ov2640.c
> @@ -932,7 +932,7 @@ static int ov2640_get_fmt(struct v4l2_subdev *sd,
>   format->format = *mf;
>   return 0;
>  #else
> - return -ENOTTY;
> + return -EINVAL;
>  #endif
>   }
>
> diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
> index 5ed2413eac8a..a71277e361ff 100644
> --- a/drivers/media/i2c/ov2659.c
> +++ b/drivers/media/i2c/ov2659.c
> @@ -1055,7 +1055,7 @@ static int ov2659_get_fmt(struct v4l2_subdev *sd,
>   mutex_unlock(&ov2659->lock);
>   return 0;
>  #else
> - return -ENOTTY;
> + return -EINVAL;
>  #endif
>   }
>
> @@ -1131,8 +1131,6 @@ static int ov2659_set_fmt(struct v4l2_subdev *sd,
>  #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
>   mf = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
>   *mf = fmt->format;
> -#else
> - ret = -ENOTTY;
>  #endif
>   } else {
>   s64 val;
> diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
> index b10bcfabaeeb..164f983c1814 100644
> --- a/drivers/media/i2c/ov2680.c
> +++ b/drivers/media/i2c/ov2680.c
> @@ -675,7 +675,7 @@ static int ov2680_get_fmt(struct v4l2_subdev *sd,
>  #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
>   fmt = v4l2_subdev_get_try_format(&sensor->sd, cfg, format->pad);
>  #else
> - ret = -ENOTTY;
> + ret = -EINVAL;
>  #endif
>   } else {
>   fmt = &sensor->fmt;
> @@ -723,10 +723,7 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd,
>  #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
>   try_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
>   format->format = *try_fmt;
> -#else
> - ret = -ENOTTY;
>  #endif
> -
>   goto unlock;
>   }
>
> diff --git a/drivers/media/i2c/ov5695.c b/drivers/media/i2c/ov5695.c
> index e65a94353175..34b7046d9702 100644
> --- a/drivers/media/i2c/ov5695.c
> +++ b/drivers/media/i2c/ov5695.c
> @@ -823,9 +823,6 @@ static int ov5695_set_fmt(struct v4l2_subdev *sd,
>   if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
>  #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
>   *v4l2_subdev_get_try_format(sd, cfg, fmt->pad) = fmt->format;
> -#else
> - mutex_unlock(&ov5695->mutex);
> - return -ENOTTY;
>  #endif
>   } else {
>   ov5695->cur_mode = mode;
> @@ -856,7 +853,7 @@ static int ov5695_get_fmt(struct v4l2_subdev *sd,
>   fmt->format = *v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
>  #else
>   mutex_unlock(&ov5695->mutex);
> - return -ENOTTY;
> + return -EINVAL;
>  #endif
>   } else {
>   fmt->format.width = mode->width;
> diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
> index 70bb870b1d08..a2f8f19bca7c 100644
> --- a/drivers/media/i2c/ov7740.c
> +++ b/drivers/media/i2c/ov7740.c
> @@ -827,13 +827,9 @@ static int ov7740_set_fmt(struct v4l2_subdev *sd,
>  #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
>   mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
>   *mbus_fmt = format->format;
> -
> +#endif
>   mutex_unlock(&ov7740->mutex);
>   return 0;
> -#else
> - ret = -ENOTTY;
> - goto error;
> -#endif
>   }
>
>   ret = ov7740_try_fmt_internal(sd, &format->format, &ov

Re: [RFC PATCH 0/5] Add enum_fmt flag for coded formats with dynamic resolution switching

[Hans Verkuil]

On 7/19/19 4:45 AM, Tomasz Figa wrote:
> On Mon, Jul 15, 2019 at 9:37 PM Hans Verkuil  wrote:
>>
>> On 6/11/19 10:13 AM, Hans Verkuil wrote:
>>> On 6/9/19 4:38 PM, Maxime Jourdan wrote:
 Hello,

 This RFC proposes a new format flag - V4L2_FMT_FLAG_DYN_RESOLUTION - used
 to tag coded formats for which the device supports dynamic resolution
 switching, via V4L2_EVENT_SOURCE_CHANGE.
 This includes the initial "source change" where the device is able to
 tell userspace about the coded resolution and the DPB size (which
 sometimes translates to V4L2_CID_MIN_BUFFERS_FOR_CAPTURE).
>>>
>>> Shouldn't the initial source change still be there? The amlogic decoder
>>> is capable of determining the resolution of the stream, right? It just
>>> can't handle mid-stream changes.
>>
>> I've been thinking about this a bit more: there are three different HW 
>> capabilities:
>>
>> 1) The hardware cannot parse the resolution at all and userspace has to tell 
>> it
>> via S_FMT.
>>
>> 2) The hardware can parse the initial resolution, but is not able to handle
>> mid-stream resolution changes.
>>
>> 3) The hardware can parse the initial resolution and all following mid-stream
>> resolution changes.
>>
>> We can consider 2 the default situation.
> 
> Any particular reason for 2 being the default? I'm especially
> wondering about that as most of the drivers actually provide 3.

Various reasons:

1) I prefer to have a flag indicating what IS supported rather than what
   isn't.
2) An application that checks this flag and doesn't see it (i.e. if a
   flag-aware application is used with an older kernel where these flags
   aren't set) will still work, but with possibly reduced functionality.
   If the flag would indicate that something is NOT supported, then they
   would fail when combined with an older kernel and a driver that doesn't
   support dynamic resolution changes.
3) None of the encoders support it, so there too it makes sense to have
   'no dynamic resolution change' as the default. It's nicely symmetrical
   for encoders and decoders.
4) Some formats do not support it, so again, having no dynamic res changes
   as the default makes sense.

Regards,

Hans

> 
>>
>> In case of 1 the SOURCE_CHANGE event is absent and userspace cannot subscribe
>> to it. Question: do we want to flag this with the format as well? I.e. with a
>> V4L2_FMT_FLAG_MANUAL_RESOLUTION? I think just not implementing the 
>> SOURCE_CHANGE
>> event (and documenting this) is sufficient.
>>
>> In case of 3 the format sets the V4L2_FMT_FLAG_DYN_RESOLUTION flag.
>>
>> What do you think?
>>
>> Regards,
>>
>> Hans
>>
>>>
>>> Regards,
>>>
>>>   Hans
>>>
 This flag is mainly aimed at stateful decoder drivers.

 This RFC is motivated by my development on the amlogic video decoder
 driver, which does not support dynamic resolution switching for older
 coded formats (MPEG 1/2, MPEG 4 part II, H263). It does however support
 it for the newer formats (H264, HEVC, VP9).

 The specification regarding stateful video decoders should be amended
 to include that, in the absence of this flag for a certain format,
 userspace is expected to extract the coded resolution and allocate
 a sufficient amount of capture buffers on its own.
 I understand that this point may be tricky, since older kernels with
 close-to-spec drivers would not have this flag available, yet would
 fully support dynamic resolution switching.
 However, with the spec not merged in yet, I wanted to have your opinion
 on this late addition.

 The RFC patches also adds support for this flag for the 4 following
 stateful decoder drivers:
  - venus
  - s5p-mfc
  - mtk-vcodec
  - vicodec

 Maxime Jourdan (5):
   media: videodev2: add V4L2_FMT_FLAG_DYN_RESOLUTION
   media: venus: vdec: flag OUTPUT formats with
 V4L2_FMT_FLAG_DYN_RESOLUTION
   media: s5p_mfc_dec: flag OUTPUT formats with
 V4L2_FMT_FLAG_DYN_RESOLUTION
   media: mtk-vcodec: flag OUTPUT formats with
 V4L2_FMT_FLAG_DYN_RESOLUTION
   media: vicodec: flag vdec/stateful OUTPUT formats with
 V4L2_FMT_FLAG_DYN_RESOLUTION

  Documentation/media/uapi/v4l/vidioc-enum-fmt.rst   |  7 +++
  drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c |  4 
  drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h |  1 +
  drivers/media/platform/qcom/venus/core.h   |  1 +
  drivers/media/platform/qcom/venus/vdec.c   | 11 +++
  drivers/media/platform/s5p-mfc/s5p_mfc_common.h|  1 +
  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c   | 13 +
  drivers/media/platform/vicodec/vicodec-core.c  |  2 ++
  include/uapi/linux/videodev2.h |  5 +++--
  9 files changed, 43 insertions(+), 2 deletions(-)

>>>
>>

Re: [PATCH 3/4] rcar-vin: Add support for V4L2_FIELD_ALTERNATE

[Kieran Bingham]

Hi Niklas,

On 05/07/2019 05:55, Niklas Söderlund wrote:
> The hardware is capable to passing V4L2_FIELD_ALTERNATE to user-space.
> Allow users to request this field format but still default to using the
> hardware interlacer if alternating is not explicitly requested.

I'm afraid I have found this patch quite difficult to review accurately ...

I think I can infer that we are removing an existing workaround where
V4L2_FIELD_ALTERNATE was converted to V4L2_FIELD_INTERLACED_xx formats,
and also now where we used to store 'frame' heights, we store 'field'
heights...

Is that somewhere close as an approximation? (Perhaps it might be good
to detail some of that in the commit message, at least any bits that are
accurate of course)


I might have to look at this again later, or let some other eyeballs
look as I'm afraid I don't feel that I've got a good overview of it yet.

I wonder if it could be split in anyway to be clearer, but it's hard to
tell :-)

Perhaps it's just me being unable to see all the changes at once.


< Some of my discussion comments below might seem out of order, as I've
made multiple passes through this :-D >

> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c  | 54 +++--
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 31 +---
>  2 files changed, 42 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c 
> b/drivers/media/platform/rcar-vin/rcar-dma.c
> index 372d6b106b9970d2..7ac1733455221fe0 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -526,12 +526,17 @@ static void rvin_set_coeff(struct rvin_dev *vin, 
> unsigned short xs)
>  
>  static void rvin_crop_scale_comp_gen2(struct rvin_dev *vin)
>  {
> + unsigned int crop_height;
>   u32 xs, ys;
>  
>   /* Set scaling coefficient */
> + crop_height = vin->crop.height;
> + if (V4L2_FIELD_IS_INTERLACED(vin->format.field))
> + crop_height *= 2;
> +
>   ys = 0;
> - if (vin->crop.height != vin->compose.height)
> - ys = (4096 * vin->crop.height) / vin->compose.height;
> + if (crop_height != vin->compose.height)
> + ys = (4096 * crop_height) / vin->compose.height;
>   rvin_write(vin, ys, VNYS_REG);
>  
>   xs = 0;
> @@ -554,16 +559,11 @@ static void rvin_crop_scale_comp_gen2(struct rvin_dev 
> *vin)
>   rvin_write(vin, 0, VNSPPOC_REG);
>   rvin_write(vin, 0, VNSLPOC_REG);
>   rvin_write(vin, vin->format.width - 1, VNEPPOC_REG);
> - switch (vin->format.field) {
> - case V4L2_FIELD_INTERLACED:
> - case V4L2_FIELD_INTERLACED_TB:
> - case V4L2_FIELD_INTERLACED_BT:
> +
> + if (V4L2_FIELD_IS_INTERLACED(vin->format.field))

Ok, so I had to go check - V4L2_FIELD_IS_INTERLACED() does not include
'_ALTERNATE' - so this hunk is an improvement, but a somewhat unrelated
change.

Perhaps this could be split out to before this patch, anything to
simplify this patch would be good.

>   rvin_write(vin, vin->format.height / 2 - 1, VNELPOC_REG);
> - break;
> - default:
> + else
>   rvin_write(vin, vin->format.height - 1, VNELPOC_REG);
> - break;
> - }
>  
>   vin_dbg(vin,
>   "Pre-Clip: %ux%u@%u:%u YS: %d XS: %d Post-Clip: %ux%u@%u:%u\n",
> @@ -577,21 +577,9 @@ void rvin_crop_scale_comp(struct rvin_dev *vin)
>   /* Set Start/End Pixel/Line Pre-Clip */
>   rvin_write(vin, vin->crop.left, VNSPPRC_REG);
>   rvin_write(vin, vin->crop.left + vin->crop.width - 1, VNEPPRC_REG);
> + rvin_write(vin, vin->crop.top, VNSLPRC_REG);
> + rvin_write(vin, vin->crop.top + vin->crop.height - 1, VNELPRC_REG);

Should those be s/vin->crop.height/crop_height/ ? 

How come there's no comparable if (V4L2_FIELD_IS_INTERLACED... in this
function?

Oh - because actually rvin_crop_scale_comp_gen2() is called from within
this function. They are not parallel functions for two implementations.


>  
> - switch (vin->format.field) {
> - case V4L2_FIELD_INTERLACED:
> - case V4L2_FIELD_INTERLACED_TB:
> - case V4L2_FIELD_INTERLACED_BT:
> - rvin_write(vin, vin->crop.top / 2, VNSLPRC_REG);
> - rvin_write(vin, (vin->crop.top + vin->crop.height) / 2 - 1,
> -VNELPRC_REG);
> - break;

So - I think if i understand correctly - we used to store the
frame-height in vin->crop, and now we store the field height.

Is that right ?
 (where field-height == frame-height on progressive frames)



> - default:
> - rvin_write(vin, vin->crop.top, VNSLPRC_REG);
> - rvin_write(vin, vin->crop.top + vin->crop.height - 1,
> -VNELPRC_REG);
> - break;
> - }
>  
>   /* TODO: Add support for the UDS scaler. */
>   if (vin->info->model != RCAR_GEN3)
> @@ -636,6 +624,9 @@ static int rvin_setup(struct rvin_dev *vin)
>   

scancodes identified as invalid

[JohnG]

I have POWER_KEY scancodes, that work as RAW data with ir-ctl -s [file], 
but are not working with ir-ctl -S:


    0x20df10ef (POWER_KEY for my LG tv)
    0x02FD48B7 (POWER_KEY for my Toshiba tv)

rc6_mce:0x800f0410 works, as does nec32:0x800f0410 (though not 
nec:0x800f0410), so it doesn't seem to be an issue with 32-bit codes.


I use AnalysIR, and it indicates the protocol is NEC for the above 
codes, though irdb.tk indicates the protocol is NEC1.


1) is there a description of the various protocols? (how can I know if I 
should use nec or nec32, with a 32 bit value?)


2) are my scancodes failing to work, because they are (possibly) NEC1 
protocol?


3) if NEC1 is not the same as nec or nec32, is it scheduled to be added 
to ir-ctl in the near future?


JohnG

Re: Issues with ov5640 camera on i.MX6Q

[Laura Nao]

Hi Loic,

Thanks for your reply.

On 7/19/19 8:56 AM, Loic Poulain wrote:

Hi Laura,


On Thu, 18 Jul 2019 at 15:03, Laura Nao > wrote:


Hello Loic,

I'm having some issues with RAW8 mode on the OV5640 camera and I'd like
to kindly ask for your advice, as I saw that you added support for RAW
mode in the mainline kernel driver.

I'm trying to capture some raw images on a i.MX6Q based board. I
configured the pipeline as follows:

media-ctl -l "'ov5640 1-003c':0 -> 'imx6-mipi-csi2':0[1]"
media-ctl -l "'imx6-mipi-csi2':2 -> 'ipu1_csi1':0[1]"
media-ctl -l "'ipu1_csi1':2 -> 'ipu1_csi1 capture':0[1]"
media-ctl -V "'ov5640 1-003c':0 [fmt:SBGGR8_1X8/2592x1944 field:none]"
media-ctl -V "'imx6-mipi-csi2':2 [fmt:SBGGR8_1X8/2592x1944 field:none]"

I'm capturing the frame using v4l-utils:

v4l2-ctl -d /dev/video5 --verbose --set-fmt
video=width=2592,height=1944,pixelformat=BA81 --stream-mmap
--stream-count=1 --stream-to=bggr_2592x1944.raw


Did you also try with other resolutions?


The images I'm obtaining are completely garbled. I tried both 5.2
mainline and 5.1.18 kernels.


Did you try enabling the sensor test pattern, would be interesting to 
check if there is some coherency in the raw buffer.




I did some tests with the color bars test pattern enabled at different 
resolutions. The 640x480 frame seems to be the closest to the test 
pattern, even though the image is still not perfectly aligned.

With increasing resolution, the image gets more garbled.

I uploaded some images with different resolutions here:

https://imgur.com/a/2o9WZMs

As reference, I'm using the bayer2rgb tool to convert the raw files to 
rgb (https://github.com/jdthomas/bayer2rgb).



I definitely need to give a shot to last driver version. Last time I 
tried I backported the driver to a 4.14 tree:

https://git.linaro.org/people/loic.poulain/linux.git/log/?h=qcomlt-4.14-ov5640
And it worked on my side (with dragonboard 410c).


I'm able to correctly capture YUV frames in all resolutions with the
same driver (with the pipeline configured to go through the
ipu1_ic_prpenc node first).

Do you have any insight on what might be causing these issues? Is the
PLL configuration supposed to be changed when RAW8 format is selected?


Well, you can have a lower PCLK with RAW format, but it should work anyway.

Regards,
Loic


I also quickly tested the raw capture with a 4.14 kernel with your 
patches applied, but the resulting images looks pretty much the same 
(640x480 is the only resolution close to the expected frame).


Best,

Laura

Re: MyGica T230 dvb-t2 data corruption since commit 5fa8815

[Jan Pieter van Woerkom]

dvbsky: add MyGica T230.
Moved from cxusb driver as that driver can't handle FX2 FIFO issue.

Signed-off-by: Jan Pieter van Woerkom 
---
diff -ru a/drivers/media/usb/dvb-usb-v2/dvbsky.c 
b/drivers/media/usb/dvb-usb-v2/dvbsky.c
--- a/drivers/media/usb/dvb-usb-v2/dvbsky.c 2019-07-08 00:41:56.0 
+0200
+++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c 2019-07-19 17:50:54.671341146 
+0200
@@ -561,11 +561,18 @@
 
/* attach tuner */
si2157_config.fe = adap->fe[0];
-   si2157_config.if_port = 0;
-
-   state->i2c_client_tuner = dvb_module_probe("si2157", "si2141",
+   if (le16_to_cpu(d->udev->descriptor.idProduct) == USB_PID_MYGICA_T230) {
+   si2157_config.if_port = 1;
+   state->i2c_client_tuner = dvb_module_probe("si2157", NULL,
+  i2c_adapter,
+  0x60, &si2157_config);
+   }
+   else {
+   si2157_config.if_port = 0;
+   state->i2c_client_tuner = dvb_module_probe("si2157", "si2141",
   i2c_adapter,
   0x60, &si2157_config);
+   }
if (!state->i2c_client_tuner) {
dvb_module_release(state->i2c_client_demod);
return -ENODEV;
@@ -787,6 +794,9 @@
{ DVB_USB_DEVICE(USB_VID_TERRATEC, USB_PID_TERRATEC_CINERGY_S2_R4,
&dvbsky_s960_props, "Terratec Cinergy S2 Rev.4",
RC_MAP_DVBSKY) },
+   { DVB_USB_DEVICE(USB_VID_CONEXANT, USB_PID_MYGICA_T230,
+   &mygica_t230c_props, "MyGica Mini DVB-T2 USB Stick T230",
+   RC_MAP_TOTAL_MEDIA_IN_HAND_02) },
{ DVB_USB_DEVICE(USB_VID_CONEXANT, USB_PID_MYGICA_T230C,
&mygica_t230c_props, "MyGica Mini DVB-T2 USB Stick T230C",
RC_MAP_TOTAL_MEDIA_IN_HAND_02) },

Re: [PATCH v3] media: v4l: ctrls: Add debug messages

[Ezequiel Garcia]

On Thu, 2019-06-27 at 09:38 +0200, Hans Verkuil wrote:
> Hi Ezequiel,
> 
> 'checkpatch.pl --strict' finds way too many issues.
> 
> I'm pretty certain you didn't run it.
> 

Shame on me, I did run it and skipped fixing the long lines :-(

> Please fix the issues and post a v4.
> 
> A lot (but not all) of the warnings report lines that are too long
> and from what I can see most are easily fixed without reducing
> readability.
> 

I have fixed all the long lines. IMHO, the remaining warnings
are not worth fixing:

CHECK: Macro argument reuse 'vdev' - possible side-effects?
#77: FILE: drivers/media/v4l2-core/v4l2-ctrls.c:21:
+#define dprintk(vdev, fmt, arg...) do {
\
+   if (!WARN_ON(!vdev) && ((vdev)->dev_debug & V4L2_DEV_DEBUG_CTRL)) \
+   printk(KERN_DEBUG pr_fmt("%s: %s: " fmt),   \
+  __func__, video_device_node_name(vdev), ##arg);  \
+} while (0)

CHECK: Macro argument 'vdev' may be better as '(vdev)' to avoid precedence 
issues
#77: FILE: drivers/media/v4l2-core/v4l2-ctrls.c:21:
+#define dprintk(vdev, fmt, arg...) do {
\
+   if (!WARN_ON(!vdev) && ((vdev)->dev_debug & V4L2_DEV_DEBUG_CTRL)) \
+   printk(KERN_DEBUG pr_fmt("%s: %s: " fmt),   \
+  __func__, video_device_node_name(vdev), ##arg);  \
+} while (0)

WARNING: Prefer [subsystem eg: netdev]_dbg([subsystem]dev, ... then 
dev_dbg(dev, ... then pr_debug(...  to printk(KERN_DEBUG ...
#79: FILE: drivers/media/v4l2-core/v4l2-ctrls.c:23:
+   printk(KERN_DEBUG pr_fmt("%s: %s: " fmt),   \

CHECK: Comparison to NULL could be written "!ref"
#116: FILE: drivers/media/v4l2-core/v4l2-ctrls.c:3255:
+   if (ref == NULL) {

CHECK: Comparison to NULL could be written "!hdl"
#249: FILE: drivers/media/v4l2-core/v4l2-ctrls.c:3730:
+   if (hdl == NULL) {

I believe the dprintk macro is OK as-is. We use printk KERN_DEBUG because
we already have a parameter guarding it. This is consistent
with the v4l2 way.

The NULL comparison warning is valid, but it was already
like this before. I'd rather not change existing code.

I'll post a v4.

Ezequiel

> Regards,
> 
>   Hans
> 
> On 6/22/19 12:28 PM, Ezequiel Garcia wrote:
> > Currently, the v4l2 control code is a bit silent on errors.
> > Add debug messages on (hopefully) most of the error paths.
> > 
> > Signed-off-by: Ezequiel Garcia 
> > ---
> > Changes from v2:
> > * Check and noisy-warn if vdev is NULL.
> > 
> > Changes from v1:
> > * Drop changes in the debug parameter semantics.
> > * Drop new module debug parameter.
> > * Add documentation.
> > * Add a debug error in all places where control can fail.
> > * Reorder the vdev parameter, to make the patch less invasive.
> > ---
> >  Documentation/media/kapi/v4l2-dev.rst  |   1 +
> >  drivers/media/platform/omap3isp/ispvideo.c |   2 +-
> >  drivers/media/v4l2-core/v4l2-ctrls.c   | 106 -
> >  drivers/media/v4l2-core/v4l2-ioctl.c   |  12 +--
> >  drivers/media/v4l2-core/v4l2-subdev.c  |   6 +-
> >  include/media/v4l2-ctrls.h |   9 +-
> >  include/media/v4l2-ioctl.h |   2 +
> >  7 files changed, 100 insertions(+), 38 deletions(-)
> > 
> > diff --git a/Documentation/media/kapi/v4l2-dev.rst 
> > b/Documentation/media/kapi/v4l2-dev.rst
> > index b359f1804bbe..4c5a15c53dbf 100644
> > --- a/Documentation/media/kapi/v4l2-dev.rst
> > +++ b/Documentation/media/kapi/v4l2-dev.rst
> > @@ -288,6 +288,7 @@ Mask  Description
> >  0x08  Log the read and write file operations and the VIDIOC_QBUF and
> >VIDIOC_DQBUF ioctls.
> >  0x10  Log the poll file operation.
> > +0x20  Log error and messages in the control operations.
> >  = 
> >  
> >  Video device cleanup
> > diff --git a/drivers/media/platform/omap3isp/ispvideo.c 
> > b/drivers/media/platform/omap3isp/ispvideo.c
> > index 175bbed9a235..abc945cc05c9 100644
> > --- a/drivers/media/platform/omap3isp/ispvideo.c
> > +++ b/drivers/media/platform/omap3isp/ispvideo.c
> > @@ -1028,7 +1028,7 @@ static int isp_video_check_external_subdevs(struct 
> > isp_video *video,
> > ctrls.count = 1;
> > ctrls.controls = &ctrl;
> >  
> > -   ret = v4l2_g_ext_ctrls(pipe->external->ctrl_handler, NULL, &ctrls);
> > +   ret = v4l2_g_ext_ctrls(pipe->external->ctrl_handler, &video->video, 
> > NULL, &ctrls);
> > if (ret < 0) {
> > dev_warn(isp->dev, "no pixel rate control in subdev %s\n",
> >  pipe->external->name);
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> > b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index 2d7525e2d9eb..1c8ae4501870 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -6,6 +6,8 @@
> >  
> >   */
> >  
> > +#define pr_fmt(fmt) "v4l2-ctrls: " fmt
> > +
> >  #include 
> >  #incl

[PATCH v4] media: v4l: ctrls: Add debug messages

[Ezequiel Garcia]

Currently, the v4l2 control code is a bit silent on errors.
Add debug messages on (hopefully) most of the error paths.

Signed-off-by: Ezequiel Garcia 
---
Changes from v3:
* Fix checkpatch.pl warnings about long lines.

Changes from v2:
* Check and noisy-warn if vdev is NULL.

Changes from v1:
* Drop changes in the debug parameter semantics.
* Drop new module debug parameter.
* Add documentation.
* Add a debug error in all places where control can fail.
* Reorder the vdev parameter, to make the patch less invasive.
---
 Documentation/media/kapi/v4l2-dev.rst  |   1 +
 drivers/media/platform/omap3isp/ispvideo.c |   4 +-
 drivers/media/v4l2-core/v4l2-ctrls.c   | 126 -
 drivers/media/v4l2-core/v4l2-ioctl.c   |  18 ++-
 drivers/media/v4l2-core/v4l2-subdev.c  |   6 +-
 include/media/v4l2-ctrls.h |   9 +-
 include/media/v4l2-ioctl.h |   2 +
 7 files changed, 127 insertions(+), 39 deletions(-)

diff --git a/Documentation/media/kapi/v4l2-dev.rst 
b/Documentation/media/kapi/v4l2-dev.rst
index b359f1804bbe..4c5a15c53dbf 100644
--- a/Documentation/media/kapi/v4l2-dev.rst
+++ b/Documentation/media/kapi/v4l2-dev.rst
@@ -288,6 +288,7 @@ Mask  Description
 0x08  Log the read and write file operations and the VIDIOC_QBUF and
   VIDIOC_DQBUF ioctls.
 0x10  Log the poll file operation.
+0x20  Log error and messages in the control operations.
 = 
 
 Video device cleanup
diff --git a/drivers/media/platform/omap3isp/ispvideo.c 
b/drivers/media/platform/omap3isp/ispvideo.c
index 175bbed9a235..083cce840dc1 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -1027,8 +1027,8 @@ static int isp_video_check_external_subdevs(struct 
isp_video *video,
 
ctrls.count = 1;
ctrls.controls = &ctrl;
-
-   ret = v4l2_g_ext_ctrls(pipe->external->ctrl_handler, NULL, &ctrls);
+   ret = v4l2_g_ext_ctrls(pipe->external->ctrl_handler, &video->video,
+  NULL, &ctrls);
if (ret < 0) {
dev_warn(isp->dev, "no pixel rate control in subdev %s\n",
 pipe->external->name);
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 2d7525e2d9eb..0f5d5fb9fb21 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -6,6 +6,8 @@
 
  */
 
+#define pr_fmt(fmt) "v4l2-ctrls: " fmt
+
 #include 
 #include 
 #include 
@@ -16,6 +18,12 @@
 #include 
 #include 
 
+#define dprintk(vdev, fmt, arg...) do {
\
+   if (!WARN_ON(!vdev) && ((vdev)->dev_debug & V4L2_DEV_DEBUG_CTRL)) \
+   printk(KERN_DEBUG pr_fmt("%s: %s: " fmt),   \
+  __func__, video_device_node_name(vdev), ##arg);  \
+} while (0)
+
 #define has_op(master, op) \
(master->ops && master->ops->op)
 #define call_op(master, op) \
@@ -3211,6 +3219,7 @@ static int v4l2_ctrl_request_bind(struct media_request 
*req,
 static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
 struct v4l2_ext_controls *cs,
 struct v4l2_ctrl_helper *helpers,
+struct video_device *vdev,
 bool get)
 {
struct v4l2_ctrl_helper *h;
@@ -3228,20 +3237,31 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler 
*hdl,
if (cs->which &&
cs->which != V4L2_CTRL_WHICH_DEF_VAL &&
cs->which != V4L2_CTRL_WHICH_REQUEST_VAL &&
-   V4L2_CTRL_ID2WHICH(id) != cs->which)
+   V4L2_CTRL_ID2WHICH(id) != cs->which) {
+   dprintk(vdev,
+   "invalid which 0x%x or control id 0x%x\n",
+   cs->which, id);
return -EINVAL;
+   }
 
/* Old-style private controls are not allowed for
   extended controls */
-   if (id >= V4L2_CID_PRIVATE_BASE)
+   if (id >= V4L2_CID_PRIVATE_BASE) {
+   dprintk(vdev,
+   "old-style private controls not allowed\n");
return -EINVAL;
+   }
ref = find_ref_lock(hdl, id);
-   if (ref == NULL)
+   if (ref == NULL) {
+   dprintk(vdev, "cannot find control id 0x%x\n", id);
return -EINVAL;
+   }
h->ref = ref;
ctrl = ref->ctrl;
-   if (ctrl->flags & V4L2_CTRL_FLAG_DISABLED)
+   if (ctrl->flags & V4L2_CTRL_FLAG_DISABLED) {
+   dprintk(vdev, "control id 0x%x is disabled\n", id);
return -EINVAL;
+   }
 
if (ctrl->cluster[0]->