From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-174.mta1.migadu.com (out-174.mta1.migadu.com [95.215.58.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 76D5A56446 for ; Sat, 2 Nov 2024 15:31:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.174 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730561498; cv=none; b=EcCCl+R4SS2/I8esuF4t2aysRl5GoBZ/hHqgtfqNpGZ+Jq8Y2jPMzg9nEblGw4c8DAGVBPXRWadFdOokCeizvzQW/U68RG2CJ0Bggiy5E6kup5288Lf+95ywcrqcaTHK7om28/aG0ZCt7z69vOT8Kn136I8wGSeGhTVL+8NE9vc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730561498; c=relaxed/simple; bh=B3BW8rfL9QJ8zunAdaD3K/b5ZFleIFGLjYQu34s4OWE=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=fOsutwYwbg+ki5GZdUKII9Es3kmBf0gI6lgnLekc7WNZhYdms1fy6e73iRip+LiVzPbfko4q3/xaAkV6E3t4ddSpaUphxFkluU6X2S/0k1scuOtbOBTjRMAVVZrIQOYzBY6sB4/HhfFD6WmSlyigAHCne1uKyzhZkMQyd3FFfi4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=i2WJHXyK; arc=none smtp.client-ip=95.215.58.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="i2WJHXyK" Message-ID: <91d75479-8569-40e1-914a-27268d66b5c0@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1730561492; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8FBHpeeHnkGexRTn09DvpW55xu1PDbDpzSjMrgnDg+k=; b=i2WJHXyKclJ+QbNVPY6OUqU+Xyi2WAkPbzDKHgEKxgtOrGX51uDUW5zVOXGjo2BU9v8mZ6 oXM6jiL8shlzW8hLytFQAO1qHAJYhlJPlqedUk798h/cgsxKB6DZA957NVNLiA+90SnM6/ C01ukS5dz935D8wFxpQxm/9Im4cOKtE= Date: Sat, 2 Nov 2024 23:31:24 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH v3] drm/etnaviv: Request pages from DMA32 zone on addressing_limited To: Lucas Stach , Xiaolei Wang , linux+etnaviv@armlinux.org.uk, christian.gmeiner@gmail.com, airlied@gmail.com, daniel@ffwll.ch Cc: etnaviv@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20240903020857.3250038-1-xiaolei.wang@windriver.com> <7a6ffbb773784dee0ea3ee87e563ac4e4f7c9c90.camel@pengutronix.de> Content-Language: en-US X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Sui Jingfeng In-Reply-To: <7a6ffbb773784dee0ea3ee87e563ac4e4f7c9c90.camel@pengutronix.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT Hi, On 2024/10/1 20:17, Lucas Stach wrote: > Hi Xiaolei, > > Am Dienstag, dem 03.09.2024 um 10:08 +0800 schrieb Xiaolei Wang: >> Remove __GFP_HIGHMEM when requesting a page from DMA32 zone, >> and since all vivante GPUs in the system will share the same >> DMA constraints, move the check of whether to get a page from >> DMA32 to etnaviv_bind(). >> >> Fixes: b72af445cd38 ("drm/etnaviv: request pages from DMA32 zone when needed") >> Suggested-by: Sui Jingfeng >> Signed-off-by: Xiaolei Wang >> --- >> >> change log >> >> v1: >> https://patchwork.kernel.org/project/dri-devel/patch/20240806104733.2018783-1-xiaolei.wang@windriver.com/ >> >> v2: >> Modify the issue of not retaining GFP_USER in v1 and update the commit log. >> >> v3: >> Use "priv->shm_gfp_mask = GFP_USER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;" >> instead of >> "priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;" > I don't understand this part of the changes in the new version. Why > should we drop the HIGHMEM bit always and not only in the case where > dma addressing is limited? This seems overly restrictive. While reading the implementation of the dma_alloc_attrs() function, except allocate memory from device coherent pool, the rest implementation just mask out __GFP_DMA, __GFP_DMA32 and __GFP_HIGHMEM anyway. So ? ``` void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle, gfp_t flag, unsigned long attrs) { const struct dma_map_ops *ops = get_dma_ops(dev); void *cpu_addr; WARN_ON_ONCE(!dev->coherent_dma_mask); /* * DMA allocations can never be turned back into a page pointer, so * requesting compound pages doesn't make sense (and can't even be * supported at all by various backends). */ if (WARN_ON_ONCE(flag & __GFP_COMP)) return NULL; if (dma_alloc_from_dev_coherent(dev, size, dma_handle, &cpu_addr)) return cpu_addr; /* let the implementation decide on the zone to allocate from: */ flag &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM); if (dma_alloc_direct(dev, ops)) cpu_addr = dma_direct_alloc(dev, size, dma_handle, flag, attrs); else if (use_dma_iommu(dev)) cpu_addr = iommu_dma_alloc(dev, size, dma_handle, flag, attrs); else if (ops->alloc) cpu_addr = ops->alloc(dev, size, dma_handle, flag, attrs); else return NULL; return cpu_addr; } ``` > Regards, > Lucas > >> and move the check of whether to get a page from DMA32 to etnaviv_bind(). >> >> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 10 +++++++++- >> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 8 -------- >> 2 files changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c >> index 6500f3999c5f..8cb2c3ec8e5d 100644 >> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c >> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c >> @@ -536,7 +536,15 @@ static int etnaviv_bind(struct device *dev) >> mutex_init(&priv->gem_lock); >> INIT_LIST_HEAD(&priv->gem_list); >> priv->num_gpus = 0; >> - priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN; >> + priv->shm_gfp_mask = GFP_USER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN; >> + >> + /* >> + * If the GPU is part of a system with DMA addressing limitations, >> + * request pages for our SHM backend buffers from the DMA32 zone to >> + * hopefully avoid performance killing SWIOTLB bounce buffering. >> + */ >> + if (dma_addressing_limited(dev)) >> + priv->shm_gfp_mask |= GFP_DMA32; >> >> priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(drm->dev); >> if (IS_ERR(priv->cmdbuf_suballoc)) { >> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c >> index 7c7f97793ddd..5e753dd42f72 100644 >> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c >> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c >> @@ -839,14 +839,6 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu) >> if (ret) >> goto fail; >> >> - /* >> - * If the GPU is part of a system with DMA addressing limitations, >> - * request pages for our SHM backend buffers from the DMA32 zone to >> - * hopefully avoid performance killing SWIOTLB bounce buffering. >> - */ >> - if (dma_addressing_limited(gpu->dev)) >> - priv->shm_gfp_mask |= GFP_DMA32; >> - >> /* Create buffer: */ >> ret = etnaviv_cmdbuf_init(priv->cmdbuf_suballoc, &gpu->buffer, >> PAGE_SIZE); -- Best regards, Sui