From patchwork Wed Sep 7 17:05:10 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stefan Agner X-Patchwork-Id: 9319695 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C83F8607D3 for ; Wed, 7 Sep 2016 17:10:36 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CBC6829396 for ; Wed, 7 Sep 2016 17:10:36 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C029129409; Wed, 7 Sep 2016 17:10:36 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id B9A5629396 for ; Wed, 7 Sep 2016 17:10:35 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 01F5D6EB7B; Wed, 7 Sep 2016 17:10:33 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail.kmu-office.ch (mail.kmu-office.ch [178.209.48.109]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7FB066EB7B for ; Wed, 7 Sep 2016 17:10:30 +0000 (UTC) Received: from webmail.kmu-office.ch (unknown [178.209.48.103]) by mail.kmu-office.ch (Postfix) with ESMTPSA id B770A5C148D; Wed, 7 Sep 2016 19:05:15 +0200 (CEST) MIME-Version: 1.0 Date: Wed, 07 Sep 2016 10:05:10 -0700 From: Stefan Agner To: Meng Yi , robh+dt@kernel.org, frowand.list@gmail.com, broonie@kernel.org Subject: Re: [PATCH v3] drm/fsl-dcu: Implement gamma_lut atomic crtc properties In-Reply-To: <1473240152-47112-1-git-send-email-meng.yi@nxp.com> References: <1473240152-47112-1-git-send-email-meng.yi@nxp.com> Message-ID: <6ea9b8690ecec22db0f0e5b76357be50@agner.ch> X-Sender: stefan@agner.ch User-Agent: Roundcube Webmail/1.1.3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=agner.ch; s=dkim; t=1473267915; bh=EujcZsXtgkPlCeA9BbsR+sgx1sclfeflWZyd8R7MEWU=; h=MIME-Version:Content-Type:Content-Transfer-Encoding:Date:From:To:Cc:Subject:In-Reply-To:References:Message-ID; b=UAE4VdNkSkPh3f8vJ7BuyaMS9akfgGzW5XFS+pJDzsZmGAPObWXza3pgOuJVVD91BgySLwXciVfQewwh6tYg0q+H99xvtvsPKM/qa31l1yLkeNTN3QgR9kqtYhIHweFnfUN1eyPr3+AiaJCI1H6WtTPk0EmD1jwUEpQTfJAAKY4= Cc: jianwei.wang.chn@gmail.com, dri-devel@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP On 2016-09-07 02:22, Meng Yi wrote: > Gamma correction is optional and can be used to adjust the color > output values to match the gamut of a particular TFT LCD panel > Errata: > Gamma_R, Gamma_G and Gamma_B registers are little-endian registers > while the rest of the address-space in 2D-ACE is big-endian. > Workaround: > Split the DCU regs into "regs", "palette", "gamma" and "cursor". > Create a second regmap for gamma memory space using little endian. > > Suggested-by: Stefan Agner > Signed-off-by: Meng Yi > --- > Changes in V3: > -created a second regmap for gamma > -updated the DCU DT binding > --- > .../devicetree/bindings/display/fsl,dcu.txt | 6 +++- > drivers/gpu/drm/fsl-dcu/Kconfig | 6 ++++ > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 38 ++++++++++++++++++++++ > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 30 ++++++++++++++++- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 8 +++++ > 5 files changed, 86 insertions(+), 2 deletions(-) > Please also extend the documentation of reg and reg-names above. > diff --git a/Documentation/devicetree/bindings/display/fsl,dcu.txt > b/Documentation/devicetree/bindings/display/fsl,dcu.txt > index 63ec2a6..1b1321a 100644 > --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt > +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt > @@ -20,7 +20,11 @@ Optional properties: > Examples: > dcu: dcu@2ce0000 { > compatible = "fsl,ls1021a-dcu"; > - reg = <0x0 0x2ce0000 0x0 0x10000>; > + reg = <0x0 0x2ce0000 0x0 0x2000>, > + <0x0 0x2ce2000 0x0 0x2000>, > + <0x0 0x2ce4000 0x0 0xc00>, > + <0x0 0x2ce4c00 0x0 0x400>; > + reg-names = "regs", "palette", "gamma", "cursor"; > clocks = <&platform_clk 0>, <&platform_clk 0>; > clock-names = "dcu", "pix"; > big-endian; Looks good to me, I feel splitting up the register map makes sense anyway. It is also documented that way: 36.4 Memory Map Table 36-5. Memory map Parameter Address Range Register address space 0x0000 – 0x1FFF Palette/Tile address space 0x2000 – 0x3FFF Gamma_R address space 0x4000 – 0x43FF Gamma_G address space 0x4400 – 0x47FF Gamma_B address space 0x4800 – 0x4BFF Cursor address space 0x4C00 – 0x4FFF Can I have an Ack from the device tree maintainers here? > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c > @@ -22,6 +22,22 @@ > #include "fsl_dcu_drm_drv.h" > #include "fsl_dcu_drm_plane.h" > > +static void fsl_crtc_gamma_set(struct drm_crtc *crtc, struct > drm_color_lut *lut, > + uint32_t size) > +{ > + struct fsl_dcu_drm_device *fsl_dev = crtc->dev->dev_private; > + unsigned int i; > + > + for (i = 0; i < size; i++) { > + regmap_write(fsl_dev->regmap_gamma, FSL_GAMMA_R + 4 * i, > + lut[i].red); > + regmap_write(fsl_dev->regmap_gamma, FSL_GAMMA_G + 4 * i, > + lut[i].green); > + regmap_write(fsl_dev->regmap_gamma, FSL_GAMMA_B + 4 * i, > + lut[i].blue); > + } > +} > + > static void fsl_dcu_drm_crtc_atomic_flush(struct drm_crtc *crtc, > struct drm_crtc_state *old_crtc_state) > { > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > @@ -48,6 +48,15 @@ static const struct regmap_config fsl_dcu_regmap_config = { > .volatile_reg = fsl_dcu_drm_is_volatile_reg, > }; > > +static const struct regmap_config fsl_dcu_regmap_gamma_config = { > + .reg_bits = 32, > + .reg_stride = 4, > + .val_bits = 32, > + .val_format_endian = REGMAP_ENDIAN_LITTLE, I would like to have a comment here why we force little endian. > + > + .volatile_reg = fsl_dcu_drm_is_volatile_reg, > +}; > + > @@ -365,6 +374,25 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) > return PTR_ERR(fsl_dev->regmap); > } > > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "gamma"); > + if (!res) { > + dev_err(dev, "could not get gamma memory resource\n"); > + return -ENODEV; > + } > + > + base_gamma = devm_ioremap_resource(dev, res); > + if (IS_ERR(base_gamma)) { > + ret = PTR_ERR(base_gamma); > + return ret; > + } > + > + fsl_dev->regmap_gamma = devm_regmap_init_mmio(dev, base_gamma, > + &fsl_dcu_regmap_config); > + if (IS_ERR(fsl_dev->regmap_gamma)) { > + dev_err(dev, "regmap_gamma init failed\n"); > + return PTR_ERR(fsl_dev->regmap_gamma); > + } > + Mark, what do you think, is this a reasonable approach to work around this errata? --- Stefan diff --git a/Documentation/devicetree/bindings/display/fsl,dcu.txt b/Documentation/devicetree/bindings/display/fsl,dcu.txt index 63ec2a6..1b1321a 100644 --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt @@ -20,7 +20,11 @@ Optional properties: Examples: dcu: dcu@2ce0000 { compatible = "fsl,ls1021a-dcu"; - reg = <0x0 0x2ce0000 0x0 0x10000>; + reg = <0x0 0x2ce0000 0x0 0x2000>, + <0x0 0x2ce2000 0x0 0x2000>, + <0x0 0x2ce4000 0x0 0xc00>, + <0x0 0x2ce4c00 0x0 0x400>; + reg-names = "regs", "palette", "gamma", "cursor"; clocks = <&platform_clk 0>, <&platform_clk 0>; clock-names = "dcu", "pix"; big-endian;