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

[Jiang Liu (Gerry)]

On 2013/6/14 12:07, Alexander E. Patrakov wrote:
> 2013/6/14 Jiang Liu (Gerry) <jiang.liu@xxxxxxxxxx>:
> > On 2013/6/14 10:30, Yinghai Lu wrote:
> > > On Thu, Jun 13, 2013 at 7:09 PM, Jiang Liu (Gerry) <jiang.liu@xxxxxxxxxx>
> > > wrote:
> > > > On 2013/6/14 2:42, Yinghai Lu wrote:
> > > > > On Thu, Jun 13, 2013 at 9:32 AM, Jiang Liu <jiang.liu@xxxxxxxxxx> wrote:
> > > > > > Alexander E. Patrakov <patrakov@xxxxxxxxx> 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);
> > ---
>
>
> The patch helped, thanks. Note: I have tested it together with
> pci_move_pcibios_add_bus_down.patch, I don't know yet if
> pci_move_pcibios_add_bus_down.patch is needed.
What's the situation now? Could you please send out dmesgs\ioports\iomem?
Thanks!

> --
> Alexander E. Patrakov
>
> .


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/