From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::242; helo=mail-wm0-x242.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x242.google.com (mail-wm0-x242.google.com [IPv6:2a00:1450:400c:c09::242]) (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 B43E72215BD9A for ; Mon, 29 Jan 2018 08:23:23 -0800 (PST) Received: by mail-wm0-x242.google.com with SMTP id x4so45668579wmc.0 for ; Mon, 29 Jan 2018 08:28:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=G9Htd1neQaeckzFvScEjZzvZ1wpNmsVyTve72oWon9k=; b=WeF3bGwfDXpc6sOurMMoN4igcMOd5PMXfHcS5HKBMCRwMVAQY/cpEDjGe/azj2iI2+ YCF5tRP+mnTxLHRp0cf6bMNfYuSfgx9BGIPgeUfV73lPjT7pwFMK4iZtxlG10My57Ug1 5Rvq6NJKDn/uo4ZIcLlzYzT5A2VnND0VGmpJI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=G9Htd1neQaeckzFvScEjZzvZ1wpNmsVyTve72oWon9k=; b=e2b5KUFORyZNUTNK/xW5B5PemyNdpFfWhoCnAS/4jH12TIh6AdV0Y+vJaid8CcCxuB 7tcM9my4fS2JCXJzyMGDBc0l9mdwgcV6vgqAUKpkmF/l/BeoTKT6F+kIScq27flkS3rT mHmWcrveprzRgF0SYI0xz3exKiSNGTD+OuG3bAd45Rxjb4oiRKDk2W3N/3HdjpB5SZXI S0YuBHLZ1t7jfjT1f83kFeohVeShoHZ2mhN2uZCiLTDjCPS2YevexXARkYL6L7vdMXSh d9p29HW7amcYqpIBn6bwaGmLGY9+x3LZBIrpOlpijEfUwzEyAEiXvYDna3KYyXCNhqoY nVMw== X-Gm-Message-State: AKwxytfC6yUQGxOsrfJ4qeWLVHcFAgu7w7bz7lSjf2z0u9e5SvxfSPgW vBZwqA3jG1OoDp5kO3E6B+aet2wxuQQ= X-Google-Smtp-Source: AH8x2268gKwunLoXHEtjaWARK1R+UvRkQSPMg0/acPMH8aS+VKqlJdFb7apRS+w9zJ7IyyO1xZ8Z3w== X-Received: by 10.28.168.8 with SMTP id r8mr19982960wme.157.1517243335958; Mon, 29 Jan 2018 08:28:55 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id u43sm6069355wrf.63.2018.01.29.08.28.54 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 29 Jan 2018 08:28:54 -0800 (PST) Date: Mon, 29 Jan 2018 16:28:53 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: Thomas Abraham , "edk2-devel@lists.01.org" Message-ID: <20180129162853.n7rbndiihj4twio7@bivouac.eciton.net> References: <1516986299-1616-1-git-send-email-thomas.abraham@arm.com> <20180129145104.hwd4afi4l3kjf6an@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) 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:23:24 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Jan 29, 2018 at 04:13:40PM +0000, Ard Biesheuvel wrote: > 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 Thanks. Pushed as cf8aef9c96 with Reviewed-by: Leif Lindholm [Reworded adjacent code comments.] Signed-off-by: Leif Lindholm and Ard's ACK. / Leif