* Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework
From: Russell King - ARM Linux @ 2013-06-18 9:38 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: <008a01ce6c02$e00a9f50$a01fddf0$%dae@samsung.com>
On Tue, Jun 18, 2013 at 06:04:44PM +0900, Inki Dae wrote:
>
>
> > -----Original Message-----
> > From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk]
> > Sent: Tuesday, June 18, 2013 5:43 PM
> > 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: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.
> >
>
> It's not so. Please see Dave's drm next. recently dmabuf support for the CMA
> helper has been merged to there.
The point stands: CMA is DMA coherent memory. It doesn't need and must
never be dma-map-sg'd or dma-sync'd or dma-unmap'd.
^ permalink raw reply
* Re: [PATCH] build some drivers only when compile-testing
From: Jiri Slaby @ 2013-06-18 9:21 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: <20130618085154.GN5461@arwen.pp.htv.fi>
On 06/18/2013 10:51 AM, Felipe Balbi wrote:
> right, but my argument is that I rather not have either. Depend on
> PCI if you us PCI, depend on EXTCON if you use extcon, but no
> driver should have an ARCH dependency. Specially since it lets
> people include mach/* and asm/* headers because "it doesn't break
> compilation for anyone".
The argument is elsewhere. If I understand correctly, Kconfig is for
users, not to be hi-jacked by kernel developers. And users should not
really care about our development processes, cross compilations or
whatever bells and whistles we use. They just don't want to see
drivers which they have no way to *use*, they indeed don't care
whether some more compile at all. We do not want every kernel packager
for every distro out in the wild, to go through all the help texts, to
see whether they should compile and package a driver or not. It's a
tedious work and this option would save time to the packagers.
Try to package and maintain a kernel for a distribution, you will find
out what a cool and surprising work that is...
In the best case I would vote for hard dependencies as cross-compilers
are easy to obtain and set up nowadays. But well, we still want to
("cross") compile drivers, so let's add COMPILE_TEST and use that to
save time to automatic builds.
--
js
suse labs
^ permalink raw reply
* RE: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework
From: Inki Dae @ 2013-06-18 9:04 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 5:43 PM
> 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: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.
>
It's not so. Please see Dave's drm next. recently dmabuf support for the CMA
helper has been merged to there.
> 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
Correct. TODO yet.
> 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
Most people know that.
> a display device driver. It shouldn't be used within a single driver
> as a means of passing buffers between userspace and kernel space.
What I try to do is not really such ugly thing. What I try to do is to
notify that, when CPU tries to access a buffer , to kernel side through
dmabuf interface. So it's not really to send the buffer to kernel.
Thanks,
Inki Dae
^ permalink raw reply
* Re: [PATCH] build some drivers only when compile-testing
From: Felipe Balbi @ 2013-06-18 8:51 UTC (permalink / raw)
To: Michal Marek
Cc: balbi, 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: <51C01E04.2050908@suse.cz>
[-- Attachment #1: Type: text/plain, Size: 2279 bytes --]
Hi,
On Tue, Jun 18, 2013 at 10:44:52AM +0200, Michal Marek wrote:
> > 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.
>
> But that you have complex dependencies in some drivers is mostly
> orthogonal to the two choices of syntax, isn't it?
>
> depends on ARCH1 || ARCH2 || .... || ARCH8 || COMPILE_TEST
>
> vs.
>
> archdepends on ARCH1 || ARCH2 || .... || ARCH8
right, but my argument is that I rather not have either. Depend on PCI
if you us PCI, depend on EXTCON if you use extcon, but no driver should
have an ARCH dependency. Specially since it lets people include mach/*
and asm/* headers because "it doesn't break compilation for anyone".
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] video: mxsfb: fix color settings for 18bit data bus and 32bpp
From: Hector Palacios @ 2013-06-18 8:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130618083229.GC15037@lukather>
On 06/18/2013 10:32 AM, maxime.ripard@free-electrons.com wrote:
> 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.
I just resent it. Thanks for the reminder.
--
Héctor Palacios
Best regards,
--
Hector Palacios
^ permalink raw reply
* Re: [PATCH] build some drivers only when compile-testing
From: Michal Marek @ 2013-06-18 8:44 UTC (permalink / raw)
To: balbi
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: <20130618083425.GI5461@arwen.pp.htv.fi>
Dne 18.6.2013 10:34, Felipe Balbi napsal(a):
> 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.
But that you have complex dependencies in some drivers is mostly
orthogonal to the two choices of syntax, isn't it?
depends on ARCH1 || ARCH2 || .... || ARCH8 || COMPILE_TEST
vs.
archdepends on ARCH1 || ARCH2 || .... || ARCH8
Michal
^ 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
* 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
* 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: [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] 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
* [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] 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
* 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
* [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
* [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
* 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
* 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: [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: [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: 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
* (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: [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
* 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: [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
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