From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x22e.google.com (mail-wm0-x22e.google.com [IPv6:2a00:1450:400c:c09::22e]) (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 AC84521A13482 for ; Thu, 4 May 2017 04:15:02 -0700 (PDT) Received: by mail-wm0-x22e.google.com with SMTP id w64so14987289wma.0 for ; Thu, 04 May 2017 04:15:02 -0700 (PDT) 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=YVQyYe27q+6Lm9ZxZY3lD9B+0ctUXOgVLa88RRQxoAw=; b=W89Ts45gY8PCUVvIgg0AOhbpXrIxzcq3dW1uF3OMDYVegc64DJBoVclAimBk0TMC0v MbfC9mQ832D0NekFHaefmYaJdqWVunvlL149CAj/OYE7FvKF5nXYSmABtYF8Xu/rtMvR 6UPwEHvA16/G33OIWoMmzVi8nQfUwdWkc+AX4= 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=YVQyYe27q+6Lm9ZxZY3lD9B+0ctUXOgVLa88RRQxoAw=; b=rLicnf2FdgVyMI+pj6URVipfV2A1yHY9nOXNptes6wAxsYjdoF/t3W2W1NytH+RIZG F3SvpoevVzn2YsayGDJ8z6uj/qduGVWNucqX5QfyUL4J0h/PDDBB3PhlFL2W2cmaOzdE PWVe/4ql+F+90KS/Ery4J4Uk1NpUcLNznOWN0sz+suyksGbyERs293HYaJM7AOOeGGpX ezSxewMZfpUm7CQLU/lWiz49NpBvb0goXjndiKx7bGdng5r1H5Vr+Ecxatpgu+yLR7TL i3sBuajHkM087q8gKx3d7WmfH6uiOP+WgFTDZ4Bf/DeU5PFRk1H8g4P3nx/46bOkIGvi VoBw== X-Gm-Message-State: AN3rC/4ziPwdH0qFvL7YmzKXqtxJ6ibe/N77M6MSPeyQuwOAqCY8rfxH jPJd6ep60spLcOr7 X-Received: by 10.28.4.216 with SMTP id 207mr1477460wme.142.1493896501091; Thu, 04 May 2017 04:15:01 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id z84sm1274115wmh.27.2017.05.04.04.15.00 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 04 May 2017 04:15:00 -0700 (PDT) Date: Thu, 4 May 2017 12:14:58 +0100 From: Leif Lindholm To: Girish Pathak Cc: edk2-devel , "ard.biesheuvel@linaro.org" , "ryan.harkin@linaro.org" , Evan Lloyd , Sami Mujawar , Alexei Fedorov Message-ID: <20170504111458.GU1657@bivouac.eciton.net> References: MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) 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 11:15:03 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Girish, 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 Graphics > Output Protocol implementation. Currently for, ARM platforms, the UEFI Graphics > 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 display > modes, memory management of the frame buffer, and clock/mux setting. The DXE > driver implements the GOP protocol and manages the respective display IP. > Although this implementation works fine for current platforms, we think the way > the current DXE driver sources are linked is sub-optimal and can be improved to > meet recommendation of the EDKII Module Writer's Guide. The real improvement is the reduction in code duplication. > 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 part of > the DXE. The problem is, there are two .inf files for HdLcdGraphicsOutputDxe > and PL111LcdGraphicsOutputDxe and both link common code LcdGraphicsOutputDxe.c > and LcdGraphicsOutputBlt.c with display IP source instead of a library instance > , 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 common > LcdGraphicsOutputDxe DXE driver. This will help to clearly partition > implementation of the Graphics Output Protocol into three separate components, > a platform specific component for the display IP, a display IP specific > component and GOP common code. This all sounds imminently sensible. One request for clarifiation below. > So instead of the current structure in ArmPlatformPkg for display DXE: > > ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf (HDLCD GOP DXE) > ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111LcdGraphicsOutputDxe.inf (PL111 GOP DXE) > > We propose a structure like: > > ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf (GOP DXE independent of hardware/platform) Are we expecting this one to be truly independent, or simply common between most/all ARM Ltd. display controllers? > ArmPlatformPkg/Drivers/HdLcd/HdLcd.inf (Library code managing HDLCD HW IP) > ArmPlatformPkg/Drivers/PL111/PL11Lcd.inf (Library code managing PL111 HW IP) > > LcdGraphicsOutputDxe.inf will link to LcdPlatformLib and LcdHwLib which can be > selected for the platform in the platform specific .dsc file. > > e.g. > > Under LibraryClasses we might have: > LcdPlatformLib|ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf > LcdHwLib|ArmPlatformPkg/Drivers/HdLcd/HdLcd.inf > > And Under Components: > ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf (Common GOP DXE code for ARM Platforms) > > This is a significant change and we would like to invite viewpoints before we > proceed with implementing these. Since the change would only be with respect to > display aspects of ArmPlatformPkg we don't foresee any impact on any other > functionality. > > 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. Sounds good to me. Regards, Leif