please add "geode" to driver list in hw/xfree86/common/xf86AutoConfig.c

Bug #219630 reported by Martin-Éric Racine on 2008-04-19
24
Affects Status Importance Assigned to Milestone
xorg (Ubuntu)
Medium
Unassigned
Hardy
Medium
Unassigned
xserver-xorg-video-geode (Ubuntu)
Undecided
Unassigned
Hardy
Undecided
Unassigned
xserver-xorg-video-nsc (Ubuntu)
Undecided
Unassigned
Hardy
Undecided
Unassigned

Bug Description

Binary package hint: xserver-xorg-core

To complete auto-configuration of the -geode driver in X.org, "geode" must be either added to hw/xfree86/common/xf86AutoConfig.c or made to replace the deprecated "amd". An ideal compromise would perhaps be to put "geode" right before "amd" in the list, keeping "amd" in the list as a transitional measure, for those upgrading from older releases to Hardy.

Martin-Éric Racine (q-funk) wrote :

Here's a patch made from upstream git.

Timo Aaltonen (tjaalton) wrote :

Fixed in git.

Changed in xorg-server:
importance: Undecided → Medium
status: New → Fix Committed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package xorg-server - 2:1.4.1~git20080131-1ubuntu10

---------------
xorg-server (2:1.4.1~git20080131-1ubuntu10) intrepid; urgency=low

  [Timo Aaltonen]
  * 166_fix_lpl_monitors.diff:
    - fix LPL monitors properly (LP: #204065)

  [Martin-Eric Racine]
  * 167_xf86AutoConfig_geode_addition.diff:
    - Autodetect geode video devices (LP: #219630)

  [Bryce Harrington]
  * 166_fix_lpl_monitors.diff:
    - Modified to apply to Ubuntu
  * 168_closedir.patch:
    - Fix crash on PS3 due to closing a dir that wasn't successfully opened
      (LP: #217647)

 -- Bryce Harrington <email address hidden> Tue, 13 May 2008 00:03:40 -0700

Changed in xorg-server:
status: Fix Committed → Fix Released
Martin Pitt (pitti) wrote :

Accepted into -proposed, please test and give feedback here

Changed in xorg-server:
status: New → Fix Committed

I have a GX2 based device (viglen MPC-l, made by FIC as ION, also sold as Linutop2) where the graphics card has a manufacturer ID of "100b", not "1022" as specified in the patch. As such xorg doesn't auto detect the geode card, and uses vesa instead.

From lspci:-

00:01.1 VGA compatible controller: National Semiconductor Corporation Geode GX2 Graphics Processor

00:01.1 0300: 100b:0030

Further hw details at http://popey.com/~alan/viglen/

Martin-Éric Racine (q-funk) wrote :

What you have is technically an ION 503. Linutop uses another ION variant with an LX800 instead.

The NSC driver has a PCI ID conflict with the GEODE driver, because the GX2 was originally manufactured by NSC. The result is that GX2 support is currently very sketchy in NSC and disabled in GEODE.

We are working on a remedy that would involve importing support for older Geode models into the GEODE driver. This is scheduled to happen sometimes at the end of this year. Until this has been done, I'm afraid that GX2 support falls in the cracks between two drivers.

Steve Langasek (vorlon) wrote :

This fix has been in -proposed now for 20 days; is no one able to test whether it fixes the problem?

Martin-Éric Racine (q-funk) wrote :

Steve, my understanding is that David Bensimon at Canonical has been testing for various Geode issues. He might be able to answer this.

Martin-Éric Racine (q-funk) wrote :

PS: I forgot to mention that the way Debian and Ubuntu handle PCI ID matching seems to differ from upstream's intended method in some ways. Since finding out about this, I have produced two proposed fixes, both sitting in my PPA:

1) an updated -geode package that uses the Debian/Ubuntu-specific method. This package also moves the backward-compatibility symbolic links to the -amd transitional package, because they were found to cause more hard than good, but might still be useful to people initially upgrading from earlier Ubuntu releases that offered -amd.

2) an updated -nsc package that removes some PCI ID that conflict with the -cyrix and -geode packages.

If accepted by the SRU team, I would propose that both be uploaded together into hardy-proposed and released with hardy.1 ASAP.

Martin Pitt (pitti) wrote :

Martin-Eric, can you please minimal debdiffs with appropriate changelogs here? I'll sponsor them.

On Tue, Jun 10, 2008 at 10:13 AM, Martin Pitt wrote:
> Martin-Eric, can you please minimal debdiffs with appropriate changelogs
> here? I'll sponsor them.

For NSC, that's easy enough, since I started from what's currently in Hardy.

For GEODE, just to be safe, comparing to which version should I
produce the diff?

Martin-Éric Racine (q-funk) wrote :

Here's the debdiff for NSC. Some comments:

* config.sub and config.guess were regenerated, otherwise Lintian complained loudly with an E: that the files were too old.
* The ChangeLog mysteriously seems to have changed as a result of this. I have no idea why.

Martin-Éric Racine [2008-06-10 10:12 -0000]:
> For GEODE, just to be safe, comparing to which version should I
> produce the diff?

The one in hardy-proposed, please (2.9.0-1ubuntu2.1). Thanks!

Martin-Éric Racine (q-funk) wrote :

Here's the GEODE debdiff.

Martin Pitt (pitti) wrote :

geode debdiff:
 - why is it necessary to move around the -amd symlink in the SRU, and thus mess with new Conflics/Replaces:?
 - what is the rationale for dropping all the DEB_* settings in debian/rules?

Judging by the noisy changelog diff, this is really a backport of the latest version in intrepid/sid, as opposed to a minimally intrusive patch to fix just this issue. I guess the important bits are the additional ID in geode.ids and moving "script" from -amd to -geode perhaps?

Please supply minimal debdiffs which can be reviewed and avoid any unnecessary change, which might just cause trouble later on.

nsc debdiff: same like above, but amplified by 10. Adding new packages (-dbg), changing the build system, changing config.{guess,sub} are things which are inappropriate for an SRU.

On Thu, Jun 12, 2008 at 11:50 AM, Martin Pitt wrote:
> geode debdiff:
> - why is it necessary to move around the -amd symlink in the SRU, and thus mess with new Conflics/Replaces:?

To make it possible to completely purge the symlinks without purging
the driver. This is necessary to fix operation in LTSP.

> - what is the rationale for dropping all the DEB_* settings in debian/rules?

Not dropped. They were accidentally duplicated before.

> Judging by the noisy changelog diff, this is really a backport of the
> latest version in intrepid/sid, as opposed to a minimally intrusive
> patch to fix just this issue. I guess the important bits are the
> additional ID in geode.ids and moving "script" from -amd to -geode
> perhaps?

geode.ids and making the symbolic links removable.

> Please supply minimal debdiffs which can be reviewed and avoid any
> unnecessary change, which might just cause trouble later on.

All the above changes *are* necessary. Trying to avoid them earlier is
what resulted in 2.1 not fixing anything.

> nsc debdiff: same like above, but amplified by 10. Adding new packages
> (-dbg), changing the build system, changing config.{guess,sub} are
> things which are inappropriate for an SRU.

I'm sorry, but I cannot work with the XSF scripts.

Martin Pitt (pitti) wrote :

Thanks for the -geode clarifications. That one is OK with me then.

> > nsc debdiff: same like above, but amplified by 10. Adding new packages
> > (-dbg), changing the build system, changing config.{guess,sub} are
> > things which are inappropriate for an SRU.

> I'm sorry, but I cannot work with the XSF scripts.

That means what? You need help from Bryce or Timo to backport just the relevant fixes or to revert the irrelevant packaging changes?

Martin-Éric Racine (q-funk) wrote :

> Thanks for the -geode clarifications. That one is OK with me then.
>
>> > nsc debdiff: same like above, but amplified by 10. Adding new packages
>> > (-dbg), changing the build system, changing config.{guess,sub} are
>> > things which are inappropriate for an SRU.
>
>> I'm sorry, but I cannot work with the XSF scripts.
>
> That means what? You need help from Bryce or Timo to backport just the
> relevant fixes or to revert the irrelevant packaging changes?

If they can just revert the Debian auto-generagtion patch and install
the static nsc.id, yes, that would work too.

Bryce Harrington (bryce) wrote :

As per comment #9 (which Martin-Eric and I have discussed separately), the patch to xserver that this sru was originally filed for is not necessary. Like it was said in #9, Debian uses an entirely different mechanism for detecting pci ids, so patching support into the xserver is unnecessary, and this AFAWK the 2:1.4.1~git20080131-1ubuntu9.1 changeset can be dropped as an SRU candidate. The change is not incorrect, it's just probably harmlessly irrelevant; if we find otherwise we may re-introduce the patch, but if so it should first be verified in Intrepid as actually needed.

I would recommend that new SRU's be opened each for the -geode and -nsc updates. It sounds from pitti's review like the patches need further baking and positive verification in Intrepid before bringing to the SRU team. Once those bugs are opened, this SRU can be closed as invalid.

Steve Langasek (vorlon) wrote :

Please open SRU tasks on this bug for the related -geode and -nsc updates, rather than opening a new bug.

If the xserver change is "harmlessly irrelevant", I believe that package should be dropped from hardy-proposed. Let me know if you disagree with this for whatever reason.

Bryce Harrington (bryce) wrote :

Yes, please go ahead and drop it.

Changed in xorg-server:
status: Fix Committed → Invalid
Bryce Harrington (bryce) wrote :

I've uploaded Martin-Eric's current -nsc and -geode into intrepid, although as per pitti's feedback the patches need further work before they can be backported.

Changed in xserver-xorg-video-geode:
status: New → In Progress
Changed in xserver-xorg-video-nsc:
status: New → In Progress
Martin Pitt (pitti) wrote :

I removed xorg-server from hardy-proposed.

I already said in comment 19 that I'm ok with the -geode patch, confirming this. The -nsc patch needs work, since it currently introduces a NEW package, and changes other things in the packaging.

Changed in xserver-xorg-video-geode:
status: New → Confirmed
Kees Cook (kees) wrote :

As a quick note, to clear up any possible confusion, the xorg-server security update that just went out was numbered ubuntu9.2 to avoid the -proposed version (and did not contain the 9.1 code). So all is well and the next upload into -proposed will need an ubuntu9.3 version, as expected.

Martin-Éric Racine (q-funk) wrote :

Please see my PPA for updated XSERVER-XORG-CORE, NSC and GEODE packages:

NSC: Required one change from Debian, because Hardy has an older version of debhelper. Otherwise, pretty much usable as-is. The main change it introduces is the elimination of PCI ID for non-NSC devices from the nsc.ids it ships. Everything else is to bring the deprecated package in line with recent Debian policy changes.

GEODE: is a backport of 2.10.0, which includes both the BIOS-less DDC polling from 2.9.0, plus the addition of support for the OLPC XO-1 and several bug fixes. Since supporting new hardware is suitable for an SRU, I'm hereby nominating this for Hardy.1 release.

XSERVER-XORG-CORE: I have made exactly one change to the Hardy metapackage, to replace the dependency on AMD with a dependency on GEODE. The rest is identical.

NOTE: several users have confirmed that the only to succeed in getting the GEODE and NSC drivers to work in LTSP is by upgrading to the combination of above drivers and updated X core. The key problem seems to be that the backward-compatibility symbolic link between geode_drv.so and amd_drv.so doesn't work as intended on Debian/Ubuntu (it works as expected on other distributions), plus the GX2 code in NSC simply doesn't work as advertised, which requires letting GEODE handle it. Also, the metapackage's dependency on the transitional package (which contains those symbolic links) seems to hinder one's chances of letting APT autoremove it.

Martin-Éric Racine (q-funk) wrote :

Here's the NSC debdiff.

Martin-Éric Racine (q-funk) wrote :

Here's the GEODE debdiff.

Martin-Éric Racine (q-funk) wrote :

Adding this might have some beneficial side-effects, but appears to be ignored by Debian/Ubuntu in favor of the DRIVER.ids file provided by each chipset driver.

Marking as invalid for xorg-server, since fixing this really requires updated -geode and -nsc instead. Those packages have already been approved for Intrepid, but still need to be backported for Hardy, to permanently close the issue for LTS.

Changed in xorg-server:
status: Fix Released → Invalid
Martin-Éric Racine (q-funk) wrote :

To retrace my steps and verify that this indeed solves the Geode issues, some people might find either of the two following methods suitable:

http://q-funk.blogspot.com/2008/06/howto-make-geode-thin-clients-work-on.html

http://q-funk.blogspot.com/2008/06/howto-build-clean-ltsp-boot-image-that.html

Martin Pitt (pitti) wrote :

Martin-Eric,

the geode debdiff in comment 14 (http://launchpadlibrarian.net/15171834/debdiff_xf86-video-geode.txt) looks ok, and I already ack'ed this. Your new debdiff in comment 27 is huge again, and again changes a lot of unrelated things (build system, etc.) and also has an inappropriate changelog delta. Again: For SRUs, there should be *one* changelog entry with an appropriate description (readable by mere-mortal computer users) with an LP: #xxxxxx bug reference, and a minimal patch. So is the previous debdiff still valid?

nsc: still huge. Getting the new upstream version is ok for me (since it's a sharply focused device driver), and thus the autotools/libtool noise is ok, but changing all the packaging underneath it seems dangerous to me (debian/xsfbs/*.mk). How do these changes influence the build and the generated .debs?

To finally get this SRU done, I really advise you to prepare a proper SRU for hardy, instead of just keep preparing intrepid/unstable backports. Take the new upstream version, use the hardy packaging, and just apply the patches which are necessary, this will make it much easier to review, and much less prone to regressions.

Thank you!

Martin-Éric Racine (q-funk) wrote :

Martin,

Avoiding a full port and trying to squeeze every file into the framework already set by 2.8.0 is precisely what has resulted in failed attempts to resolve Hardy's issue in the past. The issue is fully resolved in Intrepid and proven to work precisely because everything went in; only Hardy remains unresolved.

nsc: this is not a new upstream. It's a new Debian only.

Martin-Éric Racine (q-funk) wrote :

Things that need to happen to really fix this for Hardy:

1) make xserver-xorg-video-all stop depending on -amd and start depending on -geode.

2) import an updated Debian version of -nsc that removes conflicting PCI ID. What's in Intrepid cannot build as-is, because Hardy has an old dpkg-dev, so I modified this as per the package in my PPA.

3) import an updated Debian version of -geode that completely eliminates the symbolic links to amd_drv.so and amd.ids, and reintroduces the PCI ID for GX2 hardware. This must include a warning to users who are upgrading to update their static xorg.conf if they have one. Optionally, it could use the new 2.10.0 upstream which adds support for OLPC hardware, but this is not a must. I could recycle the packaging used there with a 2.9.0 release for Hardy.

Martin-Éric Racine [2008-06-24 9:55 -0000]:
> Avoiding a full port and trying to squeeze every file into the framework
> already set by 2.8.0 is precisely what has resulted in failed attempts
> to resolve Hardy's issue in the past.

That's not what I asked for; using the new upstream version is ok. I
just wanted an explanation why the build system changes
(debian/xsf*/*.mk) were necessary.

Martin-Éric Racine (q-funk) wrote :

Since this involves changes to 3 packages, let's address those separately:

1) xorg metapackage: Depending on the transitional -amd package is the wrong thing to do. It should have been changed to depend on -geode before Hardy was released.

2) nsc: changes in XSF scripts were performed by Brice Goglin at Debian. Of what I understood, this fixes previously improper target cleaning. This being said, I'm not that familiar with XSF scripts (I use CDBS for GEODE), so perhaps him or Bryce Harrington would manage to explain how this works this better than me.

3) geode: I think this one has been abundantly discussed. :)

Martin-Éric Racine (q-funk) wrote :

The consolidated change for NSC would look like this (minus the change from hardy to hardy-proposed).

Martin Pitt (pitti) wrote :

http://launchpadlibrarian.net/15556613/debdiff_2.8.3-2_to_2.8.3-2ubuntu1.txt looks ok to me now, approved for SRU. I'll sponsor this in a bit.

(Note to self: for sponsoring, change hardy -> hardy-proposed and fix LP: # reference syntax in changelog.)

Martin Pitt (pitti) wrote :

Sponsored -nsc, upload waiting in the queue for Steve to process (we are in 8.04.1 freeze now).

Changed in xserver-xorg-video-nsc:
status: New → In Progress
Martin-Éric Racine (q-funk) wrote :

Here's the consolidated changes for GEODE against what's already in hardy-proposed.

Martin-Éric Racine (q-funk) wrote :

PS: given how long this issue has been dragging on, is there any chances of getting a freeze exception for 8.04.1 at all, on all 3 packages?

Martin-Éric Racine (q-funk) wrote :

Here's the changes for xorg.

Martin-Éric Racine (q-funk) wrote :

Typos fixed here. Used this one for xorg, instead.

Martin Pitt (pitti) wrote :

-geode looks good now. Sponsored, and uploaded to the queue, waiting for Steve's review/ack of freeze exception.

Changed in xserver-xorg-video-geode:
status: Confirmed → In Progress
Martin Pitt (pitti) wrote :

xorg dependency change needs to be applied to intrepid.

Changed in xorg-server:
status: Invalid → Triaged
Martin Pitt (pitti) wrote :

Your last xorg debdiff was against the wrong package, but that was trivial to fix. Really uploaded debdiff attached.

Sponsored, waiting for Steve to ack now.

Changed in xorg:
status: Invalid → In Progress
Martin-Éric Racine (q-funk) wrote :

xorg dependency change is for Hardy.

AFAIK it's already fixed for Intrepid. Bryce, Timo, can you confirm?

Martin-Éric Racine (q-funk) wrote :

Another consequence of this cherry-picking, as opposed to simply importing existing changes where all issues have been dealt with, is that it's easy to forget parts of the solution.

In this particular case, GEODE misses a "Conflicts: xserver-xorg-video-amd (<< ${binary:Version})" on the xserver-xorg-video-geode target.

Martin Pitt (pitti) wrote :

Reuploaded -geode with the added Conflicts: . It isn't strictly necessary, it already has the corresponding Replaces:, but let's be correct here.

Steve Langasek (vorlon) wrote :

I'm not happy to see the debian build system changes in the -nsc SRU; they should be almost entirely irrelevant, but in the process they still introduce a risk of regression. This is not something that should ever be included in an SRU, and if I approve it it's only because I feel I'm wasting my time trying to get a minimal diff when this point has already been made to no avail, and we might as well accept the package into -proposed and regression-test it there.

Given the late landing, therefore, I believe this SRU should be held until after 8.04.1 is buildable from hardy-updates only.

BTW, it's also very confusing to have a patch which changes, and then /comments out/ a line; it took me far too long to parse this:

-+ awk '/^#define.*PCI_CHIP/ {print $$3}' ${srcdir}/nsc_driver.c | sed -e 's/0x/1078/' > nsc.ids
-+ awk '/^#define.*PCI_CHIP/ {print $$3}' ${srcdir}/nsc_driver.c | sed -e 's/0x/100B/' >> nsc.ids
++ awk '/^#define.*PCI_CHIP/ {print $$3}' ${srcdir}/nsc_driver.c | sort -u | grep -v 0030 | sed -e 's/0x/100B/' > nsc.ids
++# awk '/^#define.*PCI_CHIP/ {print $$3}' ${srcdir}/nsc_driver.c | sort -u | grep -v 0030 | sed -e 's/0x/1078/' >> nsc.ids

because I kept overlooking the '#' at the beginning of that line...

Since this changes which driver is being used for a set of chips, which AIUI aren't unsupported by nsc they're just supported "better" by the cyrix driver, SRU validation needs to include feedback from users of these precise chips. (Ideally, the SRU justification would also include explicit pointers to the problems with using the nsc driver instead of the cyrix driver, which AFAICS is completely absent here.) This also contributes to making this an inappropriate change to include at the last minute before 8.04.1.

Martin-Éric Racine (q-funk) wrote :

Steve, I have kept the diff as small as humanely possible. This being said, fixing both packaging issues and hardware issues within the scope of the same SRU has not been easy. My apologies if it's still too big to fit the constraints of an SRU.

One important thing to keep in mind is to avoid PCI ID conflicts in X.org at all cost. Right now, we have some in 8.04.0 and they prevent LTSP from operating normally with any Geode hardware covered by any of those 3 drivers. Given this, waiting until until 8.04.2 to fix this is out of the question.

Instead, we need to find a compromise that is mutually acceptable to everyone, to get this into 8.04.1 now, especially given how we need to get all 3 changes in at the same time, because of versioned Conflicts present in 2 of the 3 packages (xserver-xorg-video-all, xserver-xorg-video-geode, xserver-video-nsc) I proposed for this SRU.

I suppose that I could revert the Debian build system changes made by Brice Goglin, but I feel uneasy doing so, because the Debian XSF scripts are a real mess that I simply cannot work with. I'll instead trust that Brice made his changes right and accept them verbatim, minus one small change to make this build on Hardy dependencies.

I welcome your thoughts on any last- minute change that the Ubuntu X team or myself could implement to make this more acceptable for this SRU.

As for hardware testing, I have only access to hardware covered by NSC and GEODE drivers. The above fix restores the operation on those. I unfortunately don't have access to CYRIX hardware.

Steve Langasek (vorlon) wrote :

I've reviewed the -geode package in -proposed, and discussed this with Martin-Eric on IRC. Documenting the outcome of that discussion here.

There are two changes that should be made to the -geode SRU if possible:

- The changelog is incomplete, it doesn't document the fact that the previously-removed PCI ID is now re-added. This seems to be caused by an ambiguous comment from Martin. :)
- The description of the -amd transitional package now suggests that static xorg.conf should be updated before purging the package. In fact, a static xorg.conf will be broken as soon as the transitional package is /installed/. This should be clarified to reduce confusion.

We also discussed the -nsc package further, and I've agreed to accept a reduced SRU for 8.04.1 that *only* drops the geode PCI ID, a change which has been tested elsewhere and a case which is known to be broken today. The cyrix PCI IDs should not be changed this close to the point release.

Steve Langasek (vorlon) wrote :

Accepted xorg into hardy-proposed, please test and give feedback here.

Changed in xorg:
importance: Undecided → Medium
status: In Progress → Fix Committed
Bryce Harrington (bryce) wrote :

Uploaded a stripped down version of the patch to hardy-proposed.

Changed in xserver-xorg-video-nsc:
status: In Progress → Fix Committed
Bryce Harrington (bryce) wrote :

Here's the debdiff I uploaded for -nsc. In fact the version currently in hardy didn't seem to even build, due to failure to apply the 01 patch; this fixes things up so it builds properly. Someone should please test this against geode hardware (ideally both stuff using -geode, and stuff using -nsc, but at least the former).

Martin-Éric Racine (q-funk) wrote :

We'd need the updated -all and -geode for the conflict to not happen during testing.

Bryce Harrington (bryce) wrote :

Added -geode to -all in xorg on Intrepid, so the -geode/Intrepid task is completed.

Changed in xserver-xorg-video-geode:
status: In Progress → Fix Released
Bryce Harrington (bryce) wrote :

-nsc on Intrepid has been sync'd to debian, so this task is done too.

Changed in xserver-xorg-video-nsc:
status: In Progress → Fix Released
Changed in xorg:
status: Triaged → Fix Released
Martin-Éric Racine (q-funk) wrote :

-geode is also in sync with the newer Debian 2.10.0-3 in Intrepid.

Dan Quade (danquade) wrote :

After this upgrade I cannot login anymore. The resolution changes with every keystroke in KDM (Hardy 8.04 64bit)

Martin-Éric Racine (q-funk) wrote :

unimatrix, exactly which packages did you upgrade and to which versions? On exactly what thin client hardware (brand name, manufacturer, model)?

Dan Quade (danquade) wrote :

I upgraded everything that was there to upgrade. Here's a list:
x11-common 1:7.3+10ubuntu10.1
xserver-xorg-input-all 1:7.3+10ubuntu10.1
xserver-xorg-video-all 1:7.3+10ubuntu10.1
xserver-xorg 1:7.3+10ubuntu10.1
xbase-clients 1:7.3+10ubuntu10.1
xorg 1:7.3+10ubuntu10.1
xutils 1:7.3+10ubuntu10.1
All the update changelogs pointed me to this launchpad bug, so I assumed I'm at the right place, right?

The problem is X-specific (happens in KDM and KDE), with USB and PS/2 keyboards.
My machine is a desktop PC, AMD 3500+ 64bit, graphic card is a nVidia 6600 (the problem stays with both nv or nvidia drivers).

Dan Quade (danquade) wrote :

Another quick update:
It is impossible to type even when I connect to the computer over VNC (no letters get through, but the resolution doesn't jump either).
The only thing that works (locally and over VNC) is the numpad when numlock is on.

Martin-Éric Racine (q-funk) wrote :

In that case, it's an entirely different issue than the one being discussed here.
Please file a new bug against package 'xorg' and explain the issue again there.

Dan Quade (danquade) wrote :

Yeah I also downgraded the packages and nothing changed. It really has nothing to do with this. Weird coincidence though.

Steve Langasek (vorlon) wrote :

-nsc package is also now accepted into hardy-proposed.

Steve Langasek (vorlon) on 2008-06-26
Changed in xserver-xorg-video-geode:
milestone: none → ubuntu-8.04.1
Changed in xserver-xorg-video-nsc:
milestone: none → ubuntu-8.04.1
Bryce Harrington (bryce) wrote :

Is this a more correct description?

Package: xserver-xorg-video-amd
Architecture: i386
Priority: extra
Depends: xserver-xorg-video-geode
Conflicts: xserver-xorg-video-geode (<< ${binary:Version})
Replaces: xserver-xorg-video-geode (<< ${binary:Version})
Description: Geode GX2/LX display driver (dummy transitional package)
 This transitional package helps users of the existing AMD driver to update
 their X.org setup. It can safely be removed after the upgrade is completed.
 .
 If you are using a static xorg.conf, note that installation of this
 transitional package will break static xorg.conf's. If you're using a
 static xorg.conf, please make sure to edit the Device section and change
 the Driver value from "amd" to "geode".
 .
 In all other cases, you can safely purge this package immediately.

Steve Langasek (vorlon) wrote :

xserver-xorg-video-geode accepted into -proposed, please test.

There are two issues that I've noted in the latest upload:
- xserver-xorg-video-amd is made to conflict/replace older versions of xserver-xorg-video-geode. I don't see any justification for this; there are no file conflicts between the two packages (since the -amd package is now essentially empty!); if the intent was to ensure that the -geode package is upgraded, it would have been more appropriate to use a versioned depends. In practice, I don't believe this is enough of an issue to block pushing to -proposed (or to -updates).
- Bryce has re-raised the question of whether dropping the amd_drv.so symlink is appropriate/necessary. It *will* be a regression for anyone who was using a static xorg.conf successfully with Driver "amd" in 8.04, as this config will suddenly stop working. Now, it may be the case that the set of users who are able to use Driver "amd" successfully in 8.04 is zero, in which case this change makes sense and is not a regression; but so far, the only justification given is that the symlink somehow causes problems for LTSP users, who are a subset of Ubuntu users and not the only use case that needs to be taken into account in an SRU.

Resolution of this latter issue is a condition of considering this SRU verified, and it should not be copied to -updates until we have a definitive answer here.

Changed in xserver-xorg-video-geode:
status: In Progress → Fix Committed
Bryce Harrington (bryce) wrote :

Here's a version with those two issues addressed. Hopefully this is deployable. I've also uploaded this to hardy-proposed.

My question about dropping the symlink is mostly a curiosity if with all the other adjustments we've made, if whatever the original issue is has been addressed, and no longer requires dropping the link for amd_drv.so. I can't seem to find a decent description of the original problem that prompted dropping the link as a solution, so it's hard to understand if dropping it is still necessary.

Therefore, I would suggest we deploy this version without that omission, let the LTSP people test, and if they still have trouble, to file a better bug report about it so we can understand what the actual problem is.

Martin-Éric Racine (q-funk) wrote :

NO! Deploying without removing the symbolic link is exactly what we want to avoid. The symbolic link MUST GO. This is already verified. Let's not make the same mistake as for 2.1, shall we?

Martin-Éric Racine (q-funk) wrote :

For NSC, it looks good - although I would have felt safer removing Cyrix ID numbers.

For GEODE, the Conflicts against NSC has the wrong version. It should be << new NSC with the fixed ID, including the 1: epoch.

Martin-Éric Racine (q-funk) wrote :

I thought that it had been discussed to oblivion but, much to my amazement, nobody seems to remember why dropping the symbolic link is essential:

The symbolic link creates PCI conflicts too, because both the driver and the one called via the symbolic link claim support for the same chips.

So, yes, the symbolic link must be dropped for this update to really fix the issue.

Marc Tardif (cr3) wrote :

First, the nsc driver has been confirmed to work on Koolu hardware from the packages available in hardy-proposed as of June 26th. Furthermore, they were apparently known to work previously as well from the following versions:

ii xorg 1:7.3+10ubuntu10 X.Org X Window System
ii xserver-xorg 1:7.3+10ubuntu10 the X.Org X server
ii xserver-xorg-core 2:1.4.1~git20080131-1ubuntu9.1 Xorg X server - core server
ii xserver-xorg-video-ati 1:6.8.0-1 X.Org X server -- ATI display driver
ii xserver-xorg-video-geode 2.9.0-4ubuntu2 X.org server -- Geode GX2/LX display driver
ii xserver-xorg-video-nsc 1:2.8.3-2ubuntu3.3 X.Org X server -- NSC display driver

Bryce Harrington (bryce) wrote :

Martin, the problem is that the evidence to support your claim is not documented in a bug report anywhere.

It is quite possible there is a solution we could deploy *other* than dropping the symbolic link, but because no one has adequately documented the problem, it's difficult to understand. Maybe you're right, but I want to see the proof.

Bryce Harrington (bryce) wrote :

Debs with the above patch are here:

http://people.ubuntu.com/~bryce/Testing/geode/

If someone could please install these, along with -geode_2.9.0-4ubuntu3 from hardy-proposed, and verify the issue with LTSP is resolved, then I think we can upload this xserver patch.

Steve Langasek (vorlon) wrote :

Oliver Grawert has provided some analysis of this symlink case and found that the symlink itself does not cause problems; the problems only manifest when the kernel framebuffer module is loaded. I'm persuaded by his and Bryce's investigations that the symlink is not the direct cause of the problems, and that even if it is indirectly related, for compatibility reasons dropping the symlink is not the appropriate choice of action in the SRU due to the implied regressions.

I'm therefore accepting Bryce's subsequent upload of the -geode package into hardy-proposed, which restores the symlink. If you still see problems with LTSP and the amd_drv.so link installed, please first try to blacklist the kernel framebuffer module (Oliver can provide a recipe for this) and report the results.

Martin-Éric Racine (q-funk) wrote :

Guys, please don't do this. This has been abundantly tested enough as it is, by myself and others. The symlink causes too many problems. It was removed for good reasons. Do NOT re-introduce it!

Martin-Éric Racine (q-funk) wrote :

Steve, no, blacklisting the framebuffer module doesn't fix the issue.

However, if you REALLY insist on keeping the symbolic link, then at least make it purgeable by putting it in the -amd transitional package, not in the main -geode package.

Steve Langasek (vorlon) wrote :

On Sat, Jun 28, 2008 at 05:41:06AM -0000, Martin-Éric Racine wrote:
> Guys, please don't do this. This has been abundantly tested enough as it
> is, by myself and others. The symlink causes too many problems. It was
> removed for good reasons. Do NOT re-introduce it!

The problem is that we have a very real risk of regression for users who
have static xorg.conf, and still no clear understanding of what the reasons
are that necessitated the removal of the symlink; i.e., we have the
statement that it broke LTSP, but on the other hand we don't have any logs
that show what broke, we have a statement of the X maintainer that it
shouldn't have broken, and we have Oliver's tests which show it didn't break
for him.

Can we please pinpoint what's different between the systems where it broke
for you, and the system where Oliver found that it worked fine with the
symlink? When it appeared that the symlink broke the use of the driver for
all or most users, I was willing to allow this as an SRU; now it appears
that others who have tested the package can't reproduce the problem, which
both lowers the benefit of the SRU (since it's not broken for everyone) and
increases the severity of the regression (since users who have Driver "amd"
set don't necessarily have unusable systems).

> However, if you REALLY insist on keeping the symbolic link, then at
> least make it purgeable by putting it in the -amd transitional package,
> not in the main -geode package.

Oh yes, that would be fine with me; I don't understand why the compat
symlink wasn't put there in the first place. If you think that's the best
option, I can do an upload of this tomorrow.

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
<email address hidden> <email address hidden>

Martin-Éric Racine (q-funk) wrote :

Oliver hasn't specified the system on which he tested this, so I cannot comment on his results.

However, I have tested this on a variety of Geode-based systems by a variety of vendors and found the exact same behavior on each of them: that leaving the symbolic link confuses X, while removing it makes LTSP work where it previously didn't and doesn't affect operation on standalone hosts that don't use a static config (hosts with a static config need their config updated because of the X anyhow).

As for the symbolic link, after trying various approaches, I have come to the conclusion that removing it and expecting users to upgrade their static config (if any - not everyone uses it) once for all was the best approach, while leaving the symbolic link there only postpones the unavoidable day when the config has to be upgraded. It also removes the confusion that X experiences with the -configure option.

I'll also point out that blacklisting the framebuffer is entirely the wrong approach and essentially moves the problem to another package where it doesn't belong. Besides, on some systems, such as the OLPC, the driver is *expected* to leverage the framebuffer.

Oliver Grawert (ogra) wrote :

i was testing (for multiple hours) on the DBE61 and what i found is that LTSP due to its extremely short bootprocess brings up X *exactly* while the framebuffer initializes. this race condition produces a hardlock with black screen. delaying X by some seconds or backlisting the framebuffer (like it was in all former releases for the majority of the framebuffer drivers) solves that issue ... it didnt matter if the symlink was in place or not (i tested all possible package combinations that night). the actual LTSP problem is the frambuffer here which will be disabled completely for 8.04.2 as nothing in LTSP makes use of framebuffers at all.

Martin-Éric Racine (q-funk) wrote :

Oliver, I'm sorry but I haven't observed this behavior you describe on any of the LX hardware I have access to.

However, removing the symbolic link solved the issue on each and every LX hardware I tested, while leaving it prevented X from launching, again on every LX hardware I have access to.

Steve Langasek (vorlon) wrote :

no reports of regressions with -nsc, copying to hardy-updates.

Changed in xserver-xorg-video-nsc:
status: Fix Committed → Fix Released
Steve Langasek (vorlon) wrote :

On Mon, Jun 30, 2008 at 12:17:02PM -0000, Martin-Éric Racine wrote:
> Oliver, I'm sorry but I haven't observed this behavior you describe on
> any of the LX hardware I have access to.

> However, removing the symbolic link solved the issue on each and every
> LX hardware I tested, while leaving it prevented X from launching, again
> on every LX hardware I have access to.

It turns out that the package that has been uploaded to -proposed was not a
valid test. Although the changelog said the symlink had been re-added, it
was not, and I didn't notice this when accepting the package.

I'm uploading a package to -proposed, which does add the symlink back,
*only* to the transitional -amd package. That means that any problems will
be restricted to users who are upgrading from gutsy and have the -amd
package still installed.

I have asked Oliver to do a spot test by re-adding the symlink to his LTSP
chroot by hand, to confirm that this breaks things for him too. I fully
expect that it will.

I have also asked Marc Tardif to do a local install test, to check whether
this symlink breaks anything for users who aren't using LTSP's Xorg
-configure, since that's the use case (users with a static xorg.conf who are
upgrading) where we potentially have a regression from 8.04.

*Only* if Marc reports that it works for him with the symlink present, I
will accept the symlink-ing package into -proposed. (If he reports that it
doesn't work, then presumably the symlink won't provide any benefit to users
with static xorg.conf, either.)

I'll give this to the end of day today (UTC-7) to be resolved; if we don't
reach a resolution, I'll default to the package currently in -proposed which
does not include the symlink.

FWIW, I agree with you in principle that leaving the symbolic link there is
postponing the inevitable; unfortunately, we have to contend with the fact
that this symbolic link did *not* get dropped prior to the release of Ubuntu
8.04, and dropping it now may represent a regression for users who are
already an Ubuntu 8.04. We should avoid that if at all possible.

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
<email address hidden> <email address hidden>

Steve Langasek (vorlon) wrote :

xorg copied to hardy-updates.

Changed in xorg:
status: Fix Committed → Fix Released
Oliver Grawert (ogra) wrote :

ok, actually adding/removing the link manually instead of assuming the package does it helps to verify.
if the symlink is there in LTSP Xorg -configure fails completely, locking the device (you can switch consoles but the boot stalls and no getty's or X come up, you are left with a blinking cursor at the top left of a black terminal).
since the only purpose of Xorg -configure is to create a basic xorg.conf the ltsp scripts can parse to replace values, it is also not possible to use lts.conf variables like XSERVER=geode here.
the only solution apart from removing the -amd package (which should then remove the symlink) will be to use a pregenerated xorg.conf.
both solutions sadly also require a resource intensive rebuild of the client image with ltsp-update-image to take effect.

Martin-Éric Racine (q-funk) wrote :

Oliver, please note that now that xserver-xorg-video-all finally Depends on -geode rather than -amd, it should be possible to build a clean chroot that will work out of the box, since the symbolic link will not be present in -geode. Thus, this should finally work OK with the packages currently in -proposed.

Marc Tardif (cr3) wrote :

After installing the alternate image from 20080701 on a Koolu device, the display appears properly with a resolution of 1280x1024. The loaded driver is geode from the xserver-xorg-video-geode package version 2.9.0-1ubuntu.2.3.

Upon request from Steve Langasek, I have run the following commands and then restarted X:

  cd /usr/lib/xorg/modules/drivers
  sudo ln -s geode_drv.so amd_drv.so

After restarting, the display still appears properly and the same driver is loaded.

Steve Langasek (vorlon) wrote :

Because of Marc's confirmation that the symlink does not have an adverse effect when not using Xorg -configure, and the fact that this symlink will only be present in the transitional -amd package which is no longer pulled in via -video-all, I'm going ahead and accepting the ubuntu2.4 package into hardy-proposed because I believe this is the least-disruptive option for an SRU.

Please test and give feedback, so that if there are any regressions that have been overlooked, we can try to address them in the short time remaining before 8.04.1.

Martin Pitt (pitti) wrote :

-geode copied to hardy-updates.

Changed in xserver-xorg-video-geode:
status: Fix Committed → Fix Released
Martin-Éric Racine (q-funk) wrote :

Actually, GEODE 2.9.0-1ubuntu2.4 still conflicts against an incorrect version of NSC. Please look at the NSC version that is in hardy-updates and make GEODE 2.9.0-1ubuntu2.5 conflict against the previous release (including the epoch in the NSC version!).

Martin-Éric Racine (q-funk) wrote :

Therefore making xserver-xorg-video-geode

Conflicts: xserver-xorg-video-nsc (<< 1:2.8.3-2ubuntu0.1)

Martin-Éric Racine (q-funk) wrote :

...which, again, only apply to hardy-updates. Intrepid already has what we need.

Steve Langasek (vorlon) wrote :

On Sat, Jul 05, 2008 at 05:09:34PM -0000, Martin-Éric Racine wrote:
> Actually, GEODE 2.9.0-1ubuntu2.4 still conflicts against an incorrect
> version of NSC. Please look at the NSC version that is in hardy-updates
> and make GEODE 2.9.0-1ubuntu2.5 conflict against the previous release
> (including the epoch in the NSC version!).

Reading the changelog of the package, I don't see an explanation of why the
conflicts was needed at all. The only explanation I can think of is that it
was to force the removal of the old version of nsc which provided
overlapping PCI IDs. That's a reasonable thing to do, but I don't think
it's sufficient justification for another SRU round.

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
<email address hidden> <email address hidden>

Martin-Éric Racine (q-funk) wrote :

This is to conflict against older versions of NSC that include overlapping PCI ID numbers. This way, either someone updates both packages, or GEODE will remove the outdated NSC to ensure that no PCI ID conflict will remain.

To post a comment you must log in.