From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=188.166.76.209; helo=mail.aheymans.xyz; envelope-from=arthur@aheymans.xyz; receiver=edk2-devel@lists.01.org Received: from mail.aheymans.xyz (mail.aheymans.xyz [188.166.76.209]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8951E2222C24C for ; Fri, 26 Jan 2018 01:03:17 -0800 (PST) Received: from x200_coreboot.localdomain (d8D86483E.access.telenet.be [141.134.72.62]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mail.aheymans.xyz (Postfix) with ESMTPSA id 79E54C200A; Fri, 26 Jan 2018 10:08:45 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=aheymans.xyz; s=mail; t=1516957725; bh=+dLuCW/DKvs9s23YDQiaPvIFpsa5TkLRDf4zqiE0784=; h=From:To:Cc:Subject:References:Date:In-Reply-To; b=Vtbaa51naHhnQjaczHwcVjpKOkFLsGpxSj2SWWQf9GJBLIVxtMgngEOcI6dQ4nbud USGkocd7hmkB4QdN86tgr1gyTwtBp8veME8n0pRBiCJtPtaNVXDIIU3H01zquu4HDr 4+9aYa+0OjKRziYx+i+y1I93676q0GdsWY1r4FCQ= Received: by x200_coreboot.localdomain (Postfix, from userid 1000) id 80BFB1024DE; Fri, 26 Jan 2018 10:08:44 +0100 (CET) From: Arthur Heymans To: "You\, Benjamin" Cc: "edk2-devel\@lists.01.org" References: <20180124105736.14877-1-arthur@aheymans.xyz> <87mv12gvjc.fsf@aheymans.xyz> Date: Fri, 26 Jan 2018 10:08:44 +0100 In-Reply-To: (Benjamin You's message of "Fri, 26 Jan 2018 08:40:14 +0000") Message-ID: <87efmdyokj.fsf@aheymans.xyz> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Subject: Re: [PATCH] CorebootPayloadPkg: Use correct BytesPerScanLine X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Jan 2018 09:03:19 -0000 Content-Type: text/plain "You, Benjamin" writes: > > I noticed in coreboot-4.7\src\include\edid.h, there are following comments: > > /* 3 variables needed for coreboot framebuffer. > * In most cases, they are the same as the ha > * and va variables, but not always, as in the > * case of a 1366 wide display. > */ > u32 x_resolution; > u32 y_resolution; > u32 bytes_per_line; > > And in coreboot-4.7\src\lib\edid.c: > > edid->bytes_per_line = ALIGN_UP(edid->mode.ha * > div_round_up(fb_bpp, 8), row_byte_alignment); > edid->x_resolution = edid->bytes_per_line / (fb_bpp / 8); > This is how x_resolution initially gets set after the EDID is read, but it is further modified to satisfy the display controllers needs, e.g. src/northbridge/intel/gm45/gma.c: edid->bytes_per_line = (edid->bytes_per_line + 63) & ~63; before it gets send to code that sets up the coreboot tables from which payloads extract this information: set_vbe_mode_info_valid(edid, lfb); There are also other code paths that don't use src/lib/edid.c to set up the framebuffer. In src/drivers/intel/gma/hires_fb/gma.adb we have: x_resolution => word32 (fb.Width), y_resolution => word32 (fb.Height), bytes_per_line => 4 * word32 (fb.Stride), > Above calculations derive x_resolution from the roundup value of > bytes_per_line. In case of 1366 display, it would produce a x_resolution of > 1376, which is larger than 1366 but satisfies the equation of > bytes_per_line == (HorizontalResolution * (BitsPerPixel / 8) > It is only initialized to that when the EDID is read. The code that actually sets up the hardware further modifies it or passes on the value it needs to bytes_per_line. > It appears this is what Coreboot produces right now. Not sure if there are > other cases leading to Coreboot producing framebuffer parameters NOT > satisfying the above equation. > well given that other code touches edid_bytes_per_lines, there are many examples where this is not satisfied. > BTW, do you think the above calculation of x_resolution hides the > information of display and should be fixed? > >> So tianocore should use the value coreboot provides it instead of trying >> to compute/guess it. >> >> > Thanks, >> > >> > - ben >> > >> >> I hope this clarifies it. >> >> Arthur >> >> >> -----Original Message----- >> >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of >> >> arthur@aheymans.xyz >> >> Sent: Wednesday, January 24, 2018 6:58 PM >> >> To: edk2-devel@lists.01.org >> >> Cc: Arthur Heymans >> >> Subject: [edk2] [PATCH] CorebootPayloadPkg: Use correct BytesPerScanLine >> >> >> >> From: Arthur Heymans >> >> >> >> Fetch BytesPerScanLine from coreboot table to reflect how the actual >> >> framebuffer is set up instead of guessing it from the horizontal >> >> resolution. >> >> >> >> This fixes a garbled display when HorizontalResolution * (BitsPerPixel >> >> / 8) and pFbInfo->BytesPerScanLine don't match. >> >> >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> >> Signed-off-by: Arthur Heymans >> >> >> >> diff --git a/CorebootPayloadPkg/FbGop/FbGop.c >> >> b/CorebootPayloadPkg/FbGop/FbGop.c >> >> index 37d6def7f7..6790617033 100644 >> >> --- a/CorebootPayloadPkg/FbGop/FbGop.c >> >> +++ b/CorebootPayloadPkg/FbGop/FbGop.c >> >> @@ -822,7 +822,7 @@ FbGopCheckForVbe ( >> >> BitsPerPixel = pFbInfo->BitsPerPixel; >> >> HorizontalResolution = pFbInfo->HorizontalResolution; >> >> VerticalResolution = pFbInfo->VerticalResolution; >> >> - BytesPerScanLine = HorizontalResolution * (BitsPerPixel / 8); >> >> + BytesPerScanLine = pFbInfo->BytesPerScanLine; >> >> >> >> ModeBuffer = (FB_VIDEO_MODE_DATA *) AllocatePool ( >> >> >> >> >> >> ModeNumber * sizeof >> >> (FB_VIDEO_MODE_DATA) >> >> -- >> >> 2.16.1 >> >> >> >> _______________________________________________ >> >> edk2-devel mailing list >> >> edk2-devel@lists.01.org >> >> https://lists.01.org/mailman/listinfo/edk2-devel >> <#secure method=pgpmime mode=sign> >> >> -- >> Arthur Heymans > > Thanks, > > - ben -- Arthur Heymans