| Message ID | 20180923135812.29574-6-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | Resume BYT/CHT LPSS I2C controller before the iGPU | expand |
On 09/23/2018 04:58 PM, Hans de Goede wrote: > On some Bay Trail systems the GPU ACPI fwnode has power-resources which > point to the PMIC, which is connected over the LPSS I2C5 controller. > > This one was quite nasty to debug, unlike on CHT where the same problem > leads to errors like these: > > i2c_designware 808622C1:06: controller timed out > ACPI Error: AE_ERROR, Returned by Handler for [UserDefinedRegion] > ACPI Error: Method parse/execution failed \_SB.P18W._ON, AE_ERROR > video LNXVIDEO:00: Failed to change power state to D0 > > On BYT the read-modify-write done by drivers/acpi/pmic/intel_pmic_xpower.c > on the AXP288 PMIC register to change the power-resource state *seems* to > succeed. > > But in reality, because the I2C controller has not been resumed yet, the > read silently fails and returns the wrong value, where as the write does > succeed, writing back the wrong value for all the other power-resources > in the same register, turning off a bunch of them. Which of course does > not end well. > > This commit adds a RPM consumer link from the GPU (which has a LNXVIDEO > HID) to the BYT LPSS I2C5 controller, so that the I2C controller gets > resumed before the GPU is resumed and thus before we try to change the > power-resource. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Note I'm also submitting an independent patch to the i2c-designware driver > to catch the case of i2c transfers being done before the drivers resume() > method has completed, to make debugging future similar problems easier. > --- > drivers/acpi/acpi_lpss.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c > index fe37fd67331d..75672004db87 100644 > --- a/drivers/acpi/acpi_lpss.c > +++ b/drivers/acpi/acpi_lpss.c > @@ -473,6 +473,7 @@ struct lpss_device_links { > static const struct lpss_device_links lpss_device_links[] = { > {"808622C1", "7", "80860F14", "3", DL_FLAG_PM_RUNTIME}, > {"808622C1", "7", "LNXVIDEO", NULL, DL_FLAG_PM_RUNTIME}, > + {"80860F41", "5", "LNXVIDEO", NULL, DL_FLAG_PM_RUNTIME}, > }; I have a BYT with this PUNIT configuration although I haven't seen this problem. Most probably due not having a stressful test setup enough. But this patch doesn't break anything either for me so Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c index fe37fd67331d..75672004db87 100644 --- a/drivers/acpi/acpi_lpss.c +++ b/drivers/acpi/acpi_lpss.c @@ -473,6 +473,7 @@ struct lpss_device_links { static const struct lpss_device_links lpss_device_links[] = { {"808622C1", "7", "80860F14", "3", DL_FLAG_PM_RUNTIME}, {"808622C1", "7", "LNXVIDEO", NULL, DL_FLAG_PM_RUNTIME}, + {"80860F41", "5", "LNXVIDEO", NULL, DL_FLAG_PM_RUNTIME}, }; static bool hid_uid_match(struct acpi_device *adev,
On some Bay Trail systems the GPU ACPI fwnode has power-resources which point to the PMIC, which is connected over the LPSS I2C5 controller. This one was quite nasty to debug, unlike on CHT where the same problem leads to errors like these: i2c_designware 808622C1:06: controller timed out ACPI Error: AE_ERROR, Returned by Handler for [UserDefinedRegion] ACPI Error: Method parse/execution failed \_SB.P18W._ON, AE_ERROR video LNXVIDEO:00: Failed to change power state to D0 On BYT the read-modify-write done by drivers/acpi/pmic/intel_pmic_xpower.c on the AXP288 PMIC register to change the power-resource state *seems* to succeed. But in reality, because the I2C controller has not been resumed yet, the read silently fails and returns the wrong value, where as the write does succeed, writing back the wrong value for all the other power-resources in the same register, turning off a bunch of them. Which of course does not end well. This commit adds a RPM consumer link from the GPU (which has a LNXVIDEO HID) to the BYT LPSS I2C5 controller, so that the I2C controller gets resumed before the GPU is resumed and thus before we try to change the power-resource. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Note I'm also submitting an independent patch to the i2c-designware driver to catch the case of i2c transfers being done before the drivers resume() method has completed, to make debugging future similar problems easier. --- drivers/acpi/acpi_lpss.c | 1 + 1 file changed, 1 insertion(+)