[Intel-gfx] [PATCH] drm/i915: Use intel_panel_actually_set_backlight() to disable PWM backlight

[Jani Nikula]

On Wed, 24 Oct 2018, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> On Wed, Oct 24, 2018 at 05:35:51PM +0100, Chris Wilson wrote:
>> Quoting Ville Syrjälä (2018-10-24 17:33:17)
>> > On Wed, Oct 24, 2018 at 05:02:18PM +0100, Chris Wilson wrote:
>> > > Quoting Ville Syrjala (2018-10-24 16:52:08)
>> > > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> > > > 
>> > > > Use intel_panel_actually_set_backlight() instead of a direct
>> > > > call to pwm_config() in pwm_disable_backlight().
>> > > > 
>> > > > The main benefit is consistent debug logging when we turn off the
>> > > > backlight. Currently we see nothing in dmesg which made me wonder
>> > > > whether the backlight was even getting turned off properly.
>> > > > 
>> > > > The second benefit is consistency; This is what we do for all
>> > > > the other backlight implementations.
>> > > 
>> > > It will also have the effect of calling
>> > > intel_panel_compute_brightness(0) which one presumes is desired?
>> > 
>> > We do it for everything else so it must be good?
>> > 
>> > > 
>> > > Just worrying if the inverted brightness quirk is ever used with pwm.
>> > 
>> > If we have to invert for normal operation I don't know why
>> > we wouldn't want to invert when shutting down the backlight.
>> 
>> Neither do I, just seems weird to set pwm to full to turn it off.
>
> I guess it's just a matter of active low vs. active high. But yeah, not
> sure we should be doing this at all when going through the pwm
> subsystem. Looks like it already has something to handle the inverted
> polarity.

I haven't seen a valid use of invert brightness outside of PCI id 0x2a42
and haven't seen a bug report about it in years. I think otherwise we
should be covered with the proper use of the polarity bit.

I wouldn't oppose a patch warning about invert brightness use outside of
gen4 or even more specific.

On the patch at hand,

Acked-by: Jani Nikula <jani.nikula at intel.com>


BR,
Jani.


>
>> 
>> Whatever, the disparity is silly,
>> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
>
> Ta.

-- 
Jani Nikula, Intel Open Source Graphics Center