Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove
From: Johan Hovold @ 2013-10-23  9:55 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold, stable
In-Reply-To: <1382522143-32072-1-git-send-email-jhovold@gmail.com>

Make sure to honour gpio polarity also at remove so that the backlight
is actually disabled on boards with active-low enable pin.

Cc: stable@vger.kernel.org
Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 8aac273..3cb0094 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -205,8 +205,10 @@ static int atmel_pwm_bl_remove(struct platform_device *pdev)
 {
 	struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
 
-	if (pwmbl->gpio_on != -1)
-		gpio_set_value(pwmbl->gpio_on, 0);
+	if (pwmbl->gpio_on != -1) {
+		gpio_set_value(pwmbl->gpio_on,
+					0 ^ pwmbl->pdata->on_active_low);
+	}
 	pwm_channel_disable(&pwmbl->pwmc);
 	pwm_channel_free(&pwmbl->pwmc);
 
-- 
1.8.4


^ permalink raw reply related

* [PATCH 3/9] backlight: atmel-pwm-bl: fix module autoload
From: Johan Hovold @ 2013-10-23  9:55 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1382522143-32072-1-git-send-email-jhovold@gmail.com>

Add missing module alias which is needed for module autoloading.

Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 3cb0094..cc5a5ed 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -229,3 +229,4 @@ module_platform_driver(atmel_pwm_bl_driver);
 MODULE_AUTHOR("Hans-Christian egtvedt <hans-christian.egtvedt@atmel.com>");
 MODULE_DESCRIPTION("Atmel PWM backlight driver");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:atmel-pwm-bl");
-- 
1.8.4


^ permalink raw reply related

* [PATCH 4/9] backlight: atmel-pwm-bl: clean up probe error handling
From: Johan Hovold @ 2013-10-23  9:55 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1382522143-32072-1-git-send-email-jhovold@gmail.com>

Clean up probe error handling by checking parameters before any
allocations and removing an obsolete error label. Also remove
unnecessary reset of private gpio number.

Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index cc5a5ed..52a8134 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -126,40 +126,33 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 	struct atmel_pwm_bl *pwmbl;
 	int retval;
 
+	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata)
+		return -ENODEV;
+
+	if (pdata->pwm_compare_max < pdata->pwm_duty_max ||
+			pdata->pwm_duty_min > pdata->pwm_duty_max ||
+			pdata->pwm_frequency = 0)
+		return -EINVAL;
+
 	pwmbl = devm_kzalloc(&pdev->dev, sizeof(struct atmel_pwm_bl),
 				GFP_KERNEL);
 	if (!pwmbl)
 		return -ENOMEM;
 
 	pwmbl->pdev = pdev;
-
-	pdata = dev_get_platdata(&pdev->dev);
-	if (!pdata) {
-		retval = -ENODEV;
-		goto err_free_mem;
-	}
-
-	if (pdata->pwm_compare_max < pdata->pwm_duty_max ||
-			pdata->pwm_duty_min > pdata->pwm_duty_max ||
-			pdata->pwm_frequency = 0) {
-		retval = -EINVAL;
-		goto err_free_mem;
-	}
-
 	pwmbl->pdata = pdata;
 	pwmbl->gpio_on = pdata->gpio_on;
 
 	retval = pwm_channel_alloc(pdata->pwm_channel, &pwmbl->pwmc);
 	if (retval)
-		goto err_free_mem;
+		return retval;
 
 	if (pwmbl->gpio_on != -1) {
 		retval = devm_gpio_request(&pdev->dev, pwmbl->gpio_on,
 					"gpio_atmel_pwm_bl");
-		if (retval) {
-			pwmbl->gpio_on = -1;
+		if (retval)
 			goto err_free_pwm;
-		}
 
 		/* Turn display off by default. */
 		retval = gpio_direction_output(pwmbl->gpio_on,
@@ -197,7 +190,7 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 
 err_free_pwm:
 	pwm_channel_free(&pwmbl->pwmc);
-err_free_mem:
+
 	return retval;
 }
 
-- 
1.8.4


^ permalink raw reply related

* [PATCH 5/9] backlight: atmel-pwm-bl: clean up get_intensity
From: Johan Hovold @ 2013-10-23  9:55 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1382522143-32072-1-git-send-email-jhovold@gmail.com>

Clean up get_intensity to increase readability.

Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 52a8134..504061c 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -70,15 +70,14 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
 {
 	struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
+	u32 cdty;
 	u32 intensity;
 
-	if (pwmbl->pdata->pwm_active_low) {
-		intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
-			pwmbl->pdata->pwm_duty_min;
-	} else {
-		intensity = pwmbl->pdata->pwm_duty_max -
-			pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
-	}
+	cdty = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
+	if (pwmbl->pdata->pwm_active_low)
+		intensity = cdty - pwmbl->pdata->pwm_duty_min;
+	else
+		intensity = pwmbl->pdata->pwm_duty_max - cdty;
 
 	return (u16)intensity;
 }
-- 
1.8.4


^ permalink raw reply related

* [PATCH 6/9] backlight: atmel-pwm-bl: remove unused include
From: Johan Hovold @ 2013-10-23  9:55 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1382522143-32072-1-git-send-email-jhovold@gmail.com>

Remove unused include of clk.h.

Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 504061c..db68cab 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -12,7 +12,6 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/fb.h>
-#include <linux/clk.h>
 #include <linux/gpio.h>
 #include <linux/backlight.h>
 #include <linux/atmel_pwm.h>
-- 
1.8.4


^ permalink raw reply related

* [PATCH 7/9] backlight: atmel-pwm-bl: use gpio_is_valid
From: Johan Hovold @ 2013-10-23  9:55 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1382522143-32072-1-git-send-email-jhovold@gmail.com>

Use gpio_is_valid rather than open coding the more restrictive != -1
test.

Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index db68cab..ffc30d2 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -48,7 +48,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 		pwm_duty = pwmbl->pdata->pwm_duty_min;
 
 	if (!intensity) {
-		if (pwmbl->gpio_on != -1) {
+		if (gpio_is_valid(pwmbl->gpio_on)) {
 			gpio_set_value(pwmbl->gpio_on,
 					0 ^ pwmbl->pdata->on_active_low);
 		}
@@ -57,7 +57,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 	} else {
 		pwm_channel_enable(&pwmbl->pwmc);
 		pwm_channel_writel(&pwmbl->pwmc, PWM_CUPD, pwm_duty);
-		if (pwmbl->gpio_on != -1) {
+		if (gpio_is_valid(pwmbl->gpio_on)) {
 			gpio_set_value(pwmbl->gpio_on,
 					1 ^ pwmbl->pdata->on_active_low);
 		}
@@ -146,7 +146,7 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 	if (retval)
 		return retval;
 
-	if (pwmbl->gpio_on != -1) {
+	if (gpio_is_valid(pwmbl->gpio_on)) {
 		retval = devm_gpio_request(&pdev->dev, pwmbl->gpio_on,
 					"gpio_atmel_pwm_bl");
 		if (retval)
@@ -196,7 +196,7 @@ static int atmel_pwm_bl_remove(struct platform_device *pdev)
 {
 	struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
 
-	if (pwmbl->gpio_on != -1) {
+	if (gpio_is_valid(pwmbl->gpio_on)) {
 		gpio_set_value(pwmbl->gpio_on,
 					0 ^ pwmbl->pdata->on_active_low);
 	}
-- 
1.8.4


^ permalink raw reply related

* [PATCH 8/9] backlight: atmel-pwm-bl: refactor gpio_on handling
From: Johan Hovold @ 2013-10-23  9:55 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1382522143-32072-1-git-send-email-jhovold@gmail.com>

Add helper function to control the gpio_on signal.

Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index ffc30d2..1277e0c 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -26,6 +26,14 @@ struct atmel_pwm_bl {
 	int					gpio_on;
 };
 
+static void atmel_pwm_bl_set_gpio_on(struct atmel_pwm_bl *pwmbl, int on)
+{
+	if (!gpio_is_valid(pwmbl->gpio_on))
+		return;
+
+	gpio_set_value(pwmbl->gpio_on, on ^ pwmbl->pdata->on_active_low);
+}
+
 static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 {
 	struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
@@ -48,19 +56,13 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 		pwm_duty = pwmbl->pdata->pwm_duty_min;
 
 	if (!intensity) {
-		if (gpio_is_valid(pwmbl->gpio_on)) {
-			gpio_set_value(pwmbl->gpio_on,
-					0 ^ pwmbl->pdata->on_active_low);
-		}
+		atmel_pwm_bl_set_gpio_on(pwmbl, 0);
 		pwm_channel_writel(&pwmbl->pwmc, PWM_CUPD, pwm_duty);
 		pwm_channel_disable(&pwmbl->pwmc);
 	} else {
 		pwm_channel_enable(&pwmbl->pwmc);
 		pwm_channel_writel(&pwmbl->pwmc, PWM_CUPD, pwm_duty);
-		if (gpio_is_valid(pwmbl->gpio_on)) {
-			gpio_set_value(pwmbl->gpio_on,
-					1 ^ pwmbl->pdata->on_active_low);
-		}
+		atmel_pwm_bl_set_gpio_on(pwmbl, 1);
 	}
 
 	return 0;
@@ -196,10 +198,7 @@ static int atmel_pwm_bl_remove(struct platform_device *pdev)
 {
 	struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
 
-	if (gpio_is_valid(pwmbl->gpio_on)) {
-		gpio_set_value(pwmbl->gpio_on,
-					0 ^ pwmbl->pdata->on_active_low);
-	}
+	atmel_pwm_bl_set_gpio_on(pwmbl, 0);
 	pwm_channel_disable(&pwmbl->pwmc);
 	pwm_channel_free(&pwmbl->pwmc);
 
-- 
1.8.4


^ permalink raw reply related

* [PATCH 9/9] backlight: atmel-pwm-bl: use gpio_request_one
From: Johan Hovold @ 2013-10-23  9:55 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1382522143-32072-1-git-send-email-jhovold@gmail.com>

Use devm_gpio_request_one rather than requesting and setting direction
in two calls.

Acked-by: Jingoo Han <jg1.han@samsung.com>:w
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 1277e0c..5ea2517 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -124,6 +124,7 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 	const struct atmel_pwm_bl_platform_data *pdata;
 	struct backlight_device *bldev;
 	struct atmel_pwm_bl *pwmbl;
+	int flags;
 	int retval;
 
 	pdata = dev_get_platdata(&pdev->dev);
@@ -149,14 +150,14 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 		return retval;
 
 	if (gpio_is_valid(pwmbl->gpio_on)) {
-		retval = devm_gpio_request(&pdev->dev, pwmbl->gpio_on,
-					"gpio_atmel_pwm_bl");
-		if (retval)
-			goto err_free_pwm;
-
 		/* Turn display off by default. */
-		retval = gpio_direction_output(pwmbl->gpio_on,
-				0 ^ pdata->on_active_low);
+		if (pdata->on_active_low)
+			flags = GPIOF_OUT_INIT_HIGH;
+		else
+			flags = GPIOF_OUT_INIT_LOW;
+
+		retval = devm_gpio_request_one(&pdev->dev, pwmbl->gpio_on,
+						flags, "gpio_atmel_pwm_bl");
 		if (retval)
 			goto err_free_pwm;
 	}
-- 
1.8.4


^ permalink raw reply related

* Re: How to set fops in "struct platform_pwm_backlight_data"?
From: Mark Zhang @ 2013-10-23 10:31 UTC (permalink / raw)
  To: Thierry Reding
  Cc: rpurdie, jg1.han, Jean-Christophe PLAGNIOL-VILLARD,
	tomi.valkeinen, linux-pwm, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20131023090925.GC11954@ulmo.nvidia.com>

On 10/23/2013 05:09 PM, Thierry Reding wrote:
> On Wed, Oct 23, 2013 at 04:49:29PM +0800, Mark Zhang wrote:
>> On 10/23/2013 04:00 PM, Thierry Reding wrote:
>>> On Wed, Oct 23, 2013 at 10:16:24AM +0800, Mark Zhang wrote:
>>>> On 10/22/2013 08:49 PM, Thierry Reding wrote:
>>>>> On Tue, Oct 22, 2013 at 04:55:09PM +0800, Mark Zhang wrote:
>>>>>> On 10/22/2013 03:24 PM, Thierry Reding wrote:
>>>>>>> On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
>>>>>> [...]
>>>>>>>>>
>>>>>>>>
>>>>>>>> Okay, I just want to set the "notify" function pointer in "struct
>>>>>>>> platform_pwm_backlight_data", because I want to tune the brightness
>>>>>>>> value before the pwm-bl sets the brightness to hardware. I don't know
>>>>>>>> how to do that, unless we define the platform data explicitly.
>>>>>>>
>>>>>>> Okay, my question should have been what you need the functions for and
>>>>>>> why you think you need them.
>>>>>>>
>>>>>>
>>>>>> If I understanding you correctly, I suppose I've said that: "because I
>>>>>> want to tune the brightness value before the pwm-bl sets the brightness
>>>>>> to hardware".
>>>>>
>>>>> Why do you want to tune the brightness value? What are you trying to
>>>>> achieve?
>>>>>
>>>>
>>>> Oh, Tegra has a feature named PRISM(aka SmartDimmer). It changes the
>>>> color value to make the display looks bright so that we can reduce the
>>>> backlight brightness to save power. So everytime PRISM is triggered, we
>>>> call "backlight_update_status", then in the "notify" callback, we change
>>>> the brightness value which pwm-bl provides by considering the PRISM
>>>> compensations.
>>>
>>> If you automatically call backlight_update_status() everytime PRISM
>>> gives you new data, can't you just pass the correct value in in the
>>> first place so that you don't have to tweak it in the .notify()
>>> callback?
>>
>> OK, how to do that? -- "pass the correct value in in the first place"?
> 
> Well, if you call backlight_update_status(), then you can pass in a
> brightness value, right? You usually do that by setting the backlight's
> props.brightness field.
> 
> So when PRISM gives you new data, you could just read out the current
> brightness, compute the new one based on the current one and the PRISM
> data, set the props.brightness field to that value and then call
> backlight_update_status().
> 

The param of "backlight_update_status" is "struct backlight_device *".
So you mean after I get a pointer of correct backlight device, just set
the brightness value I want to it's "props.brightness" directly?

I think the "struct backlight_device" should be opaque(although it's
definition is in include/linux/backlight.h, I know that), so it's better
not to touch it's member directly, that's why I wanna use that "notify"
callback.

Mark
> Thierry
> 

^ permalink raw reply

* Re: How to set fops in "struct platform_pwm_backlight_data"?
From: Mark Zhang @ 2013-10-23 10:36 UTC (permalink / raw)
  To: Thierry Reding
  Cc: rpurdie, jg1.han, Jean-Christophe PLAGNIOL-VILLARD,
	tomi.valkeinen, linux-pwm, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20131023090925.GC11954@ulmo.nvidia.com>

On 10/23/2013 05:09 PM, Thierry Reding wrote:
> On Wed, Oct 23, 2013 at 04:49:29PM +0800, Mark Zhang wrote:
>> On 10/23/2013 04:00 PM, Thierry Reding wrote:
>>> On Wed, Oct 23, 2013 at 10:16:24AM +0800, Mark Zhang wrote:
>>>> On 10/22/2013 08:49 PM, Thierry Reding wrote:
>>>>> On Tue, Oct 22, 2013 at 04:55:09PM +0800, Mark Zhang wrote:
>>>>>> On 10/22/2013 03:24 PM, Thierry Reding wrote:
>>>>>>> On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
>>>>>> [...]
>>>>>>>>>
>>>>>>>>
>>>>>>>> Okay, I just want to set the "notify" function pointer in "struct
>>>>>>>> platform_pwm_backlight_data", because I want to tune the brightness
>>>>>>>> value before the pwm-bl sets the brightness to hardware. I don't know
>>>>>>>> how to do that, unless we define the platform data explicitly.
>>>>>>>
>>>>>>> Okay, my question should have been what you need the functions for and
>>>>>>> why you think you need them.
>>>>>>>
>>>>>>
>>>>>> If I understanding you correctly, I suppose I've said that: "because I
>>>>>> want to tune the brightness value before the pwm-bl sets the brightness
>>>>>> to hardware".
>>>>>
>>>>> Why do you want to tune the brightness value? What are you trying to
>>>>> achieve?
>>>>>
>>>>
>>>> Oh, Tegra has a feature named PRISM(aka SmartDimmer). It changes the
>>>> color value to make the display looks bright so that we can reduce the
>>>> backlight brightness to save power. So everytime PRISM is triggered, we
>>>> call "backlight_update_status", then in the "notify" callback, we change
>>>> the brightness value which pwm-bl provides by considering the PRISM
>>>> compensations.
>>>
>>> If you automatically call backlight_update_status() everytime PRISM
>>> gives you new data, can't you just pass the correct value in in the
>>> first place so that you don't have to tweak it in the .notify()
>>> callback?
>>
>> OK, how to do that? -- "pass the correct value in in the first place"?
> 
> Well, if you call backlight_update_status(), then you can pass in a
> brightness value, right? You usually do that by setting the backlight's
> props.brightness field.
> 
> So when PRISM gives you new data, you could just read out the current
> brightness, compute the new one based on the current one and the PRISM
> data, set the props.brightness field to that value and then call
> backlight_update_status().
> 

Okay, anyway, I got the idea. Actually it's simpler. :)
I'm just curious that if we can do that in this way, why we need
"notify" anymore?

Mark
> Thierry
> 

^ permalink raw reply

* Re: How to set fops in "struct platform_pwm_backlight_data"?
From: Mark Zhang @ 2013-10-23 10:46 UTC (permalink / raw)
  To: Thierry Reding
  Cc: rpurdie, jg1.han, Jean-Christophe PLAGNIOL-VILLARD,
	tomi.valkeinen, linux-pwm, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <5267A691.8050503@gmail.com>

On 10/23/2013 06:36 PM, Mark Zhang wrote:
> On 10/23/2013 05:09 PM, Thierry Reding wrote:
>> On Wed, Oct 23, 2013 at 04:49:29PM +0800, Mark Zhang wrote:
>>> On 10/23/2013 04:00 PM, Thierry Reding wrote:
>>>> On Wed, Oct 23, 2013 at 10:16:24AM +0800, Mark Zhang wrote:
>>>>> On 10/22/2013 08:49 PM, Thierry Reding wrote:
>>>>>> On Tue, Oct 22, 2013 at 04:55:09PM +0800, Mark Zhang wrote:
>>>>>>> On 10/22/2013 03:24 PM, Thierry Reding wrote:
>>>>>>>> On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
>>>>>>> [...]
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Okay, I just want to set the "notify" function pointer in "struct
>>>>>>>>> platform_pwm_backlight_data", because I want to tune the brightness
>>>>>>>>> value before the pwm-bl sets the brightness to hardware. I don't know
>>>>>>>>> how to do that, unless we define the platform data explicitly.
>>>>>>>>
>>>>>>>> Okay, my question should have been what you need the functions for and
>>>>>>>> why you think you need them.
>>>>>>>>
>>>>>>>
>>>>>>> If I understanding you correctly, I suppose I've said that: "because I
>>>>>>> want to tune the brightness value before the pwm-bl sets the brightness
>>>>>>> to hardware".
>>>>>>
>>>>>> Why do you want to tune the brightness value? What are you trying to
>>>>>> achieve?
>>>>>>
>>>>>
>>>>> Oh, Tegra has a feature named PRISM(aka SmartDimmer). It changes the
>>>>> color value to make the display looks bright so that we can reduce the
>>>>> backlight brightness to save power. So everytime PRISM is triggered, we
>>>>> call "backlight_update_status", then in the "notify" callback, we change
>>>>> the brightness value which pwm-bl provides by considering the PRISM
>>>>> compensations.
>>>>
>>>> If you automatically call backlight_update_status() everytime PRISM
>>>> gives you new data, can't you just pass the correct value in in the
>>>> first place so that you don't have to tweak it in the .notify()
>>>> callback?
>>>
>>> OK, how to do that? -- "pass the correct value in in the first place"?
>>
>> Well, if you call backlight_update_status(), then you can pass in a
>> brightness value, right? You usually do that by setting the backlight's
>> props.brightness field.
>>
>> So when PRISM gives you new data, you could just read out the current
>> brightness, compute the new one based on the current one and the PRISM
>> data, set the props.brightness field to that value and then call
>> backlight_update_status().
>>
> 
> Okay, anyway, I got the idea. Actually it's simpler. :)
> I'm just curious that if we can do that in this way, why we need
> "notify" anymore?

Oh, by the way, how about "check_fb" fops? Is there some kind of
substitution as well? I mean, if I wanna set "check_fb" and also want to
define the backlight device via DT, is there a way to do that?

Mark
> 
> Mark
>> Thierry
>>

^ permalink raw reply

* Re: How to set fops in "struct platform_pwm_backlight_data"?
From: Thierry Reding @ 2013-10-23 10:51 UTC (permalink / raw)
  To: Mark Zhang
  Cc: rpurdie, jg1.han, Jean-Christophe PLAGNIOL-VILLARD,
	tomi.valkeinen, linux-pwm, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <5267A691.8050503@gmail.com>

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

On Wed, Oct 23, 2013 at 06:36:01PM +0800, Mark Zhang wrote:
> On 10/23/2013 05:09 PM, Thierry Reding wrote:
> > On Wed, Oct 23, 2013 at 04:49:29PM +0800, Mark Zhang wrote:
> >> On 10/23/2013 04:00 PM, Thierry Reding wrote:
> >>> On Wed, Oct 23, 2013 at 10:16:24AM +0800, Mark Zhang wrote:
> >>>> On 10/22/2013 08:49 PM, Thierry Reding wrote:
> >>>>> On Tue, Oct 22, 2013 at 04:55:09PM +0800, Mark Zhang wrote:
> >>>>>> On 10/22/2013 03:24 PM, Thierry Reding wrote:
> >>>>>>> On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
> >>>>>> [...]
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> Okay, I just want to set the "notify" function pointer in "struct
> >>>>>>>> platform_pwm_backlight_data", because I want to tune the brightness
> >>>>>>>> value before the pwm-bl sets the brightness to hardware. I don't know
> >>>>>>>> how to do that, unless we define the platform data explicitly.
> >>>>>>>
> >>>>>>> Okay, my question should have been what you need the functions for and
> >>>>>>> why you think you need them.
> >>>>>>>
> >>>>>>
> >>>>>> If I understanding you correctly, I suppose I've said that: "because I
> >>>>>> want to tune the brightness value before the pwm-bl sets the brightness
> >>>>>> to hardware".
> >>>>>
> >>>>> Why do you want to tune the brightness value? What are you trying to
> >>>>> achieve?
> >>>>>
> >>>>
> >>>> Oh, Tegra has a feature named PRISM(aka SmartDimmer). It changes the
> >>>> color value to make the display looks bright so that we can reduce the
> >>>> backlight brightness to save power. So everytime PRISM is triggered, we
> >>>> call "backlight_update_status", then in the "notify" callback, we change
> >>>> the brightness value which pwm-bl provides by considering the PRISM
> >>>> compensations.
> >>>
> >>> If you automatically call backlight_update_status() everytime PRISM
> >>> gives you new data, can't you just pass the correct value in in the
> >>> first place so that you don't have to tweak it in the .notify()
> >>> callback?
> >>
> >> OK, how to do that? -- "pass the correct value in in the first place"?
> > 
> > Well, if you call backlight_update_status(), then you can pass in a
> > brightness value, right? You usually do that by setting the backlight's
> > props.brightness field.
> > 
> > So when PRISM gives you new data, you could just read out the current
> > brightness, compute the new one based on the current one and the PRISM
> > data, set the props.brightness field to that value and then call
> > backlight_update_status().
> > 
> 
> Okay, anyway, I got the idea. Actually it's simpler. :)
> I'm just curious that if we can do that in this way, why we need
> "notify" anymore?

We don't need it any longer for DT, you've just proven that. =)

However we still need to support board files, and the .notify() callback
is the only way for board files to use board specific means to enable or
disable the backlight depending on the brightness value.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: How to set fops in "struct platform_pwm_backlight_data"?
From: Thierry Reding @ 2013-10-23 10:54 UTC (permalink / raw)
  To: Mark Zhang
  Cc: rpurdie, jg1.han, Jean-Christophe PLAGNIOL-VILLARD,
	tomi.valkeinen, linux-pwm, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <5267A582.9070504@gmail.com>

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

On Wed, Oct 23, 2013 at 06:31:30PM +0800, Mark Zhang wrote:
> On 10/23/2013 05:09 PM, Thierry Reding wrote:
> > On Wed, Oct 23, 2013 at 04:49:29PM +0800, Mark Zhang wrote:
> >> On 10/23/2013 04:00 PM, Thierry Reding wrote:
> >>> On Wed, Oct 23, 2013 at 10:16:24AM +0800, Mark Zhang wrote:
> >>>> On 10/22/2013 08:49 PM, Thierry Reding wrote:
> >>>>> On Tue, Oct 22, 2013 at 04:55:09PM +0800, Mark Zhang wrote:
> >>>>>> On 10/22/2013 03:24 PM, Thierry Reding wrote:
> >>>>>>> On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
> >>>>>> [...]
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> Okay, I just want to set the "notify" function pointer in "struct
> >>>>>>>> platform_pwm_backlight_data", because I want to tune the brightness
> >>>>>>>> value before the pwm-bl sets the brightness to hardware. I don't know
> >>>>>>>> how to do that, unless we define the platform data explicitly.
> >>>>>>>
> >>>>>>> Okay, my question should have been what you need the functions for and
> >>>>>>> why you think you need them.
> >>>>>>>
> >>>>>>
> >>>>>> If I understanding you correctly, I suppose I've said that: "because I
> >>>>>> want to tune the brightness value before the pwm-bl sets the brightness
> >>>>>> to hardware".
> >>>>>
> >>>>> Why do you want to tune the brightness value? What are you trying to
> >>>>> achieve?
> >>>>>
> >>>>
> >>>> Oh, Tegra has a feature named PRISM(aka SmartDimmer). It changes the
> >>>> color value to make the display looks bright so that we can reduce the
> >>>> backlight brightness to save power. So everytime PRISM is triggered, we
> >>>> call "backlight_update_status", then in the "notify" callback, we change
> >>>> the brightness value which pwm-bl provides by considering the PRISM
> >>>> compensations.
> >>>
> >>> If you automatically call backlight_update_status() everytime PRISM
> >>> gives you new data, can't you just pass the correct value in in the
> >>> first place so that you don't have to tweak it in the .notify()
> >>> callback?
> >>
> >> OK, how to do that? -- "pass the correct value in in the first place"?
> > 
> > Well, if you call backlight_update_status(), then you can pass in a
> > brightness value, right? You usually do that by setting the backlight's
> > props.brightness field.
> > 
> > So when PRISM gives you new data, you could just read out the current
> > brightness, compute the new one based on the current one and the PRISM
> > data, set the props.brightness field to that value and then call
> > backlight_update_status().
> > 
> 
> The param of "backlight_update_status" is "struct backlight_device *".
> So you mean after I get a pointer of correct backlight device, just set
> the brightness value I want to it's "props.brightness" directly?

Yes.

> I think the "struct backlight_device" should be opaque(although it's
> definition is in include/linux/backlight.h, I know that), so it's better
> not to touch it's member directly, that's why I wanna use that "notify"
> callback.

Well, backlight_update_status() is the only way to change the brightness
of a backlight. If nobody ever calls backlight_update_status() then the
.notify() callback will never be called either.

By the way, what method do you use to control the backlight brightness?
Do you use the sysfs interface from userspace?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: How to set fops in "struct platform_pwm_backlight_data"?
From: Thierry Reding @ 2013-10-23 10:58 UTC (permalink / raw)
  To: Mark Zhang
  Cc: rpurdie, jg1.han, Jean-Christophe PLAGNIOL-VILLARD,
	tomi.valkeinen, linux-pwm, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <5267A8F9.9050907@gmail.com>

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

On Wed, Oct 23, 2013 at 06:46:17PM +0800, Mark Zhang wrote:
> On 10/23/2013 06:36 PM, Mark Zhang wrote:
> > On 10/23/2013 05:09 PM, Thierry Reding wrote:
> >> On Wed, Oct 23, 2013 at 04:49:29PM +0800, Mark Zhang wrote:
> >>> On 10/23/2013 04:00 PM, Thierry Reding wrote:
> >>>> On Wed, Oct 23, 2013 at 10:16:24AM +0800, Mark Zhang wrote:
> >>>>> On 10/22/2013 08:49 PM, Thierry Reding wrote:
> >>>>>> On Tue, Oct 22, 2013 at 04:55:09PM +0800, Mark Zhang wrote:
> >>>>>>> On 10/22/2013 03:24 PM, Thierry Reding wrote:
> >>>>>>>> On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
> >>>>>>> [...]
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Okay, I just want to set the "notify" function pointer in "struct
> >>>>>>>>> platform_pwm_backlight_data", because I want to tune the brightness
> >>>>>>>>> value before the pwm-bl sets the brightness to hardware. I don't know
> >>>>>>>>> how to do that, unless we define the platform data explicitly.
> >>>>>>>>
> >>>>>>>> Okay, my question should have been what you need the functions for and
> >>>>>>>> why you think you need them.
> >>>>>>>>
> >>>>>>>
> >>>>>>> If I understanding you correctly, I suppose I've said that: "because I
> >>>>>>> want to tune the brightness value before the pwm-bl sets the brightness
> >>>>>>> to hardware".
> >>>>>>
> >>>>>> Why do you want to tune the brightness value? What are you trying to
> >>>>>> achieve?
> >>>>>>
> >>>>>
> >>>>> Oh, Tegra has a feature named PRISM(aka SmartDimmer). It changes the
> >>>>> color value to make the display looks bright so that we can reduce the
> >>>>> backlight brightness to save power. So everytime PRISM is triggered, we
> >>>>> call "backlight_update_status", then in the "notify" callback, we change
> >>>>> the brightness value which pwm-bl provides by considering the PRISM
> >>>>> compensations.
> >>>>
> >>>> If you automatically call backlight_update_status() everytime PRISM
> >>>> gives you new data, can't you just pass the correct value in in the
> >>>> first place so that you don't have to tweak it in the .notify()
> >>>> callback?
> >>>
> >>> OK, how to do that? -- "pass the correct value in in the first place"?
> >>
> >> Well, if you call backlight_update_status(), then you can pass in a
> >> brightness value, right? You usually do that by setting the backlight's
> >> props.brightness field.
> >>
> >> So when PRISM gives you new data, you could just read out the current
> >> brightness, compute the new one based on the current one and the PRISM
> >> data, set the props.brightness field to that value and then call
> >> backlight_update_status().
> >>
> > 
> > Okay, anyway, I got the idea. Actually it's simpler. :)
> > I'm just curious that if we can do that in this way, why we need
> > "notify" anymore?
> 
> Oh, by the way, how about "check_fb" fops? Is there some kind of
> substitution as well? I mean, if I wanna set "check_fb" and also want to
> define the backlight device via DT, is there a way to do that?

No, there's no substitution for that. .check_fb() is used to verify that
a backlight device is associated with a framebuffer device. There are
better ways to check for that with DT, although nothing has been merged
into mainline yet.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: How to set fops in "struct platform_pwm_backlight_data"?
From: Mark Zhang @ 2013-10-23 11:33 UTC (permalink / raw)
  To: Thierry Reding
  Cc: rpurdie, jg1.han, Jean-Christophe PLAGNIOL-VILLARD,
	tomi.valkeinen, linux-pwm, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20131023105456.GB15082@ulmo.nvidia.com>

On 10/23/2013 06:54 PM, Thierry Reding wrote:
> On Wed, Oct 23, 2013 at 06:31:30PM +0800, Mark Zhang wrote:
>> On 10/23/2013 05:09 PM, Thierry Reding wrote:
>>> On Wed, Oct 23, 2013 at 04:49:29PM +0800, Mark Zhang wrote:
>>>> On 10/23/2013 04:00 PM, Thierry Reding wrote:
>>>>> On Wed, Oct 23, 2013 at 10:16:24AM +0800, Mark Zhang wrote:
>>>>>> On 10/22/2013 08:49 PM, Thierry Reding wrote:
>>>>>>> On Tue, Oct 22, 2013 at 04:55:09PM +0800, Mark Zhang wrote:
>>>>>>>> On 10/22/2013 03:24 PM, Thierry Reding wrote:
>>>>>>>>> On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
>>>>>>>> [...]
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Okay, I just want to set the "notify" function pointer in "struct
>>>>>>>>>> platform_pwm_backlight_data", because I want to tune the brightness
>>>>>>>>>> value before the pwm-bl sets the brightness to hardware. I don't know
>>>>>>>>>> how to do that, unless we define the platform data explicitly.
>>>>>>>>>
>>>>>>>>> Okay, my question should have been what you need the functions for and
>>>>>>>>> why you think you need them.
>>>>>>>>>
>>>>>>>>
>>>>>>>> If I understanding you correctly, I suppose I've said that: "because I
>>>>>>>> want to tune the brightness value before the pwm-bl sets the brightness
>>>>>>>> to hardware".
>>>>>>>
>>>>>>> Why do you want to tune the brightness value? What are you trying to
>>>>>>> achieve?
>>>>>>>
>>>>>>
>>>>>> Oh, Tegra has a feature named PRISM(aka SmartDimmer). It changes the
>>>>>> color value to make the display looks bright so that we can reduce the
>>>>>> backlight brightness to save power. So everytime PRISM is triggered, we
>>>>>> call "backlight_update_status", then in the "notify" callback, we change
>>>>>> the brightness value which pwm-bl provides by considering the PRISM
>>>>>> compensations.
>>>>>
>>>>> If you automatically call backlight_update_status() everytime PRISM
>>>>> gives you new data, can't you just pass the correct value in in the
>>>>> first place so that you don't have to tweak it in the .notify()
>>>>> callback?
>>>>
>>>> OK, how to do that? -- "pass the correct value in in the first place"?
>>>
>>> Well, if you call backlight_update_status(), then you can pass in a
>>> brightness value, right? You usually do that by setting the backlight's
>>> props.brightness field.
>>>
>>> So when PRISM gives you new data, you could just read out the current
>>> brightness, compute the new one based on the current one and the PRISM
>>> data, set the props.brightness field to that value and then call
>>> backlight_update_status().
>>>
>>
>> The param of "backlight_update_status" is "struct backlight_device *".
>> So you mean after I get a pointer of correct backlight device, just set
>> the brightness value I want to it's "props.brightness" directly?
> 
> Yes.
> 
>> I think the "struct backlight_device" should be opaque(although it's
>> definition is in include/linux/backlight.h, I know that), so it's better
>> not to touch it's member directly, that's why I wanna use that "notify"
>> callback.
> 
> Well, backlight_update_status() is the only way to change the brightness
> of a backlight. If nobody ever calls backlight_update_status() then the
> .notify() callback will never be called either.
> 

Yes, so this is exactly my understanding before I posted this thread. I
think "struct backlight_device" is a handle, we get it and call
"backlight_update_status", then if we wanna change it's default behavior
or brightness value, we overwrite the fops it provides.

> By the way, what method do you use to control the backlight brightness?
> Do you use the sysfs interface from userspace?
> 

Yes.

Mark
> Thierry
> 

^ permalink raw reply

* Re: How to set fops in "struct platform_pwm_backlight_data"?
From: Mark Zhang @ 2013-10-23 11:34 UTC (permalink / raw)
  To: Thierry Reding
  Cc: rpurdie, jg1.han, Jean-Christophe PLAGNIOL-VILLARD,
	tomi.valkeinen, linux-pwm, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20131023105821.GC15082@ulmo.nvidia.com>

On 10/23/2013 06:58 PM, Thierry Reding wrote:
> On Wed, Oct 23, 2013 at 06:46:17PM +0800, Mark Zhang wrote:
>> On 10/23/2013 06:36 PM, Mark Zhang wrote:
>>> On 10/23/2013 05:09 PM, Thierry Reding wrote:
>>>> On Wed, Oct 23, 2013 at 04:49:29PM +0800, Mark Zhang wrote:
>>>>> On 10/23/2013 04:00 PM, Thierry Reding wrote:
>>>>>> On Wed, Oct 23, 2013 at 10:16:24AM +0800, Mark Zhang wrote:
>>>>>>> On 10/22/2013 08:49 PM, Thierry Reding wrote:
>>>>>>>> On Tue, Oct 22, 2013 at 04:55:09PM +0800, Mark Zhang wrote:
>>>>>>>>> On 10/22/2013 03:24 PM, Thierry Reding wrote:
>>>>>>>>>> On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
>>>>>>>>> [...]
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Okay, I just want to set the "notify" function pointer in "struct
>>>>>>>>>>> platform_pwm_backlight_data", because I want to tune the brightness
>>>>>>>>>>> value before the pwm-bl sets the brightness to hardware. I don't know
>>>>>>>>>>> how to do that, unless we define the platform data explicitly.
>>>>>>>>>>
>>>>>>>>>> Okay, my question should have been what you need the functions for and
>>>>>>>>>> why you think you need them.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> If I understanding you correctly, I suppose I've said that: "because I
>>>>>>>>> want to tune the brightness value before the pwm-bl sets the brightness
>>>>>>>>> to hardware".
>>>>>>>>
>>>>>>>> Why do you want to tune the brightness value? What are you trying to
>>>>>>>> achieve?
>>>>>>>>
>>>>>>>
>>>>>>> Oh, Tegra has a feature named PRISM(aka SmartDimmer). It changes the
>>>>>>> color value to make the display looks bright so that we can reduce the
>>>>>>> backlight brightness to save power. So everytime PRISM is triggered, we
>>>>>>> call "backlight_update_status", then in the "notify" callback, we change
>>>>>>> the brightness value which pwm-bl provides by considering the PRISM
>>>>>>> compensations.
>>>>>>
>>>>>> If you automatically call backlight_update_status() everytime PRISM
>>>>>> gives you new data, can't you just pass the correct value in in the
>>>>>> first place so that you don't have to tweak it in the .notify()
>>>>>> callback?
>>>>>
>>>>> OK, how to do that? -- "pass the correct value in in the first place"?
>>>>
>>>> Well, if you call backlight_update_status(), then you can pass in a
>>>> brightness value, right? You usually do that by setting the backlight's
>>>> props.brightness field.
>>>>
>>>> So when PRISM gives you new data, you could just read out the current
>>>> brightness, compute the new one based on the current one and the PRISM
>>>> data, set the props.brightness field to that value and then call
>>>> backlight_update_status().
>>>>
>>>
>>> Okay, anyway, I got the idea. Actually it's simpler. :)
>>> I'm just curious that if we can do that in this way, why we need
>>> "notify" anymore?
>>
>> Oh, by the way, how about "check_fb" fops? Is there some kind of
>> substitution as well? I mean, if I wanna set "check_fb" and also want to
>> define the backlight device via DT, is there a way to do that?
> 
> No, there's no substitution for that. .check_fb() is used to verify that
> a backlight device is associated with a framebuffer device. There are
> better ways to check for that with DT, although nothing has been merged
> into mainline yet.
> 

Okay, thanks for the explanation.

Mark
> Thierry
> 

^ permalink raw reply

* [PATCHv3][ 1/5] fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_*
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: linux-arm-kernel

Without that fix, drivers using the fb_videomode_from_videomode
  function will not be able to get certain information because
  some DISPLAY_FLAGS_* have no corresponding FB_SYNC_*.

Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
ChangeLog v2->v3:
- Added Jean-Christophe PLAGNIOL-VILLARD's ACK.
---
 drivers/video/fbmon.c   |    4 ++++
 include/uapi/linux/fb.h |    2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 6103fa6..29a9ed0 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -1402,6 +1402,10 @@ int fb_videomode_from_videomode(const struct videomode *vm,
 		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
 	if (vm->flags & DISPLAY_FLAGS_VSYNC_HIGH)
 		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
+		fbmode->sync |= FB_SYNC_DE_HIGH_ACT;
+	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+		fbmode->sync |= FB_SYNC_PIXDAT_HIGH_ACT;
 	if (vm->flags & DISPLAY_FLAGS_INTERLACED)
 		fbmode->vmode |= FB_VMODE_INTERLACED;
 	if (vm->flags & DISPLAY_FLAGS_DOUBLESCAN)
diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
index fb795c3..30487df 100644
--- a/include/uapi/linux/fb.h
+++ b/include/uapi/linux/fb.h
@@ -215,6 +215,8 @@ struct fb_bitfield {
 					/* vtotal = 144d/288n/576i => PAL  */
 					/* vtotal = 121d/242n/484i => NTSC */
 #define FB_SYNC_ON_GREEN	32	/* sync on green */
+#define FB_SYNC_DE_HIGH_ACT	64	/* data enable high active */
+#define FB_SYNC_PIXDAT_HIGH_ACT	64	/* data enable high active */
 
 #define FB_VMODE_NONINTERLACED  0	/* non interlaced */
 #define FB_VMODE_INTERLACED	1	/* interlaced	*/
-- 
1.7.9.5


^ permalink raw reply related

* [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1382532229-32755-1-git-send-email-denis@eukrea.com>

Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v3:
- The device tree bindings were reworked in order to make it look more like the
  IPUv3 bindings.
- The interface_pix_fmt property now looks like the IPUv3 one.
---
 .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
 drivers/video/Kconfig                              |    2 +
 drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
 3 files changed, 147 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt

diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
new file mode 100644
index 0000000..0b31374
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
@@ -0,0 +1,35 @@
+Freescale MX3 fb
+========
+
+Required properties:
+- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
+  imx35.
+- reg: should be register base and length as documented in the datasheet.
+- clocks: Handle to the ipu_gate clock.
+
+Example:
+
+lcdc: mx3fb@53fc00b4 {
+	compatible = "fsl,mx3-fb";
+	reg = <0x53fc00b4 0x0b>;
+	clocks = <&clks 55>;
+};
+
+Display support
+=======+Required properties:
+- model : The user-visible name of the display.
+
+Optional properties:
+- interface_pix_fmt: How this display is connected to the
+  crtc. Currently supported types: "rgb24", "rgb565", "rgb666"
+
+It can also have an optional timing subnode as described in
+  Documentation/devicetree/bindings/video/display-timing.txt.
+
+Example:
+
+display@di0 {
+	    interface-pix-fmt = "rgb666";
+	    model = "CMO-QVGA";
+};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 14317b7..2a638df 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2359,6 +2359,8 @@ config FB_MX3
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
+	select VIDEOMODE_HELPERS
+	select FB_MODE_HELPERS
 	default y
 	help
 	  This is a framebuffer device for the i.MX31 LCD Controller. So
diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index 804f874..de5a6c8 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -31,6 +31,8 @@
 #include <linux/platform_data/dma-imx.h>
 #include <linux/platform_data/video-mx3fb.h>
 
+#include <video/of_display_timing.h>
+
 #include <asm/io.h>
 #include <asm/uaccess.h>
 
@@ -757,11 +759,13 @@ static int __set_par(struct fb_info *fbi, bool lock)
 			sig_cfg.Hsync_pol = true;
 		if (fbi->var.sync & FB_SYNC_VERT_HIGH_ACT)
 			sig_cfg.Vsync_pol = true;
-		if (fbi->var.sync & FB_SYNC_CLK_INVERT)
+		if ((fbi->var.sync & FB_SYNC_CLK_INVERT) ||
+		    (fbi->var.sync & FB_SYNC_PIXDAT_HIGH_ACT))
 			sig_cfg.clk_pol = true;
 		if (fbi->var.sync & FB_SYNC_DATA_INVERT)
 			sig_cfg.data_pol = true;
-		if (fbi->var.sync & FB_SYNC_OE_ACT_HIGH)
+		if ((fbi->var.sync & FB_SYNC_OE_ACT_HIGH) ||
+		    (fbi->var.sync & FB_SYNC_DE_HIGH_ACT))
 			sig_cfg.enable_pol = true;
 		if (fbi->var.sync & FB_SYNC_CLK_IDLE_EN)
 			sig_cfg.clkidle_en = true;
@@ -1351,21 +1355,75 @@ static struct fb_info *mx3fb_init_fbinfo(struct device *dev, struct fb_ops *ops)
 	return fbi;
 }
 
+static int match_dt_disp_data(const char *property)
+{
+	if (!strcmp("rgb666", property))
+		return IPU_DISP_DATA_MAPPING_RGB666;
+	else if (!strcmp("rgb565", property))
+		return IPU_DISP_DATA_MAPPING_RGB565;
+	else if (!strcmp("rgb24", property))
+		return IPU_DISP_DATA_MAPPING_RGB888;
+	else
+		return -EINVAL;
+}
+
 static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 {
 	struct device *dev = mx3fb->dev;
 	struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
-	const char *name = mx3fb_pdata->name;
+	struct device_node *np = dev->of_node;
+	const char *name;
+	const char *ipu_disp_format;
 	unsigned int irq;
 	struct fb_info *fbi;
 	struct mx3fb_info *mx3fbi;
 	const struct fb_videomode *mode;
 	int ret, num_modes;
+	struct fb_videomode of_mode;
+	struct device_node *display_np, *root_np;
+
+	if (np) {
+		root_np = of_find_node_by_path("/");
+		if (!root_np) {
+			dev_err(dev, "Can't get the \"/\" node.\n");
+			return -EINVAL;
+		}
+
+		display_np = of_find_node_by_name(root_np, "display");
+		if (!display_np) {
+			dev_err(dev, "Can't get the display device node.\n");
+			return -EINVAL;
+		}
+
+		of_property_read_string(display_np, "interface-pix-fmt",
+					&ipu_disp_format);
+		if (!ipu_disp_format) {
+			mx3fb->disp_data_fmt = IPU_DISP_DATA_MAPPING_RGB666;
+			dev_warn(dev,
+				"ipu display data mapping was not defined, using the default rgb666.\n");
+		} else {
+			mx3fb->disp_data_fmt +				match_dt_disp_data(ipu_disp_format);
+		}
+
+		if (mx3fb->disp_data_fmt = -EINVAL) {
+			dev_err(dev, "Illegal display data format \"%s\"\n",
+				ipu_disp_format);
+			return -EINVAL;
+		}
 
-	if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
-		dev_err(dev, "Illegal display data format %d\n",
+		of_property_read_string(display_np, "model", &name);
+		if (ret) {
+			dev_err(dev, "Missing display model name\n");
+			return -EINVAL;
+		}
+	} else {
+		name = mx3fb_pdata->name;
+		if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
+			dev_err(dev, "Illegal display data format %d\n",
 				mx3fb_pdata->disp_data_fmt);
-		return -EINVAL;
+			return -EINVAL;
+		}
 	}
 
 	ichan->client = mx3fb;
@@ -1386,12 +1444,24 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 		goto emode;
 	}
 
-	if (mx3fb_pdata->mode && mx3fb_pdata->num_modes) {
-		mode = mx3fb_pdata->mode;
-		num_modes = mx3fb_pdata->num_modes;
+	if (np) {
+		ret = of_get_fb_videomode(display_np, &of_mode,
+					  OF_USE_NATIVE_MODE);
+		if (!ret) {
+			mode = &of_mode;
+			num_modes = 1;
+		} else {
+			mode = mx3fb_modedb;
+			num_modes = ARRAY_SIZE(mx3fb_modedb);
+		}
 	} else {
-		mode = mx3fb_modedb;
-		num_modes = ARRAY_SIZE(mx3fb_modedb);
+		if (mx3fb_pdata->mode || !mx3fb_pdata->num_modes) {
+			mode = mx3fb_pdata->mode;
+			num_modes = mx3fb_pdata->num_modes;
+		} else {
+			mode = mx3fb_modedb;
+			num_modes = ARRAY_SIZE(mx3fb_modedb);
+		}
 	}
 
 	if (!fb_find_mode(&fbi->var, fbi, fb_mode, mode,
@@ -1421,7 +1491,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	mx3fbi->mx3fb		= mx3fb;
 	mx3fbi->blank		= FB_BLANK_NORMAL;
 
-	mx3fb->disp_data_fmt	= mx3fb_pdata->disp_data_fmt;
+	if (!np)
+		mx3fb->disp_data_fmt = mx3fb_pdata->disp_data_fmt;
 
 	init_completion(&mx3fbi->flip_cmpl);
 	disable_irq(ichan->eof_irq);
@@ -1449,13 +1520,26 @@ emode:
 	return ret;
 }
 
+static int imx_dma_is_dt_ipu(struct dma_chan *chan)
+{
+	/* Example: 53fc0000.ipu */
+	if (chan && chan->device && chan->device->dev) {
+		char *name = dev_name(chan->device->dev);
+		name = strchr(name, '.');
+		if (name)
+			return !strcmp(name, ".ipu");
+	}
+
+	return 0;
+}
+
 static bool chan_filter(struct dma_chan *chan, void *arg)
 {
 	struct dma_chan_request *rq = arg;
 	struct device *dev;
 	struct mx3fb_platform_data *mx3fb_pdata;
 
-	if (!imx_dma_is_ipu(chan))
+	if (!imx_dma_is_ipu(chan) && !imx_dma_is_dt_ipu(chan))
 		return false;
 
 	if (!rq)
@@ -1464,8 +1548,12 @@ static bool chan_filter(struct dma_chan *chan, void *arg)
 	dev = rq->mx3fb->dev;
 	mx3fb_pdata = dev_get_platdata(dev);
 
-	return rq->id = chan->chan_id &&
-		mx3fb_pdata->dma_dev = chan->device->dev;
+	/* When using the devicetree, mx3fb_pdata is NULL */
+	if (imx_dma_is_dt_ipu(chan))
+		return rq->id = chan->chan_id;
+	else
+		return rq->id = chan->chan_id &&
+			mx3fb_pdata->dma_dev = chan->device->dev;
 }
 
 static void release_fbi(struct fb_info *fbi)
@@ -1565,9 +1653,16 @@ static int mx3fb_remove(struct platform_device *dev)
 	return 0;
 }
 
+static struct of_device_id mx3fb_of_dev_id[] = {
+	{ .compatible = "fsl,mx3-fb", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mx3fb_of_dev_id);
+
 static struct platform_driver mx3fb_driver = {
 	.driver = {
 		.name = MX3FB_NAME,
+		.of_match_table = mx3fb_of_dev_id,
 		.owner = THIS_MODULE,
 	},
 	.probe = mx3fb_probe,
-- 
1.7.9.5


^ permalink raw reply related

* [PATCHv3][ 4/5] video: mx3fb: Introduce regulator support.
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1382532229-32755-1-git-send-email-denis@eukrea.com>

This commit is based on the following commit by Fabio Estevam:
  4344429 video: mxsfb: Introduce regulator support

Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v3:
- The prints are now replaced with non line wrapped prints.
- The regulator retrival has been adapted to the new DT bindings which looks
  more like the IPUv3 ones.
- The regulator_is_enabled checks were kept, because regulator_disable do not
  do such check.
---
 drivers/video/mx3fb.c |   67 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index de5a6c8..1f2ce6d 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -27,6 +27,7 @@
 #include <linux/clk.h>
 #include <linux/mutex.h>
 #include <linux/dma/ipu-dma.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/platform_data/dma-imx.h>
 #include <linux/platform_data/video-mx3fb.h>
@@ -269,6 +270,7 @@ struct mx3fb_info {
 	struct dma_async_tx_descriptor	*txd;
 	dma_cookie_t			cookie;
 	struct scatterlist		sg[2];
+	struct regulator		*reg_lcd;
 
 	struct fb_var_screeninfo	cur_var; /* current var info */
 };
@@ -1005,6 +1007,7 @@ static void __blank(int blank, struct fb_info *fbi)
 	struct mx3fb_info *mx3_fbi = fbi->par;
 	struct mx3fb_data *mx3fb = mx3_fbi->mx3fb;
 	int was_blank = mx3_fbi->blank;
+	int ret;
 
 	mx3_fbi->blank = blank;
 
@@ -1023,6 +1026,15 @@ static void __blank(int blank, struct fb_info *fbi)
 	case FB_BLANK_HSYNC_SUSPEND:
 	case FB_BLANK_NORMAL:
 		sdc_set_brightness(mx3fb, 0);
+		if (mx3_fbi->reg_lcd) {
+			if (regulator_is_enabled(mx3_fbi->reg_lcd)) {
+				ret = regulator_disable(mx3_fbi->reg_lcd);
+				if (ret)
+					dev_warn(fbi->device,
+						 "lcd regulator disable failed with error: %d\n",
+						 ret);
+			}
+		}
 		memset((char *)fbi->screen_base, 0, fbi->fix.smem_len);
 		/* Give LCD time to update - enough for 50 and 60 Hz */
 		msleep(25);
@@ -1030,6 +1042,15 @@ static void __blank(int blank, struct fb_info *fbi)
 		break;
 	case FB_BLANK_UNBLANK:
 		sdc_enable_channel(mx3_fbi);
+		if (mx3_fbi->reg_lcd) {
+			if (!regulator_is_enabled(mx3_fbi->reg_lcd)) {
+				ret = regulator_enable(mx3_fbi->reg_lcd);
+				if (ret)
+					dev_warn(fbi->device,
+						 "lcd regulator enable failed with error: %d\n",
+						 ret);
+			}
+		}
 		sdc_set_brightness(mx3fb, mx3fb->backlight_level);
 		break;
 	}
@@ -1206,6 +1227,7 @@ static int mx3fb_suspend(struct platform_device *pdev, pm_message_t state)
 {
 	struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
 	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
+	int ret;
 
 	console_lock();
 	fb_set_suspend(mx3fb->fbi, 1);
@@ -1214,7 +1236,15 @@ static int mx3fb_suspend(struct platform_device *pdev, pm_message_t state)
 	if (mx3_fbi->blank = FB_BLANK_UNBLANK) {
 		sdc_disable_channel(mx3_fbi);
 		sdc_set_brightness(mx3fb, 0);
-
+		if (mx3_fbi->reg_lcd) {
+			if (regulator_is_enabled(mx3_fbi->reg_lcd)) {
+				ret = regulator_disable(mx3_fbi->reg_lcd);
+				if (ret)
+					dev_warn(&pdev->dev,
+						 "lcd regulator disable failed with error: %d\n",
+						 ret);
+			}
+		}
 	}
 	return 0;
 }
@@ -1226,10 +1256,20 @@ static int mx3fb_resume(struct platform_device *pdev)
 {
 	struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
 	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
+	int ret;
 
 	if (mx3_fbi->blank = FB_BLANK_UNBLANK) {
 		sdc_enable_channel(mx3_fbi);
 		sdc_set_brightness(mx3fb, mx3fb->backlight_level);
+		if (mx3_fbi->reg_lcd) {
+			if (!regulator_is_enabled(mx3_fbi->reg_lcd)) {
+				ret = regulator_enable(mx3_fbi->reg_lcd);
+				if (ret)
+					dev_warn(&pdev->dev,
+						 "lcd regulator enable failed with error: %d\n",
+						 ret);
+			}
+		}
 	}
 
 	console_lock();
@@ -1373,6 +1413,7 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
 	struct device_node *np = dev->of_node;
 	const char *name;
+	const char *regulator_name;
 	const char *ipu_disp_format;
 	unsigned int irq;
 	struct fb_info *fbi;
@@ -1395,6 +1436,9 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 			return -EINVAL;
 		}
 
+		of_property_read_string(display_np, "regulator-name",
+					&regulator_name);
+
 		of_property_read_string(display_np, "interface-pix-fmt",
 					&ipu_disp_format);
 		if (!ipu_disp_format) {
@@ -1509,6 +1553,24 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	if (ret < 0)
 		goto erfb;
 
+	/* In dt mode,
+	 * using devm_regulator_get would require that the proprety referencing
+	 * the regulator phandle has to be inside the mx3fb node.
+	 */
+	if (np) {
+		if (regulator_name)
+			mx3fbi->reg_lcd = regulator_get(NULL, regulator_name);
+	} else {
+		mx3fbi->reg_lcd = devm_regulator_get(dev, "lcd");
+	}
+
+	if (IS_ERR(mx3fbi->reg_lcd)) {
+		dev_warn(mx3fb->dev, "Operating without regulator \"lcd\"\n");
+		mx3fbi->reg_lcd = NULL;
+	} else {
+		dev_info(mx3fb->dev, "Using \"lcd\" Regulator\n");
+	}
+
 	return 0;
 
 erfb:
@@ -1575,6 +1637,7 @@ static int mx3fb_probe(struct platform_device *pdev)
 	dma_cap_mask_t mask;
 	struct dma_chan *chan;
 	struct dma_chan_request rq;
+	struct mx3fb_info *mx3_fbi;
 
 	/*
 	 * Display Interface (DI) and Synchronous Display Controller (SDC)
@@ -1630,6 +1693,8 @@ ersdc0:
 	dmaengine_put();
 	iounmap(mx3fb->reg_base);
 eremap:
+	mx3_fbi = mx3fb->fbi->par;
+	regulator_put(mx3_fbi->reg_lcd);
 	kfree(mx3fb);
 	dev_err(dev, "mx3fb: failed to register fb\n");
 	return ret;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCHv3][ 5/5] ARM: dts: mbimxsd35 Add video and displays support.
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1382532229-32755-1-git-send-email-denis@eukrea.com>

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Eric Bénard <eric@eukrea.com>

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v3:
- The dts were adapted to the new DT bindings which looks more like the IPUv3
  ones.
---
 .../imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts  |   61 ++++++++++++++++++++
 .../imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts  |   51 ++++++++++++++++
 .../imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts   |   51 ++++++++++++++++
 3 files changed, 163 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts

diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts
new file mode 100644
index 0000000..6d186ca
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts
@@ -0,0 +1,61 @@
+/*
+ * Copyright 2013 Eukréa Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include "imx35-eukrea-mbimxsd35-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD35 with the CMO-QVGA Display";
+	compatible = "eukrea,mbimxsd35-baseboard-cmo-qvga", "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
+
+	display@di0 {
+		regulator-name = "lcd";
+		interface-pix-fmt = "rgb666";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_lcdc_1>;
+		model = "CMO-QVGA";
+		display-timings {
+			qvga_timings: 320x240 {
+				clock-frequency = <6500000>;
+				hactive = <320>;
+				vactive = <240>;
+				hback-porch = <68>;
+				hfront-porch = <20>;
+				vback-porch = <15>;
+				vfront-porch = <4>;
+				hsync-len = <30>;
+				vsync-len = <3>;
+			};
+		};
+	};
+
+	reg_lcd_3v3: lcd-en {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_reg_lcd_3v3>;
+		regulator-name = "lcd";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&gpio1 4 0>;
+		enable-active-high;
+	};
+};
+
+&lcdc {
+	lcd-supply = <&reg_lcd_3v3>;
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts
new file mode 100644
index 0000000..ccf5f48
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2013 Eukréa Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include "imx35-eukrea-mbimxsd35-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD35 with the DVI-SVGA Display";
+	compatible = "eukrea,mbimxsd35-baseboard-dvi-svga", "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
+	display@di0 {
+		interface-pix-fmt = "rgb666";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_lcdc_1>;
+		model = "DVI-SVGA";
+		display-timings {
+			svga_timings: 800x600 {
+				clock-frequency = <40000000>;
+				hactive = <800>;
+				vactive = <600>;
+				hback-porch = <75>;
+				hfront-porch = <75>;
+				vback-porch = <7>;
+				vfront-porch = <75>;
+				hsync-len = <7>;
+				vsync-len = <7>;
+				hsync-active = <1>;
+				vsync-active = <1>;
+				de-active = <1>;
+				pixelclk-active = <0>;
+			};
+		};
+	};
+};
+
+&lcdc {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts
new file mode 100644
index 0000000..6e31684
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2013 Eukréa Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include "imx35-eukrea-mbimxsd35-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD35 with the DVI-VGA Display";
+	compatible = "eukrea,mbimxsd35-baseboard-dvi-vga", "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
+	display@di0 {
+		interface-pix-fmt = "rgb666";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_lcdc_1>;
+		model = "DVI-VGA";
+		display-timings {
+			vga_timings: 640x480 {
+				clock-frequency = <31250000>;
+				hactive = <640>;
+				vactive = <480>;
+				hback-porch = <100>;
+				hfront-porch = <100>;
+				vback-porch = <7>;
+				vfront-porch = <100>;
+				hsync-len = <7>;
+				vsync-len = <7>;
+				hsync-active = <1>;
+				vsync-active = <1>;
+				de-active = <1>;
+				pixelclk-active = <0>;
+			};
+		};
+	};
+};
+
+&lcdc {
+	status = "okay";
+};
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH] drm/omap: fix (un)registering irqs inside an irq handler
From: Tomi Valkeinen @ 2013-10-24  6:50 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-fbdev, Tomi Valkeinen

omapdrm (un)registers irqs inside an irq handler. The problem is that
the (un)register function uses dispc_runtime_get/put() to enable the
clocks, and those functions are not irq safe by default.

This was kind of fixed in 48664b21aeeffb40c5fa06843f18052e2c4ec9ef
(OMAPDSS: DISPC: set irq_safe for runtime PM), which makes dispc's
runtime calls irq-safe.

However, using pm_runtime_irq_safe in dispc makes the parent of dispc,
dss, always enabled, effectively preventing PM for the whole DSS module.

This patch makes omapdrm behave better by adding new irq (un)register
functions that do not use dispc_runtime_get/put, and using those
functions in interrupt context. Thus we can make dispc again
non-irq-safe, allowing proper PM.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c |  6 +++---
 drivers/gpu/drm/omapdrm/omap_drv.h  |  2 ++
 drivers/gpu/drm/omapdrm/omap_irq.c  | 22 ++++++++++++++++++----
 drivers/video/omap2/dss/dispc.c     |  1 -
 4 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 0fd2eb1..e6241c2 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -411,7 +411,7 @@ static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 	struct drm_crtc *crtc = &omap_crtc->base;
 	DRM_ERROR("%s: errors: %08x\n", omap_crtc->name, irqstatus);
 	/* avoid getting in a flood, unregister the irq until next vblank */
-	omap_irq_unregister(crtc->dev, &omap_crtc->error_irq);
+	__omap_irq_unregister(crtc->dev, &omap_crtc->error_irq);
 }
 
 static void omap_crtc_apply_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
@@ -421,13 +421,13 @@ static void omap_crtc_apply_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 	struct drm_crtc *crtc = &omap_crtc->base;
 
 	if (!omap_crtc->error_irq.registered)
-		omap_irq_register(crtc->dev, &omap_crtc->error_irq);
+		__omap_irq_register(crtc->dev, &omap_crtc->error_irq);
 
 	if (!dispc_mgr_go_busy(omap_crtc->channel)) {
 		struct omap_drm_private *priv  				crtc->dev->dev_private;
 		DBG("%s: apply done", omap_crtc->name);
-		omap_irq_unregister(crtc->dev, &omap_crtc->apply_irq);
+		__omap_irq_unregister(crtc->dev, &omap_crtc->apply_irq);
 		queue_work(priv->wq, &omap_crtc->apply_work);
 	}
 }
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 30b95b7..cfb6c2e 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -145,6 +145,8 @@ irqreturn_t omap_irq_handler(DRM_IRQ_ARGS);
 void omap_irq_preinstall(struct drm_device *dev);
 int omap_irq_postinstall(struct drm_device *dev);
 void omap_irq_uninstall(struct drm_device *dev);
+void __omap_irq_register(struct drm_device *dev, struct omap_drm_irq *irq);
+void __omap_irq_unregister(struct drm_device *dev, struct omap_drm_irq *irq);
 void omap_irq_register(struct drm_device *dev, struct omap_drm_irq *irq);
 void omap_irq_unregister(struct drm_device *dev, struct omap_drm_irq *irq);
 int omap_drm_irq_uninstall(struct drm_device *dev);
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
index 9263db1..b08b902 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -45,12 +45,11 @@ static void omap_irq_update(struct drm_device *dev)
 	dispc_read_irqenable();        /* flush posted write */
 }
 
-void omap_irq_register(struct drm_device *dev, struct omap_drm_irq *irq)
+void __omap_irq_register(struct drm_device *dev, struct omap_drm_irq *irq)
 {
 	struct omap_drm_private *priv = dev->dev_private;
 	unsigned long flags;
 
-	dispc_runtime_get();
 	spin_lock_irqsave(&list_lock, flags);
 
 	if (!WARN_ON(irq->registered)) {
@@ -60,14 +59,21 @@ void omap_irq_register(struct drm_device *dev, struct omap_drm_irq *irq)
 	}
 
 	spin_unlock_irqrestore(&list_lock, flags);
+}
+
+void omap_irq_register(struct drm_device *dev, struct omap_drm_irq *irq)
+{
+	dispc_runtime_get();
+
+	__omap_irq_register(dev, irq);
+
 	dispc_runtime_put();
 }
 
-void omap_irq_unregister(struct drm_device *dev, struct omap_drm_irq *irq)
+void __omap_irq_unregister(struct drm_device *dev, struct omap_drm_irq *irq)
 {
 	unsigned long flags;
 
-	dispc_runtime_get();
 	spin_lock_irqsave(&list_lock, flags);
 
 	if (!WARN_ON(!irq->registered)) {
@@ -77,6 +83,14 @@ void omap_irq_unregister(struct drm_device *dev, struct omap_drm_irq *irq)
 	}
 
 	spin_unlock_irqrestore(&list_lock, flags);
+}
+
+void omap_irq_unregister(struct drm_device *dev, struct omap_drm_irq *irq)
+{
+	dispc_runtime_get();
+
+	__omap_irq_unregister(dev, irq);
+
 	dispc_runtime_put();
 }
 
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 4779750..02a7340 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -3691,7 +3691,6 @@ static int __init omap_dispchw_probe(struct platform_device *pdev)
 	}
 
 	pm_runtime_enable(&pdev->dev);
-	pm_runtime_irq_safe(&pdev->dev);
 
 	r = dispc_runtime_get();
 	if (r)
-- 
1.8.1.2


^ permalink raw reply related

* [PATCHv6][ 1/7] video: imxfb: Introduce regulator support.
From: Denis Carikli @ 2013-10-24  8:00 UTC (permalink / raw)
  To: linux-arm-kernel

This commit is based on the following commit by Fabio Estevam:
  4344429 video: mxsfb: Introduce regulator support

Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
 drivers/video/imxfb.c |   29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 44ee678..4bf3837 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -28,6 +28,7 @@
 #include <linux/cpufreq.h>
 #include <linux/clk.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <linux/dma-mapping.h>
 #include <linux/io.h>
 #include <linux/math64.h>
@@ -145,6 +146,7 @@ struct imxfb_info {
 	struct clk		*clk_ipg;
 	struct clk		*clk_ahb;
 	struct clk		*clk_per;
+	struct regulator	*reg_lcd;
 	enum imxfb_type		devtype;
 	bool			enabled;
 
@@ -563,12 +565,23 @@ static void imxfb_exit_backlight(struct imxfb_info *fbi)
 
 static void imxfb_enable_controller(struct imxfb_info *fbi)
 {
+	int ret;
 
 	if (fbi->enabled)
 		return;
 
 	pr_debug("Enabling LCD controller\n");
 
+	if (fbi->reg_lcd) {
+		ret = regulator_enable(fbi->reg_lcd);
+		if (ret) {
+			dev_err(&fbi->pdev->dev,
+				"lcd regulator enable failed with error: %d\n",
+				ret);
+			return;
+		}
+	}
+
 	writel(fbi->screen_dma, fbi->regs + LCDC_SSA);
 
 	/* panning offset 0 (0 pixel offset)        */
@@ -597,6 +610,8 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
 
 static void imxfb_disable_controller(struct imxfb_info *fbi)
 {
+	int ret;
+
 	if (!fbi->enabled)
 		return;
 
@@ -613,6 +628,14 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
 	fbi->enabled = false;
 
 	writel(0, fbi->regs + LCDC_RMCR);
+
+	if (fbi->reg_lcd) {
+		ret = regulator_disable(fbi->reg_lcd);
+		if (ret)
+			dev_err(&fbi->pdev->dev,
+				"lcd regulator disable failed with error: %d\n",
+				ret);
+	}
 }
 
 static int imxfb_blank(int blank, struct fb_info *info)
@@ -1020,6 +1043,12 @@ static int imxfb_probe(struct platform_device *pdev)
 		goto failed_register;
 	}
 
+	fbi->reg_lcd = devm_regulator_get(&pdev->dev, "lcd");
+	if (IS_ERR(fbi->reg_lcd)) {
+		dev_info(&pdev->dev, "No lcd regulator used.\n");
+		fbi->reg_lcd = NULL;
+	}
+
 	imxfb_enable_controller(fbi);
 	fbi->pdev = pdev;
 #ifdef PWMR_BACKLIGHT_AVAILABLE
-- 
1.7.9.5


^ permalink raw reply related

* [PATCHv6][ 2/7] video: imxfb: Also add pwmr for the device tree.
From: Denis Carikli @ 2013-10-24  8:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1382601665-6830-1-git-send-email-denis@eukrea.com>

pwmr has to be set to get the imxfb backlight work,
though pwmr was only configurable trough the platform data.

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 .../devicetree/bindings/video/fsl,imx-fb.txt       |    3 +++
 drivers/video/imxfb.c                              |    2 ++
 2 files changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
index 46da08d..ac457ae 100644
--- a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
+++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
@@ -17,6 +17,9 @@ Required nodes:
 Optional properties:
 - fsl,dmacr: DMA Control Register value. This is optional. By default, the
 	register is not modified as recommended by the datasheet.
+- fsl,pwmr:  LCDC PWM Contrast Control Register value. That property is
+	optional, but defining it is necessary to get the backlight working. If that
+	property is ommited, the register is zeroed.
 - fsl,lscr1: LCDC Sharp Configuration Register value.
 
 Example:
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 4bf3837..f09372d 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -833,6 +833,8 @@ static int imxfb_init_fbinfo(struct platform_device *pdev)
 
 		of_property_read_u32(np, "fsl,dmacr", &fbi->dmacr);
 
+		of_property_read_u32(np, "fsl,pwmr", &fbi->pwmr);
+
 		/* These two function pointers could be used by some specific
 		 * platforms. */
 		fbi->lcd_power = NULL;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCHv7][ 1/6] video: imxfb: Introduce regulator support.
From: Denis Carikli @ 2013-10-24  8:58 UTC (permalink / raw)
  To: linux-arm-kernel

This commit is based on the following commit by Fabio Estevam:
  4344429 video: mxsfb: Introduce regulator support

Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
 drivers/video/imxfb.c |   29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 44ee678..4bf3837 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -28,6 +28,7 @@
 #include <linux/cpufreq.h>
 #include <linux/clk.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <linux/dma-mapping.h>
 #include <linux/io.h>
 #include <linux/math64.h>
@@ -145,6 +146,7 @@ struct imxfb_info {
 	struct clk		*clk_ipg;
 	struct clk		*clk_ahb;
 	struct clk		*clk_per;
+	struct regulator	*reg_lcd;
 	enum imxfb_type		devtype;
 	bool			enabled;
 
@@ -563,12 +565,23 @@ static void imxfb_exit_backlight(struct imxfb_info *fbi)
 
 static void imxfb_enable_controller(struct imxfb_info *fbi)
 {
+	int ret;
 
 	if (fbi->enabled)
 		return;
 
 	pr_debug("Enabling LCD controller\n");
 
+	if (fbi->reg_lcd) {
+		ret = regulator_enable(fbi->reg_lcd);
+		if (ret) {
+			dev_err(&fbi->pdev->dev,
+				"lcd regulator enable failed with error: %d\n",
+				ret);
+			return;
+		}
+	}
+
 	writel(fbi->screen_dma, fbi->regs + LCDC_SSA);
 
 	/* panning offset 0 (0 pixel offset)        */
@@ -597,6 +610,8 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
 
 static void imxfb_disable_controller(struct imxfb_info *fbi)
 {
+	int ret;
+
 	if (!fbi->enabled)
 		return;
 
@@ -613,6 +628,14 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
 	fbi->enabled = false;
 
 	writel(0, fbi->regs + LCDC_RMCR);
+
+	if (fbi->reg_lcd) {
+		ret = regulator_disable(fbi->reg_lcd);
+		if (ret)
+			dev_err(&fbi->pdev->dev,
+				"lcd regulator disable failed with error: %d\n",
+				ret);
+	}
 }
 
 static int imxfb_blank(int blank, struct fb_info *info)
@@ -1020,6 +1043,12 @@ static int imxfb_probe(struct platform_device *pdev)
 		goto failed_register;
 	}
 
+	fbi->reg_lcd = devm_regulator_get(&pdev->dev, "lcd");
+	if (IS_ERR(fbi->reg_lcd)) {
+		dev_info(&pdev->dev, "No lcd regulator used.\n");
+		fbi->reg_lcd = NULL;
+	}
+
 	imxfb_enable_controller(fbi);
 	fbi->pdev = pdev;
 #ifdef PWMR_BACKLIGHT_AVAILABLE
-- 
1.7.9.5


^ permalink raw reply related

* [PATCHv7][ 2/6] video: imxfb: Also add pwmr for the device tree.
From: Denis Carikli @ 2013-10-24  8:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1382605103-9595-1-git-send-email-denis@eukrea.com>

pwmr has to be set to get the imxfb backlight work,
though pwmr was only configurable trough the platform data.

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 .../devicetree/bindings/video/fsl,imx-fb.txt       |    3 +++
 drivers/video/imxfb.c                              |    2 ++
 2 files changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
index 46da08d..ac457ae 100644
--- a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
+++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
@@ -17,6 +17,9 @@ Required nodes:
 Optional properties:
 - fsl,dmacr: DMA Control Register value. This is optional. By default, the
 	register is not modified as recommended by the datasheet.
+- fsl,pwmr:  LCDC PWM Contrast Control Register value. That property is
+	optional, but defining it is necessary to get the backlight working. If that
+	property is ommited, the register is zeroed.
 - fsl,lscr1: LCDC Sharp Configuration Register value.
 
 Example:
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 4bf3837..f09372d 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -833,6 +833,8 @@ static int imxfb_init_fbinfo(struct platform_device *pdev)
 
 		of_property_read_u32(np, "fsl,dmacr", &fbi->dmacr);
 
+		of_property_read_u32(np, "fsl,pwmr", &fbi->pwmr);
+
 		/* These two function pointers could be used by some specific
 		 * platforms. */
 		fbi->lcd_power = NULL;
-- 
1.7.9.5


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox