Den 26.12.2017 07.39, skrev Meghana Madhyastha:
On Thu, Dec 21, 2017 at 11:52:43AM +0100, Noralf Trønnes wrote:
Den 11.12.2017 18.56, skrev Noralf Trønnes:
Den 11.12.2017 18.45, skrev Noralf Trønnes:
Den 11.12.2017 15.58, skrev Meghana Madhyastha:
On Mon, Dec 11, 2017 at 03:12:06PM +0100, Noralf Trønnes wrote:
Den 11.12.2017 14.17, skrev Meghana Madhyastha:
On Sat, Dec 09, 2017 at 03:09:28PM +0100, Noralf Trønnes wrote:
Den 21.10.2017 13.55, skrev Meghana Madhyastha:
Changes in v14:
- s/backlight_get/of_find_backlight/ in patch 2/3
- Change commit message in patch 3/3 from requiring to acquiring
Meghana Madhyastha (3):
drm/tinydrm: Move helper functions from tinydrm-helpers to
backlight.h
drm/tinydrm: Move tinydrm_of_find_backlight to backlight.c
drm/tinydrm: Add devres versions of of_find_backlight
I tried the patchset and this is what I got:
[ 8.057792] Unable to handle kernel paging request at virtual
address
fffffe6b
<snip>
[ 9.144181] ---[ end trace 149c05934b6a6dcc ]---
Is the reason possibly because we have omitted error checking on the
return value of backlight_enable function ?
tinydrm_enable_backlight and
tinydrm_disable_baclight do this.
Eg.
ret = backlight_update_status(backlight);
if (ret)
DRM_ERROR("Failed to enable backlight %d\n", ret);
I'm not sure, just asking whether this could be a possible reason
for the above trace.
The crash happens during probe.
I guess you'll figure this out when you get some hw to test on.
I have set up the raspberry pi and have built and boot into the custom
kernel
but I am waiting for the panel to arrive. Meanwhile, any thoughts on
error message ? Sorry for the trivial question, but I did not quite
understand the crash message and how to replicate it.
of_find_backlight() can return an error pointer (-EPROBE_DEFER):
diff --git a/drivers/video/backlight/backlight.c
b/drivers/video/backlight/backlight.c
index 4bb7bf3ee443..57370c5d51f0 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -635,8 +635,8 @@ struct backlight_device
*devm_of_find_backlight(struct device *dev)
int ret;
bd = of_find_backlight(dev);
- if (!bd)
- return NULL;
+ if (IS_ERR_OR_NULL(bd))
+ return bd;
ret = devm_add_action(dev, devm_backlight_put, bd);
if (ret) {
That solved the crash, but the backlight didn't turn on.
I had to do this as well:
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 5c441d4c049c..6f9925f10a7c 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -139,6 +139,8 @@ static inline int backlight_enable(struct
backlight_device *bd)
if (!bd)
return 0;
+ if (!bd->props.brightness)
+ bd->props.brightness = bd->props.max_brightness;
No, this is wrong, it should happen on probe, not every time it's enabled.
So maybe it should be done in of_find_backlight() or something.
I see that I'm currently doing it in tinydrm_of_find_backlight().
I'm not happy with this brightness hack of mine really.
Since I last looked at this I see that pwm_bl has gained the ability to
start in a power off state (pwm_backlight_initial_power_state()).
However the gpio_backlight driver doesn't do this. gpio_backlight has a
'default-on' property, but the problem is that it sets brightness, not
power state. So the absense of the property sets brightness to zero,
which makes the driver turn off backlight on probe. This seems to be
fine, but then systemd-backlight comes along and restores brightness
to 1, since that's what it was on shutdown. This turns on backlight and
now I have a glaring white uninitialized panel waiting for the display
driver to set it up.
This patch would solve my problem:
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index e470da95d806..54bb722e1ea3 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -142,7 +142,8 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
return PTR_ERR(bl);
}
- bl->props.brightness = gbl->def_value;
+ bl->props.brightness = 1;
+ bl->props.state = gbl->def_value ? 0 : BL_CORE_FBBLANK;
backlight_update_status(bl);
platform_set_drvdata(pdev, bl);
I have a few questions here.
So if I understood the problem correctly, you do not want the backlight
brightness set to 1 by systemd-backlight because it sets the backlight
on before the display driver is set up. My question is, how does pwm_bl
avoid this ? Even if it starts off in a power off state, won't the same
thing happen i.e won't systemd set the brightness and subsequently turn
on the backlight ?
Look at the update_status implementation in both drivers and you'll see
that if blanking is active, they use a value of zero for brightness.
pwm_bl can set the blanking/power state during probe and still have a
default brightness which will be used when leaving the power off state.
gpio_backlight can only set the default brightness level during probe, not
the power state.
It is confusing that the backlight subsystem has so many ways to control
power state (props power/fb_blank/state).
Also, I didn't understand your patch. What does gbl->def_value specify?
bl->props.state represents the power state right ? Is your patch setting
it to 0 prevent systemd-backlight from kicking in because we are dealing
with the power property and not the brightness property ?
In the current implementation it specifies brightness. I would like this to
determine power state instead.
systemd sets brightness only, it doesn't affect the power state. So if
the state is off, changing the brightness doesn't affect the current output.
I was trying to understand some of the functions in gpio_backlight.c and
was confused about one more thing. What is the difference between
gpio_backlight_probe and gpio_backlight_probe_dt ?
There are often two ways of configuring a device in Linux, from Device Tree
or platform data (legacy). This driver supports both. If the
platform_device is created from Device Tree, of_node is set and that's
where the driver looks for info.
I am also a little confused about how gpio backlight and pwm backlight
are separate/different (because both would require the gpio pins).
Is there any documentation here which I could refer to?
Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
Documentation/devicetree/bindings/leds/backlight/gpio-backlight.txt
Noralf.
Thanks and regards,
Meghana
The problem is that this will most likely break 2 in-kernel users of
gpio-backlight which doesn't set the 'default-on' property:
arch/arm/boot/dts/omap4-var-dvk-om44.dts
arm/boot/dts/imx27-eukrea-mbimxsd27-baseboard.dts
AFAICT they rely on systemd-backlight to turn on backlight by setting
brightness to 1.
So maybe my hack is _the_ soulution after all, but I'm no expert on
the backlight subsystem and it's corner cases.
Noralf.
bd->props.power = FB_BLANK_UNBLANK;
bd->props.state &= ~BL_CORE_FBBLANK;
This is my backlight node[1]:
backlight: backlight {
compatible = "gpio-backlight";
gpios = <&gpio 18 0>; // GPIO_ACTIVE_HIGH
};
Not certain that this is the "correct" solution, but I can't use the
gpio-backlight property default-on, because that would turn on backlight
before the pipeline is enabled.
You can dry-run the driver without a panel connected, it can work
without being able to read from the controller.
Noralf.
[1]
https://github.com/notro/tinydrm/blob/master/rpi-overlays/rpi-display-overlay.dts
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
