From mboxrd@z Thu Jan 1 00:00:00 1970 From: eric@anholt.net (Eric Anholt) Date: Tue, 17 Apr 2018 11:44:55 -0700 Subject: [PATCH 2/2 v2] drm/pl111: Enable device-specific assigned memory In-Reply-To: References: <20180406141935.6801-1-linus.walleij@linaro.org> <20180406141935.6801-2-linus.walleij@linaro.org> <87fu46o67b.fsf@anholt.net> Message-ID: <87in8pzn88.fsf@anholt.net> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Linus Walleij writes: > On Sun, Apr 8, 2018 at 3:08 AM, Eric Anholt wrote: > >>> if (of_property_read_u32(dev->of_node, "max-memory-bandwidth", >>> &priv->memory_bw)) { >>> dev_info(dev, "no max memory bandwidth specified, assume unlimited\n"); >>> @@ -275,7 +280,8 @@ static int pl111_amba_probe(struct amba_device *amba_dev, >>> priv->regs = devm_ioremap_resource(dev, &amba_dev->res); >>> if (IS_ERR(priv->regs)) { >>> dev_err(dev, "%s failed mmio\n", __func__); >>> - return PTR_ERR(priv->regs); >>> + ret = PTR_ERR(priv->regs); >>> + goto mem_rel; >>> } >> >> Shouldn't this error path be jumping to dev_unref, as well? We're >> trying to match drm_dev_alloc(), right? > > OK I fixed. > >> I'm still a little bothered that we're allowing imports to pl111 of CMA >> buffers that we can't scan out. Could we just refuse all >> .gem_prime_import*() on this platform? > > I am sorry but I do not understand how CMA, dmabuf and GEM play > together to decode this and figure out what to do. > > Do you mean that if we find device assigned memory, i.e. that there > is this special memory which is all the controller can scan out, > I should just implement .gem_prime_impport() instead of the > currently assigned drm_gem_prime_import() to something just > returning return ERR_PTR(-EINVAL); so it always fails? > > What about .gem_prime_import_sg_table()? Exporting should > work fine since the memory is always readable by CPU. Oh, I think you still want drm_gem_prime_import()'s path for "I'm importing an fd from this same driver to into another process". So, yeah, have our .import_sg_table hook throw -EINVAL if called on the device memory platform, and otherwise call down to drm_gem_cma_prime_import_sg_table(). -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 832 bytes Desc: not available URL: From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: [PATCH 2/2 v2] drm/pl111: Enable device-specific assigned memory Date: Tue, 17 Apr 2018 11:44:55 -0700 Message-ID: <87in8pzn88.fsf@anholt.net> References: <20180406141935.6801-1-linus.walleij@linaro.org> <20180406141935.6801-2-linus.walleij@linaro.org> <87fu46o67b.fsf@anholt.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0428430033==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id B02836E166 for ; Tue, 17 Apr 2018 18:44:57 +0000 (UTC) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Linus Walleij Cc: Liviu Dudau , "open list:DRM PANEL DRIVERS" , Mali DP Maintainers , Daniel Vetter , Linux ARM List-Id: dri-devel@lists.freedesktop.org --===============0428430033== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain Linus Walleij writes: > On Sun, Apr 8, 2018 at 3:08 AM, Eric Anholt wrote: > >>> if (of_property_read_u32(dev->of_node, "max-memory-bandwidth", >>> &priv->memory_bw)) { >>> dev_info(dev, "no max memory bandwidth specified, assume unlimited\n"); >>> @@ -275,7 +280,8 @@ static int pl111_amba_probe(struct amba_device *amba_dev, >>> priv->regs = devm_ioremap_resource(dev, &amba_dev->res); >>> if (IS_ERR(priv->regs)) { >>> dev_err(dev, "%s failed mmio\n", __func__); >>> - return PTR_ERR(priv->regs); >>> + ret = PTR_ERR(priv->regs); >>> + goto mem_rel; >>> } >> >> Shouldn't this error path be jumping to dev_unref, as well? We're >> trying to match drm_dev_alloc(), right? > > OK I fixed. > >> I'm still a little bothered that we're allowing imports to pl111 of CMA >> buffers that we can't scan out. Could we just refuse all >> .gem_prime_import*() on this platform? > > I am sorry but I do not understand how CMA, dmabuf and GEM play > together to decode this and figure out what to do. > > Do you mean that if we find device assigned memory, i.e. that there > is this special memory which is all the controller can scan out, > I should just implement .gem_prime_impport() instead of the > currently assigned drm_gem_prime_import() to something just > returning return ERR_PTR(-EINVAL); so it always fails? > > What about .gem_prime_import_sg_table()? Exporting should > work fine since the memory is always readable by CPU. Oh, I think you still want drm_gem_prime_import()'s path for "I'm importing an fd from this same driver to into another process". So, yeah, have our .import_sg_table hook throw -EINVAL if called on the device memory platform, and otherwise call down to drm_gem_cma_prime_import_sg_table(). --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlrWQKcACgkQtdYpNtH8 nujRHA/9H+kNCMCwRBJjK9Wa/q3aXzRbcW8jKjprUaoSzrOm9TtTCTCOpZaYCUc7 0t+yNnA6BWau3ycIWx+fFlMmYVgzWuPC0wLB7VrcU6XPgKfe/wHNnVRzkDCQE5rY vpci0Z4fxiR4BpQP17YCpGVtnP0/O21q9s9qRm3I1o4P188O/OhonT/YIEaPavp8 M5FQEm4VD/qnjnbvfyqM+uSHGmd43cJZEvP0a6bNiR9tc2sdo4mmXAzXhNAnqH8V PXPrStM2nBW9onhfurKGw3q1oTA5ufe22cbQVdILPpcYOND+Ltrd7cI9VXAQBoNi p9VR2gEzGxu5InS/oJ2QPHsX5m9RKDk4fUgqOtWLU+XXOE4pqU2/Bj8Jfl4E0blD Qreussbi7DjLRs1f8MO0yoP54HswJuUqEgKL8mLRt6c37iAUdUEu9GiuyBa7tWNK ZkZJi9l9+tGFrgABIC9ntdFIQRLBbLafYGR5kHwGVEI4wPJis64cYDebSUUSMyid W4HLS9NjJZ+PgXkm9SD8h0mDwHb/PtMBDo6EpuwvZ5eFBb2HUwLaI7iOmbCRt1EP Sbd2lnu5i++2vXJ31nGbSBEqYi1z9+gQRS2Su5QojQhLtNxjXznwLoyykjERGRqX k3MOf3Ia9gHB3D3H/ter8d1Ryq/oM+zWZwtIW38c0MQ+AIId0wQ= =xu2c -----END PGP SIGNATURE----- --=-=-=-- --===============0428430033== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0428430033==--