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::242; helo=mail-io0-x242.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x242.google.com (mail-io0-x242.google.com [IPv6:2607:f8b0:4001:c06::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 9529D2219BCAF for ; Sat, 23 Dec 2017 05:22:39 -0800 (PST) Received: by mail-io0-x242.google.com with SMTP id 14so20669471iou.2 for ; Sat, 23 Dec 2017 05:27:30 -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=lOSkBKdvhsLCAt3CgGwmFQmZj79GTat+vb6P3c9MK0s=; b=GpgFCaT9VW4ckGvRlsZ9D6hfnc7qElNH5VnapDD/uCBEzcdJKyWi8me9xzhKL8ky9z oeBL00YtRxNG8uVMV79a7fQBLkAe9Fd0wBk8yCQTzDKFncoOKd4ppcHHFUqrJ3oM0SLn 3SHTxFQHGUvuRe6A1psm3ZINjMepyXmQI91x4= 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=lOSkBKdvhsLCAt3CgGwmFQmZj79GTat+vb6P3c9MK0s=; b=HZ4sttrzo4TySlY1u8H8Zhp1/fFuwLVceXLxUp6qwcXAGhgj9V2/2j+PH4uyjJi90W UPSTiv6v4/LHmkZsIUiJMakKNvAWHySITJLmVNxw6Lbkz+IwP+mXGZgviihwKpLVARjY GdJZ1HSEdVuozVbPJvA8V4+LbdA7u2ExlnERBdgADxy3LdBydks0P1KaDfnFOf/Y3YEf polorJffbqTE/ALs60hUWUJbp02pDui+efO3htorEaAg3mC6U1mQnjHfBDQqun6x27rL b++z7G+1J0eFJo6PxB6rhv3f24fqkf4dZUJeygFCiX2Ow6EIq+BmTO9TJItP2I4bqRyg Pg6w== X-Gm-Message-State: AKGB3mLdKG/b9989ZQ2pT+KzmOgg9/HpdHfDrl/Jzz4/WPNOYCAnqhO2 qb/umDdgOHiqixSuG2nAiOsAjLImhTlfwIvB4ThH6qJq0nQ= X-Google-Smtp-Source: ACJfBouje3489tUix1TaITIOVE2WhMhQDcguh+Tpgq1uWIHtHde7hEeQs284vy3bCL9u/giBR7i1QuT4x1b3atomXB4= X-Received: by 10.107.27.84 with SMTP id b81mr21605073iob.43.1514035650025; Sat, 23 Dec 2017 05:27:30 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.52.14 with HTTP; Sat, 23 Dec 2017 05:27:29 -0800 (PST) In-Reply-To: <20171222183418.8616-8-evan.lloyd@arm.com> References: <20171222183418.8616-1-evan.lloyd@arm.com> <20171222183418.8616-8-evan.lloyd@arm.com> From: Ard Biesheuvel Date: Sat, 23 Dec 2017 13:27:29 +0000 Message-ID: To: Evan Lloyd Cc: "edk2-devel@lists.01.org" , <"ard.biesheuvel@linaro.org"@arm.com>, <"leif.lindholm@linaro.org"@arm.com>, <"Matteo.Carlini@arm.com"@arm.com>, <"nd@arm.com"@arm.com> Subject: Re: [PATCH v2 07/13] ArmPlatformPkg: Redefine LcdPlatformGetTimings function 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: Sat, 23 Dec 2017 13:22:40 -0000 Content-Type: text/plain; charset="UTF-8" On 22 December 2017 at 18:34, wrote: > From: Girish Pathak > > The LcdPlatformGetTimings interface function takes similar sets of > multiple parameters for horizontal and vertical timings which can be > aggregated in a common data type. This change defines a structure > SCAN_TIMINGS for this which can be used to describe both horizontal and > vertical scan timings, and accordingly redefines the > LcdPlatformGetTiming interface, greatly reducing the amount of data > passed about. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Girish Pathak > Signed-off-by: Evan Lloyd > --- > ArmPlatformPkg/Include/Library/LcdPlatformLib.h | 31 ++++++----- > ArmPlatformPkg/Library/HdLcd/HdLcd.c | 56 +++++++++----------- > ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c | 49 +++++++++-------- > 3 files changed, 68 insertions(+), 68 deletions(-) > > diff --git a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h > index 2a70307031fc21c8fb0d655d358ca9a102a95920..0943f28a19133e7bc558f9d529bb8ac8f66ba3fd 100644 > --- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h > +++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h > @@ -153,6 +153,14 @@ typedef enum { > LCD_BITS_PER_PIXEL_12_444 > } LCD_BPP; > > +// Display timing settings. > +typedef struct { > + UINT32 Resolution; > + UINT32 Sync; > + UINT32 BackPorch; > + UINT32 FrontPorch; > +} SCAN_TIMINGS; > + > /** Platform related initialization function. > > @param[in] Handle Handle to the LCD device instance. > @@ -228,14 +236,11 @@ LcdPlatformQueryMode ( > > @param[in] ModeNumber Mode Number. > > - @param[out] HRes Pointer to horizontal resolution. > - @param[out] HSync Pointer to horizontal sync width. > - @param[out] HBackPorch Pointer to horizontal back porch. > - @param[out] HFrontPorch Pointer to horizontal front porch. > - @param[out] VRes Pointer to vertical resolution. > - @param[out] VSync Pointer to vertical sync width. > - @param[out] VBackPorch Pointer to vertical back porch. > - @param[out] VFrontPorch Pointer to vertical front porch. > + @param[out] Horizontal Pointer to horizontal timing parameters. > + (Resolution, Sync, Back porch, Front porch) > + @param[out] Vertical Pointer to vertical timing parameters. > + (Resolution, Sync, Back porch, Front porch) > + > > @retval EFI_SUCCESS Display timing information for the requested > mode returned successfully. > @@ -244,14 +249,8 @@ LcdPlatformQueryMode ( > EFI_STATUS > LcdPlatformGetTimings ( > IN UINT32 ModeNumber, > - OUT UINT32* HRes, > - OUT UINT32* HSync, > - OUT UINT32* HBackPorch, > - OUT UINT32* HFrontPorch, > - OUT UINT32* VRes, > - OUT UINT32* VSync, > - OUT UINT32* VBackPorch, > - OUT UINT32* VFrontPorch > + OUT CONST SCAN_TIMINGS **Horizontal, > + OUT CONST SCAN_TIMINGS **Vertical > ); > > /** Return bits per pixel information for a mode number. > diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c b/ArmPlatformPkg/Library/HdLcd/HdLcd.c > index b2779af041fae58d712270002cc7d6d277360311..1fc04c2d14d8185370454be459a23bdec41f6602 100644 > --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c > +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c > @@ -98,35 +98,26 @@ LcdSetMode ( > IN CONST UINT32 ModeNumber > ) > { > - EFI_STATUS Status; > - UINT32 HRes; > - UINT32 HSync; > - UINT32 HBackPorch; > - UINT32 HFrontPorch; > - UINT32 VRes; > - UINT32 VSync; > - UINT32 VBackPorch; > - UINT32 VFrontPorch; > - UINT32 BytesPerPixel; > - LCD_BPP LcdBpp; > + EFI_STATUS Status; > + CONST SCAN_TIMINGS *Horizontal; > + CONST SCAN_TIMINGS *Vertical; Please align the * with the 'S' in Status. With that fixed, Reviewed-by: Ard Biesheuvel > + UINT32 BytesPerPixel; > + LCD_BPP LcdBpp; > > // Set the video mode timings and other relevant information > Status = LcdPlatformGetTimings ( > ModeNumber, > - &HRes, > - &HSync, > - &HBackPorch, > - &HFrontPorch, > - &VRes, > - &VSync, > - &VBackPorch, > - &VFrontPorch > + &Horizontal, > + &Vertical > ); > if (EFI_ERROR (Status)) { > ASSERT (FALSE); > return Status; > } > > + ASSERT (Horizontal != NULL); > + ASSERT (Vertical != NULL); > + > Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp); > if (EFI_ERROR (Status)) { > ASSERT (FALSE); > @@ -139,21 +130,26 @@ LcdSetMode ( > MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_DISABLE); > > // Update the frame buffer information with the new settings > - MmioWrite32 (HDLCD_REG_FB_LINE_LENGTH, HRes * BytesPerPixel); > - MmioWrite32 (HDLCD_REG_FB_LINE_PITCH, HRes * BytesPerPixel); > - MmioWrite32 (HDLCD_REG_FB_LINE_COUNT, VRes - 1); > + MmioWrite32 ( > + HDLCD_REG_FB_LINE_LENGTH, > + Horizontal->Resolution * BytesPerPixel > + ); > + > + MmioWrite32 (HDLCD_REG_FB_LINE_PITCH, Horizontal->Resolution * BytesPerPixel); > + > + MmioWrite32 (HDLCD_REG_FB_LINE_COUNT, Vertical->Resolution - 1); > > // Set the vertical timing information > - MmioWrite32 (HDLCD_REG_V_SYNC, VSync); > - MmioWrite32 (HDLCD_REG_V_BACK_PORCH, VBackPorch); > - MmioWrite32 (HDLCD_REG_V_DATA, VRes - 1); > - MmioWrite32 (HDLCD_REG_V_FRONT_PORCH, VFrontPorch); > + MmioWrite32 (HDLCD_REG_V_SYNC, Vertical->Sync); > + MmioWrite32 (HDLCD_REG_V_BACK_PORCH, Vertical->BackPorch); > + MmioWrite32 (HDLCD_REG_V_DATA, Vertical->Resolution - 1); > + MmioWrite32 (HDLCD_REG_V_FRONT_PORCH, Vertical->FrontPorch); > > // Set the horizontal timing information > - MmioWrite32 (HDLCD_REG_H_SYNC, HSync); > - MmioWrite32 (HDLCD_REG_H_BACK_PORCH, HBackPorch); > - MmioWrite32 (HDLCD_REG_H_DATA, HRes - 1); > - MmioWrite32 (HDLCD_REG_H_FRONT_PORCH, HFrontPorch); > + MmioWrite32 (HDLCD_REG_H_SYNC, Horizontal->Sync); > + MmioWrite32 (HDLCD_REG_H_BACK_PORCH, Horizontal->BackPorch); > + MmioWrite32 (HDLCD_REG_H_DATA, Horizontal->Resolution - 1); > + MmioWrite32 (HDLCD_REG_H_FRONT_PORCH, Horizontal->FrontPorch); > > // Enable the controller > MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_ENABLE); > diff --git a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c > index b236cbfcfffadd6fa7610dcae5c5bc748aac0f69..d1aba083fdc2ee4a7c25294955c4413465dca1e8 100644 > --- a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c > +++ b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c > @@ -81,35 +81,26 @@ LcdSetMode ( > IN CONST UINT32 ModeNumber > ) > { > - EFI_STATUS Status; > - UINT32 HRes; > - UINT32 HSync; > - UINT32 HBackPorch; > - UINT32 HFrontPorch; > - UINT32 VRes; > - UINT32 VSync; > - UINT32 VBackPorch; > - UINT32 VFrontPorch; > - UINT32 LcdControl; > - LCD_BPP LcdBpp; > + EFI_STATUS Status; > + CONST SCAN_TIMINGS *Horizontal; > + CONST SCAN_TIMINGS *Vertical; > + UINT32 LcdControl; > + LCD_BPP LcdBpp; > > // Set the video mode timings and other relevant information > Status = LcdPlatformGetTimings ( > ModeNumber, > - &HRes, > - &HSync, > - &HBackPorch, > - &HFrontPorch, > - &VRes, > - &VSync, > - &VBackPorch, > - &VFrontPorch > + &Horizontal, > + &Vertical > ); > if (EFI_ERROR (Status)) { > ASSERT (FALSE); > return Status; > } > > + ASSERT (Horizontal != NULL); > + ASSERT (Vertical != NULL); > + > Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp); > if (EFI_ERROR (Status)) { > ASSERT (FALSE); > @@ -122,15 +113,29 @@ LcdSetMode ( > // Set Timings > MmioWrite32 ( > PL111_REG_LCD_TIMING_0, > - HOR_AXIS_PANEL (HBackPorch, HFrontPorch, HSync, HRes) > + HOR_AXIS_PANEL ( > + Horizontal->BackPorch, > + Horizontal->FrontPorch, > + Horizontal->Sync, > + Horizontal->Resolution > + ) > ); > > MmioWrite32 ( > PL111_REG_LCD_TIMING_1, > - VER_AXIS_PANEL (VBackPorch, VFrontPorch, VSync, VRes) > + VER_AXIS_PANEL ( > + Vertical->BackPorch, > + Vertical->FrontPorch, > + Vertical->Sync, > + Vertical->Resolution > + ) > + ); > + > + MmioWrite32 ( > + PL111_REG_LCD_TIMING_2, > + CLK_SIG_POLARITY (Horizontal->Resolution) > ); > > - MmioWrite32 (PL111_REG_LCD_TIMING_2, CLK_SIG_POLARITY (HRes)); > MmioWrite32 (PL111_REG_LCD_TIMING_3, 0); > > // PL111_REG_LCD_CONTROL > -- > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") >