* Re: [PATCH linux-next] mfd: max8925: max8925_backlight_probe: Silence 'statement with no effect' war
From: Arnd Bergmann @ 2013-03-11 15:29 UTC (permalink / raw)
To: Tim Gardner
Cc: linux-kernel, Richard Purdie, Florian Tobias Schandinat,
linux-fbdev
In-Reply-To: <1362939145-88329-1-git-send-email-tim.gardner@canonical.com>
On Sunday 10 March 2013, Tim Gardner wrote:
> drivers/video/backlight/max8925_bl.c: In function 'max8925_backlight_probe':
> drivers/video/backlight/max8925_bl.c:177:3: warning: statement with no effect [-Wunused-value]
>
> gcc version 4.6.3
>
> Convert max8925_backlight_dt_init() to an 'inline void' since it is only
> called from one place where the return code is ignored. Protect the
> guts of the function with '#ifdef CONFIG_OF'.
>
> Cc: Richard Purdie <rpurdie@rpsys.net>
> Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Cc: linux-fbdev@vger.kernel.org
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
> drivers/video/backlight/max8925_bl.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
I had already sent a better patch for this earlier, see "mfd: max8925:
fix trivial build warning for non-dt". Unfortunately when I looked into
the problem again, I found more problems with the original patches,
apparently the DT bindings were never properly reviewed. It may be
better to revert the original patch for 3.9 and do it better for 3.10.
Arnd
^ permalink raw reply
* Re: [PATCH v2 2/2] video: imxfb: Add DT support
From: Markus Pargmann @ 2013-03-11 19:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130311102540.GB23082@e106331-lin.cambridge.arm.com>
Hi,
On Mon, Mar 11, 2013 at 10:25:40AM +0000, Mark Rutland wrote:
> Hi,
>
> Any reason for not CCing devicetree-discuss?
There is no reason, sorry, I forgot CC, I will add it to CC for the next
version.
>
> I have a couple of comments on the binding and the way it's parsed.
>
> On Tue, Mar 05, 2013 at 05:30:08PM +0000, Markus Pargmann wrote:
> > Add devicetree support for imx framebuffer driver. It uses the generic
> > display bindings and helper functions.
> >
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > Cc: Fabio Estevam <festevam@gmail.com>
> > ---
> >
> > Notes:
> > Changes in v2:
> > - Removed pwmr register property
> > - Cleanup of devicetree binding documentation
> > - Use default values for pwmr and lscr1
> >
> > .../devicetree/bindings/video/fsl,imx-fb.txt | 49 ++++++
> > drivers/video/imxfb.c | 182 +++++++++++++++++----
> > 2 files changed, 197 insertions(+), 34 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> >
> > diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > new file mode 100644
> > index 0000000..e1a53a3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > @@ -0,0 +1,49 @@
> > +Freescale imx21 Framebuffer
> > +
> > +This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
> > +
> > +Required properties:
> > +- compatible : "fsl,<chip>-fb", chip should be imx1 or imx21
> > +- reg : Should contain 1 register ranges(address and length)
> > +- interrupts : One interrupt of the fb dev
> > +
> > +Required nodes:
> > +- display: Phandle to a display node as described in
> > + Documentation/devicetree/bindings/video/display-timing.txt
> > + Additional, the display node has to define properties:
> > + - bpp: Bits per pixel
> > + - pcr: LCDC PCR value
>
> As these are non-standard, it would be good to prefix them (e.g. "fsl,pcr").
Okay.
> If you need them, why are they not a good fit for the generic binding?
I think bpp could be used by some other drivers but not of the majority.
There are actually already some of them having bindings for bpp, e.g.
Documentation/devicetree/bindings/video/via,vt8500-fb.txt .
>
> I'm not familiar with the hardware, what is the PCR exactly?
PCR is an integer that encodes a lot of bools to specify the behavior of
the imxfb-lcd interaction. The alternative would be a lot of optional
bool properties which are parsed in the driver to construct it that way.
>
> [...]
>
> > -static int __init imxfb_probe(struct platform_device *pdev)
> > +static int imxfb_of_read_mode(struct device_node *np,
> > + struct imx_fb_videomode *imxfb_mode)
> > +{
> > + int ret;
> > + struct fb_videomode *of_mode = &imxfb_mode->mode;
> > + u32 bpp;
> > + u32 pcr;
> > +
> > + ret = of_property_read_string(np, "model", &of_mode->name);
> > + if (ret)
> > + of_mode->name = NULL;
> > +
> > + ret = of_get_fb_videomode(np, of_mode, OF_USE_NATIVE_MODE);
> > + if (ret)
> > + return ret;
> > +
> > + ret = of_property_read_u32(np, "bpp", &bpp);
> > + ret |= of_property_read_u32(np, "pcr", &pcr);
> > +
> > + if (ret)
> > + return ret;
>
> Is this return value used anywhere in anything more than an "if (!err)"
> capacity? If so it may be worth having individual return value checks:
>
> If one call returns -EINVAL (-22) and the other -ENODATA (-61), out the other
> end we'd get -EISDIR (-21). If we don't care particularly about which error
> code we actually pass on, we could always return a sensible code when ret is
> nonzero:
>
> if (ret)
> return -EINVAL;
Yes, the error codes are directly passed through the probe function,
so I will change it to return -EINVAL and print an device error message.
>
> > +
> > + if (bpp > 255)
> > + return -EINVAL;
>
> Might it also be worth checking for 0 here?
Yes, changed.
>
> [...]
>
> > @@ -837,15 +914,51 @@ static int __init imxfb_probe(struct platform_device *pdev)
> >
> > fbi = info->par;
> >
> > - if (!fb_mode)
> > - fb_mode = pdata->mode[0].mode.name;
> > -
> > platform_set_drvdata(pdev, info);
> >
> > ret = imxfb_init_fbinfo(pdev);
> > if (ret < 0)
> > goto failed_init;
> >
> > + if (pdata) {
> > + if (!fb_mode)
> > + fb_mode = pdata->mode[0].mode.name;
> > +
> > + fbi->mode = pdata->mode;
> > + fbi->num_modes = pdata->num_modes;
> > + } else {
> > + struct device_node *display_np;
> > + fb_mode = NULL;
> > +
> > + display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
> > + if (!display_np) {
> > + dev_err(&pdev->dev, "No display defined in devicetree\n");
> > + ret = -EINVAL;
> > + goto failed_of_parse;
> > + }
> > +
> > + /*
> > + * imxfb does not support more modes, we choose only the native
> > + * mode.
> > + */
> > + fbi->num_modes = 1;
> > +
> > + fbi->mode = devm_kzalloc(&pdev->dev,
> > + sizeof(struct imx_fb_videomode), GFP_KERNEL);
> > + if (!fbi->mode) {
> > + ret = -ENOMEM;
> > + goto failed_of_parse;
> > + }
> > +
> > + ret = imxfb_of_read_mode(display_np, fbi->mode);
> > + if (ret)
> > + goto failed_of_parse;
> > + }
> > +
> > + for (i = 0, m = &fbi->mode[0]; i < fbi->num_modes; i++, m++)
> > + info->fix.smem_len = max_t(size_t, info->fix.smem_len,
> > + m->mode.xres * m->mode.yres * m->bpp / 8);
>
> Surely this is broken if bpp is not as multiple of 8?
>
> If we can only handle multiples of 8, could we not sanity check this earlier?
>
> If there's no strong preference for describing it in bits, could we not
> describe it in bytes and side-step the issue?
I think it is more common using bits per pixel. Indeed the for loop
seems to be broken. I fixed it by calculating the maximum bytes used per
pixel before. A grep through the kernel shows that there seem to be some
displays using bpp that are not a multiple of 8.
Thanks for your comments,
Markus
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH] backlight: fix typo "MACH_SAM9...EK" three times
From: Paul Bolle @ 2013-03-11 20:47 UTC (permalink / raw)
To: Richard Purdie, Florian Tobias Schandinat; +Cc: linux-fbdev, linux-kernel
Fix three typos (originally) introduced by commit
a9a84c37d1ee50db8f3752b117caf2b48dcd4f8a ("atmel_lcdfb: backlight
control").
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
0) Tested by grepping the tree.
1) So two of these typos were introduced in v2.6.25. (The third was
introduced in commit 915190f7d4f08e413e5fde6b0abcd5375aeacdf4 ("[ARM]
5614/1: at91: atmel_lcdfb: add at91sam9g10 support to atmel LCD
driver")). Checking these commits reveals that the default value of 'y'
has never been set automatically in all releases since v2.6.25! Perhaps
this line might as well be dropped.
drivers/video/backlight/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index db10d01..a4481df 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -161,7 +161,7 @@ if BACKLIGHT_CLASS_DEVICE
config BACKLIGHT_ATMEL_LCDC
bool "Atmel LCDC Contrast-as-Backlight control"
depends on FB_ATMEL
- default y if MACH_SAM9261EK || MACH_SAM9G10EK || MACH_SAM9263EK
+ default y if MACH_AT91SAM9261EK || MACH_AT91SAM9G10EK || MACH_AT91SAM9263EK
help
This provides a backlight control internal to the Atmel LCDC
driver. If the LCD "contrast control" on your board is wired
--
1.7.11.7
^ permalink raw reply related
* [PATCH v2] video: backlight: adp5520: fix compiler warning in adp5520_show
From: Devendra Naga @ 2013-03-11 23:27 UTC (permalink / raw)
To: linux-fbdev
while compiling with make W=1 (gcc gcc (GCC) 4.7.2 20121109 (Red Hat 4.7.2-8))
found the following warning
drivers/video/backlight/adp5520_bl.c: In function ‘adp5520_show’:
drivers/video/backlight/adp5520_bl.c:146:6: warning: variable ‘error’ set but not used [-Wunused-but-set-variable]
fixed by checking the return value of the variable
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Michael Hennerich <michael.hennerich@analog.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
---
drivers/video/backlight/adp5520_bl.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/adp5520_bl.c b/drivers/video/backlight/adp5520_bl.c
index a1e41d4..d923f23 100644
--- a/drivers/video/backlight/adp5520_bl.c
+++ b/drivers/video/backlight/adp5520_bl.c
@@ -143,13 +143,16 @@ static int adp5520_bl_setup(struct backlight_device *bl)
static ssize_t adp5520_show(struct device *dev, char *buf, int reg)
{
struct adp5520_bl *data = dev_get_drvdata(dev);
- int error;
+ int ret;
uint8_t reg_val;
mutex_lock(&data->lock);
- error = adp5520_read(data->master, reg, ®_val);
+ ret = adp5520_read(data->master, reg, ®_val);
mutex_unlock(&data->lock);
+ if (ret < 0)
+ return ret;
+
return sprintf(buf, "%u\n", reg_val);
}
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH] backlight: fix typo "MACH_SAM9...EK" three times
From: Jingoo Han @ 2013-03-11 23:50 UTC (permalink / raw)
To: 'Paul Bolle', 'Andrew Morton'
Cc: 'Richard Purdie', 'Florian Tobias Schandinat',
linux-fbdev, linux-kernel
In-Reply-To: <1363034823.3137.110.camel@x61.thuisdomein>
On Tuesday, March 12, 2013 5:47 AM, Paul Bolle wrote:
>
> Fix three typos (originally) introduced by commit
> a9a84c37d1ee50db8f3752b117caf2b48dcd4f8a ("atmel_lcdfb: backlight
> control").
>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
CC'ed Andrew Morton.
It looks good.
Acked-by: Jingoo Han <jg1.han@samsung.com>
Best regards,
Jingoo Han
> ---
> 0) Tested by grepping the tree.
>
> 1) So two of these typos were introduced in v2.6.25. (The third was
> introduced in commit 915190f7d4f08e413e5fde6b0abcd5375aeacdf4 ("[ARM]
> 5614/1: at91: atmel_lcdfb: add at91sam9g10 support to atmel LCD
> driver")). Checking these commits reveals that the default value of 'y'
> has never been set automatically in all releases since v2.6.25! Perhaps
> this line might as well be dropped.
>
> drivers/video/backlight/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index db10d01..a4481df 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -161,7 +161,7 @@ if BACKLIGHT_CLASS_DEVICE
> config BACKLIGHT_ATMEL_LCDC
> bool "Atmel LCDC Contrast-as-Backlight control"
> depends on FB_ATMEL
> - default y if MACH_SAM9261EK || MACH_SAM9G10EK || MACH_SAM9263EK
> + default y if MACH_AT91SAM9261EK || MACH_AT91SAM9G10EK || MACH_AT91SAM9263EK
> help
> This provides a backlight control internal to the Atmel LCDC
> driver. If the LCD "contrast control" on your board is wired
> --
> 1.7.11.7
>
^ permalink raw reply
* Re: [PATCH v2] video: backlight: adp5520: fix compiler warning in adp5520_show
From: Jingoo Han @ 2013-03-11 23:52 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1363044454-25111-1-git-send-email-devendra.aaru@gmail.com>
On Tuesday, March 12, 2013 8:28 AM, Devendra Naga wrote:
>
> while compiling with make W=1 (gcc gcc (GCC) 4.7.2 20121109 (Red Hat 4.7.2-8))
> found the following warning
>
> drivers/video/backlight/adp5520_bl.c: In function ‘adp5520_show’:
> drivers/video/backlight/adp5520_bl.c:146:6: warning: variable ‘error’ set but not used [-Wunused-but-
> set-variable]
>
> fixed by checking the return value of the variable
>
> Cc: Jingoo Han <jg1.han@samsung.com>
Acked-by: Jingoo Han <jg1.han@samsung.com>
Best regards,
Jingoo Han
> Cc: Michael Hennerich <michael.hennerich@analog.com>
> Cc: Richard Purdie <rpurdie@rpsys.net>
> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
> ---
> drivers/video/backlight/adp5520_bl.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/backlight/adp5520_bl.c b/drivers/video/backlight/adp5520_bl.c
> index a1e41d4..d923f23 100644
> --- a/drivers/video/backlight/adp5520_bl.c
> +++ b/drivers/video/backlight/adp5520_bl.c
> @@ -143,13 +143,16 @@ static int adp5520_bl_setup(struct backlight_device *bl)
> static ssize_t adp5520_show(struct device *dev, char *buf, int reg)
> {
> struct adp5520_bl *data = dev_get_drvdata(dev);
> - int error;
> + int ret;
> uint8_t reg_val;
>
> mutex_lock(&data->lock);
> - error = adp5520_read(data->master, reg, ®_val);
> + ret = adp5520_read(data->master, reg, ®_val);
> mutex_unlock(&data->lock);
>
> + if (ret < 0)
> + return ret;
> +
> return sprintf(buf, "%u\n", reg_val);
> }
>
> --
> 1.8.1.2
^ permalink raw reply
* [PATCH 1/5] videomode: simplify videomode Kconfig and Makefile
From: Tomi Valkeinen @ 2013-03-12 10:19 UTC (permalink / raw)
To: Steffen Trumtrar, linux-fbdev, dri-devel, Laurent Pinchart; +Cc: Tomi Valkeinen
This patch simplifies videomode related Kconfig and Makefile. After this
patch, there's only one non-user selectable Kconfig option left,
VIDEOMODE_HELPERS. The reasons for the change:
* Videomode helper functions are not something that should be shown in
the kernel configuration options. The related code should just be
included if it's needed, i.e. selected by drivers using videomode.
* There's no need to have separate Kconfig options for videomode and
display_timing. First of all, the amount of code for both is quite
small. Second, videomode depends on display_timing, and display_timing
in itself is not really useful, so both would be included in any case.
* CONFIG_VIDEOMODE is a bit vague name, and CONFIG_VIDEOMODE_HELPERS
describes better what's included.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
drivers/gpu/drm/drm_modes.c | 8 ++++----
drivers/gpu/drm/tilcdc/Kconfig | 3 +--
drivers/video/Kconfig | 22 ++--------------------
drivers/video/Makefile | 8 ++++----
drivers/video/fbmon.c | 8 ++++----
5 files changed, 15 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 04fa6f1..0698c0e 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -506,7 +506,7 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh,
}
EXPORT_SYMBOL(drm_gtf_mode);
-#if IS_ENABLED(CONFIG_VIDEOMODE)
+#ifdef CONFIG_VIDEOMODE_HELPERS
int drm_display_mode_from_videomode(const struct videomode *vm,
struct drm_display_mode *dmode)
{
@@ -540,9 +540,8 @@ int drm_display_mode_from_videomode(const struct videomode *vm,
return 0;
}
EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
-#endif
-#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+#ifdef CONFIG_OF
/**
* of_get_drm_display_mode - get a drm_display_mode from devicetree
* @np: device_node with the timing specification
@@ -572,7 +571,8 @@ int of_get_drm_display_mode(struct device_node *np,
return 0;
}
EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
-#endif
+#endif /* CONFIG_OF */
+#endif /* CONFIG_VIDEOMODE_HELPERS */
/**
* drm_mode_set_name - set the name on a mode
diff --git a/drivers/gpu/drm/tilcdc/Kconfig b/drivers/gpu/drm/tilcdc/Kconfig
index d24d040..e461e99 100644
--- a/drivers/gpu/drm/tilcdc/Kconfig
+++ b/drivers/gpu/drm/tilcdc/Kconfig
@@ -4,8 +4,7 @@ config DRM_TILCDC
select DRM_KMS_HELPER
select DRM_KMS_CMA_HELPER
select DRM_GEM_CMA_HELPER
- select OF_VIDEOMODE
- select OF_DISPLAY_TIMING
+ select VIDEOMODE_HELPERS
select BACKLIGHT_CLASS_DEVICE
help
Choose this option if you have an TI SoC with LCDC display
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 4c1546f..2a81b11 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -31,26 +31,8 @@ config VIDEO_OUTPUT_CONTROL
This framework adds support for low-level control of the video
output switch.
-config DISPLAY_TIMING
- bool
-
-config VIDEOMODE
- bool
-
-config OF_DISPLAY_TIMING
- bool "Enable device tree display timing support"
- depends on OF
- select DISPLAY_TIMING
- help
- helper to parse display timings from the devicetree
-
-config OF_VIDEOMODE
- bool "Enable device tree videomode support"
- depends on OF
- select VIDEOMODE
- select OF_DISPLAY_TIMING
- help
- helper to get videomodes from the devicetree
+config VIDEOMODE_HELPERS
+ bool
config HDMI
bool
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 9df3873..e414378 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -171,7 +171,7 @@ obj-$(CONFIG_FB_VIRTUAL) += vfb.o
#video output switch sysfs driver
obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
-obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
-obj-$(CONFIG_OF_DISPLAY_TIMING) += of_display_timing.o
-obj-$(CONFIG_VIDEOMODE) += videomode.o
-obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
+obj-$(CONFIG_VIDEOMODE_HELPERS) += display_timing.o videomode.o
+ifeq ($(CONFIG_OF),y)
+obj-$(CONFIG_VIDEOMODE_HELPERS) += of_display_timing.o of_videomode.o
+endif
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 94ad0f7..368cedf 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -1376,7 +1376,7 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
return err;
}
-#if IS_ENABLED(CONFIG_VIDEOMODE)
+#ifdef CONFIG_VIDEOMODE_HELPERS
int fb_videomode_from_videomode(const struct videomode *vm,
struct fb_videomode *fbmode)
{
@@ -1424,9 +1424,8 @@ int fb_videomode_from_videomode(const struct videomode *vm,
return 0;
}
EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
-#endif
-#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+#ifdef CONFIG_OF
static inline void dump_fb_videomode(const struct fb_videomode *m)
{
pr_debug("fb_videomode = %ux%u@%uHz (%ukHz) %u %u %u %u %u %u %u %u %u\n",
@@ -1465,7 +1464,8 @@ int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
return 0;
}
EXPORT_SYMBOL_GPL(of_get_fb_videomode);
-#endif
+#endif /* CONFIG_OF */
+#endif /* CONFIG_VIDEOMODE_HELPERS */
#else
int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/5] videomode: combine videomode dmt_flags and data_flags
From: Tomi Valkeinen @ 2013-03-12 10:19 UTC (permalink / raw)
To: Steffen Trumtrar, linux-fbdev, dri-devel, Laurent Pinchart; +Cc: Tomi Valkeinen
In-Reply-To: <1363083578-17062-1-git-send-email-tomi.valkeinen@ti.com>
Both videomode and display_timing contain flags describing the modes.
These are stored in dmt_flags and data_flags. There's no need to
separate these flags, and having separate fields just makes the flags
more difficult to use.
This patch combines the fields and renames VESA_DMT_* flags to
DISPLAY_FLAGS_*.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
drivers/gpu/drm/drm_modes.c | 12 ++++++------
drivers/video/fbmon.c | 8 ++++----
drivers/video/of_display_timing.c | 19 +++++++++----------
drivers/video/videomode.c | 3 +--
include/video/display_timing.h | 26 +++++++++++---------------
include/video/videomode.h | 3 +--
6 files changed, 32 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 0698c0e..f83f071 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -523,17 +523,17 @@ int drm_display_mode_from_videomode(const struct videomode *vm,
dmode->clock = vm->pixelclock / 1000;
dmode->flags = 0;
- if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+ if (vm->flags & DISPLAY_FLAGS_HSYNC_HIGH)
dmode->flags |= DRM_MODE_FLAG_PHSYNC;
- else if (vm->dmt_flags & VESA_DMT_HSYNC_LOW)
+ else if (vm->flags & DISPLAY_FLAGS_HSYNC_LOW)
dmode->flags |= DRM_MODE_FLAG_NHSYNC;
- if (vm->dmt_flags & VESA_DMT_VSYNC_HIGH)
+ if (vm->flags & DISPLAY_FLAGS_VSYNC_HIGH)
dmode->flags |= DRM_MODE_FLAG_PVSYNC;
- else if (vm->dmt_flags & VESA_DMT_VSYNC_LOW)
+ else if (vm->flags & DISPLAY_FLAGS_VSYNC_LOW)
dmode->flags |= DRM_MODE_FLAG_NVSYNC;
- if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
+ if (vm->flags & DISPLAY_FLAGS_INTERLACED)
dmode->flags |= DRM_MODE_FLAG_INTERLACE;
- if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
+ if (vm->flags & DISPLAY_FLAGS_DOUBLESCAN)
dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
drm_mode_set_name(dmode);
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 368cedf..e5cc2fd 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -1398,13 +1398,13 @@ int fb_videomode_from_videomode(const struct videomode *vm,
fbmode->sync = 0;
fbmode->vmode = 0;
- if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+ if (vm->flags & DISPLAY_FLAGS_HSYNC_HIGH)
fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
- if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+ if (vm->flags & DISPLAY_FLAGS_HSYNC_HIGH)
fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
- if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
+ if (vm->flags & DISPLAY_FLAGS_INTERLACED)
fbmode->vmode |= FB_VMODE_INTERLACED;
- if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
+ if (vm->flags & DISPLAY_FLAGS_DOUBLESCAN)
fbmode->vmode |= FB_VMODE_DOUBLE;
fbmode->flag = 0;
diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
index 13ecd98..56009bc 100644
--- a/drivers/video/of_display_timing.c
+++ b/drivers/video/of_display_timing.c
@@ -79,25 +79,24 @@ static struct display_timing *of_get_display_timing(struct device_node *np)
ret |= parse_timing_property(np, "vsync-len", &dt->vsync_len);
ret |= parse_timing_property(np, "clock-frequency", &dt->pixelclock);
- dt->dmt_flags = 0;
- dt->data_flags = 0;
+ dt->flags = 0;
if (!of_property_read_u32(np, "vsync-active", &val))
- dt->dmt_flags |= val ? VESA_DMT_VSYNC_HIGH :
- VESA_DMT_VSYNC_LOW;
+ dt->flags |= val ? DISPLAY_FLAGS_VSYNC_HIGH :
+ DISPLAY_FLAGS_VSYNC_LOW;
if (!of_property_read_u32(np, "hsync-active", &val))
- dt->dmt_flags |= val ? VESA_DMT_HSYNC_HIGH :
- VESA_DMT_HSYNC_LOW;
+ dt->flags |= val ? DISPLAY_FLAGS_HSYNC_HIGH :
+ DISPLAY_FLAGS_HSYNC_LOW;
if (!of_property_read_u32(np, "de-active", &val))
- dt->data_flags |= val ? DISPLAY_FLAGS_DE_HIGH :
+ dt->flags |= val ? DISPLAY_FLAGS_DE_HIGH :
DISPLAY_FLAGS_DE_LOW;
if (!of_property_read_u32(np, "pixelclk-active", &val))
- dt->data_flags |= val ? DISPLAY_FLAGS_PIXDATA_POSEDGE :
+ dt->flags |= val ? DISPLAY_FLAGS_PIXDATA_POSEDGE :
DISPLAY_FLAGS_PIXDATA_NEGEDGE;
if (of_property_read_bool(np, "interlaced"))
- dt->data_flags |= DISPLAY_FLAGS_INTERLACED;
+ dt->flags |= DISPLAY_FLAGS_INTERLACED;
if (of_property_read_bool(np, "doublescan"))
- dt->data_flags |= DISPLAY_FLAGS_DOUBLESCAN;
+ dt->flags |= DISPLAY_FLAGS_DOUBLESCAN;
if (ret) {
pr_err("%s: error reading timing properties\n",
diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
index 21c47a2..810afff 100644
--- a/drivers/video/videomode.c
+++ b/drivers/video/videomode.c
@@ -31,8 +31,7 @@ int videomode_from_timing(const struct display_timings *disp,
vm->vback_porch = display_timing_get_value(&dt->vback_porch, TE_TYP);
vm->vsync_len = display_timing_get_value(&dt->vsync_len, TE_TYP);
- vm->dmt_flags = dt->dmt_flags;
- vm->data_flags = dt->data_flags;
+ vm->flags = dt->flags;
return 0;
}
diff --git a/include/video/display_timing.h b/include/video/display_timing.h
index 71e9a38..a8a4be5 100644
--- a/include/video/display_timing.h
+++ b/include/video/display_timing.h
@@ -12,19 +12,16 @@
#include <linux/bitops.h>
#include <linux/types.h>
-/* VESA display monitor timing parameters */
-#define VESA_DMT_HSYNC_LOW BIT(0)
-#define VESA_DMT_HSYNC_HIGH BIT(1)
-#define VESA_DMT_VSYNC_LOW BIT(2)
-#define VESA_DMT_VSYNC_HIGH BIT(3)
-
-/* display specific flags */
-#define DISPLAY_FLAGS_DE_LOW BIT(0) /* data enable flag */
-#define DISPLAY_FLAGS_DE_HIGH BIT(1)
-#define DISPLAY_FLAGS_PIXDATA_POSEDGE BIT(2) /* drive data on pos. edge */
-#define DISPLAY_FLAGS_PIXDATA_NEGEDGE BIT(3) /* drive data on neg. edge */
-#define DISPLAY_FLAGS_INTERLACED BIT(4)
-#define DISPLAY_FLAGS_DOUBLESCAN BIT(5)
+#define DISPLAY_FLAGS_HSYNC_LOW BIT(0)
+#define DISPLAY_FLAGS_HSYNC_HIGH BIT(1)
+#define DISPLAY_FLAGS_VSYNC_LOW BIT(2)
+#define DISPLAY_FLAGS_VSYNC_HIGH BIT(3)
+#define DISPLAY_FLAGS_DE_LOW BIT(4) /* data enable flag */
+#define DISPLAY_FLAGS_DE_HIGH BIT(5)
+#define DISPLAY_FLAGS_PIXDATA_POSEDGE BIT(6) /* drive data on pos. edge */
+#define DISPLAY_FLAGS_PIXDATA_NEGEDGE BIT(7) /* drive data on neg. edge */
+#define DISPLAY_FLAGS_INTERLACED BIT(8)
+#define DISPLAY_FLAGS_DOUBLESCAN BIT(9)
/*
* A single signal can be specified via a range of minimal and maximal values
@@ -72,8 +69,7 @@ struct display_timing {
struct timing_entry vback_porch; /* ver. back porch */
struct timing_entry vsync_len; /* ver. sync len */
- unsigned int dmt_flags; /* VESA DMT flags */
- unsigned int data_flags; /* video data flags */
+ unsigned int flags; /* display flags */
};
/*
diff --git a/include/video/videomode.h b/include/video/videomode.h
index a421562..f4ae6ed 100644
--- a/include/video/videomode.h
+++ b/include/video/videomode.h
@@ -29,8 +29,7 @@ struct videomode {
u32 vback_porch;
u32 vsync_len;
- unsigned int dmt_flags; /* VESA DMT flags */
- unsigned int data_flags; /* video data flags */
+ unsigned int flags; /* display flags */
};
/**
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/5] videomode: create enum for videomode's display flags
From: Tomi Valkeinen @ 2013-03-12 10:19 UTC (permalink / raw)
To: Steffen Trumtrar, linux-fbdev, dri-devel, Laurent Pinchart; +Cc: Tomi Valkeinen
In-Reply-To: <1363083578-17062-1-git-send-email-tomi.valkeinen@ti.com>
Instead of having plain defines for the videomode's flags, add an enum
for the flags. This makes the flags clearer to use, as the enum tells
which values can be used with the flags field.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
include/video/display_timing.h | 28 +++++++++++++++++-----------
include/video/videomode.h | 2 +-
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/include/video/display_timing.h b/include/video/display_timing.h
index a8a4be5..b63471d 100644
--- a/include/video/display_timing.h
+++ b/include/video/display_timing.h
@@ -12,16 +12,22 @@
#include <linux/bitops.h>
#include <linux/types.h>
-#define DISPLAY_FLAGS_HSYNC_LOW BIT(0)
-#define DISPLAY_FLAGS_HSYNC_HIGH BIT(1)
-#define DISPLAY_FLAGS_VSYNC_LOW BIT(2)
-#define DISPLAY_FLAGS_VSYNC_HIGH BIT(3)
-#define DISPLAY_FLAGS_DE_LOW BIT(4) /* data enable flag */
-#define DISPLAY_FLAGS_DE_HIGH BIT(5)
-#define DISPLAY_FLAGS_PIXDATA_POSEDGE BIT(6) /* drive data on pos. edge */
-#define DISPLAY_FLAGS_PIXDATA_NEGEDGE BIT(7) /* drive data on neg. edge */
-#define DISPLAY_FLAGS_INTERLACED BIT(8)
-#define DISPLAY_FLAGS_DOUBLESCAN BIT(9)
+enum display_flags {
+ DISPLAY_FLAGS_HSYNC_LOW = BIT(0),
+ DISPLAY_FLAGS_HSYNC_HIGH = BIT(1),
+ DISPLAY_FLAGS_VSYNC_LOW = BIT(2),
+ DISPLAY_FLAGS_VSYNC_HIGH = BIT(3),
+
+ /* data enable flag */
+ DISPLAY_FLAGS_DE_LOW = BIT(4),
+ DISPLAY_FLAGS_DE_HIGH = BIT(5),
+ /* drive data on pos. edge */
+ DISPLAY_FLAGS_PIXDATA_POSEDGE = BIT(6),
+ /* drive data on neg. edge */
+ DISPLAY_FLAGS_PIXDATA_NEGEDGE = BIT(7),
+ DISPLAY_FLAGS_INTERLACED = BIT(8),
+ DISPLAY_FLAGS_DOUBLESCAN = BIT(9),
+};
/*
* A single signal can be specified via a range of minimal and maximal values
@@ -69,7 +75,7 @@ struct display_timing {
struct timing_entry vback_porch; /* ver. back porch */
struct timing_entry vsync_len; /* ver. sync len */
- unsigned int flags; /* display flags */
+ enum display_flags flags; /* display flags */
};
/*
diff --git a/include/video/videomode.h b/include/video/videomode.h
index f4ae6ed..8b12e60 100644
--- a/include/video/videomode.h
+++ b/include/video/videomode.h
@@ -29,7 +29,7 @@ struct videomode {
u32 vback_porch;
u32 vsync_len;
- unsigned int flags; /* display flags */
+ enum display_flags flags; /* display flags */
};
/**
--
1.7.10.4
^ permalink raw reply related
* [PATCH 4/5] videomode: remove timing_entry_index
From: Tomi Valkeinen @ 2013-03-12 10:19 UTC (permalink / raw)
To: Steffen Trumtrar, linux-fbdev, dri-devel, Laurent Pinchart; +Cc: Tomi Valkeinen
In-Reply-To: <1363083578-17062-1-git-send-email-tomi.valkeinen@ti.com>
Display timing's fields have minimum, typical and maximum values. These
can be accessed by using timing_entry_index enum, and
display_timing_get_value() function.
There's no real need for this extra complexity. The values can be
accessed more easily by just using the min/typ/max fields.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
drivers/video/videomode.c | 18 +++++++++---------
include/video/display_timing.h | 25 -------------------------
2 files changed, 9 insertions(+), 34 deletions(-)
diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
index 810afff..a3d95f2 100644
--- a/drivers/video/videomode.c
+++ b/drivers/video/videomode.c
@@ -20,16 +20,16 @@ int videomode_from_timing(const struct display_timings *disp,
if (!dt)
return -EINVAL;
- vm->pixelclock = display_timing_get_value(&dt->pixelclock, TE_TYP);
- vm->hactive = display_timing_get_value(&dt->hactive, TE_TYP);
- vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, TE_TYP);
- vm->hback_porch = display_timing_get_value(&dt->hback_porch, TE_TYP);
- vm->hsync_len = display_timing_get_value(&dt->hsync_len, TE_TYP);
+ vm->pixelclock = dt->pixelclock.typ;
+ vm->hactive = dt->hactive.typ;
+ vm->hfront_porch = dt->hfront_porch.typ;
+ vm->hback_porch = dt->hback_porch.typ;
+ vm->hsync_len = dt->hsync_len.typ;
- vm->vactive = display_timing_get_value(&dt->vactive, TE_TYP);
- vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, TE_TYP);
- vm->vback_porch = display_timing_get_value(&dt->vback_porch, TE_TYP);
- vm->vsync_len = display_timing_get_value(&dt->vsync_len, TE_TYP);
+ vm->vactive = dt->vactive.typ;
+ vm->vfront_porch = dt->vfront_porch.typ;
+ vm->vback_porch = dt->vback_porch.typ;
+ vm->vsync_len = dt->vsync_len.typ;
vm->flags = dt->flags;
diff --git a/include/video/display_timing.h b/include/video/display_timing.h
index b63471d..5d0259b 100644
--- a/include/video/display_timing.h
+++ b/include/video/display_timing.h
@@ -39,12 +39,6 @@ struct timing_entry {
u32 max;
};
-enum timing_entry_index {
- TE_MIN = 0,
- TE_TYP = 1,
- TE_MAX = 2,
-};
-
/*
* Single "mode" entry. This describes one set of signal timings a display can
* have in one setting. This struct can later be converted to struct videomode
@@ -91,25 +85,6 @@ struct display_timings {
struct display_timing **timings;
};
-/* get value specified by index from struct timing_entry */
-static inline u32 display_timing_get_value(const struct timing_entry *te,
- enum timing_entry_index index)
-{
- switch (index) {
- case TE_MIN:
- return te->min;
- break;
- case TE_TYP:
- return te->typ;
- break;
- case TE_MAX:
- return te->max;
- break;
- default:
- return te->typ;
- }
-}
-
/* get one entry from struct display_timings */
static inline struct display_timing *display_timings_get(const struct
display_timings *disp,
--
1.7.10.4
^ permalink raw reply related
* [PATCH 5/5] videomode: rename fields
From: Tomi Valkeinen @ 2013-03-12 10:19 UTC (permalink / raw)
To: Steffen Trumtrar, linux-fbdev, dri-devel, Laurent Pinchart; +Cc: Tomi Valkeinen
In-Reply-To: <1363083578-17062-1-git-send-email-tomi.valkeinen@ti.com>
Structs videomode and display_timing have rather long field names for
the timing values. Nothing wrong with that as such, but this patch
changes them to abbreviations for the following reasons:
* The timing values often need to be used in calculations, and long
field names makes their direct use clumsier.
* The current names are a bit of a mishmash: some words are used as
such, some are shortened, and for some only first letter is used. Some
names use underscode, some don't. All this makes it difficult to
remember what the field names are.
* The abbreviations used in this patch are very common, and there
shouldn't be any misunderstanding about their meaning.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
drivers/gpu/drm/drm_modes.c | 18 +++++++++---------
drivers/video/fbmon.c | 24 +++++++++++-------------
drivers/video/of_display_timing.c | 16 ++++++++--------
drivers/video/videomode.c | 16 ++++++++--------
include/video/display_timing.h | 16 ++++++++--------
include/video/videomode.h | 18 +++++++++---------
6 files changed, 53 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index f83f071..d744e95 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -510,15 +510,15 @@ EXPORT_SYMBOL(drm_gtf_mode);
int drm_display_mode_from_videomode(const struct videomode *vm,
struct drm_display_mode *dmode)
{
- dmode->hdisplay = vm->hactive;
- dmode->hsync_start = dmode->hdisplay + vm->hfront_porch;
- dmode->hsync_end = dmode->hsync_start + vm->hsync_len;
- dmode->htotal = dmode->hsync_end + vm->hback_porch;
+ dmode->hdisplay = vm->hact;
+ dmode->hsync_start = dmode->hdisplay + vm->hfp;
+ dmode->hsync_end = dmode->hsync_start + vm->hsl;
+ dmode->htotal = dmode->hsync_end + vm->hbp;
- dmode->vdisplay = vm->vactive;
- dmode->vsync_start = dmode->vdisplay + vm->vfront_porch;
- dmode->vsync_end = dmode->vsync_start + vm->vsync_len;
- dmode->vtotal = dmode->vsync_end + vm->vback_porch;
+ dmode->vdisplay = vm->vact;
+ dmode->vsync_start = dmode->vdisplay + vm->vfp;
+ dmode->vsync_end = dmode->vsync_start + vm->vsl;
+ dmode->vtotal = dmode->vsync_end + vm->vbp;
dmode->clock = vm->pixelclock / 1000;
@@ -565,7 +565,7 @@ int of_get_drm_display_mode(struct device_node *np,
drm_display_mode_from_videomode(&vm, dmode);
pr_debug("%s: got %dx%d display mode from %s\n",
- of_node_full_name(np), vm.hactive, vm.vactive, np->name);
+ of_node_full_name(np), vm.hact, vm.vact, np->name);
drm_mode_debug_printmodeline(dmode);
return 0;
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index e5cc2fd..8103fc9 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -1382,15 +1382,15 @@ int fb_videomode_from_videomode(const struct videomode *vm,
{
unsigned int htotal, vtotal;
- fbmode->xres = vm->hactive;
- fbmode->left_margin = vm->hback_porch;
- fbmode->right_margin = vm->hfront_porch;
- fbmode->hsync_len = vm->hsync_len;
+ fbmode->xres = vm->hact;
+ fbmode->left_margin = vm->hbp;
+ fbmode->right_margin = vm->hfp;
+ fbmode->hsync_len = vm->hsl;
- fbmode->yres = vm->vactive;
- fbmode->upper_margin = vm->vback_porch;
- fbmode->lower_margin = vm->vfront_porch;
- fbmode->vsync_len = vm->vsync_len;
+ fbmode->yres = vm->vact;
+ fbmode->upper_margin = vm->vbp;
+ fbmode->lower_margin = vm->vfp;
+ fbmode->vsync_len = vm->vsl;
/* prevent division by zero in KHZ2PICOS macro */
fbmode->pixclock = vm->pixelclock ?
@@ -1408,10 +1408,8 @@ int fb_videomode_from_videomode(const struct videomode *vm,
fbmode->vmode |= FB_VMODE_DOUBLE;
fbmode->flag = 0;
- htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
- vm->hsync_len;
- vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
- vm->vsync_len;
+ htotal = vm->hact + vm->hfp + vm->hbp + vm->hsl;
+ vtotal = vm->vact + vm->vfp + vm->vbp + vm->vsl;
/* prevent division by zero */
if (htotal && vtotal) {
fbmode->refresh = vm->pixelclock / (htotal * vtotal);
@@ -1458,7 +1456,7 @@ int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
fb_videomode_from_videomode(&vm, fb);
pr_debug("%s: got %dx%d display mode from %s\n",
- of_node_full_name(np), vm.hactive, vm.vactive, np->name);
+ of_node_full_name(np), vm.hact, vm.vact, np->name);
dump_fb_videomode(fb);
return 0;
diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
index 56009bc..79660937 100644
--- a/drivers/video/of_display_timing.c
+++ b/drivers/video/of_display_timing.c
@@ -69,14 +69,14 @@ static struct display_timing *of_get_display_timing(struct device_node *np)
return NULL;
}
- ret |= parse_timing_property(np, "hback-porch", &dt->hback_porch);
- ret |= parse_timing_property(np, "hfront-porch", &dt->hfront_porch);
- ret |= parse_timing_property(np, "hactive", &dt->hactive);
- ret |= parse_timing_property(np, "hsync-len", &dt->hsync_len);
- ret |= parse_timing_property(np, "vback-porch", &dt->vback_porch);
- ret |= parse_timing_property(np, "vfront-porch", &dt->vfront_porch);
- ret |= parse_timing_property(np, "vactive", &dt->vactive);
- ret |= parse_timing_property(np, "vsync-len", &dt->vsync_len);
+ ret |= parse_timing_property(np, "hback-porch", &dt->hbp);
+ ret |= parse_timing_property(np, "hfront-porch", &dt->hfp);
+ ret |= parse_timing_property(np, "hactive", &dt->hact);
+ ret |= parse_timing_property(np, "hsync-len", &dt->hsl);
+ ret |= parse_timing_property(np, "vback-porch", &dt->vbp);
+ ret |= parse_timing_property(np, "vfront-porch", &dt->vfp);
+ ret |= parse_timing_property(np, "vactive", &dt->vact);
+ ret |= parse_timing_property(np, "vsync-len", &dt->vsl);
ret |= parse_timing_property(np, "clock-frequency", &dt->pixelclock);
dt->flags = 0;
diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
index a3d95f2..c42689a 100644
--- a/drivers/video/videomode.c
+++ b/drivers/video/videomode.c
@@ -21,15 +21,15 @@ int videomode_from_timing(const struct display_timings *disp,
return -EINVAL;
vm->pixelclock = dt->pixelclock.typ;
- vm->hactive = dt->hactive.typ;
- vm->hfront_porch = dt->hfront_porch.typ;
- vm->hback_porch = dt->hback_porch.typ;
- vm->hsync_len = dt->hsync_len.typ;
+ vm->hact = dt->hact.typ;
+ vm->hfp = dt->hfp.typ;
+ vm->hbp = dt->hbp.typ;
+ vm->hsl = dt->hsl.typ;
- vm->vactive = dt->vactive.typ;
- vm->vfront_porch = dt->vfront_porch.typ;
- vm->vback_porch = dt->vback_porch.typ;
- vm->vsync_len = dt->vsync_len.typ;
+ vm->vact = dt->vact.typ;
+ vm->vfp = dt->vfp.typ;
+ vm->vbp = dt->vbp.typ;
+ vm->vsl = dt->vsl.typ;
vm->flags = dt->flags;
diff --git a/include/video/display_timing.h b/include/video/display_timing.h
index 5d0259b..db98ef9 100644
--- a/include/video/display_timing.h
+++ b/include/video/display_timing.h
@@ -59,15 +59,15 @@ struct timing_entry {
struct display_timing {
struct timing_entry pixelclock;
- struct timing_entry hactive; /* hor. active video */
- struct timing_entry hfront_porch; /* hor. front porch */
- struct timing_entry hback_porch; /* hor. back porch */
- struct timing_entry hsync_len; /* hor. sync len */
+ struct timing_entry hact; /* hor. active video */
+ struct timing_entry hfp; /* hor. front porch */
+ struct timing_entry hbp; /* hor. back porch */
+ struct timing_entry hsl; /* hor. sync len */
- struct timing_entry vactive; /* ver. active video */
- struct timing_entry vfront_porch; /* ver. front porch */
- struct timing_entry vback_porch; /* ver. back porch */
- struct timing_entry vsync_len; /* ver. sync len */
+ struct timing_entry vact; /* ver. active video */
+ struct timing_entry vfp; /* ver. front porch */
+ struct timing_entry vbp; /* ver. back porch */
+ struct timing_entry vsl; /* ver. sync len */
enum display_flags flags; /* display flags */
};
diff --git a/include/video/videomode.h b/include/video/videomode.h
index 8b12e60..b601c0c 100644
--- a/include/video/videomode.h
+++ b/include/video/videomode.h
@@ -19,15 +19,15 @@
struct videomode {
unsigned long pixelclock; /* pixelclock in Hz */
- u32 hactive;
- u32 hfront_porch;
- u32 hback_porch;
- u32 hsync_len;
-
- u32 vactive;
- u32 vfront_porch;
- u32 vback_porch;
- u32 vsync_len;
+ u32 hact;
+ u32 hfp;
+ u32 hbp;
+ u32 hsl;
+
+ u32 vact;
+ u32 vfp;
+ u32 vbp;
+ u32 vsl;
enum display_flags flags; /* display flags */
};
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 1/5] videomode: simplify videomode Kconfig and Makefile
From: Laurent Pinchart @ 2013-03-12 13:34 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, dri-devel, Steffen Trumtrar
In-Reply-To: <1363083578-17062-1-git-send-email-tomi.valkeinen@ti.com>
Hi Tomi,
Thanks for the patch.
On Tuesday 12 March 2013 12:19:34 Tomi Valkeinen wrote:
> This patch simplifies videomode related Kconfig and Makefile. After this
> patch, there's only one non-user selectable Kconfig option left,
> VIDEOMODE_HELPERS. The reasons for the change:
>
> * Videomode helper functions are not something that should be shown in
> the kernel configuration options. The related code should just be
> included if it's needed, i.e. selected by drivers using videomode.
>
> * There's no need to have separate Kconfig options for videomode and
> display_timing. First of all, the amount of code for both is quite
> small. Second, videomode depends on display_timing, and display_timing
> in itself is not really useful, so both would be included in any case.
>
> * CONFIG_VIDEOMODE is a bit vague name, and CONFIG_VIDEOMODE_HELPERS
> describes better what's included.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
I like that.
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 5/5] videomode: rename fields
From: Laurent Pinchart @ 2013-03-12 13:37 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, dri-devel, Steffen Trumtrar
In-Reply-To: <1363083578-17062-5-git-send-email-tomi.valkeinen@ti.com>
Hi Tomi,
Thanks for the patch.
On Tuesday 12 March 2013 12:19:38 Tomi Valkeinen wrote:
> Structs videomode and display_timing have rather long field names for
> the timing values. Nothing wrong with that as such, but this patch
> changes them to abbreviations for the following reasons:
>
> * The timing values often need to be used in calculations, and long
> field names makes their direct use clumsier.
>
> * The current names are a bit of a mishmash: some words are used as
> such, some are shortened, and for some only first letter is used. Some
> names use underscode, some don't. All this makes it difficult to
> remember what the field names are.
>
> * The abbreviations used in this patch are very common, and there
> shouldn't be any misunderstanding about their meaning.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
I have no strong opinion on this, but I find the existing names easier to
read. I might be biased by having read them often though.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 5/5] videomode: rename fields
From: Tomi Valkeinen @ 2013-03-12 13:40 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-fbdev, dri-devel, Steffen Trumtrar
In-Reply-To: <16625567.P7GdYge43z@avalon>
[-- Attachment #1: Type: text/plain, Size: 1479 bytes --]
Hi,
On 2013-03-12 15:37, Laurent Pinchart wrote:
> Hi Tomi,
>
> Thanks for the patch.
>
> On Tuesday 12 March 2013 12:19:38 Tomi Valkeinen wrote:
>> Structs videomode and display_timing have rather long field names for
>> the timing values. Nothing wrong with that as such, but this patch
>> changes them to abbreviations for the following reasons:
>>
>> * The timing values often need to be used in calculations, and long
>> field names makes their direct use clumsier.
>>
>> * The current names are a bit of a mishmash: some words are used as
>> such, some are shortened, and for some only first letter is used. Some
>> names use underscode, some don't. All this makes it difficult to
>> remember what the field names are.
>>
>> * The abbreviations used in this patch are very common, and there
>> shouldn't be any misunderstanding about their meaning.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> ---
>
> I have no strong opinion on this, but I find the existing names easier to
> read. I might be biased by having read them often though.
Yes, the last patch was a bit of a "teaser" =). I found myself typoing
them a lot, using helper local variables to shorten the code lines, and
as I mention in the description, I find them a bit of a mishmash. So,
while they're not used in any drivers yet, I thought it'd be worth a
shot to change them.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Re: [PATCH 5/5] videomode: rename fields
From: Steffen Trumtrar @ 2013-03-12 13:53 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, dri-devel, Laurent Pinchart
In-Reply-To: <1363083578-17062-5-git-send-email-tomi.valkeinen@ti.com>
On Tue, Mar 12, 2013 at 12:19:38PM +0200, Tomi Valkeinen wrote:
> Structs videomode and display_timing have rather long field names for
> the timing values. Nothing wrong with that as such, but this patch
> changes them to abbreviations for the following reasons:
>
> * The timing values often need to be used in calculations, and long
> field names makes their direct use clumsier.
>
> * The current names are a bit of a mishmash: some words are used as
> such, some are shortened, and for some only first letter is used. Some
> names use underscode, some don't. All this makes it difficult to
> remember what the field names are.
>
> * The abbreviations used in this patch are very common, and there
> shouldn't be any misunderstanding about their meaning.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
> drivers/gpu/drm/drm_modes.c | 18 +++++++++---------
> drivers/video/fbmon.c | 24 +++++++++++-------------
> drivers/video/of_display_timing.c | 16 ++++++++--------
> drivers/video/videomode.c | 16 ++++++++--------
> include/video/display_timing.h | 16 ++++++++--------
> include/video/videomode.h | 18 +++++++++---------
> 6 files changed, 53 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index f83f071..d744e95 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -510,15 +510,15 @@ EXPORT_SYMBOL(drm_gtf_mode);
> int drm_display_mode_from_videomode(const struct videomode *vm,
> struct drm_display_mode *dmode)
> {
> - dmode->hdisplay = vm->hactive;
> - dmode->hsync_start = dmode->hdisplay + vm->hfront_porch;
> - dmode->hsync_end = dmode->hsync_start + vm->hsync_len;
> - dmode->htotal = dmode->hsync_end + vm->hback_porch;
> + dmode->hdisplay = vm->hact;
> + dmode->hsync_start = dmode->hdisplay + vm->hfp;
> + dmode->hsync_end = dmode->hsync_start + vm->hsl;
> + dmode->htotal = dmode->hsync_end + vm->hbp;
>
> - dmode->vdisplay = vm->vactive;
> - dmode->vsync_start = dmode->vdisplay + vm->vfront_porch;
> - dmode->vsync_end = dmode->vsync_start + vm->vsync_len;
> - dmode->vtotal = dmode->vsync_end + vm->vback_porch;
> + dmode->vdisplay = vm->vact;
> + dmode->vsync_start = dmode->vdisplay + vm->vfp;
> + dmode->vsync_end = dmode->vsync_start + vm->vsl;
> + dmode->vtotal = dmode->vsync_end + vm->vbp;
>
> dmode->clock = vm->pixelclock / 1000;
>
> @@ -565,7 +565,7 @@ int of_get_drm_display_mode(struct device_node *np,
> drm_display_mode_from_videomode(&vm, dmode);
>
> pr_debug("%s: got %dx%d display mode from %s\n",
> - of_node_full_name(np), vm.hactive, vm.vactive, np->name);
> + of_node_full_name(np), vm.hact, vm.vact, np->name);
> drm_mode_debug_printmodeline(dmode);
>
> return 0;
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index e5cc2fd..8103fc9 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -1382,15 +1382,15 @@ int fb_videomode_from_videomode(const struct videomode *vm,
> {
> unsigned int htotal, vtotal;
>
> - fbmode->xres = vm->hactive;
> - fbmode->left_margin = vm->hback_porch;
> - fbmode->right_margin = vm->hfront_porch;
> - fbmode->hsync_len = vm->hsync_len;
> + fbmode->xres = vm->hact;
> + fbmode->left_margin = vm->hbp;
> + fbmode->right_margin = vm->hfp;
> + fbmode->hsync_len = vm->hsl;
>
> - fbmode->yres = vm->vactive;
> - fbmode->upper_margin = vm->vback_porch;
> - fbmode->lower_margin = vm->vfront_porch;
> - fbmode->vsync_len = vm->vsync_len;
> + fbmode->yres = vm->vact;
> + fbmode->upper_margin = vm->vbp;
> + fbmode->lower_margin = vm->vfp;
> + fbmode->vsync_len = vm->vsl;
>
> /* prevent division by zero in KHZ2PICOS macro */
> fbmode->pixclock = vm->pixelclock ?
> @@ -1408,10 +1408,8 @@ int fb_videomode_from_videomode(const struct videomode *vm,
> fbmode->vmode |= FB_VMODE_DOUBLE;
> fbmode->flag = 0;
>
> - htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> - vm->hsync_len;
> - vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> - vm->vsync_len;
> + htotal = vm->hact + vm->hfp + vm->hbp + vm->hsl;
> + vtotal = vm->vact + vm->vfp + vm->vbp + vm->vsl;
> /* prevent division by zero */
> if (htotal && vtotal) {
> fbmode->refresh = vm->pixelclock / (htotal * vtotal);
> @@ -1458,7 +1456,7 @@ int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
> fb_videomode_from_videomode(&vm, fb);
>
> pr_debug("%s: got %dx%d display mode from %s\n",
> - of_node_full_name(np), vm.hactive, vm.vactive, np->name);
> + of_node_full_name(np), vm.hact, vm.vact, np->name);
> dump_fb_videomode(fb);
>
> return 0;
> diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
> index 56009bc..79660937 100644
> --- a/drivers/video/of_display_timing.c
> +++ b/drivers/video/of_display_timing.c
> @@ -69,14 +69,14 @@ static struct display_timing *of_get_display_timing(struct device_node *np)
> return NULL;
> }
>
> - ret |= parse_timing_property(np, "hback-porch", &dt->hback_porch);
> - ret |= parse_timing_property(np, "hfront-porch", &dt->hfront_porch);
> - ret |= parse_timing_property(np, "hactive", &dt->hactive);
> - ret |= parse_timing_property(np, "hsync-len", &dt->hsync_len);
> - ret |= parse_timing_property(np, "vback-porch", &dt->vback_porch);
> - ret |= parse_timing_property(np, "vfront-porch", &dt->vfront_porch);
> - ret |= parse_timing_property(np, "vactive", &dt->vactive);
> - ret |= parse_timing_property(np, "vsync-len", &dt->vsync_len);
> + ret |= parse_timing_property(np, "hback-porch", &dt->hbp);
> + ret |= parse_timing_property(np, "hfront-porch", &dt->hfp);
> + ret |= parse_timing_property(np, "hactive", &dt->hact);
> + ret |= parse_timing_property(np, "hsync-len", &dt->hsl);
> + ret |= parse_timing_property(np, "vback-porch", &dt->vbp);
> + ret |= parse_timing_property(np, "vfront-porch", &dt->vfp);
> + ret |= parse_timing_property(np, "vactive", &dt->vact);
> + ret |= parse_timing_property(np, "vsync-len", &dt->vsl);
> ret |= parse_timing_property(np, "clock-frequency", &dt->pixelclock);
>
> dt->flags = 0;
> diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
> index a3d95f2..c42689a 100644
> --- a/drivers/video/videomode.c
> +++ b/drivers/video/videomode.c
> @@ -21,15 +21,15 @@ int videomode_from_timing(const struct display_timings *disp,
> return -EINVAL;
>
> vm->pixelclock = dt->pixelclock.typ;
> - vm->hactive = dt->hactive.typ;
> - vm->hfront_porch = dt->hfront_porch.typ;
> - vm->hback_porch = dt->hback_porch.typ;
> - vm->hsync_len = dt->hsync_len.typ;
> + vm->hact = dt->hact.typ;
> + vm->hfp = dt->hfp.typ;
> + vm->hbp = dt->hbp.typ;
> + vm->hsl = dt->hsl.typ;
>
> - vm->vactive = dt->vactive.typ;
> - vm->vfront_porch = dt->vfront_porch.typ;
> - vm->vback_porch = dt->vback_porch.typ;
> - vm->vsync_len = dt->vsync_len.typ;
> + vm->vact = dt->vact.typ;
> + vm->vfp = dt->vfp.typ;
> + vm->vbp = dt->vbp.typ;
> + vm->vsl = dt->vsl.typ;
>
> vm->flags = dt->flags;
>
> diff --git a/include/video/display_timing.h b/include/video/display_timing.h
> index 5d0259b..db98ef9 100644
> --- a/include/video/display_timing.h
> +++ b/include/video/display_timing.h
> @@ -59,15 +59,15 @@ struct timing_entry {
> struct display_timing {
> struct timing_entry pixelclock;
>
> - struct timing_entry hactive; /* hor. active video */
> - struct timing_entry hfront_porch; /* hor. front porch */
> - struct timing_entry hback_porch; /* hor. back porch */
> - struct timing_entry hsync_len; /* hor. sync len */
> + struct timing_entry hact; /* hor. active video */
> + struct timing_entry hfp; /* hor. front porch */
> + struct timing_entry hbp; /* hor. back porch */
> + struct timing_entry hsl; /* hor. sync len */
>
> - struct timing_entry vactive; /* ver. active video */
> - struct timing_entry vfront_porch; /* ver. front porch */
> - struct timing_entry vback_porch; /* ver. back porch */
> - struct timing_entry vsync_len; /* ver. sync len */
> + struct timing_entry vact; /* ver. active video */
> + struct timing_entry vfp; /* ver. front porch */
> + struct timing_entry vbp; /* ver. back porch */
> + struct timing_entry vsl; /* ver. sync len */
>
> enum display_flags flags; /* display flags */
> };
> diff --git a/include/video/videomode.h b/include/video/videomode.h
> index 8b12e60..b601c0c 100644
> --- a/include/video/videomode.h
> +++ b/include/video/videomode.h
> @@ -19,15 +19,15 @@
> struct videomode {
> unsigned long pixelclock; /* pixelclock in Hz */
>
> - u32 hactive;
> - u32 hfront_porch;
> - u32 hback_porch;
> - u32 hsync_len;
> -
> - u32 vactive;
> - u32 vfront_porch;
> - u32 vback_porch;
> - u32 vsync_len;
> + u32 hact;
> + u32 hfp;
> + u32 hbp;
> + u32 hsl;
> +
> + u32 vact;
> + u32 vfp;
> + u32 vbp;
> + u32 vsl;
>
> enum display_flags flags; /* display flags */
> };
>
Hi,
I really don't like this. It may be shorter, but I think it makes it really
hard to read. And the direct mapping of DT<->C is lost.
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* RE: [PATCH v3] video: Add Hyper-V Synthetic Video Frame Buffer Driver
From: Haiyang Zhang @ 2013-03-12 17:06 UTC (permalink / raw)
To: FlorianSchandinat@gmx.de, akpm@linux-foundation.org,
linux-fbdev@vger.kernel.org
Cc: KY Srinivasan, olaf@aepfle.de, jasowang@redhat.com,
linux-kernel@vger.kernel.org, devel@linuxdriverproject.org
In-Reply-To: <1362779141-9612-1-git-send-email-haiyangz@microsoft.com>
> -----Original Message-----
> From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-
> owner@vger.kernel.org] On Behalf Of Haiyang Zhang
> Sent: Friday, March 08, 2013 4:46 PM
> To: FlorianSchandinat@gmx.de; linux-fbdev@vger.kernel.org
> Cc: Haiyang Zhang; KY Srinivasan; olaf@aepfle.de; jasowang@redhat.com;
> linux-kernel@vger.kernel.org; devel@linuxdriverproject.org
> Subject: [PATCH v3] video: Add Hyper-V Synthetic Video Frame Buffer
> Driver
>
> This is the driver for the Hyper-V Synthetic Video, which supports
> screen
> resolution up to Full HD 1920x1080 on Windows Server 2012 host, and
> 1600x1200
> on Windows Server 2008 R2 or earlier.
> It also solves the double mouse cursor issue of the emulated video mode.
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
>
Hi, Florian and Andrew,
So far, I have responded to all comments from the community, and made the
recommended updates. Do you have any additional comments on this driver?
Thanks,
- Haiyang
^ permalink raw reply
* How to manage OMAP display drivers in the future
From: Tomi Valkeinen @ 2013-03-13 8:57 UTC (permalink / raw)
To: Dave Airlie; +Cc: linux-fbdev, dri-devel@lists.freedesktop.org
Hi Dave,
I'm writing this mail to get some ideas how we should manage OMAP's
display drivers in the future.
As a short intro, we have the following players around:
omapdss - omapdss handles the DSS (display subsystem) hardware. omapdss
doesn't do any buffer management or expose any userspace API (except a
few sysfs files), so it doesn't do anything by itself.
(drivers/video/omap2/dss/)
panel drivers - Drivers for various panel models. The panel drivers use
omapdss API to manage the video bus. (drivers/video/omap2/displays/)
omapfb - Framebuffer driver, uses omapdss to handle the HW.
(drivers/video/omap2/omapfb/)
omap_vout - V4L2 driver for showing video, uses omapdss to handle the
HW. (drivers/media/platform/omap/)
omapdrm - DRM driver, uses omapdss to handle the HW.
(drivers/gpu/drm/omapdrm/)
omapdss and the panel drivers form the lowest level layer. omapfb and
omap_vout can be used at the same time, but omapdrm must be used alone,
without omapfb or omap_vout.
omapfb and omap_vout are not much developed anymore, even though they
are still commonly used. Most of the development happens in omapdss,
panel drivers and omapdrm.
So that's what we have now. In the distant future I see omapfb and
omap_vout disappear totally, the panel drivers would be made generic
using Common Display Framework, and omapdss and omapdrm would more or
less be merged together. However, all that is still far away, and we
need some plan to go forward for now.
Most pressing question is how to get OMAP display patches merged. It
seems that there's not really an fbdev maintainer for the time being,
and fbdev tree has been the route for omapdss, panels and omapfb in the
past. Now that omapdrm is the new main driver for omap display, fbdev
would be somewhat wrong in any case.
Dave, how would you feel about merging changes to all the above
components through DRM tree? Merging all the above together would be the
easiest way, as the changes may have dependencies to each other.
As I said, most of the development should be in omapdss, panels and
omapdrm. There would be an occasional fix for omapfb and omap_vout, or
small changes when omapdss changes require changes elsewhere.
Tomi
^ permalink raw reply
* Re: [PATCH 0/5] at91: atmel_lcdfb: regression fixes and cpu_is removal
From: Nicolas Ferre @ 2013-03-13 10:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5139B169.7050807@atmel.com>
On 03/08/2013 10:37 AM, Nicolas Ferre :
> On 02/10/2013 07:45 PM, Johan Hovold :
>> On Sun, Feb 10, 2013 at 1:47 AM, Olof Johansson <olof@lixom.net> wrote:
>>> On Fri, Feb 08, 2013 at 05:35:13PM +0100, Nicolas Ferre wrote:
>>>> These patches fix a regression in 16-bpp support for older SOCs which
>>>> use IBGR:555 rather than BGR:565 pixel layout. Use SOC-type to
>>>> determine if the controller uses the intensity-bit and restore the
>>>> old layout in that case.
>>>>
>>>> The last patch is a removal of uses of cpu_is_xxxx() macros in
>>>> atmel_lcdfb with a platform-device-id table and static
>>>> configurations.
>>>>
>>>>
>>>> Patches from Johan Hovold taken from: "[PATCH 0/3] atmel_lcdfb: fix
>>>> 16-bpp regression" and "[PATCH v2 0/3] ARM: at91/avr32/atmel_lcdfb:
>>>> remove cpu_is macros" patch series to form a clean patch series with
>>>> my signature.
>>>>
>>>> Arnd, Olof, as it seems that old fbdev drivers are not so much
>>>> reviewed those days, can we take the decision to queue this material
>>>> through arm-soc with other AT91 drivers updates?
>>>
>>> It would be beneficial to get an ack from Florian. Was he involved in
>>> the review of the code that regressed 16-bpp support in the first
>>> place? When was the regression introduced?
>>
>> In v3.4 by commit 787f9fd2328 ("atmel_lcdfb: support 16bit BGR:565 mode,
>> remove unsupported 15bit modes").
>
> Arnd, Olof,
>
> Please tell me if I can do something to ease the adoption of these
> patches during 3.9-rc timeframe (I can rebase it on top of 3.9-rc1 to
> avoid any conflict: the file board-neocore926.c was removed during the
> merge window).
> Johan has written the series a long time ago and we still do not have it
> in mainline.
>
> If the option to ask Andrew is better in your opinion, please tell me.
In case we end-up with an agreement on the path those fixes should
follow, here is the location of my updated material rebased on top of 3.9-rc2:
The following changes since commit f6161aa153581da4a3867a2d1a7caf4be19b6ec9:
Linux 3.9-rc2 (2013-03-10 16:54:19 -0700)
are available in the git repository at:
git://github.com/at91linux/linux-at91.git at91-3.9-fixesLCD
for you to fetch changes up to bbd44f6bd9d1aa735b180b29b5719d63a8e87b55:
ARM: at91/avr32/atmel_lcdfb: add platform device-id table (2013-03-13 11:05:12 +0100)
----------------------------------------------------------------
Johan Hovold (5):
atmel_lcdfb: fix 16-bpp modes on older SOCs
ARM: at91: fix LCD-wiring mode
ARM: at91/avr32/atmel_lcdfb: add bus-clock entry
atmel_lcdfb: move lcdcon2 register access to compute_hozval
ARM: at91/avr32/atmel_lcdfb: add platform device-id table
arch/arm/mach-at91/at91sam9261.c | 2 +
arch/arm/mach-at91/at91sam9261_devices.c | 6 +-
arch/arm/mach-at91/at91sam9263.c | 1 +
arch/arm/mach-at91/at91sam9263_devices.c | 2 +-
arch/arm/mach-at91/at91sam9g45.c | 2 +
arch/arm/mach-at91/at91sam9g45_devices.c | 6 +-
arch/arm/mach-at91/at91sam9rl.c | 1 +
arch/arm/mach-at91/at91sam9rl_devices.c | 2 +-
arch/avr32/mach-at32ap/at32ap700x.c | 6 +-
drivers/video/atmel_lcdfb.c | 130 +++++++++++++++++++++++++++--------
include/video/atmel_lcdc.h | 4 +-
11 files changed, 126 insertions(+), 36 deletions(-)
Thanks, best regards,
--
Nicolas Ferre
^ permalink raw reply
* Re: [PATCH] video: add ili922x lcd driver
From: Anatolij Gustschin @ 2013-03-13 10:55 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Andrew Morton', 'LKML', linux-fbdev,
'Stefano Babic', 'Richard Purdie',
'Florian Tobias Schandinat'
In-Reply-To: <00c701ce029a$e42d3470$ac879d50$%han@samsung.com>
On Mon, 04 Feb 2013 14:45:51 +0900
Jingoo Han <jg1.han@samsung.com> wrote:
...
> > diff --git a/drivers/video/backlight/ili922x.c b/drivers/video/backlight/ili922x.c
> > new file mode 100644
> > index 0000000..18c33df
> > --- /dev/null
> > +++ b/drivers/video/backlight/ili922x.c
> > @@ -0,0 +1,586 @@
> > +/*
> > + * (C) Copyright 2008
> > + * Stefano Babic, DENX Software Engineering, sbabic@denx.de.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> > + * MA 02111-1307 USA
>
> Please remove this comment. It is hard to keep track of the address of
> Free Software Foundation.
> Also, above mentioned address is not the same with the current address.
okay, will do.
> > + *
> > + * This driver implements a lcd device for the ILITEK 922x display
> > + * controller. The interface to the display is SPI and the display's
> > + * memory is cyclically updated
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/errno.h>
> > +#include <linux/string.h>
> > +#include <linux/slab.h>
> > +#include <linux/delay.h>
> > +#include <linux/fb.h>
> > +#include <linux/init.h>
> > +#include <linux/lcd.h>
> > +#include <linux/of.h>
> > +#include <linux/spi/spi.h>
>
> Would you order inclusions of <linux/xxx.h> according to alphabetical
> ordering, for readability?
I'll do it in the next patch version.
...
> > + xfer.bits_per_word = 8;
> > + xfer.len = NUM_DUMMY_BYTES;
> > + spi_message_add_tail(&xfer, &m);
> > + ret = spi_sync(spi, &m);
> > +
> > + udelay(10);
>
> How about replacing udelay() with usleep_range()?
as I see now the send_dummy() is a nop, I'll remove it entirely.
...
> > +static u16 read_status(struct spi_device *spi)
> > +{
> > + struct spi_message m;
>
> Would you replace 'm' with 'msg' or 'message' for the readability?
> It's too short, even though 'm' is used in include/linux/spi/spi.h.
>
> struct spi_message msg;
okay.
...
> > +static int read_reg(struct spi_device *spi, u8 reg, u16 *rx)
> > +{
> > + struct spi_message m;
> > + struct spi_transfer xfer_regindex, xfer_regvalue;
> > + unsigned char tbuf[CMD_BUFSIZE];
> > + unsigned char rbuf[CMD_BUFSIZE];
> > + int ret = 0, len = 0, i, send_bytes;
> > +
> > + send_dummy(spi);
> > +
> > + memset(&xfer_regindex, 0, sizeof(struct spi_transfer));
> > + memset(&xfer_regvalue, 0, sizeof(struct spi_transfer));
> > + spi_message_init(&m);
> > + xfer_regindex.tx_buf = tbuf;
> > + xfer_regindex.rx_buf = rbuf;
> > + xfer_regindex.cs_change = 1;
> > + CHECK_FREQ_REG(spi, &xfer_regindex);
> > +
> > + tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_INDEX,
> > + START_RW_WRITE));
> > + tbuf[1] = set_tx_byte(0);
> > + tbuf[2] = set_tx_byte(reg);
> > + xfer_regindex.bits_per_word = 8;
> > + len = xfer_regindex.len = 3;
> > + spi_message_add_tail(&xfer_regindex, &m);
> > +
> > + send_bytes = len;
> > +
> > + tbuf[len++] = set_tx_byte(START_BYTE(ili922x_id, START_RS_REG,
> > + START_RW_READ));
> > + for (i = len; i < CMD_BUFSIZE; i++)
> > + tbuf[i] = set_tx_byte(0); /* dummy */
> > +
> > + xfer_regvalue.cs_change = 1;
> > + xfer_regvalue.len = 4;
>
> I don't understand why length 4 is necessary.
> In my opinion, length 3 seems to be enough.
> - tbuf[4] is used for sending 'START_BYTE(ili922x_id, START_RS_REG, START_RW_READ)'.
> - rbuf[5] and rbuf[6] are used for receiving value as below.
> *rx = (rbuf[1 + send_bytes] << 8) + rbuf[2 + send_bytes];
>
> However, tbuf[7] or rbuf[7] seems to be unnecessary.
> If I'm wrong, please let me know kindly.
will fix, thanks.
...
> > +static void ili922x_reg_dump(struct spi_device *spi)
> > +{
> > + u8 reg;
> > + u16 rx;
> > +
> > + pr_info("ILI922x configuration registers:\n");
>
> Please replace pr_info() with dev_info() as below.
>
> dev_err(&spi->dev, "ILI922x configuration registers:\n");
okay, I'll use dev_dbg().
>
> > + for (reg = REG_START_OSCILLATION;
> > + reg <= REG_OTP_PROGRAMMING_ID_KEY; reg++) {
> > + read_reg(spi, reg, &rx);
> > + pr_info("reg @ 0x%02X: 0x%04X\n", reg, rx);
>
> Same as above.
will change.
...
> > +static int ili922x_poweron(struct spi_device *spi)
> > +{
> > + int ret = 0;
>
> Initialization is not necessary.
> Just declare it as below:
> int ret;
okay.
> > +
> > + /* Power on */
> > + ret = write_reg(spi, REG_POWER_CONTROL_1, 0x0000);
> > + mdelay(10);
> > + ret += write_reg(spi, REG_POWER_CONTROL_2, 0x0000);
> > + ret += write_reg(spi, REG_POWER_CONTROL_3, 0x0000);
> > + mdelay(40);
> > + ret += write_reg(spi, REG_POWER_CONTROL_4, 0x0000);
> > + mdelay(40);
> > + ret += write_reg(spi, 0x56, 0x080F);
>
> Would you replace this hard-coded address with the bit definition?
It is an undocumented register. I do not know what to use for the
bit definition. Do you know where this register is documented?
Otherwise I'll add a comment that it is not documented.
...
> > + ret += write_reg(spi, REG_POWER_CONTROL_1, 0x4240);
> > + mdelay(10);
> > + ret += write_reg(spi, REG_POWER_CONTROL_2, 0x0000);
> > + ret += write_reg(spi, REG_POWER_CONTROL_3, 0x0014);
> > + mdelay(40);
> > + ret += write_reg(spi, REG_POWER_CONTROL_4, 0x1319);
> > + mdelay(40);
>
> How about replacing mdelay() with msleep()?
will fix.
...
> > +static int ili922x_poweroff(struct spi_device *spi)
> > +{
> > + int ret = 0;
>
> Initialization is not necessary.
> Just declare it as below:
> int ret;
>
>
> > +
> > + /* Power off */
> > + ret = write_reg(spi, REG_POWER_CONTROL_1, 0x0000);
> > + mdelay(10);
> > + ret += write_reg(spi, REG_POWER_CONTROL_2, 0x0000);
> > + ret += write_reg(spi, REG_POWER_CONTROL_3, 0x0000);
> > + mdelay(40);
> > + ret += write_reg(spi, REG_POWER_CONTROL_4, 0x0000);
> > + mdelay(40);
>
> Same as above.
...
> > +static void ili922x_display_init(struct spi_device *spi)
> > +{
> > + write_reg(spi, REG_START_OSCILLATION, 1);
> > + mdelay(10);
>
> Same as above.
okay, will change.
Thanks,
Anatolij
^ permalink raw reply
* [PATCH v2] video: backlight: add ili922x lcd driver
From: Anatolij Gustschin @ 2013-03-13 12:43 UTC (permalink / raw)
To: linux-fbdev
From: Stefano Babic <sbabic@denx.de>
Add LCD driver for Ilitek ILI9221/ILI9222 controller.
The driver uses SPI interface for controller access
and configuration and RGB interface for graphics data
transfer.
Signed-off-by: Stefano Babic <sbabic@denx.de>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
---
Changes in v2:
- simplify licence header (remove old FSF address)
- sort include statements
- drop send_dummy() as it was a NOP
- use 'msg' for spi message vars
- fix spi xfer length in read_reg()
- use dev_dbg() instead of pr_info()
- drop not needed local variable initialisations
- replace short mdelay() with usleep_range()
and longer mdelay() with msleep()
- rewise read_status() to consider spi errors
and add a comment explaining the xfer length
- use 'ili922x_' prefix for register read/write
and status functions
- rebased on top of linux-next tree
drivers/video/backlight/Kconfig | 7 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/ili922x.c | 555 +++++++++++++++++++++++++++++++++++++
3 files changed, 563 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/backlight/ili922x.c
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index db10d01..8613b87 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -59,6 +59,13 @@ config LCD_LTV350QV
The LTV350QV panel is present on all ATSTK1000 boards.
+config LCD_ILI922X
+ tristate "ILI Technology ILI9221/ILI9222 support"
+ depends on SPI
+ help
+ If you have a panel based on the ILI9221/9222 controller
+ chip then say y to include a driver for it.
+
config LCD_ILI9320
tristate "ILI Technology ILI9320 controller support"
depends on SPI
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 96c4d62..92711fe 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_LCD_CLASS_DEVICE) += lcd.o
obj-$(CONFIG_LCD_CORGI) += corgi_lcd.o
obj-$(CONFIG_LCD_HP700) += jornada720_lcd.o
obj-$(CONFIG_LCD_HX8357) += hx8357.o
+obj-$(CONFIG_LCD_ILI922X) += ili922x.o
obj-$(CONFIG_LCD_ILI9320) += ili9320.o
obj-$(CONFIG_LCD_L4F00242T03) += l4f00242t03.o
obj-$(CONFIG_LCD_LD9040) += ld9040.o
diff --git a/drivers/video/backlight/ili922x.c b/drivers/video/backlight/ili922x.c
new file mode 100644
index 0000000..8b8465e
--- /dev/null
+++ b/drivers/video/backlight/ili922x.c
@@ -0,0 +1,555 @@
+/*
+ * (C) Copyright 2008
+ * Stefano Babic, DENX Software Engineering, sbabic@denx.de.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This driver implements a lcd device for the ILITEK 922x display
+ * controller. The interface to the display is SPI and the display's
+ * memory is cyclically updated over the RGB interface.
+ */
+
+#include <linux/fb.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/lcd.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/string.h>
+
+/* Register offset, see manual section 8.2 */
+#define REG_START_OSCILLATION 0x00
+#define REG_DRIVER_CODE_READ 0x00
+#define REG_DRIVER_OUTPUT_CONTROL 0x01
+#define REG_LCD_AC_DRIVEING_CONTROL 0x02
+#define REG_ENTRY_MODE 0x03
+#define REG_COMPARE_1 0x04
+#define REG_COMPARE_2 0x05
+#define REG_DISPLAY_CONTROL_1 0x07
+#define REG_DISPLAY_CONTROL_2 0x08
+#define REG_DISPLAY_CONTROL_3 0x09
+#define REG_FRAME_CYCLE_CONTROL 0x0B
+#define REG_EXT_INTF_CONTROL 0x0C
+#define REG_POWER_CONTROL_1 0x10
+#define REG_POWER_CONTROL_2 0x11
+#define REG_POWER_CONTROL_3 0x12
+#define REG_POWER_CONTROL_4 0x13
+#define REG_RAM_ADDRESS_SET 0x21
+#define REG_WRITE_DATA_TO_GRAM 0x22
+#define REG_RAM_WRITE_MASK1 0x23
+#define REG_RAM_WRITE_MASK2 0x24
+#define REG_GAMMA_CONTROL_1 0x30
+#define REG_GAMMA_CONTROL_2 0x31
+#define REG_GAMMA_CONTROL_3 0x32
+#define REG_GAMMA_CONTROL_4 0x33
+#define REG_GAMMA_CONTROL_5 0x34
+#define REG_GAMMA_CONTROL_6 0x35
+#define REG_GAMMA_CONTROL_7 0x36
+#define REG_GAMMA_CONTROL_8 0x37
+#define REG_GAMMA_CONTROL_9 0x38
+#define REG_GAMMA_CONTROL_10 0x39
+#define REG_GATE_SCAN_CONTROL 0x40
+#define REG_VERT_SCROLL_CONTROL 0x41
+#define REG_FIRST_SCREEN_DRIVE_POS 0x42
+#define REG_SECOND_SCREEN_DRIVE_POS 0x43
+#define REG_RAM_ADDR_POS_H 0x44
+#define REG_RAM_ADDR_POS_V 0x45
+#define REG_OSCILLATOR_CONTROL 0x4F
+#define REG_GPIO 0x60
+#define REG_OTP_VCM_PROGRAMMING 0x61
+#define REG_OTP_VCM_STATUS_ENABLE 0x62
+#define REG_OTP_PROGRAMMING_ID_KEY 0x65
+
+/*
+ * maximum frequency for register access
+ * (not for the GRAM access)
+ */
+#define ILITEK_MAX_FREQ_REG 4000000
+
+/*
+ * Device ID as found in the datasheet (supports 9221 and 9222)
+ */
+#define ILITEK_DEVICE_ID 0x9220
+#define ILITEK_DEVICE_ID_MASK 0xFFF0
+
+/* Last two bits in the START BYTE */
+#define START_RS_INDEX 0
+#define START_RS_REG 1
+#define START_RW_WRITE 0
+#define START_RW_READ 1
+
+/**
+ * START_BYTE(id, rs, rw)
+ *
+ * Set the start byte according to the required operation.
+ * The start byte is defined as:
+ * ----------------------------------
+ * | 0 | 1 | 1 | 1 | 0 | ID | RS | RW |
+ * ----------------------------------
+ * @id: display's id as set by the manufacturer
+ * @rs: operation type bit, one of:
+ * - START_RS_INDEX set the index register
+ * - START_RS_REG write/read registers/GRAM
+ * @rw: read/write operation
+ * - START_RW_WRITE write
+ * - START_RW_READ read
+ */
+#define START_BYTE(id, rs, rw) \
+ (0x70 | (((id) & 0x01) << 2) | (((rs) & 0x01) << 1) | ((rw) & 0x01))
+
+/**
+ * CHECK_FREQ_REG(spi_device s, spi_transfer x) - Check the frequency
+ * for the SPI transfer. According to the datasheet, the controller
+ * accept higher frequency for the GRAM transfer, but it requires
+ * lower frequency when the registers are read/written.
+ * The macro sets the frequency in the spi_transfer structure if
+ * the frequency exceeds the maximum value.
+ */
+#define CHECK_FREQ_REG(s, x) \
+ do { \
+ if (s->max_speed_hz > ILITEK_MAX_FREQ_REG) \
+ ((struct spi_transfer *)x)->speed_hz = \
+ ILITEK_MAX_FREQ_REG; \
+ } while (0)
+
+#define CMD_BUFSIZE 16
+
+#define POWER_IS_ON(pwr) ((pwr) <= FB_BLANK_NORMAL)
+
+#define set_tx_byte(b) (tx_invert ? ~(b) : b)
+
+/**
+ * ili922x_id - id as set by manufacturer
+ */
+static int ili922x_id = 1;
+module_param(ili922x_id, int, 0);
+
+static int tx_invert;
+module_param(tx_invert, int, 0);
+
+/**
+ * driver's private structure
+ */
+struct ili922x {
+ struct spi_device *spi;
+ struct lcd_device *ld;
+ int power;
+};
+
+/**
+ * ili922x_read_status - read status register from display
+ * @spi: spi device
+ * @rs: output value
+ */
+static int ili922x_read_status(struct spi_device *spi, u16 *rs)
+{
+ struct spi_message msg;
+ struct spi_transfer xfer;
+ unsigned char tbuf[CMD_BUFSIZE];
+ unsigned char rbuf[CMD_BUFSIZE];
+ int ret, i;
+
+ memset(&xfer, 0, sizeof(struct spi_transfer));
+ spi_message_init(&msg);
+ xfer.tx_buf = tbuf;
+ xfer.rx_buf = rbuf;
+ xfer.cs_change = 1;
+ CHECK_FREQ_REG(spi, &xfer);
+
+ tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_INDEX,
+ START_RW_READ));
+ /*
+ * we need 4-byte xfer here due to invalid dummy byte
+ * received after start byte
+ */
+ for (i = 1; i < 4; i++)
+ tbuf[i] = set_tx_byte(0); /* dummy */
+
+ xfer.bits_per_word = 8;
+ xfer.len = 4;
+ spi_message_add_tail(&xfer, &msg);
+ ret = spi_sync(spi, &msg);
+ if (ret < 0) {
+ dev_dbg(&spi->dev, "Error sending SPI message 0x%x", ret);
+ return ret;
+ }
+
+ *rs = (rbuf[2] << 8) + rbuf[3];
+ return 0;
+}
+
+/**
+ * ili922x_read - read register from display
+ * @spi: spi device
+ * @reg: offset of the register to be read
+ * @rx: output value
+ */
+static int ili922x_read(struct spi_device *spi, u8 reg, u16 *rx)
+{
+ struct spi_message msg;
+ struct spi_transfer xfer_regindex, xfer_regvalue;
+ unsigned char tbuf[CMD_BUFSIZE];
+ unsigned char rbuf[CMD_BUFSIZE];
+ int ret, len = 0, send_bytes;
+
+ memset(&xfer_regindex, 0, sizeof(struct spi_transfer));
+ memset(&xfer_regvalue, 0, sizeof(struct spi_transfer));
+ spi_message_init(&msg);
+ xfer_regindex.tx_buf = tbuf;
+ xfer_regindex.rx_buf = rbuf;
+ xfer_regindex.cs_change = 1;
+ CHECK_FREQ_REG(spi, &xfer_regindex);
+
+ tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_INDEX,
+ START_RW_WRITE));
+ tbuf[1] = set_tx_byte(0);
+ tbuf[2] = set_tx_byte(reg);
+ xfer_regindex.bits_per_word = 8;
+ len = xfer_regindex.len = 3;
+ spi_message_add_tail(&xfer_regindex, &msg);
+
+ send_bytes = len;
+
+ tbuf[len++] = set_tx_byte(START_BYTE(ili922x_id, START_RS_REG,
+ START_RW_READ));
+ tbuf[len++] = set_tx_byte(0);
+ tbuf[len] = set_tx_byte(0);
+
+ xfer_regvalue.cs_change = 1;
+ xfer_regvalue.len = 3;
+ xfer_regvalue.tx_buf = &tbuf[send_bytes];
+ xfer_regvalue.rx_buf = &rbuf[send_bytes];
+ CHECK_FREQ_REG(spi, &xfer_regvalue);
+
+ spi_message_add_tail(&xfer_regvalue, &msg);
+ ret = spi_sync(spi, &msg);
+ if (ret < 0) {
+ dev_dbg(&spi->dev, "Error sending SPI message 0x%x", ret);
+ return ret;
+ }
+
+ *rx = (rbuf[1 + send_bytes] << 8) + rbuf[2 + send_bytes];
+ return 0;
+}
+
+/**
+ * ili922x_write - write a controller register
+ * @spi: struct spi_device *
+ * @reg: offset of the register to be written
+ * @value: value to be written
+ */
+static int ili922x_write(struct spi_device *spi, u8 reg, u16 value)
+{
+ struct spi_message msg;
+ struct spi_transfer xfer_regindex, xfer_regvalue;
+ unsigned char tbuf[CMD_BUFSIZE];
+ unsigned char rbuf[CMD_BUFSIZE];
+ int ret, len = 0;
+
+ memset(&xfer_regindex, 0, sizeof(struct spi_transfer));
+ memset(&xfer_regvalue, 0, sizeof(struct spi_transfer));
+
+ spi_message_init(&msg);
+ xfer_regindex.tx_buf = tbuf;
+ xfer_regindex.rx_buf = rbuf;
+ xfer_regindex.cs_change = 1;
+ CHECK_FREQ_REG(spi, &xfer_regindex);
+
+ tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_INDEX,
+ START_RW_WRITE));
+ tbuf[1] = set_tx_byte(0);
+ tbuf[2] = set_tx_byte(reg);
+ xfer_regindex.bits_per_word = 8;
+ xfer_regindex.len = 3;
+ spi_message_add_tail(&xfer_regindex, &msg);
+
+ ret = spi_sync(spi, &msg);
+
+ spi_message_init(&msg);
+ len = 0;
+ tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_REG,
+ START_RW_WRITE));
+ tbuf[1] = set_tx_byte((value & 0xFF00) >> 8);
+ tbuf[2] = set_tx_byte(value & 0x00FF);
+
+ xfer_regvalue.cs_change = 1;
+ xfer_regvalue.len = 3;
+ xfer_regvalue.tx_buf = tbuf;
+ xfer_regvalue.rx_buf = rbuf;
+ CHECK_FREQ_REG(spi, &xfer_regvalue);
+
+ spi_message_add_tail(&xfer_regvalue, &msg);
+
+ ret = spi_sync(spi, &msg);
+ if (ret < 0) {
+ dev_err(&spi->dev, "Error sending SPI message 0x%x", ret);
+ return ret;
+ }
+ return 0;
+}
+
+#ifdef DEBUG
+/**
+ * ili922x_reg_dump - dump all registers
+ */
+static void ili922x_reg_dump(struct spi_device *spi)
+{
+ u8 reg;
+ u16 rx;
+
+ dev_dbg(&spi->dev, "ILI922x configuration registers:\n");
+ for (reg = REG_START_OSCILLATION;
+ reg <= REG_OTP_PROGRAMMING_ID_KEY; reg++) {
+ ili922x_read(spi, reg, &rx);
+ dev_dbg(&spi->dev, "reg @ 0x%02X: 0x%04X\n", reg, rx);
+ }
+}
+#else
+static inline void ili922x_reg_dump(struct spi_device *spi) {}
+#endif
+
+/**
+ * set_write_to_gram_reg - initialize the display to write the GRAM
+ * @spi: spi device
+ */
+static void set_write_to_gram_reg(struct spi_device *spi)
+{
+ struct spi_message msg;
+ struct spi_transfer xfer;
+ unsigned char tbuf[CMD_BUFSIZE];
+
+ memset(&xfer, 0, sizeof(struct spi_transfer));
+
+ spi_message_init(&msg);
+ xfer.tx_buf = tbuf;
+ xfer.rx_buf = NULL;
+ xfer.cs_change = 1;
+
+ tbuf[0] = START_BYTE(ili922x_id, START_RS_INDEX, START_RW_WRITE);
+ tbuf[1] = 0;
+ tbuf[2] = REG_WRITE_DATA_TO_GRAM;
+
+ xfer.bits_per_word = 8;
+ xfer.len = 3;
+ spi_message_add_tail(&xfer, &msg);
+ spi_sync(spi, &msg);
+}
+
+/**
+ * ili922x_poweron - turn the display on
+ * @spi: spi device
+ *
+ * The sequence to turn on the display is taken from
+ * the datasheet and/or the example code provided by the
+ * manufacturer.
+ */
+static int ili922x_poweron(struct spi_device *spi)
+{
+ int ret;
+
+ /* Power on */
+ ret = ili922x_write(spi, REG_POWER_CONTROL_1, 0x0000);
+ usleep_range(10000, 10500);
+ ret += ili922x_write(spi, REG_POWER_CONTROL_2, 0x0000);
+ ret += ili922x_write(spi, REG_POWER_CONTROL_3, 0x0000);
+ msleep(40);
+ ret += ili922x_write(spi, REG_POWER_CONTROL_4, 0x0000);
+ msleep(40);
+ /* register 0x56 is not documented in the datasheet */
+ ret += ili922x_write(spi, 0x56, 0x080F);
+ ret += ili922x_write(spi, REG_POWER_CONTROL_1, 0x4240);
+ usleep_range(10000, 10500);
+ ret += ili922x_write(spi, REG_POWER_CONTROL_2, 0x0000);
+ ret += ili922x_write(spi, REG_POWER_CONTROL_3, 0x0014);
+ msleep(40);
+ ret += ili922x_write(spi, REG_POWER_CONTROL_4, 0x1319);
+ msleep(40);
+
+ return ret;
+}
+
+/**
+ * ili922x_poweroff - turn the display off
+ * @spi: spi device
+ */
+static int ili922x_poweroff(struct spi_device *spi)
+{
+ int ret;
+
+ /* Power off */
+ ret = ili922x_write(spi, REG_POWER_CONTROL_1, 0x0000);
+ usleep_range(10000, 10500);
+ ret += ili922x_write(spi, REG_POWER_CONTROL_2, 0x0000);
+ ret += ili922x_write(spi, REG_POWER_CONTROL_3, 0x0000);
+ msleep(40);
+ ret += ili922x_write(spi, REG_POWER_CONTROL_4, 0x0000);
+ msleep(40);
+
+ return ret;
+}
+
+/**
+ * ili922x_display_init - initialize the display by setting
+ * the configuration registers
+ * @spi: spi device
+ */
+static void ili922x_display_init(struct spi_device *spi)
+{
+ ili922x_write(spi, REG_START_OSCILLATION, 1);
+ usleep_range(10000, 10500);
+ ili922x_write(spi, REG_DRIVER_OUTPUT_CONTROL, 0x691B);
+ ili922x_write(spi, REG_LCD_AC_DRIVEING_CONTROL, 0x0700);
+ ili922x_write(spi, REG_ENTRY_MODE, 0x1030);
+ ili922x_write(spi, REG_COMPARE_1, 0x0000);
+ ili922x_write(spi, REG_COMPARE_2, 0x0000);
+ ili922x_write(spi, REG_DISPLAY_CONTROL_1, 0x0037);
+ ili922x_write(spi, REG_DISPLAY_CONTROL_2, 0x0202);
+ ili922x_write(spi, REG_DISPLAY_CONTROL_3, 0x0000);
+ ili922x_write(spi, REG_FRAME_CYCLE_CONTROL, 0x0000);
+
+ /* Set RGB interface */
+ ili922x_write(spi, REG_EXT_INTF_CONTROL, 0x0110);
+
+ ili922x_poweron(spi);
+
+ ili922x_write(spi, REG_GAMMA_CONTROL_1, 0x0302);
+ ili922x_write(spi, REG_GAMMA_CONTROL_2, 0x0407);
+ ili922x_write(spi, REG_GAMMA_CONTROL_3, 0x0304);
+ ili922x_write(spi, REG_GAMMA_CONTROL_4, 0x0203);
+ ili922x_write(spi, REG_GAMMA_CONTROL_5, 0x0706);
+ ili922x_write(spi, REG_GAMMA_CONTROL_6, 0x0407);
+ ili922x_write(spi, REG_GAMMA_CONTROL_7, 0x0706);
+ ili922x_write(spi, REG_GAMMA_CONTROL_8, 0x0000);
+ ili922x_write(spi, REG_GAMMA_CONTROL_9, 0x0C06);
+ ili922x_write(spi, REG_GAMMA_CONTROL_10, 0x0F00);
+ ili922x_write(spi, REG_RAM_ADDRESS_SET, 0x0000);
+ ili922x_write(spi, REG_GATE_SCAN_CONTROL, 0x0000);
+ ili922x_write(spi, REG_VERT_SCROLL_CONTROL, 0x0000);
+ ili922x_write(spi, REG_FIRST_SCREEN_DRIVE_POS, 0xDB00);
+ ili922x_write(spi, REG_SECOND_SCREEN_DRIVE_POS, 0xDB00);
+ ili922x_write(spi, REG_RAM_ADDR_POS_H, 0xAF00);
+ ili922x_write(spi, REG_RAM_ADDR_POS_V, 0xDB00);
+ ili922x_reg_dump(spi);
+ set_write_to_gram_reg(spi);
+}
+
+static int ili922x_lcd_power(struct ili922x *lcd, int power)
+{
+ int ret = 0;
+
+ if (POWER_IS_ON(power) && !POWER_IS_ON(lcd->power))
+ ret = ili922x_poweron(lcd->spi);
+ else if (!POWER_IS_ON(power) && POWER_IS_ON(lcd->power))
+ ret = ili922x_poweroff(lcd->spi);
+
+ if (!ret)
+ lcd->power = power;
+
+ return ret;
+}
+
+static int ili922x_set_power(struct lcd_device *ld, int power)
+{
+ struct ili922x *ili = lcd_get_data(ld);
+
+ return ili922x_lcd_power(ili, power);
+}
+
+static int ili922x_get_power(struct lcd_device *ld)
+{
+ struct ili922x *ili = lcd_get_data(ld);
+
+ return ili->power;
+}
+
+static struct lcd_ops ili922x_ops = {
+ .get_power = ili922x_get_power,
+ .set_power = ili922x_set_power,
+};
+
+static int ili922x_probe(struct spi_device *spi)
+{
+ struct ili922x *ili;
+ struct lcd_device *lcd;
+ int ret;
+ u16 reg = 0;
+
+ ili = devm_kzalloc(&spi->dev, sizeof(*ili), GFP_KERNEL);
+ if (!ili) {
+ dev_err(&spi->dev, "cannot alloc priv data\n");
+ return -ENOMEM;
+ }
+
+ ili->spi = spi;
+ dev_set_drvdata(&spi->dev, ili);
+
+ /* check if the device is connected */
+ ret = ili922x_read(spi, REG_DRIVER_CODE_READ, ®);
+ if (ret || ((reg & ILITEK_DEVICE_ID_MASK) != ILITEK_DEVICE_ID)) {
+ dev_err(&spi->dev,
+ "no LCD found: Chip ID 0x%x, ret %d\n",
+ reg, ret);
+ return -ENODEV;
+ } else {
+ dev_info(&spi->dev, "ILI%x found, SPI freq %d, mode %d\n",
+ reg, spi->max_speed_hz, spi->mode);
+ }
+
+ ret = ili922x_read_status(spi, ®);
+ if (ret) {
+ dev_err(&spi->dev, "reading RS failed...\n");
+ return ret;
+ } else
+ dev_dbg(&spi->dev, "status: 0x%x\n", reg);
+
+ ili922x_display_init(spi);
+
+ ili->power = FB_BLANK_POWERDOWN;
+
+ lcd = lcd_device_register("ili922xlcd", &spi->dev, ili,
+ &ili922x_ops);
+ if (IS_ERR(lcd)) {
+ dev_err(&spi->dev, "cannot register LCD\n");
+ return PTR_ERR(lcd);
+ }
+
+ ili->ld = lcd;
+ spi_set_drvdata(spi, ili);
+
+ ili922x_lcd_power(ili, FB_BLANK_UNBLANK);
+
+ return 0;
+}
+
+static int ili922x_remove(struct spi_device *spi)
+{
+ struct ili922x *ili = spi_get_drvdata(spi);
+
+ ili922x_poweroff(spi);
+ lcd_device_unregister(ili->ld);
+ return 0;
+}
+
+static struct spi_driver ili922x_driver = {
+ .driver = {
+ .name = "ili922x",
+ .owner = THIS_MODULE,
+ },
+ .probe = ili922x_probe,
+ .remove = ili922x_remove,
+};
+
+module_spi_driver(ili922x_driver);
+
+MODULE_AUTHOR("Stefano Babic <sbabic@denx.de>");
+MODULE_DESCRIPTION("ILI9221/9222 LCD driver");
+MODULE_LICENSE("GPL");
+MODULE_PARM_DESC(ili922x_id, "set controller identifier (default=1)");
+MODULE_PARM_DESC(tx_invert, "invert bytes before sending");
--
1.7.5.4
^ permalink raw reply related
* [PATCH] fbdev: fsl-diu-fb: optionally configure frame buffer depth in DT
From: Anatolij Gustschin @ 2013-03-13 15:24 UTC (permalink / raw)
To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA
Cc: Florian Tobias Schandinat,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Timur Tabi,
Rob Herring
Add support for 'depth' property to configure default frame
buffer color depth over device tree.
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
.../devicetree/bindings/powerpc/fsl/diu.txt | 2 ++
drivers/video/fsl-diu-fb.c | 16 ++++++++++++++++
2 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/diu.txt b/Documentation/devicetree/bindings/powerpc/fsl/diu.txt
index b66cb6d..502aae5 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/diu.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/diu.txt
@@ -12,6 +12,7 @@ Required properties:
services interrupts for this device.
Optional properties:
+- depth : default frame buffer color depth
- edid : verbatim EDID data block describing attached display.
Data from the detailed timing descriptor will be used to
program the display controller.
@@ -30,5 +31,6 @@ Example for MPC5121:
reg = <0x2100 0x100>;
interrupts = <64 0x8>;
interrupt-parent = <&ipic>;
+ depth = <16>;
edid = [edid-data];
};
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 41fbd94..2287817 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1534,6 +1534,7 @@ static int fsl_diu_probe(struct platform_device *pdev)
const void *prop;
unsigned int i;
int ret;
+ u32 depth;
data = dmam_alloc_coherent(&pdev->dev, sizeof(struct fsl_diu_data),
&dma_addr, GFP_DMA | __GFP_ZERO);
@@ -1584,6 +1585,21 @@ static int fsl_diu_probe(struct platform_device *pdev)
data->has_edid = true;
}
+ if (!of_property_read_u32(np, "depth", &depth)) {
+ switch (depth) {
+ case 32:
+ case 24:
+ case 16:
+ case 8:
+ default_bpp = depth;
+ break;
+ default:
+ dev_err(&pdev->dev,
+ "%s: invalid depth property\n",
+ np->full_name);
+ }
+ }
+
data->diu_reg = of_iomap(np, 0);
if (!data->diu_reg) {
dev_err(&pdev->dev, "cannot map DIU registers\n");
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH] fbdev: fsl-diu-fb: optionally configure frame buffer depth in DT
From: Timur Tabi @ 2013-03-13 15:37 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1363188249-29341-1-git-send-email-agust@denx.de>
On 03/13/2013 10:24 AM, Anatolij Gustschin wrote:
> @@ -30,5 +31,6 @@ Example for MPC5121:
> reg = <0x2100 0x100>;
> interrupts = <64 0x8>;
> interrupt-parent = <&ipic>;
> + depth = <16>;
NACK.
Device trees are supposed to be used for describing the hardware, not
for software configuration. Besides, the driver already supports a
command-line parameter for the color depth. Your patch just overrides
that parameter (default_bpp) with a device tree property.
module_param_named(mode, fb_mode, charp, 0);
MODULE_PARM_DESC(mode,
"Specify resolution as \"<xres>x<yres>[-<bpp>][@<refresh>]\" ");
module_param_named(bpp, default_bpp, ulong, 0);
MODULE_PARM_DESC(bpp, "Specify bit-per-pixel if not specified in 'mode'");
module_param_named(monitor, monitor_string, charp, 0);
MODULE_PARM_DESC(monitor, "Specify the monitor port "
"(\"dvi\", \"lvds\", or \"dlvds\") if supported by the platform");
^ permalink raw reply
* Re: [PATCH v2] video: backlight: add ili922x lcd driver
From: Andrew Morton @ 2013-03-13 23:15 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1363178617-21940-1-git-send-email-agust@denx.de>
On Wed, 13 Mar 2013 13:43:37 +0100 Anatolij Gustschin <agust@denx.de> wrote:
> From: Stefano Babic <sbabic@denx.de>
>
> Add LCD driver for Ilitek ILI9221/ILI9222 controller.
> The driver uses SPI interface for controller access
> and configuration and RGB interface for graphics data
> transfer.
>
> ...
>
> +#define START_BYTE(id, rs, rw) \
> + (0x70 | (((id) & 0x01) << 2) | (((rs) & 0x01) << 1) | ((rw) & 0x01))
> +#define CHECK_FREQ_REG(s, x) \
> + do { \
> + if (s->max_speed_hz > ILITEK_MAX_FREQ_REG) \
> + ((struct spi_transfer *)x)->speed_hz = \
> + ILITEK_MAX_FREQ_REG; \
> + } while (0)
> +#define POWER_IS_ON(pwr) ((pwr) <= FB_BLANK_NORMAL)
> +#define set_tx_byte(b) (tx_invert ? ~(b) : b)
All of the above could have been implemented as regular old C
functions, and would be better if they were!
It otherwise all looks nice.
^ permalink raw reply
* [PATCH 0/6] Remove mxsfb auxdata
From: Shawn Guo @ 2013-03-14 5:53 UTC (permalink / raw)
To: linux-arm-kernel
The mxsfb driver was converted to device tree by asking platform to
pass mxsfb_platform_data with auxdata. We did this to have mach-mxs
be DT only platform when there was no videomode bindings. Now with
the videomode bindings and helpers in place, the series changes mxsfb
driver to get display_timings from device tree, and removes mxsfb
platform_data and auxdata completely.
To maintain the bisectability, the series needs to go via single tree,
and I prefer to arm-soc tree.
Shawn Guo (6):
video: mxsfb: use devm_* managed functions
video: mxsfb: remove fb_phys/fb_size from platform_data
video: mxsfb: remove dotclk_delay from platform_data
video: mxsfb: get display timings from device tree
ARM: mxs: move display timing configurations into device tree
video: mxsfb: remove mxsfb_platform_data
Documentation/devicetree/bindings/fb/mxsfb.txt | 34 ++++
arch/arm/boot/dts/imx23-evk.dts | 25 +++
arch/arm/boot/dts/imx28-apf28dev.dts | 25 +++
arch/arm/boot/dts/imx28-apx4devkit.dts | 25 +++
arch/arm/boot/dts/imx28-cfa10049.dts | 25 +++
arch/arm/boot/dts/imx28-evk.dts | 25 +++
arch/arm/boot/dts/imx28-m28evk.dts | 25 +++
arch/arm/mach-mxs/mach-mxs.c | 153 -----------------
drivers/video/Kconfig | 2 +
drivers/video/mxsfb.c | 219 +++++++++++++++---------
include/linux/mxsfb.h | 49 ------
11 files changed, 320 insertions(+), 287 deletions(-)
delete mode 100644 include/linux/mxsfb.h
--
1.7.9.5
^ permalink raw reply
* [PATCH 1/6] video: mxsfb: use devm_* managed functions
From: Shawn Guo @ 2013-03-14 5:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1363240410-16865-1-git-send-email-shawn.guo@linaro.org>
Use devm_* managed functions to make code a little cleaner.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
drivers/video/mxsfb.c | 50 ++++++++++++++++---------------------------------
1 file changed, 16 insertions(+), 34 deletions(-)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 755556c..2c5ab15 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -801,23 +801,19 @@ static int mxsfb_probe(struct platform_device *pdev)
return -ENODEV;
}
- if (!request_mem_region(res->start, resource_size(res), pdev->name))
- return -EBUSY;
-
fb_info = framebuffer_alloc(sizeof(struct mxsfb_info), &pdev->dev);
if (!fb_info) {
dev_err(&pdev->dev, "Failed to allocate fbdev\n");
- ret = -ENOMEM;
- goto error_alloc_info;
+ return -ENOMEM;
}
host = to_imxfb_host(fb_info);
- host->base = ioremap(res->start, resource_size(res));
- if (!host->base) {
+ host->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(host->base)) {
dev_err(&pdev->dev, "ioremap failed\n");
- ret = -ENOMEM;
- goto error_ioremap;
+ ret = PTR_ERR(host->base);
+ goto fb_release;
}
host->pdev = pdev;
@@ -828,13 +824,13 @@ static int mxsfb_probe(struct platform_device *pdev)
pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
if (IS_ERR(pinctrl)) {
ret = PTR_ERR(pinctrl);
- goto error_getpin;
+ goto fb_release;
}
- host->clk = clk_get(&host->pdev->dev, NULL);
+ host->clk = devm_clk_get(&host->pdev->dev, NULL);
if (IS_ERR(host->clk)) {
ret = PTR_ERR(host->clk);
- goto error_getclock;
+ goto fb_release;
}
panel_enable = of_get_named_gpio_flags(pdev->dev.of_node,
@@ -849,21 +845,22 @@ static int mxsfb_probe(struct platform_device *pdev)
dev_err(&pdev->dev,
"failed to request gpio %d: %d\n",
panel_enable, ret);
- goto error_panel_enable;
+ goto fb_release;
}
}
- fb_info->pseudo_palette = kmalloc(sizeof(u32) * 16, GFP_KERNEL);
+ fb_info->pseudo_palette = devm_kzalloc(&pdev->dev, sizeof(u32) * 16,
+ GFP_KERNEL);
if (!fb_info->pseudo_palette) {
ret = -ENOMEM;
- goto error_pseudo_pallette;
+ goto fb_release;
}
INIT_LIST_HEAD(&fb_info->modelist);
ret = mxsfb_init_fbinfo(host);
if (ret != 0)
- goto error_init_fb;
+ goto fb_release;
for (i = 0; i < pdata->mode_count; i++)
fb_add_videomode(&pdata->mode_list[i], &fb_info->modelist);
@@ -880,7 +877,7 @@ static int mxsfb_probe(struct platform_device *pdev)
ret = register_framebuffer(fb_info);
if (ret != 0) {
dev_err(&pdev->dev,"Failed to register framebuffer\n");
- goto error_register;
+ goto fb_destroy;
}
if (!host->enabled) {
@@ -893,22 +890,12 @@ static int mxsfb_probe(struct platform_device *pdev)
return 0;
-error_register:
+fb_destroy:
if (host->enabled)
clk_disable_unprepare(host->clk);
fb_destroy_modelist(&fb_info->modelist);
-error_init_fb:
- kfree(fb_info->pseudo_palette);
-error_pseudo_pallette:
-error_panel_enable:
- clk_put(host->clk);
-error_getclock:
-error_getpin:
- iounmap(host->base);
-error_ioremap:
+fb_release:
framebuffer_release(fb_info);
-error_alloc_info:
- release_mem_region(res->start, resource_size(res));
return ret;
}
@@ -917,19 +904,14 @@ static int mxsfb_remove(struct platform_device *pdev)
{
struct fb_info *fb_info = platform_get_drvdata(pdev);
struct mxsfb_info *host = to_imxfb_host(fb_info);
- struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (host->enabled)
mxsfb_disable_controller(fb_info);
unregister_framebuffer(fb_info);
- kfree(fb_info->pseudo_palette);
mxsfb_free_videomem(host);
- iounmap(host->base);
- clk_put(host->clk);
framebuffer_release(fb_info);
- release_mem_region(res->start, resource_size(res));
platform_set_drvdata(pdev, NULL);
--
1.7.9.5
^ permalink raw reply related
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