Skip to content

Commit 2d7b4cd

Browse files
torvaldstorvalds
committed
Merge tag 'backlight-next-5.15' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight
Pull backlight updates from Lee Jones: "Fix-ups: - Improve bootloader/kernel device handover Bug Fixes: - Stabilise backlight in ktd253 driver" * tag 'backlight-next-5.15' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight: backlight: pwm_bl: Improve bootloader/kernel device handover backlight: ktd253: Stabilize backlight
2 parents 86406a9 + 79fad92 commit 2d7b4cd

2 files changed

Lines changed: 83 additions & 46 deletions

File tree

‎drivers/video/backlight/ktd253-backlight.c‎

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
#define KTD253_T_LOW_NS (200 + 10) /* Additional 10ns as safety factor */
2727
#define KTD253_T_HIGH_NS (200 + 10) /* Additional 10ns as safety factor */
28+
#define KTD253_T_OFF_CRIT_NS 100000 /* 100 us, now it doesn't look good */
2829
#define KTD253_T_OFF_MS 3
2930

3031
struct ktd253_backlight {
@@ -34,13 +35,50 @@ struct ktd253_backlight {
3435
u16 ratio;
3536
};
3637

38+
static void ktd253_backlight_set_max_ratio(struct ktd253_backlight *ktd253)
39+
{
40+
gpiod_set_value_cansleep(ktd253->gpiod, 1);
41+
ndelay(KTD253_T_HIGH_NS);
42+
/* We always fall back to this when we power on */
43+
}
44+
45+
static int ktd253_backlight_stepdown(struct ktd253_backlight *ktd253)
46+
{
47+
/*
48+
* These GPIO operations absolutely can NOT sleep so no _cansleep
49+
* suffixes, and no using GPIO expanders on slow buses for this!
50+
*
51+
* The maximum number of cycles of the loop is 32 so the time taken
52+
* should nominally be:
53+
* (T_LOW_NS + T_HIGH_NS + loop_time) * 32
54+
*
55+
* Architectures do not always support ndelay() and we will get a few us
56+
* instead. If we get to a critical time limit an interrupt has likely
57+
* occured in the low part of the loop and we need to restart from the
58+
* top so we have the backlight in a known state.
59+
*/
60+
u64 ns;
61+
62+
ns = ktime_get_ns();
63+
gpiod_set_value(ktd253->gpiod, 0);
64+
ndelay(KTD253_T_LOW_NS);
65+
gpiod_set_value(ktd253->gpiod, 1);
66+
ns = ktime_get_ns() - ns;
67+
if (ns >= KTD253_T_OFF_CRIT_NS) {
68+
dev_err(ktd253->dev, "PCM on backlight took too long (%llu ns)\n", ns);
69+
return -EAGAIN;
70+
}
71+
ndelay(KTD253_T_HIGH_NS);
72+
return 0;
73+
}
74+
3775
static int ktd253_backlight_update_status(struct backlight_device *bl)
3876
{
3977
struct ktd253_backlight *ktd253 = bl_get_data(bl);
4078
int brightness = backlight_get_brightness(bl);
4179
u16 target_ratio;
4280
u16 current_ratio = ktd253->ratio;
43-
unsigned long flags;
81+
int ret;
4482

4583
dev_dbg(ktd253->dev, "new brightness/ratio: %d/32\n", brightness);
4684

@@ -62,37 +100,34 @@ static int ktd253_backlight_update_status(struct backlight_device *bl)
62100
}
63101

64102
if (current_ratio == 0) {
65-
gpiod_set_value_cansleep(ktd253->gpiod, 1);
66-
ndelay(KTD253_T_HIGH_NS);
67-
/* We always fall back to this when we power on */
103+
ktd253_backlight_set_max_ratio(ktd253);
68104
current_ratio = KTD253_MAX_RATIO;
69105
}
70106

71-
/*
72-
* WARNING:
73-
* The loop to set the correct current level is performed
74-
* with interrupts disabled as it is timing critical.
75-
* The maximum number of cycles of the loop is 32
76-
* so the time taken will be (T_LOW_NS + T_HIGH_NS + loop_time) * 32,
77-
*/
78-
local_irq_save(flags);
79107
while (current_ratio != target_ratio) {
80108
/*
81109
* These GPIO operations absolutely can NOT sleep so no
82110
* _cansleep suffixes, and no using GPIO expanders on
83111
* slow buses for this!
84112
*/
85-
gpiod_set_value(ktd253->gpiod, 0);
86-
ndelay(KTD253_T_LOW_NS);
87-
gpiod_set_value(ktd253->gpiod, 1);
88-
ndelay(KTD253_T_HIGH_NS);
89-
/* After 1/32 we loop back to 32/32 */
90-
if (current_ratio == KTD253_MIN_RATIO)
113+
ret = ktd253_backlight_stepdown(ktd253);
114+
if (ret == -EAGAIN) {
115+
/*
116+
* Something disturbed the backlight setting code when
117+
* running so we need to bring the PWM back to a known
118+
* state. This shouldn't happen too much.
119+
*/
120+
gpiod_set_value_cansleep(ktd253->gpiod, 0);
121+
msleep(KTD253_T_OFF_MS);
122+
ktd253_backlight_set_max_ratio(ktd253);
123+
current_ratio = KTD253_MAX_RATIO;
124+
} else if (current_ratio == KTD253_MIN_RATIO) {
125+
/* After 1/32 we loop back to 32/32 */
91126
current_ratio = KTD253_MAX_RATIO;
92-
else
127+
} else {
93128
current_ratio--;
129+
}
94130
}
95-
local_irq_restore(flags);
96131
ktd253->ratio = current_ratio;
97132

98133
dev_dbg(ktd253->dev, "new ratio set to %d/32\n", target_ratio);

‎drivers/video/backlight/pwm_bl.c‎

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,33 @@ static bool pwm_backlight_is_linear(struct platform_pwm_backlight_data *data)
409409
static int pwm_backlight_initial_power_state(const struct pwm_bl_data *pb)
410410
{
411411
struct device_node *node = pb->dev->of_node;
412+
bool active = true;
413+
414+
/*
415+
* If the enable GPIO is present, observable (either as input
416+
* or output) and off then the backlight is not currently active.
417+
* */
418+
if (pb->enable_gpio && gpiod_get_value_cansleep(pb->enable_gpio) == 0)
419+
active = false;
420+
421+
if (!regulator_is_enabled(pb->power_supply))
422+
active = false;
423+
424+
if (!pwm_is_enabled(pb->pwm))
425+
active = false;
426+
427+
/*
428+
* Synchronize the enable_gpio with the observed state of the
429+
* hardware.
430+
*/
431+
if (pb->enable_gpio)
432+
gpiod_direction_output(pb->enable_gpio, active);
433+
434+
/*
435+
* Do not change pb->enabled here! pb->enabled essentially
436+
* tells us if we own one of the regulator's use counts and
437+
* right now we do not.
438+
*/
412439

413440
/* Not booted with device tree or no phandle link to the node */
414441
if (!node || !node->phandle)
@@ -420,20 +447,7 @@ static int pwm_backlight_initial_power_state(const struct pwm_bl_data *pb)
420447
* assume that another driver will enable the backlight at the
421448
* appropriate time. Therefore, if it is disabled, keep it so.
422449
*/
423-
424-
/* if the enable GPIO is disabled, do not enable the backlight */
425-
if (pb->enable_gpio && gpiod_get_value_cansleep(pb->enable_gpio) == 0)
426-
return FB_BLANK_POWERDOWN;
427-
428-
/* The regulator is disabled, do not enable the backlight */
429-
if (!regulator_is_enabled(pb->power_supply))
430-
return FB_BLANK_POWERDOWN;
431-
432-
/* The PWM is disabled, keep it like this */
433-
if (!pwm_is_enabled(pb->pwm))
434-
return FB_BLANK_POWERDOWN;
435-
436-
return FB_BLANK_UNBLANK;
450+
return active ? FB_BLANK_UNBLANK: FB_BLANK_POWERDOWN;
437451
}
438452

439453
static int pwm_backlight_probe(struct platform_device *pdev)
@@ -486,18 +500,6 @@ static int pwm_backlight_probe(struct platform_device *pdev)
486500
goto err_alloc;
487501
}
488502

489-
/*
490-
* If the GPIO is not known to be already configured as output, that
491-
* is, if gpiod_get_direction returns either 1 or -EINVAL, change the
492-
* direction to output and set the GPIO as active.
493-
* Do not force the GPIO to active when it was already output as it
494-
* could cause backlight flickering or we would enable the backlight too
495-
* early. Leave the decision of the initial backlight state for later.
496-
*/
497-
if (pb->enable_gpio &&
498-
gpiod_get_direction(pb->enable_gpio) != 0)
499-
gpiod_direction_output(pb->enable_gpio, 1);
500-
501503
pb->power_supply = devm_regulator_get(&pdev->dev, "power");
502504
if (IS_ERR(pb->power_supply)) {
503505
ret = PTR_ERR(pb->power_supply);

0 commit comments

Comments
 (0)