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

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2274 bytes --]

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@redhat.com>
> > Acked-by: Jani Nikula <jani.nikula@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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx