Re: v3.14-rc7: Backlight control broken on Dell XPS13

Re: v3.14-rc7: Backlight control broken on Dell XPS13

[Daniel Vetter]

Please never just send a mail to maintainers, always cc mailing lists.

On Mon, Mar 17, 2014 at 2:33 PM, Romain Francoise <romain@orebokech.com> wrote:
> Moving from v3.13 to v3.14-rc7 breaks backlight control on my Dell
> XPS13, which I assume is because of this change:
>
> http://git.kernel.org/linus/bc0bb9fd1c7810407ab810d204bbaecb255fddde
>
> Is this right? Is backlight supposed to work correctly without the quirk
> now?

Yeah, it should work without the quirk. Have you tested with the patch
reverted to confirm that it's indeed this commit and not something
else (e.g. in the acpi code)?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: v3.14-rc7: Backlight control broken on Dell XPS13

[Romain Francoise]

Hi Daniel,

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> Please never just send a mail to maintainers, always cc mailing lists.

Yes, sorry, I sent this privately because it wasn't a fully-formed bug
report; I just wanted Jani to confirm that it wasn't intentionally
broken as of -rc7.

> Have you tested with the patch reverted to confirm that it's indeed
> this commit and not something else (e.g. in the acpi code)?

I hadn't, but I've now reverted the revert and things work again as in
v3.13. So as far as I can tell the quirk is still definitely needed.

Thanks!
-r

Re: v3.14-rc7: Backlight control broken on Dell XPS13

[Jani Nikula]

On Mon, 17 Mar 2014, Romain Francoise <romain@orebokech.com> wrote:
> Hi Daniel,
>
> Daniel Vetter <daniel.vetter@ffwll.ch> writes:
>
>> Please never just send a mail to maintainers, always cc mailing lists.
>
> Yes, sorry, I sent this privately because it wasn't a fully-formed bug
> report; I just wanted Jani to confirm that it wasn't intentionally
> broken as of -rc7.
>
>> Have you tested with the patch reverted to confirm that it's indeed
>> this commit and not something else (e.g. in the acpi code)?
>
> I hadn't, but I've now reverted the revert and things work again as in
> v3.13. So as far as I can tell the quirk is still definitely needed.

I still don't think having the quirk is the way to go. Let's fix this
properly, shall we?

Please file a new bug on DRM/Intel component at [1]. Please attach
intel_reg_dumper output (tool from intel-gpu-tools [2]) with and without
the revert. Please also attach dmesg with drm.debug=0xe from early boot.

Do you have the snb or ivb model?

BR,
Jani.


[1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI
[2] http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/

>
> Thanks!
> -r

-- 
Jani Nikula, Intel Open Source Technology Center

Re: v3.14-rc7: Backlight control broken on Dell XPS13

[Jani Nikula]

On Mon, 17 Mar 2014, Jani Nikula <jani.nikula@intel.com> wrote:
> On Mon, 17 Mar 2014, Romain Francoise <romain@orebokech.com> wrote:
>> Hi Daniel,
>>
>> Daniel Vetter <daniel.vetter@ffwll.ch> writes:
>>
>>> Please never just send a mail to maintainers, always cc mailing lists.
>>
>> Yes, sorry, I sent this privately because it wasn't a fully-formed bug
>> report; I just wanted Jani to confirm that it wasn't intentionally
>> broken as of -rc7.
>>
>>> Have you tested with the patch reverted to confirm that it's indeed
>>> this commit and not something else (e.g. in the acpi code)?
>>
>> I hadn't, but I've now reverted the revert and things work again as in
>> v3.13. So as far as I can tell the quirk is still definitely needed.
>
> I still don't think having the quirk is the way to go. Let's fix this
> properly, shall we?
>
> Please file a new bug on DRM/Intel component at [1]. Please attach
> intel_reg_dumper output (tool from intel-gpu-tools [2]) with and without
> the revert. Please also attach dmesg with drm.debug=0xe from early boot.

The register dump from *before* i915 gets loaded is also interesting.

BR,
Jani.

>
> Do you have the snb or ivb model?
>
> BR,
> Jani.
>
>
> [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI
> [2] http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/
>
>>
>> Thanks!
>> -r
>
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

Re: v3.14-rc7: Backlight control broken on Dell XPS13

[Romain Francoise]

Jani Nikula <jani.nikula@intel.com> writes:

> Please file a new bug on DRM/Intel component at [1]. Please attach
> intel_reg_dumper output (tool from intel-gpu-tools [2]) with and without
> the revert. Please also attach dmesg with drm.debug=0xe from early
> boot.

Done, https://bugs.freedesktop.org/show_bug.cgi?id=76276

Let me know if you need more info.

> Do you have the snb or ivb model?

IVB.

> The register dump from *before* i915 gets loaded is also interesting.

That's more difficult to procure, is it really necessary?

Thanks,
-r

Re: v3.14-rc7: Backlight control broken on Dell XPS13

[Jani Nikula]

On Mon, 17 Mar 2014, Romain Francoise <romain@orebokech.com> wrote:
> Hi Daniel,
>
> Daniel Vetter <daniel.vetter@ffwll.ch> writes:
>
>> Please never just send a mail to maintainers, always cc mailing lists.
>
> Yes, sorry, I sent this privately because it wasn't a fully-formed bug
> report; I just wanted Jani to confirm that it wasn't intentionally
> broken as of -rc7.
>
>> Have you tested with the patch reverted to confirm that it's indeed
>> this commit and not something else (e.g. in the acpi code)?
>
> I hadn't, but I've now reverted the revert and things work again as in
> v3.13. So as far as I can tell the quirk is still definitely needed.

Here's two patches to try, separately, *without* the quirk:

Patch #1:

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index cb058408c70e..ef26115dbe68 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -659,7 +659,7 @@ static void pch_enable_backlight(struct intel_connector *connector)
 	/* This won't stick until the above enable. */
 	intel_panel_actually_set_backlight(connector, panel->backlight.level);
 
-	pch_ctl2 = panel->backlight.max << 16;
+	pch_ctl2 = panel->backlight.max << 16 | panel->backlight.level;
 	I915_WRITE(BLC_PWM_PCH_CTL2, pch_ctl2);
 
 	pch_ctl1 = 0;

Patch #2:

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index cb058408c70e..10849104fa1e 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -669,6 +669,8 @@ static void pch_enable_backlight(struct intel_connector *connector)
 	I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1);
 	POSTING_READ(BLC_PWM_PCH_CTL1);
 	I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_PWM_ENABLE);
+
+	intel_panel_actually_set_backlight(connector, panel->backlight.level);
 }
 
 static void i9xx_enable_backlight(struct intel_connector *connector)



-- 
Jani Nikula, Intel Open Source Technology Center

Re: v3.14-rc7: Backlight control broken on Dell XPS13

[Romain Francoise]

Jani Nikula <jani.nikula@intel.com> writes:

> Here's two patches to try, separately, *without* the quirk: [...]

Okay, I've tried both and neither works. However, I've noticed that
while the backlight level never changes even though I'm changing it with
Fn+F4/Fn+F5, it *does* get registered somewhere because if I bring it
all the day down and reboot, the brightness is reduced in the next BIOS
POST (loading i915 gets it back to 100%).

Thanks,
-r