[PATCH] drm/qxl: validate monitors config modes
> Due to some recent changes in
> drm_helper_probe_single_connector_modes_merge_bits(), old custom modes
> were not being pruned properly. In current kernels,
> drm_mode_validate_basic() is called to sanity-check each mode in the
> list. If the sanity-check passes, the mode's status gets set to to
> MODE_OK. In older kernels this check was not done, so old custom modes
> would still have a status of MODE_UNVERIFIED at this point, and would
> therefore be pruned later in the function.
>
> As a result of this new behavior, the list of modes for a device always
> includes every custom mode ever configured for the device, with the
> largest one listed first. Since desktop environments usually choose the
> first preferred mode when a hotplug event is emitted, this had the
> result of making it very difficult for the user to reduce the size of
> the display.
>
> The qxl driver did implement the mode_valid connector function, but it
> was empty. In order to restore the old behavior where old custom modes
> are pruned, we implement a proper mode_valid function for the qxl
> driver. This function now checks each mode against the last configured
> custom mode and the list of standard modes. If the mode doesn't match
> any of these, its status is set to MODE_BAD so that it will be pruned as
> expected.
Hi Jonathon,
this seems reasonable, it is missing a Signed-off-by line though, and we should
probably also cc stable for it.
Dave.
> ---
> drivers/gpu/drm/qxl/qxl_display.c | 66
> ---
> drivers/gpu/drm/qxl/qxl_drv.h | 2 ++
> 2 files changed, 42 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c
> b/drivers/gpu/drm/qxl/qxl_display.c
> index a8dbb3e..7c6225c 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -160,9 +160,35 @@ static int qxl_add_monitors_config_modes(struct
> drm_connector *connector,
> *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;
> }
>
> +static struct mode_size {
> + int w;
> + int h;
> +} common_modes[] = {
> + { 640, 480},
> + { 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,
> unsigned pwidth,
> unsigned pheight)
> @@ -170,29 +196,6 @@ static int qxl_add_common_modes(struct drm_connector
> *connector,
> struct drm_device *dev = connector->dev;
> struct drm_display_mode *mode = NULL;
> int i;
> - struct mode_size {
> - int w;
> - int h;
> - } common_modes[] = {
> - { 640, 480},
> - { 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}
> - };
> -
> 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);
> @@ -823,11 +826,22 @@ static int qxl_conn_get_modes(struct drm_connector
> *connector)
> static int qxl_conn_mode_valid(struct drm_connector *connector,
>struct drm_display_mode *mode)
> {
> + 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 */
> - DRM_DEBUG("%s: %dx%d status=%d\n", mode->name, mode->hdisplay,
> - mode->vdisplay, mode->status);
> - return MODE_OK;
> +
> + 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)
> +
[PATCH] drm/qxl: validate monitors config modes
Due to some recent changes in
drm_helper_probe_single_connector_modes_merge_bits(), old custom modes
were not being pruned properly. In current kernels,
drm_mode_validate_basic() is called to sanity-check each mode in the
list. If the sanity-check passes, the mode's status gets set to to
MODE_OK. In older kernels this check was not done, so old custom modes
would still have a status of MODE_UNVERIFIED at this point, and would
therefore be pruned later in the function.
As a result of this new behavior, the list of modes for a device always
includes every custom mode ever configured for the device, with the
largest one listed first. Since desktop environments usually choose the
first preferred mode when a hotplug event is emitted, this had the
result of making it very difficult for the user to reduce the size of
the display.
The qxl driver did implement the mode_valid connector function, but it
was empty. In order to restore the old behavior where old custom modes
are pruned, we implement a proper mode_valid function for the qxl
driver. This function now checks each mode against the last configured
custom mode and the list of standard modes. If the mode doesn't match
any of these, its status is set to MODE_BAD so that it will be pruned as
expected.
---
drivers/gpu/drm/qxl/qxl_display.c | 66 ---
drivers/gpu/drm/qxl/qxl_drv.h | 2 ++
2 files changed, 42 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index a8dbb3e..7c6225c 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -160,9 +160,35 @@ static int qxl_add_monitors_config_modes(struct
drm_connector *connector,
*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;
}
+static struct mode_size {
+ int w;
+ int h;
+} common_modes[] = {
+ { 640, 480},
+ { 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,
unsigned pwidth,
unsigned pheight)
@@ -170,29 +196,6 @@ static int qxl_add_common_modes(struct drm_connector
*connector,
struct drm_device *dev = connector->dev;
struct drm_display_mode *mode = NULL;
int i;
- struct mode_size {
- int w;
- int h;
- } common_modes[] = {
- { 640, 480},
- { 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}
- };
-
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);
@@ -823,11 +826,22 @@ static int qxl_conn_get_modes(struct drm_connector
*connector)
static int qxl_conn_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
{
+ 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 */
- DRM_DEBUG("%s: %dx%d status=%d\n", mode->name, mode->hdisplay,
- mode->vdisplay, mode->status);
- return MODE_OK;
+
+ 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;
}
static struct drm_encoder *qxl_best_encoder(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index d854969..01a8694 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -325,6 +325,8 @@ struct qxl_device {
