linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
@ 2016-10-22 20:58 Lukasz Majewski
  2016-10-24  2:34 ` Jingoo Han
                   ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Lukasz Majewski @ 2016-10-22 20:58 UTC (permalink / raw)
  To: Thierry Reding, Jingoo Han, Lee Jones, Jean-Christophe Plagniol-Villard
  Cc: Tomi Valkeinen, linux-pwm, linux-fbdev, linux-kernel,
	Fabio Estevam, Fabio Estevam, Liu Ying, Lukasz Majewski

The commit: a55944ca82d287ca099ca90413af857af9086773 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.7 and 4.8 kernels.
It applies on 4.9-rcX SHA1: dcd4693cf47801b7d988ea897519de90dfd25d17.

---
 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 8040fd6..def39e8 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -203,7 +203,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);
@@ -349,6 +349,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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-10-22 20:58 [PATCH] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe() Lukasz Majewski
@ 2016-10-24  2:34 ` Jingoo Han
  2016-10-24 21:00   ` Lukasz Majewski
  2016-11-01  7:25 ` [PATCH v2] " Lukasz Majewski
  2016-12-26 22:21 ` [PATCH v2 RESEND] " Lukasz Majewski
  2 siblings, 1 reply; 21+ messages in thread
From: Jingoo Han @ 2016-10-24  2:34 UTC (permalink / raw)
  To: 'Lukasz Majewski'
  Cc: 'Thierry Reding', 'Lee Jones',
	'Jean-Christophe Plagniol-Villard',
	'Tomi Valkeinen',
	linux-pwm, linux-fbdev, linux-kernel, 'Fabio Estevam',
	'Fabio Estevam', 'Liu Ying'

On Saturday, October 22, 2016, Lukasz Majewski wrote:
> 
> The commit: a55944ca82d287ca099ca90413af857af9086773 has posed some extra

Please add the subject of the patch, in order to let people know which patch
you mention exactly. Please loot at other commits that fixed bugs or
behavior
of other patches.

> 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.7 and 4.8 kernels.
> It applies on 4.9-rcX SHA1: dcd4693cf47801b7d988ea897519de90dfd25d17.
> 
> ---
>  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 8040fd6..def39e8 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -203,7 +203,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); @@ -
> 349,6 +349,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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-10-24  2:34 ` Jingoo Han
@ 2016-10-24 21:00   ` Lukasz Majewski
  2016-10-25 22:47     ` Jingoo Han
  0 siblings, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2016-10-24 21:00 UTC (permalink / raw)
  To: Jingoo Han
  Cc: 'Thierry Reding', 'Lee Jones',
	'Jean-Christophe Plagniol-Villard',
	'Tomi Valkeinen',
	linux-pwm, linux-fbdev, linux-kernel, 'Fabio Estevam',
	'Fabio Estevam', 'Liu Ying'

[-- Attachment #1: Type: text/plain, Size: 3988 bytes --]

Hi Jingoo,

> On Saturday, October 22, 2016, Lukasz Majewski wrote:
> > 
> > The commit: a55944ca82d287ca099ca90413af857af9086773 has posed some
> > extra
> 
> Please add the subject of the patch, in order to let people know
> which patch you mention exactly. Please loot at other commits that
> fixed bugs or behavior
> of other patches.

Thanks for the tip.

The mentioned patch:

commit a55944ca82d287ca099ca90413af857af9086773
Author: Liu Ying <Ying.Liu@freescale.com>
Date:   Thu Apr 3 14:48:54 2014 -0700

    backlight: update bd state & fb_blank properties when necessary
    
    We don't have to update the state and fb_blank properties of a backlight
    device every time a blanking or unblanking event comes because they may
    have already been what we want.  Another thought is that one backlight
    device may be shared by multiple framebuffers.  The backlight driver
    should take the backlight device as a resource shared by all the
    associated framebuffers.
    
    This patch adds some logic to record each framebuffer's backlight usage
    to determine the backlight device use count and whether the two
    properties should be updated or not.  To be more specific, only one
    unblank operation on a certain blanked framebuffer may increase the
    backlight device's use count by one, while one blank operation on a
    certain unblanked framebuffer may decrease the use count by one, because
    the userspace is likely to unblank an unblanked framebuffer or blank a
    blanked framebuffer.


Could you provide more feedback regarding provided fix?

Thanks in advance,

Łukasz Majewski

> 
> > 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.7 and 4.8
> > kernels. It applies on 4.9-rcX SHA1:
> > dcd4693cf47801b7d988ea897519de90dfd25d17.
> > 
> > ---
> >  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 8040fd6..def39e8 100644
> > --- a/drivers/video/backlight/pwm_bl.c
> > +++ b/drivers/video/backlight/pwm_bl.c
> > @@ -203,7 +203,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); @@ - 349,6 +349,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
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-10-24 21:00   ` Lukasz Majewski
@ 2016-10-25 22:47     ` Jingoo Han
  2016-10-27  3:46       ` Lukasz Majewski
  0 siblings, 1 reply; 21+ messages in thread
From: Jingoo Han @ 2016-10-25 22:47 UTC (permalink / raw)
  To: 'Lukasz Majewski'
  Cc: 'Thierry Reding', 'Lee Jones',
	'Jean-Christophe Plagniol-Villard',
	'Tomi Valkeinen',
	linux-pwm, linux-fbdev, linux-kernel, 'Fabio Estevam',
	'Fabio Estevam', 'Liu Ying'

On Monday, October 24, 2016, Lukasz Majewski wrote:
> 
> Hi Jingoo,
> 
> > On Saturday, October 22, 2016, Lukasz Majewski wrote:
> > >
> > > The commit: a55944ca82d287ca099ca90413af857af9086773 has posed some
> > > extra

Please add the 'subject' of patch as below.

The commit a55944ca82d2 ("backlight: update bd state & fb_blank properties when necessary ")
has posted some extra.

Best regards,
Jingoo Han


> >
> > Please add the subject of the patch, in order to let people know
> > which patch you mention exactly. Please loot at other commits that
> > fixed bugs or behavior
> > of other patches.
> 
> Thanks for the tip.
> 
> The mentioned patch:
> 
> commit a55944ca82d287ca099ca90413af857af9086773
> Author: Liu Ying <Ying.Liu@freescale.com>
> Date:   Thu Apr 3 14:48:54 2014 -0700
> 
>     backlight: update bd state & fb_blank properties when necessary
> 
>     We don't have to update the state and fb_blank properties of a
> backlight
>     device every time a blanking or unblanking event comes because they
> may
>     have already been what we want.  Another thought is that one backlight
>     device may be shared by multiple framebuffers.  The backlight driver
>     should take the backlight device as a resource shared by all the
>     associated framebuffers.
> 
>     This patch adds some logic to record each framebuffer's backlight
> usage
>     to determine the backlight device use count and whether the two
>     properties should be updated or not.  To be more specific, only one
>     unblank operation on a certain blanked framebuffer may increase the
>     backlight device's use count by one, while one blank operation on a
>     certain unblanked framebuffer may decrease the use count by one,
> because
>     the userspace is likely to unblank an unblanked framebuffer or blank a
>     blanked framebuffer.
> 
> 
> Could you provide more feedback regarding provided fix?
> 
> Thanks in advance,
> 
> Łukasz Majewski
> 
> >
> > > 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.7 and 4.8
> > > kernels. It applies on 4.9-rcX SHA1:
> > > dcd4693cf47801b7d988ea897519de90dfd25d17.
> > >
> > > ---
> > >  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 8040fd6..def39e8 100644
> > > --- a/drivers/video/backlight/pwm_bl.c
> > > +++ b/drivers/video/backlight/pwm_bl.c
> > > @@ -203,7 +203,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); @@ - 349,6 +349,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
> >
> >

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-10-25 22:47     ` Jingoo Han
@ 2016-10-27  3:46       ` Lukasz Majewski
  0 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2016-10-27  3:46 UTC (permalink / raw)
  To: Jingoo Han
  Cc: 'Thierry Reding', 'Lee Jones',
	'Jean-Christophe Plagniol-Villard',
	'Tomi Valkeinen',
	linux-pwm, linux-fbdev, linux-kernel, 'Fabio Estevam',
	'Fabio Estevam', 'Liu Ying'

[-- Attachment #1: Type: text/plain, Size: 4941 bytes --]

Hi Jingoo,

> On Monday, October 24, 2016, Lukasz Majewski wrote:
> > 
> > Hi Jingoo,
> > 
> > > On Saturday, October 22, 2016, Lukasz Majewski wrote:
> > > >
> > > > The commit: a55944ca82d287ca099ca90413af857af9086773 has posed
> > > > some extra
> 
> Please add the 'subject' of patch as below.
> 
> The commit a55944ca82d2 ("backlight: update bd state & fb_blank
> properties when necessary ") has posted some extra.

Ok, I will add such information.

However, I'm more interested in some technical comments regarding
proposed fix.

Could you provide some feedback on the technical side of the patch?

Best regards,
Łukasz Majewski

> 
> Best regards,
> Jingoo Han
> 
> 
> > >
> > > Please add the subject of the patch, in order to let people know
> > > which patch you mention exactly. Please loot at other commits that
> > > fixed bugs or behavior
> > > of other patches.
> > 
> > Thanks for the tip.
> > 
> > The mentioned patch:
> > 
> > commit a55944ca82d287ca099ca90413af857af9086773
> > Author: Liu Ying <Ying.Liu@freescale.com>
> > Date:   Thu Apr 3 14:48:54 2014 -0700
> > 
> >     backlight: update bd state & fb_blank properties when necessary
> > 
> >     We don't have to update the state and fb_blank properties of a
> > backlight
> >     device every time a blanking or unblanking event comes because
> > they may
> >     have already been what we want.  Another thought is that one
> > backlight device may be shared by multiple framebuffers.  The
> > backlight driver should take the backlight device as a resource
> > shared by all the associated framebuffers.
> > 
> >     This patch adds some logic to record each framebuffer's
> > backlight usage
> >     to determine the backlight device use count and whether the two
> >     properties should be updated or not.  To be more specific, only
> > one unblank operation on a certain blanked framebuffer may increase
> > the backlight device's use count by one, while one blank operation
> > on a certain unblanked framebuffer may decrease the use count by
> > one, because
> >     the userspace is likely to unblank an unblanked framebuffer or
> > blank a blanked framebuffer.
> > 
> > 
> > Could you provide more feedback regarding provided fix?
> > 
> > Thanks in advance,
> > 
> > Łukasz Majewski
> > 
> > >
> > > > 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.7 and 4.8
> > > > kernels. It applies on 4.9-rcX SHA1:
> > > > dcd4693cf47801b7d988ea897519de90dfd25d17.
> > > >
> > > > ---
> > > >  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 8040fd6..def39e8 100644
> > > > --- a/drivers/video/backlight/pwm_bl.c
> > > > +++ b/drivers/video/backlight/pwm_bl.c
> > > > @@ -203,7 +203,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); @@ - 349,6 +349,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
> > >
> > >
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-10-22 20:58 [PATCH] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe() Lukasz Majewski
  2016-10-24  2:34 ` Jingoo Han
@ 2016-11-01  7:25 ` Lukasz Majewski
  2016-11-08 22:25   ` Lukasz Majewski
  2016-11-10  8:16   ` Lee Jones
  2016-12-26 22:21 ` [PATCH v2 RESEND] " Lukasz Majewski
  2 siblings, 2 replies; 21+ messages in thread
From: Lukasz Majewski @ 2016-11-01  7:25 UTC (permalink / raw)
  To: Thierry Reding, Lee Jones, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen
  Cc: linux-pwm, linux-kernel, Fabio Estevam, Fabio Estevam,
	linux-fbdev, Liu Ying, Lukasz 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 4.9-rc3
SHA1: a909d3e636995ba7c349e2ca5dbb528154d4ac30
---
Changes for v2:
- Update commit message with proper other commit reference

---
 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);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-11-01  7:25 ` [PATCH v2] " Lukasz Majewski
@ 2016-11-08 22:25   ` Lukasz Majewski
  2016-11-09 14:52     ` Lee Jones
  2016-11-10  8:16     ` Lee Jones
  2016-11-10  8:16   ` Lee Jones
  1 sibling, 2 replies; 21+ messages in thread
From: Lukasz Majewski @ 2016-11-08 22:25 UTC (permalink / raw)
  To: Thierry Reding, Lee Jones, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen
  Cc: linux-pwm, linux-kernel, Fabio Estevam, Fabio Estevam,
	linux-fbdev, Liu Ying

[-- Attachment #1: Type: text/plain, Size: 2404 bytes --]

Dear All,

> 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.

COmments/acks are more than welcome :-)

Best regards,
Łukasz Majewski

> 
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> ---
> The patch has been tested on i.MX6q with 4.9-rc3
> SHA1: a909d3e636995ba7c349e2ca5dbb528154d4ac30
> ---
> Changes for v2:
> - Update commit message with proper other commit reference
> 
> ---
>  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);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-11-08 22:25   ` Lukasz Majewski
@ 2016-11-09 14:52     ` Lee Jones
  2016-11-09 21:05       ` Lukasz Majewski
  2016-11-10  8:16     ` Lee Jones
  1 sibling, 1 reply; 21+ messages in thread
From: Lee Jones @ 2016-11-09 14:52 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Thierry Reding, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-pwm, linux-kernel, Fabio Estevam, Fabio Estevam,
	linux-fbdev, Liu Ying

On Tue, 08 Nov 2016, Lukasz Majewski wrote:

> Dear All,
> 
> > 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.
> 
> COmments/acks are more than welcome :-)

I thought Jingoo already replied to you?

> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > ---
> > The patch has been tested on i.MX6q with 4.9-rc3
> > SHA1: a909d3e636995ba7c349e2ca5dbb528154d4ac30
> > ---
> > Changes for v2:
> > - Update commit message with proper other commit reference
> > 
> > ---
> >  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);
> 



-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-11-09 14:52     ` Lee Jones
@ 2016-11-09 21:05       ` Lukasz Majewski
  2016-11-10  7:47         ` Lee Jones
  0 siblings, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2016-11-09 21:05 UTC (permalink / raw)
  To: Lee Jones
  Cc: Thierry Reding, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-pwm, linux-kernel, Fabio Estevam, Fabio Estevam,
	linux-fbdev, Liu Ying

[-- Attachment #1: Type: text/plain, Size: 2902 bytes --]

Hi Lee,

> On Tue, 08 Nov 2016, Lukasz Majewski wrote:
> 
> > Dear All,
> > 
> > > 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.
> > 
> > COmments/acks are more than welcome :-)
> 
> I thought Jingoo already replied to you?

Yes, Jingoo replied to me and asked for correcting the patch
description. The corrected patch is the v2.


Best regards,
Łukasz Majewski

> 
> > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > > ---
> > > The patch has been tested on i.MX6q with 4.9-rc3
> > > SHA1: a909d3e636995ba7c349e2ca5dbb528154d4ac30
> > > ---
> > > Changes for v2:
> > > - Update commit message with proper other commit reference
> > > 
> > > ---
> > >  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);
> > 
> 
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-11-09 21:05       ` Lukasz Majewski
@ 2016-11-10  7:47         ` Lee Jones
  0 siblings, 0 replies; 21+ messages in thread
From: Lee Jones @ 2016-11-10  7:47 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Thierry Reding, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-pwm, linux-kernel, Fabio Estevam, Fabio Estevam,
	linux-fbdev, Liu Ying

On Wed, 09 Nov 2016, Lukasz Majewski wrote:

> Hi Lee,
> 
> > On Tue, 08 Nov 2016, Lukasz Majewski wrote:
> > 
> > > Dear All,
> > > 
> > > > 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.
> > > 
> > > COmments/acks are more than welcome :-)
> > 
> > I thought Jingoo already replied to you?
> 
> Yes, Jingoo replied to me and asked for correcting the patch
> description. The corrected patch is the v2.

Ah, I see.  You've attached the v4 to the original set.

Ball is in Jingoo's court.

> > > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > > > ---
> > > > The patch has been tested on i.MX6q with 4.9-rc3
> > > > SHA1: a909d3e636995ba7c349e2ca5dbb528154d4ac30
> > > > ---
> > > > Changes for v2:
> > > > - Update commit message with proper other commit reference
> > > > 
> > > > ---
> > > >  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);
> > > 
> > 
> > 
> > 
> 



-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-11-08 22:25   ` Lukasz Majewski
  2016-11-09 14:52     ` Lee Jones
@ 2016-11-10  8:16     ` Lee Jones
  2016-11-10 23:20       ` Lukasz Majewski
  1 sibling, 1 reply; 21+ messages in thread
From: Lee Jones @ 2016-11-10  8:16 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Thierry Reding, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-pwm, linux-kernel, Fabio Estevam, Fabio Estevam,
	linux-fbdev, Liu Ying

On Tue, 08 Nov 2016, Lukasz Majewski wrote:

> Dear All,
> 
> > 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.
> 
> COmments/acks are more than welcome :-)

Ah, it looks like you dropped Jingoo from the list!

I'll reply to the original mail with him in CC.

> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > ---
> > The patch has been tested on i.MX6q with 4.9-rc3
> > SHA1: a909d3e636995ba7c349e2ca5dbb528154d4ac30
> > ---
> > Changes for v2:
> > - Update commit message with proper other commit reference
> > 
> > ---
> >  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);
> 



-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-11-01  7:25 ` [PATCH v2] " Lukasz Majewski
  2016-11-08 22:25   ` Lukasz Majewski
@ 2016-11-10  8:16   ` Lee Jones
  1 sibling, 0 replies; 21+ messages in thread
From: Lee Jones @ 2016-11-10  8:16 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Thierry Reding, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-pwm, linux-kernel, Fabio Estevam, Fabio Estevam,
	linux-fbdev, Liu Ying, jingoohan1

Adding Jingoo.

On Tue, 01 Nov 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 4.9-rc3
> SHA1: a909d3e636995ba7c349e2ca5dbb528154d4ac30
> ---
> Changes for v2:
> - Update commit message with proper other commit reference
> 
> ---
>  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);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-11-10  8:16     ` Lee Jones
@ 2016-11-10 23:20       ` Lukasz Majewski
  0 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2016-11-10 23:20 UTC (permalink / raw)
  To: Lee Jones
  Cc: Thierry Reding, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-pwm, linux-kernel, Fabio Estevam, Fabio Estevam,
	linux-fbdev, Liu Ying

[-- Attachment #1: Type: text/plain, Size: 2952 bytes --]

Dear Lee,

> On Tue, 08 Nov 2016, Lukasz Majewski wrote:
> 
> > Dear All,
> > 
> > > 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.
> > 
> > COmments/acks are more than welcome :-)
> 
> Ah, it looks like you dropped Jingoo from the list!
> 

I can only apologize for such situation.... strange mistake.

> I'll reply to the original mail with him in CC.

Thank you for your help.

Best regards,
Lukasz Majewski

> 
> > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > > ---
> > > The patch has been tested on i.MX6q with 4.9-rc3
> > > SHA1: a909d3e636995ba7c349e2ca5dbb528154d4ac30
> > > ---
> > > Changes for v2:
> > > - Update commit message with proper other commit reference
> > > 
> > > ---
> > >  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);
> > 
> 
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 RESEND] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-10-22 20:58 [PATCH] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe() Lukasz Majewski
  2016-10-24  2:34 ` Jingoo Han
  2016-11-01  7:25 ` [PATCH v2] " Lukasz Majewski
@ 2016-12-26 22:21 ` Lukasz Majewski
  2017-01-02 23:54   ` Lukasz Majewski
                     ` (3 more replies)
  2 siblings, 4 replies; 21+ messages in thread
From: Lukasz Majewski @ 2016-12-26 22:21 UTC (permalink / raw)
  To: Thierry Reding, Lee Jones, linux-pwm, linux-fbdev, linux-kernel
  Cc: Liu Ying, Fabio Estevam, Fabio Estevam, Jingoo Han,
	Tomi Valkeinen, Lukasz 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);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 RESEND] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-12-26 22:21 ` [PATCH v2 RESEND] " Lukasz Majewski
@ 2017-01-02 23:54   ` Lukasz Majewski
  2017-01-03  8:30   ` Lee Jones
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-02 23:54 UTC (permalink / raw)
  To: Thierry Reding, Lee Jones, linux-pwm, linux-fbdev, linux-kernel,
	Jingoo Han
  Cc: Liu Ying, Fabio Estevam, Fabio Estevam, Tomi Valkeinen

[-- Attachment #1: Type: text/plain, Size: 2557 bytes --]

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);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 RESEND] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-12-26 22:21 ` [PATCH v2 RESEND] " Lukasz Majewski
  2017-01-02 23:54   ` Lukasz Majewski
@ 2017-01-03  8:30   ` Lee Jones
  2017-01-06 12:29     ` Lukasz Majewski
  2017-01-13  0:27     ` Lukasz Majewski
  2017-01-23 16:09   ` Thierry Reding
  2017-01-24  9:37   ` Lee Jones
  3 siblings, 2 replies; 21+ messages in thread
From: Lee Jones @ 2017-01-03  8:30 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Thierry Reding, linux-pwm, linux-fbdev, linux-kernel, Liu Ying,
	Fabio Estevam, Fabio Estevam, Jingoo Han, Tomi Valkeinen

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);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 RESEND] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2017-01-03  8:30   ` Lee Jones
@ 2017-01-06 12:29     ` Lukasz Majewski
  2017-01-13  0:27     ` Lukasz Majewski
  1 sibling, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-06 12:29 UTC (permalink / raw)
  To: Lee Jones, Thierry Reding, Jingoo Han
  Cc: linux-pwm, linux-fbdev, linux-kernel, Liu Ying, Fabio Estevam,
	Fabio Estevam, Tomi Valkeinen

[-- Attachment #1: Type: text/plain, Size: 2507 bytes --]

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);
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 RESEND] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2017-01-03  8:30   ` Lee Jones
  2017-01-06 12:29     ` Lukasz Majewski
@ 2017-01-13  0:27     ` Lukasz Majewski
  2017-01-23  0:50       ` Lukasz Majewski
  1 sibling, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-13  0:27 UTC (permalink / raw)
  To: Thierry Reding, Jingoo Han
  Cc: Lee Jones, linux-pwm, linux-fbdev, linux-kernel, Liu Ying,
	Fabio Estevam, Fabio Estevam, Tomi Valkeinen, l.majewski

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 RESEND] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2017-01-13  0:27     ` Lukasz Majewski
@ 2017-01-23  0:50       ` Lukasz Majewski
  0 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-23  0:50 UTC (permalink / raw)
  To: Thierry Reding, Jingoo Han, Lee Jones
  Cc: linux-pwm, linux-fbdev, linux-kernel, Liu Ying, Fabio Estevam,
	Fabio Estevam, Tomi Valkeinen

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 RESEND] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-12-26 22:21 ` [PATCH v2 RESEND] " Lukasz Majewski
  2017-01-02 23:54   ` Lukasz Majewski
  2017-01-03  8:30   ` Lee Jones
@ 2017-01-23 16:09   ` Thierry Reding
  2017-01-24  9:37   ` Lee Jones
  3 siblings, 0 replies; 21+ messages in thread
From: Thierry Reding @ 2017-01-23 16:09 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Lee Jones, linux-pwm, linux-fbdev, linux-kernel, Liu Ying,
	Fabio Estevam, Fabio Estevam, Jingoo Han, Tomi Valkeinen

[-- Attachment #1: Type: text/plain, Size: 2617 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 RESEND] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
  2016-12-26 22:21 ` [PATCH v2 RESEND] " Lukasz Majewski
                     ` (2 preceding siblings ...)
  2017-01-23 16:09   ` Thierry Reding
@ 2017-01-24  9:37   ` Lee Jones
  3 siblings, 0 replies; 21+ messages in thread
From: Lee Jones @ 2017-01-24  9:37 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Thierry Reding, linux-pwm, linux-fbdev, linux-kernel, Liu Ying,
	Fabio Estevam, Fabio Estevam, Jingoo Han, Tomi Valkeinen

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);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2017-01-24  9:33 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-22 20:58 [PATCH] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe() Lukasz Majewski
2016-10-24  2:34 ` Jingoo Han
2016-10-24 21:00   ` Lukasz Majewski
2016-10-25 22:47     ` Jingoo Han
2016-10-27  3:46       ` Lukasz Majewski
2016-11-01  7:25 ` [PATCH v2] " Lukasz Majewski
2016-11-08 22:25   ` Lukasz Majewski
2016-11-09 14:52     ` Lee Jones
2016-11-09 21:05       ` Lukasz Majewski
2016-11-10  7:47         ` Lee Jones
2016-11-10  8:16     ` Lee Jones
2016-11-10 23:20       ` Lukasz Majewski
2016-11-10  8:16   ` Lee Jones
2016-12-26 22:21 ` [PATCH v2 RESEND] " Lukasz Majewski
2017-01-02 23:54   ` Lukasz Majewski
2017-01-03  8:30   ` Lee Jones
2017-01-06 12:29     ` Lukasz Majewski
2017-01-13  0:27     ` Lukasz Majewski
2017-01-23  0:50       ` Lukasz Majewski
2017-01-23 16:09   ` Thierry Reding
2017-01-24  9:37   ` Lee Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).