Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   David Laight <David.Laight@ACULAB.COM>
To:     "'15330273260@189.cn'" <15330273260@189.cn>,
        Sui Jingfeng <suijingfeng@loongson.cn>,
        Li Yi <liyi@loongson.cn>
CC:     Thomas Zimmermann <tzimmermann@suse.de>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "loongson-kernel@lists.loongnix.cn" 
        <loongson-kernel@lists.loongnix.cn>
Subject: RE: [PATCH] drm/drm_vblank.c: avoid unsigned int to signed int cast
Thread-Topic: [PATCH] drm/drm_vblank.c: avoid unsigned int to signed int cast
Thread-Index: AQHZiBwkXHI4wBkIl0yfSK4Xtptrs69eSnTg
Date:   Wed, 17 May 2023 10:59:14 +0000
Message-ID: <f6bd362145124f34a1af800dd330f8e9@AcuMS.aculab.com>
References: <20230516173026.2990705-1-15330273260@189.cn>
In-Reply-To: <20230516173026.2990705-1-15330273260@189.cn>
Accept-Language: en-GB, en-US
MIME-Version: 1.0
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Precedence: bulk

From: 15330273260@189.cn
> Sent: 16 May 2023 18:30
> 
> From: Sui Jingfeng <suijingfeng@loongson.cn>
> 
> Both mode->crtc_htotal and mode->crtc_vtotal are u16 type,
> mode->crtc_htotal * mode->crtc_vtotal will results a unsigned type.

Nope, u16 gets promoted to 'signed int' and the result of the
multiply is also signed.

> Using a u32 is enough to store the result, but considering that the
> result will be casted to u64 soon after. We use a u64 type directly.
> So there no need to cast it to signed type and cast back then.
....
> -		int frame_size = mode->crtc_htotal * mode->crtc_vtotal;
> +		u64 frame_size = mode->crtc_htotal * mode->crtc_vtotal;
...
> -		framedur_ns = div_u64((u64) frame_size * 1000000, dotclock);
> +		framedur_ns = div_u64(frame_size * 1000000, dotclock);

The (u64) cast is there to extend the value to 64bits, not
because the original type is signed.
The compiler will detect that the old code is a 32x32 multiply
where a 64bit result is needed, that may not be true for the
changed code (it would need to track back as far as the u16s).

It is not uncommon to force a 64bit result from a multiply
by making the constant 64bit. As in:
	div_u64(frame_size * 1000000ULL, dotclock);

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)