* Re: [PATCH v2] of: Add videomode helper
From: Laurent Pinchart @ 2012-08-08 12:41 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20120705165029.GU30009-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi Sascha,
Sorry for the late reply.
On Thursday 05 July 2012 18:50:29 Sascha Hauer wrote:
> On Thu, Jul 05, 2012 at 04:08:07PM +0200, Laurent Pinchart wrote:
> > > +++ b/Documentation/devicetree/bindings/video/displaymode
> > > @@ -0,0 +1,40 @@
> > > +videomode bindings
> > > +=========
> > > +
> > > +Required properties:
> > > + - xres, yres: Display resolution
> > > + - left-margin, right-margin, hsync-len: Horizontal Display timing
> > > parameters + in pixels
> > > + upper-margin, lower-margin, vsync-len: Vertical display timing
> > > parameters in + lines
> > > + - clock: displayclock in Hz
> > > +
> > > +Optional properties:
> > > + - width-mm, height-mm: Display dimensions in mm
> >
> > I've always had mixed feelings about the physical display dimension being
> > part of the display mode. Those are properties of the panel/display
> > instead of the mode. Storing them as part of the mode can be convenient,
> > but we then run into consistency issues (developers have to remember in
> > which display mode instances the values are available, and in which
> > instances they're set to 0 for instance). If we want to clean this up,
> > this patch would be a good occasion.
>
> This sounds like a display node with one or more node subnodes, like:
>
> display {
> width_mm = <>;
> height_mm = <>;
> mode {
> xres = <>;
> yres = <>;
> ...
> };
> };
>
> Is that what you mean or are you thinking of something else?
Yes, that's exactly what I meant.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v2] of: Add videomode helper
From: Laurent Pinchart @ 2012-08-08 12:40 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org
In-Reply-To: <20120803073844.GK1451-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi Sascha,
On Friday 03 August 2012 09:38:44 Sascha Hauer wrote:
> On Thu, Aug 02, 2012 at 01:35:40PM -0600, Stephen Warren wrote:
> > On 07/04/2012 01:56 AM, Sascha Hauer wrote:
> > > This patch adds a helper function for parsing videomodes from the
> > > devicetree. The videomode can be either converted to a struct
> > > drm_display_mode or a struct fb_videomode.
> > >
> > > diff --git a/Documentation/devicetree/bindings/video/displaymode
> > > b/Documentation/devicetree/bindings/video/displaymode
> > >
> > > +Required properties:
> > > + - xres, yres: Display resolution
> > > + - left-margin, right-margin, hsync-len: Horizontal Display timing
> > > parameters + in pixels
> > > + upper-margin, lower-margin, vsync-len: Vertical display timing
> > > parameters in + lines
> >
> > Perhaps bike-shedding, but...
> >
> > For the margin property names, wouldn't it be better to use terminology
> > more commonly used for video timings rather than Linux FB naming. In
> > other words naming like:
> >
> > hactive, hsync-len, hfront-porch, hback-porch?
>
> Can do. Just to make sure:
>
> hactive = xres
> hsync-len = hsync-len
> hfront-porch = right-margin
> hback-porch = left-margin
That's correct. On the vertical direction, vfront-porch = lower-margin and
vback-porch = upper-margin.
>
> ?
>
> > This node appears to describe a video mode, not a display, hence the
> > node name seems wrong.
> >
> > Many displays can support multiple different video modes. As mentioned
> > elsewhere, properties like display width/height are properties of the
> > display not the mode.
> >
> > So, I might expect something more like the following (various overhead
> > properties like reg/#address-cells etc. elided for simplicity):
> >
> > disp: display {
> >
> > width-mm = <...>;
> > height-mm = <...>;
> > modes {
> >
> > mode@0 {
> >
> > /* 1920x1080p24 */
> > clock = <52000000>;
> > xres = <1920>;
> > yres = <1080>;
> > left-margin = <25>;
> > right-margin = <25>;
> > hsync-len = <25>;
> > lower-margin = <2>;
> > upper-margin = <2>;
> > vsync-len = <2>;
> > hsync-active-high;
> >
> > };
> > mode@1 {
> > };
> >
> > };
> >
> > };
>
> Ok, we can do this.
>
> > display-connector {
> >
> > display = <&disp>;
> > // interface-specific properties such as pixel format here
> >
> > };
> >
> > Finally, have you considered just using an EDID instead of creating
> > something custom? I know that creating an EDID is harder than writing a
>
> > few simple properties into a DT, but EDIDs have the following advantages:
> I have considered using EDID and I also tried it. It's painful. There
> are no (open) tools available for creating EDID. That's something we
> could change of course. Then when generating a devicetree there is
> always an extra step involved creating the EDID blob. Once the EDID
> blob is in devicetree it is not parsable anymore by mere humans, so
> to see what we've got there is another tool involved to generate a
> readable form again.
>
> > a) They're already standardized and very common.
>
> Indeed, that's a big plus for EDID. I have no intention of removing EDID
> data from the devicetree. There are situations where EDID is handy, for
> example when you get EDID data provided by your vendor.
>
> > b) They allow other information such as a display's HDMI audio
> > capabilities to be represented, which can then feed into an ALSA driver.
> >
> > c) The few LCD panel datasheets I've seen actually quote their
> > specification as an EDID already, so deriving the EDID may actually be
> > easy.
> >
> > d) People familiar with displays are almost certainly familiar with
> > EDID's mode representation. There are many ways of representing display
> > modes (sync position vs. porch widths, htotal specified rather than
> > specifying all the components and hence htotal being calculated etc.).
> > Not everyone will be familiar with all representations. Conversion
> > errors are less likely if the target is EDID's familiar format.
>
> You seem to think about a different class of displays for which EDID
> indeed is a better way to handle. What I have to deal with here mostly
> are dumb displays which:
>
> - can only handle their native resolution
> - Have no audio capabilities at all
> - come with a datasheet which specify a min/typ/max triplet for
> xres,hsync,..., often enough they are scanned to pdf from some previously
> printed paper.
>
> These displays are very common on embedded devices, probably that's the
> reason I did not even think about the possibility that a single display
> might have different modes.
>
> > e) You'll end up with exactly the same data as if you have a DDC-based
> > external monitor rather than an internal panel, so you end up getting to
> > a common path in display handling code much more quickly.
>
> All we have in our display driver currently is:
>
> edidp = of_get_property(np, "edid", &imxpd->edid_len);
> if (edidp) {
> imxpd->edid = kmemdup(edidp, imxpd->edid_len, GFP_KERNEL);
> } else {
> ret = of_get_video_mode(np, &imxpd->mode, NULL);
> if (!ret)
> imxpd->mode_valid = 1;
> }
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 1/6] OMAPDSS: DISPC: cleanup cpu_is_xxxx checks
From: Tomi Valkeinen @ 2012-08-08 12:36 UTC (permalink / raw)
To: Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1344425874-28222-1-git-send-email-cmahapatra@ti.com>
[-- Attachment #1: Type: text/plain, Size: 7991 bytes --]
On Wed, 2012-08-08 at 17:07 +0530, Chandrabhanu Mahapatra wrote:
> All the cpu_is checks have been moved to dispc_init_features function providing
> a much more generic and cleaner interface. The OMAP version and revision
> specific functions are initialized by dispc_features structure local to dispc.c.
>
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
> drivers/video/omap2/dss/dispc.c | 476 ++++++++++++++++++++++++++-------------
> 1 file changed, 315 insertions(+), 161 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 5b289c5..7e0b080 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -75,12 +75,60 @@ enum omap_burst_size {
> #define REG_FLD_MOD(idx, val, start, end) \
> dispc_write_reg(idx, FLD_MOD(dispc_read_reg(idx), val, start, end))
>
> +static int dispc_ovl_calc_scaling_24xx(enum omap_channel channel,
> + const struct omap_video_timings *mgr_timings, u16 width, u16 height,
> + u16 out_width, u16 out_height, enum omap_color_mode color_mode,
> + bool *five_taps, int *x_predecim, int *y_predecim, int *decim_x,
> + int *decim_y, u16 pos_x, unsigned long *core_clk);
> +static int dispc_ovl_calc_scaling_34xx(enum omap_channel channel,
> + const struct omap_video_timings *mgr_timings, u16 width, u16 height,
> + u16 out_width, u16 out_height, enum omap_color_mode color_mode,
> + bool *five_taps, int *x_predecim, int *y_predecim, int *decim_x,
> + int *decim_y, u16 pos_x, unsigned long *core_clk);
> +static int dispc_ovl_calc_scaling_44xx(enum omap_channel channel,
> + const struct omap_video_timings *mgr_timings, u16 width, u16 height,
> + u16 out_width, u16 out_height, enum omap_color_mode color_mode,
> + bool *five_taps, int *x_predecim, int *y_predecim, int *decim_x,
> + int *decim_y, u16 pos_x, unsigned long *core_clk);
> +
> +static unsigned long calc_core_clk_24xx(enum omap_channel channel, u16 width,
> + u16 height, u16 out_width, u16 out_height);
> +static unsigned long calc_core_clk_34xx(enum omap_channel channel, u16 width,
> + u16 height, u16 out_width, u16 out_height);
> +static unsigned long calc_core_clk_44xx(enum omap_channel channel, u16 width,
> + u16 height, u16 out_width, u16 out_height);
> +
> +static bool _dispc_lcd_timings_ok_24xx(int hsw, int hfp, int hbp,
> + int vsw, int vfp, int vbp);
> +static bool _dispc_lcd_timings_ok_44xx(int hsw, int hfp, int hbp,
> + int vsw, int vfp, int vbp);
> +
> +static void _dispc_mgr_set_lcd_timings_hv_24xx(enum omap_channel channel,
> + int hsw, int hfp, int hbp, int vsw, int vfp, int vbp);
> +static void _dispc_mgr_set_lcd_timings_hv_44xx(enum omap_channel channel,
> + int hsw, int hfp, int hbp, int vsw, int vfp, int vbp);
While it's nice to have the initialization of struct dispc_features in
the beginning of dispc.c, it requires the above prototypes. And in the
future we may require more. For that reason I think it's better to
initialize the dispc_features at the end of dispc.c, just above
dispc_init_features(). This would be kinda similar to how drivers often
initialize their ops.
> +static const struct dispc_features omap2_dispc_features = {
> + .calc_scaling = dispc_ovl_calc_scaling_24xx,
> + .calc_core_clk = calc_core_clk_24xx,
> + .lcd_timings_ok = _dispc_lcd_timings_ok_24xx,
> + .set_lcd_timings_hv = _dispc_mgr_set_lcd_timings_hv_24xx,
> +};
> +
> +static const struct dispc_features omap3_2_1_dispc_features = {
> + .calc_scaling = dispc_ovl_calc_scaling_34xx,
> + .calc_core_clk = calc_core_clk_34xx,
> + .lcd_timings_ok = _dispc_lcd_timings_ok_24xx,
> + .set_lcd_timings_hv = _dispc_mgr_set_lcd_timings_hv_24xx,
> +};
> +
> +static const struct dispc_features omap3_3_0_dispc_features = {
> + .calc_scaling = dispc_ovl_calc_scaling_34xx,
> + .calc_core_clk = calc_core_clk_34xx,
> + .lcd_timings_ok = _dispc_lcd_timings_ok_44xx,
> + .set_lcd_timings_hv = _dispc_mgr_set_lcd_timings_hv_44xx,
> +};
> +
> +static const struct dispc_features omap4_dispc_features = {
> + .calc_scaling = dispc_ovl_calc_scaling_44xx,
> + .calc_core_clk = calc_core_clk_44xx,
> + .lcd_timings_ok = _dispc_lcd_timings_ok_44xx,
> + .set_lcd_timings_hv = _dispc_mgr_set_lcd_timings_hv_44xx,
> +};
During runtime we only require one of these, others can be discarded.
This can be accomplished with the combination of "__initdata" for these,
and "__init" for dispc_init_features().
However, because even the one we need will be discarded, we need to copy
the values. This could be done either by having the dispc_features
struct inside dispc struct (instead of a pointer), or allocating memory
for it with devm_kzalloc(). The latter allows us to keep it const, but
I'm not sure which approach is better (if either).
> -static bool _dispc_lcd_timings_ok(int hsw, int hfp, int hbp,
> +static bool _dispc_lcd_timings_ok_24xx(int hsw, int hfp, int hbp,
> int vsw, int vfp, int vbp)
> {
> - if (cpu_is_omap24xx() || omap_rev() < OMAP3430_REV_ES3_0) {
> - if (hsw < 1 || hsw > 64 ||
> - hfp < 1 || hfp > 256 ||
> - hbp < 1 || hbp > 256 ||
> - vsw < 1 || vsw > 64 ||
> - vfp < 0 || vfp > 255 ||
> - vbp < 0 || vbp > 255)
> - return false;
> - } else {
> - if (hsw < 1 || hsw > 256 ||
> - hfp < 1 || hfp > 4096 ||
> - hbp < 1 || hbp > 4096 ||
> - vsw < 1 || vsw > 256 ||
> - vfp < 0 || vfp > 4095 ||
> - vbp < 0 || vbp > 4095)
> - return false;
> - }
> -
> + if (hsw < 1 || hsw > 64 ||
> + hfp < 1 || hfp > 256 ||
> + hbp < 1 || hbp > 256 ||
> + vsw < 1 || vsw > 64 ||
> + vfp < 0 || vfp > 255 ||
> + vbp < 0 || vbp > 255)
> + return false;
> + return true;
> +}
> +static bool _dispc_lcd_timings_ok_44xx(int hsw, int hfp, int hbp,
> + int vsw, int vfp, int vbp)
> +{
> + if (hsw < 1 || hsw > 256 ||
> + hfp < 1 || hfp > 4096 ||
> + hbp < 1 || hbp > 4096 ||
> + vsw < 1 || vsw > 256 ||
> + vfp < 0 || vfp > 4095 ||
> + vbp < 0 || vbp > 4095)
> + return false;
> return true;
> }
I think we should use separate functions only when the code is
different. Here the code is the same, we just use different max values.
So instead of these functions, I suggest to add those max values into
struct dispc_features.
> @@ -2633,7 +2757,8 @@ bool dispc_mgr_timings_ok(enum omap_channel channel,
> timings_ok = _dispc_mgr_size_ok(timings->x_res, timings->y_res);
>
> if (dss_mgr_is_lcd(channel))
> - timings_ok = timings_ok && _dispc_lcd_timings_ok(timings->hsw,
> + timings_ok = timings_ok &&
> + dispc.feat->lcd_timings_ok(timings->hsw,
> timings->hfp, timings->hbp,
> timings->vsw, timings->vfp,
> timings->vbp);
> @@ -2641,6 +2766,34 @@ bool dispc_mgr_timings_ok(enum omap_channel channel,
> return timings_ok;
> }
>
> +static void _dispc_mgr_set_lcd_timings_hv_24xx(enum omap_channel channel,
> + int hsw, int hfp, int hbp, int vsw, int vfp, int vbp)
> +{
> + u32 timing_h, timing_v;
> +
> + timing_h = FLD_VAL(hsw-1, 5, 0) | FLD_VAL(hfp-1, 15, 8) |
> + FLD_VAL(hbp-1, 27, 20);
> + timing_v = FLD_VAL(vsw-1, 5, 0) | FLD_VAL(vfp, 15, 8) |
> + FLD_VAL(vbp, 27, 20);
> +
> + dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
> + dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
> +}
> +
> +static void _dispc_mgr_set_lcd_timings_hv_44xx(enum omap_channel channel,
> + int hsw, int hfp, int hbp, int vsw, int vfp, int vbp)
> +{
> + u32 timing_h, timing_v;
> +
> + timing_h = FLD_VAL(hsw-1, 7, 0) | FLD_VAL(hfp-1, 19, 8) |
> + FLD_VAL(hbp-1, 31, 20);
> + timing_v = FLD_VAL(vsw-1, 7, 0) | FLD_VAL(vfp, 19, 8) |
> + FLD_VAL(vbp, 31, 20);
> +
> + dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
> + dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
> +}
Same thing here. The code is the same, only the bit fields are larger.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH] fbdev: Add Renesas vdc4 framebuffer driver
From: Phil Edworthy @ 2012-08-08 12:14 UTC (permalink / raw)
To: linux-fbdev
The vdc4 display hardware is found on the sh7269 device.
Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
drivers/video/Kconfig | 10 +
drivers/video/Makefile | 1 +
drivers/video/ren_vdc4fb.c | 653 ++++++++++++++++++++++++++++++++++++++++++++
include/video/ren_vdc4fb.h | 19 ++
4 files changed, 683 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/ren_vdc4fb.c
create mode 100644 include/video/ren_vdc4fb.h
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 0217f74..89c9250 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1990,6 +1990,16 @@ config FB_W100
If unsure, say N.
+config FB_REN_VDC4FB
+ tristate "Renesas VDC4 framebuffer support"
+ depends on FB && CPU_SUBTYPE_SH7269
+ select FB_SYS_FILLRECT
+ select FB_SYS_COPYAREA
+ select FB_SYS_IMAGEBLIT
+ select FB_SYS_FOPS
+ ---help---
+ Frame buffer driver for the Renesas VDC4.
+
config FB_SH_MOBILE_LCDC
tristate "SuperH Mobile LCDC framebuffer support"
depends on FB && (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index ee8dafb..ba69fcb 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -140,6 +140,7 @@ obj-$(CONFIG_SH_MIPI_DSI) += sh_mipi_dsi.o
obj-$(CONFIG_FB_SH_MOBILE_HDMI) += sh_mobile_hdmi.o
obj-$(CONFIG_FB_SH_MOBILE_MERAM) += sh_mobile_meram.o
obj-$(CONFIG_FB_SH_MOBILE_LCDC) += sh_mobile_lcdcfb.o
+obj-$(CONFIG_FB_REN_VDC4FB) += ren_vdc4fb.o
obj-$(CONFIG_FB_OMAP) += omap/
obj-y += omap2/
obj-$(CONFIG_XEN_FBDEV_FRONTEND) += xen-fbfront.o
diff --git a/drivers/video/ren_vdc4fb.c b/drivers/video/ren_vdc4fb.c
new file mode 100644
index 0000000..1a31e85
--- /dev/null
+++ b/drivers/video/ren_vdc4fb.c
@@ -0,0 +1,653 @@
+/*
+ * Renesas VDC4 Framebuffer
+ *
+ * Based on sh_mobile_lcdcfb.c
+ * Copyright (c) 2012 Renesas Electronics Europe Ltd
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/mm.h>
+#include <linux/clk.h>
+#include <linux/sh_clk.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/vmalloc.h>
+#include <linux/module.h>
+#include <video/ren_vdc4fb.h>
+
+#define PALETTE_NR 16
+
+struct ren_vdc4_priv {
+ void __iomem *base;
+ int irq;
+ struct clk *dot_clk;
+ struct clk *clk;
+ struct fb_info *info;
+ dma_addr_t dma_handle;
+ struct ren_vdc4_info *cfg;
+ u32 pseudo_palette[PALETTE_NR];
+};
+
+/* Register offsets/reading and writing functions */
+enum {
+ SCL0_UPDATE, SCL0_FRC1, SCL0_FRC2, SCL0_FRC3,
+ SCL0_FRC4, SCL0_FRC5, SCL0_FRC6, SCL0_FRC7,
+ SCL0_DS1, SCL0_US1,
+
+ GR1_UPDATE, GR1_AB1,
+
+ GR2_UPDATE, GR2_AB1,
+
+ GR3_UPDATE, GR3_FLM_RD, GR3_FLM1, GR3_FLM2,
+ GR3_FLM3, GR3_FLM4, GR3_FLM5, GR3_FLM6, GR3_AB1,
+ GR3_AB2, GR3_AB3, GR3_AB4, GR3_AB5, GR3_AB6,
+ GR3_AB7, GR3_AB8, GR3_AB9, GR3_AB10, GR3_AB11,
+ GR3_BASE, GR3_CLUT_INT, GR3_MON,
+
+ TCON_UPDATE, TCON_TIM, TCON_TIM_STVA1, TCON_TIM_STVA2,
+ TCON_TIM_STVB1, TCON_TIM_STVB2, TCON_TIM_STH1,
+ TCON_TIM_STH2, TCON_TIM_STB1, TCON_TIM_STB2,
+ TCON_TIM_CPV1, TCON_TIM_CPV2, TCON_TIM_POLA1,
+ TCON_TIM_POLA2, TCON_TIM_POLB1, TCON_TIM_POLB2,
+ TCON_TIM_DE,
+
+ OUT_UPDATE, OUT_SET, OUT_BRIGHT1,
+ OUT_BRIGHT2, OUT_CONTRAST, OUT_PDTHA, OUT_CLK_PHASE,
+
+ SYSCNT_INT1, SYSCNT_INT2, SYSCNT_INT3, SYSCNT_INT4,
+ SYSCNT_PANEL_CLK, SYSCNT_CLUT
+};
+
+static unsigned long vdc4_offsets[] = {
+ [SCL0_UPDATE] = 0x0100,
+ [SCL0_FRC1] = 0x0104,
+ [SCL0_FRC2] = 0x0108,
+ [SCL0_FRC3] = 0x010C,
+ [SCL0_FRC4] = 0x0110,
+ [SCL0_FRC5] = 0x0114,
+ [SCL0_FRC6] = 0x0118,
+ [SCL0_FRC7] = 0x011C,
+ [SCL0_DS1] = 0x012C,
+ [SCL0_US1] = 0x0148,
+ [GR1_UPDATE] = 0x0200,
+ [GR1_AB1] = 0x0220,
+ [GR2_UPDATE] = 0x0300,
+ [GR2_AB1] = 0x0320,
+ [GR3_UPDATE] = 0x0380,
+ [GR3_FLM_RD] = 0x0384,
+ [GR3_FLM1] = 0x0388,
+ [GR3_FLM2] = 0x038C,
+ [GR3_FLM3] = 0x0390,
+ [GR3_FLM4] = 0x0394,
+ [GR3_FLM5] = 0x0398,
+ [GR3_FLM6] = 0x039C,
+ [GR3_AB1] = 0x03A0,
+ [GR3_AB2] = 0x03A4,
+ [GR3_AB3] = 0x03A8,
+ [GR3_AB4] = 0x03AC,
+ [GR3_AB5] = 0x03B0,
+ [GR3_AB6] = 0x03B4,
+ [GR3_AB7] = 0x03B8,
+ [GR3_AB8] = 0x03BC,
+ [GR3_AB9] = 0x03C0,
+ [GR3_AB10] = 0x03C4,
+ [GR3_AB11] = 0x03C8,
+ [GR3_BASE] = 0x03CC,
+ [GR3_CLUT_INT] = 0x03D0,
+ [GR3_MON] = 0x03D4,
+ [TCON_UPDATE] = 0x0580,
+ [TCON_TIM] = 0x0584,
+ [TCON_TIM_STVA1] = 0x0588,
+ [TCON_TIM_STVA2] = 0x058C,
+ [TCON_TIM_STVB1] = 0x0590,
+ [TCON_TIM_STVB2] = 0x0594,
+ [TCON_TIM_STH1] = 0x0598,
+ [TCON_TIM_STH2] = 0x059C,
+ [TCON_TIM_STB1] = 0x05A0,
+ [TCON_TIM_STB2] = 0x05A4,
+ [TCON_TIM_CPV1] = 0x05A8,
+ [TCON_TIM_CPV2] = 0x05AC,
+ [TCON_TIM_POLA1] = 0x05B0,
+ [TCON_TIM_POLA2] = 0x05B4,
+ [TCON_TIM_POLB1] = 0x05B8,
+ [TCON_TIM_POLB2] = 0x05BC,
+ [TCON_TIM_DE] = 0x05C0,
+ [OUT_UPDATE] = 0x0600,
+ [OUT_SET] = 0x0604,
+ [OUT_BRIGHT1] = 0x0608,
+ [OUT_BRIGHT2] = 0x060C,
+ [OUT_CONTRAST] = 0x0610,
+ [OUT_PDTHA] = 0x0614,
+ [OUT_CLK_PHASE] = 0x0624,
+ [SYSCNT_INT1] = 0x0680,
+ [SYSCNT_INT2] = 0x0684,
+ [SYSCNT_INT3] = 0x0688,
+ [SYSCNT_INT4] = 0x068C,
+ [SYSCNT_PANEL_CLK] = 0x0690, /* 16-bit */
+ [SYSCNT_CLUT] = 0x0692, /* 16-bit */
+};
+
+/* SYSCNT */
+#define ICKEN (1 << 8)
+
+/* SCL Syncs */
+#define FREE_RUN_VSYNC 0x0001
+
+/* OUTPUT */
+#define OUT_FMT_RGB666 (1 << 12)
+
+/* TCON Timings */
+#define STVB_SEL_BITS 0x0007
+#define STVB_HS_SEL 2
+
+#define STH2_SEL_BITS 0x0007
+#define STH2_DE_SEL 7
+
+/* OUTCLK */
+#define LCD_DATA_EDGE 0x0100
+#define STVB_EDGE 0x0020
+#define STH_EDGE 0x0010
+
+/* SCL_UPDATE */
+#define SCL0_UPDATE_BIT 0x0100
+#define SCL0_VEN_BIT 0x0010
+
+/* TCON_UPDATE */
+#define TCON_VEN_BIT 0x0001
+
+/* OUT_UPDATE */
+#define OUTCNT_VEN_BIT 0x0001
+
+/* GR_UPDATE */
+#define P_VEN_UPDATE 0x0010
+#define IBUS_VEN_UPDATE 0x0001
+
+/* GR_AB1 */
+#define DISPSEL_BCKGND 0x0000
+#define DISPSEL_LOWER 0x0001
+#define DISPSEL_CUR 0x0002
+
+/* GR_FLM_RD */
+#define FB_R_ENB 0x01
+
+
+static void vdc4_write(struct ren_vdc4_priv *priv,
+ unsigned long reg_offs, unsigned long data)
+{
+ if ((SYSCNT_PANEL_CLK = reg_offs) || (SYSCNT_CLUT = reg_offs))
+ iowrite16(data, priv->base + vdc4_offsets[reg_offs]);
+ else
+ iowrite32(data, priv->base + vdc4_offsets[reg_offs]);
+}
+
+static unsigned long vdc4_read(struct ren_vdc4_priv *priv,
+ unsigned long reg_offs)
+{
+ if ((SYSCNT_PANEL_CLK = reg_offs) || (SYSCNT_CLUT = reg_offs))
+ return ioread16(priv->base + vdc4_offsets[reg_offs]);
+ else
+ return ioread32(priv->base + vdc4_offsets[reg_offs]);
+}
+
+static irqreturn_t ren_vdc4_irq(int irq, void *data)
+{
+ /* Not currently implemented/used */
+ return IRQ_HANDLED;
+}
+
+static void lcd_clear_display(struct ren_vdc4_priv *priv)
+{
+ unsigned char *pdest;
+ unsigned long size;
+
+ pdest = (unsigned char *)priv->dma_handle;
+ size = priv->cfg->lcd_cfg.xres * priv->cfg->lcd_cfg.yres * 2;
+
+ memset(pdest, 0, size);
+}
+
+static void restart_tft_display(struct ren_vdc4_priv *priv,
+ int clock_source)
+{
+ struct fb_videomode *lcd;
+ unsigned long h;
+ unsigned long v;
+ unsigned long tmp;
+
+ /* FB setup */
+ lcd = &priv->cfg->lcd_cfg;
+ lcd_clear_display(priv);
+
+ /* VDC clock Setup */
+ tmp = priv->cfg->clock_divider;
+ tmp |= clock_source << 12;
+ tmp |= ICKEN;
+ vdc4_write(priv, SYSCNT_PANEL_CLK, tmp);
+
+ /* Clear and Disable all interrupts */
+ vdc4_write(priv, SYSCNT_INT1, 0);
+ vdc4_write(priv, SYSCNT_INT2, 0);
+ vdc4_write(priv, SYSCNT_INT3, 0);
+ vdc4_write(priv, SYSCNT_INT4, 0);
+
+ /* Setup free-running syncs */
+ vdc4_write(priv, SCL0_FRC3, FREE_RUN_VSYNC);
+
+ /* Disable scale up/down */
+ vdc4_write(priv, SCL0_DS1, 0);
+ vdc4_write(priv, SCL0_US1, 0);
+
+ /* Timing registers */
+ h = lcd->hsync_len + lcd->left_margin + lcd->xres + lcd->right_margin;
+ v = lcd->vsync_len + lcd->upper_margin + lcd->yres + lcd->lower_margin;
+ tmp = (v - 1) << 16;
+ tmp |= h - 1;
+ vdc4_write(priv, SCL0_FRC4, tmp);
+
+ vdc4_write(priv, TCON_TIM, (((h - 1) / 2) << 16));
+
+ tmp = (lcd->vsync_len + lcd->upper_margin) << 16;
+ tmp |= lcd->yres;
+ vdc4_write(priv, SCL0_FRC6, tmp);
+ vdc4_write(priv, TCON_TIM_STVB1, tmp);
+ vdc4_write(priv, GR3_AB2, tmp);
+
+ tmp = lcd->left_margin << 16;
+ tmp |= lcd->xres;
+ vdc4_write(priv, SCL0_FRC7, tmp);
+ vdc4_write(priv, TCON_TIM_STB1, tmp);
+ vdc4_write(priv, GR3_AB3, tmp);
+
+ vdc4_write(priv, SCL0_FRC1, 0);
+ vdc4_write(priv, SCL0_FRC2, 0);
+ vdc4_write(priv, SCL0_FRC5, 0);
+
+ /* Set output format */
+ vdc4_write(priv, OUT_SET, OUT_FMT_RGB666);
+
+ /* STH TCON Timing */
+ tmp = priv->cfg->hs_pulse_width;
+ tmp |= priv->cfg->hs_start_pos << 16;
+ vdc4_write(priv, TCON_TIM_STH1, tmp);
+
+ /* Setup STVB as HSYNC */
+ tmp = vdc4_read(priv, TCON_TIM_STVB2);
+ tmp &= ~STVB_SEL_BITS;
+ tmp |= STVB_HS_SEL;
+ vdc4_write(priv, TCON_TIM_STVB2, tmp);
+
+ tmp = vdc4_read(priv, OUT_CLK_PHASE);
+ tmp &= ~STVB_EDGE;
+ vdc4_write(priv, OUT_CLK_PHASE, tmp);
+
+ /* Setup STH as DE */
+ tmp = vdc4_read(priv, TCON_TIM_STH2);
+ tmp &= ~STH2_SEL_BITS;
+ tmp |= STH2_DE_SEL;
+ vdc4_write(priv, TCON_TIM_STH2, tmp);
+
+ tmp = vdc4_read(priv, OUT_CLK_PHASE);
+ tmp &= ~STH_EDGE;
+ vdc4_write(priv, OUT_CLK_PHASE, tmp);
+
+ /* Output clock rising edge */
+ tmp = vdc4_read(priv, OUT_CLK_PHASE);
+ tmp &= ~LCD_DATA_EDGE;
+ vdc4_write(priv, OUT_CLK_PHASE, tmp);
+
+ /* Setup graphics buffers and update all registers */
+ vdc4_write(priv, GR1_AB1, DISPSEL_BCKGND);
+ vdc4_write(priv, GR2_AB1, DISPSEL_LOWER);
+ vdc4_write(priv, GR3_AB1, DISPSEL_CUR);
+
+ /* Setup framebuffer base/output */
+ vdc4_write(priv, GR3_FLM_RD, FB_R_ENB);
+
+ vdc4_write(priv, GR3_FLM2, (unsigned long)priv->info->screen_base);
+
+ vdc4_write(priv, GR3_FLM3, (lcd->xres * 2) << 16);
+
+ tmp = vdc4_read(priv, GR3_FLM5);
+ tmp |= lcd->yres << 16;
+ vdc4_write(priv, GR3_FLM5, tmp);
+
+ tmp = lcd->xres << 16;
+ vdc4_write(priv, GR3_FLM6, tmp);
+
+ /* Apply all register settings */
+ vdc4_write(priv, SCL0_UPDATE, SCL0_VEN_BIT | SCL0_UPDATE_BIT);
+ vdc4_write(priv, GR1_UPDATE, P_VEN_UPDATE);
+ vdc4_write(priv, GR2_UPDATE, P_VEN_UPDATE);
+ vdc4_write(priv, GR3_UPDATE, P_VEN_UPDATE | IBUS_VEN_UPDATE);
+ vdc4_write(priv, OUT_UPDATE, OUTCNT_VEN_BIT);
+ vdc4_write(priv, TCON_UPDATE, TCON_VEN_BIT);
+}
+
+static int ren_vdc4_setup_clocks(struct platform_device *pdev,
+ int clock_source,
+ struct ren_vdc4_priv *priv)
+{
+ priv->clk = clk_get(&pdev->dev, "vdc4");
+ if (IS_ERR(priv->clk)) {
+ dev_err(&pdev->dev, "cannot get clock \"vdc4\"\n");
+ return PTR_ERR(priv->clk);
+ }
+
+ if (clock_source = VDC4_PERI_CLK) {
+ priv->dot_clk = clk_get(&pdev->dev, "peripheral_clk");
+ if (IS_ERR(priv->dot_clk)) {
+ dev_err(&pdev->dev, "cannot get peripheral clock\n");
+ clk_put(priv->clk);
+ return PTR_ERR(priv->dot_clk);
+ }
+ }
+
+ return 0;
+}
+
+static int ren_vdc4_setcolreg(u_int regno,
+ u_int red, u_int green, u_int blue,
+ u_int transp, struct fb_info *info)
+{
+ u32 *palette = info->pseudo_palette;
+
+ if (regno >= PALETTE_NR)
+ return -EINVAL;
+
+ /* only FB_VISUAL_TRUECOLOR supported */
+
+ red >>= 16 - info->var.red.length;
+ green >>= 16 - info->var.green.length;
+ blue >>= 16 - info->var.blue.length;
+ transp >>= 16 - info->var.transp.length;
+
+ palette[regno] = (red << info->var.red.offset) |
+ (green << info->var.green.offset) |
+ (blue << info->var.blue.offset) |
+ (transp << info->var.transp.offset);
+
+ return 0;
+}
+
+static struct fb_fix_screeninfo ren_vdc4_fix = {
+ .id = "Renesas VDC4FB",
+ .type = FB_TYPE_PACKED_PIXELS,
+ .visual = FB_VISUAL_TRUECOLOR,
+ .accel = FB_ACCEL_NONE,
+};
+
+static struct fb_ops ren_vdc4_ops = {
+ .owner = THIS_MODULE,
+ .fb_setcolreg = ren_vdc4_setcolreg,
+ .fb_read = fb_sys_read,
+ .fb_write = fb_sys_write,
+ .fb_fillrect = sys_fillrect,
+ .fb_copyarea = sys_copyarea,
+ .fb_imageblit = sys_imageblit,
+};
+
+static int ren_vdc4_set_bpp(struct fb_var_screeninfo *var, int bpp)
+{
+ switch (bpp) {
+ case 16: /* RGB 565 */
+ var->red.offset = 11;
+ var->red.length = 5;
+ var->green.offset = 5;
+ var->green.length = 6;
+ var->blue.offset = 0;
+ var->blue.length = 5;
+ var->transp.offset = 0;
+ var->transp.length = 0;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ var->bits_per_pixel = bpp;
+ var->red.msb_right = 0;
+ var->green.msb_right = 0;
+ var->blue.msb_right = 0;
+ var->transp.msb_right = 0;
+ return 0;
+}
+
+/* PM Functions */
+static int ren_vdc4_start(struct ren_vdc4_priv *priv,
+ int clock_source)
+{
+ int ret;
+
+ ret = clk_enable(priv->clk);
+ if (ret < 0)
+ return ret;
+
+ if (priv->dot_clk) {
+ ret = clk_enable(priv->dot_clk);
+ if (ret < 0)
+ return ret;
+ }
+
+ restart_tft_display(priv, clock_source);
+
+ return ret;
+}
+
+static void ren_vdc4_stop(struct ren_vdc4_priv *priv)
+{
+ if (priv->dot_clk)
+ clk_disable(priv->dot_clk);
+ clk_disable(priv->clk);
+}
+
+static int ren_vdc4_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+
+ ren_vdc4_stop(platform_get_drvdata(pdev));
+ return 0;
+}
+
+static int ren_vdc4_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct ren_vdc4_info *pdata = pdev->dev.platform_data;
+
+ return ren_vdc4_start(platform_get_drvdata(pdev), pdata->clock_source);
+}
+
+static const struct dev_pm_ops ren_vdc4_dev_pm_ops = {
+ .suspend = ren_vdc4_suspend,
+ .resume = ren_vdc4_resume,
+};
+
+static int ren_vdc4_remove(struct platform_device *pdev);
+
+static int __devinit ren_vdc4_probe(struct platform_device *pdev)
+{
+ struct fb_info *info;
+ struct ren_vdc4_priv *priv;
+ struct ren_vdc4_info *pdata = pdev->dev.platform_data;
+ struct resource *res;
+ void *buf;
+ int irq, error;
+
+ if (!pdata) {
+ dev_err(&pdev->dev, "no platform data defined\n");
+ return -EINVAL;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ irq = platform_get_irq(pdev, 0);
+ if (!res || irq < 0) {
+ dev_err(&pdev->dev, "cannot get platform resources\n");
+ return -ENOENT;
+ }
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ dev_err(&pdev->dev, "cannot allocate device data\n");
+ return -ENOMEM;
+ }
+
+ platform_set_drvdata(pdev, priv);
+
+ error = request_irq(irq, ren_vdc4_irq, 0, dev_name(&pdev->dev), priv);
+ if (error) {
+ dev_err(&pdev->dev, "unable to request irq\n");
+ goto err1;
+ }
+
+ priv->irq = irq;
+ pdata = pdev->dev.platform_data;
+
+ priv->cfg = pdata;
+
+ error = ren_vdc4_setup_clocks(pdev, pdata->clock_source, priv);
+ if (error) {
+ dev_err(&pdev->dev, "unable to setup clocks\n");
+ goto err1;
+ }
+
+ priv->base = ioremap_nocache(res->start, resource_size(res));
+ if (!priv->base) {
+ dev_err(&pdev->dev, "unable to ioremap\n");
+ goto err1;
+ }
+
+ priv->info = framebuffer_alloc(0, &pdev->dev);
+ if (!priv->info) {
+ dev_err(&pdev->dev, "unable to allocate fb_info\n");
+ goto err1;
+ }
+
+ info = priv->info;
+ info->fbops = &ren_vdc4_ops;
+ info->var.xres = info->var.xres_virtual = pdata->lcd_cfg.xres;
+ info->var.yres = info->var.yres_virtual = pdata->lcd_cfg.yres;
+ info->var.width = pdata->panel_width;
+ info->var.height = pdata->panel_height;
+ info->var.activate = FB_ACTIVATE_NOW;
+ info->pseudo_palette = priv->pseudo_palette;
+ error = ren_vdc4_set_bpp(&info->var, pdata->bpp);
+ if (error)
+ goto err1;
+
+ info->fix = ren_vdc4_fix;
+ info->fix.line_length = pdata->lcd_cfg.xres * (pdata->bpp / 8);
+ info->fix.smem_len = info->fix.line_length * pdata->lcd_cfg.yres;
+
+ buf = dma_alloc_coherent(&pdev->dev, info->fix.smem_len,
+ &priv->dma_handle, GFP_KERNEL);
+ if (!buf) {
+ dev_err(&pdev->dev, "unable to allocate buffer\n");
+ goto err1;
+ }
+
+ info->flags = FBINFO_FLAG_DEFAULT;
+
+ error = fb_alloc_cmap(&info->cmap, PALETTE_NR, 0);
+ if (error < 0) {
+ dev_err(&pdev->dev, "unable to allocate cmap\n");
+ goto err1;
+ }
+
+ memset(buf, 0, info->fix.smem_len);
+ info->fix.smem_start = priv->dma_handle;
+ info->screen_base = buf;
+ info->device = &pdev->dev;
+ info->par = priv;
+
+ if (error)
+ goto err1;
+
+ ren_vdc4_start(priv, pdata->clock_source);
+ if (error) {
+ dev_err(&pdev->dev, "unable to start hardware\n");
+ goto err1;
+ }
+
+ info = priv->info;
+
+ error = register_framebuffer(info);
+ if (error < 0)
+ goto err1;
+
+ dev_info(info->dev,
+ "registered %s as %udx%ud %dbpp.\n",
+ pdev->name,
+ (int) pdata->lcd_cfg.xres,
+ (int) pdata->lcd_cfg.yres,
+ pdata->bpp);
+
+ return 0;
+
+err1:
+ ren_vdc4_remove(pdev);
+ return error;
+}
+
+static int ren_vdc4_remove(struct platform_device *pdev)
+{
+ struct ren_vdc4_priv *priv = platform_get_drvdata(pdev);
+ struct fb_info *info;
+
+ if (priv->info->dev)
+ unregister_framebuffer(priv->info);
+
+ ren_vdc4_stop(priv);
+
+ info = priv->info;
+
+ if (!info || !info->device) {
+ dev_err(&pdev->dev, "Failed to dealloc/release fb_info\n");
+ } else {
+ fb_dealloc_cmap(&info->cmap);
+ framebuffer_release(info);
+ }
+
+ if (priv->dot_clk)
+ clk_put(priv->dot_clk);
+ clk_put(priv->clk);
+
+ if (priv->base)
+ iounmap(priv->base);
+
+ if (priv->irq)
+ free_irq(priv->irq, priv);
+
+ kfree(priv);
+ return 0;
+}
+
+static struct platform_driver ren_vdc4_driver = {
+ .driver = {
+ .name = "ren_vdc4fb",
+ .owner = THIS_MODULE,
+ .pm = &ren_vdc4_dev_pm_ops,
+ },
+ .probe = ren_vdc4_probe,
+ .remove = ren_vdc4_remove,
+};
+
+static int __init ren_vdc4_init(void)
+{
+ return platform_driver_register(&ren_vdc4_driver);
+}
+
+static void __exit ren_vdc4_exit(void)
+{
+ platform_driver_unregister(&ren_vdc4_driver);
+}
+
+module_init(ren_vdc4_init);
+module_exit(ren_vdc4_exit);
+
+MODULE_DESCRIPTION("Renesas VDC4 Framebuffer driver");
+MODULE_AUTHOR("Phil Edworthy <phil.edworthy@renesas.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/video/ren_vdc4fb.h b/include/video/ren_vdc4fb.h
new file mode 100644
index 0000000..e91a515
--- /dev/null
+++ b/include/video/ren_vdc4fb.h
@@ -0,0 +1,19 @@
+#ifndef __REN_VDC4_H__
+#define __REN_VDC4_H__
+
+#include <linux/fb.h>
+
+enum { VDC4_EXTCLK = 1, VDC4_PERI_CLK };
+
+struct ren_vdc4_info {
+ int bpp;
+ int clock_source;
+ int clock_divider;
+ int hs_pulse_width;
+ int hs_start_pos;
+ struct fb_videomode lcd_cfg;
+ unsigned long panel_width;
+ unsigned long panel_height;
+};
+
+#endif
--
1.7.5.4
^ permalink raw reply related
* [PATCH] video: move CNVT_TOHW to a common place
From: Manjunathappa, Prakash @ 2012-08-08 12:06 UTC (permalink / raw)
To: linux-fbdev
Replace macro "CNVT_TOHW" by inline function. Also moving it to a
common place linux/fb.h so that it can be accessed from all framebuffer
drivers.
Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
---
Applies on top of fbdev tree[1].
[1] git://github.com/schandinat/linux-2.6.git fbdev-next
drivers/video/68328fb.c | 2 --
drivers/video/fb-puv3.c | 2 --
drivers/video/fsl-diu-fb.c | 5 -----
drivers/video/grvga.c | 6 ------
drivers/video/imxfb.c | 1 -
drivers/video/matrox/matroxfb_base.h | 2 --
drivers/video/pm2fb.c | 2 --
drivers/video/pm3fb.c | 2 --
drivers/video/skeletonfb.c | 2 --
drivers/video/tdfxfb.c | 3 ---
drivers/video/vfb.c | 2 --
drivers/video/xen-fbfront.c | 2 --
include/linux/fb.h | 5 +++++
13 files changed, 5 insertions(+), 31 deletions(-)
diff --git a/drivers/video/68328fb.c b/drivers/video/68328fb.c
index a425d65..5a1f360 100644
--- a/drivers/video/68328fb.c
+++ b/drivers/video/68328fb.c
@@ -318,7 +318,6 @@ static int mc68x328fb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
* (blue << blue.offset) | (transp << transp.offset)
* RAMDAC does not exist
*/
-#define CNVT_TOHW(val,width) ((((val)<<(width))+0x7FFF-(val))>>16)
switch (info->fix.visual) {
case FB_VISUAL_TRUECOLOR:
case FB_VISUAL_PSEUDOCOLOR:
@@ -335,7 +334,6 @@ static int mc68x328fb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
transp = CNVT_TOHW(transp, 8);
break;
}
-#undef CNVT_TOHW
/* Truecolor has hardware independent palette */
if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
u32 v;
diff --git a/drivers/video/fb-puv3.c b/drivers/video/fb-puv3.c
index 60a787f..c84a907 100644
--- a/drivers/video/fb-puv3.c
+++ b/drivers/video/fb-puv3.c
@@ -565,7 +565,6 @@ static int unifb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
(red * 77 + green * 151 + blue * 28) >> 8;
}
-#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
switch (info->fix.visual) {
case FB_VISUAL_TRUECOLOR:
case FB_VISUAL_PSEUDOCOLOR:
@@ -582,7 +581,6 @@ static int unifb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
transp = CNVT_TOHW(transp, 8);
break;
}
-#undef CNVT_TOHW
/* Truecolor has hardware independent palette */
if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
u32 v;
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 458c006..878c588 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1008,11 +1008,6 @@ static int fsl_diu_set_par(struct fb_info *info)
return 0;
}
-static inline __u32 CNVT_TOHW(__u32 val, __u32 width)
-{
- return ((val << width) + 0x7FFF - val) >> 16;
-}
-
/*
* Set a single color register. The values supplied have a 16 bit magnitude
* which needs to be scaled in this function for the hardware. Things to take
diff --git a/drivers/video/grvga.c b/drivers/video/grvga.c
index 5245f9a..452038f 100644
--- a/drivers/video/grvga.c
+++ b/drivers/video/grvga.c
@@ -203,17 +203,11 @@ static int grvga_setcolreg(unsigned regno, unsigned red, unsigned green, unsigne
red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
}
-
-
-#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
-
red = CNVT_TOHW(red, info->var.red.length);
green = CNVT_TOHW(green, info->var.green.length);
blue = CNVT_TOHW(blue, info->var.blue.length);
transp = CNVT_TOHW(transp, info->var.transp.length);
-#undef CNVT_TOHW
-
/* In PSEUDOCOLOR we use the hardware CLUT */
if (info->fix.visual = FB_VISUAL_PSEUDOCOLOR)
__raw_writel((regno << 24) | (red << 16) | (green << 8) | blue,
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index caad368..b008fde 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -224,7 +224,6 @@ static int imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
struct imxfb_info *fbi = info->par;
u_int val, ret = 1;
-#define CNVT_TOHW(val,width) ((((val)<<(width))+0x7FFF-(val))>>16)
if (regno < fbi->palette_size) {
val = (CNVT_TOHW(red, 4) << 8) |
(CNVT_TOHW(green,4) << 4) |
diff --git a/drivers/video/matrox/matroxfb_base.h b/drivers/video/matrox/matroxfb_base.h
index 11ed57b..ace1e12 100644
--- a/drivers/video/matrox/matroxfb_base.h
+++ b/drivers/video/matrox/matroxfb_base.h
@@ -116,8 +116,6 @@
#define MX_VISUAL_DIRECTCOLOR FB_VISUAL_TRUECOLOR
#define MX_VISUAL_PSEUDOCOLOR FB_VISUAL_PSEUDOCOLOR
-#define CNVT_TOHW(val,width) ((((val)<<(width))+0x7FFF-(val))>>16)
-
/* G-series and Mystique have (almost) same DAC */
#undef NEED_DAC1064
#if defined(CONFIG_FB_MATROX_MYSTIQUE) || defined(CONFIG_FB_MATROX_G)
diff --git a/drivers/video/pm2fb.c b/drivers/video/pm2fb.c
index df31a24..85e810c 100644
--- a/drivers/video/pm2fb.c
+++ b/drivers/video/pm2fb.c
@@ -908,7 +908,6 @@ static int pm2fb_setcolreg(unsigned regno, unsigned red, unsigned green,
* (blue << blue.offset) | (transp << transp.offset)
* RAMDAC does not exist
*/
-#define CNVT_TOHW(val, width) ((((val) << (width)) + 0x7FFF -(val)) >> 16)
switch (info->fix.visual) {
case FB_VISUAL_TRUECOLOR:
case FB_VISUAL_PSEUDOCOLOR:
@@ -927,7 +926,6 @@ static int pm2fb_setcolreg(unsigned regno, unsigned red, unsigned green,
transp = CNVT_TOHW(transp, 8);
break;
}
-#undef CNVT_TOHW
/* Truecolor has hardware independent palette */
if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
u32 v;
diff --git a/drivers/video/pm3fb.c b/drivers/video/pm3fb.c
index 055e527..8cb30f8 100644
--- a/drivers/video/pm3fb.c
+++ b/drivers/video/pm3fb.c
@@ -1109,12 +1109,10 @@ static int pm3fb_setcolreg(unsigned regno, unsigned red, unsigned green,
* This is the point where the color is converted to something that
* is acceptable by the hardware.
*/
-#define CNVT_TOHW(val, width) ((((val) << (width)) + 0x7FFF - (val)) >> 16)
red = CNVT_TOHW(red, info->var.red.length);
green = CNVT_TOHW(green, info->var.green.length);
blue = CNVT_TOHW(blue, info->var.blue.length);
transp = CNVT_TOHW(transp, info->var.transp.length);
-#undef CNVT_TOHW
if (info->fix.visual = FB_VISUAL_TRUECOLOR ||
info->fix.visual = FB_VISUAL_DIRECTCOLOR) {
diff --git a/drivers/video/skeletonfb.c b/drivers/video/skeletonfb.c
index 5b6abc6..fbf8b75 100644
--- a/drivers/video/skeletonfb.c
+++ b/drivers/video/skeletonfb.c
@@ -359,12 +359,10 @@ static int xxxfb_setcolreg(unsigned regno, unsigned red, unsigned green,
* This is the point where the color is converted to something that
* is acceptable by the hardware.
*/
-#define CNVT_TOHW(val,width) ((((val)<<(width))+0x7FFF-(val))>>16)
red = CNVT_TOHW(red, info->var.red.length);
green = CNVT_TOHW(green, info->var.green.length);
blue = CNVT_TOHW(blue, info->var.blue.length);
transp = CNVT_TOHW(transp, info->var.transp.length);
-#undef CNVT_TOHW
/*
* This is the point where the function feeds the color to the hardware
* palette after converting the colors to something acceptable by
diff --git a/drivers/video/tdfxfb.c b/drivers/video/tdfxfb.c
index e026724..2eb6c63 100644
--- a/drivers/video/tdfxfb.c
+++ b/drivers/video/tdfxfb.c
@@ -752,9 +752,6 @@ static int tdfxfb_set_par(struct fb_info *info)
return 0;
}
-/* A handy macro shamelessly pinched from matroxfb */
-#define CNVT_TOHW(val, width) ((((val) << (width)) + 0x7FFF - (val)) >> 16)
-
static int tdfxfb_setcolreg(unsigned regno, unsigned red, unsigned green,
unsigned blue, unsigned transp,
struct fb_info *info)
diff --git a/drivers/video/vfb.c b/drivers/video/vfb.c
index 501a922..bec437c 100644
--- a/drivers/video/vfb.c
+++ b/drivers/video/vfb.c
@@ -335,7 +335,6 @@ static int vfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
* (blue << blue.offset) | (transp << transp.offset)
* RAMDAC does not exist
*/
-#define CNVT_TOHW(val,width) ((((val)<<(width))+0x7FFF-(val))>>16)
switch (info->fix.visual) {
case FB_VISUAL_TRUECOLOR:
case FB_VISUAL_PSEUDOCOLOR:
@@ -352,7 +351,6 @@ static int vfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
transp = CNVT_TOHW(transp, 8);
break;
}
-#undef CNVT_TOHW
/* Truecolor has hardware independent palette */
if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
u32 v;
diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
index b7f5173..6896744 100644
--- a/drivers/video/xen-fbfront.c
+++ b/drivers/video/xen-fbfront.c
@@ -217,12 +217,10 @@ static int xenfb_setcolreg(unsigned regno, unsigned red, unsigned green,
if (regno > info->cmap.len)
return 1;
-#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
red = CNVT_TOHW(red, info->var.red.length);
green = CNVT_TOHW(green, info->var.green.length);
blue = CNVT_TOHW(blue, info->var.blue.length);
transp = CNVT_TOHW(transp, info->var.transp.length);
-#undef CNVT_TOHW
v = (red << info->var.red.offset) |
(green << info->var.green.offset) |
diff --git a/include/linux/fb.h b/include/linux/fb.h
index ac3f1c6..8a2b3ca 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -397,6 +397,11 @@ struct fb_cursor {
struct fb_image image; /* Cursor image */
};
+static inline __u32 CNVT_TOHW(__u32 val, __u32 width)
+{
+ return ((val << width) + 0x7FFF - val) >> 16;
+}
+
#ifdef CONFIG_FB_BACKLIGHT
/* Settings for the generic backlight code */
#define FB_BACKLIGHT_LEVELS 128
--
1.7.1
^ permalink raw reply related
* [PATCH 3/6] OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
From: Chandrabhanu Mahapatra @ 2012-08-08 11:50 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <19a46cc15dc7add0671dd8c32f5398249c0420ab.1343912533.git.cmahapatra@ti.com>
All the cpu_is checks have been moved to dss_init_features function providing a
much more generic and cleaner interface. The OMAP version and revision specific
functions are initialized by dss_features structure local to dss.c.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/dss.c | 154 ++++++++++++++++++++++++++++++-----------
1 file changed, 114 insertions(+), 40 deletions(-)
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 7b1c6ac..f5971ac 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -65,6 +65,20 @@ struct dss_reg {
static int dss_runtime_get(void);
static void dss_runtime_put(void);
+static bool check_dss_cinfo_fck(void);
+static bool check_dss_cinfo_fck_34xx(void);
+
+static int dss_get_clk_24xx(struct clk *clk);
+static int dss_get_clk_3xxx(struct clk *clk);
+static int dss_get_clk_44xx(struct clk *clk);
+
+struct dss_features {
+ u16 fck_div_max;
+ int factor;
+ bool (*check_cinfo_fck) (void);
+ int (*get_clk) (struct clk *clk);
+};
+
static struct {
struct platform_device *pdev;
void __iomem *base;
@@ -83,6 +97,8 @@ static struct {
bool ctx_valid;
u32 ctx[DSS_SZ_REGS / sizeof(u32)];
+
+ const struct dss_features *feat;
} dss;
static const char * const dss_generic_clk_source_names[] = {
@@ -91,6 +107,34 @@ static const char * const dss_generic_clk_source_names[] = {
[OMAP_DSS_CLK_SRC_FCK] = "DSS_FCK",
};
+static const struct dss_features omap2_dss_features = {
+ .fck_div_max = 16,
+ .factor = 2,
+ .check_cinfo_fck = check_dss_cinfo_fck,
+ .get_clk = dss_get_clk_24xx,
+};
+
+static const struct dss_features omap34_dss_features = {
+ .fck_div_max = 16,
+ .factor = 2,
+ .check_cinfo_fck = check_dss_cinfo_fck_34xx,
+ .get_clk = dss_get_clk_3xxx,
+};
+
+static const struct dss_features omap36_dss_features = {
+ .fck_div_max = 32,
+ .factor = 1,
+ .check_cinfo_fck = check_dss_cinfo_fck,
+ .get_clk = dss_get_clk_3xxx,
+};
+
+static const struct dss_features omap4_dss_features = {
+ .fck_div_max = 32,
+ .factor = 1,
+ .check_cinfo_fck = check_dss_cinfo_fck,
+ .get_clk = dss_get_clk_44xx,
+};
+
static inline void dss_write_reg(const struct dss_reg idx, u32 val)
{
__raw_writel(val, dss.base + idx.idx);
@@ -236,7 +280,6 @@ const char *dss_get_generic_clk_source_name(enum omap_dss_clk_source clk_src)
return dss_generic_clk_source_names[clk_src];
}
-
void dss_dump_clocks(struct seq_file *s)
{
unsigned long dpll4_ck_rate;
@@ -259,18 +302,10 @@ void dss_dump_clocks(struct seq_file *s)
seq_printf(s, "dpll4_ck %lu\n", dpll4_ck_rate);
- if (cpu_is_omap3630() || cpu_is_omap44xx())
- seq_printf(s, "%s (%s) = %lu / %lu = %lu\n",
- fclk_name, fclk_real_name,
- dpll4_ck_rate,
- dpll4_ck_rate / dpll4_m4_ck_rate,
- fclk_rate);
- else
- seq_printf(s, "%s (%s) = %lu / %lu * 2 = %lu\n",
- fclk_name, fclk_real_name,
- dpll4_ck_rate,
- dpll4_ck_rate / dpll4_m4_ck_rate,
- fclk_rate);
+ seq_printf(s, "%s (%s) = %lu / %lu * %d = %lu\n",
+ fclk_name, fclk_real_name, dpll4_ck_rate,
+ dpll4_ck_rate / dpll4_m4_ck_rate,
+ dss.feat->factor, fclk_rate);
} else {
seq_printf(s, "%s (%s) = %lu\n",
fclk_name, fclk_real_name,
@@ -461,6 +496,25 @@ unsigned long dss_get_dpll4_rate(void)
return 0;
}
+static bool check_dss_cinfo_fck_34xx(void)
+{
+ unsigned long prate = dss_get_dpll4_rate();
+ unsigned long fck = clk_get_rate(dss.dss_clk);
+
+ if (prate = dss.cache_prate || dss.cache_dss_cinfo.fck = fck)
+ return true;
+ return false;
+}
+
+static bool check_dss_cinfo_fck(void)
+{
+ unsigned long fck = clk_get_rate(dss.dss_clk);
+
+ if (dss.cache_dss_cinfo.fck = fck)
+ return true;
+ return false;
+}
+
int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
struct dispc_clock_info *dispc_cinfo)
{
@@ -470,7 +524,7 @@ int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
unsigned long fck, max_dss_fck;
- u16 fck_div, fck_div_max = 16;
+ u16 fck_div;
int match = 0;
int min_fck_per_pck;
@@ -479,10 +533,7 @@ int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
max_dss_fck = dss_feat_get_param_max(FEAT_PARAM_DSS_FCK);
- fck = clk_get_rate(dss.dss_clk);
- if (req_pck = dss.cache_req_pck &&
- ((cpu_is_omap34xx() && prate = dss.cache_prate) ||
- dss.cache_dss_cinfo.fck = fck)) {
+ if (req_pck = dss.cache_req_pck && dss.feat->check_cinfo_fck()) {
DSSDBG("dispc clock info found from cache.\n");
*dss_cinfo = dss.cache_dss_cinfo;
*dispc_cinfo = dss.cache_dispc_cinfo;
@@ -519,13 +570,10 @@ retry:
goto found;
} else {
- if (cpu_is_omap3630() || cpu_is_omap44xx())
- fck_div_max = 32;
-
- for (fck_div = fck_div_max; fck_div > 0; --fck_div) {
+ for (fck_div = dss.feat->fck_div_max; fck_div > 0; --fck_div) {
struct dispc_clock_info cur_dispc;
- if (fck_div_max = 32)
+ if (dss.feat->fck_div_max = 32)
fck = prate / fck_div;
else
fck = prate / fck_div * 2;
@@ -619,6 +667,32 @@ enum dss_hdmi_venc_clk_source_select dss_get_hdmi_venc_clk_source(void)
return REG_GET(DSS_CONTROL, 15, 15);
}
+static int dss_get_clk_24xx(struct clk *clk)
+{
+ clk = NULL;
+ return true;
+}
+
+static int dss_get_clk_3xxx(struct clk *clk)
+{
+ clk = clk_get(NULL, "dpll4_m4_ck");
+ if (IS_ERR(clk)) {
+ DSSERR("Failed to get dpll4_m4_ck\n");
+ return PTR_ERR(clk);
+ }
+ return true;
+}
+
+static int dss_get_clk_44xx(struct clk *clk)
+{
+ clk = clk_get(NULL, "dpll_per_m5x2_ck");
+ if (IS_ERR(clk)) {
+ DSSERR("Failed to get dpll_per_m5x2_ck\n");
+ return PTR_ERR(clk);
+ }
+ return true;
+}
+
static int dss_get_clocks(void)
{
struct clk *clk;
@@ -633,23 +707,9 @@ static int dss_get_clocks(void)
dss.dss_clk = clk;
- if (cpu_is_omap34xx()) {
- clk = clk_get(NULL, "dpll4_m4_ck");
- if (IS_ERR(clk)) {
- DSSERR("Failed to get dpll4_m4_ck\n");
- r = PTR_ERR(clk);
- goto err;
- }
- } else if (cpu_is_omap44xx()) {
- clk = clk_get(NULL, "dpll_per_m5x2_ck");
- if (IS_ERR(clk)) {
- DSSERR("Failed to get dpll_per_m5x2_ck\n");
- r = PTR_ERR(clk);
- goto err;
- }
- } else { /* omap24xx */
- clk = NULL;
- }
+ r = dss.feat->get_clk(clk);
+ if (r != true)
+ goto err;
dss.dpll4_m4_ck = clk;
@@ -704,6 +764,18 @@ void dss_debug_dump_clocks(struct seq_file *s)
}
#endif
+static void dss_init_features(void)
+{
+ if (cpu_is_omap24xx())
+ dss.feat = &omap2_dss_features;
+ else if (cpu_is_omap34xx())
+ dss.feat = &omap34_dss_features;
+ else if (cpu_is_omap3630())
+ dss.feat = &omap36_dss_features;
+ else
+ dss.feat = &omap4_dss_features;
+}
+
/* DSS HW IP initialisation */
static int __init omap_dsshw_probe(struct platform_device *pdev)
{
@@ -750,6 +822,8 @@ static int __init omap_dsshw_probe(struct platform_device *pdev)
dss.lcd_clk_source[0] = OMAP_DSS_CLK_SRC_FCK;
dss.lcd_clk_source[1] = OMAP_DSS_CLK_SRC_FCK;
+ dss_init_features();
+
rev = dss_read_reg(DSS_REVISION);
printk(KERN_INFO "OMAP DSS rev %d.%d\n",
FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0));
--
1.7.10
^ permalink raw reply related
* [PATCH 1/6] OMAPDSS: DISPC: cleanup cpu_is_xxxx checks
From: Chandrabhanu Mahapatra @ 2012-08-08 11:49 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <5344e530a125ef5c5dfeb00e54b7d32df6169aa9.1343912532.git.cmahapatra@ti.com>
All the cpu_is checks have been moved to dispc_init_features function providing
a much more generic and cleaner interface. The OMAP version and revision
specific functions are initialized by dispc_features structure local to dispc.c.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/dispc.c | 476 ++++++++++++++++++++++++++-------------
1 file changed, 315 insertions(+), 161 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 5b289c5..7e0b080 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -75,12 +75,60 @@ enum omap_burst_size {
#define REG_FLD_MOD(idx, val, start, end) \
dispc_write_reg(idx, FLD_MOD(dispc_read_reg(idx), val, start, end))
+static int dispc_ovl_calc_scaling_24xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings, u16 width, u16 height,
+ u16 out_width, u16 out_height, enum omap_color_mode color_mode,
+ bool *five_taps, int *x_predecim, int *y_predecim, int *decim_x,
+ int *decim_y, u16 pos_x, unsigned long *core_clk);
+static int dispc_ovl_calc_scaling_34xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings, u16 width, u16 height,
+ u16 out_width, u16 out_height, enum omap_color_mode color_mode,
+ bool *five_taps, int *x_predecim, int *y_predecim, int *decim_x,
+ int *decim_y, u16 pos_x, unsigned long *core_clk);
+static int dispc_ovl_calc_scaling_44xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings, u16 width, u16 height,
+ u16 out_width, u16 out_height, enum omap_color_mode color_mode,
+ bool *five_taps, int *x_predecim, int *y_predecim, int *decim_x,
+ int *decim_y, u16 pos_x, unsigned long *core_clk);
+
+static unsigned long calc_core_clk_24xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height);
+static unsigned long calc_core_clk_34xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height);
+static unsigned long calc_core_clk_44xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height);
+
+static bool _dispc_lcd_timings_ok_24xx(int hsw, int hfp, int hbp,
+ int vsw, int vfp, int vbp);
+static bool _dispc_lcd_timings_ok_44xx(int hsw, int hfp, int hbp,
+ int vsw, int vfp, int vbp);
+
+static void _dispc_mgr_set_lcd_timings_hv_24xx(enum omap_channel channel,
+ int hsw, int hfp, int hbp, int vsw, int vfp, int vbp);
+static void _dispc_mgr_set_lcd_timings_hv_44xx(enum omap_channel channel,
+ int hsw, int hfp, int hbp, int vsw, int vfp, int vbp);
+
struct dispc_irq_stats {
unsigned long last_reset;
unsigned irq_count;
unsigned irqs[32];
};
+struct dispc_features {
+ int (*calc_scaling) (enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk);
+ unsigned long (*calc_core_clk) (enum omap_channel channel,
+ u16 width, u16 height, u16 out_width, u16 out_height);
+ bool (*lcd_timings_ok) (int hsw, int hfp, int hbp,
+ int vsw, int vfp, int vbp);
+ void (*set_lcd_timings_hv) (enum omap_channel channel, int hsw, int hfp,
+ int hbp, int vsw, int vfp, int vbp);
+};
+
static struct {
struct platform_device *pdev;
void __iomem *base;
@@ -101,6 +149,8 @@ static struct {
bool ctx_valid;
u32 ctx[DISPC_SZ_REGS / sizeof(u32)];
+ const struct dispc_features *feat;
+
#ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS
spinlock_t irq_stats_lock;
struct dispc_irq_stats irq_stats;
@@ -210,6 +260,34 @@ static const struct {
},
};
+static const struct dispc_features omap2_dispc_features = {
+ .calc_scaling = dispc_ovl_calc_scaling_24xx,
+ .calc_core_clk = calc_core_clk_24xx,
+ .lcd_timings_ok = _dispc_lcd_timings_ok_24xx,
+ .set_lcd_timings_hv = _dispc_mgr_set_lcd_timings_hv_24xx,
+};
+
+static const struct dispc_features omap3_2_1_dispc_features = {
+ .calc_scaling = dispc_ovl_calc_scaling_34xx,
+ .calc_core_clk = calc_core_clk_34xx,
+ .lcd_timings_ok = _dispc_lcd_timings_ok_24xx,
+ .set_lcd_timings_hv = _dispc_mgr_set_lcd_timings_hv_24xx,
+};
+
+static const struct dispc_features omap3_3_0_dispc_features = {
+ .calc_scaling = dispc_ovl_calc_scaling_34xx,
+ .calc_core_clk = calc_core_clk_34xx,
+ .lcd_timings_ok = _dispc_lcd_timings_ok_44xx,
+ .set_lcd_timings_hv = _dispc_mgr_set_lcd_timings_hv_44xx,
+};
+
+static const struct dispc_features omap4_dispc_features = {
+ .calc_scaling = dispc_ovl_calc_scaling_44xx,
+ .calc_core_clk = calc_core_clk_44xx,
+ .lcd_timings_ok = _dispc_lcd_timings_ok_44xx,
+ .set_lcd_timings_hv = _dispc_mgr_set_lcd_timings_hv_44xx,
+};
+
static void _omap_dispc_set_irqs(void);
static inline void dispc_write_reg(const u16 idx, u32 val)
@@ -1939,7 +2017,18 @@ static unsigned long calc_core_clk_five_taps(enum omap_channel channel,
return core_clk;
}
-static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
+static unsigned long calc_core_clk_24xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height)
+{
+ unsigned long pclk = dispc_mgr_pclk_rate(channel);
+
+ if (height > out_height && width > out_width)
+ return pclk * 4;
+ else
+ return pclk * 2;
+}
+
+static unsigned long calc_core_clk_34xx(enum omap_channel channel, u16 width,
u16 height, u16 out_width, u16 out_height)
{
unsigned int hf, vf;
@@ -1958,25 +2047,163 @@ static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
hf = 2;
else
hf = 1;
-
if (height > out_height)
vf = 2;
else
vf = 1;
- if (cpu_is_omap24xx()) {
- if (vf > 1 && hf > 1)
- return pclk * 4;
- else
- return pclk * 2;
- } else if (cpu_is_omap34xx()) {
- return pclk * vf * hf;
- } else {
- if (hf > 1)
- return DIV_ROUND_UP(pclk, out_width) * width;
- else
- return pclk;
+ return pclk * vf * hf;
+}
+
+static unsigned long calc_core_clk_44xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height)
+{
+ unsigned long pclk = dispc_mgr_pclk_rate(channel);
+
+ if (width > out_width)
+ return DIV_ROUND_UP(pclk, out_width) * width;
+ else
+ return pclk;
+}
+
+static int dispc_ovl_calc_scaling_24xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk)
+{
+ int error;
+ u16 in_width, in_height;
+ int min_factor = min(*decim_x, *decim_y);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+ *five_taps = false;
+
+ do {
+ in_height = DIV_ROUND_UP(height, *decim_y);
+ in_width = DIV_ROUND_UP(width, *decim_x);
+ *core_clk = dispc.feat->calc_core_clk(channel, in_width,
+ in_height, out_width, out_height);
+ error = (in_width > maxsinglelinewidth || !*core_clk ||
+ *core_clk > dispc_core_clk_rate());
+ if (error) {
+ if (*decim_x = *decim_y) {
+ *decim_x = min_factor;
+ ++*decim_y;
+ } else {
+ swap(*decim_x, *decim_y);
+ if (*decim_x < *decim_y)
+ ++*decim_x;
+ }
+ }
+ } while (*decim_x <= *x_predecim && *decim_y <= *y_predecim && error);
+
+ if (in_width > maxsinglelinewidth) {
+ DSSERR("Cannot scale max input width exceeded");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int dispc_ovl_calc_scaling_34xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk)
+{
+ int error;
+ u16 in_width, in_height;
+ int min_factor = min(*decim_x, *decim_y);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+
+ do {
+ in_height = DIV_ROUND_UP(height, *decim_y);
+ in_width = DIV_ROUND_UP(width, *decim_x);
+ *core_clk = calc_core_clk_five_taps(channel, mgr_timings,
+ in_width, in_height, out_width, out_height, color_mode);
+
+ error = check_horiz_timing_omap3(channel, mgr_timings, pos_x,
+ in_width, in_height, out_width, out_height);
+
+ if (in_width > maxsinglelinewidth)
+ if (in_height > out_height &&
+ in_height < out_height * 2)
+ *five_taps = false;
+ if (!*five_taps)
+ *core_clk = dispc.feat->calc_core_clk(channel, in_width,
+ in_height, out_width, out_height);
+
+ error = (error || in_width > maxsinglelinewidth * 2 ||
+ (in_width > maxsinglelinewidth && *five_taps) ||
+ !*core_clk || *core_clk > dispc_core_clk_rate());
+ if (error) {
+ if (*decim_x = *decim_y) {
+ *decim_x = min_factor;
+ ++*decim_y;
+ } else {
+ swap(*decim_x, *decim_y);
+ if (*decim_x < *decim_y)
+ ++*decim_x;
+ }
+ }
+ } while (*decim_x <= *x_predecim && *decim_y <= *y_predecim && error);
+
+ if (check_horiz_timing_omap3(channel, mgr_timings, pos_x, width, height,
+ out_width, out_height)){
+ DSSERR("horizontal timing too tight\n");
+ return -EINVAL;
+ }
+
+ if (in_width > (maxsinglelinewidth * 2)) {
+ DSSERR("Cannot setup scaling");
+ DSSERR("width exceeds maximum width possible");
+ return -EINVAL;
+ }
+
+ if (in_width > maxsinglelinewidth && *five_taps) {
+ DSSERR("cannot setup scaling with five taps");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int dispc_ovl_calc_scaling_44xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk)
+{
+ u16 in_width, in_width_max;
+ int decim_x_min = *decim_x;
+ u16 in_height = DIV_ROUND_UP(height, *decim_y);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+
+ in_width_max = dispc_core_clk_rate() /
+ DIV_ROUND_UP(dispc_mgr_pclk_rate(channel), out_width);
+ *decim_x = DIV_ROUND_UP(width, in_width_max);
+
+ *decim_x = *decim_x > decim_x_min ? *decim_x : decim_x_min;
+ if (*decim_x > *x_predecim)
+ return -EINVAL;
+
+ do {
+ in_width = DIV_ROUND_UP(width, *decim_x);
+ } while (*decim_x <= *x_predecim &&
+ in_width > maxsinglelinewidth && ++*decim_x);
+
+ if (in_width > maxsinglelinewidth) {
+ DSSERR("Cannot scale width exceeds max line width");
+ return -EINVAL;
}
+
+ *core_clk = dispc.feat->calc_core_clk(channel, in_width, in_height,
+ out_width, out_height);
+ return 0;
}
static int dispc_ovl_calc_scaling(enum omap_plane plane,
@@ -1988,12 +2215,9 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
{
struct omap_overlay *ovl = omap_dss_get_overlay(plane);
const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
- const int maxsinglelinewidth - dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
const int max_decim_limit = 16;
unsigned long core_clk = 0;
- int decim_x, decim_y, error, min_factor;
- u16 in_width, in_height, in_width_max = 0;
+ int decim_x, decim_y, ret;
if (width = out_width && height = out_height)
return 0;
@@ -2017,118 +2241,17 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
decim_x = DIV_ROUND_UP(DIV_ROUND_UP(width, out_width), maxdownscale);
decim_y = DIV_ROUND_UP(DIV_ROUND_UP(height, out_height), maxdownscale);
- min_factor = min(decim_x, decim_y);
-
if (decim_x > *x_predecim || out_width > width * 8)
return -EINVAL;
if (decim_y > *y_predecim || out_height > height * 8)
return -EINVAL;
- if (cpu_is_omap24xx()) {
- *five_taps = false;
-
- do {
- in_height = DIV_ROUND_UP(height, decim_y);
- in_width = DIV_ROUND_UP(width, decim_x);
- core_clk = calc_core_clk(channel, in_width, in_height,
- out_width, out_height);
- error = (in_width > maxsinglelinewidth || !core_clk ||
- core_clk > dispc_core_clk_rate());
- if (error) {
- if (decim_x = decim_y) {
- decim_x = min_factor;
- decim_y++;
- } else {
- swap(decim_x, decim_y);
- if (decim_x < decim_y)
- decim_x++;
- }
- }
- } while (decim_x <= *x_predecim && decim_y <= *y_predecim &&
- error);
-
- if (in_width > maxsinglelinewidth) {
- DSSERR("Cannot scale max input width exceeded");
- return -EINVAL;
- }
- } else if (cpu_is_omap34xx()) {
-
- do {
- in_height = DIV_ROUND_UP(height, decim_y);
- in_width = DIV_ROUND_UP(width, decim_x);
- core_clk = calc_core_clk_five_taps(channel, mgr_timings,
- in_width, in_height, out_width, out_height,
- color_mode);
-
- error = check_horiz_timing_omap3(channel, mgr_timings,
- pos_x, in_width, in_height, out_width,
- out_height);
-
- if (in_width > maxsinglelinewidth)
- if (in_height > out_height &&
- in_height < out_height * 2)
- *five_taps = false;
- if (!*five_taps)
- core_clk = calc_core_clk(channel, in_width,
- in_height, out_width, out_height);
- error = (error || in_width > maxsinglelinewidth * 2 ||
- (in_width > maxsinglelinewidth && *five_taps) ||
- !core_clk || core_clk > dispc_core_clk_rate());
- if (error) {
- if (decim_x = decim_y) {
- decim_x = min_factor;
- decim_y++;
- } else {
- swap(decim_x, decim_y);
- if (decim_x < decim_y)
- decim_x++;
- }
- }
- } while (decim_x <= *x_predecim && decim_y <= *y_predecim
- && error);
-
- if (check_horiz_timing_omap3(channel, mgr_timings, pos_x, width,
- height, out_width, out_height)){
- DSSERR("horizontal timing too tight\n");
- return -EINVAL;
- }
-
- if (in_width > (maxsinglelinewidth * 2)) {
- DSSERR("Cannot setup scaling");
- DSSERR("width exceeds maximum width possible");
- return -EINVAL;
- }
-
- if (in_width > maxsinglelinewidth && *five_taps) {
- DSSERR("cannot setup scaling with five taps");
- return -EINVAL;
- }
- } else {
- int decim_x_min = decim_x;
- in_height = DIV_ROUND_UP(height, decim_y);
- in_width_max = dispc_core_clk_rate() /
- DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
- out_width);
- decim_x = DIV_ROUND_UP(width, in_width_max);
-
- decim_x = decim_x > decim_x_min ? decim_x : decim_x_min;
- if (decim_x > *x_predecim)
- return -EINVAL;
-
- do {
- in_width = DIV_ROUND_UP(width, decim_x);
- } while (decim_x <= *x_predecim &&
- in_width > maxsinglelinewidth && decim_x++);
-
- if (in_width > maxsinglelinewidth) {
- DSSERR("Cannot scale width exceeds max line width");
- return -EINVAL;
- }
-
- core_clk = calc_core_clk(channel, in_width, in_height,
- out_width, out_height);
- }
+ ret = dispc.feat->calc_scaling(channel, mgr_timings, width, height,
+ out_width, out_height, color_mode, five_taps, x_predecim,
+ y_predecim, &decim_x, &decim_y, pos_x, &core_clk);
+ if (ret)
+ return ret;
DSSDBG("required core clk rate = %lu Hz\n", core_clk);
DSSDBG("current core clk rate = %lu Hz\n", dispc_core_clk_rate());
@@ -2601,27 +2724,28 @@ static bool _dispc_mgr_size_ok(u16 width, u16 height)
height <= dss_feat_get_param_max(FEAT_PARAM_MGR_HEIGHT);
}
-static bool _dispc_lcd_timings_ok(int hsw, int hfp, int hbp,
+static bool _dispc_lcd_timings_ok_24xx(int hsw, int hfp, int hbp,
int vsw, int vfp, int vbp)
{
- if (cpu_is_omap24xx() || omap_rev() < OMAP3430_REV_ES3_0) {
- if (hsw < 1 || hsw > 64 ||
- hfp < 1 || hfp > 256 ||
- hbp < 1 || hbp > 256 ||
- vsw < 1 || vsw > 64 ||
- vfp < 0 || vfp > 255 ||
- vbp < 0 || vbp > 255)
- return false;
- } else {
- if (hsw < 1 || hsw > 256 ||
- hfp < 1 || hfp > 4096 ||
- hbp < 1 || hbp > 4096 ||
- vsw < 1 || vsw > 256 ||
- vfp < 0 || vfp > 4095 ||
- vbp < 0 || vbp > 4095)
- return false;
- }
-
+ if (hsw < 1 || hsw > 64 ||
+ hfp < 1 || hfp > 256 ||
+ hbp < 1 || hbp > 256 ||
+ vsw < 1 || vsw > 64 ||
+ vfp < 0 || vfp > 255 ||
+ vbp < 0 || vbp > 255)
+ return false;
+ return true;
+}
+static bool _dispc_lcd_timings_ok_44xx(int hsw, int hfp, int hbp,
+ int vsw, int vfp, int vbp)
+{
+ if (hsw < 1 || hsw > 256 ||
+ hfp < 1 || hfp > 4096 ||
+ hbp < 1 || hbp > 4096 ||
+ vsw < 1 || vsw > 256 ||
+ vfp < 0 || vfp > 4095 ||
+ vbp < 0 || vbp > 4095)
+ return false;
return true;
}
@@ -2633,7 +2757,8 @@ bool dispc_mgr_timings_ok(enum omap_channel channel,
timings_ok = _dispc_mgr_size_ok(timings->x_res, timings->y_res);
if (dss_mgr_is_lcd(channel))
- timings_ok = timings_ok && _dispc_lcd_timings_ok(timings->hsw,
+ timings_ok = timings_ok &&
+ dispc.feat->lcd_timings_ok(timings->hsw,
timings->hfp, timings->hbp,
timings->vsw, timings->vfp,
timings->vbp);
@@ -2641,6 +2766,34 @@ bool dispc_mgr_timings_ok(enum omap_channel channel,
return timings_ok;
}
+static void _dispc_mgr_set_lcd_timings_hv_24xx(enum omap_channel channel,
+ int hsw, int hfp, int hbp, int vsw, int vfp, int vbp)
+{
+ u32 timing_h, timing_v;
+
+ timing_h = FLD_VAL(hsw-1, 5, 0) | FLD_VAL(hfp-1, 15, 8) |
+ FLD_VAL(hbp-1, 27, 20);
+ timing_v = FLD_VAL(vsw-1, 5, 0) | FLD_VAL(vfp, 15, 8) |
+ FLD_VAL(vbp, 27, 20);
+
+ dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
+ dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
+}
+
+static void _dispc_mgr_set_lcd_timings_hv_44xx(enum omap_channel channel,
+ int hsw, int hfp, int hbp, int vsw, int vfp, int vbp)
+{
+ u32 timing_h, timing_v;
+
+ timing_h = FLD_VAL(hsw-1, 7, 0) | FLD_VAL(hfp-1, 19, 8) |
+ FLD_VAL(hbp-1, 31, 20);
+ timing_v = FLD_VAL(vsw-1, 7, 0) | FLD_VAL(vfp, 19, 8) |
+ FLD_VAL(vbp, 31, 20);
+
+ dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
+ dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
+}
+
static void _dispc_mgr_set_lcd_timings(enum omap_channel channel, int hsw,
int hfp, int hbp, int vsw, int vfp, int vbp,
enum omap_dss_signal_level vsync_level,
@@ -2650,25 +2803,10 @@ static void _dispc_mgr_set_lcd_timings(enum omap_channel channel, int hsw,
enum omap_dss_signal_edge sync_pclk_edge)
{
- u32 timing_h, timing_v, l;
+ u32 l;
bool onoff, rf, ipc;
- if (cpu_is_omap24xx() || omap_rev() < OMAP3430_REV_ES3_0) {
- timing_h = FLD_VAL(hsw-1, 5, 0) | FLD_VAL(hfp-1, 15, 8) |
- FLD_VAL(hbp-1, 27, 20);
-
- timing_v = FLD_VAL(vsw-1, 5, 0) | FLD_VAL(vfp, 15, 8) |
- FLD_VAL(vbp, 27, 20);
- } else {
- timing_h = FLD_VAL(hsw-1, 7, 0) | FLD_VAL(hfp-1, 19, 8) |
- FLD_VAL(hbp-1, 31, 20);
-
- timing_v = FLD_VAL(vsw-1, 7, 0) | FLD_VAL(vfp, 19, 8) |
- FLD_VAL(vbp, 31, 20);
- }
-
- dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
- dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
+ dispc.feat->set_lcd_timings_hv(channel, hsw, hfp, hbp, vsw, vfp, vbp);
switch (data_pclk_edge) {
case OMAPDSS_DRIVE_SIG_RISING_EDGE:
@@ -3671,6 +3809,20 @@ static void _omap_dispc_initial_config(void)
dispc_ovl_enable_zorder_planes();
}
+static void dispc_init_features(void)
+{
+ if (cpu_is_omap24xx()) {
+ dispc.feat = &omap2_dispc_features;
+ } else if (cpu_is_omap34xx()) {
+ if (omap_rev() < OMAP3430_REV_ES3_0)
+ dispc.feat = &omap3_2_1_dispc_features;
+ else
+ dispc.feat = &omap3_3_0_dispc_features;
+ } else {
+ dispc.feat = &omap4_dispc_features;
+ }
+}
+
/* DISPC HW IP initialisation */
static int __init omap_dispchw_probe(struct platform_device *pdev)
{
@@ -3725,6 +3877,8 @@ static int __init omap_dispchw_probe(struct platform_device *pdev)
dispc.dss_clk = clk;
+ dispc_init_features();
+
pm_runtime_enable(&pdev->dev);
r = dispc_runtime_get();
--
1.7.10
^ permalink raw reply related
* Re: [PATCH 1/2] ARM: pgtable: add pgprot_writethrough() macro
From: Grazvydas Ignotas @ 2012-08-08 11:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1337716444-30139-2-git-send-email-siarhei.siamashka@gmail.com>
On Tue, May 22, 2012 at 10:54 PM, Siarhei Siamashka
<siarhei.siamashka@gmail.com> wrote:
> Needed for remapping pages with write-through cacheable
> attribute. May be useful for framebuffers.
With this series applied, some of my framebuffer programs get over 70%
performance improvement, could we get this patch in? OMAP DSS
maintainer agreed to take the second patch if this can be merged.
> Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
> ---
> arch/arm/include/asm/pgtable.h | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
> index f66626d..04297fa 100644
> --- a/arch/arm/include/asm/pgtable.h
> +++ b/arch/arm/include/asm/pgtable.h
> @@ -103,6 +103,9 @@ extern pgprot_t pgprot_kernel;
> #define pgprot_stronglyordered(prot) \
> __pgprot_modify(prot, L_PTE_MT_MASK, L_PTE_MT_UNCACHED)
>
> +#define pgprot_writethrough(prot) \
> + __pgprot_modify(prot, L_PTE_MT_MASK, L_PTE_MT_WRITETHROUGH)
> +
> #ifdef CONFIG_ARM_DMA_MEM_BUFFERABLE
> #define pgprot_dmacoherent(prot) \
> __pgprot_modify(prot, L_PTE_MT_MASK, L_PTE_MT_BUFFERABLE | L_PTE_XN)
> --
> 1.7.3.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Gražvydas
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Archit Taneja @ 2012-08-08 9:36 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344415685.4932.14.camel@deskari>
On Wednesday 08 August 2012 02:18 PM, Tomi Valkeinen wrote:
> On Wed, 2012-08-08 at 14:08 +0530, Archit Taneja wrote:
>> On Wednesday 08 August 2012 01:43 PM, Tomi Valkeinen wrote:
>>> On Wed, 2012-08-08 at 13:29 +0530, Archit Taneja wrote:
>>>
>>>> Okay, one thing which I want to align on is that most of these functions
>>>> don't really do the actual configurations. That is, they'll just update
>>>> the private data, and the actual configuration will only happen on enable.
>>>>
>>>> We would want set_timings() op to have a direct impact. But we wouldn't
>>>> want the same for setting the data lines, that could be clubbed with
>>>> other configurations at enable. That's okay, right?
>>>
>>> I'm not sure we want/need set_timings to have direct impact. Changing
>>> the timings on the fly has some problems, like the output size changing
>>> to smaller than the overlays, and we perhaps may need to adjust the
>>> clock dividers (dispc's, DSI PLL's or PRCM's).
>>>
>>> It feels just much easier and safer to require that the mgr is disabled
>>> when these changes are made. And as far as I can see, there shouldn't be
>>> any need to change the timings via the shadow registers, as quickly as
>>> possible and during vblank...
>>
>> That makes sense. But currently set_timings for DPI has a direct impact.
>> HDMI/VENC/SDI take the easier route of disabling and enabling the interface.
>>
>> I agree it's safer and easier to make sure things are disabled first,
>> but maybe it's good to have the capability set hdmi timings on the fly
>> in the future, it would make the switch faster, same goes for reading edid.
>
> When do we need to switch mode quickly? Reading edid should not require
> disabling the output for sure.
I think I'm just finding excuses to find a use for my work done in
APPLYing manager related registers.
You are right about edid. Changing HDMI timings take a couple of seconds
now, I was wondering how much that has to do with us completely
disabling/enabling hdmi. it may be just the slowness of the monitors
which causes this.
>
> HDMI is a bit broken currently, though. I think we first enable the
> whole stuff, including video output using VGA, then we read EDID, then
> we change the mode.
>
> We should just enable enough of HDMI to be able to read EDID, and start
> the video output with the correct mode. This needs some restructuring of
> the driver, though. I tried it once quickly, but it turned out not to be
> trivial.
Right. Most likely this restructuring would allow us to modify only the
hdmi timings part when setting a new timing. We could check how much
time we save then :)
>
>> What I meant to ask was whether we should do the same for something like
>> dpi_set_data_lines(), that is, disable dpi, update the data_lines
>> private data with a new value, and enable dpi again.
>
> Hmm, I think it's better to leave disabling and enabling the output to
> the panel driver. So when the panel driver wants to use
> dpi_set_data_lines(), it needs to first disable the DPI output. If it
> doesn't, dpi_set_data_lines() returns -EBUSY.
>
> Otherwise if the panel driver does something like:
>
> dpi_set_foo()
> dpi_set_bar()
>
> Both of those could first disable output, change setting, enable output.
> Instead the panel should first disable, then call those, and then
> enable.
Right, that makes sense.
Archit
^ permalink raw reply
* Re: [rtc-linux] [PATCH 8/8] ARM: vt8500: gpio: Devicetree support for arch-vt8500
From: Arnd Bergmann @ 2012-08-08 9:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdbYxpN9RCu8j8KMBgrb=-DsYZHq7hx-ifmHhJ0ZZcRYjw@mail.gmail.com>
On Wednesday 08 August 2012, Linus Walleij wrote:
> On Wed, Aug 8, 2012 at 3:39 AM, Tony Prisk <linux@prisktech.co.nz> wrote:
>
> > Converted the existing arch-vt8500 gpio to a platform_device.
> > Added support for WM8505 and WM8650 GPIO controllers.
> (...)
> > +++ b/drivers/gpio/gpio-vt8500.c
>
> This driver looks very one-bit-per-gpio typed. Are you sure you cannot
> just reuse drivers/gpio/gpio-generic.c? Make a compelling case please...
>
> > +struct vt8500_gpio_bank_regs {
> > + int en;
> > + int dir;
> > + int data_out;
> > + int data_in;
>
> Why are all these members int? They should be u8 from reading your code.
>
> > + int ngpio;
> > +};
Not necessarily 8 bit, but definitely unsigned.
> > +static struct vt8500_gpio_data vt8500_data = {
> > + .num_banks = 7,
> > + .banks = {
> > + VT8500_BANK(0x00, 0x20, 0x40, 0x60, 26),
> > + VT8500_BANK(0x04, 0x24, 0x44, 0x64, 28),
> > + VT8500_BANK(0x08, 0x28, 0x48, 0x68, 31),
> > + VT8500_BANK(0x0C, 0x2C, 0x4C, 0x6C, 19),
> > + VT8500_BANK(0x10, 0x30, 0x50, 0x70, 19),
> > + VT8500_BANK(0x14, 0x34, 0x54, 0x74, 23),
> > + VT8500_BANK(-1, 0x3C, 0x5C, 0x7C, 9), /* external gpio */
>
> What on earth are all those magic numbers?
>
> I *guess* they're enabling some default GPIO settings etc.
No, they are the register offsets you quoted above, per bank. There
is no easy way to abstract these, and I suggested putting the
values into the source code rather than describing each bank
separately in the .dtsi file.
My feeling however is that the "vt8500_chip->regoff" is wrong, which
would mean only the first bank works. The code adds the same offsets
per bank once more that it sets in this bank table.
Arnd
^ permalink raw reply
* Re: [rtc-linux] [PATCH 8/8] ARM: vt8500: gpio: Devicetree support for arch-vt8500
From: Linus Walleij @ 2012-08-08 9:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1344389967-8465-9-git-send-email-linux@prisktech.co.nz>
On Wed, Aug 8, 2012 at 3:39 AM, Tony Prisk <linux@prisktech.co.nz> wrote:
> Converted the existing arch-vt8500 gpio to a platform_device.
> Added support for WM8505 and WM8650 GPIO controllers.
(...)
> +++ b/drivers/gpio/gpio-vt8500.c
This driver looks very one-bit-per-gpio typed. Are you sure you cannot
just reuse drivers/gpio/gpio-generic.c? Make a compelling case please...
> +struct vt8500_gpio_bank_regs {
> + int en;
> + int dir;
> + int data_out;
> + int data_in;
Why are all these members int? They should be u8 from reading your code.
> + int ngpio;
> +};
> +static struct vt8500_gpio_data vt8500_data = {
> + .num_banks = 7,
> + .banks = {
> + VT8500_BANK(0x00, 0x20, 0x40, 0x60, 26),
> + VT8500_BANK(0x04, 0x24, 0x44, 0x64, 28),
> + VT8500_BANK(0x08, 0x28, 0x48, 0x68, 31),
> + VT8500_BANK(0x0C, 0x2C, 0x4C, 0x6C, 19),
> + VT8500_BANK(0x10, 0x30, 0x50, 0x70, 19),
> + VT8500_BANK(0x14, 0x34, 0x54, 0x74, 23),
> + VT8500_BANK(-1, 0x3C, 0x5C, 0x7C, 9), /* external gpio */
What on earth are all those magic numbers?
I *guess* they're enabling some default GPIO settings etc.
But it really needs better structure, #defines for each one or
atleast include <linux/bitops.h> and say:
= BIT(4) | /* Enable GPIO pin 5 on this bank */
BIT(5); /* Enable GPIO pin 6 on this bank */
However I suspect this is board specific and should
be taken from device tree. Please elaborate on this...
Ditto for the different instances.
(...)
> + unsigned val;
Looks like all of these should be u8.
> + val = readl(vt8500_chip->base + vt8500_chip->regs->en +
> + vt8500_chip->regoff);
val = (u8) readl(...);
usw
> + val |= (1 << offset);
Use <linux/bitops.h>
val |= BIT(offset);
Apart from these remarks it's looking good...
Yours,
Linus Walleij
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Archit Taneja @ 2012-08-08 8:50 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344413580.4932.5.camel@deskari>
On Wednesday 08 August 2012 01:43 PM, Tomi Valkeinen wrote:
> On Wed, 2012-08-08 at 13:29 +0530, Archit Taneja wrote:
>
>> Okay, one thing which I want to align on is that most of these functions
>> don't really do the actual configurations. That is, they'll just update
>> the private data, and the actual configuration will only happen on enable.
>>
>> We would want set_timings() op to have a direct impact. But we wouldn't
>> want the same for setting the data lines, that could be clubbed with
>> other configurations at enable. That's okay, right?
>
> I'm not sure we want/need set_timings to have direct impact. Changing
> the timings on the fly has some problems, like the output size changing
> to smaller than the overlays, and we perhaps may need to adjust the
> clock dividers (dispc's, DSI PLL's or PRCM's).
>
> It feels just much easier and safer to require that the mgr is disabled
> when these changes are made. And as far as I can see, there shouldn't be
> any need to change the timings via the shadow registers, as quickly as
> possible and during vblank...
That makes sense. But currently set_timings for DPI has a direct impact.
HDMI/VENC/SDI take the easier route of disabling and enabling the interface.
I agree it's safer and easier to make sure things are disabled first,
but maybe it's good to have the capability set hdmi timings on the fly
in the future, it would make the switch faster, same goes for reading edid.
What I meant to ask was whether we should do the same for something like
dpi_set_data_lines(), that is, disable dpi, update the data_lines
private data with a new value, and enable dpi again.
>
> This makes me also think that if the output related settings can only be
> changed when the output is off, the apply mechanism is not really needed
> at all for these. Not that it causes any harm, but just a point I
> realized.
Hmm, unfortunately you are right. It's still good to have all the DISPC
writes only in APPLY though, and it gives us the option to do some
operation on the fly if needed in the future.
Archit
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Tomi Valkeinen @ 2012-08-08 8:48 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <50222575.6050807@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2794 bytes --]
On Wed, 2012-08-08 at 14:08 +0530, Archit Taneja wrote:
> On Wednesday 08 August 2012 01:43 PM, Tomi Valkeinen wrote:
> > On Wed, 2012-08-08 at 13:29 +0530, Archit Taneja wrote:
> >
> >> Okay, one thing which I want to align on is that most of these functions
> >> don't really do the actual configurations. That is, they'll just update
> >> the private data, and the actual configuration will only happen on enable.
> >>
> >> We would want set_timings() op to have a direct impact. But we wouldn't
> >> want the same for setting the data lines, that could be clubbed with
> >> other configurations at enable. That's okay, right?
> >
> > I'm not sure we want/need set_timings to have direct impact. Changing
> > the timings on the fly has some problems, like the output size changing
> > to smaller than the overlays, and we perhaps may need to adjust the
> > clock dividers (dispc's, DSI PLL's or PRCM's).
> >
> > It feels just much easier and safer to require that the mgr is disabled
> > when these changes are made. And as far as I can see, there shouldn't be
> > any need to change the timings via the shadow registers, as quickly as
> > possible and during vblank...
>
> That makes sense. But currently set_timings for DPI has a direct impact.
> HDMI/VENC/SDI take the easier route of disabling and enabling the interface.
>
> I agree it's safer and easier to make sure things are disabled first,
> but maybe it's good to have the capability set hdmi timings on the fly
> in the future, it would make the switch faster, same goes for reading edid.
When do we need to switch mode quickly? Reading edid should not require
disabling the output for sure.
HDMI is a bit broken currently, though. I think we first enable the
whole stuff, including video output using VGA, then we read EDID, then
we change the mode.
We should just enable enough of HDMI to be able to read EDID, and start
the video output with the correct mode. This needs some restructuring of
the driver, though. I tried it once quickly, but it turned out not to be
trivial.
> What I meant to ask was whether we should do the same for something like
> dpi_set_data_lines(), that is, disable dpi, update the data_lines
> private data with a new value, and enable dpi again.
Hmm, I think it's better to leave disabling and enabling the output to
the panel driver. So when the panel driver wants to use
dpi_set_data_lines(), it needs to first disable the DPI output. If it
doesn't, dpi_set_data_lines() returns -EBUSY.
Otherwise if the panel driver does something like:
dpi_set_foo()
dpi_set_bar()
Both of those could first disable output, change setting, enable output.
Instead the panel should first disable, then call those, and then
enable.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Tomi Valkeinen @ 2012-08-08 8:13 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <50221C4D.8000503@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1252 bytes --]
On Wed, 2012-08-08 at 13:29 +0530, Archit Taneja wrote:
> Okay, one thing which I want to align on is that most of these functions
> don't really do the actual configurations. That is, they'll just update
> the private data, and the actual configuration will only happen on enable.
>
> We would want set_timings() op to have a direct impact. But we wouldn't
> want the same for setting the data lines, that could be clubbed with
> other configurations at enable. That's okay, right?
I'm not sure we want/need set_timings to have direct impact. Changing
the timings on the fly has some problems, like the output size changing
to smaller than the overlays, and we perhaps may need to adjust the
clock dividers (dispc's, DSI PLL's or PRCM's).
It feels just much easier and safer to require that the mgr is disabled
when these changes are made. And as far as I can see, there shouldn't be
any need to change the timings via the shadow registers, as quickly as
possible and during vblank...
This makes me also think that if the output related settings can only be
changed when the output is off, the apply mechanism is not really needed
at all for these. Not that it causes any harm, but just a point I
realized.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Archit Taneja @ 2012-08-08 8:11 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344410835.17575.54.camel@lappyti>
On Wednesday 08 August 2012 12:57 PM, Tomi Valkeinen wrote:
> On Wed, 2012-08-08 at 12:17 +0530, Archit Taneja wrote:
>> On Wednesday 08 August 2012 11:55 AM, Tomi Valkeinen wrote:
>>> On Wed, 2012-08-08 at 11:35 +0530, Archit Taneja wrote:
>>>> On Tuesday 07 August 2012 08:02 PM, Tomi Valkeinen wrote:
>>>>> On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
>>>>>> This series tries to make interface drivers less dependent on omap_dss_device
>>>>>> which represents a panel/device connected to that interface. The current way of
>>>>>> configuring an interface is to populate the panel's omap_dss_device instance
>>>>>> with parameters common to the panel and the interface, they are either populated
>>>>>> in the board file, or in the panel driver. Panel timings, number of lanes
>>>>>> connected to interface, and pixel format are examples of such parameters, these
>>>>>> are then extracted by the interface driver to configure itself.
>>>>>
>>>>> The series looks good. I had only a few comments to make, but obviously
>>>>> this needs quite a bit of testing. I'll try it out.
>>>>
>>>> One thing I'm not sure about is whether these new functions should be
>>>> aware of the state of the output. For example, if we call set_timings()
>>>> with DSI video mode which is already enabled, the timings won't really
>>>> take any impact.
>>>>
>>>> Similar issues would occur when we try to make other ops like
>>>> set_data_lines() or set_pixel_format(). These need to be called before
>>>> the output is enabled. I was wondering if we would need to add
>>>> intelligence here to make panel drivers less likely to make mistakes.
>>>
>>> Hmm, true. It'd be nice if the functions returned -EBUSY if the
>>> operation cannot be done while the output is enabled.
>>>
>>> We have the dssdev->state, but we should get rid of that (or leave it to
>>> panel drivers). It'd be good if the output drivers know whether the
>>> output is enabled or not. I think this data is already tracked by
>>> apply.c. It's about ovl managers, but I think that's practically the
>>> same as output.
>>>
>>> Calling dss_mgr_enable() will set mp->enabled = true, which could be
>>> returned via dss_mgr_is_enabled() or such.
>>>
>>> Then again, it wouldn't be many lines of codes to track the enable-state
>>> in each output driver. So if we have any suspicions that mp->enabled
>>> doesn't quite work for, say, dsi, we could just add a private "enabled"
>>> member to dsi. But I don't right away see why dss_mgr_is_enabled()
>>> wouldn't work.
>>>
>>
>> I think we had discussed previously that it may not the best idea to see
>> if a manager is enabled via mp->enabled as it's always possible that it
>> changes afterwards. Same for any other parameter in APPLY's private
>> data. This was the reason why we passed privtate data to DISPC functions
>> rather than creating apply helper functions which return the value of a
>> private data. For example, we pass manager timings to dispc_ovl_setup(),
>> instead of DISPC using a function like dss_mgr_get_timings().
>
> I think that's slightly different problem. The dispc case has an issue
> with locking. If dispc_ovl_setup() is called with the apply's spinlock
> taken, neither dispc_ovl_setup() nor dss_mgr_get_timings() can take the
> lock. But if dispc_ovl_setup() is called from somewhere else, it should
> take the lock. Also, if dispc_ovl_setup() would call a function in
> apply, it'd be calling "upwards" to a higher level component.
>
> With the output driver calling apply, none of those problems is present,
> I believe.
>
>> I also don't see why dss_mgr_is_enabled() wouldn't work. The only places
>> where the manager's state will change are the output's enable and
>> disable ops. The mutex maintained by the output would ensure
>> sequential-ity between the output's enable() and set_timings() op, and
>> hence ensure the manager's state we see is fine.
>>
>> If we manage the 'enabled' state for each output interface, we would be
>> a bit more consistent with respect to other parameters. For example,
>> timings is maintained by both manager and the output. Also, if we need
>> to separate out manager configurations from outputs in the future, it
>> would probably be better for the output to query it's own state rather
>> than depending on the manager, which could be configured either earlier
>> or later.
>
> Two things that came to my mind:
>
> If the output driver uses dss_mgr_is_enabled(), if both DPI and DSI
> output drivers use the same manager, they'd both see themselves as
> enabled. Of course only one can work at a time, so I'm not sure if
> that's a practical problem. And if we had some kind of link between the
> mgr and the output driver this would not be an issue.
>
> The second thing is that we're not strictly required to have DISPC
> connected to DSI or RFBI. We could use CPU/sDMA to output the image.
> This is quite theoretical, though.
>
> So, I think using dss_mgr_is_enabled() would work, but I'm still not
> 100% sure...
>
> Well, perhaps the code should be such that dss_mgr_is_enabled() is used
> to see if the mgr is enabled, not if the output is enabled. What I mean
> with this is that if, say, set_data_lines() calls dispc to set the data
> lines, we are really interested in if the dispc's mgr is enabled, not if
> the DSI is enabled.
>
> And if some other function changes DSI configuration (but doesn't touch
> dispc), then we're not really interested in if the mgr is enabled, but
> if the DSI is enabled.
>
> That's a bit more complex than using only dss_mgr_is_enabled() or using
> only output specific enable-flag, but I think it's more correct. In
> DPI's case only dss_mgr_is_enabled() is probably needed. For DSI we may
> need a separate private enable-flag.
Okay, one thing which I want to align on is that most of these functions
don't really do the actual configurations. That is, they'll just update
the private data, and the actual configuration will only happen on enable.
We would want set_timings() op to have a direct impact. But we wouldn't
want the same for setting the data lines, that could be clubbed with
other configurations at enable. That's okay, right?
Archit
^ permalink raw reply
* Re: [RFC 08/17] OMAPDSS: DSI: Maintain own copy of timings in driver data
From: Archit Taneja @ 2012-08-08 8:06 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344409847.17575.40.camel@lappyti>
On Wednesday 08 August 2012 12:40 PM, Tomi Valkeinen wrote:
> On Wed, 2012-08-08 at 11:59 +0530, Archit Taneja wrote:
>
>> I tried out rotation on Taal, and it only works for 180 degrees(and 0 of
>> course), 90 and 270 result in no output. I'll add a
>> dss_mgr_set_timings() in omap_dsi_update, that should sort of fix it,
>> but someone would need to reconfigure the connected overlays too before
>> trying out an update.
>
> Right, but that's something omapdss/panel cannot do, it must be done by
> the user. The same problem is there with changing, say, DPI mode also.
>
> Btw, can you separate smaller cleanups/fixes to another patch series, to
> make this series even slightly smaller? I think at least the first
> patches in this series are quite separate, and the rotation fix is also.
Right, I'll do that.
Archit
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Tomi Valkeinen @ 2012-08-08 7:27 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <50220B94.5040303@ti.com>
[-- Attachment #1: Type: text/plain, Size: 5740 bytes --]
On Wed, 2012-08-08 at 12:17 +0530, Archit Taneja wrote:
> On Wednesday 08 August 2012 11:55 AM, Tomi Valkeinen wrote:
> > On Wed, 2012-08-08 at 11:35 +0530, Archit Taneja wrote:
> >> On Tuesday 07 August 2012 08:02 PM, Tomi Valkeinen wrote:
> >>> On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
> >>>> This series tries to make interface drivers less dependent on omap_dss_device
> >>>> which represents a panel/device connected to that interface. The current way of
> >>>> configuring an interface is to populate the panel's omap_dss_device instance
> >>>> with parameters common to the panel and the interface, they are either populated
> >>>> in the board file, or in the panel driver. Panel timings, number of lanes
> >>>> connected to interface, and pixel format are examples of such parameters, these
> >>>> are then extracted by the interface driver to configure itself.
> >>>
> >>> The series looks good. I had only a few comments to make, but obviously
> >>> this needs quite a bit of testing. I'll try it out.
> >>
> >> One thing I'm not sure about is whether these new functions should be
> >> aware of the state of the output. For example, if we call set_timings()
> >> with DSI video mode which is already enabled, the timings won't really
> >> take any impact.
> >>
> >> Similar issues would occur when we try to make other ops like
> >> set_data_lines() or set_pixel_format(). These need to be called before
> >> the output is enabled. I was wondering if we would need to add
> >> intelligence here to make panel drivers less likely to make mistakes.
> >
> > Hmm, true. It'd be nice if the functions returned -EBUSY if the
> > operation cannot be done while the output is enabled.
> >
> > We have the dssdev->state, but we should get rid of that (or leave it to
> > panel drivers). It'd be good if the output drivers know whether the
> > output is enabled or not. I think this data is already tracked by
> > apply.c. It's about ovl managers, but I think that's practically the
> > same as output.
> >
> > Calling dss_mgr_enable() will set mp->enabled = true, which could be
> > returned via dss_mgr_is_enabled() or such.
> >
> > Then again, it wouldn't be many lines of codes to track the enable-state
> > in each output driver. So if we have any suspicions that mp->enabled
> > doesn't quite work for, say, dsi, we could just add a private "enabled"
> > member to dsi. But I don't right away see why dss_mgr_is_enabled()
> > wouldn't work.
> >
>
> I think we had discussed previously that it may not the best idea to see
> if a manager is enabled via mp->enabled as it's always possible that it
> changes afterwards. Same for any other parameter in APPLY's private
> data. This was the reason why we passed privtate data to DISPC functions
> rather than creating apply helper functions which return the value of a
> private data. For example, we pass manager timings to dispc_ovl_setup(),
> instead of DISPC using a function like dss_mgr_get_timings().
I think that's slightly different problem. The dispc case has an issue
with locking. If dispc_ovl_setup() is called with the apply's spinlock
taken, neither dispc_ovl_setup() nor dss_mgr_get_timings() can take the
lock. But if dispc_ovl_setup() is called from somewhere else, it should
take the lock. Also, if dispc_ovl_setup() would call a function in
apply, it'd be calling "upwards" to a higher level component.
With the output driver calling apply, none of those problems is present,
I believe.
> I also don't see why dss_mgr_is_enabled() wouldn't work. The only places
> where the manager's state will change are the output's enable and
> disable ops. The mutex maintained by the output would ensure
> sequential-ity between the output's enable() and set_timings() op, and
> hence ensure the manager's state we see is fine.
>
> If we manage the 'enabled' state for each output interface, we would be
> a bit more consistent with respect to other parameters. For example,
> timings is maintained by both manager and the output. Also, if we need
> to separate out manager configurations from outputs in the future, it
> would probably be better for the output to query it's own state rather
> than depending on the manager, which could be configured either earlier
> or later.
Two things that came to my mind:
If the output driver uses dss_mgr_is_enabled(), if both DPI and DSI
output drivers use the same manager, they'd both see themselves as
enabled. Of course only one can work at a time, so I'm not sure if
that's a practical problem. And if we had some kind of link between the
mgr and the output driver this would not be an issue.
The second thing is that we're not strictly required to have DISPC
connected to DSI or RFBI. We could use CPU/sDMA to output the image.
This is quite theoretical, though.
So, I think using dss_mgr_is_enabled() would work, but I'm still not
100% sure...
Well, perhaps the code should be such that dss_mgr_is_enabled() is used
to see if the mgr is enabled, not if the output is enabled. What I mean
with this is that if, say, set_data_lines() calls dispc to set the data
lines, we are really interested in if the dispc's mgr is enabled, not if
the DSI is enabled.
And if some other function changes DSI configuration (but doesn't touch
dispc), then we're not really interested in if the mgr is enabled, but
if the DSI is enabled.
That's a bit more complex than using only dss_mgr_is_enabled() or using
only output specific enable-flag, but I think it's more correct. In
DPI's case only dss_mgr_is_enabled() is probably needed. For DSI we may
need a separate private enable-flag.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 08/17] OMAPDSS: DSI: Maintain own copy of timings in driver data
From: Tomi Valkeinen @ 2012-08-08 7:10 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <5022073B.6070109@ti.com>
[-- Attachment #1: Type: text/plain, Size: 743 bytes --]
On Wed, 2012-08-08 at 11:59 +0530, Archit Taneja wrote:
> I tried out rotation on Taal, and it only works for 180 degrees(and 0 of
> course), 90 and 270 result in no output. I'll add a
> dss_mgr_set_timings() in omap_dsi_update, that should sort of fix it,
> but someone would need to reconfigure the connected overlays too before
> trying out an update.
Right, but that's something omapdss/panel cannot do, it must be done by
the user. The same problem is there with changing, say, DPI mode also.
Btw, can you separate smaller cleanups/fixes to another patch series, to
make this series even slightly smaller? I think at least the first
patches in this series are quite separate, and the rotation fix is also.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Archit Taneja @ 2012-08-08 6:59 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344407158.17575.21.camel@lappyti>
On Wednesday 08 August 2012 11:55 AM, Tomi Valkeinen wrote:
> On Wed, 2012-08-08 at 11:35 +0530, Archit Taneja wrote:
>> On Tuesday 07 August 2012 08:02 PM, Tomi Valkeinen wrote:
>>> On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
>>>> This series tries to make interface drivers less dependent on omap_dss_device
>>>> which represents a panel/device connected to that interface. The current way of
>>>> configuring an interface is to populate the panel's omap_dss_device instance
>>>> with parameters common to the panel and the interface, they are either populated
>>>> in the board file, or in the panel driver. Panel timings, number of lanes
>>>> connected to interface, and pixel format are examples of such parameters, these
>>>> are then extracted by the interface driver to configure itself.
>>>
>>> The series looks good. I had only a few comments to make, but obviously
>>> this needs quite a bit of testing. I'll try it out.
>>
>> One thing I'm not sure about is whether these new functions should be
>> aware of the state of the output. For example, if we call set_timings()
>> with DSI video mode which is already enabled, the timings won't really
>> take any impact.
>>
>> Similar issues would occur when we try to make other ops like
>> set_data_lines() or set_pixel_format(). These need to be called before
>> the output is enabled. I was wondering if we would need to add
>> intelligence here to make panel drivers less likely to make mistakes.
>
> Hmm, true. It'd be nice if the functions returned -EBUSY if the
> operation cannot be done while the output is enabled.
>
> We have the dssdev->state, but we should get rid of that (or leave it to
> panel drivers). It'd be good if the output drivers know whether the
> output is enabled or not. I think this data is already tracked by
> apply.c. It's about ovl managers, but I think that's practically the
> same as output.
>
> Calling dss_mgr_enable() will set mp->enabled = true, which could be
> returned via dss_mgr_is_enabled() or such.
>
> Then again, it wouldn't be many lines of codes to track the enable-state
> in each output driver. So if we have any suspicions that mp->enabled
> doesn't quite work for, say, dsi, we could just add a private "enabled"
> member to dsi. But I don't right away see why dss_mgr_is_enabled()
> wouldn't work.
>
I think we had discussed previously that it may not the best idea to see
if a manager is enabled via mp->enabled as it's always possible that it
changes afterwards. Same for any other parameter in APPLY's private
data. This was the reason why we passed privtate data to DISPC functions
rather than creating apply helper functions which return the value of a
private data. For example, we pass manager timings to dispc_ovl_setup(),
instead of DISPC using a function like dss_mgr_get_timings().
I also don't see why dss_mgr_is_enabled() wouldn't work. The only places
where the manager's state will change are the output's enable and
disable ops. The mutex maintained by the output would ensure
sequential-ity between the output's enable() and set_timings() op, and
hence ensure the manager's state we see is fine.
If we manage the 'enabled' state for each output interface, we would be
a bit more consistent with respect to other parameters. For example,
timings is maintained by both manager and the output. Also, if we need
to separate out manager configurations from outputs in the future, it
would probably be better for the output to query it's own state rather
than depending on the manager, which could be configured either earlier
or later.
Archit
^ permalink raw reply
* Re: [RFC 08/17] OMAPDSS: DSI: Maintain own copy of timings in driver data
From: Archit Taneja @ 2012-08-08 6:41 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344406508.17575.12.camel@lappyti>
On Wednesday 08 August 2012 11:45 AM, Tomi Valkeinen wrote:
> On Wed, 2012-08-08 at 11:27 +0530, Archit Taneja wrote:
>
>> I am a bit unclear about resolution when it comes to command mode panels.
>
> Right, it's a bit confusing. And I'm not 100% sure how to manage the
> rotation.
>
>> For command mode panels, we can perform rotation at the panel side. That
>> is, the panel refreshes itself by fetching pixels from it's buffer in a
>> rotated way. Is that right?
>
> Yes. Well, actually I think the panel stores the pixels in rotated
> manner when it receives them from OMAP, but it's practically the same.
>
> One thing to realize is that this kind of rotation is a bit limited:
> because there's only one buffer, OMAP will write pixels to the buffers
> at the same time as the panel shows them. When rotating, this leads to
> tearing.
>
> If the panel has double buffer, that solves the problem, but I haven't
> seen such panels. Another option is to update the panel in two parts,
> like N9 does, but that's timing sensitive and a bit tricky.
>
>> If the original resolution is 864x480, and we set rotation at panel side
>> to make the rotation 480x864, the DISPC manager size should also be
>> configured at 480x864 right?
>
> Yep. When we use the panel rotation, from OMAP's point of view the panel
> resolution has changed.
>
>> We seem to be setting the manager timings only once when DSI is enabled.
>> After that, setting rotation doesn't impact manager size.
>
> Hmm, previously the mgr size was set before each update. I wonder if
> that code has been dropped, probably because we removed the support for
> partial updates at one point. Without partial updates, the size stays
> the same, except obviously with rotation. I think I just forgot about
> rotation at that time.
I tried out rotation on Taal, and it only works for 180 degrees(and 0 of
course), 90 and 270 result in no output. I'll add a
dss_mgr_set_timings() in omap_dsi_update, that should sort of fix it,
but someone would need to reconfigure the connected overlays too before
trying out an update.
>
>> I am asking this to understand if we need to keep resolution as a
>> separate parameter than timings. That is, timings represents the initial
>> width and height of the panel, and resolution represents the current
>> width and height of the panel.
>
> I'm not sure. I think that OMAP doesn't really need to know about the
> initial resolution. It doesn't really matter from OMAP's point of view.
>
> I think I originally kept timings and resolution separately, and the
> idea was that timings represent the panel's timings, i.e. how it updates
> the screen from its own memory. And resolution represents the usable
> resolution, from OMAP's point of view.
>
> While I haven't seen such a cmd mode panel, there could be a command
> sent to the panel to configure its timings. For this we need real
> timings, not the rotated resolution.
>
> However, even in that case the DISPC doesn't need to know about those
> timings, they would be handled by the panel driver (which could,
> perhaps, reconfigure the DSI bus speed to match the new timings). So I
> think that inside omapdss, we don't need separate timings and resolution
> for DSI cmd mode panels.
Ok.
Archit
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Tomi Valkeinen @ 2012-08-08 6:25 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <502201B7.9030205@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2279 bytes --]
On Wed, 2012-08-08 at 11:35 +0530, Archit Taneja wrote:
> On Tuesday 07 August 2012 08:02 PM, Tomi Valkeinen wrote:
> > On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
> >> This series tries to make interface drivers less dependent on omap_dss_device
> >> which represents a panel/device connected to that interface. The current way of
> >> configuring an interface is to populate the panel's omap_dss_device instance
> >> with parameters common to the panel and the interface, they are either populated
> >> in the board file, or in the panel driver. Panel timings, number of lanes
> >> connected to interface, and pixel format are examples of such parameters, these
> >> are then extracted by the interface driver to configure itself.
> >
> > The series looks good. I had only a few comments to make, but obviously
> > this needs quite a bit of testing. I'll try it out.
>
> One thing I'm not sure about is whether these new functions should be
> aware of the state of the output. For example, if we call set_timings()
> with DSI video mode which is already enabled, the timings won't really
> take any impact.
>
> Similar issues would occur when we try to make other ops like
> set_data_lines() or set_pixel_format(). These need to be called before
> the output is enabled. I was wondering if we would need to add
> intelligence here to make panel drivers less likely to make mistakes.
Hmm, true. It'd be nice if the functions returned -EBUSY if the
operation cannot be done while the output is enabled.
We have the dssdev->state, but we should get rid of that (or leave it to
panel drivers). It'd be good if the output drivers know whether the
output is enabled or not. I think this data is already tracked by
apply.c. It's about ovl managers, but I think that's practically the
same as output.
Calling dss_mgr_enable() will set mp->enabled = true, which could be
returned via dss_mgr_is_enabled() or such.
Then again, it wouldn't be many lines of codes to track the enable-state
in each output driver. So if we have any suspicions that mp->enabled
doesn't quite work for, say, dsi, we could just add a private "enabled"
member to dsi. But I don't right away see why dss_mgr_is_enabled()
wouldn't work.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* RE: [PATCH V4 0/2] arm: samsung: Move FIMD headers to include/video/
From: Kukjin Kim @ 2012-08-08 6:20 UTC (permalink / raw)
To: 'Leela Krishna Amudala', linux-arm-kernel,
linux-samsung-soc
Cc: dri-devel, linux-fbdev, ben-linux, m.szyprowski,
'Florian Tobias Schandinat'
In-Reply-To: <1344342872-7904-1-git-send-email-l.krishna@samsung.com>
Leela Krishna Amudala wrote:
>
> This patchset moves the contents of regs-fb-v4.h and regs-fb.h from arch
> side
> to include/video/samsung_fimd.h
>
> This patchset is created and rebased against master branch of torvalds
> tree.
> Tested on smdk5250 board, build tested for other boards.
>
> Changes from version 3:
> - Squashed 1st, 3rd, 4th and 5th patches from version 3
>
> Changes from version 2:
> - Changed the EXYNOS5_xxx macro name to FIMD_V8_xxx
> - Moved the header inclusion under same kind
>
> Changes from version 1:
> - Split the patches as per Sylwester comments
> - Changed FIMD_V8_xxx macro to EXYNOS5_xxx
>
> Leela Krishna Amudala (2):
> include/video: move fimd register headers from platform to
> include/video
> include/video: Add register offsets for FIMD version 8
>
> arch/arm/mach-exynos/mach-nuri.c | 2 +-
> arch/arm/mach-exynos/mach-origen.c | 2 +-
> arch/arm/mach-exynos/mach-smdk4x12.c | 2 +-
> arch/arm/mach-exynos/mach-smdkv310.c | 2 +-
> arch/arm/mach-exynos/mach-universal_c210.c | 2 +-
> arch/arm/mach-exynos/setup-fimd0.c | 2 +-
> arch/arm/mach-s3c24xx/mach-smdk2416.c | 2 +-
> arch/arm/mach-s3c64xx/mach-anw6410.c | 2 +-
> arch/arm/mach-s3c64xx/mach-crag6410.c | 2 +-
> arch/arm/mach-s3c64xx/mach-hmt.c | 2 +-
> arch/arm/mach-s3c64xx/mach-mini6410.c | 2 +-
> arch/arm/mach-s3c64xx/mach-ncp.c | 2 +-
> arch/arm/mach-s3c64xx/mach-real6410.c | 2 +-
> arch/arm/mach-s3c64xx/mach-smartq5.c | 2 +-
> arch/arm/mach-s3c64xx/mach-smartq7.c | 2 +-
> arch/arm/mach-s3c64xx/mach-smdk6410.c | 2 +-
> arch/arm/mach-s5p64x0/mach-smdk6440.c | 2 +-
> arch/arm/mach-s5p64x0/mach-smdk6450.c | 2 +-
> arch/arm/mach-s5pc100/mach-smdkc100.c | 2 +-
> arch/arm/mach-s5pv210/mach-aquila.c | 2 +-
> arch/arm/mach-s5pv210/mach-goni.c | 2 +-
> arch/arm/mach-s5pv210/mach-smdkv210.c | 2 +-
> arch/arm/plat-samsung/include/plat/regs-fb-v4.h | 159
----------------
> ----
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 +-
> drivers/video/s3c-fb.c | 2 +-
> .../plat/regs-fb.h => include/video/samsung_fimd.h | 152
> +++++++++++++++++--
> 26 files changed, 165 insertions(+), 194 deletions(-)
> delete mode 100644 arch/arm/plat-samsung/include/plat/regs-fb-v4.h
> rename arch/arm/plat-samsung/include/plat/regs-fb.h =>
> include/video/samsung_fimd.h (73%)
Looks OK to me. Will apply into topic branch, 'v3.7-for-florian' for
Florian's pulling.
If there is no objection for a couple of days, I will merge this into
for-next and send to Florian.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Archit Taneja @ 2012-08-08 6:17 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344349948.7216.82.camel@lappyti>
On Tuesday 07 August 2012 08:02 PM, Tomi Valkeinen wrote:
> On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
>> This series tries to make interface drivers less dependent on omap_dss_device
>> which represents a panel/device connected to that interface. The current way of
>> configuring an interface is to populate the panel's omap_dss_device instance
>> with parameters common to the panel and the interface, they are either populated
>> in the board file, or in the panel driver. Panel timings, number of lanes
>> connected to interface, and pixel format are examples of such parameters, these
>> are then extracted by the interface driver to configure itself.
>
> The series looks good. I had only a few comments to make, but obviously
> this needs quite a bit of testing. I'll try it out.
One thing I'm not sure about is whether these new functions should be
aware of the state of the output. For example, if we call set_timings()
with DSI video mode which is already enabled, the timings won't really
take any impact.
Similar issues would occur when we try to make other ops like
set_data_lines() or set_pixel_format(). These need to be called before
the output is enabled. I was wondering if we would need to add
intelligence here to make panel drivers less likely to make mistakes.
Archit
^ permalink raw reply
* Re: [RFC 08/17] OMAPDSS: DSI: Maintain own copy of timings in driver data
From: Tomi Valkeinen @ 2012-08-08 6:15 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <5021FFDC.6080006@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2871 bytes --]
On Wed, 2012-08-08 at 11:27 +0530, Archit Taneja wrote:
> I am a bit unclear about resolution when it comes to command mode panels.
Right, it's a bit confusing. And I'm not 100% sure how to manage the
rotation.
> For command mode panels, we can perform rotation at the panel side. That
> is, the panel refreshes itself by fetching pixels from it's buffer in a
> rotated way. Is that right?
Yes. Well, actually I think the panel stores the pixels in rotated
manner when it receives them from OMAP, but it's practically the same.
One thing to realize is that this kind of rotation is a bit limited:
because there's only one buffer, OMAP will write pixels to the buffers
at the same time as the panel shows them. When rotating, this leads to
tearing.
If the panel has double buffer, that solves the problem, but I haven't
seen such panels. Another option is to update the panel in two parts,
like N9 does, but that's timing sensitive and a bit tricky.
> If the original resolution is 864x480, and we set rotation at panel side
> to make the rotation 480x864, the DISPC manager size should also be
> configured at 480x864 right?
Yep. When we use the panel rotation, from OMAP's point of view the panel
resolution has changed.
> We seem to be setting the manager timings only once when DSI is enabled.
> After that, setting rotation doesn't impact manager size.
Hmm, previously the mgr size was set before each update. I wonder if
that code has been dropped, probably because we removed the support for
partial updates at one point. Without partial updates, the size stays
the same, except obviously with rotation. I think I just forgot about
rotation at that time.
> I am asking this to understand if we need to keep resolution as a
> separate parameter than timings. That is, timings represents the initial
> width and height of the panel, and resolution represents the current
> width and height of the panel.
I'm not sure. I think that OMAP doesn't really need to know about the
initial resolution. It doesn't really matter from OMAP's point of view.
I think I originally kept timings and resolution separately, and the
idea was that timings represent the panel's timings, i.e. how it updates
the screen from its own memory. And resolution represents the usable
resolution, from OMAP's point of view.
While I haven't seen such a cmd mode panel, there could be a command
sent to the panel to configure its timings. For this we need real
timings, not the rotated resolution.
However, even in that case the DISPC doesn't need to know about those
timings, they would be handled by the panel driver (which could,
perhaps, reconfigure the DSI bus speed to match the new timings). So I
think that inside omapdss, we don't need separate timings and resolution
for DSI cmd mode panels.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 13/17] OMAPDSS: SDI: Create a function to set timings
From: Archit Taneja @ 2012-08-08 6:10 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344349218.7216.80.camel@lappyti>
On Tuesday 07 August 2012 07:50 PM, Tomi Valkeinen wrote:
> On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
>> Create function omapdss_sdi_set_timings(), this can be used by a SDI panel
>> driver without disabling/enabling the SDI interface. This is similar to the
>> set_timings op of the DPI interface driver. It calls sdi_set_mode() which only
>> configures the DISPC timings and DSS/DISPC clock dividers.
>
> I don't think this works, as the SDI PLL uses pclk-free, and if pclk
> changes, PLL lock probably breaks.
>
> OMAP3430 TRM explains the sequence how to configure settings on the fly,
> but that's not very simple. Just turning the output off and on is much
> easier.
Right, I'll make set_timings() just disable and enable SDI like before.
Archit
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox