Re: [BUGFIX 0/9] Fix bug 59501 and code improvement for dock driver

["Jiang Liu (Gerry)" <jiang.liu@huawei.com>]

On 2013/6/14 10:30, Yinghai Lu wrote:
> On Thu, Jun 13, 2013 at 7:09 PM, Jiang Liu (Gerry) <jiang.liu@huawei.com> wrote:
>> On 2013/6/14 2:42, Yinghai Lu wrote:
>>>
>>> On Thu, Jun 13, 2013 at 9:32 AM, Jiang Liu <jiang.liu@huawei.com> wrote:
>>>>
>>>> Alexander E. Patrakov <patrakov@gmail.com> reports two bugs related to
>>>> dock station support on Sony VAIO VPCZ23A4R. Actually there are at least
>>>> four bugs related to Sony VAIO VPCZ23A4R dock support.
>>>> 1) can't correctly detect hotplug slot for dock state
>>>> 2) resource leak on undocking
>>>> 3) resource allocation failure for dock devices
>>>> 4) one bug in intel_snd_hda driver
>>>>
>>>> The first patch fixes issue 1, and the second patch fixes issue 2.
>>>> These two patches, if accepted, should be material for stable branches
>>>> too.
>>>> Patch 3-9 are code improvement for ACPI and dock driver.
>>>>
>>>> I have found the root cause for issue three, but still working on
>>>> solutions, and seems can't be solve in short time. So please help
>>>> to review and test patches for issue 1) and 2) first.
>>>
>>>
>>> the 3) is about pci resource allocation?
>>> because pcibios_add_bus is called too early?
>>>
>>> If that is case, we should have something like attached patch for it.
>>>
>>> With that, we will not need to worry about _OSC set for 3.10 etc.
>>>
>> Hi Yinghai,
>>     Seems not related to pcibios_add_bus(). According to my
>> investigation, the issue is caused by difference in PCI resource
>> assignment between boot time and runtime hotplug. On x86 platforms,
>> it respects PCI resource assignment from BIOS and only reassign
>> resources for unassigned BARs. But with acpiphp, it ignores BIOS
>> resource assignment and reassign all resources by OS.
>>     If we have enough resources, reassigning all PCI resources should
>> work too, but may fail if we are under resource constraints. On the
>> other handle, current PCI IOMM align algorithm may waste huge MMIO
>> address space if we have some PCI devices with huge IOMM BAR.
>>     On this Sony laptop, BIOS allocates limited IOMM resources for
>> the dock station and the dock station has a gfx which has a 256MB
>> IOMM BAR. So current acpiphp driver fails to allocate resources
>> for most devices on the dock station.
>
> Is it a regression?
Not sure. But a little concern about check_hotplug_bridge(), it treats
dock station and devices on dock station with _EJD as hot-plug-gable
PCI bus and reserve extra resources for possible hot-adding. But I
think we should only reserve extra resource for dock station, and should
not reserve resource for devices on station with _EJD method.

>
>>      Currently I'm trying to change acpiphp to respect BIOS resource
>> assignment by calling pcibios_survey_resource_bus(), as in pci_root.c.
>> The other way is to change the IOMM resource allocation algorithm,
>> but obviously it's much more risky of regressions if changing the
>> algorithm.
>
> that is not going to help, need to increase bridge resource.
>
> please check if BIOS have setup option about hotplug MMIO pad size.
For the first step, I'm trying to make hotplug case work in the same way 
as boot time. Do you think this patch help?

diff --git a/drivers/pci/hotplug/acpiphp_glue.c 
b/drivers/pci/hotplug/acpiphp_gl
index 270fdba..12e3f6e 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -837,13 +837,13 @@ static int __ref enable_device(struct acpiphp_slot 
*slot)
                                 max = pci_scan_bridge(bus, dev, max, pass);
                                 if (pass && dev->subordinate) {
                                         check_hotplug_bridge(slot, dev);
- 
pci_bus_size_bridges(dev->subordinate);
+ 
pcibios_resource_survey_bus(dev->subordi
                                 }
                         }
                 }
         }

-       pci_bus_assign_resources(bus);
+       pci_assign_unassigned_bus_resources(bus);
         acpiphp_sanitize_bus(bus);
         acpiphp_set_hpp_values(bus);
         acpiphp_set_acpi_region(slot);
---

>
> Yinghai
>
> .
>