Re: [PATCH v5 1/5] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure
Hello Adrian,
I am testing hese changes on my STM32F769-DISCO and i found that:
On Mon, Mar 30, 2020 at 2:35 PM Adrian Ratiu wrote:
>
> In order to support multiple versions of the Synopsis MIPI DSI host
> controller, which have different register layouts but almost identical
> HW protocols, we add a regmap infrastructure which can abstract away
> register accesses for platform drivers using the bridge.
>
> The controller HW revision is detected during bridge probe which will
> be used in future commits to load the relevant register layout which
> the bridge will use transparently to the platform drivers.
>
> Signed-off-by: Adrian Ratiu
> ---
> New in v5.
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 208 ++
> 1 file changed, 117 insertions(+), 91 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 5ef0f154aa7b..6d9e2f21c9cc 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -227,6 +228,7 @@ struct dw_mipi_dsi {
> struct drm_bridge *panel_bridge;
> struct device *dev;
> void __iomem *base;
> + struct regmap *regs;
>
> struct clk *pclk;
>
> @@ -235,6 +237,7 @@ struct dw_mipi_dsi {
> u32 lanes;
> u32 format;
> unsigned long mode_flags;
> + u32 hw_version;
>
> #ifdef CONFIG_DEBUG_FS
> struct dentry *debugfs;
> @@ -249,6 +252,13 @@ struct dw_mipi_dsi {
> const struct dw_mipi_dsi_plat_data *plat_data;
> };
>
> +static const struct regmap_config dw_mipi_dsi_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .name = "dw-mipi-dsi",
> +};
> +
> /*
> * Check if either a link to a master or slave is present
> */
> @@ -280,16 +290,6 @@ static inline struct dw_mipi_dsi *bridge_to_dsi(struct
> drm_bridge *bridge)
> return container_of(bridge, struct dw_mipi_dsi, bridge);
> }
>
> -static inline void dsi_write(struct dw_mipi_dsi *dsi, u32 reg, u32 val)
> -{
> - writel(val, dsi->base + reg);
> -}
> -
> -static inline u32 dsi_read(struct dw_mipi_dsi *dsi, u32 reg)
> -{
> - return readl(dsi->base + reg);
> -}
> -
> static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
>struct mipi_dsi_device *device)
> {
> @@ -366,29 +366,29 @@ static void dw_mipi_message_config(struct dw_mipi_dsi
> *dsi,
> if (lpm)
> val |= CMD_MODE_ALL_LP;
>
> - dsi_write(dsi, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
> - dsi_write(dsi, DSI_CMD_MODE_CFG, val);
> + regmap_write(dsi->regs, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
> + regmap_write(dsi->regs, DSI_CMD_MODE_CFG, val);
> }
>
> static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32
> hdr_val)
> {
> int ret;
> - u32 val, mask;
> + u32 val = 0, mask;
>
> - ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> -val, !(val & GEN_CMD_FULL), 1000,
> -CMD_PKT_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> + val, !(val & GEN_CMD_FULL), 1000,
> + CMD_PKT_STATUS_TIMEOUT_US);
> if (ret) {
> dev_err(dsi->dev, "failed to get available command FIFO\n");
> return ret;
> }
>
> - dsi_write(dsi, DSI_GEN_HDR, hdr_val);
> + regmap_write(dsi->regs, DSI_GEN_HDR, hdr_val);
>
> mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY;
> - ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> -val, (val & mask) == mask,
> -1000, CMD_PKT_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> + val, (val & mask) == mask,
> + 1000, CMD_PKT_STATUS_TIMEOUT_US);
> if (ret) {
> dev_err(dsi->dev, "failed to write command FIFO\n");
> return ret;
> @@ -403,24 +403,26 @@ static int dw_mipi_dsi_write(struct dw_mipi_dsi *dsi,
> const u8 *tx_buf = packet->payload;
> int len = packet->payload_length, pld_data_bytes = sizeof(u32), ret;
> __le32 word;
> - u32 val;
> + u32 val = 0;
>
> while (len) {
> if (len < pld_data_bytes) {
> word = 0;
> memcpy(, tx_buf, len);
> - dsi_write(dsi, DSI_GEN_PLD_DATA, le32_to_cpu(word));
> + regmap_write(dsi->regs, DSI_GEN_PLD_DATA,
> +le32_to_cpu(word));
>
Re: [PATCH v5 1/5] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure
Forgot to mention mention DSI version 1.1, see ref manual for
STM32F769 for more details:
https://www.google.com/url?sa=t=j==s=web=1=2ahUKEwiRm4mhy8LoAhUP4BoKHaiLAJcQFjAAegQIBRAB=https%3A%2F%2Fwww.st.com%2Fresource%2Fen%2Freference_manual%2Fdm00224583-stm32f76xxx-and-stm32f77xxx-advanced-armbased-32bit-mcus-stmicroelectronics.pdf=AOvVaw14nl_UqwBs39ORzC0yaope
On Mon, Mar 30, 2020 at 6:58 PM adrian61 wrote:
>
> Hello Adrian,
>
> I am testing hese changes on my STM32F769-DISCO and i found that:
>
> On Mon, Mar 30, 2020 at 2:35 PM Adrian Ratiu
> wrote:
> >
> > In order to support multiple versions of the Synopsis MIPI DSI host
> > controller, which have different register layouts but almost identical
> > HW protocols, we add a regmap infrastructure which can abstract away
> > register accesses for platform drivers using the bridge.
> >
> > The controller HW revision is detected during bridge probe which will
> > be used in future commits to load the relevant register layout which
> > the bridge will use transparently to the platform drivers.
> >
> > Signed-off-by: Adrian Ratiu
> > ---
> > New in v5.
> > ---
> > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 208 ++
> > 1 file changed, 117 insertions(+), 91 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > index 5ef0f154aa7b..6d9e2f21c9cc 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > @@ -15,6 +15,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #include
> > @@ -227,6 +228,7 @@ struct dw_mipi_dsi {
> > struct drm_bridge *panel_bridge;
> > struct device *dev;
> > void __iomem *base;
> > + struct regmap *regs;
> >
> > struct clk *pclk;
> >
> > @@ -235,6 +237,7 @@ struct dw_mipi_dsi {
> > u32 lanes;
> > u32 format;
> > unsigned long mode_flags;
> > + u32 hw_version;
> >
> > #ifdef CONFIG_DEBUG_FS
> > struct dentry *debugfs;
> > @@ -249,6 +252,13 @@ struct dw_mipi_dsi {
> > const struct dw_mipi_dsi_plat_data *plat_data;
> > };
> >
> > +static const struct regmap_config dw_mipi_dsi_regmap_cfg = {
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .name = "dw-mipi-dsi",
> > +};
> > +
> > /*
> > * Check if either a link to a master or slave is present
> > */
> > @@ -280,16 +290,6 @@ static inline struct dw_mipi_dsi *bridge_to_dsi(struct
> > drm_bridge *bridge)
> > return container_of(bridge, struct dw_mipi_dsi, bridge);
> > }
> >
> > -static inline void dsi_write(struct dw_mipi_dsi *dsi, u32 reg, u32 val)
> > -{
> > - writel(val, dsi->base + reg);
> > -}
> > -
> > -static inline u32 dsi_read(struct dw_mipi_dsi *dsi, u32 reg)
> > -{
> > - return readl(dsi->base + reg);
> > -}
> > -
> > static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> >struct mipi_dsi_device *device)
> > {
> > @@ -366,29 +366,29 @@ static void dw_mipi_message_config(struct dw_mipi_dsi
> > *dsi,
> > if (lpm)
> > val |= CMD_MODE_ALL_LP;
> >
> > - dsi_write(dsi, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
> > - dsi_write(dsi, DSI_CMD_MODE_CFG, val);
> > + regmap_write(dsi->regs, DSI_LPCLK_CTRL, lpm ? 0 :
> > PHY_TXREQUESTCLKHS);
> > + regmap_write(dsi->regs, DSI_CMD_MODE_CFG, val);
> > }
> >
> > static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32
> > hdr_val)
> > {
> > int ret;
> > - u32 val, mask;
> > + u32 val = 0, mask;
> >
> > - ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> > -val, !(val & GEN_CMD_FULL), 1000,
> > -CMD_PKT_STATUS_TIMEOUT_US);
> > + ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> > + val, !(val & GEN_CMD_FULL), 1000,
> > + CMD_PKT_STATUS_TIMEOUT_US);
> > if (ret) {
> > dev_err(dsi->dev, "failed to get available command FIFO\n");
> > return ret;
> > }
> >
> > - dsi_write(dsi, DSI_GEN_HDR, hdr_val);
> > + regmap_write(dsi->regs, DSI_GEN_HDR, hdr_val);
> >
> > mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY;
> > - ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> > -val, (val & mask) == mask,
> > -1000, CMD_PKT_STATUS_TIMEOUT_US);
> > + ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> > + val, (val & mask) == mask,
> > + 1000, CMD_PKT_STATUS_TIMEOUT_US);
> > if (ret) {
> > dev_err(dsi->dev, "failed to write command FIFO\n");
> >
Re: [PATCH v5 1/5] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure
On Mon, 30 Mar 2020, adrian61 wrote:
Hello Adrian,
I am testing hese changes on my STM32F769-DISCO and i found
that:
On Mon, Mar 30, 2020 at 2:35 PM Adrian Ratiu
wrote:
In order to support multiple versions of the Synopsis MIPI DSI
host controller, which have different register layouts but
almost identical HW protocols, we add a regmap infrastructure
which can abstract away register accesses for platform drivers
using the bridge.
The controller HW revision is detected during bridge probe
which will be used in future commits to load the relevant
register layout which the bridge will use transparently to the
platform drivers.
Signed-off-by: Adrian Ratiu ---
New in v5. ---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 208
++ 1 file changed, 117 insertions(+), 91
deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c index
5ef0f154aa7b..6d9e2f21c9cc 100644 ---
a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c +++
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c @@ -15,6 +15,7
@@
#include #include
#include
+#include
#include
#include
@@ -227,6 +228,7 @@ struct dw_mipi_dsi {
struct drm_bridge *panel_bridge; struct device *dev;
void __iomem *base;
+ struct regmap *regs;
struct clk *pclk;
@@ -235,6 +237,7 @@ struct dw_mipi_dsi {
u32 lanes; u32 format; unsigned long mode_flags;
+ u32 hw_version;
#ifdef CONFIG_DEBUG_FS
struct dentry *debugfs;
@@ -249,6 +252,13 @@ struct dw_mipi_dsi {
const struct dw_mipi_dsi_plat_data *plat_data;
};
+static const struct regmap_config dw_mipi_dsi_regmap_cfg = { +
.reg_bits = 32, + .val_bits = 32, + .reg_stride =
4, + .name = "dw-mipi-dsi", +}; +
/*
* Check if either a link to a master or slave is present */
@@ -280,16 +290,6 @@ static inline struct dw_mipi_dsi
*bridge_to_dsi(struct drm_bridge *bridge)
return container_of(bridge, struct dw_mipi_dsi,
bridge);
}
-static inline void dsi_write(struct dw_mipi_dsi *dsi, u32 reg,
u32 val) -{ - writel(val, dsi->base + reg); -} - -static
inline u32 dsi_read(struct dw_mipi_dsi *dsi, u32 reg) -{ -
return readl(dsi->base + reg); -} -
static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device
*device)
{
@@ -366,29 +366,29 @@ static void dw_mipi_message_config(struct
dw_mipi_dsi *dsi,
if (lpm)
val |= CMD_MODE_ALL_LP;
- dsi_write(dsi, DSI_LPCLK_CTRL, lpm ? 0 :
PHY_TXREQUESTCLKHS); - dsi_write(dsi, DSI_CMD_MODE_CFG,
val); + regmap_write(dsi->regs, DSI_LPCLK_CTRL, lpm ? 0 :
PHY_TXREQUESTCLKHS); + regmap_write(dsi->regs,
DSI_CMD_MODE_CFG, val);
}
static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi
*dsi, u32 hdr_val) {
int ret;
- u32 val, mask; + u32 val = 0, mask;
- ret = readl_poll_timeout(dsi->base +
DSI_CMD_PKT_STATUS, -val, !(val
& GEN_CMD_FULL), 1000, -
CMD_PKT_STATUS_TIMEOUT_US); + ret =
regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS, +
val, !(val & GEN_CMD_FULL), 1000, +
CMD_PKT_STATUS_TIMEOUT_US);
if (ret) {
dev_err(dsi->dev, "failed to get available
command FIFO\n"); return ret;
}
- dsi_write(dsi, DSI_GEN_HDR, hdr_val); +
regmap_write(dsi->regs, DSI_GEN_HDR, hdr_val);
mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY;
- ret = readl_poll_timeout(dsi->base +
DSI_CMD_PKT_STATUS, -val, (val
& mask) == mask, -1000,
CMD_PKT_STATUS_TIMEOUT_US); + ret =
regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS, +
val, (val & mask) == mask, +
1000, CMD_PKT_STATUS_TIMEOUT_US);
if (ret) {
dev_err(dsi->dev, "failed to write command
FIFO\n"); return ret;
@@ -403,24 +403,26 @@ static int dw_mipi_dsi_write(struct
dw_mipi_dsi *dsi,
const u8 *tx_buf = packet->payload; int len =
packet->payload_length, pld_data_bytes = sizeof(u32),
ret; __le32 word;
- u32 val; + u32 val = 0;
while (len) {
if (len < pld_data_bytes) {
word = 0; memcpy(, tx_buf, len);
- dsi_write(dsi, DSI_GEN_PLD_DATA,
le32_to_cpu(word)); +
regmap_write(dsi->regs, DSI_GEN_PLD_DATA, +
le32_to_cpu(word));
len = 0;
} else {
memcpy(, tx_buf, pld_data_bytes);
- dsi_write(dsi, DSI_GEN_PLD_DATA,
le32_to_cpu(word)); +
regmap_write(dsi->regs, DSI_GEN_PLD_DATA, +
le32_to_cpu(word));
tx_buf += pld_data_bytes; len -=
pld_data_bytes;
[PATCH v5 1/5] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure
In order to support multiple versions of the Synopsis MIPI DSI host
controller, which have different register layouts but almost identical
HW protocols, we add a regmap infrastructure which can abstract away
register accesses for platform drivers using the bridge.
The controller HW revision is detected during bridge probe which will
be used in future commits to load the relevant register layout which
the bridge will use transparently to the platform drivers.
Signed-off-by: Adrian Ratiu
---
New in v5.
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 208 ++
1 file changed, 117 insertions(+), 91 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 5ef0f154aa7b..6d9e2f21c9cc 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
@@ -227,6 +228,7 @@ struct dw_mipi_dsi {
struct drm_bridge *panel_bridge;
struct device *dev;
void __iomem *base;
+ struct regmap *regs;
struct clk *pclk;
@@ -235,6 +237,7 @@ struct dw_mipi_dsi {
u32 lanes;
u32 format;
unsigned long mode_flags;
+ u32 hw_version;
#ifdef CONFIG_DEBUG_FS
struct dentry *debugfs;
@@ -249,6 +252,13 @@ struct dw_mipi_dsi {
const struct dw_mipi_dsi_plat_data *plat_data;
};
+static const struct regmap_config dw_mipi_dsi_regmap_cfg = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .name = "dw-mipi-dsi",
+};
+
/*
* Check if either a link to a master or slave is present
*/
@@ -280,16 +290,6 @@ static inline struct dw_mipi_dsi *bridge_to_dsi(struct
drm_bridge *bridge)
return container_of(bridge, struct dw_mipi_dsi, bridge);
}
-static inline void dsi_write(struct dw_mipi_dsi *dsi, u32 reg, u32 val)
-{
- writel(val, dsi->base + reg);
-}
-
-static inline u32 dsi_read(struct dw_mipi_dsi *dsi, u32 reg)
-{
- return readl(dsi->base + reg);
-}
-
static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device *device)
{
@@ -366,29 +366,29 @@ static void dw_mipi_message_config(struct dw_mipi_dsi
*dsi,
if (lpm)
val |= CMD_MODE_ALL_LP;
- dsi_write(dsi, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
- dsi_write(dsi, DSI_CMD_MODE_CFG, val);
+ regmap_write(dsi->regs, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
+ regmap_write(dsi->regs, DSI_CMD_MODE_CFG, val);
}
static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val)
{
int ret;
- u32 val, mask;
+ u32 val = 0, mask;
- ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
-val, !(val & GEN_CMD_FULL), 1000,
-CMD_PKT_STATUS_TIMEOUT_US);
+ ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
+ val, !(val & GEN_CMD_FULL), 1000,
+ CMD_PKT_STATUS_TIMEOUT_US);
if (ret) {
dev_err(dsi->dev, "failed to get available command FIFO\n");
return ret;
}
- dsi_write(dsi, DSI_GEN_HDR, hdr_val);
+ regmap_write(dsi->regs, DSI_GEN_HDR, hdr_val);
mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY;
- ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
-val, (val & mask) == mask,
-1000, CMD_PKT_STATUS_TIMEOUT_US);
+ ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
+ val, (val & mask) == mask,
+ 1000, CMD_PKT_STATUS_TIMEOUT_US);
if (ret) {
dev_err(dsi->dev, "failed to write command FIFO\n");
return ret;
@@ -403,24 +403,26 @@ static int dw_mipi_dsi_write(struct dw_mipi_dsi *dsi,
const u8 *tx_buf = packet->payload;
int len = packet->payload_length, pld_data_bytes = sizeof(u32), ret;
__le32 word;
- u32 val;
+ u32 val = 0;
while (len) {
if (len < pld_data_bytes) {
word = 0;
memcpy(, tx_buf, len);
- dsi_write(dsi, DSI_GEN_PLD_DATA, le32_to_cpu(word));
+ regmap_write(dsi->regs, DSI_GEN_PLD_DATA,
+le32_to_cpu(word));
len = 0;
} else {
memcpy(, tx_buf, pld_data_bytes);
- dsi_write(dsi, DSI_GEN_PLD_DATA, le32_to_cpu(word));
+ regmap_write(dsi->regs, DSI_GEN_PLD_DATA,
+le32_to_cpu(word));
tx_buf += pld_data_bytes;
len
