From mboxrd@z Thu Jan 1 00:00:00 1970 From: eric@anholt.net (Eric Anholt) Date: Sat, 10 Feb 2018 17:14:30 +0000 Subject: [PATCH 3/3] drm/pl111: Use max memory bandwidth for resolution In-Reply-To: <20180206121854.4407-4-linus.walleij@linaro.org> References: <20180206121854.4407-1-linus.walleij@linaro.org> <20180206121854.4407-4-linus.walleij@linaro.org> Message-ID: <876074dax5.fsf@anholt.net> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Linus Walleij writes: > We were previously selecting 1024x768 and 32BPP as the default > set-up for the PL111 consumers. > > This does not work on elder systems: the device tree bindings > support a property "max-memory-bandwidth" in bytes/second that > states that if you exceed this the memory bus will saturate. > The result is flickering and unstable images. > > Parse the "max-memory-bandwidth" and respect it when > intializing the driver. On the RealView PB11MP, Versatile and > Integrator/CP we get a nice console as default with this code. > > Signed-off-by: Linus Walleij > --- > ChangeLog v1->v2: > - Exploit the new .mode_valid() callback we added to the > simple KMS helper. > - Use the hardcoded bits per pixel per variant instead of > trying to be heuristic about this setting for now. > --- > drivers/gpu/drm/pl111/pl111_display.c | 30 ++++++++++++++++++++++++++++++ > drivers/gpu/drm/pl111/pl111_drm.h | 1 + > drivers/gpu/drm/pl111/pl111_drv.c | 6 ++++++ > 3 files changed, 37 insertions(+) > > diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c > index d75923896609..a1ca9e1ffe15 100644 > --- a/drivers/gpu/drm/pl111/pl111_display.c > +++ b/drivers/gpu/drm/pl111/pl111_display.c > @@ -50,6 +50,35 @@ irqreturn_t pl111_irq(int irq, void *data) > return status; > } > > +static enum drm_mode_status > +pl111_mode_valid(struct drm_crtc *crtc, > + const struct drm_display_mode *mode) > +{ > + struct drm_device *drm = crtc->dev; > + struct pl111_drm_dev_private *priv = drm->dev_private; > + u32 cpp = priv->variant->fb_bpp / 8; > + u64 bw; Using the variant->fb_bpp for mode_valid checks here feels wrong to me -- it means a larger mode wouldn't be considered valid on a 32bpp-preferred platform when 16bpp would make it work, and a 16bpp platform will happily try to set a 32bpp mode that exceeds the bandwidth. On the other hand, if it makes things work most of the time I'm also kind of OK with it. Anyone else want to chime in here? > + /* > + * We use the pixelclock to also account for interlaced modes, the > + * resulting bandwidth is in bytes per second. > + */ > + bw = mode->clock * 1000; /* In Hz */ > + bw = bw * mode->hdisplay * mode->vdisplay * cpp; > + bw = div_u64(bw, mode->htotal * mode->vtotal); > + > + if (bw > priv->memory_bw) { > + DRM_DEBUG("%d x %d @ %d Hz, %d cpp, bw %llu too fast\n", > + mode->hdisplay, mode->vdisplay, mode->clock, cpp, bw); > + > + return MODE_BAD; > + } > + DRM_DEBUG("%d x %d @ %d Hz, %d cpp, bw %llu bytes/s OK\n", > + mode->hdisplay, mode->vdisplay, mode->clock, cpp, bw); > + > + return MODE_OK; > +} Maybe DRM_DEBUG_KMS for these? -------------- 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 3/3] drm/pl111: Use max memory bandwidth for resolution Date: Sat, 10 Feb 2018 17:14:30 +0000 Message-ID: <876074dax5.fsf@anholt.net> References: <20180206121854.4407-1-linus.walleij@linaro.org> <20180206121854.4407-4-linus.walleij@linaro.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2000985890==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id AC0806E127 for ; Sat, 10 Feb 2018 17:17:05 +0000 (UTC) In-Reply-To: <20180206121854.4407-4-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 , Peter Ujfalusi , Tomi Valkeinen Cc: linux-arm-kernel@lists.infradead.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============2000985890== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Linus Walleij writes: > We were previously selecting 1024x768 and 32BPP as the default > set-up for the PL111 consumers. > > This does not work on elder systems: the device tree bindings > support a property "max-memory-bandwidth" in bytes/second that > states that if you exceed this the memory bus will saturate. > The result is flickering and unstable images. > > Parse the "max-memory-bandwidth" and respect it when > intializing the driver. On the RealView PB11MP, Versatile and > Integrator/CP we get a nice console as default with this code. > > Signed-off-by: Linus Walleij > --- > ChangeLog v1->v2: > - Exploit the new .mode_valid() callback we added to the > simple KMS helper. > - Use the hardcoded bits per pixel per variant instead of > trying to be heuristic about this setting for now. > --- > drivers/gpu/drm/pl111/pl111_display.c | 30 ++++++++++++++++++++++++++++++ > drivers/gpu/drm/pl111/pl111_drm.h | 1 + > drivers/gpu/drm/pl111/pl111_drv.c | 6 ++++++ > 3 files changed, 37 insertions(+) > > diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl11= 1/pl111_display.c > index d75923896609..a1ca9e1ffe15 100644 > --- a/drivers/gpu/drm/pl111/pl111_display.c > +++ b/drivers/gpu/drm/pl111/pl111_display.c > @@ -50,6 +50,35 @@ irqreturn_t pl111_irq(int irq, void *data) > return status; > } >=20=20 > +static enum drm_mode_status > +pl111_mode_valid(struct drm_crtc *crtc, > + const struct drm_display_mode *mode) > +{ > + struct drm_device *drm =3D crtc->dev; > + struct pl111_drm_dev_private *priv =3D drm->dev_private; > + u32 cpp =3D priv->variant->fb_bpp / 8; > + u64 bw; Using the variant->fb_bpp for mode_valid checks here feels wrong to me =2D- it means a larger mode wouldn't be considered valid on a 32bpp-preferred platform when 16bpp would make it work, and a 16bpp platform will happily try to set a 32bpp mode that exceeds the bandwidth. On the other hand, if it makes things work most of the time I'm also kind of OK with it. Anyone else want to chime in here? > + /* > + * We use the pixelclock to also account for interlaced modes, the > + * resulting bandwidth is in bytes per second. > + */ > + bw =3D mode->clock * 1000; /* In Hz */ > + bw =3D bw * mode->hdisplay * mode->vdisplay * cpp; > + bw =3D div_u64(bw, mode->htotal * mode->vtotal); > + > + if (bw > priv->memory_bw) { > + DRM_DEBUG("%d x %d @ %d Hz, %d cpp, bw %llu too fast\n", > + mode->hdisplay, mode->vdisplay, mode->clock, cpp, bw); > + > + return MODE_BAD; > + } > + DRM_DEBUG("%d x %d @ %d Hz, %d cpp, bw %llu bytes/s OK\n", > + mode->hdisplay, mode->vdisplay, mode->clock, cpp, bw); > + > + return MODE_OK; > +} Maybe DRM_DEBUG_KMS for these? --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlp/KHYACgkQtdYpNtH8 nuijvw//VcU9NfrxBCpfEdWfOpTNmKsTMlKPqvB5DKZI0bl+YCg+gC7wGOIE9h5H WGncMNLDUoSPK+lVic+dJyQFUwxF8t3ZYJNtYOLgl+DI7nQHZkQAXvRcGPr3KJpN EIVFKoSR3ohoiEXgL9UyvEemePIi6ZinhnOXv8Icx8m0eaErXCny1M0PiX/aEphr taFqgSfJ+l9PkwSDwTfaoIFIeiziSxyXiS9XdqgKsc5xKQgZgQYSDyiNV+J4fUrU oqw2BRTc0ru+m7T+1Tc4cECAv2YoK47mo3TBxLAyk0xhHwmuBy8tJAS4HWQWHhrY zhPQmMXcJNlQijjJhfKdY3HGdf2JzrfZrdTU9QctosdZMTNH+yR0V9P2P18/X0ke A4F9j2Q/MT7+rX79hMBKmJr8Szuquu8AFrUR4eZccWFPQdPRmWjkWI9iYX2yj08a cOk+FTiJHuQ9cPb5YSjn62HNeHzKbHxB9sPiWzVJMULQhqzznRpdu9L6VIGxRxLw /+8s3pU6kyCs8tw+4gZhnqikrBc5zDH0bmd/tr7/UVn4JSrNJThdaBkOaBEnuyH1 D466KBRf3rLm4g6vSl3u4UIwUKFHfEyAZWSPUbsN0lsYFYgBehxZ3otcHT7xu3c3 duwPFryCrT3jR3dFoAf1QuJJmyxUGKQNCNXLO4WzA2rYlNx0+j4= =XYQ6 -----END PGP SIGNATURE----- --=-=-=-- --===============2000985890== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============2000985890==--