* Re: [PATCH v3 6/9] radeon: Switch to arch_phys_wc_add and add a missing ..._del
From: Jerome Glisse @ 2013-05-15 14:49 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Alex Deucher, linux-kernel, dri-devel, linux-fbdev, Daniel Vetter,
Dave Airlie
In-Reply-To: <CALCETrVo-6cnKZsDJnBJcwGjpPbW9QAYriyjAZavaZ8O2OkjnQ@mail.gmail.com>
On Tue, May 14, 2013 at 5:35 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Tue, May 14, 2013 at 6:37 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
>> On Tue, May 14, 2013 at 8:58 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>> On Mon, May 13, 2013 at 7:58 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>>>
>>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>
>> I believe it will break something but we could deal with the fallout
>> once it happens.
>
> FWIW, I'm running with this code on my machine right now using the
> radeon driver. Everything seems fine. If I build without MTRRs and
> without PAT, though, graphics are slow (as expected). So I think
> everything's okay.
>
> --Andy
I am worried on p4 where i last see issue with that notably with agp.
Cheers,
Jerome
^ permalink raw reply
* Re: Introduce a new helper framework for buffer synchronization
From: Rob Clark @ 2013-05-15 14:06 UTC (permalink / raw)
To: Inki Dae
Cc: linux-fbdev, DRI mailing list, Kyungmin Park, myungjoo.ham,
YoungJun Cho, linux-arm-kernel, linux-media
In-Reply-To: <00cf01ce512b$bacc5540$3064ffc0$%dae@samsung.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.
BR,
-R
> 6) dma access to buffer
> 7) wait for buffer <- at user side - ioctl(fd, DMA_BUF_GET_FENCE, ...)
> - but DMA is already accessing the buffer so blocked.
> 8) signal <- at device driver
> 9) the thread, blocked at 7), is waked up by 8)
> - and then prepare-access (dma_buf_begin_cpu_access)
> 10 cpu access to buffer
>
> Basically, 'wait for buffer' includes buffer synchronization, committing
> processing, and cache operation. The buffer synchronization means that a
> current thread should wait for other threads accessing a shared buffer until
> the completion of their access. And the committing processing means that a
> current thread possesses the shared buffer so any trying to access the
> shared buffer by another thread makes the thread to be blocked. However, as
> I already mentioned before, it seems that these user interfaces are so ugly
> yet. So we need better way.
>
> Give me more comments if there is my missing point :)
>
> Thanks,
> Inki Dae
>
>> BR,
>> -R
>>
>>
>> > 2) finish-access (dma_buf_end _cpu_access)
>> > 3) dma access to buffer
>> >
>> > 1) and 2) are coupled with one function: we have implemented
>> > fence_helper_commit_reserve() for it.
>> >
>> > Cache control(cache clean or cache invalidate) is performed properly
>> > checking previous access type and current access type.
>> > And the below is actual codes for it,
>
^ permalink raw reply
* Re: [PATCH] console/font: Refactor font support code selection logic
From: Hans Verkuil @ 2013-05-15 11:45 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Florian Tobias Schandinat, linux-fbdev, Ismael Luceno,
Thomas Winischhofer, Thomas Bogendoerfer, Helge Deller,
linux-media, devel, linux-usb, linux-kernel
In-Reply-To: <1368618050-26895-1-git-send-email-geert@linux-m68k.org>
On Wed 15 May 2013 13:40:50 Geert Uytterhoeven wrote:
> The current Makefile rules to build font support are messy and buggy.
> Replace them by Kconfig rules:
> - Introduce CONFIG_FONT_SUPPORT, which controls the building of all font
> code,
> - Select CONFIG_FONT_SUPPORT for all drivers that use fonts,
> - Select CONFIG_FONT_8x16 for all drivers that default to the VGA8x16
> font,
> - Drop the bogus console dependency for CONFIG_VIDEO_VIVI.
>
> This fixes (if CONFIG_SOLO6X10=y and there are no built-in console
> drivers):
>
> drivers/built-in.o: In function `solo_osd_print':
> drivers/staging/media/solo6x10/solo6x10-enc.c:144: undefined reference to `.find_font'
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
That looks much more sane. Thanks!
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
> drivers/media/platform/Kconfig | 2 +-
> drivers/staging/media/solo6x10/Kconfig | 2 ++
> drivers/usb/misc/sisusbvga/Kconfig | 1 +
> drivers/video/console/Kconfig | 12 ++++++++++--
> drivers/video/console/Makefile | 14 +++++---------
> 5 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 0cbe1ff..c1f29d5 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -220,7 +220,7 @@ if V4L_TEST_DRIVERS
> config VIDEO_VIVI
> tristate "Virtual Video Driver"
> depends on VIDEO_DEV && VIDEO_V4L2 && !SPARC32 && !SPARC64
> - depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE
> + select FONT_SUPPORT
> select FONT_8x16
> select VIDEOBUF2_VMALLOC
> default n
> diff --git a/drivers/staging/media/solo6x10/Kconfig b/drivers/staging/media/solo6x10/Kconfig
> index ec32776..b34bb6c 100644
> --- a/drivers/staging/media/solo6x10/Kconfig
> +++ b/drivers/staging/media/solo6x10/Kconfig
> @@ -1,6 +1,8 @@
> config SOLO6X10
> tristate "Softlogic 6x10 MPEG codec cards"
> depends on PCI && VIDEO_DEV && SND && I2C
> + select FONT_SUPPORT
> + select FONT_8x16
> select VIDEOBUF2_DMA_SG
> select VIDEOBUF2_DMA_CONTIG
> select SND_PCM
> diff --git a/drivers/usb/misc/sisusbvga/Kconfig b/drivers/usb/misc/sisusbvga/Kconfig
> index 0d03a52..36bc28c 100644
> --- a/drivers/usb/misc/sisusbvga/Kconfig
> +++ b/drivers/usb/misc/sisusbvga/Kconfig
> @@ -2,6 +2,7 @@
> config USB_SISUSBVGA
> tristate "USB 2.0 SVGA dongle support (Net2280/SiS315)"
> depends on (USB_MUSB_HDRC || USB_EHCI_HCD)
> + select FONT_SUPPORT if USB_SISUSBVGA_CON
> ---help---
> Say Y here if you intend to attach a USB2VGA dongle based on a
> Net2280 and a SiS315 chip.
> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> index bc922c4..baf27dc 100644
> --- a/drivers/video/console/Kconfig
> +++ b/drivers/video/console/Kconfig
> @@ -62,6 +62,7 @@ config MDA_CONSOLE
> config SGI_NEWPORT_CONSOLE
> tristate "SGI Newport Console support"
> depends on SGI_IP22
> + select FONT_SUPPORT
> help
> Say Y here if you want the console on the Newport aka XL graphics
> card of your Indy. Most people say Y here.
> @@ -91,6 +92,7 @@ config FRAMEBUFFER_CONSOLE
> tristate "Framebuffer Console support"
> depends on FB
> select CRC32
> + select FONT_SUPPORT
> help
> Low-level framebuffer-based console driver.
>
> @@ -123,12 +125,18 @@ config FRAMEBUFFER_CONSOLE_ROTATION
> config STI_CONSOLE
> bool "STI text console"
> depends on PARISC
> + select FONT_SUPPORT
> default y
> help
> The STI console is the builtin display/keyboard on HP-PARISC
> machines. Say Y here to build support for it into your kernel.
> The alternative is to use your primary serial port as a console.
>
> +config FONT_SUPPORT
> + tristate
> +
> +if FONT_SUPPORT
> +
> config FONTS
> bool "Select compiled-in fonts"
> depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE
> @@ -158,7 +166,6 @@ config FONT_8x8
>
> config FONT_8x16
> bool "VGA 8x16 font" if FONTS
> - depends on FRAMEBUFFER_CONSOLE || SGI_NEWPORT_CONSOLE || STI_CONSOLE || USB_SISUSBVGA_CON
> default y if !SPARC && !FONTS
> help
> This is the "high resolution" font for the VGA frame buffer (the one
> @@ -226,7 +233,6 @@ config FONT_10x18
>
> config FONT_AUTOSELECT
> def_bool y
> - depends on FRAMEBUFFER_CONSOLE || SGI_NEWPORT_CONSOLE || STI_CONSOLE || USB_SISUSBVGA_CON
> depends on !FONT_8x8
> depends on !FONT_6x11
> depends on !FONT_7x14
> @@ -238,5 +244,7 @@ config FONT_AUTOSELECT
> depends on !FONT_10x18
> select FONT_8x16
>
> +endif # FONT_SUPPORT
> +
> endmenu
>
> diff --git a/drivers/video/console/Makefile b/drivers/video/console/Makefile
> index a862e91..3a11b63 100644
> --- a/drivers/video/console/Makefile
> +++ b/drivers/video/console/Makefile
> @@ -18,14 +18,14 @@ font-objs-$(CONFIG_FONT_MINI_4x6) += font_mini_4x6.o
>
> font-objs += $(font-objs-y)
>
> -# Each configuration option enables a list of files.
> +obj-$(CONFIG_FONT_SUPPORT) += font.o
>
> obj-$(CONFIG_DUMMY_CONSOLE) += dummycon.o
> -obj-$(CONFIG_SGI_NEWPORT_CONSOLE) += newport_con.o font.o
> -obj-$(CONFIG_STI_CONSOLE) += sticon.o sticore.o font.o
> +obj-$(CONFIG_SGI_NEWPORT_CONSOLE) += newport_con.o
> +obj-$(CONFIG_STI_CONSOLE) += sticon.o sticore.o
> obj-$(CONFIG_VGA_CONSOLE) += vgacon.o
> obj-$(CONFIG_MDA_CONSOLE) += mdacon.o
> -obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbcon.o bitblit.o font.o softcursor.o
> +obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbcon.o bitblit.o softcursor.o
> ifeq ($(CONFIG_FB_TILEBLITTING),y)
> obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += tileblit.o
> endif
> @@ -34,8 +34,4 @@ obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \
> fbcon_ccw.o
> endif
>
> -obj-$(CONFIG_FB_STI) += sticore.o font.o
> -
> -ifeq ($(CONFIG_USB_SISUSBVGA_CON),y)
> -obj-$(CONFIG_USB_SISUSBVGA) += font.o
> -endif
> +obj-$(CONFIG_FB_STI) += sticore.o
>
^ permalink raw reply
* [PATCH] console/font: Refactor font support code selection logic
From: Geert Uytterhoeven @ 2013-05-15 11:40 UTC (permalink / raw)
To: Florian Tobias Schandinat, linux-fbdev
Cc: Hans Verkuil, Ismael Luceno, Thomas Winischhofer,
Thomas Bogendoerfer, Helge Deller, linux-media, devel, linux-usb,
linux-kernel, Geert Uytterhoeven
The current Makefile rules to build font support are messy and buggy.
Replace them by Kconfig rules:
- Introduce CONFIG_FONT_SUPPORT, which controls the building of all font
code,
- Select CONFIG_FONT_SUPPORT for all drivers that use fonts,
- Select CONFIG_FONT_8x16 for all drivers that default to the VGA8x16
font,
- Drop the bogus console dependency for CONFIG_VIDEO_VIVI.
This fixes (if CONFIG_SOLO6X10=y and there are no built-in console
drivers):
drivers/built-in.o: In function `solo_osd_print':
drivers/staging/media/solo6x10/solo6x10-enc.c:144: undefined reference to `.find_font'
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
drivers/media/platform/Kconfig | 2 +-
drivers/staging/media/solo6x10/Kconfig | 2 ++
drivers/usb/misc/sisusbvga/Kconfig | 1 +
drivers/video/console/Kconfig | 12 ++++++++++--
drivers/video/console/Makefile | 14 +++++---------
5 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 0cbe1ff..c1f29d5 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -220,7 +220,7 @@ if V4L_TEST_DRIVERS
config VIDEO_VIVI
tristate "Virtual Video Driver"
depends on VIDEO_DEV && VIDEO_V4L2 && !SPARC32 && !SPARC64
- depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE
+ select FONT_SUPPORT
select FONT_8x16
select VIDEOBUF2_VMALLOC
default n
diff --git a/drivers/staging/media/solo6x10/Kconfig b/drivers/staging/media/solo6x10/Kconfig
index ec32776..b34bb6c 100644
--- a/drivers/staging/media/solo6x10/Kconfig
+++ b/drivers/staging/media/solo6x10/Kconfig
@@ -1,6 +1,8 @@
config SOLO6X10
tristate "Softlogic 6x10 MPEG codec cards"
depends on PCI && VIDEO_DEV && SND && I2C
+ select FONT_SUPPORT
+ select FONT_8x16
select VIDEOBUF2_DMA_SG
select VIDEOBUF2_DMA_CONTIG
select SND_PCM
diff --git a/drivers/usb/misc/sisusbvga/Kconfig b/drivers/usb/misc/sisusbvga/Kconfig
index 0d03a52..36bc28c 100644
--- a/drivers/usb/misc/sisusbvga/Kconfig
+++ b/drivers/usb/misc/sisusbvga/Kconfig
@@ -2,6 +2,7 @@
config USB_SISUSBVGA
tristate "USB 2.0 SVGA dongle support (Net2280/SiS315)"
depends on (USB_MUSB_HDRC || USB_EHCI_HCD)
+ select FONT_SUPPORT if USB_SISUSBVGA_CON
---help---
Say Y here if you intend to attach a USB2VGA dongle based on a
Net2280 and a SiS315 chip.
diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index bc922c4..baf27dc 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -62,6 +62,7 @@ config MDA_CONSOLE
config SGI_NEWPORT_CONSOLE
tristate "SGI Newport Console support"
depends on SGI_IP22
+ select FONT_SUPPORT
help
Say Y here if you want the console on the Newport aka XL graphics
card of your Indy. Most people say Y here.
@@ -91,6 +92,7 @@ config FRAMEBUFFER_CONSOLE
tristate "Framebuffer Console support"
depends on FB
select CRC32
+ select FONT_SUPPORT
help
Low-level framebuffer-based console driver.
@@ -123,12 +125,18 @@ config FRAMEBUFFER_CONSOLE_ROTATION
config STI_CONSOLE
bool "STI text console"
depends on PARISC
+ select FONT_SUPPORT
default y
help
The STI console is the builtin display/keyboard on HP-PARISC
machines. Say Y here to build support for it into your kernel.
The alternative is to use your primary serial port as a console.
+config FONT_SUPPORT
+ tristate
+
+if FONT_SUPPORT
+
config FONTS
bool "Select compiled-in fonts"
depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE
@@ -158,7 +166,6 @@ config FONT_8x8
config FONT_8x16
bool "VGA 8x16 font" if FONTS
- depends on FRAMEBUFFER_CONSOLE || SGI_NEWPORT_CONSOLE || STI_CONSOLE || USB_SISUSBVGA_CON
default y if !SPARC && !FONTS
help
This is the "high resolution" font for the VGA frame buffer (the one
@@ -226,7 +233,6 @@ config FONT_10x18
config FONT_AUTOSELECT
def_bool y
- depends on FRAMEBUFFER_CONSOLE || SGI_NEWPORT_CONSOLE || STI_CONSOLE || USB_SISUSBVGA_CON
depends on !FONT_8x8
depends on !FONT_6x11
depends on !FONT_7x14
@@ -238,5 +244,7 @@ config FONT_AUTOSELECT
depends on !FONT_10x18
select FONT_8x16
+endif # FONT_SUPPORT
+
endmenu
diff --git a/drivers/video/console/Makefile b/drivers/video/console/Makefile
index a862e91..3a11b63 100644
--- a/drivers/video/console/Makefile
+++ b/drivers/video/console/Makefile
@@ -18,14 +18,14 @@ font-objs-$(CONFIG_FONT_MINI_4x6) += font_mini_4x6.o
font-objs += $(font-objs-y)
-# Each configuration option enables a list of files.
+obj-$(CONFIG_FONT_SUPPORT) += font.o
obj-$(CONFIG_DUMMY_CONSOLE) += dummycon.o
-obj-$(CONFIG_SGI_NEWPORT_CONSOLE) += newport_con.o font.o
-obj-$(CONFIG_STI_CONSOLE) += sticon.o sticore.o font.o
+obj-$(CONFIG_SGI_NEWPORT_CONSOLE) += newport_con.o
+obj-$(CONFIG_STI_CONSOLE) += sticon.o sticore.o
obj-$(CONFIG_VGA_CONSOLE) += vgacon.o
obj-$(CONFIG_MDA_CONSOLE) += mdacon.o
-obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbcon.o bitblit.o font.o softcursor.o
+obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbcon.o bitblit.o softcursor.o
ifeq ($(CONFIG_FB_TILEBLITTING),y)
obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += tileblit.o
endif
@@ -34,8 +34,4 @@ obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \
fbcon_ccw.o
endif
-obj-$(CONFIG_FB_STI) += sticore.o font.o
-
-ifeq ($(CONFIG_USB_SISUSBVGA_CON),y)
-obj-$(CONFIG_USB_SISUSBVGA) += font.o
-endif
+obj-$(CONFIG_FB_STI) += sticore.o
--
1.7.0.4
^ permalink raw reply related
* RE: Introduce a new helper framework for buffer synchronization
From: Inki Dae @ 2013-05-15 5:19 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <51909DB4.2060208@canonical.com>
> -----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)
6) dma access to buffer
7) wait for buffer <- at user side - ioctl(fd, DMA_BUF_GET_FENCE, ...)
- but DMA is already accessing the buffer so blocked.
8) signal <- at device driver
9) the thread, blocked at 7), is waked up by 8)
- and then prepare-access (dma_buf_begin_cpu_access)
10 cpu access to buffer
Basically, 'wait for buffer' includes buffer synchronization, committing
processing, and cache operation. The buffer synchronization means that a
current thread should wait for other threads accessing a shared buffer until
the completion of their access. And the committing processing means that a
current thread possesses the shared buffer so any trying to access the
shared buffer by another thread makes the thread to be blocked. However, as
I already mentioned before, it seems that these user interfaces are so ugly
yet. So we need better way.
Give me more comments if there is my missing point :)
Thanks,
Inki Dae
> BR,
> -R
>
>
> > 2) finish-access (dma_buf_end _cpu_access)
> > 3) dma access to buffer
> >
> > 1) and 2) are coupled with one function: we have implemented
> > fence_helper_commit_reserve() for it.
> >
> > Cache control(cache clean or cache invalidate) is performed properly
> > checking previous access type and current access type.
> > And the below is actual codes for it,
^ permalink raw reply
* [PATCH] ps3fb: Fix compile warning
From: Geoff Levand @ 2013-05-15 3:12 UTC (permalink / raw)
To: linux-fbdev
Fix the following compile warning when -Wparentheses is set:
drivers/video/ps3fb.c: warning: suggest parentheses around ‘+’ inside ‘<<’
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
Hi Florian,
Please apply.
-Geoff
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 d9f08c6..a397271d 100644
--- a/drivers/video/ps3fb.c
+++ b/drivers/video/ps3fb.c
@@ -710,7 +710,7 @@ static int ps3fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
r = vm_iomap_memory(vma, info->fix.smem_start, info->fix.smem_len);
dev_dbg(info->device, "ps3fb: mmap framebuffer P(%lx)->V(%lx)\n",
- info->fix.smem_start + vma->vm_pgoff << PAGE_SHIFT,
+ (info->fix.smem_start + vma->vm_pgoff) << PAGE_SHIFT,
vma->vm_start);
return r;
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH v3 6/9] radeon: Switch to arch_phys_wc_add and add a missing ..._del
From: Andy Lutomirski @ 2013-05-14 21:35 UTC (permalink / raw)
To: Jerome Glisse
Cc: Alex Deucher, linux-kernel, dri-devel, linux-fbdev, Daniel Vetter,
Dave Airlie
In-Reply-To: <CAH3drwbSLdjd4Y8x8YfzhM5WK+A-L7mnYeS0kdqu7MwEB5kbCQ@mail.gmail.com>
On Tue, May 14, 2013 at 6:37 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
> On Tue, May 14, 2013 at 8:58 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Mon, May 13, 2013 at 7:58 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>>
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> I believe it will break something but we could deal with the fallout
> once it happens.
FWIW, I'm running with this code on my machine right now using the
radeon driver. Everything seems fine. If I build without MTRRs and
without PAT, though, graphics are slow (as expected). So I think
everything's okay.
--Andy
^ permalink raw reply
* Re: [PATCH V5 5/5] fbcon: queue work on power efficient wq
From: Tejun Heo @ 2013-05-14 17:57 UTC (permalink / raw)
To: Viresh Kumar
Cc: davem, airlied, axboe, tglx, peterz, mingo, rostedt,
linux-rt-users, linux-kernel, robin.randhawa, Steve.Bannister,
Liviu.Dudau, charles.garcia-tobin, arvind.chauhan, linaro-kernel,
patches, linux-fbdev
In-Reply-To: <89924f001e3ddd66ab41a16500569a976ea846fd.1366803121.git.viresh.kumar@linaro.org>
On Wed, Apr 24, 2013 at 05:12:57PM +0530, Viresh Kumar wrote:
> fbcon uses workqueues and it has no real dependency of scheduling these on the
> cpu which scheduled them.
>
> On a idle system, it is observed that and idle cpu wakes up many times just to
> service this work. It would be better if we can schedule it on a cpu which the
> scheduler believes to be the most appropriate one.
>
> This patch replaces system_wq with system_power_efficient_wq.
>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: linux-fbdev@vger.kernel.org
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Dave, applied this to wq/for-3.11. Please holler if you want this to
be routed differently.
Thanks!
--
tejun
^ permalink raw reply
* Re: Build regressions/improvements in v3.10-rc1 (cris)
From: Jesper Nilsson @ 2013-05-14 16:33 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linux Kernel Development, Linux Fbdev development list,
linux-cris-kernel
In-Reply-To: <CAMuHMdW6p6tP=wwBNV9uAAbwwgpyzMiPxsBSJNOUazhznYC4iQ@mail.gmail.com>
On Tue, May 14, 2013 at 02:06:51PM +0200, Geert Uytterhoeven wrote:
> On Sun, May 12, 2013 at 10:44 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > drivers/video/console/fonts.c:71:2: error: #error No fonts configured.: 2 errors in 2 logs
> > v3.10-rc1/cris/cris-allmodconfig v3.10-rc1/cris/cris-allyesconfig
> >
> > Fbdev issue?
>
> This is caused by cris not using the generic drivers/Kconfig, and thus not
> traversing drivers/video/console/Kconfig.
> As the build system does traverse drivers/video/console/Makefile, fonts.c
> is compiled with an inconsistent configuration.
>
> Two solutions:
> 1. Add drivers/video/console/Kconfig to arch/cris/Kconfig,
> 2. Switch cris to drivers/Kconfig,
>
> I prefer two, as this is what's done by all (except h8300) other
> architectures. This will seriously broaden the scope of allmodconfig,
> though, and require more fixes (e.g. missing <asm/vga.h>).
I would have no objections a conversion of CRIS to use the
drivers/Kconfig, though it feels a bit strange to compile
video drivers for SoCs that don't have any video out hardware.
> Note: The decision logic for compiling drivers/video/console/fonts.c is
> overly complicated, and seems to be buggy for some stuff living
> outside drivers drivers/video (drivers/media/platform/vivi.c and
> drivers/staging/media/solo6x10/solo6x10-enc.c). I think this should
> be resolved in Kconfig logic, using a new FONT_SUPPORT
> symbol (FONTS is already taken).
>
> 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
/^JN - Jesper Nilsson
--
Jesper Nilsson -- jesper.nilsson@axis.com
^ permalink raw reply
* Re: [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver
From: Tomasz Figa @ 2013-05-14 14:16 UTC (permalink / raw)
To: Vikas Sajjan
Cc: jg1.han, linux-samsung-soc, kgene.kim, devicetree-discuss,
patches, linaro-kernel, rpurdie, FlorianSchandinat, linux-fbdev
In-Reply-To: <1368536152-13370-2-git-send-email-vikas.sajjan@linaro.org>
Hi Vikas,
On Tuesday 14 of May 2013 18:25:51 Vikas Sajjan wrote:
> Adds GPIO parsing functionality for "LCD backlight" and "LCD enable"
> GPIO pins of exynos dp controller.
>
> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> ---
> drivers/video/exynos/exynos_dp_core.c | 45
> +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
>
I don't think that Exynos DP driver is right place for such code. Backlight
and LCD drivers are responsible for backlight and LCD power control using
backlight and LCD subsystems.
IMHO the correct solution would be to either extend existing backlight/lcd
drivers found in drivers/video/backlight to support direct GPIO control and
parse GPIO pins from device tree or create new gpio_bl and gpio_lcd drivers.
CCing Richard, Florian and linux-fbdev.
Best regards,
Tomasz
^ permalink raw reply
* Re: Introduce a new helper framework for buffer synchronization
From: Rob Clark @ 2013-05-14 13:38 UTC (permalink / raw)
To: Inki Dae
Cc: linux-fbdev, DRI mailing list, Kyungmin Park, myungjoo.ham,
YoungJun Cho, linux-arm-kernel, linux-media
In-Reply-To: <006a01ce504e$0de3b0e0$29ab12a0$%dae@samsung.com>
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.
BR,
-R
> 2) finish-access (dma_buf_end _cpu_access)
> 3) dma access to buffer
>
> 1) and 2) are coupled with one function: we have implemented
> fence_helper_commit_reserve() for it.
>
> Cache control(cache clean or cache invalidate) is performed properly
> checking previous access type and current access type.
> And the below is actual codes for it,
^ permalink raw reply
* Re: [PATCH v3 6/9] radeon: Switch to arch_phys_wc_add and add a missing ..._del
From: Jerome Glisse @ 2013-05-14 13:37 UTC (permalink / raw)
To: Alex Deucher
Cc: linux-fbdev, Daniel Vetter, linux-kernel, dri-devel,
Andy Lutomirski
In-Reply-To: <CADnq5_O6tZeRVq=gmJfn3JWQGgECSE1JuonpSKgFCYSD8RCqHQ@mail.gmail.com>
On Tue, May 14, 2013 at 8:58 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Mon, May 13, 2013 at 7:58 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
I believe it will break something but we could deal with the fallout
once it happens.
>> ---
>> drivers/gpu/drm/radeon/radeon_object.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
>> index d3aface..15cd34b 100644
>> --- a/drivers/gpu/drm/radeon/radeon_object.c
>> +++ b/drivers/gpu/drm/radeon/radeon_object.c
>> @@ -321,8 +321,8 @@ void radeon_bo_force_delete(struct radeon_device *rdev)
>> int radeon_bo_init(struct radeon_device *rdev)
>> {
>> /* Add an MTRR for the VRAM */
>> - rdev->mc.vram_mtrr = mtrr_add(rdev->mc.aper_base, rdev->mc.aper_size,
>> - MTRR_TYPE_WRCOMB, 1);
>> + rdev->mc.vram_mtrr = arch_phys_wc_add(rdev->mc.aper_base,
>> + rdev->mc.aper_size);
>> DRM_INFO("Detected VRAM RAM=%lluM, BAR=%lluM\n",
>> rdev->mc.mc_vram_size >> 20,
>> (unsigned long long)rdev->mc.aper_size >> 20);
>> @@ -334,6 +334,7 @@ int radeon_bo_init(struct radeon_device *rdev)
>> void radeon_bo_fini(struct radeon_device *rdev)
>> {
>> radeon_ttm_fini(rdev);
>> + arch_phys_wc_del(rdev->mc.vram_mtrr);
>> }
>>
>> void radeon_bo_list_add_object(struct radeon_bo_list *lobj,
>> --
>> 1.8.1.4
>>
^ permalink raw reply
* Re: [PATCH v3 6/9] radeon: Switch to arch_phys_wc_add and add a missing ..._del
From: Alex Deucher @ 2013-05-14 12:58 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel, dri-devel, linux-fbdev, Daniel Vetter,
Jerome Glisse, Dave Airlie
In-Reply-To: <bf7788cfd4695d05f2b414735744105906650e42.1368485053.git.luto@amacapital.net>
On Mon, May 13, 2013 at 7:58 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/radeon/radeon_object.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index d3aface..15cd34b 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -321,8 +321,8 @@ void radeon_bo_force_delete(struct radeon_device *rdev)
> int radeon_bo_init(struct radeon_device *rdev)
> {
> /* Add an MTRR for the VRAM */
> - rdev->mc.vram_mtrr = mtrr_add(rdev->mc.aper_base, rdev->mc.aper_size,
> - MTRR_TYPE_WRCOMB, 1);
> + rdev->mc.vram_mtrr = arch_phys_wc_add(rdev->mc.aper_base,
> + rdev->mc.aper_size);
> DRM_INFO("Detected VRAM RAM=%lluM, BAR=%lluM\n",
> rdev->mc.mc_vram_size >> 20,
> (unsigned long long)rdev->mc.aper_size >> 20);
> @@ -334,6 +334,7 @@ int radeon_bo_init(struct radeon_device *rdev)
> void radeon_bo_fini(struct radeon_device *rdev)
> {
> radeon_ttm_fini(rdev);
> + arch_phys_wc_del(rdev->mc.vram_mtrr);
> }
>
> void radeon_bo_list_add_object(struct radeon_bo_list *lobj,
> --
> 1.8.1.4
>
^ permalink raw reply
* Re: Build regressions/improvements in v3.10-rc1 (cris)
From: Geert Uytterhoeven @ 2013-05-14 12:06 UTC (permalink / raw)
To: Linux Kernel Development; +Cc: Cris, Linux Fbdev development list
In-Reply-To: <alpine.DEB.2.00.1305122242430.5463@ayla.of.borg>
On Sun, May 12, 2013 at 10:44 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> drivers/video/console/fonts.c:71:2: error: #error No fonts configured.: 2 errors in 2 logs
> v3.10-rc1/cris/cris-allmodconfig v3.10-rc1/cris/cris-allyesconfig
>
> Fbdev issue?
This is caused by cris not using the generic drivers/Kconfig, and thus not
traversing drivers/video/console/Kconfig.
As the build system does traverse drivers/video/console/Makefile, fonts.c
is compiled with an inconsistent configuration.
Two solutions:
1. Add drivers/video/console/Kconfig to arch/cris/Kconfig,
2. Switch cris to drivers/Kconfig,
I prefer two, as this is what's done by all (except h8300) other
architectures. This will seriously broaden the scope of allmodconfig,
though, and require more fixes (e.g. missing <asm/vga.h>).
Note: The decision logic for compiling drivers/video/console/fonts.c is
overly complicated, and seems to be buggy for some stuff living
outside drivers drivers/video (drivers/media/platform/vivi.c and
drivers/staging/media/solo6x10/solo6x10-enc.c). I think this should
be resolved in Kconfig logic, using a new FONT_SUPPORT
symbol (FONTS is already taken).
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] drivers: video: mxsfb: clean use of devm_ioremap_resource()
From: Laurent Navet @ 2013-05-14 9:25 UTC (permalink / raw)
To: FlorianSchandinat; +Cc: linux-fbdev, linux-kernel, Laurent Navet
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 | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 21223d4..0f3d0fc 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -884,9 +884,10 @@ static int mxsfb_probe(struct platform_device *pdev)
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;
+ host->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(host->base)) {
+ ret = PTR_ERR(host->base);
+ goto fb_release;
}
fb_info = framebuffer_alloc(sizeof(struct mxsfb_info), &pdev->dev);
@@ -897,13 +898,6 @@ static int mxsfb_probe(struct platform_device *pdev)
host = to_imxfb_host(fb_info);
- 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;
- }
-
host->pdev = pdev;
platform_set_drvdata(pdev, host);
--
1.7.10.4
^ permalink raw reply related
* RE: Introduce a new helper framework for buffer synchronization
From: Inki Dae @ 2013-05-14 2:52 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <51909DB4.2060208@canonical.com>
> -----Original Message-----
> From: Rob Clark [mailto:robdclark@gmail.com]
> Sent: Tuesday, May 14, 2013 2:58 AM
> 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 1:18 PM, Inki Dae <inki.dae@samsung.com> wrote:
> >
> >
> > 2013/5/13 Rob Clark <robdclark@gmail.com>
> >>
> >> On Mon, May 13, 2013 at 8:21 AM, Inki Dae <inki.dae@samsung.com> wrote:
> >> >
> >> >> In that case you still wouldn't give userspace control over the
> fences.
> >> >> I
> >> >> don't see any way that can end well.
> >> >> What if userspace never signals? What if userspace gets killed by
> oom
> >> >> killer. Who keeps track of that?
> >> >>
> >> >
> >> > In all cases, all kernel resources to user fence will be released by
> >> > kernel
> >> > once the fence is timed out: never signaling and process killing by
> oom
> >> > killer makes the fence timed out. And if we use mmap mechanism you
> >> > mentioned
> >> > before, I think user resource could also be freed properly.
> >>
> >>
> >> I tend to agree w/ Maarten here.. there is no good reason for
> >> userspace to be *signaling* fences. The exception might be some blob
> >> gpu drivers which don't have enough knowledge in the kernel to figure
> >> out what to do. (In which case you can add driver private ioctls for
> >> that.. still not the right thing to do but at least you don't make a
> >> public API out of it.)
> >>
> >
> > Please do not care whether those are generic or not. Let's see the
> following
> > three things. First, it's cache operation. As you know, ARM SoC has ACP
> > (Accelerator Coherency Port) and can be connected to DMA engine or
> similar
> > devices. And this port is used for cache coherency between CPU cache and
> DMA
> > device. However, most devices on ARM based embedded systems don't use
> the
> > ACP port. So they need proper cache operation before and after of DMA or
> CPU
> > access in case of using cachable mapping. Actually, I see many Linux
> based
> > platforms call cache control interfaces directly for that. I think the
> > reason, they do so, is that kernel isn't aware of when and how CPU
> accessed
> > memory.
>
> I think we had kicked around the idea of giving dmabuf's a
> prepare/finish ioctl quite some time back. This is probably something
> that should be at least a bit decoupled from fences. (Possibly
> 'prepare' waits for dma access to complete, but not the other way
> around.)
>
> And I did implement in omapdrm support for simulating coherency via
> page fault-in / shoot-down.. It is one option that makes it
> completely transparent to userspace, although there is some
> performance const, so I suppose it depends a bit on your use-case.
>
> > And second, user process has to do so many things in case of using
> shared
> > memory with DMA device. User process should understand how DMA device is
> > operated and when interfaces for controling the DMA device are called.
> Such
> > things would make user application so complicated.
> >
> > And third, it's performance optimization to multimedia and graphics
> devices.
> > As I mentioned already, we should consider sequential processing for
> buffer
> > sharing between CPU and DMA device. This means that CPU should stay with
> > idle until DMA device is completed and vise versa.
> >
> > That is why I proposed such user interfaces. Of course, these interfaces
> > might be so ugly yet: for this, Maarten pointed already out and I agree
> with
> > him. But there must be another better way. Aren't you think we need
> similar
> > thing? With such interfaces, cache control and buffer synchronization
> can be
> > performed in kernel level. Moreover, user applization doesn't need to
> > consider DMA device controlling anymore. Therefore, one thread can
> access a
> > shared buffer and the other can control DMA device with the shared
> buffer in
> > parallel. We can really make the best use of CPU and DMA idle time. In
> other
> > words, we can really make the best use of multi tasking OS, Linux.
> >
> > So could you please tell me about that there is any reason we don't use
> > public API for it? I think we can add and use public API if NECESSARY.
>
> 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
2) finish-access (dma_buf_end _cpu_access)
3) dma access to buffer
1) and 2) are coupled with one function: we have implemented
fence_helper_commit_reserve() for it.
Cache control(cache clean or cache invalidate) is performed properly
checking previous access type and current access type.
And the below is actual codes for it,
static void fence_helper_cache_ops(struct fence_helper *fh)
{
struct seqno_fence_dmabuf *sfd;
list_for_each_entry(sfd, &fh->sf.sync_buf_list, list) {
struct dma_buf *dmabuf = sfd->sync_buf;
if (WARN_ON(!dmabuf))
continue;
/* first time access. */
if (!dmabuf->access_type)
goto out;
if (dmabuf->access_type = DMA_BUF_ACCESS_WRITE &&
((fh->type = (DMA_BUF_ACCESS_READ |
DMA_BUF_ACCESS_DMA)) ||
(fh->type = (DMA_BUF_ACCESS_WRITE |
DMA_BUF_ACCESS_DMA))))
/* cache clean */
dma_buf_end_cpu_access(dmabuf, 0, dmabuf->size,
DMA_TO_DEVICE);
else if (dmabuf->access_type = (DMA_BUF_ACCESS_WRITE |
DMA_BUF_ACCESS_DMA) &&
(fh->type = DMA_BUF_ACCESS_READ))
/* cache invalidate */
dma_buf_begin_cpu_access(dmabuf, 0, dmabuf->size,
DMA_FROM_DEVICE);
out:
/* Update access type to new one. */
dmabuf->access_type = fh->type;
}
}
The above function is called after wait for buffer.
Thus, we can check who (CPU or DMA) and how (READ or WRITE) accessed and
accesses buffer with this approach. In other words, now kernel is aware of
buffer access by CPU also.
Thanks,
Inki Dae
> I suppose you could combine the syscall for #1 and #2.. not sure if
> that is a good idea or not. But you don't need to. And there is
> never really any need for userspace to signal a fence.
>
> BR,
> -R
>
> > Thanks,
> > Inki Dae
> >
> >>
> >> BR,
> >> -R
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
^ permalink raw reply
* RE: [PATCH 02/15] video: ep93xx: remove unnecessary platform_set_drvdata()
From: H Hartley Sweeten @ 2013-05-14 0:47 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <000201ce4e45$a5da7330$f18f5990$@samsung.com>
On Saturday, May 11, 2013 5:47 AM, Jingoo Han wrote:
> The driver core clears the driver data to NULL after device_release
> or on probe failure, since commit 0998d0631001288a5974afc0b2a5f568bcdecb4d
> (device-core: Ensure drvdata = NULL when no driver is bound).
> Thus, it is not needed to manually clear the device driver data to NULL.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
> drivers/video/ep93xx-fb.c | 2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>
^ permalink raw reply
* [PATCH v3 9/9] drm: Don't leak phys_wc "handles" to userspace
From: Andy Lutomirski @ 2013-05-13 23:58 UTC (permalink / raw)
To: linux-kernel, dri-devel, linux-fbdev
Cc: Daniel Vetter, Jerome Glisse, Alex Deucher, Dave Airlie,
Andy Lutomirski
In-Reply-To: <cover.1368485053.git.luto@amacapital.net>
I didn't fix this in the earlier patch -- it would have broken the
build due to the now-deleted garbage in drm_os_linux.h.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
drivers/gpu/drm/drm_bufs.c | 9 +++++++++
drivers/gpu/drm/drm_ioctl.c | 15 ++++++++++++++-
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index 0190fce..5a4dbb4 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -414,6 +414,15 @@ int drm_addmap_ioctl(struct drm_device *dev, void *data,
/* avoid a warning on 64-bit, this casting isn't very nice, but the API is set so too late */
map->handle = (void *)(unsigned long)maplist->user_token;
+
+ /*
+ * It appears that there are no users of this value whatsoever --
+ * drmAddMap just discards it. Let's not encourage its use.
+ * (Keeping drm_addmap_core's returned mtrr value would be wrong --
+ * it's not a real mtrr index anymore.)
+ */
+ map->mtrr = -1;
+
return 0;
}
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index e77bd8b..ffd7a7b 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -38,6 +38,9 @@
#include <linux/pci.h>
#include <linux/export.h>
+#ifdef CONFIG_X86
+#include <asm/mtrr.h>
+#endif
/**
* Get the bus id.
@@ -181,7 +184,17 @@ int drm_getmap(struct drm_device *dev, void *data,
map->type = r_list->map->type;
map->flags = r_list->map->flags;
map->handle = (void *)(unsigned long) r_list->user_token;
- map->mtrr = r_list->map->mtrr;
+
+#ifdef CONFIG_X86
+ /*
+ * There appears to be exactly one user of the mtrr index: dritest.
+ * It's easy enough to keep it working on non-PAT systems.
+ */
+ map->mtrr = phys_wc_to_mtrr_index(r_list->map->mtrr);
+#else
+ map->mtrr = -1;
+#endif
+
mutex_unlock(&dev->struct_mutex);
return 0;
--
1.8.1.4
^ permalink raw reply related
* [PATCH v3 8/9] drm: Remove mtrr_add and mtrr_del fallback hack for non-MTRR systems
From: Andy Lutomirski @ 2013-05-13 23:58 UTC (permalink / raw)
To: linux-kernel, dri-devel, linux-fbdev
Cc: Daniel Vetter, Jerome Glisse, Alex Deucher, Dave Airlie,
Andy Lutomirski
In-Reply-To: <cover.1368485053.git.luto@amacapital.net>
There are no users left in drivers/gpu.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
include/drm/drmP.h | 5 +----
include/drm/drm_os_linux.h | 16 ----------------
2 files changed, 1 insertion(+), 20 deletions(-)
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3e6cfa0..7a9fef5 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -55,16 +55,13 @@
#include <linux/mm.h>
#include <linux/cdev.h>
#include <linux/mutex.h>
+#include <linux/io.h>
#include <linux/slab.h>
#if defined(__alpha__) || defined(__powerpc__)
#include <asm/pgtable.h> /* For pte_wrprotect */
#endif
-#include <asm/io.h>
#include <asm/mman.h>
#include <asm/uaccess.h>
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
-#endif
#if defined(CONFIG_AGP) || defined(CONFIG_AGP_MODULE)
#include <linux/types.h>
#include <linux/agp_backend.h>
diff --git a/include/drm/drm_os_linux.h b/include/drm/drm_os_linux.h
index 3933691..35c7c2b 100644
--- a/include/drm/drm_os_linux.h
+++ b/include/drm/drm_os_linux.h
@@ -65,22 +65,6 @@ struct no_agp_kern {
#define DRM_AGP_KERN struct no_agp_kern
#endif
-#if !(__OS_HAS_MTRR)
-static __inline__ int mtrr_add(unsigned long base, unsigned long size,
- unsigned int type, char increment)
-{
- return -ENODEV;
-}
-
-static __inline__ int mtrr_del(int reg, unsigned long base, unsigned long size)
-{
- return -ENODEV;
-}
-
-#define MTRR_TYPE_WRCOMB 1
-
-#endif
-
/** Other copying of data to kernel space */
#define DRM_COPY_FROM_USER(arg1, arg2, arg3) \
copy_from_user(arg1, arg2, arg3)
--
1.8.1.4
^ permalink raw reply related
* [PATCH v3 7/9] uvesafb: Clean up MTRR code
From: Andy Lutomirski @ 2013-05-13 23:58 UTC (permalink / raw)
To: linux-kernel, dri-devel, linux-fbdev
Cc: Daniel Vetter, Jerome Glisse, Alex Deucher, Dave Airlie,
Andy Lutomirski
In-Reply-To: <cover.1368485053.git.luto@amacapital.net>
The old code allowed very strange memory types. Now it works like
all the other video drivers: ioremap_wc is used unconditionally,
and MTRRs are set if PAT is unavailable (unless MTRR is disabled
by a module parameter).
UC, WB, and WT support is gone. If there are MTRR conflicts that prevent
addition of a WC MTRR, adding a non-conflicting MTRR is pointless; it's
better to just turn off MTRR support entirely.
As an added bonus, any MTRR added is freed on unload.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
Documentation/fb/uvesafb.txt | 16 ++++------
drivers/video/uvesafb.c | 70 +++++++++++---------------------------------
include/video/uvesafb.h | 1 +
3 files changed, 23 insertions(+), 64 deletions(-)
diff --git a/Documentation/fb/uvesafb.txt b/Documentation/fb/uvesafb.txt
index eefdd91..f6362d8 100644
--- a/Documentation/fb/uvesafb.txt
+++ b/Documentation/fb/uvesafb.txt
@@ -81,17 +81,11 @@ pmipal Use the protected mode interface for palette changes.
mtrr:n Setup memory type range registers for the framebuffer
where n:
- 0 - disabled (equivalent to nomtrr) (default)
- 1 - uncachable
- 2 - write-back
- 3 - write-combining
- 4 - write-through
-
- If you see the following in dmesg, choose the type that matches
- the old one. In this example, use "mtrr:2".
-...
-mtrr: type mismatch for e0000000,8000000 old: write-back new: write-combining
-...
+ 0 - disabled (equivalent to nomtrr)
+ 3 - write-combining (default)
+
+ Values other than 0 and 3 will result in a warning and will be
+ treated just like 3.
nomtrr Do not use memory type range registers.
diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
index d428445..8701f96 100644
--- a/drivers/video/uvesafb.c
+++ b/drivers/video/uvesafb.c
@@ -24,9 +24,6 @@
#ifdef CONFIG_X86
#include <video/vga.h>
#endif
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
-#endif
#include "edid.h"
static struct cb_id uvesafb_cn_id = {
@@ -1540,67 +1537,30 @@ static void uvesafb_init_info(struct fb_info *info, struct vbe_mode_ib *mode)
static void uvesafb_init_mtrr(struct fb_info *info)
{
-#ifdef CONFIG_MTRR
+ struct uvesafb_par *par = info->par;
+
if (mtrr && !(info->fix.smem_start & (PAGE_SIZE - 1))) {
int temp_size = info->fix.smem_len;
- unsigned int type = 0;
- switch (mtrr) {
- case 1:
- type = MTRR_TYPE_UNCACHABLE;
- break;
- case 2:
- type = MTRR_TYPE_WRBACK;
- break;
- case 3:
- type = MTRR_TYPE_WRCOMB;
- break;
- case 4:
- type = MTRR_TYPE_WRTHROUGH;
- break;
- default:
- type = 0;
- break;
- }
+ int rc;
- if (type) {
- int rc;
+ /* Find the largest power-of-two */
+ temp_size = roundup_pow_of_two(temp_size);
- /* Find the largest power-of-two */
- temp_size = roundup_pow_of_two(temp_size);
+ /* Try and find a power of two to add */
+ do {
+ rc = arch_phys_wc_add(info->fix.smem_start, temp_size);
+ temp_size >>= 1;
+ } while (temp_size >= PAGE_SIZE && rc = -EINVAL);
- /* Try and find a power of two to add */
- do {
- rc = mtrr_add(info->fix.smem_start,
- temp_size, type, 1);
- temp_size >>= 1;
- } while (temp_size >= PAGE_SIZE && rc = -EINVAL);
- }
+ if (rc >= 0)
+ par->mtrr_handle = rc;
}
-#endif /* CONFIG_MTRR */
}
static void uvesafb_ioremap(struct fb_info *info)
{
-#ifdef CONFIG_X86
- switch (mtrr) {
- case 1: /* uncachable */
- info->screen_base = ioremap_nocache(info->fix.smem_start, info->fix.smem_len);
- break;
- case 2: /* write-back */
- info->screen_base = ioremap_cache(info->fix.smem_start, info->fix.smem_len);
- break;
- case 3: /* write-combining */
- info->screen_base = ioremap_wc(info->fix.smem_start, info->fix.smem_len);
- break;
- case 4: /* write-through */
- default:
- info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
- break;
- }
-#else
- info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
-#endif /* CONFIG_X86 */
+ info->screen_base = ioremap_wc(info->fix.smem_start, info->fix.smem_len);
}
static ssize_t uvesafb_show_vbe_ver(struct device *dev,
@@ -1851,6 +1811,7 @@ static int uvesafb_remove(struct platform_device *dev)
unregister_framebuffer(info);
release_region(0x3c0, 32);
iounmap(info->screen_base);
+ arch_phys_wc_del(par->mtrr_handle);
release_mem_region(info->fix.smem_start, info->fix.smem_len);
fb_destroy_modedb(info->monspecs.modedb);
fb_dealloc_cmap(&info->cmap);
@@ -1930,6 +1891,9 @@ static int uvesafb_setup(char *options)
}
}
+ if (mtrr != 3 && mtrr != 1)
+ pr_warn("uvesafb: mtrr should be set to 0 or 3; %d is unsupported", mtrr);
+
return 0;
}
#endif /* !MODULE */
diff --git a/include/video/uvesafb.h b/include/video/uvesafb.h
index 1a91850..30f5362 100644
--- a/include/video/uvesafb.h
+++ b/include/video/uvesafb.h
@@ -134,6 +134,7 @@ struct uvesafb_par {
int mode_idx;
struct vbe_crtc_ib crtc;
+ int mtrr_handle;
};
#endif /* _UVESAFB_H */
--
1.8.1.4
^ permalink raw reply related
* [PATCH v3 6/9] radeon: Switch to arch_phys_wc_add and add a missing ..._del
From: Andy Lutomirski @ 2013-05-13 23:58 UTC (permalink / raw)
To: linux-kernel, dri-devel, linux-fbdev
Cc: Daniel Vetter, Jerome Glisse, Alex Deucher, Dave Airlie,
Andy Lutomirski
In-Reply-To: <cover.1368485053.git.luto@amacapital.net>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
drivers/gpu/drm/radeon/radeon_object.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index d3aface..15cd34b 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -321,8 +321,8 @@ void radeon_bo_force_delete(struct radeon_device *rdev)
int radeon_bo_init(struct radeon_device *rdev)
{
/* Add an MTRR for the VRAM */
- rdev->mc.vram_mtrr = mtrr_add(rdev->mc.aper_base, rdev->mc.aper_size,
- MTRR_TYPE_WRCOMB, 1);
+ rdev->mc.vram_mtrr = arch_phys_wc_add(rdev->mc.aper_base,
+ rdev->mc.aper_size);
DRM_INFO("Detected VRAM RAM=%lluM, BAR=%lluM\n",
rdev->mc.mc_vram_size >> 20,
(unsigned long long)rdev->mc.aper_size >> 20);
@@ -334,6 +334,7 @@ int radeon_bo_init(struct radeon_device *rdev)
void radeon_bo_fini(struct radeon_device *rdev)
{
radeon_ttm_fini(rdev);
+ arch_phys_wc_del(rdev->mc.vram_mtrr);
}
void radeon_bo_list_add_object(struct radeon_bo_list *lobj,
--
1.8.1.4
^ permalink raw reply related
* [PATCH v3 5/9] i915: Use arch_phys_wc_{add,del}
From: Andy Lutomirski @ 2013-05-13 23:58 UTC (permalink / raw)
To: linux-kernel, dri-devel, linux-fbdev
Cc: Daniel Vetter, Jerome Glisse, Alex Deucher, Dave Airlie,
Andy Lutomirski
In-Reply-To: <cover.1368485053.git.luto@amacapital.net>
i915 open-coded logic that was essentially equivalent to the new API.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
Changes from v1: Don't zero the mtrr handle after freeing it
drivers/gpu/drm/i915/i915_dma.c | 42 ++++-------------------------------------
1 file changed, 4 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4fa6beb..b0bb381 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -42,7 +42,6 @@
#include <linux/vga_switcheroo.h>
#include <linux/slab.h>
#include <acpi/video.h>
-#include <asm/pat.h>
#define LP_RING(d) (&((struct drm_i915_private *)(d))->ring[RCS])
@@ -1393,29 +1392,6 @@ void i915_master_destroy(struct drm_device *dev, struct drm_master *master)
master->driver_priv = NULL;
}
-static void
-i915_mtrr_setup(struct drm_i915_private *dev_priv, unsigned long base,
- unsigned long size)
-{
- dev_priv->mm.gtt_mtrr = -1;
-
-#if defined(CONFIG_X86_PAT)
- if (cpu_has_pat)
- return;
-#endif
-
- /* Set up a WC MTRR for non-PAT systems. This is more common than
- * one would think, because the kernel disables PAT on first
- * generation Core chips because WC PAT gets overridden by a UC
- * MTRR if present. Even if a UC MTRR isn't present.
- */
- dev_priv->mm.gtt_mtrr = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
- if (dev_priv->mm.gtt_mtrr < 0) {
- DRM_INFO("MTRR allocation failed. Graphics "
- "performance may suffer.\n");
- }
-}
-
static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
{
struct apertures_struct *ap;
@@ -1552,8 +1528,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
goto out_rmmap;
}
- i915_mtrr_setup(dev_priv, dev_priv->gtt.mappable_base,
- aperture_size);
+ dev_priv->mm.gtt_mtrr = arch_phys_wc_add(dev_priv->gtt.mappable_base,
+ aperture_size);
/* The i915 workqueue is primarily used for batched retirement of
* requests (and thus managing bo) once the task has been completed
@@ -1656,12 +1632,7 @@ out_gem_unload:
intel_teardown_mchbar(dev);
destroy_workqueue(dev_priv->wq);
out_mtrrfree:
- if (dev_priv->mm.gtt_mtrr >= 0) {
- mtrr_del(dev_priv->mm.gtt_mtrr,
- dev_priv->gtt.mappable_base,
- aperture_size);
- dev_priv->mm.gtt_mtrr = -1;
- }
+ arch_phys_wc_del(dev_priv->mm.gtt_mtrr);
io_mapping_free(dev_priv->gtt.mappable);
out_rmmap:
pci_iounmap(dev->pdev, dev_priv->regs);
@@ -1697,12 +1668,7 @@ int i915_driver_unload(struct drm_device *dev)
cancel_delayed_work_sync(&dev_priv->mm.retire_work);
io_mapping_free(dev_priv->gtt.mappable);
- if (dev_priv->mm.gtt_mtrr >= 0) {
- mtrr_del(dev_priv->mm.gtt_mtrr,
- dev_priv->gtt.mappable_base,
- dev_priv->gtt.mappable_end);
- dev_priv->mm.gtt_mtrr = -1;
- }
+ arch_phys_wc_del(dev_priv->mm.gtt_mtrr);
acpi_video_unregister();
--
1.8.1.4
^ permalink raw reply related
* [PATCH v3 4/9] drm,agpgart: Use pgprot_writecombine for AGP maps and make the MTRR optional
From: Andy Lutomirski @ 2013-05-13 23:58 UTC (permalink / raw)
To: linux-kernel, dri-devel, linux-fbdev
Cc: Daniel Vetter, Jerome Glisse, Alex Deucher, Dave Airlie,
Andy Lutomirski
In-Reply-To: <cover.1368485053.git.luto@amacapital.net>
I'm not sure I understand the intent of the previous behavior. mmap
on /dev/agpgart and DRM_AGP maps had no cache flags set, so they
would be fully cacheable. But the DRM code (most of the time) would
add a write-combining MTRR that would change the effective memory
type to WC.
The new behavior just requests WC explicitly for all AGP maps.
If there is any code out there that expects cacheable access to the
AGP aperture (because the drm driver doesn't request an MTRR or
because it's using /dev/agpgart directly), then it will now end up
with a UC or WC mapping, depending on the architecture and PAT
availability. But cacheable access to the aperture seems like it's
asking for trouble, because, AIUI, the aperture is an alias of RAM.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
It's conceivable that libpciaccess could have issues with this due to
memtype conflicts, but I think this is unlikely (as long as a WC
mapping gets there first, I think the conflict resolution rules will
all work out). In any case, everything that maps the AGP aperture
ought to be using /dev/agpgart or the DRM API, in which case
everything should be okay.
I don't have anything with an AGP slot to test AFAIK.
drivers/char/agp/frontend.c | 8 +++++---
drivers/gpu/drm/drm_pci.c | 8 ++++----
drivers/gpu/drm/drm_stub.c | 10 ++--------
drivers/gpu/drm/drm_vm.c | 11 ++++-------
4 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/drivers/char/agp/frontend.c b/drivers/char/agp/frontend.c
index 2e04433..1b19239 100644
--- a/drivers/char/agp/frontend.c
+++ b/drivers/char/agp/frontend.c
@@ -603,7 +603,8 @@ static int agp_mmap(struct file *file, struct vm_area_struct *vma)
vma->vm_ops = kerninfo.vm_ops;
} else if (io_remap_pfn_range(vma, vma->vm_start,
(kerninfo.aper_base + offset) >> PAGE_SHIFT,
- size, vma->vm_page_prot)) {
+ size,
+ pgprot_writecombine(vma->vm_page_prot))) {
goto out_again;
}
mutex_unlock(&(agp_fe.agp_mutex));
@@ -618,8 +619,9 @@ static int agp_mmap(struct file *file, struct vm_area_struct *vma)
if (kerninfo.vm_ops) {
vma->vm_ops = kerninfo.vm_ops;
} else if (io_remap_pfn_range(vma, vma->vm_start,
- kerninfo.aper_base >> PAGE_SHIFT,
- size, vma->vm_page_prot)) {
+ kerninfo.aper_base >> PAGE_SHIFT,
+ size,
+ pgprot_writecombine(vma->vm_page_prot))) {
goto out_again;
}
mutex_unlock(&(agp_fe.agp_mutex));
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index bd719e9..d0f6699 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -278,10 +278,10 @@ int drm_pci_agp_init(struct drm_device *dev)
}
if (drm_core_has_MTRR(dev)) {
if (dev->agp)
- dev->agp->agp_mtrr - mtrr_add(dev->agp->agp_info.aper_base,
- dev->agp->agp_info.aper_size *
- 1024 * 1024, MTRR_TYPE_WRCOMB, 1);
+ dev->agp->agp_mtrr = arch_phys_wc_add(
+ dev->agp->agp_info.aper_base,
+ dev->agp->agp_info.aper_size *
+ 1024 * 1024);
}
}
return 0;
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 7d30802..9e2acdf 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -451,14 +451,8 @@ void drm_put_dev(struct drm_device *dev)
drm_lastclose(dev);
- if (drm_core_has_MTRR(dev) && drm_core_has_AGP(dev) &&
- dev->agp && dev->agp->agp_mtrr >= 0) {
- int retval;
- retval = mtrr_del(dev->agp->agp_mtrr,
- dev->agp->agp_info.aper_base,
- dev->agp->agp_info.aper_size * 1024 * 1024);
- DRM_DEBUG("mtrr_del=%d\n", retval);
- }
+ if (drm_core_has_MTRR(dev) && drm_core_has_AGP(dev) && dev->agp)
+ arch_phys_wc_del(dev->agp->agp_mtrr);
if (dev->driver->unload)
dev->driver->unload(dev);
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index 163f436..b1e4ec8 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -49,13 +49,10 @@ static pgprot_t drm_io_prot(struct drm_local_map *map,
pgprot_t tmp = vm_get_page_prot(vma->vm_flags);
#if defined(__i386__) || defined(__x86_64__)
- if (map->type != _DRM_AGP) {
- if (map->type = _DRM_FRAME_BUFFER ||
- map->flags & _DRM_WRITE_COMBINING)
- tmp = pgprot_writecombine(tmp);
- else
- tmp = pgprot_noncached(tmp);
- }
+ if (map->type = _DRM_REGISTERS && !(map->flags & _DRM_WRITE_COMBINING))
+ tmp = pgprot_noncached(tmp);
+ else
+ tmp = pgprot_writecombine(tmp);
#elif defined(__powerpc__)
pgprot_val(tmp) |= _PAGE_NO_CACHE;
if (map_type = _DRM_REGISTERS)
--
1.8.1.4
^ permalink raw reply related
* [PATCH v3 3/9] drm: Update drm_addmap and drm_mmap to use PAT WC instead of MTRRs
From: Andy Lutomirski @ 2013-05-13 23:58 UTC (permalink / raw)
To: linux-kernel, dri-devel, linux-fbdev
Cc: Daniel Vetter, Jerome Glisse, Alex Deucher, Dave Airlie,
Andy Lutomirski
In-Reply-To: <cover.1368485053.git.luto@amacapital.net>
Previously, DRM_FRAME_BUFFER mappings, as well as DRM_REGISTERS
mappings with DRM_WRITE_COMBINING set, resulted in an unconditional
MTRR being added but the actual mappings being created as UC-.
Now these mappings have the MTRR added only if needed, but they will
be mapped with pgprot_writecombine.
The non-WC DRM_REGISTERS case now uses pgprot_noncached instead of
hardcoding the bit twiddling.
The DRM_AGP case is unchanged for now.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
drivers/gpu/drm/drm_bufs.c | 17 +++++++++--------
drivers/gpu/drm/drm_vm.c | 23 +++++++++++------------
2 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index 0128147..0190fce 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -210,12 +210,16 @@ static int drm_addmap_core(struct drm_device * dev, resource_size_t offset,
if (drm_core_has_MTRR(dev)) {
if (map->type = _DRM_FRAME_BUFFER ||
(map->flags & _DRM_WRITE_COMBINING)) {
- map->mtrr = mtrr_add(map->offset, map->size,
- MTRR_TYPE_WRCOMB, 1);
+ map->mtrr + arch_phys_wc_add(map->offset, map->size);
}
}
if (map->type = _DRM_REGISTERS) {
- map->handle = ioremap(map->offset, map->size);
+ if (map->flags & _DRM_WRITE_COMBINING)
+ map->handle = ioremap_wc(map->offset,
+ map->size);
+ else
+ map->handle = ioremap(map->offset, map->size);
if (!map->handle) {
kfree(map);
return -ENOMEM;
@@ -451,11 +455,8 @@ int drm_rmmap_locked(struct drm_device *dev, struct drm_local_map *map)
iounmap(map->handle);
/* FALLTHROUGH */
case _DRM_FRAME_BUFFER:
- if (drm_core_has_MTRR(dev) && map->mtrr >= 0) {
- int retcode;
- retcode = mtrr_del(map->mtrr, map->offset, map->size);
- DRM_DEBUG("mtrr_del=%d\n", retcode);
- }
+ if (drm_core_has_MTRR(dev))
+ arch_phys_wc_del(map->mtrr);
break;
case _DRM_SHM:
vfree(map->handle);
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index db7bd29..163f436 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -43,14 +43,18 @@
static void drm_vm_open(struct vm_area_struct *vma);
static void drm_vm_close(struct vm_area_struct *vma);
-static pgprot_t drm_io_prot(uint32_t map_type, struct vm_area_struct *vma)
+static pgprot_t drm_io_prot(struct drm_local_map *map,
+ struct vm_area_struct *vma)
{
pgprot_t tmp = vm_get_page_prot(vma->vm_flags);
#if defined(__i386__) || defined(__x86_64__)
- if (boot_cpu_data.x86 > 3 && map_type != _DRM_AGP) {
- pgprot_val(tmp) |= _PAGE_PCD;
- pgprot_val(tmp) &= ~_PAGE_PWT;
+ if (map->type != _DRM_AGP) {
+ if (map->type = _DRM_FRAME_BUFFER ||
+ map->flags & _DRM_WRITE_COMBINING)
+ tmp = pgprot_writecombine(tmp);
+ else
+ tmp = pgprot_noncached(tmp);
}
#elif defined(__powerpc__)
pgprot_val(tmp) |= _PAGE_NO_CACHE;
@@ -250,13 +254,8 @@ static void drm_vm_shm_close(struct vm_area_struct *vma)
switch (map->type) {
case _DRM_REGISTERS:
case _DRM_FRAME_BUFFER:
- if (drm_core_has_MTRR(dev) && map->mtrr >= 0) {
- int retcode;
- retcode = mtrr_del(map->mtrr,
- map->offset,
- map->size);
- DRM_DEBUG("mtrr_del = %d\n", retcode);
- }
+ if (drm_core_has_MTRR(dev))
+ arch_phys_wc_del(map->mtrr);
iounmap(map->handle);
break;
case _DRM_SHM:
@@ -617,7 +616,7 @@ int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma)
case _DRM_REGISTERS:
offset = drm_core_get_reg_ofs(dev);
vma->vm_flags |= VM_IO; /* not in core dump */
- vma->vm_page_prot = drm_io_prot(map->type, vma);
+ vma->vm_page_prot = drm_io_prot(map, vma);
if (io_remap_pfn_range(vma, vma->vm_start,
(map->offset + offset) >> PAGE_SHIFT,
vma->vm_end - vma->vm_start,
--
1.8.1.4
^ permalink raw reply related
* [PATCH v3 2/9] drm (ast,cirrus,mgag200,nouveau,savage,vmwgfx): Remove drm_mtrr_{add,del}
From: Andy Lutomirski @ 2013-05-13 23:58 UTC (permalink / raw)
To: linux-kernel, dri-devel, linux-fbdev
Cc: Daniel Vetter, Jerome Glisse, Alex Deucher, Dave Airlie,
Andy Lutomirski
In-Reply-To: <cover.1368485053.git.luto@amacapital.net>
This replaces drm_mtrr_{add,del} with arch_phys_wc_{add,del}. The
interface is simplified (because the base and size parameters to
drm_mtrr_del never did anything), and it no longer adds MTRRs on
systems that don't need them.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
drivers/gpu/drm/ast/ast_ttm.c | 13 +++--------
drivers/gpu/drm/cirrus/cirrus_ttm.c | 15 ++++--------
drivers/gpu/drm/mgag200/mgag200_ttm.c | 14 ++++--------
drivers/gpu/drm/nouveau/nouveau_ttm.c | 13 ++++-------
drivers/gpu/drm/savage/savage_bci.c | 43 ++++++++++++-----------------------
drivers/gpu/drm/savage/savage_drv.h | 5 +---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 10 ++++----
include/drm/drmP.h | 29 -----------------------
8 files changed, 35 insertions(+), 107 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
index 3602731..c4574fd 100644
--- a/drivers/gpu/drm/ast/ast_ttm.c
+++ b/drivers/gpu/drm/ast/ast_ttm.c
@@ -271,26 +271,19 @@ int ast_mm_init(struct ast_private *ast)
return ret;
}
- ast->fb_mtrr = drm_mtrr_add(pci_resource_start(dev->pdev, 0),
- pci_resource_len(dev->pdev, 0),
- DRM_MTRR_WC);
+ ast->fb_mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 0),
+ pci_resource_len(dev->pdev, 0));
return 0;
}
void ast_mm_fini(struct ast_private *ast)
{
- struct drm_device *dev = ast->dev;
ttm_bo_device_release(&ast->ttm.bdev);
ast_ttm_global_release(ast);
- if (ast->fb_mtrr >= 0) {
- drm_mtrr_del(ast->fb_mtrr,
- pci_resource_start(dev->pdev, 0),
- pci_resource_len(dev->pdev, 0), DRM_MTRR_WC);
- ast->fb_mtrr = -1;
- }
+ arch_phys_wc_del(ast->fb_mtrr);
}
void ast_ttm_placement(struct ast_bo *bo, int domain)
diff --git a/drivers/gpu/drm/cirrus/cirrus_ttm.c b/drivers/gpu/drm/cirrus/cirrus_ttm.c
index 1413a26..09f06d3 100644
--- a/drivers/gpu/drm/cirrus/cirrus_ttm.c
+++ b/drivers/gpu/drm/cirrus/cirrus_ttm.c
@@ -271,9 +271,8 @@ int cirrus_mm_init(struct cirrus_device *cirrus)
return ret;
}
- cirrus->fb_mtrr = drm_mtrr_add(pci_resource_start(dev->pdev, 0),
- pci_resource_len(dev->pdev, 0),
- DRM_MTRR_WC);
+ cirrus->fb_mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 0),
+ pci_resource_len(dev->pdev, 0));
cirrus->mm_inited = true;
return 0;
@@ -281,8 +280,6 @@ int cirrus_mm_init(struct cirrus_device *cirrus)
void cirrus_mm_fini(struct cirrus_device *cirrus)
{
- struct drm_device *dev = cirrus->dev;
-
if (!cirrus->mm_inited)
return;
@@ -290,12 +287,8 @@ void cirrus_mm_fini(struct cirrus_device *cirrus)
cirrus_ttm_global_release(cirrus);
- if (cirrus->fb_mtrr >= 0) {
- drm_mtrr_del(cirrus->fb_mtrr,
- pci_resource_start(dev->pdev, 0),
- pci_resource_len(dev->pdev, 0), DRM_MTRR_WC);
- cirrus->fb_mtrr = -1;
- }
+ arch_phys_wc_del(cirrus->fb_mtrr);
+ cirrus->fb_mtrr = 0;
}
void cirrus_ttm_placement(struct cirrus_bo *bo, int domain)
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c
index 8fc9d92..5c6f3c8 100644
--- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
@@ -270,26 +270,20 @@ int mgag200_mm_init(struct mga_device *mdev)
return ret;
}
- mdev->fb_mtrr = drm_mtrr_add(pci_resource_start(dev->pdev, 0),
- pci_resource_len(dev->pdev, 0),
- DRM_MTRR_WC);
+ mdev->fb_mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 0),
+ pci_resource_len(dev->pdev, 0));
return 0;
}
void mgag200_mm_fini(struct mga_device *mdev)
{
- struct drm_device *dev = mdev->dev;
ttm_bo_device_release(&mdev->ttm.bdev);
mgag200_ttm_global_release(mdev);
- if (mdev->fb_mtrr >= 0) {
- drm_mtrr_del(mdev->fb_mtrr,
- pci_resource_start(dev->pdev, 0),
- pci_resource_len(dev->pdev, 0), DRM_MTRR_WC);
- mdev->fb_mtrr = -1;
- }
+ arch_phys_wc_del(mdev->fb_mtrr);
+ mdev->fb_mtrr = 0;
}
void mgag200_ttm_placement(struct mgag200_bo *bo, int domain)
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index 9be9cb5..63fa6a5 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -377,9 +377,8 @@ nouveau_ttm_init(struct nouveau_drm *drm)
return ret;
}
- drm->ttm.mtrr = drm_mtrr_add(pci_resource_start(dev->pdev, 1),
- pci_resource_len(dev->pdev, 1),
- DRM_MTRR_WC);
+ drm->ttm.mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 1),
+ pci_resource_len(dev->pdev, 1));
/* GART init */
if (drm->agp.stat != ENABLED) {
@@ -414,10 +413,6 @@ nouveau_ttm_fini(struct nouveau_drm *drm)
nouveau_ttm_global_release(drm);
- if (drm->ttm.mtrr >= 0) {
- drm_mtrr_del(drm->ttm.mtrr,
- pci_resource_start(drm->dev->pdev, 1),
- pci_resource_len(drm->dev->pdev, 1), DRM_MTRR_WC);
- drm->ttm.mtrr = -1;
- }
+ arch_phys_wc_del(drm->ttm.mtrr);
+ drm->ttm.mtrr = 0;
}
diff --git a/drivers/gpu/drm/savage/savage_bci.c b/drivers/gpu/drm/savage/savage_bci.c
index b55c1d6..bd6b2cf 100644
--- a/drivers/gpu/drm/savage/savage_bci.c
+++ b/drivers/gpu/drm/savage/savage_bci.c
@@ -570,9 +570,6 @@ int savage_driver_firstopen(struct drm_device *dev)
unsigned int fb_rsrc, aper_rsrc;
int ret = 0;
- dev_priv->mtrr[0].handle = -1;
- dev_priv->mtrr[1].handle = -1;
- dev_priv->mtrr[2].handle = -1;
if (S3_SAVAGE3D_SERIES(dev_priv->chipset)) {
fb_rsrc = 0;
fb_base = pci_resource_start(dev->pdev, 0);
@@ -584,21 +581,14 @@ int savage_driver_firstopen(struct drm_device *dev)
if (pci_resource_len(dev->pdev, 0) = 0x08000000) {
/* Don't make MMIO write-cobining! We need 3
* MTRRs. */
- dev_priv->mtrr[0].base = fb_base;
- dev_priv->mtrr[0].size = 0x01000000;
- dev_priv->mtrr[0].handle - drm_mtrr_add(dev_priv->mtrr[0].base,
- dev_priv->mtrr[0].size, DRM_MTRR_WC);
- dev_priv->mtrr[1].base = fb_base + 0x02000000;
- dev_priv->mtrr[1].size = 0x02000000;
- dev_priv->mtrr[1].handle - drm_mtrr_add(dev_priv->mtrr[1].base,
- dev_priv->mtrr[1].size, DRM_MTRR_WC);
- dev_priv->mtrr[2].base = fb_base + 0x04000000;
- dev_priv->mtrr[2].size = 0x04000000;
- dev_priv->mtrr[2].handle - drm_mtrr_add(dev_priv->mtrr[2].base,
- dev_priv->mtrr[2].size, DRM_MTRR_WC);
+ dev_priv->mtrr_handles[0] + arch_phys_wc_add(fb_base, 0x01000000);
+ dev_priv->mtrr_handles[1] + arch_phys_wc_add(fb_base + 0x02000000,
+ 0x02000000);
+ dev_priv->mtrr_handles[2] + arch_phys_wc_add(fb_base + 0x04000000,
+ 0x04000000);
} else {
DRM_ERROR("strange pci_resource_len %08llx\n",
(unsigned long long)
@@ -616,11 +606,9 @@ int savage_driver_firstopen(struct drm_device *dev)
if (pci_resource_len(dev->pdev, 1) = 0x08000000) {
/* Can use one MTRR to cover both fb and
* aperture. */
- dev_priv->mtrr[0].base = fb_base;
- dev_priv->mtrr[0].size = 0x08000000;
- dev_priv->mtrr[0].handle - drm_mtrr_add(dev_priv->mtrr[0].base,
- dev_priv->mtrr[0].size, DRM_MTRR_WC);
+ dev_priv->mtrr_handles[0] + arch_phys_wc_add(fb_base,
+ 0x08000000);
} else {
DRM_ERROR("strange pci_resource_len %08llx\n",
(unsigned long long)
@@ -660,11 +648,10 @@ void savage_driver_lastclose(struct drm_device *dev)
drm_savage_private_t *dev_priv = dev->dev_private;
int i;
- for (i = 0; i < 3; ++i)
- if (dev_priv->mtrr[i].handle >= 0)
- drm_mtrr_del(dev_priv->mtrr[i].handle,
- dev_priv->mtrr[i].base,
- dev_priv->mtrr[i].size, DRM_MTRR_WC);
+ for (i = 0; i < 3; ++i) {
+ arch_phys_wc_del(dev_priv->mtrr_handles[i]);
+ dev_priv->mtrr_handles[i] = 0;
+ }
}
int savage_driver_unload(struct drm_device *dev)
diff --git a/drivers/gpu/drm/savage/savage_drv.h b/drivers/gpu/drm/savage/savage_drv.h
index df2aac6..c05082a 100644
--- a/drivers/gpu/drm/savage/savage_drv.h
+++ b/drivers/gpu/drm/savage/savage_drv.h
@@ -160,10 +160,7 @@ typedef struct drm_savage_private {
drm_local_map_t *cmd_dma;
drm_local_map_t fake_dma;
- struct {
- int handle;
- unsigned long base, size;
- } mtrr[3];
+ int mtrr_handles[3];
/* BCI and status-related stuff */
volatile uint32_t *status_ptr, *bci_ptr;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 07dfd82..78e2164 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -565,8 +565,8 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
dev_priv->has_gmr = false;
}
- dev_priv->mmio_mtrr = drm_mtrr_add(dev_priv->mmio_start,
- dev_priv->mmio_size, DRM_MTRR_WC);
+ dev_priv->mmio_mtrr = arch_phys_wc_add(dev_priv->mmio_start,
+ dev_priv->mmio_size);
dev_priv->mmio_virt = ioremap_wc(dev_priv->mmio_start,
dev_priv->mmio_size);
@@ -664,8 +664,7 @@ out_no_device:
out_err4:
iounmap(dev_priv->mmio_virt);
out_err3:
- drm_mtrr_del(dev_priv->mmio_mtrr, dev_priv->mmio_start,
- dev_priv->mmio_size, DRM_MTRR_WC);
+ arch_phys_wc_del(dev_priv->mmio_mtrr);
if (dev_priv->has_gmr)
(void) ttm_bo_clean_mm(&dev_priv->bdev, VMW_PL_GMR);
(void)ttm_bo_clean_mm(&dev_priv->bdev, TTM_PL_VRAM);
@@ -709,8 +708,7 @@ static int vmw_driver_unload(struct drm_device *dev)
ttm_object_device_release(&dev_priv->tdev);
iounmap(dev_priv->mmio_virt);
- drm_mtrr_del(dev_priv->mmio_mtrr, dev_priv->mmio_start,
- dev_priv->mmio_size, DRM_MTRR_WC);
+ arch_phys_wc_del(dev_priv->mmio_mtrr);
if (dev_priv->has_gmr)
(void)ttm_bo_clean_mm(&dev_priv->bdev, VMW_PL_GMR);
(void)ttm_bo_clean_mm(&dev_priv->bdev, TTM_PL_VRAM);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 2d94d74..3e6cfa0 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1249,37 +1249,8 @@ static inline int drm_core_has_MTRR(struct drm_device *dev)
{
return drm_core_check_feature(dev, DRIVER_USE_MTRR);
}
-
-#define DRM_MTRR_WC MTRR_TYPE_WRCOMB
-
-static inline int drm_mtrr_add(unsigned long offset, unsigned long size,
- unsigned int flags)
-{
- return mtrr_add(offset, size, flags, 1);
-}
-
-static inline int drm_mtrr_del(int handle, unsigned long offset,
- unsigned long size, unsigned int flags)
-{
- return mtrr_del(handle, offset, size);
-}
-
#else
#define drm_core_has_MTRR(dev) (0)
-
-#define DRM_MTRR_WC 0
-
-static inline int drm_mtrr_add(unsigned long offset, unsigned long size,
- unsigned int flags)
-{
- return 0;
-}
-
-static inline int drm_mtrr_del(int handle, unsigned long offset,
- unsigned long size, unsigned int flags)
-{
- return 0;
-}
#endif
static inline void drm_device_set_unplugged(struct drm_device *dev)
--
1.8.1.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox