Thread (33 messages) 33 messages, 5 authors, 2008-09-04

[PATCH] Add fb_check_var() for fixed mode device.

From: Takashi Yoshii <hidden>
Date: 2008-09-04 01:56:39
Subsystem: framebuffer layer, the rest · Maintainers: Helge Deller, Linus Torvalds

Take 2.

This adds sanitizing. But main effect is, as a result, adding
 FBIOPUT_VSCREENINFO on fixed mode device following functionality.
. return error when request screen is bigger than the fixed mode.
. round pan values if invalid
. do set pan
same as drivers which has own check_var.
quoted
quoted
quoted
+	/* do round _up_ */
+	if (var->xres_virtual < xoffset + var->xres)
+		var->xres_virtual = xoffset + var->xres;
+	if (var->yres_virtual < yoffset + yres)
+		var->yres_virtual = yoffset + yres;
Why this part? var->[xy]res{,_virtual} will be overwritten by the
correct values later anyway.
This was to a part of pan check rounding [xy]res_virtual up instead of
 rounding [xy]offset down, and check them in lines below there.
But as you pointed, it's weired for this(checking against constants) case.
quoted
+	/* pan is acceptable only if we have fb_pan_display) */
+	if ( (var->yoffset || var->xoffset) && !info->fbops->fb_pan_display )
+		return -EINVAL;
I changed this non-error, but resetting pan values with info->var.
This supports HW which can't change pan but have initial fixed offsets.
# So far, not in kernel source, though.
quoted
You should validate var->[xy]offset against constant->[xy]res_virtual
and info->fix.[xy]{pan,wrap}step.
I add explicit rounding down code this time, instead of returning error.
I am not sure which is right. But because I found 10 drivers round them down,
 6 return error, 2 set const value. 
# and 80 (20(with pan) + 60(no pan)) ignore them.

/yoshii

drivers/video/fbmem.c:fb_check_var(): New function
 for fixed mode device which doesn't provide its own check_var function.
   
Signed-off-by: Takashi YOSHII <redacted>
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index c6b8e92..243e5f4 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -904,6 +904,68 @@ static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var,
 	return err;
 }
 
+/* Sanity check for drivers which can't change video mode */
+static int
+fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
+{
+	struct fb_var_screeninfo *constant = &info->var;
+	struct fb_fix_screeninfo *fix = &info->fix;
+	__u32 xoffset = var->xoffset;
+	__u32 yoffset = var->yoffset;
+	__u32 activate = var->activate;
+	__u32 ywrap = var->vmode & FB_VMODE_YWRAP;
+
+	/* Bigger is error, smaller is OK */
+	if ((var->xres > constant->xres) ||
+	    (var->yres > constant->yres) ||
+	    (var->xres_virtual > constant->xres_virtual) ||
+	    (var->yres_virtual > constant->yres_virtual))
+		return -EINVAL;
+
+	/* Ohly length are checked for bitfields */
+	if ((var->bits_per_pixel > constant->bits_per_pixel) ||
+	    (var->red.length > constant->red.length) ||
+	    (var->green.length > constant->green.length) ||
+	    (var->blue.length > constant->blue.length))
+		return -EINVAL;
+
+	/* Boolean parameters can't be rounded, should be equal */
+	if ((!var->grayscale != !constant->grayscale) ||
+	    (!var->nonstd != !constant->nonstd) ||
+	    ((var->vmode ^ constant->vmode) &~FB_VMODE_YWRAP))
+		return -EINVAL;
+
+	/* round offsets */
+	if (xoffset > constant->xres_virtual - constant->xres)
+		xoffset = constant->xres_virtual - constant->xres;
+	if (fix->xpanstep)
+		xoffset -= (var->xoffset % fix->xpanstep);
+
+	if (ywrap) {
+		if (yoffset > constant->yres_virtual -1)
+			yoffset = constant->yres_virtual -1;
+		if (fix->ywrapstep)
+			yoffset -= (yoffset % fix->ywrapstep);
+	} else {
+		if (yoffset > constant->yres_virtual - constant->yres)
+			yoffset = constant->yres_virtual - constant->yres;
+		if (fix->ypanstep)
+			yoffset -= (yoffset % fix->ypanstep);
+	}
+
+	/* Copy most */
+	*var = *constant;
+	/* Restore some that are not a part of video mode */
+	var->activate = activate;
+	/*  You can't alter pan parameters without fb_pan_display() */
+	if ( info->fbops->fb_pan_display ) {
+		var->xoffset = xoffset;
+		var->yoffset = yoffset;
+		var->vmode = (constant->vmode &~FB_VMODE_YWRAP) | ywrap;
+	}
+	return 0;
+}
+
 int
 fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 {
@@ -940,9 +1002,11 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 
 		if (info->fbops->fb_check_var) {
 			ret = info->fbops->fb_check_var(var, info);
-			if (ret)
-				goto done;
-		}
+		} else
+			ret = fb_check_var(var, info);
+
+		if (ret)
+			goto done;
 
 		if ((var->activate & FB_ACTIVATE_MASK) == FB_ACTIVATE_NOW) {
 			struct fb_videomode mode;



-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help