Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH v16 RESEND 0/7] of: add display helper
From: Steffen Trumtrar @ 2013-01-24  8:19 UTC (permalink / raw)
  To: Leela Krishna Amudala
  Cc: linux-fbdev, Mohammed, Afzal, David Airlie, devicetree-discuss,
	Florian Tobias Schandinat, dri-devel, Rob Clark, Tomi Valkeinen,
	Laurent Pinchart, kernel, Guennady Liakhovetski, linux-media
In-Reply-To: <CAL1wa8d6wXdxgVJ=c2ma2Adm-RkF3S5ga219dSrh1fFo0L9X8w@mail.gmail.com>

On Thu, Jan 24, 2013 at 10:44:50AM +0530, Leela Krishna Amudala wrote:
> Steffen,
> 
> You can add my tested-by for this series.. :)
> I have been using them for Exynos: smdk5250 board.
> 

Thanks. I'll use that opportunity for a v17 that is rebased onto 3.8-rc4.

Regards,
Steffen

> On Wed, Jan 23, 2013 at 2:42 PM, Steffen Trumtrar
> <s.trumtrar@pengutronix.de> wrote:
> > On Tue, Jan 22, 2013 at 03:50:48PM -0600, Rob Clark wrote:
> >> On Mon, Jan 21, 2013 at 5:07 AM, Steffen Trumtrar
> >> <s.trumtrar@pengutronix.de> wrote:
> >> > Hi!
> >> >
> >> > There was still no maintainer, that commented, ack'd, nack'd, apply'd the
> >> > series. So, this is just a resend.
> >> > The patches were tested with:
> >> >
> >> >         - v15 on Tegra by Thierry
> >> >         - sh-mobile-lcdcfb by Laurent
> >> >         - MX53QSB by Marek
> >> >         - Exynos: smdk5250 by Leela
> >> >         - AM335X EVM & AM335X EVM-SK by Afzal
> >> >         - imx6q: sabrelite, sabresd by Philipp and me
> >> >         - imx53: tqma53/mba53 by me
> >>
> >>
> >> btw, you can add my tested-by for this series..  I've been using them
> >> for the tilcdc lcd-panel output driver support.
> >>
> >
> > Thanks. The more drivers the merrier ;-)
> >
> > Steffen
> >
> >> >
> >> >
> >> > Changes since v15:
> >> >         - move include/linux/{videomode,display_timing}.h to include/video
> >> >         - move include/linux/of_{videomode,display_timing}.h to include/video
> >> >         - reimplement flags: add VESA flags and data flags
> >> >         - let pixelclock in struct videomode be unsigned long
> >> >         - rename of_display_timings_exists to of_display_timings_exist
> >> >         - revise logging/error messages: replace __func__ with np->full_name
> >> >         - rename pixelclk-inverted to pixelclk-active
> >> >         - revise comments in code
> >> >
> >> > Changes since v14:
> >> >         - fix "const struct *" warning
> >> >                 (reported by: Leela Krishna Amudala <l.krishna@samsung.com>)
> >> >         - return -EINVAL when htotal or vtotal are zero
> >> >         - remove unreachable code in of_get_display_timings
> >> >         - include headers in .c files and not implicit in .h
> >> >         - sort includes alphabetically
> >> >         - fix lower/uppercase in binding documentation
> >> >         - rebase onto v3.7-rc7
> >> >
> >> > Changes since v13:
> >> >         - fix "const struct *" warning
> >> >                 (reported by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>)
> >> >         - prevent division by zero in fb_videomode_from_videomode
> >> >
> >> > Changes since v12:
> >> >         - rename struct display_timing to via_display_timing in via subsystem
> >> >         - fix refreshrate calculation
> >> >         - fix "const struct *" warnings
> >> >                 (reported by: Manjunathappa, Prakash <prakash.pm@ti.com>)
> >> >         - some CodingStyle fixes
> >> >         - rewrite parts of commit messages and display-timings.txt
> >> >         - let display_timing_get_value get all values instead of just typical
> >> >
> >> > Changes since v11:
> >> >         - make pointers const where applicable
> >> >         - add reviewed-by Laurent Pinchart
> >> >
> >> > Changes since v10:
> >> >         - fix function name (drm_)display_mode_from_videomode
> >> >         - add acked-by, reviewed-by, tested-by
> >> >
> >> > Changes since v9:
> >> >         - don't leak memory when previous timings were correct
> >> >         - CodingStyle fixes
> >> >         - move blank lines around
> >> >
> >> > Changes since v8:
> >> >         - fix memory leaks
> >> >         - change API to be more consistent (foo_from_bar(struct bar, struct foo))
> >> >         - include headers were necessary
> >> >         - misc minor bugfixes
> >> >
> >> > Changes since v7:
> >> >         - move of_xxx to drivers/video
> >> >         - remove non-binding documentation from display-timings.txt
> >> >         - squash display_timings and videomode in one patch
> >> >         - misc minor fixes
> >> >
> >> > Changes since v6:
> >> >         - get rid of some empty lines etc.
> >> >         - move functions to their subsystems
> >> >         - split of_ from non-of_ functions
> >> >         - add at least some kerneldoc to some functions
> >> >
> >> > Changes since v5:
> >> >         - removed all display stuff and just describe timings
> >> >
> >> > Changes since v4:
> >> >         - refactored functions
> >> >
> >> > Changes since v3:
> >> >         - print error messages
> >> >         - free alloced memory
> >> >         - general cleanup
> >> >
> >> > Changes since v2:
> >> >         - use hardware-near property-names
> >> >         - provide a videomode structure
> >> >         - allow ranges for all properties (<min,typ,max>)
> >> >         - functions to get display_mode or fb_videomode
> >> >
> >> >
> >> > Regards,
> >> > Steffen
> >> >
> >> >
> >> > Steffen Trumtrar (7):
> >> >   viafb: rename display_timing to via_display_timing
> >> >   video: add display_timing and videomode
> >> >   video: add of helper for display timings/videomode
> >> >   fbmon: add videomode helpers
> >> >   fbmon: add of_videomode helpers
> >> >   drm_modes: add videomode helpers
> >> >   drm_modes: add of_videomode helpers
> >> >
> >> >  .../devicetree/bindings/video/display-timing.txt   |  109 +++++++++
> >> >  drivers/gpu/drm/drm_modes.c                        |   70 ++++++
> >> >  drivers/video/Kconfig                              |   21 ++
> >> >  drivers/video/Makefile                             |    4 +
> >> >  drivers/video/display_timing.c                     |   24 ++
> >> >  drivers/video/fbmon.c                              |   94 ++++++++
> >> >  drivers/video/of_display_timing.c                  |  239 ++++++++++++++++++++
> >> >  drivers/video/of_videomode.c                       |   54 +++++
> >> >  drivers/video/via/hw.c                             |    6 +-
> >> >  drivers/video/via/hw.h                             |    2 +-
> >> >  drivers/video/via/lcd.c                            |    2 +-
> >> >  drivers/video/via/share.h                          |    2 +-
> >> >  drivers/video/via/via_modesetting.c                |    8 +-
> >> >  drivers/video/via/via_modesetting.h                |    6 +-
> >> >  drivers/video/videomode.c                          |   39 ++++
> >> >  include/drm/drmP.h                                 |    9 +
> >> >  include/linux/fb.h                                 |    8 +
> >> >  include/video/display_timing.h                     |  124 ++++++++++
> >> >  include/video/of_display_timing.h                  |   20 ++
> >> >  include/video/of_videomode.h                       |   18 ++
> >> >  include/video/videomode.h                          |   48 ++++
> >> >  21 files changed, 894 insertions(+), 13 deletions(-)
> >> >  create mode 100644 Documentation/devicetree/bindings/video/display-timing.txt
> >> >  create mode 100644 drivers/video/display_timing.c
> >> >  create mode 100644 drivers/video/of_display_timing.c
> >> >  create mode 100644 drivers/video/of_videomode.c
> >> >  create mode 100644 drivers/video/videomode.c
> >> >  create mode 100644 include/video/display_timing.h
> >> >  create mode 100644 include/video/of_display_timing.h
> >> >  create mode 100644 include/video/of_videomode.h
> >> >  create mode 100644 include/video/videomode.h
> >> >
> >> > --
> >> > 1.7.10.4
> >> >
> >>
> >
> > --
> > Pengutronix e.K.                           |                             |
> > Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> > Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* RE: [PATCH v16 RESEND 0/7] of: add display helper
From: Mohammed, Afzal @ 2013-01-24  8:47 UTC (permalink / raw)
  To: Steffen Trumtrar, Leela Krishna Amudala
  Cc: linux-fbdev@vger.kernel.org, David Airlie,
	devicetree-discuss@lists.ozlabs.org, Florian Tobias Schandinat,
	dri-devel@lists.freedesktop.org, Rob Clark, Valkeinen, Tomi,
	Laurent Pinchart, kernel@pengutronix.de, Guennady Liakhovetski,
	linux-media@vger.kernel.org
In-Reply-To: <20130124081958.GA28406@pengutronix.de>

SGkgU3RlZmZlbiwNCg0KT24gVGh1LCBKYW4gMjQsIDIwMTMgYXQgMTM6NDk6NTgsIFN0ZWZmZW4g
VHJ1bXRyYXIgd3JvdGU6DQoNCj4gVGhhbmtzLiBJJ2xsIHVzZSB0aGF0IG9wcG9ydHVuaXR5IGZv
ciBhIHYxNyB0aGF0IGlzIHJlYmFzZWQgb250byAzLjgtcmM0Lg0KDQpBcyB5b3UgYXJlIGdvaW5n
IHRvIGhhdmUgYSB2MTcsIGlmIHlvdSBjYW4gZm9sZCB0aGUgZGlmZlsxXQ0KKHRoYXQgSSBtZW50
aW9uZWQgZWFybGllcikgaW50byB0aGUgcGF0Y2gsDQoiZmJtb246IGFkZCBvZl92aWRlb21vZGUg
aGVscGVycyIsIGl0IHdvdWxkIGJlIGhlbHBmdWwuIA0KDQpSZWdhcmRzDQpBZnphbA0KDQpbMV0N
Cg0KZGlmZiAtLWdpdCBhL2luY2x1ZGUvbGludXgvZmIuaCBiL2luY2x1ZGUvbGludXgvZmIuaA0K
aW5kZXggNThiOTg2MC4uMGNlMzBkMSAxMDA2NDQNCi0tLSBhL2luY2x1ZGUvbGludXgvZmIuaA0K
KysrIGIvaW5jbHVkZS9saW51eC9mYi5oDQpAQCAtNzE2LDkgKzcxNiwxOSBAQCBleHRlcm4gdm9p
ZCBmYl9kZXN0cm95X21vZGVkYihzdHJ1Y3QgZmJfdmlkZW9tb2RlICptb2RlZGIpOw0KIGV4dGVy
biBpbnQgZmJfZmluZF9tb2RlX2N2dChzdHJ1Y3QgZmJfdmlkZW9tb2RlICptb2RlLCBpbnQgbWFy
Z2lucywgaW50IHJiKTsNCiBleHRlcm4gdW5zaWduZWQgY2hhciAqZmJfZGRjX3JlYWQoc3RydWN0
IGkyY19hZGFwdGVyICphZGFwdGVyKTsNCiANCisjaWYgZGVmaW5lZChDT05GSUdfT0ZfVklERU9N
T0RFKSAmJiBkZWZpbmVkKENPTkZJR19GQl9NT0RFX0hFTFBFUlMpDQogZXh0ZXJuIGludCBvZl9n
ZXRfZmJfdmlkZW9tb2RlKHN0cnVjdCBkZXZpY2Vfbm9kZSAqbnAsDQogICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgc3RydWN0IGZiX3ZpZGVvbW9kZSAqZmIsDQogICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgaW50IGluZGV4KTsNCisjZWxzZQ0KK3N0YXRpYyBpbmxpbmUgaW50IG9m
X2dldF9mYl92aWRlb21vZGUoc3RydWN0IGRldmljZV9ub2RlICpucCwNCisgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgc3RydWN0IGZiX3ZpZGVvbW9kZSAqZmIsDQorICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGludCBpbmRleCkNCit7DQorICAgICAgIHJl
dHVybiAtRUlOVkFMOw0KK30NCisjZW5kaWYNCisNCiBleHRlcm4gaW50IGZiX3ZpZGVvbW9kZV9m
cm9tX3ZpZGVvbW9kZShjb25zdCBzdHJ1Y3QgdmlkZW9tb2RlICp2bSwNCiAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgIHN0cnVjdCBmYl92aWRlb21vZGUgKmZibW9kZSk7DQoN
Cg=

^ permalink raw reply

* Re: [PATCH 2/2] drm/i915: fixup per-crtc locking in intel_release_load_detect_pipe
From: Daniel Vetter @ 2013-01-24  9:55 UTC (permalink / raw)
  To: Dave Airlie
  Cc: Daniel Vetter, Intel Graphics Development, linux-fbdev-devel,
	LKML, DRI Development
In-Reply-To: <1358958309-5342-3-git-send-email-daniel.vetter@ffwll.ch>

On Wed, Jan 23, 2013 at 05:25:09PM +0100, Daniel Vetter wrote:
> One of the early return cases missed the mutex unlocking. Hilarity
> ensued.
> 
> This regression has been introduced in
> 
> commit 7b24056be6db7ce907baffdd4cf142ab774ea60c
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Wed Dec 12 00:35:33 2012 +0100
> 
>     drm: don't hold crtc mutexes for connector ->detect callbacks
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59750
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Tested-by: Cancan Feng <cancan.feng@intel.com>

Dave, please pick this one up for your drm-next tree since the issue only
happens there due to the modeset locking rework.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply

* RE: [PATCH v4 12/12] video: da8xx-fb: CCF clock divider handling
From: Mohammed, Afzal @ 2013-01-24 11:36 UTC (permalink / raw)
  To: Mike Turquette, linux-fbdev@vger.kernel.org,
	linux-omap@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: Florian Tobias Schandinat, Valkeinen, Tomi, Grant Likely,
	Rob Herring, Rob Landley
In-Reply-To: <20130123202204.9205.66575@quantum>

SGkgTWlrZSwNCg0KT24gVGh1LCBKYW4gMjQsIDIwMTMgYXQgMDE6NTI6MDQsIE1pa2UgVHVycXVl
dHRlIHdyb3RlOg0KPiBRdW90aW5nIEFmemFsIE1vaGFtbWVkICgyMDEzLTAxLTIzIDAzOjQ4OjU2
KQ0KDQo+ID4gK3N0YXRpYyBpbmxpbmUgdm9pZCBkYTh4eF9mYl9jbGtjX2VuYWJsZSh2b2lkKQ0K
PiA+ICt7DQo+ID4gICAgICAgICBpZiAobGNkX3JldmlzaW9uID09IExDRF9WRVJTSU9OXzIpDQo+
ID4gICAgICAgICAgICAgICAgIGxjZGNfd3JpdGUoTENEX1YyX0RNQV9DTEtfRU4gfCBMQ0RfVjJf
TElERF9DTEtfRU4gfA0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgTENEX1Yy
X0NPUkVfQ0xLX0VOLCBMQ0RfQ0xLX0VOQUJMRV9SRUcpOw0KDQo+ID4gK3N0YXRpYyBpbmxpbmUg
aW50IGRhOHh4X2ZiX2NhbGNfY29uZmlnX2Nsa19kaXZpZGVyKHN0cnVjdCBkYTh4eF9mYl9wYXIg
KnBhciwNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgc3RydWN0IGZiX3ZpZGVvbW9kZSAqbW9kZSkNCj4gPiArew0KDQo+ID4gKyAgICAgICBy
ZXQgPSBjbGtfc2V0X3JhdGUocGFyLT5jaGlsZF9jbGssIFBJQ09TMktIWihtb2RlLT5waXhjbG9j
aykgKiAxMDAwKTsNCj4gPiArICAgICAgIGlmIChJU19FUlJfVkFMVUUocmV0KSkgew0KPiA+ICsg
ICAgICAgICAgICAgICBkZXZfZXJyKHBhci0+ZGV2LCAidW5hYmxlIHRvIHNldHVwIHBpeGVsIGNs
b2NrIG9mICV1IHBzIiwNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICBtb2RlLT5waXhjbG9j
ayk7DQo+ID4gKyAgICAgICAgICAgICAgIHJldHVybiByZXQ7DQo+ID4gKyAgICAgICB9DQo+ID4g
KyAgICAgICBkYTh4eF9mYl9jbGtjX2VuYWJsZSgpOw0KDQo+IEl0IGxvb2tzIGxpa2UgeW91IGFy
ZSB1c2luZyB0aGUgbGVnYWN5IG1ldGhvZCB0byBlbmFibGUvZGlzYWJsZSB0aGUNCj4gY2xvY2sg
YW5kIHVzaW5nIHRoZSBDQ0YgYmFzaWMgZGl2aWRlciB0byBzZXQgdGhlIHJhdGUuICBUaGlzIGZl
ZWxzIGEgYml0DQo+IGhhY2t5IHRvIG1lLiAgSWYgeW91IHdhbnQgdG8gbW9kZWwgeW91ciBjbG9j
ayBpbiBDQ0YgdGhlbiB5b3Ugc2hvdWxkDQo+IHByb2JhYmx5IG1vZGVsIHRoZSB3aG9sZSBjbG9j
aywgbm90IGp1c3QgdGhlIHJhdGUtY2hhbmdlIGFzcGVjdHMgb2YgaXQuDQoNCkluaXRpYWxseSBJ
IHRob3VnaHQgYWJvdXQgaXQsIGJ1dCBzZWVpbmcgcmVxdWlyZW1lbnQgb2YgMyBnYXRlIGNsb2Nr
cw0KKGR1ZSB0byAzIGJpdHMgbWVhbnQgZm9yIGRpZmZlcmVudCBwdXJwb3NlcyAtIERNQSwgTElE
RCBhbmQgQ09SRQ0KZnVuY3Rpb25hbGl0aWVzKSwgZmVsdCB0aGF0IGhhdmluZyA0IGNsb2NrcyAo
MyBnYXRlICsgMSBkaXZpZGVyKSBpbg0KZHJpdmVyIHdvdWxkIGJlIGFuIG92ZXJkZXNpZ24gW2xl
YXZpbmcgYSBicmFuY2ggaW5zdGVhZCBvZiBhIGxlYWYgb2YNCnRoZSB0cmVlIGluIGRyaXZlciA7
KV0uDQoNCj4gSGF2ZSB5b3UgbG9va2VkIGF0IHRoZSBjb21wb3NpdGUgY2xvY2sgcGF0Y2hlcyBm
cm9tIFByYXNoYW50PyAgVGhvc2UNCj4gbWlnaHQgZ2l2ZSB5b3UgdGhlIGRpdmlkZXIrZ2F0ZSBw
cm9wZXJ0aWVzIHRoYXQgeW91IGFyZSBsb29raW5nIGZvcjoNCj4gaHR0cDovL2FydGljbGUuZ21h
bmUub3JnL2dtYW5lLmxpbnV4Lmtlcm5lbC8xNDE2Njk3DQoNClRoYW5rcyBmb3IgdGhlIHBvaW50
ZXIsDQoNCk5vdyB3aXRoIHRoZSBjb21wb3NpdGUgY2xvY2sgaW4gbWluZCwgaXQgd2FzIHRyaWVk
IHRvIHJlbGF0ZSB0byB3aGF0DQp3YXMgcmVxdWlyZWQgZm9yIHRoZSBwcmVzZW50IHNjZW5hcmlv
Lg0KDQpTbyB0aGVyZSBhcmUgMyAtIExJREQgaXMgYWN0dWFsbHkgbm90IGZvciBwcmVzZW50IHVz
ZSBjYXNlLCBDT1JFIGNvdWxkDQpiZSBjbHViYmVkIHdpdGggdGhlIGRpdmlkZXIgdG8gaGF2ZSBh
IGNvbXBvc2l0ZSBjbG9jay4gQW5kIENPUkUgaXMNCmluIGZ1bmN0aW9uYWwgY2xvY2sgcGF0aCBh
bmQgbG9naWNhbGx5IGl0J3MgcGVyZmVjdGx5IGFscmlnaHQgdG8gaGF2ZQ0KdGhlIGNvbXBvc2l0
ZSBjbG9jay4NCg0KQW5kIG5vdyB3ZSBhcmUgbGVmdCB3aXRoIERNQSwgdGhpcyBpcyBhY3R1YWxs
eSBpbiB0aGUgaW50ZXJmYWNlIGNsb2NrDQpwYXRoIHdoaWNoIGRyaXZlciBpbiB1bmF3YXJlLiBB
biBvcHRpb24gd291bGQgYmUgdG8gaGF2ZSBETUEgY2xvY2sNCmFzIGNoaWxkIG9mIENPUkUgcGx1
cyBkaXZpZGVyIGNvbXBvc2l0ZSBjbG9jaywgZXZlbiB0aG91Z2ggbG9naWNhbGx5DQpETUEgY2Fu
J3QgYmUgY29uc2lkZXJlZCBpbiB0aGUgc2FtZSBwYXRoLg0KDQpBbHNvIHRyaWVkIG5vdCBlbmFi
bGluZyBETUEgY2xvY2ssIGJ1dCBkcml2ZXIgaXMgYWJsZSB0byBwcm92aWRlDQpkaXNwbGF5IHdp
dGhvdXQgYW55IGlzc3Vlcywgc28gd2FzIHRoaW5raW5nIHdoZXRoZXIgdG8gYXZvaWQNCmluc3Rh
bnRpYXRpbmcgRE1BIGNsb2NrIGF0IGFsbCBhbmQgaGVuY2UgdG8gaGF2ZSBhIHNpbXBsZSBzaW5n
bGUNCmNvbXBvc2l0ZSBjbG9jay4gVHJ5aW5nIHRvIGdldCBpbmZvcm1hdGlvbiBpbnRlcm5hbGx5
IG9uIHdoZXRoZXINCm5vdCBzZXR0aW5nIERNQSBjbG9jayBiaXRzIHdvdWxkIGFjdHVhbGx5IG1h
a2UgYSBkaWZmZXJlbmNlLg0KDQpJZiB5b3UgaGF2ZSBhbnkgb3BpbmlvbiBvbiBob3cgdG8gZGVh
bCBoZXJlLCBsZXQgbWUga25vdy4NCg0KUmVnYXJkcw0KQWZ6YWwNCg0KDQo

^ permalink raw reply

* Re: RE: [PATCH v4 12/12] video: da8xx-fb: CCF clock divider handling
From: Mike Turquette @ 2013-01-24 17:00 UTC (permalink / raw)
  To: Mohammed, Afzal, linux-fbdev@vger.kernel.org,
	linux-omap@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: Florian Tobias Schandinat, Valkeinen, Tomi, Grant Likely,
	Rob Herring, Rob Landley
In-Reply-To: <C8443D0743D26F4388EA172BF4E2A7A93EA92ADF@DBDE01.ent.ti.com>

Quoting Mohammed, Afzal (2013-01-24 03:36:02)
> Hi Mike,
> 
> On Thu, Jan 24, 2013 at 01:52:04, Mike Turquette wrote:
> > Quoting Afzal Mohammed (2013-01-23 03:48:56)
> 
> > > +static inline void da8xx_fb_clkc_enable(void)
> > > +{
> > >         if (lcd_revision = LCD_VERSION_2)
> > >                 lcdc_write(LCD_V2_DMA_CLK_EN | LCD_V2_LIDD_CLK_EN |
> > >                                 LCD_V2_CORE_CLK_EN, LCD_CLK_ENABLE_REG);
> 
> > > +static inline int da8xx_fb_calc_config_clk_divider(struct da8xx_fb_par *par,
> > > +                                                   struct fb_videomode *mode)
> > > +{
> 
> > > +       ret = clk_set_rate(par->child_clk, PICOS2KHZ(mode->pixclock) * 1000);
> > > +       if (IS_ERR_VALUE(ret)) {
> > > +               dev_err(par->dev, "unable to setup pixel clock of %u ps",
> > > +                       mode->pixclock);
> > > +               return ret;
> > > +       }
> > > +       da8xx_fb_clkc_enable();
> 
> > It looks like you are using the legacy method to enable/disable the
> > clock and using the CCF basic divider to set the rate.  This feels a bit
> > hacky to me.  If you want to model your clock in CCF then you should
> > probably model the whole clock, not just the rate-change aspects of it.
> 
> Initially I thought about it, but seeing requirement of 3 gate clocks
> (due to 3 bits meant for different purposes - DMA, LIDD and CORE
> functionalities), felt that having 4 clocks (3 gate + 1 divider) in
> driver would be an overdesign [leaving a branch instead of a leaf of
> the tree in driver ;)].
> 
> > Have you looked at the composite clock patches from Prashant?  Those
> > might give you the divider+gate properties that you are looking for:
> > http://article.gmane.org/gmane.linux.kernel/1416697
> 
> Thanks for the pointer,
> 
> Now with the composite clock in mind, it was tried to relate to what
> was required for the present scenario.
> 
> So there are 3 - LIDD is actually not for present use case, CORE could
> be clubbed with the divider to have a composite clock. And CORE is
> in functional clock path and logically it's perfectly alright to have
> the composite clock.
> 

Some of the clock names are a bit generic, so a question that I'm going
to repeat throughout my response: "is this clock only inside of your
video IP ?"

Regarding the CORE clock, is this only inside of your IP or are you
referring to the SoC CORE clock which is driven by a DPLL and clocks
DDR and many other peripherals (often MMC, UART, etc)?

Note that this is from my past experience with OMAP, and I'm making an
assumption that the clock scheme between OMAP and Da Vinci/AM335x parts
isn't very different.

Is there a public TRM I can look at?  It would help me understand this
without having to ask you so many annoying questions ;)

> And now we are left with DMA, this is actually in the interface clock
> path which driver in unaware. An option would be to have DMA clock
> as child of CORE plus divider composite clock, even though logically
> DMA can't be considered in the same path.
> 

Why is the driver unaware of the interface clk?  For instance OMAP3 had
separate fclk and iclk for IPs and drivers would call clk_enable on
both.  Or am I misunderstanding something?

In general I don't think the clock subtree should be modeled in a way
that is convenient for software, but instead model the actual hardware.
Trust me, if you don't model the actual hardware then you will be very
confused when you come back and revisit this code in 6 months and can't
remember why things are so weird looking.

Thanks,
Mike

> Also tried not enabling DMA clock, but driver is able to provide
> display without any issues, so was thinking whether to avoid
> instantiating DMA clock at all and hence to have a simple single
> composite clock. Trying to get information internally on whether
> not setting DMA clock bits would actually make a difference.
> 
> If you have any opinion on how to deal here, let me know.
> 
> Regards
> Afzal

^ permalink raw reply

* [git pull] fbcon locking fixes.
From: Dave Airlie @ 2013-01-25  0:42 UTC (permalink / raw)
  To: torvalds, Andrew Morton; +Cc: DRI mailing list, linux-kernel, linux-fbdev


Hi Linus,

These patches have been sailing around long enough, waiting for a maintainer
to reappear, so I've decided enough is enough, lockdep is kinda useful to have.

Thanks to Daniel for annoying me enough :-)

Dave 'not the fbdev maintainer' Airlie.

The following changes since commit ba2ab41f3d68f277860768b2c5b197768b196332:

  Merge tag 'pm+acpi-for-3.8-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm (2013-01-24 10:19:13 -0800)

are available in the git repository at:


  git://people.freedesktop.org/~airlied/linux fbcon-locking-fixes

for you to fetch changes up to a9d2331a8eccc17408abd5a7cd3e463745254d65:

  fb: Yet another band-aid for fixing lockdep mess (2013-01-25 10:34:49 +1000)

----------------------------------------------------------------
Alan Cox (1):
      fb: rework locking to fix lock ordering on takeover

Takashi Iwai (1):
      fb: Yet another band-aid for fixing lockdep mess

 drivers/tty/vt/vt.c           | 134 ++++++++++++++++++++++++++++++------------
 drivers/video/console/fbcon.c |  33 ++++++++++-
 drivers/video/fbmem.c         |   9 ++-
 drivers/video/fbsysfs.c       |   3 +
 include/linux/console.h       |   2 +
 include/linux/vt_kern.h       |   2 +
 6 files changed, 140 insertions(+), 43 deletions(-)

^ permalink raw reply

* Re: [git pull] fbcon locking fixes.
From: Andrew Morton @ 2013-01-25  0:50 UTC (permalink / raw)
  To: Dave Airlie
  Cc: torvalds, DRI mailing list, linux-kernel, linux-fbdev,
	Maarten Lankhorst
In-Reply-To: <alpine.DEB.2.00.1301250041001.25545@skynet.skynet.ie>

On Fri, 25 Jan 2013 00:42:37 +0000 (GMT)
Dave Airlie <airlied@linux.ie> wrote:

> These patches have been sailing around long enough, waiting for a maintainer
> to reappear, so I've decided enough is enough, lockdep is kinda useful to have.
> 
> Thanks to Daniel for annoying me enough :-)

Me too, but the patches broke Maarten's EFI driver setup:
https://lkml.org/lkml/2013/1/15/203.

^ permalink raw reply

* Re: [git pull] fbcon locking fixes.
From: Linus Torvalds @ 2013-01-25  0:53 UTC (permalink / raw)
  To: Dave Airlie
  Cc: Andrew Morton, DRI mailing list, Linux Kernel Mailing List,
	linux-fbdev@vger.kernel.org
In-Reply-To: <alpine.DEB.2.00.1301250041001.25545@skynet.skynet.ie>

On Thu, Jan 24, 2013 at 4:42 PM, Dave Airlie <airlied@linux.ie> wrote:
>
> These patches have been sailing around long enough, waiting for a maintainer
> to reappear, so I've decided enough is enough, lockdep is kinda useful to have.

Last this was tried, these patches failed miserably.

They caused instant lockdep splat and then a total lockup with efifb.
It may be that Takashi's patch helps fix that problem, but it's in no
way clear that it does, so the patch series isn't at all obviously
stable.

Yes, lockdep is indeed "kinda useful", and there clearly are locking
problems in fbdev. But I'm not seeing myself pulling these for 3.8.
They've been too problematic to pull in at this late stage.

              Linus

^ permalink raw reply

* Re: [git pull] fbcon locking fixes.
From: Dave Airlie @ 2013-01-25  1:06 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Dave Airlie, Andrew Morton, DRI mailing list,
	Linux Kernel Mailing List, linux-fbdev@vger.kernel.org
In-Reply-To: <CA+55aFxzDv0emWgyde7iEWpgZarAGaExrKbEHFr-UcbhoeH_Cw@mail.gmail.com>

On Fri, Jan 25, 2013 at 10:53 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Jan 24, 2013 at 4:42 PM, Dave Airlie <airlied@linux.ie> wrote:
>>
>> These patches have been sailing around long enough, waiting for a maintainer
>> to reappear, so I've decided enough is enough, lockdep is kinda useful to have.
>
> Last this was tried, these patches failed miserably.
>
> They caused instant lockdep splat and then a total lockup with efifb.
> It may be that Takashi's patch helps fix that problem, but it's in no
> way clear that it does, so the patch series isn't at all obviously
> stable.
>
> Yes, lockdep is indeed "kinda useful", and there clearly are locking
> problems in fbdev. But I'm not seeing myself pulling these for 3.8.
> They've been too problematic to pull in at this late stage.
>

Okay I'll fix the efifb problem and then maybe queue them for -next.

Dave.

^ permalink raw reply

* [PATCH] fbcon: fix locking harder
From: Dave Airlie @ 2013-01-25  1:43 UTC (permalink / raw)
  To: linux-fbdev; +Cc: dri-devel, linux-kernel, Dave Airlie

Okay so Alan's patch handled the case where there was no registered fbcon,
however the other path entered in set_con2fb_map pit.

In there we called fbcon_takeover, but we also took the console lock in a couple
of places. So push the console lock out to the callers of set_con2fb_map,

this means fbmem and switcheroo needed to take the lock around the fb notifier
entry points that lead to this.

This should fix the efifb regression seen by Maarten.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/vga/vga_switcheroo.c |  3 +++
 drivers/video/console/fbcon.c    | 11 ++++++++---
 drivers/video/fbmem.c            |  2 ++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index fa60add..cf787e1 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -25,6 +25,7 @@
 #include <linux/fb.h>
 
 #include <linux/pci.h>
+#include <linux/console.h>
 #include <linux/vga_switcheroo.h>
 
 #include <linux/vgaarb.h>
@@ -337,8 +338,10 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client)
 
 	if (new_client->fb_info) {
 		struct fb_event event;
+		console_lock();
 		event.info = new_client->fb_info;
 		fb_notifier_call_chain(FB_EVENT_REMAP_ALL_CONSOLE, &event);
+		console_unlock();
 	}
 
 	ret = vgasr_priv.handler->switchto(new_client->id);
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 2aef9ca..2e2d959 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -842,6 +842,8 @@ static void con2fb_init_display(struct vc_data *vc, struct fb_info *info,
  *
  *	Maps a virtual console @unit to a frame buffer device
  *	@newidx.
+ *
+ *	This should be called with the console lock held.
  */
 static int set_con2fb_map(int unit, int newidx, int user)
 {
@@ -859,7 +861,7 @@ static int set_con2fb_map(int unit, int newidx, int user)
 
 	if (!search_for_mapped_con() || !con_is_bound(&fb_con)) {
 		info_idx = newidx;
-		return fbcon_takeover(0);
+		return do_fbcon_takeover(0);
 	}
 
 	if (oldidx != -1)
@@ -867,7 +869,6 @@ static int set_con2fb_map(int unit, int newidx, int user)
 
 	found = search_fb_in_map(newidx);
 
-	console_lock();
 	con2fb_map[unit] = newidx;
 	if (!err && !found)
  		err = con2fb_acquire_newinfo(vc, info, unit, oldidx);
@@ -894,7 +895,6 @@ static int set_con2fb_map(int unit, int newidx, int user)
 	if (!search_fb_in_map(info_idx))
 		info_idx = newidx;
 
-	console_unlock();
  	return err;
 }
 
@@ -3019,6 +3019,7 @@ static inline int fbcon_unbind(void)
 }
 #endif /* CONFIG_VT_HW_CONSOLE_BINDING */
 
+/* called with console_lock held */
 static int fbcon_fb_unbind(int idx)
 {
 	int i, new_idx = -1, ret = 0;
@@ -3045,6 +3046,7 @@ static int fbcon_fb_unbind(int idx)
 	return ret;
 }
 
+/* called with console_lock held */
 static int fbcon_fb_unregistered(struct fb_info *info)
 {
 	int i, idx;
@@ -3082,6 +3084,7 @@ static int fbcon_fb_unregistered(struct fb_info *info)
 	return 0;
 }
 
+/* called with console_lock held */
 static void fbcon_remap_all(int idx)
 {
 	int i;
@@ -3126,6 +3129,7 @@ static inline void fbcon_select_primary(struct fb_info *info)
 }
 #endif /* CONFIG_FRAMEBUFFER_DETECT_PRIMARY */
 
+/* called with console_lock held */
 static int fbcon_fb_registered(struct fb_info *info)
 {
 	int ret = 0, i, idx;
@@ -3278,6 +3282,7 @@ static int fbcon_event_notify(struct notifier_block *self,
 		ret = fbcon_fb_unregistered(info);
 		break;
 	case FB_EVENT_SET_CONSOLE_MAP:
+		/* called with console lock held */
 		con2fb = event->data;
 		ret = set_con2fb_map(con2fb->console - 1,
 				     con2fb->framebuffer, 1);
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 070b9a1..dc61c12 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1177,8 +1177,10 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
 		event.data = &con2fb;
 		if (!lock_fb_info(info))
 			return -ENODEV;
+		console_lock();
 		event.info = info;
 		ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, &event);
+		console_unlock();
 		unlock_fb_info(info);
 		break;
 	case FBIOBLANK:
-- 
1.8.1


^ permalink raw reply related

* Re: [git pull] fbcon locking fixes.
From: Dave Airlie @ 2013-01-25  1:45 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Dave Airlie, Andrew Morton, DRI mailing list,
	Linux Kernel Mailing List, linux-fbdev@vger.kernel.org,
	m.b.lankhorst
In-Reply-To: <CAPM=9tz6_=k1Ze5h2935-W59BCVy9KXA_z6t8inW6_-Cxw244Q@mail.gmail.com>

On Fri, Jan 25, 2013 at 11:06 AM, Dave Airlie <airlied@gmail.com> wrote:
> On Fri, Jan 25, 2013 at 10:53 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> On Thu, Jan 24, 2013 at 4:42 PM, Dave Airlie <airlied@linux.ie> wrote:
>>>
>>> These patches have been sailing around long enough, waiting for a maintainer
>>> to reappear, so I've decided enough is enough, lockdep is kinda useful to have.
>>
>> Last this was tried, these patches failed miserably.
>>
>> They caused instant lockdep splat and then a total lockup with efifb.
>> It may be that Takashi's patch helps fix that problem, but it's in no
>> way clear that it does, so the patch series isn't at all obviously
>> stable.
>>
>> Yes, lockdep is indeed "kinda useful", and there clearly are locking
>> problems in fbdev. But I'm not seeing myself pulling these for 3.8.
>> They've been too problematic to pull in at this late stage.
>>
>
> Okay I'll fix the efifb problem and then maybe queue them for -next.

Okay I've just sent out another fbcon patch to fix the locking harder.

There was a path going into set_con2fb_path if an fb driver was
already registered, I just pushed the locking out further on anyone
going in there.

it boots on my EFI macbook here.

Dave.

^ permalink raw reply

* Re: [git pull] fbcon locking fixes.
From: Linus Torvalds @ 2013-01-25  1:57 UTC (permalink / raw)
  To: Dave Airlie
  Cc: Maarten Lankhorst, linux-fbdev@vger.kernel.org,
	Linux Kernel Mailing List, DRI mailing list, Andrew Morton
In-Reply-To: <CAPM=9txUNG_CeK+YBhcA_47BVv4Z2GAmEC_J59cY+D9nRgv54A@mail.gmail.com>

On Thu, Jan 24, 2013 at 5:45 PM, Dave Airlie <airlied@gmail.com> wrote:
>
> Okay I've just sent out another fbcon patch to fix the locking harder.
>
> There was a path going into set_con2fb_path if an fb driver was
> already registered, I just pushed the locking out further on anyone
> going in there.
>
> it boots on my EFI macbook here.

Ok, good. Sounds like we'll finally get it fixed, but I'm still too
much of a scaredy-cat to take it for now, so -next it is...

              Linus

^ permalink raw reply

* [PATCH] vgacon/vt: clear buffer attributes when we load a 512 character font (v2)
From: Dave Airlie @ 2013-01-25  2:21 UTC (permalink / raw)
  To: linux-fbdev; +Cc: dri-devel, linux-kernel, Dave Airlie

From: Dave Airlie <airlied@redhat.com>

When we switch from 256->512 byte font rendering mode, it means the
current contents of the screen is being reinterpreted. The bit that holds
the high bit of the 9-bit font, may have been previously set, and thus
the new font misrenders.

The problem case we see is grub2 writes spaces with the bit set, so it
ends up with data like 0x820, which gets reinterpreted into 0x120 char
which the font translates into G with a circumflex. This flashes up on
screen at boot and is quite ugly.

A current side effect of this patch though is that any rendering on the
screen changes color to a slightly darker color, but at least the screen
no longer corrupts.

v2: as suggested by hpa, always clear the attribute space, whether we
are are going to or from 512 chars.

Signed-off-by: Dave Airlie <airlied@redhat.com>

f
---
 drivers/tty/vt/vt.c            |  2 +-
 drivers/video/console/vgacon.c | 22 +++++++++++++++-------
 include/linux/vt_kern.h        |  1 +
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 8fd8968..c8067ae 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -638,7 +638,7 @@ static inline void save_screen(struct vc_data *vc)
  *	Redrawing of screen
  */
 
-static void clear_buffer_attributes(struct vc_data *vc)
+void clear_buffer_attributes(struct vc_data *vc)
 {
 	unsigned short *p = (unsigned short *)vc->vc_origin;
 	int count = vc->vc_screenbuf_size / 2;
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index d449a74..5855d17 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -1064,7 +1064,7 @@ static int vgacon_do_font_op(struct vgastate *state,char *arg,int set,int ch512)
 	unsigned short video_port_status = vga_video_port_reg + 6;
 	int font_select = 0x00, beg, i;
 	char *charmap;
-	
+	bool clear_attribs = false;
 	if (vga_video_type != VIDEO_TYPE_EGAM) {
 		charmap = (char *) VGA_MAP_MEM(colourmap, 0);
 		beg = 0x0e;
@@ -1169,12 +1169,6 @@ static int vgacon_do_font_op(struct vgastate *state,char *arg,int set,int ch512)
 
 	/* if 512 char mode is already enabled don't re-enable it. */
 	if ((set) && (ch512 != vga_512_chars)) {
-		/* attribute controller */
-		for (i = 0; i < MAX_NR_CONSOLES; i++) {
-			struct vc_data *c = vc_cons[i].d;
-			if (c && c->vc_sw = &vga_con)
-				c->vc_hi_font_mask = ch512 ? 0x0800 : 0;
-		}
 		vga_512_chars = ch512;
 		/* 256-char: enable intensity bit
 		   512-char: disable intensity bit */
@@ -1185,8 +1179,22 @@ static int vgacon_do_font_op(struct vgastate *state,char *arg,int set,int ch512)
 		   it means, but it works, and it appears necessary */
 		inb_p(video_port_status);
 		vga_wattr(state->vgabase, VGA_AR_ENABLE_DISPLAY, 0);	
+		clear_attribs = true;
 	}
 	raw_spin_unlock_irq(&vga_lock);
+
+	if (clear_attribs) {
+		for (i = 0; i < MAX_NR_CONSOLES; i++) {
+			struct vc_data *c = vc_cons[i].d;
+			if (c && c->vc_sw = &vga_con) {
+				/* force hi font mask to 0, so we always clear
+				   the bit on either transition */
+				c->vc_hi_font_mask = 0x00;
+				clear_buffer_attributes(c);
+				c->vc_hi_font_mask = ch512 ? 0x0800 : 0;
+			}
+		}
+	}
 	return 0;
 }
 
diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
index 50ae7d0..1f55665 100644
--- a/include/linux/vt_kern.h
+++ b/include/linux/vt_kern.h
@@ -47,6 +47,7 @@ int con_set_cmap(unsigned char __user *cmap);
 int con_get_cmap(unsigned char __user *cmap);
 void scrollback(struct vc_data *vc, int lines);
 void scrollfront(struct vc_data *vc, int lines);
+void clear_buffer_attributes(struct vc_data *vc);
 void update_region(struct vc_data *vc, unsigned long start, int count);
 void redraw_screen(struct vc_data *vc, int is_switch);
 #define update_screen(x) redraw_screen(x, 0)
-- 
1.8.1


^ permalink raw reply related

* Re: [patch] backlight: s6e63m0: report ->gamma_table_count correctly
From: Jingoo Han @ 2013-01-25  2:22 UTC (permalink / raw)
  To: 'Dan Carpenter'
  Cc: 'Andrew Morton', 'Inki Dae',
	'Richard Purdie', 'Florian Tobias Schandinat',
	linux-fbdev, kernel-janitors, linux-kernel, 'Jingoo Han'
In-Reply-To: <20130124070524.GD5611@elgon.mountain>

On Thursday, January 24, 2013 10:45 PM, Dan Carpenter wrote

CC'ed Andrew Morton, Inki Dae.

> 
> gamma_table has 3 arrays which each hold MAX_GAMMA_LEVEL pointers to
> int.
> 
> The current code sets ->gamma_table_count to 6 on 64bit arches and to 3
> on 32 bit arches.  It should be 3 on everything.

Actually, I don't know it is right.
However, it is certain that this panel is currently used on 32 bit arches
such as ARM SoCs.

> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> This is from reading the code.  We use ->gamma_table_count in sysfs file
> but other than that I'm not sure what it's for.  I am not very familiar
> with this code.
> 
> diff --git a/drivers/video/backlight/s6e63m0.c b/drivers/video/backlight/s6e63m0.c
> index 2126b96..9c2677f 100644
> --- a/drivers/video/backlight/s6e63m0.c
> +++ b/drivers/video/backlight/s6e63m0.c
> @@ -766,7 +766,7 @@ static int s6e63m0_probe(struct spi_device *spi)
>  	 * know that.
>  	 */
>  	lcd->gamma_table_count > -	    sizeof(gamma_table) / (MAX_GAMMA_LEVEL * sizeof(int));
> +	    sizeof(gamma_table) / (MAX_GAMMA_LEVEL * sizeof(int *));
> 
>  	ret = device_create_file(&(spi->dev), &dev_attr_gamma_mode);
>  	if (ret < 0)
> --
> 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: [PATCH] backlight: add an AS3711 PMIC backlight driver
From: Jingoo Han @ 2013-01-25  5:20 UTC (permalink / raw)
  To: 'Guennadi Liakhovetski'
  Cc: 'Andrew Morton', linux-kernel, linux-fbdev, linux-sh,
	'Magnus Damm', 'Richard Purdie'
In-Reply-To: <Pine.LNX.4.64.1301081846020.1794@axis700.grange>

On Wednesday, January 09, 2013 2:55 AM, Guennadi Liakhovetski wrote
> 
> This is an initial commit of a backlight driver, using step-up DCDC power
> supplies on AS3711 PMIC. Only one mode has actually been tested, several
> further modes have been implemented "dry," but disabled to avoid accidental
> hardware damage. Anyone wishing to use any of those modes will have to
> modify the driver.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

CC'ed Andrew Morton.

Hi Guennadi Liakhovetski,

I have reviewed this patch with AS3711 datasheet.
I cannot find any problems. It looks good.
However, some hardcoded numbers need to be changed
to the bit definitions.


Acked-by: Jingoo Han <jg1.han@samsung.com>


Best regards,
Jingoo Han

> ---
> 
> Tested on sh73a0-based kzm9g board. As the commit message says, only one
> mode has been tested and is enabled. That mode copies the sample code from
> the manufacturer. Deviations from that code proved to be fatal for the
> hardware...
> 
>  drivers/video/backlight/Kconfig     |    7 +
>  drivers/video/backlight/Makefile    |    1 +
>  drivers/video/backlight/as3711_bl.c |  379 +++++++++++++++++++++++++++++++++++
>  3 files changed, 387 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/video/backlight/as3711_bl.c
> 
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 765a945..6ef0ede 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -390,6 +390,13 @@ config BACKLIGHT_TPS65217
>  	  If you have a Texas Instruments TPS65217 say Y to enable the
>  	  backlight driver.
> 
> +config BACKLIGHT_AS3711
> +	tristate "AS3711 Backlight"
> +	depends on BACKLIGHT_CLASS_DEVICE && MFD_AS3711
> +	help
> +	  If you have an Austrian Microsystems AS3711 say Y to enable the
> +	  backlight driver.
> +
>  endif # BACKLIGHT_CLASS_DEVICE
> 
>  endif # BACKLIGHT_LCD_SUPPORT
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index e7ce729..d3e188f 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -45,3 +45,4 @@ obj-$(CONFIG_BACKLIGHT_PCF50633)	+= pcf50633-backlight.o
>  obj-$(CONFIG_BACKLIGHT_AAT2870) += aat2870_bl.o
>  obj-$(CONFIG_BACKLIGHT_OT200) += ot200_bl.o
>  obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
> +obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o
> diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c
> new file mode 100644
> index 0000000..c6bc65d
> --- /dev/null
> +++ b/drivers/video/backlight/as3711_bl.c
> @@ -0,0 +1,379 @@
> +/*
> + * AS3711 PMIC backlight driver, using DCDC Step Up Converters
> + *
> + * Copyright (C) 2012 Renesas Electronics Corporation
> + * Author: Guennadi Liakhovetski, <g.liakhovetski@gmx.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the version 2 of the GNU General Public License as
> + * published by the Free Software Foundation
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/fb.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/as3711.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +enum as3711_bl_type {
> +	AS3711_BL_SU1,
> +	AS3711_BL_SU2,
> +};
> +
> +struct as3711_bl_data {
> +	bool powered;
> +	const char *fb_name;
> +	struct device *fb_dev;
> +	enum as3711_bl_type type;
> +	int brightness;
> +	struct backlight_device *bl;
> +};
> +
> +struct as3711_bl_supply {
> +	struct as3711_bl_data su1;
> +	struct as3711_bl_data su2;
> +	const struct as3711_bl_pdata *pdata;
> +	struct as3711 *as3711;
> +};
> +
> +static struct as3711_bl_supply *to_supply(struct as3711_bl_data *su)
> +{
> +	switch (su->type) {
> +	case AS3711_BL_SU1:
> +		return container_of(su, struct as3711_bl_supply, su1);
> +	case AS3711_BL_SU2:
> +		return container_of(su, struct as3711_bl_supply, su2);
> +	}
> +	return NULL;
> +}
> +
> +static int as3711_set_brightness_auto_i(struct as3711_bl_data *data,
> +					unsigned int brightness)
> +{
> +	struct as3711_bl_supply *supply = to_supply(data);
> +	struct as3711 *as3711 = supply->as3711;
> +	const struct as3711_bl_pdata *pdata = supply->pdata;
> +	int ret = 0;
> +
> +	/* Only all equal current values are supported */
> +	if (pdata->su2_auto_curr1)
> +		ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE,
> +				   brightness);
> +	if (!ret && pdata->su2_auto_curr2)
> +		ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE,
> +				   brightness);
> +	if (!ret && pdata->su2_auto_curr3)
> +		ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE,
> +				   brightness);
> +
> +	return ret;
> +}
> +
> +static int as3711_set_brightness_v(struct as3711 *as3711,
> +				   unsigned int brightness,
> +				   unsigned int reg)
> +{
> +	if (brightness > 31)
> +		return -EINVAL;
> +
> +	return regmap_update_bits(as3711->regmap, reg, 0xf0,
> +				  brightness << 4);
> +}
> +
> +static int as3711_bl_su2_reset(struct as3711_bl_supply *supply)
> +{
> +	struct as3711 *as3711 = supply->as3711;
> +	int ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_5,
> +				     3, supply->pdata->su2_fbprot);
> +	if (!ret)
> +		ret = regmap_update_bits(as3711->regmap,
> +					 AS3711_STEPUP_CONTROL_2, 1, 0);
> +	if (!ret)
> +		ret = regmap_update_bits(as3711->regmap,
> +					 AS3711_STEPUP_CONTROL_2, 1, 1);
> +	return ret;
> +}
> +
> +/*
> + * Someone with less fragile or less expensive hardware could try to simplify
> + * the brightness adjustment procedure.
> + */
> +static int as3711_bl_update_status(struct backlight_device *bl)
> +{
> +	struct as3711_bl_data *data = bl_get_data(bl);
> +	struct as3711_bl_supply *supply = to_supply(data);
> +	struct as3711 *as3711 = supply->as3711;
> +	int brightness = bl->props.brightness;
> +	int ret = 0;
> +
> +	dev_dbg(&bl->dev, "%s(): brightness %u, pwr %x, blank %x, state %x\n",
> +		__func__, bl->props.brightness, bl->props.power,
> +		bl->props.fb_blank, bl->props.state);
> +
> +	if (bl->props.power != FB_BLANK_UNBLANK ||
> +	    bl->props.fb_blank != FB_BLANK_UNBLANK ||
> +	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> +		brightness = 0;
> +
> +	if (data->type = AS3711_BL_SU1) {
> +		ret = as3711_set_brightness_v(as3711, brightness,
> +					      AS3711_STEPUP_CONTROL_1);
> +	} else {
> +		const struct as3711_bl_pdata *pdata = supply->pdata;
> +
> +		switch (pdata->su2_feedback) {
> +		case AS3711_SU2_VOLTAGE:
> +			ret = as3711_set_brightness_v(as3711, brightness,
> +						      AS3711_STEPUP_CONTROL_2);
> +			break;
> +		case AS3711_SU2_CURR_AUTO:
> +			ret = as3711_set_brightness_auto_i(data, brightness / 4);
> +			if (ret < 0)
> +				return ret;
> +			if (brightness) {
> +				ret = as3711_bl_su2_reset(supply);
> +				if (ret < 0)
> +					return ret;
> +				udelay(500);
> +				ret = as3711_set_brightness_auto_i(data, brightness);
> +			} else {
> +				ret = regmap_update_bits(as3711->regmap,
> +						AS3711_STEPUP_CONTROL_2, 1, 0);
> +			}
> +			break;
> +		/* Manual one current feedback pin below */
> +		case AS3711_SU2_CURR1:
> +			ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE,
> +					   brightness);
> +			break;
> +		case AS3711_SU2_CURR2:
> +			ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE,
> +					   brightness);
> +			break;
> +		case AS3711_SU2_CURR3:
> +			ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE,
> +					   brightness);
> +			break;
> +		default:
> +			ret = -EINVAL;
> +		}
> +	}
> +	if (!ret)
> +		data->brightness = brightness;
> +
> +	return ret;
> +}
> +
> +static int as3711_bl_get_brightness(struct backlight_device *bl)
> +{
> +	struct as3711_bl_data *data = bl_get_data(bl);
> +
> +	return data->brightness;
> +}
> +
> +static const struct backlight_ops as3711_bl_ops = {
> +	.update_status	= as3711_bl_update_status,
> +	.get_brightness	= as3711_bl_get_brightness,
> +};
> +
> +static int as3711_bl_init_su2(struct as3711_bl_supply *supply)
> +{
> +	struct as3711 *as3711 = supply->as3711;
> +	const struct as3711_bl_pdata *pdata = supply->pdata;
> +	u8 ctl = 0;
> +	int ret;
> +
> +	dev_dbg(as3711->dev, "%s(): use %u\n", __func__, pdata->su2_feedback);
> +
> +	/* Turn SU2 off */
> +	ret = regmap_write(as3711->regmap, AS3711_STEPUP_CONTROL_2, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (pdata->su2_feedback) {
> +	case AS3711_SU2_VOLTAGE:
> +		ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 0);
> +		break;
> +	case AS3711_SU2_CURR1:
> +		ctl = 1;
> +		ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 1);
> +		break;
> +	case AS3711_SU2_CURR2:
> +		ctl = 4;
> +		ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 2);
> +		break;
> +	case AS3711_SU2_CURR3:
> +		ctl = 0x10;
> +		ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 3);
> +		break;
> +	case AS3711_SU2_CURR_AUTO:
> +		if (pdata->su2_auto_curr1)
> +			ctl = 2;
> +		if (pdata->su2_auto_curr2)
> +			ctl |= 8;
> +		if (pdata->su2_auto_curr3)
> +			ctl |= 0x20;
> +		ret = 0;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (!ret)
> +		ret = regmap_write(as3711->regmap, AS3711_CURR_CONTROL, ctl);
> +
> +	return ret;
> +}
> +
> +static int as3711_bl_register(struct platform_device *pdev,
> +			      unsigned int max_brightness, struct as3711_bl_data *su)
> +{
> +	struct backlight_properties props = {.type = BACKLIGHT_RAW,};
> +	struct backlight_device *bl;
> +
> +	/* max tuning I = 31uA for voltage- and 38250uA for current-feedback */
> +	props.max_brightness = max_brightness;
> +
> +	bl = backlight_device_register(su->type = AS3711_BL_SU1 ?
> +				       "as3711-su1" : "as3711-su2",
> +				       &pdev->dev, su,
> +				       &as3711_bl_ops, &props);
> +	if (IS_ERR(bl)) {
> +		dev_err(&pdev->dev, "failed to register backlight\n");
> +		return PTR_ERR(bl);
> +	}
> +
> +	bl->props.brightness = props.max_brightness;
> +
> +	backlight_update_status(bl);
> +
> +	su->bl = bl;
> +
> +	return 0;
> +}
> +
> +static int as3711_backlight_probe(struct platform_device *pdev)
> +{
> +	struct as3711_bl_pdata *pdata = dev_get_platdata(&pdev->dev);
> +	struct as3711 *as3711 = dev_get_drvdata(pdev->dev.parent);
> +	struct as3711_bl_supply *supply;
> +	struct as3711_bl_data *su;
> +	unsigned int max_brightness;
> +	int ret;
> +
> +	if (!pdata || (!pdata->su1_fb && !pdata->su2_fb)) {
> +		dev_err(&pdev->dev, "No platform data, exiting...\n");
> +		return -ENODEV;
> +	}
> +
> +	/*
> +	 * Due to possible hardware damage I chose to block all modes,
> +	 * unsupported on my hardware. Anyone, wishing to use any of those modes
> +	 * will have to first review the code, then activate and test it.
> +	 */
> +	if (pdata->su1_fb ||
> +	    pdata->su2_fbprot != AS3711_SU2_GPIO4 ||
> +	    pdata->su2_feedback != AS3711_SU2_CURR_AUTO) {
> +		dev_warn(&pdev->dev,
> +			 "Attention! An untested mode has been chosen!\n"
> +			 "Please, review the code, enable, test, and report success:-)\n");
> +		return -EINVAL;
> +	}
> +
> +	supply = devm_kzalloc(&pdev->dev, sizeof(*supply), GFP_KERNEL);
> +	if (!supply)
> +		return -ENOMEM;
> +
> +	supply->as3711 = as3711;
> +	supply->pdata = pdata;
> +
> +	if (pdata->su1_fb) {
> +		su = &supply->su1;
> +		su->fb_name = pdata->su1_fb;
> +		su->type = AS3711_BL_SU1;
> +
> +		max_brightness = min(pdata->su1_max_uA, 31);
> +		ret = as3711_bl_register(pdev, max_brightness, su);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	if (pdata->su2_fb) {
> +		su = &supply->su2;
> +		su->fb_name = pdata->su2_fb;
> +		su->type = AS3711_BL_SU2;
> +
> +		switch (pdata->su2_fbprot) {
> +		case AS3711_SU2_GPIO2:
> +		case AS3711_SU2_GPIO3:
> +		case AS3711_SU2_GPIO4:
> +		case AS3711_SU2_LX_SD4:
> +			break;
> +		default:
> +			ret = -EINVAL;
> +			goto esu2;
> +		}
> +
> +		switch (pdata->su2_feedback) {
> +		case AS3711_SU2_VOLTAGE:
> +			max_brightness = min(pdata->su2_max_uA, 31);
> +			break;
> +		case AS3711_SU2_CURR1:
> +		case AS3711_SU2_CURR2:
> +		case AS3711_SU2_CURR3:
> +		case AS3711_SU2_CURR_AUTO:
> +			max_brightness = min(pdata->su2_max_uA / 150, 255);
> +			break;
> +		default:
> +			ret = -EINVAL;
> +			goto esu2;
> +		}
> +
> +		ret = as3711_bl_init_su2(supply);
> +		if (ret < 0)
> +			return ret;
> +
> +		ret = as3711_bl_register(pdev, max_brightness, su);
> +		if (ret < 0)
> +			goto esu2;
> +	}
> +
> +	platform_set_drvdata(pdev, supply);
> +
> +	return 0;
> +
> +esu2:
> +	backlight_device_unregister(supply->su1.bl);
> +	return ret;
> +}
> +
> +static int as3711_backlight_remove(struct platform_device *pdev)
> +{
> +	struct as3711_bl_supply *supply = platform_get_drvdata(pdev);
> +
> +	backlight_device_unregister(supply->su1.bl);
> +	backlight_device_unregister(supply->su2.bl);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver as3711_backlight_driver = {
> +	.driver		= {
> +		.name	= "as3711-backlight",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= as3711_backlight_probe,
> +	.remove		= as3711_backlight_remove,
> +};
> +
> +module_platform_driver(as3711_backlight_driver);
> +
> +MODULE_DESCRIPTION("Backlight Driver for AS3711 PMICs");
> +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:as3711-backlight");
> --
> 1.7.2.5
> 
> --
> 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

* [patch 1/2] video/kyro: delete some dead code
From: Dan Carpenter @ 2013-01-25  6:42 UTC (permalink / raw)
  To: Florian Tobias Schandinat; +Cc: linux-fbdev, linux-kernel, kernel-janitors

_OLDCODE is never defined.  Delete it.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/video/kyro/STG4000OverlayDevice.c b/drivers/video/kyro/STG4000OverlayDevice.c
index 0aeeaa1..84b6ea8 100644
--- a/drivers/video/kyro/STG4000OverlayDevice.c
+++ b/drivers/video/kyro/STG4000OverlayDevice.c
@@ -430,10 +430,7 @@ int SetOverlayViewPort(volatile STG4000REG __iomem *pSTGReg,
 	 */
 	ulSrc = srcDest.ulSrcX2 - srcDest.ulSrcX1;
 	ulDest = srcDest.lDstX2 - srcDest.lDstX1;
-#ifdef _OLDCODE
-	ulLeft = srcDest.ulDstX1;
-	ulRight = srcDest.ulDstX2;
-#else
+
 	if (srcDest.ulDstX1 > 2) {
 		ulLeft = srcDest.ulDstX1 + 2;
 		ulRight = srcDest.ulDstX2 + 1;
@@ -441,7 +438,7 @@ int SetOverlayViewPort(volatile STG4000REG __iomem *pSTGReg,
 		ulLeft = srcDest.ulDstX1;
 		ulRight = srcDest.ulDstX2 + 1;
 	}
-#endif
+
 	/* first work out the position we are to display as offset from the source of the buffer */
 	bResult = 1;
 

^ permalink raw reply related

* [patch 2/2] video/kyro: some potential divide by zero bugs
From: Dan Carpenter @ 2013-01-25  6:42 UTC (permalink / raw)
  To: linux-fbdev

These values come from the user in kyrofb_ioctl().  There are a couple
potential divide by zero problems and I have added tests for them.
There were a couple tests to prevent divide by zero bugs already in the
code that I moved next to the assignments.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/video/kyro/STG4000OverlayDevice.c b/drivers/video/kyro/STG4000OverlayDevice.c
index 84b6ea8..d03a1e4 100644
--- a/drivers/video/kyro/STG4000OverlayDevice.c
+++ b/drivers/video/kyro/STG4000OverlayDevice.c
@@ -365,15 +365,18 @@ int SetOverlayViewPort(volatile STG4000REG __iomem *pSTGReg,
 	ulSrcBottom = srcDest.ulSrcY2;
 
 	ulSrc = ulSrcBottom - ulSrcTop;
-	ulDest = srcDest.lDstY2 - srcDest.lDstY1;	/* on-screen overlay */
-
 	if (ulSrc <= 1)
 		return -EINVAL;
+	ulDest = srcDest.lDstY2 - srcDest.lDstY1;	/* on-screen overlay */
+	if (ulDest = -1)
+		return -EINVAL;
 
 	/* First work out the position we are to display as offset from the
 	 * source of the buffer
 	 */
 	ulFxScale = (ulDest << 11) / ulSrc;	/* fixed point scale factor */
+	if (ulFxScale = 0)
+		return -EINVAL;
 	ulFxOffset = (srcDest.lDstY2 - srcDest.ulDstY2) << 11;
 
 	ulSrcBottom = ulSrcBottom - (ulFxOffset / ulFxScale);
@@ -430,6 +433,8 @@ int SetOverlayViewPort(volatile STG4000REG __iomem *pSTGReg,
 	 */
 	ulSrc = srcDest.ulSrcX2 - srcDest.ulSrcX1;
 	ulDest = srcDest.lDstX2 - srcDest.lDstX1;
+	if (ulDest = 0)
+		return -EINVAL;
 
 	if (srcDest.ulDstX1 > 2) {
 		ulLeft = srcDest.ulDstX1 + 2;
@@ -438,14 +443,14 @@ int SetOverlayViewPort(volatile STG4000REG __iomem *pSTGReg,
 		ulLeft = srcDest.ulDstX1;
 		ulRight = srcDest.ulDstX2 + 1;
 	}
+	if (ulRight - ulLeft + 2 = 0)
+		return -EINVAL;
+
 
 	/* first work out the position we are to display as offset from the source of the buffer */
 	bResult = 1;
 
 	do {
-		if (ulDest = 0)
-			return -EINVAL;
-
 		/* source pixels per dest pixel <<11 */
 		ulFxScale = ((ulSrc - 1) << 11) / (ulDest);
 

^ permalink raw reply related

* Re: [PATCH] backlight: add an AS3711 PMIC backlight driver
From: Guennadi Liakhovetski @ 2013-01-25  7:48 UTC (permalink / raw)
  To: Jingoo Han
  Cc: 'Andrew Morton', linux-kernel, linux-fbdev, linux-sh,
	'Magnus Damm', 'Richard Purdie'
In-Reply-To: <00c901cdfabb$a9989570$fcc9c050$%han@samsung.com>

Hi Jingoo Han

On Fri, 25 Jan 2013, Jingoo Han wrote:

> On Wednesday, January 09, 2013 2:55 AM, Guennadi Liakhovetski wrote
> > 
> > This is an initial commit of a backlight driver, using step-up DCDC power
> > supplies on AS3711 PMIC. Only one mode has actually been tested, several
> > further modes have been implemented "dry," but disabled to avoid accidental
> > hardware damage. Anyone wishing to use any of those modes will have to
> > modify the driver.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> 
> CC'ed Andrew Morton.
> 
> Hi Guennadi Liakhovetski,
> 
> I have reviewed this patch with AS3711 datasheet.
> I cannot find any problems. It looks good.

Thanks for the review.

> However, some hardcoded numbers need to be changed
> to the bit definitions.

Which specific hardcoded values do you mean? A while ago in a discussion 
on one of kernel mailing lists a conclusion has been made, that macros do 
not always improve code readability. E.g. in a statement like this

+	case AS3711_SU2_CURR_AUTO:
+		if (pdata->su2_auto_curr1)
+			ctl = 2;
+		if (pdata->su2_auto_curr2)
+			ctl |= 8;
+		if (pdata->su2_auto_curr3)
+			ctl |= 0x20;

making it 

+	case AS3711_SU2_CURR_AUTO:
+		if (pdata->su2_auto_curr1)
+			ctl = SU2_AUTO_CURR1;

would hardly make it more readable. IMHO it is already pretty clear, that 
bit 1 enables the current-1 sink. As long as these fields are only used at 
one location in the driver (and they are), using a macro and defining it 
elsewhere only makes it harder to see actual values. Speaking of this, a 
comment like

		/*
		 * Select, which current sinks shall be used for automatic 
		 * feedback selection
		 */

would help much more than any macro names. But as it stands, I think the 
current version is also possible to understand :-) If desired, however, 
comments can be added in an incremental patch.

Thanks
Guennadi

> Acked-by: Jingoo Han <jg1.han@samsung.com>
> 
> 
> Best regards,
> Jingoo Han
> 
> > ---
> > 
> > Tested on sh73a0-based kzm9g board. As the commit message says, only one
> > mode has been tested and is enabled. That mode copies the sample code from
> > the manufacturer. Deviations from that code proved to be fatal for the
> > hardware...
> > 
> >  drivers/video/backlight/Kconfig     |    7 +
> >  drivers/video/backlight/Makefile    |    1 +
> >  drivers/video/backlight/as3711_bl.c |  379 +++++++++++++++++++++++++++++++++++
> >  3 files changed, 387 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/video/backlight/as3711_bl.c
> > 
> > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> > index 765a945..6ef0ede 100644
> > --- a/drivers/video/backlight/Kconfig
> > +++ b/drivers/video/backlight/Kconfig
> > @@ -390,6 +390,13 @@ config BACKLIGHT_TPS65217
> >  	  If you have a Texas Instruments TPS65217 say Y to enable the
> >  	  backlight driver.
> > 
> > +config BACKLIGHT_AS3711
> > +	tristate "AS3711 Backlight"
> > +	depends on BACKLIGHT_CLASS_DEVICE && MFD_AS3711
> > +	help
> > +	  If you have an Austrian Microsystems AS3711 say Y to enable the
> > +	  backlight driver.
> > +
> >  endif # BACKLIGHT_CLASS_DEVICE
> > 
> >  endif # BACKLIGHT_LCD_SUPPORT
> > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> > index e7ce729..d3e188f 100644
> > --- a/drivers/video/backlight/Makefile
> > +++ b/drivers/video/backlight/Makefile
> > @@ -45,3 +45,4 @@ obj-$(CONFIG_BACKLIGHT_PCF50633)	+= pcf50633-backlight.o
> >  obj-$(CONFIG_BACKLIGHT_AAT2870) += aat2870_bl.o
> >  obj-$(CONFIG_BACKLIGHT_OT200) += ot200_bl.o
> >  obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
> > +obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o
> > diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c
> > new file mode 100644
> > index 0000000..c6bc65d
> > --- /dev/null
> > +++ b/drivers/video/backlight/as3711_bl.c
> > @@ -0,0 +1,379 @@
> > +/*
> > + * AS3711 PMIC backlight driver, using DCDC Step Up Converters
> > + *
> > + * Copyright (C) 2012 Renesas Electronics Corporation
> > + * Author: Guennadi Liakhovetski, <g.liakhovetski@gmx.de>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the version 2 of the GNU General Public License as
> > + * published by the Free Software Foundation
> > + */
> > +
> > +#include <linux/backlight.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/fb.h>
> > +#include <linux/kernel.h>
> > +#include <linux/mfd/as3711.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +
> > +enum as3711_bl_type {
> > +	AS3711_BL_SU1,
> > +	AS3711_BL_SU2,
> > +};
> > +
> > +struct as3711_bl_data {
> > +	bool powered;
> > +	const char *fb_name;
> > +	struct device *fb_dev;
> > +	enum as3711_bl_type type;
> > +	int brightness;
> > +	struct backlight_device *bl;
> > +};
> > +
> > +struct as3711_bl_supply {
> > +	struct as3711_bl_data su1;
> > +	struct as3711_bl_data su2;
> > +	const struct as3711_bl_pdata *pdata;
> > +	struct as3711 *as3711;
> > +};
> > +
> > +static struct as3711_bl_supply *to_supply(struct as3711_bl_data *su)
> > +{
> > +	switch (su->type) {
> > +	case AS3711_BL_SU1:
> > +		return container_of(su, struct as3711_bl_supply, su1);
> > +	case AS3711_BL_SU2:
> > +		return container_of(su, struct as3711_bl_supply, su2);
> > +	}
> > +	return NULL;
> > +}
> > +
> > +static int as3711_set_brightness_auto_i(struct as3711_bl_data *data,
> > +					unsigned int brightness)
> > +{
> > +	struct as3711_bl_supply *supply = to_supply(data);
> > +	struct as3711 *as3711 = supply->as3711;
> > +	const struct as3711_bl_pdata *pdata = supply->pdata;
> > +	int ret = 0;
> > +
> > +	/* Only all equal current values are supported */
> > +	if (pdata->su2_auto_curr1)
> > +		ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE,
> > +				   brightness);
> > +	if (!ret && pdata->su2_auto_curr2)
> > +		ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE,
> > +				   brightness);
> > +	if (!ret && pdata->su2_auto_curr3)
> > +		ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE,
> > +				   brightness);
> > +
> > +	return ret;
> > +}
> > +
> > +static int as3711_set_brightness_v(struct as3711 *as3711,
> > +				   unsigned int brightness,
> > +				   unsigned int reg)
> > +{
> > +	if (brightness > 31)
> > +		return -EINVAL;
> > +
> > +	return regmap_update_bits(as3711->regmap, reg, 0xf0,
> > +				  brightness << 4);
> > +}
> > +
> > +static int as3711_bl_su2_reset(struct as3711_bl_supply *supply)
> > +{
> > +	struct as3711 *as3711 = supply->as3711;
> > +	int ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_5,
> > +				     3, supply->pdata->su2_fbprot);
> > +	if (!ret)
> > +		ret = regmap_update_bits(as3711->regmap,
> > +					 AS3711_STEPUP_CONTROL_2, 1, 0);
> > +	if (!ret)
> > +		ret = regmap_update_bits(as3711->regmap,
> > +					 AS3711_STEPUP_CONTROL_2, 1, 1);
> > +	return ret;
> > +}
> > +
> > +/*
> > + * Someone with less fragile or less expensive hardware could try to simplify
> > + * the brightness adjustment procedure.
> > + */
> > +static int as3711_bl_update_status(struct backlight_device *bl)
> > +{
> > +	struct as3711_bl_data *data = bl_get_data(bl);
> > +	struct as3711_bl_supply *supply = to_supply(data);
> > +	struct as3711 *as3711 = supply->as3711;
> > +	int brightness = bl->props.brightness;
> > +	int ret = 0;
> > +
> > +	dev_dbg(&bl->dev, "%s(): brightness %u, pwr %x, blank %x, state %x\n",
> > +		__func__, bl->props.brightness, bl->props.power,
> > +		bl->props.fb_blank, bl->props.state);
> > +
> > +	if (bl->props.power != FB_BLANK_UNBLANK ||
> > +	    bl->props.fb_blank != FB_BLANK_UNBLANK ||
> > +	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> > +		brightness = 0;
> > +
> > +	if (data->type = AS3711_BL_SU1) {
> > +		ret = as3711_set_brightness_v(as3711, brightness,
> > +					      AS3711_STEPUP_CONTROL_1);
> > +	} else {
> > +		const struct as3711_bl_pdata *pdata = supply->pdata;
> > +
> > +		switch (pdata->su2_feedback) {
> > +		case AS3711_SU2_VOLTAGE:
> > +			ret = as3711_set_brightness_v(as3711, brightness,
> > +						      AS3711_STEPUP_CONTROL_2);
> > +			break;
> > +		case AS3711_SU2_CURR_AUTO:
> > +			ret = as3711_set_brightness_auto_i(data, brightness / 4);
> > +			if (ret < 0)
> > +				return ret;
> > +			if (brightness) {
> > +				ret = as3711_bl_su2_reset(supply);
> > +				if (ret < 0)
> > +					return ret;
> > +				udelay(500);
> > +				ret = as3711_set_brightness_auto_i(data, brightness);
> > +			} else {
> > +				ret = regmap_update_bits(as3711->regmap,
> > +						AS3711_STEPUP_CONTROL_2, 1, 0);
> > +			}
> > +			break;
> > +		/* Manual one current feedback pin below */
> > +		case AS3711_SU2_CURR1:
> > +			ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE,
> > +					   brightness);
> > +			break;
> > +		case AS3711_SU2_CURR2:
> > +			ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE,
> > +					   brightness);
> > +			break;
> > +		case AS3711_SU2_CURR3:
> > +			ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE,
> > +					   brightness);
> > +			break;
> > +		default:
> > +			ret = -EINVAL;
> > +		}
> > +	}
> > +	if (!ret)
> > +		data->brightness = brightness;
> > +
> > +	return ret;
> > +}
> > +
> > +static int as3711_bl_get_brightness(struct backlight_device *bl)
> > +{
> > +	struct as3711_bl_data *data = bl_get_data(bl);
> > +
> > +	return data->brightness;
> > +}
> > +
> > +static const struct backlight_ops as3711_bl_ops = {
> > +	.update_status	= as3711_bl_update_status,
> > +	.get_brightness	= as3711_bl_get_brightness,
> > +};
> > +
> > +static int as3711_bl_init_su2(struct as3711_bl_supply *supply)
> > +{
> > +	struct as3711 *as3711 = supply->as3711;
> > +	const struct as3711_bl_pdata *pdata = supply->pdata;
> > +	u8 ctl = 0;
> > +	int ret;
> > +
> > +	dev_dbg(as3711->dev, "%s(): use %u\n", __func__, pdata->su2_feedback);
> > +
> > +	/* Turn SU2 off */
> > +	ret = regmap_write(as3711->regmap, AS3711_STEPUP_CONTROL_2, 0);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	switch (pdata->su2_feedback) {
> > +	case AS3711_SU2_VOLTAGE:
> > +		ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 0);
> > +		break;
> > +	case AS3711_SU2_CURR1:
> > +		ctl = 1;
> > +		ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 1);
> > +		break;
> > +	case AS3711_SU2_CURR2:
> > +		ctl = 4;
> > +		ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 2);
> > +		break;
> > +	case AS3711_SU2_CURR3:
> > +		ctl = 0x10;
> > +		ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 3);
> > +		break;
> > +	case AS3711_SU2_CURR_AUTO:
> > +		if (pdata->su2_auto_curr1)
> > +			ctl = 2;
> > +		if (pdata->su2_auto_curr2)
> > +			ctl |= 8;
> > +		if (pdata->su2_auto_curr3)
> > +			ctl |= 0x20;
> > +		ret = 0;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!ret)
> > +		ret = regmap_write(as3711->regmap, AS3711_CURR_CONTROL, ctl);
> > +
> > +	return ret;
> > +}
> > +
> > +static int as3711_bl_register(struct platform_device *pdev,
> > +			      unsigned int max_brightness, struct as3711_bl_data *su)
> > +{
> > +	struct backlight_properties props = {.type = BACKLIGHT_RAW,};
> > +	struct backlight_device *bl;
> > +
> > +	/* max tuning I = 31uA for voltage- and 38250uA for current-feedback */
> > +	props.max_brightness = max_brightness;
> > +
> > +	bl = backlight_device_register(su->type = AS3711_BL_SU1 ?
> > +				       "as3711-su1" : "as3711-su2",
> > +				       &pdev->dev, su,
> > +				       &as3711_bl_ops, &props);
> > +	if (IS_ERR(bl)) {
> > +		dev_err(&pdev->dev, "failed to register backlight\n");
> > +		return PTR_ERR(bl);
> > +	}
> > +
> > +	bl->props.brightness = props.max_brightness;
> > +
> > +	backlight_update_status(bl);
> > +
> > +	su->bl = bl;
> > +
> > +	return 0;
> > +}
> > +
> > +static int as3711_backlight_probe(struct platform_device *pdev)
> > +{
> > +	struct as3711_bl_pdata *pdata = dev_get_platdata(&pdev->dev);
> > +	struct as3711 *as3711 = dev_get_drvdata(pdev->dev.parent);
> > +	struct as3711_bl_supply *supply;
> > +	struct as3711_bl_data *su;
> > +	unsigned int max_brightness;
> > +	int ret;
> > +
> > +	if (!pdata || (!pdata->su1_fb && !pdata->su2_fb)) {
> > +		dev_err(&pdev->dev, "No platform data, exiting...\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	/*
> > +	 * Due to possible hardware damage I chose to block all modes,
> > +	 * unsupported on my hardware. Anyone, wishing to use any of those modes
> > +	 * will have to first review the code, then activate and test it.
> > +	 */
> > +	if (pdata->su1_fb ||
> > +	    pdata->su2_fbprot != AS3711_SU2_GPIO4 ||
> > +	    pdata->su2_feedback != AS3711_SU2_CURR_AUTO) {
> > +		dev_warn(&pdev->dev,
> > +			 "Attention! An untested mode has been chosen!\n"
> > +			 "Please, review the code, enable, test, and report success:-)\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	supply = devm_kzalloc(&pdev->dev, sizeof(*supply), GFP_KERNEL);
> > +	if (!supply)
> > +		return -ENOMEM;
> > +
> > +	supply->as3711 = as3711;
> > +	supply->pdata = pdata;
> > +
> > +	if (pdata->su1_fb) {
> > +		su = &supply->su1;
> > +		su->fb_name = pdata->su1_fb;
> > +		su->type = AS3711_BL_SU1;
> > +
> > +		max_brightness = min(pdata->su1_max_uA, 31);
> > +		ret = as3711_bl_register(pdev, max_brightness, su);
> > +		if (ret < 0)
> > +			return ret;
> > +	}
> > +
> > +	if (pdata->su2_fb) {
> > +		su = &supply->su2;
> > +		su->fb_name = pdata->su2_fb;
> > +		su->type = AS3711_BL_SU2;
> > +
> > +		switch (pdata->su2_fbprot) {
> > +		case AS3711_SU2_GPIO2:
> > +		case AS3711_SU2_GPIO3:
> > +		case AS3711_SU2_GPIO4:
> > +		case AS3711_SU2_LX_SD4:
> > +			break;
> > +		default:
> > +			ret = -EINVAL;
> > +			goto esu2;
> > +		}
> > +
> > +		switch (pdata->su2_feedback) {
> > +		case AS3711_SU2_VOLTAGE:
> > +			max_brightness = min(pdata->su2_max_uA, 31);
> > +			break;
> > +		case AS3711_SU2_CURR1:
> > +		case AS3711_SU2_CURR2:
> > +		case AS3711_SU2_CURR3:
> > +		case AS3711_SU2_CURR_AUTO:
> > +			max_brightness = min(pdata->su2_max_uA / 150, 255);
> > +			break;
> > +		default:
> > +			ret = -EINVAL;
> > +			goto esu2;
> > +		}
> > +
> > +		ret = as3711_bl_init_su2(supply);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		ret = as3711_bl_register(pdev, max_brightness, su);
> > +		if (ret < 0)
> > +			goto esu2;
> > +	}
> > +
> > +	platform_set_drvdata(pdev, supply);
> > +
> > +	return 0;
> > +
> > +esu2:
> > +	backlight_device_unregister(supply->su1.bl);
> > +	return ret;
> > +}
> > +
> > +static int as3711_backlight_remove(struct platform_device *pdev)
> > +{
> > +	struct as3711_bl_supply *supply = platform_get_drvdata(pdev);
> > +
> > +	backlight_device_unregister(supply->su1.bl);
> > +	backlight_device_unregister(supply->su2.bl);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver as3711_backlight_driver = {
> > +	.driver		= {
> > +		.name	= "as3711-backlight",
> > +		.owner	= THIS_MODULE,
> > +	},
> > +	.probe		= as3711_backlight_probe,
> > +	.remove		= as3711_backlight_remove,
> > +};
> > +
> > +module_platform_driver(as3711_backlight_driver);
> > +
> > +MODULE_DESCRIPTION("Backlight Driver for AS3711 PMICs");
> > +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de");
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("platform:as3711-backlight");
> > --
> > 1.7.2.5
> > 
> > --
> > 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
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* Re: [PATCH v16 RESEND 0/7] of: add display helper
From: Steffen Trumtrar @ 2013-01-25  8:04 UTC (permalink / raw)
  To: Mohammed, Afzal
  Cc: Leela Krishna Amudala, linux-fbdev@vger.kernel.org, David Airlie,
	devicetree-discuss@lists.ozlabs.org, Florian Tobias Schandinat,
	dri-devel@lists.freedesktop.org, Rob Clark, Valkeinen, Tomi,
	Laurent Pinchart, kernel@pengutronix.de, Guennady Liakhovetski,
	linux-media@vger.kernel.org
In-Reply-To: <C8443D0743D26F4388EA172BF4E2A7A93EA928F9@DBDE01.ent.ti.com>

Hi Afzal,

On Thu, Jan 24, 2013 at 08:47:02AM +0000, Mohammed, Afzal wrote:
> Hi Steffen,
> 
> On Thu, Jan 24, 2013 at 13:49:58, Steffen Trumtrar wrote:
> 
> > Thanks. I'll use that opportunity for a v17 that is rebased onto 3.8-rc4.
> 
> As you are going to have a v17, if you can fold the diff[1]
> (that I mentioned earlier) into the patch,
> "fbmon: add of_videomode helpers", it would be helpful. 
> 

I thought about it and I will not include that patch. Sorry.
In one of the previous versions of the series I had something like that and
it was suggested to remove it. If I leave it like it is, one gets a compile
time error like you do. And that is correct, because you shouldn't use the
function if you do not have of_videomode enabled. You should use one of the
underlying functions that are non-DT and called by of_get_fb_videomode.

Regards,
Steffen

> Regards
> Afzal
> 
> [1]
> 
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 58b9860..0ce30d1 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -716,9 +716,19 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
>  extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
>  extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
>  
> +#if defined(CONFIG_OF_VIDEOMODE) && defined(CONFIG_FB_MODE_HELPERS)
>  extern int of_get_fb_videomode(struct device_node *np,
>                                struct fb_videomode *fb,
>                                int index);
> +#else
> +static inline int of_get_fb_videomode(struct device_node *np,
> +                                     struct fb_videomode *fb,
> +                                     int index)
> +{
> +       return -EINVAL;
> +}
> +#endif
> +
>  extern int fb_videomode_from_videomode(const struct videomode *vm,
>                                        struct fb_videomode *fbmode);
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* [PATCH 1/3] fb: backlight: Add the Himax HX-8357B LCD controller
From: Maxime Ripard @ 2013-01-25  8:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1359104048-26823-1-git-send-email-maxime.ripard@free-electrons.com>

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 drivers/video/backlight/Kconfig  |    7 +
 drivers/video/backlight/Makefile |    1 +
 drivers/video/backlight/hx8357.c |  482 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 490 insertions(+)
 create mode 100644 drivers/video/backlight/hx8357.c

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 765a945..c39bed0 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -126,6 +126,13 @@ config LCD_AMS369FG06
 	  If you have an AMS369FG06 AMOLED Panel, say Y to enable its
 	  LCD control driver.
 
+config LCD_HX8357
+	tristate "Himax HX-8357 LCD Driver"
+	depends on SPI
+	help
+	  If you have a HX-8357 LCD panel, say Y to enable its LCD control
+	  driver.
+
 endif # LCD_CLASS_DEVICE
 
 #
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index e7ce729..b69d391 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_LCD_TOSA)		   += tosa_lcd.o
 obj-$(CONFIG_LCD_S6E63M0)	+= s6e63m0.o
 obj-$(CONFIG_LCD_LD9040)	+= ld9040.o
 obj-$(CONFIG_LCD_AMS369FG06)	+= ams369fg06.o
+obj-$(CONFIG_LCD_HX8357)	+= hx8357.o
 
 obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE) += backlight.o
 obj-$(CONFIG_BACKLIGHT_ATMEL_PWM)    += atmel-pwm-bl.o
diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
new file mode 100644
index 0000000..00925c0
--- /dev/null
+++ b/drivers/video/backlight/hx8357.c
@@ -0,0 +1,482 @@
+/*
+ * Driver for the Himax HX-8357 LCD Controller
+ *
+ * Copyright 2012 Free Electrons
+ *
+ * Licensed under the GPLv2 or later.
+ */
+
+#include <linux/delay.h>
+#include <linux/lcd.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/spi/spi.h>
+
+#define HX8357_NUM_IM_PINS	3
+
+#define HX8357_SWRESET			0x01
+#define HX8357_GET_RED_CHANNEL		0x06
+#define HX8357_GET_GREEN_CHANNEL	0x07
+#define HX8357_GET_BLUE_CHANNEL		0x08
+#define HX8357_GET_POWER_MODE		0x0a
+#define HX8357_GET_MADCTL		0x0b
+#define HX8357_GET_PIXEL_FORMAT		0x0c
+#define HX8357_GET_DISPLAY_MODE		0x0d
+#define HX8357_GET_SIGNAL_MODE		0x0e
+#define HX8357_GET_DIAGNOSTIC_RESULT	0x0f
+#define HX8357_ENTER_SLEEP_MODE		0x10
+#define HX8357_EXIT_SLEEP_MODE		0x11
+#define HX8357_ENTER_PARTIAL_MODE	0x12
+#define HX8357_ENTER_NORMAL_MODE	0x13
+#define HX8357_EXIT_INVERSION_MODE	0x20
+#define HX8357_ENTER_INVERSION_MODE	0x21
+#define HX8357_SET_DISPLAY_OFF		0x28
+#define HX8357_SET_DISPLAY_ON		0x29
+#define HX8357_SET_COLUMN_ADDRESS	0x2a
+#define HX8357_SET_PAGE_ADDRESS		0x2b
+#define HX8357_WRITE_MEMORY_START	0x2c
+#define HX8357_READ_MEMORY_START	0x2e
+#define HX8357_SET_PARTIAL_AREA		0x30
+#define HX8357_SET_SCROLL_AREA		0x33
+#define HX8357_SET_TEAR_OFF		0x34
+#define HX8357_SET_TEAR_ON		0x35
+#define HX8357_SET_ADDRESS_MODE		0x36
+#define HX8357_SET_SCROLL_START		0x37
+#define HX8357_EXIT_IDLE_MODE		0x38
+#define HX8357_ENTER_IDLE_MODE		0x39
+#define HX8357_SET_PIXEL_FORMAT		0x3a
+#define HX8357_SET_PIXEL_FORMAT_DBI_3BIT	(0x1)
+#define HX8357_SET_PIXEL_FORMAT_DBI_16BIT	(0x5)
+#define HX8357_SET_PIXEL_FORMAT_DBI_18BIT	(0x6)
+#define HX8357_SET_PIXEL_FORMAT_DPI_3BIT	(0x1 << 4)
+#define HX8357_SET_PIXEL_FORMAT_DPI_16BIT	(0x5 << 4)
+#define HX8357_SET_PIXEL_FORMAT_DPI_18BIT	(0x6 << 4)
+#define HX8357_WRITE_MEMORY_CONTINUE	0x3c
+#define HX8357_READ_MEMORY_CONTINUE	0x3e
+#define HX8357_SET_TEAR_SCAN_LINES	0x44
+#define HX8357_GET_SCAN_LINES		0x45
+#define HX8357_READ_DDB_START		0xa1
+#define HX8357_SET_DISPLAY_MODE		0xb4
+#define HX8357_SET_DISPLAY_MODE_RGB_THROUGH	(0x3)
+#define HX8357_SET_DISPLAY_MODE_RGB_INTERFACE	(1 << 4)
+#define HX8357_SET_PANEL_DRIVING	0xc0
+#define HX8357_SET_DISPLAY_FRAME	0xc5
+#define HX8357_SET_RGB			0xc6
+#define HX8357_SET_RGB_ENABLE_HIGH		(1 << 1)
+#define HX8357_SET_GAMMA		0xc8
+#define HX8357_SET_POWER		0xd0
+#define HX8357_SET_VCOM			0xd1
+#define HX8357_SET_POWER_NORMAL		0xd2
+#define HX8357_SET_PANEL_RELATED	0xe9
+
+struct hx8357_data {
+	unsigned		im_pins[HX8357_NUM_IM_PINS];
+	unsigned		reset;
+	struct spi_device	*spi;
+	int			state;
+};
+
+static int hx8357_spi_write_then_read(struct lcd_device *lcdev,
+				void *txbuf, u16 txlen,
+				void *rxbuf, u16 rxlen)
+{
+	struct hx8357_data *lcd = lcd_get_data(lcdev);
+	struct spi_message msg;
+	struct spi_transfer xfer[2];
+	u16 *local_txbuf = NULL;
+	int ret = 0;
+
+	memset(xfer, 0, sizeof(xfer));
+	spi_message_init(&msg);
+
+	if (txlen) {
+		int i;
+
+		local_txbuf = kcalloc(sizeof(*local_txbuf), txlen, GFP_KERNEL);
+
+		if (!local_txbuf) {
+			dev_err(&lcdev->dev, "Couldn't allocate data buffer.\n");
+			return -ENOMEM;
+		}
+
+		for (i = 0; i < txlen; i++) {
+			local_txbuf[i] = ((u8 *)txbuf)[i];
+			if (i > 0)
+				local_txbuf[i] |= 1 << 8;
+		}
+
+		xfer[0].len = 2 * txlen;
+		xfer[0].bits_per_word = 9;
+		xfer[0].tx_buf = local_txbuf;
+		spi_message_add_tail(&xfer[0], &msg);
+	}
+
+	if (rxlen) {
+		xfer[1].len = rxlen;
+		xfer[1].bits_per_word = 8;
+		xfer[1].rx_buf = rxbuf;
+		spi_message_add_tail(&xfer[1], &msg);
+	}
+
+	ret = spi_sync(lcd->spi, &msg);
+	if (ret < 0)
+		dev_err(&lcdev->dev, "Couldn't send SPI data.\n");
+
+	if (txlen)
+		kfree(local_txbuf);
+
+	return ret;
+}
+
+static inline int hx8357_spi_write_array(struct lcd_device *lcdev,
+					u8 *value, u8 len)
+{
+	return hx8357_spi_write_then_read(lcdev, value, len, NULL, 0);
+}
+
+static inline int hx8357_spi_write_byte(struct lcd_device *lcdev,
+					u8 value)
+{
+	return hx8357_spi_write_then_read(lcdev, &value, 1, NULL, 0);
+}
+
+static int hx8357_enter_standby(struct lcd_device *lcdev)
+{
+	int ret;
+
+	ret = hx8357_spi_write_byte(lcdev, HX8357_SET_DISPLAY_OFF);
+	if (ret < 0)
+		return ret;
+
+	usleep_range(10000, 12000);
+
+	ret = hx8357_spi_write_byte(lcdev, HX8357_ENTER_SLEEP_MODE);
+	if (ret < 0)
+		return ret;
+
+	msleep(120);
+
+	return 0;
+}
+
+static int hx8357_exit_standby(struct lcd_device *lcdev)
+{
+	int ret;
+
+	ret = hx8357_spi_write_byte(lcdev, HX8357_EXIT_SLEEP_MODE);
+	if (ret < 0)
+		return ret;
+
+	msleep(120);
+
+	ret = hx8357_spi_write_byte(lcdev, HX8357_SET_DISPLAY_ON);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int hx8357_lcd_init(struct lcd_device *lcdev)
+{
+	struct hx8357_data *lcd = lcd_get_data(lcdev);
+	u8 buf[16];
+	int ret;
+
+	/*
+	 * Set the interface selection pins to SPI mode, with three
+	 * wires
+	 */
+	gpio_set_value_cansleep(lcd->im_pins[0], 1);
+	gpio_set_value_cansleep(lcd->im_pins[1], 0);
+	gpio_set_value_cansleep(lcd->im_pins[2], 1);
+
+	/* Reset the screen */
+	gpio_set_value(lcd->reset, 1);
+	usleep_range(10000, 12000);
+	gpio_set_value(lcd->reset, 0);
+	usleep_range(10000, 12000);
+	gpio_set_value(lcd->reset, 1);
+	msleep(120);
+
+	buf[0] = HX8357_SET_POWER;
+	buf[1] = 0x44;
+	buf[2] = 0x41;
+	buf[3] = 0x06;
+	ret = hx8357_spi_write_array(lcdev, buf, 4);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = HX8357_SET_VCOM;
+	buf[1] = 0x40;
+	buf[2] = 0x10;
+	ret = hx8357_spi_write_array(lcdev, buf, 3);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = HX8357_SET_POWER_NORMAL;
+	buf[1] = 0x05;
+	buf[2] = 0x12;
+	ret = hx8357_spi_write_array(lcdev, buf, 3);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = HX8357_SET_PANEL_DRIVING;
+	buf[1] = 0x14;
+	buf[2] = 0x3b;
+	buf[3] = 0x00;
+	buf[4] = 0x02;
+	buf[5] = 0x11;
+	ret = hx8357_spi_write_array(lcdev, buf, 6);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = HX8357_SET_DISPLAY_FRAME;
+	buf[1] = 0x0c;
+	ret = hx8357_spi_write_array(lcdev, buf, 2);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = HX8357_SET_PANEL_RELATED;
+	buf[1] = 0x01;
+	ret = hx8357_spi_write_array(lcdev, buf, 2);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = 0xea;
+	buf[1] = 0x03;
+	buf[2] = 0x00;
+	buf[3] = 0x00;
+	ret = hx8357_spi_write_array(lcdev, buf, 4);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = 0xeb;
+	buf[1] = 0x40;
+	buf[2] = 0x54;
+	buf[3] = 0x26;
+	buf[4] = 0xdb;
+	ret = hx8357_spi_write_array(lcdev, buf, 5);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = HX8357_SET_GAMMA;
+	buf[1] = 0x00;
+	buf[2] = 0x15;
+	buf[3] = 0x00;
+	buf[4] = 0x22;
+	buf[5] = 0x00;
+	buf[6] = 0x08;
+	buf[7] = 0x77;
+	buf[8] = 0x26;
+	buf[9] = 0x77;
+	buf[10] = 0x22;
+	buf[11] = 0x04;
+	buf[12] = 0x00;
+	ret = hx8357_spi_write_array(lcdev, buf, 13);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = HX8357_SET_ADDRESS_MODE;
+	buf[1] = 0xc0;
+	ret = hx8357_spi_write_array(lcdev, buf, 2);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = HX8357_SET_PIXEL_FORMAT;
+	buf[1] = HX8357_SET_PIXEL_FORMAT_DPI_18BIT |
+		HX8357_SET_PIXEL_FORMAT_DBI_18BIT;
+	ret = hx8357_spi_write_array(lcdev, buf, 2);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = HX8357_SET_COLUMN_ADDRESS;
+	buf[1] = 0x00;
+	buf[2] = 0x00;
+	buf[3] = 0x01;
+	buf[4] = 0x3f;
+	ret = hx8357_spi_write_array(lcdev, buf, 5);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = HX8357_SET_PAGE_ADDRESS;
+	buf[1] = 0x00;
+	buf[2] = 0x00;
+	buf[3] = 0x01;
+	buf[4] = 0xdf;
+	ret = hx8357_spi_write_array(lcdev, buf, 5);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = HX8357_SET_RGB;
+	buf[1] = 0x02;
+	ret = hx8357_spi_write_array(lcdev, buf, 2);
+	if (ret < 0)
+		return ret;
+
+	buf[0] = HX8357_SET_DISPLAY_MODE;
+	buf[1] = HX8357_SET_DISPLAY_MODE_RGB_THROUGH |
+		HX8357_SET_DISPLAY_MODE_RGB_INTERFACE;
+	ret = hx8357_spi_write_array(lcdev, buf, 2);
+	if (ret < 0)
+		return ret;
+
+	ret = hx8357_spi_write_byte(lcdev, HX8357_EXIT_SLEEP_MODE);
+	if (ret < 0)
+		return ret;
+
+	msleep(120);
+
+	ret = hx8357_spi_write_byte(lcdev, HX8357_SET_DISPLAY_ON);
+	if (ret < 0)
+		return ret;
+
+	usleep_range(5000, 7000);
+
+	ret = hx8357_spi_write_byte(lcdev, HX8357_WRITE_MEMORY_START);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+#define POWER_IS_ON(pwr)	((pwr) <= FB_BLANK_NORMAL)
+
+static int hx8357_set_power(struct lcd_device *lcdev, int power)
+{
+	struct hx8357_data *lcd = lcd_get_data(lcdev);
+	int ret = 0;
+
+	if (POWER_IS_ON(power) && !POWER_IS_ON(lcd->state))
+		ret = hx8357_exit_standby(lcdev);
+	else if (!POWER_IS_ON(power) && POWER_IS_ON(lcd->state))
+		ret = hx8357_enter_standby(lcdev);
+
+	if (ret = 0)
+		lcd->state = power;
+	else
+		dev_warn(&lcdev->dev, "failed to set power mode %d\n", power);
+
+	return ret;
+}
+
+static int hx8357_get_power(struct lcd_device *lcdev)
+{
+	struct hx8357_data *lcd = lcd_get_data(lcdev);
+
+	return lcd->state;
+}
+
+static struct lcd_ops hx8357_ops = {
+	.set_power	= hx8357_set_power,
+	.get_power	= hx8357_get_power,
+};
+
+static int hx8357_probe(struct spi_device *spi)
+{
+	struct lcd_device *lcdev;
+	struct hx8357_data *lcd;
+	int i, ret;
+
+	lcd = devm_kzalloc(&spi->dev, sizeof(*lcd), GFP_KERNEL);
+	if (!lcd) {
+		dev_err(&spi->dev, "Couldn't allocate lcd internal structure!\n");
+		return -ENOMEM;
+	}
+
+	ret = spi_setup(spi);
+	if (ret < 0) {
+		dev_err(&spi->dev, "SPI setup failed.\n");
+		return ret;
+	}
+
+	lcd->spi = spi;
+
+	lcd->reset = of_get_named_gpio(spi->dev.of_node, "gpios-reset", 0);
+	if (!gpio_is_valid(lcd->reset)) {
+		dev_err(&spi->dev, "Missing dt property: gpios-reset\n");
+		return -EINVAL;
+	}
+
+	ret = devm_gpio_request_one(&spi->dev, lcd->reset,
+				    GPIOF_OUT_INIT_HIGH,
+				    "hx8357-reset");
+	if (ret) {
+		dev_err(&spi->dev,
+			"failed to request gpio %d: %d\n",
+			lcd->reset, ret);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < HX8357_NUM_IM_PINS; i++) {
+		lcd->im_pins[i] = of_get_named_gpio(spi->dev.of_node,
+						"im-gpios", i);
+		if (lcd->im_pins[i] = -EPROBE_DEFER) {
+			dev_info(&spi->dev, "GPIO requested is not here yet, deferring the probe\n");
+			return -EPROBE_DEFER;
+		}
+		if (!gpio_is_valid(lcd->im_pins[i])) {
+			dev_err(&spi->dev, "Missing dt property: im-gpios\n");
+			return -EINVAL;
+		}
+
+		ret = devm_gpio_request_one(&spi->dev, lcd->im_pins[i],
+					GPIOF_OUT_INIT_LOW, "im_pins");
+		if (ret) {
+			dev_err(&spi->dev, "failed to request gpio %d: %d\n",
+				lcd->im_pins[i], ret);
+			return -EINVAL;
+		}
+	}
+
+	lcdev = lcd_device_register("mxsfb", &spi->dev, lcd, &hx8357_ops);
+	if (IS_ERR(lcdev)) {
+		ret = PTR_ERR(lcdev);
+		return ret;
+	}
+	spi_set_drvdata(spi, lcdev);
+
+	ret = hx8357_lcd_init(lcdev);
+	if (ret) {
+		dev_err(&spi->dev, "Couldn't initialize panel\n");
+		goto init_error;
+	}
+
+	dev_info(&spi->dev, "Panel probed\n");
+
+	return 0;
+
+init_error:
+	lcd_device_unregister(lcdev);
+	return ret;
+}
+
+static int hx8357_remove(struct spi_device *spi)
+{
+	struct lcd_device *lcdev = spi_get_drvdata(spi);
+
+	lcd_device_unregister(lcdev);
+	return 0;
+}
+
+static const struct of_device_id hx8357_dt_ids[] = {
+	{ .compatible = "himax,hx8357" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, hx8357_dt_ids);
+
+static struct spi_driver hx8357_driver = {
+	.probe  = hx8357_probe,
+	.remove = hx8357_remove,
+	.driver = {
+		.name = "hx8357",
+		.of_match_table = of_match_ptr(hx8357_dt_ids),
+	},
+};
+
+module_spi_driver(hx8357_driver);
+
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
+MODULE_DESCRIPTION("Himax HX-8357 LCD Driver");
+MODULE_LICENSE("GPL");
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH v17 0/7] of: add display helper
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Hi!

This is basically just a resend of v16 that was rebased onto v3.8-rc4 and has
two new "tested-by"s from Rob and Leela.

The patches were tested with:

        - v15 on Tegra by Thierry
        - sh-mobile-lcdcfb by Laurent
        - MX53QSB by Marek
        - Exynos: smdk5250 by Leela
        - AM335X EVM & AM335X EVM-SK by Afzal
	- tilcdc lcd-panel by Rob
        - imx6q: sabrelite, sabresd by Philipp and me
        - imx53: tqma53/mba53 by me


Changes since v16:
	- rebased from 3.7 to 3.8-rc4

Changes since v15:
        - move include/linux/{videomode,display_timing}.h to include/video
        - move include/linux/of_{videomode,display_timing}.h to include/video
        - reimplement flags: add VESA flags and data flags
        - let pixelclock in struct videomode be unsigned long
        - rename of_display_timings_exists to of_display_timings_exist
        - revise logging/error messages: replace __func__ with np->full_name
        - rename pixelclk-inverted to pixelclk-active
        - revise comments in code

Changes since v14:
        - fix "const struct *" warning
                (reported by: Leela Krishna Amudala <l.krishna@samsung.com>)
        - return -EINVAL when htotal or vtotal are zero
        - remove unreachable code in of_get_display_timings
        - include headers in .c files and not implicit in .h
        - sort includes alphabetically
        - fix lower/uppercase in binding documentation
        - rebase onto v3.7-rc7

Changes since v13:
        - fix "const struct *" warning
                (reported by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>)
        - prevent division by zero in fb_videomode_from_videomode

Changes since v12:
        - rename struct display_timing to via_display_timing in via subsystem
        - fix refreshrate calculation
        - fix "const struct *" warnings
                (reported by: Manjunathappa, Prakash <prakash.pm@ti.com>)
        - some CodingStyle fixes
        - rewrite parts of commit messages and display-timings.txt
        - let display_timing_get_value get all values instead of just typical

Changes since v11:
        - make pointers const where applicable
        - add reviewed-by Laurent Pinchart

Changes since v10:
        - fix function name (drm_)display_mode_from_videomode
        - add acked-by, reviewed-by, tested-by

Changes since v9:
        - don't leak memory when previous timings were correct
        - CodingStyle fixes
        - move blank lines around

Changes since v8:
        - fix memory leaks
        - change API to be more consistent (foo_from_bar(struct bar, struct foo))
        - include headers were necessary
        - misc minor bugfixes

Changes since v7:
        - move of_xxx to drivers/video
        - remove non-binding documentation from display-timings.txt
        - squash display_timings and videomode in one patch
        - misc minor fixes

Changes since v6:
        - get rid of some empty lines etc.
        - move functions to their subsystems
        - split of_ from non-of_ functions
        - add at least some kerneldoc to some functions

Changes since v5:
        - removed all display stuff and just describe timings

Changes since v4:
        - refactored functions

Changes since v3:
        - print error messages
        - free alloced memory
        - general cleanup

Changes since v2:
        - use hardware-near property-names
        - provide a videomode structure
        - allow ranges for all properties (<min,typ,max>)
        - functions to get display_mode or fb_videomode


Regards
Steffen

Steffen Trumtrar (7):
  viafb: rename display_timing to via_display_timing
  video: add display_timing and videomode
  video: add of helper for display timings/videomode
  fbmon: add videomode helpers
  fbmon: add of_videomode helpers
  drm_modes: add videomode helpers
  drm_modes: add of_videomode helpers

 .../devicetree/bindings/video/display-timing.txt   |  109 +++++++++
 drivers/gpu/drm/drm_modes.c                        |   70 ++++++
 drivers/video/Kconfig                              |   21 ++
 drivers/video/Makefile                             |    4 +
 drivers/video/display_timing.c                     |   24 ++
 drivers/video/fbmon.c                              |   94 ++++++++
 drivers/video/of_display_timing.c                  |  239 ++++++++++++++++++++
 drivers/video/of_videomode.c                       |   54 +++++
 drivers/video/via/hw.c                             |    6 +-
 drivers/video/via/hw.h                             |    2 +-
 drivers/video/via/lcd.c                            |    2 +-
 drivers/video/via/share.h                          |    2 +-
 drivers/video/via/via_modesetting.c                |    8 +-
 drivers/video/via/via_modesetting.h                |    6 +-
 drivers/video/videomode.c                          |   39 ++++
 include/drm/drmP.h                                 |    9 +
 include/linux/fb.h                                 |    8 +
 include/video/display_timing.h                     |  124 ++++++++++
 include/video/of_display_timing.h                  |   20 ++
 include/video/of_videomode.h                       |   18 ++
 include/video/videomode.h                          |   48 ++++
 21 files changed, 894 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/display-timing.txt
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/video/display_timing.h
 create mode 100644 include/video/of_display_timing.h
 create mode 100644 include/video/of_videomode.h
 create mode 100644 include/video/videomode.h

-- 
1.7.10.4


^ permalink raw reply

* [PATCH v17 1/7] viafb: rename display_timing to via_display_timing
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel
In-Reply-To: <1359104515-8907-1-git-send-email-s.trumtrar@pengutronix.de>

The struct display_timing is specific to the via subsystem. The naming leads to
collisions with the new struct display_timing, which is supposed to be a shared
struct between different subsystems.
To clean this up, prepend the existing struct with the subsystem it is specific
to.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/video/via/hw.c              |    6 +++---
 drivers/video/via/hw.h              |    2 +-
 drivers/video/via/lcd.c             |    2 +-
 drivers/video/via/share.h           |    2 +-
 drivers/video/via/via_modesetting.c |    8 ++++----
 drivers/video/via/via_modesetting.h |    6 +++---
 6 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index 80233da..22450908 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -1467,10 +1467,10 @@ void viafb_set_vclock(u32 clk, int set_iga)
 	via_write_misc_reg_mask(0x0C, 0x0C); /* select external clock */
 }
 
-struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
+struct via_display_timing var_to_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres)
 {
-	struct display_timing timing;
+	struct via_display_timing timing;
 	u16 dx = (var->xres - cxres) / 2, dy = (var->yres - cyres) / 2;
 
 	timing.hor_addr = cxres;
@@ -1491,7 +1491,7 @@ struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
 void viafb_fill_crtc_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres, int iga)
 {
-	struct display_timing crt_reg = var_to_timing(var,
+	struct via_display_timing crt_reg = var_to_timing(var,
 		cxres ? cxres : var->xres, cyres ? cyres : var->yres);
 
 	if (iga = IGA1)
diff --git a/drivers/video/via/hw.h b/drivers/video/via/hw.h
index a820575..3be073c 100644
--- a/drivers/video/via/hw.h
+++ b/drivers/video/via/hw.h
@@ -637,7 +637,7 @@ extern int viafb_LCD_ON;
 extern int viafb_DVI_ON;
 extern int viafb_hotplug;
 
-struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
+struct via_display_timing var_to_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres);
 void viafb_fill_crtc_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres, int iga);
diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
index 980ee1b..5d21ff4 100644
--- a/drivers/video/via/lcd.c
+++ b/drivers/video/via/lcd.c
@@ -549,7 +549,7 @@ void viafb_lcd_set_mode(const struct fb_var_screeninfo *var, u16 cxres,
 	int panel_hres = plvds_setting_info->lcd_panel_hres;
 	int panel_vres = plvds_setting_info->lcd_panel_vres;
 	u32 clock;
-	struct display_timing timing;
+	struct via_display_timing timing;
 	struct fb_var_screeninfo panel_var;
 	const struct fb_videomode *mode_crt_table, *panel_crt_table;
 
diff --git a/drivers/video/via/share.h b/drivers/video/via/share.h
index 3158dfc..65c65c6 100644
--- a/drivers/video/via/share.h
+++ b/drivers/video/via/share.h
@@ -319,7 +319,7 @@ struct crt_mode_table {
 	int refresh_rate;
 	int h_sync_polarity;
 	int v_sync_polarity;
-	struct display_timing crtc;
+	struct via_display_timing crtc;
 };
 
 struct io_reg {
diff --git a/drivers/video/via/via_modesetting.c b/drivers/video/via/via_modesetting.c
index 0e431ae..0b414b0 100644
--- a/drivers/video/via/via_modesetting.c
+++ b/drivers/video/via/via_modesetting.c
@@ -30,9 +30,9 @@
 #include "debug.h"
 
 
-void via_set_primary_timing(const struct display_timing *timing)
+void via_set_primary_timing(const struct via_display_timing *timing)
 {
-	struct display_timing raw;
+	struct via_display_timing raw;
 
 	raw.hor_total = timing->hor_total / 8 - 5;
 	raw.hor_addr = timing->hor_addr / 8 - 1;
@@ -88,9 +88,9 @@ void via_set_primary_timing(const struct display_timing *timing)
 	via_write_reg_mask(VIACR, 0x17, 0x80, 0x80);
 }
 
-void via_set_secondary_timing(const struct display_timing *timing)
+void via_set_secondary_timing(const struct via_display_timing *timing)
 {
-	struct display_timing raw;
+	struct via_display_timing raw;
 
 	raw.hor_total = timing->hor_total - 1;
 	raw.hor_addr = timing->hor_addr - 1;
diff --git a/drivers/video/via/via_modesetting.h b/drivers/video/via/via_modesetting.h
index 06e09fe..f6a6503 100644
--- a/drivers/video/via/via_modesetting.h
+++ b/drivers/video/via/via_modesetting.h
@@ -33,7 +33,7 @@
 #define VIA_PITCH_MAX	0x3FF8
 
 
-struct display_timing {
+struct via_display_timing {
 	u16 hor_total;
 	u16 hor_addr;
 	u16 hor_blank_start;
@@ -49,8 +49,8 @@ struct display_timing {
 };
 
 
-void via_set_primary_timing(const struct display_timing *timing);
-void via_set_secondary_timing(const struct display_timing *timing);
+void via_set_primary_timing(const struct via_display_timing *timing);
+void via_set_secondary_timing(const struct via_display_timing *timing);
 void via_set_primary_address(u32 addr);
 void via_set_secondary_address(u32 addr);
 void via_set_primary_pitch(u32 pitch);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH v17 2/7] video: add display_timing and videomode
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel
In-Reply-To: <1359104515-8907-1-git-send-email-s.trumtrar@pengutronix.de>

Add display_timing structure and the according helper functions. This allows
the description of a display via its supported timing parameters.

Also, add helper functions to convert from display timings to a generic videomode
structure.

The struct display_timing specifies all needed parameters to describe the signal
properties of a display in one mode. This includes
    - ranges for signals that may have min-, max- and typical values
    - single integers for signals that can be on, off or are ignored
    - booleans for signals that are either on or off

As a display may support multiple modes like this, a struct display_timings is
added, that holds all given struct display_timing pointers and declares the
native mode of the display.

Although a display may state that a signal can be in a range, it is driven with
fixed values that indicate a videomode. Therefore graphic drivers don't need all
the information of struct display_timing, but would generate a videomode from
the given set of supported signal timings and work with that.

The video subsystems all define their own structs that describe a mode and work
with that (e.g. fb_videomode or drm_display_mode). To slowly replace all those
various structures and allow code reuse across those subsystems, add struct
videomode as a generic description.

This patch only includes the most basic fields in struct videomode. All missing
fields that are needed to have a really generic video mode description can be
added at a later stage.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 drivers/video/Kconfig          |    6 ++
 drivers/video/Makefile         |    2 +
 drivers/video/display_timing.c |   24 ++++++++
 drivers/video/videomode.c      |   39 +++++++++++++
 include/video/display_timing.h |  124 ++++++++++++++++++++++++++++++++++++++++
 include/video/videomode.h      |   48 ++++++++++++++++
 6 files changed, 243 insertions(+)
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/video/display_timing.h
 create mode 100644 include/video/videomode.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index e7068c5..09a8f0d 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -33,6 +33,12 @@ config VIDEO_OUTPUT_CONTROL
 	  This framework adds support for low-level control of the video 
 	  output switch.
 
+config DISPLAY_TIMING
+       bool
+
+config VIDEOMODE
+       bool
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 768a137..e0dd820 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -168,3 +168,5 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
+obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_VIDEOMODE) += videomode.o
diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
new file mode 100644
index 0000000..5e1822c
--- /dev/null
+++ b/drivers/video/display_timing.c
@@ -0,0 +1,24 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/export.h>
+#include <linux/slab.h>
+#include <video/display_timing.h>
+
+void display_timings_release(struct display_timings *disp)
+{
+	if (disp->timings) {
+		unsigned int i;
+
+		for (i = 0; i < disp->num_timings; i++)
+			kfree(disp->timings[i]);
+		kfree(disp->timings);
+	}
+	kfree(disp);
+}
+EXPORT_SYMBOL_GPL(display_timings_release);
diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
new file mode 100644
index 0000000..21c47a2
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,39 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <video/display_timing.h>
+#include <video/videomode.h>
+
+int videomode_from_timing(const struct display_timings *disp,
+			  struct videomode *vm, unsigned int index)
+{
+	struct display_timing *dt;
+
+	dt = display_timings_get(disp, index);
+	if (!dt)
+		return -EINVAL;
+
+	vm->pixelclock = display_timing_get_value(&dt->pixelclock, TE_TYP);
+	vm->hactive = display_timing_get_value(&dt->hactive, TE_TYP);
+	vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, TE_TYP);
+	vm->hback_porch = display_timing_get_value(&dt->hback_porch, TE_TYP);
+	vm->hsync_len = display_timing_get_value(&dt->hsync_len, TE_TYP);
+
+	vm->vactive = display_timing_get_value(&dt->vactive, TE_TYP);
+	vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, TE_TYP);
+	vm->vback_porch = display_timing_get_value(&dt->vback_porch, TE_TYP);
+	vm->vsync_len = display_timing_get_value(&dt->vsync_len, TE_TYP);
+
+	vm->dmt_flags = dt->dmt_flags;
+	vm->data_flags = dt->data_flags;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_from_timing);
diff --git a/include/video/display_timing.h b/include/video/display_timing.h
new file mode 100644
index 0000000..71e9a38
--- /dev/null
+++ b/include/video/display_timing.h
@@ -0,0 +1,124 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * description of display timings
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_DISPLAY_TIMING_H
+#define __LINUX_DISPLAY_TIMING_H
+
+#include <linux/bitops.h>
+#include <linux/types.h>
+
+/* VESA display monitor timing parameters */
+#define VESA_DMT_HSYNC_LOW		BIT(0)
+#define VESA_DMT_HSYNC_HIGH		BIT(1)
+#define VESA_DMT_VSYNC_LOW		BIT(2)
+#define VESA_DMT_VSYNC_HIGH		BIT(3)
+
+/* display specific flags */
+#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
+#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
+#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
+#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
+#define DISPLAY_FLAGS_INTERLACED	BIT(4)
+#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
+
+/*
+ * A single signal can be specified via a range of minimal and maximal values
+ * with a typical value, that lies somewhere inbetween.
+ */
+struct timing_entry {
+	u32 min;
+	u32 typ;
+	u32 max;
+};
+
+enum timing_entry_index {
+	TE_MIN = 0,
+	TE_TYP = 1,
+	TE_MAX = 2,
+};
+
+/*
+ * Single "mode" entry. This describes one set of signal timings a display can
+ * have in one setting. This struct can later be converted to struct videomode
+ * (see include/video/videomode.h). As each timing_entry can be defined as a
+ * range, one struct display_timing may become multiple struct videomodes.
+ *
+ * Example: hsync active high, vsync active low
+ *
+ *				    Active Video
+ * Video  ______________________XXXXXXXXXXXXXXXXXXXXXX_____________________
+ *	  |<- sync ->|<- back ->|<----- active ----->|<- front ->|<- sync..
+ *	  |	     |	 porch  |		     |	 porch	 |
+ *
+ * HSync _|¯¯¯¯¯¯¯¯¯¯|___________________________________________|¯¯¯¯¯¯¯¯¯
+ *
+ * VSync ¯|__________|¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯|_________
+ */
+struct display_timing {
+	struct timing_entry pixelclock;
+
+	struct timing_entry hactive;		/* hor. active video */
+	struct timing_entry hfront_porch;	/* hor. front porch */
+	struct timing_entry hback_porch;	/* hor. back porch */
+	struct timing_entry hsync_len;		/* hor. sync len */
+
+	struct timing_entry vactive;		/* ver. active video */
+	struct timing_entry vfront_porch;	/* ver. front porch */
+	struct timing_entry vback_porch;	/* ver. back porch */
+	struct timing_entry vsync_len;		/* ver. sync len */
+
+	unsigned int dmt_flags;			/* VESA DMT flags */
+	unsigned int data_flags;		/* video data flags */
+};
+
+/*
+ * This describes all timing settings a display provides.
+ * The native_mode is the default setting for this display.
+ * Drivers that can handle multiple videomodes should work with this struct and
+ * convert each entry to the desired end result.
+ */
+struct display_timings {
+	unsigned int num_timings;
+	unsigned int native_mode;
+
+	struct display_timing **timings;
+};
+
+/* get value specified by index from struct timing_entry */
+static inline u32 display_timing_get_value(const struct timing_entry *te,
+					   enum timing_entry_index index)
+{
+	switch (index) {
+	case TE_MIN:
+		return te->min;
+		break;
+	case TE_TYP:
+		return te->typ;
+		break;
+	case TE_MAX:
+		return te->max;
+		break;
+	default:
+		return te->typ;
+	}
+}
+
+/* get one entry from struct display_timings */
+static inline struct display_timing *display_timings_get(const struct
+							 display_timings *disp,
+							 unsigned int index)
+{
+	if (disp->num_timings > index)
+		return disp->timings[index];
+	else
+		return NULL;
+}
+
+void display_timings_release(struct display_timings *disp);
+
+#endif
diff --git a/include/video/videomode.h b/include/video/videomode.h
new file mode 100644
index 0000000..a421562
--- /dev/null
+++ b/include/video/videomode.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * generic videomode description
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_VIDEOMODE_H
+#define __LINUX_VIDEOMODE_H
+
+#include <linux/types.h>
+#include <video/display_timing.h>
+
+/*
+ * Subsystem independent description of a videomode.
+ * Can be generated from struct display_timing.
+ */
+struct videomode {
+	unsigned long pixelclock;	/* pixelclock in Hz */
+
+	u32 hactive;
+	u32 hfront_porch;
+	u32 hback_porch;
+	u32 hsync_len;
+
+	u32 vactive;
+	u32 vfront_porch;
+	u32 vback_porch;
+	u32 vsync_len;
+
+	unsigned int dmt_flags;	/* VESA DMT flags */
+	unsigned int data_flags; /* video data flags */
+};
+
+/**
+ * videomode_from_timing - convert display timing to videomode
+ * @disp: structure with all possible timing entries
+ * @vm: return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function converts a struct display_timing to a struct videomode.
+ */
+int videomode_from_timing(const struct display_timings *disp,
+			  struct videomode *vm, unsigned int index);
+
+#endif
-- 
1.7.10.4


^ permalink raw reply related

* =?UTF-8?q?=5BPATCH=20v17=203/7=5D=20video=3A=20add=20of=20helper=20for=20display=20timings/videomode
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Philipp Zabel, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Tomi Valkeinen,
	Stephen Warren, Florian Tobias Schandinat, Rob Clark,
	Leela Krishna Amudala, Mohammed, Afzal, kernel
In-Reply-To: <1359104515-8907-1-git-send-email-s.trumtrar@pengutronix.de>

This adds support for reading display timings from DT into a struct
display_timings. The of_display_timing implementation supports multiple
subnodes. All children are read into an array, that can be queried.

If no native mode is specified, the first subnode will be used.

For cases where the graphics driver knows there can be only one
mode description or where the driver only supports one mode, a helper
function of_get_videomode is added, that gets a struct videomode from DT.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 .../devicetree/bindings/video/display-timing.txt   |  109 +++++++++
 drivers/video/Kconfig                              |   15 ++
 drivers/video/Makefile                             |    2 +
 drivers/video/of_display_timing.c                  |  239 ++++++++++++++++++++
 drivers/video/of_videomode.c                       |   54 +++++
 include/video/of_display_timing.h                  |   20 ++
 include/video/of_videomode.h                       |   18 ++
 7 files changed, 457 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display-timing.txt
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 include/video/of_display_timing.h
 create mode 100644 include/video/of_videomode.h

diff --git a/Documentation/devicetree/bindings/video/display-timing.txt b/Documentation/devicetree/bindings/video/display-timing.txt
new file mode 100644
index 0000000..1500385
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display-timing.txt
@@ -0,0 +1,109 @@
+display-timing bindings
+===========+
+display-timings node
+--------------------
+
+required properties:
+ - none
+
+optional properties:
+ - native-mode: The native mode for the display, in case multiple modes are
+		provided. When omitted, assume the first node is the native.
+
+timing subnode
+--------------
+
+required properties:
+ - hactive, vactive: display resolution
+ - hfront-porch, hback-porch, hsync-len: horizontal display timing parameters
+   in pixels
+   vfront-porch, vback-porch, vsync-len: vertical display timing parameters in
+   lines
+ - clock-frequency: display clock in Hz
+
+optional properties:
+ - hsync-active: hsync pulse is active low/high/ignored
+ - vsync-active: vsync pulse is active low/high/ignored
+ - de-active: data-enable pulse is active low/high/ignored
+ - pixelclk-active: with
+			- active high = drive pixel data on rising edge/
+					sample data on falling edge
+			- active low  = drive pixel data on falling edge/
+					sample data on rising edge
+			- ignored     = ignored
+ - interlaced (bool): boolean to enable interlaced mode
+ - doublescan (bool): boolean to enable doublescan mode
+
+All the optional properties that are not bool follow the following logic:
+    <1>: high active
+    <0>: low active
+    omitted: not used on hardware
+
+There are different ways of describing the capabilities of a display. The
+devicetree representation corresponds to the one commonly found in datasheets
+for displays. If a display supports multiple signal timings, the native-mode
+can be specified.
+
+The parameters are defined as:
+
+  +----------+-------------------------------------+----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vback_porch                 |          |       |
+  |          |        ↓                            |          |       |
+  +----------#######################################----------+-------+
+  |          #        ↑                            #          |       |
+  |          #        |                            #          |       |
+  |  hback   #        |                            #  hfront  | hsync |
+  |   porch  #        |       hactive              #  porch   |  len  |
+  |<-------->#<-------+--------------------------->#<-------->|<----->|
+  |          #        |                            #          |       |
+  |          #        |vactive                     #          |       |
+  |          #        |                            #          |       |
+  |          #        ↓                            #          |       |
+  +----------#######################################----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vfront_porch                |          |       |
+  |          |        ↓                            |          |       |
+  +----------+-------------------------------------+----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vsync_len                   |          |       |
+  |          |        ↓                            |          |       |
+  +----------+-------------------------------------+----------+-------+
+
+Example:
+
+	display-timings {
+		native-mode = <&timing0>;
+		timing0: 1080p24 {
+			/* 1920x1080p24 */
+			clock-frequency = <52000000>;
+			hactive = <1920>;
+			vactive = <1080>;
+			hfront-porch = <25>;
+			hback-porch = <25>;
+			hsync-len = <25>;
+			vback-porch = <2>;
+			vfront-porch = <2>;
+			vsync-len = <2>;
+			hsync-active = <1>;
+		};
+	};
+
+Every required property also supports the use of ranges, so the commonly used
+datasheet description with minimum, typical and maximum values can be used.
+
+Example:
+
+	timing1: timing {
+		/* 1920x1080p24 */
+		clock-frequency = <148500000>;
+		hactive = <1920>;
+		vactive = <1080>;
+		hsync-len = <0 44 60>;
+		hfront-porch = <80 88 95>;
+		hback-porch = <100 148 160>;
+		vfront-porch = <0 4 6>;
+		vback-porch = <0 36 50>;
+		vsync-len = <0 5 6>;
+	};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 09a8f0d..807c7fa 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -39,6 +39,21 @@ config DISPLAY_TIMING
 config VIDEOMODE
        bool
 
+config OF_DISPLAY_TIMING
+	bool "Enable device tree display timing support"
+	depends on OF
+	select DISPLAY_TIMING
+	help
+	  helper to parse display timings from the devicetree
+
+config OF_VIDEOMODE
+	bool "Enable device tree videomode support"
+	depends on OF
+	select VIDEOMODE
+	select OF_DISPLAY_TIMING
+	help
+	  helper to get videomodes from the devicetree
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index e0dd820..f592f3b 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -169,4 +169,6 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
 obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_OF_DISPLAY_TIMING) += of_display_timing.o
 obj-$(CONFIG_VIDEOMODE) += videomode.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
new file mode 100644
index 0000000..13ecd98
--- /dev/null
+++ b/drivers/video/of_display_timing.c
@@ -0,0 +1,239 @@
+/*
+ * OF helpers for parsing display timings
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * based on of_videomode.c by Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/export.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <video/display_timing.h>
+#include <video/of_display_timing.h>
+
+/**
+ * parse_timing_property - parse timing_entry from device_node
+ * @np: device_node with the property
+ * @name: name of the property
+ * @result: will be set to the return value
+ *
+ * DESCRIPTION:
+ * Every display_timing can be specified with either just the typical value or
+ * a range consisting of min/typ/max. This function helps handling this
+ **/
+static int parse_timing_property(struct device_node *np, const char *name,
+			  struct timing_entry *result)
+{
+	struct property *prop;
+	int length, cells, ret;
+
+	prop = of_find_property(np, name, &length);
+	if (!prop) {
+		pr_err("%s: could not find property %s\n",
+			of_node_full_name(np), name);
+		return -EINVAL;
+	}
+
+	cells = length / sizeof(u32);
+	if (cells = 1) {
+		ret = of_property_read_u32(np, name, &result->typ);
+		result->min = result->typ;
+		result->max = result->typ;
+	} else if (cells = 3) {
+		ret = of_property_read_u32_array(np, name, &result->min, cells);
+	} else {
+		pr_err("%s: illegal timing specification in %s\n",
+			of_node_full_name(np), name);
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+/**
+ * of_get_display_timing - parse display_timing entry from device_node
+ * @np: device_node with the properties
+ **/
+static struct display_timing *of_get_display_timing(struct device_node *np)
+{
+	struct display_timing *dt;
+	u32 val = 0;
+	int ret = 0;
+
+	dt = kzalloc(sizeof(*dt), GFP_KERNEL);
+	if (!dt) {
+		pr_err("%s: could not allocate display_timing struct\n",
+			of_node_full_name(np));
+		return NULL;
+	}
+
+	ret |= parse_timing_property(np, "hback-porch", &dt->hback_porch);
+	ret |= parse_timing_property(np, "hfront-porch", &dt->hfront_porch);
+	ret |= parse_timing_property(np, "hactive", &dt->hactive);
+	ret |= parse_timing_property(np, "hsync-len", &dt->hsync_len);
+	ret |= parse_timing_property(np, "vback-porch", &dt->vback_porch);
+	ret |= parse_timing_property(np, "vfront-porch", &dt->vfront_porch);
+	ret |= parse_timing_property(np, "vactive", &dt->vactive);
+	ret |= parse_timing_property(np, "vsync-len", &dt->vsync_len);
+	ret |= parse_timing_property(np, "clock-frequency", &dt->pixelclock);
+
+	dt->dmt_flags = 0;
+	dt->data_flags = 0;
+	if (!of_property_read_u32(np, "vsync-active", &val))
+		dt->dmt_flags |= val ? VESA_DMT_VSYNC_HIGH :
+				VESA_DMT_VSYNC_LOW;
+	if (!of_property_read_u32(np, "hsync-active", &val))
+		dt->dmt_flags |= val ? VESA_DMT_HSYNC_HIGH :
+				VESA_DMT_HSYNC_LOW;
+	if (!of_property_read_u32(np, "de-active", &val))
+		dt->data_flags |= val ? DISPLAY_FLAGS_DE_HIGH :
+				DISPLAY_FLAGS_DE_LOW;
+	if (!of_property_read_u32(np, "pixelclk-active", &val))
+		dt->data_flags |= val ? DISPLAY_FLAGS_PIXDATA_POSEDGE :
+				DISPLAY_FLAGS_PIXDATA_NEGEDGE;
+
+	if (of_property_read_bool(np, "interlaced"))
+		dt->data_flags |= DISPLAY_FLAGS_INTERLACED;
+	if (of_property_read_bool(np, "doublescan"))
+		dt->data_flags |= DISPLAY_FLAGS_DOUBLESCAN;
+
+	if (ret) {
+		pr_err("%s: error reading timing properties\n",
+			of_node_full_name(np));
+		kfree(dt);
+		return NULL;
+	}
+
+	return dt;
+}
+
+/**
+ * of_get_display_timings - parse all display_timing entries from a device_node
+ * @np: device_node with the subnodes
+ **/
+struct display_timings *of_get_display_timings(struct device_node *np)
+{
+	struct device_node *timings_np;
+	struct device_node *entry;
+	struct device_node *native_mode;
+	struct display_timings *disp;
+
+	if (!np) {
+		pr_err("%s: no devicenode given\n", of_node_full_name(np));
+		return NULL;
+	}
+
+	timings_np = of_find_node_by_name(np, "display-timings");
+	if (!timings_np) {
+		pr_err("%s: could not find display-timings node\n",
+			of_node_full_name(np));
+		return NULL;
+	}
+
+	disp = kzalloc(sizeof(*disp), GFP_KERNEL);
+	if (!disp) {
+		pr_err("%s: could not allocate struct disp'\n",
+			of_node_full_name(np));
+		goto dispfail;
+	}
+
+	entry = of_parse_phandle(timings_np, "native-mode", 0);
+	/* assume first child as native mode if none provided */
+	if (!entry)
+		entry = of_get_next_child(np, NULL);
+	/* if there is no child, it is useless to go on */
+	if (!entry) {
+		pr_err("%s: no timing specifications given\n",
+			of_node_full_name(np));
+		goto entryfail;
+	}
+
+	pr_debug("%s: using %s as default timing\n",
+		of_node_full_name(np), entry->name);
+
+	native_mode = entry;
+
+	disp->num_timings = of_get_child_count(timings_np);
+	if (disp->num_timings = 0) {
+		/* should never happen, as entry was already found above */
+		pr_err("%s: no timings specified\n", of_node_full_name(np));
+		goto entryfail;
+	}
+
+	disp->timings = kzalloc(sizeof(struct display_timing *) *
+				disp->num_timings, GFP_KERNEL);
+	if (!disp->timings) {
+		pr_err("%s: could not allocate timings array\n",
+			of_node_full_name(np));
+		goto entryfail;
+	}
+
+	disp->num_timings = 0;
+	disp->native_mode = 0;
+
+	for_each_child_of_node(timings_np, entry) {
+		struct display_timing *dt;
+
+		dt = of_get_display_timing(entry);
+		if (!dt) {
+			/*
+			 * to not encourage wrong devicetrees, fail in case of
+			 * an error
+			 */
+			pr_err("%s: error in timing %d\n",
+				of_node_full_name(np), disp->num_timings + 1);
+			goto timingfail;
+		}
+
+		if (native_mode = entry)
+			disp->native_mode = disp->num_timings;
+
+		disp->timings[disp->num_timings] = dt;
+		disp->num_timings++;
+	}
+	of_node_put(timings_np);
+	/*
+	 * native_mode points to the device_node returned by of_parse_phandle
+	 * therefore call of_node_put on it
+	 */
+	of_node_put(native_mode);
+
+	pr_debug("%s: got %d timings. Using timing #%d as default\n",
+		of_node_full_name(np), disp->num_timings,
+		disp->native_mode + 1);
+
+	return disp;
+
+timingfail:
+	if (native_mode)
+		of_node_put(native_mode);
+	display_timings_release(disp);
+entryfail:
+	kfree(disp);
+dispfail:
+	of_node_put(timings_np);
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_get_display_timings);
+
+/**
+ * of_display_timings_exist - check if a display-timings node is provided
+ * @np: device_node with the timing
+ **/
+int of_display_timings_exist(struct device_node *np)
+{
+	struct device_node *timings_np;
+
+	if (!np)
+		return -EINVAL;
+
+	timings_np = of_parse_phandle(np, "display-timings", 0);
+	if (!timings_np)
+		return -EINVAL;
+
+	of_node_put(timings_np);
+	return 1;
+}
+EXPORT_SYMBOL_GPL(of_display_timings_exist);
diff --git a/drivers/video/of_videomode.c b/drivers/video/of_videomode.c
new file mode 100644
index 0000000..5b8066c
--- /dev/null
+++ b/drivers/video/of_videomode.c
@@ -0,0 +1,54 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/of.h>
+#include <video/display_timing.h>
+#include <video/of_display_timing.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
+
+/**
+ * of_get_videomode - get the videomode #<index> from devicetree
+ * @np - devicenode with the display_timings
+ * @vm - set to return value
+ * @index - index into list of display_timings
+ *	    (Set this to OF_USE_NATIVE_MODE to use whatever mode is
+ *	     specified as native mode in the DT.)
+ *
+ * DESCRIPTION:
+ * Get a list of all display timings and put the one
+ * specified by index into *vm. This function should only be used, if
+ * only one videomode is to be retrieved. A driver that needs to work
+ * with multiple/all videomodes should work with
+ * of_get_display_timings instead.
+ **/
+int of_get_videomode(struct device_node *np, struct videomode *vm,
+		     int index)
+{
+	struct display_timings *disp;
+	int ret;
+
+	disp = of_get_display_timings(np);
+	if (!disp) {
+		pr_err("%s: no timings specified\n", of_node_full_name(np));
+		return -EINVAL;
+	}
+
+	if (index = OF_USE_NATIVE_MODE)
+		index = disp->native_mode;
+
+	ret = videomode_from_timing(disp, vm, index);
+	if (ret)
+		return ret;
+
+	display_timings_release(disp);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_videomode);
diff --git a/include/video/of_display_timing.h b/include/video/of_display_timing.h
new file mode 100644
index 0000000..8016eb7
--- /dev/null
+++ b/include/video/of_display_timing.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * display timings of helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_DISPLAY_TIMING_H
+#define __LINUX_OF_DISPLAY_TIMING_H
+
+struct device_node;
+struct display_timings;
+
+#define OF_USE_NATIVE_MODE -1
+
+struct display_timings *of_get_display_timings(struct device_node *np);
+int of_display_timings_exist(struct device_node *np);
+
+#endif
diff --git a/include/video/of_videomode.h b/include/video/of_videomode.h
new file mode 100644
index 0000000..a07efcc
--- /dev/null
+++ b/include/video/of_videomode.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * videomode of-helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_VIDEOMODE_H
+#define __LINUX_OF_VIDEOMODE_H
+
+struct device_node;
+struct videomode;
+
+int of_get_videomode(struct device_node *np, struct videomode *vm,
+		     int index);
+
+#endif /* __LINUX_OF_VIDEOMODE_H */
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH v17 4/7] fbmon: add videomode helpers
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel
In-Reply-To: <1359104515-8907-1-git-send-email-s.trumtrar@pengutronix.de>

Add a function to convert from the generic videomode to a fb_videomode.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 drivers/video/fbmon.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h    |    4 ++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..17ce135 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <video/edid.h>
+#include <video/videomode.h>
 #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
 #include <asm/pci-bridge.h>
@@ -1373,6 +1374,57 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
 	kfree(timings);
 	return err;
 }
+
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int fb_videomode_from_videomode(const struct videomode *vm,
+				struct fb_videomode *fbmode)
+{
+	unsigned int htotal, vtotal;
+
+	fbmode->xres = vm->hactive;
+	fbmode->left_margin = vm->hback_porch;
+	fbmode->right_margin = vm->hfront_porch;
+	fbmode->hsync_len = vm->hsync_len;
+
+	fbmode->yres = vm->vactive;
+	fbmode->upper_margin = vm->vback_porch;
+	fbmode->lower_margin = vm->vfront_porch;
+	fbmode->vsync_len = vm->vsync_len;
+
+	/* prevent division by zero in KHZ2PICOS macro */
+	fbmode->pixclock = vm->pixelclock ?
+			KHZ2PICOS(vm->pixelclock / 1000) : 0;
+
+	fbmode->sync = 0;
+	fbmode->vmode = 0;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
+		fbmode->vmode |= FB_VMODE_INTERLACED;
+	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
+		fbmode->vmode |= FB_VMODE_DOUBLE;
+	fbmode->flag = 0;
+
+	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
+		 vm->hsync_len;
+	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
+		 vm->vsync_len;
+	/* prevent division by zero */
+	if (htotal && vtotal) {
+		fbmode->refresh = vm->pixelclock / (htotal * vtotal);
+	/* a mode must have htotal and vtotal != 0 or it is invalid */
+	} else {
+		fbmode->refresh = 0;
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
+#endif
+
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index c7a9571..100a176 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -19,6 +19,7 @@ struct vm_area_struct;
 struct fb_info;
 struct device;
 struct file;
+struct videomode;
 
 /* Definitions below are used in the parsed monitor specs */
 #define FB_DPMS_ACTIVE_OFF	1
@@ -714,6 +715,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+extern int fb_videomode_from_videomode(const struct videomode *vm,
+				       struct fb_videomode *fbmode);
+
 /* drivers/video/modedb.c */
 #define VESA_MODEDB_SIZE 34
 extern void fb_var_to_videomode(struct fb_videomode *mode,
-- 
1.7.10.4


^ 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