[PATCH v6 0/8] drm: rcar-du: Add Color Management Module (CMM)

[PATCH v6 0/8] drm: rcar-du: Add Color Management Module (CMM)

[Jacopo Mondi]

Minimal increment to the CMM series, this time should really be the last one.

Just missing Rob's ack on [1/8] and Laurent's one on [5/8].

Changelog is minimal:
CMM
- Remove the cmm_config.enable flag. The cmm_config.table field validity is
  used to enable/disable the LUT operations
- Expand comments as suggested by Laurent

CRTC
- use drm_color_lut_size() to check the LUT table size
- Inline calls to rcar_cmm_enable()/disable()
- Add TODO entries as suggested by Laurent

For the record, the full series changelog is available at:
https://paste.debian.net/1107427/

v5 from yesterday with informations on testing is available at:
https://lkml.org/lkml/2019/10/15/337

Geert will you collect for DTS patches for the next release?
I assume the DU changes go through Laurent instead ?

Thanks
   j

Jacopo Mondi (8):
  dt-bindings: display: renesas,cmm: Add R-Car CMM documentation
  dt-bindings: display, renesas,du: Document cmms property
  drm: rcar-du: Add support for CMM
  drm: rcar-du: kms: Initialize CMM instances
  drm: rcar-du: crtc: Control CMM operations
  drm: rcar-du: crtc: Register GAMMA_LUT properties
  arm64: dts: renesas: Add CMM units to Gen3 SoCs
  drm: rcar-du: kms: Expand comment in vsps parsing routine

 .../bindings/display/renesas,cmm.yaml         |  67 ++++++
 .../bindings/display/renesas,du.txt           |   5 +
 arch/arm64/boot/dts/renesas/r8a7795.dtsi      |  39 ++++
 arch/arm64/boot/dts/renesas/r8a7796.dtsi      |  31 ++-
 arch/arm64/boot/dts/renesas/r8a77965.dtsi     |  31 ++-
 arch/arm64/boot/dts/renesas/r8a77990.dtsi     |  21 ++
 arch/arm64/boot/dts/renesas/r8a77995.dtsi     |  21 ++
 drivers/gpu/drm/rcar-du/Kconfig               |   7 +
 drivers/gpu/drm/rcar-du/Makefile              |   1 +
 drivers/gpu/drm/rcar-du/rcar_cmm.c            | 212 ++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_cmm.h            |  58 +++++
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  65 ++++++
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h        |   2 +
 drivers/gpu/drm/rcar-du/rcar_du_drv.h         |   2 +
 drivers/gpu/drm/rcar-du/rcar_du_group.c       |  10 +
 drivers/gpu/drm/rcar-du/rcar_du_group.h       |   2 +
 drivers/gpu/drm/rcar-du/rcar_du_kms.c         |  82 ++++++-
 drivers/gpu/drm/rcar-du/rcar_du_regs.h        |   5 +
 18 files changed, 658 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/renesas,cmm.yaml
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h

--
2.23.0

[PATCH v6 1/8] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[Jacopo Mondi]

Add device tree bindings documentation for the Renesas R-Car Display
Unit Color Management Module.

CMM is the image enhancement module available on each R-Car DU video
channel on R-Car Gen2 and Gen3 SoCs (V3H and V3M excluded).

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 .../bindings/display/renesas,cmm.yaml         | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/renesas,cmm.yaml

diff --git a/Documentation/devicetree/bindings/display/renesas,cmm.yaml b/Documentation/devicetree/bindings/display/renesas,cmm.yaml
new file mode 100644
index 000000000000..a57037b9e9ba
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/renesas,cmm.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/renesas,cmm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Car Color Management Module (CMM)
+
+maintainers:
+  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+  - Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
+  - Jacopo Mondi <jacopo+renesas@jmondi.org>
+
+description: |+
+  Renesas R-Car color management module connected to R-Car DU video channels.
+  It provides image enhancement functions such as 1-D look-up tables (LUT),
+  3-D look-up tables (CLU), 1D-histogram generation (HGO), and color
+  space conversion (CSC).
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - enum:
+          - renesas,r8a7795-cmm
+          - renesas,r8a7796-cmm
+          - renesas,r8a77965-cmm
+          - renesas,r8a77990-cmm
+          - renesas,r8a77995-cmm
+        - const: renesas,rcar-gen3-cmm
+      - items:
+        - const: renesas,rcar-gen2-cmm
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - resets
+  - power-domains
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r8a7796-cpg-mssr.h>
+    #include <dt-bindings/power/r8a7796-sysc.h>
+
+    cmm0: cmm@fea40000 {
+         compatible = "renesas,r8a7796-cmm",
+                      "renesas,rcar-gen3-cmm";
+         reg = <0 0xfea40000 0 0x1000>;
+         power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+         clocks = <&cpg CPG_MOD 711>;
+         resets = <&cpg 711>;
+    };
--
2.23.0

Re: [PATCH v6 1/8] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[Rob Herring]

On Wed, 16 Oct 2019 10:55:41 +0200, Jacopo Mondi wrote:
> Add device tree bindings documentation for the Renesas R-Car Display
> Unit Color Management Module.
> 
> CMM is the image enhancement module available on each R-Car DU video
> channel on R-Car Gen2 and Gen3 SoCs (V3H and V3M excluded).
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  .../bindings/display/renesas,cmm.yaml         | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/renesas,cmm.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v6 1/8] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[Rob Herring]

On Wed, 16 Oct 2019 10:55:41 +0200, Jacopo Mondi wrote:
> Add device tree bindings documentation for the Renesas R-Car Display
> Unit Color Management Module.
> 
> CMM is the image enhancement module available on each R-Car DU video
> channel on R-Car Gen2 and Gen3 SoCs (V3H and V3M excluded).
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  .../bindings/display/renesas,cmm.yaml         | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/renesas,cmm.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v6 2/8] dt-bindings: display, renesas,du: Document cmms property

[Jacopo Mondi]

Document the newly added 'cmms' property which accepts a list of phandle
and channel index pairs that point to the CMM units available for each
Display Unit output video channel.

Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 Documentation/devicetree/bindings/display/renesas,du.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
index c97dfacad281..3d9809b486b6 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.txt
+++ b/Documentation/devicetree/bindings/display/renesas,du.txt
@@ -45,6 +45,10 @@ Required Properties:
     instance that serves the DU channel, and the channel index identifies the
     LIF instance in that VSP.
 
+  - renesas,cmms: A list of phandles to the CMM instances present in the SoC,
+    one for each available DU channel. The property shall not be specified for
+    SoCs that do not provide any CMM (such as V3M and V3H).
+
 Required nodes:
 
 The connections to the DU output video ports are modeled using the OF graph
@@ -91,6 +95,7 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
 			 <&cpg CPG_MOD 721>;
 		clock-names = "du.0", "du.1", "du.2", "du.3";
 		vsps = <&vspd0 0>, <&vspd1 0>, <&vspd2 0>, <&vspd0 1>;
+		renesas,cmms = <&cmm0>, <&cmm1>, <&cmm2>, <&cmm3>;
 
 		ports {
 			#address-cells = <1>;
-- 
2.23.0

[PATCH v6 3/8] drm: rcar-du: Add support for CMM

[Jacopo Mondi]

Add a driver for the R-Car Display Unit Color Correction Module.

In most of Gen3 SoCs, each DU output channel is provided with a CMM unit
to perform image enhancement and color correction.

Add support for CMM through a driver that supports configuration of
the 1-dimensional LUT table. More advanced CMM features will be
implemented on top of this initial one.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/gpu/drm/rcar-du/Kconfig    |   7 +
 drivers/gpu/drm/rcar-du/Makefile   |   1 +
 drivers/gpu/drm/rcar-du/rcar_cmm.c | 212 +++++++++++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_cmm.h |  58 ++++++++
 4 files changed, 278 insertions(+)
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index 1529849e217e..539d232790d1 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -13,6 +13,13 @@ config DRM_RCAR_DU
 	  Choose this option if you have an R-Car chipset.
 	  If M is selected the module will be called rcar-du-drm.
 
+config DRM_RCAR_CMM
+	bool "R-Car DU Color Management Module (CMM) Support"
+	depends on DRM && OF
+	depends on DRM_RCAR_DU
+	help
+	  Enable support for R-Car Color Management Module (CMM).
+
 config DRM_RCAR_DW_HDMI
 	tristate "R-Car DU Gen3 HDMI Encoder Support"
 	depends on DRM && OF
diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
index 6c2ed9c46467..4d1187ccc3e5 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
 rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
 rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
 
+obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
 obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
 obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
 obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c b/drivers/gpu/drm/rcar-du/rcar_cmm.c
new file mode 100644
index 000000000000..4170626208cf
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * rcar_cmm.c -- R-Car Display Unit Color Management Module
+ *
+ * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
+ */
+
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include <drm/drm_color_mgmt.h>
+
+#include "rcar_cmm.h"
+
+#define CM2_LUT_CTRL		0x0000
+#define CM2_LUT_CTRL_LUT_EN	BIT(0)
+#define CM2_LUT_TBL_BASE	0x0600
+#define CM2_LUT_TBL(__i)	(CM2_LUT_TBL_BASE + (__i) * 4)
+
+struct rcar_cmm {
+	void __iomem *base;
+
+	/*
+	 * @lut:		1D-LUT state
+	 * @lut.enabled:	1D-LUT enabled flag
+	 */
+	struct {
+		bool enabled;
+	} lut;
+};
+
+static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg)
+{
+	return ioread32(rcmm->base + reg);
+}
+
+static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 data)
+{
+	iowrite32(data, rcmm->base + reg);
+}
+
+/*
+ * rcar_cmm_lut_write() - Scale the DRM LUT table entries to hardware precision
+ *			  and write to the CMM registers
+ * @rcmm: Pointer to the CMM device
+ * @drm_lut: Pointer to the DRM LUT table
+ */
+static void rcar_cmm_lut_write(struct rcar_cmm *rcmm,
+			       const struct drm_color_lut *drm_lut)
+{
+	unsigned int i;
+
+	for (i = 0; i < CM2_LUT_SIZE; ++i) {
+		u32 entry = drm_color_lut_extract(drm_lut[i].red, 8) << 16
+			  | drm_color_lut_extract(drm_lut[i].green, 8) << 8
+			  | drm_color_lut_extract(drm_lut[i].blue, 8);
+
+		rcar_cmm_write(rcmm, CM2_LUT_TBL(i), entry);
+	}
+}
+
+/*
+ * rcar_cmm_setup() - Configure the CMM unit
+ * @pdev: The platform device associated with the CMM instance
+ * @config: The CMM unit configuration
+ *
+ * Configure the CMM unit with the given configuration. Currently enabling,
+ * disabling and programming of the 1-D LUT unit is supported.
+ *
+ * TODO: Add support for LUT double buffer operations to avoid updating the
+ * LUT table entries while a frame is being displayed.
+ */
+int rcar_cmm_setup(struct platform_device *pdev,
+		   const struct rcar_cmm_config *config)
+{
+	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
+
+	/* Disable LUT if no table is provided. */
+	if (!config->lut.table) {
+		if (rcmm->lut.enabled) {
+			rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
+			rcmm->lut.enabled = false;
+		}
+
+		return 0;
+	}
+
+	/* Enable LUT and program the new gamma table values. */
+	if (!rcmm->lut.enabled) {
+		rcar_cmm_write(rcmm, CM2_LUT_CTRL, CM2_LUT_CTRL_LUT_EN);
+		rcmm->lut.enabled = true;
+	}
+
+	rcar_cmm_lut_write(rcmm, config->lut.table);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rcar_cmm_setup);
+
+/*
+ * rcar_cmm_enable() - Enable the CMM unit
+ * @pdev: The platform device associated with the CMM instance
+ *
+ * When the output of the corresponding DU channel is routed to the CMM unit,
+ * the unit shall be enabled before the DU channel is started, and remain
+ * enabled until the channel is stopped. The CMM unit shall be disabled with
+ * rcar_cmm_disable().
+ *
+ * Calls to rcar_cmm_enable() and rcar_cmm_disable() are not reference-counted.
+ * It is an error to attempt to enable an already enabled CMM unit, or to
+ * attempt to disable a disabled unit.
+ */
+int rcar_cmm_enable(struct platform_device *pdev)
+{
+	int ret;
+
+	ret = pm_runtime_get_sync(&pdev->dev);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rcar_cmm_enable);
+
+/*
+ * rcar_cmm_disable() - Disable the CMM unit
+ * @pdev: The platform device associated with the CMM instance
+ *
+ * See rcar_cmm_enable() for usage information.
+ *
+ * Disabling the CMM unit disable all the internal processing blocks. The CMM
+ * state shall thus be restored with rcar_cmm_setup() when re-enabling the CMM
+ * unit after the next rcar_cmm_enable() call.
+ */
+void rcar_cmm_disable(struct platform_device *pdev)
+{
+	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
+
+	rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
+	rcmm->lut.enabled = false;
+
+	pm_runtime_put(&pdev->dev);
+}
+EXPORT_SYMBOL_GPL(rcar_cmm_disable);
+
+/*
+ * rcar_cmm_init() - Initialize the CMM unit
+ * @pdev: The platform device associated with the CMM instance
+ *
+ * Return: 0 on success, -EPROBE_DEFER if the CMM is not available yet,
+ *         -ENODEV if the DRM_RCAR_CMM config option is disabled
+ */
+int rcar_cmm_init(struct platform_device *pdev)
+{
+	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
+
+	if (!rcmm)
+		return -EPROBE_DEFER;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rcar_cmm_init);
+
+static int rcar_cmm_probe(struct platform_device *pdev)
+{
+	struct rcar_cmm *rcmm;
+
+	rcmm = devm_kzalloc(&pdev->dev, sizeof(*rcmm), GFP_KERNEL);
+	if (!rcmm)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, rcmm);
+
+	rcmm->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(rcmm->base))
+		return PTR_ERR(rcmm->base);
+
+	pm_runtime_enable(&pdev->dev);
+
+	return 0;
+}
+
+static int rcar_cmm_remove(struct platform_device *pdev)
+{
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id rcar_cmm_of_table[] = {
+	{ .compatible = "renesas,rcar-gen3-cmm", },
+	{ .compatible = "renesas,rcar-gen2-cmm", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, rcar_cmm_of_table);
+
+static struct platform_driver rcar_cmm_platform_driver = {
+	.probe		= rcar_cmm_probe,
+	.remove		= rcar_cmm_remove,
+	.driver		= {
+		.name	= "rcar-cmm",
+		.of_match_table = rcar_cmm_of_table,
+	},
+};
+
+module_platform_driver(rcar_cmm_platform_driver);
+
+MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas@jmondi.org>");
+MODULE_DESCRIPTION("Renesas R-Car CMM Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.h b/drivers/gpu/drm/rcar-du/rcar_cmm.h
new file mode 100644
index 000000000000..b5f7ec6db04a
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_cmm.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * rcar_cmm.h -- R-Car Display Unit Color Management Module
+ *
+ * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
+ */
+
+#ifndef __RCAR_CMM_H__
+#define __RCAR_CMM_H__
+
+#define CM2_LUT_SIZE		256
+
+struct drm_color_lut;
+struct platform_device;
+
+/**
+ * struct rcar_cmm_config - CMM configuration
+ *
+ * @lut:	1D-LUT configuration
+ * @lut.table:	1D-LUT table entries. Disable LUT operations when NULL
+ */
+struct rcar_cmm_config {
+	struct {
+		struct drm_color_lut *table;
+	} lut;
+};
+
+#if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
+int rcar_cmm_init(struct platform_device *pdev);
+
+int rcar_cmm_enable(struct platform_device *pdev);
+void rcar_cmm_disable(struct platform_device *pdev);
+
+int rcar_cmm_setup(struct platform_device *pdev,
+		   const struct rcar_cmm_config *config);
+#else
+static inline int rcar_cmm_init(struct platform_device *pdev)
+{
+	return -ENODEV;
+}
+
+static inline int rcar_cmm_enable(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static inline void rcar_cmm_disable(struct platform_device *pdev)
+{
+}
+
+static inline int rcar_cmm_setup(struct platform_device *pdev,
+				 const struct rcar_cmm_config *config)
+{
+	return 0;
+}
+#endif /* IS_ENABLED(CONFIG_DRM_RCAR_CMM) */
+
+#endif /* __RCAR_CMM_H__ */
-- 
2.23.0

Re: [PATCH v6 3/8] drm: rcar-du: Add support for CMM

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Oct 16, 2019 at 10:55:43AM +0200, Jacopo Mondi wrote:
> Add a driver for the R-Car Display Unit Color Correction Module.
> 
> In most of Gen3 SoCs, each DU output channel is provided with a CMM unit
> to perform image enhancement and color correction.
> 
> Add support for CMM through a driver that supports configuration of
> the 1-dimensional LUT table. More advanced CMM features will be
> implemented on top of this initial one.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/gpu/drm/rcar-du/Kconfig    |   7 +
>  drivers/gpu/drm/rcar-du/Makefile   |   1 +
>  drivers/gpu/drm/rcar-du/rcar_cmm.c | 212 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_cmm.h |  58 ++++++++
>  4 files changed, 278 insertions(+)
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h
> 
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index 1529849e217e..539d232790d1 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -13,6 +13,13 @@ config DRM_RCAR_DU
>  	  Choose this option if you have an R-Car chipset.
>  	  If M is selected the module will be called rcar-du-drm.
>  
> +config DRM_RCAR_CMM
> +	bool "R-Car DU Color Management Module (CMM) Support"
> +	depends on DRM && OF
> +	depends on DRM_RCAR_DU
> +	help
> +	  Enable support for R-Car Color Management Module (CMM).
> +
>  config DRM_RCAR_DW_HDMI
>  	tristate "R-Car DU Gen3 HDMI Encoder Support"
>  	depends on DRM && OF
> diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
> index 6c2ed9c46467..4d1187ccc3e5 100644
> --- a/drivers/gpu/drm/rcar-du/Makefile
> +++ b/drivers/gpu/drm/rcar-du/Makefile
> @@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
>  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
>  rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
>  
> +obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
>  obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
>  obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
>  obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
> diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> new file mode 100644
> index 000000000000..4170626208cf
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> @@ -0,0 +1,212 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * rcar_cmm.c -- R-Car Display Unit Color Management Module
> + *
> + * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
> + */
> +
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include <drm/drm_color_mgmt.h>
> +
> +#include "rcar_cmm.h"
> +
> +#define CM2_LUT_CTRL		0x0000
> +#define CM2_LUT_CTRL_LUT_EN	BIT(0)
> +#define CM2_LUT_TBL_BASE	0x0600
> +#define CM2_LUT_TBL(__i)	(CM2_LUT_TBL_BASE + (__i) * 4)
> +
> +struct rcar_cmm {
> +	void __iomem *base;
> +
> +	/*
> +	 * @lut:		1D-LUT state
> +	 * @lut.enabled:	1D-LUT enabled flag
> +	 */
> +	struct {
> +		bool enabled;
> +	} lut;
> +};
> +
> +static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg)
> +{
> +	return ioread32(rcmm->base + reg);
> +}
> +
> +static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 data)
> +{
> +	iowrite32(data, rcmm->base + reg);
> +}
> +
> +/*
> + * rcar_cmm_lut_write() - Scale the DRM LUT table entries to hardware precision
> + *			  and write to the CMM registers
> + * @rcmm: Pointer to the CMM device
> + * @drm_lut: Pointer to the DRM LUT table
> + */
> +static void rcar_cmm_lut_write(struct rcar_cmm *rcmm,
> +			       const struct drm_color_lut *drm_lut)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < CM2_LUT_SIZE; ++i) {
> +		u32 entry = drm_color_lut_extract(drm_lut[i].red, 8) << 16
> +			  | drm_color_lut_extract(drm_lut[i].green, 8) << 8
> +			  | drm_color_lut_extract(drm_lut[i].blue, 8);
> +
> +		rcar_cmm_write(rcmm, CM2_LUT_TBL(i), entry);
> +	}
> +}
> +
> +/*
> + * rcar_cmm_setup() - Configure the CMM unit
> + * @pdev: The platform device associated with the CMM instance
> + * @config: The CMM unit configuration
> + *
> + * Configure the CMM unit with the given configuration. Currently enabling,
> + * disabling and programming of the 1-D LUT unit is supported.

Did you miss the comment in the previous version about explaining when
rcar_cmm_setup() can be called (basically requiring the CMM to be
enabled) ? I can fix this when applying if you tell me what I should
write.

> + *
> + * TODO: Add support for LUT double buffer operations to avoid updating the
> + * LUT table entries while a frame is being displayed.
> + */
> +int rcar_cmm_setup(struct platform_device *pdev,
> +		   const struct rcar_cmm_config *config)
> +{
> +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> +
> +	/* Disable LUT if no table is provided. */
> +	if (!config->lut.table) {
> +		if (rcmm->lut.enabled) {
> +			rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
> +			rcmm->lut.enabled = false;
> +		}
> +
> +		return 0;
> +	}
> +
> +	/* Enable LUT and program the new gamma table values. */
> +	if (!rcmm->lut.enabled) {
> +		rcar_cmm_write(rcmm, CM2_LUT_CTRL, CM2_LUT_CTRL_LUT_EN);
> +		rcmm->lut.enabled = true;
> +	}
> +
> +	rcar_cmm_lut_write(rcmm, config->lut.table);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rcar_cmm_setup);
> +
> +/*
> + * rcar_cmm_enable() - Enable the CMM unit
> + * @pdev: The platform device associated with the CMM instance
> + *
> + * When the output of the corresponding DU channel is routed to the CMM unit,
> + * the unit shall be enabled before the DU channel is started, and remain
> + * enabled until the channel is stopped. The CMM unit shall be disabled with
> + * rcar_cmm_disable().
> + *
> + * Calls to rcar_cmm_enable() and rcar_cmm_disable() are not reference-counted.
> + * It is an error to attempt to enable an already enabled CMM unit, or to
> + * attempt to disable a disabled unit.
> + */
> +int rcar_cmm_enable(struct platform_device *pdev)
> +{
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(&pdev->dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rcar_cmm_enable);
> +
> +/*
> + * rcar_cmm_disable() - Disable the CMM unit
> + * @pdev: The platform device associated with the CMM instance
> + *
> + * See rcar_cmm_enable() for usage information.
> + *
> + * Disabling the CMM unit disable all the internal processing blocks. The CMM
> + * state shall thus be restored with rcar_cmm_setup() when re-enabling the CMM
> + * unit after the next rcar_cmm_enable() call.
> + */
> +void rcar_cmm_disable(struct platform_device *pdev)
> +{
> +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> +
> +	rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
> +	rcmm->lut.enabled = false;
> +
> +	pm_runtime_put(&pdev->dev);
> +}
> +EXPORT_SYMBOL_GPL(rcar_cmm_disable);
> +
> +/*
> + * rcar_cmm_init() - Initialize the CMM unit
> + * @pdev: The platform device associated with the CMM instance
> + *
> + * Return: 0 on success, -EPROBE_DEFER if the CMM is not available yet,
> + *         -ENODEV if the DRM_RCAR_CMM config option is disabled
> + */
> +int rcar_cmm_init(struct platform_device *pdev)
> +{
> +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> +
> +	if (!rcmm)
> +		return -EPROBE_DEFER;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rcar_cmm_init);
> +
> +static int rcar_cmm_probe(struct platform_device *pdev)
> +{
> +	struct rcar_cmm *rcmm;
> +
> +	rcmm = devm_kzalloc(&pdev->dev, sizeof(*rcmm), GFP_KERNEL);
> +	if (!rcmm)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, rcmm);
> +
> +	rcmm->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(rcmm->base))
> +		return PTR_ERR(rcmm->base);
> +
> +	pm_runtime_enable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static int rcar_cmm_remove(struct platform_device *pdev)
> +{
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rcar_cmm_of_table[] = {
> +	{ .compatible = "renesas,rcar-gen3-cmm", },
> +	{ .compatible = "renesas,rcar-gen2-cmm", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, rcar_cmm_of_table);
> +
> +static struct platform_driver rcar_cmm_platform_driver = {
> +	.probe		= rcar_cmm_probe,
> +	.remove		= rcar_cmm_remove,
> +	.driver		= {
> +		.name	= "rcar-cmm",
> +		.of_match_table = rcar_cmm_of_table,
> +	},
> +};
> +
> +module_platform_driver(rcar_cmm_platform_driver);
> +
> +MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas@jmondi.org>");
> +MODULE_DESCRIPTION("Renesas R-Car CMM Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.h b/drivers/gpu/drm/rcar-du/rcar_cmm.h
> new file mode 100644
> index 000000000000..b5f7ec6db04a
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.h
> @@ -0,0 +1,58 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * rcar_cmm.h -- R-Car Display Unit Color Management Module
> + *
> + * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
> + */
> +
> +#ifndef __RCAR_CMM_H__
> +#define __RCAR_CMM_H__
> +
> +#define CM2_LUT_SIZE		256
> +
> +struct drm_color_lut;
> +struct platform_device;
> +
> +/**
> + * struct rcar_cmm_config - CMM configuration
> + *
> + * @lut:	1D-LUT configuration
> + * @lut.table:	1D-LUT table entries. Disable LUT operations when NULL
> + */
> +struct rcar_cmm_config {
> +	struct {
> +		struct drm_color_lut *table;
> +	} lut;
> +};
> +
> +#if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
> +int rcar_cmm_init(struct platform_device *pdev);
> +
> +int rcar_cmm_enable(struct platform_device *pdev);
> +void rcar_cmm_disable(struct platform_device *pdev);
> +
> +int rcar_cmm_setup(struct platform_device *pdev,
> +		   const struct rcar_cmm_config *config);
> +#else
> +static inline int rcar_cmm_init(struct platform_device *pdev)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline int rcar_cmm_enable(struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +
> +static inline void rcar_cmm_disable(struct platform_device *pdev)
> +{
> +}
> +
> +static inline int rcar_cmm_setup(struct platform_device *pdev,
> +				 const struct rcar_cmm_config *config)
> +{
> +	return 0;
> +}
> +#endif /* IS_ENABLED(CONFIG_DRM_RCAR_CMM) */
> +
> +#endif /* __RCAR_CMM_H__ */

-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 3/8] drm: rcar-du: Add support for CMM

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Oct 16, 2019 at 10:55:43AM +0200, Jacopo Mondi wrote:
> Add a driver for the R-Car Display Unit Color Correction Module.
> 
> In most of Gen3 SoCs, each DU output channel is provided with a CMM unit
> to perform image enhancement and color correction.
> 
> Add support for CMM through a driver that supports configuration of
> the 1-dimensional LUT table. More advanced CMM features will be
> implemented on top of this initial one.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/gpu/drm/rcar-du/Kconfig    |   7 +
>  drivers/gpu/drm/rcar-du/Makefile   |   1 +
>  drivers/gpu/drm/rcar-du/rcar_cmm.c | 212 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_cmm.h |  58 ++++++++
>  4 files changed, 278 insertions(+)
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h
> 
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index 1529849e217e..539d232790d1 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -13,6 +13,13 @@ config DRM_RCAR_DU
>  	  Choose this option if you have an R-Car chipset.
>  	  If M is selected the module will be called rcar-du-drm.
>  
> +config DRM_RCAR_CMM
> +	bool "R-Car DU Color Management Module (CMM) Support"
> +	depends on DRM && OF
> +	depends on DRM_RCAR_DU
> +	help
> +	  Enable support for R-Car Color Management Module (CMM).
> +
>  config DRM_RCAR_DW_HDMI
>  	tristate "R-Car DU Gen3 HDMI Encoder Support"
>  	depends on DRM && OF
> diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
> index 6c2ed9c46467..4d1187ccc3e5 100644
> --- a/drivers/gpu/drm/rcar-du/Makefile
> +++ b/drivers/gpu/drm/rcar-du/Makefile
> @@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
>  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
>  rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
>  
> +obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
>  obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
>  obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
>  obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
> diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> new file mode 100644
> index 000000000000..4170626208cf
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> @@ -0,0 +1,212 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * rcar_cmm.c -- R-Car Display Unit Color Management Module
> + *
> + * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
> + */
> +
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include <drm/drm_color_mgmt.h>
> +
> +#include "rcar_cmm.h"
> +
> +#define CM2_LUT_CTRL		0x0000
> +#define CM2_LUT_CTRL_LUT_EN	BIT(0)
> +#define CM2_LUT_TBL_BASE	0x0600
> +#define CM2_LUT_TBL(__i)	(CM2_LUT_TBL_BASE + (__i) * 4)
> +
> +struct rcar_cmm {
> +	void __iomem *base;
> +
> +	/*
> +	 * @lut:		1D-LUT state
> +	 * @lut.enabled:	1D-LUT enabled flag
> +	 */
> +	struct {
> +		bool enabled;
> +	} lut;
> +};
> +
> +static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg)
> +{
> +	return ioread32(rcmm->base + reg);
> +}
> +
> +static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 data)
> +{
> +	iowrite32(data, rcmm->base + reg);
> +}
> +
> +/*
> + * rcar_cmm_lut_write() - Scale the DRM LUT table entries to hardware precision
> + *			  and write to the CMM registers
> + * @rcmm: Pointer to the CMM device
> + * @drm_lut: Pointer to the DRM LUT table
> + */
> +static void rcar_cmm_lut_write(struct rcar_cmm *rcmm,
> +			       const struct drm_color_lut *drm_lut)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < CM2_LUT_SIZE; ++i) {
> +		u32 entry = drm_color_lut_extract(drm_lut[i].red, 8) << 16
> +			  | drm_color_lut_extract(drm_lut[i].green, 8) << 8
> +			  | drm_color_lut_extract(drm_lut[i].blue, 8);
> +
> +		rcar_cmm_write(rcmm, CM2_LUT_TBL(i), entry);
> +	}
> +}
> +
> +/*
> + * rcar_cmm_setup() - Configure the CMM unit
> + * @pdev: The platform device associated with the CMM instance
> + * @config: The CMM unit configuration
> + *
> + * Configure the CMM unit with the given configuration. Currently enabling,
> + * disabling and programming of the 1-D LUT unit is supported.

Did you miss the comment in the previous version about explaining when
rcar_cmm_setup() can be called (basically requiring the CMM to be
enabled) ? I can fix this when applying if you tell me what I should
write.

> + *
> + * TODO: Add support for LUT double buffer operations to avoid updating the
> + * LUT table entries while a frame is being displayed.
> + */
> +int rcar_cmm_setup(struct platform_device *pdev,
> +		   const struct rcar_cmm_config *config)
> +{
> +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> +
> +	/* Disable LUT if no table is provided. */
> +	if (!config->lut.table) {
> +		if (rcmm->lut.enabled) {
> +			rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
> +			rcmm->lut.enabled = false;
> +		}
> +
> +		return 0;
> +	}
> +
> +	/* Enable LUT and program the new gamma table values. */
> +	if (!rcmm->lut.enabled) {
> +		rcar_cmm_write(rcmm, CM2_LUT_CTRL, CM2_LUT_CTRL_LUT_EN);
> +		rcmm->lut.enabled = true;
> +	}
> +
> +	rcar_cmm_lut_write(rcmm, config->lut.table);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rcar_cmm_setup);
> +
> +/*
> + * rcar_cmm_enable() - Enable the CMM unit
> + * @pdev: The platform device associated with the CMM instance
> + *
> + * When the output of the corresponding DU channel is routed to the CMM unit,
> + * the unit shall be enabled before the DU channel is started, and remain
> + * enabled until the channel is stopped. The CMM unit shall be disabled with
> + * rcar_cmm_disable().
> + *
> + * Calls to rcar_cmm_enable() and rcar_cmm_disable() are not reference-counted.
> + * It is an error to attempt to enable an already enabled CMM unit, or to
> + * attempt to disable a disabled unit.
> + */
> +int rcar_cmm_enable(struct platform_device *pdev)
> +{
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(&pdev->dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rcar_cmm_enable);
> +
> +/*
> + * rcar_cmm_disable() - Disable the CMM unit
> + * @pdev: The platform device associated with the CMM instance
> + *
> + * See rcar_cmm_enable() for usage information.
> + *
> + * Disabling the CMM unit disable all the internal processing blocks. The CMM
> + * state shall thus be restored with rcar_cmm_setup() when re-enabling the CMM
> + * unit after the next rcar_cmm_enable() call.
> + */
> +void rcar_cmm_disable(struct platform_device *pdev)
> +{
> +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> +
> +	rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
> +	rcmm->lut.enabled = false;
> +
> +	pm_runtime_put(&pdev->dev);
> +}
> +EXPORT_SYMBOL_GPL(rcar_cmm_disable);
> +
> +/*
> + * rcar_cmm_init() - Initialize the CMM unit
> + * @pdev: The platform device associated with the CMM instance
> + *
> + * Return: 0 on success, -EPROBE_DEFER if the CMM is not available yet,
> + *         -ENODEV if the DRM_RCAR_CMM config option is disabled
> + */
> +int rcar_cmm_init(struct platform_device *pdev)
> +{
> +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> +
> +	if (!rcmm)
> +		return -EPROBE_DEFER;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rcar_cmm_init);
> +
> +static int rcar_cmm_probe(struct platform_device *pdev)
> +{
> +	struct rcar_cmm *rcmm;
> +
> +	rcmm = devm_kzalloc(&pdev->dev, sizeof(*rcmm), GFP_KERNEL);
> +	if (!rcmm)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, rcmm);
> +
> +	rcmm->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(rcmm->base))
> +		return PTR_ERR(rcmm->base);
> +
> +	pm_runtime_enable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static int rcar_cmm_remove(struct platform_device *pdev)
> +{
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rcar_cmm_of_table[] = {
> +	{ .compatible = "renesas,rcar-gen3-cmm", },
> +	{ .compatible = "renesas,rcar-gen2-cmm", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, rcar_cmm_of_table);
> +
> +static struct platform_driver rcar_cmm_platform_driver = {
> +	.probe		= rcar_cmm_probe,
> +	.remove		= rcar_cmm_remove,
> +	.driver		= {
> +		.name	= "rcar-cmm",
> +		.of_match_table = rcar_cmm_of_table,
> +	},
> +};
> +
> +module_platform_driver(rcar_cmm_platform_driver);
> +
> +MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas@jmondi.org>");
> +MODULE_DESCRIPTION("Renesas R-Car CMM Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.h b/drivers/gpu/drm/rcar-du/rcar_cmm.h
> new file mode 100644
> index 000000000000..b5f7ec6db04a
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.h
> @@ -0,0 +1,58 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * rcar_cmm.h -- R-Car Display Unit Color Management Module
> + *
> + * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
> + */
> +
> +#ifndef __RCAR_CMM_H__
> +#define __RCAR_CMM_H__
> +
> +#define CM2_LUT_SIZE		256
> +
> +struct drm_color_lut;
> +struct platform_device;
> +
> +/**
> + * struct rcar_cmm_config - CMM configuration
> + *
> + * @lut:	1D-LUT configuration
> + * @lut.table:	1D-LUT table entries. Disable LUT operations when NULL
> + */
> +struct rcar_cmm_config {
> +	struct {
> +		struct drm_color_lut *table;
> +	} lut;
> +};
> +
> +#if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
> +int rcar_cmm_init(struct platform_device *pdev);
> +
> +int rcar_cmm_enable(struct platform_device *pdev);
> +void rcar_cmm_disable(struct platform_device *pdev);
> +
> +int rcar_cmm_setup(struct platform_device *pdev,
> +		   const struct rcar_cmm_config *config);
> +#else
> +static inline int rcar_cmm_init(struct platform_device *pdev)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline int rcar_cmm_enable(struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +
> +static inline void rcar_cmm_disable(struct platform_device *pdev)
> +{
> +}
> +
> +static inline int rcar_cmm_setup(struct platform_device *pdev,
> +				 const struct rcar_cmm_config *config)
> +{
> +	return 0;
> +}
> +#endif /* IS_ENABLED(CONFIG_DRM_RCAR_CMM) */
> +
> +#endif /* __RCAR_CMM_H__ */

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v6 3/8] drm: rcar-du: Add support for CMM

[Jacopo Mondi]

[-- Attachment #1: Type: text/plain, Size: 11539 bytes --]

Hi Laurent,

On Wed, Oct 16, 2019 at 04:45:26PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, Oct 16, 2019 at 10:55:43AM +0200, Jacopo Mondi wrote:
> > Add a driver for the R-Car Display Unit Color Correction Module.
> >
> > In most of Gen3 SoCs, each DU output channel is provided with a CMM unit
> > to perform image enhancement and color correction.
> >
> > Add support for CMM through a driver that supports configuration of
> > the 1-dimensional LUT table. More advanced CMM features will be
> > implemented on top of this initial one.
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  drivers/gpu/drm/rcar-du/Kconfig    |   7 +
> >  drivers/gpu/drm/rcar-du/Makefile   |   1 +
> >  drivers/gpu/drm/rcar-du/rcar_cmm.c | 212 +++++++++++++++++++++++++++++
> >  drivers/gpu/drm/rcar-du/rcar_cmm.h |  58 ++++++++
> >  4 files changed, 278 insertions(+)
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h
> >
> > diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> > index 1529849e217e..539d232790d1 100644
> > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > @@ -13,6 +13,13 @@ config DRM_RCAR_DU
> >  	  Choose this option if you have an R-Car chipset.
> >  	  If M is selected the module will be called rcar-du-drm.
> >
> > +config DRM_RCAR_CMM
> > +	bool "R-Car DU Color Management Module (CMM) Support"
> > +	depends on DRM && OF
> > +	depends on DRM_RCAR_DU
> > +	help
> > +	  Enable support for R-Car Color Management Module (CMM).
> > +
> >  config DRM_RCAR_DW_HDMI
> >  	tristate "R-Car DU Gen3 HDMI Encoder Support"
> >  	depends on DRM && OF
> > diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
> > index 6c2ed9c46467..4d1187ccc3e5 100644
> > --- a/drivers/gpu/drm/rcar-du/Makefile
> > +++ b/drivers/gpu/drm/rcar-du/Makefile
> > @@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
> >  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
> >  rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
> >
> > +obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
> >  obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
> >  obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
> >  obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > new file mode 100644
> > index 000000000000..4170626208cf
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > @@ -0,0 +1,212 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * rcar_cmm.c -- R-Car Display Unit Color Management Module
> > + *
> > + * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +
> > +#include <drm/drm_color_mgmt.h>
> > +
> > +#include "rcar_cmm.h"
> > +
> > +#define CM2_LUT_CTRL		0x0000
> > +#define CM2_LUT_CTRL_LUT_EN	BIT(0)
> > +#define CM2_LUT_TBL_BASE	0x0600
> > +#define CM2_LUT_TBL(__i)	(CM2_LUT_TBL_BASE + (__i) * 4)
> > +
> > +struct rcar_cmm {
> > +	void __iomem *base;
> > +
> > +	/*
> > +	 * @lut:		1D-LUT state
> > +	 * @lut.enabled:	1D-LUT enabled flag
> > +	 */
> > +	struct {
> > +		bool enabled;
> > +	} lut;
> > +};
> > +
> > +static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg)
> > +{
> > +	return ioread32(rcmm->base + reg);
> > +}
> > +
> > +static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 data)
> > +{
> > +	iowrite32(data, rcmm->base + reg);
> > +}
> > +
> > +/*
> > + * rcar_cmm_lut_write() - Scale the DRM LUT table entries to hardware precision
> > + *			  and write to the CMM registers
> > + * @rcmm: Pointer to the CMM device
> > + * @drm_lut: Pointer to the DRM LUT table
> > + */
> > +static void rcar_cmm_lut_write(struct rcar_cmm *rcmm,
> > +			       const struct drm_color_lut *drm_lut)
> > +{
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < CM2_LUT_SIZE; ++i) {
> > +		u32 entry = drm_color_lut_extract(drm_lut[i].red, 8) << 16
> > +			  | drm_color_lut_extract(drm_lut[i].green, 8) << 8
> > +			  | drm_color_lut_extract(drm_lut[i].blue, 8);
> > +
> > +		rcar_cmm_write(rcmm, CM2_LUT_TBL(i), entry);
> > +	}
> > +}
> > +
> > +/*
> > + * rcar_cmm_setup() - Configure the CMM unit
> > + * @pdev: The platform device associated with the CMM instance
> > + * @config: The CMM unit configuration
> > + *
> > + * Configure the CMM unit with the given configuration. Currently enabling,
> > + * disabling and programming of the 1-D LUT unit is supported.
>
> Did you miss the comment in the previous version about explaining when
> rcar_cmm_setup() can be called (basically requiring the CMM to be
> enabled) ? I can fix this when applying if you tell me what I should
> write.
>

No I didn't.

As the DU is the only user of this function, I don't get why we should
specify here which is the order of the calls, as they are performed by
the crct driver in the right order already.

Please feel free to add whatever you consider appropriate to this
comment section when applying.

Thanks
   j

> > + *
> > + * TODO: Add support for LUT double buffer operations to avoid updating the
> > + * LUT table entries while a frame is being displayed.
> > + */
> > +int rcar_cmm_setup(struct platform_device *pdev,
> > +		   const struct rcar_cmm_config *config)
> > +{
> > +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> > +
> > +	/* Disable LUT if no table is provided. */
> > +	if (!config->lut.table) {
> > +		if (rcmm->lut.enabled) {
> > +			rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
> > +			rcmm->lut.enabled = false;
> > +		}
> > +
> > +		return 0;
> > +	}
> > +
> > +	/* Enable LUT and program the new gamma table values. */
> > +	if (!rcmm->lut.enabled) {
> > +		rcar_cmm_write(rcmm, CM2_LUT_CTRL, CM2_LUT_CTRL_LUT_EN);
> > +		rcmm->lut.enabled = true;
> > +	}
> > +
> > +	rcar_cmm_lut_write(rcmm, config->lut.table);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(rcar_cmm_setup);
> > +
> > +/*
> > + * rcar_cmm_enable() - Enable the CMM unit
> > + * @pdev: The platform device associated with the CMM instance
> > + *
> > + * When the output of the corresponding DU channel is routed to the CMM unit,
> > + * the unit shall be enabled before the DU channel is started, and remain
> > + * enabled until the channel is stopped. The CMM unit shall be disabled with
> > + * rcar_cmm_disable().
> > + *
> > + * Calls to rcar_cmm_enable() and rcar_cmm_disable() are not reference-counted.
> > + * It is an error to attempt to enable an already enabled CMM unit, or to
> > + * attempt to disable a disabled unit.
> > + */
> > +int rcar_cmm_enable(struct platform_device *pdev)
> > +{
> > +	int ret;
> > +
> > +	ret = pm_runtime_get_sync(&pdev->dev);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(rcar_cmm_enable);
> > +
> > +/*
> > + * rcar_cmm_disable() - Disable the CMM unit
> > + * @pdev: The platform device associated with the CMM instance
> > + *
> > + * See rcar_cmm_enable() for usage information.
> > + *
> > + * Disabling the CMM unit disable all the internal processing blocks. The CMM
> > + * state shall thus be restored with rcar_cmm_setup() when re-enabling the CMM
> > + * unit after the next rcar_cmm_enable() call.
> > + */
> > +void rcar_cmm_disable(struct platform_device *pdev)
> > +{
> > +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> > +
> > +	rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
> > +	rcmm->lut.enabled = false;
> > +
> > +	pm_runtime_put(&pdev->dev);
> > +}
> > +EXPORT_SYMBOL_GPL(rcar_cmm_disable);
> > +
> > +/*
> > + * rcar_cmm_init() - Initialize the CMM unit
> > + * @pdev: The platform device associated with the CMM instance
> > + *
> > + * Return: 0 on success, -EPROBE_DEFER if the CMM is not available yet,
> > + *         -ENODEV if the DRM_RCAR_CMM config option is disabled
> > + */
> > +int rcar_cmm_init(struct platform_device *pdev)
> > +{
> > +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> > +
> > +	if (!rcmm)
> > +		return -EPROBE_DEFER;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(rcar_cmm_init);
> > +
> > +static int rcar_cmm_probe(struct platform_device *pdev)
> > +{
> > +	struct rcar_cmm *rcmm;
> > +
> > +	rcmm = devm_kzalloc(&pdev->dev, sizeof(*rcmm), GFP_KERNEL);
> > +	if (!rcmm)
> > +		return -ENOMEM;
> > +	platform_set_drvdata(pdev, rcmm);
> > +
> > +	rcmm->base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(rcmm->base))
> > +		return PTR_ERR(rcmm->base);
> > +
> > +	pm_runtime_enable(&pdev->dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static int rcar_cmm_remove(struct platform_device *pdev)
> > +{
> > +	pm_runtime_disable(&pdev->dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id rcar_cmm_of_table[] = {
> > +	{ .compatible = "renesas,rcar-gen3-cmm", },
> > +	{ .compatible = "renesas,rcar-gen2-cmm", },
> > +	{ },
> > +};
> > +MODULE_DEVICE_TABLE(of, rcar_cmm_of_table);
> > +
> > +static struct platform_driver rcar_cmm_platform_driver = {
> > +	.probe		= rcar_cmm_probe,
> > +	.remove		= rcar_cmm_remove,
> > +	.driver		= {
> > +		.name	= "rcar-cmm",
> > +		.of_match_table = rcar_cmm_of_table,
> > +	},
> > +};
> > +
> > +module_platform_driver(rcar_cmm_platform_driver);
> > +
> > +MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas@jmondi.org>");
> > +MODULE_DESCRIPTION("Renesas R-Car CMM Driver");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.h b/drivers/gpu/drm/rcar-du/rcar_cmm.h
> > new file mode 100644
> > index 000000000000..b5f7ec6db04a
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.h
> > @@ -0,0 +1,58 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * rcar_cmm.h -- R-Car Display Unit Color Management Module
> > + *
> > + * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
> > + */
> > +
> > +#ifndef __RCAR_CMM_H__
> > +#define __RCAR_CMM_H__
> > +
> > +#define CM2_LUT_SIZE		256
> > +
> > +struct drm_color_lut;
> > +struct platform_device;
> > +
> > +/**
> > + * struct rcar_cmm_config - CMM configuration
> > + *
> > + * @lut:	1D-LUT configuration
> > + * @lut.table:	1D-LUT table entries. Disable LUT operations when NULL
> > + */
> > +struct rcar_cmm_config {
> > +	struct {
> > +		struct drm_color_lut *table;
> > +	} lut;
> > +};
> > +
> > +#if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
> > +int rcar_cmm_init(struct platform_device *pdev);
> > +
> > +int rcar_cmm_enable(struct platform_device *pdev);
> > +void rcar_cmm_disable(struct platform_device *pdev);
> > +
> > +int rcar_cmm_setup(struct platform_device *pdev,
> > +		   const struct rcar_cmm_config *config);
> > +#else
> > +static inline int rcar_cmm_init(struct platform_device *pdev)
> > +{
> > +	return -ENODEV;
> > +}
> > +
> > +static inline int rcar_cmm_enable(struct platform_device *pdev)
> > +{
> > +	return 0;
> > +}
> > +
> > +static inline void rcar_cmm_disable(struct platform_device *pdev)
> > +{
> > +}
> > +
> > +static inline int rcar_cmm_setup(struct platform_device *pdev,
> > +				 const struct rcar_cmm_config *config)
> > +{
> > +	return 0;
> > +}
> > +#endif /* IS_ENABLED(CONFIG_DRM_RCAR_CMM) */
> > +
> > +#endif /* __RCAR_CMM_H__ */
>
> --
> Regards,
>
> Laurent Pinchart

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v6 3/8] drm: rcar-du: Add support for CMM

[Laurent Pinchart]

Hi Jacopo,

On Thu, Oct 17, 2019 at 02:43:49PM +0200, Jacopo Mondi wrote:
> On Wed, Oct 16, 2019 at 04:45:26PM +0300, Laurent Pinchart wrote:
> > On Wed, Oct 16, 2019 at 10:55:43AM +0200, Jacopo Mondi wrote:
> > > Add a driver for the R-Car Display Unit Color Correction Module.
> > >
> > > In most of Gen3 SoCs, each DU output channel is provided with a CMM unit
> > > to perform image enhancement and color correction.
> > >
> > > Add support for CMM through a driver that supports configuration of
> > > the 1-dimensional LUT table. More advanced CMM features will be
> > > implemented on top of this initial one.
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  drivers/gpu/drm/rcar-du/Kconfig    |   7 +
> > >  drivers/gpu/drm/rcar-du/Makefile   |   1 +
> > >  drivers/gpu/drm/rcar-du/rcar_cmm.c | 212 +++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/rcar-du/rcar_cmm.h |  58 ++++++++
> > >  4 files changed, 278 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
> > >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h
> > >
> > > diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> > > index 1529849e217e..539d232790d1 100644
> > > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > > @@ -13,6 +13,13 @@ config DRM_RCAR_DU
> > >  	  Choose this option if you have an R-Car chipset.
> > >  	  If M is selected the module will be called rcar-du-drm.
> > >
> > > +config DRM_RCAR_CMM
> > > +	bool "R-Car DU Color Management Module (CMM) Support"
> > > +	depends on DRM && OF
> > > +	depends on DRM_RCAR_DU
> > > +	help
> > > +	  Enable support for R-Car Color Management Module (CMM).
> > > +
> > >  config DRM_RCAR_DW_HDMI
> > >  	tristate "R-Car DU Gen3 HDMI Encoder Support"
> > >  	depends on DRM && OF
> > > diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
> > > index 6c2ed9c46467..4d1187ccc3e5 100644
> > > --- a/drivers/gpu/drm/rcar-du/Makefile
> > > +++ b/drivers/gpu/drm/rcar-du/Makefile
> > > @@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
> > >  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
> > >  rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
> > >
> > > +obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
> > >  obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
> > >  obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
> > >  obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > > new file mode 100644
> > > index 000000000000..4170626208cf
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > > @@ -0,0 +1,212 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * rcar_cmm.c -- R-Car Display Unit Color Management Module
> > > + *
> > > + * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > + */
> > > +
> > > +#include <linux/io.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/pm_runtime.h>
> > > +
> > > +#include <drm/drm_color_mgmt.h>
> > > +
> > > +#include "rcar_cmm.h"
> > > +
> > > +#define CM2_LUT_CTRL		0x0000
> > > +#define CM2_LUT_CTRL_LUT_EN	BIT(0)
> > > +#define CM2_LUT_TBL_BASE	0x0600
> > > +#define CM2_LUT_TBL(__i)	(CM2_LUT_TBL_BASE + (__i) * 4)
> > > +
> > > +struct rcar_cmm {
> > > +	void __iomem *base;
> > > +
> > > +	/*
> > > +	 * @lut:		1D-LUT state
> > > +	 * @lut.enabled:	1D-LUT enabled flag
> > > +	 */
> > > +	struct {
> > > +		bool enabled;
> > > +	} lut;
> > > +};
> > > +
> > > +static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg)
> > > +{
> > > +	return ioread32(rcmm->base + reg);
> > > +}
> > > +
> > > +static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 data)
> > > +{
> > > +	iowrite32(data, rcmm->base + reg);
> > > +}
> > > +
> > > +/*
> > > + * rcar_cmm_lut_write() - Scale the DRM LUT table entries to hardware precision
> > > + *			  and write to the CMM registers
> > > + * @rcmm: Pointer to the CMM device
> > > + * @drm_lut: Pointer to the DRM LUT table
> > > + */
> > > +static void rcar_cmm_lut_write(struct rcar_cmm *rcmm,
> > > +			       const struct drm_color_lut *drm_lut)
> > > +{
> > > +	unsigned int i;
> > > +
> > > +	for (i = 0; i < CM2_LUT_SIZE; ++i) {
> > > +		u32 entry = drm_color_lut_extract(drm_lut[i].red, 8) << 16
> > > +			  | drm_color_lut_extract(drm_lut[i].green, 8) << 8
> > > +			  | drm_color_lut_extract(drm_lut[i].blue, 8);
> > > +
> > > +		rcar_cmm_write(rcmm, CM2_LUT_TBL(i), entry);
> > > +	}
> > > +}
> > > +
> > > +/*
> > > + * rcar_cmm_setup() - Configure the CMM unit
> > > + * @pdev: The platform device associated with the CMM instance
> > > + * @config: The CMM unit configuration
> > > + *
> > > + * Configure the CMM unit with the given configuration. Currently enabling,
> > > + * disabling and programming of the 1-D LUT unit is supported.
> >
> > Did you miss the comment in the previous version about explaining when
> > rcar_cmm_setup() can be called (basically requiring the CMM to be
> > enabled) ? I can fix this when applying if you tell me what I should
> > write.
> 
> No I didn't.
> 
> As the DU is the only user of this function, I don't get why we should
> specify here which is the order of the calls, as they are performed by
> the crct driver in the right order already.

I think it's important as we're dealing with two different drivers. With
proper API documentation working on the DU driver won't require having
active knowledge of the CMM driver design, and vice-versa. Only when
changing API in a way that would break the contract would we need to
carefully study both sides. It's easy today, the documentation is for in
a few months when this won't be fresh in your memory anymore :-)

> Please feel free to add whatever you consider appropriate to this
> comment section when applying.
> 
> > > + *
> > > + * TODO: Add support for LUT double buffer operations to avoid updating the
> > > + * LUT table entries while a frame is being displayed.
> > > + */
> > > +int rcar_cmm_setup(struct platform_device *pdev,
> > > +		   const struct rcar_cmm_config *config)
> > > +{
> > > +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> > > +
> > > +	/* Disable LUT if no table is provided. */
> > > +	if (!config->lut.table) {
> > > +		if (rcmm->lut.enabled) {
> > > +			rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
> > > +			rcmm->lut.enabled = false;
> > > +		}
> > > +
> > > +		return 0;
> > > +	}
> > > +
> > > +	/* Enable LUT and program the new gamma table values. */
> > > +	if (!rcmm->lut.enabled) {
> > > +		rcar_cmm_write(rcmm, CM2_LUT_CTRL, CM2_LUT_CTRL_LUT_EN);
> > > +		rcmm->lut.enabled = true;
> > > +	}
> > > +
> > > +	rcar_cmm_lut_write(rcmm, config->lut.table);
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(rcar_cmm_setup);
> > > +
> > > +/*
> > > + * rcar_cmm_enable() - Enable the CMM unit
> > > + * @pdev: The platform device associated with the CMM instance
> > > + *
> > > + * When the output of the corresponding DU channel is routed to the CMM unit,
> > > + * the unit shall be enabled before the DU channel is started, and remain
> > > + * enabled until the channel is stopped. The CMM unit shall be disabled with
> > > + * rcar_cmm_disable().
> > > + *
> > > + * Calls to rcar_cmm_enable() and rcar_cmm_disable() are not reference-counted.
> > > + * It is an error to attempt to enable an already enabled CMM unit, or to
> > > + * attempt to disable a disabled unit.
> > > + */
> > > +int rcar_cmm_enable(struct platform_device *pdev)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = pm_runtime_get_sync(&pdev->dev);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(rcar_cmm_enable);
> > > +
> > > +/*
> > > + * rcar_cmm_disable() - Disable the CMM unit
> > > + * @pdev: The platform device associated with the CMM instance
> > > + *
> > > + * See rcar_cmm_enable() for usage information.
> > > + *
> > > + * Disabling the CMM unit disable all the internal processing blocks. The CMM
> > > + * state shall thus be restored with rcar_cmm_setup() when re-enabling the CMM
> > > + * unit after the next rcar_cmm_enable() call.
> > > + */
> > > +void rcar_cmm_disable(struct platform_device *pdev)
> > > +{
> > > +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> > > +
> > > +	rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
> > > +	rcmm->lut.enabled = false;
> > > +
> > > +	pm_runtime_put(&pdev->dev);
> > > +}
> > > +EXPORT_SYMBOL_GPL(rcar_cmm_disable);
> > > +
> > > +/*
> > > + * rcar_cmm_init() - Initialize the CMM unit
> > > + * @pdev: The platform device associated with the CMM instance
> > > + *
> > > + * Return: 0 on success, -EPROBE_DEFER if the CMM is not available yet,
> > > + *         -ENODEV if the DRM_RCAR_CMM config option is disabled
> > > + */
> > > +int rcar_cmm_init(struct platform_device *pdev)
> > > +{
> > > +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> > > +
> > > +	if (!rcmm)
> > > +		return -EPROBE_DEFER;
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(rcar_cmm_init);
> > > +
> > > +static int rcar_cmm_probe(struct platform_device *pdev)
> > > +{
> > > +	struct rcar_cmm *rcmm;
> > > +
> > > +	rcmm = devm_kzalloc(&pdev->dev, sizeof(*rcmm), GFP_KERNEL);
> > > +	if (!rcmm)
> > > +		return -ENOMEM;
> > > +	platform_set_drvdata(pdev, rcmm);
> > > +
> > > +	rcmm->base = devm_platform_ioremap_resource(pdev, 0);
> > > +	if (IS_ERR(rcmm->base))
> > > +		return PTR_ERR(rcmm->base);
> > > +
> > > +	pm_runtime_enable(&pdev->dev);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int rcar_cmm_remove(struct platform_device *pdev)
> > > +{
> > > +	pm_runtime_disable(&pdev->dev);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static const struct of_device_id rcar_cmm_of_table[] = {
> > > +	{ .compatible = "renesas,rcar-gen3-cmm", },
> > > +	{ .compatible = "renesas,rcar-gen2-cmm", },
> > > +	{ },
> > > +};
> > > +MODULE_DEVICE_TABLE(of, rcar_cmm_of_table);
> > > +
> > > +static struct platform_driver rcar_cmm_platform_driver = {
> > > +	.probe		= rcar_cmm_probe,
> > > +	.remove		= rcar_cmm_remove,
> > > +	.driver		= {
> > > +		.name	= "rcar-cmm",
> > > +		.of_match_table = rcar_cmm_of_table,
> > > +	},
> > > +};
> > > +
> > > +module_platform_driver(rcar_cmm_platform_driver);
> > > +
> > > +MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas@jmondi.org>");
> > > +MODULE_DESCRIPTION("Renesas R-Car CMM Driver");
> > > +MODULE_LICENSE("GPL v2");
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.h b/drivers/gpu/drm/rcar-du/rcar_cmm.h
> > > new file mode 100644
> > > index 000000000000..b5f7ec6db04a
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.h
> > > @@ -0,0 +1,58 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * rcar_cmm.h -- R-Car Display Unit Color Management Module
> > > + *
> > > + * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > + */
> > > +
> > > +#ifndef __RCAR_CMM_H__
> > > +#define __RCAR_CMM_H__
> > > +
> > > +#define CM2_LUT_SIZE		256
> > > +
> > > +struct drm_color_lut;
> > > +struct platform_device;
> > > +
> > > +/**
> > > + * struct rcar_cmm_config - CMM configuration
> > > + *
> > > + * @lut:	1D-LUT configuration
> > > + * @lut.table:	1D-LUT table entries. Disable LUT operations when NULL
> > > + */
> > > +struct rcar_cmm_config {
> > > +	struct {
> > > +		struct drm_color_lut *table;
> > > +	} lut;
> > > +};
> > > +
> > > +#if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
> > > +int rcar_cmm_init(struct platform_device *pdev);
> > > +
> > > +int rcar_cmm_enable(struct platform_device *pdev);
> > > +void rcar_cmm_disable(struct platform_device *pdev);
> > > +
> > > +int rcar_cmm_setup(struct platform_device *pdev,
> > > +		   const struct rcar_cmm_config *config);
> > > +#else
> > > +static inline int rcar_cmm_init(struct platform_device *pdev)
> > > +{
> > > +	return -ENODEV;
> > > +}
> > > +
> > > +static inline int rcar_cmm_enable(struct platform_device *pdev)
> > > +{
> > > +	return 0;
> > > +}
> > > +
> > > +static inline void rcar_cmm_disable(struct platform_device *pdev)
> > > +{
> > > +}
> > > +
> > > +static inline int rcar_cmm_setup(struct platform_device *pdev,
> > > +				 const struct rcar_cmm_config *config)
> > > +{
> > > +	return 0;
> > > +}
> > > +#endif /* IS_ENABLED(CONFIG_DRM_RCAR_CMM) */
> > > +
> > > +#endif /* __RCAR_CMM_H__ */

-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 3/8] drm: rcar-du: Add support for CMM

[Laurent Pinchart]

Hi Jacopo,

On Thu, Oct 17, 2019 at 02:43:49PM +0200, Jacopo Mondi wrote:
> On Wed, Oct 16, 2019 at 04:45:26PM +0300, Laurent Pinchart wrote:
> > On Wed, Oct 16, 2019 at 10:55:43AM +0200, Jacopo Mondi wrote:
> > > Add a driver for the R-Car Display Unit Color Correction Module.
> > >
> > > In most of Gen3 SoCs, each DU output channel is provided with a CMM unit
> > > to perform image enhancement and color correction.
> > >
> > > Add support for CMM through a driver that supports configuration of
> > > the 1-dimensional LUT table. More advanced CMM features will be
> > > implemented on top of this initial one.
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  drivers/gpu/drm/rcar-du/Kconfig    |   7 +
> > >  drivers/gpu/drm/rcar-du/Makefile   |   1 +
> > >  drivers/gpu/drm/rcar-du/rcar_cmm.c | 212 +++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/rcar-du/rcar_cmm.h |  58 ++++++++
> > >  4 files changed, 278 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
> > >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h
> > >
> > > diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> > > index 1529849e217e..539d232790d1 100644
> > > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > > @@ -13,6 +13,13 @@ config DRM_RCAR_DU
> > >  	  Choose this option if you have an R-Car chipset.
> > >  	  If M is selected the module will be called rcar-du-drm.
> > >
> > > +config DRM_RCAR_CMM
> > > +	bool "R-Car DU Color Management Module (CMM) Support"
> > > +	depends on DRM && OF
> > > +	depends on DRM_RCAR_DU
> > > +	help
> > > +	  Enable support for R-Car Color Management Module (CMM).
> > > +
> > >  config DRM_RCAR_DW_HDMI
> > >  	tristate "R-Car DU Gen3 HDMI Encoder Support"
> > >  	depends on DRM && OF
> > > diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
> > > index 6c2ed9c46467..4d1187ccc3e5 100644
> > > --- a/drivers/gpu/drm/rcar-du/Makefile
> > > +++ b/drivers/gpu/drm/rcar-du/Makefile
> > > @@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
> > >  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
> > >  rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
> > >
> > > +obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
> > >  obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
> > >  obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
> > >  obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > > new file mode 100644
> > > index 000000000000..4170626208cf
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > > @@ -0,0 +1,212 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * rcar_cmm.c -- R-Car Display Unit Color Management Module
> > > + *
> > > + * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > + */
> > > +
> > > +#include <linux/io.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/pm_runtime.h>
> > > +
> > > +#include <drm/drm_color_mgmt.h>
> > > +
> > > +#include "rcar_cmm.h"
> > > +
> > > +#define CM2_LUT_CTRL		0x0000
> > > +#define CM2_LUT_CTRL_LUT_EN	BIT(0)
> > > +#define CM2_LUT_TBL_BASE	0x0600
> > > +#define CM2_LUT_TBL(__i)	(CM2_LUT_TBL_BASE + (__i) * 4)
> > > +
> > > +struct rcar_cmm {
> > > +	void __iomem *base;
> > > +
> > > +	/*
> > > +	 * @lut:		1D-LUT state
> > > +	 * @lut.enabled:	1D-LUT enabled flag
> > > +	 */
> > > +	struct {
> > > +		bool enabled;
> > > +	} lut;
> > > +};
> > > +
> > > +static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg)
> > > +{
> > > +	return ioread32(rcmm->base + reg);
> > > +}
> > > +
> > > +static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 data)
> > > +{
> > > +	iowrite32(data, rcmm->base + reg);
> > > +}
> > > +
> > > +/*
> > > + * rcar_cmm_lut_write() - Scale the DRM LUT table entries to hardware precision
> > > + *			  and write to the CMM registers
> > > + * @rcmm: Pointer to the CMM device
> > > + * @drm_lut: Pointer to the DRM LUT table
> > > + */
> > > +static void rcar_cmm_lut_write(struct rcar_cmm *rcmm,
> > > +			       const struct drm_color_lut *drm_lut)
> > > +{
> > > +	unsigned int i;
> > > +
> > > +	for (i = 0; i < CM2_LUT_SIZE; ++i) {
> > > +		u32 entry = drm_color_lut_extract(drm_lut[i].red, 8) << 16
> > > +			  | drm_color_lut_extract(drm_lut[i].green, 8) << 8
> > > +			  | drm_color_lut_extract(drm_lut[i].blue, 8);
> > > +
> > > +		rcar_cmm_write(rcmm, CM2_LUT_TBL(i), entry);
> > > +	}
> > > +}
> > > +
> > > +/*
> > > + * rcar_cmm_setup() - Configure the CMM unit
> > > + * @pdev: The platform device associated with the CMM instance
> > > + * @config: The CMM unit configuration
> > > + *
> > > + * Configure the CMM unit with the given configuration. Currently enabling,
> > > + * disabling and programming of the 1-D LUT unit is supported.
> >
> > Did you miss the comment in the previous version about explaining when
> > rcar_cmm_setup() can be called (basically requiring the CMM to be
> > enabled) ? I can fix this when applying if you tell me what I should
> > write.
> 
> No I didn't.
> 
> As the DU is the only user of this function, I don't get why we should
> specify here which is the order of the calls, as they are performed by
> the crct driver in the right order already.

I think it's important as we're dealing with two different drivers. With
proper API documentation working on the DU driver won't require having
active knowledge of the CMM driver design, and vice-versa. Only when
changing API in a way that would break the contract would we need to
carefully study both sides. It's easy today, the documentation is for in
a few months when this won't be fresh in your memory anymore :-)

> Please feel free to add whatever you consider appropriate to this
> comment section when applying.
> 
> > > + *
> > > + * TODO: Add support for LUT double buffer operations to avoid updating the
> > > + * LUT table entries while a frame is being displayed.
> > > + */
> > > +int rcar_cmm_setup(struct platform_device *pdev,
> > > +		   const struct rcar_cmm_config *config)
> > > +{
> > > +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> > > +
> > > +	/* Disable LUT if no table is provided. */
> > > +	if (!config->lut.table) {
> > > +		if (rcmm->lut.enabled) {
> > > +			rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
> > > +			rcmm->lut.enabled = false;
> > > +		}
> > > +
> > > +		return 0;
> > > +	}
> > > +
> > > +	/* Enable LUT and program the new gamma table values. */
> > > +	if (!rcmm->lut.enabled) {
> > > +		rcar_cmm_write(rcmm, CM2_LUT_CTRL, CM2_LUT_CTRL_LUT_EN);
> > > +		rcmm->lut.enabled = true;
> > > +	}
> > > +
> > > +	rcar_cmm_lut_write(rcmm, config->lut.table);
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(rcar_cmm_setup);
> > > +
> > > +/*
> > > + * rcar_cmm_enable() - Enable the CMM unit
> > > + * @pdev: The platform device associated with the CMM instance
> > > + *
> > > + * When the output of the corresponding DU channel is routed to the CMM unit,
> > > + * the unit shall be enabled before the DU channel is started, and remain
> > > + * enabled until the channel is stopped. The CMM unit shall be disabled with
> > > + * rcar_cmm_disable().
> > > + *
> > > + * Calls to rcar_cmm_enable() and rcar_cmm_disable() are not reference-counted.
> > > + * It is an error to attempt to enable an already enabled CMM unit, or to
> > > + * attempt to disable a disabled unit.
> > > + */
> > > +int rcar_cmm_enable(struct platform_device *pdev)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = pm_runtime_get_sync(&pdev->dev);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(rcar_cmm_enable);
> > > +
> > > +/*
> > > + * rcar_cmm_disable() - Disable the CMM unit
> > > + * @pdev: The platform device associated with the CMM instance
> > > + *
> > > + * See rcar_cmm_enable() for usage information.
> > > + *
> > > + * Disabling the CMM unit disable all the internal processing blocks. The CMM
> > > + * state shall thus be restored with rcar_cmm_setup() when re-enabling the CMM
> > > + * unit after the next rcar_cmm_enable() call.
> > > + */
> > > +void rcar_cmm_disable(struct platform_device *pdev)
> > > +{
> > > +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> > > +
> > > +	rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
> > > +	rcmm->lut.enabled = false;
> > > +
> > > +	pm_runtime_put(&pdev->dev);
> > > +}
> > > +EXPORT_SYMBOL_GPL(rcar_cmm_disable);
> > > +
> > > +/*
> > > + * rcar_cmm_init() - Initialize the CMM unit
> > > + * @pdev: The platform device associated with the CMM instance
> > > + *
> > > + * Return: 0 on success, -EPROBE_DEFER if the CMM is not available yet,
> > > + *         -ENODEV if the DRM_RCAR_CMM config option is disabled
> > > + */
> > > +int rcar_cmm_init(struct platform_device *pdev)
> > > +{
> > > +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> > > +
> > > +	if (!rcmm)
> > > +		return -EPROBE_DEFER;
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(rcar_cmm_init);
> > > +
> > > +static int rcar_cmm_probe(struct platform_device *pdev)
> > > +{
> > > +	struct rcar_cmm *rcmm;
> > > +
> > > +	rcmm = devm_kzalloc(&pdev->dev, sizeof(*rcmm), GFP_KERNEL);
> > > +	if (!rcmm)
> > > +		return -ENOMEM;
> > > +	platform_set_drvdata(pdev, rcmm);
> > > +
> > > +	rcmm->base = devm_platform_ioremap_resource(pdev, 0);
> > > +	if (IS_ERR(rcmm->base))
> > > +		return PTR_ERR(rcmm->base);
> > > +
> > > +	pm_runtime_enable(&pdev->dev);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int rcar_cmm_remove(struct platform_device *pdev)
> > > +{
> > > +	pm_runtime_disable(&pdev->dev);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static const struct of_device_id rcar_cmm_of_table[] = {
> > > +	{ .compatible = "renesas,rcar-gen3-cmm", },
> > > +	{ .compatible = "renesas,rcar-gen2-cmm", },
> > > +	{ },
> > > +};
> > > +MODULE_DEVICE_TABLE(of, rcar_cmm_of_table);
> > > +
> > > +static struct platform_driver rcar_cmm_platform_driver = {
> > > +	.probe		= rcar_cmm_probe,
> > > +	.remove		= rcar_cmm_remove,
> > > +	.driver		= {
> > > +		.name	= "rcar-cmm",
> > > +		.of_match_table = rcar_cmm_of_table,
> > > +	},
> > > +};
> > > +
> > > +module_platform_driver(rcar_cmm_platform_driver);
> > > +
> > > +MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas@jmondi.org>");
> > > +MODULE_DESCRIPTION("Renesas R-Car CMM Driver");
> > > +MODULE_LICENSE("GPL v2");
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.h b/drivers/gpu/drm/rcar-du/rcar_cmm.h
> > > new file mode 100644
> > > index 000000000000..b5f7ec6db04a
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.h
> > > @@ -0,0 +1,58 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * rcar_cmm.h -- R-Car Display Unit Color Management Module
> > > + *
> > > + * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > + */
> > > +
> > > +#ifndef __RCAR_CMM_H__
> > > +#define __RCAR_CMM_H__
> > > +
> > > +#define CM2_LUT_SIZE		256
> > > +
> > > +struct drm_color_lut;
> > > +struct platform_device;
> > > +
> > > +/**
> > > + * struct rcar_cmm_config - CMM configuration
> > > + *
> > > + * @lut:	1D-LUT configuration
> > > + * @lut.table:	1D-LUT table entries. Disable LUT operations when NULL
> > > + */
> > > +struct rcar_cmm_config {
> > > +	struct {
> > > +		struct drm_color_lut *table;
> > > +	} lut;
> > > +};
> > > +
> > > +#if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
> > > +int rcar_cmm_init(struct platform_device *pdev);
> > > +
> > > +int rcar_cmm_enable(struct platform_device *pdev);
> > > +void rcar_cmm_disable(struct platform_device *pdev);
> > > +
> > > +int rcar_cmm_setup(struct platform_device *pdev,
> > > +		   const struct rcar_cmm_config *config);
> > > +#else
> > > +static inline int rcar_cmm_init(struct platform_device *pdev)
> > > +{
> > > +	return -ENODEV;
> > > +}
> > > +
> > > +static inline int rcar_cmm_enable(struct platform_device *pdev)
> > > +{
> > > +	return 0;
> > > +}
> > > +
> > > +static inline void rcar_cmm_disable(struct platform_device *pdev)
> > > +{
> > > +}
> > > +
> > > +static inline int rcar_cmm_setup(struct platform_device *pdev,
> > > +				 const struct rcar_cmm_config *config)
> > > +{
> > > +	return 0;
> > > +}
> > > +#endif /* IS_ENABLED(CONFIG_DRM_RCAR_CMM) */
> > > +
> > > +#endif /* __RCAR_CMM_H__ */

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6.1 3/8] drm: rcar-du: Add support for CMM

[Jacopo Mondi]

Add a driver for the R-Car Display Unit Color Correction Module.

In most of Gen3 SoCs, each DU output channel is provided with a CMM unit
to perform image enhancement and color correction.

Add support for CMM through a driver that supports configuration of
the 1-dimensional LUT table. More advanced CMM features will be
implemented on top of this initial one.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

---
v6 -> v6.1
- Expand rcar_cmm_setup() function documentation to detail its relationship
  with rcar_cmm_enable() and their call order precedence.
---

 drivers/gpu/drm/rcar-du/Kconfig    |   7 +
 drivers/gpu/drm/rcar-du/Makefile   |   1 +
 drivers/gpu/drm/rcar-du/rcar_cmm.c | 217 +++++++++++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_cmm.h |  58 ++++++++
 4 files changed, 283 insertions(+)
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index 1529849e217e..539d232790d1 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -13,6 +13,13 @@ config DRM_RCAR_DU
 	  Choose this option if you have an R-Car chipset.
 	  If M is selected the module will be called rcar-du-drm.

+config DRM_RCAR_CMM
+	bool "R-Car DU Color Management Module (CMM) Support"
+	depends on DRM && OF
+	depends on DRM_RCAR_DU
+	help
+	  Enable support for R-Car Color Management Module (CMM).
+
 config DRM_RCAR_DW_HDMI
 	tristate "R-Car DU Gen3 HDMI Encoder Support"
 	depends on DRM && OF
diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
index 6c2ed9c46467..4d1187ccc3e5 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
 rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
 rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o

+obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
 obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
 obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
 obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c b/drivers/gpu/drm/rcar-du/rcar_cmm.c
new file mode 100644
index 000000000000..952316eb202b
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c
@@ -0,0 +1,217 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * rcar_cmm.c -- R-Car Display Unit Color Management Module
+ *
+ * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
+ */
+
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include <drm/drm_color_mgmt.h>
+
+#include "rcar_cmm.h"
+
+#define CM2_LUT_CTRL		0x0000
+#define CM2_LUT_CTRL_LUT_EN	BIT(0)
+#define CM2_LUT_TBL_BASE	0x0600
+#define CM2_LUT_TBL(__i)	(CM2_LUT_TBL_BASE + (__i) * 4)
+
+struct rcar_cmm {
+	void __iomem *base;
+
+	/*
+	 * @lut:		1D-LUT state
+	 * @lut.enabled:	1D-LUT enabled flag
+	 */
+	struct {
+		bool enabled;
+	} lut;
+};
+
+static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg)
+{
+	return ioread32(rcmm->base + reg);
+}
+
+static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 data)
+{
+	iowrite32(data, rcmm->base + reg);
+}
+
+/*
+ * rcar_cmm_lut_write() - Scale the DRM LUT table entries to hardware precision
+ *			  and write to the CMM registers
+ * @rcmm: Pointer to the CMM device
+ * @drm_lut: Pointer to the DRM LUT table
+ */
+static void rcar_cmm_lut_write(struct rcar_cmm *rcmm,
+			       const struct drm_color_lut *drm_lut)
+{
+	unsigned int i;
+
+	for (i = 0; i < CM2_LUT_SIZE; ++i) {
+		u32 entry = drm_color_lut_extract(drm_lut[i].red, 8) << 16
+			  | drm_color_lut_extract(drm_lut[i].green, 8) << 8
+			  | drm_color_lut_extract(drm_lut[i].blue, 8);
+
+		rcar_cmm_write(rcmm, CM2_LUT_TBL(i), entry);
+	}
+}
+
+/*
+ * rcar_cmm_setup() - Configure the CMM unit
+ * @pdev: The platform device associated with the CMM instance
+ * @config: The CMM unit configuration
+ *
+ * Configure the CMM unit with the given configuration. Currently enabling,
+ * disabling and programming of the 1-D LUT unit is supported.
+ *
+ * As rcar_cmm_setup() accesses the CMM registers the unit should be powered
+ * and its functional clock enabled. To guarantee this, before any call to
+ * this function is made, the CMM unit has to be enabled by calling
+ * rcar_cmm_enable() first.
+ *
+ * TODO: Add support for LUT double buffer operations to avoid updating the
+ * LUT table entries while a frame is being displayed.
+ */
+int rcar_cmm_setup(struct platform_device *pdev,
+		   const struct rcar_cmm_config *config)
+{
+	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
+
+	/* Disable LUT if no table is provided. */
+	if (!config->lut.table) {
+		if (rcmm->lut.enabled) {
+			rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
+			rcmm->lut.enabled = false;
+		}
+
+		return 0;
+	}
+
+	/* Enable LUT and program the new gamma table values. */
+	if (!rcmm->lut.enabled) {
+		rcar_cmm_write(rcmm, CM2_LUT_CTRL, CM2_LUT_CTRL_LUT_EN);
+		rcmm->lut.enabled = true;
+	}
+
+	rcar_cmm_lut_write(rcmm, config->lut.table);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rcar_cmm_setup);
+
+/*
+ * rcar_cmm_enable() - Enable the CMM unit
+ * @pdev: The platform device associated with the CMM instance
+ *
+ * When the output of the corresponding DU channel is routed to the CMM unit,
+ * the unit shall be enabled before the DU channel is started, and remain
+ * enabled until the channel is stopped. The CMM unit shall be disabled with
+ * rcar_cmm_disable().
+ *
+ * Calls to rcar_cmm_enable() and rcar_cmm_disable() are not reference-counted.
+ * It is an error to attempt to enable an already enabled CMM unit, or to
+ * attempt to disable a disabled unit.
+ */
+int rcar_cmm_enable(struct platform_device *pdev)
+{
+	int ret;
+
+	ret = pm_runtime_get_sync(&pdev->dev);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rcar_cmm_enable);
+
+/*
+ * rcar_cmm_disable() - Disable the CMM unit
+ * @pdev: The platform device associated with the CMM instance
+ *
+ * See rcar_cmm_enable() for usage information.
+ *
+ * Disabling the CMM unit disable all the internal processing blocks. The CMM
+ * state shall thus be restored with rcar_cmm_setup() when re-enabling the CMM
+ * unit after the next rcar_cmm_enable() call.
+ */
+void rcar_cmm_disable(struct platform_device *pdev)
+{
+	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
+
+	rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
+	rcmm->lut.enabled = false;
+
+	pm_runtime_put(&pdev->dev);
+}
+EXPORT_SYMBOL_GPL(rcar_cmm_disable);
+
+/*
+ * rcar_cmm_init() - Initialize the CMM unit
+ * @pdev: The platform device associated with the CMM instance
+ *
+ * Return: 0 on success, -EPROBE_DEFER if the CMM is not available yet,
+ *         -ENODEV if the DRM_RCAR_CMM config option is disabled
+ */
+int rcar_cmm_init(struct platform_device *pdev)
+{
+	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
+
+	if (!rcmm)
+		return -EPROBE_DEFER;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rcar_cmm_init);
+
+static int rcar_cmm_probe(struct platform_device *pdev)
+{
+	struct rcar_cmm *rcmm;
+
+	rcmm = devm_kzalloc(&pdev->dev, sizeof(*rcmm), GFP_KERNEL);
+	if (!rcmm)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, rcmm);
+
+	rcmm->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(rcmm->base))
+		return PTR_ERR(rcmm->base);
+
+	pm_runtime_enable(&pdev->dev);
+
+	return 0;
+}
+
+static int rcar_cmm_remove(struct platform_device *pdev)
+{
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id rcar_cmm_of_table[] = {
+	{ .compatible = "renesas,rcar-gen3-cmm", },
+	{ .compatible = "renesas,rcar-gen2-cmm", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, rcar_cmm_of_table);
+
+static struct platform_driver rcar_cmm_platform_driver = {
+	.probe		= rcar_cmm_probe,
+	.remove		= rcar_cmm_remove,
+	.driver		= {
+		.name	= "rcar-cmm",
+		.of_match_table = rcar_cmm_of_table,
+	},
+};
+
+module_platform_driver(rcar_cmm_platform_driver);
+
+MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas@jmondi.org>");
+MODULE_DESCRIPTION("Renesas R-Car CMM Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.h b/drivers/gpu/drm/rcar-du/rcar_cmm.h
new file mode 100644
index 000000000000..b5f7ec6db04a
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_cmm.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * rcar_cmm.h -- R-Car Display Unit Color Management Module
+ *
+ * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
+ */
+
+#ifndef __RCAR_CMM_H__
+#define __RCAR_CMM_H__
+
+#define CM2_LUT_SIZE		256
+
+struct drm_color_lut;
+struct platform_device;
+
+/**
+ * struct rcar_cmm_config - CMM configuration
+ *
+ * @lut:	1D-LUT configuration
+ * @lut.table:	1D-LUT table entries. Disable LUT operations when NULL
+ */
+struct rcar_cmm_config {
+	struct {
+		struct drm_color_lut *table;
+	} lut;
+};
+
+#if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
+int rcar_cmm_init(struct platform_device *pdev);
+
+int rcar_cmm_enable(struct platform_device *pdev);
+void rcar_cmm_disable(struct platform_device *pdev);
+
+int rcar_cmm_setup(struct platform_device *pdev,
+		   const struct rcar_cmm_config *config);
+#else
+static inline int rcar_cmm_init(struct platform_device *pdev)
+{
+	return -ENODEV;
+}
+
+static inline int rcar_cmm_enable(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static inline void rcar_cmm_disable(struct platform_device *pdev)
+{
+}
+
+static inline int rcar_cmm_setup(struct platform_device *pdev,
+				 const struct rcar_cmm_config *config)
+{
+	return 0;
+}
+#endif /* IS_ENABLED(CONFIG_DRM_RCAR_CMM) */
+
+#endif /* __RCAR_CMM_H__ */
--
2.23.0

Re: [PATCH 6.1 3/8] drm: rcar-du: Add support for CMM

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Thu, Oct 17, 2019 at 03:43:32PM +0200, Jacopo Mondi wrote:
> Add a driver for the R-Car Display Unit Color Correction Module.
> 
> In most of Gen3 SoCs, each DU output channel is provided with a CMM unit
> to perform image enhancement and color correction.
> 
> Add support for CMM through a driver that supports configuration of
> the 1-dimensional LUT table. More advanced CMM features will be
> implemented on top of this initial one.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> ---
> v6 -> v6.1
> - Expand rcar_cmm_setup() function documentation to detail its relationship
>   with rcar_cmm_enable() and their call order precedence.
> ---
> 
>  drivers/gpu/drm/rcar-du/Kconfig    |   7 +
>  drivers/gpu/drm/rcar-du/Makefile   |   1 +
>  drivers/gpu/drm/rcar-du/rcar_cmm.c | 217 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_cmm.h |  58 ++++++++
>  4 files changed, 283 insertions(+)
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h
> 
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index 1529849e217e..539d232790d1 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -13,6 +13,13 @@ config DRM_RCAR_DU
>  	  Choose this option if you have an R-Car chipset.
>  	  If M is selected the module will be called rcar-du-drm.
> 
> +config DRM_RCAR_CMM
> +	bool "R-Car DU Color Management Module (CMM) Support"
> +	depends on DRM && OF
> +	depends on DRM_RCAR_DU
> +	help
> +	  Enable support for R-Car Color Management Module (CMM).
> +
>  config DRM_RCAR_DW_HDMI
>  	tristate "R-Car DU Gen3 HDMI Encoder Support"
>  	depends on DRM && OF
> diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
> index 6c2ed9c46467..4d1187ccc3e5 100644
> --- a/drivers/gpu/drm/rcar-du/Makefile
> +++ b/drivers/gpu/drm/rcar-du/Makefile
> @@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
>  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
>  rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
> 
> +obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
>  obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
>  obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
>  obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
> diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> new file mode 100644
> index 000000000000..952316eb202b
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * rcar_cmm.c -- R-Car Display Unit Color Management Module
> + *
> + * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
> + */
> +
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include <drm/drm_color_mgmt.h>
> +
> +#include "rcar_cmm.h"
> +
> +#define CM2_LUT_CTRL		0x0000
> +#define CM2_LUT_CTRL_LUT_EN	BIT(0)
> +#define CM2_LUT_TBL_BASE	0x0600
> +#define CM2_LUT_TBL(__i)	(CM2_LUT_TBL_BASE + (__i) * 4)
> +
> +struct rcar_cmm {
> +	void __iomem *base;
> +
> +	/*
> +	 * @lut:		1D-LUT state
> +	 * @lut.enabled:	1D-LUT enabled flag
> +	 */
> +	struct {
> +		bool enabled;
> +	} lut;
> +};
> +
> +static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg)
> +{
> +	return ioread32(rcmm->base + reg);
> +}
> +
> +static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 data)
> +{
> +	iowrite32(data, rcmm->base + reg);
> +}
> +
> +/*
> + * rcar_cmm_lut_write() - Scale the DRM LUT table entries to hardware precision
> + *			  and write to the CMM registers
> + * @rcmm: Pointer to the CMM device
> + * @drm_lut: Pointer to the DRM LUT table
> + */
> +static void rcar_cmm_lut_write(struct rcar_cmm *rcmm,
> +			       const struct drm_color_lut *drm_lut)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < CM2_LUT_SIZE; ++i) {
> +		u32 entry = drm_color_lut_extract(drm_lut[i].red, 8) << 16
> +			  | drm_color_lut_extract(drm_lut[i].green, 8) << 8
> +			  | drm_color_lut_extract(drm_lut[i].blue, 8);
> +
> +		rcar_cmm_write(rcmm, CM2_LUT_TBL(i), entry);
> +	}
> +}
> +
> +/*
> + * rcar_cmm_setup() - Configure the CMM unit
> + * @pdev: The platform device associated with the CMM instance
> + * @config: The CMM unit configuration
> + *
> + * Configure the CMM unit with the given configuration. Currently enabling,
> + * disabling and programming of the 1-D LUT unit is supported.
> + *
> + * As rcar_cmm_setup() accesses the CMM registers the unit should be powered
> + * and its functional clock enabled. To guarantee this, before any call to
> + * this function is made, the CMM unit has to be enabled by calling
> + * rcar_cmm_enable() first.

We seem to have trouble agreeing on the right level of detail :-) It's
no big deal, and I'll take this patch in my tree, but I'd like to
explain my rationale, hoping that you'll prove me wrong or I'll convince
you :-)

Documenting a public API (this function is exported) is making a
contract. The documentation should state what the contract is, in order
for the caller to know how to use it. That's the level of detail I think
we need : anything that's part of the contract should be documented
(hence my review of the previous version). On the other hand, how the
function operates internally isn't part of the contract. The fact that
it accesses registers is internal implementation, as is the fact that
the functional clock should be enabled. That part thus isn't something
that needs to be documented externally, doing so would expand the API
contract, and the caller may rely on it. It's not a concern here, but in
general, if you document in a public API a side effect of a function,
then you won't be able to modify that function later in a way that would
modify that side effect. Of course, here we're dealing with an internal
API and we can modify both the CMM and the DU side in any case. Striking
the right level of details in documentation is not easy, and I hope that
this explanation can be of general usefulness.

This being said,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> + *
> + * TODO: Add support for LUT double buffer operations to avoid updating the
> + * LUT table entries while a frame is being displayed.
> + */
> +int rcar_cmm_setup(struct platform_device *pdev,
> +		   const struct rcar_cmm_config *config)
> +{
> +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> +
> +	/* Disable LUT if no table is provided. */
> +	if (!config->lut.table) {
> +		if (rcmm->lut.enabled) {
> +			rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
> +			rcmm->lut.enabled = false;
> +		}
> +
> +		return 0;
> +	}
> +
> +	/* Enable LUT and program the new gamma table values. */
> +	if (!rcmm->lut.enabled) {
> +		rcar_cmm_write(rcmm, CM2_LUT_CTRL, CM2_LUT_CTRL_LUT_EN);
> +		rcmm->lut.enabled = true;
> +	}
> +
> +	rcar_cmm_lut_write(rcmm, config->lut.table);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rcar_cmm_setup);
> +
> +/*
> + * rcar_cmm_enable() - Enable the CMM unit
> + * @pdev: The platform device associated with the CMM instance
> + *
> + * When the output of the corresponding DU channel is routed to the CMM unit,
> + * the unit shall be enabled before the DU channel is started, and remain
> + * enabled until the channel is stopped. The CMM unit shall be disabled with
> + * rcar_cmm_disable().
> + *
> + * Calls to rcar_cmm_enable() and rcar_cmm_disable() are not reference-counted.
> + * It is an error to attempt to enable an already enabled CMM unit, or to
> + * attempt to disable a disabled unit.
> + */
> +int rcar_cmm_enable(struct platform_device *pdev)
> +{
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(&pdev->dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rcar_cmm_enable);
> +
> +/*
> + * rcar_cmm_disable() - Disable the CMM unit
> + * @pdev: The platform device associated with the CMM instance
> + *
> + * See rcar_cmm_enable() for usage information.
> + *
> + * Disabling the CMM unit disable all the internal processing blocks. The CMM
> + * state shall thus be restored with rcar_cmm_setup() when re-enabling the CMM
> + * unit after the next rcar_cmm_enable() call.
> + */
> +void rcar_cmm_disable(struct platform_device *pdev)
> +{
> +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> +
> +	rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
> +	rcmm->lut.enabled = false;
> +
> +	pm_runtime_put(&pdev->dev);
> +}
> +EXPORT_SYMBOL_GPL(rcar_cmm_disable);
> +
> +/*
> + * rcar_cmm_init() - Initialize the CMM unit
> + * @pdev: The platform device associated with the CMM instance
> + *
> + * Return: 0 on success, -EPROBE_DEFER if the CMM is not available yet,
> + *         -ENODEV if the DRM_RCAR_CMM config option is disabled
> + */
> +int rcar_cmm_init(struct platform_device *pdev)
> +{
> +	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> +
> +	if (!rcmm)
> +		return -EPROBE_DEFER;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rcar_cmm_init);
> +
> +static int rcar_cmm_probe(struct platform_device *pdev)
> +{
> +	struct rcar_cmm *rcmm;
> +
> +	rcmm = devm_kzalloc(&pdev->dev, sizeof(*rcmm), GFP_KERNEL);
> +	if (!rcmm)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, rcmm);
> +
> +	rcmm->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(rcmm->base))
> +		return PTR_ERR(rcmm->base);
> +
> +	pm_runtime_enable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static int rcar_cmm_remove(struct platform_device *pdev)
> +{
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rcar_cmm_of_table[] = {
> +	{ .compatible = "renesas,rcar-gen3-cmm", },
> +	{ .compatible = "renesas,rcar-gen2-cmm", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, rcar_cmm_of_table);
> +
> +static struct platform_driver rcar_cmm_platform_driver = {
> +	.probe		= rcar_cmm_probe,
> +	.remove		= rcar_cmm_remove,
> +	.driver		= {
> +		.name	= "rcar-cmm",
> +		.of_match_table = rcar_cmm_of_table,
> +	},
> +};
> +
> +module_platform_driver(rcar_cmm_platform_driver);
> +
> +MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas@jmondi.org>");
> +MODULE_DESCRIPTION("Renesas R-Car CMM Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.h b/drivers/gpu/drm/rcar-du/rcar_cmm.h
> new file mode 100644
> index 000000000000..b5f7ec6db04a
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.h
> @@ -0,0 +1,58 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * rcar_cmm.h -- R-Car Display Unit Color Management Module
> + *
> + * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@jmondi.org>
> + */
> +
> +#ifndef __RCAR_CMM_H__
> +#define __RCAR_CMM_H__
> +
> +#define CM2_LUT_SIZE		256
> +
> +struct drm_color_lut;
> +struct platform_device;
> +
> +/**
> + * struct rcar_cmm_config - CMM configuration
> + *
> + * @lut:	1D-LUT configuration
> + * @lut.table:	1D-LUT table entries. Disable LUT operations when NULL
> + */
> +struct rcar_cmm_config {
> +	struct {
> +		struct drm_color_lut *table;
> +	} lut;
> +};
> +
> +#if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
> +int rcar_cmm_init(struct platform_device *pdev);
> +
> +int rcar_cmm_enable(struct platform_device *pdev);
> +void rcar_cmm_disable(struct platform_device *pdev);
> +
> +int rcar_cmm_setup(struct platform_device *pdev,
> +		   const struct rcar_cmm_config *config);
> +#else
> +static inline int rcar_cmm_init(struct platform_device *pdev)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline int rcar_cmm_enable(struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +
> +static inline void rcar_cmm_disable(struct platform_device *pdev)
> +{
> +}
> +
> +static inline int rcar_cmm_setup(struct platform_device *pdev,
> +				 const struct rcar_cmm_config *config)
> +{
> +	return 0;
> +}
> +#endif /* IS_ENABLED(CONFIG_DRM_RCAR_CMM) */
> +
> +#endif /* __RCAR_CMM_H__ */

-- 
Regards,

Laurent Pinchart

[PATCH v6 4/8] drm: rcar-du: kms: Initialize CMM instances

[Jacopo Mondi]

Implement device tree parsing to collect the available CMM instances
described by the 'renesas,cmms' property. Associate CMMs with CRTCs and
store a mask of active CMMs in the DU group for later enablement.

Enforce the probe and suspend/resume ordering of DU and CMM by creating
a stateless device link between the two.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |  6 ++
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h  |  2 +
 drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  2 +
 drivers/gpu/drm/rcar-du/rcar_du_group.h |  2 +
 drivers/gpu/drm/rcar-du/rcar_du_kms.c   | 76 +++++++++++++++++++++++++
 5 files changed, 88 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 2da46e3dc4ae..23f1d6cc1719 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -1194,6 +1194,12 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
 	if (ret < 0)
 		return ret;
 
+	/* CMM might be disabled for this CRTC. */
+	if (rcdu->cmms[swindex]) {
+		rcrtc->cmm = rcdu->cmms[swindex];
+		rgrp->cmms_mask |= BIT(hwindex % 2);
+	}
+
 	drm_crtc_helper_add(crtc, &crtc_helper_funcs);
 
 	/* Start with vertical blanking interrupt reporting disabled. */
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index 3b7fc668996f..5f2940c42225 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -39,6 +39,7 @@ struct rcar_du_vsp;
  * @vblank_wait: wait queue used to signal vertical blanking
  * @vblank_count: number of vertical blanking interrupts to wait for
  * @group: CRTC group this CRTC belongs to
+ * @cmm: CMM associated with this CRTC
  * @vsp: VSP feeding video to this CRTC
  * @vsp_pipe: index of the VSP pipeline feeding video to this CRTC
  * @writeback: the writeback connector
@@ -64,6 +65,7 @@ struct rcar_du_crtc {
 	unsigned int vblank_count;
 
 	struct rcar_du_group *group;
+	struct platform_device *cmm;
 	struct rcar_du_vsp *vsp;
 	unsigned int vsp_pipe;
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index 1327cd0df90a..61504c54e2ec 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -13,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/wait.h>
 
+#include "rcar_cmm.h"
 #include "rcar_du_crtc.h"
 #include "rcar_du_group.h"
 #include "rcar_du_vsp.h"
@@ -85,6 +86,7 @@ struct rcar_du_device {
 	struct rcar_du_encoder *encoders[RCAR_DU_OUTPUT_MAX];
 
 	struct rcar_du_group groups[RCAR_DU_MAX_GROUPS];
+	struct platform_device *cmms[RCAR_DU_MAX_CRTCS];
 	struct rcar_du_vsp vsps[RCAR_DU_MAX_VSPS];
 
 	struct {
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h b/drivers/gpu/drm/rcar-du/rcar_du_group.h
index 87950c1f6a52..e9906609c635 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h
@@ -22,6 +22,7 @@ struct rcar_du_device;
  * @mmio_offset: registers offset in the device memory map
  * @index: group index
  * @channels_mask: bitmask of populated DU channels in this group
+ * @cmms_mask: bitmask of available CMMs in this group
  * @num_crtcs: number of CRTCs in this group (1 or 2)
  * @use_count: number of users of the group (rcar_du_group_(get|put))
  * @used_crtcs: number of CRTCs currently in use
@@ -37,6 +38,7 @@ struct rcar_du_group {
 	unsigned int index;
 
 	unsigned int channels_mask;
+	unsigned int cmms_mask;
 	unsigned int num_crtcs;
 	unsigned int use_count;
 	unsigned int used_crtcs;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 2dc9caee8767..7c9fb5860e54 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -17,7 +17,9 @@
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_vblank.h>
 
+#include <linux/device.h>
 #include <linux/of_graph.h>
+#include <linux/of_platform.h>
 #include <linux/wait.h>
 
 #include "rcar_du_crtc.h"
@@ -614,6 +616,75 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
 	return ret;
 }
 
+static int rcar_du_cmm_init(struct rcar_du_device *rcdu)
+{
+	const struct device_node *np = rcdu->dev->of_node;
+	unsigned int i;
+	int cells;
+
+	cells = of_property_count_u32_elems(np, "renesas,cmms");
+	if (cells == -EINVAL)
+		return 0;
+
+	if (cells > rcdu->num_crtcs) {
+		dev_err(rcdu->dev,
+			"Invalid number of entries in 'renesas,cmms'\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < cells; ++i) {
+		struct platform_device *pdev;
+		struct device_link *link;
+		struct device_node *cmm;
+		int ret;
+
+		cmm = of_parse_phandle(np, "renesas,cmms", i);
+		if (IS_ERR(cmm)) {
+			dev_err(rcdu->dev,
+				"Failed to parse 'renesas,cmms' property\n");
+			return PTR_ERR(cmm);
+		}
+
+		if (!of_device_is_available(cmm)) {
+			/* It's fine to have a phandle to a non-enabled CMM. */
+			of_node_put(cmm);
+			continue;
+		}
+
+		pdev = of_find_device_by_node(cmm);
+		if (IS_ERR(pdev)) {
+			dev_err(rcdu->dev, "No device found for CMM%u\n", i);
+			of_node_put(cmm);
+			return PTR_ERR(pdev);
+		}
+
+		of_node_put(cmm);
+
+		/*
+		 * -ENODEV is used to report that the CMM config option is
+		 * disabled: return 0 and let the DU continue probing.
+		 */
+		ret = rcar_cmm_init(pdev);
+		if (ret)
+			return ret == -ENODEV ? 0 : ret;
+
+		/*
+		 * Enforce suspend/resume ordering by making the CMM a provider
+		 * of the DU: CMM is suspended after and resumed before the DU.
+		 */
+		link = device_link_add(rcdu->dev, &pdev->dev, DL_FLAG_STATELESS);
+		if (!link) {
+			dev_err(rcdu->dev,
+				"Failed to create device link to CMM%u\n", i);
+			return -EINVAL;
+		}
+
+		rcdu->cmms[i] = pdev;
+	}
+
+	return 0;
+}
+
 int rcar_du_modeset_init(struct rcar_du_device *rcdu)
 {
 	static const unsigned int mmio_offsets[] = {
@@ -704,6 +775,11 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
 			return ret;
 	}
 
+	/* Initialize the Color Management Modules. */
+	ret = rcar_du_cmm_init(rcdu);
+	if (ret)
+		return ret;
+
 	/* Create the CRTCs. */
 	for (swindex = 0, hwindex = 0; swindex < rcdu->num_crtcs; ++hwindex) {
 		struct rcar_du_group *rgrp;
-- 
2.23.0

[PATCH v6 5/8] drm: rcar-du: crtc: Control CMM operations

[Jacopo Mondi]

Implement CMM handling in the crtc begin and enable atomic callbacks,
and enable CMM unit through the Display Extensional Functions
register at group setup time.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 55 +++++++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_group.c | 10 +++++
 drivers/gpu/drm/rcar-du/rcar_du_regs.h  |  5 +++
 3 files changed, 70 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 23f1d6cc1719..d7ad491577f3 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -21,6 +21,7 @@
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_vblank.h>
 
+#include "rcar_cmm.h"
 #include "rcar_du_crtc.h"
 #include "rcar_du_drv.h"
 #include "rcar_du_encoder.h"
@@ -474,6 +475,45 @@ static void rcar_du_crtc_wait_page_flip(struct rcar_du_crtc *rcrtc)
 	rcar_du_crtc_finish_page_flip(rcrtc);
 }
 
+/* -----------------------------------------------------------------------------
+ * Color Management Module (CMM)
+ */
+
+static int rcar_du_cmm_check(struct drm_crtc *crtc,
+			     struct drm_crtc_state *state)
+{
+	struct drm_property_blob *drm_lut = state->gamma_lut;
+	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
+	struct device *dev = rcrtc->dev->dev;
+
+	if (!rcrtc->cmm || !drm_lut)
+		return 0;
+
+	/* We only accept fully populated LUT tables. */
+	if (drm_color_lut_size(drm_lut) != CM2_LUT_SIZE) {
+		dev_err(dev, "invalid gamma lut size: %lu bytes\n",
+			drm_lut->length);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void rcar_du_cmm_setup(struct drm_crtc *crtc)
+{
+	struct drm_property_blob *drm_lut = crtc->state->gamma_lut;
+	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
+	struct rcar_cmm_config cmm_config = {};
+
+	if (!rcrtc->cmm)
+		return;
+
+	if (drm_lut)
+		cmm_config.lut.table = (struct drm_color_lut *)drm_lut->data;
+
+	rcar_cmm_setup(rcrtc->cmm, &cmm_config);
+}
+
 /* -----------------------------------------------------------------------------
  * Start/Stop and Suspend/Resume
  */
@@ -619,6 +659,9 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
 	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
 		rcar_du_vsp_disable(rcrtc);
 
+	if (rcrtc->cmm)
+		rcar_cmm_disable(rcrtc->cmm);
+
 	/*
 	 * Select switch sync mode. This stops display operation and configures
 	 * the HSYNC and VSYNC signals as inputs.
@@ -642,6 +685,11 @@ static int rcar_du_crtc_atomic_check(struct drm_crtc *crtc,
 {
 	struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(state);
 	struct drm_encoder *encoder;
+	int ret;
+
+	ret = rcar_du_cmm_check(crtc, state);
+	if (ret)
+		return ret;
 
 	/* Store the routes from the CRTC output to the DU outputs. */
 	rstate->outputs = 0;
@@ -667,6 +715,8 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
 	struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(crtc->state);
 	struct rcar_du_device *rcdu = rcrtc->dev;
 
+	if (rcrtc->cmm)
+		rcar_cmm_enable(rcrtc->cmm);
 	rcar_du_crtc_get(rcrtc);
 
 	/*
@@ -686,6 +736,7 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
 	}
 
 	rcar_du_crtc_start(rcrtc);
+	rcar_du_cmm_setup(crtc);
 }
 
 static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
@@ -739,6 +790,10 @@ static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc,
 	 */
 	rcar_du_crtc_get(rcrtc);
 
+	/* If the active state changed, we let .atomic_enable handle CMM. */
+	if (crtc->state->color_mgmt_changed && !crtc->state->active_changed)
+		rcar_du_cmm_setup(crtc);
+
 	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
 		rcar_du_vsp_atomic_begin(rcrtc);
 }
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index 9eee47969e77..88a783ceb3e9 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -135,6 +135,7 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp)
 static void rcar_du_group_setup(struct rcar_du_group *rgrp)
 {
 	struct rcar_du_device *rcdu = rgrp->dev;
+	u32 defr7 = DEFR7_CODE;
 
 	/* Enable extended features */
 	rcar_du_group_write(rgrp, DEFR, DEFR_CODE | DEFR_DEFE);
@@ -147,6 +148,15 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
 
 	rcar_du_group_setup_pins(rgrp);
 
+	/*
+	 * TODO: Handle routing of the DU output to CMM dynamically, as we
+	 * should bypass CMM completely when no color management feature is
+	 * used.
+	 */
+	defr7 |= (rgrp->cmms_mask & BIT(1) ? DEFR7_CMME1 : 0) |
+		 (rgrp->cmms_mask & BIT(0) ? DEFR7_CMME0 : 0);
+	rcar_du_group_write(rgrp, DEFR7, defr7);
+
 	if (rcdu->info->gen >= 2) {
 		rcar_du_group_setup_defr8(rgrp);
 		rcar_du_group_setup_didsr(rgrp);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
index bc87f080b170..fb9964949368 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
@@ -197,6 +197,11 @@
 #define DEFR6_MLOS1		(1 << 2)
 #define DEFR6_DEFAULT		(DEFR6_CODE | DEFR6_TCNE1)
 
+#define DEFR7			0x000ec
+#define DEFR7_CODE		(0x7779 << 16)
+#define DEFR7_CMME1		BIT(6)
+#define DEFR7_CMME0		BIT(4)
+
 /* -----------------------------------------------------------------------------
  * R8A7790-only Control Registers
  */
-- 
2.23.0

Re: [PATCH v6 5/8] drm: rcar-du: crtc: Control CMM operations

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Oct 16, 2019 at 10:55:45AM +0200, Jacopo Mondi wrote:
> Implement CMM handling in the crtc begin and enable atomic callbacks,
> and enable CMM unit through the Display Extensional Functions
> register at group setup time.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 55 +++++++++++++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_du_group.c | 10 +++++
>  drivers/gpu/drm/rcar-du/rcar_du_regs.h  |  5 +++
>  3 files changed, 70 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 23f1d6cc1719..d7ad491577f3 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -21,6 +21,7 @@
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drm_vblank.h>
>  
> +#include "rcar_cmm.h"
>  #include "rcar_du_crtc.h"
>  #include "rcar_du_drv.h"
>  #include "rcar_du_encoder.h"
> @@ -474,6 +475,45 @@ static void rcar_du_crtc_wait_page_flip(struct rcar_du_crtc *rcrtc)
>  	rcar_du_crtc_finish_page_flip(rcrtc);
>  }
>  
> +/* -----------------------------------------------------------------------------
> + * Color Management Module (CMM)
> + */
> +
> +static int rcar_du_cmm_check(struct drm_crtc *crtc,
> +			     struct drm_crtc_state *state)
> +{
> +	struct drm_property_blob *drm_lut = state->gamma_lut;
> +	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> +	struct device *dev = rcrtc->dev->dev;
> +
> +	if (!rcrtc->cmm || !drm_lut)

As the gamma LUT property is only created when CMM is present, can't you
drop the first part of this condition ?

> +		return 0;
> +
> +	/* We only accept fully populated LUT tables. */
> +	if (drm_color_lut_size(drm_lut) != CM2_LUT_SIZE) {
> +		dev_err(dev, "invalid gamma lut size: %lu bytes\n",
> +			drm_lut->length);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void rcar_du_cmm_setup(struct drm_crtc *crtc)
> +{
> +	struct drm_property_blob *drm_lut = crtc->state->gamma_lut;
> +	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> +	struct rcar_cmm_config cmm_config = {};
> +
> +	if (!rcrtc->cmm)
> +		return;
> +
> +	if (drm_lut)
> +		cmm_config.lut.table = (struct drm_color_lut *)drm_lut->data;
> +
> +	rcar_cmm_setup(rcrtc->cmm, &cmm_config);
> +}
> +
>  /* -----------------------------------------------------------------------------
>   * Start/Stop and Suspend/Resume
>   */
> @@ -619,6 +659,9 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>  	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
>  		rcar_du_vsp_disable(rcrtc);
>  
> +	if (rcrtc->cmm)
> +		rcar_cmm_disable(rcrtc->cmm);
> +
>  	/*
>  	 * Select switch sync mode. This stops display operation and configures
>  	 * the HSYNC and VSYNC signals as inputs.
> @@ -642,6 +685,11 @@ static int rcar_du_crtc_atomic_check(struct drm_crtc *crtc,
>  {
>  	struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(state);
>  	struct drm_encoder *encoder;
> +	int ret;
> +
> +	ret = rcar_du_cmm_check(crtc, state);
> +	if (ret)
> +		return ret;
>  
>  	/* Store the routes from the CRTC output to the DU outputs. */
>  	rstate->outputs = 0;
> @@ -667,6 +715,8 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
>  	struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(crtc->state);
>  	struct rcar_du_device *rcdu = rcrtc->dev;
>  
> +	if (rcrtc->cmm)
> +		rcar_cmm_enable(rcrtc->cmm);
>  	rcar_du_crtc_get(rcrtc);
>  
>  	/*
> @@ -686,6 +736,7 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
>  	}
>  
>  	rcar_du_crtc_start(rcrtc);
> +	rcar_du_cmm_setup(crtc);

Let's add a TODO here to figure out why we can't setup CMM before
starting the CRTC.

With these two small issues fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  }
>  
>  static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
> @@ -739,6 +790,10 @@ static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc,
>  	 */
>  	rcar_du_crtc_get(rcrtc);
>  
> +	/* If the active state changed, we let .atomic_enable handle CMM. */
> +	if (crtc->state->color_mgmt_changed && !crtc->state->active_changed)
> +		rcar_du_cmm_setup(crtc);
> +
>  	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
>  		rcar_du_vsp_atomic_begin(rcrtc);
>  }
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index 9eee47969e77..88a783ceb3e9 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -135,6 +135,7 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp)
>  static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>  {
>  	struct rcar_du_device *rcdu = rgrp->dev;
> +	u32 defr7 = DEFR7_CODE;
>  
>  	/* Enable extended features */
>  	rcar_du_group_write(rgrp, DEFR, DEFR_CODE | DEFR_DEFE);
> @@ -147,6 +148,15 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>  
>  	rcar_du_group_setup_pins(rgrp);
>  
> +	/*
> +	 * TODO: Handle routing of the DU output to CMM dynamically, as we
> +	 * should bypass CMM completely when no color management feature is
> +	 * used.
> +	 */
> +	defr7 |= (rgrp->cmms_mask & BIT(1) ? DEFR7_CMME1 : 0) |
> +		 (rgrp->cmms_mask & BIT(0) ? DEFR7_CMME0 : 0);
> +	rcar_du_group_write(rgrp, DEFR7, defr7);
> +
>  	if (rcdu->info->gen >= 2) {
>  		rcar_du_group_setup_defr8(rgrp);
>  		rcar_du_group_setup_didsr(rgrp);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> index bc87f080b170..fb9964949368 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> @@ -197,6 +197,11 @@
>  #define DEFR6_MLOS1		(1 << 2)
>  #define DEFR6_DEFAULT		(DEFR6_CODE | DEFR6_TCNE1)
>  
> +#define DEFR7			0x000ec
> +#define DEFR7_CODE		(0x7779 << 16)
> +#define DEFR7_CMME1		BIT(6)
> +#define DEFR7_CMME0		BIT(4)
> +
>  /* -----------------------------------------------------------------------------
>   * R8A7790-only Control Registers
>   */

-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 5/8] drm: rcar-du: crtc: Control CMM operations

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Oct 16, 2019 at 10:55:45AM +0200, Jacopo Mondi wrote:
> Implement CMM handling in the crtc begin and enable atomic callbacks,
> and enable CMM unit through the Display Extensional Functions
> register at group setup time.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 55 +++++++++++++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_du_group.c | 10 +++++
>  drivers/gpu/drm/rcar-du/rcar_du_regs.h  |  5 +++
>  3 files changed, 70 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 23f1d6cc1719..d7ad491577f3 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -21,6 +21,7 @@
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drm_vblank.h>
>  
> +#include "rcar_cmm.h"
>  #include "rcar_du_crtc.h"
>  #include "rcar_du_drv.h"
>  #include "rcar_du_encoder.h"
> @@ -474,6 +475,45 @@ static void rcar_du_crtc_wait_page_flip(struct rcar_du_crtc *rcrtc)
>  	rcar_du_crtc_finish_page_flip(rcrtc);
>  }
>  
> +/* -----------------------------------------------------------------------------
> + * Color Management Module (CMM)
> + */
> +
> +static int rcar_du_cmm_check(struct drm_crtc *crtc,
> +			     struct drm_crtc_state *state)
> +{
> +	struct drm_property_blob *drm_lut = state->gamma_lut;
> +	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> +	struct device *dev = rcrtc->dev->dev;
> +
> +	if (!rcrtc->cmm || !drm_lut)

As the gamma LUT property is only created when CMM is present, can't you
drop the first part of this condition ?

> +		return 0;
> +
> +	/* We only accept fully populated LUT tables. */
> +	if (drm_color_lut_size(drm_lut) != CM2_LUT_SIZE) {
> +		dev_err(dev, "invalid gamma lut size: %lu bytes\n",
> +			drm_lut->length);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void rcar_du_cmm_setup(struct drm_crtc *crtc)
> +{
> +	struct drm_property_blob *drm_lut = crtc->state->gamma_lut;
> +	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> +	struct rcar_cmm_config cmm_config = {};
> +
> +	if (!rcrtc->cmm)
> +		return;
> +
> +	if (drm_lut)
> +		cmm_config.lut.table = (struct drm_color_lut *)drm_lut->data;
> +
> +	rcar_cmm_setup(rcrtc->cmm, &cmm_config);
> +}
> +
>  /* -----------------------------------------------------------------------------
>   * Start/Stop and Suspend/Resume
>   */
> @@ -619,6 +659,9 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>  	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
>  		rcar_du_vsp_disable(rcrtc);
>  
> +	if (rcrtc->cmm)
> +		rcar_cmm_disable(rcrtc->cmm);
> +
>  	/*
>  	 * Select switch sync mode. This stops display operation and configures
>  	 * the HSYNC and VSYNC signals as inputs.
> @@ -642,6 +685,11 @@ static int rcar_du_crtc_atomic_check(struct drm_crtc *crtc,
>  {
>  	struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(state);
>  	struct drm_encoder *encoder;
> +	int ret;
> +
> +	ret = rcar_du_cmm_check(crtc, state);
> +	if (ret)
> +		return ret;
>  
>  	/* Store the routes from the CRTC output to the DU outputs. */
>  	rstate->outputs = 0;
> @@ -667,6 +715,8 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
>  	struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(crtc->state);
>  	struct rcar_du_device *rcdu = rcrtc->dev;
>  
> +	if (rcrtc->cmm)
> +		rcar_cmm_enable(rcrtc->cmm);
>  	rcar_du_crtc_get(rcrtc);
>  
>  	/*
> @@ -686,6 +736,7 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
>  	}
>  
>  	rcar_du_crtc_start(rcrtc);
> +	rcar_du_cmm_setup(crtc);

Let's add a TODO here to figure out why we can't setup CMM before
starting the CRTC.

With these two small issues fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  }
>  
>  static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
> @@ -739,6 +790,10 @@ static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc,
>  	 */
>  	rcar_du_crtc_get(rcrtc);
>  
> +	/* If the active state changed, we let .atomic_enable handle CMM. */
> +	if (crtc->state->color_mgmt_changed && !crtc->state->active_changed)
> +		rcar_du_cmm_setup(crtc);
> +
>  	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
>  		rcar_du_vsp_atomic_begin(rcrtc);
>  }
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index 9eee47969e77..88a783ceb3e9 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -135,6 +135,7 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp)
>  static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>  {
>  	struct rcar_du_device *rcdu = rgrp->dev;
> +	u32 defr7 = DEFR7_CODE;
>  
>  	/* Enable extended features */
>  	rcar_du_group_write(rgrp, DEFR, DEFR_CODE | DEFR_DEFE);
> @@ -147,6 +148,15 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>  
>  	rcar_du_group_setup_pins(rgrp);
>  
> +	/*
> +	 * TODO: Handle routing of the DU output to CMM dynamically, as we
> +	 * should bypass CMM completely when no color management feature is
> +	 * used.
> +	 */
> +	defr7 |= (rgrp->cmms_mask & BIT(1) ? DEFR7_CMME1 : 0) |
> +		 (rgrp->cmms_mask & BIT(0) ? DEFR7_CMME0 : 0);
> +	rcar_du_group_write(rgrp, DEFR7, defr7);
> +
>  	if (rcdu->info->gen >= 2) {
>  		rcar_du_group_setup_defr8(rgrp);
>  		rcar_du_group_setup_didsr(rgrp);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> index bc87f080b170..fb9964949368 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> @@ -197,6 +197,11 @@
>  #define DEFR6_MLOS1		(1 << 2)
>  #define DEFR6_DEFAULT		(DEFR6_CODE | DEFR6_TCNE1)
>  
> +#define DEFR7			0x000ec
> +#define DEFR7_CODE		(0x7779 << 16)
> +#define DEFR7_CMME1		BIT(6)
> +#define DEFR7_CMME0		BIT(4)
> +
>  /* -----------------------------------------------------------------------------
>   * R8A7790-only Control Registers
>   */

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v6.1 5/8] drm: rcar-du: crtc: Control CMM operations

[Jacopo Mondi]

Implement CMM handling in the crtc begin and enable atomic callbacks,
and enable CMM unit through the Display Extensional Functions
register at group setup time.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

---
v6 -> v6.1
- Drop check for CMM in rcar_du_cmm_check as if the gamma_table property is
  available, a CMM unit for this CRTC was registered
- Add TODO note to investigate how the activation order of CMM and CRTC
  impact on the first displayed fram
---

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 61 +++++++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_group.c | 10 ++++
 drivers/gpu/drm/rcar-du/rcar_du_regs.h  |  5 ++
 3 files changed, 76 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 23f1d6cc1719..3f0f16946f42 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -21,6 +21,7 @@
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_vblank.h>

+#include "rcar_cmm.h"
 #include "rcar_du_crtc.h"
 #include "rcar_du_drv.h"
 #include "rcar_du_encoder.h"
@@ -474,6 +475,45 @@ static void rcar_du_crtc_wait_page_flip(struct rcar_du_crtc *rcrtc)
 	rcar_du_crtc_finish_page_flip(rcrtc);
 }

+/* -----------------------------------------------------------------------------
+ * Color Management Module (CMM)
+ */
+
+static int rcar_du_cmm_check(struct drm_crtc *crtc,
+			     struct drm_crtc_state *state)
+{
+	struct drm_property_blob *drm_lut = state->gamma_lut;
+	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
+	struct device *dev = rcrtc->dev->dev;
+
+	if (!drm_lut)
+		return 0;
+
+	/* We only accept fully populated LUT tables. */
+	if (drm_color_lut_size(drm_lut) != CM2_LUT_SIZE) {
+		dev_err(dev, "invalid gamma lut size: %lu bytes\n",
+			drm_lut->length);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void rcar_du_cmm_setup(struct drm_crtc *crtc)
+{
+	struct drm_property_blob *drm_lut = crtc->state->gamma_lut;
+	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
+	struct rcar_cmm_config cmm_config = {};
+
+	if (!rcrtc->cmm)
+		return;
+
+	if (drm_lut)
+		cmm_config.lut.table = (struct drm_color_lut *)drm_lut->data;
+
+	rcar_cmm_setup(rcrtc->cmm, &cmm_config);
+}
+
 /* -----------------------------------------------------------------------------
  * Start/Stop and Suspend/Resume
  */
@@ -619,6 +659,9 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
 	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
 		rcar_du_vsp_disable(rcrtc);

+	if (rcrtc->cmm)
+		rcar_cmm_disable(rcrtc->cmm);
+
 	/*
 	 * Select switch sync mode. This stops display operation and configures
 	 * the HSYNC and VSYNC signals as inputs.
@@ -642,6 +685,11 @@ static int rcar_du_crtc_atomic_check(struct drm_crtc *crtc,
 {
 	struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(state);
 	struct drm_encoder *encoder;
+	int ret;
+
+	ret = rcar_du_cmm_check(crtc, state);
+	if (ret)
+		return ret;

 	/* Store the routes from the CRTC output to the DU outputs. */
 	rstate->outputs = 0;
@@ -667,6 +715,8 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
 	struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(crtc->state);
 	struct rcar_du_device *rcdu = rcrtc->dev;

+	if (rcrtc->cmm)
+		rcar_cmm_enable(rcrtc->cmm);
 	rcar_du_crtc_get(rcrtc);

 	/*
@@ -686,6 +736,13 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
 	}

 	rcar_du_crtc_start(rcrtc);
+
+	/*
+	 * TODO: The chip manual indicates that CMM tables should be written
+	 * after the DU channel has been activated. Investigate the impact
+	 * of this restriction on the first displayed frame.
+	 */
+	rcar_du_cmm_setup(crtc);
 }

 static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
@@ -739,6 +796,10 @@ static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc,
 	 */
 	rcar_du_crtc_get(rcrtc);

+	/* If the active state changed, we let .atomic_enable handle CMM. */
+	if (crtc->state->color_mgmt_changed && !crtc->state->active_changed)
+		rcar_du_cmm_setup(crtc);
+
 	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
 		rcar_du_vsp_atomic_begin(rcrtc);
 }
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index 9eee47969e77..88a783ceb3e9 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -135,6 +135,7 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp)
 static void rcar_du_group_setup(struct rcar_du_group *rgrp)
 {
 	struct rcar_du_device *rcdu = rgrp->dev;
+	u32 defr7 = DEFR7_CODE;

 	/* Enable extended features */
 	rcar_du_group_write(rgrp, DEFR, DEFR_CODE | DEFR_DEFE);
@@ -147,6 +148,15 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)

 	rcar_du_group_setup_pins(rgrp);

+	/*
+	 * TODO: Handle routing of the DU output to CMM dynamically, as we
+	 * should bypass CMM completely when no color management feature is
+	 * used.
+	 */
+	defr7 |= (rgrp->cmms_mask & BIT(1) ? DEFR7_CMME1 : 0) |
+		 (rgrp->cmms_mask & BIT(0) ? DEFR7_CMME0 : 0);
+	rcar_du_group_write(rgrp, DEFR7, defr7);
+
 	if (rcdu->info->gen >= 2) {
 		rcar_du_group_setup_defr8(rgrp);
 		rcar_du_group_setup_didsr(rgrp);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
index bc87f080b170..fb9964949368 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
@@ -197,6 +197,11 @@
 #define DEFR6_MLOS1		(1 << 2)
 #define DEFR6_DEFAULT		(DEFR6_CODE | DEFR6_TCNE1)

+#define DEFR7			0x000ec
+#define DEFR7_CODE		(0x7779 << 16)
+#define DEFR7_CMME1		BIT(6)
+#define DEFR7_CMME0		BIT(4)
+
 /* -----------------------------------------------------------------------------
  * R8A7790-only Control Registers
  */
--
2.23.0

Re: [PATCH v6.1 5/8] drm: rcar-du: crtc: Control CMM operations

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Thu, Oct 17, 2019 at 03:44:09PM +0200, Jacopo Mondi wrote:
> Implement CMM handling in the crtc begin and enable atomic callbacks,
> and enable CMM unit through the Display Extensional Functions
> register at group setup time.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> v6 -> v6.1
> - Drop check for CMM in rcar_du_cmm_check as if the gamma_table property is
>   available, a CMM unit for this CRTC was registered
> - Add TODO note to investigate how the activation order of CMM and CRTC
>   impact on the first displayed fram

Thank you :-)

> ---
> 
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 61 +++++++++++++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_du_group.c | 10 ++++
>  drivers/gpu/drm/rcar-du/rcar_du_regs.h  |  5 ++
>  3 files changed, 76 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 23f1d6cc1719..3f0f16946f42 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -21,6 +21,7 @@
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drm_vblank.h>
> 
> +#include "rcar_cmm.h"
>  #include "rcar_du_crtc.h"
>  #include "rcar_du_drv.h"
>  #include "rcar_du_encoder.h"
> @@ -474,6 +475,45 @@ static void rcar_du_crtc_wait_page_flip(struct rcar_du_crtc *rcrtc)
>  	rcar_du_crtc_finish_page_flip(rcrtc);
>  }
> 
> +/* -----------------------------------------------------------------------------
> + * Color Management Module (CMM)
> + */
> +
> +static int rcar_du_cmm_check(struct drm_crtc *crtc,
> +			     struct drm_crtc_state *state)
> +{
> +	struct drm_property_blob *drm_lut = state->gamma_lut;
> +	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> +	struct device *dev = rcrtc->dev->dev;
> +
> +	if (!drm_lut)
> +		return 0;
> +
> +	/* We only accept fully populated LUT tables. */
> +	if (drm_color_lut_size(drm_lut) != CM2_LUT_SIZE) {
> +		dev_err(dev, "invalid gamma lut size: %lu bytes\n",
> +			drm_lut->length);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void rcar_du_cmm_setup(struct drm_crtc *crtc)
> +{
> +	struct drm_property_blob *drm_lut = crtc->state->gamma_lut;
> +	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> +	struct rcar_cmm_config cmm_config = {};
> +
> +	if (!rcrtc->cmm)
> +		return;
> +
> +	if (drm_lut)
> +		cmm_config.lut.table = (struct drm_color_lut *)drm_lut->data;
> +
> +	rcar_cmm_setup(rcrtc->cmm, &cmm_config);
> +}
> +
>  /* -----------------------------------------------------------------------------
>   * Start/Stop and Suspend/Resume
>   */
> @@ -619,6 +659,9 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>  	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
>  		rcar_du_vsp_disable(rcrtc);
> 
> +	if (rcrtc->cmm)
> +		rcar_cmm_disable(rcrtc->cmm);
> +
>  	/*
>  	 * Select switch sync mode. This stops display operation and configures
>  	 * the HSYNC and VSYNC signals as inputs.
> @@ -642,6 +685,11 @@ static int rcar_du_crtc_atomic_check(struct drm_crtc *crtc,
>  {
>  	struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(state);
>  	struct drm_encoder *encoder;
> +	int ret;
> +
> +	ret = rcar_du_cmm_check(crtc, state);
> +	if (ret)
> +		return ret;
> 
>  	/* Store the routes from the CRTC output to the DU outputs. */
>  	rstate->outputs = 0;
> @@ -667,6 +715,8 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
>  	struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(crtc->state);
>  	struct rcar_du_device *rcdu = rcrtc->dev;
> 
> +	if (rcrtc->cmm)
> +		rcar_cmm_enable(rcrtc->cmm);
>  	rcar_du_crtc_get(rcrtc);
> 
>  	/*
> @@ -686,6 +736,13 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
>  	}
> 
>  	rcar_du_crtc_start(rcrtc);
> +
> +	/*
> +	 * TODO: The chip manual indicates that CMM tables should be written
> +	 * after the DU channel has been activated. Investigate the impact
> +	 * of this restriction on the first displayed frame.
> +	 */
> +	rcar_du_cmm_setup(crtc);
>  }
> 
>  static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
> @@ -739,6 +796,10 @@ static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc,
>  	 */
>  	rcar_du_crtc_get(rcrtc);
> 
> +	/* If the active state changed, we let .atomic_enable handle CMM. */
> +	if (crtc->state->color_mgmt_changed && !crtc->state->active_changed)
> +		rcar_du_cmm_setup(crtc);
> +
>  	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
>  		rcar_du_vsp_atomic_begin(rcrtc);
>  }
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index 9eee47969e77..88a783ceb3e9 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -135,6 +135,7 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp)
>  static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>  {
>  	struct rcar_du_device *rcdu = rgrp->dev;
> +	u32 defr7 = DEFR7_CODE;
> 
>  	/* Enable extended features */
>  	rcar_du_group_write(rgrp, DEFR, DEFR_CODE | DEFR_DEFE);
> @@ -147,6 +148,15 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
> 
>  	rcar_du_group_setup_pins(rgrp);
> 
> +	/*
> +	 * TODO: Handle routing of the DU output to CMM dynamically, as we
> +	 * should bypass CMM completely when no color management feature is
> +	 * used.
> +	 */
> +	defr7 |= (rgrp->cmms_mask & BIT(1) ? DEFR7_CMME1 : 0) |
> +		 (rgrp->cmms_mask & BIT(0) ? DEFR7_CMME0 : 0);
> +	rcar_du_group_write(rgrp, DEFR7, defr7);
> +
>  	if (rcdu->info->gen >= 2) {
>  		rcar_du_group_setup_defr8(rgrp);
>  		rcar_du_group_setup_didsr(rgrp);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> index bc87f080b170..fb9964949368 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> @@ -197,6 +197,11 @@
>  #define DEFR6_MLOS1		(1 << 2)
>  #define DEFR6_DEFAULT		(DEFR6_CODE | DEFR6_TCNE1)
> 
> +#define DEFR7			0x000ec
> +#define DEFR7_CODE		(0x7779 << 16)
> +#define DEFR7_CMME1		BIT(6)
> +#define DEFR7_CMME0		BIT(4)
> +
>  /* -----------------------------------------------------------------------------
>   * R8A7790-only Control Registers
>   */

-- 
Regards,

Laurent Pinchart

[PATCH v6 6/8] drm: rcar-du: crtc: Register GAMMA_LUT properties

[Jacopo Mondi]

Enable the GAMMA_LUT KMS property using the framework helpers to
register the property and set the associated gamma table maximum size.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index d7ad491577f3..a0a512b81aed 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -1130,6 +1130,7 @@ static const struct drm_crtc_funcs crtc_funcs_gen3 = {
 	.set_crc_source = rcar_du_crtc_set_crc_source,
 	.verify_crc_source = rcar_du_crtc_verify_crc_source,
 	.get_crc_sources = rcar_du_crtc_get_crc_sources,
+	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 };
 
 /* -----------------------------------------------------------------------------
@@ -1253,6 +1254,9 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
 	if (rcdu->cmms[swindex]) {
 		rcrtc->cmm = rcdu->cmms[swindex];
 		rgrp->cmms_mask |= BIT(hwindex % 2);
+
+		drm_mode_crtc_set_gamma_size(crtc, CM2_LUT_SIZE);
+		drm_crtc_enable_color_mgmt(crtc, 0, false, CM2_LUT_SIZE);
 	}
 
 	drm_crtc_helper_add(crtc, &crtc_helper_funcs);
-- 
2.23.0

[PATCH v6 7/8] arm64: dts: renesas: Add CMM units to Gen3 SoCs

[Jacopo Mondi]

Add CMM units to Renesas R-Car Gen3 SoC that support it, and reference them
from the Display Unit they are connected to.

Sort the 'vsps', 'renesas,cmm' and 'status' properties in the DU unit
consistently in all the involved DTS.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm64/boot/dts/renesas/r8a7795.dtsi  | 39 +++++++++++++++++++++++
 arch/arm64/boot/dts/renesas/r8a7796.dtsi  | 31 +++++++++++++++++-
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 31 +++++++++++++++++-
 arch/arm64/boot/dts/renesas/r8a77990.dtsi | 21 ++++++++++++
 arch/arm64/boot/dts/renesas/r8a77995.dtsi | 21 ++++++++++++
 5 files changed, 141 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index 6675462f7585..e16757af8c27 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -2939,6 +2939,42 @@
 			iommus = <&ipmmu_vi1 10>;
 		};
 
+		cmm0: cmm@fea40000 {
+			compatible = "renesas,r8a7795-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea40000 0 0x1000>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 711>;
+			resets = <&cpg 711>;
+		};
+
+		cmm1: cmm@fea50000 {
+			compatible = "renesas,r8a7795-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea50000 0 0x1000>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 710>;
+			resets = <&cpg 710>;
+		};
+
+		cmm2: cmm@fea60000 {
+			compatible = "renesas,r8a7795-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea60000 0 0x1000>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 709>;
+			resets = <&cpg 709>;
+		};
+
+		cmm3: cmm@fea70000 {
+			compatible = "renesas,r8a7795-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea70000 0 0x1000>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 708>;
+			resets = <&cpg 708>;
+		};
+
 		csi20: csi2@fea80000 {
 			compatible = "renesas,r8a7795-csi2";
 			reg = <0 0xfea80000 0 0x10000>;
@@ -3142,7 +3178,10 @@
 				 <&cpg CPG_MOD 722>,
 				 <&cpg CPG_MOD 721>;
 			clock-names = "du.0", "du.1", "du.2", "du.3";
+
+			renesas,cmms = <&cmm0>, <&cmm1>, <&cmm2>, <&cmm3>;
 			vsps = <&vspd0 0>, <&vspd1 0>, <&vspd2 0>, <&vspd0 1>;
+
 			status = "disabled";
 
 			ports {
diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 822c96601d3c..597c47f3f994 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -2641,6 +2641,33 @@
 			renesas,fcp = <&fcpvi0>;
 		};
 
+		cmm0: cmm@fea40000 {
+			compatible = "renesas,r8a7796-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea40000 0 0x1000>;
+			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 711>;
+			resets = <&cpg 711>;
+		};
+
+		cmm1: cmm@fea50000 {
+			compatible = "renesas,r8a7796-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea50000 0 0x1000>;
+			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 710>;
+			resets = <&cpg 710>;
+		};
+
+		cmm2: cmm@fea60000 {
+			compatible = "renesas,r8a7796-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea60000 0 0x1000>;
+			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 709>;
+			resets = <&cpg 709>;
+		};
+
 		csi20: csi2@fea80000 {
 			compatible = "renesas,r8a7796-csi2";
 			reg = <0 0xfea80000 0 0x10000>;
@@ -2791,10 +2818,12 @@
 				 <&cpg CPG_MOD 723>,
 				 <&cpg CPG_MOD 722>;
 			clock-names = "du.0", "du.1", "du.2";
-			status = "disabled";
 
+			renesas,cmms = <&cmm0>, <&cmm1>, <&cmm2>;
 			vsps = <&vspd0 0>, <&vspd1 0>, <&vspd2 0>;
 
+			status = "disabled";
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 4ae163220f60..c3da8d26ccba 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -2320,6 +2320,33 @@
 			resets = <&cpg 611>;
 		};
 
+		cmm0: cmm@fea40000 {
+			compatible = "renesas,r8a77965-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea40000 0 0x1000>;
+			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 711>;
+			resets = <&cpg 711>;
+		};
+
+		cmm1: cmm@fea50000 {
+			compatible = "renesas,r8a77965-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea50000 0 0x1000>;
+			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 710>;
+			resets = <&cpg 710>;
+		};
+
+		cmm3: cmm@fea70000 {
+			compatible = "renesas,r8a77965-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea70000 0 0x1000>;
+			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 708>;
+			resets = <&cpg 708>;
+		};
+
 		csi20: csi2@fea80000 {
 			compatible = "renesas,r8a77965-csi2";
 			reg = <0 0xfea80000 0 0x10000>;
@@ -2467,10 +2494,12 @@
 				 <&cpg CPG_MOD 723>,
 				 <&cpg CPG_MOD 721>;
 			clock-names = "du.0", "du.1", "du.3";
-			status = "disabled";
 
+			renesas,cmms = <&cmm0>, <&cmm1>, <&cmm3>;
 			vsps = <&vspd0 0>, <&vspd1 0>, <&vspd0 1>;
 
+			status = "disabled";
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index 455954c3d98e..bab9b7f96c72 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -1727,6 +1727,24 @@
 			iommus = <&ipmmu_vi0 9>;
 		};
 
+		cmm0: cmm@fea40000 {
+			compatible = "renesas,r8a77990-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea40000 0 0x1000>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 711>;
+			resets = <&cpg 711>;
+		};
+
+		cmm1: cmm@fea50000 {
+			compatible = "renesas,r8a77990-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea50000 0 0x1000>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 710>;
+			resets = <&cpg 710>;
+		};
+
 		csi40: csi2@feaa0000 {
 			compatible = "renesas,r8a77990-csi2";
 			reg = <0 0xfeaa0000 0 0x10000>;
@@ -1768,7 +1786,10 @@
 			clock-names = "du.0", "du.1";
 			resets = <&cpg 724>;
 			reset-names = "du.0";
+
+			renesas,cmms = <&cmm0>, <&cmm1>;
 			vsps = <&vspd0 0>, <&vspd1 0>;
+
 			status = "disabled";
 
 			ports {
diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
index 183fef86cf7c..871c70cc2d2e 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
@@ -993,6 +993,24 @@
 			iommus = <&ipmmu_vi0 9>;
 		};
 
+		cmm0: cmm@fea40000 {
+			compatible = "renesas,r8a77995-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea40000 0 0x1000>;
+			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 711>;
+			resets = <&cpg 711>;
+		};
+
+		cmm1: cmm@fea50000 {
+			compatible = "renesas,r8a77995-cmm",
+				     "renesas,rcar-gen3-cmm";
+			reg = <0 0xfea50000 0 0x1000>;
+			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 710>;
+			resets = <&cpg 710>;
+		};
+
 		du: display@feb00000 {
 			compatible = "renesas,du-r8a77995";
 			reg = <0 0xfeb00000 0 0x40000>;
@@ -1003,7 +1021,10 @@
 			clock-names = "du.0", "du.1";
 			resets = <&cpg 724>;
 			reset-names = "du.0";
+
+			renesas,cmms = <&cmm0>, <&cmm1>;
 			vsps = <&vspd0 0>, <&vspd1 0>;
+
 			status = "disabled";
 
 			ports {
-- 
2.23.0

Re: [PATCH v6 7/8] arm64: dts: renesas: Add CMM units to Gen3 SoCs

[Geert Uytterhoeven]

On Wed, Oct 16, 2019 at 10:55 AM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> Add CMM units to Renesas R-Car Gen3 SoC that support it, and reference them
> from the Display Unit they are connected to.
>
> Sort the 'vsps', 'renesas,cmm' and 'status' properties in the DU unit
> consistently in all the involved DTS.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.5.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v6 7/8] arm64: dts: renesas: Add CMM units to Gen3 SoCs

[Geert Uytterhoeven]

On Wed, Oct 16, 2019 at 10:55 AM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> Add CMM units to Renesas R-Car Gen3 SoC that support it, and reference them
> from the Display Unit they are connected to.
>
> Sort the 'vsps', 'renesas,cmm' and 'status' properties in the DU unit
> consistently in all the involved DTS.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.5.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v6 8/8] drm: rcar-du: kms: Expand comment in vsps parsing routine

[Jacopo Mondi]

Expand comment in the 'vsps' parsing routine to specify the LIF
channel index defaults to 0 in case the second cell of the property
is not specified to remain compatible with older DT bindings.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/gpu/drm/rcar-du/rcar_du_kms.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 7c9fb5860e54..186422ac552b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -587,7 +587,11 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
 
 		vsps[j].crtcs_mask |= BIT(i);
 
-		/* Store the VSP pointer and pipe index in the CRTC. */
+		/*
+		 * Store the VSP pointer and pipe index in the CRTC. If the
+		 * second cell of the 'vsps' specifier isn't present, default
+		 * to 0 to remain compatible with older DT bindings.
+		 */
 		rcdu->crtcs[i].vsp = &rcdu->vsps[j];
 		rcdu->crtcs[i].vsp_pipe = cells >= 1 ? args.args[0] : 0;
 	}
-- 
2.23.0

Re: [PATCH v6 0/8] drm: rcar-du: Add Color Management Module (CMM)

[Laurent Pinchart]

Hi Jacopo,

Thank you for your work.

On Wed, Oct 16, 2019 at 10:55:40AM +0200, Jacopo Mondi wrote:
> Minimal increment to the CMM series, this time should really be the last one.
> 
> Just missing Rob's ack on [1/8] and Laurent's one on [5/8].
> 
> Changelog is minimal:
> CMM
> - Remove the cmm_config.enable flag. The cmm_config.table field validity is
>   used to enable/disable the LUT operations
> - Expand comments as suggested by Laurent
> 
> CRTC
> - use drm_color_lut_size() to check the LUT table size
> - Inline calls to rcar_cmm_enable()/disable()
> - Add TODO entries as suggested by Laurent
> 
> For the record, the full series changelog is available at:
> https://paste.debian.net/1107427/
> 
> v5 from yesterday with informations on testing is available at:
> https://lkml.org/lkml/2019/10/15/337
> 
> Geert will you collect for DTS patches for the next release?
> I assume the DU changes go through Laurent instead ?

I've taken patch 1/8 to 6/8 and 8/8 in my tree. I expected Geert to take
7/8.

> Jacopo Mondi (8):
>   dt-bindings: display: renesas,cmm: Add R-Car CMM documentation
>   dt-bindings: display, renesas,du: Document cmms property
>   drm: rcar-du: Add support for CMM
>   drm: rcar-du: kms: Initialize CMM instances
>   drm: rcar-du: crtc: Control CMM operations
>   drm: rcar-du: crtc: Register GAMMA_LUT properties
>   arm64: dts: renesas: Add CMM units to Gen3 SoCs
>   drm: rcar-du: kms: Expand comment in vsps parsing routine
> 
>  .../bindings/display/renesas,cmm.yaml         |  67 ++++++
>  .../bindings/display/renesas,du.txt           |   5 +
>  arch/arm64/boot/dts/renesas/r8a7795.dtsi      |  39 ++++
>  arch/arm64/boot/dts/renesas/r8a7796.dtsi      |  31 ++-
>  arch/arm64/boot/dts/renesas/r8a77965.dtsi     |  31 ++-
>  arch/arm64/boot/dts/renesas/r8a77990.dtsi     |  21 ++
>  arch/arm64/boot/dts/renesas/r8a77995.dtsi     |  21 ++
>  drivers/gpu/drm/rcar-du/Kconfig               |   7 +
>  drivers/gpu/drm/rcar-du/Makefile              |   1 +
>  drivers/gpu/drm/rcar-du/rcar_cmm.c            | 212 ++++++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_cmm.h            |  58 +++++
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  65 ++++++
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h        |   2 +
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h         |   2 +
>  drivers/gpu/drm/rcar-du/rcar_du_group.c       |  10 +
>  drivers/gpu/drm/rcar-du/rcar_du_group.h       |   2 +
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c         |  82 ++++++-
>  drivers/gpu/drm/rcar-du/rcar_du_regs.h        |   5 +
>  18 files changed, 658 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/display/renesas,cmm.yaml
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 0/8] drm: rcar-du: Add Color Management Module (CMM)

[Laurent Pinchart]

Hi Jacopo,

Thank you for your work.

On Wed, Oct 16, 2019 at 10:55:40AM +0200, Jacopo Mondi wrote:
> Minimal increment to the CMM series, this time should really be the last one.
> 
> Just missing Rob's ack on [1/8] and Laurent's one on [5/8].
> 
> Changelog is minimal:
> CMM
> - Remove the cmm_config.enable flag. The cmm_config.table field validity is
>   used to enable/disable the LUT operations
> - Expand comments as suggested by Laurent
> 
> CRTC
> - use drm_color_lut_size() to check the LUT table size
> - Inline calls to rcar_cmm_enable()/disable()
> - Add TODO entries as suggested by Laurent
> 
> For the record, the full series changelog is available at:
> https://paste.debian.net/1107427/
> 
> v5 from yesterday with informations on testing is available at:
> https://lkml.org/lkml/2019/10/15/337
> 
> Geert will you collect for DTS patches for the next release?
> I assume the DU changes go through Laurent instead ?

I've taken patch 1/8 to 6/8 and 8/8 in my tree. I expected Geert to take
7/8.

> Jacopo Mondi (8):
>   dt-bindings: display: renesas,cmm: Add R-Car CMM documentation
>   dt-bindings: display, renesas,du: Document cmms property
>   drm: rcar-du: Add support for CMM
>   drm: rcar-du: kms: Initialize CMM instances
>   drm: rcar-du: crtc: Control CMM operations
>   drm: rcar-du: crtc: Register GAMMA_LUT properties
>   arm64: dts: renesas: Add CMM units to Gen3 SoCs
>   drm: rcar-du: kms: Expand comment in vsps parsing routine
> 
>  .../bindings/display/renesas,cmm.yaml         |  67 ++++++
>  .../bindings/display/renesas,du.txt           |   5 +
>  arch/arm64/boot/dts/renesas/r8a7795.dtsi      |  39 ++++
>  arch/arm64/boot/dts/renesas/r8a7796.dtsi      |  31 ++-
>  arch/arm64/boot/dts/renesas/r8a77965.dtsi     |  31 ++-
>  arch/arm64/boot/dts/renesas/r8a77990.dtsi     |  21 ++
>  arch/arm64/boot/dts/renesas/r8a77995.dtsi     |  21 ++
>  drivers/gpu/drm/rcar-du/Kconfig               |   7 +
>  drivers/gpu/drm/rcar-du/Makefile              |   1 +
>  drivers/gpu/drm/rcar-du/rcar_cmm.c            | 212 ++++++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_cmm.h            |  58 +++++
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  65 ++++++
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h        |   2 +
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h         |   2 +
>  drivers/gpu/drm/rcar-du/rcar_du_group.c       |  10 +
>  drivers/gpu/drm/rcar-du/rcar_du_group.h       |   2 +
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c         |  82 ++++++-
>  drivers/gpu/drm/rcar-du/rcar_du_regs.h        |   5 +
>  18 files changed, 658 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/display/renesas,cmm.yaml
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h
> 

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v6 0/8] drm: rcar-du: Add Color Management Module (CMM)

[Geert Uytterhoeven]

Hi Laurent,

On Thu, Oct 17, 2019 at 9:14 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Wed, Oct 16, 2019 at 10:55:40AM +0200, Jacopo Mondi wrote:
> > Minimal increment to the CMM series, this time should really be the last one.
> >
> > Just missing Rob's ack on [1/8] and Laurent's one on [5/8].
> >
> > Changelog is minimal:
> > CMM
> > - Remove the cmm_config.enable flag. The cmm_config.table field validity is
> >   used to enable/disable the LUT operations
> > - Expand comments as suggested by Laurent
> >
> > CRTC
> > - use drm_color_lut_size() to check the LUT table size
> > - Inline calls to rcar_cmm_enable()/disable()
> > - Add TODO entries as suggested by Laurent
> >
> > For the record, the full series changelog is available at:
> > https://paste.debian.net/1107427/
> >
> > v5 from yesterday with informations on testing is available at:
> > https://lkml.org/lkml/2019/10/15/337
> >
> > Geert will you collect for DTS patches for the next release?
> > I assume the DU changes go through Laurent instead ?
>
> I've taken patch 1/8 to 6/8 and 8/8 in my tree. I expected Geert to take
> 7/8.

And so I did. 7/8 is now in arm-soc/for-next.

However, your drm/du/next branch seems to contain 7/8 instead of 8/8?

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v6 0/8] drm: rcar-du: Add Color Management Module (CMM)

[Geert Uytterhoeven]

Hi Laurent,

On Thu, Oct 17, 2019 at 9:14 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Wed, Oct 16, 2019 at 10:55:40AM +0200, Jacopo Mondi wrote:
> > Minimal increment to the CMM series, this time should really be the last one.
> >
> > Just missing Rob's ack on [1/8] and Laurent's one on [5/8].
> >
> > Changelog is minimal:
> > CMM
> > - Remove the cmm_config.enable flag. The cmm_config.table field validity is
> >   used to enable/disable the LUT operations
> > - Expand comments as suggested by Laurent
> >
> > CRTC
> > - use drm_color_lut_size() to check the LUT table size
> > - Inline calls to rcar_cmm_enable()/disable()
> > - Add TODO entries as suggested by Laurent
> >
> > For the record, the full series changelog is available at:
> > https://paste.debian.net/1107427/
> >
> > v5 from yesterday with informations on testing is available at:
> > https://lkml.org/lkml/2019/10/15/337
> >
> > Geert will you collect for DTS patches for the next release?
> > I assume the DU changes go through Laurent instead ?
>
> I've taken patch 1/8 to 6/8 and 8/8 in my tree. I expected Geert to take
> 7/8.

And so I did. 7/8 is now in arm-soc/for-next.

However, your drm/du/next branch seems to contain 7/8 instead of 8/8?

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v6 0/8] drm: rcar-du: Add Color Management Module (CMM)

[Laurent Pinchart]

Hi Geert,

On Tue, Nov 05, 2019 at 05:14:20PM +0100, Geert Uytterhoeven wrote:
> On Thu, Oct 17, 2019 at 9:14 PM Laurent Pinchart wrote:
> > On Wed, Oct 16, 2019 at 10:55:40AM +0200, Jacopo Mondi wrote:
> > > Minimal increment to the CMM series, this time should really be the last one.
> > >
> > > Just missing Rob's ack on [1/8] and Laurent's one on [5/8].
> > >
> > > Changelog is minimal:
> > > CMM
> > > - Remove the cmm_config.enable flag. The cmm_config.table field validity is
> > >   used to enable/disable the LUT operations
> > > - Expand comments as suggested by Laurent
> > >
> > > CRTC
> > > - use drm_color_lut_size() to check the LUT table size
> > > - Inline calls to rcar_cmm_enable()/disable()
> > > - Add TODO entries as suggested by Laurent
> > >
> > > For the record, the full series changelog is available at:
> > > https://paste.debian.net/1107427/
> > >
> > > v5 from yesterday with informations on testing is available at:
> > > https://lkml.org/lkml/2019/10/15/337
> > >
> > > Geert will you collect for DTS patches for the next release?
> > > I assume the DU changes go through Laurent instead ?
> >
> > I've taken patch 1/8 to 6/8 and 8/8 in my tree. I expected Geert to take
> > 7/8.
> 
> And so I did. 7/8 is now in arm-soc/for-next.
> 
> However, your drm/du/next branch seems to contain 7/8 instead of 8/8?

That was a mistake that I fixed in a second pull request (see
https://lists.freedesktop.org/archives/dri-devel/2019-October/241110.html),
I forgot to update the drm/du/next branch, but the tag should be
correct. I've now fixed the problem in the branch as well.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 0/8] drm: rcar-du: Add Color Management Module (CMM)

[Laurent Pinchart]

Hi Geert,

On Tue, Nov 05, 2019 at 05:14:20PM +0100, Geert Uytterhoeven wrote:
> On Thu, Oct 17, 2019 at 9:14 PM Laurent Pinchart wrote:
> > On Wed, Oct 16, 2019 at 10:55:40AM +0200, Jacopo Mondi wrote:
> > > Minimal increment to the CMM series, this time should really be the last one.
> > >
> > > Just missing Rob's ack on [1/8] and Laurent's one on [5/8].
> > >
> > > Changelog is minimal:
> > > CMM
> > > - Remove the cmm_config.enable flag. The cmm_config.table field validity is
> > >   used to enable/disable the LUT operations
> > > - Expand comments as suggested by Laurent
> > >
> > > CRTC
> > > - use drm_color_lut_size() to check the LUT table size
> > > - Inline calls to rcar_cmm_enable()/disable()
> > > - Add TODO entries as suggested by Laurent
> > >
> > > For the record, the full series changelog is available at:
> > > https://paste.debian.net/1107427/
> > >
> > > v5 from yesterday with informations on testing is available at:
> > > https://lkml.org/lkml/2019/10/15/337
> > >
> > > Geert will you collect for DTS patches for the next release?
> > > I assume the DU changes go through Laurent instead ?
> >
> > I've taken patch 1/8 to 6/8 and 8/8 in my tree. I expected Geert to take
> > 7/8.
> 
> And so I did. 7/8 is now in arm-soc/for-next.
> 
> However, your drm/du/next branch seems to contain 7/8 instead of 8/8?

That was a mistake that I fixed in a second pull request (see
https://lists.freedesktop.org/archives/dri-devel/2019-October/241110.html),
I forgot to update the drm/du/next branch, but the tag should be
correct. I've now fixed the problem in the branch as well.

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel