diff mbox series

[v3,50/56] drm/omap: dsi: simplify pin config

Message ID 20201105120333.947408-51-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show
Series Convert DSI code to use drm_mipi_dsi and drm_panel | expand

Commit Message

Tomi Valkeinen Nov. 5, 2020, 12:03 p.m. UTC 
From: Sebastian Reichel <sebastian.reichel@collabora.com>

Simplify DSI pin config, which always originates from DT
nowadays. With the code being fully contained in the DSI
encoder, we can drop the public structure.

Since the function is no longer exposed, it now directly
takes the private DSI data pointer. This drop a pointless
conversion and means the pins can be configured earlier.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dsi.c     | 33 +++++++++------------------
 drivers/gpu/drm/omapdrm/dss/omapdss.h | 15 ------------
 2 files changed, 11 insertions(+), 37 deletions(-)

Comments

Laurent Pinchart Nov. 9, 2020, 11:09 a.m. UTC | #1 
Hi Tomi and Sebastian,

Thank you for the patch.

On Thu, Nov 05, 2020 at 02:03:27PM +0200, Tomi Valkeinen wrote:
> From: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> Simplify DSI pin config, which always originates from DT
> nowadays. With the code being fully contained in the DSI
> encoder, we can drop the public structure.
> 
> Since the function is no longer exposed, it now directly
> takes the private DSI data pointer. This drop a pointless

s/drop/drops/

> conversion and means the pins can be configured earlier.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dsi.c     | 33 +++++++++------------------
>  drivers/gpu/drm/omapdrm/dss/omapdss.h | 15 ------------
>  2 files changed, 11 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index f47d7e3bb631..76e4f607d8cf 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -3568,12 +3568,9 @@ static void dsi_proto_timings(struct dsi_data *dsi)
>  	}
>  }
>  
> -static int dsi_configure_pins(struct omap_dss_device *dssdev,
> -		const struct omap_dsi_pin_config *pin_cfg)
> +static int dsi_configure_pins(struct dsi_data *dsi,
> +		int num_pins, const u32 *pins)
>  {
> -	struct dsi_data *dsi = to_dsi_data(dssdev);
> -	int num_pins;
> -	const int *pins;
>  	struct dsi_lane_config lanes[DSI_MAX_NR_LANES];
>  	int num_lanes;
>  	int i;
> @@ -3586,9 +3583,6 @@ static int dsi_configure_pins(struct omap_dss_device *dssdev,
>  		DSI_LANE_DATA4,
>  	};
>  
> -	num_pins = pin_cfg->num_pins;
> -	pins = pin_cfg->pins;
> -
>  	if (num_pins < 4 || num_pins > dsi->num_lanes_supported * 2
>  			|| num_pins % 2 != 0)
>  		return -EINVAL;
> @@ -3600,7 +3594,7 @@ static int dsi_configure_pins(struct omap_dss_device *dssdev,
>  
>  	for (i = 0; i < num_pins; i += 2) {
>  		u8 lane, pol;
> -		int dx, dy;
> +		u32 dx, dy;

Is this change needed ?

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  
>  		dx = pins[i];
>  		dy = pins[i + 1];
> @@ -5481,9 +5475,8 @@ static int dsi_probe_of(struct dsi_data *dsi)
>  	struct property *prop;
>  	u32 lane_arr[10];
>  	int len, num_pins;
> -	int r, i;
> +	int r;
>  	struct device_node *ep;
> -	struct omap_dsi_pin_config pin_cfg;
>  
>  	ep = of_graph_get_endpoint_by_regs(node, 0, 0);
>  	if (!ep)
> @@ -5511,11 +5504,7 @@ static int dsi_probe_of(struct dsi_data *dsi)
>  		goto err;
>  	}
>  
> -	pin_cfg.num_pins = num_pins;
> -	for (i = 0; i < num_pins; ++i)
> -		pin_cfg.pins[i] = (int)lane_arr[i];
> -
> -	r = dsi_configure_pins(&dsi->output, &pin_cfg);
> +	r = dsi_configure_pins(dsi, num_pins, lane_arr);
>  	if (r) {
>  		dev_err(dsi->dev, "failed to configure pins");
>  		goto err;
> @@ -5728,6 +5717,12 @@ static int dsi_probe(struct platform_device *pdev)
>  	dsi->host.ops = &omap_dsi_host_ops;
>  	dsi->host.dev = &pdev->dev;
>  
> +	r = dsi_probe_of(dsi);
> +	if (r) {
> +		DSSERR("Invalid DSI DT data\n");
> +		goto err_pm_disable;
> +	}
> +
>  	r = mipi_dsi_host_register(&dsi->host);
>  	if (r < 0) {
>  		dev_err(&pdev->dev, "failed to register DSI host: %d\n", r);
> @@ -5738,12 +5733,6 @@ static int dsi_probe(struct platform_device *pdev)
>  	if (r)
>  		goto err_dsi_host_unregister;
>  
> -	r = dsi_probe_of(dsi);
> -	if (r) {
> -		DSSERR("Invalid DSI DT data\n");
> -		goto err_uninit_output;
> -	}
> -
>  	r = component_add(&pdev->dev, &dsi_component_ops);
>  	if (r)
>  		goto err_uninit_output;
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> index a1236b8ef7ea..4a0826c8fed5 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -243,21 +243,6 @@ struct omap_overlay_manager_info {
>  	struct omap_dss_cpr_coefs cpr_coefs;
>  };
>  
> -/* 22 pins means 1 clk lane and 10 data lanes */
> -#define OMAP_DSS_MAX_DSI_PINS 22
> -
> -struct omap_dsi_pin_config {
> -	int num_pins;
> -	/*
> -	 * pin numbers in the following order:
> -	 * clk+, clk-
> -	 * data1+, data1-
> -	 * data2+, data2-
> -	 * ...
> -	 */
> -	int pins[OMAP_DSS_MAX_DSI_PINS];
> -};
> -
>  struct omap_dss_writeback_info {
>  	u32 paddr;
>  	u32 p_uv_addr;
Tomi Valkeinen Nov. 11, 2020, 12:24 p.m. UTC | #2 
On 09/11/2020 13:09, Laurent Pinchart wrote:
> Hi Tomi and Sebastian,
> 
> Thank you for the patch.
> 
> On Thu, Nov 05, 2020 at 02:03:27PM +0200, Tomi Valkeinen wrote:
>> From: Sebastian Reichel <sebastian.reichel@collabora.com>
>>
>> Simplify DSI pin config, which always originates from DT
>> nowadays. With the code being fully contained in the DSI
>> encoder, we can drop the public structure.
>>
>> Since the function is no longer exposed, it now directly
>> takes the private DSI data pointer. This drop a pointless
> 
> s/drop/drops/
> 
>> conversion and means the pins can be configured earlier.
>>
>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/dss/dsi.c     | 33 +++++++++------------------
>>  drivers/gpu/drm/omapdrm/dss/omapdss.h | 15 ------------
>>  2 files changed, 11 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
>> index f47d7e3bb631..76e4f607d8cf 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
>> @@ -3568,12 +3568,9 @@ static void dsi_proto_timings(struct dsi_data *dsi)
>>  	}
>>  }
>>  
>> -static int dsi_configure_pins(struct omap_dss_device *dssdev,
>> -		const struct omap_dsi_pin_config *pin_cfg)
>> +static int dsi_configure_pins(struct dsi_data *dsi,
>> +		int num_pins, const u32 *pins)
>>  {
>> -	struct dsi_data *dsi = to_dsi_data(dssdev);
>> -	int num_pins;
>> -	const int *pins;
>>  	struct dsi_lane_config lanes[DSI_MAX_NR_LANES];
>>  	int num_lanes;
>>  	int i;
>> @@ -3586,9 +3583,6 @@ static int dsi_configure_pins(struct omap_dss_device *dssdev,
>>  		DSI_LANE_DATA4,
>>  	};
>>  
>> -	num_pins = pin_cfg->num_pins;
>> -	pins = pin_cfg->pins;
>> -
>>  	if (num_pins < 4 || num_pins > dsi->num_lanes_supported * 2
>>  			|| num_pins % 2 != 0)
>>  		return -EINVAL;
>> @@ -3600,7 +3594,7 @@ static int dsi_configure_pins(struct omap_dss_device *dssdev,
>>  
>>  	for (i = 0; i < num_pins; i += 2) {
>>  		u8 lane, pol;
>> -		int dx, dy;
>> +		u32 dx, dy;
> 
> Is this change needed ?

The pins array is now u32, so the above is correct. However, a bit below we check if dx < 0, which
doesn't make sense anymore, so I'll drop that check.

 Tomi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
index f47d7e3bb631..76e4f607d8cf 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -3568,12 +3568,9 @@  static void dsi_proto_timings(struct dsi_data *dsi)
 	}
 }
 
-static int dsi_configure_pins(struct omap_dss_device *dssdev,
-		const struct omap_dsi_pin_config *pin_cfg)
+static int dsi_configure_pins(struct dsi_data *dsi,
+		int num_pins, const u32 *pins)
 {
-	struct dsi_data *dsi = to_dsi_data(dssdev);
-	int num_pins;
-	const int *pins;
 	struct dsi_lane_config lanes[DSI_MAX_NR_LANES];
 	int num_lanes;
 	int i;
@@ -3586,9 +3583,6 @@  static int dsi_configure_pins(struct omap_dss_device *dssdev,
 		DSI_LANE_DATA4,
 	};
 
-	num_pins = pin_cfg->num_pins;
-	pins = pin_cfg->pins;
-
 	if (num_pins < 4 || num_pins > dsi->num_lanes_supported * 2
 			|| num_pins % 2 != 0)
 		return -EINVAL;
@@ -3600,7 +3594,7 @@  static int dsi_configure_pins(struct omap_dss_device *dssdev,
 
 	for (i = 0; i < num_pins; i += 2) {
 		u8 lane, pol;
-		int dx, dy;
+		u32 dx, dy;
 
 		dx = pins[i];
 		dy = pins[i + 1];
@@ -5481,9 +5475,8 @@  static int dsi_probe_of(struct dsi_data *dsi)
 	struct property *prop;
 	u32 lane_arr[10];
 	int len, num_pins;
-	int r, i;
+	int r;
 	struct device_node *ep;
-	struct omap_dsi_pin_config pin_cfg;
 
 	ep = of_graph_get_endpoint_by_regs(node, 0, 0);
 	if (!ep)
@@ -5511,11 +5504,7 @@  static int dsi_probe_of(struct dsi_data *dsi)
 		goto err;
 	}
 
-	pin_cfg.num_pins = num_pins;
-	for (i = 0; i < num_pins; ++i)
-		pin_cfg.pins[i] = (int)lane_arr[i];
-
-	r = dsi_configure_pins(&dsi->output, &pin_cfg);
+	r = dsi_configure_pins(dsi, num_pins, lane_arr);
 	if (r) {
 		dev_err(dsi->dev, "failed to configure pins");
 		goto err;
@@ -5728,6 +5717,12 @@  static int dsi_probe(struct platform_device *pdev)
 	dsi->host.ops = &omap_dsi_host_ops;
 	dsi->host.dev = &pdev->dev;
 
+	r = dsi_probe_of(dsi);
+	if (r) {
+		DSSERR("Invalid DSI DT data\n");
+		goto err_pm_disable;
+	}
+
 	r = mipi_dsi_host_register(&dsi->host);
 	if (r < 0) {
 		dev_err(&pdev->dev, "failed to register DSI host: %d\n", r);
@@ -5738,12 +5733,6 @@  static int dsi_probe(struct platform_device *pdev)
 	if (r)
 		goto err_dsi_host_unregister;
 
-	r = dsi_probe_of(dsi);
-	if (r) {
-		DSSERR("Invalid DSI DT data\n");
-		goto err_uninit_output;
-	}
-
 	r = component_add(&pdev->dev, &dsi_component_ops);
 	if (r)
 		goto err_uninit_output;
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index a1236b8ef7ea..4a0826c8fed5 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -243,21 +243,6 @@  struct omap_overlay_manager_info {
 	struct omap_dss_cpr_coefs cpr_coefs;
 };
 
-/* 22 pins means 1 clk lane and 10 data lanes */
-#define OMAP_DSS_MAX_DSI_PINS 22
-
-struct omap_dsi_pin_config {
-	int num_pins;
-	/*
-	 * pin numbers in the following order:
-	 * clk+, clk-
-	 * data1+, data1-
-	 * data2+, data2-
-	 * ...
-	 */
-	int pins[OMAP_DSS_MAX_DSI_PINS];
-};
-
 struct omap_dss_writeback_info {
 	u32 paddr;
 	u32 p_uv_addr;