* [PATCH 1/2] mt9v022: fix pixel clock
@ 2011-04-06 14:01 Teresa Gámez
2011-04-06 14:01 ` [PATCH 2/2] mt9m111: " Teresa Gámez
2011-04-07 11:08 ` [PATCH 1/2] mt9v022: " Guennadi Liakhovetski
0 siblings, 2 replies; 12+ messages in thread
From: Teresa Gámez @ 2011-04-06 14:01 UTC (permalink / raw)
To: linux-media; +Cc: Teresa Gámez
Measurements show that the setup of the pixel clock is not correct.
The 'Invert Pixel Clock' bit has to be set to 1 for falling edge
and not for rising.
Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
---
drivers/media/video/mt9v022.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/mt9v022.c b/drivers/media/video/mt9v022.c
index 6a784c8..dec2a69 100644
--- a/drivers/media/video/mt9v022.c
+++ b/drivers/media/video/mt9v022.c
@@ -228,7 +228,7 @@ static int mt9v022_set_bus_param(struct soc_camera_device *icd,
flags = soc_camera_apply_sensor_flags(icl, flags);
- if (flags & SOCAM_PCLK_SAMPLE_RISING)
+ if (flags & SOCAM_PCLK_SAMPLE_FALLING)
pixclk |= 0x10;
if (!(flags & SOCAM_HSYNC_ACTIVE_HIGH))
--
1.7.0.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] mt9m111: fix pixel clock
2011-04-06 14:01 [PATCH 1/2] mt9v022: fix pixel clock Teresa Gámez
@ 2011-04-06 14:01 ` Teresa Gámez
2011-04-07 11:25 ` Guennadi Liakhovetski
2011-05-18 11:12 ` Guennadi Liakhovetski
2011-04-07 11:08 ` [PATCH 1/2] mt9v022: " Guennadi Liakhovetski
1 sibling, 2 replies; 12+ messages in thread
From: Teresa Gámez @ 2011-04-06 14:01 UTC (permalink / raw)
To: linux-media; +Cc: Teresa Gámez
This camera driver supports only rising edge, which is the default
setting of the device. The function mt9m111_setup_pixfmt() overwrites
this setting. So the driver actually uses falling edge.
This patch corrects that.
Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
---
drivers/media/video/mt9m111.c | 14 ++++++++++++--
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 53fa2a7..4040a96 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -315,10 +315,20 @@ static int mt9m111_setup_rect(struct i2c_client *client,
static int mt9m111_setup_pixfmt(struct i2c_client *client, u16 outfmt)
{
int ret;
+ u16 mask = MT9M111_OUTFMT_PROCESSED_BAYER | MT9M111_OUTFMT_RGB |
+ MT9M111_OUTFMT_BYPASS_IFP | MT9M111_OUTFMT_SWAP_RGB_EVEN |
+ MT9M111_OUTFMT_RGB565 | MT9M111_OUTFMT_RGB555 |
+ MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr |
+ MT9M111_OUTFMT_SWAP_YCbCr_C_Y;
- ret = reg_write(OUTPUT_FORMAT_CTRL2_A, outfmt);
+ ret = reg_clear(OUTPUT_FORMAT_CTRL2_A, mask);
if (!ret)
- ret = reg_write(OUTPUT_FORMAT_CTRL2_B, outfmt);
+ ret = reg_set(OUTPUT_FORMAT_CTRL2_A, outfmt);
+ if (!ret)
+ ret = reg_clear(OUTPUT_FORMAT_CTRL2_B, mask);
+ if (!ret)
+ ret = reg_set(OUTPUT_FORMAT_CTRL2_B, outfmt);
+
return ret;
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] mt9v022: fix pixel clock
2011-04-06 14:01 [PATCH 1/2] mt9v022: fix pixel clock Teresa Gámez
2011-04-06 14:01 ` [PATCH 2/2] mt9m111: " Teresa Gámez
@ 2011-04-07 11:08 ` Guennadi Liakhovetski
2011-04-07 12:38 ` Teresa Gamez
[not found] ` <OF0E7310A6.B4F9559D-ONC125786B.003E2F29-C125786B.004202D7@phytec.de>
1 sibling, 2 replies; 12+ messages in thread
From: Guennadi Liakhovetski @ 2011-04-07 11:08 UTC (permalink / raw)
To: Teresa Gámez; +Cc: linux-media
On Wed, 6 Apr 2011, Teresa Gámez wrote:
> Measurements show that the setup of the pixel clock is not correct.
> The 'Invert Pixel Clock' bit has to be set to 1 for falling edge
> and not for rising.
Doesn't seem correct to me. The mt9v022 datasheet says:
<quote>
Invert pixel clock. When set, LINE_VALID,
FRAME_VALID, and DOUT is set up to the rising edge
of pixel clock, PIXCLK. When clear, they are set up to
the falling edge of PIXCLK.
</quote>
and this works for present mt9v022 configurations, which include at least
two boards: PXA270-based arch/arm/mach-pxa/pcm990-baseboard.c and i.MX31
based arch/arm/mach-mx3/mach-pcm037.c. If this is different for your
board, maybe you have to set the SOCAM_SENSOR_INVERT_PCLK flag in your
"struct soc_camera_link" instance.
Thanks
Guennadi
> Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
> ---
> drivers/media/video/mt9v022.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/video/mt9v022.c b/drivers/media/video/mt9v022.c
> index 6a784c8..dec2a69 100644
> --- a/drivers/media/video/mt9v022.c
> +++ b/drivers/media/video/mt9v022.c
> @@ -228,7 +228,7 @@ static int mt9v022_set_bus_param(struct soc_camera_device *icd,
>
> flags = soc_camera_apply_sensor_flags(icl, flags);
>
> - if (flags & SOCAM_PCLK_SAMPLE_RISING)
> + if (flags & SOCAM_PCLK_SAMPLE_FALLING)
> pixclk |= 0x10;
>
> if (!(flags & SOCAM_HSYNC_ACTIVE_HIGH))
> --
> 1.7.0.4
>
> --
> 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
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] mt9m111: fix pixel clock
2011-04-06 14:01 ` [PATCH 2/2] mt9m111: " Teresa Gámez
@ 2011-04-07 11:25 ` Guennadi Liakhovetski
2011-05-18 11:12 ` Guennadi Liakhovetski
1 sibling, 0 replies; 12+ messages in thread
From: Guennadi Liakhovetski @ 2011-04-07 11:25 UTC (permalink / raw)
To: Teresa Gámez; +Cc: linux-media
On Wed, 6 Apr 2011, Teresa Gámez wrote:
> This camera driver supports only rising edge, which is the default
> setting of the device. The function mt9m111_setup_pixfmt() overwrites
> this setting. So the driver actually uses falling edge.
> This patch corrects that.
Ok, this does indeed look like a bug in the driver. But this is not the
best way to fix the problem. We should extend the driver to properly
implement both pclk polarities _and_ adjust PXA270 boards, using this
chip: mioa701, ezx (twice), em-x270.
Thanks
Guennadi
>
> Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
> ---
> drivers/media/video/mt9m111.c | 14 ++++++++++++--
> 1 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
> index 53fa2a7..4040a96 100644
> --- a/drivers/media/video/mt9m111.c
> +++ b/drivers/media/video/mt9m111.c
> @@ -315,10 +315,20 @@ static int mt9m111_setup_rect(struct i2c_client *client,
> static int mt9m111_setup_pixfmt(struct i2c_client *client, u16 outfmt)
> {
> int ret;
> + u16 mask = MT9M111_OUTFMT_PROCESSED_BAYER | MT9M111_OUTFMT_RGB |
> + MT9M111_OUTFMT_BYPASS_IFP | MT9M111_OUTFMT_SWAP_RGB_EVEN |
> + MT9M111_OUTFMT_RGB565 | MT9M111_OUTFMT_RGB555 |
> + MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr |
> + MT9M111_OUTFMT_SWAP_YCbCr_C_Y;
>
> - ret = reg_write(OUTPUT_FORMAT_CTRL2_A, outfmt);
> + ret = reg_clear(OUTPUT_FORMAT_CTRL2_A, mask);
> if (!ret)
> - ret = reg_write(OUTPUT_FORMAT_CTRL2_B, outfmt);
> + ret = reg_set(OUTPUT_FORMAT_CTRL2_A, outfmt);
> + if (!ret)
> + ret = reg_clear(OUTPUT_FORMAT_CTRL2_B, mask);
> + if (!ret)
> + ret = reg_set(OUTPUT_FORMAT_CTRL2_B, outfmt);
> +
> return ret;
> }
>
> --
> 1.7.0.4
>
> --
> 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
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] mt9v022: fix pixel clock
2011-04-07 11:08 ` [PATCH 1/2] mt9v022: " Guennadi Liakhovetski
@ 2011-04-07 12:38 ` Teresa Gamez
[not found] ` <OF0E7310A6.B4F9559D-ONC125786B.003E2F29-C125786B.004202D7@phytec.de>
1 sibling, 0 replies; 12+ messages in thread
From: Teresa Gamez @ 2011-04-07 12:38 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-media
Hello Guennadi,
Sorry for the first mail...
The datasheet also says (see table 3):
<quote>
Pixel clock out. DOUT is valid on rising edge of this
clock.
</quote>
There is a difference between DOUT beeing vaild and DOUT beeing set up.
So does SOCAM_PCLK_SAMPLE_RISING mean that the data is valid at rising
edge or does it mean the data is set up at rising edge?
I have tested this with a pcm038 but I will also make meassurements with
the pcm037.
Teresa
Am Donnerstag, den 07.04.2011, 13:08 +0200 schrieb Guennadi
Liakhovetski:
> On Wed, 6 Apr 2011, Teresa Gámez wrote:
>
> > Measurements show that the setup of the pixel clock is not correct.
> > The 'Invert Pixel Clock' bit has to be set to 1 for falling edge
> > and not for rising.
>
> Doesn't seem correct to me. The mt9v022 datasheet says:
>
> <quote>
> Invert pixel clock. When set, LINE_VALID,
> FRAME_VALID, and DOUT is set up to the rising edge
> of pixel clock, PIXCLK. When clear, they are set up to
> the falling edge of PIXCLK.
> </quote>
>
> and this works for present mt9v022 configurations, which include at least
> two boards: PXA270-based arch/arm/mach-pxa/pcm990-baseboard.c and i.MX31
> based arch/arm/mach-mx3/mach-pcm037.c. If this is different for your
> board, maybe you have to set the SOCAM_SENSOR_INVERT_PCLK flag in your
> "struct soc_camera_link" instance.
>
> Thanks
> Guennadi
>
> > Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
> > ---
> > drivers/media/video/mt9v022.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/media/video/mt9v022.c b/drivers/media/video/mt9v022.c
> > index 6a784c8..dec2a69 100644
> > --- a/drivers/media/video/mt9v022.c
> > +++ b/drivers/media/video/mt9v022.c
> > @@ -228,7 +228,7 @@ static int mt9v022_set_bus_param(struct soc_camera_device *icd,
> >
> > flags = soc_camera_apply_sensor_flags(icl, flags);
> >
> > - if (flags & SOCAM_PCLK_SAMPLE_RISING)
> > + if (flags & SOCAM_PCLK_SAMPLE_FALLING)
> > pixclk |= 0x10;
> >
> > if (!(flags & SOCAM_HSYNC_ACTIVE_HIGH))
> > --
> > 1.7.0.4
> >
> > --
> > 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
> >
>
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Antwort: Re: [PATCH 1/2] mt9v022: fix pixel clock
[not found] ` <OF0E7310A6.B4F9559D-ONC125786B.003E2F29-C125786B.004202D7@phytec.de>
@ 2011-04-07 12:41 ` Guennadi Liakhovetski
2011-04-08 13:27 ` Teresa Gamez
0 siblings, 1 reply; 12+ messages in thread
From: Guennadi Liakhovetski @ 2011-04-07 12:41 UTC (permalink / raw)
To: Teresa Gamez; +Cc: linux-media
Hello Teresa
On Thu, 7 Apr 2011, Teresa Gamez wrote:
> Hello Guennadi,
>
> the datasheet also says (see table 3):
>
> <quote>
> Pixel clock out. DOUT is valid on rising edge of this
> clock.
> </quote>
>
> There is a difference between DOUT beeing vaild and DOUT beeing set up.
> So does SOCAM_PCLK_SAMPLE_RISING mean that the data is valid at rising
> edge or
> does it mean the data is set up at rising edge?
Hm, yeah, looks like a typical example of a copy-paste datasheet to me:-(
And now we don't know which of the two is actually supposed to be true. As
for "set up" vs. "valid" - not sure, whether there is indeed a difference
between them. To me "set up _TO_ the rising edge" is a short way to set
"set up to be valid at the rising edge," however, I might be wrong. Can
you tell me in more detail what and where (at the sensor board or on the
baseboard) you measured and what it looked like? I think, Figure 7 and the
description below it are interesting. From that diagram I would indeed say
indeed the DOUT pins are valid and should be sampled at the rising edge by
default - when bit 4 in 0x74 is not set. SOCAM_PCLK_SAMPLE_RISING means,
that the data should be sampled at the rising of pclkm, i.e., it is valid
there.
So, yes, if your measurements agree with figure 7 from the datasheet, we
shall assume, that the driver implements the pclk polarity wrongly. But
the fix should be more extensive, than what you've submitted: if we invert
driver's behaviour, we should also invert board configuration of all
driver users: pcm990 and pcm037. Or we have to test them and verify, that
the inverted pclk polarity doesn't megatively affect the image quality, or
maybe even improves it.
Thanks
Guennadi
> I have tested this with a pcm038 but I will also make meassurements with
> the pcm037.
>
> Teresa
>
> Guennadi Liakhovetski <g.liakhovetski@gmx.de> schrieb am 07.04.2011
> 13:08:11:
>
> > Von: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > An: Teresa Gámez <t.gamez@phytec.de>
> > Kopie: linux-media@vger.kernel.org
> > Datum: 07.04.2011 13:08
> > Betreff: Re: [PATCH 1/2] mt9v022: fix pixel clock
> >
> > On Wed, 6 Apr 2011, Teresa Gámez wrote:
> >
> > > Measurements show that the setup of the pixel clock is not correct.
> > > The 'Invert Pixel Clock' bit has to be set to 1 for falling edge
> > > and not for rising.
> >
> > Doesn't seem correct to me. The mt9v022 datasheet says:
> >
> > <quote>
> > Invert pixel clock. When set, LINE_VALID,
> > FRAME_VALID, and DOUT is set up to the rising edge
> > of pixel clock, PIXCLK. When clear, they are set up to
> > the falling edge of PIXCLK.
> > </quote>
> >
> > and this works for present mt9v022 configurations, which include at
> least
> > two boards: PXA270-based arch/arm/mach-pxa/pcm990-baseboard.c and i.MX31
>
> > based arch/arm/mach-mx3/mach-pcm037.c. If this is different for your
> > board, maybe you have to set the SOCAM_SENSOR_INVERT_PCLK flag in your
> > "struct soc_camera_link" instance.
> >
> > Thanks
> > Guennadi
> >
> > > Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
> > > ---
> > > drivers/media/video/mt9v022.c | 2 +-
> > > 1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/drivers/media/video/mt9v022.c
> b/drivers/media/video/mt9v022.c
> > > index 6a784c8..dec2a69 100644
> > > --- a/drivers/media/video/mt9v022.c
> > > +++ b/drivers/media/video/mt9v022.c
> > > @@ -228,7 +228,7 @@ static int mt9v022_set_bus_param(struct
> > soc_camera_device *icd,
> > >
> > > flags = soc_camera_apply_sensor_flags(icl, flags);
> > >
> > > - if (flags & SOCAM_PCLK_SAMPLE_RISING)
> > > + if (flags & SOCAM_PCLK_SAMPLE_FALLING)
> > > pixclk |= 0x10;
> > >
> > > if (!(flags & SOCAM_HSYNC_ACTIVE_HIGH))
> > > --
> > > 1.7.0.4
> > >
> > > --
> > > 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
> > >
> >
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Antwort: Re: [PATCH 1/2] mt9v022: fix pixel clock
2011-04-07 12:41 ` Antwort: " Guennadi Liakhovetski
@ 2011-04-08 13:27 ` Teresa Gamez
2011-04-12 11:22 ` Teresa Gamez
0 siblings, 1 reply; 12+ messages in thread
From: Teresa Gamez @ 2011-04-08 13:27 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-media
Hello Guennadi,
Am Donnerstag, den 07.04.2011, 14:41 +0200 schrieb Guennadi
Liakhovetski:
> Hello Teresa
>
> On Thu, 7 Apr 2011, Teresa Gamez wrote:
>
> > Hello Guennadi,
> >
> > the datasheet also says (see table 3):
> >
> > <quote>
> > Pixel clock out. DOUT is valid on rising edge of this
> > clock.
> > </quote>
> >
> > There is a difference between DOUT beeing vaild and DOUT beeing set up.
> > So does SOCAM_PCLK_SAMPLE_RISING mean that the data is valid at rising
> > edge or
> > does it mean the data is set up at rising edge?
>
> Hm, yeah, looks like a typical example of a copy-paste datasheet to me:-(
> And now we don't know which of the two is actually supposed to be true. As
> for "set up" vs. "valid" - not sure, whether there is indeed a difference
> between them. To me "set up _TO_ the rising edge" is a short way to set
> "set up to be valid at the rising edge," however, I might be wrong. Can
> you tell me in more detail what and where (at the sensor board or on the
> baseboard) you measured and what it looked like? I think, Figure 7 and the
> description below it are interesting. From that diagram I would indeed say
> indeed the DOUT pins are valid and should be sampled at the rising edge by
> default - when bit 4 in 0x74 is not set. SOCAM_PCLK_SAMPLE_RISING means,
> that the data should be sampled at the rising of pclkm, i.e., it is valid
> there.
I meassured the outgoing pins from the baseboard to the camera board and
checked the PCLK and D0 to see at which point the data is valid. I have
also checked the quality of the image.
All tests where made with sensor_type=color
My results for pcm038 are with following register settings:
mx2_camera
0x0 CSICR1: 0x10020b92
-> rising edge
mt9v022
0x74 PIXCLK_FV_LV: 0x00000010
-> rising edge (which I think is falling edge)
meassured: falling edge (ugly image, wrong colors)
Now I set the SOCAM_SENSOR_INVERT_PCLK flag in the platformcode for the
mt9v022:
mx2_camera
0x0 CSICR1 0x10020b92
-> rising edge
mt9v022
0x74 PIXCLK_FV_LV 0x00000000
-> falling edge (which I think is rising edge)
meassured: rising edge (image is OK)
Now changed the PCLK of the mx2_camera:
mx2_camera
0x0 CSICR1 0x10020b90
-> falling edge
mt9v022
0x74 PIXCLK_FV_LV 0x00000010
-> rising edge (which I think is falling edge)
meassured: falling edge (image is OK)
>
> So, yes, if your measurements agree with figure 7 from the datasheet, we
> shall assume, that the driver implements the pclk polarity wrongly. But
> the fix should be more extensive, than what you've submitted: if we invert
> driver's behaviour, we should also invert board configuration of all
> driver users: pcm990 and pcm037. Or we have to test them and verify, that
> the inverted pclk polarity doesn't megatively affect the image quality, or
> maybe even improves it.
>
> Thanks
> Guennadi
>
> > I have tested this with a pcm038 but I will also make meassurements with
> > the pcm037.
> >
Same results with the pcm037:
mx3_camera
0x60 CSI_SENS_CONF: 0x00000700
-> rising edge
mt9v022
0x74 PIXCLK_FV_LV: 0x00000010
-> rising edge (which I think is falling edge)
meassured: falling edge (ulgy image, looks like b/w with pixel errors)
Set SOCAM_SENSOR_INVERT_PCLK flag in the platformcode for the mt9v022:
mx3_camera
0x60 CSI_SENS_CONF: 0x00000700
-> rising edge
mt9v022
0x74 PIXCLK_FV_LV 0x00000000
-> falling edge (which I think is rising edge)
meassured: rising edge (image is OK)
Additionally set MX3_CAMERA_PCP of the mx3_camera flags
mx3_camera
0x60 CSI_SENS_CONF: 0x00000708
-> falling edge
mt9v022
0x74 PIXCLK_FV_LV: 0x00000010
-> rising edge (which I think is falling edge)
meassured: falling edge (image is OK)
Removed SOCAM_SENSOR_INVERT_PCLK flag for the mt9v022:
mx3_camera
0x60 CSI_SENS_CONF: 0x00000708
-> falling edge
mt9v022
0x74 PIXCLK_FV_LV 0x00000000
-> falling edge (which I think is rising edge)
meassured: risging edge (ugly image, looks like the first one)
I have noticed that on our pcm037 BSP the SOCAM_SENSOR_INVERT_PCLK flag
for the camera was set to "fix" this issue.
I will continue this test on the pcm990.
Teresa
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Antwort: Re: [PATCH 1/2] mt9v022: fix pixel clock
2011-04-08 13:27 ` Teresa Gamez
@ 2011-04-12 11:22 ` Teresa Gamez
2011-04-13 6:31 ` Guennadi Liakhovetski
0 siblings, 1 reply; 12+ messages in thread
From: Teresa Gamez @ 2011-04-12 11:22 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-media
Am Freitag, den 08.04.2011, 15:27 +0200 schrieb Teresa Gamez:
> Hello Guennadi,
>
> Am Donnerstag, den 07.04.2011, 14:41 +0200 schrieb Guennadi
> Liakhovetski:
> > Hello Teresa
> >
> > On Thu, 7 Apr 2011, Teresa Gamez wrote:
> >
> > > Hello Guennadi,
> > >
> > > the datasheet also says (see table 3):
> > >
> > > <quote>
> > > Pixel clock out. DOUT is valid on rising edge of this
> > > clock.
> > > </quote>
> > >
> > > There is a difference between DOUT beeing vaild and DOUT beeing set up.
> > > So does SOCAM_PCLK_SAMPLE_RISING mean that the data is valid at rising
> > > edge or
> > > does it mean the data is set up at rising edge?
> >
> > Hm, yeah, looks like a typical example of a copy-paste datasheet to me:-(
> > And now we don't know which of the two is actually supposed to be true. As
> > for "set up" vs. "valid" - not sure, whether there is indeed a difference
> > between them. To me "set up _TO_ the rising edge" is a short way to set
> > "set up to be valid at the rising edge," however, I might be wrong. Can
> > you tell me in more detail what and where (at the sensor board or on the
> > baseboard) you measured and what it looked like? I think, Figure 7 and the
> > description below it are interesting. From that diagram I would indeed say
> > indeed the DOUT pins are valid and should be sampled at the rising edge by
> > default - when bit 4 in 0x74 is not set. SOCAM_PCLK_SAMPLE_RISING means,
> > that the data should be sampled at the rising of pclkm, i.e., it is valid
> > there.
>
> I meassured the outgoing pins from the baseboard to the camera board and
> checked the PCLK and D0 to see at which point the data is valid. I have
> also checked the quality of the image.
> All tests where made with sensor_type=color
>
> My results for pcm038 are with following register settings:
>
> mx2_camera
> 0x0 CSICR1: 0x10020b92
> -> rising edge
>
> mt9v022
> 0x74 PIXCLK_FV_LV: 0x00000010
> -> rising edge (which I think is falling edge)
>
> meassured: falling edge (ugly image, wrong colors)
>
> Now I set the SOCAM_SENSOR_INVERT_PCLK flag in the platformcode for the
> mt9v022:
>
> mx2_camera
> 0x0 CSICR1 0x10020b92
> -> rising edge
>
> mt9v022
> 0x74 PIXCLK_FV_LV 0x00000000
> -> falling edge (which I think is rising edge)
>
> meassured: rising edge (image is OK)
>
> Now changed the PCLK of the mx2_camera:
>
> mx2_camera
> 0x0 CSICR1 0x10020b90
> -> falling edge
>
> mt9v022
> 0x74 PIXCLK_FV_LV 0x00000010
> -> rising edge (which I think is falling edge)
>
> meassured: falling edge (image is OK)
>
> >
> > So, yes, if your measurements agree with figure 7 from the datasheet, we
> > shall assume, that the driver implements the pclk polarity wrongly. But
> > the fix should be more extensive, than what you've submitted: if we invert
> > driver's behaviour, we should also invert board configuration of all
> > driver users: pcm990 and pcm037. Or we have to test them and verify, that
> > the inverted pclk polarity doesn't megatively affect the image quality, or
> > maybe even improves it.
> >
> > Thanks
> > Guennadi
> >
> > > I have tested this with a pcm038 but I will also make meassurements with
> > > the pcm037.
> > >
>
> Same results with the pcm037:
>
> mx3_camera
> 0x60 CSI_SENS_CONF: 0x00000700
> -> rising edge
>
> mt9v022
> 0x74 PIXCLK_FV_LV: 0x00000010
> -> rising edge (which I think is falling edge)
>
> meassured: falling edge (ulgy image, looks like b/w with pixel errors)
>
> Set SOCAM_SENSOR_INVERT_PCLK flag in the platformcode for the mt9v022:
> mx3_camera
> 0x60 CSI_SENS_CONF: 0x00000700
> -> rising edge
>
> mt9v022
> 0x74 PIXCLK_FV_LV 0x00000000
> -> falling edge (which I think is rising edge)
>
> meassured: rising edge (image is OK)
>
> Additionally set MX3_CAMERA_PCP of the mx3_camera flags
>
> mx3_camera
> 0x60 CSI_SENS_CONF: 0x00000708
> -> falling edge
>
> mt9v022
> 0x74 PIXCLK_FV_LV: 0x00000010
> -> rising edge (which I think is falling edge)
>
> meassured: falling edge (image is OK)
>
> Removed SOCAM_SENSOR_INVERT_PCLK flag for the mt9v022:
>
> mx3_camera
> 0x60 CSI_SENS_CONF: 0x00000708
> -> falling edge
>
> mt9v022
> 0x74 PIXCLK_FV_LV 0x00000000
> -> falling edge (which I think is rising edge)
>
> meassured: risging edge (ugly image, looks like the first one)
>
> I have noticed that on our pcm037 BSP the SOCAM_SENSOR_INVERT_PCLK flag
> for the camera was set to "fix" this issue.
> I will continue this test on the pcm990.
>
Got the same result with the pcm990:
pxa_camera
0x50000010 CICR4: 0x00880001
-> rising edge (0 << 22)
mt9v022
0x74 PIXCLK_FV_LV: 0x00000010
-> rising edge (1 << 4) (which I think is falling edge)
meassured: falling edge (some pixel have wrong colors)
---
Now set the SOCAM_SENSOR_INVERT_PCLK for mt9v022:
pxa_camera
0x50000010 CICR4: 0x00880001
-> rising edge (0 << 22)
mt9v022
0x74 PIXCLK_FV_LV: 0x00000000
-> falling edge (0 << 4) (which I think is rising edge)
meassured: rising edge (image is OK)
---
Additionaly set the PXA_CAMERA_PCP flag:
pxa_camera
0x50000010 CICR4: 0x00c80001
-> falling edge (1 << 22)
mt9v022
0x74 PIXCLK_FV_LV: 0x00000010
-> rising edge (1 << 4) (which I think is falling edge)
meassured: falling edge (image is OK)
---
Removed SOCAM_SENSOR_INVERT_PCLK again:
pxa_camera
0x50000010 CICR4: 0x00c80001
-> falling edge (1 << 22)
mt9v022
0x74 PIXCLK_FV_LV: 0x00000000
-> falling edge (0 << 4) (which I think is rising edge)
meassured: rising edge (same as above, image shows wrong colored pixels)
I hope thats sufficed.
Teresa
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Antwort: Re: [PATCH 1/2] mt9v022: fix pixel clock
2011-04-12 11:22 ` Teresa Gamez
@ 2011-04-13 6:31 ` Guennadi Liakhovetski
2011-04-14 14:33 ` Teresa Gamez
0 siblings, 1 reply; 12+ messages in thread
From: Guennadi Liakhovetski @ 2011-04-13 6:31 UTC (permalink / raw)
To: Teresa Gamez; +Cc: linux-media
Hello Teresa
Thanks very much for your extensive testing! I'm afraid, I don't have the
time right now to go through all those register settings, so, can we,
maybe, do the following: we currently have two platforms in the mainline,
that use mt9v022. We believe, that the driver itself implements the pixel
clock edge configuration wrongly. Could you please provide patches to
1. fix mt9v022 to match our new understanding
2. if needed - additionally fix pcm037 by setting the
SOCAM_SENSOR_INVERT_PCLK flag
3. same for pcm990 / pcm027
I also noticed your "wrong colours" result - if colours are completely
swapped, maybe you just have to adjust your Bayer decoder?
Once we get those patches, I'll try to test them, but I've a very tight
schedule atm, so, maybe I'll just believe you to get them on time for
2.6.39.
Thanks
Guennadi
On Tue, 12 Apr 2011, Teresa Gamez wrote:
> Am Freitag, den 08.04.2011, 15:27 +0200 schrieb Teresa Gamez:
> > Hello Guennadi,
> >
> > Am Donnerstag, den 07.04.2011, 14:41 +0200 schrieb Guennadi
> > Liakhovetski:
> > > Hello Teresa
> > >
> > > On Thu, 7 Apr 2011, Teresa Gamez wrote:
> > >
> > > > Hello Guennadi,
> > > >
> > > > the datasheet also says (see table 3):
> > > >
> > > > <quote>
> > > > Pixel clock out. DOUT is valid on rising edge of this
> > > > clock.
> > > > </quote>
> > > >
> > > > There is a difference between DOUT beeing vaild and DOUT beeing set up.
> > > > So does SOCAM_PCLK_SAMPLE_RISING mean that the data is valid at rising
> > > > edge or
> > > > does it mean the data is set up at rising edge?
> > >
> > > Hm, yeah, looks like a typical example of a copy-paste datasheet to me:-(
> > > And now we don't know which of the two is actually supposed to be true. As
> > > for "set up" vs. "valid" - not sure, whether there is indeed a difference
> > > between them. To me "set up _TO_ the rising edge" is a short way to set
> > > "set up to be valid at the rising edge," however, I might be wrong. Can
> > > you tell me in more detail what and where (at the sensor board or on the
> > > baseboard) you measured and what it looked like? I think, Figure 7 and the
> > > description below it are interesting. From that diagram I would indeed say
> > > indeed the DOUT pins are valid and should be sampled at the rising edge by
> > > default - when bit 4 in 0x74 is not set. SOCAM_PCLK_SAMPLE_RISING means,
> > > that the data should be sampled at the rising of pclkm, i.e., it is valid
> > > there.
> >
> > I meassured the outgoing pins from the baseboard to the camera board and
> > checked the PCLK and D0 to see at which point the data is valid. I have
> > also checked the quality of the image.
> > All tests where made with sensor_type=color
> >
> > My results for pcm038 are with following register settings:
> >
> > mx2_camera
> > 0x0 CSICR1: 0x10020b92
> > -> rising edge
> >
> > mt9v022
> > 0x74 PIXCLK_FV_LV: 0x00000010
> > -> rising edge (which I think is falling edge)
> >
> > meassured: falling edge (ugly image, wrong colors)
> >
> > Now I set the SOCAM_SENSOR_INVERT_PCLK flag in the platformcode for the
> > mt9v022:
> >
> > mx2_camera
> > 0x0 CSICR1 0x10020b92
> > -> rising edge
> >
> > mt9v022
> > 0x74 PIXCLK_FV_LV 0x00000000
> > -> falling edge (which I think is rising edge)
> >
> > meassured: rising edge (image is OK)
> >
> > Now changed the PCLK of the mx2_camera:
> >
> > mx2_camera
> > 0x0 CSICR1 0x10020b90
> > -> falling edge
> >
> > mt9v022
> > 0x74 PIXCLK_FV_LV 0x00000010
> > -> rising edge (which I think is falling edge)
> >
> > meassured: falling edge (image is OK)
> >
> > >
> > > So, yes, if your measurements agree with figure 7 from the datasheet, we
> > > shall assume, that the driver implements the pclk polarity wrongly. But
> > > the fix should be more extensive, than what you've submitted: if we invert
> > > driver's behaviour, we should also invert board configuration of all
> > > driver users: pcm990 and pcm037. Or we have to test them and verify, that
> > > the inverted pclk polarity doesn't megatively affect the image quality, or
> > > maybe even improves it.
> > >
> > > Thanks
> > > Guennadi
> > >
> > > > I have tested this with a pcm038 but I will also make meassurements with
> > > > the pcm037.
> > > >
> >
> > Same results with the pcm037:
> >
> > mx3_camera
> > 0x60 CSI_SENS_CONF: 0x00000700
> > -> rising edge
> >
> > mt9v022
> > 0x74 PIXCLK_FV_LV: 0x00000010
> > -> rising edge (which I think is falling edge)
> >
> > meassured: falling edge (ulgy image, looks like b/w with pixel errors)
> >
> > Set SOCAM_SENSOR_INVERT_PCLK flag in the platformcode for the mt9v022:
> > mx3_camera
> > 0x60 CSI_SENS_CONF: 0x00000700
> > -> rising edge
> >
> > mt9v022
> > 0x74 PIXCLK_FV_LV 0x00000000
> > -> falling edge (which I think is rising edge)
> >
> > meassured: rising edge (image is OK)
> >
> > Additionally set MX3_CAMERA_PCP of the mx3_camera flags
> >
> > mx3_camera
> > 0x60 CSI_SENS_CONF: 0x00000708
> > -> falling edge
> >
> > mt9v022
> > 0x74 PIXCLK_FV_LV: 0x00000010
> > -> rising edge (which I think is falling edge)
> >
> > meassured: falling edge (image is OK)
> >
> > Removed SOCAM_SENSOR_INVERT_PCLK flag for the mt9v022:
> >
> > mx3_camera
> > 0x60 CSI_SENS_CONF: 0x00000708
> > -> falling edge
> >
> > mt9v022
> > 0x74 PIXCLK_FV_LV 0x00000000
> > -> falling edge (which I think is rising edge)
> >
> > meassured: risging edge (ugly image, looks like the first one)
> >
> > I have noticed that on our pcm037 BSP the SOCAM_SENSOR_INVERT_PCLK flag
> > for the camera was set to "fix" this issue.
> > I will continue this test on the pcm990.
> >
>
> Got the same result with the pcm990:
>
> pxa_camera
> 0x50000010 CICR4: 0x00880001
> -> rising edge (0 << 22)
> mt9v022
> 0x74 PIXCLK_FV_LV: 0x00000010
> -> rising edge (1 << 4) (which I think is falling edge)
>
> meassured: falling edge (some pixel have wrong colors)
>
> ---
> Now set the SOCAM_SENSOR_INVERT_PCLK for mt9v022:
>
> pxa_camera
> 0x50000010 CICR4: 0x00880001
> -> rising edge (0 << 22)
>
> mt9v022
> 0x74 PIXCLK_FV_LV: 0x00000000
> -> falling edge (0 << 4) (which I think is rising edge)
>
> meassured: rising edge (image is OK)
>
> ---
> Additionaly set the PXA_CAMERA_PCP flag:
>
> pxa_camera
> 0x50000010 CICR4: 0x00c80001
> -> falling edge (1 << 22)
>
> mt9v022
> 0x74 PIXCLK_FV_LV: 0x00000010
> -> rising edge (1 << 4) (which I think is falling edge)
>
> meassured: falling edge (image is OK)
>
> ---
> Removed SOCAM_SENSOR_INVERT_PCLK again:
>
> pxa_camera
> 0x50000010 CICR4: 0x00c80001
> -> falling edge (1 << 22)
>
> mt9v022
> 0x74 PIXCLK_FV_LV: 0x00000000
> -> falling edge (0 << 4) (which I think is rising edge)
>
> meassured: rising edge (same as above, image shows wrong colored pixels)
>
>
> I hope thats sufficed.
>
> Teresa
>
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Antwort: Re: [PATCH 1/2] mt9v022: fix pixel clock
2011-04-13 6:31 ` Guennadi Liakhovetski
@ 2011-04-14 14:33 ` Teresa Gamez
0 siblings, 0 replies; 12+ messages in thread
From: Teresa Gamez @ 2011-04-14 14:33 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-media
Am Mittwoch, den 13.04.2011, 08:31 +0200 schrieb Guennadi Liakhovetski:
> Hello Teresa
>
> Thanks very much for your extensive testing! I'm afraid, I don't have the
> time right now to go through all those register settings, so, can we,
> maybe, do the following: we currently have two platforms in the mainline,
> that use mt9v022. We believe, that the driver itself implements the pixel
> clock edge configuration wrongly. Could you please provide patches to
>
> 1. fix mt9v022 to match our new understanding
> 2. if needed - additionally fix pcm037 by setting the
> SOCAM_SENSOR_INVERT_PCLK flag
> 3. same for pcm990 / pcm027
As I can see only the driver code has to be changed.
Platformcode for pcm037 and pcm990 is ok.
I will resend the patch.
> I also noticed your "wrong colours" result - if colours are completely
> swapped, maybe you just have to adjust your Bayer decoder?
They are not completely swapped. There some areas where it looks fine
and others where the colors are wrong or the pixels are noisy.
> Once we get those patches, I'll try to test them, but I've a very tight
> schedule atm, so, maybe I'll just believe you to get them on time for
> 2.6.39.
Thank you for testing this, too.
Teresa
>
> Thanks
> Guennadi
>
> On Tue, 12 Apr 2011, Teresa Gamez wrote:
>
> > Am Freitag, den 08.04.2011, 15:27 +0200 schrieb Teresa Gamez:
> > > Hello Guennadi,
> > >
> > > Am Donnerstag, den 07.04.2011, 14:41 +0200 schrieb Guennadi
> > > Liakhovetski:
> > > > Hello Teresa
> > > >
> > > > On Thu, 7 Apr 2011, Teresa Gamez wrote:
> > > >
> > > > > Hello Guennadi,
> > > > >
> > > > > the datasheet also says (see table 3):
> > > > >
> > > > > <quote>
> > > > > Pixel clock out. DOUT is valid on rising edge of this
> > > > > clock.
> > > > > </quote>
> > > > >
> > > > > There is a difference between DOUT beeing vaild and DOUT beeing set up.
> > > > > So does SOCAM_PCLK_SAMPLE_RISING mean that the data is valid at rising
> > > > > edge or
> > > > > does it mean the data is set up at rising edge?
> > > >
> > > > Hm, yeah, looks like a typical example of a copy-paste datasheet to me:-(
> > > > And now we don't know which of the two is actually supposed to be true. As
> > > > for "set up" vs. "valid" - not sure, whether there is indeed a difference
> > > > between them. To me "set up _TO_ the rising edge" is a short way to set
> > > > "set up to be valid at the rising edge," however, I might be wrong. Can
> > > > you tell me in more detail what and where (at the sensor board or on the
> > > > baseboard) you measured and what it looked like? I think, Figure 7 and the
> > > > description below it are interesting. From that diagram I would indeed say
> > > > indeed the DOUT pins are valid and should be sampled at the rising edge by
> > > > default - when bit 4 in 0x74 is not set. SOCAM_PCLK_SAMPLE_RISING means,
> > > > that the data should be sampled at the rising of pclkm, i.e., it is valid
> > > > there.
> > >
> > > I meassured the outgoing pins from the baseboard to the camera board and
> > > checked the PCLK and D0 to see at which point the data is valid. I have
> > > also checked the quality of the image.
> > > All tests where made with sensor_type=color
> > >
> > > My results for pcm038 are with following register settings:
> > >
> > > mx2_camera
> > > 0x0 CSICR1: 0x10020b92
> > > -> rising edge
> > >
> > > mt9v022
> > > 0x74 PIXCLK_FV_LV: 0x00000010
> > > -> rising edge (which I think is falling edge)
> > >
> > > meassured: falling edge (ugly image, wrong colors)
> > >
> > > Now I set the SOCAM_SENSOR_INVERT_PCLK flag in the platformcode for the
> > > mt9v022:
> > >
> > > mx2_camera
> > > 0x0 CSICR1 0x10020b92
> > > -> rising edge
> > >
> > > mt9v022
> > > 0x74 PIXCLK_FV_LV 0x00000000
> > > -> falling edge (which I think is rising edge)
> > >
> > > meassured: rising edge (image is OK)
> > >
> > > Now changed the PCLK of the mx2_camera:
> > >
> > > mx2_camera
> > > 0x0 CSICR1 0x10020b90
> > > -> falling edge
> > >
> > > mt9v022
> > > 0x74 PIXCLK_FV_LV 0x00000010
> > > -> rising edge (which I think is falling edge)
> > >
> > > meassured: falling edge (image is OK)
> > >
> > > >
> > > > So, yes, if your measurements agree with figure 7 from the datasheet, we
> > > > shall assume, that the driver implements the pclk polarity wrongly. But
> > > > the fix should be more extensive, than what you've submitted: if we invert
> > > > driver's behaviour, we should also invert board configuration of all
> > > > driver users: pcm990 and pcm037. Or we have to test them and verify, that
> > > > the inverted pclk polarity doesn't megatively affect the image quality, or
> > > > maybe even improves it.
> > > >
> > > > Thanks
> > > > Guennadi
> > > >
> > > > > I have tested this with a pcm038 but I will also make meassurements with
> > > > > the pcm037.
> > > > >
> > >
> > > Same results with the pcm037:
> > >
> > > mx3_camera
> > > 0x60 CSI_SENS_CONF: 0x00000700
> > > -> rising edge
> > >
> > > mt9v022
> > > 0x74 PIXCLK_FV_LV: 0x00000010
> > > -> rising edge (which I think is falling edge)
> > >
> > > meassured: falling edge (ulgy image, looks like b/w with pixel errors)
> > >
> > > Set SOCAM_SENSOR_INVERT_PCLK flag in the platformcode for the mt9v022:
> > > mx3_camera
> > > 0x60 CSI_SENS_CONF: 0x00000700
> > > -> rising edge
> > >
> > > mt9v022
> > > 0x74 PIXCLK_FV_LV 0x00000000
> > > -> falling edge (which I think is rising edge)
> > >
> > > meassured: rising edge (image is OK)
> > >
> > > Additionally set MX3_CAMERA_PCP of the mx3_camera flags
> > >
> > > mx3_camera
> > > 0x60 CSI_SENS_CONF: 0x00000708
> > > -> falling edge
> > >
> > > mt9v022
> > > 0x74 PIXCLK_FV_LV: 0x00000010
> > > -> rising edge (which I think is falling edge)
> > >
> > > meassured: falling edge (image is OK)
> > >
> > > Removed SOCAM_SENSOR_INVERT_PCLK flag for the mt9v022:
> > >
> > > mx3_camera
> > > 0x60 CSI_SENS_CONF: 0x00000708
> > > -> falling edge
> > >
> > > mt9v022
> > > 0x74 PIXCLK_FV_LV 0x00000000
> > > -> falling edge (which I think is rising edge)
> > >
> > > meassured: risging edge (ugly image, looks like the first one)
> > >
> > > I have noticed that on our pcm037 BSP the SOCAM_SENSOR_INVERT_PCLK flag
> > > for the camera was set to "fix" this issue.
> > > I will continue this test on the pcm990.
> > >
> >
> > Got the same result with the pcm990:
> >
> > pxa_camera
> > 0x50000010 CICR4: 0x00880001
> > -> rising edge (0 << 22)
> > mt9v022
> > 0x74 PIXCLK_FV_LV: 0x00000010
> > -> rising edge (1 << 4) (which I think is falling edge)
> >
> > meassured: falling edge (some pixel have wrong colors)
> >
> > ---
> > Now set the SOCAM_SENSOR_INVERT_PCLK for mt9v022:
> >
> > pxa_camera
> > 0x50000010 CICR4: 0x00880001
> > -> rising edge (0 << 22)
> >
> > mt9v022
> > 0x74 PIXCLK_FV_LV: 0x00000000
> > -> falling edge (0 << 4) (which I think is rising edge)
> >
> > meassured: rising edge (image is OK)
> >
> > ---
> > Additionaly set the PXA_CAMERA_PCP flag:
> >
> > pxa_camera
> > 0x50000010 CICR4: 0x00c80001
> > -> falling edge (1 << 22)
> >
> > mt9v022
> > 0x74 PIXCLK_FV_LV: 0x00000010
> > -> rising edge (1 << 4) (which I think is falling edge)
> >
> > meassured: falling edge (image is OK)
> >
> > ---
> > Removed SOCAM_SENSOR_INVERT_PCLK again:
> >
> > pxa_camera
> > 0x50000010 CICR4: 0x00c80001
> > -> falling edge (1 << 22)
> >
> > mt9v022
> > 0x74 PIXCLK_FV_LV: 0x00000000
> > -> falling edge (0 << 4) (which I think is rising edge)
> >
> > meassured: rising edge (same as above, image shows wrong colored pixels)
> >
> >
> > I hope thats sufficed.
> >
> > Teresa
> >
> >
>
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] mt9m111: fix pixel clock
2011-04-06 14:01 ` [PATCH 2/2] mt9m111: " Teresa Gámez
2011-04-07 11:25 ` Guennadi Liakhovetski
@ 2011-05-18 11:12 ` Guennadi Liakhovetski
2011-05-19 11:07 ` Teresa Gamez
1 sibling, 1 reply; 12+ messages in thread
From: Guennadi Liakhovetski @ 2011-05-18 11:12 UTC (permalink / raw)
To: Teresa Gámez; +Cc: linux-media
Hi Teresa
I've verified the mt9v022 patch - finally I have noticed the difference,
it does look correct! As for this one, how about this version of your
patch:
diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 53fa2a7..ebebed9 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -315,10 +315,20 @@ static int mt9m111_setup_rect(struct i2c_client *client,
static int mt9m111_setup_pixfmt(struct i2c_client *client, u16 outfmt)
{
int ret;
+ u16 mask = MT9M111_OUTFMT_PROCESSED_BAYER | MT9M111_OUTFMT_RGB |
+ MT9M111_OUTFMT_BYPASS_IFP | MT9M111_OUTFMT_SWAP_RGB_EVEN |
+ MT9M111_OUTFMT_RGB565 | MT9M111_OUTFMT_RGB555 |
+ MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr |
+ MT9M111_OUTFMT_SWAP_YCbCr_C_Y;
- ret = reg_write(OUTPUT_FORMAT_CTRL2_A, outfmt);
+ ret = reg_read(OUTPUT_FORMAT_CTRL2_A);
+ if (ret >= 0)
+ ret = reg_write(OUTPUT_FORMAT_CTRL2_A, (ret & ~mask) | outfmt);
if (!ret)
- ret = reg_write(OUTPUT_FORMAT_CTRL2_B, outfmt);
+ ret = reg_read(OUTPUT_FORMAT_CTRL2_B);
+ if (ret >= 0)
+ ret = reg_write(OUTPUT_FORMAT_CTRL2_B, (ret & ~mask) | outfmt);
+
return ret;
}
It reduces the number of I2C accesses and avoids writing some not
necessarily desired value to the register. Does this look ok to you? Could
you maybe test - I've got no mt9m111 cameras.
Thanks
Guennadi
On Wed, 6 Apr 2011, Teresa Gámez wrote:
> This camera driver supports only rising edge, which is the default
> setting of the device. The function mt9m111_setup_pixfmt() overwrites
> this setting. So the driver actually uses falling edge.
> This patch corrects that.
>
> Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
> ---
> drivers/media/video/mt9m111.c | 14 ++++++++++++--
> 1 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
> index 53fa2a7..4040a96 100644
> --- a/drivers/media/video/mt9m111.c
> +++ b/drivers/media/video/mt9m111.c
> @@ -315,10 +315,20 @@ static int mt9m111_setup_rect(struct i2c_client *client,
> static int mt9m111_setup_pixfmt(struct i2c_client *client, u16 outfmt)
> {
> int ret;
> + u16 mask = MT9M111_OUTFMT_PROCESSED_BAYER | MT9M111_OUTFMT_RGB |
> + MT9M111_OUTFMT_BYPASS_IFP | MT9M111_OUTFMT_SWAP_RGB_EVEN |
> + MT9M111_OUTFMT_RGB565 | MT9M111_OUTFMT_RGB555 |
> + MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr |
> + MT9M111_OUTFMT_SWAP_YCbCr_C_Y;
>
> - ret = reg_write(OUTPUT_FORMAT_CTRL2_A, outfmt);
> + ret = reg_clear(OUTPUT_FORMAT_CTRL2_A, mask);
> if (!ret)
> - ret = reg_write(OUTPUT_FORMAT_CTRL2_B, outfmt);
> + ret = reg_set(OUTPUT_FORMAT_CTRL2_A, outfmt);
> + if (!ret)
> + ret = reg_clear(OUTPUT_FORMAT_CTRL2_B, mask);
> + if (!ret)
> + ret = reg_set(OUTPUT_FORMAT_CTRL2_B, outfmt);
> +
> return ret;
> }
>
> --
> 1.7.0.4
>
> --
> 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
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] mt9m111: fix pixel clock
2011-05-18 11:12 ` Guennadi Liakhovetski
@ 2011-05-19 11:07 ` Teresa Gamez
0 siblings, 0 replies; 12+ messages in thread
From: Teresa Gamez @ 2011-05-19 11:07 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-media
Am Mittwoch, den 18.05.2011, 13:12 +0200 schrieb Guennadi Liakhovetski:
> Hi Teresa
>
> I've verified the mt9v022 patch - finally I have noticed the difference,
> it does look correct! As for this one, how about this version of your
> patch:
Great! Thank you.
>
> diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
> index 53fa2a7..ebebed9 100644
> --- a/drivers/media/video/mt9m111.c
> +++ b/drivers/media/video/mt9m111.c
> @@ -315,10 +315,20 @@ static int mt9m111_setup_rect(struct i2c_client *client,
> static int mt9m111_setup_pixfmt(struct i2c_client *client, u16 outfmt)
> {
> int ret;
> + u16 mask = MT9M111_OUTFMT_PROCESSED_BAYER | MT9M111_OUTFMT_RGB |
> + MT9M111_OUTFMT_BYPASS_IFP | MT9M111_OUTFMT_SWAP_RGB_EVEN |
> + MT9M111_OUTFMT_RGB565 | MT9M111_OUTFMT_RGB555 |
> + MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr |
> + MT9M111_OUTFMT_SWAP_YCbCr_C_Y;
>
> - ret = reg_write(OUTPUT_FORMAT_CTRL2_A, outfmt);
> + ret = reg_read(OUTPUT_FORMAT_CTRL2_A);
> + if (ret >= 0)
> + ret = reg_write(OUTPUT_FORMAT_CTRL2_A, (ret & ~mask) | outfmt);
> if (!ret)
> - ret = reg_write(OUTPUT_FORMAT_CTRL2_B, outfmt);
> + ret = reg_read(OUTPUT_FORMAT_CTRL2_B);
> + if (ret >= 0)
> + ret = reg_write(OUTPUT_FORMAT_CTRL2_B, (ret & ~mask) | outfmt);
> +
> return ret;
> }
>
>
> It reduces the number of I2C accesses and avoids writing some not
> necessarily desired value to the register. Does this look ok to you? Could
> you maybe test - I've got no mt9m111 cameras.
>
Yes, this looks good. And I tested it successfully with a mt9m131.
Tested-by: Teresa Gámez <t.gamez@phytec.de>
Teresa
> Thanks
> Guennadi
>
> On Wed, 6 Apr 2011, Teresa Gámez wrote:
>
> > This camera driver supports only rising edge, which is the default
> > setting of the device. The function mt9m111_setup_pixfmt() overwrites
> > this setting. So the driver actually uses falling edge.
> > This patch corrects that.
> >
> > Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
> > ---
> > drivers/media/video/mt9m111.c | 14 ++++++++++++--
> > 1 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
> > index 53fa2a7..4040a96 100644
> > --- a/drivers/media/video/mt9m111.c
> > +++ b/drivers/media/video/mt9m111.c
> > @@ -315,10 +315,20 @@ static int mt9m111_setup_rect(struct i2c_client *client,
> > static int mt9m111_setup_pixfmt(struct i2c_client *client, u16 outfmt)
> > {
> > int ret;
> > + u16 mask = MT9M111_OUTFMT_PROCESSED_BAYER | MT9M111_OUTFMT_RGB |
> > + MT9M111_OUTFMT_BYPASS_IFP | MT9M111_OUTFMT_SWAP_RGB_EVEN |
> > + MT9M111_OUTFMT_RGB565 | MT9M111_OUTFMT_RGB555 |
> > + MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr |
> > + MT9M111_OUTFMT_SWAP_YCbCr_C_Y;
> >
> > - ret = reg_write(OUTPUT_FORMAT_CTRL2_A, outfmt);
> > + ret = reg_clear(OUTPUT_FORMAT_CTRL2_A, mask);
> > if (!ret)
> > - ret = reg_write(OUTPUT_FORMAT_CTRL2_B, outfmt);
> > + ret = reg_set(OUTPUT_FORMAT_CTRL2_A, outfmt);
> > + if (!ret)
> > + ret = reg_clear(OUTPUT_FORMAT_CTRL2_B, mask);
> > + if (!ret)
> > + ret = reg_set(OUTPUT_FORMAT_CTRL2_B, outfmt);
> > +
> > return ret;
> > }
> >
> > --
> > 1.7.0.4
> >
> > --
> > 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
> >
>
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-05-19 11:07 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-06 14:01 [PATCH 1/2] mt9v022: fix pixel clock Teresa Gámez
2011-04-06 14:01 ` [PATCH 2/2] mt9m111: " Teresa Gámez
2011-04-07 11:25 ` Guennadi Liakhovetski
2011-05-18 11:12 ` Guennadi Liakhovetski
2011-05-19 11:07 ` Teresa Gamez
2011-04-07 11:08 ` [PATCH 1/2] mt9v022: " Guennadi Liakhovetski
2011-04-07 12:38 ` Teresa Gamez
[not found] ` <OF0E7310A6.B4F9559D-ONC125786B.003E2F29-C125786B.004202D7@phytec.de>
2011-04-07 12:41 ` Antwort: " Guennadi Liakhovetski
2011-04-08 13:27 ` Teresa Gamez
2011-04-12 11:22 ` Teresa Gamez
2011-04-13 6:31 ` Guennadi Liakhovetski
2011-04-14 14:33 ` Teresa Gamez
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.