Re: [PATCH] [media] hdpvr: Simplify the logic that checks for error

[Hans Verkuil]

On Mon May 27 2013 14:04:29 Mauro Carvalho Chehab wrote:
> At get_video_info, there's a somewhat complex logic that checks
> for error.
> 
> That logic can be highly simplified, as usb_control_msg will
> only return a negative value, or the buffer length, as it does
> the transfers via DMA.
> 
> While here, document why this particular driver is returning -EFAULT,
> instead of the USB error code.

Nacked-by: Hans Verkuil 

The EFAULT comment is wrong. The way it is done today is that the error
return of this function is never passed on to userspace.

It's getting messy, so I think it is best if I make two patches based on this
patch and on Leo's fourth patch and post those. If everyone agrees on my 
solution,
then they can be merged.

Sorry Leo, I wasn't aware when we discussed the usb_control_msg return values
before that usb_control_msg() will either return an error or the buffer length,
and nothing else.

Your fourth patch introduced some bugs which I hadn't realized until yesterday.
Which is why it wasn't merged. The main problem with your fourth patch was that
it passed on the get_video_info error code to userspace, but that error code was
for internal use only, and -EFAULT is an inappropriate error code to pass on.

Regards,

Hans

> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/usb/hdpvr/hdpvr-control.c | 23 +--
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c 
> b/drivers/media/usb/hdpvr/hdpvr-control.c
> index d1a3d84..a015a24 100644
> --- a/drivers/media/usb/hdpvr/hdpvr-control.c
> +++ b/drivers/media/usb/hdpvr/hdpvr-control.c
> @@ -56,12 +56,6 @@ int get_video_info(struct hdpvr_device *dev, struct 
> hdpvr_video_info *vidinf)
> 0x1400, 0x0003,
> dev->usbc_buf, 5,
> 1000);
> - if (ret == 5) {
> - vidinf->width   = dev->usbc_buf[1] << 8 | dev->usbc_buf[0];
> - vidinf->height  = dev->usbc_buf[3] << 8 | dev->usbc_buf[2];
> - vidinf->fps = dev->usbc_buf[4];
> - }
> -
>  #ifdef HDPVR_DEBUG
>   if (hdpvr_debug & MSG_INFO) {
>   char print_buf[15];
> @@ -73,11 +67,20 @@ int get_video_info(struct hdpvr_device *dev, struct 
> hdpvr_video_info *vidinf)
>  #endif
>   mutex_unlock(&dev->usbc_mutex);
>  
> - if (ret > 0 && ret != 5) { /* fail if unexpected byte count returned */
> - ret = -EFAULT;
> - }
> + /*
> +  * Returning EFAULT is wrong. Unfortunately, MythTV hdpvr
> +  * handling code was written to expect this specific error,
> +  * instead of accepting any error code. So, we can't fix it
> +  * in Kernel without breaking userspace.
> +  */
> + if (ret < 0)
> + return -EFAULT;
>  
> - return ret < 0 ? ret : 0;
> + vidinf->width   = dev->usbc_buf[1] << 8 | dev->usbc_buf[0];
> + vidinf->height  = dev->usbc_buf[3] << 8 | dev->usbc_buf[2];
> + vidinf->fps = dev->usbc_buf[4];
> +
> + return 0;
>  }
>  
>  int get_input_lines_info(struct hdpvr_device *dev)
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] go7007 firmware updates

[Hans Verkuil]

On Mon May 27 2013 23:53:15 Ben Hutchings wrote:
> On Mon, 2013-05-27 at 21:56 +0200, Hans Verkuil wrote:
> > On Mon May 27 2013 18:24:32 Ben Hutchings wrote:
> > > On Thu, 2013-05-23 at 10:25 +0200, Hans Verkuil wrote:
> > > > Hi Ben, David,
> > > > 
> > > > The go7007 staging driver has been substantially overhauled for kernel 
> > > > 3.10.
> > > > As part of that process the firmware situation has been improved as 
> > > > well.
> > > > 
> > > > While Micronas allowed the firmware to be redistributed, it was never 
> > > > made
> > > > part of linux-firmware. Only the firmwares for the Sensoray S2250 were 
> > > > added
> > > > in the past, but those need the go7007*.bin firmwares as well to work.
> > > > 
> > > > This pull request collects all the firmwares necessary to support all 
> > > > the
> > > > go7007 devices into the go7007 directory. With this change the go7007 
> > > > driver
> > > > will work out-of-the-box starting with kernel 3.10.
> > > [...]
> > > 
> > > You should not rename files like this.  linux-firmware is not versioned
> > > and needs to be compatible with old and new kernel versions, so far as
> > > possible.
> > 
> > I understand, and I wouldn't have renamed these two firmware files if it
> > wasn't for the fact that 1) it concerns a staging driver, so in my view
> > backwards compatibility is not a requirement,
> 
> This driver (or set of drivers) has been requesting go7007fw.bin,
> go7007tv.bin, s2250.fw and s2250_loader.fw for nearly 5 years.  It's a
> bit late to say those were just temporary filenames.

Why not? It is a staging driver for good reasons. Just because it is in staging
for a long time (because nobody found the time to actually work on it until
3.10) doesn't mean it magically becomes non-staging. The Kconfig in staging
says:

  This option allows you to select a number of drivers that are
  not of the "normal" Linux kernel quality level.  These drivers
  are placed here in order to get a wider audience to make use of
  them.  Please note that these drivers are under heavy
  development, may or may not work, and may contain userspace
  interfaces that most likely will be changed in the near
  future.

In other words, there are no guarantees. That's the whole point of staging.
Once it is moved out of staging everything changes, of course.

Note that it is not just the firmware loading that has changed in 3.10, also
several custom ioctls were removed, thus changing the userspace API, and for
3.11/12 I will add new ioctls to support motion detection. Once that's done
the driver can move out of staging.

> > and 2) the firmware files
> > currently in linux-firmware were never enough to make the Sensoray S2250
> > work, you always needed additional external firmwares as well.
> > 
> > > So the filenames in linux-firmware should match whatever the driver has
> > > used up to now.  If the driver has been changed in 3.10-rc to use
> > > different filenames, it's not too late to revert this mistake in the
> > > driver.  But if such a change was made earlier, we'll need to add
> > > symlinks.
> > 
> > I can revert the rename action, but I would rather not do it. I believe
> > there are good reasons for doing this, especially since the current
> > situation is effectively broken anyway due to the missing firmware files.
> 
> Were the 'new' files unavailable to the public, or only available from a
> manufacturer web site?

They have been available from various websites where people hosted their own
version of the out-of-tree driver from before the driver was moved to staging.
They originated from the manufacturer, which no longer exists. To my knowledge
there isn't any 'canonical' site containing the firmwares.

Loading the firmware was actually quite complex for some go7007 devices,
requiring a userspace utility and special udev rules. That's all changed in
3.10.

Also note that some of the firmware files on those websites were in a hex
format. Starting with 3.10 these files are converted to a smaller binary
format and now use a standard cypress loader kernel module instead of requiring
in-kernel hex parsing.

Regards,

Hans

> 
> Ben.
> 
> > If you really don't want to rename the two S2250 files, then I'll make
> > a patch reverting those to the original filename.
> > 
> > Pete, if you have an opinion regarding this, please let us know. After all,
> > it concerns a Sensoray device.
> 
> 
--
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: Introduce a new helper framework for buffer synchronization

[Inki Dae]

> -Original Message-
> From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of
> Daniel Vetter
> Sent: Tuesday, May 28, 2013 1:02 AM
> To: Rob Clark
> Cc: Inki Dae; Maarten Lankhorst; linux-fbdev; YoungJun Cho; Kyungmin Park;
> myungjoo.ham; DRI mailing list; linux-arm-ker...@lists.infradead.org;
> linux-media@vger.kernel.org
> Subject: Re: Introduce a new helper framework for buffer synchronization
> 
> On Mon, May 27, 2013 at 5:47 PM, Rob Clark  wrote:
> > On Mon, May 27, 2013 at 6:38 AM, Inki Dae  wrote:
> >> Hi all,
> >>
> >> I have been removed previous branch and added new one with more
cleanup.
> >> This time, the fence helper doesn't include user side interfaces and
> cache
> >> operation relevant codes anymore because not only we are not sure that
> >> coupling those two things, synchronizing caches and buffer access
> between
> >> CPU and CPU, CPU and DMA, and DMA and DMA with fences, in kernel side
> is a
> >> good idea yet but also existing codes for user side have problems with
> badly
> >> behaved or crashing userspace. So this could be more discussed later.
> >>
> >> The below is a new branch,
> >>
> >> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> exynos.git/?h=dma-f
> >> ence-helper
> >>
> >> And fence helper codes,
> >>
> >> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> exynos.git/commit/?
> >> h=dma-fence-helper&id=adcbc0fe7e285ce866e5816e5e21443dcce01005
> >>
> >> And example codes for device driver,
> >>
> >> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> exynos.git/commit/?
> >> h=dma-fence-helper&id=d2ce7af23835789602a99d0ccef1f53cdd5caaae
> >>
> >> I think the time is not yet ripe for RFC posting: maybe existing dma
> fence
> >> and reservation need more review and addition work. So I'd glad for
> somebody
> >> giving other opinions and advices in advance before RFC posting.
> >
> > thoughts from a *really* quick, pre-coffee, first look:
> > * any sort of helper to simplify single-buffer sort of use-cases (v4l)
> > probably wouldn't want to bake in assumption that seqno_fence is used.
> 
> Yeah, which is why Maarten&I discussed ideas already for what needs to
> be improved in the current dma-buf interface code to make this Just
> Work. At least as long as a driver doesn't want to add new fences,
> which would be especially useful for all kinds of gpu access.
> 
> > * I guess g2d is probably not actually a simple use case, since I
> > expect you can submit blits involving multiple buffers :-P
> 
> Yeah, on a quick read the current fence helper code seems to be a bit
> limited in scope.
> 
> > * otherwise, you probably don't want to depend on dmabuf, which is why
> > reservation/fence is split out the way it is..  you want to be able to
> > use a single reservation/fence mechanism within your driver without
> > having to care about which buffers are exported to dmabuf's and which
> > are not.  Creating a dmabuf for every GEM bo is too heavyweight.
> 
> That's pretty much the reason that reservations are free-standing from
> dma_bufs. The idea is to embed them into the gem/ttm/v4l buffer
> object. Maarten also has some helpers to keep track of multi-buffer
> ww_mutex locking and fence attaching in his reservation helpers, but I
> think we should wait with those until we have drivers using them.
> 
> For now I think the priority should be to get the basic stuff in and
> ttm as the first user established. Then we can go nuts later on.
> 
> > I'm not entirely sure if reservation/fence could/should be made any
> > simpler for multi-buffer users.  Probably the best thing to do is just
> > get reservation/fence rolled out in a few drivers and see if some
> > common patterns emerge.
> 
> I think we can make the 1 buffer per dma op (i.e. 1:1
> dma_buf->reservation : fence mapping) work fairly simple in dma_buf
> with maybe a dma_buf_attachment_start_dma/end_dma helpers. But there's
> also still the open that currently there's no way to flush cpu caches
> for dma access without unmapping the attachement (or resorting to


That was what I tried adding user interfaces to dmabuf: coupling
synchronizing caches and buffer access between CPU and CPU, CPU and DMA, and
DMA and DMA with fences in kernel side. We need something to do between
mapping and unmapping attachment.

> trick which might not work), so we have a few gaping holes in the
> interface already anyway.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

--
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: Introduce a new helper framework for buffer synchronization

[Inki Dae]

> -Original Message-
> From: linux-fbdev-ow...@vger.kernel.org [mailto:linux-fbdev-
> ow...@vger.kernel.org] On Behalf Of Rob Clark
> Sent: Tuesday, May 28, 2013 12:48 AM
> To: Inki Dae
> Cc: Maarten Lankhorst; Daniel Vetter; linux-fbdev; YoungJun Cho; Kyungmin
> Park; myungjoo.ham; DRI mailing list;
linux-arm-ker...@lists.infradead.org;
> linux-media@vger.kernel.org
> Subject: Re: Introduce a new helper framework for buffer synchronization
> 
> On Mon, May 27, 2013 at 6:38 AM, Inki Dae  wrote:
> > Hi all,
> >
> > I have been removed previous branch and added new one with more cleanup.
> > This time, the fence helper doesn't include user side interfaces and
> cache
> > operation relevant codes anymore because not only we are not sure that
> > coupling those two things, synchronizing caches and buffer access
> between
> > CPU and CPU, CPU and DMA, and DMA and DMA with fences, in kernel side is
> a
> > good idea yet but also existing codes for user side have problems with
> badly
> > behaved or crashing userspace. So this could be more discussed later.
> >
> > The below is a new branch,
> >
> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> exynos.git/?h=dma-f
> > ence-helper
> >
> > And fence helper codes,
> >
> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> exynos.git/commit/?
> > h=dma-fence-helper&id=adcbc0fe7e285ce866e5816e5e21443dcce01005
> >
> > And example codes for device driver,
> >
> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> exynos.git/commit/?
> > h=dma-fence-helper&id=d2ce7af23835789602a99d0ccef1f53cdd5caaae
> >
> > I think the time is not yet ripe for RFC posting: maybe existing dma
> fence
> > and reservation need more review and addition work. So I'd glad for
> somebody
> > giving other opinions and advices in advance before RFC posting.
> 
> thoughts from a *really* quick, pre-coffee, first look:
> * any sort of helper to simplify single-buffer sort of use-cases (v4l)
> probably wouldn't want to bake in assumption that seqno_fence is used.
> * I guess g2d is probably not actually a simple use case, since I
> expect you can submit blits involving multiple buffers :-P

I don't think so. One and more buffers can be used: seqno_fence also has
only one buffer. Actually, we have already applied this approach to most
devices; multimedia, gpu and display controller. And this approach shows
more performance; reduced power consumption against traditional way. And g2d
example is just to show you how to apply my approach to device driver.

> * otherwise, you probably don't want to depend on dmabuf, which is why
> reservation/fence is split out the way it is..  you want to be able to
> use a single reservation/fence mechanism within your driver without
> having to care about which buffers are exported to dmabuf's and which
> are not.  Creating a dmabuf for every GEM bo is too heavyweight.

Right. But I think we should dealt with this separately. Actually, we are
trying to use reservation for gpu pipe line synchronization such as sgx sync
object and this approach is used without dmabuf. In order words, some device
can use only reservation for such pipe line synchronization and at the same
time, fence helper or similar thing with dmabuf for buffer synchronization.

> 
> I'm not entirely sure if reservation/fence could/should be made any
> simpler for multi-buffer users.  Probably the best thing to do is just
> get reservation/fence rolled out in a few drivers and see if some
> common patterns emerge.
> 
> BR,
> -R
> 
> >
> > Thanks,
> > Inki Dae
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH stable < v3.7] media mantis: fix silly crash case

[Ben Hutchings]

On Tue, 2013-05-21 at 15:07 +0200, Bjørn Mork wrote:
> Hello,
> 
> Please apply mainline commit e1d45ae to any maintained stable kernel
> prior to v3.7.  I just hit this bug on a Debian 3.2.41-2+deb7u2 kernel:
[...]

Queued up for 3.2, thanks.

Ben.

-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.


signature.asc
Description: This is a digitally signed message part

RE: Introduce a new helper framework for buffer synchronization

[Inki Dae]

> -Original Message-
> From: Maarten Lankhorst [mailto:maarten.lankho...@canonical.com]
> Sent: Tuesday, May 28, 2013 12:23 AM
> To: Inki Dae
> Cc: 'Daniel Vetter'; 'Rob Clark'; 'linux-fbdev'; 'YoungJun Cho'; 'Kyungmin
> Park'; 'myungjoo.ham'; 'DRI mailing list'; linux-arm-
> ker...@lists.infradead.org; linux-media@vger.kernel.org
> Subject: Re: Introduce a new helper framework for buffer synchronization
> 
> Hey,
> 
> Op 27-05-13 12:38, Inki Dae schreef:
> > Hi all,
> >
> > I have been removed previous branch and added new one with more cleanup.
> > This time, the fence helper doesn't include user side interfaces and
> cache
> > operation relevant codes anymore because not only we are not sure that
> > coupling those two things, synchronizing caches and buffer access
> between
> > CPU and CPU, CPU and DMA, and DMA and DMA with fences, in kernel side is
> a
> > good idea yet but also existing codes for user side have problems with
> badly
> > behaved or crashing userspace. So this could be more discussed later.
> >
> > The below is a new branch,
> >
> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> exynos.git/?h=dma-f
> > ence-helper
> >
> > And fence helper codes,
> >
> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> exynos.git/commit/?
> > h=dma-fence-helper&id=adcbc0fe7e285ce866e5816e5e21443dcce01005
> >
> > And example codes for device driver,
> >
> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> exynos.git/commit/?
> > h=dma-fence-helper&id=d2ce7af23835789602a99d0ccef1f53cdd5caaae
> >
> > I think the time is not yet ripe for RFC posting: maybe existing dma
> fence
> > and reservation need more review and addition work. So I'd glad for
> somebody
> > giving other opinions and advices in advance before RFC posting.
> >
> NAK.
> 
> For examples for how to handle locking properly, see Documentation/ww-
> mutex-design.txt in my recent tree.
> I could list what I believe is wrong with your implementation, but real
> problem is that the approach you're taking is wrong.

I just removed ticket stubs to show my approach you guys as simple as
possible, and I just wanted to show that we could use buffer synchronization
mechanism without ticket stubs.

Question, WW-Mutexes could be used for all devices? I guess this has
dependence on x86 gpu: gpu has VRAM and it means different memory domain.
And could you tell my why shared fence should have only eight objects? I
think we could need more than eight objects for read access. Anyway I think
I don't surely understand yet so there might be my missing point.

Thanks,
Inki Dae

> 

> ~Maarten

--
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: EM28xx - new device ID - Ion "Video Forever" USB capture dongle

[alxgomz]

Hi Philip,

Thank you for sharing this bit of info. I just bought what I think to be the
very same device from local maplin too.
I have loaded it using your tweak.
Just like you the composite video input works just great, however I can't
get any sound captured using the RCA audio leads. 
The S-video doesn't work either :( (only one new v4l2 video source is
registred loading the device driver). 

I have done all my test using ffmpeg (and arecord to test audio only) but I
am quite confident the outcome would be the same with other programs.

When loading the driver with card=9, I can see in the log:

"em2860 #0: Sigmatel audio processor detected(stac 9752)"

I wonder how much "detection" there is here under the hood, as I suspect the
module param may have forced this (wrongly perhaps).
So as far as I am concerned, this ion thingy doesn't match exactly, the 9 card.
How can I gather more informations about that device?

Regards Alex.

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

Re: [GIT PULL] go7007 firmware updates

[Ben Hutchings]

On Mon, 2013-05-27 at 21:56 +0200, Hans Verkuil wrote:
> On Mon May 27 2013 18:24:32 Ben Hutchings wrote:
> > On Thu, 2013-05-23 at 10:25 +0200, Hans Verkuil wrote:
> > > Hi Ben, David,
> > > 
> > > The go7007 staging driver has been substantially overhauled for kernel 
> > > 3.10.
> > > As part of that process the firmware situation has been improved as well.
> > > 
> > > While Micronas allowed the firmware to be redistributed, it was never made
> > > part of linux-firmware. Only the firmwares for the Sensoray S2250 were 
> > > added
> > > in the past, but those need the go7007*.bin firmwares as well to work.
> > > 
> > > This pull request collects all the firmwares necessary to support all the
> > > go7007 devices into the go7007 directory. With this change the go7007 
> > > driver
> > > will work out-of-the-box starting with kernel 3.10.
> > [...]
> > 
> > You should not rename files like this.  linux-firmware is not versioned
> > and needs to be compatible with old and new kernel versions, so far as
> > possible.
> 
> I understand, and I wouldn't have renamed these two firmware files if it
> wasn't for the fact that 1) it concerns a staging driver, so in my view
> backwards compatibility is not a requirement,

This driver (or set of drivers) has been requesting go7007fw.bin,
go7007tv.bin, s2250.fw and s2250_loader.fw for nearly 5 years.  It's a
bit late to say those were just temporary filenames.

> and 2) the firmware files
> currently in linux-firmware were never enough to make the Sensoray S2250
> work, you always needed additional external firmwares as well.
> 
> > So the filenames in linux-firmware should match whatever the driver has
> > used up to now.  If the driver has been changed in 3.10-rc to use
> > different filenames, it's not too late to revert this mistake in the
> > driver.  But if such a change was made earlier, we'll need to add
> > symlinks.
> 
> I can revert the rename action, but I would rather not do it. I believe
> there are good reasons for doing this, especially since the current
> situation is effectively broken anyway due to the missing firmware files.

Were the 'new' files unavailable to the public, or only available from a
manufacturer web site?

Ben.

> If you really don't want to rename the two S2250 files, then I'll make
> a patch reverting those to the original filename.
> 
> Pete, if you have an opinion regarding this, please let us know. After all,
> it concerns a Sensoray device.

-- 
Ben Hutchings
Experience is what causes a person to make new mistakes instead of old ones.


signature.asc
Description: This is a digitally signed message part

Re: [GIT PULL] go7007 firmware updates

[Hans Verkuil]

On Mon May 27 2013 18:24:32 Ben Hutchings wrote:
> On Thu, 2013-05-23 at 10:25 +0200, Hans Verkuil wrote:
> > Hi Ben, David,
> > 
> > The go7007 staging driver has been substantially overhauled for kernel 3.10.
> > As part of that process the firmware situation has been improved as well.
> > 
> > While Micronas allowed the firmware to be redistributed, it was never made
> > part of linux-firmware. Only the firmwares for the Sensoray S2250 were added
> > in the past, but those need the go7007*.bin firmwares as well to work.
> > 
> > This pull request collects all the firmwares necessary to support all the
> > go7007 devices into the go7007 directory. With this change the go7007 driver
> > will work out-of-the-box starting with kernel 3.10.
> [...]
> 
> You should not rename files like this.  linux-firmware is not versioned
> and needs to be compatible with old and new kernel versions, so far as
> possible.

I understand, and I wouldn't have renamed these two firmware files if it
wasn't for the fact that 1) it concerns a staging driver, so in my view
backwards compatibility is not a requirement, and 2) the firmware files
currently in linux-firmware were never enough to make the Sensoray S2250
work, you always needed additional external firmwares as well.

> So the filenames in linux-firmware should match whatever the driver has
> used up to now.  If the driver has been changed in 3.10-rc to use
> different filenames, it's not too late to revert this mistake in the
> driver.  But if such a change was made earlier, we'll need to add
> symlinks.

I can revert the rename action, but I would rather not do it. I believe
there are good reasons for doing this, especially since the current
situation is effectively broken anyway due to the missing firmware files.

If you really don't want to rename the two S2250 files, then I'll make
a patch reverting those to the original filename.

Pete, if you have an opinion regarding this, please let us know. After all,
it concerns a Sensoray device.

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

cron job: media_tree daily build: WARNINGS

[Hans Verkuil]

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

Results of the daily build of media_tree:

date:   Mon May 27 19:00:24 CEST 2013
git branch: test
git hash:   7eac97d7e714429f7ef1ba5d35f94c07f4c34f8e
gcc version:i686-linux-gcc (GCC) 4.8.0
host hardware:  x86_64
host os:3.8-3.slh.2-amd64

linux-git-arm-davinci: OK
linux-git-arm-exynos: WARNINGS
linux-git-arm-omap: WARNINGS
linux-git-blackfin: WARNINGS
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.31.14-i686: WARNINGS
linux-2.6.32.27-i686: WARNINGS
linux-2.6.33.7-i686: WARNINGS
linux-2.6.34.7-i686: WARNINGS
linux-2.6.35.9-i686: WARNINGS
linux-2.6.36.4-i686: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.10-rc1-i686: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.2.37-i686: WARNINGS
linux-3.3.8-i686: WARNINGS
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-2.6.31.14-x86_64: WARNINGS
linux-2.6.32.27-x86_64: WARNINGS
linux-2.6.33.7-x86_64: WARNINGS
linux-2.6.34.7-x86_64: WARNINGS
linux-2.6.35.9-x86_64: WARNINGS
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.10-rc1-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
apps: WARNINGS
spec-git: OK
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 Media Infrastructure API from this daily build is here:

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

Re: [GIT PULL] go7007 firmware updates

[Ben Hutchings]

On Thu, 2013-05-23 at 10:25 +0200, Hans Verkuil wrote:
> Hi Ben, David,
> 
> The go7007 staging driver has been substantially overhauled for kernel 3.10.
> As part of that process the firmware situation has been improved as well.
> 
> While Micronas allowed the firmware to be redistributed, it was never made
> part of linux-firmware. Only the firmwares for the Sensoray S2250 were added
> in the past, but those need the go7007*.bin firmwares as well to work.
> 
> This pull request collects all the firmwares necessary to support all the
> go7007 devices into the go7007 directory. With this change the go7007 driver
> will work out-of-the-box starting with kernel 3.10.
[...]

You should not rename files like this.  linux-firmware is not versioned
and needs to be compatible with old and new kernel versions, so far as
possible.

So the filenames in linux-firmware should match whatever the driver has
used up to now.  If the driver has been changed in 3.10-rc to use
different filenames, it's not too late to revert this mistake in the
driver.  But if such a change was made earlier, we'll need to add
symlinks.

Ben.

-- 
Ben Hutchings
Experience is what causes a person to make new mistakes instead of old ones.


signature.asc
Description: This is a digitally signed message part

Re: Introduce a new helper framework for buffer synchronization

[Daniel Vetter]

On Mon, May 27, 2013 at 5:47 PM, Rob Clark  wrote:
> On Mon, May 27, 2013 at 6:38 AM, Inki Dae  wrote:
>> Hi all,
>>
>> I have been removed previous branch and added new one with more cleanup.
>> This time, the fence helper doesn't include user side interfaces and cache
>> operation relevant codes anymore because not only we are not sure that
>> coupling those two things, synchronizing caches and buffer access between
>> CPU and CPU, CPU and DMA, and DMA and DMA with fences, in kernel side is a
>> good idea yet but also existing codes for user side have problems with badly
>> behaved or crashing userspace. So this could be more discussed later.
>>
>> The below is a new branch,
>>
>> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/?h=dma-f
>> ence-helper
>>
>> And fence helper codes,
>>
>> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?
>> h=dma-fence-helper&id=adcbc0fe7e285ce866e5816e5e21443dcce01005
>>
>> And example codes for device driver,
>>
>> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?
>> h=dma-fence-helper&id=d2ce7af23835789602a99d0ccef1f53cdd5caaae
>>
>> I think the time is not yet ripe for RFC posting: maybe existing dma fence
>> and reservation need more review and addition work. So I'd glad for somebody
>> giving other opinions and advices in advance before RFC posting.
>
> thoughts from a *really* quick, pre-coffee, first look:
> * any sort of helper to simplify single-buffer sort of use-cases (v4l)
> probably wouldn't want to bake in assumption that seqno_fence is used.

Yeah, which is why Maarten&I discussed ideas already for what needs to
be improved in the current dma-buf interface code to make this Just
Work. At least as long as a driver doesn't want to add new fences,
which would be especially useful for all kinds of gpu access.

> * I guess g2d is probably not actually a simple use case, since I
> expect you can submit blits involving multiple buffers :-P

Yeah, on a quick read the current fence helper code seems to be a bit
limited in scope.

> * otherwise, you probably don't want to depend on dmabuf, which is why
> reservation/fence is split out the way it is..  you want to be able to
> use a single reservation/fence mechanism within your driver without
> having to care about which buffers are exported to dmabuf's and which
> are not.  Creating a dmabuf for every GEM bo is too heavyweight.

That's pretty much the reason that reservations are free-standing from
dma_bufs. The idea is to embed them into the gem/ttm/v4l buffer
object. Maarten also has some helpers to keep track of multi-buffer
ww_mutex locking and fence attaching in his reservation helpers, but I
think we should wait with those until we have drivers using them.

For now I think the priority should be to get the basic stuff in and
ttm as the first user established. Then we can go nuts later on.

> I'm not entirely sure if reservation/fence could/should be made any
> simpler for multi-buffer users.  Probably the best thing to do is just
> get reservation/fence rolled out in a few drivers and see if some
> common patterns emerge.

I think we can make the 1 buffer per dma op (i.e. 1:1
dma_buf->reservation : fence mapping) work fairly simple in dma_buf
with maybe a dma_buf_attachment_start_dma/end_dma helpers. But there's
also still the open that currently there's no way to flush cpu caches
for dma access without unmapping the attachement (or resorting to
trick which might not work), so we have a few gaping holes in the
interface already anyway.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
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: Introduce a new helper framework for buffer synchronization

[Rob Clark]

On Mon, May 27, 2013 at 6:38 AM, Inki Dae  wrote:
> Hi all,
>
> I have been removed previous branch and added new one with more cleanup.
> This time, the fence helper doesn't include user side interfaces and cache
> operation relevant codes anymore because not only we are not sure that
> coupling those two things, synchronizing caches and buffer access between
> CPU and CPU, CPU and DMA, and DMA and DMA with fences, in kernel side is a
> good idea yet but also existing codes for user side have problems with badly
> behaved or crashing userspace. So this could be more discussed later.
>
> The below is a new branch,
>
> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/?h=dma-f
> ence-helper
>
> And fence helper codes,
>
> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?
> h=dma-fence-helper&id=adcbc0fe7e285ce866e5816e5e21443dcce01005
>
> And example codes for device driver,
>
> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?
> h=dma-fence-helper&id=d2ce7af23835789602a99d0ccef1f53cdd5caaae
>
> I think the time is not yet ripe for RFC posting: maybe existing dma fence
> and reservation need more review and addition work. So I'd glad for somebody
> giving other opinions and advices in advance before RFC posting.

thoughts from a *really* quick, pre-coffee, first look:
* any sort of helper to simplify single-buffer sort of use-cases (v4l)
probably wouldn't want to bake in assumption that seqno_fence is used.
* I guess g2d is probably not actually a simple use case, since I
expect you can submit blits involving multiple buffers :-P
* otherwise, you probably don't want to depend on dmabuf, which is why
reservation/fence is split out the way it is..  you want to be able to
use a single reservation/fence mechanism within your driver without
having to care about which buffers are exported to dmabuf's and which
are not.  Creating a dmabuf for every GEM bo is too heavyweight.

I'm not entirely sure if reservation/fence could/should be made any
simpler for multi-buffer users.  Probably the best thing to do is just
get reservation/fence rolled out in a few drivers and see if some
common patterns emerge.

BR,
-R

>
> Thanks,
> Inki Dae
>
--
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: Introduce a new helper framework for buffer synchronization

[Maarten Lankhorst]

Hey,

Op 27-05-13 12:38, Inki Dae schreef:
> Hi all,
>
> I have been removed previous branch and added new one with more cleanup.
> This time, the fence helper doesn't include user side interfaces and cache
> operation relevant codes anymore because not only we are not sure that
> coupling those two things, synchronizing caches and buffer access between
> CPU and CPU, CPU and DMA, and DMA and DMA with fences, in kernel side is a
> good idea yet but also existing codes for user side have problems with badly
> behaved or crashing userspace. So this could be more discussed later.
>
> The below is a new branch,
>   
> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/?h=dma-f
> ence-helper
>
> And fence helper codes,
>   
> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?
> h=dma-fence-helper&id=adcbc0fe7e285ce866e5816e5e21443dcce01005
>
> And example codes for device driver,
>   
> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?
> h=dma-fence-helper&id=d2ce7af23835789602a99d0ccef1f53cdd5caaae
>
> I think the time is not yet ripe for RFC posting: maybe existing dma fence
> and reservation need more review and addition work. So I'd glad for somebody
> giving other opinions and advices in advance before RFC posting.
>
NAK.

For examples for how to handle locking properly, see 
Documentation/ww-mutex-design.txt in my recent tree.
I could list what I believe is wrong with your implementation, but real problem 
is that the approach you're taking is wrong.

~Maarten
--
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: InstantFM

[Patrice Levesque]

> Good to hear, your device has "software version 0, hardware version 7",
> right? If you can confirm then I'll lower the version requirement in
> the kernel too match (so that you'll no longer get the warning message).

Yes, software version 0.



-- 
 --|--
|
Patrice Levesque
 http://ptaff.ca/
video4linux.wa...@ptaff.ca
|
 --|--
--


signature.asc
Description: Digital signature

Re: [PATCH v3 2/3] mutex: add support for wound/wait style locks, v3

[Daniel Vetter]

On Mon, May 27, 2013 at 4:47 PM, Daniel Vetter  wrote:
> On Mon, May 27, 2013 at 10:21 AM, Peter Zijlstra  wrote:
>> On Wed, May 22, 2013 at 07:24:38PM +0200, Maarten Lankhorst wrote:
>>> >> +static inline void ww_acquire_init(struct ww_acquire_ctx *ctx,
>>> >> + struct ww_class *ww_class)
>>> >> +{
>>> >> +  ctx->task = current;
>>> >> +  do {
>>> >> +  ctx->stamp = atomic_long_inc_return(&ww_class->stamp);
>>> >> +  } while (unlikely(!ctx->stamp));
>>> > I suppose we'll figure something out when this becomes a bottleneck. 
>>> > Ideally
>>> > we'd do something like:
>>> >
>>> >  ctx->stamp = local_clock();
>>> >
>>> > but for now we cannot guarantee that's not jiffies, and I suppose that's 
>>> > a tad
>>> > too coarse to work for this.
>>> This might mess up when 2 cores happen to return exactly the same time, how 
>>> do you choose a winner in that case?
>>> EDIT: Using pointer address like you suggested below is fine with me. ctx 
>>> pointer would be static enough.
>>
>> Right, but for now I suppose the 'global' atomic is ok, if/when we find
>> it hurts performance we can revisit. I was just spewing ideas :-)
>
> We could do a simple
>
> ctx->stamp = (local_clock() << nr_cpu_shift) | local_processor_id()
>
> to work around any bad luck in grabbing the ticket. With sufficient
> fine clocks the bias towards smaller cpu ids would be rather
> irrelevant. Just wanted to drop this idea before I'll forget about it
> again ;-)
Not a good idea to throw around random ideas right after a work-out.
This is broken since different threads could end up with the same low
bits. Comparing ctx pointers otoh on top of the timestamp should work.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
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/3] mutex: add support for wound/wait style locks, v3

[Daniel Vetter]

On Mon, May 27, 2013 at 10:21 AM, Peter Zijlstra  wrote:
> On Wed, May 22, 2013 at 07:24:38PM +0200, Maarten Lankhorst wrote:
>> >> +static inline void ww_acquire_init(struct ww_acquire_ctx *ctx,
>> >> + struct ww_class *ww_class)
>> >> +{
>> >> +  ctx->task = current;
>> >> +  do {
>> >> +  ctx->stamp = atomic_long_inc_return(&ww_class->stamp);
>> >> +  } while (unlikely(!ctx->stamp));
>> > I suppose we'll figure something out when this becomes a bottleneck. 
>> > Ideally
>> > we'd do something like:
>> >
>> >  ctx->stamp = local_clock();
>> >
>> > but for now we cannot guarantee that's not jiffies, and I suppose that's a 
>> > tad
>> > too coarse to work for this.
>> This might mess up when 2 cores happen to return exactly the same time, how 
>> do you choose a winner in that case?
>> EDIT: Using pointer address like you suggested below is fine with me. ctx 
>> pointer would be static enough.
>
> Right, but for now I suppose the 'global' atomic is ok, if/when we find
> it hurts performance we can revisit. I was just spewing ideas :-)

We could do a simple

ctx->stamp = (local_clock() << nr_cpu_shift) | local_processor_id()

to work around any bad luck in grabbing the ticket. With sufficient
fine clocks the bias towards smaller cpu ids would be rather
irrelevant. Just wanted to drop this idea before I'll forget about it
again ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
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

Creative WebCam Live! Pro recognized, initialised but unusable

[kewlcat]

Hello
 
I'm having trouble with a Creative WebCam Live! Pro, usb id 041e:4038 "Creative 
Technology, Ltd ORITE CCD Webcam [PC370R]"
This device is supposed to be "supported" but is marked as "untested". 
Apparently it works up to the point where sq930x fail with this message : 
gspca_sq930x: reg_r 001f failed -32
 
See a more complete log here : http://pastebin.com/SMTuhdAF
 
The kernel I'm using is a custom made 3.9.3.
What other information do you need to debug this issue ? What options must I 
activate in the kernel in order to get a more verbose syslog regarding this 
component ?
Is there any hope at all for this device, as there seems to be no information 
available at all from Creative or any other source ?
 
Sincerely,
  =^.^=
--
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] [media] hdpvr: Simplify the logic that checks for error

[Mauro Carvalho Chehab]

At get_video_info, there's a somewhat complex logic that checks
for error.

That logic can be highly simplified, as usb_control_msg will
only return a negative value, or the buffer length, as it does
the transfers via DMA.

While here, document why this particular driver is returning -EFAULT,
instead of the USB error code.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/hdpvr/hdpvr-control.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c 
b/drivers/media/usb/hdpvr/hdpvr-control.c
index d1a3d84..a015a24 100644
--- a/drivers/media/usb/hdpvr/hdpvr-control.c
+++ b/drivers/media/usb/hdpvr/hdpvr-control.c
@@ -56,12 +56,6 @@ int get_video_info(struct hdpvr_device *dev, struct 
hdpvr_video_info *vidinf)
  0x1400, 0x0003,
  dev->usbc_buf, 5,
  1000);
-   if (ret == 5) {
-   vidinf->width   = dev->usbc_buf[1] << 8 | dev->usbc_buf[0];
-   vidinf->height  = dev->usbc_buf[3] << 8 | dev->usbc_buf[2];
-   vidinf->fps = dev->usbc_buf[4];
-   }
-
 #ifdef HDPVR_DEBUG
if (hdpvr_debug & MSG_INFO) {
char print_buf[15];
@@ -73,11 +67,20 @@ int get_video_info(struct hdpvr_device *dev, struct 
hdpvr_video_info *vidinf)
 #endif
mutex_unlock(&dev->usbc_mutex);
 
-   if (ret > 0 && ret != 5) { /* fail if unexpected byte count returned */
-   ret = -EFAULT;
-   }
+   /*
+* Returning EFAULT is wrong. Unfortunately, MythTV hdpvr
+* handling code was written to expect this specific error,
+* instead of accepting any error code. So, we can't fix it
+* in Kernel without breaking userspace.
+*/
+   if (ret < 0)
+   return -EFAULT;
 
-   return ret < 0 ? ret : 0;
+   vidinf->width   = dev->usbc_buf[1] << 8 | dev->usbc_buf[0];
+   vidinf->height  = dev->usbc_buf[3] << 8 | dev->usbc_buf[2];
+   vidinf->fps = dev->usbc_buf[4];
+
+   return 0;
 }
 
 int get_input_lines_info(struct hdpvr_device *dev)
-- 
1.8.1.4

--
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/3] mutex: add support for wound/wait style locks, v3

[Maarten Lankhorst]

Op 27-05-13 13:15, Peter Zijlstra schreef:
> On Mon, May 27, 2013 at 12:52:00PM +0200, Maarten Lankhorst wrote:
>> The reason ttm needed it was because there was another lock that interacted
>> with the ctx lock in a weird way. The ww lock it was using was inverted with 
>> another
>> lock, so it had to grab that lock first, perform a trylock on the ww lock, 
>> and if that failed
>> unlock the lock, wait for it to be unlocked, then retry the same thing again.
>> I'm so glad I managed to fix that mess, if you really need ww_mutex_trylock 
>> with a ctx,
>> it's an indication your locking is wrong.
>>
>> For ww_mutex_trylock with a context to be of any use you would also need to 
>> return
>> 0 or a -errno, (-EDEADLK, -EBUSY (already locked by someone else), or 
>> -EALREADY).
>> This would make the trylock very different from other trylocks, and very 
>> confusing because
>> if (ww_mutex_trylock(lock, ctx)) would not do what you would think it would 
>> do.
> Yuck ;-)
>
> Anyway, what I was thinking of is something like:
>
>   T0  T1
>
>   try A
>   lock B
>   lock B
>   lock A
>
> Now, if for some reason T1 won the lottery such that T0 would have to be
> wounded, T0's context would indicate its the first entry and not return
> -EDEADLK.
And this sounds like something lockdep is designed to complain about.

Nothing stops you from doing try A then doing try B, which would be the correct 
way to deal with this situation.
Why would you trylock one, and then not do the same for another?

> OTOH, anybody doing creative things like that might well deserve
> whatever they get ;-)
Indeed!

>>> The thing is; if there could exist something like:
>>>
>>>   ww_mutex_trylock(struct ww_mutex *, struct ww_acquire_ctx *ctx);
>>>
>>> Then we should not now take away that name and make it mean something
>>> else; namely: ww_mutex_trylock_single().
>>>
>>> Unless we want to allow .ctx=NULL to mean _single.
>>>
>>> As to why I proposed that (.ctx=NULL meaning _single); I suppose because
>>> I'm a minimalist at heart.
>> Minimalism isn't bad, it's just knowing when to sto
> :-)
>

--
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/3] mutex: add support for wound/wait style locks, v3

[Peter Zijlstra]

On Mon, May 27, 2013 at 12:52:00PM +0200, Maarten Lankhorst wrote:
> The reason ttm needed it was because there was another lock that interacted
> with the ctx lock in a weird way. The ww lock it was using was inverted with 
> another
> lock, so it had to grab that lock first, perform a trylock on the ww lock, 
> and if that failed
> unlock the lock, wait for it to be unlocked, then retry the same thing again.
> I'm so glad I managed to fix that mess, if you really need ww_mutex_trylock 
> with a ctx,
> it's an indication your locking is wrong.
> 
> For ww_mutex_trylock with a context to be of any use you would also need to 
> return
> 0 or a -errno, (-EDEADLK, -EBUSY (already locked by someone else), or 
> -EALREADY).
> This would make the trylock very different from other trylocks, and very 
> confusing because
> if (ww_mutex_trylock(lock, ctx)) would not do what you would think it would 
> do.

Yuck ;-)

Anyway, what I was thinking of is something like:

T0  T1

try A
lock B
lock B
lock A

Now, if for some reason T1 won the lottery such that T0 would have to be
wounded, T0's context would indicate its the first entry and not return
-EDEADLK.

OTOH, anybody doing creative things like that might well deserve
whatever they get ;-)

> > The thing is; if there could exist something like:
> >
> >   ww_mutex_trylock(struct ww_mutex *, struct ww_acquire_ctx *ctx);
> >
> > Then we should not now take away that name and make it mean something
> > else; namely: ww_mutex_trylock_single().
> >
> > Unless we want to allow .ctx=NULL to mean _single.
> >
> > As to why I proposed that (.ctx=NULL meaning _single); I suppose because
> > I'm a minimalist at heart.
> Minimalism isn't bad, it's just knowing when to sto

:-)
--
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/3] mutex: add support for wound/wait style locks, v3

[Maarten Lankhorst]

Op 27-05-13 12:24, Peter Zijlstra schreef:
> On Mon, May 27, 2013 at 12:01:50PM +0200, Maarten Lankhorst wrote:
>>> Again, early.. monday.. would a trylock, even if successful still need
>>> the ctx?
>> No ctx for trylock is supported. You can still do a trylock while
>> holding a context, but the mutex won't be a part of the context.
>> Normal lockdep rules apply. lib/locking-selftest.c:
>>
>> context + ww_mutex_lock first, then a trylock:
>> dotest(ww_test_context_try, SUCCESS, LOCKTYPE_WW);
>>
>> trylock first, then context + ww_mutex_lock:
>> dotest(ww_test_try_context, FAILURE, LOCKTYPE_WW);
>>
>> For now I don't want to add support for a trylock with context, I'm
>> very glad I managed to fix ttm locking to not require this any more,
>> and it was needed there only because it was a workaround for the
>> locking being wrong.  There was no annotation for the buffer locking
>> it was using, so the real problem wasn't easy to spot.
> Ah, ok. 
>
> My question really was whether there even was sense for a trylock with
> context. I couldn't come up with a case for it; but I think I see one
> now.
The reason ttm needed it was because there was another lock that interacted
with the ctx lock in a weird way. The ww lock it was using was inverted with 
another
lock, so it had to grab that lock first, perform a trylock on the ww lock, and 
if that failed
unlock the lock, wait for it to be unlocked, then retry the same thing again.
I'm so glad I managed to fix that mess, if you really need ww_mutex_trylock 
with a ctx,
it's an indication your locking is wrong.

For ww_mutex_trylock with a context to be of any use you would also need to 
return
0 or a -errno, (-EDEADLK, -EBUSY (already locked by someone else), or 
-EALREADY).
This would make the trylock very different from other trylocks, and very 
confusing because
if (ww_mutex_trylock(lock, ctx)) would not do what you would think it would do.
> The thing is; if there could exist something like:
>
>   ww_mutex_trylock(struct ww_mutex *, struct ww_acquire_ctx *ctx);
>
> Then we should not now take away that name and make it mean something
> else; namely: ww_mutex_trylock_single().
>
> Unless we want to allow .ctx=NULL to mean _single.
>
> As to why I proposed that (.ctx=NULL meaning _single); I suppose because
> I'm a minimalist at heart.
Minimalism isn't bad, it's just knowing when to sto

--
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: Introduce a new helper framework for buffer synchronization

[Inki Dae]

Hi all,

I have been removed previous branch and added new one with more cleanup.
This time, the fence helper doesn't include user side interfaces and cache
operation relevant codes anymore because not only we are not sure that
coupling those two things, synchronizing caches and buffer access between
CPU and CPU, CPU and DMA, and DMA and DMA with fences, in kernel side is a
good idea yet but also existing codes for user side have problems with badly
behaved or crashing userspace. So this could be more discussed later.

The below is a new branch,

https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/?h=dma-f
ence-helper

And fence helper codes,

https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?
h=dma-fence-helper&id=adcbc0fe7e285ce866e5816e5e21443dcce01005

And example codes for device driver,

https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?
h=dma-fence-helper&id=d2ce7af23835789602a99d0ccef1f53cdd5caaae

I think the time is not yet ripe for RFC posting: maybe existing dma fence
and reservation need more review and addition work. So I'd glad for somebody
giving other opinions and advices in advance before RFC posting.

Thanks,
Inki Dae

--
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/3] mutex: add support for wound/wait style locks, v3

[Peter Zijlstra]

On Mon, May 27, 2013 at 12:01:50PM +0200, Maarten Lankhorst wrote:
> > Again, early.. monday.. would a trylock, even if successful still need
> > the ctx?
> No ctx for trylock is supported. You can still do a trylock while
> holding a context, but the mutex won't be a part of the context.
> Normal lockdep rules apply. lib/locking-selftest.c:
> 
> context + ww_mutex_lock first, then a trylock:
> dotest(ww_test_context_try, SUCCESS, LOCKTYPE_WW);
> 
> trylock first, then context + ww_mutex_lock:
> dotest(ww_test_try_context, FAILURE, LOCKTYPE_WW);
> 
> For now I don't want to add support for a trylock with context, I'm
> very glad I managed to fix ttm locking to not require this any more,
> and it was needed there only because it was a workaround for the
> locking being wrong.  There was no annotation for the buffer locking
> it was using, so the real problem wasn't easy to spot.

Ah, ok. 

My question really was whether there even was sense for a trylock with
context. I couldn't come up with a case for it; but I think I see one
now.

The thing is; if there could exist something like:

  ww_mutex_trylock(struct ww_mutex *, struct ww_acquire_ctx *ctx);

Then we should not now take away that name and make it mean something
else; namely: ww_mutex_trylock_single().

Unless we want to allow .ctx=NULL to mean _single.

As to why I proposed that (.ctx=NULL meaning _single); I suppose because
I'm a minimalist at heart.
--
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/3] mutex: add support for wound/wait style locks, v3

[Maarten Lankhorst]

Op 27-05-13 10:21, Peter Zijlstra schreef:
> On Wed, May 22, 2013 at 07:24:38PM +0200, Maarten Lankhorst wrote:
 +static inline void ww_acquire_init(struct ww_acquire_ctx *ctx,
 + struct ww_class *ww_class)
 +{
 +  ctx->task = current;
 +  do {
 +  ctx->stamp = atomic_long_inc_return(&ww_class->stamp);
 +  } while (unlikely(!ctx->stamp));
>>> I suppose we'll figure something out when this becomes a bottleneck. Ideally
>>> we'd do something like:
>>>
>>>  ctx->stamp = local_clock();
>>>
>>> but for now we cannot guarantee that's not jiffies, and I suppose that's a 
>>> tad
>>> too coarse to work for this.
>> This might mess up when 2 cores happen to return exactly the same time, how 
>> do you choose a winner in that case?
>> EDIT: Using pointer address like you suggested below is fine with me. ctx 
>> pointer would be static enough.
> Right, but for now I suppose the 'global' atomic is ok, if/when we find
> it hurts performance we can revisit. I was just spewing ideas :-)
If  accurate timers are available it wouldn't be a bad idea. I fixed up the 
code to at least support this case should it happen.
For now the source of the stamp is still a single atomic_long.

>>> Also, why is 0 special?
>> Oops, 0 is no longer special.
>>
>> I used to set the samp directly on the lock, so 0 used to mean no ctx set.
> Ah, ok :-)
>
 +static inline int __must_check ww_mutex_trylock_single(struct ww_mutex 
 *lock)
 +{
 +  return mutex_trylock(&lock->base);
 +}
>>> trylocks can never deadlock they don't block per definition, I don't see the
>>> point of the _single() thing here.
>> I called it single because they weren't annotated into any ctx. I can drop 
>> the _single suffix though,
>> but you'd still need to unlock with unlock_single, or we need to remove that 
>> distinction altogether,
>> lose a few lockdep checks and only have a one unlock function.
> Again, early.. monday.. would a trylock, even if successful still need
> the ctx?
No ctx for trylock is supported. You can still do a trylock while holding a 
context, but the mutex won't be
a part of the context. Normal lockdep rules apply. lib/locking-selftest.c:

context + ww_mutex_lock first, then a trylock:
dotest(ww_test_context_try, SUCCESS, LOCKTYPE_WW);

trylock first, then context + ww_mutex_lock:
dotest(ww_test_try_context, FAILURE, LOCKTYPE_WW);

For now I don't want to add support for a trylock with context, I'm very glad I 
managed to fix ttm locking
to not require this any more, and it was needed there only because it was a 
workaround for the locking
being wrong.  There was no annotation for the buffer locking it was using, so 
the real problem wasn't easy to spot.

~Maarten
--
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/3] mutex: add support for wound/wait style locks, v3

[Maarten Lankhorst]

Op 27-05-13 11:13, Peter Zijlstra schreef:
> On Mon, May 27, 2013 at 10:26:39AM +0200, Maarten Lankhorst wrote:
>> Op 27-05-13 10:00, Peter Zijlstra schreef:
>>> On Wed, May 22, 2013 at 07:24:38PM +0200, Maarten Lankhorst wrote:
>> +- Functions to only acquire a single w/w mutex, which results in the 
>> exact same
>> +  semantics as a normal mutex. These functions have the _single postfix.
> This is missing rationale.
 trylock_single is useful when iterating over a list, and you want to evict 
 a bo, but only the first one that can be acquired.
 lock_single is useful when only a single bo needs to be acquired, for 
 example to lock a buffer during mmap.
>>> OK, so given that its still early, monday and I haven't actually spend
>>> much time thinking on this; would it be possible to make:
>>> ww_mutex_lock(.ctx=NULL) act like ww_mutex_lock_single()?
>>>
>>> The idea is that if we don't provide a ctx, we'll get a different
>>> lockdep annotation; mutex_lock() vs mutex_lock_nest_lock(). So if we
>>> then go and make a mistake, lockdep should warn us.
>>>
>>> Would that work or should I stock up on morning juice?
>>>
>> It's easy to merge unlock_single and unlock, which I did in the next version 
>> I'll post.
>> Lockdep will already warn if ww_mutex_lock and ww_mutex_lock_single are both
>> used. ww_test_block_context and ww_test_context_block in 
>> lib/locking-selftest.c
>> are the testcases for this.
>>
>> The locking paths are too different, it will end up with doing "if (ctx == 
>> NULL) mutex_lock(); else ww_mutex_lock();"
> I was more thinking like:
>
> int __sched ww_mutex_lock(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
> {
>   might_sleep();
>   return __mutex_lock_common(&lock->base, TASK_UNINTERRUPTIBLE, 0,
>  ctx ? ctx->dep_map : NULL, _RET_IP_,
>  ctx, 0);
> }
>
> That should make ww_mutex_lock(.ctx=NULL) equivalent to
> mutex_lock(&lock->base), no?
>
> Anyway, implementation aside, it would again reduce the interface some.
>
It doesn't work like that. __builtin_constant_p(ctx == NULL) will evaluate to 
false in __mutex_lock_common, even if you call ww_mutex_lock(lock, NULL);
gcc cannot prove at compile time whether ctx == NULL is true or false for the 
__mutex_lock_common inlining here, so __builtin_constant_p() will return false.

And again, that's just saying

ww_mutex_lock() {
if (ctx)
original ww_mutex_lock's slowpath(lock, ctx);
else
mutex_lock's slowpath(lock->base);
}

And the next version will already remove unlock_single, and this is the 
implementation for lock_single currently:
static inline void ww_mutex_lock_single(struct ww_mutex *lock)
{
mutex_lock(&lock->base);
}

So why do you want to merge it?

~Maarten

--
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 22/24] drivers/media/radio/radio-maxiradio: Convert to module_pci_driver

[Hans Verkuil]

On Mon 27 May 2013 04:31:59 Libo Chen wrote:
> use module_pci_driver instead of init/exit, make code clean
> 
> Signed-off-by: Libo Chen 

Acked-by: Hans Verkuil 

> ---
>  drivers/media/radio/radio-maxiradio.c |   13 +
>  1 files changed, 1 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/radio/radio-maxiradio.c 
> b/drivers/media/radio/radio-maxiradio.c
> index bd4d3a7..1d1c9e1 100644
> --- a/drivers/media/radio/radio-maxiradio.c
> +++ b/drivers/media/radio/radio-maxiradio.c
> @@ -200,15 +200,4 @@ static struct pci_driver maxiradio_driver = {
>   .remove = maxiradio_remove,
>  };
>  
> -static int __init maxiradio_init(void)
> -{
> - return pci_register_driver(&maxiradio_driver);
> -}
> -
> -static void __exit maxiradio_exit(void)
> -{
> - pci_unregister_driver(&maxiradio_driver);
> -}
> -
> -module_init(maxiradio_init);
> -module_exit(maxiradio_exit);
> +module_pci_driver(maxiradio_driver);
> 
--
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/3] mutex: add support for wound/wait style locks, v3

[Peter Zijlstra]

On Mon, May 27, 2013 at 10:26:39AM +0200, Maarten Lankhorst wrote:
> Op 27-05-13 10:00, Peter Zijlstra schreef:
> > On Wed, May 22, 2013 at 07:24:38PM +0200, Maarten Lankhorst wrote:
>  +- Functions to only acquire a single w/w mutex, which results in the 
>  exact same
>  +  semantics as a normal mutex. These functions have the _single postfix.
> >>> This is missing rationale.
> >> trylock_single is useful when iterating over a list, and you want to evict 
> >> a bo, but only the first one that can be acquired.
> >> lock_single is useful when only a single bo needs to be acquired, for 
> >> example to lock a buffer during mmap.
> > OK, so given that its still early, monday and I haven't actually spend
> > much time thinking on this; would it be possible to make:
> > ww_mutex_lock(.ctx=NULL) act like ww_mutex_lock_single()?
> >
> > The idea is that if we don't provide a ctx, we'll get a different
> > lockdep annotation; mutex_lock() vs mutex_lock_nest_lock(). So if we
> > then go and make a mistake, lockdep should warn us.
> >
> > Would that work or should I stock up on morning juice?
> >
> It's easy to merge unlock_single and unlock, which I did in the next version 
> I'll post.
> Lockdep will already warn if ww_mutex_lock and ww_mutex_lock_single are both
> used. ww_test_block_context and ww_test_context_block in 
> lib/locking-selftest.c
> are the testcases for this.
> 
> The locking paths are too different, it will end up with doing "if (ctx == 
> NULL) mutex_lock(); else ww_mutex_lock();"

I was more thinking like:

int __sched ww_mutex_lock(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
{
might_sleep();
return __mutex_lock_common(&lock->base, TASK_UNINTERRUPTIBLE, 0,
   ctx ? ctx->dep_map : NULL, _RET_IP_,
   ctx, 0);
}

That should make ww_mutex_lock(.ctx=NULL) equivalent to
mutex_lock(&lock->base), no?

Anyway, implementation aside, it would again reduce the interface some.
--
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] Fix spelling of Qt in .desktop file (typo)

[Hans Verkuil]

On Sun 26 May 2013 10:15:51 Diego Viola wrote:
> Proper spelling of Qt is Qt, not QT.  "QT" is often confused with
> QuickTime, here is a minor patch to fix this issue in the .desktop file.

Thanks, I've committed this.

Regards,

Hans

> 
> Signed-off-by: Diego Viola 
> ---
>  utils/qv4l2/qv4l2.desktop | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utils/qv4l2/qv4l2.desktop b/utils/qv4l2/qv4l2.desktop
> index 00f3e33..69413e1 100644
> --- a/utils/qv4l2/qv4l2.desktop
> +++ b/utils/qv4l2/qv4l2.desktop
> @@ -1,5 +1,5 @@
>  [Desktop Entry]
> -Name=QT V4L2 test Utility
> +Name=Qt V4L2 test Utility
>  Name[pt]=Utilitário de teste V4L2
>  Comment=Allow testing Video4Linux devices
>  Comment[pt]=Permite testar dispositivos Video4Linux
> 
--
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: i2c: mt9p031: add OF support

[Sascha Hauer]

On Sun, May 26, 2013 at 06:38:54PM +0530, Prabhakar Lad wrote:
> From: Lad, Prabhakar 
> 
> add OF support for the mt9p031 sensor driver.
> Alongside this patch sorts the header inclusion alphabetically.
> 
> Signed-off-by: Lad, Prabhakar 
> Cc: Hans Verkuil 
> Cc: Laurent Pinchart 
> Cc: Mauro Carvalho Chehab 
> Cc: Guennadi Liakhovetski 
> Cc: Sylwester Nawrocki 
> Cc: Sakari Ailus 
> Cc: Grant Likely 
> Cc: Sascha Hauer 
> Cc: Rob Herring 
> Cc: Rob Landley 
> Cc: Arnd Bergmann 
> Cc: devicetree-disc...@lists.ozlabs.org
> Cc: davinci-linux-open-sou...@linux.davincidsp.com
> Cc: linux-...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org

Reviewed-by: Sascha Hauer 

Sascha

> ---
>  Changes for NON RFC v1:
>  1: added missing call for of_node_put().
>  
>  Changes for RFC v4 (https://patchwork.kernel.org/patch/2556251/):
>  1: Renamed "gpio-reset" property to "reset-gpios".
>  2: Dropped assigning the driver data from the of node.
> 
>  Changes for RFC v3(https://patchwork.kernel.org/patch/2515921/):
>  1: Dropped check if gpio-reset is valid.
>  2: Fixed some code nits.
>  3: Included a reference to the V4L2 DT bindings documentation.
> 
>  Changes for RFC v2 (https://patchwork.kernel.org/patch/2510201/):
>  1: Used '-' instead of '_' for device properties.
>  2: Specified gpio reset pin as phandle in device node.
>  3: Handle platform data properly even if kernel is compiled with
> devicetree support.
>  4: Used dev_* for messages in drivers instead of pr_*.
>  5: Changed compatible property to "aptina,mt9p031" and "aptina,mt9p031m".
>  6: Sorted the header inclusion alphabetically and fixed some minor code nits.
>  
>  RFC v1: https://patchwork.kernel.org/patch/2498791/
>  
>  .../devicetree/bindings/media/i2c/mt9p031.txt  |   40 ++
>  drivers/media/i2c/mt9p031.c|   43 
> +++-
>  2 files changed, 81 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt 
> b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> new file mode 100644
> index 000..59d613c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> @@ -0,0 +1,40 @@
> +* Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
> +
> +The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image sensor 
> with
> +an active array size of 2592H x 1944V. It is programmable through a simple
> +two-wire serial interface.
> +
> +Required Properties :
> +- compatible : value should be either one among the following
> + (a) "aptina,mt9p031" for mt9p031 sensor
> + (b) "aptina,mt9p031m" for mt9p031m sensor
> +
> +- input-clock-frequency : Input clock frequency.
> +
> +- pixel-clock-frequency : Pixel clock frequency.
> +
> +Optional Properties :
> +- reset-gpios: Chip reset GPIO
> +
> +For further reading of port node refer 
> Documentation/devicetree/bindings/media/
> +video-interfaces.txt.
> +
> +Example:
> +
> + i2c0@1c22000 {
> + ...
> + ...
> + mt9p031@5d {
> + compatible = "aptina,mt9p031";
> + reg = <0x5d>;
> + reset-gpios = <&gpio3 30 0>;
> +
> + port {
> + mt9p031_1: endpoint {
> + input-clock-frequency = <600>;
> + pixel-clock-frequency = <9600>;
> + };
> + };
> + };
> + ...
> + };
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index bf49899..bb1f993 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -16,9 +16,10 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -28,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "aptina-pll.h"
> @@ -928,10 +930,37 @@ static const struct v4l2_subdev_internal_ops 
> mt9p031_subdev_internal_ops = {
>   * Driver initialization and probing
>   */
>  
> +static struct mt9p031_platform_data *
> +mt9p031_get_pdata(struct i2c_client *client)
> +{
> + struct mt9p031_platform_data *pdata = NULL;
> + struct device_node *np;
> +
> + if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
> + return client->dev.platform_data;
> +
> + np = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
> + if (!np)
> + return NULL;
> +
> + pdata = devm_kzalloc(&client->dev, sizeof(struct mt9p031_platform_data),
> +  GFP_KERNEL);
> + if (!pdata)
> + goto done;
> +
> + pdata->reset = of_get_named_gpio(client->dev.of_node, "reset-gpios", 0);
> + of_property_read_u32(np, "input-clock-frequency", &pdata->ext_freq);
> + o

Re: [PATCH v4] adv7180: add more subdev video ops

[Hans Verkuil]

Hi Sergei, Vladimir,

Just one small remaining issue:

On Thu 23 May 2013 22:05:42 Sergei Shtylyov wrote:
> From: Vladimir Barinov 
> 
> Add subdev video ops for ADV7180 video decoder.  This makes decoder usable on
> the soc-camera drivers.
> 
> Signed-off-by: Vladimir Barinov 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch is against the 'media_tree.git' repo.
> 
> Changes from version 3:
> - set the field format independent of a video standard in try_mbus_fmt() 
> method;
> - removed adv7180_g_mbus_fmt(), adv7180_g_mbus_fmt(), and the 'fmt' field from
>   'struct adv7180_state', and so use adv7180_try_mbus_fmt()  to implement both
>   g_mbus_fmt() and s_mbus_fmt() methods;
> - removed cropcap() method.
> 
> Changes from version 2:
> - set the field format depending on video standard in try_mbus_fmt() method;
> - removed querystd() method calls from try_mbus_fmt() and cropcap() methods;
> - removed g_crop() method.
> 
>  drivers/media/i2c/adv7180.c |   46 
> 
>  1 file changed, 46 insertions(+)
> 
> Index: media_tree/drivers/media/i2c/adv7180.c
> ===
> --- media_tree.orig/drivers/media/i2c/adv7180.c
> +++ media_tree/drivers/media/i2c/adv7180.c
> @@ -1,6 +1,8 @@
>  /*
>   * adv7180.c Analog Devices ADV7180 video decoder driver
>   * Copyright (c) 2009 Intel Corporation
> + * Copyright (C) 2013 Cogent Embedded, Inc.
> + * Copyright (C) 2013 Renesas Solutions Corp.
>   *
>   * 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
> @@ -397,10 +399,54 @@ static void adv7180_exit_controls(struct
>   v4l2_ctrl_handler_free(&state->ctrl_hdl);
>  }
>  
> +static int adv7180_enum_mbus_fmt(struct v4l2_subdev *sd, unsigned int index,
> +  enum v4l2_mbus_pixelcode *code)
> +{
> + if (index > 0)
> + return -EINVAL;
> +
> + *code = V4L2_MBUS_FMT_YUYV8_2X8;
> +
> + return 0;
> +}
> +
> +static int adv7180_try_mbus_fmt(struct v4l2_subdev *sd,
> + struct v4l2_mbus_framefmt *fmt)

Since this is used for set/get and try I recommend renaming it to
adv7180_mbus_fmt.

> +{
> + struct adv7180_state *state = to_state(sd);
> +
> + fmt->code = V4L2_MBUS_FMT_YUYV8_2X8;
> + fmt->colorspace = V4L2_COLORSPACE_SMPTE170M;
> + fmt->field = V4L2_FIELD_INTERLACED;
> + fmt->width = 720;
> + fmt->height = state->curr_norm & V4L2_STD_525_60 ? 480 : 576;
> +
> + return 0;
> +}
> +
> +static int adv7180_g_mbus_config(struct v4l2_subdev *sd,
> +  struct v4l2_mbus_config *cfg)
> +{
> + /*
> +  * The ADV7180 sensor supports BT.601/656 output modes.
> +  * The BT.656 is default and not yet configurable by s/w.
> +  */
> + cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
> +  V4L2_MBUS_DATA_ACTIVE_HIGH;
> + cfg->type = V4L2_MBUS_BT656;
> +
> + return 0;
> +}
> +
>  static const struct v4l2_subdev_video_ops adv7180_video_ops = {
>   .querystd = adv7180_querystd,
>   .g_input_status = adv7180_g_input_status,
>   .s_routing = adv7180_s_routing,
> + .enum_mbus_fmt = adv7180_enum_mbus_fmt,
> + .try_mbus_fmt = adv7180_try_mbus_fmt,
> + .g_mbus_fmt = adv7180_try_mbus_fmt,
> + .s_mbus_fmt = adv7180_try_mbus_fmt,
> + .g_mbus_config = adv7180_g_mbus_config,
>  };
>  
>  static const struct v4l2_subdev_core_ops adv7180_core_ops = {

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 v5] V4L2: I2C: ML86V7667 video decoder driver

[Hans Verkuil]

Hi Sergei, Vladimir,

Just two small things to fix, see below:

On Thu 23 May 2013 22:07:32 Sergei Shtylyov wrote:
> From: Vladimir Barinov 
> 
> Add OKI Semiconductor ML86V7667 video decoder driver.
> 
> Signed-off-by: Vladimir Barinov 
> [Sergei: added v4l2_device_unregister_subdev() call to the error cleanup path 
> of
> ml86v7667_probe(); some cleanup.]
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch is against the 'media_tree.git' repo.
> 
> Changes since version 4:
> - set the field format independent of a video standard in try_mbus_fmt() 
> method;
> - removed ml86v7667_g_mbus_fmt(), ml86v7667_g_mbus_fmt(), and the 'fmt'  field
>   from 'struct ml86v7667_priv', and so use ml86v7667_try_mbus_fmt() to 
> implement
>   both g_mbus_fmt() and s_mbus_fmt() methods;
> - removed cropcap() method;
> - removed permission check from g_register() and s_register() methods.
> 
> Changes since version 3:
> - set the field format depending on video standard in try_mbus_fmt() method;
> - refreshed the patch.
> 
> Changes since version 2:
> - removed now unused #include;
> - fixed querystd() method to return currently detected video standard;
> - switched from using V4L2_STD_[PAL|NTSC] to using V4L2_STD_[625_50|525_60].
> 
> Changes since the original posting:
> - fixed ACCC_CHROMA_CB_MASK;
> - got rid of the autodetection feature;
> - removed querystd() method calls from other methods;
> - removed deprecated g_chip_ident() method.
> 
>  drivers/media/i2c/Kconfig |9 
>  drivers/media/i2c/Makefile|1 
>  drivers/media/i2c/ml86v7667.c |  437 
> ++
>  3 files changed, 447 insertions(+)
> 
> Index: media_tree/drivers/media/i2c/Kconfig
> ===
> --- media_tree.orig/drivers/media/i2c/Kconfig
> +++ media_tree/drivers/media/i2c/Kconfig
> @@ -245,6 +245,15 @@ config VIDEO_KS0127
> To compile this driver as a module, choose M here: the
> module will be called ks0127.
>  
> +config VIDEO_ML86V7667
> + tristate "OKI ML86V7667 video decoder"
> + depends on VIDEO_V4L2 && I2C
> + ---help---
> +   Support for the OKI Semiconductor ML86V7667 video decoder.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called ml86v7667.
> +
>  config VIDEO_SAA7110
>   tristate "Philips SAA7110 video decoder"
>   depends on VIDEO_V4L2 && I2C
> Index: media_tree/drivers/media/i2c/Makefile
> ===
> --- media_tree.orig/drivers/media/i2c/Makefile
> +++ media_tree/drivers/media/i2c/Makefile
> @@ -70,3 +70,4 @@ obj-$(CONFIG_VIDEO_AS3645A) += as3645a.o
>  obj-$(CONFIG_VIDEO_SMIAPP_PLL)   += smiapp-pll.o
>  obj-$(CONFIG_VIDEO_AK881X)   += ak881x.o
>  obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
> +obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o
> Index: media_tree/drivers/media/i2c/ml86v7667.c
> ===
> --- /dev/null
> +++ media_tree/drivers/media/i2c/ml86v7667.c
> @@ -0,0 +1,437 @@
> +/*
> + * OKI Semiconductor ML86V7667 video decoder driver
> + *
> + * Author: Vladimir Barinov 
> + * Copyright (C) 2013 Cogent Embedded, Inc.
> + * Copyright (C) 2013 Renesas Solutions Corp.
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DRV_NAME "ml86v7667"
> +
> +/* Subaddresses */
> +#define MRA_REG  0x00 /* Mode Register A */
> +#define MRC_REG  0x02 /* Mode Register C */
> +#define LUMC_REG 0x0C /* Luminance Control */
> +#define CLC_REG  0x10 /* Contrast level control */
> +#define SSEPL_REG0x11 /* Sync separation level */
> +#define CHRCA_REG0x12 /* Chrominance Control A */
> +#define ACCC_REG 0x14 /* ACC Loop filter & Chrominance control */
> +#define ACCRC_REG0x15 /* ACC Reference level control */
> +#define HUE_REG  0x16 /* Hue control */
> +#define ADC2_REG 0x1F /* ADC Register 2 */
> +#define PLLR1_REG0x20 /* PLL Register 1 */
> +#define STATUS_REG   0x2C /* STATUS Register */
> +
> +/* Mode Register A register bits */
> +#define MRA_OUTPUT_MODE_MASK (3 << 6)
> +#define MRA_ITUR_BT601   (1 << 6)
> +#define MRA_ITUR_BT656   (0 << 6)
> +#define MRA_INPUT_MODE_MASK  (7 << 3)
> +#define MRA_PAL_BT601(4 << 3)
> +#define MRA_NTSC_BT601   (0 << 3)
> +#define MRA_REGISTER_MODE(1 << 0)
> +
> +/* Mode Register C register bits */
> +#define M

Re: [PATCH v3 2/3] mutex: add support for wound/wait style locks, v3

[Peter Zijlstra]

On Wed, May 22, 2013 at 06:49:04PM +0200, Daniel Vetter wrote:
> - _slow functions can check whether all acquire locks have been
> released and whether the caller is indeed blocking on the contending
> lock. Not doing so could either result in needless spinning instead of
> blocking (when blocking on the wrong lock) or in deadlocks (when not
> dropping all acquired).

We could add ww_mutex_assert_context_empty() or somesuch so that
paranoid people have a means of expressing themselves :-)
--
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/3] mutex: add support for wound/wait style locks, v3

[Maarten Lankhorst]

Op 27-05-13 10:00, Peter Zijlstra schreef:
> On Wed, May 22, 2013 at 07:24:38PM +0200, Maarten Lankhorst wrote:
 +- Functions to only acquire a single w/w mutex, which results in the 
 exact same
 +  semantics as a normal mutex. These functions have the _single postfix.
>>> This is missing rationale.
>> trylock_single is useful when iterating over a list, and you want to evict a 
>> bo, but only the first one that can be acquired.
>> lock_single is useful when only a single bo needs to be acquired, for 
>> example to lock a buffer during mmap.
> OK, so given that its still early, monday and I haven't actually spend
> much time thinking on this; would it be possible to make:
> ww_mutex_lock(.ctx=NULL) act like ww_mutex_lock_single()?
>
> The idea is that if we don't provide a ctx, we'll get a different
> lockdep annotation; mutex_lock() vs mutex_lock_nest_lock(). So if we
> then go and make a mistake, lockdep should warn us.
>
> Would that work or should I stock up on morning juice?
>
It's easy to merge unlock_single and unlock, which I did in the next version 
I'll post.
Lockdep will already warn if ww_mutex_lock and ww_mutex_lock_single are both
used. ww_test_block_context and ww_test_context_block in lib/locking-selftest.c
are the testcases for this.

The locking paths are too different, it will end up with doing "if (ctx == 
NULL) mutex_lock(); else ww_mutex_lock();"

~Maarten

--
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/3] mutex: add support for wound/wait style locks, v3

[Peter Zijlstra]

On Wed, May 22, 2013 at 07:24:38PM +0200, Maarten Lankhorst wrote:
> >> +static inline void ww_acquire_init(struct ww_acquire_ctx *ctx,
> >> + struct ww_class *ww_class)
> >> +{
> >> +  ctx->task = current;
> >> +  do {
> >> +  ctx->stamp = atomic_long_inc_return(&ww_class->stamp);
> >> +  } while (unlikely(!ctx->stamp));
> > I suppose we'll figure something out when this becomes a bottleneck. Ideally
> > we'd do something like:
> >
> >  ctx->stamp = local_clock();
> >
> > but for now we cannot guarantee that's not jiffies, and I suppose that's a 
> > tad
> > too coarse to work for this.
> This might mess up when 2 cores happen to return exactly the same time, how 
> do you choose a winner in that case?
> EDIT: Using pointer address like you suggested below is fine with me. ctx 
> pointer would be static enough.

Right, but for now I suppose the 'global' atomic is ok, if/when we find
it hurts performance we can revisit. I was just spewing ideas :-)

> > Also, why is 0 special?
> Oops, 0 is no longer special.
> 
> I used to set the samp directly on the lock, so 0 used to mean no ctx set.

Ah, ok :-)

> >> +static inline int __must_check ww_mutex_trylock_single(struct ww_mutex 
> >> *lock)
> >> +{
> >> +  return mutex_trylock(&lock->base);
> >> +}
> > trylocks can never deadlock they don't block per definition, I don't see the
> > point of the _single() thing here.
> I called it single because they weren't annotated into any ctx. I can drop 
> the _single suffix though,
> but you'd still need to unlock with unlock_single, or we need to remove that 
> distinction altogether,
> lose a few lockdep checks and only have a one unlock function.

Again, early.. monday.. would a trylock, even if successful still need
the ctx?


--
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/3] mutex: add support for wound/wait style locks, v3

[Peter Zijlstra]

On Wed, May 22, 2013 at 07:24:38PM +0200, Maarten Lankhorst wrote:
> >> +- Functions to only acquire a single w/w mutex, which results in the 
> >> exact same
> >> +  semantics as a normal mutex. These functions have the _single postfix.
> > This is missing rationale.

> trylock_single is useful when iterating over a list, and you want to evict a 
> bo, but only the first one that can be acquired.
> lock_single is useful when only a single bo needs to be acquired, for example 
> to lock a buffer during mmap.

OK, so given that its still early, monday and I haven't actually spend
much time thinking on this; would it be possible to make:
ww_mutex_lock(.ctx=NULL) act like ww_mutex_lock_single()?

The idea is that if we don't provide a ctx, we'll get a different
lockdep annotation; mutex_lock() vs mutex_lock_nest_lock(). So if we
then go and make a mistake, lockdep should warn us.

Would that work or should I stock up on morning juice?
--
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] [media] s5p-mfc: Remove special clock usage in driver

[Arun Kumar K]

MFC uses two clocks - MFC gate clock and special clock
which is named as "sclk_mfc" in exynos4 and "aclk_333" in
exynos5 SoC. The driver was doing just a clk_prepare on
this special clock without a clk_enable call. As this
sclk is the parent of gate clock, it gets prepared and
enabled along with the gate clock. So there is no need
for the driver to use this sclk. This patch removes the
sclk usage from driver.

Signed-off-by: Arun Kumar K 
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c|2 --
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |1 -
 drivers/media/platform/s5p-mfc/s5p_mfc_pm.c |   19 ---
 3 files changed, 22 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 01f9ae0..5635eff 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1362,7 +1362,6 @@ static struct s5p_mfc_variant mfc_drvdata_v5 = {
.port_num   = MFC_NUM_PORTS,
.buf_size   = &buf_size_v5,
.buf_align  = &mfc_buf_align_v5,
-   .mclk_name  = "sclk_mfc",
.fw_name= "s5p-mfc.fw",
 };
 
@@ -1389,7 +1388,6 @@ static struct s5p_mfc_variant mfc_drvdata_v6 = {
.port_num   = MFC_NUM_PORTS_V6,
.buf_size   = &buf_size_v6,
.buf_align  = &mfc_buf_align_v6,
-   .mclk_name  = "aclk_333",
.fw_name= "s5p-mfc-v6.fw",
 };
 
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h 
b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index 202d1d7..4a08c1a 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -231,7 +231,6 @@ struct s5p_mfc_variant {
unsigned int port_num;
struct s5p_mfc_buf_size *buf_size;
struct s5p_mfc_buf_align *buf_align;
-   char*mclk_name;
char*fw_name;
 };
 
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
index 6aa38a5..cab6e0b 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
@@ -50,19 +50,6 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
goto err_p_ip_clk;
}
 
-   pm->clock = clk_get(&dev->plat_dev->dev, dev->variant->mclk_name);
-   if (IS_ERR(pm->clock)) {
-   mfc_err("Failed to get MFC clock\n");
-   ret = PTR_ERR(pm->clock);
-   goto err_g_ip_clk_2;
-   }
-
-   ret = clk_prepare(pm->clock);
-   if (ret) {
-   mfc_err("Failed to prepare MFC clock\n");
-   goto err_p_ip_clk_2;
-   }
-
atomic_set(&pm->power, 0);
 #ifdef CONFIG_PM_RUNTIME
pm->device = &dev->plat_dev->dev;
@@ -72,10 +59,6 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
atomic_set(&clk_ref, 0);
 #endif
return 0;
-err_p_ip_clk_2:
-   clk_put(pm->clock);
-err_g_ip_clk_2:
-   clk_unprepare(pm->clock_gate);
 err_p_ip_clk:
clk_put(pm->clock_gate);
 err_g_ip_clk:
@@ -86,8 +69,6 @@ void s5p_mfc_final_pm(struct s5p_mfc_dev *dev)
 {
clk_unprepare(pm->clock_gate);
clk_put(pm->clock_gate);
-   clk_unprepare(pm->clock);
-   clk_put(pm->clock);
 #ifdef CONFIG_PM_RUNTIME
pm_runtime_disable(pm->device);
 #endif
-- 
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