* [PATCH v9 04/10] drm/mediatek: add BLS component
From: YT Shen @ 2016-11-11 11:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478865346-19043-1-git-send-email-yt.shen@mediatek.com>
Add BLS component for PWM + GAMMA function
Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 5 ++++-
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 2 ++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 661a4a0..b78b2e6 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -235,6 +235,7 @@ static void mtk_gamma_set(struct mtk_ddp_comp *comp,
[MTK_DISP_PWM] = "pwm",
[MTK_DISP_MUTEX] = "mutex",
[MTK_DISP_OD] = "od",
+ [MTK_DISP_BLS] = "bls",
};
struct mtk_ddp_comp_match {
@@ -245,6 +246,7 @@ struct mtk_ddp_comp_match {
static const struct mtk_ddp_comp_match mtk_ddp_matches[DDP_COMPONENT_ID_MAX] = {
[DDP_COMPONENT_AAL] = { MTK_DISP_AAL, 0, &ddp_aal },
+ [DDP_COMPONENT_BLS] = { MTK_DISP_BLS, 0, NULL },
[DDP_COMPONENT_COLOR0] = { MTK_DISP_COLOR, 0, &ddp_color },
[DDP_COMPONENT_COLOR1] = { MTK_DISP_COLOR, 1, &ddp_color },
[DDP_COMPONENT_DPI0] = { MTK_DPI, 0, NULL },
@@ -303,7 +305,8 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node,
comp->id = comp_id;
comp->funcs = funcs ?: mtk_ddp_matches[comp_id].funcs;
- if (comp_id == DDP_COMPONENT_DPI0 ||
+ if (comp_id == DDP_COMPONENT_BLS ||
+ comp_id == DDP_COMPONENT_DPI0 ||
comp_id == DDP_COMPONENT_DSI0 ||
comp_id == DDP_COMPONENT_PWM0) {
comp->regs = NULL;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index 2f6872a..30faf46 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -36,11 +36,13 @@ enum mtk_ddp_comp_type {
MTK_DISP_PWM,
MTK_DISP_MUTEX,
MTK_DISP_OD,
+ MTK_DISP_BLS,
MTK_DDP_COMP_TYPE_MAX,
};
enum mtk_ddp_comp_id {
DDP_COMPONENT_AAL,
+ DDP_COMPONENT_BLS,
DDP_COMPONENT_COLOR0,
DDP_COMPONENT_COLOR1,
DDP_COMPONENT_DPI0,
--
1.9.1
^ permalink raw reply related
* [PATCH v9 05/10] drm/mediatek: update display module connections
From: YT Shen @ 2016-11-11 11:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478865346-19043-1-git-send-email-yt.shen@mediatek.com>
update connections for OVL, RDMA, BLS, DSI
Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index b77d456..a9b209c 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -32,6 +32,10 @@
#define DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN 0x0c8
#define DISP_REG_CONFIG_MMSYS_CG_CON0 0x100
+#define DISP_REG_CONFIG_DISP_OVL_MOUT_EN 0x030
+#define DISP_REG_CONFIG_OUT_SEL 0x04c
+#define DISP_REG_CONFIG_DSI_SEL 0x050
+
#define DISP_REG_MUTEX_EN(n) (0x20 + 0x20 * (n))
#define DISP_REG_MUTEX(n) (0x24 + 0x20 * (n))
#define DISP_REG_MUTEX_RST(n) (0x28 + 0x20 * (n))
@@ -71,6 +75,10 @@
#define DPI0_SEL_IN_RDMA1 0x1
#define COLOR1_SEL_IN_OVL1 0x1
+#define OVL_MOUT_EN_RDMA 0x1
+#define BLS_TO_DSI_RDMA1_TO_DPI1 0x8
+#define DSI_SEL_IN_BLS 0x0
+
struct mtk_disp_mutex {
int id;
bool claimed;
@@ -111,6 +119,9 @@ static unsigned int mtk_ddp_mout_en(enum mtk_ddp_comp_id cur,
if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_COLOR0) {
*addr = DISP_REG_CONFIG_DISP_OVL0_MOUT_EN;
value = OVL0_MOUT_EN_COLOR0;
+ } else if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_RDMA0) {
+ *addr = DISP_REG_CONFIG_DISP_OVL_MOUT_EN;
+ value = OVL_MOUT_EN_RDMA;
} else if (cur == DDP_COMPONENT_OD && next == DDP_COMPONENT_RDMA0) {
*addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN;
value = OD_MOUT_EN_RDMA0;
@@ -148,6 +159,9 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id cur,
} else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) {
*addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN;
value = COLOR1_SEL_IN_OVL1;
+ } else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
+ *addr = DISP_REG_CONFIG_DSI_SEL;
+ value = DSI_SEL_IN_BLS;
} else {
value = 0;
}
@@ -155,6 +169,15 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id cur,
return value;
}
+static void mtk_ddp_sout_sel(void __iomem *config_regs,
+ enum mtk_ddp_comp_id cur,
+ enum mtk_ddp_comp_id next)
+{
+ if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0)
+ writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
+ config_regs + DISP_REG_CONFIG_OUT_SEL);
+}
+
void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
enum mtk_ddp_comp_id cur,
enum mtk_ddp_comp_id next)
@@ -167,6 +190,8 @@ void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
writel_relaxed(reg, config_regs + addr);
}
+ mtk_ddp_sout_sel(config_regs, cur, next);
+
value = mtk_ddp_sel_in(cur, next, &addr);
if (value) {
reg = readl_relaxed(config_regs + addr) | value;
--
1.9.1
^ permalink raw reply related
* [PATCH v9 06/10] drm/mediatek: cleaning up and refine
From: YT Shen @ 2016-11-11 11:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478865346-19043-1-git-send-email-yt.shen@mediatek.com>
cleaning up unused define and refine function name and variable
Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 77 ++++++++++++++++------------------
drivers/gpu/drm/mediatek/mtk_mipi_tx.c | 8 ++--
2 files changed, 41 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 28b2044..4efeb38 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -27,9 +27,6 @@
#include "mtk_drm_ddp_comp.h"
-#define DSI_VIDEO_FIFO_DEPTH (1920 / 4)
-#define DSI_HOST_FIFO_DEPTH 64
-
#define DSI_START 0x00
#define DSI_CON_CTRL 0x10
@@ -46,7 +43,7 @@
#define MIX_MODE BIT(17)
#define DSI_TXRX_CTRL 0x18
-#define VC_NUM (2 << 0)
+#define VC_NUM BIT(1)
#define LANE_NUM (0xf << 2)
#define DIS_EOT BIT(6)
#define NULL_EN BIT(7)
@@ -158,11 +155,11 @@ static void mtk_dsi_mask(struct mtk_dsi *dsi, u32 offset, u32 mask, u32 data)
writel((temp & ~mask) | (data & mask), dsi->regs + offset);
}
-static void dsi_phy_timconfig(struct mtk_dsi *dsi)
+static void mtk_dsi_phy_timconfig(struct mtk_dsi *dsi)
{
u32 timcon0, timcon1, timcon2, timcon3;
- unsigned int ui, cycle_time;
- unsigned int lpx;
+ u32 ui, cycle_time;
+ u32 lpx;
ui = 1000 / dsi->data_rate + 0x01;
cycle_time = 8000 / dsi->data_rate + 0x01;
@@ -192,7 +189,7 @@ static void mtk_dsi_disable(struct mtk_dsi *dsi)
mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, 0);
}
-static void mtk_dsi_reset(struct mtk_dsi *dsi)
+static void mtk_dsi_reset_engine(struct mtk_dsi *dsi)
{
mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_RESET, DSI_RESET);
mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_RESET, 0);
@@ -235,8 +232,8 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
}
mtk_dsi_enable(dsi);
- mtk_dsi_reset(dsi);
- dsi_phy_timconfig(dsi);
+ mtk_dsi_reset_engine(dsi);
+ mtk_dsi_phy_timconfig(dsi);
return 0;
@@ -249,33 +246,33 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
return ret;
}
-static void dsi_clk_ulp_mode_enter(struct mtk_dsi *dsi)
+static void mtk_dsi_clk_ulp_mode_enter(struct mtk_dsi *dsi)
{
mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
}
-static void dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
+static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
{
mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_WAKEUP_EN, LC_WAKEUP_EN);
mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_WAKEUP_EN, 0);
}
-static void dsi_lane0_ulp_mode_enter(struct mtk_dsi *dsi)
+static void mtk_dsi_lane0_ulp_mode_enter(struct mtk_dsi *dsi)
{
mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_HS_TX_EN, 0);
mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
}
-static void dsi_lane0_ulp_mode_leave(struct mtk_dsi *dsi)
+static void mtk_dsi_lane0_ulp_mode_leave(struct mtk_dsi *dsi)
{
mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_WAKEUP_EN, LD0_WAKEUP_EN);
mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_WAKEUP_EN, 0);
}
-static bool dsi_clk_hs_state(struct mtk_dsi *dsi)
+static bool mtk_dsi_clk_hs_state(struct mtk_dsi *dsi)
{
u32 tmp_reg1;
@@ -283,15 +280,15 @@ static bool dsi_clk_hs_state(struct mtk_dsi *dsi)
return ((tmp_reg1 & LC_HS_TX_EN) == 1) ? true : false;
}
-static void dsi_clk_hs_mode(struct mtk_dsi *dsi, bool enter)
+static void mtk_dsi_clk_hs_mode(struct mtk_dsi *dsi, bool enter)
{
- if (enter && !dsi_clk_hs_state(dsi))
+ if (enter && !mtk_dsi_clk_hs_state(dsi))
mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, LC_HS_TX_EN);
- else if (!enter && dsi_clk_hs_state(dsi))
+ else if (!enter && mtk_dsi_clk_hs_state(dsi))
mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
}
-static void dsi_set_mode(struct mtk_dsi *dsi)
+static void mtk_dsi_set_mode(struct mtk_dsi *dsi)
{
u32 vid_mode = CMD_MODE;
@@ -306,7 +303,7 @@ static void dsi_set_mode(struct mtk_dsi *dsi)
writel(vid_mode, dsi->regs + DSI_MODE_CTRL);
}
-static void dsi_ps_control_vact(struct mtk_dsi *dsi)
+static void mtk_dsi_ps_control_vact(struct mtk_dsi *dsi)
{
struct videomode *vm = &dsi->vm;
u32 dsi_buf_bpp, ps_wc;
@@ -340,7 +337,7 @@ static void dsi_ps_control_vact(struct mtk_dsi *dsi)
writel(ps_wc, dsi->regs + DSI_HSTX_CKL_WC);
}
-static void dsi_rxtx_control(struct mtk_dsi *dsi)
+static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
{
u32 tmp_reg;
@@ -365,9 +362,9 @@ static void dsi_rxtx_control(struct mtk_dsi *dsi)
writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
}
-static void dsi_ps_control(struct mtk_dsi *dsi)
+static void mtk_dsi_ps_control(struct mtk_dsi *dsi)
{
- unsigned int dsi_tmp_buf_bpp;
+ u32 dsi_tmp_buf_bpp;
u32 tmp_reg;
switch (dsi->format) {
@@ -397,12 +394,12 @@ static void dsi_ps_control(struct mtk_dsi *dsi)
writel(tmp_reg, dsi->regs + DSI_PSCTRL);
}
-static void dsi_config_vdo_timing(struct mtk_dsi *dsi)
+static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
{
- unsigned int horizontal_sync_active_byte;
- unsigned int horizontal_backporch_byte;
- unsigned int horizontal_frontporch_byte;
- unsigned int dsi_tmp_buf_bpp;
+ u32 horizontal_sync_active_byte;
+ u32 horizontal_backporch_byte;
+ u32 horizontal_frontporch_byte;
+ u32 dsi_tmp_buf_bpp;
struct videomode *vm = &dsi->vm;
@@ -431,7 +428,7 @@ static void dsi_config_vdo_timing(struct mtk_dsi *dsi)
writel(horizontal_backporch_byte, dsi->regs + DSI_HBP_WC);
writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC);
- dsi_ps_control(dsi);
+ mtk_dsi_ps_control(dsi);
}
static void mtk_dsi_start(struct mtk_dsi *dsi)
@@ -448,8 +445,8 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
if (--dsi->refcount != 0)
return;
- dsi_lane0_ulp_mode_enter(dsi);
- dsi_clk_ulp_mode_enter(dsi);
+ mtk_dsi_lane0_ulp_mode_enter(dsi);
+ mtk_dsi_clk_ulp_mode_enter(dsi);
mtk_dsi_disable(dsi);
@@ -479,18 +476,18 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
return;
}
- dsi_rxtx_control(dsi);
+ mtk_dsi_rxtx_control(dsi);
- dsi_clk_ulp_mode_leave(dsi);
- dsi_lane0_ulp_mode_leave(dsi);
- dsi_clk_hs_mode(dsi, 0);
- dsi_set_mode(dsi);
+ mtk_dsi_clk_ulp_mode_leave(dsi);
+ mtk_dsi_lane0_ulp_mode_leave(dsi);
+ mtk_dsi_clk_hs_mode(dsi, 0);
+ mtk_dsi_set_mode(dsi);
- dsi_ps_control_vact(dsi);
- dsi_config_vdo_timing(dsi);
+ mtk_dsi_ps_control_vact(dsi);
+ mtk_dsi_config_vdo_timing(dsi);
- dsi_set_mode(dsi);
- dsi_clk_hs_mode(dsi, 1);
+ mtk_dsi_set_mode(dsi);
+ mtk_dsi_clk_hs_mode(dsi, 1);
mtk_dsi_start(dsi);
diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
index 935a8ef..108d31a 100644
--- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
+++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
@@ -134,7 +134,7 @@ struct mtk_mipitx_data {
struct mtk_mipi_tx {
struct device *dev;
void __iomem *regs;
- unsigned int data_rate;
+ u32 data_rate;
const struct mtk_mipitx_data *driver_data;
struct clk_hw pll_hw;
struct clk *pll;
@@ -172,7 +172,7 @@ static void mtk_mipi_tx_update_bits(struct mtk_mipi_tx *mipi_tx, u32 offset,
static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
{
struct mtk_mipi_tx *mipi_tx = mtk_mipi_tx_from_clk_hw(hw);
- unsigned int txdiv, txdiv0, txdiv1;
+ u8 txdiv, txdiv0, txdiv1;
u64 pcw;
dev_dbg(mipi_tx->dev, "prepare: %u Hz\n", mipi_tx->data_rate);
@@ -326,7 +326,7 @@ static unsigned long mtk_mipi_tx_pll_recalc_rate(struct clk_hw *hw,
static int mtk_mipi_tx_power_on_signal(struct phy *phy)
{
struct mtk_mipi_tx *mipi_tx = phy_get_drvdata(phy);
- unsigned int reg;
+ u32 reg;
for (reg = MIPITX_DSI_CLOCK_LANE;
reg <= MIPITX_DSI_DATA_LANE3; reg += 4)
@@ -357,7 +357,7 @@ static int mtk_mipi_tx_power_on(struct phy *phy)
static void mtk_mipi_tx_power_off_signal(struct phy *phy)
{
struct mtk_mipi_tx *mipi_tx = phy_get_drvdata(phy);
- unsigned int reg;
+ u32 reg;
mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_TOP_CON,
RG_DSI_PAD_TIE_LOW_EN);
--
1.9.1
^ permalink raw reply related
* [PATCH v9 07/10] drm/mediatek: add dsi interrupt control
From: YT Shen @ 2016-11-11 11:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478865346-19043-1-git-send-email-yt.shen@mediatek.com>
From: shaoming chen <shaoming.chen@mediatek.com>
add dsi interrupt control
Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 93 ++++++++++++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 4efeb38..e5832837 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -18,6 +18,7 @@
#include <drm/drm_panel.h>
#include <linux/clk.h>
#include <linux/component.h>
+#include <linux/irq.h>
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/of_graph.h>
@@ -29,6 +30,16 @@
#define DSI_START 0x00
+#define DSI_INTEN 0x08
+
+#define DSI_INTSTA 0x0c
+#define LPRX_RD_RDY_INT_FLAG BIT(0)
+#define CMD_DONE_INT_FLAG BIT(1)
+#define TE_RDY_INT_FLAG BIT(2)
+#define VM_DONE_INT_FLAG BIT(3)
+#define EXT_TE_RDY_INT_FLAG BIT(4)
+#define DSI_BUSY BIT(31)
+
#define DSI_CON_CTRL 0x10
#define DSI_RESET BIT(0)
#define DSI_EN BIT(1)
@@ -71,6 +82,9 @@
#define DSI_HSTX_CKL_WC 0x64
+#define DSI_RACK 0x84
+#define RACK BIT(0)
+
#define DSI_PHY_LCCON 0x104
#define LC_HS_TX_EN BIT(0)
#define LC_ULPM_EN BIT(1)
@@ -131,6 +145,8 @@ struct mtk_dsi {
struct videomode vm;
int refcount;
bool enabled;
+ u32 irq_data;
+ wait_queue_head_t irq_wait_queue;
};
static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
@@ -437,6 +453,64 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
writel(1, dsi->regs + DSI_START);
}
+static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
+{
+ u32 inten = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
+
+ writel(inten, dsi->regs + DSI_INTEN);
+}
+
+static void mtk_dsi_irq_data_set(struct mtk_dsi *dsi, u32 irq_bit)
+{
+ dsi->irq_data |= irq_bit;
+}
+
+static __maybe_unused void mtk_dsi_irq_data_clear(struct mtk_dsi *dsi, u32 irq_bit)
+{
+ dsi->irq_data &= ~irq_bit;
+}
+
+static __maybe_unused s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
+ unsigned int timeout)
+{
+ s32 ret = 0;
+ unsigned long jiffies = msecs_to_jiffies(timeout);
+
+ ret = wait_event_interruptible_timeout(dsi->irq_wait_queue,
+ dsi->irq_data & irq_flag,
+ jiffies);
+ if (ret == 0) {
+ dev_info(dsi->dev, "Wait DSI IRQ(0x%08x) Timeout\n", irq_flag);
+
+ mtk_dsi_enable(dsi);
+ mtk_dsi_reset_engine(dsi);
+ }
+
+ return ret;
+}
+
+static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
+{
+ struct mtk_dsi *dsi = dev_id;
+ u32 status, tmp;
+ u32 flag = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
+
+ status = readl(dsi->regs + DSI_INTSTA) & flag;
+
+ if (status) {
+ do {
+ mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK);
+ tmp = readl(dsi->regs + DSI_INTSTA);
+ } while (tmp & DSI_BUSY);
+
+ mtk_dsi_mask(dsi, DSI_INTSTA, status, 0);
+ mtk_dsi_irq_data_set(dsi, status);
+ wake_up_interruptible(&dsi->irq_wait_queue);
+ }
+
+ return IRQ_HANDLED;
+}
+
static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
{
if (WARN_ON(dsi->refcount == 0))
@@ -485,6 +559,7 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
mtk_dsi_ps_control_vact(dsi);
mtk_dsi_config_vdo_timing(dsi);
+ mtk_dsi_set_interrupt_enable(dsi);
mtk_dsi_set_mode(dsi);
mtk_dsi_clk_hs_mode(dsi, 1);
@@ -793,6 +868,7 @@ static int mtk_dsi_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct device_node *remote_node, *endpoint;
struct resource *regs;
+ int irq_num;
int comp_id;
int ret;
@@ -869,6 +945,23 @@ static int mtk_dsi_probe(struct platform_device *pdev)
return ret;
}
+ irq_num = platform_get_irq(pdev, 0);
+ if (irq_num < 0) {
+ dev_err(&pdev->dev, "failed to request dsi irq resource\n");
+ return -EPROBE_DEFER;
+ }
+
+ irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW);
+ ret = devm_request_irq(&pdev->dev, irq_num, mtk_dsi_irq,
+ IRQF_TRIGGER_LOW, dev_name(&pdev->dev), dsi);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to request mediatek dsi irq\n");
+ return -EPROBE_DEFER;
+ }
+ dev_info(dev, "dsi irq num is 0x%x\n", irq_num);
+
+ init_waitqueue_head(&dsi->irq_wait_queue);
+
platform_set_drvdata(pdev, dsi);
return component_add(&pdev->dev, &mtk_dsi_component_ops);
--
1.9.1
^ permalink raw reply related
* [PATCH v9 08/10] drm/mediatek: add dsi transfer function
From: YT Shen @ 2016-11-11 11:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478865346-19043-1-git-send-email-yt.shen@mediatek.com>
From: shaoming chen <shaoming.chen@mediatek.com>
add dsi read/write commands for transfer function
Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 168 ++++++++++++++++++++++++++++++++++++-
1 file changed, 166 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index e5832837..860b84f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -24,6 +24,7 @@
#include <linux/of_graph.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
+#include <video/mipi_display.h>
#include <video/videomode.h>
#include "mtk_drm_ddp_comp.h"
@@ -80,8 +81,16 @@
#define DSI_HBP_WC 0x54
#define DSI_HFP_WC 0x58
+#define DSI_CMDQ_SIZE 0x60
+#define CMDQ_SIZE 0x3f
+
#define DSI_HSTX_CKL_WC 0x64
+#define DSI_RX_DATA0 0x74
+#define DSI_RX_DATA1 0x78
+#define DSI_RX_DATA2 0x7c
+#define DSI_RX_DATA3 0x80
+
#define DSI_RACK 0x84
#define RACK BIT(0)
@@ -117,8 +126,23 @@
#define CLK_HS_POST (0xff << 8)
#define CLK_HS_EXIT (0xff << 16)
+#define DSI_CMDQ0 0x180
+#define CONFIG (0xff << 0)
+#define SHORT_PACKET 0
+#define LONG_PACKET 2
+#define BTA BIT(2)
+#define DATA_ID (0xff << 8)
+#define DATA_0 (0xff << 16)
+#define DATA_1 (0xff << 24)
+
#define NS_TO_CYCLE(n, c) ((n) / (c) + (((n) % (c)) ? 1 : 0))
+#define MTK_DSI_HOST_IS_READ(type) \
+ ((type == MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM) || \
+ (type == MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM) || \
+ (type == MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM) || \
+ (type == MIPI_DSI_DCS_READ))
+
struct phy;
struct mtk_dsi {
@@ -465,12 +489,12 @@ static void mtk_dsi_irq_data_set(struct mtk_dsi *dsi, u32 irq_bit)
dsi->irq_data |= irq_bit;
}
-static __maybe_unused void mtk_dsi_irq_data_clear(struct mtk_dsi *dsi, u32 irq_bit)
+static void mtk_dsi_irq_data_clear(struct mtk_dsi *dsi, u32 irq_bit)
{
dsi->irq_data &= ~irq_bit;
}
-static __maybe_unused s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
+static s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
unsigned int timeout)
{
s32 ret = 0;
@@ -807,9 +831,149 @@ static int mtk_dsi_host_detach(struct mipi_dsi_host *host,
return 0;
}
+static void mtk_dsi_wait_for_idle(struct mtk_dsi *dsi)
+{
+ u32 timeout_ms = 500000; /* total 1s ~ 2s timeout */
+
+ while (timeout_ms--) {
+ if (!(readl(dsi->regs + DSI_INTSTA) & DSI_BUSY))
+ break;
+
+ usleep_range(2, 4);
+ }
+
+ if (timeout_ms == 0) {
+ dev_info(dsi->dev, "polling dsi wait not busy timeout!\n");
+
+ mtk_dsi_enable(dsi);
+ mtk_dsi_reset_engine(dsi);
+ }
+}
+
+static u32 mtk_dsi_recv_cnt(u8 type, u8 *read_data)
+{
+ switch (type) {
+ case MIPI_DSI_RX_GENERIC_SHORT_READ_RESPONSE_1BYTE:
+ case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_1BYTE:
+ return 1;
+ case MIPI_DSI_RX_GENERIC_SHORT_READ_RESPONSE_2BYTE:
+ case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_2BYTE:
+ return 2;
+ case MIPI_DSI_RX_GENERIC_LONG_READ_RESPONSE:
+ case MIPI_DSI_RX_DCS_LONG_READ_RESPONSE:
+ return read_data[1] + read_data[2] * 16;
+ case MIPI_DSI_RX_ACKNOWLEDGE_AND_ERROR_REPORT:
+ DRM_INFO("type is 0x02, try again\n");
+ break;
+ default:
+ DRM_INFO("type(0x%x) cannot be non-recognite\n", type);
+ break;
+ }
+
+ return 0;
+}
+
+static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
+{
+ const char *tx_buf = msg->tx_buf;
+ u8 config, cmdq_size, cmdq_off, type = msg->type;
+ u32 reg_val, cmdq_mask, i;
+
+ if (MTK_DSI_HOST_IS_READ(type))
+ config = BTA;
+ else
+ config = (msg->tx_len > 2) ? LONG_PACKET : SHORT_PACKET;
+
+ if (msg->tx_len > 2) {
+ cmdq_size = 1 + (msg->tx_len + 3) / 4;
+ cmdq_off = 4;
+ cmdq_mask = CONFIG | DATA_ID | DATA_0 | DATA_1;
+ reg_val = (msg->tx_len << 16) | (type << 8) | config;
+ } else {
+ cmdq_size = 1;
+ cmdq_off = 2;
+ cmdq_mask = CONFIG | DATA_ID;
+ reg_val = (type << 8) | config;
+ }
+
+ for (i = 0; i < msg->tx_len; i++)
+ writeb(tx_buf[i], dsi->regs + DSI_CMDQ0 + cmdq_off + i);
+
+ mtk_dsi_mask(dsi, DSI_CMDQ0, cmdq_mask, reg_val);
+ mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
+}
+
+static ssize_t mtk_dsi_host_send_cmd(struct mtk_dsi *dsi,
+ const struct mipi_dsi_msg *msg, u8 flag)
+{
+ mtk_dsi_wait_for_idle(dsi);
+ mtk_dsi_irq_data_clear(dsi, flag);
+ mtk_dsi_cmdq(dsi, msg);
+ mtk_dsi_start(dsi);
+
+ if (!mtk_dsi_wait_for_irq_done(dsi, flag, 2000))
+ return -1;
+ else
+ return 0;
+}
+
+static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
+ const struct mipi_dsi_msg *msg)
+{
+ struct mtk_dsi *dsi = host_to_dsi(host);
+ u32 recv_cnt, i;
+ u8 read_data[16];
+ void *src_addr;
+ u8 irq_flag = CMD_DONE_INT_FLAG;
+
+ if (readl(dsi->regs + DSI_MODE_CTRL) & MODE) {
+ dev_info(dsi->dev, "dsi engine is not command mode\n");
+ return -1;
+ }
+
+ if (MTK_DSI_HOST_IS_READ(msg->type))
+ irq_flag |= LPRX_RD_RDY_INT_FLAG;
+
+ if (mtk_dsi_host_send_cmd(dsi, msg, irq_flag) < 0)
+ return -1;
+
+ if (!MTK_DSI_HOST_IS_READ(msg->type))
+ return 0;
+
+ if (!msg->rx_buf) {
+ dev_info(dsi->dev, "dsi receive buffer size may be NULL\n");
+ return -1;
+ }
+
+ for (i = 0; i < 16; i++)
+ *(read_data + i) = readb(dsi->regs + DSI_RX_DATA0 + i);
+
+ recv_cnt = mtk_dsi_recv_cnt(read_data[0], read_data);
+
+ if (recv_cnt > 2)
+ src_addr = &read_data[4];
+ else
+ src_addr = &read_data[1];
+
+ if (recv_cnt > 10)
+ recv_cnt = 10;
+
+ if (recv_cnt > msg->rx_len)
+ recv_cnt = msg->rx_len;
+
+ if (recv_cnt)
+ memcpy(msg->rx_buf, src_addr, recv_cnt);
+
+ dev_info(dsi->dev, "dsi get %d byte data from the panel address(0x%x)\n",
+ recv_cnt, *((u8 *)(msg->tx_buf)));
+
+ return recv_cnt;
+}
+
static const struct mipi_dsi_host_ops mtk_dsi_ops = {
.attach = mtk_dsi_host_attach,
.detach = mtk_dsi_host_detach,
+ .transfer = mtk_dsi_host_transfer,
};
static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
--
1.9.1
^ permalink raw reply related
* [PATCH v9 09/10] drm/mediatek: update DSI sub driver flow for sending commands to panel
From: YT Shen @ 2016-11-11 11:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478865346-19043-1-git-send-email-yt.shen@mediatek.com>
This patch update enable/disable flow of DSI module and MIPI TX module.
Original flow works on there is a bridge chip: DSI -> bridge -> panel.
In this case: DSI -> panel, the DSI sub driver flow should be updated.
We need to initialize DSI first so that we can send commands to panel.
Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 110 ++++++++++++++++++++++++++-------
drivers/gpu/drm/mediatek/mtk_mipi_tx.c | 32 +++++-----
2 files changed, 103 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 860b84f..12a1206 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -94,6 +94,8 @@
#define DSI_RACK 0x84
#define RACK BIT(0)
+#define DSI_MEM_CONTI 0x90
+
#define DSI_PHY_LCCON 0x104
#define LC_HS_TX_EN BIT(0)
#define LC_ULPM_EN BIT(1)
@@ -126,6 +128,10 @@
#define CLK_HS_POST (0xff << 8)
#define CLK_HS_EXIT (0xff << 16)
+#define DSI_VM_CMD_CON 0x130
+#define VM_CMD_EN BIT(0)
+#define TS_VFP_EN BIT(5)
+
#define DSI_CMDQ0 0x180
#define CONFIG (0xff << 0)
#define SHORT_PACKET 0
@@ -219,12 +225,12 @@ static void mtk_dsi_phy_timconfig(struct mtk_dsi *dsi)
writel(timcon3, dsi->regs + DSI_PHY_TIMECON3);
}
-static void mtk_dsi_enable(struct mtk_dsi *dsi)
+static void mtk_dsi_engine_enable(struct mtk_dsi *dsi)
{
mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, DSI_EN);
}
-static void mtk_dsi_disable(struct mtk_dsi *dsi)
+static void mtk_dsi_engine_disable(struct mtk_dsi *dsi)
{
mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, 0);
}
@@ -249,7 +255,9 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
* mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
* we set mipi_ratio is 1.05.
*/
- dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
+ dsi->data_rate = dsi->vm.pixelclock * 12 * 21;
+ dsi->data_rate /= (dsi->lanes * 1000 * 10);
+ dev_info(dev, "set mipitx's data rate: %dMHz\n", dsi->data_rate);
ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
if (ret < 0) {
@@ -271,7 +279,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
goto err_disable_engine_clk;
}
- mtk_dsi_enable(dsi);
+ mtk_dsi_engine_enable(dsi);
mtk_dsi_reset_engine(dsi);
mtk_dsi_phy_timconfig(dsi);
@@ -289,7 +297,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
static void mtk_dsi_clk_ulp_mode_enter(struct mtk_dsi *dsi)
{
mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
- mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
+ mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, LC_ULPM_EN);
}
static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
@@ -302,7 +310,7 @@ static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
static void mtk_dsi_lane0_ulp_mode_enter(struct mtk_dsi *dsi)
{
mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_HS_TX_EN, 0);
- mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
+ mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, LD0_ULPM_EN);
}
static void mtk_dsi_lane0_ulp_mode_leave(struct mtk_dsi *dsi)
@@ -338,11 +346,21 @@ static void mtk_dsi_set_mode(struct mtk_dsi *dsi)
if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) &&
!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE))
vid_mode = BURST_MODE;
+ else
+ vid_mode = SYNC_EVENT_MODE;
}
writel(vid_mode, dsi->regs + DSI_MODE_CTRL);
}
+static void mtk_dsi_set_vm_cmd(struct mtk_dsi *dsi)
+{
+ writel(0x3c, dsi->regs + DSI_MEM_CONTI);
+
+ mtk_dsi_mask(dsi, DSI_VM_CMD_CON, VM_CMD_EN, VM_CMD_EN);
+ mtk_dsi_mask(dsi, DSI_VM_CMD_CON, TS_VFP_EN, TS_VFP_EN);
+}
+
static void mtk_dsi_ps_control_vact(struct mtk_dsi *dsi)
{
struct videomode *vm = &dsi->vm;
@@ -399,6 +417,9 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
break;
}
+ tmp_reg |= (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) << 6;
+ tmp_reg |= (dsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET) >> 3;
+
writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
}
@@ -477,6 +498,16 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
writel(1, dsi->regs + DSI_START);
}
+static void mtk_dsi_stop(struct mtk_dsi *dsi)
+{
+ writel(0, dsi->regs + DSI_START);
+}
+
+static void mtk_dsi_set_cmd_mode(struct mtk_dsi *dsi)
+{
+ writel(CMD_MODE, dsi->regs + DSI_MODE_CTRL);
+}
+
static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
{
u32 inten = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
@@ -506,7 +537,7 @@ static s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
if (ret == 0) {
dev_info(dsi->dev, "Wait DSI IRQ(0x%08x) Timeout\n", irq_flag);
- mtk_dsi_enable(dsi);
+ mtk_dsi_engine_enable(dsi);
mtk_dsi_reset_engine(dsi);
}
@@ -535,6 +566,17 @@ static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static s32 mtk_dsi_switch_to_cmd_mode(struct mtk_dsi *dsi, u8 irq_flag, u32 t)
+{
+ mtk_dsi_irq_data_clear(dsi, irq_flag);
+ mtk_dsi_set_cmd_mode(dsi);
+
+ if (!mtk_dsi_wait_for_irq_done(dsi, irq_flag, t))
+ return -1;
+ else
+ return 0;
+}
+
static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
{
if (WARN_ON(dsi->refcount == 0))
@@ -543,11 +585,6 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
if (--dsi->refcount != 0)
return;
- mtk_dsi_lane0_ulp_mode_enter(dsi);
- mtk_dsi_clk_ulp_mode_enter(dsi);
-
- mtk_dsi_disable(dsi);
-
clk_disable_unprepare(dsi->engine_clk);
clk_disable_unprepare(dsi->digital_clk);
@@ -561,35 +598,45 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
if (dsi->enabled)
return;
- if (dsi->panel) {
- if (drm_panel_prepare(dsi->panel)) {
- DRM_ERROR("failed to setup the panel\n");
- return;
- }
- }
-
ret = mtk_dsi_poweron(dsi);
if (ret < 0) {
DRM_ERROR("failed to power on dsi\n");
return;
}
+ usleep_range(20000, 21000);
+
mtk_dsi_rxtx_control(dsi);
+ mtk_dsi_phy_timconfig(dsi);
+ mtk_dsi_ps_control_vact(dsi);
+ mtk_dsi_set_vm_cmd(dsi);
+ mtk_dsi_config_vdo_timing(dsi);
+ mtk_dsi_set_interrupt_enable(dsi);
+ mtk_dsi_engine_enable(dsi);
mtk_dsi_clk_ulp_mode_leave(dsi);
mtk_dsi_lane0_ulp_mode_leave(dsi);
mtk_dsi_clk_hs_mode(dsi, 0);
- mtk_dsi_set_mode(dsi);
- mtk_dsi_ps_control_vact(dsi);
- mtk_dsi_config_vdo_timing(dsi);
- mtk_dsi_set_interrupt_enable(dsi);
+ if (dsi->panel) {
+ if (drm_panel_prepare(dsi->panel)) {
+ DRM_ERROR("failed to prepare the panel\n");
+ return;
+ }
+ }
mtk_dsi_set_mode(dsi);
mtk_dsi_clk_hs_mode(dsi, 1);
mtk_dsi_start(dsi);
+ if (dsi->panel) {
+ if (drm_panel_enable(dsi->panel)) {
+ DRM_ERROR("failed to enable the panel\n");
+ return;
+ }
+ }
+
dsi->enabled = true;
}
@@ -605,6 +652,21 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi)
}
}
+ mtk_dsi_stop(dsi);
+ mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500);
+
+ if (dsi->panel) {
+ if (drm_panel_unprepare(dsi->panel)) {
+ DRM_ERROR("failed to unprepare the panel\n");
+ return;
+ }
+ }
+
+ mtk_dsi_reset_engine(dsi);
+ mtk_dsi_lane0_ulp_mode_enter(dsi);
+ mtk_dsi_clk_ulp_mode_enter(dsi);
+ mtk_dsi_engine_disable(dsi);
+
mtk_dsi_poweroff(dsi);
dsi->enabled = false;
@@ -845,7 +907,7 @@ static void mtk_dsi_wait_for_idle(struct mtk_dsi *dsi)
if (timeout_ms == 0) {
dev_info(dsi->dev, "polling dsi wait not busy timeout!\n");
- mtk_dsi_enable(dsi);
+ mtk_dsi_engine_enable(dsi);
mtk_dsi_reset_engine(dsi);
}
}
diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
index 108d31a..34e95c6 100644
--- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
+++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
@@ -177,7 +177,9 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
dev_dbg(mipi_tx->dev, "prepare: %u Hz\n", mipi_tx->data_rate);
- if (mipi_tx->data_rate >= 500000000) {
+ if (mipi_tx->data_rate > 1250000000) {
+ return -EINVAL;
+ } else if (mipi_tx->data_rate >= 500000000) {
txdiv = 1;
txdiv0 = 0;
txdiv1 = 0;
@@ -201,6 +203,10 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
return -EINVAL;
}
+ mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_TOP_CON,
+ RG_DSI_LNT_IMP_CAL_CODE | RG_DSI_LNT_HS_BIAS_EN,
+ (8 << 4) | RG_DSI_LNT_HS_BIAS_EN);
+
mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_BG_CON,
RG_DSI_VOUT_MSK |
RG_DSI_BG_CKEN | RG_DSI_BG_CORE_EN,
@@ -210,24 +216,18 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
usleep_range(30, 100);
- mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_TOP_CON,
- RG_DSI_LNT_IMP_CAL_CODE | RG_DSI_LNT_HS_BIAS_EN,
- (8 << 4) | RG_DSI_LNT_HS_BIAS_EN);
-
- mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_CON,
- RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN);
+ mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_CON,
+ RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN,
+ RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN);
mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_PWR,
RG_DSI_MPPLL_SDM_PWR_ON |
RG_DSI_MPPLL_SDM_ISO_EN,
RG_DSI_MPPLL_SDM_PWR_ON);
- mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
- RG_DSI_MPPLL_PLL_EN);
-
mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
- RG_DSI_MPPLL_TXDIV0 | RG_DSI_MPPLL_TXDIV1 |
- RG_DSI_MPPLL_PREDIV,
+ RG_DSI_MPPLL_PREDIV | RG_DSI_MPPLL_TXDIV0 |
+ RG_DSI_MPPLL_TXDIV1 | RG_DSI_MPPLL_POSDIV,
(txdiv0 << 3) | (txdiv1 << 5));
/*
@@ -242,10 +242,12 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
26000000);
writel(pcw, mipi_tx->regs + MIPITX_DSI_PLL_CON2);
- mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
- RG_DSI_MPPLL_SDM_FRA_EN);
+ mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
+ RG_DSI_MPPLL_SDM_FRA_EN,
+ RG_DSI_MPPLL_SDM_FRA_EN);
- mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_PLL_CON0, RG_DSI_MPPLL_PLL_EN);
+ mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
+ RG_DSI_MPPLL_PLL_EN, RG_DSI_MPPLL_PLL_EN);
usleep_range(20, 100);
--
1.9.1
^ permalink raw reply related
* [PATCH v9 10/10] drm/mediatek: add support for Mediatek SoC MT2701
From: YT Shen @ 2016-11-11 11:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478865346-19043-1-git-send-email-yt.shen@mediatek.com>
This patch add support for the Mediatek MT2701 DISP subsystem.
There is only one OVL engine in MT2701.
Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 6 ++++++
drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 ++++++
drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 17 +++++++++++++++++
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 6 ++++++
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 29 +++++++++++++++++++++++++++++
drivers/gpu/drm/mediatek/mtk_dsi.c | 1 +
drivers/gpu/drm/mediatek/mtk_mipi_tx.c | 6 ++++++
7 files changed, 71 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 1139834..d174905 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -286,11 +286,17 @@ static int mtk_disp_ovl_remove(struct platform_device *pdev)
return 0;
}
+static const struct mtk_ddp_comp_driver_data mt2701_ovl_driver_data = {
+ .ovl = {0x0040, 1 << 12, 0}
+};
+
static const struct mtk_ddp_comp_driver_data mt8173_ovl_driver_data = {
.ovl = {0x0f40, 0, 1 << 12}
};
static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
+ { .compatible = "mediatek,mt2701-disp-ovl",
+ .data = &mt2701_ovl_driver_data},
{ .compatible = "mediatek,mt8173-disp-ovl",
.data = &mt8173_ovl_driver_data},
{},
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
index b4225e2..5d363de 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
@@ -226,11 +226,17 @@ static int mtk_disp_rdma_remove(struct platform_device *pdev)
return 0;
}
+static const struct mtk_ddp_comp_driver_data mt2701_rdma_driver_data = {
+ .rdma_fifo_pseudo_size = SZ_4K,
+};
+
static const struct mtk_ddp_comp_driver_data mt8173_rdma_driver_data = {
.rdma_fifo_pseudo_size = SZ_8K,
};
static const struct of_device_id mtk_disp_rdma_driver_dt_match[] = {
+ { .compatible = "mediatek,mt2701-disp-rdma",
+ .data = &mt2701_rdma_driver_data},
{ .compatible = "mediatek,mt8173-disp-rdma",
.data = &mt8173_rdma_driver_data},
{},
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index a9b209c..8130f3d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -60,6 +60,13 @@
#define MT8173_MUTEX_MOD_DISP_PWM1 BIT(24)
#define MT8173_MUTEX_MOD_DISP_OD BIT(25)
+#define MT2701_MUTEX_MOD_DISP_OVL BIT(3)
+#define MT2701_MUTEX_MOD_DISP_WDMA BIT(6)
+#define MT2701_MUTEX_MOD_DISP_COLOR BIT(7)
+#define MT2701_MUTEX_MOD_DISP_BLS BIT(9)
+#define MT2701_MUTEX_MOD_DISP_RDMA0 BIT(10)
+#define MT2701_MUTEX_MOD_DISP_RDMA1 BIT(12)
+
#define MUTEX_SOF_SINGLE_MODE 0
#define MUTEX_SOF_DSI0 1
#define MUTEX_SOF_DSI1 2
@@ -92,6 +99,15 @@ struct mtk_ddp {
const unsigned int *mutex_mod;
};
+static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
+ [DDP_COMPONENT_BLS] = MT2701_MUTEX_MOD_DISP_BLS,
+ [DDP_COMPONENT_COLOR0] = MT2701_MUTEX_MOD_DISP_COLOR,
+ [DDP_COMPONENT_OVL0] = MT2701_MUTEX_MOD_DISP_OVL,
+ [DDP_COMPONENT_RDMA0] = MT2701_MUTEX_MOD_DISP_RDMA0,
+ [DDP_COMPONENT_RDMA1] = MT2701_MUTEX_MOD_DISP_RDMA1,
+ [DDP_COMPONENT_WDMA0] = MT2701_MUTEX_MOD_DISP_WDMA,
+};
+
static const unsigned int mt8173_mutex_mod[DDP_COMPONENT_ID_MAX] = {
[DDP_COMPONENT_AAL] = MT8173_MUTEX_MOD_DISP_AAL,
[DDP_COMPONENT_COLOR0] = MT8173_MUTEX_MOD_DISP_COLOR0,
@@ -390,6 +406,7 @@ static int mtk_ddp_remove(struct platform_device *pdev)
}
static const struct of_device_id ddp_driver_dt_match[] = {
+ { .compatible = "mediatek,mt2701-disp-mutex", .data = mt2701_mutex_mod},
{ .compatible = "mediatek,mt8173-disp-mutex", .data = mt8173_mutex_mod},
{},
};
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index b78b2e6..60d4783 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -265,11 +265,17 @@ struct mtk_ddp_comp_match {
[DDP_COMPONENT_WDMA1] = { MTK_DISP_WDMA, 1, NULL },
};
+static const struct mtk_ddp_comp_driver_data mt2701_color_driver_data = {
+ .color_offset = 0x0f00,
+};
+
static const struct mtk_ddp_comp_driver_data mt8173_color_driver_data = {
.color_offset = 0x0c00,
};
static const struct of_device_id mtk_disp_color_driver_dt_match[] = {
+ { .compatible = "mediatek,mt2701-disp-color",
+ .data = &mt2701_color_driver_data},
{ .compatible = "mediatek,mt8173-disp-color",
.data = &mt8173_color_driver_data},
{},
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 5f9b5e8..f5cb6f0 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -126,6 +126,19 @@ static int mtk_atomic_commit(struct drm_device *drm,
.atomic_commit = mtk_atomic_commit,
};
+static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = {
+ DDP_COMPONENT_OVL0,
+ DDP_COMPONENT_RDMA0,
+ DDP_COMPONENT_COLOR0,
+ DDP_COMPONENT_BLS,
+ DDP_COMPONENT_DSI0,
+};
+
+static const enum mtk_ddp_comp_id mt2701_mtk_ddp_ext[] = {
+ DDP_COMPONENT_RDMA1,
+ DDP_COMPONENT_DPI0,
+};
+
static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
DDP_COMPONENT_OVL0,
DDP_COMPONENT_COLOR0,
@@ -145,6 +158,14 @@ static int mtk_atomic_commit(struct drm_device *drm,
DDP_COMPONENT_DPI0,
};
+static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
+ .main_path = mt2701_mtk_ddp_main,
+ .main_len = ARRAY_SIZE(mt2701_mtk_ddp_main),
+ .ext_path = mt2701_mtk_ddp_ext,
+ .ext_len = ARRAY_SIZE(mt2701_mtk_ddp_ext),
+ .shadow_register = true,
+};
+
static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
.main_path = mt8173_mtk_ddp_main,
.main_len = ARRAY_SIZE(mt8173_mtk_ddp_main),
@@ -340,16 +361,22 @@ static void mtk_drm_unbind(struct device *dev)
};
static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
+ { .compatible = "mediatek,mt2701-disp-ovl", .data = (void *)MTK_DISP_OVL },
{ .compatible = "mediatek,mt8173-disp-ovl", .data = (void *)MTK_DISP_OVL },
+ { .compatible = "mediatek,mt2701-disp-rdma", .data = (void *)MTK_DISP_RDMA },
{ .compatible = "mediatek,mt8173-disp-rdma", .data = (void *)MTK_DISP_RDMA },
{ .compatible = "mediatek,mt8173-disp-wdma", .data = (void *)MTK_DISP_WDMA },
+ { .compatible = "mediatek,mt2701-disp-color", .data = (void *)MTK_DISP_COLOR },
{ .compatible = "mediatek,mt8173-disp-color", .data = (void *)MTK_DISP_COLOR },
{ .compatible = "mediatek,mt8173-disp-aal", .data = (void *)MTK_DISP_AAL},
{ .compatible = "mediatek,mt8173-disp-gamma", .data = (void *)MTK_DISP_GAMMA, },
{ .compatible = "mediatek,mt8173-disp-ufoe", .data = (void *)MTK_DISP_UFOE },
+ { .compatible = "mediatek,mt2701-dsi", .data = (void *)MTK_DSI },
{ .compatible = "mediatek,mt8173-dsi", .data = (void *)MTK_DSI },
{ .compatible = "mediatek,mt8173-dpi", .data = (void *)MTK_DPI },
+ { .compatible = "mediatek,mt2701-disp-mutex", .data = (void *)MTK_DISP_MUTEX },
{ .compatible = "mediatek,mt8173-disp-mutex", .data = (void *)MTK_DISP_MUTEX },
+ { .compatible = "mediatek,mt2701-disp-pwm", .data = (void *)MTK_DISP_BLS },
{ .compatible = "mediatek,mt8173-disp-pwm", .data = (void *)MTK_DISP_PWM },
{ .compatible = "mediatek,mt8173-disp-od", .data = (void *)MTK_DISP_OD },
{ }
@@ -522,6 +549,8 @@ static SIMPLE_DEV_PM_OPS(mtk_drm_pm_ops, mtk_drm_sys_suspend,
mtk_drm_sys_resume);
static const struct of_device_id mtk_drm_of_ids[] = {
+ { .compatible = "mediatek,mt2701-mmsys",
+ .data = &mt2701_mmsys_driver_data},
{ .compatible = "mediatek,mt8173-mmsys",
.data = &mt8173_mmsys_driver_data},
{ }
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 12a1206..fe9786c 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -1204,6 +1204,7 @@ static int mtk_dsi_remove(struct platform_device *pdev)
}
static const struct of_device_id mtk_dsi_of_match[] = {
+ { .compatible = "mediatek,mt2701-dsi" },
{ .compatible = "mediatek,mt8173-dsi" },
{ },
};
diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
index 34e95c6..944fb1d 100644
--- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
+++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
@@ -467,11 +467,17 @@ static int mtk_mipi_tx_remove(struct platform_device *pdev)
return 0;
}
+static const struct mtk_mipitx_data mt2701_mipitx_data = {
+ .data = (3 << 8)
+};
+
static const struct mtk_mipitx_data mt8173_mipitx_data = {
.data = (0 << 8)
};
static const struct of_device_id mtk_mipi_tx_match[] = {
+ { .compatible = "mediatek,mt2701-mipi-tx",
+ .data = &mt2701_mipitx_data },
{ .compatible = "mediatek,mt8173-mipi-tx",
.data = &mt8173_mipitx_data },
{},
--
1.9.1
^ permalink raw reply related
* [RESEND PATCH v1 02/11] dt-bindings: hisi: Add Hisilicon HiP05/06/07 Sysctrl and Djtag dts bindings
From: Anurup M @ 2016-11-11 11:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161111115346.GC11945@leverpostej>
On Friday 11 November 2016 05:23 PM, Mark Rutland wrote:
> On Fri, Nov 11, 2016 at 04:49:03PM +0530, Anurup M wrote:
>> On Thursday 10 November 2016 10:53 PM, Mark Rutland wrote:
>>> On Thu, Nov 03, 2016 at 01:41:58AM -0400, Anurup M wrote:
>>>> diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
>>>> +Example:
>>>> + /* for Hisilicon HiP05 djtag for CPU sysctrl */
>>>> + djtag0: djtag at 80010000 {
>>>> + compatible = "hisilicon,hip05-cpu-djtag-v1";
>>>> + reg = <0x0 0x80010000 0x0 0x10000>;
>>>> +
>>>> + /* For L3 cache PMU */
>>>> + pmul3c0 {
>>>> + compatible = "hisilicon,hisi-pmu-l3c-v1";
>>>> + scl-id = <0x02>;
>>>> + num-events = <0x16>;
>>>> + num-counters = <0x08>;
>>>> + module-id = <0x04>;
>>>> + num-banks = <0x04>;
>>>> + cfgen-map = <0x02 0x04 0x01 0x08>;
>>>> + counter-reg = <0x170>;
>>>> + evctrl-reg = <0x04>;
>>>> + event-en = <0x1000000>;
>>>> + evtype-reg = <0x140>;
>>>> + };
>>> This sub-node needs a binding document.
>>>
>>> These properties are completely opaque
>> The L3 cache PMU bindings are defined @bindings/arm/hisilicon/pmu.txt.
>> Is it OK that I document here(hisilicon/djtag.txt), a reference to
>> the PMU bindings doc ?
> At this point in the series, that file does not exist yet, and this is
> an undocumented beinding.
>
> Please introduce this sub-node long with the PMU bindings, later in the
> series.
Thanks, I got your suggestion. Will add this later in series.
> Thanks,
> Mark.
^ permalink raw reply
* PM regression with LED changes in next-20161109
From: Pavel Machek @ 2016-11-11 12:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <80b645e7-c3fa-8001-d9b1-c3c8c40394fd@gmail.com>
On Thu 2016-11-10 22:34:07, Jacek Anaszewski wrote:
> Hi,
>
> On 11/10/2016 09:29 PM, Pavel Machek wrote:
> >On Thu 2016-11-10 10:55:37, Tony Lindgren wrote:
> >>* Pavel Machek <pavel@ucw.cz> [161110 09:29]:
> >>>Hi!
> >>>
> >>>>>>>Looks like commit 883d32ce3385 ("leds: core: Add support for poll()ing
> >>>>>>>the sysfs brightness attr for changes.") breaks runtime PM for me.
> >>>>>>>
> >>>>>>>On my omap dm3730 based test system, idle power consumption is over 70
> >>>>>>>times higher now with this patch! It goes from about 6mW for the core
> >>>>>>>system to over 440mW during idle meaning there's some busy timer now
> >>>>>>>active.
> >>>>>>>
> >>>>>>>Reverting this patch fixes the issue. Any ideas?
> >>>
> >>>Are you using any LED that toggles with high frequency? Like perhaps
> >>>LED that is lit when CPU is active?
> >>
> >>Yeah one of them seems to have cpu0 as the default trigger.
> >
> >Aha. Its quite obvious we don't want to notify sysfs each time that
> >one is toggled, right?
> >
> >IMO brightness should display max brightness for the trigger, as Hans
> >suggested, anything else is madness for trigger such as cpu activity.
>
> Are you suggesting that we should revert changes introduced
> by below patch?
>
> commit 29d76dfa29fe22583aefddccda0bc56aa81035dc
> Author: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> Date: Tue Mar 18 09:47:48 2008 +0000
>
> leds: Add support to leds with readable status
>
> Some led hardware allows drivers to query the led state, and this patch
> adds a hook to let the led class take advantage of that information when
> available.
>
> Without this functionality, when access to the led hardware is not
> exclusive (i.e. firmware or hardware might change its state behind the
> kernel's back), reality goes out of sync with the led class' idea of
> what
> the led is doing, which is annoying at best.
Hmm. So userland can read the LED state, and it can get _some_ value
back, but it can not know if it is current state or not.
I don't think that's a good interface. I see it is from 2008... is
someone using it? Maybe it is too late for revert.
But I'd certainly not extend it with poll.
IMO reading/polling should only be available with some triggers. It
does not make sense with "CPU load" trigger. It makes sense with
"keyboard light changeable by hardware" trigger.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161111/cdf0fc1a/attachment.sig>
^ permalink raw reply
* pull request: linux-firmware: Update Mediatek MT8173 VPU firmware
From: Andrew-CT Chen @ 2016-11-11 12:09 UTC (permalink / raw)
To: linux-arm-kernel
Hi linux-firmware maintainers,
The following changes since commit a179db97914da5e650c21ba8f9b0bae04a0f8a41:
amdgpu: update SMC firmware for VI parts (2016-10-05 10:30:11 -0700)
are available in the git repository at:
https://github.com/andrewct-chen/linux_fw_vpu_v1.0.3.git v1.0.3
for you to fetch changes up to f52fd5b4f156bd1eef672d29abbdc57cf310ac1b:
mediatek: update MT8173 VPU firmware for idle state (2016-11-11 19:34:56 +0800)
----------------------------------------------------------------
Andrew-CT Chen (1):
mediatek: update MT8173 VPU firmware for idle state
vpu_d.bin | Bin 4082928 -> 4082928 bytes
vpu_p.bin | Bin 131160 -> 131160 bytes
2 files changed, 0 insertions(+), 0 deletions(-)
^ permalink raw reply
* [PATCH] staging: vc04_services: Remove explicit NULL pointer
From: Maninder Singh @ 2016-11-11 12:16 UTC (permalink / raw)
To: linux-arm-kernel
Replace direct comparisons to NULL i.e.
'x == NULL' with '!x'
'x != NULL' with 'x'
Signed-off-by: Maninder Singh <maninder.s2@samsung.com>
---
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 32 ++++++++++----------
.../vc04_services/interface/vchiq_arm/vchiq_core.c | 4 +--
.../vc04_services/interface/vchiq_arm/vchiq_shim.c | 4 +--
.../vc04_services/interface/vchiq_arm/vchiq_util.c | 4 +--
4 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 8fcd940..0068a1c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -533,7 +533,7 @@ struct vchiq_io_copy_callback_context {
/* Remove all services */
i = 0;
while ((service = next_service_by_instance(instance->state,
- instance, &i)) != NULL) {
+ instance, &i))) {
status = vchiq_remove_service(service->handle);
unlock_service(service);
if (status != VCHIQ_SUCCESS)
@@ -614,7 +614,7 @@ struct vchiq_io_copy_callback_context {
&args.params, srvstate,
instance, user_service_free);
- if (service != NULL) {
+ if (service) {
user_service->service = service;
user_service->userdata = userdata;
user_service->instance = instance;
@@ -661,7 +661,7 @@ struct vchiq_io_copy_callback_context {
VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
service = find_service_for_instance(instance, handle);
- if (service != NULL) {
+ if (service) {
USER_SERVICE_T *user_service =
(USER_SERVICE_T *)service->base.userdata;
/* close_pending is false on first entry, and when the
@@ -687,7 +687,7 @@ struct vchiq_io_copy_callback_context {
VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
service = find_service_for_instance(instance, handle);
- if (service != NULL) {
+ if (service) {
USER_SERVICE_T *user_service =
(USER_SERVICE_T *)service->base.userdata;
/* close_pending is false on first entry, and when the
@@ -714,7 +714,7 @@ struct vchiq_io_copy_callback_context {
VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
service = find_service_for_instance(instance, handle);
- if (service != NULL) {
+ if (service) {
status = (cmd == VCHIQ_IOC_USE_SERVICE) ?
vchiq_use_service_internal(service) :
vchiq_release_service_internal(service);
@@ -747,7 +747,7 @@ struct vchiq_io_copy_callback_context {
service = find_service_for_instance(instance, args.handle);
- if ((service != NULL) && (args.count <= MAX_ELEMENTS)) {
+ if (service && (args.count <= MAX_ELEMENTS)) {
/* Copy elements into kernel space */
VCHIQ_ELEMENT_T elements[MAX_ELEMENTS];
if (copy_from_user(elements, args.elements,
@@ -1063,7 +1063,7 @@ struct vchiq_io_copy_callback_context {
spin_unlock(&msg_queue_spinlock);
up(&user_service->remove_event);
- if (header == NULL)
+ if (!header)
ret = -ENOTCONN;
else if (header->size <= args.bufsize) {
/* Copy to user space if msgbuf is not NULL */
@@ -1161,7 +1161,7 @@ struct vchiq_io_copy_callback_context {
VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
service = find_closed_service_for_instance(instance, handle);
- if (service != NULL) {
+ if (service) {
USER_SERVICE_T *user_service =
(USER_SERVICE_T *)service->base.userdata;
close_delivered(user_service);
@@ -1305,7 +1305,7 @@ struct vchiq_io_copy_callback_context {
/* Mark all services for termination... */
i = 0;
while ((service = next_service_by_instance(state, instance,
- &i)) != NULL) {
+ &i))) {
USER_SERVICE_T *user_service = service->base.userdata;
/* Wake the slot handler if the msg queue is full. */
@@ -1317,8 +1317,8 @@ struct vchiq_io_copy_callback_context {
/* ...and wait for them to die */
i = 0;
- while ((service = next_service_by_instance(state, instance, &i))
- != NULL) {
+ while ((service = next_service_by_instance(state,
+ instance, &i))) {
USER_SERVICE_T *user_service = service->base.userdata;
down(&service->remove_event);
@@ -1560,7 +1560,7 @@ struct vchiq_io_copy_callback_context {
num_pages = (offset + num_bytes + PAGE_SIZE - 1) / PAGE_SIZE;
pages = kmalloc(sizeof(struct page *) * num_pages, GFP_KERNEL);
- if (pages == NULL) {
+ if (!pages) {
vchiq_log_error(vchiq_arm_log_level,
"Unable to allocation memory for %d pages\n",
num_pages);
@@ -1592,7 +1592,7 @@ struct vchiq_io_copy_callback_context {
if (page_idx != prev_idx) {
- if (page != NULL)
+ if (page)
kunmap(page);
page = pages[page_idx];
kmapped_virt_ptr = kmap(page);
@@ -1610,7 +1610,7 @@ struct vchiq_io_copy_callback_context {
}
out:
- if (page != NULL)
+ if (page)
kunmap(page);
for (page_idx = 0; page_idx < num_pages; page_idx++)
@@ -2659,7 +2659,7 @@ struct vchiq_io_copy_callback_context {
int use_count = 0, i;
i = 0;
while ((service = next_service_by_instance(instance->state,
- instance, &i)) != NULL) {
+ instance, &i))) {
use_count += service->service_use_count;
unlock_service(service);
}
@@ -2685,7 +2685,7 @@ struct vchiq_io_copy_callback_context {
int i;
i = 0;
while ((service = next_service_by_instance(instance->state,
- instance, &i)) != NULL) {
+ instance, &i))) {
service->trace = trace;
unlock_service(service);
}
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 7440db2..82f6496 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3273,7 +3273,7 @@ static const char *msg_type_str(unsigned int msg_type)
if (!service ||
(service->srvstate != VCHIQ_SRVSTATE_OPEN) ||
- ((memhandle == VCHI_MEM_HANDLE_INVALID) && (offset == NULL)) ||
+ ((memhandle == VCHI_MEM_HANDLE_INVALID) && (!offset)) ||
(vchiq_check_service(service) != VCHIQ_SUCCESS))
goto error_exit;
@@ -3910,7 +3910,7 @@ void vchiq_log_dump_mem(const char *label, uint32_t addr, const void *void_mem,
}
*s++ = '\0';
- if ((label != NULL) && (*label != '\0'))
+ if (label && (*label != '\0'))
vchiq_log_trace(VCHIQ_LOG_TRACE,
"%s: %08x: %s", label, addr, line_buf);
else
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
index d977139..b6d0a75 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
@@ -639,7 +639,7 @@ int32_t vchi_service_open(VCHI_INSTANCE_T instance_handle,
}
}
- return (service != NULL) ? 0 : -1;
+ return (service) ? 0 : -1;
}
EXPORT_SYMBOL(vchi_service_open);
@@ -671,7 +671,7 @@ int32_t vchi_service_create(VCHI_INSTANCE_T instance_handle,
}
}
- return (service != NULL) ? 0 : -1;
+ return (service) ? 0 : -1;
}
EXPORT_SYMBOL(vchi_service_create);
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c
index 384acb8..ac6a386 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c
@@ -52,7 +52,7 @@ int vchiu_queue_init(VCHIU_QUEUE_T *queue, int size)
sema_init(&queue->push, 0);
queue->storage = kzalloc(size * sizeof(VCHIQ_HEADER_T *), GFP_KERNEL);
- if (queue->storage == NULL) {
+ if (!queue->storage) {
vchiu_queue_delete(queue);
return 0;
}
@@ -61,7 +61,7 @@ int vchiu_queue_init(VCHIU_QUEUE_T *queue, int size)
void vchiu_queue_delete(VCHIU_QUEUE_T *queue)
{
- if (queue->storage != NULL)
+ if (queue->storage)
kfree(queue->storage);
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3] spi: spi-fsl-dspi: Add DMA support for Vybrid
From: Mark Brown @ 2016-11-11 12:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <836dd6a84fa149cafdbb0b53e792d305febf6207.1478778329.git.maitysanchayan@gmail.com>
On Thu, Nov 10, 2016 at 05:49:15PM +0530, Sanchayan Maity wrote:
A couple of small things, please send followup patches fixing them.
> + rx_word = is_double_byte_mode(dspi);
> +
> + len = rx_word ? (dma->curr_xfer_len / 2) : dma->curr_xfer_len;
Please use normal if statements, they're much easier to read.
> +err_slave_config:
> + devm_kfree(dev, dma->rx_dma_buf);
> +err_rx_dma_buf:
> + devm_kfree(dev, dma->tx_dma_buf);
You really shouldn't need to explicitly free things like this if you're
using devm_, especially in the error path from the probe function like
this where a failure is just going to result in the device failing to
instantiate so you won't have the allocation sitting around unused for
any length of time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161111/6447812f/attachment.sig>
^ permalink raw reply
* [PATCH v2 2/4] dt-bindings: Add TI SCI PM Domains
From: Ulf Hansson @ 2016-11-11 12:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <90e91588-d2fc-19be-5a28-63c801a8d061@ti.com>
On 10 November 2016 at 20:56, Dave Gerlach <d-gerlach@ti.com> wrote:
> Rob, Ulf, Jon,
>
> On 10/27/2016 08:15 AM, Dave Gerlach wrote:
>>
>> +Jon
>> On 10/26/2016 04:59 PM, Rob Herring wrote:
>>>
>>> On Mon, Oct 24, 2016 at 12:00 PM, Kevin Hilman <khilman@baylibre.com>
>>> wrote:
>>>>
>>>> Dave Gerlach <d-gerlach@ti.com> writes:
>>>>
>>>>> Hi,
>>>>> On 10/21/2016 01:48 PM, Kevin Hilman wrote:
>>>>>>
>>>>>> Dave Gerlach <d-gerlach@ti.com> writes:
>>>>>>
>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>>>>> control device power states.
>>>>>>>
>>>>>>> Also, provide macros representing each device index as understood
>>>>>>> by TI SCI to be used in the device node power-domain references.
>>>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>>>
>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>>>> ---
>>>>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 54
>>>>>>> +++++++++++++
>>>>>>> MAINTAINERS | 2 +
>>>>>>> include/dt-bindings/genpd/k2g.h | 90
>>>>>>> ++++++++++++++++++++++
>>>>>>> 3 files changed, 146 insertions(+)
>>>>>>> create mode 100644
>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..32f38a349656
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> @@ -0,0 +1,54 @@
>>>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>>>> +---------------------------------------------
>>>>>>> +
>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...)
>>>>>>> that is
>>>>>>> +responsible for controlling the state of the IPs that are present.
>>>>>>> +Communication between the host processor running an OS and the
>>>>>>> system
>>>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm
>>>>>>> domain
>>>>>>> +implementation plugs into the generic pm domain framework and makes
>>>>>>> use of
>>>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>>>> +
>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>>> +
>>>>>>> +PM Domain Node
>>>>>>> +==============
>>>>>>> +The PM domain node represents the global PM domain managed by the
>>>>>>> PMMC,
>>>>>>> +which in this case is the single implementation as documented by the
>>>>>>> generic
>>>>>>> +PM domain bindings in
>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>>>>> +
>>>>>>> +Required Properties:
>>>>>>> +--------------------
>>>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>>>> +- #power-domain-cells: Must be 0.
>>>>>>> +- ti,sci: Phandle to the TI SCI device to use for managing the
>>>>>>> devices.
>>>>>>>
>>>>>>> +Example:
>>>>>>> +--------------------
>>>>>>> +k2g_pds: k2g_pds {
>>>>>>
>>>>>>
>>>>>> should use generic name like "power-contoller", e.g. k2g_pds:
>>>>>> power-controller
>>>>>
>>>>>
>>>>> Ok, that makes more sense.
>>>>>
>>>>>>
>>>>>>> + compatible = "ti,sci-pm-domain";
>>>>>>> + #power-domain-cells = <0>;
>>>>>>> + ti,sci = <&pmmc>;
>>>>>>> +};
>>>>>>> +
>>>>>>> +PM Domain Consumers
>>>>>>> +===================
>>>>>>> +Hardware blocks that require SCI control over their state must
>>>>>>> provide
>>>>>>> +a reference to the sci-pm-domain they are part of and a unique
>>>>>>> device
>>>>>>> +specific ID that identifies the device.
>>>>>>> +
>>>>>>> +Required Properties:
>>>>>>> +--------------------
>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain
>>>>>>> node.
>>>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI
>>>>>>> to
>>>>>>> + be used for device control.
>>>>>>
>>>>>>
>>>>>> This ID doesn't look right.
>>>>>>
>>>>>> Why not use #power-domain-cells = <1> and pass the index in the DT?
>>>>>> ...
>>>
>>>
>>> Exactly. ti,sci-id is a NAK for me.
>>
>>
>> I was told not to use the onecell during v1 discussion. I agree this would
>> be
>> ideal but I cannot due to what the bindings represent, the phandle
>> parameter is
>> an index into a list of genpds, whereas we need an actual ID number we can
>> use
>> and I do not have the ability to get that from the phandle.
>>
>> @Ulf/Jon, is there any hope of bringing back custom xlate functions for
>> genpd
>> providers? I don't have a good background on why it was even removed. I
>> can
>> maintain a single genpd for all devices but I need a way to parse this ID,
>> whether it's from a separate property or a phandle. It is locked now to
>> indexing
>> into a list of genpds but I need additional per device information for
>> devices
>> bound to a genpd and I need either a custom parameter or the ability to
>> parse
>> the phandle myself.
>>
>
> Any comments here? The meaning of the phandle onecell is fixed in the genpd
> framework so I'm not sure how we want to move forward with this, I need to
> pass a power domain ID to the genpd driver, and if this shouldn't be a new
> property I'm not sure what direction we should take.
>
> Regards,
> Dave
>
>
>>>
>>>>>>
>>>>>>> +See dt-bindings/genpd/k2g.h for the list of valid identifiers for
>>>>>>> k2g.
>>>>>>> +
>>>>>>> +Example:
>>>>>>> +--------------------
>>>>>>> +uart0: serial at 02530c00 {
>>>>>>> + compatible = "ns16550a";
>>>>>>> + ...
>>>>>>> + power-domains = <&k2g_pds>;
>>>>>>> + ti,sci-id = <K2G_DEV_UART0>;
>>>>>>
>>>>>>
>>>>>> ... like this:
>>>>>>
>>>>>> power-domains = <&k2g_pds K2G_DEV_UART0>;
>>>>>
>>>>>
>>>>> That's how I did it in version one actually. I was able to define my
>>>>> own xlate function to parse the phandle and get that index, but Ulf
>>>>> pointed me to this series by Jon Hunter [1] that simplified genpd
>>>>> providers and dropped the concept of adding your own xlate. This locks
>>>>> the onecell approach to using a fixed static array of genpds that get
>>>>> indexed into (without passing the index to the provider, just the
>>>>> genpd that's looked up), which doesn't fit our usecase, as we don't
>>>>> want a 1 to 1 genpd to device mapping based on the comments provided
>>>>> in v1. Now we just use the genpd device attach/detach hooks to parse
>>>>> the sci-id and then use it in the genpd device start/stop hooks.
>>>
>>>
>>> I have no idea what any of this means. All sounds like driver
>>> architecture, not anything to do with bindings.
>>
>>
>> This was a response to Kevin, not part of binding description.
>>
>>>
>>>>
>>>> Ah, right. I remember now. This approach allows you to use a single
>>>> genpd as discussed earlier.
>>>>
>>>> Makes sense now, suggestion retracted.
>>>
>>>
>>> IIRC, the bindings in Jon's case had a node for each domain and didn't
>>> need any additional property.
>>
>>
>> Yes but we only have one domain and index into it, not into a list of
>> domains,
Exactly. And this my main point as well. We are not talking about a
domain property but a device property.
>> so the additional property is solving a different problem.
Yes.
Perhaps you could try to elaborate about what the TI SCI ID really
represents for the device, as to help Rob understand the bigger
picture?
To me, the TI SCI ID, is similar to a "conid" for any another "device
resource" (like clock, pinctrl, regulator etc) which we can describe
in DT and assign to a device node. The only difference here, is that
we don't have common API to fetch the resource (like clk_get(),
regulator_get()), but instead we fetches the device's resource from
SoC specific code, via genpd's device ->attach() callback.
Hope that helps.
Kind regards
Uffe
^ permalink raw reply
* [PATCH v7 00/16] ACPI IORT ARM SMMU support
From: Hanjun Guo @ 2016-11-11 12:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161109141948.19244-1-lorenzo.pieralisi@arm.com>
On 11/09/2016 10:19 PM, Lorenzo Pieralisi wrote:
> This patch series is v7 of a previous posting:
>
> https://lkml.org/lkml/2016/10/18/506
>
> v6 -> v7
> - Rebased against v4.9-rc4
> - Fixed IORT probing on ACPI systems with missing IORT table
> - Fixed SMMUv1/v2 global interrupt detection
> - Updated iommu_ops firmware look-up
Although no major update for the new version, I tested
this patchset again on hisilicon d03, and the platform
device works as before with SMMUv3.
Thanks
Hanjun
^ permalink raw reply
* [PATCH] arm64: dts: Add ARM PMU node for exynos7
From: Alim Akhtar @ 2016-11-11 13:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4922ada9-2eb2-f0cf-e4b1-a74f4397f97c@arm.com>
Hi Robin,
On 11/10/2016 07:07 PM, Robin Murphy wrote:
> Hi Alim,
>
> On 10/11/16 03:30, Alim Akhtar wrote:
>> This patch adds ARM Performance Monitor Unit dt node for exynos7.
>> PMU provides various statistics on the operation of the CPU and
>> memory system at runtime, which are very useful when debugging or
>> profiling code. This enables the same.
>>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>> arch/arm64/boot/dts/exynos/exynos7.dtsi | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi b/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> index e0d0d01..53ce4be 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> @@ -472,6 +472,14 @@
>> status = "disabled";
>> };
>>
>> + arm-pmu {
>> + compatible = "arm,cortex-a57-pmu", "arm,armv8-pmuv3";
>> + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
>
> Per Documentation/devicetree/bindings/arm/pmu.txt there should also be
> an "interrupt-affinity" property describing which SPI belongs to which core.
>
Thanks for review, will resend after adding "interrupt-affinity" property.
> Robin.
>
>> + };
>> +
>> timer {
>> compatible = "arm,armv8-timer";
>> interrupts = <GIC_PPI 13
>>
>
>
>
>
^ permalink raw reply
* Applied "spi: spi-fsl-dspi: Add DMA support for Vybrid" to the spi tree
From: Mark Brown @ 2016-11-11 13:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <836dd6a84fa149cafdbb0b53e792d305febf6207.1478778329.git.maitysanchayan@gmail.com>
The patch
spi: spi-fsl-dspi: Add DMA support for Vybrid
has been applied to the spi tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 90ba37033cb94207e97c4ced9be575770438213b Mon Sep 17 00:00:00 2001
From: Sanchayan Maity <maitysanchayan@gmail.com>
Date: Thu, 10 Nov 2016 17:49:15 +0530
Subject: [PATCH] spi: spi-fsl-dspi: Add DMA support for Vybrid
Add DMA support for Vybrid.
Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi-fsl-dspi.c | 301 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 300 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 35c0dd945668..bc64700b514d 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -15,6 +15,8 @@
#include <linux/clk.h>
#include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
#include <linux/err.h>
#include <linux/errno.h>
#include <linux/interrupt.h>
@@ -40,6 +42,7 @@
#define TRAN_STATE_WORD_ODD_NUM 0x04
#define DSPI_FIFO_SIZE 4
+#define DSPI_DMA_BUFSIZE (DSPI_FIFO_SIZE * 1024)
#define SPI_MCR 0x00
#define SPI_MCR_MASTER (1 << 31)
@@ -71,6 +74,11 @@
#define SPI_SR_EOQF 0x10000000
#define SPI_SR_TCFQF 0x80000000
+#define SPI_RSER_TFFFE BIT(25)
+#define SPI_RSER_TFFFD BIT(24)
+#define SPI_RSER_RFDFE BIT(17)
+#define SPI_RSER_RFDFD BIT(16)
+
#define SPI_RSER 0x30
#define SPI_RSER_EOQFE 0x10000000
#define SPI_RSER_TCFQE 0x80000000
@@ -108,6 +116,8 @@
#define SPI_TCR_TCNT_MAX 0x10000
+#define DMA_COMPLETION_TIMEOUT msecs_to_jiffies(3000)
+
struct chip_data {
u32 mcr_val;
u32 ctar_val;
@@ -117,6 +127,7 @@ struct chip_data {
enum dspi_trans_mode {
DSPI_EOQ_MODE = 0,
DSPI_TCFQ_MODE,
+ DSPI_DMA_MODE,
};
struct fsl_dspi_devtype_data {
@@ -125,7 +136,7 @@ struct fsl_dspi_devtype_data {
};
static const struct fsl_dspi_devtype_data vf610_data = {
- .trans_mode = DSPI_EOQ_MODE,
+ .trans_mode = DSPI_DMA_MODE,
.max_clock_factor = 2,
};
@@ -139,6 +150,22 @@ static const struct fsl_dspi_devtype_data ls2085a_data = {
.max_clock_factor = 8,
};
+struct fsl_dspi_dma {
+ u32 curr_xfer_len;
+
+ u32 *tx_dma_buf;
+ struct dma_chan *chan_tx;
+ dma_addr_t tx_dma_phys;
+ struct completion cmd_tx_complete;
+ struct dma_async_tx_descriptor *tx_desc;
+
+ u32 *rx_dma_buf;
+ struct dma_chan *chan_rx;
+ dma_addr_t rx_dma_phys;
+ struct completion cmd_rx_complete;
+ struct dma_async_tx_descriptor *rx_desc;
+};
+
struct fsl_dspi {
struct spi_master *master;
struct platform_device *pdev;
@@ -165,6 +192,7 @@ struct fsl_dspi {
u32 waitflags;
u32 spi_tcnt;
+ struct fsl_dspi_dma *dma;
};
static inline int is_double_byte_mode(struct fsl_dspi *dspi)
@@ -176,6 +204,263 @@ static inline int is_double_byte_mode(struct fsl_dspi *dspi)
return ((val & SPI_FRAME_BITS_MASK) == SPI_FRAME_BITS(8)) ? 0 : 1;
}
+static void dspi_tx_dma_callback(void *arg)
+{
+ struct fsl_dspi *dspi = arg;
+ struct fsl_dspi_dma *dma = dspi->dma;
+
+ complete(&dma->cmd_tx_complete);
+}
+
+static void dspi_rx_dma_callback(void *arg)
+{
+ struct fsl_dspi *dspi = arg;
+ struct fsl_dspi_dma *dma = dspi->dma;
+ int rx_word;
+ int i, len;
+ u16 d;
+
+ rx_word = is_double_byte_mode(dspi);
+
+ len = rx_word ? (dma->curr_xfer_len / 2) : dma->curr_xfer_len;
+
+ if (!(dspi->dataflags & TRAN_STATE_RX_VOID)) {
+ for (i = 0; i < len; i++) {
+ d = dspi->dma->rx_dma_buf[i];
+ rx_word ? (*(u16 *)dspi->rx = d) :
+ (*(u8 *)dspi->rx = d);
+ dspi->rx += rx_word + 1;
+ }
+ }
+
+ complete(&dma->cmd_rx_complete);
+}
+
+static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
+{
+ struct fsl_dspi_dma *dma = dspi->dma;
+ struct device *dev = &dspi->pdev->dev;
+ int time_left;
+ int tx_word;
+ int i, len;
+ u16 val;
+
+ tx_word = is_double_byte_mode(dspi);
+
+ len = tx_word ? (dma->curr_xfer_len / 2) : dma->curr_xfer_len;
+
+ for (i = 0; i < len - 1; i++) {
+ val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
+ dspi->dma->tx_dma_buf[i] =
+ SPI_PUSHR_TXDATA(val) | SPI_PUSHR_PCS(dspi->cs) |
+ SPI_PUSHR_CTAS(0) | SPI_PUSHR_CONT;
+ dspi->tx += tx_word + 1;
+ }
+
+ val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
+ dspi->dma->tx_dma_buf[i] = SPI_PUSHR_TXDATA(val) |
+ SPI_PUSHR_PCS(dspi->cs) |
+ SPI_PUSHR_CTAS(0);
+ dspi->tx += tx_word + 1;
+
+ dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
+ dma->tx_dma_phys,
+ DSPI_DMA_BUFSIZE, DMA_MEM_TO_DEV,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ if (!dma->tx_desc) {
+ dev_err(dev, "Not able to get desc for DMA xfer\n");
+ return -EIO;
+ }
+
+ dma->tx_desc->callback = dspi_tx_dma_callback;
+ dma->tx_desc->callback_param = dspi;
+ if (dma_submit_error(dmaengine_submit(dma->tx_desc))) {
+ dev_err(dev, "DMA submit failed\n");
+ return -EINVAL;
+ }
+
+ dma->rx_desc = dmaengine_prep_slave_single(dma->chan_rx,
+ dma->rx_dma_phys,
+ DSPI_DMA_BUFSIZE, DMA_DEV_TO_MEM,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ if (!dma->rx_desc) {
+ dev_err(dev, "Not able to get desc for DMA xfer\n");
+ return -EIO;
+ }
+
+ dma->rx_desc->callback = dspi_rx_dma_callback;
+ dma->rx_desc->callback_param = dspi;
+ if (dma_submit_error(dmaengine_submit(dma->rx_desc))) {
+ dev_err(dev, "DMA submit failed\n");
+ return -EINVAL;
+ }
+
+ reinit_completion(&dspi->dma->cmd_rx_complete);
+ reinit_completion(&dspi->dma->cmd_tx_complete);
+
+ dma_async_issue_pending(dma->chan_rx);
+ dma_async_issue_pending(dma->chan_tx);
+
+ time_left = wait_for_completion_timeout(&dspi->dma->cmd_tx_complete,
+ DMA_COMPLETION_TIMEOUT);
+ if (time_left == 0) {
+ dev_err(dev, "DMA tx timeout\n");
+ dmaengine_terminate_all(dma->chan_tx);
+ dmaengine_terminate_all(dma->chan_rx);
+ return -ETIMEDOUT;
+ }
+
+ time_left = wait_for_completion_timeout(&dspi->dma->cmd_rx_complete,
+ DMA_COMPLETION_TIMEOUT);
+ if (time_left == 0) {
+ dev_err(dev, "DMA rx timeout\n");
+ dmaengine_terminate_all(dma->chan_tx);
+ dmaengine_terminate_all(dma->chan_rx);
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static int dspi_dma_xfer(struct fsl_dspi *dspi)
+{
+ struct fsl_dspi_dma *dma = dspi->dma;
+ struct device *dev = &dspi->pdev->dev;
+ int curr_remaining_bytes;
+ int bytes_per_buffer;
+ int tx_word;
+ int ret = 0;
+
+ tx_word = is_double_byte_mode(dspi);
+ curr_remaining_bytes = dspi->len;
+ while (curr_remaining_bytes) {
+ /* Check if current transfer fits the DMA buffer */
+ dma->curr_xfer_len = curr_remaining_bytes;
+ bytes_per_buffer = DSPI_DMA_BUFSIZE /
+ (DSPI_FIFO_SIZE / (tx_word ? 2 : 1));
+ if (curr_remaining_bytes > bytes_per_buffer)
+ dma->curr_xfer_len = bytes_per_buffer;
+
+ ret = dspi_next_xfer_dma_submit(dspi);
+ if (ret) {
+ dev_err(dev, "DMA transfer failed\n");
+ goto exit;
+
+ } else {
+ curr_remaining_bytes -= dma->curr_xfer_len;
+ if (curr_remaining_bytes < 0)
+ curr_remaining_bytes = 0;
+ dspi->len = curr_remaining_bytes;
+ }
+ }
+
+exit:
+ return ret;
+}
+
+static int dspi_request_dma(struct fsl_dspi *dspi, phys_addr_t phy_addr)
+{
+ struct fsl_dspi_dma *dma;
+ struct dma_slave_config cfg;
+ struct device *dev = &dspi->pdev->dev;
+ int ret;
+
+ dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
+ if (!dma)
+ return -ENOMEM;
+
+ dma->chan_rx = dma_request_slave_channel(dev, "rx");
+ if (!dma->chan_rx) {
+ dev_err(dev, "rx dma channel not available\n");
+ ret = -ENODEV;
+ return ret;
+ }
+
+ dma->chan_tx = dma_request_slave_channel(dev, "tx");
+ if (!dma->chan_tx) {
+ dev_err(dev, "tx dma channel not available\n");
+ ret = -ENODEV;
+ goto err_tx_channel;
+ }
+
+ dma->tx_dma_buf = dma_alloc_coherent(dev, DSPI_DMA_BUFSIZE,
+ &dma->tx_dma_phys, GFP_KERNEL);
+ if (!dma->tx_dma_buf) {
+ ret = -ENOMEM;
+ goto err_tx_dma_buf;
+ }
+
+ dma->rx_dma_buf = dma_alloc_coherent(dev, DSPI_DMA_BUFSIZE,
+ &dma->rx_dma_phys, GFP_KERNEL);
+ if (!dma->rx_dma_buf) {
+ ret = -ENOMEM;
+ goto err_rx_dma_buf;
+ }
+
+ cfg.src_addr = phy_addr + SPI_POPR;
+ cfg.dst_addr = phy_addr + SPI_PUSHR;
+ cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ cfg.src_maxburst = 1;
+ cfg.dst_maxburst = 1;
+
+ cfg.direction = DMA_DEV_TO_MEM;
+ ret = dmaengine_slave_config(dma->chan_rx, &cfg);
+ if (ret) {
+ dev_err(dev, "can't configure rx dma channel\n");
+ ret = -EINVAL;
+ goto err_slave_config;
+ }
+
+ cfg.direction = DMA_MEM_TO_DEV;
+ ret = dmaengine_slave_config(dma->chan_tx, &cfg);
+ if (ret) {
+ dev_err(dev, "can't configure tx dma channel\n");
+ ret = -EINVAL;
+ goto err_slave_config;
+ }
+
+ dspi->dma = dma;
+ init_completion(&dma->cmd_tx_complete);
+ init_completion(&dma->cmd_rx_complete);
+
+ return 0;
+
+err_slave_config:
+ devm_kfree(dev, dma->rx_dma_buf);
+err_rx_dma_buf:
+ devm_kfree(dev, dma->tx_dma_buf);
+err_tx_dma_buf:
+ dma_release_channel(dma->chan_tx);
+err_tx_channel:
+ dma_release_channel(dma->chan_rx);
+
+ devm_kfree(dev, dma);
+ dspi->dma = NULL;
+
+ return ret;
+}
+
+static void dspi_release_dma(struct fsl_dspi *dspi)
+{
+ struct fsl_dspi_dma *dma = dspi->dma;
+ struct device *dev = &dspi->pdev->dev;
+
+ if (dma) {
+ if (dma->chan_tx) {
+ dma_unmap_single(dev, dma->tx_dma_phys,
+ DSPI_DMA_BUFSIZE, DMA_TO_DEVICE);
+ dma_release_channel(dma->chan_tx);
+ }
+
+ if (dma->chan_rx) {
+ dma_unmap_single(dev, dma->rx_dma_phys,
+ DSPI_DMA_BUFSIZE, DMA_FROM_DEVICE);
+ dma_release_channel(dma->chan_rx);
+ }
+ }
+}
+
static void hz_to_spi_baud(char *pbr, char *br, int speed_hz,
unsigned long clkrate)
{
@@ -424,6 +709,12 @@ static int dspi_transfer_one_message(struct spi_master *master,
regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_TCFQE);
dspi_tcfq_write(dspi);
break;
+ case DSPI_DMA_MODE:
+ regmap_write(dspi->regmap, SPI_RSER,
+ SPI_RSER_TFFFE | SPI_RSER_TFFFD |
+ SPI_RSER_RFDFE | SPI_RSER_RFDFD);
+ status = dspi_dma_xfer(dspi);
+ goto out;
default:
dev_err(&dspi->pdev->dev, "unsupported trans_mode %u\n",
trans_mode);
@@ -733,6 +1024,13 @@ static int dspi_probe(struct platform_device *pdev)
if (ret)
goto out_master_put;
+ if (dspi->devtype_data->trans_mode == DSPI_DMA_MODE) {
+ if (dspi_request_dma(dspi, res->start)) {
+ dev_err(&pdev->dev, "can't get dma channels\n");
+ goto out_clk_put;
+ }
+ }
+
master->max_speed_hz =
clk_get_rate(dspi->clk) / dspi->devtype_data->max_clock_factor;
@@ -761,6 +1059,7 @@ static int dspi_remove(struct platform_device *pdev)
struct fsl_dspi *dspi = spi_master_get_devdata(master);
/* Disconnect from the SPI framework */
+ dspi_release_dma(dspi);
clk_disable_unprepare(dspi->clk);
spi_unregister_master(dspi->master);
--
2.10.2
^ permalink raw reply related
* [PATCH v6 3/9] drm/hisilicon/hibmc: Add support for frame buffer
From: Rongrong Zou @ 2016-11-11 13:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOw6vbJweGkWzXJwNSm7LmhDhW1gLG8=wu4dBw+dL-cvaBvaCw@mail.gmail.com>
? 2016/11/11 2:30, Sean Paul ??:
> On Fri, Oct 28, 2016 at 3:27 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
>> Add support for fbdev and kms fb management.
>>
>> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
>> ---
>> drivers/gpu/drm/hisilicon/hibmc/Makefile | 2 +-
>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 17 ++
>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 24 ++
>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 255 ++++++++++++++++++++++
>> drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 66 ++++++
>> 5 files changed, 363 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> index d5c40b8..810a37e 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> @@ -1,5 +1,5 @@
>> ccflags-y := -Iinclude/drm
>> -hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_power.o hibmc_ttm.o
>> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_fbdev.o hibmc_drm_power.o hibmc_ttm.o
>>
>> obj-$(CONFIG_DRM_HISI_HIBMC) +=hibmc-drm.o
>> #obj-y += hibmc-drm.o
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> index 81f4301..5ac7a7e 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> @@ -66,11 +66,23 @@ static void hibmc_disable_vblank(struct drm_device *dev, unsigned int pipe)
>>
>> static int hibmc_pm_suspend(struct device *dev)
>> {
>> + struct pci_dev *pdev = to_pci_dev(dev);
>> + struct drm_device *drm_dev = pci_get_drvdata(pdev);
>> + struct hibmc_drm_device *hidev = drm_dev->dev_private;
>> +
>> + drm_fb_helper_set_suspend_unlocked(&hidev->fbdev->helper, 1);
>> +
>
> We have atomic helpers for suspend/resume now. Please use those.
drm_atomic_helper_suspend/resume()?
>
>> return 0;
>> }
>>
>> static int hibmc_pm_resume(struct device *dev)
>> {
>> + struct pci_dev *pdev = to_pci_dev(dev);
>> + struct drm_device *drm_dev = pci_get_drvdata(pdev);
>> + struct hibmc_drm_device *hidev = drm_dev->dev_private;
>> +
>> + drm_fb_helper_set_suspend_unlocked(&hidev->fbdev->helper, 0);
>> +
>> return 0;
>> }
>>
>> @@ -170,6 +182,7 @@ static int hibmc_unload(struct drm_device *dev)
>> {
>> struct hibmc_drm_device *hidev = dev->dev_private;
>>
>> + hibmc_fbdev_fini(hidev);
>> hibmc_mm_fini(hidev);
>> hibmc_hw_fini(hidev);
>> dev->dev_private = NULL;
>> @@ -195,6 +208,10 @@ static int hibmc_load(struct drm_device *dev, unsigned long flags)
>> if (ret)
>> goto err;
>>
>> + ret = hibmc_fbdev_init(hidev);
>> + if (ret)
>> + goto err;
>> +
>> return 0;
>>
>> err:
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> index db8d80e..a40e9a7 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> @@ -20,9 +20,22 @@
>> #define HIBMC_DRM_DRV_H
>>
>> #include <drm/drmP.h>
>> +#include <drm/drm_fb_helper.h>
>> #include <drm/ttm/ttm_bo_driver.h>
>> #include <drm/drm_gem.h>
>>
>> +struct hibmc_framebuffer {
>> + struct drm_framebuffer fb;
>> + struct drm_gem_object *obj;
>> + bool is_fbdev_fb;
>> +};
>> +
>> +struct hibmc_fbdev {
>> + struct drm_fb_helper helper;
>> + struct hibmc_framebuffer *fb;
>> + int size;
>> +};
>> +
>> struct hibmc_drm_device {
>> /* hw */
>> void __iomem *mmio;
>> @@ -41,9 +54,13 @@ struct hibmc_drm_device {
>> bool initialized;
>> } ttm;
>>
>> + /* fbdev */
>> + struct hibmc_fbdev *fbdev;
>> bool mm_inited;
>> };
>>
>> +#define to_hibmc_framebuffer(x) container_of(x, struct hibmc_framebuffer, fb)
>> +
>> struct hibmc_bo {
>> struct ttm_buffer_object bo;
>> struct ttm_placement placement;
>> @@ -65,8 +82,15 @@ static inline struct hibmc_bo *gem_to_hibmc_bo(struct drm_gem_object *gem)
>>
>> #define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
>>
>> +int hibmc_fbdev_init(struct hibmc_drm_device *hidev);
>> +void hibmc_fbdev_fini(struct hibmc_drm_device *hidev);
>> +
>> int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel,
>> struct drm_gem_object **obj);
>> +struct hibmc_framebuffer *
>> +hibmc_framebuffer_init(struct drm_device *dev,
>> + const struct drm_mode_fb_cmd2 *mode_cmd,
>> + struct drm_gem_object *obj);
>>
>> int hibmc_mm_init(struct hibmc_drm_device *hibmc);
>> void hibmc_mm_fini(struct hibmc_drm_device *hibmc);
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
>> new file mode 100644
>> index 0000000..630124b
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
>> @@ -0,0 +1,255 @@
>> +/* Hisilicon Hibmc SoC drm driver
>> + *
>> + * Based on the bochs drm driver.
>> + *
>> + * Copyright (c) 2016 Huawei Limited.
>> + *
>> + * Author:
>> + * Rongrong Zou <zourongrong@huawei.com>
>> + * Rongrong Zou <zourongrong@gmail.com>
>> + * Jianhua Li <lijianhua@huawei.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + */
>> +
>> +#include <drm/drm_crtc.h>
>> +#include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_fb_helper.h>
>> +
>> +#include "hibmc_drm_drv.h"
>> +
>> +/* ---------------------------------------------------------------------- */
>
> Please remove
will do that, thanks.
>
>> +
>> +static int hibmcfb_create_object(
>> + struct hibmc_drm_device *hidev,
>> + const struct drm_mode_fb_cmd2 *mode_cmd,
>> + struct drm_gem_object **gobj_p)
>> +{
>> + struct drm_gem_object *gobj;
>> + struct drm_device *dev = hidev->dev;
>> + u32 size;
>> + int ret = 0;
>> +
>> + size = mode_cmd->pitches[0] * mode_cmd->height;
>> + ret = hibmc_gem_create(dev, size, true, &gobj);
>> + if (ret)
>> + return ret;
>> +
>> + *gobj_p = gobj;
>> + return ret;
>> +}
>> +
>> +static struct fb_ops hibmc_drm_fb_ops = {
>> + .owner = THIS_MODULE,
>> + .fb_check_var = drm_fb_helper_check_var,
>> + .fb_set_par = drm_fb_helper_set_par,
>> + .fb_fillrect = drm_fb_helper_sys_fillrect,
>> + .fb_copyarea = drm_fb_helper_sys_copyarea,
>> + .fb_imageblit = drm_fb_helper_sys_imageblit,
>> + .fb_pan_display = drm_fb_helper_pan_display,
>> + .fb_blank = drm_fb_helper_blank,
>> + .fb_setcmap = drm_fb_helper_setcmap,
>> +};
>> +
>> +static int hibmc_drm_fb_create(struct drm_fb_helper *helper,
>> + struct drm_fb_helper_surface_size *sizes)
>> +{
>> + struct hibmc_fbdev *hi_fbdev =
>> + container_of(helper, struct hibmc_fbdev, helper);
>> + struct hibmc_drm_device *hidev =
>> + (struct hibmc_drm_device *)helper->dev->dev_private;
>> + struct fb_info *info;
>> + struct hibmc_framebuffer *hibmc_fb;
>> + struct drm_framebuffer *fb;
>> + struct drm_mode_fb_cmd2 mode_cmd;
>> + struct drm_gem_object *gobj = NULL;
>> + int ret = 0;
>> + size_t size;
>> + unsigned int bytes_per_pixel;
>> + struct hibmc_bo *bo = NULL;
>> +
>> + DRM_DEBUG_DRIVER("surface width(%d), height(%d) and bpp(%d)\n",
>> + sizes->surface_width, sizes->surface_height,
>> + sizes->surface_bpp);
>> + sizes->surface_depth = 32;
>> +
>> + bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
>> +
>> + mode_cmd.width = sizes->surface_width;
>> + mode_cmd.height = sizes->surface_height;
>> + mode_cmd.pitches[0] = mode_cmd.width * bytes_per_pixel;
>> + mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
>> + sizes->surface_depth);
>> +
>> + size = roundup(mode_cmd.pitches[0] * mode_cmd.height, PAGE_SIZE);
>
> It's somewhat curious that you used ALIGN in the previous patch and
> roundup here. But anyways, I think PAGE_ALIGN would be the most
> appropriate thing to do here.
agreed, thanks.
>
>> +
>> + ret = hibmcfb_create_object(hidev, &mode_cmd, &gobj);
>> + if (ret) {
>> + DRM_ERROR("failed to create fbcon backing object %d\r\n",
> ret);
>
> \r, yikes!!!
will delete it, thanks.
>
>> + return -ENOMEM;
>> + }
>> +
>> + bo = gem_to_hibmc_bo(gobj);
>> +
>> + ret = ttm_bo_reserve(&bo->bo, true, false, NULL);
>> + if (ret)
>
> Print error here?
will do.
>
> How about cleaning up gobj?
you are right,
>
>> + return ret;
>> +
>> + ret = hibmc_bo_pin(bo, TTM_PL_FLAG_VRAM, NULL);
>> + if (ret) {
>> + DRM_ERROR("failed to pin fbcon\n");
>
> Print ret
>
> ttm_bo_unreserve? It seems like you're missing clean-up in all of the
> error paths in this function. Can you please make sure everything is
> tidied up?
ok, thanks.
>
>> + return ret;
>> + }
>> +
>> + ret = ttm_bo_kmap(&bo->bo, 0, bo->bo.num_pages, &bo->kmap);
>> +
>
> nit: extra space
do you mean extra line?
>
>> + if (ret) {
>> + DRM_ERROR("failed to kmap fbcon\n");
>
> Print ret
ok, thanks.
>
>> + ttm_bo_unreserve(&bo->bo);
>> + return ret;
>> + }
>> +
>> + ttm_bo_unreserve(&bo->bo);
>
> Move this between ttm_bo_kmap and if (ret), then remove it from inside
> the conditional.
This is fine with me, thanks.
>
>> +
>> + info = drm_fb_helper_alloc_fbi(helper);
>> + if (IS_ERR(info))
>
> Print error
ok, thanks.
>
>> + return PTR_ERR(info);
>> +
>> + info->par = hi_fbdev;
>> +
>> + hibmc_fb = hibmc_framebuffer_init(hidev->dev, &mode_cmd, gobj);
>> + if (IS_ERR(hibmc_fb)) {
>> + drm_fb_helper_release_fbi(helper);
>> + return PTR_ERR(hibmc_fb);
>> + }
>> +
>> + hi_fbdev->fb = hibmc_fb;
>> + hidev->fbdev->size = size;
>> + fb = &hibmc_fb->fb;
>
> The fb variable isn't necessary, and really, the hibmc_fb doesn't
> really help either, it just masks what's actually happening IMO.
i will clean the two variables.
>
>> + if (!fb) {
>> + DRM_INFO("fb is NULL\n");
>> + return -EINVAL;
>> + }
>> +
>> + hi_fbdev->helper.fb = fb;
>> +
>> + strcpy(info->fix.id, "hibmcdrmfb");
>> +
>> + info->flags = FBINFO_DEFAULT;
>> + info->fbops = &hibmc_drm_fb_ops;
>> +
>> + drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
>> + drm_fb_helper_fill_var(info, &hidev->fbdev->helper, sizes->fb_width,
>> + sizes->fb_height);
>> +
>> + info->screen_base = bo->kmap.virtual;
>> + info->screen_size = size;
>> +
>> + info->fix.smem_start = bo->bo.mem.bus.offset + bo->bo.mem.bus.base;
>> + info->fix.smem_len = size;
>> +
>> + return 0;
>> +}
>> +
>> +static void hibmc_fbdev_destroy(struct hibmc_fbdev *fbdev)
>> +{
>> + struct hibmc_framebuffer *gfb = fbdev->fb;
>> + struct drm_fb_helper *fbh = &fbdev->helper;
>> +
>> + DRM_DEBUG_DRIVER("hibmc_fbdev_destroy\n");
>
> Not useful
ok, will remove.
>
>> +
>> + drm_fb_helper_unregister_fbi(fbh);
>> + drm_fb_helper_release_fbi(fbh);
>> +
>> + drm_fb_helper_fini(fbh);
>> +
>> + if (gfb)
>> + drm_framebuffer_unreference(&gfb->fb);
>> +
>> + kfree(fbdev);
>> +}
>> +
>> +static const struct drm_fb_helper_funcs hibmc_fbdev_helper_funcs = {
>> + .fb_probe = hibmc_drm_fb_create,
>> +};
>> +
>> +int hibmc_fbdev_init(struct hibmc_drm_device *hidev)
>> +{
>> + int ret;
>> + struct fb_var_screeninfo *var;
>> + struct fb_fix_screeninfo *fix;
>> + struct hibmc_fbdev *hifbdev;
>> +
>> + hifbdev = kzalloc(sizeof(*hifbdev), GFP_KERNEL);
>
> devm_kzalloc?
sounds good, so there need no kfree at hibmc_fbdev_destroy(),
thanks.
>
>> + if (!hifbdev)
>> + return -ENOMEM;
>> +
>> + hidev->fbdev = hifbdev;
>> + drm_fb_helper_prepare(hidev->dev, &hifbdev->helper,
>> + &hibmc_fbdev_helper_funcs);
>> +
>> + /* Now just one crtc and one channel */
>> + ret = drm_fb_helper_init(hidev->dev,
>> + &hifbdev->helper, 1, 1);
>> +
>
> nit: extra line
ok, thanks.
>
>> + if (ret)
>> + return ret;
>> +
>> + ret = drm_fb_helper_single_add_all_connectors(&hifbdev->helper);
>> + if (ret)
>> + goto fini;
>> +
>> + ret = drm_fb_helper_initial_config(&hifbdev->helper, 16);
>> + if (ret)
>> + goto fini;
>> +
>> + var = &hifbdev->helper.fbdev->var;
>> + fix = &hifbdev->helper.fbdev->fix;
>> +
>> + DRM_DEBUG("Member of info->var is :\n"
>> + "xres=%d\n"
>> + "yres=%d\n"
>> + "xres_virtual=%d\n"
>> + "yres_virtual=%d\n"
>> + "xoffset=%d\n"
>> + "yoffset=%d\n"
>> + "bits_per_pixel=%d\n"
>> + "...\n", var->xres, var->yres, var->xres_virtual,
>> + var->yres_virtual, var->xoffset, var->yoffset,
>> + var->bits_per_pixel);
>> + DRM_DEBUG("Member of info->fix is :\n"
>> + "smem_start=%lx\n"
>> + "smem_len=%d\n"
>> + "type=%d\n"
>> + "type_aux=%d\n"
>> + "visual=%d\n"
>> + "xpanstep=%d\n"
>> + "ypanstep=%d\n"
>> + "ywrapstep=%d\n"
>> + "line_length=%d\n"
>> + "accel=%d\n"
>> + "capabilities=%d\n"
>> + "...\n", fix->smem_start, fix->smem_len, fix->type,
>> + fix->type_aux, fix->visual, fix->xpanstep,
>> + fix->ypanstep, fix->ywrapstep, fix->line_length,
>> + fix->accel, fix->capabilities);
>
> You've been using DRM_DEBUG_DRIVER elsewhere, you should probably use
> it here, too.
ok, thanks.
>
>> +
>> + return 0;
>> +
>> +fini:
>> + drm_fb_helper_fini(&hifbdev->helper);
>> + return ret;
>> +}
>> +
>> +void hibmc_fbdev_fini(struct hibmc_drm_device *hidev)
>> +{
>> + if (!hidev->fbdev)
>> + return;
>> +
>> + hibmc_fbdev_destroy(hidev->fbdev);
>> + hidev->fbdev = NULL;
>> +}
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>> index 0802ebd..9822f62 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>> @@ -488,3 +488,69 @@ int hibmc_dumb_mmap_offset(struct drm_file *file, struct drm_device *dev,
>> drm_gem_object_unreference_unlocked(obj);
>> return 0;
>> }
>> +
>> +/* ---------------------------------------------------------------------- */
>> +
>
> Please remove
will do.
>
>> +static void hibmc_user_framebuffer_destroy(struct drm_framebuffer *fb)
>> +{
>> + struct hibmc_framebuffer *hibmc_fb = to_hibmc_framebuffer(fb);
>> +
>> + drm_gem_object_unreference_unlocked(hibmc_fb->obj);
>> + drm_framebuffer_cleanup(fb);
>> + kfree(hibmc_fb);
>> +}
>> +
>> +static const struct drm_framebuffer_funcs hibmc_fb_funcs = {
>> + .destroy = hibmc_user_framebuffer_destroy,
>> +};
>> +
>> +struct hibmc_framebuffer *
>> +hibmc_framebuffer_init(struct drm_device *dev,
>> + const struct drm_mode_fb_cmd2 *mode_cmd,
>> + struct drm_gem_object *obj)
>> +{
>> + struct hibmc_framebuffer *hibmc_fb;
>> + int ret;
>> +
>> + hibmc_fb = kzalloc(sizeof(*hibmc_fb), GFP_KERNEL);
>> + if (!hibmc_fb)
>
> Print error
ok, thanks.
Regards,
Rongrong.
>
>> + return ERR_PTR(-ENOMEM);
>> +
>> + drm_helper_mode_fill_fb_struct(&hibmc_fb->fb, mode_cmd);
>> + hibmc_fb->obj = obj;
>> + ret = drm_framebuffer_init(dev, &hibmc_fb->fb, &hibmc_fb_funcs);
>> + if (ret) {
>> + DRM_ERROR("drm_framebuffer_init failed: %d\n", ret);
>> + kfree(hibmc_fb);
>> + return ERR_PTR(ret);
>> + }
>> +
>> + return hibmc_fb;
>> +}
>> +
>> +static struct drm_framebuffer *
>> +hibmc_user_framebuffer_create(struct drm_device *dev,
>> + struct drm_file *filp,
>> + const struct drm_mode_fb_cmd2 *mode_cmd)
>> +{
>> + struct drm_gem_object *obj;
>> + struct hibmc_framebuffer *hibmc_fb;
>> +
>> + DRM_DEBUG_DRIVER("%dx%d, format %c%c%c%c\n",
>> + mode_cmd->width, mode_cmd->height,
>> + (mode_cmd->pixel_format) & 0xff,
>> + (mode_cmd->pixel_format >> 8) & 0xff,
>> + (mode_cmd->pixel_format >> 16) & 0xff,
>> + (mode_cmd->pixel_format >> 24) & 0xff);
>> +
>> + obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);
>> + if (!obj)
>> + return ERR_PTR(-ENOENT);
>> +
>> + hibmc_fb = hibmc_framebuffer_init(dev, mode_cmd, obj);
>> + if (IS_ERR(hibmc_fb)) {
>> + drm_gem_object_unreference_unlocked(obj);
>> + return ERR_PTR((long)hibmc_fb);
>> + }
>> + return &hibmc_fb->fb;
>> +}
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> _______________________________________________
> linuxarm mailing list
> linuxarm at huawei.com
> http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm
>
> .
>
^ permalink raw reply
* [PATCH v6 2/9] drm/hisilicon/hibmc: Add video memory management
From: Sean Paul @ 2016-11-11 13:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5825A8A3.5070409@huawei.com>
On Fri, Nov 11, 2016 at 6:16 AM, Rongrong Zou <zourongrong@huawei.com> wrote:
> ? 2016/11/11 1:35, Sean Paul ??:
>>
>> On Fri, Oct 28, 2016 at 3:27 AM, Rongrong Zou <zourongrong@gmail.com>
>> wrote:
>>>
>>> Hibmc have 32m video memory which can be accessed through PCIe by host,
>>> we use ttm to manage these memory.
>>>
>>> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
>>> ---
>>> drivers/gpu/drm/hisilicon/hibmc/Kconfig | 1 +
>>> drivers/gpu/drm/hisilicon/hibmc/Makefile | 2 +-
>>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 12 +
>>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 46 +++
>>> drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 490
>>> ++++++++++++++++++++++++
>>> 5 files changed, 550 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>>>
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Kconfig
>>> b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
>>> index a9af90d..bcb8c18 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/Kconfig
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
>>> @@ -1,6 +1,7 @@
>>> config DRM_HISI_HIBMC
>>> tristate "DRM Support for Hisilicon Hibmc"
>>> depends on DRM && PCI
>>> + select DRM_TTM
>>>
>>> help
>>> Choose this option if you have a Hisilicon Hibmc soc chipset.
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> index 97cf4a0..d5c40b8 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> @@ -1,5 +1,5 @@
>>> ccflags-y := -Iinclude/drm
>>> -hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_power.o
>>> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_power.o hibmc_ttm.o
>>>
>>> obj-$(CONFIG_DRM_HISI_HIBMC) +=hibmc-drm.o
>>> #obj-y += hibmc-drm.o
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> index 4669d42..81f4301 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> @@ -31,6 +31,7 @@
>>> #ifdef CONFIG_COMPAT
>>> .compat_ioctl = drm_compat_ioctl,
>>> #endif
>>> + .mmap = hibmc_mmap,
>>> .poll = drm_poll,
>>> .read = drm_read,
>>> .llseek = no_llseek,
>>> @@ -46,6 +47,8 @@ static void hibmc_disable_vblank(struct drm_device
>>> *dev, unsigned int pipe)
>>> }
>>>
>>> static struct drm_driver hibmc_driver = {
>>> + .driver_features = DRIVER_GEM,
>>> +
>>
>>
>> nit: extra space
>>
>>> .fops = &hibmc_fops,
>>> .name = "hibmc",
>>> .date = "20160828",
>>> @@ -55,6 +58,10 @@ static void hibmc_disable_vblank(struct drm_device
>>> *dev, unsigned int pipe)
>>> .get_vblank_counter = drm_vblank_no_hw_counter,
>>> .enable_vblank = hibmc_enable_vblank,
>>> .disable_vblank = hibmc_disable_vblank,
>>> + .gem_free_object_unlocked = hibmc_gem_free_object,
>>> + .dumb_create = hibmc_dumb_create,
>>> + .dumb_map_offset = hibmc_dumb_mmap_offset,
>>> + .dumb_destroy = drm_gem_dumb_destroy,
>>> };
>>>
>>> static int hibmc_pm_suspend(struct device *dev)
>>> @@ -163,6 +170,7 @@ static int hibmc_unload(struct drm_device *dev)
>>> {
>>> struct hibmc_drm_device *hidev = dev->dev_private;
>>>
>>> + hibmc_mm_fini(hidev);
>>> hibmc_hw_fini(hidev);
>>> dev->dev_private = NULL;
>>> return 0;
>>> @@ -183,6 +191,10 @@ static int hibmc_load(struct drm_device *dev,
>>> unsigned long flags)
>>> if (ret)
>>> goto err;
>>>
>>> + ret = hibmc_mm_init(hidev);
>>> + if (ret)
>>> + goto err;
>>> +
>>> return 0;
>>>
>>> err:
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> index 0037341..db8d80e 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> @@ -20,6 +20,8 @@
>>> #define HIBMC_DRM_DRV_H
>>>
>>> #include <drm/drmP.h>
>>> +#include <drm/ttm/ttm_bo_driver.h>
>>> +#include <drm/drm_gem.h>
>>
>>
>> nit: alphabetize
>
>
> will fix it, thanks.
>
>>
>>>
>>> struct hibmc_drm_device {
>>> /* hw */
>>> @@ -30,6 +32,50 @@ struct hibmc_drm_device {
>>>
>>> /* drm */
>>> struct drm_device *dev;
>>> +
>>> + /* ttm */
>>> + struct {
>>> + struct drm_global_reference mem_global_ref;
>>> + struct ttm_bo_global_ref bo_global_ref;
>>> + struct ttm_bo_device bdev;
>>> + bool initialized;
>>> + } ttm;
>>
>>
>> I don't think you gain anything other than keystrokes from the substruct
>
>
> I'm sorry i didn't catch you, i looked at the all drivers used ttm such
> as ast/bochs/cirrus/mgag200/qxl/virtio_gpu, they all embedded the ttm
> substruct
> into the driver-private struct.
>
> so do you mean
> struct hibmc_drm_device {
> /* hw */
> void __iomem *mmio;
> void __iomem *fb_map;
> unsigned long fb_base;
> unsigned long fb_size;
>
> /* drm */
> struct drm_device *dev;
> struct drm_plane plane;
> struct drm_crtc crtc;
> struct drm_encoder encoder;
> struct drm_connector connector;
> bool mode_config_initialized;
>
> /* ttm */
> struct drm_global_reference mem_global_ref;
> struct ttm_bo_global_ref bo_global_ref;
> struct ttm_bo_device bdev;
> bool initialized;
> ...
> };
> ?
Yeah, that's what I was thinking
>
>>
>>> +
>>> + bool mm_inited;
>>> };
>>>
>>> +struct hibmc_bo {
>>> + struct ttm_buffer_object bo;
>>> + struct ttm_placement placement;
>>> + struct ttm_bo_kmap_obj kmap;
>>> + struct drm_gem_object gem;
>>> + struct ttm_place placements[3];
>>> + int pin_count;
>>> +};
>>> +
>>> +static inline struct hibmc_bo *hibmc_bo(struct ttm_buffer_object *bo)
>>> +{
>>> + return container_of(bo, struct hibmc_bo, bo);
>>> +}
>>> +
>>> +static inline struct hibmc_bo *gem_to_hibmc_bo(struct drm_gem_object
>>> *gem)
>>> +{
>>> + return container_of(gem, struct hibmc_bo, gem);
>>> +}
>>> +
>>> +#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
>>
>>
>> Hide this in ttm.c
>
>
> ok, will do that.
> thanks for pointing it out.
>
>
>>
>>> +
>>> +int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel,
>>> + struct drm_gem_object **obj);
>>> +
>>> +int hibmc_mm_init(struct hibmc_drm_device *hibmc);
>>> +void hibmc_mm_fini(struct hibmc_drm_device *hibmc);
>>> +int hibmc_bo_pin(struct hibmc_bo *bo, u32 pl_flag, u64 *gpu_addr);
>>> +void hibmc_gem_free_object(struct drm_gem_object *obj);
>>> +int hibmc_dumb_create(struct drm_file *file, struct drm_device *dev,
>>> + struct drm_mode_create_dumb *args);
>>> +int hibmc_dumb_mmap_offset(struct drm_file *file, struct drm_device
>>> *dev,
>>> + u32 handle, u64 *offset);
>>> +int hibmc_mmap(struct file *filp, struct vm_area_struct *vma);
>>> +
>>> #endif
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>>> new file mode 100644
>>> index 0000000..0802ebd
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>>> @@ -0,0 +1,490 @@
>>> +/* Hisilicon Hibmc SoC drm driver
>>> + *
>>> + * Based on the bochs drm driver.
>>> + *
>>> + * Copyright (c) 2016 Huawei Limited.
>>> + *
>>> + * Author:
>>> + * Rongrong Zou <zourongrong@huawei.com>
>>> + * Rongrong Zou <zourongrong@gmail.com>
>>> + * Jianhua Li <lijianhua@huawei.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + */
>>> +
>>> +#include "hibmc_drm_drv.h"
>>> +#include <ttm/ttm_page_alloc.h>
>>> +#include <drm/drm_crtc_helper.h>
>>> +#include <drm/drm_atomic_helper.h>
>>> +
>>> +static inline struct hibmc_drm_device *
>>> +hibmc_bdev(struct ttm_bo_device *bd)
>>> +{
>>> + return container_of(bd, struct hibmc_drm_device, ttm.bdev);
>>> +}
>>> +
>>> +static int
>>> +hibmc_ttm_mem_global_init(struct drm_global_reference *ref)
>>> +{
>>> + return ttm_mem_global_init(ref->object);
>>> +}
>>> +
>>> +static void
>>> +hibmc_ttm_mem_global_release(struct drm_global_reference *ref)
>>> +{
>>> + ttm_mem_global_release(ref->object);
>>> +}
>>> +
>>> +static int hibmc_ttm_global_init(struct hibmc_drm_device *hibmc)
>>> +{
>>> + struct drm_global_reference *global_ref;
>>> + int r;
>>
>>
>> nit: try not to use one character variable names unless it's for the
>> purpose of a loop (ie: i,j). You also use ret elsewhere in the driver,
>> so it'd be nice to remain consistent
>
>
> the whole file is delivered from bochs ttm, i didn't modify anything except
> some checkpatch warnings and the 'hibmc_' prefix. Unfortunately, some
> problems were delivered too.
Yeah, seems like it. Perhaps you can post patches to fix these issues
in the other drivers too :)
>
>>
>>> +
>>> + global_ref = &hibmc->ttm.mem_global_ref;
>>
>>
>> I think using the global_ref local obfuscates what you're doing here.
>> It saves you 6 characters while typing, but adds a layer of
>> indirection for all future readers.
>>
>>> + global_ref->global_type = DRM_GLOBAL_TTM_MEM;
>>> + global_ref->size = sizeof(struct ttm_mem_global);
>>> + global_ref->init = &hibmc_ttm_mem_global_init;
>>> + global_ref->release = &hibmc_ttm_mem_global_release;
>>> + r = drm_global_item_ref(global_ref);
>>> + if (r != 0) {
>>
>>
>> nit: if (r)
>
>
> will fix it,
> thanks.
> BTW, i wonder why checkpatch.pl didn't report it.
>
>
>>
>>> + DRM_ERROR("Failed setting up TTM memory accounting
>>> subsystem.\n"
>>> + );
>>
>>
>> Breaking up the line for one character is probably not worthwhile, and
>> you should really print the error. How about:
>>
>> DRM_ERROR("Could not get ref on ttm global ret=%d.\n", ret);
>
>
> i like your solution, thanks.
>
>
>>
>>
>>> + return r;
>>> + }
>>> +
>>> + hibmc->ttm.bo_global_ref.mem_glob =
>>> + hibmc->ttm.mem_global_ref.object;
>>> + global_ref = &hibmc->ttm.bo_global_ref.ref;
>>> + global_ref->global_type = DRM_GLOBAL_TTM_BO;
>>> + global_ref->size = sizeof(struct ttm_bo_global);
>>> + global_ref->init = &ttm_bo_global_init;
>>> + global_ref->release = &ttm_bo_global_release;
>>> + r = drm_global_item_ref(global_ref);
>>> + if (r != 0) {
>>> + DRM_ERROR("Failed setting up TTM BO subsystem.\n");
>>> + drm_global_item_unref(&hibmc->ttm.mem_global_ref);
>>> + return r;
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +static void
>>> +hibmc_ttm_global_release(struct hibmc_drm_device *hibmc)
>>> +{
>>> + if (!hibmc->ttm.mem_global_ref.release)
>>
>>
>> Are you actually hitting this condition? This seems like it's papering
>> over something else.
>
>
> it was also delivered from others, i looked at the xxx_ttm_global_init
> function, 'mem_global_ref.release' is assigned unconditionally, so i
> think this condition never be hit, it may be hit when release twice,
> but this won't take place in my driver.
>
Yeah, that's what I was hoping for. So perhaps we can remove this?
>>
>>> + return;
>>> +
>>> + drm_global_item_unref(&hibmc->ttm.bo_global_ref.ref);
>>> + drm_global_item_unref(&hibmc->ttm.mem_global_ref);
>>> + hibmc->ttm.mem_global_ref.release = NULL;
>>> +}
>>> +
>>> +static void hibmc_bo_ttm_destroy(struct ttm_buffer_object *tbo)
>>> +{
>>> + struct hibmc_bo *bo;
>>> +
>>> + bo = container_of(tbo, struct hibmc_bo, bo);
>>
>>
>> nit: No need to split this into a separate line.
>
>
> agreed, thanks.
>
>>
>>> +
>>> + drm_gem_object_release(&bo->gem);
>>> + kfree(bo);
>>> +}
>>> +
>>> +static bool hibmc_ttm_bo_is_hibmc_bo(struct ttm_buffer_object *bo)
>>> +{
>>> + if (bo->destroy == &hibmc_bo_ttm_destroy)
>>> + return true;
>>> + return false;
>>
>>
>> return bo->destroy == &hibmc_bo_ttm_destroy;
>
>
> looks better to me.
>
>
>>
>>> +}
>>> +
>>> +static int
>>> +hibmc_bo_init_mem_type(struct ttm_bo_device *bdev, u32 type,
>>> + struct ttm_mem_type_manager *man)
>>> +{
>>> + switch (type) {
>>> + case TTM_PL_SYSTEM:
>>> + man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>>> + man->available_caching = TTM_PL_MASK_CACHING;
>>> + man->default_caching = TTM_PL_FLAG_CACHED;
>>> + break;
>>> + case TTM_PL_VRAM:
>>> + man->func = &ttm_bo_manager_func;
>>> + man->flags = TTM_MEMTYPE_FLAG_FIXED |
>>> + TTM_MEMTYPE_FLAG_MAPPABLE;
>>> + man->available_caching = TTM_PL_FLAG_UNCACHED |
>>> + TTM_PL_FLAG_WC;
>>> + man->default_caching = TTM_PL_FLAG_WC;
>>> + break;
>>> + default:
>>> + DRM_ERROR("Unsupported memory type %u\n", type);
>>> + return -EINVAL;
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +void hibmc_ttm_placement(struct hibmc_bo *bo, int domain)
>>> +{
>>> + u32 c = 0;
>>
>>
>> Can you please use a more descriptive name than 'c'?
>
>
> ok, will do that.
>
>>
>>> + u32 i;
>>> +
>>> + bo->placement.placement = bo->placements;
>>> + bo->placement.busy_placement = bo->placements;
>>> + if (domain & TTM_PL_FLAG_VRAM)
>>> + bo->placements[c++].flags = TTM_PL_FLAG_WC |
>>> + TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_VRAM;
>>
>>
>> nit: you're alignment is off here and below
>
>
> is it correct?
>
> if (domain & TTM_PL_FLAG_VRAM)
> bo->placements[c++].flags = TTM_PL_FLAG_WC |
> TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_VRAM;
> if (domain & TTM_PL_FLAG_SYSTEM)
> bo->placements[c++].flags = TTM_PL_MASK_CACHING |
> TTM_PL_FLAG_SYSTEM;
> if (!c)
> bo->placements[c++].flags = TTM_PL_MASK_CACHING |
> TTM_PL_FLAG_SYSTEM;
>
Pretty much anything other than lining them up one under the other is better
>>
>>> + if (domain & TTM_PL_FLAG_SYSTEM)
>>> + bo->placements[c++].flags = TTM_PL_MASK_CACHING |
>>> + TTM_PL_FLAG_SYSTEM;
>>> + if (!c)
>>> + bo->placements[c++].flags = TTM_PL_MASK_CACHING |
>>> + TTM_PL_FLAG_SYSTEM;
>>> +
>>> + bo->placement.num_placement = c;
>>> + bo->placement.num_busy_placement = c;
>>> + for (i = 0; i < c; ++i) {
>>
>>
>> nit: we tend towards post-increment in kernel
>
>
> agreed, thanks.
>
>
>>
>>> + bo->placements[i].fpfn = 0;
>>> + bo->placements[i].lpfn = 0;
>>> + }
>>> +}
>>> +
>>> +static void
>>> +hibmc_bo_evict_flags(struct ttm_buffer_object *bo, struct ttm_placement
>>> *pl)
>>> +{
>>> + struct hibmc_bo *hibmcbo = hibmc_bo(bo);
>>> +
>>> + if (!hibmc_ttm_bo_is_hibmc_bo(bo))
>>> + return;
>>> +
>>> + hibmc_ttm_placement(hibmcbo, TTM_PL_FLAG_SYSTEM);
>>> + *pl = hibmcbo->placement;
>>> +}
>>> +
>>> +static int hibmc_bo_verify_access(struct ttm_buffer_object *bo,
>>> + struct file *filp)
>>> +{
>>> + struct hibmc_bo *hibmcbo = hibmc_bo(bo);
>>> +
>>> + return drm_vma_node_verify_access(&hibmcbo->gem.vma_node,
>>> + filp->private_data);
>>> +}
>>> +
>>> +static int hibmc_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
>>> + struct ttm_mem_reg *mem)
>>> +{
>>> + struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>>> + struct hibmc_drm_device *hibmc = hibmc_bdev(bdev);
>>> +
>>> + mem->bus.addr = NULL;
>>> + mem->bus.offset = 0;
>>> + mem->bus.size = mem->num_pages << PAGE_SHIFT;
>>> + mem->bus.base = 0;
>>> + mem->bus.is_iomem = false;
>>> + if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
>>> + return -EINVAL;
>>> + switch (mem->mem_type) {
>>> + case TTM_PL_SYSTEM:
>>> + /* system memory */
>>> + return 0;
>>> + case TTM_PL_VRAM:
>>> + mem->bus.offset = mem->start << PAGE_SHIFT;
>>> + mem->bus.base = pci_resource_start(hibmc->dev->pdev, 0);
>>> + mem->bus.is_iomem = true;
>>> + break;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +static void hibmc_ttm_io_mem_free(struct ttm_bo_device *bdev,
>>> + struct ttm_mem_reg *mem)
>>> +{
>>> +}
>>
>>
>> No need to stub this, the caller does a NULL-check before invoking
>
>
> will delete it, thanks.
>
>>
>>> +
>>> +static void hibmc_ttm_backend_destroy(struct ttm_tt *tt)
>>> +{
>>> + ttm_tt_fini(tt);
>>> + kfree(tt);
>>> +}
>>> +
>>> +static struct ttm_backend_func hibmc_tt_backend_func = {
>>> + .destroy = &hibmc_ttm_backend_destroy,
>>> +};
>>> +
>>> +static struct ttm_tt *hibmc_ttm_tt_create(struct ttm_bo_device *bdev,
>>> + unsigned long size,
>>> + u32 page_flags,
>>> + struct page *dummy_read_page)
>>> +{
>>> + struct ttm_tt *tt;
>>> +
>>> + tt = kzalloc(sizeof(*tt), GFP_KERNEL);
>>> + if (!tt)
>>
>>
>> Print error
>
>
> ok, will do that, thanks.
>
>>
>>> + return NULL;
>>> + tt->func = &hibmc_tt_backend_func;
>>> + if (ttm_tt_init(tt, bdev, size, page_flags, dummy_read_page)) {
>>
>>
>> Here too?
>
>
> ditto
>
>
>>
>>> + kfree(tt);
>>> + return NULL;
>>> + }
>>> + return tt;
>>> +}
>>> +
>>> +static int hibmc_ttm_tt_populate(struct ttm_tt *ttm)
>>> +{
>>> + return ttm_pool_populate(ttm);
>>> +}
>>> +
>>> +static void hibmc_ttm_tt_unpopulate(struct ttm_tt *ttm)
>>> +{
>>> + ttm_pool_unpopulate(ttm);
>>> +}
>>> +
>>> +struct ttm_bo_driver hibmc_bo_driver = {
>>> + .ttm_tt_create = hibmc_ttm_tt_create,
>>> + .ttm_tt_populate = hibmc_ttm_tt_populate,
>>> + .ttm_tt_unpopulate = hibmc_ttm_tt_unpopulate,
>>> + .init_mem_type = hibmc_bo_init_mem_type,
>>> + .evict_flags = hibmc_bo_evict_flags,
>>> + .move = NULL,
>>> + .verify_access = hibmc_bo_verify_access,
>>> + .io_mem_reserve = &hibmc_ttm_io_mem_reserve,
>>> + .io_mem_free = &hibmc_ttm_io_mem_free,
>>> + .lru_tail = &ttm_bo_default_lru_tail,
>>> + .swap_lru_tail = &ttm_bo_default_swap_lru_tail,
>>> +};
>>> +
>>> +int hibmc_mm_init(struct hibmc_drm_device *hibmc)
>>> +{
>>> + int ret;
>>> + struct drm_device *dev = hibmc->dev;
>>> + struct ttm_bo_device *bdev = &hibmc->ttm.bdev;
>>> +
>>> + ret = hibmc_ttm_global_init(hibmc);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = ttm_bo_device_init(&hibmc->ttm.bdev,
>>> + hibmc->ttm.bo_global_ref.ref.object,
>>> + &hibmc_bo_driver,
>>> + dev->anon_inode->i_mapping,
>>> + DRM_FILE_PAGE_OFFSET,
>>> + true);
>>> + if (ret) {
>>
>>
>> Call hibmc_ttm_global_release here?
>
>
> agreed, thanks for pointing it out.
>
>>
>>> + DRM_ERROR("Error initialising bo driver; %d\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + ret = ttm_bo_init_mm(bdev, TTM_PL_VRAM,
>>> + hibmc->fb_size >> PAGE_SHIFT);
>>> + if (ret) {
>>
>>
>> Clean up here as well?
>
>
> ditto
>
>
>>
>>> + DRM_ERROR("Failed ttm VRAM init: %d\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + hibmc->mm_inited = true;
>>> + return 0;
>>> +}
>>> +
>>> +void hibmc_mm_fini(struct hibmc_drm_device *hibmc)
>>> +{
>>> + if (!hibmc->mm_inited)
>>> + return;
>>> +
>>> + ttm_bo_device_release(&hibmc->ttm.bdev);
>>> + hibmc_ttm_global_release(hibmc);
>>> + hibmc->mm_inited = false;
>>> +}
>>> +
>>> +int hibmc_bo_create(struct drm_device *dev, int size, int align,
>>> + u32 flags, struct hibmc_bo **phibmcbo)
>>> +{
>>> + struct hibmc_drm_device *hibmc = dev->dev_private;
>>> + struct hibmc_bo *hibmcbo;
>>> + size_t acc_size;
>>> + int ret;
>>> +
>>> + hibmcbo = kzalloc(sizeof(*hibmcbo), GFP_KERNEL);
>>> + if (!hibmcbo)
>>> + return -ENOMEM;
>>> +
>>> + ret = drm_gem_object_init(dev, &hibmcbo->gem, size);
>>> + if (ret) {
>>> + kfree(hibmcbo);
>>> + return ret;
>>> + }
>>> +
>>> + hibmcbo->bo.bdev = &hibmc->ttm.bdev;
>>> +
>>> + hibmc_ttm_placement(hibmcbo, TTM_PL_FLAG_VRAM |
>>> TTM_PL_FLAG_SYSTEM);
>>> +
>>> + acc_size = ttm_bo_dma_acc_size(&hibmc->ttm.bdev, size,
>>> + sizeof(struct hibmc_bo));
>>> +
>>> + ret = ttm_bo_init(&hibmc->ttm.bdev, &hibmcbo->bo, size,
>>> + ttm_bo_type_device, &hibmcbo->placement,
>>> + align >> PAGE_SHIFT, false, NULL, acc_size,
>>> + NULL, NULL, hibmc_bo_ttm_destroy);
>>> + if (ret)
>>
>>
>> Missing hibmcbo clean up here
>
>
> i looked at all other ttm drivers and all of them return directly when
> ttm_bo_init
> failed, however, i think it is better to clean up here, should i call
> hibmc_bo_unref(&hibmc_bo) here ?
>
Yeah, that should work (might want to test it, though ;)
>>
>>> + return ret;
>>> +
>>> + *phibmcbo = hibmcbo;
>>> + return 0;
>>> +}
>>> +
>>> +static inline u64 hibmc_bo_gpu_offset(struct hibmc_bo *bo)
>>> +{
>>> + return bo->bo.offset;
>>> +}
>>
>>
>> I don't think this function provides any value
>
>
> do you nean i use bo->bo.offset instead of calling hibmc_bo_gpu_offset()?
>
yes
>>
>>> +
>>> +int hibmc_bo_pin(struct hibmc_bo *bo, u32 pl_flag, u64 *gpu_addr)
>>> +{
>>> + int i, ret;
>>> +
>>> + if (bo->pin_count) {
>>> + bo->pin_count++;
>>> + if (gpu_addr)
>>> + *gpu_addr = hibmc_bo_gpu_offset(bo);
>>
>>
>> Are you missing a return here?
>
>
> Thanks for pointing it out!
>
>
>>
>>> + }
>>> +
>>> + hibmc_ttm_placement(bo, pl_flag);
>>> + for (i = 0; i < bo->placement.num_placement; i++)
>>> + bo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
>>> + ret = ttm_bo_validate(&bo->bo, &bo->placement, false, false);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + bo->pin_count = 1;
>>> + if (gpu_addr)
>>> + *gpu_addr = hibmc_bo_gpu_offset(bo);
>>> + return 0;
>>> +}
>>> +
>>> +int hibmc_bo_push_sysram(struct hibmc_bo *bo)
>>> +{
>>> + int i, ret;
>>> +
>>> + if (!bo->pin_count) {
>>> + DRM_ERROR("unpin bad %p\n", bo);
>>> + return 0;
>>> + }
>>> + bo->pin_count--;
>>> + if (bo->pin_count)
>>> + return 0;
>>> +
>>> + if (bo->kmap.virtual)
>>
>>
>> ttm_bo_kunmap already does this check so you don't have to
>
>
> agreed. will remove this condition.
>
>>
>>> + ttm_bo_kunmap(&bo->kmap);
>>> +
>>> + hibmc_ttm_placement(bo, TTM_PL_FLAG_SYSTEM);
>>> + for (i = 0; i < bo->placement.num_placement ; i++)
>>> + bo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
>>> +
>>> + ret = ttm_bo_validate(&bo->bo, &bo->placement, false, false);
>>> + if (ret) {
>>> + DRM_ERROR("pushing to VRAM failed\n");
>>
>>
>> Print ret
>
>
> ok, thanks.
>
>
>>
>>> + return ret;
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +int hibmc_mmap(struct file *filp, struct vm_area_struct *vma)
>>> +{
>>> + struct drm_file *file_priv;
>>> + struct hibmc_drm_device *hibmc;
>>> +
>>> + if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET))
>>> + return -EINVAL;
>>> +
>>> + file_priv = filp->private_data;
>>> + hibmc = file_priv->minor->dev->dev_private;
>>> + return ttm_bo_mmap(filp, vma, &hibmc->ttm.bdev);
>>> +}
>>> +
>>> +int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel,
>>> + struct drm_gem_object **obj)
>>> +{
>>> + struct hibmc_bo *hibmcbo;
>>> + int ret;
>>> +
>>> + *obj = NULL;
>>> +
>>> + size = PAGE_ALIGN(size);
>>> + if (size == 0)
>>
>>
>> Print error
>
>
> ditto
>
>>
>>> + return -EINVAL;
>>> +
>>> + ret = hibmc_bo_create(dev, size, 0, 0, &hibmcbo);
>>> + if (ret) {
>>> + if (ret != -ERESTARTSYS)
>>> + DRM_ERROR("failed to allocate GEM object\n");
>>
>>
>> Print ret
>
>
> ditto
>
>>
>>> + return ret;
>>> + }
>>> + *obj = &hibmcbo->gem;
>>> + return 0;
>>> +}
>>> +
>>> +int hibmc_dumb_create(struct drm_file *file, struct drm_device *dev,
>>> + struct drm_mode_create_dumb *args)
>>> +{
>>> + struct drm_gem_object *gobj;
>>> + u32 handle;
>>> + int ret;
>>> +
>>> + args->pitch = ALIGN(args->width * ((args->bpp + 7) / 8), 16);
>>
>>
>> What's up with the bpp + 7 here? Perhaps you're looking for DIV_ROUND_UP?
>
>
> Yes, that sounds sane.
>
sane is what i usually aim for :)
Sean
>>
>>
>>> + args->size = args->pitch * args->height;
>>> +
>>> + ret = hibmc_gem_create(dev, args->size, false,
>>> + &gobj);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = drm_gem_handle_create(file, gobj, &handle);
>>> + drm_gem_object_unreference_unlocked(gobj);
>>> + if (ret)
>>
>>
>> Print error here
>
>
> agreed.
>
>
>>
>>> + return ret;
>>> +
>>> + args->handle = handle;
>>> + return 0;
>>> +}
>>> +
>>> +static void hibmc_bo_unref(struct hibmc_bo **bo)
>>> +{
>>> + struct ttm_buffer_object *tbo;
>>> +
>>> + if ((*bo) == NULL)
>>> + return;
>>> +
>>> + tbo = &((*bo)->bo);
>>> + ttm_bo_unref(&tbo);
>>> + *bo = NULL;
>>> +}
>>> +
>>> +void hibmc_gem_free_object(struct drm_gem_object *obj)
>>> +{
>>> + struct hibmc_bo *hibmcbo = gem_to_hibmc_bo(obj);
>>> +
>>> + hibmc_bo_unref(&hibmcbo);
>>> +}
>>> +
>>> +static u64 hibmc_bo_mmap_offset(struct hibmc_bo *bo)
>>> +{
>>> + return drm_vma_node_offset_addr(&bo->bo.vma_node);
>>> +}
>>> +
>>> +int hibmc_dumb_mmap_offset(struct drm_file *file, struct drm_device
>>> *dev,
>>> + u32 handle, u64 *offset)
>>> +{
>>> + struct drm_gem_object *obj;
>>> + struct hibmc_bo *bo;
>>> +
>>> + obj = drm_gem_object_lookup(file, handle);
>>> + if (!obj)
>>> + return -ENOENT;
>>> +
>>> + bo = gem_to_hibmc_bo(obj);
>>> + *offset = hibmc_bo_mmap_offset(bo);
>>> +
>>> + drm_gem_object_unreference_unlocked(obj);
>>> + return 0;
>>> +}
>
>
> Regards,
> Rongrong.
>
>>> --
>>> 1.9.1
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>> _______________________________________________
>> linuxarm mailing list
>> linuxarm at huawei.com
>> http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm
>>
>> .
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH V6 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping
From: Hanjun Guo @ 2016-11-11 13:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <60c1d53146c0aebd3a05095823229224@codeaurora.org>
On 11/10/2016 11:02 PM, agustinv at codeaurora.org wrote:
> Hey Hanjun,
>
> On 2016-11-09 21:36, Hanjun Guo wrote:
>> Hi Marc, Rafael, Lorenzo,
>>
>> Since we agreed to add a probe deferral if we failed to get irq
>> resources which mirroring the DT does (patch 1 in this patch set),
>> I think the last blocker to make things work both for Agustin and
>> me [1] is this patch, which makes the interrupt producer and consumer
>> work in ACPI, we have two different solution for one thing, we'd happy
>> to work together for one solution, could you give some suggestions
>> please?
>>
>> [1]:
>> https://mail-archive.com/linux-kernel at vger.kernel.org/msg1257419.html
>>
>> Agustin, I have some comments below.
>>
>> On 2016/10/29 4:48, Agustin Vega-Frias wrote:
>>> This allows irqchip drivers to associate an ACPI DSDT device to
>>> an IRQ domain and provides support for using the ResourceSource
>>> in Extended IRQ Resources to find the domain and map the IRQs
>>> specified on that domain.
>>>
>>> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
>>> ---
>>> drivers/acpi/Makefile | 1 +
>>> drivers/acpi/irqdomain.c | 119
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>
>> Could we just reuse the gsi.c and not introduce a new
>> file, probably we can change the gsi.c to irqdomain.c
>> or something similar, then reuse the code in gsi.c.
>
> I was thinking just that after we chatted off-list.
Great.
> I might revisit and see what I come up with given that we already have
> a device argument and we could pass the IRQ source there.
Sorry, I'm little confused here, why we "already have a device
argument"? in drivers/acpi/resource.c, it just use NULL as device
and we can't pass dev directly for the API is using now, could
you explain more?
Thanks
Hanjun
^ permalink raw reply
* [PATCH v6 3/9] drm/hisilicon/hibmc: Add support for frame buffer
From: Sean Paul @ 2016-11-11 13:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5825C496.5070905@huawei.com>
On Fri, Nov 11, 2016 at 8:16 AM, Rongrong Zou <zourongrong@huawei.com> wrote:
> ? 2016/11/11 2:30, Sean Paul ??:
>>
>> On Fri, Oct 28, 2016 at 3:27 AM, Rongrong Zou <zourongrong@gmail.com>
>> wrote:
>>>
>>> Add support for fbdev and kms fb management.
>>>
>>> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
>>> ---
>>> drivers/gpu/drm/hisilicon/hibmc/Makefile | 2 +-
>>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 17 ++
>>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 24 ++
>>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 255
>>> ++++++++++++++++++++++
>>> drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 66 ++++++
>>> 5 files changed, 363 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
>>>
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> index d5c40b8..810a37e 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> @@ -1,5 +1,5 @@
>>> ccflags-y := -Iinclude/drm
>>> -hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_power.o hibmc_ttm.o
>>> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_fbdev.o hibmc_drm_power.o
>>> hibmc_ttm.o
>>>
>>> obj-$(CONFIG_DRM_HISI_HIBMC) +=hibmc-drm.o
>>> #obj-y += hibmc-drm.o
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> index 81f4301..5ac7a7e 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> @@ -66,11 +66,23 @@ static void hibmc_disable_vblank(struct drm_device
>>> *dev, unsigned int pipe)
>>>
>>> static int hibmc_pm_suspend(struct device *dev)
>>> {
>>> + struct pci_dev *pdev = to_pci_dev(dev);
>>> + struct drm_device *drm_dev = pci_get_drvdata(pdev);
>>> + struct hibmc_drm_device *hidev = drm_dev->dev_private;
>>> +
>>> + drm_fb_helper_set_suspend_unlocked(&hidev->fbdev->helper, 1);
>>> +
>>
>>
>> We have atomic helpers for suspend/resume now. Please use those.
>
>
> drm_atomic_helper_suspend/resume()?
>
Correct
>
>>
>>> return 0;
>>> }
>>>
>>> static int hibmc_pm_resume(struct device *dev)
>>> {
>>> + struct pci_dev *pdev = to_pci_dev(dev);
>>> + struct drm_device *drm_dev = pci_get_drvdata(pdev);
>>> + struct hibmc_drm_device *hidev = drm_dev->dev_private;
>>> +
>>> + drm_fb_helper_set_suspend_unlocked(&hidev->fbdev->helper, 0);
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -170,6 +182,7 @@ static int hibmc_unload(struct drm_device *dev)
>>> {
>>> struct hibmc_drm_device *hidev = dev->dev_private;
>>>
>>> + hibmc_fbdev_fini(hidev);
>>> hibmc_mm_fini(hidev);
>>> hibmc_hw_fini(hidev);
>>> dev->dev_private = NULL;
>>> @@ -195,6 +208,10 @@ static int hibmc_load(struct drm_device *dev,
>>> unsigned long flags)
>>> if (ret)
>>> goto err;
>>>
>>> + ret = hibmc_fbdev_init(hidev);
>>> + if (ret)
>>> + goto err;
>>> +
>>> return 0;
>>>
>>> err:
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> index db8d80e..a40e9a7 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> @@ -20,9 +20,22 @@
>>> #define HIBMC_DRM_DRV_H
>>>
>>> #include <drm/drmP.h>
>>> +#include <drm/drm_fb_helper.h>
>>> #include <drm/ttm/ttm_bo_driver.h>
>>> #include <drm/drm_gem.h>
>>>
>>> +struct hibmc_framebuffer {
>>> + struct drm_framebuffer fb;
>>> + struct drm_gem_object *obj;
>>> + bool is_fbdev_fb;
>>> +};
>>> +
>>> +struct hibmc_fbdev {
>>> + struct drm_fb_helper helper;
>>> + struct hibmc_framebuffer *fb;
>>> + int size;
>>> +};
>>> +
>>> struct hibmc_drm_device {
>>> /* hw */
>>> void __iomem *mmio;
>>> @@ -41,9 +54,13 @@ struct hibmc_drm_device {
>>> bool initialized;
>>> } ttm;
>>>
>>> + /* fbdev */
>>> + struct hibmc_fbdev *fbdev;
>>> bool mm_inited;
>>> };
>>>
>>> +#define to_hibmc_framebuffer(x) container_of(x, struct
>>> hibmc_framebuffer, fb)
>>> +
>>> struct hibmc_bo {
>>> struct ttm_buffer_object bo;
>>> struct ttm_placement placement;
>>> @@ -65,8 +82,15 @@ static inline struct hibmc_bo *gem_to_hibmc_bo(struct
>>> drm_gem_object *gem)
>>>
>>> #define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
>>>
>>> +int hibmc_fbdev_init(struct hibmc_drm_device *hidev);
>>> +void hibmc_fbdev_fini(struct hibmc_drm_device *hidev);
>>> +
>>> int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel,
>>> struct drm_gem_object **obj);
>>> +struct hibmc_framebuffer *
>>> +hibmc_framebuffer_init(struct drm_device *dev,
>>> + const struct drm_mode_fb_cmd2 *mode_cmd,
>>> + struct drm_gem_object *obj);
>>>
>>> int hibmc_mm_init(struct hibmc_drm_device *hibmc);
>>> void hibmc_mm_fini(struct hibmc_drm_device *hibmc);
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
>>> new file mode 100644
>>> index 0000000..630124b
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
>>> @@ -0,0 +1,255 @@
>>> +/* Hisilicon Hibmc SoC drm driver
>>> + *
>>> + * Based on the bochs drm driver.
>>> + *
>>> + * Copyright (c) 2016 Huawei Limited.
>>> + *
>>> + * Author:
>>> + * Rongrong Zou <zourongrong@huawei.com>
>>> + * Rongrong Zou <zourongrong@gmail.com>
>>> + * Jianhua Li <lijianhua@huawei.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + */
>>> +
>>> +#include <drm/drm_crtc.h>
>>> +#include <drm/drm_crtc_helper.h>
>>> +#include <drm/drm_fb_helper.h>
>>> +
>>> +#include "hibmc_drm_drv.h"
>>> +
>>> +/*
>>> ---------------------------------------------------------------------- */
>>
>>
>> Please remove
>
>
> will do that, thanks.
>
>
>>
>>> +
>>> +static int hibmcfb_create_object(
>>> + struct hibmc_drm_device *hidev,
>>> + const struct drm_mode_fb_cmd2 *mode_cmd,
>>> + struct drm_gem_object **gobj_p)
>>> +{
>>> + struct drm_gem_object *gobj;
>>> + struct drm_device *dev = hidev->dev;
>>> + u32 size;
>>> + int ret = 0;
>>> +
>>> + size = mode_cmd->pitches[0] * mode_cmd->height;
>>> + ret = hibmc_gem_create(dev, size, true, &gobj);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + *gobj_p = gobj;
>>> + return ret;
>>> +}
>>> +
>>> +static struct fb_ops hibmc_drm_fb_ops = {
>>> + .owner = THIS_MODULE,
>>> + .fb_check_var = drm_fb_helper_check_var,
>>> + .fb_set_par = drm_fb_helper_set_par,
>>> + .fb_fillrect = drm_fb_helper_sys_fillrect,
>>> + .fb_copyarea = drm_fb_helper_sys_copyarea,
>>> + .fb_imageblit = drm_fb_helper_sys_imageblit,
>>> + .fb_pan_display = drm_fb_helper_pan_display,
>>> + .fb_blank = drm_fb_helper_blank,
>>> + .fb_setcmap = drm_fb_helper_setcmap,
>>> +};
>>> +
>>> +static int hibmc_drm_fb_create(struct drm_fb_helper *helper,
>>> + struct drm_fb_helper_surface_size *sizes)
>>> +{
>>> + struct hibmc_fbdev *hi_fbdev =
>>> + container_of(helper, struct hibmc_fbdev, helper);
>>> + struct hibmc_drm_device *hidev =
>>> + (struct hibmc_drm_device *)helper->dev->dev_private;
>>> + struct fb_info *info;
>>> + struct hibmc_framebuffer *hibmc_fb;
>>> + struct drm_framebuffer *fb;
>>> + struct drm_mode_fb_cmd2 mode_cmd;
>>> + struct drm_gem_object *gobj = NULL;
>>> + int ret = 0;
>>> + size_t size;
>>> + unsigned int bytes_per_pixel;
>>> + struct hibmc_bo *bo = NULL;
>>> +
>>> + DRM_DEBUG_DRIVER("surface width(%d), height(%d) and bpp(%d)\n",
>>> + sizes->surface_width, sizes->surface_height,
>>> + sizes->surface_bpp);
>>> + sizes->surface_depth = 32;
>>> +
>>> + bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
>>> +
>>> + mode_cmd.width = sizes->surface_width;
>>> + mode_cmd.height = sizes->surface_height;
>>> + mode_cmd.pitches[0] = mode_cmd.width * bytes_per_pixel;
>>> + mode_cmd.pixel_format =
>>> drm_mode_legacy_fb_format(sizes->surface_bpp,
>>> +
>>> sizes->surface_depth);
>>> +
>>> + size = roundup(mode_cmd.pitches[0] * mode_cmd.height, PAGE_SIZE);
>>
>>
>> It's somewhat curious that you used ALIGN in the previous patch and
>> roundup here. But anyways, I think PAGE_ALIGN would be the most
>> appropriate thing to do here.
>
>
> agreed, thanks.
>
>>
>>> +
>>> + ret = hibmcfb_create_object(hidev, &mode_cmd, &gobj);
>>> + if (ret) {
>>> + DRM_ERROR("failed to create fbcon backing object %d\r\n",
>>
>> ret);
>>
>> \r, yikes!!!
>
>
> will delete it, thanks.
>
>>
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + bo = gem_to_hibmc_bo(gobj);
>>> +
>>> + ret = ttm_bo_reserve(&bo->bo, true, false, NULL);
>>> + if (ret)
>>
>>
>> Print error here?
>
>
> will do.
>
>>
>> How about cleaning up gobj?
>
>
> you are right,
>
>>
>>> + return ret;
>>> +
>>> + ret = hibmc_bo_pin(bo, TTM_PL_FLAG_VRAM, NULL);
>>> + if (ret) {
>>> + DRM_ERROR("failed to pin fbcon\n");
>>
>>
>> Print ret
>>
>> ttm_bo_unreserve? It seems like you're missing clean-up in all of the
>> error paths in this function. Can you please make sure everything is
>> tidied up?
>
>
> ok, thanks.
>
>>
>>> + return ret;
>>> + }
>>> +
>>> + ret = ttm_bo_kmap(&bo->bo, 0, bo->bo.num_pages, &bo->kmap);
>>> +
>>
>>
>> nit: extra space
>
>
> do you mean extra line?
>
yes, apologies.
>>
>>> + if (ret) {
>>> + DRM_ERROR("failed to kmap fbcon\n");
>>
>>
>> Print ret
>
>
> ok, thanks.
>
>>
>>> + ttm_bo_unreserve(&bo->bo);
>>> + return ret;
>>> + }
>>> +
>>> + ttm_bo_unreserve(&bo->bo);
>>
>>
>> Move this between ttm_bo_kmap and if (ret), then remove it from inside
>> the conditional.
>
>
> This is fine with me, thanks.
>
>>
>>> +
>>> + info = drm_fb_helper_alloc_fbi(helper);
>>> + if (IS_ERR(info))
>>
>>
>> Print error
>
>
> ok, thanks.
>
>>
>>> + return PTR_ERR(info);
>>> +
>>> + info->par = hi_fbdev;
>>> +
>>> + hibmc_fb = hibmc_framebuffer_init(hidev->dev, &mode_cmd, gobj);
>>> + if (IS_ERR(hibmc_fb)) {
>>> + drm_fb_helper_release_fbi(helper);
>>> + return PTR_ERR(hibmc_fb);
>>> + }
>>> +
>>> + hi_fbdev->fb = hibmc_fb;
>>> + hidev->fbdev->size = size;
>>> + fb = &hibmc_fb->fb;
>>
>>
>> The fb variable isn't necessary, and really, the hibmc_fb doesn't
>> really help either, it just masks what's actually happening IMO.
>
>
> i will clean the two variables.
>
>
>>
>>> + if (!fb) {
>>> + DRM_INFO("fb is NULL\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + hi_fbdev->helper.fb = fb;
>>> +
>>> + strcpy(info->fix.id, "hibmcdrmfb");
>>> +
>>> + info->flags = FBINFO_DEFAULT;
>>> + info->fbops = &hibmc_drm_fb_ops;
>>> +
>>> + drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
>>> + drm_fb_helper_fill_var(info, &hidev->fbdev->helper,
>>> sizes->fb_width,
>>> + sizes->fb_height);
>>> +
>>> + info->screen_base = bo->kmap.virtual;
>>> + info->screen_size = size;
>>> +
>>> + info->fix.smem_start = bo->bo.mem.bus.offset +
>>> bo->bo.mem.bus.base;
>>> + info->fix.smem_len = size;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void hibmc_fbdev_destroy(struct hibmc_fbdev *fbdev)
>>> +{
>>> + struct hibmc_framebuffer *gfb = fbdev->fb;
>>> + struct drm_fb_helper *fbh = &fbdev->helper;
>>> +
>>> + DRM_DEBUG_DRIVER("hibmc_fbdev_destroy\n");
>>
>>
>> Not useful
>
>
> ok, will remove.
>
>
>>
>>> +
>>> + drm_fb_helper_unregister_fbi(fbh);
>>> + drm_fb_helper_release_fbi(fbh);
>>> +
>>> + drm_fb_helper_fini(fbh);
>>> +
>>> + if (gfb)
>>> + drm_framebuffer_unreference(&gfb->fb);
>>> +
>>> + kfree(fbdev);
>>> +}
>>> +
>>> +static const struct drm_fb_helper_funcs hibmc_fbdev_helper_funcs = {
>>> + .fb_probe = hibmc_drm_fb_create,
>>> +};
>>> +
>>> +int hibmc_fbdev_init(struct hibmc_drm_device *hidev)
>>> +{
>>> + int ret;
>>> + struct fb_var_screeninfo *var;
>>> + struct fb_fix_screeninfo *fix;
>>> + struct hibmc_fbdev *hifbdev;
>>> +
>>> + hifbdev = kzalloc(sizeof(*hifbdev), GFP_KERNEL);
>>
>>
>> devm_kzalloc?
>
>
> sounds good, so there need no kfree at hibmc_fbdev_destroy(),
> thanks.
>
yep, exactly
>>
>>> + if (!hifbdev)
>>> + return -ENOMEM;
>>> +
>>> + hidev->fbdev = hifbdev;
>>> + drm_fb_helper_prepare(hidev->dev, &hifbdev->helper,
>>> + &hibmc_fbdev_helper_funcs);
>>> +
>>> + /* Now just one crtc and one channel */
>>> + ret = drm_fb_helper_init(hidev->dev,
>>> + &hifbdev->helper, 1, 1);
>>> +
>>
>>
>> nit: extra line
>
>
> ok, thanks.
>
>
>>
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = drm_fb_helper_single_add_all_connectors(&hifbdev->helper);
>>> + if (ret)
>>> + goto fini;
>>> +
>>> + ret = drm_fb_helper_initial_config(&hifbdev->helper, 16);
>>> + if (ret)
>>> + goto fini;
>>> +
>>> + var = &hifbdev->helper.fbdev->var;
>>> + fix = &hifbdev->helper.fbdev->fix;
>>> +
>>> + DRM_DEBUG("Member of info->var is :\n"
>>> + "xres=%d\n"
>>> + "yres=%d\n"
>>> + "xres_virtual=%d\n"
>>> + "yres_virtual=%d\n"
>>> + "xoffset=%d\n"
>>> + "yoffset=%d\n"
>>> + "bits_per_pixel=%d\n"
>>> + "...\n", var->xres, var->yres, var->xres_virtual,
>>> + var->yres_virtual, var->xoffset, var->yoffset,
>>> + var->bits_per_pixel);
>>> + DRM_DEBUG("Member of info->fix is :\n"
>>> + "smem_start=%lx\n"
>>> + "smem_len=%d\n"
>>> + "type=%d\n"
>>> + "type_aux=%d\n"
>>> + "visual=%d\n"
>>> + "xpanstep=%d\n"
>>> + "ypanstep=%d\n"
>>> + "ywrapstep=%d\n"
>>> + "line_length=%d\n"
>>> + "accel=%d\n"
>>> + "capabilities=%d\n"
>>> + "...\n", fix->smem_start, fix->smem_len, fix->type,
>>> + fix->type_aux, fix->visual, fix->xpanstep,
>>> + fix->ypanstep, fix->ywrapstep, fix->line_length,
>>> + fix->accel, fix->capabilities);
>>
>>
>> You've been using DRM_DEBUG_DRIVER elsewhere, you should probably use
>> it here, too.
>
>
> ok, thanks.
>
>
>>
>>> +
>>> + return 0;
>>> +
>>> +fini:
>>> + drm_fb_helper_fini(&hifbdev->helper);
>>> + return ret;
>>> +}
>>> +
>>> +void hibmc_fbdev_fini(struct hibmc_drm_device *hidev)
>>> +{
>>> + if (!hidev->fbdev)
>>> + return;
>>> +
>>> + hibmc_fbdev_destroy(hidev->fbdev);
>>> + hidev->fbdev = NULL;
>>> +}
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>>> index 0802ebd..9822f62 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>>> @@ -488,3 +488,69 @@ int hibmc_dumb_mmap_offset(struct drm_file *file,
>>> struct drm_device *dev,
>>> drm_gem_object_unreference_unlocked(obj);
>>> return 0;
>>> }
>>> +
>>> +/*
>>> ---------------------------------------------------------------------- */
>>> +
>>
>>
>> Please remove
>
>
> will do.
>
>>
>>> +static void hibmc_user_framebuffer_destroy(struct drm_framebuffer *fb)
>>> +{
>>> + struct hibmc_framebuffer *hibmc_fb = to_hibmc_framebuffer(fb);
>>> +
>>> + drm_gem_object_unreference_unlocked(hibmc_fb->obj);
>>> + drm_framebuffer_cleanup(fb);
>>> + kfree(hibmc_fb);
>>> +}
>>> +
>>> +static const struct drm_framebuffer_funcs hibmc_fb_funcs = {
>>> + .destroy = hibmc_user_framebuffer_destroy,
>>> +};
>>> +
>>> +struct hibmc_framebuffer *
>>> +hibmc_framebuffer_init(struct drm_device *dev,
>>> + const struct drm_mode_fb_cmd2 *mode_cmd,
>>> + struct drm_gem_object *obj)
>>> +{
>>> + struct hibmc_framebuffer *hibmc_fb;
>>> + int ret;
>>> +
>>> + hibmc_fb = kzalloc(sizeof(*hibmc_fb), GFP_KERNEL);
>>> + if (!hibmc_fb)
>>
>>
>> Print error
>
>
> ok, thanks.
>
> Regards,
> Rongrong.
>
>>
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + drm_helper_mode_fill_fb_struct(&hibmc_fb->fb, mode_cmd);
>>> + hibmc_fb->obj = obj;
>>> + ret = drm_framebuffer_init(dev, &hibmc_fb->fb, &hibmc_fb_funcs);
>>> + if (ret) {
>>> + DRM_ERROR("drm_framebuffer_init failed: %d\n", ret);
>>> + kfree(hibmc_fb);
>>> + return ERR_PTR(ret);
>>> + }
>>> +
>>> + return hibmc_fb;
>>> +}
>>> +
>>> +static struct drm_framebuffer *
>>> +hibmc_user_framebuffer_create(struct drm_device *dev,
>>> + struct drm_file *filp,
>>> + const struct drm_mode_fb_cmd2 *mode_cmd)
>>> +{
>>> + struct drm_gem_object *obj;
>>> + struct hibmc_framebuffer *hibmc_fb;
>>> +
>>> + DRM_DEBUG_DRIVER("%dx%d, format %c%c%c%c\n",
>>> + mode_cmd->width, mode_cmd->height,
>>> + (mode_cmd->pixel_format) & 0xff,
>>> + (mode_cmd->pixel_format >> 8) & 0xff,
>>> + (mode_cmd->pixel_format >> 16) & 0xff,
>>> + (mode_cmd->pixel_format >> 24) & 0xff);
>>> +
>>> + obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);
>>> + if (!obj)
>>> + return ERR_PTR(-ENOENT);
>>> +
>>> + hibmc_fb = hibmc_framebuffer_init(dev, mode_cmd, obj);
>>> + if (IS_ERR(hibmc_fb)) {
>>> + drm_gem_object_unreference_unlocked(obj);
>>> + return ERR_PTR((long)hibmc_fb);
>>> + }
>>> + return &hibmc_fb->fb;
>>> +}
>>> --
>>> 1.9.1
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>> _______________________________________________
>> linuxarm mailing list
>> linuxarm at huawei.com
>> http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm
>>
>> .
>>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH V6 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping
From: Hanjun Guo @ 2016-11-11 13:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161110175814.GA12446@red-moon>
Hi Lorenzo,
On 11/11/2016 01:58 AM, Lorenzo Pieralisi wrote:
> On Thu, Nov 10, 2016 at 10:02:35AM -0500, agustinv at codeaurora.org wrote:
>> Hey Hanjun,
>>
>> On 2016-11-09 21:36, Hanjun Guo wrote:
>>> Hi Marc, Rafael, Lorenzo,
>>>
>>> Since we agreed to add a probe deferral if we failed to get irq
>>> resources which mirroring the DT does (patch 1 in this patch set),
>>> I think the last blocker to make things work both for Agustin and
>>> me [1] is this patch, which makes the interrupt producer and consumer
>>> work in ACPI, we have two different solution for one thing, we'd happy
>>> to work together for one solution, could you give some suggestions
>>> please?
>>>
>>> [1]: https://mail-archive.com/linux-kernel at vger.kernel.org/msg1257419.html
>>>
>>> Agustin, I have some comments below.
>>>
>>> On 2016/10/29 4:48, Agustin Vega-Frias wrote:
>>>> This allows irqchip drivers to associate an ACPI DSDT device to
>>>> an IRQ domain and provides support for using the ResourceSource
>>>> in Extended IRQ Resources to find the domain and map the IRQs
>>>> specified on that domain.
>>>>
>>>> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
>>>> ---
>>>> drivers/acpi/Makefile | 1 +
>>>> drivers/acpi/irqdomain.c | 119
>>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>
>>> Could we just reuse the gsi.c and not introduce a new
>>> file, probably we can change the gsi.c to irqdomain.c
>>> or something similar, then reuse the code in gsi.c.
>>
>> I was thinking just that after we chatted off-list.
>
> Yes, that's a fair point.
>
>> I might revisit and see what I come up with given that we already have
>> a device argument and we could pass the IRQ source there.
>
> I agree with the approach taken by this patch, I do not like much
> passing around struct acpi_resource_source *source (in particular
> the dummy struct) I do not think it is needed, I will comment on
> the code.
thanks for your time to have a look:)
>
> Hopefully there is not any buggy FW out there that does use the
> resource source inappropriately otherwise we will notice on x86/ia64
> (ie you can't blame FW if it breaks the kernel) but I suspect the
> only way to find out is by trying, the patch has to go through Rafael's
> review anyway before getting there so it is fine.
I think we can avoid that by not touching the logic that x86/ia64
already used, but only adding interrupt producer/consumer function.
Thanks
Hanjun
^ permalink raw reply
* [PATCH v5 6/8] Documentation: bindings: add compatible specific to legacy SCPI protocol
From: Rob Herring @ 2016-11-11 13:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ed58424e-e538-227a-37b5-b35fbe4f96ba@arm.com>
On Fri, Nov 11, 2016 at 1:48 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> On 10/11/16 19:03, Olof Johansson wrote:
>> On Thu, Nov 10, 2016 at 6:34 AM, Sudeep Holla <sudeep.holla@arm.com>
>> wrote:
[...]
>>> E.g. Amlogic follows most of the legacy protocol though it deviates in
>>> couple of things which we can handle with platform specific compatible
>>> (in the following patch in the series). When another user(Rockchip ?)
>>> make use of this legacy protocol, we can start using those platform
>>> specific compatible for deviations only.
>>>
>>> Is that not acceptable ?
>>
>>
>> If there's no shared legacy feature set, then it's probably less
>> useful to have a shared less precise compatible value.
>>
>
> There is and will be some shared feature set for sure. At the least the
> standard command set will be shared.
>
>> What the main point I was trying to get across was that we shouldn't
>> expand the generic binding with per-vendor compatible fields, instead
>> we should have those as extensions on the side.
>>
>
> Yes I get the point. We will have per-vendor compatibles for handle the
> deviations but generic one to handle the shared set.
>
>> I'm also a little apprehensive of using "legacy", it goes in the same
>> bucket as "misc". At some point 1.0 will be legacy too, etc.
>>
>
> True and I agree, how about "arm,scpi-pre-1.0" instead ?
That's still meaningless. Convince me that multiple implementations
are identical, then we can have a common property. For example, how
many releases did ARM make before 1.0.
Rob
^ permalink raw reply
* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Gabriele Paoloni @ 2016-11-11 13:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <10334260.ztLXZ2Oynd@wuerfel>
Hi Arnd
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 10 November 2016 16:07
> To: Gabriele Paoloni
> Cc: linux-arm-kernel at lists.infradead.org; Yuanzhichang;
> mark.rutland at arm.com; devicetree at vger.kernel.org;
> lorenzo.pieralisi at arm.com; minyard at acm.org; linux-pci at vger.kernel.org;
> benh at kernel.crashing.org; John Garry; will.deacon at arm.com; linux-
> kernel at vger.kernel.org; xuwei (O); Linuxarm; zourongrong at gmail.com;
> robh+dt at kernel.org; kantyzc at 163.com; linux-serial at vger.kernel.org;
> catalin.marinas at arm.com; olof at lixom.net; liviu.dudau at arm.com;
> bhelgaas at googl e.com; zhichang.yuan02 at gmail.com
> Subject: Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on
> Hip06
>
> On Thursday, November 10, 2016 3:36:49 PM CET Gabriele Paoloni wrote:
> >
> > Where should we get the range from? For LPC we know that it is going
> > Work on anything that is not used by PCI I/O space, and this is
> > why we use [0, PCIBIOS_MIN_IO]
>
> It should be allocated the same way we allocate PCI config space
> segments. This is currently done with the io_range list in
> drivers/pci/pci.c, which isn't perfect but could be extended
> if necessary. Based on what others commented here, I'd rather
> make the differences between ISA/LPC and PCI I/O ranges smaller
> than larger.
I am not sure this would make sense...
IMHO all the mechanism around io_range_list is needed to provide the
"mapping" between I/O tokens and physical CPU addresses.
Currently the available tokens range from 0 to IO_SPACE_LIMIT.
As you know the I/O memory accessors operate on whatever
__of_address_to_resource sets into the resource (start, end).
With this special device in place we cannot know if a resource is
assigned with an I/O token or a physical address, unless we forbid
the I/O tokens to be in a specific range.
So this is why we are changing the offsets of all the functions
handling io_range_list (to make sure that a range is forbidden to
the tokens and is available to the physical addresses).
We have chosen this forbidden range to be [0, PCIBIOS_MIN_IO)
because this is the maximum physical I/O range that a non PCI device
can operate on and because we believe this does not impose much
restriction on the available I/O token range; that now is
[PCIBIOS_MIN_IO, IO_SPACE_LIMIT].
So we believe that the chosen forbidden range can accommodate
any special ISA bus device with no much constraint on the rest
of I/O tokens...
>
> > > Your current version has
> > >
> > > if (arm64_extio_ops->pfout) \
> > > arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> > > addr, value, sizeof(type)); \
> > >
> > > Instead, just subtract the start of the range from the logical
> > > port number to transform it back into a bus-local port number:
> >
> > These accessors do not operate on IO tokens:
> >
> > If (arm64_extio_ops->start > addr || arm64_extio_ops->end < addr)
> > addr is not going to be an I/O token; in fact patch 2/3 imposes that
> > the I/O tokens will start at PCIBIOS_MIN_IO. So from 0 to
> PCIBIOS_MIN_IO
> > we have free physical addresses that the accessors can operate on.
>
> Ah, I missed that part. I'd rather not use PCIBIOS_MIN_IO to refer to
> the logical I/O tokens, the purpose of that macro is really meant
> for allocating PCI I/O port numbers within the address space of
> one bus.
As I mentioned above, special devices operate on CPU addresses directly,
not I/O tokens. For them there is no way to distinguish....
>
> Note that it's equally likely that whichever next platform needs
> non-mapped I/O access like this actually needs them for PCI I/O space,
> and that will use it on addresses registered to a PCI host bridge.
Ok so here you are talking about a platform that has got an I/O range
under the PCI host controller, right?
And this I/O range cannot be directly memory mapped but needs special
redirections for the I/O tokens, right?
In this scenario registering the I/O ranges with the forbidden range
implemented by the current patch would still allow to redirect I/O
tokens as long as arm64_extio_ops->start >= PCIBIOS_MIN_IO
So effectively the special PCI host controller
1) knows the physical range that needs special redirection
2) register such range
3) uses pci_pio_to_address() to retrieve the IO tokens for the
special accessors
4) sets arm64_extio_ops->start/end to the IO tokens retrieved in 3)
So to be honest I think this patch can fit well both with
special PCI controllers that need I/O tokens redirection and with
special non-PCI controllers that need non-PCI I/O physical
address redirection...
Thanks (and sorry for the long reply but I didn't know how
to make the explanation shorter :) )
Gab
>
> If we separate the two steps:
>
> a) assign a range of logical I/O port numbers to a bus
> b) register a set of helpers for redirecting logical I/O
> port to a helper function
>
> then I think the code will get cleaner and more flexible.
> It should actually then be able to replace the powerpc
> specific implementation.
>
> Arnd
^ permalink raw reply
* [PATCH v3 0/2] arm64: Support systems without FP/ASIMD
From: Marc Zyngier @ 2016-11-11 13:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478613381-5718-1-git-send-email-suzuki.poulose@arm.com>
On 08/11/16 13:56, Suzuki K Poulose wrote:
> This series adds supports to the kernel and KVM hyp to handle
> systems without FP/ASIMD properly. At the moment the kernel
> doesn't check if the FP unit is available before accessing
> the registers (e.g during context switch). Also for KVM,
> we trap the FP/ASIMD accesses and handle it by injecting an
> undefined instruction into the VM on systems without FP.
>
> Tested on a FVP_Base-AEM-v8A model by disabling VFP on at
> least one CPU ( -C clusterX.cpuY.vfp-present=0 ).
>
> Changes since V2:
> - Dropped cleanup patch for arm64/crypto/aes-ce-ccm-glue.c
> - Removed static_key check from cpus_have_cap. All users with
> constant caps should use the new API to make use of static_keys.
> - Removed a dedicated static_key used in irqchip-gic-v3.c for
> Cavium errata with the new API.
>
> Applies on v4.9-rc4 + [1] (which is pushed for rc5)
>
> [1] http://marc.info/?l=linux-arm-kernel&m=147819889813214&w=2
>
>
> Suzuki K Poulose (2):
> arm64: Add hypervisor safe helper for checking constant capabilities
> arm64: Support systems without FP/ASIMD
>
> arch/arm64/include/asm/cpucaps.h | 3 ++-
> arch/arm64/include/asm/cpufeature.h | 24 +++++++++++++++++-------
> arch/arm64/include/asm/neon.h | 3 ++-
> arch/arm64/kernel/cpufeature.c | 17 ++++++++++++++++-
> arch/arm64/kernel/fpsimd.c | 14 ++++++++++++++
> arch/arm64/kernel/process.c | 2 +-
> arch/arm64/kvm/handle_exit.c | 11 +++++++++++
> arch/arm64/kvm/hyp/hyp-entry.S | 9 ++++++++-
> arch/arm64/kvm/hyp/switch.c | 5 ++++-
> drivers/irqchip/irq-gic-v3.c | 13 +------------
> 10 files changed, 76 insertions(+), 25 deletions(-)
>
For the series:
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
How do we plan on merging this? Catalin, are you willing to take it all?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v14 4/9] acpi/arm64: Add GTDT table parse driver
From: Hanjun Guo @ 2016-11-11 13:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CADyBb7tLJpPiMqWs4KZE=uCpo3HX3WcNVju2UKNuPOR0+M4UDw@mail.gmail.com>
On 10/26/2016 07:10 PM, Fu Wei wrote:
> Hi Mark,
>
> On 21 October 2016 at 00:37, Mark Rutland <mark.rutland@arm.com> wrote:
>> Hi,
>>
>> As a heads-up, on v4.9-rc1 I see conflicts at least against
>> arch/arm64/Kconfig. Luckily git am -3 seems to be able to fix that up
>> automatically, but this will need to be rebased before the next posting
>> and/or merging.
>>
>> On Thu, Sep 29, 2016 at 02:17:12AM +0800, fu.wei at linaro.org wrote:
>>> +static int __init map_gt_gsi(u32 interrupt, u32 flags)
>>> +{
>>> + int trigger, polarity;
>>> +
>>> + if (!interrupt)
>>> + return 0;
>>
>> Urgh.
>>
>> Only the secure interrupt (which we do not need) is optional in this
>> manner, and (hilariously), zero appears to also be a valid GSIV, per
>> figure 5-24 in the ACPI 6.1 spec.
>>
>> So, I think that:
>>
>> (a) we should not bother parsing the secure interrupt
>
> If I understand correctly, from this point of view, kernel don't
> handle the secure interrupt.
> But the current arm_arch_timer driver still enable/disable/request
> PHYS_SECURE_PPI
> with PHYS_NONSECURE_PPI.
> That means we still need to parse the secure interrupt.
> Please correct me, if I misunderstand something? :-)
>
>> (b) we should drop the check above
>
> yes, if zero is a valid GSIV, this makes sense.
>
>> (c) we should report the spec issue to the ASWG
>>
>>> +/*
>>> + * acpi_gtdt_c3stop - got c3stop info from GTDT
>>> + *
>>> + * Returns 1 if the timer is powered in deep idle state, 0 otherwise.
>>> + */
>>> +bool __init acpi_gtdt_c3stop(void)
>>> +{
>>> + struct acpi_table_gtdt *gtdt = acpi_gtdt_desc.gtdt;
>>> +
>>> + return !(gtdt->non_secure_el1_flags & ACPI_GTDT_ALWAYS_ON);
>>> +}
>>
>> It looks like this can differ per interrupt. Shouldn't we check the
>> appropriate one?
>
> yes, I think you are right.
I think Mark already clarified this it's a global flag which defined
in the spec, and we don't need to update it.
Thanks
Hanjun
^ 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