Skip to contentSkip to conversation

Bug#619019: [PATCH 1/2] i915: Remove 'pipe A force' quirk for 855GM and 845G

PATCH In review Ben Hutchings 3 participants 8 messages Mar 20, 2011 – Mar 22, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Applying this quirk to the 855GM in all systems causes regressions (Debian bugs #493096, #619019). Instead, apply the quirk to specific models as listed in the old X driver.

I don't see any explanation for this quirk being applied to the 845G, except perhaps that VT switching used to hang if pipe A was turned off. However, that seems to be a problem only when using UMS. So remove the quirk for the 845G as well.

---
Needs people to test it, as I can't.

Ben.

 drivers/gpu/drm/i915/intel_display.c |   23 +++++++++++++++++++----
 1 files changed, 19 insertions(+), 4 deletions(-)
drivers/gpu/drm/i915/intel_display.c +19 -4
index 3106c0d..a490f75 100644
@@ -7345,19 +7345,34 @@ struct intel_quirk intel_quirks[] = {
 
 	/* Thinkpad R31 needs pipe A force quirk */
 	{ 0x3577, 0x1014, 0x0505, quirk_pipea_force },
+	/* Dell Latitude D400 needs pipe A force quirk (LP: #228519) */
+	{ 0x3582, 0x1028, 0x0139, quirk_pipea_force },
+	/* Dell Latitude D500 needs pipe A force quirk */
+	{ 0x3582, 0x1028, 0x0152, quirk_pipea_force },
+	/* Dell Latitude D505 needs pipe A force quirk (LP: #235643) */
+	{ 0x3582, 0x1028, 0x0163, quirk_pipea_force },
+	/* Dell Latitude X300 needs pipe A force quirk */
+	{ 0x3582, 0x1028, 0x014f, quirk_pipea_force },
+	/* Dell Inspiron 510m needs pipe A force quirk */
+	{ 0x3582, 0x1028, 0x0164, quirk_pipea_force },
+	/* Toshiba Satellite A30 needs pipe A force quirk */
+	{ 0x3582, 0x1179, 0xff00, quirk_pipea_force },
 	/* Toshiba Protege R-205, S-209 needs pipe A force quirk */
 	{ 0x2592, 0x1179, 0x0001, quirk_pipea_force },
+	/* Intel 855GM hardware (See LP: #216490) */
+	{ 0x3582, 0x1028, 0x00c8, quirk_pipea_force },
+	/* Intel 855GM hardware (See Novell Bugzilla #406123) */
+	{ 0x3582, 0x10cf, 0x1215, quirk_pipea_force },
+	/* HP Pavilion ze4944ea needs pipe A force quirk (See LP: #242389) */
+	{ 0x3582, 0x103c, 0x3084, quirk_pipea_force },
 
 	/* ThinkPad X30 needs pipe A force quirk (LP: #304614) */
 	{ 0x3577,  0x1014, 0x0513, quirk_pipea_force },
 	/* ThinkPad X40 needs pipe A force quirk */
+	{ 0x3582, 0x1014, 0x0557, quirk_pipea_force },
 
 	/* ThinkPad T60 needs pipe A force quirk (bug #16494) */
 	{ 0x2782, 0x17aa, 0x201a, quirk_pipea_force },
-
-	/* 855 & before need to leave pipe A & dpll A up */
-	{ 0x3582, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force },
-	{ 0x2562, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force },
 };
 
 static void intel_init_quirks(struct drm_device *dev)
Signature · 1 line
1.7.4.1
#1 lore raw

PATCH 2/2 i915: Apply the 'pipe A force' quirk to some more models

Add some more IDs as listed in the old X driver.

---
Is there some reason these were omitted from the kernel driver? If not, it seems sensible to include them.

Ben.

 drivers/gpu/drm/i915/intel_display.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
drivers/gpu/drm/i915/intel_display.c +8 -0
index a490f75..fbba580 100644
@@ -7342,6 +7342,8 @@ struct intel_quirk intel_quirks[] = {
 	{ 0x2a42, 0x103c, 0x30eb, quirk_pipea_force },
 	/* HP Mini needs pipe A force quirk (LP: #322104) */
 	{ 0x27ae,0x103c, 0x361a, quirk_pipea_force },
+	/* HP Mini 5101 needs pipe A force quirk */
+	{ 0x27ae, 0x103c, 0x3632, quirk_pipea_force },
 
 	/* Thinkpad R31 needs pipe A force quirk */
 	{ 0x3577, 0x1014, 0x0505, quirk_pipea_force },
@@ -7373,6 +7375,12 @@ struct intel_quirk intel_quirks[] = {
 
 	/* ThinkPad T60 needs pipe A force quirk (bug #16494) */
 	{ 0x2782, 0x17aa, 0x201a, quirk_pipea_force },
+
+	/* Sony vaio VGN-SZ4MN (See LP: #212163) */
+	{ 0x3577, 0x104d, 0x81e6, quirk_pipea_force },
+
+	/* Quanta Gigabyte W251U (See LP: #244242) */
+	{ 0x27a2, 0x152d, 0x0755, quirk_pipea_force },
 };
 
 static void intel_init_quirks(struct drm_device *dev)
Signature · 1 line
1.7.4.1
#2 lore raw

Re: [PATCH 2/2] i915: Apply the pipe A force quirk to some more models

Quoted 5 lines · Ben Hutchings

Add some more IDs as listed in the old X driver.

---
Is there some reason these were omitted from the kernel driver?

Yes, because they are non-gen2 chipsets and should have never needed the pipe-A quirk.

*If* the quirk did indeed and still does help those systems, then we need to root cause the real bug.
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
#3 lore raw

Re: [PATCH 1/2] i915: Remove pipe A force quirk for 855GM and 845G

In reply to Ben Hutchings
Quoted 8 lines · Ben Hutchings

Applying this quirk to the 855GM in all systems causes regressions (Debian bugs #493096, #619019). Instead, apply the quirk to specific models as listed in the old X driver.

I don't see any explanation for this quirk being applied to the 845G, except perhaps that VT switching used to hang if pipe A was turned off. However, that seems to be a problem only when using UMS. So remove the quirk for the 845G as well.

The quirk should only be required for 830M due to the numerous instances where a unit on the second pipe is actually wired into the clock on the first pipe. (And so it is easiest to keep the first pipe active at all times.)

I'd prefer the quirk table to disappear and simply be replaced by IS_830M(). However, that requires testing and so should only be done piecemeal. And leaves some doubt as to why the other machines were in the quirk table in the first place.

Can you please repost each of these removals as a separate patch and lets try and get a tested-by for each one? (Make sure the tester includes the model name for his machine so we can double check the veracity of the change.)
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
#4 lore raw

Re: [PATCH 1/2] i915: Remove pipe A force quirk for 855GM and 845G

Quoted 10 lines · Chris Wilson
Quoted 8 lines · Ben Hutchings

Applying this quirk to the 855GM in all systems causes regressions (Debian bugs #493096, #619019). Instead, apply the quirk to specific models as listed in the old X driver.

I don't see any explanation for this quirk being applied to the 845G, except perhaps that VT switching used to hang if pipe A was turned off. However, that seems to be a problem only when using UMS. So remove the quirk for the 845G as well.

Quoted 4 lines

The quirk should only be required for 830M due to the numerous instances where a unit on the second pipe is actually wired into the clock on the first pipe. (And so it is easiest to keep the first pipe active at all times.)

When you say 'wired into', is this part of the chip design or something done on the board?

Jesse, why did you add the quirk for other chips?

Quoted 4 lines

I'd prefer the quirk table to disappear and simply be replaced by IS_830M(). However, that requires testing and so should only be done piecemeal. And leaves some doubt as to why the other machines were in the quirk table in the first place.

The commit messages referring to VT switching suggest that the problems related to disabling part A may actually have been related to handover to the console driver before KMS.

Quoted 4 lines

Can you please repost each of these removals as a separate patch and lets try and get a tested-by for each one? (Make sure the tester includes the model name for his machine so we can double check the veracity of the change.)

I already have 4 regression reports for the addition of the quirk for 855GM:

http://bugs.debian.org/618665
http://bugs.debian.org/618997
http://bugs.debian.org/619019
http://bugs.debian.org/619192

and one on an unidentified (as yet) chip:

http://bugs.debian.org/619199

So I can just send a patch to revert 855GM.

The odd thing about these reports is that the regression is reported to occur before the system has ever been suspended, and to be fixed (or mitigated) by suspending and resuming. I don't understand why the quirk even comes into play during boot.

Ben.

Signature · 2 lines
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

#5 lore raw

Re: [PATCH 1/2] i915: Remove pipe A force quirk for 855GM and 845G

Quoted 11 lines · Ben Hutchings
Quoted 10 lines · Chris Wilson
Quoted 8 lines · Ben Hutchings

Applying this quirk to the 855GM in all systems causes regressions (Debian bugs #493096, #619019). Instead, apply the quirk to specific models as listed in the old X driver.

I don't see any explanation for this quirk being applied to the 845G, except perhaps that VT switching used to hang if pipe A was turned off. However, that seems to be a problem only when using UMS. So remove the quirk for the 845G as well.

Quoted 7 lines
Quoted 4 lines

The quirk should only be required for 830M due to the numerous instances where a unit on the second pipe is actually wired into the clock on the first pipe. (And so it is easiest to keep the first pipe active at all times.)

When you say 'wired into', is this part of the chip design or something done on the board?

It is mentioned as a "feature" in several places of the specs for the 830G chipset.

Quoted 10 lines

Jesse, why did you add the quirk for other chips?

Quoted 4 lines

I'd prefer the quirk table to disappear and simply be replaced by IS_830M(). However, that requires testing and so should only be done piecemeal. And leaves some doubt as to why the other machines were in the quirk table in the first place.

The commit messages referring to VT switching suggest that the problems related to disabling part A may actually have been related to handover to the console driver before KMS.

That sounds promising that the code was indeed papering over bugs... Doesn't sound too promising for the state of our code though. :(

Quoted 13 lines
Quoted 4 lines

Can you please repost each of these removals as a separate patch and lets try and get a tested-by for each one? (Make sure the tester includes the model name for his machine so we can double check the veracity of the change.)

I already have 4 regression reports for the addition of the quirk for 855GM:

http://bugs.debian.org/618665
http://bugs.debian.org/618997
http://bugs.debian.org/619019
http://bugs.debian.org/619192

Quoted 5 lines

and one on an unidentified (as yet) chip:

http://bugs.debian.org/619199

So I can just send a patch to revert 855GM.

I'd still prefer a patch for each quirk removal.
Along with a tested-by. ;-)

The little bit of time invested now in preparing small commits with be of great benefit should anyone need to investigate later. We have to gradually wean ourselves off the quirk table and convince everybody that the code does actually know how to modeset the chip!

Quoted 4 lines

The odd thing about these reports is that the regression is reported to occur before the system has ever been suspended, and to be fixed (or mitigated) by suspending and resuming. I don't understand why the quirk even comes into play during boot.

During the switch-over from BIOS we disable all the outputs - instant bug.

Many thanks for preparing these patches,
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
#6 lore raw

Re: [PATCH 1/2] i915: Remove pipe A force quirk for 855GM and 845G

In reply to Ben Hutchings
Quoted 14 lines · Ben Hutchings
Quoted 4 lines · Chris Wilson

Can you please repost each of these removals as a separate patch and lets try and get a tested-by for each one? (Make sure the tester includes the model name for his machine so we can double check the veracity of the change.)

I already have 4 regression reports for the addition of the quirk for 855GM:

http://bugs.debian.org/618665
http://bugs.debian.org/618997
http://bugs.debian.org/619019
http://bugs.debian.org/619192

Quoted 5 lines

and one on an unidentified (as yet) chip:

http://bugs.debian.org/619199

So I can just send a patch to revert 855GM.

Yes. Having just been poked by Julien to look at the original quirk table and so seeing the generic match all 855GM, please do send that patch.
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
#7 lore raw

Re: [PATCH 1/2] i915: Remove pipe A force quirk for 855GM and 845G

In reply to Ben Hutchings
Quoted 16 lines · On Tue, 22 Mar 2011 03:05:45 +0000 Ben Hutchings <[email protected]> wrote:
Quoted 10 lines · Chris Wilson
Quoted 8 lines · Ben Hutchings

Applying this quirk to the 855GM in all systems causes regressions (Debian bugs #493096, #619019). Instead, apply the quirk to specific models as listed in the old X driver.

I don't see any explanation for this quirk being applied to the 845G, except perhaps that VT switching used to hang if pipe A was turned off. However, that seems to be a problem only when using UMS. So remove the quirk for the 845G as well.

Quoted 4 lines

The quirk should only be required for 830M due to the numerous instances where a unit on the second pipe is actually wired into the clock on the first pipe. (And so it is easiest to keep the first pipe active at all times.)

Quoted 4 lines

When you say 'wired into', is this part of the chip design or something done on the board?

Jesse, why did you add the quirk for other chips?

The DDX driver had an option to force enable pipe A, and we told people to report bugs when they needed it. So we got a bunch from Ubuntu and elsewhere and added them without too much investigation into the root cause (as you say below it could have been something else).

Quoted 8 lines
Quoted 4 lines

I'd prefer the quirk table to disappear and simply be replaced by IS_830M(). However, that requires testing and so should only be done piecemeal. And leaves some doubt as to why the other machines were in the quirk table in the first place.

The commit messages referring to VT switching suggest that the problems related to disabling part A may actually have been related to handover to the console driver before KMS.

Yes, definitely possible. We didn't have all the assertion checks we have now, so we may have just been masking other problems without knowing it.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center