From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH 1/5] drm/omap: return error if dma_alloc_writecombine fails Date: Tue, 23 Jun 2015 11:18:16 +0300 Message-ID: <55891648.2030402@ti.com> References: <1434622239-15629-1-git-send-email-tomi.valkeinen@ti.com> <1434622239-15629-2-git-send-email-tomi.valkeinen@ti.com> <4966397.anhbQhtP9U@avalon> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0906701856==" Return-path: 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) In-Reply-To: <4966397.anhbQhtP9U@avalon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============0906701856== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Tfi5Lgq68q6Eirs4Wh7KdjRP0GlqixIq6" --Tfi5Lgq68q6Eirs4Wh7KdjRP0GlqixIq6 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 18/06/15 17:27, Laurent Pinchart wrote: > Hi Tomi, >=20 > Thank you for the patch. >=20 > 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 =3D dma_alloc_writecombine(dev->dev, size, >> &omap_obj->paddr, GFP_KERNEL); >> - if (omap_obj->vaddr) >> - flags |=3D OMAP_BO_DMA; >> + if (!omap_obj->vaddr) { >> + spin_lock(&priv->list_lock); >> + list_del(&omap_obj->mm_list); >> + spin_unlock(&priv->list_lock); >=20 > Wouldn't it be simpler to move the list_add after the "if ((flags &=20 > OMAP_BO_SCANOUT) && !priv->has_dmm)" block ? You could then avoid the=20 > 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 =20 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 doe= s 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. =20 This patch makes the driver return an error if dma_alloc_writecombine= fails. =20 Signed-off-by: Tomi Valkeinen 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_dev= ice *dev, =20 omap_obj =3D 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; =20 obj =3D &omap_obj->base; =20 @@ -1392,11 +1388,19 @@ struct drm_gem_object *omap_gem_new(struct drm_de= vice *dev, */ omap_obj->vaddr =3D dma_alloc_writecombine(dev->dev, size, &omap_obj->paddr, GFP_KERNEL); - if (omap_obj->vaddr) - flags |=3D OMAP_BO_DMA; + if (!omap_obj->vaddr) { + kfree(omap_obj); =20 + return NULL; + } + + flags |=3D OMAP_BO_DMA; } =20 + spin_lock(&priv->list_lock); + list_add(&omap_obj->mm_list, &priv->obj_list); + spin_unlock(&priv->list_lock); + omap_obj->flags =3D flags; =20 if (flags & OMAP_BO_TILED) { --Tfi5Lgq68q6Eirs4Wh7KdjRP0GlqixIq6 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJViRZIAAoJEPo9qoy8lh71H2QQAKfQOlpTFOYM66OaZNaWgS4g PQDb+kJLgx9G+ExwbhjZLADdmQxE5CiYhe8wyko68cttwuS/rw5VYJ39U1ONDTGp 3IC70oA9BxxXhN1CFxPvgwf0Eiz8RThiIiI9m5gG7AZ+Nex5OxhvnU1H5bcA7WOo J+Y0UStN9bQ2FXEPDJeBXZPEUKeNoanObVx8uKgqdSpcH4sm8EU1DkGAZiN6Tj67 ZxNCCI6XE1z1UZX9scM8SRRN76OpadyEtjj2aFCl24Sq5MrEnxUxQ1FSHdMJ/6XT fKvJy1R7APRjEDb2tu1nO+lahpCAHNZhCW70IWR+WqdUXWb2vevqEUEySXiTgDPS pxqBFFo8pNVmYM0alWSf11ZKhG5eCoMSobPVaHjzQF//AwAcAWBocHdwXquMMVHk 1ompCGZCKqq+qIwFMWjDlDeS6qpRnNOsC9WymSkEW8QSVa93mfEC26PJ807+C0tV 3/TMtzZy1j5f1FcGtE3EUcjlAcvDObHRn9MFvR3GQO+iaKQQMIRDaKAXg9DvhjB6 CY1sKhE/wHj5sSWE0RQimpPb3jIBb2WXzVmbyDQQ0ixwzATOGXSSKQJ+py8gEtvs IB0OoAyzh+9v+iJ+VtVNM+oSa9uF2ndvXtr5HVjIsdiJGHA00oIGjz9K6JOyL64D dKR4IipSIZIkImMBCLsk =L+av -----END PGP SIGNATURE----- --Tfi5Lgq68q6Eirs4Wh7KdjRP0GlqixIq6-- --===============0906701856== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============0906701856==--