From mboxrd@z Thu Jan 1 00:00:00 1970 From: eric@anholt.net (Eric Anholt) Date: Sat, 07 Apr 2018 18:08:40 -0700 Subject: [PATCH 2/2 v2] drm/pl111: Enable device-specific assigned memory In-Reply-To: <20180406141935.6801-2-linus.walleij@linaro.org> References: <20180406141935.6801-1-linus.walleij@linaro.org> <20180406141935.6801-2-linus.walleij@linaro.org> Message-ID: <87fu46o67b.fsf@anholt.net> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Linus Walleij writes: > The Versatile Express has 8 MB of dedicated video RAM (VRAM) > on the motherboard, which is what we should be using for the > PL111 if available. On this platform, the memory backplane > is constructed so that only this memory will work properly > with the CLCD on the motherboard, using any other memory > region just gives random snow on the display. > > The CA9 Versatile Express also has a PL111 instance on its > core tile. This is OK, it has been tested with the motherboard > VRAM and that works just as fine as regular CMA memory. > > The memory is assigned to the device using the memory-region > device tree property and a "shared-dma-pool" reserved > memory pool like this: > > reserved-memory { > #address-cells = <1>; > #size-cells = <1>; > ranges; > > vram: vram at 48000000 { > compatible = "shared-dma-pool"; > reg = <0x48000000 0x00800000>; > no-map; > }; > }; > > clcd at 1f000 { > compatible = "arm,pl111", "arm,primecell"; > (...) > memory-region = <&vram>; > }?; > > Cc: Liviu Dudau > Cc: Mali DP Maintainers > Signed-off-by: Linus Walleij > --- > ChangeLog v1->v2: > - Make sure to also call of_reserved_mem_device_release() at > remove() and errorpath. > --- > drivers/gpu/drm/pl111/pl111_drv.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c > index 4621259d5387..bc57c15d9fe4 100644 > --- a/drivers/gpu/drm/pl111/pl111_drv.c > +++ b/drivers/gpu/drm/pl111/pl111_drv.c > @@ -60,6 +60,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -257,6 +258,10 @@ static int pl111_amba_probe(struct amba_device *amba_dev, > drm->dev_private = priv; > priv->variant = variant; > > + ret = of_reserved_mem_device_init(dev); > + if (!ret) > + dev_info(dev, "using device-specific reserved memory\n"); > + > 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? 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? -------------- 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: Sat, 07 Apr 2018 18:08:40 -0700 Message-ID: <87fu46o67b.fsf@anholt.net> References: <20180406141935.6801-1-linus.walleij@linaro.org> <20180406141935.6801-2-linus.walleij@linaro.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1032161630==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id F31B86E096 for ; Sun, 8 Apr 2018 01:08:43 +0000 (UTC) In-Reply-To: <20180406141935.6801-2-linus.walleij@linaro.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Linus Walleij , Daniel Vetter , Jani Nikula , Sean Paul , Liviu Dudau Cc: Mali DP Maintainers , linux-arm-kernel@lists.infradead.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1032161630== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Linus Walleij writes: > The Versatile Express has 8 MB of dedicated video RAM (VRAM) > on the motherboard, which is what we should be using for the > PL111 if available. On this platform, the memory backplane > is constructed so that only this memory will work properly > with the CLCD on the motherboard, using any other memory > region just gives random snow on the display. > > The CA9 Versatile Express also has a PL111 instance on its > core tile. This is OK, it has been tested with the motherboard > VRAM and that works just as fine as regular CMA memory. > > The memory is assigned to the device using the memory-region > device tree property and a "shared-dma-pool" reserved > memory pool like this: > > reserved-memory { > #address-cells =3D <1>; > #size-cells =3D <1>; > ranges; > > vram: vram@48000000 { > compatible =3D "shared-dma-pool"; > reg =3D <0x48000000 0x00800000>; > no-map; > }; > }; > > clcd@1f000 { > compatible =3D "arm,pl111", "arm,primecell"; > (...) > memory-region =3D <&vram>; > }=C2=B7; > > Cc: Liviu Dudau > Cc: Mali DP Maintainers > Signed-off-by: Linus Walleij > --- > ChangeLog v1->v2: > - Make sure to also call of_reserved_mem_device_release() at > remove() and errorpath. > --- > drivers/gpu/drm/pl111/pl111_drv.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl= 111_drv.c > index 4621259d5387..bc57c15d9fe4 100644 > --- a/drivers/gpu/drm/pl111/pl111_drv.c > +++ b/drivers/gpu/drm/pl111/pl111_drv.c > @@ -60,6 +60,7 @@ > #include > #include > #include > +#include >=20=20 > #include > #include > @@ -257,6 +258,10 @@ static int pl111_amba_probe(struct amba_device *amba= _dev, > drm->dev_private =3D priv; > priv->variant =3D variant; >=20=20 > + ret =3D of_reserved_mem_device_init(dev); > + if (!ret) > + dev_info(dev, "using device-specific reserved memory\n"); > + > 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 =3D 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 =3D 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? 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? --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlrJa5gACgkQtdYpNtH8 nugJqA/+O6V2bN2zchX7Xh3SrPLSqUX6mI5QplSAerEXa2LU1JZVcLUicO4OOMR2 QmTlRdJUdEAab1xrAxlh05trDVYQD4BulY6JHIve1c9XB58/pUDdNGCQmlejDtNg 1vYwBrLpCBQBlnXRdR1XceYFOZhEq4pXprgN7pYXiFwd3SH1bLWbFkOvW3y/1dVR WYeMcX+iUsgDK7qsvY8BhTl59YsUoO3EWsYs88lJ55CpSWq9IUrJ+8hHtv/wUCil qsXTvS5LBiMMO4JZuaB9RkNQu8e1nYBh1AJTkN8JYABDVvco8t2xQ90umaMQ9haY VqCp4NEIna7/FwMUMI5/7Lt1Sr3NKi3qtJtU4DygLL76/7I96Wg6eOni12Kr+T9n 6UGKuivWOriLrjvx0vrznkTVrU8nJV5NIcADZ/YpTXPo4G4Xuu6AY8yi16I83+Vv wBJVpWn17N1oq4lC/+vuPhN6B9lv3MkZbOrvCXfGpdi1+sPzD7nqilwMTpSunVyj FgqMREuRvQbEnqb3Jl/LqryGK4yAgy20J8uiSOnyvxOnHqmC/xtIncr/rODhsdLg lEqpxO5ujroDE1VlqmShi0ti3nROCPDwkdCXWHP0NNyddWb+5nCfSMdDSuEfXp4p x4FGIOJzls5buXK7sNrdOlFaa7gR2eHSCOcUtAjAJkEVxlVJ0z4= =L5Ns -----END PGP SIGNATURE----- --=-=-=-- --===============1032161630== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1032161630==--