All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Courbot <acourbot@nvidia.com>
To: Thierry Reding <thierry.reding@avionic-design.de>
Cc: Stephen Warren <swarren@wwwdotorg.org>,
	"linux-fbdev@vger.kernel.org" <linux-fbdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	Mark Zhang <markz@nvidia.com>,
	"gnurou@gmail.com" <gnurou@gmail.com>
Subject: Re: [PATCH 2/3] tegra: pwm-backlight: add tegra pwm-bl driver
Date: Wed, 23 Jan 2013 09:45:39 +0000	[thread overview]
Message-ID: <2178053.V0UsGT5PW9@percival> (raw)
In-Reply-To: <20130122070630.GA14728@avionic-0098.adnet.avionic-design.de>

> I'm confused. Why would you want to call into pwm_bl directly? If we're
> going to split this up into separate platform devices, why not look up a
> given backlight device and use the backlight API on that? The pieces of
> the puzzle are all there: you can use of_find_backlight_by_node() to
> obtain a backlight device from a device tree node, so I'd expect the DT
> to look something like this:
> 
> 	backlight: backlight {
> 		compatible = "pwm-backlight";
> 		...
> 	};

This would still prevent any power control from the backlight driver. I.e. if 
someone sets the brightness to 0 through sysfs, we cannot power the backlight 
off as pwm-backlight cannot control more than the PWM without platform 
callbacks. Backlight could only be powered off as a result of a fb blank event.

> 	panel: panel {
> 		compatible = "...";
> 		...
> 		backlight = <&backlight>;
> 		...
> 	};

So all the power control of both the panel and backlight would be performed 
from this device's driver. How would it plug into tegra-drm? I would see 
tegra_panel as a new member of the tegra_output structure, with one callback 
invoked from tegra_encoder_dpms(). Does that look sane?

> After that you can wire it up with host1x using something like:
> 
> 	host1x {
> 		dc@54200000 {
> 			rgb {
> 				status = "okay";
> 
> 				nvidia,panel = <&panel>;
> 			};
> 		};
> 	};

Indeed. So if we do that, the DRM DPMS functions would take care of the 
panel/backlight powering and the backlight driver will control the PWM after 
this, through the FB notifier. This is a little bit different from the "official" 
power sequence, but I just tested controlling the PWM at the very end of the 
sequence and it works just as well. If you think this looks better I don't 
mind doing it that way, it is actually a good excuse for me to dive into the 
DRM code.

Anyway, this will only be a temporary solution, CDF is the only way to do this 
right.

Alex.


WARNING: multiple messages have this Message-ID (diff)
From: Alex Courbot <acourbot@nvidia.com>
To: Thierry Reding <thierry.reding@avionic-design.de>
Cc: Stephen Warren <swarren@wwwdotorg.org>,
	"linux-fbdev@vger.kernel.org" <linux-fbdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	Mark Zhang <markz@nvidia.com>,
	"gnurou@gmail.com" <gnurou@gmail.com>
Subject: Re: [PATCH 2/3] tegra: pwm-backlight: add tegra pwm-bl driver
Date: Wed, 23 Jan 2013 18:45:39 +0900	[thread overview]
Message-ID: <2178053.V0UsGT5PW9@percival> (raw)
In-Reply-To: <20130122070630.GA14728@avionic-0098.adnet.avionic-design.de>

> I'm confused. Why would you want to call into pwm_bl directly? If we're
> going to split this up into separate platform devices, why not look up a
> given backlight device and use the backlight API on that? The pieces of
> the puzzle are all there: you can use of_find_backlight_by_node() to
> obtain a backlight device from a device tree node, so I'd expect the DT
> to look something like this:
> 
> 	backlight: backlight {
> 		compatible = "pwm-backlight";
> 		...
> 	};

This would still prevent any power control from the backlight driver. I.e. if 
someone sets the brightness to 0 through sysfs, we cannot power the backlight 
off as pwm-backlight cannot control more than the PWM without platform 
callbacks. Backlight could only be powered off as a result of a fb blank event.

> 	panel: panel {
> 		compatible = "...";
> 		...
> 		backlight = <&backlight>;
> 		...
> 	};

So all the power control of both the panel and backlight would be performed 
from this device's driver. How would it plug into tegra-drm? I would see 
tegra_panel as a new member of the tegra_output structure, with one callback 
invoked from tegra_encoder_dpms(). Does that look sane?

> After that you can wire it up with host1x using something like:
> 
> 	host1x {
> 		dc@54200000 {
> 			rgb {
> 				status = "okay";
> 
> 				nvidia,panel = <&panel>;
> 			};
> 		};
> 	};

Indeed. So if we do that, the DRM DPMS functions would take care of the 
panel/backlight powering and the backlight driver will control the PWM after 
this, through the FB notifier. This is a little bit different from the "official" 
power sequence, but I just tested controlling the PWM at the very end of the 
sequence and it works just as well. If you think this looks better I don't 
mind doing it that way, it is actually a good excuse for me to dive into the 
DRM code.

Anyway, this will only be a temporary solution, CDF is the only way to do this 
right.

Alex.

  reply	other threads:[~2013-01-23  9:45 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-19 10:30 [PATCH 0/3] pwm-backlight: add subdrivers & Tegra support Alexandre Courbot
2013-01-19 10:30 ` Alexandre Courbot
2013-01-19 10:30 ` [PATCH 1/3] pwm-backlight: add subdriver mechanism Alexandre Courbot
2013-01-19 10:30   ` Alexandre Courbot
2013-01-19 10:30 ` [PATCH 2/3] tegra: pwm-backlight: add tegra pwm-bl driver Alexandre Courbot
2013-01-19 10:30   ` Alexandre Courbot
     [not found]   ` <1358591420-7790-3-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-01-21  7:35     ` Mark Zhang
2013-01-21  7:35       ` Mark Zhang
2013-01-21  7:35       ` Mark Zhang
2013-01-21  8:24       ` Alex Courbot
2013-01-21  8:24         ` Alex Courbot
2013-01-21  8:35         ` Mark Zhang
2013-01-21  8:35           ` Mark Zhang
2013-01-21  8:35           ` Mark Zhang
     [not found]       ` <50FCEFDE.8000705-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-01-21  8:52         ` Marc Dietrich
2013-01-21  8:52           ` Marc Dietrich
2013-01-21  8:52           ` Marc Dietrich
2013-01-21  8:55           ` Mark Zhang
2013-01-21  8:55             ` Mark Zhang
2013-01-21 17:46     ` Stephen Warren
2013-01-21 17:46       ` Stephen Warren
2013-01-21 17:46       ` Stephen Warren
     [not found]       ` <50FD7EF9.1010205-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-01-22  3:24         ` Alex Courbot
2013-01-22  3:24           ` Alex Courbot
2013-01-22  3:24           ` Alex Courbot
2013-01-22  7:06           ` Thierry Reding
2013-01-22  7:06             ` Thierry Reding
2013-01-23  9:45             ` Alex Courbot [this message]
2013-01-23  9:45               ` Alex Courbot
2013-01-24  6:10               ` Alex Courbot
2013-01-24  6:10                 ` Alex Courbot
2013-01-24  6:10                 ` Alex Courbot
2013-01-22 16:30           ` Stephen Warren
2013-01-22 16:30             ` Stephen Warren
2013-01-22 16:30             ` Stephen Warren
2013-01-23 10:15     ` Leela Krishna Amudala
2013-01-23 10:27       ` Leela Krishna Amudala
2013-01-23 10:15       ` Leela Krishna Amudala
     [not found]       ` <CAL1wa8d2BS3RxdsdUyCqF20ZKe46jUZcfUKitnpP9Lgb9aB5hw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-01-23 10:29         ` Alex Courbot
2013-01-23 10:29           ` Alex Courbot
2013-01-23 10:29           ` Alex Courbot
     [not found] ` <1358591420-7790-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-01-19 10:30   ` [PATCH 3/3] tegra: ventana: of: add host1x device to DT Alexandre Courbot
2013-01-19 10:30     ` Alexandre Courbot
2013-01-19 10:30     ` Alexandre Courbot
2013-01-20  3:38   ` [PATCH 0/3] pwm-backlight: add subdrivers & Tegra support Mark Zhang
2013-01-20  3:38     ` Mark Zhang
2013-01-20  3:38     ` Mark Zhang
2013-01-20  5:26     ` Alexandre Courbot
2013-01-20  5:26       ` Alexandre Courbot
2013-01-20  5:51       ` Mark Zhang
2013-01-20  5:51         ` Mark Zhang
2013-01-21  2:09   ` Mark Zhang
2013-01-21  2:09     ` Mark Zhang
2013-01-21  2:09     ` Mark Zhang
     [not found]     ` <50FCA346.2070608-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-01-21  2:59       ` Mark Zhang
2013-01-21  2:59         ` Mark Zhang
2013-01-21  2:59         ` Mark Zhang
2013-01-21  7:49 ` Thierry Reding
2013-01-21  7:49   ` Thierry Reding
     [not found]   ` <20130121074928.GE15508-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org>
2013-01-21  8:18     ` Alex Courbot
2013-01-21  8:18       ` Alex Courbot
2013-01-21  8:18       ` Alex Courbot
2013-01-22  7:17       ` Thierry Reding
2013-01-22  7:17         ` Thierry Reding
2013-01-22  7:17         ` Thierry Reding

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2178053.V0UsGT5PW9@percival \
    --to=acourbot@nvidia.com \
    --cc=gnurou@gmail.com \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=markz@nvidia.com \
    --cc=swarren@wwwdotorg.org \
    --cc=thierry.reding@avionic-design.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.