[PATCH 5/5] videomode: rename fields

Steffen Trumtrar s.trumtrar at pengutronix.de
Tue Mar 12 06:53:12 PDT 2013


On Tue, Mar 12, 2013 at 12:19:38PM +0200, Tomi Valkeinen wrote:
> Structs videomode and display_timing have rather long field names for
> the timing values. Nothing wrong with that as such, but this patch
> changes them to abbreviations for the following reasons:
> 
> * The timing values often need to be used in calculations, and long
>   field names makes their direct use clumsier.
> 
> * The current names are a bit of a mishmash: some words are used as
>   such, some are shortened, and for some only first letter is used. Some
>   names use underscode, some don't. All this makes it difficult to
>   remember what the field names are.
> 
> * The abbreviations used in this patch are very common, and there
>   shouldn't be any misunderstanding about their meaning.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
> Cc: Steffen Trumtrar <s.trumtrar at pengutronix.de>
> ---
>  drivers/gpu/drm/drm_modes.c       |   18 +++++++++---------
>  drivers/video/fbmon.c             |   24 +++++++++++-------------
>  drivers/video/of_display_timing.c |   16 ++++++++--------
>  drivers/video/videomode.c         |   16 ++++++++--------
>  include/video/display_timing.h    |   16 ++++++++--------
>  include/video/videomode.h         |   18 +++++++++---------
>  6 files changed, 53 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index f83f071..d744e95 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -510,15 +510,15 @@ EXPORT_SYMBOL(drm_gtf_mode);
>  int drm_display_mode_from_videomode(const struct videomode *vm,
>  				    struct drm_display_mode *dmode)
>  {
> -	dmode->hdisplay = vm->hactive;
> -	dmode->hsync_start = dmode->hdisplay + vm->hfront_porch;
> -	dmode->hsync_end = dmode->hsync_start + vm->hsync_len;
> -	dmode->htotal = dmode->hsync_end + vm->hback_porch;
> +	dmode->hdisplay = vm->hact;
> +	dmode->hsync_start = dmode->hdisplay + vm->hfp;
> +	dmode->hsync_end = dmode->hsync_start + vm->hsl;
> +	dmode->htotal = dmode->hsync_end + vm->hbp;
>  
> -	dmode->vdisplay = vm->vactive;
> -	dmode->vsync_start = dmode->vdisplay + vm->vfront_porch;
> -	dmode->vsync_end = dmode->vsync_start + vm->vsync_len;
> -	dmode->vtotal = dmode->vsync_end + vm->vback_porch;
> +	dmode->vdisplay = vm->vact;
> +	dmode->vsync_start = dmode->vdisplay + vm->vfp;
> +	dmode->vsync_end = dmode->vsync_start + vm->vsl;
> +	dmode->vtotal = dmode->vsync_end + vm->vbp;
>  
>  	dmode->clock = vm->pixelclock / 1000;
>  
> @@ -565,7 +565,7 @@ int of_get_drm_display_mode(struct device_node *np,
>  	drm_display_mode_from_videomode(&vm, dmode);
>  
>  	pr_debug("%s: got %dx%d display mode from %s\n",
> -		of_node_full_name(np), vm.hactive, vm.vactive, np->name);
> +		of_node_full_name(np), vm.hact, vm.vact, np->name);
>  	drm_mode_debug_printmodeline(dmode);
>  
>  	return 0;
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index e5cc2fd..8103fc9 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -1382,15 +1382,15 @@ int fb_videomode_from_videomode(const struct videomode *vm,
>  {
>  	unsigned int htotal, vtotal;
>  
> -	fbmode->xres = vm->hactive;
> -	fbmode->left_margin = vm->hback_porch;
> -	fbmode->right_margin = vm->hfront_porch;
> -	fbmode->hsync_len = vm->hsync_len;
> +	fbmode->xres = vm->hact;
> +	fbmode->left_margin = vm->hbp;
> +	fbmode->right_margin = vm->hfp;
> +	fbmode->hsync_len = vm->hsl;
>  
> -	fbmode->yres = vm->vactive;
> -	fbmode->upper_margin = vm->vback_porch;
> -	fbmode->lower_margin = vm->vfront_porch;
> -	fbmode->vsync_len = vm->vsync_len;
> +	fbmode->yres = vm->vact;
> +	fbmode->upper_margin = vm->vbp;
> +	fbmode->lower_margin = vm->vfp;
> +	fbmode->vsync_len = vm->vsl;
>  
>  	/* prevent division by zero in KHZ2PICOS macro */
>  	fbmode->pixclock = vm->pixelclock ?
> @@ -1408,10 +1408,8 @@ int fb_videomode_from_videomode(const struct videomode *vm,
>  		fbmode->vmode |= FB_VMODE_DOUBLE;
>  	fbmode->flag = 0;
>  
> -	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> -		 vm->hsync_len;
> -	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> -		 vm->vsync_len;
> +	htotal = vm->hact + vm->hfp + vm->hbp + vm->hsl;
> +	vtotal = vm->vact + vm->vfp + vm->vbp + vm->vsl;
>  	/* prevent division by zero */
>  	if (htotal && vtotal) {
>  		fbmode->refresh = vm->pixelclock / (htotal * vtotal);
> @@ -1458,7 +1456,7 @@ int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
>  	fb_videomode_from_videomode(&vm, fb);
>  
>  	pr_debug("%s: got %dx%d display mode from %s\n",
> -		of_node_full_name(np), vm.hactive, vm.vactive, np->name);
> +		of_node_full_name(np), vm.hact, vm.vact, np->name);
>  	dump_fb_videomode(fb);
>  
>  	return 0;
> diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
> index 56009bc..79660937 100644
> --- a/drivers/video/of_display_timing.c
> +++ b/drivers/video/of_display_timing.c
> @@ -69,14 +69,14 @@ static struct display_timing *of_get_display_timing(struct device_node *np)
>  		return NULL;
>  	}
>  
> -	ret |= parse_timing_property(np, "hback-porch", &dt->hback_porch);
> -	ret |= parse_timing_property(np, "hfront-porch", &dt->hfront_porch);
> -	ret |= parse_timing_property(np, "hactive", &dt->hactive);
> -	ret |= parse_timing_property(np, "hsync-len", &dt->hsync_len);
> -	ret |= parse_timing_property(np, "vback-porch", &dt->vback_porch);
> -	ret |= parse_timing_property(np, "vfront-porch", &dt->vfront_porch);
> -	ret |= parse_timing_property(np, "vactive", &dt->vactive);
> -	ret |= parse_timing_property(np, "vsync-len", &dt->vsync_len);
> +	ret |= parse_timing_property(np, "hback-porch", &dt->hbp);
> +	ret |= parse_timing_property(np, "hfront-porch", &dt->hfp);
> +	ret |= parse_timing_property(np, "hactive", &dt->hact);
> +	ret |= parse_timing_property(np, "hsync-len", &dt->hsl);
> +	ret |= parse_timing_property(np, "vback-porch", &dt->vbp);
> +	ret |= parse_timing_property(np, "vfront-porch", &dt->vfp);
> +	ret |= parse_timing_property(np, "vactive", &dt->vact);
> +	ret |= parse_timing_property(np, "vsync-len", &dt->vsl);
>  	ret |= parse_timing_property(np, "clock-frequency", &dt->pixelclock);
>  
>  	dt->flags = 0;
> diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
> index a3d95f2..c42689a 100644
> --- a/drivers/video/videomode.c
> +++ b/drivers/video/videomode.c
> @@ -21,15 +21,15 @@ int videomode_from_timing(const struct display_timings *disp,
>  		return -EINVAL;
>  
>  	vm->pixelclock = dt->pixelclock.typ;
> -	vm->hactive = dt->hactive.typ;
> -	vm->hfront_porch = dt->hfront_porch.typ;
> -	vm->hback_porch = dt->hback_porch.typ;
> -	vm->hsync_len = dt->hsync_len.typ;
> +	vm->hact = dt->hact.typ;
> +	vm->hfp = dt->hfp.typ;
> +	vm->hbp = dt->hbp.typ;
> +	vm->hsl = dt->hsl.typ;
>  
> -	vm->vactive = dt->vactive.typ;
> -	vm->vfront_porch = dt->vfront_porch.typ;
> -	vm->vback_porch = dt->vback_porch.typ;
> -	vm->vsync_len = dt->vsync_len.typ;
> +	vm->vact = dt->vact.typ;
> +	vm->vfp = dt->vfp.typ;
> +	vm->vbp = dt->vbp.typ;
> +	vm->vsl = dt->vsl.typ;
>  
>  	vm->flags = dt->flags;
>  
> diff --git a/include/video/display_timing.h b/include/video/display_timing.h
> index 5d0259b..db98ef9 100644
> --- a/include/video/display_timing.h
> +++ b/include/video/display_timing.h
> @@ -59,15 +59,15 @@ struct timing_entry {
>  struct display_timing {
>  	struct timing_entry pixelclock;
>  
> -	struct timing_entry hactive;		/* hor. active video */
> -	struct timing_entry hfront_porch;	/* hor. front porch */
> -	struct timing_entry hback_porch;	/* hor. back porch */
> -	struct timing_entry hsync_len;		/* hor. sync len */
> +	struct timing_entry hact;		/* hor. active video */
> +	struct timing_entry hfp;		/* hor. front porch */
> +	struct timing_entry hbp;		/* hor. back porch */
> +	struct timing_entry hsl;		/* hor. sync len */
>  
> -	struct timing_entry vactive;		/* ver. active video */
> -	struct timing_entry vfront_porch;	/* ver. front porch */
> -	struct timing_entry vback_porch;	/* ver. back porch */
> -	struct timing_entry vsync_len;		/* ver. sync len */
> +	struct timing_entry vact;		/* ver. active video */
> +	struct timing_entry vfp;		/* ver. front porch */
> +	struct timing_entry vbp;		/* ver. back porch */
> +	struct timing_entry vsl;		/* ver. sync len */
>  
>  	enum display_flags flags;		/* display flags */
>  };
> diff --git a/include/video/videomode.h b/include/video/videomode.h
> index 8b12e60..b601c0c 100644
> --- a/include/video/videomode.h
> +++ b/include/video/videomode.h
> @@ -19,15 +19,15 @@
>  struct videomode {
>  	unsigned long pixelclock;	/* pixelclock in Hz */
>  
> -	u32 hactive;
> -	u32 hfront_porch;
> -	u32 hback_porch;
> -	u32 hsync_len;
> -
> -	u32 vactive;
> -	u32 vfront_porch;
> -	u32 vback_porch;
> -	u32 vsync_len;
> +	u32 hact;
> +	u32 hfp;
> +	u32 hbp;
> +	u32 hsl;
> +
> +	u32 vact;
> +	u32 vfp;
> +	u32 vbp;
> +	u32 vsl;
>  
>  	enum display_flags flags; /* display flags */
>  };
> 

Hi,

I really don't like this. It may be shorter, but I think it makes it really
hard to read. And the direct mapping of DT<->C is lost.

Regards,
Steffen


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


More information about the dri-devel mailing list