From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 04800C4727C for ; Tue, 29 Sep 2020 16:50:27 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 19BCA206F7 for ; Tue, 29 Sep 2020 16:50:25 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="BVXaKGm/"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="GAhWRQlc" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 19BCA206F7 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=LB/yZZeh2uLLMD1u/3HJYVnyy3e9VQsDomfnRkiiABc=; b=BVXaKGm/up3KoVaeluJ3JedxL 4iTAI6ZTFu6Lrv0zfMPztCGKJT6gTHxQe708jgs6igeg0r3JRo1d3GcSxT/c35Z4P/JYJw/HdkCWI aEafcouoNq3v0ApXD0UhH4mjVKHtrpipik/tfy43Kgh/+2p431iLGHWysp+Z+5I4H70i6yrwx3K71 sNInN8H6Qek/FiTADC8GrbPVwb1w/w7FJ/sofXanL7tuYZGIaZLs1nAso8eVkFh5ZxtWDS/JOb3VS g5BQORrpkgj6snSK0iUMnrPanktlm+7ZFMUAamPf0qS0VIvl+4yeKUqMswyHCd8ffEKSHA02ON39D ZOXwFd8Tg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kNInt-0004za-AS; Tue, 29 Sep 2020 16:48:37 +0000 Received: from mail-wm1-x344.google.com ([2a00:1450:4864:20::344]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kNInq-0004z3-7A for linux-arm-kernel@lists.infradead.org; Tue, 29 Sep 2020 16:48:36 +0000 Received: by mail-wm1-x344.google.com with SMTP id q9so5325968wmj.2 for ; Tue, 29 Sep 2020 09:48:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=njCSwAE8t0kUCLXks/LaqxI4SHX0eCoDaeYurLzOfIw=; b=GAhWRQlchfqCPk1nSK0dojYeLDlgVC1Do/n9jvqynpUI4YjFiQ/36o52bABLyDZBzD F2gz33K3/VphsaxGxILTx3p22lUafKh55abiYVZIY6larVWsnbDK53Aipmpi3v1xWNmw aU2u2mEspZ9LiTPe8A0+pwUMJc5+S83nffotM= 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:content-transfer-encoding :in-reply-to; bh=njCSwAE8t0kUCLXks/LaqxI4SHX0eCoDaeYurLzOfIw=; b=IQRVTmivFqxGcCu8zbRs1F7Jg4p2U03/RY37ehvxatIQAivqcB1KYf2IbLCFBSNYWt +gqJtr+BD/7sCimZHqFs5fO+YHfBXdIKCLNLMUqtxw3Vk1//N06ltSCJ8MxfZtI5eNK7 WfL7XH9ajlHrqQjYQEyUloX0A6Z0bQusxZKb+i330xJ1C+Icx8ZWQw4epnW0/O9yurYI uYfogIIpYo3YvNnC3flZMLeQkeY7iEQWhSbhkN9uWcI7qXU+NCl+cGe+sSvU9cGF+lsZ +BwedeLRpDVFooxmC5YgpEjMy8tMxW1JugX6j73vzbJBrY9E0Ti0QR+Q0hQNROBVLifJ lkrw== X-Gm-Message-State: AOAM532ni5Lr90W9Y3nS4CtGOjPl6+2in0PFlbm41v/W1ixQ/jJmXNkH dB6xgX6EBCs/ZRfPg6NigpJuBw== X-Google-Smtp-Source: ABdhPJzhyqW0H+WvK4kshuFov1K7YMIRBOxzWDXPBPdbNnUTXLOXQLygMbN1WelRphOypVsc2pEacw== X-Received: by 2002:a1c:f415:: with SMTP id z21mr5514940wma.88.1601398111778; Tue, 29 Sep 2020 09:48:31 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:57f4:0:efd0:b9e5:5ae6:c2fa]) by smtp.gmail.com with ESMTPSA id q15sm6638363wrr.8.2020.09.29.09.48.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Sep 2020 09:48:30 -0700 (PDT) Date: Tue, 29 Sep 2020 18:48:28 +0200 From: Daniel Vetter To: Peter Collingbourne Subject: Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver" Message-ID: <20200929164828.GP438822@phenom.ffwll.local> References: <20200928200856.1897023-1-pcc@google.com> <0dc9f36b-417d-bfad-9eb7-858c3041ff0c@baylibre.com> <20200929093201.GL438822@phenom.ffwll.local> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Operating-System: Linux phenom 5.7.0-1-amd64 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200929_124834_508504_7186302B X-CRM114-Status: GOOD ( 44.87 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Neil Armstrong , Linus Walleij , Kevin Brodsky , Russell King , ML dri-devel , Eric Anholt , Daniel Vetter , Sean Paul , Linux ARM Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > Hi, > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > Also revert the follow-up change "drm: pl111: Absorb the external > > > > register header". > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > The fbdev driver is used by Android's FVP configuration. Using the > > > > DRM driver together with DRM's fbdev emulation results in a failure > > > > to boot Android. The root cause is that Android's generic fbdev > > > > userspace driver relies on the ability to set the pixel format via > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > > > Can't Android FVP use drm-hwcomposer instead ? > = > Not without kernel changes. See e.g. > https://www.spinics.net/lists/dri-devel/msg255883.html That discussion seems to have died down with no further action. I also was kinda under the assumption that with Android these buffers would be allocated directly from dma-buf heaps/ion, so this all seems not that well baked out. > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > emulation, then let's do that. Not keep fbdev drivers on life support f= or > > longer than necessary. > = > That should have been done *before* removing the old driver, which was > a userspace break that was introduced in 5.9rc1. We shouldn't leave > userspace broken for the period of time that it would take to develop > that change. Even if such a change were developed before 5.9 is > released, it probably wouldn't be considered a bug fix that would be > eligible for 5.9. > = > > > > > > > > Neil > > > > > > > > > > > There have been other less critical behavioral differences identifi= ed > > > > between the fbdev driver and the DRM driver with fbdev emulation. T= he > > > > DRM driver exposes different values for the panel's width, height a= nd > > > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > > > > with yres_virtual greater than the maximum supported value instead > > > > of letting the syscall succeed and setting yres_virtual based on yr= es. > > > > Also something that should be fixed I think in upstream, in the drm fbd= ev > > emulation. At least doesn't sound like it's something unfixable. > = > Again, it should have been fixed before removing the old driver. Yeah, but we also don't have a whole lot people who seem to push for this. So the only way to find the people who still care is to remove the fbdev drivers. fbdev is dead. I'm totally fine reverting the driver to shut up the regression report, but it's not fixing anything long term. There's not going to be new drivers, or new hw support in existing drivers in fbdev. Also note that there's not spec nor test suite for fbdev, so "you guys should have known before removing drivers" doesn't work. -Daniel > = > Peter > = > > -Daniel > > > > > > > > > > Signed-off-by: Peter Collingbourne > > > > --- > > > > View this change in Gerrit: https://linux-review.googlesource.com/q= /I2d7e59b0e693d9fec206d40df190c5aa02844b56 > > > > > > > > MAINTAINERS | 5 + > > > > drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + > > > > drivers/gpu/drm/pl111/pl111_display.c | 1 + > > > > drivers/gpu/drm/pl111/pl111_drm.h | 73 -- > > > > drivers/gpu/drm/pl111/pl111_drv.c | 1 + > > > > drivers/gpu/drm/pl111/pl111_versatile.c | 1 + > > > > drivers/video/fbdev/Kconfig | 20 + > > > > drivers/video/fbdev/Makefile | 1 + > > > > drivers/video/fbdev/amba-clcd.c | 986 ++++++++++++++++++++= ++++ > > > > include/linux/amba/clcd-regs.h | 87 +++ > > > > include/linux/amba/clcd.h | 290 +++++++ > > > > 11 files changed, 1393 insertions(+), 73 deletions(-) > > > > create mode 100644 drivers/video/fbdev/amba-clcd.c > > > > create mode 100644 include/linux/amba/clcd-regs.h > > > > create mode 100644 include/linux/amba/clcd.h > > > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > index 190c7fa2ea01..671c1fa79e64 100644 > > > > --- a/MAINTAINERS > > > > +++ b/MAINTAINERS > > > > @@ -1460,6 +1460,11 @@ S: Odd Fixes > > > > F: drivers/amba/ > > > > F: include/linux/amba/bus.h > > > > > > > > +ARM PRIMECELL CLCD PL110 DRIVER > > > > +M: Russell King > > > > +S: Odd Fixes > > > > +F: drivers/video/fbdev/amba-clcd.* > > > > + > > > > ARM PRIMECELL KMI PL050 DRIVER > > > > M: Russell King > > > > S: Odd Fixes > > > > diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c b/drivers/gpu/dr= m/pl111/pl111_debugfs.c > > > > index 317f68abf18b..26ca8cdf3e60 100644 > > > > --- a/drivers/gpu/drm/pl111/pl111_debugfs.c > > > > +++ b/drivers/gpu/drm/pl111/pl111_debugfs.c > > > > @@ -3,6 +3,7 @@ > > > > * Copyright =A9 2017 Broadcom > > > > */ > > > > > > > > +#include > > > > #include > > > > > > > > #include > > > > diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/dr= m/pl111/pl111_display.c > > > > index b3e8697cafcf..703ddc803c55 100644 > > > > --- a/drivers/gpu/drm/pl111/pl111_display.c > > > > +++ b/drivers/gpu/drm/pl111/pl111_display.c > > > > @@ -9,6 +9,7 @@ > > > > * Copyright (C) 2011 Texas Instruments > > > > */ > > > > > > > > +#include > > > > #include > > > > #include > > > > #include > > > > diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl= 111/pl111_drm.h > > > > index 2a46b5bd8576..ba399bcb792f 100644 > > > > --- a/drivers/gpu/drm/pl111/pl111_drm.h > > > > +++ b/drivers/gpu/drm/pl111/pl111_drm.h > > > > @@ -23,79 +23,6 @@ > > > > #include > > > > #include > > > > > > > > -/* > > > > - * CLCD Controller Internal Register addresses > > > > - */ > > > > -#define CLCD_TIM0 0x00000000 > > > > -#define CLCD_TIM1 0x00000004 > > > > -#define CLCD_TIM2 0x00000008 > > > > -#define CLCD_TIM3 0x0000000c > > > > -#define CLCD_UBAS 0x00000010 > > > > -#define CLCD_LBAS 0x00000014 > > > > - > > > > -#define CLCD_PL110_IENB 0x00000018 > > > > -#define CLCD_PL110_CNTL 0x0000001c > > > > -#define CLCD_PL110_STAT 0x00000020 > > > > -#define CLCD_PL110_INTR 0x00000024 > > > > -#define CLCD_PL110_UCUR 0x00000028 > > > > -#define CLCD_PL110_LCUR 0x0000002C > > > > - > > > > -#define CLCD_PL111_CNTL 0x00000018 > > > > -#define CLCD_PL111_IENB 0x0000001c > > > > -#define CLCD_PL111_RIS 0x00000020 > > > > -#define CLCD_PL111_MIS 0x00000024 > > > > -#define CLCD_PL111_ICR 0x00000028 > > > > -#define CLCD_PL111_UCUR 0x0000002c > > > > -#define CLCD_PL111_LCUR 0x00000030 > > > > - > > > > -#define CLCD_PALL 0x00000200 > > > > -#define CLCD_PALETTE 0x00000200 > > > > - > > > > -#define TIM2_PCD_LO_MASK GENMASK(4, 0) > > > > -#define TIM2_PCD_LO_BITS 5 > > > > -#define TIM2_CLKSEL (1 << 5) > > > > -#define TIM2_ACB_MASK GENMASK(10, 6) > > > > -#define TIM2_IVS (1 << 11) > > > > -#define TIM2_IHS (1 << 12) > > > > -#define TIM2_IPC (1 << 13) > > > > -#define TIM2_IOE (1 << 14) > > > > -#define TIM2_BCD (1 << 26) > > > > -#define TIM2_PCD_HI_MASK GENMASK(31, 27) > > > > -#define TIM2_PCD_HI_BITS 5 > > > > -#define TIM2_PCD_HI_SHIFT 27 > > > > - > > > > -#define CNTL_LCDEN (1 << 0) > > > > -#define CNTL_LCDBPP1 (0 << 1) > > > > -#define CNTL_LCDBPP2 (1 << 1) > > > > -#define CNTL_LCDBPP4 (2 << 1) > > > > -#define CNTL_LCDBPP8 (3 << 1) > > > > -#define CNTL_LCDBPP16 (4 << 1) > > > > -#define CNTL_LCDBPP16_565 (6 << 1) > > > > -#define CNTL_LCDBPP16_444 (7 << 1) > > > > -#define CNTL_LCDBPP24 (5 << 1) > > > > -#define CNTL_LCDBW (1 << 4) > > > > -#define CNTL_LCDTFT (1 << 5) > > > > -#define CNTL_LCDMONO8 (1 << 6) > > > > -#define CNTL_LCDDUAL (1 << 7) > > > > -#define CNTL_BGR (1 << 8) > > > > -#define CNTL_BEBO (1 << 9) > > > > -#define CNTL_BEPO (1 << 10) > > > > -#define CNTL_LCDPWR (1 << 11) > > > > -#define CNTL_LCDVCOMP(x) ((x) << 12) > > > > -#define CNTL_LDMAFIFOTIME (1 << 15) > > > > -#define CNTL_WATERMARK (1 << 16) > > > > - > > > > -/* ST Microelectronics variant bits */ > > > > -#define CNTL_ST_1XBPP_444 0x0 > > > > -#define CNTL_ST_1XBPP_5551 (1 << 17) > > > > -#define CNTL_ST_1XBPP_565 (1 << 18) > > > > -#define CNTL_ST_CDWID_12 0x0 > > > > -#define CNTL_ST_CDWID_16 (1 << 19) > > > > -#define CNTL_ST_CDWID_18 (1 << 20) > > > > -#define CNTL_ST_CDWID_24 ((1 << 19) | (1 << 20)) > > > > -#define CNTL_ST_CEAEN (1 << 21) > > > > -#define CNTL_ST_LCDBPP24_PACKED (6 << 1) > > > > - > > > > #define CLCD_IRQ_NEXTBASE_UPDATE BIT(2) > > > > > > > > struct drm_minor; > > > > diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl= 111/pl111_drv.c > > > > index 96e58fda75d8..da0c39dae874 100644 > > > > --- a/drivers/gpu/drm/pl111/pl111_drv.c > > > > +++ b/drivers/gpu/drm/pl111/pl111_drv.c > > > > @@ -47,6 +47,7 @@ > > > > */ > > > > > > > > #include > > > > +#include > > > > #include > > > > #include > > > > #include > > > > diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/= drm/pl111/pl111_versatile.c > > > > index bdd883f4f0da..1127082e9a60 100644 > > > > --- a/drivers/gpu/drm/pl111/pl111_versatile.c > > > > +++ b/drivers/gpu/drm/pl111/pl111_versatile.c > > > > @@ -9,6 +9,7 @@ > > > > * Major contributions and discoveries by Russell King. > > > > */ > > > > > > > > +#include > > > > #include > > > > #include > > > > #include > > > > diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kcon= fig > > > > index b2c9dd4f0cb5..402e85450bb5 100644 > > > > --- a/drivers/video/fbdev/Kconfig > > > > +++ b/drivers/video/fbdev/Kconfig > > > > @@ -272,6 +272,26 @@ config FB_PM2_FIFO_DISCONNECT > > > > help > > > > Support the Permedia2 FIFO disconnect feature. > > > > > > > > +config FB_ARMCLCD > > > > + tristate "ARM PrimeCell PL110 support" > > > > + depends on ARM || ARM64 || COMPILE_TEST > > > > + depends on FB && ARM_AMBA && HAS_IOMEM > > > > + select FB_CFB_FILLRECT > > > > + select FB_CFB_COPYAREA > > > > + select FB_CFB_IMAGEBLIT > > > > + select FB_MODE_HELPERS if OF > > > > + select VIDEOMODE_HELPERS if OF > > > > + select BACKLIGHT_CLASS_DEVICE if OF > > > > + help > > > > + This framebuffer device driver is for the ARM PrimeCell PL110 > > > > + Colour LCD controller. ARM PrimeCells provide the building > > > > + blocks for System on a Chip devices. > > > > + > > > > + If you want to compile this as a module (=3Dcode which can be > > > > + inserted into and removed from the running kernel), say M > > > > + here and read . The m= odule > > > > + will be called amba-clcd. > > > > + > > > > config FB_ACORN > > > > bool "Acorn VIDC support" > > > > depends on (FB =3D y) && ARM && ARCH_ACORN > > > > diff --git a/drivers/video/fbdev/Makefile b/drivers/video/fbdev/Mak= efile > > > > index cad4fb64442a..a0705b99e643 100644 > > > > --- a/drivers/video/fbdev/Makefile > > > > +++ b/drivers/video/fbdev/Makefile > > > > @@ -75,6 +75,7 @@ obj-$(CONFIG_FB_HIT) +=3D hitfb.o > > > > obj-$(CONFIG_FB_ATMEL) +=3D atmel_lcdfb.o > > > > obj-$(CONFIG_FB_PVR2) +=3D pvr2fb.o > > > > obj-$(CONFIG_FB_VOODOO1) +=3D sstfb.o > > > > +obj-$(CONFIG_FB_ARMCLCD) +=3D amba-clcd.o > > > > obj-$(CONFIG_FB_GOLDFISH) +=3D goldfishfb.o > > > > obj-$(CONFIG_FB_68328) +=3D 68328fb.o > > > > obj-$(CONFIG_FB_GBE) +=3D gbefb.o > > > > diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/= amba-clcd.c > > > > new file mode 100644 > > > > index 000000000000..b7682de412d8 > > > > --- /dev/null > > > > +++ b/drivers/video/fbdev/amba-clcd.c > > > > @@ -0,0 +1,986 @@ > > > > +/* > > > > + * linux/drivers/video/amba-clcd.c > > > > + * > > > > + * Copyright (C) 2001 ARM Limited, by David A Rusling > > > > + * Updated to 2.5, Deep Blue Solutions Ltd. > > > > + * > > > > + * This file is subject to the terms and conditions of the GNU Gen= eral Public > > > > + * License. See the file COPYING in the main directory of this ar= chive > > > > + * for more details. > > > > + * > > > > + * ARM PrimeCell PL110 Color LCD Controller > > > > + */ > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include