From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH 12/33] drm/omap: use dma_mapping_error in omap_gem_dma_sync Date: Thu, 25 Feb 2016 17:45:48 +0200 Message-ID: <56CF21AC.80702@ti.com> References: <1455875288-4370-1-git-send-email-tomi.valkeinen@ti.com> <1455875288-4370-13-git-send-email-tomi.valkeinen@ti.com> <5297813.Cubs3arpmg@avalon> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0453750063==" Return-path: Received: from devils.ext.ti.com (devils.ext.ti.com [198.47.26.153]) by gabe.freedesktop.org (Postfix) with ESMTPS id A6A036E8E5 for ; Thu, 25 Feb 2016 15:45:53 +0000 (UTC) In-Reply-To: <5297813.Cubs3arpmg@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 --===============0453750063== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="4qWbEbjat2nRm2dUoI5WRwF2ADkX73ri5" --4qWbEbjat2nRm2dUoI5WRwF2ADkX73ri5 Content-Type: multipart/mixed; boundary="ldqINI6xQDiEe87NT2piHFMTTNFrDx2Le" From: Tomi Valkeinen To: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org, Rob Clark Message-ID: <56CF21AC.80702@ti.com> Subject: Re: [PATCH 12/33] drm/omap: use dma_mapping_error in omap_gem_dma_sync References: <1455875288-4370-1-git-send-email-tomi.valkeinen@ti.com> <1455875288-4370-13-git-send-email-tomi.valkeinen@ti.com> <5297813.Cubs3arpmg@avalon> In-Reply-To: <5297813.Cubs3arpmg@avalon> --ldqINI6xQDiEe87NT2piHFMTTNFrDx2Le Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 24/02/16 00:14, Laurent Pinchart wrote: > Hi Tomi, >=20 > Thank you for the patch. >=20 > On Friday 19 February 2016 11:47:47 Tomi Valkeinen wrote: >> omap_gem_dma_sync() calls dma_map_page() but does not check the possib= le >> error with dma_mapping_error(). If DMA-API debugging is enabled, the >> debug layer will give a warning if dma_mapping_error() has not been >> used. >> >> This patch adds proper error handling to omap_gem_dma_sync(). >> >> Signed-off-by: Tomi Valkeinen >> --- >> drivers/gpu/drm/omapdrm/omap_gem.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c >> b/drivers/gpu/drm/omapdrm/omap_gem.c index 7098190815f1..a60c30a59f7e >> 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_gem.c >> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c >> @@ -775,9 +775,18 @@ void omap_gem_dma_sync(struct drm_gem_object *obj= , >> >> for (i =3D 0; i < npages; i++) { >> if (!omap_obj->addrs[i]) { >> - omap_obj->addrs[i] =3D dma_map_page(dev->dev, pages[i], 0, >> + dma_addr_t addr; >> + >> + addr =3D dma_map_page(dev->dev, pages[i], 0, >> PAGE_SIZE, DMA_BIDIRECTIONAL); >> + >> + if (dma_mapping_error(dev->dev, addr)) { >> + dev_warn(dev->dev, "omap_gem_dma_sync: dma_map_page=20 > failed\n"); >=20 > Same comment as for the previous patch. >=20 > No need to unmap ? I don't know... Maybe, maybe not. The function doesn't return any error, and the mapped pages are stored in omap_obj->addrs[]. So, if the failure was temporary, next time we have already mapped the pages that did succeed. If the failure was not temporary, well, we don't pass any error anyway, so the pages have to be unmapped somewhere in any case. So possibly we could unmap, but I don't see us leaking anything even if the dma_map_page fails. Tomi --ldqINI6xQDiEe87NT2piHFMTTNFrDx2Le-- --4qWbEbjat2nRm2dUoI5WRwF2ADkX73ri5 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 iQIcBAEBCAAGBQJWzyGsAAoJEPo9qoy8lh713LAP/RFMcSEQW88EUIY6UMXajp9g LS8x8CHqGJE2MMzcN5gnSLOwhzDx18YuCYzPcSuyYTZJnGBCJCx1nM8rYRxppPDU O813Flb6aQ5vKVi6SiAbuV8H5hJqqQfypXNOZaR365UaFyamU3rvYyEwXoQjGYGk LyMzG02u9Dv7NoSAgAYpNvQ6FS5bf1y0F9USapuJRMSkEECStVoSE62C7uSrussv uclUIqyYHE4ODvH354oBYCk/UCh6Fk2ELjpFfQMNuCDyMbTIn27t5gCR5km8lYAV BdDkH05ujKw64OXl2CQ9pMx77X1hzeionqlJ/IFVskU2XE97SRNjIoblCVvqxjbZ iPODCXo2bwqYW4EKBvEx2Q5UBz1Tt9wQ39BDsS8njoQfgBKhOifOoHxWX9bk2J7d yJoW8JdNJjiI+e9uFV9Js7UR9N3yJjC7AOPEUcsf/Xhqy+Di1p4Nn3P8cmte8H1j MTzl4YDeDZNwfzD0CvR2YV55xmvpmY/ySZyPosj9FUO51FihOfxQJTAOo7bZwRpQ b+GgMqA94LIl+Co7FH22mEwaawE/hP5zWmzzaQr9VbyVUuyWS2nkGbV7oKICAQ9x 5p/Bnej9yTvoEri1ufKShQxoOm5Vn2LWPfLv06bIGfjtXNHw0jZUTb8zFjskuRAV HWiDGbTMgcCCO4pFzwoR =hZB4 -----END PGP SIGNATURE----- --4qWbEbjat2nRm2dUoI5WRwF2ADkX73ri5-- --===============0453750063== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0453750063==--