linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* matroxfb: remove incorrect Matrox G200eV support
@ 2011-03-01 17:50 Gary Hade
  2011-03-01 18:29 ` Linus Torvalds
  0 siblings, 1 reply; 6+ messages in thread
From: Gary Hade @ 2011-03-01 17:50 UTC (permalink / raw)
  To: linux-fbdev, linux-kernel
  Cc: Darrick J. Wong, Krzysztof Helt, Petr Vandrovec, Andrew Morton,
	Linus Torvalds, Gary Hade


From: Gary Hade <garyhade@us.ibm.com>

Remove incorrect Matrox G200eV support that was previously added
by commit e3a1938805d2e81b27d3d348788644f3bad004f2

One serious issue with the G200eV support that I have reproduced
on a Matrox G200eV equipped IBM x3650 M2 is the lack of text (login
banner, login prompt, etc) on the console when X not running and
lack of text on all of the virtual consoles after X is started.

Signed-off-by: Gary Hade <garyhade@us.ibm.com>
Cc: Darrick J. Wong <djwong@us.ibm.com>
Cc: Krzysztof Helt <krzysztof.h1@wp.pl>
Cc: Petr Vandrovec <vandrove@vc.cvut.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>

---
 drivers/video/matrox/matroxfb_base.c |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/drivers/video/matrox/matroxfb_base.c b/drivers/video/matrox/matroxfb_base.c
index a082deb..a74439a 100644
--- a/drivers/video/matrox/matroxfb_base.c
+++ b/drivers/video/matrox/matroxfb_base.c
@@ -1461,13 +1461,6 @@ static struct board {
 		MGA_G100,
 		&vbG100,
 		"MGA-G100 (AGP)"},
-	{PCI_VENDOR_ID_MATROX,	PCI_DEVICE_ID_MATROX_G200EV_PCI,	0xFF,
-		0,			0,
-		DEVF_G200,
-		230000,
-		MGA_G200,
-		&vbG200,
-		"MGA-G200eV (PCI)"},
 	{PCI_VENDOR_ID_MATROX,	PCI_DEVICE_ID_MATROX_G200_PCI,	0xFF,
 		0,			0,
 		DEVF_G200,
@@ -2119,8 +2112,6 @@ static struct pci_device_id matroxfb_devices[] = {
 		PCI_ANY_ID,	PCI_ANY_ID,	0, 0, 0},
 	{PCI_VENDOR_ID_MATROX,	PCI_DEVICE_ID_MATROX_G100_AGP,
 		PCI_ANY_ID,	PCI_ANY_ID,	0, 0, 0},
-	{PCI_VENDOR_ID_MATROX,	PCI_DEVICE_ID_MATROX_G200EV_PCI,
-		PCI_ANY_ID,	PCI_ANY_ID,	0, 0, 0},
 	{PCI_VENDOR_ID_MATROX,	PCI_DEVICE_ID_MATROX_G200_PCI,
 		PCI_ANY_ID,	PCI_ANY_ID,	0, 0, 0},
 	{PCI_VENDOR_ID_MATROX,	PCI_DEVICE_ID_MATROX_G200_AGP,

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: matroxfb: remove incorrect Matrox G200eV support
  2011-03-01 17:50 matroxfb: remove incorrect Matrox G200eV support Gary Hade
@ 2011-03-01 18:29 ` Linus Torvalds
  2011-03-04 20:29   ` [PATCH v2] " Darrick J. Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2011-03-01 18:29 UTC (permalink / raw)
  To: Gary Hade
  Cc: linux-fbdev, linux-kernel, Darrick J. Wong, Krzysztof Helt,
	Petr Vandrovec, Andrew Morton

On Tue, Mar 1, 2011 at 9:50 AM, Gary Hade <garyhade@us.ibm.com> wrote:
>
> Remove incorrect Matrox G200eV support that was previously added
> by commit e3a1938805d2e81b27d3d348788644f3bad004f2

That commit goes back to 2.6.28 (and is dated Oct 2008).

That means that you really need to describe the problem more:

> One serious issue with the G200eV support that I have reproduced
> on a Matrox G200eV equipped IBM x3650 M2 is the lack of text (login
> banner, login prompt, etc) on the console when X not running and
> lack of text on all of the virtual consoles after X is started.

.. but without the commit, you get no fb at all, right? So even with
the commit, at worst you'd just be able to not use the matroxfb
driver, right?

What I'm trying to say is that I suspect there are G200eV users that
likely prefer having the support in - even though it's not perfect.
And the fact that the commit has been there for 2.5 years without
comments makes me very loath to just revert it.

                           Linus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] matroxfb: remove incorrect Matrox G200eV support
  2011-03-01 18:29 ` Linus Torvalds
@ 2011-03-04 20:29   ` Darrick J. Wong
  2011-03-07  8:59     ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2011-03-04 20:29 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Gary Hade, linux-fbdev, linux-kernel, Krzysztof Helt,
	Petr Vandrovec, Andrew Morton

On Tue, Mar 01, 2011 at 10:29:23AM -0800, Linus Torvalds wrote:
> On Tue, Mar 1, 2011 at 9:50 AM, Gary Hade <garyhade@us.ibm.com> wrote:
> >
> > Remove incorrect Matrox G200eV support that was previously added
> > by commit e3a1938805d2e81b27d3d348788644f3bad004f2
> 
> That commit goes back to 2.6.28 (and is dated Oct 2008).
> 
> That means that you really need to describe the problem more:
> 
> > One serious issue with the G200eV support that I have reproduced
> > on a Matrox G200eV equipped IBM x3650 M2 is the lack of text (login
> > banner, login prompt, etc) on the console when X not running and
> > lack of text on all of the virtual consoles after X is started.

On an IBM x3650m2, loading the matroxfb-base fb driver for the builtin Matrox
G200eV VGA chip results in no usable analog signal coming out of the VGA port.
I noticed the "outputs=" parameter to that driver, and forced all outputs on;
when I did that, the display showed a crazed jumble of static and powered off.
The display would not power on again until I removed and reinserted the power
plug.  Other displays reacted less violently but not functionally, either.

> .. but without the commit, you get no fb at all, right? So even with
> the commit, at worst you'd just be able to not use the matroxfb
> driver, right?

Yep.  We contacted Matrox to see if they would help us sort out the output
misprogramming issue, and they declined.  Evidently they're no longer
interested in matroxfb support.

> What I'm trying to say is that I suspect there are G200eV users that
> likely prefer having the support in - even though it's not perfect.
> And the fact that the commit has been there for 2.5 years without
> comments makes me very loath to just revert it.

This is all quite strange -- 2.5 years ago when I wrote the patch it seemed to
work ok.  On newer revisions of the x3650M2 it seems broken.  The original
machine I wrote it for was cut up ages ago.

I suppose we could simply blacklist any G200eV with a subsystem vendor ID of
0x1014 (IBM) until we figure out how to correct the driver.  Our customers will
be deprived, but as it seems to be broken across most of our product lines I
doubt any of them are making serious use of it anyway. :)

Something like this?
--
matroxfb: Blacklist IBM G200eV chips

On an IBM x3650m2, loading the matroxfb-base fb driver for the builtin Matrox
G200eV VGA chip results in no usable analog signal coming out of the VGA port.
I noticed the "outputs=" parameter to that driver, and forced all outputs on;
when I did that, the display showed a crazed jumble of static and powered off.
The display would not power on again until I removed and reinserted the power
plug.  Other displays reacted less violently but not functionally, either.

For now, don't talk to the G200eV if it has an IBM subvendor ID.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/video/matrox/matroxfb_base.c |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/drivers/video/matrox/matroxfb_base.c b/drivers/video/matrox/matroxfb_base.c
index a082deb..a86e401 100644
--- a/drivers/video/matrox/matroxfb_base.c
+++ b/drivers/video/matrox/matroxfb_base.c
@@ -1385,6 +1385,17 @@ static struct video_board vbG400		= {0x2000000, 0x1000000, FB_ACCEL_MATROX_MGAG4
 #define DEVF_G450	(DEVF_GCORE | DEVF_ANY_VXRES | DEVF_SUPPORT32MB | DEVF_TEXT16B | DEVF_CRTC2 | DEVF_G450DAC | DEVF_SRCORG | DEVF_DUALHEAD)
 #define DEVF_G550	(DEVF_G450)
 
+static struct blacklisted_board {
+	unsigned short vendor, device, svid, sid;
+		} black_list[] = {
+#ifdef CONFIG_FB_MATROX_G
+	/* Onboard G200eV in IBM servers cause display failures */
+	{PCI_VENDOR_ID_MATROX,	PCI_DEVICE_ID_MATROX_G200EV_PCI,
+	 PCI_VENDOR_ID_IBM, 0},
+#endif
+	{0, 0, 0, 0},
+};
+
 static struct board {
 	unsigned short vendor, device, rev, svid, sid;
 	unsigned int flags;
@@ -2012,10 +2023,11 @@ static void matroxfb_unregister_device(struct matrox_fb_info* minfo) {
 
 static int matroxfb_probe(struct pci_dev* pdev, const struct pci_device_id* dummy) {
 	struct board* b;
+	struct blacklisted_board *d;
 	u_int16_t svid;
 	u_int16_t sid;
 	struct matrox_fb_info* minfo;
-	int err;
+	int err, ignore;
 	u_int32_t cmd;
 	DBG(__func__)
 
@@ -2025,6 +2037,17 @@ static int matroxfb_probe(struct pci_dev* pdev, const struct pci_device_id* dumm
 		if ((b->vendor != pdev->vendor) || (b->device != pdev->device) || (b->rev < pdev->revision)) continue;
 		if (b->svid)
 			if ((b->svid != svid) || (b->sid != sid)) continue;
+		ignore = 0;
+		for (d = black_list; d->vendor; d++)
+			if (d->vendor = pdev->vendor &&
+			    d->device = pdev->device &&
+			    d->svid = svid &&
+			    (!d->sid || d->sid = sid)) {
+				ignore = 1;
+				break;
+			}
+		if (ignore)
+			continue;
 		break;
 	}
 	/* not match... */

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] matroxfb: remove incorrect Matrox G200eV support
  2011-03-04 20:29   ` [PATCH v2] " Darrick J. Wong
@ 2011-03-07  8:59     ` Benjamin Herrenschmidt
  2011-03-11 22:23       ` Gary Hade
  0 siblings, 1 reply; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2011-03-07  8:59 UTC (permalink / raw)
  To: djwong
  Cc: Linus Torvalds, Gary Hade, linux-fbdev, linux-kernel,
	Krzysztof Helt, Petr Vandrovec, Andrew Morton

On Fri, 2011-03-04 at 12:29 -0800, Darrick J. Wong wrote:
> This is all quite strange -- 2.5 years ago when I wrote the patch it
> seemed to
> work ok.  On newer revisions of the x3650M2 it seems broken.  The
> original
> machine I wrote it for was cut up ages ago.
> 
> I suppose we could simply blacklist any G200eV with a subsystem vendor
> ID of
> 0x1014 (IBM) until we figure out how to correct the driver.  Our
> customers will
> be deprived, but as it seems to be broken across most of our product
> lines I
> doubt any of them are making serious use of it anyway. :)
> 
> Something like this? 

Does X work with the open source drivers ? In that case a better
approach would be to try to figure out what's different between the way
the 2 drivers setup the card registers and fix matroxfb..

Cheers,
Ben.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] matroxfb: remove incorrect Matrox G200eV support
  2011-03-07  8:59     ` Benjamin Herrenschmidt
@ 2011-03-11 22:23       ` Gary Hade
  2011-03-16 19:58         ` Yannick Heneault
  0 siblings, 1 reply; 6+ messages in thread
From: Gary Hade @ 2011-03-11 22:23 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: djwong, Linus Torvalds, Gary Hade, linux-fbdev, linux-kernel,
	Krzysztof Helt, Petr Vandrovec, Andrew Morton, yannick_heneault

On Mon, Mar 07, 2011 at 07:59:16PM +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2011-03-04 at 12:29 -0800, Darrick J. Wong wrote:
> > This is all quite strange -- 2.5 years ago when I wrote the patch it
> > seemed to
> > work ok.  On newer revisions of the x3650M2 it seems broken.  The
> > original
> > machine I wrote it for was cut up ages ago.
> > 
> > I suppose we could simply blacklist any G200eV with a subsystem vendor
> > ID of
> > 0x1014 (IBM) until we figure out how to correct the driver.  Our
> > customers will
> > be deprived, but as it seems to be broken across most of our product
> > lines I
> > doubt any of them are making serious use of it anyway. :)
> > 
> > Something like this? 
> 
> Does X work with the open source drivers ?

Yes, X works fine on at least the IBM System x boxes that have
the Matrox G200eV.

> In that case a better
> approach would be to try to figure out what's different between the way
> the 2 drivers setup the card registers and fix matroxfb..

I suppose someone could attempt this but with vesafb available as
an alternate fb provider and no known demand for repaired G200eV
support in matroxfb, I am not sure what benefit it would provide.

Gary


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] matroxfb: remove incorrect Matrox G200eV support
  2011-03-11 22:23       ` Gary Hade
@ 2011-03-16 19:58         ` Yannick Heneault
  0 siblings, 0 replies; 6+ messages in thread
From: Yannick Heneault @ 2011-03-16 19:58 UTC (permalink / raw)
  To: Gary Hade
  Cc: Benjamin Herrenschmidt, djwong, Linus Torvalds, linux-fbdev,
	linux-kernel, Krzysztof Helt, Petr Vandrovec, Andrew Morton

It impossible that this patch should have work on a system. The patch 
only declare the G200eV as a regular G200 which is not case. Many 
registers are different, including at least the PLL programming 
sequence. If the G200eV is programmed like a regular G200, it will not 
display anything.

Yannick

On 03/11/2011 05:23 PM, Gary Hade wrote:
> On Mon, Mar 07, 2011 at 07:59:16PM +1100, Benjamin Herrenschmidt wrote:
>    
>> On Fri, 2011-03-04 at 12:29 -0800, Darrick J. Wong wrote:
>>      
>>> This is all quite strange -- 2.5 years ago when I wrote the patch it
>>> seemed to
>>> work ok.  On newer revisions of the x3650M2 it seems broken.  The
>>> original
>>> machine I wrote it for was cut up ages ago.
>>>
>>> I suppose we could simply blacklist any G200eV with a subsystem vendor
>>> ID of
>>> 0x1014 (IBM) until we figure out how to correct the driver.  Our
>>> customers will
>>> be deprived, but as it seems to be broken across most of our product
>>> lines I
>>> doubt any of them are making serious use of it anyway. :)
>>>
>>> Something like this?
>>>        
>> Does X work with the open source drivers ?
>>      
> Yes, X works fine on at least the IBM System x boxes that have
> the Matrox G200eV.
>
>    
>> In that case a better
>> approach would be to try to figure out what's different between the way
>> the 2 drivers setup the card registers and fix matroxfb..
>>      
> I suppose someone could attempt this but with vesafb available as
> an alternate fb provider and no known demand for repaired G200eV
> support in matroxfb, I am not sure what benefit it would provide.
>
> Gary
>
>
>    


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-03-16 19:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-01 17:50 matroxfb: remove incorrect Matrox G200eV support Gary Hade
2011-03-01 18:29 ` Linus Torvalds
2011-03-04 20:29   ` [PATCH v2] " Darrick J. Wong
2011-03-07  8:59     ` Benjamin Herrenschmidt
2011-03-11 22:23       ` Gary Hade
2011-03-16 19:58         ` Yannick Heneault

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).