[11/33] fbdev/sh_mobile: remove sh_mobile_lcdc_display_notify
diff mbox series

Message ID 20190524085354.27411-12-daniel.vetter@ffwll.ch
State New
Headers show
Series
  • fbcon notifier begone!
Related show

Commit Message

Daniel Vetter May 24, 2019, 8:53 a.m. UTC
It's dead code, and removing it avoids me having to understand
what it's doing with lock_fb_info.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/video/fbdev/sh_mobile_lcdcfb.c | 63 --------------------------
 drivers/video/fbdev/sh_mobile_lcdcfb.h |  5 --
 2 files changed, 68 deletions(-)

Comments

Sam Ravnborg May 25, 2019, 3:01 p.m. UTC | #1
Hi Daniel

> It's dead code, and removing it avoids me having to understand
> what it's doing with lock_fb_info.

I pushed the series through my build tests which include the sh
architecture.

One error and one warning was triggered from sh_mobile_lcdcfb.c.
The rest was fine.

The patch below removed the sole user of
sh_mobile_lcdc_must_reconfigure() so this triggers a warning.

And I also get the following error:
drivers/video/fbdev/sh_mobile_lcdcfb.c: In function ‘sh_mobile_fb_reconfig’:
drivers/video/fbdev/sh_mobile_lcdcfb.c:1800:2: error: implicit declaration of function ‘fbcon_update_vcs’; did you mean ‘file_update_time’? [-Werror=implicit-function-declaration]
  fbcon_update_vcs(info, true);
  ^~~~~~~~~~~~~~~~
  file_update_time

I did not check but assume the error was triggered in patch 28 where
fbcon_update_vcs() in introduced.


Both are trivially fixed by appended patch.

	Sam

diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index bb1a610d0363..b8454424910d 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -15,6 +15,7 @@
 #include <linux/ctype.h>
 #include <linux/dma-mapping.h>
 #include <linux/delay.h>
+#include <linux/fbcon.h>
 #include <linux/gpio.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
@@ -533,25 +534,6 @@ static void sh_mobile_lcdc_display_off(struct sh_mobile_lcdc_chan *ch)
 		ch->tx_dev->ops->display_off(ch->tx_dev);
 }
 
-static bool
-sh_mobile_lcdc_must_reconfigure(struct sh_mobile_lcdc_chan *ch,
-				const struct fb_videomode *new_mode)
-{
-	dev_dbg(ch->info->dev, "Old %ux%u, new %ux%u\n",
-		ch->display.mode.xres, ch->display.mode.yres,
-		new_mode->xres, new_mode->yres);
-
-	/* It can be a different monitor with an equal video-mode */
-	if (fb_mode_is_equal(&ch->display.mode, new_mode))
-		return false;
-
-	dev_dbg(ch->info->dev, "Switching %u -> %u lines\n",
-		ch->display.mode.yres, new_mode->yres);
-	ch->display.mode = *new_mode;
-
-	return true;
-}
-
 static int sh_mobile_lcdc_check_var(struct fb_var_screeninfo *var,
 				    struct fb_info *info);
 

 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/video/fbdev/sh_mobile_lcdcfb.c | 63 --------------------------
>  drivers/video/fbdev/sh_mobile_lcdcfb.h |  5 --
>  2 files changed, 68 deletions(-)
> 
> diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> index dc46be38c970..c5924f5e98c6 100644
> --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
> +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> @@ -556,67 +556,6 @@ sh_mobile_lcdc_must_reconfigure(struct sh_mobile_lcdc_chan *ch,
>  static int sh_mobile_lcdc_check_var(struct fb_var_screeninfo *var,
>  				    struct fb_info *info);
>  
> -static int sh_mobile_lcdc_display_notify(struct sh_mobile_lcdc_chan *ch,
> -					 enum sh_mobile_lcdc_entity_event event,
> -					 const struct fb_videomode *mode,
> -					 const struct fb_monspecs *monspec)
> -{
> -	struct fb_info *info = ch->info;
> -	struct fb_var_screeninfo var;
> -	int ret = 0;
> -
> -	switch (event) {
> -	case SH_MOBILE_LCDC_EVENT_DISPLAY_CONNECT:
> -		/* HDMI plug in */
> -		console_lock();
> -		if (lock_fb_info(info)) {
> -
> -
> -			ch->display.width = monspec->max_x * 10;
> -			ch->display.height = monspec->max_y * 10;
> -
> -			if (!sh_mobile_lcdc_must_reconfigure(ch, mode) &&
> -			    info->state == FBINFO_STATE_RUNNING) {
> -				/* First activation with the default monitor.
> -				 * Just turn on, if we run a resume here, the
> -				 * logo disappears.
> -				 */
> -				info->var.width = ch->display.width;
> -				info->var.height = ch->display.height;
> -				sh_mobile_lcdc_display_on(ch);
> -			} else {
> -				/* New monitor or have to wake up */
> -				fb_set_suspend(info, 0);
> -			}
> -
> -
> -			unlock_fb_info(info);
> -		}
> -		console_unlock();
> -		break;
> -
> -	case SH_MOBILE_LCDC_EVENT_DISPLAY_DISCONNECT:
> -		/* HDMI disconnect */
> -		console_lock();
> -		if (lock_fb_info(info)) {
> -			fb_set_suspend(info, 1);
> -			unlock_fb_info(info);
> -		}
> -		console_unlock();
> -		break;
> -
> -	case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE:
> -		/* Validate a proposed new mode */
> -		fb_videomode_to_var(&var, mode);
> -		var.bits_per_pixel = info->var.bits_per_pixel;
> -		var.grayscale = info->var.grayscale;
> -		ret = sh_mobile_lcdc_check_var(&var, info);
> -		break;
> -	}
> -
> -	return ret;
> -}
> -
>  /* -----------------------------------------------------------------------------
>   * Format helpers
>   */
> @@ -2540,8 +2479,6 @@ sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch)
>  	unsigned int max_size;
>  	unsigned int i;
>  
> -	ch->notify = sh_mobile_lcdc_display_notify;
> -
>  	/* Validate the format. */
>  	format = sh_mobile_format_info(cfg->fourcc);
>  	if (format == NULL) {
> diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.h b/drivers/video/fbdev/sh_mobile_lcdcfb.h
> index b8e47a8bd8ab..589400372098 100644
> --- a/drivers/video/fbdev/sh_mobile_lcdcfb.h
> +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.h
> @@ -87,11 +87,6 @@ struct sh_mobile_lcdc_chan {
>  	unsigned long base_addr_c;
>  	unsigned int line_size;
>  
> -	int (*notify)(struct sh_mobile_lcdc_chan *ch,
> -		      enum sh_mobile_lcdc_entity_event event,
> -		      const struct fb_videomode *mode,
> -		      const struct fb_monspecs *monspec);
> -
>  	/* Backlight */
>  	struct backlight_device *bl;
>  	unsigned int bl_brightness;
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Daniel Vetter May 27, 2019, 6:13 a.m. UTC | #2
On Sat, May 25, 2019 at 05:01:59PM +0200, Sam Ravnborg wrote:
> Hi Daniel
> 
> > It's dead code, and removing it avoids me having to understand
> > what it's doing with lock_fb_info.
> 
> I pushed the series through my build tests which include the sh
> architecture.
> 
> One error and one warning was triggered from sh_mobile_lcdcfb.c.
> The rest was fine.
> 
> The patch below removed the sole user of
> sh_mobile_lcdc_must_reconfigure() so this triggers a warning.
> 
> And I also get the following error:
> drivers/video/fbdev/sh_mobile_lcdcfb.c: In function ‘sh_mobile_fb_reconfig’:
> drivers/video/fbdev/sh_mobile_lcdcfb.c:1800:2: error: implicit declaration of function ‘fbcon_update_vcs’; did you mean ‘file_update_time’? [-Werror=implicit-function-declaration]
>   fbcon_update_vcs(info, true);
>   ^~~~~~~~~~~~~~~~
>   file_update_time
> 
> I did not check but assume the error was triggered in patch 28 where
> fbcon_update_vcs() in introduced.

Oops. Can I have your sob so I can squash this in?

Thanks, Daniel

> 
> 
> Both are trivially fixed by appended patch.
> 
> 	Sam
> 
> diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> index bb1a610d0363..b8454424910d 100644
> --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
> +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> @@ -15,6 +15,7 @@
>  #include <linux/ctype.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/delay.h>
> +#include <linux/fbcon.h>
>  #include <linux/gpio.h>
>  #include <linux/init.h>
>  #include <linux/interrupt.h>
> @@ -533,25 +534,6 @@ static void sh_mobile_lcdc_display_off(struct sh_mobile_lcdc_chan *ch)
>  		ch->tx_dev->ops->display_off(ch->tx_dev);
>  }
>  
> -static bool
> -sh_mobile_lcdc_must_reconfigure(struct sh_mobile_lcdc_chan *ch,
> -				const struct fb_videomode *new_mode)
> -{
> -	dev_dbg(ch->info->dev, "Old %ux%u, new %ux%u\n",
> -		ch->display.mode.xres, ch->display.mode.yres,
> -		new_mode->xres, new_mode->yres);
> -
> -	/* It can be a different monitor with an equal video-mode */
> -	if (fb_mode_is_equal(&ch->display.mode, new_mode))
> -		return false;
> -
> -	dev_dbg(ch->info->dev, "Switching %u -> %u lines\n",
> -		ch->display.mode.yres, new_mode->yres);
> -	ch->display.mode = *new_mode;
> -
> -	return true;
> -}
> -
>  static int sh_mobile_lcdc_check_var(struct fb_var_screeninfo *var,
>  				    struct fb_info *info);
>  
> 
>  
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/video/fbdev/sh_mobile_lcdcfb.c | 63 --------------------------
> >  drivers/video/fbdev/sh_mobile_lcdcfb.h |  5 --
> >  2 files changed, 68 deletions(-)
> > 
> > diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> > index dc46be38c970..c5924f5e98c6 100644
> > --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
> > +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> > @@ -556,67 +556,6 @@ sh_mobile_lcdc_must_reconfigure(struct sh_mobile_lcdc_chan *ch,
> >  static int sh_mobile_lcdc_check_var(struct fb_var_screeninfo *var,
> >  				    struct fb_info *info);
> >  
> > -static int sh_mobile_lcdc_display_notify(struct sh_mobile_lcdc_chan *ch,
> > -					 enum sh_mobile_lcdc_entity_event event,
> > -					 const struct fb_videomode *mode,
> > -					 const struct fb_monspecs *monspec)
> > -{
> > -	struct fb_info *info = ch->info;
> > -	struct fb_var_screeninfo var;
> > -	int ret = 0;
> > -
> > -	switch (event) {
> > -	case SH_MOBILE_LCDC_EVENT_DISPLAY_CONNECT:
> > -		/* HDMI plug in */
> > -		console_lock();
> > -		if (lock_fb_info(info)) {
> > -
> > -
> > -			ch->display.width = monspec->max_x * 10;
> > -			ch->display.height = monspec->max_y * 10;
> > -
> > -			if (!sh_mobile_lcdc_must_reconfigure(ch, mode) &&
> > -			    info->state == FBINFO_STATE_RUNNING) {
> > -				/* First activation with the default monitor.
> > -				 * Just turn on, if we run a resume here, the
> > -				 * logo disappears.
> > -				 */
> > -				info->var.width = ch->display.width;
> > -				info->var.height = ch->display.height;
> > -				sh_mobile_lcdc_display_on(ch);
> > -			} else {
> > -				/* New monitor or have to wake up */
> > -				fb_set_suspend(info, 0);
> > -			}
> > -
> > -
> > -			unlock_fb_info(info);
> > -		}
> > -		console_unlock();
> > -		break;
> > -
> > -	case SH_MOBILE_LCDC_EVENT_DISPLAY_DISCONNECT:
> > -		/* HDMI disconnect */
> > -		console_lock();
> > -		if (lock_fb_info(info)) {
> > -			fb_set_suspend(info, 1);
> > -			unlock_fb_info(info);
> > -		}
> > -		console_unlock();
> > -		break;
> > -
> > -	case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE:
> > -		/* Validate a proposed new mode */
> > -		fb_videomode_to_var(&var, mode);
> > -		var.bits_per_pixel = info->var.bits_per_pixel;
> > -		var.grayscale = info->var.grayscale;
> > -		ret = sh_mobile_lcdc_check_var(&var, info);
> > -		break;
> > -	}
> > -
> > -	return ret;
> > -}
> > -
> >  /* -----------------------------------------------------------------------------
> >   * Format helpers
> >   */
> > @@ -2540,8 +2479,6 @@ sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch)
> >  	unsigned int max_size;
> >  	unsigned int i;
> >  
> > -	ch->notify = sh_mobile_lcdc_display_notify;
> > -
> >  	/* Validate the format. */
> >  	format = sh_mobile_format_info(cfg->fourcc);
> >  	if (format == NULL) {
> > diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.h b/drivers/video/fbdev/sh_mobile_lcdcfb.h
> > index b8e47a8bd8ab..589400372098 100644
> > --- a/drivers/video/fbdev/sh_mobile_lcdcfb.h
> > +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.h
> > @@ -87,11 +87,6 @@ struct sh_mobile_lcdc_chan {
> >  	unsigned long base_addr_c;
> >  	unsigned int line_size;
> >  
> > -	int (*notify)(struct sh_mobile_lcdc_chan *ch,
> > -		      enum sh_mobile_lcdc_entity_event event,
> > -		      const struct fb_videomode *mode,
> > -		      const struct fb_monspecs *monspec);
> > -
> >  	/* Backlight */
> >  	struct backlight_device *bl;
> >  	unsigned int bl_brightness;
> > -- 
> > 2.20.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Sam Ravnborg May 27, 2019, 6:52 a.m. UTC | #3
On Mon, May 27, 2019 at 08:13:06AM +0200, Daniel Vetter wrote:
> On Sat, May 25, 2019 at 05:01:59PM +0200, Sam Ravnborg wrote:
> > Hi Daniel
> > 
> > > It's dead code, and removing it avoids me having to understand
> > > what it's doing with lock_fb_info.
> > 
> > I pushed the series through my build tests which include the sh
> > architecture.
> > 
> > One error and one warning was triggered from sh_mobile_lcdcfb.c.
> > The rest was fine.
> > 
> > The patch below removed the sole user of
> > sh_mobile_lcdc_must_reconfigure() so this triggers a warning.
> > 
> > And I also get the following error:
> > drivers/video/fbdev/sh_mobile_lcdcfb.c: In function ‘sh_mobile_fb_reconfig’:
> > drivers/video/fbdev/sh_mobile_lcdcfb.c:1800:2: error: implicit declaration of function ‘fbcon_update_vcs’; did you mean ‘file_update_time’? [-Werror=implicit-function-declaration]
> >   fbcon_update_vcs(info, true);
> >   ^~~~~~~~~~~~~~~~
> >   file_update_time
> > 
> > I did not check but assume the error was triggered in patch 28 where
> > fbcon_update_vcs() in introduced.
> 
> Oops. Can I have your sob so I can squash this in?
Yes, here we go. (It was trivial so I thought not needed, sorry)
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>

Patch
diff mbox series

diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index dc46be38c970..c5924f5e98c6 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -556,67 +556,6 @@  sh_mobile_lcdc_must_reconfigure(struct sh_mobile_lcdc_chan *ch,
 static int sh_mobile_lcdc_check_var(struct fb_var_screeninfo *var,
 				    struct fb_info *info);
 
-static int sh_mobile_lcdc_display_notify(struct sh_mobile_lcdc_chan *ch,
-					 enum sh_mobile_lcdc_entity_event event,
-					 const struct fb_videomode *mode,
-					 const struct fb_monspecs *monspec)
-{
-	struct fb_info *info = ch->info;
-	struct fb_var_screeninfo var;
-	int ret = 0;
-
-	switch (event) {
-	case SH_MOBILE_LCDC_EVENT_DISPLAY_CONNECT:
-		/* HDMI plug in */
-		console_lock();
-		if (lock_fb_info(info)) {
-
-
-			ch->display.width = monspec->max_x * 10;
-			ch->display.height = monspec->max_y * 10;
-
-			if (!sh_mobile_lcdc_must_reconfigure(ch, mode) &&
-			    info->state == FBINFO_STATE_RUNNING) {
-				/* First activation with the default monitor.
-				 * Just turn on, if we run a resume here, the
-				 * logo disappears.
-				 */
-				info->var.width = ch->display.width;
-				info->var.height = ch->display.height;
-				sh_mobile_lcdc_display_on(ch);
-			} else {
-				/* New monitor or have to wake up */
-				fb_set_suspend(info, 0);
-			}
-
-
-			unlock_fb_info(info);
-		}
-		console_unlock();
-		break;
-
-	case SH_MOBILE_LCDC_EVENT_DISPLAY_DISCONNECT:
-		/* HDMI disconnect */
-		console_lock();
-		if (lock_fb_info(info)) {
-			fb_set_suspend(info, 1);
-			unlock_fb_info(info);
-		}
-		console_unlock();
-		break;
-
-	case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE:
-		/* Validate a proposed new mode */
-		fb_videomode_to_var(&var, mode);
-		var.bits_per_pixel = info->var.bits_per_pixel;
-		var.grayscale = info->var.grayscale;
-		ret = sh_mobile_lcdc_check_var(&var, info);
-		break;
-	}
-
-	return ret;
-}
-
 /* -----------------------------------------------------------------------------
  * Format helpers
  */
@@ -2540,8 +2479,6 @@  sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch)
 	unsigned int max_size;
 	unsigned int i;
 
-	ch->notify = sh_mobile_lcdc_display_notify;
-
 	/* Validate the format. */
 	format = sh_mobile_format_info(cfg->fourcc);
 	if (format == NULL) {
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.h b/drivers/video/fbdev/sh_mobile_lcdcfb.h
index b8e47a8bd8ab..589400372098 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.h
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.h
@@ -87,11 +87,6 @@  struct sh_mobile_lcdc_chan {
 	unsigned long base_addr_c;
 	unsigned int line_size;
 
-	int (*notify)(struct sh_mobile_lcdc_chan *ch,
-		      enum sh_mobile_lcdc_entity_event event,
-		      const struct fb_videomode *mode,
-		      const struct fb_monspecs *monspec);
-
 	/* Backlight */
 	struct backlight_device *bl;
 	unsigned int bl_brightness;