Patchwork [24/48] drm: omapdrm: sdi: Pass DSS pointer to dss_sdi_*() functions

login
register
mail settings
Submitter Laurent Pinchart
Date Oct. 13, 2017, 2:59 p.m.
Message ID <20171013145944.26557-25-laurent.pinchart@ideasonboard.com>
Download mbox | patch
Permalink /patch/10005009/
State New
Headers show

Comments

Laurent Pinchart - Oct. 13, 2017, 2:59 p.m.
This removes the need to access the global DSS private data in those
functions (both for the current accesses and the future ones that will
be introduced when allocating the DSS device dynamically).

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/dss/dss.c |  8 ++++----
 drivers/gpu/drm/omapdrm/dss/dss.h | 14 ++++++++------
 drivers/gpu/drm/omapdrm/dss/sdi.c | 13 ++++++++-----
 3 files changed, 20 insertions(+), 15 deletions(-)
Sebastian Reichel - Oct. 16, 2017, 8:47 a.m.
Hi Laurent,

On Fri, Oct 13, 2017 at 05:59:20PM +0300, Laurent Pinchart wrote:
> This removes the need to access the global DSS private data in those
> functions (both for the current accesses and the future ones that will
> be introduced when allocating the DSS device dynamically).
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dss.c |  8 ++++----
>  drivers/gpu/drm/omapdrm/dss/dss.h | 14 ++++++++------
>  drivers/gpu/drm/omapdrm/dss/sdi.c | 13 ++++++++-----
>  3 files changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/dss.c
> index 7179d02e7451..f8b71e24a07d 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dss.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dss.c
> @@ -287,7 +287,7 @@ static int dss_ctrl_pll_set_control_mux(enum dss_clk_source clk_src,
>  	return 0;
>  }
>  
> -void dss_sdi_init(int datapairs)
> +void dss_sdi_init(struct dss_device *dss, int datapairs)
>  {
>  	u32 l;
>  
> @@ -306,7 +306,7 @@ void dss_sdi_init(int datapairs)
>  	dss_write_reg(DSS_PLL_CONTROL, l);
>  }
>  
> -int dss_sdi_enable(void)
> +int dss_sdi_enable(struct dss_device *dss)
>  {
>  	unsigned long timeout;
>  
> @@ -364,7 +364,7 @@ int dss_sdi_enable(void)
>  	return -ETIMEDOUT;
>  }
>  
> -void dss_sdi_disable(void)
> +void dss_sdi_disable(struct dss_device *dss)
>  {
>  	dispc_lcd_enable_signal(0);
>  
> @@ -1226,7 +1226,7 @@ static int dss_init_ports(struct platform_device *pdev)
>  			dpi_init_port(pdev, port, dss.feat->model);
>  			break;
>  		case OMAP_DISPLAY_TYPE_SDI:
> -			sdi_init_port(pdev, port);
> +			sdi_init_port(&dss, pdev, port);

I guess this should be 'dss' instead of '&dss'? Otherwise looks
fine.

-- Sebastian

>  			break;
>  		default:
>  			break;
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h b/drivers/gpu/drm/omapdrm/dss/dss.h
> index 0b8facf258cf..08651f101518 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/dss.h
> @@ -300,9 +300,9 @@ void dss_video_pll_uninit(struct dss_pll *pll);
>  
>  void dss_ctrl_pll_enable(struct dss_pll *pll, bool enable);
>  
> -void dss_sdi_init(int datapairs);
> -int dss_sdi_enable(void);
> -void dss_sdi_disable(void);
> +void dss_sdi_init(struct dss_device *dss, int datapairs);
> +int dss_sdi_enable(struct dss_device *dss);
> +void dss_sdi_disable(struct dss_device *dss);
>  
>  void dss_select_dsi_clk_source(int dsi_module,
>  		enum dss_clk_source clk_src);
> @@ -323,11 +323,13 @@ bool dss_div_calc(unsigned long pck, unsigned long fck_min,
>  
>  /* SDI */
>  #ifdef CONFIG_OMAP2_DSS_SDI
> -int sdi_init_port(struct platform_device *pdev, struct device_node *port);
> +int sdi_init_port(struct dss_device *dss, struct platform_device *pdev,
> +		  struct device_node *port);
>  void sdi_uninit_port(struct device_node *port);
>  #else
> -static inline int sdi_init_port(struct platform_device *pdev,
> -		struct device_node *port)
> +static inline int sdi_init_port(struct dss_device *dss,
> +				struct platform_device *pdev,
> +				struct device_node *port)
>  {
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/omapdrm/dss/sdi.c b/drivers/gpu/drm/omapdrm/dss/sdi.c
> index d18ad58c5a19..39cb5c8af0dc 100644
> --- a/drivers/gpu/drm/omapdrm/dss/sdi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/sdi.c
> @@ -33,6 +33,7 @@
>  
>  static struct {
>  	struct platform_device *pdev;
> +	struct dss_device *dss;
>  
>  	bool update_enabled;
>  	struct regulator *vdds_sdi_reg;
> @@ -189,8 +190,8 @@ static int sdi_display_enable(struct omap_dss_device *dssdev)
>  	 */
>  	dispc_mgr_set_clock_div(channel, &sdi.mgr_config.clock_info);
>  
> -	dss_sdi_init(sdi.datapairs);
> -	r = dss_sdi_enable();
> +	dss_sdi_init(sdi.dss, sdi.datapairs);
> +	r = dss_sdi_enable(sdi.dss);
>  	if (r)
>  		goto err_sdi_enable;
>  	mdelay(2);
> @@ -202,7 +203,7 @@ static int sdi_display_enable(struct omap_dss_device *dssdev)
>  	return 0;
>  
>  err_mgr_enable:
> -	dss_sdi_disable();
> +	dss_sdi_disable(sdi.dss);
>  err_sdi_enable:
>  err_set_dss_clock_div:
>  err_calc_clock_div:
> @@ -219,7 +220,7 @@ static void sdi_display_disable(struct omap_dss_device *dssdev)
>  
>  	dss_mgr_disable(channel);
>  
> -	dss_sdi_disable();
> +	dss_sdi_disable(sdi.dss);
>  
>  	dispc_runtime_put();
>  
> @@ -347,7 +348,8 @@ static void sdi_uninit_output(struct platform_device *pdev)
>  	omapdss_unregister_output(out);
>  }
>  
> -int sdi_init_port(struct platform_device *pdev, struct device_node *port)
> +int sdi_init_port(struct dss_device *dss, struct platform_device *pdev,
> +		  struct device_node *port)
>  {
>  	struct device_node *ep;
>  	u32 datapairs;
> @@ -364,6 +366,7 @@ int sdi_init_port(struct platform_device *pdev, struct device_node *port)
>  	}
>  
>  	sdi.datapairs = datapairs;
> +	sdi.dss = dss;
>  
>  	of_node_put(ep);
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Sebastian Reichel - Oct. 16, 2017, 9:03 a.m.
Hi,

On Mon, Oct 16, 2017 at 10:47:32AM +0200, Sebastian Reichel wrote:
> Hi Laurent,
> 
> On Fri, Oct 13, 2017 at 05:59:20PM +0300, Laurent Pinchart wrote:
> > This removes the need to access the global DSS private data in those
> > functions (both for the current accesses and the future ones that will
> > be introduced when allocating the DSS device dynamically).
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/omapdrm/dss/dss.c |  8 ++++----
> >  drivers/gpu/drm/omapdrm/dss/dss.h | 14 ++++++++------
> >  drivers/gpu/drm/omapdrm/dss/sdi.c | 13 ++++++++-----
> >  3 files changed, 20 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/dss.c
> > index 7179d02e7451..f8b71e24a07d 100644
> > --- a/drivers/gpu/drm/omapdrm/dss/dss.c
> > +++ b/drivers/gpu/drm/omapdrm/dss/dss.c
> > @@ -287,7 +287,7 @@ static int dss_ctrl_pll_set_control_mux(enum dss_clk_source clk_src,
> >  	return 0;
> >  }
> >  
> > -void dss_sdi_init(int datapairs)
> > +void dss_sdi_init(struct dss_device *dss, int datapairs)
> >  {
> >  	u32 l;
> >  
> > @@ -306,7 +306,7 @@ void dss_sdi_init(int datapairs)
> >  	dss_write_reg(DSS_PLL_CONTROL, l);
> >  }
> >  
> > -int dss_sdi_enable(void)
> > +int dss_sdi_enable(struct dss_device *dss)
> >  {
> >  	unsigned long timeout;
> >  
> > @@ -364,7 +364,7 @@ int dss_sdi_enable(void)
> >  	return -ETIMEDOUT;
> >  }
> >  
> > -void dss_sdi_disable(void)
> > +void dss_sdi_disable(struct dss_device *dss)
> >  {
> >  	dispc_lcd_enable_signal(0);
> >  
> > @@ -1226,7 +1226,7 @@ static int dss_init_ports(struct platform_device *pdev)
> >  			dpi_init_port(pdev, port, dss.feat->model);
> >  			break;
> >  		case OMAP_DISPLAY_TYPE_SDI:
> > -			sdi_init_port(pdev, port);
> > +			sdi_init_port(&dss, pdev, port);
> 
> I guess this should be 'dss' instead of '&dss'? Otherwise looks
> fine.

Oops, read to fast, this is correct.

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

-- Sebastian

> -- Sebastian
> 
> >  			break;
> >  		default:
> >  			break;
> > diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h b/drivers/gpu/drm/omapdrm/dss/dss.h
> > index 0b8facf258cf..08651f101518 100644
> > --- a/drivers/gpu/drm/omapdrm/dss/dss.h
> > +++ b/drivers/gpu/drm/omapdrm/dss/dss.h
> > @@ -300,9 +300,9 @@ void dss_video_pll_uninit(struct dss_pll *pll);
> >  
> >  void dss_ctrl_pll_enable(struct dss_pll *pll, bool enable);
> >  
> > -void dss_sdi_init(int datapairs);
> > -int dss_sdi_enable(void);
> > -void dss_sdi_disable(void);
> > +void dss_sdi_init(struct dss_device *dss, int datapairs);
> > +int dss_sdi_enable(struct dss_device *dss);
> > +void dss_sdi_disable(struct dss_device *dss);
> >  
> >  void dss_select_dsi_clk_source(int dsi_module,
> >  		enum dss_clk_source clk_src);
> > @@ -323,11 +323,13 @@ bool dss_div_calc(unsigned long pck, unsigned long fck_min,
> >  
> >  /* SDI */
> >  #ifdef CONFIG_OMAP2_DSS_SDI
> > -int sdi_init_port(struct platform_device *pdev, struct device_node *port);
> > +int sdi_init_port(struct dss_device *dss, struct platform_device *pdev,
> > +		  struct device_node *port);
> >  void sdi_uninit_port(struct device_node *port);
> >  #else
> > -static inline int sdi_init_port(struct platform_device *pdev,
> > -		struct device_node *port)
> > +static inline int sdi_init_port(struct dss_device *dss,
> > +				struct platform_device *pdev,
> > +				struct device_node *port)
> >  {
> >  	return 0;
> >  }
> > diff --git a/drivers/gpu/drm/omapdrm/dss/sdi.c b/drivers/gpu/drm/omapdrm/dss/sdi.c
> > index d18ad58c5a19..39cb5c8af0dc 100644
> > --- a/drivers/gpu/drm/omapdrm/dss/sdi.c
> > +++ b/drivers/gpu/drm/omapdrm/dss/sdi.c
> > @@ -33,6 +33,7 @@
> >  
> >  static struct {
> >  	struct platform_device *pdev;
> > +	struct dss_device *dss;
> >  
> >  	bool update_enabled;
> >  	struct regulator *vdds_sdi_reg;
> > @@ -189,8 +190,8 @@ static int sdi_display_enable(struct omap_dss_device *dssdev)
> >  	 */
> >  	dispc_mgr_set_clock_div(channel, &sdi.mgr_config.clock_info);
> >  
> > -	dss_sdi_init(sdi.datapairs);
> > -	r = dss_sdi_enable();
> > +	dss_sdi_init(sdi.dss, sdi.datapairs);
> > +	r = dss_sdi_enable(sdi.dss);
> >  	if (r)
> >  		goto err_sdi_enable;
> >  	mdelay(2);
> > @@ -202,7 +203,7 @@ static int sdi_display_enable(struct omap_dss_device *dssdev)
> >  	return 0;
> >  
> >  err_mgr_enable:
> > -	dss_sdi_disable();
> > +	dss_sdi_disable(sdi.dss);
> >  err_sdi_enable:
> >  err_set_dss_clock_div:
> >  err_calc_clock_div:
> > @@ -219,7 +220,7 @@ static void sdi_display_disable(struct omap_dss_device *dssdev)
> >  
> >  	dss_mgr_disable(channel);
> >  
> > -	dss_sdi_disable();
> > +	dss_sdi_disable(sdi.dss);
> >  
> >  	dispc_runtime_put();
> >  
> > @@ -347,7 +348,8 @@ static void sdi_uninit_output(struct platform_device *pdev)
> >  	omapdss_unregister_output(out);
> >  }
> >  
> > -int sdi_init_port(struct platform_device *pdev, struct device_node *port)
> > +int sdi_init_port(struct dss_device *dss, struct platform_device *pdev,
> > +		  struct device_node *port)
> >  {
> >  	struct device_node *ep;
> >  	u32 datapairs;
> > @@ -364,6 +366,7 @@ int sdi_init_port(struct platform_device *pdev, struct device_node *port)
> >  	}
> >  
> >  	sdi.datapairs = datapairs;
> > +	sdi.dss = dss;
> >  
> >  	of_node_put(ep);
> >  
> > -- 
> > Regards,
> > 
> > Laurent Pinchart
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel



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

Patch

diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/dss.c
index 7179d02e7451..f8b71e24a07d 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss.c
+++ b/drivers/gpu/drm/omapdrm/dss/dss.c
@@ -287,7 +287,7 @@  static int dss_ctrl_pll_set_control_mux(enum dss_clk_source clk_src,
 	return 0;
 }
 
-void dss_sdi_init(int datapairs)
+void dss_sdi_init(struct dss_device *dss, int datapairs)
 {
 	u32 l;
 
@@ -306,7 +306,7 @@  void dss_sdi_init(int datapairs)
 	dss_write_reg(DSS_PLL_CONTROL, l);
 }
 
-int dss_sdi_enable(void)
+int dss_sdi_enable(struct dss_device *dss)
 {
 	unsigned long timeout;
 
@@ -364,7 +364,7 @@  int dss_sdi_enable(void)
 	return -ETIMEDOUT;
 }
 
-void dss_sdi_disable(void)
+void dss_sdi_disable(struct dss_device *dss)
 {
 	dispc_lcd_enable_signal(0);
 
@@ -1226,7 +1226,7 @@  static int dss_init_ports(struct platform_device *pdev)
 			dpi_init_port(pdev, port, dss.feat->model);
 			break;
 		case OMAP_DISPLAY_TYPE_SDI:
-			sdi_init_port(pdev, port);
+			sdi_init_port(&dss, pdev, port);
 			break;
 		default:
 			break;
diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h b/drivers/gpu/drm/omapdrm/dss/dss.h
index 0b8facf258cf..08651f101518 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss.h
+++ b/drivers/gpu/drm/omapdrm/dss/dss.h
@@ -300,9 +300,9 @@  void dss_video_pll_uninit(struct dss_pll *pll);
 
 void dss_ctrl_pll_enable(struct dss_pll *pll, bool enable);
 
-void dss_sdi_init(int datapairs);
-int dss_sdi_enable(void);
-void dss_sdi_disable(void);
+void dss_sdi_init(struct dss_device *dss, int datapairs);
+int dss_sdi_enable(struct dss_device *dss);
+void dss_sdi_disable(struct dss_device *dss);
 
 void dss_select_dsi_clk_source(int dsi_module,
 		enum dss_clk_source clk_src);
@@ -323,11 +323,13 @@  bool dss_div_calc(unsigned long pck, unsigned long fck_min,
 
 /* SDI */
 #ifdef CONFIG_OMAP2_DSS_SDI
-int sdi_init_port(struct platform_device *pdev, struct device_node *port);
+int sdi_init_port(struct dss_device *dss, struct platform_device *pdev,
+		  struct device_node *port);
 void sdi_uninit_port(struct device_node *port);
 #else
-static inline int sdi_init_port(struct platform_device *pdev,
-		struct device_node *port)
+static inline int sdi_init_port(struct dss_device *dss,
+				struct platform_device *pdev,
+				struct device_node *port)
 {
 	return 0;
 }
diff --git a/drivers/gpu/drm/omapdrm/dss/sdi.c b/drivers/gpu/drm/omapdrm/dss/sdi.c
index d18ad58c5a19..39cb5c8af0dc 100644
--- a/drivers/gpu/drm/omapdrm/dss/sdi.c
+++ b/drivers/gpu/drm/omapdrm/dss/sdi.c
@@ -33,6 +33,7 @@ 
 
 static struct {
 	struct platform_device *pdev;
+	struct dss_device *dss;
 
 	bool update_enabled;
 	struct regulator *vdds_sdi_reg;
@@ -189,8 +190,8 @@  static int sdi_display_enable(struct omap_dss_device *dssdev)
 	 */
 	dispc_mgr_set_clock_div(channel, &sdi.mgr_config.clock_info);
 
-	dss_sdi_init(sdi.datapairs);
-	r = dss_sdi_enable();
+	dss_sdi_init(sdi.dss, sdi.datapairs);
+	r = dss_sdi_enable(sdi.dss);
 	if (r)
 		goto err_sdi_enable;
 	mdelay(2);
@@ -202,7 +203,7 @@  static int sdi_display_enable(struct omap_dss_device *dssdev)
 	return 0;
 
 err_mgr_enable:
-	dss_sdi_disable();
+	dss_sdi_disable(sdi.dss);
 err_sdi_enable:
 err_set_dss_clock_div:
 err_calc_clock_div:
@@ -219,7 +220,7 @@  static void sdi_display_disable(struct omap_dss_device *dssdev)
 
 	dss_mgr_disable(channel);
 
-	dss_sdi_disable();
+	dss_sdi_disable(sdi.dss);
 
 	dispc_runtime_put();
 
@@ -347,7 +348,8 @@  static void sdi_uninit_output(struct platform_device *pdev)
 	omapdss_unregister_output(out);
 }
 
-int sdi_init_port(struct platform_device *pdev, struct device_node *port)
+int sdi_init_port(struct dss_device *dss, struct platform_device *pdev,
+		  struct device_node *port)
 {
 	struct device_node *ep;
 	u32 datapairs;
@@ -364,6 +366,7 @@  int sdi_init_port(struct platform_device *pdev, struct device_node *port)
 	}
 
 	sdi.datapairs = datapairs;
+	sdi.dss = dss;
 
 	of_node_put(ep);