* [RFC PATCH 3/5] gpu: ipu-v3: Add ipu_idmac_get_current_buffer function
From: Philipp Zabel @ 2013-12-20 17:52 UTC (permalink / raw)
To: linux-fbdev
This function returns the currently active buffer (0 or 1)
of a double buffered IDMAC channel. It is to be used by the
CSI driver.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/gpu/ipu-v3/ipu-common.c | 9 +++++++++
include/video/imx-ipu-v3.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
index 8f98719..29c89f2 100644
--- a/drivers/gpu/ipu-v3/ipu-common.c
+++ b/drivers/gpu/ipu-v3/ipu-common.c
@@ -663,6 +663,15 @@ int ipu_module_disable(struct ipu_soc *ipu, u32 mask)
}
EXPORT_SYMBOL_GPL(ipu_module_disable);
+int ipu_idmac_get_current_buffer(struct ipuv3_channel *channel)
+{
+ struct ipu_soc *ipu = channel->ipu;
+ unsigned int chno = channel->num;
+
+ return (ipu_cm_read(ipu, IPU_CHA_CUR_BUF(chno)) & idma_mask(chno)) ? 1 : 0;
+}
+EXPORT_SYMBOL_GPL(ipu_idmac_get_current_buffer);
+
void ipu_idmac_select_buffer(struct ipuv3_channel *channel, u32 buf_num)
{
struct ipu_soc *ipu = channel->ipu;
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index 274da71..78ea31b 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -103,6 +103,7 @@ int ipu_idmac_wait_busy(struct ipuv3_channel *channel, int ms);
void ipu_idmac_set_double_buffer(struct ipuv3_channel *channel,
bool doublebuffer);
+int ipu_idmac_get_current_buffer(struct ipuv3_channel *channel);
void ipu_idmac_select_buffer(struct ipuv3_channel *channel, u32 buf_num);
/*
--
1.8.5.1
^ permalink raw reply related
* [RFC PATCH 2/5] gpu: ipu-v3: Add SMFC code
From: Philipp Zabel @ 2013-12-20 17:52 UTC (permalink / raw)
To: linux-fbdev
The Sensor Multi Fifo Controller (SMFC) is used as a buffer between
the two CSIs (writing simultaneously) and up to four IDMAC channels.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/gpu/ipu-v3/Makefile | 2 +-
drivers/gpu/ipu-v3/ipu-common.c | 10 +++++
drivers/gpu/ipu-v3/ipu-prv.h | 6 +++
drivers/gpu/ipu-v3/ipu-smfc.c | 97 +++++++++++++++++++++++++++++++++++++++++
include/video/imx-ipu-v3.h | 6 +++
5 files changed, 120 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/ipu-v3/ipu-smfc.c
diff --git a/drivers/gpu/ipu-v3/Makefile b/drivers/gpu/ipu-v3/Makefile
index d21cc37..1887972b 100644
--- a/drivers/gpu/ipu-v3/Makefile
+++ b/drivers/gpu/ipu-v3/Makefile
@@ -1,3 +1,3 @@
obj-$(CONFIG_IMX_IPUV3_CORE) += imx-ipu-v3.o
-imx-ipu-v3-objs := ipu-common.o ipu-dc.o ipu-di.o ipu-dp.o ipu-dmfc.o
+imx-ipu-v3-objs := ipu-common.o ipu-dc.o ipu-di.o ipu-dp.o ipu-dmfc.o ipu-smfc.o
diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
index a59cc6c..8f98719 100644
--- a/drivers/gpu/ipu-v3/ipu-common.c
+++ b/drivers/gpu/ipu-v3/ipu-common.c
@@ -876,8 +876,17 @@ static int ipu_submodules_init(struct ipu_soc *ipu,
goto err_dp;
}
+ ret = ipu_smfc_init(ipu, dev, ipu_base +
+ devtype->cm_ofs + IPU_CM_SMFC_REG_OFS);
+ if (ret) {
+ unit = "smfc";
+ goto err_smfc;
+ }
+
return 0;
+err_smfc:
+ ipu_dp_exit(ipu);
err_dp:
ipu_dmfc_exit(ipu);
err_dmfc:
@@ -949,6 +958,7 @@ EXPORT_SYMBOL_GPL(ipu_idmac_channel_irq);
static void ipu_submodules_exit(struct ipu_soc *ipu)
{
+ ipu_smfc_exit(ipu);
ipu_dp_exit(ipu);
ipu_dmfc_exit(ipu);
ipu_dc_exit(ipu);
diff --git a/drivers/gpu/ipu-v3/ipu-prv.h b/drivers/gpu/ipu-v3/ipu-prv.h
index 5cb075f..acf1811 100644
--- a/drivers/gpu/ipu-v3/ipu-prv.h
+++ b/drivers/gpu/ipu-v3/ipu-prv.h
@@ -151,6 +151,8 @@ struct ipuv3_channel {
struct ipu_dc_priv;
struct ipu_dmfc_priv;
struct ipu_di;
+struct ipu_smfc_priv;
+
struct ipu_devtype;
struct ipu_soc {
@@ -178,6 +180,7 @@ struct ipu_soc {
struct ipu_dp_priv *dp_priv;
struct ipu_dmfc_priv *dmfc_priv;
struct ipu_di *di_priv[2];
+ struct ipu_smfc_priv *smfc_priv;
};
void ipu_srm_dp_sync_update(struct ipu_soc *ipu);
@@ -203,4 +206,7 @@ void ipu_dc_exit(struct ipu_soc *ipu);
int ipu_cpmem_init(struct ipu_soc *ipu, struct device *dev, unsigned long base);
void ipu_cpmem_exit(struct ipu_soc *ipu);
+int ipu_smfc_init(struct ipu_soc *ipu, struct device *dev, unsigned long base);
+void ipu_smfc_exit(struct ipu_soc *ipu);
+
#endif /* __IPU_PRV_H__ */
diff --git a/drivers/gpu/ipu-v3/ipu-smfc.c b/drivers/gpu/ipu-v3/ipu-smfc.c
new file mode 100644
index 0000000..e4f85ad
--- /dev/null
+++ b/drivers/gpu/ipu-v3/ipu-smfc.c
@@ -0,0 +1,97 @@
+/*
+ * Copyright 2008-2010 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+#define DEBUG
+#include <linux/export.h>
+#include <linux/types.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/errno.h>
+#include <linux/spinlock.h>
+#include <linux/delay.h>
+#include <linux/clk.h>
+#include <video/imx-ipu-v3.h>
+
+#include "ipu-prv.h"
+
+struct ipu_smfc_priv {
+ void __iomem *base;
+ spinlock_t lock;
+};
+
+/*SMFC Registers */
+#define SMFC_MAP 0x0000
+#define SMFC_WMC 0x0004
+#define SMFC_BS 0x0008
+
+int ipu_smfc_set_burstsize(struct ipu_soc *ipu, int channel, int burstsize)
+{
+ struct ipu_smfc_priv *smfc = ipu->smfc_priv;
+ unsigned long flags;
+ u32 val, shift;
+
+ spin_lock_irqsave(&smfc->lock, flags);
+
+ shift = channel * 4;
+ val = readl(smfc->base + SMFC_BS);
+ val &= ~(0xf << shift);
+ val |= burstsize << shift;
+ writel(val, smfc->base + SMFC_BS);
+
+ spin_unlock_irqrestore(&smfc->lock, flags);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ipu_smfc_set_burstsize);
+
+int ipu_smfc_map_channel(struct ipu_soc *ipu, int channel, int csi_id, int mipi_id)
+{
+ struct ipu_smfc_priv *smfc = ipu->smfc_priv;
+ unsigned long flags;
+ u32 val, shift;
+
+ spin_lock_irqsave(&smfc->lock, flags);
+
+ shift = channel * 3;
+ val = readl(smfc->base + SMFC_MAP);
+ val &= ~(0x7 << shift);
+ val |= ((csi_id << 2) | mipi_id) << shift;
+ writel(val, smfc->base + SMFC_MAP);
+
+ spin_unlock_irqrestore(&smfc->lock, flags);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ipu_smfc_map_channel);
+
+int ipu_smfc_init(struct ipu_soc *ipu, struct device *dev,
+ unsigned long base)
+{
+ struct ipu_smfc_priv *smfc;
+
+ smfc = devm_kzalloc(dev, sizeof(*smfc), GFP_KERNEL);
+ if (!smfc)
+ return -ENOMEM;
+
+ ipu->smfc_priv = smfc;
+ spin_lock_init(&smfc->lock);
+
+ smfc->base = devm_ioremap(dev, base, PAGE_SIZE);
+ if (!smfc->base)
+ return -ENOMEM;
+
+ pr_debug("%s: ioremap 0x%08lx -> %p\n", __func__, base, smfc->base);
+
+ return 0;
+}
+
+void ipu_smfc_exit(struct ipu_soc *ipu)
+{
+}
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index fb13823..274da71 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -160,6 +160,12 @@ int ipu_dp_set_window_pos(struct ipu_dp *, u16 x_pos, u16 y_pos);
int ipu_dp_set_global_alpha(struct ipu_dp *dp, bool enable, u8 alpha,
bool bg_chan);
+/*
+ * IPU Sensor Multiple FIFO Controller (SMFC) functions
+ */
+int ipu_smfc_map_channel(struct ipu_soc *ipu, int channel, int csi_id, int mipi_id);
+int ipu_smfc_set_burstsize(struct ipu_soc *ipu, int channel, int burstsize);
+
#define IPU_CPMEM_WORD(word, ofs, size) ((((word) * 160 + (ofs)) << 8) | (size))
#define IPU_FIELD_UBO IPU_CPMEM_WORD(0, 46, 22)
--
1.8.5.1
^ permalink raw reply related
* [RFC PATCH 1/5] gpu: ipu-v3: Move i.MX IPUv3 core driver out of staging
From: Philipp Zabel @ 2013-12-20 17:52 UTC (permalink / raw)
To: linux-fbdev
The i.MX Image Processing Unit (IPU) contains a number of image processing
blocks that sit right in the middle between DRM and V4L2. Some of the modules,
such as Display Controller, Processor, and Interface (DC, DP, DI) or CMOS
Sensor Interface (CSI) and their FIFOs could be assigned to either framework,
but others, such as the dma controller (IDMAC) and image converter (IC) can
be used by both.
The IPUv3 core driver provides an internal API to access the modules, to be
used by both DRM and V4L2 IPUv3 drivers.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/gpu/Makefile | 1 +
drivers/gpu/ipu-v3/Kconfig | 7 +++++++
drivers/{staging/imx-drm => gpu}/ipu-v3/Makefile | 2 +-
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-common.c | 3 ++-
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dc.c | 3 +--
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-di.c | 2 +-
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dmfc.c | 2 +-
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dp.c | 2 +-
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-prv.h | 2 +-
drivers/staging/imx-drm/Kconfig | 11 +----------
drivers/staging/imx-drm/Makefile | 1 -
drivers/staging/imx-drm/imx-drm.h | 4 ----
drivers/staging/imx-drm/imx-tve.c | 1 +
drivers/staging/imx-drm/ipuv3-crtc.c | 2 +-
drivers/staging/imx-drm/ipuv3-plane.c | 2 +-
drivers/video/Kconfig | 1 +
.../staging/imx-drm/ipu-v3 => include/video}/imx-ipu-v3.h | 2 ++
17 files changed, 23 insertions(+), 25 deletions(-)
create mode 100644 drivers/gpu/ipu-v3/Kconfig
rename drivers/{staging/imx-drm => gpu}/ipu-v3/Makefile (59%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-common.c (99%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dc.c (99%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-di.c (99%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dmfc.c (99%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dp.c (99%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-prv.h (99%)
rename {drivers/staging/imx-drm/ipu-v3 => include/video}/imx-ipu-v3.h (99%)
diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
index d8a22c2..70da9eb 100644
--- a/drivers/gpu/Makefile
+++ b/drivers/gpu/Makefile
@@ -1,2 +1,3 @@
obj-y += drm/ vga/
obj-$(CONFIG_TEGRA_HOST1X) += host1x/
+obj-$(CONFIG_IMX_IPUV3_CORE) += ipu-v3/
diff --git a/drivers/gpu/ipu-v3/Kconfig b/drivers/gpu/ipu-v3/Kconfig
new file mode 100644
index 0000000..2f228a2
--- /dev/null
+++ b/drivers/gpu/ipu-v3/Kconfig
@@ -0,0 +1,7 @@
+config IMX_IPUV3_CORE
+ tristate "IPUv3 core support"
+ depends on SOC_IMX5 || SOC_IMX6Q || SOC_IMX6SL || ARCH_MULTIPLATFORM
+ depends on RESET_CONTROLLER
+ help
+ Choose this if you have a i.MX5/6 system and want to use the Image
+ Processing Unit. This option only enables IPU base support.
diff --git a/drivers/staging/imx-drm/ipu-v3/Makefile b/drivers/gpu/ipu-v3/Makefile
similarity index 59%
rename from drivers/staging/imx-drm/ipu-v3/Makefile
rename to drivers/gpu/ipu-v3/Makefile
index 28ed72e..d21cc37 100644
--- a/drivers/staging/imx-drm/ipu-v3/Makefile
+++ b/drivers/gpu/ipu-v3/Makefile
@@ -1,3 +1,3 @@
-obj-$(CONFIG_DRM_IMX_IPUV3_CORE) += imx-ipu-v3.o
+obj-$(CONFIG_IMX_IPUV3_CORE) += imx-ipu-v3.o
imx-ipu-v3-objs := ipu-common.o ipu-dc.o ipu-di.o ipu-dp.o ipu-dmfc.o
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
similarity index 99%
rename from drivers/staging/imx-drm/ipu-v3/ipu-common.c
rename to drivers/gpu/ipu-v3/ipu-common.c
index 96b1135..a59cc6c 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-common.c
+++ b/drivers/gpu/ipu-v3/ipu-common.c
@@ -32,7 +32,7 @@
#include <drm/drm_fourcc.h>
-#include "imx-ipu-v3.h"
+#include <video/imx-ipu-v3.h>
#include "ipu-prv.h"
static inline u32 ipu_cm_read(struct ipu_soc *ipu, unsigned offset)
@@ -539,6 +539,7 @@ enum ipu_color_space ipu_pixelformat_to_colorspace(u32 pixelformat)
case V4L2_PIX_FMT_RGB24:
case V4L2_PIX_FMT_BGR24:
case V4L2_PIX_FMT_RGB565:
+ case V4L2_PIX_FMT_BGR666:
return IPUV3_COLORSPACE_RGB;
default:
return IPUV3_COLORSPACE_UNKNOWN;
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c b/drivers/gpu/ipu-v3/ipu-dc.c
similarity index 99%
rename from drivers/staging/imx-drm/ipu-v3/ipu-dc.c
rename to drivers/gpu/ipu-v3/ipu-dc.c
index d0e3bc3..e4c0daf 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
+++ b/drivers/gpu/ipu-v3/ipu-dc.c
@@ -20,8 +20,7 @@
#include <linux/delay.h>
#include <linux/io.h>
-#include "../imx-drm.h"
-#include "imx-ipu-v3.h"
+#include <video/imx-ipu-v3.h>
#include "ipu-prv.h"
#define DC_MAP_CONF_PTR(n) (0x108 + ((n) & ~0x1) * 2)
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
similarity index 99%
rename from drivers/staging/imx-drm/ipu-v3/ipu-di.c
rename to drivers/gpu/ipu-v3/ipu-di.c
index 948a49b..3ab5204 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-di.c
+++ b/drivers/gpu/ipu-v3/ipu-di.c
@@ -23,7 +23,7 @@
#include <linux/clk-provider.h>
#include <linux/clkdev.h>
-#include "imx-ipu-v3.h"
+#include <video/imx-ipu-v3.h>
#include "ipu-prv.h"
struct ipu_di {
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c b/drivers/gpu/ipu-v3/ipu-dmfc.c
similarity index 99%
rename from drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c
rename to drivers/gpu/ipu-v3/ipu-dmfc.c
index 98070dd..d46de73 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c
+++ b/drivers/gpu/ipu-v3/ipu-dmfc.c
@@ -17,7 +17,7 @@
#include <linux/errno.h>
#include <linux/io.h>
-#include "imx-ipu-v3.h"
+#include <video/imx-ipu-v3.h>
#include "ipu-prv.h"
#define DMFC_RD_CHAN 0x0000
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c b/drivers/gpu/ipu-v3/ipu-dp.c
similarity index 99%
rename from drivers/staging/imx-drm/ipu-v3/ipu-dp.c
rename to drivers/gpu/ipu-v3/ipu-dp.c
index 58f87c8..e17fa3f 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
+++ b/drivers/gpu/ipu-v3/ipu-dp.c
@@ -19,7 +19,7 @@
#include <linux/io.h>
#include <linux/err.h>
-#include "imx-ipu-v3.h"
+#include <video/imx-ipu-v3.h>
#include "ipu-prv.h"
#define DP_SYNC 0
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-prv.h b/drivers/gpu/ipu-v3/ipu-prv.h
similarity index 99%
rename from drivers/staging/imx-drm/ipu-v3/ipu-prv.h
rename to drivers/gpu/ipu-v3/ipu-prv.h
index 4df0050..5cb075f 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-prv.h
+++ b/drivers/gpu/ipu-v3/ipu-prv.h
@@ -22,7 +22,7 @@ struct ipu_soc;
#include <linux/clk.h>
#include <linux/platform_device.h>
-#include "imx-ipu-v3.h"
+#include <video/imx-ipu-v3.h>
#define IPUV3_CHANNEL_CSI0 0
#define IPUV3_CHANNEL_CSI1 1
diff --git a/drivers/staging/imx-drm/Kconfig b/drivers/staging/imx-drm/Kconfig
index 5032ff7..dba241f 100644
--- a/drivers/staging/imx-drm/Kconfig
+++ b/drivers/staging/imx-drm/Kconfig
@@ -38,18 +38,9 @@ config DRM_IMX_LDB
Choose this to enable the internal LVDS Display Bridge (LDB)
found on i.MX53 and i.MX6 processors.
-config DRM_IMX_IPUV3_CORE
- tristate "IPUv3 core support"
- depends on DRM_IMX
- depends on RESET_CONTROLLER
- help
- Choose this if you have a i.MX5/6 system and want
- to use the IPU. This option only enables IPU base
- support.
-
config DRM_IMX_IPUV3
tristate "DRM Support for i.MX IPUv3"
depends on DRM_IMX
- depends on DRM_IMX_IPUV3_CORE
+ depends on IMX_IPUV3_CORE
help
Choose this if you have a i.MX5 or i.MX6 processor.
diff --git a/drivers/staging/imx-drm/Makefile b/drivers/staging/imx-drm/Makefile
index 8742432..4d4a506 100644
--- a/drivers/staging/imx-drm/Makefile
+++ b/drivers/staging/imx-drm/Makefile
@@ -7,7 +7,6 @@ obj-$(CONFIG_DRM_IMX_PARALLEL_DISPLAY) += parallel-display.o
obj-$(CONFIG_DRM_IMX_TVE) += imx-tve.o
obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
obj-$(CONFIG_DRM_IMX_FB_HELPER) += imx-fbdev.o
-obj-$(CONFIG_DRM_IMX_IPUV3_CORE) += ipu-v3/
imx-ipuv3-crtc-objs := ipuv3-crtc.o ipuv3-plane.o
obj-$(CONFIG_DRM_IMX_IPUV3) += imx-ipuv3-crtc.o
diff --git a/drivers/staging/imx-drm/imx-drm.h b/drivers/staging/imx-drm/imx-drm.h
index ae90c9c..6188948 100644
--- a/drivers/staging/imx-drm/imx-drm.h
+++ b/drivers/staging/imx-drm/imx-drm.h
@@ -1,10 +1,6 @@
#ifndef _IMX_DRM_H_
#define _IMX_DRM_H_
-#include <linux/videodev2.h>
-
-#define IPU_PIX_FMT_GBR24 v4l2_fourcc('G', 'B', 'R', '3')
-
struct drm_crtc;
struct drm_connector;
struct drm_device;
diff --git a/drivers/staging/imx-drm/imx-tve.c b/drivers/staging/imx-drm/imx-tve.c
index 2c44fef..7b668b0 100644
--- a/drivers/staging/imx-drm/imx-tve.c
+++ b/drivers/staging/imx-drm/imx-tve.c
@@ -29,6 +29,7 @@
#include <drm/drmP.h>
#include <drm/drm_fb_helper.h>
#include <drm/drm_crtc_helper.h>
+#include <video/imx-ipu-v3.h>
#include "imx-drm.h"
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c
index ce6ba98..e4d96f2 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -29,7 +29,7 @@
#include <drm/drm_gem_cma_helper.h>
#include <drm/drm_fb_cma_helper.h>
-#include "ipu-v3/imx-ipu-v3.h"
+#include <video/imx-ipu-v3.h>
#include "imx-drm.h"
#include "ipuv3-plane.h"
diff --git a/drivers/staging/imx-drm/ipuv3-plane.c b/drivers/staging/imx-drm/ipuv3-plane.c
index d97454a..2dde959 100644
--- a/drivers/staging/imx-drm/ipuv3-plane.c
+++ b/drivers/staging/imx-drm/ipuv3-plane.c
@@ -17,7 +17,7 @@
#include <drm/drm_fb_cma_helper.h>
#include <drm/drm_gem_cma_helper.h>
-#include "ipu-v3/imx-ipu-v3.h"
+#include "video/imx-ipu-v3.h"
#include "ipuv3-plane.h"
#define to_ipu_plane(x) container_of(x, struct ipu_plane, base)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 4f2e1b3..2195ff3 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -20,6 +20,7 @@ source "drivers/char/agp/Kconfig"
source "drivers/gpu/vga/Kconfig"
source "drivers/gpu/host1x/Kconfig"
+source "drivers/gpu/ipu-v3/Kconfig"
source "drivers/gpu/drm/Kconfig"
diff --git a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
similarity index 99%
rename from drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
rename to include/video/imx-ipu-v3.h
index 7b879ac..fb13823 100644
--- a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -25,6 +25,8 @@ enum ipuv3_type {
IPUV3H,
};
+#define IPU_PIX_FMT_GBR24 v4l2_fourcc('G', 'B', 'R', '3')
+
/*
* Bitfield of Display Interface signal polarities.
*/
--
1.8.5.1
^ permalink raw reply related
* [RFC PATCH 0/5] Move IPUv3 core out of staging, add CSI support
From: Philipp Zabel @ 2013-12-20 17:52 UTC (permalink / raw)
To: linux-fbdev
Hi,
this is mostly about the first patch, which moves the IPUv3 core code
(drivers/staging/imx-drm/ipu-v3) to drivers/gpu. host1x, which
serves a similar purpose, already sits there.
The other four patches add the necessary code for CSI and SMFC handling,
which is used by the V4L2 CSI capture driver.
Currently this is based on Russell's patch
[PATCH 62/64] imx-drm: pass an IPU ID to crtc and core
I am aware that there are now quite a few other patches in the pipeline
that touch drivers/staging/imx-drm/ipu-v3/*, so I am happy to rebase this
(or them) as needed. I'd like to move the core code out of staging so that
we can start submitting V4L2 code for video capture and scaling / colorspace
conversion in parallel.
Philipp Zabel (5):
gpu: ipu-v3: Move i.MX IPUv3 core driver out of staging
gpu: ipu-v3: Add SMFC code
gpu: ipu-v3: Add ipu_idmac_get_current_buffer function
gpu: ipu-v3: Add CSI and SMFC module enable wrappers
gpu: ipu-v3: Register the CSI modules
drivers/gpu/Makefile | 1 +
drivers/gpu/ipu-v3/Kconfig | 7 ++
drivers/{staging/imx-drm => gpu}/ipu-v3/Makefile | 4 +-
.../{staging/imx-drm => gpu}/ipu-v3/ipu-common.c | 81 ++++++++++++++++--
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dc.c | 3 +-
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-di.c | 2 +-
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dmfc.c | 2 +-
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dp.c | 2 +-
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-prv.h | 8 +-
drivers/gpu/ipu-v3/ipu-smfc.c | 97 ++++++++++++++++++++++
drivers/staging/imx-drm/Kconfig | 11 +--
drivers/staging/imx-drm/Makefile | 1 -
drivers/staging/imx-drm/imx-drm.h | 4 -
drivers/staging/imx-drm/imx-tve.c | 1 +
drivers/staging/imx-drm/ipuv3-crtc.c | 2 +-
drivers/staging/imx-drm/ipuv3-plane.c | 2 +-
drivers/video/Kconfig | 1 +
.../imx-drm/ipu-v3 => include/video}/imx-ipu-v3.h | 18 ++++
18 files changed, 217 insertions(+), 30 deletions(-)
create mode 100644 drivers/gpu/ipu-v3/Kconfig
rename drivers/{staging/imx-drm => gpu}/ipu-v3/Makefile (51%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-common.c (94%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dc.c (99%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-di.c (99%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dmfc.c (99%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dp.c (99%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-prv.h (96%)
create mode 100644 drivers/gpu/ipu-v3/ipu-smfc.c
rename {drivers/staging/imx-drm/ipu-v3 => include/video}/imx-ipu-v3.h (94%)
regards
Philipp
^ permalink raw reply
* Re: [PATCH] video/logo: don't look for the logo after system boot
From: Geert Uytterhoeven @ 2013-12-20 16:20 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1387549460-18501-1-git-send-email-bigeasy@linutronix.de>
On Fri, Dec 20, 2013 at 4:49 PM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> On 12/20/2013 04:31 PM, Sebastian Andrzej Siewior wrote:
>> On 12/20/2013 04:27 PM, Geert Uytterhoeven wrote:
>>> On Fri, Dec 20, 2013 at 3:24 PM, Sebastian Andrzej Siewior
>>> <bigeasy@linutronix.de> wrote:
>>>> If the primary GPU driver has been loaded _after_ system as a module
>>>> then this logo memory is no longer valid.
>>>> Managed to crash the system by booting a box without a GPU and then
>>>> hotpluggin => BOOM.
>>>
>>> drivers/video/fbmem.c:fb_prepare_logo() has protection against this:
>>>
>>> if (info->flags & FBINFO_MISC_TILEBLITTING ||
>>> info->flags & FBINFO_MODULE)
>>> return 0;
>>
>> but gpu driver is built-in. I just add PCI device at run-time. The same
>> thing should happen if you go to sysfs and remove the PCI device and
>> then do
>
> No I see where you are going with this. My description of the problem
> is wrong. The problem is nothing to do with the driver being a module.
Yes, I got confused by the "as a module".
> If nobody objects this or suggests a different solution then I'm going
> to provide a patch with a proper description.
Is "system_state != SYSTEM_BOOTING" the right check?
How long is is in the SYSTEM_BOOTING state?
Ah, kernel_init() does:
free_initmem();
mark_rodata_ro();
system_state = SYSTEM_RUNNING;
So this looks OK to me.
We used to have an initmem_freed flag, but it was removed 15 years
ago. 11 years ago, we got system_state, for the same purpose ;-)
Looking at commit 70802c60379fb843c485dfd4cab9e8f527d8fe81
Author: Antonino A. Daplas <adaplas@gmail.com>
Date: Tue May 8 00:38:14 2007 -0700
fbdev: don't show logo if driver or fbcon are modular
it seems we were not aware of the existence of system_state....
I guess you can remove FBINFO_MODULE after your fix?
BTW, I'm wondering if modprobing newport_con.ko crashes, too, as
it seems to call fb_find_logo() without any protection.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] video/logo: don't look for the logo after system boot
From: Sebastian Andrzej Siewior @ 2013-12-20 15:49 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1387549460-18501-1-git-send-email-bigeasy@linutronix.de>
On 12/20/2013 04:31 PM, Sebastian Andrzej Siewior wrote:
> On 12/20/2013 04:27 PM, Geert Uytterhoeven wrote:
>> On Fri, Dec 20, 2013 at 3:24 PM, Sebastian Andrzej Siewior
>> <bigeasy@linutronix.de> wrote:
>>> If the primary GPU driver has been loaded _after_ system as a module
>>> then this logo memory is no longer valid.
>>> Managed to crash the system by booting a box without a GPU and then
>>> hotpluggin => BOOM.
>>
>> drivers/video/fbmem.c:fb_prepare_logo() has protection against this:
>>
>> if (info->flags & FBINFO_MISC_TILEBLITTING ||
>> info->flags & FBINFO_MODULE)
>> return 0;
>
> but gpu driver is built-in. I just add PCI device at run-time. The same
> thing should happen if you go to sysfs and remove the PCI device and
> then do
No I see where you are going with this. My description of the problem
is wrong. The problem is nothing to do with the driver being a module.
If nobody objects this or suggests a different solution then I'm going
to provide a patch with a proper description.
Sebastian
^ permalink raw reply
* Re: [PATCH] video/logo: don't look for the logo after system boot
From: Sebastian Andrzej Siewior @ 2013-12-20 15:31 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1387549460-18501-1-git-send-email-bigeasy@linutronix.de>
On 12/20/2013 04:27 PM, Geert Uytterhoeven wrote:
> On Fri, Dec 20, 2013 at 3:24 PM, Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
>> If the primary GPU driver has been loaded _after_ system as a module
>> then this logo memory is no longer valid.
>> Managed to crash the system by booting a box without a GPU and then
>> hotpluggin => BOOM.
>
> Which GPU driver is this?
i915.
>
> drivers/video/fbmem.c:fb_prepare_logo() has protection against this:
>
> if (info->flags & FBINFO_MISC_TILEBLITTING ||
> info->flags & FBINFO_MODULE)
> return 0;
but gpu driver is built-in. I just add PCI device at run-time. The same
thing should happen if you go to sysfs and remove the PCI device and
then do
echo 1 > /sys/bus/pci/rescan
Sebastian
^ permalink raw reply
* Re: [PATCH] video/logo: don't look for the logo after system boot
From: Geert Uytterhoeven @ 2013-12-20 15:27 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1387549460-18501-1-git-send-email-bigeasy@linutronix.de>
On Fri, Dec 20, 2013 at 3:24 PM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> If the primary GPU driver has been loaded _after_ system as a module
> then this logo memory is no longer valid.
> Managed to crash the system by booting a box without a GPU and then
> hotpluggin => BOOM.
Which GPU driver is this?
drivers/video/fbmem.c:fb_prepare_logo() has protection against this:
if (info->flags & FBINFO_MISC_TILEBLITTING ||
info->flags & FBINFO_MODULE)
return 0;
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> drivers/video/logo/logo.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/video/logo/logo.c b/drivers/video/logo/logo.c
> index 080c35b..a26bb16 100644
> --- a/drivers/video/logo/logo.c
> +++ b/drivers/video/logo/logo.c
> @@ -36,6 +36,9 @@ const struct linux_logo * __init_refok fb_find_logo(int depth)
> if (nologo)
> return NULL;
>
> + if (system_state != SYSTEM_BOOTING)
> + return NULL;
> +
> if (depth >= 1) {
> #ifdef CONFIG_LOGO_LINUX_MONO
> /* Generic Linux logo */
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v4] x86: sysfb: remove sysfb when probing real hw
From: David Herrmann @ 2013-12-20 14:46 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, H. Peter Anvin, Thomas Gleixner,
the arch/x86 maintainers, linux-fbdev@vger.kernel.org, stable
In-Reply-To: <20131219183310.GF32508@gmail.com>
Hi
On Thu, Dec 19, 2013 at 7:33 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * David Herrmann <dh.herrmann@gmail.com> wrote:
>
>> +/*
>> + * Unregister the sysfb and prevent new sysfbs from getting registered. Can be
>> + * called from any context except recursively or from sysfb_register().
>> + * Used by remove_conflicting_framebuffers() and friends.
>> + */
>> +void sysfb_unregister(const struct apertures_struct *apert, bool primary)
>> +{
>> + mutex_lock(&sysfb_lock);
>> + if (!IS_ERR(sysfb_dev) && sysfb_dev) {
>> + if (sysfb_match(apert, primary)) {
>> + platform_device_unregister(sysfb_dev);
>> + sysfb_dev = ERR_PTR(-EALREADY);
>> + }
>> + } else {
>> + /* if !sysfb_dev, set err so no new sysfb is probed later */
>> + sysfb_dev = ERR_PTR(-EALREADY);
>
> Just a small detail: we can get into this 'else' branch not just with
> NULL, but also with IS_ERR(sysfb_dev). In that case we override
> whatever error code is contained in sysfb_dev and overwrite it with
> ERR_PTR(-EALREADY).
>
> (Probably not a big deal, because we don't actually ever seem to
> extract the error code from the pointer, but wanted to mention it.)
Yepp, I know, but that's just fine.
>> +#ifdef CONFIG_X86_SYSFB
>> +#include <asm/sysfb.h>
>> +#endif
>
> Pet peeve, this looks sexier:
>
> #ifdef CONFIG_X86_SYSFB
> # include <asm/sysfb.h>
> #endif
>
>> @@ -1604,6 +1607,17 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a,
>> }
>> }
>>
>> +static void remove_conflicting_sysfb(const struct apertures_struct *apert,
>> + bool primary)
>> +{
>> + if (!apert)
>> + return;
>> +
>> +#ifdef CONFIG_X86_SYSFB
>> + sysfb_unregister(apert, primary);
>> +#endif
>> +}
>
> So why not make sysfb_unregister() accept a !apert parameter (it would
> simply return), at which point remove_conflicting_sysfb() could be
> eliminated and just be replaced with a direct sysfb_unregister() call
> - with no #ifdefs.
>
> We only need #ifdefs for the sysfb_unregister() declaration in the .h
> file.
I can do that but we still need the #ifdef. sysfb is an x86-asm header
so it's not available on other archs. Even with #ifdef in the header I
still need it around the actual function call.
Thanks
David
> At first sight this looks simpler and cleaner for the fix itself - no
> need for extra cleanups for this detail.
>
> Thanks,
>
> Ingo
^ permalink raw reply
* [PATCH] video/logo: don't look for the logo after system boot
From: Sebastian Andrzej Siewior @ 2013-12-20 14:24 UTC (permalink / raw)
To: linux-fbdev
If the primary GPU driver has been loaded _after_ system as a module
then this logo memory is no longer valid.
Managed to crash the system by booting a box without a GPU and then
hotpluggin => BOOM.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/video/logo/logo.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/video/logo/logo.c b/drivers/video/logo/logo.c
index 080c35b..a26bb16 100644
--- a/drivers/video/logo/logo.c
+++ b/drivers/video/logo/logo.c
@@ -36,6 +36,9 @@ const struct linux_logo * __init_refok fb_find_logo(int depth)
if (nologo)
return NULL;
+ if (system_state != SYSTEM_BOOTING)
+ return NULL;
+
if (depth >= 1) {
#ifdef CONFIG_LOGO_LINUX_MONO
/* Generic Linux logo */
--
1.8.4.3
^ permalink raw reply related
* Re: [PATCH] fbcon: Clean up fbcon data in fb_info on FB_EVENT_FB_UNBIND with 0 fbs
From: Keith Packard @ 2013-12-20 7:10 UTC (permalink / raw)
To: linux-kernel, linux-fbdev
In-Reply-To: <1387498677-25653-1-git-send-email-keithp@keithp.com>
[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]
Keith Packard <keithp@keithp.com> writes:
> When FB_EVENT_FB_UNBIND is sent, fbcon has two paths, one path taken
> when there is another frame buffer to switch any affected vcs to and
> another path when there isn't.
What I meant to attach to this bug report but clearly failed was that I think
this case essentially *always* occurs if you have a generic frame buffer
driver loaded before a 'real' driver is loaded. In my case, this is
efifb. So, we have a reference to freed memory hanging out on a timer
list.
I'm really unsure why this hasn't caused more grief, but I can say that
it was greatly aggravated by adding 'fbcon=vc:2-6' to the kernel command
line, which limits fbcon to supporting consoles only on vt 2-6.
Definitely one of those 'how could this ever have worked, and why hasn't
my machine been crashing every day' moments when I read through the
code.
As the patch indicates, I'm not sure this patch is actually what we
want, but I'd love to know if I've isolated the root cause correctly and
then figure out what patch we actually want.
For my own part, I've got a happy customer with the patch, which is
definitely a nice way to end the day.
--
keith.packard@intel.com
[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]
^ permalink raw reply
* [PATCH] fbcon: Clean up fbcon data in fb_info on FB_EVENT_FB_UNBIND with 0 fbs
From: Keith Packard @ 2013-12-20 0:17 UTC (permalink / raw)
To: linux-kernel, linux-fbdev; +Cc: Keith Packard
When FB_EVENT_FB_UNBIND is sent, fbcon has two paths, one path taken
when there is another frame buffer to switch any affected vcs to and
another path when there isn't.
In the case where there is another frame buffer to use,
fbcon_fb_unbind calls set_con2fb_map to remap all of the affected vcs
to the replacement frame buffer. set_con2fb_map will eventually call
con2fb_release_oldinfo when the last vcs gets unmapped from the old
frame buffer.
con2fb_release_oldinfo frees the fbcon data that is hooked off of the
fb_info structure, including the cursor timer.
In the case where there isn't another frame buffer to use,
fbcon_fb_unbind simply calls fbcon_unbind, which doesn't clear the
con2fb_map or free the fbcon data hooked from the fb_info
structure. In particular, it doesn't stop the cursor blink timer. When
the fb_info structure is then freed, we end up with a timer queue
pointing into freed memory and "bad things" start happening.
This patch first changes con2fb_release_oldinfo so that it can take a
NULL pointer for the new frame buffer, but still does all of the
deallocation and cursor timer cleanup.
Finally, the patch tries to replicate some of what set_con2fb_map does
by clearing the con2fb_map for the affected vcs and calling the
modified con2fb_release_info function to clean up the fb_info structure.
Signed-off-by: Keith Packard <keithp@keithp.com>
---
drivers/video/console/fbcon.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index a92783e..ba9e149 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -787,7 +787,7 @@ static int con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo,
newinfo in an undefined state. Thus, a call to
fb_set_par() may be needed for the newinfo.
*/
- if (newinfo->fbops->fb_set_par) {
+ if (newinfo && newinfo->fbops->fb_set_par) {
ret = newinfo->fbops->fb_set_par(newinfo);
if (ret)
@@ -3056,8 +3056,31 @@ static int fbcon_fb_unbind(int idx)
if (con2fb_map[i] = idx)
set_con2fb_map(i, new_idx, 0);
}
- } else
+ } else {
+ struct fb_info *info = registered_fb[idx];
+
+ /* This is sort of like set_con2fb_map, except it maps
+ * the consoles to no device and then releases the
+ * oldinfo to free memory and cancel the cursor blink
+ * timer. I can imagine this just becoming part of
+ * set_con2fb_map where new_idx is -1
+ */
+ for (i = first_fb_vc; i <= last_fb_vc; i++) {
+ if (con2fb_map[i] = idx) {
+ con2fb_map[i] = -1;
+ if (!search_fb_in_map(idx)) {
+ ret = con2fb_release_oldinfo(vc_cons[i].d,
+ info, NULL, i,
+ idx, 0);
+ if (ret) {
+ con2fb_map[i] = idx;
+ return ret;
+ }
+ }
+ }
+ }
ret = fbcon_unbind();
+ }
return ret;
}
--
1.8.5.1
^ permalink raw reply related
* Re: [PATCH v3] x86: sysfb: remove sysfb when probing real hw
From: Ingo Molnar @ 2013-12-19 19:19 UTC (permalink / raw)
To: Stephen Warren
Cc: David Herrmann, linux-kernel, Takashi Iwai,
the arch/x86 maintainers, linux-fbdev, Geert Uytterhoeven, stable
In-Reply-To: <52B34454.1040603@wwwdotorg.org>
* Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 12/19/2013 11:55 AM, Ingo Molnar wrote:
> >
> > * Stephen Warren <swarren@wwwdotorg.org> wrote:
> >
> >> On 12/19/2013 03:13 AM, David Herrmann wrote:
> >>> With CONFIG_X86_SYSFB=y, probing real hw-drivers may result in
> >>> resource-conflicts and drivers will refuse to load. A call to
> >>> request_mem_region() will fail, if the region overlaps with the mem-region
> >>> used by simplefb. The common desktop DRM drivers (intel, nouveau, radeon)
> >>> are not affected as they don't reserve their resources, but some others
> >>> do, including (nvidiafb, cirrus, ..).
> >>
> >> I have validated that this doesn't cause any regressions on the/a
> >> non-x86 platform using simplefb, although given the main point of this
> >> patch is to fix issues on x86, I'm rather hesitant to give a tested-by
> >> tag in case someone looking back interprets it incorrectly:-)
> >
> > Tested-on-ARM-by: Stephen Warren <swarren@wwwdotorg.org>
>
> I suppose with CC: stable there's some precedent for a comment after the
> tag, so perhaps:
>
> Tested-by: Stephen Warren <swarren@wwwdotorg.org> # on ARM only
That works too.
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v3] x86: sysfb: remove sysfb when probing real hw
From: Stephen Warren @ 2013-12-19 19:09 UTC (permalink / raw)
To: Ingo Molnar
Cc: David Herrmann, linux-kernel, Takashi Iwai,
the arch/x86 maintainers, linux-fbdev, Geert Uytterhoeven, stable
In-Reply-To: <20131219185517.GA303@gmail.com>
On 12/19/2013 11:55 AM, Ingo Molnar wrote:
>
> * Stephen Warren <swarren@wwwdotorg.org> wrote:
>
>> On 12/19/2013 03:13 AM, David Herrmann wrote:
>>> With CONFIG_X86_SYSFB=y, probing real hw-drivers may result in
>>> resource-conflicts and drivers will refuse to load. A call to
>>> request_mem_region() will fail, if the region overlaps with the mem-region
>>> used by simplefb. The common desktop DRM drivers (intel, nouveau, radeon)
>>> are not affected as they don't reserve their resources, but some others
>>> do, including (nvidiafb, cirrus, ..).
>>
>> I have validated that this doesn't cause any regressions on the/a
>> non-x86 platform using simplefb, although given the main point of this
>> patch is to fix issues on x86, I'm rather hesitant to give a tested-by
>> tag in case someone looking back interprets it incorrectly:-)
>
> Tested-on-ARM-by: Stephen Warren <swarren@wwwdotorg.org>
I suppose with CC: stable there's some precedent for a comment after the
tag, so perhaps:
Tested-by: Stephen Warren <swarren@wwwdotorg.org> # on ARM only
^ permalink raw reply
* Re: [PATCH v3] x86: sysfb: remove sysfb when probing real hw
From: Ingo Molnar @ 2013-12-19 18:55 UTC (permalink / raw)
To: Stephen Warren
Cc: David Herrmann, linux-kernel, Takashi Iwai,
the arch/x86 maintainers, linux-fbdev, Geert Uytterhoeven, stable
In-Reply-To: <52B340CB.90807@wwwdotorg.org>
* Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 12/19/2013 03:13 AM, David Herrmann wrote:
> > With CONFIG_X86_SYSFB=y, probing real hw-drivers may result in
> > resource-conflicts and drivers will refuse to load. A call to
> > request_mem_region() will fail, if the region overlaps with the mem-region
> > used by simplefb. The common desktop DRM drivers (intel, nouveau, radeon)
> > are not affected as they don't reserve their resources, but some others
> > do, including (nvidiafb, cirrus, ..).
>
> I have validated that this doesn't cause any regressions on the/a
> non-x86 platform using simplefb, although given the main point of this
> patch is to fix issues on x86, I'm rather hesitant to give a tested-by
> tag in case someone looking back interprets it incorrectly:-)
Tested-on-ARM-by: Stephen Warren <swarren@wwwdotorg.org>
?
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v3] x86: sysfb: remove sysfb when probing real hw
From: Stephen Warren @ 2013-12-19 18:54 UTC (permalink / raw)
To: David Herrmann, linux-kernel
Cc: Takashi Iwai, the arch/x86 maintainers, linux-fbdev,
Geert Uytterhoeven, Ingo Molnar, stable
In-Reply-To: <1387448038-8260-1-git-send-email-dh.herrmann@gmail.com>
On 12/19/2013 03:13 AM, David Herrmann wrote:
> With CONFIG_X86_SYSFB=y, probing real hw-drivers may result in
> resource-conflicts and drivers will refuse to load. A call to
> request_mem_region() will fail, if the region overlaps with the mem-region
> used by simplefb. The common desktop DRM drivers (intel, nouveau, radeon)
> are not affected as they don't reserve their resources, but some others
> do, including (nvidiafb, cirrus, ..).
I have validated that this doesn't cause any regressions on the/a
non-x86 platform using simplefb, although given the main point of this
patch is to fix issues on x86, I'm rather hesitant to give a tested-by
tag in case someone looking back interprets it incorrectly:-)
^ permalink raw reply
* Re: [PATCH 0/4] OMAPDSS: DT support for N900 panel
From: Tony Lindgren @ 2013-12-19 18:34 UTC (permalink / raw)
To: Tomi Valkeinen, Benoît Cousson, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
linux-omap, linux-fbdev, devicetree
In-Reply-To: <20131219170028.GA24230@earth.universe>
* Sebastian Reichel <sre@ring0.de> [131219 09:01]:
> On Thu, Dec 19, 2013 at 08:42:31AM -0800, Tony Lindgren wrote:
> > * Sebastian Reichel <sre@debian.org> [131219 05:57]:
> > > On Thu, Dec 19, 2013 at 11:08:40AM +0100, Sebastian Reichel wrote:
> > > > > Are you able to check if the bootloader muxes dat3 to SDI mode?
> > > >
> > > > The bootloader's source code is not available as far as i know.
> > > >
> > > > [...], but I get an external abort on non-linefetch.
> > > > So I can't check it :(
> > >
> > > Ok. This is fixed by applying [0] (Thanks for the hint, Tomi!).
> > > This is the mux configuration from the bootloader:
> > >
> > > [...]
> > >
> > > [0] https://patchwork.kernel.org/patch/3283781/
> >
> > Do we need to update Laurent's patch with this?
>
> No, the patch is only needed to avoid the mentioned external abort
> on non-linefetch when doing "cat /sys/kernel/debug/pinctrl/.../pins".
>
> > Or can we use it as it is and maybe you can ack it?
>
> Sure. I will add an Ack.
OK thanks.
Here's my current legacy mux dumping tool if you want to look at
how the pins are muxed by the bootloader while booted to DT mode.
This is against omap-for-v3.14/dt branch, the list in pdata-quirks.c
may need updating depending on the board. And this does not account
for splitting up the pinctrl core for omap3..
Regards,
Tony
8< -----------------------------------
From 2fb3765fd5739e8f5fb4318e2576081be6d535f2 Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Thu, 19 Dec 2013 10:31:03 -0800
Subject: [PATCH] Not for merging: Allows dumping out mux entries in .dts
format using legacy mux
For pinctrl-single.c we should eventually have a user space
app to configure and display pin settings. Meanwhile, allow
using the legacy mux interface to do that:
# mount -t debugfs debugfs /sys/kernel/debug
# cat /sys/kernel/debug/omap_mux/board/wkup | grep fref_clk0_out
0x14 0x2 /* fref_clk0_out.sys_drm_msecure gpio6 OUTPUT | MODE2 */
diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
index 48094b58..40658d9 100644
--- a/arch/arm/mach-omap2/mux.c
+++ b/arch/arm/mach-omap2/mux.c
@@ -35,11 +35,10 @@
#include <linux/irq.h>
#include <linux/interrupt.h>
-
#include "omap_hwmod.h"
-
#include "soc.h"
#include "control.h"
+#include "id.h"
#include "mux.h"
#include "prm.h"
#include "common.h"
@@ -505,17 +504,17 @@ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
#define OMAP_MUX_TEST_FLAG(val, mask) \
if (((val) & (mask)) = (mask)) { \
i++; \
- flags[i] = #mask; \
+ flags[i] = #mask + sizeof("OMAP_") - 1; \
}
/* REVISIT: Add checking for non-optimal mux settings */
static inline void omap_mux_decode(struct seq_file *s, u16 val)
{
char *flags[OMAP_MUX_MAX_NR_FLAGS];
- char mode[sizeof("OMAP_MUX_MODE") + 1];
+ char mode[sizeof("MUX_MODE") + 1];
int i = -1;
- sprintf(mode, "OMAP_MUX_MODE%d", val & 0x7);
+ sprintf(mode, "MUX_MODE%d", val & 0x7);
i++;
flags[i] = mode;
@@ -553,7 +552,7 @@ static inline void omap_mux_decode(struct seq_file *s, u16 val)
}
} else {
i++;
- flags[i] = "OMAP_PIN_OUTPUT";
+ flags[i] = "PIN_OUTPUT";
}
do {
@@ -568,15 +567,26 @@ static inline void omap_mux_decode(struct seq_file *s, u16 val)
static int omap_mux_dbg_board_show(struct seq_file *s, void *unused)
{
struct omap_mux_partition *partition = s->private;
+ int pbase = (int)partition->base;
struct omap_mux_entry *e;
- u8 omap_gen = omap_rev() >> 28;
+
+ if (!(pbase & 0xfff))
+ pbase = 0x40;
+ else
+ pbase = 0;
+
+ seq_printf(s, "\t\tpinctrl-single,pins = <\n");
list_for_each_entry(e, &partition->muxmodes, node) {
struct omap_mux *m = &e->mux;
char m0_def[OMAP_MUX_DEFNAME_LEN];
char *m0_name = m->muxnames[0];
u16 val;
- int i, mode;
+ int padconf_offset, i, mode;
+
+ padconf_offset = m->reg_offset - pbase;
+ if (cpu_is_omap3630() && padconf_offset > 0x5ca)
+ continue;
if (!m0_name)
continue;
@@ -591,18 +601,18 @@ static int omap_mux_dbg_board_show(struct seq_file *s, void *unused)
}
val = omap_mux_read(partition, m->reg_offset);
mode = val & OMAP_MUX_MODE7;
- if (mode != 0)
- seq_printf(s, "/* %s */\n", m->muxnames[mode]);
-
- /*
- * XXX: Might be revisited to support differences across
- * same OMAP generation.
- */
- seq_printf(s, "OMAP%d_MUX(%s, ", omap_gen, m0_def);
+ seq_printf(s, "\t\t\t0x%x (",
+ padconf_offset);
omap_mux_decode(s, val);
- seq_printf(s, "),\n");
+ seq_printf(s, ")\t/* %s.%s */",
+ m->muxnames[0], m->muxnames[mode]);
+ seq_printf(s, " //0x%x gpio%i",
+ val, m->gpio);
+ seq_printf(s, "\n");
}
+ seq_printf(s, "\t\t>;\n");
+
return 0;
}
diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index 3d5b24d..a6d6e7e 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -22,6 +22,7 @@
#include "common-board-devices.h"
#include "dss-common.h"
#include "control.h"
+#include "mux.h"
struct pdata_init {
const char *compatible;
@@ -277,6 +278,39 @@ static struct pdata_init pdata_quirks[] __initdata = {
{ /* sentinel */ },
};
+struct board_package {
+ const char *compatible;
+ int (*fn)(struct omap_board_mux *board_mux, int flags);
+ int flags;
+};
+
+static struct board_package packages[] __initdata = {
+ { "nokia,n800", omap2420_mux_init, OMAP_PACKAGE_ZAC, },
+ { "nokia,n810", omap2420_mux_init, OMAP_PACKAGE_ZAC, },
+ { "ti,omap2430-sdp", omap2430_mux_init, OMAP_PACKAGE_ZAC, },
+ { "nokia,omap3-n900", omap3_mux_init, OMAP_PACKAGE_CBB, },
+ { "ti,omap3-evm-37xx", omap3_mux_init, OMAP_PACKAGE_CBB, },
+ { "ti,omap3-ldp", omap3_mux_init, OMAP_PACKAGE_CBB, },
+ { "ti,omap3-zoom3", omap3_mux_init, OMAP_PACKAGE_CBP, },
+ { "ti,omap4-sdp", omap4_mux_init, OMAP_PACKAGE_CBS, },
+ { "ti,omap4-panda", omap4_mux_init, OMAP_PACKAGE_CBS, },
+ { /* sentinel */ },
+};
+
+static void legacy_mux_init(void)
+{
+ struct board_package *pkg = packages;
+
+ while (pkg->compatible) {
+ if (of_machine_is_compatible(pkg->compatible)) {
+ if (pkg->fn)
+ pkg->fn(NULL, pkg->flags);
+ break;
+ }
+ pkg++;
+ }
+}
+
static void pdata_quirks_check(struct pdata_init *quirks)
{
while (quirks->compatible) {
@@ -296,4 +330,5 @@ void __init pdata_quirks_init(struct of_device_id *omap_dt_match_table)
of_platform_populate(NULL, omap_dt_match_table,
omap_auxdata_lookup, NULL);
pdata_quirks_check(pdata_quirks);
+ legacy_mux_init();
}
^ permalink raw reply related
* Re: [PATCH v4] x86: sysfb: remove sysfb when probing real hw
From: Ingo Molnar @ 2013-12-19 18:33 UTC (permalink / raw)
To: David Herrmann
Cc: linux-kernel, H. Peter Anvin, Thomas Gleixner, x86, linux-fbdev,
stable
In-Reply-To: <1387473881-26179-1-git-send-email-dh.herrmann@gmail.com>
* David Herrmann <dh.herrmann@gmail.com> wrote:
> +/*
> + * Unregister the sysfb and prevent new sysfbs from getting registered. Can be
> + * called from any context except recursively or from sysfb_register().
> + * Used by remove_conflicting_framebuffers() and friends.
> + */
> +void sysfb_unregister(const struct apertures_struct *apert, bool primary)
> +{
> + mutex_lock(&sysfb_lock);
> + if (!IS_ERR(sysfb_dev) && sysfb_dev) {
> + if (sysfb_match(apert, primary)) {
> + platform_device_unregister(sysfb_dev);
> + sysfb_dev = ERR_PTR(-EALREADY);
> + }
> + } else {
> + /* if !sysfb_dev, set err so no new sysfb is probed later */
> + sysfb_dev = ERR_PTR(-EALREADY);
Just a small detail: we can get into this 'else' branch not just with
NULL, but also with IS_ERR(sysfb_dev). In that case we override
whatever error code is contained in sysfb_dev and overwrite it with
ERR_PTR(-EALREADY).
(Probably not a big deal, because we don't actually ever seem to
extract the error code from the pointer, but wanted to mention it.)
> +#ifdef CONFIG_X86_SYSFB
> +#include <asm/sysfb.h>
> +#endif
Pet peeve, this looks sexier:
#ifdef CONFIG_X86_SYSFB
# include <asm/sysfb.h>
#endif
> @@ -1604,6 +1607,17 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a,
> }
> }
>
> +static void remove_conflicting_sysfb(const struct apertures_struct *apert,
> + bool primary)
> +{
> + if (!apert)
> + return;
> +
> +#ifdef CONFIG_X86_SYSFB
> + sysfb_unregister(apert, primary);
> +#endif
> +}
So why not make sysfb_unregister() accept a !apert parameter (it would
simply return), at which point remove_conflicting_sysfb() could be
eliminated and just be replaced with a direct sysfb_unregister() call
- with no #ifdefs.
We only need #ifdefs for the sysfb_unregister() declaration in the .h
file.
At first sight this looks simpler and cleaner for the fix itself - no
need for extra cleanups for this detail.
Thanks,
Ingo
^ permalink raw reply
* [PATCH v4] x86: sysfb: remove sysfb when probing real hw
From: David Herrmann @ 2013-12-19 17:24 UTC (permalink / raw)
To: linux-kernel
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-fbdev,
David Herrmann, stable
In-Reply-To: <1387448038-8260-1-git-send-email-dh.herrmann@gmail.com>
With CONFIG_X86_SYSFB=y, probing real hw-drivers may result in
resource-conflicts and drivers will refuse to load. A call to
request_mem_region() will fail, if the region overlaps with the mem-region
used by simplefb. The common desktop DRM drivers (intel, nouveau, radeon)
are not affected as they don't reserve their resources, but some others
do, including (nvidiafb, cirrus, ..).
The problem is that we add an IORESOURCE_MEM to the simple-framebuffer
platform-device during bootup but never release it. Probing simplefb on
this platform-device is fine, but the handover to real-hw via
remove_conflicting_framebuffers() will only unload the fbdev driver, but
keep the platform-device around. Thus, if the real hw-driver calls
request_mem_region() and friends on the same PCI region, we will get a
resource conflict and most drivers refuse to load. Users will see
errors like:
"nvidiafb: cannot reserve video memory at <xyz>"
vesafb and efifb don't store any resources, so disabling CONFIG_X86_SYSFB
and falling back to those drivers will avoid the bug. With sysfb enabled,
we need to properly unload the simple-framebuffer devices before probing
real hw-drivers.
This patch adds sysfb_unregister() for that purpose. It can be called from
any context (except from the platform-device ->probe and ->remove callback
path), synchronously unloads any global sysfb and prevents new sysfbs from
getting registered. Thus, you can call it even before any sysfb has been
loaded. Note that for now we only do that for simple-framebuffer devices,
as efi/vesa-framebuffer platform-drivers lack ->remove() callbacks.
They're not affected by the resource-conflicts, so we can fix those later.
This also changes remove_conflicting_framebuffer() to call this helper
*before* trying its heuristic to remove conflicting drivers. This way, we
unload sysfbs properly on any conflict. But to avoid dead-locks in
register_framebuffer(), we must not call sysfb_unregister() for
framebuffers probing on sysfb devices. Hence, we simply remove any
aperture from simplefb and we're good to go. simplefb is unregistered by
sysfb_unregister() now, so no reason to keep the apertures (on non-x86,
there currently is no handover from simplefb, so we're fine. If it's
added, they need to provide something like sysfb_unregister() too)
Cc: <stable@vger.kernel.org> # v3.11+
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Takashi Iwai <tiwai@suse.de>
---
v4:
- change #ifdef to CONFIG_X86_SYSFB again
arch/x86/include/asm/sysfb.h | 6 ++++
arch/x86/kernel/sysfb.c | 68 ++++++++++++++++++++++++++++++++++++++++
arch/x86/kernel/sysfb_simplefb.c | 9 ++----
drivers/video/fbmem.c | 19 +++++++++++
drivers/video/simplefb.c | 8 -----
5 files changed, 95 insertions(+), 15 deletions(-)
diff --git a/arch/x86/include/asm/sysfb.h b/arch/x86/include/asm/sysfb.h
index 2aeb3e2..0877cf1 100644
--- a/arch/x86/include/asm/sysfb.h
+++ b/arch/x86/include/asm/sysfb.h
@@ -11,6 +11,7 @@
* any later version.
*/
+#include <linux/fb.h>
#include <linux/kernel.h>
#include <linux/platform_data/simplefb.h>
#include <linux/screen_info.h>
@@ -59,6 +60,11 @@ struct efifb_dmi_info {
int flags;
};
+int __init sysfb_register(const char *name, int id,
+ const struct resource *res, unsigned int res_num,
+ const void *data, size_t data_size);
+void sysfb_unregister(const struct apertures_struct *apert, bool primary);
+
#ifdef CONFIG_EFI
extern struct efifb_dmi_info efifb_dmi_list[];
diff --git a/arch/x86/kernel/sysfb.c b/arch/x86/kernel/sysfb.c
index 193ec2c..882dc7c 100644
--- a/arch/x86/kernel/sysfb.c
+++ b/arch/x86/kernel/sysfb.c
@@ -33,11 +33,79 @@
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/mm.h>
+#include <linux/mutex.h>
#include <linux/platform_data/simplefb.h>
#include <linux/platform_device.h>
#include <linux/screen_info.h>
#include <asm/sysfb.h>
+static DEFINE_MUTEX(sysfb_lock);
+static struct platform_device *sysfb_dev;
+
+int __init sysfb_register(const char *name, int id,
+ const struct resource *res, unsigned int res_num,
+ const void *data, size_t data_size)
+{
+ struct platform_device *pd;
+ int ret = 0;
+
+ mutex_lock(&sysfb_lock);
+ if (!sysfb_dev) {
+ pd = platform_device_register_resndata(NULL, name, id,
+ res, res_num,
+ data, data_size);
+ if (IS_ERR(pd))
+ ret = PTR_ERR(pd);
+ else
+ sysfb_dev = pd;
+ }
+ mutex_unlock(&sysfb_lock);
+
+ return ret;
+}
+
+static bool sysfb_match(const struct apertures_struct *apert, bool primary)
+{
+ struct screen_info *si = &screen_info;
+ unsigned int i;
+ const struct aperture *a;
+
+ if (!apert || primary)
+ return true;
+
+ for (i = 0; i < apert->count; ++i) {
+ a = &apert->ranges[i];
+ if (a->base >= si->lfb_base &&
+ a->base < si->lfb_base + ((u64)si->lfb_size << 16))
+ return true;
+ if (si->lfb_base >= a->base &&
+ si->lfb_base < a->base + a->size)
+ return true;
+ }
+
+ return false;
+}
+
+/*
+ * Unregister the sysfb and prevent new sysfbs from getting registered. Can be
+ * called from any context except recursively or from sysfb_register().
+ * Used by remove_conflicting_framebuffers() and friends.
+ */
+void sysfb_unregister(const struct apertures_struct *apert, bool primary)
+{
+ mutex_lock(&sysfb_lock);
+ if (!IS_ERR(sysfb_dev) && sysfb_dev) {
+ if (sysfb_match(apert, primary)) {
+ platform_device_unregister(sysfb_dev);
+ sysfb_dev = ERR_PTR(-EALREADY);
+ }
+ } else {
+ /* if !sysfb_dev, set err so no new sysfb is probed later */
+ sysfb_dev = ERR_PTR(-EALREADY);
+ }
+ mutex_unlock(&sysfb_lock);
+}
+
static __init int sysfb_init(void)
{
struct screen_info *si = &screen_info;
diff --git a/arch/x86/kernel/sysfb_simplefb.c b/arch/x86/kernel/sysfb_simplefb.c
index 86179d4..a760d47 100644
--- a/arch/x86/kernel/sysfb_simplefb.c
+++ b/arch/x86/kernel/sysfb_simplefb.c
@@ -64,7 +64,6 @@ __init bool parse_mode(const struct screen_info *si,
__init int create_simplefb(const struct screen_info *si,
const struct simplefb_platform_data *mode)
{
- struct platform_device *pd;
struct resource res;
unsigned long len;
@@ -86,10 +85,6 @@ __init int create_simplefb(const struct screen_info *si,
if (res.end <= res.start)
return -EINVAL;
- pd = platform_device_register_resndata(NULL, "simple-framebuffer", 0,
- &res, 1, mode, sizeof(*mode));
- if (IS_ERR(pd))
- return PTR_ERR(pd);
-
- return 0;
+ return sysfb_register("simple-framebuffer", 0, &res, 1, mode,
+ sizeof(*mode));
}
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 010d191..7dc0491 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -35,6 +35,9 @@
#include <asm/fb.h>
+#ifdef CONFIG_X86_SYSFB
+#include <asm/sysfb.h>
+#endif
/*
* Frame buffer device initialization and setup routines
@@ -1604,6 +1607,17 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a,
}
}
+static void remove_conflicting_sysfb(const struct apertures_struct *apert,
+ bool primary)
+{
+ if (!apert)
+ return;
+
+#ifdef CONFIG_X86_SYSFB
+ sysfb_unregister(apert, primary);
+#endif
+}
+
static int do_register_framebuffer(struct fb_info *fb_info)
{
int i;
@@ -1742,6 +1756,8 @@ EXPORT_SYMBOL(unlink_framebuffer);
void remove_conflicting_framebuffers(struct apertures_struct *a,
const char *name, bool primary)
{
+ remove_conflicting_sysfb(a, primary);
+
mutex_lock(®istration_lock);
do_remove_conflicting_framebuffers(a, name, primary);
mutex_unlock(®istration_lock);
@@ -1762,6 +1778,9 @@ register_framebuffer(struct fb_info *fb_info)
{
int ret;
+ remove_conflicting_sysfb(fb_info->apertures,
+ fb_is_primary_device(fb_info));
+
mutex_lock(®istration_lock);
ret = do_register_framebuffer(fb_info);
mutex_unlock(®istration_lock);
diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
index 210f3a0..9f4a0cf 100644
--- a/drivers/video/simplefb.c
+++ b/drivers/video/simplefb.c
@@ -209,14 +209,6 @@ static int simplefb_probe(struct platform_device *pdev)
info->var.blue = params.format->blue;
info->var.transp = params.format->transp;
- info->apertures = alloc_apertures(1);
- if (!info->apertures) {
- framebuffer_release(info);
- return -ENOMEM;
- }
- info->apertures->ranges[0].base = info->fix.smem_start;
- info->apertures->ranges[0].size = info->fix.smem_len;
-
info->fbops = &simplefb_ops;
info->flags = FBINFO_DEFAULT | FBINFO_MISC_FIRMWARE;
info->screen_base = ioremap_wc(info->fix.smem_start,
--
1.8.5.1
^ permalink raw reply related
* Re: [PATCH v3] x86: sysfb: remove sysfb when probing real hw
From: David Herrmann @ 2013-12-19 17:18 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Takashi Iwai, Stephen Warren,
the arch/x86 maintainers, linux-fbdev@vger.kernel.org,
Geert Uytterhoeven, stable
In-Reply-To: <20131219170927.GC30382@gmail.com>
Hi
On Thu, Dec 19, 2013 at 6:09 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * David Herrmann <dh.herrmann@gmail.com> wrote:
>
>> Hi
>>
>> On Thu, Dec 19, 2013 at 5:36 PM, Ingo Molnar <mingo@kernel.org> wrote:
>> >
>> > * David Herrmann <dh.herrmann@gmail.com> wrote:
>> >
>> >> --- a/drivers/video/fbmem.c
>> >> +++ b/drivers/video/fbmem.c
>> >> @@ -35,6 +35,9 @@
>> >>
>> >> #include <asm/fb.h>
>> >>
>> >> +#if IS_ENABLED(CONFIG_X86)
>> >> +#include <asm/sysfb.h>
>> >> +#endif
>> >
>> > I think this can be written as:
>> >
>> > #ifdef CONFIG_X86
>> > # include <asm/sysfb.h>
>> > #endif
>> >
>> > also ... the dependency on a large, unrelated option like CONFIG_X86
>> > looks pretty ugly here - is there no other, more specific CONFIG_
>> > option that can be used here - such as CONFIG_FB_SIMPLE or
>> > CONFIG_X86_SYSFB?
>> >
>> >> +#if IS_ENABLED(CONFIG_X86)
>> >> + sysfb_unregister(apert, primary);
>> >> +#endif
>> >
>> > Ditto.
>>
>> CONFIG_X86 is probably never 'm'.. will fix that. It was
>> CONFIG_X86_SYSFB before and that works, too, but the broader X86
>> seemed more appropriate as sysfb is available on all x86.
>
> Well, sysfb is available if CONFIG_X86_SYSFB is set, right? So on
> !CONFIG_X86_SYSFB x86 kernels this code should not run, right?
No. The sysfb code is always enabled. It provides platform-devices for
firmware-framebuffers which efifb/vesafb pick up. The X86_SYSFB option
only controls whether generic system-framebuffers should be used
instead of the specific efi/vesa FBs (to be compatible to other
architectures and keep efi/vesa specifics in arch/x86/).
But the system-framebuffer conversion (to a format compatible to
simplefb/simple-framebuffer) is what may break as the legacy fbs don't
reserve resources. Hence, putting it enclosed into X86_SYSFB works for
now. But if we start registering efifb/vesafb with sysfb, too, then we
need to change it to CONFIG_X86 as all fbs are affected then.
I think using X86_SYSFB (as in v1 of this patch) is the way to go. I
will fix it up and resend v4. The broader #ifdef can be used once we
do the more complex cleanups, in which case it will go away entirely.
>> Note that I have patches here locally which move
>> sysfb_register/unregister to drivers/video/sysfb.c and add
>> include/linux/sysfb.h with dummies if CONFIG_SYSFB is not selected
>> to avoid the #ifdef. This will allow other architectures to do
>> gfx-hand-over, too. They seem too big for stable, though. That's why
>> I split them up and added it to x86/kernel/sysfb.c first.
>
> Yeah, it's fine to do those cleanups after the minimal fix. But using
> a sensible config option must still be done - we cannot just slap
> broad CONFIG_X86 dependencies into random code.
Ok.
Thanks
David
^ permalink raw reply
* Re: [PATCH v3] x86: sysfb: remove sysfb when probing real hw
From: Ingo Molnar @ 2013-12-19 17:09 UTC (permalink / raw)
To: David Herrmann
Cc: linux-kernel, Takashi Iwai, Stephen Warren,
the arch/x86 maintainers, linux-fbdev@vger.kernel.org,
Geert Uytterhoeven, stable
In-Reply-To: <CANq1E4Ss30G72t3R1iza9vOO7ESYQynjS74mB+nJ-zT2zAvyzQ@mail.gmail.com>
* David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Thu, Dec 19, 2013 at 5:36 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * David Herrmann <dh.herrmann@gmail.com> wrote:
> >
> >> --- a/drivers/video/fbmem.c
> >> +++ b/drivers/video/fbmem.c
> >> @@ -35,6 +35,9 @@
> >>
> >> #include <asm/fb.h>
> >>
> >> +#if IS_ENABLED(CONFIG_X86)
> >> +#include <asm/sysfb.h>
> >> +#endif
> >
> > I think this can be written as:
> >
> > #ifdef CONFIG_X86
> > # include <asm/sysfb.h>
> > #endif
> >
> > also ... the dependency on a large, unrelated option like CONFIG_X86
> > looks pretty ugly here - is there no other, more specific CONFIG_
> > option that can be used here - such as CONFIG_FB_SIMPLE or
> > CONFIG_X86_SYSFB?
> >
> >> +#if IS_ENABLED(CONFIG_X86)
> >> + sysfb_unregister(apert, primary);
> >> +#endif
> >
> > Ditto.
>
> CONFIG_X86 is probably never 'm'.. will fix that. It was
> CONFIG_X86_SYSFB before and that works, too, but the broader X86
> seemed more appropriate as sysfb is available on all x86.
Well, sysfb is available if CONFIG_X86_SYSFB is set, right? So on
!CONFIG_X86_SYSFB x86 kernels this code should not run, right?
> Note that I have patches here locally which move
> sysfb_register/unregister to drivers/video/sysfb.c and add
> include/linux/sysfb.h with dummies if CONFIG_SYSFB is not selected
> to avoid the #ifdef. This will allow other architectures to do
> gfx-hand-over, too. They seem too big for stable, though. That's why
> I split them up and added it to x86/kernel/sysfb.c first.
Yeah, it's fine to do those cleanups after the minimal fix. But using
a sensible config option must still be done - we cannot just slap
broad CONFIG_X86 dependencies into random code.
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v3] x86: sysfb: remove sysfb when probing real hw
From: David Herrmann @ 2013-12-19 17:03 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Takashi Iwai, Stephen Warren,
the arch/x86 maintainers, linux-fbdev@vger.kernel.org,
Geert Uytterhoeven, stable
In-Reply-To: <20131219163606.GA29243@gmail.com>
Hi
On Thu, Dec 19, 2013 at 5:36 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * David Herrmann <dh.herrmann@gmail.com> wrote:
>
>> --- a/drivers/video/fbmem.c
>> +++ b/drivers/video/fbmem.c
>> @@ -35,6 +35,9 @@
>>
>> #include <asm/fb.h>
>>
>> +#if IS_ENABLED(CONFIG_X86)
>> +#include <asm/sysfb.h>
>> +#endif
>
> I think this can be written as:
>
> #ifdef CONFIG_X86
> # include <asm/sysfb.h>
> #endif
>
> also ... the dependency on a large, unrelated option like CONFIG_X86
> looks pretty ugly here - is there no other, more specific CONFIG_
> option that can be used here - such as CONFIG_FB_SIMPLE or
> CONFIG_X86_SYSFB?
>
>> +#if IS_ENABLED(CONFIG_X86)
>> + sysfb_unregister(apert, primary);
>> +#endif
>
> Ditto.
CONFIG_X86 is probably never 'm'.. will fix that.
It was CONFIG_X86_SYSFB before and that works, too, but the broader
X86 seemed more appropriate as sysfb is available on all x86.
Note that I have patches here locally which move
sysfb_register/unregister to drivers/video/sysfb.c and add
include/linux/sysfb.h with dummies if CONFIG_SYSFB is not selected to
avoid the #ifdef. This will allow other architectures to do
gfx-hand-over, too. They seem too big for stable, though. That's why I
split them up and added it to x86/kernel/sysfb.c first.
Thanks
David
^ permalink raw reply
* Re: [PATCH 0/4] OMAPDSS: DT support for N900 panel
From: Sebastian Reichel @ 2013-12-19 17:00 UTC (permalink / raw)
To: Tony Lindgren
Cc: Tomi Valkeinen, Benoît Cousson, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
linux-omap, linux-fbdev, devicetree
In-Reply-To: <20131219163557.GM27438@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 946 bytes --]
On Thu, Dec 19, 2013 at 08:42:31AM -0800, Tony Lindgren wrote:
> * Sebastian Reichel <sre@debian.org> [131219 05:57]:
> > On Thu, Dec 19, 2013 at 11:08:40AM +0100, Sebastian Reichel wrote:
> > > > Are you able to check if the bootloader muxes dat3 to SDI mode?
> > >
> > > The bootloader's source code is not available as far as i know.
> > >
> > > [...], but I get an external abort on non-linefetch.
> > > So I can't check it :(
> >
> > Ok. This is fixed by applying [0] (Thanks for the hint, Tomi!).
> > This is the mux configuration from the bootloader:
> >
> > [...]
> >
> > [0] https://patchwork.kernel.org/patch/3283781/
>
> Do we need to update Laurent's patch with this?
No, the patch is only needed to avoid the mentioned external abort
on non-linefetch when doing "cat /sys/kernel/debug/pinctrl/.../pins".
> Or can we use it as it is and maybe you can ack it?
Sure. I will add an Ack.
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 0/4] OMAPDSS: DT support for N900 panel
From: Tony Lindgren @ 2013-12-19 16:42 UTC (permalink / raw)
To: Tomi Valkeinen, Benoît Cousson, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
linux-omap, linux-fbdev, devicetree
In-Reply-To: <20131219135608.GA11654@earth.universe>
* Sebastian Reichel <sre@debian.org> [131219 05:57]:
> On Thu, Dec 19, 2013 at 11:08:40AM +0100, Sebastian Reichel wrote:
> > > Are you able to check if the bootloader muxes dat3 to SDI mode?
> >
> > The bootloader's source code is not available as far as i know.
> >
> > [...], but I get an external abort on non-linefetch.
> > So I can't check it :(
>
> Ok. This is fixed by applying [0] (Thanks for the hint, Tomi!).
> This is the mux configuration from the bootloader:
>
> ...
> pin 96 (480020f0.0) 00000001 pinctrl-single // sdi dat1n
> pin 97 (480020f2.0) 00000001 pinctrl-single // sdi dat1p
> pin 98 (480020f4.0) 00000001 pinctrl-single // sdi dat2n
> pin 99 (480020f6.0) 00000001 pinctrl-single // sdi dat2p
> pin 100 (480020f8.0) 00000007 pinctrl-single // sdi dat3n
> pin 101 (480020fa.0) 00000007 pinctrl-single // sdi dat3p
> pin 102 (480020fc.0) 00000004 pinctrl-single
> pin 103 (480020fe.0) 00000004 pinctrl-single
> pin 104 (48002100.0) 00000004 pinctrl-single // sdi vsync
> pin 105 (48002102.0) 00004104 pinctrl-single // sdi hsync
> pin 106 (48002104.0) 00000004 pinctrl-single // sdi den
> pin 107 (48002106.0) 00000004 pinctrl-single // sdi stp
> pin 108 (48002108.0) 00000001 pinctrl-single // sdi clkp
> pin 109 (4800210a.0) 00000001 pinctrl-single // sdi clkn
> ...
>
> I guess the following entry should be added to the omap3-n900.dts file:
>
> dss_sdi_pins: pinmux_dss_sdi_pins {
> pinctrl-single,pins = <
> 0x0c0 (PIN_OUTPUT | MUX_MODE1) /* dss_data10.sdi_dat1n */
> 0x0c2 (PIN_OUTPUT | MUX_MODE1) /* dss_data11.sdi_dat1p */
> 0x0c4 (PIN_OUTPUT | MUX_MODE1) /* dss_data12.sdi_dat2n */
> 0x0c6 (PIN_OUTPUT | MUX_MODE1) /* dss_data13.sdi_dat2p */
> 0x0d8 (PIN_OUTPUT | MUX_MODE1) /* dss_data22.sdi_clkp */
> 0x0da (PIN_OUTPUT | MUX_MODE1) /* dss_data23.sdi_clkn */
> >;
> };
>
> [0] https://patchwork.kernel.org/patch/3283781/
Do we need to update Laurent's patch with this? Or can we use it as
it is and maybe you can ack it?
Regards,
Tony
^ permalink raw reply
* Re: [PATCH v3] x86: sysfb: remove sysfb when probing real hw
From: Ingo Molnar @ 2013-12-19 16:36 UTC (permalink / raw)
To: David Herrmann
Cc: linux-kernel, Takashi Iwai, Stephen Warren,
the arch/x86 maintainers, linux-fbdev, Geert Uytterhoeven, stable
In-Reply-To: <1387448038-8260-1-git-send-email-dh.herrmann@gmail.com>
* David Herrmann <dh.herrmann@gmail.com> wrote:
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -35,6 +35,9 @@
>
> #include <asm/fb.h>
>
> +#if IS_ENABLED(CONFIG_X86)
> +#include <asm/sysfb.h>
> +#endif
I think this can be written as:
#ifdef CONFIG_X86
# include <asm/sysfb.h>
#endif
also ... the dependency on a large, unrelated option like CONFIG_X86
looks pretty ugly here - is there no other, more specific CONFIG_
option that can be used here - such as CONFIG_FB_SIMPLE or
CONFIG_X86_SYSFB?
> +#if IS_ENABLED(CONFIG_X86)
> + sysfb_unregister(apert, primary);
> +#endif
Ditto.
Thanks,
Ingo
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox