* [PATCH] video: exynos_dp: use usleep_range instead of delay
From: Jingoo Han @ 2012-07-18 9:50 UTC (permalink / raw)
To: linux-fbdev
This patch replaces udelay and mdelay with usleep_range to remove
the busy loop waiting.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 14 +++++++-------
drivers/video/exynos/exynos_dp_reg.c | 4 ++--
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 9db7b9f..25907f4 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -47,7 +47,7 @@ static int exynos_dp_detect_hpd(struct exynos_dp_device *dp)
exynos_dp_init_hpd(dp);
- udelay(200);
+ usleep_range(200, 210);
while (exynos_dp_get_plug_in_status(dp) != 0) {
timeout_loop++;
@@ -55,7 +55,7 @@ static int exynos_dp_detect_hpd(struct exynos_dp_device *dp)
dev_err(dp->dev, "failed to get hpd plug status\n");
return -ETIMEDOUT;
}
- udelay(10);
+ usleep_range(10, 11);
}
return 0;
@@ -486,7 +486,7 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
u8 pre_emphasis;
u8 training_lane;
- udelay(100);
+ usleep_range(100, 101);
exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
6, link_status);
@@ -571,7 +571,7 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
u8 adjust_request[2];
- udelay(400);
+ usleep_range(400, 401);
exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
6, link_status);
@@ -739,7 +739,7 @@ static int exynos_dp_set_link_train(struct exynos_dp_device *dp,
if (retval = 0)
break;
- udelay(100);
+ usleep_range(100, 110);
}
return retval;
@@ -773,7 +773,7 @@ static int exynos_dp_config_video(struct exynos_dp_device *dp,
return -ETIMEDOUT;
}
- udelay(1);
+ usleep_range(1, 2);
}
/* Set to use the register calculated M/N video */
@@ -807,7 +807,7 @@ static int exynos_dp_config_video(struct exynos_dp_device *dp,
return -ETIMEDOUT;
}
- mdelay(1);
+ usleep_range(1000, 1001);
}
if (retval != 0)
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 6ce76d5..ce401c8 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -122,7 +122,7 @@ void exynos_dp_reset(struct exynos_dp_device *dp)
LS_CLK_DOMAIN_FUNC_EN_N;
writel(reg, dp->reg_base + EXYNOS_DP_FUNC_EN_2);
- udelay(20);
+ usleep_range(20, 30);
exynos_dp_lane_swap(dp, 0);
@@ -988,7 +988,7 @@ void exynos_dp_reset_macro(struct exynos_dp_device *dp)
writel(reg, dp->reg_base + EXYNOS_DP_PHY_TEST);
/* 10 us is the minimum reset time. */
- udelay(10);
+ usleep_range(10, 20);
reg &= ~MACRO_RST;
writel(reg, dp->reg_base + EXYNOS_DP_PHY_TEST);
--
1.7.1
^ permalink raw reply related
* RE: [PATCH v2] video: da8xx-fb: add 24bpp LCD configuration support
From: Manjunathappa, Prakash @ 2012-07-18 9:44 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1342590441-8441-1-git-send-email-prakash.pm@ti.com>
Hi Peter Korsgaard,
On Wed, Jul 18, 2012 at 12:28:21, Peter Korsgaard wrote:
> >>>>> "Prakash" = Manjunathappa, Prakash <prakash.pm@ti.com> writes:
>
> Hi,
>
> >> Personally I don't like, mix of "u32" and "unsigned int" declaration.
> >>
>
> Prakash> "unsigned int" is not guaranteed to be 32bit, may be "unsigned
> Prakash> long" is better choice.
>
> On the platforms where da8xx-fb is used it is.
>
I agree "unsigned int" will suffice for the platforms where da8xx-fb is used.
With respect to below reference from "The C Programming Language" from "Kernighan and Ritchie"
I prefer to use "unsigned long", please let me know you views.
" The intent is that short and long should provide different lengths of integers where practical; int will
normally be the natural size for a particular machine. short is often 16 bits long, and int either 16 or
32 bits. Each compiler is free to choose appropriate sizes for its own hardware, subject only to the the
restriction that shorts and ints are at least 16 bits, longs are at least 32 bits, and short is no longer
than int, which is no longer than long."
> >> > unsigned int palette_sz;
> >> > unsigned int pxl_clk;
> >> > int blank;
> >> > @@ -546,6 +546,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
> >> > return 0;
> >> > }
> >> >
> >> > +
> >> > +#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
> >>
> >> Did you run checkpatch.pl on this patch?
> >>
>
> Prakash> Yes, checkpatch.pl did not complain anything on this line. I
> Prakash> will add space around binary operators.
>
> An inline function would be nicer.
>
For the sake of uniformity with existing FB drivers, can I leave it as macro?
Thanks,
Prakash
> --
> Bye, Peter Korsgaard
>
^ permalink raw reply
* Re: [BUGFIX] video/mxsfb: fix crash when unblanking the display
From: Robin van der Gracht @ 2012-07-18 9:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20209.35406.958883.977664@ipc1.ka-ro>
Hi Lothar,
> Hi,
>
> Lothar Waßmann writes:
>> The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature.
>> The write in mxsfb_disable_controller() sets the data_cnt for the LCD
>> DMA to 0 which obviously means the max. count for the LCD DMA and
>> leads to overwriting arbitrary memory when the display is unblanked.
>>
>> Signed-off-by: Lothar Waßmann<LW@KARO-electronics.de>
>> ---
>> drivers/video/mxsfb.c | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
>> index d837d63..03cb95a 100644
>> --- a/drivers/video/mxsfb.c
>> +++ b/drivers/video/mxsfb.c
>> @@ -366,7 +366,8 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
>> loop--;
>> }
>>
>> - writel(VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4 + REG_CLR);
>> + reg = readl(host->base + LCDC_VDCTRL4);
>> + writel(reg& ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
>>
>> clk_disable(host->clk);
>>
>> --
>> 1.5.6.5
>>
> Ping. Any comments on this?
>
>
> Lothar Waßmann
I've encountered this problem to, and i can confirm your patch fixed it.
The VDCTRL4 register has no CLR feature.
Regards,
--
Robin van der Gracht
Protonic Holland.
tel.: +31 (0) 229 212928
fax.: +31 (0) 229 210930
Factorij 36 / 1689 AL Zwaag
^ permalink raw reply
* RE: [PATCH v2] video: da8xx-fb: add 24bpp LCD configuration support
From: Hiremath, Vaibhav @ 2012-07-18 7:00 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1342590441-8441-1-git-send-email-prakash.pm@ti.com>
On Wed, Jul 18, 2012 at 12:19:45, Manjunathappa, Prakash wrote:
> Hi Vaibhav,
>
> On Wed, Jul 18, 2012 at 11:41:43, Hiremath, Vaibhav wrote:
> > On Wed, Jul 18, 2012 at 11:17:21, Manjunathappa, Prakash wrote:
> > > LCD controller on am335x supports 24bpp raster configuration in addition
> > > to ones on da850. LCDC also supports 24bpp in unpacked format having
> > > ARGB:8888 32bpp format data in DDR, but it doesn't interpret alpha
> > > component of the data.
> > >
> > > Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> > > Cc: Anatolij Gustschin <agust@denx.de>
> > > ---
> > > Since v1:
> > > Addressed Tobias's review comments on calculation of pseudopalette for
> > > FB_VISUAL_TRUECOLOR type.
> > >
> > > drivers/video/da8xx-fb.c | 88 ++++++++++++++++++++++++++++-----------------
> > > 1 files changed, 55 insertions(+), 33 deletions(-)
> > >
> > > diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> > > index 47118c7..3cda461 100644
> > > --- a/drivers/video/da8xx-fb.c
> > > +++ b/drivers/video/da8xx-fb.c
> > > @@ -153,7 +153,7 @@ struct da8xx_fb_par {
> > > unsigned int dma_end;
> > > struct clk *lcdc_clk;
> > > int irq;
> > > - unsigned short pseudo_palette[16];
> > > + u32 pseudo_palette[16];
> >
> > Personally I don't like, mix of "u32" and "unsigned int" declaration.
> >
>
> "unsigned int" is not guaranteed to be 32bit, may be "unsigned long" is
> better choice.
>
> > > unsigned int palette_sz;
> > > unsigned int pxl_clk;
> > > int blank;
> > > @@ -546,6 +546,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
> > > return 0;
> > > }
> > >
> > > +
> > > +#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
> >
> > Did you run checkpatch.pl on this patch?
> >
>
> Yes, checkpatch.pl did not complain anything on this line. I will add space
> around binary operators.
>
You can choose not to do this change, since checkpatch is ok and other
drivers also does same thing.
> > > static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> > > unsigned blue, unsigned transp,
> > > struct fb_info *info)
> > > @@ -561,13 +563,33 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> > > if (info->fix.visual = FB_VISUAL_DIRECTCOLOR)
> > > return 1;
> > >
> > > - if (info->var.bits_per_pixel = 4) {
> > > - if (regno > 15)
> > > - return 1;
> > > + switch (info->fix.visual) {
> > > + case FB_VISUAL_TRUECOLOR:
> > > + red = CNVT_TOHW(red, info->var.red.length);
> > > + green = CNVT_TOHW(green, info->var.green.length);
> > > + blue = CNVT_TOHW(blue, info->var.blue.length);
> >
> > Why not add another underscore after TO? define like this => CNVT_TO_HW
> >
>
> I referred drivers/video/skeletonfb.c, I assume it is standard. Please let me know if not.
I did grep on drivers/video/ and could see lots of drivers use it (almost
same definition).
Ignore this comment as well, I could have recommended to move it to common
file and do some cleanup, but not sure about background on this macro.
Thanks,
Vaibhav
>
> > > + break;
> > > + case FB_VISUAL_PSEUDOCOLOR:
> > > + if (info->var.bits_per_pixel = 4) {
> > > + if (regno > 15)
> > > + return 1;
> > > +
> > > + if (info->var.grayscale) {
> > > + pal = regno;
> > > + } else {
> > > + red >>= 4;
> > > + green >>= 8;
> > > + blue >>= 12;
> > > +
> > > + pal = (red & 0x0f00);
> > > + pal |= (green & 0x00f0);
> > > + pal |= (blue & 0x000f);
> > > + }
> > > + if (regno = 0)
> > > + pal |= 0x2000;
> > > + palette[regno] = pal;
> > >
> > > - if (info->var.grayscale) {
> > > - pal = regno;
> > > - } else {
> > > + } else if (info->var.bits_per_pixel = 8) {
> > > red >>= 4;
> > > green >>= 8;
> > > blue >>= 12;
> > > @@ -575,36 +597,35 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> > > pal = (red & 0x0f00);
> > > pal |= (green & 0x00f0);
> > > pal |= (blue & 0x000f);
> > > - }
> > > - if (regno = 0)
> > > - pal |= 0x2000;
> > > - palette[regno] = pal;
> > > -
> > > - } else if (info->var.bits_per_pixel = 8) {
> > > - red >>= 4;
> > > - green >>= 8;
> > > - blue >>= 12;
> > > -
> > > - pal = (red & 0x0f00);
> > > - pal |= (green & 0x00f0);
> > > - pal |= (blue & 0x000f);
> > >
> > > - if (palette[regno] != pal) {
> > > - update_hw = 1;
> > > - palette[regno] = pal;
> > > + if (palette[regno] != pal) {
> > > + update_hw = 1;
> > > + palette[regno] = pal;
> > > + }
> > > }
> > > - } else if ((info->var.bits_per_pixel = 16) && regno < 16) {
> > > - red >>= (16 - info->var.red.length);
> > > - red <<= info->var.red.offset;
> > > + break;
> > > + }
> > >
> > > - green >>= (16 - info->var.green.length);
> > > - green <<= info->var.green.offset;
> > > + /* Truecolor has hardware independent palette */
> > > + if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
> > > + u32 v;
> > >
> > > - blue >>= (16 - info->var.blue.length);
> > > - blue <<= info->var.blue.offset;
> > > + if (regno > 15)
> > > + return -EINVAL;
> > >
> > > - par->pseudo_palette[regno] = red | green | blue;
> > > + v = (red << info->var.red.offset) |
> > > + (green << info->var.green.offset) |
> > > + (blue << info->var.blue.offset);
> > >
> > > + switch (info->var.bits_per_pixel) {
> > > + case 16:
> > > + ((u16 *) (info->pseudo_palette))[regno] = v;
> > > + break;
> > > + case 24:
> > > + case 32:
> > > + ((u32 *) (info->pseudo_palette))[regno] = v;
> > > + break;
> > > + }
> > > if (palette[0] != 0x4000) {
> > > update_hw = 1;
> > > palette[0] = 0x4000;
> > > @@ -617,6 +638,7 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> > >
> > > return 0;
> > > }
> > > +#undef CNVT_TOHW
> > >
> >
> > Is this required?
> >
>
> Not required really, again thought it as standard since 8 out of 10 drivers does it.
>
> Thanks,
> Prakash
>
^ permalink raw reply
* Re: [PATCH v2] video: da8xx-fb: add 24bpp LCD configuration support
From: Peter Korsgaard @ 2012-07-18 6:58 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1342590441-8441-1-git-send-email-prakash.pm@ti.com>
>>>>> "Prakash" = Manjunathappa, Prakash <prakash.pm@ti.com> writes:
Hi,
>> Personally I don't like, mix of "u32" and "unsigned int" declaration.
>>
Prakash> "unsigned int" is not guaranteed to be 32bit, may be "unsigned
Prakash> long" is better choice.
On the platforms where da8xx-fb is used it is.
>> > unsigned int palette_sz;
>> > unsigned int pxl_clk;
>> > int blank;
>> > @@ -546,6 +546,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
>> > return 0;
>> > }
>> >
>> > +
>> > +#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
>>
>> Did you run checkpatch.pl on this patch?
>>
Prakash> Yes, checkpatch.pl did not complain anything on this line. I
Prakash> will add space around binary operators.
An inline function would be nicer.
--
Bye, Peter Korsgaard
^ permalink raw reply
* RE: [PATCH v2] video: da8xx-fb: add 24bpp LCD configuration support
From: Manjunathappa, Prakash @ 2012-07-18 6:49 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1342590441-8441-1-git-send-email-prakash.pm@ti.com>
Hi Vaibhav,
On Wed, Jul 18, 2012 at 11:41:43, Hiremath, Vaibhav wrote:
> On Wed, Jul 18, 2012 at 11:17:21, Manjunathappa, Prakash wrote:
> > LCD controller on am335x supports 24bpp raster configuration in addition
> > to ones on da850. LCDC also supports 24bpp in unpacked format having
> > ARGB:8888 32bpp format data in DDR, but it doesn't interpret alpha
> > component of the data.
> >
> > Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> > Cc: Anatolij Gustschin <agust@denx.de>
> > ---
> > Since v1:
> > Addressed Tobias's review comments on calculation of pseudopalette for
> > FB_VISUAL_TRUECOLOR type.
> >
> > drivers/video/da8xx-fb.c | 88 ++++++++++++++++++++++++++++-----------------
> > 1 files changed, 55 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> > index 47118c7..3cda461 100644
> > --- a/drivers/video/da8xx-fb.c
> > +++ b/drivers/video/da8xx-fb.c
> > @@ -153,7 +153,7 @@ struct da8xx_fb_par {
> > unsigned int dma_end;
> > struct clk *lcdc_clk;
> > int irq;
> > - unsigned short pseudo_palette[16];
> > + u32 pseudo_palette[16];
>
> Personally I don't like, mix of "u32" and "unsigned int" declaration.
>
"unsigned int" is not guaranteed to be 32bit, may be "unsigned long" is
better choice.
> > unsigned int palette_sz;
> > unsigned int pxl_clk;
> > int blank;
> > @@ -546,6 +546,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
> > return 0;
> > }
> >
> > +
> > +#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
>
> Did you run checkpatch.pl on this patch?
>
Yes, checkpatch.pl did not complain anything on this line. I will add space
around binary operators.
> > static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> > unsigned blue, unsigned transp,
> > struct fb_info *info)
> > @@ -561,13 +563,33 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> > if (info->fix.visual = FB_VISUAL_DIRECTCOLOR)
> > return 1;
> >
> > - if (info->var.bits_per_pixel = 4) {
> > - if (regno > 15)
> > - return 1;
> > + switch (info->fix.visual) {
> > + case FB_VISUAL_TRUECOLOR:
> > + red = CNVT_TOHW(red, info->var.red.length);
> > + green = CNVT_TOHW(green, info->var.green.length);
> > + blue = CNVT_TOHW(blue, info->var.blue.length);
>
> Why not add another underscore after TO? define like this => CNVT_TO_HW
>
I referred drivers/video/skeletonfb.c, I assume it is standard. Please let me know if not.
> > + break;
> > + case FB_VISUAL_PSEUDOCOLOR:
> > + if (info->var.bits_per_pixel = 4) {
> > + if (regno > 15)
> > + return 1;
> > +
> > + if (info->var.grayscale) {
> > + pal = regno;
> > + } else {
> > + red >>= 4;
> > + green >>= 8;
> > + blue >>= 12;
> > +
> > + pal = (red & 0x0f00);
> > + pal |= (green & 0x00f0);
> > + pal |= (blue & 0x000f);
> > + }
> > + if (regno = 0)
> > + pal |= 0x2000;
> > + palette[regno] = pal;
> >
> > - if (info->var.grayscale) {
> > - pal = regno;
> > - } else {
> > + } else if (info->var.bits_per_pixel = 8) {
> > red >>= 4;
> > green >>= 8;
> > blue >>= 12;
> > @@ -575,36 +597,35 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> > pal = (red & 0x0f00);
> > pal |= (green & 0x00f0);
> > pal |= (blue & 0x000f);
> > - }
> > - if (regno = 0)
> > - pal |= 0x2000;
> > - palette[regno] = pal;
> > -
> > - } else if (info->var.bits_per_pixel = 8) {
> > - red >>= 4;
> > - green >>= 8;
> > - blue >>= 12;
> > -
> > - pal = (red & 0x0f00);
> > - pal |= (green & 0x00f0);
> > - pal |= (blue & 0x000f);
> >
> > - if (palette[regno] != pal) {
> > - update_hw = 1;
> > - palette[regno] = pal;
> > + if (palette[regno] != pal) {
> > + update_hw = 1;
> > + palette[regno] = pal;
> > + }
> > }
> > - } else if ((info->var.bits_per_pixel = 16) && regno < 16) {
> > - red >>= (16 - info->var.red.length);
> > - red <<= info->var.red.offset;
> > + break;
> > + }
> >
> > - green >>= (16 - info->var.green.length);
> > - green <<= info->var.green.offset;
> > + /* Truecolor has hardware independent palette */
> > + if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
> > + u32 v;
> >
> > - blue >>= (16 - info->var.blue.length);
> > - blue <<= info->var.blue.offset;
> > + if (regno > 15)
> > + return -EINVAL;
> >
> > - par->pseudo_palette[regno] = red | green | blue;
> > + v = (red << info->var.red.offset) |
> > + (green << info->var.green.offset) |
> > + (blue << info->var.blue.offset);
> >
> > + switch (info->var.bits_per_pixel) {
> > + case 16:
> > + ((u16 *) (info->pseudo_palette))[regno] = v;
> > + break;
> > + case 24:
> > + case 32:
> > + ((u32 *) (info->pseudo_palette))[regno] = v;
> > + break;
> > + }
> > if (palette[0] != 0x4000) {
> > update_hw = 1;
> > palette[0] = 0x4000;
> > @@ -617,6 +638,7 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> >
> > return 0;
> > }
> > +#undef CNVT_TOHW
> >
>
> Is this required?
>
Not required really, again thought it as standard since 8 out of 10 drivers does it.
Thanks,
Prakash
^ permalink raw reply
* RE: [PATCH v2] video: da8xx-fb: add 24bpp LCD configuration support
From: Hiremath, Vaibhav @ 2012-07-18 6:11 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1342590441-8441-1-git-send-email-prakash.pm@ti.com>
On Wed, Jul 18, 2012 at 11:17:21, Manjunathappa, Prakash wrote:
> LCD controller on am335x supports 24bpp raster configuration in addition
> to ones on da850. LCDC also supports 24bpp in unpacked format having
> ARGB:8888 32bpp format data in DDR, but it doesn't interpret alpha
> component of the data.
>
> Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> Cc: Anatolij Gustschin <agust@denx.de>
> ---
> Since v1:
> Addressed Tobias's review comments on calculation of pseudopalette for
> FB_VISUAL_TRUECOLOR type.
>
> drivers/video/da8xx-fb.c | 88 ++++++++++++++++++++++++++++-----------------
> 1 files changed, 55 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 47118c7..3cda461 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -153,7 +153,7 @@ struct da8xx_fb_par {
> unsigned int dma_end;
> struct clk *lcdc_clk;
> int irq;
> - unsigned short pseudo_palette[16];
> + u32 pseudo_palette[16];
Personally I don't like, mix of "u32" and "unsigned int" declaration.
> unsigned int palette_sz;
> unsigned int pxl_clk;
> int blank;
> @@ -546,6 +546,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
> return 0;
> }
>
> +
> +#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
Did you run checkpatch.pl on this patch?
> static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> unsigned blue, unsigned transp,
> struct fb_info *info)
> @@ -561,13 +563,33 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> if (info->fix.visual = FB_VISUAL_DIRECTCOLOR)
> return 1;
>
> - if (info->var.bits_per_pixel = 4) {
> - if (regno > 15)
> - return 1;
> + switch (info->fix.visual) {
> + case FB_VISUAL_TRUECOLOR:
> + red = CNVT_TOHW(red, info->var.red.length);
> + green = CNVT_TOHW(green, info->var.green.length);
> + blue = CNVT_TOHW(blue, info->var.blue.length);
Why not add another underscore after TO? define like this => CNVT_TO_HW
> + break;
> + case FB_VISUAL_PSEUDOCOLOR:
> + if (info->var.bits_per_pixel = 4) {
> + if (regno > 15)
> + return 1;
> +
> + if (info->var.grayscale) {
> + pal = regno;
> + } else {
> + red >>= 4;
> + green >>= 8;
> + blue >>= 12;
> +
> + pal = (red & 0x0f00);
> + pal |= (green & 0x00f0);
> + pal |= (blue & 0x000f);
> + }
> + if (regno = 0)
> + pal |= 0x2000;
> + palette[regno] = pal;
>
> - if (info->var.grayscale) {
> - pal = regno;
> - } else {
> + } else if (info->var.bits_per_pixel = 8) {
> red >>= 4;
> green >>= 8;
> blue >>= 12;
> @@ -575,36 +597,35 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> pal = (red & 0x0f00);
> pal |= (green & 0x00f0);
> pal |= (blue & 0x000f);
> - }
> - if (regno = 0)
> - pal |= 0x2000;
> - palette[regno] = pal;
> -
> - } else if (info->var.bits_per_pixel = 8) {
> - red >>= 4;
> - green >>= 8;
> - blue >>= 12;
> -
> - pal = (red & 0x0f00);
> - pal |= (green & 0x00f0);
> - pal |= (blue & 0x000f);
>
> - if (palette[regno] != pal) {
> - update_hw = 1;
> - palette[regno] = pal;
> + if (palette[regno] != pal) {
> + update_hw = 1;
> + palette[regno] = pal;
> + }
> }
> - } else if ((info->var.bits_per_pixel = 16) && regno < 16) {
> - red >>= (16 - info->var.red.length);
> - red <<= info->var.red.offset;
> + break;
> + }
>
> - green >>= (16 - info->var.green.length);
> - green <<= info->var.green.offset;
> + /* Truecolor has hardware independent palette */
> + if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
> + u32 v;
>
> - blue >>= (16 - info->var.blue.length);
> - blue <<= info->var.blue.offset;
> + if (regno > 15)
> + return -EINVAL;
>
> - par->pseudo_palette[regno] = red | green | blue;
> + v = (red << info->var.red.offset) |
> + (green << info->var.green.offset) |
> + (blue << info->var.blue.offset);
>
> + switch (info->var.bits_per_pixel) {
> + case 16:
> + ((u16 *) (info->pseudo_palette))[regno] = v;
> + break;
> + case 24:
> + case 32:
> + ((u32 *) (info->pseudo_palette))[regno] = v;
> + break;
> + }
> if (palette[0] != 0x4000) {
> update_hw = 1;
> palette[0] = 0x4000;
> @@ -617,6 +638,7 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
>
> return 0;
> }
> +#undef CNVT_TOHW
>
Is this required?
Thanks,
Vaibhav
> static void lcd_reset(struct da8xx_fb_par *par)
> {
> --
> 1.7.1
>
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source@linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
>
^ permalink raw reply
* [PATCH v2] video: da8xx-fb: add 24bpp LCD configuration support
From: Manjunathappa, Prakash @ 2012-07-18 5:59 UTC (permalink / raw)
To: linux-fbdev
LCD controller on am335x supports 24bpp raster configuration in addition
to ones on da850. LCDC also supports 24bpp in unpacked format having
ARGB:8888 32bpp format data in DDR, but it doesn't interpret alpha
component of the data.
Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
Cc: Anatolij Gustschin <agust@denx.de>
---
Since v1:
Addressed Tobias's review comments on calculation of pseudopalette for
FB_VISUAL_TRUECOLOR type.
drivers/video/da8xx-fb.c | 88 ++++++++++++++++++++++++++++-----------------
1 files changed, 55 insertions(+), 33 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 47118c7..3cda461 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -153,7 +153,7 @@ struct da8xx_fb_par {
unsigned int dma_end;
struct clk *lcdc_clk;
int irq;
- unsigned short pseudo_palette[16];
+ u32 pseudo_palette[16];
unsigned int palette_sz;
unsigned int pxl_clk;
int blank;
@@ -546,6 +546,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
return 0;
}
+
+#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
unsigned blue, unsigned transp,
struct fb_info *info)
@@ -561,13 +563,33 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
if (info->fix.visual = FB_VISUAL_DIRECTCOLOR)
return 1;
- if (info->var.bits_per_pixel = 4) {
- if (regno > 15)
- return 1;
+ switch (info->fix.visual) {
+ case FB_VISUAL_TRUECOLOR:
+ red = CNVT_TOHW(red, info->var.red.length);
+ green = CNVT_TOHW(green, info->var.green.length);
+ blue = CNVT_TOHW(blue, info->var.blue.length);
+ break;
+ case FB_VISUAL_PSEUDOCOLOR:
+ if (info->var.bits_per_pixel = 4) {
+ if (regno > 15)
+ return 1;
+
+ if (info->var.grayscale) {
+ pal = regno;
+ } else {
+ red >>= 4;
+ green >>= 8;
+ blue >>= 12;
+
+ pal = (red & 0x0f00);
+ pal |= (green & 0x00f0);
+ pal |= (blue & 0x000f);
+ }
+ if (regno = 0)
+ pal |= 0x2000;
+ palette[regno] = pal;
- if (info->var.grayscale) {
- pal = regno;
- } else {
+ } else if (info->var.bits_per_pixel = 8) {
red >>= 4;
green >>= 8;
blue >>= 12;
@@ -575,36 +597,35 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
pal = (red & 0x0f00);
pal |= (green & 0x00f0);
pal |= (blue & 0x000f);
- }
- if (regno = 0)
- pal |= 0x2000;
- palette[regno] = pal;
-
- } else if (info->var.bits_per_pixel = 8) {
- red >>= 4;
- green >>= 8;
- blue >>= 12;
-
- pal = (red & 0x0f00);
- pal |= (green & 0x00f0);
- pal |= (blue & 0x000f);
- if (palette[regno] != pal) {
- update_hw = 1;
- palette[regno] = pal;
+ if (palette[regno] != pal) {
+ update_hw = 1;
+ palette[regno] = pal;
+ }
}
- } else if ((info->var.bits_per_pixel = 16) && regno < 16) {
- red >>= (16 - info->var.red.length);
- red <<= info->var.red.offset;
+ break;
+ }
- green >>= (16 - info->var.green.length);
- green <<= info->var.green.offset;
+ /* Truecolor has hardware independent palette */
+ if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
+ u32 v;
- blue >>= (16 - info->var.blue.length);
- blue <<= info->var.blue.offset;
+ if (regno > 15)
+ return -EINVAL;
- par->pseudo_palette[regno] = red | green | blue;
+ v = (red << info->var.red.offset) |
+ (green << info->var.green.offset) |
+ (blue << info->var.blue.offset);
+ switch (info->var.bits_per_pixel) {
+ case 16:
+ ((u16 *) (info->pseudo_palette))[regno] = v;
+ break;
+ case 24:
+ case 32:
+ ((u32 *) (info->pseudo_palette))[regno] = v;
+ break;
+ }
if (palette[0] != 0x4000) {
update_hw = 1;
palette[0] = 0x4000;
@@ -617,6 +638,7 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
return 0;
}
+#undef CNVT_TOHW
static void lcd_reset(struct da8xx_fb_par *par)
{
--
1.7.1
^ permalink raw reply related
* Re: dma-buf/fbdev: one-to-many support
From: Laurent Pinchart @ 2012-07-18 0:41 UTC (permalink / raw)
To: David Herrmann; +Cc: dri-devel, linux-fbdev, Sumit Semwal, linux-media
In-Reply-To: <CANq1E4TMddx-+h5cGAn=R2VoNBQ274CBZDGV02Ku2Hgo_Hc3iA@mail.gmail.com>
Hi David,
On Tuesday 17 July 2012 14:23:18 David Herrmann wrote:
> On Tue, Jul 17, 2012 at 1:24 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> >> The main issue is that fbdev has been designed with the implicit
> >> assumption that an fbdev driver will always own the graphics memory it
> >> uses. All components in the stack, from drivers to applications, have
> >> been designed around that assumption.
> >>
> >> We could of course fix this, revamp the fbdev API and turn it into a
> >> modern graphics API, but I really wonder whether it would be worth it.
> >> DRM has been getting quite a lot of attention lately, especially from
> >> embedded developers and vendors, and the trend seems to me like the
> >> (Linux) world will gradually move from fbdev to DRM.
> >>
> >> Please feel free to disagree :-)
> >
> > I would disagree on the "main issue" bit. All the graphics cards have
> > their own formats and cache management rules. Simply sharing a buffer
> > doesn't work - which is why all of the extra gloop will be needed.
>
> This is exactly why I suggested adding an "owner" field. A driver
> could then check whether the buffer it is supposed to share/takeover
> is from a compatible (or even the same) driver/device. If it is not,
> it would simply reject using the buffer. Then again, if we have
> multiple devices that are incompatible, we are still unable to share
> the buffer. So this attempt would only be useful if we have tons of
> DisplayLink devices attached that all use the same driver, for
> example.
>
> Regarding DRM: In user-space I prefer DRM over fbdev. With the
> introduction of the dumb-buffers there isn't even the need to have
> mesa installed. However, fblog runs in kernel space and currently
> cannot use DRM as there is no in-kernel DRM API. I looked at
> drm-fops.c whether it is easy to create a very simple in-kernel API
> but then I dropped the idea as this might be too complex for a simple
> debugging-only driver. Another attempt would be making the
> drm-fb-helper more generic so we can use this layer as in-kernel DRM
> API.
>
> I had a deeper look into this this weekend and so as a summary I think
> all in-kernel graphics access is probably not worth optimizing it.
> fbcon is already working great and fblog is only used during boot and
> oopses/panics and can be restricted to a single device. I will have
> another look at the drivers in a few weeks but if you tell me that
> this is not easy to implement, I will probably have to let this idea
> go.
My gut feeling is that, given the effort required to add new APIs, it would be
more interesting to work on an in-kernel DRM API to make drmcon and drmlog
implementations possible without any fbdev compatibility layer. That's rather
a long term goal though.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 1/4] video: Add support for the Solomon SSD1307 OLED Controller
From: Thomas Petazzoni @ 2012-07-17 16:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1342540787-14930-2-git-send-email-maxime.ripard@free-electrons.com>
Le Tue, 17 Jul 2012 17:59:44 +0200,
Maxime Ripard <maxime.ripard@free-electrons.com> a écrit :
> This patches adds support for the Solomon SSD1307 OLED
patches -> patch
> controller found on the Crystalfontz CFA10036 board.
>
> It is a monochrome 128x39 display, that can operate over
> I2C or SPI.
The display is not 128x39, but rather the SSD1307 controller can drive
monochrome displays up to 128x39 pixels.
> The current driver as only be tested on the CFA-10036,
as -> has
> that is using this controller over I2C to driver a 96x16
> OLED screen.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Brian Lilly <brian@crystalfontz.com>
Ideally, it would be nice if this driver was more separated from the
display size (i.e the display size would be passed as DT properties,
for example), and that the driver would support different display
sizes. However, having an initial implementation that works for 96x16
is probably a good starting point.
> +Required properties:
> + - compatible: Should be "solomon,ssd1307fb-<bus>". The only supported bus for
> + now is i2c.
> + - reg: Should contain address of the controller on the I2C bus. Most likely
> + 0x3c or 0x3d
> + - pwm: Should contain the pwm to use according to the OF device tree PWM
> + specification [0]
> + - oled-reset-gpios: Should contain the GPIO used to reset the OLED display
Since there is only one gpio for reset, is is necessary to have a 's'
at the end of oled-reset-gpios?
> +Optional properties:
> + - reset-active-low: Is the reset gpio is active on physical low?
Shouldn't this one be called oled-reset-active-low, for consistency
with the oled-reset-gpio property?
> +static int ssd1307fb_write_cmd_array(struct i2c_client *client, u8* cmd, u32 len)
> +{
> + u8 *buf;
> + int ret;
int ret = 0;
> +
> + buf = kzalloc(len + 1, GFP_KERNEL);
> + if (!buf) {
> + dev_err(&client->dev, "Couldn't allocate sending buffer.\n");
> + ret = -ENOMEM;
> + goto error;
goto out;
> + }
> +
> + buf[0] = SSD1307FB_COMMAND;
> + memcpy(buf + 1, cmd, len);
> +
> + ret = i2c_master_send(client, buf, len + 1);
> + if (ret != len + 1) {
> + dev_err(&client->dev, "Couldn't send I2C command.\n");
> + goto error;
goto out;
> + }
> +
> + kfree(buf);
> + return 0;
delete those lines.
> +
> +error:
out:
> + kfree(buf);
> + return ret;
> +}
> +
> +static inline int ssd1307fb_write_cmd(struct i2c_client *client, u8 cmd)
> +{
> + return ssd1307fb_write_cmd_array(client, &cmd, 1);
> +}
> +
> +static int ssd1307fb_write_data_array(struct i2c_client *client, u8* cmd, u32 len)
> +{
> + u8 *buf;
> + int ret;
> +
> + buf = kzalloc(len + 1, GFP_KERNEL);
> + if (!buf) {
> + dev_err(&client->dev, "Couldn't allocate sending buffer.\n");
> + ret = -ENOMEM;
> + goto error;
> + }
> +
> + buf[0] = SSD1307FB_DATA;
> + memcpy(buf + 1, cmd, len);
> +
> + ret = i2c_master_send(client, buf, len + 1);
> + if (ret != len + 1) {
> + dev_err(&client->dev, "Couldn't send I2C data.\n");
> + goto error;
> + }
> +
> + kfree(buf);
> + return 0;
> +
> +error:
> + kfree(buf);
> + return ret;
> +}
This looks highly duplicated from the write_cmd_array(). I'm sure some
factorization is possible here :)
> +static inline int ssd1307fb_write_data(struct i2c_client *client, u8 data)
> +{
> + return ssd1307fb_write_data_array(client, &data, 1);
> +}
> +
> +static int ssd1307fb_set(struct i2c_client *client, u8 value)
> +{
> + int i, j, ret;
> +
> + for (i = 1; i <= (SSD1307FB_HEIGHT / 8); i++) {
> + ret = ssd1307fb_write_cmd(client, SSD1307FB_START_PAGE_ADDRESS + i);
> + if (ret)
> + goto i2c_error;
> +
> + ret = ssd1307fb_write_cmd(client, 0x00);
> + if (ret)
> + goto i2c_error;
> +
> + ret = ssd1307fb_write_cmd(client, 0x10);
> + if (ret)
> + goto i2c_error;
> +
> + for (j = 0; j < SSD1307FB_WIDTH; j++)
> + ssd1307fb_write_data(client, value);
> + }
> +
> + return 0;
> +
> +i2c_error:
> + dev_err(&client->dev, "Couldn't send i2c command: %d\n", ret);
> + return ret;
> +}
> +
> +static void ssd1307fb_update_display(struct ssd1307fb_par *par)
> +{
> + u8 *vmem = par->info->screen_base;
> + int i, j, k;
> +
> + /*
> + * A page is 8 bit large and covers all the line. In order to
by "large", you mean the "height", so I wouldn't use the word "large",
which I think is typically associated with "width" rather than "height".
> + * update the screen, you have to send a byte, each byte
> + * containing a bit per pixel to set, on the same column. That
> + * way, by sending a byte, you will only portion of the screen
English issue.
> + * that has a height of 8 bit and a width of 1, the right-most
> + * bit being on the top of this frame. So we have to do a bit
> + * of magic here...
How about summarizing this by something like:
The screen is divided in pages, each having a height of 8 pixels, and
the width of the screen. When sending a byte of data to the
controller, it gives the 8 bits for the current column. I.e, the first
byte are the 8 bits of the first column, then the 8 bits for the
second column, etc.
And even add some ASCII art to it:
Representation of the screen, assuming it is 5 bits wide. Each
letter-number combination is a bit that controls one pixel.
A0 A1 A2 A3 A4
B0 B1 B2 B3 B4
C0 C1 C2 C3 C4
D0 D1 D2 D3 D4
E0 E1 E2 E3 E4
F0 F1 F2 F3 F4
G0 G1 G2 G3 G4
H0 H1 H2 H3 H4
If you want to update this screen, you need to send 5 bytes:
(1) A0 B0 C0 D0 E0 F0 G0 H0
(2) A1 B1 C1 D1 E1 F1 G1 H1
(3) A2 B2 C2 D2 E2 F2 G2 H2
(4) A3 B3 C3 D3 E3 F3 G3 H3
(5) A4 B4 C4 D4 E4 F4 G4 H4
> + for (i = 0; i < (SSD1307FB_HEIGHT / 8); i++) {
> + ssd1307fb_write_cmd(par->client, SSD1307FB_START_PAGE_ADDRESS + (i + 1));
> + ssd1307fb_write_cmd(par->client, 0x00);
> + ssd1307fb_write_cmd(par->client, 0x10);
> +
> + for (j = 0; j < SSD1307FB_WIDTH; j++) {
> + u8 buf = 0;
> + for (k = 0; k < 8; k++) {
> + u32 page_length = SSD1307FB_WIDTH * i;
> + u32 index = page_length + (SSD1307FB_WIDTH * k + j) / 8;
> + u8 byte = *(vmem + index);
> + u8 bit = byte & (1 << (7 - (j % 8)));
> + bit = bit >> (7 - (j % 8));
> + buf |= bit << k;
> + }
> + ssd1307fb_write_data(par->client, buf);
> + }
> + }
> +
> + return;
> +}
> +
> +
> +static ssize_t ssd1307fb_write(struct fb_info *info, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct ssd1307fb_par *par = info->par;
> + unsigned long p = *ppos;
> + void *dst;
> + int err = 0;
> +
> + dst = (void __force *) (info->screen_base + p);
> +
> + if (copy_from_user(dst, buf, count))
> + err = -EFAULT;
> +
> + if (!err)
> + *ppos += count;
> +
> + ssd1307fb_update_display(par);
> +
> + return (err) ? err : count;
> +}
> +
> +static void ssd1307fb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
> +{
> + struct ssd1307fb_par *par = info->par;
> + sys_fillrect(info, rect);
> + ssd1307fb_update_display(par);
> +}
> +
> +static void ssd1307fb_copyarea(struct fb_info *info, const struct fb_copyarea *area)
> +{
> + struct ssd1307fb_par *par = info->par;
> + sys_copyarea(info, area);
> + ssd1307fb_update_display(par);
> +}
> +
> +static void ssd1307fb_imageblit(struct fb_info *info, const struct fb_image *image)
> +{
> + struct ssd1307fb_par *par = info->par;
> + sys_imageblit(info, image);
> + ssd1307fb_update_display(par);
> +}
> +
> +static struct fb_ops ssd1307fb_ops = {
> + .owner = THIS_MODULE,
> + .fb_read = fb_sys_read,
> + .fb_write = ssd1307fb_write,
> + .fb_fillrect = ssd1307fb_fillrect,
> + .fb_copyarea = ssd1307fb_copyarea,
> + .fb_imageblit = ssd1307fb_imageblit,
> +};
> +
> +static void ssd1307fb_deferred_io(struct fb_info *info,
> + struct list_head *pagelist)
> +{
> + ssd1307fb_update_display(info->par);
> +}
> +
> +static struct fb_deferred_io ssd1307fb_defio = {
> + .delay = HZ,
> + .deferred_io = ssd1307fb_deferred_io,
> +};
> +
> +static int __devinit ssd1307fb_probe(struct i2c_client *client, const struct i2c_device_id *id)
> +{
> + struct fb_info *info;
> + u32 vmem_size = SSD1307FB_WIDTH * SSD1307FB_HEIGHT / 8;
> + struct ssd1307fb_par *par;
> + u8 *vmem;
> + int ret;
> +
> + if (!client->dev.of_node) {
> + dev_err(&client->dev, "No device tree data found!\n");
> + ret = -EINVAL;
> + goto generic_error;
> + }
> +
> + info = framebuffer_alloc(sizeof(struct ssd1307fb_par), &client->dev);
> + if (!info) {
> + ret = -ENOMEM;
> + goto generic_error;
> + }
> +
> + vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
Error checking?
> + info->fbops = &ssd1307fb_ops;
> + info->fix = ssd1307fb_fix;
> + info->fbdefio = &ssd1307fb_defio;
> +
> + info->var = ssd1307fb_var;
> + info->var.red.length = 1;
> + info->var.red.offset = 0;
> + info->var.green.length = 1;
> + info->var.green.offset = 0;
> + info->var.blue.length = 1;
> + info->var.blue.offset = 0;
> +
> + info->screen_base = (u8 __force __iomem *)vmem;
> + info->fix.smem_len = vmem_size;
> +
> + fb_deferred_io_init(info);
> +
> + par = info->par;
> + par->info = info;
> + par->client = client;
> +
> + par->reset = of_get_named_gpio(client->dev.of_node,
> + "oled-reset-gpios", 0);
> + if (gpio_is_valid(par->reset)) {
> + int flags = GPIOF_OUT_INIT_HIGH;
> + if (of_get_property(client->dev.of_node,
> + "reset-active-low", NULL))
> + flags = GPIOF_OUT_INIT_LOW;
> + ret = devm_gpio_request_one(&client->dev, par->reset,
> + flags, "oled-reset");
> + if (ret) {
> + dev_err(&client->dev,
> + "failed to request gpio %d: %d\n",
> + par->reset, ret);
> + goto reset_oled_error;
> + }
> + }
> +
> + par->pwm = pwm_get(&client->dev, NULL);
> + if (IS_ERR(par->pwm)) {
> + dev_err(&client->dev, "Could not get PWM from device tree!\n");
> + ret = PTR_ERR(par->pwm);
> + goto pwm_error;
> + }
> +
> + par->pwm_period = pwm_get_period(par->pwm);
> +
> + dev_dbg(&client->dev, "Using PWM%d with a %dns period.\n", par->pwm->pwm, par->pwm_period);
> +
> + ret = register_framebuffer(info);
> + if (ret) {
> + dev_err(&client->dev, "Couldn't register the framebuffer\n");
> + goto fbreg_error;
> + }
> +
> + i2c_set_clientdata(client, info);
> +
> + /* Reset the screen */
> + gpio_set_value(par->reset, 1);
> + udelay(4);
> + gpio_set_value(par->reset, 0);
> + udelay(4);
> +
> + /* Enable the PWM */
> + pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
> + pwm_enable(par->pwm);
> +
> + /* Map column 127 of the OLED to segment 0 */
> + ret = ssd1307fb_write_cmd(client, SSD1307FB_SEG_REMAP_ON);
> + if (ret) {
> + dev_err(&client->dev, "Couldn't remap the screen.\n");
> + goto remap_error;
> + }
> +
> + /* Turn on the display */
> + ret = ssd1307fb_write_cmd(client, SSD1307FB_DISPLAY_ON);
> + if (ret) {
> + dev_err(&client->dev, "Couldn't turn the display on.\n");
> + goto remap_error;
> + }
> +
> + dev_info(&client->dev, "fb%d: %s framebuffer device registered, using %d bytes of video memory\n", info->node, info->fix.id, vmem_size);
> +
> + return 0;
> +
> +remap_error:
> + unregister_framebuffer(info);
> + pwm_disable(par->pwm);
> +fbreg_error:
> + pwm_put(par->pwm);
> +pwm_error:
> +reset_oled_error:
> + fb_deferred_io_cleanup(info);
> + framebuffer_release(info);
> +generic_error:
> + return ret;
> +}
> +
> +static int __devexit ssd1307fb_remove(struct i2c_client *client)
> +{
> + struct fb_info *info = i2c_get_clientdata(client);
> + struct ssd1307fb_par *par = info->par;
> + unregister_framebuffer(info);
> + pwm_disable(par->pwm);
> + pwm_put(par->pwm);
> + fb_deferred_io_cleanup(info);
> + framebuffer_release(info);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id ssd1307fb_i2c_id[] = {
> + { "ssd1307fb", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, ssd1307fb_i2c_id);
> +
> +static const struct of_device_id ssd1307fb_of_match[] = {
> + { .compatible = "solomon,ssd1307fb-i2c" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ssd1307fb_of_match);
> +
> +static struct i2c_driver ssd1307fb_driver = {
> + .probe = ssd1307fb_probe,
> + .remove = __devexit_p(ssd1307fb_remove),
> + .id_table = ssd1307fb_i2c_id,
> + .driver = {
> + .name = "ssd1307fb",
> + .of_match_table = of_match_ptr(ssd1307fb_of_match),
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +module_i2c_driver(ssd1307fb_driver);
> +
> +MODULE_DESCRIPTION("FB driver for the Solomon SSD1307 OLED controler");
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> +MODULE_LICENSE("GPL");
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 3/4] ARM: dts: mxs: Add pwm4 muxing options for imx28
From: Thomas Petazzoni @ 2012-07-17 16:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1342540787-14930-4-git-send-email-maxime.ripard@free-electrons.com>
Le Tue, 17 Jul 2012 17:59:46 +0200,
Maxime Ripard <maxime.ripard@free-electrons.com> a écrit :
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Brian Lilly <brian@crystalfontz.com>
> ---
> arch/arm/boot/dts/imx28.dtsi | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index 44ce1fd..0a565ef 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -461,6 +461,15 @@
> fsl,pull-up = <0>;
> };
>
> + pwm4_pins_a: pwm4@0 {
> + reg = <0>;
> + fsl,pinmux-ids = <
> + 0x31d0 /* MX28_PAD_PWM4__PWM_4 */
Unless my eyes are broken, it seems that the final >; is missing here.
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH 4/4] ARM: dts: mxs: add oled support for the cfa-10036
From: Maxime Ripard @ 2012-07-17 15:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1342540787-14930-1-git-send-email-maxime.ripard@free-electrons.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Brian Lilly <brian@crystalfontz.com>
---
arch/arm/boot/dts/imx28-cfa10036.dts | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/imx28-cfa10036.dts b/arch/arm/boot/dts/imx28-cfa10036.dts
index c03a577..0a5c722 100644
--- a/arch/arm/boot/dts/imx28-cfa10036.dts
+++ b/arch/arm/boot/dts/imx28-cfa10036.dts
@@ -33,11 +33,31 @@
};
apbx@80040000 {
+ pwm: pwm@80064000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm4_pins_a>;
+ status = "okay";
+ };
+
duart: serial@80074000 {
pinctrl-names = "default";
pinctrl-0 = <&duart_pins_b>;
status = "okay";
};
+
+ i2c0: i2c@80058000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c0_pins_b>;
+ status = "okay";
+
+ ssd1307: oled@3c {
+ compatible = "solomon,ssd1307fb-i2c";
+ reg = <0x3c>;
+ pwms = <&pwm 4 3000>;
+ oled-reset-gpios = <&gpio2 7 1>;
+ reset-active-low;
+ };
+ };
};
};
--
1.7.9.5
^ permalink raw reply related
* [PATCH 3/4] ARM: dts: mxs: Add pwm4 muxing options for imx28
From: Maxime Ripard @ 2012-07-17 15:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1342540787-14930-1-git-send-email-maxime.ripard@free-electrons.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Brian Lilly <brian@crystalfontz.com>
---
arch/arm/boot/dts/imx28.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 44ce1fd..0a565ef 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -461,6 +461,15 @@
fsl,pull-up = <0>;
};
+ pwm4_pins_a: pwm4@0 {
+ reg = <0>;
+ fsl,pinmux-ids = <
+ 0x31d0 /* MX28_PAD_PWM4__PWM_4 */
+ fsl,drive-strength = <0>;
+ fsl,voltage = <1>;
+ fsl,pull-up = <0>;
+ };
+
lcdif_24bit_pins_a: lcdif-24bit@0 {
reg = <0>;
fsl,pinmux-ids = <
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/4] ARM: dts: mxs: Add alternative I2C muxing options for imx28
From: Maxime Ripard @ 2012-07-17 15:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1342540787-14930-1-git-send-email-maxime.ripard@free-electrons.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Brian Lilly <brian@crystalfontz.com>
---
arch/arm/boot/dts/imx28.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 915db89..44ce1fd 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -410,6 +410,14 @@
fsl,pull-up = <1>;
};
+ i2c0_pins_b: i2c0@1 {
+ reg = <1>;
+ fsl,pinmux-ids = <0x3001 0x3011>;
+ fsl,drive-strength = <1>;
+ fsl,voltage = <1>;
+ fsl,pull-up = <1>;
+ };
+
saif0_pins_a: saif0@0 {
reg = <0>;
fsl,pinmux-ids = <
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/4] video: Add support for the Solomon SSD1307 OLED Controller
From: Maxime Ripard @ 2012-07-17 15:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1342540787-14930-1-git-send-email-maxime.ripard@free-electrons.com>
This patches adds support for the Solomon SSD1307 OLED
controller found on the Crystalfontz CFA10036 board.
It is a monochrome 128x39 display, that can operate over
I2C or SPI.
The current driver as only be tested on the CFA-10036,
that is using this controller over I2C to driver a 96x16
OLED screen.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Brian Lilly <brian@crystalfontz.com>
---
.../devicetree/bindings/video/ssd1307fb.txt | 24 ++
drivers/video/Kconfig | 14 +
drivers/video/Makefile | 1 +
drivers/video/ssd1307fb.c | 414 ++++++++++++++++++++
4 files changed, 453 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/ssd1307fb.txt
create mode 100644 drivers/video/ssd1307fb.c
diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt b/Documentation/devicetree/bindings/video/ssd1307fb.txt
new file mode 100644
index 0000000..73ba41c
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/ssd1307fb.txt
@@ -0,0 +1,24 @@
+* Solomon SSD1307 Framebuffer Driver
+
+Required properties:
+ - compatible: Should be "solomon,ssd1307fb-<bus>". The only supported bus for
+ now is i2c.
+ - reg: Should contain address of the controller on the I2C bus. Most likely
+ 0x3c or 0x3d
+ - pwm: Should contain the pwm to use according to the OF device tree PWM
+ specification [0]
+ - oled-reset-gpios: Should contain the GPIO used to reset the OLED display
+
+Optional properties:
+ - reset-active-low: Is the reset gpio is active on physical low?
+
+[0]: Documentation/devicetree/bindings/pwm/pwm.txt
+
+Examples:
+ssd1307: oled@3c {
+ compatible = "solomon,ssd1307fb-i2c";
+ reg = <0x3c>;
+ pwms = <&pwm 4 3000>;
+ oled-reset-gpios = <&gpio2 7 1>;
+ reset-active-low;
+};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 0217f74..21ae6dd 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2469,4 +2469,18 @@ config FB_SH_MOBILE_MERAM
Up to 4 memory channels can be configured, allowing 4 RGB or
2 YCbCr framebuffers to be configured.
+
+config FB_SSD1307
+ tristate "Solomon SSD1307 framebuffer support"
+ depends on FB && I2C
+ select FB_SYS_FOPS
+ select FB_SYS_FILLRECT
+ select FB_SYS_COPYAREA
+ select FB_SYS_IMAGEBLIT
+ select FB_DEFERRED_IO
+ select PWM
+ help
+ This driver implements support for the Solomon SSD1307
+ OLED controller over I2C.
+
endmenu
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index ee8dafb..6bbb72c 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -164,6 +164,7 @@ obj-$(CONFIG_FB_BFIN_7393) += bfin_adv7393fb.o
obj-$(CONFIG_FB_MX3) += mx3fb.o
obj-$(CONFIG_FB_DA8XX) += da8xx-fb.o
obj-$(CONFIG_FB_MXS) += mxsfb.o
+obj-$(CONFIG_FB_SSD1307) += ssd1307fb.o
# the test framebuffer is last
obj-$(CONFIG_FB_VIRTUAL) += vfb.o
diff --git a/drivers/video/ssd1307fb.c b/drivers/video/ssd1307fb.c
new file mode 100644
index 0000000..5d24360
--- /dev/null
+++ b/drivers/video/ssd1307fb.c
@@ -0,0 +1,414 @@
+/*
+ * Driver for the Solomon SSD1307 OLED controler
+ *
+ * Copyright 2012 Free Electrons
+ *
+ * Licensed under the GPLv2 or later.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/i2c.h>
+#include <linux/fb.h>
+#include <linux/uaccess.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/pwm.h>
+#include <linux/delay.h>
+
+#define SSD1307FB_WIDTH 96
+#define SSD1307FB_HEIGHT 16
+
+#define SSD1307FB_DATA 0x40
+#define SSD1307FB_COMMAND 0x80
+
+#define SSD1307FB_CONTRAST 0x81
+#define SSD1307FB_SEG_REMAP_ON 0xa1
+#define SSD1307FB_DISPLAY_OFF 0xae
+#define SSD1307FB_DISPLAY_ON 0xaf
+#define SSD1307FB_START_PAGE_ADDRESS 0xb0
+
+struct ssd1307fb_par {
+ struct i2c_client *client;
+ struct fb_info *info;
+ struct pwm_device *pwm;
+ u32 pwm_period;
+ int reset;
+};
+
+static struct fb_fix_screeninfo ssd1307fb_fix __devinitdata = {
+ .id = "Solomon SSD1307",
+ .type = FB_TYPE_PACKED_PIXELS,
+ .visual = FB_VISUAL_MONO10,
+ .xpanstep = 0,
+ .ypanstep = 0,
+ .ywrapstep = 0,
+ .line_length = SSD1307FB_WIDTH / 8,
+ .accel = FB_ACCEL_NONE,
+};
+
+static struct fb_var_screeninfo ssd1307fb_var __devinitdata = {
+ .xres = SSD1307FB_WIDTH,
+ .yres = SSD1307FB_HEIGHT,
+ .xres_virtual = SSD1307FB_WIDTH,
+ .yres_virtual = SSD1307FB_HEIGHT,
+ .bits_per_pixel = 1,
+};
+
+static int ssd1307fb_write_cmd_array(struct i2c_client *client, u8* cmd, u32 len)
+{
+ u8 *buf;
+ int ret;
+
+ buf = kzalloc(len + 1, GFP_KERNEL);
+ if (!buf) {
+ dev_err(&client->dev, "Couldn't allocate sending buffer.\n");
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ buf[0] = SSD1307FB_COMMAND;
+ memcpy(buf + 1, cmd, len);
+
+ ret = i2c_master_send(client, buf, len + 1);
+ if (ret != len + 1) {
+ dev_err(&client->dev, "Couldn't send I2C command.\n");
+ goto error;
+ }
+
+ kfree(buf);
+ return 0;
+
+error:
+ kfree(buf);
+ return ret;
+}
+
+static inline int ssd1307fb_write_cmd(struct i2c_client *client, u8 cmd)
+{
+ return ssd1307fb_write_cmd_array(client, &cmd, 1);
+}
+
+static int ssd1307fb_write_data_array(struct i2c_client *client, u8* cmd, u32 len)
+{
+ u8 *buf;
+ int ret;
+
+ buf = kzalloc(len + 1, GFP_KERNEL);
+ if (!buf) {
+ dev_err(&client->dev, "Couldn't allocate sending buffer.\n");
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ buf[0] = SSD1307FB_DATA;
+ memcpy(buf + 1, cmd, len);
+
+ ret = i2c_master_send(client, buf, len + 1);
+ if (ret != len + 1) {
+ dev_err(&client->dev, "Couldn't send I2C data.\n");
+ goto error;
+ }
+
+ kfree(buf);
+ return 0;
+
+error:
+ kfree(buf);
+ return ret;
+}
+
+static inline int ssd1307fb_write_data(struct i2c_client *client, u8 data)
+{
+ return ssd1307fb_write_data_array(client, &data, 1);
+}
+
+static int ssd1307fb_set(struct i2c_client *client, u8 value)
+{
+ int i, j, ret;
+
+ for (i = 1; i <= (SSD1307FB_HEIGHT / 8); i++) {
+ ret = ssd1307fb_write_cmd(client, SSD1307FB_START_PAGE_ADDRESS + i);
+ if (ret)
+ goto i2c_error;
+
+ ret = ssd1307fb_write_cmd(client, 0x00);
+ if (ret)
+ goto i2c_error;
+
+ ret = ssd1307fb_write_cmd(client, 0x10);
+ if (ret)
+ goto i2c_error;
+
+ for (j = 0; j < SSD1307FB_WIDTH; j++)
+ ssd1307fb_write_data(client, value);
+ }
+
+ return 0;
+
+i2c_error:
+ dev_err(&client->dev, "Couldn't send i2c command: %d\n", ret);
+ return ret;
+}
+
+static void ssd1307fb_update_display(struct ssd1307fb_par *par)
+{
+ u8 *vmem = par->info->screen_base;
+ int i, j, k;
+
+ /*
+ * A page is 8 bit large and covers all the line. In order to
+ * update the screen, you have to send a byte, each byte
+ * containing a bit per pixel to set, on the same column. That
+ * way, by sending a byte, you will only portion of the screen
+ * that has a height of 8 bit and a width of 1, the right-most
+ * bit being on the top of this frame. So we have to do a bit
+ * of magic here...
+ */
+
+ for (i = 0; i < (SSD1307FB_HEIGHT / 8); i++) {
+ ssd1307fb_write_cmd(par->client, SSD1307FB_START_PAGE_ADDRESS + (i + 1));
+ ssd1307fb_write_cmd(par->client, 0x00);
+ ssd1307fb_write_cmd(par->client, 0x10);
+
+ for (j = 0; j < SSD1307FB_WIDTH; j++) {
+ u8 buf = 0;
+ for (k = 0; k < 8; k++) {
+ u32 page_length = SSD1307FB_WIDTH * i;
+ u32 index = page_length + (SSD1307FB_WIDTH * k + j) / 8;
+ u8 byte = *(vmem + index);
+ u8 bit = byte & (1 << (7 - (j % 8)));
+ bit = bit >> (7 - (j % 8));
+ buf |= bit << k;
+ }
+ ssd1307fb_write_data(par->client, buf);
+ }
+ }
+
+ return;
+}
+
+
+static ssize_t ssd1307fb_write(struct fb_info *info, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct ssd1307fb_par *par = info->par;
+ unsigned long p = *ppos;
+ void *dst;
+ int err = 0;
+
+ dst = (void __force *) (info->screen_base + p);
+
+ if (copy_from_user(dst, buf, count))
+ err = -EFAULT;
+
+ if (!err)
+ *ppos += count;
+
+ ssd1307fb_update_display(par);
+
+ return (err) ? err : count;
+}
+
+static void ssd1307fb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
+{
+ struct ssd1307fb_par *par = info->par;
+ sys_fillrect(info, rect);
+ ssd1307fb_update_display(par);
+}
+
+static void ssd1307fb_copyarea(struct fb_info *info, const struct fb_copyarea *area)
+{
+ struct ssd1307fb_par *par = info->par;
+ sys_copyarea(info, area);
+ ssd1307fb_update_display(par);
+}
+
+static void ssd1307fb_imageblit(struct fb_info *info, const struct fb_image *image)
+{
+ struct ssd1307fb_par *par = info->par;
+ sys_imageblit(info, image);
+ ssd1307fb_update_display(par);
+}
+
+static struct fb_ops ssd1307fb_ops = {
+ .owner = THIS_MODULE,
+ .fb_read = fb_sys_read,
+ .fb_write = ssd1307fb_write,
+ .fb_fillrect = ssd1307fb_fillrect,
+ .fb_copyarea = ssd1307fb_copyarea,
+ .fb_imageblit = ssd1307fb_imageblit,
+};
+
+static void ssd1307fb_deferred_io(struct fb_info *info,
+ struct list_head *pagelist)
+{
+ ssd1307fb_update_display(info->par);
+}
+
+static struct fb_deferred_io ssd1307fb_defio = {
+ .delay = HZ,
+ .deferred_io = ssd1307fb_deferred_io,
+};
+
+static int __devinit ssd1307fb_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+ struct fb_info *info;
+ u32 vmem_size = SSD1307FB_WIDTH * SSD1307FB_HEIGHT / 8;
+ struct ssd1307fb_par *par;
+ u8 *vmem;
+ int ret;
+
+ if (!client->dev.of_node) {
+ dev_err(&client->dev, "No device tree data found!\n");
+ ret = -EINVAL;
+ goto generic_error;
+ }
+
+ info = framebuffer_alloc(sizeof(struct ssd1307fb_par), &client->dev);
+ if (!info) {
+ ret = -ENOMEM;
+ goto generic_error;
+ }
+
+ vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
+
+ info->fbops = &ssd1307fb_ops;
+ info->fix = ssd1307fb_fix;
+ info->fbdefio = &ssd1307fb_defio;
+
+ info->var = ssd1307fb_var;
+ info->var.red.length = 1;
+ info->var.red.offset = 0;
+ info->var.green.length = 1;
+ info->var.green.offset = 0;
+ info->var.blue.length = 1;
+ info->var.blue.offset = 0;
+
+ info->screen_base = (u8 __force __iomem *)vmem;
+ info->fix.smem_len = vmem_size;
+
+ fb_deferred_io_init(info);
+
+ par = info->par;
+ par->info = info;
+ par->client = client;
+
+ par->reset = of_get_named_gpio(client->dev.of_node,
+ "oled-reset-gpios", 0);
+ if (gpio_is_valid(par->reset)) {
+ int flags = GPIOF_OUT_INIT_HIGH;
+ if (of_get_property(client->dev.of_node,
+ "reset-active-low", NULL))
+ flags = GPIOF_OUT_INIT_LOW;
+ ret = devm_gpio_request_one(&client->dev, par->reset,
+ flags, "oled-reset");
+ if (ret) {
+ dev_err(&client->dev,
+ "failed to request gpio %d: %d\n",
+ par->reset, ret);
+ goto reset_oled_error;
+ }
+ }
+
+ par->pwm = pwm_get(&client->dev, NULL);
+ if (IS_ERR(par->pwm)) {
+ dev_err(&client->dev, "Could not get PWM from device tree!\n");
+ ret = PTR_ERR(par->pwm);
+ goto pwm_error;
+ }
+
+ par->pwm_period = pwm_get_period(par->pwm);
+
+ dev_dbg(&client->dev, "Using PWM%d with a %dns period.\n", par->pwm->pwm, par->pwm_period);
+
+ ret = register_framebuffer(info);
+ if (ret) {
+ dev_err(&client->dev, "Couldn't register the framebuffer\n");
+ goto fbreg_error;
+ }
+
+ i2c_set_clientdata(client, info);
+
+ /* Reset the screen */
+ gpio_set_value(par->reset, 1);
+ udelay(4);
+ gpio_set_value(par->reset, 0);
+ udelay(4);
+
+ /* Enable the PWM */
+ pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
+ pwm_enable(par->pwm);
+
+ /* Map column 127 of the OLED to segment 0 */
+ ret = ssd1307fb_write_cmd(client, SSD1307FB_SEG_REMAP_ON);
+ if (ret) {
+ dev_err(&client->dev, "Couldn't remap the screen.\n");
+ goto remap_error;
+ }
+
+ /* Turn on the display */
+ ret = ssd1307fb_write_cmd(client, SSD1307FB_DISPLAY_ON);
+ if (ret) {
+ dev_err(&client->dev, "Couldn't turn the display on.\n");
+ goto remap_error;
+ }
+
+ dev_info(&client->dev, "fb%d: %s framebuffer device registered, using %d bytes of video memory\n", info->node, info->fix.id, vmem_size);
+
+ return 0;
+
+remap_error:
+ unregister_framebuffer(info);
+ pwm_disable(par->pwm);
+fbreg_error:
+ pwm_put(par->pwm);
+pwm_error:
+reset_oled_error:
+ fb_deferred_io_cleanup(info);
+ framebuffer_release(info);
+generic_error:
+ return ret;
+}
+
+static int __devexit ssd1307fb_remove(struct i2c_client *client)
+{
+ struct fb_info *info = i2c_get_clientdata(client);
+ struct ssd1307fb_par *par = info->par;
+ unregister_framebuffer(info);
+ pwm_disable(par->pwm);
+ pwm_put(par->pwm);
+ fb_deferred_io_cleanup(info);
+ framebuffer_release(info);
+
+ return 0;
+}
+
+static const struct i2c_device_id ssd1307fb_i2c_id[] = {
+ { "ssd1307fb", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, ssd1307fb_i2c_id);
+
+static const struct of_device_id ssd1307fb_of_match[] = {
+ { .compatible = "solomon,ssd1307fb-i2c" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ssd1307fb_of_match);
+
+static struct i2c_driver ssd1307fb_driver = {
+ .probe = ssd1307fb_probe,
+ .remove = __devexit_p(ssd1307fb_remove),
+ .id_table = ssd1307fb_i2c_id,
+ .driver = {
+ .name = "ssd1307fb",
+ .of_match_table = of_match_ptr(ssd1307fb_of_match),
+ .owner = THIS_MODULE,
+ },
+};
+
+module_i2c_driver(ssd1307fb_driver);
+
+MODULE_DESCRIPTION("FB driver for the Solomon SSD1307 OLED controler");
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
+MODULE_LICENSE("GPL");
--
1.7.9.5
^ permalink raw reply related
* [PATCH 0/4] Add support for the OLED in the CFA10036
From: Maxime Ripard @ 2012-07-17 15:59 UTC (permalink / raw)
To: linux-arm-kernel
Hi everyone,
This patchset adds support for the solomon SSD1307 OLED controller present
in the CFA-10036 board.
It first adds the framebuffer driver for this controller, and then the
needed bits to enable it in the cfa10036 dts.
Thanks,
Maxime
Maxime Ripard (4):
video: Add support for the Solomon SSD1307 OLED Controller
ARM: dts: mxs: Add alternative I2C muxing options for imx28
ARM: dts: mxs: Add pwm4 muxing options for imx28
ARM: dts: mxs: add oled support for the cfa-10036
.../devicetree/bindings/video/ssd1307fb.txt | 24 ++
arch/arm/boot/dts/imx28-cfa10036.dts | 20 +
arch/arm/boot/dts/imx28.dtsi | 17 +
drivers/video/Kconfig | 14 +
drivers/video/Makefile | 1 +
drivers/video/ssd1307fb.c | 414 ++++++++++++++++++++
6 files changed, 490 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/ssd1307fb.txt
create mode 100644 drivers/video/ssd1307fb.c
--
1.7.9.5
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Mark Brown @ 2012-07-17 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50057514.5070501@nvidia.com>
On Tue, Jul 17, 2012 at 07:52:12PM +0530, Prashant Gaikwad wrote:
> >What happens if there's more than one supply that needs to be varied?
> Each rail's dvfs-table will have OPP nodes defined for different
> voltages and each OPP node contains frequency for all clocks
> affecting that rail.
Right, but some systems have operating points which cover a combination
of voltages and frequencies in one operating point. Your binding seems
to define operating points per supply with no tie in between supplies.
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Prashant Gaikwad @ 2012-07-17 14:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120717132032.GD27595@sirena.org.uk>
On Tuesday 17 July 2012 06:50 PM, Mark Brown wrote:
> On Tue, Jul 17, 2012 at 06:07:23PM +0530, Prashant Gaikwad wrote:
>> On Tuesday 17 July 2012 12:06 AM, Turquette, Mike wrote:
>> reg : operating voltage in microvolt
> What happens if there's more than one supply that needs to be varied?
Each rail's dvfs-table will have OPP nodes defined for different
voltages and each OPP node contains frequency for all clocks affecting
that rail.
Just for presentation:
In following example when <&ref 1> clock rate is set to 500000, sm0 need
to operate at 750000000 micrvolt and sm1 at 800000000.
dvfs-rail-1 {
reg_id = <&sm0>;
opp@750000000 {
frequency-table@102 {
frequencies = <&osc 0 314000>, <&ref 1 500000>;
};
};
};
dvfs-rail-2 {
reg_id = <&sm1>;
opp@800000000 {
frequency-table@102 {
frequencies = <&ref 1 5000000>;
};
};
};
>> tolerance : can be used to calculate required voltage. (optional,
>> can be replaced by other relevant parameter to calculate required
>> voltage)
> What are the semantics of this field?
I used "tolerance" just for example to derive the range of voltage.
May be as done for OMAP,
regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Mark Brown @ 2012-07-17 13:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50055C83.7060700@nvidia.com>
On Tue, Jul 17, 2012 at 06:07:23PM +0530, Prashant Gaikwad wrote:
> On Tuesday 17 July 2012 12:06 AM, Turquette, Mike wrote:
> reg : operating voltage in microvolt
What happens if there's more than one supply that needs to be varied?
> tolerance : can be used to calculate required voltage. (optional,
> can be replaced by other relevant parameter to calculate required
> voltage)
What are the semantics of this field?
^ permalink raw reply
* Re: [PATCH v2 11/11] MAINTAINERS: add fblog entry
From: David Herrmann @ 2012-07-17 13:15 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linux-serial, linux-kernel, florianschandinat, linux-fbdev,
gregkh, alan, bonbons
In-Reply-To: <CAMuHMdXFfMv-DV-89nueyDJ5AvFN3AXR_6ysWOfCMDFL46s-Cg@mail.gmail.com>
Hi Geert
On Tue, Jul 17, 2012 at 2:57 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Jul 9, 2012 at 8:38 PM, David Herrmann
> <dh.herrmann@googlemail.com> wrote:
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index ae8fe46..249b02a 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -2854,6 +2854,12 @@ F: drivers/video/
>>>> F: include/video/
>>>> F: include/linux/fb.h
>>>>
>>>> +FRAMEBUFFER LOG DRIVER
>>>> +M: David Herrmann <dh.herrmann@googlemail.com>
>>>> +L: linux-serial@vger.kernel.org
>>>
>>> Why linux-serial, and not linux-fbdev?
>>
>> I thought fbcon was maintained on linux-serial as it is very related
>
> It has always been maintained on linux-fbdev.
Thanks! Then I will change this to linux-fbdev.
Regards
David
^ permalink raw reply
* Re: [PATCH v2 11/11] MAINTAINERS: add fblog entry
From: Geert Uytterhoeven @ 2012-07-17 12:57 UTC (permalink / raw)
To: David Herrmann
Cc: linux-serial, linux-kernel, florianschandinat, linux-fbdev,
gregkh, alan, bonbons
In-Reply-To: <CANq1E4RyFpqxH=uG+dDA1=kfc3r8rCoEesZ31op9rCLjxyk6mQ@mail.gmail.com>
On Mon, Jul 9, 2012 at 8:38 PM, David Herrmann
<dh.herrmann@googlemail.com> wrote:
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index ae8fe46..249b02a 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -2854,6 +2854,12 @@ F: drivers/video/
>>> F: include/video/
>>> F: include/linux/fb.h
>>>
>>> +FRAMEBUFFER LOG DRIVER
>>> +M: David Herrmann <dh.herrmann@googlemail.com>
>>> +L: linux-serial@vger.kernel.org
>>
>> Why linux-serial, and not linux-fbdev?
>
> I thought fbcon was maintained on linux-serial as it is very related
It has always been maintained on linux-fbdev.
> to the vt/tty layer. If linux-fbdev is the better place, I can move
> this. I CC'ed both lists, anyway.
> I have actually no idea where it fits in best.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Prashant Gaikwad @ 2012-07-17 12:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJOA=zMKE1Z_AsYeWr22k58EDskx0eshEjv9VbLQHKFsA0uThw@mail.gmail.com>
On Tuesday 17 July 2012 12:06 AM, Turquette, Mike wrote:
> On Sun, Jul 15, 2012 at 4:42 PM, Rob Herring<robherring2@gmail.com> wrote:
>> On 07/11/2012 11:08 PM, Prashant Gaikwad wrote:
>>> On Wednesday 11 July 2012 07:33 PM, Rob Herring wrote:
>>>> On 07/11/2012 07:56 AM, Prashant Gaikwad wrote:
>>>>> cpu-dvfs-table : dvfs-table {
>>>> This should be located with the node that the frequencies correspond to.
>>>>
>>> With CAR node?
>> With the power domain it corresponds to or the cpu nodes.
>>
>>>>> compatible = "nvidia,tegra30-dvfs-table";
>>>>> reg_id =<&sm0>;
>>>>> #address-cells =<1>;
>>>>> #size-cells =<0>;
>>>>> voltage-array =<750 775 800 825 850 875 900 925 950 975
>>>>> 1000 1025 1050 1100 1125>;
>>>> The SOC is really characterized at all these voltages?
>>> Not really, but different processes of single SoC are characterized for
>>> different voltages and this array covers all those voltages.
>>>
>>>>> };
>>>>>
>>>>> device {
>>>>> dvfs =<&cpu-dvfs-table>;
>>>>> frequency-table@102 {
>>>>> reg =<0x102>;
>>>>> frequencies =<314 314 314 456 456 456 608 608 608
>>>>> 760 817 817 912 1000>;
>>>> I don't see the point of repeating frequencies.
>>>>> };
>>>>> frequency-table@002 {
>>>>> reg =<0x002>;
>>>>> frequencies =<598 598 750 750 893 893 1000>;
>>>>> };
>>>> How do you determine the voltage for a frequency on table 2?
>>>>
>>>> I'd expect a single property with freq/volt pairs or 2 properties for
>>>> freq and voltage where there is a 1:1 relationship (freq N uses
>>>> voltage N).
>>>
>>> How this will work:
>>>
>>> voltage-array =<750 775 800 825 850 875 900 925 950 975 1000 1025 1050
>>> 1100 1125>
>>> frequencies-1 =<314 314 314 456 456 456 608 608 608 760 817 817 912
>>> 1000>;
>>> frequencies-2 =<598 598 750 750 893 893 1000>;
>>>
>> I don't see the point trying to share a voltage range. Not sharing it is
>> fewer array elements (22 vs 36):
>>
>> voltage-array-1 =<750 825 900 975 1000 1050 1100>;
>> frequencies-1 =<314 456 608 760 817 912 1000>;
>>
>> voltage-array-2 =<750 800 850 900>
>> frequencies-2 =<598 750 893 1000>;
>>
> This is significantly more readable.
Instead of voltage array, I was thinking of following approach to
represent operating points for DVFS
reg : operating voltage in microvolt
tolerance : can be used to calculate required voltage. (optional, can be
replaced by other relevant parameter to calculate required voltage)
frequencies : Array of phandle, clock specifier and frequency for all
the clocks related to this rail.
opp@750000000 {
reg = <750000000>;
tolerance = <4>;
frequency-table@102 {
reg = <0x102>;
frequencies = <&osc 0 314000>, <&ref 1 500000>;
};
};
opp@800000000 {
reg = <800000000>;
tolerance = <4>;
frequency-table@102 {
reg = <0x102>;
frequencies = <&osc 0 456000>, <&ref 1 608000>;
};
frequency-table@002 {
reg = <0x002>;
frequencies = <&osc 0 400000>, <&ref 1 560000>;
};
};
It represents:
- 1:1 mapping for voltage/frequency pair.
- Voltage can be represented as range.
- relationships between clock domain and rail.
Only issue I see is, if there are large number of operating points it
will increase data in DT.
Any suggestions?
>
> Regards,
> Mike
>
>> Rob
>>
>>> Freq and voltage has 1:1 relationship but as single voltage table is
>>> used for different processes we have more entries in voltage table than
>>> freq table.
>>> Frequency table 1 is mapped till 1100mV while frequency table 2 is
>>> mapped till 900mV only, it maintains 1:1 relationship.
>>>
>>> About repeating frequencies, operating voltage for a frequency would be
>>> the highest one mapped in the table.
>>> For example, in frequency table 2 operating voltage for 750MHz would be
>>> 825mV while for 893MHz it would be 875mV. Unmapped entries could be
>>> replaced with 0 to make reading better.
>>>
>>> Advantage it provides is single voltage table used for multiple
>>> frequency tables, as can be observed from above tables, operating
>>> voltage for 314MHz in freq table 1 is 800mV while there is no frequency
>>> in table 2 at that voltage.
>>>
>>> I know this makes reading difficult but it provides flexibility,
>>>
>>> I hope it explains the implementation.
>>>
>>>> Rob
>>>>
>>>>> };
>>>>>
>>>>> Thanks& Regards,
>>>>> Prashant G
>>>>>
>>>>> _______________________________________________
>>>>> linux-arm-kernel mailing list
>>>>> linux-arm-kernel@lists.infradead.org
>>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
^ permalink raw reply
* Re: dma-buf/fbdev: one-to-many support
From: David Herrmann @ 2012-07-17 12:23 UTC (permalink / raw)
To: Alan Cox
Cc: Laurent Pinchart, linux-fbdev, Sumit Semwal, dri-devel,
linux-media
In-Reply-To: <20120717122449.07489b50@pyramind.ukuu.org.uk>
Hi Laurent and Alan
On Tue, Jul 17, 2012 at 1:24 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> The main issue is that fbdev has been designed with the implicit assumption
>> that an fbdev driver will always own the graphics memory it uses. All
>> components in the stack, from drivers to applications, have been designed
>> around that assumption.
>>
>> We could of course fix this, revamp the fbdev API and turn it into a modern
>> graphics API, but I really wonder whether it would be worth it. DRM has been
>> getting quite a lot of attention lately, especially from embedded developers
>> and vendors, and the trend seems to me like the (Linux) world will gradually
>> move from fbdev to DRM.
>>
>> Please feel free to disagree :-)
>
> I would disagree on the "main issue" bit. All the graphics cards have
> their own formats and cache management rules. Simply sharing a buffer
> doesn't work - which is why all of the extra gloop will be needed.
This is exactly why I suggested adding an "owner" field. A driver
could then check whether the buffer it is supposed to share/takeover
is from a compatible (or even the same) driver/device. If it is not,
it would simply reject using the buffer. Then again, if we have
multiple devices that are incompatible, we are still unable to share
the buffer. So this attempt would only be useful if we have tons of
DisplayLink devices attached that all use the same driver, for
example.
Regarding DRM: In user-space I prefer DRM over fbdev. With the
introduction of the dumb-buffers there isn't even the need to have
mesa installed. However, fblog runs in kernel space and currently
cannot use DRM as there is no in-kernel DRM API. I looked at
drm-fops.c whether it is easy to create a very simple in-kernel API
but then I dropped the idea as this might be too complex for a simple
debugging-only driver. Another attempt would be making the
drm-fb-helper more generic so we can use this layer as in-kernel DRM
API.
I had a deeper look into this this weekend and so as a summary I think
all in-kernel graphics access is probably not worth optimizing it.
fbcon is already working great and fblog is only used during boot and
oopses/panics and can be restricted to a single device. I will have
another look at the drivers in a few weeks but if you tell me that
this is not easy to implement, I will probably have to let this idea
go.
Thanks
David
^ permalink raw reply
* Re: dma-buf/fbdev: one-to-many support
From: Alan Cox @ 2012-07-17 11:24 UTC (permalink / raw)
To: Laurent Pinchart
Cc: David Herrmann, linux-fbdev, Sumit Semwal, dri-devel, linux-media
In-Reply-To: <1497600.3Qx4Vx9if4@avalon>
> The main issue is that fbdev has been designed with the implicit assumption
> that an fbdev driver will always own the graphics memory it uses. All
> components in the stack, from drivers to applications, have been designed
> around that assumption.
>
> We could of course fix this, revamp the fbdev API and turn it into a modern
> graphics API, but I really wonder whether it would be worth it. DRM has been
> getting quite a lot of attention lately, especially from embedded developers
> and vendors, and the trend seems to me like the (Linux) world will gradually
> move from fbdev to DRM.
>
> Please feel free to disagree :-)
I would disagree on the "main issue" bit. All the graphics cards have
their own formats and cache management rules. Simply sharing a buffer
doesn't work - which is why all of the extra gloop will be needed.
Alan
^ permalink raw reply
* Re: dma-buf/fbdev: one-to-many support
From: Laurent Pinchart @ 2012-07-17 10:39 UTC (permalink / raw)
To: David Herrmann; +Cc: dri-devel, Sumit Semwal, linux-media, linux-fbdev
In-Reply-To: <CANq1E4SbippxHHTaqLhpGjJLG12y94kWUFdB7P_EAG14o50vrQ@mail.gmail.com>
Hi David,
On Saturday 14 July 2012 16:10:56 David Herrmann wrote:
> Hi
>
> I am currently working on fblog [1] (a replacement for fbcon without
> VT dependencies) but this questions does also apply to other fbdev
> users. Is there a way to share framebuffers between fbdev devices? I
> was thinking especially of USB devices like DisplayLink. If they share
> the same screen dimensions it would increase performance a lot if I
> could display a single buffer on all the devices instead of copying it
> into each framebuffer.
>
> I was told to have a look at the dma-buf framework to implement this.
> However, looking at the fbdev dma-buf support I think that this isn't
> currently possible. Each fbdev device takes the exporter-role and
> provides a single dma-buf object. However, if I wanted to share the
> buffers, I would need to be the exporter. Or there needs to be a way
> for the fbdev devices to import a dma-buf from other fbdev devices.
>
> I also took a short look at DRM prime support and noticed that it is
> capable of importing buffers (or at least it looks like it is).
> Therefore, I was wondering whether it does make sense to add an
> "import dma-buf" callback to fbdev devices and if the fbdev driver
> supports this, I can simply draw to a single dma-buf from one fbdev
> device and push it to all other fbdev devices that share the same
> dimensions.
The main issue is that fbdev has been designed with the implicit assumption
that an fbdev driver will always own the graphics memory it uses. All
components in the stack, from drivers to applications, have been designed
around that assumption.
We could of course fix this, revamp the fbdev API and turn it into a modern
graphics API, but I really wonder whether it would be worth it. DRM has been
getting quite a lot of attention lately, especially from embedded developers
and vendors, and the trend seems to me like the (Linux) world will gradually
move from fbdev to DRM.
Please feel free to disagree :-)
> It would also be nice to allow multiple buffer-owners or a way to
> transfer ownership. That is, if the owner/exporter of the dma-buf
> vanishes, I would pass it to another fbdev device which would pick it
> up so I don't have to create a new one.
>
> I think this is only interesting for DisplayLink-devices as they are
> currently the only way to get a bunch of displays connected to a
> single machine. Anyway, if you think that this isn't worth it, I will
> probably drop this idea.
>
> Regards
> David
>
> [1] fblog kernel driver: http://lwn.net/Articles/505965/
--
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