From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::243; helo=mail-io0-x243.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x243.google.com (mail-io0-x243.google.com [IPv6:2607:f8b0:4001:c06::243]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 69B232215BDBA for ; Mon, 29 Jan 2018 08:08:07 -0800 (PST) Received: by mail-io0-x243.google.com with SMTP id l17so8169428ioc.3 for ; Mon, 29 Jan 2018 08:13:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Po7/F19gvWAmdp+VOfe2JwDUhHJkYPW8ckxXsInmfno=; b=R9z8Qhd7beLRu7InbriUutfLTfB5gBAgVb4lbxT3aOh+R2eX0OVL166+iGbMx4vQK+ jwDO2CYNzEQmnQJC/aE+Tl04TJD+J8nCdPlJqQF85SrgfTqwoYyMd38o6NK5w7GEoOqm j5gCI1JZVcj8SX/6kuYbRCZeDQLkCEdgial+I= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Po7/F19gvWAmdp+VOfe2JwDUhHJkYPW8ckxXsInmfno=; b=C3r82JJSNqxHmm1pxmKnsdsO1bAZqcIxNvE1r8/KP2tNxjRfybIsVdo2wd7b5Q1ZXZ Ejg2KyztXMNvn8cHXa4xZzhSxqZ1rkJuvaVz6MqUSw/U8PCittqb6+htsN5Iowcc2C6n rGNr/sQMTfj/1z4zgHODLsrU0oZxusqMlIUYFgC0ZWRZap/TGmUrZNJocWMNYqcLA0y4 HWb08BHQRbA/37AQ0E6+n6XPFZ6Kbix+D/32+5OfIAH8OTygchK2qmJoithMxFGEoXgS 2hWmmMJDG8dwBhqPG8goGM/3TN3NtthAphxiIVmsb4eSHeycE53uf8QeUIjRzWuPpE91 lSWQ== X-Gm-Message-State: AKwxytcAHAtF91TKpV4aWe1PXmlSdawqlP6OLYg+rBj/wM8SJITPyj4Z Fis95jCmdZ/YKUki7Is9MRmWb7QW4NHCzoBbP7IhAQ== X-Google-Smtp-Source: AH8x227fVkTxfonxuxx0OnNxvzrktUZrv1+Wr/+wxzrJ3X/aSqKiUZRUDKmKS5aYFKdsxFSaQtPnSQcoQvaYW7TFJVI= X-Received: by 10.107.33.65 with SMTP id h62mr25807758ioh.104.1517242420624; Mon, 29 Jan 2018 08:13:40 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.112.13 with HTTP; Mon, 29 Jan 2018 08:13:40 -0800 (PST) In-Reply-To: References: <1516986299-1616-1-git-send-email-thomas.abraham@arm.com> <20180129145104.hwd4afi4l3kjf6an@bivouac.eciton.net> From: Ard Biesheuvel Date: Mon, 29 Jan 2018 16:13:40 +0000 Message-ID: To: Thomas Abraham Cc: Leif Lindholm , "edk2-devel@lists.01.org" Subject: Re: [PATCH] Platform/ARM/VExpress: refine the check for DVI support 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: Mon, 29 Jan 2018 16:08:07 -0000 Content-Type: text/plain; charset="UTF-8" On 29 January 2018 at 16:01, Thomas Abraham wrote: > Hi Leif, > >> On Mon, Jan 29, 2018 at 8:21 PM, Leif Lindholm wrote: >> On Fri, Jan 26, 2018 at 10:34:59PM +0530, Thomas Abraham wrote: >> > The base models could have different values for the revision ID field >> > in the System ID register. Base models do not have support for DVI and >> > so the revision ID field should also be masked out when checking for >> > the presence of DVI support. >> > >> > Contributed-under: TianoCore Contribution Agreement 1.1 >> > Signed-off-by: Thomas Abraham >> > --- >> > .../VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | >> 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git >> > >> a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmV >> > Express.c >> > >> b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmV >> > Express.c >> > index 3f3ceb3..89ba130 100644 >> > --- >> > >> a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmV >> > Express.c >> > +++ >> b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111Lcd >> > +++ ArmVExpress.c >> > @@ -264,7 +264,7 @@ LcdPlatformSetMode ( >> > SysId = MmioRead32 (ARM_VE_SYS_ID_REG); >> > if (SysId != ARM_RTSM_SYS_ID) { >> > // Take out the FVP GIC variant to reduce the permutations. >> > - SysId &= ~ARM_FVP_SYS_ID_VARIANT_MASK; >> > + SysId &= ~(ARM_FVP_SYS_ID_VARIANT_MASK | >> > + ARM_FVP_SYS_ID_REV_MASK); >> > if (SysId != ARM_FVP_BASE_BOARD_SYS_ID) { >> > // Set the DVI into the new mode >> > Status = ArmPlatformSysConfigSet (SYS_CFG_DVIMODE, >> > mResolutions[ModeNumber].Mode); >> >> I think this change makes sense (and should arguably have been there from the >> start). But I also think it highlights that the surrounding comments are incorrect, >> and get increasingly confusing with this modification. >> >> I would suggest that >> // On the FVP models the GIC variant in encoded in bits [15:12]. >> is replaced by >> // On the FVP models, the build variant is encoded in bits [15:12]. >> >> and that >> // Take out the FVP GIC variant to reduce the permutations. >> is replaced by >> // Ignore build variant and revision. >> >> If you're happy with that, I can fold these changes to the surrounding comments >> in and commit. > > Yes, happy to have these changes folded in. Thank you!. > Likewise Acked-by: Ard Biesheuvel