Daniel Vetter <daniel.vetter@xxxxxxxx> writes: > Apparently drivers need to check all this stuff themselves, which for > most things makes sense I guess. And for everything else we luck out, > because modern distros stopped supporting any other fbdev drivers than > drm ones and I really don't want to argue anymore about who needs to > check stuff. Therefore fixing all this just for drm fbdev emulation is > good enough. > Agreed. > Note that var->active is not set or validated. This is just control > flow for fbmem.c and needs to be validated in there as needed. > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > Cc: Maxime Ripard <mripard@xxxxxxxxxx> > Cc: Thomas Zimmermann <tzimmermann@xxxxxxx> > --- [...] > > +static void __fill_var(struct fb_var_screeninfo *var, > + struct drm_framebuffer *fb) > +{ > + int i; > + > + var->xres_virtual = fb->width; > + var->yres_virtual = fb->height; > + var->accel_flags = FB_ACCELF_TEXT; > + var->bits_per_pixel = drm_format_info_bpp(fb->format, 0); > + > + var->height = var->width = 0; > + var->left_margin = var->right_margin = 0; > + var->upper_margin = var->lower_margin = 0; > + var->hsync_len = var->vsync_len = 0; > + var->sync = var->vmode = 0; > + var->rotate = 0; > + var->colorspace = 0; > + for (i = 0; i < 4; i++) > + var->reserved[i] = 0; > +} > + > /** > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var > * @var: screeninfo to check > @@ -1595,8 +1616,22 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > return -EINVAL; > } > > - var->xres_virtual = fb->width; > - var->yres_virtual = fb->height; > + __fill_var(var, fb); > + [...] There is the following here (in latest drm-misc/drm-misc-next at least): /* * Changes struct fb_var_screeninfo are currently not pushed back * to KMS, hence fail if different settings are requested. */ bpp = drm_format_info_bpp(format, 0); if (var->bits_per_pixel > bpp || var->xres > fb->width || var->yres > fb->height || var->xres_virtual > fb->width || var->yres_virtual > fb->height) { drm_dbg_kms(dev, "fb requested width/height/bpp can't fit in current fb " "request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n", var->xres, var->yres, var->bits_per_pixel, var->xres_virtual, var->yres_virtual, fb->width, fb->height, bpp); return -EINVAL; } but only the 'var->xres > fb->width || var->yres > fb->height' from the conditions checked could be false after your __fill_var() call above. You should drop the 'var->bits_per_pixel > bpp', 'var->xres_virtual > fb->width' and 'var->yres_virtual > fb->height' checks I believe since those will always be true. > + /* > + * fb_pan_display() validates this, but fb_set_par() doesn't and just > + * falls over. Note that __fill_var above adjusts y/res_virtual. > + */ > + if (var->yoffset > var->yres_virtual - var->yres || > + var->xoffset > var->xres_virtual - var->xres) > + return -EINVAL; > + > + /* We neither support grayscale nor FOURCC (also stored in here). */ > + if (var->grayscale > 0) > + return -EINVAL; > + > + if (var->nonstd) > + return -EINVAL; > > /* > * Workaround for SDL 1.2, which is known to be setting all pixel format > @@ -1612,11 +1647,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > drm_fb_helper_fill_pixel_fmt(var, format); > } > Other than what I mentioned, the patch makes sense to me. Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> -- Best regards, Javier Martinez Canillas Core Platforms Red Hat