From patchwork Sun Mar 10 21:20:34 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Rodrigo Siqueira X-Patchwork-Id: 10846469 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 C43816C2 for ; Sun, 10 Mar 2019 21:20:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 99E7F28CF9 for ; Sun, 10 Mar 2019 21:20:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8DF3B28D3A; Sun, 10 Mar 2019 21:20:44 +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 9886F28CF9 for ; Sun, 10 Mar 2019 21:20:43 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 48AA089ACD; Sun, 10 Mar 2019 21:20:41 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-qk1-x742.google.com (mail-qk1-x742.google.com [IPv6:2607:f8b0:4864:20::742]) by gabe.freedesktop.org (Postfix) with ESMTPS id 45D6D89ACD for ; Sun, 10 Mar 2019 21:20:39 +0000 (UTC) Received: by mail-qk1-x742.google.com with SMTP id b74so1549535qkg.9 for ; Sun, 10 Mar 2019 14:20:39 -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=zf6gusKMnbZyNhid9hJ5/JVnXpNQ2OF5QMOMuPpghv0=; b=EWYZqJkG1PqZctHVd2SqLd7FuHHEG65TsDl0g14iO6RTDzSj6Wzc82lKP1fEFvjUmz FnsprVB26YOTfc6xlg4DhadKP46xMcbgQ5h14latY4nAiw2PhEjuP5rYG5U0G9tTWAEt knSHNbIjD+FNstYxaUqlYdLhEF/SM5hSThvQYC4jn0uSgQk2YVjn9kJ/k6wzcWoG5ZxN cNNvtXCNd1vH38Tl0A2PY4TCautHD6HN0tS1rClnd48URwGvlYzmrhdU02dOPw1QYzbZ ltbQsbVcnU0JG/+YPLHjDtxOqX4n1u//oudNXeos6upRejpxo7XnD4XwpNSK/EV90Vxm QdPg== X-Gm-Message-State: APjAAAW0zFNwuuzzmT5ILq29gLMU9X80MmEilygIDrbmLZH4itGyGoNU 0j6J0KvqaeTuJW2qXRX+CIvrEAeIC2k= X-Google-Smtp-Source: APXvYqyLpOU4LlMiMDyaRZQitDRTBMWro/uEJldkU8cGxYd56CKaqQxwCIusS8CtWvAm3ayLb8iCVQ== X-Received: by 2002:a37:4a12:: with SMTP id x18mr22487080qka.169.1552252838067; Sun, 10 Mar 2019 14:20:38 -0700 (PDT) Received: from smtp.gmail.com ([143.107.45.1]) by smtp.gmail.com with ESMTPSA id s9sm2126251qkm.67.2019.03.10.14.20.36 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Sun, 10 Mar 2019 14:20:37 -0700 (PDT) Date: Sun, 10 Mar 2019 18:20:34 -0300 From: Rodrigo Siqueira To: Brian Starkey , Liviu Dudau , Daniel Vetter Subject: [RFC] drm/vkms: Add writeback support Message-ID: <20190310212034.mf5ymeadwledswtg@smtp.gmail.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: NeoMutt/20180716 X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:mime-version:content-disposition :content-transfer-encoding:user-agent; bh=zf6gusKMnbZyNhid9hJ5/JVnXpNQ2OF5QMOMuPpghv0=; b=HX/g+z6elRdCsUW9xEM5HUS/m6UnRQ2B1OuY2C5ReXLg11SV1vMgruLFqbKs4xy2Og Ha+bWVYs1zTSFq8Qf/tHYTIPGPyUXsPsQx74IBtOIA002Tbst6SfSvx2yTG/4SgkcD5R p6570qPlk37oQBmLyqYUKvBhBO07R8MAtI2N/ASanLnTUM8TIkWxgQl//U5JIE+W92yw Z60FsXh+zs0odZ2IdFHSl+I2f4695NEF43EoMZmATJ1wtIlNr/nVDj1Z9SwgpXk5SvED Ds/iqsRt2StjRpATwa3OfslEk0mAmYnxY94i9xbcBkzsm8TdQe0tHQwnvhea/RR/EOBY 8VDA== 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 , dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Hi, In the last few weeks, I was focused on implementing the writeback on VKMS by using the feedback that I got from Brian Starkey and Daniel Vetter in my previous attempt [1]. For testing/guiding my implementation, I’m using a patchset made by Liviu and Brian that adds writeback tests to IGT [2]. This patch, already pass the basic requirements and some of the other tests. So, in this implementation I adopted the following design: you can use writeback_connector, or you can have a virtual connector, but you cannot have both. Should I keep the virtual connector always available and make it possible to add writeback connector as the secondary connector? If so, there's any IGT test for this? Secondly, I’m facing problems related to ‘drm_writeback_signal_completion()’, for some reason that I did not know yet I continuously get the following error in a loop: [ +0.000060] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G D W 5.0.0-VKMS #102 [ +0.000004] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-20181126_142135-anatol 04/01/2014 [ +0.000033] RIP: 0010:drm_writeback_signal_completion+0x10c/0x130 [drm] [ +0.000006] Code: 01 00 00 48 89 43 e8 48 89 43 f0 48 8b 35 3c 67 b3 e4 5b 5d 41 5c 41 5d 41 5e e9 0f 7d aa e3 4c 89 ee 4c 89 e7 e8 a4 4e 18 e4 <0f> 0b 5b 5d 41 5c 41 5d 41 5e c3 e8 54 2f f8 e3 eb 9a 0f 0b e9 60 [ +0.000004] RSP: 0018:ffff98593cb83ec0 EFLAGS: 00010082 [ +0.000006] RAX: 0000000080010002 RBX: ffff98593bbb54b0 RCX: 0000000000000000 [ +0.000005] RDX: 0000000000000001 RSI: 0000000000000082 RDI: 00000000ffffffff [ +0.000004] RBP: ffff98593bbb54b0 R08: 0000000000000000 R09: ffff98593cb83e20 [ +0.000004] R10: ffff98593a07d600 R11: 0000000000000000 R12: ffff98593bbb54a8 [ +0.000004] R13: 0000000000000082 R14: 0000000000000000 R15: ffff98593cb9cd38 [ +0.000006] FS: 0000000000000000(0000) GS:ffff98593cb80000(0000) knlGS:0000000000000000 [ +0.000004] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ +0.000005] CR2: 00007f3cea2e8000 CR3: 00000000ba6ec000 CR4: 00000000000006e0 [ +0.000010] Call Trace: [ +0.000007] [ +0.000013] ? vkms_disable_vblank+0x20/0x20 [vkms] [ +0.000008] vkms_vblank_simulate+0xef/0x110 [vkms] [ +0.000008] ? vkms_disable_vblank+0x20/0x20 [vkms] [ +0.000007] __hrtimer_run_queues+0x100/0x2d0 [ +0.000008] hrtimer_interrupt+0x10a/0x220 [ +0.000008] ? kvm_sched_clock_read+0x5/0x10 [ +0.000010] smp_apic_timer_interrupt+0x69/0x160 [ +0.000007] apic_timer_interrupt+0xf/0x20 [ +0.000004] [ +0.000008] RIP: 0010:native_safe_halt+0x2/0x10 [ +0.000005] Code: 5b ff ff ff 7f 5b c3 65 48 8b 04 25 00 5c 01 00 f0 80 48 02 20 48 8b 00 a8 08 75 c3 eb 8b 90 90 90 90 90 90 90 90 90 90 fb f4 66 66 2e 0f 1f 84 00 00 00 00 00 66 90 f4 c3 90 90 90 90 90 90 [ +0.000005] RSP: 0018:ffffb937c06b3eb8 EFLAGS: 00000202 ORIG_RAX: ffffffffffffff13 [ +0.000005] RAX: 0000000000000003 RBX: 0000000000000003 RCX: 0000000000000001 [ +0.000004] RDX: 0000000000000001 RSI: ffffffffa4e6da3e RDI: ffffffffa4e6dd30 [ +0.000005] RBP: ffffffffa51252e0 R08: ffffffffa55e7438 R09: 0000000000000000 [ +0.000003] R10: 0000000000000000 R11: ffffffffa5049fb8 R12: 0000000000000000 [ +0.000004] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 [ +0.000012] default_idle+0x1a/0x170 [ +0.000010] do_idle+0x1d5/0x250 [ +0.000007] cpu_startup_entry+0x19/0x20 [ +0.000007] start_secondary+0x1aa/0x200 [ +0.000008] secondary_startup_64+0xa4/0xb0 [ +0.000008] ---[ end trace ec12666e4ecb96f7 ]--- Any ideas? Or any comments? Thanks Best Regards 1. https://patchwork.freedesktop.org/patch/257771/?series=51325&rev=1 2. https://patchwork.freedesktop.org/series/39229/ --- drivers/gpu/drm/vkms/vkms_crtc.c | 3 + drivers/gpu/drm/vkms/vkms_drv.c | 8 ++ drivers/gpu/drm/vkms/vkms_drv.h | 21 ++++ drivers/gpu/drm/vkms/vkms_output.c | 176 ++++++++++++++++++++++++++--- drivers/gpu/drm/vkms/vkms_plane.c | 2 +- 5 files changed, 195 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 8a9aeb0a9ea8..d9948208e8eb 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -19,6 +19,9 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) if (!ret) DRM_ERROR("vkms failure on handling vblank"); + if (output->writeback_enabled) + drm_writeback_signal_completion(&output->mw_connector, 0); + if (state && output->crc_enabled) { u64 frame = drm_crtc_accurate_vblank_count(crtc); diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c index 738dd6206d85..e0d7f94e3972 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -29,6 +29,10 @@ bool enable_cursor; module_param_named(enable_cursor, enable_cursor, bool, 0444); MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support"); +int enable_writeback; +module_param_named(enable_writeback, enable_writeback, int, 0444); +MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector"); + static const struct file_operations vkms_driver_fops = { .owner = THIS_MODULE, .open = drm_open, @@ -123,6 +127,10 @@ static int __init vkms_init(void) goto out_fini; } + vkms_device->output.writeback_enabled = enable_writeback; + if (enable_writeback) + DRM_INFO("Writeback connector enabled"); + ret = vkms_modeset_init(vkms_device); if (ret) goto out_fini; diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 81f1cfbeb936..2c26fe686b93 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -8,6 +8,7 @@ #include #include #include +#include #define XRES_MIN 20 #define YRES_MIN 20 @@ -46,6 +47,16 @@ struct vkms_plane_state { struct vkms_crc_data *crc_data; }; +/** + * vkms_connector_state - Driver connector specific state + * @base: base connector state + * @writeback_vaddr: keep track for writeback framebuffer destination + */ +struct vkms_connector_state { + struct drm_connector_state base; + void *writeback_vaddr; +}; + /** * vkms_crtc_state - Driver specific CRTC state * @base: base CRTC state @@ -64,10 +75,12 @@ struct vkms_output { struct drm_crtc crtc; struct drm_encoder encoder; struct drm_connector connector; + struct drm_writeback_connector mw_connector; struct hrtimer vblank_hrtimer; ktime_t period_ns; struct drm_pending_vblank_event *event; bool crc_enabled; + int writeback_enabled; /* ordered wq for crc_work */ struct workqueue_struct *crc_workq; /* protects concurrent access to crc_data */ @@ -93,6 +106,12 @@ struct vkms_gem_object { #define drm_crtc_to_vkms_output(target) \ container_of(target, struct vkms_output, crtc) +#define drm_connector_to_wb_connector(target) \ + container_of(target, struct drm_writeback_connector, base) + +#define drm_wb_to_vkms_output(target) \ + container_of(target, struct vkms_output, mw_connector) + #define drm_device_to_vkms_device(target) \ container_of(target, struct vkms_device, drm) @@ -105,6 +124,8 @@ struct vkms_gem_object { #define to_vkms_plane_state(target)\ container_of(target, struct vkms_plane_state, base) +#define to_wb_state(_state) (struct vkms_connector_state *)(_state) + /* CRTC */ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, struct drm_plane *primary, struct drm_plane *cursor); diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index 3b162b25312e..0e55e7299d2e 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -3,6 +3,8 @@ #include "vkms_drv.h" #include #include +#include +#include static void vkms_connector_destroy(struct drm_connector *connector) { @@ -10,14 +12,92 @@ static void vkms_connector_destroy(struct drm_connector *connector) drm_connector_cleanup(connector); } +static void vkms_connector_reset(struct drm_connector *connector) +{ + struct vkms_connector_state *mw_state; + struct drm_writeback_connector *wb = drm_connector_to_wb_connector(connector); + struct vkms_output *output = drm_wb_to_vkms_output(wb); + + if (!output->writeback_enabled) + return drm_atomic_helper_connector_reset(connector); + + mw_state = kzalloc(sizeof(*mw_state), GFP_KERNEL); + if (!mw_state) { + DRM_ERROR("Failed to allocate memory for connector state"); + return; + } + + if (connector->state) + __drm_atomic_helper_connector_destroy_state(connector->state); + + kfree(connector->state); + __drm_atomic_helper_connector_reset(connector, &mw_state->base); +} + +static struct drm_connector_state * +vkms_connector_duplicate_state(struct drm_connector *connector) +{ + struct vkms_connector_state *mw_state, *mw_current_state; + struct drm_writeback_connector *wb = drm_connector_to_wb_connector(connector); + struct vkms_output *output = drm_wb_to_vkms_output(wb); + + if (!output->writeback_enabled) + return drm_atomic_helper_connector_duplicate_state(connector); + + if (WARN_ON(!connector->state)) + return NULL; + + mw_state = kzalloc(sizeof(*mw_state), GFP_KERNEL); + if (!mw_state) + return NULL; + + mw_current_state = to_wb_state(connector->state); + mw_state->writeback_vaddr = mw_current_state->writeback_vaddr; + + __drm_atomic_helper_connector_duplicate_state(connector, &mw_state->base); + + return &mw_state->base; +} + static const struct drm_connector_funcs vkms_connector_funcs = { .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, + .reset = vkms_connector_reset, + .atomic_duplicate_state = vkms_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, }; +static int vkms_mw_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; + } + + if (fb->format->format != DRM_FORMAT_XRGB8888) { + struct drm_format_name_buf format_name; + + DRM_DEBUG_KMS("Invalid pixel format %s\n", + drm_get_format_name(fb->format->format, + &format_name)); + return -EINVAL; + } + + // TODO: Do we need to validate each plane? + + return 0; +} + static const struct drm_encoder_funcs vkms_encoder_funcs = { .destroy = drm_encoder_cleanup, }; @@ -32,8 +112,57 @@ static int vkms_conn_get_modes(struct drm_connector *connector) return count; } +static void vkms_mw_atomic_commit(struct drm_connector *conn, + struct drm_connector_state *state) +{ + struct vkms_device *vkmsdev = drm_device_to_vkms_device(conn->dev); + struct vkms_output *output = &vkmsdev->output; + struct drm_writeback_connector *mw_conn = &output->mw_connector; + + struct drm_connector_state *conn_state = mw_conn->base.state; + + struct vkms_connector_state *vkms_conn = to_wb_state(conn_state); + struct vkms_crc_data *primary_plane = NULL; + struct drm_plane *plane; + + if (!conn_state) + return; + + if (conn_state->writeback_job && conn_state->writeback_job->fb) { + struct drm_framebuffer *fb = conn_state->writeback_job->fb; + struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0); + struct vkms_gem_object *vkms_obj = drm_gem_to_vkms_gem(gem_obj); + + vkms_conn->writeback_vaddr = kzalloc(vkms_obj->gem.size, GFP_KERNEL); + + drm_writeback_queue_job(mw_conn, conn_state->writeback_job); + conn_state->writeback_job = NULL; + + drm_for_each_plane(plane, conn->dev) { + struct vkms_plane_state *vplane_state; + struct vkms_crc_data *crc_data; + + vplane_state = to_vkms_plane_state(plane->state); + crc_data = vplane_state->crc_data; + + if (drm_framebuffer_read_refcount(&crc_data->fb) == 0) + continue; + + if (plane->type == DRM_PLANE_TYPE_PRIMARY) + primary_plane = crc_data; + } + + memcpy(vkms_conn->writeback_vaddr, vkms_obj->vaddr, vkms_obj->gem.size); + } +} + static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = { .get_modes = vkms_conn_get_modes, + .atomic_commit = vkms_mw_atomic_commit, +}; + +static const struct drm_encoder_helper_funcs vkms_encoder_helper_funcs = { + .atomic_check = vkms_mw_encoder_atomic_check, }; int vkms_output_init(struct vkms_device *vkmsdev) @@ -62,8 +191,24 @@ 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); + if (vkmsdev->output.writeback_enabled) { + const u32 *formats = vkms_formats; + int n_formats = ARRAY_SIZE(vkms_formats); + struct drm_writeback_connector *wb_connector; + + vkmsdev->output.mw_connector.encoder.possible_crtcs = 1; + + wb_connector = &vkmsdev->output.mw_connector; + connector = &wb_connector->base; + ret = drm_writeback_connector_init(dev, wb_connector, + &vkms_connector_funcs, + &vkms_encoder_helper_funcs, + formats, n_formats); + } else { + ret = drm_connector_init(dev, connector, &vkms_connector_funcs, + DRM_MODE_CONNECTOR_VIRTUAL); + } + if (ret) { DRM_ERROR("Failed to init connector\n"); goto err_connector; @@ -77,18 +222,21 @@ int vkms_output_init(struct vkms_device *vkmsdev) 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; + if (!vkmsdev->output.writeback_enabled) { + 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; + } + + 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); diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index 0e67d2d42f0c..14259ffe47d8 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -190,7 +190,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, funcs = &vkms_primary_helper_funcs; } - ret = drm_universal_plane_init(dev, plane, 0, + ret = drm_universal_plane_init(dev, plane, 1, &vkms_plane_funcs, formats, nformats, NULL, type, NULL);