* Re: [PATCH] set data enable logic level to low
From: Tomi Valkeinen @ 2013-10-18 8:01 UTC (permalink / raw)
To: Roel Kluin
Cc: Jean-Christophe Plagniol-Villard, linux-omap, linux-fbdev,
linux-kernel
In-Reply-To: <1381704273-31540-1-git-send-email-roel.kluin@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 982 bytes --]
On 14/10/13 01:44, Roel Kluin wrote:
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> drivers/video/omap2/dss/display.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
> index fafe7c9..669a81f 100644
> --- a/drivers/video/omap2/dss/display.c
> +++ b/drivers/video/omap2/dss/display.c
> @@ -266,7 +266,7 @@ void videomode_to_omap_video_timings(const struct videomode *vm,
> OMAPDSS_SIG_ACTIVE_LOW;
> ovt->de_level = vm->flags & DISPLAY_FLAGS_DE_HIGH ?
> OMAPDSS_SIG_ACTIVE_HIGH :
> - OMAPDSS_SIG_ACTIVE_HIGH;
> + OMAPDSS_SIG_ACTIVE_LOW;
> ovt->data_pclk_edge = vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE ?
> OMAPDSS_DRIVE_SIG_RISING_EDGE :
> OMAPDSS_DRIVE_SIG_FALLING_EDGE;
>
Thanks. Next time, please write a patch description and prefix patch
subject with appropriate string. In this case "OMAPDSS: ..." would be fine.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH/RFC v3 00/19] Common Display Framework
From: Andrzej Hajda @ 2013-10-18 11:55 UTC (permalink / raw)
To: Tomi Valkeinen, Laurent Pinchart
Cc: linux-fbdev, dri-devel, Jesse Barnes, Benjamin Gaignard, Tom Gall,
Kyungmin Park, linux-media, Stephen Warren, Mark Zhang,
Alexandre Courbot, Ragesh Radhakrishnan, Thomas Petazzoni,
Sunil Joshi, Maxime Ripard, Vikas Sajjan, Marcus Lorentzon
In-Reply-To: <525FDE54.2070909@ti.com>
On 10/17/2013 02:55 PM, Tomi Valkeinen wrote:
> On 17/10/13 15:26, Andrzej Hajda wrote:
>
>> I am not sure what exactly the encoder performs, if this is only image
>> transport from dispc to panel CDF pipeline in both cases should look like:
>> dispc ----> panel
>> The only difference is that panels will be connected via different Linux bus
>> adapters, but it will be irrelevant to CDF itself. In this case I would say
>> this is DSI-master rather than encoder, or at least that the only
>> function of the
>> encoder is DSI.
> Yes, as I said, it's up to the driver writer how he wants to use CDF. If
> he doesn't see the point of representing the SoC's DSI encoder as a
> separate CDF entity, nobody forces him to do that.
Having it as an entity would cause the 'problem' of two APIs as you
described below :)
One API via control bus, another one via CDF.
>
> On OMAP, we have single DISPC with multiple parallel outputs, and a
> bunch of encoder IPs (MIPI DPI, DSI, DBI, etc). Each encoder IP can be
> connected to some of the DISPC's output. In this case, even if the DSI
> encoder does nothing special, I see it much better to represent the DSI
> encoder as a CDF entity so that the links between DISPC, DSI, and the
> DSI peripherals are all there.
>
>> If display_timings on input and output differs, I suppose it should be
>> modeled
>> as display_entity, as this is an additional functionality(not covered by
>> DSI standard AFAIK).
> Well, DSI standard is about the DSI output. Not about the encoder's
> input, or the internal operation of the encoder.
>
>>>> Of course there are some settings which are not panel dependent and those
>>>> should reside in DSI node.
>>> Exactly. And when the two panels require different non-panel-dependent
>>> settings, how do you represent them in the DT data?
>> non-panel-dependent setting cannot depend on panel, by definition :)
> With "non-panel-dependent" setting I meant something that is a property
> of the DSI master device, but still needs to be configured differently
> for each panel.
>
> Say, pin configuration. When using panel A, the first pin of the DSI
> block could be clock+. With panel B, the first pin could be clock-. This
> configuration is about DSI master, but it is different for each panel.
>
> If we have separate endpoint in the DSI master for each panel, this data
> can be there. If we don't have the endpoint, as is the case with
> separate control bus, where is that data?
I am open to propositions. For me it seems somehow similar to clock mapping
in DT (clock-names are mapped to provider clocks), so I think it could
be put in panel node and it will be parsed by DSI-master.
>
>>>> Could you describe such scenario?
>>> If we have two independent APIs, ctrl and video, that affect the same
>>> underlying hardware, the DSI bus, we could have a scenario like this:
>>>
>>> thread 1:
>>>
>>> ctrl->op_foo();
>>> ctrl->op_bar();
>>>
>>> thread 2:
>>>
>>> video->op_baz();
>>>
>>> Even if all those ops do locking properly internally, the fact that
>>> op_baz() can be called in between op_foo() and op_bar() may cause problems.
>>>
>>> To avoid that issue with two APIs we'd need something like:
>>>
>>> thread 1:
>>>
>>> ctrl->lock();
>>> ctrl->op_foo();
>>> ctrl->op_bar();
>>> ctrl->unlock();
>>>
>>> thread 2:
>>>
>>> video->lock();
>>> video->op_baz();
>>> video->unlock();
>> I should mention I was asking for real hw/drivers configuration.
>> I do not know what do you mean with video->op_baz() ?
>> DSI-master is not modeled in CDF, and only CDF provides video
>> operations.
> It was just an example of the additional complexity with regarding
> locking when using two APIs.
>
> The point is that if the panel driver has two pointers (i.e. API), one
> for the control bus, one for the video bus, and ops on both buses affect
> the same hardware, the locking is not easy.
>
> If, on the other hand, the panel driver only has one API to use, it's
> simple to require the caller to handle any locking.
I guess you are describing scenario with DSI-master having its own entity.
In such case its video ops are accessible at least to all pipeline
neightbourgs and
to pipeline controler, so I do not see how the client side locking would
work anyway.
Additionally multiple panels connected to one DSI also makes it harder.
Thus I do not see that 'client lock' apporach would work anyway, even
using video-source approach.
Andrzej
^ permalink raw reply
* [PATCH 02/11] video: imxfb: Introduce regulator support.
From: Denis Carikli @ 2013-10-18 15:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382108672-23269-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: 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 | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 44ee678..4dd7678 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,10 @@ 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))
+ fbi->reg_lcd = NULL;
+
imxfb_enable_controller(fbi);
fbi->pdev = pdev;
#ifdef PWMR_BACKLIGHT_AVAILABLE
--
1.7.9.5
^ permalink raw reply related
* [PATCHv3][ 02/11] video: imxfb: Introduce regulator support.
From: Denis Carikli @ 2013-10-18 15:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382109422-24454-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: 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 | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 44ee678..4dd7678 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,10 @@ 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))
+ fbi->reg_lcd = NULL;
+
imxfb_enable_controller(fbi);
fbi->pdev = pdev;
#ifdef PWMR_BACKLIGHT_AVAILABLE
--
1.7.9.5
^ permalink raw reply related
* [PATCHv3][ 03/11] video: imxfb: Also add pwmr for the device tree.
From: Denis Carikli @ 2013-10-18 15:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382109422-24454-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>
---
.../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 4dd7678..9da29b3 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
* Re: [PATCH v2] pwm-backlight: allow for non-increasing brightness levels
From: Mike Dunn @ 2013-10-18 19:53 UTC (permalink / raw)
To: Thierry Reding
Cc: Richard Purdie, Jingoo Han, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Grant Likely, Rob Herring,
linux-pwm-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Robert Jarzmik, Marek Vasut
In-Reply-To: <20131018074623.GA22291-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>
On 10/18/2013 12:46 AM, Thierry Reding wrote:
> On Sun, Sep 22, 2013 at 09:59:56AM -0700, Mike Dunn wrote:
>> Currently the driver assumes that the values specified in the
>> brightness-levels device tree property increase as they are parsed from
>> left to right. But boards that invert the signal between the PWM output
>> and the backlight will need to specify decreasing brightness-levels.
>> This patch removes the assumption that the last element of the array is
>> the maximum value, and instead searches the array for the maximum value
>> and uses that in the duty cycle calculation.
>>
>> Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
>> ---
>> Changelog:
>> v2:
>> - commit message reworded; correct line wrap used
>> - 'max_level' variable renamed to 'scale'
>> - loop counter variable type changed to unsigned int
>> - value held in scale changed from array index to actual maximum level
>> - blank lines added around loop for readability
>>
>> drivers/video/backlight/pwm_bl.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> Hey Mike,
>
> I've pushed a slightly different version of this patch which gets rid of
> the intermediate max variable and uses the new scale field exclusively
> to pass the same information around. Could you look at the patch from my
> for-next branch in the PWM tree and see whether that still works for the
> specific hardware that you need this for?
Yes looks good.
I also tested the current HEAD of for-next on the Palm Treo 680, including the
enable-gpios DT node property. I will have to do more work and investigation
before I will be able to try the power-supply property, though.
Thanks,
Mike
^ permalink raw reply
* Re: [PATCH v3] efifb: prevent null-deref when iterating dmi_list
From: David Herrmann @ 2013-10-19 10:40 UTC (permalink / raw)
To: linux-fbdev@vger.kernel.org
Cc: James Bates, linux-kernel, Tomi Valkeinen,
Jean-Christophe Plagniol-Villard, David Herrmann
In-Reply-To: <1380732219-5458-1-git-send-email-dh.herrmann@gmail.com>
ping
On Wed, Oct 2, 2013 at 6:43 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> From: James Bates <james.h.bates@gmail.com>
>
> The dmi_list array is initialized using gnu designated initializers, and
> therefore may contain fewer explicitly defined entries as there are
> elements in it. This is because the enum above with M_xyz constants
> contains more items than the designated initializer. Those elements not
> explicitly initialized are implicitly set to 0.
>
> Now efifb_setup() loops through all these array elements, and performs
> a strcmp on each item. For non explicitly initialized elements this will
> be a null pointer:
>
> This patch swaps the check order in the if statement, thus checks first
> whether dmi_list[i].base is null.
>
> Signed-off-by: James Bates <james.h.bates@gmail.com>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
> v3: fixes the "Author" field and James' email address
>
> drivers/video/efifb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
> index 7f9ff75..fcb9500 100644
> --- a/drivers/video/efifb.c
> +++ b/drivers/video/efifb.c
> @@ -108,8 +108,8 @@ static int efifb_setup(char *options)
> if (!*this_opt) continue;
>
> for (i = 0; i < M_UNKNOWN; i++) {
> - if (!strcmp(this_opt, efifb_dmi_list[i].optname) &&
> - efifb_dmi_list[i].base != 0) {
> + if (efifb_dmi_list[i].base != 0 &&
> + !strcmp(this_opt, efifb_dmi_list[i].optname)) {
> screen_info.lfb_base = efifb_dmi_list[i].base;
> screen_info.lfb_linelength = efifb_dmi_list[i].stride;
> screen_info.lfb_width = efifb_dmi_list[i].width;
> --
> 1.8.4
>
^ permalink raw reply
* Re: [PATCH 02/11] video: imxfb: Introduce regulator support.
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-19 10:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382108672-23269-2-git-send-email-denis@eukrea.com>
On 17:04 Fri 18 Oct , Denis Carikli wrote:
> 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 | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index 44ee678..4dd7678 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,10 @@ static int imxfb_probe(struct platform_device *pdev)
> goto failed_register;
> }
>
> + fbi->reg_lcd = devm_regulator_get(&pdev->dev, "lcd");
put a warning at list
otherwise looks ok
Best Regards,
J.
> + if (IS_ERR(fbi->reg_lcd))
> + fbi->reg_lcd = NULL;
> +
> imxfb_enable_controller(fbi);
> fbi->pdev = pdev;
> #ifdef PWMR_BACKLIGHT_AVAILABLE
> --
> 1.7.9.5
>
^ permalink raw reply
* Re: [PATCH] tmio: call tmiofb_set_par in tmiofb_probe
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-19 10:48 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1382003443-5044-1-git-send-email-dbaryshkov@gmail.com>
On 13:50 Thu 17 Oct , Dmitry Eremin-Solenikov wrote:
> Call tmiofb_set_par() to initlaize fixed variable before registering
> framebuffer. This is necessary for current kexecboot.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
> drivers/video/tmiofb.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/tmiofb.c b/drivers/video/tmiofb.c
> index deb8733..69fd0f7 100644
> --- a/drivers/video/tmiofb.c
> +++ b/drivers/video/tmiofb.c
> @@ -774,6 +774,10 @@ static int tmiofb_probe(struct platform_device *dev)
> if (retval)
> goto err_hw_init;
>
> + retval = tmiofb_set_par(info);
> + if (retval)
> + goto err_set_par;
> +
sorry no fix kexecboot
Best Regards,
J.
> fb_videomode_to_modelist(data->modes, data->num_modes,
> &info->modelist);
>
> @@ -787,7 +791,7 @@ static int tmiofb_probe(struct platform_device *dev)
> return 0;
>
> err_register_framebuffer:
> -/*err_set_par:*/
> +err_set_par:
> tmiofb_hw_stop(dev);
> err_hw_init:
> if (cell->disable)
> --
> 1.8.4.rc3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] fbdev: fix error return code in metronomefb_probe()
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-19 10:52 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <CAPgLHd-FgU-512ivq_eRPqefqb8-7qGY8aoOATiFNZYn-P_4rg@mail.gmail.com>
On 14:25 Sat 12 Oct , Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Fix to return a negative error code from the error handling
> case instead of 0, as done elsewhere in this function.
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Tomi can you take it as a fix
Best Regards,
J.
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
> drivers/video/metronomefb.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/metronomefb.c b/drivers/video/metronomefb.c
> index f30150d..0b36160 100644
> --- a/drivers/video/metronomefb.c
> +++ b/drivers/video/metronomefb.c
> @@ -690,7 +690,8 @@ static int metronomefb_probe(struct platform_device *dev)
> goto err_csum_table;
> }
>
> - if (board->setup_irq(info))
> + retval = board->setup_irq(info);
> + if (retval)
> goto err_csum_table;
>
> retval = metronome_init_regs(par);
>
^ permalink raw reply
* Re: [Patch v2][ 10/37] video: imxfb: Also add pwmr for the device tree.
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-19 10:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382022155-21954-11-git-send-email-denis@eukrea.com>
On 17:02 Thu 17 Oct , Denis Carikli wrote:
> 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>
Best Regards,
J.
^ permalink raw reply
* Re: [Patch v2][ 08/37] video: mx3fb: Add device tree suport.
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-19 11:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382022155-21954-9-git-send-email-denis@eukrea.com>
On 17:02 Thu 17 Oct , Denis Carikli wrote:
> 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: 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>
> ---
> .../devicetree/bindings/video/fsl,mx3-fb.txt | 52 ++++++++
> drivers/video/Kconfig | 2 +
> drivers/video/mx3fb.c | 133 +++++++++++++++++---
> 3 files changed, 171 insertions(+), 16 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..ae0b343
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> @@ -0,0 +1,52 @@
> +Freescale mx3 Framebuffer
> +
> +This framebuffer driver supports the imx31 and imx35 devices.
> +
> +Required properties:
> +- compatible : Must be "fsl,mx3-fb".
> +- reg : Should contain 1 register ranges(address and length).
> +- dmas : Phandle to the ipu dma node as described in
> + Documentation/devicetree/bindings/dma/dma.txt
> +- dma-names : Must be "tx".
> +- clocks : Phandle to the ipu source clock.
> +- display: Phandle to a display node as described in
> + Documentation/devicetree/bindings/video/display-timing.txt
> + Additional, the display node has to define properties:
> + - bits-per-pixel: lcd panel bit-depth.
> +
> +Optional properties:
> +- ipu-disp-format: could be "rgb666", "rgb565", or "rgb888".
> + If not specified defaults to "rgb666".
> +
> +Example:
> +
> + lcdc: mx3fb@53fc00b4 {
> + compatible = "fsl,mx3-fb";
> + reg = <0x53fc00b4 0x0b>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_lcdc_1>;
> + clocks = <&clks 55>;
> + dmas = <&ipu 14>;
> + dma-names = "tx";
> + };
> +
> + ...
> +
> + cmo_qvga: display {
> + model = "CMO-QVGA";
> + bits-per-pixel = <16>;
> + native-mode = <&qvga_timings>;
> + display-timings {
> + qvga_timings: 320x240 {
> + clock-frequency = <6500000>;
> + hactive = <320>;
> + vactive = <240>;
> + hback-porch = <30>;
> + hfront-porch = <38>;
> + vback-porch = <20>;
> + vfront-porch = <3>;
> + hsync-len = <15>;
> + vsync-len = <4>;
> + };
> + };
> + };
> 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 37704da..8683dda 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -32,6 +32,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>
>
> @@ -759,11 +761,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;
> @@ -1392,21 +1396,63 @@ 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("rgb888", property))
> + return IPU_DISP_DATA_MAPPING_RGB888;
> + else
> + return -EINVAL;
> +}
mode parsing to be a geneirc API
otherwise looks good
Best Regards,
J.
^ permalink raw reply
* Re: [Patch v2][ 07/37] video: mx3fb: Introduce regulator support.
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-19 11:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382022155-21954-8-git-send-email-denis@eukrea.com>
On 17:02 Thu 17 Oct , Denis Carikli wrote:
> 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>
> ---
> drivers/video/mx3fb.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index 804f874..37704da 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>
> @@ -267,6 +268,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 */
> };
> @@ -1001,6 +1003,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;
>
> @@ -1019,6 +1022,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)) {
do we realy need ot check if enabled?
this should be handled by the regulator API
same as clock I can disable not enabled clock
Best Regards,
J.
> + 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);
> @@ -1026,7 +1038,17 @@ 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;
> }
> }
> @@ -1202,6 +1224,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);
> @@ -1210,7 +1233,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;
> }
> @@ -1222,10 +1253,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();
> @@ -1438,6 +1479,10 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> if (ret < 0)
> goto erfb;
>
> + mx3fbi->reg_lcd = devm_regulator_get(dev, "lcd");
> + if (IS_ERR(mx3fbi->reg_lcd))
> + mx3fbi->reg_lcd = NULL;
> +
> return 0;
>
> erfb:
> --
> 1.7.9.5
>
^ permalink raw reply
* Re: [Patch v2][ 05/37] fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_*
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-19 11:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382022155-21954-6-git-send-email-denis@eukrea.com>
On 17:02 Thu 17 Oct , Denis Carikli wrote:
> 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>
Best Regards,
J.
> 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
* Re: [PATCH] tmio: call tmiofb_set_par in tmiofb_probe
From: Dmitry Eremin-Solenikov @ 2013-10-19 12:41 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1382003443-5044-1-git-send-email-dbaryshkov@gmail.com>
On Sat, Oct 19, 2013 at 2:48 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 13:50 Thu 17 Oct , Dmitry Eremin-Solenikov wrote:
>> Call tmiofb_set_par() to initlaize fixed variable before registering
>> framebuffer. This is necessary for current kexecboot.
>>
>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>> ---
>> drivers/video/tmiofb.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/tmiofb.c b/drivers/video/tmiofb.c
>> index deb8733..69fd0f7 100644
>> --- a/drivers/video/tmiofb.c
>> +++ b/drivers/video/tmiofb.c
>> @@ -774,6 +774,10 @@ static int tmiofb_probe(struct platform_device *dev)
>> if (retval)
>> goto err_hw_init;
>>
>> + retval = tmiofb_set_par(info);
>> + if (retval)
>> + goto err_set_par;
>> +
>
> sorry no fix kexecboot
So calling set_par in _probe() is deprecated/fromn upon? Why?
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Sylwester Nawrocki @ 2013-10-20 19:29 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Laurent Pinchart, Thierry Reding, Laurent Pinchart, Rob Herring,
Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Fbdev development list,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
Dave Airlie
In-Reply-To: <525FD8DD.3090509-l0cyMroinI0@public.gmane.org>
On 10/17/2013 02:32 PM, Tomi Valkeinen wrote:
> On 17/10/13 15:17, Laurent Pinchart wrote:
>> On Thursday 17 October 2013 14:59:41 Tomi Valkeinen wrote:
>>> On 17/10/13 14:51, Laurent Pinchart wrote:
>>>>> I'm not sure if there's a specific need for the port or endpoint nodes
>>>>> in cases like the above. Even if we have common properties describing
>>>>> the endpoint, I guess they could just be in the parent node.
>>>>>
>>>>> panel {
>>>>> remote =<&dc>;
>>>>> common-video-property =<asd>;
>>>>> };
>>>>>
>>>>> The above would imply one port and one endpoint. Would that work? If we
>>>>> had a function like parse_endpoint(node), we could just point it to
>>>>> either a real endpoint node, or to the device's node.
>>>>
>>>> You reference the display controller here, not a specific display
>>>> controller output. Don't most display controllers have several outputs ?
>>>
>>> Sure. Then the display controller could have more verbose description.
>>> But the panel could still have what I wrote above, except the 'remote'
>>> property would point to a real endpoint node inside the dispc node, not
>>> to the dispc node.
>>>
>>> This would, of course, need some extra code to handle the different
>>> cases, but just from DT point of view, I think all the relevant
>>> information is there.
>>
>> There's many ways to describe the same information in DT. While we could have
>> DT bindings that use different descriptions for different devices and still
>> support all of them in our code, why should we opt for that option that will
>> make the implementation much more complex when we can describe connections in
>> a simple and generic way ?
>
> My suggestion was simple and generic. I'm not proposing per-device
> custom bindings.
>
> My point was, if we can describe the connections as I described above,
> which to me sounds possible, we can simplify the panel DT data for 99.9%
> of the cases.
>
> To me, the first of these looks much nicer:
>
> panel {
> remote =<&remote-endpoint>;
> common-video-property =<asd>;
> };
>
> panel {
> port {
> endpoint {
> remote =<&remote-endpoint>;
> common-video-property =<asd>;
> };
> };
> };
>
> If that can be supported in the SW by adding complexity to a few
> functions, and it covers practically all the panels, isn't it worth it?
>
> Note that I have not tried this, so I don't know if there are issues.
> It's just a thought. Even if there's need for a endpoint node, perhaps
> the port node can be made optional.
Each endpoint node was originally supposed to contain the port configuration
properties specific to a single remote device. So there would be more than
one endpoint sub-mode of a port node only if, e.g. a display controller
could
output data through a single port to more than one panel (not necessarily
simultaneously).
If it is known in the display subsystems there is no use case where single
data output (port) is being switched between (connected to) multiple data
sinks we could probably omit the endpoint nodes in order to simplify the
bindings a bit.
We could have something like:
dispc {
disp_port: port {
remote-port = <&p_port>;
common-video-property = <>;
};
};
panel {
p_port: port {
remote-port = <&disp_port>;
common-video-property = <>;
};
};
Alternatively we could say, that for only one endpoint node needed such an
endpoint node can be omitted and the common video properties can be
specified
directly in the port node.
This would require introducing 'remote-port' phandle property, as
remote-endpoint is supposed to point to an endpoint, not a port node.
Regarding the reverse link phandle, I was wondering if it shouldn't be made
optional. We would have to think twice about such a change though, since
making an optional property back mandatory wouldn't be possible.
--
Thanks,
Sylwester
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Stephen Warren @ 2013-10-20 22:01 UTC (permalink / raw)
To: Tomi Valkeinen, Dave Airlie, Laurent Pinchart
Cc: Thierry Reding, Laurent Pinchart, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, devicetree-u79uwXL29TY76Z2rM5mHXA,
Linux Fbdev development list
In-Reply-To: <525FA4B0.8060504-l0cyMroinI0@public.gmane.org>
On 10/17/2013 09:49 AM, Tomi Valkeinen wrote:
> Hi,
>
> On 17/10/13 11:28, Dave Airlie wrote:
>>>
>>> My biggest concern here is that this will not be compatible
>>> with the CDF DT bindings. They're not complete yet, but they
>>> will require connections between entities to be described in
>>> DT, in a way very similar (or actually identical) to the V4L2
>>> DT bindings, documented in
>>> Documentation/devicetree/bindings/media/video-interfaces.txt.
>>> Could you have a look at that ? Please ignore all optional
>>> properties beside remote-endpoint, as they're V4L2 specific.
>>>
>>> I also plan to specify video bus parameters in DT for CDF, but
>>> this hasn't been finalized yet.
>>>
>>
>> While I understand this, I don't see why CDF can't enhance these
>> bindings if it has requirements > than they have without
>> disturbing the panel ones,
>>
>> is DT really that inflexible?
>>
>> It seems that have a simple description for basic panels like
>> Thierry wants to support, that can be enhanced for the other
>> cases in the future should suffice, I really don't like blocking
>> stuff that makes things work on the chance of something that
>> isn't upstream yet, its sets a bad precedent, its also breaks the
>> perfect is the enemy of good rule
>
> Just my opinion, but yes, DT is that inflexible.
I don't really agree.
A specific DT binding is supposed to be an ABI; something that can
only be changed in a backwards-compatible way (perhaps only in a
forwards-compatible way too).
However, I see no reason you can't have a simple DT binding now, and
create a more complex DT binding later. Both can exist (be defined).
Now, because DT is an ABI, we can't remove kernel support for the old
binding.
> DT bindings are like a syscall. Once they are there, you need to
> support them forever. That support can, of course, include some
> kind of DT data converters or such, that try to convert old
> bindings to new bindings at kernel boot.
I don't think we need a converter for that; just leave in both the old
and new parser/driver and treat them as different things. There's no
reason that a new kernel must provide new features when given an old
DT written to the old binding, and hence a new kernel can just treat
the old DT with the old code.
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Stephen Warren @ 2013-10-20 22:07 UTC (permalink / raw)
To: Laurent Pinchart, Thierry Reding
Cc: Tomi Valkeinen, Dave Airlie, Laurent Pinchart, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <3592147.9IprvsogX6@avalon>
On 10/17/2013 12:07 PM, Laurent Pinchart wrote:
...
>> As I said, anything that really needs a CDF binding to work
>> likely isn't "simple" anymore, therefore a separate driver can
>> easily be justified.
>
> The system as a whole would be more complex, but the panel could be
> the same. We can't have two drivers for the same piece of hardware
> in the DT world, as there will be a single compatible string and no
> way to choose between the drivers (unlike the board code world that
> could set device names to "foo- encoder-v4l2" or "foo-encoder-drm"
> and live happily with that ever after).
That's not true. We can certainly define two different compatible
values for a piece of HW if we have to. We can easily control whether
they are handled by the same or different drivers in the OS.
Now, we should try to avoid this, because then that means that the
original binding wasn't fully describing the HW. However, at least in
the case of these simple LCD panels, it's hard to see that there is
anything more than what's already in Thierry's binding. Remember, the
binding is a description of the HW, not any Linux-internal details of
how e.g. a CDF or DRM subsystem is supposed to use the HW; that had
better be embodied into the driver or subsystem code, which aren't
ABIs, and hence can be easily modified/enhanced.
^ permalink raw reply
* [RFC PATCH] fb: reorder the lock sequence to fix a potential lockdep
From: Gu Zheng @ 2013-10-21 4:12 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen
Cc: Linux Fbdev development list, linux-kernel
Following commits:
50e244cc79 fb: rework locking to fix lock ordering on takeover
e93a9a8687 fb: Yet another band-aid for fixing lockdep mess
054430e773 fbcon: fix locking harder
reworked locking to fix related lock ordering on takeover, and introduced console_lock
into fbmem, but it seems that the new lock sequence(fb_info->lock ---> console_lock)
is against with the one in console_callback(console_lock ---> fb_info->lock), and leads to
a potential deadlock as following:
[ 601.079000] ===========================
[ 601.079000] [ INFO: possible circular locking dependency detected ]
[ 601.079000] 3.11.0 #189 Not tainted
[ 601.079000] -------------------------------------------------------
[ 601.079000] kworker/0:3/619 is trying to acquire lock:
[ 601.079000] (&fb_info->lock){+.+.+.}, at: [<ffffffff81397566>] lock_fb_info+0x26/0x60
[ 601.079000]
but task is already holding lock:
[ 601.079000] (console_lock){+.+.+.}, at: [<ffffffff8141aae3>] console_callback+0x13/0x160
[ 601.079000]
which lock already depends on the new lock.
[ 601.079000]
the existing dependency chain (in reverse order) is:
[ 601.079000]
-> #1 (console_lock){+.+.+.}:
[ 601.079000] [<ffffffff810dc971>] lock_acquire+0xa1/0x140
[ 601.079000] [<ffffffff810c6267>] console_lock+0x77/0x80
[ 601.079000] [<ffffffff81399448>] register_framebuffer+0x1d8/0x320
[ 601.079000] [<ffffffff81cfb4c8>] efifb_probe+0x408/0x48f
[ 601.079000] [<ffffffff8144a963>] platform_drv_probe+0x43/0x80
[ 601.079000] [<ffffffff8144853b>] driver_probe_device+0x8b/0x390
[ 601.079000] [<ffffffff814488eb>] __driver_attach+0xab/0xb0
[ 601.079000] [<ffffffff814463bd>] bus_for_each_dev+0x5d/0xa0
[ 601.079000] [<ffffffff81447e6e>] driver_attach+0x1e/0x20
[ 601.079000] [<ffffffff81447a07>] bus_add_driver+0x117/0x290
[ 601.079000] [<ffffffff81448fea>] driver_register+0x7a/0x170
[ 601.079000] [<ffffffff8144a10a>] __platform_driver_register+0x4a/0x50
[ 601.079000] [<ffffffff8144a12d>] platform_driver_probe+0x1d/0xb0
[ 601.079000] [<ffffffff81cfb0a1>] efifb_init+0x273/0x292
[ 601.079000] [<ffffffff81002132>] do_one_initcall+0x102/0x1c0
[ 601.079000] [<ffffffff81cb80a6>] kernel_init_freeable+0x15d/0x1ef
[ 601.079000] [<ffffffff8166d2de>] kernel_init+0xe/0xf0
[ 601.079000] [<ffffffff816914ec>] ret_from_fork+0x7c/0xb0
[ 601.079000]
-> #0 (&fb_info->lock){+.+.+.}:
[ 601.079000] [<ffffffff810dc1d8>] __lock_acquire+0x1e18/0x1f10
[ 601.079000] [<ffffffff810dc971>] lock_acquire+0xa1/0x140
[ 601.079000] [<ffffffff816835ca>] mutex_lock_nested+0x7a/0x3b0
[ 601.079000] [<ffffffff81397566>] lock_fb_info+0x26/0x60
[ 601.079000] [<ffffffff813a4aeb>] fbcon_blank+0x29b/0x2e0
[ 601.079000] [<ffffffff81418658>] do_blank_screen+0x1d8/0x280
[ 601.079000] [<ffffffff8141ab34>] console_callback+0x64/0x160
[ 601.079000] [<ffffffff8108d855>] process_one_work+0x1f5/0x540
[ 601.079000] [<ffffffff8108e04c>] worker_thread+0x11c/0x370
[ 601.079000] [<ffffffff81095fbd>] kthread+0xed/0x100
[ 601.079000] [<ffffffff816914ec>] ret_from_fork+0x7c/0xb0
[ 601.079000]
other info that might help us debug this:
[ 601.079000] Possible unsafe locking scenario:
[ 601.079000] CPU0 CPU1
[ 601.079000] ---- ----
[ 601.079000] lock(console_lock);
[ 601.079000] lock(&fb_info->lock);
[ 601.079000] lock(console_lock);
[ 601.079000] lock(&fb_info->lock);
[ 601.079000]
*** DEADLOCK ***
so we reorder the lock sequence the same as it in console_callback() to
avoid this issue.
Not very sure this change is suitable, any comments is welcome.
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
drivers/video/fbmem.c | 50 +++++++++++++++++++++++++++++++-----------------
1 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index dacaf74..010d191 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1108,14 +1108,16 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
case FBIOPUT_VSCREENINFO:
if (copy_from_user(&var, argp, sizeof(var)))
return -EFAULT;
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
info->flags |= FBINFO_MISC_USEREVENT;
ret = fb_set_var(info, &var);
info->flags &= ~FBINFO_MISC_USEREVENT;
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
if (!ret && copy_to_user(argp, &var, sizeof(var)))
ret = -EFAULT;
break;
@@ -1144,12 +1146,14 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
case FBIOPAN_DISPLAY:
if (copy_from_user(&var, argp, sizeof(var)))
return -EFAULT;
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
ret = fb_pan_display(info, &var);
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
if (ret = 0 && copy_to_user(argp, &var, sizeof(var)))
return -EFAULT;
break;
@@ -1184,23 +1188,27 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
break;
}
event.data = &con2fb;
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
event.info = info;
ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, &event);
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
break;
case FBIOBLANK:
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
info->flags |= FBINFO_MISC_USEREVENT;
ret = fb_blank(info, arg);
info->flags &= ~FBINFO_MISC_USEREVENT;
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
break;
default:
if (!lock_fb_info(info))
@@ -1660,12 +1668,15 @@ static int do_register_framebuffer(struct fb_info *fb_info)
registered_fb[i] = fb_info;
event.info = fb_info;
- if (!lock_fb_info(fb_info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(fb_info)) {
+ console_unlock();
+ return -ENODEV;
+ }
+
fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
- console_unlock();
unlock_fb_info(fb_info);
+ console_unlock();
return 0;
}
@@ -1678,13 +1689,16 @@ static int do_unregister_framebuffer(struct fb_info *fb_info)
if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)
return -EINVAL;
- if (!lock_fb_info(fb_info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(fb_info)) {
+ console_unlock();
+ return -ENODEV;
+ }
+
event.info = fb_info;
ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
- console_unlock();
unlock_fb_info(fb_info);
+ console_unlock();
if (ret)
return -EINVAL;
--
1.7.7
^ permalink raw reply related
* [PATCH 1/1] video: exynos_mipi_dsi: Remove unused variable
From: Sachin Kamat @ 2013-10-21 5:44 UTC (permalink / raw)
To: linux-fbdev
'pdev' is not used anymore (Removed vide commit 7e0be9f9 "video:
exynos_mipi_dsim: Use the generic PHY driver"). Remove it and
silence the following compilation warning:
drivers/video/exynos/exynos_mipi_dsi.c:144:26: warning:
unused variable ‘pdev’ [-Wunused-variable]
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
drivers/video/exynos/exynos_mipi_dsi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
index 00b3a52..cee9602 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -141,7 +141,6 @@ static int exynos_mipi_dsi_early_blank_mode(struct mipi_dsim_device *dsim,
static int exynos_mipi_dsi_blank_mode(struct mipi_dsim_device *dsim, int power)
{
- struct platform_device *pdev = to_platform_device(dsim->dev);
struct mipi_dsim_lcd_driver *client_drv = dsim->dsim_lcd_drv;
struct mipi_dsim_lcd_device *client_dev = dsim->dsim_lcd_dev;
--
1.7.9.5
^ permalink raw reply related
* Re: [Patch v2][ 08/37] video: mx3fb: Add device tree suport.
From: Sascha Hauer @ 2013-10-21 8:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20131019110459.GM18477@ns203013.ovh.net>
On Sat, Oct 19, 2013 at 01:04:59PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:02 Thu 17 Oct , Denis Carikli wrote:
> > 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..ae0b343
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> > @@ -0,0 +1,52 @@
> > +Freescale mx3 Framebuffer
> > +
> > +This framebuffer driver supports the imx31 and imx35 devices.
> > +
> > +Required properties:
> > +- compatible : Must be "fsl,mx3-fb".
> > +- reg : Should contain 1 register ranges(address and length).
> > +- dmas : Phandle to the ipu dma node as described in
> > + Documentation/devicetree/bindings/dma/dma.txt
> > +- dma-names : Must be "tx".
> > +- clocks : Phandle to the ipu source clock.
> > +- display: Phandle to a display node as described in
> > + Documentation/devicetree/bindings/video/display-timing.txt
> > + Additional, the display node has to define properties:
> > + - bits-per-pixel: lcd panel bit-depth.
> > +
> > +Optional properties:
> > +- ipu-disp-format: could be "rgb666", "rgb565", or "rgb888".
> > + If not specified defaults to "rgb666".
> > +
> > +Example:
> > +
> > + lcdc: mx3fb@53fc00b4 {
> > + compatible = "fsl,mx3-fb";
> > + reg = <0x53fc00b4 0x0b>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_lcdc_1>;
> > + clocks = <&clks 55>;
> > + dmas = <&ipu 14>;
> > + dma-names = "tx";
> > + };
As mentioned to the v1 patch: This should really be closer to the IPUv3
binding. IPUv1 and IPUv3 are very similar hardwares. Having two
different bindings for it is painful.
Implementing IPUv1 as a DMA driver was the wrong decision back then, now
exposing the IPU to dt using the DMA binding makes it worse.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCHv4][ 02/11] video: imxfb: Introduce regulator support.
From: Denis Carikli @ 2013-10-21 9:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382349098-11502-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: 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>
---
ChangeLog v3->v4:
- Added a dev_info when there is no lcd regulator.
---
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
* [PATCHv4][ 03/11] video: imxfb: Also add pwmr for the device tree.
From: Denis Carikli @ 2013-10-21 9:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382349098-11502-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
* Re: How to set fops in "struct platform_pwm_backlight_data"?
From: Mark Zhang @ 2013-10-22 2:41 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: <5260BD8C.7000200@gmail.com>
Ping Thierry...
Mark
On 10/18/2013 12:48 PM, Mark Zhang wrote:
> On 10/17/2013 03:14 PM, Thierry Reding wrote:
>> On Thu, Oct 17, 2013 at 02:49:57PM +0800, Mark Zhang wrote:
>>> 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?
>>
>> 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
>> Generally you're not supposed to mix DT and platform data. Without more
>> info that's about all I can say.
>>
>> Thierry
>>
^ permalink raw reply
* Re: How to set fops in "struct platform_pwm_backlight_data"?
From: Thierry Reding @ 2013-10-22 7:24 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: <5260BD8C.7000200@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1799 bytes --]
On Fri, Oct 18, 2013 at 12:48:12PM +0800, Mark Zhang wrote:
> On 10/17/2013 03:14 PM, Thierry Reding wrote:
> > On Thu, Oct 17, 2013 at 02:49:57PM +0800, Mark Zhang wrote:
> >> 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?
> >
> > 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
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox