Hi, I started to work for add the writeback feature into the VKMS, and this RFC represents my first draft. As a result, I have some questions which I list below. 1. In this patch, I replaced the virtual connector by the writeback connector. Is it a good idea? Or should I keep both connectors? If I keep both connectors, should I add a module parameter to enable the writeback? 2. When I added the writeback connector (in drm_writeback_connector_init()), I noticed that I had to indicate the struct drm_encoder_helper_funcs. However, I think that VKMS does not need anything sophisticated related to encoders at the writeback perspective. Am I right? Or did I missed something? 3. I just discovered that the writeback connectors are not exposed as a connector for the userspace, as a result, IGT tests do not work and Wayland is unable to run as well. Simon Ser and Daniel Stone, explained to me that I have to update the compositor (in my case Weston) to set DRM_CLIENT_CAP_WRITEBACK_CONNECTORS via drmSetClientCap(). Since I do not have experience with userspace, anyone can point me out another aspect that I have to take a look to make the writeback work in the user space? Also, Is it possible to update the IGT test or do I need to create a new set of tests? Thanks! Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@xxxxxxxxx> --- drivers/gpu/drm/vkms/vkms_crc.c | 9 ++- drivers/gpu/drm/vkms/vkms_crtc.c | 2 + drivers/gpu/drm/vkms/vkms_drv.h | 3 +- drivers/gpu/drm/vkms/vkms_output.c | 107 +++++++++++++++++++++-------- 4 files changed, 89 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c index 9d9e8146db90..5d9ac45e00f2 100644 --- a/drivers/gpu/drm/vkms/vkms_crc.c +++ b/drivers/gpu/drm/vkms/vkms_crc.c @@ -109,7 +109,8 @@ static void compose_cursor(struct vkms_crc_data *cursor_crc, } static uint32_t _vkms_get_crc(struct vkms_crc_data *primary_crc, - struct vkms_crc_data *cursor_crc) + struct vkms_crc_data *cursor_crc, + struct vkms_output *out) { struct drm_framebuffer *fb = &primary_crc->fb; struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0); @@ -130,6 +131,10 @@ static uint32_t _vkms_get_crc(struct vkms_crc_data *primary_crc, } memcpy(vaddr_out, vkms_obj->vaddr, vkms_obj->gem.size); + // TODO: Remove this copy from crc, and make this sort of operation + // generic since we can use in different places + memcpy(out->connector.base.state->writeback_job->fb, + vkms_obj->vaddr, vkms_obj->gem.size); mutex_unlock(&vkms_obj->pages_lock); if (cursor_crc) @@ -193,7 +198,7 @@ void vkms_crc_work_handle(struct work_struct *work) } if (primary_crc) - crc32 = _vkms_get_crc(primary_crc, cursor_crc); + crc32 = _vkms_get_crc(primary_crc, cursor_crc, out); frame_end = drm_crtc_accurate_vblank_count(crtc); diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 177bbcb38306..d33ddcc40a29 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -48,6 +48,8 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) _vblank_handle(output); + drm_writeback_signal_completion(&output->connector, 0); + ret_overrun = hrtimer_forward_now(&output->vblank_hrtimer, output->period_ns); diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 1c93990693e3..1e8184ad872f 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -5,6 +5,7 @@ #include <drm/drm.h> #include <drm/drm_gem.h> #include <drm/drm_encoder.h> +#include <drm/drm_writeback.h> #include <linux/hrtimer.h> #define XRES_MIN 20 @@ -61,7 +62,7 @@ struct vkms_crtc_state { struct vkms_output { struct drm_crtc crtc; struct drm_encoder encoder; - struct drm_connector connector; + struct drm_writeback_connector connector; struct hrtimer vblank_hrtimer; ktime_t period_ns; struct drm_pending_vblank_event *event; diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index 271a0eb9042c..46622d012107 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -9,6 +9,13 @@ #include "vkms_drv.h" #include <drm/drm_crtc_helper.h> #include <drm/drm_atomic_helper.h> +#include <drm/drm_writeback.h> + +#define to_wb_state(_state) (struct vkms_writeback_connector_state *)(_state) + +struct vkms_writeback_connector_state { + struct drm_connector_state base; +}; static void vkms_connector_destroy(struct drm_connector *connector) { @@ -16,11 +23,48 @@ static void vkms_connector_destroy(struct drm_connector *connector) drm_connector_cleanup(connector); } +static void vkms_connector_reset(struct drm_connector *connector) +{ + struct vkms_writeback_connector_state *wb_state = + kzalloc(sizeof(*wb_state), GFP_KERNEL); + + if (connector->state) + __drm_atomic_helper_connector_destroy_state(connector->state); + + kfree(connector->state); + __drm_atomic_helper_connector_reset(connector, &wb_state->base); +} + +static struct drm_connector_state * +vkms_connector_duplicate_state(struct drm_connector *connector) +{ + struct vkms_writeback_connector_state *wb_state; + + if (WARN_ON(!connector->state)) + return NULL; + + wb_state = kzalloc(sizeof(*wb_state), GFP_KERNEL); + if (!wb_state) + return NULL; + + __drm_atomic_helper_connector_duplicate_state(connector, + &wb_state->base); + + return &wb_state->base; +} + +static enum drm_connector_status +vkms_connector_detect(struct drm_connector *connector, bool force) +{ + return connector_status_connected; +} + static const struct drm_connector_funcs vkms_connector_funcs = { + .reset = vkms_connector_reset, + .detect = vkms_connector_detect, .fill_modes = drm_helper_probe_single_connector_modes, .destroy = vkms_connector_destroy, - .reset = drm_atomic_helper_connector_reset, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_duplicate_state = vkms_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, }; @@ -42,12 +86,35 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = { .get_modes = vkms_conn_get_modes, }; +static int vkms_encoder_atomic_check(struct drm_encoder *encoder, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct drm_framebuffer *fb; + + if (!conn_state->writeback_job || !conn_state->writeback_job->fb) + return 0; + + fb = conn_state->writeback_job->fb; + if (fb->width != crtc_state->mode.hdisplay || + fb->height != crtc_state->mode.vdisplay) { + DRM_DEBUG_KMS("Invalid framebuffer size %ux%u\n", + fb->width, fb->height); + return -EINVAL; + } + + return 0; +} + +static const struct drm_encoder_helper_funcs vkms_encoder_helper_funcs = { + .atomic_check = vkms_encoder_atomic_check, +}; + int vkms_output_init(struct vkms_device *vkmsdev) { struct vkms_output *output = &vkmsdev->output; struct drm_device *dev = &vkmsdev->drm; - struct drm_connector *connector = &output->connector; - struct drm_encoder *encoder = &output->encoder; + struct drm_writeback_connector *connector = &output->connector; struct drm_crtc *crtc = &output->crtc; struct drm_plane *primary, *cursor = NULL; int ret; @@ -68,47 +135,29 @@ int vkms_output_init(struct vkms_device *vkmsdev) if (ret) goto err_crtc; - ret = drm_connector_init(dev, connector, &vkms_connector_funcs, - DRM_MODE_CONNECTOR_VIRTUAL); + ret = drm_writeback_connector_init(dev, connector, + &vkms_connector_funcs, + &vkms_encoder_helper_funcs, + vkms_formats, 1); if (ret) { DRM_ERROR("Failed to init connector\n"); goto err_connector; } - drm_connector_helper_add(connector, &vkms_conn_helper_funcs); + drm_connector_helper_add(&connector->base, &vkms_conn_helper_funcs); - ret = drm_connector_register(connector); + ret = drm_connector_register(&connector->base); if (ret) { DRM_ERROR("Failed to register connector\n"); goto err_connector_register; } - ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs, - DRM_MODE_ENCODER_VIRTUAL, NULL); - if (ret) { - DRM_ERROR("Failed to init encoder\n"); - goto err_encoder; - } - encoder->possible_crtcs = 1; - - ret = drm_connector_attach_encoder(connector, encoder); - if (ret) { - DRM_ERROR("Failed to attach connector to encoder\n"); - goto err_attach; - } - drm_mode_config_reset(dev); return 0; -err_attach: - drm_encoder_cleanup(encoder); - -err_encoder: - drm_connector_unregister(connector); - err_connector_register: - drm_connector_cleanup(connector); + drm_connector_cleanup(&connector->base); err_connector: drm_crtc_cleanup(crtc); -- 2.19.1 -- Rodrigo Siqueira https://siqueira.tech https://twitter.com/siqueirajordao Graduate Student Department of Computer Science University of São Paulo _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel