Re: [PATCH 2/4] drm/i915: protect backlight registers and data with a spinlock

[Chris Wilson <chris@chris-wilson.co.uk>]

On Fri, Apr 12, 2013 at 03:18:37PM +0300, Jani Nikula wrote:
> Backlight data and registers are fiddled through LVDS/eDP modeset
> enable/disable hooks, backlight sysfs files, asle interrupts, and register
> save/restore. Protect the backlight related registers and driver private
> fields using a spinlock.
> 
> The locking in register save/restore covers a little more than is strictly
> necessary, including non-modeset case, for simplicity.
> 
> v2: Cover register access, save/restore, i915_read_blc_pwm_ctl() and code
>     paths leading there.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Looks reasonable.

intel_panel_actually_set_backlight() should have a WARN_ON(!spinlocked);

The irqness of the register writes scares me slightly - since the IRQ in
question is from ACPI and we have a few bug reports along the lines of
"backlight makes the entire system sluggish" i.e. commonly associated
with bad interrupt handling. Whilst you are looking at updating the
backlight programming, can you look at pushing the writes from out
of the interrupt handler?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre