Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH V2 1/6] OMAPDSS: DISPC: Move burst_size and buffer_size to dispc_features
From: Chandrabhanu Mahapatra @ 2012-12-05 10:28 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1354702077.git.cmahapatra@ti.com>

The burst_size and buffer_size being local data to DISPC are moved to
dispc_features and so removed from struct omap_dss_features. The functions
referring to burst and buffer size are also removed from dss_features.c as they
are now accessed locally in dispc.c.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.c        |   21 +++++++++++++++++----
 drivers/video/omap2/dss/dss_features.c |   29 -----------------------------
 drivers/video/omap2/dss/dss_features.h |    3 ---
 3 files changed, 17 insertions(+), 36 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index ce594a1..bbba83f 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -107,6 +107,9 @@ struct dispc_features {
 
 	/* no DISPC_IRQ_FRAMEDONETV on this SoC */
 	bool no_framedone_tv:1;
+
+	u32 buffer_size_unit; /* in bytes */
+	u32 burst_size_unit; /* in bytes */
 };
 
 #define DISPC_MAX_NR_FIFOS 5
@@ -1050,7 +1053,7 @@ static void dispc_configure_burst_sizes(void)
 
 static u32 dispc_ovl_get_burst_size(enum omap_plane plane)
 {
-	unsigned unit = dss_feat_get_burst_size_unit();
+	unsigned unit = dispc.feat->burst_size_unit;
 	/* burst multiplier is always x8 (see dispc_configure_burst_sizes()) */
 	return unit * 8;
 }
@@ -1139,7 +1142,7 @@ static void dispc_init_fifos(void)
 	u8 start, end;
 	u32 unit;
 
-	unit = dss_feat_get_buffer_size_unit();
+	unit = dispc.feat->buffer_size_unit;
 
 	dss_feat_get_reg_field(FEAT_REG_FIFOSIZE, &start, &end);
 
@@ -1197,7 +1200,7 @@ void dispc_ovl_set_fifo_threshold(enum omap_plane plane, u32 low, u32 high)
 	u8 hi_start, hi_end, lo_start, lo_end;
 	u32 unit;
 
-	unit = dss_feat_get_buffer_size_unit();
+	unit = dispc.feat->buffer_size_unit;
 
 	WARN_ON(low % unit != 0);
 	WARN_ON(high % unit != 0);
@@ -1241,7 +1244,7 @@ void dispc_ovl_compute_fifo_thresholds(enum omap_plane plane,
 	 * buffer_units, and the fifo thresholds must be buffer_unit aligned.
 	 */
 
-	unsigned buf_unit = dss_feat_get_buffer_size_unit();
+	unsigned buf_unit = dispc.feat->buffer_size_unit;
 	unsigned ovl_fifo_size, total_fifo_size, burst_size;
 	int i;
 
@@ -4060,6 +4063,8 @@ static const struct dispc_features omap24xx_dispc_feats __initconst = {
 	.calc_core_clk		=	calc_core_clk_24xx,
 	.num_fifos		=	3,
 	.no_framedone_tv	=	true,
+	.buffer_size_unit	=	1,
+	.burst_size_unit	=	8,
 };
 
 static const struct dispc_features omap34xx_rev1_0_dispc_feats __initconst = {
@@ -4077,6 +4082,8 @@ static const struct dispc_features omap34xx_rev1_0_dispc_feats __initconst = {
 	.calc_core_clk		=	calc_core_clk_34xx,
 	.num_fifos		=	3,
 	.no_framedone_tv	=	true,
+	.buffer_size_unit	=	1,
+	.burst_size_unit	=	8,
 };
 
 static const struct dispc_features omap34xx_rev3_0_dispc_feats __initconst = {
@@ -4094,6 +4101,8 @@ static const struct dispc_features omap34xx_rev3_0_dispc_feats __initconst = {
 	.calc_core_clk		=	calc_core_clk_34xx,
 	.num_fifos		=	3,
 	.no_framedone_tv	=	true,
+	.buffer_size_unit	=	1,
+	.burst_size_unit	=	8,
 };
 
 static const struct dispc_features omap44xx_dispc_feats __initconst = {
@@ -4111,6 +4120,8 @@ static const struct dispc_features omap44xx_dispc_feats __initconst = {
 	.calc_core_clk		=	calc_core_clk_44xx,
 	.num_fifos		=	5,
 	.gfx_fifo_workaround	=	true,
+	.buffer_size_unit	=	16,
+	.burst_size_unit	=	16,
 };
 
 static const struct dispc_features omap54xx_dispc_feats __initconst = {
@@ -4128,6 +4139,8 @@ static const struct dispc_features omap54xx_dispc_feats __initconst = {
 	.calc_core_clk		=	calc_core_clk_44xx,
 	.num_fifos		=	5,
 	.gfx_fifo_workaround	=	true,
+	.buffer_size_unit	=	16,
+	.burst_size_unit	=	16,
 };
 
 static int __init dispc_init_features(struct platform_device *pdev)
diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index 1d125c6..092e21b 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -54,9 +54,6 @@ struct omap_dss_features {
 	const struct dss_param_range *dss_params;
 
 	const enum omap_dss_rotation_type supported_rotation_types;
-
-	const u32 buffer_size_unit;
-	const u32 burst_size_unit;
 };
 
 /* This struct is assigned to one of the below during initialization */
@@ -632,8 +629,6 @@ static const struct omap_dss_features omap2_dss_features = {
 	.clksrc_names = omap2_dss_clk_source_names,
 	.dss_params = omap2_dss_param_range,
 	.supported_rotation_types = OMAP_DSS_ROT_DMA | OMAP_DSS_ROT_VRFB,
-	.buffer_size_unit = 1,
-	.burst_size_unit = 8,
 };
 
 /* OMAP3 DSS Features */
@@ -653,8 +648,6 @@ static const struct omap_dss_features omap3430_dss_features = {
 	.clksrc_names = omap3_dss_clk_source_names,
 	.dss_params = omap3_dss_param_range,
 	.supported_rotation_types = OMAP_DSS_ROT_DMA | OMAP_DSS_ROT_VRFB,
-	.buffer_size_unit = 1,
-	.burst_size_unit = 8,
 };
 
 /*
@@ -677,8 +670,6 @@ static const struct omap_dss_features am35xx_dss_features = {
 	.clksrc_names = omap3_dss_clk_source_names,
 	.dss_params = omap3_dss_param_range,
 	.supported_rotation_types = OMAP_DSS_ROT_DMA | OMAP_DSS_ROT_VRFB,
-	.buffer_size_unit = 1,
-	.burst_size_unit = 8,
 };
 
 static const struct omap_dss_features omap3630_dss_features = {
@@ -697,8 +688,6 @@ static const struct omap_dss_features omap3630_dss_features = {
 	.clksrc_names = omap3_dss_clk_source_names,
 	.dss_params = omap3_dss_param_range,
 	.supported_rotation_types = OMAP_DSS_ROT_DMA | OMAP_DSS_ROT_VRFB,
-	.buffer_size_unit = 1,
-	.burst_size_unit = 8,
 };
 
 /* OMAP4 DSS Features */
@@ -720,8 +709,6 @@ static const struct omap_dss_features omap4430_es1_0_dss_features  = {
 	.clksrc_names = omap4_dss_clk_source_names,
 	.dss_params = omap4_dss_param_range,
 	.supported_rotation_types = OMAP_DSS_ROT_DMA | OMAP_DSS_ROT_TILER,
-	.buffer_size_unit = 16,
-	.burst_size_unit = 16,
 };
 
 /* For OMAP4430 ES 2.0, 2.1 and 2.2 revisions */
@@ -742,8 +729,6 @@ static const struct omap_dss_features omap4430_es2_0_1_2_dss_features = {
 	.clksrc_names = omap4_dss_clk_source_names,
 	.dss_params = omap4_dss_param_range,
 	.supported_rotation_types = OMAP_DSS_ROT_DMA | OMAP_DSS_ROT_TILER,
-	.buffer_size_unit = 16,
-	.burst_size_unit = 16,
 };
 
 /* For all the other OMAP4 versions */
@@ -764,8 +749,6 @@ static const struct omap_dss_features omap4_dss_features = {
 	.clksrc_names = omap4_dss_clk_source_names,
 	.dss_params = omap4_dss_param_range,
 	.supported_rotation_types = OMAP_DSS_ROT_DMA | OMAP_DSS_ROT_TILER,
-	.buffer_size_unit = 16,
-	.burst_size_unit = 16,
 };
 
 /* OMAP5 DSS Features */
@@ -785,8 +768,6 @@ static const struct omap_dss_features omap5_dss_features = {
 	.clksrc_names = omap5_dss_clk_source_names,
 	.dss_params = omap5_dss_param_range,
 	.supported_rotation_types = OMAP_DSS_ROT_DMA | OMAP_DSS_ROT_TILER,
-	.buffer_size_unit = 16,
-	.burst_size_unit = 16,
 };
 
 #if defined(CONFIG_OMAP4_DSS_HDMI)
@@ -892,16 +873,6 @@ const char *dss_feat_get_clk_source_name(enum omap_dss_clk_source id)
 	return omap_current_dss_features->clksrc_names[id];
 }
 
-u32 dss_feat_get_buffer_size_unit(void)
-{
-	return omap_current_dss_features->buffer_size_unit;
-}
-
-u32 dss_feat_get_burst_size_unit(void)
-{
-	return omap_current_dss_features->burst_size_unit;
-}
-
 /* DSS has_feature check */
 bool dss_has_feature(enum dss_feat_id id)
 {
diff --git a/drivers/video/omap2/dss/dss_features.h b/drivers/video/omap2/dss/dss_features.h
index 385b0af..16658e1 100644
--- a/drivers/video/omap2/dss/dss_features.h
+++ b/drivers/video/omap2/dss/dss_features.h
@@ -114,9 +114,6 @@ bool dss_feat_color_mode_supported(enum omap_plane plane,
 		enum omap_color_mode color_mode);
 const char *dss_feat_get_clk_source_name(enum omap_dss_clk_source id);
 
-u32 dss_feat_get_buffer_size_unit(void);	/* in bytes */
-u32 dss_feat_get_burst_size_unit(void);		/* in bytes */
-
 bool dss_feat_rotation_type_supported(enum omap_dss_rotation_type rot_type);
 
 bool dss_has_feature(enum dss_feat_id id);
-- 
1.7.10


^ permalink raw reply related

* [PATCH V2 0/6] OMAPDSS: Cleanup omap_dss_features
From: Chandrabhanu Mahapatra @ 2012-12-05 10:28 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1354086150.git.cmahapatra@ti.com>

Hi everyone,
this patch series aims at cleaning up of dss_features.c by moving features to
respective submodule drivers.

The 1st patch
 * moved struct omap_dss_features members burst_size_unit and buffer_size_unit
   to dispc_features
The 2nd patch
 * added definition of struct omapdss_reg_field
 * moved data from dss_reg_fields to dispc_reg_fields
The 3rd patch
 * added definition of struct omapdss_param_range
 * moved data from dss_param_range to dispc_param_ranges
The 4th patch
 * created struct feats and initialized dsi_feats
 * moved data from dss_reg_fields to dsi_reg_fields
 * created dsi_init_features() to initialize dsi_feats
The 5th patch
 * moved data from dss_param_range to dsi_param_range
The 6th patch
 * added members fld_dispc_clk_switch and dss_fck_max to struct dss_features
   and initialized corresponding dss_feats
 * removed definition of dss_reg_field and dss_param_range and their respective
   initializations as omap2_dss_reg_fields, omap3_dss_reg_fields, etc.
 * removed members reg_fields and num_reg_fields from struct omap_dss_features
 * removed enum dss_feat_reg_field and dss_range_param and corresponding
   initializations omap2_dss_param_range, omap3_dss_param_range etc.
 * removed function dss_feat_get_reg_field(), dss_feat_get_param_min() and
   dss_feat_get_param_max()

Changes from V1
 * 1st patch: added comments "in bytes" for new members burst_size_unit and
		buffer_size_unit
 * 2nd patch: renamed register_field to omapdss_reg_field
	      removed definition of enum dispc_feat_reg_field
	      created and initialized struct dispc_reg_fields
 * 3rd patch: renamed struct param_range to omapdss_param_range
	      removed definition of enum dispc_range_param
	      created and initialized struct dispc_param_ranges
 * 4th patch: removed enum dsi_feat_reg_field
	      created and initialized struct dsi_reg_fields
 * 5th patch: removed enum dsi_range_param
	      created and initialized struct dsi_param_ranges
	      added member dss_fck corresponding to FEAT_PARAM_DSS_FCK
 * 6th patch: removed struct dss_param_range and enum dss_range_param
	      removed functions dss_feat_get_param_min() and
			dss_feat_get_param_max()
 * 7th patch has been removed

All your comments and suggestions are welcome.

I have based my patches on git://gitorious.org/linux-omap-dss2/linux.git master

Reference Tree:
git://gitorious.org/linux-omap-dss2/chandrabhanus-linux.git dss_cleanup

Regards,
Chandrabhanu

Chandrabhanu Mahapatra (6):
  OMAPDSS: DISPC: Move burst_size and buffer_size to dispc_features
  OMAPDSS: DISPC: Move DISPC specific dss_reg_fields to dispc_features
  OMAPDSS: DISPC: Move DISPC specific dss_params to dispc_features
  OMAPDSS: DSI: Move DSI specific reg_fields to dsi_feats
  OMAPDSS: DSI: Move DSI specific dss_params to dsi_feats
  OMAPDSS: DSS: Add members fld_dispc_clk_switch and dss_fck_max

 drivers/video/omap2/dss/dispc.c        |  218 +++++++++++++++++++++++---------
 drivers/video/omap2/dss/dsi.c          |  206 ++++++++++++++++++++++++++----
 drivers/video/omap2/dss/dss.c          |   22 +++-
 drivers/video/omap2/dss/dss.h          |    8 ++
 drivers/video/omap2/dss/dss_features.c |  212 -------------------------------
 drivers/video/omap2/dss/dss_features.h |   36 ------
 6 files changed, 368 insertions(+), 334 deletions(-)

-- 
1.7.10


^ permalink raw reply

* Re: [PATCHv15 0/7] of: add display helper
From: Steffen Trumtrar @ 2012-12-05  9:00 UTC (permalink / raw)
  To: Leela Krishna Amudala
  Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie
In-Reply-To: <CAL1wa8fJ-mCsQFFo5wMLDRcUoOyWvTvFDLKsrrG9k-mxQTsTbg@mail.gmail.com>

Hi Leela,

unfortunately, nothing new as of yet. I have to work on other stuff at the moment.

Regards,
Steffen


On Wed, Dec 05, 2012 at 01:25:30PM +0530, Leela Krishna Amudala wrote:
> Hello Steffen,
> 
> Any update on version 16 ?
> 
> Best Wishes,
> Leela Krishna Amudala.
> 
> On Mon, Nov 26, 2012 at 2:37 PM, Steffen Trumtrar
> <s.trumtrar@pengutronix.de> wrote:
> > Hi!
> >
> > 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 bufixe
> >
> > 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
> >
> >
> > 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   |  107 ++++++++++
> >  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                              |   93 +++++++++
> >  drivers/video/of_display_timing.c                  |  219 ++++++++++++++++++++
> >  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                          |   44 ++++
> >  include/drm/drmP.h                                 |   13 ++
> >  include/linux/display_timing.h                     |  104 ++++++++++
> >  include/linux/fb.h                                 |   12 ++
> >  include/linux/of_display_timing.h                  |   20 ++
> >  include/linux/of_videomode.h                       |   18 ++
> >  include/linux/videomode.h                          |   54 +++++
> >  21 files changed, 870 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/linux/display_timing.h
> >  create mode 100644 include/linux/of_display_timing.h
> >  create mode 100644 include/linux/of_videomode.h
> >  create mode 100644 include/linux/videomode.h
> >
> > --
> > 1.7.10.4
> >
> > --
> > 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
> 

-- 
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: [PATCHv15 0/7] of: add display helper
From: Leela Krishna Amudala @ 2012-12-05  7:55 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie
In-Reply-To: <1353920848-1705-1-git-send-email-s.trumtrar@pengutronix.de>

Hello Steffen,

Any update on version 16 ?

Best Wishes,
Leela Krishna Amudala.

On Mon, Nov 26, 2012 at 2:37 PM, Steffen Trumtrar
<s.trumtrar@pengutronix.de> wrote:
> Hi!
>
> 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 bufixe
>
> 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
>
>
> 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   |  107 ++++++++++
>  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                              |   93 +++++++++
>  drivers/video/of_display_timing.c                  |  219 ++++++++++++++++++++
>  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                          |   44 ++++
>  include/drm/drmP.h                                 |   13 ++
>  include/linux/display_timing.h                     |  104 ++++++++++
>  include/linux/fb.h                                 |   12 ++
>  include/linux/of_display_timing.h                  |   20 ++
>  include/linux/of_videomode.h                       |   18 ++
>  include/linux/videomode.h                          |   54 +++++
>  21 files changed, 870 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/linux/display_timing.h
>  create mode 100644 include/linux/of_display_timing.h
>  create mode 100644 include/linux/of_videomode.h
>  create mode 100644 include/linux/videomode.h
>
> --
> 1.7.10.4
>
> --
> 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] da8xx: Fix revision check on the da8xx driver
From: Tomi Valkeinen @ 2012-12-04  9:18 UTC (permalink / raw)
  To: Manjunathappa, Prakash
  Cc: Pantelis Antoniou, Florian Tobias Schandinat,
	linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Koen Kooi, Porter, Matt, Dill, Russ, linux-omap@vger.kernel.org
In-Reply-To: <A73F36158E33644199EB82C5EC81C7BC3EA13286@DBDE01.ent.ti.com>

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

On 2012-12-04 08:36, Manjunathappa, Prakash wrote:
> Hi Tomi,
> 
> On Wed, Oct 31, 2012 at 09:21:35, Manjunathappa, Prakash wrote:
>> On Wed, Oct 31, 2012 at 21:26:24, Pantelis Antoniou wrote:
>>> The revision check fails for the beaglebone; Add new revision ID.
>>>
>>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>>> ---
>>>  drivers/video/da8xx-fb.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
>>> index 80665f6..866d804 100644
>>> --- a/drivers/video/da8xx-fb.c
>>> +++ b/drivers/video/da8xx-fb.c
>>> @@ -1283,6 +1283,7 @@ static int __devinit fb_probe(struct platform_device *device)
>>>  		lcd_revision = LCD_VERSION_1;
>>>  		break;
>>>  	case 0x4F200800:
>>> +	case 0x4F201000:
>>
>> Thanks for Correcting. This is the LCDC revision on am335x silicon in comparison
>> with to one read(0x4F200800) on emulator platform.
>>
>> Acked-by: Manjunathappa, Prakash <prakash.pm@ti.com>
>>
> 
> This patch is necessary for am335x FB, I could not see in your for-next branch,

I have only applied the patches that have been sent to me.

> could you please consider this patch for 3.8 merge window. There are no pending
> review comments on this.

Applied.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 2/7] OMAPDSS: DISPC: Move DISPC specific dss_reg_fields to dispc_features
From: Tomi Valkeinen @ 2012-12-04  8:16 UTC (permalink / raw)
  To: Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <50BC46C6.3050707@ti.com>

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

On 2012-12-03 08:29, Chandrabhanu Mahapatra wrote:
> On Thursday 29 November 2012 05:48 PM, Tomi Valkeinen wrote:
>> On 2012-11-28 12:41, Chandrabhanu Mahapatra wrote:
>>> The register fields in dss_reg_fields specific to DISPC are moved from struct
>>> omap_dss_features to corresponding dispc_reg_fields, initialized in struct
>>> dispc_features, thereby enabling local access.
>>>
>>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>>> ---
>>>  drivers/video/omap2/dss/dispc.c        |   87 ++++++++++++++++++++++++++++----
>>>  drivers/video/omap2/dss/dss.h          |    4 ++
>>>  drivers/video/omap2/dss/dss_features.c |   28 ----------
>>>  drivers/video/omap2/dss/dss_features.h |    7 ---
>>>  4 files changed, 80 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>>> index 9f259ba..21fc522 100644
>>> --- a/drivers/video/omap2/dss/dispc.c
>>> +++ b/drivers/video/omap2/dss/dispc.c
>>> @@ -80,6 +80,16 @@ struct dispc_irq_stats {
>>>  	unsigned irqs[32];
>>>  };
>>>  
>>> +enum dispc_feat_reg_field {
>>> +	FEAT_REG_FIRHINC,
>>> +	FEAT_REG_FIRVINC,
>>> +	FEAT_REG_FIFOLOWTHRESHOLD,
>>> +	FEAT_REG_FIFOHIGHTHRESHOLD,
>>> +	FEAT_REG_FIFOSIZE,
>>> +	FEAT_REG_HORIZONTALACCU,
>>> +	FEAT_REG_VERTICALACCU,
>>> +};
>>> +
>>>  struct dispc_features {
>>>  	u8 sw_start;
>>>  	u8 fp_start;
>>> @@ -107,6 +117,8 @@ struct dispc_features {
>>>  
>>>  	u32 buffer_size_unit;
>>>  	u32 burst_size_unit;
>>> +
>>> +	struct register_field *reg_fields;
>>>  };
>>
>> Hmm, would it be simpler to have an explicit struct for the reg fields.
>> I mean something like:
>>
>> struct dispc_reg_fields {
>> 	struct register_field firhinc;
>> 	struct register_field firvinc;
>> 	struct register_field fifo_low_threshold;
>> 	...
>> };
>>
>> Then accessing it would be
>>
>> dispc.feat->reg_fields.firhinc.start;
>>
>> instead of
>>
>> dispc.feat->reg_fields[FEAT_REG_FIFOSIZE].start;
>>
>> Not a big difference, but I don't see any benefit in having an array of
>> reg fields here.
>>
>>  Tomi
>>
>>
> 
> I was thinking to move reg_fields into the dispc_feats structure as
> 
> 	.burst_size_unit	=	8,
> 	.reg_fields		=	{
> 		.firhinc			= { 12,  0 },
> 		.firvinc			= { 28, 16 },
> 		.fifo_low_thresh		= { 11,  0 },
> 		.fifo_high_thresh		= { 27, 16 },
> 		.fifosize			= { 10,  0 },
> 		.hori_accu			= {  9,  0 },
> 		.vert_accu			= { 25, 16 },
> 	},
> 
> This would give us dispc.feat->reg_fields.firhinc.start;
> but at the same time would create duplicate information for
> omap34xx_rev1_0_dispc_feats and omap34xx_rev3_0_dispc_feats. However,
> this duplication never occurs anywhere else in dss.c or dsi.c.
> 
> If we still go with the older approach of having dispc_reg_fields
> outside dispc_feats the only way  it works is
> 
> .reg_fields		=	&omap2_dispc_reg_fields
> 
> which changes as dispc.feat->reg_fields->firhinc.start;
> 
> but avoids duplicate information. Both approaches seem good enough to me.

I would keep the pointer approach. We may add support for new DSS
versions, for AM3xxx or AM4xxx SoCs, which possibly may have the same
register fields as some other DSS versions.

 Tomi



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

^ permalink raw reply

* RE: [PATCH] da8xx: Fix revision check on the da8xx driver
From: Manjunathappa, Prakash @ 2012-12-04  6:36 UTC (permalink / raw)
  To: Pantelis Antoniou, Florian Tobias Schandinat, Valkeinen, Tomi
  Cc: linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Koen Kooi, Porter, Matt, Dill, Russ, linux-omap@vger.kernel.org
In-Reply-To: <A73F36158E33644199EB82C5EC81C7BC3E9E10D6@DBDE01.ent.ti.com>

Hi Tomi,

On Wed, Oct 31, 2012 at 09:21:35, Manjunathappa, Prakash wrote:
> On Wed, Oct 31, 2012 at 21:26:24, Pantelis Antoniou wrote:
> > The revision check fails for the beaglebone; Add new revision ID.
> > 
> > Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> > ---
> >  drivers/video/da8xx-fb.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> > index 80665f6..866d804 100644
> > --- a/drivers/video/da8xx-fb.c
> > +++ b/drivers/video/da8xx-fb.c
> > @@ -1283,6 +1283,7 @@ static int __devinit fb_probe(struct platform_device *device)
> >  		lcd_revision = LCD_VERSION_1;
> >  		break;
> >  	case 0x4F200800:
> > +	case 0x4F201000:
> 
> Thanks for Correcting. This is the LCDC revision on am335x silicon in comparison
> with to one read(0x4F200800) on emulator platform.
> 
> Acked-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> 

This patch is necessary for am335x FB, I could not see in your for-next branch,
could you please consider this patch for 3.8 merge window. There are no pending
review comments on this.

Thanks,
Prakash

> >  		lcd_revision = LCD_VERSION_2;
> >  		break;
> >  	default:
> > -- 
> > 1.7.12
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply

* Re: [PATCH 2/7] OMAPDSS: DISPC: Move DISPC specific dss_reg_fields to dispc_features
From: Chandrabhanu Mahapatra @ 2012-12-03  6:41 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <50B7528C.1090507@ti.com>

On Thursday 29 November 2012 05:48 PM, Tomi Valkeinen wrote:
> On 2012-11-28 12:41, Chandrabhanu Mahapatra wrote:
>> The register fields in dss_reg_fields specific to DISPC are moved from struct
>> omap_dss_features to corresponding dispc_reg_fields, initialized in struct
>> dispc_features, thereby enabling local access.
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>> ---
>>  drivers/video/omap2/dss/dispc.c        |   87 ++++++++++++++++++++++++++++----
>>  drivers/video/omap2/dss/dss.h          |    4 ++
>>  drivers/video/omap2/dss/dss_features.c |   28 ----------
>>  drivers/video/omap2/dss/dss_features.h |    7 ---
>>  4 files changed, 80 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>> index 9f259ba..21fc522 100644
>> --- a/drivers/video/omap2/dss/dispc.c
>> +++ b/drivers/video/omap2/dss/dispc.c
>> @@ -80,6 +80,16 @@ struct dispc_irq_stats {
>>  	unsigned irqs[32];
>>  };
>>  
>> +enum dispc_feat_reg_field {
>> +	FEAT_REG_FIRHINC,
>> +	FEAT_REG_FIRVINC,
>> +	FEAT_REG_FIFOLOWTHRESHOLD,
>> +	FEAT_REG_FIFOHIGHTHRESHOLD,
>> +	FEAT_REG_FIFOSIZE,
>> +	FEAT_REG_HORIZONTALACCU,
>> +	FEAT_REG_VERTICALACCU,
>> +};
>> +
>>  struct dispc_features {
>>  	u8 sw_start;
>>  	u8 fp_start;
>> @@ -107,6 +117,8 @@ struct dispc_features {
>>  
>>  	u32 buffer_size_unit;
>>  	u32 burst_size_unit;
>> +
>> +	struct register_field *reg_fields;
>>  };
> 
> Hmm, would it be simpler to have an explicit struct for the reg fields.
> I mean something like:
> 
> struct dispc_reg_fields {
> 	struct register_field firhinc;
> 	struct register_field firvinc;
> 	struct register_field fifo_low_threshold;
> 	...
> };
> 
> Then accessing it would be
> 
> dispc.feat->reg_fields.firhinc.start;
> 
> instead of
> 
> dispc.feat->reg_fields[FEAT_REG_FIFOSIZE].start;
> 
> Not a big difference, but I don't see any benefit in having an array of
> reg fields here.
> 
>  Tomi
> 
> 

I was thinking to move reg_fields into the dispc_feats structure as

	.burst_size_unit	=	8,
	.reg_fields		=	{
		.firhinc			= { 12,  0 },
		.firvinc			= { 28, 16 },
		.fifo_low_thresh		= { 11,  0 },
		.fifo_high_thresh		= { 27, 16 },
		.fifosize			= { 10,  0 },
		.hori_accu			= {  9,  0 },
		.vert_accu			= { 25, 16 },
	},

This would give us dispc.feat->reg_fields.firhinc.start;
but at the same time would create duplicate information for
omap34xx_rev1_0_dispc_feats and omap34xx_rev3_0_dispc_feats. However,
this duplication never occurs anywhere else in dss.c or dsi.c.

If we still go with the older approach of having dispc_reg_fields
outside dispc_feats the only way  it works is

.reg_fields		=	&omap2_dispc_reg_fields

which changes as dispc.feat->reg_fields->firhinc.start;

but avoids duplicate information. Both approaches seem good enough to me.


-- 
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.

^ permalink raw reply

* Re: [RFC v2 3/5] video: display: Add MIPI DBI bus support
From: Tomi Valkeinen @ 2012-11-30 12:02 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-fbdev, dri-devel, linux-media, Archit Taneja,
	Benjamin Gaignard, Bryan Wu, Inki Dae, Jesse Barker,
	Kyungmin Park, Marcus Lorentzon, Maxime Ripard, Philipp Zabel,
	Ragesh Radhakrishnan, Rob Clark, Sascha Hauer, Sebastien Guiriec,
	Sumit Semwal, Thomas Petazzoni, Tom Gall, Vikas Sajjan
In-Reply-To: <1353620736-6517-4-git-send-email-laurent.pinchart@ideasonboard.com>

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

Hi,

On 2012-11-22 23:45, Laurent Pinchart wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/video/display/Kconfig        |    4 +
>  drivers/video/display/Makefile       |    1 +
>  drivers/video/display/mipi-dbi-bus.c |  228 ++++++++++++++++++++++++++++++++++
>  include/video/display.h              |    5 +
>  include/video/mipi-dbi-bus.h         |  125 +++++++++++++++++++
>  5 files changed, 363 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/video/display/mipi-dbi-bus.c
>  create mode 100644 include/video/mipi-dbi-bus.h

I've been doing some omapdss testing with CDF and DSI, and I have some
thoughts about the bus stuff. I already told these to Laurent, but I'll
write them to the mailing list also for discussion.

So with the current CDF model we have separate control and video buses.
The control bus is represented as proper Linux bus, and video bus is
represented via custom display_entity. This sounds good on paper, and I
also agreed to this approach when we were planning CDF.

However, now I doubt that approach.

First, I want to list some examples of devices with different bus
configurations:

1) Panel without any control, only video bus
2) Panel with separate control and video buses, e.g. i2c for control,
DPI for video
3) Panel with the same control and video buses, like DSI or DBI.

The first one is simple, it's just a platform device. No questions there.

The second one can be a bit tricky. Say, if we have a panel controlled
via i2c, and DSI/DBI used for video. The problem here is that with the
current model, DSI/DBI would be represented as a real bus, for control.
But in this case there's only the video path.

So if all the DSI/DBI bus configuration is handled on the DSI/DBI
control bus side, how can it be handled with only the video bus? And if
we add the same bus configuration to the video bus side as we have on
control bus side, then we have duplicated the API, and it's also
somewhat confusing. I don't have any good suggestion for this.

Third one is kinda clear, but I feel slightly uneasy about it. In theory
we can have separate control and video buses, which use the same HW
transport. However, I feel that we'll have some trouble with the
implementation, as we'll then have two more or less independent users
for the HW transport. I can't really point out why this would not be
possible to implement, but I have a gut feeling that it will be
difficult, at least for DSI.

So I think my question is: what does it give us to have separate control
and video buses, and what does the Linux bus give us with the control bus?

I don't see us ever having a case where a device would use one of the
display buses only for control. So either the display bus is only used
for video, or it's used for both control and video. And the display bus
is always 1-to-1, so we're talking about really simple bus here.

I believe things would be much simpler if we just have one entity for
the display buses, which support both video and (when available)
control. What would be the downsides of this approach versus the current
CDF proposal?

 Tomi



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

^ permalink raw reply

* Re: [PATCH 2/7] OMAPDSS: DISPC: Move DISPC specific dss_reg_fields to dispc_features
From: Tomi Valkeinen @ 2012-11-30 10:25 UTC (permalink / raw)
  To: Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <50B8844E.2010903@ti.com>

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

On 2012-11-30 12:02, Chandrabhanu Mahapatra wrote:
> On Thursday 29 November 2012 05:35 PM, Tomi Valkeinen wrote:

>>> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
>>> index 84a7f6a..aa273d8 100644
>>> --- a/drivers/video/omap2/dss/dss.h
>>> +++ b/drivers/video/omap2/dss/dss.h
>>> @@ -143,6 +143,10 @@ struct reg_field {
>>>  	u8 low;
>>>  };
>>>  
>>> +struct register_field {
>>> +	u8 start, end;
>>> +};
>>> +
>>
>> We already have the dss_reg_field struct. I think it's better to move
>> that to dss.h, and use it, instead of creating an exact duplicate.
>>
>>  Tomi
>>
>>
> 
> register_field appears to be a more generic a name rather than
> dss_reg_field. Also I was thinking to initialise

Yes, register_field is a more generic name, and that's one reason I
don't suggest using it. There's a possibility of name clash if some
common linux framework would use a similar name. So dss_reg_field refers
to a register field, used by (omap)dss.

It could also be renamed to omapdss_reg_field, but that's a bit longer.
But perhaps naming it omapdss_reg_field would separate it better from
dss_reg_fields.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 2/7] OMAPDSS: DISPC: Move DISPC specific dss_reg_fields to dispc_features
From: Chandrabhanu Mahapatra @ 2012-11-30 10:14 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <50B74F98.2030108@ti.com>

On Thursday 29 November 2012 05:35 PM, Tomi Valkeinen wrote:
> On 2012-11-28 12:41, Chandrabhanu Mahapatra wrote:
>> The register fields in dss_reg_fields specific to DISPC are moved from struct
>> omap_dss_features to corresponding dispc_reg_fields, initialized in struct
>> dispc_features, thereby enabling local access.
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>> ---
>>  drivers/video/omap2/dss/dispc.c        |   87 ++++++++++++++++++++++++++++----
>>  drivers/video/omap2/dss/dss.h          |    4 ++
>>  drivers/video/omap2/dss/dss_features.c |   28 ----------
>>  drivers/video/omap2/dss/dss_features.h |    7 ---
>>  4 files changed, 80 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>> index 9f259ba..21fc522 100644
>> --- a/drivers/video/omap2/dss/dispc.c
>> +++ b/drivers/video/omap2/dss/dispc.c
>> @@ -80,6 +80,16 @@ struct dispc_irq_stats {
>>  	unsigned irqs[32];
>>  };
>>  
>> +enum dispc_feat_reg_field {
>> +	FEAT_REG_FIRHINC,
>> +	FEAT_REG_FIRVINC,
>> +	FEAT_REG_FIFOLOWTHRESHOLD,
>> +	FEAT_REG_FIFOHIGHTHRESHOLD,
>> +	FEAT_REG_FIFOSIZE,
>> +	FEAT_REG_HORIZONTALACCU,
>> +	FEAT_REG_VERTICALACCU,
>> +};
>> +
>>  struct dispc_features {
>>  	u8 sw_start;
>>  	u8 fp_start;
>> @@ -107,6 +117,8 @@ struct dispc_features {
>>  
>>  	u32 buffer_size_unit;
>>  	u32 burst_size_unit;
>> +
>> +	struct register_field *reg_fields;
>>  };
>>  
>>  #define DISPC_MAX_NR_FIFOS 5
>> @@ -1150,7 +1162,8 @@ static void dispc_init_fifos(void)
>>  
>>  	unit = dispc.feat->buffer_size_unit;
>>  
>> -	dss_feat_get_reg_field(FEAT_REG_FIFOSIZE, &start, &end);
>> +	start = dispc.feat->reg_fields[FEAT_REG_FIFOSIZE].start;
>> +	end = dispc.feat->reg_fields[FEAT_REG_FIFOSIZE].end;
>>  
>>  	for (fifo = 0; fifo < dispc.feat->num_fifos; ++fifo) {
>>  		size = REG_GET(DISPC_OVL_FIFO_SIZE_STATUS(fifo), start, end);
>> @@ -1214,8 +1227,10 @@ void dispc_ovl_set_fifo_threshold(enum omap_plane plane, u32 low, u32 high)
>>  	low /= unit;
>>  	high /= unit;
>>  
>> -	dss_feat_get_reg_field(FEAT_REG_FIFOHIGHTHRESHOLD, &hi_start, &hi_end);
>> -	dss_feat_get_reg_field(FEAT_REG_FIFOLOWTHRESHOLD, &lo_start, &lo_end);
>> +	hi_start = dispc.feat->reg_fields[FEAT_REG_FIFOHIGHTHRESHOLD].start;
>> +	hi_end = dispc.feat->reg_fields[FEAT_REG_FIFOHIGHTHRESHOLD].end;
>> +	lo_start = dispc.feat->reg_fields[FEAT_REG_FIFOLOWTHRESHOLD].start;
>> +	lo_end = dispc.feat->reg_fields[FEAT_REG_FIFOLOWTHRESHOLD].end;
> 
> I think these are quite verbose. Perhaps a helper function which does
> the same as dss_feat_get_reg_field() would be better. Or alternatively,
> maybe something like:
> 
> const struct dss_reg_field *hi_field > &dispc.feat->reg_fields[FEAT_REG_FIFOHIGHTHRESHOLD];
> 
> and then use "hi_field.start" instead of hi_start.
> 
> Or something else that makes the above easier to read.

OK.

> 
>> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
>> index 84a7f6a..aa273d8 100644
>> --- a/drivers/video/omap2/dss/dss.h
>> +++ b/drivers/video/omap2/dss/dss.h
>> @@ -143,6 +143,10 @@ struct reg_field {
>>  	u8 low;
>>  };
>>  
>> +struct register_field {
>> +	u8 start, end;
>> +};
>> +
> 
> We already have the dss_reg_field struct. I think it's better to move
> that to dss.h, and use it, instead of creating an exact duplicate.
> 
>  Tomi
> 
> 

register_field appears to be a more generic a name rather than
dss_reg_field. Also I was thinking to initialise

struct dispc_reg_fields {
	struct register_field firhinc;
	struct register_field firvinc;
	struct register_field fifo_low_threshold;
	...
};
 similarly, dss_reg_fields and dsi_reg_fields from register_field.

-- 
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.

^ permalink raw reply

* Re: [RFC v2 2/5] video: panel: Add DPI panel support
From: Philipp Zabel @ 2012-11-30  9:26 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-fbdev, dri-devel, linux-media, Archit Taneja,
	Benjamin Gaignard, Bryan Wu, Inki Dae, Jesse Barker,
	Kyungmin Park, Marcus Lorentzon, Maxime Ripard,
	Ragesh Radhakrishnan, Rob Clark, Sascha Hauer, Sebastien Guiriec,
	Sumit Semwal, Thomas Petazzoni, Tom Gall, Tomi Valkeinen,
	Vikas Sajjan
In-Reply-To: <1353620736-6517-3-git-send-email-laurent.pinchart@ideasonboard.com>

Hi Laurent,

Am Donnerstag, den 22.11.2012, 22:45 +0100 schrieb Laurent Pinchart:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/video/display/Kconfig     |   13 +++
>  drivers/video/display/Makefile    |    1 +
>  drivers/video/display/panel-dpi.c |  147 +++++++++++++++++++++++++++++++++++++
>  include/video/panel-dpi.h         |   24 ++++++
>  4 files changed, 185 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/video/display/panel-dpi.c
>  create mode 100644 include/video/panel-dpi.h
> 
> diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig
> index 1d533e7..0f9b990 100644
> --- a/drivers/video/display/Kconfig
> +++ b/drivers/video/display/Kconfig
> @@ -2,3 +2,16 @@ menuconfig DISPLAY_CORE
>  	tristate "Display Core"
>  	---help---
>  	  Support common display framework for graphics devices.
> +
> +if DISPLAY_CORE
> +
> +config DISPLAY_PANEL_DPI
> +	tristate "DPI (Parallel) Display Panels"
> +	---help---
> +	  Support for simple digital (parallel) pixel interface panels. Those
> +	  panels receive pixel data through a parallel bus and have no control
> +	  bus.

I have tried this driver together with the imx parallel-display with the
added patch below for device tree support.

> +	  If you are in doubt, say N.
> +
> +endif # DISPLAY_CORE
> diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile
> index bd93496..47978d4 100644
> --- a/drivers/video/display/Makefile
> +++ b/drivers/video/display/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_DISPLAY_CORE) += display-core.o
> +obj-$(CONFIG_DISPLAY_PANEL_DPI) += panel-dpi.o
> diff --git a/drivers/video/display/panel-dpi.c b/drivers/video/display/panel-dpi.c
> new file mode 100644
> index 0000000..c56197a
> --- /dev/null
> +++ b/drivers/video/display/panel-dpi.c
> @@ -0,0 +1,147 @@
> +/*
> + * DPI Display Panel
> + *
> + * Copyright (C) 2012 Renesas Solutions Corp.
> + *
> + * Contacts: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include <video/display.h>
> +#include <video/panel-dpi.h>
> +
> +struct panel_dpi {
> +	struct display_entity entity;
> +	const struct panel_dpi_platform_data *pdata;
> +};
> +
> +#define to_panel_dpi(p)		container_of(p, struct panel_dpi, entity)
> +
> +static const struct display_entity_interface_params panel_dpi_params = {
> +	.type = DISPLAY_ENTITY_INTERFACE_DPI,
> +};
> +
> +static int panel_dpi_set_state(struct display_entity *entity,
> +			       enum display_entity_state state)
> +{
> +	switch (state) {
> +	case DISPLAY_ENTITY_STATE_OFF:
> +	case DISPLAY_ENTITY_STATE_STANDBY:
> +		display_entity_set_stream(entity->source,
> +					  DISPLAY_ENTITY_STREAM_STOPPED);
> +		break;
> +
> +	case DISPLAY_ENTITY_STATE_ON:
> +		display_entity_set_stream(entity->source,
> +					  DISPLAY_ENTITY_STREAM_CONTINUOUS);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int panel_dpi_get_modes(struct display_entity *entity,
> +			       const struct videomode **modes)
> +{
> +	struct panel_dpi *panel = to_panel_dpi(entity);
> +
> +	*modes = panel->pdata->mode;
> +	return 1;
> +}
> +
> +static int panel_dpi_get_size(struct display_entity *entity,
> +			      unsigned int *width, unsigned int *height)
> +{
> +	struct panel_dpi *panel = to_panel_dpi(entity);
> +
> +	*width = panel->pdata->width;
> +	*height = panel->pdata->height;
> +	return 0;
> +}
> +
> +static int panel_dpi_get_params(struct display_entity *entity,
> +				struct display_entity_interface_params *params)
> +{
> +	*params = panel_dpi_params;
> +	return 0;
> +}
> +
> +static const struct display_entity_control_ops panel_dpi_control_ops = {
> +	.set_state = panel_dpi_set_state,
> +	.get_modes = panel_dpi_get_modes,
> +	.get_size = panel_dpi_get_size,
> +	.get_params = panel_dpi_get_params,
> +};
> +
> +static void panel_dpi_release(struct display_entity *entity)
> +{
> +	struct panel_dpi *panel = to_panel_dpi(entity);
> +
> +	kfree(panel);
> +}
> +
> +static int panel_dpi_remove(struct platform_device *pdev)
> +{
> +	struct panel_dpi *panel = platform_get_drvdata(pdev);
> +
> +	platform_set_drvdata(pdev, NULL);
> +	display_entity_unregister(&panel->entity);
> +
> +	return 0;
> +}
> +
> +static int __devinit panel_dpi_probe(struct platform_device *pdev)
> +{
> +	const struct panel_dpi_platform_data *pdata = pdev->dev.platform_data;
> +	struct panel_dpi *panel;
> +	int ret;
> +
> +	if (pdata = NULL)
> +		return -ENODEV;
> +
> +	panel = kzalloc(sizeof(*panel), GFP_KERNEL);
> +	if (panel = NULL)
> +		return -ENOMEM;
> +
> +	panel->pdata = pdata;
> +	panel->entity.dev = &pdev->dev;
> +	panel->entity.release = panel_dpi_release;

I don't understand this. Shouldn't the panel be allocated with
devm_kzalloc and display_entity_register make sure that this driver
cannot be unbound instead?

What if we call in sequence on this device's entity:
	display_entity_get(entity);
	display_entity_release(entity); /* here struct panel_dpi gets freed */
	display_entity_get(entity);
	display_entity_release(entity);

> +	panel->entity.ops.ctrl = &panel_dpi_control_ops;
> +
> +	ret = display_entity_register(&panel->entity);
> +	if (ret < 0) {
> +		kfree(panel);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, panel);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops panel_dpi_dev_pm_ops = {
> +};
> +
> +static struct platform_driver panel_dpi_driver = {
> +	.probe = panel_dpi_probe,
> +	.remove = panel_dpi_remove,
> +	.driver = {
> +		.name = "panel_dpi",
> +		.owner = THIS_MODULE,
> +		.pm = &panel_dpi_dev_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(panel_dpi_driver);
> +
> +MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
> +MODULE_DESCRIPTION("DPI Display Panel");
> +MODULE_LICENSE("GPL");
> diff --git a/include/video/panel-dpi.h b/include/video/panel-dpi.h
> new file mode 100644
> index 0000000..0547b4a
> --- /dev/null
> +++ b/include/video/panel-dpi.h
> @@ -0,0 +1,24 @@
> +/*
> + * DPI Display Panel
> + *
> + * Copyright (C) 2012 Renesas Solutions Corp.
> + *
> + * Contacts: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __PANEL_DPI_H__
> +#define __PANEL_DPI_H__
> +
> +#include <linux/videomode.h>
> +
> +struct panel_dpi_platform_data {
> +	unsigned long width;		/* Panel width in mm */
> +	unsigned long height;		/* Panel height in mm */
> +	const struct videomode *mode;
> +};
> +
> +#endif /* __PANEL_DPI_H__ */

From: Philipp Zabel <p.zabel@pengutronix.de>
Date: Thu, 29 Nov 2012 19:18:30 +0100
Subject: [PATCH] video: panel: Add device tree support to the DPI panel
 driver

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/video/display/panel-dpi.c |   56 ++++++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 4 deletions(-)

diff --git a/drivers/video/display/panel-dpi.c b/drivers/video/display/panel-dpi.c
index c56197a..5a7dd8e 100644
--- a/drivers/video/display/panel-dpi.c
+++ b/drivers/video/display/panel-dpi.c
@@ -14,6 +14,8 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_videomode.h>
 
 #include <video/display.h>
 #include <video/panel-dpi.h>
@@ -98,20 +100,60 @@ static int panel_dpi_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static int __devinit panel_dpi_probe(struct platform_device *pdev)
+static int __devinit panel_dpi_parse_dt(struct device *dev,
+					struct panel_dpi *panel)
 {
-	const struct panel_dpi_platform_data *pdata = pdev->dev.platform_data;
-	struct panel_dpi *panel;
+	struct device_node *np = dev->of_node;
+	struct panel_dpi_platform_data *pdata;
+	struct videomode *vm;
+	u32 width, height;
 	int ret;
 
+	if (!np)
+		return -ENODEV;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 	if (pdata = NULL)
+		return -ENOMEM;
+
+	vm = devm_kzalloc(dev, sizeof(*vm), GFP_KERNEL);
+	if (vm = NULL)
+		return -ENOMEM;
+
+	ret = of_get_videomode(np, vm, 0);
+	if (ret < 0)
 		return -ENODEV;
 
+	of_property_read_u32(np, "width", &width);
+	of_property_read_u32(np, "height", &height);
+
+	pdata->mode = vm;
+	pdata->width = width;
+	pdata->height = height;
+	panel->pdata = pdata;
+
+	return 0;
+}
+
+static int __devinit panel_dpi_probe(struct platform_device *pdev)
+{
+	const struct panel_dpi_platform_data *pdata = pdev->dev.platform_data;
+	struct panel_dpi *panel;
+	int ret;
+
 	panel = kzalloc(sizeof(*panel), GFP_KERNEL);
 	if (panel = NULL)
 		return -ENOMEM;
 
-	panel->pdata = pdata;
+	if (pdata) {
+		panel->pdata = pdata;
+	} else {
+		ret = panel_dpi_parse_dt(&pdev->dev, panel);
+		if (ret < 0) {
+			kfree(panel);
+			return ret;
+		}
+	}
 	panel->entity.dev = &pdev->dev;
 	panel->entity.release = panel_dpi_release;
 	panel->entity.ops.ctrl = &panel_dpi_control_ops;
@@ -130,11 +172,17 @@ static int __devinit panel_dpi_probe(struct platform_device *pdev)
 static const struct dev_pm_ops panel_dpi_dev_pm_ops = {
 };
 
+static const struct of_device_id panel_dpi_dt_ids[] = {
+	{ .compatible = "dpi-panel", },
+	{ }
+};
+
 static struct platform_driver panel_dpi_driver = {
 	.probe = panel_dpi_probe,
 	.remove = panel_dpi_remove,
 	.driver = {
 		.name = "panel_dpi",
+		.of_match_table = panel_dpi_dt_ids,
 		.owner = THIS_MODULE,
 		.pm = &panel_dpi_dev_pm_ops,
 	},
-- 
1.7.10.4

regards
Philipp


^ permalink raw reply related

* Re: [BUGFIX PATCH] video: mxsfb: fix crash when unblanking the display
From: Juergen Beisert @ 2012-11-29 13:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20663.19785.221750.909491@ipc1.ka-ro>

Hi Lothar,

Lothar Waßmann wrote:
> > The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature.
> > The write in mxsfb_disable_controller() sets the data_cnt for the LCD
> > DMA to 0 which obviously means the max. count for the LCD DMA and
> > leads to overwriting arbitrary memory when the display is unblanked.

Overwriting? It should be a reading DMA unit, shouldn't it?

> > Note: I already sent this patch in December 2011 but noone picked it up
> > obviously.
> >
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > ---
> >  drivers/video/mxsfb.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> > index 49619b4..f2a49ef 100644
> > --- a/drivers/video/mxsfb.c
> > +++ b/drivers/video/mxsfb.c
> > @@ -369,7 +369,8 @@ static void mxsfb_disable_controller(struct fb_info
> > *fb_info) loop--;
> >  	}
> >
> > -	writel(VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4 + REG_CLR);
> > +	reg = readl(host->base + LCDC_VDCTRL4);
> > +	writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
> >
> >  	clk_disable_unprepare(host->clk);
> >
> > --
> > 1.7.2.5
>
> ping? 

I'm not sure if the missing SET/CLR feature for this register is a typo in the 
i.MX23/i.MX28 data sheets or if it is intended. Lets assume its the truth 
what they have written:

Acked-by: Juergen Beisert <jbe@pengutronix.de>

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH] video: mxsfb: fix typo: 'FAILING' instead of 'FALLING'
From: Juergen Beisert @ 2012-11-29 13:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20663.20003.18426.828567@ipc1.ka-ro>

Hi Lothar,

Lothar Waßmann wrote:
> [...]
> ping?

What a nice typo :))

You can add my:
Acked-by: Juergen Beisert <jbe@pengutronix.de>

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH] backlight: corgi_lcd: Fix WARN_ON() when calling corgi_bl_set_intensity.
From: Russell King - ARM Linux @ 2012-11-29 13:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAHod+Gd3UU7NdRLAAbsX6TUugnjKBwUsDy7yBQzyykuf5GHMVQ@mail.gmail.com>

On Thu, Nov 29, 2012 at 02:09:51PM +0100, Marko Katić wrote:
> Well, the commit message was short because i thought it was a quick
> and obvious fix.

Don't always expect the person who ends up applying your patch to know
what your patch is doing.  Don't expect people who are looking back
through the git history to look at the patch to work out whether your
commit is relevant to them.  I suspect Andrew Morton doesn't know what
a "SCOOP" or "AKITA" is...

Commit messages are there not only to describe the change, but also say
why the change is necessary.  Think about it as your chance to "sell"
the patch.

One good step would be to include the warning message dump from the
kernel (your subject line says you're hitting a WARN_ON so include
that.)  At least then people can see your starting point for the patch.

^ permalink raw reply

* Re: [PATCH] backlight: corgi_lcd: Fix WARN_ON() when calling corgi_bl_set_intensity.
From: Marko Katić @ 2012-11-29 13:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <001501cdcdf2$b3f8f730$1beae590$%han@samsung.com>

On Thu, Nov 29, 2012 at 6:30 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> On Thursday, November 29, 2012 6:47 AM, Marko Katic wrote
>> Subject: [PATCH] backlight: corgi_lcd: Fix WARN_ON() when calling corgi_bl_set_intensity.
>
> CC'ed Andrew Morton.
>
>
> Hi Marko Katic,
> The commit subject and commit message are not clear.
>
> How about using subject as below?
> backlight: corgi_lcd: use gpio_set_value_cansleep()
>
> In addition, 'Fix WARN_ON() when calling corgi_bl_set_intensity'
> would be the reason why you submit this patch.
> Please make the commit message more detail.
>
> Also, I have a question on gpio driver.
> In order to trigger WARN_ON(chip->can_sleep), 'can_sleep' should be
> set as 1. However, I cannot find 'can_sleep = 1' in the PXA gpio driver.
> What gpio driver do you use to test corgi_lcd driver?
>
>
> Best regards,
> Jingoo Han
>

Well, the commit message was short because i thought it was a quick
and obvious fix.
But it doesn't really matter now since you raise a valid point with
your question.
There are two classes of devices that use this lcd, corgi and spitz
(both are in mach-pxa tree). Both have
several variants. All of them use the SCOOP chip for backlight control
(arm/common/scoop.c) except akita which
uses the max7310 gpio expander for backlight control. The SCOOP chip
driver doesn't set can_sleep but the
max7310 does. So this patch is probably only valid for akita machines.
I'll test this further and post a revised patch
soon.

^ permalink raw reply

* Re: [PATCH 2/7] OMAPDSS: DISPC: Move DISPC specific dss_reg_fields to dispc_features
From: Tomi Valkeinen @ 2012-11-29 12:18 UTC (permalink / raw)
  To: Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <a5bbbc9af3ef5ca2636d3c4dea6cbbe9834e0f62.1354086150.git.cmahapatra@ti.com>

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

On 2012-11-28 12:41, Chandrabhanu Mahapatra wrote:
> The register fields in dss_reg_fields specific to DISPC are moved from struct
> omap_dss_features to corresponding dispc_reg_fields, initialized in struct
> dispc_features, thereby enabling local access.
> 
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
>  drivers/video/omap2/dss/dispc.c        |   87 ++++++++++++++++++++++++++++----
>  drivers/video/omap2/dss/dss.h          |    4 ++
>  drivers/video/omap2/dss/dss_features.c |   28 ----------
>  drivers/video/omap2/dss/dss_features.h |    7 ---
>  4 files changed, 80 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 9f259ba..21fc522 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -80,6 +80,16 @@ struct dispc_irq_stats {
>  	unsigned irqs[32];
>  };
>  
> +enum dispc_feat_reg_field {
> +	FEAT_REG_FIRHINC,
> +	FEAT_REG_FIRVINC,
> +	FEAT_REG_FIFOLOWTHRESHOLD,
> +	FEAT_REG_FIFOHIGHTHRESHOLD,
> +	FEAT_REG_FIFOSIZE,
> +	FEAT_REG_HORIZONTALACCU,
> +	FEAT_REG_VERTICALACCU,
> +};
> +
>  struct dispc_features {
>  	u8 sw_start;
>  	u8 fp_start;
> @@ -107,6 +117,8 @@ struct dispc_features {
>  
>  	u32 buffer_size_unit;
>  	u32 burst_size_unit;
> +
> +	struct register_field *reg_fields;
>  };

Hmm, would it be simpler to have an explicit struct for the reg fields.
I mean something like:

struct dispc_reg_fields {
	struct register_field firhinc;
	struct register_field firvinc;
	struct register_field fifo_low_threshold;
	...
};

Then accessing it would be

dispc.feat->reg_fields.firhinc.start;

instead of

dispc.feat->reg_fields[FEAT_REG_FIFOSIZE].start;

Not a big difference, but I don't see any benefit in having an array of
reg fields here.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 3/7] OMAPDSS: DISPC: Move DISPC specific dss_params to dispc_features
From: Tomi Valkeinen @ 2012-11-29 12:08 UTC (permalink / raw)
  To: Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <a8148bcef07c0f8c7d2b385c1016f4bfaa90a772.1354086150.git.cmahapatra@ti.com>

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

On 2012-11-28 12:41, Chandrabhanu Mahapatra wrote:
> The DISPC specific dss_param_range are moved from struct omap_dss_features to
> corresponding DSS version specific dispc_param_range struct, initialized in
> dispc_features thereby enabling local access. The mgr_width_max and
> mgr_height_max, members of dispc_features, are also moved to dispc_param_range.
> 
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
>  drivers/video/omap2/dss/dispc.c        |   80 +++++++++++++++++++++++---------
>  drivers/video/omap2/dss/dss.h          |    4 ++
>  drivers/video/omap2/dss/dss_features.c |   16 -------
>  drivers/video/omap2/dss/dss_features.h |    3 --
>  4 files changed, 63 insertions(+), 40 deletions(-)

I think the exact same comments, as to the previous patch, apply here.
Instead of creating param_range, use the dss_param_range. And see if

dispc.feat->params[FEAT_PARAM_PCD].min

style lines can be simplified with helper func/pointer.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 2/7] OMAPDSS: DISPC: Move DISPC specific dss_reg_fields to dispc_features
From: Tomi Valkeinen @ 2012-11-29 12:05 UTC (permalink / raw)
  To: Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <a5bbbc9af3ef5ca2636d3c4dea6cbbe9834e0f62.1354086150.git.cmahapatra@ti.com>

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

On 2012-11-28 12:41, Chandrabhanu Mahapatra wrote:
> The register fields in dss_reg_fields specific to DISPC are moved from struct
> omap_dss_features to corresponding dispc_reg_fields, initialized in struct
> dispc_features, thereby enabling local access.
> 
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
>  drivers/video/omap2/dss/dispc.c        |   87 ++++++++++++++++++++++++++++----
>  drivers/video/omap2/dss/dss.h          |    4 ++
>  drivers/video/omap2/dss/dss_features.c |   28 ----------
>  drivers/video/omap2/dss/dss_features.h |    7 ---
>  4 files changed, 80 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 9f259ba..21fc522 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -80,6 +80,16 @@ struct dispc_irq_stats {
>  	unsigned irqs[32];
>  };
>  
> +enum dispc_feat_reg_field {
> +	FEAT_REG_FIRHINC,
> +	FEAT_REG_FIRVINC,
> +	FEAT_REG_FIFOLOWTHRESHOLD,
> +	FEAT_REG_FIFOHIGHTHRESHOLD,
> +	FEAT_REG_FIFOSIZE,
> +	FEAT_REG_HORIZONTALACCU,
> +	FEAT_REG_VERTICALACCU,
> +};
> +
>  struct dispc_features {
>  	u8 sw_start;
>  	u8 fp_start;
> @@ -107,6 +117,8 @@ struct dispc_features {
>  
>  	u32 buffer_size_unit;
>  	u32 burst_size_unit;
> +
> +	struct register_field *reg_fields;
>  };
>  
>  #define DISPC_MAX_NR_FIFOS 5
> @@ -1150,7 +1162,8 @@ static void dispc_init_fifos(void)
>  
>  	unit = dispc.feat->buffer_size_unit;
>  
> -	dss_feat_get_reg_field(FEAT_REG_FIFOSIZE, &start, &end);
> +	start = dispc.feat->reg_fields[FEAT_REG_FIFOSIZE].start;
> +	end = dispc.feat->reg_fields[FEAT_REG_FIFOSIZE].end;
>  
>  	for (fifo = 0; fifo < dispc.feat->num_fifos; ++fifo) {
>  		size = REG_GET(DISPC_OVL_FIFO_SIZE_STATUS(fifo), start, end);
> @@ -1214,8 +1227,10 @@ void dispc_ovl_set_fifo_threshold(enum omap_plane plane, u32 low, u32 high)
>  	low /= unit;
>  	high /= unit;
>  
> -	dss_feat_get_reg_field(FEAT_REG_FIFOHIGHTHRESHOLD, &hi_start, &hi_end);
> -	dss_feat_get_reg_field(FEAT_REG_FIFOLOWTHRESHOLD, &lo_start, &lo_end);
> +	hi_start = dispc.feat->reg_fields[FEAT_REG_FIFOHIGHTHRESHOLD].start;
> +	hi_end = dispc.feat->reg_fields[FEAT_REG_FIFOHIGHTHRESHOLD].end;
> +	lo_start = dispc.feat->reg_fields[FEAT_REG_FIFOLOWTHRESHOLD].start;
> +	lo_end = dispc.feat->reg_fields[FEAT_REG_FIFOLOWTHRESHOLD].end;

I think these are quite verbose. Perhaps a helper function which does
the same as dss_feat_get_reg_field() would be better. Or alternatively,
maybe something like:

const struct dss_reg_field *hi_field =
&dispc.feat->reg_fields[FEAT_REG_FIFOHIGHTHRESHOLD];

and then use "hi_field.start" instead of hi_start.

Or something else that makes the above easier to read.

> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> index 84a7f6a..aa273d8 100644
> --- a/drivers/video/omap2/dss/dss.h
> +++ b/drivers/video/omap2/dss/dss.h
> @@ -143,6 +143,10 @@ struct reg_field {
>  	u8 low;
>  };
>  
> +struct register_field {
> +	u8 start, end;
> +};
> +

We already have the dss_reg_field struct. I think it's better to move
that to dss.h, and use it, instead of creating an exact duplicate.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 1/7] OMAPDSS: DISPC: Move burst_size and buffer_size to dispc_features
From: Tomi Valkeinen @ 2012-11-29 12:01 UTC (permalink / raw)
  To: Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <c40e58eec3b2100368c696db05a68d49d49f1f0d.1354086150.git.cmahapatra@ti.com>

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

On 2012-11-28 12:41, Chandrabhanu Mahapatra wrote:
> The burst_size and buffer_size being local data to DISPC are moved to
> dispc_features and so removed from struct omap_dss_features. The functions
> referring to burst and buffer size are also removed from dss_features.c as they
> are now accessed locally in dispc.c.
> 
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
>  drivers/video/omap2/dss/dispc.c        |   21 +++++++++++++++++----
>  drivers/video/omap2/dss/dss_features.c |   29 -----------------------------
>  drivers/video/omap2/dss/dss_features.h |    3 ---
>  3 files changed, 17 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 3d0ff5b..9f259ba 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -104,6 +104,9 @@ struct dispc_features {
>  
>  	/* swap GFX & WB fifos */
>  	bool gfx_fifo_workaround:1;
> +
> +	u32 buffer_size_unit;
> +	u32 burst_size_unit;

Can you add the comments "in bytes" to these. Otherwise this looks good.

 Tomi



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

^ permalink raw reply

* Re: [PATCH] video: mxsfb: fix typo: 'FAILING' instead of 'FALLING'
From: Lothar Waßmann @ 2012-11-29 11:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1353588555-13168-2-git-send-email-LW@KARO-electronics.de>

Hi,

adding Juergen Beisert as the driver's author to the recipient list.

Lothar Waßmann writes:
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
>  arch/arm/mach-mxs/mach-mxs.c |    6 +++---
>  drivers/video/mxsfb.c        |    6 +++---
>  include/linux/mxsfb.h        |    2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
> index d61b915..6c17cb7 100644
> --- a/arch/arm/mach-mxs/mach-mxs.c
> +++ b/arch/arm/mach-mxs/mach-mxs.c
> @@ -42,7 +42,7 @@ static struct fb_videomode mx23evk_video_modes[] = {
>  		.hsync_len	= 1,
>  		.vsync_len	= 1,
>  		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT |
> -				  FB_SYNC_DOTCLK_FAILING_ACT,
> +				  FB_SYNC_DOTCLK_FALLING_ACT,
>  	},
>  };
>  
> @@ -60,7 +60,7 @@ static struct fb_videomode mx28evk_video_modes[] = {
>  		.hsync_len	= 10,
>  		.vsync_len	= 10,
>  		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT |
> -				  FB_SYNC_DOTCLK_FAILING_ACT,
> +				  FB_SYNC_DOTCLK_FALLING_ACT,
>  	},
>  };
>  
> @@ -96,7 +96,7 @@ static struct fb_videomode apx4devkit_video_modes[] = {
>  		.vsync_len	= 3,
>  		.sync		= FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT |
>  				  FB_SYNC_DATA_ENABLE_HIGH_ACT |
> -				  FB_SYNC_DOTCLK_FAILING_ACT,
> +				  FB_SYNC_DOTCLK_FALLING_ACT,
>  	},
>  };
>  
> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> index f2a49ef..eb45d32 100644
> --- a/drivers/video/mxsfb.c
> +++ b/drivers/video/mxsfb.c
> @@ -107,7 +107,7 @@
>  #define VDCTRL0_ENABLE_PRESENT		(1 << 28)
>  #define VDCTRL0_VSYNC_ACT_HIGH		(1 << 27)
>  #define VDCTRL0_HSYNC_ACT_HIGH		(1 << 26)
> -#define VDCTRL0_DOTCLK_ACT_FAILING	(1 << 25)
> +#define VDCTRL0_DOTCLK_ACT_FALLING	(1 << 25)
>  #define VDCTRL0_ENABLE_ACT_HIGH		(1 << 24)
>  #define VDCTRL0_VSYNC_PERIOD_UNIT	(1 << 21)
>  #define VDCTRL0_VSYNC_PULSE_WIDTH_UNIT	(1 << 20)
> @@ -458,8 +458,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
>  		vdctrl0 |= VDCTRL0_VSYNC_ACT_HIGH;
>  	if (fb_info->var.sync & FB_SYNC_DATA_ENABLE_HIGH_ACT)
>  		vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH;
> -	if (fb_info->var.sync & FB_SYNC_DOTCLK_FAILING_ACT)
> -		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FAILING;
> +	if (fb_info->var.sync & FB_SYNC_DOTCLK_FALLING_ACT)
> +		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FALLING;
>  
>  	writel(vdctrl0, host->base + LCDC_VDCTRL0);
>  
> diff --git a/include/linux/mxsfb.h b/include/linux/mxsfb.h
> index f14943d..a9eca68 100644
> --- a/include/linux/mxsfb.h
> +++ b/include/linux/mxsfb.h
> @@ -25,7 +25,7 @@
>  #define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24 bit width */
>  
>  #define FB_SYNC_DATA_ENABLE_HIGH_ACT	(1 << 6)
> -#define FB_SYNC_DOTCLK_FAILING_ACT	(1 << 7) /* failing/negtive edge sampling */
> +#define FB_SYNC_DOTCLK_FALLING_ACT	(1 << 7) /* falling/negative edge sampling */
>  
>  struct mxsfb_platform_data {
>  	struct fb_videomode *mode_list;
> -- 
> 1.7.2.5
> 
ping?


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply

* Re: [BUGFIX PATCH] video: mxsfb: fix crash when unblanking the display
From: Lothar Waßmann @ 2012-11-29 11:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1353588555-13168-1-git-send-email-LW@KARO-electronics.de>

Hi,

adding Juergen Beisert as the driver's author to the recipient list.

Lothar Waßmann writes:
> The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature.
> The write in mxsfb_disable_controller() sets the data_cnt for the LCD
> DMA to 0 which obviously means the max. count for the LCD DMA and
> leads to overwriting arbitrary memory when the display is unblanked.
> 
> Note: I already sent this patch in December 2011 but noone picked it up obviously.
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
>  drivers/video/mxsfb.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> index 49619b4..f2a49ef 100644
> --- a/drivers/video/mxsfb.c
> +++ b/drivers/video/mxsfb.c
> @@ -369,7 +369,8 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
>  		loop--;
>  	}
>  
> -	writel(VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4 + REG_CLR);
> +	reg = readl(host->base + LCDC_VDCTRL4);
> +	writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
>  
>  	clk_disable_unprepare(host->clk);
>  
> -- 
> 1.7.2.5
> 
ping?


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply

* Re: [PATCH] video: da8xx-fb: clk_get on connection id fck
From: Tomi Valkeinen @ 2012-11-29  8:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <A73F36158E33644199EB82C5EC81C7BC3E9FB8D9@DBDE01.ent.ti.com>

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

On 2012-11-23 06:33, Manjunathappa, Prakash wrote:
> Hi Tomi,
> 
> On Tue, Nov 20, 2012 at 18:32:54, Nori, Sekhar wrote:
>> On 11/20/2012 6:11 PM, Manjunathappa wrote:
>>> do clk_get on connection id "fck" to support OMAP based
>>> platforms having multiple clocks for module. Without this
>>> driver change clk_get fails on am335x.
>>>
>>> This patch is based on the discussion in community
>>> http://marc.info/?l=linux-kernel&m=135166018907827&w=2
>>>
>>> Signed-off-by: Manjunathappa <prakash.pm@ti.com>
>>> Cc: Vaibhav Hiremath <hvaibhav@ti.com>
>>
>> For the mach-davinci changes:
>>
>> Acked-by: Sekhar Nori <nsekhar@ti.com>
>>
>> Florian,
>>
>> I assume you will want to take this through your tree?
>>
> 
> Could you please pull in this?

Merged this and the "da8xx-fb: LCDC driver cleanup" to:

git://gitorious.org/linux-omap-dss2/linux.git for-next

 Tomi



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

^ permalink raw reply

* Re: [GIT PULL] Exynos DP for v3.8
From: Tomi Valkeinen @ 2012-11-29  8:41 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <28517517.77811353652672706.JavaMail.weblogic@epml06>

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

On 2012-11-23 08:37, Jingoo Han wrote:
> Hi Tomi,
> 
> Thank you for taking care of pull requests for the v3.8 merge window.
> These patches have been submitted for more than one month,
> and tested with Exynos5250.
> 
> The following changes since commit f4a75d2eb7b1e2206094b901be09adb31ba63681:
> 
>   Linux 3.7-rc6 (Fri Nov 16 17:42:40 2012 -0800)
> 
> are available in the git repository at:
>   git://github.com/jingoo/linux.git exynos-dp-next

Pulled to:

git://gitorious.org/linux-omap-dss2/linux.git for-next

 Tomi



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

^ permalink raw reply

* Re: [GIT PULL] Samsung Framebuffer for v3.8
From: Tomi Valkeinen @ 2012-11-29  8:41 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <16376487.77671353652569048.JavaMail.weblogic@epml06>

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

On 2012-11-23 08:36, Jingoo Han wrote:
> Hi Tomi,
> 
> Thank you for taking care of pull requests for the v3.8 merge window.
> These patches have been submitted for about two months,
> and tested with Exynos4210 and Exynos5250.
> 
> The following changes since commit f4a75d2eb7b1e2206094b901be09adb31ba63681:
> 
>   Linux 3.7-rc6 (Fri Nov 16 17:42:40 2012 -0800)
> 
> are available in the git repository at:
>   git://github.com/jingoo/linux.git samsung-fb-next

Pulled to:

git://gitorious.org/linux-omap-dss2/linux.git for-next

 Tomi



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

^ permalink raw reply


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