From patchwork Fri Aug 2 10:10:44 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vikas C Sajjan X-Patchwork-Id: 2837811 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 1D3179F479 for ; Fri, 2 Aug 2013 10:43:18 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id F113320589 for ; Fri, 2 Aug 2013 10:43:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1994820583 for ; Fri, 2 Aug 2013 10:43:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755125Ab3HBKnI (ORCPT ); Fri, 2 Aug 2013 06:43:08 -0400 Received: from mail-la0-f48.google.com ([209.85.215.48]:33373 "EHLO mail-la0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030363Ab3HBKLG (ORCPT ); Fri, 2 Aug 2013 06:11:06 -0400 Received: by mail-la0-f48.google.com with SMTP id hi8so299859lab.7 for ; Fri, 02 Aug 2013 03:11:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:x-gm-message-state; bh=TFMVCSnUKupc3TU0yLRlPGVkU4tr4sVluYpt+ix55hE=; b=byZbSTLPgP91cdbPUyfWchqKgiLdzXWBKgR2mGgAB6TsDl1IdCfFBh1Dtgqv8VmE2V BFJbR/Wni2ReMkfE7zehABW+2zKmMW3oZM9/HeKm0Bk6IG9sZOiI5wnHYQSSsNF4oH9w TyDt1qPy1iP8uHe24G/g6g8fXib8jH+tSp+kuLNAVmMGwLnSEP7azqQ23BpyPPEeea5I jEGU/sNmtIQXg+zZDOVa/d+M6XzZggpCpo67BOSvF5TpsmeLjghff9awQKfI0ROc7zD4 fWe15vpRCfD2WNNJtDWy8BuwxTrxPb8goTkKDNetPs2Ap8DYXnp+BeTGlrBx+CD2n6kt FnFw== X-Received: by 10.152.28.37 with SMTP id y5mr2705289lag.0.1375438264230; Fri, 02 Aug 2013 03:11:04 -0700 (PDT) MIME-Version: 1.0 Received: by 10.112.21.33 with HTTP; Fri, 2 Aug 2013 03:10:44 -0700 (PDT) In-Reply-To: References: <1375355972-25276-1-git-send-email-vikas.sajjan@linaro.org> <5151790.EBRlE0cTxf@flatron> From: Vikas Sajjan Date: Fri, 2 Aug 2013 15:40:44 +0530 Message-ID: Subject: Re: [PATCH] drm/exynos: Add check for IOMMU while passing physically continous memory flag To: Inki Dae Cc: Rob Clark , Tomasz Figa , "linux-samsung-soc@vger.kernel.org" , DRI mailing list , linux-media@vger.kernel.org, "kgene.kim" , "arun.kk" , Patch Tracking , linaro-kernel@lists.linaro.org, sunil joshi , Sylwester Nawrocki , m.szyprowski@samsung.com X-Gm-Message-State: ALoCoQnp9j9nr2sHbENHTcLoTZFtl5yJ0UTAWJqtrFAtKFZ/3Y1f5s5kBmmNg+IVHl5bStOtC5VE Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Inki Dae, On 2 August 2013 12:58, Inki Dae wrote: > > > 2013/8/2 Vikas Sajjan >> >> Hi Rob, >> >> On 2 August 2013 06:03, Rob Clark wrote: >> > On Thu, Aug 1, 2013 at 7:20 PM, Tomasz Figa >> > wrote: >> >> Hi Vikas, >> >> >> >> On Thursday 01 of August 2013 16:49:32 Vikas Sajjan wrote: >> >>> While trying to get boot-logo up on exynos5420 SMDK which has eDP >> >>> panel >> >>> connected with resolution 2560x1600, following error occured even with >> >>> IOMMU enabled: >> >>> [0.880000] [drm:lowlevel_buffer_allocate] *ERROR* failed to allocate >> >>> buffer. [0.890000] [drm] Initialized exynos 1.0.0 20110530 on minor 0 >> >>> >> >>> This patch fixes the issue by adding a check for IOMMU. >> >>> >> >>> Signed-off-by: Vikas Sajjan >> >>> Signed-off-by: Arun Kumar >> >>> --- >> >>> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 9 ++++++++- >> >>> 1 file changed, 8 insertions(+), 1 deletion(-) >> >>> >> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c >> >>> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index 8e60bd6..2a86666 >> >>> 100644 >> >>> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c >> >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c >> >>> @@ -16,6 +16,7 @@ >> >>> #include >> >>> #include >> >>> #include >> >>> +#include >> >>> >> >>> #include "exynos_drm_drv.h" >> >>> #include "exynos_drm_fb.h" >> >>> @@ -143,6 +144,7 @@ static int exynos_drm_fbdev_create(struct >> >>> drm_fb_helper *helper, struct platform_device *pdev = >> >>> dev->platformdev; >> >>> unsigned long size; >> >>> int ret; >> >>> + unsigned int flag; >> >>> >> >>> DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d\n", >> >>> sizes->surface_width, sizes->surface_height, >> >>> @@ -166,7 +168,12 @@ static int exynos_drm_fbdev_create(struct >> >>> drm_fb_helper *helper, size = mode_cmd.pitches[0] * mode_cmd.height; >> >>> >> >>> /* 0 means to allocate physically continuous memory */ >> >>> - exynos_gem_obj = exynos_drm_gem_create(dev, 0, size); >> >>> + if (!is_drm_iommu_supported(dev)) >> >>> + flag = 0; >> >>> + else >> >>> + flag = EXYNOS_BO_NONCONTIG; >> >> >> >> While noncontig memory might be used for devices that support IOMMU, >> >> there >> >> should be no problem with using contig memory for them, so this seems >> >> more >> >> like masking the original problem rather than tracking it down. >> > >> > it is probably a good idea to not require contig memory when it is not >> > needed for performance or functionality (and if it is only >> > performance, then fallback gracefully to non-contig).. but yeah, would >> > be good to know if this is masking another issue all the same >> > >> >> Whats happening with CONTIG flag and with IOMMU, is >> >> __iommu_alloc_buffer() ---> dma_alloc_from_contiguous() and in this >> function it fails at >> this condition check if (pageno >= cma->count) >> >> So I tried increasing the CONFIG_CMA_SIZE_MBYTES to 24, this check >> succeeds and it works well without my patch. >> >> But what about the case where CONFIG_CMA is disabled , yet i want >> bigger memory for a device. >> I think using IOMMU we can achieve this. >> >> correct me, if i am wrong. >> > > I'm on summer vacation so I'm afraid that I cannot test and look into it but > I guess you guy didn't declare CMA region for Exynos drm. And in this case, > the size of CMA declared region is 16MB as default. That is why works well > after increasing default size, CONFIG_CMA_SIZE_MBYTES, to 24MB. And I > mentioned long time ago, we are required to use physically contiguous memory > in case that bootloader uses physically contiguous memory for its own > framebuffer, and kernel wants to share the bootloader's framebuffer region > to resolve flickering issue while booted; that is required for product. And > one question, is there any reason that you guy should use non-contiguous > memory for framebuffer with iommu? > yeah, we could not allocate CMA region for FIMD, because the function dma_declare_contiguous() needs "dev" as the first argument and we have access to "dev" node only if it is NON-DT way of probing like the way it is done in arch/arm/mach-davinci/devices-da8xx.c But now, since the probing is through DT way, there is NO way ( Let me know if something is newly added ) to call dma_declare_contiguous() and reserve CMA region . we don't have any specific requirement for NON_CONTIG or CONTIG memory, but only requirement was to allocate a bigger memory like ( 2560 * 1600 * 4 ) for FB. But as Rob suggested, we should have fall-back case if CONTIG memory allocation fails, as below > Inki Dae > >> >> >> > BR, >> > -R >> > >> >> Could you check why the allocation fails when requesting contiguous >> >> memory? >> >> >> >> Best regards, >> >> Tomasz >> >> >> >> -- >> >> To unsubscribe from this list: send the line "unsubscribe linux-media" >> >> in >> >> the body of a message to majordomo@vger.kernel.org >> >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> >> >> -- >> Thanks and Regards >> Vikas Sajjan >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-media" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index df43fa9..15de626 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c @@ -144,7 +144,6 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper *helper, struct platform_device *pdev = dev->platformdev; unsigned long size; int ret; DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d\n", sizes->surface_width, sizes->surface_height, @@ -167,16 +166,14 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper *helper, size = mode_cmd.pitches[0] * mode_cmd.height; - /* 0 means to allocate physically continuous memory */ - exynos_gem_obj = exynos_drm_gem_create(dev, 0, size); + exynos_gem_obj = exynos_drm_gem_create(dev, EXYNOS_BO_CONTIG, size); if (IS_ERR(exynos_gem_obj)) { - ret = PTR_ERR(exynos_gem_obj); - goto err_release_framebuffer; + if(is_drm_iommu_supported(dev)) + exynos_gem_obj = exynos_drm_gem_create(dev, EXYNOS_BO_NONCONTIG, size); + if (IS_ERR(exynos_gem_obj)) { + ret = PTR_ERR(exynos_gem_obj); + goto err_release_framebuffer; + } + dev_warn("\n exynos_gem_obj for FB is allocated with non physically continuous + memory \n"); } > Thanks,