[Intel-gfx] [PATCH v2] pwm: lpss: Make builtin so that i915 can find the pwm_backlight

[Hans de Goede]

Hi,

On 20-01-17 08:18, Thierry Reding wrote:
> On Fri, Jan 20, 2017 at 08:03:33AM +0100, Thierry Reding wrote:
>> On Thu, Jan 19, 2017 at 06:58:30PM +0100, Hans de Goede wrote:
>>> The primary consumer of the lpss pwm is the i915 kms driver,
>>> the i915 driver does not support get_pwm returning -EPROBE_DEFER and
>>> its init is very complex making this is almost impossible to fix.
>>>
>>> This commit changes the PWM_LPSS Kconfig from a tristate to a bool, so
>>> that when the i915 driver loads the lpss pwm will be available avoiding
>>> the -EPROBE_DEFER issue. Note that this is identical to how the same
>>> problem was solved for the pwm-crc driver, which is used by the i915
>>> driver on other platforms.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>> Acked-by: Jani Nikula <jani.nikula at intel.com>
>>> ---
>>> Changes in v2:
>>> -Drop the pwm_add_table call (this has been moved to the acpi_lpss driver)
>>> ---
>>>  drivers/pwm/Kconfig | 12 +++---------
>>>  1 file changed, 3 insertions(+), 9 deletions(-)
>>
>> For the record I think this is completely wrong and i915 should be
>> taught how to deal with -EPROBE_DEFER. We've gone through a lot of
>> pain to clean up this kind of init-level ordering on other devices
>> and the result is, in my opinion, a *lot* better than what we had
>> before. It'd be shame to see i915 backpedal on that.
>
> Looking into i915 a little, I don't see why handling -EPROBE_DEFER would
> be very complicated. The call stack looks somewhat like this:
>
> 	i915_pci_probe()
> 	i915_driver_load()
> 	i915_load_modeset_init()
> 	intel_modeset_init()
> 	intel_setup_outputs()
> 	intel_dsi_init()
> 	intel_panel_setup_backlight()
> 	pwm_setup_backlight()
>
> In the above, intel_modeset_init() is the last one that propagates
> errors, but its caller, i915_load_modeset_init() properly handles
> failure from other function calls. Also, pwm_setup_backlight() can
> return errors to intel_panel_setup_backlight(), which will in turn
> propagate them to intel_dsi_init().
>
> So I'd think that in order to properly handle -EPROBE_DEFER you'd only
> need to propagate errors back up this way:
>
> 	intel_panel_setup_backlight()
> 	intel_dsi_init()
> 	intel_setup_outputs()
> 	intel_modeset_init()
>
> That seems to me to be far from "almost impossible".

I will let the i915 devs answer this.

IIRC a lot of setup is already done at this point, including some changes
to the hw and we really do not want to change the init-sequence, or
repeat parts of it.

I myself was actually thinking about solving this by figuring out if the
pwm is necessary and getting it early on, but the figuring out bit is non
trivial.

Anyways as said I will let the i915 devs answer this.

Regards,

Hans