date:20160510

cron job: media_tree daily build: OK

2016-05-10 Thread 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:   Wed May 11 04:00:25 CEST 2016
git branch: test
git hash:   d1532d5575696965a52b19553dd7dacf75f3fec5
gcc version:i686-linux-gcc (GCC) 5.3.0
sparse version: v0.5.0-56-g7647c77
smatch version: v0.5.0-3428-gdfe27cf
host hardware:  x86_64
host os:4.5.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16.7-i686: OK
linux-3.17.8-i686: OK
linux-3.18.7-i686: OK
linux-3.19-i686: OK
linux-4.0-i686: OK
linux-4.1.1-i686: OK
linux-4.2-i686: OK
linux-4.3-i686: OK
linux-4.4-i686: OK
linux-4.5-i686: OK
linux-4.6-rc1-i686: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16.7-x86_64: OK
linux-3.17.8-x86_64: OK
linux-3.18.7-x86_64: OK
linux-3.19-x86_64: OK
linux-4.0-x86_64: OK
linux-4.1.1-x86_64: OK
linux-4.2-x86_64: OK
linux-4.3-x86_64: OK
linux-4.4-x86_64: OK
linux-4.5-x86_64: OK
linux-4.6-rc1-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH] media: dvb_ringbuffer: Add memory barriers

2016-05-10 Thread Soeren Moch


Hi Mauro,

thanks for looking after this patch.

On 05/07/16 15:22, Mauro Carvalho Chehab wrote:

Hi Soeren,

Em Sun, 7 Feb 2016 20:22:36 +0100
Soeren Moch  escreveu:


On 27.12.2015 21:41, Soeren Moch wrote:

Implement memory barriers according to Documentation/circular-buffers.txt:
- use smp_store_release() to update ringbuffer read/write pointers
- use smp_load_acquire() to load write pointer on reader side
- use ACCESS_ONCE() to load read pointer on writer side

This fixes data stream corruptions observed e.g. on an ARM Cortex-A9
quad core system with different types (PCI, USB) of DVB tuners.

Signed-off-by: Soeren Moch 
Cc: sta...@vger.kernel.org # 3.14+


Mauro,

any news or comments on this?
Since this is a real fix for broken behaviour, can you pick this up, please?


The problem here is that I'm very reluctant to touch at the DVB core
without doing some tests myself, as things like locking can be
very sensible.


I agree. But this patch adds memory barriers (no locks) according to
Documentation/circular-buffers.txt. It should not be dangerous to
follow these guidelines.
Nevertheless, independent review and testing is always a good idea,
especially in core code.


I'll try to find some time to take a look on it for Kernel 4.8,


Thanks.


but I'd like to reproduce the bug locally.

>

Could you please provide me enough info to reproduce it (and
eventually some test MPEG-TS where you know this would happen)?


I used vdr with different types of DVB tuners on a TBS2910 board
(quad core ARM Cortex-A9, see arch/arm/boot/dts/imx6q-tbs2910.dts).
With more than one active cpu core I occasionally see data stream
corruptions in recorded ts streams. This is not the case for one
active cpu.

The problem occurs when dvb_dmxdev_buffer_write() and
dvb_dmxdev_buffer_read() are running simultaneously on different
cpu cores on architectures without strong write ordering (e.g.
on arm, not on x86). Here the write data pointer (written in
dvb_ringbuffer_write() ) can become visible to the other cpu core
(in dvb_ringbuffer_avail() ), before the actual ts packet data is
visible. dvb_ringbuffer_read_user() then can read old ts data,
although new ts data is already written into the ringbuffer
by the other cpu core.

With smp_store_release() and smp_load_acquire() the correct
write ordering is maintained, ts packet data is visible on the
reading cpu core before the updated write pointer.


I have two DekTek RF generators here, so I should be able to
play such TS and see what happens with and without the patch
on x86, arm32 and arm64.


On x86 you should not see any difference, since
smp_store_release() and smp_load_acquire() expand to
simple stores and loads there. On multi-core arm systems
you may see broken ts streams without patch, especially
when reading the dvr device very fast, so that the
dvb_ringbuffer is always close to empty.

Regards,
Soeren



Regards,
Mauro



Regards,
Soeren


---
Cc: Mauro Carvalho Chehab 
Cc: linux-media@vger.kernel.org
Cc: linux-ker...@vger.kernel.org

Since smp_store_release() and smp_load_acquire() were introduced in linux-3.14,
a 3.14+ stable tag was added. Is it desired to apply a similar patch to older
stable kernels?
---
  drivers/media/dvb-core/dvb_ringbuffer.c | 27 ++-
  1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ringbuffer.c 
b/drivers/media/dvb-core/dvb_ringbuffer.c
index 1100e98..58b5968 100644
--- a/drivers/media/dvb-core/dvb_ringbuffer.c
+++ b/drivers/media/dvb-core/dvb_ringbuffer.c
@@ -55,7 +55,7 @@ void dvb_ringbuffer_init(struct dvb_ringbuffer *rbuf, void 
*data, size_t len)

  int dvb_ringbuffer_empty(struct dvb_ringbuffer *rbuf)
  {
-   return (rbuf->pread==rbuf->pwrite);
+   return (rbuf->pread == smp_load_acquire(>pwrite));
  }


@@ -64,7 +64,7 @@ ssize_t dvb_ringbuffer_free(struct dvb_ringbuffer *rbuf)
  {
ssize_t free;

-   free = rbuf->pread - rbuf->pwrite;
+   free = ACCESS_ONCE(rbuf->pread) - rbuf->pwrite;
if (free <= 0)
free += rbuf->size;
return free-1;
@@ -76,7 +76,7 @@ ssize_t dvb_ringbuffer_avail(struct dvb_ringbuffer *rbuf)
  {
ssize_t avail;

-   avail = rbuf->pwrite - rbuf->pread;
+   avail = smp_load_acquire(>pwrite) - rbuf->pread;
if (avail < 0)
avail += rbuf->size;
return avail;
@@ -86,14 +86,15 @@ ssize_t dvb_ringbuffer_avail(struct dvb_ringbuffer *rbuf)

  void dvb_ringbuffer_flush(struct dvb_ringbuffer *rbuf)
  {
-   rbuf->pread = rbuf->pwrite;
+   smp_store_release(>pread, smp_load_acquire(>pwrite));
rbuf->error = 0;
  }
  EXPORT_SYMBOL(dvb_ringbuffer_flush);

  void dvb_ringbuffer_reset(struct dvb_ringbuffer *rbuf)
  {
-   rbuf->pread = rbuf->pwrite = 0;
+   smp_store_release(>pread, 0);
+   smp_store_release(>pwrite, 0);
rbuf->error = 0;
  }

@@ -119,12 +120,12 @@ ssize_t

Re: [PATCH] media: dvb_ringbuffer: Add memory barriers

2016-05-10 Thread Soeren Moch


On 05/07/16 15:26, Mauro Carvalho Chehab wrote:

Em Sat, 7 May 2016 10:22:35 -0300
Mauro Carvalho Chehab  escreveu:


Hi Soeren,

Em Sun, 7 Feb 2016 20:22:36 +0100
Soeren Moch  escreveu:


On 27.12.2015 21:41, Soeren Moch wrote:

Implement memory barriers according to Documentation/circular-buffers.txt:
- use smp_store_release() to update ringbuffer read/write pointers
- use smp_load_acquire() to load write pointer on reader side
- use ACCESS_ONCE() to load read pointer on writer side

This fixes data stream corruptions observed e.g. on an ARM Cortex-A9
quad core system with different types (PCI, USB) of DVB tuners.

Signed-off-by: Soeren Moch 
Cc: sta...@vger.kernel.org # 3.14+


Mauro,

any news or comments on this?
Since this is a real fix for broken behaviour, can you pick this up, please?


The problem here is that I'm very reluctant to touch at the DVB core
without doing some tests myself, as things like locking can be
very sensible.


In addition, it is good if other DVB developers could also test it.
Even being sent for some time, until now, nobody else tested it.


I know that people from the german vdrportal.de also use this patch
for quite some time now. Unfortunately they are not active on the
linux-media mailing list.



I'll try to find some time to take a look on it for Kernel 4.8,
but I'd like to reproduce the bug locally.

Could you please provide me enough info to reproduce it (and
eventually some test MPEG-TS where you know this would happen)?

I have two DekTek RF generators here, so I should be able to
play such TS and see what happens with and without the patch
on x86, arm32 and arm64.


Ah,  forgot to mention, but checkpatch.pl wants comments for the memory
barriers:



OK, when I wrote this patch (linux-4.4-rc6) checkpatch.pl did not
complain about missing comments. I will send a version 2 of this
patch to address these warnings.

Regards,
Soeren


WARNING: memory barrier without comment
#52: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:58:
+   return (rbuf->pread == smp_load_acquire(>pwrite));

WARNING: memory barrier without comment
#70: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:79:
+   avail = smp_load_acquire(>pwrite) - rbuf->pread;

WARNING: memory barrier without comment
#79: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:89:
+   smp_store_release(>pread, smp_load_acquire(>pwrite));

WARNING: memory barrier without comment
#87: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:96:
+   smp_store_release(>pread, 0);

WARNING: memory barrier without comment
#88: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:97:
+   smp_store_release(>pwrite, 0);

WARNING: memory barrier without comment
#97: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:123:
+   smp_store_release(>pread, 0);

WARNING: memory barrier without comment
#103: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:128:
+   smp_store_release(>pread, (rbuf->pread + todo) % rbuf->size);

WARNING: memory barrier without comment
#112: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:143:
+   smp_store_release(>pread, 0);

WARNING: memory barrier without comment
#117: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:147:
+   smp_store_release(>pread, (rbuf->pread + todo) % rbuf->size);

WARNING: memory barrier without comment
#126: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:162:
+   smp_store_release(>pwrite, 0);

WARNING: memory barrier without comment
#130: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:165:
+   smp_store_release(>pwrite, (rbuf->pwrite + todo) % rbuf->size);

WARNING: memory barrier without comment
#139: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:185:
+   smp_store_release(>pwrite, 0);

WARNING: memory barrier without comment
#145: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:190:
+   smp_store_release(>pwrite, (rbuf->pwrite + todo) % rbuf->size);

Thanks,
Mauro



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

Re: [PATCH] [media] v4l2-async: Pass the v4l2_async_subdev to the unbind callback

2016-05-10 Thread Javier Martinez Canillas

Hello Alban,

On 05/10/2016 09:19 AM, Alban Bedel wrote:
> v4l2_async_cleanup() is always called before before calling the
> unbind() callback. However v4l2_async_cleanup() clear the asd member,
> so when calling the unbind() callback the v4l2_async_subdev is always
> NULL. To fix this save the asd before calling v4l2_async_cleanup().
> 
> Signed-off-by: Alban Bedel 
> ---

Patch looks good to me. So after fixing the issues pointed out by Sakari:

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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] v4l2-async: Pass the v4l2_async_subdev to the unbind callback

2016-05-10 Thread Sakari Ailus

Hi Alban,

On Tue, May 10, 2016 at 03:19:14PM +0200, Alban Bedel wrote:
> v4l2_async_cleanup() is always called before before calling the

s/before //

> unbind() callback. However v4l2_async_cleanup() clear the asd member,

s/clear/clears/

> so when calling the unbind() callback the v4l2_async_subdev is always
> NULL. To fix this save the asd before calling v4l2_async_cleanup().

Oh dear...! How could this have happened? :-o

With the commit message changes,

Acked-by: Sakari Ailus 

> 
> Signed-off-by: Alban Bedel 
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index a4b224d..ceb28d4 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -220,6 +220,7 @@ void v4l2_async_notifier_unregister(struct 
> v4l2_async_notifier *notifier)
>   list_del(>list);
>  
>   list_for_each_entry_safe(sd, tmp, >done, async_list) {
> + struct v4l2_async_subdev *asd = sd->asd;
>   struct device *d;
>  
>   d = get_device(sd->dev);
> @@ -230,7 +231,7 @@ void v4l2_async_notifier_unregister(struct 
> v4l2_async_notifier *notifier)
>   device_release_driver(d);
>  
>   if (notifier->unbind)
> - notifier->unbind(notifier, sd, sd->asd);
> + notifier->unbind(notifier, sd, asd);
>  
>   /*
>* Store device at the device cache, in order to call
> @@ -313,6 +314,7 @@ EXPORT_SYMBOL(v4l2_async_register_subdev);
>  void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
>  {
>   struct v4l2_async_notifier *notifier = sd->notifier;
> + struct v4l2_async_subdev *asd = sd->asd;
>  
>   if (!sd->asd) {
>   if (!list_empty(>async_list))
> @@ -327,7 +329,7 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
>   v4l2_async_cleanup(sd);
>  
>   if (notifier->unbind)
> - notifier->unbind(notifier, sd, sd->asd);
> + notifier->unbind(notifier, sd, asd);
>  
>   mutex_unlock(_lock);
>  }

-- 
Kind regards,

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

[PATCH] [media] v4l2-async: Pass the v4l2_async_subdev to the unbind callback

2016-05-10 Thread Alban Bedel

v4l2_async_cleanup() is always called before before calling the
unbind() callback. However v4l2_async_cleanup() clear the asd member,
so when calling the unbind() callback the v4l2_async_subdev is always
NULL. To fix this save the asd before calling v4l2_async_cleanup().

Signed-off-by: Alban Bedel 
---
 drivers/media/v4l2-core/v4l2-async.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index a4b224d..ceb28d4 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -220,6 +220,7 @@ void v4l2_async_notifier_unregister(struct 
v4l2_async_notifier *notifier)
list_del(>list);
 
list_for_each_entry_safe(sd, tmp, >done, async_list) {
+   struct v4l2_async_subdev *asd = sd->asd;
struct device *d;
 
d = get_device(sd->dev);
@@ -230,7 +231,7 @@ void v4l2_async_notifier_unregister(struct 
v4l2_async_notifier *notifier)
device_release_driver(d);
 
if (notifier->unbind)
-   notifier->unbind(notifier, sd, sd->asd);
+   notifier->unbind(notifier, sd, asd);
 
/*
 * Store device at the device cache, in order to call
@@ -313,6 +314,7 @@ EXPORT_SYMBOL(v4l2_async_register_subdev);
 void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
 {
struct v4l2_async_notifier *notifier = sd->notifier;
+   struct v4l2_async_subdev *asd = sd->asd;
 
if (!sd->asd) {
if (!list_empty(>async_list))
@@ -327,7 +329,7 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
v4l2_async_cleanup(sd);
 
if (notifier->unbind)
-   notifier->unbind(notifier, sd, sd->asd);
+   notifier->unbind(notifier, sd, asd);
 
mutex_unlock(_lock);
 }
-- 
2.8.2

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

Re: [RFC PATCH 2/3] omap4: add CEC support

2016-05-10 Thread Tomi Valkeinen

On 10/05/16 15:26, Hans Verkuil wrote:

>>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>>> index 2bd9c83..1bb490f 100644
>>> --- a/arch/arm/boot/dts/omap4.dtsi
>>> +++ b/arch/arm/boot/dts/omap4.dtsi
>>> @@ -1006,8 +1006,9 @@
>>> reg = <0x58006000 0x200>,
>>>   <0x58006200 0x100>,
>>>   <0x58006300 0x100>,
>>> - <0x58006400 0x1000>;
>>> -   reg-names = "wp", "pll", "phy", "core";
>>> + <0x58006400 0x900>,
>>> + <0x58006D00 0x100>;
>>> +   reg-names = "wp", "pll", "phy", "core", "cec";
>>
>> "core" contains four blocks, all of which are currently included there
>> in the "core" space. I'm not sure why they weren't split up properly
>> when the driver was written, but I think we should either keep the core
>> as one big block, or split it up to those four sections, instead of just
>> separating the CEC block.
> 
> I don't entirely agree with that, partially because it would mean extra work 
> for
> me :-) and partially because CEC is different from the other blocks in that it
> is an optional HDMI feature.

I don't think it matters in this context if it's an optional HDMI
feature or not. This is about representing the HW memory areas, and I'd
like to keep it consistent, so either one big block, or if we want to
split it, split it up properly as shown in the TRM.

I'm fine with keeping one big "core" memory area there, that should work
fine for CEC, shouldn't it? And it would be the easiest option for you ;).

> 
>>
>>> interrupts = ;
>>> status = "disabled";
>>> ti,hwmods = "dss_hdmi";
>>> diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig 
>>> b/drivers/gpu/drm/omapdrm/dss/Kconfig
>>> index d1fa730..69638e9 100644
>>> --- a/drivers/gpu/drm/omapdrm/dss/Kconfig
>>> +++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
>>> @@ -71,9 +71,17 @@ config OMAP4_DSS_HDMI
>>> bool "HDMI support for OMAP4"
>>>  default y
>>> select OMAP2_DSS_HDMI_COMMON
>>> +   select MEDIA_CEC_EDID
>>
>> Hmm, what's in MEDIA_CEC_EDID, why does OMAP4 HDMI need to select that?
> 
> Helper functions that manipulate the physical address in an EDID. CEC may be
> optional, but the EDID isn't. These functions were just too big to make them
> static inlines, so instead it's a simple module.

Oh, I see, even if OMAP4's HDMI CEC is disabled, you use cec edid
functions in hdmi4.c.

>>> diff --git a/drivers/gpu/drm/omapdrm/dss/Makefile 
>>> b/drivers/gpu/drm/omapdrm/dss/Makefile
>>> index b651ec9..37eb597 100644
>>> --- a/drivers/gpu/drm/omapdrm/dss/Makefile
>>> +++ b/drivers/gpu/drm/omapdrm/dss/Makefile
>>> @@ -10,6 +10,9 @@ omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
>>>  omapdss-$(CONFIG_OMAP2_DSS_DSI) += dsi.o
>>>  omapdss-$(CONFIG_OMAP2_DSS_HDMI_COMMON) += hdmi_common.o hdmi_wp.o 
>>> hdmi_pll.o \
>>> hdmi_phy.o
>>> +ifeq ($(CONFIG_OMAP2_DSS_HDMI_CEC),y)
>>> +  omapdss-$(CONFIG_OMAP2_DSS_HDMI_COMMON) += hdmi_cec.o
>>> +endif
>>
>> The file should be hdmi4_cec.o, as it's for omap4. And why the ifeq?
>> Isn't just
>>
>> omapdss-$(OMAP4_DSS_HDMI_CEC) += hdmi4_cec.o
> 
> OMAP4_DSS_HDMI_CEC is a bool, not a tristate.

Yes, and that's fine. You're not compiling hdmi4_cec.o as a module,
you're compiling it into omapdss.o. Objects are added with "omapdss-y +=
xyz" to omapdss.o.

 Tomi



signature.asc
Description: OpenPGP digital signature

Re: [RFC PATCH 2/3] omap4: add CEC support

2016-05-10 Thread Hans Verkuil

Hi Tomi,

On 05/10/16 14:01, Tomi Valkeinen wrote:
> Hi Hans,
> 
> On 29/04/16 12:39, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  arch/arm/boot/dts/omap4-panda-a4.dts   |   2 +-
>>  arch/arm/boot/dts/omap4-panda-es.dts   |   2 +-
>>  arch/arm/boot/dts/omap4.dtsi   |   5 +-
>>  drivers/gpu/drm/omapdrm/dss/Kconfig|   8 +
>>  drivers/gpu/drm/omapdrm/dss/Makefile   |   3 +
>>  drivers/gpu/drm/omapdrm/dss/hdmi.h |  62 ++-
>>  drivers/gpu/drm/omapdrm/dss/hdmi4.c|  39 +++-
>>  drivers/gpu/drm/omapdrm/dss/hdmi_cec.c | 329 
>> +
>>  8 files changed, 441 insertions(+), 9 deletions(-)
>>  create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi_cec.c
> 
> First, thanks for working on this! It's really nice if we get CEC working.
> 
> Are you planning to continue working on this patch, or is this a
> proof-of-concept, and you want to move on to other things? I'm fine with
> both, the patch looks quite good and I'm sure I can continue from here
> if you want.

I am planning to continue work on this, but...

> Also, what's the status of the general CEC support, will these patches
> work on v4.7?

... I am waiting for the CEC framework to get merged. Possibly for 4.7, but more
likely for 4.8.

It will be staging initially so I have some more time to make API changes if
necessary.

These patches should work for 4.7.

> 
>> diff --git a/arch/arm/boot/dts/omap4-panda-a4.dts 
>> b/arch/arm/boot/dts/omap4-panda-a4.dts
>> index 78d3631..f0c1020 100644
>> --- a/arch/arm/boot/dts/omap4-panda-a4.dts
>> +++ b/arch/arm/boot/dts/omap4-panda-a4.dts
>> @@ -13,7 +13,7 @@
>>  /* Pandaboard Rev A4+ have external pullups on SCL & SDA */
>>  _hdmi_pins {
>>  pinctrl-single,pins = <
>> -OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)/* 
>> hdmi_cec.hdmi_cec */
>> +OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)   /* 
>> hdmi_cec.hdmi_cec */
> 
> Looks fine as we discussed, but these need to be split to separate patch
> (with explanation, of course =).
> 
>>  OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)   /* 
>> hdmi_scl.hdmi_scl */
>>  OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)   /* 
>> hdmi_sda.hdmi_sda */
>>  >;
>> diff --git a/arch/arm/boot/dts/omap4-panda-es.dts 
>> b/arch/arm/boot/dts/omap4-panda-es.dts
>> index 119f8e6..940fe4f 100644
>> --- a/arch/arm/boot/dts/omap4-panda-es.dts
>> +++ b/arch/arm/boot/dts/omap4-panda-es.dts
>> @@ -34,7 +34,7 @@
>>  /* PandaboardES has external pullups on SCL & SDA */
>>  _hdmi_pins {
>>  pinctrl-single,pins = <
>> -OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)/* 
>> hdmi_cec.hdmi_cec */
>> +OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)   /* 
>> hdmi_cec.hdmi_cec */
>>  OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)   /* 
>> hdmi_scl.hdmi_scl */
>>  OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)   /* 
>> hdmi_sda.hdmi_sda */
>>  >;
>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>> index 2bd9c83..1bb490f 100644
>> --- a/arch/arm/boot/dts/omap4.dtsi
>> +++ b/arch/arm/boot/dts/omap4.dtsi
>> @@ -1006,8 +1006,9 @@
>>  reg = <0x58006000 0x200>,
>><0x58006200 0x100>,
>><0x58006300 0x100>,
>> -  <0x58006400 0x1000>;
>> -reg-names = "wp", "pll", "phy", "core";
>> +  <0x58006400 0x900>,
>> +  <0x58006D00 0x100>;
>> +reg-names = "wp", "pll", "phy", "core", "cec";
> 
> "core" contains four blocks, all of which are currently included there
> in the "core" space. I'm not sure why they weren't split up properly
> when the driver was written, but I think we should either keep the core
> as one big block, or split it up to those four sections, instead of just
> separating the CEC block.

I don't entirely agree with that, partially because it would mean extra work for
me :-) and partially because CEC is different from the other blocks in that it
is an optional HDMI feature.

> 
>>  interrupts = ;
>>  status = "disabled";
>>  ti,hwmods = "dss_hdmi";
>> diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig 
>> b/drivers/gpu/drm/omapdrm/dss/Kconfig
>> index d1fa730..69638e9 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/Kconfig
>> +++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
>> @@ -71,9 +71,17 @@ config OMAP4_DSS_HDMI
>>  bool "HDMI support for OMAP4"
>>  default y
>>  select OMAP2_DSS_HDMI_COMMON
>> +select MEDIA_CEC_EDID
> 
> Hmm, what's in MEDIA_CEC_EDID, why does OMAP4 HDMI need to select that?

Helper functions that

Legitimate Loans

2016-05-10 Thread Financial Service

Do you need a loan? we lease loan to individuals,companies and business firm at 
a soft interest rate to those who are seriously in need of a loan.If you are in 
need of a loan contact us today.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/3] omap4: add CEC support

2016-05-10 Thread Tomi Valkeinen

Hi Hans,

On 29/04/16 12:39, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Signed-off-by: Hans Verkuil 
> ---
>  arch/arm/boot/dts/omap4-panda-a4.dts   |   2 +-
>  arch/arm/boot/dts/omap4-panda-es.dts   |   2 +-
>  arch/arm/boot/dts/omap4.dtsi   |   5 +-
>  drivers/gpu/drm/omapdrm/dss/Kconfig|   8 +
>  drivers/gpu/drm/omapdrm/dss/Makefile   |   3 +
>  drivers/gpu/drm/omapdrm/dss/hdmi.h |  62 ++-
>  drivers/gpu/drm/omapdrm/dss/hdmi4.c|  39 +++-
>  drivers/gpu/drm/omapdrm/dss/hdmi_cec.c | 329 
> +
>  8 files changed, 441 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi_cec.c

First, thanks for working on this! It's really nice if we get CEC working.

Are you planning to continue working on this patch, or is this a
proof-of-concept, and you want to move on to other things? I'm fine with
both, the patch looks quite good and I'm sure I can continue from here
if you want.

Also, what's the status of the general CEC support, will these patches
work on v4.7?

> diff --git a/arch/arm/boot/dts/omap4-panda-a4.dts 
> b/arch/arm/boot/dts/omap4-panda-a4.dts
> index 78d3631..f0c1020 100644
> --- a/arch/arm/boot/dts/omap4-panda-a4.dts
> +++ b/arch/arm/boot/dts/omap4-panda-a4.dts
> @@ -13,7 +13,7 @@
>  /* Pandaboard Rev A4+ have external pullups on SCL & SDA */
>  _hdmi_pins {
>   pinctrl-single,pins = <
> - OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)/* 
> hdmi_cec.hdmi_cec */
> + OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)   /* 
> hdmi_cec.hdmi_cec */

Looks fine as we discussed, but these need to be split to separate patch
(with explanation, of course =).

>   OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)   /* 
> hdmi_scl.hdmi_scl */
>   OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)   /* 
> hdmi_sda.hdmi_sda */
>   >;
> diff --git a/arch/arm/boot/dts/omap4-panda-es.dts 
> b/arch/arm/boot/dts/omap4-panda-es.dts
> index 119f8e6..940fe4f 100644
> --- a/arch/arm/boot/dts/omap4-panda-es.dts
> +++ b/arch/arm/boot/dts/omap4-panda-es.dts
> @@ -34,7 +34,7 @@
>  /* PandaboardES has external pullups on SCL & SDA */
>  _hdmi_pins {
>   pinctrl-single,pins = <
> - OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)/* 
> hdmi_cec.hdmi_cec */
> + OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)   /* 
> hdmi_cec.hdmi_cec */
>   OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)   /* 
> hdmi_scl.hdmi_scl */
>   OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)   /* 
> hdmi_sda.hdmi_sda */
>   >;
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 2bd9c83..1bb490f 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -1006,8 +1006,9 @@
>   reg = <0x58006000 0x200>,
> <0x58006200 0x100>,
> <0x58006300 0x100>,
> -   <0x58006400 0x1000>;
> - reg-names = "wp", "pll", "phy", "core";
> +   <0x58006400 0x900>,
> +   <0x58006D00 0x100>;
> + reg-names = "wp", "pll", "phy", "core", "cec";

"core" contains four blocks, all of which are currently included there
in the "core" space. I'm not sure why they weren't split up properly
when the driver was written, but I think we should either keep the core
as one big block, or split it up to those four sections, instead of just
separating the CEC block.

>   interrupts = ;
>   status = "disabled";
>   ti,hwmods = "dss_hdmi";
> diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig 
> b/drivers/gpu/drm/omapdrm/dss/Kconfig
> index d1fa730..69638e9 100644
> --- a/drivers/gpu/drm/omapdrm/dss/Kconfig
> +++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
> @@ -71,9 +71,17 @@ config OMAP4_DSS_HDMI
>   bool "HDMI support for OMAP4"
>  default y
>   select OMAP2_DSS_HDMI_COMMON
> + select MEDIA_CEC_EDID

Hmm, what's in MEDIA_CEC_EDID, why does OMAP4 HDMI need to select that?

>   help
> HDMI support for OMAP4 based SoCs.
>  
> +config OMAP2_DSS_HDMI_CEC

This should probably be OMAP2_DSS_HDMI4_CEC or such, as it's only for
OMAP4 HDMI. Or, following the omap4 hdmi's config name,
"OMAP4_DSS_HDMI_CEC".

> + bool "Enable HDMI CEC support for OMAP4"
> + depends on OMAP4_DSS_HDMI && MEDIA_CEC
> + default y
> + ---help---
> +   When selected the HDMI transmitter will support the CEC feature.
> +
>  config OMAP5_DSS_HDMI
>   bool "HDMI support for OMAP5"
>   default n
> diff --git a/drivers/gpu/drm/omapdrm/dss/Makefile 
> b/drivers/gpu/drm/omapdrm/dss/Makefile
> index

Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid

2016-05-10 Thread Tomi Valkeinen

Hi Hans,

On 29/04/16 12:39, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> As long as there is a valid physical address in the EDID and the omap
> CEC support is enabled, then we keep ls_oe_gpio on to ensure the CEC
> signal is passed through the tpd12s015.
> 
> Signed-off-by: Hans Verkuil 
> Suggested-by: Tomi Valkeinen 
> ---
>  drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c 
> b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> index 916a899..efbba23 100644
> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> @@ -16,6 +16,7 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  
> @@ -65,6 +66,7 @@ static void tpd_disconnect(struct omap_dss_device *dssdev,
>   return;
>  
>   gpiod_set_value_cansleep(ddata->ct_cp_hpd_gpio, 0);
> + gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
>  
>   dst->src = NULL;
>   dssdev->dst = NULL;
> @@ -142,6 +144,7 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
>  {
>   struct panel_drv_data *ddata = to_panel_data(dssdev);
>   struct omap_dss_device *in = ddata->in;
> + bool valid_phys_addr = 0;
>   int r;
>  
>   if (!gpiod_get_value_cansleep(ddata->hpd_gpio))
> @@ -151,7 +154,15 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
>  
>   r = in->ops.hdmi->read_edid(in, edid, len);
>  
> - gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
> +#ifdef CONFIG_OMAP2_DSS_HDMI_CEC
> + /*
> +  * In order to support CEC this pin should remain high
> +  * as long as the EDID has a valid physical address.
> +  */
> + valid_phys_addr =
> + cec_get_edid_phys_addr(edid, r, NULL) != CEC_PHYS_ADDR_INVALID;
> +#endif
> + gpiod_set_value_cansleep(ddata->ls_oe_gpio, valid_phys_addr);
>  
>   return r;
>  }

I think this works, but... Maybe it would be cleaner to have the LS_OE
enabled if a cable is connected. That's actually what we had earlier,
but I removed that due to a race issue:

a87a6d6b09de3118e5679c2057b99b7791b7673b ("OMAPDSS: encoder-tpd12s015:
Fix race issue with LS_OE"). Now, with CEC, there's need to have LS_OE
enabled even after reading the EDID, so I think it's better to go back
to the old model (after fixing the race issue, of course =).

 Tomi



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] [media] stk1160: Check *nplanes in queue_setup

2016-05-10 Thread Hans Verkuil

On 05/10/2016 05:06 AM, Helen Koike wrote:
> If *nplanes is not zero, it should use the requested size if valid

*nplanes is never 0 since the create_buffers ioctl isn't implemented in
this driver.

Adding support for it (simply use vb2_create_buffers) would make sense.

Regards,

Hans

> 
> Signed-off-by: Helen Koike 
> ---
>  drivers/media/usb/stk1160/stk1160-v4l.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c 
> b/drivers/media/usb/stk1160/stk1160-v4l.c
> index 77131fd..7ddbc02 100644
> --- a/drivers/media/usb/stk1160/stk1160-v4l.c
> +++ b/drivers/media/usb/stk1160/stk1160-v4l.c
> @@ -680,6 +680,9 @@ static int queue_setup(struct vb2_queue *vq,
>   *nbuffers = clamp_t(unsigned int, *nbuffers,
>   STK1160_MIN_VIDEO_BUFFERS, STK1160_MAX_VIDEO_BUFFERS);
>  
> + if (*nplanes)
> + return sizes[0] < size ? -EINVAL : 0;
> +
>   /* This means a packed colorformat */
>   *nplanes = 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: V4L2 SDR compliance test?

2016-05-10 Thread Ramesh Shanmugasundaram

Hi Mauro,

> Ramesh Shanmugasundaram  escreveu:
> 
> > Hi Mauro,
> >
> > Thanks for the clarifications.
> >
> > > Ramesh Shanmugasundaram 
> escreveu:
> > >
> > > > Hi Maintainers, All,
> > > >
> > > > Renesas R-Car SoCs have Digital Radio Interface (DRIF) controllers.
> > > > They are receive only SPI slave modules that support I2S format.
> > > > The targeted application of these controllers is SDR.
[...]
> > > If, for performance issues, it would require a faster I/O, then such
> > > tuner app should be converted to be a Kernel driver. Usually, it is
> > > not hard to convert those drivers to Kernelspace, as typically it is
> > > just a bunch of registers that should be set to make it to tune into
> > > an specific frequency and to adjust the tuner filters to the desired
> > > bandwidth and modulation type, in order to improve noise rejection.
> >
> > Yes, this is possible. However, this is Tuner chip vendor's decision
> (NDA, license issues etc.) and it is not in our control.
> 
> True, but an independent tuner driver very likely can be written with very
> little effort. All it requires is to monitor the traffic at the I2C bus
> and to write a driver that reproduces it, identifying what part of the I2C
> messages contain the frequency and enable/disable the filters.

Yes, this is possible. 

> 
> We have several such drivers in the Kernel, and that's the procedure used
> when the chipset vendor doesn't see the value of having his chipset used
> by a Linux-based device.
> 
> > As mentioned above, we can have the main SDR (DRIF) driver code to
> direct tuner subdev if present. However, when we want to upstream the DRIF
> driver we may not have a real Tuner driver/device to get the compliance
> test results. Should we run the compliance tests with a dummy stubbed
> tuner subdev?
> 
> No, an upstream requirement is to have everything tested on real hardware.
> What I recommend you is to either convince the tuner provider to send
> upstream a driver (or allow you to do so) or to switch to some other
> vendor whose driver is already in the Kernel or sees the value of having
> his chipset used on Linux.

Yep, that is a clear mandate. We shall talk to Tuner vendor and decide 
accordingly.

Thanks for your time and inputs,
Ramesh
--
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: OK

Re: [PATCH] media: dvb_ringbuffer: Add memory barriers

Re: [PATCH] media: dvb_ringbuffer: Add memory barriers

Re: [PATCH] [media] v4l2-async: Pass the v4l2_async_subdev to the unbind callback

Re: [PATCH] [media] v4l2-async: Pass the v4l2_async_subdev to the unbind callback

[PATCH] [media] v4l2-async: Pass the v4l2_async_subdev to the unbind callback

Re: [RFC PATCH 2/3] omap4: add CEC support

Re: [RFC PATCH 2/3] omap4: add CEC support

Legitimate Loans

Re: [RFC PATCH 2/3] omap4: add CEC support

Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid

Re: [PATCH] [media] stk1160: Check *nplanes in queue_setup

RE: V4L2 SDR compliance test?

13 matches

Site Navigation

Mail list logo

Footer information