[PATCH 0/3] drm/qxl: video mode tweaks

[PATCH 0/3] drm/qxl: video mode tweaks

[Gerd Hoffmann]

Gerd Hoffmann (3):
  drm/qxl: add mode/framebuffer check functions
  drm/qxl: add qxl_add_mode helper function
  drm/qxl: use kernel mode db

 drivers/gpu/drm/qxl/qxl_display.c | 138 +++++++++++++++++++-------------------
 1 file changed, 70 insertions(+), 68 deletions(-)

-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/3] drm/qxl: add mode/framebuffer check functions

[Gerd Hoffmann]

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)
+		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)
-- 
2.9.3

Re: [PATCH 1/3] drm/qxl: add mode/framebuffer check functions

[Frediano Ziglio]

> 
> 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
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 1/3] drm/qxl: add mode/framebuffer check functions

[Gerd Hoffmann]

> > +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

Re: [Spice-devel] [PATCH 1/3] drm/qxl: add mode/framebuffer check functions

[Daniel Vetter]

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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 2/3] drm/qxl: add qxl_add_mode helper function

[Gerd Hoffmann]

Add a helper function to add custom video modes to a connector.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_display.c | 84 +++++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 07a37d52c4..b87f72f59e 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -206,15 +206,36 @@ static int qxl_check_framebuffer(struct qxl_device *qdev,
 	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)
+static int qxl_add_mode(struct drm_connector *connector,
+			unsigned int width,
+			unsigned int height,
+			bool preferred)
+{
+	struct drm_device *dev = connector->dev;
+	struct qxl_device *qdev = dev->dev_private;
+	struct drm_display_mode *mode = NULL;
+	int rc;
+
+	rc = qxl_check_mode(qdev, width, height);
+	if (rc != 0)
+		return 0;
+
+	mode = drm_cvt_mode(dev, width, height, 60, false, false, false);
+	if (preferred)
+		mode->type |= DRM_MODE_TYPE_PREFERRED;
+	mode->hdisplay = width;
+	mode->vdisplay = height;
+	drm_mode_set_name(mode);
+	drm_mode_probed_add(connector, mode);
+	return 1;
+}
+
+static int qxl_add_monitors_config_modes(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
 	struct qxl_device *qdev = dev->dev_private;
 	struct qxl_output *output = drm_connector_to_qxl_output(connector);
 	int h = output->index;
-	struct drm_display_mode *mode = NULL;
 	struct qxl_head *head;
 
 	if (!qdev->monitors_config)
@@ -229,19 +250,7 @@ static int qxl_add_monitors_config_modes(struct drm_connector *connector,
 	head = &qdev->client_monitors_config->heads[h];
 	DRM_DEBUG_KMS("head %d is %dx%d\n", h, head->width, head->height);
 
-	mode = drm_cvt_mode(dev, head->width, head->height, 60, false, false,
-			    false);
-	mode->type |= DRM_MODE_TYPE_PREFERRED;
-	mode->hdisplay = head->width;
-	mode->vdisplay = head->height;
-	drm_mode_set_name(mode);
-	*pwidth = head->width;
-	*pheight = head->height;
-	drm_mode_probed_add(connector, mode);
-	/* remember the last custom size for mode validation */
-	qdev->monitors_config_width = mode->hdisplay;
-	qdev->monitors_config_height = mode->vdisplay;
-	return 1;
+	return qxl_add_mode(connector, head->width, head->height, true);
 }
 
 static struct mode_size {
@@ -267,22 +276,16 @@ static struct mode_size {
 	{1920, 1200}
 };
 
-static int qxl_add_common_modes(struct drm_connector *connector,
-                                unsigned int pwidth,
-                                unsigned int pheight)
+static int qxl_add_common_modes(struct drm_connector *connector)
 {
-	struct drm_device *dev = connector->dev;
-	struct drm_display_mode *mode = NULL;
-	int i;
+	int i, ret = 0;
 
-	for (i = 0; i < ARRAY_SIZE(common_modes); i++) {
-		mode = drm_cvt_mode(dev, common_modes[i].w, common_modes[i].h,
-				    60, false, false, false);
-		if (common_modes[i].w == pwidth && common_modes[i].h == pheight)
-			mode->type |= DRM_MODE_TYPE_PREFERRED;
-		drm_mode_probed_add(connector, mode);
-	}
-	return i - 1;
+	for (i = 0; i < ARRAY_SIZE(common_modes); i++)
+		ret += qxl_add_mode(connector,
+				    common_modes[i].w,
+				    common_modes[i].h,
+				    false);
+	return ret;
 }
 
 static void qxl_send_monitors_config(struct qxl_device *qdev)
@@ -935,14 +938,25 @@ static int qdev_crtc_init(struct drm_device *dev, int crtc_id)
 
 static int qxl_conn_get_modes(struct drm_connector *connector)
 {
+	struct drm_device *dev = connector->dev;
+	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_output *output = drm_connector_to_qxl_output(connector);
 	unsigned int pwidth = 1024;
 	unsigned int pheight = 768;
 	int ret = 0;
 
-	ret = qxl_add_monitors_config_modes(connector, &pwidth, &pheight);
-	if (ret < 0)
-		return ret;
-	ret += qxl_add_common_modes(connector, pwidth, pheight);
+	if (qdev->client_monitors_config) {
+		struct qxl_head *head;
+		head = &qdev->client_monitors_config->heads[output->index];
+		if (head->width)
+			pwidth = head->width;
+		if (head->height)
+			pheight = head->height;
+	}
+
+	ret += qxl_add_common_modes(connector);
+	ret += qxl_add_monitors_config_modes(connector);
+	drm_set_preferred_mode(connector, pwidth, pheight);
 	return ret;
 }
 
-- 
2.9.3

_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[PATCH 3/3] drm/qxl: use kernel mode db

[Gerd Hoffmann]

Add all standard modes from the kernel's video mode data base.
Keep a few non-standard modes in the qxl mode list.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_display.c | 27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index b87f72f59e..9e49472872 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -256,34 +256,20 @@ static int qxl_add_monitors_config_modes(struct drm_connector *connector)
 static struct mode_size {
 	int w;
 	int h;
-} common_modes[] = {
-	{ 640,  480},
+} extra_modes[] = {
 	{ 720,  480},
-	{ 800,  600},
-	{ 848,  480},
-	{1024,  768},
 	{1152,  768},
-	{1280,  720},
-	{1280,  800},
 	{1280,  854},
-	{1280,  960},
-	{1280, 1024},
-	{1440,  900},
-	{1400, 1050},
-	{1680, 1050},
-	{1600, 1200},
-	{1920, 1080},
-	{1920, 1200}
 };
 
-static int qxl_add_common_modes(struct drm_connector *connector)
+static int qxl_add_extra_modes(struct drm_connector *connector)
 {
 	int i, ret = 0;
 
-	for (i = 0; i < ARRAY_SIZE(common_modes); i++)
+	for (i = 0; i < ARRAY_SIZE(extra_modes); i++)
 		ret += qxl_add_mode(connector,
-				    common_modes[i].w,
-				    common_modes[i].h,
+				    extra_modes[i].w,
+				    extra_modes[i].h,
 				    false);
 	return ret;
 }
@@ -954,7 +940,8 @@ static int qxl_conn_get_modes(struct drm_connector *connector)
 			pheight = head->height;
 	}
 
-	ret += qxl_add_common_modes(connector);
+	ret += drm_add_modes_noedid(connector, 8192, 8192);
+	ret += qxl_add_extra_modes(connector);
 	ret += qxl_add_monitors_config_modes(connector);
 	drm_set_preferred_mode(connector, pwidth, pheight);
 	return ret;
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel