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=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 81174C433E0 for ; Thu, 21 Jan 2021 06:09:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 405D42389F for ; Thu, 21 Jan 2021 06:09:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726458AbhAUGIP (ORCPT ); Thu, 21 Jan 2021 01:08:15 -0500 Received: from m42-8.mailgun.net ([69.72.42.8]:22912 "EHLO m42-8.mailgun.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726473AbhAUGHR (ORCPT ); Thu, 21 Jan 2021 01:07:17 -0500 DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=mg.codeaurora.org; q=dns/txt; s=smtp; t=1611209215; h=Message-ID: References: In-Reply-To: Subject: Cc: To: From: Date: Content-Transfer-Encoding: Content-Type: MIME-Version: Sender; bh=EEQI0tFXhGI6CiVUJKLBMVeNZjfshm+qc4ajCowtZ14=; b=Hc2tS25vvcavX2H4omuMsJUWyu1j/FBKbEhr5qYIt+sYw97pgL1yikl/qNXS8iR66vhJilf5 LTuHbbJCvYPySytaQLO5hxDuw7IdUCOjL9tnq8KjouKvZgVg2IEkL2TSTyV11Dqn8BiMyPDI YEvEZFfj9BpuoMACIu2VqMeXmGQ= X-Mailgun-Sending-Ip: 69.72.42.8 X-Mailgun-Sid: WyI0MWYwYSIsICJsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnIiwgImJlOWU0YSJd Received: from smtp.codeaurora.org (ec2-35-166-182-171.us-west-2.compute.amazonaws.com [35.166.182.171]) by smtp-out-n03.prod.us-east-1.postgun.com with SMTP id 600919e22c36b2106d81ff8e (version=TLS1.2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256); Thu, 21 Jan 2021 06:06:26 GMT Sender: saiprakash.ranjan=codeaurora.org@mg.codeaurora.org Received: by smtp.codeaurora.org (Postfix, from userid 1001) id D4A80C43466; Thu, 21 Jan 2021 06:06:25 +0000 (UTC) Received: from mail.codeaurora.org (localhost.localdomain [127.0.0.1]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: saiprakash.ranjan) by smtp.codeaurora.org (Postfix) with ESMTPSA id 9D90AC433ED; Thu, 21 Jan 2021 06:06:24 +0000 (UTC) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Thu, 21 Jan 2021 11:36:24 +0530 From: Sai Prakash Ranjan To: Rob Clark Cc: Jordan Crouse , Akhil P Oommen , Konrad Dybcio , angelogioacchino.delregno@somainline.org, freedreno , David Airlie , linux-arm-msm , Linux Kernel Mailing List , Kristian H Kristensen , Daniel Vetter , Sean Paul Subject: Re: [Freedreno] [PATCH 2/2] drm/msm/a6xx: Create an A6XX GPU specific address space In-Reply-To: References: Message-ID: X-Sender: saiprakash.ranjan@codeaurora.org User-Agent: Roundcube Webmail/1.3.9 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2021-01-20 21:48, Rob Clark wrote: > On Mon, Jan 11, 2021 at 4:04 AM Sai Prakash Ranjan > wrote: >> >> A6XX GPUs have support for last level cache(LLC) also known >> as system cache and need to set the bus attributes to >> use it. Currently we use a generic adreno iommu address space >> implementation which are also used by older GPU generations >> which do not have LLC and might introduce issues accidentally >> and is not clean in a way that anymore additions of GPUs >> supporting LLC would have to be guarded under ifdefs. So keep >> the generic code separate and make the address space creation >> A6XX specific. We also have a helper to set the llc attributes >> so that if the newer GPU generations do support them, we can >> use it instead of open coding domain attribute setting for each >> GPU. >> >> Signed-off-by: Sai Prakash Ranjan >> --- >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 46 >> ++++++++++++++++++++++++- >> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 23 +++++-------- >> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 7 ++-- >> 3 files changed, 55 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> index 3b798e883f82..3c7ad51732bb 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> @@ -1239,6 +1239,50 @@ static unsigned long a6xx_gpu_busy(struct >> msm_gpu *gpu) >> return (unsigned long)busy_time; >> } >> >> +static struct msm_gem_address_space * >> +a6xx_create_address_space(struct msm_gpu *gpu, struct platform_device >> *pdev) >> +{ >> + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); >> + struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu); >> + struct iommu_domain *iommu; >> + struct msm_mmu *mmu; >> + struct msm_gem_address_space *aspace; >> + u64 start, size; >> + >> + iommu = iommu_domain_alloc(&platform_bus_type); >> + if (!iommu) >> + return NULL; >> + >> + /* >> + * This allows GPU to set the bus attributes required to use >> system >> + * cache on behalf of the iommu page table walker. >> + */ >> + if (!IS_ERR_OR_NULL(a6xx_gpu->htw_llc_slice)) >> + adreno_set_llc_attributes(iommu); >> + >> + mmu = msm_iommu_new(&pdev->dev, iommu); >> + if (IS_ERR(mmu)) { >> + iommu_domain_free(iommu); >> + return ERR_CAST(mmu); >> + } >> + >> + /* >> + * Use the aperture start or SZ_16M, whichever is greater. >> This will >> + * ensure that we align with the allocated pagetable range >> while still >> + * allowing room in the lower 32 bits for GMEM and whatnot >> + */ >> + start = max_t(u64, SZ_16M, iommu->geometry.aperture_start); >> + size = iommu->geometry.aperture_end - start + 1; >> + >> + aspace = msm_gem_address_space_create(mmu, "gpu", >> + start & GENMASK_ULL(48, 0), size); >> + >> + if (IS_ERR(aspace) && !IS_ERR(mmu)) >> + mmu->funcs->destroy(mmu); >> + >> + return aspace; >> +} >> + >> static struct msm_gem_address_space * >> a6xx_create_private_address_space(struct msm_gpu *gpu) >> { >> @@ -1285,7 +1329,7 @@ static const struct adreno_gpu_funcs funcs = { >> .gpu_state_get = a6xx_gpu_state_get, >> .gpu_state_put = a6xx_gpu_state_put, >> #endif >> - .create_address_space = >> adreno_iommu_create_address_space, >> + .create_address_space = a6xx_create_address_space, >> .create_private_address_space = >> a6xx_create_private_address_space, >> .get_rptr = a6xx_get_rptr, >> }, >> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c >> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c >> index b35914de1b27..0f184c3dd9d9 100644 >> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c >> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c >> @@ -186,11 +186,18 @@ int adreno_zap_shader_load(struct msm_gpu *gpu, >> u32 pasid) >> return zap_shader_load_mdt(gpu, adreno_gpu->info->zapfw, >> pasid); >> } >> >> +void adreno_set_llc_attributes(struct iommu_domain *iommu) >> +{ >> + struct io_pgtable_domain_attr pgtbl_cfg; >> + >> + pgtbl_cfg.quirks = IO_PGTABLE_QUIRK_ARM_OUTER_WBWA; > > btw, since quirks is the only field in the struct currently, this is > ok. But better practice to do something like: > > struct io_pgtable_domain_attr pgtbl_cfg = { > .quirks = IO_PGTABLE_QUIRK_ARM_OUTER_WBWA, > }; > > which will zero-initialize any additional fields which might be added > later, rather than inherit random garbage from the stack. > Right, I will correct this. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation