How to set fops in "struct platform_pwm_backlight_data"?

DISCUSSION Mark Zhang 2 participants 19 messages Oct 17, 2013 – Oct 23, 2013

Subsystems (2)

Framebuffer layer PWM

lore

Share & tools

lore

Mark Zhang

author

2013-10-17 06:49 UTC #0 lore raw

Hi,

This is the first time I send mail to linux-pwm, I didn't read through the mails in this list, so if somebody already asked this question, I'm sorry about that.

I wanna set some fops in "struct platform_pwm_backlight_data". But the currrent probe function in pwm_bl.c says:

-------

if (!data) {
	ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
	if (ret < 0) {
		dev_err(&pdev->dev, "failed to find platform data\n");
		return ret;
	}

	data = &defdata;
}

-------

This looks like if we set the platform data for pwm backlight device, "pwm_backlight_parse_dt" will never have a chance to be called, which means the stuffs I defined in backlight DT node will be ignored.

If I don't set the platform data for pwm backlight device, according to the pwm_backlight_probe, I will never have a chance to set some fops which I need(like "notify", "check_fb"...).

So, what I suppose to do now? Maybe there is a way to set function pointers in DT?

Mark

Thierry Reding

2013-10-17 07:14 UTC #1 lore raw

Quoted 25 lines · Mark Zhang

Hi,
This is the first time I send mail to linux-pwm, I didn't read through the mails in this list, so if somebody already asked this question, I'm sorry about that.
I wanna set some fops in "struct platform_pwm_backlight_data". But the currrent probe function in pwm_bl.c says:
-------
if (!data) {
	ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
	if (ret < 0) {
		dev_err(&pdev->dev, "failed to find platform data\n");
		return ret;
	}

	data = &defdata;
}
-------
This looks like if we set the platform data for pwm backlight device, "pwm_backlight_parse_dt" will never have a chance to be called, which means the stuffs I defined in backlight DT node will be ignored.

Quoted 6 lines

If I don't set the platform data for pwm backlight device, according to the pwm_backlight_probe, I will never have a chance to set some fops which I need(like "notify", "check_fb"...).
So, what I suppose to do now? Maybe there is a way to set function pointers in DT?

Perhaps you could describe in more detail what you need the functions for.

Generally you're not supposed to mix DT and platform data. Without more info that's about all I can say.

Thierry

Mark Zhang

author

2013-10-18 04:48 UTC #2 lore raw

Quoted 30 lines · Thierry Reding

Quoted 29 lines · Mark Zhang
Hi,
This is the first time I send mail to linux-pwm, I didn't read through the mails in this list, so if somebody already asked this question, I'm sorry about that.
I wanna set some fops in "struct platform_pwm_backlight_data". But the currrent probe function in pwm_bl.c says:
-------
if (!data) {
	ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
	if (ret < 0) {
		dev_err(&pdev->dev, "failed to find platform data\n");
		return ret;
	}

	data = &defdata;
}
-------
This looks like if we set the platform data for pwm backlight device, "pwm_backlight_parse_dt" will never have a chance to be called, which means the stuffs I defined in backlight DT node will be ignored.
If I don't set the platform data for pwm backlight device, according to the pwm_backlight_probe, I will never have a chance to set some fops which I need(like "notify", "check_fb"...).

Quoted 6 lines

Quoted 2 lines
So, what I suppose to do now? Maybe there is a way to set function pointers in DT?
Perhaps you could describe in more detail what you need the functions for.

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.

Mark

Quoted 5 lines

Generally you're not supposed to mix DT and platform data. Without more info that's about all I can say.
Thierry

Mark Zhang

author

2013-10-22 02:41 UTC #3 lore raw

Ping Thierry...

Mark

Quoted 49 lines · Mark Zhang

Quoted 30 lines · Thierry Reding
Quoted 29 lines · Mark Zhang
Hi,
This is the first time I send mail to linux-pwm, I didn't read through the mails in this list, so if somebody already asked this question, I'm sorry about that.
I wanna set some fops in "struct platform_pwm_backlight_data". But the currrent probe function in pwm_bl.c says:
-------
if (!data) {
	ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
	if (ret < 0) {
		dev_err(&pdev->dev, "failed to find platform data\n");
		return ret;
	}

	data = &defdata;
}
-------
This looks like if we set the platform data for pwm backlight device, "pwm_backlight_parse_dt" will never have a chance to be called, which means the stuffs I defined in backlight DT node will be ignored.
If I don't set the platform data for pwm backlight device, according to the pwm_backlight_probe, I will never have a chance to set some fops which I need(like "notify", "check_fb"...).
Quoted 6 lines
Quoted 2 lines
So, what I suppose to do now? Maybe there is a way to set function pointers in DT?
Perhaps you could describe in more detail what you need the functions for.
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.
Mark
Quoted 5 lines
Generally you're not supposed to mix DT and platform data. Without more info that's about all I can say.
Thierry

Thierry Reding

2013-10-22 07:24 UTC #4 lore raw

In reply to Mark Zhang

Quoted 34 lines · Mark Zhang

Quoted 33 lines · Thierry Reding
Quoted 31 lines · Mark Zhang
Hi,
This is the first time I send mail to linux-pwm, I didn't read through the mails in this list, so if somebody already asked this question, I'm sorry about that.
I wanna set some fops in "struct platform_pwm_backlight_data". But the currrent probe function in pwm_bl.c says:
-------
if (!data) {
	ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
	if (ret < 0) {
		dev_err(&pdev->dev, "failed to find platform data\n");
		return ret;
	}

	data = &defdata;
}
-------
This looks like if we set the platform data for pwm backlight device, "pwm_backlight_parse_dt" will never have a chance to be called, which means the stuffs I defined in backlight DT node will be ignored.
If I don't set the platform data for pwm backlight device, according to the pwm_backlight_probe, I will never have a chance to set some fops which I need(like "notify", "check_fb"...).
So, what I suppose to do now? Maybe there is a way to set function pointers in DT?

Quoted 8 lines

Quoted 3 lines
Perhaps you could describe in more detail what you need the functions for.
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.

Thierry

Mark Zhang

author

2013-10-22 08:55 UTC #5 lore raw

Quoted 1 line · Thierry Reding

On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:

[...]

Quoted 10 lines

Quoted 6 lines
Quoted 1 line
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".

Mark

Quoted 2 lines

Thierry

Thierry Reding

2013-10-22 12:49 UTC #6 lore raw

Quoted 10 lines · Mark Zhang

Quoted 1 line · Thierry Reding
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
[...]
Quoted 7 lines
Quoted 6 lines
Quoted 1 line
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.

Quoted 7 lines

Quoted 3 lines
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?

Thierry

Mark Zhang

author

2013-10-23 02:16 UTC #7 lore raw

Quoted 15 lines · Thierry Reding

Quoted 14 lines · Mark Zhang
Quoted 1 line · Thierry Reding
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
[...]
Quoted 10 lines
Quoted 6 lines
Quoted 1 line
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.

Quoted 7 lines

Quoted 3 lines
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.

Mark

Quoted 2 lines

Thierry

Thierry Reding

2013-10-23 08:00 UTC #8 lore raw

Quoted 24 lines · Mark Zhang

Quoted 15 lines · Thierry Reding
Quoted 14 lines · Mark Zhang
Quoted 1 line · Thierry Reding
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
[...]
Quoted 10 lines
Quoted 6 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 3 lines
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?

Quoted 6 lines

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?

Thierry

Mark Zhang

author

2013-10-23 08:49 UTC #9 lore raw

Quoted 32 lines · Thierry Reding

Quoted 24 lines · Mark Zhang
Quoted 15 lines · Thierry Reding
Quoted 14 lines · Mark Zhang
Quoted 1 line · Thierry Reding
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
[...]
Quoted 10 lines
Quoted 6 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 3 lines
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?
Quoted 6 lines
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.

Quoted 4 lines

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"?

Mark

Quoted 3 lines

Thierry

Thierry Reding

2013-10-23 09:09 UTC #10 lore raw

Quoted 33 lines · Mark Zhang

Quoted 32 lines · Thierry Reding
Quoted 30 lines · Mark Zhang
Quoted 15 lines · Thierry Reding
Quoted 14 lines · Mark Zhang
Quoted 1 line · Thierry Reding
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
[...]
Quoted 10 lines
Quoted 6 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 3 lines
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.

Quoted 6 lines

Quoted 4 lines
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().

Thierry

Mark Zhang

author

2013-10-23 10:31 UTC #11 lore raw

Quoted 41 lines · Thierry Reding

Quoted 33 lines · Mark Zhang
Quoted 32 lines · Thierry Reding
Quoted 30 lines · Mark Zhang
Quoted 15 lines · Thierry Reding
Quoted 14 lines · Mark Zhang
Quoted 1 line · Thierry Reding
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
[...]
Quoted 10 lines
Quoted 6 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 4 lines
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"?

Quoted 9 lines

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

Quoted 2 lines

Thierry

Thierry Reding

2013-10-23 10:54 UTC #12 lore raw

Quoted 52 lines · Mark Zhang

Quoted 41 lines · Thierry Reding
Quoted 33 lines · Mark Zhang
Quoted 32 lines · Thierry Reding
Quoted 30 lines · Mark Zhang
Quoted 15 lines · Thierry Reding
Quoted 14 lines · Mark Zhang
Quoted 1 line · Thierry Reding
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
[...]
Quoted 10 lines
Quoted 6 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 4 lines
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"?
Quoted 9 lines
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().

Quoted 3 lines

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.

Quoted 4 lines

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

Mark Zhang

author

2013-10-23 11:33 UTC #13 lore raw

Quoted 59 lines · Thierry Reding

Quoted 52 lines · Mark Zhang
Quoted 41 lines · Thierry Reding
Quoted 33 lines · Mark Zhang
Quoted 32 lines · Thierry Reding
Quoted 30 lines · Mark Zhang
Quoted 15 lines · Thierry Reding
Quoted 14 lines · Mark Zhang
Quoted 1 line · Thierry Reding
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
[...]
Quoted 10 lines
Quoted 6 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 4 lines
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"?
Quoted 9 lines
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().
Quoted 3 lines
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.

Quoted 9 lines

Quoted 4 lines
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.

Quoted 3 lines

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

Yes.

Mark

Quoted 2 lines

Thierry

Mark Zhang

author

2013-10-23 10:36 UTC #14 lore raw

In reply to Thierry Reding

Quoted 41 lines · Thierry Reding

Quoted 33 lines · Mark Zhang
Quoted 32 lines · Thierry Reding
Quoted 30 lines · Mark Zhang
Quoted 15 lines · Thierry Reding
Quoted 14 lines · Mark Zhang
Quoted 1 line · Thierry Reding
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
[...]
Quoted 10 lines
Quoted 6 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 4 lines
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"?

Quoted 9 lines

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

Quoted 2 lines

Thierry

Mark Zhang

author

2013-10-23 10:46 UTC #15 lore raw

Quoted 52 lines · Mark Zhang

Quoted 41 lines · Thierry Reding
Quoted 33 lines · Mark Zhang
Quoted 32 lines · Thierry Reding
Quoted 30 lines · Mark Zhang
Quoted 15 lines · Thierry Reding
Quoted 14 lines · Mark Zhang
Quoted 1 line · Thierry Reding
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
[...]
Quoted 10 lines
Quoted 6 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 4 lines
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"?
Quoted 9 lines
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().

Quoted 3 lines

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

Quoted 4 lines

Mark
Quoted 2 lines
Thierry

Thierry Reding

2013-10-23 10:58 UTC #16 lore raw

Quoted 53 lines · Mark Zhang

Quoted 52 lines · Mark Zhang
Quoted 50 lines · Thierry Reding
Quoted 33 lines · Mark Zhang
Quoted 32 lines · Thierry Reding
Quoted 30 lines · Mark Zhang
Quoted 15 lines · Thierry Reding
Quoted 14 lines · Mark Zhang
Quoted 1 line · Thierry Reding
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
[...]
Quoted 10 lines
Quoted 6 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 4 lines
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().

Quoted 7 lines

Quoted 3 lines
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

Mark Zhang

author

2013-10-23 11:34 UTC #17 lore raw

Quoted 58 lines · Thierry Reding

Quoted 57 lines · Mark Zhang
Quoted 55 lines · Mark Zhang
Quoted 41 lines · Thierry Reding
Quoted 33 lines · Mark Zhang
Quoted 32 lines · Thierry Reding
Quoted 30 lines · Mark Zhang
Quoted 15 lines · Thierry Reding
Quoted 14 lines · Mark Zhang
Quoted 1 line · Thierry Reding
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
[...]
Quoted 10 lines
Quoted 6 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 4 lines
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"?
Quoted 9 lines
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?

Quoted 9 lines

Quoted 3 lines
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

Quoted 2 lines

Thierry

Thierry Reding

2013-10-23 10:51 UTC #18 lore raw

In reply to Mark Zhang

Quoted 52 lines · Mark Zhang

Quoted 41 lines · Thierry Reding
Quoted 33 lines · Mark Zhang
Quoted 32 lines · Thierry Reding
Quoted 30 lines · Mark Zhang
Quoted 15 lines · Thierry Reding
Quoted 14 lines · Mark Zhang
Quoted 1 line · Thierry Reding
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
[...]
Quoted 10 lines
Quoted 6 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 4 lines
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"?
Quoted 9 lines
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().

Quoted 3 lines

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