* Re: [PATCH] build some drivers only when compile-testing
From: Tomi Valkeinen @ 2013-05-23 7:46 UTC (permalink / raw)
To: Jiri Slaby
Cc: jirislaby, linux-kernel, Andrew Morton, Linus Torvalds,
Jeff Mahoney, Alexander Shishkin, Greg Kroah-Hartman, linux-usb,
Florian Tobias Schandinat, linux-geode, linux-fbdev,
Richard Cochran, netdev, Ben Hutchings, Keller, Jacob E
In-Reply-To: <1369214326-6558-1-git-send-email-jslaby@suse.cz>
[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]
Hi,
On 22/05/13 12:18, Jiri Slaby wrote:
> Some drivers can be built on more platforms than they run on. This
> causes users and distributors packaging burden when they have to
> manually deselect some drivers from their allmodconfigs. Or sometimes
> it is even impossible to disable the drivers without patching the
> kernel.
>
> Introduce a new config option COMPILE_TEST and make all those drivers
> to depend on the platform they run on, or on the COMPILE_TEST option.
> Now, when users/distributors choose COMPILE_TEST=n they will not have
> the drivers in their allmodconfig setups, but developers still can
> compile-test them with COMPILE_TEST=y.
>
> Now the drivers where we use this new option:
> * PTP_1588_CLOCK_PCH: The PCH EG20T is only compatible with Intel Atom
> processors so it should depend on x86.
> * FB_GEODE: Geode is 32-bit only so only enable it for X86_32.
> * USB_CHIPIDEA_IMX: The OF_DEVICE dependency will be met on powerpc
> systems -- which do not actually support the hardware via that
> method.
I had this exact same idea some time ago. The mail below contains some
of my reasoning for this:
http://comments.gmane.org/gmane.linux.kbuild.devel/9829
I proposed a new Kconfig keyword, but Sam was quite against it as the
Kconfig language already does what is required.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH] build some drivers only when compile-testing
From: Jeff Mahoney @ 2013-05-23 3:09 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, jirislaby, linux-kernel, Andrew Morton,
Linus Torvalds, Alexander Shishkin, linux-usb,
Florian Tobias Schandinat, linux-geode, linux-fbdev,
Richard Cochran, netdev, Ben Hutchings, Keller, Jacob E
In-Reply-To: <20130523022327.GB6159@kroah.com>
On 5/22/13 10:23 PM, Greg Kroah-Hartman wrote:
> On Wed, May 22, 2013 at 11:18:46AM +0200, Jiri Slaby wrote:
>> Some drivers can be built on more platforms than they run on. This
>> causes users and distributors packaging burden when they have to
>> manually deselect some drivers from their allmodconfigs. Or sometimes
>> it is even impossible to disable the drivers without patching the
>> kernel.
>>
>> Introduce a new config option COMPILE_TEST and make all those drivers
>> to depend on the platform they run on, or on the COMPILE_TEST option.
>> Now, when users/distributors choose COMPILE_TEST=n they will not have
>> the drivers in their allmodconfig setups, but developers still can
>> compile-test them with COMPILE_TEST=y.
>
> I understand the urge, and it's getting hard for distros to handle these
> drivers that just don't work on other architectures, but it's really
> valuable to ensure that they build properly, for those of us that don't
> have many/any cross compilers set up.
>
>> Now the drivers where we use this new option:
>> * PTP_1588_CLOCK_PCH: The PCH EG20T is only compatible with Intel Atom
>> processors so it should depend on x86.
>> * FB_GEODE: Geode is 32-bit only so only enable it for X86_32.
>> * USB_CHIPIDEA_IMX: The OF_DEVICE dependency will be met on powerpc
>> systems -- which do not actually support the hardware via that
>> method.
>
> This seems ripe to start to get really messy, really quickly. Shouldn't
> "default configs" handle if this "should" be enabled for a platform or
> not, and let the rest of us just build them with no problems?
If every time a new Kconfig option is added, corresponding default
config updates come with it, sure. I just don't see that happening,
especially when it can be done much more clearly in the Kconfig while
the developer is writing the driver.
> What problems is this causing you? Are you running out of space in
> kernel packages with drivers that will never be actually used?
Wasted build resources. Wasted disk space on /every/ system the kernel
package is installed on. We're all trying to pare down the kernel
packages to eliminate wasted space and doing it manually means a bunch
of research, sometimes with incorrect assumptions about the results,
needs to be done by someone not usually associated with that code. That
research gets repeated by people maintaining kernel packages for pretty
much every distro.
>> +config COMPILE_TEST
>> + bool "Compile also drivers which will not load" if EXPERT
>
> EXPERT is getting to be the "let's hide it here" option, isn't it...
>
> I don't know, if no one else strongly objects, I can be convinced that
> this is needed, but so far, I don't see why it really is, or what this
> is going to help with.
I'm not convinced adding a || COMPILE_TEST option to every driver that
may be arch specific is the best way to go either. Perhaps adding a new
Kconfig verb called "archdepends on" or something that will evaluate as
true if COMPILE_TEST is enabled but will evaluate the conditional if
not. *waves hands*
-Jeff
--
Jeff Mahoney
SUSE Labs
^ permalink raw reply
* Re: [PATCH] build some drivers only when compile-testing
From: Greg Kroah-Hartman @ 2013-05-23 2:23 UTC (permalink / raw)
To: Jiri Slaby
Cc: jirislaby, linux-kernel, Andrew Morton, Linus Torvalds,
Jeff Mahoney, Alexander Shishkin, linux-usb,
Florian Tobias Schandinat, linux-geode, linux-fbdev,
Richard Cochran, netdev, Ben Hutchings, Keller, Jacob E
In-Reply-To: <1369214326-6558-1-git-send-email-jslaby@suse.cz>
On Wed, May 22, 2013 at 11:18:46AM +0200, Jiri Slaby wrote:
> Some drivers can be built on more platforms than they run on. This
> causes users and distributors packaging burden when they have to
> manually deselect some drivers from their allmodconfigs. Or sometimes
> it is even impossible to disable the drivers without patching the
> kernel.
>
> Introduce a new config option COMPILE_TEST and make all those drivers
> to depend on the platform they run on, or on the COMPILE_TEST option.
> Now, when users/distributors choose COMPILE_TEST=n they will not have
> the drivers in their allmodconfig setups, but developers still can
> compile-test them with COMPILE_TEST=y.
I understand the urge, and it's getting hard for distros to handle these
drivers that just don't work on other architectures, but it's really
valuable to ensure that they build properly, for those of us that don't
have many/any cross compilers set up.
> Now the drivers where we use this new option:
> * PTP_1588_CLOCK_PCH: The PCH EG20T is only compatible with Intel Atom
> processors so it should depend on x86.
> * FB_GEODE: Geode is 32-bit only so only enable it for X86_32.
> * USB_CHIPIDEA_IMX: The OF_DEVICE dependency will be met on powerpc
> systems -- which do not actually support the hardware via that
> method.
This seems ripe to start to get really messy, really quickly. Shouldn't
"default configs" handle if this "should" be enabled for a platform or
not, and let the rest of us just build them with no problems?
What problems is this causing you? Are you running out of space in
kernel packages with drivers that will never be actually used?
> +config COMPILE_TEST
> + bool "Compile also drivers which will not load" if EXPERT
EXPERT is getting to be the "let's hide it here" option, isn't it...
I don't know, if no one else strongly objects, I can be convinced that
this is needed, but so far, I don't see why it really is, or what this
is going to help with.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] video/ps3fb: Fix section mismatch warning
From: Geert Uytterhoeven @ 2013-05-21 15:11 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1369141302.3652.14.camel@clam>
On Tue, May 21, 2013 at 3:01 PM, Geoff Levand <geoff@infradead.org> wrote:
> Remove the __initdata attribute from the ps3fb_fix variable. This is in
> follow up to the removal of the __devinit attribute on the ps3fb_probe()
> routine in commit 48c68c4f1b542444f175a9e136febcecf3e704d8 (Drivers:
> video: remove __dev* attributes).
>
> Fixes build warnings like these:
>
> WARNING: vmlinux.o Section mismatch in reference from the function .ps3fb_probe() to the variable .init.data:ps3fb_fix
> The function .ps3fb_probe() references the variable __initdata ps3fb_fix.
> This is often because .ps3fb_probe lacks a __initdata annotation or the annotation of ps3fb_fix is wrong.
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH] video/ps3fb: Fix section mismatch warning
From: Geoff Levand @ 2013-05-21 13:01 UTC (permalink / raw)
To: linux-fbdev
Remove the __initdata attribute from the ps3fb_fix variable. This is in
follow up to the removal of the __devinit attribute on the ps3fb_probe()
routine in commit 48c68c4f1b542444f175a9e136febcecf3e704d8 (Drivers:
video: remove __dev* attributes).
Fixes build warnings like these:
WARNING: vmlinux.o Section mismatch in reference from the function .ps3fb_probe() to the variable .init.data:ps3fb_fix
The function .ps3fb_probe() references the variable __initdata ps3fb_fix.
This is often because .ps3fb_probe lacks a __initdata annotation or the annotation of ps3fb_fix is wrong.
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
drivers/video/ps3fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/ps3fb.c b/drivers/video/ps3fb.c
index a397271d..4819cdf 100644
--- a/drivers/video/ps3fb.c
+++ b/drivers/video/ps3fb.c
@@ -952,7 +952,7 @@ static struct fb_ops ps3fb_ops = {
.fb_compat_ioctl = ps3fb_ioctl
};
-static struct fb_fix_screeninfo ps3fb_fix __initdata = {
+static struct fb_fix_screeninfo ps3fb_fix = {
.id = DEVICE_NAME,
.type = FB_TYPE_PACKED_PIXELS,
.visual = FB_VISUAL_TRUECOLOR,
--
1.8.1.2
^ permalink raw reply related
* Find girls for chat and hookup now.
From: root @ 2013-05-21 13:01 UTC (permalink / raw)
To: linux-ext4-owner-u79uwXL29TY76Z2rM5mHXA
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-ha-dev-cunTk1MwBs9DHfOqEeIffh2eb7JE58TQ,
linux-help-mtg-iCYJNvnsbHs8qNwGfFDJ/Q,
linux-india-cunTk1MwBs9qd80yoKVOQ4aCUm+kVXi4,
linux-informix-qTB+/3op4OQ,
linux-kbuild-owner-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-pFTr4yDReMQpLY+iQtz4oH4Z0sVMXfX4hC4ANOJQIlc,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-man-u79uwXL29TY76Z2rM5mHXA,
linux-mips-VZNHf3L845pBDgjK7y7TUQ
Chat with lonely women in your area and hook up.
http://dir.orangefronthost.info
xrfhlofrtsj
^ permalink raw reply
* [PATCH V2] drivers: video: mxsfb: clean use of devm_ioremap_resource()
From: Laurent Navet @ 2013-05-21 12:33 UTC (permalink / raw)
To: FlorianSchandinat, jg1.han; +Cc: linux-fbdev, linux-kernel, Laurent Navet
In-Reply-To: <19650694.136921368693687337.JavaMail.weblogic@epml12>
Check of 'res' and calls to dev_err are already done in devm_ioremap_resource,
so no need to do them twice.
Signed-off-by: Laurent Navet <laurent.navet@gmail.com>
---
drivers/video/mxsfb.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 21223d4..6a1b338 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -883,12 +883,6 @@ static int mxsfb_probe(struct platform_device *pdev)
if (of_id)
pdev->id_entry = of_id->data;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(&pdev->dev, "Cannot get memory IO resource\n");
- return -ENODEV;
- }
-
fb_info = framebuffer_alloc(sizeof(struct mxsfb_info), &pdev->dev);
if (!fb_info) {
dev_err(&pdev->dev, "Failed to allocate fbdev\n");
@@ -897,9 +891,9 @@ static int mxsfb_probe(struct platform_device *pdev)
host = to_imxfb_host(fb_info);
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
host->base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(host->base)) {
- dev_err(&pdev->dev, "ioremap failed\n");
ret = PTR_ERR(host->base);
goto fb_release;
}
--
1.7.10.4
^ permalink raw reply related
* RE: Introduce a new helper framework for buffer synchronization
From: Inki Dae @ 2013-05-21 9:22 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <51909DB4.2060208@canonical.com>
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Tuesday, May 21, 2013 4:45 PM
> To: Inki Dae
> Cc: 'Daniel Vetter'; 'Rob Clark'; 'linux-fbdev'; 'DRI mailing list';
> 'Kyungmin Park'; 'myungjoo.ham'; 'YoungJun Cho'; linux-arm-
> kernel@lists.infradead.org; linux-media@vger.kernel.org
> Subject: Re: Introduce a new helper framework for buffer synchronization
>
> On Tue, May 21, 2013 at 04:03:06PM +0900, Inki Dae wrote:
> > > - Integration of fence syncing into dma_buf. Imo we should have a
> > > per-attachment mode which decides whether map/unmap (and the new
> sync)
> > > should wait for fences or whether the driver takes care of syncing
> > > through the new fence framework itself (for direct hw sync).
> >
> > I think it's a good idea to have per-attachment mode for buffer sync.
> But
> > I'd like to say we make sure what is the purpose of map
> > (dma_buf_map_attachment)first. This interface is used to get a sgt;
> > containing pages to physical memory region, and map that region with
> > device's iommu table. The important thing is that this interface is
> called
> > just one time when user wants to share an allocated buffer with dma. But
> cpu
> > will try to access the buffer every time as long as it wants. Therefore,
> we
> > need cache control every time cpu and dma access the shared buffer:
> cache
> > clean when cpu goes to dma and cache invalidate when dma goes to cpu.
> That
> > is why I added new interfaces, DMA_BUF_GET_FENCE and DMA_BUF_PUT_FENCE,
> to
> > dma buf framework. Of course, Those are ugly so we need a better way: I
> just
> > wanted to show you that in such way, we can synchronize the shared
> buffer
> > between cpu and dma. By any chance, is there any way that kernel can be
> > aware of when cpu accesses the shared buffer or is there any point I
> didn't
> > catch up?
>
> Well dma_buf_map/unmap_attachment should also flush/invalidate any caches,
> and with the current dma_buf spec those two functions are the _only_ means
I know that dma buf exporter should make sure that cache clean/invalidate
are done when dma_buf_map/unmap_attachement is called. For this, already we
do so. However, this function is called when dma buf import is requested by
user to map a dmabuf fd with dma. This means that dma_buf_map_attachement()
is called ONCE when user wants to share the dmabuf fd with dma. Actually, in
case of exynos drm, dma_map_sg_attrs(), performing cache operation
internally, is called when dmabuf import is requested by user.
> you have to do so. Which strictly means that if you interleave device dma
> and cpu acccess you need to unmap/map every time.
>
> Which is obviously horribly inefficient, but a known gap in the dma_buf
Right, and also this has big overhead.
> interface. Hence why I proposed to add dma_buf_sync_attachment similar to
> dma_sync_single_for_device:
>
> /**
> * dma_buf_sync_sg_attachment - sync caches for dma access
> * @attach: dma-buf attachment to sync
> * @sgt: the sg table to sync (returned by dma_buf_map_attachement)
> * @direction: dma direction to sync for
> *
> * This function synchronizes caches for device dma through the given
> * dma-buf attachment when interleaving dma from different devices and the
> * cpu. Other device dma needs to be synced also by calls to this
> * function (or a pair of dma_buf_map/unmap_attachment calls), cpu access
> * needs to be synced with dma_buf_begin/end_cpu_access.
> */
> void dma_buf_sync_sg_attachment(struct dma_buf_attachment *attach,
> struct sg_table *sgt,
> enum dma_data_direction direction)
>
> Note that "sync" here only means to synchronize caches, not wait for any
> outstanding fences. This is simply to be consistent with the established
> lingo of the dma api. How the dma-buf fences fit into this is imo a
> different topic, but my idea is that all the cache coherency barriers
> (i.e. dma_buf_map/unmap_attachment, dma_buf_sync_sg_attachment and
> dma_buf_begin/end_cpu_access) would automatically block for any attached
> fences (i.e. block for write fences when doing read-only access, block for
> all fences otherwise).
As I mentioned already, kernel can't aware of when cpu accesses a shared
buffer: user can access a shared buffer after mmap anytime and the shared
buffer should be synchronized between cpu and dma. Therefore, the above
cache coherency barriers should be called every time cpu and dma tries to
access a shared buffer, checking before and after of cpu and dma access. And
exactly, the proposed way do such thing. For this, you can refer to the
below link,
http://www.mail-archive.com/linux-media@vger.kernel.org/msg62124.html
My point is that how kernel can aware of when those cache coherency barriers
should be called to synchronize caches and buffer access between cpu and
dma.
>
> Then we could do a new dma_buf_attach_flags interface for special cases
> (might also be useful for other things, similar to the recently added
> flags in the dma api for wc/no-kernel-mapping/...). A new flag like
> DMA_BUF_ATTACH_NO_AUTOFENCE would then mean that the driver takes care of
> all
> fencing for this attachment and the dma-buf functions should not do the
> automagic fence blocking.
>
> > In Linux kernel, especially embedded system, we had tried to implement
> > generic interfaces for buffer management; how to allocate a buffer and
> how
> > to share a buffer. As a result, now we have CMA (Contiguous Memory
> > Allocator) for buffer allocation, and we have DMA-BUF for buffer sharing
> > between cpu and dma. And then how to synchronize a buffer between cpu
> and
> > dma? I think now it's best time to discuss buffer synchronization
> mechanism,
> > and that is very natural.
>
> I think it's important to differentiate between the two meanings of sync:
> - synchronize caches (i.e. cpu cache flushing in most cases)
> - and synchronize in-flight dma with fences.
>
Agree, and I meant that the buffer synchronization mechanism includes the
above two things. And I think the two things should be addressed together.
Thanks,
Inki Dae
> Cheers, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: Introduce a new helper framework for buffer synchronization
From: Daniel Vetter @ 2013-05-21 7:44 UTC (permalink / raw)
To: Inki Dae
Cc: 'Daniel Vetter', 'Rob Clark',
'linux-fbdev', 'DRI mailing list',
'Kyungmin Park', 'myungjoo.ham',
'YoungJun Cho', linux-arm-kernel, linux-media
In-Reply-To: <032701ce55f1$3e9ba4b0$bbd2ee10$%dae@samsung.com>
On Tue, May 21, 2013 at 04:03:06PM +0900, Inki Dae wrote:
> > - Integration of fence syncing into dma_buf. Imo we should have a
> > per-attachment mode which decides whether map/unmap (and the new sync)
> > should wait for fences or whether the driver takes care of syncing
> > through the new fence framework itself (for direct hw sync).
>
> I think it's a good idea to have per-attachment mode for buffer sync. But
> I'd like to say we make sure what is the purpose of map
> (dma_buf_map_attachment)first. This interface is used to get a sgt;
> containing pages to physical memory region, and map that region with
> device's iommu table. The important thing is that this interface is called
> just one time when user wants to share an allocated buffer with dma. But cpu
> will try to access the buffer every time as long as it wants. Therefore, we
> need cache control every time cpu and dma access the shared buffer: cache
> clean when cpu goes to dma and cache invalidate when dma goes to cpu. That
> is why I added new interfaces, DMA_BUF_GET_FENCE and DMA_BUF_PUT_FENCE, to
> dma buf framework. Of course, Those are ugly so we need a better way: I just
> wanted to show you that in such way, we can synchronize the shared buffer
> between cpu and dma. By any chance, is there any way that kernel can be
> aware of when cpu accesses the shared buffer or is there any point I didn't
> catch up?
Well dma_buf_map/unmap_attachment should also flush/invalidate any caches,
and with the current dma_buf spec those two functions are the _only_ means
you have to do so. Which strictly means that if you interleave device dma
and cpu acccess you need to unmap/map every time.
Which is obviously horribly inefficient, but a known gap in the dma_buf
interface. Hence why I proposed to add dma_buf_sync_attachment similar to
dma_sync_single_for_device:
/**
* dma_buf_sync_sg_attachment - sync caches for dma access
* @attach: dma-buf attachment to sync
* @sgt: the sg table to sync (returned by dma_buf_map_attachement)
* @direction: dma direction to sync for
*
* This function synchronizes caches for device dma through the given
* dma-buf attachment when interleaving dma from different devices and the
* cpu. Other device dma needs to be synced also by calls to this
* function (or a pair of dma_buf_map/unmap_attachment calls), cpu access
* needs to be synced with dma_buf_begin/end_cpu_access.
*/
void dma_buf_sync_sg_attachment(struct dma_buf_attachment *attach,
struct sg_table *sgt,
enum dma_data_direction direction)
Note that "sync" here only means to synchronize caches, not wait for any
outstanding fences. This is simply to be consistent with the established
lingo of the dma api. How the dma-buf fences fit into this is imo a
different topic, but my idea is that all the cache coherency barriers
(i.e. dma_buf_map/unmap_attachment, dma_buf_sync_sg_attachment and
dma_buf_begin/end_cpu_access) would automatically block for any attached
fences (i.e. block for write fences when doing read-only access, block for
all fences otherwise).
Then we could do a new dma_buf_attach_flags interface for special cases
(might also be useful for other things, similar to the recently added
flags in the dma api for wc/no-kernel-mapping/...). A new flag like
DMA_BUF_ATTACH_NO_AUTOFENCE would then mean that the driver takes care of all
fencing for this attachment and the dma-buf functions should not do the
automagic fence blocking.
> In Linux kernel, especially embedded system, we had tried to implement
> generic interfaces for buffer management; how to allocate a buffer and how
> to share a buffer. As a result, now we have CMA (Contiguous Memory
> Allocator) for buffer allocation, and we have DMA-BUF for buffer sharing
> between cpu and dma. And then how to synchronize a buffer between cpu and
> dma? I think now it's best time to discuss buffer synchronization mechanism,
> and that is very natural.
I think it's important to differentiate between the two meanings of sync:
- synchronize caches (i.e. cpu cache flushing in most cases)
- and synchronize in-flight dma with fences.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* RE: Introduce a new helper framework for buffer synchronization
From: Inki Dae @ 2013-05-21 7:03 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <51909DB4.2060208@canonical.com>
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Tuesday, May 21, 2013 6:31 AM
> To: Inki Dae
> Cc: Rob Clark; linux-fbdev; DRI mailing list; Kyungmin Park; myungjoo.ham;
> YoungJun Cho; linux-arm-kernel@lists.infradead.org; linux-
> media@vger.kernel.org
> Subject: Re: Introduce a new helper framework for buffer synchronization
>
> On Mon, May 20, 2013 at 11:13:04PM +0200, Daniel Vetter wrote:
> > On Sat, May 18, 2013 at 03:47:43PM +0900, Inki Dae wrote:
> > > 2013/5/15 Rob Clark <robdclark@gmail.com>
> > >
> > > > On Wed, May 15, 2013 at 1:19 AM, Inki Dae <inki.dae@samsung.com>
> wrote:
> > > > >
> > > > >
> > > > >> -----Original Message-----
> > > > >> From: Rob Clark [mailto:robdclark@gmail.com]
> > > > >> Sent: Tuesday, May 14, 2013 10:39 PM
> > > > >> To: Inki Dae
> > > > >> Cc: linux-fbdev; DRI mailing list; Kyungmin Park; myungjoo.ham;
> YoungJun
> > > > >> Cho; linux-arm-kernel@lists.infradead.org; linux-
> media@vger.kernel.org
> > > > >> Subject: Re: Introduce a new helper framework for buffer
> synchronization
> > > > >>
> > > > >> On Mon, May 13, 2013 at 10:52 PM, Inki Dae <inki.dae@samsung.com>
> > > > wrote:
> > > > >> >> well, for cache management, I think it is a better idea.. I
> didn't
> > > > >> >> really catch that this was the motivation from the initial
> patch, but
> > > > >> >> maybe I read it too quickly. But cache can be decoupled from
> > > > >> >> synchronization, because CPU access is not asynchronous. For
> > > > >> >> userspace/CPU access to buffer, you should:
> > > > >> >>
> > > > >> >> 1) wait for buffer
> > > > >> >> 2) prepare-access
> > > > >> >> 3) ... do whatever cpu access to buffer ...
> > > > >> >> 4) finish-access
> > > > >> >> 5) submit buffer for new dma-operation
> > > > >> >>
> > > > >> >
> > > > >> >
> > > > >> > For data flow from CPU to DMA device,
> > > > >> > 1) wait for buffer
> > > > >> > 2) prepare-access (dma_buf_begin_cpu_access)
> > > > >> > 3) cpu access to buffer
> > > > >> >
> > > > >> >
> > > > >> > For data flow from DMA device to CPU
> > > > >> > 1) wait for buffer
> > > > >>
> > > > >> Right, but CPU access isn't asynchronous (from the point of view
> of
> > > > >> the CPU), so there isn't really any wait step at this point. And
> if
> > > > >> you do want the CPU to be able to signal a fence from userspace
> for
> > > > >> some reason, you probably what something file/fd based so the
> > > > >> refcnting/cleanup when process dies doesn't leave some pending
> DMA
> > > > >> action wedged. But I don't really see the point of that
> complexity
> > > > >> when the CPU access isn't asynchronous in the first place.
> > > > >>
> > > > >
> > > > > There was my missing comments, please see the below sequence.
> > > > >
> > > > > For data flow from CPU to DMA device and then from DMA device to
> CPU,
> > > > > 1) wait for buffer <- at user side - ioctl(fd,
> DMA_BUF_GET_FENCE, ...)
> > > > > - including prepare-access (dma_buf_begin_cpu_access)
> > > > > 2) cpu access to buffer
> > > > > 3) wait for buffer <- at device driver
> > > > > - but CPU is already accessing the buffer so blocked.
> > > > > 4) signal <- at user side - ioctl(fd, DMA_BUF_PUT_FENCE, ...)
> > > > > 5) the thread, blocked at 3), is waked up by 4).
> > > > > - and then finish-access (dma_buf_end_cpu_access)
> > > >
> > > > right, I understand you can have background threads, etc, in
> > > > userspace. But there are already plenty of synchronization
> primitives
> > > > that can be used for cpu->cpu synchronization, either within the
> same
> > > > process or between multiple processes. For cpu access, even if it
> is
> > > > handled by background threads/processes, I think it is better to use
> > > > the traditional pthreads or unix synchronization primitives. They
> > > > have existed forever, they are well tested, and they work.
> > > >
> > > > So while it seems nice and orthogonal/clean to couple cache and
> > > > synchronization and handle dma->cpu and cpu->cpu and cpu->dma in the
> > > > same generic way, but I think in practice we have to make things
> more
> > > > complex than they otherwise need to be to do this. Otherwise I
> think
> > > > we'll be having problems with badly behaved or crashing userspace.
> > > >
> > > >
> > > Right, this is very important. I think it's not esay to solve this
> > > issue. Aand at least for ARM based embedded system, such feature is
> useful
> > > to us; coupling cache operation and buffer synchronization. I'd like
> to
> > > collect other opinions and advices to solve this issue.
> >
> > Maybe we have a bit a misunderstanding here. The kernel really should
> take
> > care of sync and cache coherency, and I agree that with the current
> > dma_buf code (and the proposed fences) that part is still a bit hazy.
> But
> > the kernel should not allow userspace to block access to a buffer until
> > userspace is done. It should only sync with any oustanding fences and
> > flush buffers before that userspace access happens.
> >
> > Then the kernel would again flush caches on the next dma access (which
> > hopefully is only started once userspace completed access). Atm this
> isn't
> > possible in an efficient way since the dma_buf api only exposes
> map/unmap
> > attachment and not a function to just sync an existing mapping like
> > dma_sync_single_for_device. I guess we should add a
> > dma_buf_sync_attachment interface for that - we already have range-based
> > cpu sync support with the begin/end_cpu_access interfaces.
>
> I think my mail here was a bit unclear, so let me try to rephrase.
> Re-reading through part of this discussion I think you raise some good
> shortcomings of the current dma_buf interfaces and the proposed fence
> patches. But I think we should address the individual pieces bit-by-bit.
> On a quick survey I see a few parts to what you're trying to solve:
>
> - More efficient cache coherency management. I think we already have all
> required interfaces for efficient cpu-side access with
> begin/end_cpu_access. So I think we only need a device-side dma sync
> mechanism to be able to flush cpu caches after userspace/cpu access has
> completed (before the next dma op).
>
> - More efficient mmap handling. The current dma_buf mmap support is
> explicitly designed as something simply, but probably dead-slow for
> last-resort fallback ops. I'm not sure whether we should fix this up and
> extend with special ioctls. But the current coherency interface should
> be good enough for you to write an efficient private mmap implementation
> for exynos drm.
I agree with you.
>
> - Integration of fence syncing into dma_buf. Imo we should have a
> per-attachment mode which decides whether map/unmap (and the new sync)
> should wait for fences or whether the driver takes care of syncing
> through the new fence framework itself (for direct hw sync).
I think it's a good idea to have per-attachment mode for buffer sync. But
I'd like to say we make sure what is the purpose of map
(dma_buf_map_attachment)first. This interface is used to get a sgt;
containing pages to physical memory region, and map that region with
device's iommu table. The important thing is that this interface is called
just one time when user wants to share an allocated buffer with dma. But cpu
will try to access the buffer every time as long as it wants. Therefore, we
need cache control every time cpu and dma access the shared buffer: cache
clean when cpu goes to dma and cache invalidate when dma goes to cpu. That
is why I added new interfaces, DMA_BUF_GET_FENCE and DMA_BUF_PUT_FENCE, to
dma buf framework. Of course, Those are ugly so we need a better way: I just
wanted to show you that in such way, we can synchronize the shared buffer
between cpu and dma. By any chance, is there any way that kernel can be
aware of when cpu accesses the shared buffer or is there any point I didn't
catch up?
> Imo cpu access should also have such a flag. I guess in both cases we
should
> sync by default.
>
> - cpu/cpu sync additions to dma_buf. Like I've said, I'm not sold at all
I think we should concentrate on cpu and dma sync.
> on this idea. I think it would be best if we try to fix up all the other
> current shortcomings first though and then take a fresh look at this
> issue here.
Right, agree.
>
> Have I missed or misunderstood something?
>
Your comments are very useful to me. Thanks again.
In Linux kernel, especially embedded system, we had tried to implement
generic interfaces for buffer management; how to allocate a buffer and how
to share a buffer. As a result, now we have CMA (Contiguous Memory
Allocator) for buffer allocation, and we have DMA-BUF for buffer sharing
between cpu and dma. And then how to synchronize a buffer between cpu and
dma? I think now it's best time to discuss buffer synchronization mechanism,
and that is very natural.
Please give me more comments and advices if there is my missing or
misunderstanding point.
Thanks,
Inki Dae
> Cheers, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: Introduce a new helper framework for buffer synchronization
From: Daniel Vetter @ 2013-05-20 21:30 UTC (permalink / raw)
To: Inki Dae
Cc: Rob Clark, linux-fbdev, DRI mailing list, Kyungmin Park,
myungjoo.ham, YoungJun Cho, linux-arm-kernel@lists.infradead.org,
linux-media@vger.kernel.org
In-Reply-To: <20130520211304.GV12292@phenom.ffwll.local>
On Mon, May 20, 2013 at 11:13:04PM +0200, Daniel Vetter wrote:
> On Sat, May 18, 2013 at 03:47:43PM +0900, Inki Dae wrote:
> > 2013/5/15 Rob Clark <robdclark@gmail.com>
> >
> > > On Wed, May 15, 2013 at 1:19 AM, Inki Dae <inki.dae@samsung.com> wrote:
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: Rob Clark [mailto:robdclark@gmail.com]
> > > >> Sent: Tuesday, May 14, 2013 10:39 PM
> > > >> To: Inki Dae
> > > >> Cc: linux-fbdev; DRI mailing list; Kyungmin Park; myungjoo.ham; YoungJun
> > > >> Cho; linux-arm-kernel@lists.infradead.org; linux-media@vger.kernel.org
> > > >> Subject: Re: Introduce a new helper framework for buffer synchronization
> > > >>
> > > >> On Mon, May 13, 2013 at 10:52 PM, Inki Dae <inki.dae@samsung.com>
> > > wrote:
> > > >> >> well, for cache management, I think it is a better idea.. I didn't
> > > >> >> really catch that this was the motivation from the initial patch, but
> > > >> >> maybe I read it too quickly. But cache can be decoupled from
> > > >> >> synchronization, because CPU access is not asynchronous. For
> > > >> >> userspace/CPU access to buffer, you should:
> > > >> >>
> > > >> >> 1) wait for buffer
> > > >> >> 2) prepare-access
> > > >> >> 3) ... do whatever cpu access to buffer ...
> > > >> >> 4) finish-access
> > > >> >> 5) submit buffer for new dma-operation
> > > >> >>
> > > >> >
> > > >> >
> > > >> > For data flow from CPU to DMA device,
> > > >> > 1) wait for buffer
> > > >> > 2) prepare-access (dma_buf_begin_cpu_access)
> > > >> > 3) cpu access to buffer
> > > >> >
> > > >> >
> > > >> > For data flow from DMA device to CPU
> > > >> > 1) wait for buffer
> > > >>
> > > >> Right, but CPU access isn't asynchronous (from the point of view of
> > > >> the CPU), so there isn't really any wait step at this point. And if
> > > >> you do want the CPU to be able to signal a fence from userspace for
> > > >> some reason, you probably what something file/fd based so the
> > > >> refcnting/cleanup when process dies doesn't leave some pending DMA
> > > >> action wedged. But I don't really see the point of that complexity
> > > >> when the CPU access isn't asynchronous in the first place.
> > > >>
> > > >
> > > > There was my missing comments, please see the below sequence.
> > > >
> > > > For data flow from CPU to DMA device and then from DMA device to CPU,
> > > > 1) wait for buffer <- at user side - ioctl(fd, DMA_BUF_GET_FENCE, ...)
> > > > - including prepare-access (dma_buf_begin_cpu_access)
> > > > 2) cpu access to buffer
> > > > 3) wait for buffer <- at device driver
> > > > - but CPU is already accessing the buffer so blocked.
> > > > 4) signal <- at user side - ioctl(fd, DMA_BUF_PUT_FENCE, ...)
> > > > 5) the thread, blocked at 3), is waked up by 4).
> > > > - and then finish-access (dma_buf_end_cpu_access)
> > >
> > > right, I understand you can have background threads, etc, in
> > > userspace. But there are already plenty of synchronization primitives
> > > that can be used for cpu->cpu synchronization, either within the same
> > > process or between multiple processes. For cpu access, even if it is
> > > handled by background threads/processes, I think it is better to use
> > > the traditional pthreads or unix synchronization primitives. They
> > > have existed forever, they are well tested, and they work.
> > >
> > > So while it seems nice and orthogonal/clean to couple cache and
> > > synchronization and handle dma->cpu and cpu->cpu and cpu->dma in the
> > > same generic way, but I think in practice we have to make things more
> > > complex than they otherwise need to be to do this. Otherwise I think
> > > we'll be having problems with badly behaved or crashing userspace.
> > >
> > >
> > Right, this is very important. I think it's not esay to solve this
> > issue. Aand at least for ARM based embedded system, such feature is useful
> > to us; coupling cache operation and buffer synchronization. I'd like to
> > collect other opinions and advices to solve this issue.
>
> Maybe we have a bit a misunderstanding here. The kernel really should take
> care of sync and cache coherency, and I agree that with the current
> dma_buf code (and the proposed fences) that part is still a bit hazy. But
> the kernel should not allow userspace to block access to a buffer until
> userspace is done. It should only sync with any oustanding fences and
> flush buffers before that userspace access happens.
>
> Then the kernel would again flush caches on the next dma access (which
> hopefully is only started once userspace completed access). Atm this isn't
> possible in an efficient way since the dma_buf api only exposes map/unmap
> attachment and not a function to just sync an existing mapping like
> dma_sync_single_for_device. I guess we should add a
> dma_buf_sync_attachment interface for that - we already have range-based
> cpu sync support with the begin/end_cpu_access interfaces.
I think my mail here was a bit unclear, so let me try to rephrase.
Re-reading through part of this discussion I think you raise some good
shortcomings of the current dma_buf interfaces and the proposed fence
patches. But I think we should address the individual pieces bit-by-bit.
On a quick survey I see a few parts to what you're trying to solve:
- More efficient cache coherency management. I think we already have all
required interfaces for efficient cpu-side access with
begin/end_cpu_access. So I think we only need a device-side dma sync
mechanism to be able to flush cpu caches after userspace/cpu access has
completed (before the next dma op).
- More efficient mmap handling. The current dma_buf mmap support is
explicitly designed as something simply, but probably dead-slow for
last-resort fallback ops. I'm not sure whether we should fix this up and
extend with special ioctls. But the current coherency interface should
be good enough for you to write an efficient private mmap implementation
for exynos drm.
- Integration of fence syncing into dma_buf. Imo we should have a
per-attachment mode which decides whether map/unmap (and the new sync)
should wait for fences or whether the driver takes care of syncing
through the new fence framework itself (for direct hw sync). Imo cpu
access should also have such a flag. I guess in both cases we should
sync by default.
- cpu/cpu sync additions to dma_buf. Like I've said, I'm not sold at all
on this idea. I think it would be best if we try to fix up all the other
current shortcomings first though and then take a fresh look at this
issue here.
Have I missed or misunderstood something?
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: Introduce a new helper framework for buffer synchronization
From: Daniel Vetter @ 2013-05-20 21:13 UTC (permalink / raw)
To: Inki Dae
Cc: Rob Clark, linux-fbdev, DRI mailing list, Kyungmin Park,
myungjoo.ham, YoungJun Cho, linux-arm-kernel@lists.infradead.org,
linux-media@vger.kernel.org
In-Reply-To: <CAAQKjZP37koEPob6yqpn-WxxTh3+O=twyvRzDiEhVJTD8BxQzw@mail.gmail.com>
On Sat, May 18, 2013 at 03:47:43PM +0900, Inki Dae wrote:
> 2013/5/15 Rob Clark <robdclark@gmail.com>
>
> > On Wed, May 15, 2013 at 1:19 AM, Inki Dae <inki.dae@samsung.com> wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Rob Clark [mailto:robdclark@gmail.com]
> > >> Sent: Tuesday, May 14, 2013 10:39 PM
> > >> To: Inki Dae
> > >> Cc: linux-fbdev; DRI mailing list; Kyungmin Park; myungjoo.ham; YoungJun
> > >> Cho; linux-arm-kernel@lists.infradead.org; linux-media@vger.kernel.org
> > >> Subject: Re: Introduce a new helper framework for buffer synchronization
> > >>
> > >> On Mon, May 13, 2013 at 10:52 PM, Inki Dae <inki.dae@samsung.com>
> > wrote:
> > >> >> well, for cache management, I think it is a better idea.. I didn't
> > >> >> really catch that this was the motivation from the initial patch, but
> > >> >> maybe I read it too quickly. But cache can be decoupled from
> > >> >> synchronization, because CPU access is not asynchronous. For
> > >> >> userspace/CPU access to buffer, you should:
> > >> >>
> > >> >> 1) wait for buffer
> > >> >> 2) prepare-access
> > >> >> 3) ... do whatever cpu access to buffer ...
> > >> >> 4) finish-access
> > >> >> 5) submit buffer for new dma-operation
> > >> >>
> > >> >
> > >> >
> > >> > For data flow from CPU to DMA device,
> > >> > 1) wait for buffer
> > >> > 2) prepare-access (dma_buf_begin_cpu_access)
> > >> > 3) cpu access to buffer
> > >> >
> > >> >
> > >> > For data flow from DMA device to CPU
> > >> > 1) wait for buffer
> > >>
> > >> Right, but CPU access isn't asynchronous (from the point of view of
> > >> the CPU), so there isn't really any wait step at this point. And if
> > >> you do want the CPU to be able to signal a fence from userspace for
> > >> some reason, you probably what something file/fd based so the
> > >> refcnting/cleanup when process dies doesn't leave some pending DMA
> > >> action wedged. But I don't really see the point of that complexity
> > >> when the CPU access isn't asynchronous in the first place.
> > >>
> > >
> > > There was my missing comments, please see the below sequence.
> > >
> > > For data flow from CPU to DMA device and then from DMA device to CPU,
> > > 1) wait for buffer <- at user side - ioctl(fd, DMA_BUF_GET_FENCE, ...)
> > > - including prepare-access (dma_buf_begin_cpu_access)
> > > 2) cpu access to buffer
> > > 3) wait for buffer <- at device driver
> > > - but CPU is already accessing the buffer so blocked.
> > > 4) signal <- at user side - ioctl(fd, DMA_BUF_PUT_FENCE, ...)
> > > 5) the thread, blocked at 3), is waked up by 4).
> > > - and then finish-access (dma_buf_end_cpu_access)
> >
> > right, I understand you can have background threads, etc, in
> > userspace. But there are already plenty of synchronization primitives
> > that can be used for cpu->cpu synchronization, either within the same
> > process or between multiple processes. For cpu access, even if it is
> > handled by background threads/processes, I think it is better to use
> > the traditional pthreads or unix synchronization primitives. They
> > have existed forever, they are well tested, and they work.
> >
> > So while it seems nice and orthogonal/clean to couple cache and
> > synchronization and handle dma->cpu and cpu->cpu and cpu->dma in the
> > same generic way, but I think in practice we have to make things more
> > complex than they otherwise need to be to do this. Otherwise I think
> > we'll be having problems with badly behaved or crashing userspace.
> >
> >
> Right, this is very important. I think it's not esay to solve this
> issue. Aand at least for ARM based embedded system, such feature is useful
> to us; coupling cache operation and buffer synchronization. I'd like to
> collect other opinions and advices to solve this issue.
Maybe we have a bit a misunderstanding here. The kernel really should take
care of sync and cache coherency, and I agree that with the current
dma_buf code (and the proposed fences) that part is still a bit hazy. But
the kernel should not allow userspace to block access to a buffer until
userspace is done. It should only sync with any oustanding fences and
flush buffers before that userspace access happens.
Then the kernel would again flush caches on the next dma access (which
hopefully is only started once userspace completed access). Atm this isn't
possible in an efficient way since the dma_buf api only exposes map/unmap
attachment and not a function to just sync an existing mapping like
dma_sync_single_for_device. I guess we should add a
dma_buf_sync_attachment interface for that - we already have range-based
cpu sync support with the begin/end_cpu_access interfaces.
Yours, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* [PATCH v2] video/matrox/matroxfb_maven: Use module_i2c_driver to register driver
From: Peter Huewe @ 2013-05-20 19:56 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: Jean Delvare, Mark Brown, Peter Huewe, linux-fbdev, linux-kernel
In-Reply-To: <20130520214206.078bc580@endymion.delvare>
Removing some boilerplate by using module_i2c_driver instead of calling
register and unregister in the otherwise empty init/exit functions.
Also removed a useless comment as suggested by Jean Delvare
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
Reviewed-by: Jean Delvare <khali@linux-fr.org>
---
drivers/video/matrox/matroxfb_maven.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/video/matrox/matroxfb_maven.c b/drivers/video/matrox/matroxfb_maven.c
index 217678e..6360796 100644
--- a/drivers/video/matrox/matroxfb_maven.c
+++ b/drivers/video/matrox/matroxfb_maven.c
@@ -1283,19 +1283,7 @@ static struct i2c_driver maven_driver={
.id_table = maven_id,
};
-static int __init matroxfb_maven_init(void)
-{
- return i2c_add_driver(&maven_driver);
-}
-
-static void __exit matroxfb_maven_exit(void)
-{
- i2c_del_driver(&maven_driver);
-}
-
+module_i2c_driver(maven_driver);
MODULE_AUTHOR("(c) 1999-2002 Petr Vandrovec <vandrove@vc.cvut.cz>");
MODULE_DESCRIPTION("Matrox G200/G400 Matrox MGA-TVO driver");
MODULE_LICENSE("GPL");
-module_init(matroxfb_maven_init);
-module_exit(matroxfb_maven_exit);
-/* we do not have __setup() yet */
--
1.8.1.5
^ permalink raw reply related
* Re: [PATCH] video/matrox/matroxfb_maven: Use module_i2c_driver to register driver
From: Jean Delvare @ 2013-05-20 19:42 UTC (permalink / raw)
To: Peter Huewe
Cc: Florian Tobias Schandinat, Wolfram Sang, Mark Brown, linux-fbdev,
linux-kernel
In-Reply-To: <1369078578-3138-1-git-send-email-peterhuewe@gmx.de>
On Mon, 20 May 2013 21:36:18 +0200, Peter Huewe wrote:
> Removing some boilerplate by using module_i2c_driver instead of calling
> register and unregister in the otherwise empty init/exit functions
>
> Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
> ---
> drivers/video/matrox/matroxfb_maven.c | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/video/matrox/matroxfb_maven.c b/drivers/video/matrox/matroxfb_maven.c
> index 217678e..fd4b64e 100644
> --- a/drivers/video/matrox/matroxfb_maven.c
> +++ b/drivers/video/matrox/matroxfb_maven.c
> @@ -1283,19 +1283,8 @@ static struct i2c_driver maven_driver={
> .id_table = maven_id,
> };
>
> -static int __init matroxfb_maven_init(void)
> -{
> - return i2c_add_driver(&maven_driver);
> -}
> -
> -static void __exit matroxfb_maven_exit(void)
> -{
> - i2c_del_driver(&maven_driver);
> -}
> -
> +module_i2c_driver(maven_driver);
> MODULE_AUTHOR("(c) 1999-2002 Petr Vandrovec <vandrove@vc.cvut.cz>");
> MODULE_DESCRIPTION("Matrox G200/G400 Matrox MGA-TVO driver");
> MODULE_LICENSE("GPL");
> -module_init(matroxfb_maven_init);
> -module_exit(matroxfb_maven_exit);
> /* we do not have __setup() yet */
This last comment could certainly go away as well, AFAICT it's a now
meaningless relic.
Other than this:
Reviewed-by: Jean Delvare <khali@linux-fr.org>
--
Jean Delvare
^ permalink raw reply
* [PATCH] video/matrox/matroxfb_maven: Use module_i2c_driver to register driver
From: Peter Huewe @ 2013-05-20 19:36 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: Wolfram Sang, Mark Brown, Peter Huewe, Jean Delvare, linux-fbdev,
linux-kernel
Removing some boilerplate by using module_i2c_driver instead of calling
register and unregister in the otherwise empty init/exit functions
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
drivers/video/matrox/matroxfb_maven.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/video/matrox/matroxfb_maven.c b/drivers/video/matrox/matroxfb_maven.c
index 217678e..fd4b64e 100644
--- a/drivers/video/matrox/matroxfb_maven.c
+++ b/drivers/video/matrox/matroxfb_maven.c
@@ -1283,19 +1283,8 @@ static struct i2c_driver maven_driver={
.id_table = maven_id,
};
-static int __init matroxfb_maven_init(void)
-{
- return i2c_add_driver(&maven_driver);
-}
-
-static void __exit matroxfb_maven_exit(void)
-{
- i2c_del_driver(&maven_driver);
-}
-
+module_i2c_driver(maven_driver);
MODULE_AUTHOR("(c) 1999-2002 Petr Vandrovec <vandrove@vc.cvut.cz>");
MODULE_DESCRIPTION("Matrox G200/G400 Matrox MGA-TVO driver");
MODULE_LICENSE("GPL");
-module_init(matroxfb_maven_init);
-module_exit(matroxfb_maven_exit);
/* we do not have __setup() yet */
--
1.8.1.5
^ permalink raw reply related
* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Stephen Warren @ 2013-05-20 15:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAVeFuJ5q4Y0QtvJX2iVmq_O2ofs1B9=MB_aA8VdcEBdM4zf2Q@mail.gmail.com>
On 05/18/2013 04:29 AM, Alexandre Courbot wrote:
> On Thu, Apr 4, 2013 at 11:39 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> +struct simplefb_format {
>> + const char *name;
>> + u32 bits_per_pixel;
>> + struct fb_bitfield red;
>> + struct fb_bitfield green;
>> + struct fb_bitfield blue;
>> + struct fb_bitfield transp;
>> +};
>> +
>> +struct simplefb_format simplefb_formats[] = {
>> + { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
>> +};
>
> I have been adding a few extra formats to this list, and I wonder if
> this could not simply be turned into a function that would directly
> convert the name string into the corresponding right format. The
> mapping between name and format seems to be a 1:1 and this would
> probably avoid errors in the future. I'm especially thinking about
> color order here - I started adding a mode that reads
>
> { "r8g8b8a8", 32, {0, 8}, {8, 8}, {16, 8}, {24, 8} },
>
> while it should probably be called "a8b8g8r8" as the order of colors
> is not the same as your r5g6b5.
>
> I can submit a patch if there is no issue with that idea.
I chose r5g6b5 rather than rgb565 specifically to make that format
trivially name machine-parsable. So, I'm not opposed to converting that
table to code. I'm not 100% sure if it's worth it or necessary by the
time we get to just 2 formats in the array, but I don't see any big
disadvantage, so why not. The DT binding documentation might want
enhancing with a more general description of how formats should be
represented if this is implemented.
^ permalink raw reply
* Re: [PATCH 0/4] FB updates for 3.11
From: Tony Prisk @ 2013-05-19 8:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1368868514-18975-1-git-send-email-linux@prisktech.co.nz>
On 18/05/13 21:15, Tony Prisk wrote:
> Patch #1 - Move register defines inside the driver and drop the header.
> Patch #2 - Convert the register defines to use the vendor preferred names.
> Patch #3 - Add a device clock to wm8505fb. Without it, only the uboot
> initialized resolution is supported.
> Patch #4 - Add support for the VGA output found on the APC8750 board.
>
> Tony Prisk (4):
> fb: vt8500: Move register defines inside driver
> fb: vt8500: Convert to use vendor register names
> fb: vt8500: Require a device clock for wm8505fb driver
> fb: vt8500: Add VGA output support to wm8505fb driver.
>
> .../devicetree/bindings/video/wm,wm8505-fb.txt | 9 +-
> drivers/video/wm8505fb.c | 195 ++++++++++++--
> drivers/video/wm8505fb_regs.h | 76 ------
> drivers/video/wmt_ge_rops.c | 280 +++++++++++++++-----
> 4 files changed, 394 insertions(+), 166 deletions(-)
> delete mode 100644 drivers/video/wm8505fb_regs.h
>
Florian/Tomi,
Please ignore these patches. This driver needs a bit more work, so I
will do a
more thorough series and resubmit.
Regards
Tony Prisk
^ permalink raw reply
* Re: [PATCH 4/4] fb: vt8500: Add VGA output support to wm8505fb driver.
From: Tony Prisk @ 2013-05-18 19:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CABjd4Yxrk3hXnB5f6u+1HcJjymDZbtQ1F1HXkZ2dzF3dwCrWhg@mail.gmail.com>
On 19/05/13 01:28, Alexey Charkov wrote:
> 2013/5/18 Tony Prisk <linux@prisktech.co.nz>:
>> The APC8750 does not support an LCD panel, but provides a VGA connector.
>> This patch adds support for the VGA interface, and defines an optional
>> devicetree property to specify the output interface. The default if not
>> specified is LCD for backward compatibility.
>>
>> Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
>> ---
>> .../devicetree/bindings/video/wm,wm8505-fb.txt | 5 ++++
>> drivers/video/wm8505fb.c | 31 ++++++++++++++++++--
>> 2 files changed, 34 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt b/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt
>> index 601416c..9f1d648 100644
>> --- a/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt
>> +++ b/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt
>> @@ -7,6 +7,10 @@ Required properties:
>> - bits-per-pixel : bit depth of framebuffer (16 or 32)
>> - clocks : phandle to DVO clock
>>
>> +Optional properties:
>> +- output-interface : the interface the fb should output on. Valid values are
>> + "lcd" or "vga". If not specified, the default is "lcd".
>> +
>> Required subnodes:
>> - display-timings: see display-timing.txt for information
>>
>> @@ -17,6 +21,7 @@ Example:
>> reg = <0xd8051700 0x200>;
>> bits-per-pixel = <16>;
>> clocks = <&clkdvo>;
>> + output-interface = "vga";
>>
>> display-timings {
>> native-mode = <&timing0>;
>> diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c
>> index f8bffc2..d1f7f33 100644
>> --- a/drivers/video/wm8505fb.c
>> +++ b/drivers/video/wm8505fb.c
>> @@ -130,12 +130,17 @@
>>
>> #define to_wm8505fb_info(__info) container_of(__info, \
>> struct wm8505fb_info, fb)
>> +
>> +#define INTERFACE_LCD 1
>> +#define INTERFACE_VGA 2
>> +
>> struct wm8505fb_info {
>> struct fb_info fb;
>> void __iomem *regbase;
>> unsigned int contrast;
>> struct device *dev;
>> struct clk *clk_dvo;
>> + int interface;
>> };
>>
>>
>> @@ -158,7 +163,11 @@ static int wm8505fb_init_hw(struct fb_info *info)
>> * 0x31C sets the correct color mode (RGB565) for WM8650
>> * Bit 8+9 (0x300) are ignored on WM8505 as reserved
>> */
>> - writel(0x31c, fbi->regbase + REG_GOVRH_YUVRGB);
>> + if (fbi->interface = INTERFACE_VGA)
>> + writel(0x338, fbi->regbase + REG_GOVRH_YUVRGB);
>> + else
>> + writel(0x31c, fbi->regbase + REG_GOVRH_YUVRGB);
>> +
>> writel(1, fbi->regbase + REG_GOVRH_DVO_PIX);
> Tony,
>
> Would it be possible to also define known bit offsets for those
> registers, while you are at this? It would probably reduce the black
> magic quite a bit :)
On my list of things to do :)
>> /* Virtual buffer size */
>> @@ -167,7 +176,12 @@ static int wm8505fb_init_hw(struct fb_info *info)
>>
>> /* black magic ;) */
>> writel(0xf, fbi->regbase + REG_GOVRH_FHI);
>> - writel(4, fbi->regbase + REG_GOVRH_DVO_SET);
>> +
>> + if (fbi->interface = INTERFACE_VGA)
>> + writel(0xe, fbi->regbase + REG_GOVRH_DVO_SET);
>> + else
>> + writel(4, fbi->regbase + REG_GOVRH_DVO_SET);
> I don't remember if HDMI is yet another option for this register or
> not... If it is, it would probably warrant defining fbi->interface as
> an enum and changing this if-else into a switch statement to let the
> compiler add its checks/warnings.
This register defines the h/v syncpolarity and enable/disable for DVO.
>
>> writel(1, fbi->regbase + REG_GOVRH_MIF);
>> writel(1, fbi->regbase + REG_GOVRH_REG_STS);
>>
>> @@ -194,11 +208,15 @@ static int wm8505fb_set_timing(struct fb_info *info)
>> writel(h_end, fbi->regbase + REG_GOVRH_ACTPX_END);
>> writel(h_all, fbi->regbase + REG_GOVRH_H_ALLPXL);
>> writel(h_sync, fbi->regbase + REG_GOVRH_HDMI_HSYNW);
>> + if (fbi->interface = INTERFACE_VGA)
>> + writel(h_sync, fbi->regbase + REG_GOVRH_VGA_HSYNW);
> Will it misbehave on LCD if you write to the VGA register unconditionally?
Don't know - wouldn't imagine so. I will test it and see.
>
>> writel(v_start, fbi->regbase + REG_GOVRH_ACTLN_BG);
>> writel(v_end, fbi->regbase + REG_GOVRH_ACTLN_END);
>> writel(v_all, fbi->regbase + REG_GOVRH_V_ALLLN);
>> writel(v_sync, fbi->regbase + REG_GOVRH_HDMI_VBISW);
>> + if (fbi->interface = INTERFACE_VGA)
>> + writel(info->var.pixclock, fbi->regbase + REG_GOVRH_VGA_VSYNW);
> Same here. I would assume that setting the pixclock should not hurt
> LCD, which would then simplify the code a little.
>
> Thanks,
> Alexey
Regards
Tony Prisk
^ permalink raw reply
* Re: [PATCH 4/4] fb: vt8500: Add VGA output support to wm8505fb driver.
From: Andy Chernyak @ 2013-05-18 13:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CABjd4Yxrk3hXnB5f6u+1HcJjymDZbtQ1F1HXkZ2dzF3dwCrWhg@mail.gmail.com>
On 05/18/2013 03:28 PM, Alexey Charkov wrote:
> 2013/5/18 Tony Prisk <linux@prisktech.co.nz>:
>
>> /* Virtual buffer size */
>> @@ -167,7 +176,12 @@ static int wm8505fb_init_hw(struct fb_info *info)
>>
>> /* black magic ;) */
>> writel(0xf, fbi->regbase + REG_GOVRH_FHI);
>> - writel(4, fbi->regbase + REG_GOVRH_DVO_SET);
>> +
>> + if (fbi->interface = INTERFACE_VGA)
>> + writel(0xe, fbi->regbase + REG_GOVRH_DVO_SET);
>> + else
>> + writel(4, fbi->regbase + REG_GOVRH_DVO_SET);
> I don't remember if HDMI is yet another option for this register or
> not... If it is, it would probably warrant defining fbi->interface as
> an enum and changing this if-else into a switch statement to let the
> compiler add its checks/warnings.
HDMI output can work simultaneously with LCD (on 8850 at least), which
fbi->interface in its current form would not allow to express.
> + if (fbi->interface = INTERFACE_VGA)
> + writel(h_sync, fbi->regbase + REG_GOVRH_VGA_HSYNW);
> Will it misbehave on LCD if you write to the VGA register unconditionally?
>
Can we have 3 flags, something like LCD_ON, VGA_ON, HDMI_ON instead of
enum for video interface selection?
Regards,
Andy.
^ permalink raw reply
* Re: [PATCH 4/4] fb: vt8500: Add VGA output support to wm8505fb driver.
From: Alexey Charkov @ 2013-05-18 13:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1368868514-18975-5-git-send-email-linux@prisktech.co.nz>
2013/5/18 Tony Prisk <linux@prisktech.co.nz>:
> The APC8750 does not support an LCD panel, but provides a VGA connector.
> This patch adds support for the VGA interface, and defines an optional
> devicetree property to specify the output interface. The default if not
> specified is LCD for backward compatibility.
>
> Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
> ---
> .../devicetree/bindings/video/wm,wm8505-fb.txt | 5 ++++
> drivers/video/wm8505fb.c | 31 ++++++++++++++++++--
> 2 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt b/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt
> index 601416c..9f1d648 100644
> --- a/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt
> +++ b/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt
> @@ -7,6 +7,10 @@ Required properties:
> - bits-per-pixel : bit depth of framebuffer (16 or 32)
> - clocks : phandle to DVO clock
>
> +Optional properties:
> +- output-interface : the interface the fb should output on. Valid values are
> + "lcd" or "vga". If not specified, the default is "lcd".
> +
> Required subnodes:
> - display-timings: see display-timing.txt for information
>
> @@ -17,6 +21,7 @@ Example:
> reg = <0xd8051700 0x200>;
> bits-per-pixel = <16>;
> clocks = <&clkdvo>;
> + output-interface = "vga";
>
> display-timings {
> native-mode = <&timing0>;
> diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c
> index f8bffc2..d1f7f33 100644
> --- a/drivers/video/wm8505fb.c
> +++ b/drivers/video/wm8505fb.c
> @@ -130,12 +130,17 @@
>
> #define to_wm8505fb_info(__info) container_of(__info, \
> struct wm8505fb_info, fb)
> +
> +#define INTERFACE_LCD 1
> +#define INTERFACE_VGA 2
> +
> struct wm8505fb_info {
> struct fb_info fb;
> void __iomem *regbase;
> unsigned int contrast;
> struct device *dev;
> struct clk *clk_dvo;
> + int interface;
> };
>
>
> @@ -158,7 +163,11 @@ static int wm8505fb_init_hw(struct fb_info *info)
> * 0x31C sets the correct color mode (RGB565) for WM8650
> * Bit 8+9 (0x300) are ignored on WM8505 as reserved
> */
> - writel(0x31c, fbi->regbase + REG_GOVRH_YUVRGB);
> + if (fbi->interface = INTERFACE_VGA)
> + writel(0x338, fbi->regbase + REG_GOVRH_YUVRGB);
> + else
> + writel(0x31c, fbi->regbase + REG_GOVRH_YUVRGB);
> +
> writel(1, fbi->regbase + REG_GOVRH_DVO_PIX);
Tony,
Would it be possible to also define known bit offsets for those
registers, while you are at this? It would probably reduce the black
magic quite a bit :)
> /* Virtual buffer size */
> @@ -167,7 +176,12 @@ static int wm8505fb_init_hw(struct fb_info *info)
>
> /* black magic ;) */
> writel(0xf, fbi->regbase + REG_GOVRH_FHI);
> - writel(4, fbi->regbase + REG_GOVRH_DVO_SET);
> +
> + if (fbi->interface = INTERFACE_VGA)
> + writel(0xe, fbi->regbase + REG_GOVRH_DVO_SET);
> + else
> + writel(4, fbi->regbase + REG_GOVRH_DVO_SET);
I don't remember if HDMI is yet another option for this register or
not... If it is, it would probably warrant defining fbi->interface as
an enum and changing this if-else into a switch statement to let the
compiler add its checks/warnings.
> writel(1, fbi->regbase + REG_GOVRH_MIF);
> writel(1, fbi->regbase + REG_GOVRH_REG_STS);
>
> @@ -194,11 +208,15 @@ static int wm8505fb_set_timing(struct fb_info *info)
> writel(h_end, fbi->regbase + REG_GOVRH_ACTPX_END);
> writel(h_all, fbi->regbase + REG_GOVRH_H_ALLPXL);
> writel(h_sync, fbi->regbase + REG_GOVRH_HDMI_HSYNW);
> + if (fbi->interface = INTERFACE_VGA)
> + writel(h_sync, fbi->regbase + REG_GOVRH_VGA_HSYNW);
Will it misbehave on LCD if you write to the VGA register unconditionally?
> writel(v_start, fbi->regbase + REG_GOVRH_ACTLN_BG);
> writel(v_end, fbi->regbase + REG_GOVRH_ACTLN_END);
> writel(v_all, fbi->regbase + REG_GOVRH_V_ALLLN);
> writel(v_sync, fbi->regbase + REG_GOVRH_HDMI_VBISW);
> + if (fbi->interface = INTERFACE_VGA)
> + writel(info->var.pixclock, fbi->regbase + REG_GOVRH_VGA_VSYNW);
Same here. I would assume that setting the pixclock should not hurt
LCD, which would then simplify the code a little.
Thanks,
Alexey
^ permalink raw reply
* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Alexandre Courbot @ 2013-05-18 10:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1365043183-28905-1-git-send-email-swarren@wwwdotorg.org>
On Thu, Apr 4, 2013 at 11:39 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> +struct simplefb_format {
> + const char *name;
> + u32 bits_per_pixel;
> + struct fb_bitfield red;
> + struct fb_bitfield green;
> + struct fb_bitfield blue;
> + struct fb_bitfield transp;
> +};
> +
> +struct simplefb_format simplefb_formats[] = {
> + { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
> +};
I have been adding a few extra formats to this list, and I wonder if
this could not simply be turned into a function that would directly
convert the name string into the corresponding right format. The
mapping between name and format seems to be a 1:1 and this would
probably avoid errors in the future. I'm especially thinking about
color order here - I started adding a mode that reads
{ "r8g8b8a8", 32, {0, 8}, {8, 8}, {16, 8}, {24, 8} },
while it should probably be called "a8b8g8r8" as the order of colors
is not the same as your r5g6b5.
I can submit a patch if there is no issue with that idea.
Alex.
^ permalink raw reply
* [PATCH 4/4] fb: vt8500: Add VGA output support to wm8505fb driver.
From: Tony Prisk @ 2013-05-18 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1368868514-18975-1-git-send-email-linux@prisktech.co.nz>
The APC8750 does not support an LCD panel, but provides a VGA connector.
This patch adds support for the VGA interface, and defines an optional
devicetree property to specify the output interface. The default if not
specified is LCD for backward compatibility.
Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
.../devicetree/bindings/video/wm,wm8505-fb.txt | 5 ++++
drivers/video/wm8505fb.c | 31 ++++++++++++++++++--
2 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt b/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt
index 601416c..9f1d648 100644
--- a/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt
+++ b/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt
@@ -7,6 +7,10 @@ Required properties:
- bits-per-pixel : bit depth of framebuffer (16 or 32)
- clocks : phandle to DVO clock
+Optional properties:
+- output-interface : the interface the fb should output on. Valid values are
+ "lcd" or "vga". If not specified, the default is "lcd".
+
Required subnodes:
- display-timings: see display-timing.txt for information
@@ -17,6 +21,7 @@ Example:
reg = <0xd8051700 0x200>;
bits-per-pixel = <16>;
clocks = <&clkdvo>;
+ output-interface = "vga";
display-timings {
native-mode = <&timing0>;
diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c
index f8bffc2..d1f7f33 100644
--- a/drivers/video/wm8505fb.c
+++ b/drivers/video/wm8505fb.c
@@ -130,12 +130,17 @@
#define to_wm8505fb_info(__info) container_of(__info, \
struct wm8505fb_info, fb)
+
+#define INTERFACE_LCD 1
+#define INTERFACE_VGA 2
+
struct wm8505fb_info {
struct fb_info fb;
void __iomem *regbase;
unsigned int contrast;
struct device *dev;
struct clk *clk_dvo;
+ int interface;
};
@@ -158,7 +163,11 @@ static int wm8505fb_init_hw(struct fb_info *info)
* 0x31C sets the correct color mode (RGB565) for WM8650
* Bit 8+9 (0x300) are ignored on WM8505 as reserved
*/
- writel(0x31c, fbi->regbase + REG_GOVRH_YUVRGB);
+ if (fbi->interface = INTERFACE_VGA)
+ writel(0x338, fbi->regbase + REG_GOVRH_YUVRGB);
+ else
+ writel(0x31c, fbi->regbase + REG_GOVRH_YUVRGB);
+
writel(1, fbi->regbase + REG_GOVRH_DVO_PIX);
/* Virtual buffer size */
@@ -167,7 +176,12 @@ static int wm8505fb_init_hw(struct fb_info *info)
/* black magic ;) */
writel(0xf, fbi->regbase + REG_GOVRH_FHI);
- writel(4, fbi->regbase + REG_GOVRH_DVO_SET);
+
+ if (fbi->interface = INTERFACE_VGA)
+ writel(0xe, fbi->regbase + REG_GOVRH_DVO_SET);
+ else
+ writel(4, fbi->regbase + REG_GOVRH_DVO_SET);
+
writel(1, fbi->regbase + REG_GOVRH_MIF);
writel(1, fbi->regbase + REG_GOVRH_REG_STS);
@@ -194,11 +208,15 @@ static int wm8505fb_set_timing(struct fb_info *info)
writel(h_end, fbi->regbase + REG_GOVRH_ACTPX_END);
writel(h_all, fbi->regbase + REG_GOVRH_H_ALLPXL);
writel(h_sync, fbi->regbase + REG_GOVRH_HDMI_HSYNW);
+ if (fbi->interface = INTERFACE_VGA)
+ writel(h_sync, fbi->regbase + REG_GOVRH_VGA_HSYNW);
writel(v_start, fbi->regbase + REG_GOVRH_ACTLN_BG);
writel(v_end, fbi->regbase + REG_GOVRH_ACTLN_END);
writel(v_all, fbi->regbase + REG_GOVRH_V_ALLLN);
writel(v_sync, fbi->regbase + REG_GOVRH_HDMI_VBISW);
+ if (fbi->interface = INTERFACE_VGA)
+ writel(info->var.pixclock, fbi->regbase + REG_GOVRH_VGA_VSYNW);
writel(1, fbi->regbase + REG_GOVRH_TG_ENABLE);
@@ -371,6 +389,7 @@ static int wm8505fb_probe(struct platform_device *pdev)
dma_addr_t fb_mem_phys;
unsigned long fb_mem_len;
void *fb_mem_virt;
+ const char *intf;
fbi = devm_kzalloc(&pdev->dev, sizeof(struct wm8505fb_info) +
sizeof(u32) * 16, GFP_KERNEL);
@@ -428,6 +447,14 @@ static int wm8505fb_probe(struct platform_device *pdev)
clk_prepare_enable(fbi->clk_dvo);
+ fbi->interface = INTERFACE_LCD;
+ ret = of_property_read_string(pdev->dev.of_node, "output-interface",
+ &intf);
+ if (!ret) {
+ if (!strcmp(intf, "vga"))
+ fbi->interface = INTERFACE_VGA;
+ }
+
fb_videomode_to_var(&fbi->fb.var, &mode);
fbi->fb.var.nonstd = 0;
--
1.7.9.5
^ permalink raw reply related
* [PATCH 3/4] fb: vt8500: Require a device clock for wm8505fb driver
From: Tony Prisk @ 2013-05-18 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1368868514-18975-1-git-send-email-linux@prisktech.co.nz>
The wm8505fb driver requires a clock to work properly. Without a clock,
the driver can only initialize the display resolution that was set in
uboot.
This patch updates the driver to get and use a clock, and updates
the devicetree documentation to indicate the requirement for a clock.
Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
.../devicetree/bindings/video/wm,wm8505-fb.txt | 4 ++-
drivers/video/wm8505fb.c | 30 +++++++++++++++++---
2 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt b/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt
index 0bcadb2..601416c 100644
--- a/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt
+++ b/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt
@@ -5,6 +5,7 @@ Required properties:
- compatible : "wm,wm8505-fb"
- reg : Should contain 1 register ranges(address and length)
- bits-per-pixel : bit depth of framebuffer (16 or 32)
+- clocks : phandle to DVO clock
Required subnodes:
- display-timings: see display-timing.txt for information
@@ -15,11 +16,12 @@ Example:
compatible = "wm,wm8505-fb";
reg = <0xd8051700 0x200>;
bits-per-pixel = <16>;
+ clocks = <&clkdvo>;
display-timings {
native-mode = <&timing0>;
timing0: 800x480 {
- clock-frequency = <0>; /* unused but required */
+ clock-frequency = <30000000>;
hactive = <800>;
vactive = <480>;
hfront-porch = <40>;
diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c
index 167a9e2..f8bffc2 100644
--- a/drivers/video/wm8505fb.c
+++ b/drivers/video/wm8505fb.c
@@ -14,6 +14,7 @@
* GNU General Public License for more details.
*/
+#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/dma-mapping.h>
#include <linux/fb.h>
@@ -130,9 +131,11 @@
#define to_wm8505fb_info(__info) container_of(__info, \
struct wm8505fb_info, fb)
struct wm8505fb_info {
- struct fb_info fb;
- void __iomem *regbase;
- unsigned int contrast;
+ struct fb_info fb;
+ void __iomem *regbase;
+ unsigned int contrast;
+ struct device *dev;
+ struct clk *clk_dvo;
};
@@ -210,6 +213,13 @@ static int wm8505fb_set_par(struct fb_info *info)
if (!fbi)
return -EINVAL;
+ if (info->var.pixclock = 0) {
+ dev_err(fbi->dev, "requested pixclock = 0\n");
+ return -EINVAL;
+ }
+
+ clk_set_rate(fbi->clk_dvo, PICOS2KHZ(info->var.pixclock)*1000);
+
if (info->var.bits_per_pixel = 32) {
info->var.red.offset = 16;
info->var.red.length = 8;
@@ -369,6 +379,8 @@ static int wm8505fb_probe(struct platform_device *pdev)
return -ENOMEM;
}
+ fbi->dev = &pdev->dev;
+
strcpy(fbi->fb.fix.id, DRIVER_NAME);
fbi->fb.fix.type = FB_TYPE_PACKED_PIXELS;
@@ -408,6 +420,14 @@ static int wm8505fb_probe(struct platform_device *pdev)
if (ret)
return ret;
+ fbi->clk_dvo = of_clk_get(pdev->dev.of_node, 0);
+ if (IS_ERR(fbi->clk_dvo)) {
+ dev_err(&pdev->dev, "Error retrieving clock\n");
+ return PTR_ERR(fbi->clk_dvo);
+ }
+
+ clk_prepare_enable(fbi->clk_dvo);
+
fb_videomode_to_var(&fbi->fb.var, &mode);
fbi->fb.var.nonstd = 0;
@@ -421,7 +441,7 @@ static int wm8505fb_probe(struct platform_device *pdev)
fb_mem_virt = dmam_alloc_coherent(&pdev->dev, fb_mem_len, &fb_mem_phys,
GFP_KERNEL);
if (!fb_mem_virt) {
- pr_err("%s: Failed to allocate framebuffer\n", __func__);
+ dev_err(&pdev->dev, "Failed to allocate framebuffer\n");
return -ENOMEM;
}
@@ -480,6 +500,8 @@ static int wm8505fb_remove(struct platform_device *pdev)
unregister_framebuffer(&fbi->fb);
+ clk_disable_unprepare(fbi->clk_dvo);
+
writel(0, fbi->regbase);
if (fbi->fb.cmap.len)
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/4] fb: vt8500: Convert to use vendor register names
From: Tony Prisk @ 2013-05-18 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1368868514-18975-1-git-send-email-linux@prisktech.co.nz>
Change all the #defines to match the vendor defined names, and change the
references in wm8505fb.c and wmt_ge_rops.c.
Add all the missing register offsets as well to prevent churn in the future.
Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
drivers/video/wm8505fb.c | 159 ++++++++++++++++--------
drivers/video/wmt_ge_rops.c | 280 +++++++++++++++++++++++++++++++++----------
2 files changed, 332 insertions(+), 107 deletions(-)
diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c
index f824af8..167a9e2 100644
--- a/drivers/video/wm8505fb.c
+++ b/drivers/video/wm8505fb.c
@@ -38,29 +38,94 @@
#define DRIVER_NAME "wm8505-fb"
-#define WMT_GOVR_COLORSPACE1 0x030
-#define WMT_GOVR_MIF_ENABLE 0x080
-#define WMT_GOVR_FBADDR 0x090
-#define WMT_GOVR_FBADDR1 0x094
-#define WMT_GOVR_XRES 0x098
-#define WMT_GOVR_XRES_VIRTUAL 0x09c
-#define WMT_GOVR_YPAN 0x0a0
-#define WMT_GOVR_XPAN 0x0a4
-#define WMT_GOVR_FHI 0x0a8
-#define WMT_GOVR_REG_UPDATE 0x0e4
-#define WMT_GOVR_TG 0x100
-#define WMT_GOVR_TIMING_H_ALL 0x108
-#define WMT_GOVR_TIMING_V_ALL 0x10c
-#define WMT_GOVR_TIMING_V_START 0x110
-#define WMT_GOVR_TIMING_V_END 0x114
-#define WMT_GOVR_TIMING_H_START 0x118
-#define WMT_GOVR_TIMING_H_END 0x11c
-#define WMT_GOVR_TIMING_V_SYNC 0x128
-#define WMT_GOVR_TIMING_H_SYNC 0x12c
-#define WMT_GOVR_DVO_SET 0x148
-#define WMT_GOVR_CONTRAST 0x1b8
-#define WMT_GOVR_BRGHTNESS 0x1bc
-#define WMT_GOVR_COLORSPACE 0x1e4
+#define REG_GOVRH_CUR_ADDR 0x0000
+#define REG_GOVRH_CUR_WIDTH 0x0004
+#define REG_GOVRH_CUR_FB_WIDTH 0x0008
+#define REG_GOVRH_CUR_VCROP 0x000C
+#define REG_GOVRH_CUR_HCROP 0x0010
+#define REG_GOVRH_CUR_HCOORD 0x0014
+#define REG_GOVRH_CUR_VCOORD 0x0018
+#define REG_GOVRH_CUR_STATUS 0x001C
+#define REG_GOVRH_CUR_COLOR_KEY 0x0020
+#define REG_GOVRH_DVO_PIX 0x0030
+#define REG_GOVRH_DVO_DLY_SEL 0x0034
+#define REG_GOVRH_INT 0x0038
+#define REG_GOVRH_DVO_BLANK_DATA 0x003C
+#define REG_GOVRH_DIRPATH 0x0040 /* WM8750+ */
+#define REG_GOVRH_MIF 0x0080
+#define REG_GOVRH_COLFMT 0x0084
+#define REG_GOVRH_SRCFMT 0x0088
+#define REG_GOVRH_DSTFMT 0x008C
+#define REG_GOVRH_YSA 0x0090
+#define REG_GOVRH_CSA 0x0094
+#define REG_GOVRH_PIXWID 0x0098
+#define REG_GOVRH_BUFWID 0x009C
+#define REG_GOVRH_VCROP 0x00A0
+#define REG_GOVRH_HCROP 0x00A4
+#define REG_GOVRH_FHI 0x00A8
+#define REG_GOVRH_COLFMT2 0x00AC
+#define REG_GOVRH_YSA2 0x00B0 /* WM8950 */
+#define REG_GOVRH_CSA2 0x00B4 /* WM8950 */
+#define REG_GOVRH_MIF_FRAME_MODE 0x00B8 /* WM8950 */
+#define REG_GOVRH_REG_STS 0x00E4
+#define REG_GOVRH_SWFLD 0x00E8
+#define REG_GOVRH_TG_ENABLE 0x0100
+#define REG_GOVRH_READ_CYC 0x0104
+#define REG_GOVRH_H_ALLPXL 0x0108
+#define REG_GOVRH_V_ALLLN 0x010C
+#define REG_GOVRH_ACTLN_BG 0x0110
+#define REG_GOVRH_ACTLN_END 0x0114
+#define REG_GOVRH_ACTPX_BG 0x0118
+#define REG_GOVRH_ACTPX_END 0x011C
+#define REG_GOVRH_VBIE_LINE 0x0120
+#define REG_GOVRH_PVBI_LINE 0x0124
+#define REG_GOVRH_HDMI_VBISW 0x0128
+#define REG_GOVRH_HDMI_HSYNW 0x012C
+#define REG_GOVRH_VSYNC_OFFSET 0x0130
+#define REG_GOVRH_FIELD_STATUS 0x0134
+#define REG_GOVRH_HDMI_3D 0x013C /* WM8950 */
+#define REG_GOVRH_DVO_SET 0x0148
+#define REG_GOVRH_CB_ENABLE 0x0150
+#define REG_GOVRH_H_ALLPXL2 0x0158
+#define REG_GOVRH_V_ALLLN2 0x015C
+#define REG_GOVRH_ACTLN_BG2 0x0160
+#define REG_GOVRH_ACTLN_END2 0x0164
+#define REG_GOVRH_ACTPX_BG2 0x0168
+#define REG_GOVRH_ACTPX_END2 0x016C
+#define REG_GOVRH_VBIE_LINE2 0x0170
+#define REG_GOVRH_PVBI_LINE2 0x0174
+#define REG_GOVRH_HDMI_VBISW2 0x0178
+#define REG_GOVRH_HDMI_HSYNW2 0x017C
+#define REG_GOVRH_LVDS_CTRL 0x0180 /* WM8750+ */
+#define REG_GOVRH_LVDS_CTRL2 0x0184 /* WM8750+ */
+#define REG_GOVRH_DAC_LP_SENSE_VAL 0x0188 /* WM8750 */
+#define REG_GOVRH_DAC_TEST_MODE 0x018C /* WM8750 */
+#define REG_GOVRH_VGA_HSYNW 0x0190 /* WM8750 */
+#define REG_GOVRH_VGA_VSYNW 0x0194 /* WM8750 */
+#define REG_GOVRH_VGA_SYNPOLAR 0x0198 /* WM8750 */
+#define REG_GOVRH_DAC_MOD 0x019C /* WM8750 */
+#define REG_GOVRH_DAC_VAL 0x01A0 /* WM8750 */
+#define REG_GOVRH_DAC_CON 0x01A4 /* WM8750 */
+#define REG_GOVRH_DAC_TEST 0x01A8 /* WM8750 */
+#define REG_GOVRH_DAC_BTEST 0x01AC /* WM8750 */
+#define REG_GOVRH_DAC_CTEST 0x01B0 /* WM8750 */
+#define REG_GOVRH_DAC_DBG 0x01B4 /* WM8750 */
+#define REG_GOVRH_CONTRAST 0x01B8
+#define REG_GOVRH_BRIGHTNESS 0x01BC
+#define REG_GOVRH_DMACSC_COEF0 0x01C0
+#define REG_GOVRH_DMACSC_COEF1 0x01C4
+#define REG_GOVRH_DMACSC_COEF2 0x01C8
+#define REG_GOVRH_DMACSC_COEF3 0x01CC
+#define REG_GOVRH_DMACSC_COEF4 0x01D0
+#define REG_GOVRH_DMACSC_COEF5 0x01D8
+#define REG_GOVRH_DMACSC_COEF6 0x01DC
+#define REG_GOVRH_CSC_MODE 0x01E0
+#define REG_GOVRH_YUVRGB 0x01E4
+#define REG_GOVRH_H264_INPUT_EN 0x01E8
+#define REG_GOVRH_DISP_EN 0x01EC /* WM8750 */
+#define REG_GOVRH_HSCALE_UP 0x01F4
+#define REG_GOVRH_IGS_MODE 0x01F8
+#define REG_GOVRH_IGS_MODE2 0x01FC
#define to_wm8505fb_info(__info) container_of(__info, \
struct wm8505fb_info, fb)
@@ -82,26 +147,26 @@ static int wm8505fb_init_hw(struct fb_info *info)
writel(0, fbi->regbase + i);
/* Set frame buffer address */
- writel(fbi->fb.fix.smem_start, fbi->regbase + WMT_GOVR_FBADDR);
- writel(fbi->fb.fix.smem_start, fbi->regbase + WMT_GOVR_FBADDR1);
+ writel(fbi->fb.fix.smem_start, fbi->regbase + REG_GOVRH_YSA);
+ writel(fbi->fb.fix.smem_start, fbi->regbase + REG_GOVRH_CSA);
/*
* Set in-memory picture format to RGB
* 0x31C sets the correct color mode (RGB565) for WM8650
* Bit 8+9 (0x300) are ignored on WM8505 as reserved
*/
- writel(0x31c, fbi->regbase + WMT_GOVR_COLORSPACE);
- writel(1, fbi->regbase + WMT_GOVR_COLORSPACE1);
+ writel(0x31c, fbi->regbase + REG_GOVRH_YUVRGB);
+ writel(1, fbi->regbase + REG_GOVRH_DVO_PIX);
/* Virtual buffer size */
- writel(info->var.xres, fbi->regbase + WMT_GOVR_XRES);
- writel(info->var.xres_virtual, fbi->regbase + WMT_GOVR_XRES_VIRTUAL);
+ writel(info->var.xres, fbi->regbase + REG_GOVRH_PIXWID);
+ writel(info->var.xres_virtual, fbi->regbase + REG_GOVRH_BUFWID);
/* black magic ;) */
- writel(0xf, fbi->regbase + WMT_GOVR_FHI);
- writel(4, fbi->regbase + WMT_GOVR_DVO_SET);
- writel(1, fbi->regbase + WMT_GOVR_MIF_ENABLE);
- writel(1, fbi->regbase + WMT_GOVR_REG_UPDATE);
+ writel(0xf, fbi->regbase + REG_GOVRH_FHI);
+ writel(4, fbi->regbase + REG_GOVRH_DVO_SET);
+ writel(1, fbi->regbase + REG_GOVRH_MIF);
+ writel(1, fbi->regbase + REG_GOVRH_REG_STS);
return 0;
}
@@ -120,19 +185,19 @@ static int wm8505fb_set_timing(struct fb_info *info)
int v_all = v_end + info->var.lower_margin;
int v_sync = info->var.vsync_len;
- writel(0, fbi->regbase + WMT_GOVR_TG);
+ writel(0, fbi->regbase + REG_GOVRH_TG_ENABLE);
- writel(h_start, fbi->regbase + WMT_GOVR_TIMING_H_START);
- writel(h_end, fbi->regbase + WMT_GOVR_TIMING_H_END);
- writel(h_all, fbi->regbase + WMT_GOVR_TIMING_H_ALL);
- writel(h_sync, fbi->regbase + WMT_GOVR_TIMING_H_SYNC);
+ writel(h_start, fbi->regbase + REG_GOVRH_ACTPX_BG);
+ writel(h_end, fbi->regbase + REG_GOVRH_ACTPX_END);
+ writel(h_all, fbi->regbase + REG_GOVRH_H_ALLPXL);
+ writel(h_sync, fbi->regbase + REG_GOVRH_HDMI_HSYNW);
- writel(v_start, fbi->regbase + WMT_GOVR_TIMING_V_START);
- writel(v_end, fbi->regbase + WMT_GOVR_TIMING_V_END);
- writel(v_all, fbi->regbase + WMT_GOVR_TIMING_V_ALL);
- writel(v_sync, fbi->regbase + WMT_GOVR_TIMING_V_SYNC);
+ writel(v_start, fbi->regbase + REG_GOVRH_ACTLN_BG);
+ writel(v_end, fbi->regbase + REG_GOVRH_ACTLN_END);
+ writel(v_all, fbi->regbase + REG_GOVRH_V_ALLLN);
+ writel(v_sync, fbi->regbase + REG_GOVRH_HDMI_VBISW);
- writel(1, fbi->regbase + WMT_GOVR_TG);
+ writel(1, fbi->regbase + REG_GOVRH_TG_ENABLE);
return 0;
}
@@ -174,7 +239,7 @@ static int wm8505fb_set_par(struct fb_info *info)
wm8505fb_set_timing(info);
writel(fbi->contrast<<16 | fbi->contrast<<8 | fbi->contrast,
- fbi->regbase + WMT_GOVR_CONTRAST);
+ fbi->regbase + REG_GOVRH_CONTRAST);
return 0;
}
@@ -250,8 +315,8 @@ static int wm8505fb_pan_display(struct fb_var_screeninfo *var,
{
struct wm8505fb_info *fbi = to_wm8505fb_info(info);
- writel(var->xoffset, fbi->regbase + WMT_GOVR_XPAN);
- writel(var->yoffset, fbi->regbase + WMT_GOVR_YPAN);
+ writel(var->xoffset, fbi->regbase + REG_GOVRH_VCROP);
+ writel(var->yoffset, fbi->regbase + REG_GOVRH_HCROP);
return 0;
}
@@ -264,7 +329,7 @@ static int wm8505fb_blank(int blank, struct fb_info *info)
wm8505fb_set_timing(info);
break;
default:
- writel(0, fbi->regbase + WMT_GOVR_TIMING_V_SYNC);
+ writel(0, fbi->regbase + REG_GOVRH_HDMI_VBISW);
break;
}
diff --git a/drivers/video/wmt_ge_rops.c b/drivers/video/wmt_ge_rops.c
index 4aaeb18..68de46a 100644
--- a/drivers/video/wmt_ge_rops.c
+++ b/drivers/video/wmt_ge_rops.c
@@ -20,29 +20,189 @@
#include <linux/platform_device.h>
#include "fb_draw.h"
-#define GE_COMMAND_OFF 0x00
-#define GE_DEPTH_OFF 0x04
-#define GE_HIGHCOLOR_OFF 0x08
-#define GE_ROPCODE_OFF 0x14
-#define GE_FIRE_OFF 0x18
-#define GE_SRCBASE_OFF 0x20
-#define GE_SRCDISPW_OFF 0x24
-#define GE_SRCDISPH_OFF 0x28
-#define GE_SRCAREAX_OFF 0x2c
-#define GE_SRCAREAY_OFF 0x30
-#define GE_SRCAREAW_OFF 0x34
-#define GE_SRCAREAH_OFF 0x38
-#define GE_DESTBASE_OFF 0x3c
-#define GE_DESTDISPW_OFF 0x40
-#define GE_DESTDISPH_OFF 0x44
-#define GE_DESTAREAX_OFF 0x48
-#define GE_DESTAREAY_OFF 0x4c
-#define GE_DESTAREAW_OFF 0x50
-#define GE_DESTAREAH_OFF 0x54
-#define GE_PAT0C_OFF 0x88 /* Pattern 0 color */
-#define GE_ENABLE_OFF 0xec
-#define GE_INTEN_OFF 0xf0
-#define GE_STATUS_OFF 0xf8
+#define GE_COMMAND 0x0000
+#define GE_COLOR_DEPTH 0x0004
+#define GE_HM_SEL 0x0008
+#define GE_PAT_TRAN_EN 0x000C
+#define GE_FONT_TRAN_EN 0x0010
+#define GE_ROP_CODE 0x0014
+#define GE_FIRE 0x0018
+#define GE_ROP_BG_CODE 0x001C
+#define GE_SRC_BADDR 0x0020
+#define GE_SRC_DISP_W 0x0024
+#define GE_SRC_DISP_H 0x0028
+#define GE_SRC_X_START 0x002C
+#define GE_SRC_Y_START 0x0030
+#define GE_SRC_WIDTH 0x0034
+#define GE_SRC_HEIGHT 0x0038
+#define GE_DES_BADDR 0x003C
+#define GE_DES_DISP_W 0x0040
+#define GE_DES_DISP_H 0x0044
+#define GE_DES_X_START 0x0048
+#define GE_DES_Y_START 0x004C
+#define GE_DES_WIDTH 0x0050
+#define GE_DES_HEIGHT 0x0054
+#define GE_FONT0_BUF 0x0058
+#define GE_FONT1_BUF 0x005C
+#define GE_FONT2_BUF 0x0060
+#define GE_FONT3_BUF 0x0064
+#define GE_PAT0_BUF 0x0068
+#define GE_PAT1_BUF 0x006C
+#define GE_PAT2_BUF 0x0070
+#define GE_PAT3_BUF 0x0074
+#define GE_PAT4_BUF 0x0078
+#define GE_PAT5_BUF 0x007C
+#define GE_PAT6_BUF 0x0080
+#define GE_PAT7_BUF 0x0084
+#define GE_PAT0_COLOR 0x0088
+#define GE_PAT1_COLOR 0x008C
+#define GE_PAT2_COLOR 0x0090
+#define GE_PAT3_COLOR 0x0094
+#define GE_PAT4_COLOR 0x0098
+#define GE_PAT5_COLOR 0x009C
+#define GE_PAT6_COLOR 0x00A0
+#define GE_PAT7_COLOR 0x00A4
+#define GE_PAT8_COLOR 0x00A8
+#define GE_PAT9_COLOR 0x00AC
+#define GE_PAT10_COLOR 0x00B0
+#define GE_PAT11_COLOR 0x00B4
+#define GE_PAT12_COLOR 0x00B8
+#define GE_PAT13_COLOR 0x00BC
+#define GE_PAT14_COLOR 0x00C0
+#define GE_PAT15_COLOR 0x00C4
+#define GE_CK_SEL 0x00C8
+#define GE_SRC_CK 0x00CC
+#define GE_DES_CK 0x00D0
+#define GE_ALPHA_SEL 0x00D4
+#define GE_BITBLT_ALPHA 0x00D8
+#define GE_DES_PATH_EN 0x00DC
+#define GE_ROTATE_MODE 0x00E0
+#define GE_MIRROR_MODE 0x00E4
+#define GE_GE_DELAY 0x00E8
+#define GE_ENABLE 0x00EC
+#define GE_INT_EN 0x00F0
+#define GE_INT_FLAG 0x00F4
+#define GE_STATUS 0x00F8
+#define GE_SWID 0x00FC
+#define GE_LN_X_START 0x0100
+#define GE_LN_X_END 0x0104
+#define GE_LN_Y_START 0x0108
+#define GE_LN_Y_END 0x0110
+#define GE_LN_TCK 0x0114
+#define GE_AMX_CSC_BYPASS 0x0118
+#define GE_C1_COEF 0x011C
+#define GE_LN_STL_TB 0x0120
+#define GE_LN_STL_RTN 0x0124
+#define GE_LN_STL_DATA 0x0128
+#define GE_LN_STL_APA 0x012C
+#define GE_BC_P1X 0x0130
+#define GE_BC_P1Y 0x0134
+#define GE_BC_P2X 0x0138
+#define GE_BC_P2Y 0x013C
+#define GE_BC_P3X 0x0140
+#define GE_BC_P3Y 0x0144
+#define GE_BC_COLOR 0x0148
+#define GE_BC_ALPHA 0x014C
+#define GE_BC_DELTA_T 0x0150
+#define GE_BC_L_STL 0x0154
+#define GE_BC_L_STL_RTN 0x0158
+#define GE_C2_COEF 0x015C
+#define GE_C3_COEF 0x0160
+#define GE_C4_COEF 0x0164
+#define GE_C5_COEF 0x0168
+#define GE_C6_COEF 0x016C
+#define GE_C7_COEF 0x0170
+#define GE_C8_COEF 0x0174
+#define GE_YUV2_Y_BADDR 0x0178
+#define GE_YUV2_C_BADDR 0x017C
+#define GE_VQ_EN 0x0180
+#define GE_VQ_SIZE 0x0184
+#define GE_VQ_UDPTR 0x0188
+#define GE_VQ_BASEADDR 0x018C
+#define GE_VQ_WRSIZE 0x0190
+#define GE_VQ_STADDRW 0x0194
+#define GE_VQ_THR 0x0198
+#define GE_VQ_YUV2_Y_FBW 0x019C
+#define GE_ROP4_EN 0x01A0
+#define GE_ALPHA_PLANE_EN 0x01A4
+#define GE_MASK_BADDR 0x01A8
+#define GE_MASK_DISP_W 0x01AC
+#define GE_MASK_DISP_H 0x01B0
+#define GE_MASK_X_START 0x01B4
+#define GE_MASK_Y_START 0x01B8
+#define GE_MASK_WIDTH 0x01BC
+#define GE_MASK_HEIGHT 0x01C0
+#define GE_DW_MASK_BADDR 0x01C4
+#define GE_ALPHA_PLANE_WBE 0x01C8
+#define GE_YUV2_C_FBW 0x01CC
+#define GE_ADAP_BLEND_EN 0x01D0
+#define GE_SRC_ALPHA_SEL 0x01D4
+#define GE_SRC_BLEND_APA 0x01D8
+#define GE_DES_ALPHA_SEL 0x01DC
+#define GE_DES_BLEND_APA 0x01E0
+#define GE_ADAP_CLAMP_EN 0x01E4
+#define GE_YUV2_C_BLEND_SEL 0x01E8
+#define GE_SRC_INDEP_MODE 0x01EC
+#define GE_C9_COEF 0x01F0
+#define GE_COEF_I 0x01F4
+#define GE_COEF_J 0x01F8
+#define GE_COEF_K 0x01FC
+#define GE_G1_CD 0x0200
+#define GE_G2_CD 0x0204
+#define GE_G1_FG_ADDR 0x0210
+#define GE_G1_BG_ADDR 0x0214
+#define GE_G1_FB_SEL 0x0218
+#define GE_G2_FG_ADDR 0x021C
+#define GE_G2_BG_ADDR 0x0220
+#define GE_G2_FB_SEL 0x0224
+#define GE_G1_X_START 0x0230
+#define GE_G1_X_END 0x0234
+#define GE_G1_Y_START 0x0238
+#define GE_G1_Y_END 0x023C
+#define GE_G2_X_START 0x0240
+#define GE_G2_X_END 0x0244
+#define GE_G2_Y_START 0x0248
+#define GE_G2_Y_END 0x024C
+#define GE_DISP_X_END 0x0250
+#define GE_DISP_Y_END 0x0254
+#define GE_AMX_CB 0x0258
+#define GE_G1_YUV_MODE_EN 0x025C
+#define GE_G2_YUV_MODE_EN 0x0260
+#define GE_G1_YUV_FMT_SEL 0x0264
+#define GE_G1_YUV_OUTP_SEL 0x0268
+#define GE_G2_YUV_FMT_SEL 0x026C
+#define GE_G2_YUV_OUTP_SEL 0x0270
+#define GE_AMX_CSC_CFG 0x0274
+#define GE_AMX_CSC_MODE 0x0278
+#define GE_AMX_Y_SUB_16_EN 0x027C
+#define GE_G1_YUV_ADDR 0x0280
+#define GE_G2_YUV_ADDR 0x0284
+#define GE_G1_CK_EN 0x0298
+#define GE_G2_CK_EN 0x029C
+#define GE_G1_C_KEY 0x02A0
+#define GE_G2_C_KEY 0x02A4
+#define GE_G1_AMX_EN 0x02A8
+#define GE_G2_AMX_EN 0x02AC
+#define GE_CK2_APA 0x02B0
+#define GE_AMX_CTL 0x02B4
+#define GE_CK_APA 0x02B8
+#define GE_FIX_APA 0x02BC
+#define GE_G1_AMX_HM 0x02C0
+#define GE_G2_AMX_HM 0x02C4
+#define GE_NH_DATA 0x02C8
+#define GE_VSYNC_STS 0x02CC
+#define GE_REG_UPD 0x02D0
+#define GE_REG_SEL 0x02D4
+#define GE_REG_AMX2_CTL 0x02D8
+#define GE_FIX2_APA 0x02DC
+#define GE_G1_H_SCALE 0x02E0
+#define GE_G2_H_SCALE 0x02E4
+#define GE_G1_FBW 0x02E8
+#define GE_G1_VCROP 0x02EC
+#define GE_G1_HCROP 0x02F0
+#define GE_G2_FBW 0x02F4
+#define GE_G2_VCROP 0x02F8
+#define GE_G2_HCROP 0x02FC
static void __iomem *regbase;
@@ -65,20 +225,20 @@ void wmt_ge_fillrect(struct fb_info *p, const struct fb_fillrect *rect)
p->fbops->fb_sync(p);
writel(p->var.bits_per_pixel = 32 ? 3 :
- (p->var.bits_per_pixel = 8 ? 0 : 1), regbase + GE_DEPTH_OFF);
- writel(p->var.bits_per_pixel = 15 ? 1 : 0, regbase + GE_HIGHCOLOR_OFF);
- writel(p->fix.smem_start, regbase + GE_DESTBASE_OFF);
- writel(p->var.xres_virtual - 1, regbase + GE_DESTDISPW_OFF);
- writel(p->var.yres_virtual - 1, regbase + GE_DESTDISPH_OFF);
- writel(rect->dx, regbase + GE_DESTAREAX_OFF);
- writel(rect->dy, regbase + GE_DESTAREAY_OFF);
- writel(rect->width - 1, regbase + GE_DESTAREAW_OFF);
- writel(rect->height - 1, regbase + GE_DESTAREAH_OFF);
-
- writel(pat, regbase + GE_PAT0C_OFF);
- writel(1, regbase + GE_COMMAND_OFF);
- writel(rect->rop = ROP_XOR ? 0x5a : 0xf0, regbase + GE_ROPCODE_OFF);
- writel(1, regbase + GE_FIRE_OFF);
+ (p->var.bits_per_pixel = 8 ? 0 : 1), regbase + GE_COLOR_DEPTH);
+ writel(p->var.bits_per_pixel = 15 ? 1 : 0, regbase + GE_HM_SEL);
+ writel(p->fix.smem_start, regbase + GE_DES_BADDR);
+ writel(p->var.xres_virtual - 1, regbase + GE_DES_DISP_W);
+ writel(p->var.yres_virtual - 1, regbase + GE_DES_DISP_H);
+ writel(rect->dx, regbase + GE_DES_X_START);
+ writel(rect->dy, regbase + GE_DES_Y_START);
+ writel(rect->width - 1, regbase + GE_DES_WIDTH);
+ writel(rect->height - 1, regbase + GE_DES_HEIGHT);
+
+ writel(pat, regbase + GE_PAT0_COLOR);
+ writel(1, regbase + GE_COMMAND);
+ writel(rect->rop = ROP_XOR ? 0x5a : 0xf0, regbase + GE_ROP_CODE);
+ writel(1, regbase + GE_FIRE);
}
EXPORT_SYMBOL_GPL(wmt_ge_fillrect);
@@ -91,34 +251,34 @@ void wmt_ge_copyarea(struct fb_info *p, const struct fb_copyarea *area)
p->fbops->fb_sync(p);
writel(p->var.bits_per_pixel > 16 ? 3 :
- (p->var.bits_per_pixel > 8 ? 1 : 0), regbase + GE_DEPTH_OFF);
-
- writel(p->fix.smem_start, regbase + GE_SRCBASE_OFF);
- writel(p->var.xres_virtual - 1, regbase + GE_SRCDISPW_OFF);
- writel(p->var.yres_virtual - 1, regbase + GE_SRCDISPH_OFF);
- writel(area->sx, regbase + GE_SRCAREAX_OFF);
- writel(area->sy, regbase + GE_SRCAREAY_OFF);
- writel(area->width - 1, regbase + GE_SRCAREAW_OFF);
- writel(area->height - 1, regbase + GE_SRCAREAH_OFF);
-
- writel(p->fix.smem_start, regbase + GE_DESTBASE_OFF);
- writel(p->var.xres_virtual - 1, regbase + GE_DESTDISPW_OFF);
- writel(p->var.yres_virtual - 1, regbase + GE_DESTDISPH_OFF);
- writel(area->dx, regbase + GE_DESTAREAX_OFF);
- writel(area->dy, regbase + GE_DESTAREAY_OFF);
- writel(area->width - 1, regbase + GE_DESTAREAW_OFF);
- writel(area->height - 1, regbase + GE_DESTAREAH_OFF);
-
- writel(0xcc, regbase + GE_ROPCODE_OFF);
- writel(1, regbase + GE_COMMAND_OFF);
- writel(1, regbase + GE_FIRE_OFF);
+ (p->var.bits_per_pixel > 8 ? 1 : 0), regbase + GE_COLOR_DEPTH);
+
+ writel(p->fix.smem_start, regbase + GE_SRC_BADDR);
+ writel(p->var.xres_virtual - 1, regbase + GE_SRC_DISP_W);
+ writel(p->var.yres_virtual - 1, regbase + GE_SRC_DISP_H);
+ writel(area->sx, regbase + GE_SRC_X_START);
+ writel(area->sy, regbase + GE_SRC_Y_START);
+ writel(area->width - 1, regbase + GE_SRC_WIDTH);
+ writel(area->height - 1, regbase + GE_SRC_HEIGHT);
+
+ writel(p->fix.smem_start, regbase + GE_DES_BADDR);
+ writel(p->var.xres_virtual - 1, regbase + GE_DES_DISP_W);
+ writel(p->var.yres_virtual - 1, regbase + GE_DES_DISP_H);
+ writel(area->dx, regbase + GE_DES_X_START);
+ writel(area->dy, regbase + GE_DES_Y_START);
+ writel(area->width - 1, regbase + GE_DES_WIDTH);
+ writel(area->height - 1, regbase + GE_DES_HEIGHT);
+
+ writel(0xcc, regbase + GE_ROP_CODE);
+ writel(1, regbase + GE_COMMAND);
+ writel(1, regbase + GE_FIRE);
}
EXPORT_SYMBOL_GPL(wmt_ge_copyarea);
int wmt_ge_sync(struct fb_info *p)
{
int loops = 5000000;
- while ((readl(regbase + GE_STATUS_OFF) & 4) && --loops)
+ while ((readl(regbase + GE_STATUS) & 4) && --loops)
cpu_relax();
return loops > 0 ? 0 : -EBUSY;
}
@@ -146,7 +306,7 @@ static int wmt_ge_rops_probe(struct platform_device *pdev)
return -EBUSY;
}
- writel(1, regbase + GE_ENABLE_OFF);
+ writel(1, regbase + GE_ENABLE);
printk(KERN_INFO "Enabled support for WMT GE raster acceleration\n");
return 0;
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/4] fb: vt8500: Move register defines inside driver
From: Tony Prisk @ 2013-05-18 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1368868514-18975-1-git-send-email-linux@prisktech.co.nz>
The #defines in wm8505fb_regs.h are only used in the wm8505fb driver,
and don't need to be visible outside.
Move the defines into the driver and remove the header.
Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
drivers/video/wm8505fb.c | 25 +++++++++++++-
drivers/video/wm8505fb_regs.h | 76 -----------------------------------------
2 files changed, 24 insertions(+), 77 deletions(-)
delete mode 100644 drivers/video/wm8505fb_regs.h
diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c
index 01f9ace..f824af8 100644
--- a/drivers/video/wm8505fb.c
+++ b/drivers/video/wm8505fb.c
@@ -34,11 +34,34 @@
#include <linux/wait.h>
#include <video/of_display_timing.h>
-#include "wm8505fb_regs.h"
#include "wmt_ge_rops.h"
#define DRIVER_NAME "wm8505-fb"
+#define WMT_GOVR_COLORSPACE1 0x030
+#define WMT_GOVR_MIF_ENABLE 0x080
+#define WMT_GOVR_FBADDR 0x090
+#define WMT_GOVR_FBADDR1 0x094
+#define WMT_GOVR_XRES 0x098
+#define WMT_GOVR_XRES_VIRTUAL 0x09c
+#define WMT_GOVR_YPAN 0x0a0
+#define WMT_GOVR_XPAN 0x0a4
+#define WMT_GOVR_FHI 0x0a8
+#define WMT_GOVR_REG_UPDATE 0x0e4
+#define WMT_GOVR_TG 0x100
+#define WMT_GOVR_TIMING_H_ALL 0x108
+#define WMT_GOVR_TIMING_V_ALL 0x10c
+#define WMT_GOVR_TIMING_V_START 0x110
+#define WMT_GOVR_TIMING_V_END 0x114
+#define WMT_GOVR_TIMING_H_START 0x118
+#define WMT_GOVR_TIMING_H_END 0x11c
+#define WMT_GOVR_TIMING_V_SYNC 0x128
+#define WMT_GOVR_TIMING_H_SYNC 0x12c
+#define WMT_GOVR_DVO_SET 0x148
+#define WMT_GOVR_CONTRAST 0x1b8
+#define WMT_GOVR_BRGHTNESS 0x1bc
+#define WMT_GOVR_COLORSPACE 0x1e4
+
#define to_wm8505fb_info(__info) container_of(__info, \
struct wm8505fb_info, fb)
struct wm8505fb_info {
diff --git a/drivers/video/wm8505fb_regs.h b/drivers/video/wm8505fb_regs.h
deleted file mode 100644
index 4dd4166..0000000
--- a/drivers/video/wm8505fb_regs.h
+++ /dev/null
@@ -1,76 +0,0 @@
-/*
- * GOVR registers list for WM8505 chips
- *
- * Copyright (C) 2010 Ed Spiridonov <edo.rus@gmail.com>
- * Based on VIA/WonderMedia wm8510-govrh-reg.h
- * http://github.com/projectgus/kernel_wm8505/blob/wm8505_2.6.29/
- * drivers/video/wmt/register/wm8510/wm8510-govrh-reg.h
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-#ifndef _WM8505FB_REGS_H
-#define _WM8505FB_REGS_H
-
-/*
- * Color space select register, default value 0x1c
- * BIT0 GOVRH_DVO_YUV2RGB_ENABLE
- * BIT1 GOVRH_VGA_YUV2RGB_ENABLE
- * BIT2 GOVRH_RGB_MODE
- * BIT3 GOVRH_DAC_CLKINV
- * BIT4 GOVRH_BLANK_ZERO
- */
-#define WMT_GOVR_COLORSPACE 0x1e4
-/*
- * Another colorspace select register, default value 1
- * BIT0 GOVRH_DVO_RGB
- * BIT1 GOVRH_DVO_YUV422
- */
-#define WMT_GOVR_COLORSPACE1 0x30
-
-#define WMT_GOVR_CONTRAST 0x1b8
-#define WMT_GOVR_BRGHTNESS 0x1bc /* incompatible with RGB? */
-
-/* Framubeffer address */
-#define WMT_GOVR_FBADDR 0x90
-#define WMT_GOVR_FBADDR1 0x94 /* UV offset in YUV mode */
-
-/* Offset of visible window */
-#define WMT_GOVR_XPAN 0xa4
-#define WMT_GOVR_YPAN 0xa0
-
-#define WMT_GOVR_XRES 0x98
-#define WMT_GOVR_XRES_VIRTUAL 0x9c
-
-#define WMT_GOVR_MIF_ENABLE 0x80
-#define WMT_GOVR_FHI 0xa8
-#define WMT_GOVR_REG_UPDATE 0xe4
-
-/*
- * BIT0 GOVRH_DVO_OUTWIDTH
- * BIT1 GOVRH_DVO_SYNC_POLAR
- * BIT2 GOVRH_DVO_ENABLE
- */
-#define WMT_GOVR_DVO_SET 0x148
-
-/* Timing generator? */
-#define WMT_GOVR_TG 0x100
-
-/* Timings */
-#define WMT_GOVR_TIMING_H_ALL 0x108
-#define WMT_GOVR_TIMING_V_ALL 0x10c
-#define WMT_GOVR_TIMING_V_START 0x110
-#define WMT_GOVR_TIMING_V_END 0x114
-#define WMT_GOVR_TIMING_H_START 0x118
-#define WMT_GOVR_TIMING_H_END 0x11c
-#define WMT_GOVR_TIMING_V_SYNC 0x128
-#define WMT_GOVR_TIMING_H_SYNC 0x12c
-
-#endif /* _WM8505FB_REGS_H */
--
1.7.9.5
^ 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