Bug 14176

so the regressin happens at:

commit db54501900ad3665dd669f5708ecd04fc5aed495
Author: David Müller (ELSOFT AG) <d.mueller@elsoft.ch>
Date:   Sat Aug 29 08:54:45 2009 +0200

    drm/i915: Improve CRTDDC mapping by using VBT info
    
    Use VBT information to determine which DDC bus to use for CRTDCC.
    Fall back to GPIOA if VBT info is not available.
    
    Signed-off-by: David Müller <d.mueller@elsoft.ch>
    Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
    Signed-off-by: Eric Anholt <eric@anholt.net>
    Tested on: 855 (David), and 945GM, 965GM, GM45, and G45 (anholt)

I've done some more digging around and have the additional information about what exactly is going wrong. 

It looks like I'm bailing out of intel_init_bios early at drivers/gpu/drm/i915/intel_bios.c:396:

    bios = pci_map_rom(pdev, &size);
    if (!bios)
        return -1;

It is around this time, that I see
[    0.447220] i915 0000:00:02.0: BAR 6: can't allocate resource (bogus alignment) [0x0-0x0] flags 0x0
[    0.447227] [drm] failed to find VBIOS tables

It looks like this was happening even before the offending commit, but I never saw any symptoms that bothered me.

This commit however introduces the new variable dev->dev_priv->crt_ddc_bus which is supposed to be initialised to -1 in a call to parse_general_definitions down at 418, but that never actually happens because I bail out before then. It therefore looks like dev->dev_priv->crt_ddc_bus=0 instead.

The real problem with this occurs at drivers/gpu/drm/i915/intel_crt.c:552:

    /* Set up the DDC bus. */
    if (IS_IGDNG(dev))
        i2c_reg = PCH_GPIOA;
    else {
        i2c_reg = GPIOA;
        /* Use VBT information for CRT DDC if available */
        if (dev_priv->crt_ddc_bus != -1)
            i2c_reg = dev_priv->crt_ddc_bus;
    }

The nested if statment was added in this commit. Before, I was walking away with i2c_reg=GPIOA, which gave me the desired behaviour. Now that's getting overwritten with 0 since that's the erroneous value of dev_priv->crt_ddc_bus which is not -1.

In conclusion, we would seemingly need one or both of the following 

1) make sure that dev_priv->crt_ddc_bus = -1 gets set even if intel_init_bios doesn't complete.
2) fix the call to pci_map_rom so that intel_init_bios does complete.

The first one seems like the easiest. I really have no idea what's happening with pci_map_rom.

Kevin

Good catch.  I'd like to figure out why your ROM isn't getting mapped; not having VBT data available makes many things more difficult as we're depending on it for various things these days.

Based on the dmesg, it sounds like we're not actually allocating any resource space for it?  If you enable the PCI debug output you may get more info about why the video ROM BAR isn't getting space.

Unfortunately, it doesn't appear that pci has as nice a debugging infrastructure as drm. However, with a few printks, I was able to trace the following codepath

drivers/pci/rom.c:pci_map_rom


    if (res->flags & IORESOURCE_ROM_SHADOW) {
    ...
    } else {
        if (res->flags &
             (IORESOURCE_ROM_COPY | IORESOURCE_ROM_BIOS_COPY)) {

        } else {
    }

Oops, ignore the above.

I traced it down to falling off the end of the following switch statement in kernel/resource.c:579

resource_size_t resource_alignment(struct resource *res)
{
        switch (res->flags & (IORESOURCE_SIZEALIGN | IORESOURCE_STARTALIGN)) {
        case IORESOURCE_SIZEALIGN:
                return resource_size(res);
        case IORESOURCE_STARTALIGN:
                return res->start;
        default: 
                return 0;
        }
}

So it looks like pdev->res->flags has neither IORESOURCE_SIZEALIGN  nor IORESOURCE_STARTALIGN set. Where is this supposed to happen?

Hi, Kevin
    Please try the latest linus kernel(2.6.33-rc1/rc2) and see whether the issue still exists?
    Maybe this issue can be fixed by the following commit:
    >commit 29874f44fbcbc24b231b42c9956f8f9de9407231
Author: Shaohua Li <shaohua.li@intel.com>
Date:   Wed Nov 18 15:15:02 2009 +0800
 
    drm/i915: fix gpio register detection logic for BIOS without VBT

Thanks.

This looks like it should make things work a bit better. Unfortunately I won't have an external vga display to test it for an other week or so. In any case, I hate to be a spoiler, but doesn't this rely on the uninitialised value of crt_ddc_bus being 0? Nominally this appears to be the case, but is this a safe assumption?

I've been digging around to try and determine why I'm not getting space for my vbt and am having trouble pinning anything down. 

I have confirmed that drm_get_dev() at drivers/gpu/drm/drm_stub.c: 400 is returning successfully. I'm guessing however that it somewhere around here that I should be getting space allocated for my ROM. In particular pci_enable_device() looks like a good place. This is just a wrapper to __pci_enable_device() (drivers/pci/pci.c: 930) where flags=(IORESOURCE_MEM | IORESOURCE_IO) and

	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
		if (dev->resource[i].flags & flags)
			bars |= (1 << i);

	err = do_pci_enable_device(dev, bars);

However my kernel doesn't boot if i stick a printk in there to see what "bars" is likely because this function is used very early on before such printing is possible. Am I even on the right track here?

I do however note that the first line in the above posted printks has video ACPI saying "rom: no". Is it possible that my hardware is just set up to supply the necessary information?

I have confirmed that 29874f44fbcbc24b231b42c9956f8f9de9407231 restores the expected behaviour in my case, however my previous question about the generality of this fix remains.

Marking as fixed since you no longer have the problem.  If you still have the ROM allocation failure and want to track it down you could try the linux-pci@vger.kernel.org list and/or file a PCI bug.