Re: [PATCH v2 12/48] ths8200: Add pad-level DV timings operations
On Tue, Mar 11, 2014 at 4:45 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: The video enum_dv_timings and dv_timings_cap operations are deprecated. Implement the pad-level version of those operations to prepare for the removal of the video version. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com Regards, --Prabhakar lad --- drivers/media/i2c/ths8200.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/media/i2c/ths8200.c b/drivers/media/i2c/ths8200.c index f72561e..c4ec8b2 100644 --- a/drivers/media/i2c/ths8200.c +++ b/drivers/media/i2c/ths8200.c @@ -410,6 +410,9 @@ static int ths8200_g_dv_timings(struct v4l2_subdev *sd, static int ths8200_enum_dv_timings(struct v4l2_subdev *sd, struct v4l2_enum_dv_timings *timings) { + if (timings-pad != 0) + return -EINVAL; + return v4l2_enum_dv_timings_cap(timings, ths8200_timings_cap, NULL, NULL); } @@ -417,6 +420,9 @@ static int ths8200_enum_dv_timings(struct v4l2_subdev *sd, static int ths8200_dv_timings_cap(struct v4l2_subdev *sd, struct v4l2_dv_timings_cap *cap) { + if (cap-pad != 0) + return -EINVAL; + *cap = ths8200_timings_cap; return 0; } @@ -430,10 +436,16 @@ static const struct v4l2_subdev_video_ops ths8200_video_ops = { .dv_timings_cap = ths8200_dv_timings_cap, }; +static const struct v4l2_subdev_pad_ops ths8200_pad_ops = { + .enum_dv_timings = ths8200_enum_dv_timings, + .dv_timings_cap = ths8200_dv_timings_cap, +}; + /* V4L2 top level operation handlers */ static const struct v4l2_subdev_ops ths8200_ops = { .core = ths8200_core_ops, .video = ths8200_video_ops, + .pad = ths8200_pad_ops, }; static int ths8200_probe(struct i2c_client *client, -- 1.8.3.2 -- 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 v2 24/48] tvp7002: Remove deprecated video-level DV timings operations
On Tue, Mar 11, 2014 at 4:45 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: The video enum_dv_timings and dv_timings_cap operations are deprecated and unused. Remove them. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com Regards, --Prabhakar lad --- drivers/media/i2c/tvp7002.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c index 9f56fd5..fa901a9 100644 --- a/drivers/media/i2c/tvp7002.c +++ b/drivers/media/i2c/tvp7002.c @@ -926,7 +926,6 @@ static const struct v4l2_subdev_core_ops tvp7002_core_ops = { static const struct v4l2_subdev_video_ops tvp7002_video_ops = { .g_dv_timings = tvp7002_g_dv_timings, .s_dv_timings = tvp7002_s_dv_timings, - .enum_dv_timings = tvp7002_enum_dv_timings, .query_dv_timings = tvp7002_query_dv_timings, .s_stream = tvp7002_s_stream, .g_mbus_fmt = tvp7002_mbus_fmt, -- 1.8.3.2 -- 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 v2 13/48] tvp7002: Add pad-level DV timings operations
On Tue, Mar 11, 2014 at 4:45 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: The video enum_dv_timings operation is deprecated. Implement the pad-level version of the operation to prepare for the removal of the video version. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com Regards, --Prabhakar lad --- drivers/media/i2c/tvp7002.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c index 912e1cc..9f56fd5 100644 --- a/drivers/media/i2c/tvp7002.c +++ b/drivers/media/i2c/tvp7002.c @@ -832,6 +832,9 @@ static int tvp7002_log_status(struct v4l2_subdev *sd) static int tvp7002_enum_dv_timings(struct v4l2_subdev *sd, struct v4l2_enum_dv_timings *timings) { + if (timings-pad != 0) + return -EINVAL; + /* Check requested format index is within range */ if (timings-index = NUM_TIMINGS) return -EINVAL; @@ -937,6 +940,7 @@ static const struct v4l2_subdev_pad_ops tvp7002_pad_ops = { .enum_mbus_code = tvp7002_enum_mbus_code, .get_fmt = tvp7002_get_pad_format, .set_fmt = tvp7002_set_pad_format, + .enum_dv_timings = tvp7002_enum_dv_timings, }; /* V4L2 top level operation handlers */ -- 1.8.3.2 -- 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 v2 23/48] ths8200: Remove deprecated video-level DV timings operations
On Tue, Mar 11, 2014 at 4:45 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: The video enum_dv_timings and dv_timings_cap operations are deprecated and unused. Remove them. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com Regards, --Prabhakar lad --- drivers/media/i2c/ths8200.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/media/i2c/ths8200.c b/drivers/media/i2c/ths8200.c index c4ec8b2..656d889 100644 --- a/drivers/media/i2c/ths8200.c +++ b/drivers/media/i2c/ths8200.c @@ -432,8 +432,6 @@ static const struct v4l2_subdev_video_ops ths8200_video_ops = { .s_stream = ths8200_s_stream, .s_dv_timings = ths8200_s_dv_timings, .g_dv_timings = ths8200_g_dv_timings, - .enum_dv_timings = ths8200_enum_dv_timings, - .dv_timings_cap = ths8200_dv_timings_cap, }; static const struct v4l2_subdev_pad_ops ths8200_pad_ops = { -- 1.8.3.2 -- 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 v2 15/48] media: davinci: vpif: Switch to pad-level DV operations
Hi Laurent, Thanks for the patch. On Tue, Mar 11, 2014 at 4:45 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: The video-level enum_dv_timings and dv_timings_cap operations are deprecated in favor of the pad-level versions. All subdev drivers implement the pad-level versions, switch to them. Cc: Prabhakar Lad prabhakar.cse...@gmail.com Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com Regards, --Prabhakar lad --- drivers/media/platform/davinci/vpif_capture.c | 4 +++- drivers/media/platform/davinci/vpif_display.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c index cd6da8b..16a1958 100644 --- a/drivers/media/platform/davinci/vpif_capture.c +++ b/drivers/media/platform/davinci/vpif_capture.c @@ -1723,7 +1723,9 @@ vpif_enum_dv_timings(struct file *file, void *priv, struct channel_obj *ch = fh-channel; int ret; - ret = v4l2_subdev_call(ch-sd, video, enum_dv_timings, timings); + timings-pad = 0; + + ret = v4l2_subdev_call(ch-sd, pad, enum_dv_timings, timings); if (ret == -ENOIOCTLCMD || ret == -ENODEV) return -EINVAL; return ret; diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c index fd68236..e1edefe 100644 --- a/drivers/media/platform/davinci/vpif_display.c +++ b/drivers/media/platform/davinci/vpif_display.c @@ -1380,7 +1380,9 @@ vpif_enum_dv_timings(struct file *file, void *priv, struct channel_obj *ch = fh-channel; int ret; - ret = v4l2_subdev_call(ch-sd, video, enum_dv_timings, timings); + timings-pad = 0; + + ret = v4l2_subdev_call(ch-sd, pad, enum_dv_timings, timings); if (ret == -ENOIOCTLCMD || ret == -ENODEV) return -EINVAL; return ret; -- 1.8.3.2 -- 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 16/48] media: staging: davinci: vpfe: Switch to pad-level DV operations
Hi Laurent, Thanks for the patch. On Tue, Mar 11, 2014 at 4:45 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: The video-level enum_dv_timings and dv_timings_cap operations are deprecated in favor of the pad-level versions. All subdev drivers implement the pad-level versions, switch to them. Cc: Prabhakar Lad prabhakar.cse...@gmail.com Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com Regards, --Prabhakar lad --- drivers/staging/media/davinci_vpfe/vpfe_video.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c b/drivers/staging/media/davinci_vpfe/vpfe_video.c index 1f3b0f9..a1655a8 100644 --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c @@ -987,8 +987,10 @@ vpfe_enum_dv_timings(struct file *file, void *fh, struct vpfe_device *vpfe_dev = video-vpfe_dev; struct v4l2_subdev *subdev = video-current_ext_subdev-subdev; + timings-pad = 0; + v4l2_dbg(1, debug, vpfe_dev-v4l2_dev, vpfe_enum_dv_timings\n); - return v4l2_subdev_call(subdev, video, enum_dv_timings, timings); + return v4l2_subdev_call(subdev, pad, enum_dv_timings, timings); } /* -- 1.8.3.2 -- 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 39/48] v4l: subdev: Remove deprecated video-level DV timings operations
On Tue, Mar 11, 2014 at 4:45 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: The video enum_dv_timings and dv_timings_cap operations are deprecated and unused. Remove them. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Sakari Ailus sakari.ai...@linux.intel.com Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com Regards, --Prabhakar lad --- include/media/v4l2-subdev.h | 4 1 file changed, 4 deletions(-) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 2b5ec32..ab2b59d 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -330,12 +330,8 @@ struct v4l2_subdev_video_ops { struct v4l2_dv_timings *timings); int (*g_dv_timings)(struct v4l2_subdev *sd, struct v4l2_dv_timings *timings); - int (*enum_dv_timings)(struct v4l2_subdev *sd, - struct v4l2_enum_dv_timings *timings); int (*query_dv_timings)(struct v4l2_subdev *sd, struct v4l2_dv_timings *timings); - int (*dv_timings_cap)(struct v4l2_subdev *sd, - struct v4l2_dv_timings_cap *cap); int (*enum_mbus_fmt)(struct v4l2_subdev *sd, unsigned int index, enum v4l2_mbus_pixelcode *code); int (*enum_mbus_fsizes)(struct v4l2_subdev *sd, -- 1.8.3.2 -- 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 v2 06/48] v4l: Add pad-level DV timings subdev operations
On Tue, Mar 11, 2014 at 4:45 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com Regards, --Prabhakar lad --- include/media/v4l2-subdev.h| 4 include/uapi/linux/videodev2.h | 10 -- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 1752530..2b5ec32 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -509,6 +509,10 @@ struct v4l2_subdev_pad_ops { struct v4l2_subdev_selection *sel); int (*get_edid)(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid); int (*set_edid)(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid); + int (*dv_timings_cap)(struct v4l2_subdev *sd, + struct v4l2_dv_timings_cap *cap); + int (*enum_dv_timings)(struct v4l2_subdev *sd, + struct v4l2_enum_dv_timings *timings); #ifdef CONFIG_MEDIA_CONTROLLER int (*link_validate)(struct v4l2_subdev *sd, struct media_link *link, struct v4l2_subdev_format *source_fmt, diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 17acba8..72fbbd4 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1103,12 +1103,15 @@ struct v4l2_dv_timings { /** struct v4l2_enum_dv_timings - DV timings enumeration * @index: enumeration index + * @pad: the pad number for which to enumerate timings (used with + * v4l-subdev nodes only) * @reserved: must be zeroed * @timings: the timings for the given index */ struct v4l2_enum_dv_timings { __u32 index; - __u32 reserved[3]; + __u32 pad; + __u32 reserved[2]; struct v4l2_dv_timings timings; }; @@ -1146,11 +1149,14 @@ struct v4l2_bt_timings_cap { /** struct v4l2_dv_timings_cap - DV timings capabilities * @type: the type of the timings (same as in struct v4l2_dv_timings) + * @pad: the pad number for which to query capabilities (used with + * v4l-subdev nodes only) * @bt:the BT656/1120 timings capabilities */ struct v4l2_dv_timings_cap { __u32 type; - __u32 reserved[3]; + __u32 pad; + __u32 reserved[2]; union { struct v4l2_bt_timings_cap bt; __u32 raw_data[32]; -- 1.8.3.2 -- 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
[PATCH v3 00/14] v4l: ti-vpe: Some VPE fixes and enhancements
This patch set mainly consists of minor fixes for the VPE driver. These fixes ensure the following: - The VPE module can be inserted and removed successively. - Make sure that smaller resolutions like qcif work correctly. - Prevent race condition between firmware loading and an open call to the v4l2 device. - Prevent the possibility of output m2m queue not having sufficient 'ready' buffers. - Some VPDMA data descriptor fields weren't understood correctly before. They are now used correctly. The rest of the patches add some minor features like DMA buf support and cropping/composing. Reference branch: g...@github.com:boddob/linux.git vpe_for_315 Changes in v3: - improvements in selection API patch. - querycap fixes for v4l2 compliance. - v4l2_buffer 'field' and flags' fixes for compliance. - fixes in try_fmt vpe_open for compliance. - rename a IOMEM resource for better DT compatibility. Changes in v2: - selection API used instead of older cropping API. - Typo fix. - Some changes in patch 6/7 to support composing on the capture side of VPE. Archit Taneja (14): v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs v4l: ti-vpe: register video device only when firmware is loaded v4l: ti-vpe: Use video_device_release_empty v4l: ti-vpe: Allow DMABUF buffer type support v4l: ti-vpe: Allow usage of smaller images v4l: ti-vpe: Fix some params in VPE data descriptors v4l: ti-vpe: Add selection API in VPE driver v4l: ti-vpe: Rename csc memory resource name v4l: ti-vpe: report correct capabilities in querycap v4l: ti-vpe: Use correct bus_info name for the device in querycap v4l: ti-vpe: Fix initial configuration queue data v4l: ti-vpe: zero out reserved fields in try_fmt v4l: ti-vpe: Set correct field parameter for output and capture buffers v4l: ti-vpe: retain v4l2_buffer flags for captured buffers drivers/media/platform/ti-vpe/csc.c | 2 +- drivers/media/platform/ti-vpe/vpdma.c | 68 ++--- drivers/media/platform/ti-vpe/vpdma.h | 17 ++- drivers/media/platform/ti-vpe/vpe.c | 263 -- 4 files changed, 281 insertions(+), 69 deletions(-) -- 1.8.3.2 -- 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 v3 03/14] v4l: ti-vpe: Use video_device_release_empty
The video_device struct is currently embedded in the driver data struct vpe_dev. A vpe_dev instance is allocated by the driver, and the memory for the vfd is a part of this struct. The v4l2 core, however, manages the removal of the vfd region, through the video_device's .release() op, which currently is the helper video_device_release. This causes memory corruption, and leads to issues when we try to re-insert the vpe module. Use the video_device_release_empty helper function instead Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index f1eae67..0363df6 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -2000,7 +2000,7 @@ static struct video_device vpe_videodev = { .fops = vpe_fops, .ioctl_ops = vpe_ioctl_ops, .minor = -1, - .release= video_device_release, + .release= video_device_release_empty, .vfl_dir= VFL_DIR_M2M, }; -- 1.8.3.2 -- 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 v3 01/14] v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs
VPE has a ctrl parameter which decides how many mem to mem transactions the active job from the job queue can perform. The driver's job_ready() made sure that the number of ready source buffers are sufficient for the job to execute successfully. But it didn't make sure if there are sufficient ready destination buffers in the capture queue for the VPE output. If the time taken by VPE to process a single frame is really slow, then it's possible that we don't need to imply such a restriction on the dst queue, but really fast transactions(small resolution, no de-interlacing) may cause us to hit the condition where we don't have any free buffers for the VPE to write on. Add the extra check in job_ready() to make sure we have the sufficient amount of destination buffers. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 7a77a5b..f3143ac 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -887,6 +887,9 @@ static int job_ready(void *priv) if (v4l2_m2m_num_src_bufs_ready(ctx-m2m_ctx) needed) return 0; + if (v4l2_m2m_num_dst_bufs_ready(ctx-m2m_ctx) needed) + return 0; + return 1; } -- 1.8.3.2 -- 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 v3 11/14] v4l: ti-vpe: Fix initial configuration queue data
The vpe output and capture queues are initially configured to default values in vpe_open(). A G_FMT before any S_FMTs will result in these values being populated. The colorspace and bytesperline parameter of this initial configuration are incorrect. This breaks compliance when as we get 'TRY_FMT(G_FMT) != G_FMT'. Fix the initial queue configuration such that it wouldn't need to be fixed by try_fmt. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 5591d04..85d1122 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -2012,9 +2012,11 @@ static int vpe_open(struct file *file) s_q_data-fmt = vpe_formats[2]; s_q_data-width = 1920; s_q_data-height = 1080; - s_q_data-sizeimage[VPE_LUMA] = (s_q_data-width * s_q_data-height * + s_q_data-bytesperline[VPE_LUMA] = (s_q_data-width * s_q_data-fmt-vpdma_fmt[VPE_LUMA]-depth) 3; - s_q_data-colorspace = V4L2_COLORSPACE_SMPTE170M; + s_q_data-sizeimage[VPE_LUMA] = (s_q_data-bytesperline[VPE_LUMA] * + s_q_data-height); + s_q_data-colorspace = V4L2_COLORSPACE_REC709; s_q_data-field = V4L2_FIELD_NONE; s_q_data-c_rect.left = 0; s_q_data-c_rect.top = 0; -- 1.8.3.2 -- 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 v3 13/14] v4l: ti-vpe: Set correct field parameter for output and capture buffers
The vpe driver wasn't setting the correct field parameter for dequed CAPTURE type buffers for the case where the captured output is progressive. Set the field to V4L2_FIELD_NONE for the completed destination buffers when the captured output is progressive. For OUTPUT type buffers, a queued buffer's field is forced to V4L2_FIELD_NONE if the pixel format(configured through s_fmt for the buffer type V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE specifies) the field type isn't interlaced. If the pixel format specified was V4L2_FIELD_ALTERNATE, and the queued buffer's field isn't V4L2_FIELD_TOP or V4L2_FIELD_BOTTOM, the vb2 buf_prepare op returns an error. This ensures compliance, and that the dequeued output and captured buffers contain the field type that the driver used internally. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 970408a..c884910 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1296,10 +1296,10 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) d_buf-timecode = s_buf-timecode; } d_buf-sequence = ctx-sequence; - d_buf-field = ctx-field; d_q_data = ctx-q_data[Q_DATA_DST]; if (d_q_data-flags Q_DATA_INTERLACED) { + d_buf-field = ctx-field; if (ctx-field == V4L2_FIELD_BOTTOM) { ctx-sequence++; ctx-field = V4L2_FIELD_TOP; @@ -1308,6 +1308,7 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) ctx-field = V4L2_FIELD_BOTTOM; } } else { + d_buf-field = V4L2_FIELD_NONE; ctx-sequence++; } @@ -1871,6 +1872,16 @@ static int vpe_buf_prepare(struct vb2_buffer *vb) q_data = get_q_data(ctx, vb-vb2_queue-type); num_planes = q_data-fmt-coplanar ? 2 : 1; + if (vb-vb2_queue-type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { + if (!(q_data-flags Q_DATA_INTERLACED)) { + vb-v4l2_buf.field = V4L2_FIELD_NONE; + } else { + if (vb-v4l2_buf.field != V4L2_FIELD_TOP || + vb-v4l2_buf.field != V4L2_FIELD_BOTTOM) + return -EINVAL; + } + } + for (i = 0; i num_planes; i++) { if (vb2_plane_size(vb, i) q_data-sizeimage[i]) { vpe_err(ctx-dev, -- 1.8.3.2 -- 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 v3 07/14] v4l: ti-vpe: Add selection API in VPE driver
Add selection ioctl ops. For VPE, cropping makes sense only for the input to VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers). For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP results in selecting a rectangle region within the source buffer. Setting the crop/compose rectangles should successfully result in re-configuration of registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 141 1 file changed, 141 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index ece9b96..4abb85c 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx, { switch (type) { case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: + case V4L2_BUF_TYPE_VIDEO_OUTPUT: return ctx-q_data[Q_DATA_SRC]; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: + case V4L2_BUF_TYPE_VIDEO_CAPTURE: return ctx-q_data[Q_DATA_DST]; default: BUG(); @@ -1587,6 +1589,142 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f) return set_srcdst_params(ctx); } +static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s) +{ + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + case V4L2_SEL_TGT_COMPOSE: + /* +* COMPOSE target is only valid for capture buffer type, for +* output buffer type, assign existing crop parameters to the +* selection rectangle +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) + break; + + s-r = q_data-c_rect; + return 0; + + case V4L2_SEL_TGT_CROP: + /* +* CROP target is only valid for output buffer type, for capture +* buffer type, assign existing compose parameters to the +* selection rectangle +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) + break; + + s-r = q_data-c_rect; + return 0; + + /* +* bound and default crop/compose targets are invalid targets to +* try/set +*/ + default: + return -EINVAL; + } + + if (s-r.top 0 || s-r.left 0) { + vpe_err(ctx-dev, negative values for top and left\n); + s-r.top = s-r.left = 0; + } + + v4l_bound_align_image(s-r.width, MIN_W, q_data-width, 1, + s-r.height, MIN_H, q_data-height, H_ALIGN, S_ALIGN); + + /* adjust left/top if cropping rectangle is out of bounds */ + if (s-r.left + s-r.width q_data-width) + s-r.left = q_data-width - s-r.width; + if (s-r.top + s-r.height q_data-height) + s-r.top = q_data-height - s-r.height; + + return 0; +} + +static int vpe_g_selection(struct file *file, void *fh, + struct v4l2_selection *s) +{ + struct vpe_ctx *ctx = file2ctx(file); + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + /* return width and height from S_FMT of the respective buffer type */ + case V4L2_SEL_TGT_COMPOSE_DEFAULT: + case V4L2_SEL_TGT_COMPOSE_BOUNDS: + case V4L2_SEL_TGT_CROP_BOUNDS: + case V4L2_SEL_TGT_CROP_DEFAULT: + s-r.left = 0; + s-r.top = 0; + s-r.width = q_data-width; + s-r.height = q_data-height; + return 0; + + /* +* CROP target holds for the output buffer type, and COMPOSE target +* holds for the capture buffer type. We still return the c_rect params +* for both the target types. +*/ + case V4L2_SEL_TGT_COMPOSE: + case V4L2_SEL_TGT_CROP: + s-r.left = q_data-c_rect.left; + s-r.top = q_data-c_rect.top; + s-r.width = q_data-c_rect.width; + s-r.height = q_data-c_rect.height; + return 0; + } + + return
[PATCH v3 05/14] v4l: ti-vpe: Allow usage of smaller images
The minimum width and height for VPE input/output was kept as 128 pixels. VPE doesn't have a constraint on the image height, it requires the image width to be at least 16 bytes. Change the minimum supported dimensions to 32x32. This allows us to de-interlace qcif content. A smaller image size than 32x32 didn't make much sense, so stopped at this. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 0e7573a..dbdc338 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -49,8 +49,8 @@ #define VPE_MODULE_NAME vpe /* minimum and maximum frame sizes */ -#define MIN_W 128 -#define MIN_H 128 +#define MIN_W 32 +#define MIN_H 32 #define MAX_W 1920 #define MAX_H 1080 -- 1.8.3.2 -- 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 v3 04/14] v4l: ti-vpe: Allow DMABUF buffer type support
For OMAP and DRA7x, we generally allocate video and graphics buffers through omapdrm since the corresponding omap-gem driver provides DMM-Tiler backed contiguous buffers. omapdrm is a dma-buf exporter. These buffers are used by other drivers in the video pipeline. Add VB2_DMABUF flag to the io_modes of the vb2 output and capture queues. This allows the driver to import dma shared buffers. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 0363df6..0e7573a 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1770,7 +1770,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(src_vq, 0, sizeof(*src_vq)); src_vq-type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; - src_vq-io_modes = VB2_MMAP; + src_vq-io_modes = VB2_MMAP | VB2_DMABUF; src_vq-drv_priv = ctx; src_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); src_vq-ops = vpe_qops; @@ -1783,7 +1783,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(dst_vq, 0, sizeof(*dst_vq)); dst_vq-type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; - dst_vq-io_modes = VB2_MMAP; + dst_vq-io_modes = VB2_MMAP | VB2_DMABUF; dst_vq-drv_priv = ctx; dst_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); dst_vq-ops = vpe_qops; -- 1.8.3.2 -- 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 v3 10/14] v4l: ti-vpe: Use correct bus_info name for the device in querycap
The bus_info parameter in v4l2_capabilities expects a 'platform_' prefix. This wasn't done in the driver and hence was breaking compliance. Update the bus_info parameter accordingly. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 46b9d44..5591d04 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1346,7 +1346,8 @@ static int vpe_querycap(struct file *file, void *priv, { strncpy(cap-driver, VPE_MODULE_NAME, sizeof(cap-driver) - 1); strncpy(cap-card, VPE_MODULE_NAME, sizeof(cap-card) - 1); - strlcpy(cap-bus_info, VPE_MODULE_NAME, sizeof(cap-bus_info)); + snprintf(cap-bus_info, sizeof(cap-bus_info), platform:%s, + VPE_MODULE_NAME); cap-device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING; cap-capabilities = cap-device_caps | V4L2_CAP_DEVICE_CAPS; return 0; -- 1.8.3.2 -- 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 v3 12/14] v4l: ti-vpe: zero out reserved fields in try_fmt
Zero out the reserved formats in v4l2_pix_format_mplane and v4l2_plane_pix_format members of the returned v4l2_format pointer when passed through TRY_FMT ioctl. This ensures that the user doesn't interpret the non-zero fields as some data passed by the driver, and ensures compliance. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 85d1122..970408a 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1488,6 +1488,7 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, } } + memset(pix-reserved, 0, sizeof(pix-reserved)); for (i = 0; i pix-num_planes; i++) { plane_fmt = pix-plane_fmt[i]; depth = fmt-vpdma_fmt[i]-depth; @@ -1499,6 +1500,8 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, plane_fmt-sizeimage = (pix-height * pix-width * depth) 3; + + memset(plane_fmt-reserved, 0, sizeof(plane_fmt-reserved)); } return 0; -- 1.8.3.2 -- 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 v3 09/14] v4l: ti-vpe: report correct capabilities in querycap
querycap currently returns V4L2_CAP_VIDEO_M2M as a capability, this should be V4L2_CAP_VIDEO_M2M_MPLANE instead, as the driver supports multiplanar formats. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 4abb85c..46b9d44 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1347,7 +1347,7 @@ static int vpe_querycap(struct file *file, void *priv, strncpy(cap-driver, VPE_MODULE_NAME, sizeof(cap-driver) - 1); strncpy(cap-card, VPE_MODULE_NAME, sizeof(cap-card) - 1); strlcpy(cap-bus_info, VPE_MODULE_NAME, sizeof(cap-bus_info)); - cap-device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING; + cap-device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING; cap-capabilities = cap-device_caps | V4L2_CAP_DEVICE_CAPS; return 0; } -- 1.8.3.2 -- 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 v3 14/14] v4l: ti-vpe: retain v4l2_buffer flags for captured buffers
The dequed CAPTURE_MPLANE type buffers don't contain the flags that the originally queued OUTPUT_MPLANE type buffers have. This breaks compliance. Copy the source v4l2_buffer flags to the destination v4l2_buffer flags before they are dequed. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index c884910..f7759e8 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1288,13 +1288,12 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) s_buf = s_vb-v4l2_buf; d_buf = d_vb-v4l2_buf; + d_buf-flags = s_buf-flags; + d_buf-timestamp = s_buf-timestamp; - d_buf-flags = ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK; - d_buf-flags |= s_buf-flags V4L2_BUF_FLAG_TSTAMP_SRC_MASK; - if (s_buf-flags V4L2_BUF_FLAG_TIMECODE) { - d_buf-flags |= V4L2_BUF_FLAG_TIMECODE; + if (s_buf-flags V4L2_BUF_FLAG_TIMECODE) d_buf-timecode = s_buf-timecode; - } + d_buf-sequence = ctx-sequence; d_q_data = ctx-q_data[Q_DATA_DST]; -- 1.8.3.2 -- 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 v3 08/14] v4l: ti-vpe: Rename csc memory resource name
Rename the memory block resource vpe_csc to csc since it also exists within the VIP IP block. This would make the name more generic, and both VPE and VIP DT nodes in the future can use it. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/csc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/csc.c b/drivers/media/platform/ti-vpe/csc.c index acfea50..039 100644 --- a/drivers/media/platform/ti-vpe/csc.c +++ b/drivers/media/platform/ti-vpe/csc.c @@ -180,7 +180,7 @@ struct csc_data *csc_create(struct platform_device *pdev) csc-pdev = pdev; csc-res = platform_get_resource_byname(pdev, IORESOURCE_MEM, - vpe_csc); + csc); if (csc-res == NULL) { dev_err(pdev-dev, missing platform resources data\n); return ERR_PTR(-ENODEV); -- 1.8.3.2 -- 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 v3 02/14] v4l: ti-vpe: register video device only when firmware is loaded
vpe fops(vpe_open in particular) should be called only when VPDMA firmware is loaded. File operations on the video device are possible the moment it is registered. Currently, we register the video device for VPE at driver probe, after calling a vpdma helper to initialize VPDMA and load firmware. This function is non-blocking(it calls request_firmware_nowait()), and doesn't ensure that the firmware is actually loaded when it returns. We remove the device registration from vpe probe, and move it to a callback provided by the vpe driver to the vpdma library, through vpdma_create(). The ready field in vpdma_data is no longer needed since we always have firmware loaded before the device is registered. A minor problem with this approach is that if the video_register_device fails(which doesn't really happen), the vpe platform device would be registered. however, there won't be any v4l2 device corresponding to it. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpdma.c | 8 +++-- drivers/media/platform/ti-vpe/vpdma.h | 7 +++-- drivers/media/platform/ti-vpe/vpe.c | 55 --- 3 files changed, 41 insertions(+), 29 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c index e8175e7..73dd38e 100644 --- a/drivers/media/platform/ti-vpe/vpdma.c +++ b/drivers/media/platform/ti-vpe/vpdma.c @@ -781,7 +781,7 @@ static void vpdma_firmware_cb(const struct firmware *f, void *context) /* already initialized */ if (read_field_reg(vpdma, VPDMA_LIST_ATTR, VPDMA_LIST_RDY_MASK, VPDMA_LIST_RDY_SHFT)) { - vpdma-ready = true; + vpdma-cb(vpdma-pdev); return; } @@ -811,7 +811,7 @@ static void vpdma_firmware_cb(const struct firmware *f, void *context) goto free_buf; } - vpdma-ready = true; + vpdma-cb(vpdma-pdev); free_buf: vpdma_unmap_desc_buf(vpdma, fw_dma_buf); @@ -839,7 +839,8 @@ static int vpdma_load_firmware(struct vpdma_data *vpdma) return 0; } -struct vpdma_data *vpdma_create(struct platform_device *pdev) +struct vpdma_data *vpdma_create(struct platform_device *pdev, + void (*cb)(struct platform_device *pdev)) { struct resource *res; struct vpdma_data *vpdma; @@ -854,6 +855,7 @@ struct vpdma_data *vpdma_create(struct platform_device *pdev) } vpdma-pdev = pdev; + vpdma-cb = cb; res = platform_get_resource_byname(pdev, IORESOURCE_MEM, vpdma); if (res == NULL) { diff --git a/drivers/media/platform/ti-vpe/vpdma.h b/drivers/media/platform/ti-vpe/vpdma.h index cf40f11..bf5f8bb 100644 --- a/drivers/media/platform/ti-vpe/vpdma.h +++ b/drivers/media/platform/ti-vpe/vpdma.h @@ -35,8 +35,8 @@ struct vpdma_data { struct platform_device *pdev; - /* tells whether vpdma firmware is loaded or not */ - bool ready; + /* callback to VPE driver when the firmware is loaded */ + void (*cb)(struct platform_device *pdev); }; enum vpdma_data_format_type { @@ -208,6 +208,7 @@ void vpdma_set_frame_start_event(struct vpdma_data *vpdma, void vpdma_dump_regs(struct vpdma_data *vpdma); /* initialize vpdma, passed with VPE's platform device pointer */ -struct vpdma_data *vpdma_create(struct platform_device *pdev); +struct vpdma_data *vpdma_create(struct platform_device *pdev, + void (*cb)(struct platform_device *pdev)); #endif diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index f3143ac..f1eae67 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1817,11 +1817,6 @@ static int vpe_open(struct file *file) vpe_dbg(dev, vpe_open\n); - if (!dev-vpdma-ready) { - vpe_err(dev, vpdma firmware not loaded\n); - return -ENODEV; - } - ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); if (!ctx) return -ENOMEM; @@ -2039,10 +2034,40 @@ static void vpe_runtime_put(struct platform_device *pdev) WARN_ON(r 0 r != -ENOSYS); } +static void vpe_fw_cb(struct platform_device *pdev) +{ + struct vpe_dev *dev = platform_get_drvdata(pdev); + struct video_device *vfd; + int ret; + + vfd = dev-vfd; + *vfd = vpe_videodev; + vfd-lock = dev-dev_mutex; + vfd-v4l2_dev = dev-v4l2_dev; + + ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0); + if (ret) { + vpe_err(dev, Failed to register video device\n); + + vpe_set_clock_enable(dev, 0); + vpe_runtime_put(pdev); + pm_runtime_disable(pdev-dev); + v4l2_m2m_release(dev-m2m_dev); + vb2_dma_contig_cleanup_ctx(dev-alloc_ctx); + v4l2_device_unregister(dev-v4l2_dev); + + return; + } + +
[PATCH v3 06/14] v4l: ti-vpe: Fix some params in VPE data descriptors
Some parameters of the VPE descriptors were understood incorrectly. They are now fixed. The fixes are explained as follows: - When adding an inbound data descriptor to the VPDMA descriptor list, we intend to use c_rect as the cropped region fetched by VPDMA. Therefore, c_rect-width shouldn't be used to calculate the line stride, the original image width should be used for that. We add a 'width' argument which gives the buffer width in memory. - frame_width and frame_height describe the complete width and height of the client to which the channel is connected. If there are multiple channels fetching data and providing to the same client, the above 2 arguments should be the width and height of the region covered by all the channels. In the case where there is only one channel providing pixel data to the client (like in VPE), frame_width and frame_height should be the cropped width and cropped height respectively. The calculation of these params is done in the vpe driver now. - start_h and start_v is also used in the case of multiple channels to describe where each channel should start filling pixel data. We don't use this in VPE, and pass 0s to the vpdma_add_in_dtd() helper. - Some minor changes are made to the vpdma_add_out_dtd() helper. The c_rect param is used for specifying the 'composition' target, and 'width' is added to calculate the line stride. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpdma.c | 60 +++ drivers/media/platform/ti-vpe/vpdma.h | 10 +++--- drivers/media/platform/ti-vpe/vpe.c | 18 +++ 3 files changed, 64 insertions(+), 24 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c index 73dd38e..a51a013 100644 --- a/drivers/media/platform/ti-vpe/vpdma.c +++ b/drivers/media/platform/ti-vpe/vpdma.c @@ -614,8 +614,17 @@ static void dump_dtd(struct vpdma_dtd *dtd) /* * append an outbound data transfer descriptor to the given descriptor list, * this sets up a 'client to memory' VPDMA transfer for the given VPDMA channel + * + * @list: vpdma desc list to which we add this decriptor + * @width: width of the image in pixels in memory + * @c_rect: compose params of output image + * @fmt: vpdma data format of the buffer + * dma_addr: dma address as seen by VPDMA + * chan: VPDMA channel + * flags: VPDMA flags to configure some descriptor fileds */ -void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, +void vpdma_add_out_dtd(struct vpdma_desc_list *list, int width, + const struct v4l2_rect *c_rect, const struct vpdma_data_format *fmt, dma_addr_t dma_addr, enum vpdma_channel chan, u32 flags) { @@ -623,6 +632,7 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, int field = 0; int notify = 1; int channel, next_chan; + struct v4l2_rect rect = *c_rect; int depth = fmt-depth; int stride; struct vpdma_dtd *dtd; @@ -630,11 +640,15 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, channel = next_chan = chan_info[chan].num; if (fmt-type == VPDMA_DATA_FMT_TYPE_YUV - fmt-data_type == DATA_TYPE_C420) + fmt-data_type == DATA_TYPE_C420) { + rect.height = 1; + rect.top = 1; depth = 8; + } - stride = ALIGN((depth * c_rect-width) 3, VPDMA_STRIDE_ALIGN); - dma_addr += (c_rect-left * depth) 3; + stride = ALIGN((depth * width) 3, VPDMA_STRIDE_ALIGN); + + dma_addr += rect.top * stride + (rect.left * depth 3); dtd = list-next; WARN_ON((void *)(dtd + 1) (list-buf.addr + list-buf.size)); @@ -664,31 +678,48 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, /* * append an inbound data transfer descriptor to the given descriptor list, * this sets up a 'memory to client' VPDMA transfer for the given VPDMA channel + * + * @list: vpdma desc list to which we add this decriptor + * @width: width of the image in pixels in memory(not the cropped width) + * @c_rect: crop params of input image + * @fmt: vpdma data format of the buffer + * dma_addr: dma address as seen by VPDMA + * chan: VPDMA channel + * field: top or bottom field info of the input image + * flags: VPDMA flags to configure some descriptor fileds + * frame_width/height: the complete width/height of the image presented to the + * client (this makes sense when multiple channels are + * connected to the same client, forming a larger frame) + * start_h, start_v: position where the given channel starts providing pixel + * data to the client (makes sense when multiple channels + * contribute to the client) */ -void vpdma_add_in_dtd(struct
Re: [PATCH v2 27/48] v4l: Validate fields in the core code for subdev EDID ioctls
On Tue, Mar 11, 2014 at 12:15:38AM +0100, Laurent Pinchart wrote: The subdev EDID ioctls receive a pad field that must reference an existing pad and an EDID field that must point to a buffer. Validate both fields in the core code instead of duplicating validation in all drivers. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Sakari Ailus sakari.ai...@linux.intel.com -- 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
Re: [PATCH 36/47] adv7604: Make output format configurable through pad format operations
On 03/10/14 23:43, Laurent Pinchart wrote: Hi Hans, On Wednesday 12 February 2014 16:01:17 Hans Verkuil wrote: On 02/05/14 17:42, Laurent Pinchart wrote: Replace the dummy video format operations by pad format operations that configure the output format. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/i2c/adv7604.c | 243 ++- include/media/adv7604.h | 47 ++--- 2 files changed, 225 insertions(+), 65 deletions(-) snip diff --git a/include/media/adv7604.h b/include/media/adv7604.h index 22811d3..2cc8e16 100644 --- a/include/media/adv7604.h +++ b/include/media/adv7604.h @@ -32,16 +32,6 @@ enum adv7604_ain_sel { ADV7604_AIN9_4_5_6_SYNC_2_1 = 4, }; -/* Bus rotation and reordering (IO register 0x04, [7:5]) */ -enum adv7604_op_ch_sel { - ADV7604_OP_CH_SEL_GBR = 0, - ADV7604_OP_CH_SEL_GRB = 1, - ADV7604_OP_CH_SEL_BGR = 2, - ADV7604_OP_CH_SEL_RGB = 3, - ADV7604_OP_CH_SEL_BRG = 4, - ADV7604_OP_CH_SEL_RBG = 5, -}; - /* Input Color Space (IO register 0x02, [7:4]) */ enum adv7604_inp_color_space { ADV7604_INP_COLOR_SPACE_LIM_RGB = 0, @@ -55,29 +45,11 @@ enum adv7604_inp_color_space { ADV7604_INP_COLOR_SPACE_AUTO = 0xf, }; -/* Select output format (IO register 0x03, [7:0]) */ -enum adv7604_op_format_sel { - ADV7604_OP_FORMAT_SEL_SDR_ITU656_8 = 0x00, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_10 = 0x01, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_12_MODE0 = 0x02, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_12_MODE1 = 0x06, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_12_MODE2 = 0x0a, - ADV7604_OP_FORMAT_SEL_DDR_422_8 = 0x20, - ADV7604_OP_FORMAT_SEL_DDR_422_10 = 0x21, - ADV7604_OP_FORMAT_SEL_DDR_422_12_MODE0 = 0x22, - ADV7604_OP_FORMAT_SEL_DDR_422_12_MODE1 = 0x23, - ADV7604_OP_FORMAT_SEL_DDR_422_12_MODE2 = 0x24, - ADV7604_OP_FORMAT_SEL_SDR_444_24 = 0x40, - ADV7604_OP_FORMAT_SEL_SDR_444_30 = 0x41, - ADV7604_OP_FORMAT_SEL_SDR_444_36_MODE0 = 0x42, - ADV7604_OP_FORMAT_SEL_DDR_444_24 = 0x60, - ADV7604_OP_FORMAT_SEL_DDR_444_30 = 0x61, - ADV7604_OP_FORMAT_SEL_DDR_444_36 = 0x62, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_16 = 0x80, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_20 = 0x81, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_24_MODE0 = 0x82, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_24_MODE1 = 0x86, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_24_MODE2 = 0x8a, +/* Select output format (IO register 0x03, [4:2]) */ +enum adv7604_op_format_mode_sel { + ADV7604_OP_FORMAT_MODE0 = 0x00, + ADV7604_OP_FORMAT_MODE1 = 0x04, + ADV7604_OP_FORMAT_MODE2 = 0x08, }; enum adv7604_drive_strength { @@ -104,11 +76,8 @@ struct adv7604_platform_data { /* Analog input muxing mode */ enum adv7604_ain_sel ain_sel; - /* Bus rotation and reordering */ - enum adv7604_op_ch_sel op_ch_sel; I would keep this as part of the platform_data. This is typically used if things are wired up in a non-standard way and so is specific to the hardware. It is not something that will change from format to format. Right, some level of configuration is needed to account for non-standard wiring. However I'm not sure where that should be handled. With exotic wiring the format at the receiver will be different than the format output by the ADV7604. From a pure ADV7604 point of view, the output format doesn't depend on the wiring. I wonder whether this shouldn't be a link property instead of being a subdev property. There's of course the question of where to store that link property if it's not part of either subdev. Even if we decide that the wiring is a property of the source subdev, I don't think we should duplicate bus reordering code in all subdev drivers. This should thus be handled by the v4l2 core (either directly or as helper functions). There are two reasons why you might want to use op_ch_sel: one is to implement weird formats like RBG. Something like that would have to be controlled through mbus and pixel fourcc codes and not by hardcoding this register. The other is to compensate for a wiring problem: we have a card where two channels were accidentally swapped. You can either redo the board or just set this register. In this case this register is IMHO a property of this subdev. It needs to know about it, because if it ever needs to output RBG in the future then it needs to compensate for reordering for wiring issues. So you set this field if you have to compensate for wiring errors, making this part of the DT/platform_data. You do not set this field when you want to support special formats, that is done in the driver itself through fourcc codes (or could be done as this isn't implemented at the moment). Regards, Hans Other than this it all looks quite nice! Much more flexible than before. - - /* Select output format */ - enum adv7604_op_format_sel op_format_sel; + /* Select output format mode */ + enum
[PATCH v2] [media] s5p-mfc: add init buffer cmd to MFCV6
From: avnd kiran avnd.ki...@samsung.com Latest MFC v6 firmware requires tile mode and loop filter setting to be done as part of Init buffer command, in sync with v7. Since there are two versions of v6 firmware with different interfaces, it is differenciated using the version number read back from firmware which is a hexadecimal value based on the firmware date. Signed-off-by: avnd kiran avnd.ki...@samsung.com Signed-off-by: Arun Kumar K arun...@samsung.com --- Changes from v1 --- - Check for v6 firmware date for differenciating old and new firmware as per comments from Kamil and Sylwester. --- drivers/media/platform/s5p-mfc/regs-mfc-v6.h|1 + drivers/media/platform/s5p-mfc/regs-mfc-v7.h|2 -- drivers/media/platform/s5p-mfc/s5p_mfc_common.h |2 ++ drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c |8 +++--- drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 30 --- 5 files changed, 34 insertions(+), 9 deletions(-) diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h index 8d0b686..b47567c 100644 --- a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h @@ -132,6 +132,7 @@ #define S5P_FIMV_D_METADATA_BUFFER_ADDR_V6 0xf448 #define S5P_FIMV_D_METADATA_BUFFER_SIZE_V6 0xf44c #define S5P_FIMV_D_NUM_MV_V6 0xf478 +#define S5P_FIMV_D_INIT_BUFFER_OPTIONS_V6 0xf47c #define S5P_FIMV_D_CPB_BUFFER_ADDR_V6 0xf4b0 #define S5P_FIMV_D_CPB_BUFFER_SIZE_V6 0xf4b4 diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v7.h b/drivers/media/platform/s5p-mfc/regs-mfc-v7.h index ea5ec2a..82c96fa 100644 --- a/drivers/media/platform/s5p-mfc/regs-mfc-v7.h +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v7.h @@ -18,8 +18,6 @@ #define S5P_FIMV_CODEC_VP8_ENC_V7 25 /* Additional registers for v7 */ -#define S5P_FIMV_D_INIT_BUFFER_OPTIONS_V7 0xf47c - #define S5P_FIMV_E_SOURCE_FIRST_ADDR_V70xf9e0 #define S5P_FIMV_E_SOURCE_SECOND_ADDR_V7 0xf9e4 #define S5P_FIMV_E_SOURCE_THIRD_ADDR_V70xf9e8 diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h index 4d17df9..f5404a6 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h @@ -287,6 +287,7 @@ struct s5p_mfc_priv_buf { * @warn_start:hardware error code from which warnings start * @mfc_ops: ops structure holding HW operation function pointers * @mfc_cmds: cmd structure holding HW commands function pointers + * @ver: firmware sub version * */ struct s5p_mfc_dev { @@ -330,6 +331,7 @@ struct s5p_mfc_dev { int warn_start; struct s5p_mfc_hw_ops *mfc_ops; struct s5p_mfc_hw_cmds *mfc_cmds; + int ver; }; /** diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c index 2475a3c..ba1d302 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c @@ -240,7 +240,6 @@ static inline void s5p_mfc_clear_cmds(struct s5p_mfc_dev *dev) /* Initialize hardware */ int s5p_mfc_init_hw(struct s5p_mfc_dev *dev) { - unsigned int ver; int ret; mfc_debug_enter(); @@ -302,12 +301,13 @@ int s5p_mfc_init_hw(struct s5p_mfc_dev *dev) return -EIO; } if (IS_MFCV6_PLUS(dev)) - ver = mfc_read(dev, S5P_FIMV_FW_VERSION_V6); + dev-ver = mfc_read(dev, S5P_FIMV_FW_VERSION_V6); else - ver = mfc_read(dev, S5P_FIMV_FW_VERSION); + dev-ver = mfc_read(dev, S5P_FIMV_FW_VERSION); mfc_debug(2, MFC F/W version : %02xyy, %02xmm, %02xdd\n, - (ver 16) 0xFF, (ver 8) 0xFF, ver 0xFF); + (dev-ver 16) 0xFF, (dev-ver 8) 0xFF, + dev-ver 0xFF); s5p_mfc_clock_off(); mfc_debug_leave(); return 0; diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c index 90edb19..356cfe5 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c @@ -14,6 +14,7 @@ #undef DEBUG +#include linux/bcd.h #include linux/delay.h #include linux/mm.h #include linux/io.h @@ -1269,6 +1270,29 @@ static int s5p_mfc_set_enc_params_vp8(struct s5p_mfc_ctx *ctx) return 0; } +/* Check if newer v6 firmware with changed init buffer interface */ +static bool s5p_mfc_is_v6_new(struct s5p_mfc_dev *dev) +{ + unsigned long cur_fw, v6_new_fw; + unsigned int y, m, d; + + if (IS_MFCV7(dev)) + return false; + + y = bcd2bin((dev-ver 16) 0xFF) + 2000; + m = bcd2bin((dev-ver 8) 0xFF); + d = bcd2bin(dev-ver 0xFF); + +
Re: [REVIEW PATCH 1/3] v4l2-subdev.h: add g_tvnorms video op
Hi Hans, On Tuesday 11 March 2014 00:32:48 Hans Verkuil wrote: On 03/11/2014 12:23 AM, Guennadi Liakhovetski wrote: Hi Hans, Thanks for taking care about this problem. I'm not sure it would be ok for me to pull this specific patch via my tree, because it's for the V4L2 core, and the other 2 patches in this series depend on this one. It's OK by me to pull this through your tree. But anyway I've got a question to this patch: On Mon, 17 Feb 2014, Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com While there was already a g_tvnorms_output video op, it's counterpart for video capture was missing. Add it. This is necessary for generic bridge drivers like soc-camera to set the video_device tvnorms field correctly. Otherwise ENUMSTD cannot work. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- include/media/v4l2-subdev.h | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index d67210a..787d078 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -264,8 +264,11 @@ struct v4l2_mbus_frame_desc { g_std_output: get current standard for video OUTPUT devices. This is ignored by video input devices. - g_tvnorms_output: get v4l2_std_id with all standards supported by video - OUTPUT device. This is ignored by video input devices. + g_tvnorms: get v4l2_std_id with all standards supported by the video + CAPTURE device. This is ignored by video output devices. + + g_tvnorms_output: get v4l2_std_id with all standards supported by the video +OUTPUT device. This is ignored by video capture devices. Why do we need two separate operations with the same functionality - one for capture and one for output? Can we have subdevices, that need to implement both? Besides, what about these two core ops: int (*g_std)(struct v4l2_subdev *sd, v4l2_std_id *norm); int (*s_std)(struct v4l2_subdev *sd, v4l2_std_id norm); ? Seems like a slightly different approach is needed? Or am I missing anything? There are drivers (ivtv) that have both capture and output subdevices. Each can have its own standard. Such drivers use v4l2_device_call_all() to call the same op for all subdevs so any subdev that can handle that op gets called. So they use it to call the s_std op to change the capture standard and they call s_std_output to change the output standard. If you can't tell the difference between capture tvnorms and output tvnorms this becomes tricky to handle. Just keep the two separate and there is no confusion. Note that the video ops have the g/s_std_output ops. It's due to historical reasons that g/s_std ended up in the core ops. They probably should be moved to the video ops, but it's just not worth the effort. It's not such a big effort, I can easily cook up a patch. However, it looks like the s_std operation is implemented by the following subdev drivers that don't implement video ops: tuner-core tvaudio sony-btf-mpx vp27smpx cx18-gpio It probably doesn't make much sense to add video ops to all of those. -- Regards, Laurent Pinchart -- 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: [REVIEW PATCH 1/3] v4l2-subdev.h: add g_tvnorms video op
On 03/11/14 10:33, Laurent Pinchart wrote: Hi Hans, On Tuesday 11 March 2014 00:32:48 Hans Verkuil wrote: On 03/11/2014 12:23 AM, Guennadi Liakhovetski wrote: Hi Hans, Thanks for taking care about this problem. I'm not sure it would be ok for me to pull this specific patch via my tree, because it's for the V4L2 core, and the other 2 patches in this series depend on this one. It's OK by me to pull this through your tree. But anyway I've got a question to this patch: On Mon, 17 Feb 2014, Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com While there was already a g_tvnorms_output video op, it's counterpart for video capture was missing. Add it. This is necessary for generic bridge drivers like soc-camera to set the video_device tvnorms field correctly. Otherwise ENUMSTD cannot work. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- include/media/v4l2-subdev.h | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index d67210a..787d078 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -264,8 +264,11 @@ struct v4l2_mbus_frame_desc { g_std_output: get current standard for video OUTPUT devices. This is ignored by video input devices. - g_tvnorms_output: get v4l2_std_id with all standards supported by video - OUTPUT device. This is ignored by video input devices. + g_tvnorms: get v4l2_std_id with all standards supported by the video + CAPTURE device. This is ignored by video output devices. + + g_tvnorms_output: get v4l2_std_id with all standards supported by the video +OUTPUT device. This is ignored by video capture devices. Why do we need two separate operations with the same functionality - one for capture and one for output? Can we have subdevices, that need to implement both? Besides, what about these two core ops: int (*g_std)(struct v4l2_subdev *sd, v4l2_std_id *norm); int (*s_std)(struct v4l2_subdev *sd, v4l2_std_id norm); ? Seems like a slightly different approach is needed? Or am I missing anything? There are drivers (ivtv) that have both capture and output subdevices. Each can have its own standard. Such drivers use v4l2_device_call_all() to call the same op for all subdevs so any subdev that can handle that op gets called. So they use it to call the s_std op to change the capture standard and they call s_std_output to change the output standard. If you can't tell the difference between capture tvnorms and output tvnorms this becomes tricky to handle. Just keep the two separate and there is no confusion. Note that the video ops have the g/s_std_output ops. It's due to historical reasons that g/s_std ended up in the core ops. They probably should be moved to the video ops, but it's just not worth the effort. It's not such a big effort, I can easily cook up a patch. However, it looks like the s_std operation is implemented by the following subdev drivers that don't implement video ops: tuner-core tvaudio sony-btf-mpx vp27smpx cx18-gpio It probably doesn't make much sense to add video ops to all of those. Well, if you move it to the video ops, then you have to. This was one of the reasons I decided at the time to keep g/s_std in core since they are used by drivers that otherwise do not need video ops. But in hindsight I always thought is was a mistake and I was just a bit too lazy. I really don't think it's worth the effort to change it all, but I won't hold you back either :-) 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] em28xx: fix PCTV 290e LNA oops
Pointer to device state has been moved to different location during some change. PCTV 290e LNA function still uses old pointer, carried over FE priv, and it crash. Reported-by: Janne Kujanpää jik...@iki.fi Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/usb/em28xx/em28xx-dvb.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c index 6638394..9b3f033 100644 --- a/drivers/media/usb/em28xx/em28xx-dvb.c +++ b/drivers/media/usb/em28xx/em28xx-dvb.c @@ -717,7 +717,8 @@ static void pctv_520e_init(struct em28xx *dev) static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe) { struct dtv_frontend_properties *c = fe-dtv_property_cache; - struct em28xx *dev = fe-dvb-priv; + struct em28xx_i2c_bus *i2c_bus = fe-dvb-priv; + struct em28xx *dev = i2c_bus-dev; #ifdef CONFIG_GPIOLIB struct em28xx_dvb *dvb = dev-dvb; int ret; -- 1.8.5.3 -- 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] v4l: subdev: Move [gs]_std operation to video ops
The g_std and s_std operations are video-related, move them to the video ops where they belong. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/i2c/adv7180.c | 6 +- drivers/media/i2c/adv7183.c | 4 ++-- drivers/media/i2c/adv7842.c | 4 ++-- drivers/media/i2c/bt819.c | 2 +- drivers/media/i2c/cx25840/cx25840-core.c| 4 ++-- drivers/media/i2c/ks0127.c | 6 +- drivers/media/i2c/ml86v7667.c | 2 +- drivers/media/i2c/msp3400-driver.c | 2 +- drivers/media/i2c/saa6752hs.c | 2 +- drivers/media/i2c/saa7110.c | 2 +- drivers/media/i2c/saa7115.c | 2 +- drivers/media/i2c/saa717x.c | 2 +- drivers/media/i2c/saa7191.c | 2 +- drivers/media/i2c/soc_camera/tw9910.c | 4 ++-- drivers/media/i2c/sony-btf-mpx.c| 10 +- drivers/media/i2c/tvaudio.c | 6 +- drivers/media/i2c/tvp514x.c | 2 +- drivers/media/i2c/tvp5150.c | 2 +- drivers/media/i2c/tw2804.c | 2 +- drivers/media/i2c/tw9903.c | 2 +- drivers/media/i2c/tw9906.c | 2 +- drivers/media/i2c/vp27smpx.c| 6 +- drivers/media/i2c/vpx3220.c | 2 +- drivers/media/pci/bt8xx/bttv-driver.c | 2 +- drivers/media/pci/cx18/cx18-av-core.c | 2 +- drivers/media/pci/cx18/cx18-fileops.c | 2 +- drivers/media/pci/cx18/cx18-gpio.c | 6 +- drivers/media/pci/cx18/cx18-ioctl.c | 2 +- drivers/media/pci/cx23885/cx23885-video.c | 4 ++-- drivers/media/pci/cx88/cx88-core.c | 2 +- drivers/media/pci/ivtv/ivtv-fileops.c | 2 +- drivers/media/pci/ivtv/ivtv-ioctl.c | 2 +- drivers/media/pci/saa7134/saa7134-video.c | 4 ++-- drivers/media/pci/saa7146/mxb.c | 14 +++--- drivers/media/pci/sta2x11/sta2x11_vip.c | 4 ++-- drivers/media/pci/zoran/zoran_device.c | 2 +- drivers/media/pci/zoran/zoran_driver.c | 2 +- drivers/media/platform/blackfin/bfin_capture.c | 4 ++-- drivers/media/platform/davinci/vpfe_capture.c | 2 +- drivers/media/platform/davinci/vpif_capture.c | 2 +- drivers/media/platform/davinci/vpif_display.c | 2 +- drivers/media/platform/fsl-viu.c| 2 +- drivers/media/platform/soc_camera/soc_camera.c | 4 ++-- drivers/media/platform/timblogiw.c | 2 +- drivers/media/platform/vino.c | 6 +++--- drivers/media/usb/au0828/au0828-video.c | 4 ++-- drivers/media/usb/cx231xx/cx231xx-417.c | 2 +- drivers/media/usb/cx231xx/cx231xx-video.c | 6 +++--- drivers/media/usb/em28xx/em28xx-video.c | 4 ++-- drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 2 +- drivers/media/usb/stk1160/stk1160-v4l.c | 4 ++-- drivers/media/usb/tm6000/tm6000-cards.c | 2 +- drivers/media/usb/tm6000/tm6000-video.c | 2 +- drivers/media/usb/usbvision/usbvision-video.c | 2 +- drivers/media/v4l2-core/tuner-core.c| 6 +- drivers/staging/media/davinci_vpfe/vpfe_video.c | 2 +- drivers/staging/media/go7007/go7007-v4l2.c | 2 +- drivers/staging/media/go7007/s2250-board.c | 2 +- drivers/staging/media/go7007/saa7134-go7007.c | 4 include/media/v4l2-subdev.h | 6 +++--- 60 files changed, 107 insertions(+), 95 deletions(-) diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c index d7d99f1..10074a4 100644 --- a/drivers/media/i2c/adv7180.c +++ b/drivers/media/i2c/adv7180.c @@ -430,6 +430,7 @@ static int adv7180_g_mbus_config(struct v4l2_subdev *sd, } static const struct v4l2_subdev_video_ops adv7180_video_ops = { + .s_std = adv7180_s_std, .querystd = adv7180_querystd, .g_input_status = adv7180_g_input_status, .s_routing = adv7180_s_routing, @@ -440,12 +441,7 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops = { .g_mbus_config = adv7180_g_mbus_config, }; -static const struct v4l2_subdev_core_ops adv7180_core_ops = { - .s_std = adv7180_s_std, -}; - static const struct v4l2_subdev_ops adv7180_ops = { - .core = adv7180_core_ops, .video = adv7180_video_ops, }; diff --git a/drivers/media/i2c/adv7183.c b/drivers/media/i2c/adv7183.c index d45e0e3..df461b0 100644 --- a/drivers/media/i2c/adv7183.c +++ b/drivers/media/i2c/adv7183.c @@ -501,8 +501,6 @@ static const struct v4l2_ctrl_ops adv7183_ctrl_ops = { static const struct v4l2_subdev_core_ops adv7183_core_ops = { .log_status = adv7183_log_status, - .g_std = adv7183_g_std, - .s_std = adv7183_s_std,
Re: [PATCH v2 06/48] v4l: Add pad-level DV timings subdev operations
Reviewed-by: Hans Verkuil hans.verk...@cisco.com On 03/11/14 00:15, Laurent Pinchart wrote: Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- include/media/v4l2-subdev.h| 4 include/uapi/linux/videodev2.h | 10 -- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 1752530..2b5ec32 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -509,6 +509,10 @@ struct v4l2_subdev_pad_ops { struct v4l2_subdev_selection *sel); int (*get_edid)(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid); int (*set_edid)(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid); + int (*dv_timings_cap)(struct v4l2_subdev *sd, + struct v4l2_dv_timings_cap *cap); + int (*enum_dv_timings)(struct v4l2_subdev *sd, +struct v4l2_enum_dv_timings *timings); #ifdef CONFIG_MEDIA_CONTROLLER int (*link_validate)(struct v4l2_subdev *sd, struct media_link *link, struct v4l2_subdev_format *source_fmt, diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 17acba8..72fbbd4 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1103,12 +1103,15 @@ struct v4l2_dv_timings { /** struct v4l2_enum_dv_timings - DV timings enumeration * @index: enumeration index + * @pad: the pad number for which to enumerate timings (used with + * v4l-subdev nodes only) * @reserved:must be zeroed * @timings: the timings for the given index */ struct v4l2_enum_dv_timings { __u32 index; - __u32 reserved[3]; + __u32 pad; + __u32 reserved[2]; struct v4l2_dv_timings timings; }; @@ -1146,11 +1149,14 @@ struct v4l2_bt_timings_cap { /** struct v4l2_dv_timings_cap - DV timings capabilities * @type:the type of the timings (same as in struct v4l2_dv_timings) + * @pad: the pad number for which to query capabilities (used with + * v4l-subdev nodes only) * @bt: the BT656/1120 timings capabilities */ struct v4l2_dv_timings_cap { __u32 type; - __u32 reserved[3]; + __u32 pad; + __u32 reserved[2]; union { struct v4l2_bt_timings_cap bt; __u32 raw_data[32]; -- 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
[GIT PULL for 3.15] mem2mem patches
The following changes since commit f2d7313534072a5fe192e7cf46204b413acef479: [media] drx-d: add missing braces in drxd_hard.c:DRXD_init (2014-03-09 09:20:50 -0300) are available in the git repository at: git://linuxtv.org/kdebski/media_tree_2.git master for you to fetch changes up to 0dceda80c0cc903a491ec76264768dd2bc4faeda: mem2mem_testdev: improve field handling (2014-03-11 11:22:23 +0100) Hans Verkuil (7): mem2mem_testdev: use 40ms default transfer time. mem2mem_testdev: pick default format with try_fmt. mem2mem_testdev: set priv to 0 mem2mem_testdev: add USERPTR support. mem2mem_testdev: return pending buffers in stop_streaming() mem2mem_testdev: fix field, sequence and time copying mem2mem_testdev: improve field handling Joonyoung Shim (1): s5p-mfc: Replaced commas with semicolons. Seung-Woo Kim (1): s5p-mfc: remove meaningless memory bank assignment drivers/media/platform/mem2mem_testdev.c | 94 +++-- drivers/media/platform/s5p-mfc/s5p_mfc.c |8 +-- drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c |2 - 3 files changed, 75 insertions(+), 29 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] drx-j: use ber_count var
drivers/media/dvb-frontends/drx39xyj/drxj.c: In function 'ctrl_get_qam_sig_quality': drivers/media/dvb-frontends/drx39xyj/drxj.c:9468:6: warning: variable 'ber_cnt' set but not used [-Wunused-but-set-variable] u32 ber_cnt = 0; /* BER count */ ^ By reading the comment, it is said that BER should be calculated as: qam_pre_rs_ber = frac_times1e6( ber_cnt, rs_bit_cnt ); Also, it makes sense to take the mantissa into account, so fix the code to do what's commented. Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index 0c0e9f3b108f..41d4bfe66764 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -9583,7 +9583,7 @@ ctrl_get_qam_sig_quality(struct drx_demod_instance *demod) if (m (rs_bit_cnt (e + 1)) || (rs_bit_cnt e) == 0) qam_pre_rs_ber = 50 * rs_bit_cnt e; else - qam_pre_rs_ber = m; + qam_pre_rs_ber = ber_cnt; /* post RS BER = 100* (11.17 * FEC_OC_SNC_FAIL_COUNT__A) / */ /* (1504.0 * FEC_OC_SNC_FAIL_PERIOD__A) */ -- 1.8.5.3 -- 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 26/48] v4l: Add support for DV timings ioctls on subdev nodes
On 03/11/14 00:15, Laurent Pinchart wrote: Validate the pad field in the core code whenever specified. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- .../DocBook/media/v4l/vidioc-dv-timings-cap.xml| 27 +++ .../DocBook/media/v4l/vidioc-enum-dv-timings.xml | 30 +- drivers/media/v4l2-core/v4l2-subdev.c | 27 +++ include/uapi/linux/v4l2-subdev.h | 5 4 files changed, 77 insertions(+), 12 deletions(-) diff --git a/Documentation/DocBook/media/v4l/vidioc-dv-timings-cap.xml b/Documentation/DocBook/media/v4l/vidioc-dv-timings-cap.xml index cd7720d..28a8c1e 100644 --- a/Documentation/DocBook/media/v4l/vidioc-dv-timings-cap.xml +++ b/Documentation/DocBook/media/v4l/vidioc-dv-timings-cap.xml @@ -1,11 +1,12 @@ refentry id=vidioc-dv-timings-cap refmeta -refentrytitleioctl VIDIOC_DV_TIMINGS_CAP/refentrytitle +refentrytitleioctl VIDIOC_DV_TIMINGS_CAP, VIDIOC_SUBDEV_DV_TIMINGS_CAP/refentrytitle manvol; /refmeta refnamediv refnameVIDIOC_DV_TIMINGS_CAP/refname +refnameVIDIOC_SUBDEV_DV_TIMINGS_CAP/refname refpurposeThe capabilities of the Digital Video receiver/transmitter/refpurpose /refnamediv @@ -33,7 +34,7 @@ varlistentry termparameterrequest/parameter/term listitem - paraVIDIOC_DV_TIMINGS_CAP/para + paraVIDIOC_DV_TIMINGS_CAP, VIDIOC_SUBDEV_DV_TIMINGS_CAP/para /listitem /varlistentry varlistentry @@ -54,10 +55,19 @@ interface and may change in the future./para /note -paraTo query the capabilities of the DV receiver/transmitter applications can call -this ioctl and the driver will fill in the structure. Note that drivers may return +paraTo query the capabilities of the DV receiver/transmitter applications +can call the constantVIDIOC_DV_TIMINGS_CAP/constant ioctl on a video node +and the driver will fill in the structure. Note that drivers may return different values after switching the video input or output./para +paraWhen implemented by the driver DV capabilities of subdevices can be +queried by calling the constantVIDIOC_SUBDEV_DV_TIMINGS_CAP/constant ioctl +directly on a subdevice node. The capabilities are specific to inputs (for DV +receivers) or outputs (for DV transmitters), applications must specify the +desired pad number in the v4l2-dv-timings-cap; structfieldpad/structfield +field. Attempts to query capabilities on a pad that doesn't support them will +return an EINVAL;./para + table pgwide=1 frame=none id=v4l2-bt-timings-cap titlestruct structnamev4l2_bt_timings_cap/structname/title tgroup cols=3 @@ -127,7 +137,14 @@ different values after switching the video input or output./para /row row entry__u32/entry - entrystructfieldreserved/structfield[3]/entry + entrystructfieldpad/structfield/entry + entryPad number as reported by the media controller API. This field + is only used when operating on a subdevice node. When operating on a + video node applications must set this field to zero./entry + /row + row + entry__u32/entry + entrystructfieldreserved/structfield[2]/entry entryReserved for future extensions. Drivers must set the array to zero./entry /row row diff --git a/Documentation/DocBook/media/v4l/vidioc-enum-dv-timings.xml b/Documentation/DocBook/media/v4l/vidioc-enum-dv-timings.xml index b3e17c1..b9fdfea 100644 --- a/Documentation/DocBook/media/v4l/vidioc-enum-dv-timings.xml +++ b/Documentation/DocBook/media/v4l/vidioc-enum-dv-timings.xml @@ -1,11 +1,12 @@ refentry id=vidioc-enum-dv-timings refmeta -refentrytitleioctl VIDIOC_ENUM_DV_TIMINGS/refentrytitle +refentrytitleioctl VIDIOC_ENUM_DV_TIMINGS, VIDIOC_SUBDEV_ENUM_DV_TIMINGS/refentrytitle manvol; /refmeta refnamediv refnameVIDIOC_ENUM_DV_TIMINGS/refname +refnameVIDIOC_SUBDEV_ENUM_DV_TIMINGS/refname refpurposeEnumerate supported Digital Video timings/refpurpose /refnamediv @@ -33,7 +34,7 @@ varlistentry termparameterrequest/parameter/term listitem - paraVIDIOC_ENUM_DV_TIMINGS/para + paraVIDIOC_ENUM_DV_TIMINGS, VIDIOC_SUBDEV_ENUM_DV_TIMINGS/para /listitem /varlistentry varlistentry @@ -61,14 +62,21 @@ standards or even custom timings that are not in this list./para paraTo query the available timings, applications initialize the structfieldindex/structfield field and zero the reserved array of v4l2-enum-dv-timings; -and call the constantVIDIOC_ENUM_DV_TIMINGS/constant ioctl with a pointer to this -structure. Drivers fill the rest of the structure or return an +and call the constantVIDIOC_ENUM_DV_TIMINGS/constant ioctl on a video node
[PATCH] drx-j: Fix post-BER calculus on QAM modulation
There are two troubles there: 1) the bit error measure were not accumulating; 2) it was missing the bit count. Fix them. Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index 41d4bfe66764..b8c5a851c29e 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -9620,7 +9620,8 @@ ctrl_get_qam_sig_quality(struct drx_demod_instance *demod) p-pre_bit_count.stat[0].uvalue += rs_bit_cnt e; } - p-post_bit_error.stat[0].uvalue = qam_post_rs_ber; + p-post_bit_error.stat[0].uvalue += qam_post_rs_ber; + p-post_bit_count.stat[0].uvalue += rs_bit_cnt e; p-block_error.stat[0].uvalue += pkt_errs; -- 1.8.5.3 -- 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 27/48] v4l: Validate fields in the core code for subdev EDID ioctls
On 03/11/14 00:15, Laurent Pinchart wrote: The subdev EDID ioctls receive a pad field that must reference an existing pad and an EDID field that must point to a buffer. Validate both fields in the core code instead of duplicating validation in all drivers. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/i2c/ad9389b.c | 2 -- drivers/media/i2c/adv7511.c | 2 -- drivers/media/i2c/adv7604.c | 4 drivers/media/i2c/adv7842.c | 4 drivers/media/v4l2-core/v4l2-subdev.c | 24 5 files changed, 20 insertions(+), 16 deletions(-) diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c index 4cdff9e..5b78828 100644 --- a/drivers/media/i2c/ad9389b.c +++ b/drivers/media/i2c/ad9389b.c @@ -683,8 +683,6 @@ static int ad9389b_get_edid(struct v4l2_subdev *sd, return -EINVAL; if (edid-blocks == 0 || edid-blocks 256) return -EINVAL; - if (!edid-edid) - return -EINVAL; if (!state-edid.segments) { v4l2_dbg(1, debug, sd, EDID segment 0 not found\n); return -ENODATA; diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c index de7ddf5..ff1c2cd 100644 --- a/drivers/media/i2c/adv7511.c +++ b/drivers/media/i2c/adv7511.c @@ -784,8 +784,6 @@ static int adv7511_get_edid(struct v4l2_subdev *sd, return -EINVAL; if ((edid-blocks == 0) || (edid-blocks 256)) return -EINVAL; - if (!edid-edid) - return -EINVAL; if (!state-edid.segments) { v4l2_dbg(1, debug, sd, EDID segment 0 not found\n); return -ENODATA; diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index 71c8570..de3db42 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -1673,8 +1673,6 @@ static int adv7604_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi return -EINVAL; if (edid-start_block == 1) edid-blocks = 1; - if (!edid-edid) - return -EINVAL; if (edid-blocks state-edid.blocks) edid-blocks = state-edid.blocks; @@ -1761,8 +1759,6 @@ static int adv7604_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi edid-blocks = 2; return -E2BIG; } - if (!edid-edid) - return -EINVAL; v4l2_dbg(2, debug, sd, %s: write EDID pad %d, edid.present = 0x%x\n, __func__, edid-pad, state-edid.present); diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c index 7fd9325..33558c8 100644 --- a/drivers/media/i2c/adv7842.c +++ b/drivers/media/i2c/adv7842.c @@ -2035,8 +2035,6 @@ static int adv7842_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi return -EINVAL; if (edid-start_block == 1) edid-blocks = 1; - if (!edid-edid) - return -EINVAL; switch (edid-pad) { case ADV7842_EDID_PORT_A: @@ -2071,8 +2069,6 @@ static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *e) return -EINVAL; if (e-blocks 2) return -E2BIG; - if (!e-edid) - return -EINVAL; /* todo, per edid */ state-aspect_ratio = v4l2_calc_aspect_ratio(e-edid[0x15], diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 853fb84..9fff1eb 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -349,11 +349,27 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) sd, pad, set_selection, subdev_fh, sel); } - case VIDIOC_SUBDEV_G_EDID: - return v4l2_subdev_call(sd, pad, get_edid, arg); + case VIDIOC_SUBDEV_G_EDID: { + struct v4l2_subdev_edid *edid = arg; - case VIDIOC_SUBDEV_S_EDID: - return v4l2_subdev_call(sd, pad, set_edid, arg); + if (edid-pad = sd-entity.num_pads) + return -EINVAL; + if (edid-edid == NULL) + return -EINVAL; + + return v4l2_subdev_call(sd, pad, get_edid, edid); + } + + case VIDIOC_SUBDEV_S_EDID: { + struct v4l2_subdev_edid *edid = arg; + + if (edid-pad = sd-entity.num_pads) + return -EINVAL; + if (edid-edid == NULL) + return -EINVAL; If edid-blocks == 0, then edid-edid may be NULL. So this should read: if (edid-blocks edid-edid == NULL) This is true for both G and S_EDID ioctls. Regards, Hans + + return v4l2_subdev_call(sd, pad, set_edid, edid); + } case VIDIOC_SUBDEV_DV_TIMINGS_CAP: { struct
[linuxtv-media:master 499/499] drivers/media/dvb-frontends/drx39xyj/drx39xxj.h:22:0: error: unterminated #ifndef
tree: git://linuxtv.org/media_tree.git master head: 164e5cfb7d37e4826a8337029716f4885657d859 commit: 164e5cfb7d37e4826a8337029716f4885657d859 [499/499] [media] drx39xxj.h: Fix undefined reference to attach function config: make ARCH=m68k allmodconfig All error/warnings: In file included from drivers/media/usb/em28xx/em28xx-dvb.c:44:0: drivers/media/dvb-frontends/drx39xyj/drx39xxj.h:22:0: error: unterminated #ifndef vim +22 drivers/media/dvb-frontends/drx39xyj/drx39xxj.h 38b2df95 Devin Heitmueller 2012-08-13 16 * 38b2df95 Devin Heitmueller 2012-08-13 17 * You should have received a copy of the GNU General Public License 38b2df95 Devin Heitmueller 2012-08-13 18 * along with this program; if not, write to the Free Software 38b2df95 Devin Heitmueller 2012-08-13 19 * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.= 38b2df95 Devin Heitmueller 2012-08-13 20 */ 38b2df95 Devin Heitmueller 2012-08-13 21 38b2df95 Devin Heitmueller 2012-08-13 @22 #ifndef DRX39XXJ_H 38b2df95 Devin Heitmueller 2012-08-13 23 #define DRX39XXJ_H 38b2df95 Devin Heitmueller 2012-08-13 24 38b2df95 Devin Heitmueller 2012-08-13 25 #include linux/dvb/frontend.h :: The code at line 22 was first introduced by commit :: 38b2df95c53be4bd5421d933ca0dabbcb82741d0 [media] drx-j: add a driver for Trident drx-j frontend :: TO: Devin Heitmueller dheitmuel...@kernellabs.com :: CC: Mauro Carvalho Chehab m.che...@samsung.com --- 0-DAY kernel build testing backend Open Source Technology Center http://lists.01.org/mailman/listinfo/kbuild Intel Corporation -- 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 27/48] v4l: Validate fields in the core code for subdev EDID ioctls
Hi Hans, On Tuesday 11 March 2014 11:45:09 Hans Verkuil wrote: On 03/11/14 00:15, Laurent Pinchart wrote: The subdev EDID ioctls receive a pad field that must reference an existing pad and an EDID field that must point to a buffer. Validate both fields in the core code instead of duplicating validation in all drivers. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/i2c/ad9389b.c | 2 -- drivers/media/i2c/adv7511.c | 2 -- drivers/media/i2c/adv7604.c | 4 drivers/media/i2c/adv7842.c | 4 drivers/media/v4l2-core/v4l2-subdev.c | 24 5 files changed, 20 insertions(+), 16 deletions(-) [snip] diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 853fb84..9fff1eb 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -349,11 +349,27 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) sd, pad, set_selection, subdev_fh, sel); } - case VIDIOC_SUBDEV_G_EDID: - return v4l2_subdev_call(sd, pad, get_edid, arg); + case VIDIOC_SUBDEV_G_EDID: { + struct v4l2_subdev_edid *edid = arg; - case VIDIOC_SUBDEV_S_EDID: - return v4l2_subdev_call(sd, pad, set_edid, arg); + if (edid-pad = sd-entity.num_pads) + return -EINVAL; + if (edid-edid == NULL) + return -EINVAL; + + return v4l2_subdev_call(sd, pad, get_edid, edid); + } + + case VIDIOC_SUBDEV_S_EDID: { + struct v4l2_subdev_edid *edid = arg; + + if (edid-pad = sd-entity.num_pads) + return -EINVAL; + if (edid-edid == NULL) + return -EINVAL; If edid-blocks == 0, then edid-edid may be NULL. So this should read: if (edid-blocks edid-edid == NULL) OK, I'll fix that. This is true for both G and S_EDID ioctls. What's the point of G_EDID with blocks == 0 ? Testing whether the ioctl is supported ? + + return v4l2_subdev_call(sd, pad, set_edid, edid); + } case VIDIOC_SUBDEV_DV_TIMINGS_CAP: { struct v4l2_dv_timings_cap *cap = arg; -- Regards, Laurent Pinchart -- 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 27/48] v4l: Validate fields in the core code for subdev EDID ioctls
On 03/11/14 11:57, Laurent Pinchart wrote: Hi Hans, On Tuesday 11 March 2014 11:45:09 Hans Verkuil wrote: On 03/11/14 00:15, Laurent Pinchart wrote: The subdev EDID ioctls receive a pad field that must reference an existing pad and an EDID field that must point to a buffer. Validate both fields in the core code instead of duplicating validation in all drivers. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/i2c/ad9389b.c | 2 -- drivers/media/i2c/adv7511.c | 2 -- drivers/media/i2c/adv7604.c | 4 drivers/media/i2c/adv7842.c | 4 drivers/media/v4l2-core/v4l2-subdev.c | 24 5 files changed, 20 insertions(+), 16 deletions(-) [snip] diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 853fb84..9fff1eb 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -349,11 +349,27 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) sd, pad, set_selection, subdev_fh, sel); } - case VIDIOC_SUBDEV_G_EDID: - return v4l2_subdev_call(sd, pad, get_edid, arg); + case VIDIOC_SUBDEV_G_EDID: { + struct v4l2_subdev_edid *edid = arg; - case VIDIOC_SUBDEV_S_EDID: - return v4l2_subdev_call(sd, pad, set_edid, arg); + if (edid-pad = sd-entity.num_pads) + return -EINVAL; + if (edid-edid == NULL) + return -EINVAL; + + return v4l2_subdev_call(sd, pad, get_edid, edid); + } + + case VIDIOC_SUBDEV_S_EDID: { + struct v4l2_subdev_edid *edid = arg; + + if (edid-pad = sd-entity.num_pads) + return -EINVAL; + if (edid-edid == NULL) + return -EINVAL; If edid-blocks == 0, then edid-edid may be NULL. So this should read: if (edid-blocks edid-edid == NULL) OK, I'll fix that. This is true for both G and S_EDID ioctls. What's the point of G_EDID with blocks == 0 ? Testing whether the ioctl is supported ? Now that you mention it, yes, that would be a good use :-) But I was thinking that you can call it once with blocks == 0, then the driver will fill in the real number of blocks and you can use that to size the edid array correctly. 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 26/48] v4l: Add support for DV timings ioctls on subdev nodes
Hi Hans, On Tuesday 11 March 2014 11:38:39 Hans Verkuil wrote: On 03/11/14 00:15, Laurent Pinchart wrote: Validate the pad field in the core code whenever specified. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- .../DocBook/media/v4l/vidioc-dv-timings-cap.xml| 27 + .../DocBook/media/v4l/vidioc-enum-dv-timings.xml | 30 + drivers/media/v4l2-core/v4l2-subdev.c | 27 + include/uapi/linux/v4l2-subdev.h | 5 4 files changed, 77 insertions(+), 12 deletions(-) [snip] diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h index 9fe3493..6f5c5de 100644 --- a/include/uapi/linux/v4l2-subdev.h +++ b/include/uapi/linux/v4l2-subdev.h @@ -169,5 +169,10 @@ struct v4l2_subdev_edid { #define VIDIOC_SUBDEV_S_SELECTION _IOWR('V', 62, struct v4l2_subdev_selection) #define VIDIOC_SUBDEV_G_EDID _IOWR('V', 40, struct v4l2_subdev_edid) #define VIDIOC_SUBDEV_S_EDID _IOWR('V', 41, struct v4l2_subdev_edid) +#define VIDIOC_SUBDEV_DV_TIMINGS_CAP _IOWR('V', 42, struct v4l2_dv_timings_cap) +#define VIDIOC_SUBDEV_ENUM_DV_TIMINGS _IOWR('V', 43, struct v4l2_enum_dv_timings) +#define VIDIOC_SUBDEV_QUERY_DV_TIMINGS _IOR('V', 44, struct v4l2_dv_timings) +#define VIDIOC_SUBDEV_G_DV_TIMINGS _IOWR('V', 45, struct v4l2_dv_timings) +#define VIDIOC_SUBDEV_S_DV_TIMINGS _IOWR('V', 46, struct v4l2_dv_timings) Is there a reason for not using the same ioctls numbers as in videodev2.h? The advantage is that the core compat32 support will automatically work (it doesn't have to do anything yet, but that might change in the future). Unless there is a really good reason I would keep them the same, just as we did with the EDID ioctls. I'll fix that. -- Regards, Laurent Pinchart -- 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
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue Mar 11 11:47:29 CET 2014 git branch: test git hash: 0d49e7761173520ff02cec6f11d581f8ebca764d gcc version:i686-linux-gcc (GCC) 4.8.2 sparse version: 0.4.5-rc1 host hardware: x86_64 host os:3.13-5.slh.4-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: WARNINGS 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: 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.31.14-i686: ERRORS linux-2.6.32.27-i686: ERRORS linux-2.6.33.7-i686: ERRORS linux-2.6.34.7-i686: ERRORS linux-2.6.35.9-i686: ERRORS linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: ERRORS linux-3.0.60-i686: ERRORS linux-3.1.10-i686: ERRORS linux-3.2.37-i686: ERRORS linux-3.3.8-i686: ERRORS linux-3.4.27-i686: ERRORS linux-3.5.7-i686: ERRORS linux-3.6.11-i686: ERRORS linux-3.7.4-i686: ERRORS linux-3.8-i686: ERRORS linux-3.9.2-i686: ERRORS linux-3.10.1-i686: ERRORS linux-3.11.1-i686: ERRORS linux-3.12-i686: ERRORS linux-3.13-i686: ERRORS linux-3.14-rc1-i686: ERRORS linux-2.6.31.14-x86_64: ERRORS linux-2.6.32.27-x86_64: ERRORS linux-2.6.33.7-x86_64: ERRORS linux-2.6.34.7-x86_64: ERRORS linux-2.6.35.9-x86_64: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.37-x86_64: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.4-x86_64: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-x86_64: ERRORS linux-3.10.1-x86_64: ERRORS linux-3.11.1-x86_64: ERRORS linux-3.12-x86_64: ERRORS linux-3.13-x86_64: ERRORS linux-3.14-rc1-x86_64: ERRORS apps: OK spec-git: OK sparse version: 0.4.5-rc1 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/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: [linuxtv-media:master 499/499] drivers/media/dvb-frontends/drx39xyj/drx39xxj.h:22:0: error: unterminated #ifndef
Em Tue, 11 Mar 2014 18:48:53 +0800 kbuild test robot fengguang...@intel.com escreveu: tree: git://linuxtv.org/media_tree.git master head: 164e5cfb7d37e4826a8337029716f4885657d859 commit: 164e5cfb7d37e4826a8337029716f4885657d859 [499/499] [media] drx39xxj.h: Fix undefined reference to attach function config: make ARCH=m68k allmodconfig Sorry, I had to rebase everything, due to some OF patches that caused some mess. I ended by merging a wrong version of this patch. Anyway, this was fixed already. 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 v6 4/8] of: Reduce indentation in of_graph_get_next_endpoint
Hi Laurent, Am Montag, den 10.03.2014, 20:19 +0100 schrieb Laurent Pinchart: On Friday 07 March 2014 18:40:54 Philipp Zabel wrote: While we look at of_graph_get_next_endpoint(), could you explain the reason behind the extra reference count increase on the prev node: /* * Avoid dropping prev node refcount to 0 when getting the next * child below. */ of_node_get(prev); This unfortunately makes using the function in for_each style macros a hassle. If that part wasn't there and all users that want to keep using prev after the call were expected to increase refcount themselves, we could have a #define of_graph_for_each_endpoint(parent, endpoint) \ for (endpoint = of_graph_get_next_endpoint(parent, NULL); \ endpoint != NULL; \ endpoint = of_graph_get_next_endpoint(parent, endpoint)) I don't know what the exact design decision was (Sylwester might know), but I suspect it's mostly about historical reasons. I see no reason that would prevent modifying the current behaviour to make a for-each loop easier to implement. Thanks, I'll include a patch to change this in the next round, then. regards Philipp -- 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: [GIT PULL] Move device tree graph parsing helpers to drivers/of
Em Mon, 10 Mar 2014 14:37:58 + Grant Likely grant.lik...@linaro.org escreveu: On Mon, 10 Mar 2014 10:26:30 -0300, Mauro Carvalho Chehab m.che...@samsung.com wrote: Em Fri, 07 Mar 2014 18:23:30 + Grant Likely grant.lik...@linaro.org escreveu: On Thu, 06 Mar 2014 18:13:20 +0100, Philipp Zabel p.za...@pengutronix.de wrote: Hi Mauro, Russell, I have temporarily removed the simplified bindings at Sylwester's request and updated the branch with the acks. The following changes since commit 0414855fdc4a40da05221fc6062cccbc0c30f169: Linux 3.14-rc5 (2014-03-02 18:56:16 -0800) are available in the git repository at: git://git.pengutronix.de/git/pza/linux.git topic/of-graph for you to fetch changes up to d484700a36952c6675aa47dec4d7a536929aa922: of: Warn if of_graph_parse_endpoint is called with the root node (2014-03-06 17:41:54 +0100) Nak. I made comments that haven't been resolved yet. I've replied with more detail tonight. The big issues are how drivers handle the optional 'ports' node and I do not agree to the double-linkage in the binding description. If I understood well, you're requesting to revert all those six patches that were imported via git pull from my tree (and from Greg and Russell), right? E. g. reverting those changesets: d484700a3695 f2a575f67695 4329b93b283c 6ff60d397b17 4d56ed5a009b fd9fdb78a9bf as it seems that there's no easy way to revert a git pull. All trees containing the branch would need to be reverted. I suspect that this will likely cause some harm when merging from our trees upstream. It means any tree containing that branch *must* be rewound. Grr... Done. I really preferred not doing that. From my side, it became too late to apply those OF changes on my tree for 3.15, as I will not prevent other patches to be applied on my tree due to that. 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 36/47] adv7604: Make output format configurable through pad format operations
Hi Hans, On Tuesday 11 March 2014 10:05:00 Hans Verkuil wrote: On 03/10/14 23:43, Laurent Pinchart wrote: On Wednesday 12 February 2014 16:01:17 Hans Verkuil wrote: On 02/05/14 17:42, Laurent Pinchart wrote: Replace the dummy video format operations by pad format operations that configure the output format. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/i2c/adv7604.c | 243 - include/media/adv7604.h | 47 ++--- 2 files changed, 225 insertions(+), 65 deletions(-) snip diff --git a/include/media/adv7604.h b/include/media/adv7604.h index 22811d3..2cc8e16 100644 --- a/include/media/adv7604.h +++ b/include/media/adv7604.h @@ -32,16 +32,6 @@ enum adv7604_ain_sel { ADV7604_AIN9_4_5_6_SYNC_2_1 = 4, }; -/* Bus rotation and reordering (IO register 0x04, [7:5]) */ -enum adv7604_op_ch_sel { - ADV7604_OP_CH_SEL_GBR = 0, - ADV7604_OP_CH_SEL_GRB = 1, - ADV7604_OP_CH_SEL_BGR = 2, - ADV7604_OP_CH_SEL_RGB = 3, - ADV7604_OP_CH_SEL_BRG = 4, - ADV7604_OP_CH_SEL_RBG = 5, -}; - /* Input Color Space (IO register 0x02, [7:4]) */ enum adv7604_inp_color_space { ADV7604_INP_COLOR_SPACE_LIM_RGB = 0, @@ -55,29 +45,11 @@ enum adv7604_inp_color_space { [snip] @@ -104,11 +76,8 @@ struct adv7604_platform_data { /* Analog input muxing mode */ enum adv7604_ain_sel ain_sel; - /* Bus rotation and reordering */ - enum adv7604_op_ch_sel op_ch_sel; I would keep this as part of the platform_data. This is typically used if things are wired up in a non-standard way and so is specific to the hardware. It is not something that will change from format to format. Right, some level of configuration is needed to account for non-standard wiring. However I'm not sure where that should be handled. With exotic wiring the format at the receiver will be different than the format output by the ADV7604. From a pure ADV7604 point of view, the output format doesn't depend on the wiring. I wonder whether this shouldn't be a link property instead of being a subdev property. There's of course the question of where to store that link property if it's not part of either subdev. Even if we decide that the wiring is a property of the source subdev, I don't think we should duplicate bus reordering code in all subdev drivers. This should thus be handled by the v4l2 core (either directly or as helper functions). There are two reasons why you might want to use op_ch_sel: one is to implement weird formats like RBG. Something like that would have to be controlled through mbus and pixel fourcc codes and not by hardcoding this register. Agreed. This patch only adds a subset of the possible output formats. More formats can be added later as needed. The other is to compensate for a wiring problem: we have a card where two channels were accidentally swapped. You can either redo the board or just set this register. In this case this register is IMHO a property of this subdev. It needs to know about it, because if it ever needs to output RBG in the future then it needs to compensate for reordering for wiring issues. So you set this field if you have to compensate for wiring errors, making this part of the DT/platform_data. You do not set this field when you want to support special formats, that is done in the driver itself through fourcc codes (or could be done as this isn't implemented at the moment). I agree with the use case, but I'm not sure whether that's the best way to support it. Let's say the system is design for RGB, but the G and B channels have been swapped by mistake on the board. To make it work, the ADV7604 would need to output RBG, and the bridge would receive RGB. Data reordering would thus happen on the link. There's two ways to expose this to userspace. One of them would be to make the real formats visible to applications. We would need to extend the link API to show that reordering occurs. Userspace would need to explicitly configure the adv7604 driver with RBG at its source pad and the bridge driver with RGB at its sink pad. This has the advantage of correctly modeling the hardware, and not pushing workarounds for board-level issues to individual drivers. Another solution would be to hide the wiring problem from userspace and handle it inside the adv7604 driver. Note that, depending on the hardware configuration, it could be the bridge driver that need to implement reordering. Userspace would configure both ends of the link to RGB and wouldn't be aware of the problem. This has the advantage of hiding the problem in the kernel. However, it would require implementing the same workaround in potentially many drivers. If we decide to go for the second solution I would like to make it a bit more generic than just keeping the op_ch_sel field in platform data. I think we should instead have a generic
RE: [PATCH] [media] s5p-mfc: Don't try to resubmit VP8 bitstream buffer for decode.
Hi Arun, From: Arun Kumar K [mailto:arunkk.sams...@gmail.com] On Behalf Of Arun Kumar K Sent: Friday, March 07, 2014 9:26 AM From: Pawel Osciak posc...@chromium.org Currently, for formats that are not H264, MFC driver will check the consumed stream size returned by the firmware and, based on that, will try to decide whether the bitstream buffer contained more than one frame. If the size of the buffer is larger than the consumed stream, it assumes that there are more frames in the buffer and that the buffer should be resubmitted for decode. This rarely works though and actually introduces problems, because: - v7 firmware will always return consumed stream size equal to whatever the driver passed to it when running decode (which is the size of the whole buffer), which means we will never try to resubmit, because the firmware will always tell us that it consumed all the data we passed to it; This does sound like a hardware bug/feature. So in v7 the buffer is never resubmitted, yes? And this patch makes no difference for v7? - v6 firmware will return the number of consumed bytes, but will not include the padding (stuffing) bytes that are allowed after the frame in VP8. Since there is no way of figuring out how many of those bytes follow the frame without getting the frame size from IVF headers (or somewhere else, but not from the stream itself), the driver tries to guess that padding size is not larger than 4 bytes, which is not always true; How about v5 of MFC? I need to do some additional testing, as I don't want to introduce any regressions. I remember that this check was a result of a fair amount of work and testing with v5. The only way to make it work is to queue only one frame per buffer from userspace and the check in the kernel is useless and wrong for VP8. MPEG4 still seems to require it, so keep it only for that format. Is your goal to give more than one frame in a single buffer and have the buffer resubmitted? Or the opposite - you are getting the frame resubmitted without the need? By the contents of this patch I guess the latter, on the other hand I do remember that at some point the idea was to be able to queue more than one frame per buffer. I don't remember exactly who was opting for the ability to queue more frames in a single buffer... Best wishes, -- Kamil Debski Samsung RD Institute Poland Signed-off-by: Pawel Osciak posc...@chromium.org Signed-off-by: Arun Kumar K arun...@samsung.com --- drivers/media/platform/s5p-mfc/s5p_mfc.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index e2aac59..66c1775 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -360,7 +360,7 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx *ctx, list); ctx-consumed_stream += s5p_mfc_hw_call(dev-mfc_ops, get_consumed_stream, dev); - if (ctx-codec_mode != S5P_MFC_CODEC_H264_DEC + if (ctx-codec_mode == S5P_MFC_CODEC_MPEG4_DEC ctx-consumed_stream + STUFF_BYTE src_buf-b-v4l2_planes[0].bytesused) { /* Run MFC again on the same buffer */ -- 1.7.9.5 -- 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 36/47] adv7604: Make output format configurable through pad format operations
Hi Laurent, On 03/11/14 12:16, Laurent Pinchart wrote: Hi Hans, On Tuesday 11 March 2014 10:05:00 Hans Verkuil wrote: On 03/10/14 23:43, Laurent Pinchart wrote: On Wednesday 12 February 2014 16:01:17 Hans Verkuil wrote: On 02/05/14 17:42, Laurent Pinchart wrote: Replace the dummy video format operations by pad format operations that configure the output format. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/i2c/adv7604.c | 243 - include/media/adv7604.h | 47 ++--- 2 files changed, 225 insertions(+), 65 deletions(-) snip diff --git a/include/media/adv7604.h b/include/media/adv7604.h index 22811d3..2cc8e16 100644 --- a/include/media/adv7604.h +++ b/include/media/adv7604.h @@ -32,16 +32,6 @@ enum adv7604_ain_sel { ADV7604_AIN9_4_5_6_SYNC_2_1 = 4, }; -/* Bus rotation and reordering (IO register 0x04, [7:5]) */ -enum adv7604_op_ch_sel { - ADV7604_OP_CH_SEL_GBR = 0, - ADV7604_OP_CH_SEL_GRB = 1, - ADV7604_OP_CH_SEL_BGR = 2, - ADV7604_OP_CH_SEL_RGB = 3, - ADV7604_OP_CH_SEL_BRG = 4, - ADV7604_OP_CH_SEL_RBG = 5, -}; - /* Input Color Space (IO register 0x02, [7:4]) */ enum adv7604_inp_color_space { ADV7604_INP_COLOR_SPACE_LIM_RGB = 0, @@ -55,29 +45,11 @@ enum adv7604_inp_color_space { [snip] @@ -104,11 +76,8 @@ struct adv7604_platform_data { /* Analog input muxing mode */ enum adv7604_ain_sel ain_sel; - /* Bus rotation and reordering */ - enum adv7604_op_ch_sel op_ch_sel; I would keep this as part of the platform_data. This is typically used if things are wired up in a non-standard way and so is specific to the hardware. It is not something that will change from format to format. Right, some level of configuration is needed to account for non-standard wiring. However I'm not sure where that should be handled. With exotic wiring the format at the receiver will be different than the format output by the ADV7604. From a pure ADV7604 point of view, the output format doesn't depend on the wiring. I wonder whether this shouldn't be a link property instead of being a subdev property. There's of course the question of where to store that link property if it's not part of either subdev. Even if we decide that the wiring is a property of the source subdev, I don't think we should duplicate bus reordering code in all subdev drivers. This should thus be handled by the v4l2 core (either directly or as helper functions). There are two reasons why you might want to use op_ch_sel: one is to implement weird formats like RBG. Something like that would have to be controlled through mbus and pixel fourcc codes and not by hardcoding this register. Agreed. This patch only adds a subset of the possible output formats. More formats can be added later as needed. The other is to compensate for a wiring problem: we have a card where two channels were accidentally swapped. You can either redo the board or just set this register. In this case this register is IMHO a property of this subdev. It needs to know about it, because if it ever needs to output RBG in the future then it needs to compensate for reordering for wiring issues. So you set this field if you have to compensate for wiring errors, making this part of the DT/platform_data. You do not set this field when you want to support special formats, that is done in the driver itself through fourcc codes (or could be done as this isn't implemented at the moment). I agree with the use case, but I'm not sure whether that's the best way to support it. Let's say the system is design for RGB, but the G and B channels have been swapped by mistake on the board. To make it work, the ADV7604 would need to output RBG, and the bridge would receive RGB. Data reordering would thus happen on the link. There's two ways to expose this to userspace. One of them would be to make the real formats visible to applications. We would need to extend the link API to show that reordering occurs. Userspace would need to explicitly configure the adv7604 driver with RBG at its source pad and the bridge driver with RGB at its sink pad. This has the advantage of correctly modeling the hardware, and not pushing workarounds for board-level issues to individual drivers. Another solution would be to hide the wiring problem from userspace and handle it inside the adv7604 driver. Note that, depending on the hardware configuration, it could be the bridge driver that need to implement reordering. Userspace would configure both ends of the link to RGB and wouldn't be aware of the problem. This has the advantage of hiding the problem in the kernel. However, it would require implementing the same workaround in potentially many drivers. If we decide to go for the second solution I would like to make it a bit more generic than just keeping the op_ch_sel field in platform data. I think we should
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
Hi Philipp, On Monday 10 March 2014 16:40:30 Philipp Zabel wrote: Am Montag, den 10.03.2014, 16:15 +0100 schrieb Laurent Pinchart: On Monday 10 March 2014 14:58:15 Grant Likely wrote: On Mon, 10 Mar 2014 14:52:53 +0100, Laurent Pinchart wrote: On Monday 10 March 2014 12:18:20 Tomi Valkeinen wrote: On 08/03/14 13:41, Grant Likely wrote: Ok. If we go for single directional link, the question is then: which way? And is the direction different for display and camera, which are kind of reflections of each other? In general I would recommend choosing whichever device you would sensibly think of as a master. In the camera case I would choose the camera controller node instead of the camera itself, and in the display case I would choose the display controller instead of the panel. The binding author needs to choose what she things makes the most sense, but drivers can still use if it it turns out to be 'backwards' I would perhaps choose the same approach, but at the same time I think it's all but clear. The display controller doesn't control the panel any more than a DMA controller controls, say, the display controller. In fact, in earlier versions of OMAP DSS DT support I had a simpler port description, and in that I had the panel as the master (i.e. link from panel to dispc) because the panel driver uses the display controller's features to provide the panel device a data stream. And even with the current OMAP DSS DT version, which uses the v4l2 style ports/endpoints, the driver model is still the same, and only links towards upstream are used. So one reason I'm happy with the dual-linking is that I can easily follow the links from the downstream entities to upstream entities, and other people, who have different driver model, can easily do the opposite. But I agree that single-linking is enough and this can be handled at runtime, even if it makes the code more complex. And perhaps requires extra data in the dts, to give the start points for the graph. In theory unidirectional links in DT are indeed enough. However, let's not forget the following. - There's no such thing as single start points for graphs. Sure, in some simple cases the graph will have a single start point, but that's not a generic rule. For instance the camera graphs http://ideasonboard.org/media/omap3isp.ps and http://ideasonboard.org/media/eyecam.ps have two camera sensors, and thus two starting points from a data flow point of view. And if you want a better understanding of how complex media graphs can become, have a look at http://ideasonboard.org/media/vsp1.0.pdf (that's a real world example, albeit all connections are internal to the SoC in that particular case, and don't need to be described in DT). - There's also no such thing as a master device that can just point to slave devices. Once again simple cases exist where that model could work, but real world examples exist of complex pipelines with dozens of elements all implemented by a separate IP core and handled by separate drivers, forming a graph with long chains and branches. We thus need real graph bindings. - Finally, having no backlinks in DT would make the software implementation very complex. We need to be able to walk the graph in a generic way without having any of the IP core drivers loaded, and without any specific starting point. We would thus need to parse the complete DT tree, looking at all nodes and trying to find out whether they're part of the graph we're trying to walk. The complexity of the operation would be at best quadratic to the number of nodes in the whole DT and to the number of nodes in the graph. Not really. To being with, you cannot determine any meaning of a node across the tree (aside from it being an endpoint) That's the important part. I can assume the target node of the remote-endpoint phandle to be an endpoint, and can thus assume that it implements the of-graph bindings. That's all I need to be able to walk the graph in a generic way. without also understanding the binding that the node is a part of. That means you need to have something matching against the compatible string on both ends of the linkage. For instance: panel { compatible = acme,lvds-panel; lvds-port: port { }; }; display-controller { compatible = encom,video; port { remote-endpoint = lvds-port; }; }; In the above example, the encom,video driver has absolutely zero information about what the acme,lvds-panel binding actually implements. There needs to be both a driver for the acme,lvds-panel binding and one for the encom,video binding (even if the
cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue Mar 11 12:12:39 CET 2014 git branch: test git hash: 0d49e7761173520ff02cec6f11d581f8ebca764d gcc version:i686-linux-gcc (GCC) 4.8.2 sparse version: 0.4.5-rc1 host hardware: x86_64 host os:3.13-5.slh.4-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: WARNINGS 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: 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.31.14-i686: WARNINGS 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-i686: OK linux-3.13-i686: OK linux-3.14-rc1-i686: OK linux-2.6.31.14-x86_64: WARNINGS 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-x86_64: OK linux-3.13-x86_64: OK linux-3.14-rc1-x86_64: OK apps: OK spec-git: OK sparse version: 0.4.5-rc1 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/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 v2 00/48] ADV7611 support
Hi Laurent, You can add my: Reviewed-by: Hans Verkuil hans.verk...@cisco.com for patches 28-35, 37-45 and 47. For patches 46 and 48 (DT related) add my: Acked-by: Hans Verkuil hans.verk...@cisco.com For patches 26 and 27 I posted review comments, and patch 36 is removing support for the op_ch_sel field which I really need on my system. So that needs to be resolved. I've tested the whole lot (with an ugly workaround for patch 36) on my hardware successfully. Regards, Hans On 03/11/14 00:15, Laurent Pinchart wrote: Hello, This patch set implements support for the ADV7611 in the adv7604 driver. It also comes up with new features such as output format configuration through pad format operations, hot-plug detect control through GPIO and DT support. Patches 06/48 to 24/48 and 39/48 replace the subdev video DV timings query cap and enum operations with pad-level equivalents. I've split driver changes in one patch per driver to make review easier, but I can squash them together if desired. I believe I've addressed all comments received on v1, except the one related to op_ch_sel in patch adv7604: Make output format configurable through pad format operations which is still open for discussion. Patches 02/48 to 05/48 have been acked in v1 already, I will send a pull request for them separately if a v3 of this series ends up being needed. I'd like to get patch 01/48 upstream soon as well. Changes compared to v1: - Check the edid and pad fields for various ioctls in the subdev core - Switch to the descriptor-based GPIO API - Leave enum adv7604_pad in header file - Keep the hotplug notifier - Fix compilation breakage when !CONFIG_OF due to directly dereferencing the return value of of_match_node() - Move patch v4l: subdev: Remove deprecated video-level DV timings operations later in the series to avoid bisection breakages - Document struct v4l2_enum_dv_timings reserved field as being set to 0 by both drivers and application - Document pad field of struct v4l2_enum_dv_timings and struct v4l2_dv_timings_cap as being used for subdev nodes only - Typo fixes in documentation Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Tomasz Stanislawski t.stanisl...@samsung.com Cc: Scott Jiang scott.jiang.li...@gmail.com Cc: Prabhakar Lad prabhakar.cse...@gmail.com Lars-Peter Clausen (4): adv7604: Add missing include to linux/types.h adv7604: Add support for asynchronous probing adv7604: Don't put info string arrays on the stack adv7604: Add adv7611 support Laurent Pinchart (44): v4l: of: Support empty port nodes v4l: Add UYVY10_2X10 and VYUY10_2X10 media bus pixel codes v4l: Add UYVY10_1X20 and VYUY10_1X20 media bus pixel codes v4l: Add 12-bit YUV 4:2:0 media bus pixel codes v4l: Add 12-bit YUV 4:2:2 media bus pixel codes v4l: Add pad-level DV timings subdev operations ad9389b: Add pad-level DV timings operations adv7511: Add pad-level DV timings operations adv7842: Add pad-level DV timings operations s5p-tv: hdmi: Add pad-level DV timings operations s5p-tv: hdmiphy: Add pad-level DV timings operations ths8200: Add pad-level DV timings operations tvp7002: Add pad-level DV timings operations media: bfin_capture: Switch to pad-level DV operations media: davinci: vpif: Switch to pad-level DV operations media: staging: davinci: vpfe: Switch to pad-level DV operations s5p-tv: mixer: Switch to pad-level DV operations ad9389b: Remove deprecated video-level DV timings operations adv7511: Remove deprecated video-level DV timings operations adv7842: Remove deprecated video-level DV timings operations s5p-tv: hdmi: Remove deprecated video-level DV timings operations s5p-tv: hdmiphy: Remove deprecated video-level DV timings operation ths8200: Remove deprecated video-level DV timings operations tvp7002: Remove deprecated video-level DV timings operations v4l: Improve readability by not wrapping ioctl number #define's v4l: Add support for DV timings ioctls on subdev nodes v4l: Validate fields in the core code for subdev EDID ioctls adv7604: Add 16-bit read functions for CP and HDMI adv7604: Cache register contents when reading multiple bits adv7604: Remove subdev control handlers adv7604: Add sink pads adv7604: Make output format configurable through pad format operations adv7604: Add pad-level DV timings support adv7604: Remove deprecated video-level DV timings operations v4l: subdev: Remove deprecated video-level DV timings operations adv7604: Inline the to_sd function adv7604: Store I2C addresses and clients in arrays adv7604: Replace *_and_or() functions with *_clr_set() adv7604: Sort headers alphabetically adv7604: Support hot-plug detect control through a GPIO adv7604: Specify the default input through platform data adv7604: Add DT support adv7604: Add LLC polarity configuration adv7604: Add endpoint properties to DT
Re: [PATCH v3 05/14] v4l: ti-vpe: Allow usage of smaller images
On 03/11/14 09:33, Archit Taneja wrote: The minimum width and height for VPE input/output was kept as 128 pixels. VPE doesn't have a constraint on the image height, it requires the image width to be at least 16 bytes. Change the minimum supported dimensions to 32x32. This allows us to de-interlace qcif content. A smaller image size than 32x32 didn't make much sense, so stopped at this. Signed-off-by: Archit Taneja arc...@ti.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 0e7573a..dbdc338 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -49,8 +49,8 @@ #define VPE_MODULE_NAME vpe /* minimum and maximum frame sizes */ -#define MIN_W128 -#define MIN_H128 +#define MIN_W32 +#define MIN_H32 #define MAX_W1920 #define MAX_H1080 -- 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 v3 04/14] v4l: ti-vpe: Allow DMABUF buffer type support
On 03/11/14 09:33, Archit Taneja wrote: For OMAP and DRA7x, we generally allocate video and graphics buffers through omapdrm since the corresponding omap-gem driver provides DMM-Tiler backed contiguous buffers. omapdrm is a dma-buf exporter. These buffers are used by other drivers in the video pipeline. Add VB2_DMABUF flag to the io_modes of the vb2 output and capture queues. This allows the driver to import dma shared buffers. Signed-off-by: Archit Taneja arc...@ti.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 0363df6..0e7573a 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1770,7 +1770,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(src_vq, 0, sizeof(*src_vq)); src_vq-type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; - src_vq-io_modes = VB2_MMAP; + src_vq-io_modes = VB2_MMAP | VB2_DMABUF; src_vq-drv_priv = ctx; src_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); src_vq-ops = vpe_qops; @@ -1783,7 +1783,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(dst_vq, 0, sizeof(*dst_vq)); dst_vq-type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; - dst_vq-io_modes = VB2_MMAP; + dst_vq-io_modes = VB2_MMAP | VB2_DMABUF; dst_vq-drv_priv = ctx; dst_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); dst_vq-ops = vpe_qops; -- 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 v3 03/14] v4l: ti-vpe: Use video_device_release_empty
On 03/11/14 09:33, Archit Taneja wrote: The video_device struct is currently embedded in the driver data struct vpe_dev. A vpe_dev instance is allocated by the driver, and the memory for the vfd is a part of this struct. The v4l2 core, however, manages the removal of the vfd region, through the video_device's .release() op, which currently is the helper video_device_release. This causes memory corruption, and leads to issues when we try to re-insert the vpe module. Use the video_device_release_empty helper function instead Signed-off-by: Archit Taneja arc...@ti.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/ti-vpe/vpe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index f1eae67..0363df6 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -2000,7 +2000,7 @@ static struct video_device vpe_videodev = { .fops = vpe_fops, .ioctl_ops = vpe_ioctl_ops, .minor = -1, - .release= video_device_release, + .release= video_device_release_empty, .vfl_dir= VFL_DIR_M2M, }; -- 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 01/48] v4l: of: Support empty port nodes
Hi Laurent, On 11/03/14 00:15, Laurent Pinchart wrote: Empty port nodes are allowed but currently unsupported as the v4l2_of_get_next_endpoint() function assumes that all port nodes have at least an endpoint. Fix this. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Sylwester Nawrocki s.nawro...@samsung.com --- drivers/media/v4l2-core/v4l2-of.c | 52 +-- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c index 42e3e8a..4c07343 100644 --- a/drivers/media/v4l2-core/v4l2-of.c +++ b/drivers/media/v4l2-core/v4l2-of.c @@ -166,43 +166,51 @@ struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent, struct device_node *prev) { struct device_node *endpoint; - struct device_node *port = NULL; + struct device_node *port; if (!parent) return NULL; + /* + * Start by locating the port node. If no previous endpoint is specified + * search for the first port node, otherwise get the previous endpoint + * parent port node. + */ if (!prev) { struct device_node *node; - /* - * It's the first call, we have to find a port subnode - * within this node or within an optional 'ports' node. - */ + node = of_get_child_by_name(parent, ports); if (node) parent = node; port = of_get_child_by_name(parent, port); + of_node_put(node); - if (port) { - /* Found a port, get an endpoint. */ - endpoint = of_get_next_child(port, NULL); - of_node_put(port); - } else { - endpoint = NULL; - } - - if (!endpoint) - pr_err(%s(): no endpoint nodes specified for %s\n, + if (!port) { + pr_err(%s(): no port node found in %s\n, __func__, parent-full_name); - of_node_put(node); + return NULL; + } } else { port = of_get_parent(prev); - if (!port) + if (!port) { /* Hm, has someone given us the root node ?... */ return NULL; + } - /* Avoid dropping prev node refcount to 0. */ + /* + * Avoid dropping prev node refcount to 0 when getting the next + * child below. + */ of_node_get(prev); + } + + while (1) { + /* + * Now that we have a port node, get the next endpoint by + * getting the next child. If the previous endpoint is NULL this + * will return the first child. + */ endpoint = of_get_next_child(port, prev); if (endpoint) { of_node_put(port); @@ -210,18 +218,14 @@ struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent, } /* No more endpoints under this port, try the next one. */ + prev = NULL; + do { port = of_get_next_child(parent, port); if (!port) return NULL; } while (of_node_cmp(port-name, port)); - - /* Pick up the first endpoint in this port. */ - endpoint = of_get_next_child(port, NULL); - of_node_put(port); } - - return endpoint; } EXPORT_SYMBOL(v4l2_of_get_next_endpoint); -- Regards, Sylwester -- 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 v3 07/14] v4l: ti-vpe: Add selection API in VPE driver
Hi Archit, A few small comments below... On 03/11/14 09:33, Archit Taneja wrote: Add selection ioctl ops. For VPE, cropping makes sense only for the input to VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers). For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP results in selecting a rectangle region within the source buffer. Setting the crop/compose rectangles should successfully result in re-configuration of registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 141 1 file changed, 141 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index ece9b96..4abb85c 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx, { switch (type) { case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: + case V4L2_BUF_TYPE_VIDEO_OUTPUT: return ctx-q_data[Q_DATA_SRC]; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: + case V4L2_BUF_TYPE_VIDEO_CAPTURE: return ctx-q_data[Q_DATA_DST]; default: BUG(); @@ -1587,6 +1589,142 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f) return set_srcdst_params(ctx); } +static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s) +{ + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + case V4L2_SEL_TGT_COMPOSE: + /* + * COMPOSE target is only valid for capture buffer type, for + * output buffer type, assign existing crop parameters to the + * selection rectangle + */ + if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) + break; Shouldn't this return -EINVAL? + + s-r = q_data-c_rect; + return 0; + + case V4L2_SEL_TGT_CROP: + /* + * CROP target is only valid for output buffer type, for capture + * buffer type, assign existing compose parameters to the + * selection rectangle + */ + if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) + break; Ditto. + + s-r = q_data-c_rect; + return 0; + + /* + * bound and default crop/compose targets are invalid targets to + * try/set + */ + default: + return -EINVAL; + } + + if (s-r.top 0 || s-r.left 0) { + vpe_err(ctx-dev, negative values for top and left\n); + s-r.top = s-r.left = 0; + } + + v4l_bound_align_image(s-r.width, MIN_W, q_data-width, 1, + s-r.height, MIN_H, q_data-height, H_ALIGN, S_ALIGN); + + /* adjust left/top if cropping rectangle is out of bounds */ + if (s-r.left + s-r.width q_data-width) + s-r.left = q_data-width - s-r.width; + if (s-r.top + s-r.height q_data-height) + s-r.top = q_data-height - s-r.height; + + return 0; +} + +static int vpe_g_selection(struct file *file, void *fh, + struct v4l2_selection *s) +{ + struct vpe_ctx *ctx = file2ctx(file); + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + /* return width and height from S_FMT of the respective buffer type */ + case V4L2_SEL_TGT_COMPOSE_DEFAULT: + case V4L2_SEL_TGT_COMPOSE_BOUNDS: + case V4L2_SEL_TGT_CROP_BOUNDS: + case V4L2_SEL_TGT_CROP_DEFAULT: + s-r.left = 0; + s-r.top = 0; + s-r.width = q_data-width; + s-r.height = q_data-height; The crop targets only make sense for type OUTPUT and the compose only for type CAPTURE. Add some checks for that. + return 0; + + /* + * CROP target holds for the output buffer type, and COMPOSE target + * holds for the capture buffer type. We still return the c_rect params + * for both the target types. + */ + case V4L2_SEL_TGT_COMPOSE: + case V4L2_SEL_TGT_CROP: + s-r.left =
Re: [PATCH v3 09/14] v4l: ti-vpe: report correct capabilities in querycap
On 03/11/14 09:33, Archit Taneja wrote: querycap currently returns V4L2_CAP_VIDEO_M2M as a capability, this should be V4L2_CAP_VIDEO_M2M_MPLANE instead, as the driver supports multiplanar formats. Signed-off-by: Archit Taneja arc...@ti.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/ti-vpe/vpe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 4abb85c..46b9d44 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1347,7 +1347,7 @@ static int vpe_querycap(struct file *file, void *priv, strncpy(cap-driver, VPE_MODULE_NAME, sizeof(cap-driver) - 1); strncpy(cap-card, VPE_MODULE_NAME, sizeof(cap-card) - 1); strlcpy(cap-bus_info, VPE_MODULE_NAME, sizeof(cap-bus_info)); - cap-device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING; + cap-device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING; cap-capabilities = cap-device_caps | V4L2_CAP_DEVICE_CAPS; return 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
Re: [PATCH v3 10/14] v4l: ti-vpe: Use correct bus_info name for the device in querycap
On 03/11/14 09:33, Archit Taneja wrote: The bus_info parameter in v4l2_capabilities expects a 'platform_' prefix. This wasn't done in the driver and hence was breaking compliance. Update the bus_info parameter accordingly. Signed-off-by: Archit Taneja arc...@ti.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/ti-vpe/vpe.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 46b9d44..5591d04 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1346,7 +1346,8 @@ static int vpe_querycap(struct file *file, void *priv, { strncpy(cap-driver, VPE_MODULE_NAME, sizeof(cap-driver) - 1); strncpy(cap-card, VPE_MODULE_NAME, sizeof(cap-card) - 1); - strlcpy(cap-bus_info, VPE_MODULE_NAME, sizeof(cap-bus_info)); + snprintf(cap-bus_info, sizeof(cap-bus_info), platform:%s, + VPE_MODULE_NAME); cap-device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING; cap-capabilities = cap-device_caps | V4L2_CAP_DEVICE_CAPS; return 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
Re: [PATCH v3 11/14] v4l: ti-vpe: Fix initial configuration queue data
On 03/11/14 09:33, Archit Taneja wrote: The vpe output and capture queues are initially configured to default values in vpe_open(). A G_FMT before any S_FMTs will result in these values being populated. The colorspace and bytesperline parameter of this initial configuration are incorrect. This breaks compliance when as we get 'TRY_FMT(G_FMT) != G_FMT'. Fix the initial queue configuration such that it wouldn't need to be fixed by try_fmt. Signed-off-by: Archit Taneja arc...@ti.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/ti-vpe/vpe.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 5591d04..85d1122 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -2012,9 +2012,11 @@ static int vpe_open(struct file *file) s_q_data-fmt = vpe_formats[2]; s_q_data-width = 1920; s_q_data-height = 1080; - s_q_data-sizeimage[VPE_LUMA] = (s_q_data-width * s_q_data-height * + s_q_data-bytesperline[VPE_LUMA] = (s_q_data-width * s_q_data-fmt-vpdma_fmt[VPE_LUMA]-depth) 3; - s_q_data-colorspace = V4L2_COLORSPACE_SMPTE170M; + s_q_data-sizeimage[VPE_LUMA] = (s_q_data-bytesperline[VPE_LUMA] * + s_q_data-height); + s_q_data-colorspace = V4L2_COLORSPACE_REC709; s_q_data-field = V4L2_FIELD_NONE; s_q_data-c_rect.left = 0; s_q_data-c_rect.top = 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
Re: [PATCH v3 12/14] v4l: ti-vpe: zero out reserved fields in try_fmt
On 03/11/14 09:33, Archit Taneja wrote: Zero out the reserved formats in v4l2_pix_format_mplane and v4l2_plane_pix_format members of the returned v4l2_format pointer when passed through TRY_FMT ioctl. This ensures that the user doesn't interpret the non-zero fields as some data passed by the driver, and ensures compliance. Signed-off-by: Archit Taneja arc...@ti.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/ti-vpe/vpe.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 85d1122..970408a 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1488,6 +1488,7 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, } } + memset(pix-reserved, 0, sizeof(pix-reserved)); for (i = 0; i pix-num_planes; i++) { plane_fmt = pix-plane_fmt[i]; depth = fmt-vpdma_fmt[i]-depth; @@ -1499,6 +1500,8 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, plane_fmt-sizeimage = (pix-height * pix-width * depth) 3; + + memset(plane_fmt-reserved, 0, sizeof(plane_fmt-reserved)); } return 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
Re: [GIT PULL] SDR API
On 10.03.2014 21:38, Antti Palosaari wrote: That is just same set I sent earlier too, but rebased to latest media/master and 6 small compliance fix. PULL request update. I rebased that again to todays media/master as master was rebased. The following changes since commit 0d49e7761173520ff02cec6f11d581f8ebca764d: drx-j: Fix post-BER calculus on QAM modulation (2014-03-11 07:43:54 -0300) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git sdr_review_v4 for you to fetch changes up to 5356c649ca0551095120b37abcae001e0d573865: msi3101: fix v4l2-compliance issues (2014-03-11 14:25:16 +0200) Antti Palosaari (37): v4l: add RF tuner channel bandwidth control v4l: reorganize RF tuner control ID numbers v4l: uapi: add SDR formats CU8 and CU16LE v4l: add enum_freq_bands support to tuner sub-device v4l: add control for RF tuner PLL lock flag DocBook: V4L: add V4L2_SDR_FMT_CU8 - 'CU08' DocBook: V4L: add V4L2_SDR_FMT_CU16LE - 'CU16' DocBook: document RF tuner bandwidth controls DocBook: media: document PLL lock control DocBook: media: add some general info about RF tuners msi3101: convert to SDR API msi001: Mirics MSi001 silicon tuner driver msi3101: use msi001 tuner driver MAINTAINERS: add msi001 driver MAINTAINERS: add msi3101 driver msi3101: clamp mmap buffers to reasonable level e4000: convert DVB tuner to I2C driver model e4000: implement controls via v4l2 control framework e4000: fix PLL calc to allow higher frequencies e4000: implement PLL lock v4l control e4000: get rid of DVB i2c_gate_ctrl() e4000: convert to Regmap API e4000: rename some variables rtl2832_sdr: Realtek RTL2832 SDR driver module rtl28xxu: constify demod config structs rtl28xxu: attach SDR extension module rtl28xxu: fix switch-case style issue rtl28xxu: use muxed RTL2832 I2C adapters for E4000 and RTL2832_SDR rtl2832_sdr: expose e4000 controls to user r820t: add manual gain controls rtl2832_sdr: expose R820T controls to user MAINTAINERS: add rtl2832_sdr driver v4l: rename v4l2_format_sdr to v4l2_sdr_format rtl2832_sdr: clamp bandwidth to nearest legal value in automode rtl28xxu: depends on I2C_MUX msi001: fix v4l2-compliance issues msi3101: fix v4l2-compliance issues Hans Verkuil (1): rtl2832u_sdr: fixing v4l2-compliance issues Documentation/DocBook/media/v4l/controls.xml | 51 +++- Documentation/DocBook/media/v4l/dev-sdr.xml |2 +- Documentation/DocBook/media/v4l/pixfmt-sdr-cu08.xml | 44 Documentation/DocBook/media/v4l/pixfmt-sdr-cu16le.xml | 46 Documentation/DocBook/media/v4l/pixfmt.xml|3 + MAINTAINERS | 30 +++ drivers/media/tuners/Kconfig |1 + drivers/media/tuners/e4000.c | 598 +-- drivers/media/tuners/e4000.h | 21 +- drivers/media/tuners/e4000_priv.h | 86 ++- drivers/media/tuners/r820t.c | 137 ++- drivers/media/tuners/r820t.h | 10 + drivers/media/usb/dvb-usb-v2/Kconfig |2 +- drivers/media/usb/dvb-usb-v2/Makefile |1 + drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 90 +-- drivers/media/usb/dvb-usb-v2/rtl28xxu.h |2 + drivers/media/v4l2-core/v4l2-ctrls.c |9 + drivers/media/v4l2-core/v4l2-ioctl.c |2 +- drivers/staging/media/Kconfig |2 + drivers/staging/media/Makefile|2 + drivers/staging/media/msi3101/Kconfig |7 +- drivers/staging/media/msi3101/Makefile|1 + drivers/staging/media/msi3101/msi001.c| 500 +++ drivers/staging/media/msi3101/sdr-msi3101.c | 1564 +- drivers/staging/media/rtl2832u_sdr/Kconfig|7 + drivers/staging/media/rtl2832u_sdr/Makefile |6 + drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c | 1501 + drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.h | 51 include/media/v4l2-subdev.h |1 + include/uapi/linux/v4l2-controls.h| 15 +- include/uapi/linux/videodev2.h| 10 +- 31 files changed, 3531 insertions(+), 1271 deletions(-) create mode
Re: [PATCH v6 03/10] Documentation: devicetree: Update Samsung FIMC DT binding
Hi Sylwester, Thank you for the patch. On Thursday 06 March 2014 17:20:12 Sylwester Nawrocki wrote: This patch documents following updates of the Exynos4 SoC camera subsystem devicetree binding: - addition of #clock-cells property to 'camera' node - the #clock-cells property is needed when the sensor sub-devices use clock provided by the camera host interface, - addition of an optional clock-output-names property, - change of the clock-frequency at image sensor node from mandatory to an optional property - the sensor devices can now control their clock themselves and there should be no need to require this property by the camera host device binding, a default frequency value can often be used. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com --- Changes since v5: - none. Changes since v4: - dropped a requirement of specific order of values in clocks/ clock-names properties (Mark) and reference to clock-names in clock-output-names property description (Mark). --- .../devicetree/bindings/media/samsung-fimc.txt | 34 - 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt index 96312f6..dbd4020 100644 --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt @@ -32,6 +32,21 @@ way around. The 'camera' node must include at least one 'fimc' child node. +Optional properties: + +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node and it should + be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively. + +- clock-output-names: from the common clock bindings, should contain names of + clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks respectively. Wouldn't it be better to document the cam_mclk_a and cam_mclk_b names explicitly ? Or do you expect different names to be used in different DT files ? And as they correspond to the CAM_A_CLKOUT and CAM_B_CLKOUT pins, shouldn't they be named cam_a_clkout and cam_b_clkout ? + +Note: #clock-cells and clock-output-names are mandatory properties if external +image sensor devices reference 'camera' device node as a clock provider. + What's the reason not to make them always mandatory ? Backward compatibility only ? If so wouldn't it make sense to document the properties as mandatory from now on, and treating them as optional in the driver for backward compatibility ? 'fimc' device nodes --- @@ -97,8 +112,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller (e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt. -The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensor device +nodes. Default value when this property is not present is 24 MHz. This bothers me. Having the FIMC driver read the clock-frequence property from the sensor DT nodes feels like a layering violation. Shouldn't the sensor drivers call clk_set_rate() explicitly instead ? Example: @@ -114,7 +129,7 @@ Example: vddio-supply = ...; clock-frequency = 2400; - clocks = ...; + clocks = camera 1; clock-names = mclk; port { @@ -135,7 +150,7 @@ Example: vddio-supply = ...; clock-frequency = 2400; - clocks = ...; + clocks = camera 0; clock-names = mclk; port { @@ -149,12 +164,15 @@ Example: camera { compatible = samsung,fimc, simple-bus; - #address-cells = 1; - #size-cells = 1; - status = okay; - + clocks = clock 132, clock 133; + clock-names = sclk_cam0, sclk_cam1; The documentation mentions that clock-names must contain sclk_cam0, sclk_cam1, pxl_async0, pxl_async1. Are the last two optional ? If so I think you should clarify the description of the clock-names property. This can be done in a separate patch. + #clock-cells = 1; + clock-output-names = cam_mclk_a, cam_mclk_b; pinctrl-names = default;
Re: [PATCH v3 13/14] v4l: ti-vpe: Set correct field parameter for output and capture buffers
On 03/11/14 09:33, Archit Taneja wrote: The vpe driver wasn't setting the correct field parameter for dequed CAPTURE type buffers for the case where the captured output is progressive. Set the field to V4L2_FIELD_NONE for the completed destination buffers when the captured output is progressive. For OUTPUT type buffers, a queued buffer's field is forced to V4L2_FIELD_NONE if the pixel format(configured through s_fmt for the buffer type V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE specifies) the field type isn't interlaced. If the pixel format specified was V4L2_FIELD_ALTERNATE, and the queued buffer's field isn't V4L2_FIELD_TOP or V4L2_FIELD_BOTTOM, the vb2 buf_prepare op returns an error. This ensures compliance, and that the dequeued output and captured buffers contain the field type that the driver used internally. Signed-off-by: Archit Taneja arc...@ti.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/ti-vpe/vpe.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 970408a..c884910 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1296,10 +1296,10 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) d_buf-timecode = s_buf-timecode; } d_buf-sequence = ctx-sequence; - d_buf-field = ctx-field; d_q_data = ctx-q_data[Q_DATA_DST]; if (d_q_data-flags Q_DATA_INTERLACED) { + d_buf-field = ctx-field; if (ctx-field == V4L2_FIELD_BOTTOM) { ctx-sequence++; ctx-field = V4L2_FIELD_TOP; @@ -1308,6 +1308,7 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) ctx-field = V4L2_FIELD_BOTTOM; } } else { + d_buf-field = V4L2_FIELD_NONE; ctx-sequence++; } @@ -1871,6 +1872,16 @@ static int vpe_buf_prepare(struct vb2_buffer *vb) q_data = get_q_data(ctx, vb-vb2_queue-type); num_planes = q_data-fmt-coplanar ? 2 : 1; + if (vb-vb2_queue-type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { + if (!(q_data-flags Q_DATA_INTERLACED)) { + vb-v4l2_buf.field = V4L2_FIELD_NONE; + } else { + if (vb-v4l2_buf.field != V4L2_FIELD_TOP || + vb-v4l2_buf.field != V4L2_FIELD_BOTTOM) + return -EINVAL; + } + } + for (i = 0; i num_planes; i++) { if (vb2_plane_size(vb, i) q_data-sizeimage[i]) { vpe_err(ctx-dev, -- 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 v3 14/14] v4l: ti-vpe: retain v4l2_buffer flags for captured buffers
On 03/11/14 09:33, Archit Taneja wrote: The dequed CAPTURE_MPLANE type buffers don't contain the flags that the originally queued OUTPUT_MPLANE type buffers have. This breaks compliance. Copy the source v4l2_buffer flags to the destination v4l2_buffer flags before they are dequed. Signed-off-by: Archit Taneja arc...@ti.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/ti-vpe/vpe.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index c884910..f7759e8 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1288,13 +1288,12 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) s_buf = s_vb-v4l2_buf; d_buf = d_vb-v4l2_buf; + d_buf-flags = s_buf-flags; + d_buf-timestamp = s_buf-timestamp; - d_buf-flags = ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK; - d_buf-flags |= s_buf-flags V4L2_BUF_FLAG_TSTAMP_SRC_MASK; - if (s_buf-flags V4L2_BUF_FLAG_TIMECODE) { - d_buf-flags |= V4L2_BUF_FLAG_TIMECODE; + if (s_buf-flags V4L2_BUF_FLAG_TIMECODE) d_buf-timecode = s_buf-timecode; - } + d_buf-sequence = ctx-sequence; d_q_data = ctx-q_data[Q_DATA_DST]; -- 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] [media] s5p-mfc: Don't try to resubmit VP8 bitstream buffer for decode.
Hi Kamil, On Tue, Mar 11, 2014 at 4:59 PM, Kamil Debski k.deb...@samsung.com wrote: Hi Arun, From: Arun Kumar K [mailto:arunkk.sams...@gmail.com] On Behalf Of Arun Kumar K Sent: Friday, March 07, 2014 9:26 AM From: Pawel Osciak posc...@chromium.org Currently, for formats that are not H264, MFC driver will check the consumed stream size returned by the firmware and, based on that, will try to decide whether the bitstream buffer contained more than one frame. If the size of the buffer is larger than the consumed stream, it assumes that there are more frames in the buffer and that the buffer should be resubmitted for decode. This rarely works though and actually introduces problems, because: - v7 firmware will always return consumed stream size equal to whatever the driver passed to it when running decode (which is the size of the whole buffer), which means we will never try to resubmit, because the firmware will always tell us that it consumed all the data we passed to it; This does sound like a hardware bug/feature. So in v7 the buffer is never resubmitted, yes? And this patch makes no difference for v7? To give some background to this - I have added this patch [1] so that multi-frame input buffer is supported by the driver. [1] https://patchwork.linuxtv.org/patch/15448/ This patch was added to address one test case in VP8 decoding where 2 frames used to come in a single buffer. Without this patch, it used to skip decoding of the 2nd frame even though firmware returned consumed bytes as only 1 frame size. Now this concept gave issues when we tried to decode the VP8 stream encoded using v7 firmware. In that case, an arbitrary amount of padding bytes were added by the firmware after every frame which is allowed as per VP8 standard. While decoding this using v6, firmware returns less consumed bytes than the input buffer, but the remaining bytes are just padding. So firmware used to throw error if we re-submit this stuffing bytes as a new frame. In v7 firmware, this problem doesnt exist as it consumes (atleast indicates that it consumed) all the input buffer. So we came to the conclusion that the testcase of giving multiple frames in one input buffer itself was wrong and hence it was removed. Now this concept is needed only for MPEG4 packed PB case which this code for resubmitting input buffer was meant for (before my patch made it generic). - v6 firmware will return the number of consumed bytes, but will not include the padding (stuffing) bytes that are allowed after the frame in VP8. Since there is no way of figuring out how many of those bytes follow the frame without getting the frame size from IVF headers (or somewhere else, but not from the stream itself), the driver tries to guess that padding size is not larger than 4 bytes, which is not always true; How about v5 of MFC? I need to do some additional testing, as I don't want to introduce any regressions. I remember that this check was a result of a fair amount of work and testing with v5. I hope it wont give any issues in v5 also as it was never meant to handle multiple frames in one input buffer. The only way to make it work is to queue only one frame per buffer from userspace and the check in the kernel is useless and wrong for VP8. MPEG4 still seems to require it, so keep it only for that format. Is your goal to give more than one frame in a single buffer and have the buffer resubmitted? Or the opposite - you are getting the frame resubmitted without the need? By the contents of this patch I guess the latter, on the other hand I do remember that at some point the idea was to be able to queue more than one frame per buffer. I don't remember exactly who was opting for the ability to queue more frames in a single buffer... I hope now it is clear. We want to disallow multiple frames in one buffer as the behavior is not consistent in v6 and its not allowed anyway from v7 onwards. Regards Arun Best wishes, -- Kamil Debski Samsung RD Institute Poland Signed-off-by: Pawel Osciak posc...@chromium.org Signed-off-by: Arun Kumar K arun...@samsung.com --- drivers/media/platform/s5p-mfc/s5p_mfc.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index e2aac59..66c1775 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -360,7 +360,7 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx *ctx, list); ctx-consumed_stream += s5p_mfc_hw_call(dev-mfc_ops, get_consumed_stream, dev); - if (ctx-codec_mode != S5P_MFC_CODEC_H264_DEC + if (ctx-codec_mode == S5P_MFC_CODEC_MPEG4_DEC ctx-consumed_stream + STUFF_BYTE
Re: [PATCH v3 07/14] v4l: ti-vpe: Add selection API in VPE driver
On Tuesday 11 March 2014 05:51 PM, Hans Verkuil wrote: Hi Archit, A few small comments below... On 03/11/14 09:33, Archit Taneja wrote: Add selection ioctl ops. For VPE, cropping makes sense only for the input to VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers). For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP results in selecting a rectangle region within the source buffer. Setting the crop/compose rectangles should successfully result in re-configuration of registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 141 1 file changed, 141 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index ece9b96..4abb85c 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx, { switch (type) { case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: + case V4L2_BUF_TYPE_VIDEO_OUTPUT: return ctx-q_data[Q_DATA_SRC]; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: + case V4L2_BUF_TYPE_VIDEO_CAPTURE: return ctx-q_data[Q_DATA_DST]; default: BUG(); @@ -1587,6 +1589,142 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f) return set_srcdst_params(ctx); } +static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s) +{ + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + case V4L2_SEL_TGT_COMPOSE: + /* +* COMPOSE target is only valid for capture buffer type, for +* output buffer type, assign existing crop parameters to the +* selection rectangle +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) + break; Shouldn't this return -EINVAL? compose only makes sense for CAPTURE. So, it breaks and performs the calculations after the switch statement. + + s-r = q_data-c_rect; + return 0; The above 2 lines are called for if we try to set compose for OUTPUT. I don't return an error here, just keep the size as the original rect size, and return 0. I'll replace these 2 lines with 'return -EINVAL;' + + case V4L2_SEL_TGT_CROP: + /* +* CROP target is only valid for output buffer type, for capture +* buffer type, assign existing compose parameters to the +* selection rectangle +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) + break; Ditto. + + s-r = q_data-c_rect; + return 0; + + /* +* bound and default crop/compose targets are invalid targets to +* try/set +*/ + default: + return -EINVAL; + } + + if (s-r.top 0 || s-r.left 0) { + vpe_err(ctx-dev, negative values for top and left\n); + s-r.top = s-r.left = 0; + } + + v4l_bound_align_image(s-r.width, MIN_W, q_data-width, 1, + s-r.height, MIN_H, q_data-height, H_ALIGN, S_ALIGN); + + /* adjust left/top if cropping rectangle is out of bounds */ + if (s-r.left + s-r.width q_data-width) + s-r.left = q_data-width - s-r.width; + if (s-r.top + s-r.height q_data-height) + s-r.top = q_data-height - s-r.height; + + return 0; +} + +static int vpe_g_selection(struct file *file, void *fh, + struct v4l2_selection *s) +{ + struct vpe_ctx *ctx = file2ctx(file); + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + /* return width and height from S_FMT of the respective buffer type */ + case V4L2_SEL_TGT_COMPOSE_DEFAULT: + case V4L2_SEL_TGT_COMPOSE_BOUNDS: + case V4L2_SEL_TGT_CROP_BOUNDS: + case V4L2_SEL_TGT_CROP_DEFAULT: + s-r.left = 0; + s-r.top = 0; + s-r.width = q_data-width; + s-r.height = q_data-height; The crop targets only make sense
[GIT PULL FOR v3.15] DocBook fixes and a new pci skeleton driver template
Hi Mauro, This patch series adds a bunch of docbook fixes, posted here earlier: http://www.spinics.net/lists/linux-media/msg74059.html and it adds a pci skeleton driver originally written for FOSDEM 2014 and posted earlier here: http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/75338 No existing drivers are changed, just docbook modifications and a new template driver. For the ELC this year I hope to create a USB skeleton driver. Regards, Hans The following changes since commit 4a1df5e8f6712df3b5f8aeb09771a1169ddd8e8c: [media] s2255drv: memory leak fix (2014-03-11 09:28:31 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git docbook for you to fetch changes up to baedfa602a02b0935a15fcdbef6ac9a65e957bb2: v4l2-pci-skeleton: add a V4L2 PCI skeleton driver (2014-03-11 13:38:02 +0100) Hans Verkuil (6): DocBook media: update STREAMON/OFF documentation. DocBook: fix incorrect code example DocBook media: clarify v4l2_buffer/plane fields. DocBook media: fix broken FIELD_ALTERNATE description. DocBook media: clarify v4l2_pix_format and v4l2_pix_format_mplane fields v4l2-pci-skeleton: add a V4L2 PCI skeleton driver Documentation/DocBook/media/v4l/dev-osd.xml | 22 +- Documentation/DocBook/media/v4l/io.xml | 61 +++-- Documentation/DocBook/media/v4l/pixfmt.xml | 33 ++- Documentation/DocBook/media/v4l/vidioc-streamon.xml | 28 +- drivers/media/pci/Kconfig | 1 + drivers/media/pci/Makefile | 2 + drivers/media/pci/skeleton/Kconfig | 11 + drivers/media/pci/skeleton/Makefile | 1 + drivers/media/pci/skeleton/v4l2-pci-skeleton.c | 912 ++ 9 files changed, 1025 insertions(+), 46 deletions(-) create mode 100644 drivers/media/pci/skeleton/Kconfig create mode 100644 drivers/media/pci/skeleton/Makefile create mode 100644 drivers/media/pci/skeleton/v4l2-pci-skeleton.c -- 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 v3 07/14] v4l: ti-vpe: Add selection API in VPE driver
On 03/11/14 13:46, Archit Taneja wrote: On Tuesday 11 March 2014 05:51 PM, Hans Verkuil wrote: Hi Archit, A few small comments below... On 03/11/14 09:33, Archit Taneja wrote: Add selection ioctl ops. For VPE, cropping makes sense only for the input to VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers). For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP results in selecting a rectangle region within the source buffer. Setting the crop/compose rectangles should successfully result in re-configuration of registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 141 1 file changed, 141 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index ece9b96..4abb85c 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx, { switch (type) { case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: +case V4L2_BUF_TYPE_VIDEO_OUTPUT: return ctx-q_data[Q_DATA_SRC]; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: +case V4L2_BUF_TYPE_VIDEO_CAPTURE: return ctx-q_data[Q_DATA_DST]; default: BUG(); @@ -1587,6 +1589,142 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f) return set_srcdst_params(ctx); } +static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s) +{ +struct vpe_q_data *q_data; + +if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) +(s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) +return -EINVAL; + +q_data = get_q_data(ctx, s-type); +if (!q_data) +return -EINVAL; + +switch (s-target) { +case V4L2_SEL_TGT_COMPOSE: +/* + * COMPOSE target is only valid for capture buffer type, for + * output buffer type, assign existing crop parameters to the + * selection rectangle + */ +if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) +break; Shouldn't this return -EINVAL? compose only makes sense for CAPTURE. So, it breaks and performs the calculations after the switch statement. It's so easy to get confused here. + +s-r = q_data-c_rect; +return 0; The above 2 lines are called for if we try to set compose for OUTPUT. I don't return an error here, just keep the size as the original rect size, and return 0. I'll replace these 2 lines with 'return -EINVAL;' Yes, that makes more sense. + +case V4L2_SEL_TGT_CROP: +/* + * CROP target is only valid for output buffer type, for capture + * buffer type, assign existing compose parameters to the + * selection rectangle + */ +if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) +break; Ditto. + +s-r = q_data-c_rect; +return 0; + +/* + * bound and default crop/compose targets are invalid targets to + * try/set + */ +default: +return -EINVAL; +} + +if (s-r.top 0 || s-r.left 0) { +vpe_err(ctx-dev, negative values for top and left\n); +s-r.top = s-r.left = 0; +} + +v4l_bound_align_image(s-r.width, MIN_W, q_data-width, 1, +s-r.height, MIN_H, q_data-height, H_ALIGN, S_ALIGN); + +/* adjust left/top if cropping rectangle is out of bounds */ +if (s-r.left + s-r.width q_data-width) +s-r.left = q_data-width - s-r.width; +if (s-r.top + s-r.height q_data-height) +s-r.top = q_data-height - s-r.height; + +return 0; +} + +static int vpe_g_selection(struct file *file, void *fh, +struct v4l2_selection *s) +{ +struct vpe_ctx *ctx = file2ctx(file); +struct vpe_q_data *q_data; + +if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) +(s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) +return -EINVAL; + +q_data = get_q_data(ctx, s-type); +if (!q_data) +return -EINVAL; + +switch (s-target) { +/* return width and height from S_FMT of the respective buffer type */ +case V4L2_SEL_TGT_COMPOSE_DEFAULT: +case V4L2_SEL_TGT_COMPOSE_BOUNDS: +case V4L2_SEL_TGT_CROP_BOUNDS: +case V4L2_SEL_TGT_CROP_DEFAULT: +s-r.left = 0; +s-r.top = 0; +s-r.width = q_data-width; +s-r.height = q_data-height; The crop targets only make sense for type OUTPUT and the compose only for type CAPTURE. Add some checks for that. I return the image size for G_FMT irrespective
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
On 11/03/14 13:43, Laurent Pinchart wrote: We could scan the whole tree for entities, ports and endpoints once, in the base oftree code, and put that into a graph structure, adding the backlinks. The of_graph_* helpers could then use that graph instead of the device tree. That could work. The complexity would still be quadratic, but we would parse the full device tree once only. The runtime complexity would still be increased, as the graph helpers would need to find the endpoint object in the parsed graph corresponding to the DT node they get as an argument. That's proportional to the number of graph elements, not the total number of DT nodes, so I suppose it's not too bad. We also need to make sure this would work with insertion of DT fragments at runtime. Probably not a big deal, but it has to be kept in mind. About the endpoint linking direction... As I think was suggested, the base logic would be to make endpoints point outward from the SoC, i.e. a display controller would point to a panel, and a capture controller would point to a sensor. But how about this case: I have a simple video pipeline with a display controller, an encoder and a panel, as follows: dispc - encoder - panel Here the arrows show which way the remote-endpoint links point. So looking at the encoder, the encoder's input port is pointed at by the dispc, and the encoder's output port points at the panel. Then, I have a capture pipeline, with a capture controller, an encoder (the same one that was used for display above) and a sensor, as follows: camc - encoder - sensor Again the arrows show the links. Note that here the encoder's _output_ port is pointed at by the camc, and the encoder's _input_ port points at the sensor. So depending on the use case, the endpoints would point to opposite direction from the encoder's point of view. And if I gathered Grant's opinion correctly (correct me if I'm wrong), he thinks things should be explicit, i.e. the bindings for, say, an encoder should state that the encoder's output endpoint _must_ contain a remote-endpoint property, whereas the encoder's input endpoint _must not_ contain a remote-endpoint property. Tomi signature.asc Description: OpenPGP digital signature
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
On 03/10/2014 04:15 PM, Laurent Pinchart wrote: Hi Grant, On Monday 10 March 2014 14:58:15 Grant Likely wrote: On Mon, 10 Mar 2014 14:52:53 +0100, Laurent Pinchart wrote: On Monday 10 March 2014 12:18:20 Tomi Valkeinen wrote: On 08/03/14 13:41, Grant Likely wrote: Ok. If we go for single directional link, the question is then: which way? And is the direction different for display and camera, which are kind of reflections of each other? In general I would recommend choosing whichever device you would sensibly think of as a master. In the camera case I would choose the camera controller node instead of the camera itself, and in the display case I would choose the display controller instead of the panel. The binding author needs to choose what she things makes the most sense, but drivers can still use if it it turns out to be 'backwards' I would perhaps choose the same approach, but at the same time I think it's all but clear. The display controller doesn't control the panel any more than a DMA controller controls, say, the display controller. In fact, in earlier versions of OMAP DSS DT support I had a simpler port description, and in that I had the panel as the master (i.e. link from panel to dispc) because the panel driver uses the display controller's features to provide the panel device a data stream. And even with the current OMAP DSS DT version, which uses the v4l2 style ports/endpoints, the driver model is still the same, and only links towards upstream are used. So one reason I'm happy with the dual-linking is that I can easily follow the links from the downstream entities to upstream entities, and other people, who have different driver model, can easily do the opposite. But I agree that single-linking is enough and this can be handled at runtime, even if it makes the code more complex. And perhaps requires extra data in the dts, to give the start points for the graph. In theory unidirectional links in DT are indeed enough. However, let's not forget the following. - There's no such thing as single start points for graphs. Sure, in some simple cases the graph will have a single start point, but that's not a generic rule. For instance the camera graphs http://ideasonboard.org/media/omap3isp.ps and http://ideasonboard.org/media/eyecam.ps have two camera sensors, and thus two starting points from a data flow point of view. And if you want a better understanding of how complex media graphs can become, have a look at http://ideasonboard.org/media/vsp1.0.pdf (that's a real world example, albeit all connections are internal to the SoC in that particular case, and don't need to be described in DT). - There's also no such thing as a master device that can just point to slave devices. Once again simple cases exist where that model could work, but real world examples exist of complex pipelines with dozens of elements all implemented by a separate IP core and handled by separate drivers, forming a graph with long chains and branches. We thus need real graph bindings. - Finally, having no backlinks in DT would make the software implementation very complex. We need to be able to walk the graph in a generic way without having any of the IP core drivers loaded, and without any specific starting point. We would thus need to parse the complete DT tree, looking at all nodes and trying to find out whether they're part of the graph we're trying to walk. The complexity of the operation would be at best quadratic to the number of nodes in the whole DT and to the number of nodes in the graph. Not really. To being with, you cannot determine any meaning of a node across the tree (aside from it being an endpoint) That's the important part. I can assume the target node of the remote-endpoint phandle to be an endpoint, and can thus assume that it implements the of-graph bindings. That's all I need to be able to walk the graph in a generic way. without also understanding the binding that the node is a part of. That means you need to have something matching against the compatible string on both ends of the linkage. For instance: panel { compatible = acme,lvds-panel; lvds-port: port { }; }; display-controller { compatible = encom,video; port { remote-endpoint = lvds-port; }; }; In the above example, the encom,video driver has absolutely zero information about what the acme,lvds-panel binding actually implements. There needs to be both a driver for the acme,lvds-panel binding and one for the encom,video binding (even if the acme,lvds-panel binding is very thin and defers the functionality to the video controller). I absolutely agree with that. We need a driver for each device (in this case the acme panel and the encom display controller), and we need those drivers to register entities (in the generic sense of the term) for them to be able to communicate with each other. The
Re: [PATCH v3 07/14] v4l: ti-vpe: Add selection API in VPE driver
On Tuesday 11 March 2014 06:19 PM, Hans Verkuil wrote: On 03/11/14 13:46, Archit Taneja wrote: On Tuesday 11 March 2014 05:51 PM, Hans Verkuil wrote: Hi Archit, A few small comments below... On 03/11/14 09:33, Archit Taneja wrote: snip Yes. If for no other reason that I plan on adding crop/compose/scaling support to v4l2-compliance soon, and this will probably be one of the tests. Thanks, will update. Thanks for reviewing of the series. Archit Thanks, Archit -- 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: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
Hi Tomi, On Tuesday 11 March 2014 14:59:20 Tomi Valkeinen wrote: On 11/03/14 13:43, Laurent Pinchart wrote: We could scan the whole tree for entities, ports and endpoints once, in the base oftree code, and put that into a graph structure, adding the backlinks. The of_graph_* helpers could then use that graph instead of the device tree. That could work. The complexity would still be quadratic, but we would parse the full device tree once only. The runtime complexity would still be increased, as the graph helpers would need to find the endpoint object in the parsed graph corresponding to the DT node they get as an argument. That's proportional to the number of graph elements, not the total number of DT nodes, so I suppose it's not too bad. We also need to make sure this would work with insertion of DT fragments at runtime. Probably not a big deal, but it has to be kept in mind. About the endpoint linking direction... As I think was suggested, the base logic would be to make endpoints point outward from the SoC, i.e. a display controller would point to a panel, and a capture controller would point to a sensor. But how about this case: I have a simple video pipeline with a display controller, an encoder and a panel, as follows: dispc - encoder - panel Here the arrows show which way the remote-endpoint links point. So looking at the encoder, the encoder's input port is pointed at by the dispc, and the encoder's output port points at the panel. Then, I have a capture pipeline, with a capture controller, an encoder (the same one that was used for display above) and a sensor, as follows: camc - encoder - sensor Again the arrows show the links. Note that here the encoder's _output_ port is pointed at by the camc, and the encoder's _input_ port points at the sensor. So depending on the use case, the endpoints would point to opposite direction from the encoder's point of view. And if I gathered Grant's opinion correctly (correct me if I'm wrong), he thinks things should be explicit, i.e. the bindings for, say, an encoder should state that the encoder's output endpoint _must_ contain a remote-endpoint property, whereas the encoder's input endpoint _must not_ contain a remote-endpoint property. Actually my understand was that DT links would have the same direction as the data flow. There would be no ambiguity in that case as the direction of the data flow is known. What happens for bidirectional data flows still need to be discussed though. And if we want to use the of-graph bindings to describe graphs without a data flow, a decision will need to be taken there too. -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part.
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
On 11/03/14 15:16, Laurent Pinchart wrote: And if I gathered Grant's opinion correctly (correct me if I'm wrong), he thinks things should be explicit, i.e. the bindings for, say, an encoder should state that the encoder's output endpoint _must_ contain a remote-endpoint property, whereas the encoder's input endpoint _must not_ contain a remote-endpoint property. Actually my understand was that DT links would have the same direction as the data flow. There would be no ambiguity in that case as the direction of the Ok. At least at some point in the discussion the rule of thumb was to have the master point at the slave, which are a bit vague terms, but I think both display and camera controllers were given as examples of master. data flow is known. What happens for bidirectional data flows still need to be discussed though. And if we want to use the of-graph bindings to describe graphs without a data flow, a decision will need to be taken there too. Yep. My worry is that if the link direction is defined in the bindings for the device, somebody will at some point have a complex board which happens to use two devices in such a way, that either neither of them point to each other, or both point to each other. Maybe we can make sure that never happens, but I feel a bit nervous about it especially for bi-directional cases. If the link has no clear main-dataflow direction, it may be a bit difficult to say which way to link. With double-linking all those concerns just disappear. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH v6 03/10] Documentation: devicetree: Update Samsung FIMC DT binding
Hi Laurent, Thanks for your review. On 11/03/14 13:30, Laurent Pinchart wrote: [...] --- .../devicetree/bindings/media/samsung-fimc.txt | 34 - 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt index 96312f6..dbd4020 100644 --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt @@ -32,6 +32,21 @@ way around. The 'camera' node must include at least one 'fimc' child node. +Optional properties: + +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node and it should + be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively. + +- clock-output-names: from the common clock bindings, should contain names of + clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks respectively. Wouldn't it be better to document the cam_mclk_a and cam_mclk_b names explicitly ? Or do you expect different names to be used in different DT files ? And as they correspond to the CAM_A_CLKOUT and CAM_B_CLKOUT pins, shouldn't they be named cam_a_clkout and cam_b_clkout ? Basically I could use fixed names for these clocks, I just wanted to keep a possibility to override them in dts to avoid any possible clock name collisions, rather than keep a list of different names per SoC in the driver. Right now fixed names could also be used for all SoCs I'm aware of, nevertheless I would prefer to keep the clock-output-names property. cam_a_clkout, cam_b_clkout may be indeed better names, I'll change that. +Note: #clock-cells and clock-output-names are mandatory properties if external +image sensor devices reference 'camera' device node as a clock provider. + What's the reason not to make them always mandatory ? Backward compatibility only ? If so wouldn't it make sense to document the properties as mandatory from now on, and treating them as optional in the driver for backward compatibility ? Yes, it's for backwards compatibility only. It may be a good idea to just document them as required, since this is how the device is expected to be described in DT from now. I'll just make these a required properties, the driver already handles them as optional. 'fimc' device nodes --- @@ -97,8 +112,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller (e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt. -The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensor device +nodes. Default value when this property is not present is 24 MHz. This bothers me. Having the FIMC driver read the clock-frequence property from the sensor DT nodes feels like a layering violation. Shouldn't the sensor drivers call clk_set_rate() explicitly instead ? It is supposed to do so, after this whole patch series. So the camera controller driver will not need such properties. What do you think about removing this sentence altogether ? Example: @@ -114,7 +129,7 @@ Example: vddio-supply = ...; clock-frequency = 2400; -clocks = ...; +clocks = camera 1; clock-names = mclk; port { @@ -135,7 +150,7 @@ Example: vddio-supply = ...; clock-frequency = 2400; -clocks = ...; +clocks = camera 0; clock-names = mclk; port { @@ -149,12 +164,15 @@ Example: camera { compatible = samsung,fimc, simple-bus; -#address-cells = 1; -#size-cells = 1; -status = okay; - +clocks = clock 132, clock 133; +clock-names = sclk_cam0, sclk_cam1; The documentation mentions that clock-names must contain sclk_cam0, sclk_cam1, pxl_async0, pxl_async1. Are the last two optional ? If so I think you should clarify the description of the clock-names property. This can be done in a separate patch. pxl_async0, pxl_async1 are mandatory, I'll add them also into this example dts. +#clock-cells = 1; +clock-output-names = cam_mclk_a, cam_mclk_b; pinctrl-names = default; pinctrl-0 = cam_port_a_clk_active; +
[PATCH] drx39xyj: fix 64 bit division on 32 bit arch
Fix this linker warning: WARNING: __divdi3 [media_build/v4l/drx39xyj.ko] undefined! Signed-off-by: Gianluca Gennari gennar...@gmail.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index 2352327..0dec073 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -12002,13 +12002,16 @@ static int drx39xxj_read_signal_strength(struct dvb_frontend *fe, static int drx39xxj_read_snr(struct dvb_frontend *fe, u16 *snr) { struct dtv_frontend_properties *p = fe-dtv_property_cache; + u64 tmp64; if (p-cnr.stat[0].scale == FE_SCALE_NOT_AVAILABLE) { *snr = 0; return 0; } - *snr = p-cnr.stat[0].svalue / 10; + tmp64 = p-cnr.stat[0].svalue; + do_div(tmp64, 10); + *snr = tmp64; return 0; } -- 1.9.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
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
Hi, Am Dienstag, den 11.03.2014, 15:27 +0200 schrieb Tomi Valkeinen: On 11/03/14 15:16, Laurent Pinchart wrote: And if I gathered Grant's opinion correctly (correct me if I'm wrong), he thinks things should be explicit, i.e. the bindings for, say, an encoder should state that the encoder's output endpoint _must_ contain a remote-endpoint property, whereas the encoder's input endpoint _must not_ contain a remote-endpoint property. Actually my understand was that DT links would have the same direction as the data flow. There would be no ambiguity in that case as the direction of the Ok. At least at some point in the discussion the rule of thumb was to have the master point at the slave, which are a bit vague terms, but I think both display and camera controllers were given as examples of master. data flow is known. What happens for bidirectional data flows still need to be discussed though. And if we want to use the of-graph bindings to describe graphs without a data flow, a decision will need to be taken there too. Yep. My worry is that if the link direction is defined in the bindings for the device, somebody will at some point have a complex board which happens to use two devices in such a way, that either neither of them point to each other, or both point to each other. Maybe we can make sure that never happens, but I feel a bit nervous about it especially for bi-directional cases. If the link has no clear main-dataflow direction, it may be a bit difficult to say which way to link. With double-linking all those concerns just disappear. another possibility would be to just leave it open in the generic bindings: Two endpoints are considered linked together if any of them contains a 'remote-endpoint' phandle property that points to the other. If both contain a 'remote-endpoint' property, both must point at each other. As long as we make sure that the generic code is able to generate the undirected graph from this, we could then let more specific bindings (like video-interfaces.txt) define that the phandle links always should point in both directions, in data flow direction, north, or outwards from the 'master'. regards Philipp -- 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 v4 3/3] Documentation: of: Document graph bindings
On 10/03/14 07:53, Tomi Valkeinen wrote: On 08/03/14 14:25, Grant Likely wrote: Sure. If endpoints are logical, then only create the ones actually hooked up. No problem there. But nor do I see any issue with having empty connections if the board author things it makes sense to have them in the dtsi. I don't think they are usually logical, although they probably might be in some cases. The endpoint nodes are supposed to be logical, they just group properties describing a port's configuration. As I see it, a port is a group of pins in a hardware component, and two endpoints define a connection between two ports, which on the HW level are the wires between the ports. So a port with two endpoints is a group of pins, with wires that go from the same pins to two different components. It could be approximated like this, but I don't think it is needed. I would rather stay with only port nodes mapped to hardware and the endpoint nodes logical. -- Regards, Sylwester -- 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 v4 1/3] [media] of: move graph helpers from drivers/media/v4l2-core to drivers/of
On 03/10/2014 12:42 PM, Laurent Pinchart wrote: Hi Andrzej, I like that idea. I would prefer making the 'port' nodes mandatory and the 'ports' and 'endpoint' nodes optional. Leaving the 'port' node out slightly decreases readability in my opinion, but making the 'endpoint' node optional increases it. That's just my point of view though. I want to propose another solution to simplify bindings, in fact I have few ideas to consider: 1. Use named ports instead of address-cells/regs. Ie instead of port@number schema, use port-function. This will allow to avoid ports node and #address-cells, #size-cells, reg properties. Additionally it should increase readability of the bindings. device { port-dsi { endpoint { ... }; }; port-rgb { endpoint { ... }; }; }; It is little bit like with gpios vs reset-gpios properties. Another advantage I see we do not need do mappings of port numbers to functions between dts, drivers and documentation. The problem with this approach is that ports are identified by a number inside the kernel, so we would still need to define name to number mappings, or switch to port names internally first. The mapping will be only internal in the driver. Anyway the bindings should be kernel agnostic. Andrzej 2. Similar approach can be taken to endpoint nodes, in fact as endpoints are children of port node and as I understand port node have no other children we can use any name instead of endpoint@number, of course some convention can be helpful. device { port-dsi { ep-soc1 { ... }; ep-soc2 { ... }; }; port-rgb { ep-panel { ... }; }; }; I see less issues here, as we don't need to number endpoints if I'm not mistaken. I would like to add that those ideas would work nicely with Sylwester's proposition of skipping endpoints nodes in case there is only one endpoint - the most common cases are devices with one or two ports, each port having only one remote endpoint. The complete graph for DSI/LVDS bridge I work recently will look like: dsim { dsim_ep: port-dsi { remote-endpoint = bridge_dsi_ep; }; }; bridge { bridge_dsi_ep: port-dsi { remote-endpoint = dsim_ep; }; bridge_lvds_ep: port-lvds { remote-endpoint = panel_ep; }; }; panel { port-lvds { remote-endpoint bridge_lvds_ep; }; }; -- 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] saa7134: Add support for SnaZio TvPVR PRO
Em Mon, 10 Feb 2014 07:28:31 +0200 Antti Palosaari cr...@iki.fi escreveu: Moikka! On 10.02.2014 01:44, GEORGE wrote: b/drivers/media/rc/keymaps/rc-snazio-tvpvr-pro.c new file mode 100644 index 000..44f0c81 --- /dev/null +++ b/drivers/media/rc/keymaps/rc-snazio-tvpvr-pro.c @@ -0,0 +1,116 @@ +/* rc-snazio-tvpvr-pro.h - Keytable for snazio_tvpvr_pro Remote Controller + * + * keymap imported from ir-keymaps.c + * + * Copyright (c) 2010 by Mauro Carvalho Chehab + * Copyright (c) 2014 by POJAR GEORGE geoubu...@gmail.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include media/rc-map.h +#include linux/module.h + +/* + Keycodes for remote on the SnaZio TvPVR PRO. + POJAR GEORGE geoubu...@gmail.com +*/ + +static struct rc_map_table snazio_tvpvr_pro[] = { + +/* Remote Button Layout + +POWER SOURCE SCANMUTE +TV/FM 1 2 3 +| 4 5 6 +| 7 8 9 +^^UP0 + RECALL +vvDNRECORD STOPPLAY + +MINIMIZE ZOOM + + CH+ + VOL- VOL+ + CH- + +SNAPSHOT MTS + + FUNC RESET +*/ + +{ 0x01, KEY_1 },/* 1 */ +{ 0x0b, KEY_2 },/* 2 */ +{ 0x1b, KEY_3 },/* 3 */ +{ 0x05, KEY_4 },/* 4 */ +{ 0x09, KEY_5 },/* 5 */ +{ 0x15, KEY_6 },/* 6 */ +{ 0x06, KEY_7 },/* 7 */ +{ 0x0a, KEY_8 },/* 8 */ +{ 0x12, KEY_9 },/* 9 */ +{ 0x02, KEY_0 },/* 0 */ +{ 0x10, KEY_KPPLUS },/* + */ +{ 0x13, KEY_AGAIN },/* Recall */ Clearly, your emailer is doing something wrong here, replacing tabs by white spaces. That prevents us to be apply to apply it. Also, please see the Antti comments below. + +{ 0x1e, KEY_POWER },/* Power */ +{ 0x07, KEY_VIDEO },/* Source */ +{ 0x1c, KEY_SEARCH },/* Scan */ +{ 0x18, KEY_MUTE },/* Mute */ + +{ 0x03, KEY_RADIO },/* TV/FM */ +/* The next four keys are duplicates that appear to send the + same IR code as Ch+, Ch-, , and . The raw code assigned + to them is the actual code + 0x20 - they will never be + detected as such unless some way is discovered to distinguish + these buttons from those that have the same code. */ +{ 0x3f, KEY_RIGHT },/* | and Ch+ */ +{ 0x37, KEY_LEFT },/* | and Ch- */ +{ 0x2c, KEY_UP },/* ^^Up and */ +{ 0x24, KEY_DOWN },/* vvDn and */ + +{ 0x00, KEY_RECORD },/* Record */ +{ 0x08, KEY_STOP },/* Stop */ +{ 0x11, KEY_PLAY },/* Play */ + +{ 0x0f, KEY_CLOSE },/* Minimize */ +{ 0x19, KEY_ZOOM },/* Zoom */ +{ 0x1a, KEY_CAMERA },/* Snapshot */ +{ 0x0d, KEY_LANGUAGE },/* MTS */ + +{ 0x14, KEY_VOLUMEDOWN },/* Vol- */ +{ 0x16, KEY_VOLUMEUP },/* Vol+ */ +{ 0x17, KEY_CHANNELDOWN },/* Ch- */ +{ 0x1f, KEY_CHANNELUP },/* Ch+ */ + +{ 0x04, KEY_REWIND },/* */ +{ 0x0e, KEY_MENU },/* Function */ +{ 0x0c, KEY_FASTFORWARD },/* */ +{ 0x1d, KEY_RESTART },/* Reset */ +}; + +static struct rc_map_list snazio_tvpvr_pro_map = { +.map = { +.scan= snazio_tvpvr_pro, +.size= ARRAY_SIZE(snazio_tvpvr_pro), +.rc_type = RC_TYPE_UNKNOWN,/* Legacy IR type */ +.name= RC_MAP_SNAZIO_TVPVR_PRO, +} +}; + +static int __init init_rc_map_snazio_tvpvr_pro(void) +{ +return rc_map_register(snazio_tvpvr_pro_map); +} + +static void __exit exit_rc_map_snazio_tvpvr_pro(void) +{ +rc_map_unregister(snazio_tvpvr_pro_map); +} + +module_init(init_rc_map_snazio_tvpvr_pro) +module_exit(exit_rc_map_snazio_tvpvr_pro) + +MODULE_LICENSE(GPL); +MODULE_AUTHOR(POJAR GEORGE geoubu...@gmail.com); diff --git a/include/media/rc-map.h b/include/media/rc-map.h index e5aa240..b802af4 100644 --- a/include/media/rc-map.h +++ b/include/media/rc-map.h @@ -177,6 +177,7 @@ void rc_map_init(void); #define RC_MAP_REAL_AUDIO_220_32_KEYS rc-real-audio-220-32-keys #define RC_MAP_REDDO rc-reddo #define RC_MAP_SNAPSTREAM_FIREFLYrc-snapstream-firefly +#define RC_MAP_SNAZIO_TVPVR_PRO rc-snazio-tvpvr-pro #define RC_MAP_STREAMZAP rc-streamzap #define RC_MAP_TBS_NEC rc-tbs-nec #define RC_MAP_TECHNISAT_USB2rc-technisat-usb2
Re: [PATCH v6 03/10] Documentation: devicetree: Update Samsung FIMC DT binding
Hi Sylwester, On Tuesday 11 March 2014 14:38:37 Sylwester Nawrocki wrote: Hi Laurent, Thanks for your review. You're welcome. On 11/03/14 13:30, Laurent Pinchart wrote: [...] --- .../devicetree/bindings/media/samsung-fimc.txt | 34 +- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt index 96312f6..dbd4020 100644 --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt @@ -32,6 +32,21 @@ way around. The 'camera' node must include at least one 'fimc' child node. +Optional properties: + +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node and it should + be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively. + +- clock-output-names: from the common clock bindings, should contain names of + clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks respectively. Wouldn't it be better to document the cam_mclk_a and cam_mclk_b names explicitly ? Or do you expect different names to be used in different DT files ? And as they correspond to the CAM_A_CLKOUT and CAM_B_CLKOUT pins, shouldn't they be named cam_a_clkout and cam_b_clkout ? Basically I could use fixed names for these clocks, I just wanted to keep a possibility to override them in dts to avoid any possible clock name collisions, rather than keep a list of different names per SoC in the driver. Right now fixed names could also be used for all SoCs I'm aware of, nevertheless I would prefer to keep the clock-output-names property. cam_a_clkout, cam_b_clkout may be indeed better names, I'll change that. OK, variable names are fine with me. +Note: #clock-cells and clock-output-names are mandatory properties if external +image sensor devices reference 'camera' device node as a clock provider. + What's the reason not to make them always mandatory ? Backward compatibility only ? If so wouldn't it make sense to document the properties as mandatory from now on, and treating them as optional in the driver for backward compatibility ? Yes, it's for backwards compatibility only. It may be a good idea to just document them as required, since this is how the device is expected to be described in DT from now. I'll just make these a required properties, the driver already handles them as optional. OK. 'fimc' device nodes --- @@ -97,8 +112,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller (e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt. -The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensor device +nodes. Default value when this property is not present is 24 MHz. This bothers me. Having the FIMC driver read the clock-frequence property from the sensor DT nodes feels like a layering violation. Shouldn't the sensor drivers call clk_set_rate() explicitly instead ? It is supposed to do so, after this whole patch series. So the camera controller driver will not need such properties. What do you think about removing this sentence altogether ? Sure. As the FIMC won't access the clock-frequency property of the sensor anymore after this patch series, let's just drop the mention of clock- frequency. Example: @@ -114,7 +129,7 @@ Example: vddio-supply = ...; clock-frequency = 2400; - clocks = ...; + clocks = camera 1; clock-names = mclk; port { @@ -135,7 +150,7 @@ Example: vddio-supply = ...; clock-frequency = 2400; - clocks = ...; + clocks = camera 0; clock-names = mclk; port { @@ -149,12 +164,15 @@ Example: camera { compatible = samsung,fimc, simple-bus; - #address-cells = 1; - #size-cells = 1; - status = okay; - + clocks = clock 132, clock 133; + clock-names = sclk_cam0, sclk_cam1; The documentation mentions that clock-names must contain
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
Hi Grant, Am Montag, den 10.03.2014, 14:58 + schrieb Grant Likely: On Mon, 10 Mar 2014 14:52:53 +0100, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: On Monday 10 March 2014 12:18:20 Tomi Valkeinen wrote: On 08/03/14 13:41, Grant Likely wrote: Ok. If we go for single directional link, the question is then: which way? And is the direction different for display and camera, which are kind of reflections of each other? In general I would recommend choosing whichever device you would sensibly think of as a master. In the camera case I would choose the camera controller node instead of the camera itself, and in the display case I would choose the display controller instead of the panel. The binding author needs to choose what she things makes the most sense, but drivers can still use if it it turns out to be 'backwards' I would perhaps choose the same approach, but at the same time I think it's all but clear. The display controller doesn't control the panel any more than a DMA controller controls, say, the display controller. In fact, in earlier versions of OMAP DSS DT support I had a simpler port description, and in that I had the panel as the master (i.e. link from panel to dispc) because the panel driver uses the display controller's features to provide the panel device a data stream. And even with the current OMAP DSS DT version, which uses the v4l2 style ports/endpoints, the driver model is still the same, and only links towards upstream are used. So one reason I'm happy with the dual-linking is that I can easily follow the links from the downstream entities to upstream entities, and other people, who have different driver model, can easily do the opposite. But I agree that single-linking is enough and this can be handled at runtime, even if it makes the code more complex. And perhaps requires extra data in the dts, to give the start points for the graph. In theory unidirectional links in DT are indeed enough. However, let's not forget the following. - There's no such thing as single start points for graphs. Sure, in some simple cases the graph will have a single start point, but that's not a generic rule. For instance the camera graphs http://ideasonboard.org/media/omap3isp.ps and http://ideasonboard.org/media/eyecam.ps have two camera sensors, and thus two starting points from a data flow point of view. And if you want a better understanding of how complex media graphs can become, have a look at http://ideasonboard.org/media/vsp1.0.pdf (that's a real world example, albeit all connections are internal to the SoC in that particular case, and don't need to be described in DT). - There's also no such thing as a master device that can just point to slave devices. Once again simple cases exist where that model could work, but real world examples exist of complex pipelines with dozens of elements all implemented by a separate IP core and handled by separate drivers, forming a graph with long chains and branches. We thus need real graph bindings. - Finally, having no backlinks in DT would make the software implementation very complex. We need to be able to walk the graph in a generic way without having any of the IP core drivers loaded, and without any specific starting point. We would thus need to parse the complete DT tree, looking at all nodes and trying to find out whether they're part of the graph we're trying to walk. The complexity of the operation would be at best quadratic to the number of nodes in the whole DT and to the number of nodes in the graph. Not really. To being with, you cannot determine any meaning of a node across the tree (aside from it being an endpoint) without also understanding the binding that the node is a part of. That means you need to have something matching against the compatible string on both ends of the linkage. For instance: panel { compatible = acme,lvds-panel; lvds-port: port { }; }; display-controller { compatible = encom,video; port { remote-endpoint = lvds-port; }; }; In the above example, the encom,video driver has absolutely zero information about what the acme,lvds-panel binding actually implements. There needs to be both a driver for the acme,lvds-panel binding and one for the encom,video binding (even if the acme,lvds-panel binding is very thin and defers the functionality to the video controller). What you want here is the drivers to register each side of the connection. That could be modeled with something like the following (pseudocode): struct of_endpoint { struct list_head list; struct device_node *ep_node; void *context; void (*cb)(struct of_endpoint *ep, void *data); } int of_register_port(struct device *node,
[PATCH v3 26/48] v4l: Add support for DV timings ioctls on subdev nodes
Validate the pad field in the core code whenever specified. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- .../DocBook/media/v4l/vidioc-dv-timings-cap.xml| 27 +++ .../DocBook/media/v4l/vidioc-enum-dv-timings.xml | 30 +- drivers/media/v4l2-core/v4l2-subdev.c | 27 +++ include/uapi/linux/v4l2-subdev.h | 5 4 files changed, 77 insertions(+), 12 deletions(-) diff --git a/Documentation/DocBook/media/v4l/vidioc-dv-timings-cap.xml b/Documentation/DocBook/media/v4l/vidioc-dv-timings-cap.xml index cd7720d..28a8c1e 100644 --- a/Documentation/DocBook/media/v4l/vidioc-dv-timings-cap.xml +++ b/Documentation/DocBook/media/v4l/vidioc-dv-timings-cap.xml @@ -1,11 +1,12 @@ refentry id=vidioc-dv-timings-cap refmeta -refentrytitleioctl VIDIOC_DV_TIMINGS_CAP/refentrytitle +refentrytitleioctl VIDIOC_DV_TIMINGS_CAP, VIDIOC_SUBDEV_DV_TIMINGS_CAP/refentrytitle manvol; /refmeta refnamediv refnameVIDIOC_DV_TIMINGS_CAP/refname +refnameVIDIOC_SUBDEV_DV_TIMINGS_CAP/refname refpurposeThe capabilities of the Digital Video receiver/transmitter/refpurpose /refnamediv @@ -33,7 +34,7 @@ varlistentry termparameterrequest/parameter/term listitem - paraVIDIOC_DV_TIMINGS_CAP/para + paraVIDIOC_DV_TIMINGS_CAP, VIDIOC_SUBDEV_DV_TIMINGS_CAP/para /listitem /varlistentry varlistentry @@ -54,10 +55,19 @@ interface and may change in the future./para /note -paraTo query the capabilities of the DV receiver/transmitter applications can call -this ioctl and the driver will fill in the structure. Note that drivers may return +paraTo query the capabilities of the DV receiver/transmitter applications +can call the constantVIDIOC_DV_TIMINGS_CAP/constant ioctl on a video node +and the driver will fill in the structure. Note that drivers may return different values after switching the video input or output./para +paraWhen implemented by the driver DV capabilities of subdevices can be +queried by calling the constantVIDIOC_SUBDEV_DV_TIMINGS_CAP/constant ioctl +directly on a subdevice node. The capabilities are specific to inputs (for DV +receivers) or outputs (for DV transmitters), applications must specify the +desired pad number in the v4l2-dv-timings-cap; structfieldpad/structfield +field. Attempts to query capabilities on a pad that doesn't support them will +return an EINVAL;./para + table pgwide=1 frame=none id=v4l2-bt-timings-cap titlestruct structnamev4l2_bt_timings_cap/structname/title tgroup cols=3 @@ -127,7 +137,14 @@ different values after switching the video input or output./para /row row entry__u32/entry - entrystructfieldreserved/structfield[3]/entry + entrystructfieldpad/structfield/entry + entryPad number as reported by the media controller API. This field + is only used when operating on a subdevice node. When operating on a + video node applications must set this field to zero./entry + /row + row + entry__u32/entry + entrystructfieldreserved/structfield[2]/entry entryReserved for future extensions. Drivers must set the array to zero./entry /row row diff --git a/Documentation/DocBook/media/v4l/vidioc-enum-dv-timings.xml b/Documentation/DocBook/media/v4l/vidioc-enum-dv-timings.xml index b3e17c1..b9fdfea 100644 --- a/Documentation/DocBook/media/v4l/vidioc-enum-dv-timings.xml +++ b/Documentation/DocBook/media/v4l/vidioc-enum-dv-timings.xml @@ -1,11 +1,12 @@ refentry id=vidioc-enum-dv-timings refmeta -refentrytitleioctl VIDIOC_ENUM_DV_TIMINGS/refentrytitle +refentrytitleioctl VIDIOC_ENUM_DV_TIMINGS, VIDIOC_SUBDEV_ENUM_DV_TIMINGS/refentrytitle manvol; /refmeta refnamediv refnameVIDIOC_ENUM_DV_TIMINGS/refname +refnameVIDIOC_SUBDEV_ENUM_DV_TIMINGS/refname refpurposeEnumerate supported Digital Video timings/refpurpose /refnamediv @@ -33,7 +34,7 @@ varlistentry termparameterrequest/parameter/term listitem - paraVIDIOC_ENUM_DV_TIMINGS/para + paraVIDIOC_ENUM_DV_TIMINGS, VIDIOC_SUBDEV_ENUM_DV_TIMINGS/para /listitem /varlistentry varlistentry @@ -61,14 +62,21 @@ standards or even custom timings that are not in this list./para paraTo query the available timings, applications initialize the structfieldindex/structfield field and zero the reserved array of v4l2-enum-dv-timings; -and call the constantVIDIOC_ENUM_DV_TIMINGS/constant ioctl with a pointer to this -structure. Drivers fill the rest of the structure or return an +and call the constantVIDIOC_ENUM_DV_TIMINGS/constant ioctl on a video node with a +pointer to this structure. Drivers fill the rest of the structure or return an EINVAL; when the index is out of
[PATCH v3 36/48] adv7604: Make output format configurable through pad format operations
Replace the dummy video format operations by pad format operations that configure the output format. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/i2c/adv7604.c | 280 include/media/adv7604.h | 56 - 2 files changed, 275 insertions(+), 61 deletions(-) diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index 851b350..5aa7c29 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -53,6 +53,28 @@ MODULE_LICENSE(GPL); /* ADV7604 system clock frequency */ #define ADV7604_fsc (28636360) +#define ADV7604_RGB_OUT(1 1) + +#define ADV7604_OP_FORMAT_SEL_8BIT (0 0) +#define ADV7604_OP_FORMAT_SEL_10BIT(1 0) +#define ADV7604_OP_FORMAT_SEL_12BIT(2 0) + +#define ADV7604_OP_MODE_SEL_SDR_422(0 5) +#define ADV7604_OP_MODE_SEL_DDR_422(1 5) +#define ADV7604_OP_MODE_SEL_SDR_444(2 5) +#define ADV7604_OP_MODE_SEL_DDR_444(3 5) +#define ADV7604_OP_MODE_SEL_SDR_422_2X (4 5) +#define ADV7604_OP_MODE_SEL_ADI_CM (5 5) + +#define ADV7604_OP_CH_SEL_GBR (0 5) +#define ADV7604_OP_CH_SEL_GRB (1 5) +#define ADV7604_OP_CH_SEL_BGR (2 5) +#define ADV7604_OP_CH_SEL_RGB (3 5) +#define ADV7604_OP_CH_SEL_BRG (4 5) +#define ADV7604_OP_CH_SEL_RBG (5 5) + +#define ADV7604_OP_SWAP_CB_CR (1 0) + enum adv7604_type { ADV7604, ADV7611, @@ -63,6 +85,14 @@ struct adv7604_reg_seq { u8 val; }; +struct adv7604_format_info { + enum v4l2_mbus_pixelcode code; + u8 op_ch_sel; + bool rgb_out; + bool swap_cb_cr; + u8 op_format_sel; +}; + struct adv7604_chip_info { enum adv7604_type type; @@ -78,6 +108,9 @@ struct adv7604_chip_info { unsigned int tdms_lock_mask; unsigned int fmt_change_digital_mask; + const struct adv7604_format_info *formats; + unsigned int nformats; + void (*set_termination)(struct v4l2_subdev *sd, bool enable); void (*setup_irqs)(struct v4l2_subdev *sd); unsigned int (*read_hdmi_pixelclock)(struct v4l2_subdev *sd); @@ -101,12 +134,18 @@ struct adv7604_chip_info { struct adv7604_state { const struct adv7604_chip_info *info; struct adv7604_platform_data pdata; + struct v4l2_subdev sd; struct media_pad pads[ADV7604_PAD_MAX]; unsigned int source_pad; + struct v4l2_ctrl_handler hdl; + enum adv7604_pad selected_input; + struct v4l2_dv_timings timings; + const struct adv7604_format_info *format; + struct { u8 edid[256]; u32 present; @@ -771,6 +810,93 @@ static void adv7604_write_reg_seq(struct v4l2_subdev *sd, adv7604_write_reg(sd, reg_seq[i].reg, reg_seq[i].val); } +/* - + * Format helpers + */ + +static const struct adv7604_format_info adv7604_formats[] = { + { V4L2_MBUS_FMT_RGB888_1X24, ADV7604_OP_CH_SEL_RGB, true, false, + ADV7604_OP_MODE_SEL_SDR_444 | ADV7604_OP_FORMAT_SEL_8BIT }, + { V4L2_MBUS_FMT_YUYV8_2X8, ADV7604_OP_CH_SEL_RGB, false, false, + ADV7604_OP_MODE_SEL_SDR_422 | ADV7604_OP_FORMAT_SEL_8BIT }, + { V4L2_MBUS_FMT_YVYU8_2X8, ADV7604_OP_CH_SEL_RGB, false, true, + ADV7604_OP_MODE_SEL_SDR_422 | ADV7604_OP_FORMAT_SEL_8BIT }, + { V4L2_MBUS_FMT_YUYV10_2X10, ADV7604_OP_CH_SEL_RGB, false, false, + ADV7604_OP_MODE_SEL_SDR_422 | ADV7604_OP_FORMAT_SEL_10BIT }, + { V4L2_MBUS_FMT_YVYU10_2X10, ADV7604_OP_CH_SEL_RGB, false, true, + ADV7604_OP_MODE_SEL_SDR_422 | ADV7604_OP_FORMAT_SEL_10BIT }, + { V4L2_MBUS_FMT_YUYV12_2X12, ADV7604_OP_CH_SEL_RGB, false, false, + ADV7604_OP_MODE_SEL_SDR_422 | ADV7604_OP_FORMAT_SEL_12BIT }, + { V4L2_MBUS_FMT_YVYU12_2X12, ADV7604_OP_CH_SEL_RGB, false, true, + ADV7604_OP_MODE_SEL_SDR_422 | ADV7604_OP_FORMAT_SEL_12BIT }, + { V4L2_MBUS_FMT_UYVY8_1X16, ADV7604_OP_CH_SEL_RBG, false, false, + ADV7604_OP_MODE_SEL_SDR_422_2X | ADV7604_OP_FORMAT_SEL_8BIT }, + { V4L2_MBUS_FMT_VYUY8_1X16, ADV7604_OP_CH_SEL_RBG, false, true, + ADV7604_OP_MODE_SEL_SDR_422_2X | ADV7604_OP_FORMAT_SEL_8BIT }, + { V4L2_MBUS_FMT_YUYV8_1X16, ADV7604_OP_CH_SEL_RGB, false, false, + ADV7604_OP_MODE_SEL_SDR_422_2X | ADV7604_OP_FORMAT_SEL_8BIT }, + { V4L2_MBUS_FMT_YVYU8_1X16, ADV7604_OP_CH_SEL_RGB, false, true, + ADV7604_OP_MODE_SEL_SDR_422_2X | ADV7604_OP_FORMAT_SEL_8BIT }, + { V4L2_MBUS_FMT_UYVY10_1X20, ADV7604_OP_CH_SEL_RBG, false, false,
[PATCH v3 27/48] v4l: Validate fields in the core code for subdev EDID ioctls
The subdev EDID ioctls receive a pad field that must reference an existing pad and an EDID field that must point to a buffer. Validate both fields in the core code instead of duplicating validation in all drivers. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/ad9389b.c | 2 -- drivers/media/i2c/adv7511.c | 2 -- drivers/media/i2c/adv7604.c | 4 drivers/media/i2c/adv7842.c | 4 drivers/media/v4l2-core/v4l2-subdev.c | 24 5 files changed, 20 insertions(+), 16 deletions(-) diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c index 4cdff9e..5b78828 100644 --- a/drivers/media/i2c/ad9389b.c +++ b/drivers/media/i2c/ad9389b.c @@ -683,8 +683,6 @@ static int ad9389b_get_edid(struct v4l2_subdev *sd, return -EINVAL; if (edid-blocks == 0 || edid-blocks 256) return -EINVAL; - if (!edid-edid) - return -EINVAL; if (!state-edid.segments) { v4l2_dbg(1, debug, sd, EDID segment 0 not found\n); return -ENODATA; diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c index de7ddf5..ff1c2cd 100644 --- a/drivers/media/i2c/adv7511.c +++ b/drivers/media/i2c/adv7511.c @@ -784,8 +784,6 @@ static int adv7511_get_edid(struct v4l2_subdev *sd, return -EINVAL; if ((edid-blocks == 0) || (edid-blocks 256)) return -EINVAL; - if (!edid-edid) - return -EINVAL; if (!state-edid.segments) { v4l2_dbg(1, debug, sd, EDID segment 0 not found\n); return -ENODATA; diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index 71c8570..de3db42 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -1673,8 +1673,6 @@ static int adv7604_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi return -EINVAL; if (edid-start_block == 1) edid-blocks = 1; - if (!edid-edid) - return -EINVAL; if (edid-blocks state-edid.blocks) edid-blocks = state-edid.blocks; @@ -1761,8 +1759,6 @@ static int adv7604_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi edid-blocks = 2; return -E2BIG; } - if (!edid-edid) - return -EINVAL; v4l2_dbg(2, debug, sd, %s: write EDID pad %d, edid.present = 0x%x\n, __func__, edid-pad, state-edid.present); diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c index a319275..3c5a7d9 100644 --- a/drivers/media/i2c/adv7842.c +++ b/drivers/media/i2c/adv7842.c @@ -2035,8 +2035,6 @@ static int adv7842_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi return -EINVAL; if (edid-start_block == 1) edid-blocks = 1; - if (!edid-edid) - return -EINVAL; switch (edid-pad) { case ADV7842_EDID_PORT_A: @@ -2071,8 +2069,6 @@ static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *e) return -EINVAL; if (e-blocks 2) return -E2BIG; - if (!e-edid) - return -EINVAL; /* todo, per edid */ state-aspect_ratio = v4l2_calc_aspect_ratio(e-edid[0x15], diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 853fb84..f6185f9 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -349,11 +349,27 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) sd, pad, set_selection, subdev_fh, sel); } - case VIDIOC_SUBDEV_G_EDID: - return v4l2_subdev_call(sd, pad, get_edid, arg); + case VIDIOC_SUBDEV_G_EDID: { + struct v4l2_subdev_edid *edid = arg; - case VIDIOC_SUBDEV_S_EDID: - return v4l2_subdev_call(sd, pad, set_edid, arg); + if (edid-pad = sd-entity.num_pads) + return -EINVAL; + if (edid-blocks edid-edid == NULL) + return -EINVAL; + + return v4l2_subdev_call(sd, pad, get_edid, edid); + } + + case VIDIOC_SUBDEV_S_EDID: { + struct v4l2_subdev_edid *edid = arg; + + if (edid-pad = sd-entity.num_pads) + return -EINVAL; + if (edid-blocks edid-edid == NULL) + return -EINVAL; + + return v4l2_subdev_call(sd, pad, set_edid, edid); + } case VIDIOC_SUBDEV_DV_TIMINGS_CAP: { struct v4l2_dv_timings_cap *cap = arg; -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to
[PATCH v3 42/48] adv7604: Replace *_and_or() functions with *_clr_set()
The *_and_or() functions take an 'and' bitmask to be ANDed with the register value before ORing it with th 'or' bitmask. As the functions are used to mask and set bits selectively, this requires the caller to invert the 'and' bitmask and is thus error prone. Replace those functions with a *_clr_set() variant that takes a mask of bits to be cleared instead of a mask of bits to be kept. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/i2c/adv7604.c | 86 ++--- 1 file changed, 43 insertions(+), 43 deletions(-) Changes since v2: - Rebased on top of 36/48 v3. diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index 4421fea..ed29d02 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -429,9 +429,9 @@ static inline int io_write(struct v4l2_subdev *sd, u8 reg, u8 val) return adv_smbus_write_byte_data(state, ADV7604_PAGE_IO, reg, val); } -static inline int io_write_and_or(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 val) +static inline int io_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 val) { - return io_write(sd, reg, (io_read(sd, reg) mask) | val); + return io_write(sd, reg, (io_read(sd, reg) ~mask) | val); } static inline int avlink_read(struct v4l2_subdev *sd, u8 reg) @@ -462,9 +462,9 @@ static inline int cec_write(struct v4l2_subdev *sd, u8 reg, u8 val) return adv_smbus_write_byte_data(state, ADV7604_PAGE_CEC, reg, val); } -static inline int cec_write_and_or(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 val) +static inline int cec_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 val) { - return cec_write(sd, reg, (cec_read(sd, reg) mask) | val); + return cec_write(sd, reg, (cec_read(sd, reg) ~mask) | val); } static inline int infoframe_read(struct v4l2_subdev *sd, u8 reg) @@ -538,9 +538,9 @@ static inline int rep_write(struct v4l2_subdev *sd, u8 reg, u8 val) return adv_smbus_write_byte_data(state, ADV7604_PAGE_REP, reg, val); } -static inline int rep_write_and_or(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 val) +static inline int rep_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 val) { - return rep_write(sd, reg, (rep_read(sd, reg) mask) | val); + return rep_write(sd, reg, (rep_read(sd, reg) ~mask) | val); } static inline int edid_read(struct v4l2_subdev *sd, u8 reg) @@ -629,9 +629,9 @@ static inline int hdmi_write(struct v4l2_subdev *sd, u8 reg, u8 val) return adv_smbus_write_byte_data(state, ADV7604_PAGE_HDMI, reg, val); } -static inline int hdmi_write_and_or(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 val) +static inline int hdmi_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 val) { - return hdmi_write(sd, reg, (hdmi_read(sd, reg) mask) | val); + return hdmi_write(sd, reg, (hdmi_read(sd, reg) ~mask) | val); } static inline int test_read(struct v4l2_subdev *sd, u8 reg) @@ -667,9 +667,9 @@ static inline int cp_write(struct v4l2_subdev *sd, u8 reg, u8 val) return adv_smbus_write_byte_data(state, ADV7604_PAGE_CP, reg, val); } -static inline int cp_write_and_or(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 val) +static inline int cp_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 val) { - return cp_write(sd, reg, (cp_read(sd, reg) mask) | val); + return cp_write(sd, reg, (cp_read(sd, reg) ~mask) | val); } static inline int vdp_read(struct v4l2_subdev *sd, u8 reg) @@ -947,7 +947,7 @@ static int configure_predefined_video_timings(struct v4l2_subdev *sd, io_write(sd, 0x17, 0x5a); } /* disable embedded syncs for auto graphics mode */ - cp_write_and_or(sd, 0x81, 0xef, 0x00); + cp_write_clr_set(sd, 0x81, 0x10, 0x00); cp_write(sd, 0x8f, 0x00); cp_write(sd, 0x90, 0x00); cp_write(sd, 0xa2, 0x00); @@ -1005,7 +1005,7 @@ static void configure_custom_video_timings(struct v4l2_subdev *sd, io_write(sd, 0x00, 0x07); /* video std */ io_write(sd, 0x01, 0x02); /* prim mode */ /* enable embedded syncs for auto graphics mode */ - cp_write_and_or(sd, 0x81, 0xef, 0x10); + cp_write_clr_set(sd, 0x81, 0x10, 0x10); /* Should only be set in auto-graphics mode [REF_02, p. 91-92] */ /* setup PLL_DIV_MAN_EN and PLL_DIV_RATIO */ @@ -1115,21 +1115,21 @@ static void set_rgb_quantization_range(struct v4l2_subdev *sd) if (state-selected_input == ADV7604_PAD_VGA_RGB) { /* Receiving analog RGB signal * Set RGB full range (0-255) */ - io_write_and_or(sd, 0x02, 0x0f, 0x10); + io_write_clr_set(sd, 0x02, 0xf0, 0x10); break; }
[PATCH v3 46/48] adv7604: Add DT support
Parse the device tree node to populate platform data. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Hans Verkuil hans.verk...@cisco.com --- .../devicetree/bindings/media/i2c/adv7604.txt | 56 + drivers/media/i2c/adv7604.c| 92 ++ 2 files changed, 134 insertions(+), 14 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/i2c/adv7604.txt Changes since v2: - Rebased on top of 36/48 v3. diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt b/Documentation/devicetree/bindings/media/i2c/adv7604.txt new file mode 100644 index 000..0845c50 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt @@ -0,0 +1,56 @@ +* Analog Devices ADV7604/11 video decoder with HDMI receiver + +The ADV7604 and ADV7611 are multiformat video decoders with an integrated HDMI +receiver. The ADV7604 has four multiplexed HDMI inputs and one analog input, +and the ADV7611 has one HDMI input and no analog input. + +Required Properties: + + - compatible: Must contain one of the following +- adi,adv7604 for the ADV7604 +- adi,adv7611 for the ADV7611 + + - reg: I2C slave address + + - hpd-gpios: References to the GPIOs that control the HDMI hot-plug +detection pins, one per HDMI input. The active flag indicates the GPIO +level that enables hot-plug detection. + +Optional Properties: + + - reset-gpios: Reference to the GPIO connected to the device's reset pin. + + - adi,default-input: Index of the input to be configured as default. Valid +values are 0..5 for the ADV7604 and 0 for the ADV7611. + + - adi,disable-power-down: Boolean property. When set forces the device to +ignore the power-down pin. The property is valid for the ADV7604 only as +the ADV7611 has no power-down pin. + + - adi,disable-cable-reset: Boolean property. When set disables the HDMI +receiver automatic reset when the HDMI cable is unplugged. + +Example: + + hdmi_receiver@4c { + compatible = adi,adv7611; + reg = 0x4c; + + reset-gpios = ioexp 0 GPIO_ACTIVE_LOW; + hpd-gpios = ioexp 2 GPIO_ACTIVE_HIGH; + + adi,default-input = 0; + + #address-cells = 1; + #size-cells = 0; + + port@0 { + reg = 0; + }; + port@1 { + reg = 1; + hdmi_in: endpoint { + remote-endpoint = ccdc_in; + }; + }; + }; diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index b352b62..5901b78 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -2663,13 +2663,70 @@ static const struct adv7604_chip_info adv7604_chip_info[] = { }, }; +static struct i2c_device_id adv7604_i2c_id[] = { + { adv7604, (kernel_ulong_t)adv7604_chip_info[ADV7604] }, + { adv7611, (kernel_ulong_t)adv7604_chip_info[ADV7611] }, + { } +}; +MODULE_DEVICE_TABLE(i2c, adv7604_i2c_id); + +static struct of_device_id adv7604_of_id[] = { + { .compatible = adi,adv7604, .data = adv7604_chip_info[ADV7604] }, + { .compatible = adi,adv7611, .data = adv7604_chip_info[ADV7611] }, + { } +}; +MODULE_DEVICE_TABLE(of, adv7604_of_id); + +static int adv7604_parse_dt(struct adv7604_state *state) +{ + struct device_node *np; + int ret; + + np = state-i2c_clients[ADV7604_PAGE_IO]-dev.of_node; + + state-pdata.disable_pwrdnb = + of_property_read_bool(np, adi,disable-power-down); + state-pdata.disable_cable_det_rst = + of_property_read_bool(np, adi,disable-cable-reset); + + ret = of_property_read_u32(np, adi,default-input, + state-pdata.default_input); + if (ret 0) + state-pdata.default_input = -1; + + /* Disable the interrupt for now as no DT-based board uses it. */ + state-pdata.int1_config = ADV7604_INT1_CONFIG_DISABLED; + + /* Use the default I2C addresses. */ + state-pdata.i2c_addresses[ADV7604_PAGE_AVLINK] = 0x42; + state-pdata.i2c_addresses[ADV7604_PAGE_CEC] = 0x40; + state-pdata.i2c_addresses[ADV7604_PAGE_INFOFRAME] = 0x3e; + state-pdata.i2c_addresses[ADV7604_PAGE_ESDP] = 0x38; + state-pdata.i2c_addresses[ADV7604_PAGE_DPP] = 0x3c; + state-pdata.i2c_addresses[ADV7604_PAGE_AFE] = 0x26; + state-pdata.i2c_addresses[ADV7604_PAGE_REP] = 0x32; + state-pdata.i2c_addresses[ADV7604_PAGE_EDID] = 0x36; + state-pdata.i2c_addresses[ADV7604_PAGE_HDMI] = 0x34; + state-pdata.i2c_addresses[ADV7604_PAGE_TEST] = 0x30; + state-pdata.i2c_addresses[ADV7604_PAGE_CP] = 0x22; + state-pdata.i2c_addresses[ADV7604_PAGE_VDP] = 0x24; + + /* HACK: Hardcode the remaining platform data fields. */ + state-pdata.blank_data
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
Hi Philipp, On Tuesday 11 March 2014 16:07:00 Philipp Zabel wrote: Am Montag, den 10.03.2014, 14:58 + schrieb Grant Likely: On Mon, 10 Mar 2014 14:52:53 +0100, Laurent Pinchart wrote: [snip] In theory unidirectional links in DT are indeed enough. However, let's not forget the following. - There's no such thing as single start points for graphs. Sure, in some simple cases the graph will have a single start point, but that's not a generic rule. For instance the camera graphs http://ideasonboard.org/media/omap3isp.ps and http://ideasonboard.org/media/eyecam.ps have two camera sensors, and thus two starting points from a data flow point of view. And if you want a better understanding of how complex media graphs can become, have a look at http://ideasonboard.org/media/vsp1.0.pdf (that's a real world example, albeit all connections are internal to the SoC in that particular case, and don't need to be described in DT). - There's also no such thing as a master device that can just point to slave devices. Once again simple cases exist where that model could work, but real world examples exist of complex pipelines with dozens of elements all implemented by a separate IP core and handled by separate drivers, forming a graph with long chains and branches. We thus need real graph bindings. - Finally, having no backlinks in DT would make the software implementation very complex. We need to be able to walk the graph in a generic way without having any of the IP core drivers loaded, and without any specific starting point. We would thus need to parse the complete DT tree, looking at all nodes and trying to find out whether they're part of the graph we're trying to walk. The complexity of the operation would be at best quadratic to the number of nodes in the whole DT and to the number of nodes in the graph. Not really. To being with, you cannot determine any meaning of a node across the tree (aside from it being an endpoint) without also understanding the binding that the node is a part of. That means you need to have something matching against the compatible string on both ends of the linkage. For instance: panel { compatible = acme,lvds-panel; lvds-port: port { }; }; display-controller { compatible = encom,video; port { remote-endpoint = lvds-port; }; }; In the above example, the encom,video driver has absolutely zero information about what the acme,lvds-panel binding actually implements. There needs to be both a driver for the acme,lvds-panel binding and one for the encom,video binding (even if the acme,lvds-panel binding is very thin and defers the functionality to the video controller). What you want here is the drivers to register each side of the connection. That could be modeled with something like the following (pseudocode): struct of_endpoint { struct list_head list; struct device_node *ep_node; void *context; void (*cb)(struct of_endpoint *ep, void *data); } int of_register_port(struct device *node, void (*cb)(struct of_endpoint *ep, void *data), void *data) { struct of_endpoint *ep = kzalloc(sizeof(*ep), GFP_KERNEL); ep-ep_node = node; ep-data = data; ep-callback = cb; /* store the endpoint to a list */ /* check if the endpoint has a remote-endpoint link */ /* If so, then link the two together and call the * callbacks */ } That's neither expensive or complicated. Originally I suggested walking the whole tree multiple times, but as mentioned that doesn't scale, and as I thought about the above it isn't even a valid thing to do. Everything has to be driven by drivers, so even if the backlinks are there, nothing can be done with the link until the other side goes through enumeration independently. I have implemented your suggestion as follows. Basically, this allows either endpoint to contain the remote-endpoint link, as long as all drivers register their endpoints in the probe function and return -EPROBE_DEFER from their component framework bind callback until all their endpoints are connected. Beside bringing the whole graph down when a single component can't be probed (either because the corresponding hardware devices is missing, broken, or the driver isn't loaded), that's adding even one more level of complexity with an additional callback. I'm afraid I can't accept it as-is, the result is just too complex for device drivers and not flexible enough. I want to keep the ability to walk the graph without requiring all components to be probed by their respective driver. What happened to your suggestion of parsing the whole DT once at boot time ? From fdda1fb2bd133200d4620adcbb28697cb360e1cb Mon Sep 17 00:00:00 2001 From: Philipp Zabel p.za...@pengutronix.de
[linuxtv-media:master 448/499] ERROR: __divdi3 [drivers/media/dvb-frontends/drx39xyj/drx39xyj.ko] undefined!
tree: git://linuxtv.org/media_tree.git master head: 7b802ce7e8c67510389fdbbe29edd87a75df3a93 commit: 03fdfbfd3b5944bfd210541a83c9b222e2c20920 [448/499] [media] drx-j: Prepare to use DVBv5 stats config: make ARCH=m68k allmodconfig Note: the linuxtv-media/master HEAD 7b802ce7e8c67510389fdbbe29edd87a75df3a93 builds fine. It only hurts bisectibility. All error/warnings: ERROR: __divdi3 [drivers/media/dvb-frontends/drx39xyj/drx39xyj.ko] undefined! --- 0-DAY kernel build testing backend Open Source Technology Center http://lists.01.org/mailman/listinfo/kbuild Intel Corporation -- 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 v3 26/48] v4l: Add support for DV timings ioctls on subdev nodes
On 03/11/2014 04:09 PM, Laurent Pinchart wrote: Validate the pad field in the core code whenever specified. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com Regards, Hans --- .../DocBook/media/v4l/vidioc-dv-timings-cap.xml| 27 +++ .../DocBook/media/v4l/vidioc-enum-dv-timings.xml | 30 +- drivers/media/v4l2-core/v4l2-subdev.c | 27 +++ include/uapi/linux/v4l2-subdev.h | 5 4 files changed, 77 insertions(+), 12 deletions(-) diff --git a/Documentation/DocBook/media/v4l/vidioc-dv-timings-cap.xml b/Documentation/DocBook/media/v4l/vidioc-dv-timings-cap.xml index cd7720d..28a8c1e 100644 --- a/Documentation/DocBook/media/v4l/vidioc-dv-timings-cap.xml +++ b/Documentation/DocBook/media/v4l/vidioc-dv-timings-cap.xml @@ -1,11 +1,12 @@ refentry id=vidioc-dv-timings-cap refmeta -refentrytitleioctl VIDIOC_DV_TIMINGS_CAP/refentrytitle +refentrytitleioctl VIDIOC_DV_TIMINGS_CAP, VIDIOC_SUBDEV_DV_TIMINGS_CAP/refentrytitle manvol; /refmeta refnamediv refnameVIDIOC_DV_TIMINGS_CAP/refname +refnameVIDIOC_SUBDEV_DV_TIMINGS_CAP/refname refpurposeThe capabilities of the Digital Video receiver/transmitter/refpurpose /refnamediv @@ -33,7 +34,7 @@ varlistentry termparameterrequest/parameter/term listitem - paraVIDIOC_DV_TIMINGS_CAP/para + paraVIDIOC_DV_TIMINGS_CAP, VIDIOC_SUBDEV_DV_TIMINGS_CAP/para /listitem /varlistentry varlistentry @@ -54,10 +55,19 @@ interface and may change in the future./para /note -paraTo query the capabilities of the DV receiver/transmitter applications can call -this ioctl and the driver will fill in the structure. Note that drivers may return +paraTo query the capabilities of the DV receiver/transmitter applications +can call the constantVIDIOC_DV_TIMINGS_CAP/constant ioctl on a video node +and the driver will fill in the structure. Note that drivers may return different values after switching the video input or output./para +paraWhen implemented by the driver DV capabilities of subdevices can be +queried by calling the constantVIDIOC_SUBDEV_DV_TIMINGS_CAP/constant ioctl +directly on a subdevice node. The capabilities are specific to inputs (for DV +receivers) or outputs (for DV transmitters), applications must specify the +desired pad number in the v4l2-dv-timings-cap; structfieldpad/structfield +field. Attempts to query capabilities on a pad that doesn't support them will +return an EINVAL;./para + table pgwide=1 frame=none id=v4l2-bt-timings-cap titlestruct structnamev4l2_bt_timings_cap/structname/title tgroup cols=3 @@ -127,7 +137,14 @@ different values after switching the video input or output./para /row row entry__u32/entry - entrystructfieldreserved/structfield[3]/entry + entrystructfieldpad/structfield/entry + entryPad number as reported by the media controller API. This field + is only used when operating on a subdevice node. When operating on a + video node applications must set this field to zero./entry + /row + row + entry__u32/entry + entrystructfieldreserved/structfield[2]/entry entryReserved for future extensions. Drivers must set the array to zero./entry /row row diff --git a/Documentation/DocBook/media/v4l/vidioc-enum-dv-timings.xml b/Documentation/DocBook/media/v4l/vidioc-enum-dv-timings.xml index b3e17c1..b9fdfea 100644 --- a/Documentation/DocBook/media/v4l/vidioc-enum-dv-timings.xml +++ b/Documentation/DocBook/media/v4l/vidioc-enum-dv-timings.xml @@ -1,11 +1,12 @@ refentry id=vidioc-enum-dv-timings refmeta -refentrytitleioctl VIDIOC_ENUM_DV_TIMINGS/refentrytitle +refentrytitleioctl VIDIOC_ENUM_DV_TIMINGS, VIDIOC_SUBDEV_ENUM_DV_TIMINGS/refentrytitle manvol; /refmeta refnamediv refnameVIDIOC_ENUM_DV_TIMINGS/refname +refnameVIDIOC_SUBDEV_ENUM_DV_TIMINGS/refname refpurposeEnumerate supported Digital Video timings/refpurpose /refnamediv @@ -33,7 +34,7 @@ varlistentry termparameterrequest/parameter/term listitem - paraVIDIOC_ENUM_DV_TIMINGS/para + paraVIDIOC_ENUM_DV_TIMINGS, VIDIOC_SUBDEV_ENUM_DV_TIMINGS/para /listitem /varlistentry varlistentry @@ -61,14 +62,21 @@ standards or even custom timings that are not in this list./para paraTo query the available timings, applications initialize the structfieldindex/structfield field and zero the reserved array of v4l2-enum-dv-timings; -and call the constantVIDIOC_ENUM_DV_TIMINGS/constant ioctl with a pointer to this -structure. Drivers fill the rest of the structure or return an
[GIT PULL] m88ds3103 fixes
I just noticed from patchwork these old patches are pending. It appears I have not pull requested or mail is just missed from the reason or the other. They are just fine for 3.15, but m88ds3103: fix bug on .set_tone() is stuff for 3.14 too. I know it is very late, but given the fact it fixes existing bug and that driver has gone to 3.14 I hope that one patch could be sent to 3.14. regards Antti The following changes since commit 587d1b06e07b4a079453c74ba9edf17d21931049: [media] rc-core: reuse device numbers (2014-01-15 11:46:37 -0200) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git pctv_461e for you to fetch changes up to e0d125fb17ac2bb5a992d2d761d1a0a2b42546aa: m88ds3103: fix bug on .set_tone() (2014-02-01 22:28:21 +0200) Antti Palosaari (4): m88ds3103: remove dead code m88ds3103: remove dead code 2nd part m88ds3103: possible uninitialized scalar variable m88ds3103: fix bug on .set_tone() drivers/media/dvb-frontends/m88ds3103.c | 30 -- 1 file changed, 8 insertions(+), 22 deletions(-) -- 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 v3 27/48] v4l: Validate fields in the core code for subdev EDID ioctls
On 03/11/2014 04:09 PM, Laurent Pinchart wrote: The subdev EDID ioctls receive a pad field that must reference an existing pad and an EDID field that must point to a buffer. Validate both fields in the core code instead of duplicating validation in all drivers. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Sakari Ailus sakari.ai...@linux.intel.com Here is my: Reviewed-by: Hans Verkuil hans.verk...@cisco.com But take note: the adv7604 driver does not handle a get_edid with edid-blocks == 0 correctly: it should fill in the blocks field with the real number of blocks and return 0 instead of returning EINVAL. I also read through the spec again and it does not actually explicitly say that you can use G_EDID with edid-blocks == 0, but I think it makes a lot of sense to do that. All existing drivers that use get_edid all return -EINVAL if blocks == 0, so this patch does not change anything with that. I plan on making a patch to clarify the spec and update the drivers, but you might want to make a patch for adv7604 yourself instead of waiting for me. I leave that up to you. Anyway, this patch is fine. Regards, Hans --- drivers/media/i2c/ad9389b.c | 2 -- drivers/media/i2c/adv7511.c | 2 -- drivers/media/i2c/adv7604.c | 4 drivers/media/i2c/adv7842.c | 4 drivers/media/v4l2-core/v4l2-subdev.c | 24 5 files changed, 20 insertions(+), 16 deletions(-) diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c index 4cdff9e..5b78828 100644 --- a/drivers/media/i2c/ad9389b.c +++ b/drivers/media/i2c/ad9389b.c @@ -683,8 +683,6 @@ static int ad9389b_get_edid(struct v4l2_subdev *sd, return -EINVAL; if (edid-blocks == 0 || edid-blocks 256) return -EINVAL; - if (!edid-edid) - return -EINVAL; if (!state-edid.segments) { v4l2_dbg(1, debug, sd, EDID segment 0 not found\n); return -ENODATA; diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c index de7ddf5..ff1c2cd 100644 --- a/drivers/media/i2c/adv7511.c +++ b/drivers/media/i2c/adv7511.c @@ -784,8 +784,6 @@ static int adv7511_get_edid(struct v4l2_subdev *sd, return -EINVAL; if ((edid-blocks == 0) || (edid-blocks 256)) return -EINVAL; - if (!edid-edid) - return -EINVAL; if (!state-edid.segments) { v4l2_dbg(1, debug, sd, EDID segment 0 not found\n); return -ENODATA; diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index 71c8570..de3db42 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -1673,8 +1673,6 @@ static int adv7604_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi return -EINVAL; if (edid-start_block == 1) edid-blocks = 1; - if (!edid-edid) - return -EINVAL; if (edid-blocks state-edid.blocks) edid-blocks = state-edid.blocks; @@ -1761,8 +1759,6 @@ static int adv7604_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi edid-blocks = 2; return -E2BIG; } - if (!edid-edid) - return -EINVAL; v4l2_dbg(2, debug, sd, %s: write EDID pad %d, edid.present = 0x%x\n, __func__, edid-pad, state-edid.present); diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c index a319275..3c5a7d9 100644 --- a/drivers/media/i2c/adv7842.c +++ b/drivers/media/i2c/adv7842.c @@ -2035,8 +2035,6 @@ static int adv7842_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi return -EINVAL; if (edid-start_block == 1) edid-blocks = 1; - if (!edid-edid) - return -EINVAL; switch (edid-pad) { case ADV7842_EDID_PORT_A: @@ -2071,8 +2069,6 @@ static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *e) return -EINVAL; if (e-blocks 2) return -E2BIG; - if (!e-edid) - return -EINVAL; /* todo, per edid */ state-aspect_ratio = v4l2_calc_aspect_ratio(e-edid[0x15], diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 853fb84..f6185f9 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -349,11 +349,27 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) sd, pad, set_selection, subdev_fh, sel); } - case VIDIOC_SUBDEV_G_EDID: - return v4l2_subdev_call(sd, pad, get_edid, arg); + case VIDIOC_SUBDEV_G_EDID: { + struct v4l2_subdev_edid *edid = arg; - case VIDIOC_SUBDEV_S_EDID: - return v4l2_subdev_call(sd, pad,
[linuxtv-media:master 446/499] drxj.c:undefined reference to `__divdi3'
tree: git://linuxtv.org/media_tree.git master head: c3c2077d9579472b07581ecdaf6cc5a60b1700bc commit: 03fdfbfd3b5944bfd210541a83c9b222e2c20920 [446/499] [media] drx-j: Prepare to use DVBv5 stats config: make ARCH=i386 allyesconfig Note: the linuxtv-media/master HEAD c3c2077d9579472b07581ecdaf6cc5a60b1700bc builds fine. It only hurts bisectibility. All error/warnings: drivers/built-in.o: In function `drx39xxj_read_snr': drxj.c:(.text+0x1277c0d): undefined reference to `__divdi3' --- 0-DAY kernel build testing backend Open Source Technology Center http://lists.01.org/mailman/listinfo/kbuild Intel Corporation -- 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 v7 3/10] Documentation: devicetree: Update Samsung FIMC DT binding
This patch documents following updates of the Exynos4 SoC camera subsystem devicetree binding: - addition of #clock-cells and clock-output-names properties to 'camera' node - these are now needed so the image sensor sub-devices can reference clocks provided by the camera host interface, - dropped a note about required clock-frequency properties at the image sensor nodes; the sensor devices can now control their clock explicitly through the clk API and there is no need to require this property in the camera host interface binding. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com --- Resending only single patch which changed. Changes since v6: - #clock-cells, clock-output-names documented as mandatory properties; - renamed cam_mclk_{a,b} to cam_{a,b}_clkout in the example dts, this now matches changes in exynos4.dtsi further in the patch series; - marked samsung,camclk-out property as deprecated. Changes since v5: - none. Changes since v4: - dropped a requirement of specific order of values in clocks/ clock-names properties (Mark) and reference to clock-names in clock-output-names property description (Mark). --- .../devicetree/bindings/media/samsung-fimc.txt | 46 +--- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt index 96312f6..1908a5f 100644 --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt @@ -15,11 +15,21 @@ Common 'camera' node Required properties: -- compatible : must be samsung,fimc, simple-bus -- clocks : list of clock specifiers, corresponding to entries in - the clock-names property; -- clock-names : must contain sclk_cam0, sclk_cam1, pxl_async0, - pxl_async1 entries, matching entries in the clocks property. +- compatible: must be samsung,fimc, simple-bus +- clocks: list of clock specifiers, corresponding to entries in + the clock-names property; +- clock-names : must contain sclk_cam0, sclk_cam1, pxl_async0, + pxl_async1 entries, matching entries in the clocks property. + +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node and it should + be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively. + +- clock-output-names: from the common clock bindings, should contain names of + clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks respectively. The pinctrl bindings defined in ../pinctrl/pinctrl-bindings.txt must be used to define a required pinctrl state named default and optional pinctrl states: @@ -32,6 +42,7 @@ way around. The 'camera' node must include at least one 'fimc' child node. + 'fimc' device nodes --- @@ -88,8 +99,8 @@ port nodes specifies data input - 0, 1 indicates input A, B respectively. Optional properties -- samsung,camclk-out : specifies clock output for remote sensor, - 0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT; +- samsung,camclk-out (deprecated) : specifies clock output for remote sensor, + 0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT; Image sensor nodes -- @@ -97,8 +108,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller (e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt. -The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensor device +nodes. Default value when this property is not present is 24 MHz. Example: @@ -114,7 +125,7 @@ Example: vddio-supply = ...; clock-frequency = 2400; - clocks = ...; + clocks = camera 1; clock-names = mclk; port { @@ -135,7 +146,7 @@ Example: vddio-supply = ...; clock-frequency = 2400; - clocks = ...; + clocks = camera 0; clock-names = mclk; port { @@ -149,12 +160,17 @@ Example: camera { compatible = samsung,fimc, simple-bus; - #address-cells = 1; - #size-cells = 1; - status = okay; - + clocks = clock 132, clock 133, clock 351, +clock 352; + clock-names = sclk_cam0, sclk_cam1,
Re: [PATCH v3 27/48] v4l: Validate fields in the core code for subdev EDID ioctls
Hi Hans, On Tuesday 11 March 2014 16:44:27 Hans Verkuil wrote: On 03/11/2014 04:09 PM, Laurent Pinchart wrote: The subdev EDID ioctls receive a pad field that must reference an existing pad and an EDID field that must point to a buffer. Validate both fields in the core code instead of duplicating validation in all drivers. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Sakari Ailus sakari.ai...@linux.intel.com Here is my: Reviewed-by: Hans Verkuil hans.verk...@cisco.com But take note: the adv7604 driver does not handle a get_edid with edid-blocks == 0 correctly: it should fill in the blocks field with the real number of blocks and return 0 instead of returning EINVAL. Should it also set edid-start_block to 0 ? I also read through the spec again and it does not actually explicitly say that you can use G_EDID with edid-blocks == 0, but I think it makes a lot of sense to do that. All existing drivers that use get_edid all return -EINVAL if blocks == 0, so this patch does not change anything with that. I plan on making a patch to clarify the spec and update the drivers, but you might want to make a patch for adv7604 yourself instead of waiting for me. I leave that up to you. Anyway, this patch is fine. Regards, Hans --- drivers/media/i2c/ad9389b.c | 2 -- drivers/media/i2c/adv7511.c | 2 -- drivers/media/i2c/adv7604.c | 4 drivers/media/i2c/adv7842.c | 4 drivers/media/v4l2-core/v4l2-subdev.c | 24 5 files changed, 20 insertions(+), 16 deletions(-) diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c index 4cdff9e..5b78828 100644 --- a/drivers/media/i2c/ad9389b.c +++ b/drivers/media/i2c/ad9389b.c @@ -683,8 +683,6 @@ static int ad9389b_get_edid(struct v4l2_subdev *sd, return -EINVAL; if (edid-blocks == 0 || edid-blocks 256) return -EINVAL; - if (!edid-edid) - return -EINVAL; if (!state-edid.segments) { v4l2_dbg(1, debug, sd, EDID segment 0 not found\n); return -ENODATA; diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c index de7ddf5..ff1c2cd 100644 --- a/drivers/media/i2c/adv7511.c +++ b/drivers/media/i2c/adv7511.c @@ -784,8 +784,6 @@ static int adv7511_get_edid(struct v4l2_subdev *sd, return -EINVAL; if ((edid-blocks == 0) || (edid-blocks 256)) return -EINVAL; - if (!edid-edid) - return -EINVAL; if (!state-edid.segments) { v4l2_dbg(1, debug, sd, EDID segment 0 not found\n); return -ENODATA; diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index 71c8570..de3db42 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -1673,8 +1673,6 @@ static int adv7604_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi return -EINVAL; if (edid-start_block == 1) edid-blocks = 1; - if (!edid-edid) - return -EINVAL; if (edid-blocks state-edid.blocks) edid-blocks = state-edid.blocks; @@ -1761,8 +1759,6 @@ static int adv7604_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi edid-blocks = 2; return -E2BIG; } - if (!edid-edid) - return -EINVAL; v4l2_dbg(2, debug, sd, %s: write EDID pad %d, edid.present = 0x%x\n, __func__, edid-pad, state-edid.present); diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c index a319275..3c5a7d9 100644 --- a/drivers/media/i2c/adv7842.c +++ b/drivers/media/i2c/adv7842.c @@ -2035,8 +2035,6 @@ static int adv7842_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi return -EINVAL; if (edid-start_block == 1) edid-blocks = 1; - if (!edid-edid) - return -EINVAL; switch (edid-pad) { case ADV7842_EDID_PORT_A: @@ -2071,8 +2069,6 @@ static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *e) return -EINVAL; if (e-blocks 2) return -E2BIG; - if (!e-edid) - return -EINVAL; /* todo, per edid */ state-aspect_ratio = v4l2_calc_aspect_ratio(e-edid[0x15], diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 853fb84..f6185f9 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -349,11 +349,27 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) sd, pad, set_selection, subdev_fh, sel);
Re: [GIT PULL] m88ds3103 fixes
DROP that request! I will split it to 2 requests, one for 3.14 and one for 3.15. Antti On 11.03.2014 17:42, Antti Palosaari wrote: I just noticed from patchwork these old patches are pending. It appears I have not pull requested or mail is just missed from the reason or the other. They are just fine for 3.15, but m88ds3103: fix bug on .set_tone() is stuff for 3.14 too. I know it is very late, but given the fact it fixes existing bug and that driver has gone to 3.14 I hope that one patch could be sent to 3.14. regards Antti The following changes since commit 587d1b06e07b4a079453c74ba9edf17d21931049: [media] rc-core: reuse device numbers (2014-01-15 11:46:37 -0200) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git pctv_461e for you to fetch changes up to e0d125fb17ac2bb5a992d2d761d1a0a2b42546aa: m88ds3103: fix bug on .set_tone() (2014-02-01 22:28:21 +0200) Antti Palosaari (4): m88ds3103: remove dead code m88ds3103: remove dead code 2nd part m88ds3103: possible uninitialized scalar variable m88ds3103: fix bug on .set_tone() drivers/media/dvb-frontends/m88ds3103.c | 30 -- 1 file changed, 8 insertions(+), 22 deletions(-) -- 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 v3 27/48] v4l: Validate fields in the core code for subdev EDID ioctls
On 03/11/2014 05:08 PM, Laurent Pinchart wrote: Hi Hans, On Tuesday 11 March 2014 16:44:27 Hans Verkuil wrote: On 03/11/2014 04:09 PM, Laurent Pinchart wrote: The subdev EDID ioctls receive a pad field that must reference an existing pad and an EDID field that must point to a buffer. Validate both fields in the core code instead of duplicating validation in all drivers. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Sakari Ailus sakari.ai...@linux.intel.com Here is my: Reviewed-by: Hans Verkuil hans.verk...@cisco.com But take note: the adv7604 driver does not handle a get_edid with edid-blocks == 0 correctly: it should fill in the blocks field with the real number of blocks and return 0 instead of returning EINVAL. Should it also set edid-start_block to 0 ? I don't think so. It makes sense to just set blocks to the total number of available blocks - edid-start_block. Note that if edid-start_block = total number of EDID blocks, then -ENODATA should be returned. 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
[GIT PULL 3.14] m88ds3103 bugfix
I have forgotten to pull request that earlier. It is simple one bit wrong in one register. regards Antti The following changes since commit 81d428cbdb9e630f4424bf81522cd35394beba76: [media] saa7134: fix WARN_ON during resume (2014-03-11 10:17:06 -0300) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git m88ds3103_bugfix_3.14 for you to fetch changes up to 9f0a66dd910d16165a9c3862c86283010e4bf5c0: m88ds3103: fix bug on .set_tone() (2014-03-11 18:08:51 +0200) Antti Palosaari (1): m88ds3103: fix bug on .set_tone() drivers/media/dvb-frontends/m88ds3103.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 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 v7 3/10] Documentation: devicetree: Update Samsung FIMC DT binding
Hi Sylwester, Thank you for the patch. On Tuesday 11 March 2014 17:00:35 Sylwester Nawrocki wrote: This patch documents following updates of the Exynos4 SoC camera subsystem devicetree binding: - addition of #clock-cells and clock-output-names properties to 'camera' node - these are now needed so the image sensor sub-devices can reference clocks provided by the camera host interface, - dropped a note about required clock-frequency properties at the image sensor nodes; the sensor devices can now control their clock explicitly through the clk API and there is no need to require this property in the camera host interface binding. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com --- Resending only single patch which changed. Changes since v6: - #clock-cells, clock-output-names documented as mandatory properties; - renamed cam_mclk_{a,b} to cam_{a,b}_clkout in the example dts, this now matches changes in exynos4.dtsi further in the patch series; - marked samsung,camclk-out property as deprecated. Changes since v5: - none. Changes since v4: - dropped a requirement of specific order of values in clocks/ clock-names properties (Mark) and reference to clock-names in clock-output-names property description (Mark). --- .../devicetree/bindings/media/samsung-fimc.txt | 46 +--- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt index 96312f6..1908a5f 100644 --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt [snip] Image sensor nodes -- @@ -97,8 +108,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller (e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt. -The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensor device +nodes. Default value when this property is not present is 24 MHz. I think you forgot to drop that sentence. -- Regards, Laurent Pinchart -- 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 v3 27/48] v4l: Validate fields in the core code for subdev EDID ioctls
Hi Hans, On Tuesday 11 March 2014 17:11:07 Hans Verkuil wrote: On 03/11/2014 05:08 PM, Laurent Pinchart wrote: Hi Hans, On Tuesday 11 March 2014 16:44:27 Hans Verkuil wrote: On 03/11/2014 04:09 PM, Laurent Pinchart wrote: The subdev EDID ioctls receive a pad field that must reference an existing pad and an EDID field that must point to a buffer. Validate both fields in the core code instead of duplicating validation in all drivers. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Sakari Ailus sakari.ai...@linux.intel.com Here is my: Reviewed-by: Hans Verkuil hans.verk...@cisco.com But take note: the adv7604 driver does not handle a get_edid with edid-blocks == 0 correctly: it should fill in the blocks field with the real number of blocks and return 0 instead of returning EINVAL. Should it also set edid-start_block to 0 ? I don't think so. It makes sense to just set blocks to the total number of available blocks - edid-start_block. OK. Note that if edid-start_block = total number of EDID blocks, then -ENODATA should be returned. What if S_EDID hasn't been called yet ? Should the driver set edid-blocks to 0 and return success ? Or should it return -ENODATA ? There's quite a few possible combinations, we should probably start by clarifying the spec. -- Regards, Laurent Pinchart -- 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
[GIT PULL 3.15] m88ds3103 fix non-important issues
These are reported by Coverity and has no functionality changes. regards Antti The following changes since commit c3c2077d9579472b07581ecdaf6cc5a60b1700bc: [media] nuvoton-cir: Activate PNP device when probing (2014-03-11 12:22:50 -0300) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git m88ds3103_3.15 for you to fetch changes up to f8b959e39d19e44a008cea3730af0256f4be5f5c: m88ds3103: possible uninitialized scalar variable (2014-03-11 18:17:29 +0200) Antti Palosaari (3): m88ds3103: remove dead code m88ds3103: remove dead code 2nd part m88ds3103: possible uninitialized scalar variable drivers/media/dvb-frontends/m88ds3103.c | 28 +++- 1 file changed, 7 insertions(+), 21 deletions(-) -- 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 v7 3/10] Documentation: devicetree: Update Samsung FIMC DT binding
Hi Laurent, On 11/03/14 17:20, Laurent Pinchart wrote: Image sensor nodes -- @@ -97,8 +108,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller (e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt. -The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensor device +nodes. Default value when this property is not present is 24 MHz. I think you forgot to drop that sentence. Indeed, sorry, I'll fix that and repost. -- Thanks, Sylwester -- 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