From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x22b.google.com (mail-wm0-x22b.google.com [IPv6:2a00:1450:400c:c09::22b]) (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 2A21521A134AE for ; Thu, 4 May 2017 06:34:16 -0700 (PDT) Received: by mail-wm0-x22b.google.com with SMTP id m123so15945423wma.0 for ; Thu, 04 May 2017 06:34:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=EIFlSnr7gHMpM6Vah+yR2KOses2Jr4Cxqv0RyUnwPjo=; b=CfGbIR/QrxyIwY92srapwuAHygYzefd9ba6kyF/rvGORhHSk7ptN4NuC3FPX5/L67J nQx2VRhG+By6xBxR2TR4Puj7L6tcJYJgfOY7GgBiAqxn3eASqwOGcOsz1prc0CkweYYm 34nQcQORk7RKpZwRNOdUtC1qjQOAVNkx3PN10= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=EIFlSnr7gHMpM6Vah+yR2KOses2Jr4Cxqv0RyUnwPjo=; b=njz1vnstjscMYhBOzYTgh+FbuES9k9DcEgdNjJ5vlMGqBUfXHJPm//W2WvL0IeXcoE ppls3eE6stX7N3e62MTTH+2lbsRNvoXBu+i5qWD0m/B4XTYmxjaSNuR/m85eTuR4oupJ 1e1sAEg5C2J7WKoivxqrJ+Bb/jSTB93Ja+ZQaUA6LShVSJ3ym34r18AZnZGQGVmxfcdC zv7P95MJTGH27Lllx955bd0CzZIF1v1iHosITGdwv/+5w5P98dx5v6wzPQyjWWnA53ML MCecHTOHMMBmjWZ41lSPDyBs05qo2B8UlQ7Zfsde0om+mQu98EWd3fnEtzFatihVxoGn zpIg== X-Gm-Message-State: AODbwcDHst7IZck6TTxs393+g5B2y5MJAYPLcYyhwJhkrWSbvOE8y5RO Q6G6cEnIrcAT4TZk X-Received: by 10.28.31.16 with SMTP id f16mr1876662wmf.118.1493904853587; Thu, 04 May 2017 06:34:13 -0700 (PDT) Received: from [100.117.96.59] (102.16.90.92.rev.sfr.net. [92.90.16.102]) by smtp.gmail.com with ESMTPSA id 77sm1407148wmw.24.2017.05.04.06.34.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 04 May 2017 06:34:12 -0700 (PDT) Mime-Version: 1.0 (1.0) From: Ard Biesheuvel X-Mailer: iPhone Mail (14D27) In-Reply-To: <20170504111458.GU1657@bivouac.eciton.net> Date: Thu, 4 May 2017 14:34:09 +0100 Cc: Girish Pathak , edk2-devel , "ryan.harkin@linaro.org" , Evan Lloyd , Sami Mujawar , Alexei Fedorov Message-Id: <5D22060A-68B9-4632-A884-07220A737BA9@linaro.org> References: <20170504111458.GU1657@bivouac.eciton.net> To: Leif Lindholm Subject: Re: ArmPlatformPkg: LcdGraphicsOutputDxe, PL111, and HdLcd rejig X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 May 2017 13:34:16 -0000 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable > On 4 May 2017, at 12:14, Leif Lindholm wrote: >=20 > Hi Girish, >=20 >> On Wed, May 03, 2017 at 04:58:33PM +0000, Girish Pathak wrote: >> With a view to an upcoming change, we have been examining the current Gra= phics >> Output Protocol implementation. Currently for, ARM platforms, the UEFI Gr= aphics >> Output Protocol is implemented using a platform specific Library >> (PL111LcdArmVExpressLib/HdLcdArmVExpressLib) and a DXE driver >> (PL111LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe). The platform specific= >> library handles platform specific variations such as platform supported d= isplay >> modes, memory management of the frame buffer, and clock/mux setting. The D= XE >> driver implements the GOP protocol and manages the respective display IP.= >> Although this implementation works fine for current platforms, we think t= he way >> the current DXE driver sources are linked is sub-optimal and can be impro= ved to >> meet recommendation of the EDKII Module Writer's Guide. >=20 > The real improvement is the reduction in code duplication. >=20 Perhaps we shoukd move to the UEFI driver model at the same time? >> The DXE driver source contains three source files per driver. The files >> LcdGraphicsOutputDxe.c and LcdGraphicsOutputBlt.c implement common >> functionality whereas HdLcd.c/PL111Lcd.c implement display IP specific p= art of >> the DXE. The problem is, there are two .inf files for HdLcdGraphicsOutput= Dxe >> and PL111LcdGraphicsOutputDxe and both link common code LcdGraphicsOutput= Dxe.c >> and LcdGraphicsOutputBlt.c with display IP source instead of a library in= stance >> , which seems incorrect and can be improved. We propose to separate HdLcd= .c >> and PL111Lcd.c and create independent libraries managing only respective >> display IP which can then be instantiated as LcdHwLib and linked in a com= mon >> LcdGraphicsOutputDxe DXE driver. This will help to clearly partition >> implementation of the Graphics Output Protocol into three separate compon= ents, >> a platform specific component for the display IP, a display IP specific >> component and GOP common code. >=20 > This all sounds imminently sensible. One request for clarifiation below. >=20 >> So instead of the current structure in ArmPlatformPkg for display DXE: >>=20 >> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf = (HDLCD GOP DXE) >> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111LcdGraphicsOutputDxe.in= f (PL111 GOP DXE) >>=20 >> We propose a structure like: >>=20 >> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf (= GOP DXE independent of hardware/platform) >=20 > Are we expecting this one to be truly independent, or simply common > between most/all ARM Ltd. display controllers? >=20 >> ArmPlatformPkg/Drivers/HdLcd/HdLcd.inf (Library code managing HDLCD H= W IP) >> ArmPlatformPkg/Drivers/PL111/PL11Lcd.inf (Library code managing PL111= HW IP) >>=20 >> LcdGraphicsOutputDxe.inf will link to LcdPlatformLib and LcdHwLib which c= an be >> selected for the platform in the platform specific .dsc file. >>=20 >> e.g. >>=20 >> Under LibraryClasses we might have: >> LcdPlatformLib|ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressL= ib/HdLcdArmVExpressLib.inf >> LcdHwLib|ArmPlatformPkg/Drivers/HdLcd/HdLcd.inf >>=20 >> And Under Components: >> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf (= Common GOP DXE code for ARM Platforms) >>=20 >> This is a significant change and we would like to invite viewpoints befor= e we >> proceed with implementing these. Since the change would only be with resp= ect to >> display aspects of ArmPlatformPkg we don't foresee any impact on any othe= r >> functionality. >>=20 >> Please reply if you feel this intended change might impact you and why ? >> Unless objections are raised, we will soon submit the patches for review.= >=20 > Sounds good to me. >=20 > Regards, >=20 > Leif