From patchwork Tue Jun 23 08:18:16 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 6659231 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 6C7569F380 for ; Tue, 23 Jun 2015 08:18:28 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 887BD20613 for ; Tue, 23 Jun 2015 08:18:27 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 975C7204EA for ; Tue, 23 Jun 2015 08:18:26 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 58C916E331; Tue, 23 Jun 2015 01:18:25 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from devils.ext.ti.com (devils.ext.ti.com [198.47.26.153]) by gabe.freedesktop.org (Postfix) with ESMTP id CDCC86E331 for ; Tue, 23 Jun 2015 01:18:23 -0700 (PDT) Received: from dflxv15.itg.ti.com ([128.247.5.124]) by devils.ext.ti.com (8.13.7/8.13.7) with ESMTP id t5N8IIw5009880; Tue, 23 Jun 2015 03:18:18 -0500 Received: from DLEE71.ent.ti.com (dlee71.ent.ti.com [157.170.170.114]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id t5N8IHEi030243; Tue, 23 Jun 2015 03:18:18 -0500 Received: from dlep33.itg.ti.com (157.170.170.75) by DLEE71.ent.ti.com (157.170.170.114) with Microsoft SMTP Server id 14.3.224.2; Tue, 23 Jun 2015 03:18:17 -0500 Received: from [192.168.2.6] (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep33.itg.ti.com (8.14.3/8.13.8) with ESMTP id t5N8IGVi026739; Tue, 23 Jun 2015 03:18:16 -0500 Message-ID: <55891648.2030402@ti.com> Date: Tue, 23 Jun 2015 11:18:16 +0300 From: Tomi Valkeinen User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Laurent Pinchart Subject: Re: [PATCH 1/5] drm/omap: return error if dma_alloc_writecombine fails References: <1434622239-15629-1-git-send-email-tomi.valkeinen@ti.com> <1434622239-15629-2-git-send-email-tomi.valkeinen@ti.com> <4966397.anhbQhtP9U@avalon> In-Reply-To: <4966397.anhbQhtP9U@avalon> Cc: dri-devel@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 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" X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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 On 18/06/15 17:27, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Thursday 18 June 2015 13:10:35 Tomi Valkeinen wrote: >> On a platform with no TILER (e.g. omap3, am43xx), when the user wants to >> allocate buffer with flag OMAP_BO_SCANOUT, the buffer needs to be >> allocated with dma_alloc_writecombine. For some reason the driver does >> not return an error if that alloc fails, instead it continues without >> backing memory. This leads to errors later when the user tries to use >> the buffer. >> >> This patch makes the driver return an error if dma_alloc_writecombine >> fails. >> >> Signed-off-by: Tomi Valkeinen >> --- >> drivers/gpu/drm/omapdrm/omap_gem.c | 12 ++++++++++-- >> 1 file changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c >> b/drivers/gpu/drm/omapdrm/omap_gem.c index 2ab77801cf5f..51c5635aff62 >> 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_gem.c >> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c >> @@ -1392,9 +1392,17 @@ struct drm_gem_object *omap_gem_new(struct drm_device >> *dev, */ >> omap_obj->vaddr = dma_alloc_writecombine(dev->dev, size, >> &omap_obj->paddr, GFP_KERNEL); >> - if (omap_obj->vaddr) >> - flags |= OMAP_BO_DMA; >> + if (!omap_obj->vaddr) { >> + spin_lock(&priv->list_lock); >> + list_del(&omap_obj->mm_list); >> + spin_unlock(&priv->list_lock); > > Wouldn't it be simpler to move the list_add after the "if ((flags & > OMAP_BO_SCANOUT) && !priv->has_dmm)" block ? You could then avoid the > list_del. Thanks, it's cleaner that way: commit 70a7c80ae9f787031fa3eebc70df024deadde09f (HEAD) Author: Tomi Valkeinen Date: Tue Mar 17 15:31:11 2015 +0200 drm/omap: return error if dma_alloc_writecombine fails On a platform with no TILER (e.g. omap3, am43xx), when the user wants to allocate buffer with flag OMAP_BO_SCANOUT, the buffer needs to be allocated with dma_alloc_writecombine. For some reason the driver does not return an error if that alloc fails, instead it continues without backing memory. This leads to errors later when the user tries to use the buffer. This patch makes the driver return an error if dma_alloc_writecombine fails. Signed-off-by: Tomi Valkeinen Acked-by: Laurent Pinchart diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c index 2ab77801cf5f..874eb6dcf94b 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem.c +++ b/drivers/gpu/drm/omapdrm/omap_gem.c @@ -1378,11 +1378,7 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev, omap_obj = kzalloc(sizeof(*omap_obj), GFP_KERNEL); if (!omap_obj) - goto fail; - - spin_lock(&priv->list_lock); - list_add(&omap_obj->mm_list, &priv->obj_list); - spin_unlock(&priv->list_lock); + return NULL; obj = &omap_obj->base; @@ -1392,11 +1388,19 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev, */ omap_obj->vaddr = dma_alloc_writecombine(dev->dev, size, &omap_obj->paddr, GFP_KERNEL); - if (omap_obj->vaddr) - flags |= OMAP_BO_DMA; + if (!omap_obj->vaddr) { + kfree(omap_obj); + return NULL; + } + + flags |= OMAP_BO_DMA; } + spin_lock(&priv->list_lock); + list_add(&omap_obj->mm_list, &priv->obj_list); + spin_unlock(&priv->list_lock); + omap_obj->flags = flags; if (flags & OMAP_BO_TILED) {