* [PATCH linux-next] fb: make fp_get_options name argument const
From: Vincent Stehlé @ 2013-06-18 14:23 UTC (permalink / raw)
To: linux-next, linux-fbdev
Cc: Vincent Stehlé, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Dave Airlie, trivial
drm_get_connector_name now returns a const value, which causes the following
compilation warning:
drivers/gpu/drm/drm_fb_helper.c: In function ‘drm_fb_helper_parse_command_line’:
drivers/gpu/drm/drm_fb_helper.c:127:3: warning: passing argument 1 of ‘fb_get_options’ discards ‘const’ qualifier from pointer target type [enabled by default]
In file included from drivers/gpu/drm/drm_fb_helper.c:35:0:
include/linux/fb.h:627:12: note: expected ‘char *’ but argument is of type ‘const char *’
As fb_get_options uses its name argument as read only, make it const. This
fixes the aforementioned compilation warning.
Signed-off-by: Vincent Stehlé <vincent.stehle@freescale.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: trivial@kernel.org
---
Hi,
I remarked this warning while building linux-next with imx_v6_v7_defconfig.
Is changing fb_get_options prototype "permitted", please?
Best regards,
V.
drivers/video/fbmem.c | 2 +-
include/linux/fb.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 098bfc6..d8d5779 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1881,7 +1881,7 @@ static int ofonly __read_mostly;
*
* NOTE: Needed to maintain backwards compatibility
*/
-int fb_get_options(char *name, char **option)
+int fb_get_options(const char *name, char **option)
{
char *opt, *options = NULL;
int retval = 0;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index d49c60f..ffac70a 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -624,7 +624,7 @@ extern void fb_pad_aligned_buffer(u8 *dst, u32 d_pitch, u8 *src, u32 s_pitch, u3
extern void fb_set_suspend(struct fb_info *info, int state);
extern int fb_get_color_depth(struct fb_var_screeninfo *var,
struct fb_fix_screeninfo *fix);
-extern int fb_get_options(char *name, char **option);
+extern int fb_get_options(const char *name, char **option);
extern int fb_new_modelist(struct fb_info *info);
extern struct fb_info *registered_fb[FB_MAX];
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/2] i915: i915_gem: Print a size_t with %z
From: Fabio Estevam @ 2013-06-18 13:26 UTC (permalink / raw)
To: plagnioj
Cc: Fabio Estevam, linux-fbdev, Daniel Vetter, intel-gfx, Dave Airlie,
festevam
In-Reply-To: <1371561988-30989-1-git-send-email-fabio.estevam@freescale.com>
Fix the following build warning:
drivers/gpu/drm/i915/i915_gem.c:3129:3: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'size_t' [-Wformat]
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: <intel-gfx@lists.freedesktop.org>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6e469e6..d497ca5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3126,7 +3126,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
* before evicting everything in a vain attempt to find space.
*/
if (obj->base.size > gtt_max) {
- DRM_ERROR("Attempting to bind an object larger than the aperture: object=%zd > %s aperture=%ld\n",
+ DRM_ERROR("Attempting to bind an object larger than the aperture: object=%zd > %s aperture=%zd\n",
obj->base.size,
map_and_fenceable ? "mappable" : "total",
gtt_max);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 1/2] video: fbmem: Use 'const char' in fb_get_options()
From: Fabio Estevam @ 2013-06-18 13:26 UTC (permalink / raw)
To: linux-fbdev
Commit d20d31748 (drm: Constify the pretty-print functions) causes the following
build warning:
drivers/gpu/drm/drm_fb_helper.c:127:3: warning: passing argument 1 of 'fb_get_options' discards 'const' qualifier from pointer target type [enabled by default]
Let's change the first argument of fb_get_options from 'char *' to 'const char *'
so that we can get rid of this warning.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/video/fbmem.c | 2 +-
include/linux/fb.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 098bfc6..d8d5779 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1881,7 +1881,7 @@ static int ofonly __read_mostly;
*
* NOTE: Needed to maintain backwards compatibility
*/
-int fb_get_options(char *name, char **option)
+int fb_get_options(const char *name, char **option)
{
char *opt, *options = NULL;
int retval = 0;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index d49c60f..ffac70a 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -624,7 +624,7 @@ extern void fb_pad_aligned_buffer(u8 *dst, u32 d_pitch, u8 *src, u32 s_pitch, u3
extern void fb_set_suspend(struct fb_info *info, int state);
extern int fb_get_color_depth(struct fb_var_screeninfo *var,
struct fb_fix_screeninfo *fix);
-extern int fb_get_options(char *name, char **option);
+extern int fb_get_options(const char *name, char **option);
extern int fb_new_modelist(struct fb_info *info);
extern struct fb_info *registered_fb[FB_MAX];
--
1.8.1.2
^ permalink raw reply related
* Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework
From: Russell King - ARM Linux @ 2013-06-18 10:46 UTC (permalink / raw)
To: Daniel Vetter
Cc: Inki Dae, Maarten Lankhorst, linux-fbdev, Kyungmin Park,
DRI mailing list, Rob Clark, myungjoo.ham, YoungJun Cho,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <CAKMK7uECS=eqNB-Ud6s=NvdYMPfxgJaGPbUx9hANfP6kb02j_w@mail.gmail.com>
On Tue, Jun 18, 2013 at 09:00:16AM +0200, Daniel Vetter wrote:
> On Mon, Jun 17, 2013 at 04:42:37PM +0100, Russell King - ARM Linux wrote:
> > 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.
That's why I also talk below about adding the dma_buf_map/sync callbacks
below, so that a dma_buf implementation can do whatever is necessary with
the sg at the point of use.
However, you are correct that this should be unnecessary, as DRM should
only be calling dma_buf_map_attachment() when it needs to know about the
memory behind the object. The problem is that people will cache that
information - it also may be expensive to setup for the dma_buf - as it
involves counting the number of pages, memory allocation, and maybe
obtaining the set of pages from shmem.
With (a) plus the callbacks below, it means that step is only performed
once, and then the DMA API part is entirely separate - we can have our
cake and eat it in that regard.
> > 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.
Note that the DMA API debug doesn't allow you to change the direction
argument on an existing mapping (neither should it, again this is
documented in the DMA API stuff in Documentation/). This is where you
would need the complete set of four functions I mention above which
reflect the functionality of the DMA API.
The dma_buf implementation doesn't _have_ to implement them if they
are no-ops - for example, your dma_buf sg array contains DMA pointers,
and the memory is already coherent in some way (for example, it's a
separate chunk of memory which isn't part of system RAM.)
^ permalink raw reply
* Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework
From: Lucas Stach @ 2013-06-18 9:47 UTC (permalink / raw)
To: Inki Dae
Cc: 'Russell King - ARM Linux', 'linux-fbdev',
'Kyungmin Park', 'DRI mailing list',
'myungjoo.ham', 'YoungJun Cho', linux-arm-kernel,
linux-media
In-Reply-To: <008a01ce6c02$e00a9f50$a01fddf0$%dae@samsung.com>
Am Dienstag, den 18.06.2013, 18:04 +0900 schrieb Inki Dae:
[...]
>
> > 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
>
The most basic question about why you are trying to implement this sort
of thing in the dma_buf framework still stands.
Once you imported a dma_buf into your DRM driver it's a GEM object and
you can and should use the native DRM ioctls to prepare/end a CPU access
to this BO. Then internally to your driver you can use the dma_buf
reservation/fence stuff to provide the necessary cross-device sync.
Regards,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [RFC 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
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