* 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][ 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] 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] 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 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 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 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
* [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
* [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
* [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
* 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
* 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 v2] pwm-backlight: allow for non-increasing brightness levels
From: Thierry Reding @ 2013-10-18 7:46 UTC (permalink / raw)
To: Mike Dunn
Cc: Richard Purdie, Jingoo Han, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Grant Likely, Rob Herring, linux-pwm, linux-fbdev,
linux-kernel, devicetree, Robert Jarzmik, Marek Vasut
In-Reply-To: <1379869196-19377-1-git-send-email-mikedunn@newsguy.com>
[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]
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?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: How to set fops in "struct platform_pwm_backlight_data"?
From: Mark Zhang @ 2013-10-18 4:48 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: <20131017071445.GA2502@ulmo.nvidia.com>
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
* [Patch v2][ 10/37] video: imxfb: Also add pwmr for the device tree.
From: Denis Carikli @ 2013-10-17 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382022155-21954-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
* [Patch v2][ 09/37] video: imxfb: Introduce regulator support.
From: Denis Carikli @ 2013-10-17 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382022155-21954-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
* [Patch v2][ 08/37] video: mx3fb: Add device tree suport.
From: Denis Carikli @ 2013-10-17 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382022155-21954-1-git-send-email-denis@eukrea.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: 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;
+}
+
static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
{
struct device *dev = mx3fb->dev;
struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
- const char *name = mx3fb_pdata->name;
+ struct device_node *np = dev->of_node;
+ const char *name;
+ const char *ipu_disp_format;
unsigned int irq;
struct fb_info *fbi;
struct mx3fb_info *mx3fbi;
const struct fb_videomode *mode;
int ret, num_modes;
+ struct fb_videomode of_mode;
+ enum disp_data_mapping disp_data_fmt = IPU_DISP_DATA_MAPPING_RGB666;
+ struct device_node *display_np, *timings_np;
+
+ if (np) {
+ of_property_read_string(np, "ipu-disp-format",
+ &ipu_disp_format);
+ if (!ipu_disp_format) {
+ dev_err(dev, "Can't get ipu display data mapping.\n");
+ return -EINVAL;
+ }
+
+ mx3fb->disp_data_fmt = match_dt_disp_data(ipu_disp_format);
+ if (mx3fb->disp_data_fmt = -EINVAL) {
+ dev_err(dev, "Illegal display data format \"%s\"\n",
+ ipu_disp_format);
+ return -EINVAL;
+ }
- if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
- dev_err(dev, "Illegal display data format %d\n",
+ display_np = of_parse_phandle(np, "display", 0);
+ if (!display_np) {
+ dev_err(dev, "failed to find display phandle\n");
+ return -ENOENT;
+ }
+
+ of_property_read_string(display_np, "model", &name);
+ } else {
+ name = mx3fb_pdata->name;
+ if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
+ dev_err(dev, "Illegal display data format %d\n",
mx3fb_pdata->disp_data_fmt);
- return -EINVAL;
+ return -EINVAL;
+ }
}
ichan->client = mx3fb;
@@ -1427,12 +1473,39 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
goto emode;
}
- if (mx3fb_pdata->mode && mx3fb_pdata->num_modes) {
- mode = mx3fb_pdata->mode;
- num_modes = mx3fb_pdata->num_modes;
+ if (np) {
+ of_property_read_string(np, "ipu-disp-format",
+ &ipu_disp_format);
+ if (!ipu_disp_format) {
+ dev_err(dev, "Can't get ipu display data mapping.\n");
+ return -EINVAL;
+ }
+
+ disp_data_fmt = match_dt_disp_data(ipu_disp_format);
+
+ if (disp_data_fmt = -EINVAL) {
+ dev_err(dev, "Illegal display data format \"%s\" for "
+ "the node %s\n", ipu_disp_format, np->name);
+ return -EINVAL;
+ }
+
+ ret = of_get_fb_videomode(display_np, &of_mode,
+ OF_USE_NATIVE_MODE);
+ if (!ret) {
+ mode = &of_mode;
+ num_modes = 1;
+ } else {
+ mode = mx3fb_modedb;
+ num_modes = ARRAY_SIZE(mx3fb_modedb);
+ }
} else {
- mode = mx3fb_modedb;
- num_modes = ARRAY_SIZE(mx3fb_modedb);
+ if (mx3fb_pdata->mode || !mx3fb_pdata->num_modes) {
+ mode = mx3fb_pdata->mode;
+ num_modes = mx3fb_pdata->num_modes;
+ } else {
+ mode = mx3fb_modedb;
+ num_modes = ARRAY_SIZE(mx3fb_modedb);
+ }
}
if (!fb_find_mode(&fbi->var, fbi, fb_mode, mode,
@@ -1462,7 +1535,10 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
mx3fbi->mx3fb = mx3fb;
mx3fbi->blank = FB_BLANK_NORMAL;
- mx3fb->disp_data_fmt = mx3fb_pdata->disp_data_fmt;
+ if (!np)
+ mx3fb->disp_data_fmt = mx3fb_pdata->disp_data_fmt;
+ else
+ mx3fb->disp_data_fmt = disp_data_fmt;
init_completion(&mx3fbi->flip_cmpl);
disable_irq(ichan->eof_irq);
@@ -1494,13 +1570,26 @@ emode:
return ret;
}
+static int imx_dma_is_dt_ipu(struct dma_chan *chan)
+{
+ /* Example: 53fc0000.ipu */
+ if (chan && chan->device && chan->device->dev) {
+ char *name = dev_name(chan->device->dev);
+ name = strchr(name, '.');
+ if (name)
+ return !strcmp(name, ".ipu");
+ }
+
+ return 0;
+}
+
static bool chan_filter(struct dma_chan *chan, void *arg)
{
struct dma_chan_request *rq = arg;
struct device *dev;
struct mx3fb_platform_data *mx3fb_pdata;
- if (!imx_dma_is_ipu(chan))
+ if (!imx_dma_is_ipu(chan) && !imx_dma_is_dt_ipu(chan))
return false;
if (!rq)
@@ -1509,8 +1598,12 @@ static bool chan_filter(struct dma_chan *chan, void *arg)
dev = rq->mx3fb->dev;
mx3fb_pdata = dev_get_platdata(dev);
- return rq->id = chan->chan_id &&
- mx3fb_pdata->dma_dev = chan->device->dev;
+ /* When using the devicetree, mx3fb_pdata is NULL */
+ if (imx_dma_is_dt_ipu(chan))
+ return rq->id = chan->chan_id;
+ else
+ return rq->id = chan->chan_id &&
+ mx3fb_pdata->dma_dev = chan->device->dev;
}
static void release_fbi(struct fb_info *fbi)
@@ -1567,7 +1660,8 @@ static int mx3fb_probe(struct platform_device *pdev)
dma_cap_set(DMA_SLAVE, mask);
dma_cap_set(DMA_PRIVATE, mask);
rq.id = IDMAC_SDC_0;
- chan = dma_request_channel(mask, chan_filter, &rq);
+ chan = dma_request_slave_channel_compat(mask, chan_filter, &rq, dev,
+ "tx");
if (!chan) {
ret = -EBUSY;
goto ersdc0;
@@ -1610,9 +1704,16 @@ static int mx3fb_remove(struct platform_device *dev)
return 0;
}
+static struct of_device_id mx3fb_of_dev_id[] = {
+ { .compatible = "fsl,mx3-fb", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mx3fb_of_dev_id);
+
static struct platform_driver mx3fb_driver = {
.driver = {
.name = MX3FB_NAME,
+ .of_match_table = mx3fb_of_dev_id,
.owner = THIS_MODULE,
},
.probe = mx3fb_probe,
--
1.7.9.5
^ permalink raw reply related
* [Patch v2][ 07/37] video: mx3fb: Introduce regulator support.
From: Denis Carikli @ 2013-10-17 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382022155-21954-1-git-send-email-denis@eukrea.com>
This commit is based on the following commit by Fabio Estevam:
4344429 video: mxsfb: Introduce regulator support
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
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)) {
+ 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 related
* [Patch v2][ 05/37] fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_*
From: Denis Carikli @ 2013-10-17 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382022155-21954-1-git-send-email-denis@eukrea.com>
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>
---
drivers/video/fbmon.c | 4 ++++
include/uapi/linux/fb.h | 2 ++
2 files changed, 6 insertions(+)
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 6103fa6..29a9ed0 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -1402,6 +1402,10 @@ int fb_videomode_from_videomode(const struct videomode *vm,
fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
if (vm->flags & DISPLAY_FLAGS_VSYNC_HIGH)
fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+ if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
+ fbmode->sync |= FB_SYNC_DE_HIGH_ACT;
+ if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+ fbmode->sync |= FB_SYNC_PIXDAT_HIGH_ACT;
if (vm->flags & DISPLAY_FLAGS_INTERLACED)
fbmode->vmode |= FB_VMODE_INTERLACED;
if (vm->flags & DISPLAY_FLAGS_DOUBLESCAN)
diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
index fb795c3..30487df 100644
--- a/include/uapi/linux/fb.h
+++ b/include/uapi/linux/fb.h
@@ -215,6 +215,8 @@ struct fb_bitfield {
/* vtotal = 144d/288n/576i => PAL */
/* vtotal = 121d/242n/484i => NTSC */
#define FB_SYNC_ON_GREEN 32 /* sync on green */
+#define FB_SYNC_DE_HIGH_ACT 64 /* data enable high active */
+#define FB_SYNC_PIXDAT_HIGH_ACT 64 /* data enable high active */
#define FB_VMODE_NONINTERLACED 0 /* non interlaced */
#define FB_VMODE_INTERLACED 1 /* interlaced */
--
1.7.9.5
^ permalink raw reply related
* Backlight enabled by default
From: Thierry Reding @ 2013-10-17 14:41 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Jingoo Han, 'Tomi Valkeinen', 'Richard Purdie',
'Jean-Christophe Plagniol-Villard', linux-fbdev,
linux-kernel, 'Laurent Pinchart'
[-- Attachment #1: Type: text/plain, Size: 2126 bytes --]
While working on integrating backlight functionality with the DRM sub-
system, I came across an oddity of sorts. The backlight subsystem seems
to have an implicit policy of enabling the backlight device when it's
registered. Pretty much every driver seems to do something like this in
.probe():
bl->props.brightness = default_brightness;
bl->props.fb_blank = FB_BLANK_UNBLANK;
backlight_update_status(bl);
There are variations of this theme, but the tendency is certainly to
enable the backlight once it has been registered. Some don't even set
the .fb_blank field, so it will usually stay initialized to zero, which
happens to be FB_BLANK_UNBLANK, so the result will be the same.
I understand that this is actually useful when using something such as
fbdev where the backlight isn't necessarily bound to a display device,
but when used in conjunction with a higher-level API such as DRM, then
this default behaviour becomes somewhat annoying.
The problem is that backlight devices are usually instantiated
separately from the display driver, so with the current default the
backlight will be enabled at some random point in time during boot.
However, DRM for example provides for a display to control very
precisely when the backlight should be enabled, which in turn makes it
easy to light it up right after the display has initialized. If the
backlight is turned on right after it has been probed this could mean
that the display hasn't been initialized yet and therefore there's no
meaningful content yet, and worse, the display might show garbage during
initialization of the display controller.
So I wonder if perhaps a better default would be to not enable the
backlight on boot. If so, this will actually cause a regression of some
sort on non-DRM systems because suddenly the backlight is no longer
enabled by default on boot.
Does anyone have any comments or ideas?
My first reaction was to add a property to the DT so that the driver
could skip enabling the backlight, but I don't think that will be
accepted because it encodes software policy in DT rather than purely
a description of hardware.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH v2] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Marek Belisko @ 2013-10-17 14:14 UTC (permalink / raw)
To: tomi.valkeinen
Cc: plagnioj, linux-kernel, linux-omap, linux-fbdev, Marek Belisko,
H. Nikolaus Schaller
Signed-off-by: Marek Belisko <marek@goldelico.com>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
changes from v1:
- reworked to be spi driver instead platform with custom spi bitbang
this configuration was tested with spi_gpio bitbang driver on gta04 board
and works fine (thanks Tomi and Lars-Peter for comments)
- address previous comments
drivers/video/omap2/displays-new/Kconfig | 6 +
drivers/video/omap2/displays-new/Makefile | 1 +
.../omap2/displays-new/panel-tpo-td028ttec1.c | 486 +++++++++++++++++++++
include/video/omap-panel-data.h | 13 +
4 files changed, 506 insertions(+)
create mode 100644 drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c
diff --git a/drivers/video/omap2/displays-new/Kconfig b/drivers/video/omap2/displays-new/Kconfig
index 6c90885..1054249 100644
--- a/drivers/video/omap2/displays-new/Kconfig
+++ b/drivers/video/omap2/displays-new/Kconfig
@@ -56,6 +56,12 @@ config DISPLAY_PANEL_SHARP_LS037V7DW01
help
LCD Panel used in TI's SDP3430 and EVM boards
+config DISPLAY_PANEL_TPO_TD028TTEC1
+ tristate "TPO TD028TTEC1 LCD Panel"
+ depends on SPI
+ help
+ LCD panel used in Openmoko.
+
config DISPLAY_PANEL_TPO_TD043MTEA1
tristate "TPO TD043MTEA1 LCD Panel"
depends on SPI
diff --git a/drivers/video/omap2/displays-new/Makefile b/drivers/video/omap2/displays-new/Makefile
index 5aeb11b..0323a8a 100644
--- a/drivers/video/omap2/displays-new/Makefile
+++ b/drivers/video/omap2/displays-new/Makefile
@@ -8,5 +8,6 @@ obj-$(CONFIG_DISPLAY_PANEL_DSI_CM) += panel-dsi-cm.o
obj-$(CONFIG_DISPLAY_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o
obj-$(CONFIG_DISPLAY_PANEL_LGPHILIPS_LB035Q02) += panel-lgphilips-lb035q02.o
obj-$(CONFIG_DISPLAY_PANEL_SHARP_LS037V7DW01) += panel-sharp-ls037v7dw01.o
+obj-$(CONFIG_DISPLAY_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
obj-$(CONFIG_DISPLAY_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
obj-$(CONFIG_DISPLAY_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
diff --git a/drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c b/drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c
new file mode 100644
index 0000000..5a44d30
--- /dev/null
+++ b/drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c
@@ -0,0 +1,486 @@
+/*
+ * Toppoly TD028TTEC1 panel support
+ *
+ * Copyright (C) 2008 Nokia Corporation
+ * Author: Tomi Valkeinen <tomi.valkeinen@nokia.com>
+ *
+ * Neo 1973 code (jbt6k74.c):
+ * Copyright (C) 2006-2007 by OpenMoko, Inc.
+ * Author: Harald Welte <laforge@openmoko.org>
+ *
+ * Ported and adapted from Neo 1973 U-Boot by:
+ * H. Nikolaus Schaller <hns@goldelico.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/spi/spi.h>
+#include <linux/gpio.h>
+#include <video/omapdss.h>
+#include <video/omap-panel-data.h>
+
+struct panel_drv_data {
+ struct omap_dss_device dssdev;
+ struct omap_dss_device *in;
+
+ int data_lines;
+
+ struct omap_video_timings videomode;
+
+ struct spi_device *spi_dev;
+
+ u16 tx_buf[4];
+};
+
+static struct omap_video_timings td028ttec1_panel_timings = {
+ .x_res = 480,
+ .y_res = 640,
+ .pixel_clock = 22153,
+ .hfp = 24,
+ .hsw = 8,
+ .hbp = 8,
+ .vfp = 4,
+ .vsw = 2,
+ .vbp = 2,
+
+ .vsync_level = OMAPDSS_SIG_ACTIVE_LOW,
+ .hsync_level = OMAPDSS_SIG_ACTIVE_LOW,
+
+ .data_pclk_edge = OMAPDSS_DRIVE_SIG_FALLING_EDGE,
+ .de_level = OMAPDSS_SIG_ACTIVE_HIGH,
+ .sync_pclk_edge = OMAPDSS_DRIVE_SIG_OPPOSITE_EDGES,
+};
+
+#define JBT_COMMAND 0x000
+#define JBT_DATA 0x100
+
+int jbt_reg_write_nodata(struct panel_drv_data *ddata, u8 reg)
+{
+ int rc;
+
+ ddata->tx_buf[0] = JBT_COMMAND | reg;
+ rc = spi_write(ddata->spi_dev, (u8 *)ddata->tx_buf,
+ 1*sizeof(u16));
+ if (rc != 0)
+ dev_err(&ddata->spi_dev->dev,
+ "jbt_reg_write_nodata spi_write ret %d\n", rc);
+
+ return rc;
+}
+
+int jbt_reg_write(struct panel_drv_data *ddata, u8 reg, u8 data)
+{
+ int rc;
+
+ ddata->tx_buf[0] = JBT_COMMAND | reg;
+ ddata->tx_buf[1] = JBT_DATA | data;
+ rc = spi_write(ddata->spi_dev, (u8 *)ddata->tx_buf,
+ 2*sizeof(u16));
+ if (rc != 0)
+ dev_err(&ddata->spi_dev->dev,
+ "jbt_reg_write spi_write ret %d\n", rc);
+
+ return rc;
+}
+
+int jbt_reg_write16(struct panel_drv_data *ddata, u8 reg, u16 data)
+{
+ int rc;
+
+ ddata->tx_buf[0] = JBT_COMMAND | reg;
+ ddata->tx_buf[1] = JBT_DATA | (data >> 8);
+ ddata->tx_buf[2] = JBT_DATA | (data & 0xff);
+
+ rc = spi_write(ddata->spi_dev, (u8 *)ddata->tx_buf,
+ 3*sizeof(u16));
+
+ if (rc != 0)
+ dev_err(&ddata->spi_dev->dev,
+ "jbt_reg_write16 spi_write ret %d\n", rc);
+
+ return rc;
+}
+
+enum jbt_register {
+ JBT_REG_SLEEP_IN = 0x10,
+ JBT_REG_SLEEP_OUT = 0x11,
+
+ JBT_REG_DISPLAY_OFF = 0x28,
+ JBT_REG_DISPLAY_ON = 0x29,
+
+ JBT_REG_RGB_FORMAT = 0x3a,
+ JBT_REG_QUAD_RATE = 0x3b,
+
+ JBT_REG_POWER_ON_OFF = 0xb0,
+ JBT_REG_BOOSTER_OP = 0xb1,
+ JBT_REG_BOOSTER_MODE = 0xb2,
+ JBT_REG_BOOSTER_FREQ = 0xb3,
+ JBT_REG_OPAMP_SYSCLK = 0xb4,
+ JBT_REG_VSC_VOLTAGE = 0xb5,
+ JBT_REG_VCOM_VOLTAGE = 0xb6,
+ JBT_REG_EXT_DISPL = 0xb7,
+ JBT_REG_OUTPUT_CONTROL = 0xb8,
+ JBT_REG_DCCLK_DCEV = 0xb9,
+ JBT_REG_DISPLAY_MODE1 = 0xba,
+ JBT_REG_DISPLAY_MODE2 = 0xbb,
+ JBT_REG_DISPLAY_MODE = 0xbc,
+ JBT_REG_ASW_SLEW = 0xbd,
+ JBT_REG_DUMMY_DISPLAY = 0xbe,
+ JBT_REG_DRIVE_SYSTEM = 0xbf,
+
+ JBT_REG_SLEEP_OUT_FR_A = 0xc0,
+ JBT_REG_SLEEP_OUT_FR_B = 0xc1,
+ JBT_REG_SLEEP_OUT_FR_C = 0xc2,
+ JBT_REG_SLEEP_IN_LCCNT_D = 0xc3,
+ JBT_REG_SLEEP_IN_LCCNT_E = 0xc4,
+ JBT_REG_SLEEP_IN_LCCNT_F = 0xc5,
+ JBT_REG_SLEEP_IN_LCCNT_G = 0xc6,
+
+ JBT_REG_GAMMA1_FINE_1 = 0xc7,
+ JBT_REG_GAMMA1_FINE_2 = 0xc8,
+ JBT_REG_GAMMA1_INCLINATION = 0xc9,
+ JBT_REG_GAMMA1_BLUE_OFFSET = 0xca,
+
+ JBT_REG_BLANK_CONTROL = 0xcf,
+ JBT_REG_BLANK_TH_TV = 0xd0,
+ JBT_REG_CKV_ON_OFF = 0xd1,
+ JBT_REG_CKV_1_2 = 0xd2,
+ JBT_REG_OEV_TIMING = 0xd3,
+ JBT_REG_ASW_TIMING_1 = 0xd4,
+ JBT_REG_ASW_TIMING_2 = 0xd5,
+
+ JBT_REG_HCLOCK_VGA = 0xec,
+ JBT_REG_HCLOCK_QVGA = 0xed,
+};
+
+#define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev)
+
+static int td028ttec1_panel_connect(struct omap_dss_device *dssdev)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+ struct omap_dss_device *in = ddata->in;
+ int r;
+
+ if (omapdss_device_is_connected(dssdev))
+ return 0;
+
+ r = in->ops.dpi->connect(in, dssdev);
+ if (r)
+ return r;
+
+ return 0;
+}
+
+static void td028ttec1_panel_disconnect(struct omap_dss_device *dssdev)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+ struct omap_dss_device *in = ddata->in;
+
+ if (!omapdss_device_is_connected(dssdev))
+ return;
+
+ in->ops.dpi->disconnect(in, dssdev);
+}
+
+static int td028ttec1_panel_enable(struct omap_dss_device *dssdev)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+ struct omap_dss_device *in = ddata->in;
+ int r;
+
+ if (!omapdss_device_is_connected(dssdev))
+ return -ENODEV;
+
+ if (omapdss_device_is_enabled(dssdev))
+ return 0;
+
+ in->ops.dpi->set_data_lines(in, ddata->data_lines);
+ in->ops.dpi->set_timings(in, &ddata->videomode);
+
+ r = in->ops.dpi->enable(in);
+ if (r)
+ return r;
+
+ dev_dbg(dssdev->dev, "td028ttec1_panel_enable() - state %d\n",
+ dssdev->state);
+
+ /*
+ * according to data sheet: wait 50ms (Tpos of LCM). However, 50ms
+ * seems unreliable with later LCM batches, increasing to 90ms
+ */
+ msleep(90);
+
+ /* three times command zero */
+ r |= jbt_reg_write_nodata(ddata, 0x00);
+ msleep(1000);
+ r |= jbt_reg_write_nodata(ddata, 0x00);
+ msleep(1000);
+ r |= jbt_reg_write_nodata(ddata, 0x00);
+ msleep(1000);
+
+ if (r) {
+ dev_warn(dssdev->dev, "transfer error\n");
+ goto transfer_err;
+ }
+
+ /* deep standby out */
+ r |= jbt_reg_write(ddata, JBT_REG_POWER_ON_OFF, 0x17);
+
+ /* RGB I/F on, RAM write off, QVGA through, SIGCON enable */
+ r |= jbt_reg_write(ddata, JBT_REG_DISPLAY_MODE, 0x80);
+
+ /* Quad mode off */
+ r |= jbt_reg_write(ddata, JBT_REG_QUAD_RATE, 0x00);
+
+ /* AVDD on, XVDD on */
+ r |= jbt_reg_write(ddata, JBT_REG_POWER_ON_OFF, 0x16);
+
+ /* Output control */
+ r |= jbt_reg_write16(ddata, JBT_REG_OUTPUT_CONTROL, 0xfff9);
+
+ /* Sleep mode off */
+ r |= jbt_reg_write_nodata(ddata, JBT_REG_SLEEP_OUT);
+
+ /* at this point we have like 50% grey */
+
+ /* initialize register set */
+ r |= jbt_reg_write(ddata, JBT_REG_DISPLAY_MODE1, 0x01);
+ r |= jbt_reg_write(ddata, JBT_REG_DISPLAY_MODE2, 0x00);
+ r |= jbt_reg_write(ddata, JBT_REG_RGB_FORMAT, 0x60);
+ r |= jbt_reg_write(ddata, JBT_REG_DRIVE_SYSTEM, 0x10);
+ r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_OP, 0x56);
+ r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_MODE, 0x33);
+ r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_FREQ, 0x11);
+ r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_FREQ, 0x11);
+ r |= jbt_reg_write(ddata, JBT_REG_OPAMP_SYSCLK, 0x02);
+ r |= jbt_reg_write(ddata, JBT_REG_VSC_VOLTAGE, 0x2b);
+ r |= jbt_reg_write(ddata, JBT_REG_VCOM_VOLTAGE, 0x40);
+ r |= jbt_reg_write(ddata, JBT_REG_EXT_DISPL, 0x03);
+ r |= jbt_reg_write(ddata, JBT_REG_DCCLK_DCEV, 0x04);
+ /*
+ * default of 0x02 in JBT_REG_ASW_SLEW responsible for 72Hz requirement
+ * to avoid red / blue flicker
+ */
+ r |= jbt_reg_write(ddata, JBT_REG_ASW_SLEW, 0x04);
+ r |= jbt_reg_write(ddata, JBT_REG_DUMMY_DISPLAY, 0x00);
+
+ r |= jbt_reg_write(ddata, JBT_REG_SLEEP_OUT_FR_A, 0x11);
+ r |= jbt_reg_write(ddata, JBT_REG_SLEEP_OUT_FR_B, 0x11);
+ r |= jbt_reg_write(ddata, JBT_REG_SLEEP_OUT_FR_C, 0x11);
+ r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_D, 0x2040);
+ r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_E, 0x60c0);
+ r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_F, 0x1020);
+ r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_G, 0x60c0);
+
+ r |= jbt_reg_write16(ddata, JBT_REG_GAMMA1_FINE_1, 0x5533);
+ r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_FINE_2, 0x00);
+ r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_INCLINATION, 0x00);
+ r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_BLUE_OFFSET, 0x00);
+
+ r |= jbt_reg_write16(ddata, JBT_REG_HCLOCK_VGA, 0x1f0);
+ r |= jbt_reg_write(ddata, JBT_REG_BLANK_CONTROL, 0x02);
+ r |= jbt_reg_write16(ddata, JBT_REG_BLANK_TH_TV, 0x0804);
+
+ r |= jbt_reg_write(ddata, JBT_REG_CKV_ON_OFF, 0x01);
+ r |= jbt_reg_write16(ddata, JBT_REG_CKV_1_2, 0x0000);
+
+ r |= jbt_reg_write16(ddata, JBT_REG_OEV_TIMING, 0x0d0e);
+ r |= jbt_reg_write16(ddata, JBT_REG_ASW_TIMING_1, 0x11a4);
+ r |= jbt_reg_write(ddata, JBT_REG_ASW_TIMING_2, 0x0e);
+
+ r |= jbt_reg_write_nodata(ddata, JBT_REG_DISPLAY_ON);
+
+ dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
+
+transfer_err:
+
+ return r ? -EIO : 0;
+}
+
+static void td028ttec1_panel_disable(struct omap_dss_device *dssdev)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+ struct omap_dss_device *in = ddata->in;
+
+ if (!omapdss_device_is_enabled(dssdev))
+ return;
+
+ dev_dbg(dssdev->dev, "td028ttec1_panel_disable()\n");
+
+ in->ops.dpi->disable(in);
+
+ jbt_reg_write_nodata(ddata, JBT_REG_DISPLAY_OFF);
+ jbt_reg_write16(ddata, JBT_REG_OUTPUT_CONTROL, 0x8002);
+ jbt_reg_write_nodata(ddata, JBT_REG_SLEEP_IN);
+ jbt_reg_write(ddata, JBT_REG_POWER_ON_OFF, 0x00);
+
+ dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
+}
+
+static void td028ttec1_panel_set_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+ struct omap_dss_device *in = ddata->in;
+
+ ddata->videomode = *timings;
+ dssdev->panel.timings = *timings;
+
+ in->ops.dpi->set_timings(in, timings);
+}
+
+static void td028ttec1_panel_get_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+
+ *timings = ddata->videomode;
+}
+
+static int td028ttec1_panel_check_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+ struct omap_dss_device *in = ddata->in;
+
+ return in->ops.dpi->check_timings(in, timings);
+}
+
+static struct omap_dss_driver td028ttec1_ops = {
+ .connect = td028ttec1_panel_connect,
+ .disconnect = td028ttec1_panel_disconnect,
+
+ .enable = td028ttec1_panel_enable,
+ .disable = td028ttec1_panel_disable,
+
+ .set_timings = td028ttec1_panel_set_timings,
+ .get_timings = td028ttec1_panel_get_timings,
+ .check_timings = td028ttec1_panel_check_timings,
+};
+
+static int td028ttec1_panel_probe_pdata(struct spi_device *spi)
+{
+ const struct panel_tpo_td028ttec1_platform_data *pdata;
+ struct panel_drv_data *ddata = dev_get_drvdata(&spi->dev);
+ struct omap_dss_device *dssdev, *in;
+
+ pdata = dev_get_platdata(&spi->dev);
+
+ in = omap_dss_find_output(pdata->source);
+ if (in = NULL) {
+ dev_err(&spi->dev, "failed to find video source '%s'\n",
+ pdata->source);
+ return -EPROBE_DEFER;
+ }
+
+ ddata->in = in;
+
+ ddata->data_lines = pdata->data_lines;
+
+ dssdev = &ddata->dssdev;
+ dssdev->name = pdata->name;
+
+ return 0;
+}
+
+static int td028ttec1_panel_probe(struct spi_device *spi)
+{
+ struct panel_drv_data *ddata;
+ struct omap_dss_device *dssdev;
+ int r;
+
+ dev_dbg(&spi->dev, "%s\n", __func__);
+
+ spi->bits_per_word = 9;
+ spi->mode = SPI_MODE_3;
+
+ r = spi_setup(spi);
+ if (r < 0) {
+ dev_err(&spi->dev, "spi_setup failed: %d\n", r);
+ return r;
+ }
+
+ ddata = devm_kzalloc(&spi->dev, sizeof(*ddata), GFP_KERNEL);
+ if (ddata = NULL)
+ return -ENOMEM;
+
+ dev_set_drvdata(&spi->dev, ddata);
+
+ ddata->spi_dev = spi;
+
+ if (dev_get_platdata(&spi->dev)) {
+ r = td028ttec1_panel_probe_pdata(spi);
+ if (r)
+ return r;
+ } else {
+ return -ENODEV;
+ }
+
+ ddata->videomode = td028ttec1_panel_timings;
+
+ dssdev = &ddata->dssdev;
+ dssdev->dev = &spi->dev;
+ dssdev->driver = &td028ttec1_ops;
+ dssdev->type = OMAP_DISPLAY_TYPE_DPI;
+ dssdev->owner = THIS_MODULE;
+ dssdev->panel.timings = ddata->videomode;
+ dssdev->phy.dpi.data_lines = ddata->data_lines;
+
+ r = omapdss_register_display(dssdev);
+ if (r) {
+ dev_err(&spi->dev, "Failed to register panel\n");
+ goto err_reg;
+ }
+
+ return 0;
+
+err_reg:
+ omap_dss_put_device(ddata->in);
+ return r;
+}
+
+static int td028ttec1_panel_remove(struct spi_device *spi)
+{
+ struct panel_drv_data *ddata = dev_get_drvdata(&spi->dev);
+ struct omap_dss_device *dssdev = &ddata->dssdev;
+ struct omap_dss_device *in = ddata->in;
+
+ dev_dbg(&ddata->spi_dev->dev, "%s\n", __func__);
+
+ omapdss_unregister_display(dssdev);
+
+ td028ttec1_panel_disable(dssdev);
+ td028ttec1_panel_disconnect(dssdev);
+
+ omap_dss_put_device(in);
+
+ return 0;
+}
+
+static struct spi_driver td028ttec1_spi_driver = {
+ .probe = td028ttec1_panel_probe,
+ .remove = td028ttec1_panel_remove,
+
+ .driver = {
+ .name = "panel-tpo-td028ttec1",
+ .owner = THIS_MODULE,
+ },
+};
+
+module_spi_driver(td028ttec1_spi_driver);
+
+MODULE_AUTHOR("H. Nikolaus Schaller <hns@goldelico.com>");
+MODULE_DESCRIPTION("Toppoly TD028TTEC1 panel driver");
+MODULE_LICENSE("GPL");
diff --git a/include/video/omap-panel-data.h b/include/video/omap-panel-data.h
index f7ac8d9..69279c0 100644
--- a/include/video/omap-panel-data.h
+++ b/include/video/omap-panel-data.h
@@ -238,4 +238,17 @@ struct panel_nec_nl8048hl11_platform_data {
int qvga_gpio;
};
+/**
+ * panel-tpo-td028ttec1 platform data
+ * @name: name for display entity
+ * @source: name of the display entity used as a video source
+ * @data_lines: number of DPI datalines
+ */
+struct panel_tpo_td028ttec1_platform_data {
+ const char *name;
+ const char *source;
+
+ int data_lines;
+};
+
#endif /* __OMAP_PANEL_DATA_H */
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH/RFC v3 00/19] Common Display Framework
From: Tomi Valkeinen @ 2013-10-17 12:55 UTC (permalink / raw)
To: Andrzej Hajda, 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: <525FD760.20506@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 4219 bytes --]
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.
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?
>>> 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 one scenario, when two panels are connected to single DSI-master.
> In such case both can call DSI ops, but I do not know how do you want to
> prevent it in case of your CDF-T implementation.
No, that was not the case I was describing. This was about a single panel.
If we have two independent APIs, we need to define how locking is
managed for those APIs. Even if in practice both APIs are used by the
same driver, and the driver can manage the locking, that's not really a
valid requirement. It'd be almost the same as requiring that gpio API
cannot be called at the same time as i2c API.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Thierry Reding @ 2013-10-17 12:55 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Tomi Valkeinen, Laurent Pinchart, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie,
Rob Clark, Daniel Vetter
In-Reply-To: <2414405.blLIekLlE8@avalon>
[-- Attachment #1: Type: text/plain, Size: 4135 bytes --]
On Thu, Oct 17, 2013 at 02:23:32PM +0200, Laurent Pinchart wrote:
> Hi Thierry,
>
> On Thursday 17 October 2013 14:06:47 Thierry Reding wrote:
> > On Thu, Oct 17, 2013 at 01:15:22PM +0200, Laurent Pinchart wrote:
> > > On Thursday 17 October 2013 13:05:18 Thierry Reding wrote:
> > > > On Thu, Oct 17, 2013 at 01:22:21PM +0300, Tomi Valkeinen wrote:
> > > > > On 17/10/13 11:53, Thierry Reding wrote:
> > > > > > I keep wondering why we absolutely must have compatibility between
> > > > > > CDF and this simple panel binding. DT content is supposed to concern
> > > > > > itself with the description of hardware only. What about the
> > > > > > following isn't an accurate description of the panel hardware?
> > > > > >
> > > > > > panel: panel {
> > > > > > compatible = "cptt,claa101wb01";
> > > > > >
> > > > > > power-supply = <&vdd_pnl_reg>;
> > > > > > enable-gpios = <&gpio 90 0>;
> > > > > >
> > > > > > backlight = <&backlight>;
> > > > > > };
> > > > > >
> > > > > > dsi-controller {
> > > > > > panel = <&panel>;
> > > > > > };
> > > > >
> > > > > That's quite similar to how my current out-of-mainline OMAP DSS DT
> > > > > bindings work. The difference is that I have a reference from the
> > > > > panel node to the video source (dsi-controller), instead of the other
> > > > > way around. I just find it more natural. It works the same way as,
> > > > > say, regulators, gpios, etc.
> > > >
> > > > I suppose that depends on the way you look at it. In the above proposal
> > > > I consider the output (DSI controller) to use the panel as a resource
> > > > that provides a certain set of services (query mode timings, provide a
> > > > way to enable or disable the panel). Similarly the panel uses the
> > > > backlight as a resource that provides a certain set of services (such as
> > > > changing the brightness).
> > > >
> > > > The above also follows the natural order of control. The panel has no
> > > > way to control the DSI output. However, it is the output that controls
> > > > when a panel is required and turn it on as appropriate.
> > >
> > > I'm no DSI expert, but I know enough about it to be sure that Tomi will
> > > disagree. DSI panels can have complex power sequences that require the
> > > input stream to be finely controlled (for instance it might require a
> > > clock without video frames for some time, switch a GPIO or regulator,
> > > send a command to the panel, and then only get video frames). For that
> > > reason all developers I've talked to who have an in-depth knowledge of
> > > DSI and DSI panels told me that the panel needs to control the video bus,
> > > and request the video source to start/stop the video stream.
> >
> > Oh, I'm very well aware of the various flavours of funkiness that DSI
> > panels come in. But it's wrong to say that the panel needs to control
> > the video bus. There's simply no way that a panel can actually do that.
> > It is true, however, that in order to make this work in a maintainable
> > fashion, the DSI panel *driver* may need to control the DSI bus. That's
> > an entirely different story.
>
> Sure, but I don't think that's really related to the DT bindings. We don't
> have to model every electrical signal in a detailed way that matches the
> direction of the electrons flow :-) What we need to model is a connection
> between a display controller and a panel (possibly with a direction). What I'd
> like to do is to express that link in a way that can also express more complex
> pipeline topologies. I don't want to make it overly complex, I had hoped that
> my DT bindings proposal would be a good approach as it's both generic and
> still pretty simple.
I get that, and for what it's worth I do think that your proposal looks
simple enough and if it can solve any of the problem you're facing with
CDF, then that's great.
But I don't think we should force inclusion of these properties on every
panel, even if it doesn't use any of the graph functionality. Is there
any problem with making them optional?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Thierry Reding @ 2013-10-17 12:46 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Laurent Pinchart, Tomi Valkeinen, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie
In-Reply-To: <1673630.A5yYaNBe3X@avalon>
[-- Attachment #1: Type: text/plain, Size: 12174 bytes --]
On Thu, Oct 17, 2013 at 02:14:45PM +0200, Laurent Pinchart wrote:
> On Thursday 17 October 2013 13:41:40 Thierry Reding wrote:
> > On Thu, Oct 17, 2013 at 01:02:38PM +0200, Laurent Pinchart wrote:
> > > On Thursday 17 October 2013 10:53:43 Thierry Reding wrote:
> > [...]
> >
> > > CDF needs to model video pipelines that can be more complex than just a
> > > display controller with a single output connected to a panel with a single
> > > input. For that reason the DT bindings need to describe connections in a
> > > generic enough way. The above DT fragment might be slightly more complex
> > > than one would expect when thinking about the really simple case only,
> > > but it's nowhere near overly complex in my opinion.
> > >
> > > Please note that, when a device has as single port, the ports node can be
> > > omitted, and the port doesn't need to be numbered. You would then end up
> > > with>
> > > dc: display-controller {
> > > port {
> > > remote-endpoint = <&panel>;
> > > };
> > > };
> > >
> > > panel: panel {
> > > port {
> > > remote-endpoint = <&dc>;
> > > };
> > > };
> > >
> > > I don't think there's a way to simplify it further.
> >
> > That binding looks completely inconsistent to me. If we can write it either
> > using a ports node with port@X subnodes, in my opinion we can equally well
> > just use a unidirectional link for the case where we don't need the link
> > back from the panel to the output. Where's the difference?
> >
> > Consider for example the case where an LVDS panel is connected to a simple
> > LVDS output. For what possible reason would the panel need to access the
> > output?
>
> The point is that the video pipeline must be described in DT. Having a per-
> device way to represent connections would be a nightmare to support from an
> implementation point of view, hence the need for a generic way to describe
> them.
Okay, so we're back to the need to describe pipelines in DT. At the risk
of sounding selfish: I don't care about pipelines. I just want us to
settle on a way to represent a dumb panel in DT so that it can be
enabled when it needs to. Furthermore I don't have any hardware that
exhibits any of these "advanced" features, so I'm totally unqualified to
work on any of this.
Can we please try to be a little pragmatic here and solve one problem at
a time? I am aware that solving this for panels may require some amount
of foresight, but let's not try to solve everything at once at the risk
of not getting anything done at all.
> > > I'll still keep trying of course, but not for years.I have yet to
> > > see someone proposing a viable solution to share drivers between DRM and
> > > V4L2, which is a real pressing requirement, partly due to DT.
> >
> > Yeah... I still don't get that side of the argument. Why exactly do we need
> > to share the drivers again?
>
> Think about a scaler IP in a SoC or FPGA, with one instance used in a camera
> capture pipeline, supported by a V4L2 driver, and one instance used in a video
> output pipeline, supported by a DRM driver. We want a single driver for that
> IP core. Same story for video encoders.
Yes, I see. This doesn't immediately concern the simple panel problem
that I'm trying to solve, so perhaps we can have a separate discussion
about it. Perhaps this would more easily be solved by providing some
sort of helper library for the lower level control of the device and
wrapping that with V4L2 or DRM APIs.
> > If we really need to support display output within V4L2, shouldn't we
> > instead improve interoperability between the two and use DRM/KMS exclusively
> > for the output part? Then the problem of having to share drivers between
> > subsystems goes away and we'll actually be using the right subsystem for the
> > right job at hand.
>
> In the long term we definitely should improve interoperability between the
> two. I'd go further than that, having one API for video capture and one API
> for video output doesn't make much sense, except for historical reasons.
Having two separate subsystems has worked out pretty well so far. In my
opinion having strong boundaries between subsystems helps with design.
> > Of course none of that is relevant to the topic of DT bindings, which is
> > what we should probably be concentrating on.
>
> It actually is. Given that DT bindings must describe hardware, the scaler (or
> encoder, or other entity) would use the same bindings regardless of the Linux
> subsystem that will be involved. We thus need to make sure the bindings will
> be usable on both sides.
Perhaps a single driver could expose both interfaces?
> > I'm not at all opposed to the idea of CDF or the problems it tries to
> > address. I don't think it necessarily should be a separate framework for
> > reasons explained earlier. But even if it were to end up in a separate
> > framework there's absolutely nothing preventing us from adding a DRM panel
> > driver that hooks into CDF as a "backend" of sorts for panels that require
> > something more complex than the simple-panel binding.
>
> Once again it's not about panel having complex needs, but more about using
> simple panels in complex pipelines. I'm fine with the drm_panel
> infrastructure, what I would like is DT bindings that describe connections in
> a more future-proof way. The rest is fine.
And I've already said elsewhere that the bindings in their current form
are easily extensible to cater for the needs of CDF.
> > But that's precisely the point. Why would we need to go back from the panel
> > to the display controller? Especially for dumb panels that can't or don't
> > have to be configured in any way. Even if they needed some sort of setup,
> > why can't that be done from the display controller/output.
> >
> > Even given a setup where a DSI controller needs to write some registers in a
> > panel upon initialization, I don't see why the reverse connection needs to
> > be described. The fact alone that an output dereferences a panel's phandle
> > should be enough to connect both of them and have any panel driver use the
> > DSI controller that it's been attached to for the programming.
> >
> > That's very much analogous to how I2C works. There are no connections back
> > to the I2C master from the slaves. Yet each I2C client driver manages to use
> > the services provided by the I2C master to perform transactions on the I2C
> > bus. In a similar way the DSI controller is the bus master that talks to DSI
> > panels. DSI panels don't actively talk to the DSI controller.
>
> It's all about modeling video pipeline graphs in DT. To be able to walk the
> graph we need to describe connections. Not having bidirectional information
> means that we restrict the points at which we can start walking the graph,
> potentially making our life much more difficult in the future.
>
> What's so wrong about adding a port node and link information to the panel DT
> node ? It describe what's there: the panel has one input, why not make that
> explicit ?
What's wrong with it is that there's no way to verify the soundness of
the design by means of a full implementation because we're missing the
majority of the pieces. Just putting the nodes into DT to provide some
imaginary future-proofness isn't helpful either. Without any code that
actually uses them they are useless.
And again, why should we add them right away (while not needed) when
they can easily be added in a backwards-compatible manner at some later
point when there's actually any use for them and they can actually be
tested in some larger framework?
> > > > > > +static void panel_simple_enable(struct drm_panel *panel)
> > > > > > +{
> > > > > > + struct panel_simple *p = to_panel_simple(panel);
> > > > > > + int err;
> > > > > > +
> > > > > > + if (p->enabled)
> > > > > > + return;
> > > > > > +
> > > > > > + err = regulator_enable(p->supply);
> > > > > > + if (err < 0)
> > > > > > + dev_err(panel->dev, "failed to enable supply: %d\n", err);
> > > > >
> > > > > Is that really a non-fatal error ? Shouldn't the enable operation
> > > > > return an int ?
> > > >
> > > > There's no way to propagate this in DRM, so why go through the trouble
> > > > of returning the error? Furthermore, there's nothing that the caller
> > > > could do to remedy the situation anyway.
> > >
> > > That's a DRM issue, which could be fixed. While the caller can't remedy
> > > the situation, it should at least report the error to the application
> > > instead of silently ignoring it.
> >
> > Perhaps. It's not really relevant to the discussion and can always be
> > fixed in a subsequent patch.
>
> Most things can be fixed by a subsequent patch, that's not a good enough
> reason not to address the known problems before pushing the code to mainline
> (to clarify my point of view, there's no need to fix DRM to use the return
> value before pushing this patch set to mainline, but I'd like a v2 with an int
> return value).
Why? What's the use of returning an error if you know up front that it
can't be used? This should be changed if or when we "fix" DRM to
propagate errors.
> > > > > Instead of hardcoding the modes in the driver, which would then
> > > > > require to be updated for every new simple panel model (and we know
> > > > > there are lots of them), why don't you specify the modes in the panel
> > > > > DT node ? The simple panel driver would then become much more generic.
> > > > > It would also allow board designers to tweak h/v sync timings
> > > > > depending on the system requirements.
> > > >
> > > > Sigh... we keep second-guessing ourselves. Back at the time when power
> > > > sequences were designed (and NAKed at the last minute), we all decided
> > > > that the right thing to do would be to use specific compatible values
> > > > for individual panels, because that would allow us to encode the power
> > > > sequencing within the driver. And when we already have the panel model
> > > > encoded in the compatible value, we might just as well encode the mode
> > > > within the driver for that panel. Otherwise we'll have to keep adding
> > > > the same mode timings for every board that uses the same panel.
> > > >
> > > > I do agree though that it might be useful to tweak the mode in case the
> > > > default one doesn't work. How about we provide a means to override the
> > > > mode encoded in the driver using one specified in the DT? That's
> > > > obviously a backwards-compatible change, so it could be added if or when
> > > > it becomes necessary.
> > >
> > > I share Tomi's point of view here. Your three panels use the same power
> > > sequence, so I believe we should have a generic panel compatible string
> > > that would use modes described in DT for the common case. Only if a panel
> > > required something more complex which can't (or which could, but won't
> > > for any reason) accurately be described in DT would you need to extend
> > > the driver.
> >
> > I don't see the point quite frankly. You seem to be assuming that every
> > panel will only be used in a single board.
>
> No, but in practice that's often the case, at least for boards with .dts files
> in the mainline kernel.
>
> > However what you're proposing will require the mode timings to be repeated
> > in every board's DT file, if the same panel is ever used on more than a
> > single board.
>
> Is that a problem ? You could #include a .dtsi for the panel that would
> specify the video mode if you want to avoid multiple copies.
Yes, I don't think it's desirable to duplicate data needlessly in DT. It
also makes the binding more difficult to use. If I know that the panel
is one supported by the simple-panel binding, I can just go and add the
right compatible value without having to bother looking up the mode
timings and duplicating them. That way DT writers get to concern
themselves only with the variable data.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Tomi Valkeinen @ 2013-10-17 12:32 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Thierry Reding, Laurent Pinchart, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie
In-Reply-To: <3768216.eiA2v5KI6a@avalon>
[-- Attachment #1: Type: text/plain, Size: 2463 bytes --]
On 17/10/13 15:17, Laurent Pinchart wrote:
> Hi Tomi,
>
> 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.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 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