[PATCH] Fix colorspace on tm6010

[PATCH] Fix colorspace on tm6010

[Mauro Carvalho Chehab]

The enclosed patch fixes the color format on tm6010. What happened is that the patch
adding fourcc control on tm6010 had one small cut-and-paste trouble: it was
changing the wrong register ;)

I've fixed it. So, now, colors are working fine. I'll be applying it at my git. This
way, the current git contains a tm6000 code that is not so bad.

With this patch, analog video on tm6000/tm6010 are working again (but see the
patch comments bellow). Yet, there are a large number of TODO items for this driver:
- Fix the loss of some blocks when receiving the URB's;
- Add a lock at tm6000_read_write_usb() to prevent two simultaneous access to the
URB control transfers;
- Properly add the locks at tm6000-video;
- Add audio support;
- Add IR support;
- Do several cleanups;
- I think that frame1/frame0 are inverted. This causes a funny effect at the image.
  the fix is trivial, but require some tests.
- My tm6010 devices sometimes insist on stop working. I need to turn them off, removing
  from my machine and wait for a while for it to work again. I'm starting to think that
  it is an overheat issue;
- Sometimes, tm6010 doesn't read eeprom at the proper time (hardware bug). So, the device 
  got miss-detected as a "generic" tm6000. This can be really bad if the tuner is the 
  Low Power one, as it may result on loading the high power firmware, that could damage 
  the device. Maybe we may read eeprom to double check, when the device is marked as "generic".
- Coding Style fixes;

I'll be committing a patch with the above TODO items at tm6000/README

(Bee/Stefan/Dmitri, feel free to add more things at the todo - We need to write a README file

The lack of locks still generate some OOPS'es, but I was not able of get any Panic. So,
I'll likely add it at upstream drivers/staging at the next merge window.

-- 

Cheers,
Mauro


commit c621ed883a26dc705c38ad698f6a19a6260f172f
Author: Mauro Carvalho Chehab <mchehab@redhat.com>
Date:   Mon May 3 04:25:59 2010 -0300

    V4L/DVB: Fix color format with tm6010
    
    The values for the fourcc format were correct, but applied to the
    wrong register. With this change, video is now barely working again with
    tm6000.
    
    While here, let's remove, for now, the memset. This way, people can
    have some image when testing this device.
    
    Yet to be fixed: parts of the image frame are missed. As we don't clean
    the buffers anymore, this is "recovered" by repeating the values from a
    previous frame. The quality is bad, since the image pixels will contain
    data from some previous frames, generating weird delay artifacts.
    
    Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/staging/tm6000/tm6000-core.c b/drivers/staging/tm6000/tm6000-core.c
index 860553f..bfbc53b 100644
--- a/drivers/staging/tm6000/tm6000-core.c
+++ b/drivers/staging/tm6000/tm6000-core.c
@@ -156,10 +156,13 @@ int tm6000_get_reg32 (struct tm6000_core *dev, u8 req, u16 value, u16 index)
 void tm6000_set_fourcc_format(struct tm6000_core *dev)
 {
 	if (dev->dev_type == TM6010) {
+		int val;
+
+		val = tm6000_get_reg(dev, TM6010_REQ07_RCC_ACTIVE_VIDEO_IF, 0) & 0xfc;
 		if (dev->fourcc == V4L2_PIX_FMT_UYVY)
-			tm6000_set_reg(dev, TM6010_REQ07_RC1_TRESHOLD, 0xd0);
+			tm6000_set_reg(dev, TM6010_REQ07_RCC_ACTIVE_VIDEO_IF, val);
 		else
-			tm6000_set_reg(dev, TM6010_REQ07_RC1_TRESHOLD, 0x90);
+			tm6000_set_reg(dev, TM6010_REQ07_RCC_ACTIVE_VIDEO_IF, val | 1);
 	} else {
 		if (dev->fourcc == V4L2_PIX_FMT_UYVY)
 			tm6000_set_reg(dev, TM6010_REQ07_RC1_TRESHOLD, 0xd0);
diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c
index 4444487..9554472 100644
--- a/drivers/staging/tm6000/tm6000-video.c
+++ b/drivers/staging/tm6000/tm6000-video.c
@@ -149,8 +149,8 @@ static inline void get_next_buf(struct tm6000_dmaqueue *dma_q,
 
 	/* Cleans up buffer - Usefull for testing for frame/URB loss */
 	outp = videobuf_to_vmalloc(&(*buf)->vb);
-	if (outp)
-		memset(outp, 0, (*buf)->vb.size);
+//	if (outp)
+//		memset(outp, 0, (*buf)->vb.size);
 
 	return;
 }

Re: [PATCH] Fix colorspace on tm6010

[Bee Hock Goh]

lot of good changes to tm6000. Unfortunately, I am not able to test
any of this at the moment. Git not working for me anymore as 2.6.33
insist to freeze my machine on boot.

Reverting to hg does not work as well after my upgrade to lucid. :)

Apparently, its now complain about invalid module format.

if everything work out again, I would like to try and get the audio working.


On Mon, May 3, 2010 at 3:39 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> The enclosed patch fixes the color format on tm6010. What happened is that the patch
> adding fourcc control on tm6010 had one small cut-and-paste trouble: it was
> changing the wrong register ;)
>
> I've fixed it. So, now, colors are working fine. I'll be applying it at my git. This
> way, the current git contains a tm6000 code that is not so bad.
>
> With this patch, analog video on tm6000/tm6010 are working again (but see the
> patch comments bellow). Yet, there are a large number of TODO items for this driver:
> - Fix the loss of some blocks when receiving the URB's;
> - Add a lock at tm6000_read_write_usb() to prevent two simultaneous access to the
> URB control transfers;
> - Properly add the locks at tm6000-video;
> - Add audio support;
> - Add IR support;
> - Do several cleanups;
> - I think that frame1/frame0 are inverted. This causes a funny effect at the image.
>  the fix is trivial, but require some tests.
> - My tm6010 devices sometimes insist on stop working. I need to turn them off, removing
>  from my machine and wait for a while for it to work again. I'm starting to think that
>  it is an overheat issue;
> - Sometimes, tm6010 doesn't read eeprom at the proper time (hardware bug). So, the device
>  got miss-detected as a "generic" tm6000. This can be really bad if the tuner is the
>  Low Power one, as it may result on loading the high power firmware, that could damage
>  the device. Maybe we may read eeprom to double check, when the device is marked as "generic".
> - Coding Style fixes;
>
> I'll be committing a patch with the above TODO items at tm6000/README
>
> (Bee/Stefan/Dmitri, feel free to add more things at the todo - We need to write a README file
>
> The lack of locks still generate some OOPS'es, but I was not able of get any Panic. So,
> I'll likely add it at upstream drivers/staging at the next merge window.
>
> --
>
> Cheers,
> Mauro
>
>
> commit c621ed883a26dc705c38ad698f6a19a6260f172f
> Author: Mauro Carvalho Chehab <mchehab@redhat.com>
> Date:   Mon May 3 04:25:59 2010 -0300
>
>    V4L/DVB: Fix color format with tm6010
>
>    The values for the fourcc format were correct, but applied to the
>    wrong register. With this change, video is now barely working again with
>    tm6000.
>
>    While here, let's remove, for now, the memset. This way, people can
>    have some image when testing this device.
>
>    Yet to be fixed: parts of the image frame are missed. As we don't clean
>    the buffers anymore, this is "recovered" by repeating the values from a
>    previous frame. The quality is bad, since the image pixels will contain
>    data from some previous frames, generating weird delay artifacts.
>
>    Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
>
> diff --git a/drivers/staging/tm6000/tm6000-core.c b/drivers/staging/tm6000/tm6000-core.c
> index 860553f..bfbc53b 100644
> --- a/drivers/staging/tm6000/tm6000-core.c
> +++ b/drivers/staging/tm6000/tm6000-core.c
> @@ -156,10 +156,13 @@ int tm6000_get_reg32 (struct tm6000_core *dev, u8 req, u16 value, u16 index)
>  void tm6000_set_fourcc_format(struct tm6000_core *dev)
>  {
>        if (dev->dev_type == TM6010) {
> +               int val;
> +
> +               val = tm6000_get_reg(dev, TM6010_REQ07_RCC_ACTIVE_VIDEO_IF, 0) & 0xfc;
>                if (dev->fourcc == V4L2_PIX_FMT_UYVY)
> -                       tm6000_set_reg(dev, TM6010_REQ07_RC1_TRESHOLD, 0xd0);
> +                       tm6000_set_reg(dev, TM6010_REQ07_RCC_ACTIVE_VIDEO_IF, val);
>                else
> -                       tm6000_set_reg(dev, TM6010_REQ07_RC1_TRESHOLD, 0x90);
> +                       tm6000_set_reg(dev, TM6010_REQ07_RCC_ACTIVE_VIDEO_IF, val | 1);
>        } else {
>                if (dev->fourcc == V4L2_PIX_FMT_UYVY)
>                        tm6000_set_reg(dev, TM6010_REQ07_RC1_TRESHOLD, 0xd0);
> diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c
> index 4444487..9554472 100644
> --- a/drivers/staging/tm6000/tm6000-video.c
> +++ b/drivers/staging/tm6000/tm6000-video.c
> @@ -149,8 +149,8 @@ static inline void get_next_buf(struct tm6000_dmaqueue *dma_q,
>
>        /* Cleans up buffer - Usefull for testing for frame/URB loss */
>        outp = videobuf_to_vmalloc(&(*buf)->vb);
> -       if (outp)
> -               memset(outp, 0, (*buf)->vb.size);
> +//     if (outp)
> +//             memset(outp, 0, (*buf)->vb.size);
>
>        return;
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] Fix colorspace on tm6010

[Mauro Carvalho Chehab]

Bee Hock Goh wrote:
> lot of good changes to tm6000. Unfortunately, I am not able to test
> any of this at the moment. Git not working for me anymore as 2.6.33
> insist to freeze my machine on boot.
> 
> Reverting to hg does not work as well after my upgrade to lucid. :)
> 
> Apparently, its now complain about invalid module format.
> 
> if everything work out again, I would like to try and get the audio working.
> 

Due to your last email, I suspect you already found a solution to make your
distro to work again.

Audio should be trivial: just finish writing the tm6000-alsa logic
(currently, it is just an skeleton), based on snd-usb-audio (you might also
use the em28xx module as reference, but snd-usb-audio should be the primary
one, since it is maintained by alsa guys, so, in thesis, it probably use better
the alsa API). I bet that just copying the values of the alsa stream to the
audio channels will be enough for having audio.

The thing is: before going to audio, we need to be sure that we're not loosing
anything at copy_streams logic, since, while it is not that bad to loose a video
packet (as we may just repeat the last frame, as the current logic), but this
doesn't work with audio.

-- 

Cheers,
Mauro

Re: [PATCH] Fix colorspace on tm6010

[Bee Hock Goh]

Its too much time to fix it so I simply reinstall the OS but I will be
working with hg tree for the time being.

True but having the audio will probably help to ascertain how bad the
frame loss is. And also writing the audio module will be trivial to
you but it will take some time for me.


On Tue, May 4, 2010 at 7:47 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Bee Hock Goh wrote:
>> lot of good changes to tm6000. Unfortunately, I am not able to test
>> any of this at the moment. Git not working for me anymore as 2.6.33
>> insist to freeze my machine on boot.
>>
>> Reverting to hg does not work as well after my upgrade to lucid. :)
>>
>> Apparently, its now complain about invalid module format.
>>
>> if everything work out again, I would like to try and get the audio working.
>>
>
> Due to your last email, I suspect you already found a solution to make your
> distro to work again.
>
> Audio should be trivial: just finish writing the tm6000-alsa logic
> (currently, it is just an skeleton), based on snd-usb-audio (you might also
> use the em28xx module as reference, but snd-usb-audio should be the primary
> one, since it is maintained by alsa guys, so, in thesis, it probably use better
> the alsa API). I bet that just copying the values of the alsa stream to the
> audio channels will be enough for having audio.
>
> The thing is: before going to audio, we need to be sure that we're not loosing
> anything at copy_streams logic, since, while it is not that bad to loose a video
> packet (as we may just repeat the last frame, as the current logic), but this
> doesn't work with audio.
>
> --
>
> Cheers,
> Mauro
>

Re: [PATCH] Fix colorspace on tm6010

[Mauro Carvalho Chehab]

Bee Hock Goh wrote:
> Its too much time to fix it so I simply reinstall the OS but I will be
> working with hg tree for the time being.
> 
> True but having the audio will probably help to ascertain how bad the
> frame loss is. And also writing the audio module will be trivial to
> you but it will take some time for me.

I never said it is fast ;) The first alsa driver at v4l-dvb took a long time,
but, after having the first, the others are (almost) cut-and-paste stuff,
plus some time to debug.

-- 

Cheers,
Mauro