* [PATCH] OMAPDSS: HDMI: Discard phy_tx_enabled member
From: jaswinder.singh @ 2012-06-23 8:19 UTC (permalink / raw)
To: tomi.valkeinen, mythripk
Cc: linux-omap, linux-fbdev, andy.green, n-dechesne, Jassi Brar
In-Reply-To: <1339797701-11540-1-git-send-email-jaswinder.singh@linaro.org>
From: Jassi Brar <jaswinder.singh@linaro.org>
It is simpler to read the current status from a register as compared
to maintaining a state variable to hold the information.
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
drivers/video/omap2/dss/ti_hdmi.h | 1 -
drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 11 ++++-------
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
index e734cb4..d174ca1 100644
--- a/drivers/video/omap2/dss/ti_hdmi.h
+++ b/drivers/video/omap2/dss/ti_hdmi.h
@@ -177,7 +177,6 @@ struct hdmi_ip_data {
/* ti_hdmi_4xxx_ip private data. These should be in a separate struct */
int hpd_gpio;
- bool phy_tx_enabled;
};
int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
index 4dae1b2..3fa3d98 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
@@ -157,6 +157,10 @@ static int hdmi_pll_init(struct hdmi_ip_data *ip_data)
/* PHY_PWR_CMD */
static int hdmi_set_phy_pwr(struct hdmi_ip_data *ip_data, enum hdmi_phy_pwr val)
{
+ /* Return if already the state */
+ if (REG_GET(hdmi_wp_base(ip_data), HDMI_WP_PWR_CTRL, 5, 4) = val)
+ return 0;
+
/* Command for power control of HDMI PHY */
REG_FLD_MOD(hdmi_wp_base(ip_data), HDMI_WP_PWR_CTRL, val, 7, 6);
@@ -241,11 +245,6 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
hpd = gpio_get_value(ip_data->hpd_gpio);
- if (hpd = ip_data->phy_tx_enabled) {
- spin_unlock_irqrestore(&phy_tx_lock, flags);
- return 0;
- }
-
if (hpd)
r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
else
@@ -257,7 +256,6 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
goto err;
}
- ip_data->phy_tx_enabled = hpd;
err:
spin_unlock_irqrestore(&phy_tx_lock, flags);
return r;
@@ -327,7 +325,6 @@ void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data)
free_irq(gpio_to_irq(ip_data->hpd_gpio), ip_data);
hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_OFF);
- ip_data->phy_tx_enabled = false;
}
static int hdmi_core_ddc_init(struct hdmi_ip_data *ip_data)
--
1.7.4.1
^ permalink raw reply related
* [PATCH] OMAPDSS: HDMI: Cache EDID
From: jaswinder.singh @ 2012-06-23 8:18 UTC (permalink / raw)
To: tomi.valkeinen, mythripk
Cc: linux-omap, linux-fbdev, andy.green, n-dechesne, Jassi Brar
From: Jassi Brar <jaswinder.singh@linaro.org>
We can easily keep track of latest EDID from the display and hence avoid
expensive EDID re-reads over I2C.
This could also help some cheapo displays that provide EDID reliably only
immediately after asserting HPD and not later.
Even with good displays, there is something in OMAPDSS that apparantly
messes up DDC occasionally while EDID is being read, giving the
"operation stopped when reading edid" error.
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
drivers/video/omap2/dss/hdmi.c | 1 +
drivers/video/omap2/dss/ti_hdmi.h | 2 ++
drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 23 ++++++++++++++++++++---
3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 900e621..0a8c825 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -764,6 +764,7 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
hdmi.ip_data.core_av_offset = HDMI_CORE_AV;
hdmi.ip_data.pll_offset = HDMI_PLLCTRL;
hdmi.ip_data.phy_offset = HDMI_PHY;
+ hdmi.ip_data.edid_len = 0; /* Invalidate EDID Cache */
mutex_init(&hdmi.ip_data.lock);
hdmi_panel_init();
diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
index cc292b8..4735860 100644
--- a/drivers/video/omap2/dss/ti_hdmi.h
+++ b/drivers/video/omap2/dss/ti_hdmi.h
@@ -178,6 +178,8 @@ struct hdmi_ip_data {
/* ti_hdmi_4xxx_ip private data. These should be in a separate struct */
int hpd_gpio;
struct mutex lock;
+ u8 edid_cached[256];
+ unsigned edid_len;
};
int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
index 04acca9..2633ade 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
@@ -243,10 +243,13 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
hpd = gpio_get_value(ip_data->hpd_gpio);
- if (hpd)
+ if (hpd) {
r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
- else
+ } else {
+ /* Invalidate EDID Cache */
+ ip_data->edid_len = 0;
r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON);
+ }
if (r) {
DSSERR("Failed to %s PHY TX power\n",
@@ -454,6 +457,11 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data,
{
int r, l;
+ if (ip_data->edid_len) {
+ memcpy(edid, ip_data->edid_cached, ip_data->edid_len);
+ return ip_data->edid_len;
+ }
+
if (len < 128)
return -EINVAL;
@@ -474,12 +482,21 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data,
l += 128;
}
+ ip_data->edid_len = l;
+ memcpy(ip_data->edid_cached, edid, l);
+
return l;
}
bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
{
- return gpio_get_value(ip_data->hpd_gpio);
+ if (gpio_get_value(ip_data->hpd_gpio))
+ return true;
+
+ /* Invalidate EDID Cache */
+ ip_data->edid_len = 0;
+
+ return false;
}
static void hdmi_core_init(struct hdmi_core_video_config *video_cfg,
--
1.7.4.1
^ permalink raw reply related
* [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: jaswinder.singh @ 2012-06-23 8:18 UTC (permalink / raw)
To: tomi.valkeinen, mythripk
Cc: linux-omap, linux-fbdev, andy.green, n-dechesne, Jassi Brar
From: Jassi Brar <jaswinder.singh@linaro.org>
If the runtime PM of the device is disabled (for example in resume from
suspend path), it doesn't make sense to attempt pm_runtime_get/put, esp
when their return values affect the control flow path.
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
Currenlty HDMI fails to come up in the suspend-resume path.
This patch helps that real-world scenario.
drivers/video/omap2/dss/dispc.c | 6 ++++++
drivers/video/omap2/dss/dsi.c | 6 ++++++
drivers/video/omap2/dss/dss.c | 6 ++++++
drivers/video/omap2/dss/hdmi.c | 6 ++++++
drivers/video/omap2/dss/rfbi.c | 6 ++++++
drivers/video/omap2/dss/venc.c | 6 ++++++
6 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 4749ac3..2c3266f 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -372,6 +372,9 @@ int dispc_runtime_get(void)
DSSDBG("dispc_runtime_get\n");
+ if (!pm_runtime_enabled(&dispc.pdev->dev))
+ return 0;
+
r = pm_runtime_get_sync(&dispc.pdev->dev);
WARN_ON(r < 0);
return r < 0 ? r : 0;
@@ -383,6 +386,9 @@ void dispc_runtime_put(void)
DSSDBG("dispc_runtime_put\n");
+ if (!pm_runtime_enabled(&dispc.pdev->dev))
+ return;
+
r = pm_runtime_put_sync(&dispc.pdev->dev);
WARN_ON(r < 0);
}
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index ca8382d..6db4cb1 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -1062,6 +1062,9 @@ int dsi_runtime_get(struct platform_device *dsidev)
DSSDBG("dsi_runtime_get\n");
+ if (!pm_runtime_enabled(&dsi->pdev->dev))
+ return 0;
+
r = pm_runtime_get_sync(&dsi->pdev->dev);
WARN_ON(r < 0);
return r < 0 ? r : 0;
@@ -1074,6 +1077,9 @@ void dsi_runtime_put(struct platform_device *dsidev)
DSSDBG("dsi_runtime_put\n");
+ if (!pm_runtime_enabled(&dsi->pdev->dev))
+ return;
+
r = pm_runtime_put_sync(&dsi->pdev->dev);
WARN_ON(r < 0);
}
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 7706323..5e52224 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -719,6 +719,9 @@ static int dss_runtime_get(void)
DSSDBG("dss_runtime_get\n");
+ if (!pm_runtime_enabled(&dss.pdev->dev))
+ return 0;
+
r = pm_runtime_get_sync(&dss.pdev->dev);
WARN_ON(r < 0);
return r < 0 ? r : 0;
@@ -730,6 +733,9 @@ static void dss_runtime_put(void)
DSSDBG("dss_runtime_put\n");
+ if (!pm_runtime_enabled(&dss.pdev->dev))
+ return;
+
r = pm_runtime_put_sync(&dss.pdev->dev);
WARN_ON(r < 0 && r != -EBUSY);
}
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 0738090..900e621 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -123,6 +123,9 @@ static int hdmi_runtime_get(void)
DSSDBG("hdmi_runtime_get\n");
+ if (!pm_runtime_enabled(&hdmi.pdev->dev))
+ return 0;
+
r = pm_runtime_get_sync(&hdmi.pdev->dev);
WARN_ON(r < 0);
if (r < 0)
@@ -137,6 +140,9 @@ static void hdmi_runtime_put(void)
DSSDBG("hdmi_runtime_put\n");
+ if (!pm_runtime_enabled(&hdmi.pdev->dev))
+ return;
+
r = pm_runtime_put_sync(&hdmi.pdev->dev);
WARN_ON(r < 0);
}
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index 3d8c206..401384a 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -129,6 +129,9 @@ static int rfbi_runtime_get(void)
DSSDBG("rfbi_runtime_get\n");
+ if (!pm_runtime_enabled(&rfbi.pdev->dev))
+ return 0;
+
r = pm_runtime_get_sync(&rfbi.pdev->dev);
WARN_ON(r < 0);
return r < 0 ? r : 0;
@@ -140,6 +143,9 @@ static void rfbi_runtime_put(void)
DSSDBG("rfbi_runtime_put\n");
+ if (!pm_runtime_enabled(&rfbi.pdev->dev))
+ return;
+
r = pm_runtime_put_sync(&rfbi.pdev->dev);
WARN_ON(r < 0);
}
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 2b89739..edd8710 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -390,6 +390,9 @@ static int venc_runtime_get(void)
DSSDBG("venc_runtime_get\n");
+ if (!pm_runtime_enabled(&venc.pdev->dev))
+ return 0;
+
r = pm_runtime_get_sync(&venc.pdev->dev);
WARN_ON(r < 0);
return r < 0 ? r : 0;
@@ -401,6 +404,9 @@ static void venc_runtime_put(void)
DSSDBG("venc_runtime_put\n");
+ if (!pm_runtime_enabled(&venc.pdev->dev))
+ return;
+
r = pm_runtime_put_sync(&venc.pdev->dev);
WARN_ON(r < 0);
}
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH] grvga: Fix error handling issues
From: Julia Lawall @ 2012-06-22 16:21 UTC (permalink / raw)
To: Emil Goode
Cc: Julia Lawall, FlorianSchandinat, linux-fbdev, linux-kernel,
kernel-janitors
In-Reply-To: <1340382044.3963.2.camel@localhost>
On Fri, 22 Jun 2012, Emil Goode wrote:
> Good idea, I will take another look at it tomorrow.
There is a devm_ function that combines request_mem_region and ioremap
that could be useful.
julia
>
> Thanks,
>
> Emil
>
> On Fri, 2012-06-22 at 18:12 +0200, Julia Lawall wrote:
> > Maybe you could used the devm_ functions for request_mem_region and
> > ioremap so that the error handling can just be dropped?
> >
> > julia
> >
> > On Fri, 22 Jun 2012, Emil Goode wrote:
> >
> > > This patch fixes two problems with the error handling in the
> > > grvga_probe function.
> > >
> > > - If the call to grvga_parse_custom on line 370 fails we use
> > > the wrong label so that release_mem_region will be called
> > > without a call to request_mem_region being made.
> > >
> > > - If the call to ioremap on line 436 fails we should not try
> > > to call iounmap. I added an if statement to check whether or
> > > not a call to iounmap should be made.
> > >
> > > - I also changed the names of the labels to make the code
> > > easier to read.
> > >
> > > Signed-off-by: Emil Goode <emilgoode@gmail.com>
> > > ---
> > > drivers/video/grvga.c | 34 ++++++++++++++++++----------------
> > > 1 file changed, 18 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/video/grvga.c b/drivers/video/grvga.c
> > > index da066c2..d9d688a 100644
> > > --- a/drivers/video/grvga.c
> > > +++ b/drivers/video/grvga.c
> > > @@ -354,7 +354,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > */
> > > if (fb_get_options("grvga", &options)) {
> > > retval = -ENODEV;
> > > - goto err;
> > > + goto free_fb;
> > > }
> > >
> > > if (!options || !*options)
> > > @@ -370,7 +370,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > if (grvga_parse_custom(this_opt, &info->var) < 0) {
> > > dev_err(&dev->dev, "Failed to parse custom mode (%s).\n", this_opt);
> > > retval = -EINVAL;
> > > - goto err1;
> > > + goto free_fb;
> > > }
> > > } else if (!strncmp(this_opt, "addr", 4))
> > > grvga_fix_addr = simple_strtoul(this_opt + 5, NULL, 16);
> > > @@ -390,7 +390,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > if (!request_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]), "grlib-svgactrl regs")) {
> > > dev_err(&dev->dev, "registers already mapped\n");
> > > retval = -EBUSY;
> > > - goto err;
> > > + goto free_fb;
> > > }
> > >
> > > par->regs = of_ioremap(&dev->resource[0], 0,
> > > @@ -400,14 +400,14 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > if (!par->regs) {
> > > dev_err(&dev->dev, "failed to map registers\n");
> > > retval = -ENOMEM;
> > > - goto err1;
> > > + goto release_regs;
> > > }
> > >
> > > retval = fb_alloc_cmap(&info->cmap, 256, 0);
> > > if (retval < 0) {
> > > dev_err(&dev->dev, "failed to allocate mem with fb_alloc_cmap\n");
> > > retval = -ENOMEM;
> > > - goto err2;
> > > + goto unmap_regs;
> > > }
> > >
> > > if (mode_opt) {
> > > @@ -415,7 +415,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > grvga_modedb, sizeof(grvga_modedb), &grvga_modedb[0], 8);
> > > if (!retval || retval = 4) {
> > > retval = -EINVAL;
> > > - goto err3;
> > > + goto dealloc_cmap;
> > > }
> > > }
> > >
> > > @@ -430,7 +430,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > if (!request_mem_region(physical_start, grvga_mem_size, dev->name)) {
> > > dev_err(&dev->dev, "failed to request memory region\n");
> > > retval = -ENOMEM;
> > > - goto err3;
> > > + goto dealloc_cmap;
> > > }
> > >
> > > virtual_start = (unsigned long) ioremap(physical_start, grvga_mem_size);
> > > @@ -438,7 +438,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > if (!virtual_start) {
> > > dev_err(&dev->dev, "error mapping framebuffer memory\n");
> > > retval = -ENOMEM;
> > > - goto err4;
> > > + goto free_mem;
> > > }
> > > } else { /* Allocate frambuffer memory */
> > >
> > > @@ -451,7 +451,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > "unable to allocate framebuffer memory (%lu bytes)\n",
> > > grvga_mem_size);
> > > retval = -ENOMEM;
> > > - goto err3;
> > > + goto dealloc_cmap;
> > > }
> > >
> > > physical_start = dma_map_single(&dev->dev, (void *)virtual_start, grvga_mem_size, DMA_TO_DEVICE);
> > > @@ -484,7 +484,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > retval = register_framebuffer(info);
> > > if (retval < 0) {
> > > dev_err(&dev->dev, "failed to register framebuffer\n");
> > > - goto err4;
> > > + goto free_mem;
> > > }
> > >
> > > __raw_writel(physical_start, &par->regs->fb_pos);
> > > @@ -493,21 +493,23 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > >
> > > return 0;
> > >
> > > -err4:
> > > +free_mem:
> > > dev_set_drvdata(&dev->dev, NULL);
> > > if (grvga_fix_addr) {
> > > release_mem_region(physical_start, grvga_mem_size);
> > > - iounmap((void *)virtual_start);
> > > + if (virtual_start)
> > > + iounmap((void *)virtual_start);
> > > } else
> > > kfree((void *)virtual_start);
> > > -err3:
> > > +
> > > +dealloc_cmap:
> > > fb_dealloc_cmap(&info->cmap);
> > > -err2:
> > > +unmap_regs:
> > > of_iounmap(&dev->resource[0], par->regs,
> > > resource_size(&dev->resource[0]));
> > > -err1:
> > > +release_regs:
> > > release_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]));
> > > -err:
> > > +free_fb:
> > > framebuffer_release(info);
> > >
> > > return retval;
> > > --
> > > 1.7.10
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH] grvga: Fix error handling issues
From: Emil Goode @ 2012-06-22 16:20 UTC (permalink / raw)
To: Julia Lawall
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <alpine.DEB.2.02.1206221811150.1901@hadrien>
Good idea, I will take another look at it tomorrow.
Thanks,
Emil
On Fri, 2012-06-22 at 18:12 +0200, Julia Lawall wrote:
> Maybe you could used the devm_ functions for request_mem_region and
> ioremap so that the error handling can just be dropped?
>
> julia
>
> On Fri, 22 Jun 2012, Emil Goode wrote:
>
> > This patch fixes two problems with the error handling in the
> > grvga_probe function.
> >
> > - If the call to grvga_parse_custom on line 370 fails we use
> > the wrong label so that release_mem_region will be called
> > without a call to request_mem_region being made.
> >
> > - If the call to ioremap on line 436 fails we should not try
> > to call iounmap. I added an if statement to check whether or
> > not a call to iounmap should be made.
> >
> > - I also changed the names of the labels to make the code
> > easier to read.
> >
> > Signed-off-by: Emil Goode <emilgoode@gmail.com>
> > ---
> > drivers/video/grvga.c | 34 ++++++++++++++++++----------------
> > 1 file changed, 18 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/video/grvga.c b/drivers/video/grvga.c
> > index da066c2..d9d688a 100644
> > --- a/drivers/video/grvga.c
> > +++ b/drivers/video/grvga.c
> > @@ -354,7 +354,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > */
> > if (fb_get_options("grvga", &options)) {
> > retval = -ENODEV;
> > - goto err;
> > + goto free_fb;
> > }
> >
> > if (!options || !*options)
> > @@ -370,7 +370,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > if (grvga_parse_custom(this_opt, &info->var) < 0) {
> > dev_err(&dev->dev, "Failed to parse custom mode (%s).\n", this_opt);
> > retval = -EINVAL;
> > - goto err1;
> > + goto free_fb;
> > }
> > } else if (!strncmp(this_opt, "addr", 4))
> > grvga_fix_addr = simple_strtoul(this_opt + 5, NULL, 16);
> > @@ -390,7 +390,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > if (!request_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]), "grlib-svgactrl regs")) {
> > dev_err(&dev->dev, "registers already mapped\n");
> > retval = -EBUSY;
> > - goto err;
> > + goto free_fb;
> > }
> >
> > par->regs = of_ioremap(&dev->resource[0], 0,
> > @@ -400,14 +400,14 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > if (!par->regs) {
> > dev_err(&dev->dev, "failed to map registers\n");
> > retval = -ENOMEM;
> > - goto err1;
> > + goto release_regs;
> > }
> >
> > retval = fb_alloc_cmap(&info->cmap, 256, 0);
> > if (retval < 0) {
> > dev_err(&dev->dev, "failed to allocate mem with fb_alloc_cmap\n");
> > retval = -ENOMEM;
> > - goto err2;
> > + goto unmap_regs;
> > }
> >
> > if (mode_opt) {
> > @@ -415,7 +415,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > grvga_modedb, sizeof(grvga_modedb), &grvga_modedb[0], 8);
> > if (!retval || retval = 4) {
> > retval = -EINVAL;
> > - goto err3;
> > + goto dealloc_cmap;
> > }
> > }
> >
> > @@ -430,7 +430,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > if (!request_mem_region(physical_start, grvga_mem_size, dev->name)) {
> > dev_err(&dev->dev, "failed to request memory region\n");
> > retval = -ENOMEM;
> > - goto err3;
> > + goto dealloc_cmap;
> > }
> >
> > virtual_start = (unsigned long) ioremap(physical_start, grvga_mem_size);
> > @@ -438,7 +438,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > if (!virtual_start) {
> > dev_err(&dev->dev, "error mapping framebuffer memory\n");
> > retval = -ENOMEM;
> > - goto err4;
> > + goto free_mem;
> > }
> > } else { /* Allocate frambuffer memory */
> >
> > @@ -451,7 +451,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > "unable to allocate framebuffer memory (%lu bytes)\n",
> > grvga_mem_size);
> > retval = -ENOMEM;
> > - goto err3;
> > + goto dealloc_cmap;
> > }
> >
> > physical_start = dma_map_single(&dev->dev, (void *)virtual_start, grvga_mem_size, DMA_TO_DEVICE);
> > @@ -484,7 +484,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > retval = register_framebuffer(info);
> > if (retval < 0) {
> > dev_err(&dev->dev, "failed to register framebuffer\n");
> > - goto err4;
> > + goto free_mem;
> > }
> >
> > __raw_writel(physical_start, &par->regs->fb_pos);
> > @@ -493,21 +493,23 @@ static int __devinit grvga_probe(struct platform_device *dev)
> >
> > return 0;
> >
> > -err4:
> > +free_mem:
> > dev_set_drvdata(&dev->dev, NULL);
> > if (grvga_fix_addr) {
> > release_mem_region(physical_start, grvga_mem_size);
> > - iounmap((void *)virtual_start);
> > + if (virtual_start)
> > + iounmap((void *)virtual_start);
> > } else
> > kfree((void *)virtual_start);
> > -err3:
> > +
> > +dealloc_cmap:
> > fb_dealloc_cmap(&info->cmap);
> > -err2:
> > +unmap_regs:
> > of_iounmap(&dev->resource[0], par->regs,
> > resource_size(&dev->resource[0]));
> > -err1:
> > +release_regs:
> > release_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]));
> > -err:
> > +free_fb:
> > framebuffer_release(info);
> >
> > return retval;
> > --
> > 1.7.10
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
^ permalink raw reply
* Re: [PATCH] grvga: Fix error handling issues
From: Julia Lawall @ 2012-06-22 16:12 UTC (permalink / raw)
To: Emil Goode; +Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <1340381479-6371-1-git-send-email-emilgoode@gmail.com>
Maybe you could used the devm_ functions for request_mem_region and
ioremap so that the error handling can just be dropped?
julia
On Fri, 22 Jun 2012, Emil Goode wrote:
> This patch fixes two problems with the error handling in the
> grvga_probe function.
>
> - If the call to grvga_parse_custom on line 370 fails we use
> the wrong label so that release_mem_region will be called
> without a call to request_mem_region being made.
>
> - If the call to ioremap on line 436 fails we should not try
> to call iounmap. I added an if statement to check whether or
> not a call to iounmap should be made.
>
> - I also changed the names of the labels to make the code
> easier to read.
>
> Signed-off-by: Emil Goode <emilgoode@gmail.com>
> ---
> drivers/video/grvga.c | 34 ++++++++++++++++++----------------
> 1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/video/grvga.c b/drivers/video/grvga.c
> index da066c2..d9d688a 100644
> --- a/drivers/video/grvga.c
> +++ b/drivers/video/grvga.c
> @@ -354,7 +354,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> */
> if (fb_get_options("grvga", &options)) {
> retval = -ENODEV;
> - goto err;
> + goto free_fb;
> }
>
> if (!options || !*options)
> @@ -370,7 +370,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> if (grvga_parse_custom(this_opt, &info->var) < 0) {
> dev_err(&dev->dev, "Failed to parse custom mode (%s).\n", this_opt);
> retval = -EINVAL;
> - goto err1;
> + goto free_fb;
> }
> } else if (!strncmp(this_opt, "addr", 4))
> grvga_fix_addr = simple_strtoul(this_opt + 5, NULL, 16);
> @@ -390,7 +390,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> if (!request_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]), "grlib-svgactrl regs")) {
> dev_err(&dev->dev, "registers already mapped\n");
> retval = -EBUSY;
> - goto err;
> + goto free_fb;
> }
>
> par->regs = of_ioremap(&dev->resource[0], 0,
> @@ -400,14 +400,14 @@ static int __devinit grvga_probe(struct platform_device *dev)
> if (!par->regs) {
> dev_err(&dev->dev, "failed to map registers\n");
> retval = -ENOMEM;
> - goto err1;
> + goto release_regs;
> }
>
> retval = fb_alloc_cmap(&info->cmap, 256, 0);
> if (retval < 0) {
> dev_err(&dev->dev, "failed to allocate mem with fb_alloc_cmap\n");
> retval = -ENOMEM;
> - goto err2;
> + goto unmap_regs;
> }
>
> if (mode_opt) {
> @@ -415,7 +415,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> grvga_modedb, sizeof(grvga_modedb), &grvga_modedb[0], 8);
> if (!retval || retval = 4) {
> retval = -EINVAL;
> - goto err3;
> + goto dealloc_cmap;
> }
> }
>
> @@ -430,7 +430,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> if (!request_mem_region(physical_start, grvga_mem_size, dev->name)) {
> dev_err(&dev->dev, "failed to request memory region\n");
> retval = -ENOMEM;
> - goto err3;
> + goto dealloc_cmap;
> }
>
> virtual_start = (unsigned long) ioremap(physical_start, grvga_mem_size);
> @@ -438,7 +438,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> if (!virtual_start) {
> dev_err(&dev->dev, "error mapping framebuffer memory\n");
> retval = -ENOMEM;
> - goto err4;
> + goto free_mem;
> }
> } else { /* Allocate frambuffer memory */
>
> @@ -451,7 +451,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> "unable to allocate framebuffer memory (%lu bytes)\n",
> grvga_mem_size);
> retval = -ENOMEM;
> - goto err3;
> + goto dealloc_cmap;
> }
>
> physical_start = dma_map_single(&dev->dev, (void *)virtual_start, grvga_mem_size, DMA_TO_DEVICE);
> @@ -484,7 +484,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> retval = register_framebuffer(info);
> if (retval < 0) {
> dev_err(&dev->dev, "failed to register framebuffer\n");
> - goto err4;
> + goto free_mem;
> }
>
> __raw_writel(physical_start, &par->regs->fb_pos);
> @@ -493,21 +493,23 @@ static int __devinit grvga_probe(struct platform_device *dev)
>
> return 0;
>
> -err4:
> +free_mem:
> dev_set_drvdata(&dev->dev, NULL);
> if (grvga_fix_addr) {
> release_mem_region(physical_start, grvga_mem_size);
> - iounmap((void *)virtual_start);
> + if (virtual_start)
> + iounmap((void *)virtual_start);
> } else
> kfree((void *)virtual_start);
> -err3:
> +
> +dealloc_cmap:
> fb_dealloc_cmap(&info->cmap);
> -err2:
> +unmap_regs:
> of_iounmap(&dev->resource[0], par->regs,
> resource_size(&dev->resource[0]));
> -err1:
> +release_regs:
> release_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]));
> -err:
> +free_fb:
> framebuffer_release(info);
>
> return retval;
> --
> 1.7.10
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" 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] grvga: Fix error handling issues
From: Emil Goode @ 2012-06-22 16:11 UTC (permalink / raw)
To: FlorianSchandinat; +Cc: linux-fbdev, linux-kernel, kernel-janitors, Emil Goode
This patch fixes two problems with the error handling in the
grvga_probe function.
- If the call to grvga_parse_custom on line 370 fails we use
the wrong label so that release_mem_region will be called
without a call to request_mem_region being made.
- If the call to ioremap on line 436 fails we should not try
to call iounmap. I added an if statement to check whether or
not a call to iounmap should be made.
- I also changed the names of the labels to make the code
easier to read.
Signed-off-by: Emil Goode <emilgoode@gmail.com>
---
drivers/video/grvga.c | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/video/grvga.c b/drivers/video/grvga.c
index da066c2..d9d688a 100644
--- a/drivers/video/grvga.c
+++ b/drivers/video/grvga.c
@@ -354,7 +354,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
*/
if (fb_get_options("grvga", &options)) {
retval = -ENODEV;
- goto err;
+ goto free_fb;
}
if (!options || !*options)
@@ -370,7 +370,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
if (grvga_parse_custom(this_opt, &info->var) < 0) {
dev_err(&dev->dev, "Failed to parse custom mode (%s).\n", this_opt);
retval = -EINVAL;
- goto err1;
+ goto free_fb;
}
} else if (!strncmp(this_opt, "addr", 4))
grvga_fix_addr = simple_strtoul(this_opt + 5, NULL, 16);
@@ -390,7 +390,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
if (!request_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]), "grlib-svgactrl regs")) {
dev_err(&dev->dev, "registers already mapped\n");
retval = -EBUSY;
- goto err;
+ goto free_fb;
}
par->regs = of_ioremap(&dev->resource[0], 0,
@@ -400,14 +400,14 @@ static int __devinit grvga_probe(struct platform_device *dev)
if (!par->regs) {
dev_err(&dev->dev, "failed to map registers\n");
retval = -ENOMEM;
- goto err1;
+ goto release_regs;
}
retval = fb_alloc_cmap(&info->cmap, 256, 0);
if (retval < 0) {
dev_err(&dev->dev, "failed to allocate mem with fb_alloc_cmap\n");
retval = -ENOMEM;
- goto err2;
+ goto unmap_regs;
}
if (mode_opt) {
@@ -415,7 +415,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
grvga_modedb, sizeof(grvga_modedb), &grvga_modedb[0], 8);
if (!retval || retval = 4) {
retval = -EINVAL;
- goto err3;
+ goto dealloc_cmap;
}
}
@@ -430,7 +430,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
if (!request_mem_region(physical_start, grvga_mem_size, dev->name)) {
dev_err(&dev->dev, "failed to request memory region\n");
retval = -ENOMEM;
- goto err3;
+ goto dealloc_cmap;
}
virtual_start = (unsigned long) ioremap(physical_start, grvga_mem_size);
@@ -438,7 +438,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
if (!virtual_start) {
dev_err(&dev->dev, "error mapping framebuffer memory\n");
retval = -ENOMEM;
- goto err4;
+ goto free_mem;
}
} else { /* Allocate frambuffer memory */
@@ -451,7 +451,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
"unable to allocate framebuffer memory (%lu bytes)\n",
grvga_mem_size);
retval = -ENOMEM;
- goto err3;
+ goto dealloc_cmap;
}
physical_start = dma_map_single(&dev->dev, (void *)virtual_start, grvga_mem_size, DMA_TO_DEVICE);
@@ -484,7 +484,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
retval = register_framebuffer(info);
if (retval < 0) {
dev_err(&dev->dev, "failed to register framebuffer\n");
- goto err4;
+ goto free_mem;
}
__raw_writel(physical_start, &par->regs->fb_pos);
@@ -493,21 +493,23 @@ static int __devinit grvga_probe(struct platform_device *dev)
return 0;
-err4:
+free_mem:
dev_set_drvdata(&dev->dev, NULL);
if (grvga_fix_addr) {
release_mem_region(physical_start, grvga_mem_size);
- iounmap((void *)virtual_start);
+ if (virtual_start)
+ iounmap((void *)virtual_start);
} else
kfree((void *)virtual_start);
-err3:
+
+dealloc_cmap:
fb_dealloc_cmap(&info->cmap);
-err2:
+unmap_regs:
of_iounmap(&dev->resource[0], par->regs,
resource_size(&dev->resource[0]));
-err1:
+release_regs:
release_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]));
-err:
+free_fb:
framebuffer_release(info);
return retval;
--
1.7.10
^ permalink raw reply related
* [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
From: Rajendra Nayak @ 2012-06-22 13:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1340372890-10091-1-git-send-email-rnayak@ti.com>
In preparation of OMAP moving to Common Clk Framework(CCF) add clk_prepare()
and clk_unprepare() for the omapdss clocks.
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: <linux-fbdev@vger.kernel.org>
---
drivers/video/omap2/dss/dsi.c | 5 ++++-
drivers/video/omap2/dss/hdmi.c | 5 ++++-
drivers/video/omap2/dss/venc.c | 5 ++++-
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index ca8382d..81b474d 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -4776,6 +4776,7 @@ static int dsi_get_clocks(struct platform_device *dsidev)
}
dsi->sys_clk = clk;
+ clk_prepare(dsi->sys_clk);
return 0;
}
@@ -4786,8 +4787,10 @@ static void dsi_put_clocks(struct platform_device *dsidev)
if (dsi->dss_clk)
clk_put(dsi->dss_clk);
- if (dsi->sys_clk)
+ if (dsi->sys_clk) {
+ clk_unprepare(dsi->sys_clk);
clk_put(dsi->sys_clk);
+ }
}
static void __init dsi_probe_pdata(struct platform_device *dsidev)
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 8195c71..0b7ded3 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -544,14 +544,17 @@ static int hdmi_get_clocks(struct platform_device *pdev)
}
hdmi.sys_clk = clk;
+ clk_prepare(hdmi.sys_clk);
return 0;
}
static void hdmi_put_clocks(void)
{
- if (hdmi.sys_clk)
+ if (hdmi.sys_clk) {
+ clk_unprepare(hdmi.sys_clk);
clk_put(hdmi.sys_clk);
+ }
}
#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 2b89739..62f395d 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -818,14 +818,17 @@ static int venc_get_clocks(struct platform_device *pdev)
}
venc.tv_dac_clk = clk;
+ clk_prepare(venc.tv_dac_clk);
return 0;
}
static void venc_put_clocks(void)
{
- if (venc.tv_dac_clk)
+ if (venc.tv_dac_clk) {
+ clk_unprepare(venc.tv_dac_clk);
clk_put(venc.tv_dac_clk);
+ }
}
static void __init venc_probe_pdata(struct platform_device *pdev)
--
1.7.1
^ permalink raw reply related
* [PATCH] fbdev: Add ioctls FBIO_COPYRECT, FBIO_FILLRECT, lest these operations be done in software.
From: Strake @ 2012-06-21 12:22 UTC (permalink / raw)
To: Florian Tobias Schandinat, linux-fbdev, linux-kernel
From: Matthew Farkas-Dyck <strake888@gmail.com>
Add ioctls FBIO_COPYRECT, FBIO_FILLRECT, lest these operations be done
in software.
Signed-off-by: Matthew Farkas-Dyck <strake888@gmail.com>
---
Reason: If it can be done in hardware, it ought to be. This will
enable user-mode software to do so.
commit 2c002c58b6c5d5b211423da8eaa242ca443fdd72
Author: strake <strake888@gmail.com>
Date: Wed Jan 1 19:10:24 2003 -0500
Add ioctls FBIO_COPYRECT, FBIO_FILLRECT
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 4ac1201..f525e91 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1019,6 +1019,8 @@ static long do_fb_ioctl(struct fb_info *info,
unsigned int cmd,
struct fb_cmap cmap_from;
struct fb_cmap_user cmap;
struct fb_event event;
+ struct fb_copyarea copy;
+ struct fb_fillrect fill;
void __user *argp = (void __user *)arg;
long ret = 0;
@@ -1126,6 +1128,26 @@ static long do_fb_ioctl(struct fb_info *info,
unsigned int cmd,
console_unlock();
unlock_fb_info(info);
break;
+ case FBIO_FILLRECT:
+ if (copy_from_user(&fill, argp, sizeof(fill)))
+ return -EFAULT;
+ if (!lock_fb_info(info))
+ return -ENODEV;
+ console_lock();
+ (*(info->fbops->fb_fillrect))(info, &fill);
+ console_unlock();
+ unlock_fb_info(info);
+ break;
+ case FBIO_COPYRECT:
+ if (copy_from_user(©, argp, sizeof(copy)))
+ return -EFAULT;
+ if (!lock_fb_info(info))
+ return -ENODEV;
+ console_lock();
+ (*(info->fbops->fb_copyarea))(info, ©);
+ console_unlock();
+ unlock_fb_info(info);
+ break;
default:
if (!lock_fb_info(info))
return -ENODEV;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index d1631d3..fc18459 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -39,6 +39,8 @@
#define FBIOPUT_MODEINFO 0x4617
#define FBIOGET_DISPINFO 0x4618
#define FBIO_WAITFORVSYNC _IOW('F', 0x20, __u32)
+#define FBIO_COPYRECT _IOW('F', 0x22, struct fb_copyarea)
+#define FBIO_FILLRECT _IOW('F', 0x23, struct fb_fillrect)
#define FB_TYPE_PACKED_PIXELS 0 /* Packed Pixels */
#define FB_TYPE_PLANES 1 /* Non interleaved planes */
^ permalink raw reply related
* [PATCH] video: exynos_dp: remove duplicated declarations from header file
From: Jingoo Han @ 2012-06-21 9:57 UTC (permalink / raw)
To: linux-fbdev
Some functions are declared twice in header file; thus, these
declarations are unnecessary.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.h | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 1e0f998..8526e54 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -85,10 +85,6 @@ void exynos_dp_set_link_bandwidth(struct exynos_dp_device *dp, u32 bwtype);
void exynos_dp_get_link_bandwidth(struct exynos_dp_device *dp, u32 *bwtype);
void exynos_dp_set_lane_count(struct exynos_dp_device *dp, u32 count);
void exynos_dp_get_lane_count(struct exynos_dp_device *dp, u32 *count);
-void exynos_dp_set_link_bandwidth(struct exynos_dp_device *dp, u32 bwtype);
-void exynos_dp_get_link_bandwidth(struct exynos_dp_device *dp, u32 *bwtype);
-void exynos_dp_set_lane_count(struct exynos_dp_device *dp, u32 count);
-void exynos_dp_get_lane_count(struct exynos_dp_device *dp, u32 *count);
void exynos_dp_enable_enhanced_mode(struct exynos_dp_device *dp, bool enable);
void exynos_dp_set_training_pattern(struct exynos_dp_device *dp,
enum pattern_set pattern);
--
1.7.1
^ permalink raw reply related
* Re: [Linaro-mm-sig] [RFC/PATCH] fb: Add dma-buf support
From: Laurent Pinchart @ 2012-06-21 9:05 UTC (permalink / raw)
To: Tomasz Stanislawski; +Cc: linux-fbdev, linaro-mm-sig, linux-media
In-Reply-To: <4FE1EF42.1000403@samsung.com>
Hi Tomasz,
Thank you for the review.
On Wednesday 20 June 2012 17:41:54 Tomasz Stanislawski wrote:
> Hi Laurent,
> Thank you for the patch.
>
> On 06/20/2012 04:09 PM, Laurent Pinchart wrote:
> > Add support for the dma-buf exporter role to the frame buffer API. The
> > importer role isn't meaningful for frame buffer devices, as the frame
> > buffer device model doesn't allow using externally allocated memory.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >
> > Documentation/fb/api.txt | 36 ++++++++++++++++++++++++++++++++++++
> > drivers/video/fbmem.c | 36 ++++++++++++++++++++++++++++++++++++
> > include/linux/fb.h | 12 ++++++++++++
> > 3 files changed, 84 insertions(+), 0 deletions(-)
>
> [snip]
>
> > +The export a frame buffer as a dma-buf file descriptors, applications
> > call the +FBIOGET_DMABUF ioctl. The ioctl takes a pointer to a
> > fb_dmabuf_export +structure.
> > +
> > +struct fb_dmabuf_export {
> > + __u32 fd;
> > + __u32 flags;
> > +};
>
> What do you think about adding some reserved fields to struct
> fb_dmabuf_export to make it future-proof for DMABUF extensions?
Already done. I've added them at the last minute, and for some reason I've
sent the previous version of the patch.
> > +
> > +The flag field specifies the flags to be used when creating the dma-buf
> > file +descriptor. The only supported flag is O_CLOEXEC. If the call is
> > successful, +the driver will set the fd field to a file descriptor
> > corresponding to the +dma-buf object.
> > +
> > +Applications can then pass the file descriptors to another application or
> > +another device driver. The dma-buf object is automatically
> > reference-counted, +applications can and should close the file descriptor
> > as soon as they don't +need it anymore. The underlying dma-buf object
> > will not be freed before the +last device that uses the dma-buf object
> > releases it.
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index 0dff12a..400e449 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -15,6 +15,7 @@
> >
> > #include <linux/compat.h>
> > #include <linux/types.h>
> >
> > +#include <linux/dma-buf.h>
> >
> > #include <linux/errno.h>
> > #include <linux/kernel.h>
> > #include <linux/major.h>
> >
> > @@ -1074,6 +1075,23 @@ fb_blank(struct fb_info *info, int blank)
> >
> > return ret;
> >
> > }
> >
> > +#ifdef CONFIG_DMA_SHARED_BUFFER
> > +int
> > +fb_get_dmabuf(struct fb_info *info, int flags)
> > +{
> > + struct dma_buf *dmabuf;
> > +
> > + if (info->fbops->fb_dmabuf_export = NULL)
> > + return -ENOTTY;
> > +
> > + dmabuf = info->fbops->fb_dmabuf_export(info);
>
> IMO, it is not a good idea to delegate an implementation of DMABUF ops to
> the driver. Maybe exporting could be handled inside FB stack. As I
> understand, the FB stack needs only 'get-scatterlist' ops from an FB driver.
> All other DMABUF magic does not need driver involvement, does it?
Beside creating the sg-list, drivers also need to prevent the underlying
memory from being freed. If an application requests a frame buffer format or
size change, the driver could resize the underlying memory, which needs to be
prevented as long as the buffer is exported.
The dma-buf support implementation in the sh_mobile_lcdcfb driver also takes a
reference to the device and the driver for the duration of the export, to
prevent the driver from being unloaded and the device from being released.
static struct dma_buf *sh_mobile_dmabuf_export(struct fb_info *info)
{
struct sh_mobile_lcdc_chan *ch = info->par;
struct dma_buf *dmabuf;
dmabuf = dma_buf_export(ch, &sh_mobile_lcdc_dmabuf_ops, ch->fb_size, 0);
if (!IS_ERR(dmabuf)) {
/* We already hold references to the module and the device, so
* we don't have to take them prior to calling dma_buf_export().
*/
get_device(ch->lcdc->dev);
__module_get(THIS_MODULE);
}
return dmabuf;
}
This could be moved in the fbdev core though, as ch->lcdc->dev is available
trhough info->dev, and THIS_MODULE could be replaced with info->dev->driver-
>owner.
Moving the implementation to the fbdev core would thus require 3 callbacks to
lock the frame buffer, get the sg list, and release the frame buffer. However,
in practice, we might not have drivers that resize buffers when the format or
size is modified, in which case a single callback would be enough.
> > + if (IS_ERR(dmabuf))
> > + return PTR_ERR(dmabuf);
> > +
> > + return dma_buf_fd(dmabuf, flags);
> > +}
> > +#endif
> > +
>
> [snip]
>
> > diff --git a/include/linux/fb.h b/include/linux/fb.h
> > index ac3f1c6..c9fee75 100644
> > --- a/include/linux/fb.h
> > +++ b/include/linux/fb.h
> > @@ -701,6 +708,11 @@ struct fb_ops {
> >
> > /* called at KDB enter and leave time to prepare the console */
> > int (*fb_debug_enter)(struct fb_info *info);
> > int (*fb_debug_leave)(struct fb_info *info);
> >
> > +
> > +#ifdef CONFIG_DMA_SHARED_BUFFER
> > + /* Export the frame buffer as a dmabuf object */
> > + struct dma_buf *(*fb_dmabuf_export)(struct fb_info *info);
> > +#endif
>
> Memory trashing or even kernel crash may happen if a module compiled
> with CONFIG_DMA_SHARED_BUFFER enabled is loaded into kernel with
> CONFIG_DMA_SHARED_BUFFER disabled.
I'll remove the #ifdef. The memory savings are not worth the potential issues.
> > };
> >
> > #ifdef CONFIG_FB_TILEBLITTING
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [Linaro-mm-sig] [RFC/PATCH] fb: Add dma-buf support
From: Laurent Pinchart @ 2012-06-21 8:46 UTC (permalink / raw)
To: Jesse Barker; +Cc: linux-fbdev, linaro-mm-sig, linux-media
In-Reply-To: <CAC57bwtvfYDGO91r3zRry+WENm7x=UZ6TdOkXpmHDbQWT0feRA@mail.gmail.com>
Hi Jesse,
Thank you for the review.
On Wednesday 20 June 2012 08:09:19 Jesse Barker wrote:
> Laurent,
>
> Your recent documentation efforts are extremely commendable. Just a
> couple of small nits below...
>
> On Wed, Jun 20, 2012 at 7:09 AM, Laurent Pinchart
>
> <laurent.pinchart@ideasonboard.com> wrote:
> > Add support for the dma-buf exporter role to the frame buffer API. The
> > importer role isn't meaningful for frame buffer devices, as the frame
> > buffer device model doesn't allow using externally allocated memory.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Documentation/fb/api.txt | 36 ++++++++++++++++++++++++++++++++++++
> > drivers/video/fbmem.c | 36 ++++++++++++++++++++++++++++++++++++
> > include/linux/fb.h | 12 ++++++++++++
> > 3 files changed, 84 insertions(+), 0 deletions(-)
> >
> > diff --git a/Documentation/fb/api.txt b/Documentation/fb/api.txt
> > index d4ff7de..f0b2173 100644
> > --- a/Documentation/fb/api.txt
> > +++ b/Documentation/fb/api.txt
> > @@ -304,3 +304,39 @@ extensions.
> > Upon successful format configuration, drivers update the
> > fb_fix_screeninfo
> > type, visual and line_length fields depending on the selected format. The
> > type and visual fields are set to FB_TYPE_FOURCC and FB_VISUAL_FOURCC
> > respectively. +
> > +
> > +5. DMA buffer sharing
> > +---------------------
> > +
> > +The dma-buf kernel framework allows DMA buffers to be shared across
> > devices +and applications. Sharing buffers across display devices and
> > video capture or +video decoding devices allow zero-copy operation when
> > displaying video content +produced by a hardware device such as a camera
> > or a hardware codec. This is +crucial to achieve optimal system
> > performances during video display. +
> > +While dma-buf supports both exporting internally allocated memory as a
> > dma-buf +object (known as the exporter role) and importing a dma-buf
> > object to be used +as device memory (known as the importer role), the
> > frame buffer API only +supports the exporter role, as the frame buffer
> > device model doesn't support +using externally-allocated memory.
> > +
> > +The export a frame buffer as a dma-buf file descriptors, applications
> > call the
> s/The/To
> s/descriptors/descriptor
Fixed, thanks.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [Linaro-mm-sig] [RFC/PATCH] fb: Add dma-buf support
From: Tomasz Stanislawski @ 2012-06-20 15:41 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-fbdev, linaro-mm-sig, linux-media
In-Reply-To: <1340201368-20751-1-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Laurent,
Thank you for the patch.
On 06/20/2012 04:09 PM, Laurent Pinchart wrote:
> Add support for the dma-buf exporter role to the frame buffer API. The
> importer role isn't meaningful for frame buffer devices, as the frame
> buffer device model doesn't allow using externally allocated memory.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Documentation/fb/api.txt | 36 ++++++++++++++++++++++++++++++++++++
> drivers/video/fbmem.c | 36 ++++++++++++++++++++++++++++++++++++
> include/linux/fb.h | 12 ++++++++++++
> 3 files changed, 84 insertions(+), 0 deletions(-)
>
[snip]
> +The export a frame buffer as a dma-buf file descriptors, applications call the
> +FBIOGET_DMABUF ioctl. The ioctl takes a pointer to a fb_dmabuf_export
> +structure.
> +
> +struct fb_dmabuf_export {
> + __u32 fd;
> + __u32 flags;
> +};
What do you think about adding some reserved fields to
struct fb_dmabuf_export to make it future-proof for
DMABUF extensions?
> +
> +The flag field specifies the flags to be used when creating the dma-buf file
> +descriptor. The only supported flag is O_CLOEXEC. If the call is successful,
> +the driver will set the fd field to a file descriptor corresponding to the
> +dma-buf object.
> +
> +Applications can then pass the file descriptors to another application or
> +another device driver. The dma-buf object is automatically reference-counted,
> +applications can and should close the file descriptor as soon as they don't
> +need it anymore. The underlying dma-buf object will not be freed before the
> +last device that uses the dma-buf object releases it.
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 0dff12a..400e449 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -15,6 +15,7 @@
>
> #include <linux/compat.h>
> #include <linux/types.h>
> +#include <linux/dma-buf.h>
> #include <linux/errno.h>
> #include <linux/kernel.h>
> #include <linux/major.h>
> @@ -1074,6 +1075,23 @@ fb_blank(struct fb_info *info, int blank)
> return ret;
> }
>
> +#ifdef CONFIG_DMA_SHARED_BUFFER
> +int
> +fb_get_dmabuf(struct fb_info *info, int flags)
> +{
> + struct dma_buf *dmabuf;
> +
> + if (info->fbops->fb_dmabuf_export = NULL)
> + return -ENOTTY;
> +
> + dmabuf = info->fbops->fb_dmabuf_export(info);
IMO, it is not a good idea to delegate an implementation of
DMABUF ops to the driver. Maybe exporting could be handled
inside FB stack. As I understand, the FB stack needs only
'get-scatterlist' ops from an FB driver. All other
DMABUF magic does not need driver involvement, does it?
> + if (IS_ERR(dmabuf))
> + return PTR_ERR(dmabuf);
> +
> + return dma_buf_fd(dmabuf, flags);
> +}
> +#endif
> +
>
[snip]
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index ac3f1c6..c9fee75 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -701,6 +708,11 @@ struct fb_ops {
> /* called at KDB enter and leave time to prepare the console */
> int (*fb_debug_enter)(struct fb_info *info);
> int (*fb_debug_leave)(struct fb_info *info);
> +
> +#ifdef CONFIG_DMA_SHARED_BUFFER
> + /* Export the frame buffer as a dmabuf object */
> + struct dma_buf *(*fb_dmabuf_export)(struct fb_info *info);
> +#endif
Memory trashing or even kernel crash may happen if a module compiled
with CONFIG_DMA_SHARED_BUFFER enabled is loaded into kernel with
CONFIG_DMA_SHARED_BUFFER disabled.
> };
>
> #ifdef CONFIG_FB_TILEBLITTING
^ permalink raw reply
* Re: [Linaro-mm-sig] [RFC/PATCH] fb: Add dma-buf support
From: Jesse Barker @ 2012-06-20 15:09 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-fbdev, linaro-mm-sig, linux-media
In-Reply-To: <1340201368-20751-1-git-send-email-laurent.pinchart@ideasonboard.com>
Laurent,
Your recent documentation efforts are extremely commendable. Just a
couple of small nits below...
On Wed, Jun 20, 2012 at 7:09 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Add support for the dma-buf exporter role to the frame buffer API. The
> importer role isn't meaningful for frame buffer devices, as the frame
> buffer device model doesn't allow using externally allocated memory.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Documentation/fb/api.txt | 36 ++++++++++++++++++++++++++++++++++++
> drivers/video/fbmem.c | 36 ++++++++++++++++++++++++++++++++++++
> include/linux/fb.h | 12 ++++++++++++
> 3 files changed, 84 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/fb/api.txt b/Documentation/fb/api.txt
> index d4ff7de..f0b2173 100644
> --- a/Documentation/fb/api.txt
> +++ b/Documentation/fb/api.txt
> @@ -304,3 +304,39 @@ extensions.
> Upon successful format configuration, drivers update the fb_fix_screeninfo
> type, visual and line_length fields depending on the selected format. The type
> and visual fields are set to FB_TYPE_FOURCC and FB_VISUAL_FOURCC respectively.
> +
> +
> +5. DMA buffer sharing
> +---------------------
> +
> +The dma-buf kernel framework allows DMA buffers to be shared across devices
> +and applications. Sharing buffers across display devices and video capture or
> +video decoding devices allow zero-copy operation when displaying video content
> +produced by a hardware device such as a camera or a hardware codec. This is
> +crucial to achieve optimal system performances during video display.
> +
> +While dma-buf supports both exporting internally allocated memory as a dma-buf
> +object (known as the exporter role) and importing a dma-buf object to be used
> +as device memory (known as the importer role), the frame buffer API only
> +supports the exporter role, as the frame buffer device model doesn't support
> +using externally-allocated memory.
> +
> +The export a frame buffer as a dma-buf file descriptors, applications call the
s/The/To
s/descriptors/descriptor
cheers,
Jesse
> +FBIOGET_DMABUF ioctl. The ioctl takes a pointer to a fb_dmabuf_export
> +structure.
> +
> +struct fb_dmabuf_export {
> + __u32 fd;
> + __u32 flags;
> +};
> +
> +The flag field specifies the flags to be used when creating the dma-buf file
> +descriptor. The only supported flag is O_CLOEXEC. If the call is successful,
> +the driver will set the fd field to a file descriptor corresponding to the
> +dma-buf object.
> +
> +Applications can then pass the file descriptors to another application or
> +another device driver. The dma-buf object is automatically reference-counted,
> +applications can and should close the file descriptor as soon as they don't
> +need it anymore. The underlying dma-buf object will not be freed before the
> +last device that uses the dma-buf object releases it.
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 0dff12a..400e449 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -15,6 +15,7 @@
>
> #include <linux/compat.h>
> #include <linux/types.h>
> +#include <linux/dma-buf.h>
> #include <linux/errno.h>
> #include <linux/kernel.h>
> #include <linux/major.h>
> @@ -1074,6 +1075,23 @@ fb_blank(struct fb_info *info, int blank)
> return ret;
> }
>
> +#ifdef CONFIG_DMA_SHARED_BUFFER
> +int
> +fb_get_dmabuf(struct fb_info *info, int flags)
> +{
> + struct dma_buf *dmabuf;
> +
> + if (info->fbops->fb_dmabuf_export = NULL)
> + return -ENOTTY;
> +
> + dmabuf = info->fbops->fb_dmabuf_export(info);
> + if (IS_ERR(dmabuf))
> + return PTR_ERR(dmabuf);
> +
> + return dma_buf_fd(dmabuf, flags);
> +}
> +#endif
> +
> static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> unsigned long arg)
> {
> @@ -1084,6 +1102,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> struct fb_cmap cmap_from;
> struct fb_cmap_user cmap;
> struct fb_event event;
> + struct fb_dmabuf_export dmaexp;
> void __user *argp = (void __user *)arg;
> long ret = 0;
>
> @@ -1191,6 +1210,23 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> console_unlock();
> unlock_fb_info(info);
> break;
> +#ifdef CONFIG_DMA_SHARED_BUFFER
> + case FBIOGET_DMABUF:
> + if (copy_from_user(&dmaexp, argp, sizeof(dmaexp)))
> + return -EFAULT;
> +
> + if (!lock_fb_info(info))
> + return -ENODEV;
> + dmaexp.fd = fb_get_dmabuf(info, dmaexp.flags);
> + unlock_fb_info(info);
> +
> + if (dmaexp.fd < 0)
> + return dmaexp.fd;
> +
> + ret = copy_to_user(argp, &dmaexp, sizeof(dmaexp))
> + ? -EFAULT : 0;
> + break;
> +#endif
> default:
> if (!lock_fb_info(info))
> return -ENODEV;
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index ac3f1c6..c9fee75 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -39,6 +39,7 @@
> #define FBIOPUT_MODEINFO 0x4617
> #define FBIOGET_DISPINFO 0x4618
> #define FBIO_WAITFORVSYNC _IOW('F', 0x20, __u32)
> +#define FBIOGET_DMABUF _IOR('F', 0x21, struct fb_dmabuf_export)
>
> #define FB_TYPE_PACKED_PIXELS 0 /* Packed Pixels */
> #define FB_TYPE_PLANES 1 /* Non interleaved planes */
> @@ -403,6 +404,11 @@ struct fb_cursor {
> #define FB_BACKLIGHT_MAX 0xFF
> #endif
>
> +struct fb_dmabuf_export {
> + __u32 fd;
> + __u32 flags;
> +};
> +
> #ifdef __KERNEL__
>
> #include <linux/fs.h>
> @@ -418,6 +424,7 @@ struct vm_area_struct;
> struct fb_info;
> struct device;
> struct file;
> +struct dma_buf;
>
> /* Definitions below are used in the parsed monitor specs */
> #define FB_DPMS_ACTIVE_OFF 1
> @@ -701,6 +708,11 @@ struct fb_ops {
> /* called at KDB enter and leave time to prepare the console */
> int (*fb_debug_enter)(struct fb_info *info);
> int (*fb_debug_leave)(struct fb_info *info);
> +
> +#ifdef CONFIG_DMA_SHARED_BUFFER
> + /* Export the frame buffer as a dmabuf object */
> + struct dma_buf *(*fb_dmabuf_export)(struct fb_info *info);
> +#endif
> };
>
> #ifdef CONFIG_FB_TILEBLITTING
> --
> Regards,
>
> Laurent Pinchart
>
>
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-mm-sig
^ permalink raw reply
* [RFC/PATCH] fb: Add dma-buf support
From: Laurent Pinchart @ 2012-06-20 14:09 UTC (permalink / raw)
To: linux-fbdev; +Cc: linaro-mm-sig, linux-media
Add support for the dma-buf exporter role to the frame buffer API. The
importer role isn't meaningful for frame buffer devices, as the frame
buffer device model doesn't allow using externally allocated memory.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Documentation/fb/api.txt | 36 ++++++++++++++++++++++++++++++++++++
drivers/video/fbmem.c | 36 ++++++++++++++++++++++++++++++++++++
include/linux/fb.h | 12 ++++++++++++
3 files changed, 84 insertions(+), 0 deletions(-)
diff --git a/Documentation/fb/api.txt b/Documentation/fb/api.txt
index d4ff7de..f0b2173 100644
--- a/Documentation/fb/api.txt
+++ b/Documentation/fb/api.txt
@@ -304,3 +304,39 @@ extensions.
Upon successful format configuration, drivers update the fb_fix_screeninfo
type, visual and line_length fields depending on the selected format. The type
and visual fields are set to FB_TYPE_FOURCC and FB_VISUAL_FOURCC respectively.
+
+
+5. DMA buffer sharing
+---------------------
+
+The dma-buf kernel framework allows DMA buffers to be shared across devices
+and applications. Sharing buffers across display devices and video capture or
+video decoding devices allow zero-copy operation when displaying video content
+produced by a hardware device such as a camera or a hardware codec. This is
+crucial to achieve optimal system performances during video display.
+
+While dma-buf supports both exporting internally allocated memory as a dma-buf
+object (known as the exporter role) and importing a dma-buf object to be used
+as device memory (known as the importer role), the frame buffer API only
+supports the exporter role, as the frame buffer device model doesn't support
+using externally-allocated memory.
+
+The export a frame buffer as a dma-buf file descriptors, applications call the
+FBIOGET_DMABUF ioctl. The ioctl takes a pointer to a fb_dmabuf_export
+structure.
+
+struct fb_dmabuf_export {
+ __u32 fd;
+ __u32 flags;
+};
+
+The flag field specifies the flags to be used when creating the dma-buf file
+descriptor. The only supported flag is O_CLOEXEC. If the call is successful,
+the driver will set the fd field to a file descriptor corresponding to the
+dma-buf object.
+
+Applications can then pass the file descriptors to another application or
+another device driver. The dma-buf object is automatically reference-counted,
+applications can and should close the file descriptor as soon as they don't
+need it anymore. The underlying dma-buf object will not be freed before the
+last device that uses the dma-buf object releases it.
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 0dff12a..400e449 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -15,6 +15,7 @@
#include <linux/compat.h>
#include <linux/types.h>
+#include <linux/dma-buf.h>
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/major.h>
@@ -1074,6 +1075,23 @@ fb_blank(struct fb_info *info, int blank)
return ret;
}
+#ifdef CONFIG_DMA_SHARED_BUFFER
+int
+fb_get_dmabuf(struct fb_info *info, int flags)
+{
+ struct dma_buf *dmabuf;
+
+ if (info->fbops->fb_dmabuf_export = NULL)
+ return -ENOTTY;
+
+ dmabuf = info->fbops->fb_dmabuf_export(info);
+ if (IS_ERR(dmabuf))
+ return PTR_ERR(dmabuf);
+
+ return dma_buf_fd(dmabuf, flags);
+}
+#endif
+
static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
unsigned long arg)
{
@@ -1084,6 +1102,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
struct fb_cmap cmap_from;
struct fb_cmap_user cmap;
struct fb_event event;
+ struct fb_dmabuf_export dmaexp;
void __user *argp = (void __user *)arg;
long ret = 0;
@@ -1191,6 +1210,23 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
console_unlock();
unlock_fb_info(info);
break;
+#ifdef CONFIG_DMA_SHARED_BUFFER
+ case FBIOGET_DMABUF:
+ if (copy_from_user(&dmaexp, argp, sizeof(dmaexp)))
+ return -EFAULT;
+
+ if (!lock_fb_info(info))
+ return -ENODEV;
+ dmaexp.fd = fb_get_dmabuf(info, dmaexp.flags);
+ unlock_fb_info(info);
+
+ if (dmaexp.fd < 0)
+ return dmaexp.fd;
+
+ ret = copy_to_user(argp, &dmaexp, sizeof(dmaexp))
+ ? -EFAULT : 0;
+ break;
+#endif
default:
if (!lock_fb_info(info))
return -ENODEV;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index ac3f1c6..c9fee75 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -39,6 +39,7 @@
#define FBIOPUT_MODEINFO 0x4617
#define FBIOGET_DISPINFO 0x4618
#define FBIO_WAITFORVSYNC _IOW('F', 0x20, __u32)
+#define FBIOGET_DMABUF _IOR('F', 0x21, struct fb_dmabuf_export)
#define FB_TYPE_PACKED_PIXELS 0 /* Packed Pixels */
#define FB_TYPE_PLANES 1 /* Non interleaved planes */
@@ -403,6 +404,11 @@ struct fb_cursor {
#define FB_BACKLIGHT_MAX 0xFF
#endif
+struct fb_dmabuf_export {
+ __u32 fd;
+ __u32 flags;
+};
+
#ifdef __KERNEL__
#include <linux/fs.h>
@@ -418,6 +424,7 @@ struct vm_area_struct;
struct fb_info;
struct device;
struct file;
+struct dma_buf;
/* Definitions below are used in the parsed monitor specs */
#define FB_DPMS_ACTIVE_OFF 1
@@ -701,6 +708,11 @@ struct fb_ops {
/* called at KDB enter and leave time to prepare the console */
int (*fb_debug_enter)(struct fb_info *info);
int (*fb_debug_leave)(struct fb_info *info);
+
+#ifdef CONFIG_DMA_SHARED_BUFFER
+ /* Export the frame buffer as a dmabuf object */
+ struct dma_buf *(*fb_dmabuf_export)(struct fb_info *info);
+#endif
};
#ifdef CONFIG_FB_TILEBLITTING
--
Regards,
Laurent Pinchart
^ permalink raw reply related
* Re: [PATCH] video: backlight: remove unused header
From: Donghwa Lee @ 2012-06-20 10:32 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: Paul Bolle, linux-fbdev, linux-kernel, Inki Dae, Kyungmin Park
In-Reply-To: <4FE1233E.9040801@gmx.de>
On 6/20/2012 10:11 AM, Florian Tobias Schandinat wrote:
> Hi Paul,
>
> On 06/13/2012 07:47 AM, Paul Bolle wrote:
>> Commit 9befe40f6e018e508b047eb76d189ede9b4ff03d ("video: backlight:
>> support s6e8ax0 panel driver based on MIPI DSI") added s6e8ax0.h, but
>> no file includes it. That's probably a good thing, because it declares
>> an extern void function that is defined static int in s6e8ax0.c.
>> Besides, that function is also wrapped in the module_init() macro, which
>> should do everything needed to make that function available to the code
>> outside of s6e8ax0.c. This header can safely be removed.
>>
>> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
>
> it would have been a good idea to CC the authors of this file to get an
> answer to your questions and allow them to comment on the patch. (done)
> I'd like to hear their feedback before I apply your patch but it looks
> reasonable.
>
>
> Thanks,
>
> Florian Tobias Schandinat
>
>> ---
>> 0) Tested mainly by using various git tools on the (history of the)
>> tree.
>>
>> 1) Shouldn't s6e8ax0_init() and s6e8ax0_exit(), both in s6e8ax0.c, carry
>> the usual __init and __exit attributes?
>>
>> 2) But note that all the module related code in s6e8ax0.c seems moot
>> currently: EXYNOS_LCD_S6E8AX0 is a boolean Kconfig symbol, so the code
>> can only be used builtin. So, as far as I can tell, either that symbol
>> (and the symbols on which it depends) should be made tristate, or the
>> module related code can be removed from s6e8ax0.c.
>>
>> drivers/video/exynos/s6e8ax0.h | 21 ---------------------
>> 1 files changed, 0 insertions(+), 21 deletions(-)
>> delete mode 100644 drivers/video/exynos/s6e8ax0.h
>>
>> diff --git a/drivers/video/exynos/s6e8ax0.h b/drivers/video/exynos/s6e8ax0.h
>> deleted file mode 100644
>> index 1f1b270..0000000
>> --- a/drivers/video/exynos/s6e8ax0.h
>> +++ /dev/null
>> @@ -1,21 +0,0 @@
>> -/* linux/drivers/video/backlight/s6e8ax0.h
>> - *
>> - * MIPI-DSI based s6e8ax0 AMOLED LCD Panel definitions.
>> - *
>> - * Copyright (c) 2011 Samsung Electronics
>> - *
>> - * Inki Dae, <inki.dae@samsung.com>
>> - * Donghwa Lee <dh09.lee@samsung.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.
>> -*/
>> -
>> -#ifndef _S6E8AX0_H
>> -#define _S6E8AX0_H
>> -
>> -extern void s6e8ax0_init(void);
>> -
>> -#endif
>> -
>
>
Hi,
I had looked through that code. I agree that header file will be removed.
There is no reason to maintain it.
Thank you,
Donghwa Lee
^ permalink raw reply
* Re: [PULL for v3.6] SH Mobile LCDC fixes and planes support
From: Laurent Pinchart @ 2012-06-20 8:31 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <3246665.HJN7RE5zfe@avalon>
Hi Florian,
On Wednesday 20 June 2012 01:01:31 Florian Tobias Schandinat wrote:
> On 06/16/2012 02:07 PM, Laurent Pinchart wrote:
> > Hi Florian,
> >
> > Could you please pull the following LCDC patches ?
>
> looks good, except...
>
> > The following changes since commit
b67989515defba7412acff01162e5bb1f0f5923a:
> > video: s3c-fb: fix possible division by zero in s3c_fb_calc_pixclk
> >
> > (2012-06-13 17:34:16 +0000)
> >
> > are available in the git repository at:
> > git://linuxtv.org/pinchartl/fbdev.git planes
> >
> > Guennadi Liakhovetski (1):
> > fbdev: sh_mipi_dsi: fix a section mismatch
> >
> > Laurent Pinchart (4):
> > fbdev: sh_mobile_lcdc: Don't confuse line size with pitch
>
> ...for this patch it would have been better if you based your work on a
> more recent tree of Linus that already contains it instead of asking me
> to pull it again [I now forwarded my fbdev-next to -rc3]. It's not a big
> issue as git handles such situations but pushing patches around that are
> essentially nops should be avoided I think. Well, if you decide to fix
> it I'd be happy, otherwise I'll pull this in a few days nonetheless.
Sorry about that. I've rebased the branch on top of the latest fbdev-next and
pushed it to git://linuxtv.org/pinchartl/fbdev.git planes, that no-op patches
isn't present anymore.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v2] video: exynos_dp: fix build warning due to uninitialized value
From: Jingoo Han @ 2012-06-20 1:25 UTC (permalink / raw)
To: 'Florian Tobias Schandinat'
Cc: linux-fbdev, linux-kernel, 'Olof Johansson',
'Jingoo Han'
This patch fixes build warning due to uninitialized value dereference.
drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_set_link_train':
drivers/video/exynos/exynos_dp_core.c:529:18: warning: 'reg' may be used uninitialized in this function [-Wuninitialized]
drivers/video/exynos/exynos_dp_core.c:395:6: note: 'reg' was declared here
Signed-off-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index a36b2d2..9db7b9f 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -407,6 +407,9 @@ static unsigned int exynos_dp_get_lane_link_training(
case 3:
reg = exynos_dp_get_lane3_link_training(dp);
break;
+ default:
+ WARN_ON(1);
+ return 0;
}
return reg;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] video: backlight: remove unused header
From: Florian Tobias Schandinat @ 2012-06-20 1:11 UTC (permalink / raw)
To: Paul Bolle
Cc: linux-fbdev, linux-kernel, Donghwa Lee, Inki Dae, Kyungmin Park
In-Reply-To: <1339573651.30984.130.camel@x61.thuisdomein>
Hi Paul,
On 06/13/2012 07:47 AM, Paul Bolle wrote:
> Commit 9befe40f6e018e508b047eb76d189ede9b4ff03d ("video: backlight:
> support s6e8ax0 panel driver based on MIPI DSI") added s6e8ax0.h, but
> no file includes it. That's probably a good thing, because it declares
> an extern void function that is defined static int in s6e8ax0.c.
> Besides, that function is also wrapped in the module_init() macro, which
> should do everything needed to make that function available to the code
> outside of s6e8ax0.c. This header can safely be removed.
>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
it would have been a good idea to CC the authors of this file to get an
answer to your questions and allow them to comment on the patch. (done)
I'd like to hear their feedback before I apply your patch but it looks
reasonable.
Thanks,
Florian Tobias Schandinat
> ---
> 0) Tested mainly by using various git tools on the (history of the)
> tree.
>
> 1) Shouldn't s6e8ax0_init() and s6e8ax0_exit(), both in s6e8ax0.c, carry
> the usual __init and __exit attributes?
>
> 2) But note that all the module related code in s6e8ax0.c seems moot
> currently: EXYNOS_LCD_S6E8AX0 is a boolean Kconfig symbol, so the code
> can only be used builtin. So, as far as I can tell, either that symbol
> (and the symbols on which it depends) should be made tristate, or the
> module related code can be removed from s6e8ax0.c.
>
> drivers/video/exynos/s6e8ax0.h | 21 ---------------------
> 1 files changed, 0 insertions(+), 21 deletions(-)
> delete mode 100644 drivers/video/exynos/s6e8ax0.h
>
> diff --git a/drivers/video/exynos/s6e8ax0.h b/drivers/video/exynos/s6e8ax0.h
> deleted file mode 100644
> index 1f1b270..0000000
> --- a/drivers/video/exynos/s6e8ax0.h
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -/* linux/drivers/video/backlight/s6e8ax0.h
> - *
> - * MIPI-DSI based s6e8ax0 AMOLED LCD Panel definitions.
> - *
> - * Copyright (c) 2011 Samsung Electronics
> - *
> - * Inki Dae, <inki.dae@samsung.com>
> - * Donghwa Lee <dh09.lee@samsung.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.
> -*/
> -
> -#ifndef _S6E8AX0_H
> -#define _S6E8AX0_H
> -
> -extern void s6e8ax0_init(void);
> -
> -#endif
> -
^ permalink raw reply
* Re: [PULL for v3.6] SH Mobile LCDC fixes and planes support
From: Florian Tobias Schandinat @ 2012-06-20 1:01 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <3246665.HJN7RE5zfe@avalon>
Hi Laurent,
On 06/16/2012 02:07 PM, Laurent Pinchart wrote:
> Hi Florian,
>
> Could you please pull the following LCDC patches ?
looks good, except...
>
> The following changes since commit b67989515defba7412acff01162e5bb1f0f5923a:
>
> video: s3c-fb: fix possible division by zero in s3c_fb_calc_pixclk
> (2012-06-13 17:34:16 +0000)
>
> are available in the git repository at:
> git://linuxtv.org/pinchartl/fbdev.git planes
>
> Guennadi Liakhovetski (1):
> fbdev: sh_mipi_dsi: fix a section mismatch
>
> Laurent Pinchart (4):
> fbdev: sh_mobile_lcdc: Don't confuse line size with pitch
...for this patch it would have been better if you based your work on a
more recent tree of Linus that already contains it instead of asking me
to pull it again [I now forwarded my fbdev-next to -rc3]. It's not a big
issue as git handles such situations but pushing patches around that are
essentially nops should be avoided I think. Well, if you decide to fix
it I'd be happy, otherwise I'll pull this in a few days nonetheless.
Best regards,
Florian Tobias Schandinat
> fbdev: sh_mobile_lcdc: Constify sh_mobile_lcdc_fix structure
> fbdev: sh_mobile_lcdc: Rename fb operation handlers with a common prefix
> fbdev: sh_mobile_lcdc: Implement overlays support
>
> .../sysfs-devices-platform-sh_mobile_lcdc_fb | 44 +
> drivers/video/sh_mipi_dsi.c | 7 +-
> drivers/video/sh_mobile_lcdcfb.c | 987 +++++++++++++++++--
> drivers/video/sh_mobile_lcdcfb.h | 1 +
> include/video/sh_mobile_lcdc.h | 7 +
> 5 files changed, 944 insertions(+), 102 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-
> sh_mobile_lcdc_fb
>
^ permalink raw reply
* Re: [PATCH] video: exynos_dp: fix build warning due to uninitialized value
From: Jingoo Han @ 2012-06-20 0:54 UTC (permalink / raw)
To: 'Florian Tobias Schandinat'
Cc: linux-fbdev, linux-kernel, 'Olof Johansson',
'Jingoo Han'
In-Reply-To: <4FE11D60.9040109@gmx.de>
On Wed, June 20, 2012 at 9:46 AM, Florian Tobias Schandinat wrote:
> Hi Jingoo,
>
> On 06/19/2012 06:36 AM, Jingoo Han wrote:
> > This patch fixes build warning due to uninitialized value dereference.
> > @@ -407,6 +407,8 @@ static unsigned int exynos_dp_get_lane_link_training(
> > case 3:
> > reg = exynos_dp_get_lane3_link_training(dp);
> > break;
> > + default:
> > + return -EINVAL;
>
> Why do you consider returning -EINVAL here a good idea? As far as I can
> see the caller does not check the return value for an error condition
> and I doubt that the hardware understands what -EINVAL means. If you
> want to fix the warning, make sure that you don't hide actual errors,
> add an WARN or BUG or make sure the error code makes it to userspace.
>
OK, I will fix it and send the v2 patch.
Thank you.
Best regards,
Jingoo Han
^ permalink raw reply
* Re: [PATCH] video: exynos_dp: fix build warning due to uninitialized value
From: Florian Tobias Schandinat @ 2012-06-20 0:46 UTC (permalink / raw)
To: Jingoo Han; +Cc: linux-fbdev, linux-kernel, 'Olof Johansson'
In-Reply-To: <001501cd4de5$d955a290$8c00e7b0$%han@samsung.com>
Hi Jingoo,
On 06/19/2012 06:36 AM, Jingoo Han wrote:
> This patch fixes build warning due to uninitialized value dereference.
>
> drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_set_link_train':
> drivers/video/exynos/exynos_dp_core.c:529:18: warning: 'reg' may be used uninitialized in this function [-Wuninitialized]
> drivers/video/exynos/exynos_dp_core.c:395:6: note: 'reg' was declared here
>
> Signed-off-by: Olof Johansson <olof@lixom.net>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
> drivers/video/exynos/exynos_dp_core.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index a36b2d2..9b2cdff 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -407,6 +407,8 @@ static unsigned int exynos_dp_get_lane_link_training(
> case 3:
> reg = exynos_dp_get_lane3_link_training(dp);
> break;
> + default:
> + return -EINVAL;
Why do you consider returning -EINVAL here a good idea? As far as I can
see the caller does not check the return value for an error condition
and I doubt that the hardware understands what -EINVAL means. If you
want to fix the warning, make sure that you don't hide actual errors,
add an WARN or BUG or make sure the error code makes it to userspace.
> }
>
> return reg;
Best regards,
Florian Tobias Schandinat
^ permalink raw reply
* Re: [PATCH 1/2] video: s3c-fb: clear SHADOWCON register when clearing hardware window registers
From: Florian Tobias Schandinat @ 2012-06-20 0:26 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <001801cd4779$a2c2c220$e8484660$%han@samsung.com>
On 06/11/2012 02:26 AM, Jingoo Han wrote:
> All bits of SHADOWCON register should be cleared when clearing
> hardware window registers; however, some bits of SHADOWCON register
> are not cleared previously.
A bit more detailed description of what this might cause would be good.
I assume you are fixing this because it is more correct but there are no
known bugs related to this? Anyway, please try to give more information
next time.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Applied both patches.
Thanks,
Florian Tobias Schandinat
> ---
> drivers/video/s3c-fb.c | 10 ++++++++--
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index 5f9d8e6..b5c2939 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -1348,8 +1348,14 @@ static void s3c_fb_clear_win(struct s3c_fb *sfb, int win)
> writel(0, regs + VIDOSD_A(win, sfb->variant));
> writel(0, regs + VIDOSD_B(win, sfb->variant));
> writel(0, regs + VIDOSD_C(win, sfb->variant));
> - reg = readl(regs + SHADOWCON);
> - writel(reg & ~SHADOWCON_WINx_PROTECT(win), regs + SHADOWCON);
> +
> + if (sfb->variant.has_shadowcon) {
> + reg = readl(sfb->regs + SHADOWCON);
> + reg &= ~(SHADOWCON_WINx_PROTECT(win) |
> + SHADOWCON_CHx_ENABLE(win) |
> + SHADOWCON_CHx_LOCAL_ENABLE(win));
> + writel(reg, sfb->regs + SHADOWCON);
> + }
> }
>
> static int __devinit s3c_fb_probe(struct platform_device *pdev)
^ permalink raw reply
* [PATCH] video: exynos_dp: fix build warning due to uninitialized value
From: Jingoo Han @ 2012-06-19 6:36 UTC (permalink / raw)
To: 'Florian Tobias Schandinat'
Cc: linux-fbdev, linux-kernel, 'Olof Johansson',
'Jingoo Han'
This patch fixes build warning due to uninitialized value dereference.
drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_set_link_train':
drivers/video/exynos/exynos_dp_core.c:529:18: warning: 'reg' may be used uninitialized in this function [-Wuninitialized]
drivers/video/exynos/exynos_dp_core.c:395:6: note: 'reg' was declared here
Signed-off-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index a36b2d2..9b2cdff 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -407,6 +407,8 @@ static unsigned int exynos_dp_get_lane_link_training(
case 3:
reg = exynos_dp_get_lane3_link_training(dp);
break;
+ default:
+ return -EINVAL;
}
return reg;
--
1.7.1
^ permalink raw reply related
* Re: [RFC 00/10] fblog: framebuffer kernel log driver
From: David Herrmann @ 2012-06-18 19:06 UTC (permalink / raw)
To: linux-serial
Cc: Florian Tobias Schandinat, linux-fbdev, linux-kernel,
Greg Kroah-Hartman, David Herrmann
In-Reply-To: <1339884266-9201-1-git-send-email-dh.herrmann@googlemail.com>
Hi
On Sun, Jun 17, 2012 at 12:04 AM, David Herrmann
<dh.herrmann@googlemail.com> wrote:
> Hi
>
> As some might know I am working on making CONFIG_VT obsolete. But as a developer
> it is often useful to have a kernel-log on the screen during boot to debug many
> common kernel(-config) errors. However, without CONFIG_VT we cannot use the
> VGA/framebbufer consoles either. Therefore, I am working on a small driver
> called "fblog".
>
> This driver simply writes the kernel log to all connected framebuffers. It works
> similar to fbcon but removes all the complexity of the virtual terminals. There
> is a sysfs attribute called "active" that allows to enable/disable fblog so
> user-space can start an xserver or similar.
>
> The main purpose is debugging kernel boot problems. Therefore, it is not
> optimized for speed and I tried keeping it simple. I splitted the patches
> into 10 small chunks to make review easier.
>
> I would be glad if someone could review this and tell me whether this is
> something we could include in mainline or not.
>
>
> There are still some issues but apart from them it works fine on my
> machine (x86):
> - I register the fblog device during module_init and need to call
> module_get(). However, this means it is impossible to call "rmmod fblog" as
> fblog has a reference to itself. Using "rmmod -f fblog" works fine but is a
> bit ugly. Is there a nice way to fix this? Otherwise I would need to call
> device_get() in module_exit() if there is a pending user of the fblog-device
> even though I unregistered it.
> - I redraw all framebuffers while holding the console-lock. This may slow down
> machines with more than 2 framebuffers (like 10 or 20). However, as this is
> supposed to be a debug driver, I think I can ignore this? If someone wants
> to improve the redraw logic to avoid redrawing the whole screen all the
> time, I would be glad to include it in this patchset :)
> - I am really no expert regarding the framebuffer subsystem. So I would
> appreciate it if someone could comment whether I need to handle the events
> in a different way or whether it is ok the way it is now.
One additional issue:
With udlfb.c we have hotplug capable framebuffers. However, fbcon and
fblog currently never close a framebuffer if not explicitely requested
by user-space. Therefore, if a framebuffer device is removed, the
FB_EVENT_FB_UNREGISTER event will never be sent because fbcon/fblog
still have a reference to the framebuffer(-driver). Therefore, the
number of available fbs will grow until there are no more free
indices.
See dlfb_usb_disconnect() in udlfb.c for an example. It does not
invoke unregister_framebuffer() unless the last user closed the FB.
udlfb disables the console on its framebuffer devices to avoid this,
but this doesn't seem to be a good solution.
How about sending an FB_EVENT_FB_DISCONNECT event during unlink_framebuffer()?
This still doesn't force user-space to close /dev/fbX but it at least
will make it possible to fblog/fbcon to close the framebuffer. fbmem.c
can then still be modified to mark the open file as dead so user-space
will also close the device hopefully.
Regards
David
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox