From patchwork Mon Oct 22 12:27:18 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rodrigo Siqueira X-Patchwork-Id: 10651969 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6DE9190 for ; Mon, 22 Oct 2018 12:27:33 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5251228A48 for ; Mon, 22 Oct 2018 12:27:33 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5067328A1B; Mon, 22 Oct 2018 12:27:33 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 77CF428A5A for ; Mon, 22 Oct 2018 12:27:32 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A449D89DDD; Mon, 22 Oct 2018 12:27:24 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-qt1-x843.google.com (mail-qt1-x843.google.com [IPv6:2607:f8b0:4864:20::843]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1893889E5B for ; Mon, 22 Oct 2018 12:27:23 +0000 (UTC) Received: by mail-qt1-x843.google.com with SMTP id l9-v6so990257qtj.12 for ; Mon, 22 Oct 2018 05:27:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition:content-transfer-encoding:user-agent; bh=8QmNg4ZTKo8AjpGqR16e5VGbS3uRzgCCBGrl2wrFGuc=; b=IKXLFad3+YYtTskxU76s44XEkx6KhKCHlsVPxLlj8qG8m/u0X/hR5ln/cXWjGNsvda r8uXeda0YE8sFsNck2EroQAekE0RUUzFftbkGOT3Vb1tWCKBImYqgDI6pU5QeoECylQN FCDMSJA/5ehFqzwaRcCeQYln0bClMvZ519aI8Ld8AwSfDo8oK4b5Jr9RRg72qlDHSObV YmpYTDtcJNtS66IVV+M0Im9IsX00lBGyvqzWnr9TDcVLzea2inxgjui4o0fu780tF7Gp hlYIyXaDkjy0AVgiLEC7zUE0+mNwUTYt7+TJBLjG8Q6Qdpe4f3gFajDFI90gg8L2F7CI KGLA== X-Gm-Message-State: ABuFfoi2jQOkrIGJJljOMsSLEHgCOcPXMkFAj4hrq/LUzOsV+Q6Z6SRI zIIj+l7pDSYHxswFQVTqmxlAjAKe X-Google-Smtp-Source: ACcGV620w9cWLbpIWFDO964fezUORttJto/e3xInTsmUM++RftA2WgK2Qe90lZao9JHg0mXKR43gSg== X-Received: by 2002:ac8:1096:: with SMTP id a22-v6mr43215033qtj.359.1540211241853; Mon, 22 Oct 2018 05:27:21 -0700 (PDT) Received: from smtp.gmail.com ([143.107.45.1]) by smtp.gmail.com with ESMTPSA id r54-v6sm10865254qtk.27.2018.10.22.05.27.19 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 22 Oct 2018 05:27:21 -0700 (PDT) Date: Mon, 22 Oct 2018 09:27:18 -0300 From: Rodrigo Siqueira To: dri-devel@lists.freedesktop.org Subject: [RFC] drm/vkms: Add writeback support Message-ID: <20181022122718.girdp4ybkw3wrsmn@smtp.gmail.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: NeoMutt/20180716 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Simon Ser Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 --- 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 #include #include +#include #include #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 #include +#include + +#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);