Bug 13285

(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Tue, 12 May 2009 01:40:48 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=13285
> 
>            Summary: INTELFB: Colors display incorrectly
>            Product: Drivers
>            Version: 2.5
>     Kernel Version: 2.6.30-rc5

This is a post-2.6.29 regression.

>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Console/Framebuffers
>         AssignedTo: jsimmons@infradead.org
>         ReportedBy: samanddeanus@yahoo.com
>         Regression: Yes
> 
> 
> On my system, the colors are displaying incorrectly when using the Intel
> framebuffer.  For example the Tux penguin logo has a blue background, and
> when
> I start X11 with the fbdev drivers, the xterm also has a blue background,
> when
> it is set up to have a white background.
> 
> This does not occur in kernel 2.6.29 -- I can see the Tasmanian devil in a
> penguin mask (Tuz) just fine and can view images, etc on the framebuffer.
> 

The only change to drivers/video/intelfb/ since 2.6.29 was
347486bb108fa6e0fd2753c1be3519d6be2516ed ("intelfb: support i854")
which added a device ID.

Do we think that this regression is due to fbdev changes, or to DRI
changes?

Thanks.

Reply-To: krzysztof.h1@poczta.fm

On Tue, 12 May 2009 15:19:34 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> 
> (switched to email.  Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
> 
> On Tue, 12 May 2009 01:40:48 GMT
> bugzilla-daemon@bugzilla.kernel.org wrote:
> > 
> > On my system, the colors are displaying incorrectly when using the Intel
> > framebuffer.  For example the Tux penguin logo has a blue background, and
> when
> > I start X11 with the fbdev drivers, the xterm also has a blue background,
> when
> > it is set up to have a white background.
> > 
> > This does not occur in kernel 2.6.29 -- I can see the Tasmanian devil in a
> > penguin mask (Tuz) just fine and can view images, etc on the framebuffer.
> > 
> 
> The only change to drivers/video/intelfb/ since 2.6.29 was
> 347486bb108fa6e0fd2753c1be3519d6be2516ed ("intelfb: support i854")
> which added a device ID.
> 
> Do we think that this regression is due to fbdev changes, or to DRI
> changes?
> 

There were changes to fbdev common code as well. 

Dean, could you post more info about your system? A good starting point
is an output of the dmesg command.

Kind regards,
Krzysztof

----------------------------------------------------------------------
Dzwonki na komork�!
Sprawdz >> http://link.interia.pl/f2161

Created attachment 21368 [details]
Kernel DMESG

Reply-To: krzysztof.h1@poczta.fm

On Tue, 12 May 2009 15:19:34 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> 
> (switched to email.  Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
> 
> On Tue, 12 May 2009 01:40:48 GMT
> bugzilla-daemon@bugzilla.kernel.org wrote:
> 
> > http://bugzilla.kernel.org/show_bug.cgi?id=13285
> > 
> >            Summary: INTELFB: Colors display incorrectly
> >            Product: Drivers
> >            Version: 2.5
> >     Kernel Version: 2.6.30-rc5
> 
> This is a post-2.6.29 regression.
> 
> >           Platform: All
> >         OS/Version: Linux
> >               Tree: Mainline
> >             Status: NEW
> >           Severity: normal
> >           Priority: P1
> >          Component: Console/Framebuffers
> >         AssignedTo: jsimmons@infradead.org
> >         ReportedBy: samanddeanus@yahoo.com
> >         Regression: Yes
> > 
> > 
> > On my system, the colors are displaying incorrectly when using the Intel
> > framebuffer.  For example the Tux penguin logo has a blue background, and
> when
> > I start X11 with the fbdev drivers, the xterm also has a blue background,
> when
> > it is set up to have a white background.
> > 
> > This does not occur in kernel 2.6.29 -- I can see the Tasmanian devil in a
> > penguin mask (Tuz) just fine and can view images, etc on the framebuffer.
> > 
> 

I have reproduced the bug on Intel 852/855 chipset (Asus A3L laptop). 

It took me long because I had problems with a hard drive and had to wipe it with a low-level format tool.

I am trying to identify cause of the bug. 

Regards,
Krzysztof

----------------------------------------------------------------------
Fantastyczne nagrody do zgarniecia!
Zagraj >> http://link.interia.pl/f2177

Reply-To: krzysztof.h1@poczta.fm

From: Krzysztof Helt <krzysztof.h1@wp.pl>

The intelfb driver sets color map depending on currently active pipe. However, if an LVDS 
display is attached (like in laptop) the active pipe variable is never set. The default value is 
PIPE_A and can be wrong.
Set up the pipe variable during driver initialization after hardware state was read.

I also found by experiment that if both pipes were enabled, the PIPE_B is used (active).

The problem is visible in the 8 bpp mode if colors above 15 are used. The first 16 color
entries are displayed correctly.

Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>

---
This is not a regression. I have reproduced it in the 2.6.28 easily.

Dean, please test the patch.

diff --git a/drivers/video/intelfb/intelfbdrv.c b/drivers/video/intelfb/intelfbdrv.c
index ace14fe..b47f6dd 100644
--- a/drivers/video/intelfb/intelfbdrv.c
+++ b/drivers/video/intelfb/intelfbdrv.c
@@ -871,6 +871,12 @@ static int __devinit intelfb_pci_register(struct pci_dev *pdev,
 
 	intelfbhw_print_hw_state(dinfo, &dinfo->save_state);
 
+	/* Check whether pipe A or pipe B is enabled. */
+	if (dinfo->save_state.pipe_a_conf & PIPECONF_ENABLE)
+		dinfo->pipe = PIPE_A;
+	if (dinfo->save_state.pipe_b_conf & PIPECONF_ENABLE)
+		dinfo->pipe = PIPE_B;
+
 	if (bailearly == 18)
 		bailout(dinfo);
 

----------------------------------------------------------------------
Dzwonki na komork�!
Sprawdz >> http://link.interia.pl/f2161

On Sun, 17 May 2009 08:17:43 +0200 Krzysztof Helt <krzysztof.h1@poczta.fm> wrote:

> From: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> The intelfb driver sets color map depending on currently active pipe.
> However, if an LVDS 
> display is attached (like in laptop) the active pipe variable is never set.
> The default value is 
> PIPE_A and can be wrong.
> Set up the pipe variable during driver initialization after hardware state
> was read.
> 
> I also found by experiment that if both pipes were enabled, the PIPE_B is
> used (active).
> 
> The problem is visible in the 8 bpp mode if colors above 15 are used. The
> first 16 color
> entries are displayed correctly.
> 
> Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> ---
> This is not a regression. I have reproduced it in the 2.6.28 easily.

hm, Dean's original report had

    This does not occur in kernel 2.6.29 -- I can see the Tasmanian
    devil in a penguin mask (Tuz) just fine and can view images, etc on
    the framebuffer.

> Dean, please test the patch.

Yes please.

> diff --git a/drivers/video/intelfb/intelfbdrv.c
> b/drivers/video/intelfb/intelfbdrv.c
> index ace14fe..b47f6dd 100644
> --- a/drivers/video/intelfb/intelfbdrv.c
> +++ b/drivers/video/intelfb/intelfbdrv.c
> @@ -871,6 +871,12 @@ static int __devinit intelfb_pci_register(struct pci_dev
> *pdev,
>  
>       intelfbhw_print_hw_state(dinfo, &dinfo->save_state);
>  
> +     /* Check whether pipe A or pipe B is enabled. */
> +     if (dinfo->save_state.pipe_a_conf & PIPECONF_ENABLE)
> +             dinfo->pipe = PIPE_A;
> +     if (dinfo->save_state.pipe_b_conf & PIPECONF_ENABLE)
> +             dinfo->pipe = PIPE_B;
> +
>       if (bailearly == 18)
>               bailout(dinfo);

Reply-To: krzysztof.h1@poczta.fm

On Sat, 16 May 2009 23:19:32 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Sun, 17 May 2009 08:17:43 +0200 Krzysztof Helt <krzysztof.h1@poczta.fm>
> wrote:
> 
> > This is not a regression. I have reproduced it in the 2.6.28 easily.
> 
> hm, Dean's original report had
> 
>     This does not occur in kernel 2.6.29 -- I can see the Tasmanian
>     devil in a penguin mask (Tuz) just fine and can view images, etc on
>     the framebuffer.
> 

I can confirm that Tuz is also broken on my laptop (kernel v2.6.29).
Maybe Dean had set different color depth ("vga=" parameter) for the older kernel?

The dmesg output for the 2.6.29 would clear any doubts.

Regards,
Krzysztof

----------------------------------------------------------------------
Dzwonki na komork�!
Sprawdz >> http://link.interia.pl/f2161

2009/5/17 Krzysztof Helt <krzysztof.h1@poczta.fm>:
> On Sat, 16 May 2009 23:19:32 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
>
>> On Sun, 17 May 2009 08:17:43 +0200 Krzysztof Helt <krzysztof.h1@poczta.fm>
>> wrote:
>>
>> > This is not a regression. I have reproduced it in the 2.6.28 easily.
>>
>> hm, Dean's original report had
>>
>>     This does not occur in kernel 2.6.29 -- I can see the Tasmanian
>>     devil in a penguin mask (Tuz) just fine and can view images, etc on
>>     the framebuffer.
>>
>
> I can confirm that Tuz is also broken on my laptop (kernel v2.6.29).
> Maybe Dean had set different color depth ("vga=" parameter) for the older
> kernel?
>
> The dmesg output for the 2.6.29 would clear any doubts.
>

For me this is broken ever since 2.6.26 on a Mac Mini with all of
efifb/intelfb/vesafb but perhaps this is a different issue.

I will try to rebuild 2.6.29 with intelfb and the patch to see if that
makes a difference.

Currently efifb does give correct geometry but wrong colours for me,
the other framebuffers would also produce picture with wrong geometry
with 2.6.26.

Thanks

Michal

I can confirm that the patch fixes the bug.

Is there a reason this patch is not yet merged as 2.6.30 is close ?

Dropped from the list of recent regressions on the basis of comment #5.

(In reply to comment #10)
> Is there a reason this patch is not yet merged as 2.6.30 is close ?

Probably no one has pushed it to Linus yet.

Can somebody send me the patch with the sign-offs and 'tested-by's in 
email? I hate picking up patches directly from bugzilla. But considering 
how late in the 2.6.30 cycle we are, and how long this bug has been around 
(apparently at least since 2.6.26), I'd almost prefer to get it after 
2.6.30 is out.

Tha main reason for that is simply that I wonder a bit about that PIPE_B 
thing if both pipes are enabled. It may be PIPE_B for Krzysztof and other 
people who are testing this bug, but what about the potential multitudes 
that have it as PIPE_A and thus are happy with the code as-is?

In other words, the case I'm worried about is when the pipe conf registers 
say that both PIPE_A _and_ PIPE_B are enabled. We used to use PIPE_A in 
that case, the patch makes it use PIPE_B. That's a big change, and it 
might well be that there are tons and tons of users that would complain, 
because for _them_, PIPE_A is the right thing!

IOW, maybe the _real_ bug is that "intelfbhw_setcolreg()" shouldn't set 
just palette color register for the one pipe, but it should look at which 
pipe(s) are enabled, and set them for the enabled pipe(s).

I dunno. I don't know the hardware well enough. Is PIPE_A usually the CRT, 
and PIPE_B the LVDS panel, or what? Has somebody tested booting with both 
a CRT and the panel enabled?

End result: I really don't feel very safe applying the patch as-is. I 
realize that it fixes a bug for people, but I just don't have the 
confidence to say that it won't cause another bug for other people. I 
could apply it early in the next merge window, or I could be convinced 
that it's safe, but as is, I really don't want to just apply it.

		Linus

Reply-To: krzysztof.h1@poczta.fm

On Mon, 25 May 2009 19:14:29 -0700 (PDT)
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 
> 
> Can somebody send me the patch with the sign-offs and 'tested-by's in 
> email? I hate picking up patches directly from bugzilla. But considering 
> how late in the 2.6.30 cycle we are, and how long this bug has been around 
> (apparently at least since 2.6.26), I'd almost prefer to get it after 
> 2.6.30 is out.
> 
> Tha main reason for that is simply that I wonder a bit about that PIPE_B 
> thing if both pipes are enabled. It may be PIPE_B for Krzysztof and other 
> people who are testing this bug, but what about the potential multitudes 
> that have it as PIPE_A and thus are happy with the code as-is?
> 

The previous version has not detected the active pipe. It was always the PIPE_A
regardless how the pipes were set. I will dig into documentation to find out what 
these pipes mean.

> In other words, the case I'm worried about is when the pipe conf registers 
> say that both PIPE_A _and_ PIPE_B are enabled. We used to use PIPE_A in 
> that case, the patch makes it use PIPE_B. That's a big change, and it 
> might well be that there are tons and tons of users that would complain, 
> because for _them_, PIPE_A is the right thing!
> 
> IOW, maybe the _real_ bug is that "intelfbhw_setcolreg()" shouldn't set 
> just palette color register for the one pipe, but it should look at which 
> pipe(s) are enabled, and set them for the enabled pipe(s).
> 

It is done this way indirectly. The pipe is detected in the fb_set_par() function
and then used later. If the module is compiled-in (required for laptops) the
fb_set_par() is never called so the pipe is never detected. The PIPE_A is
simply the default value.

> I dunno. I don't know the hardware well enough. Is PIPE_A usually the CRT, 
> and PIPE_B the LVDS panel, or what? Has somebody tested booting with both 
> a CRT and the panel enabled?
> 

I have tested this. The 2.6.28 kernel gives correct logo on CRT + laptop but broken colors
on the LVDS (laptop built-in LCD). The kernel 2.6.30 with the patch gives correct logos for 
both the CRT and the LVDS.

> End result: I really don't feel very safe applying the patch as-is. I 
> realize that it fixes a bug for people, but I just don't have the 
> confidence to say that it won't cause another bug for other people. I 
> could apply it early in the next merge window, or I could be convinced 
> that it's safe, but as is, I really don't want to just apply it.
> 

The fbdev-devel list is orphaned and Andrew Morton harvest the list
for patches from time to time.
This patch can probably wait till the 2.6.31 to get more people to test this
(Mac mini is a good candidate because it should use the CRT output).

Kind regards,
Krzysztof
PS. I will be unavailable till weekend due to business trip to another country.

----------------------------------------------------------------------
Weekend w gorach za 100 PLN!
Sprawdz >>> http://link.interia.pl/f218c

On Tue, 26 May 2009, Krzysztof Helt wrote:
> 
> The previous version has not detected the active pipe. It was always the
> PIPE_A
> regardless how the pipes were set.

I realize that.

However, it's the "now it's always PIPE_B if both pipes are active" part 
that I don't like.

If the patch looked like

	if (pipe_a_active)
		pipe = PIPE_A;
	else
		pipe = PIPE_B;

then I'd be much less nervous - the patch would give the old semantics for 
when it makes sense.

But the patch is

	if (pipe_a_active)
		pipe = PIPE_A;
	if (pipe_b_active)
		pipe = PIPE_B;

and you apparently tested and it has to be that way for your case (ie you 
want the PIPE_B case even if pipe A was active). 

And that makes me nervous, because now you're introducing the _new_ 
semantics for a case where the old semantics migth well still make sense.

And no, no amount of testing _by_you_, or by anybody who actively sees 
this bugzilla entry as a bug is worth anything at all. Why? Because it's a 
self-selected sample, and BY DEFINITION you wouldn't have anybody testing 
it for whom PIPE_A is the correct pipe!

This is why I'm not at all impressed by "this patch makes the problem go 
away", without some kind of reassurance that it also doesn't introduce 
more problems. 

If you can point to other sources (documentation, X.org, the newer KMS 
intel driver, whatever) that implies that PIPE_B really is the right 
choice when both are enabled, then that would be really good. As it is, I 
don't think that patch is 2.6.30 material.

			Linus

Reply-To: krzysztof.h1@poczta.fm

On Tue, 26 May 2009 11:52:03 -0700 (PDT)
Linus Torvalds <torvalds@linux-foundation.org> wrote:


> If you can point to other sources (documentation, X.org, the newer KMS 
> intel driver, whatever) that implies that PIPE_B really is the right 
> choice when both are enabled, then that would be really good. As it is, I 
> don't think that patch is 2.6.30 material.
> 

Linus is right. The patch is incorrect way to fix the bug. I read the documentation
from http://intellinuxgraphics.org and it is more complicated.

Both, the old and my way of detecting active display (through detecting pipes)
are wrong. I will prepare a new patch for weekend.

Regards,
Krzysztof

 	
----------------------------------------------------------------------
Sprawdz pogode na dzis!
Kliknij >>> http://link.interia.pl/f217d

Reply-To: krzysztof.h1@poczta.fm

From: Krzysztof Helt <krzysztof.h1@wp.pl>

The intelfb driver sets color map depending on currently active pipe. However, if an LVDS 
display is attached (like in laptop) the active pipe variable is never set. The default value is 
PIPE_A and can be wrong.
Set up the pipe variable during driver initialization after hardware state was read.

Also, the detection of the active display (and hence the pipe) is wrong. The pipes are assigned
to so called planes. Both pipes are always enabled on my laptop but only one plane is enabled
(the plane A for the CRT or the plane B for the LVDS). Change active pipe detection code
to take into account a status of the plane assigned to each pipe.

The problem is visible in the 8 bpp mode if colors above 15 are used. The first 16 color
entries are displayed correctly.

The graphics chip description is here (G45 vol. 3):
http://intellinuxgraphics.org/documentation.html

Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>

---
The second version of the fix to this problem. Now, it is much more sophisticated
based on the knowledge gained from documentation available at http://intellinuxgraphics.org/.

It does not change a default behaviour (assumed pipe A) for all cases except the case that only 
the plane assigned to the pipe B is active. It is enough to fix the issue for me.

Please test it.

diff -urp linux-ref/drivers/video/intelfb/intelfbdrv.c linux-2.6.30-rc7/drivers/video/intelfb/intelfbdrv.c
--- linux-ref/drivers/video/intelfb/intelfbdrv.c	2009-05-23 23:47:00.000000000 +0200
+++ linux-2.6.30-rc7/drivers/video/intelfb/intelfbdrv.c	2009-05-29 07:37:27.000000000 +0200
@@ -874,6 +874,9 @@ static int __devinit intelfb_pci_registe
 	if (bailearly == 18)
 		bailout(dinfo);
 
+	/* read active pipe */
+	dinfo->pipe = intelfbhw_active_pipe(&dinfo->save_state);
+
 	/* Cursor initialisation */
 	if (dinfo->hwcursor) {
 		intelfbhw_cursor_init(dinfo);
diff -urp linux-ref/drivers/video/intelfb/intelfbhw.c linux-2.6.30-rc7/drivers/video/intelfb/intelfbhw.c
--- linux-ref/drivers/video/intelfb/intelfbhw.c	2009-05-23 23:47:00.000000000 +0200
+++ linux-2.6.30-rc7/drivers/video/intelfb/intelfbhw.c	2009-05-29 07:42:06.000000000 +0200
@@ -469,6 +469,32 @@ void intelfbhw_do_blank(int blank, struc
 }
 
 
+/* Check which pipe is connected to an active display plane. */
+int intelfbhw_active_pipe(const struct intelfb_hwstate *hw)
+{
+	int pipe = -1;
+
+	/* keep old default behaviour - prefer PIPE_A */
+	if (hw->disp_b_ctrl & DISPPLANE_PLANE_ENABLE) {
+		pipe = (hw->disp_b_ctrl >> DISPPLANE_SEL_PIPE_SHIFT);
+		pipe &= PIPE_MASK;
+		if (unlikely(pipe == PIPE_A))
+			return PIPE_A;
+	}
+	if (hw->disp_a_ctrl & DISPPLANE_PLANE_ENABLE) {
+		pipe = (hw->disp_a_ctrl >> DISPPLANE_SEL_PIPE_SHIFT);
+		pipe &= PIPE_MASK;
+		if (likely(pipe == PIPE_A))
+			return PIPE_A;
+	}
+	/* Impossible that no pipe is selected - return PIPE_A */
+	WARN_ON(pipe == -1);
+	if (unlikely(pipe == -1))
+		pipe = PIPE_A;
+
+	return pipe;
+}
+
 void intelfbhw_setcolreg(struct intelfb_info *dinfo, unsigned regno,
 			 unsigned red, unsigned green, unsigned blue,
 			 unsigned transp)
@@ -1019,7 +1045,7 @@ int intelfbhw_mode_to_hw(struct intelfb_
 			 struct intelfb_hwstate *hw,
 			 struct fb_var_screeninfo *var)
 {
-	int pipe = PIPE_A;
+	int pipe = intelfbhw_active_pipe(hw);
 	u32 *dpll, *fp0, *fp1;
 	u32 m1, m2, n, p1, p2, clock_target, clock;
 	u32 hsync_start, hsync_end, hblank_start, hblank_end, htotal, hactive;
@@ -1033,12 +1059,6 @@ int intelfbhw_mode_to_hw(struct intelfb_
 	/* Disable VGA */
 	hw->vgacntrl |= VGA_DISABLE;
 
-	/* Check whether pipe A or pipe B is enabled. */
-	if (hw->pipe_a_conf & PIPECONF_ENABLE)
-		pipe = PIPE_A;
-	else if (hw->pipe_b_conf & PIPECONF_ENABLE)
-		pipe = PIPE_B;
-
 	/* Set which pipe's registers will be set. */
 	if (pipe == PIPE_B) {
 		dpll = &hw->dpll_b;
@@ -1262,7 +1282,6 @@ int intelfbhw_mode_to_hw(struct intelfb_
 int intelfbhw_program_mode(struct intelfb_info *dinfo,
 			   const struct intelfb_hwstate *hw, int blank)
 {
-	int pipe = PIPE_A;
 	u32 tmp;
 	const u32 *dpll, *fp0, *fp1, *pipe_conf;
 	const u32 *hs, *ht, *hb, *vs, *vt, *vb, *ss;
@@ -1272,7 +1291,7 @@ int intelfbhw_program_mode(struct intelf
 	u32 src_size_reg;
 	u32 count, tmp_val[3];
 
-	/* Assume single pipe, display plane A, analog CRT. */
+	/* Assume single pipe */
 
 #if VERBOSE > 0
 	DBG_MSG("intelfbhw_program_mode\n");
@@ -1283,15 +1302,9 @@ int intelfbhw_program_mode(struct intelf
 	tmp |= VGA_DISABLE;
 	OUTREG(VGACNTRL, tmp);
 
-	/* Check whether pipe A or pipe B is enabled. */
-	if (hw->pipe_a_conf & PIPECONF_ENABLE)
-		pipe = PIPE_A;
-	else if (hw->pipe_b_conf & PIPECONF_ENABLE)
-		pipe = PIPE_B;
+	dinfo->pipe = intelfbhw_active_pipe(hw);
 
-	dinfo->pipe = pipe;
-
-	if (pipe == PIPE_B) {
+	if (dinfo->pipe == PIPE_B) {
 		dpll = &hw->dpll_b;
 		fp0 = &hw->fpb0;
 		fp1 = &hw->fpb1;
diff -urp linux-ref/drivers/video/intelfb/intelfbhw.h linux-2.6.30-rc7/drivers/video/intelfb/intelfbhw.h
--- linux-ref/drivers/video/intelfb/intelfbhw.h	2009-05-23 23:47:00.000000000 +0200
+++ linux-2.6.30-rc7/drivers/video/intelfb/intelfbhw.h	2009-05-29 07:43:54.000000000 +0200
@@ -604,5 +604,6 @@ extern void intelfbhw_cursor_reset(struc
 extern int intelfbhw_enable_irq(struct intelfb_info *dinfo);
 extern void intelfbhw_disable_irq(struct intelfb_info *dinfo);
 extern int intelfbhw_wait_for_vsync(struct intelfb_info *dinfo, u32 pipe);
+extern int intelfbhw_active_pipe(const struct intelfb_hwstate *hw);
 
 #endif /* _INTELFBHW_H */



----------------------------------------------------------------------
Kup wlasne mieszkanie za 33 tys. zl.
Sprawdz >>> http://link.interia.pl/f21a3

On Sat, 30 May 2009 13:58:33 +0200
Krzysztof Helt <krzysztof.h1@poczta.fm> wrote:

> The intelfb driver sets color map depending on currently active pipe.
> However, if an LVDS 
> display is attached (like in laptop) the active pipe variable is never set.
> The default value is 
> PIPE_A and can be wrong.
> Set up the pipe variable during driver initialization after hardware state
> was read.
> 
> Also, the detection of the active display (and hence the pipe) is wrong. The
> pipes are assigned
> to so called planes. Both pipes are always enabled on my laptop but only one
> plane is enabled
> (the plane A for the CRT or the plane B for the LVDS). Change active pipe
> detection code
> to take into account a status of the plane assigned to each pipe.
> 
> The problem is visible in the 8 bpp mode if colors above 15 are used. The
> first 16 color
> entries are displayed correctly.
> 
> The graphics chip description is here (G45 vol. 3):
> http://intellinuxgraphics.org/documentation.html
> 
> Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> ---
> The second version of the fix to this problem. Now, it is much more
> sophisticated
> based on the knowledge gained from documentation available at
> http://intellinuxgraphics.org/.
> 
> It does not change a default behaviour (assumed pipe A) for all cases except
> the case that only 
> the plane assigned to the pipe B is active. It is enough to fix the issue for
> me.

I queued this.

> Please test it.

But it would great be Dean and/or Michal were to be able to test it, please.

This revised patch written according to the Intel graphics documentation indeed fixes the problem.

2009/6/2 Andrew Morton <akpm@linux-foundation.org>:
> On Sat, 30 May 2009 13:58:33 +0200
> Krzysztof Helt <krzysztof.h1@poczta.fm> wrote:
>
>> The intelfb driver sets color map depending on currently active pipe.
>> However, if an LVDS
>> display is attached (like in laptop) the active pipe variable is never set.
>> The default value is
>> PIPE_A and can be wrong.
>> Set up the pipe variable during driver initialization after hardware state
>> was read.
>>
>> Also, the detection of the active display (and hence the pipe) is wrong. The
>> pipes are assigned
>> to so called planes. Both pipes are always enabled on my laptop but only one
>> plane is enabled
>> (the plane A for the CRT or the plane B for the LVDS). Change active pipe
>> detection code
>> to take into account a status of the plane assigned to each pipe.
>>
>> The problem is visible in the 8 bpp mode if colors above 15 are used. The
>> first 16 color
>> entries are displayed correctly.
>>
>> The graphics chip description is here (G45 vol. 3):
>> http://intellinuxgraphics.org/documentation.html
>>
>> Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
>>
>> ---
>> The second version of the fix to this problem. Now, it is much more
>> sophisticated
>> based on the knowledge gained from documentation available at
>> http://intellinuxgraphics.org/.
>>
>> It does not change a default behaviour (assumed pipe A) for all cases except
>> the case that only
>> the plane assigned to the pipe B is active. It is enough to fix the issue
>> for me.
>
> I queued this.
>
>> Please test it.
>
> But it would great be Dean and/or Michal were to be able to test it, please.
>

Thanks for the patch.

Unfortunately I did not get to testing the patch yet.

According to the description it is supposed to resolve some confusion
over what pipe is enabled or not.

X server reports the pipes connected as follows:
(II) intel(0):   Pipe A is on
(II) intel(0):   Display plane A is now enabled and connected to pipe A.
(II) intel(0):   Pipe B is off
(II) intel(0):   Display plane B is now disabled and connected to pipe A.
(II) intel(0):   Output VGA is connected to pipe none
(II) intel(0):   Output TMDS-1 is connected to pipe A
(II) intel(0):   Output TV is connected to pipe none

However, I also get this warning before the outputs are listed:
(WW) intel(0): Couldn't detect panel mode.  Disabling panel

Is this a configuration that would likely be affected by the issue
fixed here or do I have a different problem?

I am currently not using intelfb because last time I tried it produced
even worse results than efifb (which does suffer from the wrong colors
as well).

Thanks

Michal

On Wed, 3 Jun 2009, bugzilla-daemon@bugzilla.kernel.org wrote:
> 
> I am currently not using intelfb because last time I tried it produced
> even worse results than efifb (which does suffer from the wrong colors
> as well).

Side note: have people tried to enable DRM_I915 and then DRM_I915_KMS 
("kernel mode setting"). That's the _new_ Intel frame buffer driver that 
knows how to set modes dynamically etc.

It requires a really modern X server (ie we're talking distributions like 
fedora-11), but even if you don't have that, it would be interesting to 
hear if you can boot into text-mode, and if fbcon works there (disable 
INTELFB entirely)..

			Linus

Reply-To: krzysztof.h1@poczta.fm

On Wed, 3 Jun 2009 11:27:17 +0200
Michal Suchanek <hramrach@centrum.cz> wrote:

> Unfortunately I did not get to testing the patch yet.
> 
> According to the description it is supposed to resolve some confusion
> over what pipe is enabled or not.
> 
> X server reports the pipes connected as follows:
> (II) intel(0):   Pipe A is on
> (II) intel(0):   Display plane A is now enabled and connected to pipe A.
> (II) intel(0):   Pipe B is off
> (II) intel(0):   Display plane B is now disabled and connected to pipe A.
> (II) intel(0):   Output VGA is connected to pipe none
> (II) intel(0):   Output TMDS-1 is connected to pipe A
> (II) intel(0):   Output TV is connected to pipe none
> 
> However, I also get this warning before the outputs are listed:
> (WW) intel(0): Couldn't detect panel mode.  Disabling panel
> 
> Is this a configuration that would likely be affected by the issue
> fixed here or do I have a different problem?
> 

Frankly speaking, I don't know. Please describe your problem.

The panel mode I suppose is LVDS (LCD panel directly connected
to the chip) which is possible only in laptops and tablets or other
computers where the LCD panel is integrated with the main unit
(e.g. "desk-lamp" like Apple computers). All other computers which you
connect the display by external cable (DVI/HDMI or VGA) does not
work in the "panel mode".

Kind regards,
Krzysztof


----------------------------------------------------------------------
Sprawdz wiadomosci z Twojego regionu!
Kliknij >>> http://link.interia.pl/f21ba

2009/6/4 Krzysztof Helt <krzysztof.h1@poczta.fm>:
> On Wed, 3 Jun 2009 11:27:17 +0200
> Michal Suchanek <hramrach@centrum.cz> wrote:
>
>> Unfortunately I did not get to testing the patch yet.
>>
>> According to the description it is supposed to resolve some confusion
>> over what pipe is enabled or not.
>>
>> X server reports the pipes connected as follows:
>> (II) intel(0):   Pipe A is on
>> (II) intel(0):   Display plane A is now enabled and connected to pipe A.
>> (II) intel(0):   Pipe B is off
>> (II) intel(0):   Display plane B is now disabled and connected to pipe A.
>> (II) intel(0):   Output VGA is connected to pipe none
>> (II) intel(0):   Output TMDS-1 is connected to pipe A
>> (II) intel(0):   Output TV is connected to pipe none
>>
>> However, I also get this warning before the outputs are listed:
>> (WW) intel(0): Couldn't detect panel mode.  Disabling panel
>>
>> Is this a configuration that would likely be affected by the issue
>> fixed here or do I have a different problem?
>>
>
> Frankly speaking, I don't know. Please describe your problem.
>
> The panel mode I suppose is LVDS (LCD panel directly connected
> to the chip) which is possible only in laptops and tablets or other
> computers where the LCD panel is integrated with the main unit
> (e.g. "desk-lamp" like Apple computers). All other computers which you
> connect the display by external cable (DVI/HDMI or VGA) does not
> work in the "panel mode".

This is a mac mini with single TMDS display only.

Thanks

Michal

On Fri, 5 Jun 2009, bugzilla-daemon@bugzilla.kernel.org wrote:
> >>
> >> However, I also get this warning before the outputs are listed:
> >> (WW) intel(0): Couldn't detect panel mode.  Disabling panel
>
> This is a mac mini with single TMDS display only.

Oh, the Mac Mini is a known screw-up by Apple.

The BIOS reports that it has a panel attached (they must have just copied 
things from their laptops). It obviously doesn't. So the warning comes 
from that, I suspect.

(The reason I know this is that I've got several Mac Mini's, an this was 
one of the problems that caused X breakage early on - X would decide to 
use the attached panel, and limit the resolution to the panel resolution. 
Which is not so good when you want 1280x1050, but the fake attached panel 
is claiming to be 1024x768 or something like that. I forget the exact 
details, but the end result is that we have a few Mac Mini quirks now)

			Linus

I still have 
intelfb-fix-color-map-setting-with-an-lvds-display.patch
and
intelfb-fix-setting-of-active-pipe-with-lvds-displays.patch

sitting in -mm but I'm unsure what to do with them.

http://userweb.kernel.org/~akpm/mmotm/broken-out/intelfb-fix-color-map-setting-with-an-lvds-display.patch
http://userweb.kernel.org/~akpm/mmotm/broken-out/intelfb-fix-setting-of-active-pipe-with-lvds-displays.patch

Can we revisit all this please?

On Thu, 18 Jun 2009, bugzilla-daemon@bugzilla.kernel.org wrote:
>
> I still have 
> intelfb-fix-color-map-setting-with-an-lvds-display.patch
> and
> intelfb-fix-setting-of-active-pipe-with-lvds-displays.patch
> 
> sitting in -mm but I'm unsure what to do with them.

Drop the first one.

The second one looks fine to me (and makes the first one pointless)

Did we get confirmation that it worked for everybody?

		Linus

Hello

2009/6/2 Andrew Morton <akpm@linux-foundation.org>:
> On Sat, 30 May 2009 13:58:33 +0200
> Krzysztof Helt <krzysztof.h1@poczta.fm> wrote:
>
>> The intelfb driver sets color map depending on currently active pipe.
>> However, if an LVDS
>> display is attached (like in laptop) the active pipe variable is never set.
>> The default value is
>> PIPE_A and can be wrong.
>> Set up the pipe variable during driver initialization after hardware state
>> was read.
>>
>> Also, the detection of the active display (and hence the pipe) is wrong. The
>> pipes are assigned
>> to so called planes. Both pipes are always enabled on my laptop but only one
>> plane is enabled
>> (the plane A for the CRT or the plane B for the LVDS). Change active pipe
>> detection code
>> to take into account a status of the plane assigned to each pipe.
>>
>> The problem is visible in the 8 bpp mode if colors above 15 are used. The
>> first 16 color
>> entries are displayed correctly.
>>
>> The graphics chip description is here (G45 vol. 3):
>> http://intellinuxgraphics.org/documentation.html
>>
>> Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
>>
>> ---
>> The second version of the fix to this problem. Now, it is much more
>> sophisticated
>> based on the knowledge gained from documentation available at
>> http://intellinuxgraphics.org/.
>>
>> It does not change a default behaviour (assumed pipe A) for all cases except
>> the case that only
>> the plane assigned to the pipe B is active. It is enough to fix the issue
>> for me.
>
> I queued this.
>
>> Please test it.
>
> But it would great be Dean and/or Michal were to be able to test it, please.
>

Sorry about the late reply.

In the end I could not test the patch on my system. Not only does the
patch not apply but intelfb fails to produce any screen output at all.
the command line is like this:

video=intelfb:mode=1280x1024 fbcon=rotate:1

Here is a boot with both intelfb and efifb. For some reason the efifb
is shown in this case but breaks so I thought I was looking at intelfb
because the output was different. Both geometry and colours are
broken. Part of the console (top, physical right) is invisible, part
(right, physical bottom) is not drawn (shows black initially and
garbage after switching to X and back).


Jun 29 12:50:30 uvt316-2 kernel: [    0.366168] efifb: dmi detected
Macmini1,1 - framebuffer at 80000000
(1024x768, stride 8192)
Jun 29 12:50:30 uvt316-2 kernel: [    0.366273] efifb: probing for efifb
Jun 29 12:50:30 uvt316-2 kernel: [    0.366486] efifb: framebuffer at
0x80000000, mapped to 0xf8100000, u
sing 6144k, total 6144k
Jun 29 12:50:30 uvt316-2 kernel: [    0.366496] efifb: mode is
1024x768x32, linelength=8192, pages=1
Jun 29 12:50:30 uvt316-2 kernel: [    0.366501] efifb: scrolling: redraw
Jun 29 12:50:30 uvt316-2 kernel: [    0.366508] efifb: Truecolor:
size=8:8:8:8, shift=24:16:8:0
Jun 29 12:50:30 uvt316-2 kernel: [    0.395938] Console: switching to
colour frame buffer device 96x64
Jun 29 12:50:30 uvt316-2 kernel: [    0.423483] fb0: EFI VGA frame buffer device
Jun 29 12:50:30 uvt316-2 kernel: [    0.427641] Linux agpgart interface v0.103
Jun 29 12:50:30 uvt316-2 kernel: [    0.427886] agpgart-intel
0000:00:00.0: Intel 945GM Chipset
Jun 29 12:50:30 uvt316-2 kernel: [    0.429006] agpgart-intel
0000:00:00.0: detected 16124K stolen memory
Jun 29 12:50:30 uvt316-2 kernel: [    0.432312] agpgart-intel
0000:00:00.0: AGP aperture is 256M @ 0x8000
0000
Jun 29 12:50:30 uvt316-2 kernel: [    0.432750] intelfb: Framebuffer
driver for Intel(R)
830M/845G/852GM/855GM/865G/915G/915GM/945G/945GM/945GME/965G/965GM
chipsets
Jun 29 12:50:30 uvt316-2 kernel: [    0.433387] intelfb: Version 0.9.6
Jun 29 12:50:30 uvt316-2 kernel: [    0.433626] intelfb 0000:00:02.0:
PCI INT A -> GSI 16 (level, low) -> IRQ 16
Jun 29 12:50:30 uvt316-2 kernel: [    0.434067] intelfb: Cannot
reserve FB region.


Intelfb only. No console output.

Jun 29 14:16:18 uvt316-2 kernel: [    0.370014] Linux agpgart interface v0.103
Jun 29 14:16:18 uvt316-2 kernel: [    0.370037] agpgart-intel
0000:00:00.0: Intel 945GM Chipset
Jun 29 14:16:18 uvt316-2 kernel: [    0.370786] agpgart-intel
0000:00:00.0: detected 16124K stolen memory
Jun 29 14:16:18 uvt316-2 kernel: [    0.373808] agpgart-intel
0000:00:00.0: AGP aperture is 256M @ 0x8000
0000
Jun 29 14:16:18 uvt316-2 kernel: [    0.373876] intelfb: Framebuffer
driver for Intel(R)
830M/845G/852GM/855GM/865G/915G/915GM/945G/945GM/945GME/965G/965GM
chipsets
Jun 29 14:16:18 uvt316-2 kernel: [    0.373886] intelfb: Version 0.9.6
Jun 29 14:16:18 uvt316-2 kernel: [    0.373937] intelfb 0000:00:02.0:
PCI INT A -> GSI 16 (level, low) -> IRQ 16
Jun 29 14:16:18 uvt316-2 kernel: [    0.373960] intelfb: 00:02.0:
Intel(R) 945GM, aperture size 256MB, stolen memory 16124kB
Jun 29 14:16:18 uvt316-2 kernel: [    0.377376] intelfb: Non-CRT
device is enabled ( DVO port B ).  Disabling mode switching.
Jun 29 14:16:18 uvt316-2 kernel: [    0.377392] intelfb: Video mode
must be programmed at boot time.



Efifb only with video=efifb fbcon=rotate:1
Produces correct geometry but broken colours.

Jun 29 15:26:59 uvt316-2 kernel: [    0.364224] efifb: dmi detected
Macmini1,1 - framebuffer at 80000000
(1024x768, stride 8192)
Jun 29 15:26:59 uvt316-2 kernel: [    0.364328] efifb: probing for efifb
Jun 29 15:26:59 uvt316-2 kernel: [    0.364609] efifb: framebuffer at
0x80000000, mapped to 0xf8100000, u
sing 8192k, total 8192k
Jun 29 15:26:59 uvt316-2 kernel: [    0.364619] efifb: mode is
1280x1024x16, linelength=8192, pages=1
Jun 29 15:26:59 uvt316-2 kernel: [    0.364624] efifb: scrolling: redraw
Jun 29 15:26:59 uvt316-2 kernel: [    0.364631] efifb: Truecolor:
size=8:8:8:8, shift=24:16:8:0
Jun 29 15:26:59 uvt316-2 kernel: [    0.389698] Console: switching to
colour frame buffer device 128x80
Jun 29 15:26:59 uvt316-2 kernel: [    0.413244] fb0: EFI VGA frame buffer device