* Re: [PATCH v2 00/24] video/da8xx-fb fbdev driver enhance to support TI am335x SoC
From: Tomi Valkeinen @ 2013-07-31 10:04 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1375208791-15781-1-git-send-email-detheridge@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2347 bytes --]
Hi,
On 30/07/13 21:26, Darren Etheridge wrote:
> 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/?submitter=39101
> if you want to see the history.
Comments on the whole series:
Most of the patches are originally from Afzal. I believe some of the
patches are unchanged, but some are changed by you. In cases like this
you should pick one of the following options for each patch:
- If the patch is unchanged, send the patch as it is, having From: Afzal
line there.
- If you have changed the patch, send the patch having From: Afzal line,
but marking in the description that you've changed it (and what you
did). This should be done if the changes are small.
- If you changed a lot in the patch, send the patch with yourself as the
author, signed off by only you, but mention that it's based on Afzal's work.
The point here is that if you change the patch, it's no longer Afzal's
original patch. Afzal hasn't reviewed it, so signed-off-by Afzal is not
correct. You could've introduced horrible bugs in the patch, and I'm
sure Afzal doesn't want to see that a patch in the kernel introducing
horrible bugs is from him (when it is not from him).
Of course, if you have actively discussed the patches with Afzal, and
he's okay with all the changes you've made, then the patches are fine.
Another thing are the DT related patches. They should be sent to
devicetree@vger.kernel.org for review. And I think the DT patches should
be squashed into one, as they are quite short and having them as a whole
makes it easier to look at them. You could probably move the DT patches
to a separate series, so that we can merge the rest of the improvements,
and manage DT separately.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH v2 21/24] video: da8xx-fb: setup struct lcd_ctrl_config for dt
From: Tomi Valkeinen @ 2013-07-31 9:33 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1375208791-15781-22-git-send-email-detheridge@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1615 bytes --]
On 30/07/13 21:26, Darren Etheridge wrote:
> From: Afzal Mohammed <afzal@ti.com>
>
> strcut lcd_ctrl_config information required for driver is currently
> obtained via platform data. To handle DT probing, create
> lcd_ctrl_config and populate it with default values, these values are
> sufficient for the panels so far used with this controller to work.
>
> Signed-off-by: Afzal Mohammed <afzal@ti.com>
> Signed-off-by: Darren Etheridge <detheridge@ti.com>
> ---
> drivers/video/da8xx-fb.c | 34 +++++++++++++++++++++++++++++++++-
> 1 files changed, 33 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 697b07c..fe3d79e 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
<snip>
> static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
> {
> struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
> @@ -1346,7 +1375,10 @@ static int fb_probe(struct platform_device *device)
> break;
> }
>
> - lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
In the previous patch you allow fb_pdata==NULL if booting with DT.
However, this patch shows that fb_pdata is still accessed. So I think
you need to move the previous patch after this to avoid a crash between
this and the previous patch.
> + if (device->dev.of_node)
> + lcd_cfg = da8xx_fb_create_cfg(device);
> + else
> + lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
fb_pdata->controller_data is void *, so you don't need to cast it
explicitly.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [patch] vga16fb: remove an unused variable
From: Jingoo Han @ 2013-07-31 9:02 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20130731085414.GC8210@elgon.mountain>
On Wednesday, July 31, 2013 5:54 PM, Dan Carpenter wrote:
>
> Commit e21d2170f3 "video: remove unnecessary platform_set_drvdata()"
> introduces an unused variable compile warning.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Hi Dan Carpenter,
The same patch was submitted by Luis Henriques 2 weeks ago. :)
Also, the patch was already applied by Tomi Valkeinen.
It will be merged to 3.11-rc4.
Thank you for caring it.
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 */
^ permalink raw reply
* [patch] vga16fb: remove an unused variable
From: Dan Carpenter @ 2013-07-31 8:54 UTC (permalink / raw)
To: linux-fbdev
Commit e21d2170f3 "video: remove unnecessary platform_set_drvdata()"
introduces an unused variable compile warning.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
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 */
^ permalink raw reply related
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Felipe Balbi @ 2013-07-31 6:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <51F8A440.8010803@ti.com>
[-- Attachment #1: Type: text/plain, Size: 3325 bytes --]
Hi,
On Wed, Jul 31, 2013 at 11:14:32AM +0530, Kishon Vijay Abraham I wrote:
> >>>>> 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.
>
> If we are using PLATFORM_DEVID_AUTO, then I dont see any way we can give the
> correct binding information to the PHY framework. I think we can drop having
> this non-dt support in PHY framework? I see only one platform (OMAP3) going to
> be needing this non-dt support and we can use the USB PHY library for it.
you shouldn't drop support for non-DT platform, in any case we lived
without DT (and still do) for years. Gotta find a better way ;-)
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Kishon Vijay Abraham I @ 2013-07-31 5:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130730071106.GC16441@radagast>
Hi,
On Tuesday 30 July 2013 12:41 PM, Felipe Balbi wrote:
> 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.
If we are using PLATFORM_DEVID_AUTO, then I dont see any way we can give the
correct binding information to the PHY framework. I think we can drop having
this non-dt support in PHY framework? I see only one platform (OMAP3) going to
be needing this non-dt support and we can use the USB PHY library for it.
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH] fbdev: fix build warning in vga16fb.c
From: Gu Zheng @ 2013-07-31 3:28 UTC (permalink / raw)
To: Xishi Qiu; +Cc: plagnioj, tomi.valkeinen, linux-fbdev, LKML
In-Reply-To: <51F8829E.4080605@huawei.com>
Hoho, Tomi has applied the patch from Lius to fix this warning.
And this is the sixth patch to fix the same issue since last week.
Thanks,
Gu
On 07/31/2013 11:21 AM, Xishi Qiu wrote:
> When building v3.11-rc3, I get the following warning:
> ...
> drivers/video/vga16fb.c: In function ‘vga16fb_destroy’:
> drivers/video/vga16fb.c:1268: warning: unused variable ‘dev’
> ...
>
> Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
> ---
> drivers/video/vga16fb.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> 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 */
^ permalink raw reply
* [PATCH] fbdev: fix build warning in vga16fb.c
From: Xishi Qiu @ 2013-07-31 3:21 UTC (permalink / raw)
To: plagnioj, tomi.valkeinen, linux-fbdev, LKML
When building v3.11-rc3, I get the following warning:
...
drivers/video/vga16fb.c: In function ‘vga16fb_destroy’:
drivers/video/vga16fb.c:1268: warning: unused variable ‘dev’
...
Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
---
drivers/video/vga16fb.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
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.7.1
^ permalink raw reply related
* [PATCH v2 24/24] video/da8xx-fb adding am33xx as dependency
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
Updating Kconfig to allow am33xx to include lcdc fbdev driver
including some extra dependencies needed by device tree mods.
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/Kconfig | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 2e937bd..8ae6d2a 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2226,15 +2226,16 @@ config FB_SH7760
panels <= 320 pixel horizontal resolution.
config FB_DA8XX
- tristate "DA8xx/OMAP-L1xx Framebuffer support"
- depends on FB && ARCH_DAVINCI_DA8XX
+ tristate "DA8xx/OMAP-L1xx/AM335x Framebuffer support"
+ depends on FB && (ARCH_DAVINCI_DA8XX || SOC_AM33XX)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
select FB_CFB_REV_PIXELS_IN_BYTE
+ select VIDEOMODE_HELPERS
---help---
This is the frame buffer device driver for the TI LCD controller
- found on DA8xx/OMAP-L1xx SoCs.
+ found on DA8xx/OMAP-L1xx/AM335x SoCs.
If unsure, say N.
config FB_VIRTUAL
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 23/24] video: da8xx-fb: make clock naming consistent
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
Clean up the code, so that the names of the various clock variables
are consistent to it is clear what variable is associated with what
clock.
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 471931e..131cf4c 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -179,7 +179,7 @@ struct da8xx_fb_par {
#ifdef CONFIG_CPU_FREQ
struct notifier_block freq_transition;
#endif
- unsigned int lcd_fck_rate;
+ unsigned int lcdc_clk_rate;
void (*panel_power_ctrl)(int);
u32 pseudo_palette[16];
struct fb_videomode mode;
@@ -692,14 +692,14 @@ static int da8xx_fb_config_clk_divider(struct da8xx_fb_par *par,
{
int ret;
- if (par->lcd_fck_rate != lcdc_clk_rate) {
+ if (par->lcdc_clk_rate != lcdc_clk_rate) {
ret = clk_set_rate(par->lcdc_clk, lcdc_clk_rate);
if (IS_ERR_VALUE(ret)) {
dev_err(par->dev,
"unable to set clock rate at %u\n", lcdc_clk_rate);
return ret;
}
- par->lcd_fck_rate = clk_get_rate(par->lcdc_clk);
+ par->lcdc_clk_rate = clk_get_rate(par->lcdc_clk);
}
/* Configure the LCD clock divisor. */
@@ -721,7 +721,7 @@ static unsigned int da8xx_fb_calc_clk_divider(struct da8xx_fb_par *par,
pixclock = PICOS2KHZ(pixclock) * 1000;
- *lcdc_clk_rate = par->lcd_fck_rate;
+ *lcdc_clk_rate = par->lcdc_clk_rate;
if (pixclock < (*lcdc_clk_rate / CLK_MAX_DIV)) {
*lcdc_clk_rate = clk_round_rate(par->lcdc_clk, pixclock * CLK_MAX_DIV);
@@ -1026,8 +1026,8 @@ static int lcd_da8xx_cpufreq_transition(struct notifier_block *nb,
par = container_of(nb, struct da8xx_fb_par, freq_transition);
if (val = CPUFREQ_POSTCHANGE) {
- if (par->lcd_fck_rate != clk_get_rate(par->lcdc_clk)) {
- par->lcd_fck_rate = clk_get_rate(par->lcdc_clk);
+ if (par->lcdc_clk_rate != clk_get_rate(par->lcdc_clk)) {
+ par->lcdc_clk_rate = clk_get_rate(par->lcdc_clk);
lcd_disable_raster(DA8XX_FRAME_WAIT);
da8xx_fb_calc_config_clk_divider(par, &par->mode);
if (par->blank = FB_BLANK_UNBLANK)
@@ -1368,8 +1368,8 @@ static int fb_probe(struct platform_device *device)
struct lcd_ctrl_config *lcd_cfg;
struct fb_videomode *lcdc_info;
struct fb_info *da8xx_fb_info;
- struct clk *fb_clk = NULL;
struct da8xx_fb_par *par;
+ struct clk *tmp_lcdc_clk;
int ret;
unsigned long ulcm;
@@ -1389,8 +1389,8 @@ static int fb_probe(struct platform_device *device)
return -EADDRNOTAVAIL;
}
- fb_clk = devm_clk_get(&device->dev, "fck");
- if (IS_ERR(fb_clk)) {
+ tmp_lcdc_clk = devm_clk_get(&device->dev, "fck");
+ if (IS_ERR(tmp_lcdc_clk)) {
dev_err(&device->dev, "Can not get device clock\n");
return -ENODEV;
}
@@ -1435,8 +1435,8 @@ 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);
+ par->lcdc_clk = tmp_lcdc_clk;
+ par->lcdc_clk_rate = clk_get_rate(par->lcdc_clk);
if (fb_pdata && fb_pdata->panel_power_ctrl) {
par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
par->panel_power_ctrl(1);
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 22/24] video: da8xx-fb: set upstream clock rate (if reqd)
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
LCDC IP has a clock divider to adjust pixel clock, this limits pixel
clock range to fck/255 - fck/2(fck - rate of input clock to LCDC IP).
In the case of AM335x, where this IP is present, default fck is not
sufficient to provide normal pixel clock rates, hence rendering this
driver unusable on AM335x.
If input clock too is configurable, allowable range of pixel clock
would increase. Here initially it is checked whether with present fck,
divider in IP could be configured to obtain required rate, if not,
fck is adjusted. This makes it usable on AM335x.
Note:
Another solution would be to model an inherited basic clock divider of
CCF, an advantage would be a better possible resolution for pixel clk.
And trying to instantiate a CCF clock would mean that to be consistent,
3 bits being turned on to enable clocks of LCDC IP would have to be
modeled as gate clocks. Now that would bring in a total of 4 clocks,
including necessity to create a new inherited divider clock, and that
mean a branch of clock tree would be present in LCDC driver. This
would add complexity to LCDC driver bringing in considerable amount
of clock handling code, and this would not bring in much advantage
for existing use cases other than providing a higher resolution of
pixel clock. And existing use cases work without relying on clock
modeling. Another fact is that out of the two platform's using this
driver DaVinci is not yet converted to CCF. In future if higher
resolution of pixel clock is required, and probably after DaVinci is
CCF'ed, modeling clock nodes inside driver may be considered.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 80 ++++++++++++++++++++++++++++++++++-----------
1 files changed, 60 insertions(+), 20 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index fe3d79e..471931e 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -133,6 +133,9 @@
#define WSI_TIMEOUT 50
#define PALETTE_SIZE 256
+#define CLK_MIN_DIV 2
+#define CLK_MAX_DIV 255
+
static void __iomem *da8xx_fb_reg_base;
static struct resource *lcdc_regs;
static unsigned int lcd_revision;
@@ -684,38 +687,71 @@ static void da8xx_fb_lcd_reset(void)
}
}
-static inline unsigned da8xx_fb_calc_clk_divider(struct da8xx_fb_par *par,
- unsigned pixclock)
-{
- return par->lcd_fck_rate / (PICOS2KHZ(pixclock) * 1000);
-}
-
-static inline unsigned da8xx_fb_round_clk(struct da8xx_fb_par *par,
- unsigned pixclock)
+static int da8xx_fb_config_clk_divider(struct da8xx_fb_par *par,
+ unsigned lcdc_clk_div, unsigned lcdc_clk_rate)
{
- unsigned div;
+ int ret;
- div = da8xx_fb_calc_clk_divider(par, pixclock);
- return KHZ2PICOS(par->lcd_fck_rate / (1000 * div));
-}
+ if (par->lcd_fck_rate != lcdc_clk_rate) {
+ ret = clk_set_rate(par->lcdc_clk, lcdc_clk_rate);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(par->dev,
+ "unable to set clock rate at %u\n", lcdc_clk_rate);
+ return ret;
+ }
+ par->lcd_fck_rate = clk_get_rate(par->lcdc_clk);
+ }
-static inline void da8xx_fb_config_clk_divider(unsigned div)
-{
/* Configure the LCD clock divisor. */
- lcdc_write(LCD_CLK_DIVISOR(div) |
+ lcdc_write(LCD_CLK_DIVISOR(lcdc_clk_div) |
(LCD_RASTER_MODE & 0x1), LCD_CTRL_REG);
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);
+
+ return 0;
+}
+
+static unsigned int da8xx_fb_calc_clk_divider(struct da8xx_fb_par *par,
+ unsigned pixclock,
+ unsigned *lcdc_clk_rate)
+{
+ unsigned lcdc_clk_div;
+
+ pixclock = PICOS2KHZ(pixclock) * 1000;
+
+ *lcdc_clk_rate = par->lcd_fck_rate;
+
+ if (pixclock < (*lcdc_clk_rate / CLK_MAX_DIV)) {
+ *lcdc_clk_rate = clk_round_rate(par->lcdc_clk, pixclock * CLK_MAX_DIV);
+ lcdc_clk_div = CLK_MAX_DIV;
+ } else if (pixclock > (*lcdc_clk_rate / CLK_MIN_DIV)) {
+ *lcdc_clk_rate = clk_round_rate(par->lcdc_clk, pixclock * CLK_MIN_DIV);
+ lcdc_clk_div = CLK_MIN_DIV;
+ } else {
+ lcdc_clk_div = *lcdc_clk_rate / pixclock;
+ }
+
+ return lcdc_clk_div;
}
-static inline void da8xx_fb_calc_config_clk_divider(struct da8xx_fb_par *par,
- struct fb_videomode *mode)
+static int 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);
+ unsigned lcdc_clk_rate;
+ unsigned lcdc_clk_div = da8xx_fb_calc_clk_divider(par, mode->pixclock, &lcdc_clk_rate);
- da8xx_fb_config_clk_divider(div);
+ return da8xx_fb_config_clk_divider(par, lcdc_clk_div, lcdc_clk_rate);
+}
+
+static inline unsigned da8xx_fb_round_clk(struct da8xx_fb_par *par,
+ unsigned pixclock)
+{
+ unsigned lcdc_clk_div, lcdc_clk_rate;
+
+ lcdc_clk_div = da8xx_fb_calc_clk_divider(par, pixclock, &lcdc_clk_rate);
+ return KHZ2PICOS(lcdc_clk_rate / (1000 * lcdc_clk_div));
}
static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
@@ -724,7 +760,11 @@ static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
u32 bpp;
int ret = 0;
- da8xx_fb_calc_config_clk_divider(par, panel);
+ ret = da8xx_fb_calc_config_clk_divider(par, panel);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(par->dev, "unable to configure clock\n");
+ return ret;
+ }
if (panel->sync & FB_SYNC_CLK_INVERT)
lcdc_write((lcdc_read(LCD_RASTER_TIMING_2_REG) |
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 21/24] video: da8xx-fb: setup struct lcd_ctrl_config for dt
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
strcut lcd_ctrl_config information required for driver is currently
obtained via platform data. To handle DT probing, create
lcd_ctrl_config and populate it with default values, these values are
sufficient for the panels so far used with this controller to work.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 34 +++++++++++++++++++++++++++++++++-
1 files changed, 33 insertions(+), 1 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 697b07c..fe3d79e 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1255,6 +1255,35 @@ static struct fb_ops da8xx_fb_ops = {
.fb_blank = cfb_blank,
};
+static struct lcd_ctrl_config *da8xx_fb_create_cfg(struct platform_device *dev)
+{
+ struct lcd_ctrl_config *cfg;
+
+ cfg = devm_kzalloc(&dev->dev, sizeof(struct fb_videomode), GFP_KERNEL);
+ if (!cfg) {
+ dev_err(&dev->dev, "memory allocation failed\n");
+ return NULL;
+ }
+
+ /* default values */
+
+ if (lcd_revision = LCD_VERSION_1)
+ cfg->bpp = 16;
+ else
+ cfg->bpp = 32;
+
+ /*
+ * For panels so far used with this LCDC, below statement is sufficient.
+ * For new panels, if required, struct lcd_ctrl_cfg fields to be updated
+ * with additional/modified values. Those values would have to be then
+ * obtained from dt(requiring new dt bindings).
+ */
+
+ cfg->panel_shade = COLOR_ACTIVE;
+
+ return cfg;
+}
+
static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
{
struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
@@ -1346,7 +1375,10 @@ static int fb_probe(struct platform_device *device)
break;
}
- lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
+ if (device->dev.of_node)
+ lcd_cfg = da8xx_fb_create_cfg(device);
+ else
+ lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
if (!lcd_cfg) {
ret = -EINVAL;
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 20/24] video: da8xx-fb: ensure pdata only for non-dt
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
This driver is DT probe-able, hence ensure presence of platform data
only for non-DT boot.
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, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index ca8f6fc..697b07c 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1304,7 +1304,7 @@ static int fb_probe(struct platform_device *device)
int ret;
unsigned long ulcm;
- if (fb_pdata = NULL) {
+ if (fb_pdata = NULL && !device->dev.of_node) {
dev_err(&device->dev, "Can not get platform data\n");
return -ENOENT;
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 19/24] video: da8xx-fb: obtain fb_videomode info from dt
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
Obtain fb_videomode details for the connected lcd panel using the
display timing details present in DT.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
.../devicetree/bindings/video/fb-da8xx.txt | 21 ++++++++++++++++++++
drivers/video/da8xx-fb.c | 17 ++++++++++++++++
2 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/video/fb-da8xx.txt b/Documentation/devicetree/bindings/video/fb-da8xx.txt
index 581e014..0741f78 100644
--- a/Documentation/devicetree/bindings/video/fb-da8xx.txt
+++ b/Documentation/devicetree/bindings/video/fb-da8xx.txt
@@ -6,6 +6,12 @@ Required properties:
AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc"
- reg: Address range of lcdc register set
- interrupts: lcdc interrupt
+- display-timings: typical videomode of lcd panel, represented as child.
+ Refer Documentation/devicetree/bindings/video/display-timing.txt for
+ display timing binding details. If multiple videomodes are mentioned
+ in display timings node, typical videomode has to be mentioned as the
+ native mode or it has to be first child (driver cares only for native
+ videomode).
Example:
@@ -13,4 +19,19 @@ lcdc@4830e000 {
compatible = "ti,am3352-lcdc", "ti,da830-lcdc";
reg = <0x4830e000 0x1000>;
interrupts = <36>;
+ display-timings {
+ 800x480p62 {
+ clock-frequency = <30000000>;
+ hactive = <800>;
+ vactive = <480>;
+ hfront-porch = <39>;
+ hback-porch = <39>;
+ hsync-len = <47>;
+ vback-porch = <29>;
+ vfront-porch = <13>;
+ vsync-len = <2>;
+ hsync-active = <1>;
+ vsync-active = <1>;
+ };
+ };
};
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 97d6b2d..ca8f6fc 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -36,6 +36,7 @@
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/lcm.h>
+#include <video/of_display_timing.h>
#include <video/da8xx-fb.h>
#include <asm/div64.h>
@@ -1258,8 +1259,24 @@ static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
{
struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
struct fb_videomode *lcdc_info;
+ struct device_node *np = dev->dev.of_node;
int i;
+ if (np) {
+ lcdc_info = devm_kzalloc(&dev->dev,
+ sizeof(struct fb_videomode),
+ GFP_KERNEL);
+ if (!lcdc_info) {
+ dev_err(&dev->dev, "memory allocation failed\n");
+ return NULL;
+ }
+ if (of_get_fb_videomode(np, lcdc_info, OF_USE_NATIVE_MODE)) {
+ dev_err(&dev->dev, "timings not available in DT\n");
+ return NULL;
+ }
+ return lcdc_info;
+ }
+
for (i = 0, lcdc_info = known_lcd_panels;
i < ARRAY_SIZE(known_lcd_panels); i++, lcdc_info++) {
if (strcmp(fb_pdata->type, lcdc_info->name) = 0)
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 18/24] video: da8xx-fb: minimal dt support
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
Driver is provided a means to have the probe triggered by DT.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
.../devicetree/bindings/video/fb-da8xx.txt | 16 ++++++++++++++++
drivers/video/da8xx-fb.c | 7 +++++++
2 files changed, 23 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt
diff --git a/Documentation/devicetree/bindings/video/fb-da8xx.txt b/Documentation/devicetree/bindings/video/fb-da8xx.txt
new file mode 100644
index 0000000..581e014
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/fb-da8xx.txt
@@ -0,0 +1,16 @@
+TI LCD Controller on DA830/DA850/AM335x SoC's
+
+Required properties:
+- compatible:
+ DA830 - "ti,da830-lcdc"
+ AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc"
+- reg: Address range of lcdc register set
+- interrupts: lcdc interrupt
+
+Example:
+
+lcdc@4830e000 {
+ compatible = "ti,am3352-lcdc", "ti,da830-lcdc";
+ reg = <0x4830e000 0x1000>;
+ interrupts = <36>;
+};
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 983176a..97d6b2d 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1596,6 +1596,12 @@ static int fb_resume(struct platform_device *dev)
#define fb_resume NULL
#endif
+static const struct of_device_id da8xx_fb_of_match[] = {
+ {.compatible = "ti,da830-lcdc", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, da8xx_fb_of_match);
+
static struct platform_driver da8xx_fb_driver = {
.probe = fb_probe,
.remove = fb_remove,
@@ -1604,6 +1610,7 @@ static struct platform_driver da8xx_fb_driver = {
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(da8xx_fb_of_match),
},
};
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 17/24] video: da8xx-fb: invoke platform callback safely
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
Ensure that platform data is present before checking whether platform
callback is present (the one used to control backlight). So far this
was not an issue as driver was purely non-DT triggered, but now DT
support has been added.
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, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 1787ccc..983176a 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1348,7 +1348,7 @@ static int fb_probe(struct platform_device *device)
par->dev = &device->dev;
par->lcdc_clk = fb_clk;
par->lcd_fck_rate = clk_get_rate(fb_clk);
- if (fb_pdata->panel_power_ctrl) {
+ if (fb_pdata && fb_pdata->panel_power_ctrl) {
par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
par->panel_power_ctrl(1);
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 16/24] video: da8xx-fb: reorganize panel detection
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
Move panel detection to a separate function, this helps in readability
as well as makes DT support cleaner.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 42 ++++++++++++++++++++++++++----------------
1 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 10af07b..1787ccc 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1254,6 +1254,27 @@ static struct fb_ops da8xx_fb_ops = {
.fb_blank = cfb_blank,
};
+static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
+{
+ struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
+ struct fb_videomode *lcdc_info;
+ int i;
+
+ for (i = 0, lcdc_info = known_lcd_panels;
+ i < ARRAY_SIZE(known_lcd_panels); i++, lcdc_info++) {
+ if (strcmp(fb_pdata->type, lcdc_info->name) = 0)
+ break;
+ }
+
+ if (i = ARRAY_SIZE(known_lcd_panels)) {
+ dev_err(&dev->dev, "no panel found\n");
+ return NULL;
+ }
+ dev_info(&dev->dev, "found %s panel\n", lcdc_info->name);
+
+ return lcdc_info;
+}
+
static int fb_probe(struct platform_device *device)
{
struct da8xx_lcdc_platform_data *fb_pdata @@ -1263,7 +1284,7 @@ static int fb_probe(struct platform_device *device)
struct fb_info *da8xx_fb_info;
struct clk *fb_clk = NULL;
struct da8xx_fb_par *par;
- int ret, i;
+ int ret;
unsigned long ulcm;
if (fb_pdata = NULL) {
@@ -1271,6 +1292,10 @@ static int fb_probe(struct platform_device *device)
return -ENOENT;
}
+ lcdc_info = da8xx_fb_get_videomode(device);
+ if (lcdc_info = NULL)
+ return -ENODEV;
+
lcdc_regs = platform_get_resource(device, IORESOURCE_MEM, 0);
da8xx_fb_reg_base = devm_request_and_ioremap(&device->dev, lcdc_regs);
if (!da8xx_fb_reg_base) {
@@ -1304,21 +1329,6 @@ static int fb_probe(struct platform_device *device)
break;
}
- for (i = 0, lcdc_info = known_lcd_panels;
- i < ARRAY_SIZE(known_lcd_panels);
- i++, lcdc_info++) {
- if (strcmp(fb_pdata->type, lcdc_info->name) = 0)
- break;
- }
-
- if (i = ARRAY_SIZE(known_lcd_panels)) {
- dev_err(&device->dev, "GLCD: No valid panel found\n");
- ret = -ENODEV;
- goto err_pm_runtime_disable;
- } else
- dev_info(&device->dev, "GLCD: Found %s panel\n",
- fb_pdata->type);
-
lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
if (!lcd_cfg) {
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 15/24] video: da8xx-fb: ensure non-null cfg in pdata
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
Ensure that platform data contains pointer for lcd_ctrl_config.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 61150f9..10af07b 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1321,6 +1321,11 @@ static int fb_probe(struct platform_device *device)
lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
+ if (!lcd_cfg) {
+ ret = -EINVAL;
+ goto err_pm_runtime_disable;
+ }
+
da8xx_fb_info = framebuffer_alloc(sizeof(struct da8xx_fb_par),
&device->dev);
if (!da8xx_fb_info) {
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 14/24] video: da8xx-fb: use devres
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
Replace existing resource handling in the driver with managed device
resource.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 35 ++++++-----------------------------
1 files changed, 6 insertions(+), 29 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index dec333e..61150f9 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1037,12 +1037,9 @@ static int fb_remove(struct platform_device *dev)
par->p_palette_base);
dma_free_coherent(NULL, par->vram_size, par->vram_virt,
par->vram_phys);
- free_irq(par->irq, par);
pm_runtime_put_sync(&dev->dev);
pm_runtime_disable(&dev->dev);
framebuffer_release(info);
- iounmap(da8xx_fb_reg_base);
- release_mem_region(lcdc_regs->start, resource_size(lcdc_regs));
}
return 0;
@@ -1266,7 +1263,6 @@ static int fb_probe(struct platform_device *device)
struct fb_info *da8xx_fb_info;
struct clk *fb_clk = NULL;
struct da8xx_fb_par *par;
- resource_size_t len;
int ret, i;
unsigned long ulcm;
@@ -1276,29 +1272,16 @@ static int fb_probe(struct platform_device *device)
}
lcdc_regs = platform_get_resource(device, IORESOURCE_MEM, 0);
- if (!lcdc_regs) {
- dev_err(&device->dev,
- "Can not get memory resource for LCD controller\n");
- return -ENOENT;
- }
-
- len = resource_size(lcdc_regs);
-
- lcdc_regs = request_mem_region(lcdc_regs->start, len, lcdc_regs->name);
- if (!lcdc_regs)
- return -EBUSY;
-
- da8xx_fb_reg_base = ioremap(lcdc_regs->start, len);
+ da8xx_fb_reg_base = devm_request_and_ioremap(&device->dev, lcdc_regs);
if (!da8xx_fb_reg_base) {
- ret = -EBUSY;
- goto err_request_mem;
+ dev_err(&device->dev, "memory resource setup failed\n");
+ return -EADDRNOTAVAIL;
}
- fb_clk = clk_get(&device->dev, "fck");
+ fb_clk = devm_clk_get(&device->dev, "fck");
if (IS_ERR(fb_clk)) {
dev_err(&device->dev, "Can not get device clock\n");
- ret = -ENODEV;
- goto err_ioremap;
+ return -ENODEV;
}
pm_runtime_enable(&device->dev);
@@ -1459,7 +1442,7 @@ static int fb_probe(struct platform_device *device)
lcdc_irq_handler = lcdc_irq_handler_rev02;
}
- ret = request_irq(par->irq, lcdc_irq_handler, 0,
+ ret = devm_request_irq(&device->dev, par->irq, lcdc_irq_handler, 0,
DRIVER_NAME, par);
if (ret)
goto irq_freq;
@@ -1489,12 +1472,6 @@ err_pm_runtime_disable:
pm_runtime_put_sync(&device->dev);
pm_runtime_disable(&device->dev);
-err_ioremap:
- iounmap(da8xx_fb_reg_base);
-
-err_request_mem:
- release_mem_region(lcdc_regs->start, len);
-
return ret;
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 13/24] video: da8xx-fb: enable sync lost intr for v2 ip
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
The interrupt handler explicitly has code that handles the sync lost
interrupt. However the sync lost interrupt is never actually being
enabled in the LCD controller, therefore this interrupt code path is not
being exercised. This fix simply enables the generation of the sync
lost interrupt by the LCD controller so it can be dealt with
appropriately by the interrupt handler.
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, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index e172a4a..dec333e 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -318,7 +318,7 @@ static void lcd_blit(int load_mode, struct da8xx_fb_par *par)
reg_int = lcdc_read(LCD_INT_ENABLE_SET_REG) |
LCD_V2_END_OF_FRAME0_INT_ENA |
LCD_V2_END_OF_FRAME1_INT_ENA |
- LCD_FRAME_DONE;
+ LCD_FRAME_DONE | LCD_SYNC_LOST;
lcdc_write(reg_int, LCD_INT_ENABLE_SET_REG);
}
reg_dma |= LCD_DUAL_FRAME_BUFFER_ENABLE;
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 12/24] video: da8xx-fb: fix 24bpp raster configuration
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
From: Manjunathappa, Prakash <prakash.pm@ti.com>
Set only LCD_V2_TFT_24BPP_MODE bit for 24bpp and LCD_V2_TFT_24BPP_UNPACK
bit along with LCD_V2_TFT_24BPP_MODE for 32bpp configuration.
Patch is tested on am335x-evm for 24bpp and da850-evm for 16bpp
configurations.
Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 3c12861..e172a4a 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -552,10 +552,11 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
break;
case 24:
reg |= LCD_V2_TFT_24BPP_MODE;
+ break;
case 32:
+ reg |= LCD_V2_TFT_24BPP_MODE;
reg |= LCD_V2_TFT_24BPP_UNPACK;
break;
-
case 8:
par->palette_sz = 256 * 2;
break;
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 11/24] video: da8xx-fb: improve readability of code
From: Darren Etheridge @ 2013-07-30 18:26 UTC (permalink / raw)
To: linux-fbdev
Change the lcd_disable_raster funtion from using a bool to an enum
as the function is very confusing with the current api. This helps
make it clearer what the parameter is really doing.
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 24 ++++++++++++------------
include/video/da8xx-fb.h | 5 +++++
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index b25b810..3c12861 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -271,7 +271,7 @@ static inline void lcd_enable_raster(void)
}
/* Disable the Raster Engine of the LCD Controller */
-static inline void lcd_disable_raster(bool wait_for_frame_done)
+static inline void lcd_disable_raster(enum da8xx_frame_complete wait_for_frame_done)
{
u32 reg;
int ret;
@@ -283,7 +283,7 @@ static inline void lcd_disable_raster(bool wait_for_frame_done)
/* return if already disabled */
return;
- if ((wait_for_frame_done = true) && (lcd_revision = LCD_VERSION_2)) {
+ if ((wait_for_frame_done = DA8XX_FRAME_WAIT) && (lcd_revision = LCD_VERSION_2)) {
frame_done_flag = 0;
ret = wait_event_interruptible_timeout(frame_done_wq,
frame_done_flag != 0,
@@ -771,7 +771,7 @@ static irqreturn_t lcdc_irq_handler_rev02(int irq, void *arg)
u32 stat = lcdc_read(LCD_MASKED_STAT_REG);
if ((stat & LCD_SYNC_LOST) && (stat & LCD_FIFO_UNDERFLOW)) {
- lcd_disable_raster(false);
+ lcd_disable_raster(DA8XX_FRAME_NOWAIT);
lcdc_write(stat, LCD_MASKED_STAT_REG);
lcd_enable_raster();
} else if (stat & LCD_PL_LOAD_DONE) {
@@ -781,7 +781,7 @@ static irqreturn_t lcdc_irq_handler_rev02(int irq, void *arg)
* interrupt via the following write to the status register. If
* this is done after then one gets multiple PL done interrupts.
*/
- lcd_disable_raster(false);
+ lcd_disable_raster(DA8XX_FRAME_NOWAIT);
lcdc_write(stat, LCD_MASKED_STAT_REG);
@@ -834,7 +834,7 @@ static irqreturn_t lcdc_irq_handler_rev01(int irq, void *arg)
u32 reg_ras;
if ((stat & LCD_SYNC_LOST) && (stat & LCD_FIFO_UNDERFLOW)) {
- lcd_disable_raster(false);
+ lcd_disable_raster(DA8XX_FRAME_NOWAIT);
lcdc_write(stat, LCD_STAT_REG);
lcd_enable_raster();
} else if (stat & LCD_PL_LOAD_DONE) {
@@ -844,7 +844,7 @@ static irqreturn_t lcdc_irq_handler_rev01(int irq, void *arg)
* interrupt via the following write to the status register. If
* this is done after then one gets multiple PL done interrupts.
*/
- lcd_disable_raster(false);
+ lcd_disable_raster(DA8XX_FRAME_NOWAIT);
lcdc_write(stat, LCD_STAT_REG);
@@ -986,7 +986,7 @@ static int lcd_da8xx_cpufreq_transition(struct notifier_block *nb,
if (val = CPUFREQ_POSTCHANGE) {
if (par->lcd_fck_rate != clk_get_rate(par->lcdc_clk)) {
par->lcd_fck_rate = clk_get_rate(par->lcdc_clk);
- lcd_disable_raster(true);
+ lcd_disable_raster(DA8XX_FRAME_WAIT);
da8xx_fb_calc_config_clk_divider(par, &par->mode);
if (par->blank = FB_BLANK_UNBLANK)
lcd_enable_raster();
@@ -1024,7 +1024,7 @@ static int fb_remove(struct platform_device *dev)
if (par->panel_power_ctrl)
par->panel_power_ctrl(0);
- lcd_disable_raster(true);
+ lcd_disable_raster(DA8XX_FRAME_WAIT);
lcdc_write(0, LCD_RASTER_CTRL_REG);
/* disable DMA */
@@ -1140,7 +1140,7 @@ static int cfb_blank(int blank, struct fb_info *info)
if (par->panel_power_ctrl)
par->panel_power_ctrl(0);
- lcd_disable_raster(true);
+ lcd_disable_raster(DA8XX_FRAME_WAIT);
break;
default:
ret = -EINVAL;
@@ -1208,9 +1208,9 @@ static int da8xxfb_set_par(struct fb_info *info)
bool raster = da8xx_fb_is_raster_enabled();
if (raster)
- lcd_disable_raster(true);
+ lcd_disable_raster(DA8XX_FRAME_WAIT);
else
- lcd_disable_raster(false);
+ lcd_disable_raster(DA8XX_FRAME_NOWAIT);
fb_var_to_videomode(&par->mode, &info->var);
@@ -1571,7 +1571,7 @@ static int fb_suspend(struct platform_device *dev, pm_message_t state)
par->panel_power_ctrl(0);
fb_set_suspend(info, 1);
- lcd_disable_raster(true);
+ lcd_disable_raster(DA8XX_FRAME_WAIT);
lcd_context_save();
pm_runtime_put_sync(&dev->dev);
console_unlock();
diff --git a/include/video/da8xx-fb.h b/include/video/da8xx-fb.h
index f888259..efed3c3 100644
--- a/include/video/da8xx-fb.h
+++ b/include/video/da8xx-fb.h
@@ -23,6 +23,11 @@ enum raster_load_mode {
LOAD_PALETTE,
};
+enum da8xx_frame_complete {
+ DA8XX_FRAME_WAIT,
+ DA8XX_FRAME_NOWAIT,
+};
+
struct da8xx_lcdc_platform_data {
const char manu_name[10];
void *controller_data;
--
1.7.0.4
^ permalink raw reply related
* [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
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