Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [GIT PULL] fbdev changes for 3.20
From: Tomi Valkeinen @ 2015-02-11  9:35 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Linux Fbdev development list, LKML

[-- Attachment #1: Type: text/plain, Size: 6317 bytes --]

Hi Linus,

Please pull fbdev changes for 3.20.

 Tomi

The following changes since commit eaa27f34e91a14cdceed26ed6c6793ec1d186115:

  linux 3.19-rc4 (2015-01-11 12:44:53 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git tags/fbdev-3.20

for you to fetch changes up to d6c2152b3efd73be265f426b5a1bb50ec436d999:

  Merge branches '3.20/fbdev' and '3.20/omapdss' into for-next (2015-02-04 12:44:11 +0200)

----------------------------------------------------------------
fbdev changes for v3.20

* omapdss: add DRA7xxx SoC support
* fbdev: support DMT (Display Monitor Timing) calculation

----------------------------------------------------------------
Arnd Bergmann (2):
      fb: via: turn gpiolib and i2c selects into dependencies
      video/mmpfb: allow modular build

Asaf Vertz (1):
      fbdev: vgastate: remove trailing whitespaces

David Ung (4):
      video: fbdev: Add additional vesa modes
      video: fbdev: Check Standard Timing against DMT
      video: fbdev: Validate mode timing against monspec
      video: fbdev: Fix sparse warning messages

Geert Uytterhoeven (2):
      console/dummy: Move screen size selection from CPP to Kconfig
      video: Drop superfluous "select VT_HW_CONSOLE_BINDING"

Lad, Prabhakar (2):
      fbdev: ssd1307fb: drop unused function ssd1307fb_write_data()
      fbdev: ssd1307fb: set default height if not found in DT node

Mans Rullgard (1):
      video: fbdev: fix sys_copyarea

Marek Belisko (2):
      video: omapdss: Add opa362 driver
      Documentation: DT: Add documentation for ti,opa362 bindings

Mathias Krause (1):
      atyfb: mark DMI system id table as __initconst

Peter Meerwald (1):
      omapfb: Return error code when applying overlay settings fails

Prabhakar Lad (1):
      fbdev: ssd1307fb: return proper error code if write command fails

Qiang Chen (1):
      video: ocfb: Fix data type warning

Rasmus Villemoes (1):
      fbdev: omap2: Fix typo in tvc_probe_pdata

Scot Doyle (1):
      fbcon: Remove unused vblank cursor code

Sudip Mukherjee (5):
      fbdev: geocode: remove unneeded NULL check
      fbdev: geocode: remove unneeded NULL check
      fbdev: savage: remove unused variable
      video: vt8500lcdfb: remove unneeded continue
      video: hgafb: remove unneeded comparison

Tomi Valkeinen (16):
      fbdev: fix CVT vertical front and back porch values
      OMAPDSS: Add enum dss_pll_id
      OMAPDSS: PLL: add dss_pll_wait_reset_done()
      OMAPDSS: constify port arrays
      OMAPDSS: OMAP5: fix digit output's allowed mgrs
      OMAPDSS: encoder-tpd12s015: Fix race issue with LS_OE
      OMAPDSS: add define for DRA7xx HW version
      Doc/DT: Add DT binding doc for DRA7xx DSS
      OMAPDSS: DSS: Add DRA7xx base support
      OMAPDSS: Add functions for external control of PLL
      OMAPDSS: Add Video PLLs for DRA7xx
      OMAPDSS: DISPC: Add DRA7xx support
      OMAPDSS: DISPC: program dispc polarities to control module
      OMAPDSS: HDMI: Add DRA7xx support
      OMAPDSS: DPI: DRA7xx support
      Merge branches '3.20/fbdev' and '3.20/omapdss' into for-next

 .../devicetree/bindings/video/ti,dra7-dss.txt      |  69 +++++
 .../devicetree/bindings/video/ti,opa362.txt        |  38 +++
 drivers/video/console/Kconfig                      |  16 +-
 drivers/video/console/dummycon.c                   |   5 +-
 drivers/video/console/fbcon.c                      |   4 -
 drivers/video/fbdev/Kconfig                        |   5 +-
 drivers/video/fbdev/aty/atyfb_base.c               |   9 +-
 drivers/video/fbdev/core/fbcvt.c                   |   6 +-
 drivers/video/fbdev/core/fbmon.c                   | 103 ++++----
 drivers/video/fbdev/core/modedb.c                  | 111 ++++++++
 drivers/video/fbdev/core/syscopyarea.c             | 137 +++++-----
 drivers/video/fbdev/geode/gx1fb_core.c             |   6 +-
 drivers/video/fbdev/geode/gxfb_core.c              |   6 +-
 drivers/video/fbdev/geode/lxfb_core.c              |   6 +-
 drivers/video/fbdev/hgafb.c                        |   3 +-
 drivers/video/fbdev/mmp/Makefile                   |   4 +-
 drivers/video/fbdev/mmp/fb/Kconfig                 |   2 +-
 drivers/video/fbdev/ocfb.c                         |   2 +-
 drivers/video/fbdev/omap2/displays-new/Kconfig     |   6 +
 drivers/video/fbdev/omap2/displays-new/Makefile    |   1 +
 .../fbdev/omap2/displays-new/connector-analog-tv.c |   2 +-
 .../fbdev/omap2/displays-new/encoder-opa362.c      | 285 +++++++++++++++++++++
 .../fbdev/omap2/displays-new/encoder-tpd12s015.c   |  57 +----
 drivers/video/fbdev/omap2/dss/Makefile             |   2 +-
 drivers/video/fbdev/omap2/dss/dispc.c              |  54 ++++
 drivers/video/fbdev/omap2/dss/dpi.c                |  26 ++
 drivers/video/fbdev/omap2/dss/dsi.c                |   1 +
 drivers/video/fbdev/omap2/dss/dss.c                | 219 +++++++++++++++-
 drivers/video/fbdev/omap2/dss/dss.h                |  22 ++
 drivers/video/fbdev/omap2/dss/dss_features.c       |   3 +-
 drivers/video/fbdev/omap2/dss/hdmi5.c              |   1 +
 drivers/video/fbdev/omap2/dss/hdmi_phy.c           |   1 +
 drivers/video/fbdev/omap2/dss/hdmi_pll.c           |   6 +
 drivers/video/fbdev/omap2/dss/omapdss-boot-init.c  |   1 +
 drivers/video/fbdev/omap2/dss/pll.c                |  10 +
 drivers/video/fbdev/omap2/dss/video-pll.c          | 211 +++++++++++++++
 drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c    |   7 +-
 drivers/video/fbdev/savage/savagefb.h              |  12 +-
 drivers/video/fbdev/ssd1307fb.c                    |  86 ++++---
 drivers/video/fbdev/vt8500lcdfb.c                  |   4 +-
 drivers/video/vgastate.c                           |  82 +++---
 include/linux/fb.h                                 |  12 +-
 include/video/omapdss.h                            |   1 +
 43 files changed, 1342 insertions(+), 302 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/ti,dra7-dss.txt
 create mode 100644 Documentation/devicetree/bindings/video/ti,opa362.txt
 create mode 100644 drivers/video/fbdev/omap2/displays-new/encoder-opa362.c
 create mode 100644 drivers/video/fbdev/omap2/dss/video-pll.c


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH] video: ARM CLCD: Add missing error check for devm_kzalloc
From: Kiran Padwal @ 2015-02-11  9:48 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Russell King, Jean-Christophe Plagniol-Villard, linux-fbdev,
	linux-kernel, Kiran Padwal, Kiran Padwal

This patch add a missing check on the return value of devm_kzalloc,
which would cause a NULL pointer dereference in a OOM situation.

Signed-off-by: Kiran Padwal <kiran.padwal@smartplayin.com>
---
 drivers/video/fbdev/amba-clcd.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 32c0b6b..9362424 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -599,6 +599,9 @@ static int clcdfb_of_get_mode(struct device *dev, struct device_node *endpoint,
 
 	len = clcdfb_snprintf_mode(NULL, 0, mode);
 	name = devm_kzalloc(dev, len + 1, GFP_KERNEL);
+	if (!name)
+		return -ENOMEM;
+
 	clcdfb_snprintf_mode(name, len + 1, mode);
 	mode->name = name;
 
-- 
1.7.9.5


^ permalink raw reply related

* [bug report] potential memory corruption in intelfb_cursor()
From: Dan Carpenter @ 2015-02-11 15:45 UTC (permalink / raw)
  To: linux-fbdev

Some static checker stuff I'm working on complains:

	drivers/video/fbdev/intelfb/intelfbdrv.c:1678 intelfb_cursor()
	error: overflow detected.  __memcpy() 'dinfo->cursor_src' is 64
	       bytes.  limit = '1-128'

The math here is:

	size = cursor->image.width / 8 * cursor->image.height;

Since the limit for ".width" and ".height" is 32x32 then
"32 / 4 * 32 = 128" but 128 is more than the sizeof(dinfo->cursor_src)
so it creates a buffer overflow warning.

I believe that Smatch is getting 32 as the max size from the tests in
con_font_set().

drivers/video/fbdev/intelfb/intelfbdrv.c

  1632          if (cursor->set & FB_CUR_SETSIZE) {
  1633                  if (cursor->image.width > 64 || cursor->image.height > 64)

Here the limit is assumed to be 64 which would make the overflow worse
than the 32 limit mentioned earlier.

  1634                          return -ENXIO;
  1635  
  1636                  intelfbhw_cursor_reset(dinfo);
  1637          }
  1638  
  1639          if (cursor->set & FB_CUR_SETCMAP) {
  1640                  u32 fg, bg;
  1641  
  1642                  if (dinfo->depth != 8) {
  1643                          fg = dinfo->pseudo_palette[cursor->image.fg_color];
  1644                          bg = dinfo->pseudo_palette[cursor->image.bg_color];
  1645                  } else {
  1646                          fg = cursor->image.fg_color;
  1647                          bg = cursor->image.bg_color;
  1648                  }
  1649  
  1650                  intelfbhw_cursor_setcolor(dinfo, bg, fg);
  1651          }
  1652  
  1653          if (cursor->set & (FB_CUR_SETSHAPE | FB_CUR_SETIMAGE)) {
  1654                  u32 s_pitch = (ROUND_UP_TO(cursor->image.width, 8) / 8);
  1655                  u32 size = s_pitch * cursor->image.height;
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Size is calculated here.

  1656                  u8 *dat = (u8 *) cursor->image.data;
  1657                  u8 *msk = (u8 *) cursor->mask;
  1658                  u8 src[64];
  1659                  u32 i;
  1660  
  1661                  if (cursor->image.depth != 1)
  1662                          return -ENXIO;
  1663  
  1664                  switch (cursor->rop) {
  1665                  case ROP_XOR:
  1666                          for (i = 0; i < size; i++)
  1667                                  src[i] = dat[i] ^ msk[i];
  1668                          break;
  1669                  case ROP_COPY:
  1670                  default:
  1671                          for (i = 0; i < size; i++)
  1672                                  src[i] = dat[i] & msk[i];
  1673                          break;
  1674                  }
  1678                  memcpy(dinfo->cursor_src, src, size);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is what Smatch complains about.  The source and dest are 64 bytes
each but it says the user can specify a size which is 128 bytes.

  1679  
  1680                  intelfbhw_cursor_load(dinfo, cursor->image.width,
  1681                                        cursor->image.height, src);
  1682          }

I am a newbie to this code so I don't know if the warning is real or not
or how to fix it.  Sorry.

regards,
dan carpenter

^ permalink raw reply

* Odp: [PATCH 1/4] tridentfb: fix hang on Blade3D with CONFIG_CC_OPTIMIZE_FOR_SIZE
From: Krzysztof Helt @ 2015-02-11 18:01 UTC (permalink / raw)
  To: Ondrej Zary; +Cc: linux-fbdev, Kernel development list

Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>

Dnia Sobota, 7 Lutego 2015 21:53 Ondrej Zary <linux@rainbow-software.org> napisa³(a) 
> When the kernel is compiled with -Os (CONFIG_CC_OPTIMIZE_FOR_SIZE), tridentfb
> hangs the machine upon load with Blade3D cards unless acceleration is disabled.
> 
> This is caused by memcpy() which copies data byte-by-byte (rep movsb) when
> compiled with -Os. The card does not like that - it requires 32-bit access.
> 
> Use iowrite_32() instead.
> 
> Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> ---
>  drivers/video/fbdev/tridentfb.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
> index 7ed9a22..7429713 100644
> --- a/drivers/video/fbdev/tridentfb.c
> +++ b/drivers/video/fbdev/tridentfb.c
> @@ -226,7 +226,7 @@ static void blade_image_blit(struct tridentfb_par *par, const char *data,
>  	writemmr(par, DST1, point(x, y));
>  	writemmr(par, DST2, point(x + w - 1, y + h - 1));
>  
> -	memcpy(par->io_virt + 0x10000, data, 4 * size);
> +	iowrite32_rep(par->io_virt + 0x10000, data, size);
>  }
>  
>  static void blade_copy_rect(struct tridentfb_par *par,
> -- 
> Ondrej Zary




^ permalink raw reply

* Odp: [PATCH 3/4] fb_ddc: Allow I2C adapters without SCL read capability
From: Krzysztof Helt @ 2015-02-11 18:02 UTC (permalink / raw)
  To: Ondrej Zary; +Cc: linux-fbdev, Kernel development list

Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>

Dnia Sobota, 7 Lutego 2015 21:53 Ondrej Zary <linux@rainbow-software.org> napisa³(a) 
> i2c-algo-bit allows I2C adapters without SCL read capability to work but
> fb_ddc_read fails to work on them.
> 
> Fix fb_ddc_read to work with I2C adapters not capable of reading SCL.
> 
> Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> ---
>  drivers/video/fbdev/core/fb_ddc.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/fbdev/core/fb_ddc.c b/drivers/video/fbdev/core/fb_ddc.c
> index 94322cc..22c694a 100644
> --- a/drivers/video/fbdev/core/fb_ddc.c
> +++ b/drivers/video/fbdev/core/fb_ddc.c
> @@ -69,10 +69,11 @@ unsigned char *fb_ddc_read(struct i2c_adapter *adapter)
>  		algo_data->setscl(algo_data->data, 1);
>  		for (j = 0; j < 5; j++) {
>  			msleep(10);
> -			if (algo_data->getscl(algo_data->data))
> +			if (algo_data->getscl &&
> +			    algo_data->getscl(algo_data->data))
>  				break;
>  		}
> -		if (j = 5)
> +		if (algo_data->getscl && j = 5)
>  			continue;
>  
>  		algo_data->setsda(algo_data->data, 0);
> @@ -91,7 +92,8 @@ unsigned char *fb_ddc_read(struct i2c_adapter *adapter)
>  		algo_data->setscl(algo_data->data, 1);
>  		for (j = 0; j < 10; j++) {
>  			msleep(10);
> -			if (algo_data->getscl(algo_data->data))
> +			if (algo_data->getscl &&
> +			    algo_data->getscl(algo_data->data))
>  				break;
>  		}
>  
> -- 
> Ondrej Zary




^ permalink raw reply

* Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Maxime Ripard @ 2015-02-12 15:11 UTC (permalink / raw)
  To: Thomas Niederprüm
  Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150207163541.30047a33@maestro.intranet>

[-- Attachment #1: Type: text/plain, Size: 2879 bytes --]

On Sat, Feb 07, 2015 at 04:35:41PM +0100, Thomas Niederprüm wrote:
> Am Sat, 7 Feb 2015 12:18:21 +0100
> schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> 
> > Hi,
> > 
> > On Fri, Feb 06, 2015 at 11:28:10PM +0100, niederp@physik.uni-kl.de
> > wrote:
> > > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > > 
> > > It makes sense to use vmalloc to allocate the video buffer since it
> > > has to be page aligned memory for using it with mmap.
> > 
> > Please wrap your commit log at 80 chars.
> 
> I'll try to do so in future, sorry for that.
> 
> > 
> > It looks like there's numerous fbdev drivers using this (especially
> > since you copy pasted that code, without mentionning it).
> 
> Yes, I should have mentioned that in the commit message. As
> implicitly indicated in the cover letter the rvmalloc() and rvfree() are
> copy pasted from the vfb driver. Honestly, I didn't give this one too
> much thought. It seemed a viable solution to the mmap problem. For a
> bit more history on that, see my comment below.
> 
> > 
> > That should be turned into an allocator so that drivers all get this
> > right.
> > 
> > > Also deffered io seems buggy in combination with kmalloc'ed memory
> > > (crash on unloading the module).
> > 
> > And maybe that's the real issue to fix.
> 
> The problem is solved by using vmalloc ;)

Yep, but why do you need to mark the reserved pages?

...

> > > @@ -570,7 +608,7 @@ static int ssd1307fb_probe(struct i2c_client
> > > *client, info->var.blue.offset = 0;
> > >  
> > >  	info->screen_base = (u8 __force __iomem *)vmem;
> > > -	info->fix.smem_start = (unsigned long)vmem;
> > > +	info->fix.smem_start = __pa(vmem);
> > 
> > Why are you changing from virtual to physical address here?
> 
> Oh, good catch! This is still a residual of my attempts to get this
> working with kmalloc'ed memory. In the current state the driver is
> presenting a completely wrong memory address upon mmap. As reported in
> [0] info->fix.smem_start has to hold the physical address of the video
> memory if it was allocated using kmalloc. Correcting this let me run
> into the problem that the kmalloc'ed memory was not page aligned but,
> the memory address handed to userspace mmap was aligned to the next
> full page, resulting in an inaccessable display region. At that point I
> just copied the vmalloc approach from the vfb driver.
> 
> [0] http://stackoverflow.com/questions/22285151/kernel-panic-using-deferred-io-on-kmalloced-buffer

And the documentation of fb_fix_screeninfo indeed state that this
should be the physical address:
http://lxr.free-electrons.com/source/include/uapi/linux/fb.h#L158

Could you make that a separate patch?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH] video: fbdev: fix possible null dereference
From: Sudip Mukherjee @ 2015-02-12 15:59 UTC (permalink / raw)
  To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen
  Cc: Sudip Mukherjee, linux-fbdev, linux-kernel

we were dereferencing edid first and the NULL check was after
accessing that. now we are using edid only if we know that
it is not NULL.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/video/fbdev/core/fbmon.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
index 9533859..868facd 100644
--- a/drivers/video/fbdev/core/fbmon.c
+++ b/drivers/video/fbdev/core/fbmon.c
@@ -624,9 +624,6 @@ static struct fb_videomode *fb_create_modedb(unsigned char *edid, int *dbsize,
 	int num = 0, i, first = 1;
 	int ver, rev;
 
-	ver = edid[EDID_STRUCT_VERSION];
-	rev = edid[EDID_STRUCT_REVISION];
-
 	mode = kzalloc(50 * sizeof(struct fb_videomode), GFP_KERNEL);
 	if (mode = NULL)
 		return NULL;
@@ -637,6 +634,9 @@ static struct fb_videomode *fb_create_modedb(unsigned char *edid, int *dbsize,
 		return NULL;
 	}
 
+	ver = edid[EDID_STRUCT_VERSION];
+	rev = edid[EDID_STRUCT_REVISION];
+
 	*dbsize = 0;
 
 	DPRINTK("   Detailed Timings\n");
-- 
1.8.1.2


^ permalink raw reply related

* Re: [PATCH 2/8] fbdev: ssd1307fb: Unify init code and make controller configurable from device tree
From: Maxime Ripard @ 2015-02-12 16:41 UTC (permalink / raw)
  To: Thomas Niederprüm
  Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150207155933.4f1d0998@maestro.intranet>

[-- Attachment #1: Type: text/plain, Size: 5372 bytes --]

On Sat, Feb 07, 2015 at 03:59:33PM +0100, Thomas Niederprüm wrote:
> Am Sat, 7 Feb 2015 11:42:25 +0100
> schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> 
> > Hi,
> > 
> > On Fri, Feb 06, 2015 at 11:28:08PM +0100, niederp@physik.uni-kl.de
> > wrote:
> > > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > > 
> > > This patches unifies the init code for the ssd130X chips and
> > > adds device tree bindings to describe the hardware configuration
> > > of the used controller. This gets rid of the magic bit values
> > > used in the init code so far.
> > > 
> > > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > > ---
> > >  .../devicetree/bindings/video/ssd1307fb.txt        |  11 +
> > >  drivers/video/fbdev/ssd1307fb.c                    | 243
> > > ++++++++++++++------- 2 files changed, 174 insertions(+), 80
> > > deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt
> > > b/Documentation/devicetree/bindings/video/ssd1307fb.txt index
> > > 7a12542..1230f68 100644 ---
> > > a/Documentation/devicetree/bindings/video/ssd1307fb.txt +++
> > > b/Documentation/devicetree/bindings/video/ssd1307fb.txt @@ -15,6
> > > +15,17 @@ Required properties: 
> > >  Optional properties:
> > >    - reset-active-low: Is the reset gpio is active on physical low?
> > > +  - solomon,segment-remap: Invert the order of data column to
> > > segment mapping
> > > +  - solomon,offset: Map the display start line to one of COM0 -
> > > COM63
> > > +  - solomon,contrast: Set initial contrast of the display
> > > +  - solomon,prechargep1: Set the duration of the precharge period
> > > phase1
> > > +  - solomon,prechargep2: Set the duration of the precharge period
> > > phase2
> > > +  - solomon,com-alt: Enable/disable alternate COM pin configuration
> > > +  - solomon,com-lrremap: Enable/disable left-right remap of COM
> > > pins
> > > +  - solomon,com-invdir: Invert COM scan direction
> > > +  - solomon,vcomh: Set VCOMH regulator voltage
> > > +  - solomon,dclk-div: Set display clock divider
> > > +  - solomon,dclk-frq: Set display clock frequency
> > 
> > I'm sorry, but this is the wrong approach, for at least two reasons:
> > you broke all existing users of that driver, which is a clear no-go,
> 
> Unfortunately this is true. The problem is that the SSD130X
> controllers allow for a very versatile wiring of the display to the
> controller.  It's over to the manufacturer of the OLED module
> (disp+controller) to decide how it's actually wired and during
> device initialization the driver has to take care to configure the
> SSD130X controller according to that wiring. If the driver fails to
> do so you will end up having your display showing
> garbage.

How so?

Does it depend on the X, or can it change from one same controller to
another? to what extent?

The 1306 for example seems to not be using these values at all, while
the 1307 does.

> Unfortunately the current sate of the initialization code of the
> ssd1307fb driver is not very flexible in that respect. Taking a look
> at the initialization code for the ssd1306 shows that it was written
> with one very special display module in mind. Most of the magic bit
> values set there are non-default values according to the
> datasheet. The result is that the driver works with that one
> particular display module but many other (differently wired) display
> modules using a ssd1306 controller won't work without changing the
> hardcoded magic bit values.
> 
> My idea here was to set all configuration to the default values (as
> given in the datasheet) unless it is overwritten by DT. Of course,
> without a change in DT, this breaks the driver for all existing users.
> The only alternative would be to set the current values as default.
> Somehow this feels wrong to me as these values look arbitrary when you
> don't know what exact display module they were set for. But if you
> insist, I will change the default values.

Unfortunately, the DT bindings are to be considered an ABI, and we
should support booting with older DTs (not that I personally care
about it, but that's another issue). So we really don't have much
choice here.

Moreover, that issue left aside, modifying bindings like this without
fixing up the in-tree users is considered quite rude :)

> > and the DT itself should not contain any direct mapping of the
> > registers.
> > 
> 
> I think I don't get what you mean here. Is it because I do no sanity
> checks of the numbers set in DT? I was just looking for a way to hand
> over the information about the wiring of display to the driver. How
> would you propose to solve this?

What I meant was that replicating direct registers value is usually a
recipe for a later failure, especially if we can have the information
under a generic and easy to understand manner.

For example, replacing the solomon,dclk-div and solomon,dclk-frq
properties by a clock-frequency property in Hz, and computing the
divider and that register in your driver is usually better, also
because it allows to have different requirements / algorithms to
compute that if some other chip needs it.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 2/8] fbdev: ssd1307fb: Unify init code and make controller configurable from device tree
From: Maxime Ripard @ 2015-02-12 16:58 UTC (permalink / raw)
  To: Noralf Trønnes
  Cc: Thomas Niederprüm, linux-fbdev, plagnioj, tomi.valkeinen,
	linux-kernel
In-Reply-To: <54D62D09.8080404@tronnes.org>

[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]

On Sat, Feb 07, 2015 at 04:19:37PM +0100, Noralf Trønnes wrote:
> >>and the DT itself should not contain any direct mapping of the
> >>registers.
> >>
> >I think I don't get what you mean here. Is it because I do no sanity
> >checks of the numbers set in DT? I was just looking for a way to hand
> >over the information about the wiring of display to the driver. How
> >would you propose to solve this?
> 
> I have the exact same challenge with the staging/fbtft drivers.
> I have asked about this on the DT list a couple of days ago, but no answer
> yet:
> 
> Can I do register initialization from Device Tree?
> http://www.spinics.net/lists/devicetree/msg68174.html

The DT is just an hardware representation, and should not contain any
logic. Any attempts at doing so have been failures, mostly because
that kind of construct are really fragile.

What would happen if at some point some new controller pops up and
need to poke a GPIO before every write?

Every driver should contain all the needed code to initialise properly
its hardware. The only exception being stuff that are volatile by
essence, like bus addresses, GPIOs, etc.

In your case, using (and maybe extending) the generic panel bindings
look like a better way forward.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* gxt4500 on x86 (Fire GL2)
From: Ondrej Zary @ 2015-02-12 21:11 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list

Hello,
I'm trying to get gxt4500 driver to work on x86 with Diamond/ATI Fire GL2 AGP
card:
01:00.0 VGA compatible controller [0300]: IBM GXT6000P Graphics Adapter [1014:0170] (rev 02) (prog-if 00 [VGA controller])
        Subsystem: Diamond Multimedia Systems Fire GL2 [1092:0172]
        Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B+ DisINTx-
        Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Interrupt: pin A routed to IRQ 16
        Region 0: Memory at f5000000 (32-bit, non-prefetchable) [size\x128K]
        Region 1: Memory at f0000000 (32-bit, non-prefetchable) [sizedM]
        [virtual] Expansion ROM at 30000000 [disabled] [size%6K]
        Capabilities: [60] Power Management version 0
                Flags: PMEClk- DSI+ D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
        Capabilities: [70] AGP version 2.0
                Status: RQ2 Iso- ArqSz=0 Cal=0 SBA+ ITACoh- GART64- HTrans- 64bit- FW- AGP3- Rate=x1,x2,x4
                Command: RQ=1 ArqSz=0 Cal=0 SBA- AGP- GART64- 64bit- FW- Rate=x1
        Kernel driver in use: gxt4500

After patching Kconfig, the driver compiled but does not work - monitor shows
no signal after driver load. Realized that PPC is big-endian while x86 is
little-endian and there are no endian conversions in the driver so did this:

--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -137,8 +137,8 @@ static const unsigned char watfmt[] = {
 /* Colormap array; 1k entries of 4 bytes each */
 #define CMAP                   0x6000

-#define readreg(par, reg)      readl((par)->regs + (reg))
-#define writereg(par, reg, val)        writel((val), (par)->regs + (reg))
+#define readreg(par, reg)      be32_to_cpu(readl((par)->regs + (reg)))
+#define writereg(par, reg, val)        writel(cpu_to_be32(val), (par)->regs + (reg))

 struct gxt4500_par {
        void __iomem *regs;


But nothing changed, still no signal. Any ideas?

I was able to get the ancient fgl23 driver (from AMD web site) to work by
installing Debian Woody to a small partition. It's not very useful today but
it might be possible to dump the chip's registers and compare with gxt4500.

-- 
Ondrej Zary

^ permalink raw reply

* [PATCH 1/2]Trivial patch: to solve indentation warnings in amba-clcd.c driver
From: Parmeshwr Prasad @ 2015-02-13 11:21 UTC (permalink / raw)
  To: linux; +Cc: plagnioj, tomi.valkeinen, trivial, linux-fbdev, linux-kernel

Hi All,


This patch soves the indentation warning in amba-clcd.c file.
Please review the same.

From a5f58880b2a6fd7cc532c4fb2bf6543cf1945195 Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
Date: Thu, 12 Feb 2015 07:30:04 -0500
Subject: [PATCH 1/2] Trivial patch: Removed indentation warnings

Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
---
 drivers/video/fbdev/amba-clcd.c | 234 ++++++++++++++++++++--------------------
 1 file changed, 118 insertions(+), 116 deletions(-)

diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 32c0b6b..0ddc1f0 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -135,8 +135,7 @@ clcdfb_set_bitfields(struct clcd_fb *fb, struct
fb_var_screeninfo *var)
                caps = fb->panel->caps & fb->board->caps;
        else {
                /* Old way of specifying what can be used */
-               caps = fb->panel->cntl & CNTL_BGR ?
-                       CLCD_CAP_BGR : CLCD_CAP_RGB;
+               caps = fb->panel->cntl & CNTL_BGR ? CLCD_CAP_BGR : CLCD_CAP_RGB;
                /* But mask out 444 modes as they weren't supported */
                caps &= ~CLCD_CAP_444;
        }
@@ -163,12 +162,12 @@ clcdfb_set_bitfields(struct clcd_fb *fb, struct
fb_var_screeninfo *var)
                        break;
                }

-               var->red.length         = var->bits_per_pixel;
-               var->red.offset         = 0;
-               var->green.length       = var->bits_per_pixel;
-               var->green.offset       = 0;
-               var->blue.length        = var->bits_per_pixel;
-               var->blue.offset        = 0;
+               var->red.length = var->bits_per_pixel;
+               var->red.offset = 0;
+               var->green.length = var->bits_per_pixel;
+               var->green.offset = 0;
+               var->blue.length = var->bits_per_pixel;
+               var->blue.offset = 0;
                break;

        case 16:
@@ -256,7 +255,8 @@ clcdfb_set_bitfields(struct clcd_fb *fb, struct
fb_var_screeninfo *var)
                } else {
                        var->red.offset = 0;
                        var->green.offset = var->red.offset + var->red.length;
-                       var->blue.offset = var->green.offset +
var->green.length;
+                       var->blue.offset +                           var->green.offset + var->green.length;
                }
        }

@@ -288,7 +288,7 @@ static int clcdfb_set_par(struct fb_info *info)
        struct clcd_regs regs;

        fb->fb.fix.line_length = fb->fb.var.xres_virtual *
-                                fb->fb.var.bits_per_pixel / 8;
+           fb->fb.var.bits_per_pixel / 8;

        if (fb->fb.var.bits_per_pixel <= 8)
                fb->fb.fix.visual = FB_VISUAL_PSEUDOCOLOR;
@@ -317,10 +317,10 @@ static int clcdfb_set_par(struct fb_info *info)
               "CLCD: Registers set to\n"
               "  %08x %08x %08x %08x\n"
               "  %08x %08x %08x %08x\n",
-               readl(fb->regs + CLCD_TIM0), readl(fb->regs + CLCD_TIM1),
-               readl(fb->regs + CLCD_TIM2), readl(fb->regs + CLCD_TIM3),
-               readl(fb->regs + CLCD_UBAS), readl(fb->regs + CLCD_LBAS),
-               readl(fb->regs + fb->off_ienb), readl(fb->regs + fb->off_cntl));
+              readl(fb->regs + CLCD_TIM0), readl(fb->regs + CLCD_TIM1),
+              readl(fb->regs + CLCD_TIM2), readl(fb->regs + CLCD_TIM3),
+              readl(fb->regs + CLCD_UBAS), readl(fb->regs + CLCD_LBAS),
+              readl(fb->regs + fb->off_ienb), readl(fb->regs + fb->off_cntl));
 #endif

        return 0;
@@ -345,17 +345,17 @@ clcdfb_setcolreg(unsigned int regno, unsigned int red,
unsigned int green,

        if (regno < 16)
                fb->cmap[regno] = convert_bitfield(transp, &fb->fb.var.transp) |
-                                 convert_bitfield(blue, &fb->fb.var.blue) |
-                                 convert_bitfield(green, &fb->fb.var.green) |
-                                 convert_bitfield(red, &fb->fb.var.red);
+                   convert_bitfield(blue, &fb->fb.var.blue) |
+                   convert_bitfield(green, &fb->fb.var.green) |
+                   convert_bitfield(red, &fb->fb.var.red);

        if (fb->fb.fix.visual = FB_VISUAL_PSEUDOCOLOR && regno < 256) {
                int hw_reg = CLCD_PALETTE + ((regno * 2) & ~3);
                u32 val, mask, newval;

-               newval  = (red >> 11)  & 0x001f;
+               newval = (red >> 11) & 0x001f;
                newval |= (green >> 6) & 0x03e0;
-               newval |= (blue >> 1)  & 0x7c00;
+               newval |= (blue >> 1) & 0x7c00;

                /*
                 * 3.2.11: if we're configured for big endian
@@ -400,8 +400,7 @@ static int clcdfb_blank(int blank_mode, struct fb_info
*info)
        return 0;
 }

-static int clcdfb_mmap(struct fb_info *info,
-                      struct vm_area_struct *vma)
+static int clcdfb_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
        struct clcd_fb *fb = to_clcd(info);
        unsigned long len, off = vma->vm_pgoff << PAGE_SHIFT;
@@ -417,15 +416,15 @@ static int clcdfb_mmap(struct fb_info *info,
 }

 static struct fb_ops clcdfb_ops = {
-       .owner          = THIS_MODULE,
-       .fb_check_var   = clcdfb_check_var,
-       .fb_set_par     = clcdfb_set_par,
-       .fb_setcolreg   = clcdfb_setcolreg,
-       .fb_blank       = clcdfb_blank,
-       .fb_fillrect    = cfb_fillrect,
-       .fb_copyarea    = cfb_copyarea,
-       .fb_imageblit   = cfb_imageblit,
-       .fb_mmap        = clcdfb_mmap,
+       .owner = THIS_MODULE,
+       .fb_check_var = clcdfb_check_var,
+       .fb_set_par = clcdfb_set_par,
+       .fb_setcolreg = clcdfb_setcolreg,
+       .fb_blank = clcdfb_blank,
+       .fb_fillrect = cfb_fillrect,
+       .fb_copyarea = cfb_copyarea,
+       .fb_imageblit = cfb_imageblit,
+       .fb_mmap = clcdfb_mmap,
 };

 static int clcdfb_register(struct clcd_fb *fb)
@@ -459,10 +458,10 @@ static int clcdfb_register(struct clcd_fb *fb)
        if (ret)
                goto free_clk;

-       fb->fb.device           = &fb->dev->dev;
+       fb->fb.device = &fb->dev->dev;

-       fb->fb.fix.mmio_start   = fb->dev->res.start;
-       fb->fb.fix.mmio_len     = resource_size(&fb->dev->res);
+       fb->fb.fix.mmio_start = fb->dev->res.start;
+       fb->fb.fix.mmio_len = resource_size(&fb->dev->res);

        fb->regs = ioremap(fb->fb.fix.mmio_start, fb->fb.fix.mmio_len);
        if (!fb->regs) {
@@ -471,45 +470,45 @@ static int clcdfb_register(struct clcd_fb *fb)
                goto clk_unprep;
        }

-       fb->fb.fbops            = &clcdfb_ops;
-       fb->fb.flags            = FBINFO_FLAG_DEFAULT;
-       fb->fb.pseudo_palette   = fb->cmap;
+       fb->fb.fbops = &clcdfb_ops;
+       fb->fb.flags = FBINFO_FLAG_DEFAULT;
+       fb->fb.pseudo_palette = fb->cmap;

        strncpy(fb->fb.fix.id, clcd_name, sizeof(fb->fb.fix.id));
-       fb->fb.fix.type         = FB_TYPE_PACKED_PIXELS;
-       fb->fb.fix.type_aux     = 0;
-       fb->fb.fix.xpanstep     = 0;
-       fb->fb.fix.ypanstep     = 0;
-       fb->fb.fix.ywrapstep    = 0;
-       fb->fb.fix.accel        = FB_ACCEL_NONE;
-
-       fb->fb.var.xres         = fb->panel->mode.xres;
-       fb->fb.var.yres         = fb->panel->mode.yres;
-       fb->fb.var.xres_virtual = fb->panel->mode.xres;
-       fb->fb.var.yres_virtual = fb->panel->mode.yres;
+       fb->fb.fix.type = FB_TYPE_PACKED_PIXELS;
+       fb->fb.fix.type_aux = 0;
+       fb->fb.fix.xpanstep = 0;
+       fb->fb.fix.ypanstep = 0;
+       fb->fb.fix.ywrapstep = 0;
+       fb->fb.fix.accel = FB_ACCEL_NONE;
+
+       fb->fb.var.xres = fb->panel->mode.xres;
+       fb->fb.var.yres = fb->panel->mode.yres;
+       fb->fb.var.xres_virtual = fb->panel->mode.xres;
+       fb->fb.var.yres_virtual = fb->panel->mode.yres;
        fb->fb.var.bits_per_pixel = fb->panel->bpp;
-       fb->fb.var.grayscale    = fb->panel->grayscale;
-       fb->fb.var.pixclock     = fb->panel->mode.pixclock;
-       fb->fb.var.left_margin  = fb->panel->mode.left_margin;
-       fb->fb.var.right_margin = fb->panel->mode.right_margin;
-       fb->fb.var.upper_margin = fb->panel->mode.upper_margin;
-       fb->fb.var.lower_margin = fb->panel->mode.lower_margin;
-       fb->fb.var.hsync_len    = fb->panel->mode.hsync_len;
-       fb->fb.var.vsync_len    = fb->panel->mode.vsync_len;
-       fb->fb.var.sync         = fb->panel->mode.sync;
-       fb->fb.var.vmode        = fb->panel->mode.vmode;
-       fb->fb.var.activate     = FB_ACTIVATE_NOW;
-       fb->fb.var.nonstd       = 0;
-       fb->fb.var.height       = fb->panel->height;
-       fb->fb.var.width        = fb->panel->width;
-       fb->fb.var.accel_flags  = 0;
-
-       fb->fb.monspecs.hfmin   = 0;
-       fb->fb.monspecs.hfmax   = 100000;
-       fb->fb.monspecs.vfmin   = 0;
-       fb->fb.monspecs.vfmax   = 400;
+       fb->fb.var.grayscale = fb->panel->grayscale;
+       fb->fb.var.pixclock = fb->panel->mode.pixclock;
+       fb->fb.var.left_margin = fb->panel->mode.left_margin;
+       fb->fb.var.right_margin = fb->panel->mode.right_margin;
+       fb->fb.var.upper_margin = fb->panel->mode.upper_margin;
+       fb->fb.var.lower_margin = fb->panel->mode.lower_margin;
+       fb->fb.var.hsync_len = fb->panel->mode.hsync_len;
+       fb->fb.var.vsync_len = fb->panel->mode.vsync_len;
+       fb->fb.var.sync = fb->panel->mode.sync;
+       fb->fb.var.vmode = fb->panel->mode.vmode;
+       fb->fb.var.activate = FB_ACTIVATE_NOW;
+       fb->fb.var.nonstd = 0;
+       fb->fb.var.height = fb->panel->height;
+       fb->fb.var.width = fb->panel->width;
+       fb->fb.var.accel_flags = 0;
+
+       fb->fb.monspecs.hfmin = 0;
+       fb->fb.monspecs.hfmax = 100000;
+       fb->fb.monspecs.vfmin = 0;
+       fb->fb.monspecs.vfmax = 400;
        fb->fb.monspecs.dclkmin = 1000000;
-       fb->fb.monspecs.dclkmax = 100000000;
+       fb->fb.monspecs.dclkmax = 100000000;

        /*
         * Make sure that the bitfields are set appropriately.
@@ -531,7 +530,7 @@ static int clcdfb_register(struct clcd_fb *fb)
        fb_set_var(&fb->fb, &fb->fb.var);

        dev_info(&fb->dev->dev, "%s hardware, %s display\n",
-                fb->board->name, fb->panel->mode.name);
+                fb->board->name, fb->panel->mode.name);

        ret = register_framebuffer(&fb->fb);
        if (ret = 0)
@@ -540,19 +539,19 @@ static int clcdfb_register(struct clcd_fb *fb)
        printk(KERN_ERR "CLCD: cannot register framebuffer (%d)\n", ret);

        fb_dealloc_cmap(&fb->fb.cmap);
- unmap:
+unmap:
        iounmap(fb->regs);
- clk_unprep:
+clk_unprep:
        clk_unprepare(fb->clk);
- free_clk:
+free_clk:
        clk_put(fb->clk);
- out:
+out:
        return ret;
 }

 #ifdef CONFIG_OF
 static int clcdfb_of_get_dpi_panel_mode(struct device_node *node,
-               struct fb_videomode *mode)
+                                       struct fb_videomode *mode)
 {
        int err;
        struct display_timing timing;
@@ -578,7 +577,7 @@ static int clcdfb_snprintf_mode(char *buf, int size, struct
fb_videomode *mode)
 }

 static int clcdfb_of_get_mode(struct device *dev, struct device_node *endpoint,
-               struct fb_videomode *mode)
+                             struct fb_videomode *mode)
 {
        int err;
        struct device_node *panel;
@@ -612,15 +611,15 @@ static int clcdfb_of_init_tft_panel(struct clcd_fb *fb,
u32 r0, u32 g0, u32 b0)
                u32 r0, g0, b0;
                u32 caps;
        } panels[] = {
-               { 0x110, 1,  7, 13, CLCD_CAP_5551 },
-               { 0x110, 0,  8, 16, CLCD_CAP_888 },
-               { 0x111, 4, 14, 20, CLCD_CAP_444 },
-               { 0x111, 3, 11, 19, CLCD_CAP_444 | CLCD_CAP_5551 },
-               { 0x111, 3, 10, 19, CLCD_CAP_444 | CLCD_CAP_5551 |
-                                   CLCD_CAP_565 },
-               { 0x111, 0,  8, 16, CLCD_CAP_444 | CLCD_CAP_5551 |
-                                   CLCD_CAP_565 | CLCD_CAP_888 },
-       };
+               {
+               0x110, 1, 7, 13, CLCD_CAP_5551}, {
+               0x110, 0, 8, 16, CLCD_CAP_888}, {
+               0x111, 4, 14, 20, CLCD_CAP_444}, {
+               0x111, 3, 11, 19, CLCD_CAP_444 | CLCD_CAP_5551}, {
+               0x111, 3, 10, 19, CLCD_CAP_444 | CLCD_CAP_5551 |
+                           CLCD_CAP_565}, {
+       0x111, 0, 8, 16, CLCD_CAP_444 | CLCD_CAP_5551 |
+                           CLCD_CAP_565 | CLCD_CAP_888},};
        int i;

        /* Bypass pixel clock divider, data output on the falling edge */
@@ -665,7 +664,7 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
                return err;

        err = of_property_read_u32(fb->dev->dev.of_node, "max-memory-bandwidth",
-                       &max_bandwidth);
+                                  &max_bandwidth);
        if (!err) {
                /*
                 * max_bandwidth is in bytes per second and pixclock in
@@ -675,7 +674,7 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
                 * result is a valid format.
                 */
                bpp = max_bandwidth / (1000 / 8)
-                       / PICOS2KHZ(fb->panel->mode.pixclock);
+                   / PICOS2KHZ(fb->panel->mode.pixclock);
                bpp = rounddown_pow_of_two(bpp);
                if (bpp > 32)
                        bpp = 32;
@@ -690,10 +689,10 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
        fb->panel->height = -1;

        if (of_property_read_u32_array(endpoint,
-                       "arm,pl11x,tft-r0g0b0-pads",
-                       tft_r0b0g0, ARRAY_SIZE(tft_r0b0g0)) = 0)
+                                      "arm,pl11x,tft-r0g0b0-pads",
+                                      tft_r0b0g0, ARRAY_SIZE(tft_r0b0g0)) = 0)
                return clcdfb_of_init_tft_panel(fb, tft_r0b0g0[0],
-                                tft_r0b0g0[1],  tft_r0b0g0[2]);
+                                               tft_r0b0g0[1], tft_r0b0g0[2]);

        return -ENOENT;
 }
@@ -717,7 +716,9 @@ static int clcdfb_of_vram_setup(struct clcd_fb *fb)
                return -ENOMEM;

        fb->fb.fix.smem_start = of_translate_address(memory,
-                       of_get_address(memory, 0, &size, NULL));
+                                                    of_get_address(memory, 0,
+                                                                   &size,
+                                                                   NULL));
        fb->fb.fix.smem_len = size;

        return 0;
@@ -727,7 +728,6 @@ static int clcdfb_of_vram_mmap(struct clcd_fb *fb, struct
vm_area_struct *vma)
 {
        unsigned long off, user_size, kernel_size;

-
        off = vma->vm_pgoff << PAGE_SHIFT;
        user_size = vma->vm_end - vma->vm_start;
        kernel_size = fb->fb.fix.smem_len;
@@ -736,9 +736,9 @@ static int clcdfb_of_vram_mmap(struct clcd_fb *fb, struct
vm_area_struct *vma)
                return -ENXIO;

        return remap_pfn_range(vma, vma->vm_start,
-                       __phys_to_pfn(fb->fb.fix.smem_start) + vma->vm_pgoff,
-                       user_size,
-                       pgprot_writecombine(vma->vm_page_prot));
+                              __phys_to_pfn(fb->fb.fix.smem_start) +
+                              vma->vm_pgoff, user_size,
+                              pgprot_writecombine(vma->vm_page_prot));
 }

 static void clcdfb_of_vram_remove(struct clcd_fb *fb)
@@ -757,9 +757,9 @@ static int clcdfb_of_dma_setup(struct clcd_fb *fb)
                return err;

        framesize = fb->panel->mode.xres * fb->panel->mode.yres *
-                       fb->panel->bpp / 8;
+           fb->panel->bpp / 8;
        fb->fb.screen_base = dma_alloc_coherent(&fb->dev->dev, framesize,
-                       &dma, GFP_KERNEL);
+                                               &dma, GFP_KERNEL);
        if (!fb->fb.screen_base)
                return -ENOMEM;

@@ -772,19 +772,20 @@ static int clcdfb_of_dma_setup(struct clcd_fb *fb)
 static int clcdfb_of_dma_mmap(struct clcd_fb *fb, struct vm_area_struct *vma)
 {
        return dma_mmap_writecombine(&fb->dev->dev, vma, fb->fb.screen_base,
-                       fb->fb.fix.smem_start, fb->fb.fix.smem_len);
+                                    fb->fb.fix.smem_start,
+                                    fb->fb.fix.smem_len);
 }

 static void clcdfb_of_dma_remove(struct clcd_fb *fb)
 {
        dma_free_coherent(&fb->dev->dev, fb->fb.fix.smem_len,
-                       fb->fb.screen_base, fb->fb.fix.smem_start);
+                         fb->fb.screen_base, fb->fb.fix.smem_start);
 }

 static struct clcd_board *clcdfb_of_get_board(struct amba_device *dev)
 {
        struct clcd_board *board = devm_kzalloc(&dev->dev, sizeof(*board),
-                       GFP_KERNEL);
+                                               GFP_KERNEL);
        struct device_node *node = dev->dev.of_node;

        if (!board)
@@ -837,7 +838,8 @@ static int clcdfb_probe(struct amba_device *dev, const
struct amba_id *id)

        fb = kzalloc(sizeof(struct clcd_fb), GFP_KERNEL);
        if (!fb) {
-               printk(KERN_INFO "CLCD: could not allocate new clcd_fb
struct\n");
+               printk(KERN_INFO
+                      "CLCD: could not allocate new clcd_fb struct\n");
                ret = -ENOMEM;
                goto free_region;
        }
@@ -846,25 +848,25 @@ static int clcdfb_probe(struct amba_device *dev, const
struct amba_id *id)
        fb->board = board;

        dev_info(&fb->dev->dev, "PL%03x rev%u at 0x%08llx\n",
-               amba_part(dev), amba_rev(dev),
-               (unsigned long long)dev->res.start);
+                amba_part(dev), amba_rev(dev),
+                (unsigned long long)dev->res.start);

        ret = fb->board->setup(fb);
        if (ret)
                goto free_fb;

-       ret = clcdfb_register(fb);
+       ret = clcdfb_register(fb);
        if (ret = 0) {
                amba_set_drvdata(dev, fb);
                goto out;
        }

        fb->board->remove(fb);
- free_fb:
+free_fb:
        kfree(fb);
- free_region:
+free_region:
        amba_release_regions(dev);
- out:
+out:
        return ret;
 }

@@ -891,21 +893,21 @@ static int clcdfb_remove(struct amba_device *dev)

 static struct amba_id clcdfb_id_table[] = {
        {
-               .id     = 0x00041110,
-               .mask   = 0x000ffffe,
-       },
-       { 0, 0 },
+        .id = 0x00041110,
+        .mask = 0x000ffffe,
+        },
+       {0, 0},
 };

 MODULE_DEVICE_TABLE(amba, clcdfb_id_table);

 static struct amba_driver clcd_driver = {
-       .drv            = {
-               .name   = "clcd-pl11x",
-       },
-       .probe          = clcdfb_probe,
-       .remove         = clcdfb_remove,
-       .id_table       = clcdfb_id_table,
+       .drv = {
+               .name = "clcd-pl11x",
+               },
+       .probe = clcdfb_probe,
+       .remove = clcdfb_remove,
+       .id_table = clcdfb_id_table,
 };

 static int __init amba_clcdfb_init(void)
--
1.9.3


-Parmeshwr

^ permalink raw reply related

* Re: [PATCH 1/2]Trivial patch: to solve indentation warnings in amba-clcd.c driver
From: Russell King - ARM Linux @ 2015-02-13 11:35 UTC (permalink / raw)
  To: Parmeshwr Prasad
  Cc: plagnioj, tomi.valkeinen, trivial, linux-fbdev, linux-kernel
In-Reply-To: <20150213112125.GA25078@linuxteamdev.amer.dell.com>

On Fri, Feb 13, 2015 at 06:21:33AM -0500, Parmeshwr Prasad wrote:
> @@ -288,7 +288,7 @@ static int clcdfb_set_par(struct fb_info *info)
>         struct clcd_regs regs;
> 
>         fb->fb.fix.line_length = fb->fb.var.xres_virtual *
> -                                fb->fb.var.bits_per_pixel / 8;
> +           fb->fb.var.bits_per_pixel / 8;

NAK on this one.  The code as it stood before is much clearer since
we align the expression with the start of it on the preceding line.

> @@ -345,17 +345,17 @@ clcdfb_setcolreg(unsigned int regno, unsigned int red,
> unsigned int green,
> 
>         if (regno < 16)
>                 fb->cmap[regno] = convert_bitfield(transp, &fb->fb.var.transp) |
> -                                 convert_bitfield(blue, &fb->fb.var.blue) |
> -                                 convert_bitfield(green, &fb->fb.var.green) |
> -                                 convert_bitfield(red, &fb->fb.var.red);
> +                   convert_bitfield(blue, &fb->fb.var.blue) |
> +                   convert_bitfield(green, &fb->fb.var.green) |
> +                   convert_bitfield(red, &fb->fb.var.red);

Ditto.

> @@ -612,15 +611,15 @@ static int clcdfb_of_init_tft_panel(struct clcd_fb *fb,
> u32 r0, u32 g0, u32 b0)
>                 u32 r0, g0, b0;
>                 u32 caps;
>         } panels[] = {
> -               { 0x110, 1,  7, 13, CLCD_CAP_5551 },
> -               { 0x110, 0,  8, 16, CLCD_CAP_888 },
> -               { 0x111, 4, 14, 20, CLCD_CAP_444 },
> -               { 0x111, 3, 11, 19, CLCD_CAP_444 | CLCD_CAP_5551 },
> -               { 0x111, 3, 10, 19, CLCD_CAP_444 | CLCD_CAP_5551 |
> -                                   CLCD_CAP_565 },
> -               { 0x111, 0,  8, 16, CLCD_CAP_444 | CLCD_CAP_5551 |
> -                                   CLCD_CAP_565 | CLCD_CAP_888 },
> -       };
> +               {
> +               0x110, 1, 7, 13, CLCD_CAP_5551}, {
> +               0x110, 0, 8, 16, CLCD_CAP_888}, {
> +               0x111, 4, 14, 20, CLCD_CAP_444}, {
> +               0x111, 3, 11, 19, CLCD_CAP_444 | CLCD_CAP_5551}, {
> +               0x111, 3, 10, 19, CLCD_CAP_444 | CLCD_CAP_5551 |
> +                           CLCD_CAP_565}, {
> +       0x111, 0, 8, 16, CLCD_CAP_444 | CLCD_CAP_5551 |
> +                           CLCD_CAP_565 | CLCD_CAP_888},};

The original is better than what you're proposing.

> @@ -675,7 +674,7 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
>                  * result is a valid format.
>                  */
>                 bpp = max_bandwidth / (1000 / 8)
> -                       / PICOS2KHZ(fb->panel->mode.pixclock);
> +                   / PICOS2KHZ(fb->panel->mode.pixclock);

Not quite enough indentation.

> @@ -717,7 +716,9 @@ static int clcdfb_of_vram_setup(struct clcd_fb *fb)
>                 return -ENOMEM;
> 
>         fb->fb.fix.smem_start = of_translate_address(memory,
> -                       of_get_address(memory, 0, &size, NULL));
> +                                                    of_get_address(memory, 0,
> +                                                                   &size,
> +                                                                   NULL));

Thi sis the exception to the rule - where scrunching an expression so that
it takes multiple lines because of lack of right-hand space is not on.
The former version was a lot better.

> @@ -757,9 +757,9 @@ static int clcdfb_of_dma_setup(struct clcd_fb *fb)
>                 return err;
> 
>         framesize = fb->panel->mode.xres * fb->panel->mode.yres *
> -                       fb->panel->bpp / 8;
> +           fb->panel->bpp / 8;

Similar comments as before.

> @@ -837,7 +838,8 @@ static int clcdfb_probe(struct amba_device *dev, const
> struct amba_id *id)
> 
>         fb = kzalloc(sizeof(struct clcd_fb), GFP_KERNEL);
>         if (!fb) {
> -               printk(KERN_INFO "CLCD: could not allocate new clcd_fb
> struct\n");
> +               printk(KERN_INFO
> +                      "CLCD: could not allocate new clcd_fb struct\n");

Converting this to pr_info() would be better, or even dev_err().

> @@ -891,21 +893,21 @@ static int clcdfb_remove(struct amba_device *dev)
> 
>  static struct amba_id clcdfb_id_table[] = {
>         {
> -               .id     = 0x00041110,
> -               .mask   = 0x000ffffe,
> -       },
> -       { 0, 0 },
> +        .id = 0x00041110,
> +        .mask = 0x000ffffe,
> +        },
> +       {0, 0},

The original is loads better.

>  };
> 
>  MODULE_DEVICE_TABLE(amba, clcdfb_id_table);
> 
>  static struct amba_driver clcd_driver = {
> -       .drv            = {
> -               .name   = "clcd-pl11x",
> -       },
> -       .probe          = clcdfb_probe,
> -       .remove         = clcdfb_remove,
> -       .id_table       = clcdfb_id_table,
> +       .drv = {
> +               .name = "clcd-pl11x",
> +               },

Yuck.  Again, the original is loads better.

And... your message seems to suffer total whitespace breakage - all
tabs seem to be converted to 8 spaces.  Your patch will not apply
as long as that kind of breakage exists...

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* Re: [PATCH 1/2]Trivial patch: to solve indentation warnings in amba-clcd.c driver
From: Joe Perches @ 2015-02-13 11:59 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Parmeshwr Prasad, plagnioj, tomi.valkeinen, trivial, linux-fbdev,
	linux-kernel
In-Reply-To: <20150213113515.GW8656@n2100.arm.linux.org.uk>

On Fri, 2015-02-13 at 11:35 +0000, Russell King - ARM Linux wrote:
> On Fri, Feb 13, 2015 at 06:21:33AM -0500, Parmeshwr Prasad wrote:
> > @@ -288,7 +288,7 @@ static int clcdfb_set_par(struct fb_info *info)
> >         struct clcd_regs regs;
> > 
> >         fb->fb.fix.line_length = fb->fb.var.xres_virtual *
> > -                                fb->fb.var.bits_per_pixel / 8;
> > +           fb->fb.var.bits_per_pixel / 8;
> 
> NAK on this one.  The code as it stood before is much clearer since
> we align the expression with the start of it on the preceding line.

I agree with all of what Russell wrote, but maybe this;

> > @@ -717,7 +716,9 @@ static int clcdfb_of_vram_setup(struct clcd_fb *fb)
> >                 return -ENOMEM;
> > 
> >         fb->fb.fix.smem_start = of_translate_address(memory,
> > -                       of_get_address(memory, 0, &size, NULL));
> > +                                                    of_get_address(memory, 0,
> > +                                                                   &size,
> > +                                                                   NULL));
> 
> Thi sis the exception to the rule - where scrunching an expression so that
> it takes multiple lines because of lack of right-hand space is not on.
> The former version was a lot better.

Perhaps this could be better as:

	fb->fb.fix.smem_start 		 of_translate_address(memory,
				      of_get_address(memory, 0, &size, NULL));

But sometimes using multiple statements instead of
embedding function calls as arguments can be simpler
and more intelligible for the reader.

	__be32 addr;

	...

	addr = of_get_address(memory, 0, &size, NULL);
	fb->fb.fix.smem_start = of_translate_address(memory, addr);




^ permalink raw reply

* Re: [PATCH 1/2]Trivial patch: to solve indentation warnings in amba-clcd.c driver
From: Russell King - ARM Linux @ 2015-02-13 12:03 UTC (permalink / raw)
  To: Joe Perches
  Cc: Parmeshwr Prasad, plagnioj, tomi.valkeinen, trivial, linux-fbdev,
	linux-kernel
In-Reply-To: <1423828796.2795.7.camel@perches.com>

On Fri, Feb 13, 2015 at 03:59:56AM -0800, Joe Perches wrote:
> Perhaps this could be better as:
> 
> 	fb->fb.fix.smem_start > 		 of_translate_address(memory,
> 				      of_get_address(memory, 0, &size, NULL));
> 
> But sometimes using multiple statements instead of
> embedding function calls as arguments can be simpler
> and more intelligible for the reader.
> 
> 	__be32 addr;
> 
> 	...
> 
> 	addr = of_get_address(memory, 0, &size, NULL);
> 	fb->fb.fix.smem_start = of_translate_address(memory, addr);

Yes, that would be a better solution, thanks.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* Re: [PATCH 2/2]Trivial patch: to solve indentation warnings in amba-clcd.c driver
From: Parmeshwr Prasad @ 2015-02-13 13:28 UTC (permalink / raw)
  To: Joe Perches
  Cc: Russell King - ARM Linux, plagnioj@jcrosoft.com,
	tomi.valkeinen@ti.com, trivial@kernel.org,
	linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1423828796.2795.7.camel@perches.com>


This is second patch in series.
In driver in_atomic we should not use to check if code is unning in IRQ.
clcdfb_sleep() function is used to give some delay between operation.
I have used schedule_timeout() function for same amount of delay.

Please review the same.
Patch 1/2 I will fix comments and re-submitt.
From 06b275e44a3951ec0a57af59aea2a4c82595e456 Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
Date: Fri, 13 Feb 2015 06:10:34 -0500
Subject: [PATCH 2/2] Patch to replace in_atomic with proper function from
 amba-clcd.c driver. In driver code in_atomic should not be used to check the
 IRQ or kernel context. to remove that I have used schedule_timeout() with
 respective delay. In this patch I have also removed printk() with respective
 pr_* functions

Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
---
 drivers/video/fbdev/amba-clcd.c | 48 +++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 0ddc1f0..7e630f5c 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -33,8 +33,9 @@
 #include <video/display_timing.h>
 #include <video/of_display_timing.h>
 #include <video/videomode.h>
-
-#include <asm/sizes.h>
+#include <linux/jiffies.h>
+#include <linux/sched.h>
+#include <linux/sizes.h>

 #define to_clcd(info)  container_of(info, struct clcd_fb, fb)

@@ -42,16 +43,15 @@
 static const char *clcd_name = "CLCD FB";

 /*
- * Unfortunately, the enable/disable functions may be called either from
- * process or IRQ context, and we _need_ to delay.  This is _not_ good.
+ * Hardware need certain time for power setting
  */
-static inline void clcdfb_sleep(unsigned int ms)
+static inline void clcdfb_sleep(signed long ms)
 {
-       if (in_atomic()) {
-               mdelay(ms);
-       } else {
-               msleep(ms);
-       }
+       unsigned long delay;
+
+       delay = jiffies + (HZ / 1000) * ms;
+       set_current_state(TASK_INTERRUPTIBLE);
+       schedule_timeout(delay);
 }

 static inline void clcdfb_set_start(struct clcd_fb *fb)
@@ -313,14 +313,13 @@ static int clcdfb_set_par(struct fb_info *info)
        clcdfb_enable(fb, regs.cntl);

 #ifdef DEBUG
-       printk(KERN_INFO
-              "CLCD: Registers set to\n"
-              "  %08x %08x %08x %08x\n"
-              "  %08x %08x %08x %08x\n",
-              readl(fb->regs + CLCD_TIM0), readl(fb->regs + CLCD_TIM1),
-              readl(fb->regs + CLCD_TIM2), readl(fb->regs + CLCD_TIM3),
-              readl(fb->regs + CLCD_UBAS), readl(fb->regs + CLCD_LBAS),
-              readl(fb->regs + fb->off_ienb), readl(fb->regs + fb->off_cntl));
+       pr_info("CLCD: Registers set to\n"
+               "  %08x %08x %08x %08x\n"
+               "  %08x %08x %08x %08x\n",
+               readl(fb->regs + CLCD_TIM0), readl(fb->regs + CLCD_TIM1),
+               readl(fb->regs + CLCD_TIM2), readl(fb->regs + CLCD_TIM3),
+               readl(fb->regs + CLCD_UBAS), readl(fb->regs + CLCD_LBAS),
+               readl(fb->regs + fb->off_ienb), readl(fb->regs + fb->off_cntl));
 #endif

        return 0;
@@ -392,11 +391,10 @@ static int clcdfb_blank(int blank_mode, struct fb_info
*info)
 {
        struct clcd_fb *fb = to_clcd(info);

-       if (blank_mode != 0) {
+       if (blank_mode != 0)
                clcdfb_disable(fb);
-       } else {
+       else
                clcdfb_enable(fb, fb->clcd_cntl);
-       }
        return 0;
 }

@@ -465,7 +463,7 @@ static int clcdfb_register(struct clcd_fb *fb)

        fb->regs = ioremap(fb->fb.fix.mmio_start, fb->fb.fix.mmio_len);
        if (!fb->regs) {
-               printk(KERN_ERR "CLCD: unable to remap registers\n");
+               pr_err("CLCD: unable to remap registers\n");
                ret = -ENOMEM;
                goto clk_unprep;
        }
@@ -536,7 +534,7 @@ static int clcdfb_register(struct clcd_fb *fb)
        if (ret = 0)
                goto out;

-       printk(KERN_ERR "CLCD: cannot register framebuffer (%d)\n", ret);
+       pr_info("CLCD: cannot register framebuffer (%d)\n", ret);

        fb_dealloc_cmap(&fb->fb.cmap);
 unmap:
@@ -832,14 +830,12 @@ static int clcdfb_probe(struct amba_device *dev, const
struct amba_id *id)

        ret = amba_request_regions(dev, NULL);
        if (ret) {
-               printk(KERN_ERR "CLCD: unable to reserve regs region\n");
+               pr_err("CLCD: unable to reserve regs region\n");
                goto out;
        }

        fb = kzalloc(sizeof(struct clcd_fb), GFP_KERNEL);
        if (!fb) {
-               printk(KERN_INFO
-                      "CLCD: could not allocate new clcd_fb struct\n");
                ret = -ENOMEM;
                goto free_region;
        }
--
1.9.3

-Parmeshwr
On Fri, Feb 13, 2015 at 05:59:56AM -0600, Joe Perches wrote:
> On Fri, 2015-02-13 at 11:35 +0000, Russell King - ARM Linux wrote:
> > On Fri, Feb 13, 2015 at 06:21:33AM -0500, Parmeshwr Prasad wrote:
> > > @@ -288,7 +288,7 @@ static int clcdfb_set_par(struct fb_info *info)
> > >         struct clcd_regs regs;
> > > 
> > >         fb->fb.fix.line_length = fb->fb.var.xres_virtual *
> > > -                                fb->fb.var.bits_per_pixel / 8;
> > > +           fb->fb.var.bits_per_pixel / 8;
> > 
> > NAK on this one.  The code as it stood before is much clearer since
> > we align the expression with the start of it on the preceding line.
> 
> I agree with all of what Russell wrote, but maybe this;
> 
> > > @@ -717,7 +716,9 @@ static int clcdfb_of_vram_setup(struct clcd_fb *fb)
> > >                 return -ENOMEM;
> > > 
> > >         fb->fb.fix.smem_start = of_translate_address(memory,
> > > -                       of_get_address(memory, 0, &size, NULL));
> > > +                                                    of_get_address(memory, 0,
> > > +                                                                   &size,
> > > +                                                                   NULL));
> > 
> > Thi sis the exception to the rule - where scrunching an expression so that
> > it takes multiple lines because of lack of right-hand space is not on.
> > The former version was a lot better.
> 
> Perhaps this could be better as:
> 
> 	fb->fb.fix.smem_start > 		 of_translate_address(memory,
> 				      of_get_address(memory, 0, &size, NULL));
> 
> But sometimes using multiple statements instead of
> embedding function calls as arguments can be simpler
> and more intelligible for the reader.
> 
> 	__be32 addr;
> 
> 	...
> 
> 	addr = of_get_address(memory, 0, &size, NULL);
> 	fb->fb.fix.smem_start = of_translate_address(memory, addr);
> 
> 
> 

^ permalink raw reply related

* Re: [PATCH 2/2]Trivial patch: to solve indentation warnings in amba-clcd.c driver
From: Russell King - ARM Linux @ 2015-02-13 13:36 UTC (permalink / raw)
  To: Parmeshwr Prasad
  Cc: Joe Perches, plagnioj@jcrosoft.com, tomi.valkeinen@ti.com,
	trivial@kernel.org, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20150213132805.GA1296@linuxteamdev.amer.dell.com>

On Fri, Feb 13, 2015 at 08:28:10AM -0500, Parmeshwr Prasad wrote:
> 
> This is second patch in series.
> In driver in_atomic we should not use to check if code is unning in IRQ.
> clcdfb_sleep() function is used to give some delay between operation.
> I have used schedule_timeout() function for same amount of delay.

Frankly, this patch is a mess.  It seems to contain unrelated changes.

Please always review your own patches before you send them - this will
allow you to catch such errors before you post them publically.

The change to clcdfb_sleep() is wrong in any case - you will end up
calling schedule_timeout() from illegal contexts (eg, when the
framebuffer gets blanked/unblanked.)

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* Re: [PATCH 2/2]Trivial patch: to solve indentation warnings in amba-clcd.c driver
From: Sudip Mukherjee @ 2015-02-14 10:20 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Parmeshwr Prasad, Joe Perches, plagnioj@jcrosoft.com,
	tomi.valkeinen@ti.com, trivial@kernel.org,
	linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20150213133618.GZ8656@n2100.arm.linux.org.uk>

On Fri, Feb 13, 2015 at 01:36:18PM +0000, Russell King - ARM Linux wrote:
> On Fri, Feb 13, 2015 at 08:28:10AM -0500, Parmeshwr Prasad wrote:
> > 
> > This is second patch in series.
> > In driver in_atomic we should not use to check if code is unning in IRQ.
> > clcdfb_sleep() function is used to give some delay between operation.
> > I have used schedule_timeout() function for same amount of delay.
> 
> Frankly, this patch is a mess.  It seems to contain unrelated changes.
and this patch is corrupted. your commit message is a mess and the maintainer has to edit it by hand before applying.
if i remember correctly you have been told multiple times by many people to fix your commit message.
and try to use git send-email to send your patches.

regards
sudip


> 
> Please always review your own patches before you send them - this will
> allow you to catch such errors before you post them publically.
> 
> The change to clcdfb_sleep() is wrong in any case - you will end up
> calling schedule_timeout() from illegal contexts (eg, when the
> framebuffer gets blanked/unblanked.)
> 
> -- 

^ permalink raw reply

* Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Thomas Niederprüm @ 2015-02-14 14:22 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150212151121.GJ2079@lukather>

Am Thu, 12 Feb 2015 16:11:21 +0100
schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:

> On Sat, Feb 07, 2015 at 04:35:41PM +0100, Thomas Niederprüm wrote:
> > Am Sat, 7 Feb 2015 12:18:21 +0100
> > schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> > 
> > > Hi,
> > > 
> > > On Fri, Feb 06, 2015 at 11:28:10PM +0100, niederp@physik.uni-kl.de
> > > wrote:
> > > > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > > > 
> > > > It makes sense to use vmalloc to allocate the video buffer
> > > > since it has to be page aligned memory for using it with mmap.
> > > 
> > > Please wrap your commit log at 80 chars.
> > 
> > I'll try to do so in future, sorry for that.
> > 
> > > 
> > > It looks like there's numerous fbdev drivers using this
> > > (especially since you copy pasted that code, without mentionning
> > > it).
> > 
> > Yes, I should have mentioned that in the commit message. As
> > implicitly indicated in the cover letter the rvmalloc() and
> > rvfree() are copy pasted from the vfb driver. Honestly, I didn't
> > give this one too much thought. It seemed a viable solution to the
> > mmap problem. For a bit more history on that, see my comment below.
> > 
> > > 
> > > That should be turned into an allocator so that drivers all get
> > > this right.
> > > 
> > > > Also deffered io seems buggy in combination with kmalloc'ed
> > > > memory (crash on unloading the module).
> > > 
> > > And maybe that's the real issue to fix.
> > 
> > The problem is solved by using vmalloc ;)
> 
> Yep, but why do you need to mark the reserved pages?
> 
> ...

As far as I understood mmaped memory is marked as userspace memory in
the page table and is therefore subject to swapping. The pages are
marked reserved to make clear that this memory can not be swapped and
thus lock the pages in memory. See discussions [0,1,2]. 

[0]http://stackoverflow.com/questions/10760479/mmap-kernel-buffer-to-user-space
[1]http://www.linuxquestions.org/questions/linux-kernel-70/why-why-setpagereserved-is-needed-when-map-a-kernel-space-to-user-space-885176/
[2]https://sites.google.com/site/skartikeyan/mmap.html

> 
> > > > @@ -570,7 +608,7 @@ static int ssd1307fb_probe(struct i2c_client
> > > > *client, info->var.blue.offset = 0;
> > > >  
> > > >  	info->screen_base = (u8 __force __iomem *)vmem;
> > > > -	info->fix.smem_start = (unsigned long)vmem;
> > > > +	info->fix.smem_start = __pa(vmem);
> > > 
> > > Why are you changing from virtual to physical address here?
> > 
> > Oh, good catch! This is still a residual of my attempts to get this
> > working with kmalloc'ed memory. In the current state the driver is
> > presenting a completely wrong memory address upon mmap. As reported
> > in [0] info->fix.smem_start has to hold the physical address of the
> > video memory if it was allocated using kmalloc. Correcting this let
> > me run into the problem that the kmalloc'ed memory was not page
> > aligned but, the memory address handed to userspace mmap was
> > aligned to the next full page, resulting in an inaccessable display
> > region. At that point I just copied the vmalloc approach from the
> > vfb driver.
> > 
> > [0]
> > http://stackoverflow.com/questions/22285151/kernel-panic-using-deferred-io-on-kmalloced-buffer
> 
> And the documentation of fb_fix_screeninfo indeed state that this
> should be the physical address:
> http://lxr.free-electrons.com/source/include/uapi/linux/fb.h#L158
> 
> Could you make that a separate patch?

Will be a separate patch in v2.

> 
> Thanks,
> Maxime
> 

Thomas


^ permalink raw reply

* Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Maxime Ripard @ 2015-02-14 15:36 UTC (permalink / raw)
  To: Thomas Niederprüm
  Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150214152212.1643da7d@maestro.intranet>

[-- Attachment #1: Type: text/plain, Size: 2579 bytes --]

On Sat, Feb 14, 2015 at 03:22:12PM +0100, Thomas Niederprüm wrote:
> Am Thu, 12 Feb 2015 16:11:21 +0100
> schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> 
> > On Sat, Feb 07, 2015 at 04:35:41PM +0100, Thomas Niederprüm wrote:
> > > Am Sat, 7 Feb 2015 12:18:21 +0100
> > > schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> > > 
> > > > Hi,
> > > > 
> > > > On Fri, Feb 06, 2015 at 11:28:10PM +0100, niederp@physik.uni-kl.de
> > > > wrote:
> > > > > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > > > > 
> > > > > It makes sense to use vmalloc to allocate the video buffer
> > > > > since it has to be page aligned memory for using it with mmap.
> > > > 
> > > > Please wrap your commit log at 80 chars.
> > > 
> > > I'll try to do so in future, sorry for that.
> > > 
> > > > 
> > > > It looks like there's numerous fbdev drivers using this
> > > > (especially since you copy pasted that code, without mentionning
> > > > it).
> > > 
> > > Yes, I should have mentioned that in the commit message. As
> > > implicitly indicated in the cover letter the rvmalloc() and
> > > rvfree() are copy pasted from the vfb driver. Honestly, I didn't
> > > give this one too much thought. It seemed a viable solution to the
> > > mmap problem. For a bit more history on that, see my comment below.
> > > 
> > > > 
> > > > That should be turned into an allocator so that drivers all get
> > > > this right.
> > > > 
> > > > > Also deffered io seems buggy in combination with kmalloc'ed
> > > > > memory (crash on unloading the module).
> > > > 
> > > > And maybe that's the real issue to fix.
> > > 
> > > The problem is solved by using vmalloc ;)
> > 
> > Yep, but why do you need to mark the reserved pages?
> > 
> > ...
> 
> As far as I understood mmaped memory is marked as userspace memory in
> the page table and is therefore subject to swapping. The pages are
> marked reserved to make clear that this memory can not be swapped and
> thus lock the pages in memory. See discussions [0,1,2]. 
> 
> [0]http://stackoverflow.com/questions/10760479/mmap-kernel-buffer-to-user-space
> [1]http://www.linuxquestions.org/questions/linux-kernel-70/why-why-setpagereserved-is-needed-when-map-a-kernel-space-to-user-space-885176/
> [2]https://sites.google.com/site/skartikeyan/mmap.html

Hmmm, both https://lwn.net/Articles/28746/ and
http://linux-mm.org/DeviceDriverMmap tell otherwise :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 5/8] fbdev: ssd1307fb: Add module parameter bitsperpixel.
From: Maxime Ripard @ 2015-02-14 15:54 UTC (permalink / raw)
  To: Thomas Niederprüm
  Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150207170503.045490aa@maestro.intranet>

[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]

On Sat, Feb 07, 2015 at 05:05:03PM +0100, Thomas Niederprüm wrote:
> Am Sat, 7 Feb 2015 12:20:43 +0100
> schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> 
> > On Fri, Feb 06, 2015 at 11:28:11PM +0100, niederp@physik.uni-kl.de
> > wrote:
> > > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > > 
> > > This patch adds a module parameter 'bitsperpixel' to adjust the
> > > colordepth of the framebuffer. All values >1 will result in memory
> > > map of the requested color depth. However only the MSB of each
> > > pixel will be sent to the device. The framebuffer identifies itself
> > > as a grayscale display with the specified depth.
> > 
> > I'm not sure this is the right thing to do.
> > 
> > The bits per pixel for this display is rightfully defined, used and
> > reported to the userspace, why would you want to change that?
> 
> You are right of course. The display is 1bpp and it reports to be 1
> bpp. The problem is that there is almost no userspace library that can
> handle 1 bit framebuffers correctly. So it is nice if the framebuffer
> (optionally) can expose itself as 8 bits per pixel grayscale to the
> userspace program. As an example this allows to run DirectFB on the
> framebuffer, which is not possible out of the box for 1bpp.
> 
> Also note that if do not set the module parameter at load time
> the framebuffer will be 1bpp. So you have to actively set that module
> parameter to make the framebuffer pretend to be more than 1bpp.
> 
> In any case I don't cling to that patch, I just thought it was a nice
> feature.

I'd say that the right fix would be to patch DirectFB, instead of
faking that in the kernel.

But again, that's probably Tomi's call, not mine.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 2/8] fbdev: ssd1307fb: Unify init code and make controller configurable from device tree
From: Thomas Niederprüm @ 2015-02-14 16:12 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150212164147.GK2079@lukather>

Am Thu, 12 Feb 2015 17:41:47 +0100
schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:

> On Sat, Feb 07, 2015 at 03:59:33PM +0100, Thomas Niederprüm wrote:
> > Am Sat, 7 Feb 2015 11:42:25 +0100
> > schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> > 
> > > Hi,
> > > 
> > > On Fri, Feb 06, 2015 at 11:28:08PM +0100, niederp@physik.uni-kl.de
> > > wrote:
> > > > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > > > 
> > > > This patches unifies the init code for the ssd130X chips and
> > > > adds device tree bindings to describe the hardware configuration
> > > > of the used controller. This gets rid of the magic bit values
> > > > used in the init code so far.
> > > > 
> > > > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > > > ---
> > > >  .../devicetree/bindings/video/ssd1307fb.txt        |  11 +
> > > >  drivers/video/fbdev/ssd1307fb.c                    | 243
> > > > ++++++++++++++------- 2 files changed, 174 insertions(+), 80
> > > > deletions(-)
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/video/ssd1307fb.txt
> > > > b/Documentation/devicetree/bindings/video/ssd1307fb.txt index
> > > > 7a12542..1230f68 100644 ---
> > > > a/Documentation/devicetree/bindings/video/ssd1307fb.txt +++
> > > > b/Documentation/devicetree/bindings/video/ssd1307fb.txt @@
> > > > -15,6 +15,17 @@ Required properties: Optional properties:
> > > >    - reset-active-low: Is the reset gpio is active on physical
> > > > low?
> > > > +  - solomon,segment-remap: Invert the order of data column to
> > > > segment mapping
> > > > +  - solomon,offset: Map the display start line to one of COM0 -
> > > > COM63
> > > > +  - solomon,contrast: Set initial contrast of the display
> > > > +  - solomon,prechargep1: Set the duration of the precharge
> > > > period phase1
> > > > +  - solomon,prechargep2: Set the duration of the precharge
> > > > period phase2
> > > > +  - solomon,com-alt: Enable/disable alternate COM pin
> > > > configuration
> > > > +  - solomon,com-lrremap: Enable/disable left-right remap of COM
> > > > pins
> > > > +  - solomon,com-invdir: Invert COM scan direction
> > > > +  - solomon,vcomh: Set VCOMH regulator voltage
> > > > +  - solomon,dclk-div: Set display clock divider
> > > > +  - solomon,dclk-frq: Set display clock frequency
> > > 
> > > I'm sorry, but this is the wrong approach, for at least two
> > > reasons: you broke all existing users of that driver, which is a
> > > clear no-go,
> > 
> > Unfortunately this is true. The problem is that the SSD130X
> > controllers allow for a very versatile wiring of the display to the
> > controller.  It's over to the manufacturer of the OLED module
> > (disp+controller) to decide how it's actually wired and during
> > device initialization the driver has to take care to configure the
> > SSD130X controller according to that wiring. If the driver fails to
> > do so you will end up having your display showing
> > garbage.
> 
> How so?

One good example is the segment remap. It basically allows to invert the
order of the output pins connecting to the oled panel. This gives the
manufacturer of the module the freedom wire it the one way or the
other, depending on the PCB restrictions/panel layout (Section 10.1.12
of [0], 10.1.8 in [1], 9.1.8 in [2]). However, once the panel is
connected to the controller it's determined whether the segment remap
is needed or not. Setting the segment remap as done in the current
initialization code for ssd1306 and ssd1307 makes my display module
show it's contents mirrored left to right, probably since the
manufacturer decided not to connect the panel in an inverted order.

The same applies to the com-alt, com-lrremap and com-invdir values,
which define different possibilities for the COM signals pin
configuration (Section 10.1.26 of [0], 10.1.18 in [1], 9.1.18 in [2])
and readout direction of the video memory (Section 10.1.21 of [0],
10.1.14 in [1], 9.1.14 in [2]). Setting com-alt incorrectly leaves
every other line of the display blank. Setting com-lrremap incorrectly
produces a very distorted image. Setting com-invdir incorrectly flips
the image upside down.

IMHO at least these four hardware-specific properties need to be known
to the driver in order to initialize the hardware correctly.

> 
> Does it depend on the X, or can it change from one same controller to
> another? to what extent?

Unfortunately I do not posses any hardware utilizing a ssd1306 or
ssd1307 controller. My primary and only target device is a Newhaven
NHD-3.12-25664UCB2 OLED display module using an SSD1305 controller. I
just inferred from the datasheets of ssd1306/7 [1,2] that they should
behave the same since the registers are bit to bit identical (except
for the VHCOM register). Maybe that was a bit too naive :/

> 
> The 1306 for example seems to not be using these values at all, while
> the 1307 does.

That is surprising. In that case I would like to ask the guys from
Solomon why they describe all these options in the SSD1306 datasheet
[1]. But in any case, isn't that good news for the problem of setting
the default values. When the 1306 isn't using these values anyway we can
not break the initialization by setting different default values. In
this case the problem of the default values boils down to the segment
remap only since this is set in the init code of the 1307, while the
default would be to leave it off.

> 
> > Unfortunately the current sate of the initialization code of the
> > ssd1307fb driver is not very flexible in that respect. Taking a look
> > at the initialization code for the ssd1306 shows that it was written
> > with one very special display module in mind. Most of the magic bit
> > values set there are non-default values according to the
> > datasheet. The result is that the driver works with that one
> > particular display module but many other (differently wired) display
> > modules using a ssd1306 controller won't work without changing the
> > hardcoded magic bit values.
> > 
> > My idea here was to set all configuration to the default values (as
> > given in the datasheet) unless it is overwritten by DT. Of course,
> > without a change in DT, this breaks the driver for all existing
> > users. The only alternative would be to set the current values as
> > default. Somehow this feels wrong to me as these values look
> > arbitrary when you don't know what exact display module they were
> > set for. But if you insist, I will change the default values.
> 
> Unfortunately, the DT bindings are to be considered an ABI, and we
> should support booting with older DTs (not that I personally care
> about it, but that's another issue). So we really don't have much
> choice here.
> 
> Moreover, that issue left aside, modifying bindings like this without
> fixing up the in-tree users is considered quite rude :)

I didn't intend to be rude, sorry. A quick search revealed that there
is luckily only one in-tree user, which is imx28-cfa10036.dts. In case
it will be necessary I will include a patch to fix this.

> > > and the DT itself should not contain any direct mapping of the
> > > registers.
> > > 
> > 
> > I think I don't get what you mean here. Is it because I do no sanity
> > checks of the numbers set in DT? I was just looking for a way to
> > hand over the information about the wiring of display to the
> > driver. How would you propose to solve this?
> 
> What I meant was that replicating direct registers value is usually a
> recipe for a later failure, especially if we can have the information
> under a generic and easy to understand manner.
> 
> For example, replacing the solomon,dclk-div and solomon,dclk-frq
> properties by a clock-frequency property in Hz, and computing the
> divider and that register in your driver is usually better, also
> because it allows to have different requirements / algorithms to
> compute that if some other chip needs it.

I'll give that a try, even though that particular one is not trivial
since the documentation on the actual frequency that is set by the
dclk-freq is very poor (not present for 1306/1307 [1,2], just a graph
for 1305 [0]).

For the properties describing the hardware pin configuration (see above)
I see no real alternative. Maybe they can all be covered by one DT
property like:
solomon,com-cfg = PINCFG_SEGREMAP | PINCFG_COMALT | PINCFG_COMINV |
PINCFG_COMLRRM
each PINCFG_* setting one bit. The driver will then translate this into
the correct settings for the 130X registers. The only problem here is
that this implicitly assumes the default values of each bit to be 0.

> 
> Maxime
> 

[0]http://www.newhavendisplay.com/app_notes/SSD1305.pdf
[1]http://www.adafruit.com/datasheets/SSD1306.pdf
[2]http://www.displayfuture.com/Display/datasheet/controller/SSD1307.pdf

Thomas

^ permalink raw reply

* [PATCH] gxt4500: enable on non-PPC architectures
From: Ondrej Zary @ 2015-02-14 22:30 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list

These chips can be present at least on x86 too - Fire GL2 AGP has GXT6000P but
this driver is currently limited to PPC.
Enable it for all architectures and add chip configuration for little-endian.

Tested on x86 with Fire GL2 AGP.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
 drivers/video/fbdev/Kconfig   |    5 +++--
 drivers/video/fbdev/gxt4500.c |    7 +++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 08a7a04..cb15734 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2143,7 +2143,7 @@ config FB_UDL
 
 config FB_IBM_GXT4500
 	tristate "Framebuffer support for IBM GXT4000P/4500P/6000P/6500P adaptors"
-	depends on FB && PPC
+	depends on FB
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -2151,7 +2151,8 @@ config FB_IBM_GXT4500
 	  Say Y here to enable support for the IBM GXT4000P/6000P and
 	  GXT4500P/6500P display adaptor based on Raster Engine RC1000,
 	  found on some IBM System P (pSeries) machines. This driver
-	  doesn't use Geometry Engine GT1000.
+	  doesn't use Geometry Engine GT1000. This driver also supports
+	  AGP Fire GL2/3/4 cards on x86.
 
 config FB_PS3
 	tristate "PS3 GPU framebuffer driver"
diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 135d78a..1bf9894 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -670,8 +670,15 @@ static int gxt4500_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	pci_set_drvdata(pdev, info);
 
+#ifdef __BIG_ENDIAN
 	/* Set byte-swapping for DFA aperture for all pixel sizes */
 	pci_write_config_dword(pdev, CFG_ENDIAN0, 0x333300);
+#else /* __LITTLE_ENDIAN */
+	/* not sure what this means but fgl23 driver does that */
+	pci_write_config_dword(pdev, CFG_ENDIAN0, 0x2300);
+/*	pci_write_config_dword(pdev, CFG_ENDIAN0 + 4, 0x400000);*/
+	pci_write_config_dword(pdev, CFG_ENDIAN0 + 8, 0x98530000);
+#endif
 
 	info->fbops = &gxt4500_ops;
 	info->flags = FBINFO_FLAG_DEFAULT;
-- 
Ondrej Zary


^ permalink raw reply related

* [PATCH] gxt4500: fix 16bpp 565 mode
From: Ondrej Zary @ 2015-02-14 23:09 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list

Fix wrong colors in 16bpp 565 mode.
Not sure if this is OK for big-endian systems or it breaks them.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
 drivers/video/fbdev/gxt4500.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 1bf9894..1f2fd5b 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -525,7 +525,7 @@ static int gxt4500_setcolreg(unsigned int reg, unsigned int red,
 		u32 val = reg;
 		switch (par->pixfmt) {
 		case DFA_PIX_16BIT_565:
-			val |= (reg << 11) | (reg << 6);
+			val |= (reg << 11) | (reg << 5);
 			break;
 		case DFA_PIX_16BIT_1555:
 			val |= (reg << 10) | (reg << 5);
-- 
Ondrej Zary


^ permalink raw reply related

* [PATCH] framebuffer: don't link fb_devio into kernel image unconditionally
From: Harald Geyer @ 2015-02-16 18:34 UTC (permalink / raw)
  To: linux-fbdev

CONFIG_FB_DEFERRED_IO is defined as bool while CONFIG_FB is defined as
tristate. Currently fb_defio.o is linked into the kernel image even if
CONFIG_FB=m. 

I fix this by updating the Makefile to link fb_defio.o into fb.o and thus
go into one place with the other core framebuffer code.

This has been tested on arm/sunxi.

Signed-off-by: Harald Geyer <harald@ccbib.org>
---
 drivers/video/fbdev/core/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index 67f28e2..23d86a8 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_FB_CMDLINE)          += fb_cmdline.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
                                      modedb.o fbcvt.o
+fb-$(CONFIG_FB_DEFERRED_IO)       += fb_defio.o
 fb-objs                           := $(fb-y)
 
 obj-$(CONFIG_FB_CFB_FILLRECT)  += cfbfillrect.o
@@ -14,4 +15,3 @@ obj-$(CONFIG_FB_SYS_IMAGEBLIT) += sysimgblt.o
 obj-$(CONFIG_FB_SYS_FOPS)      += fb_sys_fops.o
 obj-$(CONFIG_FB_SVGALIB)       += svgalib.o
 obj-$(CONFIG_FB_DDC)           += fb_ddc.o
-obj-$(CONFIG_FB_DEFERRED_IO)   += fb_defio.o
-- 
2.1.4


^ permalink raw reply related

* [PATCH 1/5] gxt4500: enable on non-PPC architectures
From: Ondrej Zary @ 2015-02-17 22:33 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list

These chips can be present at least on x86 too - Fire GL2 AGP has GXT6000P but
this driver is currently limited to PPC.
Enable it for all architectures and add chip configuration for little-endian.

Tested on x86 with Fire GL2 AGP.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
 drivers/video/fbdev/Kconfig   |    5 +++--
 drivers/video/fbdev/gxt4500.c |    7 +++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 08a7a04..cb15734 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2143,7 +2143,7 @@ config FB_UDL
 
 config FB_IBM_GXT4500
 	tristate "Framebuffer support for IBM GXT4000P/4500P/6000P/6500P adaptors"
-	depends on FB && PPC
+	depends on FB
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -2151,7 +2151,8 @@ config FB_IBM_GXT4500
 	  Say Y here to enable support for the IBM GXT4000P/6000P and
 	  GXT4500P/6500P display adaptor based on Raster Engine RC1000,
 	  found on some IBM System P (pSeries) machines. This driver
-	  doesn't use Geometry Engine GT1000.
+	  doesn't use Geometry Engine GT1000. This driver also supports
+	  AGP Fire GL2/3/4 cards on x86.
 
 config FB_PS3
 	tristate "PS3 GPU framebuffer driver"
diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 135d78a..1bf9894 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -670,8 +670,15 @@ static int gxt4500_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	pci_set_drvdata(pdev, info);
 
+#ifdef __BIG_ENDIAN
 	/* Set byte-swapping for DFA aperture for all pixel sizes */
 	pci_write_config_dword(pdev, CFG_ENDIAN0, 0x333300);
+#else /* __LITTLE_ENDIAN */
+	/* not sure what this means but fgl23 driver does that */
+	pci_write_config_dword(pdev, CFG_ENDIAN0, 0x2300);
+/*	pci_write_config_dword(pdev, CFG_ENDIAN0 + 4, 0x400000);*/
+	pci_write_config_dword(pdev, CFG_ENDIAN0 + 8, 0x98530000);
+#endif
 
 	info->fbops = &gxt4500_ops;
 	info->flags = FBINFO_FLAG_DEFAULT;
-- 
Ondrej Zary


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox