From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH 3/4] drm/omapdrm: Work-a-round for errata i734 (LCD1 Gamma) in DSS dispc Date: Mon, 23 May 2016 14:00:27 +0300 Message-ID: <5742E2CB.20009@ti.com> References: <573F0F48.7090902@ti.com> <5742D273.5020204@ti.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1665654764==" Return-path: Received: from devils.ext.ti.com (devils.ext.ti.com [198.47.26.153]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7CCE86E58F for ; Mon, 23 May 2016 11:00:34 +0000 (UTC) In-Reply-To: <5742D273.5020204@ti.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jyri Sarha , dri-devel@lists.freedesktop.org Cc: peter.ujfalusi@ti.com, laurent.pinchart@ideasonboard.com List-Id: dri-devel@lists.freedesktop.org --===============1665654764== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="esihfr42kuI2NXu7tDTrNJWWBswxSsAsu" --esihfr42kuI2NXu7tDTrNJWWBswxSsAsu Content-Type: multipart/mixed; boundary="sP9KKsAHaO14crHrCcP2FDMfPIwjO0Lkc" From: Tomi Valkeinen To: Jyri Sarha , dri-devel@lists.freedesktop.org Cc: airlied@linux.ie, daniel@ffwll.ch, peter.ujfalusi@ti.com, bparrot@ti.com, laurent.pinchart@ideasonboard.com Message-ID: <5742E2CB.20009@ti.com> Subject: Re: [PATCH 3/4] drm/omapdrm: Work-a-round for errata i734 (LCD1 Gamma) in DSS dispc References: <573F0F48.7090902@ti.com> <5742D273.5020204@ti.com> In-Reply-To: <5742D273.5020204@ti.com> --sP9KKsAHaO14crHrCcP2FDMfPIwjO0Lkc Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 23/05/16 12:50, Jyri Sarha wrote: > On 05/20/16 16:21, Tomi Valkeinen wrote: >>>> + >>>> +static int dispc_errata_i734_wa_init(void) >>>> +{ >>>> + size_t buff_size =3D i734.ovli.width * i734.ovli.height * >> "buf_size" would match the other names =3D). >> >> Usually I try to do only simple initializations when declaring the >> locals. This is a bit on the complex side, and also there's a check >> below that can make all this init calculation not needed. >> >=20 > Compiler optimizations should delay the initialization to happen only i= f > it is needed. That is true. My worry is more on the functional side: if you do, say, calculations there and, e.g. do a division, it's easy to get division by zero if all the variables are not actually valid. Or if you do get_foo(channel), and get_foo() only works for certain channels, again you could hit a crash. Those kind of things happen easily when you have code like: int foo =3D get_foo(channel); if (channel !=3D XYZ) return; >>>> + >>>> +static void dispc_errata_i734_wa(void) >>>> +{ >>>> + u32 vsync_irq =3D dispc_mgr_get_vsync_irq(OMAP_DSS_CHANNEL_LCD); >>>> + u32 framedone_irq =3D dispc_mgr_get_framedone_irq(OMAP_DSS_CHANNEL= _LCD); >>>> + u32 gatestate =3D REG_GET(DISPC_CONTROL, 8, 4); >> Here too, too complex initializations. Especially doing register >> reads/writes should be something done in the "real" code. The bits you= >> read might not even be valid on some platforms which don't have the i7= 34. >> >=20 > You are right about the register read. Compiler should not optimize the= > register access. But - but with your permission - I'll keep the IRQ mas= k > initializations as they are quite constant in nature anyway. Yes, those should be fine. Presuming all DSS versions have CHANNEL_LCD, which happens to be the case =3D). Tomi --sP9KKsAHaO14crHrCcP2FDMfPIwjO0Lkc-- --esihfr42kuI2NXu7tDTrNJWWBswxSsAsu 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 iQIcBAEBCAAGBQJXQuLLAAoJEPo9qoy8lh71oUoP/jimBT+RbwDzpHd0w9JZJeB4 vF+SHNqwFD7Z7fM+/PXH7ycrHmRKTQjcIEi5qDwdFTLpCdTRRCSls8EzjRc2jjWp GXN90lGQqeDkyEYgxEMiABWtjPL/KMYEXEPsCpSQRN2boZpe8SAerhvzEjhfO+qL m9qx/YbwXsG1/YCkl5fA+Ahhojl0hxlLHRloyPtdsbf/+0Tm5QTA57amU5t8IBPY uVEzJbsp7rXGw/Sexzga+RZpPa/iY9ulRLk8FE4B5lBMX0zxMNwVgPZ9YuSxY8sR k5HrCLZtWNwWc8tSjiufPKJ3iwookJK2SCejL/F25je3MPWt8UOzn7wjepHdYr3Q wChLZYOb6HUw2XLQhTk6/1FUA47031bpHBZLJqd5ksN0AWCCbD5LHnMrxV1zOSjJ rUn4uQIGeTt9RlS/VfyVBijsGk78DrHSJNbxMqtm8ms90mr42DOMNz1JgnACkHfz 7lS+tySBESnwhHGDY49w2YBBeBhoaAql+G7X0+qROap0SudpXNQbGS5X0mtGqv/z Smv935E1COKuN37nsYosibqv6yIA/Vx1Ikby0m+jy6LKorspNKfk9uwIShG3USLb r6R86p8BFAF0F9mq+Ksp4PfOLcCV9sR8SA3FFW8fWLWO17zKwSX/H+ljjXL8Fnty NzSaDzQ6l2j2gMAJdwzw =Fbn7 -----END PGP SIGNATURE----- --esihfr42kuI2NXu7tDTrNJWWBswxSsAsu-- --===============1665654764== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1665654764==--