Re: [PATCH v2 0/6] Fix tvp5150 regression with em28xx

[Mauro Carvalho Chehab]

Em Wed, 21 Dec 2016 09:11:36 -0500
Hi Devin,

Devin Heitmueller  escreveu:

> Hi Mauro,
> 
> > With that, I completed the tests on HVR-950. My tests covered:
> > - S-Video, Composite, TV
> > - 480i and 480p
> > - Closed Captions (with HVR-350 - it seems that MediaMVP doesn't
> >   produce NTSC CC).  
> 
> FYI:  the MediaMVP HD can be configured to output NTSC CC over VBI.
> If you want that functionality, I can dig out the script.  In fact
> I've got an alternate GUI which just plays a clip on boot and lets you
> select all the different resolutions/framerates available for
> composte/component/HDMI (for both PAL and NTSC) just by hitting
> buttons on the remote.  If you're interested, let me know and I'll dig
> it up.  It's a great tool to have, especially when doing work with
> HDMI where there are many more possible combinations to choose from.

Yeah, both things are interesting! My plan is to use MediaMVP as a
test device for analog TV, as it provides a way better output than
HVR-350 and it is easier to use.


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 v2 0/6] Fix tvp5150 regression with em28xx

[Devin Heitmueller]

Hi Mauro,

> With that, I completed the tests on HVR-950. My tests covered:
> - S-Video, Composite, TV
> - 480i and 480p
> - Closed Captions (with HVR-350 - it seems that MediaMVP doesn't
>   produce NTSC CC).

FYI:  the MediaMVP HD can be configured to output NTSC CC over VBI.
If you want that functionality, I can dig out the script.  In fact
I've got an alternate GUI which just plays a clip on boot and lets you
select all the different resolutions/framerates available for
composte/component/HDMI (for both PAL and NTSC) just by hitting
buttons on the remote.  If you're interested, let me know and I'll dig
it up.  It's a great tool to have, especially when doing work with
HDMI where there are many more possible combinations to choose from.

Devin

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

Re: [PATCH v2 0/6] Fix tvp5150 regression with em28xx

[Mauro Carvalho Chehab]

Em Mon, 12 Dec 2016 18:37:01 +0200
Laurent Pinchart  escreveu:

> Hello,
> 
> On Monday 12 Dec 2016 13:22:50 Javier Martinez Canillas wrote:
> > On 12/12/2016 06:51 AM, Mauro Carvalho Chehab wrote:  
> > > Em Fri,  9 Dec 2016 13:47:13 +0200 Laurent Pinchart escreveu:  
> > >> Hello,
> > >> 
> > >> This patch series fixes a regression reported by Devin Heitmueller that
> > >> affects a large number of em28xx. The problem was introduced by
> > >> 
> > >> commit 13d52fe40f1f7bbad49128e8ee6a2fe5e13dd18d
> > >> Author: Mauro Carvalho Chehab 
> > >> Date:   Tue Jan 26 06:59:39 2016 -0200
> > >> 
> > >> [media] em28xx: fix implementation of s_stream
> > >> 
> > >> that started calling s_stream(1) in the em28xx driver when enabling the
> > >> stream, resulting in the tvp5150 s_stream() operation writing several
> > >> registers with values fit for other platforms (namely OMAP3, possibly
> > >> others) but not for em28xx.
> > >> 
> > >> The series starts with two unrelated drive-by cleanups and an unrelated
> > >> bug fix. It then continues with a patch to remove an unneeded and armful
> > >> call to tvp5150_reset() when getting the format from the subdevice (4/6),
> > >> an update of an invalid comment and the addition of macros for register
> > >> bits in order to make the code more readable (5/6) and actually allow
> > >> following the incorrect code flow, and finally a rework of the
> > >> s_stream() operation to fix the problem.
> > >> 
> > >> Compared to v1,
> > >> 
> > >> - Patch 4/5 now calls tvp5150_reset() at probe time
> > >> - Patch 5/6 is fixed with an extra ~ removed
> > >> 
> > >> I haven't been able to test this with an em28xx device as I don't own any
> > >> that contains a tvp5150, but Mauro reported that the series fixes the
> > >> issue with his device.
> > >> 
> > >> I also haven't been able to test anything on an OMAP3 platform, as the
> > >> tvp5150 driver go broken on DT-based systems by  
> > > 
> > > I applied today patches 1 to 3, as I don't see any risk of regressions
> > > there. Stable was c/c on patch 3.
> > > 
> > > I want to do more tests on patches 4-6, with both progressive video and
> > > RF. It would also be nice if someone could test it on OMAP3, to be sure
> > > that no regressions happened there.  
> > 
> > I've tested patches 4-6 on a IGEPv2 and video capture is still working for
> > both composite input AIP1A (after changing the hardcoded selected input)
> > and AIP1B.
> > 
> > The patches also look good to me, so please feel free to add my Reviewed-by
> > and Tested-by tags on these.
> > 
> > I wasn't able to test S-Video since my S-Video source broke (an old DVD
> > player) but this never worked for me anyways with this board.  
> 
> I've tested the patches too, in composite mode only as my hardware has a 
> single input. The image quality isn't very good, but I believe that's due to 
> my source. It shouldn't be related to this patch series at least.

Yesterday, I was able to make my device that generates 480p to work again,
and bought a RF modulator.

I used HVR-350 and Hauppauge MediaMVP as image sources producing NTSC output,
and Kernel 4.9 + media patches for 4.10 + tvp5150 v2 patch series.

With that, I completed the tests on HVR-950. My tests covered:
- S-Video, Composite, TV
- 480i and 480p
- Closed Captions (with HVR-350 - it seems that MediaMVP doesn't
  produce NTSC CC).

PS.: I did some tests with PAL output too, with HVR-350.

> I tried both BT.656 and parallel bus modes. The latter didn't work properly, 
> but it wasn't supported when I worked on TVP5151 + OMAP3 support in the first 
> place anyway, so it's not a regression, just something to eventually fix (if 
> I 
> have too much free time).

With that, it seems that BT.656 is working fine. So, I'm merging the
patches and will send them on the next pull request.

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 v2 0/6] Fix tvp5150 regression with em28xx

[Laurent Pinchart]

Hello,

On Monday 12 Dec 2016 13:22:50 Javier Martinez Canillas wrote:
> On 12/12/2016 06:51 AM, Mauro Carvalho Chehab wrote:
> > Em Fri,  9 Dec 2016 13:47:13 +0200 Laurent Pinchart escreveu:
> >> Hello,
> >> 
> >> This patch series fixes a regression reported by Devin Heitmueller that
> >> affects a large number of em28xx. The problem was introduced by
> >> 
> >> commit 13d52fe40f1f7bbad49128e8ee6a2fe5e13dd18d
> >> Author: Mauro Carvalho Chehab 
> >> Date:   Tue Jan 26 06:59:39 2016 -0200
> >> 
> >> [media] em28xx: fix implementation of s_stream
> >> 
> >> that started calling s_stream(1) in the em28xx driver when enabling the
> >> stream, resulting in the tvp5150 s_stream() operation writing several
> >> registers with values fit for other platforms (namely OMAP3, possibly
> >> others) but not for em28xx.
> >> 
> >> The series starts with two unrelated drive-by cleanups and an unrelated
> >> bug fix. It then continues with a patch to remove an unneeded and armful
> >> call to tvp5150_reset() when getting the format from the subdevice (4/6),
> >> an update of an invalid comment and the addition of macros for register
> >> bits in order to make the code more readable (5/6) and actually allow
> >> following the incorrect code flow, and finally a rework of the
> >> s_stream() operation to fix the problem.
> >> 
> >> Compared to v1,
> >> 
> >> - Patch 4/5 now calls tvp5150_reset() at probe time
> >> - Patch 5/6 is fixed with an extra ~ removed
> >> 
> >> I haven't been able to test this with an em28xx device as I don't own any
> >> that contains a tvp5150, but Mauro reported that the series fixes the
> >> issue with his device.
> >> 
> >> I also haven't been able to test anything on an OMAP3 platform, as the
> >> tvp5150 driver go broken on DT-based systems by
> > 
> > I applied today patches 1 to 3, as I don't see any risk of regressions
> > there. Stable was c/c on patch 3.
> > 
> > I want to do more tests on patches 4-6, with both progressive video and
> > RF. It would also be nice if someone could test it on OMAP3, to be sure
> > that no regressions happened there.
> 
> I've tested patches 4-6 on a IGEPv2 and video capture is still working for
> both composite input AIP1A (after changing the hardcoded selected input)
> and AIP1B.
> 
> The patches also look good to me, so please feel free to add my Reviewed-by
> and Tested-by tags on these.
> 
> I wasn't able to test S-Video since my S-Video source broke (an old DVD
> player) but this never worked for me anyways with this board.

I've tested the patches too, in composite mode only as my hardware has a 
single input. The image quality isn't very good, but I believe that's due to 
my source. It shouldn't be related to this patch series at least.

I tried both BT.656 and parallel bus modes. The latter didn't work properly, 
but it wasn't supported when I worked on TVP5151 + OMAP3 support in the first 
place anyway, so it's not a regression, just something to eventually fix (if I 
have too much free time).

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v2 0/6] Fix tvp5150 regression with em28xx

[Javier Martinez Canillas]

Hello Mauro,

On 12/12/2016 06:51 AM, Mauro Carvalho Chehab wrote:
> Hi Laurent,
> 
> Em Fri,  9 Dec 2016 13:47:13 +0200
> Laurent Pinchart  escreveu:
> 
>> Hello,
>>
>> This patch series fixes a regression reported by Devin Heitmueller that
>> affects a large number of em28xx. The problem was introduced by
>>
>> commit 13d52fe40f1f7bbad49128e8ee6a2fe5e13dd18d
>> Author: Mauro Carvalho Chehab 
>> Date:   Tue Jan 26 06:59:39 2016 -0200
>>
>> [media] em28xx: fix implementation of s_stream
>>
>> that started calling s_stream(1) in the em28xx driver when enabling the
>> stream, resulting in the tvp5150 s_stream() operation writing several
>> registers with values fit for other platforms (namely OMAP3, possibly others)
>> but not for em28xx.
>>
>> The series starts with two unrelated drive-by cleanups and an unrelated bug
>> fix. It then continues with a patch to remove an unneeded and armful call to
>> tvp5150_reset() when getting the format from the subdevice (4/6), an update 
>> of
>> an invalid comment and the addition of macros for register bits in order to
>> make the code more readable (5/6) and actually allow following the incorrect
>> code flow, and finally a rework of the s_stream() operation to fix the
>> problem.
>>
>> Compared to v1,
>>
>> - Patch 4/5 now calls tvp5150_reset() at probe time
>> - Patch 5/6 is fixed with an extra ~ removed
>>
>> I haven't been able to test this with an em28xx device as I don't own any 
>> that
>> contains a tvp5150, but Mauro reported that the series fixes the issue with
>> his device.
>>
>> I also haven't been able to test anything on an OMAP3 platform, as the 
>> tvp5150
>> driver go broken on DT-based systems by
> 
> I applied today patches 1 to 3, as I don't see any risk of regressions
> there. Stable was c/c on patch 3.
> 
> I want to do more tests on patches 4-6, with both progressive video and RF. 
> It would also be nice if someone could test it on OMAP3, to be sure that no
> regressions happened there.
>

I've tested patches 4-6 on a IGEPv2 and video capture is still working for both
composite input AIP1A (after changing the hardcoded selected input) and AIP1B.

The patches also look good to me, so please feel free to add my Reviewed-by and
Tested-by tags on these.

I wasn't able to test S-Video since my S-Video source broke (an old DVD player)
but this never worked for me anyways with this board.

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 v2 0/6] Fix tvp5150 regression with em28xx

[Mauro Carvalho Chehab]

Hi Laurent,

Em Fri,  9 Dec 2016 13:47:13 +0200
Laurent Pinchart  escreveu:

> Hello,
> 
> This patch series fixes a regression reported by Devin Heitmueller that
> affects a large number of em28xx. The problem was introduced by
> 
> commit 13d52fe40f1f7bbad49128e8ee6a2fe5e13dd18d
> Author: Mauro Carvalho Chehab 
> Date:   Tue Jan 26 06:59:39 2016 -0200
> 
> [media] em28xx: fix implementation of s_stream
> 
> that started calling s_stream(1) in the em28xx driver when enabling the
> stream, resulting in the tvp5150 s_stream() operation writing several
> registers with values fit for other platforms (namely OMAP3, possibly others)
> but not for em28xx.
> 
> The series starts with two unrelated drive-by cleanups and an unrelated bug
> fix. It then continues with a patch to remove an unneeded and armful call to
> tvp5150_reset() when getting the format from the subdevice (4/6), an update of
> an invalid comment and the addition of macros for register bits in order to
> make the code more readable (5/6) and actually allow following the incorrect
> code flow, and finally a rework of the s_stream() operation to fix the
> problem.
> 
> Compared to v1,
> 
> - Patch 4/5 now calls tvp5150_reset() at probe time
> - Patch 5/6 is fixed with an extra ~ removed
> 
> I haven't been able to test this with an em28xx device as I don't own any that
> contains a tvp5150, but Mauro reported that the series fixes the issue with
> his device.
> 
> I also haven't been able to test anything on an OMAP3 platform, as the tvp5150
> driver go broken on DT-based systems by

I applied today patches 1 to 3, as I don't see any risk of regressions
there. Stable was c/c on patch 3.

I want to do more tests on patches 4-6, with both progressive video and RF. 
It would also be nice if someone could test it on OMAP3, to be sure that no
regressions happened there.

So, my goal is to send those patches for -rc2, assuming that we can do
such tests, as it is likely too late for -rc1, as we'll have a short merge
window this time.

Regards,
Mauro

> commit f7b4b54e63643b740c598e044874c4bffa0f04f2
> Author: Javier Martinez Canillas 
> Date:   Fri Feb 5 17:09:58 2016 -0200
> 
> [media] tvp5150: add HW input connectors support
> 
> Fixing it will be the topic of another patch series.
> 
> Laurent Pinchart (6):
>   v4l: tvp5150: Compile tvp5150_link_setup out if
> !CONFIG_MEDIA_CONTROLLER
>   v4l: tvp5150: Don't inline the tvp5150_selmux() function
>   v4l: tvp5150: Add missing break in set control handler
>   v4l: tvp5150: Reset device at probe time, not in get/set format
> handlers
>   v4l: tvp5150: Fix comment regarding output pin muxing
>   v4l: tvp5150: Don't override output pinmuxing at stream on/off time
> 
>  drivers/media/i2c/tvp5150.c | 63 
> +
>  drivers/media/i2c/tvp5150_reg.h |  9 ++
>  2 files changed, 48 insertions(+), 24 deletions(-)
> 



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

[PATCH v2 0/6] Fix tvp5150 regression with em28xx

[Laurent Pinchart]

Hello,

This patch series fixes a regression reported by Devin Heitmueller that
affects a large number of em28xx. The problem was introduced by

commit 13d52fe40f1f7bbad49128e8ee6a2fe5e13dd18d
Author: Mauro Carvalho Chehab 
Date:   Tue Jan 26 06:59:39 2016 -0200

[media] em28xx: fix implementation of s_stream

that started calling s_stream(1) in the em28xx driver when enabling the
stream, resulting in the tvp5150 s_stream() operation writing several
registers with values fit for other platforms (namely OMAP3, possibly others)
but not for em28xx.

The series starts with two unrelated drive-by cleanups and an unrelated bug
fix. It then continues with a patch to remove an unneeded and armful call to
tvp5150_reset() when getting the format from the subdevice (4/6), an update of
an invalid comment and the addition of macros for register bits in order to
make the code more readable (5/6) and actually allow following the incorrect
code flow, and finally a rework of the s_stream() operation to fix the
problem.

Compared to v1,

- Patch 4/5 now calls tvp5150_reset() at probe time
- Patch 5/6 is fixed with an extra ~ removed

I haven't been able to test this with an em28xx device as I don't own any that
contains a tvp5150, but Mauro reported that the series fixes the issue with
his device.

I also haven't been able to test anything on an OMAP3 platform, as the tvp5150
driver go broken on DT-based systems by

commit f7b4b54e63643b740c598e044874c4bffa0f04f2
Author: Javier Martinez Canillas 
Date:   Fri Feb 5 17:09:58 2016 -0200

[media] tvp5150: add HW input connectors support

Fixing it will be the topic of another patch series.

Laurent Pinchart (6):
  v4l: tvp5150: Compile tvp5150_link_setup out if
!CONFIG_MEDIA_CONTROLLER
  v4l: tvp5150: Don't inline the tvp5150_selmux() function
  v4l: tvp5150: Add missing break in set control handler
  v4l: tvp5150: Reset device at probe time, not in get/set format
handlers
  v4l: tvp5150: Fix comment regarding output pin muxing
  v4l: tvp5150: Don't override output pinmuxing at stream on/off time

 drivers/media/i2c/tvp5150.c | 63 +
 drivers/media/i2c/tvp5150_reg.h |  9 ++
 2 files changed, 48 insertions(+), 24 deletions(-)

-- 
Regards,

Laurent Pinchart

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