From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D35B3C433F5 for ; Fri, 3 Sep 2021 09:51:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B9BC960FC4 for ; Fri, 3 Sep 2021 09:51:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1348917AbhICJvU (ORCPT ); Fri, 3 Sep 2021 05:51:20 -0400 Received: from foss.arm.com ([217.140.110.172]:39970 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234932AbhICJvR (ORCPT ); Fri, 3 Sep 2021 05:51:17 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 467E41FB; Fri, 3 Sep 2021 02:50:16 -0700 (PDT) Received: from e122027.lan (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E97293F694; Fri, 3 Sep 2021 02:50:14 -0700 (PDT) From: Steven Price To: Rob Herring , Tomeu Vizoso , Alyssa Rosenzweig , Boris Brezillon Cc: Steven Price , Daniel Vetter , David Airlie , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: [PATCH v2] drm/panfrost: Calculate lock region size correctly Date: Fri, 3 Sep 2021 10:49:57 +0100 Message-Id: <20210903094957.74560-1-steven.price@arm.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org It turns out that when locking a region, the region must be a naturally aligned power of 2. The upshot of this is that if the desired region crosses a 'large boundary' the region size must be increased significantly to ensure that the locked region completely covers the desired region. Previous calculations (including in kbase for the proprietary driver) failed to take this into account. Since it's known that the lock region must be naturally aligned we can compute the required size by looking at the highest bit position which changes between the start/end of the lock region (subtracting 1 from the end because the end address is exclusive). The start address is then aligned based on the size (this is technically unnecessary as the hardware will ignore these bits, but the spec advises to do this "to avoid confusion"). Reviewed-by: Boris Brezillon Signed-off-by: Steven Price --- drivers/gpu/drm/panfrost/panfrost_mmu.c | 30 +++++++++++++++++++------ 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index dfe5f1d29763..e2629b8d6a02 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -58,17 +58,33 @@ static int write_cmd(struct panfrost_device *pfdev, u32 as_nr, u32 cmd) } static void lock_region(struct panfrost_device *pfdev, u32 as_nr, - u64 iova, u64 size) + u64 region_start, u64 size) { u8 region_width; - u64 region = iova & PAGE_MASK; + u64 region; + u64 region_end = region_start + size; - /* The size is encoded as ceil(log2) minus(1), which may be calculated - * with fls. The size must be clamped to hardware bounds. + if (!size) + return; + + /* + * The locked region is a naturally aligned power of 2 block encoded as + * log2 minus(1). + * Calculate the desired start/end and look for the highest bit which + * differs. The smallest naturally aligned block must include this bit + * change, the desired region starts with this bit (and subsequent bits) + * zeroed and ends with the bit (and subsequent bits) set to one. */ - size = max_t(u64, size, AS_LOCK_REGION_MIN_SIZE); - region_width = fls64(size - 1) - 1; - region |= region_width; + region_width = max(fls64(region_start ^ (region_end - 1)), + const_ilog2(AS_LOCK_REGION_MIN_SIZE)) - 1; + + /* + * Mask off the low bits of region_start (which would be ignored by + * the hardware anyway) + */ + region_start &= GENMASK_ULL(63, region_width); + + region = region_width | region_start; /* Lock the region that needs to be updated */ mmu_write(pfdev, AS_LOCKADDR_LO(as_nr), region & 0xFFFFFFFFUL); -- 2.30.2