Created attachment 109061 [details]
Hack to add backlight support for DV8P
Backlight support for Dell Venue 8 Pro (8086:0f31) tablet is currently missing. The attached patch adds support for it in a ugly way and needs to be cleanup by someone more familiar with intel graphic. It is based on a hack made by Jon Pry for his Asus T100.
Shobhit is working on it.
Please attach /sys/kernel/debug/dri/0/i915_opregion.
Created attachment 109099 [details]
i915 opregion of Dell Venue 8 Pro
What's the current status of Shobhits proper backlight implementation? Is there any patch available that we can test on our devices yet?
(In reply to Jan-Michael Brummer from comment #0)
> Created attachment 109061 [details]
> Hack to add backlight support for DV8P
> Backlight support for Dell Venue 8 Pro (8086:0f31) tablet is currently
> missing. The attached patch adds support for it in a ugly way and needs to
> be cleanup by someone more familiar with intel graphic. It is based on a
> hack made by Jon Pry for his Asus T100.
Got Aava Inari 8 (8086:0f31 too) with backlight level fixed at boot (and a weird "milk flooding" when DPMS kicks in...) -- should it be safe to try your patch given the same SoC but quite different panel and probably firmware (64-bit one)?
The magic constants in both
look the same at least and the panel specific delay handling seems to have been dropped in the latter but that's about as much as I can read in interdiff.
Michael: the patch in Fedlet is this patch combined with an additional one Jan-Michael sent to me by email which makes the change conditional on a kernel parameter.
Thank you, Adam.
Created attachment 110092 [details]
i915_opregion for Aava Inari 8
Forgot to attach /sys/kernel/debug/dri/0/i915_opregion
As a matter of fact, the patch works on Inari 8 as well.
Does anyone else get this BUG with i915.force_backlight_pmic=1?
(setting it back to 0 makes this disappear)
BUG: scheduling while atomic: Xorg/589/0x00000002
[ 321.034563] Call Trace:
[ 321.034580] [<ffffffff8159679f>] dump_stack+0x4e/0x71
[ 321.034595] [<ffffffff8159386e>] __schedule_bug+0x45/0x53
[ 321.034609] [<ffffffff81599518>] __schedule+0x848/0x860
[ 321.034623] [<ffffffff81599694>] schedule+0x24/0x70
[ 321.034637] [<ffffffff8159c57c>] schedule_hrtimeout_range_clock+0x11c/0x150
[ 321.034653] [<ffffffff810d1a10>] ? update_rmtp+0x70/0x70
[ 321.034669] [<ffffffff810d24bf>] ? hrtimer_start_range_ns+0xf/0x20
[ 321.034684] [<ffffffff8159c5be>] schedule_hrtimeout_range+0xe/0x10
[ 321.034698] [<ffffffff810cf11b>] usleep_range+0x3b/0x40
[ 321.034728] [<ffffffffa002314f>] __i2c_dw_enable+0x2f/0xa0 [i2c_designware_core]
[ 321.034756] [<ffffffffa0023c7c>] i2c_dw_xfer+0x1bc/0x540 [i2c_designware_core]
[ 321.034772] [<ffffffff8144ac6d>] ? __i2c_transfer+0x6d/0x280
[ 321.034786] [<ffffffff8144ac6d>] __i2c_transfer+0x6d/0x280
[ 321.034801] [<ffffffff8144b530>] i2c_transfer+0x50/0xb0
[ 321.034815] [<ffffffff8144bbda>] i2c_master_send+0x3a/0x50
[ 321.034831] [<ffffffff81413635>] regmap_i2c_write+0x15/0x40
[ 321.034845] [<ffffffff8140f2cd>] _regmap_raw_write+0x76d/0x7a0
[ 321.034860] [<ffffffff8140f4a1>] _regmap_bus_raw_write+0x61/0x90
[ 321.034874] [<ffffffff8140e39a>] _regmap_write+0x7a/0x150
[ 321.034888] [<ffffffff8141013a>] regmap_write+0x4a/0x70
[ 321.034904] [<ffffffff814255f1>] intel_soc_pmic_writeb+0x21/0x30
[ 321.034999] [<ffffffffa042d296>] vlv_pmic_disable_backlight+0x26/0x30 [i915]
[ 321.035089] [<ffffffffa042e1c6>] intel_panel_disable_backlight+0x96/0xc0 [i915]
[ 321.035176] [<ffffffffa0423b17>] intel_dsi_post_disable+0x57/0x500 [i915]
[ 321.035260] [<ffffffffa03f7789>] ? intel_disable_pipe+0x129/0x2f0 [i915]
[ 321.035343] [<ffffffffa0401bf4>] i9xx_crtc_disable+0xf4/0x6f0 [i915]
[ 321.035428] [<ffffffffa03fbc61>] intel_crtc_control+0x51/0x120 [i915]
[ 321.035519] [<ffffffffa03fbd97>] intel_crtc_update_dpms+0x67/0x80 [i915]
[ 321.035603] [<ffffffffa0406791>] intel_connector_dpms+0x41/0x70 [i915]
[ 321.035657] [<ffffffffa02f4edc>] drm_mode_obj_set_property_ioctl+0x35c/0x370 [drm]
[ 321.035711] [<ffffffffa02f4f1b>] drm_mode_connector_property_set_ioctl+0x2b/0x30 [drm]
[ 321.035754] [<ffffffffa02e5a56>] drm_ioctl+0x1f6/0x680 [drm]
[ 321.035808] [<ffffffffa02f4ef0>] ? drm_mode_obj_set_property_ioctl+0x370/0x370 [drm]
[ 321.035835] [<ffffffff81021ad9>] ? init_fpu+0x59/0xc0
[ 321.035851] [<ffffffff811cfe4e>] do_vfs_ioctl+0x7e/0x520
[ 321.035865] [<ffffffff811da044>] ? __fget+0x74/0xb0
[ 321.035880] [<ffffffff811d0381>] SyS_ioctl+0x91/0xb0
[ 321.035895] [<ffffffff8159ddac>] ? int_check_syscall_exit_work+0x34/0x3d
[ 321.035910] [<ffffffff8159db29>] system_call_fastpath+0x12/0x17
For those playing along at home, I see on intel-gfx that Shobhit does seem to be working on this stuff. This is the latest version of his RFC patch set:
Doesn't look like it actually does backlight control yet, only enable/disable.
Thanks for the work Shobhit!
Also note that Jan-Michael's hack needs a very minor rediff for 3.19, change this line in the patch:
int intel_panel_setup_backlight(struct drm_connector *connector)
int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe)
and it'll apply (HAND RE-DIFFING FOR TEH WIN)
Created attachment 112786 [details] [review]
parameter-modifiable version of JMB's hack, rediffed for 3.19rc5
OK, so the patch actually needs a bit more rediffing for 3.19rc5. here's the one I'm using; it's a combination of this one and a follow-up JMB sent me via email which allows it to be toggled with a parameter. Set 'i915.force_backlight_pmic=1' to use the hack, otherwise the driver will act as normal.
damnit, that version doesn't work either, i'll attach another later.
Did you look at 3.19rc6, Adam? (my -rc4 attempts have been undermined by brcmfmac-related lockups so far, trying to come up with another bugreport)
I got it to compile but it hangs on KMS activation, I suspect my hacks to get pipe were wrong. I was planning to build without the patch today.
I don't think anything's changed between rc5 and rc6, I've been looking directly at Linus' tree.
(In reply to Adam Williamson from comment #11)
> For those playing along at home, I see on intel-gfx that Shobhit does seem
> to be working on this stuff. This is the latest version of his RFC patch set:
> Doesn't look like it actually does backlight control yet, only
> Thanks for the work Shobhit!
Backlight control will need a backlight class driver and I am just waiting for the current approach to get accepted before implementing that. Implemented in current patch set enable/disable to atleast save power during suspend/resume.
http://lists.freedesktop.org/archives/intel-gfx/2015-February/060160.html seems to be Shobhit's latest upstream state of the art.
J-M sent me a version of his hack re-diffed against 3.19...which again fails to apply against current 3.20-pre. I re-diffed it again for 3.20-pre and it applies and boots, but doesn't actually seem to control anything.
So, I'm gonna do a fedlet kernel build with Shobhit's latest patch instead and see how that goes; that might be something other folks could use to test, if they're running Fedlet or a variant of it or something.
(In reply to Adam Williamson from comment #18)
> I re-diffed it again for 3.20-pre and it applies and boots
Care to attach? ;-)
> but doesn't actually seem to control anything.
...that's my experience with code in 3.19 as well. There was some hope that it might eliminate slowly whitening screen (see comment 1) but it's still around.
Created attachment 113722 [details] [review]
J-M B's hack, re-diffed for 3.19 final (by him)
Created attachment 113723 [details] [review]
J-M B's hack, further re-diffed for 3.20 current (by me) (doesn't appear to work)
(In reply to Adam Williamson from comment #20)
> J-M B's hack, re-diffed for 3.19 final (by him)
This one does work for me, thanks.
Latest patch sets for trying out. Both panel enable and back-light enable/disable is supported. Last patch(9) is more of quick test. Once I fix that properly, back-light brightness control should also work -
Thanks Shobhit! Should CONFIG_PWM_CRC depend on CONFIG_PWM ? I don't think it does at present; I seemed to manage to get a build to run with CONFIG_PWM_CRC=y but CONFIG_PWM unset, and it failed because pwm_config_alternate() was not defined (because its definition in pwm.h is inside a #if IS_ENABLED(CONFIG_PWM) block).
Yes CONFIG_PWM_CRC depends on CONFIG_PWM and additionally on CONFIG_INTEL_SOC_PMIC. CONFIG_PWM_CRC is in drivers/pwm/Kconfig which needs CONFIG_PWM enabled first to be able to select CONFIG_PWM_CRC.
(In reply to Shobhit from comment #23)
> Latest patch sets for trying out. Both panel enable and back-light
> enable/disable is supported. Last patch(9) is more of quick test. Once I fix
> that properly, back-light brightness control should also work -
Shobhit, I tried to apply your patch against 4.0 kernel. Unfortunately, I get
[ 3.205922] crystal_cove_pwm: probe of crystal_cove_pwm failed with error -22
on Acer Aspire Switch 10 with Intel Atom (BayTrail CR?) Z3735F. J-M B's hack never worked for me no matter what kernel version. Is there any debug info I can provide?
Not sure if it is BYT-CR. But if it is then LPSS PWM is used on those. I think the LPSS PWM driver is already upstreamed(drivers/pwm/pwm-lpss.c. We would have to use that PWM chip in this case instead of PMIC based one.
if(dev_priv->vbt.dsi.config->pwm_blc == 1)
then it is confirmed to be using LPSS instead of PMIC
(In reply to Shobhit from comment #27)
> Not sure if it is BYT-CR. But if it is then LPSS PWM is used on those. I
> think the LPSS PWM driver is already upstreamed(drivers/pwm/pwm-lpss.c. We
> would have to use that PWM chip in this case instead of PMIC based one.
> if(dev_priv->vbt.dsi.config->pwm_blc == 1)
> then it is confirmed to be using LPSS instead of PMIC
Yes, it's using LPSS then.
Created attachment 115306 [details]
Kernel config fie tested on AsusT100
Latest series -
I have also attached my config file for testing. Using this AsusT100 works fine using the /sys/class/backlight/intel_backlight interface
(In reply to Shobhit from comment #29)
> Created attachment 115306 [details]
> Kernel config fie tested on AsusT100
> Latest series -
> I have also attached my config file for testing. Using this AsusT100 works
> fine using the /sys/class/backlight/intel_backlight interface
Hello. I have a Asus T100TAF wich has Z3735F.
Tried the previously mentioned patch but brightness control is not working.
Furter examination got me to the realisation that it needs LPSS instead of PMIC for the backlight brightness control
Is there a patch aviabile for the Z3735F?
(In reply to Luka Karinja from comment #30)
> (In reply to Shobhit from comment #29)
> > Created attachment 115306 [details]
> > Kernel config fie tested on AsusT100
> > Latest series -
> > http://lists.freedesktop.org/archives/intel-gfx/2015-April/065313.html
> > I have also attached my config file for testing. Using this AsusT100 works
> > fine using the /sys/class/backlight/intel_backlight interface
> Hello. I have a Asus T100TAF wich has Z3735F.
> Tried the previously mentioned patch but brightness control is not working.
> Furter examination got me to the realisation that it needs LPSS instead of
> PMIC for the backlight brightness control
> Is there a patch aviabile for the Z3735F?
Not yet. I need to first get the PMIC based patches merged and that's when I am coming back for this LPSS. This *is* in my bin list.
@Shobhit: I've tested your patches on Dell Venue 8 Pro but it is not working. Log shows:
[ 4.369291] [drm:intel_dsi_init [i915]] *ERROR* Failed to own gpio for panel control
[ 4.369889] [drm:pwm_setup_backlight [i915]] *ERROR* Failed to own the pwm chip
This is caused by the soc pmic driver loaded *after* i915, and that's why the pwm list is empty when requesting it. I've added it directly to the kernel and tested i915 as build-in and module, but it doesn't change the effect. Do you have a helping hand?
Confirming I see the same messages with my latest Fedlet kernel build (to which I applied the latest available patch series from the mailing list).
This will happen because this device as I remember from previous discussions uses LPSS for backlight and not PMIC and the PMIC device also might be different and the ACPI enumeration for the same might not be available. Hence it is likely that the we will not get the gpio and pwn chips. Today the PWM supported is only for CRC PMIC which is not there on this platform. There needs to be some work done to use LPSS based PWM. That driver is already there in the kernel. Plumbing in i915 PWM support is pending.
At least my variant of the DV8P uses the PMIC for backlight control. After modifying the driver to always call setup_backlight it allows me to disable the screen. Your logic withinn PMIC module also matches my first patch (added as first message in this bug issue).
(In reply to Jan-Michael Brummer from comment #35)
> At least my variant of the DV8P uses the PMIC for backlight control. After
> modifying the driver to always call setup_backlight it allows me to disable
> the screen. Your logic withinn PMIC module also matches my first patch
> (added as first message in this bug issue).
Ok, I got confused in the previous replies. two devices are being mixed in this bug discussion. Can you try with Linux-next where all the patches are merged and one of the config file already attached as reference. I have tested with this config file and it works.
I've tried linux-next with your latest config file (+ y to emmc) but it produces the same error. PWMs are started after i915. That's understandable as there is no real dependency within i915 to the pwm crc module.
This bug is about PMIC backlight control. The code to support this, both on BYT and CHV, has been upstream for a while now, so I'm closing the bug. Thanks for the report.
There still remains the issue with probe ordering: if i915 gets probed before all the related pmic/i2c drivers, pwm_get() fails as described in comment #32. If you want to file a new bug for that, I don't mind.
As a workaround, these config options worked for me on Surface 3 PMIC backlight:
Thanks, Jani. I haven't had time / motivation to play with my fedlet for a while unfortunately.
(In reply to Jani Nikula from comment #38)
> There still remains the issue with probe ordering: if i915 gets probed
> before all the related pmic/i2c drivers, pwm_get() fails as described in
> comment #32. If you want to file a new bug for that, I don't mind.
That has been filed as bug 96571.