* [PATCH] mx3fb: Fix parameter to sdc_init_panel
@ 2011-02-10 10:05 Alexander Stein
2011-02-10 10:44 ` Guennadi Liakhovetski
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Stein @ 2011-02-10 10:05 UTC (permalink / raw)
To: linux-fbdev
h_end_width and v_end_width should only contain the front porches.
Otherwise, SCREEN_WIDTH (pixel clocks between HSYNC) in SDC_HOR_CONF
and SCREEN_HEIGHT (rows between VSYNC) in SDC_VER_CONF get false values.
Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
drivers/video/mx3fb.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index ef71e56..9040833 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -881,12 +881,11 @@ static int __set_par(struct fb_info *fbi, bool lock)
IPU_PIX_FMT_BGR666 : IPU_PIX_FMT_RGB666,
fbi->var.left_margin,
fbi->var.hsync_len,
- fbi->var.right_margin +
- fbi->var.hsync_len,
+ fbi->var.right_margin,
fbi->var.upper_margin,
fbi->var.vsync_len,
- fbi->var.lower_margin +
- fbi->var.vsync_len, sig_cfg) != 0) {
+ fbi->var.lower_margin,
+ sig_cfg) != 0) {
dev_err(fbi->device,
"mx3fb: Error initializing panel.\n");
return -EINVAL;
--
1.7.3.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] mx3fb: Fix parameter to sdc_init_panel
2011-02-10 10:05 [PATCH] mx3fb: Fix parameter to sdc_init_panel Alexander Stein
@ 2011-02-10 10:44 ` Guennadi Liakhovetski
0 siblings, 0 replies; 19+ messages in thread
From: Guennadi Liakhovetski @ 2011-02-10 10:44 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 10 Feb 2011, Alexander Stein wrote:
> h_end_width and v_end_width should only contain the front porches.
> Otherwise, SCREEN_WIDTH (pixel clocks between HSYNC) in SDC_HOR_CONF
> and SCREEN_HEIGHT (rows between VSYNC) in SDC_VER_CONF get false values.
Hm, no, this corrupts image on my pcm990 test-board. What makes you think
this patch is needed? How do you see, that current values are "false?"
Thanks
Guennadi
>
> Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> ---
> drivers/video/mx3fb.c | 7 +++----
> 1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index ef71e56..9040833 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -881,12 +881,11 @@ static int __set_par(struct fb_info *fbi, bool lock)
> IPU_PIX_FMT_BGR666 : IPU_PIX_FMT_RGB666,
> fbi->var.left_margin,
> fbi->var.hsync_len,
> - fbi->var.right_margin +
> - fbi->var.hsync_len,
> + fbi->var.right_margin,
> fbi->var.upper_margin,
> fbi->var.vsync_len,
> - fbi->var.lower_margin +
> - fbi->var.vsync_len, sig_cfg) != 0) {
> + fbi->var.lower_margin,
> + sig_cfg) != 0) {
> dev_err(fbi->device,
> "mx3fb: Error initializing panel.\n");
> return -EINVAL;
> --
> 1.7.3.4
>
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] mx3fb: Fix parameter to sdc_init_panel
@ 2011-02-10 10:44 ` Guennadi Liakhovetski
0 siblings, 0 replies; 19+ messages in thread
From: Guennadi Liakhovetski @ 2011-02-10 10:44 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 10 Feb 2011, Alexander Stein wrote:
> h_end_width and v_end_width should only contain the front porches.
> Otherwise, SCREEN_WIDTH (pixel clocks between HSYNC) in SDC_HOR_CONF
> and SCREEN_HEIGHT (rows between VSYNC) in SDC_VER_CONF get false values.
Hm, no, this corrupts image on my pcm990 test-board. What makes you think
this patch is needed? How do you see, that current values are "false?"
Thanks
Guennadi
>
> Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> ---
> drivers/video/mx3fb.c | 7 +++----
> 1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index ef71e56..9040833 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -881,12 +881,11 @@ static int __set_par(struct fb_info *fbi, bool lock)
> IPU_PIX_FMT_BGR666 : IPU_PIX_FMT_RGB666,
> fbi->var.left_margin,
> fbi->var.hsync_len,
> - fbi->var.right_margin +
> - fbi->var.hsync_len,
> + fbi->var.right_margin,
> fbi->var.upper_margin,
> fbi->var.vsync_len,
> - fbi->var.lower_margin +
> - fbi->var.vsync_len, sig_cfg) != 0) {
> + fbi->var.lower_margin,
> + sig_cfg) != 0) {
> dev_err(fbi->device,
> "mx3fb: Error initializing panel.\n");
> return -EINVAL;
> --
> 1.7.3.4
>
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] mx3fb: Fix parameter to sdc_init_panel
2011-02-10 10:44 ` Guennadi Liakhovetski
@ 2011-02-10 11:00 ` Guennadi Liakhovetski
-1 siblings, 0 replies; 19+ messages in thread
From: Guennadi Liakhovetski @ 2011-02-10 11:00 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 10 Feb 2011, Guennadi Liakhovetski wrote:
> On Thu, 10 Feb 2011, Alexander Stein wrote:
>
> > h_end_width and v_end_width should only contain the front porches.
> > Otherwise, SCREEN_WIDTH (pixel clocks between HSYNC) in SDC_HOR_CONF
> > and SCREEN_HEIGHT (rows between VSYNC) in SDC_VER_CONF get false values.
>
> Hm, no, this corrupts image on my pcm990 test-board. What makes you think
> this patch is needed? How do you see, that current values are "false?"
>
> Thanks
> Guennadi
>
> >
> > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> > ---
> > drivers/video/mx3fb.c | 7 +++----
> > 1 files changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> > index ef71e56..9040833 100644
> > --- a/drivers/video/mx3fb.c
> > +++ b/drivers/video/mx3fb.c
> > @@ -881,12 +881,11 @@ static int __set_par(struct fb_info *fbi, bool lock)
Also just occurred to me - your offset of 881 lines is 100 lines below my
offset for the same code - against what tree is this patch, or do you have
some local changes there? Can it be, that that's the reason for your
problem?
Thanks
Guennadi
> > IPU_PIX_FMT_BGR666 : IPU_PIX_FMT_RGB666,
> > fbi->var.left_margin,
> > fbi->var.hsync_len,
> > - fbi->var.right_margin +
> > - fbi->var.hsync_len,
> > + fbi->var.right_margin,
> > fbi->var.upper_margin,
> > fbi->var.vsync_len,
> > - fbi->var.lower_margin +
> > - fbi->var.vsync_len, sig_cfg) != 0) {
> > + fbi->var.lower_margin,
> > + sig_cfg) != 0) {
> > dev_err(fbi->device,
> > "mx3fb: Error initializing panel.\n");
> > return -EINVAL;
> > --
> > 1.7.3.4
> >
> >
>
> ---
> 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] 19+ messages in thread
* [PATCH] mx3fb: Fix parameter to sdc_init_panel
@ 2011-02-10 11:00 ` Guennadi Liakhovetski
0 siblings, 0 replies; 19+ messages in thread
From: Guennadi Liakhovetski @ 2011-02-10 11:00 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 10 Feb 2011, Guennadi Liakhovetski wrote:
> On Thu, 10 Feb 2011, Alexander Stein wrote:
>
> > h_end_width and v_end_width should only contain the front porches.
> > Otherwise, SCREEN_WIDTH (pixel clocks between HSYNC) in SDC_HOR_CONF
> > and SCREEN_HEIGHT (rows between VSYNC) in SDC_VER_CONF get false values.
>
> Hm, no, this corrupts image on my pcm990 test-board. What makes you think
> this patch is needed? How do you see, that current values are "false?"
>
> Thanks
> Guennadi
>
> >
> > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> > ---
> > drivers/video/mx3fb.c | 7 +++----
> > 1 files changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> > index ef71e56..9040833 100644
> > --- a/drivers/video/mx3fb.c
> > +++ b/drivers/video/mx3fb.c
> > @@ -881,12 +881,11 @@ static int __set_par(struct fb_info *fbi, bool lock)
Also just occurred to me - your offset of 881 lines is 100 lines below my
offset for the same code - against what tree is this patch, or do you have
some local changes there? Can it be, that that's the reason for your
problem?
Thanks
Guennadi
> > IPU_PIX_FMT_BGR666 : IPU_PIX_FMT_RGB666,
> > fbi->var.left_margin,
> > fbi->var.hsync_len,
> > - fbi->var.right_margin +
> > - fbi->var.hsync_len,
> > + fbi->var.right_margin,
> > fbi->var.upper_margin,
> > fbi->var.vsync_len,
> > - fbi->var.lower_margin +
> > - fbi->var.vsync_len, sig_cfg) != 0) {
> > + fbi->var.lower_margin,
> > + sig_cfg) != 0) {
> > dev_err(fbi->device,
> > "mx3fb: Error initializing panel.\n");
> > return -EINVAL;
> > --
> > 1.7.3.4
> >
> >
>
> ---
> 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] 19+ messages in thread
* Re: [PATCH] mx3fb: Fix parameter to sdc_init_panel
2011-02-10 11:00 ` Guennadi Liakhovetski
@ 2011-02-10 11:27 ` Alexander Stein
-1 siblings, 0 replies; 19+ messages in thread
From: Alexander Stein @ 2011-02-10 11:27 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 10 February 2011, 12:00:09 Guennadi Liakhovetski wrote:
> On Thu, 10 Feb 2011, Guennadi Liakhovetski wrote:
> > On Thu, 10 Feb 2011, Alexander Stein wrote:
> > > h_end_width and v_end_width should only contain the front porches.
> > > Otherwise, SCREEN_WIDTH (pixel clocks between HSYNC) in SDC_HOR_CONF
> > > and SCREEN_HEIGHT (rows between VSYNC) in SDC_VER_CONF get false
> > > values.
> >
> > Hm, no, this corrupts image on my pcm990 test-board. What makes you think
> > this patch is needed? How do you see, that current values are "false?"
Ok, i will just write about HSYNC the problem about VSYNC is similar.
Let's assume we have a display with the following properties.
left_margin = 38
right_margin = 32
hsync_len = 20
width = 800
Without the patch we get the following arguments on sdc_init_panel
width = 800
h_start_width = 38 (left_margin)
h_sync_width = 20 (hsync_len)
h_end_width = 52 = 32 + 20 (right_margin + hsync_len)
So we will write into SDC_HOR_CONF:
reg = ((h_sync_width - 1) << 26) |
((width + h_start_width + h_end_width - 1) << 16);
reg = ((20 - 1) << 26) |
((800 + 38 + 52 - 1) << 16);
So the value SCREEN_WIDTH get written is
width + left_margin + right_margin + hsync_len
which is IMO wrong. The description in the reference manual states:
> Screen width minus 1. Specifies the number of pixel clock periods between
> the last HSYNC and the new HSYNC.
It should just be: width + left_margin + right_margin
I fell accross it, as I have a display with 800 px width and rather big
left_margin. The sum got greater than 1023 which is what SCREEN_WIDTH can hold
at maximum.
I don't know which display you used, but I guess your right_margin (and
lower_margin) needs to be adjusted.
> > > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> > > ---
> > >
> > > drivers/video/mx3fb.c | 7 +++----
> > > 1 files changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> > > index ef71e56..9040833 100644
> > > --- a/drivers/video/mx3fb.c
> > > +++ b/drivers/video/mx3fb.c
> > > @@ -881,12 +881,11 @@ static int __set_par(struct fb_info *fbi, bool
> > > lock)
>
> Also just occurred to me - your offset of 881 lines is 100 lines below my
> offset for the same code - against what tree is this patch, or do you have
> some local changes there? Can it be, that that's the reason for your
> problem?
Oh, I didn't rebase this patch on current linux git. This patch is on my
2.6.34 kernel. Anyway, since 2.6.34 there are no related patches on mx3fb.c.
Alexander
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] mx3fb: Fix parameter to sdc_init_panel
@ 2011-02-10 11:27 ` Alexander Stein
0 siblings, 0 replies; 19+ messages in thread
From: Alexander Stein @ 2011-02-10 11:27 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 10 February 2011, 12:00:09 Guennadi Liakhovetski wrote:
> On Thu, 10 Feb 2011, Guennadi Liakhovetski wrote:
> > On Thu, 10 Feb 2011, Alexander Stein wrote:
> > > h_end_width and v_end_width should only contain the front porches.
> > > Otherwise, SCREEN_WIDTH (pixel clocks between HSYNC) in SDC_HOR_CONF
> > > and SCREEN_HEIGHT (rows between VSYNC) in SDC_VER_CONF get false
> > > values.
> >
> > Hm, no, this corrupts image on my pcm990 test-board. What makes you think
> > this patch is needed? How do you see, that current values are "false?"
Ok, i will just write about HSYNC the problem about VSYNC is similar.
Let's assume we have a display with the following properties.
left_margin = 38
right_margin = 32
hsync_len = 20
width = 800
Without the patch we get the following arguments on sdc_init_panel
width = 800
h_start_width = 38 (left_margin)
h_sync_width = 20 (hsync_len)
h_end_width = 52 = 32 + 20 (right_margin + hsync_len)
So we will write into SDC_HOR_CONF:
reg = ((h_sync_width - 1) << 26) |
((width + h_start_width + h_end_width - 1) << 16);
reg = ((20 - 1) << 26) |
((800 + 38 + 52 - 1) << 16);
So the value SCREEN_WIDTH get written is
width + left_margin + right_margin + hsync_len
which is IMO wrong. The description in the reference manual states:
> Screen width minus 1. Specifies the number of pixel clock periods between
> the last HSYNC and the new HSYNC.
It should just be: width + left_margin + right_margin
I fell accross it, as I have a display with 800 px width and rather big
left_margin. The sum got greater than 1023 which is what SCREEN_WIDTH can hold
at maximum.
I don't know which display you used, but I guess your right_margin (and
lower_margin) needs to be adjusted.
> > > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> > > ---
> > >
> > > drivers/video/mx3fb.c | 7 +++----
> > > 1 files changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> > > index ef71e56..9040833 100644
> > > --- a/drivers/video/mx3fb.c
> > > +++ b/drivers/video/mx3fb.c
> > > @@ -881,12 +881,11 @@ static int __set_par(struct fb_info *fbi, bool
> > > lock)
>
> Also just occurred to me - your offset of 881 lines is 100 lines below my
> offset for the same code - against what tree is this patch, or do you have
> some local changes there? Can it be, that that's the reason for your
> problem?
Oh, I didn't rebase this patch on current linux git. This patch is on my
2.6.34 kernel. Anyway, since 2.6.34 there are no related patches on mx3fb.c.
Alexander
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] mx3fb: Fix parameter to sdc_init_panel
2011-02-10 11:27 ` Alexander Stein
@ 2011-02-10 12:48 ` Guennadi Liakhovetski
-1 siblings, 0 replies; 19+ messages in thread
From: Guennadi Liakhovetski @ 2011-02-10 12:48 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 10 Feb 2011, Alexander Stein wrote:
> On Thursday 10 February 2011, 12:00:09 Guennadi Liakhovetski wrote:
> > On Thu, 10 Feb 2011, Guennadi Liakhovetski wrote:
> > > On Thu, 10 Feb 2011, Alexander Stein wrote:
> > > > h_end_width and v_end_width should only contain the front porches.
> > > > Otherwise, SCREEN_WIDTH (pixel clocks between HSYNC) in SDC_HOR_CONF
> > > > and SCREEN_HEIGHT (rows between VSYNC) in SDC_VER_CONF get false
> > > > values.
> > >
> > > Hm, no, this corrupts image on my pcm990 test-board. What makes you think
> > > this patch is needed? How do you see, that current values are "false?"
(I certainly meant the pcm037 module)
> Ok, i will just write about HSYNC the problem about VSYNC is similar.
> Let's assume we have a display with the following properties.
> left_margin = 38
> right_margin = 32
> hsync_len = 20
> width = 800
>
> Without the patch we get the following arguments on sdc_init_panel
> width = 800
> h_start_width = 38 (left_margin)
> h_sync_width = 20 (hsync_len)
> h_end_width = 52 = 32 + 20 (right_margin + hsync_len)
>
> So we will write into SDC_HOR_CONF:
>
> reg = ((h_sync_width - 1) << 26) |
> ((width + h_start_width + h_end_width - 1) << 16);
> reg = ((20 - 1) << 26) |
> ((800 + 38 + 52 - 1) << 16);
>
> So the value SCREEN_WIDTH get written is
> width + left_margin + right_margin + hsync_len
> which is IMO wrong. The description in the reference manual states:
> > Screen width minus 1. Specifies the number of pixel clock periods between
> > the last HSYNC and the new HSYNC.
> It should just be: width + left_margin + right_margin
> I fell accross it, as I have a display with 800 px width and rather big
> left_margin. The sum got greater than 1023 which is what SCREEN_WIDTH can hold
> at maximum.
>
> I don't know which display you used, but I guess your right_margin (and
> lower_margin) needs to be adjusted.
Ok, agree. But then you need to adjust all current users and have them
tested... Otherwise, of course, you can just adjust your platform panel
data to work with the current calculations, even though in principle your
patch seems to be doing the right thing (tm), according to the manual. But
if applied as is without fixing all the users, it can very well break
them...
Thanks
Guennadi
> > > > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> > > > ---
> > > >
> > > > drivers/video/mx3fb.c | 7 +++----
> > > > 1 files changed, 3 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> > > > index ef71e56..9040833 100644
> > > > --- a/drivers/video/mx3fb.c
> > > > +++ b/drivers/video/mx3fb.c
> > > > @@ -881,12 +881,11 @@ static int __set_par(struct fb_info *fbi, bool
> > > > lock)
> >
> > Also just occurred to me - your offset of 881 lines is 100 lines below my
> > offset for the same code - against what tree is this patch, or do you have
> > some local changes there? Can it be, that that's the reason for your
> > problem?
>
> Oh, I didn't rebase this patch on current linux git. This patch is on my
> 2.6.34 kernel. Anyway, since 2.6.34 there are no related patches on mx3fb.c.
>
> Alexander
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] mx3fb: Fix parameter to sdc_init_panel
@ 2011-02-10 12:48 ` Guennadi Liakhovetski
0 siblings, 0 replies; 19+ messages in thread
From: Guennadi Liakhovetski @ 2011-02-10 12:48 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 10 Feb 2011, Alexander Stein wrote:
> On Thursday 10 February 2011, 12:00:09 Guennadi Liakhovetski wrote:
> > On Thu, 10 Feb 2011, Guennadi Liakhovetski wrote:
> > > On Thu, 10 Feb 2011, Alexander Stein wrote:
> > > > h_end_width and v_end_width should only contain the front porches.
> > > > Otherwise, SCREEN_WIDTH (pixel clocks between HSYNC) in SDC_HOR_CONF
> > > > and SCREEN_HEIGHT (rows between VSYNC) in SDC_VER_CONF get false
> > > > values.
> > >
> > > Hm, no, this corrupts image on my pcm990 test-board. What makes you think
> > > this patch is needed? How do you see, that current values are "false?"
(I certainly meant the pcm037 module)
> Ok, i will just write about HSYNC the problem about VSYNC is similar.
> Let's assume we have a display with the following properties.
> left_margin = 38
> right_margin = 32
> hsync_len = 20
> width = 800
>
> Without the patch we get the following arguments on sdc_init_panel
> width = 800
> h_start_width = 38 (left_margin)
> h_sync_width = 20 (hsync_len)
> h_end_width = 52 = 32 + 20 (right_margin + hsync_len)
>
> So we will write into SDC_HOR_CONF:
>
> reg = ((h_sync_width - 1) << 26) |
> ((width + h_start_width + h_end_width - 1) << 16);
> reg = ((20 - 1) << 26) |
> ((800 + 38 + 52 - 1) << 16);
>
> So the value SCREEN_WIDTH get written is
> width + left_margin + right_margin + hsync_len
> which is IMO wrong. The description in the reference manual states:
> > Screen width minus 1. Specifies the number of pixel clock periods between
> > the last HSYNC and the new HSYNC.
> It should just be: width + left_margin + right_margin
> I fell accross it, as I have a display with 800 px width and rather big
> left_margin. The sum got greater than 1023 which is what SCREEN_WIDTH can hold
> at maximum.
>
> I don't know which display you used, but I guess your right_margin (and
> lower_margin) needs to be adjusted.
Ok, agree. But then you need to adjust all current users and have them
tested... Otherwise, of course, you can just adjust your platform panel
data to work with the current calculations, even though in principle your
patch seems to be doing the right thing (tm), according to the manual. But
if applied as is without fixing all the users, it can very well break
them...
Thanks
Guennadi
> > > > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> > > > ---
> > > >
> > > > drivers/video/mx3fb.c | 7 +++----
> > > > 1 files changed, 3 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> > > > index ef71e56..9040833 100644
> > > > --- a/drivers/video/mx3fb.c
> > > > +++ b/drivers/video/mx3fb.c
> > > > @@ -881,12 +881,11 @@ static int __set_par(struct fb_info *fbi, bool
> > > > lock)
> >
> > Also just occurred to me - your offset of 881 lines is 100 lines below my
> > offset for the same code - against what tree is this patch, or do you have
> > some local changes there? Can it be, that that's the reason for your
> > problem?
>
> Oh, I didn't rebase this patch on current linux git. This patch is on my
> 2.6.34 kernel. Anyway, since 2.6.34 there are no related patches on mx3fb.c.
>
> Alexander
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] mx3fb: Fix parameter to sdc_init_panel
2011-02-10 12:48 ` Guennadi Liakhovetski
@ 2011-02-10 13:01 ` Alexander Stein
-1 siblings, 0 replies; 19+ messages in thread
From: Alexander Stein @ 2011-02-10 13:01 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 10 February 2011, 13:48:23 Guennadi Liakhovetski wrote:
> > Ok, i will just write about HSYNC the problem about VSYNC is similar.
> > Let's assume we have a display with the following properties.
> > left_margin = 38
> > right_margin = 32
> > hsync_len = 20
> > width = 800
> >
> > Without the patch we get the following arguments on sdc_init_panel
> > width = 800
> > h_start_width = 38 (left_margin)
> > h_sync_width = 20 (hsync_len)
> > h_end_width = 52 = 32 + 20 (right_margin + hsync_len)
> >
> > So we will write into SDC_HOR_CONF:
> >
> > reg = ((h_sync_width - 1) << 26) |
> >
> > ((width + h_start_width + h_end_width - 1) << 16);
> >
> > reg = ((20 - 1) << 26) |
> >
> > ((800 + 38 + 52 - 1) << 16);
> >
> > So the value SCREEN_WIDTH get written is
> > width + left_margin + right_margin + hsync_len
> >
> > which is IMO wrong. The description in the reference manual states:
> > > Screen width minus 1. Specifies the number of pixel clock periods
> > > between the last HSYNC and the new HSYNC.
> >
> > It should just be: width + left_margin + right_margin
> > I fell accross it, as I have a display with 800 px width and rather big
> > left_margin. The sum got greater than 1023 which is what SCREEN_WIDTH can
> > hold at maximum.
> >
> > I don't know which display you used, but I guess your right_margin (and
> > lower_margin) needs to be adjusted.
>
> Ok, agree. But then you need to adjust all current users and have them
> tested... Otherwise, of course, you can just adjust your platform panel
> data to work with the current calculations, even though in principle your
> patch seems to be doing the right thing (tm), according to the manual. But
> if applied as is without fixing all the users, it can very well break
> them...
Ok, I've just seen there are some model defines in mx3fb.c which would also be
affected.
I'll try to find all mx3fb users and fix their videomodes and repost a new
patch.
Alexander
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] mx3fb: Fix parameter to sdc_init_panel
@ 2011-02-10 13:01 ` Alexander Stein
0 siblings, 0 replies; 19+ messages in thread
From: Alexander Stein @ 2011-02-10 13:01 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 10 February 2011, 13:48:23 Guennadi Liakhovetski wrote:
> > Ok, i will just write about HSYNC the problem about VSYNC is similar.
> > Let's assume we have a display with the following properties.
> > left_margin = 38
> > right_margin = 32
> > hsync_len = 20
> > width = 800
> >
> > Without the patch we get the following arguments on sdc_init_panel
> > width = 800
> > h_start_width = 38 (left_margin)
> > h_sync_width = 20 (hsync_len)
> > h_end_width = 52 = 32 + 20 (right_margin + hsync_len)
> >
> > So we will write into SDC_HOR_CONF:
> >
> > reg = ((h_sync_width - 1) << 26) |
> >
> > ((width + h_start_width + h_end_width - 1) << 16);
> >
> > reg = ((20 - 1) << 26) |
> >
> > ((800 + 38 + 52 - 1) << 16);
> >
> > So the value SCREEN_WIDTH get written is
> > width + left_margin + right_margin + hsync_len
> >
> > which is IMO wrong. The description in the reference manual states:
> > > Screen width minus 1. Specifies the number of pixel clock periods
> > > between the last HSYNC and the new HSYNC.
> >
> > It should just be: width + left_margin + right_margin
> > I fell accross it, as I have a display with 800 px width and rather big
> > left_margin. The sum got greater than 1023 which is what SCREEN_WIDTH can
> > hold at maximum.
> >
> > I don't know which display you used, but I guess your right_margin (and
> > lower_margin) needs to be adjusted.
>
> Ok, agree. But then you need to adjust all current users and have them
> tested... Otherwise, of course, you can just adjust your platform panel
> data to work with the current calculations, even though in principle your
> patch seems to be doing the right thing (tm), according to the manual. But
> if applied as is without fixing all the users, it can very well break
> them...
Ok, I've just seen there are some model defines in mx3fb.c which would also be
affected.
I'll try to find all mx3fb users and fix their videomodes and repost a new
patch.
Alexander
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] mx3fb: Fix parameter to sdc_init_panel
2011-02-10 13:01 ` Alexander Stein
@ 2011-02-14 7:46 ` Alexander Stein
-1 siblings, 0 replies; 19+ messages in thread
From: Alexander Stein @ 2011-02-14 7:46 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 10 February 2011, 14:01:17 Alexander Stein wrote:
> On Thursday 10 February 2011, 13:48:23 Guennadi Liakhovetski wrote:
> > > I don't know which display you used, but I guess your right_margin (and
> > > lower_margin) needs to be adjusted.
> >
> > Ok, agree. But then you need to adjust all current users and have them
> > tested... Otherwise, of course, you can just adjust your platform panel
> > data to work with the current calculations, even though in principle your
> > patch seems to be doing the right thing (tm), according to the manual.
> > But if applied as is without fixing all the users, it can very well
> > break them...
>
> Ok, I've just seen there are some model defines in mx3fb.c which would also
> be affected.
> I'll try to find all mx3fb users and fix their videomodes and repost a new
> patch.
Uh, this seems next to impossible for me to handle it my own. I checked some
users of the mx3fb but I cant say which ones will be affected and which ones
not (e.g. without any hsync/vsync, on data enable).
I've not idea how to fix it then.
Alexander
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] mx3fb: Fix parameter to sdc_init_panel
@ 2011-02-14 7:46 ` Alexander Stein
0 siblings, 0 replies; 19+ messages in thread
From: Alexander Stein @ 2011-02-14 7:46 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 10 February 2011, 14:01:17 Alexander Stein wrote:
> On Thursday 10 February 2011, 13:48:23 Guennadi Liakhovetski wrote:
> > > I don't know which display you used, but I guess your right_margin (and
> > > lower_margin) needs to be adjusted.
> >
> > Ok, agree. But then you need to adjust all current users and have them
> > tested... Otherwise, of course, you can just adjust your platform panel
> > data to work with the current calculations, even though in principle your
> > patch seems to be doing the right thing (tm), according to the manual.
> > But if applied as is without fixing all the users, it can very well
> > break them...
>
> Ok, I've just seen there are some model defines in mx3fb.c which would also
> be affected.
> I'll try to find all mx3fb users and fix their videomodes and repost a new
> patch.
Uh, this seems next to impossible for me to handle it my own. I checked some
users of the mx3fb but I cant say which ones will be affected and which ones
not (e.g. without any hsync/vsync, on data enable).
I've not idea how to fix it then.
Alexander
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] mx3fb: Fix parameter to sdc_init_panel
2011-02-14 7:46 ` Alexander Stein
@ 2011-02-14 7:57 ` Guennadi Liakhovetski
-1 siblings, 0 replies; 19+ messages in thread
From: Guennadi Liakhovetski @ 2011-02-14 7:57 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 14 Feb 2011, Alexander Stein wrote:
> On Thursday 10 February 2011, 14:01:17 Alexander Stein wrote:
> > On Thursday 10 February 2011, 13:48:23 Guennadi Liakhovetski wrote:
> > > > I don't know which display you used, but I guess your right_margin (and
> > > > lower_margin) needs to be adjusted.
> > >
> > > Ok, agree. But then you need to adjust all current users and have them
> > > tested... Otherwise, of course, you can just adjust your platform panel
> > > data to work with the current calculations, even though in principle your
> > > patch seems to be doing the right thing (tm), according to the manual.
> > > But if applied as is without fixing all the users, it can very well
> > > break them...
> >
> > Ok, I've just seen there are some model defines in mx3fb.c which would also
> > be affected.
> > I'll try to find all mx3fb users and fix their videomodes and repost a new
> > patch.
>
> Uh, this seems next to impossible for me to handle it my own. I checked some
> users of the mx3fb but I cant say which ones will be affected and which ones
> not (e.g. without any hsync/vsync, on data enable).
> I've not idea how to fix it then.
Well, you can, of course, adjust parameters for your platform to be able
to run with this incorrect parameter definition...
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] mx3fb: Fix parameter to sdc_init_panel
@ 2011-02-14 7:57 ` Guennadi Liakhovetski
0 siblings, 0 replies; 19+ messages in thread
From: Guennadi Liakhovetski @ 2011-02-14 7:57 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 14 Feb 2011, Alexander Stein wrote:
> On Thursday 10 February 2011, 14:01:17 Alexander Stein wrote:
> > On Thursday 10 February 2011, 13:48:23 Guennadi Liakhovetski wrote:
> > > > I don't know which display you used, but I guess your right_margin (and
> > > > lower_margin) needs to be adjusted.
> > >
> > > Ok, agree. But then you need to adjust all current users and have them
> > > tested... Otherwise, of course, you can just adjust your platform panel
> > > data to work with the current calculations, even though in principle your
> > > patch seems to be doing the right thing (tm), according to the manual.
> > > But if applied as is without fixing all the users, it can very well
> > > break them...
> >
> > Ok, I've just seen there are some model defines in mx3fb.c which would also
> > be affected.
> > I'll try to find all mx3fb users and fix their videomodes and repost a new
> > patch.
>
> Uh, this seems next to impossible for me to handle it my own. I checked some
> users of the mx3fb but I cant say which ones will be affected and which ones
> not (e.g. without any hsync/vsync, on data enable).
> I've not idea how to fix it then.
Well, you can, of course, adjust parameters for your platform to be able
to run with this incorrect parameter definition...
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] mx3fb: Fix parameter to sdc_init_panel
2011-02-14 7:57 ` Guennadi Liakhovetski
@ 2011-02-14 12:06 ` Alexander Stein
-1 siblings, 0 replies; 19+ messages in thread
From: Alexander Stein @ 2011-02-14 12:06 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 14 February 2011, 08:57:04 Guennadi Liakhovetski wrote:
> On Mon, 14 Feb 2011, Alexander Stein wrote:
> > On Thursday 10 February 2011, 14:01:17 Alexander Stein wrote:
> > > On Thursday 10 February 2011, 13:48:23 Guennadi Liakhovetski wrote:
> > > > > I don't know which display you used, but I guess your right_margin
> > > > > (and lower_margin) needs to be adjusted.
> > > >
> > > > Ok, agree. But then you need to adjust all current users and have
> > > > them tested... Otherwise, of course, you can just adjust your
> > > > platform panel data to work with the current calculations, even
> > > > though in principle your patch seems to be doing the right thing
> > > > (tm), according to the manual. But if applied as is without fixing
> > > > all the users, it can very well break them...
> > >
> > > Ok, I've just seen there are some model defines in mx3fb.c which would
> > > also be affected.
> > > I'll try to find all mx3fb users and fix their videomodes and repost a
> > > new patch.
> >
> > Uh, this seems next to impossible for me to handle it my own. I checked
> > some users of the mx3fb but I cant say which ones will be affected and
> > which ones not (e.g. without any hsync/vsync, on data enable).
> > I've not idea how to fix it then.
>
> Well, you can, of course, adjust parameters for your platform to be able
> to run with this incorrect parameter definition...
This is just an ugly work-around. Depending on the settings you may even get
an overflow resulting in a not-working display at all.
Alexander
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] mx3fb: Fix parameter to sdc_init_panel
@ 2011-02-14 12:06 ` Alexander Stein
0 siblings, 0 replies; 19+ messages in thread
From: Alexander Stein @ 2011-02-14 12:06 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 14 February 2011, 08:57:04 Guennadi Liakhovetski wrote:
> On Mon, 14 Feb 2011, Alexander Stein wrote:
> > On Thursday 10 February 2011, 14:01:17 Alexander Stein wrote:
> > > On Thursday 10 February 2011, 13:48:23 Guennadi Liakhovetski wrote:
> > > > > I don't know which display you used, but I guess your right_margin
> > > > > (and lower_margin) needs to be adjusted.
> > > >
> > > > Ok, agree. But then you need to adjust all current users and have
> > > > them tested... Otherwise, of course, you can just adjust your
> > > > platform panel data to work with the current calculations, even
> > > > though in principle your patch seems to be doing the right thing
> > > > (tm), according to the manual. But if applied as is without fixing
> > > > all the users, it can very well break them...
> > >
> > > Ok, I've just seen there are some model defines in mx3fb.c which would
> > > also be affected.
> > > I'll try to find all mx3fb users and fix their videomodes and repost a
> > > new patch.
> >
> > Uh, this seems next to impossible for me to handle it my own. I checked
> > some users of the mx3fb but I cant say which ones will be affected and
> > which ones not (e.g. without any hsync/vsync, on data enable).
> > I've not idea how to fix it then.
>
> Well, you can, of course, adjust parameters for your platform to be able
> to run with this incorrect parameter definition...
This is just an ugly work-around. Depending on the settings you may even get
an overflow resulting in a not-working display at all.
Alexander
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] mx3fb: Fix parameter to sdc_init_panel
2011-02-14 12:06 ` Alexander Stein
@ 2011-02-16 13:47 ` Guennadi Liakhovetski
-1 siblings, 0 replies; 19+ messages in thread
From: Guennadi Liakhovetski @ 2011-02-16 13:47 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 14 Feb 2011, Alexander Stein wrote:
> On Monday 14 February 2011, 08:57:04 Guennadi Liakhovetski wrote:
> > On Mon, 14 Feb 2011, Alexander Stein wrote:
> > > On Thursday 10 February 2011, 14:01:17 Alexander Stein wrote:
> > > > On Thursday 10 February 2011, 13:48:23 Guennadi Liakhovetski wrote:
> > > > > > I don't know which display you used, but I guess your right_margin
> > > > > > (and lower_margin) needs to be adjusted.
> > > > >
> > > > > Ok, agree. But then you need to adjust all current users and have
> > > > > them tested... Otherwise, of course, you can just adjust your
> > > > > platform panel data to work with the current calculations, even
> > > > > though in principle your patch seems to be doing the right thing
> > > > > (tm), according to the manual. But if applied as is without fixing
> > > > > all the users, it can very well break them...
> > > >
> > > > Ok, I've just seen there are some model defines in mx3fb.c which would
> > > > also be affected.
> > > > I'll try to find all mx3fb users and fix their videomodes and repost a
> > > > new patch.
> > >
> > > Uh, this seems next to impossible for me to handle it my own. I checked
> > > some users of the mx3fb but I cant say which ones will be affected and
> > > which ones not (e.g. without any hsync/vsync, on data enable).
> > > I've not idea how to fix it then.
> >
> > Well, you can, of course, adjust parameters for your platform to be able
> > to run with this incorrect parameter definition...
>
> This is just an ugly work-around. Depending on the settings you may even get
> an overflow resulting in a not-working display at all.
Sure, I know, that this is a work-around. But I don't see options apart
from
1. fix the driver and maybe a couple of platforms, that we have at hand
and hope, that others will fix theirs or shout loud enough, when they
discover, that their platforms are broken
2. fix the driver and all users and try to get as many of them as we can
to test our fixes
3. leave the driver and work around the bug in all new platforms
Choose your poison;)
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] mx3fb: Fix parameter to sdc_init_panel
@ 2011-02-16 13:47 ` Guennadi Liakhovetski
0 siblings, 0 replies; 19+ messages in thread
From: Guennadi Liakhovetski @ 2011-02-16 13:47 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 14 Feb 2011, Alexander Stein wrote:
> On Monday 14 February 2011, 08:57:04 Guennadi Liakhovetski wrote:
> > On Mon, 14 Feb 2011, Alexander Stein wrote:
> > > On Thursday 10 February 2011, 14:01:17 Alexander Stein wrote:
> > > > On Thursday 10 February 2011, 13:48:23 Guennadi Liakhovetski wrote:
> > > > > > I don't know which display you used, but I guess your right_margin
> > > > > > (and lower_margin) needs to be adjusted.
> > > > >
> > > > > Ok, agree. But then you need to adjust all current users and have
> > > > > them tested... Otherwise, of course, you can just adjust your
> > > > > platform panel data to work with the current calculations, even
> > > > > though in principle your patch seems to be doing the right thing
> > > > > (tm), according to the manual. But if applied as is without fixing
> > > > > all the users, it can very well break them...
> > > >
> > > > Ok, I've just seen there are some model defines in mx3fb.c which would
> > > > also be affected.
> > > > I'll try to find all mx3fb users and fix their videomodes and repost a
> > > > new patch.
> > >
> > > Uh, this seems next to impossible for me to handle it my own. I checked
> > > some users of the mx3fb but I cant say which ones will be affected and
> > > which ones not (e.g. without any hsync/vsync, on data enable).
> > > I've not idea how to fix it then.
> >
> > Well, you can, of course, adjust parameters for your platform to be able
> > to run with this incorrect parameter definition...
>
> This is just an ugly work-around. Depending on the settings you may even get
> an overflow resulting in a not-working display at all.
Sure, I know, that this is a work-around. But I don't see options apart
from
1. fix the driver and maybe a couple of platforms, that we have at hand
and hope, that others will fix theirs or shout loud enough, when they
discover, that their platforms are broken
2. fix the driver and all users and try to get as many of them as we can
to test our fixes
3. leave the driver and work around the bug in all new platforms
Choose your poison;)
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2011-02-16 13:47 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-10 10:05 [PATCH] mx3fb: Fix parameter to sdc_init_panel Alexander Stein
2011-02-10 10:44 ` Guennadi Liakhovetski
2011-02-10 10:44 ` Guennadi Liakhovetski
2011-02-10 11:00 ` Guennadi Liakhovetski
2011-02-10 11:00 ` Guennadi Liakhovetski
2011-02-10 11:27 ` Alexander Stein
2011-02-10 11:27 ` Alexander Stein
2011-02-10 12:48 ` Guennadi Liakhovetski
2011-02-10 12:48 ` Guennadi Liakhovetski
2011-02-10 13:01 ` Alexander Stein
2011-02-10 13:01 ` Alexander Stein
2011-02-14 7:46 ` Alexander Stein
2011-02-14 7:46 ` Alexander Stein
2011-02-14 7:57 ` Guennadi Liakhovetski
2011-02-14 7:57 ` Guennadi Liakhovetski
2011-02-14 12:06 ` Alexander Stein
2011-02-14 12:06 ` Alexander Stein
2011-02-16 13:47 ` Guennadi Liakhovetski
2011-02-16 13:47 ` Guennadi Liakhovetski
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.