| Message ID | 20191119151818.67531-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/i915 / LPSS / mfd: Select correct PWM controller to use based on VBT | expand |
On Tue, 19 Nov 2019, Hans de Goede <hdegoede@redhat.com> wrote: > Hi All, > > This series needs to be merged through a single tree, to keep things > bisectable. I have even considered just squashing all 3 patches into 1, > but having separate commits seems better, but that does lead to an > intermediate state where the backlight sysfs interface will be broken > (and fixed 2 commits later). See below for some background info. > > The changes to drivers/acpi/acpi_lpss.c and drivers/mfd/intel_soc_pmic_core.c > are quite small and should not lead to any conflicts, so I believe that > it would be best to merge this entire series through the drm-intel tree. > > Lee, may I have your Acked-by for merging the mfd change through the > drm-intel tree? > > Rafael, may I have your Acked-by for merging the acpi_lpss change through the > drm-intel tree? > > Regards, > > Hans > > p.s. > > The promised background info: > > We have this long standing issue where instead of looking in the i915 > VBT (Video BIOS Table) to see if we should use the PWM block of the SoC > or of the PMIC to control the backlight of a DSI panel, we rely on > drivers/acpi/acpi_lpss.c and/or drivers/mfd/intel_soc_pmic_core.c > registering a pwm with the generic name of "pwm_backlight" and then the > i915 panel code does a pwm_get(dev, "pwm_backlight"). > > We have some heuristics in drivers/acpi/acpi_lpss.c to not register the > lookup if a Crystal Cove PMIC is presend and the mfd/intel_soc_pmic_core.c > code simply assumes that since there is a PMIC the PMIC PWM block will > be used. Basically we are winging it. > > Recently I've learned about 2 different BYT devices: > Point of View MOBII TAB-P800W > Acer Switch 10 SW5-012 > > Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS > PWM controller (and the VBT correctly indicates this), so here our old > heuristics fail. > > This series renams the PWM lookups registered by the LPSS / > intel_soc_pmic_core.c code from "pwm_backlight" to "pwm_soc_backlight" resp. > "pwm_pmic_backlight" and in the LPSS case also dropping the heuristics when > to register the lookup. This combined with teaching the i915 panel to call > pwm_get for the right lookup-name depending on the VBT bits resolves this. Hans, thanks for your continued efforts in digging into the bottom of this! I'm sure there are a number of related bugs still open at fdo bugzilla. It all makes sense, Acked-by: Jani Nikula <jani.nikula@intel.com> for merging through whichever tree. Thanks, Jani.
On Tue, Nov 19, 2019 at 04:18:15PM +0100, Hans de Goede wrote: > Hi All, > > This series needs to be merged through a single tree, to keep things > bisectable. I have even considered just squashing all 3 patches into 1, > but having separate commits seems better, but that does lead to an > intermediate state where the backlight sysfs interface will be broken > (and fixed 2 commits later). See below for some background info. > > The changes to drivers/acpi/acpi_lpss.c and drivers/mfd/intel_soc_pmic_core.c > are quite small and should not lead to any conflicts, so I believe that > it would be best to merge this entire series through the drm-intel tree. > > Lee, may I have your Acked-by for merging the mfd change through the > drm-intel tree? > > Rafael, may I have your Acked-by for merging the acpi_lpss change through the > drm-intel tree? > Entire series (or a single patch) makes sense to me. Thanks for fixing this old hardware! FWIW, Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Regards, > > Hans > > p.s. > > The promised background info: > > We have this long standing issue where instead of looking in the i915 > VBT (Video BIOS Table) to see if we should use the PWM block of the SoC > or of the PMIC to control the backlight of a DSI panel, we rely on > drivers/acpi/acpi_lpss.c and/or drivers/mfd/intel_soc_pmic_core.c > registering a pwm with the generic name of "pwm_backlight" and then the > i915 panel code does a pwm_get(dev, "pwm_backlight"). > > We have some heuristics in drivers/acpi/acpi_lpss.c to not register the > lookup if a Crystal Cove PMIC is presend and the mfd/intel_soc_pmic_core.c > code simply assumes that since there is a PMIC the PMIC PWM block will > be used. Basically we are winging it. > > Recently I've learned about 2 different BYT devices: > Point of View MOBII TAB-P800W > Acer Switch 10 SW5-012 > > Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS > PWM controller (and the VBT correctly indicates this), so here our old > heuristics fail. > > This series renams the PWM lookups registered by the LPSS / > intel_soc_pmic_core.c code from "pwm_backlight" to "pwm_soc_backlight" resp. > "pwm_pmic_backlight" and in the LPSS case also dropping the heuristics when > to register the lookup. This combined with teaching the i915 panel to call > pwm_get for the right lookup-name depending on the VBT bits resolves this. >
Hi, On 19-11-2019 20:33, Patchwork wrote: > == Series Details == > > Series: drm/i915 / LPSS / mfd: Select correct PWM controller to use based on VBT > URL : https://patchwork.freedesktop.org/series/69686/ > State : failure > > == Summary == > > CI Bug Log - changes from CI_DRM_7377 -> Patchwork_15332 > ==================================================== > > Summary > ------- > > **FAILURE** > > Serious unknown changes coming with Patchwork_15332 absolutely need to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_15332, please notify your bug team to allow them > to document this new failure mode, which will reduce false positives in CI. > > External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15332/index.html > > Possible new issues > ------------------- > > Here are the unknown changes that may have been introduced in Patchwork_15332: > > ### IGT changes ### > > #### Possible regressions #### > > * igt@gem_exec_suspend@basic-s0: > - fi-icl-u3: [PASS][1] -> [DMESG-WARN][2] > [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7377/fi-icl-u3/igt@gem_exec_suspend@basic-s0.html > [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15332/fi-icl-u3/igt@gem_exec_suspend@basic-s0.html False positive, the oops reported here has nothing to do with this patch series, the test system in question does not even seem to use a DSI panel, so this entire patch-set basically is a no-op. Regards, Hans