Hi Hans,

On Thu, Jul 09, 2020 at 04:40:56PM +0200, Hans de Goede wrote:
> On 7/9/20 4:14 PM, Sam Ravnborg wrote:
> > On Wed, Jul 08, 2020 at 11:14:16PM +0200, Hans de Goede wrote:
> > > Here is v4 of my patch series converting the i915 driver's code for
> > > controlling the panel's backlight with an external PWM controller to
> > > use the atomic PWM API. See below for the changelog.
> > 
> > Why is it that i915 cannot use the pwm_bl driver for backlight?
> > I have not studied the code - just wondering.
> 
> The intel_panel.c code deals with 7 different types of PWM controllers
> which are built into the GPU + support for external PWM controllers
> through the kernel's PWM subsystem.
> 
> pwm_bl will work for the external PWM controller case, but not for
> the others. On top of that the intel_panel code integrates which
> the video BIOS, getting things like frequency, minimum value
> and if the range is inverted (0% duty == backlight brightness max).
> I'm not even sure if pwm_bl supports all of this, but even if it
> does the intel_panel code handles this in a unified manner for
> all supported PWM controllers, including the ones which are
> an integral part of the GPU.

pwm_bl handles inverted PWM just fine. I'm unsure what "integrates which
the video BIOS" means, but I don't see how "handling 7 different types
of PWM controllers explicitly and others using the PWM API" can be seen
as "unified manner" compared to "provide a pwm driver for whatever might
be in the GPU and then use generic code (PWM API, pwm_bl) to drive it".

Maybe I'm not understanding some involved complexity here?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |