* Re: [PATCH] omapfb: In omapfb_probe return -EPROBE_DEFER when display driver is not loaded yet
From: Pali Rohár @ 2013-08-04 8:36 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Pavel Machek, Jean-Christophe Plagniol-Villard, linux-omap,
linux-fbdev, linux-kernel, Aaro Koskinen, Tony Lindgren
In-Reply-To: <51EE5483.7000201@ti.com>
[-- Attachment #1: Type: Text/Plain, Size: 3217 bytes --]
Hello,
On Tuesday 23 July 2013 12:01:39 Tomi Valkeinen wrote:
> On 13/07/13 21:27, Pavel Machek wrote:
> > On Wed 2013-07-10 15:08:59, Pali Rohár wrote:
> >> * On RX-51 probing for acx565akm driver is later then for
> >> omapfb which cause that omapfb probe fail and framebuffer
> >> is not working * EPROBE_DEFER causing that kernel try to
> >> probe for omapfb later again which fixing this problem
> >>
> >> * Without this patch display on Nokia RX-51 (N900) phone
> >> not working
> >>
> >> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> >
> > Tested-by: Pavel Machek <pavel@ucw.cz>
>
> Which kernel version is this? Does it have
> dfbc32316c6991010328c21e6046b05bac57eb84 (OMAPFB: defer probe
> if no displays)?
>
Kernel 3.10, so it have above commit.
> Then again, rx51 has tv-output, which probably gets probed
> early. So omapfb does see one functional display, and starts,
> even if the LCD is not available yet.
>
> > (Actually, do we know which commit broke the ordering? We
> > may want to simply revert that one...)
>
> Well, my understand that this is how it's supposed to work.
> There's no defined order with the driver probes, and the
> drivers just have to deal with their dependencies not being
> there yet.
>
> I don't have a perfect solution for this problem for omapfb.
> omapfb doesn't support dynamically adding displays, so all
> the displays it uses have to be probed before omapfb. And
> omapfb doesn't know which displays will be probed.
>
> The patch below was added for 3.11. Does it fix the issue for
> you? Perhaps it should be added for 3.10 also.
>
> Tomi
>
I tested patch below and it fixing our problem too. So it could be
added to 3.10.
> commit e9f322b4913e5d3e5c5d21dc462ca6f8a86e1df1
> Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Date: Thu May 23 16:41:25 2013 +0300
>
> OMAPFB: use EPROBE_DEFER if default display is not present
>
> Currently omapfb returns EPROBE_DEFER if no displays have
> been probed at the time omapfb is probed. However, sometimes
> some of the displays have been probed at that time, but not
> all. We can't return EPROBE_DEFER in that case, because then
> one missing driver would cause omapfb to defer always,
> preventing any display from working.
>
> However, if the user has defined a default display, we can
> presume that the driver for that display is eventually
> loaded. Thus, this patch changes omapfb to return
> EPROBE_DEFER in case default display is not found.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>
> diff --git a/drivers/video/omap2/omapfb/omapfb-main.c
> b/drivers/video/omap2/omapfb/omapfb-main.c index
> 528e453..27d6905 100644
> --- a/drivers/video/omap2/omapfb/omapfb-main.c
> +++ b/drivers/video/omap2/omapfb/omapfb-main.c
> @@ -2503,7 +2503,7 @@ static int omapfb_probe(struct
> platform_device *pdev)
>
> if (def_display == NULL) {
> dev_err(fbdev->dev, "failed to find default
> display\n"); - r = -EINVAL;
> + r = -EPROBE_DEFER;
> goto cleanup;
> }
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* [PATCH] fbdev: suppress warning when assigning vga-save/restore base
From: David Herrmann @ 2013-08-04 17:25 UTC (permalink / raw)
To: linux-fbdev
Cc: linux-kernel, H. Peter Anvin, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Ondrej Zajicek, David Miller, David Herrmann
If drivers use "struct resource" objects to retrieve the vga-base, they
must correctly cast the integer to pointer. With x86+PAE we have 32bit
pointers but 64bit resource_size_t. Hence, cast it to "unsigned long"
before casting to "void*" to suppress warnings due to size differences.
As IO addresses are always low addresses, we can safely drop the higher
part of the address. This is what these drivers did before, anyway.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Reported-by: H. Peter Anvin <hpa@zytor.com>
---
Hi
hpa reported build-warnings on i386+PAE:
/home/hpa/kernel/distwork/drivers/video/arkfb.c:1019:23: warning: cast to
pointer from integer of different size [-Wint-to-pointer-cast]
par->state.vgabase = (void __iomem *) vga_res.start;
^
/home/hpa/kernel/distwork/drivers/video/s3fb.c: In function ‘s3_pci_probe’:
/home/hpa/kernel/distwork/drivers/video/s3fb.c:1186:23: warning: cast to pointer
from integer of different size [-Wint-to-pointer-cast]
par->state.vgabase = (void __iomem *) vga_res.start;
^
This is due to resource_size_t being 64bit but "void*" 32bit. This patch tries
to suppress these warnings but I am not really comfortable fixing this. I have
no idea whether my assumption (IO address are 32bit) is right. Please verify.
@David: The following 3 commits of yours presumably introduced the warnings. I
would be glad if you could review this:
commit 94c322c30bd14ae6cdd369cb4a1f94c5c3809ac9
Author: David Miller <davem@davemloft.net>
Date: Tue Jan 11 23:54:21 2011 +0000
s3fb: Compute VGA base iomem pointer explicitly.
commit 892c24ca40ffebf6d0ca4cc1454e58db131a4f5a
Author: David Miller <davem@davemloft.net>
Date: Tue Jan 11 23:54:35 2011 +0000
arkfb: Compute VGA base iomem pointer explicitly.
commit 6a2f6d5e970afbc1b8b08bafae9d9138a3206960
Author: David Miller <davem@davemloft.net>
Date: Tue Jan 11 23:55:17 2011 +0000
vt8623fb: Compute VGA base iomem pointer explicitly.
Cheers
David
drivers/video/arkfb.c | 2 +-
drivers/video/s3fb.c | 2 +-
drivers/video/vt8623fb.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/arkfb.c b/drivers/video/arkfb.c
index 94a51f1..a4e487c 100644
--- a/drivers/video/arkfb.c
+++ b/drivers/video/arkfb.c
@@ -1016,7 +1016,7 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
pcibios_bus_to_resource(dev, &vga_res, &bus_reg);
- par->state.vgabase = (void __iomem *) vga_res.start;
+ par->state.vgabase = (void __iomem *)(unsigned long) vga_res.start;
/* FIXME get memsize */
regval = vga_rseq(par->state.vgabase, 0x10);
diff --git a/drivers/video/s3fb.c b/drivers/video/s3fb.c
index 47ca86c..fc2208b 100644
--- a/drivers/video/s3fb.c
+++ b/drivers/video/s3fb.c
@@ -1183,7 +1183,7 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
pcibios_bus_to_resource(dev, &vga_res, &bus_reg);
- par->state.vgabase = (void __iomem *) vga_res.start;
+ par->state.vgabase = (void __iomem *)(unsigned long) vga_res.start;
/* Unlock regs */
cr38 = vga_rcrt(par->state.vgabase, 0x38);
diff --git a/drivers/video/vt8623fb.c b/drivers/video/vt8623fb.c
index e9557fa..3039d82 100644
--- a/drivers/video/vt8623fb.c
+++ b/drivers/video/vt8623fb.c
@@ -729,7 +729,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
pcibios_bus_to_resource(dev, &vga_res, &bus_reg);
- par->state.vgabase = (void __iomem *) vga_res.start;
+ par->state.vgabase = (void __iomem *)(unsigned long) vga_res.start;
/* Find how many physical memory there is on card */
memsize1 = (vga_rseq(par->state.vgabase, 0x34) + 1) >> 1;
--
1.8.3.4
^ permalink raw reply related
* Re: [PATCH RESEND 0/8] x86 platform framebuffers
From: David Herrmann @ 2013-08-04 17:30 UTC (permalink / raw)
To: H. Peter Anvin
Cc: linux-kernel, David Airlie, Geert Uytterhoeven, Stephen Warren,
Peter Jones, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, Andrew Morton
In-Reply-To: <cf7f27cd-4b3e-41bf-ba26-0955a93b04ec@email.android.com>
Hi
On Sat, Aug 3, 2013 at 5:53 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> When compiled for i386 PAE phys_addr_t is 64 bits but pointers are 32 bits.
Yepp, that makes sense. I am not really comfortable fixing this as I
have no idea how PAE actually works, but I digged through git history
and sent a patch to linux-fbdev (I put you on CC). The (long)-cast
should be sufficient, I guess.
Were there any other build warnings you encountered?
Thanks
David
> David Herrmann <dh.herrmann@gmail.com> wrote:
>>Hi
>>
>>On Sat, Aug 3, 2013 at 1:39 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>>> On 08/02/2013 05:05 AM, David Herrmann wrote:
>>>> Hi
>>>>
>>>> I cut down my previous series to no longer include the SimpleDRM
>>driver. If
>>>> anyone is interested, you can find it here:
>>>> http://lwn.net/Articles/558104/
>>>> I will resend it once these preparation patches are in.
>>>>
>>>> Changes since v2:
>>>> - added common x86 formats (reported by hpa) (patch #5)
>>>>
>>>> This whole series (including simpledrm) is tested by Stephen and me.
>>I would be
>>>> glad if maintainers could ack/nack this so I can continue my work.
>>>>
>>>> This series is pretty small and just converts x86 to use
>>platform-devices
>>>> instead of global objects to pass framebuffer data to drivers. The
>>commit
>>>> messages explain everything in detail.
>>>> The idea is to create a "platform-framebuffer" device which drivers
>>can bind to.
>>>> If x86 boot code detectes efi or vesa framebuffers, it creates
>>efi-framebuffer
>>>> or vesa-framebuffer devices instead.
>>>>
>>>> Additionally, if the modes are compatible, "simple-framebuffer"
>>devices are
>>>> created so simplefb can be used on x86. This feature is only enabled
>>if
>>>> CONFIG_X86_SYSFB is selected (off by default) so users without
>>simplefb still
>>>> get boot logs.
>>>>
>>>> @Stephen: I wasn't sure whether you tested the efi/vesa framebuffer
>>changes,
>>>> too, so I didn't add your tested-by there. And I changed patch #5 so
>>I dropped
>>>> it there, too. Thanks for testing!
>>>>
>>>
>>> I am getting a bunch of new warnings with this patchset, typically of
>>> the form:
>>>
>>>
>>> /home/hpa/kernel/distwork/drivers/video/arkfb.c:1019:23: warning:
>>cast
>>> to pointer from integer of different size [-Wint-to-pointer-cast]
>>> par->state.vgabase = (void __iomem *) vga_res.start;
>>> ^
>>> /home/hpa/kernel/distwork/drivers/video/s3fb.c: In function
>>‘s3_pci_probe’:
>>> /home/hpa/kernel/distwork/drivers/video/s3fb.c:1186:23: warning: cast
>>to
>>> pointer from integer of different size [-Wint-to-pointer-cast]
>>> par->state.vgabase = (void __iomem *) vga_res.start;
>>> ^
>>> I have pushed it out to a topic branch in the tip tree, partly to
>>give
>>> Fengguang's build bot a run at it (it is excellent at spotting the
>>> origin of new warnings), but this needs to be fixed; we will not
>>merge
>>> this branch in its current form.
>>
>>Thanks for looking at it. However, these warnings seem unrelated to my
>>patchset. I didn't change any vga/fb headers nor did I touch
>>arkfb/s3fb. Furthermore, I cannot reproduce these warnings. "vga_res"
>>is a "struct resource", "->start" is of type "resource_size_t" which
>>is typedef'ed to "phys_addr_t". Seems like the build-bot used some
>>random config with CONFIG_PHYS_ADDR_T_64BIT set but building on 32bit
>>(or something like that).
>>
>>Is there any place where I can see the compiler log? Or the used
>>.config file?
>>
>>Thanks
>>David
>
> --
> Sent from my mobile phone. Please excuse brevity and lack of formatting.
^ permalink raw reply
* Re: [PATCH] fbdev: suppress warning when assigning vga-save/restore base
From: H. Peter Anvin @ 2013-08-04 17:33 UTC (permalink / raw)
To: David Herrmann
Cc: linux-fbdev, linux-kernel, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Ondrej Zajicek, David Miller
In-Reply-To: <1375637141-2878-1-git-send-email-dh.herrmann@gmail.com>
On 08/04/2013 10:25 AM, David Herrmann wrote:
> If drivers use "struct resource" objects to retrieve the vga-base, they
> must correctly cast the integer to pointer. With x86+PAE we have 32bit
> pointers but 64bit resource_size_t. Hence, cast it to "unsigned long"
> before casting to "void*" to suppress warnings due to size differences.
>
> As IO addresses are always low addresses, we can safely drop the higher
> part of the address. This is what these drivers did before, anyway.
>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> Reported-by: H. Peter Anvin <hpa@zytor.com>
I'm still bothered here. Casting between resource_size_t and void *
implies a confusion between physical and virtual addresses. That may be
unavoidable for some reason, but I'd like to know what exactly is confused.
Anyone who can dig backwards and summarize? In other words:
Where in the current code do we stuff a physical address in a pointer,
or a virtual address into a non-pointer?
-hpa
^ permalink raw reply
* Re: [PATCH RESEND 0/8] x86 platform framebuffers
From: H. Peter Anvin @ 2013-08-04 17:34 UTC (permalink / raw)
To: David Herrmann
Cc: linux-kernel, David Airlie, Geert Uytterhoeven, Stephen Warren,
Peter Jones, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, Andrew Morton
In-Reply-To: <CANq1E4S+MSv2s+ikPuJ7+jzbQfQFxjdj7wBk54e=n57GtZ0eeA@mail.gmail.com>
On 08/04/2013 10:30 AM, David Herrmann wrote:
> Hi
>
> On Sat, Aug 3, 2013 at 5:53 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> When compiled for i386 PAE phys_addr_t is 64 bits but pointers are 32 bits.
>
> Yepp, that makes sense. I am not really comfortable fixing this as I
> have no idea how PAE actually works, but I digged through git history
> and sent a patch to linux-fbdev (I put you on CC). The (long)-cast
> should be sufficient, I guess.
>
> Were there any other build warnings you encountered?
>
> Thanks
> David
>
It looks like Fengguang's bot hasn't run yet, so I don't have a summary.
I asked Fengguang what I need to do to kick it off.
-hpa
^ permalink raw reply
* Re: [PATCH] fbdev: suppress warning when assigning vga-save/restore base
From: David Miller @ 2013-08-05 1:51 UTC (permalink / raw)
To: hpa
Cc: dh.herrmann, linux-fbdev, linux-kernel, plagnioj, tomi.valkeinen,
santiago
In-Reply-To: <51FE907A.2090201@zytor.com>
From: "H. Peter Anvin" <hpa@zytor.com>
Date: Sun, 04 Aug 2013 10:33:46 -0700
> Anyone who can dig backwards and summarize? In other words:
>
> Where in the current code do we stuff a physical address in a pointer,
> or a virtual address into a non-pointer?
The VGA register accessors try to accomodate iomem and ioport
accesses.
If they are given a non-NULL iomem pointer 'regbase' they use
iomem accesses, otherwise they do direct ISA port poking.
And yes the drivers in question are making some brash assumptions.
I suspect they should be using ioremap() or similar.
^ permalink raw reply
* RE: [PATCH 4/5] fb: Add DCU framebuffer driver for Vybrid VF610 platform
From: Wang Huan-B18965 @ 2013-08-05 9:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130729111457.GB3029@pengutronix.de>
SGksIFNhc2NoYSwNCg0KPiBPbiBGcmksIEp1bCAxMiwgMjAxMyBhdCAwMjowNzo1NVBNICswODAw
LCBBbGlzb24gV2FuZyB3cm90ZToNCj4gPiBUaGUgRGlzcGxheSBDb250cm9sbGVyIFVuaXQgKERD
VSkgbW9kdWxlIGlzIGEgc3lzdGVtIG1hc3RlciB0aGF0DQo+ID4gZmV0Y2hlcyBncmFwaGljcyBz
dG9yZWQgaW4gaW50ZXJuYWwgb3IgZXh0ZXJuYWwgbWVtb3J5IGFuZCBkaXNwbGF5cw0KPiA+IHRo
ZW0gb24gYSBURlQgTENEIHBhbmVsLiBBIHdpZGUgcmFuZ2Ugb2YgcGFuZWwgc2l6ZXMgaXMgc3Vw
cG9ydGVkIGFuZA0KPiA+IHRoZSB0aW1pbmcgb2YgdGhlIGludGVyZmFjZSBzaWduYWxzIGlzIGhp
Z2hseSBjb25maWd1cmFibGUuDQo+ID4gR3JhcGhpY3MgYXJlIHJlYWQgZGlyZWN0bHkgZnJvbSBt
ZW1vcnkgYW5kIHRoZW4gYmxlbmRlZCBpbiByZWFsLXRpbWUsDQo+ID4gd2hpY2ggYWxsb3dzIGZv
ciBkeW5hbWljIGNvbnRlbnQgY3JlYXRpb24gd2l0aCBtaW5pbWFsIENQVQ0KPiBpbnRlcnZlbnRp
b24uDQo+IA0KPiBPbmx5IGEgcmV2aWV3IG9mIHRoZSBjb2RlIGlubGluZS4NCj4gDQo+IE1heWJl
IHRoZSByZWFsIHF1ZXN0aW9uIGlzIHdoZXRoZXIgd2Ugd2FudCB0byBpbnRyb2R1Y2UgYW5vdGhl
cg0KPiBmcmFtZWJ1ZmZlciBkcml2ZXIgYXQgYWxsIGluc3RlYWQgb2YgbWFraW5nIGl0IGEgRFJN
IGRyaXZlci4NCltBbGlzb24gV2FuZ10gSSB0aGluayBEQ1UgbW9kdWxlIGlzIG1vcmUgc3VpdGFi
bGUgdG8gYmUgZGVzaWduZWQgYXMgYSBmcmFtZWJ1ZmZlciBkcml2ZXIgdGhhbiBhIERSTSBkcml2
ZXIuIEp1c3QgbGlrZSBESVUgZnJhbWVidWZmZXIgZHJpdmVyIGZvciBQb3dlclBDLg0KPiANCj4g
PiArDQo+ID4gKyNkZWZpbmUgRFJJVkVSX05BTUUJCQkiZnNsLWRjdS1mYiINCj4gPiArDQo+ID4g
KyNkZWZpbmUgRENVX0RDVV9NT0RFCQkJMHgwMDEwDQo+ID4gKyNkZWZpbmUgRENVX01PREVfQkxF
TkRfSVRFUih4KQkJKHggPDwgMjApDQo+IA0KPiBXaGF0J3MgdGhlIHJlc3VsdCBvZiBEQ1VfTU9E
RV9CTEVORF9JVEVSKDEgKyAxKT8NCltBbGlzb24gV2FuZ10gSXMgaXQgcmVhbGx5IG5lY2Vzc2Fy
eT8gSSBkb24ndCB1c2UgdGhpcyBtYWNybyBsaWtlIERDVV9NT0RFX0JMRU5EX0lURVIoMSArIDEp
LCBqdXN0IHVzZSBEQ1VfTU9ERV9CTEVORF9JVEVSKDIpLg0KPiANCj4gPiArc3RhdGljIHN0cnVj
dCBmYl92aWRlb21vZGUgZGN1X21vZGVfZGJbXSA9IHsNCj4gPiArCXsNCj4gPiArCQkubmFtZQkJ
PSAiNDgweDI3MiIsDQo+ID4gKwkJLnJlZnJlc2gJPSA3NSwNCj4gPiArCQkueHJlcwkJPSA0ODAs
DQo+ID4gKwkJLnlyZXMJCT0gMjcyLA0KPiA+ICsJCS5waXhjbG9jawk9IDkxOTk2LA0KPiA+ICsJ
CS5sZWZ0X21hcmdpbgk9IDIsDQo+ID4gKwkJLnJpZ2h0X21hcmdpbgk9IDIsDQo+ID4gKwkJLnVw
cGVyX21hcmdpbgk9IDEsDQo+ID4gKwkJLmxvd2VyX21hcmdpbgk9IDEsDQo+ID4gKwkJLmhzeW5j
X2xlbgk9IDQxLA0KPiA+ICsJCS52c3luY19sZW4JPSAyLA0KPiA+ICsJCS5zeW5jCQk9IEZCX1NZ
TkNfQ09NUF9ISUdIX0FDVCB8IEZCX1NZTkNfVkVSVF9ISUdIX0FDVCwNCj4gPiArCQkudm1vZGUJ
CT0gRkJfVk1PREVfTk9OSU5URVJMQUNFRCwNCj4gPiArCX0sDQo+ID4gK307DQo+IA0KPiBXZSBo
YXZlIHdheXMgdG8gZGVzY3JpYmUgYSBwYW5lbCBpbiBkdC4gVXNlIHRoZW0uDQpbQWxpc29uIFdh
bmddIE9rLiBJIGRvbid0IGtub3cgaXQgaXMgY29tbW9uIHRvIGRlc2NyaWJlIHRoZSBwYW5lbCBp
biBkdHMgZm9yIHRoZSBsYXRlc3Qga2VybmVsLiBUaGFua3MuDQo+IA0KPiA+ICsNCj4gPiArc3Rh
dGljIHN0cnVjdCBtZmJfaW5mbyBtZmJfdGVtcGxhdGVbXSA9IHsNCj4gPiArCXsNCj4gPiArCS5p
bmRleCA9IExBWUVSMCwNCj4gPiArCS5pZCA9ICJMYXllcjAiLA0KPiA+ICsJLmFscGhhID0gMHhm
ZiwNCj4gPiArCS5ibGVuZCA9IDAsDQo+ID4gKwkuY291bnQgPSAwLA0KPiA+ICsJLnhfbGF5ZXJf
ZCA9IDAsDQo+ID4gKwkueV9sYXllcl9kID0gMCwNCj4gPiArCX0sDQo+IA0KPiBXcm9uZyBpbmRl
bnRhdGlvbi4NCltBbGlzb24gV2FuZ10gSSB3aWxsIGZpeCBpdCBpbiBuZXh0IHZlcnNpb24uDQo+
IA0KPiA+ICsJZGVmYXVsdDoNCj4gPiArCQlwcmludGsoS0VSTl9FUlIgInVuc3VwcG9ydGVkIGNv
bG9yIGRlcHRoOiAldVxuIiwNCj4gPiArCQkJdmFyLT5iaXRzX3Blcl9waXhlbCk7DQo+IA0KPiBV
c2UgZGV2XyogZm9yIHByaW50aW5nIG1lc3NhZ2VzIGluIGRyaXZlcnMuDQpbQWxpc29uIFdhbmdd
IE9rLg0KPiANCj4gPiArc3RhdGljIGludCBmc2xfZGN1X2NoZWNrX3ZhcihzdHJ1Y3QgZmJfdmFy
X3NjcmVlbmluZm8gKnZhciwNCj4gPiArCQlzdHJ1Y3QgZmJfaW5mbyAqaW5mbykNCj4gPiArew0K
PiA+ICsJaWYgKHZhci0+eHJlc192aXJ0dWFsIDwgdmFyLT54cmVzKQ0KPiA+ICsJCXZhci0+eHJl
c192aXJ0dWFsID0gdmFyLT54cmVzOw0KPiA+ICsJaWYgKHZhci0+eXJlc192aXJ0dWFsIDwgdmFy
LT55cmVzKQ0KPiA+ICsJCXZhci0+eXJlc192aXJ0dWFsID0gdmFyLT55cmVzOw0KPiA+ICsNCj4g
PiArCWlmICh2YXItPnhvZmZzZXQgPCAwKQ0KPiA+ICsJCXZhci0+eG9mZnNldCA9IDA7DQo+ID4g
Kw0KPiA+ICsJaWYgKHZhci0+eW9mZnNldCA8IDApDQo+ID4gKwkJdmFyLT55b2Zmc2V0ID0gMDsN
Cj4gDQo+IEV2ZXIgc2VlbiBhbiB1bnNpZ25lZCB2YWx1ZSBnb2luZyBiZWxvdyB6ZXJvPw0KW0Fs
aXNvbiBXYW5nXSBZb3UgYXJlIHJpZ2h0LCBJIHdpbGwgcmVtb3ZlIHRoZW0gaW4gbmV4dCB2ZXJz
aW9uLg0KPiANCj4gPiArCWRlZmF1bHQ6DQo+ID4gKwkJcHJpbnRrKEtFUk5fRVJSICJ1bnN1cHBv
cnRlZCBjb2xvciBkZXB0aDogJXVcbiIsDQo+ID4gKwkJCXZhci0+Yml0c19wZXJfcGl4ZWwpOw0K
PiANCj4gQlVHKCkuIFRoaXMgY2FuJ3QgaGFwcGVuIHNpbmNlIHlvdSBtYWtlIHRoYXQgc3VyZSBh
Ym92ZS4NCltBbGlzb24gV2FuZ10gWW91IGFyZSByaWdodCwgSSB3aWxsIHJlbW92ZSBpdCBpbiBu
ZXh0IHZlcnNpb24uDQo+IA0KPiA+ICsJCXJldHVybiAtRUlOVkFMOw0KPiA+ICsJfQ0KPiA+ICsN
Cj4gPiArCXJldHVybiAwOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICtzdGF0aWMgaW50IGNhbGNfZGl2
X3JhdGlvKHN0cnVjdCBmYl9pbmZvICppbmZvKSB7DQo+IA0KPiBVc2UgYSBjb25zaXN0ZW50IG5h
bWVzcGFjZSBmb3IgZnVuY3Rpb24gbmFtZXMgKGZzbF9kY3VfKQ0KW0FsaXNvbiBXYW5nXSBJcyBp
dCBuZWNlc3NhcnkgdG8gdXNlIGEgY29uc2lzdGVudCBuYW1lc3BhY2UgZm9yIHRoaXMgZ2VuZXJp
YyBmdW5jdGlvbj8gSSB0aGluayBpdCBpcyBuZWNlc3NhcnkgdG8gdXNlIGEgY29uc2lzdGVudCBu
YW1lc3BhY2UgKGZzbF9kY3VfKSBmb3IgdGhlIGZ1bmN0aW9uIG5hbWVzIGluIHN0cnVjdHVyZSBm
c2xfZGN1X29wcy4NCj4gDQo+ID4gKwl3cml0ZWwoRENVX1RIUkVTSE9MRF9MU19CRl9WUygweDMp
IHwNCj4gRENVX1RIUkVTSE9MRF9PVVRfQlVGX0hJR0goMHg3OCkgfA0KPiA+ICsJCURDVV9USFJF
U0hPTERfT1VUX0JVRl9MT1coMCksIGRjdWZiLT5yZWdfYmFzZSArDQo+IERDVV9USFJFU0hPTEQp
Ow0KPiA+ICsNCj4gPiArCWVuYWJsZV9jb250cm9sbGVyKGluZm8pOw0KPiA+ICt9DQo+IA0KPiBN
YWtlIHlvdXIgZnVuY3Rpb25zIHN5bW1ldHJpYy4gSWYgdGhlcmUncyB1cGRhdGVfY29udHJvbGxl
cigpLCB0aGUNCj4gZnVuY3Rpb24gc2hvdWxkIGRvIGV4YWN0bHkgdGhhdCwgaXQgc2hvdWxkICpu
b3QqIGVuYWJsZSB0aGUgY29udHJvbGxlci4NCj4gQ2FsbCB0aGlzIGZyb20gdGhlIHVzZXJzIG9m
IHRoaXMgZnVuY3Rpb24gaWYgbmVjZXNzYXJ5Lg0KW0FsaXNvbiBXYW5nXSBPaywgSSB3aWxsIG1v
dmUgdGhpcyBmdW5jdGlvbiBvdXQsIGFuZCBhZGQgaXQgaGVyZS4NCmlmIChtZmJpLT5pbmRleCA9
PSBMQVlFUjApIHsNCgl1cGRhdGVfY29udHJvbGxlcihpbmZvKTsNCisJZW5hYmxlX2NvbnRyb2xs
ZXIoaW5mbyk7DQp9DQo+IA0KPiA+ICsJCWlmIChjb3B5X3RvX3VzZXIoYnVmLCAmYWxwaGEsIHNp
emVvZihhbHBoYSkpKQ0KPiA+ICsJCQlyZXR1cm4gLUVGQVVMVDsNCj4gPiArCQlicmVhazsNCj4g
PiArCWNhc2UgTUZCX1NFVF9BTFBIQToNCj4gPiArCQlpZiAoY29weV9mcm9tX3VzZXIoJmFscGhh
LCBidWYsIHNpemVvZihhbHBoYSkpKQ0KPiA+ICsJCQlyZXR1cm4gLUVGQVVMVDsNCj4gPiArCQlt
ZmJpLT5ibGVuZCA9IDE7DQo+ID4gKwkJbWZiaS0+YWxwaGEgPSBhbHBoYTsNCj4gPiArCQlmc2xf
ZGN1X3NldF9wYXIoaW5mbyk7DQo+ID4gKwkJYnJlYWs7DQo+IA0KPiBJcyBpdCBzdGlsbCBzdGF0
ZSBvZiB0aGUgYXJ0IHRvIGludHJvZHVjZSBpb2N0bHMgaW4gdGhlIGZiIGZyYW1ld29yaw0KPiB3
aXRob3V0IGFueSBraW5kIG9mIGRvY3VtZW50YXRpb24/DQpbQWxpc29uIFdhbmddIERvIHlvdSBt
ZWFuIEkgbmVlZCB0byBhZGQgYSBkb2N1bWVudGF0aW9uIGluIERvY3VtZW50YXRpb24vZmIvPw0K
PiANCj4gPiArCWRlZmF1bHQ6DQo+ID4gKwkJcHJpbnRrKEtFUk5fRVJSICJ1bmtub3duIGlvY3Rs
IGNvbW1hbmQgKDB4JTA4WClcbiIsIGNtZCk7DQo+IA0KPiBXaGF0IHNoYWxsIGEgcmVhZGVyIG9m
IHRoZSBrZXJuZWwgbG9nIGRvIHdpdGggYSBtZXNzYWdlIGxpa2UgdGhpcz8gSXQncw0KPiB1dHRl
cmx5IHVzZWxlc3Mgd2hlbiBoZSBkb2Vzbid0IGV2ZW4gbm93IHdoaWNoIGRldmljZSBmYWlsZWQg
aGVyZS4gSnVzdA0KPiBkcm9wIHRoaXMuDQpbQWxpc29uIFdhbmddIE9rLg0KPiANCj4gPiArc3Rh
dGljIHZvaWQgZW5hYmxlX2ludGVycnVwdHMoc3RydWN0IGRjdV9mYl9kYXRhICpkY3VmYikgew0K
PiA+ICsJdTMyIGludF9tYXNrID0gcmVhZGwoZGN1ZmItPnJlZ19iYXNlICsgRENVX0lOVF9NQVNL
KTsNCj4gPiArDQo+ID4gKwl3cml0ZWwoaW50X21hc2sgJiB+RENVX0lOVF9NQVNLX1VORFJVTiwg
ZGN1ZmItPnJlZ19iYXNlICsNCj4gPiArRENVX0lOVF9NQVNLKTsgfQ0KPiANCj4gSW5saW5lIHRo
aXMgY29kZSB3aGVyZSB5b3UgbmVlZCBpdC4gSW50cm9kdWNpbmcgYSBmdW5jdGlvbiBmb3IgYSBz
aW5nbGUNCj4gcmVnaXN0ZXIgd3JpdGUgc2VlbXMgcXVpdGUgdXNlbGVzcy4NCltBbGlzb24gV2Fu
Z10gT2ssIEkgd2lsbCBpbmxpbmUgaXQuDQo+IA0KPiA+ICtzdGF0aWMgaW50IGluc3RhbGxfZnJh
bWVidWZmZXIoc3RydWN0IGZiX2luZm8gKmluZm8pIHsNCj4gPiArCXN0cnVjdCBtZmJfaW5mbyAq
bWZiaSA9IGluZm8tPnBhcjsNCj4gPiArCXN0cnVjdCBmYl92aWRlb21vZGUgKmRiID0gZGN1X21v
ZGVfZGI7DQo+ID4gKwl1bnNpZ25lZCBpbnQgZGJzaXplID0gQVJSQVlfU0laRShkY3VfbW9kZV9k
Yik7DQo+ID4gKwlpbnQgcmV0Ow0KPiA+ICsNCj4gPiArCWluZm8tPnZhci5hY3RpdmF0ZSA9IEZC
X0FDVElWQVRFX05PVzsNCj4gPiArCWluZm8tPmZib3BzID0gJmZzbF9kY3Vfb3BzOw0KPiA+ICsJ
aW5mby0+ZmxhZ3MgPSBGQklORk9fRkxBR19ERUZBVUxUOw0KPiA+ICsJaW5mby0+cHNldWRvX3Bh
bGV0dGUgPSAmbWZiaS0+cHNldWRvX3BhbGV0dGU7DQo+ID4gKw0KPiA+ICsJZmJfYWxsb2NfY21h
cCgmaW5mby0+Y21hcCwgMTYsIDApOw0KPiA+ICsNCj4gPiArCXJldCA9IGZiX2ZpbmRfbW9kZSgm
aW5mby0+dmFyLCBpbmZvLCBmYl9tb2RlLCBkYiwgZGJzaXplLA0KPiA+ICsJCQlOVUxMLCBkZWZh
dWx0X2JwcCk7DQo+ID4gKw0KPiA+ICsJaWYgKGZzbF9kY3VfY2hlY2tfdmFyKCZpbmZvLT52YXIs
IGluZm8pKSB7DQo+ID4gKwkJcmV0ID0gLUVJTlZBTDsNCj4gDQo+IFByb3BhZ2F0ZSB0aGUgZXJy
b3IuDQpbQWxpc29uIFdhbmddIEkgd2lsbCBmaXggaW4gbmV4dCB2ZXJzaW9uLg0KPiANCj4gPiAr
CQlnb3RvIGZhaWxlZF9jaGVja3ZhcjsNCj4gPiArCX0NCj4gPiArDQo+ID4gKwlpZiAocmVnaXN0
ZXJfZnJhbWVidWZmZXIoaW5mbykgPCAwKSB7DQo+ID4gKwkJcmV0ID0gLUVJTlZBTDsNCj4gDQo+
IGRpdHRvDQpbQWxpc29uIFdhbmddIEkgd2lsbCBmaXggaW4gbmV4dCB2ZXJzaW9uLg0KPiANCj4g
PiArc3RhdGljIGlycXJldHVybl90IGZzbF9kY3VfaXJxKGludCBpcnEsIHZvaWQgKmRldl9pZCkg
ew0KPiA+ICsJc3RydWN0IGRjdV9mYl9kYXRhICpkY3VmYiA9IGRldl9pZDsNCj4gPiArCXVuc2ln
bmVkIGludCBzdGF0dXMgPSByZWFkbChkY3VmYi0+cmVnX2Jhc2UgKyBEQ1VfSU5UX1NUQVRVUyk7
DQo+ID4gKwl1MzIgZGN1X21vZGU7DQo+ID4gKw0KPiA+ICsJaWYgKHN0YXR1cykgew0KPiANCj4g
U2F2ZSBpbmRlbmRhdGlvbiBsZXZlbCBieSBiYWlsaW5nIG91dCBlYXJseS4NCltBbGlzb24gV2Fu
Z10gV2hhdCBkbyB5b3UgbWVhbj8NCj4gDQo+ID4gKwkJaWYgKHN0YXR1cyAmIERDVV9JTlRfU1RB
VFVTX1VORFJVTikgew0KPiA+ICsJCQlkY3VfbW9kZSA9IHJlYWRsKGRjdWZiLT5yZWdfYmFzZSAr
IERDVV9EQ1VfTU9ERSk7DQo+ID4gKwkJCWRjdV9tb2RlICY9IH5EQ1VfTU9ERV9EQ1VfTU9ERV9N
QVNLOw0KPiA+ICsJCQl3cml0ZWwoZGN1X21vZGUgfCBEQ1VfTU9ERV9EQ1VfTU9ERShEQ1VfTU9E
RV9PRkYpLA0KPiA+ICsJCQkJZGN1ZmItPnJlZ19iYXNlICsgRENVX0RDVV9NT0RFKTsNCj4gPiAr
CQkJdWRlbGF5KDEpOw0KPiA+ICsJCQl3cml0ZWwoZGN1X21vZGUgfCBEQ1VfTU9ERV9EQ1VfTU9E
RShEQ1VfTU9ERV9OT1JNQUwpLA0KPiA+ICsJCQkJZGN1ZmItPnJlZ19iYXNlICsgRENVX0RDVV9N
T0RFKTsNCj4gPiArCQl9DQo+ID4gKwkJd3JpdGVsKHN0YXR1cywgZGN1ZmItPnJlZ19iYXNlICsg
RENVX0lOVF9TVEFUVVMpOw0KPiA+ICsJCXJldHVybiBJUlFfSEFORExFRDsNCj4gPiArCX0NCj4g
PiArCXJldHVybiBJUlFfTk9ORTsNCj4gPiArfQ0KPiA+ICsNCj4gPiArCWlmIChJU19FUlIodGNv
bl9jbGspKSB7DQo+ID4gKwkJcmV0ID0gUFRSX0VSUih0Y29uX2Nsayk7DQo+ID4gKwkJZ290byBm
YWlsZWRfZ2V0Y2xvY2s7DQo+ID4gKwl9DQo+ID4gKwljbGtfcHJlcGFyZV9lbmFibGUodGNvbl9j
bGspOw0KPiA+ICsNCj4gPiArCXRjb25fcmVnID0gb2ZfaW9tYXAodGNvbl9ucCwgMCk7DQo+IA0K
PiBVc2UgZGV2bV8qDQpbQWxpc29uIFdhbmddIE9rLg0KPiANCj4gPiArCWRjdWZiLT5pcnEgPSBw
bGF0Zm9ybV9nZXRfaXJxKHBkZXYsIDApOw0KPiA+ICsJaWYgKCFkY3VmYi0+aXJxKSB7DQo+ID4g
KwkJcmV0ID0gLUVJTlZBTDsNCj4gPiArCQlnb3RvIGZhaWxlZF9nZXRpcnE7DQo+ID4gKwl9DQo+
ID4gKw0KPiA+ICsJcmV0ID0gcmVxdWVzdF9pcnEoZGN1ZmItPmlycSwgZnNsX2RjdV9pcnEsIDAs
IERSSVZFUl9OQU1FLCBkY3VmYik7DQo+IA0KPiBVc2UgZGV2bV9yZXF1ZXN0X2lycQ0KW0FsaXNv
biBXYW5nXSBPay4NCj4gDQo+ID4gKwlpZiAocmV0KSB7DQo+ID4gKwkJZGV2X2VycigmcGRldi0+
ZGV2LCAiY291bGQgbm90IHJlcXVlc3QgaXJxXG4iKTsNCj4gPiArCQlnb3RvIGZhaWxlZF9yZXF1
ZXN0aXJxOw0KPiA+ICsJfQ0KPiA+ICsNCj4gPiArCS8qIFB1dCBUQ09OIGluIGJ5cGFzcyBtb2Rl
LCBzbyB0aGUgaW5wdXQgc2lnbmFscyBmcm9tIERDVSBhcmUNCj4gcGFzc2VkDQo+ID4gKwkgKiB0
aHJvdWdoIFRDT04gdW5jaGFuZ2VkICovDQo+ID4gKwlyZXQgPSBieXBhc3NfdGNvbihwZGV2LT5k
ZXYub2Zfbm9kZSk7DQo+ID4gKwlpZiAocmV0KSB7DQo+ID4gKwkJZGV2X2VycigmcGRldi0+ZGV2
LCAiY291bGQgbm90IGJ5cGFzcyBUQ09OXG4iKTsNCj4gPiArCQlnb3RvIGZhaWxlZF9ieXBhc3N0
Y29uOw0KPiA+ICsJfQ0KPiA+ICsNCj4gPiArCWRjdWZiLT5jbGsgPSBkZXZtX2Nsa19nZXQoJnBk
ZXYtPmRldiwgImRjdSIpOw0KPiA+ICsJaWYgKElTX0VSUihkY3VmYi0+Y2xrKSkgew0KPiA+ICsJ
CWRldl9lcnIoJnBkZXYtPmRldiwgImNvdWxkIG5vdCBnZXQgY2xvY2tcbiIpOw0KPiA+ICsJCWdv
dG8gZmFpbGVkX2dldGNsb2NrOw0KPiANCj4gWW91IHdpbGwgcmV0dXJuIDAgaGVyZS4NCltBbGlz
b24gV2FuZ10gWW91IGFyZSByaWdodCwgSSB3aWxsIGZpeCBpdCBpbiBuZXh0IHZlcnNpb24uDQo+
IA0KPiA+ICsJfQ0KPiA+ICsJY2xrX3ByZXBhcmVfZW5hYmxlKGRjdWZiLT5jbGspOw0KPiA+ICsN
Cj4gPiArCWZvciAoaSA9IDA7IGkgPCBBUlJBWV9TSVpFKGRjdWZiLT5mc2xfZGN1X2luZm8pOyBp
KyspIHsNCj4gPiArCQlkY3VmYi0+ZnNsX2RjdV9pbmZvW2ldID0NCj4gPiArCQkJZnJhbWVidWZm
ZXJfYWxsb2Moc2l6ZW9mKHN0cnVjdCBtZmJfaW5mbyksICZwZGV2LQ0KPiA+ZGV2KTsNCj4gPiAr
CQlpZiAoIWRjdWZiLT5mc2xfZGN1X2luZm9baV0pIHsNCj4gPiArCQkJcmV0ID0gRU5PTUVNOw0K
PiANCj4gLUVOT01FTQ0KPiANCj4gPiArZmFpbGVkX2FsbG9jX2ZyYW1lYnVmZmVyOg0KPiA+ICtm
YWlsZWRfZ2V0Y2xvY2s6DQo+ID4gK2ZhaWxlZF9ieXBhc3N0Y29uOg0KPiA+ICsJZnJlZV9pcnEo
ZGN1ZmItPmlycSwgZGN1ZmIpOw0KPiA+ICtmYWlsZWRfcmVxdWVzdGlycToNCj4gPiArZmFpbGVk
X2dldGlycToNCj4gPiArCWlvdW5tYXAoZGN1ZmItPnJlZ19iYXNlKTsNCj4gDQo+IFlvdSB1c2Vk
IGRldm1faW9yZW1hcCwgc28gZHJvcCB0aGlzLg0KW0FsaXNvbiBXYW5nXSBPay4NCj4gDQo+ID4g
K2ZhaWxlZF9pb3JlbWFwOg0KPiA+ICsJa2ZyZWUoZGN1ZmIpOw0KPiANCj4gVGhpcyBpcyBhbGxv
Y2F0ZWQgd2l0aCBkZXZtXyouIERyb3AgdGhpcy4NCltBbGlzb24gV2FuZ10gT2suDQoNClRoYW5r
cyBmb3IgeW91ciBjb21tZW50cy4NCg0KDQpCZXN0IFJlZ2FyZHMsDQpBbGlzb24gV2FuZw0KDQoN
Cg=
^ permalink raw reply
* Re: [PATCH 4/5] fb: Add DCU framebuffer driver for Vybrid VF610 platform
From: Sascha Hauer @ 2013-08-05 10:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <81BA6E5E0BC2344391CABCEE22D1B6D83CA6F0@039-SN1MPN1-002.039d.mgd.msft.net>
On Mon, Aug 05, 2013 at 09:51:40AM +0000, Wang Huan-B18965 wrote:
> Hi, Sascha,
>
> > On Fri, Jul 12, 2013 at 02:07:55PM +0800, Alison Wang wrote:
> > > The Display Controller Unit (DCU) module is a system master that
> > > fetches graphics stored in internal or external memory and displays
> > > them on a TFT LCD panel. A wide range of panel sizes is supported and
> > > the timing of the interface signals is highly configurable.
> > > Graphics are read directly from memory and then blended in real-time,
> > > which allows for dynamic content creation with minimal CPU
> > intervention.
> >
> > Only a review of the code inline.
> >
> > Maybe the real question is whether we want to introduce another
> > framebuffer driver at all instead of making it a DRM driver.
> [Alison Wang] I think DCU module is more suitable to be designed as a framebuffer driver than a DRM driver. Just like DIU framebuffer driver for PowerPC.
> >
> > > +
> > > +#define DRIVER_NAME "fsl-dcu-fb"
> > > +
> > > +#define DCU_DCU_MODE 0x0010
> > > +#define DCU_MODE_BLEND_ITER(x) (x << 20)
> >
> > What's the result of DCU_MODE_BLEND_ITER(1 + 1)?
> [Alison Wang] Is it really necessary? I don't use this macro like DCU_MODE_BLEND_ITER(1 + 1), just use DCU_MODE_BLEND_ITER(2).
<irony>
No it's not necessary. We can just add incorrect macros everywhere and
fix them as necessary after a long bug squashing session
</irony>
YES! This is necessary.
> > > + return 0;
> > > +}
> > > +
> > > +static int calc_div_ratio(struct fb_info *info) {
> >
> > Use a consistent namespace for function names (fsl_dcu_)
> [Alison Wang] Is it necessary to use a consistent namespace for this generic function? I think it is necessary to use a consistent namespace (fsl_dcu_) for the function names in structure fsl_dcu_ops.
This function calculates some divider out of a struct fb_info. This is
by no means generic.
> > > + if (copy_to_user(buf, &alpha, sizeof(alpha)))
> > > + return -EFAULT;
> > > + break;
> > > + case MFB_SET_ALPHA:
> > > + if (copy_from_user(&alpha, buf, sizeof(alpha)))
> > > + return -EFAULT;
> > > + mfbi->blend = 1;
> > > + mfbi->alpha = alpha;
> > > + fsl_dcu_set_par(info);
> > > + break;
> >
> > Is it still state of the art to introduce ioctls in the fb framework
> > without any kind of documentation?
> [Alison Wang] Do you mean I need to add a documentation in Documentation/fb/?
This was more a question to the fb maintainers.
> > > +static irqreturn_t fsl_dcu_irq(int irq, void *dev_id) {
> > > + struct dcu_fb_data *dcufb = dev_id;
> > > + unsigned int status = readl(dcufb->reg_base + DCU_INT_STATUS);
> > > + u32 dcu_mode;
> > > +
> > > + if (status) {
> >
> > Save indendation level by bailing out early.
> [Alison Wang] What do you mean?
Instead of doing:
if (bla) {
dothis();
dothat();
return;
}
return;
it's easier to read:
if (!bla)
return;
dothis();
dothat();
return;
Note that dothis() and dothat() are one indentation level to the left
and thus have more space per line.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 4/5] fb: Add DCU framebuffer driver for Vybrid VF610 platform
From: Robert Schwebel @ 2013-08-05 13:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <81BA6E5E0BC2344391CABCEE22D1B6D83CA6F0@039-SN1MPN1-002.039d.mgd.msft.net>
On Mon, Aug 05, 2013 at 09:51:40AM +0000, Wang Huan-B18965 wrote:
> > On Fri, Jul 12, 2013 at 02:07:55PM +0800, Alison Wang wrote:
> > > The Display Controller Unit (DCU) module is a system master that
> > > fetches graphics stored in internal or external memory and displays
> > > them on a TFT LCD panel. A wide range of panel sizes is supported and
> > > the timing of the interface signals is highly configurable.
> > > Graphics are read directly from memory and then blended in real-time,
> > > which allows for dynamic content creation with minimal CPU
> > intervention.
> >
> > Only a review of the code inline.
> >
> > Maybe the real question is whether we want to introduce another
> > framebuffer driver at all instead of making it a DRM driver.
> [Alison Wang] I think DCU module is more suitable to be designed as a framebuffer driver than a DRM driver. Just like DIU framebuffer driver for PowerPC.
We looked at the Vybrid datasheet and it's DCU section last week, and
with its 64 planes, the controller really wants to get a DRM driver.
rsc
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 4/5] fb: Add DCU framebuffer driver for Vybrid VF610 platform
From: Lucas Stach @ 2013-08-05 14:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130805130911.GF30920@pengutronix.de>
Am Montag, den 05.08.2013, 15:09 +0200 schrieb Robert Schwebel:
> On Mon, Aug 05, 2013 at 09:51:40AM +0000, Wang Huan-B18965 wrote:
> > > On Fri, Jul 12, 2013 at 02:07:55PM +0800, Alison Wang wrote:
> > > > The Display Controller Unit (DCU) module is a system master that
> > > > fetches graphics stored in internal or external memory and displays
> > > > them on a TFT LCD panel. A wide range of panel sizes is supported and
> > > > the timing of the interface signals is highly configurable.
> > > > Graphics are read directly from memory and then blended in real-time,
> > > > which allows for dynamic content creation with minimal CPU
> > > intervention.
> > >
> > > Only a review of the code inline.
> > >
> > > Maybe the real question is whether we want to introduce another
> > > framebuffer driver at all instead of making it a DRM driver.
> > [Alison Wang] I think DCU module is more suitable to be designed as a framebuffer driver than a DRM driver. Just like DIU framebuffer driver for PowerPC.
>
> We looked at the Vybrid datasheet and it's DCU section last week, and
> with its 64 planes, the controller really wants to get a DRM driver.
>
Exactly, with it's extensive hardware composition capabilities the
controller begs for being driven by a proper DRM driver. There is no way
in fbdev to properly support all those hardware planes without
introducing a lot of non standard ioctls, which in turn will force you
into writing a lot of custom userspace code instead of running proven
technology that sits on top of KMS.
Doing things in DRM might be slightly more work for the initial bring
up, but will pay off in the long run when you are going to really use
the hardware caps of the controller.
Regards,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [RFC 0/1] drm/pl111: Initial drm/kms driver for pl111
From: Rob Clark @ 2013-08-05 18:07 UTC (permalink / raw)
To: Tom Cooksey
Cc: dri-devel, linux-fbdev, Pawel Moll, linux-arm-kernel, linux-media,
linaro-mm-sig
In-Reply-To: <51ffdc7e.06b8b40a.2cc8.0fe0SMTPIN_ADDED_BROKEN@mx.google.com>
On Mon, Aug 5, 2013 at 1:10 PM, Tom Cooksey <tom.cooksey@arm.com> wrote:
> Hi Rob,
>
> +linux-media, +linaro-mm-sig for discussion of video/camera
> buffer constraints...
>
>
>> On Fri, Jul 26, 2013 at 11:58 AM, Tom Cooksey <tom.cooksey@arm.com>
>> wrote:
>> >> > * It abuses flags parameter of DRM_IOCTL_MODE_CREATE_DUMB to also
>> >> > allocate buffers for the GPU. Still not sure how to resolve
>> >> > this as we don't use DRM for our GPU driver.
>> >>
>> >> any thoughts/plans about a DRM GPU driver? Ideally long term (esp.
>> >> once the dma-fence stuff is in place), we'd have gpu-specific drm
>> >> (gpu-only, no kms) driver, and SoC/display specific drm/kms driver,
>> >> using prime/dmabuf to share between the two.
>> >
>> > The "extra" buffers we were allocating from armsoc DDX were really
>> > being allocated through DRM/GEM so we could get an flink name
>> > for them and pass a reference to them back to our GPU driver on
>> > the client side. If it weren't for our need to access those
>> > extra off-screen buffers with the GPU we wouldn't need to
>> > allocate them with DRM at all. So, given they are really "GPU"
>> > buffers, it does absolutely make sense to allocate them in a
>> > different driver to the display driver.
>> >
>> > However, to avoid unnecessary memcpys & related cache
>> > maintenance ops, we'd also like the GPU to render into buffers
>> > which are scanned out by the display controller. So let's say
>> > we continue using DRM_IOCTL_MODE_CREATE_DUMB to allocate scan
>> > out buffers with the display's DRM driver but a custom ioctl
>> > on the GPU's DRM driver to allocate non scanout, off-screen
>> > buffers. Sounds great, but I don't think that really works
>> > with DRI2. If we used two drivers to allocate buffers, which
>> > of those drivers do we return in DRI2ConnectReply? Even if we
>> > solve that somehow, GEM flink names are name-spaced to a
>> > single device node (AFAIK). So when we do a DRI2GetBuffers,
>> > how does the EGL in the client know which DRM device owns GEM
>> > flink name "1234"? We'd need some pretty dirty hacks.
>>
>> You would return the name of the display driver allocating the
>> buffers. On the client side you can use generic ioctls to go from
>> flink -> handle -> dmabuf. So the client side would end up opening
>> both the display drm device and the gpu, but without needing to know
>> too much about the display.
>
> I think the bit I was missing was that a GEM bo for a buffer imported
> using dma_buf/PRIME can still be flink'd. So the display controller's
> DRM driver allocates scan-out buffers via the DUMB buffer allocate
> ioctl. Those scan-out buffers than then be exported from the
> dispaly's DRM driver and imported into the GPU's DRM driver using
> PRIME. Once imported into the GPU's driver, we can use flink to get a
> name for that buffer within the GPU DRM driver's name-space to return
> to the DRI2 client. That same namespace is also what DRI2 back-buffers
> are allocated from, so I think that could work... Except...
>
(and.. the general direction is that things will move more to just use
dmabuf directly, ie. wayland or dri3)
>
>> > Anyway, that latter case also gets quite difficult. The "GPU"
>> > DRM driver would need to know the constraints of the display
>> > controller when allocating buffers intended to be scanned out.
>> > For example, pl111 typically isn't behind an IOMMU and so
>> > requires physically contiguous memory. We'd have to teach the
>> > GPU's DRM driver about the constraints of the display HW. Not
>> > exactly a clean driver model. :-(
>> >
>> > I'm still a little stuck on how to proceed, so any ideas
>> > would greatly appreciated! My current train of thought is
>> > having a kind of SoC-specific DRM driver which allocates
>> > buffers for both display and GPU within a single GEM
>> > namespace. That SoC-specific DRM driver could then know the
>> > constraints of both the GPU and the display HW. We could then
>> > use PRIME to export buffers allocated with the SoC DRM driver
>> > and import them into the GPU and/or display DRM driver.
>>
>> Usually if the display drm driver is allocating the buffers that might
>> be scanned out, it just needs to have minimal knowledge of the GPU
>> (pitch alignment constraints). I don't think we need a 3rd device
>> just to allocate buffers.
>
> While Mali can render to pretty much any buffer, there is a mild
> performance improvement to be had if the buffer stride is aligned to
> the AXI bus's max burst length when drawing to the buffer.
I suspect the display controllers might frequently benefit if the
pitch is aligned to AXI burst length too..
> So in some respects, there is a constraint on how buffers which will
> be drawn to using the GPU are allocated. I don't really like the idea
> of teaching the display controller DRM driver about the GPU buffer
> constraints, even if they are fairly trivial like this. If the same
> display HW IP is being used on several SoCs, it seems wrong somehow
> to enforce those GPU constraints if some of those SoCs don't have a
> GPU.
Well, I suppose you could get min_pitch_alignment from devicetree, or
something like this..
In the end, the easy solution is just to make the display allocate to
the worst-case pitch alignment. In the early days of dma-buf
discussions, we kicked around the idea of negotiating or
programatically describing the constraints, but that didn't really
seem like a bounded problem.
> We may also then have additional constraints when sharing buffers
> between the display HW and video decode or even camera ISP HW.
> Programmatically describing buffer allocation constraints is very
> difficult and I'm not sure you can actually do it - there's some
> pretty complex constraints out there! E.g. I believe there's a
> platform where Y and UV planes of the reference frame need to be in
> separate DRAM banks for real-time 1080p decode, or something like
> that?
yes, this was discussed. This is different from pitch/format/size
constraints.. it is really just a placement constraint (ie. where do
the physical pages go). IIRC the conclusion was to use a dummy
devices with it's own CMA pool for attaching the Y vs UV buffers.
> Anyway, I guess my point is that even if we solve how to allocate
> buffers which will be shared between the GPU and display HW such that
> both sets of constraints are satisfied, that may not be the end of
> the story.
>
that was part of the reason to punt this problem to userspace ;-)
In practice, the kernel drivers doesn't usually know too much about
the dimensions/format/etc.. that is really userspace level knowledge.
There are a few exceptions when the kernel needs to know how to setup
GTT/etc for tiled buffers, but normally this sort of information is up
at the next level up (userspace, and drm_framebuffer in case of
scanout). Userspace media frameworks like GStreamer already have a
concept of format/caps negotiation. For non-display<->gpu sharing, I
think this is probably where this sort of constraint negotiation
should be handled.
BR,
-R
>
> Cheers,
>
> Tom
>
>
>
>
>
^ permalink raw reply
* source code file for the generic vga driver?
From: Haiyang Zhang @ 2013-08-05 19:12 UTC (permalink / raw)
To: linux-fbdev@vger.kernel.org,
driverdev-devel@linuxdriverproject.org, Tomi Valkeinen,
Jean-Christophe Plagniol-Villard, akpm@linux-foundation.org
Hi folks,
I'm working on an issue of HyperV synthetic frame buffer driver, which seems to have
a conflict with the generic vga driver (not the vesa driver). I hope to read and trace into
the source code for the generic vga driver...
Can anyone point me to the source code file for the generic vga driver in the kernel tree?
Thanks,
- Haiyang
^ permalink raw reply
* Re: [PATCH] fbdev: suppress warning when assigning vga-save/restore base
From: Ondrej Zajicek @ 2013-08-05 20:29 UTC (permalink / raw)
To: David Miller
Cc: hpa, dh.herrmann, linux-fbdev, linux-kernel, plagnioj,
tomi.valkeinen, Ondrej Zary
In-Reply-To: <20130804.185146.1621476659301936865.davem@davemloft.net>
On Sun, Aug 04, 2013 at 06:51:46PM -0700, David Miller wrote:
> From: "H. Peter Anvin" <hpa@zytor.com>
> Date: Sun, 04 Aug 2013 10:33:46 -0700
>
> > Anyone who can dig backwards and summarize? In other words:
> >
> > Where in the current code do we stuff a physical address in a pointer,
> > or a virtual address into a non-pointer?
>
> The VGA register accessors try to accomodate iomem and ioport
> accesses.
>
> If they are given a non-NULL iomem pointer 'regbase' they use
> iomem accesses, otherwise they do direct ISA port poking.
>
> And yes the drivers in question are making some brash assumptions.
> I suspect they should be using ioremap() or similar.
Well, these drivers were written without MMIO (port IO only with NULL
instead of 'vgabase' for VGA register accessors). They were converted in
patches 94c322c30bd14ae6cdd369cb4a1f94c5c3809ac9,
f8645933513c65ac55f23c63b2649097289795c6 and a few others (from David
Miller) to potentially use MMIO by using 'vgabase' instead of NULL:
pcibios_bus_to_resource(dev, &vga_res, &bus_reg);
par->state.vgabase = (void __iomem *) vga_res.start;
How this could even work? AFAIK these cards have to be explicitly programmed
to enable MMIO (which was not done in the patches). These patches claim that
it is for multi-domain PCI. I would guess that vgabase is NULL in common
configurations but if it is non-NULL, it probably wouldn't work, unless
there is some hardware magic that transparently converts MMIO (from CPU PoV)
to port IO (from card/PCI PoV).
Note that there are some later patches (86c0f043a737dadf034a4e6f29aefb074f4a1146)
from Ondrej Zary that independently enable MMIO, but they use par->mmio
field instead of par->state.vgabase .
--
Elen sila lumenn' omentielvo
Ondrej 'SanTiago' Zajicek (email: santiago@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."
^ permalink raw reply
* Re: [PATCH] fbdev: suppress warning when assigning vga-save/restore base
From: H. Peter Anvin @ 2013-08-05 21:02 UTC (permalink / raw)
To: Ondrej Zajicek
Cc: David Miller, dh.herrmann, linux-fbdev, linux-kernel, plagnioj,
tomi.valkeinen, Ondrej Zary
In-Reply-To: <20130805202955.GH22789@localhost>
On 08/05/2013 01:29 PM, Ondrej Zajicek wrote:
>
> How this could even work? AFAIK these cards have to be explicitly programmed
> to enable MMIO (which was not done in the patches). These patches claim that
> it is for multi-domain PCI. I would guess that vgabase is NULL in common
> configurations but if it is non-NULL, it probably wouldn't work, unless
> there is some hardware magic that transparently converts MMIO (from CPU PoV)
> to port IO (from card/PCI PoV).
>
They presumably use iowrite/ioread, which use either I/O instructions or
memory instructions depending on the address.
-hpa
^ permalink raw reply
* Re: [PATCH] fbdev: suppress warning when assigning vga-save/restore base
From: H. Peter Anvin @ 2013-08-05 21:14 UTC (permalink / raw)
To: Ondrej Zajicek
Cc: David Miller, dh.herrmann, linux-fbdev, linux-kernel, plagnioj,
tomi.valkeinen, Ondrej Zary
In-Reply-To: <520012D4.7020701@zytor.com>
On 08/05/2013 02:02 PM, H. Peter Anvin wrote:
> On 08/05/2013 01:29 PM, Ondrej Zajicek wrote:
>>
>> How this could even work? AFAIK these cards have to be explicitly programmed
>> to enable MMIO (which was not done in the patches). These patches claim that
>> it is for multi-domain PCI. I would guess that vgabase is NULL in common
>> configurations but if it is non-NULL, it probably wouldn't work, unless
>> there is some hardware magic that transparently converts MMIO (from CPU PoV)
>> to port IO (from card/PCI PoV).
>>
>
> They presumably use iowrite/ioread, which use either I/O instructions or
> memory instructions depending on the address.
>
So the confusion really is in the iowrite interface, which can take
either a physical port address or a virtual memory address...
-hpa
^ permalink raw reply
* [PATCH v3 00/20] video: da8xx-fb: driver enhance to support TI am335x SoC
From: Darren Etheridge @ 2013-08-05 22:02 UTC (permalink / raw)
To: linux-fbdev
Changes in v3:
Address the patch authorship requirements
Pull out the DT changes into a separate series
Fix an issue where FB_MODE_HELPERS was not included in build
but is a dependency.
Include the devm_ changes and error reporting that Prabhakar Lad
had pushed a patch for,
A few more cosmetic changes
Add a patch to remove the use on inline from a number of functions
Changes in v2:
Addressing review comments from Tomi Valkeinen:
Dropped readl/writel patch as it is not necessary for non-cached,
non-buffered memory.
Many cosmetic changes to make code easier to understand
This is primarily a resend of a series of patches that were original
submitted to linux-fbdev back in January of 2013 for 3.8 by Afzal
Mohammed. I have rebased them on 3.10 and also made sure they
apply cleanly to the 'for-next' branch of linux-fbdev git.
The patches enable use of the current mainline da8xx-fb driver on the
TI AM335x SOC along with some bug fixes and cleanup.
The original patch series can be found here:
https://patchwork.kernel.org/project/linux-fbdev/list/?submitter9101
if you want to see the history.
Afzal Mohammed (11):
video: da8xx-fb: fb_check_var enhancement
video: da8xx-fb: simplify lcd_reset
video: da8xx-fb: use modedb helper to update var
video: da8xx-fb: remove unneeded "var" initialization
video: da8xx-fb: store current display information
video: da8xx-fb: store clk rate even if !CPUFREQ
video: da8xx-fb: store struct device *
video: da8xx-fb: report correct pixclock
video: da8xx-fb: enable sync lost intr for v2 ip
video: da8xx-fb: ensure non-null cfg in pdata
video: da8xx-fb: reorganize panel detection
Darren Etheridge (9):
video: da8xx-fb: pix clk and clk div handling cleanup
video: da8xx-fb: fb_set_par support
video: da8xx-fb: improve readability of code
video: da8xx-fb: fix 24bpp raster configuration
video: da8xx-fb: use devres
video: da8xx-fb: set upstream clock rate (if reqd)
video: da8xx-fb: make clock naming consistent
video: da8xx-fb: let compiler decide what to inline
video: da8xx-fb: adding am33xx as dependency
drivers/video/Kconfig | 8 +-
drivers/video/da8xx-fb.c | 352 +++++++++++++++++++++++++++-------------------
include/video/da8xx-fb.h | 5 +
3 files changed, 216 insertions(+), 149 deletions(-)
^ permalink raw reply
* [PATCH v3 01/20] video: da8xx-fb: fb_check_var enhancement
From: Darren Etheridge @ 2013-08-05 22:02 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
Check whether "struct fb_var_screeninfo" fields are sane, if not
update it to be within allowed limits.
If user sends down buggy "var" values, this will bring those within
allowable limits. And fb_set_par is not supposed to change "var"
values, fb_check_var has to ensure that values are proper.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 0810939..d00dd17 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -888,6 +888,9 @@ static int fb_check_var(struct fb_var_screeninfo *var,
struct fb_info *info)
{
int err = 0;
+ struct da8xx_fb_par *par = info->par;
+ int bpp = var->bits_per_pixel >> 3;
+ unsigned long line_size = var->xres_virtual * bpp;
if (var->bits_per_pixel > 16 && lcd_revision = LCD_VERSION_1)
return -EINVAL;
@@ -955,6 +958,21 @@ static int fb_check_var(struct fb_var_screeninfo *var,
var->green.msb_right = 0;
var->blue.msb_right = 0;
var->transp.msb_right = 0;
+
+ if (line_size * var->yres_virtual > par->vram_size)
+ var->yres_virtual = par->vram_size / line_size;
+
+ if (var->yres > var->yres_virtual)
+ var->yres = var->yres_virtual;
+
+ if (var->xres > var->xres_virtual)
+ var->xres = var->xres_virtual;
+
+ if (var->xres + var->xoffset > var->xres_virtual)
+ var->xoffset = var->xres_virtual - var->xres;
+ if (var->yres + var->yoffset > var->yres_virtual)
+ var->yoffset = var->yres_virtual - var->yres;
+
return err;
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 02/20] video: da8xx-fb: simplify lcd_reset
From: Darren Etheridge @ 2013-08-05 22:02 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
lcd_reset function doesn't require any arguement, remove it.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index d00dd17..52977b1 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -681,7 +681,7 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
}
#undef CNVT_TOHW
-static void lcd_reset(struct da8xx_fb_par *par)
+static void da8xx_fb_lcd_reset(void)
{
/* Disable the Raster if previously Enabled */
lcd_disable_raster(false);
@@ -721,7 +721,7 @@ static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
u32 bpp;
int ret = 0;
- lcd_reset(par);
+ da8xx_fb_lcd_reset();
/* Calculate the divider */
lcd_calc_clk_divider(par);
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 03/20] video: da8xx-fb: use modedb helper to update var
From: Darren Etheridge @ 2013-08-05 22:02 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
modedb structure is now used to store panel information, run modedb
helper over it for initial update of "var" information instead of
equating each fields.
While at it, remove redundant update of bits_per_pixel.
Note: pixclock is overridden with proper value using an existing code
as currently modedb is having it in Hz instead of ps, this would be
fixed in a later change and this overide would be removed.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 18 ++----------------
1 files changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 52977b1..a1f6544 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1329,6 +1329,8 @@ static int fb_probe(struct platform_device *device)
par->panel_power_ctrl(1);
}
+ fb_videomode_to_var(&da8xx_fb_var, lcdc_info);
+
if (lcd_init(par, lcd_cfg, lcdc_info) < 0) {
dev_err(&device->dev, "lcd_init failed\n");
ret = -EFAULT;
@@ -1381,25 +1383,9 @@ static int fb_probe(struct platform_device *device)
goto err_release_pl_mem;
}
- /* Initialize par */
- da8xx_fb_info->var.bits_per_pixel = lcd_cfg->bpp;
-
- da8xx_fb_var.xres = lcdc_info->xres;
- da8xx_fb_var.xres_virtual = lcdc_info->xres;
-
- da8xx_fb_var.yres = lcdc_info->yres;
- da8xx_fb_var.yres_virtual = lcdc_info->yres * LCD_NUM_BUFFERS;
-
da8xx_fb_var.grayscale lcd_cfg->panel_shade = MONOCHROME ? 1 : 0;
da8xx_fb_var.bits_per_pixel = lcd_cfg->bpp;
-
- da8xx_fb_var.hsync_len = lcdc_info->hsync_len;
- da8xx_fb_var.vsync_len = lcdc_info->vsync_len;
- da8xx_fb_var.right_margin = lcdc_info->right_margin;
- da8xx_fb_var.left_margin = lcdc_info->left_margin;
- da8xx_fb_var.lower_margin = lcdc_info->lower_margin;
- da8xx_fb_var.upper_margin = lcdc_info->upper_margin;
da8xx_fb_var.pixclock = da8xxfb_pixel_clk_period(par);
/* Initialize fbinfo */
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 04/20] video: da8xx-fb: remove unneeded "var" initialization
From: Darren Etheridge @ 2013-08-05 22:02 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
modedb helper now updates "var" information based on the detected
panel, remove the unnecessary initialization.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 22 +---------------------
1 files changed, 1 insertions(+), 21 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index a1f6544..18834fa 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -131,10 +131,6 @@
#define WSI_TIMEOUT 50
#define PALETTE_SIZE 256
-#define LEFT_MARGIN 64
-#define RIGHT_MARGIN 64
-#define UPPER_MARGIN 32
-#define LOWER_MARGIN 32
static void __iomem *da8xx_fb_reg_base;
static struct resource *lcdc_regs;
@@ -184,23 +180,7 @@ struct da8xx_fb_par {
u32 pseudo_palette[16];
};
-/* Variable Screen Information */
-static struct fb_var_screeninfo da8xx_fb_var = {
- .xoffset = 0,
- .yoffset = 0,
- .transp = {0, 0, 0},
- .nonstd = 0,
- .activate = 0,
- .height = -1,
- .width = -1,
- .accel_flags = 0,
- .left_margin = LEFT_MARGIN,
- .right_margin = RIGHT_MARGIN,
- .upper_margin = UPPER_MARGIN,
- .lower_margin = LOWER_MARGIN,
- .sync = 0,
- .vmode = FB_VMODE_NONINTERLACED
-};
+static struct fb_var_screeninfo da8xx_fb_var;
static struct fb_fix_screeninfo da8xx_fb_fix = {
.id = "DA8xx FB Drv",
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 05/20] video: da8xx-fb: store current display information
From: Darren Etheridge @ 2013-08-05 22:02 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
store current videomode and controller data so that reconfiguring can
be done easily. Reconfiguring would be required in fb_set_par, which
is going to be added soon.
If these details are not stored, the work probe does to retrieve these
information would have to repeated at the place of reconfiguring and
modifying platform data would be necessary to handle controller data
changes like bpp.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 18834fa..d060f14 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -178,6 +178,8 @@ struct da8xx_fb_par {
#endif
void (*panel_power_ctrl)(int);
u32 pseudo_palette[16];
+ struct fb_videomode mode;
+ struct lcd_ctrl_config cfg;
};
static struct fb_var_screeninfo da8xx_fb_var;
@@ -1310,6 +1312,8 @@ static int fb_probe(struct platform_device *device)
}
fb_videomode_to_var(&da8xx_fb_var, lcdc_info);
+ fb_var_to_videomode(&par->mode, &da8xx_fb_var);
+ par->cfg = *lcd_cfg;
if (lcd_init(par, lcd_cfg, lcdc_info) < 0) {
dev_err(&device->dev, "lcd_init failed\n");
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 06/20] video: da8xx-fb: store clk rate even if !CPUFREQ
From: Darren Etheridge @ 2013-08-05 22:02 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
store lcd clk rate always, i.e. irrespective of whether CPUFREQ is
enabled or not. This can be used to get clk rate directly instead of
enquiring with clock framework with clk handle every time.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index d060f14..f1d88ac 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -174,8 +174,8 @@ struct da8xx_fb_par {
unsigned int which_dma_channel_done;
#ifdef CONFIG_CPU_FREQ
struct notifier_block freq_transition;
- unsigned int lcd_fck_rate;
#endif
+ unsigned int lcd_fck_rate;
void (*panel_power_ctrl)(int);
u32 pseudo_palette[16];
struct fb_videomode mode;
@@ -1302,9 +1302,7 @@ static int fb_probe(struct platform_device *device)
par = da8xx_fb_info->par;
par->lcdc_clk = fb_clk;
-#ifdef CONFIG_CPU_FREQ
par->lcd_fck_rate = clk_get_rate(fb_clk);
-#endif
par->pxl_clk = lcdc_info->pixclock;
if (fb_pdata->panel_power_ctrl) {
par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 07/20] video: da8xx-fb: pix clk and clk div handling cleanup
From: Darren Etheridge @ 2013-08-05 22:02 UTC (permalink / raw)
To: linux-fbdev
Based on original patch by: Afzal Mohammed <afzal@ti.com>
Use the new modedb field to store pix clk. Reorganize existing clock
divider functions with names now corresponding to what they do, add
common function prefix.
Fix existing panel modedb pixclock to be in ps instead of Hz. This
needed a change in the way clock divider is calculated. As modedb
pixclock information is now in ps, override on "var" pixclock over
modedb to var conversion is removed.
v2:
Changed pixel clock configuration to use KHZ2PICOS macro
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 48 +++++++++++++++++----------------------------
1 files changed, 18 insertions(+), 30 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index f1d88ac..0fac1a0 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -160,7 +160,6 @@ struct da8xx_fb_par {
struct clk *lcdc_clk;
int irq;
unsigned int palette_sz;
- unsigned int pxl_clk;
int blank;
wait_queue_head_t vsync_wait;
int vsync_flag;
@@ -201,7 +200,7 @@ static struct fb_videomode known_lcd_panels[] = {
.name = "Sharp_LCD035Q3DG01",
.xres = 320,
.yres = 240,
- .pixclock = 4608000,
+ .pixclock = KHZ2PICOS(4607),
.left_margin = 6,
.right_margin = 8,
.upper_margin = 2,
@@ -216,7 +215,7 @@ static struct fb_videomode known_lcd_panels[] = {
.name = "Sharp_LK043T1DG01",
.xres = 480,
.yres = 272,
- .pixclock = 7833600,
+ .pixclock = KHZ2PICOS(7833),
.left_margin = 2,
.right_margin = 2,
.upper_margin = 2,
@@ -231,7 +230,7 @@ static struct fb_videomode known_lcd_panels[] = {
.name = "SP10Q010",
.xres = 320,
.yres = 240,
- .pixclock = 7833600,
+ .pixclock = KHZ2PICOS(7833),
.left_margin = 10,
.right_margin = 10,
.upper_margin = 10,
@@ -680,13 +679,14 @@ static void da8xx_fb_lcd_reset(void)
}
}
-static void lcd_calc_clk_divider(struct da8xx_fb_par *par)
+static inline unsigned da8xx_fb_calc_clk_divider(struct da8xx_fb_par *par,
+ unsigned pixclock)
{
- unsigned int lcd_clk, div;
-
- lcd_clk = clk_get_rate(par->lcdc_clk);
- div = lcd_clk / par->pxl_clk;
+ return par->lcd_fck_rate / (PICOS2KHZ(pixclock) * 1000);
+}
+static inline void da8xx_fb_config_clk_divider(unsigned div)
+{
/* Configure the LCD clock divisor. */
lcdc_write(LCD_CLK_DIVISOR(div) |
(LCD_RASTER_MODE & 0x1), LCD_CTRL_REG);
@@ -694,7 +694,14 @@ static void lcd_calc_clk_divider(struct da8xx_fb_par *par)
if (lcd_revision = LCD_VERSION_2)
lcdc_write(LCD_V2_DMA_CLK_EN | LCD_V2_LIDD_CLK_EN |
LCD_V2_CORE_CLK_EN, LCD_CLK_ENABLE_REG);
+}
+
+static inline void da8xx_fb_calc_config_clk_divider(struct da8xx_fb_par *par,
+ struct fb_videomode *mode)
+{
+ unsigned div = da8xx_fb_calc_clk_divider(par, mode->pixclock);
+ da8xx_fb_config_clk_divider(div);
}
static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
@@ -705,8 +712,7 @@ static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
da8xx_fb_lcd_reset();
- /* Calculate the divider */
- lcd_calc_clk_divider(par);
+ da8xx_fb_calc_config_clk_divider(par, panel);
if (panel->sync & FB_SYNC_CLK_INVERT)
lcdc_write((lcdc_read(LCD_RASTER_TIMING_2_REG) |
@@ -969,7 +975,7 @@ static int lcd_da8xx_cpufreq_transition(struct notifier_block *nb,
if (par->lcd_fck_rate != clk_get_rate(par->lcdc_clk)) {
par->lcd_fck_rate = clk_get_rate(par->lcdc_clk);
lcd_disable_raster(true);
- lcd_calc_clk_divider(par);
+ da8xx_fb_calc_config_clk_divider(par, &par->mode);
if (par->blank = FB_BLANK_UNBLANK)
lcd_enable_raster();
}
@@ -1195,22 +1201,6 @@ static struct fb_ops da8xx_fb_ops = {
.fb_blank = cfb_blank,
};
-/* Calculate and return pixel clock period in pico seconds */
-static unsigned int da8xxfb_pixel_clk_period(struct da8xx_fb_par *par)
-{
- unsigned int lcd_clk, div;
- unsigned int configured_pix_clk;
- unsigned long long pix_clk_period_picosec = 1000000000000ULL;
-
- lcd_clk = clk_get_rate(par->lcdc_clk);
- div = lcd_clk / par->pxl_clk;
- configured_pix_clk = (lcd_clk / div);
-
- do_div(pix_clk_period_picosec, configured_pix_clk);
-
- return pix_clk_period_picosec;
-}
-
static int fb_probe(struct platform_device *device)
{
struct da8xx_lcdc_platform_data *fb_pdata @@ -1303,7 +1293,6 @@ static int fb_probe(struct platform_device *device)
par = da8xx_fb_info->par;
par->lcdc_clk = fb_clk;
par->lcd_fck_rate = clk_get_rate(fb_clk);
- par->pxl_clk = lcdc_info->pixclock;
if (fb_pdata->panel_power_ctrl) {
par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
par->panel_power_ctrl(1);
@@ -1368,7 +1357,6 @@ static int fb_probe(struct platform_device *device)
da8xx_fb_var.grayscale lcd_cfg->panel_shade = MONOCHROME ? 1 : 0;
da8xx_fb_var.bits_per_pixel = lcd_cfg->bpp;
- da8xx_fb_var.pixclock = da8xxfb_pixel_clk_period(par);
/* Initialize fbinfo */
da8xx_fb_info->flags = FBINFO_FLAG_DEFAULT;
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 08/20] video: da8xx-fb: store struct device *
From: Darren Etheridge @ 2013-08-05 22:02 UTC (permalink / raw)
To: linux-fbdev
From: Afzal Mohammed <afzal@ti.com>
store struct device pointer so that dev_dbg/err can be used outside
of probe.
Signed-off-by: Afzal Mohammed <afzal@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 0fac1a0..7db1097 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -150,6 +150,7 @@ static inline void lcdc_write(unsigned int val, unsigned int addr)
}
struct da8xx_fb_par {
+ struct device *dev;
resource_size_t p_palette_base;
unsigned char *v_palette_base;
dma_addr_t vram_phys;
@@ -1291,6 +1292,7 @@ static int fb_probe(struct platform_device *device)
}
par = da8xx_fb_info->par;
+ par->dev = &device->dev;
par->lcdc_clk = fb_clk;
par->lcd_fck_rate = clk_get_rate(fb_clk);
if (fb_pdata->panel_power_ctrl) {
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 09/20] video: da8xx-fb: report correct pixclock
From: Darren Etheridge @ 2013-08-05 22:02 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
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