Re: [PATCH v8 01/10] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure

2020-04-28 Thread Enric Balletbo i Serra
Hi Adrian,

Thank you for your patch and to apply the changes I requested

On 27/4/20 10:19, 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.
> 
> Cc: Enric Balletbo Serra 
> Suggested-by: Ezequiel Garcia 
> Tested-by: Adrian Pop 
> Tested-by: Arnaud Ferraris 
> Signed-off-by: Adrian Ratiu 

Reviewed-by: Enric Balletbo i Serra 

> ---
> Chnages since v7:
>   - Minor checkpatch line fix
> 
> Changes since v6:
>   - Select REGMAP_MMIO in Kconfig (Enric)
>   - Drop unnecessary stack variable inits (Enric)
>   - Make bridge error ASAP after a bad revision read (Enric)
>   - Drop redundant read of hw_version in dphy_timing_config (Enric)
> 
> New in v5.
> ---
>  drivers/gpu/drm/bridge/synopsys/Kconfig   |   1 +
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 210 ++
>  2 files changed, 121 insertions(+), 90 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig 
> b/drivers/gpu/drm/bridge/synopsys/Kconfig
> index 21a1be3ced0f3..080146093b68e 100644
> --- a/drivers/gpu/drm/bridge/synopsys/Kconfig
> +++ b/drivers/gpu/drm/bridge/synopsys/Kconfig
> @@ -39,3 +39,4 @@ config DRM_DW_MIPI_DSI
>   select DRM_KMS_HELPER
>   select DRM_MIPI_DSI
>   select DRM_PANEL_BRIDGE
> + select REGMAP_MMIO
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 5ef0f154aa7bd..34b8668ae24ea 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,8 +366,8 @@ 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)
> @@ -375,20 +375,20 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct 
> dw_mipi_dsi *dsi, u32 hdr_val)
>   int ret;
>   u32 val, 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,
> +  

[PATCH v8 01/10] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure

2020-04-27 Thread Adrian Ratiu
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.

Cc: Enric Balletbo Serra 
Suggested-by: Ezequiel Garcia 
Tested-by: Adrian Pop 
Tested-by: Arnaud Ferraris 
Signed-off-by: Adrian Ratiu 
---
Chnages since v7:
  - Minor checkpatch line fix

Changes since v6:
  - Select REGMAP_MMIO in Kconfig (Enric)
  - Drop unnecessary stack variable inits (Enric)
  - Make bridge error ASAP after a bad revision read (Enric)
  - Drop redundant read of hw_version in dphy_timing_config (Enric)

New in v5.
---
 drivers/gpu/drm/bridge/synopsys/Kconfig   |   1 +
 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 210 ++
 2 files changed, 121 insertions(+), 90 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig 
b/drivers/gpu/drm/bridge/synopsys/Kconfig
index 21a1be3ced0f3..080146093b68e 100644
--- a/drivers/gpu/drm/bridge/synopsys/Kconfig
+++ b/drivers/gpu/drm/bridge/synopsys/Kconfig
@@ -39,3 +39,4 @@ config DRM_DW_MIPI_DSI
select DRM_KMS_HELPER
select DRM_MIPI_DSI
select DRM_PANEL_BRIDGE
+   select REGMAP_MMIO
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 5ef0f154aa7bd..34b8668ae24ea 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,8 +366,8 @@ 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)
@@ -375,20 +375,20 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct 
dw_mipi_dsi *dsi, u32 hdr_val)
int ret;
u32 val, 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;
@@ -409,18 +409,20 @@ static int dw_mipi_dsi_write(struct dw_mipi_dsi *dsi,