Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 09/14] OMAPDSS: Add functions for external control of PLL
From: Tomi Valkeinen @ 2015-01-27 10:50 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: Tomi Valkeinen
In-Reply-To: <1422355842-11234-1-git-send-email-tomi.valkeinen@ti.com>

Add functions which configure the control module register
CTRL_CORE_DSS_PLL_CONTROL found in DRA7xx SoCs. This register configures
whether the PLL registers are accessed internally by DSS, or externally
using OCP2SCP interface. They also configure muxes which route the PLL
output to a particular LCD overlay manager within DSS.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/fbdev/omap2/dss/dss.c | 115 ++++++++++++++++++++++++++++++++++++
 drivers/video/fbdev/omap2/dss/dss.h |   4 ++
 2 files changed, 119 insertions(+)

diff --git a/drivers/video/fbdev/omap2/dss/dss.c b/drivers/video/fbdev/omap2/dss/dss.c
index 8c79839fbe87..d75238f2c537 100644
--- a/drivers/video/fbdev/omap2/dss/dss.c
+++ b/drivers/video/fbdev/omap2/dss/dss.c
@@ -34,6 +34,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/gfp.h>
 #include <linux/sizes.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
 #include <linux/of.h>
 
 #include <video/omapdss.h>
@@ -78,6 +80,8 @@ struct dss_features {
 static struct {
 	struct platform_device *pdev;
 	void __iomem    *base;
+	struct regmap	*syscon_pll_ctrl;
+	u32		syscon_pll_ctrl_offset;
 
 	struct clk	*parent_clk;
 	struct clk	*dss_clk;
@@ -158,6 +162,99 @@ static void dss_restore_context(void)
 #undef SR
 #undef RR
 
+void dss_ctrl_pll_enable(enum dss_pll_id pll_id, bool enable)
+{
+	unsigned shift;
+	unsigned val;
+
+	if (!dss.syscon_pll_ctrl)
+		return;
+
+	val = !enable;
+
+	switch (pll_id) {
+	case DSS_PLL_VIDEO1:
+		shift = 0;
+		break;
+	case DSS_PLL_VIDEO2:
+		shift = 1;
+		break;
+	case DSS_PLL_HDMI:
+		shift = 2;
+		break;
+	default:
+		DSSERR("illegal DSS PLL ID %d\n", pll_id);
+		return;
+	}
+
+	regmap_update_bits(dss.syscon_pll_ctrl, dss.syscon_pll_ctrl_offset,
+		1 << shift, val << shift);
+}
+
+void dss_ctrl_pll_set_control_mux(enum dss_pll_id pll_id,
+	enum omap_channel channel)
+{
+	unsigned shift, val;
+
+	if (!dss.syscon_pll_ctrl)
+		return;
+
+	switch (channel) {
+	case OMAP_DSS_CHANNEL_LCD:
+		shift = 3;
+
+		switch (pll_id) {
+		case DSS_PLL_VIDEO1:
+			val = 0; break;
+		case DSS_PLL_HDMI:
+			val = 1; break;
+		default:
+			DSSERR("error in PLL mux config for LCD\n");
+			return;
+		}
+
+		break;
+	case OMAP_DSS_CHANNEL_LCD2:
+		shift = 5;
+
+		switch (pll_id) {
+		case DSS_PLL_VIDEO1:
+			val = 0; break;
+		case DSS_PLL_VIDEO2:
+			val = 1; break;
+		case DSS_PLL_HDMI:
+			val = 2; break;
+		default:
+			DSSERR("error in PLL mux config for LCD2\n");
+			return;
+		}
+
+		break;
+	case OMAP_DSS_CHANNEL_LCD3:
+		shift = 7;
+
+		switch (pll_id) {
+		case DSS_PLL_VIDEO1:
+			val = 1; break;
+		case DSS_PLL_VIDEO2:
+			val = 0; break;
+		case DSS_PLL_HDMI:
+			val = 2; break;
+		default:
+			DSSERR("error in PLL mux config for LCD3\n");
+			return;
+		}
+
+		break;
+	default:
+		DSSERR("error in PLL mux config\n");
+		return;
+	}
+
+	regmap_update_bits(dss.syscon_pll_ctrl, dss.syscon_pll_ctrl_offset,
+		0x3 << shift, val << shift);
+}
+
 void dss_sdi_init(int datapairs)
 {
 	u32 l;
@@ -923,6 +1020,7 @@ static void __exit dss_uninit_ports(struct platform_device *pdev)
 static int __init omap_dsshw_probe(struct platform_device *pdev)
 {
 	struct resource *dss_mem;
+	struct device_node *np = pdev->dev.of_node;
 	u32 rev;
 	int r;
 
@@ -979,6 +1077,23 @@ static int __init omap_dsshw_probe(struct platform_device *pdev)
 
 	dss_init_ports(pdev);
 
+	if (np && of_property_read_bool(np, "syscon-pll-ctrl")) {
+		dss.syscon_pll_ctrl = syscon_regmap_lookup_by_phandle(np,
+			"syscon-pll-ctrl");
+		if (IS_ERR(dss.syscon_pll_ctrl)) {
+			dev_err(&pdev->dev,
+				"failed to get syscon-pll-ctrl regmap\n");
+			return PTR_ERR(dss.syscon_pll_ctrl);
+		}
+
+		if (of_property_read_u32_index(np, "syscon-pll-ctrl", 1,
+				&dss.syscon_pll_ctrl_offset)) {
+			dev_err(&pdev->dev,
+				"failed to get syscon-pll-ctrl offset\n");
+			return -EINVAL;
+		}
+	}
+
 	rev = dss_read_reg(DSS_REVISION);
 	printk(KERN_INFO "OMAP DSS rev %d.%d\n",
 			FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0));
diff --git a/drivers/video/fbdev/omap2/dss/dss.h b/drivers/video/fbdev/omap2/dss/dss.h
index 4bca36a591ca..76a6eb1da090 100644
--- a/drivers/video/fbdev/omap2/dss/dss.h
+++ b/drivers/video/fbdev/omap2/dss/dss.h
@@ -274,6 +274,10 @@ u32 dss_of_port_get_port_number(struct device_node *port);
 void dss_debug_dump_clocks(struct seq_file *s);
 #endif
 
+void dss_ctrl_pll_enable(enum dss_pll_id pll_id, bool enable);
+void dss_ctrl_pll_set_control_mux(enum dss_pll_id pll_id,
+	enum omap_channel channel);
+
 void dss_sdi_init(int datapairs);
 int dss_sdi_enable(void);
 void dss_sdi_disable(void);
-- 
2.2.2


^ permalink raw reply related

* [PATCH 10/14] OMAPDSS: Add Video PLLs for DRA7xx
From: Tomi Valkeinen @ 2015-01-27 10:50 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: Tomi Valkeinen
In-Reply-To: <1422355842-11234-1-git-send-email-tomi.valkeinen@ti.com>

DRA7xx SoCs have one (DRA72x) or two (DRA74x) video PLLs. They are
basically the same as DSI PLLs on OMAPs, but without the rest of the DSI
hardware. The video PLLs also require some configuration via the CONTROL
module.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/fbdev/omap2/dss/Makefile    |   2 +-
 drivers/video/fbdev/omap2/dss/dss.c       |  58 +++++++-
 drivers/video/fbdev/omap2/dss/dss.h       |   8 ++
 drivers/video/fbdev/omap2/dss/video-pll.c | 211 ++++++++++++++++++++++++++++++
 4 files changed, 273 insertions(+), 6 deletions(-)
 create mode 100644 drivers/video/fbdev/omap2/dss/video-pll.c

diff --git a/drivers/video/fbdev/omap2/dss/Makefile b/drivers/video/fbdev/omap2/dss/Makefile
index 2ea9d382354c..b5136d3d4b77 100644
--- a/drivers/video/fbdev/omap2/dss/Makefile
+++ b/drivers/video/fbdev/omap2/dss/Makefile
@@ -2,7 +2,7 @@ obj-$(CONFIG_OMAP2_DSS_INIT) += omapdss-boot-init.o
 obj-$(CONFIG_OMAP2_DSS) += omapdss.o
 # Core DSS files
 omapdss-y := core.o dss.o dss_features.o dispc.o dispc_coefs.o display.o \
-	output.o dss-of.o pll.o
+	output.o dss-of.o pll.o video-pll.o
 # DSS compat layer files
 omapdss-y += manager.o manager-sysfs.o overlay.o overlay-sysfs.o apply.o \
 	dispc-compat.o display-sysfs.o
diff --git a/drivers/video/fbdev/omap2/dss/dss.c b/drivers/video/fbdev/omap2/dss/dss.c
index d75238f2c537..a6d10d4279f3 100644
--- a/drivers/video/fbdev/omap2/dss/dss.c
+++ b/drivers/video/fbdev/omap2/dss/dss.c
@@ -37,6 +37,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
 #include <linux/of.h>
+#include <linux/regulator/consumer.h>
 
 #include <video/omapdss.h>
 
@@ -65,9 +66,6 @@ struct dss_reg {
 #define REG_FLD_MOD(idx, val, start, end) \
 	dss_write_reg(idx, FLD_MOD(dss_read_reg(idx), val, start, end))
 
-static int dss_runtime_get(void);
-static void dss_runtime_put(void);
-
 struct dss_features {
 	u8 fck_div_max;
 	u8 dss_fck_multiplier;
@@ -99,6 +97,9 @@ static struct {
 	u32		ctx[DSS_SZ_REGS / sizeof(u32)];
 
 	const struct dss_features *feat;
+
+	struct dss_pll	*video1_pll;
+	struct dss_pll	*video2_pll;
 } dss;
 
 static const char * const dss_generic_clk_source_names[] = {
@@ -760,7 +761,7 @@ static void dss_put_clocks(void)
 		clk_put(dss.parent_clk);
 }
 
-static int dss_runtime_get(void)
+int dss_runtime_get(void)
 {
 	int r;
 
@@ -771,7 +772,7 @@ static int dss_runtime_get(void)
 	return r < 0 ? r : 0;
 }
 
-static void dss_runtime_put(void)
+void dss_runtime_put(void)
 {
 	int r;
 
@@ -1023,6 +1024,7 @@ static int __init omap_dsshw_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	u32 rev;
 	int r;
+	struct regulator *pll_regulator;
 
 	dss.pdev = pdev;
 
@@ -1094,6 +1096,40 @@ static int __init omap_dsshw_probe(struct platform_device *pdev)
 		}
 	}
 
+	pll_regulator = devm_regulator_get(&pdev->dev, "vdda_video");
+	if (IS_ERR(pll_regulator)) {
+		r = PTR_ERR(pll_regulator);
+
+		switch (r) {
+		case -ENOENT:
+			pll_regulator = NULL;
+			break;
+
+		case -EPROBE_DEFER:
+			return -EPROBE_DEFER;
+
+		default:
+			DSSERR("can't get DPLL VDDA regulator\n");
+			return r;
+		}
+	}
+
+	if (of_property_match_string(np, "reg-names", "pll1") >= 0) {
+		dss.video1_pll = dss_video_pll_init(pdev, 0, pll_regulator);
+		if (IS_ERR(dss.video1_pll)) {
+			r = PTR_ERR(dss.video1_pll);
+			goto err_pll_init;
+		}
+	}
+
+	if (of_property_match_string(np, "reg-names", "pll2") >= 0) {
+		dss.video2_pll = dss_video_pll_init(pdev, 1, pll_regulator);
+		if (IS_ERR(dss.video2_pll)) {
+			r = PTR_ERR(dss.video2_pll);
+			goto err_pll_init;
+		}
+	}
+
 	rev = dss_read_reg(DSS_REVISION);
 	printk(KERN_INFO "OMAP DSS rev %d.%d\n",
 			FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0));
@@ -1104,6 +1140,12 @@ static int __init omap_dsshw_probe(struct platform_device *pdev)
 
 	return 0;
 
+err_pll_init:
+	if (dss.video1_pll)
+		dss_video_pll_uninit(dss.video1_pll);
+
+	if (dss.video2_pll)
+		dss_video_pll_uninit(dss.video2_pll);
 err_runtime_get:
 	pm_runtime_disable(&pdev->dev);
 err_setup_clocks:
@@ -1113,6 +1155,12 @@ err_setup_clocks:
 
 static int __exit omap_dsshw_remove(struct platform_device *pdev)
 {
+	if (dss.video1_pll)
+		dss_video_pll_uninit(dss.video1_pll);
+
+	if (dss.video2_pll)
+		dss_video_pll_uninit(dss.video2_pll);
+
 	dss_uninit_ports(pdev);
 
 	pm_runtime_disable(&pdev->dev);
diff --git a/drivers/video/fbdev/omap2/dss/dss.h b/drivers/video/fbdev/omap2/dss/dss.h
index 76a6eb1da090..4812eee2622a 100644
--- a/drivers/video/fbdev/omap2/dss/dss.h
+++ b/drivers/video/fbdev/omap2/dss/dss.h
@@ -259,6 +259,9 @@ void dss_overlay_kobj_uninit(struct omap_overlay *ovl);
 int dss_init_platform_driver(void) __init;
 void dss_uninit_platform_driver(void);
 
+int dss_runtime_get(void);
+void dss_runtime_put(void);
+
 unsigned long dss_get_dispc_clk_rate(void);
 int dss_dpi_select_source(int port, enum omap_channel channel);
 void dss_select_hdmi_venc_clk_source(enum dss_hdmi_venc_clk_source_select);
@@ -266,6 +269,11 @@ enum dss_hdmi_venc_clk_source_select dss_get_hdmi_venc_clk_source(void);
 const char *dss_get_generic_clk_source_name(enum omap_dss_clk_source clk_src);
 void dss_dump_clocks(struct seq_file *s);
 
+/* DSS VIDEO PLL */
+struct dss_pll *dss_video_pll_init(struct platform_device *pdev, int id,
+	struct regulator *regulator);
+void dss_video_pll_uninit(struct dss_pll *pll);
+
 /* dss-of */
 struct device_node *dss_of_port_get_parent_device(struct device_node *port);
 u32 dss_of_port_get_port_number(struct device_node *port);
diff --git a/drivers/video/fbdev/omap2/dss/video-pll.c b/drivers/video/fbdev/omap2/dss/video-pll.c
new file mode 100644
index 000000000000..b1ec59e42940
--- /dev/null
+++ b/drivers/video/fbdev/omap2/dss/video-pll.c
@@ -0,0 +1,211 @@
+/*
+* Copyright (C) 2014 Texas Instruments Ltd
+*
+* This program is free software; you can redistribute it and/or modify it
+* under the terms of the GNU General Public License version 2 as published by
+* the Free Software Foundation.
+*
+* You should have received a copy of the GNU General Public License along with
+* this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/sched.h>
+
+#include <video/omapdss.h>
+
+#include "dss.h"
+#include "dss_features.h"
+
+struct dss_video_pll {
+	struct dss_pll pll;
+
+	struct device *dev;
+
+	void __iomem *clkctrl_base;
+};
+
+#define REG_MOD(reg, val, start, end) \
+	writel_relaxed(FLD_MOD(readl_relaxed(reg), val, start, end), reg)
+
+static void dss_dpll_enable_scp_clk(struct dss_video_pll *vpll)
+{
+	REG_MOD(vpll->clkctrl_base, 1, 14, 14); /* CIO_CLK_ICG */
+}
+
+static void dss_dpll_disable_scp_clk(struct dss_video_pll *vpll)
+{
+	REG_MOD(vpll->clkctrl_base, 0, 14, 14); /* CIO_CLK_ICG */
+}
+
+static void dss_dpll_power_enable(struct dss_video_pll *vpll)
+{
+	REG_MOD(vpll->clkctrl_base, 2, 31, 30); /* PLL_POWER_ON_ALL */
+
+	/*
+	 * DRA7x PLL CTRL's PLL_PWR_STATUS seems to always return 0,
+	 * so we have to use fixed delay here.
+	 */
+	msleep(1);
+}
+
+static void dss_dpll_power_disable(struct dss_video_pll *vpll)
+{
+	REG_MOD(vpll->clkctrl_base, 0, 31, 30);	/* PLL_POWER_OFF */
+}
+
+static int dss_video_pll_enable(struct dss_pll *pll)
+{
+	struct dss_video_pll *vpll = container_of(pll, struct dss_video_pll, pll);
+	int r;
+
+	r = dss_runtime_get();
+	if (r)
+		return r;
+
+	dss_ctrl_pll_enable(pll->id, true);
+
+	dss_dpll_enable_scp_clk(vpll);
+
+	r = dss_pll_wait_reset_done(pll);
+	if (r)
+		goto err_reset;
+
+	dss_dpll_power_enable(vpll);
+
+	return 0;
+
+err_reset:
+	dss_dpll_disable_scp_clk(vpll);
+	dss_ctrl_pll_enable(pll->id, false);
+	dss_runtime_put();
+
+	return r;
+}
+
+static void dss_video_pll_disable(struct dss_pll *pll)
+{
+	struct dss_video_pll *vpll = container_of(pll, struct dss_video_pll, pll);
+
+	dss_dpll_power_disable(vpll);
+
+	dss_dpll_disable_scp_clk(vpll);
+
+	dss_ctrl_pll_enable(pll->id, false);
+
+	dss_runtime_put();
+}
+
+static const struct dss_pll_ops dss_pll_ops = {
+	.enable = dss_video_pll_enable,
+	.disable = dss_video_pll_disable,
+	.set_config = dss_pll_write_config_type_a,
+};
+
+static const struct dss_pll_hw dss_dra7_video_pll_hw = {
+	.n_max = (1 << 8) - 1,
+	.m_max = (1 << 12) - 1,
+	.mX_max = (1 << 5) - 1,
+	.fint_min = 500000,
+	.fint_max = 2500000,
+	.clkdco_max = 1800000000,
+
+	.n_msb = 8,
+	.n_lsb = 1,
+	.m_msb = 20,
+	.m_lsb = 9,
+
+	.mX_msb[0] = 25,
+	.mX_lsb[0] = 21,
+	.mX_msb[1] = 30,
+	.mX_lsb[1] = 26,
+
+	.has_refsel = true,
+};
+
+struct dss_pll *dss_video_pll_init(struct platform_device *pdev, int id,
+	struct regulator *regulator)
+{
+	const char * const reg_name[] = { "pll1", "pll2" };
+	const char * const clkctrl_name[] = { "pll1_clkctrl", "pll2_clkctrl" };
+	const char * const clkin_name[] = { "video1_clk", "video2_clk" };
+
+	struct resource *res;
+	struct dss_video_pll *vpll;
+	void __iomem *pll_base, *clkctrl_base;
+	struct clk *clk;
+	struct dss_pll *pll;
+	int r;
+
+	/* PLL CONTROL */
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, reg_name[id]);
+	if (!res) {
+		dev_err(&pdev->dev,
+			"missing platform resource data for pll%d\n", id);
+		return ERR_PTR(-ENODEV);
+	}
+
+	pll_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pll_base)) {
+		dev_err(&pdev->dev, "failed to ioremap pll%d reg_name\n", id);
+		return ERR_CAST(pll_base);
+	}
+
+	/* CLOCK CONTROL */
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+		clkctrl_name[id]);
+	if (!res) {
+		dev_err(&pdev->dev,
+			"missing platform resource data for pll%d\n", id);
+		return ERR_PTR(-ENODEV);
+	}
+
+	clkctrl_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(clkctrl_base)) {
+		dev_err(&pdev->dev, "failed to ioremap pll%d clkctrl\n", id);
+		return ERR_CAST(clkctrl_base);
+	}
+
+	/* CLKIN */
+
+	clk = devm_clk_get(&pdev->dev, clkin_name[id]);
+	if (IS_ERR(clk)) {
+		DSSERR("can't get video pll clkin\n");
+		return ERR_CAST(clk);
+	}
+
+	vpll = devm_kzalloc(&pdev->dev, sizeof(*vpll), GFP_KERNEL);
+	if (!vpll)
+		return ERR_PTR(-ENOMEM);
+
+	vpll->dev = &pdev->dev;
+	vpll->clkctrl_base = clkctrl_base;
+
+	pll = &vpll->pll;
+
+	pll->name = id = 0 ? "video0" : "video1";
+	pll->id = id = 0 ? DSS_PLL_VIDEO1 : DSS_PLL_VIDEO2;
+	pll->clkin = clk;
+	pll->regulator = regulator;
+	pll->base = pll_base;
+	pll->hw = &dss_dra7_video_pll_hw;
+	pll->ops = &dss_pll_ops;
+
+	r = dss_pll_register(pll);
+	if (r)
+		return ERR_PTR(r);
+
+	return pll;
+}
+
+void dss_video_pll_uninit(struct dss_pll *pll)
+{
+	dss_pll_unregister(pll);
+}
-- 
2.2.2


^ permalink raw reply related

* [PATCH 11/14] OMAPDSS: DISPC: Add DRA7xx support
From: Tomi Valkeinen @ 2015-01-27 10:50 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: Tomi Valkeinen
In-Reply-To: <1422355842-11234-1-git-send-email-tomi.valkeinen@ti.com>

Add DRA7xx support to DISPC driver. The DISPC block is the same as on
OMAP5, except the PLL's used for clocking are "videoX", not "dsiX".

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/fbdev/omap2/dss/dispc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/video/fbdev/omap2/dss/dispc.c b/drivers/video/fbdev/omap2/dss/dispc.c
index 9850d9ef9a9d..48429bab294a 100644
--- a/drivers/video/fbdev/omap2/dss/dispc.c
+++ b/drivers/video/fbdev/omap2/dss/dispc.c
@@ -3037,10 +3037,16 @@ unsigned long dispc_fclk_rate(void)
 		break;
 	case OMAP_DSS_CLK_SRC_DSI_PLL_HSDIV_DISPC:
 		pll = dss_pll_find("dsi0");
+		if (!pll)
+			pll = dss_pll_find("video0");
+
 		r = pll->cinfo.clkout[0];
 		break;
 	case OMAP_DSS_CLK_SRC_DSI2_PLL_HSDIV_DISPC:
 		pll = dss_pll_find("dsi1");
+		if (!pll)
+			pll = dss_pll_find("video1");
+
 		r = pll->cinfo.clkout[0];
 		break;
 	default:
@@ -3069,10 +3075,16 @@ unsigned long dispc_mgr_lclk_rate(enum omap_channel channel)
 			break;
 		case OMAP_DSS_CLK_SRC_DSI_PLL_HSDIV_DISPC:
 			pll = dss_pll_find("dsi0");
+			if (!pll)
+				pll = dss_pll_find("video0");
+
 			r = pll->cinfo.clkout[0];
 			break;
 		case OMAP_DSS_CLK_SRC_DSI2_PLL_HSDIV_DISPC:
 			pll = dss_pll_find("dsi1");
+			if (!pll)
+				pll = dss_pll_find("video1");
+
 			r = pll->cinfo.clkout[0];
 			break;
 		default:
@@ -3668,6 +3680,7 @@ static int __init dispc_init_features(struct platform_device *pdev)
 		break;
 
 	case OMAPDSS_VER_OMAP5:
+	case OMAPDSS_VER_DRA7xx:
 		src = &omap54xx_dispc_feats;
 		break;
 
@@ -3832,6 +3845,7 @@ static const struct of_device_id dispc_of_match[] = {
 	{ .compatible = "ti,omap3-dispc", },
 	{ .compatible = "ti,omap4-dispc", },
 	{ .compatible = "ti,omap5-dispc", },
+	{ .compatible = "ti,dra7-dispc", },
 	{},
 };
 
-- 
2.2.2


^ permalink raw reply related

* [PATCH 12/14] OMAPDSS: DISPC: program dispc polarities to control module
From: Tomi Valkeinen @ 2015-01-27 10:50 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: Tomi Valkeinen
In-Reply-To: <1422355842-11234-1-git-send-email-tomi.valkeinen@ti.com>

On DRA7xx, DISPC needs to write output signal polarities not only to a
DISPC register, like for all earlier DSS versions, but to control
module's CTRL_CORE_SMA_SW_1 register.

This patch adds support to write the polarities to control module.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/fbdev/omap2/dss/dispc.c | 40 +++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/video/fbdev/omap2/dss/dispc.c b/drivers/video/fbdev/omap2/dss/dispc.c
index 48429bab294a..31b743c70272 100644
--- a/drivers/video/fbdev/omap2/dss/dispc.c
+++ b/drivers/video/fbdev/omap2/dss/dispc.c
@@ -36,6 +36,9 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/sizes.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/of.h>
 
 #include <video/omapdss.h>
 
@@ -117,6 +120,9 @@ static struct {
 	const struct dispc_features *feat;
 
 	bool is_enabled;
+
+	struct regmap *syscon_pol;
+	u32 syscon_pol_offset;
 } dispc;
 
 enum omap_color_component {
@@ -2958,6 +2964,25 @@ static void _dispc_mgr_set_lcd_timings(enum omap_channel channel, int hsw,
 		FLD_VAL(vsync_level, 12, 12);
 
 	dispc_write_reg(DISPC_POL_FREQ(channel), l);
+
+	if (dispc.syscon_pol) {
+		const int shifts[] = {
+			[OMAP_DSS_CHANNEL_LCD] = 0,
+			[OMAP_DSS_CHANNEL_LCD2] = 1,
+			[OMAP_DSS_CHANNEL_LCD3] = 2,
+		};
+
+		u32 mask, val;
+
+		mask = (1 << 0) | (1 << 3) | (1 << 6);
+		val = (rf << 0) | (ipc << 3) | (onoff << 6);
+
+		mask <<= 16 + shifts[channel];
+		val <<= 16 + shifts[channel];
+
+		regmap_update_bits(dispc.syscon_pol, dispc.syscon_pol_offset,
+			mask, val);
+	}
 }
 
 /* change name to mode? */
@@ -3741,6 +3766,7 @@ static int __init omap_dispchw_probe(struct platform_device *pdev)
 	u32 rev;
 	int r = 0;
 	struct resource *dispc_mem;
+	struct device_node *np = pdev->dev.of_node;
 
 	dispc.pdev = pdev;
 
@@ -3767,6 +3793,20 @@ static int __init omap_dispchw_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	if (np && of_property_read_bool(np, "syscon-pol")) {
+		dispc.syscon_pol = syscon_regmap_lookup_by_phandle(np, "syscon-pol");
+		if (IS_ERR(dispc.syscon_pol)) {
+			dev_err(&pdev->dev, "failed to get syscon-pol regmap\n");
+			return PTR_ERR(dispc.syscon_pol);
+		}
+
+		if (of_property_read_u32_index(np, "syscon-pol", 1,
+				&dispc.syscon_pol_offset)) {
+			dev_err(&pdev->dev, "failed to get syscon-pol offset\n");
+			return -EINVAL;
+		}
+	}
+
 	pm_runtime_enable(&pdev->dev);
 
 	r = dispc_runtime_get();
-- 
2.2.2


^ permalink raw reply related

* [PATCH 13/14] OMAPDSS: HDMI: Add DRA7xx support
From: Tomi Valkeinen @ 2015-01-27 10:50 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: Tomi Valkeinen
In-Reply-To: <1422355842-11234-1-git-send-email-tomi.valkeinen@ti.com>

Add support for DRA7xx to the HDMI driver.

The HDMI block on DRA7xx is the same as on OMAP5, except we need to
enable and disable the HDMI PLL via the CONTROL module.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/fbdev/omap2/dss/hdmi5.c    | 1 +
 drivers/video/fbdev/omap2/dss/hdmi_phy.c | 1 +
 drivers/video/fbdev/omap2/dss/hdmi_pll.c | 5 +++++
 3 files changed, 7 insertions(+)

diff --git a/drivers/video/fbdev/omap2/dss/hdmi5.c b/drivers/video/fbdev/omap2/dss/hdmi5.c
index 39aae3aa7136..3f0b34a7031a 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi5.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi5.c
@@ -787,6 +787,7 @@ static const struct dev_pm_ops hdmi_pm_ops = {
 
 static const struct of_device_id hdmi_of_match[] = {
 	{ .compatible = "ti,omap5-hdmi", },
+	{ .compatible = "ti,dra7-hdmi", },
 	{},
 };
 
diff --git a/drivers/video/fbdev/omap2/dss/hdmi_phy.c b/drivers/video/fbdev/omap2/dss/hdmi_phy.c
index bc9e07d2afbe..1f5d19c119ce 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi_phy.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi_phy.c
@@ -208,6 +208,7 @@ static int hdmi_phy_init_features(struct platform_device *pdev)
 		break;
 
 	case OMAPDSS_VER_OMAP5:
+	case OMAPDSS_VER_DRA7xx:
 		src = &omap54xx_phy_feats;
 		break;
 
diff --git a/drivers/video/fbdev/omap2/dss/hdmi_pll.c b/drivers/video/fbdev/omap2/dss/hdmi_pll.c
index b808f7c72d83..06e23a7c432c 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi_pll.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi_pll.c
@@ -104,6 +104,8 @@ static int hdmi_pll_enable(struct dss_pll *dsspll)
 	struct hdmi_wp_data *wp = pll->wp;
 	u16 r = 0;
 
+	dss_ctrl_pll_enable(DSS_PLL_HDMI, true);
+
 	r = hdmi_wp_set_pll_pwr(wp, HDMI_PLLPWRCMD_BOTHON_ALLCLKS);
 	if (r)
 		return r;
@@ -117,6 +119,8 @@ static void hdmi_pll_disable(struct dss_pll *dsspll)
 	struct hdmi_wp_data *wp = pll->wp;
 
 	hdmi_wp_set_pll_pwr(wp, HDMI_PLLPWRCMD_ALLOFF);
+
+	dss_ctrl_pll_enable(DSS_PLL_HDMI, false);
 }
 
 static const struct dss_pll_ops dsi_pll_ops = {
@@ -197,6 +201,7 @@ static int dsi_init_pll_data(struct platform_device *pdev, struct hdmi_pll_data
 		break;
 
 	case OMAPDSS_VER_OMAP5:
+	case OMAPDSS_VER_DRA7xx:
 		pll->hw = &dss_omap5_hdmi_pll_hw;
 		break;
 
-- 
2.2.2


^ permalink raw reply related

* [PATCH 14/14] OMAPDSS: DPI: DRA7xx support
From: Tomi Valkeinen @ 2015-01-27 10:50 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: Tomi Valkeinen
In-Reply-To: <1422355842-11234-1-git-send-email-tomi.valkeinen@ti.com>

Add support for DRA7xx DPI output.

DRA7xx has three DPI outputs, each of which gets its input from a DISPC
channel. However, DRA72x has only one video PLL, and DRA74x has two
video PLLs. In both cases the video PLLs need to be shared between
multiple outputs. The driver doesn't handle this at the moment.

Also, DRA7xx requires configuring CONTROL module bits to route the clock
from the PLL to the used DISPC channel.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/fbdev/omap2/dss/dpi.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/video/fbdev/omap2/dss/dpi.c b/drivers/video/fbdev/omap2/dss/dpi.c
index 9a2f8c3b102d..f83e7b030249 100644
--- a/drivers/video/fbdev/omap2/dss/dpi.c
+++ b/drivers/video/fbdev/omap2/dss/dpi.c
@@ -106,6 +106,17 @@ static struct dss_pll *dpi_get_pll(enum omap_channel channel)
 			return NULL;
 		}
 
+	case OMAPDSS_VER_DRA7xx:
+		switch (channel) {
+		case OMAP_DSS_CHANNEL_LCD:
+		case OMAP_DSS_CHANNEL_LCD2:
+			return dss_pll_find("video0");
+		case OMAP_DSS_CHANNEL_LCD3:
+			return dss_pll_find("video1");
+		default:
+			return NULL;
+		}
+
 	default:
 		return NULL;
 	}
@@ -590,6 +601,10 @@ static void dpi_init_pll(struct dpi_data *dpi)
 	if (!pll)
 		return;
 
+	/* On DRA7 we need to set a mux to use the PLL */
+	if (omapdss_get_version() = OMAPDSS_VER_DRA7xx)
+		dss_ctrl_pll_set_control_mux(pll->id, dpi->output.dispc_channel);
+
 	if (dpi_verify_dsi_pll(pll)) {
 		DSSWARN("DSI PLL not operational\n");
 		return;
@@ -615,6 +630,17 @@ static enum omap_channel dpi_get_channel(int port_num)
 	case OMAPDSS_VER_AM43xx:
 		return OMAP_DSS_CHANNEL_LCD;
 
+	case OMAPDSS_VER_DRA7xx:
+		switch (port_num) {
+		case 2:
+			return OMAP_DSS_CHANNEL_LCD3;
+		case 1:
+			return OMAP_DSS_CHANNEL_LCD2;
+		case 0:
+		default:
+			return OMAP_DSS_CHANNEL_LCD;
+		}
+
 	case OMAPDSS_VER_OMAP4430_ES1:
 	case OMAPDSS_VER_OMAP4430_ES2:
 	case OMAPDSS_VER_OMAP4:
-- 
2.2.2


^ permalink raw reply related

* Re: [PATCH] video: fbdev: sis: remove unused variables
From: Sudip Mukherjee @ 2015-01-27 13:20 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Thomas Winischhofer, Jean-Christophe Plagniol-Villard,
	linux-fbdev, linux-kernel
In-Reply-To: <54C63506.3020106@ti.com>

On Mon, Jan 26, 2015 at 02:37:26PM +0200, Tomi Valkeinen wrote:
> On 22/01/15 17:31, Sudip Mukherjee wrote:
> > removed some variables which were not used. Few calls to SiS_GetReg()
> > were left behind as they are reading from the hardare, removing them
> > might affect the overall functionality.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > ---
> > This patch will generate checkpatch error, to fix that error we need
> > to change the style of init.c and init301.c
> > 
> >  drivers/video/fbdev/sis/init.c     | 33 +++++----------------------------
> >  drivers/video/fbdev/sis/init301.c  | 10 ++--------
> >  drivers/video/fbdev/sis/sis_main.c |  9 ++++-----
> >  3 files changed, 11 insertions(+), 41 deletions(-)
> 
> Are you able to test this?
> 
> The patch doesn't look too complex, but it's still slightly too complex
> for me to comfortably merge it without any testing.

No. It is not tested on actual hardware.

Sudip

> 
>  Tomi
> 
> 



^ permalink raw reply

* Re: [PATCH] OMAPDSS: hdmi5: remove unneeded check
From: Sudip Mukherjee @ 2015-01-27 13:42 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Jean-Christophe Plagniol-Villard, linux-omap, linux-fbdev,
	linux-kernel
In-Reply-To: <54C63606.30002@ti.com>

On Mon, Jan 26, 2015 at 02:41:42PM +0200, Tomi Valkeinen wrote:
> On 13/01/15 18:46, Sudip Mukherjee wrote:
> > prior to this check we are checking for word_length_16b and if word_length_16b
> > is false then we are returning with -EINVAL.
> > So at this point word_length_16b can only be true.
> 
> True, but it looks to me the code may be extended in the future.
> 
> And if it would be clear that it won't be extended in the future, then
> there's more code changes needed to reflect that (the whole
> word_length_16b can be removed, etc).

you are the author of the code, so you will know if it will be extended .. :)

Sudip
> 
>  Tomi
> 
> 



^ permalink raw reply

* [PATCH] fb: via: turn gpiolib and i2c selects into dependencies
From: Arnd Bergmann @ 2015-01-28 20:12 UTC (permalink / raw)
  To: linux-fbdev

Device driver should not directly select subsystems. In this case
we get build warnings like

warning: (ARCH_REQUIRE_GPIOLIB && PINCTRL_AT91 && PINCTRL_NOMADIK && MFD_TC6393XB && FB_VIA) selects GPIOLIB which has unmet direct dependencies (ARCH_WANT_OPTIONAL_GPIOLIB || ARCH_REQUIRE_GPIOLIB)

which we can avoid using the normal 'depends on' statement.

Also, this patch makes it possible for DRM drivers to have a dependency
on GPIOLIB without getting circular Kconfig dependencies.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index f2c3fb7d0399..b3dd417b4719 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1530,13 +1530,11 @@ config FB_SIS_315
 
 config FB_VIA
        tristate "VIA UniChrome (Pro) and Chrome9 display support"
-       depends on FB && PCI && X86
+       depends on FB && PCI && X86 && GPIOLIB && I2C
        select FB_CFB_FILLRECT
        select FB_CFB_COPYAREA
        select FB_CFB_IMAGEBLIT
        select I2C_ALGOBIT
-       select I2C
-       select GPIOLIB
        help
 	  This is the frame buffer device driver for Graphics chips of VIA
 	  UniChrome (Pro) Family (CLE266,PM800/CN400,P4M800CE/P4M800Pro/


^ permalink raw reply related

* [PATCH] video/mmpfb: allow modular build
From: Arnd Bergmann @ 2015-01-28 20:13 UTC (permalink / raw)
  To: linux-arm-kernel

The frame buffer core can be a module, which means any fb drivers
should be able to build as modules too. This turns mmpfb into
a tristate option to allow that and fix a possible randconfig
build error.

drivers/built-in.o: In function `modes_setup':
:(.text+0x11b34): undefined reference to `fb_videomode_to_modelist'
:(.text+0x11b5c): undefined reference to `fb_videomode_to_var'

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/video/fbdev/mmp/Makefile b/drivers/video/fbdev/mmp/Makefile
index a014cb358bf8..924dd0930cc7 100644
--- a/drivers/video/fbdev/mmp/Makefile
+++ b/drivers/video/fbdev/mmp/Makefile
@@ -1 +1,3 @@
-obj-y += core.o hw/ panel/ fb/
+obj-$(CONFIG_MMP_DISP) += mmp_disp.o hw/ panel/ fb/
+
+mmp_disp-y		+= core.o
diff --git a/drivers/video/fbdev/mmp/fb/Kconfig b/drivers/video/fbdev/mmp/fb/Kconfig
index 9b0141f105f5..985e1a7cd254 100644
--- a/drivers/video/fbdev/mmp/fb/Kconfig
+++ b/drivers/video/fbdev/mmp/fb/Kconfig
@@ -1,7 +1,7 @@
 if MMP_DISP
 
 config MMP_FB
-	bool "fb driver for Marvell MMP Display Subsystem"
+	tristate "fb driver for Marvell MMP Display Subsystem"
 	depends on FB
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA


^ permalink raw reply related

* i915 framebuffer init too slow to find logo
From: S. Gilles @ 2015-01-29  2:32 UTC (permalink / raw)
  To: Tomi Valkeinen, Jean-Christophe Plagniol-Villard, Maik Broemme
  Cc: linux-fbdev, linux-kernel

Since commit 92b004d1aa9f367c372511ca0330f58216b25703 : prevent use of
logs after they have been freed, my i915 machine has no logo on boot
(reverting that commit brings it back on recent trees). My .config
builds nothing but wireless as =m, so I think this is a genuine false
positive (as predicted by the commit). Examining an augmented dmesg,
it appears that the framebuffer setup is too slow by about 0.3s, which
I wouldn't really expect from this system/driver.

Is this slowness considered worth fixing, or is this issue considered
too cosmetic? (Or is this just PEBKAC?)

Possibly useful information:

$ lspci | grep VGA
00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller (rev 09)

From various printk()s, it looks like the slow portion of
fb_console_init() is restore_fbdev_mode(), specifically
drm_mode_set_config_internal(), which takes about 0.45s, while the
fb_logo_late_init() call happens about 0.15s into that. I can give the
full details if requested.

Possibly relevant .config bits:

# Graphics support
CONFIG_AGP=y
CONFIG_AGP_INTEL=y
CONFIG_INTEL_GTT=y
CONFIG_VGA_ARB=y
CONFIG_VGA_ARB_MAX_GPUS\x16

# Direct Rendering Manager
CONFIG_DRM=y
CONFIG_DRM_KMS_HELPER=y
CONFIG_DRM_KMS_FB_HELPER=y

# I2C encoder or helper chips
CONFIG_DRM_I915=y
CONFIG_DRM_I915_KMS=y
CONFIG_DRM_I915_FBDEV=y
CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT=y

# Frame buffer Devices
CONFIG_FB=y
CONFIG_FIRMWARE_EDID=y
CONFIG_FB_CMDLINE=y
CONFIG_FB_CFB_FILLRECT=y
CONFIG_FB_CFB_COPYAREA=y
CONFIG_FB_CFB_IMAGEBLIT=y

# Frame buffer hardware drivers
CONFIG_BACKLIGHT_LCD_SUPPORT=y
CONFIG_LCD_CLASS_DEVICE=y
CONFIG_LCD_PLATFORM=y
CONFIG_BACKLIGHT_CLASS_DEVICE=y
CONFIG_BACKLIGHT_GENERIC=y
CONFIG_HDMI=y

# Console display driver support
CONFIG_VGA_CONSOLE=y
CONFIG_DUMMY_CONSOLE=y
CONFIG_FRAMEBUFFER_CONSOLE=y
CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
CONFIG_LOGO=y
CONFIG_LOGO_LINUX_MONO=y
CONFIG_LOGO_LINUX_VGA16=y
CONFIG_LOGO_LINUX_CLUT224=y

-- 
S. Gilles

^ permalink raw reply

* Re: i915 framebuffer init too slow to find logo
From: Tomi Valkeinen @ 2015-01-29  8:09 UTC (permalink / raw)
  To: S. Gilles, Jean-Christophe Plagniol-Villard, Maik Broemme
  Cc: linux-fbdev, linux-kernel, Thierry Reding
In-Reply-To: <20150129023247.GA10967@number16>

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

On 29/01/15 04:32, S. Gilles wrote:
> Since commit 92b004d1aa9f367c372511ca0330f58216b25703 : prevent use of
> logs after they have been freed, my i915 machine has no logo on boot
> (reverting that commit brings it back on recent trees). My .config
> builds nothing but wireless as =m, so I think this is a genuine false
> positive (as predicted by the commit). Examining an augmented dmesg,

It's not so much about modules, but when the code tries to use the
logos. Drivers as modules might cause the use of logos to happen later,
but that's only one possible reason.

> it appears that the framebuffer setup is too slow by about 0.3s, which
> I wouldn't really expect from this system/driver.
> 
> Is this slowness considered worth fixing, or is this issue considered
> too cosmetic? (Or is this just PEBKAC?)
> 
> Possibly useful information:
> 
> $ lspci | grep VGA
> 00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller (rev 09)
> 
> From various printk()s, it looks like the slow portion of
> fb_console_init() is restore_fbdev_mode(), specifically
> drm_mode_set_config_internal(), which takes about 0.45s, while the
> fb_logo_late_init() call happens about 0.15s into that. I can give the
> full details if requested.

When does the driver probe() happen? Does the initialization happen
outside of the probe(), via workqueue or such? If so, then the fix is
valid for your case also, as the work could be ran after the logos have
been freed.

However, it does seem that the fix seems to cause logos to disappear for
many people. I'd be interesting to know how many of those cases were
working by luck, either by

1) an async work being ran fast enough, before the logos had been freed
2) the use of logos happening after the logos had been freed, but if no
one had trashed the logo memory yet, it still works

I don't care so much about the logo myself but people do seem to like
it, so perhaps we need to change the code as Thierry suggested:
allocating memory for the logos and keeping them in memory until someone
uses them the first time, and then free the memory.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 2/3 v2] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Nicholas Mc Guire @ 2015-01-29  9:38 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: K. Y. Srinivasan, Haiyang Zhang, devel, linux-fbdev, linux-kernel
In-Reply-To: <54C62D07.3040603@ti.com>

On Mon, 26 Jan 2015, Tomi Valkeinen wrote:

> Hi,
> 
> On 25/01/15 16:47, Nicholas Mc Guire wrote:
> > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > ---
> > 
> > v2: fixed subject line
> > 
> > The return type of wait_for_completion_timeout is unsigned long not
> > int. This patch fixes up the declarations only.
> > 
> > Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
> > CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m
> 
> Why didn't you set the text above as the patch description (which is
> empty at the moment)?
> 
basically because the one-line is sufficient to understand the patch
and the rest of the information is not relevant for the git log but only
for the review

if you think it is necessary to understand the patch I'll move it and
resubmit.

thanks for your review !

hofrat

^ permalink raw reply

* Re: [PATCH] video: treat signal like timeout as failure
From: Nicholas Mc Guire @ 2015-01-29  9:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20150126125905.GI26493@n2100.arm.linux.org.uk>

On Mon, 26 Jan 2015, Russell King - ARM Linux wrote:

> On Tue, Jan 20, 2015 at 06:23:50AM +0100, Nicholas Mc Guire wrote:
> > if(!wait_for_completion_interruptible_timeout(...))
> > only handles the timeout case - this patch adds handling the
> > signal case the same as timeout and cleans up.
> > 
> > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > ---
> > 
> > Only the timeout case was being handled, return of 0 in 
> > wait_for_completion_interruptible_timeout, the signal case (-ERESTARTSYS)
> > was treated just like the case of successful completion, which is most 
> > likely not reasonable.
> > 
> > Note that exynos_mipi_dsi_wr_data/exynos_mipi_dsi_rd_data return values
> > are not checked at the call sites in s6e8ax0.c (cmd_read/cmd_write)!
> > 
> > This patch simply treats the signal case the same way as the timeout case,
> > by releasing locks and returning 0 - which might not be the right thing to
> > do - this needs a review by someone knowing the details of this driver.
> > 
> > Patch is against 3.19.0-rc5 -next-20150119
> > 
> > Patch was only compile-tested with exynos_defconfig
> > 
> >  drivers/video/fbdev/exynos/exynos_mipi_dsi_common.c |   17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/video/fbdev/exynos/exynos_mipi_dsi_common.c b/drivers/video/fbdev/exynos/exynos_mipi_dsi_common.c
> > index 2358a2f..55a7a45 100644
> > --- a/drivers/video/fbdev/exynos/exynos_mipi_dsi_common.c
> > +++ b/drivers/video/fbdev/exynos/exynos_mipi_dsi_common.c
> > @@ -157,6 +157,7 @@ int exynos_mipi_dsi_wr_data(struct mipi_dsim_device *dsim, unsigned int data_id,
> >  	const unsigned char *data0, unsigned int data_size)
> >  {
> >  	unsigned int check_rx_ack = 0;
> > +	long timeout;
> >  
> >  	if (dsim->state = DSIM_STATE_ULPS) {
> >  		dev_err(dsim->dev, "state is ULPS.\n");
> > @@ -244,9 +245,11 @@ int exynos_mipi_dsi_wr_data(struct mipi_dsim_device *dsim, unsigned int data_id,
> >  		exynos_mipi_dsi_wr_tx_header(dsim, data_id, data_size & 0xff,
> >  			(data_size & 0xff00) >> 8);
> >  
> > -		if (!wait_for_completion_interruptible_timeout(&dsim_wr_comp,
> > -							MIPI_FIFO_TIMEOUT)) {
> > -			dev_warn(dsim->dev, "command write timeout.\n");
> > +		timeout = wait_for_completion_interruptible_timeout(
> > +					&dsim_wr_comp, MIPI_FIFO_TIMEOUT);
> > +		if (timeout <= 0) {
> > +			dev_warn(dsim->dev,
> > +				"command write timed-out/interrupted.\n");
> 
> This is really silly.  Let's say that the program which results in
> this function called is using signals (eg, alarm() with SIGALRM, or
> asynchronous IO with SIGIO, etc).
> 
> Why should having a SIGALRM raised print a kernel message?  If this
> happens a lot, it will result in the kernel log being flooded with
> these messages.
> 
> Signals should not be seen as exceptional conditions.  For some programs,
> they are merely asynchronous events which are a normal part of the
> programs operation (eg, SIGIO, SIGALRM, etc.)
> 
> Please, if you are going to handle signals, then handle them properly.
> If you're not going to handle them properly, don't use a wait that
> caters for them - use wait_for_completion_killable_timeout() which
> doesn't finish waiting on a signal unless the signal is going to result
> in the death of the program.
>

the current code would treat the signal case identical with the
completion success case - and that hardly can be the intention
so while it might not be necessary to call printk in the signal
case it should in some way be handled - if there is not need to 
handle signals then it might be more resonable to use
wait_for_completion_timeout which is not interruptible.

So the key issue here is not that a signal should necessarily print
a message but that it should not be treated as the success case. The
current code will only treat timeout as an error condition and a received
signal (implying that the condition being waited for is most likely not
satisfied) as a successful completion.

thx!
hofrat 

^ permalink raw reply

* Re: [PATCH 2/3 v2] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Dan Carpenter @ 2015-01-29  9:47 UTC (permalink / raw)
  To: Nicholas Mc Guire
  Cc: Tomi Valkeinen, devel, Haiyang Zhang, linux-fbdev, linux-kernel
In-Reply-To: <20150129093839.GB23666@opentech.at>

On Thu, Jan 29, 2015 at 10:38:39AM +0100, Nicholas Mc Guire wrote:
> On Mon, 26 Jan 2015, Tomi Valkeinen wrote:
> 
> > Hi,
> > 
> > On 25/01/15 16:47, Nicholas Mc Guire wrote:
> > > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > > ---
> > > 
> > > v2: fixed subject line
> > > 
> > > The return type of wait_for_completion_timeout is unsigned long not
> > > int. This patch fixes up the declarations only.
> > >

This line is relevant for the patch description.
 
> > > Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
> > > CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m

This line is not relevant.

regards,
dan carpenter



^ permalink raw reply

* [PATCH 2/3 v3] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Nicholas Mc Guire @ 2015-01-29 10:24 UTC (permalink / raw)
  To: K. Y. Srinivasan
  Cc: Haiyang Zhang, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	devel, linux-fbdev, linux-kernel, Nicholas Mc Guire

The return type of wait_for_completion_timeout is unsigned long not
int. This patch fixes up the declarations only.

Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---

v2: fixed subject line
v3: fixed patch description as recommended by Dan Carpenter
    <dan.carpenter@oracle.com>

Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m

Patch is against 3.19.0-rc5 -next-20150123

 drivers/video/fbdev/hyperv_fb.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 4254336..807ee22 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -415,7 +415,8 @@ static int synthvid_negotiate_ver(struct hv_device *hdev, u32 ver)
 	struct fb_info *info = hv_get_drvdata(hdev);
 	struct hvfb_par *par = info->par;
 	struct synthvid_msg *msg = (struct synthvid_msg *)par->init_buf;
-	int t, ret = 0;
+	int ret = 0;
+	unsigned long t;
 
 	memset(msg, 0, sizeof(struct synthvid_msg));
 	msg->vid_hdr.type = SYNTHVID_VERSION_REQUEST;
@@ -488,7 +489,8 @@ static int synthvid_send_config(struct hv_device *hdev)
 	struct fb_info *info = hv_get_drvdata(hdev);
 	struct hvfb_par *par = info->par;
 	struct synthvid_msg *msg = (struct synthvid_msg *)par->init_buf;
-	int t, ret = 0;
+	int ret = 0;
+	unsigned long t;
 
 	/* Send VRAM location */
 	memset(msg, 0, sizeof(struct synthvid_msg));
-- 
1.7.10.4


^ permalink raw reply related

* Re: [PATCH 2/3 v2] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Nicholas Mc Guire @ 2015-01-29 10:25 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Tomi Valkeinen, devel, Haiyang Zhang, linux-fbdev, linux-kernel
In-Reply-To: <20150129094704.GT6456@mwanda>

On Thu, 29 Jan 2015, Dan Carpenter wrote:

> On Thu, Jan 29, 2015 at 10:38:39AM +0100, Nicholas Mc Guire wrote:
> > On Mon, 26 Jan 2015, Tomi Valkeinen wrote:
> > 
> > > Hi,
> > > 
> > > On 25/01/15 16:47, Nicholas Mc Guire wrote:
> > > > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > > > ---
> > > > 
> > > > v2: fixed subject line
> > > > 
> > > > The return type of wait_for_completion_timeout is unsigned long not
> > > > int. This patch fixes up the declarations only.
> > > >
> 
> This line is relevant for the patch description.
>  
> > > > Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
> > > > CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m
> 
> This line is not relevant.
>
thanks - will resend it then - assumed that the one-line would do for this
patch.

thx!
hofrat 

^ permalink raw reply

* Re: [PATCH 2/3 v2] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Tomi Valkeinen @ 2015-01-29 10:34 UTC (permalink / raw)
  To: Nicholas Mc Guire
  Cc: K. Y. Srinivasan, Haiyang Zhang, devel, linux-fbdev, linux-kernel
In-Reply-To: <20150129093839.GB23666@opentech.at>

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

On 29/01/15 11:38, Nicholas Mc Guire wrote:
> On Mon, 26 Jan 2015, Tomi Valkeinen wrote:
> 
>> Hi,
>>
>> On 25/01/15 16:47, Nicholas Mc Guire wrote:
>>> Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
>>> ---
>>>
>>> v2: fixed subject line
>>>
>>> The return type of wait_for_completion_timeout is unsigned long not
>>> int. This patch fixes up the declarations only.
>>>
>>> Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
>>> CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m
>>
>> Why didn't you set the text above as the patch description (which is
>> empty at the moment)?
>>
> basically because the one-line is sufficient to understand the patch

You didn't have one line, you had no description. Patch subject is not
patch description. In the minimal case, the description should have the
same text as the subject, but usually it's better to have a bit more
text in the description.

> and the rest of the information is not relevant for the git log but only
> for the review
> 
> if you think it is necessary to understand the patch I'll move it and
> resubmit.

Well, a good description is not only about understanding the code in the
patch. It may contain information like which platform/setup this issue
happened on, are the any possible side effects, or whatever might be
relevant for someone looking at the patch years later.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 2/3 v2] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Nicholas Mc Guire @ 2015-01-29 10:41 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: K. Y. Srinivasan, Haiyang Zhang, devel, linux-fbdev, linux-kernel
In-Reply-To: <54CA0CBA.6030900@ti.com>

On Thu, 29 Jan 2015, Tomi Valkeinen wrote:

> On 29/01/15 11:38, Nicholas Mc Guire wrote:
> > On Mon, 26 Jan 2015, Tomi Valkeinen wrote:
> > 
> >> Hi,
> >>
> >> On 25/01/15 16:47, Nicholas Mc Guire wrote:
> >>> Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> >>> ---
> >>>
> >>> v2: fixed subject line
> >>>
> >>> The return type of wait_for_completion_timeout is unsigned long not
> >>> int. This patch fixes up the declarations only.
> >>>
> >>> Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
> >>> CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m
> >>
> >> Why didn't you set the text above as the patch description (which is
> >> empty at the moment)?
> >>
> > basically because the one-line is sufficient to understand the patch
> 
> You didn't have one line, you had no description. Patch subject is not
> patch description. In the minimal case, the description should have the
> same text as the subject, but usually it's better to have a bit more
> text in the description.

ok - was not clear about this - got it.

> 
> > and the rest of the information is not relevant for the git log but only
> > for the review
> > 
> > if you think it is necessary to understand the patch I'll move it and
> > resubmit.
> 
> Well, a good description is not only about understanding the code in the
> patch. It may contain information like which platform/setup this issue
> happened on, are the any possible side effects, or whatever might be
> relevant for someone looking at the patch years later.
> 
yup - but it seemed to me that the information on the build
config and kernel version details would not really be relevant for
this cleanup patch - so if I got your right the description line should
have gone up and the config/kernel info stays below "---".

Just resent it - hope this is correct now.

thx!
hofrat


^ permalink raw reply

* Re: [PATCH 2/3 v3] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Vitaly Kuznetsov @ 2015-01-29 13:02 UTC (permalink / raw)
  To: Nicholas Mc Guire
  Cc: K. Y. Srinivasan, linux-fbdev, Haiyang Zhang, linux-kernel,
	Tomi Valkeinen, devel, Jean-Christophe Plagniol-Villard
In-Reply-To: <1422527056-24929-1-git-send-email-der.herr@hofr.at>

Nicholas Mc Guire <der.herr@hofr.at> writes:

> The return type of wait_for_completion_timeout is unsigned long not
> int. This patch fixes up the declarations only.
>
> Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>

I would be slightly better to remove ".c" from your subject like,
anyway:

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>

> ---
>
> v2: fixed subject line
> v3: fixed patch description as recommended by Dan Carpenter
>     <dan.carpenter@oracle.com>
>
> Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
> CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m
>
> Patch is against 3.19.0-rc5 -next-20150123
>
>  drivers/video/fbdev/hyperv_fb.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> index 4254336..807ee22 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -415,7 +415,8 @@ static int synthvid_negotiate_ver(struct hv_device *hdev, u32 ver)
>  	struct fb_info *info = hv_get_drvdata(hdev);
>  	struct hvfb_par *par = info->par;
>  	struct synthvid_msg *msg = (struct synthvid_msg *)par->init_buf;
> -	int t, ret = 0;
> +	int ret = 0;
> +	unsigned long t;
>
>  	memset(msg, 0, sizeof(struct synthvid_msg));
>  	msg->vid_hdr.type = SYNTHVID_VERSION_REQUEST;
> @@ -488,7 +489,8 @@ static int synthvid_send_config(struct hv_device *hdev)
>  	struct fb_info *info = hv_get_drvdata(hdev);
>  	struct hvfb_par *par = info->par;
>  	struct synthvid_msg *msg = (struct synthvid_msg *)par->init_buf;
> -	int t, ret = 0;
> +	int ret = 0;
> +	unsigned long t;
>
>  	/* Send VRAM location */
>  	memset(msg, 0, sizeof(struct synthvid_msg));

-- 
  Vitaly

^ permalink raw reply

* Re: i915 framebuffer init too slow to find logo
From: S. Gilles @ 2015-01-29 14:59 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: S. Gilles, Jean-Christophe Plagniol-Villard, Maik Broemme,
	linux-fbdev, linux-kernel, Thierry Reding
In-Reply-To: <54C9EAC3.9040509@ti.com>

On 2015-01-29T10:09:39+0200, Tomi Valkeinen wrote:
> On 29/01/15 04:32, S. Gilles wrote:
> > Since commit 92b004d1aa9f367c372511ca0330f58216b25703 : prevent use of
> > logs after they have been freed, my i915 machine has no logo on boot
> > (reverting that commit brings it back on recent trees). My .config
> > builds nothing but wireless as =m, so I think this is a genuine false
> > positive (as predicted by the commit). Examining an augmented dmesg,
> 
> It's not so much about modules, but when the code tries to use the
> logos. Drivers as modules might cause the use of logos to happen later,
> but that's only one possible reason.
> 
> > it appears that the framebuffer setup is too slow by about 0.3s, which
> > I wouldn't really expect from this system/driver.
> > 
> > Is this slowness considered worth fixing, or is this issue considered
> > too cosmetic? (Or is this just PEBKAC?)
> > 
> > Possibly useful information:
> > 
> > $ lspci | grep VGA
> > 00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller (rev 09)
> > 
> > From various printk()s, it looks like the slow portion of
> > fb_console_init() is restore_fbdev_mode(), specifically
> > drm_mode_set_config_internal(), which takes about 0.45s, while the
> > fb_logo_late_init() call happens about 0.15s into that. I can give the
> > full details if requested.
> 
> When does the driver probe() happen? Does the initialization happen
> outside of the probe(), via workqueue or such? If so, then the fix is
> valid for your case also, as the work could be ran after the logos have
> been freed.

It looks like the fix is indeed valid, since the initialization
happens without probe() in the trace: the result of putting
dump_stack() at the beginning of the relevant functions is (in far too
much detail)

...
[    0.302391] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc5+ #26
[    0.302476] Hardware name: LENOVO 4286CTO/4286CTO, BIOS 8DET42WW (1.12 ) 04/01/2011
[    0.302561]  ffff880138ffd000 ffff880139563c98 ffffffff817945db 0000000000000126
[    0.302882]  ffffffff81849d20 ffff880139563cc8 ffffffff813b7ec4 ffff880138ffd000
[    0.303189]  ffffffff81848b10 ffffffff81c3ad58 ffff880138ffd090 ffff880139563cf8
[    0.303511] Call Trace:
[    0.303578]  [<ffffffff817945db>] dump_stack+0x45/0x57
[    0.303677]  [<ffffffff813b7ec4>] i915_pci_probe+0x1a/0x68
[    0.303769]  [<ffffffff8131c209>] pci_device_probe+0x54/0xa3
[    0.303850]  [<ffffffff8144367d>] driver_probe_device+0x99/0x1c8
[    0.303916]  [<ffffffff81443841>] __driver_attach+0x5d/0x80
[    0.303980]  [<ffffffff814437e4>] ? __device_attach+0x38/0x38
[    0.305851]  [<ffffffff81441d7c>] bus_for_each_dev+0x7b/0x85
[    0.305914]  [<ffffffff81443240>] driver_attach+0x19/0x1b
[    0.305975]  [<ffffffff81442f28>] bus_add_driver+0x109/0x1d3
[    0.306038]  [<ffffffff81443c20>] driver_register+0x8a/0xc7
[    0.306141]  [<ffffffff8131bf4b>] __pci_register_driver+0x5c/0x60
[    0.306219]  [<ffffffff81d052cc>] ? ftrace_define_fields_drm_vblank_event_delivered+0x9f/0x9f
[    0.306329]  [<ffffffff813a5fa3>] drm_pci_init+0x4d/0xcd
[    0.306428]  [<ffffffff81d052cc>] ? ftrace_define_fields_drm_vblank_event_delivered+0x9f/0x9f
[    0.306505]  [<ffffffff81d05356>] i915_init+0x8a/0x92
[    0.306566]  [<ffffffff81d052cc>] ? ftrace_define_fields_drm_vblank_event_delivered+0x9f/0x9f
[    0.306650]  [<ffffffff8100030d>] do_one_initcall+0xe9/0x172
[    0.306746]  [<ffffffff81ccefb1>] kernel_init_freeable+0x117/0x19f
[    0.306827]  [<ffffffff81cce7c0>] ? initcall_blacklist+0xa3/0xa3
[    0.306926]  [<ffffffff8178e559>] ? rest_init+0xb6/0xb6
[    0.306993]  [<ffffffff8178e562>] kernel_init+0x9/0xd0
[    0.307085]  [<ffffffff8179ceec>] ret_from_fork+0x7c/0xb0
[    0.307176]  [<ffffffff8178e559>] ? rest_init+0xb6/0xb6
...
[    0.646722] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc5+ #26
[    0.646723] Hardware name: LENOVO 4286CTO/4286CTO, BIOS 8DET42WW (1.12 ) 04/01/2011
[    0.646726]  ffff8800b5aff000 ffff880139563e78 ffffffff817945db 0000000000000013
[    0.646728]  ffffffff81d00a08 ffff880139563e88 ffffffff81d00a1f ffff880139563ef8
[    0.646730]  ffffffff8100030d 0000000000000000 ffffffff81b7f6b8 0000019d00070007
[    0.646730] Call Trace:
[    0.646733]  [<ffffffff817945db>] dump_stack+0x45/0x57
[    0.646736]  [<ffffffff81d00a08>] ? fb_console_init+0x116/0x116
[    0.646738]  [<ffffffff81d00a1f>] fb_logo_late_init+0x17/0x22
[    0.646741]  [<ffffffff8100030d>] do_one_initcall+0xe9/0x172
[    0.646744]  [<ffffffff81ccefb1>] kernel_init_freeable+0x117/0x19f
[    0.646745]  [<ffffffff81cce7c0>] ? initcall_blacklist+0xa3/0xa3
[    0.646747]  [<ffffffff8178e559>] ? rest_init+0xb6/0xb6
[    0.646749]  [<ffffffff8178e562>] kernel_init+0x9/0xd0
[    0.646752]  [<ffffffff8179ceec>] ret_from_fork+0x7c/0xb0
[    0.646754]  [<ffffffff8178e559>] ? rest_init+0xb6/0xb6
...
[    1.162602] CPU: 1 PID: 6 Comm: kworker/u16:0 Not tainted 3.19.0-rc5+ #26
[    1.162605] Hardware name: LENOVO 4286CTO/4286CTO, BIOS 8DET42WW (1.12 ) 04/01/2011
[    1.162627]  0000000000000018 ffff8801395a3918 ffffffff817945db ffffffff81caeee0
[    1.162634]  0000000000000018 ffff8801395a3938 ffffffff8178f5f7 0000000000000000
[    1.162641]  ffff8800b5ff0c00 ffff8801395a3968 ffffffff813321fb 0000000000000320
[    1.162643] Call Trace:
[    1.162653]  [<ffffffff817945db>] dump_stack+0x45/0x57
[    1.162661]  [<ffffffff8178f5f7>] fb_find_logo+0xd/0x43
[    1.162669]  [<ffffffff813321fb>] fb_prepare_logo+0x87/0x12d
[    1.162675]  [<ffffffff81328fed>] fbcon_prepare_logo+0x7f/0x2e8
[    1.162680]  [<ffffffff8132b3b9>] fbcon_init+0x3d9/0x447
[    1.162688]  [<ffffffff813817de>] visual_init+0xb7/0x10d
[    1.162695]  [<ffffffff81383000>] do_bind_con_driver+0x1ab/0x2cd
[    1.162702]  [<ffffffff813835e1>] do_take_over_console+0x132/0x162
[    1.162707]  [<ffffffff813292ac>] do_fbcon_takeover+0x56/0x9a
[    1.162712]  [<ffffffff8132cb6a>] fbcon_event_notify+0x31c/0x644
[    1.162718]  [<ffffffff810bcf4f>] notifier_call_chain+0x39/0x5c
[    1.162723]  [<ffffffff810bd208>] __blocking_notifier_call_chain+0x47/0x60
[    1.162729]  [<ffffffff810bd230>] blocking_notifier_call_chain+0xf/0x11
[    1.162735]  [<ffffffff81331cc3>] fb_notifier_call_chain+0x16/0x18
[    1.162741]  [<ffffffff81333b50>] register_framebuffer+0x261/0x299
[    1.162750]  [<ffffffff8139bcfb>] drm_fb_helper_initial_config+0x26e/0x328
[    1.162757]  [<ffffffff8141da68>] intel_fbdev_initial_config+0x16/0x18
[    1.162762]  [<ffffffff810be0a0>] async_run_entry_fn+0x33/0xca
[    1.162770]  [<ffffffff810b800b>] process_one_work+0x223/0x3f9
[    1.162775]  [<ffffffff810b7f8f>] ? process_one_work+0x1a7/0x3f9
[    1.162781]  [<ffffffff810b8905>] worker_thread+0x260/0x354
[    1.162788]  [<ffffffff810b86a5>] ? cancel_delayed_work_sync+0x10/0x10
[    1.162794]  [<ffffffff810bc333>] kthread+0xe8/0xf0
[    1.162802]  [<ffffffff810bc24b>] ? kthread_create_on_node+0x1b1/0x1b1
[    1.162810]  [<ffffffff8179ceec>] ret_from_fork+0x7c/0xb0
[    1.162817]  [<ffffffff810bc24b>] ? kthread_create_on_node+0x1b1/0x1b1
...

> However, it does seem that the fix seems to cause logos to disappear for
> many people. I'd be interesting to know how many of those cases were
> working by luck, either by
> 
> 1) an async work being ran fast enough, before the logos had been freed
> 2) the use of logos happening after the logos had been freed, but if no
> one had trashed the logo memory yet, it still works

In my case, it looks like #1 is the case: free_initmem() was called
~6.96s into boot on the run matching the traces above, so the initdata
looks safe barring async trickery.

> I don't care so much about the logo myself but people do seem to like
> it, so perhaps we need to change the code as Thierry suggested:
> allocating memory for the logos and keeping them in memory until someone
> uses them the first time, and then free the memory.

I'm not too concerned about the pixels myself, but I was concerned
that the initialization was happening slow enough to be caught by this
(perhaps this might have been a warning sign if setting up the console
was depending unsafely on other initdata?). It is also rather obvious,
so it's something a novice like myself can easily bisect and report.

-- 
S. Gilles

^ permalink raw reply

* Re: [PATCH v2] fbdev: ssd1307fb: return proper error code if write command fails
From: Tomi Valkeinen @ 2015-01-30  7:40 UTC (permalink / raw)
  To: Lad, Prabhakar, LFBDEV, Jean-Christophe Plagniol-Villard,
	Maxime Ripard
  Cc: LKML
In-Reply-To: <1421348737-4606-1-git-send-email-prabhakar.csengg@gmail.com>

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

On 15/01/15 21:05, Lad, Prabhakar wrote:
> From: Prabhakar Lad <prabhakar.csengg@gmail.com>
> 
> this patch fixes ssd1307fb_ssd1306_init() function to return
> proper error codes in case of failures.
> 
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> ---
>  Changes for v2:
>  a: Added new line as per Maxime's suggestion.
> 
>  drivers/video/fbdev/ssd1307fb.c | 67 ++++++++++++++++++++++++++++++++---------
>  1 file changed, 53 insertions(+), 14 deletions(-)
> 

Thanks, queued for 3.20.

 Tomi



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

^ permalink raw reply

* Re: [PATCH] fb: via: turn gpiolib and i2c selects into dependencies
From: Tomi Valkeinen @ 2015-01-30  7:46 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <3127720.VGEDzudcPs@wuerfel>

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

On 28/01/15 22:12, Arnd Bergmann wrote:
> Device driver should not directly select subsystems. In this case
> we get build warnings like
> 
> warning: (ARCH_REQUIRE_GPIOLIB && PINCTRL_AT91 && PINCTRL_NOMADIK && MFD_TC6393XB && FB_VIA) selects GPIOLIB which has unmet direct dependencies (ARCH_WANT_OPTIONAL_GPIOLIB || ARCH_REQUIRE_GPIOLIB)
> 
> which we can avoid using the normal 'depends on' statement.
> 
> Also, this patch makes it possible for DRM drivers to have a dependency
> on GPIOLIB without getting circular Kconfig dependencies.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index f2c3fb7d0399..b3dd417b4719 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -1530,13 +1530,11 @@ config FB_SIS_315
>  
>  config FB_VIA
>         tristate "VIA UniChrome (Pro) and Chrome9 display support"
> -       depends on FB && PCI && X86
> +       depends on FB && PCI && X86 && GPIOLIB && I2C
>         select FB_CFB_FILLRECT
>         select FB_CFB_COPYAREA
>         select FB_CFB_IMAGEBLIT
>         select I2C_ALGOBIT
> -       select I2C
> -       select GPIOLIB
>         help
>  	  This is the frame buffer device driver for Graphics chips of VIA
>  	  UniChrome (Pro) Family (CLE266,PM800/CN400,P4M800CE/P4M800Pro/
> 

Thanks, queued for 3.20.

 Tomi



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

^ permalink raw reply

* Re: [PATCH] video/mmpfb: allow modular build
From: Tomi Valkeinen @ 2015-01-30  7:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8150312.I9dm5QVFgr@wuerfel>

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

On 28/01/15 22:13, Arnd Bergmann wrote:
> The frame buffer core can be a module, which means any fb drivers
> should be able to build as modules too. This turns mmpfb into
> a tristate option to allow that and fix a possible randconfig
> build error.
> 
> drivers/built-in.o: In function `modes_setup':
> :(.text+0x11b34): undefined reference to `fb_videomode_to_modelist'
> :(.text+0x11b5c): undefined reference to `fb_videomode_to_var'
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/drivers/video/fbdev/mmp/Makefile b/drivers/video/fbdev/mmp/Makefile
> index a014cb358bf8..924dd0930cc7 100644
> --- a/drivers/video/fbdev/mmp/Makefile
> +++ b/drivers/video/fbdev/mmp/Makefile
> @@ -1 +1,3 @@
> -obj-y += core.o hw/ panel/ fb/
> +obj-$(CONFIG_MMP_DISP) += mmp_disp.o hw/ panel/ fb/
> +
> +mmp_disp-y		+= core.o
> diff --git a/drivers/video/fbdev/mmp/fb/Kconfig b/drivers/video/fbdev/mmp/fb/Kconfig
> index 9b0141f105f5..985e1a7cd254 100644
> --- a/drivers/video/fbdev/mmp/fb/Kconfig
> +++ b/drivers/video/fbdev/mmp/fb/Kconfig
> @@ -1,7 +1,7 @@
>  if MMP_DISP
>  
>  config MMP_FB
> -	bool "fb driver for Marvell MMP Display Subsystem"
> +	tristate "fb driver for Marvell MMP Display Subsystem"
>  	depends on FB
>  	select FB_CFB_FILLRECT
>  	select FB_CFB_COPYAREA
> 

Thanks, queued for 3.20.

 Tomi



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

^ permalink raw reply

* Re: [PATCH v2] video: fbdev: fix sys_copyarea
From: Tomi Valkeinen @ 2015-01-30  7:54 UTC (permalink / raw)
  To: Mans Rullgard, Jean-Christophe Plagniol-Villard, linux-fbdev,
	linux-kernel
In-Reply-To: <1421889589-16362-1-git-send-email-mans@mansr.com>

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

On 22/01/15 03:19, Mans Rullgard wrote:
> The sys_copyarea() function performs the same operation as
> cfb_copyarea() but using normal memory access instead of I/O
> accessors.  Since the introduction of sys_copyarea(), there
> have been two fixes to cfb_copyarea():
> 
> - 00a9d699 ("framebuffer: fix cfb_copyarea")
> - 5b789da8 ("framebuffer: fix screen corruption when copying")
> 
> This patch incorporates the fixes into sys_copyarea() as well.
> 
> Signed-off-by: Mans Rullgard <mans@mansr.com>
> ---
> Changed in v2:
>  - Fixed wrong first/last calculation in bitcpy_rev()
> ---
>  drivers/video/fbdev/core/syscopyarea.c | 137 ++++++++++++++++-----------------
>  1 file changed, 65 insertions(+), 72 deletions(-)

Thanks, queued for 3.20.

This makes me wonder, though, if the sys and cfb versions could be
somehow combined...

 Tomi



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

^ 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