* Re: [PATCH 1/8] OMAPDSS: HDMI: Move GPIO handling to HDMI driver
From: Archit Taneja @ 2012-08-24 6:12 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, Tony Lindgren
In-Reply-To: <1345729514-2441-2-git-send-email-tomi.valkeinen@ti.com>
On Thursday 23 August 2012 07:15 PM, Tomi Valkeinen wrote:
> We currently manage HDMI GPIOs in the board files via
> platform_enable/disable calls. This won't work with device tree, and in
> any case the correct place to manage the GPIOs is in the HDMI driver.
>
> This patch moves the handling of the GPIOs to the HDMI driver. The GPIO
> handling is moved to the common hdmi.c file, and this probably needs to
> be revisited when adding OMAP5 HDMI support to see if the GPIO handling
> needs to be moved to IP specific files.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> ---
> arch/arm/mach-omap2/board-4430sdp.c | 27 +-----------
> arch/arm/mach-omap2/board-omap4panda.c | 27 +-----------
> drivers/video/omap2/dss/hdmi.c | 75 +++++++++++++++++++++++---------
> include/video/omapdss.h | 2 +
> 4 files changed, 61 insertions(+), 70 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
> index 8e17284..852e05c 100644
> --- a/arch/arm/mach-omap2/board-4430sdp.c
> +++ b/arch/arm/mach-omap2/board-4430sdp.c
> @@ -601,29 +601,6 @@ static void __init omap_sfh7741prox_init(void)
> __func__, OMAP4_SFH7741_ENABLE_GPIO, error);
> }
>
> -static struct gpio sdp4430_hdmi_gpios[] = {
> - { HDMI_GPIO_CT_CP_HPD, GPIOF_OUT_INIT_HIGH, "hdmi_gpio_ct_cp_hpd" },
> - { HDMI_GPIO_LS_OE, GPIOF_OUT_INIT_HIGH, "hdmi_gpio_ls_oe" },
> - { HDMI_GPIO_HPD, GPIOF_DIR_IN, "hdmi_gpio_hpd" },
> -};
> -
> -static int sdp4430_panel_enable_hdmi(struct omap_dss_device *dssdev)
> -{
> - int status;
> -
> - status = gpio_request_array(sdp4430_hdmi_gpios,
> - ARRAY_SIZE(sdp4430_hdmi_gpios));
> - if (status)
> - pr_err("%s: Cannot request HDMI GPIOs\n", __func__);
> -
> - return status;
> -}
> -
> -static void sdp4430_panel_disable_hdmi(struct omap_dss_device *dssdev)
> -{
> - gpio_free_array(sdp4430_hdmi_gpios, ARRAY_SIZE(sdp4430_hdmi_gpios));
> -}
> -
> static struct nokia_dsi_panel_data dsi1_panel = {
> .name = "taal",
> .reset_gpio = 102,
> @@ -718,6 +695,8 @@ static struct omap_dss_device sdp4430_lcd2_device = {
> };
>
> static struct omap_dss_hdmi_data sdp4430_hdmi_data = {
> + .ct_cp_hpd_gpio = HDMI_GPIO_CT_CP_HPD,
> + .ls_oe_gpio = HDMI_GPIO_LS_OE,
> .hpd_gpio = HDMI_GPIO_HPD,
> };
>
> @@ -725,8 +704,6 @@ static struct omap_dss_device sdp4430_hdmi_device = {
> .name = "hdmi",
> .driver_name = "hdmi_panel",
> .type = OMAP_DISPLAY_TYPE_HDMI,
> - .platform_enable = sdp4430_panel_enable_hdmi,
> - .platform_disable = sdp4430_panel_disable_hdmi,
> .channel = OMAP_DSS_CHANNEL_DIGIT,
> .data = &sdp4430_hdmi_data,
> };
> diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
> index 982fb26..5415faa 100644
> --- a/arch/arm/mach-omap2/board-omap4panda.c
> +++ b/arch/arm/mach-omap2/board-omap4panda.c
> @@ -405,30 +405,9 @@ static struct omap_dss_device omap4_panda_dvi_device = {
> .channel = OMAP_DSS_CHANNEL_LCD2,
> };
>
> -static struct gpio panda_hdmi_gpios[] = {
> - { HDMI_GPIO_CT_CP_HPD, GPIOF_OUT_INIT_HIGH, "hdmi_gpio_ct_cp_hpd" },
> - { HDMI_GPIO_LS_OE, GPIOF_OUT_INIT_HIGH, "hdmi_gpio_ls_oe" },
> - { HDMI_GPIO_HPD, GPIOF_DIR_IN, "hdmi_gpio_hpd" },
> -};
> -
> -static int omap4_panda_panel_enable_hdmi(struct omap_dss_device *dssdev)
> -{
> - int status;
> -
> - status = gpio_request_array(panda_hdmi_gpios,
> - ARRAY_SIZE(panda_hdmi_gpios));
> - if (status)
> - pr_err("Cannot request HDMI GPIOs\n");
> -
> - return status;
> -}
> -
> -static void omap4_panda_panel_disable_hdmi(struct omap_dss_device *dssdev)
> -{
> - gpio_free_array(panda_hdmi_gpios, ARRAY_SIZE(panda_hdmi_gpios));
> -}
> -
> static struct omap_dss_hdmi_data omap4_panda_hdmi_data = {
> + .ct_cp_hpd_gpio = HDMI_GPIO_CT_CP_HPD,
> + .ls_oe_gpio = HDMI_GPIO_LS_OE,
> .hpd_gpio = HDMI_GPIO_HPD,
> };
>
> @@ -436,8 +415,6 @@ static struct omap_dss_device omap4_panda_hdmi_device = {
> .name = "hdmi",
> .driver_name = "hdmi_panel",
> .type = OMAP_DISPLAY_TYPE_HDMI,
> - .platform_enable = omap4_panda_panel_enable_hdmi,
> - .platform_disable = omap4_panda_panel_disable_hdmi,
> .channel = OMAP_DSS_CHANNEL_DIGIT,
> .data = &omap4_panda_hdmi_data,
> };
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index 0cdf246..4fbe271 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -32,6 +32,7 @@
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/clk.h>
> +#include <linux/gpio.h>
> #include <video/omapdss.h>
>
> #include "ti_hdmi.h"
> @@ -61,6 +62,10 @@ static struct {
> struct hdmi_ip_data ip_data;
>
> struct clk *sys_clk;
> +
> + int ct_cp_hpd_gpio;
> + int ls_oe_gpio;
> + int hpd_gpio;
> } hdmi;
>
> /*
> @@ -314,12 +319,34 @@ static void hdmi_runtime_put(void)
>
> static int __init hdmi_init_display(struct omap_dss_device *dssdev)
> {
> + int r;
> +
> + struct gpio gpios[] = {
> + { hdmi.ct_cp_hpd_gpio, GPIOF_OUT_INIT_LOW, "hdmi_ct_cp_hpd" },
> + { hdmi.ls_oe_gpio, GPIOF_OUT_INIT_LOW, "hdmi_ls_oe" },
> + { hdmi.hpd_gpio, GPIOF_DIR_IN, "hdmi_hpd" },
> + };
> +
> DSSDBG("init_display\n");
>
> dss_init_hdmi_ip_ops(&hdmi.ip_data);
> +
> + r = gpio_request_array(gpios, ARRAY_SIZE(gpios));
> + if (r)
> + return r;
> +
Is there a reason to request these gpios in hdmi_init_display()? Why
can't these be requested simply in the probe of the hdmi platform
device. These gpios are sort of specific to the hdmi output on OMAP,
aren't they?
<snip>
Archit
^ permalink raw reply
* Re: [PATCH 1/8] OMAPDSS: HDMI: Move GPIO handling to HDMI driver
From: Tomi Valkeinen @ 2012-08-24 6:28 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev, Tony Lindgren
In-Reply-To: <50371899.6030706@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2469 bytes --]
On Fri, 2012-08-24 at 11:30 +0530, Archit Taneja wrote:
> On Thursday 23 August 2012 07:15 PM, Tomi Valkeinen wrote:
> > We currently manage HDMI GPIOs in the board files via
> > platform_enable/disable calls. This won't work with device tree, and in
> > any case the correct place to manage the GPIOs is in the HDMI driver.
> >
> > This patch moves the handling of the GPIOs to the HDMI driver. The GPIO
> > handling is moved to the common hdmi.c file, and this probably needs to
> > be revisited when adding OMAP5 HDMI support to see if the GPIO handling
> > needs to be moved to IP specific files.
<snip>
> > static int __init hdmi_init_display(struct omap_dss_device *dssdev)
> > {
> > + int r;
> > +
> > + struct gpio gpios[] = {
> > + { hdmi.ct_cp_hpd_gpio, GPIOF_OUT_INIT_LOW, "hdmi_ct_cp_hpd" },
> > + { hdmi.ls_oe_gpio, GPIOF_OUT_INIT_LOW, "hdmi_ls_oe" },
> > + { hdmi.hpd_gpio, GPIOF_DIR_IN, "hdmi_hpd" },
> > + };
> > +
> > DSSDBG("init_display\n");
> >
> > dss_init_hdmi_ip_ops(&hdmi.ip_data);
> > +
> > + r = gpio_request_array(gpios, ARRAY_SIZE(gpios));
> > + if (r)
> > + return r;
> > +
>
> Is there a reason to request these gpios in hdmi_init_display()? Why
> can't these be requested simply in the probe of the hdmi platform
> device. These gpios are sort of specific to the hdmi output on OMAP,
> aren't they?
Well, it is a bit ugly, agreed. But I'm not sure it can be helped
easily.
First, the struct omap_dss_hdmi_data where the gpios are passed from
board file, is really "panel" platform data. I.e. it's attached to the
hdmi dssdev, and thus we can't handle it in hdmi output driver's probe,
as we don't have dssdevs there.
Second, the gpios are actually not OMAP HDMI stuff. They belong to the
tpd12s015 chip, sitting between OMAP HDMI output and the HDMI connector,
which is the ESD/level shifter/whatnot (I don't remember exactly what it
does =). So making the GPIOs a property of the OMAP HDMI device wouldn't
be right either.
tpd12s015 is rather simple chip, but it still has the gpios and require
special handling. I think the only way to properly handle this would be
to have a chain of external display devices, and the tpd12s015 would be
handled by a separate driver. Until then, the gpio handling is quite
hacky.
We could move the gpios to omapdss's platform data, but it would still
be wrong, and I'm not sure if it'd be any cleaner hack.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 6/8] OMAPDSS: DSI: calculate dsi clock
From: Archit Taneja @ 2012-08-24 6:28 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1345729514-2441-7-git-send-email-tomi.valkeinen@ti.com>
On Thursday 23 August 2012 07:15 PM, Tomi Valkeinen wrote:
> Currently the way to configure clocks related to DSI (both DSI and DISPC
> clocks) happens via omapdss platform data. The reason for this is that
> configuring the DSS clocks is a very complex problem, and it's
> impossible for the SW to know requirements about things like
> interference.
>
> However, for general cases it should be fine to calculate the dividers
> for clocks in the SW. The calculated clocks are probably not perfect,
> but should work.
>
> This patch adds support to calculate the dividers when using DSI command
> mode panels. The panel gives the required DDR clock rate and LP clock
> rate, and the DSI driver configures itself and DISPC accordingly.
>
> This patch is somewhat ugly, though. The code does its job by modifying
> the platform data where the clock dividers would be if the board file
> gave them. This is not how it's going to be in the future, but allows us
> to have quite simple patch and keep the backward compatibility.
>
> It also allows the developer to still give the exact dividers from the
> board file when there's need for that, as long as the panel driver does
> not override them.
>
> There are also other areas for improvement. For example, it would be
> better if the panel driver could ask for a DSI clock in a certain range,
> as, at least command mode panels, the panel can work fine with many
> different clock speeds.
>
> While the patch is not perfect, it allows us to remove the hardcoded
> clock dividers from the board file, making it easier to bring up a new
> panel and to use device tree from omapdss.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/displays/panel-taal.c | 6 ++
> drivers/video/omap2/dss/dsi.c | 126 +++++++++++++++++++++++++++++
> include/video/omapdss.h | 2 +
> 3 files changed, 134 insertions(+)
>
> diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
> index 77aed0e..ddda96a 100644
> --- a/drivers/video/omap2/displays/panel-taal.c
> +++ b/drivers/video/omap2/displays/panel-taal.c
> @@ -1065,6 +1065,12 @@ static int taal_power_on(struct omap_dss_device *dssdev)
> omapdss_dsi_set_pixel_format(dssdev, OMAP_DSS_DSI_FMT_RGB888);
> omapdss_dsi_set_operation_mode(dssdev, OMAP_DSS_DSI_CMD_MODE);
>
> + r = omapdss_dsi_set_clocks(dssdev, 216000000, 10000000);
> + if (r) {
> + dev_err(&dssdev->dev, "failed to set HS and LP clocks\n");
> + goto err0;
> + }
> +
> r = omapdss_dsi_display_enable(dssdev);
> if (r) {
> dev_err(&dssdev->dev, "failed to enable DSI\n");
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index 96d0024..340c832 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -1454,6 +1454,68 @@ found:
> return 0;
> }
>
> +static int dsi_pll_calc_ddrfreq(struct platform_device *dsidev,
> + unsigned long req_clk, struct dsi_clock_info *cinfo)
> +{
> + struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
> + struct dsi_clock_info cur, best;
> + unsigned long dss_sys_clk, max_dss_fck, max_dsi_fck;
> + unsigned long req_clkin4ddr;
> +
> + DSSDBG("dsi_pll_calc_ddrfreq\n");
> +
> + dss_sys_clk = clk_get_rate(dsi->sys_clk);
> +
> + max_dss_fck = dss_feat_get_param_max(FEAT_PARAM_DSS_FCK);
> + max_dsi_fck = dss_feat_get_param_max(FEAT_PARAM_DSI_FCK);
> +
> + memset(&best, 0, sizeof(best));
> + memset(&cur, 0, sizeof(cur));
> +
> + cur.clkin = dss_sys_clk;
> +
> + req_clkin4ddr = req_clk * 4;
> +
> + for (cur.regn = 1; cur.regn < dsi->regn_max; ++cur.regn) {
> + cur.fint = cur.clkin / cur.regn;
> +
> + if (cur.fint > dsi->fint_max || cur.fint < dsi->fint_min)
> + continue;
> +
> + /* DSIPHY(MHz) = (2 * regm / regn) * clkin */
> + for (cur.regm = 1; cur.regm < dsi->regm_max; ++cur.regm) {
> + unsigned long a, b;
> +
> + a = 2 * cur.regm * (cur.clkin/1000);
> + b = cur.regn;
> + cur.clkin4ddr = a / b * 1000;
> +
> + if (cur.clkin4ddr > 1800 * 1000 * 1000)
> + break;
> +
> + if (abs(cur.clkin4ddr - req_clkin4ddr) <
> + abs(best.clkin4ddr - req_clkin4ddr)) {
> + best = cur;
> + DSSDBG("best %ld\n", best.clkin4ddr);
> + }
> +
> + if (cur.clkin4ddr = req_clkin4ddr)
> + goto found;
> + }
> + }
> +found:
> + best.regm_dispc = DIV_ROUND_UP(best.clkin4ddr, max_dss_fck);
> + best.dsi_pll_hsdiv_dispc_clk = best.clkin4ddr / best.regm_dispc;
> +
> + best.regm_dsi = DIV_ROUND_UP(best.clkin4ddr, max_dsi_fck);
> + best.dsi_pll_hsdiv_dsi_clk = best.clkin4ddr / best.regm_dsi;
> +
> + if (cinfo)
> + *cinfo = best;
> +
> + return 0;
> +}
> +
> int dsi_pll_set_clock_div(struct platform_device *dsidev,
> struct dsi_clock_info *cinfo)
> {
> @@ -4110,6 +4172,70 @@ int omapdss_dsi_configure_pins(struct omap_dss_device *dssdev,
> }
> EXPORT_SYMBOL(omapdss_dsi_configure_pins);
>
> +int omapdss_dsi_set_clocks(struct omap_dss_device *dssdev,
> + unsigned long ddr_clk, unsigned long lp_clk)
> +{
> + struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> + struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
> + struct dsi_clock_info cinfo;
> + struct dispc_clock_info dispc_cinfo;
> + unsigned lp_clk_div;
> + unsigned long dsi_fclk;
> + int bpp = dsi_get_pixel_size(dssdev->panel.dsi_pix_fmt);
> + unsigned long pck;
> + int r;
> +
> + DSSDBGF("ddr_clk %lu, lp_clk %lu", ddr_clk, lp_clk);
> +
> + mutex_lock(&dsi->lock);
> +
> + r = dsi_pll_calc_ddrfreq(dsidev, ddr_clk, &cinfo);
> + if (r)
> + goto err;
> +
> + dssdev->clocks.dsi.regn = cinfo.regn;
> + dssdev->clocks.dsi.regm = cinfo.regm;
> + dssdev->clocks.dsi.regm_dispc = cinfo.regm_dispc;
> + dssdev->clocks.dsi.regm_dsi = cinfo.regm_dsi;
> +
> +
> + dsi_fclk = cinfo.dsi_pll_hsdiv_dsi_clk;
> + lp_clk_div = DIV_ROUND_UP(dsi_fclk, lp_clk * 2);
> +
> + dssdev->clocks.dsi.lp_clk_div = lp_clk_div;
> +
> + /* pck = TxByteClkHS * datalanes * 8 / bitsperpixel */
This formula looks a bit simplified, we aren't considering the header
and footers of long packets that will add to the DDR clock. But I guess
not considering these would only give a higher pixel clock than needed,
which isn't that bad.
> +
> + pck = cinfo.clkin4ddr / 16 * (dsi->num_lanes_used - 1) * 8 / bpp;
> +
> + DSSDBG("finding dispc dividers for pck %lu\n", pck);
> +
> + dispc_find_clk_divs(pck, cinfo.dsi_pll_hsdiv_dispc_clk, &dispc_cinfo);
> +
> + dssdev->clocks.dispc.channel.lck_div = dispc_cinfo.lck_div;
> + dssdev->clocks.dispc.channel.pck_div = dispc_cinfo.pck_div;
> +
> +
one unnecessary new line above.
<snip>
Archit
^ permalink raw reply
* Re: [PATCH 4/8] OMAPDSS: HDMI: use vdda_hdmi_dac
From: Archit Taneja @ 2012-08-24 6:32 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1345729514-2441-5-git-send-email-tomi.valkeinen@ti.com>
On Thursday 23 August 2012 07:15 PM, Tomi Valkeinen wrote:
> The HDMI driver requires vdda_hdmi_dac power for operation, but does not
> enable it. This has worked because the regulator has been always
> enabled.
>
> But this may not always be the case, as I encountered when implementing
> HDMI device tree support.
>
> This patch changes the HDMI driver to use the vdda_hdmi_dac.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/hdmi.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index 96a6e29..ccfc677 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -33,6 +33,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/clk.h>
> #include <linux/gpio.h>
> +#include <linux/regulator/consumer.h>
> #include <video/omapdss.h>
>
> #include "ti_hdmi.h"
> @@ -62,6 +63,7 @@ static struct {
> struct hdmi_ip_data ip_data;
>
> struct clk *sys_clk;
> + struct regulator *vdda_hdmi_dac_reg;
>
> int ct_cp_hpd_gpio;
> int ls_oe_gpio;
> @@ -331,6 +333,19 @@ static int __init hdmi_init_display(struct omap_dss_device *dssdev)
>
> dss_init_hdmi_ip_ops(&hdmi.ip_data);
>
> + if (hdmi.vdda_hdmi_dac_reg = NULL) {
> + struct regulator *reg;
> +
> + reg = devm_regulator_get(&hdmi.pdev->dev, "vdda_hdmi_dac");
There is no corresponding devm_regulator_put() call here, I guess that's
what devm_* calls are supposed to help us with. But the only place I saw
the usage of dev_regulator_get() is here:
sound/soc/ux500/ux500_msp_dai.c
And here, they are doing devm_regulator_put() calls to, so I was
wondering what the deal is.
Archit
^ permalink raw reply
* Re: [PATCH 4/8] OMAPDSS: HDMI: use vdda_hdmi_dac
From: Tomi Valkeinen @ 2012-08-24 6:41 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <50371D27.1010707@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2703 bytes --]
On Fri, 2012-08-24 at 11:50 +0530, Archit Taneja wrote:
> On Thursday 23 August 2012 07:15 PM, Tomi Valkeinen wrote:
> > The HDMI driver requires vdda_hdmi_dac power for operation, but does not
> > enable it. This has worked because the regulator has been always
> > enabled.
> >
> > But this may not always be the case, as I encountered when implementing
> > HDMI device tree support.
> >
> > This patch changes the HDMI driver to use the vdda_hdmi_dac.
> >
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > ---
> > drivers/video/omap2/dss/hdmi.c | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> > index 96a6e29..ccfc677 100644
> > --- a/drivers/video/omap2/dss/hdmi.c
> > +++ b/drivers/video/omap2/dss/hdmi.c
> > @@ -33,6 +33,7 @@
> > #include <linux/pm_runtime.h>
> > #include <linux/clk.h>
> > #include <linux/gpio.h>
> > +#include <linux/regulator/consumer.h>
> > #include <video/omapdss.h>
> >
> > #include "ti_hdmi.h"
> > @@ -62,6 +63,7 @@ static struct {
> > struct hdmi_ip_data ip_data;
> >
> > struct clk *sys_clk;
> > + struct regulator *vdda_hdmi_dac_reg;
> >
> > int ct_cp_hpd_gpio;
> > int ls_oe_gpio;
> > @@ -331,6 +333,19 @@ static int __init hdmi_init_display(struct omap_dss_device *dssdev)
> >
> > dss_init_hdmi_ip_ops(&hdmi.ip_data);
> >
> > + if (hdmi.vdda_hdmi_dac_reg == NULL) {
> > + struct regulator *reg;
> > +
> > + reg = devm_regulator_get(&hdmi.pdev->dev, "vdda_hdmi_dac");
>
> There is no corresponding devm_regulator_put() call here, I guess that's
> what devm_* calls are supposed to help us with. But the only place I saw
> the usage of dev_regulator_get() is here:
>
> sound/soc/ux500/ux500_msp_dai.c
>
> And here, they are doing devm_regulator_put() calls to, so I was
> wondering what the deal is.
No idea. But there are other places also: sound/soc/codecs/wm5100.c,
sound/soc/codecs/wm8996.c, sound/soc/soc-dapm.c, and those don't use
put().
Anyway, I think it's quite clear how devm_* functions are used, so the
put call in ux500_msp_dai.c is probably just a mistake.
I also want to mention that doing the regulator_get in
hdmi_init_display() is somewhat strange, but the point is to do the
regulator_get only if a hdmi display has been defined in the board file.
If we did it unconditionally in hdmi's probe, we'd try to get the
regulator always, and it could be that there's no regulator if the hdmi
is not used on a particular board. This is again something that feels
hackish, and I'd like to find a cleaner solution to it.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 6/8] OMAPDSS: DSI: calculate dsi clock
From: Tomi Valkeinen @ 2012-08-24 8:55 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <50371C47.30708@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1143 bytes --]
On Fri, 2012-08-24 at 11:46 +0530, Archit Taneja wrote:
> On Thursday 23 August 2012 07:15 PM, Tomi Valkeinen wrote:
> > + /* pck = TxByteClkHS * datalanes * 8 / bitsperpixel */
>
> This formula looks a bit simplified, we aren't considering the header
> and footers of long packets that will add to the DDR clock. But I guess
> not considering these would only give a higher pixel clock than needed,
> which isn't that bad.
Hmm. The TRM (omap4460) gives this formula in "10.3.4.5.12 How to
Configure the DSI PLL in Video Mode". The headers/footers etc. are
handled with adjusting the blanking periods so that DISPC and DSI Tline
times match.
But obviously they are not used for command mode transfers, so perhaps
you have a point there. Then again, at least in theory, in command mode
the DISPC pck should be configurable as high as possible because the
stall mechanism should stop DISPC when DSI has had enough. And so the
pck calculation is a bit unneeded for cmd mode, we could just configure
pck to max.
But if it's correct for video mode, and very close for cmd mode, I guess
it should be fine?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-08-24 9:24 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Thierry Reding, Stephen Warren, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org
In-Reply-To: <1345542860.4085.40.camel@deskari>
On Tuesday 21 August 2012 17:54:20 Tomi Valkeinen wrote:
> > However this also means we'll essentially just be moving the board code.
>
>
> What do you mean "just"? Wasn't the point of the whole "arm board file
> mess" to get rid of the code from the board files? If the code in the
> board file is device specific code, not board specific, then the driver
> of the device is a logical place to place it.
I think Tomi has a point here - these sequences were not belonging to the
board code in the first place. They are definitely tied to the device, hence
should have been handled by the driver all along, with the board code
assigning the correct resources to the device (like the vast majority of
device drivers do).
> And as I said, I don't have any problems with some kind of generic power
> sequences. So the code in the board file could be moved and converted to
> use the power sequences, if that is better than just plain c code.
My concern now is, provided that all drivers to their job and handle how their
devices are switched on and off, when (if at all) are encoded power sequences
better than their equivalent C code? There is the matching database size issue
that you mentionned, is it a sufficient concern to justify a new kernel feature?
On the other hand some devices like panels are typically not used in many
different appliances, so maybe it is not worth to separate them from their
board definition. As Mark mentionned, having .dtsi files for the DT (and their
equivalent .h for kernels that use platform data) might be a good middle
ground.
But the line is really tight between what is code and what is data here.
Alex.
^ permalink raw reply
* Re: [PATCH 6/8] OMAPDSS: DSI: calculate dsi clock
From: Archit Taneja @ 2012-08-24 9:29 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1345798540.2614.17.camel@deskari>
On Friday 24 August 2012 02:25 PM, Tomi Valkeinen wrote:
> On Fri, 2012-08-24 at 11:46 +0530, Archit Taneja wrote:
>> On Thursday 23 August 2012 07:15 PM, Tomi Valkeinen wrote:
>
>>> + /* pck = TxByteClkHS * datalanes * 8 / bitsperpixel */
>>
>> This formula looks a bit simplified, we aren't considering the header
>> and footers of long packets that will add to the DDR clock. But I guess
>> not considering these would only give a higher pixel clock than needed,
>> which isn't that bad.
>
> Hmm. The TRM (omap4460) gives this formula in "10.3.4.5.12 How to
> Configure the DSI PLL in Video Mode". The headers/footers etc. are
> handled with adjusting the blanking periods so that DISPC and DSI Tline
> times match.
>
> But obviously they are not used for command mode transfers, so perhaps
> you have a point there. Then again, at least in theory, in command mode
> the DISPC pck should be configurable as high as possible because the
> stall mechanism should stop DISPC when DSI has had enough. And so the
> pck calculation is a bit unneeded for cmd mode, we could just configure
> pck to max.
>
> But if it's correct for video mode, and very close for cmd mode, I guess
> it should be fine?
Yes, it's fine, and we shouldn't try to have an unnecessarily high pixel
clock in command mode anyway, that would reduce the amount of
downscaling we could do.
Archit
^ permalink raw reply
* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-08-24 10:34 UTC (permalink / raw)
To: Alex Courbot
Cc: Thierry Reding, Stephen Warren, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org
In-Reply-To: <6044581.2jEzZBWCu1@percival>
[-- Attachment #1: Type: text/plain, Size: 3518 bytes --]
On Fri, 2012-08-24 at 18:24 +0900, Alex Courbot wrote:
> On Tuesday 21 August 2012 17:54:20 Tomi Valkeinen wrote:
> > And as I said, I don't have any problems with some kind of generic power
> > sequences. So the code in the board file could be moved and converted to
> > use the power sequences, if that is better than just plain c code.
>
> My concern now is, provided that all drivers to their job and handle how their
> devices are switched on and off, when (if at all) are encoded power sequences
> better than their equivalent C code? There is the matching database size issue
> that you mentionned, is it a sufficient concern to justify a new kernel feature?
Good question.
I think obviously the worst solution is to have separate .c driver files
for each panel, where the drivers do 99% the same thing.
So the question is how to represent the 1% difference the panels have.
I think it depends on the panels. If it looks like all the panel have,
say, max 2 regulators and one reset/enable gpio, and they are always
enabled in the same order (regulators first, then the gpio), it should
be easy to handle it in the driver without any power sequence framework.
If the panels require more complex setups, then the code in the panel
driver would probably start to form into a power sequence framework, and
it would make sense to have it as a separate framework.
Then again, if the panel setup is complex, it makes me wonder if it'd be
just easier to handle it with c code in a separate driver.
Also, the database size issue is a bit separate issue. There's the db
size problem with or without the framework, if we do not pass the data
from DT.
So as clarification, I see 4 different options:
- Power sequences in DT (as proposed in this series)
- Custom panel data in DT, that the driver uses to power up properly
- Power sequences in a panel database in kernel
- Custom panel data in a panel db in kernel
> On the other hand some devices like panels are typically not used in many
> different appliances, so maybe it is not worth to separate them from their
Yep, this is the reason for my concern with the database size. The DB
could contain 10k panels, of which a board uses one. The rest just waste
memory. But then again, 10k panels is probably not a realistic amount.
It's difficult to guess the amount of memory used by such a database,
though. If it uses, say, 8kB, I'm not sure if it's a reason to panic.
And, as I mentioned, it could be optimized so that the driver throws
away the unneeded panels at __init. Of course here the problem is that
the panel needs to know what panels are needed.
> board definition. As Mark mentionned, having .dtsi files for the DT (and their
> equivalent .h for kernels that use platform data) might be a good middle
> ground.
I guess it's the perfectionist in me that leans toward handling it fully
in the kernel, as I think that's architecturally correct thing to do.
The DT data should contain parameters that can be configured per board,
or nodes (like regulators) that are needed by other parts of the DT
data.
If the only reason why to do it via DT is to avoid the possible size
problem with the panel database, perhaps what we should solve is the
optimization of the database. If that seems unsolvable, then DT approach
could be used as a workaround.
And I have to say I'm not too familiar with DT, so if I'm wrong about
what DT should contain, I'm all ears =).
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 01/23] OMAPDSS: outputs: Create a new entity called outputs
From: Tomi Valkeinen @ 2012-08-24 12:41 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev, rob, sumit.semwal
In-Reply-To: <1345528711-27801-2-git-send-email-archit@ti.com>
[-- Attachment #1: Type: text/plain, Size: 3630 bytes --]
On Tue, 2012-08-21 at 11:28 +0530, Archit Taneja wrote:
> diff --git a/drivers/video/omap2/dss/output.c b/drivers/video/omap2/dss/output.c
> new file mode 100644
> index 0000000..034ebbe
> --- /dev/null
> +++ b/drivers/video/omap2/dss/output.c
> @@ -0,0 +1,58 @@
> +/*
> + * Copyright (C) 2012 Texas Instruments Ltd
> + * Author: Archit Taneja <archit@ti.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/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include <video/omapdss.h>
> +
> +#include "dss.h"
> +
> +static struct list_head output_list;
You can do:
static LIST_HEAD(output_list);
Then you don't need to initialize it separately.
> +
> +struct omap_dss_output *dss_create_output(struct platform_device *pdev)
> +{
> + struct omap_dss_output *out;
> +
> + out = kzalloc(sizeof(struct omap_dss_output *), GFP_KERNEL);
> + if (!out)
> + return NULL;
A patch that adds kzalloc but no free is always a bit suspicious =).
> +
> + out->pdev = pdev;
> +
> + list_add_tail(&out->list, &output_list);
> +
> + return out;
> +}
Instead of allocating omap_dss_output here, you could let the caller do
it, and only initialize it here with default values (if that's even
needed). Then the caller can use kzalloc, or can just embed the stuct
into its own data-struct, which may be often a better choice.
> +struct omap_dss_output *omap_dss_get_output(enum omap_dss_output_id id)
> +{
> + struct omap_dss_output *out;
> +
> + list_for_each_entry(out, &output_list, list) {
> + if (out->id == id)
> + return out;
> + }
> +
> + return NULL;
> +}
> +
> +void dss_init_outputs(void)
> +{
> + INIT_LIST_HEAD(&output_list);
> +}
> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
> index b868123..0ba613f 100644
> --- a/include/video/omapdss.h
> +++ b/include/video/omapdss.h
> @@ -207,6 +207,16 @@ enum omap_hdmi_flags {
> OMAP_HDMI_SDA_SCL_EXTERNAL_PULLUP = 1 << 0,
> };
>
> +enum omap_dss_output_id {
> + OMAP_DSS_OUTPUT_DPI = 1 << 0,
> + OMAP_DSS_OUTPUT_DBI = 1 << 1,
> + OMAP_DSS_OUTPUT_SDI = 1 << 2,
> + OMAP_DSS_OUTPUT_DSI1 = 1 << 3,
> + OMAP_DSS_OUTPUT_VENC = 1 << 4,
> + OMAP_DSS_OUTPUT_DSI2 = 1 << 5,
> + OMAP_DSS_OUTPUT_HDMI = 1 << 6,
> +};
I'm not sure about this. We already have enum omap_display_type. If you
need the instance number, you could have that as a separate int field.
Where do you need the output_id?
> +
> /* RFBI */
>
> struct rfbi_timings {
> @@ -492,6 +502,24 @@ struct omap_dsi_pin_config {
> int pins[OMAP_DSS_MAX_DSI_PINS];
> };
>
> +struct omap_dss_output {
> + struct list_head list;
> +
> + /* display type supported by the output */
> + enum omap_display_type type;
> +
> + /* output instance */
> + enum omap_dss_output_id id;
So instead of omap_dss_output_id, you'd have omap_display_type type and
int id, which together tell the supported output and also the output
driver instance.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 01/23] OMAPDSS: outputs: Create a new entity called outputs
From: Archit Taneja @ 2012-08-24 12:53 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: Archit Taneja, linux-omap, linux-fbdev, rob, sumit.semwal
In-Reply-To: <1345812069.9287.65.camel@lappyti>
On Friday 24 August 2012 06:11 PM, Tomi Valkeinen wrote:
> On Tue, 2012-08-21 at 11:28 +0530, Archit Taneja wrote:
>
>> diff --git a/drivers/video/omap2/dss/output.c b/drivers/video/omap2/dss/output.c
>> new file mode 100644
>> index 0000000..034ebbe
>> --- /dev/null
>> +++ b/drivers/video/omap2/dss/output.c
>> @@ -0,0 +1,58 @@
>> +/*
>> + * Copyright (C) 2012 Texas Instruments Ltd
>> + * Author: Archit Taneja <archit@ti.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/kernel.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +
>> +#include <video/omapdss.h>
>> +
>> +#include "dss.h"
>> +
>> +static struct list_head output_list;
>
> You can do:
>
> static LIST_HEAD(output_list);
>
> Then you don't need to initialize it separately.
Oh ok. I'll fix this.
>
>> +
>> +struct omap_dss_output *dss_create_output(struct platform_device *pdev)
>> +{
>> + struct omap_dss_output *out;
>> +
>> + out = kzalloc(sizeof(struct omap_dss_output *), GFP_KERNEL);
>> + if (!out)
>> + return NULL;
>
> A patch that adds kzalloc but no free is always a bit suspicious =).
>
>> +
>> + out->pdev = pdev;
>> +
>> + list_add_tail(&out->list, &output_list);
>> +
>> + return out;
>> +}
>
> Instead of allocating omap_dss_output here, you could let the caller do
> it, and only initialize it here with default values (if that's even
> needed). Then the caller can use kzalloc, or can just embed the stuct
> into its own data-struct, which may be often a better choice.
So output can be in each interface driver's private data, and we just
add that to our list of outputs?
>
>> +struct omap_dss_output *omap_dss_get_output(enum omap_dss_output_id id)
>> +{
>> + struct omap_dss_output *out;
>> +
>> + list_for_each_entry(out, &output_list, list) {
>> + if (out->id = id)
>> + return out;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +void dss_init_outputs(void)
>> +{
>> + INIT_LIST_HEAD(&output_list);
>> +}
>> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
>> index b868123..0ba613f 100644
>> --- a/include/video/omapdss.h
>> +++ b/include/video/omapdss.h
>> @@ -207,6 +207,16 @@ enum omap_hdmi_flags {
>> OMAP_HDMI_SDA_SCL_EXTERNAL_PULLUP = 1 << 0,
>> };
>>
>> +enum omap_dss_output_id {
>> + OMAP_DSS_OUTPUT_DPI = 1 << 0,
>> + OMAP_DSS_OUTPUT_DBI = 1 << 1,
>> + OMAP_DSS_OUTPUT_SDI = 1 << 2,
>> + OMAP_DSS_OUTPUT_DSI1 = 1 << 3,
>> + OMAP_DSS_OUTPUT_VENC = 1 << 4,
>> + OMAP_DSS_OUTPUT_DSI2 = 1 << 5,
>> + OMAP_DSS_OUTPUT_HDMI = 1 << 6,
>> +};
>
> I'm not sure about this. We already have enum omap_display_type. If you
> need the instance number, you could have that as a separate int field.
>
> Where do you need the output_id?
output_id is used to take care of situations where there our multiple
outputs of the same type, like DSI1 and DSI2. An enum helps when we
check if an overlay manager supports that output instance or not. For
ex, on OMAP4, LCD1 connects to DSI1 and not DSI2.
I add a func called dss_feat_get_supported_outputs(channel) later to
check for this. When setting a new output for a manager, we just do an
'&' to see if the output in question is in the mask of the manager's set
of supported outputs.
>
>> +
>> /* RFBI */
>>
>> struct rfbi_timings {
>> @@ -492,6 +502,24 @@ struct omap_dsi_pin_config {
>> int pins[OMAP_DSS_MAX_DSI_PINS];
>> };
>>
>> +struct omap_dss_output {
>> + struct list_head list;
>> +
>> + /* display type supported by the output */
>> + enum omap_display_type type;
>> +
>> + /* output instance */
>> + enum omap_dss_output_id id;
>
> So instead of omap_dss_output_id, you'd have omap_display_type type and
> int id, which together tell the supported output and also the output
> driver instance.
>
> Tomi
>
^ permalink raw reply
* Re: [PATCH 02/23] OMAPDSS: outputs: Create and initialize output instances
From: Tomi Valkeinen @ 2012-08-24 13:14 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev, rob, sumit.semwal
In-Reply-To: <1345528711-27801-3-git-send-email-archit@ti.com>
[-- Attachment #1: Type: text/plain, Size: 3915 bytes --]
On Tue, 2012-08-21 at 11:28 +0530, Archit Taneja wrote:
> Create output instances by having an init function in the probes of the platform
> device drivers for different interfaces. Create a small function for each
> interface to initialize the output entity's fields type and id.
>
> In the probe of each interface driver, the output entities are created before
> the *_probe_pdata() functions intentionally. This is done to ensure that the
> output entity is prepared before the panels connected to the output are
> registered. We need the output entities to be ready because OMAPDSS will try
> to make connections between overlays, managers, outputs and devices during the
> panel's probe.
You're referring to the recheck_connections stuff? I have a patch that
moves that to omapfb side. But of course it doesn't hurt to initialize
the output early.
We should generally do the initialization in output driver's probe more
or less so that we first setup everything related to the output driver,
and after that we register the dssdevs. But I think that's what is
already done.
So, no complaints =).
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
> drivers/video/omap2/dss/dpi.c | 20 ++++++++++++++++++++
> drivers/video/omap2/dss/dsi.c | 26 ++++++++++++++++++++++++--
> drivers/video/omap2/dss/hdmi.c | 18 ++++++++++++++++++
> drivers/video/omap2/dss/rfbi.c | 19 +++++++++++++++++++
> drivers/video/omap2/dss/sdi.c | 20 ++++++++++++++++++++
> drivers/video/omap2/dss/venc.c | 20 ++++++++++++++++++++
> 6 files changed, 121 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
> index f260343..4eca2e7 100644
> --- a/drivers/video/omap2/dss/dpi.c
> +++ b/drivers/video/omap2/dss/dpi.c
> @@ -408,10 +408,30 @@ static void __init dpi_probe_pdata(struct platform_device *pdev)
> }
> }
>
> +static int __init dpi_init_output(struct platform_device *pdev)
> +{
> + struct omap_dss_output *out;
> +
> + out = dss_create_output(pdev);
> + if (!out)
> + return -ENOMEM;
> +
> + out->id = OMAP_DSS_OUTPUT_DPI;
> + out->type = OMAP_DISPLAY_TYPE_DPI;
> +
> + return 0;
> +}
> +
> static int __init omap_dpi_probe(struct platform_device *pdev)
> {
> + int r;
> +
> mutex_init(&dpi.lock);
>
> + r = dpi_init_output(pdev);
> + if (r)
> + return r;
> +
> dpi_probe_pdata(pdev);
>
> return 0;
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index 659b6cd..22e0873 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -4903,6 +4903,23 @@ static void __init dsi_probe_pdata(struct platform_device *dsidev)
> }
> }
>
> +static int __init dsi_init_output(struct platform_device *dsidev,
> + struct dsi_data *dsi)
> +{
> + struct omap_dss_output *out;
> +
> + out = dss_create_output(dsidev);
> + if (!out)
> + return -ENOMEM;
> +
> + out->id = dsi->module_id == 0 ?
> + OMAP_DSS_OUTPUT_DSI1 : OMAP_DSS_OUTPUT_DSI2;
> +
> + out->type = OMAP_DISPLAY_TYPE_DSI;
> +
> + return 0;
As I mentioned in the last email, I think this could be something like:
struct omap_dss_output *out = &dsi->output;
out->pdev = dsidev;
out->id = xxx;
out->type = yyy;
dss_register_output(out);
> +}
> +
> /* DSI1 HW IP initialisation */
> static int __init omap_dsihw_probe(struct platform_device *dsidev)
> {
> @@ -4997,10 +5014,14 @@ static int __init omap_dsihw_probe(struct platform_device *dsidev)
> else
> dsi->num_lanes_supported = 3;
>
> - dsi_probe_pdata(dsidev);
> -
> dsi_runtime_put(dsidev);
>
> + r = dsi_init_output(dsidev, dsi);
> + if (r)
> + goto err_init_output;
> +
> + dsi_probe_pdata(dsidev);
> +
Why do you change the sequence here? Isn't it enough to just add the
init_output before probe_pdata?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH v5] da8xx-fb: allow frame to complete after disabling LCDC
From: Manjunathappa, Prakash @ 2012-08-24 13:25 UTC (permalink / raw)
To: linux-fbdev
Wait for active frame transfer to complete after disabling LCDC.
At the same this wait is not be required when there are sync and
underflow errors.
Patch applies for revision 2 of LCDC present am335x.
More information on disable and reset sequence can be found in
section 13.4.6 of AM335x TRM @www.ti.com/am335x.
Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
---
Applies on top of fbdev-next of Florian Tobias Schandinat's tree.
Since v4:
Minor nit, removed extra line.
Since v3:
Rely on frame done interrupt instead of polling for it.
Since v2:
Optimized the lcd_disable_raster function.
Since v1:
Changed the commit message, also added link to hardware specification.
drivers/video/da8xx-fb.c | 52 +++++++++++++++++++++++++++++++++++----------
1 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 7ae9d53..32f0d06 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -27,6 +27,7 @@
#include <linux/platform_device.h>
#include <linux/uaccess.h>
#include <linux/interrupt.h>
+#include <linux/wait.h>
#include <linux/clk.h>
#include <linux/cpufreq.h>
#include <linux/console.h>
@@ -48,6 +49,7 @@
#define LCD_PL_LOAD_DONE BIT(6)
#define LCD_FIFO_UNDERFLOW BIT(5)
#define LCD_SYNC_LOST BIT(2)
+#define LCD_FRAME_DONE BIT(0)
/* LCD DMA Control Register */
#define LCD_DMA_BURST_SIZE(x) ((x) << 4)
@@ -135,6 +137,8 @@ static resource_size_t da8xx_fb_reg_base;
static struct resource *lcdc_regs;
static unsigned int lcd_revision;
static irq_handler_t lcdc_irq_handler;
+static wait_queue_head_t frame_done_wq;
+static int frame_done_flag;
static inline unsigned int lcdc_read(unsigned int addr)
{
@@ -288,13 +292,26 @@ static inline void lcd_enable_raster(void)
}
/* Disable the Raster Engine of the LCD Controller */
-static inline void lcd_disable_raster(void)
+static inline void lcd_disable_raster(bool wait_for_frame_done)
{
u32 reg;
+ int ret;
reg = lcdc_read(LCD_RASTER_CTRL_REG);
if (reg & LCD_RASTER_ENABLE)
lcdc_write(reg & ~LCD_RASTER_ENABLE, LCD_RASTER_CTRL_REG);
+ else
+ /* return if already disabled */
+ return;
+
+ if ((wait_for_frame_done = true) && (lcd_revision = LCD_VERSION_2)) {
+ frame_done_flag = 0;
+ ret = wait_event_interruptible_timeout(frame_done_wq,
+ frame_done_flag != 0,
+ msecs_to_jiffies(50));
+ if (ret = 0)
+ pr_err("LCD Controller timed out\n");
+ }
}
static void lcd_blit(int load_mode, struct da8xx_fb_par *par)
@@ -321,7 +338,8 @@ static void lcd_blit(int load_mode, struct da8xx_fb_par *par)
} else {
reg_int = lcdc_read(LCD_INT_ENABLE_SET_REG) |
LCD_V2_END_OF_FRAME0_INT_ENA |
- LCD_V2_END_OF_FRAME1_INT_ENA;
+ LCD_V2_END_OF_FRAME1_INT_ENA |
+ LCD_FRAME_DONE;
lcdc_write(reg_int, LCD_INT_ENABLE_SET_REG);
}
reg_dma |= LCD_DUAL_FRAME_BUFFER_ENABLE;
@@ -638,7 +656,7 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
static void lcd_reset(struct da8xx_fb_par *par)
{
/* Disable the Raster if previously Enabled */
- lcd_disable_raster();
+ lcd_disable_raster(false);
/* DMA has to be disabled */
lcdc_write(0, LCD_DMA_CTRL_REG);
@@ -734,7 +752,7 @@ static irqreturn_t lcdc_irq_handler_rev02(int irq, void *arg)
u32 stat = lcdc_read(LCD_MASKED_STAT_REG);
if ((stat & LCD_SYNC_LOST) && (stat & LCD_FIFO_UNDERFLOW)) {
- lcd_disable_raster();
+ lcd_disable_raster(false);
lcdc_write(stat, LCD_MASKED_STAT_REG);
lcd_enable_raster();
} else if (stat & LCD_PL_LOAD_DONE) {
@@ -744,7 +762,7 @@ static irqreturn_t lcdc_irq_handler_rev02(int irq, void *arg)
* interrupt via the following write to the status register. If
* this is done after then one gets multiple PL done interrupts.
*/
- lcd_disable_raster();
+ lcd_disable_raster(false);
lcdc_write(stat, LCD_MASKED_STAT_REG);
@@ -775,6 +793,14 @@ static irqreturn_t lcdc_irq_handler_rev02(int irq, void *arg)
par->vsync_flag = 1;
wake_up_interruptible(&par->vsync_wait);
}
+
+ /* Set only when controller is disabled and at the end of
+ * active frame
+ */
+ if (stat & BIT(0)) {
+ frame_done_flag = 1;
+ wake_up_interruptible(&frame_done_wq);
+ }
}
lcdc_write(0, LCD_END_OF_INT_IND_REG);
@@ -789,7 +815,7 @@ static irqreturn_t lcdc_irq_handler_rev01(int irq, void *arg)
u32 reg_ras;
if ((stat & LCD_SYNC_LOST) && (stat & LCD_FIFO_UNDERFLOW)) {
- lcd_disable_raster();
+ lcd_disable_raster(false);
lcdc_write(stat, LCD_STAT_REG);
lcd_enable_raster();
} else if (stat & LCD_PL_LOAD_DONE) {
@@ -799,7 +825,7 @@ static irqreturn_t lcdc_irq_handler_rev01(int irq, void *arg)
* interrupt via the following write to the status register. If
* this is done after then one gets multiple PL done interrupts.
*/
- lcd_disable_raster();
+ lcd_disable_raster(false);
lcdc_write(stat, LCD_STAT_REG);
@@ -898,7 +924,7 @@ static int lcd_da8xx_cpufreq_transition(struct notifier_block *nb,
if (val = CPUFREQ_POSTCHANGE) {
if (par->lcd_fck_rate != clk_get_rate(par->lcdc_clk)) {
par->lcd_fck_rate = clk_get_rate(par->lcdc_clk);
- lcd_disable_raster();
+ lcd_disable_raster(true);
lcd_calc_clk_divider(par);
lcd_enable_raster();
}
@@ -935,7 +961,7 @@ static int __devexit fb_remove(struct platform_device *dev)
if (par->panel_power_ctrl)
par->panel_power_ctrl(0);
- lcd_disable_raster();
+ lcd_disable_raster(true);
lcdc_write(0, LCD_RASTER_CTRL_REG);
/* disable DMA */
@@ -1051,7 +1077,7 @@ static int cfb_blank(int blank, struct fb_info *info)
if (par->panel_power_ctrl)
par->panel_power_ctrl(0);
- lcd_disable_raster();
+ lcd_disable_raster(true);
break;
default:
ret = -EINVAL;
@@ -1356,8 +1382,10 @@ static int __devinit fb_probe(struct platform_device *device)
if (lcd_revision = LCD_VERSION_1)
lcdc_irq_handler = lcdc_irq_handler_rev01;
- else
+ else {
+ init_waitqueue_head(&frame_done_wq);
lcdc_irq_handler = lcdc_irq_handler_rev02;
+ }
ret = request_irq(par->irq, lcdc_irq_handler, 0,
DRIVER_NAME, par);
@@ -1411,7 +1439,7 @@ static int fb_suspend(struct platform_device *dev, pm_message_t state)
par->panel_power_ctrl(0);
fb_set_suspend(info, 1);
- lcd_disable_raster();
+ lcd_disable_raster(true);
clk_disable(par->lcdc_clk);
console_unlock();
--
1.7.1
^ permalink raw reply related
* [PATCH] qxl-virtio: introducing virtio-qxl driver.
From: Erlon Cruz @ 2012-08-24 19:22 UTC (permalink / raw)
To: linux-kernel
Cc: linux-fbdev, alevy, FlorianSchandinat, Erlon Cruz,
Fabiano Fidêncio, Rafael F. Santos
The qxl-virtio driver implements a QXL driver using VirtIO as transport, thus
enabling the use of QXL/Spice in non-PCI architectures. In the actual QXL
driver, all communication between Host and Guest is done through PCI shared
memory.
Signed-off-by: Erlon R. Cruz <erlon.cruz@br.flextronics.com>
Signed-off-by: Fabiano Fidêncio <Fabiano.Fidencio@fit-tecnologia.org.br>
Signed-off-by: Rafael F. Santos <fonsecasantos.rafael@gmail.com>
---
.gitignore | 1 +
drivers/video/Kconfig | 6 +
drivers/video/Makefile | 3 +
drivers/video/virtio-qxl-bridge.c | 628 +++++++++++++++++++++++++++++++++++++
include/linux/Kbuild | 1 +
include/linux/virtio_bridge.h | 159 ++++++++++
6 files changed, 798 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/virtio-qxl-bridge.c
create mode 100644 include/linux/virtio_bridge.h
diff --git a/.gitignore b/.gitignore
index 57af07c..ebe7e3e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -84,3 +84,4 @@ GTAGS
*.orig
*~
\#*#
+/nbproject/private/
\ No newline at end of file
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 0217f74..466863b 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2469,4 +2469,10 @@ config FB_SH_MOBILE_MERAM
Up to 4 memory channels can be configured, allowing 4 RGB or
2 YCbCr framebuffers to be configured.
+config VIRTIO_QXL
+ tristate "QXL driver over VirtIO"
+ depends on EXPERIMENTAL && VIRTIO
+ help
+ This driver provides a QXL video device using virtio transport.
+
endmenu
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index ee8dafb..a331743 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -170,3 +170,6 @@ obj-$(CONFIG_FB_VIRTUAL) += vfb.o
#video output switch sysfs driver
obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
+
+# Virtio QLX
+obj-$(CONFIG_VIRTIO_QXL) += virtio-qxl-bridge.o
diff --git a/drivers/video/virtio-qxl-bridge.c b/drivers/video/virtio-qxl-bridge.c
new file mode 100644
index 0000000..c799943
--- /dev/null
+++ b/drivers/video/virtio-qxl-bridge.c
@@ -0,0 +1,628 @@
+/*
+ * Virtio QXL Device
+ *
+ *
+ * Authors:
+ * Erlon R. Cruz <erlon.cruz@br.flextronics.com>
+ * Rafael F. Santos <Rafael.Santos@fit-tecnologia.org.br>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/virtio.h>
+#include <linux/virtio_ring.h>
+#include <linux/virtio_config.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_bridge.h>
+#include <linux/mm.h>
+#include <linux/semaphore.h>
+#include <linux/fs.h>
+#include <linux/errno.h>
+#include <linux/list.h>
+#include <linux/cdev.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <asm/current.h>
+#include <asm/page.h>
+#include <linux/rwsem.h>
+#include <linux/sched.h>
+
+#define DRIVER_STRING "virtio-qxl-bridge"
+#define SG_ELEMENTS 128
+
+#define DEBUG_ERROR 1
+#define DEBUG_INFO 1
+#define DEBUG_VM 0
+#define DEBUG_SG 0
+#define DEBUG_PUSH_AREA 0
+#define DEBUG_PULL_AREA 0
+#define DEBUG_IOCTL 0
+#define DEBUG_IOPWRITE 0
+#define DEBUG_FOPS 0
+
+ #define dprintk(_level, _fmt, ...) \
+do { \
+ if (_level) { \
+ printk(_fmt, ## __VA_ARGS__); \
+ } \
+} while (0)
+
+static inline void printHexa(void *buf, int len)
+{
+ uint8_t *cur, *ubuf;
+ ubuf = (uint8_t *) buf;
+
+ for (cur = ubuf; (cur - ubuf) < len; cur++)
+#ifdef __KERNEL__
+ printk("%02X", *cur);
+#else
+ ErrorF("%02X", *cur);
+#endif
+
+}
+
+dev_t dev;
+int devindex, devno;
+struct semaphore idxsem;
+struct class *virtio_qxl_class;
+
+struct qxl_usrmem_desc {
+ char __user *start;
+ unsigned int len;
+};
+
+struct virtio_qxl_bridge {
+ spinlock_t lock;
+ struct virtio_device *vdev;
+ struct virtqueue *vq;
+ struct scatterlist sg[SG_ELEMENTS];
+ struct scatterlist *vgasg;
+ struct cdev cdev;
+ struct virtioqxl_config config;
+ struct semaphore sem;
+ struct qxl_usrmem_desc mem_desc;
+ struct page **user_pages;
+};
+
+struct vbr_req {
+ struct list_head list;
+ struct vbr_proto_hdr hdr;
+ u8 status;
+};
+
+void put_userpages_vector(struct page **pages, int num_pages, bool dirty)
+{
+ int i;
+
+ for (i = 0; i < num_pages; i++) {
+ if (dirty)
+ set_page_dirty_lock(pages[i]);
+ put_page(pages[i]);
+ }
+}
+
+static int userpages_fill_sg(struct page **upages, struct scatterlist *sg,
+ u8 __user *virt, int length, int write)
+{
+ struct page *pg;
+ int sg_entries = 0, pagesidx = 0;
+ int tmp_off, rc;
+ uint8_t *ustart, *uend;
+
+ ustart = virt;
+ uend = ustart + length;
+ tmp_off = ((ulong) ustart & ~PAGE_MASK);
+ /* unaligned */
+ if (tmp_off) {
+ int gap = 0, cplen;
+
+ /* Length from offset to the end of the page */
+ gap = PAGE_SIZE - tmp_off;
+ if (gap > length)
+ cplen = length;
+ else
+ cplen = gap;
+
+ down_read(¤t->mm->mmap_sem);
+ rc = get_user_pages(current,
+ current->mm,
+ (unsigned long)ustart & PAGE_MASK,
+ 1, write, 0, upages + pagesidx++, NULL);
+ up_read(¤t->mm->mmap_sem);
+
+ pg = upages[pagesidx - 1];
+ sg_set_page(&sg[sg_entries++], pg, cplen, tmp_off);
+ ustart += cplen;
+ dprintk(DEBUG_SG,
+ "%s: Unaligned buffer: tt len %d, offset %d, gap %d,"
+ " unaligned head %d", __func__, length, tmp_off, gap,
+ cplen);
+ } else {
+ dprintk(DEBUG_SG, "%s: Aligned buffer tt len %d", __func__,
+ length);
+ }
+
+ /* Now start is aligned rigth? hooope so */
+ while (uend - ustart >= PAGE_SIZE) {
+
+ down_read(¤t->mm->mmap_sem);
+ rc = get_user_pages(current,
+ current->mm,
+ (unsigned long)ustart,
+ 1, write, 0, upages + pagesidx++, NULL);
+ up_read(¤t->mm->mmap_sem);
+
+ pg = upages[pagesidx - 1];
+ sg_set_page(&sg[sg_entries++], pg, PAGE_SIZE, 0);
+ ustart += PAGE_SIZE;
+ }
+
+ if (uend - ustart > 0) {
+
+ down_read(¤t->mm->mmap_sem);
+ rc = get_user_pages(current,
+ current->mm,
+ (unsigned long)ustart,
+ 1, write, 0, upages + pagesidx++, NULL);
+ up_read(¤t->mm->mmap_sem);
+
+ if (rc < 0)
+ goto fail;
+
+ pg = upages[pagesidx - 1];
+ sg_set_page(&sg[sg_entries++], pg, uend - ustart, 0);
+ dprintk(DEBUG_SG, " unaligned tail %d", (int)(uend - ustart));
+ ustart += uend - ustart;
+ }
+
+ if (ustart != uend)
+ panic("Anomalous behavior when filling SG\n");
+
+ dprintk(DEBUG_SG, " %d SG entries\n", sg_entries);
+ return sg_entries;
+
+fail:
+ put_userpages_vector(upages, pagesidx - 1, false);
+ return 0;
+}
+
+static int send_packet(struct virtio_qxl_bridge *devdata,
+ int what, char __user *buffer, int len, int flags)
+{
+ int in, out, mapped_entries = 0;
+ unsigned int readlen;
+ struct vbr_req *rq;
+ struct scatterlist *sg;
+
+ in = out = 0;
+ rq = kzalloc(sizeof(*rq), GFP_KERNEL);
+
+ rq->hdr.flags |= flags;
+ rq->hdr.function = what;
+ /* The offset of the buffer based on the start of video memory */
+ rq->hdr.param = buffer - devdata->mem_desc.start;
+ rq->hdr.len = len;
+
+ switch (what) {
+ case VIRTIOQXL_GETCFG:
+ sg = devdata->sg;
+ sg_set_buf(&sg[out++], &rq->hdr, sizeof(rq->hdr));
+ sg_set_buf(&sg[out + in++], buffer, len);
+ break;
+
+ case VIRTIOQXL_IOPORT_WRITE:
+ sg = devdata->sg;
+ sg_set_buf(&sg[out++], &rq->hdr, sizeof(rq->hdr));
+ sg_set_buf(&sg[out++], buffer, len);
+ break;
+
+ case VIRTIOQXL_GET_RAM:
+ sg = devdata->vgasg;
+ sg_set_buf(&sg[out++], &rq->hdr, sizeof(rq->hdr));
+ in += mapped_entries + userpages_fill_sg(devdata->user_pages, &sg[out], buffer,
+ len, 1);
+
+ break;
+
+ case VIRTIOQXL_SET_RAM:
+ sg = devdata->vgasg;
+ sg_set_buf(&sg[out++], &rq->hdr, sizeof(rq->hdr));
+ out += mapped_entries + userpages_fill_sg(devdata->user_pages, &sg[out], buffer,
+ len, 0);
+ break;
+ default:
+ panic("%s: virtio QXL request (%d) not supported\n",
+ DRIVER_STRING, what);
+ break;
+ }
+
+ sg_set_buf(&sg[out + in++], &rq->status, sizeof(rq->status));
+
+ if (virtqueue_add_buf(devdata->vq, sg, out, in, rq, GFP_KERNEL) < 0) {
+ dprintk(DEBUG_ERROR, "%s: error adding buffer\n",
+ DRIVER_STRING);
+ return -1;
+ }
+
+ virtqueue_kick(devdata->vq);
+ while (!virtqueue_get_buf(devdata->vq, &readlen))
+ cpu_relax();
+
+ if (mapped_entries)
+ put_userpages_vector(devdata->user_pages, mapped_entries,
+ (in > 1));
+
+ kfree(rq);
+ return 0;
+}
+
+static int get_from_host(struct virtio_qxl_bridge *devdata, uint what,
+ void *bufto, int len)
+{
+ return send_packet(devdata, what, bufto, len, CONFIG_READ);
+}
+
+static int set_on_host(struct virtio_qxl_bridge *devdata, uint what,
+ void *buffrom, int len)
+{
+ return send_packet(devdata, what, buffrom, len, CONFIG_WRITE);
+}
+
+static int device_open(struct inode *inode, struct file *file)
+{
+ struct virtio_qxl_bridge *virtiodata;
+
+ dprintk(DEBUG_INFO, "%s: entering %s\n", DRIVER_STRING, __func__);
+
+ virtiodata + container_of(inode->i_cdev, struct virtio_qxl_bridge, cdev);
+ file->private_data = virtiodata;
+
+ return 0;
+}
+
+static long device_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+ struct virtio_qxl_bridge *brdev = file->private_data;
+
+ switch (cmd) {
+
+ case QXL_IOCTL_QXL_IO_GETCFG:{
+ struct virtioqxl_config *cfg;
+ cfg = &brdev->config;
+ copy_to_user((void __user *)arg, cfg,
+ sizeof(struct virtioqxl_config));
+ break;
+ }
+
+ case QXL_IOCTL_QXL_IO_SET_RAMSTART:{
+
+ copy_from_user(&brdev->mem_desc.start,
+ (void __user *)arg,
+ sizeof(brdev->mem_desc.start));
+ brdev->mem_desc.len + brdev->config.vramsize + brdev->config.ramsize +
+ brdev->config.romsize;
+
+ dprintk(DEBUG_INFO, "%s: user ram start %p\n",
+ DRIVER_STRING, brdev->mem_desc.start);
+ break;
+ }
+
+ case QXL_IOCTL_NOTIFY_CMD:
+ case QXL_IOCTL_NOTIFY_CURSOR:
+ case QXL_IOCTL_UPDATE_AREA:
+ case QXL_IOCTL_UPDATE_IRQ:
+ case QXL_IOCTL_NOTIFY_OOM:
+ case QXL_IOCTL_RESET:
+ case QXL_IOCTL_SET_MODE:
+ case QXL_IOCTL_LOG:
+ case QXL_IOCTL_MEMSLOT_ADD:
+ case QXL_IOCTL_MEMSLOT_DEL:
+ case QXL_IOCTL_DETACH_PRIMARY:
+ case QXL_IOCTL_ATTACH_PRIMARY:
+ case QXL_IOCTL_CREATE_PRIMARY:
+ case QXL_IOCTL_DESTROY_PRIMARY:
+ case QXL_IOCTL_DESTROY_SURFACE_WAIT:
+ case QXL_IOCTL_DESTROY_ALL_SURFACES:
+ case QXL_IOCTL_UPDATE_AREA_ASYNC:
+ case QXL_IOCTL_MEMSLOT_ADD_ASYNC:
+ case QXL_IOCTL_CREATE_PRIMARY_ASYNC:
+ case QXL_IOCTL_DESTROY_PRIMARY_ASYNC:
+ case QXL_IOCTL_DESTROY_SURFACE_ASYNC:
+ case QXL_IOCTL_DESTROY_ALL_SURFACES_ASYNC:
+ case QXL_IOCTL_FLUSH_SURFACES_ASYNC:
+ case QXL_IOCTL_FLUSH_RELEASE:{
+ struct iowrite_cmd *iocmd + kmalloc(sizeof(*iocmd), GFP_KERNEL);
+ iocmd->port = _IOC_NR(cmd);
+ iocmd->arg = arg;
+ dprintk(DEBUG_IOPWRITE, " port %d, arg %d\n",
+ iocmd->port, iocmd->arg);
+ set_on_host(brdev, VIRTIOQXL_IOPORT_WRITE, iocmd,
+ sizeof(*iocmd));
+ kfree(iocmd);
+ break;
+ }
+
+ default:
+ dprintk(DEBUG_INFO, "%s: IOCTL not handled %ui\n", __func__,
+ cmd);
+ return -ENOTTY;
+ }
+
+ return 0;
+}
+
+static ssize_t device_read(struct file *file, char __user *buf, size_t len,
+ loff_t *f_pos)
+{
+ struct virtio_qxl_bridge *virtiodata = file->private_data;
+
+ if (!virtiodata->mem_desc.start)
+ return -EFAULT;
+
+ if (buf < virtiodata->mem_desc.start ||
+ (buf + len) >
+ (virtiodata->mem_desc.start + virtiodata->mem_desc.len))
+ return -EFAULT;
+
+ if (len <= 0)
+ return 0;
+
+ dprintk(DEBUG_FOPS,
+ "%s: virtio_qxl_bridge: %s: reading %d bytes, "
+ "useraddr = %p, videomem offset = %lu\n",
+ DRIVER_STRING, __func__, (int)len, buf,
+ buf - virtiodata->mem_desc.start);
+
+ return get_from_host(virtiodata, VIRTIOQXL_GET_RAM, buf, len);
+}
+
+static ssize_t device_write(struct file *file, const char __user *buf,
+ size_t len, loff_t *f_pos)
+{
+ struct virtio_qxl_bridge *virtiodata = file->private_data;
+
+ if (!virtiodata->mem_desc.start)
+ return -EFAULT;
+
+ if (buf < virtiodata->mem_desc.start ||
+ (buf + len) >
+ (virtiodata->mem_desc.start + virtiodata->mem_desc.len))
+ return -EFAULT;
+
+ if (len <= 0)
+ return 0;
+
+ dprintk(DEBUG_FOPS,
+ "%s: virtio_qxl_bridge: %s: writing %d bytes, "
+ "useraddr = %p, videomem offset = %lu\n",
+ DRIVER_STRING, __func__, (int)len, buf,
+ buf - virtiodata->mem_desc.start);
+
+ return set_on_host(virtiodata, VIRTIOQXL_SET_RAM, (char *)buf, len);
+}
+
+int device_release(struct inode *inode, struct file *file)
+{
+ dprintk(DEBUG_FOPS, "%s: %s\n", DRIVER_STRING, __func__);
+ return 0;
+}
+
+const struct file_operations device_fops = {
+ .owner = THIS_MODULE,
+ .unlocked_ioctl = device_ioctl,
+ .read = device_read,
+ .write = device_write,
+ .open = device_open,
+ .release = device_release
+};
+
+static int setup_cdev(struct virtio_qxl_bridge *virtiodata)
+{
+ int minor, err = 0;
+ struct device *device;
+
+ down(&idxsem);
+ minor = devindex;
+ devindex++;
+ up(&idxsem);
+
+ devno = MKDEV(MAJOR(dev), minor);
+
+ dprintk(DEBUG_INFO, "%s: %s: adding char device %d/%d\n",
+ DRIVER_STRING, __func__, MAJOR(dev), minor);
+
+ /* Print driver port string */
+ cdev_init(&virtiodata->cdev, &device_fops);
+ virtiodata->cdev.owner = THIS_MODULE;
+ virtiodata->cdev.ops = &device_fops;
+ err = cdev_add(&virtiodata->cdev, devno, 1);
+ if (err) {
+ dprintk(DEBUG_INFO, "%s: error %d adding char device %d",
+ DRIVER_STRING, err, devindex);
+ return err;
+ }
+
+ /* Create a sysfs class entry */
+ device + device_create(virtio_qxl_class, NULL, devno, NULL, "virtioqxl%u",
+ minor);
+ if (IS_ERR(device)) {
+ dprintk(DEBUG_INFO, "%s: error %li creating device %d\n",
+ DRIVER_STRING, PTR_ERR(device), devindex);
+ err = PTR_ERR(device);
+ cdev_del(&virtiodata->cdev);
+ }
+ return err;
+}
+
+static void release_cdev(struct virtio_qxl_bridge *virtiodata)
+{
+ device_destroy(virtio_qxl_class, devno);
+ cdev_del(&virtiodata->cdev);
+}
+
+static int __devinit qxl_bridge_probe(struct virtio_device *vdev)
+{
+ int memlen, sg_elements, err = 0;
+ struct virtio_qxl_bridge *brdev;
+ struct virtioqxl_config *cfg;
+
+ dprintk(DEBUG_INFO, "%s: probing\n", DRIVER_STRING);
+
+ brdev = kmalloc(sizeof(struct virtio_qxl_bridge), GFP_KERNEL);
+ if (!brdev)
+ return -ENOMEM;
+
+ dprintk(DEBUG_INFO, "%s: allocated %lu bytes as virtio_data\n",
+ DRIVER_STRING, sizeof(struct virtio_qxl_bridge));
+
+ brdev->vq = virtio_find_single_vq(vdev, NULL, "requests");
+ if (IS_ERR(brdev->vq)) {
+ err = PTR_ERR(brdev->vq);
+ dprintk(DEBUG_ERROR, "%s: error finding vq\n", DRIVER_STRING);
+ goto out_find;
+ }
+
+ cfg = &brdev->config;
+ brdev->vdev = vdev;
+ brdev->vdev->priv = brdev;
+
+ spin_lock_init(&brdev->lock);
+ sg_init_table(brdev->sg, SG_ELEMENTS);
+ sema_init(&brdev->sem, 1);
+
+ err = setup_cdev(brdev);
+ if (err < 0)
+ goto out_cdev;
+
+ get_from_host(brdev, VIRTIOQXL_GETCFG, cfg, sizeof(*cfg));
+ memlen = cfg->ramsize + cfg->vramsize + cfg->romsize;
+ dprintk(DEBUG_INFO,
+ "%s: got config information from host: ram size %d, "
+ "vram size %d, rom size %d\n", DRIVER_STRING, cfg->ramsize,
+ cfg->vramsize, cfg->romsize);
+
+ /* Memmory + alignment + head/tail */
+ sg_elements = cfg->vramsize / PAGE_SIZE + 1 + 2;
+ brdev->vgasg + kmalloc(sizeof(struct scatterlist) * sg_elements, GFP_KERNEL);
+ if (!brdev->vgasg) {
+ dprintk(DEBUG_ERROR, "%s: error allocating SG memory\n",
+ DRIVER_STRING);
+ goto out_driver_mem;
+ }
+ dprintk(DEBUG_INFO, "%s: allocated table for %d SG elements\n",
+ DRIVER_STRING, sg_elements);
+
+ sg_init_table(brdev->vgasg, sg_elements);
+
+ brdev->mem_desc.start = NULL;
+ brdev->user_pages + kmalloc((cfg->vramsize / PAGE_SIZE) * sizeof(struct page),
+ GFP_KERNEL);
+ if (!brdev->user_pages) {
+ dprintk(DEBUG_ERROR, "%s: error allocating page table memory\n",
+ DRIVER_STRING);
+ goto out_driver_mem2;
+ }
+
+ return 0;
+
+out_driver_mem2:
+ kfree(brdev->vgasg);
+out_driver_mem:
+ release_cdev(brdev);
+out_cdev:
+ vdev->config->reset(vdev);
+ vdev->config->del_vqs(vdev);
+out_find:
+ kfree(brdev);
+ return err;
+}
+
+static void __devexit qxl_bridge_remove(struct virtio_device *vdev)
+{
+ struct virtio_qxl_bridge *brdata = vdev->priv;
+
+ kfree(brdata->user_pages);
+ kfree(brdata->vgasg);
+ release_cdev(brdata);
+ vdev->config->reset(vdev);
+ vdev->config->del_vqs(vdev);
+ dprintk(DEBUG_INFO, "%s: removing\n", DRIVER_STRING);
+ kfree(brdata);
+}
+
+static const struct virtio_device_id id_table[] = {
+ {6, VIRTIO_DEV_ANY_ID},
+ {0},
+};
+
+static struct virtio_driver __refdata virtio_qxl = {
+ .driver.name = KBUILD_MODNAME,
+ .driver.owner = THIS_MODULE,
+ .id_table = id_table,
+ .probe = qxl_bridge_probe,
+ .remove = __devexit_p(qxl_bridge_remove),
+};
+
+static int __init init(void)
+{
+ int ret = 0;
+ dev = 0;
+ devindex = 0;
+ devno = 0;
+
+ dprintk(DEBUG_INFO, "%s: init\n", DRIVER_STRING);
+
+ ret = alloc_chrdev_region(&dev, 0, 1, "virtio-qxl-bridge");
+ if (ret < 0) {
+ dprintk(DEBUG_INFO, "%s: can't get major %d\n", DRIVER_STRING,
+ MAJOR(dev));
+ goto out;
+ }
+
+ sema_init(&idxsem, 1);
+
+ virtio_qxl_class = class_create(THIS_MODULE, "virtioqxl");
+ if (IS_ERR(virtio_qxl_class)) {
+ dprintk(DEBUG_INFO, "%s: error creating driver class.\n",
+ DRIVER_STRING);
+ ret = PTR_ERR(virtio_qxl_class);
+ goto class_out;
+ }
+
+ ret = register_virtio_driver(&virtio_qxl);
+ if (!ret)
+ return ret;
+
+ class_destroy(virtio_qxl_class);
+class_out:
+ unregister_chrdev_region(dev, 1);
+out:
+ return ret;
+}
+
+static void __exit finish(void)
+{
+ unregister_virtio_driver(&virtio_qxl);
+ class_destroy(virtio_qxl_class);
+ unregister_chrdev_region(dev, 1);
+ dprintk(DEBUG_INFO, "%s: ...exit.\n", DRIVER_STRING);
+}
+
+module_init(init);
+module_exit(finish);
+
+MODULE_DEVICE_TABLE(virtio, id_table);
+MODULE_DESCRIPTION("VirtIO QXL bridge");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/Kbuild b/include/linux/Kbuild
index fa21760..8b814f6 100644
--- a/include/linux/Kbuild
+++ b/include/linux/Kbuild
@@ -397,6 +397,7 @@ header-y += videodev2.h
header-y += virtio_9p.h
header-y += virtio_balloon.h
header-y += virtio_blk.h
+header-y += virtio_bridge.h
header-y += virtio_config.h
header-y += virtio_console.h
header-y += virtio_ids.h
diff --git a/include/linux/virtio_bridge.h b/include/linux/virtio_bridge.h
new file mode 100644
index 0000000..a5f6472
--- /dev/null
+++ b/include/linux/virtio_bridge.h
@@ -0,0 +1,159 @@
+/*
+ * Virtio QXL
+ *
+ *
+ * Authors:
+ * Erlon R. Cruz <erlon.cruz@br.flextronics.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef VIRTIO_BRIDGE_H
+#define VIRTIO_BRIDGE_H
+
+#include <linux/types.h>
+
+#ifdef __KERNEL__
+/* Taken from spice-protocol */
+enum {
+ QXL_IO_NOTIFY_CMD,
+ QXL_IO_NOTIFY_CURSOR,
+ QXL_IO_UPDATE_AREA,
+ QXL_IO_UPDATE_IRQ,
+ QXL_IO_NOTIFY_OOM,
+ QXL_IO_RESET,
+ QXL_IO_SET_MODE, /* qxl-1 */
+ QXL_IO_LOG,
+ /* appended for qxl-2 */
+ QXL_IO_MEMSLOT_ADD,
+ QXL_IO_MEMSLOT_DEL,
+ QXL_IO_DETACH_PRIMARY,
+ QXL_IO_ATTACH_PRIMARY,
+ QXL_IO_CREATE_PRIMARY,
+ QXL_IO_DESTROY_PRIMARY,
+ QXL_IO_DESTROY_SURFACE_WAIT,
+ QXL_IO_DESTROY_ALL_SURFACES,
+ /* appended for qxl-3 */
+ QXL_IO_UPDATE_AREA_ASYNC,
+ QXL_IO_MEMSLOT_ADD_ASYNC,
+ QXL_IO_CREATE_PRIMARY_ASYNC,
+ QXL_IO_DESTROY_PRIMARY_ASYNC,
+ QXL_IO_DESTROY_SURFACE_ASYNC,
+ QXL_IO_DESTROY_ALL_SURFACES_ASYNC,
+ QXL_IO_FLUSH_SURFACES_ASYNC,
+ QXL_IO_FLUSH_RELEASE,
+ QXL_IO_RANGE_SIZE
+};
+
+/* Transport operations between guest kernel and host */
+enum {
+ VIRTIOQXL_GETCFG,
+ VIRTIOQXL_IOPORT_WRITE,
+ VIRTIOQXL_GET_RAM,
+ VIRTIOQXL_SET_RAM
+};
+
+enum {
+ VIRTIOQXL_STATUS_DONE,
+ VIRTIOQXL_STATUS_ERROR,
+ VIRTIOQXL_STATUS_RANGE
+};
+
+/* TODO: Merge this num with guest_host_cmd? */
+/* Read configs from the host device */
+#define CONFIG_READ 0x00000001
+/* Write configs on the host device */
+#define CONFIG_WRITE 0x00000002
+
+#endif
+
+/* Commands betweend xf86 driver and kernel virtio driver */
+enum {
+ QXL_IO_PUSH_AREA = 100,
+ QXL_IO_PULL_AREA,
+ QXL_IO_GETCFG,
+ QXL_IO_SET_RAMSTART
+};
+
+struct qxl_ram_area {
+ __u32 offset;
+ __u32 len;
+};
+
+struct vbr_proto_hdr {
+ __u32 function;
+ __u32 flags;
+ __u32 param; /* Parameter related var */
+ __u32 len;
+};
+
+struct iowrite_cmd {
+ __u32 port;
+ __u32 arg;
+};
+
+struct virtioqxl_config {
+ __u32 configsize;
+ __u32 ramsize;
+ __u32 vramsize;
+ __u32 romsize;
+ __u32 virtiomem[0];
+};
+
+#define QXLMAGIC 'v'
+
+#define QXL_IOCTL_NOTIFY_CMD _IOW(QXLMAGIC,\
+ QXL_IO_NOTIFY_CMD, __u32)
+#define QXL_IOCTL_NOTIFY_CURSOR _IOW(QXLMAGIC, \
+ QXL_IO_NOTIFY_CURSOR, __u32)
+#define QXL_IOCTL_UPDATE_AREA _IOW(QXLMAGIC, \
+ QXL_IO_UPDATE_AREA, __u32)
+#define QXL_IOCTL_UPDATE_IRQ _IOW(QXLMAGIC, \
+ QXL_IO_UPDATE_IRQ, __u32)
+#define QXL_IOCTL_NOTIFY_OOM _IOW(QXLMAGIC, \
+ QXL_IO_NOTIFY_OOM, __u32)
+#define QXL_IOCTL_RESET _IOW(QXLMAGIC, \
+ QXL_IO_RESET, __u32)
+#define QXL_IOCTL_SET_MODE _IOW(QXLMAGIC, \
+ QXL_IO_SET_MODE, __u32)
+#define QXL_IOCTL_LOG _IOW(QXLMAGIC, \
+ QXL_IO_LOG, __u32)
+#define QXL_IOCTL_MEMSLOT_ADD _IOW(QXLMAGIC, \
+ QXL_IO_MEMSLOT_ADD, __u32)
+#define QXL_IOCTL_MEMSLOT_DEL _IOW(QXLMAGIC, \
+ QXL_IO_MEMSLOT_DEL, __u32)
+#define QXL_IOCTL_DETACH_PRIMARY _IOW(QXLMAGIC, \
+ QXL_IO_DETACH_PRIMARY, __u32)
+#define QXL_IOCTL_ATTACH_PRIMARY _IOW(QXLMAGIC, \
+ QXL_IO_ATTACH_PRIMARY, __u32)
+#define QXL_IOCTL_CREATE_PRIMARY _IOW(QXLMAGIC, \
+ QXL_IO_CREATE_PRIMARY, __u32)
+#define QXL_IOCTL_DESTROY_PRIMARY _IOW(QXLMAGIC, \
+ QXL_IO_DESTROY_PRIMARY, __u32)
+#define QXL_IOCTL_DESTROY_SURFACE_WAIT _IOW(QXLMAGIC, \
+ QXL_IO_DESTROY_SURFACE_WAIT, __u32)
+#define QXL_IOCTL_DESTROY_ALL_SURFACES _IOW(QXLMAGIC, \
+ QXL_IO_DESTROY_ALL_SURFACES, __u32)
+#define QXL_IOCTL_UPDATE_AREA_ASYNC _IOW(QXLMAGIC, \
+ QXL_IO_UPDATE_AREA_ASYNC, __u32)
+#define QXL_IOCTL_MEMSLOT_ADD_ASYNC _IOW(QXLMAGIC, \
+ QXL_IO_MEMSLOT_ADD_ASYNC, __u32)
+#define QXL_IOCTL_CREATE_PRIMARY_ASYNC _IOW(QXLMAGIC, \
+ QXL_IO_CREATE_PRIMARY_ASYNC, __u32)
+#define QXL_IOCTL_DESTROY_PRIMARY_ASYNC _IOW(QXLMAGIC, \
+ QXL_IO_DESTROY_PRIMARY_ASYNC, __u32)
+#define QXL_IOCTL_DESTROY_SURFACE_ASYNC _IOW(QXLMAGIC, \
+ QXL_IO_DESTROY_SURFACE_ASYNC, __u32)
+#define QXL_IOCTL_DESTROY_ALL_SURFACES_ASYNC _IOW(QXLMAGIC, \
+ QXL_IO_DESTROY_ALL_SURFACES_ASYNC, __u32)
+#define QXL_IOCTL_FLUSH_SURFACES_ASYNC _IOW(QXLMAGIC, \
+ QXL_IO_FLUSH_SURFACES_ASYNC, __u32)
+#define QXL_IOCTL_FLUSH_RELEASE _IOW(QXLMAGIC, \
+ QXL_IO_FLUSH_RELEASE, __u32)
+#define QXL_IOCTL_QXL_IO_GETCFG _IOW(QXLMAGIC, \
+ QXL_IO_GETCFG, struct virtioqxl_config)
+#define QXL_IOCTL_QXL_IO_SET_RAMSTART _IOW(QXLMAGIC, \
+ QXL_IO_SET_RAMSTART, __u32)
+#endif /* VIRTIO_BRIDGE_H */
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCHv4 0/9] *** ARM: Update arch-vt8500 to Devicetree ***
From: Stephen Warren @ 2012-08-25 3:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1345707346-9035-1-git-send-email-linux@prisktech.co.nz>
On 08/23/2012 01:35 AM, Tony Prisk wrote:
> This patchset updates arch-vt8500 to devicetree and removes all the old-style
> code. Support for WM8650 has also been added.
>
> Example dts/dtsi files are given for the three currently supported models.
>
> Major changes:
>
> GPIO code has been converted to a platform_device and rewritten as WM8505
> support was broken. Add support for WM8650 gpio controller.
>
> UHCI support was missing. Added this as a generic non-pci uhci controller as
> it doesn't require anything special. Should be usable by any system that doesn't
> have special requirements to get the UHCI controller working.
>
> Framebuffer code patched to support WM8650. The bindings for this are of concern
> but there doesn't seem to be a formalized binding yet. This patch is based off
> Sascha Hauer's current patch on the dri-devel mailing list and should be easily
> patched out when its finalized.
>
> Patchset based on Arnd's arm-soc/for-next branch.
I believe all the issues I pointed out are fixed in this series. I'm not
sure I reviewed it in enough detail to ack it, but I'm fine with what I saw.
^ permalink raw reply
* Re: [PATCH 0/9] SH Mobile LCDC and MERAM patches
From: Simon Horman @ 2012-08-25 5:31 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1342658362-19491-1-git-send-email-laurent.pinchart@ideasonboard.com>
On Thu, Jul 19, 2012 at 02:39:13AM +0200, Laurent Pinchart wrote:
> Hi,
>
> Here are 9 patches for the SH Mobile LCDC and MERAM drivers. Patches 1/9 to
> 4/9 and 9/9 have previously been posted as part of the "SH Mobile LCDC
> MERAM-based frame buffer backing store" RFC series. Patch 5/9 has also been
> posted as part of the same series, albeit included in patch 9/9.
>
> As most of those patches have already been posted for review (patches 6/9 to
> 8/9 that haven't been posted already are small fixes), I plan to send a pull
> request soon, to get the patches in v3.6 if possible.
Hi Laurent,
could you let me know what the status of these patches is with regards to
merging.
> Laurent Pinchart (9):
> sh_mobile_meram: Rename operations to cache_[alloc|free|update]
> sh_mobile_meram: Use direct function calls for the public API
> sh_mobile_meram: Add direct MERAM allocation API
> fbdev: sh_mobile_lcdc: Destroy mutex at remove time
> fbdev: sh_mobile_lcdc: Fix line pitch computation
> fbdev: sh_mobile_lcdc: Use channel configuration to initialize fb
> device
> fbdev: sh_mobile_lcdc: Support horizontal panning
> fbdev: sh_mobile_lcdc: Fix overlay registers update during pan
> operation
> fbdev: sh_mobile_lcdc: Fix pan offset computation in YUV mode
>
> drivers/video/sh_mobile_lcdcfb.c | 209 +++++++++++++++++----------------
> drivers/video/sh_mobile_lcdcfb.h | 5 +-
> drivers/video/sh_mobile_meram.c | 235 +++++++++++++++++++++-----------------
> include/video/sh_mobile_meram.h | 71 ++++++++----
> 4 files changed, 293 insertions(+), 227 deletions(-)
^ permalink raw reply
* Re: [PATCH 0/9] SH Mobile LCDC and MERAM patches
From: Laurent Pinchart @ 2012-08-25 9:54 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1342658362-19491-1-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Simon,
On Saturday 25 August 2012 14:31:51 Simon Horman wrote:
> On Thu, Jul 19, 2012 at 02:39:13AM +0200, Laurent Pinchart wrote:
> > Hi,
> >
> > Here are 9 patches for the SH Mobile LCDC and MERAM drivers. Patches 1/9
> > to 4/9 and 9/9 have previously been posted as part of the "SH Mobile LCDC
> > MERAM-based frame buffer backing store" RFC series. Patch 5/9 has also
> > been posted as part of the same series, albeit included in patch 9/9.
> >
> > As most of those patches have already been posted for review (patches 6/9
> > to 8/9 that haven't been posted already are small fixes), I plan to send
> > a pull request soon, to get the patches in v3.6 if possible.
>
> Hi Laurent,
>
> could you let me know what the status of these patches is with regards to
> merging.
They have been merged in v3.6-rc.
> > Laurent Pinchart (9):
> > sh_mobile_meram: Rename operations to cache_[alloc|free|update]
> > sh_mobile_meram: Use direct function calls for the public API
> > sh_mobile_meram: Add direct MERAM allocation API
> > fbdev: sh_mobile_lcdc: Destroy mutex at remove time
> > fbdev: sh_mobile_lcdc: Fix line pitch computation
> > fbdev: sh_mobile_lcdc: Use channel configuration to initialize fb
> > device
> > fbdev: sh_mobile_lcdc: Support horizontal panning
> > fbdev: sh_mobile_lcdc: Fix overlay registers update during pan
> > operation
> > fbdev: sh_mobile_lcdc: Fix pan offset computation in YUV mode
> >
> > drivers/video/sh_mobile_lcdcfb.c | 209 +++++++++++++++++----------------
> > drivers/video/sh_mobile_lcdcfb.h | 5 +-
> > drivers/video/sh_mobile_meram.c | 235 ++++++++++++++++++---------------
> > include/video/sh_mobile_meram.h | 71 ++++++++----
> > 4 files changed, 293 insertions(+), 227 deletions(-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 02/23] OMAPDSS: outputs: Create and initialize output instances
From: Archit Taneja @ 2012-08-27 6:31 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, rob, sumit.semwal
In-Reply-To: <1345814042.9287.72.camel@lappyti>
On Friday 24 August 2012 06:44 PM, Tomi Valkeinen wrote:
> On Tue, 2012-08-21 at 11:28 +0530, Archit Taneja wrote:
>> Create output instances by having an init function in the probes of the platform
>> device drivers for different interfaces. Create a small function for each
>> interface to initialize the output entity's fields type and id.
>>
>> In the probe of each interface driver, the output entities are created before
>> the *_probe_pdata() functions intentionally. This is done to ensure that the
>> output entity is prepared before the panels connected to the output are
>> registered. We need the output entities to be ready because OMAPDSS will try
>> to make connections between overlays, managers, outputs and devices during the
>> panel's probe.
>
> You're referring to the recheck_connections stuff? I have a patch that
> moves that to omapfb side. But of course it doesn't hurt to initialize
> the output early.
I've seen that patch. omapfb would need to take care of connecting
outputs to displays, and managers to outputs. This is added in
recheck_connections done in a patch #9 of the series.
The question is whether we want some initial connections made between
outputs and displays by DSS, or should that be left completely to
omapfb/omapdrm?
>
> We should generally do the initialization in output driver's probe more
> or less so that we first setup everything related to the output driver,
> and after that we register the dssdevs. But I think that's what is
> already done.
>
> So, no complaints =).
Another thing that comes up with delaying the recheck_connections stuff
is that we can't assume that at the point of panel driver's probe, there
is an output connected to the display. That makes it a bit tricky to
call an output function in the panel's probe, since it isn't connected
to any output at all. An example is when we request for a VC in
taal_probe. Since the panel isn't connected to any output yet, we can't
really call a dsi function to request for the VC. This particular case
can be solved by requesting VCs only when we enable the panel(probably
makes more sense this way), but there might be other situations which
could get tricky to tackle.
>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>> drivers/video/omap2/dss/dpi.c | 20 ++++++++++++++++++++
>> drivers/video/omap2/dss/dsi.c | 26 ++++++++++++++++++++++++--
>> drivers/video/omap2/dss/hdmi.c | 18 ++++++++++++++++++
>> drivers/video/omap2/dss/rfbi.c | 19 +++++++++++++++++++
>> drivers/video/omap2/dss/sdi.c | 20 ++++++++++++++++++++
>> drivers/video/omap2/dss/venc.c | 20 ++++++++++++++++++++
>> 6 files changed, 121 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
>> index f260343..4eca2e7 100644
>> --- a/drivers/video/omap2/dss/dpi.c
>> +++ b/drivers/video/omap2/dss/dpi.c
>> @@ -408,10 +408,30 @@ static void __init dpi_probe_pdata(struct platform_device *pdev)
>> }
>> }
>>
>> +static int __init dpi_init_output(struct platform_device *pdev)
>> +{
>> + struct omap_dss_output *out;
>> +
>> + out = dss_create_output(pdev);
>> + if (!out)
>> + return -ENOMEM;
>> +
>> + out->id = OMAP_DSS_OUTPUT_DPI;
>> + out->type = OMAP_DISPLAY_TYPE_DPI;
>> +
>> + return 0;
>> +}
>> +
>> static int __init omap_dpi_probe(struct platform_device *pdev)
>> {
>> + int r;
>> +
>> mutex_init(&dpi.lock);
>>
>> + r = dpi_init_output(pdev);
>> + if (r)
>> + return r;
>> +
>> dpi_probe_pdata(pdev);
>>
>> return 0;
>> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
>> index 659b6cd..22e0873 100644
>> --- a/drivers/video/omap2/dss/dsi.c
>> +++ b/drivers/video/omap2/dss/dsi.c
>> @@ -4903,6 +4903,23 @@ static void __init dsi_probe_pdata(struct platform_device *dsidev)
>> }
>> }
>>
>> +static int __init dsi_init_output(struct platform_device *dsidev,
>> + struct dsi_data *dsi)
>> +{
>> + struct omap_dss_output *out;
>> +
>> + out = dss_create_output(dsidev);
>> + if (!out)
>> + return -ENOMEM;
>> +
>> + out->id = dsi->module_id = 0 ?
>> + OMAP_DSS_OUTPUT_DSI1 : OMAP_DSS_OUTPUT_DSI2;
>> +
>> + out->type = OMAP_DISPLAY_TYPE_DSI;
>> +
>> + return 0;
>
> As I mentioned in the last email, I think this could be something like:
>
> struct omap_dss_output *out = &dsi->output;
>
> out->pdev = dsidev;
> out->id = xxx;
> out->type = yyy;
> dss_register_output(out);
>
Right, this is much better, will do it this way.
>
>> +}
>> +
>> /* DSI1 HW IP initialisation */
>> static int __init omap_dsihw_probe(struct platform_device *dsidev)
>> {
>> @@ -4997,10 +5014,14 @@ static int __init omap_dsihw_probe(struct platform_device *dsidev)
>> else
>> dsi->num_lanes_supported = 3;
>>
>> - dsi_probe_pdata(dsidev);
>> -
>> dsi_runtime_put(dsidev);
>>
>> + r = dsi_init_output(dsidev, dsi);
>> + if (r)
>> + goto err_init_output;
>> +
>> + dsi_probe_pdata(dsidev);
>> +
>
> Why do you change the sequence here? Isn't it enough to just add the
> init_output before probe_pdata?
Yes, I think I didn't see the point in keeping the clocks on for
dsi_init_output() and dsi_probe_pdata(), so tried to incorporate that in
this patch too :), I'll change this back to the old way, it doesn't make
sense in moving around pm runtime calls in this series.
Archit
^ permalink raw reply
* Re: [PATCH 02/23] OMAPDSS: outputs: Create and initialize output instances
From: Tomi Valkeinen @ 2012-08-27 6:44 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev, rob, sumit.semwal
In-Reply-To: <503B115C.7040904@ti.com>
[-- Attachment #1: Type: text/plain, Size: 3143 bytes --]
On Mon, 2012-08-27 at 11:49 +0530, Archit Taneja wrote:
> On Friday 24 August 2012 06:44 PM, Tomi Valkeinen wrote:
> > On Tue, 2012-08-21 at 11:28 +0530, Archit Taneja wrote:
> >> Create output instances by having an init function in the probes of the platform
> >> device drivers for different interfaces. Create a small function for each
> >> interface to initialize the output entity's fields type and id.
> >>
> >> In the probe of each interface driver, the output entities are created before
> >> the *_probe_pdata() functions intentionally. This is done to ensure that the
> >> output entity is prepared before the panels connected to the output are
> >> registered. We need the output entities to be ready because OMAPDSS will try
> >> to make connections between overlays, managers, outputs and devices during the
> >> panel's probe.
> >
> > You're referring to the recheck_connections stuff? I have a patch that
> > moves that to omapfb side. But of course it doesn't hurt to initialize
> > the output early.
>
> I've seen that patch. omapfb would need to take care of connecting
> outputs to displays, and managers to outputs. This is added in
> recheck_connections done in a patch #9 of the series.
>
> The question is whether we want some initial connections made between
> outputs and displays by DSS, or should that be left completely to
> omapfb/omapdrm?
Good question. I don't know. Perhaps we should set initial connections
there, as the cases where we have multiple displays per output are quite
rare.
> > We should generally do the initialization in output driver's probe more
> > or less so that we first setup everything related to the output driver,
> > and after that we register the dssdevs. But I think that's what is
> > already done.
> >
> > So, no complaints =).
>
> Another thing that comes up with delaying the recheck_connections stuff
> is that we can't assume that at the point of panel driver's probe, there
> is an output connected to the display. That makes it a bit tricky to
> call an output function in the panel's probe, since it isn't connected
> to any output at all. An example is when we request for a VC in
> taal_probe. Since the panel isn't connected to any output yet, we can't
> really call a dsi function to request for the VC. This particular case
> can be solved by requesting VCs only when we enable the panel(probably
> makes more sense this way), but there might be other situations which
> could get tricky to tackle.
Right. Well, as you said, we can easily move the stuff from taal's probe
to enable. There shouldn't be any problems to that.
However, this problem is part of the bigger problem that I haven't been
able to solve properly: how to manage the probe/enable stuff for panels.
Everything would be simple and easy if we had just one panel per output,
and we could just get and configure everything at probe. But we can have
multiple panels per output, of which only one can be used at a time...
That's why we currently acquire most of the display resources at enable,
instead of probe.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 00/19] SH Mobile LCDC panel cleanup (including board code)
From: Simon Horman @ 2012-08-27 8:54 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1345122054-16013-1-git-send-email-laurent.pinchart@ideasonboard.com>
On Thu, Aug 16, 2012 at 03:00:35PM +0200, Laurent Pinchart wrote:
> Hi everybody,
>
> As part of a general panel API implementation (more on that a bit later), I've
> cleaned up panel support in the LCDC driver and in the related board code. The
> result is 19 patches independent of the generic panel API that I would like to
> get reviewed by the respective board maintainers.
Hi Paul,
I'm happy for you to take all of this, including the mach-shmobile
portions through your tree if that works for you.
>
> Laurent Pinchart (19):
> fbdev: sh_mobile_lcdc: Get display dimensions from the channel
> structure
> fbdev: sh_mobile_lcdc: Rename mode argument to modes
> fbdev: sh_mobile_lcdc: Remove priv argument from channel and overlay
> init
> ARM: mach-shmobile: ag5evm: Add LCDC tx_dev field to platform data
> fbdev: sh_mipi_dsi: Add channel field to platform data
> ARM: mach-shmobile: Initiliaze the new sh_mipi_dsi_info channel field
> fbdev: sh_mipi_dsi: Use the sh_mipi_dsi_info channel field
> fbdev: sh_mipi_dsi: Use the LCDC entity default mode
> fbdev: sh_mipi_dsi: Remove last reference to LCDC platform data
> ARM: mach-shmobile: Remove the unused sh_mipi_dsi_info lcd_chan field
> fbdev: sh_mipi_dsi: Remove the unused sh_mipi_dsi_info lcd_chan field
> fbdev: sh_mobile_lcdc: Store the backlight brightness internally
> ARM: mach-shmobile: mackerel: Removed unused get_brightness callback
> sh: ap325rxa: Remove unused get_brightness LCDC callback
> sh: ecovec24: Remove unused get_brightness LCDC callback
> fbdev: sh_mobile_lcdc: Remove unused get_brightness pdata callback
> ARM: mach-shmobile: ag5evm: Use the backlight API for brightness
> control
> sh: kfr2r09: Use the backlight API for brightness control
> fbdev: sh_mobile_lcdc: Make sh_mobile_lcdc_sys_bus_ops static
>
> arch/arm/mach-shmobile/board-ag5evm.c | 198 ++++++++++++++-------------
> arch/arm/mach-shmobile/board-ap4evb.c | 4 +-
> arch/arm/mach-shmobile/board-mackerel.c | 6 -
> arch/sh/boards/mach-ap325rxa/setup.c | 6 -
> arch/sh/boards/mach-ecovec24/setup.c | 6 -
> arch/sh/boards/mach-kfr2r09/lcd_wqvga.c | 16 +--
> arch/sh/boards/mach-kfr2r09/setup.c | 7 +-
> arch/sh/include/mach-kfr2r09/mach/kfr2r09.h | 6 +-
> drivers/video/sh_mipi_dsi.c | 69 ++++------
> drivers/video/sh_mobile_lcdcfb.c | 54 ++++----
> drivers/video/sh_mobile_lcdcfb.h | 1 +
> include/video/sh_mipi_dsi.h | 4 +-
> include/video/sh_mobile_lcdc.h | 1 -
> 13 files changed, 172 insertions(+), 206 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" 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
* [PATCH] OMAPDSS: Correct DISPC_IRQ bit definitions for LCD3
From: Chandrabhanu Mahapatra @ 2012-08-27 8:55 UTC (permalink / raw)
To: tomi.valkeinen
Cc: mark.tyler, linux-omap, linux-fbdev, Chandrabhanu Mahapatra
The DISPC_IRQ bit definitions pertaining to channel LCD3 as DISPC_IRQ_VSYNC3,
DISPC_IRQ_SYNC_LOST3, DISPC_IRQ_ACBIAS_COUNT_STAT3 AND DISPC_IRQ_FRAMEDONE3
which were incorrectly set in previous LCD3 patches have been corrected here.
Reported-by: Mark Tyler <mark.tyler@ti.com>
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
include/video/omapdss.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index b868123..9c7cca3 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -48,10 +48,10 @@
#define DISPC_IRQ_FRAMEDONEWB (1 << 23)
#define DISPC_IRQ_FRAMEDONETV (1 << 24)
#define DISPC_IRQ_WBBUFFEROVERFLOW (1 << 25)
-#define DISPC_IRQ_FRAMEDONE3 (1 << 26)
-#define DISPC_IRQ_VSYNC3 (1 << 27)
-#define DISPC_IRQ_ACBIAS_COUNT_STAT3 (1 << 28)
-#define DISPC_IRQ_SYNC_LOST3 (1 << 29)
+#define DISPC_IRQ_SYNC_LOST3 (1 << 27)
+#define DISPC_IRQ_VSYNC3 (1 << 28)
+#define DISPC_IRQ_ACBIAS_COUNT_STAT3 (1 << 29)
+#define DISPC_IRQ_FRAMEDONE3 (1 << 30)
struct omap_dss_device;
struct omap_overlay_manager;
--
1.7.10
^ permalink raw reply related
* Re: [PATCH 0/9] SH Mobile LCDC and MERAM patches
From: Simon Horman @ 2012-08-27 8:59 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1342658362-19491-1-git-send-email-laurent.pinchart@ideasonboard.com>
On Sat, Aug 25, 2012 at 11:54:44AM +0200, Laurent Pinchart wrote:
> Hi Simon,
>
> On Saturday 25 August 2012 14:31:51 Simon Horman wrote:
> > On Thu, Jul 19, 2012 at 02:39:13AM +0200, Laurent Pinchart wrote:
> > > Hi,
> > >
> > > Here are 9 patches for the SH Mobile LCDC and MERAM drivers. Patches 1/9
> > > to 4/9 and 9/9 have previously been posted as part of the "SH Mobile LCDC
> > > MERAM-based frame buffer backing store" RFC series. Patch 5/9 has also
> > > been posted as part of the same series, albeit included in patch 9/9.
> > >
> > > As most of those patches have already been posted for review (patches 6/9
> > > to 8/9 that haven't been posted already are small fixes), I plan to send
> > > a pull request soon, to get the patches in v3.6 if possible.
> >
> > Hi Laurent,
> >
> > could you let me know what the status of these patches is with regards to
> > merging.
>
> They have been merged in v3.6-rc.
Thanks, sorry for not working that out myself.
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Correct DISPC_IRQ bit definitions for LCD3
From: Tomi Valkeinen @ 2012-08-27 9:26 UTC (permalink / raw)
To: Chandrabhanu Mahapatra; +Cc: mark.tyler, linux-omap, linux-fbdev
In-Reply-To: <1346057599-5691-1-git-send-email-cmahapatra@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]
On Mon, 2012-08-27 at 14:23 +0530, Chandrabhanu Mahapatra wrote:
> The DISPC_IRQ bit definitions pertaining to channel LCD3 as DISPC_IRQ_VSYNC3,
> DISPC_IRQ_SYNC_LOST3, DISPC_IRQ_ACBIAS_COUNT_STAT3 AND DISPC_IRQ_FRAMEDONE3
> which were incorrectly set in previous LCD3 patches have been corrected here.
>
> Reported-by: Mark Tyler <mark.tyler@ti.com>
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
> include/video/omapdss.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
> index b868123..9c7cca3 100644
> --- a/include/video/omapdss.h
> +++ b/include/video/omapdss.h
> @@ -48,10 +48,10 @@
> #define DISPC_IRQ_FRAMEDONEWB (1 << 23)
> #define DISPC_IRQ_FRAMEDONETV (1 << 24)
> #define DISPC_IRQ_WBBUFFEROVERFLOW (1 << 25)
> -#define DISPC_IRQ_FRAMEDONE3 (1 << 26)
> -#define DISPC_IRQ_VSYNC3 (1 << 27)
> -#define DISPC_IRQ_ACBIAS_COUNT_STAT3 (1 << 28)
> -#define DISPC_IRQ_SYNC_LOST3 (1 << 29)
> +#define DISPC_IRQ_SYNC_LOST3 (1 << 27)
> +#define DISPC_IRQ_VSYNC3 (1 << 28)
> +#define DISPC_IRQ_ACBIAS_COUNT_STAT3 (1 << 29)
> +#define DISPC_IRQ_FRAMEDONE3 (1 << 30)
>
> struct omap_dss_device;
> struct omap_overlay_manager;
Thanks, applied.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 00/19] SH Mobile LCDC panel cleanup (including board code)
From: Laurent Pinchart @ 2012-08-27 15:35 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1345122054-16013-1-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Simon,
On Monday 27 August 2012 17:54:52 Simon Horman wrote:
> On Thu, Aug 16, 2012 at 03:00:35PM +0200, Laurent Pinchart wrote:
> > Hi everybody,
> >
> > As part of a general panel API implementation (more on that a bit later),
> > I've cleaned up panel support in the LCDC driver and in the related board
> > code. The result is 19 patches independent of the generic panel API that
> > I would like to get reviewed by the respective board maintainers.
>
> Hi Paul,
>
> I'm happy for you to take all of this, including the mach-shmobile
> portions through your tree if that works for you.
I usually push the LCDC patches through the fbdev tree. I'm fine with both
options though.
> > Laurent Pinchart (19):
> > fbdev: sh_mobile_lcdc: Get display dimensions from the channel
> > structure
> > fbdev: sh_mobile_lcdc: Rename mode argument to modes
> > fbdev: sh_mobile_lcdc: Remove priv argument from channel and overlay
> > init
> > ARM: mach-shmobile: ag5evm: Add LCDC tx_dev field to platform data
> > fbdev: sh_mipi_dsi: Add channel field to platform data
> > ARM: mach-shmobile: Initiliaze the new sh_mipi_dsi_info channel field
> > fbdev: sh_mipi_dsi: Use the sh_mipi_dsi_info channel field
> > fbdev: sh_mipi_dsi: Use the LCDC entity default mode
> > fbdev: sh_mipi_dsi: Remove last reference to LCDC platform data
> > ARM: mach-shmobile: Remove the unused sh_mipi_dsi_info lcd_chan field
> > fbdev: sh_mipi_dsi: Remove the unused sh_mipi_dsi_info lcd_chan field
> > fbdev: sh_mobile_lcdc: Store the backlight brightness internally
> > ARM: mach-shmobile: mackerel: Removed unused get_brightness callback
> > sh: ap325rxa: Remove unused get_brightness LCDC callback
> > sh: ecovec24: Remove unused get_brightness LCDC callback
> > fbdev: sh_mobile_lcdc: Remove unused get_brightness pdata callback
> > ARM: mach-shmobile: ag5evm: Use the backlight API for brightness
> > control
> > sh: kfr2r09: Use the backlight API for brightness control
> > fbdev: sh_mobile_lcdc: Make sh_mobile_lcdc_sys_bus_ops static
> >
> > arch/arm/mach-shmobile/board-ag5evm.c | 198 +++++++++++-----------
> > arch/arm/mach-shmobile/board-ap4evb.c | 4 +-
> > arch/arm/mach-shmobile/board-mackerel.c | 6 -
> > arch/sh/boards/mach-ap325rxa/setup.c | 6 -
> > arch/sh/boards/mach-ecovec24/setup.c | 6 -
> > arch/sh/boards/mach-kfr2r09/lcd_wqvga.c | 16 +--
> > arch/sh/boards/mach-kfr2r09/setup.c | 7 +-
> > arch/sh/include/mach-kfr2r09/mach/kfr2r09.h | 6 +-
> > drivers/video/sh_mipi_dsi.c | 69 ++++------
> > drivers/video/sh_mobile_lcdcfb.c | 54 ++++----
> > drivers/video/sh_mobile_lcdcfb.h | 1 +
> > include/video/sh_mipi_dsi.h | 4 +-
> > include/video/sh_mobile_lcdc.h | 1 -
> > 13 files changed, 172 insertions(+), 206 deletions(-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH] drivrs/video/jz4740_fb.c: Use SIMPLE_DEV_PM_OPS instead of dev_pm_ops
From: Marcos Paulo de Souza @ 2012-08-28 4:38 UTC (permalink / raw)
To: FlorianSchandinat; +Cc: linux-fbdev, linux-kernel, rjw, Marcos Paulo de Souza
Just a cleanup, not functional changes.
Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
---
Just compile test. Please double check.
drivers/video/jz4740_fb.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
index de36693..a53fb7d 100644
--- a/drivers/video/jz4740_fb.c
+++ b/drivers/video/jz4740_fb.c
@@ -807,12 +807,7 @@ static int jzfb_resume(struct device *dev)
return 0;
}
-static const struct dev_pm_ops jzfb_pm_ops = {
- .suspend = jzfb_suspend,
- .resume = jzfb_resume,
- .poweroff = jzfb_suspend,
- .restore = jzfb_resume,
-};
+static SIMPLE_DEV_PM_OPS(jzfb_pm_ops, jzfb_suspend, jzfb_resume);
#define JZFB_PM_OPS (&jzfb_pm_ops)
--
1.7.9.5
^ permalink raw reply related
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