From patchwork Wed Sep 4 20:18:06 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: wxiaowen@codeaurora.org X-Patchwork-Id: 11131397 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 7178B14E5 for ; Wed, 4 Sep 2019 20:18:10 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 597E02087E for ; Wed, 4 Sep 2019 20:18:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 597E02087E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B3E4F89C2C; Wed, 4 Sep 2019 20:18:07 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from smtp.codeaurora.org (smtp.codeaurora.org [198.145.29.96]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1D49089C2C; Wed, 4 Sep 2019 20:18:07 +0000 (UTC) Received: by smtp.codeaurora.org (Postfix, from userid 1000) id E0FAB60328; Wed, 4 Sep 2019 20:18:06 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on pdx-caf-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.7 required=2.0 tests=ALL_TRUSTED,BAYES_00, DKIM_INVALID,DKIM_SIGNED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.codeaurora.org (Postfix) with ESMTP id 6B18F60328; Wed, 4 Sep 2019 20:18:06 +0000 (UTC) MIME-Version: 1.0 Date: Wed, 04 Sep 2019 16:18:06 -0400 From: wxiaowen@codeaurora.org To: dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org Subject: drm-lease: sharing planes between DRM lessees Message-ID: X-Sender: wxiaowen@codeaurora.org User-Agent: Roundcube Webmail/1.2.5 X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1567628286; bh=miGD45TQwCn6giNXnD5NYhMWxrY752FGT3E5+jz5y3I=; h=Date:From:To:Subject:From; b=QlXEtFBxXREA5JPPwuPpEwFXnOsabaCjZbno6fYvgwGAQ0cgoWcj8ObQMe7VlC3qR s/0idERtPC1gMaJoGrO0/3DNkvVFKeAfkSEiiqd+IpeMiTXpJJO7A4VTMqH4O6ZC7E t+qhWn3F/Pdg5ZdnFKzTfsyLbqYhdB9NxulcBk/A= X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1567628286; bh=miGD45TQwCn6giNXnD5NYhMWxrY752FGT3E5+jz5y3I=; h=Date:From:To:Subject:From; b=QlXEtFBxXREA5JPPwuPpEwFXnOsabaCjZbno6fYvgwGAQ0cgoWcj8ObQMe7VlC3qR s/0idERtPC1gMaJoGrO0/3DNkvVFKeAfkSEiiqd+IpeMiTXpJJO7A4VTMqH4O6ZC7E t+qhWn3F/Pdg5ZdnFKzTfsyLbqYhdB9NxulcBk/A= 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: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" There is a use case to share h/w pipe resources between lessee masters. Two planes are created and allocated to two masters, but internally they share the same h/w pipe: masterA masterB planeA planeB | | \--- same H/W ---/ A seamless handoff between the shared plane is also required, that in above diagram if planeB is staged and committed, planeA should be unstaged at the same time to handoff h/w pipe to planeB. The idea is to maximize the use of h/w pipe, so that if plane is not used in one master, it can be reused in another master. This idea is similar to universal plane used between crtcs, but this time we need to move it to crtc in another master. ### Option 1 ###: Based on the requirement, we planned to add a shared plane check in drm_atomic_helper_check_modeset(), to add affected shared planes from other lessees and unstage them in the same commit: + /** * drm_atomic_helper_check_modeset - validate state object for modeset changes * @dev: DRM device @@ -570,6 +604,10 @@ static enum drm_mode_status mode_valid_path(struct drm_connector *connector, int i, ret; unsigned connectors_mask = 0; + ret = update_shared_planes(dev, state) + if (ret) + return ret; + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { bool has_connectors = !!new_crtc_state->connector_mask; We have a question for above update_shared_planes function, that can kernel modify affected plane/crtc states implicitly for shared planes from another lessee? This created an inter-access-lease-master behavior and we're not sure if it's allowed. Please note that if this method of accessing is acceptable, we can maintain the change at the vendor level subdriver and above drm_atomic_helper_check_modeset changes can be avoided. ### Option2 ###: Another approach is to hide hardware ownership inside vendor's subdriver. In this approach, there are no DRM state changes during handoff, only the validation in vendor subdriver will fail, if two commits from different masters tried to stage the planes with the same h/w pipe. This approach doesn't use DRM framework to update the plane states. Detaching old plane and attaching new plane will be in two different atomic commits from two masters so no inter-access-lease-master problem here, but this also resulting DRM state not reflecting the actual h/w pipe status during handoff commit, and vendor subdriver needs to check/update more internal states than the atomic state in the commit. Thanks, Xiaowen Wu diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index f7eccab..d0eb578 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -516,6 +516,40 @@ static enum drm_mode_status mode_valid_path(struct drm_connector *connector, return 0; } +static int +update_shared_planes(struct drm_device *dev, struct drm_atomic_state *state) +{ + struct drm_plane *plane; + struct drm_plane_state *plane_state; + uint32_t plane_mask = 0; + int i, ret; + + for_each_plane_in_state(state, plane, plane_state, i) { + if (plane->type == DRM_PLANE_TYPE_SHARED && plane_state->crtc) + plane_mask |= (1 << drm_plane_index(plane)); + } + + drm_for_each_sibling_plane_mask(plane, dev, plane_mask) { + /* + * sibling planes should not appear in plane_mask, otherwise + * we have multiple sibling planes staged in one commit + */ + if (plane_mask & (1 << drm_plane_index(plane))) + return -EINVAL; + + plane_state = drm_atomic_get_plane_state(state, plane); + if (IS_ERR(plane_state)) + return PTR_ERR(plane_state); + + drm_atomic_set_fb_for_plane(plane_state, NULL); + ret = drm_atomic_set_crtc_for_plane(plane_state, NULL); + if (ret) + return ret; + } + + return 0; +}