From patchwork Thu Jun 6 22:27:50 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10980533 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 676AB924 for ; Thu, 6 Jun 2019 22:28:24 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 56ACB1FFE4 for ; Thu, 6 Jun 2019 22:28:24 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4B0CC27EE2; Thu, 6 Jun 2019 22:28:24 +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,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 AD60A1FFE4 for ; Thu, 6 Jun 2019 22:28:23 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E190389A44; Thu, 6 Jun 2019 22:28:15 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-ed1-x543.google.com (mail-ed1-x543.google.com [IPv6:2a00:1450:4864:20::543]) by gabe.freedesktop.org (Postfix) with ESMTPS id 888C589A34 for ; Thu, 6 Jun 2019 22:28:13 +0000 (UTC) Received: by mail-ed1-x543.google.com with SMTP id k8so3461037edr.11 for ; Thu, 06 Jun 2019 15:28:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=SAiiI6VzkaAeDYl1tw4fgVAOH4SmvYxgGwYm8RdcpiY=; b=cCgM+WyVZgU0vGLXY2CwfiSGwVt3jocf0A3VhyX6kyfUtbn6gQvlZj7uf8c8+WYOQd evI7EyWjHZv0A4xbb484M0ppNNB2KNJZcyqL5ErEyoKHvZ6B+jLRXNRUbUNrLtGfIEIr rKTJ5wrt0LWyy3rZXepsq0kCBRngi5NaHW3ZvY1pirKZI2iIQ3rWBulKu95uZdmiFPkY c47mmpZkkN5kWZt02Xfn86+tATw0YwbUSv+W77YI4zoT7BpNDt9E5Cybg/5ueVKvUyD5 +dAWK32m8cTGqV2XkrroglXz9+9HdQcYEieNZ4rCN/PDk0KQqpKcBXvfkcm2vktzGYs3 4Dqw== X-Gm-Message-State: APjAAAV8pubeoSuql8tpz0dHLBjEpQ68aRGuoT2ADHwTA1vgddZ+QY/4 QlWVx8kJhkAeKPt+avz3t1H8ZulmZIE= X-Google-Smtp-Source: APXvYqxHNk2JTdfioKKSUuyPiNtWE/jv43ZBBcB3JNfIREvMhKkso8QEPbTm3pOv/fso8Qx5yb0pTg== X-Received: by 2002:a17:906:1f55:: with SMTP id d21mr42757738ejk.87.1559860091740; Thu, 06 Jun 2019 15:28:11 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id z10sm54228edl.35.2019.06.06.15.28.10 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Thu, 06 Jun 2019 15:28:10 -0700 (PDT) From: Daniel Vetter To: DRI Development Subject: [PATCH 09/10] drm/vkms: totally reworked crc data tracking Date: Fri, 7 Jun 2019 00:27:50 +0200 Message-Id: <20190606222751.32567-10-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190606222751.32567-1-daniel.vetter@ffwll.ch> References: <20190606222751.32567-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=SAiiI6VzkaAeDYl1tw4fgVAOH4SmvYxgGwYm8RdcpiY=; b=NngFIKOe+Kxi3Xi4NvzfKDuu36KAUR132mgkidS/r2/VFZND4ilEaTcoCF0ddpB4yD wnfIw6qyF13p3d0gJcDzUkLsLruhUobwhrOlntd3Q4vG8s0HBJtAV+rDehh+NTn9v7D3 mf1B1h27VTRq5myU+5k1ldCyVeyC8oaHuTZhk= 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: Daniel Vetter , Haneen Mohammed , Rodrigo Siqueira , Daniel Vetter Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP The crc computation worker needs to be able to get at some data structures and framebuffer mappings, while potentially more atomic updates are going on. The solution thus far is to copy relevant bits around, but that's very tedious. Here's a new approach, which tries to be more clever, but relies on a few not-so-obvious things: - crtc_state is always updated when a plane_state changes. Therefore we can just stuff plane_state pointers into a crtc_state. That solves the problem of easily getting at the needed plane_states. - with the flushing changes from previous patches the above also holds without races due to the next atomic update being a bit eager with cleaning up pending work - we always wait for all crc work items to complete before unmapping framebuffers. - we also need to make sure that the hrtimer fires off the right worker. Keep a new distinct crc_state pointer, under the vkms_output->lock protection for this. Note that crtc->state is updated very early in the atomic commit, way before we arm the vblank event - the vblank event should always match the buffers we use to compute the crc. This also solves an issue in the hrtimer, where we've accessed drm_crtc->state without holding the right locks (we held none - oops). - in the worker itself we can then just access the plane states we need, again solving a bunch of ordering and locking issues. Accessing plane->state requires locks, accessing the private vkms_crtc_state->active_planes pointer only requires that the memory doesn't get freed too early. The idea behind vkms_crtc_state->active_planes is that this would contain all visible planes, in z-order, as a first step towards a more generic blending implementation. Note that this patch also fixes races between prepare_fb/cleanup_fb and the crc worker accessing ->vaddr. Signed-off-by: Daniel Vetter Cc: Rodrigo Siqueira Cc: Haneen Mohammed Cc: Daniel Vetter --- drivers/gpu/drm/vkms/vkms_crc.c | 21 +++-------- drivers/gpu/drm/vkms/vkms_crtc.c | 60 +++++++++++++++++++++++++++++--- drivers/gpu/drm/vkms/vkms_drv.h | 9 ++++- 3 files changed, 67 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c index 9d15e5e85830..0d31cfc32042 100644 --- a/drivers/gpu/drm/vkms/vkms_crc.c +++ b/drivers/gpu/drm/vkms/vkms_crc.c @@ -159,11 +159,8 @@ void vkms_crc_work_handle(struct work_struct *work) crc_work); struct drm_crtc *crtc = crtc_state->base.crtc; struct vkms_output *out = drm_crtc_to_vkms_output(crtc); - struct vkms_device *vdev = container_of(out, struct vkms_device, - output); struct vkms_crc_data *primary_crc = NULL; struct vkms_crc_data *cursor_crc = NULL; - struct drm_plane *plane; u32 crc32 = 0; u64 frame_start, frame_end; bool crc_pending; @@ -184,21 +181,11 @@ void vkms_crc_work_handle(struct work_struct *work) if (!crc_pending) return; - drm_for_each_plane(plane, &vdev->drm) { - struct vkms_plane_state *vplane_state; - struct vkms_crc_data *crc_data; + if (crtc_state->num_active_planes >= 1) + primary_crc = crtc_state->active_planes[0]->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_crc = crc_data; - else - cursor_crc = crc_data; - } + if (crtc_state->num_active_planes == 2) + cursor_crc = crtc_state->active_planes[1]->crc_data; if (primary_crc) crc32 = _vkms_get_crc(primary_crc, cursor_crc); diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 48a793ba4030..14156ed70415 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0+ #include "vkms_drv.h" +#include #include #include @@ -9,7 +10,7 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) struct vkms_output *output = container_of(timer, struct vkms_output, vblank_hrtimer); struct drm_crtc *crtc = &output->crtc; - struct vkms_crtc_state *state = to_vkms_crtc_state(crtc->state); + struct vkms_crtc_state *state; u64 ret_overrun; bool ret; @@ -23,6 +24,7 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) if (!ret) DRM_ERROR("vkms failure on handling vblank"); + state = output->crc_state; if (state && output->crc_enabled) { u64 frame = drm_crtc_accurate_vblank_count(crtc); @@ -124,10 +126,9 @@ static void vkms_atomic_crtc_destroy_state(struct drm_crtc *crtc, __drm_atomic_helper_crtc_destroy_state(state); - if (vkms_state) { - WARN_ON(work_pending(&vkms_state->crc_work)); - kfree(vkms_state); - } + WARN_ON(work_pending(&vkms_state->crc_work)); + kfree(vkms_state->active_planes); + kfree(vkms_state); } static void vkms_atomic_crtc_reset(struct drm_crtc *crtc) @@ -156,6 +157,52 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = { .verify_crc_source = vkms_verify_crc_source, }; +static int vkms_crtc_atomic_check(struct drm_crtc *crtc, + struct drm_crtc_state *state) +{ + struct vkms_crtc_state *vkms_state = to_vkms_crtc_state(state); + struct drm_plane *plane; + struct drm_plane_state *plane_state; + int i = 0, ret; + + if (vkms_state->active_planes) + return 0; + + ret = drm_atomic_add_affected_planes(state->state, crtc); + if (ret < 0) + return ret; + + drm_for_each_plane_mask(plane, crtc->dev, state->plane_mask) { + plane_state = drm_atomic_get_existing_plane_state(state->state, + plane); + WARN_ON(!plane_state); + + if (!plane_state->visible) + continue; + + i++; + } + + vkms_state->active_planes = kcalloc(i, sizeof(plane), GFP_KERNEL); + if (!vkms_state->active_planes) + return -ENOMEM; + vkms_state->num_active_planes = i; + + i = 0; + drm_for_each_plane_mask(plane, crtc->dev, state->plane_mask) { + plane_state = drm_atomic_get_existing_plane_state(state->state, + plane); + + if (!plane_state->visible) + continue; + + vkms_state->active_planes[i++] = + to_vkms_plane_state(plane_state); + } + + return 0; +} + static void vkms_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { @@ -197,10 +244,13 @@ static void vkms_crtc_atomic_flush(struct drm_crtc *crtc, crtc->state->event = NULL; } + vkms_output->crc_state = to_vkms_crtc_state(crtc->state); + spin_unlock_irq(&vkms_output->lock); } static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = { + .atomic_check = vkms_crtc_atomic_check, .atomic_begin = vkms_crtc_atomic_begin, .atomic_flush = vkms_crtc_atomic_flush, .atomic_enable = vkms_crtc_atomic_enable, diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 2a35299bfb89..4e7450111d45 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -49,6 +49,10 @@ struct vkms_crtc_state { struct drm_crtc_state base; struct work_struct crc_work; + int num_active_planes; + /* stack of active planes for crc computation, should be in z order */ + struct vkms_plane_state **active_planes; + /* below three are protected by vkms_output.crc_lock */ bool crc_pending; u64 frame_start; @@ -62,12 +66,15 @@ struct vkms_output { struct hrtimer vblank_hrtimer; ktime_t period_ns; struct drm_pending_vblank_event *event; - bool crc_enabled; /* ordered wq for crc_work */ struct workqueue_struct *crc_workq; /* protects concurrent access to crc_data */ spinlock_t lock; + /* protected by @lock */ + bool crc_enabled; + struct vkms_crtc_state *crc_state; + spinlock_t crc_lock; };