| Message ID | 20190116083521.22673-2-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/qxl: video mode tweaks | expand |
> > Add a helper functions to check video modes. Also add a helper to check > framebuffer buffer objects, using the former for consistency. That way > we should not fail in qxl_primary_atomic_check() because video modes > which are too big will not be added to the mode list in the first place. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > drivers/gpu/drm/qxl/qxl_display.c | 37 +++++++++++++++++++------------------ > 1 file changed, 19 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/qxl/qxl_display.c > b/drivers/gpu/drm/qxl/qxl_display.c > index 1f8fddcc34..07a37d52c4 100644 > --- a/drivers/gpu/drm/qxl/qxl_display.c > +++ b/drivers/gpu/drm/qxl/qxl_display.c > @@ -191,6 +191,21 @@ void qxl_display_read_client_monitors_config(struct > qxl_device *qdev) > } > } > > +static int qxl_check_mode(struct qxl_device *qdev, > + unsigned int width, > + unsigned int height) > +{ > + if (width * height * 4 > qdev->vram_size) Is someone checking for integer overflows already? > + return -ENOMEM; > + return 0; > +} > + > +static int qxl_check_framebuffer(struct qxl_device *qdev, > + struct qxl_bo *bo) > +{ > + return qxl_check_mode(qdev, bo->surf.width, bo->surf.height); > +} > + > static int qxl_add_monitors_config_modes(struct drm_connector *connector, > unsigned *pwidth, > unsigned *pheight) > @@ -466,12 +481,7 @@ static int qxl_primary_atomic_check(struct drm_plane > *plane, > > bo = gem_to_qxl_bo(state->fb->obj[0]); > > - if (bo->surf.stride * bo->surf.height > qdev->vram_size) { > - DRM_ERROR("Mode doesn't fit in vram size (vgamem)"); > - return -EINVAL; > - } > - > - return 0; > + return qxl_check_framebuffer(qdev, bo); > } > > static int qxl_primary_apply_cursor(struct drm_plane *plane) > @@ -941,20 +951,11 @@ static enum drm_mode_status qxl_conn_mode_valid(struct > drm_connector *connector, > { > struct drm_device *ddev = connector->dev; > struct qxl_device *qdev = ddev->dev_private; > - int i; > > - /* TODO: is this called for user defined modes? (xrandr --add-mode) > - * TODO: check that the mode fits in the framebuffer */ > + if (qxl_check_mode(qdev, mode->hdisplay, mode->vdisplay) != 0) > + return MODE_BAD; > > - if (qdev->monitors_config_width == mode->hdisplay && > - qdev->monitors_config_height == mode->vdisplay) > - return MODE_OK; > - > - for (i = 0; i < ARRAY_SIZE(common_modes); i++) { > - if (common_modes[i].w == mode->hdisplay && common_modes[i].h == > mode->vdisplay) > - return MODE_OK; > - } > - return MODE_BAD; > + return MODE_OK; > } > > static struct drm_encoder *qxl_best_encoder(struct drm_connector *connector) Frediano
> > +static int qxl_check_mode(struct qxl_device *qdev, > > + unsigned int width, > > + unsigned int height) > > +{ > > + if (width * height * 4 > qdev->vram_size) > > Is someone checking for integer overflows already? Need to have a look. This is just ... > > - if (bo->surf.stride * bo->surf.height > qdev->vram_size) { > > - DRM_ERROR("Mode doesn't fit in vram size (vgamem)"); > > - return -EINVAL; > > - } ... that check moved into the new function. cheers, Gerd
On Wed, Jan 16, 2019 at 12:28:00PM +0100, Gerd Hoffmann wrote: > > > +static int qxl_check_mode(struct qxl_device *qdev, > > > + unsigned int width, > > > + unsigned int height) > > > +{ > > > + if (width * height * 4 > qdev->vram_size) > > > > Is someone checking for integer overflows already? > > Need to have a look. This is just ... The addfb ioctl checks for integer overflows for you. -Daniel > > > > - if (bo->surf.stride * bo->surf.height > qdev->vram_size) { > > > - DRM_ERROR("Mode doesn't fit in vram size (vgamem)"); > > > - return -EINVAL; > > > - } > > ... that check moved into the new function. > > cheers, > Gerd >
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index 1f8fddcc34..07a37d52c4 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -191,6 +191,21 @@ void qxl_display_read_client_monitors_config(struct qxl_device *qdev) } } +static int qxl_check_mode(struct qxl_device *qdev, + unsigned int width, + unsigned int height) +{ + if (width * height * 4 > qdev->vram_size) + return -ENOMEM; + return 0; +} + +static int qxl_check_framebuffer(struct qxl_device *qdev, + struct qxl_bo *bo) +{ + return qxl_check_mode(qdev, bo->surf.width, bo->surf.height); +} + static int qxl_add_monitors_config_modes(struct drm_connector *connector, unsigned *pwidth, unsigned *pheight) @@ -466,12 +481,7 @@ static int qxl_primary_atomic_check(struct drm_plane *plane, bo = gem_to_qxl_bo(state->fb->obj[0]); - if (bo->surf.stride * bo->surf.height > qdev->vram_size) { - DRM_ERROR("Mode doesn't fit in vram size (vgamem)"); - return -EINVAL; - } - - return 0; + return qxl_check_framebuffer(qdev, bo); } static int qxl_primary_apply_cursor(struct drm_plane *plane) @@ -941,20 +951,11 @@ static enum drm_mode_status qxl_conn_mode_valid(struct drm_connector *connector, { struct drm_device *ddev = connector->dev; struct qxl_device *qdev = ddev->dev_private; - int i; - /* TODO: is this called for user defined modes? (xrandr --add-mode) - * TODO: check that the mode fits in the framebuffer */ + if (qxl_check_mode(qdev, mode->hdisplay, mode->vdisplay) != 0) + return MODE_BAD; - if (qdev->monitors_config_width == mode->hdisplay && - qdev->monitors_config_height == mode->vdisplay) - return MODE_OK; - - for (i = 0; i < ARRAY_SIZE(common_modes); i++) { - if (common_modes[i].w == mode->hdisplay && common_modes[i].h == mode->vdisplay) - return MODE_OK; - } - return MODE_BAD; + return MODE_OK; } static struct drm_encoder *qxl_best_encoder(struct drm_connector *connector)
Add a helper functions to check video modes. Also add a helper to check framebuffer buffer objects, using the former for consistency. That way we should not fail in qxl_primary_atomic_check() because video modes which are too big will not be added to the mode list in the first place. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- drivers/gpu/drm/qxl/qxl_display.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-)