* Re: [RFC] Add co-maintainer for fbdev
From: Andrew Morton @ 2013-05-29 20:53 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Arnd Bergmann', 'Florian Tobias Schandinat',
'Olof Johansson',
'Jean-Christophe PLAGNIOL-VILLARD', linux-fbdev,
'Linus Torvalds', linux-kernel, 'Tomi Valkeinen'
In-Reply-To: <002301ce5c0c$5552b650$fff822f0$@samsung.com>
On Wed, 29 May 2013 10:32:08 +0900 Jingoo Han <jg1.han@samsung.com> wrote:
> On Wednesday, May 29, 2013 6:20 AM, Arnd Bergmann wrote:
> > On Monday 27 May 2013, Florian Tobias Schandinat wrote:
> > > >> On Fri, May 24, 2013 at 8:38 AM, Jean-Christophe PLAGNIOL-VILLARD
> > > >> <plagnioj@jcrosoft.com> wrote:
> > > >>> Hi Florian,
> > > >>>
> > > >>> As you seems very busy I'd like to propose the help you to handle the
> > > >>> fbdev subsystem to easier the rich of the fbdev patch to Linus
> > > >>>
> > > >>> As I'm working on fbdev on at91 and others and already Co-Maintain the
> > > >>> at91 mach on ARM
> > > >>>
> > > >>> And if you are not willing to continue I could take over
> > >
> > > Yeah, it would be great if you could help, at the moment I get barely
> > > any sleep, let alone that I could keep up with the majority of mail I
> > > get. I'll let you decide whether you want to be sole maintainer or not.
> >
> > For any bigger subsystems, it's good practice to have two maintainers,
> > even if one of them does all the work. I think it makes sense to have
> > Jean-Christophe as the primary maintainer and owner of the git tree,
> > but it may also be good to have you or Tomi listed as a second maintainer
> > for times when Jean-Christophe isn't available.
>
> Hi Arnd,
>
> I agree with Arnd's opinion.
> It looks good to have Florian or Tomi listed as a second maintainer.
Someone send a patch!
^ permalink raw reply
* Re: [PATCH 1/8] video: atmel_lcdfb: fix platform data struct
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-29 19:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACQ1gAhhsMw1-Znd8BC=A0rJBrB4pphCtH9v9qOQR+j2dY3vCw@mail.gmail.com>
On 19:44 Wed 29 May , Richard Genoud wrote:
> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> > On 16:36 Wed 29 May , Richard Genoud wrote:
> >> 2013/4/11 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> >> > Today we mix pdata and drivers data in the struct atmel_lcdfb_info
> >> > Fix it and introduce a new struct atmel_lcdfb_pdata for platform data only
> >> >
> >> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> >> > Cc: linux-fbdev@vger.kernel.org
> >> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> >> > Cc: Andrew Morton <akpm@linux-foundation.org>
> >> > Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
> >> > ---
> >> > arch/arm/mach-at91/at91sam9261_devices.c | 6 +-
> >> > arch/arm/mach-at91/at91sam9263_devices.c | 6 +-
> >> > arch/arm/mach-at91/at91sam9g45_devices.c | 6 +-
> >> > arch/arm/mach-at91/at91sam9rl_devices.c | 6 +-
> >> > arch/arm/mach-at91/board-sam9261ek.c | 6 +-
> >> > arch/arm/mach-at91/board-sam9263ek.c | 4 +-
> >> > arch/arm/mach-at91/board-sam9m10g45ek.c | 4 +-
> >> > arch/arm/mach-at91/board-sam9rlek.c | 4 +-
> >> > arch/arm/mach-at91/board.h | 4 +-
> >> > arch/avr32/boards/atngw100/evklcd10x.c | 6 +-
> >> > arch/avr32/boards/atngw100/mrmt.c | 4 +-
> >> > arch/avr32/boards/atstk1000/atstk1000.h | 2 +-
> >> > arch/avr32/boards/atstk1000/setup.c | 2 +-
> >> > arch/avr32/boards/favr-32/setup.c | 2 +-
> >> > arch/avr32/boards/hammerhead/setup.c | 2 +-
> >> > arch/avr32/boards/merisc/display.c | 2 +-
> >> > arch/avr32/boards/mimc200/setup.c | 4 +-
> >> > arch/avr32/mach-at32ap/at32ap700x.c | 8 +--
> >> > arch/avr32/mach-at32ap/include/mach/board.h | 4 +-
> >> > drivers/video/atmel_lcdfb.c | 104 +++++++++++++++++----------
> >> > include/video/atmel_lcdc.h | 24 +------
> >> > 21 files changed, 109 insertions(+), 101 deletions(-)
> >> >
> >> [snip]
> >> > diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> >> > index c1a2914..98733cd4 100644
> >> > --- a/drivers/video/atmel_lcdfb.c
> >> > +++ b/drivers/video/atmel_lcdfb.c
> >> > @@ -20,12 +20,45 @@
> >> > #include <linux/gfp.h>
> >> > #include <linux/module.h>
> >> > #include <linux/platform_data/atmel.h>
> >> > +#include <video/of_display_timing.h>
> >> >
> >> > #include <mach/cpu.h>
> >> > #include <asm/gpio.h>
> >> >
> >> > #include <video/atmel_lcdc.h>
> >> >
> >> > +struct atmel_lcdfb_config {
> >> > + bool have_alt_pixclock;
> >> > + bool have_hozval;
> >> > + bool have_intensity_bit;
> >> > +};
> >> > +
> >> > + /* LCD Controller info data structure, stored in device platform_data */
> >> > +struct atmel_lcdfb_info {
> >> > + spinlock_t lock;
> >> > + struct fb_info *info;
> >> > + void __iomem *mmio;
> >> > + int irq_base;
> >> > + struct work_struct task;
> >> > +
> >> > + unsigned int smem_len;
> >> > + struct platform_device *pdev;
> >> > + struct clk *bus_clk;
> >> > + struct clk *lcdc_clk;
> >> > +
> >> > + struct backlight_device *backlight;
> >> > + u8 bl_power;
> >> > + bool lcdcon_pol_negative;
> >> I think lcdcon_pol_negative should be part of pdata, because it really
> >> depends on how the PWM is wired on the board.
> >>
> >
> > maybe but no one mainline use it on any pdata for non-dt boars
> > so I did not want to expose it
> Well, at least, I'm using it :)
> (and I guess that Andreas is using it also, otherwise he wouldn't have
> introduce it !)
yes but pdata is for non-dt boards, for dt you can keep it in struct
atmel_lcdfb_info and add a property
if non-dt boards want it my answer is I do not care switch to DT
Best Regards,
J.
>
>
> Best regards,
> Richard.
^ permalink raw reply
* Re: [PATCH] fbdev: bfin-lq035q1-fb: Use dev_pm_ops
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-29 19:20 UTC (permalink / raw)
To: Michael Hennerich, Rafael J. Wysocki
Cc: Lars-Peter Clausen, Tomi Valkeinen, linux-fbdev, linux-pm
In-Reply-To: <51A60F83.7070403@analog.com>
On 16:24 Wed 29 May , Michael Hennerich wrote:
> On 05/29/2013 02:17 PM, Lars-Peter Clausen wrote:
> >Use dev_pm_ops instead of the legacy suspend/resume callbacks.
> >
> >Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Acked-by: Michael Hennerich <michael.hennerich@analog.com>
> >---
> > drivers/video/bfin-lq035q1-fb.c | 22 ++++++++++++++--------
> > 1 file changed, 14 insertions(+), 8 deletions(-)
> >
> >diff --git a/drivers/video/bfin-lq035q1-fb.c b/drivers/video/bfin-lq035q1-fb.c
> >index 29d8c04..6084c17 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,9 +190,13 @@ static int lq035q1_spidev_resume(struct spi_device *spi)
> > return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_ON);
> > }
> >+
> >+static SIMPLE_DEV_PM_OPS(lq035q1_spidev_pm_ops, lq035q1_spidev_suspend,
> >+ lq035q1_spidev_resume);
> >+#define LQ035Q1_SPIDEV_PM_OPS (&lq035q1_spidev_pm_ops)
> >+
> > #else
> >-# define lq035q1_spidev_suspend NULL
> >-# define lq035q1_spidev_resume NULL
> >+#define LQ035Q1_SPIDEV_PM_OPS NULL
> > #endif
we really need to ahve a macro like for DT of_match_ptr to drop the #else
I known it's related to this patch but someone should do it
Rafael what do you think
Best Regards,
J.
> > /* Power down all displays on reboot, poweroff or halt */
> >@@ -708,8 +715,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 = LQ035Q1_SPIDEV_PM_OPS;
> > ret = spi_register_driver(&info->spidrv);
> > if (ret < 0) {
>
>
> --
> Greetings,
> Michael
>
> --
> Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
> Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
> Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
> Margaret Seif
>
>
^ permalink raw reply
* Re: [PATCH 1/8] video: atmel_lcdfb: fix platform data struct
From: Richard Genoud @ 2013-05-29 17:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130529173538.GC23899@game.jcrosoft.org>
2013/5/29 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> On 16:36 Wed 29 May , Richard Genoud wrote:
>> 2013/4/11 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
>> > Today we mix pdata and drivers data in the struct atmel_lcdfb_info
>> > Fix it and introduce a new struct atmel_lcdfb_pdata for platform data only
>> >
>> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>> > Cc: linux-fbdev@vger.kernel.org
>> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>> > Cc: Andrew Morton <akpm@linux-foundation.org>
>> > Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
>> > ---
>> > arch/arm/mach-at91/at91sam9261_devices.c | 6 +-
>> > arch/arm/mach-at91/at91sam9263_devices.c | 6 +-
>> > arch/arm/mach-at91/at91sam9g45_devices.c | 6 +-
>> > arch/arm/mach-at91/at91sam9rl_devices.c | 6 +-
>> > arch/arm/mach-at91/board-sam9261ek.c | 6 +-
>> > arch/arm/mach-at91/board-sam9263ek.c | 4 +-
>> > arch/arm/mach-at91/board-sam9m10g45ek.c | 4 +-
>> > arch/arm/mach-at91/board-sam9rlek.c | 4 +-
>> > arch/arm/mach-at91/board.h | 4 +-
>> > arch/avr32/boards/atngw100/evklcd10x.c | 6 +-
>> > arch/avr32/boards/atngw100/mrmt.c | 4 +-
>> > arch/avr32/boards/atstk1000/atstk1000.h | 2 +-
>> > arch/avr32/boards/atstk1000/setup.c | 2 +-
>> > arch/avr32/boards/favr-32/setup.c | 2 +-
>> > arch/avr32/boards/hammerhead/setup.c | 2 +-
>> > arch/avr32/boards/merisc/display.c | 2 +-
>> > arch/avr32/boards/mimc200/setup.c | 4 +-
>> > arch/avr32/mach-at32ap/at32ap700x.c | 8 +--
>> > arch/avr32/mach-at32ap/include/mach/board.h | 4 +-
>> > drivers/video/atmel_lcdfb.c | 104 +++++++++++++++++----------
>> > include/video/atmel_lcdc.h | 24 +------
>> > 21 files changed, 109 insertions(+), 101 deletions(-)
>> >
>> [snip]
>> > diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
>> > index c1a2914..98733cd4 100644
>> > --- a/drivers/video/atmel_lcdfb.c
>> > +++ b/drivers/video/atmel_lcdfb.c
>> > @@ -20,12 +20,45 @@
>> > #include <linux/gfp.h>
>> > #include <linux/module.h>
>> > #include <linux/platform_data/atmel.h>
>> > +#include <video/of_display_timing.h>
>> >
>> > #include <mach/cpu.h>
>> > #include <asm/gpio.h>
>> >
>> > #include <video/atmel_lcdc.h>
>> >
>> > +struct atmel_lcdfb_config {
>> > + bool have_alt_pixclock;
>> > + bool have_hozval;
>> > + bool have_intensity_bit;
>> > +};
>> > +
>> > + /* LCD Controller info data structure, stored in device platform_data */
>> > +struct atmel_lcdfb_info {
>> > + spinlock_t lock;
>> > + struct fb_info *info;
>> > + void __iomem *mmio;
>> > + int irq_base;
>> > + struct work_struct task;
>> > +
>> > + unsigned int smem_len;
>> > + struct platform_device *pdev;
>> > + struct clk *bus_clk;
>> > + struct clk *lcdc_clk;
>> > +
>> > + struct backlight_device *backlight;
>> > + u8 bl_power;
>> > + bool lcdcon_pol_negative;
>> I think lcdcon_pol_negative should be part of pdata, because it really
>> depends on how the PWM is wired on the board.
>>
>
> maybe but no one mainline use it on any pdata for non-dt boars
> so I did not want to expose it
Well, at least, I'm using it :)
(and I guess that Andreas is using it also, otherwise he wouldn't have
introduce it !)
Best regards,
Richard.
^ permalink raw reply
* Re: [PATCH 1/8] video: atmel_lcdfb: fix platform data struct
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-29 17:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACQ1gAjF5kuu-czkj56-pQdTBUhvY5VodvfnF3RA_DN-O0b9NQ@mail.gmail.com>
On 16:36 Wed 29 May , Richard Genoud wrote:
> 2013/4/11 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> > Today we mix pdata and drivers data in the struct atmel_lcdfb_info
> > Fix it and introduce a new struct atmel_lcdfb_pdata for platform data only
> >
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: linux-fbdev@vger.kernel.org
> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
> > ---
> > arch/arm/mach-at91/at91sam9261_devices.c | 6 +-
> > arch/arm/mach-at91/at91sam9263_devices.c | 6 +-
> > arch/arm/mach-at91/at91sam9g45_devices.c | 6 +-
> > arch/arm/mach-at91/at91sam9rl_devices.c | 6 +-
> > arch/arm/mach-at91/board-sam9261ek.c | 6 +-
> > arch/arm/mach-at91/board-sam9263ek.c | 4 +-
> > arch/arm/mach-at91/board-sam9m10g45ek.c | 4 +-
> > arch/arm/mach-at91/board-sam9rlek.c | 4 +-
> > arch/arm/mach-at91/board.h | 4 +-
> > arch/avr32/boards/atngw100/evklcd10x.c | 6 +-
> > arch/avr32/boards/atngw100/mrmt.c | 4 +-
> > arch/avr32/boards/atstk1000/atstk1000.h | 2 +-
> > arch/avr32/boards/atstk1000/setup.c | 2 +-
> > arch/avr32/boards/favr-32/setup.c | 2 +-
> > arch/avr32/boards/hammerhead/setup.c | 2 +-
> > arch/avr32/boards/merisc/display.c | 2 +-
> > arch/avr32/boards/mimc200/setup.c | 4 +-
> > arch/avr32/mach-at32ap/at32ap700x.c | 8 +--
> > arch/avr32/mach-at32ap/include/mach/board.h | 4 +-
> > drivers/video/atmel_lcdfb.c | 104 +++++++++++++++++----------
> > include/video/atmel_lcdc.h | 24 +------
> > 21 files changed, 109 insertions(+), 101 deletions(-)
> >
> [snip]
> > diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> > index c1a2914..98733cd4 100644
> > --- a/drivers/video/atmel_lcdfb.c
> > +++ b/drivers/video/atmel_lcdfb.c
> > @@ -20,12 +20,45 @@
> > #include <linux/gfp.h>
> > #include <linux/module.h>
> > #include <linux/platform_data/atmel.h>
> > +#include <video/of_display_timing.h>
> >
> > #include <mach/cpu.h>
> > #include <asm/gpio.h>
> >
> > #include <video/atmel_lcdc.h>
> >
> > +struct atmel_lcdfb_config {
> > + bool have_alt_pixclock;
> > + bool have_hozval;
> > + bool have_intensity_bit;
> > +};
> > +
> > + /* LCD Controller info data structure, stored in device platform_data */
> > +struct atmel_lcdfb_info {
> > + spinlock_t lock;
> > + struct fb_info *info;
> > + void __iomem *mmio;
> > + int irq_base;
> > + struct work_struct task;
> > +
> > + unsigned int smem_len;
> > + struct platform_device *pdev;
> > + struct clk *bus_clk;
> > + struct clk *lcdc_clk;
> > +
> > + struct backlight_device *backlight;
> > + u8 bl_power;
> > + bool lcdcon_pol_negative;
> I think lcdcon_pol_negative should be part of pdata, because it really
> depends on how the PWM is wired on the board.
>
maybe but no one mainline use it on any pdata for non-dt boars
so I did not want to expose it
Best Regatgards,
J.
>
> Regards,
> Richard.
^ permalink raw reply
* Re: [PATCH 1/3] video: xilinxfb: Fix OF probing on little-endian systems
From: Sören Brinkmann @ 2013-05-29 16:18 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, Michal Simek, Pallav Joshi, git-dev,
Florian Tobias Schandinat, linux-fbdev
In-Reply-To: <502367ddf6d79b7b343b3165ad54cb9e2d4713db.1369843969.git.michal.simek@xilinx.com>
On Wed, May 29, 2013 at 06:12:56PM +0200, Michal Simek wrote:
> From: Michal Simek <monstr@monstr.eu>
>
> DTB is always big-endian that's why is necessary
> to convert it.
>
> Signed-off-by: Michal Simek <monstr@monstr.eu>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> drivers/video/xilinxfb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
> index af0b4fd..5af341e 100644
> --- a/drivers/video/xilinxfb.c
> +++ b/drivers/video/xilinxfb.c
> @@ -428,7 +428,7 @@ static int xilinxfb_of_probe(struct platform_device *op)
> * interface and initialize the tft_access accordingly.
> */
> p = (u32 *)of_get_property(op->dev.of_node, "xlnx,dcr-splb-slave-if", NULL);
You should consider using 'of_property_read_u32() (or one of its other
variants for arrays, strings, etc). That already implicitly does the
endian conversion.
Sören
^ permalink raw reply
* [PATCH 3/3] video: xilinxfb: Use driver for Xilinx ARM Zynq
From: Michal Simek @ 2013-05-29 16:12 UTC (permalink / raw)
To: linux-kernel
Cc: Michal Simek, Michal Simek, Pallav Joshi, git-dev,
Florian Tobias Schandinat, linux-fbdev
In-Reply-To: <502367ddf6d79b7b343b3165ad54cb9e2d4713db.1369843969.git.michal.simek@xilinx.com>
[-- Attachment #1: Type: text/plain, Size: 723 bytes --]
From: Michal Simek <monstr@monstr.eu>
Enable this driver for all Xilinx platforms.
Signed-off-by: Michal Simek <monstr@monstr.eu>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
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
* [PATCH 2/3] video: xilinxfb: Do not use out_be32 IO function
From: Michal Simek @ 2013-05-29 16:12 UTC (permalink / raw)
To: linux-kernel
Cc: Michal Simek, Michal Simek, Pallav Joshi, git-dev,
Florian Tobias Schandinat, linux-fbdev
In-Reply-To: <502367ddf6d79b7b343b3165ad54cb9e2d4713db.1369843969.git.michal.simek@xilinx.com>
[-- Attachment #1: Type: text/plain, Size: 3194 bytes --]
out_be32 IO function is not supported by ARM.
It is only available for PPC and Microblaze.
Remove all out_be32 references and start to use __raw_writel
function.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/video/xilinxfb.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 5af341e..dcf0552 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -57,7 +57,7 @@
* In case of direct PLB access the second control register will be at
* an offset of 4 as compared to the DCR access where the offset is 1
* i.e. REG_CTRL. So this is taken care in the function
- * xilinx_fb_out_be32 where it left shifts the offset 2 times in case of
+ * xilinx_fb_out32 where it left shifts the offset 2 times in case of
* direct PLB access.
*/
#define NUM_REGS 2
@@ -150,11 +150,11 @@ struct xilinxfb_drvdata {
* To perform the read/write on the registers we need to check on
* which bus its connected and call the appropriate write API.
*/
-static void xilinx_fb_out_be32(struct xilinxfb_drvdata *drvdata, u32 offset,
+static void xilinx_fb_out32(struct xilinxfb_drvdata *drvdata, u32 offset,
u32 val)
{
if (drvdata->flags & PLB_ACCESS_FLAG)
- out_be32(drvdata->regs + (offset << 2), val);
+ __raw_writel(val, drvdata->regs + (offset << 2));
#ifdef CONFIG_PPC_DCR
else
dcr_write(drvdata->dcr_host, offset, val);
@@ -197,7 +197,7 @@ xilinx_fb_blank(int blank_mode, struct fb_info *fbi)
switch (blank_mode) {
case FB_BLANK_UNBLANK:
/* turn on panel */
- xilinx_fb_out_be32(drvdata, REG_CTRL, drvdata->reg_ctrl_default);
+ xilinx_fb_out32(drvdata, REG_CTRL, drvdata->reg_ctrl_default);
break;
case FB_BLANK_NORMAL:
@@ -205,7 +205,7 @@ xilinx_fb_blank(int blank_mode, struct fb_info *fbi)
case FB_BLANK_HSYNC_SUSPEND:
case FB_BLANK_POWERDOWN:
/* turn off panel */
- xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+ xilinx_fb_out32(drvdata, REG_CTRL, 0);
default:
break;
@@ -280,13 +280,13 @@ static int xilinxfb_assign(struct device *dev,
memset_io((void __iomem *)drvdata->fb_virt, 0, fbsize);
/* Tell the hardware where the frame buffer is */
- xilinx_fb_out_be32(drvdata, REG_FB_ADDR, drvdata->fb_phys);
+ xilinx_fb_out32(drvdata, REG_FB_ADDR, drvdata->fb_phys);
/* Turn on the display */
drvdata->reg_ctrl_default = REG_CTRL_ENABLE;
if (pdata->rotate_screen)
drvdata->reg_ctrl_default |= REG_CTRL_ROTATE;
- xilinx_fb_out_be32(drvdata, REG_CTRL,
+ xilinx_fb_out32(drvdata, REG_CTRL,
drvdata->reg_ctrl_default);
/* Fill struct fb_info */
@@ -345,7 +345,7 @@ err_cmap:
iounmap(drvdata->fb_virt);
/* Turn off the display */
- xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+ xilinx_fb_out32(drvdata, REG_CTRL, 0);
err_fbmem:
if (drvdata->flags & PLB_ACCESS_FLAG)
@@ -381,7 +381,7 @@ static int xilinxfb_release(struct device *dev)
iounmap(drvdata->fb_virt);
/* Turn off the display */
- xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+ xilinx_fb_out32(drvdata, REG_CTRL, 0);
/* Release the resources, as allocated based on interface */
if (drvdata->flags & PLB_ACCESS_FLAG) {
--
1.8.2.3
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply related
* [PATCH 1/3] video: xilinxfb: Fix OF probing on little-endian systems
From: Michal Simek @ 2013-05-29 16:12 UTC (permalink / raw)
To: linux-kernel
Cc: Michal Simek, Michal Simek, Pallav Joshi, git-dev,
Florian Tobias Schandinat, linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 813 bytes --]
From: Michal Simek <monstr@monstr.eu>
DTB is always big-endian that's why is necessary
to convert it.
Signed-off-by: Michal Simek <monstr@monstr.eu>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/video/xilinxfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index af0b4fd..5af341e 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -428,7 +428,7 @@ static int xilinxfb_of_probe(struct platform_device *op)
* interface and initialize the tft_access accordingly.
*/
p = (u32 *)of_get_property(op->dev.of_node, "xlnx,dcr-splb-slave-if", NULL);
- tft_access = p ? *p : 0;
+ tft_access = p ? be32_to_cpup(p) : 0;
/*
* Fill the resource structure if its direct PLB interface
--
1.8.2.3
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply related
* Re: [PATCH 4/8] video: atmel_lcdfb: add device tree suport
From: Richard Genoud @ 2013-05-29 15:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1365692422-9565-4-git-send-email-plagnioj@jcrosoft.com>
2013/4/11 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> get display timings from device tree
> Use videomode helpers to get display timings and configurations from
> device tree
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
> .../devicetree/bindings/video/atmel,lcdc.txt | 75 ++++++
> drivers/video/Kconfig | 2 +
> drivers/video/atmel_lcdfb.c | 244 +++++++++++++++++---
> 3 files changed, 289 insertions(+), 32 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/video/atmel,lcdc.txt
>
> diff --git a/Documentation/devicetree/bindings/video/atmel,lcdc.txt b/Documentation/devicetree/bindings/video/atmel,lcdc.txt
> new file mode 100644
> index 0000000..1ec175e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/atmel,lcdc.txt
> @@ -0,0 +1,75 @@
> +Atmel LCDC Framebuffer
> +-----------------------------------------------------
> +
> +Required properties:
> +- compatible :
> + "atmel,at91sam9261-lcdc" ,
> + "atmel,at91sam9263-lcdc" ,
> + "atmel,at91sam9g10-lcdc" ,
> + "atmel,at91sam9g45-lcdc" ,
> + "atmel,at91sam9g45es-lcdc" ,
> + "atmel,at91sam9rl-lcdc" ,
> + "atmel,at32ap-lcdc"
> +- reg : Should contain 1 register ranges(address and length)
> +- interrupts : framebuffer controller interrupt
> +- display: a phandle pointing to the display node
> +
> +Required nodes:
> +- display: a display node is required to initialize the lcd panel
> + This should be in the board dts.
> +- default-mode: a videomode within the display with timing parameters
> + as specified below.
> +
> +Example:
> +
> + fb0: fb@0x00500000 {
> + compatible = "atmel,at91sam9g45-lcdc";
> + reg = <0x00500000 0x1000>;
> + interrupts = <23 3 0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_fb>;
> + display = <&display0>;
> + status = "okay";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + };
> +
> +Atmel LCDC Display
> +-----------------------------------------------------
> +Required properties (as per of_videomode_helper):
> +
> + - atmel,dmacon: dma controler configuration
> + - atmel,lcdcon2: lcd controler configuration
> + - atmel,guard-time: lcd guard time (Delay in frame periods)
> + - bits-per-pixel: lcd panel bit-depth.
> +
> +Optional properties (as per of_videomode_helper):
> + - atmel,lcdcon-backlight: enable backlight
> + - atmel,lcd-wiring-mode: lcd wiring mode "RGB" or "BRG"
> + - atmel,power-control-gpio: gpio to power on or off the LCD (as many as needed)
still on lcdcon_pol_negative, we can add something like that:
- atmel,lcdcon-pwm-pulse-low: Output PWM pulses are low level (high
level if not set)
> +
> +Example:
> + display0: display {
> + bits-per-pixel = <32>;
> + atmel,lcdcon-backlight;
> + atmel,dmacon = <0x1>;
> + atmel,lcdcon2 = <0x80008002>;
> + atmel,guard-time = <9>;
> + atmel,lcd-wiring-mode = <1>;
> +
> + display-timings {
> + native-mode = <&timing0>;
> + timing0: timing0 {
> + clock-frequency = <9000000>;
> + hactive = <480>;
> + vactive = <272>;
> + hback-porch = <1>;
> + hfront-porch = <1>;
> + vback-porch = <40>;
> + vfront-porch = <1>;
> + hsync-len = <45>;
> + vsync-len = <1>;
> + };
> + };
> + };
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 4c1546f..0687482 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -1018,6 +1018,8 @@ config FB_ATMEL
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> + select FB_MODE_HELPERS
> + select OF_VIDEOMODE
> help
> This enables support for the AT91/AT32 LCD Controller.
>
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index f67e226..4a31570 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -20,7 +20,11 @@
> #include <linux/gfp.h>
> #include <linux/module.h>
> #include <linux/platform_data/atmel.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> #include <video/of_display_timing.h>
> +#include <video/videomode.h>
>
> #include <mach/cpu.h>
> #include <asm/gpio.h>
> @@ -59,6 +63,13 @@ struct atmel_lcdfb_info {
> struct atmel_lcdfb_config *config;
> };
>
> +struct atmel_lcdfb_power_ctrl_gpio {
> + int gpio;
> + int active_low;
> +
> + struct list_head list;
> +};
> +
> #define lcdc_readl(sinfo, reg) __raw_readl((sinfo)->mmio+(reg))
> #define lcdc_writel(sinfo, reg, val) __raw_writel((val), (sinfo)->mmio+(reg))
>
> @@ -945,16 +956,187 @@ static void atmel_lcdfb_stop_clock(struct atmel_lcdfb_info *sinfo)
> clk_disable(sinfo->lcdc_clk);
> }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id atmel_lcdfb_dt_ids[] = {
> + { .compatible = "atmel,at91sam9261-lcdc" , .data = &at91sam9261_config, },
> + { .compatible = "atmel,at91sam9263-lcdc" , .data = &at91sam9263_config, },
> + { .compatible = "atmel,at91sam9g10-lcdc" , .data = &at91sam9g10_config, },
> + { .compatible = "atmel,at91sam9g45-lcdc" , .data = &at91sam9g45_config, },
> + { .compatible = "atmel,at91sam9g45es-lcdc" , .data = &at91sam9g45es_config, },
> + { .compatible = "atmel,at91sam9rl-lcdc" , .data = &at91sam9rl_config, },
> + { .compatible = "atmel,at32ap-lcdc" , .data = &at32ap_config, },
> + { /* sentinel */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, atmel_lcdfb_dt_ids);
> +
> +static const char *atmel_lcdfb_wiring_modes[] = {
> + [ATMEL_LCDC_WIRING_BGR] = "BRG",
> + [ATMEL_LCDC_WIRING_RGB] = "RGB",
> +};
> +
> +const int atmel_lcdfb_get_of_wiring_modes(struct device_node *np)
> +{
> + const char *mode;
> + int err, i;
> +
> + err = of_property_read_string(np, "atmel,lcd-wiring-mode", &mode);
> + if (err < 0)
> + return ATMEL_LCDC_WIRING_BGR;
> +
> + for (i = 0; i < ARRAY_SIZE(atmel_lcdfb_wiring_modes); i++)
> + if (!strcasecmp(mode, atmel_lcdfb_wiring_modes[i]))
> + return i;
> +
> + return -ENODEV;
> +}
> +
> +static void atmel_lcdfb_power_control_gpio(struct atmel_lcdfb_pdata *pdata, int on)
> +{
> + struct atmel_lcdfb_power_ctrl_gpio *og;
> +
> + list_for_each_entry(og, &pdata->pwr_gpios, list)
> + gpio_set_value(og->gpio, on);
> +}
> +
> +static int atmel_lcdfb_of_init(struct atmel_lcdfb_info *sinfo)
> +{
> + struct fb_info *info = sinfo->info;
> + struct atmel_lcdfb_pdata *pdata = &sinfo->pdata;
> + struct fb_var_screeninfo *var = &info->var;
> + struct device *dev = &sinfo->pdev->dev;
> + struct device_node *np Þv->of_node;
> + struct device_node *display_np;
> + struct device_node *timings_np;
> + struct display_timings *timings;
> + enum of_gpio_flags flags;
> + struct atmel_lcdfb_power_ctrl_gpio *og;
> + bool is_gpio_power = false;
> + int ret = -ENOENT;
> + int i, gpio;
> +
> + sinfo->config = (struct atmel_lcdfb_config*)
> + of_match_device(atmel_lcdfb_dt_ids, dev)->data;
> +
> + display_np = of_parse_phandle(np, "display", 0);
> + if (!display_np) {
> + dev_err(dev, "failed to find display phandle\n");
> + return -ENOENT;
> + }
> +
> + ret = of_property_read_u32(display_np, "bits-per-pixel", &var->bits_per_pixel);
> + if (ret < 0) {
> + dev_err(dev, "failed to get property bits-per-pixel\n");
> + goto put_display_node;
> + }
> +
> + ret = of_property_read_u32(display_np, "atmel,guard-time", &pdata->guard_time);
> + if (ret < 0) {
> + dev_err(dev, "failed to get property atmel,guard-time\n");
> + goto put_display_node;
> + }
> +
> + ret = of_property_read_u32(display_np, "atmel,lcdcon2", &pdata->default_lcdcon2);
> + if (ret < 0) {
> + dev_err(dev, "failed to get property atmel,lcdcon2\n");
> + goto put_display_node;
> + }
> +
> + ret = of_property_read_u32(display_np, "atmel,dmacon", &pdata->default_dmacon);
> + if (ret < 0) {
> + dev_err(dev, "failed to get property bits-per-pixel\n");
> + goto put_display_node;
> + }
> +
> + ret = -ENOMEM;
> + for (i = 0; i < of_gpio_named_count(display_np, "atmel,power-control-gpio"); i++) {
> + gpio = of_get_named_gpio_flags(display_np, "atmel,power-control-gpio",
> + i, &flags);
> + if (gpio < 0)
> + continue;
> +
> + og = devm_kzalloc(dev, sizeof(*og), GFP_KERNEL);
> + if (!og)
> + goto put_display_node;
> +
> + og->gpio = gpio;
> + og->active_low = flags & OF_GPIO_ACTIVE_LOW;
> + is_gpio_power = true;
> + ret = devm_gpio_request(dev, gpio, "lcd-power-control-gpio");
> + if (ret) {
> + dev_err(dev, "request gpio %d failed\n", gpio);
> + goto put_display_node;
> + }
> +
> + ret = gpio_direction_output(gpio, og->active_low);
> + if (ret) {
> + dev_err(dev, "set direction output gpio %d failed\n", gpio);
> + goto put_display_node;
> + }
> + }
> +
> + if (is_gpio_power)
> + pdata->atmel_lcdfb_power_control = atmel_lcdfb_power_control_gpio;
> +
> + ret = atmel_lcdfb_get_of_wiring_modes(display_np);
> + if (ret < 0) {
> + dev_err(dev, "invalid atmel,lcd-wiring-mode\n");
> + goto put_display_node;
> + }
> + pdata->lcd_wiring_mode = ret;
> +
> + pdata->lcdcon_is_backlight = of_property_read_bool(display_np, "atmel,lcdcon-backlight");
and here, something like:
pdata->lcdcon_pol_negative = of_property_read_bool(display_np,
"atmel,lcdcon-pwm-pulse-low");
would be nice
Regards,
Richard.
^ permalink raw reply
* Re: [PATCH 1/8] video: atmel_lcdfb: fix platform data struct
From: Richard Genoud @ 2013-05-29 14:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1365692422-9565-1-git-send-email-plagnioj@jcrosoft.com>
2013/4/11 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> Today we mix pdata and drivers data in the struct atmel_lcdfb_info
> Fix it and introduce a new struct atmel_lcdfb_pdata for platform data only
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
> ---
> arch/arm/mach-at91/at91sam9261_devices.c | 6 +-
> arch/arm/mach-at91/at91sam9263_devices.c | 6 +-
> arch/arm/mach-at91/at91sam9g45_devices.c | 6 +-
> arch/arm/mach-at91/at91sam9rl_devices.c | 6 +-
> arch/arm/mach-at91/board-sam9261ek.c | 6 +-
> arch/arm/mach-at91/board-sam9263ek.c | 4 +-
> arch/arm/mach-at91/board-sam9m10g45ek.c | 4 +-
> arch/arm/mach-at91/board-sam9rlek.c | 4 +-
> arch/arm/mach-at91/board.h | 4 +-
> arch/avr32/boards/atngw100/evklcd10x.c | 6 +-
> arch/avr32/boards/atngw100/mrmt.c | 4 +-
> arch/avr32/boards/atstk1000/atstk1000.h | 2 +-
> arch/avr32/boards/atstk1000/setup.c | 2 +-
> arch/avr32/boards/favr-32/setup.c | 2 +-
> arch/avr32/boards/hammerhead/setup.c | 2 +-
> arch/avr32/boards/merisc/display.c | 2 +-
> arch/avr32/boards/mimc200/setup.c | 4 +-
> arch/avr32/mach-at32ap/at32ap700x.c | 8 +--
> arch/avr32/mach-at32ap/include/mach/board.h | 4 +-
> drivers/video/atmel_lcdfb.c | 104 +++++++++++++++++----------
> include/video/atmel_lcdc.h | 24 +------
> 21 files changed, 109 insertions(+), 101 deletions(-)
>
[snip]
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index c1a2914..98733cd4 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -20,12 +20,45 @@
> #include <linux/gfp.h>
> #include <linux/module.h>
> #include <linux/platform_data/atmel.h>
> +#include <video/of_display_timing.h>
>
> #include <mach/cpu.h>
> #include <asm/gpio.h>
>
> #include <video/atmel_lcdc.h>
>
> +struct atmel_lcdfb_config {
> + bool have_alt_pixclock;
> + bool have_hozval;
> + bool have_intensity_bit;
> +};
> +
> + /* LCD Controller info data structure, stored in device platform_data */
> +struct atmel_lcdfb_info {
> + spinlock_t lock;
> + struct fb_info *info;
> + void __iomem *mmio;
> + int irq_base;
> + struct work_struct task;
> +
> + unsigned int smem_len;
> + struct platform_device *pdev;
> + struct clk *bus_clk;
> + struct clk *lcdc_clk;
> +
> + struct backlight_device *backlight;
> + u8 bl_power;
> + bool lcdcon_pol_negative;
I think lcdcon_pol_negative should be part of pdata, because it really
depends on how the PWM is wired on the board.
Regards,
Richard.
^ permalink raw reply
* Re: [PATCH] fbdev: bfin-lq035q1-fb: Use dev_pm_ops
From: Michael Hennerich @ 2013-05-29 14:24 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1369829859-13405-1-git-send-email-lars@metafoo.de>
On 05/29/2013 02:17 PM, Lars-Peter Clausen wrote:
> Use dev_pm_ops instead of the legacy suspend/resume callbacks.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Michael Hennerich <michael.hennerich@analog.com>
> ---
> drivers/video/bfin-lq035q1-fb.c | 22 ++++++++++++++--------
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/video/bfin-lq035q1-fb.c b/drivers/video/bfin-lq035q1-fb.c
> index 29d8c04..6084c17 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,9 +190,13 @@ static int lq035q1_spidev_resume(struct spi_device *spi)
>
> return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_ON);
> }
> +
> +static SIMPLE_DEV_PM_OPS(lq035q1_spidev_pm_ops, lq035q1_spidev_suspend,
> + lq035q1_spidev_resume);
> +#define LQ035Q1_SPIDEV_PM_OPS (&lq035q1_spidev_pm_ops)
> +
> #else
> -# define lq035q1_spidev_suspend NULL
> -# define lq035q1_spidev_resume NULL
> +#define LQ035Q1_SPIDEV_PM_OPS NULL
> #endif
>
> /* Power down all displays on reboot, poweroff or halt */
> @@ -708,8 +715,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 = LQ035Q1_SPIDEV_PM_OPS;
>
> ret = spi_register_driver(&info->spidrv);
> if (ret < 0) {
--
Greetings,
Michael
--
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif
^ permalink raw reply
* [PATCH] fbdev: bfin-lq035q1-fb: Use dev_pm_ops
From: Lars-Peter Clausen @ 2013-05-29 12:17 UTC (permalink / raw)
To: linux-fbdev
Use dev_pm_ops instead of the legacy suspend/resume callbacks.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/video/bfin-lq035q1-fb.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/video/bfin-lq035q1-fb.c b/drivers/video/bfin-lq035q1-fb.c
index 29d8c04..6084c17 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,9 +190,13 @@ static int lq035q1_spidev_resume(struct spi_device *spi)
return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_ON);
}
+
+static SIMPLE_DEV_PM_OPS(lq035q1_spidev_pm_ops, lq035q1_spidev_suspend,
+ lq035q1_spidev_resume);
+#define LQ035Q1_SPIDEV_PM_OPS (&lq035q1_spidev_pm_ops)
+
#else
-# define lq035q1_spidev_suspend NULL
-# define lq035q1_spidev_resume NULL
+#define LQ035Q1_SPIDEV_PM_OPS NULL
#endif
/* Power down all displays on reboot, poweroff or halt */
@@ -708,8 +715,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 = LQ035Q1_SPIDEV_PM_OPS;
ret = spi_register_driver(&info->spidrv);
if (ret < 0) {
--
1.8.0
^ permalink raw reply related
* [PATCH 1/1] MAINTAINERS: Framebuffer Layer maintainers update
From: plagnioj @ 2013-05-29 8:46 UTC (permalink / raw)
To: linux-kernel
Cc: Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen, Olof Johansson,
Andrew Morton, Linus Torvalds, Arnd Bergmann,
Florian Tobias Schandinat, linux-fbdev
From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Tomi and I will now take care of the Framebuffer Layer
The git tree is now on kernel.org
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Olof Johansson <olof@lixom.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: linux-fbdev@vger.kernel.org
---
MAINTAINERS | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index fd3a495..7714c3c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3322,11 +3322,12 @@ F: drivers/net/wan/dlci.c
F: drivers/net/wan/sdla.c
FRAMEBUFFER LAYER
-M: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
+M: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
+M: Tomi Valkeinen <tomi.valkeinen@ti.com>
L: linux-fbdev@vger.kernel.org
W: http://linux-fbdev.sourceforge.net/
Q: http://patchwork.kernel.org/project/linux-fbdev/list/
-T: git git://github.com/schandinat/linux-2.6.git fbdev-next
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/plagnioj/linux-fbdev.git
S: Maintained
F: Documentation/fb/
F: Documentation/devicetree/bindings/fb/
--
1.7.10.4
^ permalink raw reply related
* Fbdev changes
From: Tomi Valkeinen @ 2013-05-29 6:37 UTC (permalink / raw)
To: linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 485 bytes --]
Hi Jean-Christophe,
I've pushed the patches I had already collected for fbdev:
git://gitorious.org/linux-omap-dss2/linux.git fbdev-3.10-fixes
git://gitorious.org/linux-omap-dss2/linux.git fbdev-3.11
fbdev-3.10-fixes contains two fixes for 3.10.
fbdev-3.11 contains ssd1307fb patches from Maxime Ripard, and minor
improvement for the common videomode code from me.
Feel free to cherry pick/rebase the patches, I don't think a merge is
necessary for these.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* RE: Introduce a new helper framework for buffer synchronization
From: Inki Dae @ 2013-05-29 2:21 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <51909DB4.2060208@canonical.com>
> -----Original Message-----
> From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@ffwll.ch] On Behalf Of
> Daniel Vetter
> Sent: Wednesday, May 29, 2013 1:50 AM
> To: Inki Dae
> Cc: Rob Clark; Maarten Lankhorst; linux-fbdev; YoungJun Cho; Kyungmin
Park;
> myungjoo.ham; DRI mailing list; linux-arm-kernel@lists.infradead.org;
> linux-media@vger.kernel.org
> Subject: Re: Introduce a new helper framework for buffer synchronization
>
> On Tue, May 28, 2013 at 4:50 PM, Inki Dae <inki.dae@samsung.com> wrote:
> > I think I already used reservation stuff any time in that way except
> > ww-mutex. And I'm not sure that embedded system really needs ww-mutex.
> If
> > there is any case,
> > could you tell me the case? I really need more advice and
> understanding :)
>
> If you have only one driver, you can get away without ww_mutex.
> drm/i915 does it, all buffer state is protected by dev->struct_mutex.
> But as soon as you have multiple drivers sharing buffers with dma_buf
> things will blow up.
>
> Yep, current prime is broken and can lead to deadlocks.
>
> In practice it doesn't (yet) matter since only the X server does the
> sharing dance, and that one's single-threaded. Now you can claim that
> since you have all buffers pinned in embedded gfx anyway, you don't
> care. But both in desktop gfx and embedded gfx the real fun starts
> once you put fences into the mix and link them up with buffers, then
> every command submission risks that deadlock. Furthermore you can get
> unlucky and construct a circle of fences waiting on each another (only
> though if the fence singalling fires off the next batchbuffer
> asynchronously).
In our case, we haven't ever experienced deadlock yet but there is still
possible to face with deadlock in case that a process is sharing two buffer
with another process like below,
Process A committed buffer A and waits for buffer B,
Process B committed buffer B and waits for buffer A
That is deadlock and it seems that you say we can resolve deadlock issue
with ww-mutexes. And it seems that we can replace our block-wakeup mechanism
with mutex lock for more performance.
>
> To prevent such deadlocks you _absolutely_ need to lock _all_ buffers
> that take part in a command submission at once. To do that you either
> need a global lock (ugh) or ww_mutexes.
>
> So ww_mutexes are the fundamental ingredient of all this, if you don't
> see why you need them then everything piled on top is broken. I think
> until you've understood why exactly we need ww_mutexes there's not
> much point in discussing the finer issues of fences, reservation
> objects and how to integrate it with dma_bufs exactly.
>
> I'll try to clarify the motivating example in the ww_mutex
> documentation a bit, but I dunno how else I could explain this ...
>
I don't really want for you waste your time on me. I will trying to apply
ww-mutexes (v4) to the proposed framework for more understanding.
Thanks for your advices.:)
Inki Dae
> Yours, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [RFC] Add co-maintainer for fbdev
From: Jingoo Han @ 2013-05-29 1:32 UTC (permalink / raw)
To: 'Arnd Bergmann', 'Florian Tobias Schandinat'
Cc: 'Olof Johansson',
'Jean-Christophe PLAGNIOL-VILLARD', linux-fbdev,
'Linus Torvalds', linux-kernel, 'Tomi Valkeinen',
'Andrew Morton', Jingoo Han
In-Reply-To: <201305282319.30311.arnd@arndb.de>
On Wednesday, May 29, 2013 6:20 AM, Arnd Bergmann wrote:
> On Monday 27 May 2013, Florian Tobias Schandinat wrote:
> > >> On Fri, May 24, 2013 at 8:38 AM, Jean-Christophe PLAGNIOL-VILLARD
> > >> <plagnioj@jcrosoft.com> wrote:
> > >>> Hi Florian,
> > >>>
> > >>> As you seems very busy I'd like to propose the help you to handle the
> > >>> fbdev subsystem to easier the rich of the fbdev patch to Linus
> > >>>
> > >>> As I'm working on fbdev on at91 and others and already Co-Maintain the
> > >>> at91 mach on ARM
> > >>>
> > >>> And if you are not willing to continue I could take over
> >
> > Yeah, it would be great if you could help, at the moment I get barely
> > any sleep, let alone that I could keep up with the majority of mail I
> > get. I'll let you decide whether you want to be sole maintainer or not.
>
> For any bigger subsystems, it's good practice to have two maintainers,
> even if one of them does all the work. I think it makes sense to have
> Jean-Christophe as the primary maintainer and owner of the git tree,
> but it may also be good to have you or Tomi listed as a second maintainer
> for times when Jean-Christophe isn't available.
Hi Arnd,
I agree with Arnd's opinion.
It looks good to have Florian or Tomi listed as a second maintainer.
Best regards,
Jingoo Han
>
> I don't know how many patches you'd expect per release, maybe just one
> person is enough after all.
>
> Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC] Add co-maintainer for fbdev
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-29 0:34 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Florian Tobias Schandinat, Olof Johansson, linux-fbdev,
Linus Torvalds, linux-kernel@vger.kernel.org, Tomi Valkeinen,
Andrew Morton
In-Reply-To: <201305282319.30311.arnd@arndb.de>
On 23:19 Tue 28 May , Arnd Bergmann wrote:
> On Monday 27 May 2013, Florian Tobias Schandinat wrote:
> > >> On Fri, May 24, 2013 at 8:38 AM, Jean-Christophe PLAGNIOL-VILLARD
> > >> <plagnioj@jcrosoft.com> wrote:
> > >>> Hi Florian,
> > >>>
> > >>> As you seems very busy I'd like to propose the help you to handle the
> > >>> fbdev subsystem to easier the rich of the fbdev patch to Linus
> > >>>
> > >>> As I'm working on fbdev on at91 and others and already Co-Maintain the
> > >>> at91 mach on ARM
> > >>>
> > >>> And if you are not willing to continue I could take over
> >
> > Yeah, it would be great if you could help, at the moment I get barely
> > any sleep, let alone that I could keep up with the majority of mail I
> > get. I'll let you decide whether you want to be sole maintainer or not.
>
> For any bigger subsystems, it's good practice to have two maintainers,
> even if one of them does all the work. I think it makes sense to have
> Jean-Christophe as the primary maintainer and owner of the git tree,
> but it may also be good to have you or Tomi listed as a second maintainer
> for times when Jean-Christophe isn't available.
I agree with Arnd
As example I'll be mostly off first week of June.
Tomi Florian what do you think
I plan to send the patch + PULL to Linus end of this week before I work-off
For the record the new git tree is here
git://git.kernel.org/pub/scm/linux/kernel/git/plagnioj/linux-fbdev.git
Best Regrds,
J.
^ permalink raw reply
* Re: [RFC] Add co-maintainer for fbdev
From: Arnd Bergmann @ 2013-05-28 21:19 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: Olof Johansson, Jean-Christophe PLAGNIOL-VILLARD, linux-fbdev,
Linus Torvalds, linux-kernel@vger.kernel.org, Tomi Valkeinen,
Andrew Morton
In-Reply-To: <51A29DB9.7030505@gmx.de>
On Monday 27 May 2013, Florian Tobias Schandinat wrote:
> >> On Fri, May 24, 2013 at 8:38 AM, Jean-Christophe PLAGNIOL-VILLARD
> >> <plagnioj@jcrosoft.com> wrote:
> >>> Hi Florian,
> >>>
> >>> As you seems very busy I'd like to propose the help you to handle the
> >>> fbdev subsystem to easier the rich of the fbdev patch to Linus
> >>>
> >>> As I'm working on fbdev on at91 and others and already Co-Maintain the
> >>> at91 mach on ARM
> >>>
> >>> And if you are not willing to continue I could take over
>
> Yeah, it would be great if you could help, at the moment I get barely
> any sleep, let alone that I could keep up with the majority of mail I
> get. I'll let you decide whether you want to be sole maintainer or not.
For any bigger subsystems, it's good practice to have two maintainers,
even if one of them does all the work. I think it makes sense to have
Jean-Christophe as the primary maintainer and owner of the git tree,
but it may also be good to have you or Tomi listed as a second maintainer
for times when Jean-Christophe isn't available.
I don't know how many patches you'd expect per release, maybe just one
person is enough after all.
Arnd
^ permalink raw reply
* Re: Introduce a new helper framework for buffer synchronization
From: Daniel Vetter @ 2013-05-28 16:49 UTC (permalink / raw)
To: Inki Dae
Cc: Rob Clark, Maarten Lankhorst, linux-fbdev, YoungJun Cho,
Kyungmin Park, myungjoo.ham, DRI mailing list,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <00de01ce5bb2$aa7c6b30$ff754190$%dae@samsung.com>
On Tue, May 28, 2013 at 4:50 PM, Inki Dae <inki.dae@samsung.com> wrote:
> I think I already used reservation stuff any time in that way except
> ww-mutex. And I'm not sure that embedded system really needs ww-mutex. If
> there is any case,
> could you tell me the case? I really need more advice and understanding :)
If you have only one driver, you can get away without ww_mutex.
drm/i915 does it, all buffer state is protected by dev->struct_mutex.
But as soon as you have multiple drivers sharing buffers with dma_buf
things will blow up.
Yep, current prime is broken and can lead to deadlocks.
In practice it doesn't (yet) matter since only the X server does the
sharing dance, and that one's single-threaded. Now you can claim that
since you have all buffers pinned in embedded gfx anyway, you don't
care. But both in desktop gfx and embedded gfx the real fun starts
once you put fences into the mix and link them up with buffers, then
every command submission risks that deadlock. Furthermore you can get
unlucky and construct a circle of fences waiting on each another (only
though if the fence singalling fires off the next batchbuffer
asynchronously).
To prevent such deadlocks you _absolutely_ need to lock _all_ buffers
that take part in a command submission at once. To do that you either
need a global lock (ugh) or ww_mutexes.
So ww_mutexes are the fundamental ingredient of all this, if you don't
see why you need them then everything piled on top is broken. I think
until you've understood why exactly we need ww_mutexes there's not
much point in discussing the finer issues of fences, reservation
objects and how to integrate it with dma_bufs exactly.
I'll try to clarify the motivating example in the ww_mutex
documentation a bit, but I dunno how else I could explain this ...
Yours, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH] video: mxsfb: Let device core handle pinctrl
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-28 15:25 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1369619294-10362-1-git-send-email-festevam@gmail.com>
On 12:09 Tue 28 May , Fabio Estevam wrote:
> Hi Jean-Christophe,
>
> On Mon, May 27, 2013 at 11:44 AM, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> > On 22:48 Sun 26 May , Fabio Estevam wrote:
> >> From: Fabio Estevam <fabio.estevam@freescale.com>
> >>
> >> Since commit ab78029 (drivers/pinctrl: grab default handles from device core)
> >> we can rely on device core for handling pinctrl, so remove
> >> devm_pinctrl_get_select_default() from the driver.
> > Linus we should do a pass on the kernel to clean this
>
> Not sure which 'Linus' the comment was meant to be directed to, but I
> have already sent several patches removing
> devm_pinctrl_get_select_default() and many of them were already
> applied by the maintainers.
here I mean linusw
Best Regards,
J.
^ permalink raw reply
* Re: [PATCH] video: mxsfb: Let device core handle pinctrl
From: Fabio Estevam @ 2013-05-28 15:09 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1369619294-10362-1-git-send-email-festevam@gmail.com>
Hi Jean-Christophe,
On Mon, May 27, 2013 at 11:44 AM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 22:48 Sun 26 May , Fabio Estevam wrote:
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> Since commit ab78029 (drivers/pinctrl: grab default handles from device core)
>> we can rely on device core for handling pinctrl, so remove
>> devm_pinctrl_get_select_default() from the driver.
> Linus we should do a pass on the kernel to clean this
Not sure which 'Linus' the comment was meant to be directed to, but I
have already sent several patches removing
devm_pinctrl_get_select_default() and many of them were already
applied by the maintainers.
Regards,
Fabio Estevam
^ permalink raw reply
* RE: Introduce a new helper framework for buffer synchronization
From: Inki Dae @ 2013-05-28 14:50 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <51909DB4.2060208@canonical.com>
> -----Original Message-----
> From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-
> owner@vger.kernel.org] On Behalf Of Rob Clark
> Sent: Tuesday, May 28, 2013 10:49 PM
> To: Inki Dae
> Cc: Maarten Lankhorst; Daniel Vetter; linux-fbdev; YoungJun Cho; Kyungmin
> Park; myungjoo.ham; DRI mailing list;
linux-arm-kernel@lists.infradead.org;
> linux-media@vger.kernel.org
> Subject: Re: Introduce a new helper framework for buffer synchronization
>
> On Mon, May 27, 2013 at 11:56 PM, Inki Dae <inki.dae@samsung.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-
> >> owner@vger.kernel.org] On Behalf Of Rob Clark
> >> Sent: Tuesday, May 28, 2013 12:48 AM
> >> To: Inki Dae
> >> Cc: Maarten Lankhorst; Daniel Vetter; linux-fbdev; YoungJun Cho;
> Kyungmin
> >> Park; myungjoo.ham; DRI mailing list;
> > linux-arm-kernel@lists.infradead.org;
> >> linux-media@vger.kernel.org
> >> Subject: Re: Introduce a new helper framework for buffer
> synchronization
> >>
> >> On Mon, May 27, 2013 at 6:38 AM, Inki Dae <inki.dae@samsung.com> wrote:
> >> > Hi all,
> >> >
> >> > I have been removed previous branch and added new one with more
> cleanup.
> >> > This time, the fence helper doesn't include user side interfaces and
> >> cache
> >> > operation relevant codes anymore because not only we are not sure
> that
> >> > coupling those two things, synchronizing caches and buffer access
> >> between
> >> > CPU and CPU, CPU and DMA, and DMA and DMA with fences, in kernel side
> is
> >> a
> >> > good idea yet but also existing codes for user side have problems
> with
> >> badly
> >> > behaved or crashing userspace. So this could be more discussed later.
> >> >
> >> > The below is a new branch,
> >> >
> >> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> >> exynos.git/?h=dma-f
> >> > ence-helper
> >> >
> >> > And fence helper codes,
> >> >
> >> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> >> exynos.git/commit/?
> >> > h=dma-fence-helper&idcbc0fe7e285ce866e5816e5e21443dcce01005
> >> >
> >> > And example codes for device driver,
> >> >
> >> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> >> exynos.git/commit/?
> >> > h=dma-fence-helper&idÒce7af23835789602a99d0ccef1f53cdd5caaae
> >> >
> >> > I think the time is not yet ripe for RFC posting: maybe existing dma
> >> fence
> >> > and reservation need more review and addition work. So I'd glad for
> >> somebody
> >> > giving other opinions and advices in advance before RFC posting.
> >>
> >> thoughts from a *really* quick, pre-coffee, first look:
> >> * any sort of helper to simplify single-buffer sort of use-cases (v4l)
> >> probably wouldn't want to bake in assumption that seqno_fence is used.
> >> * I guess g2d is probably not actually a simple use case, since I
> >> expect you can submit blits involving multiple buffers :-P
> >
> > I don't think so. One and more buffers can be used: seqno_fence also has
> > only one buffer. Actually, we have already applied this approach to most
> > devices; multimedia, gpu and display controller. And this approach shows
> > more performance; reduced power consumption against traditional way. And
> g2d
> > example is just to show you how to apply my approach to device driver.
>
> no, you need the ww-mutex / reservation stuff any time you have
> multiple independent devices (or rings/contexts for hw that can
> support multiple contexts) which can do operations with multiple
> buffers.
I think I already used reservation stuff any time in that way except
ww-mutex. And I'm not sure that embedded system really needs ww-mutex. If
there is any case,
could you tell me the case? I really need more advice and understanding :)
Thanks,
Inki Dae
So you could conceivably hit this w/ gpu + g2d if multiple
> buffers where shared between the two. vram migration and such
> 'desktop stuff' might make the problem worse, but just because you
> don't have vram doesn't mean you don't have a problem with multiple
> buffers.
>
> >> * otherwise, you probably don't want to depend on dmabuf, which is why
> >> reservation/fence is split out the way it is.. you want to be able to
> >> use a single reservation/fence mechanism within your driver without
> >> having to care about which buffers are exported to dmabuf's and which
> >> are not. Creating a dmabuf for every GEM bo is too heavyweight.
> >
> > Right. But I think we should dealt with this separately. Actually, we
> are
> > trying to use reservation for gpu pipe line synchronization such as sgx
> sync
> > object and this approach is used without dmabuf. In order words, some
> device
> > can use only reservation for such pipe line synchronization and at the
> same
> > time, fence helper or similar thing with dmabuf for buffer
> synchronization.
>
> it is probably easier to approach from the reverse direction.. ie, get
> reservation/synchronization right first, and then dmabuf. (Well, that
> isn't really a problem because Maarten's reservation/fence patches
> support dmabuf from the beginning.)
>
> BR,
> -R
>
> >>
> >> I'm not entirely sure if reservation/fence could/should be made any
> >> simpler for multi-buffer users. Probably the best thing to do is just
> >> get reservation/fence rolled out in a few drivers and see if some
> >> common patterns emerge.
> >>
> >> BR,
> >> -R
> >>
> >> >
> >> > Thanks,
> >> > Inki Dae
> >> >
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-fbdev"
> in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: Introduce a new helper framework for buffer synchronization
From: Inki Dae @ 2013-05-28 14:43 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <51909DB4.2060208@canonical.com>
Hi Daniel,
Thank you so much. And so very useful.:) Sorry but could be give me more
comments to the below my comments? There are still things making me
confusing.:(
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Tuesday, May 28, 2013 7:33 PM
> To: Inki Dae
> Cc: 'Rob Clark'; 'Maarten Lankhorst'; 'Daniel Vetter'; 'linux-fbdev';
> 'YoungJun Cho'; 'Kyungmin Park'; 'myungjoo.ham'; 'DRI mailing list';
> linux-arm-kernel@lists.infradead.org; linux-media@vger.kernel.org
> Subject: Re: Introduce a new helper framework for buffer synchronization
>
> On Tue, May 28, 2013 at 12:56:57PM +0900, Inki Dae wrote:
> >
> >
> > > -----Original Message-----
> > > From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-
> > > owner@vger.kernel.org] On Behalf Of Rob Clark
> > > Sent: Tuesday, May 28, 2013 12:48 AM
> > > To: Inki Dae
> > > Cc: Maarten Lankhorst; Daniel Vetter; linux-fbdev; YoungJun Cho;
> Kyungmin
> > > Park; myungjoo.ham; DRI mailing list;
> > linux-arm-kernel@lists.infradead.org;
> > > linux-media@vger.kernel.org
> > > Subject: Re: Introduce a new helper framework for buffer
> synchronization
> > >
> > > On Mon, May 27, 2013 at 6:38 AM, Inki Dae <inki.dae@samsung.com>
wrote:
> > > > Hi all,
> > > >
> > > > I have been removed previous branch and added new one with more
> cleanup.
> > > > This time, the fence helper doesn't include user side interfaces and
> > > cache
> > > > operation relevant codes anymore because not only we are not sure
> that
> > > > coupling those two things, synchronizing caches and buffer access
> > > between
> > > > CPU and CPU, CPU and DMA, and DMA and DMA with fences, in kernel
> side is
> > > a
> > > > good idea yet but also existing codes for user side have problems
> with
> > > badly
> > > > behaved or crashing userspace. So this could be more discussed
later.
> > > >
> > > > The below is a new branch,
> > > >
> > > > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> > > exynos.git/?h=dma-f
> > > > ence-helper
> > > >
> > > > And fence helper codes,
> > > >
> > > > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> > > exynos.git/commit/?
> > > > h=dma-fence-helper&idcbc0fe7e285ce866e5816e5e21443dcce01005
> > > >
> > > > And example codes for device driver,
> > > >
> > > > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
> > > exynos.git/commit/?
> > > > h=dma-fence-helper&idÒce7af23835789602a99d0ccef1f53cdd5caaae
> > > >
> > > > I think the time is not yet ripe for RFC posting: maybe existing dma
> > > fence
> > > > and reservation need more review and addition work. So I'd glad for
> > > somebody
> > > > giving other opinions and advices in advance before RFC posting.
> > >
> > > thoughts from a *really* quick, pre-coffee, first look:
> > > * any sort of helper to simplify single-buffer sort of use-cases (v4l)
> > > probably wouldn't want to bake in assumption that seqno_fence is used.
> > > * I guess g2d is probably not actually a simple use case, since I
> > > expect you can submit blits involving multiple buffers :-P
> >
> > I don't think so. One and more buffers can be used: seqno_fence also has
> > only one buffer. Actually, we have already applied this approach to most
> > devices; multimedia, gpu and display controller. And this approach shows
> > more performance; reduced power consumption against traditional way. And
> g2d
> > example is just to show you how to apply my approach to device driver.
>
> Note that seqno_fence is an implementation pattern for a certain type of
> direct hw->hw synchronization which uses a shared dma_buf to exchange the
> sync cookie.
I'm afraid that I don't understand hw->hw synchronization. hw->hw
synchronization means that device has a hardware feature which supports
buffer synchronization hardware internally? And what is the sync cookie?
> The dma_buf attached to the seqno_fence has _nothing_ to do
> with the dma_buf the fence actually coordinates access to.
>
> I think that confusing is a large reason for why Maarten&I don't
> understand what you want to achieve with your fence helpers. Currently
> they're using the seqno_fence, but totally not in a way the seqno_fence
> was meant to be used.
>
> Note that with the current code there is only a pointer from dma_bufs to
> the fence. The fence itself has _no_ pointer to the dma_buf it syncs. This
> shouldn't be a problem since the fence fastpath for already signalled
> fences is completely barrier&lock free (it's just a load+bit-test), and
> fences are meant to be embedded into whatever dma tracking structure you
> already have, so no overhead there. The only ugly part is the fence
> refcounting, but I don't think we can drop that.
The below is the proposed way,
dma device has to create a fence before accessing a shared buffer, and then
check if there are other dma which are accessing the shared buffer; if exist
then the dma device should be blocked, and then it sets the fence to
reservation object of the shared buffer. And then the dma begins access to
the shared buffer. And finally, the dma signals its own fence so that other
blocked can be waked up. However, if there was another dma blocked before
signaling then another dma can access invalid fence object after waked up
because the fence object can be released after signaling. So I made another
dma takes a one reference before blocked. And I think origin version also
takes a reference at ticket_commit(). Is there wrong point in proposed way?
>
> Note that you completely reinvent this part of Maarten's fence patches by
> adding new r/w_complete completions to the reservation object, which
> completely replaces the fence stuff.
>
> Also note that a list of reservation entries is again meant to be used
> only when submitting the dma to the gpu. With your patches you seem to
> hang onto that list until dma completes.
Yeah, that is for prevent a dma from accessing a shared buffer while the
other is accessing the shared buffer. Maybe this way means software
synchronization. And your saying seems that there is another mechanism for
dealing with such thing. Could you give me more comments for it? is there
really another mechanism?
> This has the ugly side-effect
> that you need to allocate these reservation entries with kmalloc, whereas
> if you just use them in the execbuf ioctl to construct the dma you can
> usually embed it into something else you need already anyway. At least
> i915 and ttm based drivers can work that way.
>
Maybe there is my misunderstanding but I thought now dma fence and relevant
stubs tend to depend on Desktop world, especially x86 gpu. So I thought we
need a little bit customizing the dma fence for embedded system.
> Furthermore fences are specifically constructed as frankenstein-monsters
> between completion/waitqueues and callbacks. All the different use-cases
> need the different aspects:
> - busy/idle checks and bo retiring need the completion semantics
> - callbacks (in interrupt context) are used for hybrid hw->irq handler->hw
> sync approaches
>
> >
> > > * otherwise, you probably don't want to depend on dmabuf, which is why
> > > reservation/fence is split out the way it is.. you want to be able to
> > > use a single reservation/fence mechanism within your driver without
> > > having to care about which buffers are exported to dmabuf's and which
> > > are not. Creating a dmabuf for every GEM bo is too heavyweight.
> >
> > Right. But I think we should dealt with this separately. Actually, we
> are
> > trying to use reservation for gpu pipe line synchronization such as sgx
> sync
> > object and this approach is used without dmabuf. In order words, some
> device
> > can use only reservation for such pipe line synchronization and at the
> same
> > time, fence helper or similar thing with dmabuf for buffer
> synchronization.
>
> I think a quick overview of the different pieces would be in order.
>
> - ww_mutex: This is just the locking primite which allows you to grab
> multiple mutexes without the need to check for ordering (and so fear
> deadlocks).
Should ww_mutex be necessarily used for embedded system? I'm not sure that
there are any cases we have to use this feature like x86 gpu. That is also
why I removed ticket stubs from existing reservation framework.
>
> - fence: This is just a fancy kind of completion with a bit of support for
> hw->hw completion events.
hw->hw completion event means completion of dma operation? Otherwise,
hardware buffer synchronization feature? If the latter, how fence can be
used for embedded system which has no hardware feature for buffer
synchronization? Actually, that made me confusing at initial work.
> The fences themselve have no tie-in with
> buffers, ww_mutexes or anything else.
>
> - reservation: This ties together an object (doesn't need to be a buffer,
> could be any other gpu resource - see the drm/vmwgfx driver if you want
> your mind blown) with fences. Note that there's no connection the other
> way round, i.e. with the current patches you can't see which
> reservations are using which fences. Also note that other objects than
> reservations could point at fences, e.g. when the provide
> shared/exclusive semantics are not good enough for your needs.
>
> The ww_mutex in the reservation is simply the (fancy) lock which
> protects each reservation. The reason we need something fancy is that
> you need to lock all objects which are synced by the same fence
> toghether, otherwise you can race and construct deadlocks in the hw->hw
> sync part of fences.
>
> - dma_buf integration: This is simply a pointer to the reservation object
> of the underlying buffer object. We need a pointer here since otherwise
> you can construct deadlocks between dma_buf objects and native buffer
> objects.
>
> The crux is also that dma_buf integration comes last - before you can do
> that you need to have your driver converted over to use
> ww_mutexes/fences/reservations.
>
> I hope that helps to unconfuse things a bit and help you understand the
> different pieces of the fence/reservation/ww_mutex patches floating
> around.
Thank you again.:)
Thanks,
Inki Dae
>
> Cheers, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: Introduce a new helper framework for buffer synchronization
From: Rob Clark @ 2013-05-28 13:48 UTC (permalink / raw)
To: Inki Dae
Cc: Maarten Lankhorst, Daniel Vetter, linux-fbdev, YoungJun Cho,
Kyungmin Park, myungjoo.ham, DRI mailing list, linux-arm-kernel,
linux-media
In-Reply-To: <005701ce5b57$667c7d40$337577c0$%dae@samsung.com>
On Mon, May 27, 2013 at 11:56 PM, Inki Dae <inki.dae@samsung.com> wrote:
>
>
>> -----Original Message-----
>> From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-
>> owner@vger.kernel.org] On Behalf Of Rob Clark
>> Sent: Tuesday, May 28, 2013 12:48 AM
>> To: Inki Dae
>> Cc: Maarten Lankhorst; Daniel Vetter; linux-fbdev; YoungJun Cho; Kyungmin
>> Park; myungjoo.ham; DRI mailing list;
> linux-arm-kernel@lists.infradead.org;
>> linux-media@vger.kernel.org
>> Subject: Re: Introduce a new helper framework for buffer synchronization
>>
>> On Mon, May 27, 2013 at 6:38 AM, Inki Dae <inki.dae@samsung.com> wrote:
>> > Hi all,
>> >
>> > I have been removed previous branch and added new one with more cleanup.
>> > This time, the fence helper doesn't include user side interfaces and
>> cache
>> > operation relevant codes anymore because not only we are not sure that
>> > coupling those two things, synchronizing caches and buffer access
>> between
>> > CPU and CPU, CPU and DMA, and DMA and DMA with fences, in kernel side is
>> a
>> > good idea yet but also existing codes for user side have problems with
>> badly
>> > behaved or crashing userspace. So this could be more discussed later.
>> >
>> > The below is a new branch,
>> >
>> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
>> exynos.git/?h=dma-f
>> > ence-helper
>> >
>> > And fence helper codes,
>> >
>> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
>> exynos.git/commit/?
>> > h=dma-fence-helper&idcbc0fe7e285ce866e5816e5e21443dcce01005
>> >
>> > And example codes for device driver,
>> >
>> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-
>> exynos.git/commit/?
>> > h=dma-fence-helper&idÒce7af23835789602a99d0ccef1f53cdd5caaae
>> >
>> > I think the time is not yet ripe for RFC posting: maybe existing dma
>> fence
>> > and reservation need more review and addition work. So I'd glad for
>> somebody
>> > giving other opinions and advices in advance before RFC posting.
>>
>> thoughts from a *really* quick, pre-coffee, first look:
>> * any sort of helper to simplify single-buffer sort of use-cases (v4l)
>> probably wouldn't want to bake in assumption that seqno_fence is used.
>> * I guess g2d is probably not actually a simple use case, since I
>> expect you can submit blits involving multiple buffers :-P
>
> I don't think so. One and more buffers can be used: seqno_fence also has
> only one buffer. Actually, we have already applied this approach to most
> devices; multimedia, gpu and display controller. And this approach shows
> more performance; reduced power consumption against traditional way. And g2d
> example is just to show you how to apply my approach to device driver.
no, you need the ww-mutex / reservation stuff any time you have
multiple independent devices (or rings/contexts for hw that can
support multiple contexts) which can do operations with multiple
buffers. So you could conceivably hit this w/ gpu + g2d if multiple
buffers where shared between the two. vram migration and such
'desktop stuff' might make the problem worse, but just because you
don't have vram doesn't mean you don't have a problem with multiple
buffers.
>> * otherwise, you probably don't want to depend on dmabuf, which is why
>> reservation/fence is split out the way it is.. you want to be able to
>> use a single reservation/fence mechanism within your driver without
>> having to care about which buffers are exported to dmabuf's and which
>> are not. Creating a dmabuf for every GEM bo is too heavyweight.
>
> Right. But I think we should dealt with this separately. Actually, we are
> trying to use reservation for gpu pipe line synchronization such as sgx sync
> object and this approach is used without dmabuf. In order words, some device
> can use only reservation for such pipe line synchronization and at the same
> time, fence helper or similar thing with dmabuf for buffer synchronization.
it is probably easier to approach from the reverse direction.. ie, get
reservation/synchronization right first, and then dmabuf. (Well, that
isn't really a problem because Maarten's reservation/fence patches
support dmabuf from the beginning.)
BR,
-R
>>
>> I'm not entirely sure if reservation/fence could/should be made any
>> simpler for multi-buffer users. Probably the best thing to do is just
>> get reservation/fence rolled out in a few drivers and see if some
>> common patterns emerge.
>>
>> BR,
>> -R
>>
>> >
>> > Thanks,
>> > Inki Dae
>> >
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ 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