Re: [PATCH v2 06/10] usb/uvc: Support for V4L2_CTRL_WHICH_DEF_VAL

[Hans Verkuil]

Laurent, can you review this?

Regards,

Hans

On 08/21/2015 03:19 PM, Ricardo Ribalda Delgado wrote:
> This driver does not use the control infrastructure.
> Add support for the new field which on structure
>  v4l2_ext_controls
> 
> Signed-off-by: Ricardo Ribalda Delgado 
> ---
>  drivers/media/usb/uvc/uvc_v4l2.c | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c 
> b/drivers/media/usb/uvc/uvc_v4l2.c
> index 2764f43607c1..e6d3a1bcfa2f 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -980,6 +980,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void 
> *fh,
>   struct uvc_fh *handle = fh;
>   struct uvc_video_chain *chain = handle->chain;
>   struct v4l2_ext_control *ctrl = ctrls->controls;
> + struct v4l2_queryctrl qc;
>   unsigned int i;
>   int ret;
>  
> @@ -988,7 +989,14 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void 
> *fh,
>   return ret;
>  
>   for (i = 0; i < ctrls->count; ++ctrl, ++i) {
> - ret = uvc_ctrl_get(chain, ctrl);
> + if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
> + qc.id = ctrl->id;
> + ret = uvc_query_v4l2_ctrl(chain, );
> + if (!ret)
> + ctrl->value = qc.default_value;
> + } else
> + ret = uvc_ctrl_get(chain, ctrl);
> +
>   if (ret < 0) {
>   uvc_ctrl_rollback(handle);
>   ctrls->error_idx = i;
> @@ -1010,6 +1018,10 @@ static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh 
> *handle,
>   unsigned int i;
>   int ret;
>  
> + /* Default value cannot be changed */
> + if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL)
> + return -EINVAL;
> +
>   ret = uvc_ctrl_begin(chain);
>   if (ret < 0)
>   return ret;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/13] DocBook: add SDR specific info to G_MODULATOR / S_MODULATOR

[Hans Verkuil]

On 09/01/2015 11:59 PM, Antti Palosaari wrote:
> Add SDR specific notes to G_MODULATOR / S_MODULATOR documentation.
> 
> Signed-off-by: Antti Palosaari 
> ---
>  Documentation/DocBook/media/v4l/vidioc-g-modulator.xml | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml 
> b/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
> index 80167fc..affb694 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
> @@ -78,6 +78,15 @@ different audio modulation if the request cannot be 
> satisfied. However
>  this is a write-only ioctl, it does not return the actual audio
>  modulation selected.
>  
> +SDR specific modulator types are
> +V4L2_TUNER_SDR and V4L2_TUNER_RF.
> +Valid fields for these modulator types are index,
> +name, capability,
> +rangelow, rangehigh
> +and type. All the rest fields must be

s/rest/remaining/

> +initialized to zero by both application and driver.

I would drop this sentence. The spec is clear about which fields have to be set
by the user. The only thing I would mention here is that txsubchans should be
initialized to 0 by applications (we might want to use it in the future) when
calling S_MODULATOR.

For S_TUNER it is the same: only mention that audmode should be initialized to
0 for these SDR tuner types.

> +Term modulator means SDR transmitter on this context.

s/Term/The term/
s/on/in/

Note: the same typos are in patch 12/13.

Perhaps this sentence should be rewritten since it is not clear what you
mean. I guess the idea is that 'modulator' is not a good match to what actually
happens in the SDR hardware?

How about:

"Note that the term 'modulator' is a misnomer for type V4L2_TUNER_SDR since
this really is a DAC and the 'modulator' frequency is in reality the sampling
frequency of the DAC."

I hope I got that right.

And do something similar for patch 12/13.

> +
>  To change the radio frequency the  ioctl
>  is available.
>  
> 

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v8.1 19/55] [media] media: convert links from array to list

[Mauro Carvalho Chehab]

The entire logic that represent graph links were developed on a
time where there were no needs to dynamic remove links. So,
although links are created/removed one by one via some
functions, they're stored as an array inside the entity struct.

As the array may grow, there's a logic inside the code that
checks if the amount of space is not enough to store
the needed links. If it isn't the core uses krealloc()
to change the size of the link, with is bad, as it
leaves the memory fragmented.

So, convert links into a list.

Also, currently,  both source and sink entities need the link
at the graph traversal logic inside media_entity. So there's
a logic duplicating all links. That makes it to spend
twice the memory needed. This is not a big deal for today's
usage, where the number of links are not big.

Yet, if during the MC workshop discussions, it was said that
IIO graphs could have up to 4,000 entities. So, we may
want to remove the duplication on some future. The problem
is that it would require a separate linked list to store
the backlinks inside the entity, or to use a more complex
algorithm to do graph backlink traversal, with is something
that the current graph traversal inside the core can't cope
with. So, let's postpone a such change if/when it is actually
needed.

Signed-off-by: Mauro Carvalho Chehab 

---

This version contains a few bug fixes folded on it, resulted
from Javier's tests. I won't be respinning the entire series
just due to that.

Javier: Thanks for that! I'll add you proper credits on
the final version.

diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index c38ef1a72b4a..2d06bcff0946 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -622,7 +622,7 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
struct media_device *mdev = adapter->mdev;
struct media_entity  *entity, *source;
struct media_link *link, *found_link = NULL;
-   int i, ret, n_links = 0, active_links = 0;
+   int ret, n_links = 0, active_links = 0;
 
fepriv->pipe_start_entity = NULL;
 
@@ -632,8 +632,7 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
entity = fepriv->dvbdev->entity;
fepriv->pipe_start_entity = entity;
 
-   for (i = 0; i < entity->num_links; i++) {
-   link = >links[i];
+   list_for_each_entry(link, >links, list) {
if (link->sink->entity == entity) {
found_link = link;
n_links++;
@@ -659,13 +658,11 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
 
source = found_link->source->entity;
fepriv->pipe_start_entity = source;
-   for (i = 0; i < source->num_links; i++) {
+   list_for_each_entry(link, >links, list) {
struct media_entity *sink;
int flags = 0;
 
-   link = >links[i];
sink = link->sink->entity;
-
if (sink == entity)
flags = MEDIA_LNK_FL_ENABLED;
 
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 0d85c6c28004..3e649cacfc07 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -150,22 +151,21 @@ static long __media_device_enum_links(struct media_device 
*mdev,
}
 
if (links->links) {
-   struct media_link_desc __user *ulink;
-   unsigned int l;
+   struct media_link *ent_link;
+   struct media_link_desc __user *ulink = links->links;
 
-   for (l = 0, ulink = links->links; l < entity->num_links; l++) {
+   list_for_each_entry(ent_link, >links, list) {
struct media_link_desc link;
 
/* Ignore backlinks. */
-   if (entity->links[l].source->entity != entity)
+   if (ent_link->source->entity != entity)
continue;
-
memset(, 0, sizeof(link));
-   media_device_kpad_to_upad(entity->links[l].source,
+   media_device_kpad_to_upad(ent_link->source,
  );
-   media_device_kpad_to_upad(entity->links[l].sink,
+   media_device_kpad_to_upad(ent_link->sink,
  );
-   link.flags = entity->links[l].flags;
+   link.flags = ent_link->flags;
if (copy_to_user(ulink, , sizeof(*ulink)))
return -EFAULT;
ulink++;
@@ -437,6 +437,7 @@ int __must_check media_device_register_entity(struct 
media_device *mdev,
/* Warn if we apparently re-register an entity */
 

Re: [PATCH 1/3] adv7180: implement g_std() method

[Hans Verkuil]

On 09/04/2015 02:08 PM, Hans Verkuil wrote:
> On 09/04/2015 01:14 AM, Sergei Shtylyov wrote:
>> Commit cccb83f7a184 ([media] adv7180: add more subdev video ops) forgot to 
>> add
>> the g_std() video method. Its implementation seems identical to the 
>> querystd()
>> method,  so we  can just  point at adv7180_querystd()...
>>
>> Signed-off-by: Sergei Shtylyov 
>>
>> ---
>>  drivers/media/i2c/adv7180.c |1 +
>>  1 file changed, 1 insertion(+)
>>
>> Index: media_tree/drivers/media/i2c/adv7180.c
>> ===
>> --- media_tree.orig/drivers/media/i2c/adv7180.c
>> +++ media_tree/drivers/media/i2c/adv7180.c
>> @@ -717,6 +717,7 @@ static int adv7180_g_mbus_config(struct
>>  }
>>  
>>  static const struct v4l2_subdev_video_ops adv7180_video_ops = {
>> +.g_std = adv7180_querystd,
> 
> No, this isn't right. Not your fault, the adv7180 driver is badly coded.
> 
> The adv7180 driver implements this 'autodetect' mode which is enabled
> when s_std is called with V4L2_STD_ALL. This is illegal according to the
> spec. Digging deeper shows that only the sta2x11_vip.c driver uses this
> 'feature':
> 
> static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id std)
> {
> struct sta2x11_vip *vip = video_drvdata(file);
> v4l2_std_id oldstd = vip->std, newstd;
> int status;
> 
> if (V4L2_STD_ALL == std) {
> v4l2_subdev_call(vip->decoder, video, s_std, std);
> ssleep(2);
> v4l2_subdev_call(vip->decoder, video, querystd, );
> v4l2_subdev_call(vip->decoder, video, g_input_status, 
> );
> if (status & V4L2_IN_ST_NO_SIGNAL)
> return -EIO;
> std = vip->std = newstd;
> if (oldstd != std) {
> if (V4L2_STD_525_60 & std)
> vip->format = formats_60[0];
> else
> vip->format = formats_50[0];
> }
> return 0;
> }
> 
> if (oldstd != std) {
> if (V4L2_STD_525_60 & std)
> vip->format = formats_60[0];
> else
> vip->format = formats_50[0];
> }
> 
> return v4l2_subdev_call(vip->decoder, video, s_std, std);
> }
> 
> So it enables the autodetect, queries the standard and uses the resulting
> standard.
> 
> It leaves the autodetect feature on as well which can be very dangerous
> if it switches from NTSC to PAL since the buffer size increases in that
> case, potentially leading to buffer overruns.
> 
> What you should do is to completely remove the autodetect feature from the
> adv7180 driver, 

A quick follow-up to this: in adv7180_irq it does:

if (isr3 & ADV7180_IRQ3_AD_CHANGE && state->autodetect)
__adv7180_status(state, NULL, >curr_norm);

If we remove the autodetect 'feature', then this would be removed as well,
however, what would be better is to replace the call to __adv7180_status()
with generating a V4L2_EVENT_SOURCE_CHANGE event (v4l2_subdev_notify_event).

That way applications can be informed of source change events (assuming the
bridge driver will pass it through, but that's not the concern of the
adv7180 driver).

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] rcar_vin: call g_std() instead of querystd()

[Hans Verkuil]

On 09/04/2015 01:18 AM, Sergei Shtylyov wrote:
> Hans Verkuil says: "The only place querystd can be called  is in the QUERYSTD
> ioctl, all other ioctls should use the last set standard." So call the g_std()
> subdevice method instead of querystd() in the driver's set_fmt() method.
> 
> Reported-by: Hans Verkuil 
> Signed-off-by: Sergei Shtylyov 

Acked-by: Hans Verkuil 

> 
> ---
>  drivers/media/platform/soc_camera/rcar_vin.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> ===
> --- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
> +++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -1589,8 +1589,8 @@ static int rcar_vin_set_fmt(struct soc_c
>   field = pix->field;
>   break;
>   case V4L2_FIELD_INTERLACED:
> - /* Query for standard if not explicitly mentioned _TB/_BT */
> - ret = v4l2_subdev_call(sd, video, querystd, );
> + /* Get the last standard if not explicitly mentioned _TB/_BT */
> + ret = v4l2_subdev_call(sd, video, g_std, );
>   if (ret == -ENOIOCTLCMD) {
>   field = V4L2_FIELD_NONE;
>   } else if (ret < 0) {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] ml86v7667: implement g_std() method

[Hans Verkuil]

On 09/04/2015 01:16 AM, Sergei Shtylyov wrote:
> The driver was written with the 'soc_camera' use in mind, however the g_std()
> video method was forgotten. Implement it at last...
> 
> Signed-off-by: Sergei Shtylyov 

Acked-by: Hans Verkuil 

> 
> ---
>  drivers/media/i2c/ml86v7667.c |   10 ++
>  1 file changed, 10 insertions(+)
> 
> Index: media_tree/drivers/media/i2c/ml86v7667.c
> ===
> --- media_tree.orig/drivers/media/i2c/ml86v7667.c
> +++ media_tree/drivers/media/i2c/ml86v7667.c
> @@ -233,6 +233,15 @@ static int ml86v7667_g_mbus_config(struc
>   return 0;
>  }
>  
> +static int ml86v7667_g_std(struct v4l2_subdev *sd, v4l2_std_id *std)
> +{
> + struct ml86v7667_priv *priv = to_ml86v7667(sd);
> +
> + *std = priv->std;
> +
> + return 0;
> +}
> +
>  static int ml86v7667_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
>  {
>   struct ml86v7667_priv *priv = to_ml86v7667(sd);
> @@ -282,6 +291,7 @@ static const struct v4l2_ctrl_ops ml86v7
>  };
>  
>  static struct v4l2_subdev_video_ops ml86v7667_subdev_video_ops = {
> + .g_std = ml86v7667_g_std,
>   .s_std = ml86v7667_s_std,
>   .querystd = ml86v7667_querystd,
>   .g_input_status = ml86v7667_g_input_status,
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/13] hackrf: add support for transmitter

[Hans Verkuil]

Hi Antti,

Two comments, see below:

On 09/01/2015 11:59 PM, Antti Palosaari wrote:
> HackRF SDR device has both receiver and transmitter. There is limitation
> that receiver and transmitter cannot be used at the same time
> (half-duplex operation). That patch implements transmitter support to
> existing receiver only driver.
> 
> Signed-off-by: Antti Palosaari 
> ---
>  drivers/media/usb/hackrf/hackrf.c | 923 
> ++
>  1 file changed, 648 insertions(+), 275 deletions(-)
> 
> diff --git a/drivers/media/usb/hackrf/hackrf.c 
> b/drivers/media/usb/hackrf/hackrf.c
> -static unsigned int hackrf_convert_stream(struct hackrf_dev *dev,
> - void *dst, void *src, unsigned int src_len)
> +void hackrf_copy_stream(struct hackrf_dev *dev, void *dst,

Is there any reason 'static' was removed here? It's not used externally as
far as I can tell.

> + void *src, unsigned int src_len)
>  {
>   memcpy(dst, src, src_len);
>  



> +static int hackrf_s_modulator(struct file *file, void *fh,
> +const struct v4l2_modulator *a)
> +{
> + struct hackrf_dev *dev = video_drvdata(file);
> + int ret;
> +
> + dev_dbg(dev->dev, "index=%d\n", a->index);
> +
> + if (a->index == 0)
> + ret = 0;
> + else if (a->index == 1)
> + ret = 0;
> + else
> + ret = -EINVAL;
> +
> + return ret;
> +}

Why implement this at all? It's not doing anything. I'd just drop s_modulator
support.

If there is a reason why you do need it, then simplify it to:

return a->index > 1 ? -EINVAL : 0;

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/10] Support getting default values from any control

[Hans Verkuil]

This patch series looks good. I'll drop the saa7164 patches since I made a
patch series converting it to the control framework. Once Steve gives me the
Ack for that I can merge that series.

All I need to merge your series (minus the saa7164 patches) is the Ack from
Laurent for the uvc patch.

Regards,

Hans

On 08/21/2015 03:19 PM, Ricardo Ribalda Delgado wrote:
> Integer controls provide a way to get their default/initial value, but
> any other control (p_u32, p_u8.) provide no other way to get the
> initial value than unloading the module and loading it back.
> 
> *What is the actual problem?
> I have a custom control with WIDTH integer values. Every value
> represents the calibrated FPN (fixed pattern noise) correction value for that
> column
> -Application A changes the FPN correction value
> -Application B wants to restore the calibrated value but it cant :(
> 
> *What is the proposed solution?
> 
> (Kudos to Hans Verkuil!!!)
> 
> The key change is in struct v4l2_ext_controls where the __u32 ctrl_class field
> is changed to:
> 
> union {
> __u32 ctrl_class;
> __u32 which;
> };
> 
> And two new defines are added:
> 
> #define V4L2_CTRL_WHICH_CUR_VAL0
> #define V4L2_CTRL_WHICH_DEF_VAL0x0f00
> 
> The 'which' field tells you which controls are get/set/tried.
> 
> V4L2_CTRL_WHICH_CUR_VAL: the current value of the controls
> V4L2_CTRL_WHICH_DEF_VAL: the default value of the controls
> V4L2_CTRL_CLASS_*: the current value of the controls belonging to the 
> specified class.
> Note: this is deprecated usage and is only there for backwards 
> compatibility.
> Which is also why I don't think there is a need to add 
> V4L2_CTRL_WHICH_
> aliases for these defines.
> 
> 
> I have posted a copy of my working tree to
> 
> https://github.com/ribalda/linux/tree/which_def_v2
> 
> Changelog v2:
> 
> Suggested by Hans Verkuil 
> 
> Replace ctrls_class with which on all the codebase
> Changes in Documentation
> (Thanks!)
> 
> Ricardo Ribalda Delgado (10):
>   videodev2.h: Fix typo in comment
>   videodev2.h: Extend struct v4l2_ext_controls
>   media/v4l2-compat-ioctl32: Simple stylechecks
>   media/core: Replace ctrl_class with which
>   media/v4l2-core: struct struct v4l2_ext_controls param which
>   usb/uvc: Support for V4L2_CTRL_WHICH_DEF_VAL
>   media/usb/pvrusb2: Support for V4L2_CTRL_WHICH_DEF_VAL
>   media/pci/saa7164-encoder Support for V4L2_CTRL_WHICH_DEF_VAL
>   media/pci/saa7164-vbi Support for V4L2_CTRL_WHICH_DEF_VAL
>   Docbook: media: Document changes on struct v4l2_ext_controls
> 
>  Documentation/DocBook/media/v4l/v4l2.xml   |  9 
>  .../DocBook/media/v4l/vidioc-g-ext-ctrls.xml   | 28 --
>  drivers/media/pci/saa7164/saa7164-encoder.c| 59 -
>  drivers/media/pci/saa7164/saa7164-vbi.c| 61 
> +-
>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c   |  2 +-
>  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c   |  2 +-
>  drivers/media/usb/pvrusb2/pvrusb2-v4l2.c   | 17 +-
>  drivers/media/usb/uvc/uvc_v4l2.c   | 14 -
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c  | 17 +++---
>  drivers/media/v4l2-core/v4l2-ctrls.c   | 54 +--
>  drivers/media/v4l2-core/v4l2-ioctl.c   | 14 ++---
>  include/uapi/linux/videodev2.h | 14 -
>  12 files changed, 200 insertions(+), 91 deletions(-)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v8.2 19/55] [media] media: convert links from array to list

[Mauro Carvalho Chehab]

The entire logic that represent graph links were developed on a
time where there were no needs to dynamic remove links. So,
although links are created/removed one by one via some
functions, they're stored as an array inside the entity struct.

As the array may grow, there's a logic inside the code that
checks if the amount of space is not enough to store
the needed links. If it isn't the core uses krealloc()
to change the size of the link, with is bad, as it
leaves the memory fragmented.

So, convert links into a list.

Also, currently,  both source and sink entities need the link
at the graph traversal logic inside media_entity. So there's
a logic duplicating all links. That makes it to spend
twice the memory needed. This is not a big deal for today's
usage, where the number of links are not big.

Yet, if during the MC workshop discussions, it was said that
IIO graphs could have up to 4,000 entities. So, we may
want to remove the duplication on some future. The problem
is that it would require a separate linked list to store
the backlinks inside the entity, or to use a more complex
algorithm to do graph backlink traversal, with is something
that the current graph traversal inside the core can't cope
with. So, let's postpone a such change if/when it is actually
needed.

Signed-off-by: Mauro Carvalho Chehab 

---

Sorry, I did some mess with my trees, and just re-sent the same
patch as before. That's the right one with Javier fixes.
Please ignore PATCH v8.1.

This version contains a few bug fixes folded on it, resulted
from Javier's tests. I won't be respinning the entire series
just due to that.

Javier: Thanks for that! I'll add you proper credits on
the final version.

diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index c38ef1a72b4a..2d06bcff0946 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -622,7 +622,7 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
struct media_device *mdev = adapter->mdev;
struct media_entity  *entity, *source;
struct media_link *link, *found_link = NULL;
-   int i, ret, n_links = 0, active_links = 0;
+   int ret, n_links = 0, active_links = 0;
 
fepriv->pipe_start_entity = NULL;
 
@@ -632,8 +632,7 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
entity = fepriv->dvbdev->entity;
fepriv->pipe_start_entity = entity;
 
-   for (i = 0; i < entity->num_links; i++) {
-   link = >links[i];
+   list_for_each_entry(link, >links, list) {
if (link->sink->entity == entity) {
found_link = link;
n_links++;
@@ -659,13 +658,11 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
 
source = found_link->source->entity;
fepriv->pipe_start_entity = source;
-   for (i = 0; i < source->num_links; i++) {
+   list_for_each_entry(link, >links, list) {
struct media_entity *sink;
int flags = 0;
 
-   link = >links[i];
sink = link->sink->entity;
-
if (sink == entity)
flags = MEDIA_LNK_FL_ENABLED;
 
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 0d85c6c28004..3e649cacfc07 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -150,22 +151,21 @@ static long __media_device_enum_links(struct media_device 
*mdev,
}
 
if (links->links) {
-   struct media_link_desc __user *ulink;
-   unsigned int l;
+   struct media_link *ent_link;
+   struct media_link_desc __user *ulink = links->links;
 
-   for (l = 0, ulink = links->links; l < entity->num_links; l++) {
+   list_for_each_entry(ent_link, >links, list) {
struct media_link_desc link;
 
/* Ignore backlinks. */
-   if (entity->links[l].source->entity != entity)
+   if (ent_link->source->entity != entity)
continue;
-
memset(, 0, sizeof(link));
-   media_device_kpad_to_upad(entity->links[l].source,
+   media_device_kpad_to_upad(ent_link->source,
  );
-   media_device_kpad_to_upad(entity->links[l].sink,
+   media_device_kpad_to_upad(ent_link->sink,
  );
-   link.flags = entity->links[l].flags;
+   link.flags = ent_link->flags;
if (copy_to_user(ulink, , sizeof(*ulink)))
return -EFAULT;
ulink++;
@@ 

Re: [PATCH 1/3] adv7180: implement g_std() method

[Hans Verkuil]

On 09/04/2015 01:14 AM, Sergei Shtylyov wrote:
> Commit cccb83f7a184 ([media] adv7180: add more subdev video ops) forgot to add
> the g_std() video method. Its implementation seems identical to the querystd()
> method,  so we  can just  point at adv7180_querystd()...
> 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
>  drivers/media/i2c/adv7180.c |1 +
>  1 file changed, 1 insertion(+)
> 
> Index: media_tree/drivers/media/i2c/adv7180.c
> ===
> --- media_tree.orig/drivers/media/i2c/adv7180.c
> +++ media_tree/drivers/media/i2c/adv7180.c
> @@ -717,6 +717,7 @@ static int adv7180_g_mbus_config(struct
>  }
>  
>  static const struct v4l2_subdev_video_ops adv7180_video_ops = {
> + .g_std = adv7180_querystd,

No, this isn't right. Not your fault, the adv7180 driver is badly coded.

The adv7180 driver implements this 'autodetect' mode which is enabled
when s_std is called with V4L2_STD_ALL. This is illegal according to the
spec. Digging deeper shows that only the sta2x11_vip.c driver uses this
'feature':

static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id std)
{
struct sta2x11_vip *vip = video_drvdata(file);
v4l2_std_id oldstd = vip->std, newstd;
int status;

if (V4L2_STD_ALL == std) {
v4l2_subdev_call(vip->decoder, video, s_std, std);
ssleep(2);
v4l2_subdev_call(vip->decoder, video, querystd, );
v4l2_subdev_call(vip->decoder, video, g_input_status, );
if (status & V4L2_IN_ST_NO_SIGNAL)
return -EIO;
std = vip->std = newstd;
if (oldstd != std) {
if (V4L2_STD_525_60 & std)
vip->format = formats_60[0];
else
vip->format = formats_50[0];
}
return 0;
}

if (oldstd != std) {
if (V4L2_STD_525_60 & std)
vip->format = formats_60[0];
else
vip->format = formats_50[0];
}

return v4l2_subdev_call(vip->decoder, video, s_std, std);
}

So it enables the autodetect, queries the standard and uses the resulting
standard.

It leaves the autodetect feature on as well which can be very dangerous
if it switches from NTSC to PAL since the buffer size increases in that
case, potentially leading to buffer overruns.

What you should do is to completely remove the autodetect feature from the
adv7180 driver, then change the code in sta2x11_vip.c to:

static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id std)
{
struct sta2x11_vip *vip = video_drvdata(file);
int status;
int err;

if (V4L2_STD_ALL == std) {
/*
 * Note: this behavior is out-of-spec! It's kept to preserve
 * backwards compatibility.
 */
v4l2_subdev_call(vip->decoder, video, querystd, );
v4l2_subdev_call(vip->decoder, video, g_input_status, );
if (status & V4L2_IN_ST_NO_SIGNAL)
return -EIO;
}

if (vip->std == std)
return 0;

err = v4l2_subdev_call(vip->decoder, video, s_std, std);
if (err)
return err;

vip->std = std;
if (V4L2_STD_525_60 & std)
vip->format = formats_60[0];
else
vip->format = formats_50[0];
return 0;
}

For this code:

Signed-off-by: Hans Verkuil 

Note: I haven't compiled this code, and I can't test it either (no hardware).

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 19/55] [media] media: convert links from array to list

[Hans Verkuil]

On 09/04/2015 10:41 AM, Sakari Ailus wrote:
> Hi Mauro,
> 
> On Sun, Aug 30, 2015 at 12:06:30AM -0300, Mauro Carvalho Chehab wrote:
>> The entire logic that represent graph links were developed on a
>> time where there were no needs to dynamic remove links. So,
>> although links are created/removed one by one via some
>> functions, they're stored as an array inside the entity struct.
>>
>> As the array may grow, there's a logic inside the code that
>> checks if the amount of space is not enough to store
>> the needed links. If it isn't the core uses krealloc()
>> to change the size of the link, with is bad, as it
>> leaves the memory fragmented.
> 
> The Linux kernel constantly allocates and releases memory for various
> reasons. Even the V4L2 IOCTL handler does that when the argument is too
> large to fit into the stack-allocated buffer. There's no reason to avoid
> allocating and releasing small chunks of memory when hardware configuration
> changes.
> 
>>
>> So, convert links into a list.
> 
> Instead, if you're worried about the time the memory re-allocation takes
> that'd be needed to dynamically add a large number of links, I'd just
> allocate more memory at a time, say, rounding up to a power of two, and use
> vmalloc() instead of kmalloc() when the size grows over one page.
> 
> That'd avoid the vast majority of reallocs without wasting much memory: the
> granularity of memory allocation is much larger than the size of struct
> media_link except in the case of very small link numbers.
> 
> The reason I'm proposing this is that linked lists, as you're allocating
> them, can end up anywhere in the system memory. Walking over the media graph
> is quite a job if you have as many links as in your example, and the
> additional cache misses caused by scattering the data structure all over the
> system memory will not make the effort smaller.
> 
> Adding links dynamically (for a change in hardware topology?) is by far less
> performance critical than e.g. starting and stopping streaming or
> enumerating the links so the latter should have the priority IMHO.

You don't do either of these jobs very often either. As I have said before, the
enemy of media drivers is rarely performance but always complexity. Based
on the patches I've seen I agree with Mauro that a linked list is a better
fit and simplifies the code.

If you have *proof* that this hurts performance in certain real-life cases,
then this can always be optimized later.

But unless someone shows me proof that it really hurts performance in realistic
cases, I will always favor simplicity.

In all the time that I have been involved in the media subsystem the only
performance issue I am aware of was that enumerating all the links in the MC
can be too slow due to all the ioctl calls that it takes (vsp driver). This
will be solved with the proposed G_TOPOLOGY ioctl since that returns everything
in a single call (or two calls if you need to know how much memory to allocate
first).

Hmm, actually, I think there is another related issue if you want to enumerate
all combinations of pixelformats, discrete framesizes and framerates. There too
the problem is the number of ioctl calls you have to do. It's never been
important enough to do something about it, though.

Bottom line, always go for simplicity unless you can demonstrate with numbers
that there are real performance issues.

Regards,

Hans

> 
>>
>> Also, currently,  both source and sink entities need the link
>> at the graph traversal logic inside media_entity. So there's
>> a logic duplicating all links. That makes it to spend
>> twice the memory needed. This is not a big deal for today's
>> usage, where the number of links are not big.
>>
>> Yet, if during the MC workshop discussions, it was said that
>> IIO graphs could have up to 4,000 entities. So, we may
>> want to remove the duplication on some future. The problem
>> is that it would require a separate linked list to store
>> the backlinks inside the entity, or to use a more complex
>> algorithm to do graph backlink traversal, with is something
>> that the current graph traversal inside the core can't cope
>> with. So, let's postpone a such change if/when it is actually
>> needed.
> 
> Would keeping the links in a linked list help with that?
> 
> I think this could be done using both the current arrays or linked lists ---
> instead of storing links themselves, you'd store pointers to the links
> (array or a linked list) which then are stored elsewhere. Helper functions
> would be needed to e.g. loop over the links in that case though.
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/13] DocBook: add modulator type field

[Hans Verkuil]

On 09/01/2015 11:59 PM, Antti Palosaari wrote:
> Add new modulator type field to documentation.
> 
> Signed-off-by: Antti Palosaari 

Acked-by: Hans Verkuil 

> ---
>  Documentation/DocBook/media/v4l/vidioc-g-modulator.xml | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml 
> b/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
> index 7068b59..80167fc 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
> @@ -140,7 +140,13 @@ indicator, for example a stereo pilot tone.
> 
> 
>   __u32
> - reserved[4]
> + type
> + Type of the modulator, see  + linkend="v4l2-tuner-type" />.
> +   
> +   
> + __u32
> + reserved[3]
>   Reserved for future extensions. Drivers and
>  applications must set the array to zero.
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 19/55] [media] media: convert links from array to list

[Mauro Carvalho Chehab]

Em Fri, 04 Sep 2015 11:00:38 +0200
Hans Verkuil  escreveu:

> On 09/04/2015 10:41 AM, Sakari Ailus wrote:
> > Hi Mauro,
> > 
> > On Sun, Aug 30, 2015 at 12:06:30AM -0300, Mauro Carvalho Chehab wrote:
> >> The entire logic that represent graph links were developed on a
> >> time where there were no needs to dynamic remove links. So,
> >> although links are created/removed one by one via some
> >> functions, they're stored as an array inside the entity struct.
> >>
> >> As the array may grow, there's a logic inside the code that
> >> checks if the amount of space is not enough to store
> >> the needed links. If it isn't the core uses krealloc()
> >> to change the size of the link, with is bad, as it
> >> leaves the memory fragmented.
> > 
> > The Linux kernel constantly allocates and releases memory for various
> > reasons. Even the V4L2 IOCTL handler does that when the argument is too
> > large to fit into the stack-allocated buffer. There's no reason to avoid
> > allocating and releasing small chunks of memory when hardware configuration
> > changes.
> > 
> >>
> >> So, convert links into a list.
> > 
> > Instead, if you're worried about the time the memory re-allocation takes
> > that'd be needed to dynamically add a large number of links, I'd just
> > allocate more memory at a time, say, rounding up to a power of two, and use
> > vmalloc() instead of kmalloc() when the size grows over one page.

We had support to allocate more links via that "extra_links" parameter.
This never worked in practice: there was no single client for that.

> > 
> > That'd avoid the vast majority of reallocs without wasting much memory: the
> > granularity of memory allocation is much larger than the size of struct
> > media_link except in the case of very small link numbers.
> > 
> > The reason I'm proposing this is that linked lists, as you're allocating
> > them, can end up anywhere in the system memory. Walking over the media graph
> > is quite a job if you have as many links as in your example, and the
> > additional cache misses caused by scattering the data structure all over the
> > system memory will not make the effort smaller.

First of all, I agree with Hans: the major issues we have is not due
to performance, but due to complexity.

Talking about the graph traversal algorithm, it curently doesn't work
even for simple hybrid TV devices, as it currently allows to have only 64
entities, due to the loop detection code there.

Ok, it would be possible to extend it to support real case scenarios, but
the patch is not as trivial as changing the max number of entities. Just
doing that would cause the stack to crash, as the bitmap structs are 
allocated at the stack.

Also, what you're saying regards to the increase of cache misses,
this is not true. The links memory are still fragmented, as they're
allocated during entities init code. On most codes, that means that
they'll be allocated after each entity.

So, memory would look like:
entity#1
links for entity#1
entity#2
links for entity#2
...

If you want to put the links at consecutive addresses, what you would
need to do is to create first all entities, and then create the links
altogether. After this patch, drivers that have timing issues can do
it, having the memory allocated as:
entity#1
entity#2
links for entity#1
links for entity#2
...

So, I won't doubt that this patch would actually improve performance
by putting the links altogether ;)

Ok, when we have lots of dynamic entity creation/removal, the memory
will become more fragmented, but that would happen with or without
this change.

I mean, if this patch is not applied and entity#2 is created after
a long runtime, we would have:

entity#1
links for entity #1



entity#2
links for entity #2

While, on the other case, the memory would be:

entity#1
links for entity #1 (except for the link with entity#2)
backlinks for entity #1 (except for the link with entity#2)



entity#2
link between entity#1 and entity#2
other links for entity #2

I bet performance would be pretty much the same.

Anyway, let's go to real numbers. Javier ran some tests in order to check
what would be the difference using the omap3isp driver.

He hacked yavta to suppress glibc calls to printf() with this hack:

#define printf(...) do { } while (0)

Then, he measured the times with and without this patch series.
Doing the graph traversal needed for this command:
$ time ./yavta/yavta -f UYVY -s 720x312 -n 1 --capture=1 -F /dev/video2

Resulted in:
real0m0.022s
user0m0.000s
sys 0m0.010s

The complete results form Javier are at:
http://hastebin.com/turixebuyo.pl

He did that with both with and without this patch series.

On all cases, it took 10ms to setup the pipeline.

> > 
> > Adding links dynamically (for a 

[GIT PULL FOR v4.4] Various fixes

[Hans Verkuil]

Various fixes and enhancements, nothing special.

Note that the v4l2-compat-ioctl32 fix has a CC for stable from 3.10 onwards.
The arm64 architecture was added in 3.10.

Regards,

Hans

The following changes since commit 50ef28a6ac216fd8b796257a3768fef8f57b917d:

  [media] c8sectpfe: Remove select on undefined LIBELF_32 (2015-09-03 14:10:06 
-0300)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git for-v4.4a

for you to fetch changes up to f71d4170d11859760f70ce4667b9c1d4da8373eb:

  vivid: sdr cap: few enhancements (2015-09-04 15:24:42 +0200)


Alexander Kuleshov (1):
  media/pci/cobalt: Use %*ph to print small buffers

Andrew Milkovich (1):
  Staging: media: bcm2048: warnings for uninitialized variables fixed

Andrzej Hajda (1):
  v4l2-compat-ioctl32: fix alignment for ARM64

Antti Palosaari (2):
  vivid: SDR cap: add control for FM deviation
  vivid: sdr cap: few enhancements

Darek Zielski (1):
  saa7134: add Leadtek Winfast TV2100 FM card support

Nicolas Sugino (1):
  ivtv-alsa: Add index to specify device number

Zahari Doychev (1):
  m2m: fix bad unlock balance

 Documentation/video4linux/CARDLIST.saa7134|  1 +
 drivers/media/pci/cobalt/cobalt-cpld.c|  8 +++-
 drivers/media/pci/ivtv/ivtv-alsa-main.c   | 14 --
 drivers/media/pci/saa7134/saa7134-cards.c | 43 
+++
 drivers/media/pci/saa7134/saa7134-input.c |  7 +++
 drivers/media/pci/saa7134/saa7134.h   |  1 +
 drivers/media/platform/vivid/vivid-core.h |  1 +
 drivers/media/platform/vivid/vivid-ctrls.c| 37 
-
 drivers/media/platform/vivid/vivid-sdr-cap.c  | 37 
+
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |  9 +
 drivers/media/v4l2-core/v4l2-mem2mem.c| 23 ---
 drivers/staging/media/bcm2048/radio-bcm2048.c | 20 ++--
 12 files changed, 144 insertions(+), 57 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/2] vivid: add support for reduced fps in video capture

[Hans Verkuil]

On 08/23/2015 04:09 PM, Prashant Laddha wrote:
> With this patch, vivid capture thread can now generate reduced
> fps by factor of 1000 / 1001. This is controlled using a boolean
> VIVID_CID_REDUCED_FPS added in vivid control. For reduced fps,
> capture time is controlled by scaling down timeperframe_vid_cap
> with a factor of 1000 / 1001 if VIVID_CID_REDUCED_FPS is true.
> 
> Cc: Hans Verkuil 
> Signed-off-by: Prashant Laddha 
> ---
>  drivers/media/platform/vivid/vivid-core.h|  1 +
>  drivers/media/platform/vivid/vivid-ctrls.c   | 15 +++
>  drivers/media/platform/vivid/vivid-vid-cap.c |  7 ++-
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/vivid/vivid-core.h 
> b/drivers/media/platform/vivid/vivid-core.h
> index c72349c..4a2c8b7 100644
> --- a/drivers/media/platform/vivid/vivid-core.h
> +++ b/drivers/media/platform/vivid/vivid-core.h
> @@ -263,6 +263,7 @@ struct vivid_dev {
>   boolvflip;
>   boolvbi_cap_interlaced;
>   boolloop_video;
> + boolreduced_fps;
>  
>   /* Framebuffer */
>   unsigned long   video_pbase;
> diff --git a/drivers/media/platform/vivid/vivid-ctrls.c 
> b/drivers/media/platform/vivid/vivid-ctrls.c
> index 339c8b7..6becdfe 100644
> --- a/drivers/media/platform/vivid/vivid-ctrls.c
> +++ b/drivers/media/platform/vivid/vivid-ctrls.c
> @@ -78,6 +78,7 @@
>  #define VIVID_CID_TIME_WRAP  (VIVID_CID_VIVID_BASE + 39)
>  #define VIVID_CID_MAX_EDID_BLOCKS(VIVID_CID_VIVID_BASE + 40)
>  #define VIVID_CID_PERCENTAGE_FILL(VIVID_CID_VIVID_BASE + 41)
> +#define VIVID_CID_REDUCED_FPS(VIVID_CID_VIVID_BASE + 42)
>  
>  #define VIVID_CID_STD_SIGNAL_MODE(VIVID_CID_VIVID_BASE + 60)
>  #define VIVID_CID_STANDARD   (VIVID_CID_VIVID_BASE + 61)
> @@ -422,6 +423,10 @@ static int vivid_vid_cap_s_ctrl(struct v4l2_ctrl *ctrl)
>   dev->sensor_vflip = ctrl->val;
>   tpg_s_vflip(>tpg, dev->sensor_vflip ^ dev->vflip);
>   break;
> + case VIVID_CID_REDUCED_FPS:
> + dev->reduced_fps = ctrl->val;
> + vivid_update_format_cap(dev, true);
> + break;
>   case VIVID_CID_HAS_CROP_CAP:
>   dev->has_crop_cap = ctrl->val;
>   vivid_update_format_cap(dev, true);
> @@ -599,6 +604,15 @@ static const struct v4l2_ctrl_config vivid_ctrl_vflip = {
>   .step = 1,
>  };
>  
> +static const struct v4l2_ctrl_config vivid_ctrl_reduced_fps = {
> + .ops = _vid_cap_ctrl_ops,
> + .id = VIVID_CID_REDUCED_FPS,
> + .name = "Reduced fps",

I think "Reduced Framerate" is a better description.

> + .type = V4L2_CTRL_TYPE_BOOLEAN,
> + .max = 1,
> + .step = 1,
> +};
> +
>  static const struct v4l2_ctrl_config vivid_ctrl_has_crop_cap = {
>   .ops = _vid_cap_ctrl_ops,
>   .id = VIVID_CID_HAS_CROP_CAP,
> @@ -1379,6 +1393,7 @@ int vivid_create_controls(struct vivid_dev *dev, bool 
> show_ccs_cap,
>   v4l2_ctrl_new_custom(hdl_vid_cap, _ctrl_vflip, NULL);
>   v4l2_ctrl_new_custom(hdl_vid_cap, _ctrl_insert_sav, NULL);
>   v4l2_ctrl_new_custom(hdl_vid_cap, _ctrl_insert_eav, NULL);
> + v4l2_ctrl_new_custom(hdl_vid_cap, _ctrl_reduced_fps, 
> NULL);
>   if (show_ccs_cap) {
>   dev->ctrl_has_crop_cap = 
> v4l2_ctrl_new_custom(hdl_vid_cap,
>   _ctrl_has_crop_cap, NULL);
> diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c 
> b/drivers/media/platform/vivid/vivid-vid-cap.c
> index ed0b878..2b26059 100644
> --- a/drivers/media/platform/vivid/vivid-vid-cap.c
> +++ b/drivers/media/platform/vivid/vivid-vid-cap.c
> @@ -401,6 +401,7 @@ void vivid_update_format_cap(struct vivid_dev *dev, bool 
> keep_controls)
>  {
>   struct v4l2_bt_timings *bt = >dv_timings_cap.bt;
>   unsigned size;
> + u64 pixelclock;
>  
>   switch (dev->input_type[dev->input]) {
>   case WEBCAM:
> @@ -430,8 +431,12 @@ void vivid_update_format_cap(struct vivid_dev *dev, bool 
> keep_controls)
>   dev->src_rect.width = bt->width;
>   dev->src_rect.height = bt->height;
>   size = V4L2_DV_BT_FRAME_WIDTH(bt) * V4L2_DV_BT_FRAME_HEIGHT(bt);
> + if (dev->reduced_fps)

This needs a check similar to reduce_fps() in patch 1/2. Except for the fact
that the bt->flags & V4L2_DV_FL_REDUCED_FPS check is invalid for receivers.
Perhaps that check should be moved out of reduce_fps() so that that function
can be shared between capture and output.

The reduced_fps flag should only be honored for timings that allow it.

> + pixelclock = div_u64((bt->pixelclock * 1000), 1001);

No parenthesis needed around bt->pixelclock * 1000.

> + else
> + pixelclock = 

Re: [RFC PATCH 1/2] vivid: add support for reduced fps in video out.

[Hans Verkuil]

Hi Prashant,

Sorry for the late review, but here it is (finally):

On 08/23/2015 04:09 PM, Prashant Laddha wrote:
> If bt timing has REDUCED_FPS flag set, then reduce the frame rate
> by factor of 1000 / 1001. For vivid, timeperframe_vid_out indicates
> the frame timings used by video out thread. The timeperframe_vid_out
> is derived from pixel clock. Adjusting the timeperframe_vid_out by
> scaling down pixel clock with factor of 1000 / 1001.
> 
> The reduced fps is supported for CVT timings if reduced blanking v2
> (indicated by vsync = 8) is true. For CEA861 timings, reduced fps is
> supported if V4L2_DV_FL_CAN_REDUCE_FPS flag is true. For GTF timings,
> reduced fps is not supported.
> 
> The reduced fps will allow refresh rates like 29.97, 59.94 etc.
> 
> Cc: Hans Verkuil 
> Signed-off-by: Prashant Laddha 
> ---
>  drivers/media/platform/vivid/vivid-vid-out.c | 30 
> +++-
>  drivers/media/v4l2-core/v4l2-dv-timings.c|  5 +
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/vivid/vivid-vid-out.c 
> b/drivers/media/platform/vivid/vivid-vid-out.c
> index c404e27..ca6ec78 100644
> --- a/drivers/media/platform/vivid/vivid-vid-out.c
> +++ b/drivers/media/platform/vivid/vivid-vid-out.c
> @@ -213,6 +213,27 @@ const struct vb2_ops vivid_vid_out_qops = {
>  };
>  
>  /*
> + * Called to check conditions for reduced fps. For CVT timings, reduced
> + * fps is allowed only with reduced blanking v2 (vsync == 8). For CEA861
> + * timings, reduced fps is allowed if V4L2_DV_FL_CAN_REDUCE_FPS flag is
> + * true.
> + */
> +static bool reduce_fps(struct v4l2_bt_timings *bt)
> +{
> + if (!(bt->flags & V4L2_DV_FL_REDUCED_FPS))
> + return false;
> +
> + if ((bt->standards & V4L2_DV_BT_STD_CVT) && (bt->vsync == 8))

I propose that you add a static inline helper function to v4l2-dv-timings.h
to test for reduced blanking v2. This condition is way too magical...

If you add such a helper, then you should check if there is existing code
that could switch to this helper.

> + return true;

Bad indentation (one tab too far to the right).

> +
> + if ((bt->standards & V4L2_DV_BT_STD_CEA861) &&
> + (bt->flags & V4L2_DV_FL_CAN_REDUCE_FPS))
> + return true;

Ditto.

> +
> + return false;
> +}
> +
> +/*
>   * Called whenever the format has to be reset which can occur when
>   * changing outputs, standard, timings, etc.
>   */
> @@ -220,6 +241,7 @@ void vivid_update_format_out(struct vivid_dev *dev)
>  {
>   struct v4l2_bt_timings *bt = >dv_timings_out.bt;
>   unsigned size, p;
> + u64 pixelclock;
>  
>   switch (dev->output_type[dev->output]) {
>   case SVID:
> @@ -241,8 +263,14 @@ void vivid_update_format_out(struct vivid_dev *dev)
>   dev->sink_rect.width = bt->width;
>   dev->sink_rect.height = bt->height;
>   size = V4L2_DV_BT_FRAME_WIDTH(bt) * V4L2_DV_BT_FRAME_HEIGHT(bt);
> +
> + if (reduce_fps(bt))
> + pixelclock = div_u64((bt->pixelclock * 1000), 1001);

No need for the parenthesis around bt->pixelclock * 1000.

> + else
> + pixelclock = bt->pixelclock;
> +
>   dev->timeperframe_vid_out = (struct v4l2_fract) {
> - size / 100, (u32)bt->pixelclock / 100
> + size / 100, (u32)pixelclock / 100
>   };
>   if (bt->interlaced)
>   dev->field_out = V4L2_FIELD_ALTERNATE;
> diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c 
> b/drivers/media/v4l2-core/v4l2-dv-timings.c
> index 6a83d61..adc03bd 100644
> --- a/drivers/media/v4l2-core/v4l2-dv-timings.c
> +++ b/drivers/media/v4l2-core/v4l2-dv-timings.c
> @@ -210,7 +210,12 @@ bool v4l2_find_dv_timings_cap(struct v4l2_dv_timings *t,
> fnc, fnc_handle) &&
>   v4l2_match_dv_timings(t, v4l2_dv_timings_presets + i,
> pclock_delta)) {
> + u32 flags = t->bt.flags & V4L2_DV_FL_REDUCED_FPS;
> +
>   *t = v4l2_dv_timings_presets[i];
> + if (t->bt.flags & V4L2_DV_FL_CAN_REDUCE_FPS)
> + t->bt.flags |= flags;
> +

This isn't quite right. This doesn't support V4L2_DV_BT_DMT_4096X2160P60_RB
which is a CVT format with reduced blanking v2 and so it should support reduced
fps.

In theory the 'if' above should check for both the V4L2_DV_FL_CAN_REDUCE_FPS and
for CVT RB v2 (using the helper function I've proposed). However, that would
cause weird behavior with the V4L2_DV_BT_DMT_4096X2160P59_94_RB timings since
these are already reduced fps.

I think the best approach is to add V4L2_DV_FL_CAN_REDUCE_FPS to the
V4L2_DV_BT_DMT_4096X2160P60_RB definition in v4l2-dv-timings.h.

That way the code above is unchanged, and it will work 

Re: [PATCH v8 53/55] [media] v4l2-core: create MC interfaces for devnodes

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 15:23:59 +0200
Hans Verkuil  escreveu:

> On 08/30/2015 05:07 AM, Mauro Carvalho Chehab wrote:
> > V4L2 device (and subdevice) nodes should create an
> > interface, if the Media Controller support is enabled.
> > 
> > Please notice that radio devices should not create an
> > entity, as radio input/output is either via wires or
> > via ALSA.
> 
> A general note: I think this patch (and any prerequisite patches) should come 
> before
> the patches adding G_TOPOLOGY support.
> 
> What the G_TOPOLOGY ioctl returns only makes sense IMHO if this code is 
> present as well,
> so it is a more logical order for this patch series.
> 
> In addition, since G_TOPOLOGY is a userspace API it is likely that that will 
> create
> more discussions, whereas this is internal to the kernel and could be merged 
> before
> G_TOPOLOGY.
> 
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
> > b/drivers/media/v4l2-core/v4l2-dev.c
> > index 44b330589787..427a5a32b3de 100644
> > --- a/drivers/media/v4l2-core/v4l2-dev.c
> > +++ b/drivers/media/v4l2-core/v4l2-dev.c
> > @@ -194,9 +194,12 @@ static void v4l2_device_release(struct device *cd)
> > mutex_unlock(_lock);
> >  
> >  #if defined(CONFIG_MEDIA_CONTROLLER)
> > -   if (v4l2_dev->mdev &&
> > -   vdev->vfl_type != VFL_TYPE_SUBDEV)
> > -   media_device_unregister_entity(>entity);
> > +   if (v4l2_dev->mdev) {
> > +   /* Remove interfaces and interface links */
> > +   media_devnode_remove(vdev->intf_devnode);
> > +   if (vdev->vfl_type != VFL_TYPE_SUBDEV)
> > +   media_device_unregister_entity(>entity);
> 
> RADIO doesn't have an entity either, so this should probably check for both
> SUBDEV and RADIO.
> 
> I think it is cleaner if video_register_media_controller() sets a new 
> video_device
> flag: V4L2_FL_CREATED_ENTITY, and if this release function would just check 
> the
> flag.

I don't see the need for a new flag here. I guess this should do the job:

diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
b/drivers/media/v4l2-core/v4l2-dev.c
index 427a5a32b3de..298aaf6f4296 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -197,7 +197,7 @@ static void v4l2_device_release(struct device *cd)
if (v4l2_dev->mdev) {
/* Remove interfaces and interface links */
media_devnode_remove(vdev->intf_devnode);
-   if (vdev->vfl_type != VFL_TYPE_SUBDEV)
+   if (vdev->entity.type)
media_device_unregister_entity(>entity);
}
 #endif
@@ -775,6 +775,8 @@ static int video_register_media_controller(struct 
video_device *vdev, int type)
__func__);
return ret;
}
+   } else {
+   vdev->entity.type = 0;
}
 
vdev->intf_devnode = media_devnode_create(vdev->v4l2_dev->mdev,

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/13] DocBook: add SDR specific info to G_MODULATOR / S_MODULATOR

[Antti Palosaari]

On 09/04/2015 01:26 PM, Hans Verkuil wrote:

On 09/01/2015 11:59 PM, Antti Palosaari wrote:

Add SDR specific notes to G_MODULATOR / S_MODULATOR documentation.

Signed-off-by: Antti Palosaari 
---
  Documentation/DocBook/media/v4l/vidioc-g-modulator.xml | 9 +
  1 file changed, 9 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml 
b/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
index 80167fc..affb694 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
@@ -78,6 +78,15 @@ different audio modulation if the request cannot be 
satisfied. However
  this is a write-only ioctl, it does not return the actual audio
  modulation selected.

+SDR specific modulator types are
+V4L2_TUNER_SDR and V4L2_TUNER_RF.
+Valid fields for these modulator types are index,
+name, capability,
+rangelow, rangehigh
+and type. All the rest fields must be


s/rest/remaining/


+initialized to zero by both application and driver.


I would drop this sentence. The spec is clear about which fields have to be set
by the user. The only thing I would mention here is that txsubchans should be
initialized to 0 by applications (we might want to use it in the future) when
calling S_MODULATOR.

For S_TUNER it is the same: only mention that audmode should be initialized to
0 for these SDR tuner types.


+Term modulator means SDR transmitter on this context.


s/Term/The term/
s/on/in/

Note: the same typos are in patch 12/13.

Perhaps this sentence should be rewritten since it is not clear what you
mean. I guess the idea is that 'modulator' is not a good match to what actually
happens in the SDR hardware?

How about:

"Note that the term 'modulator' is a misnomer for type V4L2_TUNER_SDR since
this really is a DAC and the 'modulator' frequency is in reality the sampling
frequency of the DAC."

I hope I got that right.

And do something similar for patch 12/13.


I added it mainly because struct v4l2_modulator is somehow misleading as 
it contains both modulator and RF frontend specific stuff and especially 
misleading for SDR case as modulator is located in a host computer 
software. On DVB side modulator/demodulator and tuner are split more 
correctly.


If you look that struct v4l2_modulator:
index: common field
name: common field
capability: contains both RF frontend and modulator stuff
rangelow: RF frontend specific
rangehigh: RF frontend specific
txsubchans: modulator specific
type: common field
reserved: reserved

So actually most field on that struct v4l2_modulator are RF frontend 
specific, not modulator. Same applies to struct v4l2_tuner.


These should be probably:
tuner => receiver
modulator => transmitter

Or even better, like DVB side, split modulator/demodulator and RF stuff 
to own structs. But as it is not reasonable to start changing those, so 
I decided to add comment for tuner that it means SDR receiver and for 
modulator it means SDR transmitter.


regards
Antti




+
  To change the radio frequency the  ioctl
  is available.




Regards,

Hans



--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 48/55] [media] media_device: add a topology version field

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 15:35:38 +0200
Hans Verkuil  escreveu:

> On 08/31/2015 02:52 PM, Mauro Carvalho Chehab wrote:
> > Em Mon, 31 Aug 2015 14:29:28 +0200
> > Hans Verkuil  escreveu:
> > 
> >> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> >>> Every time a graph object is added or removed, the version
> >>> of the topology changes. That's a requirement for the new
> >>> MEDIA_IOC_G_TOPOLOGY, in order to allow userspace to know
> >>> that the topology has changed after a previous call to it.
> >>>
> >>> Signed-off-by: Mauro Carvalho Chehab 
> >>
> >> I think this should be postponed until we actually have dynamic 
> >> reconfigurable
> >> graphs.
> > 
> > So far, we're using the term "dynamic" to mean partial graph object
> > removal.
> > 
> > But even today, MC does support "dynamic" in the sense of graph
> > object additions.
> > 
> > You should notice that having a topology_version is something that
> > IMHO, it is needed since the beginning, even without dynamic
> > reconfigurable graphs, because the graph may grow in runtime.
> > 
> > That will happen, for example, if usb-snd-audio is blacklisted
> > at /etc/modprobe*, and someone connects an au0828.
> > 
> > New entities/links will be created (after Shuah patches) if one would
> > modprobe latter snd-usb-audio.
> 
> latter -> later :-)
> 
> You are right, this would trigger a topology change. I hadn't thought about
> that.
> 
> > 
> >>
> >> I would also like to reserve version 0: if 0 is returned, then the graph is
> >> static.
> > 
> > Why? Implementing this would be really hard, as that would mean that
> > G_TOPOLOGY would need to be blocked until all drivers and subdevices
> > get probed.
> > 
> > In order to implement that, some logic would be needed at the drivers
> > to identify if everything was set and unlock G_TOPOLOGY.
> 
> That wouldn't be needed if the media device node was created last. Which
> I think is a good idea anyway.

Creating the media device node last won't work. It should be the first
thing to be created, as all objects should be added to media_device
linked lists. 

Also, the numberspace should be local to a given media_device, as the
graph traversal algorithm relies on having the number of entities <= 64,
currently, in order to be able to detect loops. We should increase that
number, but removing an "as low as possible" entity number limit is not
trivial.

> 
> > 
> > What would be the gain for that? I fail to see any.
> 
> It would tell userspace that it doesn't have to cope with dynamically
> changing graphs.
> 
> Even though with the au0828 example you can expect to see cases like that,
> I can pretty much guarantee that no generic v4l2 applications will ever
> support dynamic changes.

Well, my test app supports it and it is generic ;) My plan is to use it
as a basis for the library to be used on userspace for generic apps,
extending it to be used by other tools like xawtv, qv4l2 and the dvbv5 apps.

I don't think it is hard to handle it on a generic app, and this should
be done anyway if we want dynamic support.

The logic seems actually be simple:

at G_TOPOLOGY, if the topology changes, reload the objects;

at SETUP_LINK_V2, the topology will be sent. if the driver detects that
topology changed, it returns an error.

The caller will then need to reload the topology and re-apply the
transaction to select the links, if the entities affected still
exists. In other words, if the user's intent were to change the pipeline
to receive the data at /dev/video2, e. g. something like:
./yavta/yavta -f UYVY -s 720x312 -n 1 --capture=1 -F /dev/video2

What userspace would do is to reload everything, check if /dev/video2
still exists and then redo the function that it is equivalent to the
above command, failing otherwise. That doesn't sound hard to implement.

> Those that will support it will be custom-made.
> 
> Being able to see that graphs can change dynamically would allow such apps
> to either refuse to use the device, or warn the user.

The way I see is that applications that will assume that the graph
is static will be the custom-made ones. As they know the hardware,
they can just either ignore the topology_version or wait for it to
stabilize when the hardware is still being probed.

In any case, if we end by needing to have an explicit way for the
Kernelspace to tell userspace that a graph is static, that could
be done via an extra flag at MEDIA_INFO.

Enabling this flag could be as easy as waiting for all graph
elements to be created (where the topology is still dynamic),
and raise such flag after finishing the probe sequence.

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] [media] staging: lirc: mostly checkpatch cleanups

[Maciek Borzecki]

A tiny patch series that addresses warnings or errors identified by
checkpatch.

The patches were first submitted to driver-devel sometime in June.
While on driver-devel, Sudip Mukherjee helped to cleanup all
issues. I'm resending the set to linux-media so that they can be
picked up for the media tree.

The first patch fixes minor warning with unnecessary brakes around
single statement block. The second fixes non-tab indentation. The
third patch does away with a custom dprintk() in favor of dev_dbg and
pr_debug().


Maciek Borzecki (3):
  [media] staging: lirc: remove unnecessary braces
  [media] staging: lirc: fix indentation
  [media] staging: lirc: lirc_serial: use dynamic debugs

 drivers/staging/media/lirc/lirc_imon.c   |  8 
 drivers/staging/media/lirc/lirc_sasem.c  |  4 ++--
 drivers/staging/media/lirc/lirc_serial.c | 32 ++--
 3 files changed, 16 insertions(+), 28 deletions(-)

-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] [media] staging: lirc: fix indentation

[Maciek Borzecki]

Fix non-tab indentation.

This resolves the following checkpatch problem:
ERROR: code indent should use tabs where possible

Signed-off-by: Maciek Borzecki 
---
 drivers/staging/media/lirc/lirc_sasem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_sasem.c 
b/drivers/staging/media/lirc/lirc_sasem.c
index 
9e5674341abe7368e5ec228f737e4c2d766f7d80..904a4667bbb8bebe3cb43bf5201be9d533ada07a
 100644
--- a/drivers/staging/media/lirc/lirc_sasem.c
+++ b/drivers/staging/media/lirc/lirc_sasem.c
@@ -181,10 +181,10 @@ static void deregister_from_lirc(struct sasem_context 
*context)
if (retval)
dev_err(>dev->dev,
"%s: unable to deregister from lirc (%d)\n",
-  __func__, retval);
+   __func__, retval);
else
dev_info(>dev->dev,
-"Deregistered Sasem driver (minor:%d)\n", minor);
+"Deregistered Sasem driver (minor:%d)\n", minor);
 
 }
 
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] [media] staging: lirc: remove unnecessary braces

[Maciek Borzecki]

Remove unnecessary braces where appropriate.

This removes the following checkpatch warnings:
WARNING: braces {} are not necessary for single statement blocks

Signed-off-by: Maciek Borzecki 
---
 drivers/staging/media/lirc/lirc_imon.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_imon.c 
b/drivers/staging/media/lirc/lirc_imon.c
index 
62ec9f70dae4cd87dcf6fb60b1dd81df3d568b19..05d47dc8ffb8a987dc65287d36096a78cd5f96cd
 100644
--- a/drivers/staging/media/lirc/lirc_imon.c
+++ b/drivers/staging/media/lirc/lirc_imon.c
@@ -785,13 +785,13 @@ static int imon_probe(struct usb_interface *interface,
}
 
driver = kzalloc(sizeof(struct lirc_driver), GFP_KERNEL);
-   if (!driver) {
+   if (!driver)
goto free_context;
-   }
+
rbuf = kmalloc(sizeof(struct lirc_buffer), GFP_KERNEL);
-   if (!rbuf) {
+   if (!rbuf)
goto free_driver;
-   }
+
if (lirc_buffer_init(rbuf, BUF_CHUNK_SIZE, BUF_SIZE)) {
dev_err(dev, "%s: lirc_buffer_init failed\n", __func__);
goto free_rbuf;
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] [media] staging: lirc: lirc_serial: use dynamic debugs

[Maciek Borzecki]

Replace custom debug macro dprintk() with pr_debug() or
dev_dbg(). Remove unused module param `debug`.

This removes the following checkpatch warning:
WARNING: Prefer [subsystem eg: netdev]_dbg([subsystem]dev, ... then 
dev_dbg(dev, ... then pr_debug(...  to printk(KERN_DEBUG ...
+   printk(KERN_DEBUG LIRC_DRIVER_NAME ": " \

Signed-off-by: Maciek Borzecki 
---
 drivers/staging/media/lirc/lirc_serial.c | 32 ++--
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_serial.c 
b/drivers/staging/media/lirc/lirc_serial.c
index 
465796a686c417f2c23716c8fcc32e9d51489eed..64a7b2fc5289b6c1c0c0cabfa865756fb92208ed
 100644
--- a/drivers/staging/media/lirc/lirc_serial.c
+++ b/drivers/staging/media/lirc/lirc_serial.c
@@ -109,17 +109,9 @@ static bool iommap;
 static int ioshift;
 static bool softcarrier = true;
 static bool share_irq;
-static bool debug;
 static int sense = -1; /* -1 = auto, 0 = active high, 1 = active low */
 static bool txsense;   /* 0 = active high, 1 = active low */
 
-#define dprintk(fmt, args...)  \
-   do {\
-   if (debug)  \
-   printk(KERN_DEBUG LIRC_DRIVER_NAME ": " \
-  fmt, ## args);   \
-   } while (0)
-
 /* forward declarations */
 static long send_pulse_irdeo(unsigned long length);
 static long send_pulse_homebrew(unsigned long length);
@@ -352,10 +344,9 @@ static int init_timing_params(unsigned int new_duty_cycle,
/* Derive pulse and space from the period */
pulse_width = period * duty_cycle / 100;
space_width = period - pulse_width;
-   dprintk("in init_timing_params, freq=%d, duty_cycle=%d, "
-   "clk/jiffy=%ld, pulse=%ld, space=%ld\n",
-   freq, duty_cycle, __this_cpu_read(cpu_info.loops_per_jiffy),
-   pulse_width, space_width);
+   pr_debug("in init_timing_params, freq=%d, duty_cycle=%d, clk/jiffy=%ld, 
pulse=%ld, space=%ld, conv_us_to_clocks=%ld\n",
+freq, duty_cycle, __this_cpu_read(cpu_info.loops_per_jiffy),
+pulse_width, space_width, conv_us_to_clocks);
return 0;
 }
 #else /* ! USE_RDTSC */
@@ -377,8 +368,8 @@ static int init_timing_params(unsigned int new_duty_cycle,
period = 256 * 100L / freq;
pulse_width = period * duty_cycle / 100;
space_width = period - pulse_width;
-   dprintk("in init_timing_params, freq=%d pulse=%ld, space=%ld\n",
-   freq, pulse_width, space_width);
+   pr_debug("in init_timing_params, freq=%d pulse=%ld, space=%ld\n",
+freq, pulse_width, space_width);
return 0;
 }
 #endif /* USE_RDTSC */
@@ -500,7 +491,7 @@ static void rbwrite(int l)
 {
if (lirc_buffer_full()) {
/* no new signals will be accepted */
-   dprintk("Buffer overrun\n");
+   pr_debug("Buffer overrun\n");
return;
}
lirc_buffer_write(, (void *));
@@ -790,7 +781,7 @@ static int lirc_serial_probe(struct platform_device *dev)
dev_info(>dev, "Manually using active %s receiver\n",
 sense ? "low" : "high");
 
-   dprintk("Interrupt %d, port %04x obtained\n", irq, io);
+   dev_dbg(>dev, "Interrupt %d, port %04x obtained\n", irq, io);
return 0;
 }
 
@@ -895,7 +886,7 @@ static long lirc_ioctl(struct file *filep, unsigned int 
cmd, unsigned long arg)
return -ENOIOCTLCMD;
 
case LIRC_SET_SEND_DUTY_CYCLE:
-   dprintk("SET_SEND_DUTY_CYCLE\n");
+   pr_debug("SET_SEND_DUTY_CYCLE\n");
if (!(hardware[type].features_CAN_SET_SEND_DUTY_CYCLE))
return -ENOIOCTLCMD;
 
@@ -907,7 +898,7 @@ static long lirc_ioctl(struct file *filep, unsigned int 
cmd, unsigned long arg)
return init_timing_params(value, freq);
 
case LIRC_SET_SEND_CARRIER:
-   dprintk("SET_SEND_CARRIER\n");
+   pr_debug("SET_SEND_CARRIER\n");
if (!(hardware[type].features_CAN_SET_SEND_CARRIER))
return -ENOIOCTLCMD;
 
@@ -1102,7 +1093,7 @@ static void __exit lirc_serial_exit_module(void)
 {
lirc_unregister_driver(driver.minor);
lirc_serial_exit();
-   dprintk("cleaned up module\n");
+   pr_debug("cleaned up module\n");
 }
 
 
@@ -1153,6 +1144,3 @@ MODULE_PARM_DESC(txsense, "Sense of transmitter circuit"
 
 module_param(softcarrier, bool, S_IRUGO);
 MODULE_PARM_DESC(softcarrier, "Software carrier (0 = off, 1 = on, default 
on)");
-
-module_param(debug, bool, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(debug, "Enable debugging messages");
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a 

[GIT PULL for v4.3-rc1] media updates

[Mauro Carvalho Chehab]

Hi Linus,

Please pull from:
  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.3-1

For the media subsystem patches for Kernel 4.3.

This series contain:

- New DVB frontend drivers: ascot2e, cxd2841er, horus3a, lnbh25;
- New HDMI capture driver: tc358743;
- New driver for NetUP DVB new boards (netup_unidvb);
- IR support for DVBSky cards (smipcie-ir);
- Coda driver has gain macroblock tiling support;
- Renesas R-Car gains JPEG codec driver;
- New DVB platform driver for STi boards: c8sectpfe;
- Added documentation for the media core kABI to device-drivers DocBook;
- Lots of driver fixups, cleanups and improvements.

Thanks!
Mauro

-

The following changes since commit bc0195aad0daa2ad5b0d76cce22b167bc3435590:

  Linux 4.2-rc2 (2015-07-12 15:10:30 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.3-1

for you to fetch changes up to 50ef28a6ac216fd8b796257a3768fef8f57b917d:

  [media] c8sectpfe: Remove select on undefined LIBELF_32 (2015-09-03 14:10:06 
-0300)


media updates for v4.3-rc1


Abhilash Jindal (2):
  [media] zoran: Use monotonic time
  [media] bt8xxx: Use monotonic time

Andrzej Pietrasiewicz (1):
  [media] s5p-jpeg: Eliminate double kfree()

Antonio Borneo (1):
  [media] s5c73m3: Remove redundant spi driver bus initialization

Antti Palosaari (11):
  [media] em28xx: remove unused a8293 SEC config
  [media] a8293: remove legacy media attach
  [media] a8293: use i2c_master_send / i2c_master_recv for I2C I/O
  [media] a8293: improve LNB register programming logic
  [media] a8293: coding style issues
  [media] tda10071: remove legacy media attach
  [media] tda10071: rename device state struct to dev
  [media] tda10071: convert to regmap I2C API
  [media] tda10071: protect firmware command exec with mutex
  [media] tda10071: do not get_frontend() when not ready
  [media] tda10071: implement DVBv5 statistics

Ben Dooks (1):
  [media] media: adv7180: add of match table

Benoit Parrot (2):
  [media] media: am437x-vpfe: Requested frame size and fmt overwritten by 
current sensor setting
  [media] media: am437x-vpfe: Fix a race condition during release

Christian Löpke (1):
  [media] Technisat SkyStar USB HD,(DVB-S/S2) too much URBs for arm devices

Damian Hobson-Garcia (1):
  [media] v4l: vsp1: Align crop rectangle to even boundary for YUV formats

Dan Carpenter (2):
  [media] gspca: sn9c2028: remove an unneeded condition
  [media] v4l: xilinx: missing error code

David Härdeman (5):
  [media] rc-core: fix remove uevent generation
  [media] rc-core: use an IDA rather than a bitmap
  [media] rc-core: remove the LIRC "protocol"
  [media] lmedm04: NEC scancode cleanup
  [media] rc-core: improve the lirc protocol reporting

Ezequiel Garcia (3):
  [media] stk1160: Reduce driver verbosity
  [media] stk1160: Add frame scaling support
  [media] tw68: Move PCI vendor and device IDs to pci_ids.h

Fabian Frederick (6):
  [media] v4l2-dv-timings: use swap() in v4l2_calc_aspect_ratio()
  [media] wl128x: use swap() in fm_rdsparse_swapbytes()
  [media] saa7146: use swap() in sort_and_eliminate()
  [media] saa6588: use swap() in saa6588_i2c_poll()
  [media] btcx-risc: use swap() in btcx_sort_clips()
  [media] ttusb-dec: use swap() in swap_bytes()

Fabien Dessenne (3):
  [media] bdisp: composing support
  [media] bdisp: add debug info for RGB24 format
  [media] bdisp: fix debug info memory access

Fabio Estevam (2):
  [media] mantis: Fix error handling in mantis_dma_init()
  [media] c8sectpfe: Use %pad to print 'dma_addr_t'

Fengguang Wu (1):
  [media] i2c: fix platform_no_drv_owner.cocci warnings

Geert Uytterhoeven (4):
  [media] adv7604/cobalt: Allow compile test if !GPIOLIB
  [media] dvb-frontends: Make all DVB Frontends visible if COMPILE_TEST=y
  [media] i2c: Make all i2c devices visible if COMPILE_TEST=y
  [media] tuners: Make all TV tuners visible if COMPILE_TEST=y

Hans Verkuil (60):
  [media] stk1160: fix sequence handling
  [media] rc/Kconfig: fix indentation problem
  [media] v4l2-dv-timings: log if the timing is reduced blanking V2
  [media] clock-sh7724.c: fix sh-vou clock identifier
  [media] sh-vou: use resource managed calls
  [media] sh-vou: fix querycap support
  [media] sh-vou: use v4l2_fh
  [media] sh-vou: support compulsory G/S/ENUM_OUTPUT ioctls
  [media] sh-vou: fix incorrect initial pixelformat
  [media] sh-vou: replace g/s_crop/cropcap by g/s_selection
  [media] sh-vou: let sh_vou_s_fmt_vid_out call sh_vou_try_fmt_vid_out
  [media] sh-vou: fix bytesperline
  [media] sh-vou: convert to vb2
  [media] sh-vou: add support 

cron job: media_tree daily build: OK

[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:   Sat Sep  5 04:00:24 CEST 2015
git branch: test
git hash:   50ef28a6ac216fd8b796257a3768fef8f57b917d
gcc version:i686-linux-gcc (GCC) 5.1.0
sparse version: v0.5.0-51-ga53cea2
smatch version: 0.4.1-3153-g7d56ab3
host hardware:  x86_64
host os:4.0.0-3.slh.1-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: 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.32.27-i686: OK
linux-2.6.33.7-i686: OK
linux-2.6.34.7-i686: OK
linux-2.6.35.9-i686: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16.7-i686: OK
linux-3.17.8-i686: OK
linux-3.18.7-i686: OK
linux-3.19-i686: OK
linux-4.0-i686: OK
linux-4.1.1-i686: OK
linux-4.2-i686: OK
linux-2.6.32.27-x86_64: OK
linux-2.6.33.7-x86_64: OK
linux-2.6.34.7-x86_64: OK
linux-2.6.35.9-x86_64: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16.7-x86_64: OK
linux-3.17.8-x86_64: OK
linux-3.18.7-x86_64: OK
linux-3.19-x86_64: OK
linux-4.0-x86_64: OK
linux-4.1.1-x86_64: OK
linux-4.2-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: ERRORS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 54/55] [media] au0828: unregister MC at the end

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 15:25:39 +0200
Hans Verkuil  escreveu:

> On 08/30/2015 05:07 AM, Mauro Carvalho Chehab wrote:
> > au0828_analog_unregister() calls video_unregister_device(),
> > with, in turn, calls media_devnode_remove() in order to drop
> > the media interfaces.
> > 
> > We can't release the media controller before that, or an
> > OOPS will occur:
> 
> So this patch should be moved to a place earlier in the patch series,
> right? To prevent bisects that hit this bug.

Yes, it should be before the previous patch.

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Dvb-fe-tool add features

[Tomáš Vítek]

  Hello,


   Please add to dvb-fe-tool function as a femon from dvb-apps.

   ,,-c number : samples to take (default 0 = infinite),,

   This is the number of repetitions of reading signal.

   Thanks.



--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 19/55] [media] media: convert links from array to list

[Sakari Ailus]

Hi Mauro,

On Sun, Aug 30, 2015 at 12:06:30AM -0300, Mauro Carvalho Chehab wrote:
> The entire logic that represent graph links were developed on a
> time where there were no needs to dynamic remove links. So,
> although links are created/removed one by one via some
> functions, they're stored as an array inside the entity struct.
> 
> As the array may grow, there's a logic inside the code that
> checks if the amount of space is not enough to store
> the needed links. If it isn't the core uses krealloc()
> to change the size of the link, with is bad, as it
> leaves the memory fragmented.

The Linux kernel constantly allocates and releases memory for various
reasons. Even the V4L2 IOCTL handler does that when the argument is too
large to fit into the stack-allocated buffer. There's no reason to avoid
allocating and releasing small chunks of memory when hardware configuration
changes.

> 
> So, convert links into a list.

Instead, if you're worried about the time the memory re-allocation takes
that'd be needed to dynamically add a large number of links, I'd just
allocate more memory at a time, say, rounding up to a power of two, and use
vmalloc() instead of kmalloc() when the size grows over one page.

That'd avoid the vast majority of reallocs without wasting much memory: the
granularity of memory allocation is much larger than the size of struct
media_link except in the case of very small link numbers.

The reason I'm proposing this is that linked lists, as you're allocating
them, can end up anywhere in the system memory. Walking over the media graph
is quite a job if you have as many links as in your example, and the
additional cache misses caused by scattering the data structure all over the
system memory will not make the effort smaller.

Adding links dynamically (for a change in hardware topology?) is by far less
performance critical than e.g. starting and stopping streaming or
enumerating the links so the latter should have the priority IMHO.

> 
> Also, currently,  both source and sink entities need the link
> at the graph traversal logic inside media_entity. So there's
> a logic duplicating all links. That makes it to spend
> twice the memory needed. This is not a big deal for today's
> usage, where the number of links are not big.
> 
> Yet, if during the MC workshop discussions, it was said that
> IIO graphs could have up to 4,000 entities. So, we may
> want to remove the duplication on some future. The problem
> is that it would require a separate linked list to store
> the backlinks inside the entity, or to use a more complex
> algorithm to do graph backlink traversal, with is something
> that the current graph traversal inside the core can't cope
> with. So, let's postpone a such change if/when it is actually
> needed.

Would keeping the links in a linked list help with that?

I think this could be done using both the current arrays or linked lists ---
instead of storing links themselves, you'd store pointers to the links
(array or a linked list) which then are stored elsewhere. Helper functions
would be needed to e.g. loop over the links in that case though.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html