| Message ID | 1482790896-29202-1-git-send-email-l.majewski@majess.pl |
|---|---|
| State | Deferred |
| Headers | show |
Dear All, A gentle ping for this patch :-) We had a discussion about it in a following thread: http://patchwork.ozlabs.org/patch/689786/ According to comments, I've prepared v2 of this commit and also resend it recently: http://patchwork.ozlabs.org/patch/708844/ Any new comments? Thanks in advance, Łukasz Majewski > The commit a55944ca82d2 > ("backlight: update bd state & fb_blank properties when necessary") > has posed some extra restrictions on blanking and unblanking frame > buffer device. > > Unfortunately, pwm_bl driver's probe did not initialize members of > struct backlight_device necessary for further blank/unblank operation. > > This code in case of initial unblank of backlight device (default > behaviour) sets use_count to 1 and marks this particular backlight > device as used by all available fb devices (since it is not known > during probe how much and which fb devices will be assigned). > > Without this code, the backlight works properly until one tries to > blank it manually from sysfs with "echo 1 > > /sys/class/graphics/fb0/blank". Since fb_bl_on[0] and use_count > > were both set to 0, the logic at > fb_notifier_callback (@backlight.c) thought that we didn't turn on > (unblanked) the backlight device and refuses to disable (blank) it. > As a result we see garbage from fb displayed. > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> > --- > The patch has been tested on i.MX6q with vanilla 4.9 kernel. > It applies on 4.10-rc1. > --- > drivers/video/backlight/pwm_bl.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/video/backlight/pwm_bl.c > b/drivers/video/backlight/pwm_bl.c index 1261400..6859ba0 100644 > --- a/drivers/video/backlight/pwm_bl.c > +++ b/drivers/video/backlight/pwm_bl.c > @@ -202,7 +202,7 @@ static int pwm_backlight_probe(struct > platform_device *pdev) struct pwm_bl_data *pb; > int initial_blank = FB_BLANK_UNBLANK; > struct pwm_args pargs; > - int ret; > + int ret, i; > > if (!data) { > ret = pwm_backlight_parse_dt(&pdev->dev, &defdata); > @@ -348,6 +348,14 @@ static int pwm_backlight_probe(struct > platform_device *pdev) > bl->props.brightness = data->dft_brightness; > bl->props.power = initial_blank; > + > + if (initial_blank == FB_BLANK_UNBLANK) { > + for (i = 0; i < FB_MAX; i++) > + bl->fb_bl_on[i] = true; > + > + bl->use_count = 1; > + } > + > backlight_update_status(bl); > > platform_set_drvdata(pdev, bl);
Thierry, Jingoo, Please respond to Lukasz. On Mon, 26 Dec 2016, Lukasz Majewski wrote: > The commit a55944ca82d2 > ("backlight: update bd state & fb_blank properties when necessary") > has posed some extra restrictions on blanking and unblanking frame buffer > device. > > Unfortunately, pwm_bl driver's probe did not initialize members of > struct backlight_device necessary for further blank/unblank operation. > > This code in case of initial unblank of backlight device (default > behaviour) sets use_count to 1 and marks this particular backlight device > as used by all available fb devices (since it is not known during probe > how much and which fb devices will be assigned). > > Without this code, the backlight works properly until one tries to blank it > manually from sysfs with "echo 1 > /sys/class/graphics/fb0/blank". > Since fb_bl_on[0] and use_count were both set to 0, the logic at > fb_notifier_callback (@backlight.c) thought that we didn't turn on > (unblanked) the backlight device and refuses to disable (blank) it. > As a result we see garbage from fb displayed. > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> > --- > The patch has been tested on i.MX6q with vanilla 4.9 kernel. > It applies on 4.10-rc1. > --- > drivers/video/backlight/pwm_bl.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c > index 1261400..6859ba0 100644 > --- a/drivers/video/backlight/pwm_bl.c > +++ b/drivers/video/backlight/pwm_bl.c > @@ -202,7 +202,7 @@ static int pwm_backlight_probe(struct platform_device *pdev) > struct pwm_bl_data *pb; > int initial_blank = FB_BLANK_UNBLANK; > struct pwm_args pargs; > - int ret; > + int ret, i; > > if (!data) { > ret = pwm_backlight_parse_dt(&pdev->dev, &defdata); > @@ -348,6 +348,14 @@ static int pwm_backlight_probe(struct platform_device *pdev) > > bl->props.brightness = data->dft_brightness; > bl->props.power = initial_blank; > + > + if (initial_blank == FB_BLANK_UNBLANK) { > + for (i = 0; i < FB_MAX; i++) > + bl->fb_bl_on[i] = true; > + > + bl->use_count = 1; > + } > + > backlight_update_status(bl); > > platform_set_drvdata(pdev, bl);
Hi Lee, Thierry, Jingoo, > > Please respond to Lukasz. Gentel Ping.... Best regards, Łukasz Majewski > > On Mon, 26 Dec 2016, Lukasz Majewski wrote: > > > The commit a55944ca82d2 > > ("backlight: update bd state & fb_blank properties when necessary") > > has posed some extra restrictions on blanking and unblanking frame > > buffer device. > > > > Unfortunately, pwm_bl driver's probe did not initialize members of > > struct backlight_device necessary for further blank/unblank > > operation. > > > > This code in case of initial unblank of backlight device (default > > behaviour) sets use_count to 1 and marks this particular backlight > > device as used by all available fb devices (since it is not known > > during probe how much and which fb devices will be assigned). > > > > Without this code, the backlight works properly until one tries to > > blank it manually from sysfs with "echo 1 > > > /sys/class/graphics/fb0/blank". Since fb_bl_on[0] and use_count > > > were both set to 0, the logic at > > fb_notifier_callback (@backlight.c) thought that we didn't turn on > > (unblanked) the backlight device and refuses to disable (blank) it. > > As a result we see garbage from fb displayed. > > > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> > > --- > > The patch has been tested on i.MX6q with vanilla 4.9 kernel. > > It applies on 4.10-rc1. > > --- > > drivers/video/backlight/pwm_bl.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/video/backlight/pwm_bl.c > > b/drivers/video/backlight/pwm_bl.c index 1261400..6859ba0 100644 > > --- a/drivers/video/backlight/pwm_bl.c > > +++ b/drivers/video/backlight/pwm_bl.c > > @@ -202,7 +202,7 @@ static int pwm_backlight_probe(struct > > platform_device *pdev) struct pwm_bl_data *pb; > > int initial_blank = FB_BLANK_UNBLANK; > > struct pwm_args pargs; > > - int ret; > > + int ret, i; > > > > if (!data) { > > ret = pwm_backlight_parse_dt(&pdev->dev, &defdata); > > @@ -348,6 +348,14 @@ static int pwm_backlight_probe(struct > > platform_device *pdev) > > bl->props.brightness = data->dft_brightness; > > bl->props.power = initial_blank; > > + > > + if (initial_blank == FB_BLANK_UNBLANK) { > > + for (i = 0; i < FB_MAX; i++) > > + bl->fb_bl_on[i] = true; > > + > > + bl->use_count = 1; > > + } > > + > > backlight_update_status(bl); > > > > platform_set_drvdata(pdev, bl); >
Dear All, > Thierry, Jingoo, > > Please respond to Lukasz. Yes, your response is more than welcome....... :-) Thanks in advance, Łukasz Majewski > > On Mon, 26 Dec 2016, Lukasz Majewski wrote: > > > The commit a55944ca82d2 > > ("backlight: update bd state & fb_blank properties when necessary") > > has posed some extra restrictions on blanking and unblanking frame > > buffer device. > > > > Unfortunately, pwm_bl driver's probe did not initialize members of > > struct backlight_device necessary for further blank/unblank > > operation. > > > > This code in case of initial unblank of backlight device (default > > behaviour) sets use_count to 1 and marks this particular backlight > > device as used by all available fb devices (since it is not known > > during probe how much and which fb devices will be assigned). > > > > Without this code, the backlight works properly until one tries to > > blank it manually from sysfs with "echo 1 > > > /sys/class/graphics/fb0/blank". Since fb_bl_on[0] and use_count > > > were both set to 0, the logic at > > fb_notifier_callback (@backlight.c) thought that we didn't turn on > > (unblanked) the backlight device and refuses to disable (blank) it. > > As a result we see garbage from fb displayed. > > > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> > > --- > > The patch has been tested on i.MX6q with vanilla 4.9 kernel. > > It applies on 4.10-rc1. > > --- > > drivers/video/backlight/pwm_bl.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/video/backlight/pwm_bl.c > > b/drivers/video/backlight/pwm_bl.c index 1261400..6859ba0 100644 > > --- a/drivers/video/backlight/pwm_bl.c > > +++ b/drivers/video/backlight/pwm_bl.c > > @@ -202,7 +202,7 @@ static int pwm_backlight_probe(struct > > platform_device *pdev) struct pwm_bl_data *pb; > > int initial_blank = FB_BLANK_UNBLANK; > > struct pwm_args pargs; > > - int ret; > > + int ret, i; > > > > if (!data) { > > ret = pwm_backlight_parse_dt(&pdev->dev, &defdata); > > @@ -348,6 +348,14 @@ static int pwm_backlight_probe(struct > > platform_device *pdev) > > bl->props.brightness = data->dft_brightness; > > bl->props.power = initial_blank; > > + > > + if (initial_blank == FB_BLANK_UNBLANK) { > > + for (i = 0; i < FB_MAX; i++) > > + bl->fb_bl_on[i] = true; > > + > > + bl->use_count = 1; > > + } > > + > > backlight_update_status(bl); > > > > platform_set_drvdata(pdev, bl); > Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear All, > Dear All, > > > Thierry, Jingoo, > > > > Please respond to Lukasz. > > Yes, your response is more than welcome....... :-) More, more than welcome. Ping for this patch. I do understand that kernel developers are busy people, but this patch is not reviewed since Nov last year :-) > > Thanks in advance, > > Łukasz Majewski > > > > > On Mon, 26 Dec 2016, Lukasz Majewski wrote: > > > > > The commit a55944ca82d2 > > > ("backlight: update bd state & fb_blank properties when > > > necessary") has posed some extra restrictions on blanking and > > > unblanking frame buffer device. > > > > > > Unfortunately, pwm_bl driver's probe did not initialize members of > > > struct backlight_device necessary for further blank/unblank > > > operation. > > > > > > This code in case of initial unblank of backlight device (default > > > behaviour) sets use_count to 1 and marks this particular backlight > > > device as used by all available fb devices (since it is not known > > > during probe how much and which fb devices will be assigned). > > > > > > Without this code, the backlight works properly until one tries to > > > blank it manually from sysfs with "echo 1 > > > > /sys/class/graphics/fb0/blank". Since fb_bl_on[0] and use_count > > > > were both set to 0, the logic at > > > fb_notifier_callback (@backlight.c) thought that we didn't turn on > > > (unblanked) the backlight device and refuses to disable (blank) > > > it. As a result we see garbage from fb displayed. > > > > > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> > > > --- > > > The patch has been tested on i.MX6q with vanilla 4.9 kernel. > > > It applies on 4.10-rc1. > > > --- > > > drivers/video/backlight/pwm_bl.c | 10 +++++++++- > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/video/backlight/pwm_bl.c > > > b/drivers/video/backlight/pwm_bl.c index 1261400..6859ba0 100644 > > > --- a/drivers/video/backlight/pwm_bl.c > > > +++ b/drivers/video/backlight/pwm_bl.c > > > @@ -202,7 +202,7 @@ static int pwm_backlight_probe(struct > > > platform_device *pdev) struct pwm_bl_data *pb; > > > int initial_blank = FB_BLANK_UNBLANK; > > > struct pwm_args pargs; > > > - int ret; > > > + int ret, i; > > > > > > if (!data) { > > > ret = pwm_backlight_parse_dt(&pdev->dev, > > > &defdata); @@ -348,6 +348,14 @@ static int > > > pwm_backlight_probe(struct platform_device *pdev) > > > bl->props.brightness = data->dft_brightness; > > > bl->props.power = initial_blank; > > > + > > > + if (initial_blank == FB_BLANK_UNBLANK) { > > > + for (i = 0; i < FB_MAX; i++) > > > + bl->fb_bl_on[i] = true; > > > + > > > + bl->use_count = 1; > > > + } > > > + > > > backlight_update_status(bl); > > > > > > platform_set_drvdata(pdev, bl); > > > > Best regards, > > Lukasz Majewski > > -- > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 26, 2016 at 11:21:36PM +0100, Lukasz Majewski wrote: > The commit a55944ca82d2 > ("backlight: update bd state & fb_blank properties when necessary") > has posed some extra restrictions on blanking and unblanking frame buffer > device. > > Unfortunately, pwm_bl driver's probe did not initialize members of > struct backlight_device necessary for further blank/unblank operation. > > This code in case of initial unblank of backlight device (default > behaviour) sets use_count to 1 and marks this particular backlight device > as used by all available fb devices (since it is not known during probe > how much and which fb devices will be assigned). > > Without this code, the backlight works properly until one tries to blank it > manually from sysfs with "echo 1 > /sys/class/graphics/fb0/blank". > Since fb_bl_on[0] and use_count were both set to 0, the logic at > fb_notifier_callback (@backlight.c) thought that we didn't turn on > (unblanked) the backlight device and refuses to disable (blank) it. > As a result we see garbage from fb displayed. > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> > --- > The patch has been tested on i.MX6q with vanilla 4.9 kernel. > It applies on 4.10-rc1. > --- > drivers/video/backlight/pwm_bl.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) It's difficult to wrap one's head around the above-mentioned commit a55944ca82d2 ("backlight: update bd state & fb_blank properties when necessary"). The solution borders on crazy. That said, this patch builds on top of crazy, and given that it looks okay to me. Acked-by: Thierry Reding <thierry.reding@gmail.com> > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c > index 1261400..6859ba0 100644 > --- a/drivers/video/backlight/pwm_bl.c > +++ b/drivers/video/backlight/pwm_bl.c > @@ -202,7 +202,7 @@ static int pwm_backlight_probe(struct platform_device *pdev) > struct pwm_bl_data *pb; > int initial_blank = FB_BLANK_UNBLANK; > struct pwm_args pargs; > - int ret; > + int ret, i; > > if (!data) { > ret = pwm_backlight_parse_dt(&pdev->dev, &defdata); > @@ -348,6 +348,14 @@ static int pwm_backlight_probe(struct platform_device *pdev) > > bl->props.brightness = data->dft_brightness; > bl->props.power = initial_blank; > + > + if (initial_blank == FB_BLANK_UNBLANK) { > + for (i = 0; i < FB_MAX; i++) > + bl->fb_bl_on[i] = true; > + > + bl->use_count = 1; > + } > + > backlight_update_status(bl); > > platform_set_drvdata(pdev, bl); > -- > 2.1.4 >
On Mon, 26 Dec 2016, Lukasz Majewski wrote: > The commit a55944ca82d2 > ("backlight: update bd state & fb_blank properties when necessary") > has posed some extra restrictions on blanking and unblanking frame buffer > device. > > Unfortunately, pwm_bl driver's probe did not initialize members of > struct backlight_device necessary for further blank/unblank operation. > > This code in case of initial unblank of backlight device (default > behaviour) sets use_count to 1 and marks this particular backlight device > as used by all available fb devices (since it is not known during probe > how much and which fb devices will be assigned). > > Without this code, the backlight works properly until one tries to blank it > manually from sysfs with "echo 1 > /sys/class/graphics/fb0/blank". > Since fb_bl_on[0] and use_count were both set to 0, the logic at > fb_notifier_callback (@backlight.c) thought that we didn't turn on > (unblanked) the backlight device and refuses to disable (blank) it. > As a result we see garbage from fb displayed. > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> > --- > The patch has been tested on i.MX6q with vanilla 4.9 kernel. > It applies on 4.10-rc1. > --- > drivers/video/backlight/pwm_bl.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) Applied with Thierry's crazy Ack. > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c > index 1261400..6859ba0 100644 > --- a/drivers/video/backlight/pwm_bl.c > +++ b/drivers/video/backlight/pwm_bl.c > @@ -202,7 +202,7 @@ static int pwm_backlight_probe(struct platform_device *pdev) > struct pwm_bl_data *pb; > int initial_blank = FB_BLANK_UNBLANK; > struct pwm_args pargs; > - int ret; > + int ret, i; > > if (!data) { > ret = pwm_backlight_parse_dt(&pdev->dev, &defdata); > @@ -348,6 +348,14 @@ static int pwm_backlight_probe(struct platform_device *pdev) > > bl->props.brightness = data->dft_brightness; > bl->props.power = initial_blank; > + > + if (initial_blank == FB_BLANK_UNBLANK) { > + for (i = 0; i < FB_MAX; i++) > + bl->fb_bl_on[i] = true; > + > + bl->use_count = 1; > + } > + > backlight_update_status(bl); > > platform_set_drvdata(pdev, bl);
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index 1261400..6859ba0 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -202,7 +202,7 @@ static int pwm_backlight_probe(struct platform_device *pdev) struct pwm_bl_data *pb; int initial_blank = FB_BLANK_UNBLANK; struct pwm_args pargs; - int ret; + int ret, i; if (!data) { ret = pwm_backlight_parse_dt(&pdev->dev, &defdata); @@ -348,6 +348,14 @@ static int pwm_backlight_probe(struct platform_device *pdev) bl->props.brightness = data->dft_brightness; bl->props.power = initial_blank; + + if (initial_blank == FB_BLANK_UNBLANK) { + for (i = 0; i < FB_MAX; i++) + bl->fb_bl_on[i] = true; + + bl->use_count = 1; + } + backlight_update_status(bl); platform_set_drvdata(pdev, bl);
The commit a55944ca82d2 ("backlight: update bd state & fb_blank properties when necessary") has posed some extra restrictions on blanking and unblanking frame buffer device. Unfortunately, pwm_bl driver's probe did not initialize members of struct backlight_device necessary for further blank/unblank operation. This code in case of initial unblank of backlight device (default behaviour) sets use_count to 1 and marks this particular backlight device as used by all available fb devices (since it is not known during probe how much and which fb devices will be assigned). Without this code, the backlight works properly until one tries to blank it manually from sysfs with "echo 1 > /sys/class/graphics/fb0/blank". Since fb_bl_on[0] and use_count were both set to 0, the logic at fb_notifier_callback (@backlight.c) thought that we didn't turn on (unblanked) the backlight device and refuses to disable (blank) it. As a result we see garbage from fb displayed. Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> --- The patch has been tested on i.MX6q with vanilla 4.9 kernel. It applies on 4.10-rc1. --- drivers/video/backlight/pwm_bl.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)