* Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework
From: Maarten Lankhorst @ 2013-06-17 13:31 UTC (permalink / raw)
To: Inki Dae
Cc: dri-devel, linux-fbdev, linux-arm-kernel, linux-media, daniel,
robdclark, kyungmin.park, myungjoo.ham, yj44.cho
In-Reply-To: <012501ce6b5b$3d39b0b0$b7ad1210$%dae@samsung.com>
Op 17-06-13 15:04, Inki Dae schreef:
>
>> -----Original Message-----
>> From: Maarten Lankhorst [mailto:maarten.lankhorst@canonical.com]
>> Sent: Monday, June 17, 2013 8:35 PM
>> To: Inki Dae
>> Cc: dri-devel@lists.freedesktop.org; linux-fbdev@vger.kernel.org; linux-
>> arm-kernel@lists.infradead.org; linux-media@vger.kernel.org;
>> daniel@ffwll.ch; robdclark@gmail.com; kyungmin.park@samsung.com;
>> myungjoo.ham@samsung.com; yj44.cho@samsung.com
>> Subject: Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization
>> framework
>>
>> Op 17-06-13 13:15, Inki Dae schreef:
>>> This patch adds a buffer synchronization framework based on DMA BUF[1]
>>> and reservation[2] to use dma-buf resource, and based on ww-mutexes[3]
>>> for lock mechanism.
>>>
>>> The purpose of this framework is not only to couple cache operations,
>>> and buffer access control to CPU and DMA but also to provide easy-to-use
>>> interfaces for device drivers and potentially user application
>>> (not implemented for user applications, yet). And this framework can be
>>> used for all dma devices using system memory as dma buffer, especially
>>> for most ARM based SoCs.
>>>
>>> Changelog v2:
>>> - use atomic_add_unless to avoid potential bug.
>>> - add a macro for checking valid access type.
>>> - code clean.
>>>
>>> The mechanism of this framework has the following steps,
>>> 1. Register dmabufs to a sync object - A task gets a new sync object
>> and
>>> can add one or more dmabufs that the task wants to access.
>>> This registering should be performed when a device context or an
>> event
>>> context such as a page flip event is created or before CPU accesses
> a
>> shared
>>> buffer.
>>>
>>> dma_buf_sync_get(a sync object, a dmabuf);
>>>
>>> 2. Lock a sync object - A task tries to lock all dmabufs added in
> its
>> own
>>> sync object. Basically, the lock mechanism uses ww-mutex[1] to avoid
>> dead
>>> lock issue and for race condition between CPU and CPU, CPU and DMA,
>> and DMA
>>> and DMA. Taking a lock means that others cannot access all locked
>> dmabufs
>>> until the task that locked the corresponding dmabufs, unlocks all
> the
>> locked
>>> dmabufs.
>>> This locking should be performed before DMA or CPU accesses these
>> dmabufs.
>>> dma_buf_sync_lock(a sync object);
>>>
>>> 3. Unlock a sync object - The task unlocks all dmabufs added in its
>> own sync
>>> object. The unlock means that the DMA or CPU accesses to the dmabufs
>> have
>>> been completed so that others may access them.
>>> This unlocking should be performed after DMA or CPU has completed
>> accesses
>>> to the dmabufs.
>>>
>>> dma_buf_sync_unlock(a sync object);
>>>
>>> 4. Unregister one or all dmabufs from a sync object - A task
>> unregisters
>>> the given dmabufs from the sync object. This means that the task
>> dosen't
>>> want to lock the dmabufs.
>>> The unregistering should be performed after DMA or CPU has completed
>>> accesses to the dmabufs or when dma_buf_sync_lock() is failed.
>>>
>>> dma_buf_sync_put(a sync object, a dmabuf);
>>> dma_buf_sync_put_all(a sync object);
>>>
>>> The described steps may be summarized as:
>>> get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put
>>>
>>> This framework includes the following two features.
>>> 1. read (shared) and write (exclusive) locks - A task is required to
>> declare
>>> the access type when the task tries to register a dmabuf;
>>> READ, WRITE, READ DMA, or WRITE DMA.
>>>
>>> The below is example codes,
>>> struct dmabuf_sync *sync;
>>>
>>> sync = dmabuf_sync_init(NULL, "test sync");
>>>
>>> dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
>>> ...
>>>
>>> And the below can be used as access types:
>>> DMA_BUF_ACCESS_READ,
>>> - CPU will access a buffer for read.
>>> DMA_BUF_ACCESS_WRITE,
>>> - CPU will access a buffer for read or write.
>>> DMA_BUF_ACCESS_READ | DMA_BUF_ACCESS_DMA,
>>> - DMA will access a buffer for read
>>> DMA_BUF_ACCESS_WRITE | DMA_BUF_ACCESS_DMA,
>>> - DMA will access a buffer for read or write.
>>>
>>> 2. Mandatory resource releasing - a task cannot hold a lock
>> indefinitely.
>>> A task may never try to unlock a buffer after taking a lock to the
>> buffer.
>>> In this case, a timer handler to the corresponding sync object is
>> called
>>> in five (default) seconds and then the timed-out buffer is unlocked
>> by work
>>> queue handler to avoid lockups and to enforce resources of the
> buffer.
>>> The below is how to use:
>>> 1. Allocate and Initialize a sync object:
>>> struct dmabuf_sync *sync;
>>>
>>> sync = dmabuf_sync_init(NULL, "test sync");
>>> ...
>>>
>>> 2. Add a dmabuf to the sync object when setting up dma buffer
>> relevant
>>> registers:
>>> dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
>>> ...
>>>
>>> 3. Lock all dmabufs of the sync object before DMA or CPU accesses
>>> the dmabufs:
>>> dmabuf_sync_lock(sync);
>>> ...
>>>
>>> 4. Now CPU or DMA can access all dmabufs locked in step 3.
>>>
>>> 5. Unlock all dmabufs added in a sync object after DMA or CPU
>> access
>>> to these dmabufs is completed:
>>> dmabuf_sync_unlock(sync);
>>>
>>> And call the following functions to release all resources,
>>> dmabuf_sync_put_all(sync);
>>> dmabuf_sync_fini(sync);
>>>
>>> You can refer to actual example codes:
>>> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
>> exynos.git/
>>> commit/?h=dmabuf-
>> sync&id@30bdee9bab5841ad32faade528d04cc0c5fc94
>>> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
>> exynos.git/
>>> commit/?h=dmabuf-
>> sync&idla548e9ea9e865592719ef6b1cde58366af9f5c
>>> The framework performs cache operation based on the previous and current
>> access
>>> types to the dmabufs after the locks to all dmabufs are taken:
>>> Call dma_buf_begin_cpu_access() to invalidate cache if,
>>> previous access type is DMA_BUF_ACCESS_WRITE | DMA and
>>> current access type is DMA_BUF_ACCESS_READ
>>>
>>> Call dma_buf_end_cpu_access() to clean cache if,
>>> previous access type is DMA_BUF_ACCESS_WRITE and
>>> current access type is DMA_BUF_ACCESS_READ | DMA
>>>
>>> Such cache operations are invoked via dma-buf interfaces so the dma buf
>> exporter
>>> should implement dmabuf->ops->begin_cpu_access/end_cpu_access callbacks.
>>>
>>> [1] http://lwn.net/Articles/470339/
>>> [2] http://lwn.net/Articles/532616/
>>> [3] https://patchwork-mail1.kernel.org/patch/2625321/
>>>
>> Looks to me like you're just writing an api similar to the android
>> syncpoint for this.
>> Is there any reason you had to reimplement the android syncpoint api?
> Right, only difference is that maybe android sync driver, you mentioned as
> syncpoint, doesn't use dma-buf resource. What I try to do is familiar to
> android's one and also ARM's KDS (Kernel Dependency System). I think I
> already mentioned enough through a document file about why I try to
> implement this approach based on dma-buf; coupling cache operation and
> buffer synchronization between CPU and DMA.
Making sure caching works correctly can be handled entirely in the begin_cpu_access/end_cpu_access callbacks, without requiring such.. framework
>> I'm not going into a full review, you may wish to rethink the design
> first.
>> All the criticisms I had with the original design approach still apply.
>>
> Isn't that enough if what I try to do is similar to android sync driver?
> It's very simple and that's all I try to do.:)
From what I can tell you should be able to make it work with just the use of fences, you don't need to keep the buffers locked for the entire duration,
just plug in the last fence in the correct slots and you're done..
I'm not sure if the android sync api is compatible enough, but you shouldn't need to write code in this way to make it work..
>>
>> A few things that stand out from a casual glance:
>>
>> bool is_dmabuf_sync_supported(void)
>> -> any code that needs CONFIG_DMABUF_SYNC should select it in their
>> kconfig
>> runtime enabling/disabling of synchronization is a recipe for disaster,
>> remove the !CONFIG_DMABUF_SYNC fallbacks.
>> NEVER add a runtime way to influence locking behavior.
>>
> Not enabling/disabling synchronization feature in runtime. That is
> determined at build time.
>
>> Considering you're also holding dmaobj->lock for the entire duration, is
>> there any point to not simply call begin_cpu_access/end_cpu_access, and
>> forget this ugly code ever existed?
> You mean mutex_lock(&sync->lock)? Yeah, it seems unnecessary in that case.
>
>> I still don't see the problem you're trying to solve..
>>
> It's just to implement a thin sync framework coupling cache operation. This
> approach is based on dma-buf for more generic implementation against android
> sync driver or KDS.
>
> The described steps may be summarized as:
> lock -> cache operation -> CPU or DMA access to a buffer/s -> unlock
>
> I think that there is no need to get complicated for such approach at least
> for most devices sharing system memory. Simple is best.
>
>
> Thanks,
> Inki Dae
>
>> ~Maarten
^ permalink raw reply
* [PATCH] video: of_display_timing.h: Include <video/display_timing.h>
From: Fabio Estevam @ 2013-06-17 13:43 UTC (permalink / raw)
To: linux-fbdev
Commit ffa3fd21de ("videomode: implement public of_get_display_timing()") causes
the following build warning:
include/video/of_display_timing.h:18:10: warning: 'struct display_timing' declared inside parameter list [enabled by default]
include/video/of_display_timing.h:18:10: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]
As 'struct display_timing' is defined at <video/display_timing.h>, let's include
this header to avoid the warning.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
include/video/of_display_timing.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/video/of_display_timing.h b/include/video/of_display_timing.h
index 6562ad9..a136f58 100644
--- a/include/video/of_display_timing.h
+++ b/include/video/of_display_timing.h
@@ -8,6 +8,7 @@
#ifndef __LINUX_OF_DISPLAY_TIMING_H
#define __LINUX_OF_DISPLAY_TIMING_H
+#include <video/display_timing.h>
struct device_node;
struct display_timings;
--
1.8.1.2
^ permalink raw reply related
* [GIT PULL] omapdss changes for 3.11, part 1/2
From: Tomi Valkeinen @ 2013-06-17 13:46 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: linux-fbdev, linux-omap
[-- Attachment #1: Type: text/plain, Size: 5893 bytes --]
The following changes since commit 317ddd256b9c24b0d78fa8018f80f1e495481a10:
Linux 3.10-rc5 (2013-06-08 17:41:04 -0700)
are available in the git repository at:
git://gitorious.org/linux-omap-dss2/linux.git tags/omapdss-for-3.11-1
for you to fetch changes up to 595470a7853848cb971d5ee3fed443b1e3aa0d1b:
OMAPDSS: gracefully disable overlay at error (2013-06-17 14:00:56 +0300)
----------------------------------------------------------------
OMAP display subsystem changes for 3.11 (part 1/2)
This is the first part of OMAP DSS changes for 3.11. This part contains fixes,
cleanups and reorganizations that are not directly related to the new DSS
device model that is added in part 2, although many of the reorganizations are
made to make the part 2 possible.
There should not be any functional changes visible to the user except the few
bug fixes.
The main new internal features:
- Display (dis)connect support, which allows us to explicitly (dis)connect a
whole display pipeline
- Panel list, which allows us to operate without the specific DSS bus
- Combine omap_dss_output to omap_dss_device, so that we have one generic
"entity" for display pipeline blocks
----------------------------------------------------------------
Emil Goode (1):
OMAPDSS: Remove kfree for memory allocated with devm_kzalloc
Sergey Kibrik (1):
OMAPDSS: gracefully disable overlay at error
Tomi Valkeinen (36):
OMAPDSS: add pdata->default_display_name
OMAPDSS: only probe pdata if there's one
OMAPDSS: add omap_dss_find_output()
OMAPDSS: add omap_dss_find_output_by_node()
OMAPDSS: fix dss_get_ctx_loss_count for DT
OMAPDSS: clean up dss_[ovl|mgr]_get_device()
OMAPDSS: add helpers to get mgr or output from display
OMAPDSS: split overlay manager creation
OMAPDRM: fix overlay manager handling
OMAPDSS: Implement display (dis)connect support
OMAPDSS: CORE: use devm_regulator_get
OMAPDSS: DSI: cleanup regulator init
OMAPDSS: DPI: cleanup pll & regulator init
OMAPDSS: DPI: fix regulators for DT
OMAPDSS: HDMI: add hdmi_init_regulator
OMAPDSS: SDI: clean up regulator init
OMAPDSS: SDI: fix regulators for DT
OMAPDSS: VENC: clean up regulator init
OMAPDSS: add videomode conversion support
OMAPDSS: remove dssdev uses in trivial cases
OMAPDSS: add panel list
OMAPDSS: use the panel list in omap_dss_get_next_device
OMAPDSS: don't use dss bus in suspend/resume
OMAPDSS: implement display sysfs without dss bus
OMAPDSS: Add panel dev pointer to dssdev
OMAPDSS: remove omap_dss_start/stop_device()
OMAPDSS: combine omap_dss_output into omap_dss_device
OMAPDSS: omapdss.h: add owner field to omap_dss_device
OMAPDSS: add module_get/put to omap_dss_get/put_device()
OMAPDSS: add THIS_MODULE owner to DSS outputs
OMAPDSS: output: increase refcount in find_output funcs
OMAPFB: use EPROBE_DEFER if default display is not present
OMAPDSS: HDMI: clean up PHY power handling
OMAPDSS: HDMI clean up hpd_gpio
OMAPDSS: remove unused fields in omap_dss_device
OMAPDSS: remove dispc's dependency to VENC/HDMI
drivers/gpu/drm/omapdrm/omap_crtc.c | 46 +++-
drivers/gpu/drm/omapdrm/omap_drv.c | 21 +-
drivers/gpu/drm/omapdrm/omap_drv.h | 1 +
drivers/video/omap2/displays/panel-acx565akm.c | 16 +-
drivers/video/omap2/displays/panel-generic-dpi.c | 26 +--
.../omap2/displays/panel-lgphilips-lb035q02.c | 10 +-
drivers/video/omap2/displays/panel-n8x0.c | 30 +--
.../omap2/displays/panel-nec-nl8048hl11-01b.c | 4 +-
drivers/video/omap2/displays/panel-picodlp.c | 34 ++-
.../video/omap2/displays/panel-sharp-ls037v7dw01.c | 10 +-
drivers/video/omap2/displays/panel-taal.c | 164 +++++++-------
drivers/video/omap2/displays/panel-tfp410.c | 32 +--
.../video/omap2/displays/panel-tpo-td043mtea1.c | 36 +--
drivers/video/omap2/dss/Kconfig | 1 +
drivers/video/omap2/dss/apply.c | 47 ++--
drivers/video/omap2/dss/core.c | 108 +++++----
drivers/video/omap2/dss/dispc-compat.c | 3 +-
drivers/video/omap2/dss/dispc.c | 24 +-
drivers/video/omap2/dss/display-sysfs.c | 154 +++++++------
drivers/video/omap2/dss/display.c | 246 ++++++++++++++-------
drivers/video/omap2/dss/dpi.c | 131 +++++------
drivers/video/omap2/dss/dsi.c | 135 +++++------
drivers/video/omap2/dss/dss.c | 3 +-
drivers/video/omap2/dss/dss.h | 35 +--
drivers/video/omap2/dss/dss_features.c | 1 -
drivers/video/omap2/dss/hdmi.c | 107 ++++-----
drivers/video/omap2/dss/manager-sysfs.c | 47 ++--
drivers/video/omap2/dss/manager.c | 29 ++-
drivers/video/omap2/dss/output.c | 78 ++++++-
drivers/video/omap2/dss/rfbi.c | 39 ++--
drivers/video/omap2/dss/sdi.c | 61 ++---
drivers/video/omap2/dss/ti_hdmi.h | 5 +-
drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 87 ++++----
drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h | 1 +
drivers/video/omap2/dss/venc.c | 87 +++-----
drivers/video/omap2/dss/venc_panel.c | 16 +-
drivers/video/omap2/omapfb/omapfb-ioctl.c | 9 +-
drivers/video/omap2/omapfb/omapfb-main.c | 27 +--
include/video/omapdss.h | 113 ++++++----
39 files changed, 1131 insertions(+), 893 deletions(-)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* [GIT PULL] omapdss changes for 3.11, part 2/2
From: Tomi Valkeinen @ 2013-06-17 13:47 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: linux-fbdev, linux-omap
In-Reply-To: <51BF1326.7010206@ti.com>
[-- Attachment #1: Type: text/plain, Size: 6017 bytes --]
The following changes since commit 595470a7853848cb971d5ee3fed443b1e3aa0d1b:
OMAPDSS: gracefully disable overlay at error (2013-06-17 14:00:56 +0300)
are available in the git repository at:
git://gitorious.org/linux-omap-dss2/linux.git tags/omapdss-for-3.11-2
for you to fetch changes up to c545b59515cca4ccaf920e12582a43836cddaa2b:
OMAPDSS: panels: add Kconfig comment (2013-06-17 14:33:21 +0300)
----------------------------------------------------------------
OMAP display subsystem changes for 3.11 (part 2/2)
This is the second part of OMAP DSS changes for 3.11. This part contains the
new DSS device model support.
The current OMAP panel drivers use a custom DSS bus, and there's a hard limit
of one external display block per video pipeline. In the new DSS device model
the devices/drivers are made according to the control bus of the display block,
usually platform, i2c or spi. The display blocks can also be chained so that we
can have separate drivers for setups with both external encoder and panel.
To allow the current board files, which use the old style panels, to function,
the old display drivers are left in their current state, and new ones are added
to drivers/video/omap2/displays-new/. When the board files have been converted
to use the new style panels, we can remove the old code. This is planned to
happen in v3.12.
Having to support two very different DSS device models makes the driver
somewhat confusing in some parts, and prevents us from properly cleaning up
some other parts. These cleanups will be done when the old code is removed.
The new device model is designed with CDF (Common Display Framework) in mind.
While CDF is still under work, the new DSS device model should be much more
similar to CDF's model than the old device model, which should make the
eventual conversion to CDF much easier.
----------------------------------------------------------------
Tomi Valkeinen (23):
OMAPDSS: public omapdss_register_output()
OMAPDSS: modify get/find functions to go through the device chain
OMAPDSS: add OMAP_DISPLAY_TYPE_DVI
drm/omap: DVI connector fix
OMAPDSS: DPI: Add ops
OMAPDSS: SDI: Add ops
OMAPDSS: DVI: Add ops
OMAPDSS: AnalogTV: Add ops
OMAPDSS: HDMI: Add ops
OMAPDSS: DSI: Add ops
OMAPDSS: Add new TFP410 Encoder driver
OMAPDSS: Add new TPD12S015 Encoder driver
OMAPDSS: Add new DVI Connector driver
OMAPDSS: Add new HDMI Connector driver
OMAPDSS: Add new Analog TV Connector driver
OMAPDSS: Add new simple DPI panel driver
OMAPDSS: Add new DSI Command Mode panel driver
OMAPDSS: Add Sony ACX565AKM panel driver
OMAPDSS: Add LG.Philips LB035Q02 panel driver
OMAPDSS: Add Sharp LS037V7DW01 panel driver
OMAPDSS: Add TPO TD043MTEA1 panel driver
OMAPDSS: Add NEC NL8048HL11 panel driver
OMAPDSS: panels: add Kconfig comment
drivers/gpu/drm/omapdrm/omap_drv.c | 6 +-
drivers/video/omap2/Kconfig | 1 +
drivers/video/omap2/Makefile | 1 +
drivers/video/omap2/displays-new/Kconfig | 73 ++
drivers/video/omap2/displays-new/Makefile | 12 +
.../video/omap2/displays-new/connector-analog-tv.c | 265 ++++
drivers/video/omap2/displays-new/connector-dvi.c | 351 +++++
drivers/video/omap2/displays-new/connector-hdmi.c | 375 ++++++
drivers/video/omap2/displays-new/encoder-tfp410.c | 267 ++++
.../video/omap2/displays-new/encoder-tpd12s015.c | 395 ++++++
drivers/video/omap2/displays-new/panel-dpi.c | 270 ++++
drivers/video/omap2/displays-new/panel-dsi-cm.c | 1336 ++++++++++++++++++++
.../omap2/displays-new/panel-lgphilips-lb035q02.c | 358 ++++++
.../omap2/displays-new/panel-nec-nl8048hl11.c | 394 ++++++
.../omap2/displays-new/panel-sharp-ls037v7dw01.c | 324 +++++
.../omap2/displays-new/panel-sony-acx565akm.c | 865 +++++++++++++
.../omap2/displays-new/panel-tpo-td043mtea1.c | 646 ++++++++++
drivers/video/omap2/displays/Kconfig | 2 +-
drivers/video/omap2/dss/apply.c | 14 +-
drivers/video/omap2/dss/display.c | 1 +
drivers/video/omap2/dss/dpi.c | 74 +-
drivers/video/omap2/dss/dsi.c | 97 +-
drivers/video/omap2/dss/dss.h | 4 -
drivers/video/omap2/dss/hdmi.c | 238 +++-
drivers/video/omap2/dss/output.c | 15 +-
drivers/video/omap2/dss/rfbi.c | 4 +-
drivers/video/omap2/dss/sdi.c | 82 +-
drivers/video/omap2/dss/venc.c | 76 +-
include/video/omap-panel-data.h | 209 +++
include/video/omapdss.h | 192 ++-
30 files changed, 6911 insertions(+), 36 deletions(-)
create mode 100644 drivers/video/omap2/displays-new/Kconfig
create mode 100644 drivers/video/omap2/displays-new/Makefile
create mode 100644 drivers/video/omap2/displays-new/connector-analog-tv.c
create mode 100644 drivers/video/omap2/displays-new/connector-dvi.c
create mode 100644 drivers/video/omap2/displays-new/connector-hdmi.c
create mode 100644 drivers/video/omap2/displays-new/encoder-tfp410.c
create mode 100644 drivers/video/omap2/displays-new/encoder-tpd12s015.c
create mode 100644 drivers/video/omap2/displays-new/panel-dpi.c
create mode 100644 drivers/video/omap2/displays-new/panel-dsi-cm.c
create mode 100644 drivers/video/omap2/displays-new/panel-lgphilips-lb035q02.c
create mode 100644 drivers/video/omap2/displays-new/panel-nec-nl8048hl11.c
create mode 100644 drivers/video/omap2/displays-new/panel-sharp-ls037v7dw01.c
create mode 100644 drivers/video/omap2/displays-new/panel-sony-acx565akm.c
create mode 100644 drivers/video/omap2/displays-new/panel-tpo-td043mtea1.c
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework
From: Russell King - ARM Linux @ 2013-06-17 15:42 UTC (permalink / raw)
To: Inki Dae
Cc: Maarten Lankhorst, linux-fbdev, Kyungmin Park, DRI mailing list,
Rob Clark, myungjoo.ham, YoungJun Cho, Daniel Vetter,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <CAAQKjZO_t_kZkU46bUPTpoJs_oE1KkEqS2OTrTYjjJYZzBf+XA@mail.gmail.com>
On Tue, Jun 18, 2013 at 12:03:31AM +0900, Inki Dae wrote:
> 2013/6/17 Russell King - ARM Linux <linux@arm.linux.org.uk>
> Exactly right. But that is not definitely my point. Could you please see
> the below simple example?:
> (Presume that CPU and DMA share a buffer and the buffer is mapped with user
> space as cachable)
>
> handle1 = drm_gem_fd_to_handle(a dmabuf fd); ----> 1
> ...
> va1 = drm_gem_mmap(handle1);
> va2 = drm_gem_mmap(handle2);
> va3 = malloc(size);
> ...
>
> while (conditions) {
> memcpy(va1, some data, size);
Nooooooooooooooooooooooooooooooooooooooooooooo!
Well, the first thing to say here is that under the requirements of the
DMA API, the above is immediately invalid, because you're writing to a
buffer which under the terms of the DMA API is currently owned by the
DMA agent, *not* by the CPU. You're supposed to call dma_sync_sg_for_cpu()
before you do that - but how is userspace supposed to know that requirement?
Why should userspace even _have_ to know these requirements of the DMA
API?
It's also entirely possible that drm_gem_fd_to_handle() (which indirectly
causes dma_map_sg() on the buffers scatterlist) followed by mmap'ing it
into userspace is a bug too, as it has the potential to touch caches or
stuff in ways that maybe the DMA or IOMMU may not expect - but I'm not
going to make too big a deal about that, because I don't think we have
anything that picky.
However, the first point above is the most important one, and exposing
the quirks of the DMA API to userland is certainly not a nice thing to be
doing. This needs to be fixed - we can't go and enforce an API which is
deeply embedded within the kernel all the way out to userland.
What we need is something along the lines of:
(a) dma_buf_map_attachment() _not_ to map the scatterlist for DMA.
or
(b) drm_gem_prime_import() not to call dma_buf_map_attachment() at all.
and for the scatterlist to be mapped for DMA at the point where the DMA
operation is initiated, and unmapped at the point where the DMA operation
is complete.
So no, the problem is not that we need more APIs and code - we need the
existing kernel API fixed so that we don't go exposing userspace to the
requirements of the DMA API. Unless we do that, we're going to end
up with a huge world of pain, where kernel architecture people need to
audit every damned DRM userspace implementation that happens to be run
on their platform, and that's not something arch people really can
afford to do.
Basically, I think the dma_buf stuff needs to be rewritten with the
requirements of the DMA API in the forefront of whosever mind is doing
the rewriting.
Note: the existing stuff does have the nice side effect of being able
to pass buffers which do not have a struct page * associated with them
through the dma_buf API - I think we can still preserve that by having
dma_buf provide a couple of new APIs to do the SG list map/sync/unmap,
but in any case we need to fix the existing API so that:
dma_buf_map_attachment() becomes dma_buf_get_sg()
dma_buf_unmap_attachment() becomes dma_buf_put_sg()
both getting rid of the DMA direction argument, and then we have four
new dma_buf calls:
dma_buf_map_sg()
dma_buf_unmap_sg()
dma_buf_sync_sg_for_cpu()
dma_buf_sync_sg_for_device()
which do the actual sg map/unmap via the DMA API *at the appropriate
time for DMA*.
So, the summary of this is - at the moment, I regard DRM Prime and dmabuf
to be utterly broken in design for architectures such as ARM where the
requirements of the DMA API have to be followed if you're going to have
a happy life.
^ permalink raw reply
* Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework
From: Russell King - ARM Linux @ 2013-06-17 16:01 UTC (permalink / raw)
To: Inki Dae
Cc: linux-fbdev, Kyungmin Park, DRI mailing list, Rob Clark,
myungjoo.ham, YoungJun Cho, Daniel Vetter, Maarten Lankhorst,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <20130617154237.GJ2718@n2100.arm.linux.org.uk>
On Mon, Jun 17, 2013 at 04:42:37PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 18, 2013 at 12:03:31AM +0900, Inki Dae wrote:
> > 2013/6/17 Russell King - ARM Linux <linux@arm.linux.org.uk>
> > Exactly right. But that is not definitely my point. Could you please see
> > the below simple example?:
> > (Presume that CPU and DMA share a buffer and the buffer is mapped with user
> > space as cachable)
> >
> > handle1 = drm_gem_fd_to_handle(a dmabuf fd); ----> 1
> > ...
> > va1 = drm_gem_mmap(handle1);
> > va2 = drm_gem_mmap(handle2);
> > va3 = malloc(size);
> > ...
> >
> > while (conditions) {
> > memcpy(va1, some data, size);
>
> Nooooooooooooooooooooooooooooooooooooooooooooo!
>
> Well, the first thing to say here is that under the requirements of the
> DMA API, the above is immediately invalid, because you're writing to a
> buffer which under the terms of the DMA API is currently owned by the
> DMA agent, *not* by the CPU. You're supposed to call dma_sync_sg_for_cpu()
> before you do that - but how is userspace supposed to know that requirement?
> Why should userspace even _have_ to know these requirements of the DMA
> API?
>
> It's also entirely possible that drm_gem_fd_to_handle() (which indirectly
> causes dma_map_sg() on the buffers scatterlist) followed by mmap'ing it
> into userspace is a bug too, as it has the potential to touch caches or
> stuff in ways that maybe the DMA or IOMMU may not expect - but I'm not
> going to make too big a deal about that, because I don't think we have
> anything that picky.
>
> However, the first point above is the most important one, and exposing
> the quirks of the DMA API to userland is certainly not a nice thing to be
> doing. This needs to be fixed - we can't go and enforce an API which is
> deeply embedded within the kernel all the way out to userland.
>
> What we need is something along the lines of:
> (a) dma_buf_map_attachment() _not_ to map the scatterlist for DMA.
> or
> (b) drm_gem_prime_import() not to call dma_buf_map_attachment() at all.
>
> and for the scatterlist to be mapped for DMA at the point where the DMA
> operation is initiated, and unmapped at the point where the DMA operation
> is complete.
>
> So no, the problem is not that we need more APIs and code - we need the
> existing kernel API fixed so that we don't go exposing userspace to the
> requirements of the DMA API. Unless we do that, we're going to end
> up with a huge world of pain, where kernel architecture people need to
> audit every damned DRM userspace implementation that happens to be run
> on their platform, and that's not something arch people really can
> afford to do.
>
> Basically, I think the dma_buf stuff needs to be rewritten with the
> requirements of the DMA API in the forefront of whosever mind is doing
> the rewriting.
>
> Note: the existing stuff does have the nice side effect of being able
> to pass buffers which do not have a struct page * associated with them
> through the dma_buf API - I think we can still preserve that by having
> dma_buf provide a couple of new APIs to do the SG list map/sync/unmap,
> but in any case we need to fix the existing API so that:
>
> dma_buf_map_attachment() becomes dma_buf_get_sg()
> dma_buf_unmap_attachment() becomes dma_buf_put_sg()
>
> both getting rid of the DMA direction argument, and then we have four
> new dma_buf calls:
>
> dma_buf_map_sg()
> dma_buf_unmap_sg()
> dma_buf_sync_sg_for_cpu()
> dma_buf_sync_sg_for_device()
>
> which do the actual sg map/unmap via the DMA API *at the appropriate
> time for DMA*.
>
> So, the summary of this is - at the moment, I regard DRM Prime and dmabuf
> to be utterly broken in design for architectures such as ARM where the
> requirements of the DMA API have to be followed if you're going to have
> a happy life.
An addendum to the above:
I'll also point out that the whole thing of having random buffers mapped
into userspace, and doing DMA on them is _highly_ architecture specific.
I'm told that PARISC is one architecture where this does not work (because
DMA needs to have some kind of congruence factor programmed into it which
can be different between kernel and userspace mappings of the same
physical mappings.
I ran into this when trying to come up with a cross-arch DMA-API mmap API,
and PARISC ended up killing the idea off (the remains of that attempt is
the dma_mmap_* stuff in ARM's asm/dma-mapping.h.) However, this was for
the DMA coherent stuff, not the streaming API, which is what _this_
DMA prime/dma_buf stuff is about.
What I'm saying is think _very_ _carefully_ about userspace having
interleaved access to random DMA buffers. Arguably, DRM should _not_
allow this.
^ permalink raw reply
* Re: [PATCH] video: of_display_timing.h: Include <video/display_timing.h>
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-06-17 16:09 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1371476589-4660-1-git-send-email-fabio.estevam@freescale.com>
On 10:43 Mon 17 Jun , Fabio Estevam wrote:
> Commit ffa3fd21de ("videomode: implement public of_get_display_timing()") causes
> the following build warning:
>
> include/video/of_display_timing.h:18:10: warning: 'struct display_timing' declared inside parameter list [enabled by default]
> include/video/of_display_timing.h:18:10: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]
>
> As 'struct display_timing' is defined at <video/display_timing.h>, let's include
> this header to avoid the warning.
for 3.10 or 3.11?
Best Regards,
J.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> include/video/of_display_timing.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/video/of_display_timing.h b/include/video/of_display_timing.h
> index 6562ad9..a136f58 100644
> --- a/include/video/of_display_timing.h
> +++ b/include/video/of_display_timing.h
> @@ -8,6 +8,7 @@
>
> #ifndef __LINUX_OF_DISPLAY_TIMING_H
> #define __LINUX_OF_DISPLAY_TIMING_H
> +#include <video/display_timing.h>
>
> struct device_node;
> struct display_timings;
> --
> 1.8.1.2
>
>
^ permalink raw reply
* Re: [PATCH] video: of_display_timing.h: Include <video/display_timing.h>
From: Fabio Estevam @ 2013-06-17 16:19 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1371476589-4660-1-git-send-email-fabio.estevam@freescale.com>
On Mon, Jun 17, 2013 at 1:09 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 10:43 Mon 17 Jun , Fabio Estevam wrote:
>> Commit ffa3fd21de ("videomode: implement public of_get_display_timing()") causes
>> the following build warning:
>>
>> include/video/of_display_timing.h:18:10: warning: 'struct display_timing' declared inside parameter list [enabled by default]
>> include/video/of_display_timing.h:18:10: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]
>>
>> As 'struct display_timing' is defined at <video/display_timing.h>, let's include
>> this header to avoid the warning.
> for 3.10 or 3.11?
This is 3.11 material.
^ permalink raw reply
* Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework
From: Russell King - ARM Linux @ 2013-06-17 18:21 UTC (permalink / raw)
To: Inki Dae
Cc: Maarten Lankhorst, linux-fbdev, Kyungmin Park, DRI mailing list,
Rob Clark, myungjoo.ham, YoungJun Cho, Daniel Vetter,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <CAAQKjZOokFKN85pygVnm7ShSa+O0ZzwxvQ0rFssgNLp+RO5pGg@mail.gmail.com>
On Tue, Jun 18, 2013 at 02:19:04AM +0900, Inki Dae wrote:
> It seems like that all pages of the scatterlist should be mapped with
> DMA every time DMA operation is started (or drm_xxx_set_src_dma_buffer
> function call), and the pages should be unmapped from DMA again every
> time DMA operation is completed: internally, including each cache
> operation.
Correct.
> Isn't that big overhead?
Yes, if you _have_ to do a cache operation to ensure that the DMA agent
can see the data the CPU has written.
> And If there is no problem, we should accept such overhead?
If there is no problem then why are we discussing this and why do we need
this API extension? :)
> Actually, drm_gem_fd_to_handle() includes to map a
> given dmabuf with iommu table (just logical data) of the DMA. And then, the
> device address (or iova) already mapped will be set to buffer register of
> the DMA with drm_xxx_set_src/dst_dma_buffer(handle1, ...) call.
Consider this with a PIPT cache:
dma_map_sg() - at this point, the kernel addresses of these
buffers are cleaned and invalidated for the DMA
userspace writes to the buffer, the data sits in the CPU cache
Because the cache is PIPT, this data becomes visible to the
kernel as well.
DMA is started, and it writes to the buffer
Now, at this point, which is the correct data? The data physically in the
RAM which the DMA has written, or the data in the CPU cache. It may
the answer is - they both are, and the loss of either can be a potential
data corruption issue - there is no way to tell which data should be
kept but the system is in an inconsistent state and _one_ of them will
have to be discarded.
dma_unmap_sg() - at this point, the kernel addresses of the
buffers are _invalidated_ and any data in those
cache lines is discarded
Which also means that the data in userspace will also be discarded with
PIPT caches.
This is precisely why we have buffer rules associated with the DMA API,
which are these:
dma_map_sg()
- the buffer transfers ownership from the CPU to the DMA agent.
- the CPU may not alter the buffer in any way.
while (cpu_wants_access) {
dma_sync_sg_for_cpu()
- the buffer transfers ownership from the DMA to the CPU.
- the DMA may not alter the buffer in any way.
dma_sync_sg_for_device()
- the buffer transfers ownership from the CPU to the DMA
- the CPU may not alter the buffer in any way.
}
dma_unmap_sg()
- the buffer transfers ownership from the DMA to the CPU.
- the DMA may not alter the buffer in any way.
Any violation of that is likely to lead to data corruption. Now, some
may say that the DMA API is only about the kernel mapping. Yes it is,
because it takes no regard what so ever about what happens with the
userspace mappings. This is absolutely true when you have VIVT or
aliasing VIPT caches.
Now consider that with a PIPT cache, or a non-aliasing VIPT cache (which
is exactly the same behaviourally from this aspect) any modification of
a userspace mapping is precisely the same as modifying the kernel space
mapping - and what you find is that the above rules end up _inherently_
applying to the userspace mappings as well.
In other words, userspace applications which have mapped the buffers
must _also_ respect these rules.
And there's no way in hell that I'd ever trust userspace to get this
anywhere near right, and I *really* do not like these kinds of internal
kernel API rules being exposed to userspace.
And so the idea that userspace should be allowed to setup DMA transfers
via "set source buffer", "set destination buffer" calls into an API is
just plain rediculous. No, userspace should be allowed to ask for
"please copy from buffer X to buffer Y using this transform".
Also remember that drm_xxx_set_src/dst_dma_buffer() would have to
deal with the DRM object IDs for the buffer, and not the actual buffer
information themselves - that is kept within the kernel, so the kernel
knows what's happening.
^ permalink raw reply
* (no subject)
From: AFG GTBANK LOAN @ 2013-06-17 19:28 UTC (permalink / raw)
To: linux-fbdev
Loan Syndicacion
Am AFG Guaranty Trust Bank, zu strukturieren wir Kreditlinien treffen Sie
unsere
Kunden spezifischen geschäftlichen Anforderungen und einen deutlichen
Mehrwert für unsere
Kunden Unternehmen.
eine Division der AFG Finance und Private Bank plc.
Wenn Sie erwägen, eine große Akquisition oder ein Großprojekt sind, können
Sie
brauchen eine erhebliche Menge an Kredit. AFG Guaranty Trust Bank setzen
können
zusammen das Syndikat, das die gesamte Kredit schnürt für
Sie.
Als Bank mit internationaler Reichweite, sind wir gekommen, um Darlehen zu
identifizieren
Syndizierungen als Teil unseres Kerngeschäfts und durch spitzte diese Zeile
aggressiv sind wir an einem Punkt, wo wir kommen, um als erkannt haben
Hauptakteur in diesem Bereich.
öffnen Sie ein Girokonto heute mit einem Minimum Bankguthaben von 500 £ und
Getup zu £ 10.000 als Darlehen und auch den Hauch einer Chance und gewann
die Sterne
Preis von £ 500.000 in die sparen und gewinnen promo in may.aply jetzt.
mit dem Folowing Informationen über Rechtsanwalt steven lee das Konto
Offizier.
FULL NAME;
Wohnadresse;
E-MAIL-ADRESSE;
Telefonnummer;
Nächsten KINS;
MUTTER MAIDEN NAME;
Familienstand;
BÜROADRESSE;
ALTERNATIVE Telefonnummer;
TO @ yahoo.com bar.stevenlee
NOTE; ALLE Darlehen sind für 10JAHRE RATE VALID
ANGEBOT ENDET BALD SO JETZT HURRY
^ permalink raw reply
* Re: [PATCH] build some drivers only when compile-testing
From: Jiri Slaby @ 2013-06-17 20:05 UTC (permalink / raw)
To: Jeff Mahoney, Greg Kroah-Hartman
Cc: 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, Michal Marek, tomi.valkeinen
In-Reply-To: <519D8876.9050405@suse.com>
[-- Attachment #1: Type: text/plain, Size: 3819 bytes --]
On 05/23/2013 05:09 AM, Jeff Mahoney wrote:
> 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.
But this is exactly what COMPILE_TEST will give us when set to "y", or
am I missing something?
>>> 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.
I second all the above.
>>> +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*
Sam Ravnborg (the kconfig ex-maintainer) once wrote that he doesn't want
to extend the kconfig language for this purpose (which I support). That
a config option is fine and sufficient in this case [1]. Except he
called the config option "SHOW_ALL_DRIVERS". Adding the current
maintainer to CCs ;).
[1] http://comments.gmane.org/gmane.linux.kbuild.devel/9829
The last point I inclined to the Greg's argument to remove the EXPERT
dependency.
So currently I have what is attached... Comments?
thanks,
--
js
suse labs
[-- Attachment #2: 0001-build-some-drivers-only-when-compile-testing.patch --]
[-- Type: text/x-patch, Size: 5332 bytes --]
From e818e90b4f901c8d949d08bd05735203c5e88530 Mon Sep 17 00:00:00 2001
From: Jiri Slaby <jslaby@suse.cz>
Date: Wed, 22 May 2013 10:56:24 +0200
Subject: [PATCH v2] build some drivers only when compile-testing
Some drivers can be built on more platforms than they run on. This is
a burden for users and distributors who package a kernel. They have to
manually deselect some (for them useless) drivers when updating their
configs via oldconfig. And yet, 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.
* INTEL_MID_PTI: It is specific to the Penwell type of Intel Atom
device.
[v2]
* remove EXPERT dependency
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jeff Mahoney <jeffm@suse.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: linux-geode@lists.infradead.org
Cc: linux-fbdev@vger.kernel.org
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: netdev@vger.kernel.org
Cc: Ben Hutchings <ben@decadent.org.uk>
Cc: "Keller, Jacob E" <jacob.e.keller@intel.com>
---
drivers/misc/Kconfig | 2 +-
drivers/ptp/Kconfig | 1 +
drivers/usb/chipidea/Kconfig | 6 ++++++
drivers/usb/chipidea/Makefile | 4 +---
drivers/video/geode/Kconfig | 2 +-
init/Kconfig | 14 ++++++++++++++
6 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 80889d5..8dacd4c 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -135,7 +135,7 @@ config PHANTOM
config INTEL_MID_PTI
tristate "Parallel Trace Interface for MIPI P1149.7 cJTAG standard"
- depends on PCI && TTY
+ depends on PCI && TTY && (X86_INTEL_MID || COMPILE_TEST)
default n
help
The PTI (Parallel Trace Interface) driver directs
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index 1ea6f1d..5be73ba 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -72,6 +72,7 @@ config DP83640_PHY
config PTP_1588_CLOCK_PCH
tristate "Intel PCH EG20T as PTP clock"
+ depends on X86 || COMPILE_TEST
select PTP_1588_CLOCK
help
This driver adds support for using the PCH EG20T as a PTP
diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
index b2df442..3491d86 100644
--- a/drivers/usb/chipidea/Kconfig
+++ b/drivers/usb/chipidea/Kconfig
@@ -31,4 +31,10 @@ config USB_CHIPIDEA_DEBUG
help
Say Y here to enable debugging output of the ChipIdea driver.
+config USB_CHIPIDEA_IMX
+ bool "ChipIdea IMX support"
+ depends on OF_DEVICE && (ARM || COMPILE_TEST)
+ help
+ This option enables ChipIdea support on IMX.
+
endif
diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
index 4113feb..76d66ff 100644
--- a/drivers/usb/chipidea/Makefile
+++ b/drivers/usb/chipidea/Makefile
@@ -16,6 +16,4 @@ ifneq ($(CONFIG_PCI),)
obj-$(CONFIG_USB_CHIPIDEA) += ci13xxx_pci.o
endif
-ifneq ($(CONFIG_OF),)
- obj-$(CONFIG_USB_CHIPIDEA) += ci13xxx_imx.o usbmisc_imx.o
-endif
+obj-$(CONFIG_USB_CHIPIDEA_IMX) += ci13xxx_imx.o usbmisc_imx.o
diff --git a/drivers/video/geode/Kconfig b/drivers/video/geode/Kconfig
index 21e351a..1e85552 100644
--- a/drivers/video/geode/Kconfig
+++ b/drivers/video/geode/Kconfig
@@ -3,7 +3,7 @@
#
config FB_GEODE
bool "AMD Geode family framebuffer support"
- depends on FB && PCI && X86
+ depends on FB && PCI && (X86_32 || (X86 && COMPILE_TEST))
---help---
Say 'Y' here to allow you to select framebuffer drivers for
the AMD Geode family of processors.
diff --git a/init/Kconfig b/init/Kconfig
index fd0e436..e1854b2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -53,6 +53,20 @@ config CROSS_COMPILE
need to set this unless you want the configured kernel build
directory to select the cross-compiler automatically.
+config COMPILE_TEST
+ bool "Compile also drivers which will not load"
+ default n
+ help
+ Some drivers can be compiled on a different platform than they are
+ intended to be run on. Despite they cannot be loaded there (or even
+ when they load they cannot be used due to missing HW support),
+ developers still, opposing to distributors, might want to build such
+ drivers to compile-test them.
+
+ If you are a developer and want to build everything available, say Y
+ here. If you are a user/distributor, say N here to exclude useless
+ drivers to be distributed.
+
config LOCALVERSION
string "Local version - append to kernel release"
help
--
1.8.3
^ permalink raw reply related
* Re: [PATCH v4 0/7] xilinxfb changes
From: Arnd Bergmann @ 2013-06-17 21:07 UTC (permalink / raw)
To: monstr-pSz03upnqPeHXe+LvDLADg
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Timur Tabi,
Rob Herring, Michal Simek, Tomi Valkeinen, Grant Likely
In-Reply-To: <51BED093.1040207-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
On Monday 17 June 2013, Michal Simek wrote:
> On 06/17/2013 10:56 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 07:23 Mon 17 Jun , Michal Simek wrote:
> >> On 06/06/2013 06:23 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>> On 12:13 Mon 03 Jun , Michal Simek wrote:
> >>>> Hi,
> >>>>
> >>>
> >>> Arnd can you take look on it again please
> >>>
> >>> I'll take a look on it next week
> >>
> >> Any update on this?
> > look ok but I want the Ack from Arnd
Sorry for the delay, everything looks good to me.
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply
* Re: [PATCH] build some drivers only when compile-testing
From: Michal Marek @ 2013-06-18 4:51 UTC (permalink / raw)
To: Jiri Slaby
Cc: Jeff Mahoney, Greg Kroah-Hartman, 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,
tomi.valkeinen
In-Reply-To: <51BF6BFF.7050705@suse.cz>
Dne 17.6.2013 22:05, Jiri Slaby napsal(a):
> On 05/23/2013 05:09 AM, Jeff Mahoney wrote:
>> 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.
>
> But this is exactly what COMPILE_TEST will give us when set to "y", or
> am I missing something?
>
>>>> 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.
>
> I second all the above.
>
>>>> +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*
>
> Sam Ravnborg (the kconfig ex-maintainer) once wrote that he doesn't want
> to extend the kconfig language for this purpose (which I support). That
> a config option is fine and sufficient in this case [1]. Except he
> called the config option "SHOW_ALL_DRIVERS". Adding the current
> maintainer to CCs ;).
I agree with Sam. 'depends on XY || COMPILE_TEST' is quite
self-explanatory. And even if it's not, you can look up the help text
for COMPILE_TEST. With "archdepends on" or "available on", you need to
know what to look for to override the dependency.
Michal
^ permalink raw reply
* RE: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework
From: Inki Dae @ 2013-06-18 5:27 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1371467722-665-1-git-send-email-inki.dae@samsung.com>
> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk]
> Sent: Tuesday, June 18, 2013 3:21 AM
> To: Inki Dae
> Cc: Maarten Lankhorst; linux-fbdev; Kyungmin Park; DRI mailing list; Rob
> Clark; myungjoo.ham; YoungJun Cho; Daniel Vetter; linux-arm-
> kernel@lists.infradead.org; linux-media@vger.kernel.org
> Subject: Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization
> framework
>
> On Tue, Jun 18, 2013 at 02:19:04AM +0900, Inki Dae wrote:
> > It seems like that all pages of the scatterlist should be mapped with
> > DMA every time DMA operation is started (or drm_xxx_set_src_dma_buffer
> > function call), and the pages should be unmapped from DMA again every
> > time DMA operation is completed: internally, including each cache
> > operation.
>
> Correct.
>
> > Isn't that big overhead?
>
> Yes, if you _have_ to do a cache operation to ensure that the DMA agent
> can see the data the CPU has written.
>
> > And If there is no problem, we should accept such overhead?
>
> If there is no problem then why are we discussing this and why do we need
> this API extension? :)
Ok, another issue regardless of dmabuf-sync. Reasonable to me even though
big overhead. Besides, it seems like that most DRM drivers have same issue.
Therefore, we may need to solve this issue like below:
- do not map a dmabuf with DMA. And just create/update buffer object
of importer.
- map the buffer with DMA every time DMA start or iommu page fault
occurs.
- unmap the buffer from DMA every time DMA operation is completed
With the above approach, cache operation portion of my approach,
dmabuf-sync, can be removed. However, I'm not sure that we really have to
use the above approach with a big overhead. Of course, if we don't use the
above approach then user processes would need to do each cache operation
before DMA operation is started and also after DMA operation is completed in
some cases; user space mapped with physical memory as cachable, and CPU and
DMA share the same buffer.
So I'd like to ask for other DRM maintainers. How do you think about it? it
seems like that Intel DRM (maintained by Daniel), OMAP DRM (maintained by
Rob) and GEM CMA helper also have same issue Russell pointed out. I think
not only the above approach but also the performance is very important.
Thanks,
Inki Dae
>
> > Actually, drm_gem_fd_to_handle() includes to map a
> > given dmabuf with iommu table (just logical data) of the DMA. And then,
> the
> > device address (or iova) already mapped will be set to buffer register
> of
> > the DMA with drm_xxx_set_src/dst_dma_buffer(handle1, ...) call.
>
> Consider this with a PIPT cache:
>
> dma_map_sg() - at this point, the kernel addresses of these
> buffers are cleaned and invalidated for the DMA
>
> userspace writes to the buffer, the data sits in the CPU cache
> Because the cache is PIPT, this data becomes visible to the
> kernel as well.
>
> DMA is started, and it writes to the buffer
>
> Now, at this point, which is the correct data? The data physically in the
> RAM which the DMA has written, or the data in the CPU cache. It may
> the answer is - they both are, and the loss of either can be a potential
> data corruption issue - there is no way to tell which data should be
> kept but the system is in an inconsistent state and _one_ of them will
> have to be discarded.
>
> dma_unmap_sg() - at this point, the kernel addresses of the
> buffers are _invalidated_ and any data in those
> cache lines is discarded
>
> Which also means that the data in userspace will also be discarded with
> PIPT caches.
>
> This is precisely why we have buffer rules associated with the DMA API,
> which are these:
>
> dma_map_sg()
> - the buffer transfers ownership from the CPU to the DMA agent.
> - the CPU may not alter the buffer in any way.
> while (cpu_wants_access) {
> dma_sync_sg_for_cpu()
> - the buffer transfers ownership from the DMA to the CPU.
> - the DMA may not alter the buffer in any way.
> dma_sync_sg_for_device()
> - the buffer transfers ownership from the CPU to the DMA
> - the CPU may not alter the buffer in any way.
> }
> dma_unmap_sg()
> - the buffer transfers ownership from the DMA to the CPU.
> - the DMA may not alter the buffer in any way.
>
> Any violation of that is likely to lead to data corruption. Now, some
> may say that the DMA API is only about the kernel mapping. Yes it is,
> because it takes no regard what so ever about what happens with the
> userspace mappings. This is absolutely true when you have VIVT or
> aliasing VIPT caches.
>
> Now consider that with a PIPT cache, or a non-aliasing VIPT cache (which
> is exactly the same behaviourally from this aspect) any modification of
> a userspace mapping is precisely the same as modifying the kernel space
> mapping - and what you find is that the above rules end up _inherently_
> applying to the userspace mappings as well.
>
> In other words, userspace applications which have mapped the buffers
> must _also_ respect these rules.
>
> And there's no way in hell that I'd ever trust userspace to get this
> anywhere near right, and I *really* do not like these kinds of internal
> kernel API rules being exposed to userspace.
>
> And so the idea that userspace should be allowed to setup DMA transfers
> via "set source buffer", "set destination buffer" calls into an API is
> just plain rediculous. No, userspace should be allowed to ask for
> "please copy from buffer X to buffer Y using this transform".
>
> Also remember that drm_xxx_set_src/dst_dma_buffer() would have to
> deal with the DRM object IDs for the buffer, and not the actual buffer
> information themselves - that is kept within the kernel, so the kernel
> knows what's happening.
^ permalink raw reply
* Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework
From: Daniel Vetter @ 2013-06-18 7:00 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Inki Dae, Maarten Lankhorst, linux-fbdev, Kyungmin Park,
DRI mailing list, Rob Clark, myungjoo.ham, YoungJun Cho,
Daniel Vetter, linux-arm-kernel@lists.infradead.org,
linux-media@vger.kernel.org
In-Reply-To: <20130617154237.GJ2718@n2100.arm.linux.org.uk>
On Mon, Jun 17, 2013 at 04:42:37PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 18, 2013 at 12:03:31AM +0900, Inki Dae wrote:
> > 2013/6/17 Russell King - ARM Linux <linux@arm.linux.org.uk>
> > Exactly right. But that is not definitely my point. Could you please see
> > the below simple example?:
> > (Presume that CPU and DMA share a buffer and the buffer is mapped with user
> > space as cachable)
> >
> > handle1 = drm_gem_fd_to_handle(a dmabuf fd); ----> 1
> > ...
> > va1 = drm_gem_mmap(handle1);
> > va2 = drm_gem_mmap(handle2);
> > va3 = malloc(size);
> > ...
> >
> > while (conditions) {
> > memcpy(va1, some data, size);
>
> Nooooooooooooooooooooooooooooooooooooooooooooo!
>
> Well, the first thing to say here is that under the requirements of the
> DMA API, the above is immediately invalid, because you're writing to a
> buffer which under the terms of the DMA API is currently owned by the
> DMA agent, *not* by the CPU. You're supposed to call dma_sync_sg_for_cpu()
> before you do that - but how is userspace supposed to know that requirement?
> Why should userspace even _have_ to know these requirements of the DMA
> API?
>
> It's also entirely possible that drm_gem_fd_to_handle() (which indirectly
> causes dma_map_sg() on the buffers scatterlist) followed by mmap'ing it
> into userspace is a bug too, as it has the potential to touch caches or
> stuff in ways that maybe the DMA or IOMMU may not expect - but I'm not
> going to make too big a deal about that, because I don't think we have
> anything that picky.
>
> However, the first point above is the most important one, and exposing
> the quirks of the DMA API to userland is certainly not a nice thing to be
> doing. This needs to be fixed - we can't go and enforce an API which is
> deeply embedded within the kernel all the way out to userland.
>
> What we need is something along the lines of:
> (a) dma_buf_map_attachment() _not_ to map the scatterlist for DMA.
> or
> (b) drm_gem_prime_import() not to call dma_buf_map_attachment() at all.
I strongly vote for (b) (plus adding a dma_buf_map_sync interface to allow
syncing to other devices/cpu whithout dropping the dma mappings). At least
that's been the idea behind things, but currently all (x86-based) drm
drivers cut corners here massively.
Aside: The entire reason behind hiding the dma map step in the dma-buf
attachment is to allow drivers to expose crazy iommus (e.g. the tiler unit
on omap) to other devices. So imo (a) isn't the right choice.
>
> and for the scatterlist to be mapped for DMA at the point where the DMA
> operation is initiated, and unmapped at the point where the DMA operation
> is complete.
>
> So no, the problem is not that we need more APIs and code - we need the
> existing kernel API fixed so that we don't go exposing userspace to the
> requirements of the DMA API. Unless we do that, we're going to end
> up with a huge world of pain, where kernel architecture people need to
> audit every damned DRM userspace implementation that happens to be run
> on their platform, and that's not something arch people really can
> afford to do.
>
> Basically, I think the dma_buf stuff needs to be rewritten with the
> requirements of the DMA API in the forefront of whosever mind is doing
> the rewriting.
>
> Note: the existing stuff does have the nice side effect of being able
> to pass buffers which do not have a struct page * associated with them
> through the dma_buf API - I think we can still preserve that by having
> dma_buf provide a couple of new APIs to do the SG list map/sync/unmap,
> but in any case we need to fix the existing API so that:
>
> dma_buf_map_attachment() becomes dma_buf_get_sg()
> dma_buf_unmap_attachment() becomes dma_buf_put_sg()
>
> both getting rid of the DMA direction argument, and then we have four
> new dma_buf calls:
>
> dma_buf_map_sg()
> dma_buf_unmap_sg()
> dma_buf_sync_sg_for_cpu()
> dma_buf_sync_sg_for_device()
>
> which do the actual sg map/unmap via the DMA API *at the appropriate
> time for DMA*.
Hm, my idea was to just add a dma_buf_sync_attchment for the device side
syncing, since the cpu access stuff is already bracketed with the
begin/end cpu access stuff. We might need a sync_for_cpu or so for mmap,
but imo mmap support for dma_buf is a bit insane anyway, so I don't care
too much about it.
Since such dma mappings would be really longstanding in most cases anyway
drivers could just map with BIDIRECTIONAL and do all the real flushing
with the new sync stuff.
Another piece of fun will be integration with the fencing stuff Maarten is
doing. I'm not sure whether we should integrate that into the dma-buf
interface (for simple users who don't want to deal with the full
complexity at least).
>
> So, the summary of this is - at the moment, I regard DRM Prime and dmabuf
> to be utterly broken in design for architectures such as ARM where the
> requirements of the DMA API have to be followed if you're going to have
> a happy life.
Agreed. Unfortunately there are not many real drivers shipping in
upstream, and x86 is _very_ forgiving about all this stuff.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* [patch] fbmem: return -EFAULT on copy_to_user() failure
From: Dan Carpenter @ 2013-06-18 7:05 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard
Cc: Tomi Valkeinen, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <20121112110814.GB1678@elgon.mountain>
copy_to_user() returns the number of bytes remaining to be copied.
put_user() returns -EFAULT on error.
This function ORs a bunch of stuff together and returns jumbled non-zero
values on error. It should return -EFAULT.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 098bfc6..9217be3 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1305,7 +1305,9 @@ static int do_fscreeninfo_to_user(struct fb_fix_screeninfo *fix,
err |= copy_to_user(fix32->reserved, fix->reserved,
sizeof(fix->reserved));
- return err;
+ if (err)
+ return -EFAULT;
+ return 0;
}
static int fb_get_fscreeninfo(struct fb_info *info, unsigned int cmd,
^ permalink raw reply related
* [PATCH 3/9] aty128fb: use pdev->pm_cap instead of pci_find_capability(..,PCI_CAP_ID_PM)
From: Yijing Wang @ 2013-06-18 8:15 UTC (permalink / raw)
To: Paul Mackerras, Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: linux-kernel, Hanjun Guo, jiang.liu, Yijing Wang, linux-fbdev
Pci core has been saved pm cap register offset by pdev->pm_cap in pci_pm_init()
in init path. So we can use pdev->pm_cap instead of using
pci_find_capability(pdev, PCI_CAP_ID_PM) for better performance and simplified code.
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
---
drivers/video/aty/aty128fb.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/aty/aty128fb.c b/drivers/video/aty/aty128fb.c
index 8c55011..a4dfe8c 100644
--- a/drivers/video/aty/aty128fb.c
+++ b/drivers/video/aty/aty128fb.c
@@ -2016,7 +2016,7 @@ static int aty128_init(struct pci_dev *pdev, const struct pci_device_id *ent)
aty128_init_engine(par);
- par->pm_reg = pci_find_capability(pdev, PCI_CAP_ID_PM);
+ par->pm_reg = pdev->pm_cap;
par->pdev = pdev;
par->asleep = 0;
par->lock_blank = 0;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] build some drivers only when compile-testing
From: Felipe Balbi @ 2013-06-18 8:18 UTC (permalink / raw)
To: Michal Marek
Cc: Jiri Slaby, Jeff Mahoney, Greg Kroah-Hartman, 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,
tomi.valkeinen
In-Reply-To: <51BFE754.5080301@suse.cz>
[-- Attachment #1: Type: text/plain, Size: 4500 bytes --]
Hi,
On Tue, Jun 18, 2013 at 06:51:32AM +0200, Michal Marek wrote:
> Dne 17.6.2013 22:05, Jiri Slaby napsal(a):
> > On 05/23/2013 05:09 AM, Jeff Mahoney wrote:
> >> 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.
> >
> > But this is exactly what COMPILE_TEST will give us when set to "y", or
> > am I missing something?
> >
> >>>> 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.
> >
> > I second all the above.
> >
> >>>> +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*
> >
> > Sam Ravnborg (the kconfig ex-maintainer) once wrote that he doesn't want
> > to extend the kconfig language for this purpose (which I support). That
> > a config option is fine and sufficient in this case [1]. Except he
> > called the config option "SHOW_ALL_DRIVERS". Adding the current
> > maintainer to CCs ;).
>
> I agree with Sam. 'depends on XY || COMPILE_TEST' is quite
> self-explanatory. And even if it's not, you can look up the help text
> for COMPILE_TEST. With "archdepends on" or "available on", you need to
> know what to look for to override the dependency.
you will still end up with:
depends on (ARCH_OMAP || ARCH_EXYNOS || ARCH_DAVINCI || ARCH_PPC || ...)
And every now and again that particular line will be updated to add
another arch dependency.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] build some drivers only when compile-testing
From: Jiri Slaby @ 2013-06-18 8:24 UTC (permalink / raw)
To: balbi, Michal Marek
Cc: Jeff Mahoney, Greg Kroah-Hartman, 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,
tomi.valkeinen
In-Reply-To: <20130618081858.GD5461@arwen.pp.htv.fi>
On 06/18/2013 10:18 AM, Felipe Balbi wrote:
> Hi,
>
> On Tue, Jun 18, 2013 at 06:51:32AM +0200, Michal Marek wrote:
>> Dne 17.6.2013 22:05, Jiri Slaby napsal(a):
>>> On 05/23/2013 05:09 AM, Jeff Mahoney wrote:
>>>> 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.
>>>
>>> But this is exactly what COMPILE_TEST will give us when set to
>>> "y", or am I missing something?
>>>
>>>>>> 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.
>>>
>>> I second all the above.
>>>
>>>>>> +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*
>>>
>>> Sam Ravnborg (the kconfig ex-maintainer) once wrote that he
>>> doesn't want to extend the kconfig language for this purpose
>>> (which I support). That a config option is fine and sufficient
>>> in this case [1]. Except he called the config option
>>> "SHOW_ALL_DRIVERS". Adding the current maintainer to CCs ;).
>>
>> I agree with Sam. 'depends on XY || COMPILE_TEST' is quite
>> self-explanatory. And even if it's not, you can look up the help
>> text for COMPILE_TEST. With "archdepends on" or "available on",
>> you need to know what to look for to override the dependency.
>
> you will still end up with:
>
> depends on (ARCH_OMAP || ARCH_EXYNOS || ARCH_DAVINCI || ARCH_PPC ||
> ...)
>
> And every now and again that particular line will be updated to
> add another arch dependency.
But that is perfectly fine *when* the driver is supported on those
archs only.
And come on, how much often will this "every now and again" happen? We
don't have that much cases where a driver is augmented to work on
another arch or platform. It either works on all of them => doesn't
need COMPILE_TEST, or work on one or two arches at most.
--
js
suse labs
^ permalink raw reply
* [PATCH 9/9] radeon: use pdev->pm_cap instead of pci_find_capability(..,PCI_CAP_ID_PM)
From: Yijing Wang @ 2013-06-18 8:24 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen
Cc: linux-kernel, Hanjun Guo, jiang.liu, Yijing Wang, linux-fbdev
Pci core has been saved pm cap register offset by pdev->pm_cap in pci_pm_init()
in init path. So we can use pdev->pm_cap instead of using
pci_find_capability(pdev, PCI_CAP_ID_PM) for better performance and simplified code.
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/video/aty/radeon_pm.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/aty/radeon_pm.c b/drivers/video/aty/radeon_pm.c
index 92bda58..f7091ec 100644
--- a/drivers/video/aty/radeon_pm.c
+++ b/drivers/video/aty/radeon_pm.c
@@ -2805,7 +2805,7 @@ static void radeonfb_early_resume(void *data)
void radeonfb_pm_init(struct radeonfb_info *rinfo, int dynclk, int ignore_devlist, int force_sleep)
{
/* Find PM registers in config space if any*/
- rinfo->pm_reg = pci_find_capability(rinfo->pdev, PCI_CAP_ID_PM);
+ rinfo->pm_reg = rinfo->pdev->pm_cap;
/* Enable/Disable dynamic clocks: TODO add sysfs access */
if (rinfo->family = CHIP_FAMILY_RS480)
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] video: mxsfb: fix color settings for 18bit data bus and 32bpp
From: maxime.ripard @ 2013-06-18 8:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130607090228.GI14209@lukather>
Hi Hector,
On Fri, Jun 07, 2013 at 11:02:28AM +0200, maxime.ripard@free-electrons.com wrote:
> On Fri, Jun 07, 2013 at 10:10:39AM +0200, Hector Palacios wrote:
> > For a combination of 18bit LCD data bus width and a color
> > mode of 32bpp, the driver was setting the color mapping to
> > rgb666, which is wrong, as the color in memory realy has an
> > rgb888 layout.
> >
> > This patch also removes the setting of flag CTRL_DF24 that
> > makes the driver dimiss the upper 2 bits when handling 32/24bpp
> > colors in a diplay with 18bit data bus width. This flag made
> > true color images display wrong in such configurations.
> >
> > Finally, the color mapping rgb666 has also been removed as nobody
> > is using it and high level applications like Qt5 cannot work
> > with it either.
> >
> > Reference: https://lkml.org/lkml/2013/5/23/220
> > Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> > Acked-by: Juergen Beisert <jbe@pengutronix.de>
>
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>
> Please also note that fbdev is now maintained by Jean Christophe
> Plagniol-Villard (plagnioj@jcrosoft.com, in CC), and that he is away
> until the 10th of June, so maybe it should be safe to resend it to him
> after this date.
It seems like Jean-Christophe is back online, maybe you should resend
him the patch now, it would be great to have it for 3.11.
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH] build some drivers only when compile-testing
From: Felipe Balbi @ 2013-06-18 8:34 UTC (permalink / raw)
To: Jiri Slaby
Cc: balbi, Michal Marek, Jeff Mahoney, Greg Kroah-Hartman, 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,
tomi.valkeinen
In-Reply-To: <51C01948.9060708@suse.cz>
[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]
Hi,
On Tue, Jun 18, 2013 at 10:24:40AM +0200, Jiri Slaby wrote:
> >>> Sam Ravnborg (the kconfig ex-maintainer) once wrote that he
> >>> doesn't want to extend the kconfig language for this purpose
> >>> (which I support). That a config option is fine and sufficient
> >>> in this case [1]. Except he called the config option
> >>> "SHOW_ALL_DRIVERS". Adding the current maintainer to CCs ;).
> >>
> >> I agree with Sam. 'depends on XY || COMPILE_TEST' is quite
> >> self-explanatory. And even if it's not, you can look up the help
> >> text for COMPILE_TEST. With "archdepends on" or "available on",
> >> you need to know what to look for to override the dependency.
> >
> > you will still end up with:
> >
> > depends on (ARCH_OMAP || ARCH_EXYNOS || ARCH_DAVINCI || ARCH_PPC ||
> > ...)
> >
> > And every now and again that particular line will be updated to
> > add another arch dependency.
>
> But that is perfectly fine *when* the driver is supported on those
> archs only.
>
> And come on, how much often will this "every now and again" happen? We
> don't have that much cases where a driver is augmented to work on
> another arch or platform. It either works on all of them => doesn't
> need COMPILE_TEST, or work on one or two arches at most.
MUSB alone has 8 different arch choices. Before, it used to be that core
driver was dependendent on all of them, so whenever someone wanted to
build MUSB for another arch, they had to introdude their glue code and
modify the dependency of the core driver.
Also EHCI, it works on pretty much everything, so does DWC3.
DWC3 already has three possibilities but I know of at least 3 others
which could show up soonish.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] build some drivers only when compile-testing
From: Tomi Valkeinen @ 2013-06-18 8:35 UTC (permalink / raw)
To: Michal Marek
Cc: Jiri Slaby, Jeff Mahoney, Greg Kroah-Hartman, 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: <51BFE754.5080301@suse.cz>
[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]
On 18/06/13 07:51, Michal Marek wrote:
>> Sam Ravnborg (the kconfig ex-maintainer) once wrote that he doesn't want
>> to extend the kconfig language for this purpose (which I support). That
>> a config option is fine and sufficient in this case [1]. Except he
>> called the config option "SHOW_ALL_DRIVERS". Adding the current
>> maintainer to CCs ;).
>
> I agree with Sam. 'depends on XY || COMPILE_TEST' is quite
> self-explanatory. And even if it's not, you can look up the help text
> for COMPILE_TEST. With "archdepends on" or "available on", you need to
> know what to look for to override the dependency.
I would rather have "depends on", when the code actually depends on
something (i.e. you can't compile/load the code otherwise), and "used
on"/"available on" when the code is just normally not used except on the
listed platforms (but nothing prevents from compiling and using the code
on all platforms).
But I'm fine with COMPILE_TEST or similar, I guess it's an acceptable
compromise and trivial to implement. Even if we had "used on" we'd still
need to update the Kconfig file when the code is being used on a new
platform, just like with COMPILE_TEST.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework
From: Russell King - ARM Linux @ 2013-06-18 8:43 UTC (permalink / raw)
To: Inki Dae
Cc: 'Maarten Lankhorst', 'linux-fbdev',
'Kyungmin Park', 'DRI mailing list',
'Rob Clark', 'myungjoo.ham',
'YoungJun Cho', 'Daniel Vetter', linux-arm-kernel,
linux-media
In-Reply-To: <007301ce6be4$8d5c6040$a81520c0$%dae@samsung.com>
On Tue, Jun 18, 2013 at 02:27:40PM +0900, Inki Dae wrote:
> So I'd like to ask for other DRM maintainers. How do you think about it? it
> seems like that Intel DRM (maintained by Daniel), OMAP DRM (maintained by
> Rob) and GEM CMA helper also have same issue Russell pointed out. I think
> not only the above approach but also the performance is very important.
CMA uses coherent memory to back their buffers, though that might not be
true of memory obtained from other drivers via dma_buf. Plus, there is
no support in the CMA helper for exporting or importng these buffers.
I guess Intel i915 is only used on x86, which is a coherent platform and
requires no cache maintanence for DMA.
OMAP DRM does not support importing non-DRM buffers buffers back into
DRM. Moreover, it will suffer from the problems I described if any
attempt is made to write to the buffer after it has been re-imported.
Lastly, I should point out that the dma_buf stuff is really only useful
when you need to export a dma buffer from one driver and import it into
another driver - for example to pass data from a camera device driver to
a display device driver. It shouldn't be used within a single driver
as a means of passing buffers between userspace and kernel space.
^ permalink raw reply
* [PATCH RESEND] video: mxsfb: fix color settings for 18bit data bus and 32bpp
From: Hector Palacios @ 2013-06-18 8:44 UTC (permalink / raw)
To: linux-arm-kernel
For a combination of 18bit LCD data bus width and a color
mode of 32bpp, the driver was setting the color mapping to
rgb666, which is wrong, as the color in memory realy has an
rgb888 layout.
This patch also removes the setting of flag CTRL_DF24 that
makes the driver dimiss the upper 2 bits when handling 32/24bpp
colors in a diplay with 18bit data bus width. This flag made
true color images display wrong in such configurations.
Finally, the color mapping rgb666 has also been removed as nobody
is using it and high level applications like Qt5 cannot work
with it either.
Reference: https://lkml.org/lkml/2013/5/23/220
Signed-off-by: Hector Palacios <hector.palacios@digi.com>
Acked-by: Juergen Beisert <jbe@pengutronix.de>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
drivers/video/mxsfb.c | 26 --------------------------
1 file changed, 26 deletions(-)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 21223d4..d2c5105 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -239,24 +239,6 @@ static const struct fb_bitfield def_rgb565[] = {
}
};
-static const struct fb_bitfield def_rgb666[] = {
- [RED] = {
- .offset = 16,
- .length = 6,
- },
- [GREEN] = {
- .offset = 8,
- .length = 6,
- },
- [BLUE] = {
- .offset = 0,
- .length = 6,
- },
- [TRANSP] = { /* no support for transparency */
- .length = 0,
- }
-};
-
static const struct fb_bitfield def_rgb888[] = {
[RED] = {
.offset = 16,
@@ -309,9 +291,6 @@ static int mxsfb_check_var(struct fb_var_screeninfo *var,
break;
case STMLCDIF_16BIT:
case STMLCDIF_18BIT:
- /* 24 bit to 18 bit mapping */
- rgb = def_rgb666;
- break;
case STMLCDIF_24BIT:
/* real 24 bit */
rgb = def_rgb888;
@@ -453,11 +432,6 @@ static int mxsfb_set_par(struct fb_info *fb_info)
return -EINVAL;
case STMLCDIF_16BIT:
case STMLCDIF_18BIT:
- /* 24 bit to 18 bit mapping */
- ctrl |= CTRL_DF24; /* ignore the upper 2 bits in
- * each colour component
- */
- break;
case STMLCDIF_24BIT:
/* real 24 bit */
break;
--
1.8.3
^ 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