* [PATCH] drivers/video: fsl-diu-fb: merge all allocated data into one block
From: Timur Tabi @ 2011-12-02 23:03 UTC (permalink / raw)
To: linux-fbdev
The Freescale DIU driver allocates multiple blocks of memory, including
multiple DMA buffers. Merge all of these blocks into one data structure.
Specifically:
1) struct fsl_diu_data now contains everything that needs to be allocated,
except for the framebuffers themselves. DMA'able objects are aligned correctly
within the structure.
2) struct diu_addr is no longer needed, because we don't have to manage
multiple blocks of DMA memory.
3) Since there's no diu_addr any more, macro DMA_ADDR is used to calculate
the DMA address of any field in fsl_diu_data.
4) Functions allocate_buf() and free_buf() are no longer needed, because we
now assume that dma_alloc_coherent() will allocate a page-aligned block,
and everything is properly aligned with fsl_diu_data already, so we no longer
need to align any memory blocks ourselves.
5) The "dummy" area descriptor is now defined separately from the other
five ADs, so NUM_AOIS (previously called FSL_AOI_NUM) is now set to five
instead of six. Previously, all six were combined together to avoid a
separate call to allocate_buf() just for the dummy AD.
6) framebuffer_alloc() and framebuffer_release() are no longer used. The
framebuffer is initialized manually.
7) Error handling is simplified since there's only one memory buffer
allocated.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
drivers/video/fsl-diu-fb.c | 326 +++++++++++++++++++-------------------------
1 files changed, 139 insertions(+), 187 deletions(-)
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index a16beeb..4d54188 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -36,8 +36,7 @@
#include <linux/fsl-diu-fb.h>
#include "edid.h"
-#define FSL_AOI_NUM 6 /* 5 AOIs and one dummy AOI */
- /* 1 for plane 0, 2 for plane 1&2 each */
+#define NUM_AOIS 5 /* 1 for plane 0, 2 for planes 1 & 2 each */
/* HW cursor parameters */
#define MAX_CURS 32
@@ -49,12 +48,6 @@
#define INT_PARERR 0x08 /* Display parameters error interrupt */
#define INT_LS_BF_VS 0x10 /* Lines before vsync. interrupt */
-struct diu_addr {
- void *vaddr; /* Virtual address */
- dma_addr_t paddr; /* Physical address */
- __u32 offset;
-};
-
/*
* List of supported video modes
*
@@ -330,23 +323,6 @@ static unsigned int d_cache_line_size;
static DEFINE_SPINLOCK(diu_lock);
-struct fsl_diu_data {
- struct fb_info *fsl_diu_info[FSL_AOI_NUM - 1];
- /*FSL_AOI_NUM has one dummy AOI */
- struct device_attribute dev_attr;
- struct diu_ad *dummy_ad;
- void *dummy_aoi_virt;
- unsigned int irq;
- int fb_enabled;
- enum fsl_diu_monitor_port monitor_port;
- struct diu __iomem *diu_reg;
- spinlock_t reg_lock;
- struct diu_addr ad;
- struct diu_addr gamma;
- struct diu_addr pallete;
- struct diu_addr cursor;
-};
-
enum mfb_index {
PLANE0 = 0, /* Plane 0, only one AOI that fills the screen */
PLANE1_AOI0, /* Plane 1, first AOI */
@@ -370,6 +346,44 @@ struct mfb_info {
u8 *edid_data;
};
+/**
+ * struct fsl_diu_data - per-DIU data structure
+ * @dma_addr: DMA address of this structure
+ * @fsl_diu_info: fb_info objects, one per AOI
+ * @dev_attr: sysfs structure
+ * @irq: IRQ
+ * @fb_enabled: TRUE if the DIU is enabled, FALSE if not
+ * @monitor_port: the monitor port this DIU is connected to
+ * @diu_reg: pointer to the DIU hardware registers
+ * @reg_lock: spinlock for register access
+ * @dummy_aoi: video buffer for the 4x4 32-bit dummy AOI
+ * dummy_ad: DIU Area Descriptor for the dummy AOI
+ * @ad[]: Area Descriptors for each real AOI
+ * @gamma: gamma color table
+ * @cursor: hardware cursor data
+ *
+ * This data structure must be allocated with 32-byte alignment, so that the
+ * internal fields can be aligned properly.
+ */
+struct fsl_diu_data {
+ dma_addr_t dma_addr;
+ struct fb_info fsl_diu_info[NUM_AOIS];
+ struct mfb_info mfb[NUM_AOIS];
+ struct device_attribute dev_attr;
+ unsigned int irq;
+ int fb_enabled;
+ enum fsl_diu_monitor_port monitor_port;
+ struct diu __iomem *diu_reg;
+ spinlock_t reg_lock;
+ u8 dummy_aoi[4 * 4 * 4];
+ struct diu_ad dummy_ad __aligned(8);
+ struct diu_ad ad[NUM_AOIS] __aligned(8);
+ u8 gamma[256 * 3] __aligned(32);
+ u8 cursor[MAX_CURS * MAX_CURS * 2] __aligned(32);
+} __aligned(32);
+
+/* Determine the DMA address of a member of the fsl_diu_data structure */
+#define DMA_ADDR(p, f) ((p)->dma_addr + offsetof(struct fsl_diu_data, f))
static struct mfb_info mfb_template[] = {
{
@@ -504,7 +518,7 @@ static void fsl_diu_enable_panel(struct fb_info *info)
wr_reg_wa(&hw->desc[0], ad->paddr);
break;
case PLANE1_AOI0:
- cmfbi = machine_data->fsl_diu_info[2]->par;
+ cmfbi = &machine_data->mfb[2];
if (hw->desc[1] != ad->paddr) { /* AOI0 closed */
if (cmfbi->count > 0) /* AOI1 open */
ad->next_ad @@ -515,7 +529,7 @@ static void fsl_diu_enable_panel(struct fb_info *info)
}
break;
case PLANE2_AOI0:
- cmfbi = machine_data->fsl_diu_info[4]->par;
+ cmfbi = &machine_data->mfb[4];
if (hw->desc[2] != ad->paddr) { /* AOI0 closed */
if (cmfbi->count > 0) /* AOI1 open */
ad->next_ad @@ -526,17 +540,17 @@ static void fsl_diu_enable_panel(struct fb_info *info)
}
break;
case PLANE1_AOI1:
- pmfbi = machine_data->fsl_diu_info[1]->par;
+ pmfbi = &machine_data->mfb[1];
ad->next_ad = 0;
- if (hw->desc[1] = machine_data->dummy_ad->paddr)
+ if (hw->desc[1] = machine_data->dummy_ad.paddr)
wr_reg_wa(&hw->desc[1], ad->paddr);
else /* AOI0 open */
pmfbi->ad->next_ad = cpu_to_le32(ad->paddr);
break;
case PLANE2_AOI1:
- pmfbi = machine_data->fsl_diu_info[3]->par;
+ pmfbi = &machine_data->mfb[3];
ad->next_ad = 0;
- if (hw->desc[2] = machine_data->dummy_ad->paddr)
+ if (hw->desc[2] = machine_data->dummy_ad.paddr)
wr_reg_wa(&hw->desc[2], ad->paddr);
else /* AOI0 was open */
pmfbi->ad->next_ad = cpu_to_le32(ad->paddr);
@@ -553,47 +567,47 @@ static void fsl_diu_disable_panel(struct fb_info *info)
switch (mfbi->index) {
case PLANE0:
- if (hw->desc[0] != machine_data->dummy_ad->paddr)
- wr_reg_wa(&hw->desc[0], machine_data->dummy_ad->paddr);
+ if (hw->desc[0] != machine_data->dummy_ad.paddr)
+ wr_reg_wa(&hw->desc[0], machine_data->dummy_ad.paddr);
break;
case PLANE1_AOI0:
- cmfbi = machine_data->fsl_diu_info[2]->par;
+ cmfbi = &machine_data->mfb[2];
if (cmfbi->count > 0) /* AOI1 is open */
wr_reg_wa(&hw->desc[1], cmfbi->ad->paddr);
/* move AOI1 to the first */
else /* AOI1 was closed */
- wr_reg_wa(&hw->desc[1], machine_data->dummy_ad->paddr);
+ wr_reg_wa(&hw->desc[1], machine_data->dummy_ad.paddr);
/* close AOI 0 */
break;
case PLANE2_AOI0:
- cmfbi = machine_data->fsl_diu_info[4]->par;
+ cmfbi = &machine_data->mfb[4];
if (cmfbi->count > 0) /* AOI1 is open */
wr_reg_wa(&hw->desc[2], cmfbi->ad->paddr);
/* move AOI1 to the first */
else /* AOI1 was closed */
- wr_reg_wa(&hw->desc[2], machine_data->dummy_ad->paddr);
+ wr_reg_wa(&hw->desc[2], machine_data->dummy_ad.paddr);
/* close AOI 0 */
break;
case PLANE1_AOI1:
- pmfbi = machine_data->fsl_diu_info[1]->par;
+ pmfbi = &machine_data->mfb[1];
if (hw->desc[1] != ad->paddr) {
/* AOI1 is not the first in the chain */
if (pmfbi->count > 0)
/* AOI0 is open, must be the first */
pmfbi->ad->next_ad = 0;
} else /* AOI1 is the first in the chain */
- wr_reg_wa(&hw->desc[1], machine_data->dummy_ad->paddr);
+ wr_reg_wa(&hw->desc[1], machine_data->dummy_ad.paddr);
/* close AOI 1 */
break;
case PLANE2_AOI1:
- pmfbi = machine_data->fsl_diu_info[3]->par;
+ pmfbi = &machine_data->mfb[3];
if (hw->desc[2] != ad->paddr) {
/* AOI1 is not the first in the chain */
if (pmfbi->count > 0)
/* AOI0 is open, must be the first */
pmfbi->ad->next_ad = 0;
} else /* AOI1 is the first in the chain */
- wr_reg_wa(&hw->desc[2], machine_data->dummy_ad->paddr);
+ wr_reg_wa(&hw->desc[2], machine_data->dummy_ad.paddr);
/* close AOI 1 */
break;
}
@@ -633,8 +647,8 @@ static void adjust_aoi_size_position(struct fb_var_screeninfo *var,
int lower_aoi_is_open, upper_aoi_is_open;
__u32 base_plane_width, base_plane_height, upper_aoi_height;
- base_plane_width = machine_data->fsl_diu_info[0]->var.xres;
- base_plane_height = machine_data->fsl_diu_info[0]->var.yres;
+ base_plane_width = machine_data->fsl_diu_info[0].var.xres;
+ base_plane_height = machine_data->fsl_diu_info[0].var.yres;
if (mfbi->x_aoi_d < 0)
mfbi->x_aoi_d = 0;
@@ -649,7 +663,7 @@ static void adjust_aoi_size_position(struct fb_var_screeninfo *var,
break;
case PLANE1_AOI0:
case PLANE2_AOI0:
- lower_aoi_mfbi = machine_data->fsl_diu_info[index+1]->par;
+ lower_aoi_mfbi = machine_data->fsl_diu_info[index+1].par;
lower_aoi_is_open = lower_aoi_mfbi->count > 0 ? 1 : 0;
if (var->xres > base_plane_width)
var->xres = base_plane_width;
@@ -667,9 +681,9 @@ static void adjust_aoi_size_position(struct fb_var_screeninfo *var,
break;
case PLANE1_AOI1:
case PLANE2_AOI1:
- upper_aoi_mfbi = machine_data->fsl_diu_info[index-1]->par;
+ upper_aoi_mfbi = machine_data->fsl_diu_info[index-1].par;
upper_aoi_height - machine_data->fsl_diu_info[index-1]->var.yres;
+ machine_data->fsl_diu_info[index-1].var.yres;
upper_aoi_bottom = upper_aoi_mfbi->y_aoi_d + upper_aoi_height;
upper_aoi_is_open = upper_aoi_mfbi->count > 0 ? 1 : 0;
if (var->xres > base_plane_width)
@@ -812,15 +826,15 @@ static void update_lcdc(struct fb_info *info)
struct fsl_diu_data *machine_data = mfbi->parent;
struct diu __iomem *hw;
int i, j;
- char __iomem *cursor_base, *gamma_table_base;
+ u8 *gamma_table_base;
u32 temp;
hw = machine_data->diu_reg;
diu_ops.set_monitor_port(machine_data->monitor_port);
- gamma_table_base = machine_data->gamma.vaddr;
- cursor_base = machine_data->cursor.vaddr;
+ gamma_table_base = machine_data->gamma;
+
/* Prep for DIU init - gamma table, cursor table */
for (i = 0; i <= 2; i++)
@@ -828,14 +842,14 @@ static void update_lcdc(struct fb_info *info)
*gamma_table_base++ = j;
diu_ops.set_gamma_table(machine_data->monitor_port,
- machine_data->gamma.vaddr);
+ machine_data->gamma);
disable_lcdc(info);
/* Program DIU registers */
- out_be32(&hw->gamma, machine_data->gamma.paddr);
- out_be32(&hw->cursor, machine_data->cursor.paddr);
+ out_be32(&hw->gamma, DMA_ADDR(machine_data, gamma));
+ out_be32(&hw->cursor, DMA_ADDR(machine_data, cursor));
out_be32(&hw->bgnd, 0x007F7F7F); /* BGND */
out_be32(&hw->bgnd_wb, 0); /* BGND_WB */
@@ -1423,37 +1437,6 @@ static int fsl_diu_resume(struct platform_device *ofdev)
#define fsl_diu_resume NULL
#endif /* CONFIG_PM */
-/* Align to 64-bit(8-byte), 32-byte, etc. */
-static int allocate_buf(struct device *dev, struct diu_addr *buf, u32 size,
- u32 bytes_align)
-{
- u32 offset;
- dma_addr_t mask;
-
- buf->vaddr - dma_alloc_coherent(dev, size + bytes_align, &buf->paddr,
- GFP_DMA | __GFP_ZERO);
- if (!buf->vaddr)
- return -ENOMEM;
-
- mask = bytes_align - 1;
- offset = buf->paddr & mask;
- if (offset) {
- buf->offset = bytes_align - offset;
- buf->paddr = buf->paddr + offset;
- } else
- buf->offset = 0;
-
- return 0;
-}
-
-static void free_buf(struct device *dev, struct diu_addr *buf, u32 size,
- u32 bytes_align)
-{
- dma_free_coherent(dev, size + bytes_align, buf->vaddr,
- buf->paddr - buf->offset);
-}
-
static ssize_t store_monitor(struct device *device,
struct device_attribute *attr, const char *buf, size_t count)
{
@@ -1468,11 +1451,10 @@ static ssize_t store_monitor(struct device *device,
/* All AOIs need adjust pixel format
* fsl_diu_set_par only change the pixsel format here
* unlikely to fail. */
- fsl_diu_set_par(machine_data->fsl_diu_info[0]);
- fsl_diu_set_par(machine_data->fsl_diu_info[1]);
- fsl_diu_set_par(machine_data->fsl_diu_info[2]);
- fsl_diu_set_par(machine_data->fsl_diu_info[3]);
- fsl_diu_set_par(machine_data->fsl_diu_info[4]);
+ unsigned int i;
+
+ for (i=0; i < NUM_AOIS; i++)
+ fsl_diu_set_par(&machine_data->fsl_diu_info[i]);
}
return count;
}
@@ -1499,28 +1481,52 @@ static int __devinit fsl_diu_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct mfb_info *mfbi;
- phys_addr_t dummy_ad_addr = 0;
- int ret, i, error = 0;
struct fsl_diu_data *machine_data;
int diu_mode;
+ dma_addr_t dma_addr; /* DMA addr of machine_data struct */
+ unsigned int i;
+ int ret;
- machine_data = kzalloc(sizeof(struct fsl_diu_data), GFP_KERNEL);
+ machine_data = dma_alloc_coherent(&pdev->dev,
+ sizeof(struct fsl_diu_data), &dma_addr, GFP_DMA | __GFP_ZERO);
if (!machine_data)
return -ENOMEM;
+ machine_data->dma_addr = dma_addr;
+
+ /*
+ * dma_alloc_coherent() uses a page allocator, so the address is
+ * always page-aligned. We need the memory to be 32-byte aligned,
+ * so that's good. However, if one day the allocator changes, we
+ * need to catch that. It's not worth the effort to handle unaligned
+ * alloctions now because it's highly unlikely to ever be a problem.
+ */
+ if ((unsigned long)machine_data & 31) {
+ dev_err(&pdev->dev, "misaligned allocation");
+ ret = -ENOMEM;
+ goto error;
+ }
spin_lock_init(&machine_data->reg_lock);
- for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++) {
- machine_data->fsl_diu_info[i] - framebuffer_alloc(sizeof(struct mfb_info), &pdev->dev);
- if (!machine_data->fsl_diu_info[i]) {
- dev_err(&pdev->dev, "cannot allocate memory\n");
- ret = -ENOMEM;
- goto error2;
- }
- mfbi = machine_data->fsl_diu_info[i]->par;
+ for (i = 0; i < NUM_AOIS; i++) {
+ struct fb_info *info = &machine_data->fsl_diu_info[i];
+
+ info->device = &pdev->dev;
+ info->par = &machine_data->mfb[i];
+
+ /*
+ * We store the physical address of the AD in the reserved
+ * 'paddr' field of the AD itself.
+ */
+ machine_data->ad[i].paddr = DMA_ADDR(machine_data, ad[i]);
+
+ info->fix.smem_start = 0;
+
+ /* Initialize the AOI data structure */
+ mfbi = info->par;
memcpy(mfbi, &mfb_template[i], sizeof(struct mfb_info));
mfbi->parent = machine_data;
+ mfbi->ad = &machine_data->ad[i];
if (mfbi->index = PLANE0) {
const u8 *prop;
@@ -1538,7 +1544,7 @@ static int __devinit fsl_diu_probe(struct platform_device *pdev)
if (!machine_data->diu_reg) {
dev_err(&pdev->dev, "cannot map DIU registers\n");
ret = -EFAULT;
- goto error2;
+ goto error;
}
diu_mode = in_be32(&machine_data->diu_reg->diu_mode);
@@ -1555,41 +1561,16 @@ static int __devinit fsl_diu_probe(struct platform_device *pdev)
}
machine_data->monitor_port = monitor_port;
- /* Area descriptor memory pool aligns to 64-bit boundary */
- if (allocate_buf(&pdev->dev, &machine_data->ad,
- sizeof(struct diu_ad) * FSL_AOI_NUM, 8))
- return -ENOMEM;
-
- /* Get memory for Gamma Table - 32-byte aligned memory */
- if (allocate_buf(&pdev->dev, &machine_data->gamma, 768, 32)) {
- ret = -ENOMEM;
- goto error;
- }
-
- /* For performance, cursor bitmap buffer aligns to 32-byte boundary */
- if (allocate_buf(&pdev->dev, &machine_data->cursor,
- MAX_CURS * MAX_CURS * 2, 32)) {
- ret = -ENOMEM;
- goto error;
- }
-
- i = ARRAY_SIZE(machine_data->fsl_diu_info);
- machine_data->dummy_ad = (struct diu_ad *)((u32)machine_data->ad.vaddr +
- machine_data->ad.offset) + i;
- machine_data->dummy_ad->paddr = machine_data->ad.paddr +
- i * sizeof(struct diu_ad);
- machine_data->dummy_aoi_virt = fsl_diu_alloc(64, &dummy_ad_addr);
- if (!machine_data->dummy_aoi_virt) {
- ret = -ENOMEM;
- goto error;
- }
- machine_data->dummy_ad->addr = cpu_to_le32(dummy_ad_addr);
- machine_data->dummy_ad->pix_fmt = 0x88882317;
- machine_data->dummy_ad->src_size_g_alpha = cpu_to_le32((4 << 12) | 4);
- machine_data->dummy_ad->aoi_size = cpu_to_le32((4 << 16) | 2);
- machine_data->dummy_ad->offset_xyi = 0;
- machine_data->dummy_ad->offset_xyd = 0;
- machine_data->dummy_ad->next_ad = 0;
+ /* Initialize the dummy Area Descriptor */
+ machine_data->dummy_ad.addr + cpu_to_le32(DMA_ADDR(machine_data, dummy_aoi));
+ machine_data->dummy_ad.pix_fmt = 0x88882317;
+ machine_data->dummy_ad.src_size_g_alpha = cpu_to_le32((4 << 12) | 4);
+ machine_data->dummy_ad.aoi_size = cpu_to_le32((4 << 16) | 2);
+ machine_data->dummy_ad.offset_xyi = 0;
+ machine_data->dummy_ad.offset_xyd = 0;
+ machine_data->dummy_ad.next_ad = 0;
+ machine_data->dummy_ad.paddr = DMA_ADDR(machine_data, dummy_ad);
/*
* Let DIU display splash screen if it was pre-initialized
@@ -1597,19 +1578,13 @@ static int __devinit fsl_diu_probe(struct platform_device *pdev)
*/
if (diu_mode = MFB_MODE0)
out_be32(&machine_data->diu_reg->desc[0],
- machine_data->dummy_ad->paddr);
-
- out_be32(&machine_data->diu_reg->desc[1], machine_data->dummy_ad->paddr);
- out_be32(&machine_data->diu_reg->desc[2], machine_data->dummy_ad->paddr);
-
- for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++) {
- machine_data->fsl_diu_info[i]->fix.smem_start = 0;
- mfbi = machine_data->fsl_diu_info[i]->par;
- mfbi->ad = (struct diu_ad *)((u32)machine_data->ad.vaddr
- + machine_data->ad.offset) + i;
- mfbi->ad->paddr - machine_data->ad.paddr + i * sizeof(struct diu_ad);
- ret = install_fb(machine_data->fsl_diu_info[i]);
+ machine_data->dummy_ad.paddr);
+
+ out_be32(&machine_data->diu_reg->desc[1], machine_data->dummy_ad.paddr);
+ out_be32(&machine_data->diu_reg->desc[2], machine_data->dummy_ad.paddr);
+
+ for (i = 0; i < NUM_AOIS; i++) {
+ ret = install_fb(&machine_data->fsl_diu_info[i]);
if (ret) {
dev_err(&pdev->dev, "could not register fb %d\n", i);
goto error;
@@ -1626,9 +1601,8 @@ static int __devinit fsl_diu_probe(struct platform_device *pdev)
machine_data->dev_attr.attr.mode = S_IRUGO|S_IWUSR;
machine_data->dev_attr.show = show_monitor;
machine_data->dev_attr.store = store_monitor;
- error = device_create_file(machine_data->fsl_diu_info[0]->dev,
- &machine_data->dev_attr);
- if (error) {
+ ret = device_create_file(&pdev->dev, &machine_data->dev_attr);
+ if (ret) {
dev_err(&pdev->dev, "could not create sysfs file %s\n",
machine_data->dev_attr.attr.name);
}
@@ -1637,26 +1611,13 @@ static int __devinit fsl_diu_probe(struct platform_device *pdev)
return 0;
error:
- for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++)
- uninstall_fb(machine_data->fsl_diu_info[i]);
-
- if (machine_data->ad.vaddr)
- free_buf(&pdev->dev, &machine_data->ad,
- sizeof(struct diu_ad) * FSL_AOI_NUM, 8);
- if (machine_data->gamma.vaddr)
- free_buf(&pdev->dev, &machine_data->gamma, 768, 32);
- if (machine_data->cursor.vaddr)
- free_buf(&pdev->dev, &machine_data->cursor,
- MAX_CURS * MAX_CURS * 2, 32);
- if (machine_data->dummy_aoi_virt)
- fsl_diu_free(machine_data->dummy_aoi_virt, 64);
+ for (i = 0; i < NUM_AOIS; i++)
+ uninstall_fb(&machine_data->fsl_diu_info[i]);
+
iounmap(machine_data->diu_reg);
-error2:
- for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++)
- if (machine_data->fsl_diu_info[i])
- framebuffer_release(machine_data->fsl_diu_info[i]);
- kfree(machine_data);
+ dma_free_coherent(&pdev->dev, sizeof(struct fsl_diu_data),
+ machine_data, machine_data->dma_addr);
return ret;
}
@@ -1667,25 +1628,16 @@ static int fsl_diu_remove(struct platform_device *pdev)
int i;
machine_data = dev_get_drvdata(&pdev->dev);
- disable_lcdc(machine_data->fsl_diu_info[0]);
+ disable_lcdc(&machine_data->fsl_diu_info[0]);
free_irq_local(machine_data);
- for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++)
- uninstall_fb(machine_data->fsl_diu_info[i]);
- if (machine_data->ad.vaddr)
- free_buf(&pdev->dev, &machine_data->ad,
- sizeof(struct diu_ad) * FSL_AOI_NUM, 8);
- if (machine_data->gamma.vaddr)
- free_buf(&pdev->dev, &machine_data->gamma, 768, 32);
- if (machine_data->cursor.vaddr)
- free_buf(&pdev->dev, &machine_data->cursor,
- MAX_CURS * MAX_CURS * 2, 32);
- if (machine_data->dummy_aoi_virt)
- fsl_diu_free(machine_data->dummy_aoi_virt, 64);
+
+ for (i = 0; i < NUM_AOIS; i++)
+ uninstall_fb(&machine_data->fsl_diu_info[i]);
+
iounmap(machine_data->diu_reg);
- for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++)
- if (machine_data->fsl_diu_info[i])
- framebuffer_release(machine_data->fsl_diu_info[i]);
- kfree(machine_data);
+
+ dma_free_coherent(&pdev->dev, sizeof(struct fsl_diu_data),
+ machine_data, machine_data->dma_addr);
return 0;
}
--
1.7.3.4
^ permalink raw reply related
* Re: QVGA on a hardware which supports only >= VGA
From: James Simmons @ 2011-12-02 17:57 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <CAH9NwWdUZHfhqpNwcfxK_EnsMLeEazB50rQ=m65U3+tjhpwGJQ@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2928 bytes --]
> 2011/11/18 Christian Gmeiner <christian.gmeiner@gmail.com>:
> > Hi all,
> >
> > I am currently in the process to get some local patches into mainline
> > and/or fix my
> > problems the right way. At the moment I am stuck with this problem:
> >
> > I am on a device (x86), which is Geode lx800 based and there is a
> > panel connected to
> > it. The problem is that two different screen resolutions are used. QVGA and VGA.
> > Should not sound like a problem, but the lx800 can only do VGA. So get
> > it working,
> > I need to pass the correct resolution via kernel cmd to the lxfb
> > driver and I am using
> > this patch (based on 2.6.36.4 kernel):
> >
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -1050,6 +1050,7 @@ static long do_fb_ioctl(struct fb_info *info,
> > unsigned int cmd,
> > ret = copy_to_user(argp, &var, sizeof(var)) ? -EFAULT : 0;
> > break;
> > case FBIOPUT_VSCREENINFO:
> > +#if 0
> > if (copy_from_user(&var, argp, sizeof(var)))
> > return -EFAULT;
> > if (!lock_fb_info(info))
> > @@ -1062,6 +1063,8 @@ static long do_fb_ioctl(struct fb_info *info,
> > unsigned int cmd,
> > unlock_fb_info(info);
> > if (!ret && copy_to_user(argp, &var, sizeof(var)))
> > ret = -EFAULT;
> > +#endif
> > + ret = 0;
> > break;
> > case FBIOGET_FSCREENINFO:
> > if (!lock_fb_info(info))
> > @@ -1086,6 +1089,7 @@ static long do_fb_ioctl(struct fb_info *info,
> > unsigned int cmd,
> > ret = fb_cmap_to_user(&cmap_from, &cmap);
> > break;
> > case FBIOPAN_DISPLAY:
> > +#if 0
> > if (copy_from_user(&var, argp, sizeof(var)))
> > return -EFAULT;
> > if (!lock_fb_info(info))
> > @@ -1096,6 +1100,8 @@ static long do_fb_ioctl(struct fb_info *info,
> > unsigned int cmd,
> > unlock_fb_info(info);
> > if (ret == 0 && copy_to_user(argp, &var, sizeof(var)))
> > return -EFAULT;
> > +#endif
> > + ret = 0;
> > break;
> > case FBIO_CURSOR:
> > ret = -EINVAL;
> >
> > I need to mention that I am using a custom BIOS and Linux based
> > bootloader (kexec) in the
> > boot process. At the moment I am looking into coreboot, but I wanted
> > to know how to
> > solve this "problem" the correct way.
> >
> > To sum it up: I want to use a QVGA panel with a hardware that only
> > supports >= VGA.
That breaks the userland interface. What exactly is breaking? It looks
like a userland app is causing the headaches.
^ permalink raw reply
* Re: backlight/ld9040.c: regulator control in the lcd driver
From: Mark Brown @ 2011-12-02 10:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdYrNcuch-H9ttHE3saceCM-WsB-Wx0-tih8LHxu6ftaYA@mail.gmail.com>
On Fri, Dec 02, 2011 at 11:36:13AM +0100, Linus Walleij wrote:
> I don't think that's true. You *must* have some voltage on VMMC
> to power the card, the optional part is regulating that voltage to
> different levels as requested by the card internal machinery when
> talking to it. All MMC/SD cards can run on a fixed voltage, something
> like 3.8V I think.
> In line with our previous discussions I think this should actually be
> defined as a fixed voltage regulator in case it cannot be controlled,
> because there sure as hell is a voltage there on all systems.
I have to say that I was very suspicious of this claim at the time but
there was so much pain associated with the MMC stuff that it just got
left to slide. Similarly with the use of regulator_get_exclusive() to
vary the voltage, I'd *really* expect that the code would be able to
cope with shared supplies.
^ permalink raw reply
* Re: backlight/ld9040.c: regulator control in the lcd driver
From: Linus Walleij @ 2011-12-02 10:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20111202103139.GA8245@opensource.wolfsonmicro.com>
On Fri, Dec 2, 2011 at 11:31 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Dec 02, 2011 at 05:57:35PM +0900, Kyungmin Park wrote:
>
>> As mentioned at commit message, the lcd regulator is optional part and
>> refer the mmc codes
>
>> host->vmmc = regulator_get(mmc_dev(mmc), "vmmc");
>> if (IS_ERR(host->vmmc)) {
>> pr_info("%s: no vmmc regulator found\n", mmc_hostname(mmc));
>> host->vmmc = NULL;
>> } else {
>> regulator_enable(host->vmmc);
>> }
>
>> Previous time, these codes are located at board file, but more boards
>> are used, it has same codes for all boards. so move it to drivers.
>
> In the case of MMC the MMC guys told us that this supply was entirely
> optional for MMC operation, it wasn't an essential supply for the MMC
> device to run it just enabled more features. For supplies like that
> it's OK for the regulator to fail, the driver should just not do
> whatever things are enabled by having that supply.
I don't think that's true. You *must* have some voltage on VMMC
to power the card, the optional part is regulating that voltage to
different levels as requested by the card internal machinery when
talking to it. All MMC/SD cards can run on a fixed voltage, something
like 3.8V I think.
In line with our previous discussions I think this should actually be
defined as a fixed voltage regulator in case it cannot be controlled,
because there sure as hell is a voltage there on all systems.
Yours,
Linus Walleij
^ permalink raw reply
* Re: backlight/ld9040.c: regulator control in the lcd driver
From: Mark Brown @ 2011-12-02 10:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAH9JG2XdDn26ziceug6-Dc8g3+z3AUuzL34-oHRy10qL2RjF2w@mail.gmail.com>
On Fri, Dec 02, 2011 at 05:57:35PM +0900, Kyungmin Park wrote:
> As mentioned at commit message, the lcd regulator is optional part and
> refer the mmc codes
> host->vmmc = regulator_get(mmc_dev(mmc), "vmmc");
> if (IS_ERR(host->vmmc)) {
> pr_info("%s: no vmmc regulator found\n", mmc_hostname(mmc));
> host->vmmc = NULL;
> } else {
> regulator_enable(host->vmmc);
> }
> Previous time, these codes are located at board file, but more boards
> are used, it has same codes for all boards. so move it to drivers.
In the case of MMC the MMC guys told us that this supply was entirely
optional for MMC operation, it wasn't an essential supply for the MMC
device to run it just enabled more features. For supplies like that
it's OK for the regulator to fail, the driver should just not do
whatever things are enabled by having that supply.
^ permalink raw reply
* Re: backlight/ld9040.c: regulator control in the lcd driver
From: Kyungmin Park @ 2011-12-02 10:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdan-zKk6TN15MkbEuNk5C949SL-jniK98vReFTzfvGC4w@mail.gmail.com>
On 12/2/11, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Dec 2, 2011 at 9:57 AM, Kyungmin Park <kyungmin.park@samsung.com>
> wrote:
>>>[leedonghwa]
>> [Me]
>>>> + lcd->reg_vdd3 = regulator_get(lcd->dev, "vdd");
>>>> + if (IS_ERR(lcd->reg_vdd3)) {
>>>> + dev_info(lcd->dev, "no %s regulator found\n",
>>>> "vdd");
>>>> + lcd->reg_vdd3 = NULL;
>>>> + }
>>>> +
>>>> + lcd->reg_vci = regulator_get(lcd->dev, "vci");
>>>> + if (IS_ERR(lcd->reg_vci)) {
>>>> + dev_info(lcd->dev, "no %s regulator found\n",
>>>> "vci");
>>>> + lcd->reg_vci = NULL;
>>>> + }
>>>
>>> As explained in earlier discussion with Mark regarding the SMSC911x
>>> driver regulator, treat these as errors and do not fail
>>> "gracefully" like this.
>>>
>>> Reference:
>>> http://marc.info/?l=linux-netdev&m\x131914562120725&w=2
>>
>> As mentioned at commit message, the lcd regulator is optional part and
>> refer the mmc codes
>>
>> host->vmmc = regulator_get(mmc_dev(mmc), "vmmc");
>> if (IS_ERR(host->vmmc)) {
>> pr_info("%s: no vmmc regulator found\n",
>> mmc_hostname(mmc));
>> host->vmmc = NULL;
>> } else {
>> regulator_enable(host->vmmc);
>> }
>>
>> Previous time, these codes are located at board file, but more boards
>> are used, it has same codes for all boards. so move it to drivers.
>
> I know. This was brought up in the aforementioned discussion,
> but the above is also wrong, simply. See:
> http://marc.info/?l=linux-netdev&m\x131914562120667&w=2
> http://marc.info/?l=linux-netdev&m\x131914562120690&w=2
> http://marc.info/?l=linux-netdev&m\x131914562120725&w=2
> http://marc.info/?l=linux-netdev&m\x131963332527416&w=2
>
>> In our case, it has the regulator but some boards don't.
>>
>> Umm then how to handle the regulator gracefully?
>
> Mark suggest using a fixed-voltage regulator for boards
> where the power is always on. The voltage level itself
> is optional. See:
> http://marc.info/?l=linux-netdev&m\x131963332527416&w=2
Make sense, okay send the updated patch
>
> Other approaches is to use dummy regulators, or not
> call regulator_has_full_constraints(), which means the
> regulator core will provide dummy regulators anyways.
> See:
> http://marc.info/?l=linux-netdev&m\x131973043527112&w=2
> http://marc.info/?l=linux-netdev&m\x131975178703166&w=2
>
> Whole thread of discussion:
> http://marc.info/?l=linux-netdev&w=2&r=1&s=smsc911x&q=b
>
> Yours,
> Linus Walleij
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH] video: s3c-fb: set missing bitmask of enabled hardware window
From: Jingoo Han @ 2011-12-02 10:07 UTC (permalink / raw)
To: linux-fbdev
This patch set missing bitmask of enabled hardware window which
should be checked whenever the hardware window is enabled.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/s3c-fb.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 12eaee0..fc3df1d 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -574,6 +574,7 @@ static int s3c_fb_set_par(struct fb_info *info)
}
data = WINCONx_ENWIN;
+ sfb->enabled |= (1 << win->index);
/* note, since we have to round up the bits-per-pixel, we end up
* relying on the bitfield information for r/g/b/a to work out
--
1.7.1
^ permalink raw reply related
* Re: backlight/ld9040.c: regulator control in the lcd driver
From: Linus Walleij @ 2011-12-02 10:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAH9JG2XdDn26ziceug6-Dc8g3+z3AUuzL34-oHRy10qL2RjF2w@mail.gmail.com>
On Fri, Dec 2, 2011 at 9:57 AM, Kyungmin Park <kyungmin.park@samsung.com> wrote:
>>[leedonghwa]
> [Me]
>>> + lcd->reg_vdd3 = regulator_get(lcd->dev, "vdd");
>>> + if (IS_ERR(lcd->reg_vdd3)) {
>>> + dev_info(lcd->dev, "no %s regulator found\n", "vdd");
>>> + lcd->reg_vdd3 = NULL;
>>> + }
>>> +
>>> + lcd->reg_vci = regulator_get(lcd->dev, "vci");
>>> + if (IS_ERR(lcd->reg_vci)) {
>>> + dev_info(lcd->dev, "no %s regulator found\n", "vci");
>>> + lcd->reg_vci = NULL;
>>> + }
>>
>> As explained in earlier discussion with Mark regarding the SMSC911x
>> driver regulator, treat these as errors and do not fail
>> "gracefully" like this.
>>
>> Reference:
>> http://marc.info/?l=linux-netdev&m\x131914562120725&w=2
>
> As mentioned at commit message, the lcd regulator is optional part and
> refer the mmc codes
>
> host->vmmc = regulator_get(mmc_dev(mmc), "vmmc");
> if (IS_ERR(host->vmmc)) {
> pr_info("%s: no vmmc regulator found\n", mmc_hostname(mmc));
> host->vmmc = NULL;
> } else {
> regulator_enable(host->vmmc);
> }
>
> Previous time, these codes are located at board file, but more boards
> are used, it has same codes for all boards. so move it to drivers.
I know. This was brought up in the aforementioned discussion,
but the above is also wrong, simply. See:
http://marc.info/?l=linux-netdev&m\x131914562120667&w=2
http://marc.info/?l=linux-netdev&m\x131914562120690&w=2
http://marc.info/?l=linux-netdev&m\x131914562120725&w=2
http://marc.info/?l=linux-netdev&m\x131963332527416&w=2
> In our case, it has the regulator but some boards don't.
>
> Umm then how to handle the regulator gracefully?
Mark suggest using a fixed-voltage regulator for boards
where the power is always on. The voltage level itself
is optional. See:
http://marc.info/?l=linux-netdev&m\x131963332527416&w=2
Other approaches is to use dummy regulators, or not
call regulator_has_full_constraints(), which means the
regulator core will provide dummy regulators anyways.
See:
http://marc.info/?l=linux-netdev&m\x131973043527112&w=2
http://marc.info/?l=linux-netdev&m\x131975178703166&w=2
Whole thread of discussion:
http://marc.info/?l=linux-netdev&w=2&r=1&s=smsc911x&q=b
Yours,
Linus Walleij
^ permalink raw reply
* Re: backlight/ld9040.c: regulator control in the lcd driver
From: Kyungmin Park @ 2011-12-02 8:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdbRdtsw-3FXtevuJL7G=tcE+1s11iRw=zou1G25hufBSA@mail.gmail.com>
On 12/2/11, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Dec 2, 2011 at 8:45 AM, leedonghwa <dh09.lee@samsung.com> wrote:
>
> [From your mail headers]
>> X-Mailer: Microsoft Office Outlook 12.0
>
> No please. That mailer does not work, all your whitespace is screwed up.
> Consult: Documentation/email-clients.txt
>
>> This patch supports regulator power control in the driver.
>
> Always CC the regulator maintainers on patches like this please.
>
>> + lcd->reg_vdd3 = regulator_get(lcd->dev, "vdd");
>> + if (IS_ERR(lcd->reg_vdd3)) {
>> + dev_info(lcd->dev, "no %s regulator found\n", "vdd");
>> + lcd->reg_vdd3 = NULL;
>> + }
>> +
>> + lcd->reg_vci = regulator_get(lcd->dev, "vci");
>> + if (IS_ERR(lcd->reg_vci)) {
>> + dev_info(lcd->dev, "no %s regulator found\n", "vci");
>> + lcd->reg_vci = NULL;
>> + }
>
> As explained in earlier discussion with Mark regarding the SMSC911x
> driver regulator, treat these as errors and do not fail
> "gracefully" like this.
>
> Reference:
> http://marc.info/?l=linux-netdev&m\x131914562120725&w=2
As mentioned at commit message, the lcd regulator is optional part and
refer the mmc codes
host->vmmc = regulator_get(mmc_dev(mmc), "vmmc");
if (IS_ERR(host->vmmc)) {
pr_info("%s: no vmmc regulator found\n", mmc_hostname(mmc));
host->vmmc = NULL;
} else {
regulator_enable(host->vmmc);
}
Previous time, these codes are located at board file, but more boards
are used, it has same codes for all boards. so move it to drivers.
In our case, it has the regulator but some boards don't.
Umm then how to handle the regulator gracefully?
Thank you,
Kyungmin Park
>
> Yours,
> Linus Walleij
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* Re: backlight/ld9040.c: regulator control in the lcd driver
From: Linus Walleij @ 2011-12-02 8:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <000c01ccb0c6$697c2aa0$3c747fe0$%lee@samsung.com>
On Fri, Dec 2, 2011 at 8:45 AM, leedonghwa <dh09.lee@samsung.com> wrote:
[From your mail headers]
> X-Mailer: Microsoft Office Outlook 12.0
No please. That mailer does not work, all your whitespace is screwed up.
Consult: Documentation/email-clients.txt
> This patch supports regulator power control in the driver.
Always CC the regulator maintainers on patches like this please.
> + lcd->reg_vdd3 = regulator_get(lcd->dev, "vdd");
> + if (IS_ERR(lcd->reg_vdd3)) {
> + dev_info(lcd->dev, "no %s regulator found\n", "vdd");
> + lcd->reg_vdd3 = NULL;
> + }
> +
> + lcd->reg_vci = regulator_get(lcd->dev, "vci");
> + if (IS_ERR(lcd->reg_vci)) {
> + dev_info(lcd->dev, "no %s regulator found\n", "vci");
> + lcd->reg_vci = NULL;
> + }
As explained in earlier discussion with Mark regarding the SMSC911x
driver regulator, treat these as errors and do not fail
"gracefully" like this.
Reference:
http://marc.info/?l=linux-netdev&m\x131914562120725&w=2
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v4 2/3] v4l: Add V4L2_PIX_FMT_NV24 and V4L2_PIX_FMT_NV42 formats
From: Laurent Pinchart @ 2011-11-29 18:11 UTC (permalink / raw)
To: Sylwester Nawrocki; +Cc: linux-fbdev, linux-media
In-Reply-To: <4ED5140D.4080200@samsung.com>
Hi Sylwester,
On Tuesday 29 November 2011 18:19:09 Sylwester Nawrocki wrote:
> On 11/29/2011 11:26 AM, Laurent Pinchart wrote:
> > NV24 and NV42 are planar YCbCr 4:4:4 and YCrCb 4:4:4 formats with a
> > luma plane followed by an interleaved chroma plane.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
>
> ...
>
> > + </example>
> > + </refsect1>
> > + </refentry>
> > +
> >
> > + <!--
> > +Local Variables:
> > +mode: sgml
> > +sgml-parent-document: "pixfmt.sgml"
> > +indent-tabs-mode: nil
> > +End:
> > + -->
>
> I think this comment chunk is redundant, it's just for emacs configuration.
> Every time I open the file containing litter like this I get a
> not-so-useful message, asking if I want to load the variables defined
> there or not.
I agree. I'll remove it. Thanks for pointing it out.
> I'm considering a patch cleaning up the Docbook from this emacs stuff, as I
> also made a mistake copying this comment when adding NV12M, NV12MT formats.
>
> Looks like most of people are doing that, e.g. see
>
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg38637.html
>
> ;)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v4 2/3] v4l: Add V4L2_PIX_FMT_NV24 and V4L2_PIX_FMT_NV42
From: Sylwester Nawrocki @ 2011-11-29 17:19 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-fbdev, linux-media
In-Reply-To: <1322562419-9934-3-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Laurent,
On 11/29/2011 11:26 AM, Laurent Pinchart wrote:
> NV24 and NV42 are planar YCbCr 4:4:4 and YCrCb 4:4:4 formats with a
> luma plane followed by an interleaved chroma plane.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
...
> + </example>
> + </refsect1>
> + </refentry>
> +
> + <!--
> +Local Variables:
> +mode: sgml
> +sgml-parent-document: "pixfmt.sgml"
> +indent-tabs-mode: nil
> +End:
> + -->
I think this comment chunk is redundant, it's just for emacs configuration.
Every time I open the file containing litter like this I get a not-so-useful
message, asking if I want to load the variables defined there or not.
I'm considering a patch cleaning up the Docbook from this emacs stuff, as I
also made a mistake copying this comment when adding NV12M, NV12MT formats.
Looks like most of people are doing that, e.g. see
http://www.mail-archive.com/linux-media@vger.kernel.org/msg38637.html
;)
--
Regards,
Sylwester
^ permalink raw reply
* Re: drm pixel formats update
From: Tomi Valkeinen @ 2011-11-29 16:13 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: dri-devel, ville.syrjala, linux-fbdev, linux-media
In-Reply-To: <201111291310.36589.laurent.pinchart@ideasonboard.com>
[-- Attachment #1: Type: text/plain, Size: 1264 bytes --]
On Tue, 2011-11-29 at 13:10 +0100, Laurent Pinchart wrote:
> To make it perfectly clear, I want to emphasize that I'm not trying to replace
> DRM, FBDEV and V4L2 with a new shared subsystem. What I would like to see in
> the (near future) is collaboration and sharing of core features that make
> sense to share. I believe we should address this from a "pain point" point of
> view. The one that lead me to writing this e-mail is that developing a driver
> that implements both the DRM and FBDEV APIs for video output currently
> requires various similar structures, and code to translate between all of
> them. That code can't be shared between multiple drivers, is error-prone, and
> painful to maintin.
We've been in the same situation with OMAP display driver for years. The
same low level display driver is used by FB, V4L2 and now DRM drivers,
so I didn't have much choice but to implement omapdss versions for
things like video timings and color formats.
I've been planning to write code for this almost as long as omapdss has
had this problem, but there's always been
yet-another-important-thing-to-do. So good that somebody finally brought
this up =).
I'm happy to test any code related to this with omapdss.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: drm pixel formats update
From: Hans Verkuil @ 2011-11-29 13:30 UTC (permalink / raw)
To: Laurent Pinchart
Cc: dri-devel, ville.syrjala, Tomi Valkeinen, linux-fbdev,
linux-media, Jesse Barker
In-Reply-To: <201111291310.36589.laurent.pinchart@ideasonboard.com>
On Tuesday 29 November 2011 13:10:35 Laurent Pinchart wrote:
> Hi Ville,
>
> Sorry for the late reply.
>
> (Cross-posting to the linux-fbdev and linux-media mailing lists, as the
> topics I'm about to discuss are of interest to everybody)
>
> On Wednesday 16 November 2011 19:42:23 ville.syrjala@linux.intel.com wrote:
> > I decided to go all out with the pixel format definitions. Added pretty
> > much all of the possible RGB/BGR variations. Just left out ones with
> > 16bit components and floats. Also added a whole bunch of YUV formats,
> > and 8 bit pseudocolor for good measure.
> >
> > I'm sure some of the fourccs now clash with the ones used by v4l2,
> > but that's life.
>
> Disclaimer: I realize this is a somehow controversial topic, and I'm not
> trying to start a flame war :-)
>
> What about defining a common set of fourccs for both subsystem (and using
> them in FBDEV, which currently uses the V4L2 fourccs) ?
>
> There's more and more overlap between DRM, FBDEV and V4L2, which results in
> confusion for the user and mess in the kernel. I believe cleaning this up
> will become more and more important with time, and also probably more and
> more difficult if we don't act now.
>
> There's no way we will all quickly agree on a big picture unified
> architecture (I don't even know what it should look like), so I'm thinking
> about starting small and see where it will lead us. Sharing fourccs would
> be such a first step, and would make it easier for drivers to implement
> several of the DRM, FBDEV and V4L2 APIs.
>
> Another step I'd like to take would be working on sharing the video mode
> definitions between subsystems. DRM has struct drm_mode_modeinfo and FBDEV
> has struct fb_videomode. The former can't be modified as it's used in
> userspace APIs, but I believe we should create a common in-kernel
> structure to represent modes. This would also make it easier to share the
> EDID parser between DRM and FBDEV.
>
> One issue here is names. I understand that using names from another
> subsystem in a driver that has nothing to do with it (like using
> V4L2_PIX_FMT_* in DRM, or drm_mode_modeinfo in FBDEV) can be frustrating
> for many developers, so I'd like to propose an alternative. We have a
> media-related kernel API that is cross-subsystem, that's the media
> controller. It uses the prefix media_. We could use more specific versions
> of that preview (such as media_video_ and media_display_) as a neutral
> ground.
I agree.
> Another issue is control. It's quite natural to be suspicious about
> subsystems we're not familiar with, and giving up control on things we
> consider as highly important to other subsystems feels dangerous and
> wrong. Once again, media_* could be an answer to this problem. Instead of
> trying to push other subsystems to use our APIs (which are, obviously, the
> best possible ever, as they're the ones we work on :-)) with the promise
> that we will extend them to fullfill their needs if necessary, we could
> design new shared structures and core functions together with a media_
> prefix, and make sure they fullfill all the needs from the start. What I
> like about this approach is that each of the 3 video-related subsystems
> would then benefit from the experience of the other 2.
>
> To make it perfectly clear, I want to emphasize that I'm not trying to
> replace DRM, FBDEV and V4L2 with a new shared subsystem. What I would like
> to see in the (near future) is collaboration and sharing of core features
> that make sense to share. I believe we should address this from a "pain
> point" point of view. The one that lead me to writing this e-mail is that
> developing a driver that implements both the DRM and FBDEV APIs for video
> output currently requires various similar structures, and code to
> translate between all of them. That code can't be shared between multiple
> drivers, is error-prone, and painful to maintin.
>
> I can (and actually would like to) submit an RFC, but I would first like to
> hear your opinion on the topic.
I have added Jesse Barker to the CC list. I think that to achieve closer
cooperation between the various subsystems we need to organize a few days
of talks between the main developers to 1) understand each others subsystem,
2) to discover what functionality would be good to share, and 3) figure out
some sort of roadmap on how to do this.
Linaro might be a suitable organization that can assist with at least the
organizational part of setting up such a meeting. They did a good job in the
past (in Budapest and Cambourne) with the buffer sharing discussions.
One week of discussions/brainstorming between a small(ish) group of developers
can be very, very productive in my experience.
Regards,
Hans
>
> > If anyone has problems with the way the formats are defined, please
> > speak up now! Since only Jesse has bothered to comment on my rantings
> > I can only assume people are happy with my approach to things.
> >
> > These patches should apply on top of Jesse's v3+v5 set... I think.
> > I sort of lost track of things when the patches started having
> > different version numbers. At least we're not yet into two digits
> > numbers ;)
^ permalink raw reply
* Re: drm pixel formats update
From: Laurent Pinchart @ 2011-11-29 12:10 UTC (permalink / raw)
To: dri-devel; +Cc: ville.syrjala, Tomi Valkeinen, linux-fbdev, linux-media
In-Reply-To: <1321468945-28224-1-git-send-email-ville.syrjala@linux.intel.com>
Hi Ville,
Sorry for the late reply.
(Cross-posting to the linux-fbdev and linux-media mailing lists, as the topics
I'm about to discuss are of interest to everybody)
On Wednesday 16 November 2011 19:42:23 ville.syrjala@linux.intel.com wrote:
> I decided to go all out with the pixel format definitions. Added pretty
> much all of the possible RGB/BGR variations. Just left out ones with
> 16bit components and floats. Also added a whole bunch of YUV formats,
> and 8 bit pseudocolor for good measure.
>
> I'm sure some of the fourccs now clash with the ones used by v4l2,
> but that's life.
Disclaimer: I realize this is a somehow controversial topic, and I'm not
trying to start a flame war :-)
What about defining a common set of fourccs for both subsystem (and using them
in FBDEV, which currently uses the V4L2 fourccs) ?
There's more and more overlap between DRM, FBDEV and V4L2, which results in
confusion for the user and mess in the kernel. I believe cleaning this up will
become more and more important with time, and also probably more and more
difficult if we don't act now.
There's no way we will all quickly agree on a big picture unified architecture
(I don't even know what it should look like), so I'm thinking about starting
small and see where it will lead us. Sharing fourccs would be such a first
step, and would make it easier for drivers to implement several of the DRM,
FBDEV and V4L2 APIs.
Another step I'd like to take would be working on sharing the video mode
definitions between subsystems. DRM has struct drm_mode_modeinfo and FBDEV has
struct fb_videomode. The former can't be modified as it's used in userspace
APIs, but I believe we should create a common in-kernel structure to represent
modes. This would also make it easier to share the EDID parser between DRM and
FBDEV.
One issue here is names. I understand that using names from another subsystem
in a driver that has nothing to do with it (like using V4L2_PIX_FMT_* in DRM,
or drm_mode_modeinfo in FBDEV) can be frustrating for many developers, so I'd
like to propose an alternative. We have a media-related kernel API that is
cross-subsystem, that's the media controller. It uses the prefix media_. We
could use more specific versions of that preview (such as media_video_ and
media_display_) as a neutral ground.
Another issue is control. It's quite natural to be suspicious about subsystems
we're not familiar with, and giving up control on things we consider as highly
important to other subsystems feels dangerous and wrong. Once again, media_*
could be an answer to this problem. Instead of trying to push other subsystems
to use our APIs (which are, obviously, the best possible ever, as they're the
ones we work on :-)) with the promise that we will extend them to fullfill
their needs if necessary, we could design new shared structures and core
functions together with a media_ prefix, and make sure they fullfill all the
needs from the start. What I like about this approach is that each of the 3
video-related subsystems would then benefit from the experience of the other
2.
To make it perfectly clear, I want to emphasize that I'm not trying to replace
DRM, FBDEV and V4L2 with a new shared subsystem. What I would like to see in
the (near future) is collaboration and sharing of core features that make
sense to share. I believe we should address this from a "pain point" point of
view. The one that lead me to writing this e-mail is that developing a driver
that implements both the DRM and FBDEV APIs for video output currently
requires various similar structures, and code to translate between all of
them. That code can't be shared between multiple drivers, is error-prone, and
painful to maintin.
I can (and actually would like to) submit an RFC, but I would first like to
hear your opinion on the topic.
> If anyone has problems with the way the formats are defined, please
> speak up now! Since only Jesse has bothered to comment on my rantings
> I can only assume people are happy with my approach to things.
>
> These patches should apply on top of Jesse's v3+v5 set... I think.
> I sort of lost track of things when the patches started having
> different version numbers. At least we're not yet into two digits
> numbers ;)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v4 3/3] fbdev: sh_mobile_lcdc: Support FOURCC-based format API
From: Laurent Pinchart @ 2011-11-29 10:26 UTC (permalink / raw)
To: linux-fbdev; +Cc: linux-media
In-Reply-To: <1322562419-9934-1-git-send-email-laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/arm/mach-shmobile/board-ag5evm.c | 2 +-
arch/arm/mach-shmobile/board-ap4evb.c | 4 +-
arch/arm/mach-shmobile/board-mackerel.c | 4 +-
arch/sh/boards/mach-ap325rxa/setup.c | 2 +-
arch/sh/boards/mach-ecovec24/setup.c | 2 +-
arch/sh/boards/mach-kfr2r09/setup.c | 2 +-
arch/sh/boards/mach-migor/setup.c | 4 +-
arch/sh/boards/mach-se/7724/setup.c | 2 +-
drivers/video/sh_mobile_lcdcfb.c | 360 ++++++++++++++++++++----------
include/video/sh_mobile_lcdc.h | 4 +-
10 files changed, 253 insertions(+), 133 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index 7e3dd73..80319f2 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -271,7 +271,7 @@ static struct sh_mobile_lcdc_info lcdc0_info = {
.flags = LCDC_FLAGS_DWPOL,
.lcd_size_cfg.width = 44,
.lcd_size_cfg.height = 79,
- .bpp = 16,
+ .fourcc = V4L2_PIX_FMT_RGB565,
.lcd_cfg = lcdc0_modes,
.num_cfg = ARRAY_SIZE(lcdc0_modes),
.board_cfg = {
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index 904b608..42b4dda 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -491,7 +491,7 @@ static struct sh_mobile_lcdc_info lcdc_info = {
.meram_dev = &meram_info,
.ch[0] = {
.chan = LCDC_CHAN_MAINLCD,
- .bpp = 16,
+ .fourcc = V4L2_PIX_FMT_RGB565,
.lcd_cfg = ap4evb_lcdc_modes,
.num_cfg = ARRAY_SIZE(ap4evb_lcdc_modes),
.meram_cfg = &lcd_meram_cfg,
@@ -813,7 +813,7 @@ static struct sh_mobile_lcdc_info sh_mobile_lcdc1_info = {
.meram_dev = &meram_info,
.ch[0] = {
.chan = LCDC_CHAN_MAINLCD,
- .bpp = 16,
+ .fourcc = V4L2_PIX_FMT_RGB565,
.interface_type = RGB24,
.clock_divider = 1,
.flags = LCDC_FLAGS_DWPOL,
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 9c5e598..7db6a17 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -388,7 +388,7 @@ static struct sh_mobile_lcdc_info lcdc_info = {
.clock_source = LCDC_CLK_BUS,
.ch[0] = {
.chan = LCDC_CHAN_MAINLCD,
- .bpp = 16,
+ .fourcc = V4L2_PIX_FMT_RGB565,
.lcd_cfg = mackerel_lcdc_modes,
.num_cfg = ARRAY_SIZE(mackerel_lcdc_modes),
.interface_type = RGB24,
@@ -451,7 +451,7 @@ static struct sh_mobile_lcdc_info hdmi_lcdc_info = {
.clock_source = LCDC_CLK_EXTERNAL,
.ch[0] = {
.chan = LCDC_CHAN_MAINLCD,
- .bpp = 16,
+ .fourcc = V4L2_PIX_FMT_RGB565,
.interface_type = RGB24,
.clock_divider = 1,
.flags = LCDC_FLAGS_DWPOL,
diff --git a/arch/sh/boards/mach-ap325rxa/setup.c b/arch/sh/boards/mach-ap325rxa/setup.c
index 7030f4c..7977911 100644
--- a/arch/sh/boards/mach-ap325rxa/setup.c
+++ b/arch/sh/boards/mach-ap325rxa/setup.c
@@ -207,7 +207,7 @@ static struct sh_mobile_lcdc_info lcdc_info = {
.clock_source = LCDC_CLK_EXTERNAL,
.ch[0] = {
.chan = LCDC_CHAN_MAINLCD,
- .bpp = 16,
+ .fourcc = V4L2_PIX_FMT_RGB565,
.interface_type = RGB18,
.clock_divider = 1,
.lcd_cfg = ap325rxa_lcdc_modes,
diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index 92ddce4..1d4a706 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -330,7 +330,7 @@ static struct sh_mobile_lcdc_info lcdc_info = {
.ch[0] = {
.interface_type = RGB18,
.chan = LCDC_CHAN_MAINLCD,
- .bpp = 16,
+ .fourcc = V4L2_PIX_FMT_RGB565,
.lcd_size_cfg = { /* 7.0 inch */
.width = 152,
.height = 91,
diff --git a/arch/sh/boards/mach-kfr2r09/setup.c b/arch/sh/boards/mach-kfr2r09/setup.c
index f65271a..208c9b0 100644
--- a/arch/sh/boards/mach-kfr2r09/setup.c
+++ b/arch/sh/boards/mach-kfr2r09/setup.c
@@ -146,7 +146,7 @@ static struct sh_mobile_lcdc_info kfr2r09_sh_lcdc_info = {
.clock_source = LCDC_CLK_BUS,
.ch[0] = {
.chan = LCDC_CHAN_MAINLCD,
- .bpp = 16,
+ .fourcc = V4L2_PIX_FMT_RGB565,
.interface_type = SYS18,
.clock_divider = 6,
.flags = LCDC_FLAGS_DWPOL,
diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
index e4c8119..ccf61fb 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -244,7 +244,7 @@ static struct sh_mobile_lcdc_info sh_mobile_lcdc_info = {
.clock_source = LCDC_CLK_BUS,
.ch[0] = {
.chan = LCDC_CHAN_MAINLCD,
- .bpp = 16,
+ .fourcc = V4L2_PIX_FMT_RGB565,
.interface_type = RGB16,
.clock_divider = 2,
.lcd_cfg = migor_lcd_modes,
@@ -258,7 +258,7 @@ static struct sh_mobile_lcdc_info sh_mobile_lcdc_info = {
.clock_source = LCDC_CLK_PERIPHERAL,
.ch[0] = {
.chan = LCDC_CHAN_MAINLCD,
- .bpp = 16,
+ .fourcc = V4L2_PIX_FMT_RGB565,
.interface_type = SYS16A,
.clock_divider = 10,
.lcd_cfg = migor_lcd_modes,
diff --git a/arch/sh/boards/mach-se/7724/setup.c b/arch/sh/boards/mach-se/7724/setup.c
index b747c0a..3aab70c 100644
--- a/arch/sh/boards/mach-se/7724/setup.c
+++ b/arch/sh/boards/mach-se/7724/setup.c
@@ -179,7 +179,7 @@ static struct sh_mobile_lcdc_info lcdc_info = {
.clock_source = LCDC_CLK_EXTERNAL,
.ch[0] = {
.chan = LCDC_CHAN_MAINLCD,
- .bpp = 16,
+ .fourcc = V4L2_PIX_FMT_RGB565,
.clock_divider = 1,
.lcd_size_cfg = { /* 7.0 inch */
.width = 152,
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 1f49ab4..2046666 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -17,6 +17,7 @@
#include <linux/platform_device.h>
#include <linux/dma-mapping.h>
#include <linux/interrupt.h>
+#include <linux/videodev2.h>
#include <linux/vmalloc.h>
#include <linux/ioctl.h>
#include <linux/slab.h>
@@ -102,7 +103,7 @@ struct sh_mobile_lcdc_priv {
struct sh_mobile_lcdc_chan ch[2];
struct notifier_block notifier;
int started;
- int forced_bpp; /* 2 channel LCDC must share bpp setting */
+ int forced_fourcc; /* 2 channel LCDC must share fourcc setting */
struct sh_mobile_meram_info *meram_dev;
};
@@ -215,6 +216,47 @@ struct sh_mobile_lcdc_sys_bus_ops sh_mobile_lcdc_sys_bus_ops = {
lcdc_sys_read_data,
};
+static int sh_mobile_format_fourcc(const struct fb_var_screeninfo *var)
+{
+ if (var->grayscale > 1)
+ return var->grayscale;
+
+ switch (var->bits_per_pixel) {
+ case 16:
+ return V4L2_PIX_FMT_RGB565;
+ case 24:
+ return V4L2_PIX_FMT_BGR24;
+ case 32:
+ return V4L2_PIX_FMT_BGR32;
+ default:
+ return 0;
+ }
+}
+
+static int sh_mobile_format_is_fourcc(const struct fb_var_screeninfo *var)
+{
+ return var->grayscale > 1;
+}
+
+static bool sh_mobile_format_is_yuv(const struct fb_var_screeninfo *var)
+{
+ if (var->grayscale <= 1)
+ return false;
+
+ switch (var->grayscale) {
+ case V4L2_PIX_FMT_NV12:
+ case V4L2_PIX_FMT_NV21:
+ case V4L2_PIX_FMT_NV16:
+ case V4L2_PIX_FMT_NV61:
+ case V4L2_PIX_FMT_NV24:
+ case V4L2_PIX_FMT_NV42:
+ return true;
+
+ default:
+ return false;
+ }
+}
+
static void sh_mobile_lcdc_clk_on(struct sh_mobile_lcdc_priv *priv)
{
if (atomic_inc_and_test(&priv->hw_usecnt)) {
@@ -435,7 +477,6 @@ static void __sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
{
struct sh_mobile_lcdc_chan *ch;
unsigned long tmp;
- int bpp = 0;
int k, m;
/* Enable LCDC channels. Read data from external memory, avoid using the
@@ -454,9 +495,6 @@ static void __sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
if (!ch->enabled)
continue;
- if (!bpp)
- bpp = ch->info->var.bits_per_pixel;
-
/* Power supply */
lcdc_write_chan(ch, LDPMR, 0);
@@ -487,31 +525,37 @@ static void __sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
sh_mobile_lcdc_geometry(ch);
- if (ch->info->var.nonstd) {
- tmp = (ch->info->var.nonstd << 16);
- switch (ch->info->var.bits_per_pixel) {
- case 12:
- tmp |= LDDFR_YF_420;
- break;
- case 16:
- tmp |= LDDFR_YF_422;
- break;
- case 24:
- default:
- tmp |= LDDFR_YF_444;
- break;
- }
- } else {
- switch (ch->info->var.bits_per_pixel) {
- case 16:
- tmp = LDDFR_PKF_RGB16;
- break;
- case 24:
- tmp = LDDFR_PKF_RGB24;
+ switch (sh_mobile_format_fourcc(&ch->info->var)) {
+ case V4L2_PIX_FMT_RGB565:
+ tmp = LDDFR_PKF_RGB16;
+ break;
+ case V4L2_PIX_FMT_BGR24:
+ tmp = LDDFR_PKF_RGB24;
+ break;
+ case V4L2_PIX_FMT_BGR32:
+ tmp = LDDFR_PKF_ARGB32;
+ break;
+ case V4L2_PIX_FMT_NV12:
+ case V4L2_PIX_FMT_NV21:
+ tmp = LDDFR_CC | LDDFR_YF_420;
+ break;
+ case V4L2_PIX_FMT_NV16:
+ case V4L2_PIX_FMT_NV61:
+ tmp = LDDFR_CC | LDDFR_YF_422;
+ break;
+ case V4L2_PIX_FMT_NV24:
+ case V4L2_PIX_FMT_NV42:
+ tmp = LDDFR_CC | LDDFR_YF_444;
+ break;
+ }
+
+ if (sh_mobile_format_is_yuv(&ch->info->var)) {
+ switch (ch->info->var.colorspace) {
+ case V4L2_COLORSPACE_REC709:
+ tmp |= LDDFR_CF1;
break;
- case 32:
- default:
- tmp = LDDFR_PKF_ARGB32;
+ case V4L2_COLORSPACE_JPEG:
+ tmp |= LDDFR_CF0;
break;
}
}
@@ -519,7 +563,7 @@ static void __sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
lcdc_write_chan(ch, LDDFR, tmp);
lcdc_write_chan(ch, LDMLSR, ch->pitch);
lcdc_write_chan(ch, LDSA1R, ch->base_addr_y);
- if (ch->info->var.nonstd)
+ if (sh_mobile_format_is_yuv(&ch->info->var))
lcdc_write_chan(ch, LDSA2R, ch->base_addr_c);
/* When using deferred I/O mode, configure the LCDC for one-shot
@@ -536,21 +580,23 @@ static void __sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
}
/* Word and long word swap. */
- if (priv->ch[0].info->var.nonstd)
+ switch (sh_mobile_format_fourcc(&priv->ch[0].info->var)) {
+ case V4L2_PIX_FMT_RGB565:
+ case V4L2_PIX_FMT_NV21:
+ case V4L2_PIX_FMT_NV61:
+ case V4L2_PIX_FMT_NV42:
+ tmp = LDDDSR_LS | LDDDSR_WS;
+ break;
+ case V4L2_PIX_FMT_BGR24:
+ case V4L2_PIX_FMT_NV12:
+ case V4L2_PIX_FMT_NV16:
+ case V4L2_PIX_FMT_NV24:
tmp = LDDDSR_LS | LDDDSR_WS | LDDDSR_BS;
- else {
- switch (bpp) {
- case 16:
- tmp = LDDDSR_LS | LDDDSR_WS;
- break;
- case 24:
- tmp = LDDDSR_LS | LDDDSR_WS | LDDDSR_BS;
- break;
- case 32:
- default:
- tmp = LDDDSR_LS;
- break;
- }
+ break;
+ case V4L2_PIX_FMT_BGR32:
+ default:
+ tmp = LDDDSR_LS;
+ break;
}
lcdc_write(priv, _LDDDSR, tmp);
@@ -622,12 +668,24 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
ch->meram_enabled = 0;
}
- if (!ch->info->var.nonstd)
- pixelformat = SH_MOBILE_MERAM_PF_RGB;
- else if (ch->info->var.bits_per_pixel = 24)
- pixelformat = SH_MOBILE_MERAM_PF_NV24;
- else
+ switch (sh_mobile_format_fourcc(&ch->info->var)) {
+ case V4L2_PIX_FMT_NV12:
+ case V4L2_PIX_FMT_NV21:
+ case V4L2_PIX_FMT_NV16:
+ case V4L2_PIX_FMT_NV61:
pixelformat = SH_MOBILE_MERAM_PF_NV;
+ break;
+ case V4L2_PIX_FMT_NV24:
+ case V4L2_PIX_FMT_NV42:
+ pixelformat = SH_MOBILE_MERAM_PF_NV24;
+ break;
+ case V4L2_PIX_FMT_RGB565:
+ case V4L2_PIX_FMT_BGR24:
+ case V4L2_PIX_FMT_BGR32:
+ default:
+ pixelformat = SH_MOBILE_MERAM_PF_RGB;
+ break;
+ }
ret = mdev->ops->meram_register(mdev, cfg, ch->pitch,
ch->info->var.yres, pixelformat,
@@ -845,6 +903,7 @@ static struct fb_fix_screeninfo sh_mobile_lcdc_fix = {
.xpanstep = 0,
.ypanstep = 1,
.ywrapstep = 0,
+ .capabilities = FB_CAP_FOURCC,
};
static void sh_mobile_lcdc_fillrect(struct fb_info *info,
@@ -877,8 +936,9 @@ static int sh_mobile_fb_pan_display(struct fb_var_screeninfo *var,
unsigned long new_pan_offset;
unsigned long base_addr_y, base_addr_c;
unsigned long c_offset;
+ bool yuv = sh_mobile_format_is_yuv(&info->var);
- if (!info->var.nonstd)
+ if (!yuv)
new_pan_offset = var->yoffset * info->fix.line_length
+ var->xoffset * (info->var.bits_per_pixel / 8);
else
@@ -892,7 +952,7 @@ static int sh_mobile_fb_pan_display(struct fb_var_screeninfo *var,
/* Set the source address for the next refresh */
base_addr_y = ch->dma_handle + new_pan_offset;
- if (info->var.nonstd) {
+ if (yuv) {
/* Set y offset */
c_offset = var->yoffset * info->fix.line_length
* (info->var.bits_per_pixel - 8) / 8;
@@ -900,7 +960,7 @@ static int sh_mobile_fb_pan_display(struct fb_var_screeninfo *var,
+ info->var.xres * info->var.yres_virtual
+ c_offset;
/* Set x offset */
- if (info->var.bits_per_pixel = 24)
+ if (sh_mobile_format_fourcc(&info->var) = V4L2_PIX_FMT_NV24)
base_addr_c += 2 * var->xoffset;
else
base_addr_c += var->xoffset;
@@ -924,7 +984,7 @@ static int sh_mobile_fb_pan_display(struct fb_var_screeninfo *var,
ch->base_addr_c = base_addr_c;
lcdc_write_chan_mirror(ch, LDSA1R, base_addr_y);
- if (info->var.nonstd)
+ if (yuv)
lcdc_write_chan_mirror(ch, LDSA2R, base_addr_c);
if (lcdc_chan_is_sublcd(ch))
@@ -1100,51 +1160,84 @@ static int sh_mobile_check_var(struct fb_var_screeninfo *var, struct fb_info *in
if (var->yres_virtual < var->yres)
var->yres_virtual = var->yres;
- if (var->bits_per_pixel <= 16) { /* RGB 565 */
- var->bits_per_pixel = 16;
- var->red.offset = 11;
- var->red.length = 5;
- var->green.offset = 5;
- var->green.length = 6;
- var->blue.offset = 0;
- var->blue.length = 5;
- var->transp.offset = 0;
- var->transp.length = 0;
- } else if (var->bits_per_pixel <= 24) { /* RGB 888 */
- var->bits_per_pixel = 24;
- var->red.offset = 16;
- var->red.length = 8;
- var->green.offset = 8;
- var->green.length = 8;
- var->blue.offset = 0;
- var->blue.length = 8;
- var->transp.offset = 0;
- var->transp.length = 0;
- } else if (var->bits_per_pixel <= 32) { /* RGBA 888 */
- var->bits_per_pixel = 32;
- var->red.offset = 16;
- var->red.length = 8;
- var->green.offset = 8;
- var->green.length = 8;
- var->blue.offset = 0;
- var->blue.length = 8;
- var->transp.offset = 24;
- var->transp.length = 8;
- } else
- return -EINVAL;
+ if (sh_mobile_format_is_fourcc(var)) {
+ switch (var->grayscale) {
+ case V4L2_PIX_FMT_NV12:
+ case V4L2_PIX_FMT_NV21:
+ var->bits_per_pixel = 12;
+ break;
+ case V4L2_PIX_FMT_RGB565:
+ case V4L2_PIX_FMT_NV16:
+ case V4L2_PIX_FMT_NV61:
+ var->bits_per_pixel = 16;
+ break;
+ case V4L2_PIX_FMT_BGR24:
+ case V4L2_PIX_FMT_NV24:
+ case V4L2_PIX_FMT_NV42:
+ var->bits_per_pixel = 24;
+ break;
+ case V4L2_PIX_FMT_BGR32:
+ var->bits_per_pixel = 32;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Default to RGB and JPEG color-spaces for RGB and YUV formats
+ * respectively.
+ */
+ if (!sh_mobile_format_is_yuv(var))
+ var->colorspace = V4L2_COLORSPACE_SRGB;
+ else if (var->colorspace != V4L2_COLORSPACE_REC709)
+ var->colorspace = V4L2_COLORSPACE_JPEG;
+ } else {
+ if (var->bits_per_pixel <= 16) { /* RGB 565 */
+ var->bits_per_pixel = 16;
+ var->red.offset = 11;
+ var->red.length = 5;
+ var->green.offset = 5;
+ var->green.length = 6;
+ var->blue.offset = 0;
+ var->blue.length = 5;
+ var->transp.offset = 0;
+ var->transp.length = 0;
+ } else if (var->bits_per_pixel <= 24) { /* RGB 888 */
+ var->bits_per_pixel = 24;
+ var->red.offset = 16;
+ var->red.length = 8;
+ var->green.offset = 8;
+ var->green.length = 8;
+ var->blue.offset = 0;
+ var->blue.length = 8;
+ var->transp.offset = 0;
+ var->transp.length = 0;
+ } else if (var->bits_per_pixel <= 32) { /* RGBA 888 */
+ var->bits_per_pixel = 32;
+ var->red.offset = 16;
+ var->red.length = 8;
+ var->green.offset = 8;
+ var->green.length = 8;
+ var->blue.offset = 0;
+ var->blue.length = 8;
+ var->transp.offset = 24;
+ var->transp.length = 8;
+ } else
+ return -EINVAL;
- var->red.msb_right = 0;
- var->green.msb_right = 0;
- var->blue.msb_right = 0;
- var->transp.msb_right = 0;
+ var->red.msb_right = 0;
+ var->green.msb_right = 0;
+ var->blue.msb_right = 0;
+ var->transp.msb_right = 0;
+ }
/* Make sure we don't exceed our allocated memory. */
if (var->xres_virtual * var->yres_virtual * var->bits_per_pixel / 8 >
info->fix.smem_len)
return -EINVAL;
- /* only accept the forced_bpp for dual channel configurations */
- if (p->forced_bpp && p->forced_bpp != var->bits_per_pixel)
+ /* only accept the forced_fourcc for dual channel configurations */
+ if (p->forced_fourcc &&
+ p->forced_fourcc != sh_mobile_format_fourcc(var))
return -EINVAL;
return 0;
@@ -1158,7 +1251,7 @@ static int sh_mobile_set_par(struct fb_info *info)
sh_mobile_lcdc_stop(ch->lcdc);
- if (info->var.nonstd)
+ if (sh_mobile_format_is_yuv(&info->var))
info->fix.line_length = info->var.xres;
else
info->fix.line_length = info->var.xres
@@ -1170,6 +1263,14 @@ static int sh_mobile_set_par(struct fb_info *info)
info->fix.line_length = line_length;
}
+ if (sh_mobile_format_is_fourcc(&info->var)) {
+ info->fix.type = FB_TYPE_FOURCC;
+ info->fix.visual = FB_VISUAL_FOURCC;
+ } else {
+ info->fix.type = FB_TYPE_PACKED_PIXELS;
+ info->fix.visual = FB_VISUAL_TRUECOLOR;
+ }
+
return ret;
}
@@ -1464,9 +1565,9 @@ static int __devinit sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch,
for (i = 0, mode = cfg->lcd_cfg; i < cfg->num_cfg; i++, mode++) {
unsigned int size = mode->yres * mode->xres;
- /* NV12 buffers must have even number of lines */
- if ((cfg->nonstd) && cfg->bpp = 12 &&
- (mode->yres & 0x1)) {
+ /* NV12/NV21 buffers must have even number of lines */
+ if ((cfg->fourcc = V4L2_PIX_FMT_NV12 ||
+ cfg->fourcc = V4L2_PIX_FMT_NV21) && (mode->yres & 0x1)) {
dev_err(dev, "yres must be multiple of 2 for YCbCr420 "
"mode.\n");
return -EINVAL;
@@ -1484,14 +1585,6 @@ static int __devinit sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch,
dev_dbg(dev, "Found largest videomode %ux%u\n",
max_mode->xres, max_mode->yres);
- /* Initialize fixed screen information. Restrict pan to 2 lines steps
- * for NV12.
- */
- info->fix = sh_mobile_lcdc_fix;
- info->fix.smem_len = max_size * 2 * cfg->bpp / 8;
- if (cfg->nonstd && cfg->bpp = 12)
- info->fix.ypanstep = 2;
-
/* Create the mode list. */
if (cfg->lcd_cfg = NULL) {
mode = &default_720p;
@@ -1509,19 +1602,38 @@ static int __devinit sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch,
*/
var = &info->var;
fb_videomode_to_var(var, mode);
- var->bits_per_pixel = cfg->bpp;
var->width = cfg->lcd_size_cfg.width;
var->height = cfg->lcd_size_cfg.height;
var->yres_virtual = var->yres * 2;
var->activate = FB_ACTIVATE_NOW;
+ switch (cfg->fourcc) {
+ case V4L2_PIX_FMT_RGB565:
+ var->bits_per_pixel = 16;
+ break;
+ case V4L2_PIX_FMT_BGR24:
+ var->bits_per_pixel = 24;
+ break;
+ case V4L2_PIX_FMT_BGR32:
+ var->bits_per_pixel = 32;
+ break;
+ default:
+ var->grayscale = cfg->fourcc;
+ break;
+ }
+
+ /* Make sure the memory size check won't fail. smem_len is initialized
+ * later based on var.
+ */
+ info->fix.smem_len = UINT_MAX;
ret = sh_mobile_check_var(var, info);
if (ret)
return ret;
+ max_size = max_size * var->bits_per_pixel / 8 * 2;
+
/* Allocate frame buffer memory and color map. */
- buf = dma_alloc_coherent(dev, info->fix.smem_len, &ch->dma_handle,
- GFP_KERNEL);
+ buf = dma_alloc_coherent(dev, max_size, &ch->dma_handle, GFP_KERNEL);
if (!buf) {
dev_err(dev, "unable to allocate buffer\n");
return -ENOMEM;
@@ -1530,16 +1642,27 @@ static int __devinit sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch,
ret = fb_alloc_cmap(&info->cmap, PALETTE_NR, 0);
if (ret < 0) {
dev_err(dev, "unable to allocate cmap\n");
- dma_free_coherent(dev, info->fix.smem_len,
- buf, ch->dma_handle);
+ dma_free_coherent(dev, max_size, buf, ch->dma_handle);
return ret;
}
+ /* Initialize fixed screen information. Restrict pan to 2 lines steps
+ * for NV12 and NV21.
+ */
+ info->fix = sh_mobile_lcdc_fix;
info->fix.smem_start = ch->dma_handle;
- if (var->nonstd)
+ info->fix.smem_len = max_size;
+ if (cfg->fourcc = V4L2_PIX_FMT_NV12 ||
+ cfg->fourcc = V4L2_PIX_FMT_NV21)
+ info->fix.ypanstep = 2;
+
+ if (sh_mobile_format_is_yuv(var)) {
info->fix.line_length = var->xres;
- else
- info->fix.line_length = var->xres * (cfg->bpp / 8);
+ info->fix.visual = FB_VISUAL_FOURCC;
+ } else {
+ info->fix.line_length = var->xres * var->bits_per_pixel / 8;
+ info->fix.visual = FB_VISUAL_TRUECOLOR;
+ }
info->screen_base = buf;
info->device = dev;
@@ -1626,9 +1749,9 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
goto err1;
}
- /* for dual channel LCDC (MAIN + SUB) force shared bpp setting */
+ /* for dual channel LCDC (MAIN + SUB) force shared format setting */
if (num_channels = 2)
- priv->forced_bpp = pdata->ch[0].bpp;
+ priv->forced_fourcc = pdata->ch[0].fourcc;
priv->base = ioremap_nocache(res->start, resource_size(res));
if (!priv->base)
@@ -1675,13 +1798,10 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
if (error < 0)
goto err1;
- dev_info(info->dev,
- "registered %s/%s as %dx%d %dbpp.\n",
- pdev->name,
- (ch->cfg.chan = LCDC_CHAN_MAINLCD) ?
- "mainlcd" : "sublcd",
- info->var.xres, info->var.yres,
- ch->cfg.bpp);
+ dev_info(info->dev, "registered %s/%s as %dx%d %dbpp.\n",
+ pdev->name, (ch->cfg.chan = LCDC_CHAN_MAINLCD) ?
+ "mainlcd" : "sublcd", info->var.xres, info->var.yres,
+ info->var.bits_per_pixel);
/* deferred io mode: disable clock to save power */
if (info->fbdefio || info->state = FBINFO_STATE_SUSPENDED)
diff --git a/include/video/sh_mobile_lcdc.h b/include/video/sh_mobile_lcdc.h
index 8101b72..fe30b75 100644
--- a/include/video/sh_mobile_lcdc.h
+++ b/include/video/sh_mobile_lcdc.h
@@ -174,7 +174,8 @@ struct sh_mobile_lcdc_bl_info {
struct sh_mobile_lcdc_chan_cfg {
int chan;
- int bpp;
+ int fourcc;
+ int colorspace;
int interface_type; /* selects RGBn or SYSn I/F, see above */
int clock_divider;
unsigned long flags; /* LCDC_FLAGS_... */
@@ -184,7 +185,6 @@ struct sh_mobile_lcdc_chan_cfg {
struct sh_mobile_lcdc_board_cfg board_cfg;
struct sh_mobile_lcdc_bl_info bl_info;
struct sh_mobile_lcdc_sys_bus_cfg sys_bus_cfg; /* only for SYSn I/F */
- int nonstd;
struct sh_mobile_meram_cfg *meram_cfg;
};
--
1.7.3.4
^ permalink raw reply related
* [PATCH v4 2/3] v4l: Add V4L2_PIX_FMT_NV24 and V4L2_PIX_FMT_NV42 formats
From: Laurent Pinchart @ 2011-11-29 10:26 UTC (permalink / raw)
To: linux-fbdev; +Cc: linux-media
In-Reply-To: <1322562419-9934-1-git-send-email-laurent.pinchart@ideasonboard.com>
NV24 and NV42 are planar YCbCr 4:4:4 and YCrCb 4:4:4 formats with a
luma plane followed by an interleaved chroma plane.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Documentation/DocBook/media/v4l/pixfmt-nv24.xml | 129 +++++++++++++++++++++++
Documentation/DocBook/media/v4l/pixfmt.xml | 1 +
include/linux/videodev2.h | 2 +
3 files changed, 132 insertions(+), 0 deletions(-)
create mode 100644 Documentation/DocBook/media/v4l/pixfmt-nv24.xml
diff --git a/Documentation/DocBook/media/v4l/pixfmt-nv24.xml b/Documentation/DocBook/media/v4l/pixfmt-nv24.xml
new file mode 100644
index 0000000..939c803
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/pixfmt-nv24.xml
@@ -0,0 +1,129 @@
+ <refentry>
+ <refmeta>
+ <refentrytitle>V4L2_PIX_FMT_NV24 ('NV24'), V4L2_PIX_FMT_NV42 ('NV42')</refentrytitle>
+ &manvol;
+ </refmeta>
+ <refnamediv>
+ <refname id="V4L2-PIX-FMT-NV24"><constant>V4L2_PIX_FMT_NV24</constant></refname>
+ <refname id="V4L2-PIX-FMT-NV42"><constant>V4L2_PIX_FMT_NV42</constant></refname>
+ <refpurpose>Formats with full horizontal and vertical
+chroma resolutions, also known as YUV 4:4:4. One luminance and one
+chrominance plane with alternating chroma samples as opposed to
+<constant>V4L2_PIX_FMT_YVU420</constant></refpurpose>
+ </refnamediv>
+ <refsect1>
+ <title>Description</title>
+
+ <para>These are two-plane versions of the YUV 4:4:4 format. The three
+ components are separated into two sub-images or planes. The Y plane is
+ first, with each Y sample stored in one byte per pixel. For
+ <constant>V4L2_PIX_FMT_NV24</constant>, a combined CbCr plane
+ immediately follows the Y plane in memory. The CbCr plane has the same
+ width and height, in pixels, as the Y plane (and the image). Each line
+ contains one CbCr pair per pixel, with each Cb and Cr sample stored in
+ one byte. <constant>V4L2_PIX_FMT_NV42</constant> is the same except that
+ the Cb and Cr samples are swapped, the CrCb plane starts with a Cr
+ sample.</para>
+
+ <para>If the Y plane has pad bytes after each row, then the CbCr plane
+ has twice as many pad bytes after its rows.</para>
+
+ <example>
+ <title><constant>V4L2_PIX_FMT_NV24</constant> 4 × 4
+pixel image</title>
+
+ <formalpara>
+ <title>Byte Order.</title>
+ <para>Each cell is one byte.
+ <informaltable frame="none">
+ <tgroup cols="9" align="center">
+ <colspec align="left" colwidth="2*" />
+ <tbody valign="top">
+ <row>
+ <entry>start + 0:</entry>
+ <entry>Y'<subscript>00</subscript></entry>
+ <entry>Y'<subscript>01</subscript></entry>
+ <entry>Y'<subscript>02</subscript></entry>
+ <entry>Y'<subscript>03</subscript></entry>
+ </row>
+ <row>
+ <entry>start + 4:</entry>
+ <entry>Y'<subscript>10</subscript></entry>
+ <entry>Y'<subscript>11</subscript></entry>
+ <entry>Y'<subscript>12</subscript></entry>
+ <entry>Y'<subscript>13</subscript></entry>
+ </row>
+ <row>
+ <entry>start + 8:</entry>
+ <entry>Y'<subscript>20</subscript></entry>
+ <entry>Y'<subscript>21</subscript></entry>
+ <entry>Y'<subscript>22</subscript></entry>
+ <entry>Y'<subscript>23</subscript></entry>
+ </row>
+ <row>
+ <entry>start + 12:</entry>
+ <entry>Y'<subscript>30</subscript></entry>
+ <entry>Y'<subscript>31</subscript></entry>
+ <entry>Y'<subscript>32</subscript></entry>
+ <entry>Y'<subscript>33</subscript></entry>
+ </row>
+ <row>
+ <entry>start + 16:</entry>
+ <entry>Cb<subscript>00</subscript></entry>
+ <entry>Cr<subscript>00</subscript></entry>
+ <entry>Cb<subscript>01</subscript></entry>
+ <entry>Cr<subscript>01</subscript></entry>
+ <entry>Cb<subscript>02</subscript></entry>
+ <entry>Cr<subscript>02</subscript></entry>
+ <entry>Cb<subscript>03</subscript></entry>
+ <entry>Cr<subscript>03</subscript></entry>
+ </row>
+ <row>
+ <entry>start + 24:</entry>
+ <entry>Cb<subscript>10</subscript></entry>
+ <entry>Cr<subscript>10</subscript></entry>
+ <entry>Cb<subscript>11</subscript></entry>
+ <entry>Cr<subscript>11</subscript></entry>
+ <entry>Cb<subscript>12</subscript></entry>
+ <entry>Cr<subscript>12</subscript></entry>
+ <entry>Cb<subscript>13</subscript></entry>
+ <entry>Cr<subscript>13</subscript></entry>
+ </row>
+ <row>
+ <entry>start + 32:</entry>
+ <entry>Cb<subscript>20</subscript></entry>
+ <entry>Cr<subscript>20</subscript></entry>
+ <entry>Cb<subscript>21</subscript></entry>
+ <entry>Cr<subscript>21</subscript></entry>
+ <entry>Cb<subscript>22</subscript></entry>
+ <entry>Cr<subscript>22</subscript></entry>
+ <entry>Cb<subscript>23</subscript></entry>
+ <entry>Cr<subscript>23</subscript></entry>
+ </row>
+ <row>
+ <entry>start + 40:</entry>
+ <entry>Cb<subscript>30</subscript></entry>
+ <entry>Cr<subscript>30</subscript></entry>
+ <entry>Cb<subscript>31</subscript></entry>
+ <entry>Cr<subscript>31</subscript></entry>
+ <entry>Cb<subscript>32</subscript></entry>
+ <entry>Cr<subscript>32</subscript></entry>
+ <entry>Cb<subscript>33</subscript></entry>
+ <entry>Cr<subscript>33</subscript></entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </informaltable>
+ </para>
+ </formalpara>
+ </example>
+ </refsect1>
+ </refentry>
+
+ <!--
+Local Variables:
+mode: sgml
+sgml-parent-document: "pixfmt.sgml"
+indent-tabs-mode: nil
+End:
+ -->
diff --git a/Documentation/DocBook/media/v4l/pixfmt.xml b/Documentation/DocBook/media/v4l/pixfmt.xml
index 2ff6b77..aef4615 100644
--- a/Documentation/DocBook/media/v4l/pixfmt.xml
+++ b/Documentation/DocBook/media/v4l/pixfmt.xml
@@ -714,6 +714,7 @@ information.</para>
&sub-nv12m;
&sub-nv12mt;
&sub-nv16;
+ &sub-nv24;
&sub-m420;
</section>
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 4b752d5..d2f74f8 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -343,6 +343,8 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_NV21 v4l2_fourcc('N', 'V', '2', '1') /* 12 Y/CrCb 4:2:0 */
#define V4L2_PIX_FMT_NV16 v4l2_fourcc('N', 'V', '1', '6') /* 16 Y/CbCr 4:2:2 */
#define V4L2_PIX_FMT_NV61 v4l2_fourcc('N', 'V', '6', '1') /* 16 Y/CrCb 4:2:2 */
+#define V4L2_PIX_FMT_NV24 v4l2_fourcc('N', 'V', '2', '4') /* 24 Y/CbCr 4:4:4 */
+#define V4L2_PIX_FMT_NV42 v4l2_fourcc('N', 'V', '4', '2') /* 24 Y/CrCb 4:4:4 */
/* two non contiguous planes - one Y, one Cr + Cb interleaved */
#define V4L2_PIX_FMT_NV12M v4l2_fourcc('N', 'M', '1', '2') /* 12 Y/CbCr 4:2:0 */
--
1.7.3.4
^ permalink raw reply related
* [PATCH v4 1/3] fbdev: Add FOURCC-based format configuration API
From: Laurent Pinchart @ 2011-11-29 10:26 UTC (permalink / raw)
To: linux-fbdev; +Cc: linux-media
In-Reply-To: <1322562419-9934-1-git-send-email-laurent.pinchart@ideasonboard.com>
This API will be used to support YUV frame buffer formats in a standard
way.
Last but not least, create a much needed fbdev API documentation and
document the format setting APIs.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Documentation/fb/api.txt | 306 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/fb.h | 14 ++-
2 files changed, 316 insertions(+), 4 deletions(-)
create mode 100644 Documentation/fb/api.txt
diff --git a/Documentation/fb/api.txt b/Documentation/fb/api.txt
new file mode 100644
index 0000000..8e2db6c
--- /dev/null
+++ b/Documentation/fb/api.txt
@@ -0,0 +1,306 @@
+ The Frame Buffer Device API
+ ---------------------------
+
+Last revised: June 21, 2011
+
+
+0. Introduction
+---------------
+
+This document describes the frame buffer API used by applications to interact
+with frame buffer devices. In-kernel APIs between device drivers and the frame
+buffer core are not described.
+
+Due to a lack of documentation in the original frame buffer API, drivers
+behaviours differ in subtle (and not so subtle) ways. This document describes
+the recommended API implementation, but applications should be prepared to
+deal with different behaviours.
+
+
+1. Capabilities
+---------------
+
+Device and driver capabilities are reported in the fixed screen information
+capabilities field.
+
+struct fb_fix_screeninfo {
+ ...
+ __u16 capabilities; /* see FB_CAP_* */
+ ...
+};
+
+Application should use those capabilities to find out what features they can
+expect from the device and driver.
+
+- FB_CAP_FOURCC
+
+The driver supports the four character code (FOURCC) based format setting API.
+When supported, formats are configured using a FOURCC instead of manually
+specifying color components layout.
+
+
+2. Types and visuals
+--------------------
+
+Pixels are stored in memory in hardware-dependent formats. Applications need
+to be aware of the pixel storage format in order to write image data to the
+frame buffer memory in the format expected by the hardware.
+
+Formats are described by frame buffer types and visuals. Some visuals require
+additional information, which are stored in the variable screen information
+bits_per_pixel, grayscale, red, green, blue and transp fields.
+
+Visuals describe how color information is encoded and assembled to create
+macropixels. Types describe how macropixels are stored in memory. The following
+types and visuals are supported.
+
+- FB_TYPE_PACKED_PIXELS
+
+Macropixels are stored contiguously in a single plane. If the number of bits
+per macropixel is not a multiple of 8, whether macropixels are padded to the
+next multiple of 8 bits or packed together into bytes depends on the visual.
+
+Padding at end of lines may be present and is then reported through the fixed
+screen information line_length field.
+
+- FB_TYPE_PLANES
+
+Macropixels are split across multiple planes. The number of planes is equal to
+the number of bits per macropixel, with plane i'th storing i'th bit from all
+macropixels.
+
+Planes are located contiguously in memory.
+
+- FB_TYPE_INTERLEAVED_PLANES
+
+Macropixels are split across multiple planes. The number of planes is equal to
+the number of bits per macropixel, with plane i'th storing i'th bit from all
+macropixels.
+
+Planes are interleaved in memory. The interleave factor, defined as the
+distance in bytes between the beginning of two consecutive interleaved blocks
+belonging to different planes, is stored in the fixed screen information
+type_aux field.
+
+- FB_TYPE_FOURCC
+
+Macropixels are stored in memory as described by the format FOURCC identifier
+stored in the variable screen information grayscale field.
+
+- FB_VISUAL_MONO01
+
+Pixels are black or white and stored on a number of bits (typically one)
+specified by the variable screen information bpp field.
+
+Black pixels are represented by all bits set to 1 and white pixels by all bits
+set to 0. When the number of bits per pixel is smaller than 8, several pixels
+are packed together in a byte.
+
+FB_VISUAL_MONO01 is currently used with FB_TYPE_PACKED_PIXELS only.
+
+- FB_VISUAL_MONO10
+
+Pixels are black or white and stored on a number of bits (typically one)
+specified by the variable screen information bpp field.
+
+Black pixels are represented by all bits set to 0 and white pixels by all bits
+set to 1. When the number of bits per pixel is smaller than 8, several pixels
+are packed together in a byte.
+
+FB_VISUAL_MONO01 is currently used with FB_TYPE_PACKED_PIXELS only.
+
+- FB_VISUAL_TRUECOLOR
+
+Pixels are broken into red, green and blue components, and each component
+indexes a read-only lookup table for the corresponding value. Lookup tables
+are device-dependent, and provide linear or non-linear ramps.
+
+Each component is stored in a macropixel according to the variable screen
+information red, green, blue and transp fields.
+
+- FB_VISUAL_PSEUDOCOLOR and FB_VISUAL_STATIC_PSEUDOCOLOR
+
+Pixel values are encoded as indices into a colormap that stores red, green and
+blue components. The colormap is read-only for FB_VISUAL_STATIC_PSEUDOCOLOR
+and read-write for FB_VISUAL_PSEUDOCOLOR.
+
+Each pixel value is stored in the number of bits reported by the variable
+screen information bits_per_pixel field.
+
+- FB_VISUAL_DIRECTCOLOR
+
+Pixels are broken into red, green and blue components, and each component
+indexes a programmable lookup table for the corresponding value.
+
+Each component is stored in a macropixel according to the variable screen
+information red, green, blue and transp fields.
+
+- FB_VISUAL_FOURCC
+
+Pixels are encoded and interpreted as described by the format FOURCC
+identifier stored in the variable screen information grayscale field.
+
+
+3. Screen information
+---------------------
+
+Screen information are queried by applications using the FBIOGET_FSCREENINFO
+and FBIOGET_VSCREENINFO ioctls. Those ioctls take a pointer to a
+fb_fix_screeninfo and fb_var_screeninfo structure respectively.
+
+struct fb_fix_screeninfo stores device independent unchangeable information
+about the frame buffer device and the current format. Those information can't
+be directly modified by applications, but can be changed by the driver when an
+application modifies the format.
+
+struct fb_fix_screeninfo {
+ char id[16]; /* identification string eg "TT Builtin" */
+ unsigned long smem_start; /* Start of frame buffer mem */
+ /* (physical address) */
+ __u32 smem_len; /* Length of frame buffer mem */
+ __u32 type; /* see FB_TYPE_* */
+ __u32 type_aux; /* Interleave for interleaved Planes */
+ __u32 visual; /* see FB_VISUAL_* */
+ __u16 xpanstep; /* zero if no hardware panning */
+ __u16 ypanstep; /* zero if no hardware panning */
+ __u16 ywrapstep; /* zero if no hardware ywrap */
+ __u32 line_length; /* length of a line in bytes */
+ unsigned long mmio_start; /* Start of Memory Mapped I/O */
+ /* (physical address) */
+ __u32 mmio_len; /* Length of Memory Mapped I/O */
+ __u32 accel; /* Indicate to driver which */
+ /* specific chip/card we have */
+ __u16 capabilities; /* see FB_CAP_* */
+ __u16 reserved[2]; /* Reserved for future compatibility */
+};
+
+struct fb_var_screeninfo stores device independent changeable information
+about a frame buffer device, its current format and video mode, as well as
+other miscellaneous parameters.
+
+struct fb_var_screeninfo {
+ __u32 xres; /* visible resolution */
+ __u32 yres;
+ __u32 xres_virtual; /* virtual resolution */
+ __u32 yres_virtual;
+ __u32 xoffset; /* offset from virtual to visible */
+ __u32 yoffset; /* resolution */
+
+ __u32 bits_per_pixel; /* guess what */
+ __u32 grayscale; /* 0 = color, 1 = grayscale, */
+ /* >1 = FOURCC */
+ struct fb_bitfield red; /* bitfield in fb mem if true color, */
+ struct fb_bitfield green; /* else only length is significant */
+ struct fb_bitfield blue;
+ struct fb_bitfield transp; /* transparency */
+
+ __u32 nonstd; /* != 0 Non standard pixel format */
+
+ __u32 activate; /* see FB_ACTIVATE_* */
+
+ __u32 height; /* height of picture in mm */
+ __u32 width; /* width of picture in mm */
+
+ __u32 accel_flags; /* (OBSOLETE) see fb_info.flags */
+
+ /* Timing: All values in pixclocks, except pixclock (of course) */
+ __u32 pixclock; /* pixel clock in ps (pico seconds) */
+ __u32 left_margin; /* time from sync to picture */
+ __u32 right_margin; /* time from picture to sync */
+ __u32 upper_margin; /* time from sync to picture */
+ __u32 lower_margin;
+ __u32 hsync_len; /* length of horizontal sync */
+ __u32 vsync_len; /* length of vertical sync */
+ __u32 sync; /* see FB_SYNC_* */
+ __u32 vmode; /* see FB_VMODE_* */
+ __u32 rotate; /* angle we rotate counter clockwise */
+ __u32 colorspace; /* colorspace for FOURCC-based modes */
+ __u32 reserved[4]; /* Reserved for future compatibility */
+};
+
+To modify variable information, applications call the FBIOPUT_VSCREENINFO
+ioctl with a pointer to a fb_var_screeninfo structure. If the call is
+successful, the driver will update the fixed screen information accordingly.
+
+Instead of filling the complete fb_var_screeninfo structure manually,
+applications should call the FBIOGET_VSCREENINFO ioctl and modify only the
+fields they care about.
+
+
+4. Format configuration
+-----------------------
+
+Frame buffer devices offer two ways to configure the frame buffer format: the
+legacy API and the FOURCC-based API.
+
+
+The legacy API has been the only frame buffer format configuration API for a
+long time and is thus widely used by application. It is the recommended API
+for applications when using RGB and grayscale formats, as well as legacy
+non-standard formats.
+
+To select a format, applications set the fb_var_screeninfo bits_per_pixel field
+to the desired frame buffer depth. Values up to 8 will usually map to
+monochrome, grayscale or pseudocolor visuals, although this is not required.
+
+- For grayscale formats, applications set the grayscale field to one. The red,
+ blue, green and transp fields must be set to 0 by applications and ignored by
+ drivers. Drivers must fill the red, blue and green offsets to 0 and lengths
+ to the bits_per_pixel value.
+
+- For pseudocolor formats, applications set the grayscale field to zero. The
+ red, blue, green and transp fields must be set to 0 by applications and
+ ignored by drivers. Drivers must fill the red, blue and green offsets to 0
+ and lengths to the bits_per_pixel value.
+
+- For truecolor and directcolor formats, applications set the grayscale field
+ to zero, and the red, blue, green and transp fields to describe the layout of
+ color components in memory.
+
+struct fb_bitfield {
+ __u32 offset; /* beginning of bitfield */
+ __u32 length; /* length of bitfield */
+ __u32 msb_right; /* != 0 : Most significant bit is */
+ /* right */
+};
+
+ Pixel values are bits_per_pixel wide and are split in non-overlapping red,
+ green, blue and alpha (transparency) components. Location and size of each
+ component in the pixel value are described by the fb_bitfield offset and
+ length fields. Offset are computed from the right.
+
+ Pixels are always stored in an integer number of bytes. If the number of
+ bits per pixel is not a multiple of 8, pixel values are padded to the next
+ multiple of 8 bits.
+
+Upon successful format configuration, drivers update the fb_fix_screeninfo
+type, visual and line_length fields depending on the selected format.
+
+
+The FOURCC-based API replaces format descriptions by four character codes
+(FOURCC). FOURCCs are abstract identifiers that uniquely define a format
+without explicitly describing it. This is the only API that supports YUV
+formats. Drivers are also encouraged to implement the FOURCC-based API for RGB
+and grayscale formats.
+
+Drivers that support the FOURCC-based API report this capability by setting
+the FB_CAP_FOURCC bit in the fb_fix_screeninfo capabilities field.
+
+FOURCC definitions are located in the linux/videodev2.h header. However, and
+despite starting with the V4L2_PIX_FMT_prefix, they are not restricted to V4L2
+and don't require usage of the V4L2 subsystem. FOURCC documentation is
+available in Documentation/DocBook/v4l/pixfmt.xml.
+
+To select a format, applications set the grayscale field to the desired FOURCC.
+For YUV formats, they should also select the appropriate colorspace by setting
+the colorspace field to one of the colorspaces listed in linux/videodev2.h and
+documented in Documentation/DocBook/v4l/colorspaces.xml.
+
+The red, green, blue and transp fields are not used with the FOURCC-based API.
+For forward compatibility reasons applications must zero those fields, and
+drivers must ignore them. Values other than 0 may get a meaning in future
+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.
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 1d6836c..c18122f 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -45,6 +45,7 @@
#define FB_TYPE_INTERLEAVED_PLANES 2 /* Interleaved planes */
#define FB_TYPE_TEXT 3 /* Text/attributes */
#define FB_TYPE_VGA_PLANES 4 /* EGA/VGA planes */
+#define FB_TYPE_FOURCC 5 /* Type identified by a V4L2 FOURCC */
#define FB_AUX_TEXT_MDA 0 /* Monochrome text */
#define FB_AUX_TEXT_CGA 1 /* CGA/EGA/VGA Color text */
@@ -69,6 +70,7 @@
#define FB_VISUAL_PSEUDOCOLOR 3 /* Pseudo color (like atari) */
#define FB_VISUAL_DIRECTCOLOR 4 /* Direct color */
#define FB_VISUAL_STATIC_PSEUDOCOLOR 5 /* Pseudo color readonly */
+#define FB_VISUAL_FOURCC 6 /* Visual identified by a V4L2 FOURCC */
#define FB_ACCEL_NONE 0 /* no hardware accelerator */
#define FB_ACCEL_ATARIBLITT 1 /* Atari Blitter */
@@ -154,6 +156,8 @@
#define FB_ACCEL_PUV3_UNIGFX 0xa0 /* PKUnity-v3 Unigfx */
+#define FB_CAP_FOURCC 1 /* Device supports FOURCC-based formats */
+
struct fb_fix_screeninfo {
char id[16]; /* identification string eg "TT Builtin" */
unsigned long smem_start; /* Start of frame buffer mem */
@@ -171,7 +175,8 @@ struct fb_fix_screeninfo {
__u32 mmio_len; /* Length of Memory Mapped I/O */
__u32 accel; /* Indicate to driver which */
/* specific chip/card we have */
- __u16 reserved[3]; /* Reserved for future compatibility */
+ __u16 capabilities; /* see FB_CAP_* */
+ __u16 reserved[2]; /* Reserved for future compatibility */
};
/* Interpretation of offset for color fields: All offsets are from the right,
@@ -246,8 +251,8 @@ struct fb_var_screeninfo {
__u32 yoffset; /* resolution */
__u32 bits_per_pixel; /* guess what */
- __u32 grayscale; /* != 0 Graylevels instead of colors */
-
+ __u32 grayscale; /* 0 = color, 1 = grayscale, */
+ /* >1 = FOURCC */
struct fb_bitfield red; /* bitfield in fb mem if true color, */
struct fb_bitfield green; /* else only length is significant */
struct fb_bitfield blue;
@@ -273,7 +278,8 @@ struct fb_var_screeninfo {
__u32 sync; /* see FB_SYNC_* */
__u32 vmode; /* see FB_VMODE_* */
__u32 rotate; /* angle we rotate counter clockwise */
- __u32 reserved[5]; /* Reserved for future compatibility */
+ __u32 colorspace; /* colorspace for FOURCC-based modes */
+ __u32 reserved[4]; /* Reserved for future compatibility */
};
struct fb_cmap {
--
1.7.3.4
^ permalink raw reply related
* [PATCH v4 0/3] fbdev: Add FOURCC-based format configuration API
From: Laurent Pinchart @ 2011-11-29 10:26 UTC (permalink / raw)
To: linux-fbdev; +Cc: linux-media
Hi everybody,
Here's the fourth version of the fbdev FOURCC-based format configuration API.
The third version nearly got merged, but we noticed that gcc choked on
anonymous union initializers. This has been fixed in gcc 4.6, but I don't have
the power to force Linux to set the base gcc version to 4.6 :-)
There were two ways to fix the issue, neither of which was particularly
attractive to me. The first one involved a 10k lines patch that modified all
the drivers, and the second one required using one of the reserved fields in
the fb_var_screeninfo structure. After discussing this with Florian, I've
decided to go for the second fix.
The colorspace field is thus now stored in a previously reserved field. The
fourcc field is still shared with the grayscale field, but doesn't have its
own name anymore.
I've updated the fbdev-test tool to this new API. The code is available in the
fbdev-test yuv branch at
http://git.ideasonboard.org/?pûdev-test.git;a=shortlog;h=refs/heads/yuv.
Laurent Pinchart (3):
fbdev: Add FOURCC-based format configuration API
v4l: Add V4L2_PIX_FMT_NV24 and V4L2_PIX_FMT_NV42 formats
fbdev: sh_mobile_lcdc: Support FOURCC-based format API
Documentation/DocBook/media/v4l/pixfmt-nv24.xml | 129 ++++++++
Documentation/DocBook/media/v4l/pixfmt.xml | 1 +
Documentation/fb/api.txt | 306 +++++++++++++++++++
arch/arm/mach-shmobile/board-ag5evm.c | 2 +-
arch/arm/mach-shmobile/board-ap4evb.c | 4 +-
arch/arm/mach-shmobile/board-mackerel.c | 4 +-
arch/sh/boards/mach-ap325rxa/setup.c | 2 +-
arch/sh/boards/mach-ecovec24/setup.c | 2 +-
arch/sh/boards/mach-kfr2r09/setup.c | 2 +-
arch/sh/boards/mach-migor/setup.c | 4 +-
arch/sh/boards/mach-se/7724/setup.c | 2 +-
drivers/video/sh_mobile_lcdcfb.c | 360 +++++++++++++++--------
include/linux/fb.h | 14 +-
include/linux/videodev2.h | 2 +
include/video/sh_mobile_lcdc.h | 4 +-
15 files changed, 701 insertions(+), 137 deletions(-)
create mode 100644 Documentation/DocBook/media/v4l/pixfmt-nv24.xml
create mode 100644 Documentation/fb/api.txt
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 09/11] video: Remove redundant spi driver bus
From: Tomi Valkeinen @ 2011-11-29 10:19 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: linux-kernel, Florian Tobias Schandinat, linux-fbdev, linux-omap
In-Reply-To: <1322148561-25138-9-git-send-email-lars@metafoo.de>
[-- Attachment #1: Type: text/plain, Size: 791 bytes --]
On Thu, 2011-11-24 at 16:29 +0100, Lars-Peter Clausen wrote:
> In ancient times it was necessary to manually initialize the bus field of an
> spi_driver to spi_bus_type. These days this is done in spi_driver_register(),
> so we can drop the manual assignment.
>
> The patch was generated using the following coccinelle semantic patch:
> // <smpl>
> @@
> identifier _driver;
> @@
> struct spi_driver _driver = {
> .driver = {
> - .bus = &spi_bus_type,
> },
> };
> // </smpl>
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-omap@vger.kernel.org
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH] video: s3c2410: fix checkpatch error and warnings
From: Jingoo Han @ 2011-11-29 9:48 UTC (permalink / raw)
To: linux-fbdev
This patch fixes the checkpatch errors listed below:
ERROR: do not initialise statics to 0 or NULL
WARNING: Use #include <linux/io.h> instead of <asm/io.h>
WARNING: braces {} are not necessary for single statement blocks
WARNING: braces {} are not necessary for any arm of this statement
WARNING: static char array declaration should probably be static const char
WARNING: line over 80 characters
WARNING: msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/s3c2410fb.c | 29 +++++++++++++++--------------
1 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/video/s3c2410fb.c b/drivers/video/s3c2410fb.c
index ee4c0df..77f34c6 100644
--- a/drivers/video/s3c2410fb.c
+++ b/drivers/video/s3c2410fb.c
@@ -26,8 +26,8 @@
#include <linux/platform_device.h>
#include <linux/clk.h>
#include <linux/cpufreq.h>
+#include <linux/io.h>
-#include <asm/io.h>
#include <asm/div64.h>
#include <asm/mach/map.h>
@@ -45,10 +45,10 @@
#ifdef CONFIG_FB_S3C2410_DEBUG
static int debug = 1;
#else
-static int debug = 0;
+static int debug;
#endif
-#define dprintk(msg...) if (debug) { printk(KERN_DEBUG "s3c2410fb: " msg); }
+#define dprintk(msg...) if (debug) printk(KERN_DEBUG "s3c2410fb: " msg);
/* useful functions */
@@ -567,11 +567,10 @@ static int s3c2410fb_blank(int blank_mode, struct fb_info *info)
tpal_reg += is_s3c2412(fbi) ? S3C2412_TPAL : S3C2410_TPAL;
- if (blank_mode = FB_BLANK_POWERDOWN) {
+ if (blank_mode = FB_BLANK_POWERDOWN)
s3c2410fb_lcd_enable(fbi, 0);
- } else {
+ else
s3c2410fb_lcd_enable(fbi, 1);
- }
if (blank_mode = FB_BLANK_UNBLANK)
writel(0x0, tpal_reg);
@@ -812,7 +811,7 @@ static inline void s3c2410fb_cpufreq_deregister(struct s3c2410fb_info *info)
#endif
-static char driver_name[] = "s3c2410fb";
+static const char driver_name[] = "s3c2410fb";
static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
enum s3c_drv_type drv_type)
@@ -881,7 +880,10 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
goto release_mem;
}
- info->irq_base = info->io + ((drv_type = DRV_S3C2412) ? S3C2412_LCDINTBASE : S3C2410_LCDINTBASE);
+ if (drv_type = DRV_S3C2412)
+ info->irq_base = info->io + S3C2412_LCDINTBASE;
+ else
+ info->irq_base = info->io + S3C2410_LCDINTBASE;
dprintk("devinit\n");
@@ -927,7 +929,7 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
clk_enable(info->clk);
dprintk("got and enabled clock\n");
- msleep(1);
+ usleep_range(1000, 1000);
info->clk_rate = clk_get_rate(info->clk);
@@ -975,9 +977,8 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
/* create device files */
ret = device_create_file(&pdev->dev, &dev_attr_debug);
- if (ret) {
+ if (ret)
printk(KERN_ERR "failed to add debug attribute\n");
- }
printk(KERN_INFO "fb%d: %s frame buffer device\n",
fbinfo->node, fbinfo->fix.id);
@@ -1027,7 +1028,7 @@ static int __devexit s3c2410fb_remove(struct platform_device *pdev)
s3c2410fb_cpufreq_deregister(info);
s3c2410fb_lcd_enable(info, 0);
- msleep(1);
+ usleep_range(1000, 1000);
s3c2410fb_unmap_video_memory(fbinfo);
@@ -1064,7 +1065,7 @@ static int s3c2410fb_suspend(struct platform_device *dev, pm_message_t state)
* the LCD DMA engine is not going to get back on the bus
* before the clock goes off again (bjd) */
- msleep(1);
+ usleep_range(1000, 1000);
clk_disable(info->clk);
return 0;
@@ -1076,7 +1077,7 @@ static int s3c2410fb_resume(struct platform_device *dev)
struct s3c2410fb_info *info = fbinfo->par;
clk_enable(info->clk);
- msleep(1);
+ usleep_range(1000, 1000);
s3c2410fb_init_registers(fbinfo);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 06/13] OMAPDSS: DSI: Use new lane config in
From: Carlos Chinea @ 2011-11-29 9:09 UTC (permalink / raw)
To: ext Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <1322495015.2364.12.camel@deskari>
On Mon, 2011-11-28 at 17:43 +0200, ext Tomi Valkeinen wrote:
> On Mon, 2011-11-28 at 11:08 +0200, Carlos Chinea wrote:
> > Hi Tomi,
> >
> > Just a question/suggestion, bellow:
> >
> > On Thu, 2011-11-24 at 15:29 +0200, ext Tomi Valkeinen wrote:
> > > Use the new lane config in dsi_set_lane_config().
> > >
> > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > > ---
> > > drivers/video/omap2/dss/dsi.c | 84 +++++++++++++++++++---------------------
> > > 1 files changed, 40 insertions(+), 44 deletions(-)
> > >
> > > diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> > > index aea110c..ba8d6b3 100644
> > > --- a/drivers/video/omap2/dss/dsi.c
> > > +++ b/drivers/video/omap2/dss/dsi.c
> > > @@ -2154,59 +2154,53 @@ static int dsi_parse_lane_config(struct omap_dss_device *dssdev)
> > > return 0;
> > > }
> > >
> > > -static void dsi_set_lane_config(struct omap_dss_device *dssdev)
> > > +static int dsi_set_lane_config(struct omap_dss_device *dssdev)
> > > {
> > > struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> > > + struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
> > > + static const u8 offsets[] = { 0, 4, 8, 12, 16 };
> > > + static const enum dsi_lane_function functions[] = {
> > > + DSI_LANE_CLK,
> > > + DSI_LANE_DATA1,
> > > + DSI_LANE_DATA2,
> > > + DSI_LANE_DATA3,
> > > + DSI_LANE_DATA4,
> > > + };
> >
> > Patch 05 of the series has a function (dsi_parse_lane_config) with
> > exactly the same static local declaration. Wouldn't be better to have an
> > static global declaration instead to save some space ? or are the values
> > from those functions going to differ in the near future ?
>
> True, the array could be a global, and no, I don't think they'll change
> in the near future.
>
> But the data is more like function internal stuff than global data. The
> functions want to parse and set the lane configs in particular order,
> and use the array for that.
>
> While the order happens to be the same in both functions, I still felt
> the array is internal to each function rather than global data.
Fine for me then.
Br,
Carlos
> Looking
> from outside the function, the order doesn't matter. It's just an
> internal detail.
>
> Tomi
>
^ permalink raw reply
* Re: [PATCH 06/13] OMAPDSS: DSI: Use new lane config in
From: Tomi Valkeinen @ 2011-11-28 15:43 UTC (permalink / raw)
To: Carlos Chinea; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <1322471339.17335.73.camel@groo>
[-- Attachment #1: Type: text/plain, Size: 2106 bytes --]
On Mon, 2011-11-28 at 11:08 +0200, Carlos Chinea wrote:
> Hi Tomi,
>
> Just a question/suggestion, bellow:
>
> On Thu, 2011-11-24 at 15:29 +0200, ext Tomi Valkeinen wrote:
> > Use the new lane config in dsi_set_lane_config().
> >
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > ---
> > drivers/video/omap2/dss/dsi.c | 84 +++++++++++++++++++---------------------
> > 1 files changed, 40 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> > index aea110c..ba8d6b3 100644
> > --- a/drivers/video/omap2/dss/dsi.c
> > +++ b/drivers/video/omap2/dss/dsi.c
> > @@ -2154,59 +2154,53 @@ static int dsi_parse_lane_config(struct omap_dss_device *dssdev)
> > return 0;
> > }
> >
> > -static void dsi_set_lane_config(struct omap_dss_device *dssdev)
> > +static int dsi_set_lane_config(struct omap_dss_device *dssdev)
> > {
> > struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> > + struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
> > + static const u8 offsets[] = { 0, 4, 8, 12, 16 };
> > + static const enum dsi_lane_function functions[] = {
> > + DSI_LANE_CLK,
> > + DSI_LANE_DATA1,
> > + DSI_LANE_DATA2,
> > + DSI_LANE_DATA3,
> > + DSI_LANE_DATA4,
> > + };
>
> Patch 05 of the series has a function (dsi_parse_lane_config) with
> exactly the same static local declaration. Wouldn't be better to have an
> static global declaration instead to save some space ? or are the values
> from those functions going to differ in the near future ?
True, the array could be a global, and no, I don't think they'll change
in the near future.
But the data is more like function internal stuff than global data. The
functions want to parse and set the lane configs in particular order,
and use the array for that.
While the order happens to be the same in both functions, I still felt
the array is internal to each function rather than global data. Looking
from outside the function, the order doesn't matter. It's just an
internal detail.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] video: s3c-fb: Convert to devm style allocation
From: Mark Brown @ 2011-11-28 11:41 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1322434268-25525-2-git-send-email-broonie@opensource.wolfsonmicro.com>
On Mon, Nov 28, 2011 at 07:59:27AM +0000, Jingoo Han wrote:
Don't top post!
> Your patch makes build error as follows:
> drivers/video/s3c-fb.c: In function 's3c_fb_probe':
> drivers/video/s3c-fb.c:1385: error: implicit declaration of function 'devm_request_and_ioremap'
> drivers/video/s3c-fb.c:1385: warning: assignment makes pointer from integer without a cast
> make[2]: *** [drivers/video/s3c-fb.o] Error 1
> make[1]: *** [drivers/video] Error 2
This is present in -next.
^ permalink raw reply
* Re: [PATCH v3 1/3] fbdev: Add FOURCC-based format configuration API
From: Laurent Pinchart @ 2011-11-28 11:12 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: linux-fbdev, linux-media, magnus.damm
In-Reply-To: <4ED01224.9020703@gmx.de>
Hi Florian,
On Friday 25 November 2011 23:09:40 Florian Tobias Schandinat wrote:
> On 11/24/2011 10:50 AM, Laurent Pinchart wrote:
> > Hi Florian,
> >
> > Gentle ping ?
>
> Sorry, but I'm very busy at the moment and therefore time-consuming things,
> like solving challenging problems, are delayed for some time.
No worries.
> > On Sunday 20 November 2011 11:55:22 Laurent Pinchart wrote:
> >> On Sunday 20 November 2011 03:00:33 Florian Tobias Schandinat wrote:
> >>> Hi Laurent,
> >>>
> >>> On 08/31/2011 11:18 AM, Laurent Pinchart wrote:
> >>>> This API will be used to support YUV frame buffer formats in a
> >>>> standard way.
> >>>
> >>> looks like the union is causing problems. With this patch applied I get
> >>>
> >>> errors like this:
> >>> CC [M] drivers/auxdisplay/cfag12864bfb.o
> >>>
> >>> drivers/auxdisplay/cfag12864bfb.c:57: error: unknown field ‘red’
> >>> specified in initializer
> >>
> >> *ouch*
> >>
> >> gcc < 4.6 chokes on anonymous unions initializers :-/
> >>
> >> [snip]
> >>
> >>>> @@ -246,12 +251,23 @@ struct fb_var_screeninfo {
> >>>>
> >>>> __u32 yoffset; /* resolution */
> >>>>
> >>>> __u32 bits_per_pixel; /* guess what */
> >>>>
> >>>> - __u32 grayscale; /* != 0 Graylevels instead of colors */
> >>>>
> >>>> - struct fb_bitfield red; /* bitfield in fb mem if true color, */
> >>>> - struct fb_bitfield green; /* else only length is significant */
> >>>> - struct fb_bitfield blue;
> >>>> - struct fb_bitfield transp; /* transparency */
> >>>> + union {
> >>>> + struct { /* Legacy format API */
> >>>> + __u32 grayscale; /* 0 = color, 1 = grayscale */
> >>>> + /* bitfields in fb mem if true color, else only */
> >>>> + /* length is significant */
> >>>> + struct fb_bitfield red;
> >>>> + struct fb_bitfield green;
> >>>> + struct fb_bitfield blue;
> >>>> + struct fb_bitfield transp; /* transparency */
> >>>> + };
> >>>> + struct { /* FOURCC-based format API */
> >>>> + __u32 fourcc; /* FOURCC format */
> >>>> + __u32 colorspace;
> >>>> + __u32 reserved[11];
> >>>> + } fourcc;
> >>>> + };
> >>
> >> We can't name the union, otherwise this will change the userspace API.
> >>
> >> We could "fix" the problem on the kernel side with
> >>
> >> #ifdef __KERNEL__
> >>
> >> } color;
> >>
> >> #else
> >>
> >> };
> >>
> >> #endif
> >
> > (and the structure that contains the grayscale, red, green, blue and
> > transp fields would need to be similarly named, the "rgb" name comes to
> > mind)
>
> Which, I guess, would require modifying all drivers?
Unfortunately. That can be automated using coccinelle (I wrote a semantic
patch for that), but it will still be around 10k lines of diff.
> I don't consider that a good idea. Maybe the simplest solution would be to
> drop the union idea and just accept an utterly misleading name "grayscale"
> for setting the FOURCC value.
I'll see if we can add an accessor macro to make it more explicit.
> The colorspace could use one of the reserved fields at the end or do you
> worry that we need to add a lot of other things?
For FOURCC-based format configuration I don't think we will need much more. If
we do need lots of additional fields in the future we might have to consider
an fbdev2 API ;-)
I'll resubmit patches based on this.
--
Regards,
Laurent Pinchart
^ 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