From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 06/10] drm/tegra: Handle 64-bit return from drm_crtc_vblank_count() Date: Wed, 7 Feb 2018 10:41:25 +0100 Message-ID: <20180207094125.GA2130@ulmo> References: <20180203051302.9974-1-dhinakaran.pandiyan@intel.com> <20180203051302.9974-6-dhinakaran.pandiyan@intel.com> <1517969059.11349.39.camel@dk-H97M-D3H> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1657773721==" Return-path: In-Reply-To: <1517969059.11349.39.camel@dk-H97M-D3H> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "Pandiyan, Dhinakaran" Cc: "intel-gfx@lists.freedesktop.org" , "treding@nvidia.com" , "dri-devel@lists.freedesktop.org" , "Vivi, Rodrigo" List-Id: intel-gfx@lists.freedesktop.org --===============1657773721== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="u3/rZRmxL6MmkK24" Content-Disposition: inline --u3/rZRmxL6MmkK24 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Feb 07, 2018 at 01:41:18AM +0000, Pandiyan, Dhinakaran wrote: > On Fri, 2018-02-02 at 21:12 -0800, Dhinakaran Pandiyan wrote: > > 570e86963a51 ("drm: Widen vblank count to 64-bits [v3]") changed the > > return type for drm_crtc_vblank_count() to u64. This could cause > > potential problems if the return value is used in arithmetic operations > > with a 32-bit reference HW vblank count. Explicitly typecasting this > > down to u32 either fixes a potential problem or serves to add clarity in > > case the implicit typecasting was already correct. > >=20 > > Cc: Keith Packard > > Cc: Thierry Reding >=20 >=20 > Thierry,=20 >=20 > Can I get an Ack on this please?=20 >=20 > > Signed-off-by: Dhinakaran Pandiyan > > --- > > drivers/gpu/drm/tegra/dc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > >=20 > > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c > > index b8403ed48285..49df2db2ad46 100644 > > --- a/drivers/gpu/drm/tegra/dc.c > > +++ b/drivers/gpu/drm/tegra/dc.c > > @@ -1359,7 +1359,7 @@ static u32 tegra_dc_get_vblank_counter(struct drm= _crtc *crtc) > > return host1x_syncpt_read(dc->syncpt); > > =20 > > /* fallback to software emulated VBLANK counter */ > > - return drm_crtc_vblank_count(&dc->base); > > + return (u32)drm_crtc_vblank_count(&dc->base); Isn't this the wrong way around? Shouldn't we instead make the ->get_vblank_counter() callback return u64 like drm_crtc_vblank_count()? Thierry --u3/rZRmxL6MmkK24 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAlp6ycMACgkQ3SOs138+ s6GrUQ/8CvzLJmENRRQk7w7e47ovqJuNWQu9WxV2i3RuH9ieWETwzzZpUlnwVAML 2eePiJX19v+++zcE0IUm9B423z2fc2MHBu2rZJROHiFzoiH46vbHHCOv4WJe+qiW L6nDoSxFWD2Z56YTjNlKe7VdQVOUVud02ObzOirACgaLdr6drm7yFyDpVS2lm/P5 4E2h7ed2wor2gtjqt4gyZGLiWUZRnS6cJYxdxazA8n56qsKAzr7ZDYpqGy17zv5s PaDnrdsOrrx5qdFt4KjFqvcXOlPLo4HgjQ2nxt/5UDAS1bh+B/nUOfsu8VrGy50N 2cr7ve5X7Bete46pfFbV0OYrkP3j3oB30an2RCVeisr4ieUHk48EKbi72eF/J/3x EPz0QYhBBnl0sOT85KHwSVvobtXA+ZarrehL8sJrKLhLYyangsQhLt1fwV4BffwX hdcZW9jSq4MbqVyzTIs9DziQd4188JrwqLMcEp76dkpvCfdfjKPCvqRovfC2yhnT 7t/uNmkWEj3PfmGR8wJTWQ3yPKqaXFsfOgW0CRffOaff6DSzc5h827NcUybgDmXs U7G0Pq/SbE5TwODELuzL2wxHe2+QLNCdBcNf6XLjI3V8e++bAU7HRGZGvzxrEAbX ZItJt9cCx3I+b1TSTBoBZRQGfCeDDWlluRir+71h/XPHtSpB8IU= =ze6+ -----END PGP SIGNATURE----- --u3/rZRmxL6MmkK24-- --===============1657773721== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== --===============1657773721==--