* [PATCH v4 7/7] video: xilinxfb: Use driver for Xilinx ARM Zynq
From: Michal Simek @ 2013-06-03 10:13 UTC (permalink / raw)
To: linux-kernel
Cc: Michal Simek, Michal Simek, Arnd Bergmann, Timur Tabi,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev
In-Reply-To: <cover.1370254386.git.michal.simek@xilinx.com>
[-- Attachment #1: Type: text/plain, Size: 872 bytes --]
From: Michal Simek <monstr@monstr.eu>
Enable this driver for all Xilinx platforms.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v4:
- Remove "video: xilinxfb: Fix sparse warnings"
patch because it is trying to fix incorrect API
usage and sparse should warn about it.
Changes in v3: None
Changes in v2: None
drivers/video/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 2e937bd..2c301f8 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2188,7 +2188,7 @@ config FB_PS3_DEFAULT_SIZE_M
config FB_XILINX
tristate "Xilinx frame buffer support"
- depends on FB && (XILINX_VIRTEX || MICROBLAZE)
+ depends on FB && (XILINX_VIRTEX || MICROBLAZE || ARCH_ZYNQ)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
--
1.8.2.3
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply related
* Re: [PATCH] PM: Add pm_sleep_ops_ptr() macro
From: Rafael J. Wysocki @ 2013-06-03 11:11 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Jean-Christophe PLAGNIOL-VILLARD',
'Lars-Peter Clausen', 'Michael Hennerich',
'Tomi Valkeinen', linux-fbdev, linux-pm
In-Reply-To: <002801ce6022$a2928930$e7b79b90$@samsung.com>
On Monday, June 03, 2013 03:21:51 PM Jingoo Han wrote:
> Add pm_sleep_ops_ptr() macro that allows the .pm entry in the driver structures
> to be assigned without having an #define xxx NULL for the case that PM_SLEEP is
> not enabled.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Michael Hennerich <michael.hennerich@analog.com>
Do you want me to replace the previous patch with this?
Rafael
> ---
> drivers/video/bfin-lq035q1-fb.c | 20 +++++++++++---------
> include/linux/pm.h | 6 ++++++
> 2 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/video/bfin-lq035q1-fb.c b/drivers/video/bfin-lq035q1-fb.c
> index 29d8c04..4474e64 100644
> --- a/drivers/video/bfin-lq035q1-fb.c
> +++ b/drivers/video/bfin-lq035q1-fb.c
> @@ -170,16 +170,19 @@ static int lq035q1_spidev_remove(struct spi_device *spi)
> return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_SHUT);
> }
>
> -#ifdef CONFIG_PM
> -static int lq035q1_spidev_suspend(struct spi_device *spi, pm_message_t state)
> +#ifdef CONFIG_PM_SLEEP
> +static int lq035q1_spidev_suspend(struct device *dev)
> {
> + struct spi_device *spi = to_spi_device(dev);
> +
> return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_SHUT);
> }
>
> -static int lq035q1_spidev_resume(struct spi_device *spi)
> +static int lq035q1_spidev_resume(struct device *dev)
> {
> - int ret;
> + struct spi_device *spi = to_spi_device(dev);
> struct spi_control *ctl = spi_get_drvdata(spi);
> + int ret;
>
> ret = lq035q1_control(spi, LQ035_DRIVER_OUTPUT_CTL, ctl->mode);
> if (ret)
> @@ -187,11 +190,11 @@ static int lq035q1_spidev_resume(struct spi_device *spi)
>
> return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_ON);
> }
> -#else
> -# define lq035q1_spidev_suspend NULL
> -# define lq035q1_spidev_resume NULL
> #endif
>
> +static SIMPLE_DEV_PM_OPS(lq035q1_spidev_pm_ops, lq035q1_spidev_suspend,
> + lq035q1_spidev_resume);
> +
> /* Power down all displays on reboot, poweroff or halt */
> static void lq035q1_spidev_shutdown(struct spi_device *spi)
> {
> @@ -708,8 +711,7 @@ static int bfin_lq035q1_probe(struct platform_device *pdev)
> info->spidrv.probe = lq035q1_spidev_probe;
> info->spidrv.remove = lq035q1_spidev_remove;
> info->spidrv.shutdown = lq035q1_spidev_shutdown;
> - info->spidrv.suspend = lq035q1_spidev_suspend;
> - info->spidrv.resume = lq035q1_spidev_resume;
> + info->spidrv.driver.pm = pm_sleep_ops_ptr(&lq035q1_spidev_pm_ops);
>
> ret = spi_register_driver(&info->spidrv);
> if (ret < 0) {
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index bd50d15..999d652 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -61,6 +61,12 @@ extern const char power_group_name[]; /* = "power" */
> #define pm_ops_ptr(_ptr) NULL
> #endif
>
> +#ifdef CONFIG_PM_SLEEP
> +#define pm_sleep_ops_ptr(_ptr) (_ptr)
> +#else
> +#define pm_sleep_ops_ptr(_ptr) NULL
> +#endif
> +
> typedef struct pm_message {
> int event;
> } pm_message_t;
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH] PM: Add pm_sleep_ops_ptr() macro
From: Rafael J. Wysocki @ 2013-06-03 19:54 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Jean-Christophe PLAGNIOL-VILLARD',
'Lars-Peter Clausen', 'Michael Hennerich',
'Tomi Valkeinen', linux-fbdev, linux-pm
In-Reply-To: <1867006.c6mxKpbPvX@vostro.rjw.lan>
On Monday, June 03, 2013 01:11:07 PM Rafael J. Wysocki wrote:
> On Monday, June 03, 2013 03:21:51 PM Jingoo Han wrote:
> > Add pm_sleep_ops_ptr() macro that allows the .pm entry in the driver structures
> > to be assigned without having an #define xxx NULL for the case that PM_SLEEP is
> > not enabled.
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Lars-Peter Clausen <lars@metafoo.de>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: Michael Hennerich <michael.hennerich@analog.com>
>
> Do you want me to replace the previous patch with this?
Quite frankly, I prefer the previous version, which was more general.
That is, what if someone wants to use that macro for runtime PM too?
Honestly, I think we'll be better off without any of them, so I'm dropping
the previous patch too for now.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: linux-next: Tree for Jun 3 (fonts.c & vivi)
From: Randy Dunlap @ 2013-06-03 20:34 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: linux-next, linux-kernel, linux-media, linux-fbdev
In-Reply-To: <20130603163717.a6f78476e57d92fadd6f6a23@canb.auug.org.au>
On 06/02/13 23:37, Stephen Rothwell wrote:
> Hi all,
>
> Changes since 20130531:
>
on x86_64:
warning: (VIDEO_VIVI && USB_SISUSBVGA && SOLO6X10) selects FONT_SUPPORT which has unmet direct dependencies (HAS_IOMEM && VT)
warning: (VIDEO_VIVI && FB_VGA16 && FB_S3 && FB_VT8623 && FB_ARK && USB_SISUSBVGA_CON && SOLO6X10) selects FONT_8x16 which has unmet direct dependencies (HAS_IOMEM && VT && FONT_SUPPORT)
drivers/built-in.o: In function `vivi_init':
vivi.c:(.init.text+0x1a3da): undefined reference to `find_font'
when CONFIG_VT is not enabled.
Just make CONFIG_VIDEO_VIVI depend on VT ?
--
~Randy
^ permalink raw reply
* Re: linux-next: Tree for Jun 3 (fonts.c & vivi)
From: Geert Uytterhoeven @ 2013-06-03 20:54 UTC (permalink / raw)
To: Randy Dunlap
Cc: Stephen Rothwell, Linux-Next, linux-kernel@vger.kernel.org,
linux-media, Linux Fbdev development list
In-Reply-To: <51ACFDF2.4040600@infradead.org>
On Mon, Jun 3, 2013 at 10:34 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
> On 06/02/13 23:37, Stephen Rothwell wrote:
>> Changes since 20130531:
> on x86_64:
>
> warning: (VIDEO_VIVI && USB_SISUSBVGA && SOLO6X10) selects FONT_SUPPORT which has unmet direct dependencies (HAS_IOMEM && VT)
> warning: (VIDEO_VIVI && FB_VGA16 && FB_S3 && FB_VT8623 && FB_ARK && USB_SISUSBVGA_CON && SOLO6X10) selects FONT_8x16 which has unmet direct dependencies (HAS_IOMEM && VT && FONT_SUPPORT)
I knew about thet warning. But I thought it was harmless, as none of the font
code really depends on console support...
> drivers/built-in.o: In function `vivi_init':
> vivi.c:(.init.text+0x1a3da): undefined reference to `find_font'
>
> when CONFIG_VT is not enabled.
... but I missed that drivers/video/console is not used if CONFIG_VT=y.
Sorry for that.
> Just make CONFIG_VIDEO_VIVI depend on VT ?
Does this (whitespace-damaged copy-and-paste) help?
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -12,7 +12,7 @@ fb-y := fbmem.o fbmon.o fbcmap.o
modedb.o fbcvt.o
fb-objs := $(fb-y)
-obj-$(CONFIG_VT) += console/
+obj-y += console/
obj-$(CONFIG_LOGO) += logo/
obj-y += backlight/
It shouldn't make a difference if nothing inside drivers/video/console
is enabled,
as all objects in drivers/video/console/Makefile are conditional.
BTW, my plan was to move the font code to lib/font, but I haven't done that yet.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: linux-next: Tree for Jun 3 (fonts.c & vivi)
From: Randy Dunlap @ 2013-06-03 21:06 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Stephen Rothwell, Linux-Next, linux-kernel@vger.kernel.org,
linux-media, Linux Fbdev development list
In-Reply-To: <CAMuHMdUALrScFE895xRiBvgUpVa9Tvic5M7YxefrEgyeMaSjhw@mail.gmail.com>
On 06/03/13 13:54, Geert Uytterhoeven wrote:
> On Mon, Jun 3, 2013 at 10:34 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>> On 06/02/13 23:37, Stephen Rothwell wrote:
>>> Changes since 20130531:
>> on x86_64:
>>
>> warning: (VIDEO_VIVI && USB_SISUSBVGA && SOLO6X10) selects FONT_SUPPORT which has unmet direct dependencies (HAS_IOMEM && VT)
>> warning: (VIDEO_VIVI && FB_VGA16 && FB_S3 && FB_VT8623 && FB_ARK && USB_SISUSBVGA_CON && SOLO6X10) selects FONT_8x16 which has unmet direct dependencies (HAS_IOMEM && VT && FONT_SUPPORT)
>
> I knew about thet warning. But I thought it was harmless, as none of the font
> code really depends on console support...
>
>> drivers/built-in.o: In function `vivi_init':
>> vivi.c:(.init.text+0x1a3da): undefined reference to `find_font'
>>
>> when CONFIG_VT is not enabled.
>
> ... but I missed that drivers/video/console is not used if CONFIG_VT=y.
> Sorry for that.
>
>> Just make CONFIG_VIDEO_VIVI depend on VT ?
>
> Does this (whitespace-damaged copy-and-paste) help?
Yes, that works. Thanks.
Acked-by: Randy Dunlap <rdunlap@infradead.org>
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -12,7 +12,7 @@ fb-y := fbmem.o fbmon.o fbcmap.o
> modedb.o fbcvt.o
> fb-objs := $(fb-y)
>
> -obj-$(CONFIG_VT) += console/
> +obj-y += console/
> obj-$(CONFIG_LOGO) += logo/
> obj-y += backlight/
>
> It shouldn't make a difference if nothing inside drivers/video/console
> is enabled,
> as all objects in drivers/video/console/Makefile are conditional.
>
> BTW, my plan was to move the font code to lib/font, but I haven't done that yet.
--
~Randy
^ permalink raw reply
* Re: [PATCH] video: display_timing: make parameter const
From: Laurent Pinchart @ 2013-06-04 1:21 UTC (permalink / raw)
To: dri-devel; +Cc: Dave Airlie, linux-fbdev, Florian Tobias Schandinat, kernel
In-Reply-To: <1369657985-11703-1-git-send-email-l.stach@pengutronix.de>
Hi,
On Monday 27 May 2013 14:33:05 Lucas Stach wrote:
> From: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>
> As the device_node pointer is not changed in of_get_display_timing and
> parse_timing_property it can be a const pointer.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/video/of_display_timing.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/of_display_timing.c
> b/drivers/video/of_display_timing.c index 56009bc..85c1a41 100644
> --- a/drivers/video/of_display_timing.c
> +++ b/drivers/video/of_display_timing.c
> @@ -23,7 +23,7 @@
> * Every display_timing can be specified with either just the typical value
> or * a range consisting of min/typ/max. This function helps handling this
> **/
> -static int parse_timing_property(struct device_node *np, const char *name,
> +static int parse_timing_property(const struct device_node *np, const char
> *name, struct timing_entry *result)
> {
> struct property *prop;
> @@ -56,7 +56,8 @@ static int parse_timing_property(struct device_node *np,
> const char *name, * of_get_display_timing - parse display_timing entry from
> device_node * @np: device_node with the properties
> **/
> -static struct display_timing *of_get_display_timing(struct device_node *np)
> +static struct display_timing *of_get_display_timing(const struct
> device_node + *np)
> {
> struct display_timing *dt;
> u32 val = 0;
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] PM: Add pm_sleep_ops_ptr() macro
From: Jingoo Han @ 2013-06-04 4:23 UTC (permalink / raw)
To: 'Rafael J. Wysocki'
Cc: 'Jean-Christophe PLAGNIOL-VILLARD',
'Lars-Peter Clausen', 'Michael Hennerich',
'Tomi Valkeinen', linux-fbdev, linux-pm, Jingoo Han
In-Reply-To: <3772829.L58vmUyOpk@vostro.rjw.lan>
On Tuesday, June 04, 2013 4:55 AM, Rafael J. Wysocki wrote:
> On Monday, June 03, 2013 01:11:07 PM Rafael J. Wysocki wrote:
> > On Monday, June 03, 2013 03:21:51 PM Jingoo Han wrote:
> > > Add pm_sleep_ops_ptr() macro that allows the .pm entry in the driver structures
> > > to be assigned without having an #define xxx NULL for the case that PM_SLEEP is
> > > not enabled.
> > >
> > > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > > Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > > Cc: Lars-Peter Clausen <lars@metafoo.de>
> > > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > > Cc: Michael Hennerich <michael.hennerich@analog.com>
> >
> > Do you want me to replace the previous patch with this?
>
> Quite frankly, I prefer the previous version, which was more general.
>
> That is, what if someone wants to use that macro for runtime PM too?
>
> Honestly, I think we'll be better off without any of them, so I'm dropping
> the previous patch too for now.
OK, I see.
I agree with your opinion.
Also, as I mentioned, I want other's better ideas.
Best regards,
Jingoo Han
>
> Thanks,
> Rafael
>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: linux-next: Tree for Jun 3 (fonts.c & vivi)
From: Geert Uytterhoeven @ 2013-06-04 6:50 UTC (permalink / raw)
To: Randy Dunlap
Cc: Stephen Rothwell, Linux-Next, linux-kernel@vger.kernel.org,
linux-media, Linux Fbdev development list
In-Reply-To: <51AD053B.1040403@infradead.org>
On Mon, Jun 3, 2013 at 11:06 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
> On 06/03/13 13:54, Geert Uytterhoeven wrote:
>> On Mon, Jun 3, 2013 at 10:34 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>>> On 06/02/13 23:37, Stephen Rothwell wrote:
>>>> Changes since 20130531:
>>> on x86_64:
>>>
>>> warning: (VIDEO_VIVI && USB_SISUSBVGA && SOLO6X10) selects FONT_SUPPORT which has unmet direct dependencies (HAS_IOMEM && VT)
>>> warning: (VIDEO_VIVI && FB_VGA16 && FB_S3 && FB_VT8623 && FB_ARK && USB_SISUSBVGA_CON && SOLO6X10) selects FONT_8x16 which has unmet direct dependencies (HAS_IOMEM && VT && FONT_SUPPORT)
>>
>> I knew about thet warning. But I thought it was harmless, as none of the font
>> code really depends on console support...
>>
>>> drivers/built-in.o: In function `vivi_init':
>>> vivi.c:(.init.text+0x1a3da): undefined reference to `find_font'
>>>
>>> when CONFIG_VT is not enabled.
>>
>> ... but I missed that drivers/video/console is not used if CONFIG_VT=y.
>> Sorry for that.
>>
>>> Just make CONFIG_VIDEO_VIVI depend on VT ?
>>
>> Does this (whitespace-damaged copy-and-paste) help?
>
> Yes, that works. Thanks.
>
> Acked-by: Randy Dunlap <rdunlap@infradead.org>
Thanks, I'll fold this into the original commit, which is destined for v3.10.
>> --- a/drivers/video/Makefile
>> +++ b/drivers/video/Makefile
>> @@ -12,7 +12,7 @@ fb-y := fbmem.o fbmon.o fbcmap.o
>> modedb.o fbcvt.o
>> fb-objs := $(fb-y)
>>
>> -obj-$(CONFIG_VT) += console/
>> +obj-y += console/
>> obj-$(CONFIG_LOGO) += logo/
>> obj-y += backlight/
>>
>> It shouldn't make a difference if nothing inside drivers/video/console
>> is enabled,
>> as all objects in drivers/video/console/Makefile are conditional.
>>
>> BTW, my plan was to move the font code to lib/font, but I haven't done that yet.
I'l try to do that for v3.11.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: linux-next: fbdev-next inclusing
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-06-04 11:13 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20130531051325.GQ19468@game.jcrosoft.org>
On 16:35 Mon 03 Jun , Stephen Rothwell wrote:
> Hi,
>
> On Fri, 31 May 2013 07:13:25 +0200 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
> >
> > Could you include the new fbdev -next in the linux-next please?
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/plagnioj/linux-fbdev.git for-next
>
> I have replaced Florian's tree with this one. Let me know if you want
> something different. Also, the contacts for this tree are now:
>
> Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> linux-fbdev@vger.kernel.org
if Florian wish to continue to be in Cc keep but please add
Tomi Valkeinen <tomi.valkeinen@ti.com> too
Best Regards,
J.
^ permalink raw reply
* Re: linux-next: fbdev-next inclusing
From: Stephen Rothwell @ 2013-06-05 0:49 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20130531051325.GQ19468@game.jcrosoft.org>
[-- Attachment #1: Type: text/plain, Size: 311 bytes --]
Hi,
On Tue, 4 Jun 2013 13:13:44 +0200 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
>
> if Florian wish to continue to be in Cc keep but please add
> Tomi Valkeinen <tomi.valkeinen@ti.com> too
Done. Florian?
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH] OMAPDSS: Remove kfree for memory allocated with devm_kzalloc
From: Emil Goode @ 2013-06-05 17:29 UTC (permalink / raw)
To: archit, axel.lin
Cc: linux-omap, linux-fbdev, linux-kernel, kernel-janitors,
Emil Goode
It's not necessary to free memory allocated with devm_kzalloc
in a remove function and using kfree leads to a double free.
Signed-off-by: Emil Goode <emilgoode@gmail.com>
---
drivers/video/omap2/displays/panel-picodlp.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/video/omap2/displays/panel-picodlp.c b/drivers/video/omap2/displays/panel-picodlp.c
index 62f2db0..859e111 100644
--- a/drivers/video/omap2/displays/panel-picodlp.c
+++ b/drivers/video/omap2/displays/panel-picodlp.c
@@ -469,8 +469,6 @@ static void picodlp_panel_remove(struct omap_dss_device *dssdev)
i2c_unregister_device(picod->picodlp_i2c_client);
dev_set_drvdata(&dssdev->dev, NULL);
dev_dbg(&dssdev->dev, "removing picodlp panel\n");
-
- kfree(picod);
}
static int picodlp_panel_enable(struct omap_dss_device *dssdev)
--
1.7.10.4
^ permalink raw reply related
* [PATCH] simplefb: add support for a8b8g8r8 pixel format
From: Alexandre Courbot @ 2013-06-06 7:20 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Stephen Warren,
Olof Johansson
Cc: gnurou, linux-kernel, linux-fbdev, Alexandre Courbot
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Documentation/devicetree/bindings/video/simple-framebuffer.txt | 1 +
drivers/video/simplefb.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
index 3ea4605..70c26f3 100644
--- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
+++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
@@ -12,6 +12,7 @@ Required properties:
- stride: The number of bytes in each line of the framebuffer.
- format: The format of the framebuffer surface. Valid values are:
- r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b).
+ - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r).
Example:
diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
index e2e9e3e..d7041aa 100644
--- a/drivers/video/simplefb.c
+++ b/drivers/video/simplefb.c
@@ -84,6 +84,7 @@ struct simplefb_format {
static struct simplefb_format simplefb_formats[] = {
{ "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
+ { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} },
};
struct simplefb_params {
--
1.8.3
^ permalink raw reply related
* Re: [PATCH] OMAPDSS: Remove kfree for memory allocated with devm_kzalloc
From: Archit Taneja @ 2013-06-06 7:22 UTC (permalink / raw)
To: Emil Goode
Cc: axel.lin, linux-omap, linux-fbdev, linux-kernel, kernel-janitors,
Valkeinen, Tomi
In-Reply-To: <1370453396-18043-1-git-send-email-emilgoode@gmail.com>
Hi,
On Wednesday 05 June 2013 10:59 PM, Emil Goode wrote:
> It's not necessary to free memory allocated with devm_kzalloc
> in a remove function and using kfree leads to a double free.
Looks fine to me. Tomi, could you take this for 3.11?
Archit
>
> Signed-off-by: Emil Goode <emilgoode@gmail.com>
> ---
> drivers/video/omap2/displays/panel-picodlp.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/video/omap2/displays/panel-picodlp.c b/drivers/video/omap2/displays/panel-picodlp.c
> index 62f2db0..859e111 100644
> --- a/drivers/video/omap2/displays/panel-picodlp.c
> +++ b/drivers/video/omap2/displays/panel-picodlp.c
> @@ -469,8 +469,6 @@ static void picodlp_panel_remove(struct omap_dss_device *dssdev)
> i2c_unregister_device(picod->picodlp_i2c_client);
> dev_set_drvdata(&dssdev->dev, NULL);
> dev_dbg(&dssdev->dev, "removing picodlp panel\n");
> -
> - kfree(picod);
> }
>
> static int picodlp_panel_enable(struct omap_dss_device *dssdev)
>
^ permalink raw reply
* re: uvesafb: Clean up MTRR code
From: Dan Carpenter @ 2013-06-06 7:49 UTC (permalink / raw)
To: linux-fbdev
Hello Andy Lutomirski,
This is a semi-automatic email about new static checker warnings.
The patch 63e28a7a5ffc: "uvesafb: Clean up MTRR code" from May 13,
2013, leads to the following Smatch complaint:
drivers/video/uvesafb.c:1819 uvesafb_remove()
warn: variable dereferenced before check 'par' (see line 1814)
drivers/video/uvesafb.c
1813 iounmap(info->screen_base);
1814 arch_phys_wc_del(par->mtrr_handle);
^^^^^^^^^^^^^^^^
New dereference.
1815 release_mem_region(info->fix.smem_start, info->fix.smem_len);
1816 fb_destroy_modedb(info->monspecs.modedb);
1817 fb_dealloc_cmap(&info->cmap);
1818
1819 if (par) {
^^^
Old check.
1820 if (par->vbe_modes)
1821 kfree(par->vbe_modes);
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] simplefb: add support for a8b8g8r8 pixel format
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-06-06 7:59 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen, Stephen Warren,
Olof Johansson, gnurou, linux-kernel, linux-fbdev
In-Reply-To: <1370503259-16618-1-git-send-email-acourbot@nvidia.com>
On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> Documentation/devicetree/bindings/video/simple-framebuffer.txt | 1 +
> drivers/video/simplefb.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> index 3ea4605..70c26f3 100644
> --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> @@ -12,6 +12,7 @@ Required properties:
> - stride: The number of bytes in each line of the framebuffer.
> - format: The format of the framebuffer surface. Valid values are:
> - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b).
> + - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r).
>
> Example:
>
> diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
> index e2e9e3e..d7041aa 100644
> --- a/drivers/video/simplefb.c
> +++ b/drivers/video/simplefb.c
> @@ -84,6 +84,7 @@ struct simplefb_format {
>
> static struct simplefb_format simplefb_formats[] = {
> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} },
why don't you parse the string?
so you will a real generic bindings
Best Regards,
J.
> };
>
> struct simplefb_params {
> --
> 1.8.3
>
^ permalink raw reply
* Re: [PATCH] simplefb: add support for a8b8g8r8 pixel format
From: Alex Courbot @ 2013-06-06 8:12 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD
Cc: Tomi Valkeinen, Stephen Warren, Olof Johansson, gnurou@gmail.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <3356BC4D-EEF6-4FCA-9310-5B0727EBF288@jcrosoft.com>
On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>> Documentation/devicetree/bindings/video/simple-framebuffer.txt | 1 +
>> drivers/video/simplefb.c | 1 +
>> 2 files changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>> index 3ea4605..70c26f3 100644
>> --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>> @@ -12,6 +12,7 @@ Required properties:
>> - stride: The number of bytes in each line of the framebuffer.
>> - format: The format of the framebuffer surface. Valid values are:
>> - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b).
>> + - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r).
>>
>> Example:
>>
>> diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
>> index e2e9e3e..d7041aa 100644
>> --- a/drivers/video/simplefb.c
>> +++ b/drivers/video/simplefb.c
>> @@ -84,6 +84,7 @@ struct simplefb_format {
>>
>> static struct simplefb_format simplefb_formats[] = {
>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} },
>
> why don't you parse the string?
>
> so you will a real generic bindings
Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330
The list of modes of this driver should not grow too big. Even in terms
of footprint I'd say the list should remain smaller than the parsing code.
What we can discuss though is whether we want to keep this a8b8g8r8
syntax or switch to something more standard, say "rgba8888".
Alex.
^ permalink raw reply
* Re: [PATCH] simplefb: add support for a8b8g8r8 pixel format
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-06-06 8:24 UTC (permalink / raw)
To: Alex Courbot
Cc: Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen, Stephen Warren,
Olof Johansson, gnurou@gmail.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org
In-Reply-To: <51B0446A.4090305@nvidia.com>
On Jun 6, 2013, at 10:12 AM, Alex Courbot <acourbot@nvidia.com> wrote:
> On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>
>> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
>>
>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>> ---
>>> Documentation/devicetree/bindings/video/simple-framebuffer.txt | 1 +
>>> drivers/video/simplefb.c | 1 +
>>> 2 files changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>>> index 3ea4605..70c26f3 100644
>>> --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>>> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>>> @@ -12,6 +12,7 @@ Required properties:
>>> - stride: The number of bytes in each line of the framebuffer.
>>> - format: The format of the framebuffer surface. Valid values are:
>>> - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b).
>>> + - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r).
>>>
>>> Example:
>>>
>>> diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
>>> index e2e9e3e..d7041aa 100644
>>> --- a/drivers/video/simplefb.c
>>> +++ b/drivers/video/simplefb.c
>>> @@ -84,6 +84,7 @@ struct simplefb_format {
>>>
>>> static struct simplefb_format simplefb_formats[] = {
>>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
>>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} },
>>
>> why don't you parse the string?
>>
>> so you will a real generic bindings
>
> Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330
>
> The list of modes of this driver should not grow too big. Even in terms of footprint I'd say the list should remain smaller than the parsing code.
>
> What we can discuss though is whether we want to keep this a8b8g8r8 syntax or switch to something more standard, say "rgba8888".
I'm going to be very honest I do not like the simplefb driver from the beginning
but I do found it useful. And as said in it's name it need to be *SIMPLE*
Then a huge list of compatible no way
otherwise we drop this from the simplefb and make it a generic helper
I do not want to see format parser in every drivers this need to handle at video framework level
If I see that we start to increase again and again the simplefb I will not accept
to merge the code as we must keep it simple
Best Regards,
J.
^ permalink raw reply
* Re: [PATCH] simplefb: add support for a8b8g8r8 pixel format
From: Alex Courbot @ 2013-06-06 8:27 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD
Cc: Tomi Valkeinen, Stephen Warren, Olof Johansson, gnurou@gmail.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <29610936-BB03-4844-888B-56E1C8E5DF4A@jcrosoft.com>
On 06/06/2013 05:24 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> On Jun 6, 2013, at 10:12 AM, Alex Courbot <acourbot@nvidia.com> wrote:
>
>> On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>
>>> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
>>>
>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>> ---
>>>> Documentation/devicetree/bindings/video/simple-framebuffer.txt | 1 +
>>>> drivers/video/simplefb.c | 1 +
>>>> 2 files changed, 2 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>>>> index 3ea4605..70c26f3 100644
>>>> --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>>>> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>>>> @@ -12,6 +12,7 @@ Required properties:
>>>> - stride: The number of bytes in each line of the framebuffer.
>>>> - format: The format of the framebuffer surface. Valid values are:
>>>> - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b).
>>>> + - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r).
>>>>
>>>> Example:
>>>>
>>>> diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
>>>> index e2e9e3e..d7041aa 100644
>>>> --- a/drivers/video/simplefb.c
>>>> +++ b/drivers/video/simplefb.c
>>>> @@ -84,6 +84,7 @@ struct simplefb_format {
>>>>
>>>> static struct simplefb_format simplefb_formats[] = {
>>>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
>>>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} },
>>>
>>> why don't you parse the string?
>>>
>>> so you will a real generic bindings
>>
>> Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330
>>
>> The list of modes of this driver should not grow too big. Even in terms of footprint I'd say the list should remain smaller than the parsing code.
>>
>> What we can discuss though is whether we want to keep this a8b8g8r8 syntax or switch to something more standard, say "rgba8888".
>
> I'm going to be very honest I do not like the simplefb driver from the beginning
> but I do found it useful. And as said in it's name it need to be *SIMPLE*
>
> Then a huge list of compatible no way
> otherwise we drop this from the simplefb and make it a generic helper
>
> I do not want to see format parser in every drivers this need to handle at video framework level
>
> If I see that we start to increase again and again the simplefb I will not accept
> to merge the code as we must keep it simple
In that case it's probably better to maintain a "simple" list of
supported modes, which is what this patch does.
Alex.
^ permalink raw reply
* Re: [PATCH] simplefb: add support for a8b8g8r8 pixel format
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-06-06 14:50 UTC (permalink / raw)
To: Alex Courbot
Cc: Tomi Valkeinen, Stephen Warren, Olof Johansson, gnurou@gmail.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <51B047FD.4020400@nvidia.com>
On 17:27 Thu 06 Jun , Alex Courbot wrote:
> On 06/06/2013 05:24 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >
> >On Jun 6, 2013, at 10:12 AM, Alex Courbot <acourbot@nvidia.com> wrote:
> >
> >>On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>>
> >>>On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> >>>
> >>>>Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> >>>>---
> >>>>Documentation/devicetree/bindings/video/simple-framebuffer.txt | 1 +
> >>>>drivers/video/simplefb.c | 1 +
> >>>>2 files changed, 2 insertions(+)
> >>>>
> >>>>diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> >>>>index 3ea4605..70c26f3 100644
> >>>>--- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> >>>>+++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> >>>>@@ -12,6 +12,7 @@ Required properties:
> >>>>- stride: The number of bytes in each line of the framebuffer.
> >>>>- format: The format of the framebuffer surface. Valid values are:
> >>>> - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b).
> >>>>+ - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r).
> >>>>
> >>>>Example:
> >>>>
> >>>>diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
> >>>>index e2e9e3e..d7041aa 100644
> >>>>--- a/drivers/video/simplefb.c
> >>>>+++ b/drivers/video/simplefb.c
> >>>>@@ -84,6 +84,7 @@ struct simplefb_format {
> >>>>
> >>>>static struct simplefb_format simplefb_formats[] = {
> >>>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
> >>>>+ { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} },
> >>>
> >>>why don't you parse the string?
> >>>
> >>>so you will a real generic bindings
> >>
> >>Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330
> >>
> >>The list of modes of this driver should not grow too big. Even in terms of footprint I'd say the list should remain smaller than the parsing code.
> >>
> >>What we can discuss though is whether we want to keep this a8b8g8r8 syntax or switch to something more standard, say "rgba8888".
> >
> >I'm going to be very honest I do not like the simplefb driver from the beginning
> >but I do found it useful. And as said in it's name it need to be *SIMPLE*
> >
> >Then a huge list of compatible no way
> >otherwise we drop this from the simplefb and make it a generic helper
> >
> >I do not want to see format parser in every drivers this need to handle at video framework level
> >
> >If I see that we start to increase again and again the simplefb I will not accept
> >to merge the code as we must keep it simple
>
> In that case it's probably better to maintain a "simple" list of
> supported modes, which is what this patch does.
so get out it of the simplefb other drivers can use it
Best Regards,
J.
> Alex.
>
^ permalink raw reply
* Re: [PATCH] simplefb: add support for a8b8g8r8 pixel format
From: Stephen Warren @ 2013-06-06 16:11 UTC (permalink / raw)
To: Alex Courbot
Cc: Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen, Olof Johansson,
gnurou@gmail.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org
In-Reply-To: <51B0446A.4090305@nvidia.com>
On 06/06/2013 02:12 AM, Alex Courbot wrote:
> On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>
>> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com>
>> wrote:
>>
>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
No commit description? It'd be useful to at least justify this by
mentioning that some platform will actually use this...
...
>>> static struct simplefb_format simplefb_formats[] = {
>>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
>>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} },
>>
>> why don't you parse the string?
>>
>> so you will a real generic bindings
>
> Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330
>
> The list of modes of this driver should not grow too big. Even in terms
> of footprint I'd say the list should remain smaller than the parsing code.
>
> What we can discuss though is whether we want to keep this a8b8g8r8
> syntax or switch to something more standard, say "rgba8888".
I would prefer to keep the syntax of the new formats consistent, so that
if we ever do add format-parsing code rather than table-based lookup,
all the existing formats will continue to work unchanged, without any
kind of fallback lookup table.
^ permalink raw reply
* Re: [PATCH] simplefb: add support for a8b8g8r8 pixel format
From: Stephen Warren @ 2013-06-06 16:17 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD
Cc: Alex Courbot, Tomi Valkeinen, Olof Johansson, gnurou@gmail.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <20130606145059.GU19468@game.jcrosoft.org>
On 06/06/2013 08:50 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:27 Thu 06 Jun , Alex Courbot wrote:
>> On 06/06/2013 05:24 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>
>>> On Jun 6, 2013, at 10:12 AM, Alex Courbot <acourbot@nvidia.com> wrote:
>>>
>>>> On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>>
>>>>> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
...
>>>>>> static struct simplefb_format simplefb_formats[] = {
>>>>>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
>>>>>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} },
>>>>>
>>>>> why don't you parse the string?
Jean-Christophe, I'm afraid I can't tell exactly what you're arguing for.
Here, you want code added to parse the string ...
This has already been rejected as being over-engineered, and more than
this simple driver needs. Even if it were shared code, the only
practical use of such a parsing function would be to support this
driver, since presumably any other HW-specific driver already knows
exactly which format the FB is in, and hence wouldn't need to share this
code.
>>>>> so you will a real generic bindings
>>>>
>>>> Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330
>>>>
>>>> The list of modes of this driver should not grow too big. Even in terms of footprint I'd say the list should remain smaller than the parsing code.
>>>>
>>>> What we can discuss though is whether we want to keep this a8b8g8r8 syntax or switch to something more standard, say "rgba8888".
>>>
>>> I'm going to be very honest I do not like the simplefb driver from the beginning
>>> but I do found it useful. And as said in it's name it need to be *SIMPLE*
>>>
>>> Then a huge list of compatible no way
>>> otherwise we drop this from the simplefb and make it a generic helper
>>>
>>> I do not want to see format parser in every drivers this need to handle at video framework level
... yet here you appear to be arguing against using a format parser, or
including a format parser ...
Note that a lookup table isn't any kind of shared parser; it's just a
very tiny and simple table of static data. It seems quite unlikely that
this could be a maintenance issue, even if over time a few more entries
get added to the table.
>>> If I see that we start to increase again and again the simplefb I will not accept
>>> to merge the code as we must keep it simple
>>
>> In that case it's probably better to maintain a "simple" list of
>> supported modes, which is what this patch does.
>
> so get out it of the simplefb other drivers can use it
... yet here you appear to want to move the list of modes into some
central location ...
I don't think that's useful for the reason I mentioned above: presumably
any other HW-specific driver already knows exactly which format the FB
is in, and hence wouldn't need to share this code/table.
Why don't we simply take this patch to extend this table, and then *if*
any other FB driver needs to parse a format from DT, we can move the
code(table) to a common location at that time. That will be a trivial
change, and one this patch does nothing to make any harder. Making the
code/table common before then seems like over-engineering.
^ permalink raw reply
* Re: [PATCH] simplefb: add support for a8b8g8r8 pixel format
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-06-06 16:20 UTC (permalink / raw)
To: Stephen Warren
Cc: Alex Courbot, Tomi Valkeinen, Olof Johansson, gnurou@gmail.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <51B0B4A2.4010104@wwwdotorg.org>
On 10:11 Thu 06 Jun , Stephen Warren wrote:
> On 06/06/2013 02:12 AM, Alex Courbot wrote:
> > On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>
> >> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com>
> >> wrote:
> >>
> >>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>
> No commit description? It'd be useful to at least justify this by
> mentioning that some platform will actually use this...
>
> ...
> >>> static struct simplefb_format simplefb_formats[] = {
> >>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
> >>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} },
> >>
> >> why don't you parse the string?
> >>
> >> so you will a real generic bindings
> >
> > Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330
> >
> > The list of modes of this driver should not grow too big. Even in terms
> > of footprint I'd say the list should remain smaller than the parsing code.
> >
> > What we can discuss though is whether we want to keep this a8b8g8r8
> > syntax or switch to something more standard, say "rgba8888".
>
> I would prefer to keep the syntax of the new formats consistent, so that
> if we ever do add format-parsing code rather than table-based lookup,
> all the existing formats will continue to work unchanged, without any
> kind of fallback lookup table.
I do prefer a format-parsing than any long lookup table that take time at boot
time
Best Regards,
J.
^ permalink raw reply
* Re: [PATCH v4 0/7] xilinxfb changes
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-06-06 16:23 UTC (permalink / raw)
To: Michal Simek
Cc: Timur Tabi, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tomi Valkeinen, Grant Likely
In-Reply-To: <cover.1370254386.git.michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
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
Best Regards,
J.
> I have done more changes in the driver to support probing
> on little and big endian system where detection is done
> directly on the hardware.
> I have also done some cleanups to get it to the better shape.
>
> Thanks for your review,
> Michal
>
> Changes in v4:
> - Acked by Arnd
> - Remove "video: xilinxfb: Fix sparse warnings"
> patch because it is trying to fix incorrect API
> usage and sparse should warn about it.
>
> Changes in v3:
> - fix commit message
> - Remove out_be IO name from function name
> - Change patch subject from "Do not use out_be32 IO function"
> to "Do not name out_be32 in function name"
> - New patch in this patchset based on discussions
> - New patch in this patchset based on discussions
> - New patch in this patchset
> - New patch in this patchset based on discussions
>
> Changes in v2:
> - use of_property_read_u32 helper function
>
> Michal Simek (7):
> video: xilinxfb: Fix OF probing on little-endian systems
> video: xilinxfb: Do not name out_be32 in function name
> video: xilinxfb: Rename PLB_ACCESS_FLAG to BUS_ACCESS_FLAG
> video: xilinxfb: Use drvdata->regs_phys instead of physaddr
> video: xilinxfb: Group bus initialization
> video: xilinxfb: Add support for little endian accesses
> video: xilinxfb: Use driver for Xilinx ARM Zynq
>
> drivers/video/Kconfig | 2 +-
> drivers/video/xilinxfb.c | 135 +++++++++++++++++++++++------------------------
> 2 files changed, 68 insertions(+), 69 deletions(-)
>
> --
> 1.8.2.3
>
^ permalink raw reply
* Re: [PATCH] simplefb: add support for a8b8g8r8 pixel format
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-06-06 16:29 UTC (permalink / raw)
To: Stephen Warren
Cc: Alex Courbot, Tomi Valkeinen, Olof Johansson, gnurou@gmail.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <51B0B61C.2000008@wwwdotorg.org>
On 10:17 Thu 06 Jun , Stephen Warren wrote:
> On 06/06/2013 08:50 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 17:27 Thu 06 Jun , Alex Courbot wrote:
> >> On 06/06/2013 05:24 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>>
> >>> On Jun 6, 2013, at 10:12 AM, Alex Courbot <acourbot@nvidia.com> wrote:
> >>>
> >>>> On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>>>>
> >>>>> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> ...
> >>>>>> static struct simplefb_format simplefb_formats[] = {
> >>>>>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
> >>>>>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} },
> >>>>>
> >>>>> why don't you parse the string?
>
> Jean-Christophe, I'm afraid I can't tell exactly what you're arguing for.
>
> Here, you want code added to parse the string ...
>
> This has already been rejected as being over-engineered, and more than
> this simple driver needs. Even if it were shared code, the only
> practical use of such a parsing function would be to support this
> driver, since presumably any other HW-specific driver already knows
> exactly which format the FB is in, and hence wouldn't need to share this
> code.
>
> >>>>> so you will a real generic bindings
> >>>>
> >>>> Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330
> >>>>
> >>>> The list of modes of this driver should not grow too big. Even in terms of footprint I'd say the list should remain smaller than the parsing code.
> >>>>
> >>>> What we can discuss though is whether we want to keep this a8b8g8r8 syntax or switch to something more standard, say "rgba8888".
> >>>
> >>> I'm going to be very honest I do not like the simplefb driver from the beginning
> >>> but I do found it useful. And as said in it's name it need to be *SIMPLE*
> >>>
> >>> Then a huge list of compatible no way
> >>> otherwise we drop this from the simplefb and make it a generic helper
> >>>
> >>> I do not want to see format parser in every drivers this need to handle at video framework level
>
> ... yet here you appear to be arguing against using a format parser, or
> including a format parser ...
>
> Note that a lookup table isn't any kind of shared parser; it's just a
> very tiny and simple table of static data. It seems quite unlikely that
> this could be a maintenance issue, even if over time a few more entries
> get added to the table.
>
> >>> If I see that we start to increase again and again the simplefb I will not accept
> >>> to merge the code as we must keep it simple
> >>
> >> In that case it's probably better to maintain a "simple" list of
> >> supported modes, which is what this patch does.
> >
> > so get out it of the simplefb other drivers can use it
>
> ... yet here you appear to want to move the list of modes into some
> central location ...
>
> I don't think that's useful for the reason I mentioned above: presumably
> any other HW-specific driver already knows exactly which format the FB
> is in, and hence wouldn't need to share this code/table.
>
> Why don't we simply take this patch to extend this table, and then *if*
> any other FB driver needs to parse a format from DT, we can move the
> code(table) to a common location at that time. That will be a trivial
> change, and one this patch does nothing to make any harder. Making the
> code/table common before then seems like over-engineering.
that why I said *if I see that we start ....*
I did reject this patch but put a warning I do not want to see a huge table
if so => factorisation or parser
Best Regards,
J.
^ 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