similar to: [PATCH 2/4] qxl: limit monitor config read retries

When reading the monitor config fails, don't retry forever. If it fails ten times in a row just give up to avoid the driver hangs. Also add a small delay after each attempt, so the host has a chance to complete a partial update. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/qxl/qxl_display.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8

When reading the monitor config fails, don't retry forever. If it fails ten times in a row just give up to avoid the driver hangs. Also add a small delay after each attempt, so the host has a chance to complete a partial update. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/qxl/qxl_display.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8

qxl_io_log() sends messages over to the host (qemu) for logging. Remove the function and all callers, we can just use standard DRM_DEBUG calls (and if needed a serial console). Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/qxl/qxl_drv.h | 3 --- drivers/gpu/drm/qxl/qxl_cmd.c | 34 ++-------------------------------- drivers/gpu/drm/qxl/qxl_display.c | 27

qxl_io_log() sends messages over to the host (qemu) for logging. Remove the function and all callers, we can just use standard DRM_DEBUG calls (and if needed a serial console). Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/qxl/qxl_drv.h | 3 --- drivers/gpu/drm/qxl/qxl_cmd.c | 34 ++-------------------------------- drivers/gpu/drm/qxl/qxl_display.c | 27

If qxl_alloc_client_monitors_config() fails to allocate client_monitors_config then NULL pointer dereference occurs in function qxl_display_copy_rom_client_monitors_config() after qxl_alloc_client_monitors_config() call. The patch adds return error from qxl_alloc_client_monitors_config() and additional status for qxl_display_copy_rom_client_monitors_config return value. Found by Linux Driver

Skip the pointless and slightly confusing indirection via qdev->monitors_config->max_allowed. Just use qxl_num_crtc instead. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/qxl/qxl_display.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c

Just use qxl_num_crtc directly everywhere instead of using qdev->monitors_config->max_allowed. Drops pointless indirection and also is less confusing. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/qxl/qxl_display.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_display.c

qdev->monitors_config->max_allowed is effectively set by the qxl.num_heads module parameter, stored in the qxl_num_crtc variable. Lets get rid of the indirection and use the variable qxl_num_crtc directly. The kernel doesn't need to dereference pointers each time it needs the value, and when reading the code you don't have to trace where and why

Try to read the client monitors config at driver load time, even without explicit notification. So in case that info was filled before the driver loaded and we've missed the notifications because of that the settings will still be used. With that place we now have to take care to properly handle a empty client monitors config, so we don't trip over an uninitialized client monitors

Try to read the client monitors config at driver load time, even without explicit notification. So in case that info was filled before the driver loaded and we've missed the notifications because of that the settings will still be used. With that place we now have to take care to properly handle a empty client monitors config, so we don't trip over an uninitialized client monitors

Try to read the client monitors config at driver load time, even without explicit notification. So in case that info was filled before the driver loaded and we've missed the notifications because of that the settings will still be used. With that place we now have to take care to properly handle a empty client monitors config, so we don't trip over an uninitialized client monitors

Try to read the client monitors config at driver load time, even without explicit notification. So in case that info was filled before the driver loaded and we've missed the notifications because of that the settings will still be used. With that place we now have to take care to properly handle a empty client monitors config, so we don't trip over an uninitialized client monitors

qdev->monitors_config->max_allowed is effectively set by the qxl.num_heads module parameter, stored in the qxl_num_crtc variable. Lets get rid of the indirection and use the variable qxl_num_crtc directly. The kernel doesn't need to dereference pointers each time it needs the value, and when reading the code you don't have to trace where and why

Call qxl_add_monitors_config_modes() unconditionally. Do all sanity checks in that function. Fix sanity checks. monitors_config is the current monitor configuration, whereas client_monitors_config is the configuration requested by the spice client. So when filling the mode list, based on the spice client request, we need to look at client_monitors_config->count not

Call qxl_add_monitors_config_modes() unconditionally. Do all sanity checks in that function. Fix sanity checks. monitors_config is the current monitor configuration, whereas client_monitors_config is the configuration requested by the spice client. So when filling the mode list, based on the spice client request, we need to look at client_monitors_config->count not

Call qxl_add_monitors_config_modes() unconditionally. Do all sanity checks in that function. Fix sanity checks. monitors_config is the current monitor configuration, whereas client_monitors_config is the configuration requested by the spice client. So when filling the mode list, based on the spice client request, we need to look at client_monitors_config->count not

Call qxl_add_monitors_config_modes() unconditionally. Do all sanity checks in that function. Fix sanity checks. monitors_config is the current monitor configuration, whereas client_monitors_config is the configuration requested by the spice client. So when filling the mode list, based on the spice client request, we need to look at client_monitors_config->count not

Hi Sean. On Mon, Aug 17, 2020 at 03:58:38PM -0400, Sean Paul wrote: > From: Sean Paul <seanpaul at chromium.org> > > Introduced in the patch below, the END macro was missing 'dev' and BEGIN > macro needs drm_drv_uses_atomic_modeset() from drm_drv.h > > Fixes: bbaac1354cc9 ("drm/qxl: Replace deprecated function in qxl_display") We should not use Fixes

The encoder callbacks are only called in case the video mode changes. So any layout changes without mode changes will go unnoticed. Add qxl_crtc_update_monitors_config(), based on the old qxl_write_monitors_config_for_encoder() function. Hook it into the enable, disable and flush atomic crtc callbacks. Remove monitors_config updates from all other places. Fixes:

The encoder callbacks are only called in case the video mode changes. So any layout changes without mode changes will go unnoticed. Add qxl_crtc_update_monitors_config(), based on the old qxl_write_monitors_config_for_encoder() function. Hook it into the enable, disable and flush atomic crtc callbacks. Remove monitors_config updates from all other places. Fixes: