From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53765) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4rxH-00026u-DG for qemu-devel@nongnu.org; Fri, 24 Aug 2012 07:21:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T4rxG-0003ub-Ep for qemu-devel@nongnu.org; Fri, 24 Aug 2012 07:21:35 -0400 Message-ID: <503763B1.1040205@siemens.com> Date: Fri, 24 Aug 2012 13:21:21 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <5034FB7D.5050009@siemens.com> <503508CE.1020908@weilnetz.de> <50350C58.8060306@siemens.com> <20120824112021.GI10867@stefanha-thinkpad.localdomain> In-Reply-To: <20120824112021.GI10867@stefanha-thinkpad.localdomain> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH v2] console: Correct computation of bytes per pixel from bits per pixel List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: "qemu-trivial@nongnu.org" , Stefan Weil , "qemu-devel@nongnu.org" , BALATON Zoltan On 2012-08-24 13:20, Stefan Hajnoczi wrote: > On Wed, Aug 22, 2012 at 06:44:08PM +0200, Jan Kiszka wrote: >> On 2012-08-22 18:29, Stefan Weil wrote: >>> Am 22.08.2012 17:32, schrieb Jan Kiszka: >>>> On 2012-08-22 17:19, BALATON Zoltan wrote: >>>>> Division with round up is the correct way to compute this even if the >>>>> only case where division with round down gives incorrect result is >>>>> probably 15 bpp. This case was explicitely patched up in one of these >>>>> functions but was unhandled in the other. (I'm not sure about setting >>>>> 16 bpp for the 15bpp case either but I left that there for now.) >>>>> >>>>> Signed-off-by: BALATON Zoltan >>>>> --- >>>>> console.c | 5 ++--- >>>>> 1 file changed, 2 insertions(+), 3 deletions(-) >>>>> >>>>> v2: Use DIV_ROUND_UP and extended commit message >>>>> >>>>> diff --git a/console.c b/console.c >>>>> index 4525cc7..9df1701 100644 >>>>> --- a/console.c >>>>> +++ b/console.c >>>>> @@ -1612,7 +1612,7 @@ PixelFormat >>>>> qemu_different_endianness_pixelformat(int bpp) >>>>> memset(&pf, 0x00, sizeof(PixelFormat)); >>>>> >>>>> pf.bits_per_pixel = bpp; >>>>> - pf.bytes_per_pixel = bpp / 8; >>>>> + pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8); >>>>> pf.depth = bpp == 32 ? 24 : bpp; >>>>> >>>>> switch (bpp) { >>>>> @@ -1661,13 +1661,12 @@ PixelFormat qemu_default_pixelformat(int bpp) >>>>> memset(&pf, 0x00, sizeof(PixelFormat)); >>>>> >>>>> pf.bits_per_pixel = bpp; >>>>> - pf.bytes_per_pixel = bpp / 8; >>>>> + pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8); >>>>> pf.depth = bpp == 32 ? 24 : bpp; >>>>> >>>>> switch (bpp) { >>>>> case 15: >>>>> pf.bits_per_pixel = 16; >>>>> - pf.bytes_per_pixel = 2; >>>> Removed unintentionally? >>>> >>>> Jan >>> >>> 15 bpp no longer needs special handling because >>> the computation above is fixed by the same patch, >>> so the removal is correct. >> >> Ok, actually starting to look at this code: This is apparently a code >> cleanup, not a bug fix like the subject suggests. No valid input for >> could have actually triggered the round-up issue. Please declare it as >> such. Bonus for abort() on the invalid default cases. > > I have updated the commit message to "console: Clean up bytes per pixel > calculation". I think there was a v3 on the list, didn't check the details yet, though. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux