Re: [PATCH] drm/i915: Use intel_panel_actually_set_backlight() to disable PWM backlight

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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

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

Ta.

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