Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 02/10] arm: at91: move platfarm_data to include/linux/platform_data/atmel.h
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-11-07 11:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1352287374-25176-1-git-send-email-plagnioj@jcrosoft.com>

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: linux-ide@vger.kernel.org
Cc: linux-input@vger.kernel.org
Cc: linux-mmc@vger.kernel.org
Cc: linux-can@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-pcmcia@lists.infradead.org
Cc: rtc-linux@googlegroups.com
Cc: spi-devel-general@lists.sourceforge.net
Cc: linux-serial@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
---
HI all,

	If it's ok with everyone this will go via at91
	with the patch serie than clean up the include/mach

	For preparation to switch to arm multiarch kernel

Best Regards,
J.
 arch/arm/mach-at91/include/mach/board.h     |   55 ----------------------
 arch/avr32/mach-at32ap/include/mach/board.h |    7 ---
 drivers/ata/pata_at91.c                     |    2 +-
 drivers/input/touchscreen/atmel_tsadcc.c    |    2 +-
 drivers/mmc/host/atmel-mci.c                |    2 +-
 drivers/net/can/at91_can.c                  |    3 +-
 drivers/net/ethernet/cadence/at91_ether.c   |    2 +-
 drivers/pcmcia/at91_cf.c                    |    2 +-
 drivers/rtc/rtc-at91sam9.c                  |    2 +-
 drivers/spi/spi-atmel.c                     |    2 +-
 drivers/tty/serial/atmel_serial.c           |    2 +-
 drivers/usb/gadget/at91_udc.c               |    2 +-
 drivers/usb/gadget/atmel_usba_udc.c         |    2 +-
 drivers/usb/host/ohci-at91.c                |    2 +-
 drivers/video/atmel_lcdfb.c                 |    2 +-
 include/linux/platform_data/atmel.h         |   67 +++++++++++++++++++++++++++
 16 files changed, 80 insertions(+), 76 deletions(-)

diff --git a/arch/arm/mach-at91/include/mach/board.h b/arch/arm/mach-at91/include/mach/board.h
index c55a436..662451d 100644
--- a/arch/arm/mach-at91/include/mach/board.h
+++ b/arch/arm/mach-at91/include/mach/board.h
@@ -31,42 +31,15 @@
 #ifndef __ASM_ARCH_BOARD_H
 #define __ASM_ARCH_BOARD_H
 
-#include <linux/mtd/partitions.h>
-#include <linux/device.h>
-#include <linux/i2c.h>
-#include <linux/leds.h>
-#include <linux/spi/spi.h>
-#include <linux/usb/atmel_usba_udc.h>
-#include <linux/atmel-mci.h>
-#include <sound/atmel-ac97c.h>
-#include <linux/serial.h>
-#include <linux/platform_data/macb.h>
 #include <linux/platform_data/atmel.h>
 
  /* USB Device */
-struct at91_udc_data {
-	int	vbus_pin;		/* high = host powering us */
-	u8	vbus_active_low;	/* vbus polarity */
-	u8	vbus_polled;		/* Use polling, not interrupt */
-	int	pullup_pin;		/* active = D+ pulled up */
-	u8	pullup_active_low;	/* true = pullup_pin is active low */
-};
 extern void __init at91_add_device_udc(struct at91_udc_data *data);
 
  /* USB High Speed Device */
 extern void __init at91_add_device_usba(struct usba_platform_data *data);
 
  /* Compact Flash */
-struct at91_cf_data {
-	int	irq_pin;		/* I/O IRQ */
-	int	det_pin;		/* Card detect */
-	int	vcc_pin;		/* power switching */
-	int	rst_pin;		/* card reset */
-	u8	chipselect;		/* EBI Chip Select number */
-	u8	flags;
-#define AT91_CF_TRUE_IDE	0x01
-#define AT91_IDE_SWAP_A0_A2	0x02
-};
 extern void __init at91_add_device_cf(struct at91_cf_data *data);
 
  /* MMC / SD */
@@ -86,16 +59,6 @@ extern void __init at91_add_device_mci(short mmc_id, struct mci_platform_data *d
 extern void __init at91_add_device_eth(struct macb_platform_data *data);
 
  /* USB Host */
-#define AT91_MAX_USBH_PORTS	3
-struct at91_usbh_data {
-	int		vbus_pin[AT91_MAX_USBH_PORTS];	/* port power-control pin */
-	int             overcurrent_pin[AT91_MAX_USBH_PORTS];
-	u8		ports;				/* number of ports on root hub */
-	u8              overcurrent_supported;
-	u8              vbus_pin_active_low[AT91_MAX_USBH_PORTS];
-	u8              overcurrent_status[AT91_MAX_USBH_PORTS];
-	u8              overcurrent_changed[AT91_MAX_USBH_PORTS];
-};
 extern void __init at91_add_device_usbh(struct at91_usbh_data *data);
 extern void __init at91_add_device_usbh_ohci(struct at91_usbh_data *data);
 extern void __init at91_add_device_usbh_ehci(struct at91_usbh_data *data);
@@ -124,13 +87,6 @@ extern void __init at91_register_uart(unsigned id, unsigned portnr, unsigned pin
 
 extern struct platform_device *atmel_default_console_device;
 
-struct atmel_uart_data {
-	int			num;		/* port num */
-	short			use_dma_tx;	/* use transmit DMA? */
-	short			use_dma_rx;	/* use receive DMA? */
-	void __iomem		*regs;		/* virt. base address, if any */
-	struct serial_rs485	rs485;		/* rs485 settings */
-};
 extern void __init at91_add_device_serial(void);
 
 /*
@@ -173,24 +129,13 @@ extern void __init at91_add_device_isi(struct isi_platform_data *data,
 		bool use_pck_as_mck);
 
  /* Touchscreen Controller */
-struct at91_tsadcc_data {
-	unsigned int    adc_clock;
-	u8		pendet_debounce;
-	u8		ts_sample_hold_time;
-};
 extern void __init at91_add_device_tsadcc(struct at91_tsadcc_data *data);
 
 /* CAN */
-struct at91_can_data {
-	void (*transceiver_switch)(int on);
-};
 extern void __init at91_add_device_can(struct at91_can_data *data);
 
  /* LEDs */
 extern void __init at91_gpio_leds(struct gpio_led *leds, int nr);
 extern void __init at91_pwm_leds(struct gpio_led *leds, int nr);
 
-/* FIXME: this needs a better location, but gets stuff building again */
-extern int at91_suspend_entering_slow_clock(void);
-
 #endif
diff --git a/arch/avr32/mach-at32ap/include/mach/board.h b/arch/avr32/mach-at32ap/include/mach/board.h
index 70742ec..dca9345 100644
--- a/arch/avr32/mach-at32ap/include/mach/board.h
+++ b/arch/avr32/mach-at32ap/include/mach/board.h
@@ -34,13 +34,6 @@ extern struct platform_device *atmel_default_console_device;
 #define	ATMEL_USART_CTS		0x02
 #define	ATMEL_USART_CLK		0x04
 
-struct atmel_uart_data {
-	int		num;		/* port num */
-	short		use_dma_tx;	/* use transmit DMA? */
-	short		use_dma_rx;	/* use receive DMA? */
-	void __iomem	*regs;		/* virtual base address, if any */
-	struct serial_rs485	rs485;		/* rs485 settings */
-};
 void at32_map_usart(unsigned int hw_id, unsigned int line, int flags);
 struct platform_device *at32_add_device_usart(unsigned int id);
 
diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c
index 53d3770..2a96bb2 100644
--- a/drivers/ata/pata_at91.c
+++ b/drivers/ata/pata_at91.c
@@ -27,9 +27,9 @@
 #include <linux/libata.h>
 #include <linux/platform_device.h>
 #include <linux/ata_platform.h>
+#include <linux/platform_data/atmel.h>
 
 #include <mach/at91sam9_smc.h>
-#include <mach/board.h>
 #include <asm/gpio.h>
 
 #define DRV_NAME		"pata_at91"
diff --git a/drivers/input/touchscreen/atmel_tsadcc.c b/drivers/input/touchscreen/atmel_tsadcc.c
index 201b2d2..ea392ee 100644
--- a/drivers/input/touchscreen/atmel_tsadcc.c
+++ b/drivers/input/touchscreen/atmel_tsadcc.c
@@ -22,7 +22,7 @@
 #include <linux/clk.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
-#include <mach/board.h>
+#include <linux/platform_data/atmel.h>
 #include <mach/cpu.h>
 
 /* Register definitions based on AT91SAM9RL64 preliminary draft datasheet */
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index ddf096e..8689989 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -28,6 +28,7 @@
 #include <linux/slab.h>
 #include <linux/stat.h>
 #include <linux/types.h>
+#include <linux/platform_data/atmel.h>
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/sdio.h>
@@ -40,7 +41,6 @@
 #include <asm/unaligned.h>
 
 #include <mach/cpu.h>
-#include <mach/board.h>
 
 #include "atmel-mci-regs.h"
 
diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
index fcff73a..994b6ac 100644
--- a/drivers/net/can/at91_can.c
+++ b/drivers/net/can/at91_can.c
@@ -33,12 +33,11 @@
 #include <linux/spinlock.h>
 #include <linux/string.h>
 #include <linux/types.h>
+#include <linux/platform_data/atmel.h>
 
 #include <linux/can/dev.h>
 #include <linux/can/error.h>
 
-#include <mach/board.h>
-
 #define AT91_MB_MASK(i)		((1 << (i)) - 1)
 
 /* Common registers */
diff --git a/drivers/net/ethernet/cadence/at91_ether.c b/drivers/net/ethernet/cadence/at91_ether.c
index 4e980a7..35fc6edb 100644
--- a/drivers/net/ethernet/cadence/at91_ether.c
+++ b/drivers/net/ethernet/cadence/at91_ether.c
@@ -31,6 +31,7 @@
 #include <linux/clk.h>
 #include <linux/gfp.h>
 #include <linux/phy.h>
+#include <linux/platform_data/atmel.h>
 
 #include <asm/io.h>
 #include <asm/uaccess.h>
@@ -38,7 +39,6 @@
 
 #include <mach/at91rm9200_emac.h>
 #include <asm/gpio.h>
-#include <mach/board.h>
 
 #include "at91_ether.h"
 
diff --git a/drivers/pcmcia/at91_cf.c b/drivers/pcmcia/at91_cf.c
index 9694c1e..01463c7 100644
--- a/drivers/pcmcia/at91_cf.c
+++ b/drivers/pcmcia/at91_cf.c
@@ -17,6 +17,7 @@
 #include <linux/interrupt.h>
 #include <linux/slab.h>
 #include <linux/gpio.h>
+#include <linux/platform_data/atmel.h>
 
 #include <pcmcia/ss.h>
 
@@ -24,7 +25,6 @@
 #include <asm/io.h>
 #include <asm/sizes.h>
 
-#include <mach/board.h>
 #include <mach/at91rm9200_mc.h>
 #include <mach/at91_ramc.h>
 
diff --git a/drivers/rtc/rtc-at91sam9.c b/drivers/rtc/rtc-at91sam9.c
index 2dfe7a2..e981798 100644
--- a/drivers/rtc/rtc-at91sam9.c
+++ b/drivers/rtc/rtc-at91sam9.c
@@ -19,8 +19,8 @@
 #include <linux/interrupt.h>
 #include <linux/ioctl.h>
 #include <linux/slab.h>
+#include <linux/platform_data/atmel.h>
 
-#include <mach/board.h>
 #include <mach/at91_rtt.h>
 #include <mach/cpu.h>
 
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 16d6a83..61fb0ec 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -19,9 +19,9 @@
 #include <linux/interrupt.h>
 #include <linux/spi/spi.h>
 #include <linux/slab.h>
+#include <linux/platform_data/atmel.h>
 
 #include <asm/io.h>
-#include <mach/board.h>
 #include <asm/gpio.h>
 #include <mach/cpu.h>
 
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 3d7e1ee..5660ec2 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -39,12 +39,12 @@
 #include <linux/atmel_pdc.h>
 #include <linux/atmel_serial.h>
 #include <linux/uaccess.h>
+#include <linux/platform_data/atmel.h>
 
 #include <asm/io.h>
 #include <asm/ioctls.h>
 
 #include <asm/mach/serial_at91.h>
-#include <mach/board.h>
 
 #ifdef CONFIG_ARM
 #include <mach/cpu.h>
diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
index 89d90b5..a7b042b 100644
--- a/drivers/usb/gadget/at91_udc.c
+++ b/drivers/usb/gadget/at91_udc.c
@@ -31,6 +31,7 @@
 #include <linux/usb/gadget.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
+#include <linux/platform_data/atmel.h>
 
 #include <asm/byteorder.h>
 #include <mach/hardware.h>
@@ -38,7 +39,6 @@
 #include <asm/irq.h>
 #include <asm/gpio.h>
 
-#include <mach/board.h>
 #include <mach/cpu.h>
 #include <mach/at91sam9261_matrix.h>
 #include <mach/at91_matrix.h>
diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
index 9a9bced..a7aed84 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -21,9 +21,9 @@
 #include <linux/usb/gadget.h>
 #include <linux/usb/atmel_usba_udc.h>
 #include <linux/delay.h>
+#include <linux/platform_data/atmel.h>
 
 #include <asm/gpio.h>
-#include <mach/board.h>
 
 #include "atmel_usba_udc.h"
 
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 0bf72f9..8e62f81 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -16,11 +16,11 @@
 #include <linux/platform_device.h>
 #include <linux/of_platform.h>
 #include <linux/of_gpio.h>
+#include <linux/platform_data/atmel.h>
 
 #include <mach/hardware.h>
 #include <asm/gpio.h>
 
-#include <mach/board.h>
 #include <mach/cpu.h>
 
 #ifndef CONFIG_ARCH_AT91
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 94cac9f..12cf5f3 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -19,8 +19,8 @@
 #include <linux/backlight.h>
 #include <linux/gfp.h>
 #include <linux/module.h>
+#include <linux/platform_data/atmel.h>
 
-#include <mach/board.h>
 #include <mach/cpu.h>
 #include <asm/gpio.h>
 
diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
index b0f2c56..dbd6d53 100644
--- a/include/linux/platform_data/atmel.h
+++ b/include/linux/platform_data/atmel.h
@@ -8,6 +8,49 @@
 #define __ATMEL_H__
 
 #include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/leds.h>
+#include <linux/spi/spi.h>
+#include <linux/usb/atmel_usba_udc.h>
+#include <linux/atmel-mci.h>
+#include <sound/atmel-ac97c.h>
+#include <linux/serial.h>
+#include <linux/platform_data/macb.h>
+
+ /* USB Device */
+struct at91_udc_data {
+	int	vbus_pin;		/* high = host powering us */
+	u8	vbus_active_low;	/* vbus polarity */
+	u8	vbus_polled;		/* Use polling, not interrupt */
+	int	pullup_pin;		/* active = D+ pulled up */
+	u8	pullup_active_low;	/* true = pullup_pin is active low */
+};
+
+ /* Compact Flash */
+struct at91_cf_data {
+	int	irq_pin;		/* I/O IRQ */
+	int	det_pin;		/* Card detect */
+	int	vcc_pin;		/* power switching */
+	int	rst_pin;		/* card reset */
+	u8	chipselect;		/* EBI Chip Select number */
+	u8	flags;
+#define AT91_CF_TRUE_IDE	0x01
+#define AT91_IDE_SWAP_A0_A2	0x02
+};
+
+ /* USB Host */
+#define AT91_MAX_USBH_PORTS	3
+struct at91_usbh_data {
+	int		vbus_pin[AT91_MAX_USBH_PORTS];	/* port power-control pin */
+	int             overcurrent_pin[AT91_MAX_USBH_PORTS];
+	u8		ports;				/* number of ports on root hub */
+	u8              overcurrent_supported;
+	u8              vbus_pin_active_low[AT91_MAX_USBH_PORTS];
+	u8              overcurrent_status[AT91_MAX_USBH_PORTS];
+	u8              overcurrent_changed[AT91_MAX_USBH_PORTS];
+};
 
  /* NAND / SmartMedia */
 struct atmel_nand_data {
@@ -24,4 +67,28 @@ struct atmel_nand_data {
 	unsigned int	num_parts;
 };
 
+ /* Serial */
+struct atmel_uart_data {
+	int			num;		/* port num */
+	short			use_dma_tx;	/* use transmit DMA? */
+	short			use_dma_rx;	/* use receive DMA? */
+	void __iomem		*regs;		/* virt. base address, if any */
+	struct serial_rs485	rs485;		/* rs485 settings */
+};
+
+ /* Touchscreen Controller */
+struct at91_tsadcc_data {
+	unsigned int    adc_clock;
+	u8		pendet_debounce;
+	u8		ts_sample_hold_time;
+};
+
+/* CAN */
+struct at91_can_data {
+	void (*transceiver_switch)(int on);
+};
+
+/* FIXME: this needs a better location, but gets stuff building again */
+extern int at91_suspend_entering_slow_clock(void);
+
 #endif /* __ATMEL_H__ */
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 2/2] video: exynos_dp: move exynos_dp_config_video to a workqueue
From: Ajay Kumar @ 2012-11-07 10:53 UTC (permalink / raw)
  To: linux-fbdev

This speeds up boot significantly, since it can take up to a second to
get the display up and going, and we want to keep on booting (through
some userspace) while that happens.

Signed-off-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
---
 drivers/video/exynos/exynos_dp_core.c |   30 ++++++++++++++++++------------
 drivers/video/exynos/exynos_dp_core.h |    1 +
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index f62778c..ecae6f4 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -18,6 +18,7 @@
 #include <linux/io.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
+#include <linux/workqueue.h>
 
 #include <video/exynos_dp.h>
 
@@ -752,19 +753,22 @@ static int exynos_dp_set_link_train(struct exynos_dp_device *dp,
 	return retval;
 }
 
-static int exynos_dp_config_video(struct exynos_dp_device *dp)
+static void exynos_dp_config_video(struct work_struct *work)
 {
+	struct exynos_dp_device *dp;
 	int retval = 0;
 	int timeout_loop = 0;
 	int done_count = 0;
 
+	dp = container_of(work, struct exynos_dp_device, config_work);
+
 	exynos_dp_config_video_slave_mode(dp);
 
 	exynos_dp_set_video_color_format(dp);
 
 	if (exynos_dp_get_pll_lock_status(dp) = PLL_UNLOCKED) {
 		dev_err(dp->dev, "PLL is not locked yet.\n");
-		return -EINVAL;
+		return;
 	}
 
 	for (;;) {
@@ -773,7 +777,7 @@ static int exynos_dp_config_video(struct exynos_dp_device *dp)
 			break;
 		if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
 			dev_err(dp->dev, "Timeout of video streamclk ok\n");
-			return -ETIMEDOUT;
+			return;
 		}
 
 		usleep_range(1, 2);
@@ -807,7 +811,7 @@ static int exynos_dp_config_video(struct exynos_dp_device *dp)
 		}
 		if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
 			dev_err(dp->dev, "Timeout of video streamclk ok\n");
-			return -ETIMEDOUT;
+			return;
 		}
 
 		usleep_range(1000, 1001);
@@ -815,8 +819,6 @@ static int exynos_dp_config_video(struct exynos_dp_device *dp)
 
 	if (retval != 0)
 		dev_err(dp->dev, "Video stream is not detected!\n");
-
-	return retval;
 }
 
 static void exynos_dp_enable_scramble(struct exynos_dp_device *dp, bool enable)
@@ -933,11 +935,9 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 	exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
 
 	exynos_dp_init_video(dp);
-	ret = exynos_dp_config_video(dp);
-	if (ret) {
-		dev_err(&pdev->dev, "unable to config video\n");
-		return ret;
-	}
+
+	INIT_WORK(&dp->config_work, exynos_dp_config_video);
+	schedule_work(&dp->config_work);
 
 	platform_set_drvdata(pdev, dp);
 
@@ -949,6 +949,9 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
 	struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
 	struct exynos_dp_device *dp = platform_get_drvdata(pdev);
 
+	if (work_pending(&dp->config_work))
+		flush_work_sync(&dp->config_work);
+
 	if (pdata && pdata->phy_exit)
 		pdata->phy_exit();
 
@@ -964,6 +967,9 @@ static int exynos_dp_suspend(struct device *dev)
 	struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
 	struct exynos_dp_device *dp = platform_get_drvdata(pdev);
 
+	if (work_pending(&dp->config_work))
+		flush_work_sync(&dp->config_work);
+
 	if (pdata && pdata->phy_exit)
 		pdata->phy_exit();
 
@@ -999,7 +1005,7 @@ static int exynos_dp_resume(struct device *dev)
 	exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
 
 	exynos_dp_init_video(dp);
-	exynos_dp_config_video(dp);
+	schedule_work(&dp->config_work);
 
 	return 0;
 }
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 1e646d7..303831d 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -32,6 +32,7 @@ struct exynos_dp_device {
 
 	struct video_info	*video_info;
 	struct link_train	link_train;
+	struct work_struct	config_work;
 };
 
 /* exynos_dp_reg.c */
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 1/2] video: exynos_dp: remove redundant parameters
From: Ajay Kumar @ 2012-11-07 10:53 UTC (permalink / raw)
  To: linux-fbdev

This patch cleans up few redundant parameters keeping
the same functionality intact.

Signed-off-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
---
 drivers/video/exynos/exynos_dp_core.c |   14 +++++---------
 drivers/video/exynos/exynos_dp_core.h |    9 ++-------
 drivers/video/exynos/exynos_dp_reg.c  |   23 +++++++++--------------
 3 files changed, 16 insertions(+), 30 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index d55470e..f62778c 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -752,19 +752,15 @@ static int exynos_dp_set_link_train(struct exynos_dp_device *dp,
 	return retval;
 }
 
-static int exynos_dp_config_video(struct exynos_dp_device *dp,
-			struct video_info *video_info)
+static int exynos_dp_config_video(struct exynos_dp_device *dp)
 {
 	int retval = 0;
 	int timeout_loop = 0;
 	int done_count = 0;
 
-	exynos_dp_config_video_slave_mode(dp, video_info);
+	exynos_dp_config_video_slave_mode(dp);
 
-	exynos_dp_set_video_color_format(dp, video_info->color_depth,
-			video_info->color_space,
-			video_info->dynamic_range,
-			video_info->ycbcr_coeff);
+	exynos_dp_set_video_color_format(dp);
 
 	if (exynos_dp_get_pll_lock_status(dp) = PLL_UNLOCKED) {
 		dev_err(dp->dev, "PLL is not locked yet.\n");
@@ -937,7 +933,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 	exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
 
 	exynos_dp_init_video(dp);
-	ret = exynos_dp_config_video(dp, dp->video_info);
+	ret = exynos_dp_config_video(dp);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to config video\n");
 		return ret;
@@ -1003,7 +999,7 @@ static int exynos_dp_resume(struct device *dev)
 	exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
 
 	exynos_dp_init_video(dp);
-	exynos_dp_config_video(dp, dp->video_info);
+	exynos_dp_config_video(dp);
 
 	return 0;
 }
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 57b8a65..1e646d7 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -107,11 +107,7 @@ u32 exynos_dp_get_lane3_link_training(struct exynos_dp_device *dp);
 void exynos_dp_reset_macro(struct exynos_dp_device *dp);
 void exynos_dp_init_video(struct exynos_dp_device *dp);
 
-void exynos_dp_set_video_color_format(struct exynos_dp_device *dp,
-				u32 color_depth,
-				u32 color_space,
-				u32 dynamic_range,
-				u32 ycbcr_coeff);
+void exynos_dp_set_video_color_format(struct exynos_dp_device *dp);
 int exynos_dp_is_slave_video_stream_clock_on(struct exynos_dp_device *dp);
 void exynos_dp_set_video_cr_mn(struct exynos_dp_device *dp,
 			enum clock_recovery_m_value_type type,
@@ -121,8 +117,7 @@ void exynos_dp_set_video_timing_mode(struct exynos_dp_device *dp, u32 type);
 void exynos_dp_enable_video_master(struct exynos_dp_device *dp, bool enable);
 void exynos_dp_start_video(struct exynos_dp_device *dp);
 int exynos_dp_is_video_stream_on(struct exynos_dp_device *dp);
-void exynos_dp_config_video_slave_mode(struct exynos_dp_device *dp,
-			struct video_info *video_info);
+void exynos_dp_config_video_slave_mode(struct exynos_dp_device *dp);
 void exynos_dp_enable_scrambling(struct exynos_dp_device *dp);
 void exynos_dp_disable_scrambling(struct exynos_dp_device *dp);
 
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 3f5ca8a..db4975d 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -1034,24 +1034,20 @@ void exynos_dp_init_video(struct exynos_dp_device *dp)
 	writel(reg, dp->reg_base + EXYNOS_DP_VIDEO_CTL_8);
 }
 
-void exynos_dp_set_video_color_format(struct exynos_dp_device *dp,
-			u32 color_depth,
-			u32 color_space,
-			u32 dynamic_range,
-			u32 ycbcr_coeff)
+void exynos_dp_set_video_color_format(struct exynos_dp_device *dp)
 {
 	u32 reg;
 
 	/* Configure the input color depth, color space, dynamic range */
-	reg = (dynamic_range << IN_D_RANGE_SHIFT) |
-		(color_depth << IN_BPC_SHIFT) |
-		(color_space << IN_COLOR_F_SHIFT);
+	reg = (dp->video_info->dynamic_range << IN_D_RANGE_SHIFT) |
+		(dp->video_info->color_depth << IN_BPC_SHIFT) |
+		(dp->video_info->color_space << IN_COLOR_F_SHIFT);
 	writel(reg, dp->reg_base + EXYNOS_DP_VIDEO_CTL_2);
 
 	/* Set Input Color YCbCr Coefficients to ITU601 or ITU709 */
 	reg = readl(dp->reg_base + EXYNOS_DP_VIDEO_CTL_3);
 	reg &= ~IN_YC_COEFFI_MASK;
-	if (ycbcr_coeff)
+	if (dp->video_info->ycbcr_coeff)
 		reg |= IN_YC_COEFFI_ITU709;
 	else
 		reg |= IN_YC_COEFFI_ITU601;
@@ -1178,8 +1174,7 @@ int exynos_dp_is_video_stream_on(struct exynos_dp_device *dp)
 	return 0;
 }
 
-void exynos_dp_config_video_slave_mode(struct exynos_dp_device *dp,
-			struct video_info *video_info)
+void exynos_dp_config_video_slave_mode(struct exynos_dp_device *dp)
 {
 	u32 reg;
 
@@ -1190,17 +1185,17 @@ void exynos_dp_config_video_slave_mode(struct exynos_dp_device *dp,
 
 	reg = readl(dp->reg_base + EXYNOS_DP_VIDEO_CTL_10);
 	reg &= ~INTERACE_SCAN_CFG;
-	reg |= (video_info->interlaced << 2);
+	reg |= (dp->video_info->interlaced << 2);
 	writel(reg, dp->reg_base + EXYNOS_DP_VIDEO_CTL_10);
 
 	reg = readl(dp->reg_base + EXYNOS_DP_VIDEO_CTL_10);
 	reg &= ~VSYNC_POLARITY_CFG;
-	reg |= (video_info->v_sync_polarity << 1);
+	reg |= (dp->video_info->v_sync_polarity << 1);
 	writel(reg, dp->reg_base + EXYNOS_DP_VIDEO_CTL_10);
 
 	reg = readl(dp->reg_base + EXYNOS_DP_VIDEO_CTL_10);
 	reg &= ~HSYNC_POLARITY_CFG;
-	reg |= (video_info->h_sync_polarity << 0);
+	reg |= (dp->video_info->h_sync_polarity << 0);
 	writel(reg, dp->reg_base + EXYNOS_DP_VIDEO_CTL_10);
 
 	reg = AUDIO_MODE_SPDIF_MODE | VIDEO_MODE_SLAVE_MODE;
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Tomi Valkeinen @ 2012-11-07 10:01 UTC (permalink / raw)
  To: Rob Clark
  Cc: Tomi Valkeinen, Archit Taneja, linux-omap, linux-fbdev, dri-devel
In-Reply-To: <CAF6AEGuNyqO-teda1sv8c8m9x1j8Fm13HYuWB4N3gccJeKFTFg@mail.gmail.com>

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

On 2012-11-06 16:40, Rob Clark wrote:

> I mean, similar to how we handle the subdev for dmm..  the
> omap_drm_init() does the platform_driver_register() for the dmm device
> before the platform_driver_register() for omapdrm itself, so we know
> if there is a dmm device, the driver gets probed first before omapdrm.

Well, I consider that a bit hacky too. That's not how linux device
framework is supposed to be used. I know it makes life easier to do the
registering like that, though.

> It could be a matter of iterating through a list, or something like
> this.. that is basically an implementation detail.  But the end result
> is that the order the drivers are registered is controlled so the
> probe sequence works out properly (not to mention suspend/resume
> sequence).

I feel that this kind of solution just tries to solve the generic
problem of init/suspend ordering in a single driver, instead of fixing
the device framework itself.

Or, of course it's possible that our drive architecture just sucks, and
the device framework is fine. In that case the workaround is even worse,
and we should fix our drivers.

>> I think we should support proper hotplugging of the panels. This would
>> fix the problem about init order, but it would also give us device
>> hotplug support. Obviously nobody is going to solder panel to a running
>> board, but I don't see any reason why panels, or, more likely, panels on
>> an add-on boards (like the capes being discussed in omap ml) would not
>> be hotpluggable using whatever connector is used on the particular use case.
>>
>> And even if we don't support removing of the devices, things like the
>> add-on capes could cause the panel on the cape to be identified at some
>> late time (the panel is not described in the board file or DT data, but
>> found at runtime depending on the ID of the cape). This would add
>> another step to the init sequence that should be just right, if we don't
>> support hotplug.
> 
> If capes are really hot-pluggable, then maybe it is worth thinking
> about how to make this more dynamic.  Although it is a bigger problem,
> which involves userspace being aware that connectors can dynamically
> appear/disappear.  And the dynamic disappearing is something I worry
> about more.. it adds the possibility of all sorts of interesting race
> conditions, such as connectors disappearing in the middle of modeset.
> I prefer not making things more complicated and error prone than they
> need to be.  If there is not a legitimate use case for connector hw
> dynamically appearing/disappearing then I don't think we should go
> there.  It sounds nice and simple and clean, but in reality I think it
> just introduces a whole lot of ways for things to go wrong.  A wise

Yes, I agree that it complicates things.

> man once said:
> 
> https://github.com/robclark/kernel-omap4/blob/master/Documentation/SubmittingPatches#L700

I've done things simple lots of times in the omapdss driver, only to
have to rewrite the thing in more complex way later to accommodate new
scenarios. I think it's good to write the code in a bit more generic way
than the use case at the moment of writing requires, because more often
than not, it'll save time in the future.

Hotplugging is not some abstract future scenario, we already have
hardware that could use it. For example, omap3 SDP board has a
switchable output to DVI or LCD panel. In this case we know what the two
options are, but the disabled component is still effectually removed
from the system, and plugged back in when it's enabled.

Hotplug is not a high priority item, but I do wish we get it supported
in common panel framework. Then it's at least possible to extend drm in
the future to support it.



Anyway, this makes me wonder... omapdrm currently maps the elements of
the whole video pipeline to drm elements (encoder, connector, etc).
Would it make more sense to just map the DISPC to these drm elements?
Connector would then be the output from DISPC.

This would map the drm elements to the static hardware blocks, and the
meaning of those blocks would be quite similar to what they are in the
desktop world (I guess).

The panel driver, the external chips, and the DSS internal output blocks
(dsi, dpi, ...) would be handled separately from those drm elements. The
DSS internal blocks are static, of course, but they can be effectively
considered the same way as external chips.

The omapdrm driver needs of course to access those separate elements
also, but that shouldn't be a problem. If omapdrm needs to call a
function in the panel driver, all it needs to do is go through the chain
to find the panel. Well, except if one output connected two two panels
via a bridge chip...

And if drm is at some point extended to support panel drivers, or chains
of external display entities, it would be easier to add that support.

What would it require the manage the elements like that? Would it help?
It sounds to me that this would simplify the model.

 Tomi



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

^ permalink raw reply

* [PATCH 3/3] OMAPDSS: APPLY: Remove unnecessary call to mg_clear_shadow_dirty
From: Archit Taneja @ 2012-11-07  9:29 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1352279844-10494-1-git-send-email-archit@ti.com>

When doing a manual update in dss_mgr_start_update, we clear the shadow dirty
flags. Although there isn't any harm in clearing them. The need to clear them
out here should never arrive.

When applying configurations for a manual update manager, we never do any
register writes, i.e, calls to dss_mgr_write_regs and dss_mgr_write_regs_extra
never happen while applying. We do all these writes only when we call
dss_mgr_start_update. Hence, there is never a time when the shadow registers
are dirty.

Remove the call to mg_clear_shadow_dirty.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/apply.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 37a5d22..7a7b820 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -772,8 +772,6 @@ void dss_mgr_start_update(struct omap_overlay_manager *mgr)
 
 	dispc_mgr_enable_sync(mgr->id);
 
-	mgr_clear_shadow_dirty(mgr);
-
 	spin_unlock_irqrestore(&data_lock, flags);
 }
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 2/3] OMAPDSS: APPLY: Remove unnecessary variable in dss_apply_irq_handler
From: Archit Taneja @ 2012-11-07  9:29 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1352279844-10494-1-git-send-email-archit@ti.com>

The bool was_updating is never really used for anything. It is set to the
current value of mp->updating, but not used anywhere. Remove this variable.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/apply.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index b64a083..37a5d22 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -829,7 +829,6 @@ static void dss_apply_irq_handler(void *data, u32 mask)
 	for (i = 0; i < num_mgrs; i++) {
 		struct omap_overlay_manager *mgr;
 		struct mgr_priv_data *mp;
-		bool was_updating;
 
 		mgr = omap_dss_get_overlay_manager(i);
 		mp = get_mgr_priv(mgr);
@@ -837,7 +836,6 @@ static void dss_apply_irq_handler(void *data, u32 mask)
 		if (!mp->enabled)
 			continue;
 
-		was_updating = mp->updating;
 		mp->updating = dispc_mgr_is_enabled(i);
 
 		if (!mgr_manual_update(mgr)) {
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 1/3] OMAPDSS: APPLY: Don't treat an overlay's channel out as shadow bits
From: Archit Taneja @ 2012-11-07  9:29 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1352279844-10494-1-git-send-email-archit@ti.com>

An overlay's channel out field isn't a shadow register. The TRM says that it's
taken into effect immediately. This understanding was missing and channel out
was treated as a shadow parameter, and in overlay's private data as extra info.

Program channel out bits directly in dss_ovl_set_manager(). In order to do this
safely, we need to be totally sure that the overlay is disabled in hardware. For
auto update managers, we can assume that the overlay was truly disabled at
dss_ovl_unset_manager() through the wait_pending_extra_info_updates() call.
However, when unsetting manager for an overlay that was previously connected to
a manager in manual update, we can't be sure if the overlay is truly disabled.
That is, op->enabled might not reflect the actual state of the overlay in
hardware. The older manager may require a manual update transfer to truly
disable the overlay. We expect the user of OMAPDSS to take care of this, in
OMAPDSS, we make sure that an overlay's manager isn't unset if there if
extra_info is still dirty for that overlay.

The wrong understanding of channel out bits also explains the reason why we see
sync lost when changing an overlay's manager which was previously connected to a
manual update manager. The following sequence of events caused this:

- When we disable the overlay, no register writes are actually done since the
  manager is manual update, op->enabled is set to false, and the
  extra_info_dirty flag is set. However, in hardware, the overlay is still
  enabled in both shadow and working registers.

- When we unset the manager, the software just configures the overlay's manager
  to point to NULL.

- When we set the overlay to a new manager(which is in auto update) through
  dss_ovl_set_manager, the check  for op->enabled passes, the channel field in
  extra info is set to the new manager. When we do an apply on this manager,
  the new channel out field is set in the hardware immediately, and since the
  overlay enable bit is still set in hardware, the new manager sees that the
  overlay is enabled, and tries to retrieve pixels from it, this leads to sync
  lost as it might be in the middle of processing a frame when we set the
  channel out bit.

The solution to this was to ensure that user space does another update after
disabling the overlay, this actually worked because the overlay was now truly
disabled, and an immediate write to channel out didn't impact since the manager
saw the new overlay as disabled, and doesn't try to retrieve pixels from it.

Remove channel as an extra_info field. Make dss_ovl_unset_manager more strict
about the overlay being disabled when detaching the manager. For overlays
connected to a manual update manager, unset_manager fails if we need another
update to disable the overlay.

We still need to a manual update to ensure the overlay is disabled to get change
the overlay's manager. We could work on doing a dummy update by using DISPC's
capability to gate the different video port signals. This is left for later.

Remove the comment about the sync lost issue.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/apply.c |   44 +++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 6a21443..b64a083 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -70,7 +70,6 @@ struct ovl_priv_data {
 	bool shadow_extra_info_dirty;
 
 	bool enabled;
-	enum omap_channel channel;
 	u32 fifo_low, fifo_high;
 
 	/*
@@ -617,7 +616,6 @@ static void dss_ovl_write_regs_extra(struct omap_overlay *ovl)
 	 * disabled */
 
 	dispc_ovl_enable(ovl->id, op->enabled);
-	dispc_ovl_set_channel_out(ovl->id, op->channel);
 	dispc_ovl_set_fifo_threshold(ovl->id, op->fifo_low, op->fifo_high);
 
 	mp = get_mgr_priv(ovl->manager);
@@ -1278,39 +1276,34 @@ int dss_ovl_set_manager(struct omap_overlay *ovl,
 		goto err;
 	}
 
+	r = dispc_runtime_get();
+	if (r)
+		goto err;
+
 	spin_lock_irqsave(&data_lock, flags);
 
 	if (op->enabled) {
 		spin_unlock_irqrestore(&data_lock, flags);
 		DSSERR("overlay has to be disabled to change the manager\n");
 		r = -EINVAL;
-		goto err;
+		goto err1;
 	}
 
-	op->channel = mgr->id;
-	op->extra_info_dirty = true;
+	dispc_ovl_set_channel_out(ovl->id, mgr->id);
 
 	ovl->manager = mgr;
 	list_add_tail(&ovl->list, &mgr->overlays);
 
 	spin_unlock_irqrestore(&data_lock, flags);
 
-	/* XXX: When there is an overlay on a DSI manual update display, and
-	 * the overlay is first disabled, then moved to tv, and enabled, we
-	 * seem to get SYNC_LOST_DIGIT error.
-	 *
-	 * Waiting doesn't seem to help, but updating the manual update display
-	 * after disabling the overlay seems to fix this. This hints that the
-	 * overlay is perhaps somehow tied to the LCD output until the output
-	 * is updated.
-	 *
-	 * Userspace workaround for this is to update the LCD after disabling
-	 * the overlay, but before moving the overlay to TV.
-	 */
+	dispc_runtime_put();
 
 	mutex_unlock(&apply_lock);
 
 	return 0;
+
+err1:
+	dispc_runtime_put();
 err:
 	mutex_unlock(&apply_lock);
 	return r;
@@ -1344,9 +1337,24 @@ int dss_ovl_unset_manager(struct omap_overlay *ovl)
 	/* wait for pending extra_info updates to ensure the ovl is disabled */
 	wait_pending_extra_info_updates();
 
+	/*
+	 * For a manual update display, there is no guarantee that the overlay
+	 * is really disabled in HW, we may need an extra update from this
+	 * manager before the configurations can go in. Return an error if the
+	 * overlay needed an update from the manager.
+	 *
+	 * TODO: Instead of returning an error, try to do a dummy manager update
+	 * here to disable the overlay in hardware. Use the *GATED fields in
+	 * the DISPC_CONFIG registers to do a dummy update.
+	 */
 	spin_lock_irqsave(&data_lock, flags);
 
-	op->channel = -1;
+	if (ovl_manual_update(ovl) && op->extra_info_dirty) {
+		spin_unlock_irqrestore(&data_lock, flags);
+		DSSERR("need an update to change the manager\n");
+		r = -EINVAL;
+		goto err;
+	}
 
 	ovl->manager = NULL;
 	list_del(&ovl->list);
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 0/3] OMAPDSS: APPLY: Misc Fixes
From: Archit Taneja @ 2012-11-07  9:29 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja

This series resolves a few minor issues with APPLY. Tested on a 4430sdp, checked
switching overlays between DSI command mode and HDMI displays for any unexpected
behavior.

Reference tree:
git://gitorious.org/~boddob/linux-omap-dss2/archit-dss2-clone.git 3.8/apply_fixes

Archit Taneja (3):
  OMAPDSS: APPLY: Don't treat an overlay's channel out as shadow bits
  OMAPDSS: APPLY: Remove unnecessary variable in dss_apply_irq_handler
  OMAPDSS: APPLY: Remove unnecessary call to mg_clear_shadow_dirty

 drivers/video/omap2/dss/apply.c |   48 +++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 22 deletions(-)

-- 
1.7.9.5


^ permalink raw reply

* [PATCH 3/3] OMAPDSS: DISPC: Use output width and height to calculate row/pix inc for writeback
From: Archit Taneja @ 2012-11-07  6:27 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1352268905-31093-1-git-send-email-archit@ti.com>

When calculating row and pixel increments for graphics and video pipes, we need
to consider the dimensions of the input frame to know how to read from the
buffer. Hence, we need to calculate these parameters from the input to the
pipeline.

For writeback, the row and pixel increments need to be calculated based on the
output of the writeback pipeline, i.e, the dimensions of the frame after
scaling. Ensure that dispc driver uses values of out_width and out_height when
calling calc_dma/calc_tiler_rotation_offset.

For graphics and video pipes, the original code passed the original height as
frame_height to calc_dma_rotation_offset, and not the predecimated height. This
is left as it is for now. We need to figure out why pre decimated height isn't
needed.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 664ac6f..db14f20 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2381,7 +2381,7 @@ static int dispc_ovl_setup_common(enum omap_plane plane,
 	unsigned offset0, offset1;
 	s32 row_inc;
 	s32 pix_inc;
-	u16 frame_height = height;
+	u16 frame_width, frame_height;
 	unsigned int field_offset = 0;
 	u16 in_height = height;
 	u16 in_width = width;
@@ -2449,20 +2449,28 @@ static int dispc_ovl_setup_common(enum omap_plane plane,
 	row_inc = 0;
 	pix_inc = 0;
 
+	if (plane = OMAP_DSS_WB) {
+		frame_width = out_width;
+		frame_height = out_height;
+	} else {
+		frame_width = in_width;
+		frame_height = height;
+	}
+
 	if (rotation_type = OMAP_DSS_ROT_TILER)
-		calc_tiler_rotation_offset(screen_width, in_width,
+		calc_tiler_rotation_offset(screen_width, frame_width,
 				color_mode, fieldmode, field_offset,
 				&offset0, &offset1, &row_inc, &pix_inc,
 				x_predecim, y_predecim);
 	else if (rotation_type = OMAP_DSS_ROT_DMA)
-		calc_dma_rotation_offset(rotation, mirror,
-				screen_width, in_width, frame_height,
+		calc_dma_rotation_offset(rotation, mirror, screen_width,
+				frame_width, frame_height,
 				color_mode, fieldmode, field_offset,
 				&offset0, &offset1, &row_inc, &pix_inc,
 				x_predecim, y_predecim);
 	else
 		calc_vrfb_rotation_offset(rotation, mirror,
-				screen_width, in_width, frame_height,
+				screen_width, frame_width, frame_height,
 				color_mode, fieldmode, field_offset,
 				&offset0, &offset1, &row_inc, &pix_inc,
 				x_predecim, y_predecim);
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 2/3] OMAPDSS: DISPC: Don't allow predecimation for writeback
From: Archit Taneja @ 2012-11-07  6:27 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1352268905-31093-1-git-send-email-archit@ti.com>

Since writeback writes to a buffer instead of reading from one, predecimation
doesn't make sense for it. Configure the width and height predecimation limits
to 1 if the plane is writeback.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index f19cd37..664ac6f 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2315,9 +2315,14 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	if ((caps & OMAP_DSS_OVL_CAP_SCALE) = 0)
 		return -EINVAL;
 
-	*x_predecim = max_decim_limit;
-	*y_predecim = (rotation_type = OMAP_DSS_ROT_TILER &&
-			dss_has_feature(FEAT_BURST_2D)) ? 2 : max_decim_limit;
+	if (plane = OMAP_DSS_WB) {
+		*x_predecim = *y_predecim = 1;
+	} else {
+		*x_predecim = max_decim_limit;
+		*y_predecim = (rotation_type = OMAP_DSS_ROT_TILER &&
+				dss_has_feature(FEAT_BURST_2D)) ?
+				2 : max_decim_limit;
+	}
 
 	if (color_mode = OMAP_DSS_COLOR_CLUT1 ||
 	    color_mode = OMAP_DSS_COLOR_CLUT2 ||
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 1/3] OMAPDSS: DISPC: Fix calc_scaling_44xx() bugs for writeback pipeline
From: Archit Taneja @ 2012-11-07  6:27 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1352268905-31093-1-git-send-email-archit@ti.com>

dispc_ovl_calc_scaling_44xx() doesn't work correctly for writeback. There are
two issues with it:

- the function tries to calculate pixel clock for the input plane using
  dispc_plane_pclk_rate(), calling this with writeback as input plane results in
  a BUG(), this function shouldn't be called for writeback at all. Fix this by
  calculating pixel clock only when we are not in mem to mem mode.

- the maximum input_width is the product of the downscale ratio supported and
  the and the given output_width. This was calculated incorrectly by dividing
  output_width with maxdownscale. Fix this.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 05def42..f19cd37 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2264,14 +2264,16 @@ static int dispc_ovl_calc_scaling_44xx(enum omap_plane plane,
 	u16 in_height = DIV_ROUND_UP(height, *decim_y);
 	const int maxsinglelinewidth  				dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
-	unsigned long pclk = dispc_plane_pclk_rate(plane);
 	const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
 
-	if (mem_to_mem)
-		in_width_max = DIV_ROUND_UP(out_width, maxdownscale);
-	else
+	if (mem_to_mem) {
+		in_width_max = out_width * maxdownscale;
+	} else {
+		unsigned long pclk = dispc_plane_pclk_rate(plane);
+
 		in_width_max = dispc_core_clk_rate() /
 					DIV_ROUND_UP(pclk, out_width);
+	}
 
 	*decim_x = DIV_ROUND_UP(width, in_width_max);
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 0/3] OMAPDSS: DISPC: Writeback fixes
From: Archit Taneja @ 2012-11-07  6:27 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja

Some issues were found in the DISPC driver when performing scaling with
writeback pipeline. This series fixes those issues.

Reference tree:
git://gitorious.org/~boddob/linux-omap-dss2/archit-dss2-clone.git 3.8/writeback_fixes_dispc

Archit Taneja (3):
  OMAPDSS: DISPC: Fix calc_scaling_44xx() bugs for writeback pipeline
  OMAPDSS: DISPC: Don't allow predecimation for writeback
  OMAPDSS: DISPC: Use output width and height to calculate row/pix inc
    for writeback

 drivers/video/omap2/dss/dispc.c |   39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

-- 
1.7.9.5


^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Hugh Dickins @ 2012-11-07  6:26 UTC (permalink / raw)
  To: Alan Cox
  Cc: Sasha Levin, Daniel Vetter, Sasha Levin, Greg Kroah-Hartman,
	Jiri Slaby, linux-kernel, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <alpine.LNX.2.00.1211061950020.1712@eggly.anvils>

On Tue, 6 Nov 2012, Hugh Dickins wrote:
> 
> Ah, now I actually scan through it, I see references to blank screen:
> I'll try taking off your patch and seeing if it came up at screen
> blanking time, then put on your patch back on and try again.
> I'll report back in an hour or two.

Yes, that was it.  When the console screen blanked on 3.7.0-rc3-mm1,
it generated that lockdep splat, visible once I unblanked.  But once I
applied your patch to the kernel, lockdep kept quiet across blank/unblank.

Thanks!
Hugh

^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Hugh Dickins @ 2012-11-07  4:29 UTC (permalink / raw)
  To: Alan Cox
  Cc: Sasha Levin, Daniel Vetter, Sasha Levin, Greg Kroah-Hartman,
	Jiri Slaby, linux-kernel, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <20121106161100.216c6d79@pyramind.ukuu.org.uk>

On Tue, 6 Nov 2012, Alan Cox wrote:
> On Mon, 5 Nov 2012 12:34:44 -0800 (PST)
> Hugh Dickins <hughd@google.com> wrote:
> > On Mon, 5 Nov 2012, Alan Cox wrote:
> > > > The fbdev potential for deadlock may be years old, but the warning
> > > > (and consequent disabling of lockdep from that point on - making it
> > > > useless to everybody else in need of it) is new, and comes from the
> > > > commit below in linux-next.
> > > > 
> > > > I revert it in my own testing: if there is no quick fix to the
> > > > fbdev issue on the way, Daniel, please revert it from your tree.
> > > 
> > > If you revert it you swap it for a different deadlock - and one that
> > > happens more often I would expect. Not very useful.
> > 
> > But a deadlock we have lived with for years.  Without reverting,
> > we're prevented from discovering all the new deadlocks we're adding.
> 
> We lived with it locking boxes up on users but not knowing why. The root
> cause is loading two different framebuffers with one taking over from
> another - that should be an obscure corner case and once the fuzz testing
> can avoid.

I'm bemused, but at least I now understand why we disagreed on this.

You thought it was a lockdep splat I got in the course of fuzz testing,
or doing some other obscure test: no, I thought I got it in booting up
the laptop, so it was in the way of doing useful testing thereafter.

I'd swear that I saw it two or three times, on each boot of 3.7.0-rc3-mm1;
then lost patience and deleted all the console_lock_dep_map lines from
kernel/printk.c, after which no problem.

But /var/log/messages calls me a liar, shows only one instance, and that
10 minutes after booting: that splat appended below in case it tells you
anything new; but I've no idea what triggered iti.  (The "W" taint comes
from my using a "numaúke=2" boot option, which surprised smpboot.c to
find smt-siblings on different nodes: not related to the console, I hope).

>  
> > That would be ideal - thanks.
> 
> 
> I had a semi-informed poke at this and came up with a possible patch (not very tested)

Many thanks for your effort.

> 
> commit f4fa6c739ecc367dbb98f5be1ff626d9b2750878
> Author: Alan Cox <alan@linux.intel.com>
> Date:   Tue Nov 6 15:33:18 2012 +0000
> 
>     fb: Rework locking to fix lock ordering on takeover
>     
>     Adjust the console layer to allow a take over call where the caller already
>     holds the locks. Make the fb layer lock in order.
>     
>     This s partly a band aid, the fb layer is terminally confused about the
>     locking rules it uses for its notifiers it seems.
>     
>     Signed-off-by: Alan Cox <alan@linux.intel.com>

So I went to test this, but first tried to reproduce the orginal lockdep
splat that had irritated me so, and was utterly unsuccessful.  So although
I am now running happily with your patch applied, no ill effects observed,
this gives no confidence because I cannot reproduce the condition anyway.

Sorry to be so unhelpful, original splat without your patch below.

Ah, now I actually scan through it, I see references to blank screen:
I'll try taking off your patch and seeing if it came up at screen
blanking time, then put on your patch back on and try again.
I'll report back in an hour or two.

Hugh

===========================
[ INFO: possible circular locking dependency detected ]
3.7.0-rc3-mm1 #2 Tainted: G        W   
-------------------------------------------------------
kworker/0:1/30 is trying to acquire lock:
ACPI: Invalid Power Resource to register!
 ((fb_notifier_list).rwsem){.+.+.+}, at: [<ffffffff81080ed0>] __blocking_notifier_call_chain+0x6b/0xa2

but task is already holding lock:
 (console_lock){+.+.+.}, at: [<ffffffff812877df>] console_callback+0xc/0xf7

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (console_lock){+.+.+.}:
       [<ffffffff810a517a>] __lock_acquire+0x7fc/0x8bb
       [<ffffffff810a561c>] lock_acquire+0x57/0x6d
       [<ffffffff8106074d>] console_lock+0x67/0x69
       [<ffffffff8128599a>] register_con_driver+0x36/0x128
       [<ffffffff81285e60>] take_over_console+0x21/0x2b7
       [<ffffffff81237014>] fbcon_takeover+0x56/0x98
       [<ffffffff8123a7dd>] fbcon_event_notify+0x3bb/0x6ee
       [<ffffffff81080c32>] notifier_call_chain+0xa7/0xd4
       [<ffffffff81080ee6>] __blocking_notifier_call_chain+0x81/0xa2
       [<ffffffff81080f16>] blocking_notifier_call_chain+0xf/0x11
       [<ffffffff8122f7aa>] fb_notifier_call_chain+0x16/0x18
       [<ffffffff81231523>] register_framebuffer+0x20c/0x270
       [<ffffffff81297357>] drm_fb_helper_single_fb_probe+0x1ce/0x270
       [<ffffffff812975c3>] drm_fb_helper_initial_config+0x1ca/0x1e1
       [<ffffffff812e80bd>] intel_fbdev_init+0x76/0x89
       [<ffffffff812b430a>] i915_driver_load+0xb20/0xcf7
       [<ffffffff812a346e>] drm_get_pci_dev+0x162/0x25b
       [<ffffffff8150f1b8>] i915_pci_probe+0x60/0x69
       [<ffffffff812269cf>] local_pci_probe+0x12/0x16
       [<ffffffff812274a1>] pci_device_probe+0xbe/0xeb
       [<ffffffff812f783d>] driver_probe_device+0x91/0x19e
       [<ffffffff812f79a7>] __driver_attach+0x5d/0x80
       [<ffffffff812f5fac>] bus_for_each_dev+0x52/0x84
       [<ffffffff812f74ed>] driver_attach+0x19/0x1b
       [<ffffffff812f701f>] bus_add_driver+0xe7/0x20c
       [<ffffffff812f7f1b>] driver_register+0x8e/0x114
       [<ffffffff81227590>] __pci_register_driver+0x5a/0x5f
       [<ffffffff812a35e7>] drm_pci_init+0x80/0xe5
       [<ffffffff818c1229>] i915_init+0x66/0x68
       [<ffffffff81000231>] do_one_initcall+0x7a/0x131
       [<ffffffff81508129>] kernel_init+0x106/0x26d
       [<ffffffff81529cac>] ret_from_fork+0x7c/0xb0

-> #0 ((fb_notifier_list).rwsem){.+.+.+}:
       [<ffffffff810a3e14>] validate_chain.isra.21+0x7b0/0xd45
       [<ffffffff810a517a>] __lock_acquire+0x7fc/0x8bb
       [<ffffffff810a561c>] lock_acquire+0x57/0x6d
       [<ffffffff81526df5>] down_read+0x42/0x57
       [<ffffffff81080ed0>] __blocking_notifier_call_chain+0x6b/0xa2
       [<ffffffff81080f16>] blocking_notifier_call_chain+0xf/0x11
       [<ffffffff8122f7aa>] fb_notifier_call_chain+0x16/0x18
       [<ffffffff8122fe47>] fb_blank+0x36/0x85
       [<ffffffff81237ceb>] fbcon_blank+0x129/0x269
       [<ffffffff8128589b>] do_blank_screen+0x18a/0x253
       [<ffffffff8128789f>] console_callback+0xcc/0xf7
       [<ffffffff81076a5e>] process_one_work+0x20e/0x3a2
       [<ffffffff81076e0a>] worker_thread+0x1ee/0x2cb
       [<ffffffff8107b90a>] kthread+0xd0/0xd8
       [<ffffffff81529cac>] ret_from_fork+0x7c/0xb0

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(console_lock);
                               lock((fb_notifier_list).rwsem);
                               lock(console_lock);
  lock((fb_notifier_list).rwsem);

 *** DEADLOCK ***

3 locks held by kworker/0:1/30:
 #0:  (events){.+.+.+}, at: [<ffffffff810769f6>] process_one_work+0x1a6/0x3a2
 #1:  (console_work){+.+.+.}, at: [<ffffffff810769f6>] process_one_work+0x1a6/0x3a2
 #2:  (console_lock){+.+.+.}, at: [<ffffffff812877df>] console_callback+0xc/0xf7

stack backtrace:
Pid: 30, comm: kworker/0:1 Tainted: G        W    3.7.0-rc3-mm1 #2
Call Trace:
 [<ffffffff8151e443>] print_circular_bug+0x28d/0x29e
 [<ffffffff810a3e14>] validate_chain.isra.21+0x7b0/0xd45
 [<ffffffff8107928b>] ? __kernel_text_address+0x22/0x41
 [<ffffffff810a517a>] __lock_acquire+0x7fc/0x8bb
 [<ffffffff810a561c>] lock_acquire+0x57/0x6d
 [<ffffffff81080ed0>] ? __blocking_notifier_call_chain+0x6b/0xa2
 [<ffffffff81526df5>] down_read+0x42/0x57
 [<ffffffff81080ed0>] ? __blocking_notifier_call_chain+0x6b/0xa2
 [<ffffffff81080ed0>] __blocking_notifier_call_chain+0x6b/0xa2
 [<ffffffff81080f16>] blocking_notifier_call_chain+0xf/0x11
 [<ffffffff8122f7aa>] fb_notifier_call_chain+0x16/0x18
 [<ffffffff8122fe47>] fb_blank+0x36/0x85
 [<ffffffff81237ceb>] fbcon_blank+0x129/0x269
 [<ffffffff8152910b>] ? _raw_spin_unlock_irqrestore+0x3a/0x64
 [<ffffffff810a5dbf>] ? trace_hardirqs_on_caller+0x114/0x170
 [<ffffffff810a5e28>] ? trace_hardirqs_on+0xd/0xf
 [<ffffffff81529117>] ? _raw_spin_unlock_irqrestore+0x46/0x64
 [<ffffffff8106bc00>] ? try_to_del_timer_sync+0x48/0x54
 [<ffffffff8106bc77>] ? del_timer_sync+0x6b/0xb4
 [<ffffffff8106bc9a>] ? del_timer_sync+0x8e/0xb4
 [<ffffffff8106bc0c>] ? try_to_del_timer_sync+0x54/0x54
 [<ffffffff8128589b>] do_blank_screen+0x18a/0x253
 [<ffffffff8128789f>] console_callback+0xcc/0xf7
 [<ffffffff810769f6>] ? process_one_work+0x1a6/0x3a2
 [<ffffffff81076a5e>] process_one_work+0x20e/0x3a2
 [<ffffffff810769f6>] ? process_one_work+0x1a6/0x3a2
 [<ffffffff812877d3>] ? poke_blanked_console+0xc9/0xc9
 [<ffffffff81076e0a>] worker_thread+0x1ee/0x2cb
 [<ffffffff81076c1c>] ? process_scheduled_works+0x2a/0x2a
 [<ffffffff8107b90a>] kthread+0xd0/0xd8
 [<ffffffff8152915d>] ? _raw_spin_unlock_irq+0x28/0x50
 [<ffffffff8107b83a>] ? __init_kthread_worker+0x55/0x55
 [<ffffffff81529cac>] ret_from_fork+0x7c/0xb0
 [<ffffffff8107b83a>] ? __init_kthread_worker+0x55/0x55

^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Alan Cox @ 2012-11-06 17:38 UTC (permalink / raw)
  To: Dave Jones
  Cc: Hugh Dickins, Sasha Levin, Daniel Vetter, Sasha Levin,
	Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-fbdev,
	florianSchandinat
In-Reply-To: <20121106164214.GA18246@redhat.com>

>  > The root
>  > cause is loading two different framebuffers with one taking over from
>  > another - that should be an obscure corner case and once the fuzz testing
>  > can avoid.
>  > 
>  > I had a semi-informed poke at this and came up with a possible patch (not very tested)
> 
> If this fixes the real problems we've been seeing, I'll dance a jig.

Youtube...

At this point my bigger concern is that it'll just make something else
warn instead. The underlying problem is that fbcon layer implements a
single threaded notifier whose locking semantics are at best random. It's
not calld with a specific set of locks each time. Possibly it sohuld be
two notifiers (one for fb stuff, one for console layer stuff) but the
entire layer is horrible. I live in home the KMS guys will rip out the
useful bits and build a straight kms fb layer with refcounting and the
like 8)

Testing certainly needed and if it's still blowing up then hopefully
further traces will help fix up the other cases we don't know about.

Alan

^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Dave Jones @ 2012-11-06 16:42 UTC (permalink / raw)
  To: Alan Cox
  Cc: Hugh Dickins, Sasha Levin, Daniel Vetter, Sasha Levin,
	Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-fbdev,
	florianSchandinat
In-Reply-To: <20121106161100.216c6d79@pyramind.ukuu.org.uk>

On Tue, Nov 06, 2012 at 04:11:00PM +0000, Alan Cox wrote:

 > > But a deadlock we have lived with for years.  Without reverting,
 > > we're prevented from discovering all the new deadlocks we're adding.
 > 
 > We lived with it locking boxes up on users but not knowing why.

Circa 3.5 we got a lot more reports of this happening too for some reason.
Turns out that races are awfully resistant to bisecting too.

 > The root
 > cause is loading two different framebuffers with one taking over from
 > another - that should be an obscure corner case and once the fuzz testing
 > can avoid.
 > 
 > I had a semi-informed poke at this and came up with a possible patch (not very tested)

If this fixes the real problems we've been seeing, I'll dance a jig.
 
	Dave


^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Alan Cox @ 2012-11-06 16:11 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Sasha Levin, Daniel Vetter, Sasha Levin, Greg Kroah-Hartman,
	Jiri Slaby, linux-kernel, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <alpine.LNX.2.00.1211051231070.21645@eggly.anvils>

On Mon, 5 Nov 2012 12:34:44 -0800 (PST)
Hugh Dickins <hughd@google.com> wrote:

> On Mon, 5 Nov 2012, Alan Cox wrote:
> > > The fbdev potential for deadlock may be years old, but the warning
> > > (and consequent disabling of lockdep from that point on - making it
> > > useless to everybody else in need of it) is new, and comes from the
> > > commit below in linux-next.
> > > 
> > > I revert it in my own testing: if there is no quick fix to the
> > > fbdev issue on the way, Daniel, please revert it from your tree.
> > 
> > If you revert it you swap it for a different deadlock - and one that
> > happens more often I would expect. Not very useful.
> 
> But a deadlock we have lived with for years.  Without reverting,
> we're prevented from discovering all the new deadlocks we're adding.

We lived with it locking boxes up on users but not knowing why. The root
cause is loading two different framebuffers with one taking over from
another - that should be an obscure corner case and once the fuzz testing
can avoid.
 
> That would be ideal - thanks.


I had a semi-informed poke at this and came up with a possible patch (not very tested)


commit f4fa6c739ecc367dbb98f5be1ff626d9b2750878
Author: Alan Cox <alan@linux.intel.com>
Date:   Tue Nov 6 15:33:18 2012 +0000

    fb: Rework locking to fix lock ordering on takeover
    
    Adjust the console layer to allow a take over call where the caller already
    holds the locks. Make the fb layer lock in order.
    
    This s partly a band aid, the fb layer is terminally confused about the
    locking rules it uses for its notifiers it seems.
    
    Signed-off-by: Alan Cox <alan@linux.intel.com>

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index f87d7e8..ea57f27 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -2984,7 +2984,7 @@ int __init vty_init(const struct file_operations *console_fops)
 
 static struct class *vtconsole_class;
 
-static int bind_con_driver(const struct consw *csw, int first, int last,
+static int do_bind_con_driver(const struct consw *csw, int first, int last,
 			   int deflt)
 {
 	struct module *owner = csw->owner;
@@ -2995,7 +2995,7 @@ static int bind_con_driver(const struct consw *csw, int first, int last,
 	if (!try_module_get(owner))
 		return -ENODEV;
 
-	console_lock();
+	WARN_CONSOLE_UNLOCKED();
 
 	/* check if driver is registered */
 	for (i = 0; i < MAX_NR_CON_DRIVER; i++) {
@@ -3080,11 +3080,22 @@ static int bind_con_driver(const struct consw *csw, int first, int last,
 
 	retval = 0;
 err:
-	console_unlock();
 	module_put(owner);
 	return retval;
 };
 
+
+static int bind_con_driver(const struct consw *csw, int first, int last,
+			   int deflt)
+{
+	int ret;
+	
+	console_unlock();
+	ret = do_bind_con_driver(csw, first, last, deflt);
+	console_unlock();
+	return ret;
+}
+	
 #ifdef CONFIG_VT_HW_CONSOLE_BINDING
 static int con_is_graphics(const struct consw *csw, int first, int last)
 {
@@ -3196,9 +3207,9 @@ int unbind_con_driver(const struct consw *csw, int first, int last, int deflt)
 	if (!con_is_bound(csw))
 		con_driver->flag &= ~CON_DRIVER_FLAG_INIT;
 
-	console_unlock();
 	/* ignore return value, binding should not fail */
-	bind_con_driver(defcsw, first, last, deflt);
+	do_bind_con_driver(defcsw, first, last, deflt);
+	console_unlock();
 err:
 	module_put(owner);
 	return retval;
@@ -3489,28 +3500,18 @@ int con_debug_leave(void)
 }
 EXPORT_SYMBOL_GPL(con_debug_leave);
 
-/**
- * register_con_driver - register console driver to console layer
- * @csw: console driver
- * @first: the first console to take over, minimum value is 0
- * @last: the last console to take over, maximum value is MAX_NR_CONSOLES -1
- *
- * DESCRIPTION: This function registers a console driver which can later
- * bind to a range of consoles specified by @first and @last. It will
- * also initialize the console driver by calling con_startup().
- */
-int register_con_driver(const struct consw *csw, int first, int last)
+static int do_register_con_driver(const struct consw *csw, int first, int last)
 {
 	struct module *owner = csw->owner;
 	struct con_driver *con_driver;
 	const char *desc;
 	int i, retval = 0;
 
+	WARN_CONSOLE_UNLOCKED();
+
 	if (!try_module_get(owner))
 		return -ENODEV;
 
-	console_lock();
-
 	for (i = 0; i < MAX_NR_CON_DRIVER; i++) {
 		con_driver = &registered_con_driver[i];
 
@@ -3563,10 +3564,29 @@ int register_con_driver(const struct consw *csw, int first, int last)
 	}
 
 err:
-	console_unlock();
 	module_put(owner);
 	return retval;
 }
+
+/**
+ * register_con_driver - register console driver to console layer
+ * @csw: console driver
+ * @first: the first console to take over, minimum value is 0
+ * @last: the last console to take over, maximum value is MAX_NR_CONSOLES -1
+ *
+ * DESCRIPTION: This function registers a console driver which can later
+ * bind to a range of consoles specified by @first and @last. It will
+ * also initialize the console driver by calling con_startup().
+ */
+int register_con_driver(const struct consw *csw, int first, int last)
+{
+	int retval;
+	
+	console_lock();
+	retval = do_register_con_driver(csw, first, last);
+	console_unlock();
+	return retval;
+}
 EXPORT_SYMBOL(register_con_driver);
 
 /**
@@ -3622,6 +3642,29 @@ EXPORT_SYMBOL(unregister_con_driver);
  *
  *      take_over_console is basically a register followed by unbind
  */
+int do_take_over_console(const struct consw *csw, int first, int last, int deflt)
+{
+	int err;
+
+	err = do_register_con_driver(csw, first, last);
+	/* if we get an busy error we still want to bind the console driver
+	 * and return success, as we may have unbound the console driver
+	 * but not unregistered it.
+	*/
+	if (err = -EBUSY)
+		err = 0;
+	if (!err)
+		do_bind_con_driver(csw, first, last, deflt);
+
+	return err;
+}
+/*
+ *	If we support more console drivers, this function is used
+ *	when a driver wants to take over some existing consoles
+ *	and become default driver for newly opened ones.
+ *
+ *      take_over_console is basically a register followed by unbind
+ */
 int take_over_console(const struct consw *csw, int first, int last, int deflt)
 {
 	int err;
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index fdefa8f..c75f8ce 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -529,6 +529,34 @@ static int search_for_mapped_con(void)
 	return retval;
 }
 
+static int do_fbcon_takeover(int show_logo)
+{
+	int err, i;
+
+	if (!num_registered_fb)
+		return -ENODEV;
+
+	if (!show_logo)
+		logo_shown = FBCON_LOGO_DONTSHOW;
+
+	for (i = first_fb_vc; i <= last_fb_vc; i++)
+		con2fb_map[i] = info_idx;
+
+	err = do_take_over_console(&fb_con, first_fb_vc, last_fb_vc,
+				fbcon_is_default);
+
+	if (err) {
+		for (i = first_fb_vc; i <= last_fb_vc; i++) {
+			con2fb_map[i] = -1;
+		}
+		info_idx = -1;
+	} else {
+		fbcon_has_console_bind = 1;
+	}
+
+	return err;
+}
+
 static int fbcon_takeover(int show_logo)
 {
 	int err, i;
@@ -3115,7 +3143,7 @@ static int fbcon_fb_registered(struct fb_info *info)
 		}
 
 		if (info_idx != -1)
-			ret = fbcon_takeover(1);
+			ret = do_fbcon_takeover(1);
 	} else {
 		for (i = first_fb_vc; i <= last_fb_vc; i++) {
 			if (con2fb_map_boot[i] = idx)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 3ff0105..588bdab 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1650,7 +1650,9 @@ static int do_register_framebuffer(struct fb_info *fb_info)
 	event.info = fb_info;
 	if (!lock_fb_info(fb_info))
 		return -ENODEV;
+        console_lock();
 	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
+        console_unlock();
 	unlock_fb_info(fb_info);
 	return 0;
 }
diff --git a/include/linux/console.h b/include/linux/console.h
index dedb082..4ef4307 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -78,6 +78,7 @@ int con_is_bound(const struct consw *csw);
 int register_con_driver(const struct consw *csw, int first, int last);
 int unregister_con_driver(const struct consw *csw);
 int take_over_console(const struct consw *sw, int first, int last, int deflt);
+int do_take_over_console(const struct consw *sw, int first, int last, int deflt);
 void give_up_console(const struct consw *sw);
 #ifdef CONFIG_HW_CONSOLE
 int con_debug_enter(struct vc_data *vc);

^ permalink raw reply related

* Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Rob Clark @ 2012-11-06 14:40 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Tomi Valkeinen, Archit Taneja, linux-omap, linux-fbdev, dri-devel
In-Reply-To: <509913A4.5080408@iki.fi>

On Tue, Nov 6, 2012 at 7:41 AM, Tomi Valkeinen <tomba@iki.fi> wrote:
> On 2012-11-05 16:21, Rob Clark wrote:
>> On 11/05/2012 02:55 AM, Tomi Valkeinen wrote:
>>>>> But even then, choosing the manager is not easy, as whoever chooses the
>>>>> >>manager needs to observe all the possible displays used at the same
>>>>> >>time...
>>>> >
>>>> >Right. I was wondering if omapfb/omapdrm could understand the 'all
>>>> >possible displays information' better compared to a panel's probe.
>>>> >
>>>> >Even omapdrm/omafb can't be perfect because we could insert a panel
>>>> >driver module at any time, and omapfb/omapdrm may miss that out.
>>> True, omapdrm/fb may have a better idea. It's still unclear though.
>>> Currently we have quite strict order in the sequence the modules need to
>>> be loaded, which is quite bad and causes issues. We should make things
>>> more dynamic, so that the initialization of the drivers could happen
>>> more freely.
>>>
>>> But that creates more problems: when booting up, omapfb starts. But
>>> omapfb can't know if all the panel drivers have already been loaded.
>>> omapfb may see that DVI is the default display, but what should it do if
>>> DVI doesn't have a driver yet? It could wait, but perhaps the driver for
>>> DVI will never even be loaded.
>>
>> The encoder which is connected to the crtc (manager) is picked by
>> combination of encoder->possible_crtcs bitmask and
>> connector->best_encoder().  We could keep things limited so that the
>> association of crtc to encoder (manager to output, roughly) never
>> changes, but this isn't really the right thing to do.  It is better that
>> the dssdev not rely on knowing the manager it is attached to at probe
>> time, but instead grab resources more dynamically.
>>
>> Also, at the moment we don't really have any notification to userspace
>> about new encoders/connectors showing up (or conversely, being
>> removed).  Only about existing connectors being plugged/unplugged.  The
>> closest analogy is perhaps the USB display devices, but even there it is
>> only the entire drm device that is plugged/unplugged.  And TBH I don't
>> really see the point in supporting panel drivers being dynamically
>> loaded.  It isn't like someone is dynamically soldering on a new display
>> connector to some board that is running.  I think omapfb or omapdrm
>> probe should trigger registering the compiled-in panel drivers, so that
>> it can be sure that the dssdev's pop up before it goes and creates drm
>> connector objects.  Currently we have to hack around this in omapdrm
>> with late_initcall() to ensure the panel drivers are probed first, but
>> that is an ugly hack that I'd like to get rid of.
>
> We have panel devices and panel drivers, each of which can appear at any
> time. Both are needed for the panel probe to happen. If we don't support
> device hotplugging (dynamic creation of devices), we need to use
> late_initcall for omapfb/drm. At least I don't see any other option.
>
> You say that omapdrm should trigger registering of the drivers. How
> would that work? Do you mean that the panel drivers would register
> themselves to some common list, and omapdrm would go through this list
> when drm is loaded, calling probe for the items in the list? I guess
> that's doable, but... It's not how kernel drivers are supposed to work,
> and so doesn't sound very clean approach to me.

I mean, similar to how we handle the subdev for dmm..  the
omap_drm_init() does the platform_driver_register() for the dmm device
before the platform_driver_register() for omapdrm itself, so we know
if there is a dmm device, the driver gets probed first before omapdrm.

It could be a matter of iterating through a list, or something like
this.. that is basically an implementation detail.  But the end result
is that the order the drivers are registered is controlled so the
probe sequence works out properly (not to mention suspend/resume
sequence).

> I think we should support proper hotplugging of the panels. This would
> fix the problem about init order, but it would also give us device
> hotplug support. Obviously nobody is going to solder panel to a running
> board, but I don't see any reason why panels, or, more likely, panels on
> an add-on boards (like the capes being discussed in omap ml) would not
> be hotpluggable using whatever connector is used on the particular use case.
>
> And even if we don't support removing of the devices, things like the
> add-on capes could cause the panel on the cape to be identified at some
> late time (the panel is not described in the board file or DT data, but
> found at runtime depending on the ID of the cape). This would add
> another step to the init sequence that should be just right, if we don't
> support hotplug.

If capes are really hot-pluggable, then maybe it is worth thinking
about how to make this more dynamic.  Although it is a bigger problem,
which involves userspace being aware that connectors can dynamically
appear/disappear.  And the dynamic disappearing is something I worry
about more.. it adds the possibility of all sorts of interesting race
conditions, such as connectors disappearing in the middle of modeset.
I prefer not making things more complicated and error prone than they
need to be.  If there is not a legitimate use case for connector hw
dynamically appearing/disappearing then I don't think we should go
there.  It sounds nice and simple and clean, but in reality I think it
just introduces a whole lot of ways for things to go wrong.  A wise
man once said:

https://github.com/robclark/kernel-omap4/blob/master/Documentation/SubmittingPatches#L700

BR,
-R

> Yes, I know it's not simple =). And I'm fine with simpler approach for
> the time being, but I'd like full hotplug to be the future goal. At
> least the common panel framework should not create restrictions about
> this, even if drm wouldn't allow device hotplug.
>
>  Tomi
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Tomi Valkeinen @ 2012-11-06 13:41 UTC (permalink / raw)
  To: Rob Clark
  Cc: Tomi Valkeinen, Archit Taneja, linux-omap, linux-fbdev, dri-devel
In-Reply-To: <5097CB81.7050204@ti.com>

On 2012-11-05 16:21, Rob Clark wrote:
> On 11/05/2012 02:55 AM, Tomi Valkeinen wrote:
>>>> But even then, choosing the manager is not easy, as whoever chooses the
>>>> >>manager needs to observe all the possible displays used at the same
>>>> >>time...
>>> >
>>> >Right. I was wondering if omapfb/omapdrm could understand the 'all
>>> >possible displays information' better compared to a panel's probe.
>>> >
>>> >Even omapdrm/omafb can't be perfect because we could insert a panel
>>> >driver module at any time, and omapfb/omapdrm may miss that out.
>> True, omapdrm/fb may have a better idea. It's still unclear though.
>> Currently we have quite strict order in the sequence the modules need to
>> be loaded, which is quite bad and causes issues. We should make things
>> more dynamic, so that the initialization of the drivers could happen
>> more freely.
>>
>> But that creates more problems: when booting up, omapfb starts. But
>> omapfb can't know if all the panel drivers have already been loaded.
>> omapfb may see that DVI is the default display, but what should it do if
>> DVI doesn't have a driver yet? It could wait, but perhaps the driver for
>> DVI will never even be loaded.
> 
> The encoder which is connected to the crtc (manager) is picked by
> combination of encoder->possible_crtcs bitmask and
> connector->best_encoder().  We could keep things limited so that the
> association of crtc to encoder (manager to output, roughly) never
> changes, but this isn't really the right thing to do.  It is better that
> the dssdev not rely on knowing the manager it is attached to at probe
> time, but instead grab resources more dynamically.
> 
> Also, at the moment we don't really have any notification to userspace
> about new encoders/connectors showing up (or conversely, being
> removed).  Only about existing connectors being plugged/unplugged.  The
> closest analogy is perhaps the USB display devices, but even there it is
> only the entire drm device that is plugged/unplugged.  And TBH I don't
> really see the point in supporting panel drivers being dynamically
> loaded.  It isn't like someone is dynamically soldering on a new display
> connector to some board that is running.  I think omapfb or omapdrm
> probe should trigger registering the compiled-in panel drivers, so that
> it can be sure that the dssdev's pop up before it goes and creates drm
> connector objects.  Currently we have to hack around this in omapdrm
> with late_initcall() to ensure the panel drivers are probed first, but
> that is an ugly hack that I'd like to get rid of.

We have panel devices and panel drivers, each of which can appear at any
time. Both are needed for the panel probe to happen. If we don't support
device hotplugging (dynamic creation of devices), we need to use
late_initcall for omapfb/drm. At least I don't see any other option.

You say that omapdrm should trigger registering of the drivers. How
would that work? Do you mean that the panel drivers would register
themselves to some common list, and omapdrm would go through this list
when drm is loaded, calling probe for the items in the list? I guess
that's doable, but... It's not how kernel drivers are supposed to work,
and so doesn't sound very clean approach to me.

I think we should support proper hotplugging of the panels. This would
fix the problem about init order, but it would also give us device
hotplug support. Obviously nobody is going to solder panel to a running
board, but I don't see any reason why panels, or, more likely, panels on
an add-on boards (like the capes being discussed in omap ml) would not
be hotpluggable using whatever connector is used on the particular use case.

And even if we don't support removing of the devices, things like the
add-on capes could cause the panel on the cape to be identified at some
late time (the panel is not described in the board file or DT data, but
found at runtime depending on the ID of the cape). This would add
another step to the init sequence that should be just right, if we don't
support hotplug.

Yes, I know it's not simple =). And I'm fine with simpler approach for
the time being, but I'd like full hotplug to be the future goal. At
least the common panel framework should not create restrictions about
this, even if drm wouldn't allow device hotplug.

 Tomi


^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Hugh Dickins @ 2012-11-05 20:34 UTC (permalink / raw)
  To: Alan Cox
  Cc: Sasha Levin, Daniel Vetter, Sasha Levin, Greg Kroah-Hartman,
	Jiri Slaby, linux-kernel, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <20121105201507.79fe47d7@pyramind.ukuu.org.uk>

On Mon, 5 Nov 2012, Alan Cox wrote:
> > The fbdev potential for deadlock may be years old, but the warning
> > (and consequent disabling of lockdep from that point on - making it
> > useless to everybody else in need of it) is new, and comes from the
> > commit below in linux-next.
> > 
> > I revert it in my own testing: if there is no quick fix to the
> > fbdev issue on the way, Daniel, please revert it from your tree.
> 
> If you revert it you swap it for a different deadlock - and one that
> happens more often I would expect. Not very useful.

But a deadlock we have lived with for years.  Without reverting,
we're prevented from discovering all the new deadlocks we're adding.

> 
> I'm hoping the framebuffer maintainer will bother to respond to this
> because that's the only way it can be sorted out.

That would be ideal - thanks.

Hugh

^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Alan Cox @ 2012-11-05 20:15 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Sasha Levin, Daniel Vetter, Sasha Levin, Greg Kroah-Hartman,
	Jiri Slaby, linux-kernel, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <alpine.LNX.2.00.1211051106590.1709@eggly.anvils>

> The fbdev potential for deadlock may be years old, but the warning
> (and consequent disabling of lockdep from that point on - making it
> useless to everybody else in need of it) is new, and comes from the
> commit below in linux-next.
> 
> I revert it in my own testing: if there is no quick fix to the
> fbdev issue on the way, Daniel, please revert it from your tree.

If you revert it you swap it for a different deadlock - and one that
happens more often I would expect. Not very useful.

I'm hoping the framebuffer maintainer will bother to respond to this
because that's the only way it can be sorted out.

Alan

^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Hugh Dickins @ 2012-11-05 19:17 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Daniel Vetter, Alan Cox, Sasha Levin, Greg Kroah-Hartman,
	Jiri Slaby, linux-kernel, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <5097FEA9.2090603@oracle.com>

On Mon, 5 Nov 2012, Sasha Levin wrote:
> On 11/05/2012 12:59 PM, Alan Cox wrote:
> > On Mon, 5 Nov 2012 12:26:43 -0500
> > Sasha Levin <levinsasha928@gmail.com> wrote:
> > 
> >> Ping? Should I bisect it?
> >>
> >> On Fri, Oct 26, 2012 at 9:37 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> >>> On Thu, 25 Oct 2012 15:37:43 -0400
> >>> Sasha Levin <sasha.levin@oracle.com> wrote:
> >>>
> >>>> Hi all,
> >>>>
> >>>> While fuzzing with trinity inside a KVM tools (lkvm) guest running latest -next kernel,
> >>>> I've stumbled on the following spew:
> >>>
> >>> Looks real enough but its not a tty/vt layer spew. This is all coming out
> >>> of the core framebuffer code which doesn't seem to be able to decide what
> >>> the locking rules at the invocation of fb_notifier_call_chain are.
> >>>
> >>> It might need some console layer tweaking to provide 'register console
> >>> and I already hold the locks' or similar but that notifier needs some
> >>> kind of sanity applying as well.
> >>>
> >>> Cc'ing the fbdev folks
> > 
> > I've cc'd the framebuffer folks. I can see why its occurring but I have
> > no idea how they intend to fix it and I've not seen any replies.
> > 
> > Sorry but I've got enough other things on my plate right now without
> > trying to deal with the locking brain damage that the fbdev layer is.
> > 
> > As far as I can tell the actual bug proper is years old.
> > 
> > Alan
> > 
> 
> Ow, I figured it's something new since I've only now started seeing it in fuzz
> tests, and it reproduces pretty much every time.

The fbdev potential for deadlock may be years old, but the warning
(and consequent disabling of lockdep from that point on - making it
useless to everybody else in need of it) is new, and comes from the
commit below in linux-next.

I revert it in my own testing: if there is no quick fix to the
fbdev issue on the way, Daniel, please revert it from your tree.

Thanks,
Hugh

commit daee779718a319ff9f83e1ba3339334ac650bb22
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Sat Sep 22 19:52:11 2012 +0200

    console: implement lockdep support for console_lock
    
    Dave Airlie recently discovered a locking bug in the fbcon layer,
    where a timer_del_sync (for the blinking cursor) deadlocks with the
    timer itself, since both (want to) hold the console_lock:
    
    https://lkml.org/lkml/2012/8/21/36
    
    Unfortunately the console_lock isn't a plain mutex and hence has no
    lockdep support. Which resulted in a few days wasted of tracking down
    this bug (complicated by the fact that printk doesn't show anything
    when the console is locked) instead of noticing the bug much earlier
    with the lockdep splat.
    
    Hence I've figured I need to fix that for the next deadlock involving
    console_lock - and with kms/drm growing ever more complex locking
    that'll eventually happen.
    
    Now the console_lock has rather funky semantics, so after a quick irc
    discussion with Thomas Gleixner and Dave Airlie I've quickly ditched
    the original idead of switching to a real mutex (since it won't work)
    and instead opted to annotate the console_lock with lockdep
    information manually.
    
    There are a few special cases:
    - The console_lock state is protected by the console_sem, and usually
      grabbed/dropped at _lock/_unlock time. But the suspend/resume code
      drops the semaphore without dropping the console_lock (see
      suspend_console/resume_console). But since the same thread that did
      the suspend will do the resume, we don't need to fix up anything.
    
    - In the printk code there's a special trylock, only used to kick off
      the logbuffer printk'ing in console_unlock. But all that happens
      while lockdep is disable (since printk does a few other evil
      tricks). So no issue there, either.
    
    - The console_lock can also be acquired form irq context (but only
      with a trylock). lockdep already handles that.
    
    This all leaves us with annotating the normal console_lock, _unlock
    and _trylock functions.
    
    And yes, it works - simply unloading a drm kms driver resulted in
    lockdep complaining about the deadlock in fbcon_deinit:
    
    ===========================
    [ INFO: possible circular locking dependency detected ]
    3.6.0-rc2+ #552 Not tainted
    -------------------------------------------------------
    kms-reload/3577 is trying to acquire lock:
     ((&info->queue)){+.+...}, at: [<ffffffff81058c70>] wait_on_work+0x0/0xa7
    
    but task is already holding lock:
     (console_lock){+.+.+.}, at: [<ffffffff81264686>] bind_con_driver+0x38/0x263
    
    which lock already depends on the new lock.
    
    the existing dependency chain (in reverse order) is:
    
    -> #1 (console_lock){+.+.+.}:
           [<ffffffff81087440>] lock_acquire+0x95/0x105
           [<ffffffff81040190>] console_lock+0x59/0x5b
           [<ffffffff81209cb6>] fb_flashcursor+0x2e/0x12c
           [<ffffffff81057c3e>] process_one_work+0x1d9/0x3b4
           [<ffffffff810584a2>] worker_thread+0x1a7/0x24b
           [<ffffffff8105ca29>] kthread+0x7f/0x87
           [<ffffffff813b1204>] kernel_thread_helper+0x4/0x10
    
    -> #0 ((&info->queue)){+.+...}:
           [<ffffffff81086cb3>] __lock_acquire+0x999/0xcf6
           [<ffffffff81087440>] lock_acquire+0x95/0x105
           [<ffffffff81058cab>] wait_on_work+0x3b/0xa7
           [<ffffffff81058dd6>] __cancel_work_timer+0xbf/0x102
           [<ffffffff81058e33>] cancel_work_sync+0xb/0xd
           [<ffffffff8120a3b3>] fbcon_deinit+0x11c/0x1dc
           [<ffffffff81264793>] bind_con_driver+0x145/0x263
           [<ffffffff81264a45>] unbind_con_driver+0x14f/0x195
           [<ffffffff8126540c>] store_bind+0x1ad/0x1c1
           [<ffffffff8127cbb7>] dev_attr_store+0x13/0x1f
           [<ffffffff8116d884>] sysfs_write_file+0xe9/0x121
           [<ffffffff811145b2>] vfs_write+0x9b/0xfd
           [<ffffffff811147b7>] sys_write+0x3e/0x6b
           [<ffffffff813b0039>] system_call_fastpath+0x16/0x1b
    
    other info that might help us debug this:
    
     Possible unsafe locking scenario:
    
           CPU0                    CPU1
           ----                    ----
      lock(console_lock);
                                   lock((&info->queue));
                                   lock(console_lock);
      lock((&info->queue));
    
     *** DEADLOCK ***
    
    v2: Mark the lockdep_map static, noticed by Jani Nikula.
    
    Cc: Dave Airlie <airlied@gmail.com>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
    Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/kernel/printk.c b/kernel/printk.c
index 2d607f4..ee79f14 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -87,6 +87,12 @@ static DEFINE_SEMAPHORE(console_sem);
 struct console *console_drivers;
 EXPORT_SYMBOL_GPL(console_drivers);
 
+#ifdef CONFIG_LOCKDEP
+static struct lockdep_map console_lock_dep_map = {
+	.name = "console_lock"
+};
+#endif
+
 /*
  * This is used for debugging the mess that is the VT code by
  * keeping track if we have the console semaphore held. It's
@@ -1914,6 +1920,7 @@ void console_lock(void)
 		return;
 	console_locked = 1;
 	console_may_schedule = 1;
+	mutex_acquire(&console_lock_dep_map, 0, 0, _RET_IP_);
 }
 EXPORT_SYMBOL(console_lock);
 
@@ -1935,6 +1942,7 @@ int console_trylock(void)
 	}
 	console_locked = 1;
 	console_may_schedule = 0;
+	mutex_acquire(&console_lock_dep_map, 0, 1, _RET_IP_);
 	return 1;
 }
 EXPORT_SYMBOL(console_trylock);
@@ -2095,6 +2103,7 @@ skip:
 		local_irq_restore(flags);
 	}
 	console_locked = 0;
+	mutex_release(&console_lock_dep_map, 1, _RET_IP_);
 
 	/* Release the exclusive_console once it is used */
 	if (unlikely(exclusive_console))


^ permalink raw reply related

* Re: tty, vt: lockdep warnings
From: Sasha Levin @ 2012-11-05 18:00 UTC (permalink / raw)
  To: Alan Cox
  Cc: Sasha Levin, Greg Kroah-Hartman, Jiri Slaby,
	linux-kernel@vger.kernel.org, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <20121105175937.26f31d2a@pyramind.ukuu.org.uk>

On 11/05/2012 12:59 PM, Alan Cox wrote:
> On Mon, 5 Nov 2012 12:26:43 -0500
> Sasha Levin <levinsasha928@gmail.com> wrote:
> 
>> Ping? Should I bisect it?
>>
>> On Fri, Oct 26, 2012 at 9:37 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>>> On Thu, 25 Oct 2012 15:37:43 -0400
>>> Sasha Levin <sasha.levin@oracle.com> wrote:
>>>
>>>> Hi all,
>>>>
>>>> While fuzzing with trinity inside a KVM tools (lkvm) guest running latest -next kernel,
>>>> I've stumbled on the following spew:
>>>
>>> Looks real enough but its not a tty/vt layer spew. This is all coming out
>>> of the core framebuffer code which doesn't seem to be able to decide what
>>> the locking rules at the invocation of fb_notifier_call_chain are.
>>>
>>> It might need some console layer tweaking to provide 'register console
>>> and I already hold the locks' or similar but that notifier needs some
>>> kind of sanity applying as well.
>>>
>>> Cc'ing the fbdev folks
> 
> I've cc'd the framebuffer folks. I can see why its occurring but I have
> no idea how they intend to fix it and I've not seen any replies.
> 
> Sorry but I've got enough other things on my plate right now without
> trying to deal with the locking brain damage that the fbdev layer is.
> 
> As far as I can tell the actual bug proper is years old.
> 
> Alan
> 

Ow, I figured it's something new since I've only now started seeing it in fuzz
tests, and it reproduces pretty much every time.


Thanks,
Sasha

^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Alan Cox @ 2012-11-05 17:59 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Sasha Levin, Greg Kroah-Hartman, Jiri Slaby,
	linux-kernel@vger.kernel.org, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <CA+1xoqdEesjh1EZvR_r7Hn0GUc7741EDWn59qagngcxg4=9bjQ@mail.gmail.com>

On Mon, 5 Nov 2012 12:26:43 -0500
Sasha Levin <levinsasha928@gmail.com> wrote:

> Ping? Should I bisect it?
> 
> On Fri, Oct 26, 2012 at 9:37 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > On Thu, 25 Oct 2012 15:37:43 -0400
> > Sasha Levin <sasha.levin@oracle.com> wrote:
> >
> >> Hi all,
> >>
> >> While fuzzing with trinity inside a KVM tools (lkvm) guest running latest -next kernel,
> >> I've stumbled on the following spew:
> >
> > Looks real enough but its not a tty/vt layer spew. This is all coming out
> > of the core framebuffer code which doesn't seem to be able to decide what
> > the locking rules at the invocation of fb_notifier_call_chain are.
> >
> > It might need some console layer tweaking to provide 'register console
> > and I already hold the locks' or similar but that notifier needs some
> > kind of sanity applying as well.
> >
> > Cc'ing the fbdev folks

I've cc'd the framebuffer folks. I can see why its occurring but I have
no idea how they intend to fix it and I've not seen any replies.

Sorry but I've got enough other things on my plate right now without
trying to deal with the locking brain damage that the fbdev layer is.

As far as I can tell the actual bug proper is years old.

Alan


^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Sasha Levin @ 2012-11-05 17:26 UTC (permalink / raw)
  To: Alan Cox
  Cc: Sasha Levin, Greg Kroah-Hartman, Jiri Slaby,
	linux-kernel@vger.kernel.org, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <20121026143754.50277bd8@pyramind.ukuu.org.uk>

Ping? Should I bisect it?

On Fri, Oct 26, 2012 at 9:37 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Thu, 25 Oct 2012 15:37:43 -0400
> Sasha Levin <sasha.levin@oracle.com> wrote:
>
>> Hi all,
>>
>> While fuzzing with trinity inside a KVM tools (lkvm) guest running latest -next kernel,
>> I've stumbled on the following spew:
>
> Looks real enough but its not a tty/vt layer spew. This is all coming out
> of the core framebuffer code which doesn't seem to be able to decide what
> the locking rules at the invocation of fb_notifier_call_chain are.
>
> It might need some console layer tweaking to provide 'register console
> and I already hold the locks' or similar but that notifier needs some
> kind of sanity applying as well.
>
> Cc'ing the fbdev folks
>
> Alan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ 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