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

[Thierry Reding]

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".

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20170120/7cec94ed/attachment.sig>