* [PATCH v2 10/24] video: da8xx-fb: fb_set_par support
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
fb_set_par helps in runtime configuration of lcd controller like
changing resolution, pixel clock etc. (eg. using fbset utility)
Reconfigure lcd controller based on information passed by framework.
Enable raster back if it was already enabled.
As fb_set_par would get invoked indirectly from probe via fb_set_var,
remove existing lcdc initialization in probe and do lcdc reset in
probe so that reset happens only at the begining.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 60 +++++++++++++++++++++++++++++++++++++--------
1 files changed, 49 insertions(+), 11 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 8d73730..b25b810 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -243,6 +243,11 @@ static struct fb_videomode known_lcd_panels[] = {
},
};
+static inline bool da8xx_fb_is_raster_enabled(void)
+{
+ return !!(lcdc_read(LCD_RASTER_CTRL_REG) & LCD_RASTER_ENABLE);
+}
+
/* Enable the Raster Engine of the LCD Controller */
static inline void lcd_enable_raster(void)
{
@@ -665,9 +670,6 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
static void da8xx_fb_lcd_reset(void)
{
- /* Disable the Raster if previously Enabled */
- lcd_disable_raster(false);
-
/* DMA has to be disabled */
lcdc_write(0, LCD_DMA_CTRL_REG);
lcdc_write(0, LCD_RASTER_CTRL_REG);
@@ -720,8 +722,6 @@ static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
u32 bpp;
int ret = 0;
- da8xx_fb_lcd_reset();
-
da8xx_fb_calc_config_clk_divider(par, panel);
if (panel->sync & FB_SYNC_CLK_INVERT)
@@ -1201,9 +1201,52 @@ static int da8xx_pan_display(struct fb_var_screeninfo *var,
return ret;
}
+static int da8xxfb_set_par(struct fb_info *info)
+{
+ struct da8xx_fb_par *par = info->par;
+ int ret;
+ bool raster = da8xx_fb_is_raster_enabled();
+
+ if (raster)
+ lcd_disable_raster(true);
+ else
+ lcd_disable_raster(false);
+
+ fb_var_to_videomode(&par->mode, &info->var);
+
+ par->cfg.bpp = info->var.bits_per_pixel;
+
+ info->fix.visual = (par->cfg.bpp <= 8) ?
+ FB_VISUAL_PSEUDOCOLOR : FB_VISUAL_TRUECOLOR;
+ info->fix.line_length = (par->mode.xres * par->cfg.bpp) / 8;
+
+ ret = lcd_init(par, &par->cfg, &par->mode);
+ if (ret < 0) {
+ dev_err(par->dev, "lcd init failed\n");
+ return ret;
+ }
+
+ par->dma_start = info->fix.smem_start +
+ info->var.yoffset * info->fix.line_length +
+ info->var.xoffset * info->var.bits_per_pixel / 8;
+ par->dma_end = par->dma_start +
+ info->var.yres * info->fix.line_length - 1;
+
+ lcdc_write(par->dma_start, LCD_DMA_FRM_BUF_BASE_ADDR_0_REG);
+ lcdc_write(par->dma_end, LCD_DMA_FRM_BUF_CEILING_ADDR_0_REG);
+ lcdc_write(par->dma_start, LCD_DMA_FRM_BUF_BASE_ADDR_1_REG);
+ lcdc_write(par->dma_end, LCD_DMA_FRM_BUF_CEILING_ADDR_1_REG);
+
+ if (raster)
+ lcd_enable_raster();
+
+ return 0;
+}
+
static struct fb_ops da8xx_fb_ops = {
.owner = THIS_MODULE,
.fb_check_var = fb_check_var,
+ .fb_set_par = da8xxfb_set_par,
.fb_setcolreg = fb_setcolreg,
.fb_pan_display = da8xx_pan_display,
.fb_ioctl = fb_ioctl,
@@ -1312,14 +1355,9 @@ static int fb_probe(struct platform_device *device)
}
fb_videomode_to_var(&da8xx_fb_var, lcdc_info);
- fb_var_to_videomode(&par->mode, &da8xx_fb_var);
par->cfg = *lcd_cfg;
- if (lcd_init(par, lcd_cfg, lcdc_info) < 0) {
- dev_err(&device->dev, "lcd_init failed\n");
- ret = -EFAULT;
- goto err_release_fb;
- }
+ da8xx_fb_lcd_reset();
/* allocate frame buffer */
par->vram_size = lcdc_info->xres * lcdc_info->yres * lcd_cfg->bpp;
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 09/24] video: da8xx-fb: report correct pixclock
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
Update "var" pixclock with the value that is configurable in hardware.
This lets user know the actual pixclock.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 7db1097..8d73730 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -686,6 +686,15 @@ static inline unsigned da8xx_fb_calc_clk_divider(struct da8xx_fb_par *par,
return par->lcd_fck_rate / (PICOS2KHZ(pixclock) * 1000);
}
+static inline unsigned da8xx_fb_round_clk(struct da8xx_fb_par *par,
+ unsigned pixclock)
+{
+ unsigned div;
+
+ div = da8xx_fb_calc_clk_divider(par, pixclock);
+ return KHZ2PICOS(par->lcd_fck_rate / (1000 * div));
+}
+
static inline void da8xx_fb_config_clk_divider(unsigned div)
{
/* Configure the LCD clock divisor. */
@@ -962,6 +971,8 @@ static int fb_check_var(struct fb_var_screeninfo *var,
if (var->yres + var->yoffset > var->yres_virtual)
var->yoffset = var->yres_virtual - var->yres;
+ var->pixclock = da8xx_fb_round_clk(par, var->pixclock);
+
return err;
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 08/24] video: da8xx-fb: store struct device *
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
store struct device pointer so that dev_dbg/err can be used outside
of probe.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 0fac1a0..7db1097 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -150,6 +150,7 @@ static inline void lcdc_write(unsigned int val, unsigned int addr)
}
struct da8xx_fb_par {
+ struct device *dev;
resource_size_t p_palette_base;
unsigned char *v_palette_base;
dma_addr_t vram_phys;
@@ -1291,6 +1292,7 @@ static int fb_probe(struct platform_device *device)
}
par = da8xx_fb_info->par;
+ par->dev = &device->dev;
par->lcdc_clk = fb_clk;
par->lcd_fck_rate = clk_get_rate(fb_clk);
if (fb_pdata->panel_power_ctrl) {
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 07/24] video: da8xx-fb: pix clk and clk div handling cleanup
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
Use the new modedb field to store pix clk. Reorganize existing clock
divider functions with names now corresponding to what they do, add
common function prefix.
Fix existing panel modedb pixclock to be in ps instead of Hz. This
needed a change in the way clock divider is calculated. As modedb
pixclock information is now in ps, override on "var" pixclock over
modedb to var conversion is removed.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 48 +++++++++++++++++----------------------------
1 files changed, 18 insertions(+), 30 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index f1d88ac..0fac1a0 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -160,7 +160,6 @@ struct da8xx_fb_par {
struct clk *lcdc_clk;
int irq;
unsigned int palette_sz;
- unsigned int pxl_clk;
int blank;
wait_queue_head_t vsync_wait;
int vsync_flag;
@@ -201,7 +200,7 @@ static struct fb_videomode known_lcd_panels[] = {
.name = "Sharp_LCD035Q3DG01",
.xres = 320,
.yres = 240,
- .pixclock = 4608000,
+ .pixclock = KHZ2PICOS(4607),
.left_margin = 6,
.right_margin = 8,
.upper_margin = 2,
@@ -216,7 +215,7 @@ static struct fb_videomode known_lcd_panels[] = {
.name = "Sharp_LK043T1DG01",
.xres = 480,
.yres = 272,
- .pixclock = 7833600,
+ .pixclock = KHZ2PICOS(7833),
.left_margin = 2,
.right_margin = 2,
.upper_margin = 2,
@@ -231,7 +230,7 @@ static struct fb_videomode known_lcd_panels[] = {
.name = "SP10Q010",
.xres = 320,
.yres = 240,
- .pixclock = 7833600,
+ .pixclock = KHZ2PICOS(7833),
.left_margin = 10,
.right_margin = 10,
.upper_margin = 10,
@@ -680,13 +679,14 @@ static void da8xx_fb_lcd_reset(void)
}
}
-static void lcd_calc_clk_divider(struct da8xx_fb_par *par)
+static inline unsigned da8xx_fb_calc_clk_divider(struct da8xx_fb_par *par,
+ unsigned pixclock)
{
- unsigned int lcd_clk, div;
-
- lcd_clk = clk_get_rate(par->lcdc_clk);
- div = lcd_clk / par->pxl_clk;
+ return par->lcd_fck_rate / (PICOS2KHZ(pixclock) * 1000);
+}
+static inline void da8xx_fb_config_clk_divider(unsigned div)
+{
/* Configure the LCD clock divisor. */
lcdc_write(LCD_CLK_DIVISOR(div) |
(LCD_RASTER_MODE & 0x1), LCD_CTRL_REG);
@@ -694,7 +694,14 @@ static void lcd_calc_clk_divider(struct da8xx_fb_par *par)
if (lcd_revision = LCD_VERSION_2)
lcdc_write(LCD_V2_DMA_CLK_EN | LCD_V2_LIDD_CLK_EN |
LCD_V2_CORE_CLK_EN, LCD_CLK_ENABLE_REG);
+}
+
+static inline void da8xx_fb_calc_config_clk_divider(struct da8xx_fb_par *par,
+ struct fb_videomode *mode)
+{
+ unsigned div = da8xx_fb_calc_clk_divider(par, mode->pixclock);
+ da8xx_fb_config_clk_divider(div);
}
static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
@@ -705,8 +712,7 @@ static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
da8xx_fb_lcd_reset();
- /* Calculate the divider */
- lcd_calc_clk_divider(par);
+ da8xx_fb_calc_config_clk_divider(par, panel);
if (panel->sync & FB_SYNC_CLK_INVERT)
lcdc_write((lcdc_read(LCD_RASTER_TIMING_2_REG) |
@@ -969,7 +975,7 @@ static int lcd_da8xx_cpufreq_transition(struct notifier_block *nb,
if (par->lcd_fck_rate != clk_get_rate(par->lcdc_clk)) {
par->lcd_fck_rate = clk_get_rate(par->lcdc_clk);
lcd_disable_raster(true);
- lcd_calc_clk_divider(par);
+ da8xx_fb_calc_config_clk_divider(par, &par->mode);
if (par->blank = FB_BLANK_UNBLANK)
lcd_enable_raster();
}
@@ -1195,22 +1201,6 @@ static struct fb_ops da8xx_fb_ops = {
.fb_blank = cfb_blank,
};
-/* Calculate and return pixel clock period in pico seconds */
-static unsigned int da8xxfb_pixel_clk_period(struct da8xx_fb_par *par)
-{
- unsigned int lcd_clk, div;
- unsigned int configured_pix_clk;
- unsigned long long pix_clk_period_picosec = 1000000000000ULL;
-
- lcd_clk = clk_get_rate(par->lcdc_clk);
- div = lcd_clk / par->pxl_clk;
- configured_pix_clk = (lcd_clk / div);
-
- do_div(pix_clk_period_picosec, configured_pix_clk);
-
- return pix_clk_period_picosec;
-}
-
static int fb_probe(struct platform_device *device)
{
struct da8xx_lcdc_platform_data *fb_pdata @@ -1303,7 +1293,6 @@ static int fb_probe(struct platform_device *device)
par = da8xx_fb_info->par;
par->lcdc_clk = fb_clk;
par->lcd_fck_rate = clk_get_rate(fb_clk);
- par->pxl_clk = lcdc_info->pixclock;
if (fb_pdata->panel_power_ctrl) {
par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
par->panel_power_ctrl(1);
@@ -1368,7 +1357,6 @@ static int fb_probe(struct platform_device *device)
da8xx_fb_var.grayscale lcd_cfg->panel_shade = MONOCHROME ? 1 : 0;
da8xx_fb_var.bits_per_pixel = lcd_cfg->bpp;
- da8xx_fb_var.pixclock = da8xxfb_pixel_clk_period(par);
/* Initialize fbinfo */
da8xx_fb_info->flags = FBINFO_FLAG_DEFAULT;
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 06/24] video: da8xx-fb: store clk rate even if !CPUFREQ
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
store lcd clk rate always, i.e. irrespective of whether CPUFREQ is
enabled or not. This can be used to get clk rate directly instead of
enquiring with clock framework with clk handle every time.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index d060f14..f1d88ac 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -174,8 +174,8 @@ struct da8xx_fb_par {
unsigned int which_dma_channel_done;
#ifdef CONFIG_CPU_FREQ
struct notifier_block freq_transition;
- unsigned int lcd_fck_rate;
#endif
+ unsigned int lcd_fck_rate;
void (*panel_power_ctrl)(int);
u32 pseudo_palette[16];
struct fb_videomode mode;
@@ -1302,9 +1302,7 @@ static int fb_probe(struct platform_device *device)
par = da8xx_fb_info->par;
par->lcdc_clk = fb_clk;
-#ifdef CONFIG_CPU_FREQ
par->lcd_fck_rate = clk_get_rate(fb_clk);
-#endif
par->pxl_clk = lcdc_info->pixclock;
if (fb_pdata->panel_power_ctrl) {
par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 05/24] video: da8xx-fb: store current display information
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
store current videomode and controller data so that reconfiguring can
be done easily. Reconfiguring would be required in fb_set_par, which
is going to be added soon.
If these details are not stored, the work probe does to retrieve these
information would have to repeated at the place of reconfiguring and
modifying platform data would be necessary to handle controller data
changes like bpp.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 18834fa..d060f14 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -178,6 +178,8 @@ struct da8xx_fb_par {
#endif
void (*panel_power_ctrl)(int);
u32 pseudo_palette[16];
+ struct fb_videomode mode;
+ struct lcd_ctrl_config cfg;
};
static struct fb_var_screeninfo da8xx_fb_var;
@@ -1310,6 +1312,8 @@ static int fb_probe(struct platform_device *device)
}
fb_videomode_to_var(&da8xx_fb_var, lcdc_info);
+ fb_var_to_videomode(&par->mode, &da8xx_fb_var);
+ par->cfg = *lcd_cfg;
if (lcd_init(par, lcd_cfg, lcdc_info) < 0) {
dev_err(&device->dev, "lcd_init failed\n");
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 04/24] video: da8xx-fb: remove unneeded "var" initialization
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
modedb helper now updates "var" information based on the detected
panel, remove the unnecessary initialization.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 22 +---------------------
1 files changed, 1 insertions(+), 21 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index a1f6544..18834fa 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -131,10 +131,6 @@
#define WSI_TIMEOUT 50
#define PALETTE_SIZE 256
-#define LEFT_MARGIN 64
-#define RIGHT_MARGIN 64
-#define UPPER_MARGIN 32
-#define LOWER_MARGIN 32
static void __iomem *da8xx_fb_reg_base;
static struct resource *lcdc_regs;
@@ -184,23 +180,7 @@ struct da8xx_fb_par {
u32 pseudo_palette[16];
};
-/* Variable Screen Information */
-static struct fb_var_screeninfo da8xx_fb_var = {
- .xoffset = 0,
- .yoffset = 0,
- .transp = {0, 0, 0},
- .nonstd = 0,
- .activate = 0,
- .height = -1,
- .width = -1,
- .accel_flags = 0,
- .left_margin = LEFT_MARGIN,
- .right_margin = RIGHT_MARGIN,
- .upper_margin = UPPER_MARGIN,
- .lower_margin = LOWER_MARGIN,
- .sync = 0,
- .vmode = FB_VMODE_NONINTERLACED
-};
+static struct fb_var_screeninfo da8xx_fb_var;
static struct fb_fix_screeninfo da8xx_fb_fix = {
.id = "DA8xx FB Drv",
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 03/24] video: da8xx-fb: use modedb helper to update var
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
modedb structure is now used to store panel information, run modedb
helper over it for initial update of "var" information instead of
equating each fields.
While at it, remove redundant update of bits_per_pixel.
Note: pixclock is overridden with proper value using an existing code
as currently modedb is having it in Hz instead of ps, this would be
fixed in a later change and this overide would be removed.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 18 ++----------------
1 files changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 52977b1..a1f6544 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1329,6 +1329,8 @@ static int fb_probe(struct platform_device *device)
par->panel_power_ctrl(1);
}
+ fb_videomode_to_var(&da8xx_fb_var, lcdc_info);
+
if (lcd_init(par, lcd_cfg, lcdc_info) < 0) {
dev_err(&device->dev, "lcd_init failed\n");
ret = -EFAULT;
@@ -1381,25 +1383,9 @@ static int fb_probe(struct platform_device *device)
goto err_release_pl_mem;
}
- /* Initialize par */
- da8xx_fb_info->var.bits_per_pixel = lcd_cfg->bpp;
-
- da8xx_fb_var.xres = lcdc_info->xres;
- da8xx_fb_var.xres_virtual = lcdc_info->xres;
-
- da8xx_fb_var.yres = lcdc_info->yres;
- da8xx_fb_var.yres_virtual = lcdc_info->yres * LCD_NUM_BUFFERS;
-
da8xx_fb_var.grayscale lcd_cfg->panel_shade = MONOCHROME ? 1 : 0;
da8xx_fb_var.bits_per_pixel = lcd_cfg->bpp;
-
- da8xx_fb_var.hsync_len = lcdc_info->hsync_len;
- da8xx_fb_var.vsync_len = lcdc_info->vsync_len;
- da8xx_fb_var.right_margin = lcdc_info->right_margin;
- da8xx_fb_var.left_margin = lcdc_info->left_margin;
- da8xx_fb_var.lower_margin = lcdc_info->lower_margin;
- da8xx_fb_var.upper_margin = lcdc_info->upper_margin;
da8xx_fb_var.pixclock = da8xxfb_pixel_clk_period(par);
/* Initialize fbinfo */
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 02/24] video: da8xx-fb: simplify lcd_reset
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
lcd_reset function doesn't require any arguement, remove it.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index d00dd17..52977b1 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -681,7 +681,7 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
}
#undef CNVT_TOHW
-static void lcd_reset(struct da8xx_fb_par *par)
+static void da8xx_fb_lcd_reset(void)
{
/* Disable the Raster if previously Enabled */
lcd_disable_raster(false);
@@ -721,7 +721,7 @@ static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
u32 bpp;
int ret = 0;
- lcd_reset(par);
+ da8xx_fb_lcd_reset();
/* Calculate the divider */
lcd_calc_clk_divider(par);
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 01/24] video: da8xx-fb: fb_check_var enhancement
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
Check whether "struct fb_var_screeninfo" fields are sane, if not
update it to be within allowed limits.
If user sends down buggy "var" values, this will bring those within
allowable limits. And fb_set_par is not supposed to change "var"
values, fb_check_var has to ensure that values are proper.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 0810939..d00dd17 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -888,6 +888,9 @@ static int fb_check_var(struct fb_var_screeninfo *var,
struct fb_info *info)
{
int err = 0;
+ struct da8xx_fb_par *par = info->par;
+ int bpp = var->bits_per_pixel >> 3;
+ unsigned long line_size = var->xres_virtual * bpp;
if (var->bits_per_pixel > 16 && lcd_revision = LCD_VERSION_1)
return -EINVAL;
@@ -955,6 +958,21 @@ static int fb_check_var(struct fb_var_screeninfo *var,
var->green.msb_right = 0;
var->blue.msb_right = 0;
var->transp.msb_right = 0;
+
+ if (line_size * var->yres_virtual > par->vram_size)
+ var->yres_virtual = par->vram_size / line_size;
+
+ if (var->yres > var->yres_virtual)
+ var->yres = var->yres_virtual;
+
+ if (var->xres > var->xres_virtual)
+ var->xres = var->xres_virtual;
+
+ if (var->xres + var->xoffset > var->xres_virtual)
+ var->xoffset = var->xres_virtual - var->xres;
+ if (var->yres + var->yoffset > var->yres_virtual)
+ var->yoffset = var->yres_virtual - var->yres;
+
return err;
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 00/24] video/da8xx-fb fbdev driver enhance to support TI am335x SoC
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
Changes in v2:
Addressing review comments from Tomi Valkeinen:
Dropped readl/writel patch
Many cosmetic changes to make code easier to understand
This is primarily a resend of a series of patches that were original
submitted to linux-fbdev back in January of 2013 for 3.8 by Afzal
Mohammed. I have rebased them on 3.10 and also made sure they
apply cleanly to the 'for-next' branch of linux-fbdev git.
The patches enable use of the current mainline da8xx-fb driver on the
TI AM335x SOC along with some bug fixes and cleanup.
The original patch series can be found here:
https://patchwork.kernel.org/project/linux-fbdev/list/?submitter9101
if you want to see the history.
Afzal Mohammed (20):
video: da8xx-fb: fb_check_var enhancement
video: da8xx-fb: simplify lcd_reset
video: da8xx-fb: use modedb helper to update var
video: da8xx-fb: remove unneeded "var" initialization
video: da8xx-fb: store current display information
video: da8xx-fb: store clk rate even if !CPUFREQ
video: da8xx-fb: pix clk and clk div handling cleanup
video: da8xx-fb: store struct device *
video: da8xx-fb: report correct pixclock
video: da8xx-fb: fb_set_par support
video: da8xx-fb: enable sync lost intr for v2 ip
video: da8xx-fb: use devres
video: da8xx-fb: ensure non-null cfg in pdata
video: da8xx-fb: reorganize panel detection
video: da8xx-fb: invoke platform callback safely
video: da8xx-fb: minimal dt support
video: da8xx-fb: obtain fb_videomode info from dt
video: da8xx-fb: ensure pdata only for non-dt
video: da8xx-fb: setup struct lcd_ctrl_config for dt
video: da8xx-fb: set upstream clock rate (if reqd)
Darren Etheridge (3):
video: da8xx-fb: improve readability of code
video: da8xx-fb: make clock naming consistent
video/da8xx-fb adding am33xx as dependency
Manjunathappa, Prakash (1):
video: da8xx-fb: fix 24bpp raster configuration
.../devicetree/bindings/video/fb-da8xx.txt | 37 ++
drivers/video/Kconfig | 7 +-
drivers/video/da8xx-fb.c | 392 +++++++++++++-------
include/video/da8xx-fb.h | 5 +
4 files changed, 299 insertions(+), 142 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt
^ permalink raw reply
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Felipe Balbi @ 2013-07-30 7:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130721154653.GG16598@kroah.com>
[-- Attachment #1: Type: text/plain, Size: 6276 bytes --]
On Sun, Jul 21, 2013 at 08:46:53AM -0700, Greg KH wrote:
> On Sun, Jul 21, 2013 at 01:12:07PM +0200, Tomasz Figa wrote:
> > On Sunday 21 of July 2013 16:37:33 Kishon Vijay Abraham I wrote:
> > > Hi,
> > >
> > > On Sunday 21 July 2013 04:01 PM, Tomasz Figa wrote:
> > > > Hi,
> > > >
> > > > On Saturday 20 of July 2013 19:59:10 Greg KH wrote:
> > > >> On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote:
> > > >>> On Sat, 20 Jul 2013, Greg KH wrote:
> > > >>>>>>> That should be passed using platform data.
> > > >>>>>>
> > > >>>>>> Ick, don't pass strings around, pass pointers. If you have
> > > >>>>>> platform
> > > >>>>>> data you can get to, then put the pointer there, don't use a
> > > >>>>>> "name".
> > > >>>>>
> > > >>>>> I don't think I understood you here :-s We wont have phy pointer
> > > >>>>> when we create the device for the controller no?(it'll be done in
> > > >>>>> board file). Probably I'm missing something.
> > > >>>>
> > > >>>> Why will you not have that pointer? You can't rely on the "name"
> > > >>>> as
> > > >>>> the device id will not match up, so you should be able to rely on
> > > >>>> the pointer being in the structure that the board sets up, right?
> > > >>>>
> > > >>>> Don't use names, especially as ids can, and will, change, that is
> > > >>>> going
> > > >>>> to cause big problems. Use pointers, this is C, we are supposed to
> > > >>>> be
> > > >>>> doing that :)
> > > >>>
> > > >>> Kishon, I think what Greg means is this: The name you are using
> > > >>> must
> > > >>> be stored somewhere in a data structure constructed by the board
> > > >>> file,
> > > >>> right? Or at least, associated with some data structure somehow.
> > > >>> Otherwise the platform code wouldn't know which PHY hardware
> > > >>> corresponded to a particular name.
> > > >>>
> > > >>> Greg's suggestion is that you store the address of that data
> > > >>> structure
> > > >>> in the platform data instead of storing the name string. Have the
> > > >>> consumer pass the data structure's address when it calls phy_create,
> > > >>> instead of passing the name. Then you don't have to worry about two
> > > >>> PHYs accidentally ending up with the same name or any other similar
> > > >>> problems.
> > > >>
> > > >> Close, but the issue is that whatever returns from phy_create()
> > > >> should
> > > >> then be used, no need to call any "find" functions, as you can just
> > > >> use
> > > >> the pointer that phy_create() returns. Much like all other class api
> > > >> functions in the kernel work.
> > > >
> > > > I think there is a confusion here about who registers the PHYs.
> > > >
> > > > All platform code does is registering a platform/i2c/whatever device,
> > > > which causes a driver (located in drivers/phy/) to be instantiated.
> > > > Such drivers call phy_create(), usually in their probe() callbacks,
> > > > so platform_code has no way (and should have no way, for the sake of
> > > > layering) to get what phy_create() returns.
>
> Why not put pointers in the platform data structure that can hold these
> pointers? I thought that is why we created those structures in the
> first place. If not, what are they there for?
heh, IMO we shouldn't pass pointers of any kind through platform_data,
we want to pass data :-)
Allowing to pass pointers through that, is one of the reasons which got
us in such a big mess in ARM land, well it was much easier for a
board-file/driver writer to pass a function pointer then to create a
generic framework :-)
> > > > IMHO we need a lookup method for PHYs, just like for clocks,
> > > > regulators, PWMs or even i2c busses because there are complex cases
> > > > when passing just a name using platform data will not work. I would
> > > > second what Stephen said [1] and define a structure doing things in a
> > > > DT-like way.
> > > >
> > > > Example;
> > > >
> > > > [platform code]
> > > >
> > > > static const struct phy_lookup my_phy_lookup[] = {
> > > >
> > > > PHY_LOOKUP("s3c-hsotg.0", "otg", "samsung-usbphy.1", "phy.2"),
> > >
> > > The only problem here is that if *PLATFORM_DEVID_AUTO* is used while
> > > creating the device, the ids in the device name would change and
> > > PHY_LOOKUP wont be useful.
> >
> > I don't think this is a problem. All the existing lookup methods already
> > use ID to identify devices (see regulators, clkdev, PWMs, i2c, ...). You
> > can simply add a requirement that the ID must be assigned manually,
> > without using PLATFORM_DEVID_AUTO to use PHY lookup.
>
> And I'm saying that this idea, of using a specific name and id, is
> frought with fragility and will break in the future in various ways when
> devices get added to systems, making these strings constantly have to be
> kept up to date with different board configurations.
>
> People, NEVER, hardcode something like an id. The fact that this
> happens today with the clock code, doesn't make it right, it makes the
> clock code wrong. Others have already said that this is wrong there as
> well, as systems change and dynamic ids get used more and more.
>
> Let's not repeat the same mistakes of the past just because we refuse to
> learn from them...
>
> So again, the "find a phy by a string" functions should be removed, the
> device id should be automatically created by the phy core just to make
> things unique in sysfs, and no driver code should _ever_ be reliant on
> the number that is being created, and the pointer to the phy structure
> should be used everywhere instead.
>
> With those types of changes, I will consider merging this subsystem, but
> without them, sorry, I will not.
I'll agree with Greg here, the very fact that we see people trying to
add a requirement of *NOT* using PLATFORM_DEVID_AUTO already points to a
big problem in the framework.
The fact is that if we don't allow PLATFORM_DEVID_AUTO we will end up
adding similar infrastructure to the driver themselves to make sure we
don't end up with duplicate names in sysfs in case we have multiple
instances of the same IP in the SoC (or several of the same PCIe card).
I really don't want to go back to that.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [GIT PULL] Small fbdev fixes for 3.11-rc
From: Tomi Valkeinen @ 2013-07-30 6:31 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jean-Christophe PLAGNIOL-VILLARD, linux-kernel, linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]
Hi Linus,
The following changes since commit 3b2f64d00c46e1e4e9bd0bb9bb12619adac27a4b:
Linux 3.11-rc2 (2013-07-21 12:05:29 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git tags/fbdev-fixes-3.11-rc2
for you to fetch changes up to 7808e3291e1e101e9ad6e8263119c4a2abae05ef:
video: sh7760fb: fix to pass correct device identity to free_irq() (2013-07-29 11:25:03 +0300)
----------------------------------------------------------------
Small fbdev fixes
- Compile fixes
- atyfb initialization fix
- Fix freeing of the irq in sh7760fb & nuc900fb
----------------------------------------------------------------
Dan Carpenter (1):
fbdev/atyfb: fix recent breakage in correct_chipset()
Luis Henriques (1):
vga16fb: Remove unused variable
Michal Simek (1):
video: xilinxfb: Fix compilation warning
Tomi Valkeinen (1):
fbdev/sgivwfb: fix compilation error in sgivwfb_mmap()
Wei Yongjun (2):
video: nuc900fb: fix to pass correct device identity to request_irq()
video: sh7760fb: fix to pass correct device identity to free_irq()
drivers/video/aty/atyfb_base.c | 4 ++--
drivers/video/nuc900fb.c | 3 +--
drivers/video/sgivwfb.c | 2 +-
drivers/video/sh7760fb.c | 2 +-
drivers/video/vga16fb.c | 1 -
drivers/video/xilinxfb.c | 4 ++--
6 files changed, 7 insertions(+), 9 deletions(-)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH v2 6/9] ARM: msm: Move mach/board.h contents to common.h
From: Tomi Valkeinen @ 2013-07-30 5:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130729202031.GA20694@codeaurora.org>
[-- Attachment #1: Type: text/plain, Size: 736 bytes --]
On 29/07/13 23:20, David Brown wrote:
> On Wed, Jul 24, 2013 at 01:54:31PM -0700, Stephen Boyd wrote:
>> The contents of mach/board.h are only used by files within
>> mach-msm so there is no need to export this file outside of the
>> mach-msm directory. Move the contents of the file to common.h to
>> allow us to compile MSM in the multi-platform kernel.
>>
>> Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>> drivers/video/msm/msm_fb.c | 1 -
>
> It'd be nice to get a framebuffer Ack from this part.
>
> original patch:
> https://patchwork.kernel.org/patch/2833029/
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Tom
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [RESEND PATCH v10 1/8] drivers: phy: add generic PHY framework
From: Kishon Vijay Abraham I @ 2013-07-30 5:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <51F68F73.9060603@samsung.com>
Hi,
On Monday 29 July 2013 09:21 PM, Sylwester Nawrocki wrote:
> On 07/26/2013 02:49 PM, Kishon Vijay Abraham I wrote:
>> The PHY framework provides a set of APIs for the PHY drivers to
>> create/destroy a PHY and APIs for the PHY users to obtain a reference to the
>> PHY with or without using phandle. For dt-boot, the PHY drivers should
>> also register *PHY provider* with the framework.
>>
>> PHY drivers should create the PHY by passing id and ops like init, exit,
>> power_on and power_off. This framework is also pm runtime enabled.
>>
>> The documentation for the generic PHY framework is added in
>> Documentation/phy.txt and the documentation for dt binding can be found at
>> Documentation/devicetree/bindings/phy/phy-bindings.txt
>>
>> Cc: Tomasz Figa <t.figa@samsung.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> Acked-by: Felipe Balbi <balbi@ti.com>
>> Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> ---
>> .../devicetree/bindings/phy/phy-bindings.txt | 66 ++
>> Documentation/phy.txt | 166 +++++
>> MAINTAINERS | 8 +
>> drivers/Kconfig | 2 +
>> drivers/Makefile | 2 +
>> drivers/phy/Kconfig | 18 +
>> drivers/phy/Makefile | 5 +
>> drivers/phy/phy-core.c | 714 ++++++++++++++++++++
>> include/linux/phy/phy.h | 270 ++++++++
>> 9 files changed, 1251 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
>> create mode 100644 Documentation/phy.txt
>> create mode 100644 drivers/phy/Kconfig
>> create mode 100644 drivers/phy/Makefile
>> create mode 100644 drivers/phy/phy-core.c
>> create mode 100644 include/linux/phy/phy.h
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt b/Documentation/devicetree/bindings/phy/phy-bindings.txt
>> new file mode 100644
>> index 0000000..8ae844f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
>> @@ -0,0 +1,66 @@
>> +This document explains only the device tree data binding. For general
>> +information about PHY subsystem refer to Documentation/phy.txt
> [...]
>> @@ -0,0 +1,166 @@
>> + PHY SUBSYSTEM
>> + Kishon Vijay Abraham I <kishon@ti.com>
>> +
>> +This document explains the Generic PHY Framework along with the APIs provided,
>> +and how-to-use.
>> +
>> +1. Introduction
>> +
>> +*PHY* is the abbreviation for physical layer. It is used to connect a device
>> +to the physical medium e.g., the USB controller has a PHY to provide functions
>> +such as serialization, de-serialization, encoding, decoding and is responsible
>> +for obtaining the required data transmission rate. Note that some USB
>> +controllers have PHY functionality embedded into it and others use an external
>> +PHY. Other peripherals that use PHY include Wireless LAN, Ethernet,
>> +SATA etc.
>> +
>> +The intention of creating this framework is to bring the PHY drivers spread
>> +all over the Linux kernel to drivers/phy to increase code re-use and for
>> +better code maintainability.
>> +
>> +This framework will be of use only to devices that use external PHY (PHY
>> +functionality is not embedded within the controller).
>> +
>> +2. Registering/Unregistering the PHY provider
>> +
>> +PHY provider refers to an entity that implements one or more PHY instances.
>> +For the simple case where the PHY provider implements only a single instance of
>> +the PHY, the framework provides its own implementation of of_xlate in
>> +of_phy_simple_xlate. If the PHY provider implements multiple instances, it
>> +should provide its own implementation of of_xlate. of_xlate is used only for
>> +dt boot case.
>> +
>> +#define of_phy_provider_register(dev, xlate) \
>> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
>> +
>> +#define devm_of_phy_provider_register(dev, xlate) \
>> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
>
> This needs to be:
>
> __devm_of_phy_provider_register((dev), THIS_MODULE, (xlate))
>
> as Kamil pointed out. We've tested it here with v9 and it makes
> Bad Things happen. ;)
>
>> +of_phy_provider_register and devm_of_phy_provider_register macros can be used to
>> +register the phy_provider and it takes device and of_xlate as
>> +arguments. For the dt boot case, all PHY providers should use one of the above
>> +2 macros to register the PHY provider.
>> +
>> +void devm_of_phy_provider_unregister(struct device *dev,
>> + struct phy_provider *phy_provider);
>> +void of_phy_provider_unregister(struct phy_provider *phy_provider);
>> +
>> +devm_of_phy_provider_unregister and of_phy_provider_unregister can be used to
>> +unregister the PHY.
>> +
> [...]
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> new file mode 100644
>> index 0000000..f1d15e5
>> --- /dev/null
>> +++ b/drivers/phy/phy-core.c
>> @@ -0,0 +1,714 @@
> [...]
>> +static struct phy *phy_lookup(struct device *device, const char *port)
>> +{
>> + unsigned int count;
>> + struct phy *phy;
>> + struct device *dev;
>> + struct phy_consumer *consumers;
>> + struct class_dev_iter iter;
>
> Don't you need something like
>
> if (phy->init_data = NULL)
> return ERR_PTR(-EINVAL);
>
> to ensure there is no attempt to dereference NULL platform data ?
hmm.. perhaps a dev_WARN too..
>
>> + class_dev_iter_init(&iter, phy_class, NULL, NULL);
>> + while ((dev = class_dev_iter_next(&iter))) {
>> + phy = to_phy(dev);
>> + count = phy->init_data->num_consumers;
>> + consumers = phy->init_data->consumers;
>> + while (count--) {
>> + if (!strcmp(consumers->dev_name, dev_name(device)) &&
>> + !strcmp(consumers->port, port)) {
>> + class_dev_iter_exit(&iter);
>> + return phy;
>> + }
>> + consumers++;
>> + }
>> + }
>> +
>> + class_dev_iter_exit(&iter);
>> + return ERR_PTR(-ENODEV);
>> +}
>> +
> [...]
>> +int phy_init(struct phy *phy)
>> +{
>> + int ret;
>> +
>> + ret = phy_pm_runtime_get_sync(phy);
>> + if (ret < 0 && ret != -ENOTSUPP)
>> + return ret;
>> +
>> + mutex_lock(&phy->mutex);
>> + if (phy->init_count++ = 0 && phy->ops->init) {
>> + ret = phy->ops->init(phy);
>> + if (ret < 0) {
>> + dev_err(&phy->dev, "phy init failed --> %d\n", ret);
>> + goto out;
>
> Is this 'goto' and similar ones below really needed ?
That's just to signify an error path.. it doesn't affect anyways ;-)
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH v2 6/9] ARM: msm: Move mach/board.h contents to common.h
From: David Brown @ 2013-07-29 20:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1374699274-18388-7-git-send-email-sboyd@codeaurora.org>
On Wed, Jul 24, 2013 at 01:54:31PM -0700, Stephen Boyd wrote:
>The contents of mach/board.h are only used by files within
>mach-msm so there is no need to export this file outside of the
>mach-msm directory. Move the contents of the file to common.h to
>allow us to compile MSM in the multi-platform kernel.
>
>Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
>Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>---
> drivers/video/msm/msm_fb.c | 1 -
It'd be nice to get a framebuffer Ack from this part.
original patch:
https://patchwork.kernel.org/patch/2833029/
David
--
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* Re: [RESEND PATCH v10 1/8] drivers: phy: add generic PHY framework
From: Sylwester Nawrocki @ 2013-07-29 15:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1374842963-13545-2-git-send-email-kishon@ti.com>
On 07/26/2013 02:49 PM, Kishon Vijay Abraham I wrote:
> The PHY framework provides a set of APIs for the PHY drivers to
> create/destroy a PHY and APIs for the PHY users to obtain a reference to the
> PHY with or without using phandle. For dt-boot, the PHY drivers should
> also register *PHY provider* with the framework.
>
> PHY drivers should create the PHY by passing id and ops like init, exit,
> power_on and power_off. This framework is also pm runtime enabled.
>
> The documentation for the generic PHY framework is added in
> Documentation/phy.txt and the documentation for dt binding can be found at
> Documentation/devicetree/bindings/phy/phy-bindings.txt
>
> Cc: Tomasz Figa <t.figa@samsung.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Acked-by: Felipe Balbi <balbi@ti.com>
> Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> .../devicetree/bindings/phy/phy-bindings.txt | 66 ++
> Documentation/phy.txt | 166 +++++
> MAINTAINERS | 8 +
> drivers/Kconfig | 2 +
> drivers/Makefile | 2 +
> drivers/phy/Kconfig | 18 +
> drivers/phy/Makefile | 5 +
> drivers/phy/phy-core.c | 714 ++++++++++++++++++++
> include/linux/phy/phy.h | 270 ++++++++
> 9 files changed, 1251 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
> create mode 100644 Documentation/phy.txt
> create mode 100644 drivers/phy/Kconfig
> create mode 100644 drivers/phy/Makefile
> create mode 100644 drivers/phy/phy-core.c
> create mode 100644 include/linux/phy/phy.h
>
> diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt b/Documentation/devicetree/bindings/phy/phy-bindings.txt
> new file mode 100644
> index 0000000..8ae844f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
> @@ -0,0 +1,66 @@
> +This document explains only the device tree data binding. For general
> +information about PHY subsystem refer to Documentation/phy.txt
[...]
> @@ -0,0 +1,166 @@
> + PHY SUBSYSTEM
> + Kishon Vijay Abraham I <kishon@ti.com>
> +
> +This document explains the Generic PHY Framework along with the APIs provided,
> +and how-to-use.
> +
> +1. Introduction
> +
> +*PHY* is the abbreviation for physical layer. It is used to connect a device
> +to the physical medium e.g., the USB controller has a PHY to provide functions
> +such as serialization, de-serialization, encoding, decoding and is responsible
> +for obtaining the required data transmission rate. Note that some USB
> +controllers have PHY functionality embedded into it and others use an external
> +PHY. Other peripherals that use PHY include Wireless LAN, Ethernet,
> +SATA etc.
> +
> +The intention of creating this framework is to bring the PHY drivers spread
> +all over the Linux kernel to drivers/phy to increase code re-use and for
> +better code maintainability.
> +
> +This framework will be of use only to devices that use external PHY (PHY
> +functionality is not embedded within the controller).
> +
> +2. Registering/Unregistering the PHY provider
> +
> +PHY provider refers to an entity that implements one or more PHY instances.
> +For the simple case where the PHY provider implements only a single instance of
> +the PHY, the framework provides its own implementation of of_xlate in
> +of_phy_simple_xlate. If the PHY provider implements multiple instances, it
> +should provide its own implementation of of_xlate. of_xlate is used only for
> +dt boot case.
> +
> +#define of_phy_provider_register(dev, xlate) \
> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
> +
> +#define devm_of_phy_provider_register(dev, xlate) \
> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
This needs to be:
__devm_of_phy_provider_register((dev), THIS_MODULE, (xlate))
as Kamil pointed out. We've tested it here with v9 and it makes
Bad Things happen. ;)
> +of_phy_provider_register and devm_of_phy_provider_register macros can be used to
> +register the phy_provider and it takes device and of_xlate as
> +arguments. For the dt boot case, all PHY providers should use one of the above
> +2 macros to register the PHY provider.
> +
> +void devm_of_phy_provider_unregister(struct device *dev,
> + struct phy_provider *phy_provider);
> +void of_phy_provider_unregister(struct phy_provider *phy_provider);
> +
> +devm_of_phy_provider_unregister and of_phy_provider_unregister can be used to
> +unregister the PHY.
> +
[...]
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> new file mode 100644
> index 0000000..f1d15e5
> --- /dev/null
> +++ b/drivers/phy/phy-core.c
> @@ -0,0 +1,714 @@
[...]
> +static struct phy *phy_lookup(struct device *device, const char *port)
> +{
> + unsigned int count;
> + struct phy *phy;
> + struct device *dev;
> + struct phy_consumer *consumers;
> + struct class_dev_iter iter;
Don't you need something like
if (phy->init_data = NULL)
return ERR_PTR(-EINVAL);
to ensure there is no attempt to dereference NULL platform data ?
> + class_dev_iter_init(&iter, phy_class, NULL, NULL);
> + while ((dev = class_dev_iter_next(&iter))) {
> + phy = to_phy(dev);
> + count = phy->init_data->num_consumers;
> + consumers = phy->init_data->consumers;
> + while (count--) {
> + if (!strcmp(consumers->dev_name, dev_name(device)) &&
> + !strcmp(consumers->port, port)) {
> + class_dev_iter_exit(&iter);
> + return phy;
> + }
> + consumers++;
> + }
> + }
> +
> + class_dev_iter_exit(&iter);
> + return ERR_PTR(-ENODEV);
> +}
> +
[...]
> +int phy_init(struct phy *phy)
> +{
> + int ret;
> +
> + ret = phy_pm_runtime_get_sync(phy);
> + if (ret < 0 && ret != -ENOTSUPP)
> + return ret;
> +
> + mutex_lock(&phy->mutex);
> + if (phy->init_count++ = 0 && phy->ops->init) {
> + ret = phy->ops->init(phy);
> + if (ret < 0) {
> + dev_err(&phy->dev, "phy init failed --> %d\n", ret);
> + goto out;
Is this 'goto' and similar ones below really needed ?
> + }
> + }
> +
> +out:
> + mutex_unlock(&phy->mutex);
> + phy_pm_runtime_put(phy);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_init);
> +
> +int phy_exit(struct phy *phy)
> +{
> + int ret;
> +
> + ret = phy_pm_runtime_get_sync(phy);
> + if (ret < 0 && ret != -ENOTSUPP)
> + return ret;
> +
> + mutex_lock(&phy->mutex);
> + if (--phy->init_count = 0 && phy->ops->exit) {
> + ret = phy->ops->exit(phy);
> + if (ret < 0) {
> + dev_err(&phy->dev, "phy exit failed --> %d\n", ret);
> + goto out;
> + }
> + }
> +
> +out:
> + mutex_unlock(&phy->mutex);
> + phy_pm_runtime_put(phy);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_exit);
> +
> +int phy_power_on(struct phy *phy)
> +{
> + int ret = -ENOTSUPP;
> +
> + ret = phy_pm_runtime_get_sync(phy);
> + if (ret < 0 && ret != -ENOTSUPP)
> + return ret;
> +
> + mutex_lock(&phy->mutex);
> + if (phy->power_count++ = 0 && phy->ops->power_on) {
> + ret = phy->ops->power_on(phy);
> + if (ret < 0) {
> + dev_err(&phy->dev, "phy poweron failed --> %d\n", ret);
> + goto out;
> + }
> + }
> +
> +out:
> + mutex_unlock(&phy->mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_power_on);
> +
> +int phy_power_off(struct phy *phy)
> +{
> + int ret = -ENOTSUPP;
> +
> + mutex_lock(&phy->mutex);
> + if (--phy->power_count = 0 && phy->ops->power_off) {
> + ret = phy->ops->power_off(phy);
> + if (ret < 0) {
> + dev_err(&phy->dev, "phy poweroff failed --> %d\n", ret);
> + goto out;
> + }
> + }
> +
> +out:
> + mutex_unlock(&phy->mutex);
> + phy_pm_runtime_put(phy);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_power_off);
--
Thanks,
Sylwester
^ permalink raw reply
* Re: [RESEND PATCH v10 1/8] drivers: phy: add generic PHY framework
From: Kishon Vijay Abraham I @ 2013-07-29 15:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <023801ce8c70$427cbd20$c7763760$%debski@samsung.com>
On Monday 29 July 2013 08:58 PM, Kamil Debski wrote:
> Hi Kishon,
>
> A small fix follows inline.
>
>> From: linux-media-owner@vger.kernel.org [mailto:linux-media-
>> owner@vger.kernel.org] On Behalf Of Kishon Vijay Abraham I
>> Sent: Friday, July 26, 2013 2:49 PM
>>
>> The PHY framework provides a set of APIs for the PHY drivers to
>> create/destroy a PHY and APIs for the PHY users to obtain a reference
>> to the PHY with or without using phandle. For dt-boot, the PHY drivers
>> should also register *PHY provider* with the framework.
>>
>> PHY drivers should create the PHY by passing id and ops like init, exit,
>> power_on and power_off. This framework is also pm runtime enabled.
>>
>> The documentation for the generic PHY framework is added in
>> Documentation/phy.txt and the documentation for dt binding can be found
>> at Documentation/devicetree/bindings/phy/phy-bindings.txt
>>
>> Cc: Tomasz Figa <t.figa@samsung.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> Acked-by: Felipe Balbi <balbi@ti.com>
>> Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> ---
>> .../devicetree/bindings/phy/phy-bindings.txt | 66 ++
>> Documentation/phy.txt | 166 +++++
>> MAINTAINERS | 8 +
>> drivers/Kconfig | 2 +
>> drivers/Makefile | 2 +
>> drivers/phy/Kconfig | 18 +
>> drivers/phy/Makefile | 5 +
>> drivers/phy/phy-core.c | 714
>> ++++++++++++++++++++
>> include/linux/phy/phy.h | 270 ++++++++
>> 9 files changed, 1251 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/phy/phy-
>> bindings.txt
>> create mode 100644 Documentation/phy.txt create mode 100644
>> drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create
>> mode 100644 drivers/phy/phy-core.c create mode 100644
>> include/linux/phy/phy.h
>>
>
> [snip]
>
>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h new file
>> mode 100644 index 0000000..e444b23
>> --- /dev/null
>> +++ b/include/linux/phy/phy.h
>> @@ -0,0 +1,270 @@
>
> [snip]
>
>> +struct phy_init_data {
>> + unsigned int num_consumers;
>> + struct phy_consumer *consumers;
>> +};
>> +
>> +#define PHY_CONSUMER(_dev_name, _port) \
>> +{ \
>> + .dev_name = _dev_name, \
>> + .port = _port, \
>> +}
>> +
>> +#define to_phy(dev) (container_of((dev), struct phy, dev))
>> +
>> +#define of_phy_provider_register(dev, xlate) \
>> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
>> +
>> +#define devm_of_phy_provider_register(dev, xlate) \
>> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
>
> I think this should be:
> + __devm_of_phy_provider_register((dev), THIS_MODULE, (xlate))
> Right?
right.. thanks for spotting this.
Regards
Kishon
^ permalink raw reply
* RE: [RESEND PATCH v10 1/8] drivers: phy: add generic PHY framework
From: Kamil Debski @ 2013-07-29 15:28 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1374842963-13545-2-git-send-email-kishon@ti.com>
Hi Kishon,
A small fix follows inline.
> From: linux-media-owner@vger.kernel.org [mailto:linux-media-
> owner@vger.kernel.org] On Behalf Of Kishon Vijay Abraham I
> Sent: Friday, July 26, 2013 2:49 PM
>
> The PHY framework provides a set of APIs for the PHY drivers to
> create/destroy a PHY and APIs for the PHY users to obtain a reference
> to the PHY with or without using phandle. For dt-boot, the PHY drivers
> should also register *PHY provider* with the framework.
>
> PHY drivers should create the PHY by passing id and ops like init, exit,
> power_on and power_off. This framework is also pm runtime enabled.
>
> The documentation for the generic PHY framework is added in
> Documentation/phy.txt and the documentation for dt binding can be found
> at Documentation/devicetree/bindings/phy/phy-bindings.txt
>
> Cc: Tomasz Figa <t.figa@samsung.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Acked-by: Felipe Balbi <balbi@ti.com>
> Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> .../devicetree/bindings/phy/phy-bindings.txt | 66 ++
> Documentation/phy.txt | 166 +++++
> MAINTAINERS | 8 +
> drivers/Kconfig | 2 +
> drivers/Makefile | 2 +
> drivers/phy/Kconfig | 18 +
> drivers/phy/Makefile | 5 +
> drivers/phy/phy-core.c | 714
> ++++++++++++++++++++
> include/linux/phy/phy.h | 270 ++++++++
> 9 files changed, 1251 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/phy-
> bindings.txt
> create mode 100644 Documentation/phy.txt create mode 100644
> drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create
> mode 100644 drivers/phy/phy-core.c create mode 100644
> include/linux/phy/phy.h
>
[snip]
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h new file
> mode 100644 index 0000000..e444b23
> --- /dev/null
> +++ b/include/linux/phy/phy.h
> @@ -0,0 +1,270 @@
[snip]
> +struct phy_init_data {
> + unsigned int num_consumers;
> + struct phy_consumer *consumers;
> +};
> +
> +#define PHY_CONSUMER(_dev_name, _port) \
> +{ \
> + .dev_name = _dev_name, \
> + .port = _port, \
> +}
> +
> +#define to_phy(dev) (container_of((dev), struct phy, dev))
> +
> +#define of_phy_provider_register(dev, xlate) \
> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
> +
> +#define devm_of_phy_provider_register(dev, xlate) \
> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
I think this should be:
+ __devm_of_phy_provider_register((dev), THIS_MODULE, (xlate))
Right?
> +
> +static inline void phy_set_drvdata(struct phy *phy, void *data) {
> + dev_set_drvdata(&phy->dev, data);
> +}
> +
> +static inline void *phy_get_drvdata(struct phy *phy) {
> + return dev_get_drvdata(&phy->dev);
> +}
> +
[snip]
Best wishes,
--
Kamil Debski
Linux Kernel Developer
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [RFC 0/1] drm/pl111: Initial drm/kms driver for pl111
From: Rob Clark @ 2013-07-29 14:58 UTC (permalink / raw)
To: Tom Cooksey; +Cc: linux-arm-kernel, linux-fbdev, Pawel Moll, dri-devel
In-Reply-To: <51f29ccd.f014b40a.34cc.ffffca2aSMTPIN_ADDED_BROKEN@mx.google.com>
On Fri, Jul 26, 2013 at 11:58 AM, Tom Cooksey <tom.cooksey@arm.com> wrote:
> Hi Rob,
>
>> > * It abuses flags parameter of DRM_IOCTL_MODE_CREATE_DUMB to also
>> > allocate buffers for the GPU. Still not sure how to resolve this
>> > as we don't use DRM for our GPU driver.
>>
>> any thoughts/plans about a DRM GPU driver? Ideally long term (esp.
>> once the dma-fence stuff is in place), we'd have gpu-specific drm
>> (gpu-only, no kms) driver, and SoC/display specific drm/kms driver,
>> using prime/dmabuf to share between the two.
>
> The "extra" buffers we were allocating from armsoc DDX were really
> being allocated through DRM/GEM so we could get an flink name
> for them and pass a reference to them back to our GPU driver on
> the client side. If it weren't for our need to access those
> extra off-screen buffers with the GPU we wouldn't need to
> allocate them with DRM at all. So, given they are really "GPU"
> buffers, it does absolutely make sense to allocate them in a
> different driver to the display driver.
>
> However, to avoid unnecessary memcpys & related cache
> maintenance ops, we'd also like the GPU to render into buffers
> which are scanned out by the display controller. So let's say
> we continue using DRM_IOCTL_MODE_CREATE_DUMB to allocate scan
> out buffers with the display's DRM driver but a custom ioctl
> on the GPU's DRM driver to allocate non scanout, off-screen
> buffers. Sounds great, but I don't think that really works
> with DRI2. If we used two drivers to allocate buffers, which
> of those drivers do we return in DRI2ConnectReply? Even if we
> solve that somehow, GEM flink names are name-spaced to a
> single device node (AFAIK). So when we do a DRI2GetBuffers,
> how does the EGL in the client know which DRM device owns GEM
> flink name "1234"? We'd need some pretty dirty hacks.
You would return the name of the display driver allocating the
buffers. On the client side you can use generic ioctls to go from
flink -> handle -> dmabuf. So the client side would end up opening
both the display drm device and the gpu, but without needing to know
too much about the display.
(Probably in (for example) mesa there needs to be a bit of work to
handle this better, but I think that would be needed as well for
sharing between, say, nouveau and udl displaylink driver.. which is
really the same scenario.)
> So then we looked at allocating _all_ buffers with the GPU's
> DRM driver. That solves the DRI2 single-device-name and single
> name-space issue. It also means the GPU would _never_ render
> into buffers allocated through DRM_IOCTL_MODE_CREATE_DUMB.
Well, I think we can differentiate between shared buffers and internal
buffers (textures, vertex upload, etc, etc).
For example, in mesa/gallium driver there are two paths to getting a
buffer... ->resource_create() and ->resource_from_handle(), the
latter is the path you go for dri2 buffers shared w/ x11. The former
is buffers that are just internal to gpu (if !(bind &
PIPE_BIND_SHARED)). I guess you must have something similar in mali
driver.
> One thing I wasn't sure about is if there was an objection
> to using PRIME to export scanout buffers allocated with
> DRM_IOCTL_MODE_CREATE_DUMB and then importing them into a GPU
> driver to be rendered into? Is that a concern?
well.. it wasn't quite the original intention of the 'dumb' ioctls.
But I guess in the end if you hand the gpu a buffer with a
layout/format that it can't grok, then gpu does a staging texture plus
copy. If you had, for example, some setup w/ gpu that could only
render to tiled format, plus a display that could scanout that format,
then your DDX driver would need to allocate the dri2 buffers with
something other than dumb ioctl. (But at that point, you probably
need to implement your own EXA so you could hook in your own custom
upload-to/download-from screen fxns for sw fallbacks, so you're not
talking about using generic DDX anyways.)
> Anyway, that latter case also gets quite difficult. The "GPU"
> DRM driver would need to know the constraints of the display
> controller when allocating buffers intended to be scanned out.
> For example, pl111 typically isn't behind an IOMMU and so
> requires physically contiguous memory. We'd have to teach the
> GPU's DRM driver about the constraints of the display HW. Not
> exactly a clean driver model. :-(
>
> I'm still a little stuck on how to proceed, so any ideas
> would greatly appreciated! My current train of thought is
> having a kind of SoC-specific DRM driver which allocates
> buffers for both display and GPU within a single GEM
> namespace. That SoC-specific DRM driver could then know the
> constraints of both the GPU and the display HW. We could then
> use PRIME to export buffers allocated with the SoC DRM driver
> and import them into the GPU and/or display DRM driver.
Usually if the display drm driver is allocating the buffers that might
be scanned out, it just needs to have minimal knowledge of the GPU
(pitch alignment constraints). I don't think we need a 3rd device
just to allocate buffers.
Really I don't think the separate display drm and gpu drm device is
much different from desktop udl/displaylink case. Or desktop
integrated-gpu + external gpu.
> Note: While it doesn't use the DRM framework, the Mali T6xx
> kernel driver has supported importing buffers through dma_buf
> for some time. I've even written an EGL extension :-):
>
> <http://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_im
> port.txt>
>
>
>> I'm not entirely sure that the directions that the current CDF
>> proposals are headed is necessarily the right way forward. I'd prefer
>> to see small/incremental evolution of KMS (ie. add drm_bridge and
>> drm_panel, and refactor the existing encoder-slave). Keeping it
>> inside drm means that we can evolve it more easily, and avoid layers
>> of glue code for no good reason.
>
> I think CDF could allow vendors to re-use code they've written
> for their Android driver stack in DRM drivers more easily. Though
> I guess ideally KMS would evolve to a point where it could be used
> by an Android driver stack. I.e. Support explicit fences.
>
yeah, the best would be evolving DRM/KMS to the point that it meets
android requirements. Because I really don't think android has any
special requirements compared to, say, a wayland compositor. (Other
than the way they do synchronization. But that doesn't really *need*
to be different, as far as I can tell.) It would certainly be easier
if android dev's actually participated in upstream linux graphics, and
talked about what they needed, and maybe even contributed a patch or
two. But that is a whole different topic.
BR,
-R
>
> Cheers,
>
> Tom
>
>
>
>
>
^ permalink raw reply
* Re: [PATCH 4/5] fb: Add DCU framebuffer driver for Vybrid VF610 platform
From: Sascha Hauer @ 2013-07-29 11:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1373609276-14566-5-git-send-email-b18965@freescale.com>
On Fri, Jul 12, 2013 at 02:07:55PM +0800, Alison Wang wrote:
> The Display Controller Unit (DCU) module is a system master that
> fetches graphics stored in internal or external memory and displays
> them on a TFT LCD panel. A wide range of panel sizes is supported
> and the timing of the interface signals is highly configurable.
> Graphics are read directly from memory and then blended in real-time,
> which allows for dynamic content creation with minimal CPU intervention.
Only a review of the code inline.
Maybe the real question is whether we want to introduce another
framebuffer driver at all instead of making it a DRM driver.
> +
> +#define DRIVER_NAME "fsl-dcu-fb"
> +
> +#define DCU_DCU_MODE 0x0010
> +#define DCU_MODE_BLEND_ITER(x) (x << 20)
What's the result of DCU_MODE_BLEND_ITER(1 + 1)?
> +static struct fb_videomode dcu_mode_db[] = {
> + {
> + .name = "480x272",
> + .refresh = 75,
> + .xres = 480,
> + .yres = 272,
> + .pixclock = 91996,
> + .left_margin = 2,
> + .right_margin = 2,
> + .upper_margin = 1,
> + .lower_margin = 1,
> + .hsync_len = 41,
> + .vsync_len = 2,
> + .sync = FB_SYNC_COMP_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
> + .vmode = FB_VMODE_NONINTERLACED,
> + },
> +};
We have ways to describe a panel in dt. Use them.
> +
> +static struct mfb_info mfb_template[] = {
> + {
> + .index = LAYER0,
> + .id = "Layer0",
> + .alpha = 0xff,
> + .blend = 0,
> + .count = 0,
> + .x_layer_d = 0,
> + .y_layer_d = 0,
> + },
Wrong indentation.
> + default:
> + printk(KERN_ERR "unsupported color depth: %u\n",
> + var->bits_per_pixel);
Use dev_* for printing messages in drivers.
> +static int fsl_dcu_check_var(struct fb_var_screeninfo *var,
> + struct fb_info *info)
> +{
> + if (var->xres_virtual < var->xres)
> + var->xres_virtual = var->xres;
> + if (var->yres_virtual < var->yres)
> + var->yres_virtual = var->yres;
> +
> + if (var->xoffset < 0)
> + var->xoffset = 0;
> +
> + if (var->yoffset < 0)
> + var->yoffset = 0;
Ever seen an unsigned value going below zero?
> + default:
> + printk(KERN_ERR "unsupported color depth: %u\n",
> + var->bits_per_pixel);
BUG(). This can't happen since you make that sure above.
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int calc_div_ratio(struct fb_info *info)
> +{
Use a consistent namespace for function names (fsl_dcu_)
> + writel(DCU_THRESHOLD_LS_BF_VS(0x3) | DCU_THRESHOLD_OUT_BUF_HIGH(0x78) |
> + DCU_THRESHOLD_OUT_BUF_LOW(0), dcufb->reg_base + DCU_THRESHOLD);
> +
> + enable_controller(info);
> +}
Make your functions symmetric. If there's update_controller(), the
function should do exactly that, it should *not* enable the controller.
Call this from the users of this function if necessary.
> + if (copy_to_user(buf, &alpha, sizeof(alpha)))
> + return -EFAULT;
> + break;
> + case MFB_SET_ALPHA:
> + if (copy_from_user(&alpha, buf, sizeof(alpha)))
> + return -EFAULT;
> + mfbi->blend = 1;
> + mfbi->alpha = alpha;
> + fsl_dcu_set_par(info);
> + break;
Is it still state of the art to introduce ioctls in the fb framework
without any kind of documentation?
> + default:
> + printk(KERN_ERR "unknown ioctl command (0x%08X)\n", cmd);
What shall a reader of the kernel log do with a message like this? It's
utterly useless when he doesn't even now which device failed here. Just
drop this.
> +static void enable_interrupts(struct dcu_fb_data *dcufb)
> +{
> + u32 int_mask = readl(dcufb->reg_base + DCU_INT_MASK);
> +
> + writel(int_mask & ~DCU_INT_MASK_UNDRUN, dcufb->reg_base + DCU_INT_MASK);
> +}
Inline this code where you need it. Introducing a function for a single
register write seems quite useless.
> +static int install_framebuffer(struct fb_info *info)
> +{
> + struct mfb_info *mfbi = info->par;
> + struct fb_videomode *db = dcu_mode_db;
> + unsigned int dbsize = ARRAY_SIZE(dcu_mode_db);
> + int ret;
> +
> + info->var.activate = FB_ACTIVATE_NOW;
> + info->fbops = &fsl_dcu_ops;
> + info->flags = FBINFO_FLAG_DEFAULT;
> + info->pseudo_palette = &mfbi->pseudo_palette;
> +
> + fb_alloc_cmap(&info->cmap, 16, 0);
> +
> + ret = fb_find_mode(&info->var, info, fb_mode, db, dbsize,
> + NULL, default_bpp);
> +
> + if (fsl_dcu_check_var(&info->var, info)) {
> + ret = -EINVAL;
Propagate the error.
> + goto failed_checkvar;
> + }
> +
> + if (register_framebuffer(info) < 0) {
> + ret = -EINVAL;
ditto
> +static irqreturn_t fsl_dcu_irq(int irq, void *dev_id)
> +{
> + struct dcu_fb_data *dcufb = dev_id;
> + unsigned int status = readl(dcufb->reg_base + DCU_INT_STATUS);
> + u32 dcu_mode;
> +
> + if (status) {
Save indendation level by bailing out early.
> + if (status & DCU_INT_STATUS_UNDRUN) {
> + dcu_mode = readl(dcufb->reg_base + DCU_DCU_MODE);
> + dcu_mode &= ~DCU_MODE_DCU_MODE_MASK;
> + writel(dcu_mode | DCU_MODE_DCU_MODE(DCU_MODE_OFF),
> + dcufb->reg_base + DCU_DCU_MODE);
> + udelay(1);
> + writel(dcu_mode | DCU_MODE_DCU_MODE(DCU_MODE_NORMAL),
> + dcufb->reg_base + DCU_DCU_MODE);
> + }
> + writel(status, dcufb->reg_base + DCU_INT_STATUS);
> + return IRQ_HANDLED;
> + }
> + return IRQ_NONE;
> +}
> +
> + if (IS_ERR(tcon_clk)) {
> + ret = PTR_ERR(tcon_clk);
> + goto failed_getclock;
> + }
> + clk_prepare_enable(tcon_clk);
> +
> + tcon_reg = of_iomap(tcon_np, 0);
Use devm_*
> + dcufb->irq = platform_get_irq(pdev, 0);
> + if (!dcufb->irq) {
> + ret = -EINVAL;
> + goto failed_getirq;
> + }
> +
> + ret = request_irq(dcufb->irq, fsl_dcu_irq, 0, DRIVER_NAME, dcufb);
Use devm_request_irq
> + if (ret) {
> + dev_err(&pdev->dev, "could not request irq\n");
> + goto failed_requestirq;
> + }
> +
> + /* Put TCON in bypass mode, so the input signals from DCU are passed
> + * through TCON unchanged */
> + ret = bypass_tcon(pdev->dev.of_node);
> + if (ret) {
> + dev_err(&pdev->dev, "could not bypass TCON\n");
> + goto failed_bypasstcon;
> + }
> +
> + dcufb->clk = devm_clk_get(&pdev->dev, "dcu");
> + if (IS_ERR(dcufb->clk)) {
> + dev_err(&pdev->dev, "could not get clock\n");
> + goto failed_getclock;
You will return 0 here.
> + }
> + clk_prepare_enable(dcufb->clk);
> +
> + for (i = 0; i < ARRAY_SIZE(dcufb->fsl_dcu_info); i++) {
> + dcufb->fsl_dcu_info[i] > + framebuffer_alloc(sizeof(struct mfb_info), &pdev->dev);
> + if (!dcufb->fsl_dcu_info[i]) {
> + ret = ENOMEM;
-ENOMEM
> +failed_alloc_framebuffer:
> +failed_getclock:
> +failed_bypasstcon:
> + free_irq(dcufb->irq, dcufb);
> +failed_requestirq:
> +failed_getirq:
> + iounmap(dcufb->reg_base);
You used devm_ioremap, so drop this.
> +failed_ioremap:
> + kfree(dcufb);
This is allocated with devm_*. Drop this.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* RE: [PATCH v2 0/5] ARM: vf610: Add DCU framebuffer driver for Vybrid VF610 platform
From: Wang Huan-B18965 @ 2013-07-29 6:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <81BA6E5E0BC2344391CABCEE22D1B6D83AA3AE@039-SN1MPN1-004.039d.mgd.msft.net>
Hi, Jean-Christophe, Tomi,
Do you have any comments for these patches?
Thanks!
Best Regards,
Alison Wang
> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-
> bounces@lists.infradead.org] On Behalf Of Wang Huan-B18965
> Sent: Friday, July 19, 2013 11:49 AM
> To: plagnioj@jcrosoft.com; tomi.valkeinen@ti.com; shawn.guo@linaro.org;
> Estevam Fabio-R49496; linux-fbdev@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Cc: Jin Zhengxiong-R64188
> Subject: RE: [PATCH v2 0/5] ARM: vf610: Add DCU framebuffer driver for
> Vybrid VF610 platform
>
> Hi, Jean-Christophe,
>
> Could you please help to review these patches?
>
> Thanks a lot!
>
>
> Best Regards,
> Alison Wang
>
> > -----Original Message-----
> > From: linux-arm-kernel [mailto:linux-arm-kernel-
> > bounces@lists.infradead.org] On Behalf Of Alison Wang
> > Sent: Friday, July 12, 2013 2:08 PM
> > To: plagnioj@jcrosoft.com; tomi.valkeinen@ti.com;
> > shawn.guo@linaro.org; Estevam Fabio-R49496;
> > linux-fbdev@vger.kernel.org; linux-arm- kernel@lists.infradead.org
> > Cc: Jin Zhengxiong-R64188
> > Subject: [PATCH v2 0/5] ARM: vf610: Add DCU framebuffer driver for
> > Vybrid VF610 platform
> >
> > This series contain DCU framebuffer driver for Freescale Vybrid VF610
> > platform.
> >
> > The Display Controller Unit (DCU) module is a system master that
> > fetches graphics stored in internal or external memory and displays
> > them on a TFT LCD panel. A wide range of panel sizes is supported and
> > the timing of the interface signals is highly configurable.
> > Graphics are read directly from memory and then blended in real-time,
> > which allows for dynamic content creation with minimal CPU
> intervention.
> >
> > The features:
> >
> > (1) Full RGB888 output to TFT LCD panel.
> > (2) For the current LCD panel, WQVGA "480x272" is tested.
> > (3) Blending of each pixel using up to 4 source layers dependent on
> > size of panel.
> > (4) Each graphic layer can be placed with one pixel resolution in
> > either axis.
> > (5) Each graphic layer support RGB565 and RGB888 direct colors
> without
> > alpha channel and BGRA8888 direct colors with an alpha channel.
> > (6) Each graphic layer support alpha blending with 8-bit resolution.
> >
> > Changes in v2:
> > - Add a document for DCU framebuffer driver under
> > Documentation/devicetree/bindings/fb/.
> >
> > ----------------------------------------------------------------
> > Alison Wang (5):
> > ARM: dts: vf610: Add DCU and TCON nodes
> > ARM: dts: vf610-twr: Enable DCU and TCON devices
> > ARM: clk: vf610: Add DCU and TCON clock support
> > fb: Add DCU framebuffer driver for Vybrid VF610 platform
> > Documentation: DT: Add DCU framebuffer driver
> >
> > Documentation/devicetree/bindings/fb/fsl-dcu-fb.txt | 36 ++++
> > arch/arm/boot/dts/vf610-twr.dts | 10 +
> > arch/arm/boot/dts/vf610.dtsi | 19 +-
> > arch/arm/mach-imx/clk-vf610.c | 5 +
> > drivers/video/Kconfig | 9 +
> > drivers/video/Makefile | 1 +
> > drivers/video/fsl-dcu-fb.c | 1091
> >
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > +++++++++++++++++++++++++
> > include/dt-bindings/clock/vf610-clock.h | 3 +-
> > 8 files changed, 1172 insertions(+), 2 deletions(-) create mode
> > 100644 Documentation/devicetree/bindings/fb/fsl-dcu-fb.txt
> > create mode 100644 drivers/video/fsl-dcu-fb.c
> >
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] video: remove unused variable dev
From: Kefeng Wang @ 2013-07-29 1:27 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Tomi Valkeinen',
'Jean-Christophe Plagniol-Villard', linux-fbdev,
linux-kernel, guohanjun, 'Luis Henriques'
In-Reply-To: <009601ce8bf5$e88bfda0$b9a3f8e0$@samsung.com>
On 07/29 8:52, Jingoo Han wrote:
> On Saturday, July 27, 2013 7:05 PM, Kefeng Wang wrote:
>>
>> Due to commit: e21d2170f [video: remove unnecessary platform_set_drvdata()],
>> variable dev is unused, so remove it.
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>> drivers/video/vga16fb.c | 1 -
>> 1 file changed, 1 deletion(-)
>
> CC'ed Luis Henriques.
>
>
> Hi Kefeng Wang,
>
> The same patch was submitted by Luis Henriques 2 weeks ago.
> Also, the patch was already applied by Tomi Valkeinen.
>
> Best regards,
> Jingoo Han
>
Thanks for your reply, I got it, so ignore it.
>>
>> diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
>> index 830ded4..2827333 100644
>> --- a/drivers/video/vga16fb.c
>> +++ b/drivers/video/vga16fb.c
>> @@ -1265,7 +1265,6 @@ static void vga16fb_imageblit(struct fb_info *info, const struct fb_image *image
>>
>> static void vga16fb_destroy(struct fb_info *info)
>> {
>> - struct platform_device *dev = container_of(info->device, struct platform_device, dev);
>> iounmap(info->screen_base);
>> fb_dealloc_cmap(&info->cmap);
>> /* XXX unshare VGA regions */
>> --
>> 1.8.2.2
>
>
>
>
^ permalink raw reply
* Re: [PATCH] video: remove unused variable dev
From: Jingoo Han @ 2013-07-29 0:52 UTC (permalink / raw)
To: 'Kefeng Wang', 'Tomi Valkeinen'
Cc: 'Jean-Christophe Plagniol-Villard', linux-fbdev,
linux-kernel, guohanjun, 'Luis Henriques', Jingoo Han
In-Reply-To: <1374919513-15356-1-git-send-email-wangkefeng.wang@huawei.com>
On Saturday, July 27, 2013 7:05 PM, Kefeng Wang wrote:
>
> Due to commit: e21d2170f [video: remove unnecessary platform_set_drvdata()],
> variable dev is unused, so remove it.
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> drivers/video/vga16fb.c | 1 -
> 1 file changed, 1 deletion(-)
CC'ed Luis Henriques.
Hi Kefeng Wang,
The same patch was submitted by Luis Henriques 2 weeks ago.
Also, the patch was already applied by Tomi Valkeinen.
Best regards,
Jingoo Han
>
> diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
> index 830ded4..2827333 100644
> --- a/drivers/video/vga16fb.c
> +++ b/drivers/video/vga16fb.c
> @@ -1265,7 +1265,6 @@ static void vga16fb_imageblit(struct fb_info *info, const struct fb_image *image
>
> static void vga16fb_destroy(struct fb_info *info)
> {
> - struct platform_device *dev = container_of(info->device, struct platform_device, dev);
> iounmap(info->screen_base);
> fb_dealloc_cmap(&info->cmap);
> /* XXX unshare VGA regions */
> --
> 1.8.2.2
^ permalink raw reply
* [PATCH] video: remove unused variable dev
From: Kefeng Wang @ 2013-07-27 10:05 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel
Cc: Jingoo Han, guohanjun
Due to commit: e21d2170f [video: remove unnecessary platform_set_drvdata()],
variable dev is unused, so remove it.
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
drivers/video/vga16fb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
index 830ded4..2827333 100644
--- a/drivers/video/vga16fb.c
+++ b/drivers/video/vga16fb.c
@@ -1265,7 +1265,6 @@ static void vga16fb_imageblit(struct fb_info *info, const struct fb_image *image
static void vga16fb_destroy(struct fb_info *info)
{
- struct platform_device *dev = container_of(info->device, struct platform_device, dev);
iounmap(info->screen_base);
fb_dealloc_cmap(&info->cmap);
/* XXX unshare VGA regions */
--
1.8.2.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox