similar to: [PATCH v2 2/4] qxl: move qxl_send_monitors_config()

Needed to avoid a forward declaration in a followup patch. Pure code move, no functional change. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/qxl/qxl_display.c | 47 +++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index

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:

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

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

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

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

The qxl device supports only a single active framebuffer ("primary surface" in spice terminology). In multihead configurations are handled by defining rectangles within the primary surface for each head/crtc. Userspace which uses the qxl ioctl interface (xorg qxl driver) is aware of this limitation and will setup framebuffers and crtcs accordingly. Userspace which uses dumb

The qxl device supports only a single active framebuffer ("primary surface" in spice terminology). In multihead configurations are handled by defining rectangles within the primary surface for each head/crtc. Userspace which uses the qxl ioctl interface (xorg qxl driver) is aware of this limitation and will setup framebuffers and crtcs accordingly. Userspace which uses dumb

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

Track which bo is used as primary surface. With that in place we don't need the primary_created flag any more, we can just check the primary bo pointer instead. Also verify we don't already have a primary surface in qxl_io_create_primary(). Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/qxl/qxl_drv.h | 2 +- drivers/gpu/drm/qxl/qxl_cmd.c | 7

Track which bo is used as primary surface. With that in place we don't need the primary_created flag any more, we can just check the primary bo pointer instead. Also verify we don't already have a primary surface in qxl_io_create_primary(). Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/qxl/qxl_drv.h | 2 +- drivers/gpu/drm/qxl/qxl_cmd.c | 7

On Thu, Dec 06, 2018 at 07:53:10AM -0500, Frediano Ziglio wrote: > > > > On Thu, Dec 06, 2018 at 05:59:25AM -0500, Frediano Ziglio wrote: > > > > > > > > Just use qxl_num_crtc directly everywhere instead of using > > > > qdev->monitors_config->max_allowed. Drops pointless indirection > > > > and also is less confusing. > >

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