Re: [PATCH v2 1/4] drm: bridge: dw_mipi_dsi: access registers via a regmap

2019-11-13 Thread Adrian Ratiu
On Wed, 13 Nov 2019, Emil Velikov  
wrote:
On Wed, 6 Nov 2019 at 16:30, Adrian Ratiu 
 wrote: 


Convert the common bridge code and the two rockchip & stm 
drivers which currently use it to the regmap API in 
anticipation for further changes to make it more generic and 
add older DSI host controller support as found on i.mx6 based 
devices. 

The regmap becomes an internal state of the bridge. No 
functional changes other than requiring the platform drivers to 
use the pre-configured regmap supplied by the bridge after its 
probe() call instead of ioremp'ing the registers themselves. 

In subsequent commits the bridge will become able to detect the 
DSI host core version and init the regmap with different 
register layouts. The platform drivers will continue to use the 
regmap without modifications or worrying about the specific 
layout in use (in other words the layout is abstracted away via 
the regmap). 

Suggested-by: Boris Brezillon  
Reviewed-by: Neil Armstrong  
Reviewed-by: Emil Velikov  


I should have been clearer earlier - I didn't quite review the 
patch.  Is is now though.  Reviewed-by: Emil Velikov 



Sorry about that, I got confused and thought you reviewed it all.



Admittedly a couple of nitpicks (DRIVER_NAME, zero initialize of 
val) could have been left out.  It's not a big deal, there's no 
need to polish those.


I'll address them in v3 as well as updating your mail address.

Thanks for reviewing!



-Emil

___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/4] drm: bridge: dw_mipi_dsi: access registers via a regmap

2019-11-13 Thread Emil Velikov
On Wed, 6 Nov 2019 at 16:30, Adrian Ratiu  wrote:
>
> Convert the common bridge code and the two rockchip & stm drivers
> which currently use it to the regmap API in anticipation for further
> changes to make it more generic and add older DSI host controller
> support as found on i.mx6 based devices.
>
> The regmap becomes an internal state of the bridge. No functional
> changes other than requiring the platform drivers to use the
> pre-configured regmap supplied by the bridge after its probe() call
> instead of ioremp'ing the registers themselves.
>
> In subsequent commits the bridge will become able to detect the
> DSI host core version and init the regmap with different register
> layouts. The platform drivers will continue to use the regmap without
> modifications or worrying about the specific layout in use (in other
> words the layout is abstracted away via the regmap).
>
> Suggested-by: Boris Brezillon 
> Reviewed-by: Neil Armstrong 
> Reviewed-by: Emil Velikov 

I should have been clearer earlier - I didn't quite review the patch.
Is is now though.
Reviewed-by: Emil Velikov 

Admittedly a couple of nitpicks (DRIVER_NAME, zero initialize of val)
could have been left out.
It's not a big deal, there's no need to polish those.

-Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/4] drm: bridge: dw_mipi_dsi: access registers via a regmap

2019-11-06 Thread Adrian Ratiu
Convert the common bridge code and the two rockchip & stm drivers
which currently use it to the regmap API in anticipation for further
changes to make it more generic and add older DSI host controller
support as found on i.mx6 based devices.

The regmap becomes an internal state of the bridge. No functional
changes other than requiring the platform drivers to use the
pre-configured regmap supplied by the bridge after its probe() call
instead of ioremp'ing the registers themselves.

In subsequent commits the bridge will become able to detect the
DSI host core version and init the regmap with different register
layouts. The platform drivers will continue to use the regmap without
modifications or worrying about the specific layout in use (in other
words the layout is abstracted away via the regmap).

Suggested-by: Boris Brezillon 
Reviewed-by: Neil Armstrong 
Reviewed-by: Emil Velikov 
Signed-off-by: Adrian Ratiu 
---
 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 215 ++
 .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   |  17 +-
 drivers/gpu/drm/stm/dw_mipi_dsi-stm.c |  34 ++-
 include/drm/bridge/dw_mipi_dsi.h  |   2 +-
 4 files changed, 145 insertions(+), 123 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index b6e793bb653c..6cb57807f3f9 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 
@@ -226,7 +227,7 @@ struct dw_mipi_dsi {
struct mipi_dsi_host dsi_host;
struct drm_bridge *panel_bridge;
struct device *dev;
-   void __iomem *base;
+   struct regmap *regs;
 
struct clk *pclk;
 
@@ -249,6 +250,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 +288,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 +364,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 +401,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,
+