Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753399AbbDGMNV (ORCPT ); Tue, 7 Apr 2015 08:13:21 -0400 Received: from mail.kmu-office.ch ([178.209.48.109]:36554 "EHLO mail.kmu-office.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752721AbbDGMNS (ORCPT ); Tue, 7 Apr 2015 08:13:18 -0400 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Date: Tue, 07 Apr 2015 14:12:27 +0200 From: Stefan Agner To: Jianwei.Wang@freescale.com Cc: Scott Wood , airlied@linux.ie, dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, Xiubo Li , Huan Wang , linux-kernel@vger.kernel.org, Jason.Jin@freescale.com, linux-arm-kernel@lists.infradead.org Subject: RE: [PATCH v3 1/4] drm/layerscape: Add Freescale DCU DRM driver In-Reply-To: References: <1427348225-40731-1-git-send-email-b52261@freescale.com> Message-ID: <9ba391aa62a7da03888dace5fa68e36b@agner.ch> User-Agent: Roundcube Webmail/1.1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 61804 Lines: 1778 Hi Jianwei, On 2015-04-07 08:44, Jianwei.Wang@freescale.com wrote: > Hi Stefan, > > Thank you for your review and testing on Vybrid F610 device. This driver > just implement the basic functions and it only support the exported > framebuffer access. Some DRM interfaces are not implemented now. So your > test result is normal. I will implement these interfaces with patches soon > afterwards. I don't plan to add new features for the initial version driver, > otherwise it will be a long term for this version. > > I tested on ls1021a using TFT panel, there are no flickers on the screen > when inserting a USB HID device. I will do more test if time permits. > > By the way, could please give me some guidance on how X-Server use DRM > Interface directly? Do you have some papers or webpage about this? I'm using the modesetting X.org driver. Lots of distributions ship that driver as a package (e.g. xserver-xorg-video-modesetting in Debian, or xf86-video-modesetting in OpenEmbedded). Since 1.17 this driver has even been included into the main source tree of Xorg X-Server (http://cgit.freedesktop.org/xorg/xserver/tree/hw/xfree86/drivers/modesetting) This driver is using KMS/DRM interface and should work well for un-accelerated graphic devices. This is a a much nicer way to use X.org on top of a DRM driver, since it avoids going through the legacy fbdev interface. The man page shows how to use it: http://linux.die.net/man/4/modesetting So, when the driver is installed, it is just choosing that driver in xorg.conf: Section "Device" Identifier "dcu" Driver "modesetting" EndSection Some more comments below... > > My reply below... > >> >> Hi Jianwei, >> >> The driver worked on a Vybrid VF610 device using the exported >> framebuffer. However, when using X-Server through the DRM interface >> directly (by using the modesetting driver) I get just a black screen so >> far, still investigating the reason. What user-space interfaces did you >> test? >> >> When using the FB device and insert a USB HID device, I get some >> flickers on the screen. I didn't had those on the dcufb driver, did you >> notice something like this too? Probably related to the resolution, I'm >> using VGA resolution. >> >> Some comments below. >> >> >> On 2015-03-26 06:37, Jianwei Wang wrote: >> > This patch add support for Two Dimensional Animation and Compositing >> > Engine (2D-ACE) on the Freescale SoCs. >> > >> > 2D-ACE is a Freescale display controller. 2D-ACE describes >> > the functionality of the module extremely well its name is a value >> > that cannot be used as a token in programming languages. >> > Instead the valid token "DCU" is used to tag the register names and >> > function names. >> > >> > The Display Controller Unit (DCU) module is a system master that >> > fetches graphics stored in internal or external memory and displays >> > them on a TFT LCD panel. A wide range of panel sizes is supported >> > and the timing of the interface signals is highly configurable. >> > Graphics are read directly from memory and then blended in real-time, >> > which allows for dynamic content creation with minimal CPU >> > intervention. >> > >> > The features: >> > (1) Full RGB888 output to TFT LCD panel. >> > (2) For the current LCD panel, WQVGA "480x272" is supported. >> > (3) Blending of each pixel using up to 4 source layers >> > dependent on size of panel. >> >> modetest only shows one layer currently... > > Yes, only one plane and one framebuffer were created now, others > maybe create as user requirement or create all when initializing, > I'm not sure now. This describe the hardware feature > >> >> > (4) Each graphic layer can be placed with one pixel resolution >> > in either axis. >> > (5) Each graphic layer support RGB565 and RGB888 direct colors >> > without alpha channel >> > and BGRA8888 direct colors with an alpha channel. >> >> The array fsl_dcu_drm_plane_formats below shows more formats, does this >> commit log needs updating? >> > > I agree with your suggestion, I will update this commit log > >> > (6) Each graphic layer support alpha blending with 8-bit >> > resolution. >> > >> > This is a simplified version, only one primary plane, one >> > framebuffer created for fbdev, one crtc, one connector for >> > TFT LCD panel, an encoder. >> > >> > Signed-off-by: Alison Wang >> > Signed-off-by: Xiubo Li >> > Signed-off-by: Jianwei Wang >> > --- >> > >> > Changed in V3: >> > >> > - Test driver on Vybrid board and add compatible string >> > - Remove unused functions >> > - set default crtc for encoder >> > - replace legacy functions with atomic help functions >> > - Set the unique name of the DRM device >> > - Implement irq handle function for vblank interrupt >> > >> > Changed in v2: >> > - Add atomic support >> > - Modify bindings file >> > - Rename node for compatibility >> > - Move platform related code out for compatibility >> > >> > .../devicetree/bindings/drm/fsl/fsl,dcu.txt | 50 ++++ >> > drivers/gpu/drm/Kconfig | 2 + >> > drivers/gpu/drm/Makefile | 1 + >> > drivers/gpu/drm/fsl/Kconfig | 17 ++ >> > drivers/gpu/drm/fsl/Makefile | 8 + >> > drivers/gpu/drm/fsl/fsl_dcu_drm_connector.c | 193 ++++++++++++ >> > drivers/gpu/drm/fsl/fsl_dcu_drm_connector.h | 30 ++ >> > drivers/gpu/drm/fsl/fsl_dcu_drm_crtc.c | 165 ++++++++++ >> > drivers/gpu/drm/fsl/fsl_dcu_drm_crtc.h | 26 ++ >> > drivers/gpu/drm/fsl/fsl_dcu_drm_drv.c | 331 >> +++++++++++++++++++++ >> > drivers/gpu/drm/fsl/fsl_dcu_drm_drv.h | 210 +++++++++++++ >> > drivers/gpu/drm/fsl/fsl_dcu_drm_fbdev.c | 26 ++ >> > drivers/gpu/drm/fsl/fsl_dcu_drm_kms.c | 42 +++ >> > drivers/gpu/drm/fsl/fsl_dcu_drm_kms.h | 17 ++ >> > drivers/gpu/drm/fsl/fsl_dcu_drm_plane.c | 192 ++++++++++++ >> > drivers/gpu/drm/fsl/fsl_dcu_drm_plane.h | 23 ++ >> > 16 files changed, 1333 insertions(+) >> > create mode 100644 >> Documentation/devicetree/bindings/drm/fsl/fsl,dcu.txt >> > create mode 100644 drivers/gpu/drm/fsl/Kconfig >> > create mode 100644 drivers/gpu/drm/fsl/Makefile >> > create mode 100644 drivers/gpu/drm/fsl/fsl_dcu_drm_connector.c >> > create mode 100644 drivers/gpu/drm/fsl/fsl_dcu_drm_connector.h >> > create mode 100644 drivers/gpu/drm/fsl/fsl_dcu_drm_crtc.c >> > create mode 100644 drivers/gpu/drm/fsl/fsl_dcu_drm_crtc.h >> > create mode 100644 drivers/gpu/drm/fsl/fsl_dcu_drm_drv.c >> > create mode 100644 drivers/gpu/drm/fsl/fsl_dcu_drm_drv.h >> > create mode 100644 drivers/gpu/drm/fsl/fsl_dcu_drm_fbdev.c >> > create mode 100644 drivers/gpu/drm/fsl/fsl_dcu_drm_kms.c >> > create mode 100644 drivers/gpu/drm/fsl/fsl_dcu_drm_kms.h >> > create mode 100644 drivers/gpu/drm/fsl/fsl_dcu_drm_plane.c >> > create mode 100644 drivers/gpu/drm/fsl/fsl_dcu_drm_plane.h >> > >> > diff --git a/Documentation/devicetree/bindings/drm/fsl/fsl,dcu.txt >> > b/Documentation/devicetree/bindings/drm/fsl/fsl,dcu.txt >> > new file mode 100644 >> > index 0000000..bdc7d5b >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/drm/fsl/fsl,dcu.txt >> > @@ -0,0 +1,50 @@ >> > +Device Tree bindings for Freescale DCU DRM Driver >> > + >> > +Required properties: >> > +- compatible: Should be one of >> > + * "fsl,ls1021a-dcu". >> > + * "fsl,vf610-dcu". >> >> In the Vybrid reference manual, that IP is denoted as "DCU4". Any reason >> why you left out the DCU version? Is this different for LS1021a? On the >> other hand, with the SoC in the name, the IP should be specified clear >> enough. >> > > In the LS1021A reference manual, no DCU version related words. I asked > previous DCU owner, she told me that LS1021A use DCU5 and she also point > out that the same DCU version there may be differences between different > SoCs. So I named compatible string with SoC name. Ok, I'm fine with that, thx for the background information. > >> > +- reg: Address and length of the register set for dcu. >> > +- clocks: From common clock binding: handle to dcu clock. >> > +- clock-names: From common clock binding: Shall be "dcu". >> > +- display: The phandle to display node. >> > + >> > +Required properties: >> > +- bits-per-pixel: <16> for RGB565, >> > + <24> for RGB888, >> > + <32> for RGB8888. >> > + >> > +Required timing node for dispplay sub-node: >> > +- display-timings: Refer to binding doc display-timing.txt for >> details. >> > + >> > +Examples: >> > +dcu: dcu@2ce0000 { >> > + compatible = "fsl,ls1021a-dcu"; >> > + reg = <0x0 0x2ce0000 0x0 0x10000>; >> > + clocks = <&platform_clk 0>; >> > + clock-names = "dcu"; >> > + big-endian; >> > + display = <&display>; >> > + >> > + display: display@0 { >> > + bits-per-pixel = <24>; >> > + >> > + display-timings { >> > + native-mode = <&timing0>; >> > + timing0: nl4827hc19 { >> > + clock-frequency = <10870000>; >> > + hactive = <480>; >> > + vactive = <272>; >> > + hback-porch = <2>; >> > + hfront-porch = <2>; >> > + vback-porch = <1>; >> > + vfront-porch = <1>; >> > + hsync-len = <41>; >> > + vsync-len = <2>; >> > + hsync-active = <1>; >> > + vsync-active = <1>; >> > + }; >> > + }; >> > + }; >> > +}; >> > + >> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >> > index 151a050..a6957aa 100644 >> > --- a/drivers/gpu/drm/Kconfig >> > +++ b/drivers/gpu/drm/Kconfig >> > @@ -199,6 +199,8 @@ source "drivers/gpu/drm/bochs/Kconfig" >> > >> > source "drivers/gpu/drm/msm/Kconfig" >> > >> > +source "drivers/gpu/drm/fsl/Kconfig" >> > + >> > source "drivers/gpu/drm/tegra/Kconfig" >> > >> > source "drivers/gpu/drm/panel/Kconfig" >> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile >> > index 2c239b9..ab5b9ef 100644 >> > --- a/drivers/gpu/drm/Makefile >> > +++ b/drivers/gpu/drm/Makefile >> > @@ -56,6 +56,7 @@ obj-$(CONFIG_DRM_UDL) += udl/ >> > obj-$(CONFIG_DRM_AST) += ast/ >> > obj-$(CONFIG_DRM_ARMADA) += armada/ >> > obj-$(CONFIG_DRM_ATMEL_HLCDC) += atmel-hlcdc/ >> > +obj-$(CONFIG_DRM_FSL_DCU) += fsl/ >> >> Hm, I'm wondering whether it would be a good idea to store that driver >> under fsl-dcu/... Any opinion? >> > > I think it is good advice, I'll think about this > >> > obj-$(CONFIG_DRM_RCAR_DU) += rcar-du/ >> > obj-$(CONFIG_DRM_SHMOBILE) +=shmobile/ >> > obj-$(CONFIG_DRM_OMAP) += omapdrm/ >> > diff --git a/drivers/gpu/drm/fsl/Kconfig b/drivers/gpu/drm/fsl/Kconfig >> > new file mode 100644 >> > index 0000000..e4f8df0 >> > --- /dev/null >> > +++ b/drivers/gpu/drm/fsl/Kconfig >> > @@ -0,0 +1,17 @@ >> > +config DRM_FSL_DCU >> > + tristate "DRM Support for Freescale DCU" >> > + depends on DRM && OF && ARM >> > + select DRM_KMS_HELPER >> > + select DRM_KMS_CMA_HELPER >> > + select VIDEOMODE_HELPERS >> > + select BACKLIGHT_CLASS_DEVICE >> > + select BACKLIGHT_LCD_SUPPORT >> > + select REGMAP_MMIO >> > + select DRM_KMS_FB_HELPER >> > + select FB_SYS_FILLRECT >> > + select FB_SYS_COPYAREA >> > + select FB_SYS_IMAGEBLIT >> > + select FB_SYS_FOPS >> > + help >> > + Choose this option if you have an Freescale DCU chipset. >> > + If M is selected the module will be called fsl-dcu-drm. >> > diff --git a/drivers/gpu/drm/fsl/Makefile b/drivers/gpu/drm/fsl/Makefile >> > new file mode 100644 >> > index 0000000..5f74aee >> > --- /dev/null >> > +++ b/drivers/gpu/drm/fsl/Makefile >> > @@ -0,0 +1,8 @@ >> > +fsl-dcu-drm-y := fsl_dcu_drm_drv.o \ >> > + fsl_dcu_drm_kms.o \ >> > + fsl_dcu_drm_connector.o \ >> > + fsl_dcu_drm_plane.o \ >> > + fsl_dcu_drm_crtc.o \ >> > + fsl_dcu_drm_fbdev.o \ >> > + >> > +obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu-drm.o >> > diff --git a/drivers/gpu/drm/fsl/fsl_dcu_drm_connector.c >> > b/drivers/gpu/drm/fsl/fsl_dcu_drm_connector.c >> > new file mode 100644 >> > index 0000000..4610647 >> > --- /dev/null >> > +++ b/drivers/gpu/drm/fsl/fsl_dcu_drm_connector.c >> > @@ -0,0 +1,193 @@ >> > +/* >> > + * Copyright 2015 Freescale Semiconductor, Inc. >> > + * >> > + * Freescale DCU drm device driver >> > + * >> > + * This program is free software; you can redistribute it and/or modify >> > + * it under the terms of the GNU General Public License as published by >> > + * the Free Software Foundation; either version 2 of the License, or >> > + * (at your option) any later version. >> > + */ >> > + >> > +#include >> > + >> > +#include >> > +#include >> > +#include