Hi 2012/6/27 Daniel Vetter <daniel.vetter at ffwll.ch>: > This reverts commit f82cfb6bcda164ef3a66b8c3fc549b1f9bdd09ad. > > This breaks the backlight controls on my IVB asus zenbook with an eDP > panel. > > I guess the right fix would be to read this bit and use either the pch > or the cpu register to frob the backlight values. But that is stuff > for -next. > I was about to propose *exactly* this patch and just noticed it is here, but for -fixes instead of dinq. After playing with the CPU/PCH backlight registers, this is what I concluded: - If you want to control backlight just using the PCH: - You have to turn on BLM_PCH_PWM_ENABLE (PCH backlight enable) - You have to turn on the BLM_PCH_OVERRIDE_ENABLE bit - You have to use BLC_PWM_PCH_CTL2 to set the backlight value - Just ignore the CPU registers - If you want to control backlight using the CPU: - You have to turn on BLM_PWM_ENABLE (CPU backlight enable) - You have to turn on BLM_PCH_PWM_ENABLE (PCH backlight enable) - You have to turn *OFF* the BLM_PCH_OVERRIDE_ENABLE bit - You have to use BLC_PWM_CPU_CTL2 to set the backlight value This ivb_pcm_pwm_override function turns the override bit ON, so it completely relies on the PCH registers. The problem is that our code only changes the CPU registers... It was probably relying on the values set by the bios, and I would guess that changing the backlight levels was probably not working at all (I don't have an IVB machine with LVDS to test). If people start complaining that your revert causes problems my suggestion would be: - Undo your revert, and instead of removing the whole function, just remove the code that turns the override bit on (1 << 30). Explicitly turn it off. - Then resend the patch that kills the whole function to dinq and apply it on top of the patch I just sent a few minutes ago. Your rework of the backlight registers + my fix should probably make it safe to completely remove ivb_pch_pwm. -- Paulo Zanoni
