Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 4/4] ARM: dts: mxs: add oled support for the cfa-10036
From: Maxime Ripard @ 2012-07-17 15:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1342540787-14930-1-git-send-email-maxime.ripard@free-electrons.com>

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Brian Lilly <brian@crystalfontz.com>
---
 arch/arm/boot/dts/imx28-cfa10036.dts |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/imx28-cfa10036.dts b/arch/arm/boot/dts/imx28-cfa10036.dts
index c03a577..0a5c722 100644
--- a/arch/arm/boot/dts/imx28-cfa10036.dts
+++ b/arch/arm/boot/dts/imx28-cfa10036.dts
@@ -33,11 +33,31 @@
 		};
 
 		apbx@80040000 {
+			pwm: pwm@80064000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&pwm4_pins_a>;
+				status = "okay";
+			};
+
 			duart: serial@80074000 {
 				pinctrl-names = "default";
 				pinctrl-0 = <&duart_pins_b>;
 				status = "okay";
 			};
+
+			i2c0: i2c@80058000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&i2c0_pins_b>;
+				status = "okay";
+
+				ssd1307: oled@3c {
+					compatible = "solomon,ssd1307fb-i2c";
+					reg = <0x3c>;
+					pwms = <&pwm 4 3000>;
+					oled-reset-gpios = <&gpio2 7 1>;
+					reset-active-low;
+				};
+			};
 		};
 	};
 
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH 3/4] ARM: dts: mxs: Add pwm4 muxing options for imx28
From: Thomas Petazzoni @ 2012-07-17 16:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1342540787-14930-4-git-send-email-maxime.ripard@free-electrons.com>

Le Tue, 17 Jul 2012 17:59:46 +0200,
Maxime Ripard <maxime.ripard@free-electrons.com> a écrit :

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Brian Lilly <brian@crystalfontz.com>
> ---
>  arch/arm/boot/dts/imx28.dtsi |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index 44ce1fd..0a565ef 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -461,6 +461,15 @@
>  					fsl,pull-up = <0>;
>  				};
>  
> +				pwm4_pins_a: pwm4@0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <
> +						0x31d0 /* MX28_PAD_PWM4__PWM_4 */

Unless my eyes are broken, it seems that the final >; is missing here.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH 1/4] video: Add support for the Solomon SSD1307 OLED Controller
From: Thomas Petazzoni @ 2012-07-17 16:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1342540787-14930-2-git-send-email-maxime.ripard@free-electrons.com>

Le Tue, 17 Jul 2012 17:59:44 +0200,
Maxime Ripard <maxime.ripard@free-electrons.com> a écrit :

> This patches adds support for the Solomon SSD1307 OLED

patches -> patch

> controller found on the Crystalfontz CFA10036 board.
> 
> It is a monochrome 128x39 display, that can operate over
> I2C or SPI.

The display is not 128x39, but rather the SSD1307 controller can drive
monochrome displays up to 128x39 pixels.

> The current driver as only be tested on the CFA-10036,

as -> has

> that is using this controller over I2C to driver a 96x16
> OLED screen.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Brian Lilly <brian@crystalfontz.com>

Ideally, it would be nice if this driver was more separated from the
display size (i.e the display size would be passed as DT properties,
for example), and that the driver would support different display
sizes. However, having an initial implementation that works for 96x16
is probably a good starting point.

> +Required properties:
> +  - compatible: Should be "solomon,ssd1307fb-<bus>". The only supported bus for
> +    now is i2c.
> +  - reg: Should contain address of the controller on the I2C bus. Most likely
> +         0x3c or 0x3d
> +  - pwm: Should contain the pwm to use according to the OF device tree PWM
> +         specification [0]
> +  - oled-reset-gpios: Should contain the GPIO used to reset the OLED display

Since there is only one gpio for reset, is is necessary to have a 's'
at the end of oled-reset-gpios?

> +Optional properties:
> +  - reset-active-low: Is the reset gpio is active on physical low?

Shouldn't this one be called oled-reset-active-low, for consistency
with the oled-reset-gpio property?

> +static int ssd1307fb_write_cmd_array(struct i2c_client *client, u8* cmd, u32 len)
> +{
> +	u8 *buf;
> +	int ret;

	int ret = 0;

> +
> +	buf = kzalloc(len + 1, GFP_KERNEL);
> +	if (!buf) {
> +		dev_err(&client->dev, "Couldn't allocate sending buffer.\n");
> +		ret = -ENOMEM;
> +		goto error;

		goto out;

> +	}
> +
> +	buf[0] = SSD1307FB_COMMAND;
> +	memcpy(buf + 1, cmd, len);
> +
> +	ret = i2c_master_send(client, buf, len + 1);
> +	if (ret != len + 1) {
> +		dev_err(&client->dev, "Couldn't send I2C command.\n");
> +		goto error;

		goto out;

> +	}
> +
> +	kfree(buf);
> +	return 0;

delete those lines.

> +
> +error:

   out:
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static inline int ssd1307fb_write_cmd(struct i2c_client *client, u8 cmd)
> +{
> +	return ssd1307fb_write_cmd_array(client, &cmd, 1);
> +}
> +
> +static int ssd1307fb_write_data_array(struct i2c_client *client, u8* cmd, u32 len)
> +{
> +	u8 *buf;
> +	int ret;
> +
> +	buf = kzalloc(len + 1, GFP_KERNEL);
> +	if (!buf) {
> +		dev_err(&client->dev, "Couldn't allocate sending buffer.\n");
> +		ret = -ENOMEM;
> +		goto error;
> +	}
> +
> +	buf[0] = SSD1307FB_DATA;
> +	memcpy(buf + 1, cmd, len);
> +
> +	ret = i2c_master_send(client, buf, len + 1);
> +	if (ret != len + 1) {
> +		dev_err(&client->dev, "Couldn't send I2C data.\n");
> +		goto error;
> +	}
> +
> +	kfree(buf);
> +	return 0;
> +
> +error:
> +	kfree(buf);
> +	return ret;
> +}

This looks highly duplicated from the write_cmd_array(). I'm sure some
factorization is possible here :)

> +static inline int ssd1307fb_write_data(struct i2c_client *client, u8 data)
> +{
> +	return ssd1307fb_write_data_array(client, &data, 1);
> +}
> +
> +static int ssd1307fb_set(struct i2c_client *client, u8 value)
> +{
> +	int i, j, ret;
> +
> +	for (i = 1; i <= (SSD1307FB_HEIGHT / 8); i++) {
> +		ret = ssd1307fb_write_cmd(client, SSD1307FB_START_PAGE_ADDRESS + i);
> +		if (ret)
> +			goto i2c_error;
> +
> +		ret = ssd1307fb_write_cmd(client, 0x00);
> +		if (ret)
> +			goto i2c_error;
> +
> +		ret = ssd1307fb_write_cmd(client, 0x10);
> +		if (ret)
> +			goto i2c_error;
> +
> +		for (j = 0; j < SSD1307FB_WIDTH; j++)
> +			ssd1307fb_write_data(client, value);
> +	}
> +
> +	return 0;
> +
> +i2c_error:
> +	dev_err(&client->dev, "Couldn't send i2c command: %d\n", ret);
> +	return ret;
> +}
> +
> +static void ssd1307fb_update_display(struct ssd1307fb_par *par)
> +{
> +	u8 *vmem = par->info->screen_base;
> +	int i, j, k;
> +
> +	/* 
> +	 * A page is 8 bit large and covers all the line. In order to

by "large", you mean the "height", so I wouldn't use the word "large",
which I think is typically associated with "width" rather than "height".

> +	 * update the screen, you have to send a byte, each byte
> +	 * containing a bit per pixel to set, on the same column. That
> +	 * way, by sending a byte, you will only portion of the screen

English issue.

> +	 * that has a height of 8 bit and a width of 1, the right-most
> +	 * bit being on the top of this frame. So we have to do a bit
> +	 * of magic here...

How about summarizing this by something like:

 The screen is divided in pages, each having a height of 8 pixels, and
 the width of the screen. When sending a byte of data to the
 controller, it gives the 8 bits for the current column. I.e, the first
 byte are the 8 bits of the first column, then the 8 bits for the
 second column, etc.

And even add some ASCII art to it:

 Representation of the screen, assuming it is 5 bits wide. Each
 letter-number combination is a bit that controls one pixel.

 A0 A1 A2 A3 A4
 B0 B1 B2 B3 B4
 C0 C1 C2 C3 C4
 D0 D1 D2 D3 D4
 E0 E1 E2 E3 E4
 F0 F1 F2 F3 F4
 G0 G1 G2 G3 G4
 H0 H1 H2 H3 H4

 If you want to update this screen, you need to send 5 bytes:
  (1) A0 B0 C0 D0 E0 F0 G0 H0
  (2) A1 B1 C1 D1 E1 F1 G1 H1
  (3) A2 B2 C2 D2 E2 F2 G2 H2
  (4) A3 B3 C3 D3 E3 F3 G3 H3
  (5) A4 B4 C4 D4 E4 F4 G4 H4

> +	for (i = 0; i < (SSD1307FB_HEIGHT / 8); i++) {
> +		ssd1307fb_write_cmd(par->client, SSD1307FB_START_PAGE_ADDRESS + (i + 1));
> +		ssd1307fb_write_cmd(par->client, 0x00);
> +		ssd1307fb_write_cmd(par->client, 0x10);
> +
> +		for (j = 0; j < SSD1307FB_WIDTH; j++) {
> +			u8 buf = 0;
> +			for (k = 0; k < 8; k++) {
> +				u32 page_length = SSD1307FB_WIDTH * i;
> +				u32 index = page_length + (SSD1307FB_WIDTH * k + j) / 8;
> +				u8 byte = *(vmem + index);
> +				u8 bit = byte & (1 << (7 - (j % 8)));
> +				bit = bit >> (7 - (j % 8));
> +				buf |= bit << k;
> +			}
> +			ssd1307fb_write_data(par->client, buf);
> +		}
> +	}
> +
> +	return;
> +}
> +
> +
> +static ssize_t ssd1307fb_write(struct fb_info *info, const char __user *buf,
> +		size_t count, loff_t *ppos)
> +{
> +	struct ssd1307fb_par *par = info->par;
> +	unsigned long p = *ppos;
> +	void *dst;
> +	int err = 0;
> +
> +	dst = (void __force *) (info->screen_base + p);
> +
> +	if (copy_from_user(dst, buf, count))
> +		err = -EFAULT;
> +
> +	if  (!err)
> +		*ppos += count;
> +
> +	ssd1307fb_update_display(par);
> +
> +	return (err) ? err : count;
> +}
> +
> +static void ssd1307fb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
> +{
> +	struct ssd1307fb_par *par = info->par;
> +	sys_fillrect(info, rect);
> +	ssd1307fb_update_display(par);
> +}
> +
> +static void ssd1307fb_copyarea(struct fb_info *info, const struct fb_copyarea *area) 
> +{
> +	struct ssd1307fb_par *par = info->par;
> +	sys_copyarea(info, area);
> +	ssd1307fb_update_display(par);
> +}
> +
> +static void ssd1307fb_imageblit(struct fb_info *info, const struct fb_image *image) 
> +{
> +	struct ssd1307fb_par *par = info->par;
> +	sys_imageblit(info, image);
> +	ssd1307fb_update_display(par);
> +}
> +
> +static struct fb_ops ssd1307fb_ops = {
> +	.owner		= THIS_MODULE,
> +	.fb_read	= fb_sys_read,
> +	.fb_write	= ssd1307fb_write,
> +	.fb_fillrect	= ssd1307fb_fillrect,
> +	.fb_copyarea	= ssd1307fb_copyarea,
> +	.fb_imageblit	= ssd1307fb_imageblit,
> +};
> +
> +static void ssd1307fb_deferred_io(struct fb_info *info,
> +				struct list_head *pagelist)
> +{
> +	ssd1307fb_update_display(info->par);
> +}
> +
> +static struct fb_deferred_io ssd1307fb_defio = {
> +	.delay		= HZ,
> +	.deferred_io	= ssd1307fb_deferred_io,
> +};
> +
> +static int __devinit ssd1307fb_probe(struct i2c_client *client, const struct i2c_device_id *id)
> +{
> +	struct fb_info *info;
> +	u32 vmem_size = SSD1307FB_WIDTH * SSD1307FB_HEIGHT / 8;
> +	struct ssd1307fb_par *par;
> +	u8 *vmem;
> +	int ret;
> +
> +	if (!client->dev.of_node) {
> +		dev_err(&client->dev, "No device tree data found!\n");
> +		ret = -EINVAL;
> +		goto generic_error;
> +	}
> +
> +	info = framebuffer_alloc(sizeof(struct ssd1307fb_par), &client->dev);
> +	if (!info) {
> +		ret = -ENOMEM;
> +		goto generic_error;
> +	}
> +
> +	vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);

Error checking?

> +	info->fbops = &ssd1307fb_ops;
> +	info->fix = ssd1307fb_fix;
> +	info->fbdefio = &ssd1307fb_defio;
> +
> +	info->var = ssd1307fb_var;
> +	info->var.red.length = 1;
> +	info->var.red.offset = 0;
> +	info->var.green.length = 1;
> +	info->var.green.offset = 0;
> +	info->var.blue.length = 1;
> +	info->var.blue.offset = 0;
> +
> +	info->screen_base = (u8 __force __iomem *)vmem;
> +	info->fix.smem_len = vmem_size;
> +
> +	fb_deferred_io_init(info);
> +
> +	par = info->par;
> +	par->info = info;
> +	par->client = client;
> +
> +	par->reset = of_get_named_gpio(client->dev.of_node,
> +					 "oled-reset-gpios", 0);
> +	if (gpio_is_valid(par->reset)) {
> +		int flags = GPIOF_OUT_INIT_HIGH;
> +		if (of_get_property(client->dev.of_node,
> +				    "reset-active-low", NULL))
> +			flags = GPIOF_OUT_INIT_LOW;
> +		ret = devm_gpio_request_one(&client->dev, par->reset,
> +					    flags, "oled-reset");
> +		if (ret) {
> +			dev_err(&client->dev,
> +				"failed to request gpio %d: %d\n",
> +				par->reset, ret);
> +			goto reset_oled_error;
> +		}
> +	}
> +
> +	par->pwm = pwm_get(&client->dev, NULL);
> +	if (IS_ERR(par->pwm)) {
> +		dev_err(&client->dev, "Could not get PWM from device tree!\n");
> +		ret = PTR_ERR(par->pwm);
> +		goto pwm_error;
> +	}
> +
> +	par->pwm_period = pwm_get_period(par->pwm);
> +
> +	dev_dbg(&client->dev, "Using PWM%d with a %dns period.\n", par->pwm->pwm, par->pwm_period);
> +
> +	ret = register_framebuffer(info);
> +	if (ret) {
> +		dev_err(&client->dev, "Couldn't register the framebuffer\n");
> +		goto fbreg_error;
> +	}
> +
> +	i2c_set_clientdata(client, info);
> +
> +	/* Reset the screen */
> +	gpio_set_value(par->reset, 1);
> +	udelay(4);
> +	gpio_set_value(par->reset, 0);
> +	udelay(4);
> +
> +	/* Enable the PWM */
> +	pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
> +	pwm_enable(par->pwm);
> +
> +	/* Map column 127 of the OLED to segment 0 */
> +	ret = ssd1307fb_write_cmd(client, SSD1307FB_SEG_REMAP_ON);
> +	if (ret) {
> +		dev_err(&client->dev, "Couldn't remap the screen.\n");
> +		goto remap_error;
> +	}
> +
> +	/* Turn on the display */
> +	ret = ssd1307fb_write_cmd(client, SSD1307FB_DISPLAY_ON);
> +	if (ret) {
> +		dev_err(&client->dev, "Couldn't turn the display on.\n");
> +		goto remap_error;
> +	}
> +
> +	dev_info(&client->dev, "fb%d: %s framebuffer device registered, using %d bytes of video memory\n", info->node, info->fix.id, vmem_size);
> +
> +	return 0;
> +
> +remap_error:
> +	unregister_framebuffer(info);
> +	pwm_disable(par->pwm);
> +fbreg_error:
> +	pwm_put(par->pwm);
> +pwm_error:
> +reset_oled_error:
> +	fb_deferred_io_cleanup(info);
> +	framebuffer_release(info);
> +generic_error:
> +	return ret;
> +}
> +
> +static int __devexit ssd1307fb_remove(struct i2c_client *client)
> +{
> +	struct fb_info *info = i2c_get_clientdata(client);
> +	struct ssd1307fb_par *par = info->par;
> +	unregister_framebuffer(info);
> +	pwm_disable(par->pwm);
> +	pwm_put(par->pwm);
> +	fb_deferred_io_cleanup(info);
> +	framebuffer_release(info);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id ssd1307fb_i2c_id[] = {
> +	{ "ssd1307fb", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ssd1307fb_i2c_id);
> +
> +static const struct of_device_id ssd1307fb_of_match[] = {
> +	{ .compatible = "solomon,ssd1307fb-i2c" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ssd1307fb_of_match);
> +
> +static struct i2c_driver ssd1307fb_driver = {
> +	.probe = ssd1307fb_probe,
> +	.remove = __devexit_p(ssd1307fb_remove),
> +	.id_table = ssd1307fb_i2c_id,
> +	.driver = {
> +		.name = "ssd1307fb",
> +		.of_match_table = of_match_ptr(ssd1307fb_of_match),
> +		.owner = THIS_MODULE,
> +	},
> +};
> +
> +module_i2c_driver(ssd1307fb_driver);
> +
> +MODULE_DESCRIPTION("FB driver for the Solomon SSD1307 OLED controler");
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> +MODULE_LICENSE("GPL");



-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* Re: dma-buf/fbdev: one-to-many support
From: Laurent Pinchart @ 2012-07-18  0:41 UTC (permalink / raw)
  To: David Herrmann; +Cc: dri-devel, linux-fbdev, Sumit Semwal, linux-media
In-Reply-To: <CANq1E4TMddx-+h5cGAn=R2VoNBQ274CBZDGV02Ku2Hgo_Hc3iA@mail.gmail.com>

Hi David,

On Tuesday 17 July 2012 14:23:18 David Herrmann wrote:
> On Tue, Jul 17, 2012 at 1:24 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> >> The main issue is that fbdev has been designed with the implicit
> >> assumption that an fbdev driver will always own the graphics memory it
> >> uses. All components in the stack, from drivers to applications, have
> >> been designed around that assumption.
> >> 
> >> We could of course fix this, revamp the fbdev API and turn it into a
> >> modern graphics API, but I really wonder whether it would be worth it.
> >> DRM has been getting quite a lot of attention lately, especially from
> >> embedded developers and vendors, and the trend seems to me like the
> >> (Linux) world will gradually move from fbdev to DRM.
> >> 
> >> Please feel free to disagree :-)
> > 
> > I would disagree on the "main issue" bit. All the graphics cards have
> > their own formats and cache management rules. Simply sharing a buffer
> > doesn't work - which is why all of the extra gloop will be needed.
> 
> This is exactly why I suggested adding an "owner" field. A driver
> could then check whether the buffer it is supposed to share/takeover
> is from a compatible (or even the same) driver/device. If it is not,
> it would simply reject using the buffer. Then again, if we have
> multiple devices that are incompatible, we are still unable to share
> the buffer. So this attempt would only be useful if we have tons of
> DisplayLink devices attached that all use the same driver, for
> example.
> 
> Regarding DRM: In user-space I prefer DRM over fbdev. With the
> introduction of the dumb-buffers there isn't even the need to have
> mesa installed. However, fblog runs in kernel space and currently
> cannot use DRM as there is no in-kernel DRM API. I looked at
> drm-fops.c whether it is easy to create a very simple in-kernel API
> but then I dropped the idea as this might be too complex for a simple
> debugging-only driver. Another attempt would be making the
> drm-fb-helper more generic so we can use this layer as in-kernel DRM
> API.
> 
> I had a deeper look into this this weekend and so as a summary I think
> all in-kernel graphics access is probably not worth optimizing it.
> fbcon is already working great and fblog is only used during boot and
> oopses/panics and can be restricted to a single device. I will have
> another look at the drivers in a few weeks but if you tell me that
> this is not easy to implement, I will probably have to let this idea
> go.

My gut feeling is that, given the effort required to add new APIs, it would be 
more interesting to work on an in-kernel DRM API to make drmcon and drmlog 
implementations possible without any fbdev compatibility layer. That's rather 
a long term goal though.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* [PATCH v2] video: da8xx-fb: add 24bpp LCD configuration support
From: Manjunathappa, Prakash @ 2012-07-18  5:59 UTC (permalink / raw)
  To: linux-fbdev

LCD controller on am335x supports 24bpp raster configuration in addition
to ones on da850. LCDC also supports 24bpp in unpacked format having
ARGB:8888 32bpp format data in DDR, but it doesn't interpret alpha
component of the data.

Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
Cc: Anatolij Gustschin <agust@denx.de>
---
Since v1:
Addressed Tobias's review comments on calculation of pseudopalette for
FB_VISUAL_TRUECOLOR type.

 drivers/video/da8xx-fb.c |   88 ++++++++++++++++++++++++++++-----------------
 1 files changed, 55 insertions(+), 33 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 47118c7..3cda461 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -153,7 +153,7 @@ struct da8xx_fb_par {
 	unsigned int		dma_end;
 	struct clk *lcdc_clk;
 	int irq;
-	unsigned short pseudo_palette[16];
+	u32 pseudo_palette[16];
 	unsigned int palette_sz;
 	unsigned int pxl_clk;
 	int blank;
@@ -546,6 +546,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
 	return 0;
 }
 
+
+#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
 static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
 			      unsigned blue, unsigned transp,
 			      struct fb_info *info)
@@ -561,13 +563,33 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
 	if (info->fix.visual = FB_VISUAL_DIRECTCOLOR)
 		return 1;
 
-	if (info->var.bits_per_pixel = 4) {
-		if (regno > 15)
-			return 1;
+	switch (info->fix.visual) {
+	case FB_VISUAL_TRUECOLOR:
+		red = CNVT_TOHW(red, info->var.red.length);
+		green = CNVT_TOHW(green, info->var.green.length);
+		blue = CNVT_TOHW(blue, info->var.blue.length);
+		break;
+	case FB_VISUAL_PSEUDOCOLOR:
+		if (info->var.bits_per_pixel = 4) {
+			if (regno > 15)
+				return 1;
+
+			if (info->var.grayscale) {
+				pal = regno;
+			} else {
+				red >>= 4;
+				green >>= 8;
+				blue >>= 12;
+
+				pal = (red & 0x0f00);
+				pal |= (green & 0x00f0);
+				pal |= (blue & 0x000f);
+			}
+			if (regno = 0)
+				pal |= 0x2000;
+			palette[regno] = pal;
 
-		if (info->var.grayscale) {
-			pal = regno;
-		} else {
+		} else if (info->var.bits_per_pixel = 8) {
 			red >>= 4;
 			green >>= 8;
 			blue >>= 12;
@@ -575,36 +597,35 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
 			pal = (red & 0x0f00);
 			pal |= (green & 0x00f0);
 			pal |= (blue & 0x000f);
-		}
-		if (regno = 0)
-			pal |= 0x2000;
-		palette[regno] = pal;
-
-	} else if (info->var.bits_per_pixel = 8) {
-		red >>= 4;
-		green >>= 8;
-		blue >>= 12;
-
-		pal = (red & 0x0f00);
-		pal |= (green & 0x00f0);
-		pal |= (blue & 0x000f);
 
-		if (palette[regno] != pal) {
-			update_hw = 1;
-			palette[regno] = pal;
+			if (palette[regno] != pal) {
+				update_hw = 1;
+				palette[regno] = pal;
+			}
 		}
-	} else if ((info->var.bits_per_pixel = 16) && regno < 16) {
-		red >>= (16 - info->var.red.length);
-		red <<= info->var.red.offset;
+		break;
+	}
 
-		green >>= (16 - info->var.green.length);
-		green <<= info->var.green.offset;
+	/* Truecolor has hardware independent palette */
+	if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
+		u32 v;
 
-		blue >>= (16 - info->var.blue.length);
-		blue <<= info->var.blue.offset;
+		if (regno > 15)
+			return -EINVAL;
 
-		par->pseudo_palette[regno] = red | green | blue;
+		v = (red << info->var.red.offset) |
+			(green << info->var.green.offset) |
+			(blue << info->var.blue.offset);
 
+		switch (info->var.bits_per_pixel) {
+		case 16:
+			((u16 *) (info->pseudo_palette))[regno] = v;
+			break;
+		case 24:
+		case 32:
+			((u32 *) (info->pseudo_palette))[regno] = v;
+			break;
+		}
 		if (palette[0] != 0x4000) {
 			update_hw = 1;
 			palette[0] = 0x4000;
@@ -617,6 +638,7 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
 
 	return 0;
 }
+#undef CNVT_TOHW
 
 static void lcd_reset(struct da8xx_fb_par *par)
 {
-- 
1.7.1


^ permalink raw reply related

* RE: [PATCH v2] video: da8xx-fb: add 24bpp LCD configuration support
From: Hiremath, Vaibhav @ 2012-07-18  6:11 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1342590441-8441-1-git-send-email-prakash.pm@ti.com>

On Wed, Jul 18, 2012 at 11:17:21, Manjunathappa, Prakash wrote:
> LCD controller on am335x supports 24bpp raster configuration in addition
> to ones on da850. LCDC also supports 24bpp in unpacked format having
> ARGB:8888 32bpp format data in DDR, but it doesn't interpret alpha
> component of the data.
> 
> Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> Cc: Anatolij Gustschin <agust@denx.de>
> ---
> Since v1:
> Addressed Tobias's review comments on calculation of pseudopalette for
> FB_VISUAL_TRUECOLOR type.
> 
>  drivers/video/da8xx-fb.c |   88 ++++++++++++++++++++++++++++-----------------
>  1 files changed, 55 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 47118c7..3cda461 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -153,7 +153,7 @@ struct da8xx_fb_par {
>  	unsigned int		dma_end;
>  	struct clk *lcdc_clk;
>  	int irq;
> -	unsigned short pseudo_palette[16];
> +	u32 pseudo_palette[16];

Personally I don't like, mix of "u32" and "unsigned int" declaration.

>  	unsigned int palette_sz;
>  	unsigned int pxl_clk;
>  	int blank;
> @@ -546,6 +546,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
>  	return 0;
>  }
>  
> +
> +#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)

Did you run checkpatch.pl on this patch?

>  static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
>  			      unsigned blue, unsigned transp,
>  			      struct fb_info *info)
> @@ -561,13 +563,33 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
>  	if (info->fix.visual = FB_VISUAL_DIRECTCOLOR)
>  		return 1;
>  
> -	if (info->var.bits_per_pixel = 4) {
> -		if (regno > 15)
> -			return 1;
> +	switch (info->fix.visual) {
> +	case FB_VISUAL_TRUECOLOR:
> +		red = CNVT_TOHW(red, info->var.red.length);
> +		green = CNVT_TOHW(green, info->var.green.length);
> +		blue = CNVT_TOHW(blue, info->var.blue.length);

Why not add another underscore after TO? define like this => CNVT_TO_HW

> +		break;
> +	case FB_VISUAL_PSEUDOCOLOR:
> +		if (info->var.bits_per_pixel = 4) {
> +			if (regno > 15)
> +				return 1;
> +
> +			if (info->var.grayscale) {
> +				pal = regno;
> +			} else {
> +				red >>= 4;
> +				green >>= 8;
> +				blue >>= 12;
> +
> +				pal = (red & 0x0f00);
> +				pal |= (green & 0x00f0);
> +				pal |= (blue & 0x000f);
> +			}
> +			if (regno = 0)
> +				pal |= 0x2000;
> +			palette[regno] = pal;
>  
> -		if (info->var.grayscale) {
> -			pal = regno;
> -		} else {
> +		} else if (info->var.bits_per_pixel = 8) {
>  			red >>= 4;
>  			green >>= 8;
>  			blue >>= 12;
> @@ -575,36 +597,35 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
>  			pal = (red & 0x0f00);
>  			pal |= (green & 0x00f0);
>  			pal |= (blue & 0x000f);
> -		}
> -		if (regno = 0)
> -			pal |= 0x2000;
> -		palette[regno] = pal;
> -
> -	} else if (info->var.bits_per_pixel = 8) {
> -		red >>= 4;
> -		green >>= 8;
> -		blue >>= 12;
> -
> -		pal = (red & 0x0f00);
> -		pal |= (green & 0x00f0);
> -		pal |= (blue & 0x000f);
>  
> -		if (palette[regno] != pal) {
> -			update_hw = 1;
> -			palette[regno] = pal;
> +			if (palette[regno] != pal) {
> +				update_hw = 1;
> +				palette[regno] = pal;
> +			}
>  		}
> -	} else if ((info->var.bits_per_pixel = 16) && regno < 16) {
> -		red >>= (16 - info->var.red.length);
> -		red <<= info->var.red.offset;
> +		break;
> +	}
>  
> -		green >>= (16 - info->var.green.length);
> -		green <<= info->var.green.offset;
> +	/* Truecolor has hardware independent palette */
> +	if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
> +		u32 v;
>  
> -		blue >>= (16 - info->var.blue.length);
> -		blue <<= info->var.blue.offset;
> +		if (regno > 15)
> +			return -EINVAL;
>  
> -		par->pseudo_palette[regno] = red | green | blue;
> +		v = (red << info->var.red.offset) |
> +			(green << info->var.green.offset) |
> +			(blue << info->var.blue.offset);
>  
> +		switch (info->var.bits_per_pixel) {
> +		case 16:
> +			((u16 *) (info->pseudo_palette))[regno] = v;
> +			break;
> +		case 24:
> +		case 32:
> +			((u32 *) (info->pseudo_palette))[regno] = v;
> +			break;
> +		}
>  		if (palette[0] != 0x4000) {
>  			update_hw = 1;
>  			palette[0] = 0x4000;
> @@ -617,6 +638,7 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
>  
>  	return 0;
>  }
> +#undef CNVT_TOHW
>  

Is this required?

Thanks,
Vaibhav

>  static void lcd_reset(struct da8xx_fb_par *par)
>  {
> -- 
> 1.7.1
> 
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source@linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
> 


^ permalink raw reply

* RE: [PATCH v2] video: da8xx-fb: add 24bpp LCD configuration support
From: Manjunathappa, Prakash @ 2012-07-18  6:49 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1342590441-8441-1-git-send-email-prakash.pm@ti.com>

Hi Vaibhav,

On Wed, Jul 18, 2012 at 11:41:43, Hiremath, Vaibhav wrote:
> On Wed, Jul 18, 2012 at 11:17:21, Manjunathappa, Prakash wrote:
> > LCD controller on am335x supports 24bpp raster configuration in addition
> > to ones on da850. LCDC also supports 24bpp in unpacked format having
> > ARGB:8888 32bpp format data in DDR, but it doesn't interpret alpha
> > component of the data.
> > 
> > Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> > Cc: Anatolij Gustschin <agust@denx.de>
> > ---
> > Since v1:
> > Addressed Tobias's review comments on calculation of pseudopalette for
> > FB_VISUAL_TRUECOLOR type.
> > 
> >  drivers/video/da8xx-fb.c |   88 ++++++++++++++++++++++++++++-----------------
> >  1 files changed, 55 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> > index 47118c7..3cda461 100644
> > --- a/drivers/video/da8xx-fb.c
> > +++ b/drivers/video/da8xx-fb.c
> > @@ -153,7 +153,7 @@ struct da8xx_fb_par {
> >  	unsigned int		dma_end;
> >  	struct clk *lcdc_clk;
> >  	int irq;
> > -	unsigned short pseudo_palette[16];
> > +	u32 pseudo_palette[16];
> 
> Personally I don't like, mix of "u32" and "unsigned int" declaration.
> 

"unsigned int" is not guaranteed to be 32bit, may be "unsigned long" is
better choice.

> >  	unsigned int palette_sz;
> >  	unsigned int pxl_clk;
> >  	int blank;
> > @@ -546,6 +546,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
> >  	return 0;
> >  }
> >  
> > +
> > +#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
> 
> Did you run checkpatch.pl on this patch?
> 

Yes, checkpatch.pl did not complain anything on this line. I will add space
around binary operators.

> >  static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> >  			      unsigned blue, unsigned transp,
> >  			      struct fb_info *info)
> > @@ -561,13 +563,33 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> >  	if (info->fix.visual = FB_VISUAL_DIRECTCOLOR)
> >  		return 1;
> >  
> > -	if (info->var.bits_per_pixel = 4) {
> > -		if (regno > 15)
> > -			return 1;
> > +	switch (info->fix.visual) {
> > +	case FB_VISUAL_TRUECOLOR:
> > +		red = CNVT_TOHW(red, info->var.red.length);
> > +		green = CNVT_TOHW(green, info->var.green.length);
> > +		blue = CNVT_TOHW(blue, info->var.blue.length);
> 
> Why not add another underscore after TO? define like this => CNVT_TO_HW
> 

I referred drivers/video/skeletonfb.c, I assume it is standard. Please let me know if not.

> > +		break;
> > +	case FB_VISUAL_PSEUDOCOLOR:
> > +		if (info->var.bits_per_pixel = 4) {
> > +			if (regno > 15)
> > +				return 1;
> > +
> > +			if (info->var.grayscale) {
> > +				pal = regno;
> > +			} else {
> > +				red >>= 4;
> > +				green >>= 8;
> > +				blue >>= 12;
> > +
> > +				pal = (red & 0x0f00);
> > +				pal |= (green & 0x00f0);
> > +				pal |= (blue & 0x000f);
> > +			}
> > +			if (regno = 0)
> > +				pal |= 0x2000;
> > +			palette[regno] = pal;
> >  
> > -		if (info->var.grayscale) {
> > -			pal = regno;
> > -		} else {
> > +		} else if (info->var.bits_per_pixel = 8) {
> >  			red >>= 4;
> >  			green >>= 8;
> >  			blue >>= 12;
> > @@ -575,36 +597,35 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> >  			pal = (red & 0x0f00);
> >  			pal |= (green & 0x00f0);
> >  			pal |= (blue & 0x000f);
> > -		}
> > -		if (regno = 0)
> > -			pal |= 0x2000;
> > -		palette[regno] = pal;
> > -
> > -	} else if (info->var.bits_per_pixel = 8) {
> > -		red >>= 4;
> > -		green >>= 8;
> > -		blue >>= 12;
> > -
> > -		pal = (red & 0x0f00);
> > -		pal |= (green & 0x00f0);
> > -		pal |= (blue & 0x000f);
> >  
> > -		if (palette[regno] != pal) {
> > -			update_hw = 1;
> > -			palette[regno] = pal;
> > +			if (palette[regno] != pal) {
> > +				update_hw = 1;
> > +				palette[regno] = pal;
> > +			}
> >  		}
> > -	} else if ((info->var.bits_per_pixel = 16) && regno < 16) {
> > -		red >>= (16 - info->var.red.length);
> > -		red <<= info->var.red.offset;
> > +		break;
> > +	}
> >  
> > -		green >>= (16 - info->var.green.length);
> > -		green <<= info->var.green.offset;
> > +	/* Truecolor has hardware independent palette */
> > +	if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
> > +		u32 v;
> >  
> > -		blue >>= (16 - info->var.blue.length);
> > -		blue <<= info->var.blue.offset;
> > +		if (regno > 15)
> > +			return -EINVAL;
> >  
> > -		par->pseudo_palette[regno] = red | green | blue;
> > +		v = (red << info->var.red.offset) |
> > +			(green << info->var.green.offset) |
> > +			(blue << info->var.blue.offset);
> >  
> > +		switch (info->var.bits_per_pixel) {
> > +		case 16:
> > +			((u16 *) (info->pseudo_palette))[regno] = v;
> > +			break;
> > +		case 24:
> > +		case 32:
> > +			((u32 *) (info->pseudo_palette))[regno] = v;
> > +			break;
> > +		}
> >  		if (palette[0] != 0x4000) {
> >  			update_hw = 1;
> >  			palette[0] = 0x4000;
> > @@ -617,6 +638,7 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> >  
> >  	return 0;
> >  }
> > +#undef CNVT_TOHW
> >  
> 
> Is this required?
> 

Not required really, again thought it as standard since 8 out of 10 drivers does it.

Thanks,
Prakash

^ permalink raw reply

* Re: [PATCH v2] video: da8xx-fb: add 24bpp LCD configuration support
From: Peter Korsgaard @ 2012-07-18  6:58 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1342590441-8441-1-git-send-email-prakash.pm@ti.com>

>>>>> "Prakash" = Manjunathappa, Prakash <prakash.pm@ti.com> writes:

Hi,

 >> Personally I don't like, mix of "u32" and "unsigned int" declaration.
 >> 

 Prakash> "unsigned int" is not guaranteed to be 32bit, may be "unsigned
 Prakash> long" is better choice.

On the platforms where da8xx-fb is used it is.

 >> >  	unsigned int palette_sz;
 >> >  	unsigned int pxl_clk;
 >> >  	int blank;
 >> > @@ -546,6 +546,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
 >> >  	return 0;
 >> >  }
 >> >  
 >> > +
 >> > +#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
 >> 
 >> Did you run checkpatch.pl on this patch?
 >> 

 Prakash> Yes, checkpatch.pl did not complain anything on this line. I
 Prakash> will add space around binary operators.

An inline function would be nicer.

-- 
Bye, Peter Korsgaard

^ permalink raw reply

* RE: [PATCH v2] video: da8xx-fb: add 24bpp LCD configuration support
From: Hiremath, Vaibhav @ 2012-07-18  7:00 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1342590441-8441-1-git-send-email-prakash.pm@ti.com>

On Wed, Jul 18, 2012 at 12:19:45, Manjunathappa, Prakash wrote:
> Hi Vaibhav,
> 
> On Wed, Jul 18, 2012 at 11:41:43, Hiremath, Vaibhav wrote:
> > On Wed, Jul 18, 2012 at 11:17:21, Manjunathappa, Prakash wrote:
> > > LCD controller on am335x supports 24bpp raster configuration in addition
> > > to ones on da850. LCDC also supports 24bpp in unpacked format having
> > > ARGB:8888 32bpp format data in DDR, but it doesn't interpret alpha
> > > component of the data.
> > > 
> > > Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> > > Cc: Anatolij Gustschin <agust@denx.de>
> > > ---
> > > Since v1:
> > > Addressed Tobias's review comments on calculation of pseudopalette for
> > > FB_VISUAL_TRUECOLOR type.
> > > 
> > >  drivers/video/da8xx-fb.c |   88 ++++++++++++++++++++++++++++-----------------
> > >  1 files changed, 55 insertions(+), 33 deletions(-)
> > > 
> > > diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> > > index 47118c7..3cda461 100644
> > > --- a/drivers/video/da8xx-fb.c
> > > +++ b/drivers/video/da8xx-fb.c
> > > @@ -153,7 +153,7 @@ struct da8xx_fb_par {
> > >  	unsigned int		dma_end;
> > >  	struct clk *lcdc_clk;
> > >  	int irq;
> > > -	unsigned short pseudo_palette[16];
> > > +	u32 pseudo_palette[16];
> > 
> > Personally I don't like, mix of "u32" and "unsigned int" declaration.
> > 
> 
> "unsigned int" is not guaranteed to be 32bit, may be "unsigned long" is
> better choice.
> 
> > >  	unsigned int palette_sz;
> > >  	unsigned int pxl_clk;
> > >  	int blank;
> > > @@ -546,6 +546,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
> > >  	return 0;
> > >  }
> > >  
> > > +
> > > +#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
> > 
> > Did you run checkpatch.pl on this patch?
> > 
> 
> Yes, checkpatch.pl did not complain anything on this line. I will add space
> around binary operators.
> 

You can choose not to do this change, since checkpatch is ok and other 
drivers also does same thing.


> > >  static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> > >  			      unsigned blue, unsigned transp,
> > >  			      struct fb_info *info)
> > > @@ -561,13 +563,33 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> > >  	if (info->fix.visual = FB_VISUAL_DIRECTCOLOR)
> > >  		return 1;
> > >  
> > > -	if (info->var.bits_per_pixel = 4) {
> > > -		if (regno > 15)
> > > -			return 1;
> > > +	switch (info->fix.visual) {
> > > +	case FB_VISUAL_TRUECOLOR:
> > > +		red = CNVT_TOHW(red, info->var.red.length);
> > > +		green = CNVT_TOHW(green, info->var.green.length);
> > > +		blue = CNVT_TOHW(blue, info->var.blue.length);
> > 
> > Why not add another underscore after TO? define like this => CNVT_TO_HW
> > 
> 
> I referred drivers/video/skeletonfb.c, I assume it is standard. Please let me know if not.

I did grep on drivers/video/ and could see lots of drivers use it (almost 
same definition).
Ignore this comment as well, I could have recommended to move it to common 
file and do some cleanup, but not sure about background on this macro.

Thanks,
Vaibhav

> 
> > > +		break;
> > > +	case FB_VISUAL_PSEUDOCOLOR:
> > > +		if (info->var.bits_per_pixel = 4) {
> > > +			if (regno > 15)
> > > +				return 1;
> > > +
> > > +			if (info->var.grayscale) {
> > > +				pal = regno;
> > > +			} else {
> > > +				red >>= 4;
> > > +				green >>= 8;
> > > +				blue >>= 12;
> > > +
> > > +				pal = (red & 0x0f00);
> > > +				pal |= (green & 0x00f0);
> > > +				pal |= (blue & 0x000f);
> > > +			}
> > > +			if (regno = 0)
> > > +				pal |= 0x2000;
> > > +			palette[regno] = pal;
> > >  
> > > -		if (info->var.grayscale) {
> > > -			pal = regno;
> > > -		} else {
> > > +		} else if (info->var.bits_per_pixel = 8) {
> > >  			red >>= 4;
> > >  			green >>= 8;
> > >  			blue >>= 12;
> > > @@ -575,36 +597,35 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> > >  			pal = (red & 0x0f00);
> > >  			pal |= (green & 0x00f0);
> > >  			pal |= (blue & 0x000f);
> > > -		}
> > > -		if (regno = 0)
> > > -			pal |= 0x2000;
> > > -		palette[regno] = pal;
> > > -
> > > -	} else if (info->var.bits_per_pixel = 8) {
> > > -		red >>= 4;
> > > -		green >>= 8;
> > > -		blue >>= 12;
> > > -
> > > -		pal = (red & 0x0f00);
> > > -		pal |= (green & 0x00f0);
> > > -		pal |= (blue & 0x000f);
> > >  
> > > -		if (palette[regno] != pal) {
> > > -			update_hw = 1;
> > > -			palette[regno] = pal;
> > > +			if (palette[regno] != pal) {
> > > +				update_hw = 1;
> > > +				palette[regno] = pal;
> > > +			}
> > >  		}
> > > -	} else if ((info->var.bits_per_pixel = 16) && regno < 16) {
> > > -		red >>= (16 - info->var.red.length);
> > > -		red <<= info->var.red.offset;
> > > +		break;
> > > +	}
> > >  
> > > -		green >>= (16 - info->var.green.length);
> > > -		green <<= info->var.green.offset;
> > > +	/* Truecolor has hardware independent palette */
> > > +	if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
> > > +		u32 v;
> > >  
> > > -		blue >>= (16 - info->var.blue.length);
> > > -		blue <<= info->var.blue.offset;
> > > +		if (regno > 15)
> > > +			return -EINVAL;
> > >  
> > > -		par->pseudo_palette[regno] = red | green | blue;
> > > +		v = (red << info->var.red.offset) |
> > > +			(green << info->var.green.offset) |
> > > +			(blue << info->var.blue.offset);
> > >  
> > > +		switch (info->var.bits_per_pixel) {
> > > +		case 16:
> > > +			((u16 *) (info->pseudo_palette))[regno] = v;
> > > +			break;
> > > +		case 24:
> > > +		case 32:
> > > +			((u32 *) (info->pseudo_palette))[regno] = v;
> > > +			break;
> > > +		}
> > >  		if (palette[0] != 0x4000) {
> > >  			update_hw = 1;
> > >  			palette[0] = 0x4000;
> > > @@ -617,6 +638,7 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> > >  
> > >  	return 0;
> > >  }
> > > +#undef CNVT_TOHW
> > >  
> > 
> > Is this required?
> > 
> 
> Not required really, again thought it as standard since 8 out of 10 drivers does it.
> 
> Thanks,
> Prakash
> 


^ permalink raw reply

* Re: [BUGFIX] video/mxsfb: fix crash when unblanking the display
From: Robin van der Gracht @ 2012-07-18  9:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20209.35406.958883.977664@ipc1.ka-ro>

Hi Lothar,
> Hi,
>
> Lothar Waßmann writes:
>> The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature.
>> The write in mxsfb_disable_controller() sets the data_cnt for the LCD
>> DMA to 0 which obviously means the max. count for the LCD DMA and
>> leads to overwriting arbitrary memory when the display is unblanked.
>>
>> Signed-off-by: Lothar Waßmann<LW@KARO-electronics.de>
>> ---
>>   drivers/video/mxsfb.c |    3 ++-
>>   1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
>> index d837d63..03cb95a 100644
>> --- a/drivers/video/mxsfb.c
>> +++ b/drivers/video/mxsfb.c
>> @@ -366,7 +366,8 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
>>   		loop--;
>>   	}
>>
>> -	writel(VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4 + REG_CLR);
>> +	reg = readl(host->base + LCDC_VDCTRL4);
>> +	writel(reg&  ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
>>
>>   	clk_disable(host->clk);
>>
>> -- 
>> 1.5.6.5
>>
> Ping. Any comments on this?
>
>
> Lothar Waßmann
I've encountered this problem to, and i can confirm your patch fixed it.
The VDCTRL4 register has no CLR feature.

Regards,

-- 
Robin van der Gracht
Protonic Holland.
tel.: +31 (0) 229 212928
fax.: +31 (0) 229 210930
Factorij 36 / 1689 AL Zwaag


^ permalink raw reply

* RE: [PATCH v2] video: da8xx-fb: add 24bpp LCD configuration support
From: Manjunathappa, Prakash @ 2012-07-18  9:44 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1342590441-8441-1-git-send-email-prakash.pm@ti.com>

Hi Peter Korsgaard,

On Wed, Jul 18, 2012 at 12:28:21, Peter Korsgaard wrote:
> >>>>> "Prakash" = Manjunathappa, Prakash <prakash.pm@ti.com> writes:
> 
> Hi,
> 
>  >> Personally I don't like, mix of "u32" and "unsigned int" declaration.
>  >> 
> 
>  Prakash> "unsigned int" is not guaranteed to be 32bit, may be "unsigned
>  Prakash> long" is better choice.
> 
> On the platforms where da8xx-fb is used it is.
> 

I agree "unsigned int" will suffice for the platforms where da8xx-fb is used.
With respect to below reference from "The C Programming Language" from "Kernighan and Ritchie"
I prefer to use "unsigned long", please let me know you views.
" The intent is that short and long should provide different lengths of integers where practical; int will 
normally be the natural size for a particular machine. short is often 16 bits long, and int either 16 or 
32 bits. Each compiler is free to choose appropriate sizes for its own hardware, subject only to the the 
restriction that shorts and ints are at least 16 bits, longs are at least 32 bits, and short is no longer 
than int, which is no longer than long."

>  >> >  	unsigned int palette_sz;
>  >> >  	unsigned int pxl_clk;
>  >> >  	int blank;
>  >> > @@ -546,6 +546,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
>  >> >  	return 0;
>  >> >  }
>  >> >  
>  >> > +
>  >> > +#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
>  >> 
>  >> Did you run checkpatch.pl on this patch?
>  >> 
> 
>  Prakash> Yes, checkpatch.pl did not complain anything on this line. I
>  Prakash> will add space around binary operators.
> 
> An inline function would be nicer.
> 

For the sake of uniformity with existing FB drivers, can I leave it as macro?

Thanks,
Prakash

> -- 
> Bye, Peter Korsgaard
> 


^ permalink raw reply

* [PATCH] video: exynos_dp: use usleep_range instead of delay
From: Jingoo Han @ 2012-07-18  9:50 UTC (permalink / raw)
  To: linux-fbdev

This patch replaces udelay and mdelay with usleep_range to remove
the busy loop waiting.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/exynos/exynos_dp_core.c |   14 +++++++-------
 drivers/video/exynos/exynos_dp_reg.c  |    4 ++--
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 9db7b9f..25907f4 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -47,7 +47,7 @@ static int exynos_dp_detect_hpd(struct exynos_dp_device *dp)
 
 	exynos_dp_init_hpd(dp);
 
-	udelay(200);
+	usleep_range(200, 210);
 
 	while (exynos_dp_get_plug_in_status(dp) != 0) {
 		timeout_loop++;
@@ -55,7 +55,7 @@ static int exynos_dp_detect_hpd(struct exynos_dp_device *dp)
 			dev_err(dp->dev, "failed to get hpd plug status\n");
 			return -ETIMEDOUT;
 		}
-		udelay(10);
+		usleep_range(10, 11);
 	}
 
 	return 0;
@@ -486,7 +486,7 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
 	u8 pre_emphasis;
 	u8 training_lane;
 
-	udelay(100);
+	usleep_range(100, 101);
 
 	exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
 				6, link_status);
@@ -571,7 +571,7 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
 
 	u8 adjust_request[2];
 
-	udelay(400);
+	usleep_range(400, 401);
 
 	exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
 				6, link_status);
@@ -739,7 +739,7 @@ static int exynos_dp_set_link_train(struct exynos_dp_device *dp,
 		if (retval = 0)
 			break;
 
-		udelay(100);
+		usleep_range(100, 110);
 	}
 
 	return retval;
@@ -773,7 +773,7 @@ static int exynos_dp_config_video(struct exynos_dp_device *dp,
 			return -ETIMEDOUT;
 		}
 
-		udelay(1);
+		usleep_range(1, 2);
 	}
 
 	/* Set to use the register calculated M/N video */
@@ -807,7 +807,7 @@ static int exynos_dp_config_video(struct exynos_dp_device *dp,
 			return -ETIMEDOUT;
 		}
 
-		mdelay(1);
+		usleep_range(1000, 1001);
 	}
 
 	if (retval != 0)
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 6ce76d5..ce401c8 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -122,7 +122,7 @@ void exynos_dp_reset(struct exynos_dp_device *dp)
 		LS_CLK_DOMAIN_FUNC_EN_N;
 	writel(reg, dp->reg_base + EXYNOS_DP_FUNC_EN_2);
 
-	udelay(20);
+	usleep_range(20, 30);
 
 	exynos_dp_lane_swap(dp, 0);
 
@@ -988,7 +988,7 @@ void exynos_dp_reset_macro(struct exynos_dp_device *dp)
 	writel(reg, dp->reg_base + EXYNOS_DP_PHY_TEST);
 
 	/* 10 us is the minimum reset time. */
-	udelay(10);
+	usleep_range(10, 20);
 
 	reg &= ~MACRO_RST;
 	writel(reg, dp->reg_base + EXYNOS_DP_PHY_TEST);
-- 
1.7.1



^ permalink raw reply related

* Re: Device tree binding for DVFS table
From: Prashant Gaikwad @ 2012-07-18 12:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20120717143754.GB3107@sirena.org.uk>

On Tuesday 17 July 2012 08:07 PM, Mark Brown wrote:
> On Tue, Jul 17, 2012 at 07:52:12PM +0530, Prashant Gaikwad wrote:
>
>>> What happens if there's more than one supply that needs to be varied?
>> Each rail's dvfs-table will have OPP nodes defined for different
>> voltages and each OPP node contains frequency for all clocks
>> affecting that rail.
> Right, but some systems have operating points which cover a combination
> of voltages and frequencies in one operating point.  Your binding seems
> to define operating points per supply with no tie in between supplies.

We had discussed this in previous replies and thought that it can be 
expressed in separate DT nodes, may be regulator.

^ permalink raw reply

* [PATCH RESEND] video: da8xx-fb rev2: fix disabling of palette completion interrupt
From: Manjunathappa, Prakash @ 2012-07-18 15:33 UTC (permalink / raw)
  To: linux-fbdev

Writing '1' to particular bit of IRQENABLE_CLEAR register disables the
corresponding interrupt on revision 2 LCDC. This register was wrongly
configured to disable all previous enabled interrupts instead of
disabling only palette completion interrupt. Patch fixes it by clearing
only palette completion interrupt bit.

Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
---
Resending as my earlier patch seems like not reached fbdev mailing list.

 drivers/video/da8xx-fb.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 47118c7..88e98ea 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -715,7 +715,6 @@ static irqreturn_t lcdc_irq_handler_rev02(int irq, void *arg)
 {
 	struct da8xx_fb_par *par = arg;
 	u32 stat = lcdc_read(LCD_MASKED_STAT_REG);
-	u32 reg_int;
 
 	if ((stat & LCD_SYNC_LOST) && (stat & LCD_FIFO_UNDERFLOW)) {
 		lcd_disable_raster();
@@ -732,10 +731,8 @@ static irqreturn_t lcdc_irq_handler_rev02(int irq, void *arg)
 
 		lcdc_write(stat, LCD_MASKED_STAT_REG);
 
-		/* Disable PL completion inerrupt */
-		reg_int = lcdc_read(LCD_INT_ENABLE_CLR_REG) |
-		       (LCD_V2_PL_INT_ENA);
-		lcdc_write(reg_int, LCD_INT_ENABLE_CLR_REG);
+		/* Disable PL completion interrupt */
+		lcdc_write(LCD_V2_PL_INT_ENA, LCD_INT_ENABLE_CLR_REG);
 
 		/* Setup and start data loading mode */
 		lcd_blit(LOAD_DATA, par);
-- 
1.7.1


^ permalink raw reply related

* [PATCH RESEND] video: da8xx-fb: enable sync lost interrupt
From: Manjunathappa, Prakash @ 2012-07-18 15:42 UTC (permalink / raw)
  To: linux-fbdev

Patch enables sync lost interrupt and interrupt handler already
takes care to handle it.

Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
---
Resending as my earlier patch seems like not reached fbdev mailing list.

 drivers/video/da8xx-fb.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 88e98ea..e9d2f6e 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -54,6 +54,7 @@
 #define LCD_DMA_BURST_8			0x3
 #define LCD_DMA_BURST_16		0x4
 #define LCD_V1_END_OF_FRAME_INT_ENA	BIT(2)
+#define LCD_V1_SYNC_LOST_ENA            BIT(5)
 #define LCD_V2_END_OF_FRAME0_INT_ENA	BIT(8)
 #define LCD_V2_END_OF_FRAME1_INT_ENA	BIT(9)
 #define LCD_DUAL_FRAME_BUFFER_ENABLE	BIT(0)
@@ -441,10 +442,10 @@ static int lcd_cfg_display(const struct lcd_ctrl_config *cfg)
 
 	/* enable additional interrupts here */
 	if (lcd_revision = LCD_VERSION_1) {
-		reg |= LCD_V1_UNDERFLOW_INT_ENA;
+		reg |= LCD_V1_UNDERFLOW_INT_ENA | LCD_V1_SYNC_LOST_ENA;
 	} else {
 		reg_int = lcdc_read(LCD_INT_ENABLE_SET_REG) |
-			LCD_V2_UNDERFLOW_INT_ENA;
+			LCD_V2_UNDERFLOW_INT_ENA | LCD_SYNC_LOST;
 		lcdc_write(reg_int, LCD_INT_ENABLE_SET_REG);
 	}
 
-- 
1.7.1


^ permalink raw reply related

* [PATCH RESEND] video: da8xx-fb: fix flicker due to 1 frame delay in updated frame
From: Manjunathappa, Prakash @ 2012-07-18 15:43 UTC (permalink / raw)
  To: linux-fbdev

Flicker/tearing effect is observed with current FB driver.
Issue is because of 2 active DMA channels ping ponging among them
along with usage of 2 DDR ping pong buffers in driver. Application
unaware of active DMA channel keeps updating frame being displayed,
this leads to tearing effect.
Below steps describes the issue:
1)Initially assume both buffers FB0 and FB1 are programmed for buffer-0.
2)On EOF0: Program FB0 for buffer-1, indicate(wake up) application
 to fill up buffer-0. As FB1 is active and continues to DMA buffer-0
(which is being filled), leading to tearing/flickering issue.
3)On EOF1: Program FB1 for buffer-0, indicate(wake up) application to
 fill up buffer-1. As FB0 is active and continues to DMA buffer-1(which
 is being filled), leading to tearing/flickering issue.
4)On EOF0: Program FB0 for buffer-1, indicate(wake up) application to
fill up buffer-0. As FB1 is active and continues to DMA buffer-0(which is
being filled), leading to tearing/flickering issue.
...
Above steps depict that issue is because of 1 frame delay in frame
panned by application.

Patch fixes the issue by keeping track free DMA channel and configures
it in drivers PAN callback so that panned frame from application gets
displayed in next frame period.

Wiki below describes the issue in detail and it also has link to
application with which issue can be reproduced.
http://processors.wiki.ti.com/index.php/DA8xx_LCDC_Linux_FB_FAQs

Signed-off-by: Nellutla, Aditya <aditya.n@ti.com>
Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
---
Resending as my earlier patch seems like not reached fbdev mailing list.

 drivers/video/da8xx-fb.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index e9d2f6e..183366d 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -30,6 +30,7 @@
 #include <linux/clk.h>
 #include <linux/cpufreq.h>
 #include <linux/console.h>
+#include <linux/spinlock.h>
 #include <linux/slab.h>
 #include <video/da8xx-fb.h>
 #include <asm/div64.h>
@@ -161,6 +162,13 @@ struct da8xx_fb_par {
 	wait_queue_head_t	vsync_wait;
 	int			vsync_flag;
 	int			vsync_timeout;
+	spinlock_t		lock_for_chan_update;
+
+	/*
+	 * LCDC has 2 ping pong DMA channels, channel 0
+	 * and channel 1.
+	 */
+	unsigned int		which_dma_channel_done;
 #ifdef CONFIG_CPU_FREQ
 	struct notifier_block	freq_transition;
 	unsigned int		lcd_fck_rate;
@@ -741,6 +749,7 @@ static irqreturn_t lcdc_irq_handler_rev02(int irq, void *arg)
 		lcdc_write(stat, LCD_MASKED_STAT_REG);
 
 		if (stat & LCD_END_OF_FRAME0) {
+			par->which_dma_channel_done = 0;
 			lcdc_write(par->dma_start,
 				   LCD_DMA_FRM_BUF_BASE_ADDR_0_REG);
 			lcdc_write(par->dma_end,
@@ -750,6 +759,7 @@ static irqreturn_t lcdc_irq_handler_rev02(int irq, void *arg)
 		}
 
 		if (stat & LCD_END_OF_FRAME1) {
+			par->which_dma_channel_done = 1;
 			lcdc_write(par->dma_start,
 				   LCD_DMA_FRM_BUF_BASE_ADDR_1_REG);
 			lcdc_write(par->dma_end,
@@ -796,6 +806,7 @@ static irqreturn_t lcdc_irq_handler_rev01(int irq, void *arg)
 		lcdc_write(stat, LCD_STAT_REG);
 
 		if (stat & LCD_END_OF_FRAME0) {
+			par->which_dma_channel_done = 0;
 			lcdc_write(par->dma_start,
 				   LCD_DMA_FRM_BUF_BASE_ADDR_0_REG);
 			lcdc_write(par->dma_end,
@@ -805,6 +816,7 @@ static irqreturn_t lcdc_irq_handler_rev01(int irq, void *arg)
 		}
 
 		if (stat & LCD_END_OF_FRAME1) {
+			par->which_dma_channel_done = 1;
 			lcdc_write(par->dma_start,
 				   LCD_DMA_FRM_BUF_BASE_ADDR_1_REG);
 			lcdc_write(par->dma_end,
@@ -1050,6 +1062,7 @@ static int da8xx_pan_display(struct fb_var_screeninfo *var,
 	struct fb_fix_screeninfo    *fix = &fbi->fix;
 	unsigned int end;
 	unsigned int start;
+	unsigned long irq_flags;
 
 	if (var->xoffset != fbi->var.xoffset ||
 			var->yoffset != fbi->var.yoffset) {
@@ -1067,6 +1080,21 @@ static int da8xx_pan_display(struct fb_var_screeninfo *var,
 			end	= start + fbi->var.yres * fix->line_length - 1;
 			par->dma_start	= start;
 			par->dma_end	= end;
+			spin_lock_irqsave(&par->lock_for_chan_update,
+					irq_flags);
+			if (par->which_dma_channel_done = 0) {
+				lcdc_write(par->dma_start,
+					   LCD_DMA_FRM_BUF_BASE_ADDR_0_REG);
+				lcdc_write(par->dma_end,
+					   LCD_DMA_FRM_BUF_CEILING_ADDR_0_REG);
+			} else if (par->which_dma_channel_done = 1) {
+				lcdc_write(par->dma_start,
+					   LCD_DMA_FRM_BUF_BASE_ADDR_1_REG);
+				lcdc_write(par->dma_end,
+					   LCD_DMA_FRM_BUF_CEILING_ADDR_1_REG);
+			}
+			spin_unlock_irqrestore(&par->lock_for_chan_update,
+					irq_flags);
 		}
 	}
 
@@ -1294,6 +1322,8 @@ static int __devinit fb_probe(struct platform_device *device)
 	/* initialize the vsync wait queue */
 	init_waitqueue_head(&par->vsync_wait);
 	par->vsync_timeout = HZ / 5;
+	par->which_dma_channel_done = -1;
+	spin_lock_init(&par->lock_for_chan_update);
 
 	/* Register the Frame Buffer  */
 	if (register_framebuffer(da8xx_fb_info) < 0) {
-- 
1.7.1


^ permalink raw reply related

* [PATCH v2 1/2] video: da8xx-fb: configure FIFO threshold to reduce underflow errors
From: Manjunathappa, Prakash @ 2012-07-18 15:45 UTC (permalink / raw)
  To: linux-fbdev

Patch works around the below silicon errata:
During LCDC initialization, there is the potential for a FIFO
underflow condition to occur. A FIFO underflow condition
occurs when the input FIFO is completely empty and the LCDC
raster controller logic that drives data to the output pins
attempts to fetch data from the FIFO. When a FIFO underflow
condition occurs, incorrect data will be driven out on the
LCDC data pins.

Software should poll the FUF bit field in the LCD_STAT register
to check if an error condition has occurred or service the
interrupt if FUF_EN is enabled when FUF occurs. If the FUF bit
field has been set to 1, this will indicate an underflow
condition has occurred and then the software should execute a
reset of the LCDC via the LPSC.

This problem may occur if the LCDC FIFO threshold size
(LCDDMA_CTRL[TH_FIFO_READY]) is left at its default value after
reset. Increasing the FIFO threshold size will reduce or
eliminate underflows. Setting the threshold size to 256 double
words or larger is recommended.

Above issue is described in section 2.1.3 of silicon errata
http://www.ti.com/lit/er/sprz313e/sprz313e.pdf

Signed-off-by: Rajashekhara, Sudhakar <sudhakar.raj@ti.com>
Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
---
Seems like version 1 of this patch did not reach fbdev mailing list.
Since v1:
Removed clk_disable/clk_enable from error interrupt handling code.

 drivers/video/da8xx-fb.c |   11 +++++++----
 include/video/da8xx-fb.h |    3 +++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 183366d..186ab5a 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -353,8 +353,8 @@ static void lcd_blit(int load_mode, struct da8xx_fb_par *par)
 	lcd_enable_raster();
 }
 
-/* Configure the Burst Size of DMA */
-static int lcd_cfg_dma(int burst_size)
+/* Configure the Burst Size and fifo threhold of DMA */
+static int lcd_cfg_dma(int burst_size, int fifo_th)
 {
 	u32 reg;
 
@@ -378,6 +378,9 @@ static int lcd_cfg_dma(int burst_size)
 	default:
 		return -EINVAL;
 	}
+
+	reg |= (fifo_th << 8);
+
 	lcdc_write(reg, LCD_DMA_CTRL_REG);
 
 	return 0;
@@ -679,8 +682,8 @@ static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
 		lcdc_write((lcdc_read(LCD_RASTER_TIMING_2_REG) &
 			~LCD_INVERT_PIXEL_CLOCK), LCD_RASTER_TIMING_2_REG);
 
-	/* Configure the DMA burst size. */
-	ret = lcd_cfg_dma(cfg->dma_burst_sz);
+	/* Configure the DMA burst size and fifo threshold. */
+	ret = lcd_cfg_dma(cfg->dma_burst_sz, cfg->fifo_th);
 	if (ret < 0)
 		return ret;
 
diff --git a/include/video/da8xx-fb.h b/include/video/da8xx-fb.h
index 89d43b3..5a0e4f9 100644
--- a/include/video/da8xx-fb.h
+++ b/include/video/da8xx-fb.h
@@ -82,6 +82,9 @@ struct lcd_ctrl_config {
 
 	/* Raster Data Order Select: 1=Most-to-least 0=Least-to-most */
 	unsigned char raster_order;
+
+	/* DMA FIFO threshold */
+	int fifo_th;
 };
 
 struct lcd_sync_arg {
-- 
1.7.1


^ permalink raw reply related

* [PATCH v2 2/2] arm: da850: configure LCDC fifo threshold
From: Manjunathappa, Prakash @ 2012-07-18 15:46 UTC (permalink / raw)
  To: linux-fbdev

Configure fifo threshold in raster dma_control register to
512 bytes. This reduces LCDC underflow errors.

Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
---
Seems like version 1 of this patch did not reach fbdev mailing list.
Since v1:
No change, resending it as other patch in series has changed.
 arch/arm/mach-davinci/devices-da8xx.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index d1624a3..783eab6 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -546,6 +546,7 @@ static struct lcd_ctrl_config lcd_cfg = {
 	.sync_edge		= 0,
 	.sync_ctrl		= 1,
 	.raster_order		= 0,
+	.fifo_th		= 6,
 };
 
 struct da8xx_lcdc_platform_data sharp_lcd035q3dg01_pdata = {
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH RESEND] video: da8xx-fb: enable sync lost interrupt
From: Sergei Shtylyov @ 2012-07-18 15:56 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1342625417-6974-1-git-send-email-prakash.pm@ti.com>

Hello.

On 07/18/2012 07:30 PM, Manjunathappa, Prakash wrote:

> Patch enables sync lost interrupt and interrupt handler already
> takes care to handle it.

> Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> ---
> Resending as my earlier patch seems like not reached fbdev mailing list.

>  drivers/video/da8xx-fb.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)

> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 88e98ea..e9d2f6e 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -54,6 +54,7 @@
>  #define LCD_DMA_BURST_8			0x3
>  #define LCD_DMA_BURST_16		0x4
>  #define LCD_V1_END_OF_FRAME_INT_ENA	BIT(2)
> +#define LCD_V1_SYNC_LOST_ENA            BIT(5)

   Please indent the value with tabs, as the others.

>  #define LCD_V2_END_OF_FRAME0_INT_ENA	BIT(8)
>  #define LCD_V2_END_OF_FRAME1_INT_ENA	BIT(9)
>  #define LCD_DUAL_FRAME_BUFFER_ENABLE	BIT(0)

WBR, Sergei

^ permalink raw reply

* [PATCH] radeonfb: Add quirk for the graphics adapter in some JSxx
From: olaf @ 2012-07-18 16:49 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, linux-fbdev, Olaf Hering

From: Tony Breeds <tony@bakeyournoodle.com>

These devices are set to 640x480 by firmware, switch them to 800x600@60
so that the graphical installer can run on remote console.

Reported by IBM during SLES10 SP2 beta testing:

https://bugzilla.novell.com/show_bug.cgi?idF1002
LTC50817

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff --git a/drivers/video/aty/radeon_monitor.c b/drivers/video/aty/radeon_monitor.c
index 9261c91..5c23eac 100644
--- a/drivers/video/aty/radeon_monitor.c
+++ b/drivers/video/aty/radeon_monitor.c
@@ -730,6 +730,25 @@ static void radeon_videomode_to_var(struct fb_var_screeninfo *var,
 	var->vmode = mode->vmode;
 }
 
+#ifdef CONFIG_PPC_PSERIES
+static int is_powerblade(const char *model)
+{
+	struct device_node *root;
+	const char* cp;
+	int len, l, rc = 0;
+
+	root = of_find_node_by_path("/");
+	if (root && model) {
+		l = strlen(model);
+		cp = of_get_property(root, "model", &len);
+		if (cp)
+			rc = memcmp(model, cp, min(len, l)) = 0;
+		of_node_put(root);
+	}
+	return rc;
+}
+#endif
+
 /*
  * Build the modedb for head 1 (head 2 will come later), check panel infos
  * from either BIOS or EDID, and pick up the default mode
@@ -865,6 +884,22 @@ void __devinit radeon_check_modes(struct radeonfb_info *rinfo, const char *mode_
 			has_default_mode = 1;
  	}
 
+#ifdef CONFIG_PPC_PSERIES
+	if (!has_default_mode && (
+		is_powerblade("IBM,8842") || /* JS20 */
+		is_powerblade("IBM,8844") || /* JS21 */
+		is_powerblade("IBM,7998") || /* JS12/JS21/JS22 */
+		is_powerblade("IBM,0792") || /* QS21 */
+		is_powerblade("IBM,0793")    /* QS22 */
+	    )) {
+		printk("Falling back to 800x600 on JSxx hardware\n");
+		if (fb_find_mode(&info->var, info, "800x600@60",
+				 info->monspecs.modedb,
+				 info->monspecs.modedb_len, NULL, 8) != 0)
+			has_default_mode = 1;
+	}
+#endif
+
 	/*
 	 * Still no mode, let's pick up a default from the db
 	 */
-- 
1.7.10.4


^ permalink raw reply related

* Re: Device tree binding for DVFS table
From: Shawn Guo @ 2012-07-18 17:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4FFD77FE.8050206@nvidia.com>

On Wed, Jul 11, 2012 at 06:26:30PM +0530, Prashant Gaikwad wrote:
> Hi,
> 
> I am working on DT binding for Tegra DVFS.
> 
> For Tegra, DVFS node mainly consists of frequency and voltage pairs.
> Frequency in the pair may change for different process. E.g. for
> process 1 CPU clock frequency could be 900MHz at 1V while for
> process 2 it could be 1GHz at 1V.
> Tegra uses vendor specific ids to identify the correct frequency table.
> 
> Following is the proposed binding for voltage and frequency tables
> used in DVFS. Looking for comments/suggestions to make it generic.
> 
It seems we are trying to approach a generic binding for DVFS operating
points.  In that case, we will need a generic library helper function
to parse it.  But since we already have such a library - OPP
(drivers/base/power/opp.c) in place, I would approach the same goal
in other way around, adding device tree binding for OPP.  Please see
the patch I just sent out below and comment.

[PATCH] PM / OPP: Initialize OPP table from device tree

-- 
Regards,
Shawn


^ permalink raw reply

* Re: Device tree binding for DVFS table
From: Mark Brown @ 2012-07-18 21:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5006B01E.7020504@nvidia.com>

On Wed, Jul 18, 2012 at 06:16:22PM +0530, Prashant Gaikwad wrote:
> On Tuesday 17 July 2012 08:07 PM, Mark Brown wrote:

> >Right, but some systems have operating points which cover a combination
> >of voltages and frequencies in one operating point.  Your binding seems
> >to define operating points per supply with no tie in between supplies.

> We had discussed this in previous replies and thought that it can be
> expressed in separate DT nodes, may be regulator.

So how are these intended to be tied together?  Is the kernel supposed
to infer the set of possible operating points by looking for overlaps or
does it have a list to work with?

^ permalink raw reply

* Re: [PATCH] fbdev: Add ioctls FBIO_COPYRECT, FBIO_FILLRECT, lest these operations be done in softwar
From: Laurent Pinchart @ 2012-07-18 22:57 UTC (permalink / raw)
  To: Strake; +Cc: Florian Tobias Schandinat, linux-fbdev, linux-kernel
In-Reply-To: <CAL3m8eA=46wdwVosiE54eNx3FOuLyxfE5axKs_HjyMTGxDHRDw@mail.gmail.com>

On Thursday 21 June 2012 07:22:50 Strake wrote:
> From: Matthew Farkas-Dyck <strake888@gmail.com>
> 
> Add ioctls FBIO_COPYRECT, FBIO_FILLRECT, lest these operations be done
> in software.
> 
> Signed-off-by: Matthew Farkas-Dyck <strake888@gmail.com>
> ---
> 
> Reason: If it can be done in hardware, it ought to be. This will
> enable user-mode software to do so.

If we really want to go this way, I think we should develop a proper 2D 
acceleration API. I'm not sure whether it's worth the effort though.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* [PATCH 0/9] SH Mobile LCDC and MERAM patches
From: Laurent Pinchart @ 2012-07-19  0:39 UTC (permalink / raw)
  To: linux-fbdev

Hi,

Here are 9 patches for the SH Mobile LCDC and MERAM drivers. Patches 1/9 to
4/9 and 9/9 have previously been posted as part of the "SH Mobile LCDC
MERAM-based frame buffer backing store" RFC series. Patch 5/9 has also been
posted as part of the same series, albeit included in patch 9/9.

As most of those patches have already been posted for review (patches 6/9 to
8/9 that haven't been posted already are small fixes), I plan to send a pull
request soon, to get the patches in v3.6 if possible.

Laurent Pinchart (9):
  sh_mobile_meram: Rename operations to cache_[alloc|free|update]
  sh_mobile_meram: Use direct function calls for the public API
  sh_mobile_meram: Add direct MERAM allocation API
  fbdev: sh_mobile_lcdc: Destroy mutex at remove time
  fbdev: sh_mobile_lcdc: Fix line pitch computation
  fbdev: sh_mobile_lcdc: Use channel configuration to initialize fb
    device
  fbdev: sh_mobile_lcdc: Support horizontal panning
  fbdev: sh_mobile_lcdc: Fix overlay registers update during pan
    operation
  fbdev: sh_mobile_lcdc: Fix pan offset computation in YUV mode

 drivers/video/sh_mobile_lcdcfb.c |  209 +++++++++++++++++----------------
 drivers/video/sh_mobile_lcdcfb.h |    5 +-
 drivers/video/sh_mobile_meram.c  |  235 +++++++++++++++++++++-----------------
 include/video/sh_mobile_meram.h  |   71 ++++++++----
 4 files changed, 293 insertions(+), 227 deletions(-)

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* [PATCH 1/9] sh_mobile_meram: Rename operations to cache_[alloc|free|update]
From: Laurent Pinchart @ 2012-07-19  0:39 UTC (permalink / raw)
  To: linux-fbdev

The MERAM operations meram_register, meram_unregister and meram_update
handle LCDC cache. In preparation for "raw" MERAM allocation, rename
them to more appropriate names.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_lcdcfb.c |   32 +++----
 drivers/video/sh_mobile_lcdcfb.h |    2 +-
 drivers/video/sh_mobile_meram.c  |  176 ++++++++++++++++++-------------------
 include/video/sh_mobile_meram.h  |   21 ++---
 4 files changed, 110 insertions(+), 121 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 98e81b3..e593e81 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -1104,7 +1104,7 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 	/* Compute frame buffer base address and pitch for each channel. */
 	for (k = 0; k < ARRAY_SIZE(priv->ch); k++) {
 		int pixelformat;
-		void *meram;
+		void *cache;
 
 		ch = &priv->ch[k];
 		if (!ch->enabled)
@@ -1119,12 +1119,10 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 		    ch->cfg->meram_cfg = NULL)
 			continue;
 
-		/* we need to de-init configured ICBs before we can
-		 * re-initialize them.
-		 */
-		if (ch->meram) {
-			mdev->ops->meram_unregister(mdev, ch->meram);
-			ch->meram = NULL;
+		/* Free the allocated MERAM cache. */
+		if (ch->cache) {
+			mdev->ops->cache_free(mdev, ch->cache);
+			ch->cache = NULL;
 		}
 
 		switch (ch->format->fourcc) {
@@ -1146,14 +1144,14 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 			break;
 		}
 
-		meram = mdev->ops->meram_register(mdev, ch->cfg->meram_cfg,
+		cache = mdev->ops->cache_alloc(mdev, ch->cfg->meram_cfg,
 					ch->pitch, ch->yres, pixelformat,
 					&ch->line_size);
-		if (!IS_ERR(meram)) {
-			mdev->ops->meram_update(mdev, meram,
+		if (!IS_ERR(cache)) {
+			mdev->ops->cache_update(mdev, cache,
 					ch->base_addr_y, ch->base_addr_c,
 					&ch->base_addr_y, &ch->base_addr_c);
-			ch->meram = meram;
+			ch->cache = cache;
 		}
 	}
 
@@ -1223,12 +1221,12 @@ static void sh_mobile_lcdc_stop(struct sh_mobile_lcdc_priv *priv)
 
 		sh_mobile_lcdc_display_off(ch);
 
-		/* disable the meram */
-		if (ch->meram) {
+		/* Free the MERAM cache. */
+		if (ch->cache) {
 			struct sh_mobile_meram_info *mdev;
 			mdev = priv->meram_dev;
-			mdev->ops->meram_unregister(mdev, ch->meram);
-			ch->meram = 0;
+			mdev->ops->cache_free(mdev, ch->cache);
+			ch->cache = 0;
 		}
 
 	}
@@ -1839,11 +1837,11 @@ static int sh_mobile_lcdc_pan(struct fb_var_screeninfo *var,
 			base_addr_c += var->xoffset;
 	}
 
-	if (ch->meram) {
+	if (ch->cache) {
 		struct sh_mobile_meram_info *mdev;
 
 		mdev = priv->meram_dev;
-		mdev->ops->meram_update(mdev, ch->meram,
+		mdev->ops->cache_update(mdev, ch->cache,
 					base_addr_y, base_addr_c,
 					&base_addr_y, &base_addr_c);
 	}
diff --git a/drivers/video/sh_mobile_lcdcfb.h b/drivers/video/sh_mobile_lcdcfb.h
index 5c3bddd..e53cd11 100644
--- a/drivers/video/sh_mobile_lcdcfb.h
+++ b/drivers/video/sh_mobile_lcdcfb.h
@@ -59,7 +59,7 @@ struct sh_mobile_lcdc_chan {
 	unsigned long *reg_offs;
 	unsigned long ldmt1r_value;
 	unsigned long enabled; /* ME and SE in LDCNT2R */
-	void *meram;
+	void *cache;
 
 	struct mutex open_lock;		/* protects the use counter */
 	int use_count;
diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index 82ba830..4aa3fcb 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -194,13 +194,13 @@ static inline unsigned long meram_read_reg(void __iomem *base, unsigned int off)
 }
 
 /* -----------------------------------------------------------------------------
- * Allocation
+ * LCDC cache planes allocation, init, cleanup and free
  */
 
 /* Allocate ICBs and MERAM for a plane. */
-static int __meram_alloc(struct sh_mobile_meram_priv *priv,
-			 struct sh_mobile_meram_fb_plane *plane,
-			 size_t size)
+static int meram_plane_alloc(struct sh_mobile_meram_priv *priv,
+			     struct sh_mobile_meram_fb_plane *plane,
+			     size_t size)
 {
 	unsigned long mem;
 	unsigned long idx;
@@ -229,8 +229,8 @@ static int __meram_alloc(struct sh_mobile_meram_priv *priv,
 }
 
 /* Free ICBs and MERAM for a plane. */
-static void __meram_free(struct sh_mobile_meram_priv *priv,
-			 struct sh_mobile_meram_fb_plane *plane)
+static void meram_plane_free(struct sh_mobile_meram_priv *priv,
+			     struct sh_mobile_meram_fb_plane *plane)
 {
 	gen_pool_free(priv->pool, priv->meram + plane->marker->offset,
 		      plane->marker->size * 1024);
@@ -248,62 +248,6 @@ static int is_nvcolor(int cspace)
 	return 0;
 }
 
-/* Allocate memory for the ICBs and mark them as used. */
-static struct sh_mobile_meram_fb_cache *
-meram_alloc(struct sh_mobile_meram_priv *priv,
-	    const struct sh_mobile_meram_cfg *cfg,
-	    int pixelformat)
-{
-	struct sh_mobile_meram_fb_cache *cache;
-	unsigned int nplanes = is_nvcolor(pixelformat) ? 2 : 1;
-	int ret;
-
-	if (cfg->icb[0].meram_size = 0)
-		return ERR_PTR(-EINVAL);
-
-	if (nplanes = 2 && cfg->icb[1].meram_size = 0)
-		return ERR_PTR(-EINVAL);
-
-	cache = kzalloc(sizeof(*cache), GFP_KERNEL);
-	if (cache = NULL)
-		return ERR_PTR(-ENOMEM);
-
-	cache->nplanes = nplanes;
-
-	ret = __meram_alloc(priv, &cache->planes[0], cfg->icb[0].meram_size);
-	if (ret < 0)
-		goto error;
-
-	cache->planes[0].marker->current_reg = 1;
-	cache->planes[0].marker->pixelformat = pixelformat;
-
-	if (cache->nplanes = 1)
-		return cache;
-
-	ret = __meram_alloc(priv, &cache->planes[1], cfg->icb[1].meram_size);
-	if (ret < 0) {
-		__meram_free(priv, &cache->planes[0]);
-		goto error;
-	}
-
-	return cache;
-
-error:
-	kfree(cache);
-	return ERR_PTR(-ENOMEM);
-}
-
-/* Unmark the specified ICB as used. */
-static void meram_free(struct sh_mobile_meram_priv *priv,
-		       struct sh_mobile_meram_fb_cache *cache)
-{
-	__meram_free(priv, &cache->planes[0]);
-	if (cache->nplanes = 2)
-		__meram_free(priv, &cache->planes[1]);
-
-	kfree(cache);
-}
-
 /* Set the next address to fetch. */
 static void meram_set_next_addr(struct sh_mobile_meram_priv *priv,
 				struct sh_mobile_meram_fb_cache *cache,
@@ -355,10 +299,10 @@ meram_get_next_icb_addr(struct sh_mobile_meram_info *pdata,
 	(((x) * (y) + (MERAM_LINE_WIDTH - 1)) & ~(MERAM_LINE_WIDTH - 1))
 
 /* Initialize MERAM. */
-static int meram_init(struct sh_mobile_meram_priv *priv,
-		      struct sh_mobile_meram_fb_plane *plane,
-		      unsigned int xres, unsigned int yres,
-		      unsigned int *out_pitch)
+static int meram_plane_init(struct sh_mobile_meram_priv *priv,
+			    struct sh_mobile_meram_fb_plane *plane,
+			    unsigned int xres, unsigned int yres,
+			    unsigned int *out_pitch)
 {
 	struct sh_mobile_meram_icb *marker = plane->marker;
 	unsigned long total_byte_count = MERAM_CALC_BYTECOUNT(xres, yres);
@@ -427,8 +371,8 @@ static int meram_init(struct sh_mobile_meram_priv *priv,
 	return 0;
 }
 
-static void meram_deinit(struct sh_mobile_meram_priv *priv,
-			 struct sh_mobile_meram_fb_plane *plane)
+static void meram_plane_cleanup(struct sh_mobile_meram_priv *priv,
+				struct sh_mobile_meram_fb_plane *plane)
 {
 	/* disable ICB */
 	meram_write_icb(priv->base, plane->cache->index,  MExxCTL,
@@ -441,18 +385,60 @@ static void meram_deinit(struct sh_mobile_meram_priv *priv,
 }
 
 /* -----------------------------------------------------------------------------
- * Registration/unregistration
+ * LCDC cache operations
  */
 
-static void *sh_mobile_meram_register(struct sh_mobile_meram_info *pdata,
-				      const struct sh_mobile_meram_cfg *cfg,
-				      unsigned int xres, unsigned int yres,
-				      unsigned int pixelformat,
-				      unsigned int *pitch)
+/* Allocate memory for the ICBs and mark them as used. */
+static struct sh_mobile_meram_fb_cache *
+meram_cache_alloc(struct sh_mobile_meram_priv *priv,
+		  const struct sh_mobile_meram_cfg *cfg,
+		  int pixelformat)
+{
+	unsigned int nplanes = is_nvcolor(pixelformat) ? 2 : 1;
+	struct sh_mobile_meram_fb_cache *cache;
+	int ret;
+
+	cache = kzalloc(sizeof(*cache), GFP_KERNEL);
+	if (cache = NULL)
+		return ERR_PTR(-ENOMEM);
+
+	cache->nplanes = nplanes;
+
+	ret = meram_plane_alloc(priv, &cache->planes[0],
+				cfg->icb[0].meram_size);
+	if (ret < 0)
+		goto error;
+
+	cache->planes[0].marker->current_reg = 1;
+	cache->planes[0].marker->pixelformat = pixelformat;
+
+	if (cache->nplanes = 1)
+		return cache;
+
+	ret = meram_plane_alloc(priv, &cache->planes[1],
+				cfg->icb[1].meram_size);
+	if (ret < 0) {
+		meram_plane_free(priv, &cache->planes[0]);
+		goto error;
+	}
+
+	return cache;
+
+error:
+	kfree(cache);
+	return ERR_PTR(-ENOMEM);
+}
+
+static void *sh_mobile_cache_alloc(struct sh_mobile_meram_info *pdata,
+				   const struct sh_mobile_meram_cfg *cfg,
+				   unsigned int xres, unsigned int yres,
+				   unsigned int pixelformat,
+				   unsigned int *pitch)
 {
 	struct sh_mobile_meram_fb_cache *cache;
 	struct sh_mobile_meram_priv *priv = pdata->priv;
 	struct platform_device *pdev = pdata->pdev;
+	unsigned int nplanes = is_nvcolor(pixelformat) ? 2 : 1;
 	unsigned int out_pitch;
 
 	if (pixelformat != SH_MOBILE_MERAM_PF_NV &&
@@ -469,10 +455,16 @@ static void *sh_mobile_meram_register(struct sh_mobile_meram_info *pdata,
 		return ERR_PTR(-EINVAL);
 	}
 
+	if (cfg->icb[0].meram_size = 0)
+		return ERR_PTR(-EINVAL);
+
+	if (nplanes = 2 && cfg->icb[1].meram_size = 0)
+		return ERR_PTR(-EINVAL);
+
 	mutex_lock(&priv->lock);
 
 	/* We now register the ICBs and allocate the MERAM regions. */
-	cache = meram_alloc(priv, cfg, pixelformat);
+	cache = meram_cache_alloc(priv, cfg, pixelformat);
 	if (IS_ERR(cache)) {
 		dev_err(&pdev->dev, "MERAM allocation failed (%ld).",
 			PTR_ERR(cache));
@@ -480,14 +472,14 @@ static void *sh_mobile_meram_register(struct sh_mobile_meram_info *pdata,
 	}
 
 	/* initialize MERAM */
-	meram_init(priv, &cache->planes[0], xres, yres, &out_pitch);
+	meram_plane_init(priv, &cache->planes[0], xres, yres, &out_pitch);
 	*pitch = out_pitch;
 	if (pixelformat = SH_MOBILE_MERAM_PF_NV)
-		meram_init(priv, &cache->planes[1], xres, (yres + 1) / 2,
-			&out_pitch);
+		meram_plane_init(priv, &cache->planes[1],
+				 xres, (yres + 1) / 2, &out_pitch);
 	else if (pixelformat = SH_MOBILE_MERAM_PF_NV24)
-		meram_init(priv, &cache->planes[1], 2 * xres, (yres + 1) / 2,
-			&out_pitch);
+		meram_plane_init(priv, &cache->planes[1],
+				 2 * xres, (yres + 1) / 2, &out_pitch);
 
 err:
 	mutex_unlock(&priv->lock);
@@ -495,25 +487,29 @@ err:
 }
 
 static void
-sh_mobile_meram_unregister(struct sh_mobile_meram_info *pdata, void *data)
+sh_mobile_cache_free(struct sh_mobile_meram_info *pdata, void *data)
 {
 	struct sh_mobile_meram_fb_cache *cache = data;
 	struct sh_mobile_meram_priv *priv = pdata->priv;
 
 	mutex_lock(&priv->lock);
 
-	/* deinit & free */
-	meram_deinit(priv, &cache->planes[0]);
-	if (cache->nplanes = 2)
-		meram_deinit(priv, &cache->planes[1]);
+	/* Cleanup and free. */
+	meram_plane_cleanup(priv, &cache->planes[0]);
+	meram_plane_free(priv, &cache->planes[0]);
 
-	meram_free(priv, cache);
+	if (cache->nplanes = 2) {
+		meram_plane_cleanup(priv, &cache->planes[1]);
+		meram_plane_free(priv, &cache->planes[1]);
+	}
+
+	kfree(cache);
 
 	mutex_unlock(&priv->lock);
 }
 
 static void
-sh_mobile_meram_update(struct sh_mobile_meram_info *pdata, void *data,
+sh_mobile_cache_update(struct sh_mobile_meram_info *pdata, void *data,
 		       unsigned long base_addr_y, unsigned long base_addr_c,
 		       unsigned long *icb_addr_y, unsigned long *icb_addr_c)
 {
@@ -530,9 +526,9 @@ sh_mobile_meram_update(struct sh_mobile_meram_info *pdata, void *data,
 
 static struct sh_mobile_meram_ops sh_mobile_meram_ops = {
 	.module			= THIS_MODULE,
-	.meram_register		= sh_mobile_meram_register,
-	.meram_unregister	= sh_mobile_meram_unregister,
-	.meram_update		= sh_mobile_meram_update,
+	.cache_alloc		= sh_mobile_cache_alloc,
+	.cache_free		= sh_mobile_cache_free,
+	.cache_update		= sh_mobile_cache_update,
 };
 
 /* -----------------------------------------------------------------------------
diff --git a/include/video/sh_mobile_meram.h b/include/video/sh_mobile_meram.h
index 29b2fd3..8a5afaf 100644
--- a/include/video/sh_mobile_meram.h
+++ b/include/video/sh_mobile_meram.h
@@ -41,19 +41,14 @@ struct sh_mobile_meram_cfg {
 struct module;
 struct sh_mobile_meram_ops {
 	struct module	*module;
-	/* register usage of meram */
-	void *(*meram_register)(struct sh_mobile_meram_info *meram_dev,
-				const struct sh_mobile_meram_cfg *cfg,
-				unsigned int xres, unsigned int yres,
-				unsigned int pixelformat,
-				unsigned int *pitch);
-
-	/* unregister usage of meram */
-	void (*meram_unregister)(struct sh_mobile_meram_info *meram_dev,
-				 void *data);
-
-	/* update meram settings */
-	void (*meram_update)(struct sh_mobile_meram_info *meram_dev, void *data,
+
+	/* LCDC cache management */
+	void *(*cache_alloc)(struct sh_mobile_meram_info *meram_dev,
+			     const struct sh_mobile_meram_cfg *cfg,
+			     unsigned int xres, unsigned int yres,
+			     unsigned int pixelformat, unsigned int *pitch);
+	void (*cache_free)(struct sh_mobile_meram_info *meram_dev, void *data);
+	void (*cache_update)(struct sh_mobile_meram_info *meram_dev, void *data,
 			     unsigned long base_addr_y,
 			     unsigned long base_addr_c,
 			     unsigned long *icb_addr_y,
-- 
1.7.8.6


^ permalink raw reply related


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