diff mbox series

video: ipuv3: remove some useless code to reduce binary size

Message ID 20200525123417.15968-1-agust@denx.de
State Accepted
Commit db755b36d2ee240cae30de9cccada8fe398b3eef
Headers show
Series video: ipuv3: remove some useless code to reduce binary size | expand

Commit Message

Anatolij Gustschin May 25, 2020, 12:34 p.m. UTC 
To enable DM_VIDEO we must decrease binary size to fix build
breakage for some boards, so drop not needed code. Also add
!DM_VIDEO guards which can be later removed when last non DM
users will be converted.

Signed-off-by: Anatolij Gustschin <agust at denx.de>
---
 drivers/video/imx/ipu_disp.c     | 12 -------
 drivers/video/imx/mxc_ipuv3_fb.c | 57 ++++++++++++--------------------
 2 files changed, 22 insertions(+), 47 deletions(-)

Comments

Tom Rini May 25, 2020, 4:05 p.m. UTC | #1 
On Mon, May 25, 2020 at 02:34:17PM +0200, Anatolij Gustschin wrote:

> To enable DM_VIDEO we must decrease binary size to fix build
> breakage for some boards, so drop not needed code. Also add
> !DM_VIDEO guards which can be later removed when last non DM
> users will be converted.
> 
> Signed-off-by: Anatolij Gustschin <agust at denx.de>

Given that the migration deadline for non-DM video boards was the
v2019.07 release, what's the case for not:
1. Perform the changes here, to save size.
2. Enable conversions that can now be enabled.
3. Disable video/drop platforms on unconverted boards.
4. Drop non-DM code here.

Thanks!
Anatolij Gustschin May 26, 2020, 9:03 p.m. UTC | #2 
On Mon, 25 May 2020 12:05:14 -0400
Tom Rini trini at konsulko.com wrote:

> On Mon, May 25, 2020 at 02:34:17PM +0200, Anatolij Gustschin wrote:
> 
> > To enable DM_VIDEO we must decrease binary size to fix build
> > breakage for some boards, so drop not needed code. Also add
> > !DM_VIDEO guards which can be later removed when last non DM
> > users will be converted.
> > 
> > Signed-off-by: Anatolij Gustschin <agust at denx.de>  
> 
> Given that the migration deadline for non-DM video boards was the
> v2019.07 release, what's the case for not:
> 1. Perform the changes here, to save size.

I've submitted more patches to save size, changes here were
not enough to fix building tbs2910 board with DM video enabled.

> 2. Enable conversions that can now be enabled.

All boards using ipuv3 driver are now converted, I'll submit a
pull request to merge the changes.

> 3. Disable video/drop platforms on unconverted boards.

For boards with only DM video conversion missing, it makes sense
to disable video, but I don't know is this can be done for all
such boards (some do not have serial console and rely on working
display output). Some boards do not even enable DM, these are
candidates to drop.

> 4. Drop non-DM code here.

Patch already submitted to drop non-DM code here.

--
Anatolij
Tom Rini May 26, 2020, 9:11 p.m. UTC | #3 
On Tue, May 26, 2020 at 11:03:15PM +0200, Anatolij Gustschin wrote:
> On Mon, 25 May 2020 12:05:14 -0400
> Tom Rini trini at konsulko.com wrote:
> 
> > On Mon, May 25, 2020 at 02:34:17PM +0200, Anatolij Gustschin wrote:
> > 
> > > To enable DM_VIDEO we must decrease binary size to fix build
> > > breakage for some boards, so drop not needed code. Also add
> > > !DM_VIDEO guards which can be later removed when last non DM
> > > users will be converted.
> > > 
> > > Signed-off-by: Anatolij Gustschin <agust at denx.de>  
> > 
> > Given that the migration deadline for non-DM video boards was the
> > v2019.07 release, what's the case for not:
> > 1. Perform the changes here, to save size.
> 
> I've submitted more patches to save size, changes here were
> not enough to fix building tbs2910 board with DM video enabled.

Thanks a lot for doing all of this.

> > 2. Enable conversions that can now be enabled.
> 
> All boards using ipuv3 driver are now converted, I'll submit a
> pull request to merge the changes.

OK, I'll take it for -next so you can wait a bit more or I'll queue it
up for when I open -next.

> > 3. Disable video/drop platforms on unconverted boards.
> 
> For boards with only DM video conversion missing, it makes sense
> to disable video, but I don't know is this can be done for all
> such boards (some do not have serial console and rely on working
> display output). Some boards do not even enable DM, these are
> candidates to drop.

It's a tough call.  I think just disabling video support, and CC'ing the
maintainer is fine.  That should cause one of:
- Maintainer says they'll get to converting, does so.
- Maintainer says they don't want to maintain the platform anymore (see
  mx23evk for example).
- Maintainer email bounces.

In the latter case we can (or rather, let me know, I will..) do a
removal patch.
diff mbox series

Patch

diff --git a/drivers/video/imx/ipu_disp.c b/drivers/video/imx/ipu_disp.c
index c2f00bff18..45069897fa 100644
--- a/drivers/video/imx/ipu_disp.c
+++ b/drivers/video/imx/ipu_disp.c
@@ -1191,9 +1191,6 @@  int32_t ipu_disp_set_global_alpha(ipu_channel_t channel, unsigned char enable,
 	else
 		bg_chan = 0;
 
-	if (!g_ipu_clk_enabled)
-		clk_enable(g_ipu_clk);
-
 	if (bg_chan) {
 		reg = __raw_readl(DP_COM_CONF());
 		__raw_writel(reg & ~DP_COM_CONF_GWSEL, DP_COM_CONF());
@@ -1217,9 +1214,6 @@  int32_t ipu_disp_set_global_alpha(ipu_channel_t channel, unsigned char enable,
 	reg = __raw_readl(IPU_SRM_PRI2) | 0x8;
 	__raw_writel(reg, IPU_SRM_PRI2);
 
-	if (!g_ipu_clk_enabled)
-		clk_disable(g_ipu_clk);
-
 	return 0;
 }
 
@@ -1246,9 +1240,6 @@  int32_t ipu_disp_set_color_key(ipu_channel_t channel, unsigned char enable,
 		(channel == MEM_BG_ASYNC1 || channel == MEM_FG_ASYNC1)))
 		return -EINVAL;
 
-	if (!g_ipu_clk_enabled)
-		clk_enable(g_ipu_clk);
-
 	color_key_4rgb = 1;
 	/* Transform color key from rgb to yuv if CSC is enabled */
 	if (((fg_csc_type == RGB2YUV) && (bg_csc_type == YUV2YUV)) ||
@@ -1286,8 +1277,5 @@  int32_t ipu_disp_set_color_key(ipu_channel_t channel, unsigned char enable,
 	reg = __raw_readl(IPU_SRM_PRI2) | 0x8;
 	__raw_writel(reg, IPU_SRM_PRI2);
 
-	if (!g_ipu_clk_enabled)
-		clk_disable(g_ipu_clk);
-
 	return 0;
 }
diff --git a/drivers/video/imx/mxc_ipuv3_fb.c b/drivers/video/imx/mxc_ipuv3_fb.c
index 4044473f99..2a0a355817 100644
--- a/drivers/video/imx/mxc_ipuv3_fb.c
+++ b/drivers/video/imx/mxc_ipuv3_fb.c
@@ -38,8 +38,10 @@  DECLARE_GLOBAL_DATA_PTR;
 static int mxcfb_map_video_memory(struct fb_info *fbi);
 static int mxcfb_unmap_video_memory(struct fb_info *fbi);
 
+#if !CONFIG_IS_ENABLED(DM_VIDEO)
 /* graphics setup */
 static GraphicDevice panel;
+#endif
 static struct fb_videomode const *gmode;
 static uint8_t gdisp;
 static uint32_t gpixfmt;
@@ -120,27 +122,6 @@  static uint32_t bpp_to_pixfmt(struct fb_info *fbi)
 	return pixfmt;
 }
 
-/*
- * Set fixed framebuffer parameters based on variable settings.
- *
- * @param       info     framebuffer information pointer
- */
-static int mxcfb_set_fix(struct fb_info *info)
-{
-	struct fb_fix_screeninfo *fix = &info->fix;
-	struct fb_var_screeninfo *var = &info->var;
-
-	fix->line_length = var->xres_virtual * var->bits_per_pixel / 8;
-
-	fix->type = FB_TYPE_PACKED_PIXELS;
-	fix->accel = FB_ACCEL_NONE;
-	fix->visual = FB_VISUAL_TRUECOLOR;
-	fix->xpanstep = 1;
-	fix->ypanstep = 1;
-
-	return 0;
-}
-
 static int setup_disp_channel1(struct fb_info *fbi)
 {
 	ipu_channel_params_t params;
@@ -226,7 +207,6 @@  static int mxcfb_set_par(struct fb_info *fbi)
 
 	ipu_disable_channel(mxc_fbi->ipu_ch);
 	ipu_uninit_channel(mxc_fbi->ipu_ch);
-	mxcfb_set_fix(fbi);
 
 	mem_len = fbi->var.yres_virtual * fbi->fix.line_length;
 	if (!fbi->fix.smem_start || (mem_len > fbi->fix.smem_len)) {
@@ -499,6 +479,8 @@  static struct fb_info *mxcfb_init_fbinfo(void)
 	return fbi;
 }
 
+extern struct clk *g_ipu_clk;
+
 /*
  * Probe routine for the framebuffer driver. It is called during the
  * driver binding process. The following functions are performed in
@@ -512,16 +494,14 @@  static int mxcfb_probe(u32 interface_pix_fmt, uint8_t disp,
 {
 	struct fb_info *fbi;
 	struct mxcfb_info *mxcfbi;
-	int ret = 0;
 
 	/*
 	 * Initialize FB structures
 	 */
 	fbi = mxcfb_init_fbinfo();
-	if (!fbi) {
-		ret = -ENOMEM;
-		goto err0;
-	}
+	if (!fbi)
+		return -ENOMEM;
+
 	mxcfbi = (struct mxcfb_info *)fbi->par;
 
 	if (!g_dp_in_use) {
@@ -534,9 +514,11 @@  static int mxcfb_probe(u32 interface_pix_fmt, uint8_t disp,
 
 	mxcfbi->ipu_di = disp;
 
+	if (!ipu_clk_enabled())
+		clk_enable(g_ipu_clk);
+
 	ipu_disp_set_global_alpha(mxcfbi->ipu_ch, 1, 0x80);
 	ipu_disp_set_color_key(mxcfbi->ipu_ch, 0, 0);
-	strcpy(fbi->fix.id, "DISP3 BG");
 
 	g_dp_in_use = 1;
 
@@ -547,7 +529,8 @@  static int mxcfb_probe(u32 interface_pix_fmt, uint8_t disp,
 	mxcfbi->ipu_di_pix_fmt = interface_pix_fmt;
 	fb_videomode_to_var(&fbi->var, mode);
 	fbi->var.bits_per_pixel = 16;
-	fbi->fix.line_length = fbi->var.xres * (fbi->var.bits_per_pixel / 8);
+	fbi->fix.line_length = fbi->var.xres_virtual *
+			       (fbi->var.bits_per_pixel / 8);
 	fbi->fix.smem_len = fbi->var.yres_virtual * fbi->fix.line_length;
 
 	mxcfb_check_var(&fbi->var, fbi);
@@ -555,14 +538,13 @@  static int mxcfb_probe(u32 interface_pix_fmt, uint8_t disp,
 	/* Default Y virtual size is 2x panel size */
 	fbi->var.yres_virtual = fbi->var.yres * 2;
 
-	mxcfb_set_fix(fbi);
-
 	/* allocate fb first */
 	if (mxcfb_map_video_memory(fbi) < 0)
 		return -ENOMEM;
 
 	mxcfb_set_par(fbi);
 
+#if !CONFIG_IS_ENABLED(DM_VIDEO)
 	panel.winSizeX = mode->xres;
 	panel.winSizeY = mode->yres;
 	panel.plnSizeX = mode->xres;
@@ -573,13 +555,12 @@  static int mxcfb_probe(u32 interface_pix_fmt, uint8_t disp,
 
 	panel.gdfBytesPP = 2;
 	panel.gdfIndex = GDF_16BIT_565RGB;
-
+#endif
+#ifdef DEBUG
 	ipu_dump_registers();
+#endif
 
 	return 0;
-
-err0:
-	return ret;
 }
 
 void ipuv3_fb_shutdown(void)
@@ -604,6 +585,7 @@  void ipuv3_fb_shutdown(void)
 	}
 }
 
+#if !CONFIG_IS_ENABLED(DM_VIDEO)
 void *video_hw_init(void)
 {
 	int ret;
@@ -618,6 +600,7 @@  void *video_hw_init(void)
 
 	return (void *)&panel;
 }
+#endif
 
 int ipuv3_fb_init(struct fb_videomode const *mode,
 		  uint8_t disp,
@@ -707,8 +690,12 @@  static int ipuv3_video_bind(struct udevice *dev)
 }
 
 static const struct udevice_id ipuv3_video_ids[] = {
+#ifdef CONFIG_ARCH_MX6
 	{ .compatible = "fsl,imx6q-ipu" },
+#endif
+#ifdef CONFIG_ARCH_MX5
 	{ .compatible = "fsl,imx53-ipu" },
+#endif
 	{ }
 };