Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCHv9][ 3/3] ARM: dts: cpuimx35 Add touchscreen support.
From: Eric Bénard @ 2013-11-21  8:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20131121050922.GF18477@ns203013.ovh.net>

Le Thu, 21 Nov 2013 06:09:22 +0100,
Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> a écrit :
> > +	tsc2007: tsc2007@48 {
> > +		compatible = "ti,tsc2007";
> > +		reg = <0x48>;
> > +		interrupt-parent = <&gpio3>;
> > +		interrupts = <0x2 0x8>;
> > +		gpios = <&gpio3 2 1>;
> as explain on the binding drop this gpios this is an IRQ not a gpio
> 
> NACK
> 
please read the tsc2007's code, in the patch there is :

+static int tsc2007_get_pendown_state_dt(struct tsc2007 *ts)
+{
+       return !gpio_get_value(ts->gpio);
+}
+

http://www.mail-archive.com/linux-input@vger.kernel.org/msg06765.html

Eric

^ permalink raw reply

* Re: [PATCH] video: backlight: Remove backlight sysfs uevent
From: Henrique de Moraes Holschuh @ 2013-11-21 11:43 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Jingoo Han, 'Kyungmin Park',
	'Henrique de Moraes Holschuh', linux-fbdev, linux-kernel,
	kay, 'Richard Purdie', ibm-acpi-devel,
	platform-driver-x86
In-Reply-To: <1384990859.20536.4.camel@x230>

On Wed, 20 Nov 2013, Matthew Garrett wrote:
> On Mon, 2013-11-11 at 22:56 -0200, Henrique de Moraes Holschuh wrote:
> > On Tue, 12 Nov 2013, Jingoo Han wrote:
> > > 'thinkpad_acpi.c' uses the 'BACKLIGHT_UPDATE_SYSFS'.
> > > Henrique, can we remove it?
> > 
> > Can't you fix this by rate-limiting, or otherwise adding an attribute that
> > backlight devices should set when they need to supress change events?
> 
> It looks like this is just to force synchronisation to sysfs when using
> the /proc interface? In which case we should probably just kill
> the /proc interface.

Well, we can remove the thinkpad-acpi /proc interface as far as I'm
concerned, and that would do away with the use of BACKLIGHT_UPDATE_SYSFS by
thinkpad-acpi.  It is a major userspace ABI break, but removing everything
under /proc/acpi is one of the very few ABI breaks we actually have the
green light to do.

However, the patchset is not about this.

With this patchset applied, as far as I can tell anything that used to be
uevent-driven by the backlight class will break: when a process changes the
backlight using sysfs, other processes will not be notified of the change
anymore.  This patchset seems to break backlight uevent support in such a
way that basically renders the entire thing useless and you might as well
just remove uevent support entirely.

It is also an userspace ABI break, which we do not do lightly.

So, as far as I'm concerned, this patchset should be rejected in its present
form.  IMO, either one that preserves BACKLIGHT_UPDATE_SYSFS and fixes the
urgent issue, or one that removes uevent support entirely from the backlight
class should be proposed instead.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply

* Re: [PATCH] video: backlight: Remove backlight sysfs uevent
From: Matthew Garrett @ 2013-11-21 14:33 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Jingoo Han, 'Kyungmin Park',
	'Henrique de Moraes Holschuh', linux-fbdev, linux-kernel,
	kay, 'Richard Purdie', ibm-acpi-devel,
	platform-driver-x86
In-Reply-To: <20131121114332.GA23710@khazad-dum.debian.net>

On Thu, Nov 21, 2013 at 09:43:32AM -0200, Henrique de Moraes Holschuh wrote:

> With this patchset applied, as far as I can tell anything that used to be
> uevent-driven by the backlight class will break: when a process changes the
> backlight using sysfs, other processes will not be notified of the change
> anymore.  This patchset seems to break backlight uevent support in such a
> way that basically renders the entire thing useless and you might as well
> just remove uevent support entirely.

The uevent support was initially added to handle systems where pressing 
a hotkey generates an event (good) but the firmware automatically 
changes the brightness (bad). I have absolutely no idea why I added 
BACKLIGHT_UPDATE_SYSFS - BACKLIGHT_UPDATE_HOTKEY solves the problem I 
was trying to solve. I'm not aware of any userspace that relies on 
BACKLIGHT_UPDATE_SYSFS.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply

* [PATCH v3] video: add OpenCores VGA/LCD framebuffer driver
From: Stefan Kristiansson @ 2013-11-22  4:34 UTC (permalink / raw)
  To: linux-kernel, linux-fbdev; +Cc: tomi.valkeinen, plagnioj, Stefan Kristiansson

This adds support for the VGA/LCD core available from OpenCores:
http://opencores.org/project,vga_lcd

The driver have been tested together with both OpenRISC and
ARM (socfpga) processors.

Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
---
Changes in v2:
- Add Microblaze as an example user and fix a typo in Xilinx Zynq

Changes in v3:
- Use devm_kzalloc instead of kzalloc
- Remove superflous MODULE #ifdef
---
 drivers/video/Kconfig  |  17 ++
 drivers/video/Makefile |   1 +
 drivers/video/ocfb.c   | 471 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 489 insertions(+)
 create mode 100644 drivers/video/ocfb.c

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 84b685f..3b3f31e 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -979,6 +979,23 @@ config FB_PVR2
 	  (<file:drivers/video/pvr2fb.c>). Please see the file
 	  <file:Documentation/fb/pvr2fb.txt>.
 
+config FB_OPENCORES
+	tristate "OpenCores VGA/LCD core 2.0 framebuffer support"
+	depends on FB
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
+	default n
+	help
+	  This enables support for the OpenCores VGA/LCD core.
+
+	  The OpenCores VGA/LCD core is typically used together with
+	  softcore CPUs (e.g. OpenRISC or Microblaze) or hard processor
+	  systems (e.g. Altera socfpga or Xilinx Zynq) on FPGAs.
+
+	  The source code and specification for the core is available at
+	  <http://opencores.org/project,vga_lcd>
+
 config FB_S1D13XXX
 	tristate "Epson S1D13XXX framebuffer support"
 	depends on FB
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index e8bae8d..ae17ddf 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -150,6 +150,7 @@ obj-$(CONFIG_FB_NUC900)           += nuc900fb.o
 obj-$(CONFIG_FB_JZ4740)		  += jz4740_fb.o
 obj-$(CONFIG_FB_PUV3_UNIGFX)      += fb-puv3.o
 obj-$(CONFIG_FB_HYPERV)		  += hyperv_fb.o
+obj-$(CONFIG_FB_OPENCORES)	  += ocfb.o
 
 # Platform or fallback drivers go here
 obj-$(CONFIG_FB_UVESA)            += uvesafb.o
diff --git a/drivers/video/ocfb.c b/drivers/video/ocfb.c
new file mode 100644
index 0000000..3bf5f67
--- /dev/null
+++ b/drivers/video/ocfb.c
@@ -0,0 +1,471 @@
+/*
+ * OpenCores VGA/LCD 2.0 core frame buffer driver
+ *
+ * Copyright (C) 2013 Stefan Kristiansson, stefan.kristiansson@saunalahti.fi
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#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/mm.h>
+#include <linux/dma-mapping.h>
+#include <linux/fb.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+
+/* OCFB register defines */
+#define OCFB_CTRL	0x000
+#define OCFB_STAT	0x004
+#define OCFB_HTIM	0x008
+#define OCFB_VTIM	0x00c
+#define OCFB_HVLEN	0x010
+#define OCFB_VBARA	0x014
+#define OCFB_PALETTE	0x800
+
+#define OCFB_CTRL_VEN	0x00000001 /* Video Enable */
+#define OCFB_CTRL_HIE	0x00000002 /* HSync Interrupt Enable */
+#define OCFB_CTRL_PC	0x00000800 /* 8-bit Pseudo Color Enable*/
+#define OCFB_CTRL_CD8	0x00000000 /* Color Depth 8 */
+#define OCFB_CTRL_CD16	0x00000200 /* Color Depth 16 */
+#define OCFB_CTRL_CD24	0x00000400 /* Color Depth 24 */
+#define OCFB_CTRL_CD32	0x00000600 /* Color Depth 32 */
+#define OCFB_CTRL_VBL1	0x00000000 /* Burst Length 1 */
+#define OCFB_CTRL_VBL2	0x00000080 /* Burst Length 2 */
+#define OCFB_CTRL_VBL4	0x00000100 /* Burst Length 4 */
+#define OCFB_CTRL_VBL8	0x00000180 /* Burst Length 8 */
+
+#define PALETTE_SIZE	256
+
+#define OCFB_NAME	"OC VGA/LCD"
+
+static char *mode_option;
+
+static const struct fb_videomode default_mode = {
+	/* 640x480 @ 60 Hz, 31.5 kHz hsync */
+	NULL, 60, 640, 480, 39721, 40, 24, 32, 11, 96, 2,
+	0, FB_VMODE_NONINTERLACED
+};
+
+struct ocfb_dev {
+	struct fb_info info;
+	/* Physical and virtual addresses of control regs */
+	phys_addr_t regs_phys;
+	int regs_phys_size;
+	void __iomem *regs;
+	/* flag indicating whether the regs are little endian accessed */
+	int little_endian;
+	/* Physical and virtual addresses of framebuffer */
+	phys_addr_t fb_phys;
+	void __iomem *fb_virt;
+	u32 pseudo_palette[PALETTE_SIZE];
+};
+
+struct ocfb_par {
+	void __iomem *pal_adr;
+};
+
+static struct ocfb_par ocfb_par_priv;
+
+static struct fb_var_screeninfo ocfb_var;
+static struct fb_fix_screeninfo ocfb_fix;
+
+#ifndef MODULE
+static int __init ocfb_setup(char *options)
+{
+	char *curr_opt;
+
+	if (!options || !*options)
+		return 0;
+
+	while ((curr_opt = strsep(&options, ",")) != NULL) {
+		if (!*curr_opt)
+			continue;
+		mode_option = curr_opt;
+	}
+
+	return 0;
+}
+#endif
+
+static inline u32 ocfb_readreg(struct ocfb_dev *fbdev, loff_t offset)
+{
+	if (fbdev->little_endian)
+		return ioread32(fbdev->regs + offset);
+	else
+		return ioread32be(fbdev->regs + offset);
+}
+
+static void ocfb_writereg(struct ocfb_dev *fbdev, loff_t offset, u32 data)
+{
+	if (fbdev->little_endian)
+		iowrite32(data, fbdev->regs + offset);
+	else
+		iowrite32be(data, fbdev->regs + offset);
+}
+
+static int ocfb_setupfb(struct ocfb_dev *fbdev)
+{
+	unsigned long bpp_config;
+	struct fb_var_screeninfo *var = &fbdev->info.var;
+	struct device *dev = fbdev->info.device;
+	u32 hlen;
+	u32 vlen;
+
+	/* Disable display */
+	ocfb_writereg(fbdev, OCFB_CTRL, 0);
+
+	/* Register framebuffer address */
+	fbdev->little_endian = 0;
+	ocfb_writereg(fbdev, OCFB_VBARA, fbdev->fb_phys);
+
+	/* Detect endianess */
+	if (ocfb_readreg(fbdev, OCFB_VBARA) != fbdev->fb_phys) {
+		fbdev->little_endian = 1;
+		ocfb_writereg(fbdev, OCFB_VBARA, fbdev->fb_phys);
+	}
+
+	/* Horizontal timings */
+	ocfb_writereg(fbdev, OCFB_HTIM, (var->hsync_len - 1) << 24 |
+		      (var->right_margin - 1) << 16 | (var->xres - 1));
+
+	/* Vertical timings */
+	ocfb_writereg(fbdev, OCFB_VTIM, (var->vsync_len - 1) << 24 |
+		      (var->lower_margin - 1) << 16 | (var->yres - 1));
+
+	/* Total length of frame */
+	hlen = var->left_margin + var->right_margin + var->hsync_len +
+		var->xres;
+
+	vlen = var->upper_margin + var->lower_margin + var->vsync_len +
+		var->yres;
+
+	ocfb_writereg(fbdev, OCFB_HVLEN, (hlen - 1) << 16 | (vlen - 1));
+
+	bpp_config = OCFB_CTRL_CD8;
+	switch (var->bits_per_pixel) {
+	case 8:
+		if (!var->grayscale)
+			bpp_config |= OCFB_CTRL_PC;  /* enable palette */
+		break;
+
+	case 16:
+		bpp_config |= OCFB_CTRL_CD16;
+		break;
+
+	case 24:
+		bpp_config |= OCFB_CTRL_CD24;
+		break;
+
+	case 32:
+		bpp_config |= OCFB_CTRL_CD32;
+		break;
+
+	default:
+		dev_err(dev, "no bpp specified\n");
+		break;
+	}
+
+	/* maximum (8) VBL (video memory burst length) */
+	bpp_config |= OCFB_CTRL_VBL8;
+
+	/* Enable output */
+	ocfb_writereg(fbdev, OCFB_CTRL, (OCFB_CTRL_VEN | bpp_config));
+
+	return 0;
+}
+
+static int ocfb_setcolreg(unsigned regno, unsigned red, unsigned green,
+			  unsigned blue, unsigned transp,
+			  struct fb_info *info)
+{
+	struct ocfb_par *par = (struct ocfb_par *)info->par;
+	u32 color;
+
+	if (regno >= info->cmap.len) {
+		dev_err(info->device, "regno >= cmap.len\n");
+		return 1;
+	}
+
+	if (info->var.grayscale) {
+		/* grayscale = 0.30*R + 0.59*G + 0.11*B */
+		red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
+	}
+
+	red >>= (16 - info->var.red.length);
+	green >>= (16 - info->var.green.length);
+	blue >>= (16 - info->var.blue.length);
+	transp >>= (16 - info->var.transp.length);
+
+	if (info->var.bits_per_pixel = 8 && !info->var.grayscale) {
+		regno <<= 2;
+		color = (red << 16) | (green << 8) | blue;
+		ocfb_writereg(par->pal_adr, regno, color);
+	} else {
+		((u32 *)(info->pseudo_palette))[regno] +			(red << info->var.red.offset) |
+			(green << info->var.green.offset) |
+			(blue << info->var.blue.offset) |
+			(transp << info->var.transp.offset);
+	}
+
+	return 0;
+}
+
+static int ocfb_init_fix(struct ocfb_dev *fbdev)
+{
+	struct fb_var_screeninfo *var = &fbdev->info.var;
+	struct fb_fix_screeninfo *fix = &fbdev->info.fix;
+
+	strcpy(fix->id, OCFB_NAME);
+
+	fix->line_length = var->xres * var->bits_per_pixel/8;
+	fix->smem_len = fix->line_length * var->yres;
+	fix->type = FB_TYPE_PACKED_PIXELS;
+
+	if (var->bits_per_pixel = 8 && !var->grayscale)
+		fix->visual = FB_VISUAL_PSEUDOCOLOR;
+	else
+		fix->visual = FB_VISUAL_TRUECOLOR;
+
+	return 0;
+}
+
+static int ocfb_init_var(struct ocfb_dev *fbdev)
+{
+	struct fb_var_screeninfo *var = &fbdev->info.var;
+
+	var->accel_flags = FB_ACCEL_NONE;
+	var->activate = FB_ACTIVATE_NOW;
+	var->xres_virtual = var->xres;
+	var->yres_virtual = var->yres;
+
+	switch (var->bits_per_pixel) {
+	case 8:
+		var->transp.offset = 0;
+		var->transp.length = 0;
+		var->red.offset = 0;
+		var->red.length = 8;
+		var->green.offset = 0;
+		var->green.length = 8;
+		var->blue.offset = 0;
+		var->blue.length = 8;
+		break;
+
+	case 16:
+		var->transp.offset = 0;
+		var->transp.length = 0;
+		var->red.offset = 11;
+		var->red.length = 5;
+		var->green.offset = 5;
+		var->green.length = 6;
+		var->blue.offset = 0;
+		var->blue.length  = 5;
+		break;
+
+	case 24:
+		var->transp.offset = 0;
+		var->transp.length = 0;
+		var->red.offset = 16;
+		var->red.length = 8;
+		var->green.offset = 8;
+		var->green.length = 8;
+		var->blue.offset = 0;
+		var->blue.length = 8;
+		break;
+
+	case 32:
+		var->transp.offset = 24;
+		var->transp.length = 8;
+		var->red.offset = 16;
+		var->red.length = 8;
+		var->green.offset = 8;
+		var->green.length = 8;
+		var->blue.offset = 0;
+		var->blue.length = 8;
+		break;
+	}
+
+	return 0;
+}
+
+static struct fb_ops ocfb_ops = {
+	.owner		= THIS_MODULE,
+	.fb_setcolreg	= ocfb_setcolreg,
+	.fb_fillrect	= cfb_fillrect,
+	.fb_copyarea	= cfb_copyarea,
+	.fb_imageblit	= cfb_imageblit,
+};
+
+static int ocfb_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+	struct ocfb_dev *fbdev;
+	struct ocfb_par *par = &ocfb_par_priv;
+	struct resource *res;
+	struct resource *mmio;
+	int fbsize;
+
+	fbdev = devm_kzalloc(&pdev->dev, sizeof(*fbdev), GFP_KERNEL);
+	if (!fbdev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, fbdev);
+
+	fbdev->info.fbops = &ocfb_ops;
+	fbdev->info.var = ocfb_var;
+	fbdev->info.fix = ocfb_fix;
+	fbdev->info.device = &pdev->dev;
+	fbdev->info.par = par;
+
+	/* Video mode setup */
+	if (!fb_find_mode(&fbdev->info.var, &fbdev->info, mode_option,
+			  NULL, 0, &default_mode, 16)) {
+		dev_err(&pdev->dev, "No valid video modes found\n");
+		return -EINVAL;
+	}
+	ocfb_init_var(fbdev);
+	ocfb_init_fix(fbdev);
+
+	/* Request I/O resource */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "I/O resource request failed\n");
+		return -ENXIO;
+	}
+	fbdev->regs_phys = res->start;
+	fbdev->regs_phys_size = resource_size(res);
+	mmio = devm_request_mem_region(&pdev->dev, res->start,
+				       resource_size(res), res->name);
+	if (!mmio) {
+		dev_err(&pdev->dev, "I/O memory space request failed\n");
+		return -ENXIO;
+	}
+	fbdev->regs = devm_ioremap_nocache(&pdev->dev, mmio->start,
+					   resource_size(mmio));
+	if (!fbdev->regs) {
+		dev_err(&pdev->dev, "I/O memory remap request failed\n");
+		return -ENXIO;
+	}
+	par->pal_adr = fbdev->regs + OCFB_PALETTE;
+
+	/* Allocate framebuffer memory */
+	fbsize = fbdev->info.fix.smem_len;
+	fbdev->fb_virt = dma_alloc_coherent(&pdev->dev, PAGE_ALIGN(fbsize),
+					    &fbdev->fb_phys, GFP_KERNEL);
+	if (!fbdev->fb_virt) {
+		dev_err(&pdev->dev,
+			"Frame buffer memory allocation failed\n");
+		ret = -ENOMEM;
+		goto err_release;
+	}
+	fbdev->info.fix.smem_start = fbdev->fb_phys;
+	fbdev->info.screen_base = (void __iomem *)fbdev->fb_virt;
+	fbdev->info.pseudo_palette = fbdev->pseudo_palette;
+
+	/* Clear framebuffer */
+	memset_io((void __iomem *)fbdev->fb_virt, 0, fbsize);
+
+	/* Setup and enable the framebuffer */
+	ocfb_setupfb(fbdev);
+
+	if (fbdev->little_endian)
+		fbdev->info.flags |= FBINFO_FOREIGN_ENDIAN;
+
+	/* Allocate color map */
+	ret = fb_alloc_cmap(&fbdev->info.cmap, PALETTE_SIZE, 0);
+	if (ret) {
+		dev_err(&pdev->dev, "Color map allocation failed\n");
+		goto err_dma_free;
+	}
+
+	/* Register framebuffer */
+	ret = register_framebuffer(&fbdev->info);
+	if (ret) {
+		dev_err(&pdev->dev, "Framebuffer registration failed\n");
+		goto err_dealloc_cmap;
+	}
+
+	return 0;
+
+err_dealloc_cmap:
+	fb_dealloc_cmap(&fbdev->info.cmap);
+
+err_dma_free:
+	dma_free_coherent(&pdev->dev, PAGE_ALIGN(fbsize), fbdev->fb_virt,
+			  fbdev->fb_phys);
+err_release:
+	release_mem_region(fbdev->regs_phys, fbdev->regs_phys_size);
+
+	return ret;
+}
+
+static int ocfb_remove(struct platform_device *pdev)
+{
+	struct ocfb_dev *fbdev = platform_get_drvdata(pdev);
+
+	unregister_framebuffer(&fbdev->info);
+	fb_dealloc_cmap(&fbdev->info.cmap);
+	dma_free_coherent(&pdev->dev, PAGE_ALIGN(fbdev->info.fix.smem_len),
+			  fbdev->fb_virt, fbdev->fb_phys);
+
+	/* Disable display */
+	ocfb_writereg(fbdev, OCFB_CTRL, 0);
+
+	release_mem_region(fbdev->regs_phys, fbdev->regs_phys_size);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct of_device_id ocfb_match[] = {
+	{ .compatible = "opencores,ocfb", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ocfb_match);
+
+static struct platform_driver ocfb_driver = {
+	.probe  = ocfb_probe,
+	.remove	= ocfb_remove,
+	.driver = {
+		.name = "ocfb_fb",
+		.of_match_table = ocfb_match,
+	}
+};
+
+/*
+ * Init and exit routines
+ */
+static int __init ocfb_init(void)
+{
+#ifndef MODULE
+	char *option = NULL;
+
+	if (fb_get_options("ocfb", &option))
+		return -ENODEV;
+	ocfb_setup(option);
+#endif
+	return platform_driver_register(&ocfb_driver);
+}
+
+static void __exit ocfb_exit(void)
+{
+	platform_driver_unregister(&ocfb_driver);
+}
+
+module_init(ocfb_init);
+module_exit(ocfb_exit);
+
+MODULE_AUTHOR("Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>");
+MODULE_DESCRIPTION("OpenCores VGA/LCD 2.0 frame buffer driver");
+MODULE_LICENSE("GPL v2");
+module_param(mode_option, charp, 0);
+MODULE_PARM_DESC(mode_option, "Video mode ('<xres>x<yres>[-<bpp>][@refresh]')");
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH] video: backlight: Remove backlight sysfs uevent
From: Henrique de Moraes Holschuh @ 2013-11-22 11:36 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Jingoo Han, 'Kyungmin Park',
	'Henrique de Moraes Holschuh', linux-fbdev, linux-kernel,
	kay, 'Richard Purdie', ibm-acpi-devel,
	platform-driver-x86
In-Reply-To: <20131121143326.GA19773@srcf.ucam.org>

On Thu, 21 Nov 2013, Matthew Garrett wrote:
> On Thu, Nov 21, 2013 at 09:43:32AM -0200, Henrique de Moraes Holschuh wrote:
> > With this patchset applied, as far as I can tell anything that used to be
> > uevent-driven by the backlight class will break: when a process changes the
> > backlight using sysfs, other processes will not be notified of the change
> > anymore.  This patchset seems to break backlight uevent support in such a
> > way that basically renders the entire thing useless and you might as well
> > just remove uevent support entirely.
> 
> The uevent support was initially added to handle systems where pressing 
> a hotkey generates an event (good) but the firmware automatically 
> changes the brightness (bad). I have absolutely no idea why I added 
> BACKLIGHT_UPDATE_SYSFS - BACKLIGHT_UPDATE_HOTKEY solves the problem I 
> was trying to solve. I'm not aware of any userspace that relies on 
> BACKLIGHT_UPDATE_SYSFS.

Well, either we have userspace that rely on the uevents, or we don't. 

If we don't have any uevent users of the backlight notifications, we might
as well just rip out the feature entirely and replace it with something with
a proper design.  But that would mean all OSD is being done by time-based
open-read-close polling of sysfs or keyed to input events (and therefore
half-baked).

However, if we do have anything that rely on the uevents, it needs
BACKLIGHT_UPDATE_SYSFS.  Without it, there will be no notifications when the
backlight level is changed through sysfs.  And we *DO* have applications
that change the backlight level through sysfs.  From the top of my head, I
know KDE does when it starts, and also as a response to power management
events.  Also, userspace hotkey daemons do use the sysfs interface.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply

* Re: [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888
From: Daniel Vetter @ 2013-11-22 16:17 UTC (permalink / raw)
  To: Keith Packard
  Cc: Linux Fbdev development list, intel-gfx, dri-devel,
	linaro-mm-sig@lists.linaro.org, Mesa Dev,
	linux-media@vger.kernel.org
In-Reply-To: <86d2lsem3m.fsf@miki.keithp.com>

On Fri, Nov 22, 2013 at 12:01 PM, Keith Packard <keithp@keithp.com> wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
>
>> Hm, where do we have the canonical source for all these fourcc codes? I'm
>> asking since we have our own copy in the kernel as drm_fourcc.h, and that
>> one is part of the userspace ABI since we use it to pass around
>> framebuffer formats and format lists.
>
> I think it's the kernel? I really don't know, as the whole notion of
> fourcc codes seems crazy to me...
>
> Feel free to steal this code and stick it in the kernel if you like.

Well, I wasn't ever in favour of using fourcc codes since they're just
not standardized at all, highly redundant in some cases and also miss
lots of stuff we actually need (like all the rgb formats).

Cc'ing the heck out of this to get kernel people to hopefully notice.
Maybe someone takes charge of this ... Otherwise meh.

>> Just afraid to create long-term maintainance madness here with the
>> kernel's iron thou-shalt-not-break-userspace-ever rule ... Not likely
>> we'll ever accept srgb for framebuffers though.
>
> Would suck to collide with something we do want though.

Yeah, it'd suck. But given how fourcc works we probably have that
already, just haven't noticed yet :(
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply

* Re: [PATCH] video: backlight: Remove backlight sysfs uevent
From: Matthew Garrett @ 2013-11-22 17:15 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Jingoo Han, 'Kyungmin Park',
	'Henrique de Moraes Holschuh', linux-fbdev, linux-kernel,
	kay, 'Richard Purdie', ibm-acpi-devel,
	platform-driver-x86
In-Reply-To: <20131122113601.GB27196@khazad-dum.debian.net>

On Fri, Nov 22, 2013 at 09:36:01AM -0200, Henrique de Moraes Holschuh wrote:
> On Thu, 21 Nov 2013, Matthew Garrett wrote:
> > The uevent support was initially added to handle systems where pressing 
> > a hotkey generates an event (good) but the firmware automatically 
> > changes the brightness (bad). I have absolutely no idea why I added 
> > BACKLIGHT_UPDATE_SYSFS - BACKLIGHT_UPDATE_HOTKEY solves the problem I 
> > was trying to solve. I'm not aware of any userspace that relies on 
> > BACKLIGHT_UPDATE_SYSFS.
> 
> Well, either we have userspace that rely on the uevents, or we don't. 

We have userspace that relies on uevents of type 
BACKLIGHT_UPDATE_HOTKEY. I don't know that we have userspace that relies 
on uevents of type BACKLIGHT_UPDATE_SYSFS.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply

* Re: [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888
From: Ville Syrjälä @ 2013-11-22 17:46 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Keith Packard, Linux Fbdev development list, intel-gfx, dri-devel,
	linaro-mm-sig@lists.linaro.org, Mesa Dev,
	linux-media@vger.kernel.org
In-Reply-To: <CAKMK7uEqHKOmMFXZLKno1q08X1B=U7XcJiExHaHbO9VdMeCihQ@mail.gmail.com>

On Fri, Nov 22, 2013 at 05:17:37PM +0100, Daniel Vetter wrote:
> On Fri, Nov 22, 2013 at 12:01 PM, Keith Packard <keithp@keithp.com> wrote:
> > Daniel Vetter <daniel@ffwll.ch> writes:
> >
> >> Hm, where do we have the canonical source for all these fourcc codes? I'm
> >> asking since we have our own copy in the kernel as drm_fourcc.h, and that
> >> one is part of the userspace ABI since we use it to pass around
> >> framebuffer formats and format lists.
> >
> > I think it's the kernel? I really don't know, as the whole notion of
> > fourcc codes seems crazy to me...
> >
> > Feel free to steal this code and stick it in the kernel if you like.
> 
> Well, I wasn't ever in favour of using fourcc codes since they're just
> not standardized at all, highly redundant in some cases and also miss
> lots of stuff we actually need (like all the rgb formats).

I also argued against them, but some people wanted them for whatever
reason. And since I didn't want to argue for several years about the
subject, I just gave in and made the drm pixel formats fourcss. But
given that I just pulled the fourccs out of my ass, we can't really
cross use them between different subsystems anyway. So if we just
consider all the different fourcc namespaces totally distinct, we're
not going to have any problems.

Personally I can promise that I will _not_ be checking Mesa/whatever
code for conflicting fourccs when I need to add a new one to drm_fourcc.h.
There, now I've given fair warning and if things explode later it won't be
my fault.

However if someone wants to emulate the drm fourcc style for whatever
reason, there is a distinct pattern how I cooked them up. Well, a few
different patterns depending whether it's RGB,YUV,packed,planar etc.

> 
> Cc'ing the heck out of this to get kernel people to hopefully notice.
> Maybe someone takes charge of this ... Otherwise meh.
> 
> >> Just afraid to create long-term maintainance madness here with the
> >> kernel's iron thou-shalt-not-break-userspace-ever rule ... Not likely
> >> we'll ever accept srgb for framebuffers though.
> >
> > Would suck to collide with something we do want though.
> 
> Yeah, it'd suck. But given how fourcc works we probably have that
> already, just haven't noticed yet :(
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply

* Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888
From: Kristian Høgsberg @ 2013-11-22 22:12 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Keith Packard, Linux Fbdev development list, intel-gfx, dri-devel,
	linaro-mm-sig@lists.linaro.org, Mesa Dev,
	linux-media@vger.kernel.org
In-Reply-To: <CAKMK7uEqHKOmMFXZLKno1q08X1B=U7XcJiExHaHbO9VdMeCihQ@mail.gmail.com>

On Fri, Nov 22, 2013 at 05:17:37PM +0100, Daniel Vetter wrote:
> On Fri, Nov 22, 2013 at 12:01 PM, Keith Packard <keithp@keithp.com> wrote:
> > Daniel Vetter <daniel@ffwll.ch> writes:
> >
> >> Hm, where do we have the canonical source for all these fourcc codes? I'm
> >> asking since we have our own copy in the kernel as drm_fourcc.h, and that
> >> one is part of the userspace ABI since we use it to pass around
> >> framebuffer formats and format lists.
> >
> > I think it's the kernel? I really don't know, as the whole notion of
> > fourcc codes seems crazy to me...
> >
> > Feel free to steal this code and stick it in the kernel if you like.
> 
> Well, I wasn't ever in favour of using fourcc codes since they're just
> not standardized at all, highly redundant in some cases and also miss
> lots of stuff we actually need (like all the rgb formats).

These drm codes are not fourcc codes in any other way than that
they're defined by creating a 32 bit value by picking four characters.
I don't know what PTSD triggers people have from hearing "fourcc", but
it seems severe.  Forget all that, these codes are DRM specific
defines that are not inteded to match anything anybody else does.  It
doesn't matter if these match of conflict with v4l, fourcc.org,
wikipedia.org or what the amiga did.  They're just tokens that let us
define succintly what the pixel format of a kms framebuffer is and
tell the kernel.

I don't know what else you'd propose?  Pass an X visual in the ioctl?
An EGL config?  This is our name space, we can add stuff as we need
(as Keith is doing here). include/uapi/drm/drm_fourcc.h is the
canonical source for these values and we should add
DRM_FORMAT_SARGB8888 there to make sure we don't clash.

Why are these codes in mesa (and gbm and wl_drm protocol) then?
Because it turns out that once you have an stable and established
namespace for pixel formats (and a kernel userspace header is about as
stable and established as it gets) it makes a lot of sense to reuse
those values.

I already explained to Keith why we use different sets of format codes
in the DRI interface, but it's always fun to slam other peoples code.
Anyway, it's pretty simple, the __DRI_IMAGE_FORMAT_* defines predate
the introduction of drm_fourcc.h.  When we later added suport for
planar YUV __DRIimages, the kernel had picked up drm_fourcc.h after a
long sad bikeshedding flamewar, which included the planar formats we
needed.  At this point we could continue using our custom
__DRI_IMAGE_FORMAT_* defines or we could switch to the tokens that we
had finally converged on.  But don't let me ruin a good old snide remark.

> Cc'ing the heck out of this to get kernel people to hopefully notice.
> Maybe someone takes charge of this ... Otherwise meh.

I don't know what you want to change.  These values are already kernel
ABI, we use them in drmAddFB2, and again, I don't understand what
problem you're seeing.

Kristian

> >> Just afraid to create long-term maintainance madness here with the
> >> kernel's iron thou-shalt-not-break-userspace-ever rule ... Not likely
> >> we'll ever accept srgb for framebuffers though.
> >
> > Would suck to collide with something we do want though.
> 
> Yeah, it'd suck. But given how fourcc works we probably have that
> already, just haven't noticed yet :(
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888
From: Ville Syrjälä @ 2013-11-22 23:05 UTC (permalink / raw)
  To: Kristian Høgsberg
  Cc: Daniel Vetter, intel-gfx, Linux Fbdev development list, dri-devel,
	linaro-mm-sig@lists.linaro.org, Mesa Dev,
	linux-media@vger.kernel.org
In-Reply-To: <20131122221213.GA3234@tokamak.local>

On Fri, Nov 22, 2013 at 02:12:13PM -0800, Kristian Høgsberg wrote:
> On Fri, Nov 22, 2013 at 05:17:37PM +0100, Daniel Vetter wrote:
> > On Fri, Nov 22, 2013 at 12:01 PM, Keith Packard <keithp@keithp.com> wrote:
> > > Daniel Vetter <daniel@ffwll.ch> writes:
> > >
> > >> Hm, where do we have the canonical source for all these fourcc codes? I'm
> > >> asking since we have our own copy in the kernel as drm_fourcc.h, and that
> > >> one is part of the userspace ABI since we use it to pass around
> > >> framebuffer formats and format lists.
> > >
> > > I think it's the kernel? I really don't know, as the whole notion of
> > > fourcc codes seems crazy to me...
> > >
> > > Feel free to steal this code and stick it in the kernel if you like.
> > 
> > Well, I wasn't ever in favour of using fourcc codes since they're just
> > not standardized at all, highly redundant in some cases and also miss
> > lots of stuff we actually need (like all the rgb formats).
> 
> These drm codes are not fourcc codes in any other way than that
> they're defined by creating a 32 bit value by picking four characters.
> I don't know what PTSD triggers people have from hearing "fourcc", but
> it seems severe.  Forget all that, these codes are DRM specific
> defines that are not inteded to match anything anybody else does.  It
> doesn't matter if these match of conflict with v4l, fourcc.org,
> wikipedia.org or what the amiga did.  They're just tokens that let us
> define succintly what the pixel format of a kms framebuffer is and
> tell the kernel.
> 
> I don't know what else you'd propose?  Pass an X visual in the ioctl?
> An EGL config?  This is our name space, we can add stuff as we need
> (as Keith is doing here). include/uapi/drm/drm_fourcc.h is the
> canonical source for these values and we should add
> DRM_FORMAT_SARGB8888 there to make sure we don't clash.

What is this format anyway? -ENODOCS

If its just an srgb version of ARGB8888, then I wouldn't really want it
in drm_fourcc.h. I expect colorspacy stuff will be handled by various
crtc/plane properties in the kernel so we don't need to encode that
stuff into the fb format.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply

* Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888
From: Keith Packard @ 2013-11-22 23:36 UTC (permalink / raw)
  To: Kristian Høgsberg, Daniel Vetter
  Cc: Linux Fbdev development list, intel-gfx, dri-devel,
	linaro-mm-sig@lists.linaro.org, Mesa Dev,
	linux-media@vger.kernel.org
In-Reply-To: <20131122221213.GA3234@tokamak.local>

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

Kristian Høgsberg <hoegsberg@gmail.com> writes:

> I already explained to Keith why we use different sets of format codes
> in the DRI interface, but it's always fun to slam other peoples code.

As we discussed, my complaint isn't so much about __DRI_IMAGE_FOURCC,
but the fact that the __DRIimage interfaces use *both*
__DRI_IMAGE_FOURCC and __DRI_IMAGE_FORMAT at different times.

Ok, here's a fine thing we can actually fix -- the pattern that mesa
uses all over the place in converting formats looks like this (not to
pick on anyone, it's repeated everywhere, this is just the first one I
found in gbm_dri.c):

	static uint32_t
        gbm_dri_to_gbm_format(uint32_t dri_format)
	{
	   uint32_t ret = 0;
	
	   switch (dri_format) {
	   case __DRI_IMAGE_FORMAT_RGB565:
	      ret = GBM_FORMAT_RGB565;
	      break;
	   case __DRI_IMAGE_FORMAT_XRGB8888:
	      ret = GBM_FORMAT_XRGB8888;
	      break;
	   case __DRI_IMAGE_FORMAT_ARGB8888:
	      ret = GBM_FORMAT_ARGB8888;
	      break;
	   case __DRI_IMAGE_FORMAT_ABGR8888:
	      ret = GBM_FORMAT_ABGR8888;
	      break;
	   default:
	      ret = 0;
	      break;
	   }
	
	   return ret;
	}

The problem here is that any unknown incoming formats get translated to
garbage (0) outgoing. With fourcc codes, there is the slight advantage
that 0 is never a legal value, but it sure would be nice to print a
warning or even abort if you get a format code you don't understand as
there's no way 0 is ever going to do what you want.

Anyone have a preference? Abort? Print an error? Silently continue to do
the wrong thing?

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

^ permalink raw reply

* Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888
From: Keith Packard @ 2013-11-22 23:43 UTC (permalink / raw)
  To: Ville Syrjälä, Kristian Høgsberg
  Cc: Linux Fbdev development list, intel-gfx, dri-devel,
	linaro-mm-sig@lists.linaro.org, Mesa Dev,
	linux-media@vger.kernel.org
In-Reply-To: <20131122230504.GK10036@intel.com>

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

Ville Syrjälä <ville.syrjala@linux.intel.com> writes:

> What is this format anyway? -ENODOCS

Same as MESA_FORMAT_SARGB8 and __DRI_IMAGE_FORMAT_SARGB8 :-)

> If its just an srgb version of ARGB8888, then I wouldn't really want it
> in drm_fourcc.h. I expect colorspacy stuff will be handled by various
> crtc/plane properties in the kernel so we don't need to encode that
> stuff into the fb format.

It's not any different from splitting YUV codes from RGB codes; a
different color encoding with the same bitfields. And, for mesa, it's
necessary to differentiate between ARGB and SARGB within the same format
code given how the API is structured. So, we have choices:

 1) Let Mesa define it's own fourcc codes and risk future accidental
    collisions
 
 2) Rewrite piles of mesa code to split out the color encoding from the
    bitfield information and pass it separately.

 3) Add "reasonable" format codes like this to the kernel fourcc list.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

^ permalink raw reply

* Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888
From: Ville Syrjälä @ 2013-11-23  1:10 UTC (permalink / raw)
  To: Keith Packard
  Cc: Kristian Høgsberg, Linux Fbdev development list, intel-gfx,
	dri-devel, linaro-mm-sig@lists.linaro.org, Mesa Dev,
	linux-media@vger.kernel.org
In-Reply-To: <86pppsvw8e.fsf@miki.keithp.com>

On Fri, Nov 22, 2013 at 03:43:13PM -0800, Keith Packard wrote:
> Ville Syrjälä <ville.syrjala@linux.intel.com> writes:
> 
> > What is this format anyway? -ENODOCS
> 
> Same as MESA_FORMAT_SARGB8 and __DRI_IMAGE_FORMAT_SARGB8 :-)
> 
> > If its just an srgb version of ARGB8888, then I wouldn't really want it
> > in drm_fourcc.h. I expect colorspacy stuff will be handled by various
> > crtc/plane properties in the kernel so we don't need to encode that
> > stuff into the fb format.
> 
> It's not any different from splitting YUV codes from RGB codes;

Not really. Saying something is YUV (or rather Y'CbCr) doesn't
actually tell you the color space. It just tells you whether the
information is encoded as R+G+B or Y+Cb+Cr. How you convert between
them is another matter. You need to know the gamma, color primaries,
chroma siting for sub-sampled YCbCr formats, etc.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply

* PRIVATE MESSAGE
From: OSBORNES SOLICITORS LLP @ 2013-11-23 10:36 UTC (permalink / raw)
  To: linux-fbdev

-- 
This is to inform you that an inheritance was bequeathed in your
favour. Letters were posted to you to this regards, but returned
undelivered. Kindly contact me once you recieve this email for more
information.

Sincerely
Barr Mark Freedman

^ permalink raw reply

* Re: [ibm-acpi-devel] [PATCH] video: backlight: Remove backlight sysfs uevent
From: Henrique de Moraes Holschuh @ 2013-11-24  0:40 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-fbdev, 'Kyungmin Park', kay, Jingoo Han,
	'Henrique de Moraes Holschuh', linux-kernel,
	platform-driver-x86, ibm-acpi-devel, 'Richard Purdie'
In-Reply-To: <20131122171556.GA15680@srcf.ucam.org>

On Fri, 22 Nov 2013, Matthew Garrett wrote:
> On Fri, Nov 22, 2013 at 09:36:01AM -0200, Henrique de Moraes Holschuh wrote:
> > On Thu, 21 Nov 2013, Matthew Garrett wrote:
> > > The uevent support was initially added to handle systems where pressing 
> > > a hotkey generates an event (good) but the firmware automatically 
> > > changes the brightness (bad). I have absolutely no idea why I added 
> > > BACKLIGHT_UPDATE_SYSFS - BACKLIGHT_UPDATE_HOTKEY solves the problem I 
> > > was trying to solve. I'm not aware of any userspace that relies on 
> > > BACKLIGHT_UPDATE_SYSFS.
> > 
> > Well, either we have userspace that rely on the uevents, or we don't. 
> 
> We have userspace that relies on uevents of type 
> BACKLIGHT_UPDATE_HOTKEY. I don't know that we have userspace that relies 
> on uevents of type BACKLIGHT_UPDATE_SYSFS.

Any OSD application would have to rely on both uevent types, or it is broken
(and to test that, just write a level to sysfs and watch the OSD app fail to
tell you about the backlight level change...)

I don't know about other types of applications, though.  What other type of
applications pay attention to backlight uevents?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply

* Re: [ibm-acpi-devel] [PATCH] video: backlight: Remove backlight sysfs uevent
From: Matthew Garrett @ 2013-11-24  1:02 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: linux-fbdev, 'Kyungmin Park', kay, Jingoo Han,
	'Henrique de Moraes Holschuh', linux-kernel,
	platform-driver-x86, ibm-acpi-devel, 'Richard Purdie'
In-Reply-To: <20131124004015.GA19499@khazad-dum.debian.net>

On Sat, Nov 23, 2013 at 10:40:15PM -0200, Henrique de Moraes Holschuh wrote:
> On Fri, 22 Nov 2013, Matthew Garrett wrote:
> > We have userspace that relies on uevents of type 
> > BACKLIGHT_UPDATE_HOTKEY. I don't know that we have userspace that relies 
> > on uevents of type BACKLIGHT_UPDATE_SYSFS.
> 
> Any OSD application would have to rely on both uevent types, or it is broken
> (and to test that, just write a level to sysfs and watch the OSD app fail to
> tell you about the backlight level change...)

Right, OSDs are supposed to respond to keypresses, not arbitrary changes 
of backlight. If the user's just echoed 8 into brightness, they know 
they set the brightness to 8 - they don't need an OSD to tell them that. 
BACKLIGHT_UPDATE_HOTKEY is when the firmware itself has changed the 
brightness in response to a keypress, and so reporting the keypress 
would result in additional backlight changes.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply

* Re: [ibm-acpi-devel] [PATCH] video: backlight: Remove backlight sysfs uevent
From: Henrique de Moraes Holschuh @ 2013-11-24  3:53 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-fbdev, 'Kyungmin Park', kay, Jingoo Han,
	'Henrique de Moraes Holschuh', linux-kernel,
	platform-driver-x86, ibm-acpi-devel, 'Richard Purdie'
In-Reply-To: <20131124010257.GA6376@srcf.ucam.org>

On Sun, 24 Nov 2013, Matthew Garrett wrote:
> On Sat, Nov 23, 2013 at 10:40:15PM -0200, Henrique de Moraes Holschuh wrote:
> > On Fri, 22 Nov 2013, Matthew Garrett wrote:
> > > We have userspace that relies on uevents of type 
> > > BACKLIGHT_UPDATE_HOTKEY. I don't know that we have userspace that relies 
> > > on uevents of type BACKLIGHT_UPDATE_SYSFS.
> > 
> > Any OSD application would have to rely on both uevent types, or it is broken
> > (and to test that, just write a level to sysfs and watch the OSD app fail to
> > tell you about the backlight level change...)
> 
> Right, OSDs are supposed to respond to keypresses, not arbitrary changes 
> of backlight. If the user's just echoed 8 into brightness, they know 
> they set the brightness to 8 - they don't need an OSD to tell them that. 

It is not just the user that sets the brightness.

Still, if you're sure that all userspace users react only to the hotkey type
of event, removing the sysfs one won't break anything any further.

But it will be *really* annoying the day we revisit this because someone
started abusing the hotkey uevent and we have to deploy a proper fix (rate
limiting or switching to a proper event report interface that doesn't use
uevents).

> BACKLIGHT_UPDATE_HOTKEY is when the firmware itself has changed the 
> brightness in response to a keypress, and so reporting the keypress 
> would result in additional backlight changes.

Yeah, I know that bug quite well, thinkpads were the first victims of
idiotic feedback event loops caused by braindead userspace.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply

* Re: [PATCH v3] video: add OpenCores VGA/LCD framebuffer driver
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-11-24 14:12 UTC (permalink / raw)
  To: Stefan Kristiansson; +Cc: linux-kernel, linux-fbdev, tomi.valkeinen
In-Reply-To: <1385094870-6962-1-git-send-email-stefan.kristiansson@saunalahti.fi>

On 06:34 Fri 22 Nov     , Stefan Kristiansson wrote:
> This adds support for the VGA/LCD core available from OpenCores:
> http://opencores.org/project,vga_lcd
> 
> The driver have been tested together with both OpenRISC and
> ARM (socfpga) processors.
> 
> Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>

Tomi hold on this one I need check fee stuff on it
> ---
> Changes in v2:
> - Add Microblaze as an example user and fix a typo in Xilinx Zynq
> 
> Changes in v3:
> - Use devm_kzalloc instead of kzalloc
> - Remove superflous MODULE #ifdef
> ---
>  drivers/video/Kconfig  |  17 ++
>  drivers/video/Makefile |   1 +
>  drivers/video/ocfb.c   | 471 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 489 insertions(+)
>  create mode 100644 drivers/video/ocfb.c
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 84b685f..3b3f31e 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -979,6 +979,23 @@ config FB_PVR2
>  	  (<file:drivers/video/pvr2fb.c>). Please see the file
>  	  <file:Documentation/fb/pvr2fb.txt>.
>  
> +config FB_OPENCORES
> +	tristate "OpenCores VGA/LCD core 2.0 framebuffer support"
> +	depends on FB
> +	select FB_CFB_FILLRECT
> +	select FB_CFB_COPYAREA
> +	select FB_CFB_IMAGEBLIT
> +	default n
non need

Best Regards,
J.
> +	help
> +	  This enables support for the OpenCores VGA/LCD core.
> +
> +	  The OpenCores VGA/LCD core is typically used together with
> +	  softcore CPUs (e.g. OpenRISC or Microblaze) or hard processor
> +	  systems (e.g. Altera socfpga or Xilinx Zynq) on FPGAs.
> +
> +	  The source code and specification for the core is available at
> +	  <http://opencores.org/project,vga_lcd>
> +
>  config FB_S1D13XXX
>  	tristate "Epson S1D13XXX framebuffer support"
>  	depends on FB
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index e8bae8d..ae17ddf 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -150,6 +150,7 @@ obj-$(CONFIG_FB_NUC900)           += nuc900fb.o
>  obj-$(CONFIG_FB_JZ4740)		  += jz4740_fb.o
>  obj-$(CONFIG_FB_PUV3_UNIGFX)      += fb-puv3.o
>  obj-$(CONFIG_FB_HYPERV)		  += hyperv_fb.o
> +obj-$(CONFIG_FB_OPENCORES)	  += ocfb.o
>  
>  # Platform or fallback drivers go here
>  obj-$(CONFIG_FB_UVESA)            += uvesafb.o
> diff --git a/drivers/video/ocfb.c b/drivers/video/ocfb.c
> new file mode 100644
> index 0000000..3bf5f67
> --- /dev/null
> +++ b/drivers/video/ocfb.c
> @@ -0,0 +1,471 @@
> +/*
> + * OpenCores VGA/LCD 2.0 core frame buffer driver
> + *
> + * Copyright (C) 2013 Stefan Kristiansson, stefan.kristiansson@saunalahti.fi
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2.  This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +
> +#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/mm.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/fb.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +
> +/* OCFB register defines */
> +#define OCFB_CTRL	0x000
> +#define OCFB_STAT	0x004
> +#define OCFB_HTIM	0x008
> +#define OCFB_VTIM	0x00c
> +#define OCFB_HVLEN	0x010
> +#define OCFB_VBARA	0x014
> +#define OCFB_PALETTE	0x800
> +
> +#define OCFB_CTRL_VEN	0x00000001 /* Video Enable */
> +#define OCFB_CTRL_HIE	0x00000002 /* HSync Interrupt Enable */
> +#define OCFB_CTRL_PC	0x00000800 /* 8-bit Pseudo Color Enable*/
> +#define OCFB_CTRL_CD8	0x00000000 /* Color Depth 8 */
> +#define OCFB_CTRL_CD16	0x00000200 /* Color Depth 16 */
> +#define OCFB_CTRL_CD24	0x00000400 /* Color Depth 24 */
> +#define OCFB_CTRL_CD32	0x00000600 /* Color Depth 32 */
> +#define OCFB_CTRL_VBL1	0x00000000 /* Burst Length 1 */
> +#define OCFB_CTRL_VBL2	0x00000080 /* Burst Length 2 */
> +#define OCFB_CTRL_VBL4	0x00000100 /* Burst Length 4 */
> +#define OCFB_CTRL_VBL8	0x00000180 /* Burst Length 8 */
> +
> +#define PALETTE_SIZE	256
> +
> +#define OCFB_NAME	"OC VGA/LCD"
> +
> +static char *mode_option;
> +
> +static const struct fb_videomode default_mode = {
> +	/* 640x480 @ 60 Hz, 31.5 kHz hsync */
> +	NULL, 60, 640, 480, 39721, 40, 24, 32, 11, 96, 2,
> +	0, FB_VMODE_NONINTERLACED
> +};
> +
> +struct ocfb_dev {
> +	struct fb_info info;
> +	/* Physical and virtual addresses of control regs */
> +	phys_addr_t regs_phys;
> +	int regs_phys_size;
> +	void __iomem *regs;
> +	/* flag indicating whether the regs are little endian accessed */
> +	int little_endian;
> +	/* Physical and virtual addresses of framebuffer */
> +	phys_addr_t fb_phys;
> +	void __iomem *fb_virt;
> +	u32 pseudo_palette[PALETTE_SIZE];
> +};
> +
> +struct ocfb_par {
> +	void __iomem *pal_adr;
> +};
> +
> +static struct ocfb_par ocfb_par_priv;
> +
> +static struct fb_var_screeninfo ocfb_var;
> +static struct fb_fix_screeninfo ocfb_fix;
> +
> +#ifndef MODULE
> +static int __init ocfb_setup(char *options)
> +{
> +	char *curr_opt;
> +
> +	if (!options || !*options)
> +		return 0;
> +
> +	while ((curr_opt = strsep(&options, ",")) != NULL) {
> +		if (!*curr_opt)
> +			continue;
> +		mode_option = curr_opt;
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
> +static inline u32 ocfb_readreg(struct ocfb_dev *fbdev, loff_t offset)
> +{
> +	if (fbdev->little_endian)
> +		return ioread32(fbdev->regs + offset);
> +	else
> +		return ioread32be(fbdev->regs + offset);
> +}
> +
> +static void ocfb_writereg(struct ocfb_dev *fbdev, loff_t offset, u32 data)
> +{
> +	if (fbdev->little_endian)
> +		iowrite32(data, fbdev->regs + offset);
> +	else
> +		iowrite32be(data, fbdev->regs + offset);
> +}
> +
> +static int ocfb_setupfb(struct ocfb_dev *fbdev)
> +{
> +	unsigned long bpp_config;
> +	struct fb_var_screeninfo *var = &fbdev->info.var;
> +	struct device *dev = fbdev->info.device;
> +	u32 hlen;
> +	u32 vlen;
> +
> +	/* Disable display */
> +	ocfb_writereg(fbdev, OCFB_CTRL, 0);
> +
> +	/* Register framebuffer address */
> +	fbdev->little_endian = 0;
> +	ocfb_writereg(fbdev, OCFB_VBARA, fbdev->fb_phys);
> +
> +	/* Detect endianess */
> +	if (ocfb_readreg(fbdev, OCFB_VBARA) != fbdev->fb_phys) {
> +		fbdev->little_endian = 1;
> +		ocfb_writereg(fbdev, OCFB_VBARA, fbdev->fb_phys);
> +	}
> +
> +	/* Horizontal timings */
> +	ocfb_writereg(fbdev, OCFB_HTIM, (var->hsync_len - 1) << 24 |
> +		      (var->right_margin - 1) << 16 | (var->xres - 1));
> +
> +	/* Vertical timings */
> +	ocfb_writereg(fbdev, OCFB_VTIM, (var->vsync_len - 1) << 24 |
> +		      (var->lower_margin - 1) << 16 | (var->yres - 1));
> +
> +	/* Total length of frame */
> +	hlen = var->left_margin + var->right_margin + var->hsync_len +
> +		var->xres;
> +
> +	vlen = var->upper_margin + var->lower_margin + var->vsync_len +
> +		var->yres;
> +
> +	ocfb_writereg(fbdev, OCFB_HVLEN, (hlen - 1) << 16 | (vlen - 1));
> +
> +	bpp_config = OCFB_CTRL_CD8;
> +	switch (var->bits_per_pixel) {
> +	case 8:
> +		if (!var->grayscale)
> +			bpp_config |= OCFB_CTRL_PC;  /* enable palette */
> +		break;
> +
> +	case 16:
> +		bpp_config |= OCFB_CTRL_CD16;
> +		break;
> +
> +	case 24:
> +		bpp_config |= OCFB_CTRL_CD24;
> +		break;
> +
> +	case 32:
> +		bpp_config |= OCFB_CTRL_CD32;
> +		break;
> +
> +	default:
> +		dev_err(dev, "no bpp specified\n");
> +		break;
> +	}
> +
> +	/* maximum (8) VBL (video memory burst length) */
> +	bpp_config |= OCFB_CTRL_VBL8;
> +
> +	/* Enable output */
> +	ocfb_writereg(fbdev, OCFB_CTRL, (OCFB_CTRL_VEN | bpp_config));
> +
> +	return 0;
> +}
> +
> +static int ocfb_setcolreg(unsigned regno, unsigned red, unsigned green,
> +			  unsigned blue, unsigned transp,
> +			  struct fb_info *info)
> +{
> +	struct ocfb_par *par = (struct ocfb_par *)info->par;
> +	u32 color;
> +
> +	if (regno >= info->cmap.len) {
> +		dev_err(info->device, "regno >= cmap.len\n");
> +		return 1;
> +	}
> +
> +	if (info->var.grayscale) {
> +		/* grayscale = 0.30*R + 0.59*G + 0.11*B */
> +		red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
> +	}
> +
> +	red >>= (16 - info->var.red.length);
> +	green >>= (16 - info->var.green.length);
> +	blue >>= (16 - info->var.blue.length);
> +	transp >>= (16 - info->var.transp.length);
> +
> +	if (info->var.bits_per_pixel = 8 && !info->var.grayscale) {
> +		regno <<= 2;
> +		color = (red << 16) | (green << 8) | blue;
> +		ocfb_writereg(par->pal_adr, regno, color);
> +	} else {
> +		((u32 *)(info->pseudo_palette))[regno] > +			(red << info->var.red.offset) |
> +			(green << info->var.green.offset) |
> +			(blue << info->var.blue.offset) |
> +			(transp << info->var.transp.offset);
> +	}
> +
> +	return 0;
> +}
> +
> +static int ocfb_init_fix(struct ocfb_dev *fbdev)
> +{
> +	struct fb_var_screeninfo *var = &fbdev->info.var;
> +	struct fb_fix_screeninfo *fix = &fbdev->info.fix;
> +
> +	strcpy(fix->id, OCFB_NAME);
> +
> +	fix->line_length = var->xres * var->bits_per_pixel/8;
> +	fix->smem_len = fix->line_length * var->yres;
> +	fix->type = FB_TYPE_PACKED_PIXELS;
> +
> +	if (var->bits_per_pixel = 8 && !var->grayscale)
> +		fix->visual = FB_VISUAL_PSEUDOCOLOR;
> +	else
> +		fix->visual = FB_VISUAL_TRUECOLOR;
> +
> +	return 0;
> +}
> +
> +static int ocfb_init_var(struct ocfb_dev *fbdev)
> +{
> +	struct fb_var_screeninfo *var = &fbdev->info.var;
> +
> +	var->accel_flags = FB_ACCEL_NONE;
> +	var->activate = FB_ACTIVATE_NOW;
> +	var->xres_virtual = var->xres;
> +	var->yres_virtual = var->yres;
> +
> +	switch (var->bits_per_pixel) {
> +	case 8:
> +		var->transp.offset = 0;
> +		var->transp.length = 0;
> +		var->red.offset = 0;
> +		var->red.length = 8;
> +		var->green.offset = 0;
> +		var->green.length = 8;
> +		var->blue.offset = 0;
> +		var->blue.length = 8;
> +		break;
> +
> +	case 16:
> +		var->transp.offset = 0;
> +		var->transp.length = 0;
> +		var->red.offset = 11;
> +		var->red.length = 5;
> +		var->green.offset = 5;
> +		var->green.length = 6;
> +		var->blue.offset = 0;
> +		var->blue.length  = 5;
> +		break;
> +
> +	case 24:
> +		var->transp.offset = 0;
> +		var->transp.length = 0;
> +		var->red.offset = 16;
> +		var->red.length = 8;
> +		var->green.offset = 8;
> +		var->green.length = 8;
> +		var->blue.offset = 0;
> +		var->blue.length = 8;
> +		break;
> +
> +	case 32:
> +		var->transp.offset = 24;
> +		var->transp.length = 8;
> +		var->red.offset = 16;
> +		var->red.length = 8;
> +		var->green.offset = 8;
> +		var->green.length = 8;
> +		var->blue.offset = 0;
> +		var->blue.length = 8;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct fb_ops ocfb_ops = {
> +	.owner		= THIS_MODULE,
> +	.fb_setcolreg	= ocfb_setcolreg,
> +	.fb_fillrect	= cfb_fillrect,
> +	.fb_copyarea	= cfb_copyarea,
> +	.fb_imageblit	= cfb_imageblit,
> +};
> +
> +static int ocfb_probe(struct platform_device *pdev)
> +{
> +	int ret = 0;
> +	struct ocfb_dev *fbdev;
> +	struct ocfb_par *par = &ocfb_par_priv;
> +	struct resource *res;
> +	struct resource *mmio;
> +	int fbsize;
> +
> +	fbdev = devm_kzalloc(&pdev->dev, sizeof(*fbdev), GFP_KERNEL);
> +	if (!fbdev)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, fbdev);
> +
> +	fbdev->info.fbops = &ocfb_ops;
> +	fbdev->info.var = ocfb_var;
> +	fbdev->info.fix = ocfb_fix;
> +	fbdev->info.device = &pdev->dev;
> +	fbdev->info.par = par;
> +
> +	/* Video mode setup */
> +	if (!fb_find_mode(&fbdev->info.var, &fbdev->info, mode_option,
> +			  NULL, 0, &default_mode, 16)) {
> +		dev_err(&pdev->dev, "No valid video modes found\n");
> +		return -EINVAL;
> +	}
> +	ocfb_init_var(fbdev);
> +	ocfb_init_fix(fbdev);
> +
> +	/* Request I/O resource */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "I/O resource request failed\n");
> +		return -ENXIO;
> +	}
> +	fbdev->regs_phys = res->start;
> +	fbdev->regs_phys_size = resource_size(res);
> +	mmio = devm_request_mem_region(&pdev->dev, res->start,
> +				       resource_size(res), res->name);
> +	if (!mmio) {
> +		dev_err(&pdev->dev, "I/O memory space request failed\n");
> +		return -ENXIO;
> +	}
> +	fbdev->regs = devm_ioremap_nocache(&pdev->dev, mmio->start,
> +					   resource_size(mmio));
> +	if (!fbdev->regs) {
> +		dev_err(&pdev->dev, "I/O memory remap request failed\n");
> +		return -ENXIO;
> +	}
> +	par->pal_adr = fbdev->regs + OCFB_PALETTE;
> +
> +	/* Allocate framebuffer memory */
> +	fbsize = fbdev->info.fix.smem_len;
> +	fbdev->fb_virt = dma_alloc_coherent(&pdev->dev, PAGE_ALIGN(fbsize),
> +					    &fbdev->fb_phys, GFP_KERNEL);
> +	if (!fbdev->fb_virt) {
> +		dev_err(&pdev->dev,
> +			"Frame buffer memory allocation failed\n");
> +		ret = -ENOMEM;
> +		goto err_release;
> +	}
> +	fbdev->info.fix.smem_start = fbdev->fb_phys;
> +	fbdev->info.screen_base = (void __iomem *)fbdev->fb_virt;
> +	fbdev->info.pseudo_palette = fbdev->pseudo_palette;
> +
> +	/* Clear framebuffer */
> +	memset_io((void __iomem *)fbdev->fb_virt, 0, fbsize);
> +
> +	/* Setup and enable the framebuffer */
> +	ocfb_setupfb(fbdev);
> +
> +	if (fbdev->little_endian)
> +		fbdev->info.flags |= FBINFO_FOREIGN_ENDIAN;
> +
> +	/* Allocate color map */
> +	ret = fb_alloc_cmap(&fbdev->info.cmap, PALETTE_SIZE, 0);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Color map allocation failed\n");
> +		goto err_dma_free;
> +	}
> +
> +	/* Register framebuffer */
> +	ret = register_framebuffer(&fbdev->info);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Framebuffer registration failed\n");
> +		goto err_dealloc_cmap;
> +	}
> +
> +	return 0;
> +
> +err_dealloc_cmap:
> +	fb_dealloc_cmap(&fbdev->info.cmap);
> +
> +err_dma_free:
> +	dma_free_coherent(&pdev->dev, PAGE_ALIGN(fbsize), fbdev->fb_virt,
> +			  fbdev->fb_phys);
> +err_release:
> +	release_mem_region(fbdev->regs_phys, fbdev->regs_phys_size);
> +
> +	return ret;
> +}
> +
> +static int ocfb_remove(struct platform_device *pdev)
> +{
> +	struct ocfb_dev *fbdev = platform_get_drvdata(pdev);
> +
> +	unregister_framebuffer(&fbdev->info);
> +	fb_dealloc_cmap(&fbdev->info.cmap);
> +	dma_free_coherent(&pdev->dev, PAGE_ALIGN(fbdev->info.fix.smem_len),
> +			  fbdev->fb_virt, fbdev->fb_phys);
> +
> +	/* Disable display */
> +	ocfb_writereg(fbdev, OCFB_CTRL, 0);
> +
> +	release_mem_region(fbdev->regs_phys, fbdev->regs_phys_size);
> +	platform_set_drvdata(pdev, NULL);
> +
> +	return 0;
> +}
> +
> +static struct of_device_id ocfb_match[] = {
> +	{ .compatible = "opencores,ocfb", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ocfb_match);
> +
> +static struct platform_driver ocfb_driver = {
> +	.probe  = ocfb_probe,
> +	.remove	= ocfb_remove,
> +	.driver = {
> +		.name = "ocfb_fb",
> +		.of_match_table = ocfb_match,
> +	}
> +};
> +
> +/*
> + * Init and exit routines
> + */
> +static int __init ocfb_init(void)
> +{
> +#ifndef MODULE
> +	char *option = NULL;
> +
> +	if (fb_get_options("ocfb", &option))
> +		return -ENODEV;
> +	ocfb_setup(option);
> +#endif
> +	return platform_driver_register(&ocfb_driver);
> +}
> +
> +static void __exit ocfb_exit(void)
> +{
> +	platform_driver_unregister(&ocfb_driver);
> +}
> +
> +module_init(ocfb_init);
> +module_exit(ocfb_exit);
> +
> +MODULE_AUTHOR("Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>");
> +MODULE_DESCRIPTION("OpenCores VGA/LCD 2.0 frame buffer driver");
> +MODULE_LICENSE("GPL v2");
> +module_param(mode_option, charp, 0);
> +MODULE_PARM_DESC(mode_option, "Video mode ('<xres>x<yres>[-<bpp>][@refresh]')");
> -- 
> 1.8.3.2
> 

^ permalink raw reply

* Re: [PATCH 1/6] OMAPDSS: APPLY: set infos to dirty on enable
From: Archit Taneja @ 2013-11-25  1:18 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-fbdev, linux-omap
In-Reply-To: <1384779009-10512-2-git-send-email-tomi.valkeinen@ti.com>

On Monday 18 November 2013 06:20 PM, Tomi Valkeinen wrote:
> Currently when DISPC is suspended, the driver stores all DISPC registers
> to memory, so that they can be restored on resume. This is a bad way to
> handle suspend/resume, as it's prone to failures and requires somewhat
> large amount of extra space to store the registers.
>
> A better approach is to program the DISPC from scratch when resuming.
> This can be easily accomplished in apply layer by setting the manager
> and overlay infos to dirty when the manager is to be enabled.

I guess this won't work if we wanted to support DSI command mode 
displays. I.e, only shut DSS off and keep the panel up. In that case, we 
would need to mark the flags dirty in dss_mgr_start_update_compat().

We don't support the above use case anyway, but just pointing out :)

Archit

>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/video/omap2/dss/apply.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>
> diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
> index 60758db..6ab4cb6 100644
> --- a/drivers/video/omap2/dss/apply.c
> +++ b/drivers/video/omap2/dss/apply.c
> @@ -1072,6 +1072,7 @@ static void dss_setup_fifos(void)
>   static int dss_mgr_enable_compat(struct omap_overlay_manager *mgr)
>   {
>   	struct mgr_priv_data *mp = get_mgr_priv(mgr);
> +	struct omap_overlay *ovl;
>   	unsigned long flags;
>   	int r;
>
> @@ -1091,6 +1092,27 @@ static int dss_mgr_enable_compat(struct omap_overlay_manager *mgr)
>   		goto err;
>   	}
>
> +	/*
> +	 * Mark the info & extra_info dirty for the manager and its enabled
> +	 * overlays to force register writes. This ensures that the relevant
> +	 * registers are set after DSS has been off and the registers have been
> +	 * reset.
> +	 */
> +
> +	mp->info_dirty = true;
> +	mp->extra_info_dirty = true;
> +
> +	list_for_each_entry(ovl, &mgr->overlays, list) {
> +		struct ovl_priv_data *op = get_ovl_priv(ovl);
> +
> +		if (!op->enabled)
> +			continue;
> +
> +		op->info_dirty = true;
> +		op->extra_info_dirty = true;
> +		dispc_ovl_set_channel_out(ovl->id, mgr->id);
> +	}
> +
>   	dss_setup_fifos();
>
>   	dss_write_regs();
>


^ permalink raw reply

* Re: [PATCH 6/6] OMAPDSS: use runtime PM's autosuspend
From: Archit Taneja @ 2013-11-25  1:41 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-fbdev, linux-omap
In-Reply-To: <1384779009-10512-7-git-send-email-tomi.valkeinen@ti.com>

On Monday 18 November 2013 06:20 PM, Tomi Valkeinen wrote:
> Use runtime PM's autosuspend support with delay of 100ms.
>
> This will prevent the driver from turning the DSS modules off and on
> multiple times e.g. when loading the module.

Could you explain this a bit more?

Are you saying that when we insert the omapdss module, we have a lot of 
runtime_get/put pairs in probe, which leads to us excessive writing of 
DISPC the registers during resume, and the autosuspend feature would 
delay the effect of runtime_put() for a while?

Also, do we need to do this for all the platform devices? Could we use 
autosuspend only for the parent platform device, and the children 
platform devices don't use it? Or am I understanding things wrongly here?

Archit

>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/video/omap2/dss/dispc.c | 5 ++++-
>   drivers/video/omap2/dss/dsi.c   | 5 ++++-
>   drivers/video/omap2/dss/dss.c   | 5 ++++-
>   drivers/video/omap2/dss/dss.h   | 2 ++
>   drivers/video/omap2/dss/hdmi4.c | 5 ++++-
>   drivers/video/omap2/dss/rfbi.c  | 5 ++++-
>   drivers/video/omap2/dss/venc.c  | 5 ++++-
>   7 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 58f3626..0643eb0 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -258,7 +258,8 @@ void dispc_runtime_put(void)
>
>   	DSSDBG("dispc_runtime_put\n");
>
> -	r = pm_runtime_put_sync(&dispc.pdev->dev);
> +	pm_runtime_mark_last_busy(&dispc.pdev->dev);
> +	r = pm_runtime_put_autosuspend(&dispc.pdev->dev);
>   	WARN_ON(r < 0 && r != -ENOSYS);
>   }
>   EXPORT_SYMBOL(dispc_runtime_put);
> @@ -3440,6 +3441,8 @@ static int __init omap_dispchw_probe(struct platform_device *pdev)
>   	}
>
>   	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_set_autosuspend_delay(&pdev->dev, DSS_AUTOSUSPEND_DELAY);
> +	pm_runtime_use_autosuspend(&pdev->dev);
>
>   	r = dispc_runtime_get();
>   	if (r)
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index 6b30a58..19e4cad 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -1110,7 +1110,8 @@ void dsi_runtime_put(struct platform_device *dsidev)
>
>   	DSSDBG("dsi_runtime_put\n");
>
> -	r = pm_runtime_put_sync(&dsi->pdev->dev);
> +	pm_runtime_mark_last_busy(&dsi->pdev->dev);
> +	r = pm_runtime_put_autosuspend(&dsi->pdev->dev);
>   	WARN_ON(r < 0 && r != -ENOSYS);
>   }
>
> @@ -5416,6 +5417,8 @@ static int omap_dsihw_probe(struct platform_device *dsidev)
>   		return r;
>
>   	pm_runtime_enable(&dsidev->dev);
> +	pm_runtime_set_autosuspend_delay(&dsidev->dev, DSS_AUTOSUSPEND_DELAY);
> +	pm_runtime_use_autosuspend(&dsidev->dev);
>
>   	r = dsi_runtime_get(dsidev);
>   	if (r)
> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
> index fa7bc00..d3b0122 100644
> --- a/drivers/video/omap2/dss/dss.c
> +++ b/drivers/video/omap2/dss/dss.c
> @@ -689,7 +689,8 @@ static void dss_runtime_put(void)
>
>   	DSSDBG("dss_runtime_put\n");
>
> -	r = pm_runtime_put_sync(&dss.pdev->dev);
> +	pm_runtime_mark_last_busy(&dss.pdev->dev);
> +	r = pm_runtime_put_autosuspend(&dss.pdev->dev);
>   	WARN_ON(r < 0 && r != -ENOSYS && r != -EBUSY);
>   }
>
> @@ -821,6 +822,8 @@ static int __init omap_dsshw_probe(struct platform_device *pdev)
>   		goto err_setup_clocks;
>
>   	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_set_autosuspend_delay(&pdev->dev, DSS_AUTOSUSPEND_DELAY);
> +	pm_runtime_use_autosuspend(&pdev->dev);
>
>   	r = dss_runtime_get();
>   	if (r)
> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> index af83c4d..96505f0 100644
> --- a/drivers/video/omap2/dss/dss.h
> +++ b/drivers/video/omap2/dss/dss.h
> @@ -73,6 +73,8 @@
>   #define FLD_MOD(orig, val, start, end) \
>   	(((orig) & ~FLD_MASK(start, end)) | FLD_VAL(val, start, end))
>
> +#define DSS_AUTOSUSPEND_DELAY 100	/* in ms */
> +
>   enum dss_io_pad_mode {
>   	DSS_IO_PAD_MODE_RESET,
>   	DSS_IO_PAD_MODE_RFBI,
> diff --git a/drivers/video/omap2/dss/hdmi4.c b/drivers/video/omap2/dss/hdmi4.c
> index f1a45fe..f255641 100644
> --- a/drivers/video/omap2/dss/hdmi4.c
> +++ b/drivers/video/omap2/dss/hdmi4.c
> @@ -77,7 +77,8 @@ static void hdmi_runtime_put(void)
>
>   	DSSDBG("hdmi_runtime_put\n");
>
> -	r = pm_runtime_put_sync(&hdmi.pdev->dev);
> +	pm_runtime_mark_last_busy(&hdmi.pdev->dev);
> +	r = pm_runtime_put_autosuspend(&hdmi.pdev->dev);
>   	WARN_ON(r < 0 && r != -ENOSYS);
>   }
>
> @@ -631,6 +632,8 @@ static int omapdss_hdmihw_probe(struct platform_device *pdev)
>   	}
>
>   	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_set_autosuspend_delay(&pdev->dev, DSS_AUTOSUSPEND_DELAY);
> +	pm_runtime_use_autosuspend(&pdev->dev);
>
>   	hdmi_init_output(pdev);
>
> diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
> index 74d3ed1..20dcbfb 100644
> --- a/drivers/video/omap2/dss/rfbi.c
> +++ b/drivers/video/omap2/dss/rfbi.c
> @@ -147,7 +147,8 @@ static void rfbi_runtime_put(void)
>
>   	DSSDBG("rfbi_runtime_put\n");
>
> -	r = pm_runtime_put_sync(&rfbi.pdev->dev);
> +	pm_runtime_mark_last_busy(&rfbi.pdev->dev);
> +	r = pm_runtime_put_autosuspend(&rfbi.pdev->dev);
>   	WARN_ON(r < 0 && r != -ENOSYS);
>   }
>
> @@ -981,6 +982,8 @@ static int omap_rfbihw_probe(struct platform_device *pdev)
>   	clk_put(clk);
>
>   	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_set_autosuspend_delay(&pdev->dev, DSS_AUTOSUSPEND_DELAY);
> +	pm_runtime_use_autosuspend(&pdev->dev);
>
>   	r = rfbi_runtime_get();
>   	if (r)
> diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
> index e173961..bbd35bb 100644
> --- a/drivers/video/omap2/dss/venc.c
> +++ b/drivers/video/omap2/dss/venc.c
> @@ -410,7 +410,8 @@ static void venc_runtime_put(void)
>
>   	DSSDBG("venc_runtime_put\n");
>
> -	r = pm_runtime_put_sync(&venc.pdev->dev);
> +	pm_runtime_mark_last_busy(&venc.pdev->dev);
> +	r = pm_runtime_put_autosuspend(&venc.pdev->dev);
>   	WARN_ON(r < 0 && r != -ENOSYS);
>   }
>
> @@ -835,6 +836,8 @@ static int omap_venchw_probe(struct platform_device *pdev)
>   		return r;
>
>   	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_set_autosuspend_delay(&pdev->dev, DSS_AUTOSUSPEND_DELAY);
> +	pm_runtime_use_autosuspend(&pdev->dev);
>
>   	r = venc_runtime_get();
>   	if (r)
>


^ permalink raw reply

* [PATCH 1/4] video: asiliantfb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-11-25  2:21 UTC (permalink / raw)
  To: linux-fbdev

The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/asiliantfb.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/video/asiliantfb.c b/drivers/video/asiliantfb.c
index d611f1a..7e8ddf0 100644
--- a/drivers/video/asiliantfb.c
+++ b/drivers/video/asiliantfb.c
@@ -589,7 +589,6 @@ static void asiliantfb_remove(struct pci_dev *dp)
 	fb_dealloc_cmap(&p->cmap);
 	iounmap(p->screen_base);
 	release_mem_region(pci_resource_start(dp, 0), pci_resource_len(dp, 0));
-	pci_set_drvdata(dp, NULL);
 	framebuffer_release(p);
 }
 
-- 
1.7.10.4



^ permalink raw reply related

* [PATCH 2/4] video: nvidiafb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-11-25  3:19 UTC (permalink / raw)
  To: linux-fbdev

The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/nvidia/nvidia.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/video/nvidia/nvidia.c b/drivers/video/nvidia/nvidia.c
index ff22871..def0412 100644
--- a/drivers/video/nvidia/nvidia.c
+++ b/drivers/video/nvidia/nvidia.c
@@ -1461,7 +1461,6 @@ static void nvidiafb_remove(struct pci_dev *pd)
 	pci_release_regions(pd);
 	kfree(info->pixmap.addr);
 	framebuffer_release(info);
-	pci_set_drvdata(pd, NULL);
 	NVTRACE_LEAVE();
 }
 
-- 
1.7.10.4



^ permalink raw reply related

* [PATCH 3/4] video: rivafb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-11-25  3:19 UTC (permalink / raw)
  To: linux-fbdev

The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/riva/fbdev.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/video/riva/fbdev.c b/drivers/video/riva/fbdev.c
index a5514ac..8a8d7f0 100644
--- a/drivers/video/riva/fbdev.c
+++ b/drivers/video/riva/fbdev.c
@@ -2128,7 +2128,6 @@ static void rivafb_remove(struct pci_dev *pd)
 	pci_release_regions(pd);
 	kfree(info->pixmap.addr);
 	framebuffer_release(info);
-	pci_set_drvdata(pd, NULL);
 	NVTRACE_LEAVE();
 }
 
-- 
1.7.10.4



^ permalink raw reply related

* [PATCH 4/4] video: vmlfb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-11-25  3:20 UTC (permalink / raw)
  To: linux-fbdev

The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/vermilion/vermilion.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/video/vermilion/vermilion.c b/drivers/video/vermilion/vermilion.c
index 09a1366..048a666 100644
--- a/drivers/video/vermilion/vermilion.c
+++ b/drivers/video/vermilion/vermilion.c
@@ -383,7 +383,6 @@ static void vmlfb_disable_mmio(struct vml_par *par)
 static void vmlfb_release_devices(struct vml_par *par)
 {
 	if (atomic_dec_and_test(&par->refcount)) {
-		pci_set_drvdata(par->vdc, NULL);
 		pci_disable_device(par->gpu);
 		pci_disable_device(par->vdc);
 	}
-- 
1.7.10.4



^ permalink raw reply related

* Re: [PATCHv9][ 1/3] Input: tsc2007: Add device tree support.
From: Denis Carikli @ 2013-11-25  8:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20131119211102.GA14888@core.coreip.homeip.net>

On 11/19/2013 10:11 PM, Dmitry Torokhov wrote:
 > Input: tsc2007 misc fixes
 >
 > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
 >
 > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Tested-by: Denis Carikli <denis@eukrea.com>

Denis.

^ permalink raw reply


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