Re: [PATCHv2] drm/omap: add OMAP5 DSIPHY lane-enable support

2017-08-10 Thread Laurent Pinchart
Hi Tomi,

Thank you for the patch.

On Thursday 10 Aug 2017 12:44:01 Tomi Valkeinen wrote:
> We are missing OMAP5 DSIPHY lane-enable support, which has prevented
> OMAP5 DSI working in mainline. This patch adds the lane-enable similarly
> to the recently added OMAP4 version.
> 
> Signed-off-by: Tomi Valkeinen 
> ---
> 
> Changes:
> 
> - Fixed the check for DSI model
> - Use else if
> - Drop the unnecessary syscon check
> 
>  drivers/gpu/drm/omapdrm/dss/dsi.c | 50 +---
>  1 file changed, 42 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c
> b/drivers/gpu/drm/omapdrm/dss/dsi.c index a66d2b1a6c74..889f2d30050b 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -2109,9 +2109,6 @@ static int dsi_omap4_mux_pads(struct dsi_data *dsi,
> unsigned int lanes) u32 pipd_mask, pipd_shift;
>   u32 reg;
> 
> - if (!dsi->syscon)
> - return 0;
> -
>   if (dsi->module_id == 0) {
>   enable_mask = OMAP4_DSI1_LANEENABLE_MASK;
>   enable_shift = OMAP4_DSI1_LANEENABLE_SHIFT;
> @@ -2139,14 +2136,48 @@ static int dsi_omap4_mux_pads(struct dsi_data *dsi,
> unsigned int lanes) return 0;
>  }
> 
> +#define OMAP5_DSIPHY_SYSCON_OFFSET   0x74
> +
> +#define OMAP5_DSI1_LANEENABLE_SHIFT  24
> +#define OMAP5_DSI2_LANEENABLE_SHIFT  19
> +#define OMAP5_DSI_LANEENABLE_MASK0x1f
> +
> +static int dsi_omap5_mux_pads(struct dsi_data *dsi, unsigned int lanes)
> +{
> + u32 enable_mask, enable_shift, reg;
> +
> + if (dsi->module_id == 0)
> + enable_shift = OMAP5_DSI1_LANEENABLE_SHIFT;
> + else if (dsi->module_id == 1)
> + enable_shift = OMAP5_DSI2_LANEENABLE_SHIFT;
> + else
> + return -ENODEV;
> +
> + enable_mask = OMAP5_DSI_LANEENABLE_MASK << enable_shift;
> +
> + regmap_read(dsi->syscon, OMAP5_DSIPHY_SYSCON_OFFSET, ®);
> + reg &= ~enable_mask;
> + reg |= (lanes << enable_shift) & enable_mask;
> + regmap_write(dsi->syscon, OMAP5_DSIPHY_SYSCON_OFFSET, reg);

I should have spot that during the first review, but I think 
regmap_update_bits() would be more efficient, and would also benefit from 
internal regmap locking.

Apart from that,

Reviewed-by: Laurent Pinchart 

> +
> + return 0;
> +}
> +
>  static int dsi_enable_pads(struct dsi_data *dsi, unsigned int lane_mask)
>  {
> - return dsi_omap4_mux_pads(dsi, lane_mask);
> + if (dsi->data->model == DSI_MODEL_OMAP4)
> + return dsi_omap4_mux_pads(dsi, lane_mask);
> + if (dsi->data->model == DSI_MODEL_OMAP5)
> + return dsi_omap5_mux_pads(dsi, lane_mask);
> + return 0;
>  }
> 
>  static void dsi_disable_pads(struct dsi_data *dsi)
>  {
> - dsi_omap4_mux_pads(dsi, 0);
> + if (dsi->data->model == DSI_MODEL_OMAP4)
> + dsi_omap4_mux_pads(dsi, 0);
> + else if (dsi->data->model == DSI_MODEL_OMAP5)
> + dsi_omap5_mux_pads(dsi, 0);
>  }
> 
>  static int dsi_cio_init(struct platform_device *dsidev)
> @@ -5480,14 +5511,17 @@ static int dsi_bind(struct device *dev, struct
> device *master, void *data)
> 
>   dsi->module_id = d->id;
> 
> - if (dsi->data->model == DSI_MODEL_OMAP4) {
> + if (dsi->data->model == DSI_MODEL_OMAP4 ||
> + dsi->data->model == DSI_MODEL_OMAP5) {
>   struct device_node *np;
> 
>   /*
> -  * The OMAP4 display DT bindings don't reference the padconf
> +  * The OMAP4/5 display DT bindings don't reference the padconf
>* syscon. Our only option to retrieve it is to find it by 
name.
>*/
> - np = of_find_node_by_name(NULL, "omap4_padconf_global");
> + np = of_find_node_by_name(NULL,
> + dsi->data->model == DSI_MODEL_OMAP4 ?
> + "omap4_padconf_global" : "omap5_padconf_global");
>   if (!np)
>   return -ENODEV;

-- 
Regards,

Laurent Pinchart

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


[PATCHv2] drm/omap: add OMAP5 DSIPHY lane-enable support

2017-08-10 Thread Tomi Valkeinen
We are missing OMAP5 DSIPHY lane-enable support, which has prevented
OMAP5 DSI working in mainline. This patch adds the lane-enable similarly
to the recently added OMAP4 version.

Signed-off-by: Tomi Valkeinen 
---

Changes:

- Fixed the check for DSI model
- Use else if
- Drop the unnecessary syscon check

 drivers/gpu/drm/omapdrm/dss/dsi.c | 50 ---
 1 file changed, 42 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c 
b/drivers/gpu/drm/omapdrm/dss/dsi.c
index a66d2b1a6c74..889f2d30050b 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -2109,9 +2109,6 @@ static int dsi_omap4_mux_pads(struct dsi_data *dsi, 
unsigned int lanes)
u32 pipd_mask, pipd_shift;
u32 reg;
 
-   if (!dsi->syscon)
-   return 0;
-
if (dsi->module_id == 0) {
enable_mask = OMAP4_DSI1_LANEENABLE_MASK;
enable_shift = OMAP4_DSI1_LANEENABLE_SHIFT;
@@ -2139,14 +2136,48 @@ static int dsi_omap4_mux_pads(struct dsi_data *dsi, 
unsigned int lanes)
return 0;
 }
 
+#define OMAP5_DSIPHY_SYSCON_OFFSET 0x74
+
+#define OMAP5_DSI1_LANEENABLE_SHIFT24
+#define OMAP5_DSI2_LANEENABLE_SHIFT19
+#define OMAP5_DSI_LANEENABLE_MASK  0x1f
+
+static int dsi_omap5_mux_pads(struct dsi_data *dsi, unsigned int lanes)
+{
+   u32 enable_mask, enable_shift, reg;
+
+   if (dsi->module_id == 0)
+   enable_shift = OMAP5_DSI1_LANEENABLE_SHIFT;
+   else if (dsi->module_id == 1)
+   enable_shift = OMAP5_DSI2_LANEENABLE_SHIFT;
+   else
+   return -ENODEV;
+
+   enable_mask = OMAP5_DSI_LANEENABLE_MASK << enable_shift;
+
+   regmap_read(dsi->syscon, OMAP5_DSIPHY_SYSCON_OFFSET, ®);
+   reg &= ~enable_mask;
+   reg |= (lanes << enable_shift) & enable_mask;
+   regmap_write(dsi->syscon, OMAP5_DSIPHY_SYSCON_OFFSET, reg);
+
+   return 0;
+}
+
 static int dsi_enable_pads(struct dsi_data *dsi, unsigned int lane_mask)
 {
-   return dsi_omap4_mux_pads(dsi, lane_mask);
+   if (dsi->data->model == DSI_MODEL_OMAP4)
+   return dsi_omap4_mux_pads(dsi, lane_mask);
+   if (dsi->data->model == DSI_MODEL_OMAP5)
+   return dsi_omap5_mux_pads(dsi, lane_mask);
+   return 0;
 }
 
 static void dsi_disable_pads(struct dsi_data *dsi)
 {
-   dsi_omap4_mux_pads(dsi, 0);
+   if (dsi->data->model == DSI_MODEL_OMAP4)
+   dsi_omap4_mux_pads(dsi, 0);
+   else if (dsi->data->model == DSI_MODEL_OMAP5)
+   dsi_omap5_mux_pads(dsi, 0);
 }
 
 static int dsi_cio_init(struct platform_device *dsidev)
@@ -5480,14 +5511,17 @@ static int dsi_bind(struct device *dev, struct device 
*master, void *data)
 
dsi->module_id = d->id;
 
-   if (dsi->data->model == DSI_MODEL_OMAP4) {
+   if (dsi->data->model == DSI_MODEL_OMAP4 ||
+   dsi->data->model == DSI_MODEL_OMAP5) {
struct device_node *np;
 
/*
-* The OMAP4 display DT bindings don't reference the padconf
+* The OMAP4/5 display DT bindings don't reference the padconf
 * syscon. Our only option to retrieve it is to find it by name.
 */
-   np = of_find_node_by_name(NULL, "omap4_padconf_global");
+   np = of_find_node_by_name(NULL,
+   dsi->data->model == DSI_MODEL_OMAP4 ?
+   "omap4_padconf_global" : "omap5_padconf_global");
if (!np)
return -ENODEV;
 
-- 
2.7.4


Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


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