[CHV] Backlight init fails on Surface 3 if module load order is wrong

added Community feature: display/backlight platform: BSW/CHT platform: BYT priority::low severity::minor + 1 deleted label

Jani Nikula @jani said:

Please don't hold your breath while waiting for this. :(

The "straightforward" solution is to have i915 support probe defer until the required stuff becomes available. However we're pretty deep in the probe before we realize, whoops, we need something else. It's a complex driver with complex init, and the backoff gets tricky. If it were easy, it would have been done already.

Dunno, maybe it would be easier to just make the PWM backlight independent of i915. It's just that it's not without issues either.

Stephen Just said:

Understood.

Is there possibly some mechanism to force a deferred probe via a kernel parameter, which would achieve the same thing in most cases? I haven't seen any generic way to do this, though it sounds like much less work to implement.

Jani Nikula @jani said:

(In reply to Stephen Just from comment 2)

Is there possibly some mechanism to force a deferred probe via a kernel
parameter, which would achieve the same thing in most cases? I haven't seen
any generic way to do this, though it sounds like much less work to
implement.

I'm afraid I don't have any good ideas. Perhaps you could tweak the init system to not probe i915 until later. *shrug*.

Shobhit submitted a patch:

I did attempt implementing deferred probe, but really couldn't test without a device. Attaching the patch. Am sure it will not be as simple as the patch makes it look. So not much hope but you can give it a try and we'll know what are the problems you face with this.

Patch 124781, "-EPROBE_DEFER attemp for i915":
0001-drm-i915-Return-EPROBE_DEFER-if-we-cannot-get-GPIO-a.patch

viric@viric.name said:

I think that regarding pwm backlight on x86_64 and these patches from Shobit, it still applies what I wrote months ago in the mailing list (text copied below). There is no code at all to use an lpss pwm as backlight in a normal ACPI x86 kernel. So I don't see how it is a problem of ordering.

http://www.spinics.net/lists/intel-gfx/msg92148.html
-----------------
I think there is a lot to be done first, to be able to test this.

If I understand correctly, despite the lpss pwm may be available, it will
not be used as pwm-backlight (video/backlight/pwm_bl.c) because it depends
on "CONFIG_OF" and it is ready for devicetree only.

The only x86 that enable OF are X86_INTEL_CE and OLPC, and pwm_bl is
completely disabled without OF. I don't have any INTEL_CE or OLPC, this
LPSS of the Atom SoC is a different thing.

So, someone has to implement pwm_bl on my x86 first, for any of the
patches wrt i915 to be used at all. Should X86_INTEL_LPSS enable OF?
Should there be a devicetree definition for it first? Does someone have to
allow pwm_bl to work with ACPI? Is all that ready in some branch of some
other linux team?
--------------------

By the way, I think that bug 90075 is very well related to this (I also wrote this text therE). Isn't it?

Stephen Just said:

I think there may be some confusion: the Surface 3 seems to use pwm-crystalcove, not lpss. As far as I can tell, these are different implementations although I could be wrong.

I have not seen any dmesg logs about gpio devices as are addressed in Shobhit's attached patch.

viric@viric.name said:

Isn't there the very same problem? pwm_bl.c (PWM based backlight driver), the piece required by i915, will not do anything at all unless there is CONFIG_OF and a pwm in devicetree.

viric@viric.name said:

As far as I understand, i915 wants a "backlight control". That control is provided, for example, by pwm_bl (pwm-based backlight control), that will use any of the pwm available in the system (lpss, crystalcove, pmic, whatever). 3 pieces that have to work fine. I think that i915 works fine in my Intel Atom ACPI system, also works fine the pwm (lpss), but the glue (pwm-based backlight control) is just not there at all.

Shobhit said:

I could never test my patches as I didn't have any device using LPSS, but on looking at drivers/pwm/pwm-lpss-platform.c, I could see that it is registering a platform driver with ACPI ID - "80860F09". So I assumed it would be probed and loaded.

If that worked, then I hoped we should be able to use pwm-lpss in i915. But as I said, I could not test or debug anything with lack of device.

I am sure that the pwm-crystalcove should be working once module load problem is solved (Try attached patch in this bug)

Putting patch set attempting lpss related changes in i915 again in this bug for completeness.

https://patchwork.freedesktop.org/patch/70286/
https://patchwork.freedesktop.org/patch/70287/
https://patchwork.freedesktop.org/patch/70288/

viric@viric.name said:

I guess that the pwm driver patches are OK, as they were in this my LPSS:

http://www.spinics.net/lists/intel-gfx/msg92110.html

But the problem is that there is no glue between any PWM and i915 other than drivers/video/backlight/pwm_bl.c. It only adds support for some PWM devices to be used as backlight, and only in CONFIG_OF (embedded x86, with devicetree, non-ACPI).

So, PWM works, i915 works, but the pwm-backlight glue called by i915 is not present in x86 ACPI.

http://lxr.free-electrons.com/source/drivers/video/backlight/pwm_bl.c#L195

See how pwm_backlight_probe() calls just pwm_backlight_parse_dt(), that for non-CONFIG_OF just returns -ENODEV. Then, i915 never finds a backlight pwm for an ACPI (non-CONFIG_OF) system).

viric@viric.name said:

This is the i915 code that tries to find the pwm-backlight controller:

http://lxr.free-electrons.com/source/drivers/gpu/drm/i915/intel_panel.c#L1636

that controller is added in drivers/video/backlight/pwm_bl.c , but not for ACPI systems like my LPSS and, I guess, also this crystalcove. Someone should add ACPI code to pwm_bl.c to handle those ACPI PWM devices, because it only works for non-ACPI (very few x86, with devicetree, CONFIG_OF).

I have very little idea of kernel developing and I may be wrong, but that is how I understand the situation.

Shobhit said:

pwm_bl.c is not used and as you said is based on device tree. The code that you point in i915, I wrote based on ACPI match from drivers/mfd/intel_soc_pmic_core.c

The MFD driver will add this controller to pwm_table and will call the necessary probes. This works fine as I have tested the code for pwm-crystalcove. The pwm-crc is a cell device for MFD crystal cove PMIC.

In case of LPSS, my understanding is that we need to load drivers/pwm/pwm-lpss.c and drivers/pwm/pwm-lpss-platform.c which is again based on ACPI. The corresponding ID in the driver is "80860F09". So once we ensure that this is loading the glue path should be okay. I just could not test this because I don't have a device.

If you can enable -
CONFIG_PWM_LPSS=y
CONFIG_PWM_LPSS_PLATFORM=y

And use my other patches, I assumed it should work. Of course I maybe wrong, so if you have not tested this, can you test it and update me on your findings ? Basically check if in pwm-lpss-platform.c probe is being called or not.

viric@viric.name said:

I already tested that in April and reported about it. Is there anything new that I should test?

http://www.spinics.net/lists/intel-gfx/msg91730.html

The PWM devices are present and drivers loaded, as shown in:

http://www.spinics.net/lists/intel-gfx/msg92110.html

but they are not detected by i915. None of them is named "pwm-backlight", so I guess that I miss something that would map the existing PWM to the name "pwm-backligth", maybe like this line does for crystalcove:

http://lxr.free-electrons.com/source/drivers/mfd/intel_soc_pmic_core.c#L43

Is there anything that can map LPSS pwm to "pwm-backlight"? That's the piece I could not find anywhere.

Shobhit said:

Check the patches -
General re factoring in i915
https://patchwork.freedesktop.org/patch/70286/

Add pwm lookup table. Device is called - "pwm-lpss" Added in probe of pwm-lpss-platform.c
https://patchwork.freedesktop.org/patch/70287/

Uses the same in i915 -
https://patchwork.freedesktop.org/patch/70288/

Hope you pulled all the above patches ?

Shobhit said:

module load order is another problem because of which even with these patches we might not be able to find pwm. Try "-EPROBE_DEFER attemp for i915" in the attachment, or perhaps make pwm related stuff as in built with kernel and i915 as kernel module to ensure it loads later by udev.

viric@viric.name said:

Ah! Clear. Maybe I was looking at a tree without the patches and got confused. The patches clearly introduce the "pwm-backlight" for lpss.

Did you see that my kernel log of April shows:

[ 2.658013] pwm-byt-plat 80860F09:00: PWM device probed: pwm_num=0, mmio_base=ffffc90001a46000 clk_khz=25000
[ 2.658223] pwm-byt-plat 80860F09:01: PWM device probed: pwm_num=1, mmio_base=ffffc90001a48000 clk_khz=25000

and only at 15s i915 reports failure to own?

[ 15.203281] [drm:pwm_setup_backlight [i915]] ERROR Failed to own the pwm chip: pwm_lpss

Doesn't this mean that the order is already correct?

(I will try the patches here provided though)

Shobhit said:

Sorry, I think I missed them. BTW I found two logs in the mail - panic-mtd.txt and lpss-pwm-patches-dmesg.txt

The first set of log you gave -
[ 2.658013] pwm-byt-plat 80860F09:00: PWM device probed: pwm_num=0, mmio_base=ffffc90001a46000 clk_khz=25000
[ 2.658223] pwm-byt-plat 80860F09:01: PWM device probed: pwm_num=1, mmio_base=ffffc90001a48000 clk_khz=25000

are seen in panic-mtd.txt and which is based on 3.10 kernel version as per the logs.

Following one is seen in the other file which shows version as 4.5

[ 15.203281] [drm:pwm_setup_backlight [i915]] ERROR Failed to own the pwm chip: pwm_lpss

So I am totally confused in the relationship of the logs. Look like they are from two different kernel boot instance of two different kernel version.

Please provide me logs with latest run that you are planning to test now. put some prints in platform driver probe for LPSS pwm so that we can confirm that it found ACPI ID match or not. Also provide acpi table dump using acpidump.

viric@viric.name said:

Ah it's me that confused both logs today; sorry.

I will prepare what you request. Thank you!

viric@viric.name said:

The ACPI match worked, because definitely I have pwm in /sys:
http://www.spinics.net/lists/intel-gfx/msg92110.html

But I will add printk, otherwise we don't see the order.

[CHV] Backlight init fails on Surface 3 if module load order is wrong

Submitted by Stephen Just

Description

Blocking

Child items ...

Activity

Admin message

Admin message

[CHV] Backlight init fails on Surface 3 if module load order is wrong

Submitted by Stephen Just

Description

Blocking

Activity