From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH 3/4] drm/omap: remove align_pitch() Date: Tue, 19 Apr 2016 09:15:23 +0300 Message-ID: <5715CCFB.8090307@ti.com> References: <1460994134-12587-1-git-send-email-tomi.valkeinen@ti.com> <1460994134-12587-3-git-send-email-tomi.valkeinen@ti.com> <1769346.WUG9LYVM4h@avalon> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1895366900==" Return-path: Received: from bear.ext.ti.com (bear.ext.ti.com [192.94.94.41]) by gabe.freedesktop.org (Postfix) with ESMTPS id 032546E67F for ; Tue, 19 Apr 2016 06:15:28 +0000 (UTC) In-Reply-To: <1769346.WUG9LYVM4h@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 --===============1895366900== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="b9EBlOXqCVDVbi6OSB6LdJaF7W3qrq8TU" --b9EBlOXqCVDVbi6OSB6LdJaF7W3qrq8TU Content-Type: multipart/mixed; boundary="WXuUNjRoebkK9BvEUQF5HGolF93H0GNgg" From: Tomi Valkeinen To: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Message-ID: <5715CCFB.8090307@ti.com> Subject: Re: [PATCH 3/4] drm/omap: remove align_pitch() References: <1460994134-12587-1-git-send-email-tomi.valkeinen@ti.com> <1460994134-12587-3-git-send-email-tomi.valkeinen@ti.com> <1769346.WUG9LYVM4h@avalon> In-Reply-To: <1769346.WUG9LYVM4h@avalon> --WXuUNjRoebkK9BvEUQF5HGolF93H0GNgg Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 19/04/16 05:12, Laurent Pinchart wrote: > Hi Tomi, >=20 > Thank you for the patch. >=20 > On Monday 18 Apr 2016 18:42:13 Tomi Valkeinen wrote: >> The previous commit removed aligning the pitch to SGX's pitch >> requirement from align_pitch(). What's left is effectively a function >> that returns width * bytespp. >> >> To clean up the driver, we can remove the function and have the >> calculation inline in the two places which call align_pitch(). >> >> Signed-off-by: Tomi Valkeinen >=20 > Reviewed-by: Laurent Pinchart >=20 > I wonder, however, if the calculation is correct. Shouldn't the pitch b= e=20 > calculated as DIV_ROUND_UP(width * bpp, 8) instead of width *=20 > DIV_ROUND_UP(bpp, 8) ? Well, in practice it doesn't matter. The bpps we use, 32/24/16 are all divisible by 8. So I don't really even know why the div round up is there= in the first place. There's the 12 bit RGB, in a 16 bit container, which in theory works but = I don't think we have ever used it or really supported it. If 'bpp' is 12 i= n this case, width * DIV_ROUND_UP(bpp, 8) gives the correct pitch. But then, I g= uess 'bpp' should be 16 in this case, as the omap_gem.c part is about allocati= ng memory, without knowing the format. Then there are the 1/2/4/8 bpp CLUT modes, which we have never supported = (and they are no longer supported by the HW in recent HW). For those, DIV_ROUND_UP(width * bpp, 8) would be correct. So... I think you are right. Here: =46rom c435ebfc5953d4983c5772ec4fb7c5b0d01ecb9f Mon Sep 17 00:00:00 2001 From: Tomi Valkeinen Date: Tue, 19 Apr 2016 09:06:32 +0300 Subject: [PATCH] drm/omap: fix pitch round-up At the moment we calculate the buffer's pitch with: pitch =3D width * DIV_ROUND_UP(bpp, 8) For CLUT modes with bpp of 1/2/4/8 this gives wrong result, and the correct pitch is: pitch =3D DIV_ROUND_UP(width * bpp, 8) In practice this doesn't change anything, as we don't support CLUT modes, but it's better to have the pitch calculation correct. Signed-off-by: Tomi Valkeinen diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapd= rm/omap_fbdev.c index cb6af7757720..42831b8c46b9 100644 --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c @@ -125,8 +125,8 @@ static int omap_fbdev_create(struct drm_fb_helper *he= lper, mode_cmd.width =3D sizes->surface_width; mode_cmd.height =3D sizes->surface_height; =20 - mode_cmd.pitches[0] =3D mode_cmd.width * - DIV_ROUND_UP(sizes->surface_bpp, 8); + mode_cmd.pitches[0] =3D + DIV_ROUND_UP(mode_cmd.width * sizes->surface_bpp, 8); =20 fbdev->ywrap_enabled =3D priv->has_dmm && ywrap_enabled; if (fbdev->ywrap_enabled) { diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm= /omap_gem.c index 7ea1e00d8ca6..23af01eb1773 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem.c +++ b/drivers/gpu/drm/omapdrm/omap_gem.c @@ -660,7 +660,7 @@ int omap_gem_dumb_create(struct drm_file *file, struc= t drm_device *dev, { union omap_gem_size gsize; =20 - args->pitch =3D args->width * DIV_ROUND_UP(args->bpp, 8); + args->pitch =3D DIV_ROUND_UP(args->width * args->bpp, 8); =20 args->size =3D PAGE_ALIGN(args->pitch * args->height); =20 --WXuUNjRoebkK9BvEUQF5HGolF93H0GNgg-- --b9EBlOXqCVDVbi6OSB6LdJaF7W3qrq8TU 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 iQIcBAEBCAAGBQJXFcz7AAoJEPo9qoy8lh71GxAP/3fuHC/Dr2WD9TN4rQ7yV0P4 Tp7pmbPSvDB8QR2hz/jLTenL7wrzqw5jqhDFtE4fo3JRAAEvePl6AwmPuvrQ6a3P HTLd4QgHCD5fxpTvhSlZD59SqjJw1ynSNTs4oLgv97nKuOio7WdemogAJK5UHoAl J3Wa9FRxqaUZZnOReiVFVe482/L9W0bdlzhEH1aU2pQI07XFzIfb5d06WdxPcarM 1i5tJeDhj1xOlsljSzyyCijtBrceK3tqEAB8vthwga8eHdZWeAPX6flMKTdnLdUO Q35Sr4yC/NzfZD6B6fPZt5UsQ5AxVEsO41OhdS9ALKHny8FbRRXVzDbfxcq5tR26 7ZWr91BeoxWCzQPUiRaTwC/vNeY4Gg1RwJtEHXLQPckYtP8AVP4JKPtMDjksE+lX 6WNnUjmiNMKvK8dpvOCyNa7WXLrVBk4O8TFsrUDVpFMvbI8eBuvirHVaa0VtcHjX zkNxtUcwrqLqC3vfKROUVO+3rAdFU44z0BaCle6uQkJo3O/pLdwUFa50ulTyVphZ Qa5ZYaJRw6HgRbGdDb1wDsE1QOG6FON5Eaa8L10/jbsf3OGi8mpLvv0XM3ArRhgf bYLYCcIxKf5mGh2Kxf2+Q5EHo4Un8Sbe0fkOMUZ0EfWeQGc0SYaZnaLl5xDItCgq qeRMbojVtCbiODtnJGms =jOd0 -----END PGP SIGNATURE----- --b9EBlOXqCVDVbi6OSB6LdJaF7W3qrq8TU-- --===============1895366900== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1895366900==--