Re: [media-workshop] Tentative Agenda for the November workshop
On Tue October 23 2012 03:03:35 Laurent Pinchart wrote: > On Monday 22 October 2012 14:06:06 Hans Verkuil wrote: > > On Mon October 22 2012 13:18:46 Laurent Pinchart wrote: > > > On Monday 22 October 2012 12:53:02 Sylwester Nawrocki wrote: > > > > On 10/22/2012 12:39 PM, Laurent Pinchart wrote: > > > > > On Monday 22 October 2012 10:35:56 Hans Verkuil wrote: > > > > >> Hi all, > > > > >> > > > > >> This is the tentative agenda for the media workshop on November 8, > > > > >> 2012. If you have additional things that you want to discuss, or > > > > >> something is wrong or incomplete in this list, please let me know so > > > > >> I can update the list. > > > > > > > > > > Thank you Hans for taking care of the agenda. > > > > > > > > > >> - Explain current merging process (Mauro) > > > > >> - Open floor for discussions on how to improve it (Mauro) > > > > >> - Write down minimum requirements for new V4L2 (and DVB?) drivers, > > > > >> both for staging and mainline acceptance: which frameworks to use, > > > > >> v4l2-compliance, > > > > >> > > > > >> etc. (Hans Verkuil) > > > > >> - V4L2 ambiguities (Hans Verkuil) > > > > >> - TSMux device (a mux rather than a demux): Alain Volmat > > > > >> - dmabuf status, esp. with regards to being able to test > > > > >> (Mauro/Samsung) > > > > >> - Device tree support (Guennadi, not known yet whether this topic is > > > > >> needed) - Creating/selecting contexts for hardware that supports this > > > > >> (Samsung, only if time is available) > > > > > > > > > > This last topic will likely require lots of brainstorming, and thus > > > > > time. If the schedule permits, would anyone be interested in meeting > > > > > earlier during the week already ? > > > > > > > > My intention was to also possibly discuss it with others before the > > > > actual media workshop. Would be nice if we could have arranged such a > > > > meeting. I'm not sure about the room conditions though. It's probably > > > > not a big issue, unless there is really many people interested in that > > > > topic. > > > > > > A small room with a projector would be nice if possible, although not > > > required. Who would be interested in attending a brainstorming session on > > > contexts ? > > > > I would be, but the problem is that the conference is also interesting. > > More interesting than a brainstorming session about hardware contexts ? ;-) > There's of course talks I want to attend, but I can probably skip some of > them. > > > The only day I have really available is the Friday *after* the summit. > > We'll probably need several brainstorming sessions anyway. I'd like to > organize one before the media summit though, as we'll have limited time to > discuss the topic during the summit, which doesn't suit brainstorming > sessions > very well. Sylwester, would Samsung be able to prepare for a brainstorming session on Monday or Tuesday? Both Laurent and myself have presentations on Wednesday, so that's not the best day for such a session. Do you think we should do a half-day or a full day session on this? 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 2/2] [media] vivi: Teach it to tune FPS
On Mon October 22 2012 19:01:40 Kirill Smelkov wrote:
> On Mon, Oct 22, 2012 at 04:16:14PM +0200, Hans Verkuil wrote:
> > On Mon October 22 2012 15:54:44 Kirill Smelkov wrote:
> > > I was testing my video-over-ethernet subsystem today, and vivi seemed to
> > > be perfect video source for testing when one don't have lots of capture
> > > boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
> > > while in my country we usually use PAL (25 fps). That's why the patch.
> > > Thanks.
> >
> > Rather than introducing a module option, it's much nicer if you can
> > implement enum_frameintervals and g/s_parm. This can be made quite flexible
> > allowing you to also support 50/59.94/60 fps.
>
> Thanks for feedback. I've reworked the patch for FPS to be set via
> ->{g,s}_parm(), and yes now it is more flexble, because one can set
> different FPS on different vivi devices. Only I don't know V4L2 ioctls
> details well enough and various drivers do things differently. The patch
> is below. Is it ok?
Close, but it's not quite there.
You should run the v4l2-compliance tool from the v4l-utils.git repository
(take the master branch). That will report any errors in your implementation.
In this case g/s_parm doesn't set readbuffers (set it to 1) and if timeperframe
equals { 0, 0 }, then you should get a nominal framerate (let's stick to 29.97
for that). I would set the nominal framerate whenever the denominator == 0.
For vidioc_enum_frameintervals you need to check the IN fields and fill in the
stepwise struct.
Regards,
Hans
>
> Thanks,
> Kirill
>
>
> 8<
> From: Kirill Smelkov
> Date: Mon, 22 Oct 2012 17:25:24 +0400
> Subject: [PATCH v2] [media] vivi: Teach it to tune FPS
>
> I was testing my video-over-ethernet subsystem today, and vivi seemed to
> be perfect video source for testing when one don't have lots of capture
> boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
> while in my country we usually use PAL (25 fps).
>
> That's why here is this patch with ->enum_frameintervals and
> ->{g,s}_parm implemented as suggested by Hans Verkuil. Not sure I've
> done ->g_parm right -- some drivers clear parm memory before setting
> fields, some don't. As at is at least it works for me (tested via
> v4l2-ctl -P / -p ).
>
> Thanks.
>
> Signed-off-by: Kirill Smelkov
> ---
> drivers/media/platform/vivi.c | 50
> +--
> 1 file changed, 43 insertions(+), 7 deletions(-)
>
> V2:
>
> - reworked FPS setting from module param to via ->{g,s}_parm() as
> suggested
> by Hans Verkuil.
>
> diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
> index 3e6902a..c0855a5 100644
> --- a/drivers/media/platform/vivi.c
> +++ b/drivers/media/platform/vivi.c
> @@ -36,10 +36,6 @@
>
> #define VIVI_MODULE_NAME "vivi"
>
> -/* Wake up at about 30 fps */
> -#define WAKE_NUMERATOR 30
> -#define WAKE_DENOMINATOR 1001
> -
> #define MAX_WIDTH 1920
> #define MAX_HEIGHT 1200
>
> @@ -232,6 +228,7 @@ struct vivi_dev {
>
> /* video capture */
> struct vivi_fmt*fmt;
> + struct v4l2_fract timeperframe;
> unsigned int width, height;
> struct vb2_queue vb_vidq;
> unsigned int field_count;
> @@ -660,8 +657,8 @@ static void vivi_thread_tick(struct vivi_dev *dev)
> dprintk(dev, 2, "[%p/%d] done\n", buf, buf->vb.v4l2_buf.index);
> }
>
> -#define frames_to_ms(frames) \
> - ((frames * WAKE_NUMERATOR * 1000) / WAKE_DENOMINATOR)
> +#define frames_to_ms(dev, frames)\
> + ((frames * dev->timeperframe.numerator * 1000) /
> dev->timeperframe.denominator)
>
> static void vivi_sleep(struct vivi_dev *dev)
> {
> @@ -677,7 +674,8 @@ static void vivi_sleep(struct vivi_dev *dev)
> goto stop_task;
>
> /* Calculate time to wake up */
> - timeout = msecs_to_jiffies(frames_to_ms(1));
> + timeout = msecs_to_jiffies(frames_to_ms(dev, 1));
> +
>
> vivi_thread_tick(dev);
>
> @@ -1049,6 +1047,39 @@ static int vidioc_s_input(struct file *file, void
> *priv, unsigned int i)
> return 0;
> }
>
> +/* timeperframe is arbitrary and continous */
> +static int vidioc_enum_frameintervals(struct file *file, void *priv,
> + struct v4l2_frmivalenum *fival)
> +{
> + fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
> + return 0;
> +}
> +
> +static int vidioc_g_parm(struct file *file, void *priv, struct
> v4l2_streamparm *parm)
> +{
> + struct vivi_dev *dev = video_drvdata(file);
> +
> + if (parm->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + return -EINVAL;
> +
> + parm->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> + parm->parm.capture.timeperframe = dev->timeperframe;
> + return 0;
> +}
> +
> +static int vidioc_s_parm(struct file *file, void *priv, struct
> v4l
Re: [PATCH 2/2] [media] vivi: Teach it to tune FPS
On Mon October 22 2012 19:29:01 Kirill Smelkov wrote:
> On Mon, Oct 22, 2012 at 09:01:39PM +0400, Kirill Smelkov wrote:
> > On Mon, Oct 22, 2012 at 04:16:14PM +0200, Hans Verkuil wrote:
> > > On Mon October 22 2012 15:54:44 Kirill Smelkov wrote:
> > > > I was testing my video-over-ethernet subsystem today, and vivi seemed to
> > > > be perfect video source for testing when one don't have lots of capture
> > > > boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
> > > > while in my country we usually use PAL (25 fps). That's why the patch.
> > > > Thanks.
> > >
> > > Rather than introducing a module option, it's much nicer if you can
> > > implement enum_frameintervals and g/s_parm. This can be made quite
> > > flexible
> > > allowing you to also support 50/59.94/60 fps.
> >
> > Thanks for feedback. I've reworked the patch for FPS to be set via
> > ->{g,s}_parm(), and yes now it is more flexble, because one can set
>
> By the way, here is what I've found while working on the abovementioned
> patch:
>
> 8<
> From: Kirill Smelkov
> Date: Mon, 22 Oct 2012 21:14:01 +0400
> Subject: [PATCH] v4l2: Fix typo in struct v4l2_captureparm description
>
> Judging from what drivers do and from my experience temeperframe
> fraction is set in seconds - look e.g. here
>
> static int bttv_g_parm(struct file *file, void *f,
> struct v4l2_streamparm *parm)
> {
> struct bttv_fh *fh = f;
> struct bttv *btv = fh->btv;
>
> v4l2_video_std_frame_period(bttv_tvnorms[btv->tvnorm].v4l2_id,
> &parm->parm.capture.timeperframe);
>
> ...
>
> void v4l2_video_std_frame_period(int id, struct v4l2_fract *frameperiod)
> {
> if (id & V4L2_STD_525_60) {
> frameperiod->numerator = 1001;
> frameperiod->denominator = 3;
> } else {
> frameperiod->numerator = 1;
> frameperiod->denominator = 25;
> }
>
> and also v4l2-ctl in userspace decodes this as seconds:
>
> if (doioctl(fd, VIDIOC_G_PARM, &parm, "VIDIOC_G_PARM") == 0) {
> const struct v4l2_fract &tf = parm.parm.capture.timeperframe;
>
> ...
>
> printf("\tFrames per second: %.3f (%d/%d)\n",
> (1.0 * tf.denominator) / tf.numerator,
> tf.denominator, tf.numerator);
>
> The typo was there from day 1 - added in 2002 in e028b61b ([PATCH]
> add v4l2 api)(*)
>
> (*) found in history tree
> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
>
> Signed-off-by: Kirill Smelkov
Good catch. Luckily the V4L2 spec is correct, it is just that single comment
that's wrong.
Acked-by: Hans Verkuil
Thanks!
Hans
> ---
> include/uapi/linux/videodev2.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 57bfa59..2fff7ff 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -726,29 +726,29 @@ struct v4l2_window {
> __u32 field; /* enum v4l2_field */
> __u32 chromakey;
> struct v4l2_clip__user *clips;
> __u32 clipcount;
> void__user *bitmap;
> __u8global_alpha;
> };
>
> /*
> * C A P T U R E P A R A M E T E R S
> */
> struct v4l2_captureparm {
> __u32 capability;/* Supported modes */
> __u32 capturemode; /* Current mode */
> - struct v4l2_fract timeperframe; /* Time per frame in .1us units */
> + struct v4l2_fract timeperframe; /* Time per frame in seconds */
> __u32 extendedmode; /* Driver-specific extensions */
> __u32 readbuffers; /* # of buffers for read */
> __u32 reserved[4];
> };
>
> /* Flags for 'capability' and 'capturemode' fields */
> #define V4L2_MODE_HIGHQUALITY0x0001 /* High quality imaging mode */
> #define V4L2_CAP_TIMEPERFRAME0x1000 /* timeperframe field is
> supported */
>
> struct v4l2_outputparm {
> __u32 capability; /* Supported modes */
> __u32 outputmode; /* Current mode */
> struct v4l2_fract timeperframe; /* Time per frame in seconds */
> __u32 extendedmode; /* Driver-specific extensions */
>
--
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: [media-workshop] Tentative Agenda for the November workshop
On Monday 22 October 2012 14:06:06 Hans Verkuil wrote: > On Mon October 22 2012 13:18:46 Laurent Pinchart wrote: > > On Monday 22 October 2012 12:53:02 Sylwester Nawrocki wrote: > > > On 10/22/2012 12:39 PM, Laurent Pinchart wrote: > > > > On Monday 22 October 2012 10:35:56 Hans Verkuil wrote: > > > >> Hi all, > > > >> > > > >> This is the tentative agenda for the media workshop on November 8, > > > >> 2012. If you have additional things that you want to discuss, or > > > >> something is wrong or incomplete in this list, please let me know so > > > >> I can update the list. > > > > > > > > Thank you Hans for taking care of the agenda. > > > > > > > >> - Explain current merging process (Mauro) > > > >> - Open floor for discussions on how to improve it (Mauro) > > > >> - Write down minimum requirements for new V4L2 (and DVB?) drivers, > > > >> both for staging and mainline acceptance: which frameworks to use, > > > >> v4l2-compliance, > > > >> > > > >> etc. (Hans Verkuil) > > > >> - V4L2 ambiguities (Hans Verkuil) > > > >> - TSMux device (a mux rather than a demux): Alain Volmat > > > >> - dmabuf status, esp. with regards to being able to test > > > >> (Mauro/Samsung) > > > >> - Device tree support (Guennadi, not known yet whether this topic is > > > >> needed) - Creating/selecting contexts for hardware that supports this > > > >> (Samsung, only if time is available) > > > > > > > > This last topic will likely require lots of brainstorming, and thus > > > > time. If the schedule permits, would anyone be interested in meeting > > > > earlier during the week already ? > > > > > > My intention was to also possibly discuss it with others before the > > > actual media workshop. Would be nice if we could have arranged such a > > > meeting. I'm not sure about the room conditions though. It's probably > > > not a big issue, unless there is really many people interested in that > > > topic. > > > > A small room with a projector would be nice if possible, although not > > required. Who would be interested in attending a brainstorming session on > > contexts ? > > I would be, but the problem is that the conference is also interesting. More interesting than a brainstorming session about hardware contexts ? ;-) There's of course talks I want to attend, but I can probably skip some of them. > The only day I have really available is the Friday *after* the summit. We'll probably need several brainstorming sessions anyway. I'd like to organize one before the media summit though, as we'll have limited time to discuss the topic during the summit, which doesn't suit brainstorming sessions very well. -- 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 2/2] [media]: mx2_camera: Fix regression caused by clock conversion
Hi Guennadi On Mon, Oct 22, 2012 at 7:07 PM, Guennadi Liakhovetski wrote: > ? I don't find a clock named "per" and associated with "mx2-camera.0" in > arch/arm/mach-imx/clk-imx27.c. I only find it in clk-imx25.c. Does this > mean, that this your patch is for i.MX25? But you're saying it's for > i.MX27. Confused... I provide this mx27 clock in the first patch of the series: http://patchwork.linuxtv.org/patch/14915/ Regards, Fabio Estevam -- 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: [media-workshop] Tentative Agenda for the November workshop
On Mon, 22 Oct 2012, Hans Verkuil wrote: > Hi all, > > This is the tentative agenda for the media workshop on November 8, 2012. > If you have additional things that you want to discuss, or something is wrong > or incomplete in this list, please let me know so I can update the list. > > - Explain current merging process (Mauro) > - Open floor for discussions on how to improve it (Mauro) > - Write down minimum requirements for new V4L2 (and DVB?) drivers, both for > staging and mainline acceptance: which frameworks to use, v4l2-compliance, > etc. (Hans Verkuil) > - V4L2 ambiguities (Hans Verkuil) > - TSMux device (a mux rather than a demux): Alain Volmat > - dmabuf status, esp. with regards to being able to test (Mauro/Samsung) > - Device tree support (Guennadi, not known yet whether this topic is needed) + asynchronous probing, I guess. It's probably implicitly included though. Thanks Guennadi > - Creating/selecting contexts for hardware that supports this (Samsung, only > if time is available) > > For those whose name is behind a topic: please prepare something before the > meeting. > > We have currently 17 or 18 attendees of a maximum of 25, so there is room > for a few more people. > > Regards, > > Hans > > ___ > media-workshop mailing list > media-works...@linuxtv.org > http://www.linuxtv.org/cgi-bin/mailman/listinfo/media-workshop > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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: [media-workshop] Tentative Agenda for the November workshop
Hi Laurent On Mon, 22 Oct 2012, Laurent Pinchart wrote: > Hi Sylwester, > > On Monday 22 October 2012 12:53:02 Sylwester Nawrocki wrote: > > On 10/22/2012 12:39 PM, Laurent Pinchart wrote: > > > On Monday 22 October 2012 10:35:56 Hans Verkuil wrote: > > >> Hi all, > > >> > > >> This is the tentative agenda for the media workshop on November 8, 2012. > > >> If you have additional things that you want to discuss, or something is > > >> wrong or incomplete in this list, please let me know so I can update the > > >> list. > > > > > > Thank you Hans for taking care of the agenda. > > > > > >> - Explain current merging process (Mauro) > > >> - Open floor for discussions on how to improve it (Mauro) > > >> - Write down minimum requirements for new V4L2 (and DVB?) drivers, both > > >> for > > >> > > >> staging and mainline acceptance: which frameworks to use, > > >> v4l2-compliance, > > >> > > >> etc. (Hans Verkuil) > > >> - V4L2 ambiguities (Hans Verkuil) > > >> - TSMux device (a mux rather than a demux): Alain Volmat > > >> - dmabuf status, esp. with regards to being able to test (Mauro/Samsung) > > >> - Device tree support (Guennadi, not known yet whether this topic is > > >> needed) - Creating/selecting contexts for hardware that supports this > > >> (Samsung, only if time is available) > > > > > > This last topic will likely require lots of brainstorming, and thus time. > > > If the schedule permits, would anyone be interested in meeting earlier > > > during the week already ? > > > > My intention was to also possibly discuss it with others before the actual > > media workshop. Would be nice if we could have arranged such a meeting. > > I'm not sure about the room conditions though. It's probably not a big > > issue, unless there is really many people interested in that topic. > > A small room with a projector would be nice if possible, although not > required. Who would be interested in attending a brainstorming session on > contexts ? I'd also be interested to participate. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 2/2] [media]: mx2_camera: Fix regression caused by clock conversion
Hi Fabio
On Tue, 9 Oct 2012, Fabio Estevam wrote:
> Since mx27 transitioned to the commmon clock framework in 3.5, the correct way
> to acquire the csi clock is to get csi_ahb and csi_per clocks separately.
>
> By not doing so the camera sensor does not probe correctly:
>
> soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0
> mx2-camera mx2-camera.0: Camera driver attached to camera 0
> ov2640 0-0030: Product ID error fb:fb
> mx2-camera mx2-camera.0: Camera driver detached from camera 0
> mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency:
> 6650
>
> Adapt the mx2_camera driver to the new clock framework and make it functional
> again.
>
> Tested-by: Gaëtan Carlier
> Tested-by: Javier Martin
> Signed-off-by: Fabio Estevam
I've got a question to this your patch: could you explain to me, which
clock is obtained by
> + pcdev->clk_csi_per = devm_clk_get(&pdev->dev, "per");
? I don't find a clock named "per" and associated with "mx2-camera.0" in
arch/arm/mach-imx/clk-imx27.c. I only find it in clk-imx25.c. Does this
mean, that this your patch is for i.MX25? But you're saying it's for
i.MX27. Confused...
Thanks
Guennadi
> ---
> Changes since v2:
> - Fix clock error handling code as pointed out by Russell King
> Changes since v1:
> - Rebased against linux-next 20121008.
> drivers/media/platform/soc_camera/mx2_camera.c | 50
> ++--
> 1 file changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/platform/soc_camera/mx2_camera.c
> b/drivers/media/platform/soc_camera/mx2_camera.c
> index 403d7f1..382b305 100644
> --- a/drivers/media/platform/soc_camera/mx2_camera.c
> +++ b/drivers/media/platform/soc_camera/mx2_camera.c
> @@ -272,7 +272,8 @@ struct mx2_camera_dev {
> struct device *dev;
> struct soc_camera_host soc_host;
> struct soc_camera_device *icd;
> - struct clk *clk_csi, *clk_emma_ahb, *clk_emma_ipg;
> + struct clk *clk_emma_ahb, *clk_emma_ipg;
> + struct clk *clk_csi_ahb, *clk_csi_per;
>
> void __iomem*base_csi, *base_emma;
>
> @@ -432,7 +433,8 @@ static void mx2_camera_deactivate(struct mx2_camera_dev
> *pcdev)
> {
> unsigned long flags;
>
> - clk_disable_unprepare(pcdev->clk_csi);
> + clk_disable_unprepare(pcdev->clk_csi_ahb);
> + clk_disable_unprepare(pcdev->clk_csi_per);
> writel(0, pcdev->base_csi + CSICR1);
> if (cpu_is_mx27()) {
> writel(0, pcdev->base_emma + PRP_CNTL);
> @@ -460,10 +462,14 @@ static int mx2_camera_add_device(struct
> soc_camera_device *icd)
> if (pcdev->icd)
> return -EBUSY;
>
> - ret = clk_prepare_enable(pcdev->clk_csi);
> + ret = clk_prepare_enable(pcdev->clk_csi_ahb);
> if (ret < 0)
> return ret;
>
> + ret = clk_prepare_enable(pcdev->clk_csi_per);
> + if (ret < 0)
> + goto exit_csi_ahb;
> +
> csicr1 = CSICR1_MCLKEN;
>
> if (cpu_is_mx27())
> @@ -480,6 +486,11 @@ static int mx2_camera_add_device(struct
> soc_camera_device *icd)
>icd->devnum);
>
> return 0;
> +
> +exit_csi_ahb:
> + clk_disable_unprepare(pcdev->clk_csi_ahb);
> +
> + return ret;
> }
>
> static void mx2_camera_remove_device(struct soc_camera_device *icd)
> @@ -1725,27 +1736,35 @@ static int __devinit mx2_camera_probe(struct
> platform_device *pdev)
> goto exit;
> }
>
> - pcdev->clk_csi = devm_clk_get(&pdev->dev, "ahb");
> - if (IS_ERR(pcdev->clk_csi)) {
> - dev_err(&pdev->dev, "Could not get csi clock\n");
> - err = PTR_ERR(pcdev->clk_csi);
> + pcdev->clk_csi_ahb = devm_clk_get(&pdev->dev, "ahb");
> + if (IS_ERR(pcdev->clk_csi_ahb)) {
> + dev_err(&pdev->dev, "Could not get csi ahb clock\n");
> + err = PTR_ERR(pcdev->clk_csi_ahb);
> goto exit;
> }
>
> + pcdev->clk_csi_per = devm_clk_get(&pdev->dev, "per");
> + if (IS_ERR(pcdev->clk_csi_per)) {
> + dev_err(&pdev->dev, "Could not get csi per clock\n");
> + err = PTR_ERR(pcdev->clk_csi_per);
> + goto exit_csi_ahb;
> + }
> +
> pcdev->pdata = pdev->dev.platform_data;
> if (pcdev->pdata) {
> long rate;
>
> pcdev->platform_flags = pcdev->pdata->flags;
>
> - rate = clk_round_rate(pcdev->clk_csi, pcdev->pdata->clk * 2);
> + rate = clk_round_rate(pcdev->clk_csi_per,
> + pcdev->pdata->clk * 2);
> if (rate <= 0) {
> err = -ENODEV;
> - goto exit;
> + goto exit_csi_per;
> }
> - err = clk_set_rate(pcdev->clk_csi, rate);
> + err = clk_set_rate(pcdev->clk_csi_per, rate);
> if (err < 0)
> - goto exit;
> +
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:Mon Oct 22 19:00:28 CEST 2012 git hash:74df06daf632ce2d321d01cb046004768352efc4 gcc version: i686-linux-gcc (GCC) 4.7.1 host hardware:x86_64 host os: 3.4.07-marune linux-git-arm-eabi-davinci: WARNINGS linux-git-arm-eabi-exynos: WARNINGS linux-git-arm-eabi-omap: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: WARNINGS linux-git-mips: WARNINGS linux-git-powerpc64: OK linux-git-sh: WARNINGS linux-git-x86_64: WARNINGS linux-2.6.31.12-i686: WARNINGS linux-2.6.32.6-i686: WARNINGS linux-2.6.33-i686: WARNINGS linux-2.6.34-i686: WARNINGS linux-2.6.35.3-i686: WARNINGS linux-2.6.36-i686: WARNINGS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-i686: WARNINGS linux-3.2.1-i686: WARNINGS linux-3.3-i686: WARNINGS linux-3.4-i686: WARNINGS linux-3.5-i686: WARNINGS linux-3.6-i686: WARNINGS linux-3.7-rc1-i686: WARNINGS linux-2.6.31.12-x86_64: WARNINGS linux-2.6.32.6-x86_64: WARNINGS linux-2.6.33-x86_64: WARNINGS linux-2.6.34-x86_64: WARNINGS linux-2.6.35.3-x86_64: WARNINGS linux-2.6.36-x86_64: WARNINGS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-x86_64: WARNINGS linux-3.2.1-x86_64: WARNINGS linux-3.3-x86_64: WARNINGS linux-3.4-x86_64: WARNINGS linux-3.5-x86_64: WARNINGS linux-3.6-x86_64: WARNINGS linux-3.7-rc1-x86_64: WARNINGS apps: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.tar.bz2 The V4L-DVB specification 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: Re: A reliable kernel panic (3.6.2) and system crash when visiting a particular website
On Mon, 22 Oct 2012, Artem S. Tashkinov wrote:
> OK, here's what the kernel prints with your patch:
>
> usb 6.1.4: ep 86 list del corruption prev: e5103b54 e5103a94 e51039d4
>
> A small delay before I got thousands of list_del corruption messages would
> have been nice, but I managed to catch the message anyway.
All right. Here's a new patch, which will print more information and
will provide a 10-second delay.
For this to be useful, you should capture a usbmon trace at the same
time. The relevant entries will show up in the trace shortly before
_and_ shortly after the error message appears.
Alan Stern
P.S.: It will help if you unplug as many of the other USB devices as
possible before running this test.
Index: usb-3.6/drivers/usb/core/hcd.c
===
--- usb-3.6.orig/drivers/usb/core/hcd.c
+++ usb-3.6/drivers/usb/core/hcd.c
@@ -1083,6 +1083,8 @@ EXPORT_SYMBOL_GPL(usb_calc_bus_time);
/*-*/
+static bool list_error;
+
/**
* usb_hcd_link_urb_to_ep - add an URB to its endpoint queue
* @hcd: host controller to which @urb was submitted
@@ -1193,6 +1195,25 @@ void usb_hcd_unlink_urb_from_ep(struct u
{
/* clear all state linking urb to this dev (and hcd) */
spin_lock(&hcd_urb_list_lock);
+ {
+ struct list_head *cur = &urb->urb_list;
+ struct list_head *prev = cur->prev;
+ struct list_head *next = cur->next;
+
+ if (prev->next != cur && !list_error) {
+ list_error = true;
+ dev_err(&urb->dev->dev,
+ "ep %x list del corruption prev: %p %p %p %p
%p\n",
+ urb->ep->desc.bEndpointAddress,
+ cur, prev, prev->next, next, next->prev);
+ dev_err(&urb->dev->dev,
+ "head %p urb %p urbprev %p urbnext %p\n",
+ &urb->ep->urb_list, urb,
+ list_entry(prev, struct urb, urb_list),
+ list_entry(next, struct urb, urb_list));
+ mdelay(1);
+ }
+ }
list_del_init(&urb->urb_list);
spin_unlock(&hcd_urb_list_lock);
}
--
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: Re: A reliable kernel panic (3.6.2) and system crash when visiting a particular website
On Oct 22, 2012, Alan Stern wrote: > A BUG() at these points would crash the machine hard. And where we > came from doesn't matter; what matters is the values in the pointers. OK, here's what the kernel prints with your patch: usb 6.1.4: ep 86 list del corruption prev: e5103b54 e5103a94 e51039d4 A small delay before I got thousands of list_del corruption messages would have been nice, but I managed to catch the message anyway. Artem -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [media] vivi: Teach it to tune FPS
On Mon, Oct 22, 2012 at 09:01:39PM +0400, Kirill Smelkov wrote:
> On Mon, Oct 22, 2012 at 04:16:14PM +0200, Hans Verkuil wrote:
> > On Mon October 22 2012 15:54:44 Kirill Smelkov wrote:
> > > I was testing my video-over-ethernet subsystem today, and vivi seemed to
> > > be perfect video source for testing when one don't have lots of capture
> > > boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
> > > while in my country we usually use PAL (25 fps). That's why the patch.
> > > Thanks.
> >
> > Rather than introducing a module option, it's much nicer if you can
> > implement enum_frameintervals and g/s_parm. This can be made quite flexible
> > allowing you to also support 50/59.94/60 fps.
>
> Thanks for feedback. I've reworked the patch for FPS to be set via
> ->{g,s}_parm(), and yes now it is more flexble, because one can set
By the way, here is what I've found while working on the abovementioned
patch:
8<
From: Kirill Smelkov
Date: Mon, 22 Oct 2012 21:14:01 +0400
Subject: [PATCH] v4l2: Fix typo in struct v4l2_captureparm description
Judging from what drivers do and from my experience temeperframe
fraction is set in seconds - look e.g. here
static int bttv_g_parm(struct file *file, void *f,
struct v4l2_streamparm *parm)
{
struct bttv_fh *fh = f;
struct bttv *btv = fh->btv;
v4l2_video_std_frame_period(bttv_tvnorms[btv->tvnorm].v4l2_id,
&parm->parm.capture.timeperframe);
...
void v4l2_video_std_frame_period(int id, struct v4l2_fract *frameperiod)
{
if (id & V4L2_STD_525_60) {
frameperiod->numerator = 1001;
frameperiod->denominator = 3;
} else {
frameperiod->numerator = 1;
frameperiod->denominator = 25;
}
and also v4l2-ctl in userspace decodes this as seconds:
if (doioctl(fd, VIDIOC_G_PARM, &parm, "VIDIOC_G_PARM") == 0) {
const struct v4l2_fract &tf = parm.parm.capture.timeperframe;
...
printf("\tFrames per second: %.3f (%d/%d)\n",
(1.0 * tf.denominator) / tf.numerator,
tf.denominator, tf.numerator);
The typo was there from day 1 - added in 2002 in e028b61b ([PATCH]
add v4l2 api)(*)
(*) found in history tree
git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
Signed-off-by: Kirill Smelkov
---
include/uapi/linux/videodev2.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 57bfa59..2fff7ff 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -726,29 +726,29 @@ struct v4l2_window {
__u32 field; /* enum v4l2_field */
__u32 chromakey;
struct v4l2_clip__user *clips;
__u32 clipcount;
void__user *bitmap;
__u8global_alpha;
};
/*
* C A P T U R E P A R A M E T E R S
*/
struct v4l2_captureparm {
__u32 capability;/* Supported modes */
__u32 capturemode; /* Current mode */
- struct v4l2_fract timeperframe; /* Time per frame in .1us units */
+ struct v4l2_fract timeperframe; /* Time per frame in seconds */
__u32 extendedmode; /* Driver-specific extensions */
__u32 readbuffers; /* # of buffers for read */
__u32 reserved[4];
};
/* Flags for 'capability' and 'capturemode' fields */
#define V4L2_MODE_HIGHQUALITY 0x0001 /* High quality imaging mode */
#define V4L2_CAP_TIMEPERFRAME 0x1000 /* timeperframe field is supported */
struct v4l2_outputparm {
__u32 capability; /* Supported modes */
__u32 outputmode; /* Current mode */
struct v4l2_fract timeperframe; /* Time per frame in seconds */
__u32 extendedmode; /* Driver-specific extensions */
--
1.8.0.rc3.331.g5b9a629
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [media] vivi: Teach it to tune FPS
On Mon, Oct 22, 2012 at 04:16:14PM +0200, Hans Verkuil wrote:
> On Mon October 22 2012 15:54:44 Kirill Smelkov wrote:
> > I was testing my video-over-ethernet subsystem today, and vivi seemed to
> > be perfect video source for testing when one don't have lots of capture
> > boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
> > while in my country we usually use PAL (25 fps). That's why the patch.
> > Thanks.
>
> Rather than introducing a module option, it's much nicer if you can
> implement enum_frameintervals and g/s_parm. This can be made quite flexible
> allowing you to also support 50/59.94/60 fps.
Thanks for feedback. I've reworked the patch for FPS to be set via
->{g,s}_parm(), and yes now it is more flexble, because one can set
different FPS on different vivi devices. Only I don't know V4L2 ioctls
details well enough and various drivers do things differently. The patch
is below. Is it ok?
Thanks,
Kirill
8<
From: Kirill Smelkov
Date: Mon, 22 Oct 2012 17:25:24 +0400
Subject: [PATCH v2] [media] vivi: Teach it to tune FPS
I was testing my video-over-ethernet subsystem today, and vivi seemed to
be perfect video source for testing when one don't have lots of capture
boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
while in my country we usually use PAL (25 fps).
That's why here is this patch with ->enum_frameintervals and
->{g,s}_parm implemented as suggested by Hans Verkuil. Not sure I've
done ->g_parm right -- some drivers clear parm memory before setting
fields, some don't. As at is at least it works for me (tested via
v4l2-ctl -P / -p ).
Thanks.
Signed-off-by: Kirill Smelkov
---
drivers/media/platform/vivi.c | 50 +--
1 file changed, 43 insertions(+), 7 deletions(-)
V2:
- reworked FPS setting from module param to via ->{g,s}_parm() as suggested
by Hans Verkuil.
diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index 3e6902a..c0855a5 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -36,10 +36,6 @@
#define VIVI_MODULE_NAME "vivi"
-/* Wake up at about 30 fps */
-#define WAKE_NUMERATOR 30
-#define WAKE_DENOMINATOR 1001
-
#define MAX_WIDTH 1920
#define MAX_HEIGHT 1200
@@ -232,6 +228,7 @@ struct vivi_dev {
/* video capture */
struct vivi_fmt*fmt;
+ struct v4l2_fract timeperframe;
unsigned int width, height;
struct vb2_queue vb_vidq;
unsigned int field_count;
@@ -660,8 +657,8 @@ static void vivi_thread_tick(struct vivi_dev *dev)
dprintk(dev, 2, "[%p/%d] done\n", buf, buf->vb.v4l2_buf.index);
}
-#define frames_to_ms(frames) \
- ((frames * WAKE_NUMERATOR * 1000) / WAKE_DENOMINATOR)
+#define frames_to_ms(dev, frames) \
+ ((frames * dev->timeperframe.numerator * 1000) /
dev->timeperframe.denominator)
static void vivi_sleep(struct vivi_dev *dev)
{
@@ -677,7 +674,8 @@ static void vivi_sleep(struct vivi_dev *dev)
goto stop_task;
/* Calculate time to wake up */
- timeout = msecs_to_jiffies(frames_to_ms(1));
+ timeout = msecs_to_jiffies(frames_to_ms(dev, 1));
+
vivi_thread_tick(dev);
@@ -1049,6 +1047,39 @@ static int vidioc_s_input(struct file *file, void *priv,
unsigned int i)
return 0;
}
+/* timeperframe is arbitrary and continous */
+static int vidioc_enum_frameintervals(struct file *file, void *priv,
+struct v4l2_frmivalenum *fival)
+{
+ fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
+ return 0;
+}
+
+static int vidioc_g_parm(struct file *file, void *priv, struct v4l2_streamparm
*parm)
+{
+ struct vivi_dev *dev = video_drvdata(file);
+
+ if (parm->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ return -EINVAL;
+
+ parm->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
+ parm->parm.capture.timeperframe = dev->timeperframe;
+ return 0;
+}
+
+static int vidioc_s_parm(struct file *file, void *priv, struct v4l2_streamparm
*parm)
+{
+ struct vivi_dev *dev = video_drvdata(file);
+
+ if (parm->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ return -EINVAL;
+
+ dev->timeperframe.numerator =
parm->parm.capture.timeperframe.numerator;
+ dev->timeperframe.denominator =
parm->parm.capture.timeperframe.denominator ?: 1;
+
+ return 0;
+}
+
/* --- controls -- */
static int vivi_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
@@ -1207,6 +1238,9 @@ static const struct v4l2_ioctl_ops vivi_ioctl_ops = {
.vidioc_enum_input= vidioc_enum_input,
.vidioc_g_input = vidioc_g_input,
.vidioc_s_input = vidioc_s_input,
+ .vidioc_enum_frameintervals = vidioc_enum_frameintervals,
+ .vidio
Re: [PATCH 3.6.0- 4/5] media/soc_camera: use module_platform_driver macro
Hi Srinivas
On Wed, 10 Oct 2012, Srinivas KANDAGATLA wrote:
> From: Srinivas Kandagatla
>
> This patch removes some code duplication by using
> module_platform_driver.
>
> Signed-off-by: Srinivas Kandagatla
Thanks for the patch. It is indeed correct, but an identical patch is
already upstream:
http://git.linuxtv.org/media_tree.git/commit/ec0341b3b7817a5e8ebcf26091dde28dce2d7821
Thanks
Guennadi
> ---
> drivers/media/platform/soc_camera/soc_camera.c | 14 +-
> 1 files changed, 1 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/platform/soc_camera/soc_camera.c
> b/drivers/media/platform/soc_camera/soc_camera.c
> index 3be9294..d4bfe29 100644
> --- a/drivers/media/platform/soc_camera/soc_camera.c
> +++ b/drivers/media/platform/soc_camera/soc_camera.c
> @@ -1585,19 +1585,7 @@ static struct platform_driver __refdata
> soc_camera_pdrv = {
> .owner = THIS_MODULE,
> },
> };
> -
> -static int __init soc_camera_init(void)
> -{
> - return platform_driver_register(&soc_camera_pdrv);
> -}
> -
> -static void __exit soc_camera_exit(void)
> -{
> - platform_driver_unregister(&soc_camera_pdrv);
> -}
> -
> -module_init(soc_camera_init);
> -module_exit(soc_camera_exit);
> +module_platform_driver(soc_camera_pdrv);
>
> MODULE_DESCRIPTION("Image capture bus driver");
> MODULE_AUTHOR("Guennadi Liakhovetski ");
> --
> 1.7.0.4
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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: A reliable kernel panic (3.6.2) and system crash when visiting a particular website
On Mon, 22 Oct 2012, Daniel Mack wrote: > On 22.10.2012 17:17, Alan Stern wrote: > > On Sun, 21 Oct 2012, Artem S. Tashkinov wrote: > > > >> dmesg messages up to a crash can be seen here: > >> https://bugzilla.kernel.org/attachment.cgi?id=84221 > > > > The first problem in the log is endpoint list corruption. Here's a > > debugging patch which should provide a little more information. > > Maybe add a BUG() after each of these dev_err() so we stop at the first > occurance and also see where we're coming from? A BUG() at these points would crash the machine hard. And where we came from doesn't matter; what matters is the values in the pointers. Alan Stern -- 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
[RESEND-PATCH] media:davinci: clk - {prepare/unprepare} for common clk
As a first step towards migrating davinci platforms to use common clock
framework, replace all instances of clk_enable() with clk_prepare_enable()
and clk_disable() with clk_disable_unprepare().
Also fixes some issues related to clk clean up in the driver
Signed-off-by: Murali Karicheri
---
rebased to v3.7-rc1
drivers/media/platform/davinci/dm355_ccdc.c |8 ++--
drivers/media/platform/davinci/dm644x_ccdc.c | 16 ++--
drivers/media/platform/davinci/isif.c|5 -
drivers/media/platform/davinci/vpbe.c| 10 +++---
drivers/media/platform/davinci/vpif.c|8
5 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/drivers/media/platform/davinci/dm355_ccdc.c
b/drivers/media/platform/davinci/dm355_ccdc.c
index ce0e413..030950d 100644
--- a/drivers/media/platform/davinci/dm355_ccdc.c
+++ b/drivers/media/platform/davinci/dm355_ccdc.c
@@ -1003,7 +1003,7 @@ static int __devinit dm355_ccdc_probe(struct
platform_device *pdev)
status = PTR_ERR(ccdc_cfg.mclk);
goto fail_nomap;
}
- if (clk_enable(ccdc_cfg.mclk)) {
+ if (clk_prepare_enable(ccdc_cfg.mclk)) {
status = -ENODEV;
goto fail_mclk;
}
@@ -1014,7 +1014,7 @@ static int __devinit dm355_ccdc_probe(struct
platform_device *pdev)
status = PTR_ERR(ccdc_cfg.sclk);
goto fail_mclk;
}
- if (clk_enable(ccdc_cfg.sclk)) {
+ if (clk_prepare_enable(ccdc_cfg.sclk)) {
status = -ENODEV;
goto fail_sclk;
}
@@ -1034,8 +1034,10 @@ static int __devinit dm355_ccdc_probe(struct
platform_device *pdev)
printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name);
return 0;
fail_sclk:
+ clk_disable_unprepare(ccdc_cfg.sclk);
clk_put(ccdc_cfg.sclk);
fail_mclk:
+ clk_disable_unprepare(ccdc_cfg.mclk);
clk_put(ccdc_cfg.mclk);
fail_nomap:
iounmap(ccdc_cfg.base_addr);
@@ -1050,6 +1052,8 @@ static int dm355_ccdc_remove(struct platform_device *pdev)
{
struct resource *res;
+ clk_disable_unprepare(ccdc_cfg.sclk);
+ clk_disable_unprepare(ccdc_cfg.mclk);
clk_put(ccdc_cfg.mclk);
clk_put(ccdc_cfg.sclk);
iounmap(ccdc_cfg.base_addr);
diff --git a/drivers/media/platform/davinci/dm644x_ccdc.c
b/drivers/media/platform/davinci/dm644x_ccdc.c
index ee7942b..0215ab6 100644
--- a/drivers/media/platform/davinci/dm644x_ccdc.c
+++ b/drivers/media/platform/davinci/dm644x_ccdc.c
@@ -994,7 +994,7 @@ static int __devinit dm644x_ccdc_probe(struct
platform_device *pdev)
status = PTR_ERR(ccdc_cfg.mclk);
goto fail_nomap;
}
- if (clk_enable(ccdc_cfg.mclk)) {
+ if (clk_prepare_enable(ccdc_cfg.mclk)) {
status = -ENODEV;
goto fail_mclk;
}
@@ -1005,7 +1005,7 @@ static int __devinit dm644x_ccdc_probe(struct
platform_device *pdev)
status = PTR_ERR(ccdc_cfg.sclk);
goto fail_mclk;
}
- if (clk_enable(ccdc_cfg.sclk)) {
+ if (clk_prepare_enable(ccdc_cfg.sclk)) {
status = -ENODEV;
goto fail_sclk;
}
@@ -1013,8 +1013,10 @@ static int __devinit dm644x_ccdc_probe(struct
platform_device *pdev)
printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name);
return 0;
fail_sclk:
+ clk_disable_unprepare(ccdc_cfg.sclk);
clk_put(ccdc_cfg.sclk);
fail_mclk:
+ clk_disable_unprepare(ccdc_cfg.mclk);
clk_put(ccdc_cfg.mclk);
fail_nomap:
iounmap(ccdc_cfg.base_addr);
@@ -1029,6 +1031,8 @@ static int dm644x_ccdc_remove(struct platform_device
*pdev)
{
struct resource *res;
+ clk_disable_unprepare(ccdc_cfg.mclk);
+ clk_disable_unprepare(ccdc_cfg.sclk);
clk_put(ccdc_cfg.mclk);
clk_put(ccdc_cfg.sclk);
iounmap(ccdc_cfg.base_addr);
@@ -1046,8 +1050,8 @@ static int dm644x_ccdc_suspend(struct device *dev)
/* Disable CCDC */
ccdc_enable(0);
/* Disable both master and slave clock */
- clk_disable(ccdc_cfg.mclk);
- clk_disable(ccdc_cfg.sclk);
+ clk_disable_unprepare(ccdc_cfg.mclk);
+ clk_disable_unprepare(ccdc_cfg.sclk);
return 0;
}
@@ -1055,8 +1059,8 @@ static int dm644x_ccdc_suspend(struct device *dev)
static int dm644x_ccdc_resume(struct device *dev)
{
/* Enable both master and slave clock */
- clk_enable(ccdc_cfg.mclk);
- clk_enable(ccdc_cfg.sclk);
+ clk_prepare_enable(ccdc_cfg.mclk);
+ clk_prepare_enable(ccdc_cfg.sclk);
/* Restore CCDC context */
ccdc_restore_context();
diff --git a/drivers/media/platform/davinci/isif.c
b/drivers/media/platform/davinci/isif.c
index b99d542..2c26c3e 100644
--- a/drivers/media/platform/davinci/isif.c
+++ b/drivers/media/platform/davinci/isif.c
@@ -1053,7 +1053
[RESEND-PATCH] media:davinci: clk - {prepare/unprepare} for common clk
As a first step towards migrating davinci platforms to use common clock
framework, replace all instances of clk_enable() with clk_prepare_enable()
and clk_disable() with clk_disable_unprepare().
Also fixes some issues related to clk clean up in the driver
Signed-off-by: Murali Karicheri
---
rebased to v3.7-rc1
drivers/media/platform/davinci/dm355_ccdc.c |8 ++--
drivers/media/platform/davinci/dm644x_ccdc.c | 16 ++--
drivers/media/platform/davinci/isif.c|5 -
drivers/media/platform/davinci/vpbe.c| 10 +++---
drivers/media/platform/davinci/vpif.c|8
5 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/drivers/media/platform/davinci/dm355_ccdc.c
b/drivers/media/platform/davinci/dm355_ccdc.c
index ce0e413..030950d 100644
--- a/drivers/media/platform/davinci/dm355_ccdc.c
+++ b/drivers/media/platform/davinci/dm355_ccdc.c
@@ -1003,7 +1003,7 @@ static int __devinit dm355_ccdc_probe(struct
platform_device *pdev)
status = PTR_ERR(ccdc_cfg.mclk);
goto fail_nomap;
}
- if (clk_enable(ccdc_cfg.mclk)) {
+ if (clk_prepare_enable(ccdc_cfg.mclk)) {
status = -ENODEV;
goto fail_mclk;
}
@@ -1014,7 +1014,7 @@ static int __devinit dm355_ccdc_probe(struct
platform_device *pdev)
status = PTR_ERR(ccdc_cfg.sclk);
goto fail_mclk;
}
- if (clk_enable(ccdc_cfg.sclk)) {
+ if (clk_prepare_enable(ccdc_cfg.sclk)) {
status = -ENODEV;
goto fail_sclk;
}
@@ -1034,8 +1034,10 @@ static int __devinit dm355_ccdc_probe(struct
platform_device *pdev)
printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name);
return 0;
fail_sclk:
+ clk_disable_unprepare(ccdc_cfg.sclk);
clk_put(ccdc_cfg.sclk);
fail_mclk:
+ clk_disable_unprepare(ccdc_cfg.mclk);
clk_put(ccdc_cfg.mclk);
fail_nomap:
iounmap(ccdc_cfg.base_addr);
@@ -1050,6 +1052,8 @@ static int dm355_ccdc_remove(struct platform_device *pdev)
{
struct resource *res;
+ clk_disable_unprepare(ccdc_cfg.sclk);
+ clk_disable_unprepare(ccdc_cfg.mclk);
clk_put(ccdc_cfg.mclk);
clk_put(ccdc_cfg.sclk);
iounmap(ccdc_cfg.base_addr);
diff --git a/drivers/media/platform/davinci/dm644x_ccdc.c
b/drivers/media/platform/davinci/dm644x_ccdc.c
index ee7942b..0215ab6 100644
--- a/drivers/media/platform/davinci/dm644x_ccdc.c
+++ b/drivers/media/platform/davinci/dm644x_ccdc.c
@@ -994,7 +994,7 @@ static int __devinit dm644x_ccdc_probe(struct
platform_device *pdev)
status = PTR_ERR(ccdc_cfg.mclk);
goto fail_nomap;
}
- if (clk_enable(ccdc_cfg.mclk)) {
+ if (clk_prepare_enable(ccdc_cfg.mclk)) {
status = -ENODEV;
goto fail_mclk;
}
@@ -1005,7 +1005,7 @@ static int __devinit dm644x_ccdc_probe(struct
platform_device *pdev)
status = PTR_ERR(ccdc_cfg.sclk);
goto fail_mclk;
}
- if (clk_enable(ccdc_cfg.sclk)) {
+ if (clk_prepare_enable(ccdc_cfg.sclk)) {
status = -ENODEV;
goto fail_sclk;
}
@@ -1013,8 +1013,10 @@ static int __devinit dm644x_ccdc_probe(struct
platform_device *pdev)
printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name);
return 0;
fail_sclk:
+ clk_disable_unprepare(ccdc_cfg.sclk);
clk_put(ccdc_cfg.sclk);
fail_mclk:
+ clk_disable_unprepare(ccdc_cfg.mclk);
clk_put(ccdc_cfg.mclk);
fail_nomap:
iounmap(ccdc_cfg.base_addr);
@@ -1029,6 +1031,8 @@ static int dm644x_ccdc_remove(struct platform_device
*pdev)
{
struct resource *res;
+ clk_disable_unprepare(ccdc_cfg.mclk);
+ clk_disable_unprepare(ccdc_cfg.sclk);
clk_put(ccdc_cfg.mclk);
clk_put(ccdc_cfg.sclk);
iounmap(ccdc_cfg.base_addr);
@@ -1046,8 +1050,8 @@ static int dm644x_ccdc_suspend(struct device *dev)
/* Disable CCDC */
ccdc_enable(0);
/* Disable both master and slave clock */
- clk_disable(ccdc_cfg.mclk);
- clk_disable(ccdc_cfg.sclk);
+ clk_disable_unprepare(ccdc_cfg.mclk);
+ clk_disable_unprepare(ccdc_cfg.sclk);
return 0;
}
@@ -1055,8 +1059,8 @@ static int dm644x_ccdc_suspend(struct device *dev)
static int dm644x_ccdc_resume(struct device *dev)
{
/* Enable both master and slave clock */
- clk_enable(ccdc_cfg.mclk);
- clk_enable(ccdc_cfg.sclk);
+ clk_prepare_enable(ccdc_cfg.mclk);
+ clk_prepare_enable(ccdc_cfg.sclk);
/* Restore CCDC context */
ccdc_restore_context();
diff --git a/drivers/media/platform/davinci/isif.c
b/drivers/media/platform/davinci/isif.c
index b99d542..2c26c3e 100644
--- a/drivers/media/platform/davinci/isif.c
+++ b/drivers/media/platform/davinci/isif.c
@@ -1053,7 +1053
Re: A reliable kernel panic (3.6.2) and system crash when visiting a particular website
On 22.10.2012 17:17, Alan Stern wrote:
> On Sun, 21 Oct 2012, Artem S. Tashkinov wrote:
>
>> dmesg messages up to a crash can be seen here:
>> https://bugzilla.kernel.org/attachment.cgi?id=84221
>
> The first problem in the log is endpoint list corruption. Here's a
> debugging patch which should provide a little more information.
Maybe add a BUG() after each of these dev_err() so we stop at the first
occurance and also see where we're coming from?
> drivers/usb/core/hcd.c | 36
> 1 file changed, 36 insertions(+)
>
> Index: usb-3.6/drivers/usb/core/hcd.c
> ===
> --- usb-3.6.orig/drivers/usb/core/hcd.c
> +++ usb-3.6/drivers/usb/core/hcd.c
> @@ -1083,6 +1083,8 @@ EXPORT_SYMBOL_GPL(usb_calc_bus_time);
>
> /*-*/
>
> +static bool list_error;
> +
> /**
> * usb_hcd_link_urb_to_ep - add an URB to its endpoint queue
> * @hcd: host controller to which @urb was submitted
> @@ -1126,6 +1128,20 @@ int usb_hcd_link_urb_to_ep(struct usb_hc
>*/
> if (HCD_RH_RUNNING(hcd)) {
> urb->unlinked = 0;
> +
> + {
> + struct list_head *cur = &urb->ep->urb_list;
> + struct list_head *prev = cur->prev;
> +
> + if (prev->next != cur && !list_error) {
> + list_error = true;
> + dev_err(&urb->dev->dev,
> + "ep %x list add corruption: %p %p %p\n",
> + urb->ep->desc.bEndpointAddress,
> + cur, prev, prev->next);
> + }
> + }
> +
> list_add_tail(&urb->urb_list, &urb->ep->urb_list);
> } else {
> rc = -ESHUTDOWN;
> @@ -1193,6 +1209,26 @@ void usb_hcd_unlink_urb_from_ep(struct u
> {
> /* clear all state linking urb to this dev (and hcd) */
> spin_lock(&hcd_urb_list_lock);
> + {
> + struct list_head *cur = &urb->urb_list;
> + struct list_head *prev = cur->prev;
> + struct list_head *next = cur->next;
> +
> + if (prev->next != cur && !list_error) {
> + list_error = true;
> + dev_err(&urb->dev->dev,
> + "ep %x list del corruption prev: %p %p %p\n",
> + urb->ep->desc.bEndpointAddress,
> + cur, prev, prev->next);
> + }
> + if (next->prev != cur && !list_error) {
> + list_error = true;
> + dev_err(&urb->dev->dev,
> + "ep %x list del corruption next: %p %p %p\n",
> + urb->ep->desc.bEndpointAddress,
> + cur, next, next->prev);
> + }
> + }
> list_del_init(&urb->urb_list);
> spin_unlock(&hcd_urb_list_lock);
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] media: V4L2: support asynchronous subdevice registration
On Mon October 22 2012 16:48:05 Guennadi Liakhovetski wrote: > On Mon, 22 Oct 2012, Hans Verkuil wrote: > > > On Mon October 22 2012 14:50:14 Guennadi Liakhovetski wrote: > > > On Mon, 22 Oct 2012, Hans Verkuil wrote: > > > > > > > On Mon October 22 2012 13:08:12 Guennadi Liakhovetski wrote: > > > > > Hi Hans > > > > > > > > > > Thanks for reviewing the patch. > > > > > > > > > > On Mon, 22 Oct 2012, Hans Verkuil wrote: > > > > > > > > > > > Hi Guennadi, > > > > > > > > > > > > I've reviewed this patch and I have a few questions: > > > > > > > > > > > > On Sat October 20 2012 00:20:24 Guennadi Liakhovetski wrote: > > > > > > > Currently bridge device drivers register devices for all > > > > > > > subdevices > > > > > > > synchronously, tupically, during their probing. E.g. if an I2C > > > > > > > CMOS sensor > > > > > > > is attached to a video bridge device, the bridge driver will > > > > > > > create an I2C > > > > > > > device and wait for the respective I2C driver to probe. This > > > > > > > makes linking > > > > > > > of devices straight forward, but this approach cannot be used with > > > > > > > intrinsically asynchronous and unordered device registration > > > > > > > systems like > > > > > > > the Flattened Device Tree. To support such systems this patch > > > > > > > adds an > > > > > > > asynchronous subdevice registration framework to V4L2. To use it > > > > > > > respective > > > > > > > (e.g. I2C) subdevice drivers must request deferred probing as > > > > > > > long as their > > > > > > > bridge driver hasn't probed. The bridge driver during its probing > > > > > > > submits a > > > > > > > an arbitrary number of subdevice descriptor groups to the > > > > > > > framework to > > > > > > > manage. After that it can add callbacks to each of those groups > > > > > > > to be > > > > > > > called at various stages during subdevice probing, e.g. after > > > > > > > completion. > > > > > > > Then the bridge driver can request single groups to be probed, > > > > > > > finish its > > > > > > > own probing and continue its video subsystem configuration from > > > > > > > its > > > > > > > callbacks. > > > > > > > > > > > > What is the purpose of allowing multiple groups? > > > > > > > > > > To support, e.g. multiple sensors connected to a single bridge. > > > > > > > > So, isn't that one group with two sensor subdevs? > > > > > > No, one group consists of all subdevices, necessary to operate a single > > > video pipeline. A simple group only contains a sensor. More complex > > > groups > > > can contain a CSI-2 interface, a line shifter, or anything else. > > > > Why? Why would you want to wait for completion of multiple groups? You need > > all > > subdevs to be registered. If you split them up in multiple groups, then you > > have to wait until all those groups have completed, which only makes the > > bridge > > driver more complex. It adds nothing to the problem that we're trying to > > solve. > > I see it differently. Firstly, there's no waiting. If they are independent, then that's true. But in almost all cases you need them all. Even in cases where theoretically you can 'activate' groups independently, it doesn't add anything. It's overengineering, trying to solve a problem that doesn't exist. Just keep it simple, that's hard enough. > Secondly, you don't > need all of them. With groups as soon as one group is complete you can > start using it. If you require all your subdevices to complete their > probing before you can use anything. In fact, some subdevices might never > probe, but groups, that don't need them can be used regardless. > > > > > A bridge driver has a list of subdevs. There is no concept of 'groups'. > > > > Perhaps > > > > I misunderstand? > > > > > > Well, we have a group ID, which can be used for what I'm proposing groups > > > for. At least on soc-camera we use the group ID exactly for this purpose. > > > We attach all subdevices to a V4L2 device, but assign group IDs according > > > to pipelines. Then subdevice operations only act on members of one > > > pipeline. I know that we currently don't specify precisely what that > > > group > > > ID should be used for in general. So, this my group concept is an > > > extension of what we currently have in V4L2. > > > > How the grp_id field is used is entirely up to the bridge driver. It may not > > be used at all, it may uniquely identify each subdev, it may put each subdev > > in a particular group and perhaps a single subdev might belong to multiple > > groups. There is no standard concept of a group. It's just a simple method > > (actually, more of a hack) of allowing bridge drivers to call ops for some > > subset of the sub-devices. > > Yes, I know, at least it's something that loosely indicates a group > concept in the current code:-) > > > Frankly, I wonder if most of the drivers that use grp_id actually need it at > > all. > > > > Just drop the group concept, things can b
Re: Re: Re: Re: Re: Re: A reliable kernel panic (3.6.2) and system crash when visiting a particular website
On Sun, 21 Oct 2012, Artem S. Tashkinov wrote:
> dmesg messages up to a crash can be seen here:
> https://bugzilla.kernel.org/attachment.cgi?id=84221
The first problem in the log is endpoint list corruption. Here's a
debugging patch which should provide a little more information.
Alan Stern
drivers/usb/core/hcd.c | 36
1 file changed, 36 insertions(+)
Index: usb-3.6/drivers/usb/core/hcd.c
===
--- usb-3.6.orig/drivers/usb/core/hcd.c
+++ usb-3.6/drivers/usb/core/hcd.c
@@ -1083,6 +1083,8 @@ EXPORT_SYMBOL_GPL(usb_calc_bus_time);
/*-*/
+static bool list_error;
+
/**
* usb_hcd_link_urb_to_ep - add an URB to its endpoint queue
* @hcd: host controller to which @urb was submitted
@@ -1126,6 +1128,20 @@ int usb_hcd_link_urb_to_ep(struct usb_hc
*/
if (HCD_RH_RUNNING(hcd)) {
urb->unlinked = 0;
+
+ {
+ struct list_head *cur = &urb->ep->urb_list;
+ struct list_head *prev = cur->prev;
+
+ if (prev->next != cur && !list_error) {
+ list_error = true;
+ dev_err(&urb->dev->dev,
+ "ep %x list add corruption: %p %p %p\n",
+ urb->ep->desc.bEndpointAddress,
+ cur, prev, prev->next);
+ }
+ }
+
list_add_tail(&urb->urb_list, &urb->ep->urb_list);
} else {
rc = -ESHUTDOWN;
@@ -1193,6 +1209,26 @@ void usb_hcd_unlink_urb_from_ep(struct u
{
/* clear all state linking urb to this dev (and hcd) */
spin_lock(&hcd_urb_list_lock);
+ {
+ struct list_head *cur = &urb->urb_list;
+ struct list_head *prev = cur->prev;
+ struct list_head *next = cur->next;
+
+ if (prev->next != cur && !list_error) {
+ list_error = true;
+ dev_err(&urb->dev->dev,
+ "ep %x list del corruption prev: %p %p %p\n",
+ urb->ep->desc.bEndpointAddress,
+ cur, prev, prev->next);
+ }
+ if (next->prev != cur && !list_error) {
+ list_error = true;
+ dev_err(&urb->dev->dev,
+ "ep %x list del corruption next: %p %p %p\n",
+ urb->ep->desc.bEndpointAddress,
+ cur, next, next->prev);
+ }
+ }
list_del_init(&urb->urb_list);
spin_unlock(&hcd_urb_list_lock);
}
--
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:davinci: clk - {prepare/unprepare} for common clk framework
On 10/20/2012 02:39 AM, Prabhakar Lad wrote:
Hi Murali,
On Friday 19 October 2012 11:42 PM, Murali Karicheri wrote:
As a first step towards migrating davinci platforms to use common clock
framework, replace all instances of clk_enable() with clk_prepare_enable()
and clk_disable() with clk_disable_unprepare().
Also fixes some issues related to clk clean up in the driver
Signed-off-by: Murali Karicheri
---
drivers/media/video/davinci/dm355_ccdc.c |8 ++--
drivers/media/video/davinci/dm644x_ccdc.c | 16 ++--
drivers/media/video/davinci/isif.c|5 -
drivers/media/video/davinci/vpbe.c| 10 +++---
drivers/media/video/davinci/vpif.c|8
5 files changed, 31 insertions(+), 16 deletions(-)
Thanks for the patch. Can you rebase this patch on 3.7, Since
the folder structure for media drivers has been reorganised.
And for some reason this patch hasn't reached any mailing list.
Thanks And Regards,
--Prabhakar Lad
Prabakhar,
I have re-sent the patches rebased to 3.7rc1. You are right, at times my
patches don't show up on the mailing list. I am not sure why. It is not
that none of them reaches the list, but some do make it. Do you have any
suggestion why this could be happening? I will check with our IT first
to see if emails are getting blocked at TI server for some reason.
Meanwhile, could you test these patches with video drivers and request a
merge to 3.7rcx? The patches are available at
https://gitorious.org/~m-karicheri/linux-davinci/linux-davinci-clk/commits/video-clk-patches-v3.7rc1
https://gitorious.org/~m-karicheri/linux-davinci/linux-davinci-clk/commit/539b2de5913000cbf6f594e63b680673368dd9a2
Murali
diff --git a/drivers/media/video/davinci/dm355_ccdc.c
b/drivers/media/video/davinci/dm355_ccdc.c
index 5b68847..af88cce 100644
--- a/drivers/media/video/davinci/dm355_ccdc.c
+++ b/drivers/media/video/davinci/dm355_ccdc.c
@@ -1003,7 +1003,7 @@ static int __init dm355_ccdc_probe(struct platform_device
*pdev)
status = PTR_ERR(ccdc_cfg.mclk);
goto fail_nomap;
}
- if (clk_enable(ccdc_cfg.mclk)) {
+ if (clk_prepare_enable(ccdc_cfg.mclk)) {
status = -ENODEV;
goto fail_mclk;
}
@@ -1014,7 +1014,7 @@ static int __init dm355_ccdc_probe(struct platform_device
*pdev)
status = PTR_ERR(ccdc_cfg.sclk);
goto fail_mclk;
}
- if (clk_enable(ccdc_cfg.sclk)) {
+ if (clk_prepare_enable(ccdc_cfg.sclk)) {
status = -ENODEV;
goto fail_sclk;
}
@@ -1034,8 +1034,10 @@ static int __init dm355_ccdc_probe(struct
platform_device *pdev)
printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name);
return 0;
fail_sclk:
+ clk_disable_unprepare(ccdc_cfg.sclk);
clk_put(ccdc_cfg.sclk);
fail_mclk:
+ clk_disable_unprepare(ccdc_cfg.mclk);
clk_put(ccdc_cfg.mclk);
fail_nomap:
iounmap(ccdc_cfg.base_addr);
@@ -1050,6 +1052,8 @@ static int dm355_ccdc_remove(struct platform_device *pdev)
{
struct resource *res;
+ clk_disable_unprepare(ccdc_cfg.sclk);
+ clk_disable_unprepare(ccdc_cfg.mclk);
clk_put(ccdc_cfg.mclk);
clk_put(ccdc_cfg.sclk);
iounmap(ccdc_cfg.base_addr);
diff --git a/drivers/media/video/davinci/dm644x_ccdc.c
b/drivers/media/video/davinci/dm644x_ccdc.c
index 9303fe5..24388fa 100644
--- a/drivers/media/video/davinci/dm644x_ccdc.c
+++ b/drivers/media/video/davinci/dm644x_ccdc.c
@@ -994,7 +994,7 @@ static int __init dm644x_ccdc_probe(struct platform_device
*pdev)
status = PTR_ERR(ccdc_cfg.mclk);
goto fail_nomap;
}
- if (clk_enable(ccdc_cfg.mclk)) {
+ if (clk_prepare_enable(ccdc_cfg.mclk)) {
status = -ENODEV;
goto fail_mclk;
}
@@ -1005,7 +1005,7 @@ static int __init dm644x_ccdc_probe(struct
platform_device *pdev)
status = PTR_ERR(ccdc_cfg.sclk);
goto fail_mclk;
}
- if (clk_enable(ccdc_cfg.sclk)) {
+ if (clk_prepare_enable(ccdc_cfg.sclk)) {
status = -ENODEV;
goto fail_sclk;
}
@@ -1013,8 +1013,10 @@ static int __init dm644x_ccdc_probe(struct
platform_device *pdev)
printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name);
return 0;
fail_sclk:
+ clk_disable_unprepare(ccdc_cfg.sclk);
clk_put(ccdc_cfg.sclk);
fail_mclk:
+ clk_disable_unprepare(ccdc_cfg.mclk);
clk_put(ccdc_cfg.mclk);
fail_nomap:
iounmap(ccdc_cfg.base_addr);
@@ -1029,6 +1031,8 @@ static int dm644x_ccdc_remove(struct platform_device
*pdev)
{
struct resource *res;
+ clk_disable_unprepare(ccdc_cfg.mclk);
+ clk_disable_unprepare(ccdc_cfg.sclk);
clk_put(ccdc_cfg.mclk);
clk_put(ccdc_cfg.sclk);
iounmap(ccdc_cfg.base
Re: [PATCH 2/2] media: V4L2: support asynchronous subdevice registration
On Mon, 22 Oct 2012, Hans Verkuil wrote: > On Mon October 22 2012 14:50:14 Guennadi Liakhovetski wrote: > > On Mon, 22 Oct 2012, Hans Verkuil wrote: > > > > > On Mon October 22 2012 13:08:12 Guennadi Liakhovetski wrote: > > > > Hi Hans > > > > > > > > Thanks for reviewing the patch. > > > > > > > > On Mon, 22 Oct 2012, Hans Verkuil wrote: > > > > > > > > > Hi Guennadi, > > > > > > > > > > I've reviewed this patch and I have a few questions: > > > > > > > > > > On Sat October 20 2012 00:20:24 Guennadi Liakhovetski wrote: > > > > > > Currently bridge device drivers register devices for all subdevices > > > > > > synchronously, tupically, during their probing. E.g. if an I2C CMOS > > > > > > sensor > > > > > > is attached to a video bridge device, the bridge driver will create > > > > > > an I2C > > > > > > device and wait for the respective I2C driver to probe. This makes > > > > > > linking > > > > > > of devices straight forward, but this approach cannot be used with > > > > > > intrinsically asynchronous and unordered device registration > > > > > > systems like > > > > > > the Flattened Device Tree. To support such systems this patch adds > > > > > > an > > > > > > asynchronous subdevice registration framework to V4L2. To use it > > > > > > respective > > > > > > (e.g. I2C) subdevice drivers must request deferred probing as long > > > > > > as their > > > > > > bridge driver hasn't probed. The bridge driver during its probing > > > > > > submits a > > > > > > an arbitrary number of subdevice descriptor groups to the framework > > > > > > to > > > > > > manage. After that it can add callbacks to each of those groups to > > > > > > be > > > > > > called at various stages during subdevice probing, e.g. after > > > > > > completion. > > > > > > Then the bridge driver can request single groups to be probed, > > > > > > finish its > > > > > > own probing and continue its video subsystem configuration from its > > > > > > callbacks. > > > > > > > > > > What is the purpose of allowing multiple groups? > > > > > > > > To support, e.g. multiple sensors connected to a single bridge. > > > > > > So, isn't that one group with two sensor subdevs? > > > > No, one group consists of all subdevices, necessary to operate a single > > video pipeline. A simple group only contains a sensor. More complex groups > > can contain a CSI-2 interface, a line shifter, or anything else. > > Why? Why would you want to wait for completion of multiple groups? You need > all > subdevs to be registered. If you split them up in multiple groups, then you > have to wait until all those groups have completed, which only makes the > bridge > driver more complex. It adds nothing to the problem that we're trying to > solve. I see it differently. Firstly, there's no waiting. Secondly, you don't need all of them. With groups as soon as one group is complete you can start using it. If you require all your subdevices to complete their probing before you can use anything. In fact, some subdevices might never probe, but groups, that don't need them can be used regardless. > > > A bridge driver has a list of subdevs. There is no concept of 'groups'. > > > Perhaps > > > I misunderstand? > > > > Well, we have a group ID, which can be used for what I'm proposing groups > > for. At least on soc-camera we use the group ID exactly for this purpose. > > We attach all subdevices to a V4L2 device, but assign group IDs according > > to pipelines. Then subdevice operations only act on members of one > > pipeline. I know that we currently don't specify precisely what that group > > ID should be used for in general. So, this my group concept is an > > extension of what we currently have in V4L2. > > How the grp_id field is used is entirely up to the bridge driver. It may not > be used at all, it may uniquely identify each subdev, it may put each subdev > in a particular group and perhaps a single subdev might belong to multiple > groups. There is no standard concept of a group. It's just a simple method > (actually, more of a hack) of allowing bridge drivers to call ops for some > subset of the sub-devices. Yes, I know, at least it's something that loosely indicates a group concept in the current code:-) > Frankly, I wonder if most of the drivers that use grp_id actually need it at > all. > > Just drop the group concept, things can be simplified quite a bit without it. So far I think we should keep it. Also think about our DT layout. A bridge can have several ports each with multiple links (maybe it has already been decided to change names, don't remember by heart, sorry). Each of them would then start a group. > > > > > I can't think of any reason > > > > > why you would want to have more than one group. If you have just one > > > > > group > > > > > you can simplify this code quite a bit: most of the v4l2_async_group > > > > > fields > > > > > can just become part of struct v4l2_device, you don't need the 'lis
Re: [PATCH 1/2] [media] vivi: Kill BUFFER_TIMEOUT macro
On Mon October 22 2012 15:54:43 Kirill Smelkov wrote: > Usage of BUFFER_TIMEOUT has gone in 2008 in 78718e5d (V4L/DVB (7492): > vivi: Simplify the vivi driver and avoid deadlocks), but the macro > remains. Say goodbye to it. > > Signed-off-by: Kirill Smelkov Acked-by: Hans Verkuil > --- > drivers/media/platform/vivi.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c > index b366b05..3e6902a 100644 > --- a/drivers/media/platform/vivi.c > +++ b/drivers/media/platform/vivi.c > @@ -39,7 +39,6 @@ > /* Wake up at about 30 fps */ > #define WAKE_NUMERATOR 30 > #define WAKE_DENOMINATOR 1001 > -#define BUFFER_TIMEOUT msecs_to_jiffies(500) /* 0.5 seconds */ > > #define MAX_WIDTH 1920 > #define MAX_HEIGHT 1200 > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [media] vivi: Teach it to tune FPS
On Mon October 22 2012 15:54:44 Kirill Smelkov wrote:
> I was testing my video-over-ethernet subsystem today, and vivi seemed to
> be perfect video source for testing when one don't have lots of capture
> boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
> while in my country we usually use PAL (25 fps). That's why the patch.
> Thanks.
Rather than introducing a module option, it's much nicer if you can
implement enum_frameintervals and g/s_parm. This can be made quite flexible
allowing you to also support 50/59.94/60 fps.
Regards,
Hans
>
> Signed-off-by: Kirill Smelkov
> ---
> drivers/media/platform/vivi.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
> index 3e6902a..48325f4 100644
> --- a/drivers/media/platform/vivi.c
> +++ b/drivers/media/platform/vivi.c
> @@ -36,10 +36,6 @@
>
> #define VIVI_MODULE_NAME "vivi"
>
> -/* Wake up at about 30 fps */
> -#define WAKE_NUMERATOR 30
> -#define WAKE_DENOMINATOR 1001
> -
> #define MAX_WIDTH 1920
> #define MAX_HEIGHT 1200
>
> @@ -58,6 +54,11 @@ static unsigned n_devs = 1;
> module_param(n_devs, uint, 0644);
> MODULE_PARM_DESC(n_devs, "number of video devices to create");
>
> +static struct v4l2_fract fps = { 3, 1001 }; /* ~ 30 fps by default */
> +static unsigned __fps[2], __nfps;
> +module_param_array_named(fps, __fps, uint, &__nfps, 0644);
> +MODULE_PARM_DESC(fps, "frames per second as ratio (e.g. 3,1001 or
> 25,1)");
> +
> static unsigned debug;
> module_param(debug, uint, 0644);
> MODULE_PARM_DESC(debug, "activates debug info");
> @@ -661,7 +662,7 @@ static void vivi_thread_tick(struct vivi_dev *dev)
> }
>
> #define frames_to_ms(frames) \
> - ((frames * WAKE_NUMERATOR * 1000) / WAKE_DENOMINATOR)
> + ((frames * fps.denominator * 1000) / fps.numerator)
>
> static void vivi_sleep(struct vivi_dev *dev)
> {
> @@ -1376,6 +1377,13 @@ static int __init vivi_init(void)
> if (n_devs <= 0)
> n_devs = 1;
>
> + if (__nfps > 0) {
> + fps.numerator = __fps[0];
> + fps.denominator = (__nfps > 1) ? __fps[1] : 1;
> + }
> + if (fps.numerator <= 0)
> + fps.numerator = 1;
> +
> for (i = 0; i < n_devs; i++) {
> ret = vivi_create_instance(i);
> if (ret) {
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] [media] vivi: Teach it to tune FPS
I was testing my video-over-ethernet subsystem today, and vivi seemed to
be perfect video source for testing when one don't have lots of capture
boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
while in my country we usually use PAL (25 fps). That's why the patch.
Thanks.
Signed-off-by: Kirill Smelkov
---
drivers/media/platform/vivi.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index 3e6902a..48325f4 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -36,10 +36,6 @@
#define VIVI_MODULE_NAME "vivi"
-/* Wake up at about 30 fps */
-#define WAKE_NUMERATOR 30
-#define WAKE_DENOMINATOR 1001
-
#define MAX_WIDTH 1920
#define MAX_HEIGHT 1200
@@ -58,6 +54,11 @@ static unsigned n_devs = 1;
module_param(n_devs, uint, 0644);
MODULE_PARM_DESC(n_devs, "number of video devices to create");
+static struct v4l2_fract fps = { 3, 1001 }; /* ~ 30 fps by default */
+static unsigned __fps[2], __nfps;
+module_param_array_named(fps, __fps, uint, &__nfps, 0644);
+MODULE_PARM_DESC(fps, "frames per second as ratio (e.g. 3,1001 or 25,1)");
+
static unsigned debug;
module_param(debug, uint, 0644);
MODULE_PARM_DESC(debug, "activates debug info");
@@ -661,7 +662,7 @@ static void vivi_thread_tick(struct vivi_dev *dev)
}
#define frames_to_ms(frames) \
- ((frames * WAKE_NUMERATOR * 1000) / WAKE_DENOMINATOR)
+ ((frames * fps.denominator * 1000) / fps.numerator)
static void vivi_sleep(struct vivi_dev *dev)
{
@@ -1376,6 +1377,13 @@ static int __init vivi_init(void)
if (n_devs <= 0)
n_devs = 1;
+ if (__nfps > 0) {
+ fps.numerator = __fps[0];
+ fps.denominator = (__nfps > 1) ? __fps[1] : 1;
+ }
+ if (fps.numerator <= 0)
+ fps.numerator = 1;
+
for (i = 0; i < n_devs; i++) {
ret = vivi_create_instance(i);
if (ret) {
--
1.8.0.rc3.331.g5b9a629
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] [media] vivi: Kill BUFFER_TIMEOUT macro
Usage of BUFFER_TIMEOUT has gone in 2008 in 78718e5d (V4L/DVB (7492): vivi: Simplify the vivi driver and avoid deadlocks), but the macro remains. Say goodbye to it. Signed-off-by: Kirill Smelkov --- drivers/media/platform/vivi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c index b366b05..3e6902a 100644 --- a/drivers/media/platform/vivi.c +++ b/drivers/media/platform/vivi.c @@ -39,7 +39,6 @@ /* Wake up at about 30 fps */ #define WAKE_NUMERATOR 30 #define WAKE_DENOMINATOR 1001 -#define BUFFER_TIMEOUT msecs_to_jiffies(500) /* 0.5 seconds */ #define MAX_WIDTH 1920 #define MAX_HEIGHT 1200 -- 1.8.0.rc3.331.g5b9a629 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] media: V4L2: support asynchronous subdevice registration
On Mon October 22 2012 14:50:14 Guennadi Liakhovetski wrote: > On Mon, 22 Oct 2012, Hans Verkuil wrote: > > > On Mon October 22 2012 13:08:12 Guennadi Liakhovetski wrote: > > > Hi Hans > > > > > > Thanks for reviewing the patch. > > > > > > On Mon, 22 Oct 2012, Hans Verkuil wrote: > > > > > > > Hi Guennadi, > > > > > > > > I've reviewed this patch and I have a few questions: > > > > > > > > On Sat October 20 2012 00:20:24 Guennadi Liakhovetski wrote: > > > > > Currently bridge device drivers register devices for all subdevices > > > > > synchronously, tupically, during their probing. E.g. if an I2C CMOS > > > > > sensor > > > > > is attached to a video bridge device, the bridge driver will create > > > > > an I2C > > > > > device and wait for the respective I2C driver to probe. This makes > > > > > linking > > > > > of devices straight forward, but this approach cannot be used with > > > > > intrinsically asynchronous and unordered device registration systems > > > > > like > > > > > the Flattened Device Tree. To support such systems this patch adds an > > > > > asynchronous subdevice registration framework to V4L2. To use it > > > > > respective > > > > > (e.g. I2C) subdevice drivers must request deferred probing as long as > > > > > their > > > > > bridge driver hasn't probed. The bridge driver during its probing > > > > > submits a > > > > > an arbitrary number of subdevice descriptor groups to the framework to > > > > > manage. After that it can add callbacks to each of those groups to be > > > > > called at various stages during subdevice probing, e.g. after > > > > > completion. > > > > > Then the bridge driver can request single groups to be probed, finish > > > > > its > > > > > own probing and continue its video subsystem configuration from its > > > > > callbacks. > > > > > > > > What is the purpose of allowing multiple groups? > > > > > > To support, e.g. multiple sensors connected to a single bridge. > > > > So, isn't that one group with two sensor subdevs? > > No, one group consists of all subdevices, necessary to operate a single > video pipeline. A simple group only contains a sensor. More complex groups > can contain a CSI-2 interface, a line shifter, or anything else. Why? Why would you want to wait for completion of multiple groups? You need all subdevs to be registered. If you split them up in multiple groups, then you have to wait until all those groups have completed, which only makes the bridge driver more complex. It adds nothing to the problem that we're trying to solve. > > A bridge driver has a list of subdevs. There is no concept of 'groups'. > > Perhaps > > I misunderstand? > > Well, we have a group ID, which can be used for what I'm proposing groups > for. At least on soc-camera we use the group ID exactly for this purpose. > We attach all subdevices to a V4L2 device, but assign group IDs according > to pipelines. Then subdevice operations only act on members of one > pipeline. I know that we currently don't specify precisely what that group > ID should be used for in general. So, this my group concept is an > extension of what we currently have in V4L2. How the grp_id field is used is entirely up to the bridge driver. It may not be used at all, it may uniquely identify each subdev, it may put each subdev in a particular group and perhaps a single subdev might belong to multiple groups. There is no standard concept of a group. It's just a simple method (actually, more of a hack) of allowing bridge drivers to call ops for some subset of the sub-devices. Frankly, I wonder if most of the drivers that use grp_id actually need it at all. Just drop the group concept, things can be simplified quite a bit without it. > > > > > I can't think of any reason > > > > why you would want to have more than one group. If you have just one > > > > group > > > > you can simplify this code quite a bit: most of the v4l2_async_group > > > > fields > > > > can just become part of struct v4l2_device, you don't need the 'list' > > > > and > > > > 'v4l2_dev' fields anymore and the 'bind' and 'complete' callbacks can be > > > > implemented using the v4l2_device notify callback which we already have. > > > > > > > > > > > > > > Signed-off-by: Guennadi Liakhovetski > > > > > --- > > > > > > > > > > One more thing to note about this patch. Subdevice drivers, > > > > > supporting > > > > > asynchronous probing, and using this framework, need a unified way to > > > > > detect, whether their probing should succeed or they should request > > > > > deferred probing. I implement this using device platform data. This > > > > > means, > > > > > that all subdevice drivers, wishing to use this API will have to use > > > > > the > > > > > same platform data struct. I don't think this is a major > > > > > inconvenience, > > > > > but if we decide against this, we'll have to add a V4L2 function to > > > > > verify > > > > > "are you ready for me or not." The latter
Re: [PATCH 1/2] media: V4L2: add temporary clock helpers
Hi Guennadi,
A couple more comments.
On Saturday 20 October 2012 00:20:20 Guennadi Liakhovetski wrote:
> Typical video devices like camera sensors require an external clock source.
> Many such devices cannot even access their hardware registers without a
> running clock. These clock sources should be controlled by their consumers.
> This should be performed, using the generic clock framework. Unfortunately
> so far only very few systems have been ported to that framework. This patch
> adds a set of temporary helpers, mimicking the generic clock API, to V4L2.
> Platforms, adopting the clock API, should switch to using it. Eventually
> this temporary API should be removed.
>
> Signed-off-by: Guennadi Liakhovetski
> ---
> drivers/media/v4l2-core/Makefile |2 +-
> drivers/media/v4l2-core/v4l2-clk.c | 126 +
> include/media/v4l2-clk.h | 48 ++
> 3 files changed, 175 insertions(+), 1 deletions(-)
> create mode 100644 drivers/media/v4l2-core/v4l2-clk.c
> create mode 100644 include/media/v4l2-clk.h
>
> diff --git a/drivers/media/v4l2-core/Makefile
> b/drivers/media/v4l2-core/Makefile index 00f64d6..cb5fede 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -5,7 +5,7 @@
> tuner-objs := tuner-core.o
>
> videodev-objs:= v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o
> \
> - v4l2-event.o v4l2-ctrls.o v4l2-subdev.o
> + v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
> ifeq ($(CONFIG_COMPAT),y)
>videodev-objs += v4l2-compat-ioctl32.o
> endif
> diff --git a/drivers/media/v4l2-core/v4l2-clk.c
> b/drivers/media/v4l2-core/v4l2-clk.c new file mode 100644
> index 000..7d457e4
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-clk.c
> @@ -0,0 +1,126 @@
> +/*
> + * V4L2 clock service
> + *
> + * Copyright (C) 2012, Guennadi Liakhovetski
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +static DEFINE_MUTEX(clk_lock);
> +static LIST_HEAD(v4l2_clk);
> +
> +struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id)
> +{
> + struct v4l2_clk *clk = NULL;
> +
> + mutex_lock(&clk_lock);
> + if (!id) {
> + if (list_is_singular(&v4l2_clk)) {
> + clk = list_entry(&v4l2_clk, struct v4l2_clk, list);
> + if (!strstr(sd->name, clk->dev_id))
> + clk = ERR_PTR(-ENODEV);
> + } else {
> + clk = ERR_PTR(-EINVAL);
> + }
> + } else {
> + list_for_each_entry(clk, &v4l2_clk, list) {
> + if (!strcmp(id, clk->id) &&
> + !strcmp(sd->name, clk->dev_id))
> + break;
> + }
> + if (&clk->list == &v4l2_clk)
> + clk = ERR_PTR(-ENODEV);
> + }
> + mutex_unlock(&clk_lock);
> +
> + if (!IS_ERR(clk) &&
> + !try_module_get(clk->ops->owner))
> + clk = ERR_PTR(-ENODEV);
> +
> + return clk;
> +}
> +EXPORT_SYMBOL(v4l2_clk_get);
> +
> +void v4l2_clk_put(struct v4l2_clk *clk)
> +{
> + if (!IS_ERR(clk))
> + module_put(clk->ops->owner);
> +}
> +EXPORT_SYMBOL(v4l2_clk_put);
> +
> +int v4l2_clk_enable(struct v4l2_clk *clk)
> +{
> + if (!clk->ops->enable)
> + return -ENOSYS;
Are enable/disable supposed to be refcounted ? What about adding the refcount
(or a boolean enable field) to struct v4l2_clk and handling it here and in
v4l2_clk_disable() ?
> + return clk->ops->enable(clk);
> +}
> +EXPORT_SYMBOL(v4l2_clk_enable);
> +
> +void v4l2_clk_disable(struct v4l2_clk *clk)
> +{
> + if (clk->ops->disable)
> + clk->ops->disable(clk);
> +}
> +EXPORT_SYMBOL(v4l2_clk_disable);
> +
> +unsigned long v4l2_clk_get_rate(struct v4l2_clk *clk)
> +{
> + if (!clk->ops->get_rate)
> + return -ENOSYS;
> + return clk->ops->get_rate(clk);
> +}
> +EXPORT_SYMBOL(v4l2_clk_get_rate);
> +
> +int v4l2_clk_set_rate(struct v4l2_clk *clk, unsigned long rate)
> +{
> + if (!clk->ops->set_rate)
> + return -ENOSYS;
> + return clk->ops->set_rate(clk, rate);
What's the expected behaviour when calling v4l2_clk_get/set_rate on a disabled
clock ?
> +}
> +EXPORT_SYMBOL(v4l2_clk_set_rate);
> +
> +struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops,
> +const char *dev_name,
> +const char *name)
> +{
> + struct v4l2_clk *clk;
> +
> + if (!ops || !ops->owner || (!list_empty(&v4l2_clk) && !name))
> + return ERR_PTR(-EINVAL);
> +
> + clk = kzalloc(sizeof(struct v4l2_cl
Re: [PATCH 1/2] media: V4L2: add temporary clock helpers
Hi Guennadi,
Thanks for the patch. Here are a few comments in addition to what Sylwester
already mentioned.
On Saturday 20 October 2012 00:20:20 Guennadi Liakhovetski wrote:
> Typical video devices like camera sensors require an external clock source.
> Many such devices cannot even access their hardware registers without a
> running clock. These clock sources should be controlled by their consumers.
> This should be performed, using the generic clock framework. Unfortunately
> so far only very few systems have been ported to that framework. This patch
> adds a set of temporary helpers, mimicking the generic clock API, to V4L2.
> Platforms, adopting the clock API, should switch to using it. Eventually
> this temporary API should be removed.
>
> Signed-off-by: Guennadi Liakhovetski
> ---
> drivers/media/v4l2-core/Makefile |2 +-
> drivers/media/v4l2-core/v4l2-clk.c | 126 +
> include/media/v4l2-clk.h | 48 ++
> 3 files changed, 175 insertions(+), 1 deletions(-)
> create mode 100644 drivers/media/v4l2-core/v4l2-clk.c
> create mode 100644 include/media/v4l2-clk.h
>
> diff --git a/drivers/media/v4l2-core/Makefile
> b/drivers/media/v4l2-core/Makefile index 00f64d6..cb5fede 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -5,7 +5,7 @@
> tuner-objs := tuner-core.o
>
> videodev-objs:= v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o
> \
> - v4l2-event.o v4l2-ctrls.o v4l2-subdev.o
> + v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
> ifeq ($(CONFIG_COMPAT),y)
>videodev-objs += v4l2-compat-ioctl32.o
> endif
> diff --git a/drivers/media/v4l2-core/v4l2-clk.c
> b/drivers/media/v4l2-core/v4l2-clk.c new file mode 100644
> index 000..7d457e4
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-clk.c
> @@ -0,0 +1,126 @@
> +/*
> + * V4L2 clock service
> + *
> + * Copyright (C) 2012, Guennadi Liakhovetski
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +static DEFINE_MUTEX(clk_lock);
> +static LIST_HEAD(v4l2_clk);
> +
> +struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id)
> +{
> + struct v4l2_clk *clk = NULL;
> +
> + mutex_lock(&clk_lock);
> + if (!id) {
> + if (list_is_singular(&v4l2_clk)) {
> + clk = list_entry(&v4l2_clk, struct v4l2_clk, list);
> + if (!strstr(sd->name, clk->dev_id))
> + clk = ERR_PTR(-ENODEV);
> + } else {
> + clk = ERR_PTR(-EINVAL);
> + }
> + } else {
> + list_for_each_entry(clk, &v4l2_clk, list) {
> + if (!strcmp(id, clk->id) &&
> + !strcmp(sd->name, clk->dev_id))
> + break;
> + }
> + if (&clk->list == &v4l2_clk)
> + clk = ERR_PTR(-ENODEV);
> + }
> + mutex_unlock(&clk_lock);
> +
> + if (!IS_ERR(clk) &&
> + !try_module_get(clk->ops->owner))
> + clk = ERR_PTR(-ENODEV);
> +
> + return clk;
> +}
> +EXPORT_SYMBOL(v4l2_clk_get);
> +
> +void v4l2_clk_put(struct v4l2_clk *clk)
> +{
> + if (!IS_ERR(clk))
> + module_put(clk->ops->owner);
> +}
> +EXPORT_SYMBOL(v4l2_clk_put);
> +
> +int v4l2_clk_enable(struct v4l2_clk *clk)
> +{
> + if (!clk->ops->enable)
> + return -ENOSYS;
> + return clk->ops->enable(clk);
> +}
> +EXPORT_SYMBOL(v4l2_clk_enable);
> +
> +void v4l2_clk_disable(struct v4l2_clk *clk)
> +{
> + if (clk->ops->disable)
> + clk->ops->disable(clk);
> +}
> +EXPORT_SYMBOL(v4l2_clk_disable);
> +
> +unsigned long v4l2_clk_get_rate(struct v4l2_clk *clk)
> +{
> + if (!clk->ops->get_rate)
> + return -ENOSYS;
> + return clk->ops->get_rate(clk);
> +}
> +EXPORT_SYMBOL(v4l2_clk_get_rate);
> +
> +int v4l2_clk_set_rate(struct v4l2_clk *clk, unsigned long rate)
> +{
> + if (!clk->ops->set_rate)
> + return -ENOSYS;
> + return clk->ops->set_rate(clk, rate);
> +}
> +EXPORT_SYMBOL(v4l2_clk_set_rate);
> +
> +struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops,
> +const char *dev_name,
> +const char *name)
> +{
> + struct v4l2_clk *clk;
> +
> + if (!ops || !ops->owner || (!list_empty(&v4l2_clk) && !name))
> + return ERR_PTR(-EINVAL);
> +
> + clk = kzalloc(sizeof(struct v4l2_clk), GFP_KERNEL);
> + if (!clk)
> + return ERR_PTR(-ENOMEM);
> +
> + clk->ops = ops;
> + clk->id = name;
> + clk->dev_id = dev_name;
What about kstrdup() ing name and d
Re: [PATCH 2/2] media: V4L2: support asynchronous subdevice registration
On Mon, 22 Oct 2012, Hans Verkuil wrote: > On Mon October 22 2012 13:08:12 Guennadi Liakhovetski wrote: > > Hi Hans > > > > Thanks for reviewing the patch. > > > > On Mon, 22 Oct 2012, Hans Verkuil wrote: > > > > > Hi Guennadi, > > > > > > I've reviewed this patch and I have a few questions: > > > > > > On Sat October 20 2012 00:20:24 Guennadi Liakhovetski wrote: > > > > Currently bridge device drivers register devices for all subdevices > > > > synchronously, tupically, during their probing. E.g. if an I2C CMOS > > > > sensor > > > > is attached to a video bridge device, the bridge driver will create an > > > > I2C > > > > device and wait for the respective I2C driver to probe. This makes > > > > linking > > > > of devices straight forward, but this approach cannot be used with > > > > intrinsically asynchronous and unordered device registration systems > > > > like > > > > the Flattened Device Tree. To support such systems this patch adds an > > > > asynchronous subdevice registration framework to V4L2. To use it > > > > respective > > > > (e.g. I2C) subdevice drivers must request deferred probing as long as > > > > their > > > > bridge driver hasn't probed. The bridge driver during its probing > > > > submits a > > > > an arbitrary number of subdevice descriptor groups to the framework to > > > > manage. After that it can add callbacks to each of those groups to be > > > > called at various stages during subdevice probing, e.g. after > > > > completion. > > > > Then the bridge driver can request single groups to be probed, finish > > > > its > > > > own probing and continue its video subsystem configuration from its > > > > callbacks. > > > > > > What is the purpose of allowing multiple groups? > > > > To support, e.g. multiple sensors connected to a single bridge. > > So, isn't that one group with two sensor subdevs? No, one group consists of all subdevices, necessary to operate a single video pipeline. A simple group only contains a sensor. More complex groups can contain a CSI-2 interface, a line shifter, or anything else. > A bridge driver has a list of subdevs. There is no concept of 'groups'. > Perhaps > I misunderstand? Well, we have a group ID, which can be used for what I'm proposing groups for. At least on soc-camera we use the group ID exactly for this purpose. We attach all subdevices to a V4L2 device, but assign group IDs according to pipelines. Then subdevice operations only act on members of one pipeline. I know that we currently don't specify precisely what that group ID should be used for in general. So, this my group concept is an extension of what we currently have in V4L2. > > > I can't think of any reason > > > why you would want to have more than one group. If you have just one group > > > you can simplify this code quite a bit: most of the v4l2_async_group > > > fields > > > can just become part of struct v4l2_device, you don't need the 'list' and > > > 'v4l2_dev' fields anymore and the 'bind' and 'complete' callbacks can be > > > implemented using the v4l2_device notify callback which we already have. > > > > > > > > > > > Signed-off-by: Guennadi Liakhovetski > > > > --- > > > > > > > > One more thing to note about this patch. Subdevice drivers, supporting > > > > asynchronous probing, and using this framework, need a unified way to > > > > detect, whether their probing should succeed or they should request > > > > deferred probing. I implement this using device platform data. This > > > > means, > > > > that all subdevice drivers, wishing to use this API will have to use > > > > the > > > > same platform data struct. I don't think this is a major inconvenience, > > > > but if we decide against this, we'll have to add a V4L2 function to > > > > verify > > > > "are you ready for me or not." The latter would be inconvenient, > > > > because > > > > then we would have to look through all registered subdevice descriptor > > > > groups for this specific subdevice. > > > > > > I have to admit that I don't quite follow this. I guess I would need to > > > see > > > this being used in an actual driver. > > > > The issue is simple: subdevice drivers have to recognise, when it's still > > too early to probe and return -EPROBE_DEFER. If you have a sensor, whose > > master clock is supplied from an external oscillator. You load its driver, > > it will happily get a clock reference and find no reason to fail probe(). > > It will initialise its subdevice and return from probing. Then, when your > > bridge driver probes, it will have no way to find that subdevice. > > This problem is specific to platform subdev drivers, right? Since for i2c > subdevs you can use i2c_get_clientdata(). But how do you get the client? With DT you can follow our "remote" phandle, and without DT? > I wonder whether it isn't a better idea to have platform_data embed a standard > struct containing a v4l2_subdev pointer. Subdevs can use container_of to get > fr
[PATCH RESEND 2/2] media: davinci: vpbe: set device capabilities
From: Lad, Prabhakar set device_caps and also change the driver and bus_info to proper values as per standard. Signed-off-by: Lad, Prabhakar Signed-off-by: Manjunath Hadli --- drivers/media/platform/davinci/vpbe_display.c |9 ++--- 1 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c index 974957f..2bfde79 100644 --- a/drivers/media/platform/davinci/vpbe_display.c +++ b/drivers/media/platform/davinci/vpbe_display.c @@ -702,9 +702,12 @@ static int vpbe_display_querycap(struct file *file, void *priv, struct vpbe_device *vpbe_dev = fh->disp_dev->vpbe_dev; cap->version = VPBE_DISPLAY_VERSION_CODE; - cap->capabilities = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING; - strlcpy(cap->driver, VPBE_DISPLAY_DRIVER, sizeof(cap->driver)); - strlcpy(cap->bus_info, "platform", sizeof(cap->bus_info)); + cap->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING; + cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS; + snprintf(cap->driver, sizeof(cap->driver), "%s", + dev_name(vpbe_dev->pdev)); + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s", +dev_name(vpbe_dev->pdev)); strlcpy(cap->card, vpbe_dev->cfg->module_name, sizeof(cap->card)); return 0; -- 1.7.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND 1/2] media: davinci: vpbe: migrate driver to videobuf2
From: Lad, Prabhakar
This patch migrates VPBE display driver to videobuf2 framework.
Signed-off-by: Lad, Prabhakar
Signed-off-by: Manjunath Hadli
---
drivers/media/platform/davinci/Kconfig|2 +-
drivers/media/platform/davinci/vpbe_display.c | 296 +++--
include/media/davinci/vpbe_display.h | 15 +-
3 files changed, 188 insertions(+), 125 deletions(-)
diff --git a/drivers/media/platform/davinci/Kconfig
b/drivers/media/platform/davinci/Kconfig
index 78e26d2..3c56037 100644
--- a/drivers/media/platform/davinci/Kconfig
+++ b/drivers/media/platform/davinci/Kconfig
@@ -101,7 +101,7 @@ config VIDEO_DM644X_VPBE
tristate "DM644X VPBE HW module"
depends on ARCH_DAVINCI_DM644x
select VIDEO_VPSS_SYSTEM
- select VIDEOBUF_DMA_CONTIG
+ select VIDEOBUF2_DMA_CONTIG
help
Enables VPBE modules used for display on a DM644x
SoC.
diff --git a/drivers/media/platform/davinci/vpbe_display.c
b/drivers/media/platform/davinci/vpbe_display.c
index 161c776..974957f 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -47,6 +47,9 @@ static int debug;
module_param(debug, int, 0644);
+static int vpbe_set_osd_display_params(struct vpbe_display *disp_dev,
+ struct vpbe_layer *layer);
+
static int venc_is_second_field(struct vpbe_display *disp_dev)
{
struct vpbe_device *vpbe_dev = disp_dev->vpbe_dev;
@@ -73,10 +76,11 @@ static void vpbe_isr_even_field(struct vpbe_display
*disp_obj,
if (layer->cur_frm == layer->next_frm)
return;
ktime_get_ts(&timevalue);
- layer->cur_frm->ts.tv_sec = timevalue.tv_sec;
- layer->cur_frm->ts.tv_usec = timevalue.tv_nsec / NSEC_PER_USEC;
- layer->cur_frm->state = VIDEOBUF_DONE;
- wake_up_interruptible(&layer->cur_frm->done);
+ layer->cur_frm->vb.v4l2_buf.timestamp.tv_sec =
+ timevalue.tv_sec;
+ layer->cur_frm->vb.v4l2_buf.timestamp.tv_usec =
+ timevalue.tv_nsec / NSEC_PER_USEC;
+ vb2_buffer_done(&layer->cur_frm->vb, VB2_BUF_STATE_DONE);
/* Make cur_frm pointing to next_frm */
layer->cur_frm = layer->next_frm;
}
@@ -99,16 +103,14 @@ static void vpbe_isr_odd_field(struct vpbe_display
*disp_obj,
* otherwise hold on current frame
* Get next from the buffer queue
*/
- layer->next_frm = list_entry(
- layer->dma_queue.next,
- struct videobuf_buffer,
- queue);
+ layer->next_frm = list_entry(layer->dma_queue.next,
+ struct vpbe_disp_buffer, list);
/* Remove that from the buffer queue */
- list_del(&layer->next_frm->queue);
+ list_del(&layer->next_frm->list);
spin_unlock(&disp_obj->dma_queue_lock);
/* Mark state of the frame to active */
- layer->next_frm->state = VIDEOBUF_ACTIVE;
- addr = videobuf_to_dma_contig(layer->next_frm);
+ layer->next_frm->vb.state = VB2_BUF_STATE_ACTIVE;
+ addr = vb2_dma_contig_plane_dma_addr(&layer->next_frm->vb, 0);
osd_device->ops.start_layer(osd_device,
layer->layer_info.id,
addr,
@@ -199,39 +201,29 @@ static irqreturn_t venc_isr(int irq, void *arg)
/*
* vpbe_buffer_prepare()
- * This is the callback function called from videobuf_qbuf() function
+ * This is the callback function called from vb2_qbuf() function
* the buffer is prepared and user space virtual address is converted into
* physical address
*/
-static int vpbe_buffer_prepare(struct videobuf_queue *q,
- struct videobuf_buffer *vb,
- enum v4l2_field field)
+static int vpbe_buffer_prepare(struct vb2_buffer *vb)
{
- struct vpbe_fh *fh = q->priv_data;
+ struct vpbe_fh *fh = vb2_get_drv_priv(vb->vb2_queue);
+ struct vb2_queue *q = vb->vb2_queue;
struct vpbe_layer *layer = fh->layer;
struct vpbe_device *vpbe_dev = fh->disp_dev->vpbe_dev;
unsigned long addr;
- int ret;
v4l2_dbg(1, debug, &vpbe_dev->v4l2_dev,
"vpbe_buffer_prepare\n");
- /* If buffer is not initialized, initialize it */
- if (VIDEOBUF_NEEDS_INIT == vb->state) {
- vb->width = layer->pix_fmt.width;
- vb->height = layer->pix_fmt.height;
- vb->size = layer->pix_fmt.sizeimage;
- vb->field = field;
-
- ret = videobuf_iolock(q, vb, NULL);
- if (ret < 0) {
- v4l2_err(&vpbe_dev->v4l2_dev, "Failed to map \
- user address\n");
+ if (vb->state != VB2_BUF_STATE_ACTIVE &&
+ vb->state != VB2_BUF_STATE_PREPARED) {
+ vb2_set_plane_payload(vb, 0, layer
[PATCH RESEND 0/2] Davinci VPBE migration to vb2 and setting the device caps
From: Lad, Prabhakar The first patch of the series migrates the VPBE driver to usage of videobuf2 framework. Second patch sets the device caps. Resending the series, since it didn't reach the DLOS mailing list. Lad, Prabhakar (2): media: davinci: vpbe: migrate driver to videobuf2 media: davinci: vpbe: set device capabilities drivers/media/platform/davinci/Kconfig|2 +- drivers/media/platform/davinci/vpbe_display.c | 305 +++-- include/media/davinci/vpbe_display.h | 15 +- 3 files changed, 194 insertions(+), 128 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND] media: davinci: vpbe: fix build warning
Hi Prabhakar,
On Monday 22 October 2012 17:47:51 Prabhakar Lad wrote:
> From: Lad, Prabhakar
>
> Warnings were generated because of the following commit changed data type
> for address pointer
>
> 195bbca ARM: 7500/1: io: avoid writeback addressing modes for __raw_
> accessors add __iomem annotation to fix following warnings
>
> drivers/media/platform/davinci/vpbe_osd.c: In function ‘osd_read’:
> drivers/media/platform/davinci/vpbe_osd.c:49:2: warning: passing
> argument 1 of ‘__raw_readl’ makes pointer from integer without a cast
> [enabled by default] arch/arm/include/asm/io.h:104:19: note: expected
> ‘const volatile
> void *’ but argument is of type ‘long unsigned int’
>
> Signed-off-by: Lad, Prabhakar
> Signed-off-by: Manjunath Hadli
> ---
> Resending the patch since, it didn't reach the DLOS mailing list.
>
> drivers/media/platform/davinci/vpbe_osd.c | 16
> 1 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpbe_osd.c
> b/drivers/media/platform/davinci/vpbe_osd.c index bba299d..9ab9280 100644
> --- a/drivers/media/platform/davinci/vpbe_osd.c
> +++ b/drivers/media/platform/davinci/vpbe_osd.c
> @@ -46,14 +46,14 @@ static inline u32 osd_read(struct osd_state *sd, u32
> offset) {
> struct osd_state *osd = sd;
>
> - return readl(osd->osd_base + offset);
> + return readl(IOMEM(osd->osd_base + offset));
A better fix, in my opinion, would be to change the osd->osd_base field to be
a void __iomem * instead of long unsigned int.
> }
>
> static inline u32 osd_write(struct osd_state *sd, u32 val, u32 offset)
> {
> struct osd_state *osd = sd;
>
> - writel(val, osd->osd_base + offset);
> + writel(val, IOMEM(osd->osd_base + offset));
>
> return val;
> }
> @@ -63,9 +63,9 @@ static inline u32 osd_set(struct osd_state *sd, u32 mask,
> u32 offset) struct osd_state *osd = sd;
>
> u32 addr = osd->osd_base + offset;
> - u32 val = readl(addr) | mask;
> + u32 val = readl(IOMEM(addr)) | mask;
>
> - writel(val, addr);
> + writel(val, IOMEM(addr));
>
> return val;
> }
> @@ -75,9 +75,9 @@ static inline u32 osd_clear(struct osd_state *sd, u32
> mask, u32 offset) struct osd_state *osd = sd;
>
> u32 addr = osd->osd_base + offset;
> - u32 val = readl(addr) & ~mask;
> + u32 val = readl(IOMEM(addr)) & ~mask;
>
> - writel(val, addr);
> + writel(val, IOMEM(addr));
>
> return val;
> }
> @@ -88,9 +88,9 @@ static inline u32 osd_modify(struct osd_state *sd, u32
> mask, u32 val, struct osd_state *osd = sd;
>
> u32 addr = osd->osd_base + offset;
> - u32 new_val = (readl(addr) & ~mask) | (val & mask);
> + u32 new_val = (readl(IOMEM(addr)) & ~mask) | (val & mask);
>
> - writel(new_val, addr);
> + writel(new_val, IOMEM(addr));
>
> return new_val;
> }
--
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
[PATCH RESEND] media: davinci: vpbe: fix build warning
From: Lad, Prabhakar
Warnings were generated because of the following commit changed data type for
address pointer
195bbca ARM: 7500/1: io: avoid writeback addressing modes for __raw_ accessors
add __iomem annotation to fix following warnings
drivers/media/platform/davinci/vpbe_osd.c: In function ‘osd_read’:
drivers/media/platform/davinci/vpbe_osd.c:49:2: warning: passing
argument 1 of ‘__raw_readl’ makes pointer from integer without a cast [enabled
by default]
arch/arm/include/asm/io.h:104:19: note: expected ‘const volatile
void *’ but argument is of type ‘long unsigned int’
Signed-off-by: Lad, Prabhakar
Signed-off-by: Manjunath Hadli
---
Resending the patch since, it didn't reach the DLOS mailing list.
drivers/media/platform/davinci/vpbe_osd.c | 16
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/davinci/vpbe_osd.c
b/drivers/media/platform/davinci/vpbe_osd.c
index bba299d..9ab9280 100644
--- a/drivers/media/platform/davinci/vpbe_osd.c
+++ b/drivers/media/platform/davinci/vpbe_osd.c
@@ -46,14 +46,14 @@ static inline u32 osd_read(struct osd_state *sd, u32 offset)
{
struct osd_state *osd = sd;
- return readl(osd->osd_base + offset);
+ return readl(IOMEM(osd->osd_base + offset));
}
static inline u32 osd_write(struct osd_state *sd, u32 val, u32 offset)
{
struct osd_state *osd = sd;
- writel(val, osd->osd_base + offset);
+ writel(val, IOMEM(osd->osd_base + offset));
return val;
}
@@ -63,9 +63,9 @@ static inline u32 osd_set(struct osd_state *sd, u32 mask, u32
offset)
struct osd_state *osd = sd;
u32 addr = osd->osd_base + offset;
- u32 val = readl(addr) | mask;
+ u32 val = readl(IOMEM(addr)) | mask;
- writel(val, addr);
+ writel(val, IOMEM(addr));
return val;
}
@@ -75,9 +75,9 @@ static inline u32 osd_clear(struct osd_state *sd, u32 mask,
u32 offset)
struct osd_state *osd = sd;
u32 addr = osd->osd_base + offset;
- u32 val = readl(addr) & ~mask;
+ u32 val = readl(IOMEM(addr)) & ~mask;
- writel(val, addr);
+ writel(val, IOMEM(addr));
return val;
}
@@ -88,9 +88,9 @@ static inline u32 osd_modify(struct osd_state *sd, u32 mask,
u32 val,
struct osd_state *osd = sd;
u32 addr = osd->osd_base + offset;
- u32 new_val = (readl(addr) & ~mask) | (val & mask);
+ u32 new_val = (readl(IOMEM(addr)) & ~mask) | (val & mask);
- writel(new_val, addr);
+ writel(new_val, IOMEM(addr));
return new_val;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3isp: Replace printk with dev_*
On Mon, Oct 22, 2012 at 01:44:50PM +0200, Laurent Pinchart wrote: > Use the dev_* message logging API instead of raw printk. > > Signed-off-by: Laurent Pinchart > --- > drivers/media/platform/omap3isp/isp.c | 12 ++-- > drivers/media/platform/omap3isp/ispcsi2.c |6 +++--- > drivers/media/platform/omap3isp/ispcsiphy.c |2 +- > drivers/media/platform/omap3isp/ispvideo.c |3 ++- > 4 files changed, 12 insertions(+), 11 deletions(-) Exactly what I was about to say. Acked-by: Sakari Ailus -- 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
[PATCH RESEND] ARM: dm365: replace V4L2_OUT_CAP_CUSTOM_TIMINGS with V4L2_OUT_CAP_DV_TIMINGS
From: Lad, Prabhakar
This patch replaces V4L2_OUT_CAP_CUSTOM_TIMINGS macro with
V4L2_OUT_CAP_DV_TIMINGS. As V4L2_OUT_CAP_CUSTOM_TIMINGS is being phased
out.
Signed-off-by: Lad, Prabhakar
Signed-off-by: Manjunath Hadli
Cc: Sekhar Nori
---
Resending the patch since, it didn't reach the DLOS mailing list.
This patch is based on the following patch series,
ARM: davinci: dm365 EVM: add support for VPBE display
(https://patchwork.kernel.org/patch/1295071/)
arch/arm/mach-davinci/board-dm365-evm.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm365-evm.c
b/arch/arm/mach-davinci/board-dm365-evm.c
index 2924d61..771abb5 100644
--- a/arch/arm/mach-davinci/board-dm365-evm.c
+++ b/arch/arm/mach-davinci/board-dm365-evm.c
@@ -514,7 +514,7 @@ static struct vpbe_output dm365evm_vpbe_outputs[] = {
.index = 1,
.name = "Component",
.type = V4L2_OUTPUT_TYPE_ANALOG,
- .capabilities = V4L2_OUT_CAP_CUSTOM_TIMINGS,
+ .capabilities = V4L2_OUT_CAP_DV_TIMINGS,
},
.subdev_name= VPBE_VENC_SUBDEV_NAME,
.default_mode = "480p59_94",
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [media-workshop] Tentative Agenda for the November workshop
On Mon October 22 2012 13:18:46 Laurent Pinchart wrote: > Hi Sylwester, > > On Monday 22 October 2012 12:53:02 Sylwester Nawrocki wrote: > > On 10/22/2012 12:39 PM, Laurent Pinchart wrote: > > > On Monday 22 October 2012 10:35:56 Hans Verkuil wrote: > > >> Hi all, > > >> > > >> This is the tentative agenda for the media workshop on November 8, 2012. > > >> If you have additional things that you want to discuss, or something is > > >> wrong or incomplete in this list, please let me know so I can update the > > >> list. > > > > > > Thank you Hans for taking care of the agenda. > > > > > >> - Explain current merging process (Mauro) > > >> - Open floor for discussions on how to improve it (Mauro) > > >> - Write down minimum requirements for new V4L2 (and DVB?) drivers, both > > >> for > > >> > > >> staging and mainline acceptance: which frameworks to use, > > >> v4l2-compliance, > > >> > > >> etc. (Hans Verkuil) > > >> - V4L2 ambiguities (Hans Verkuil) > > >> - TSMux device (a mux rather than a demux): Alain Volmat > > >> - dmabuf status, esp. with regards to being able to test (Mauro/Samsung) > > >> - Device tree support (Guennadi, not known yet whether this topic is > > >> needed) - Creating/selecting contexts for hardware that supports this > > >> (Samsung, only if time is available) > > > > > > This last topic will likely require lots of brainstorming, and thus time. > > > If the schedule permits, would anyone be interested in meeting earlier > > > during the week already ? > > > > My intention was to also possibly discuss it with others before the actual > > media workshop. Would be nice if we could have arranged such a meeting. > > I'm not sure about the room conditions though. It's probably not a big > > issue, unless there is really many people interested in that topic. > > A small room with a projector would be nice if possible, although not > required. Who would be interested in attending a brainstorming session on > contexts ? I would be, but the problem is that the conference is also interesting. The only day I have really available is the Friday *after* the summit. 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 2/2] media: V4L2: support asynchronous subdevice registration
On Mon October 22 2012 13:08:12 Guennadi Liakhovetski wrote: > Hi Hans > > Thanks for reviewing the patch. > > On Mon, 22 Oct 2012, Hans Verkuil wrote: > > > Hi Guennadi, > > > > I've reviewed this patch and I have a few questions: > > > > On Sat October 20 2012 00:20:24 Guennadi Liakhovetski wrote: > > > Currently bridge device drivers register devices for all subdevices > > > synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor > > > is attached to a video bridge device, the bridge driver will create an I2C > > > device and wait for the respective I2C driver to probe. This makes linking > > > of devices straight forward, but this approach cannot be used with > > > intrinsically asynchronous and unordered device registration systems like > > > the Flattened Device Tree. To support such systems this patch adds an > > > asynchronous subdevice registration framework to V4L2. To use it > > > respective > > > (e.g. I2C) subdevice drivers must request deferred probing as long as > > > their > > > bridge driver hasn't probed. The bridge driver during its probing submits > > > a > > > an arbitrary number of subdevice descriptor groups to the framework to > > > manage. After that it can add callbacks to each of those groups to be > > > called at various stages during subdevice probing, e.g. after completion. > > > Then the bridge driver can request single groups to be probed, finish its > > > own probing and continue its video subsystem configuration from its > > > callbacks. > > > > What is the purpose of allowing multiple groups? > > To support, e.g. multiple sensors connected to a single bridge. So, isn't that one group with two sensor subdevs? A bridge driver has a list of subdevs. There is no concept of 'groups'. Perhaps I misunderstand? > > I can't think of any reason > > why you would want to have more than one group. If you have just one group > > you can simplify this code quite a bit: most of the v4l2_async_group fields > > can just become part of struct v4l2_device, you don't need the 'list' and > > 'v4l2_dev' fields anymore and the 'bind' and 'complete' callbacks can be > > implemented using the v4l2_device notify callback which we already have. > > > > > > > > Signed-off-by: Guennadi Liakhovetski > > > --- > > > > > > One more thing to note about this patch. Subdevice drivers, supporting > > > asynchronous probing, and using this framework, need a unified way to > > > detect, whether their probing should succeed or they should request > > > deferred probing. I implement this using device platform data. This > > > means, > > > that all subdevice drivers, wishing to use this API will have to use the > > > same platform data struct. I don't think this is a major inconvenience, > > > but if we decide against this, we'll have to add a V4L2 function to > > > verify > > > "are you ready for me or not." The latter would be inconvenient, because > > > then we would have to look through all registered subdevice descriptor > > > groups for this specific subdevice. > > > > I have to admit that I don't quite follow this. I guess I would need to see > > this being used in an actual driver. > > The issue is simple: subdevice drivers have to recognise, when it's still > too early to probe and return -EPROBE_DEFER. If you have a sensor, whose > master clock is supplied from an external oscillator. You load its driver, > it will happily get a clock reference and find no reason to fail probe(). > It will initialise its subdevice and return from probing. Then, when your > bridge driver probes, it will have no way to find that subdevice. This problem is specific to platform subdev drivers, right? Since for i2c subdevs you can use i2c_get_clientdata(). I wonder whether it isn't a better idea to have platform_data embed a standard struct containing a v4l2_subdev pointer. Subdevs can use container_of to get from the address of that struct to the full platform_data, and you don't have that extra dereference (i.e. a pointer to the new struct which has a pointer to the actual platform_data). The impact of that change is much smaller for existing subdevs. And if it isn't necessary for i2c subdev drivers, then I think we should do this only for platform drivers. > > > > drivers/media/v4l2-core/Makefile |3 +- > > > drivers/media/v4l2-core/v4l2-async.c | 249 > > > + > > > drivers/media/v4l2-core/v4l2-device.c |2 + > > > include/media/v4l2-async.h| 88 > > > include/media/v4l2-device.h |6 + > > > include/media/v4l2-subdev.h | 16 ++ > > > 6 files changed, 363 insertions(+), 1 deletions(-) > > > create mode 100644 drivers/media/v4l2-core/v4l2-async.c > > > create mode 100644 include/media/v4l2-async.h > > > > > > diff --git a/drivers/media/v4l2-core/Makefile > > > b/drivers/media/v4l2-core/Makefile > > > index cb5fede..074e01c 100644 > > > --- a/drivers/media/v4l2-
[PATCH] saa7134: Add pm_qos_request to fix video corruption
The SAA7134 appears to have trouble buffering more than one line of video
when doing DMA. Rather than try to fix the driver to cope (as has been done
by Andy Walls for the cx18 driver), put in a pm_qos_request to limit deep
sleep exit latencies.
The visible effect of not having this is that seemingly random lines are
only partly transferred - if you feed in a static image, you see a portion
of the image "flicker" into place.
Signed-off-by: Simon Farnsworth
---
As per the comment, note that I've not been able to nail down the maximum
latency the SAA7134 can cope with. I know that the chip has a 1KiB FIFO
buffer, so I'm assuming that it can store half a line of video at a time, on
the basis of 720 luma, 360 Cb, 360 Cr samples, totalling 1440 bytes per
line. If this is a bad assumption (I've not been able to find register-level
documentation for the chip, so I don't know what
saa_writel(SAA7134_FIFO_SIZE, 0x08070503) does in saa7134_hw_enable1() in
saa7134-core.c), that value will need adjusting to match the real FIFO
latency.
drivers/media/pci/saa7134/saa7134-video.c | 12
drivers/media/pci/saa7134/saa7134.h | 2 ++
2 files changed, 14 insertions(+)
diff --git a/drivers/media/pci/saa7134/saa7134-video.c
b/drivers/media/pci/saa7134/saa7134-video.c
index 4a77124..dbc0b5d 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -2248,6 +2248,15 @@ static int saa7134_streamon(struct file *file, void
*priv,
if (!res_get(dev, fh, res))
return -EBUSY;
+ /* The SAA7134 has a 1K FIFO; the assumption here is that that's
+* enough for half a line of video in the configuration Linux uses.
+* If it isn't, reduce the 31 usec down to the maximum FIFO time
+* allowance.
+*/
+ pm_qos_add_request(&fh->qos_request,
+ PM_QOS_CPU_DMA_LATENCY,
+ 31);
+
return videobuf_streamon(saa7134_queue(fh));
}
@@ -2259,6 +2269,8 @@ static int saa7134_streamoff(struct file *file, void
*priv,
struct saa7134_dev *dev = fh->dev;
int res = saa7134_resource(fh);
+ pm_qos_remove_request(&fh->qos_request);
+
err = videobuf_streamoff(saa7134_queue(fh));
if (err < 0)
return err;
diff --git a/drivers/media/pci/saa7134/saa7134.h
b/drivers/media/pci/saa7134/saa7134.h
index c24b651..d09393b 100644
--- a/drivers/media/pci/saa7134/saa7134.h
+++ b/drivers/media/pci/saa7134/saa7134.h
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
@@ -469,6 +470,7 @@ struct saa7134_fh {
enum v4l2_buf_type type;
unsigned int resources;
enum v4l2_priority prio;
+ struct pm_qos_request_list qos_request;
/* video overlay */
struct v4l2_window win;
--
1.7.11.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] omap3isp: Replace printk with dev_*
Use the dev_* message logging API instead of raw printk.
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/omap3isp/isp.c | 12 ++--
drivers/media/platform/omap3isp/ispcsi2.c |6 +++---
drivers/media/platform/omap3isp/ispcsiphy.c |2 +-
drivers/media/platform/omap3isp/ispvideo.c |3 ++-
4 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index cb9bc34..e8e724e 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -1679,7 +1679,7 @@ isp_register_subdev_group(struct isp_device *isp,
adapter = i2c_get_adapter(board_info->i2c_adapter_id);
if (adapter == NULL) {
- printk(KERN_ERR "%s: Unable to get I2C adapter %d for "
+ dev_err(isp->dev, "%s: Unable to get I2C adapter %d for
"
"device %s\n", __func__,
board_info->i2c_adapter_id,
board_info->board_info->type);
@@ -1689,7 +1689,7 @@ isp_register_subdev_group(struct isp_device *isp,
subdev = v4l2_i2c_new_subdev_board(&isp->v4l2_dev, adapter,
board_info->board_info, NULL);
if (subdev == NULL) {
- printk(KERN_ERR "%s: Unable to register subdev %s\n",
+ dev_err(isp->dev, "%s: Unable to register subdev %s\n",
__func__, board_info->board_info->type);
continue;
}
@@ -1714,7 +1714,7 @@ static int isp_register_entities(struct isp_device *isp)
isp->media_dev.link_notify = isp_pipeline_link_notify;
ret = media_device_register(&isp->media_dev);
if (ret < 0) {
- printk(KERN_ERR "%s: Media device registration failed (%d)\n",
+ dev_err(isp->dev, "%s: Media device registration failed (%d)\n",
__func__, ret);
return ret;
}
@@ -1722,7 +1722,7 @@ static int isp_register_entities(struct isp_device *isp)
isp->v4l2_dev.mdev = &isp->media_dev;
ret = v4l2_device_register(isp->dev, &isp->v4l2_dev);
if (ret < 0) {
- printk(KERN_ERR "%s: V4L2 device registration failed (%d)\n",
+ dev_err(isp->dev, "%s: V4L2 device registration failed (%d)\n",
__func__, ret);
goto done;
}
@@ -1809,8 +1809,8 @@ static int isp_register_entities(struct isp_device *isp)
break;
default:
- printk(KERN_ERR "%s: invalid interface type %u\n",
- __func__, subdevs->interface);
+ dev_err(isp->dev, "%s: invalid interface type %u\n",
+ __func__, subdevs->interface);
ret = -EINVAL;
goto done;
}
diff --git a/drivers/media/platform/omap3isp/ispcsi2.c
b/drivers/media/platform/omap3isp/ispcsi2.c
index 6a3ff79..783f4b0 100644
--- a/drivers/media/platform/omap3isp/ispcsi2.c
+++ b/drivers/media/platform/omap3isp/ispcsi2.c
@@ -517,7 +517,7 @@ int omap3isp_csi2_reset(struct isp_csi2_device *csi2)
} while (soft_reset_retries < 5);
if (soft_reset_retries == 5) {
- printk(KERN_ERR "CSI2: Soft reset try count exceeded!\n");
+ dev_err(isp->dev, "CSI2: Soft reset try count exceeded!\n");
return -EBUSY;
}
@@ -535,8 +535,8 @@ int omap3isp_csi2_reset(struct isp_csi2_device *csi2)
} while (--i > 0);
if (i == 0) {
- printk(KERN_ERR
- "CSI2: Reset for CSI2_96M_FCLK domain Failed!\n");
+ dev_err(isp->dev,
+ "CSI2: Reset for CSI2_96M_FCLK domain Failed!\n");
return -EBUSY;
}
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
b/drivers/media/platform/omap3isp/ispcsiphy.c
index d6eb4f9..3d56b33 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -157,7 +157,7 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32
power)
} while ((reg != power >> 2) && (retry_count < 100));
if (retry_count == 100) {
- printk(KERN_ERR "CSI2 CIO set power failed!\n");
+ dev_err(phy->isp->dev, "CSI2 CIO set power failed!\n");
return -EBUSY;
}
diff --git a/drivers/media/platform/omap3isp/ispvideo.c
b/drivers/media/platform/omap3isp/ispvideo.c
index 75cd309..8759247 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -1391,7 +1391,8 @@ int omap3isp_video_register(struct isp_video *video,
struct v4l2_device *vdev)
ret = video_register_device(&video->vide
Re: [media-workshop] Tentative Agenda for the November workshop
Hi Sylwester, On Monday 22 October 2012 12:53:02 Sylwester Nawrocki wrote: > On 10/22/2012 12:39 PM, Laurent Pinchart wrote: > > On Monday 22 October 2012 10:35:56 Hans Verkuil wrote: > >> Hi all, > >> > >> This is the tentative agenda for the media workshop on November 8, 2012. > >> If you have additional things that you want to discuss, or something is > >> wrong or incomplete in this list, please let me know so I can update the > >> list. > > > > Thank you Hans for taking care of the agenda. > > > >> - Explain current merging process (Mauro) > >> - Open floor for discussions on how to improve it (Mauro) > >> - Write down minimum requirements for new V4L2 (and DVB?) drivers, both > >> for > >> > >> staging and mainline acceptance: which frameworks to use, > >> v4l2-compliance, > >> > >> etc. (Hans Verkuil) > >> - V4L2 ambiguities (Hans Verkuil) > >> - TSMux device (a mux rather than a demux): Alain Volmat > >> - dmabuf status, esp. with regards to being able to test (Mauro/Samsung) > >> - Device tree support (Guennadi, not known yet whether this topic is > >> needed) - Creating/selecting contexts for hardware that supports this > >> (Samsung, only if time is available) > > > > This last topic will likely require lots of brainstorming, and thus time. > > If the schedule permits, would anyone be interested in meeting earlier > > during the week already ? > > My intention was to also possibly discuss it with others before the actual > media workshop. Would be nice if we could have arranged such a meeting. > I'm not sure about the room conditions though. It's probably not a big > issue, unless there is really many people interested in that topic. A small room with a projector would be nice if possible, although not required. Who would be interested in attending a brainstorming session on contexts ? -- 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: [media-workshop] Tentative Agenda for the November workshop
Hi, On 10/22/2012 12:57 PM, Alain VOLMAT wrote: > Hi all, > > Could someone summaries very rapidly what is this create/select context stuff > ? > For now I do not plan to be in Barcelona more than 1 day but at the same time > don't want to miss something that might be useful for us. Please see this thread for more details: http://www.mail-archive.com/linux-media@vger.kernel.org/msg52168.html This is about creating, selecting device configuration contexts and handling e.g. camera modes like viewfinder and still capture. Currently only mem-to-mem devices in V4L2 API are allowed to support multiple device contexts, and those are per file handle. I hope this clarifies it a bit for you. -- 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 2/2] media: V4L2: support asynchronous subdevice registration
Hi Hans Thanks for reviewing the patch. On Mon, 22 Oct 2012, Hans Verkuil wrote: > Hi Guennadi, > > I've reviewed this patch and I have a few questions: > > On Sat October 20 2012 00:20:24 Guennadi Liakhovetski wrote: > > Currently bridge device drivers register devices for all subdevices > > synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor > > is attached to a video bridge device, the bridge driver will create an I2C > > device and wait for the respective I2C driver to probe. This makes linking > > of devices straight forward, but this approach cannot be used with > > intrinsically asynchronous and unordered device registration systems like > > the Flattened Device Tree. To support such systems this patch adds an > > asynchronous subdevice registration framework to V4L2. To use it respective > > (e.g. I2C) subdevice drivers must request deferred probing as long as their > > bridge driver hasn't probed. The bridge driver during its probing submits a > > an arbitrary number of subdevice descriptor groups to the framework to > > manage. After that it can add callbacks to each of those groups to be > > called at various stages during subdevice probing, e.g. after completion. > > Then the bridge driver can request single groups to be probed, finish its > > own probing and continue its video subsystem configuration from its > > callbacks. > > What is the purpose of allowing multiple groups? To support, e.g. multiple sensors connected to a single bridge. > I can't think of any reason > why you would want to have more than one group. If you have just one group > you can simplify this code quite a bit: most of the v4l2_async_group fields > can just become part of struct v4l2_device, you don't need the 'list' and > 'v4l2_dev' fields anymore and the 'bind' and 'complete' callbacks can be > implemented using the v4l2_device notify callback which we already have. > > > > > Signed-off-by: Guennadi Liakhovetski > > --- > > > > One more thing to note about this patch. Subdevice drivers, supporting > > asynchronous probing, and using this framework, need a unified way to > > detect, whether their probing should succeed or they should request > > deferred probing. I implement this using device platform data. This means, > > that all subdevice drivers, wishing to use this API will have to use the > > same platform data struct. I don't think this is a major inconvenience, > > but if we decide against this, we'll have to add a V4L2 function to verify > > "are you ready for me or not." The latter would be inconvenient, because > > then we would have to look through all registered subdevice descriptor > > groups for this specific subdevice. > > I have to admit that I don't quite follow this. I guess I would need to see > this being used in an actual driver. The issue is simple: subdevice drivers have to recognise, when it's still too early to probe and return -EPROBE_DEFER. If you have a sensor, whose master clock is supplied from an external oscillator. You load its driver, it will happily get a clock reference and find no reason to fail probe(). It will initialise its subdevice and return from probing. Then, when your bridge driver probes, it will have no way to find that subdevice. > > drivers/media/v4l2-core/Makefile |3 +- > > drivers/media/v4l2-core/v4l2-async.c | 249 > > + > > drivers/media/v4l2-core/v4l2-device.c |2 + > > include/media/v4l2-async.h| 88 > > include/media/v4l2-device.h |6 + > > include/media/v4l2-subdev.h | 16 ++ > > 6 files changed, 363 insertions(+), 1 deletions(-) > > create mode 100644 drivers/media/v4l2-core/v4l2-async.c > > create mode 100644 include/media/v4l2-async.h > > > > diff --git a/drivers/media/v4l2-core/Makefile > > b/drivers/media/v4l2-core/Makefile > > index cb5fede..074e01c 100644 > > --- a/drivers/media/v4l2-core/Makefile > > +++ b/drivers/media/v4l2-core/Makefile > > @@ -5,7 +5,8 @@ > > tuner-objs := tuner-core.o > > > > videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o > > \ > > - v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o > > + v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \ > > + v4l2-async.o > > ifeq ($(CONFIG_COMPAT),y) > >videodev-objs += v4l2-compat-ioctl32.o > > endif > > diff --git a/drivers/media/v4l2-core/v4l2-async.c > > b/drivers/media/v4l2-core/v4l2-async.c > > new file mode 100644 > > index 000..871f116 > > --- /dev/null > > +++ b/drivers/media/v4l2-core/v4l2-async.c > > @@ -0,0 +1,249 @@ > > +/* > > + * V4L2 asynchronous subdevice registration API > > + * > > + * Copyright (C) 2012, Guennadi Liakhovetski > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Soft
RE: [media-workshop] Tentative Agenda for the November workshop
Hi all, Could someone summaries very rapidly what is this create/select context stuff ? For now I do not plan to be in Barcelona more than 1 day but at the same time don't want to miss something that might be useful for us. Regards, Alain > -Original Message- > From: media-workshop-boun...@linuxtv.org [mailto:media-workshop- > boun...@linuxtv.org] On Behalf Of Sylwester Nawrocki > Sent: Monday, October 22, 2012 12:53 PM > To: Laurent Pinchart > Cc: media-works...@linuxtv.org; linux-media > Subject: Re: [media-workshop] Tentative Agenda for the November > workshop > > Hi Laurent, > > On 10/22/2012 12:39 PM, Laurent Pinchart wrote: > > Hello, > > > > On Monday 22 October 2012 10:35:56 Hans Verkuil wrote: > >> Hi all, > >> > >> This is the tentative agenda for the media workshop on November 8, > 2012. > >> If you have additional things that you want to discuss, or something > >> is wrong or incomplete in this list, please let me know so I can > >> update the list. > > > > Thank you Hans for taking care of the agenda. > > > >> - Explain current merging process (Mauro) > >> - Open floor for discussions on how to improve it (Mauro) > >> - Write down minimum requirements for new V4L2 (and DVB?) drivers, > both for > >> staging and mainline acceptance: which frameworks to use, > >> v4l2-compliance, etc. (Hans Verkuil) > >> - V4L2 ambiguities (Hans Verkuil) > >> - TSMux device (a mux rather than a demux): Alain Volmat > >> - dmabuf status, esp. with regards to being able to test > >> (Mauro/Samsung) > >> - Device tree support (Guennadi, not known yet whether this topic is > >> needed) > >> - Creating/selecting contexts for hardware that supports this > >> (Samsung, only if time is available) > > > > This last topic will likely require lots of brainstorming, and thus > > time. If the schedule permits, would anyone be interested in meeting > > earlier during the week already ? > > My intention was to also possibly discuss it with others before the actual > media workshop. Would be nice if we could have arranged such a meeting. > I'm not sure about the room conditions though. It's probably not a big issue, > unless there is really many people interested in that topic. > > -- > Regards, > Sylwester > > > ___ > media-workshop mailing list > media-works...@linuxtv.org > http://www.linuxtv.org/cgi-bin/mailman/listinfo/media-workshop -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] omap3isp: Find source pad from external entity
Hi Sakari,
Thanks for the patch.
On Sunday 21 October 2012 00:48:18 Sakari Ailus wrote:
> No longer assume pad number 0 is the source pad of the external entity. Find
> the source pad from the external entity and use it instead.
>
> Signed-off-by: Sakari Ailus
> Acked-by: Laurent Pinchart
> ---
> drivers/media/platform/omap3isp/isp.c | 14 +-
> 1 files changed, 13 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 5ea5520..5f75798 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -1767,6 +1767,7 @@ static int isp_register_entities(struct isp_device
> *isp) struct media_entity *input;
> unsigned int flags;
> unsigned int pad;
> + unsigned int i;
>
> sensor = isp_register_subdev_group(isp, subdevs->subdevs);
> if (sensor == NULL)
> @@ -1814,7 +1815,18 @@ static int isp_register_entities(struct isp_device
> *isp) goto done;
> }
>
> - ret = media_entity_create_link(&sensor->entity, 0, input, pad,
> + for (i = 0; i < sensor->entity.num_pads; i++) {
> + if (sensor->entity.pads[i].flags & MEDIA_PAD_FL_SOURCE)
> + break;
> + }
> + if (i == sensor->entity.num_pads) {
> + dev_err(isp->dev,
> + "no source pads in external entities\n");
Nitpicking, "no source pad in external entity". If that's fine with you I'll
modify this when applying this patch to my tree.
> + ret = -EINVAL;
> + goto done;
> + }
> +
> + ret = media_entity_create_link(&sensor->entity, i, input, pad,
> flags);
> if (ret < 0)
> goto done;
--
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 1/2] omap3isp: Add resizer data rate configuration to resizer_link_validate
Hi Sakari,
Thanks for the patch.
On Sunday 21 October 2012 00:48:17 Sakari Ailus wrote:
> The configuration of many other blocks depend on resizer maximum data rate.
> Get the value from resizer at link validation time.
>
> Signed-off-by: Sakari Ailus
> Acked-by: Laurent Pinchart
> ---
> drivers/media/platform/omap3isp/ispresizer.c | 15 +++
> drivers/media/platform/omap3isp/ispvideo.c | 54 ---
> 2 files changed, 15 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/ispresizer.c
> b/drivers/media/platform/omap3isp/ispresizer.c index d11fb26..bb5fb4a
> 100644
> --- a/drivers/media/platform/omap3isp/ispresizer.c
> +++ b/drivers/media/platform/omap3isp/ispresizer.c
> @@ -1532,6 +1532,20 @@ static int resizer_set_format(struct v4l2_subdev *sd,
> struct v4l2_subdev_fh *fh, return 0;
> }
>
> +static int resizer_link_validate(struct v4l2_subdev *sd,
> + struct media_link *link,
> + struct v4l2_subdev_format *source_fmt,
> + struct v4l2_subdev_format *sink_fmt)
> +{
> + struct isp_res_device *res = v4l2_get_subdevdata(sd);
> + struct isp_pipeline *pipe = to_isp_pipeline(&sd->entity);
> +
> + omap3isp_resizer_max_rate(res, &pipe->max_rate);
> +
> + return v4l2_subdev_link_validate_default(sd, link,
> + source_fmt, sink_fmt);
> +}
> +
> /*
> * resizer_init_formats - Initialize formats on all pads
> * @sd: ISP resizer V4L2 subdevice
> @@ -1570,6 +1584,7 @@ static const struct v4l2_subdev_pad_ops
> resizer_v4l2_pad_ops = { .set_fmt = resizer_set_format,
> .get_selection = resizer_get_selection,
> .set_selection = resizer_set_selection,
> + .link_validate = resizer_link_validate,
> };
>
> /* subdev operations */
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c index a0b737fe..aae70f7 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -280,55 +280,6 @@ static int isp_video_get_graph_data(struct isp_video
> *video, return 0;
> }
>
> -/*
> - * Validate a pipeline by checking both ends of all links for format
> - * discrepancies.
> - *
> - * Compute the minimum time per frame value as the maximum of time per
> frame - * limits reported by every block in the pipeline.
> - *
> - * Return 0 if all formats match, or -EPIPE if at least one link is found
> with - * different formats on its two ends or if the pipeline doesn't start
> with a - * video source (either a subdev with no input pad, or a non-subdev
> entity). - */
> -static int isp_video_validate_pipeline(struct isp_pipeline *pipe)
> -{
> - struct isp_device *isp = pipe->output->isp;
> - struct media_pad *pad;
> - struct v4l2_subdev *subdev;
> -
> - subdev = isp_video_remote_subdev(pipe->output, NULL);
> - if (subdev == NULL)
> - return -EPIPE;
> -
> - while (1) {
> - /* Retrieve the sink format */
> - pad = &subdev->entity.pads[0];
> - if (!(pad->flags & MEDIA_PAD_FL_SINK))
> - break;
> -
> - /* Update the maximum frame rate */
> - if (subdev == &isp->isp_res.subdev)
> - omap3isp_resizer_max_rate(&isp->isp_res,
> - &pipe->max_rate);
> -
> - /* Retrieve the source format. Return an error if no source
> - * entity can be found, and stop checking the pipeline if the
> - * source entity isn't a subdev.
> - */
> - pad = media_entity_remote_source(pad);
> - if (pad == NULL)
> - return -EPIPE;
I think we're loosing this check if you remove this function. A pipeline with
no source will not be flagged as invalid. Maybe it would be possible to move
this check to the media core.
> -
> - if (media_entity_type(pad->entity) != MEDIA_ENT_T_V4L2_SUBDEV)
> - break;
> -
> - subdev = media_entity_to_v4l2_subdev(pad->entity);
> - }
> -
> - return 0;
> -}
> -
> static int
> __isp_video_get_format(struct isp_video *video, struct v4l2_format *format)
> {
> @@ -1056,11 +1007,6 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type) if (ret < 0)
> goto err_check_format;
>
> - /* Validate the pipeline and update its state. */
> - ret = isp_video_validate_pipeline(pipe);
> - if (ret < 0)
> - goto err_check_format;
> -
> pipe->error = false;
>
> spin_lock_irqsave(&pipe->lock, flags);
--
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: [media-workshop] Tentative Agenda for the November workshop
Hi Laurent, On 10/22/2012 12:39 PM, Laurent Pinchart wrote: > Hello, > > On Monday 22 October 2012 10:35:56 Hans Verkuil wrote: >> Hi all, >> >> This is the tentative agenda for the media workshop on November 8, 2012. >> If you have additional things that you want to discuss, or something is >> wrong or incomplete in this list, please let me know so I can update the >> list. > > Thank you Hans for taking care of the agenda. > >> - Explain current merging process (Mauro) >> - Open floor for discussions on how to improve it (Mauro) >> - Write down minimum requirements for new V4L2 (and DVB?) drivers, both for >> staging and mainline acceptance: which frameworks to use, v4l2-compliance, >> etc. (Hans Verkuil) >> - V4L2 ambiguities (Hans Verkuil) >> - TSMux device (a mux rather than a demux): Alain Volmat >> - dmabuf status, esp. with regards to being able to test (Mauro/Samsung) >> - Device tree support (Guennadi, not known yet whether this topic is needed) >> - Creating/selecting contexts for hardware that supports this (Samsung, >> only if time is available) > > This last topic will likely require lots of brainstorming, and thus time. If > the schedule permits, would anyone be interested in meeting earlier during > the > week already ? My intention was to also possibly discuss it with others before the actual media workshop. Would be nice if we could have arranged such a meeting. I'm not sure about the room conditions though. It's probably not a big issue, unless there is really many people interested in that topic. -- 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: [media-workshop] Tentative Agenda for the November workshop
Hello, On Monday 22 October 2012 10:35:56 Hans Verkuil wrote: > Hi all, > > This is the tentative agenda for the media workshop on November 8, 2012. > If you have additional things that you want to discuss, or something is > wrong or incomplete in this list, please let me know so I can update the > list. Thank you Hans for taking care of the agenda. > - Explain current merging process (Mauro) > - Open floor for discussions on how to improve it (Mauro) > - Write down minimum requirements for new V4L2 (and DVB?) drivers, both for > staging and mainline acceptance: which frameworks to use, v4l2-compliance, > etc. (Hans Verkuil) > - V4L2 ambiguities (Hans Verkuil) > - TSMux device (a mux rather than a demux): Alain Volmat > - dmabuf status, esp. with regards to being able to test (Mauro/Samsung) > - Device tree support (Guennadi, not known yet whether this topic is needed) > - Creating/selecting contexts for hardware that supports this (Samsung, > only if time is available) This last topic will likely require lots of brainstorming, and thus time. If the schedule permits, would anyone be interested in meeting earlier during the week already ? > For those whose name is behind a topic: please prepare something before the > meeting. > > We have currently 17 or 18 attendees of a maximum of 25, so there is room > for a few more people. -- 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
[PATCH] s5p-csis: Select S5P_SETUP_MIPIPHY
After commit ccbfd1d49dc0d6a "[media] s5p-csis: Replace phy_enable platform.." s5p-csis depends now on the commmon MIPI CSIS/MIPI DSIM DPHY control code. Add select S5P_SETUP_MIPIPHY to make this dependency explicit, until the code from arch/arm/plat-samsung is moved to drivers/ directory and converted to a regular phy driver module. Signed-off-by: Sylwester Nawrocki Signed-off-by: Kyungmin Park --- drivers/media/platform/s5p-fimc/Kconfig |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/platform/s5p-fimc/Kconfig b/drivers/media/platform/s5p-fimc/Kconfig index 8f090a8..c16b20d 100644 --- a/drivers/media/platform/s5p-fimc/Kconfig +++ b/drivers/media/platform/s5p-fimc/Kconfig @@ -24,6 +24,7 @@ config VIDEO_S5P_FIMC config VIDEO_S5P_MIPI_CSIS tristate "S5P/EXYNOS MIPI-CSI2 receiver (MIPI-CSIS) driver" depends on REGULATOR + select S5P_SETUP_MIPIPHY help This is a V4L2 driver for Samsung S5P and EXYNOS4 SoC MIPI-CSI2 receiver (MIPI-CSIS) devices. -- 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 2/2] media: V4L2: support asynchronous subdevice registration
Hi Guennadi,
I've reviewed this patch and I have a few questions:
On Sat October 20 2012 00:20:24 Guennadi Liakhovetski wrote:
> Currently bridge device drivers register devices for all subdevices
> synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor
> is attached to a video bridge device, the bridge driver will create an I2C
> device and wait for the respective I2C driver to probe. This makes linking
> of devices straight forward, but this approach cannot be used with
> intrinsically asynchronous and unordered device registration systems like
> the Flattened Device Tree. To support such systems this patch adds an
> asynchronous subdevice registration framework to V4L2. To use it respective
> (e.g. I2C) subdevice drivers must request deferred probing as long as their
> bridge driver hasn't probed. The bridge driver during its probing submits a
> an arbitrary number of subdevice descriptor groups to the framework to
> manage. After that it can add callbacks to each of those groups to be
> called at various stages during subdevice probing, e.g. after completion.
> Then the bridge driver can request single groups to be probed, finish its
> own probing and continue its video subsystem configuration from its
> callbacks.
What is the purpose of allowing multiple groups? I can't think of any reason
why you would want to have more than one group. If you have just one group
you can simplify this code quite a bit: most of the v4l2_async_group fields
can just become part of struct v4l2_device, you don't need the 'list' and
'v4l2_dev' fields anymore and the 'bind' and 'complete' callbacks can be
implemented using the v4l2_device notify callback which we already have.
>
> Signed-off-by: Guennadi Liakhovetski
> ---
>
> One more thing to note about this patch. Subdevice drivers, supporting
> asynchronous probing, and using this framework, need a unified way to
> detect, whether their probing should succeed or they should request
> deferred probing. I implement this using device platform data. This means,
> that all subdevice drivers, wishing to use this API will have to use the
> same platform data struct. I don't think this is a major inconvenience,
> but if we decide against this, we'll have to add a V4L2 function to verify
> "are you ready for me or not." The latter would be inconvenient, because
> then we would have to look through all registered subdevice descriptor
> groups for this specific subdevice.
I have to admit that I don't quite follow this. I guess I would need to see
this being used in an actual driver.
>
> drivers/media/v4l2-core/Makefile |3 +-
> drivers/media/v4l2-core/v4l2-async.c | 249
> +
> drivers/media/v4l2-core/v4l2-device.c |2 +
> include/media/v4l2-async.h| 88
> include/media/v4l2-device.h |6 +
> include/media/v4l2-subdev.h | 16 ++
> 6 files changed, 363 insertions(+), 1 deletions(-)
> create mode 100644 drivers/media/v4l2-core/v4l2-async.c
> create mode 100644 include/media/v4l2-async.h
>
> diff --git a/drivers/media/v4l2-core/Makefile
> b/drivers/media/v4l2-core/Makefile
> index cb5fede..074e01c 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -5,7 +5,8 @@
> tuner-objs := tuner-core.o
>
> videodev-objs:= v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o
> \
> - v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
> + v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
> + v4l2-async.o
> ifeq ($(CONFIG_COMPAT),y)
>videodev-objs += v4l2-compat-ioctl32.o
> endif
> diff --git a/drivers/media/v4l2-core/v4l2-async.c
> b/drivers/media/v4l2-core/v4l2-async.c
> new file mode 100644
> index 000..871f116
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -0,0 +1,249 @@
> +/*
> + * V4L2 asynchronous subdevice registration API
> + *
> + * Copyright (C) 2012, Guennadi Liakhovetski
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +
> +static bool match_i2c(struct device *dev, struct v4l2_async_hw_device
> *hw_dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + return hw_dev->bus_type == V4L2_ASYNC_BUS_I2C &&
> + hw_dev->match.i2c.adapter_id == client->adapter->nr &&
> + hw_dev->match.i2c.address == client->addr;
> +}
> +
> +static bool match_platform(struct device *dev, struct v4l2_async_hw_device
> *hw_dev)
> +{
> + return hw_dev->bus_type == V4L2_ASYNC_BUS_PLATFORM &&
> + !strcmp(hw_dev->match.platform.name,
Re: [PATCH 1/2] media: V4L2: add temporary clock helpers
On 10/22/2012 11:14 AM, Guennadi Liakhovetski wrote:
> On Sun, 21 Oct 2012, Sylwester Nawrocki wrote:
>> On 10/20/2012 12:20 AM, Guennadi Liakhovetski wrote:
>>> Typical video devices like camera sensors require an external clock source.
>>> Many such devices cannot even access their hardware registers without a
>>> running clock. These clock sources should be controlled by their consumers.
>>> This should be performed, using the generic clock framework. Unfortunately
>>> so far only very few systems have been ported to that framework. This patch
>>> adds a set of temporary helpers, mimicking the generic clock API, to V4L2.
>>> Platforms, adopting the clock API, should switch to using it. Eventually
>>> this temporary API should be removed.
>>
>> So I gave this patch a try this weekend. I would have a few comments/
>> questions. Thank you for sharing this!
>
> You mean you actually tried to use it? Wow, impressive! :-)
>
>>> Signed-off-by: Guennadi Liakhovetski
>>> ---
>>> drivers/media/v4l2-core/Makefile |2 +-
>>> drivers/media/v4l2-core/v4l2-clk.c | 126
>>>
>>> include/media/v4l2-clk.h | 48 ++
>>> 3 files changed, 175 insertions(+), 1 deletions(-)
>>> create mode 100644 drivers/media/v4l2-core/v4l2-clk.c
>>> create mode 100644 include/media/v4l2-clk.h
>>>
>>> diff --git a/drivers/media/v4l2-core/Makefile
>>> b/drivers/media/v4l2-core/Makefile
>>> index 00f64d6..cb5fede 100644
>>> --- a/drivers/media/v4l2-core/Makefile
>>> +++ b/drivers/media/v4l2-core/Makefile
>>> @@ -5,7 +5,7 @@
>>> tuner-objs:= tuner-core.o
>>>
>>> videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o
>>> \
>>> - v4l2-event.o v4l2-ctrls.o v4l2-subdev.o
>>> + v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
>>> ifeq ($(CONFIG_COMPAT),y)
>>> videodev-objs += v4l2-compat-ioctl32.o
>>> endif
>>> diff --git a/drivers/media/v4l2-core/v4l2-clk.c
>>> b/drivers/media/v4l2-core/v4l2-clk.c
>>> new file mode 100644
>>> index 000..7d457e4
>>> --- /dev/null
>>> +++ b/drivers/media/v4l2-core/v4l2-clk.c
>>> @@ -0,0 +1,126 @@
>>> +/*
>>> + * V4L2 clock service
>>
>> A like the name :-D
>>
>>> + *
>>> + * Copyright (C) 2012, Guennadi Liakhovetski
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +#include
>>> +#include
>>> +
>>> +static DEFINE_MUTEX(clk_lock);
>>> +static LIST_HEAD(v4l2_clk);
>>
>> nit: how about naming this lists v4l2_clks ?
>>
>>> +
>>> +struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id)
>>> +{
>>> + struct v4l2_clk *clk = NULL;
>>> +
>>> + mutex_lock(&clk_lock);
>>> + if (!id) {
>>> + if (list_is_singular(&v4l2_clk)) {
>>
>> Hmm, the clock list is global, why should we assume there will be only one
>> entry with NULL v4l2_clk::id ?
>
> This is testing for a case, when the user is trying to obtain a clock
> without providing an ID, similar to how with real clocks you can do
>
> clk = clk_get(dev, NULL);
Right, but there may be a need to handle more than one clock like this.
In Samsung Exynos SoC there are two clocks, for each physical video input
bus. I will like not use this temporary clock API, as I seem to have common
clock framework and DT support available there (just need to sort out a few
crashes yet ;). But still why not to allow more than one clock ?
>> It would be useful to not provide the per
>> subdev clock id when there is only one clock used per a sub-device, which
>> is a majority of cases AFAICT.
>>
>>
>>> + clk = list_entry(&v4l2_clk, struct v4l2_clk, list);
>>> + if (!strstr(sd->name, clk->dev_id))
>>
>> Ok, then clk->dev_id is supposed to be a sub-string of sd->name,
>> looks good...
>
> Looks like the no-ID case hasn't been tested... Make it
>
> + clk = list_first_entry(&v4l2_clk, struct v4l2_clk,
> list);
> + if (strcmp(sd->name, clk->dev_id))
Right, I noticed it too, and fixed this list handling temporarily like that.
As for sd->name, this is supposed to be subdev's name as created by subdev
driver during its' probing ? And the clock may need to be registered before
subdev has been probed or the host gets hold of the subdev ?
How would we make sure the host knows _subdev's_, i.e. not it's driver's
name ?
I know there are standard functions like v4l2_i2c_subdev_init() and
v4l2_spi_subdev_init() that initialize sd->name using standar pattern, but
subdev driver can overwrite the name. I.e. to avoid I2C adapter and I2C
slave address numbers creeping in into subdev names, which are then exposed
to user space through Media Controller API.
>>
>>> +
Re: [PATCH 0/11] introduce macros for i2c_msg initialization
I have been looking at this again, with the macros
#define I2C_MSG_OP(_addr, _buf, _len, _flags) \
{ .addr = _addr, .buf = _buf, .len = _len, .flags = _flags }
#define I2C_MSG_WRITE(addr, buf, len) \
I2C_MSG_OP(addr, buf, len, 0)
#define I2C_MSG_READ(addr, buf, len) \
I2C_MSG_OP(addr, buf, len, I2C_M_RD)
#define I2C_MSG_WRITE_OP(addr, buf, len, op) \
I2C_MSG_OP(addr, buf, len, 0 | op)
#define I2C_MSG_READ_OP(addr, buf, len, op) \
I2C_MSG_OP(addr, buf, len, I2C_M_RD | op)
and the tuners files as a random first example. This time I haven't made
any adjustments for arrays of size 1.
The following file is a bit unfortunate, because a single structure is
mostly used for writing, but sometimes used for reading, in the function
tda827xa_set_params. That is, there are explicit assignments to
msg.flags.
drivers/media/tuners/tda827x.c
Currently, this is done in 8 files across the entire kernel. Would it be
a problem to use a READ or WRITE macro to initialize such a structure,
give that the type is going to change? Maybe the I2C_MSG_OP macro could
be used, with an explicit flag argument?
julia
--
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 1/2] media: V4L2: add temporary clock helpers
Hi Sylwester
On Sun, 21 Oct 2012, Sylwester Nawrocki wrote:
> Hi Guennadi,
>
> On 10/20/2012 12:20 AM, Guennadi Liakhovetski wrote:
> > Typical video devices like camera sensors require an external clock source.
> > Many such devices cannot even access their hardware registers without a
> > running clock. These clock sources should be controlled by their consumers.
> > This should be performed, using the generic clock framework. Unfortunately
> > so far only very few systems have been ported to that framework. This patch
> > adds a set of temporary helpers, mimicking the generic clock API, to V4L2.
> > Platforms, adopting the clock API, should switch to using it. Eventually
> > this temporary API should be removed.
>
> So I gave this patch a try this weekend. I would have a few comments/
> questions. Thank you for sharing this!
You mean you actually tried to use it? Wow, impressive! :-)
> > Signed-off-by: Guennadi Liakhovetski
> > ---
> > drivers/media/v4l2-core/Makefile |2 +-
> > drivers/media/v4l2-core/v4l2-clk.c | 126
> >
> > include/media/v4l2-clk.h | 48 ++
> > 3 files changed, 175 insertions(+), 1 deletions(-)
> > create mode 100644 drivers/media/v4l2-core/v4l2-clk.c
> > create mode 100644 include/media/v4l2-clk.h
> >
> > diff --git a/drivers/media/v4l2-core/Makefile
> > b/drivers/media/v4l2-core/Makefile
> > index 00f64d6..cb5fede 100644
> > --- a/drivers/media/v4l2-core/Makefile
> > +++ b/drivers/media/v4l2-core/Makefile
> > @@ -5,7 +5,7 @@
> > tuner-objs:= tuner-core.o
> >
> > videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o
> > \
> > - v4l2-event.o v4l2-ctrls.o v4l2-subdev.o
> > + v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
> > ifeq ($(CONFIG_COMPAT),y)
> > videodev-objs += v4l2-compat-ioctl32.o
> > endif
> > diff --git a/drivers/media/v4l2-core/v4l2-clk.c
> > b/drivers/media/v4l2-core/v4l2-clk.c
> > new file mode 100644
> > index 000..7d457e4
> > --- /dev/null
> > +++ b/drivers/media/v4l2-core/v4l2-clk.c
> > @@ -0,0 +1,126 @@
> > +/*
> > + * V4L2 clock service
>
> A like the name :-D
>
> > + *
> > + * Copyright (C) 2012, Guennadi Liakhovetski
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +#include
> > +
> > +static DEFINE_MUTEX(clk_lock);
> > +static LIST_HEAD(v4l2_clk);
>
> nit: how about naming this lists v4l2_clks ?
>
> > +
> > +struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id)
> > +{
> > + struct v4l2_clk *clk = NULL;
> > +
> > + mutex_lock(&clk_lock);
> > + if (!id) {
> > + if (list_is_singular(&v4l2_clk)) {
>
> Hmm, the clock list is global, why should we assume there will be only one
> entry with NULL v4l2_clk::id ?
This is testing for a case, when the user is trying to obtain a clock
without providing an ID, similar to how with real clocks you can do
clk = clk_get(dev, NULL);
> It would be useful to not provide the per
> subdev clock id when there is only one clock used per a sub-device, which
> is a majority of cases AFAICT.
>
>
> > + clk = list_entry(&v4l2_clk, struct v4l2_clk, list);
> > + if (!strstr(sd->name, clk->dev_id))
>
> Ok, then clk->dev_id is supposed to be a sub-string of sd->name,
> looks good...
Looks like the no-ID case hasn't been tested... Make it
+ clk = list_first_entry(&v4l2_clk, struct v4l2_clk,
list);
+ if (strcmp(sd->name, clk->dev_id))
>
> > + clk = ERR_PTR(-ENODEV);
> > + } else {
> > + clk = ERR_PTR(-EINVAL);
> > + }
> > + } else {
> > + list_for_each_entry(clk,&v4l2_clk, list) {
> > + if (!strcmp(id, clk->id)&&
> > + !strcmp(sd->name, clk->dev_id))
>
> but why we are doing a "strong" check here ? Couldn't the second strcmp()
> be just strstr() ?
I prefer both to be strcmp to avoid degenerate cases with just one letter
etc.
> > + break;
> > + }
> > + if (&clk->list ==&v4l2_clk)
> > + clk = ERR_PTR(-ENODEV);
> > + }
> > + mutex_unlock(&clk_lock);
> > +
> > + if (!IS_ERR(clk)&&
> > + !try_module_get(clk->ops->owner))
> > + clk = ERR_PTR(-ENODEV);
> > +
> > + return clk;
> > +}
> > +EXPORT_SYMBOL(v4l2_clk_get);
> >
> > +void v4l2_clk_put(struct v4l2_clk *clk)
> > +{
> > + if (!IS_ERR(clk))
> > + module_put(clk->ops->owner);
> > +}
> > +EXPORT_SYMBOL(v4l2_clk_put);
> > +
> > +int v4l2_clk_enable(struct v4l2_clk *clk)
> > +{
> > + if (!clk->ops->enable)
Tentative Agenda for the November workshop
Hi all, This is the tentative agenda for the media workshop on November 8, 2012. If you have additional things that you want to discuss, or something is wrong or incomplete in this list, please let me know so I can update the list. - Explain current merging process (Mauro) - Open floor for discussions on how to improve it (Mauro) - Write down minimum requirements for new V4L2 (and DVB?) drivers, both for staging and mainline acceptance: which frameworks to use, v4l2-compliance, etc. (Hans Verkuil) - V4L2 ambiguities (Hans Verkuil) - TSMux device (a mux rather than a demux): Alain Volmat - dmabuf status, esp. with regards to being able to test (Mauro/Samsung) - Device tree support (Guennadi, not known yet whether this topic is needed) - Creating/selecting contexts for hardware that supports this (Samsung, only if time is available) For those whose name is behind a topic: please prepare something before the meeting. We have currently 17 or 18 attendees of a maximum of 25, so there is room for a few more people. 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] Add chipid to fc2580.c
diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c
index aff39ae..102d942 100644
I found a fellow Asus U3100+ user (mentioned him before with the
firmware issue) that even when using the latest firmware, still see's
0xff as the chipID.
--- a/drivers/media/tuners/fc2580.c
+++ b/drivers/media/tuners/fc2580.c
@@ -497,6 +497,7 @@ struct dvb_frontend *fc2580_attach(struct
dvb_frontend *fe,
switch (chip_id) {
case 0x56:
case 0x5a:
+ case 0xff:
break;
default:
goto err;
--
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 1/2 v6] of: add helper to parse display timings
On Sat, Oct 20, 2012 at 09:59:51PM +0200, Thierry Reding wrote:
> On Thu, Oct 04, 2012 at 07:59:19PM +0200, Steffen Trumtrar wrote:
> [...]
> > diff --git a/include/linux/of_display_timings.h
> > b/include/linux/of_display_timings.h
> [...]
> > +struct display_timings {
> > + unsigned int num_timings;
> > + unsigned int default_timing;
> > +
> > + struct signal_timing **timings;
> > +};
> > +
> > +struct timing_entry {
> > + u32 min;
> > + u32 typ;
> > + u32 max;
> > +};
> > +
> > +struct signal_timing {
>
> I'm slightly confused by the naming here. signal_timing seems overly
> generic in this context. Is there any reason why this isn't called
> display_timing or even display_mode?
>
You are right. I actually already changed that, for the same reasons.
It will be called display_timing in the next version, as I think that's what it
really
is.
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v6] of: add generic videomode description
On Sat, Oct 20, 2012 at 01:04:12PM +0200, Thierry Reding wrote:
> On Thu, Oct 04, 2012 at 07:59:20PM +0200, Steffen Trumtrar wrote:
> [...]
> > diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
> [...]
> > +#if defined(CONFIG_DRM)
>
> This should be:
>
> #if IS_ENABLED(CONFIG_DRM)
>
> or the code below won't be included if DRM is built as a module. But see
> my other replies as to how we can probably handle this better by moving
> this into the DRM subsystem.
>
I already started with moving...now I only need some time to finish with it.
> > +int videomode_to_display_mode(struct videomode *vm, struct
> > drm_display_mode *dmode)
> > +{
> > + memset(dmode, 0, sizeof(*dmode));
>
> It appears the usual method to obtain a drm_display_mode to allocate it
> using drm_mode_create(), which will allocate it and associate it with
> the struct drm_device.
>
> Now, if you do a memset() on the structure you'll overwrite a number of
> fields that have previously been initialized and are actually required
> to get everything cleaned up properly later on.
>
> So I think we should remove the call to memset().
>
I was not aware of that. The memset has to go than, of course.
> > +int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
> > + int index)
> > +{
> [...]
> > +}
> > +EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
>
> This should be:
>
> EXPORT_SYMBOL_GPL(of_get_fb_videomode);
Oops.
Regrads,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
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: Config does not allow DVB USB cards in 3.6 kernel
On 22.10.2012 07:20, cij...@volny.cz wrote: Hello, I use several USB DVBT tuners. Till 3.5 kernel I was able to compile kernel with support for them. Since 3.6, the oldconfig does not enable support even it is present in .config. Would anybody tell me which feature must be enabled in 3.6 to enable DVB-T tuners? I checked KCONFIG and I would say I have everything included... My .config for 3.5.7 kernel is included. Thank you for your help Best regards Michal I had the same problem with a few different 3.6 kernel versions. There is no official statement but I suspect it's caused by refactoring of v4l and rewriting drivers into new driver model. There are much more dvb drivers in 3.7 kernel (but probably still not all of those existing in 3.5). So I think 3.5 is for now last kernel which can be used in HTPCs/PVRs. It's pity that such important shortcoming has no clear info in log of kernel changes. Seeing progress in 3.7 I see that probably 3.8 will gain most of DVB drivers. Marx -- 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
