[PATCH v2 0/7] Ingenic-drm improvements + new pixel formats

[PATCH v2 0/7] Ingenic-drm improvements + new pixel formats

[Paul Cercueil]

Hi,

This is a V2 of my small patchset "Small improvements to ingenic-drm"
that I sent about two weeks ago. In that time, I worked on new
improvements, so I thought I'd just add them to the patchset, since
I needed a V2 anyway.

Sam: the patches you acked have been all slightly modified, I kept your
acked-by on them, please tell me if that's OK.

Cheers,
-Paul

Paul Cercueil (7):
  drm/ingenic: Reset pixclock rate when parent clock rate changes
  drm/ingenic: Add support for reserved memory
  drm/ingenic: Alloc F0 and F1 DMA descriptors at once
  drm/ingenic: Support handling different pixel formats in F0/F1 planes
  drm/ingenic: Add support for paletted 8bpp
  drm/ingenic: Add support for 30-bit modes
  drm/ingenic: Add support for 24-bit modes

 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 262 +++++++++++++++++++---
 drivers/gpu/drm/ingenic/ingenic-drm.h     |   3 +
 2 files changed, 228 insertions(+), 37 deletions(-)

-- 
2.28.0

[PATCH v2 1/7] drm/ingenic: Reset pixclock rate when parent clock rate changes

[Paul Cercueil]

Old Ingenic SoCs can overclock very well, up to +50% of their nominal
clock rate, whithout requiring overvolting or anything like that, just
by changing the rate of the main PLL. Unfortunately, all clocks on the
system are derived from that PLL, and when the PLL rate is updated, so
is our pixel clock.

To counter that issue, we make sure that the panel is in VBLANK before
the rate change happens, and we will then re-set the pixel clock rate
afterwards, once the PLL has been changed, to be as close as possible to
the pixel rate requested by the encoder.

v2: Add comment about mutex usage

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 61 ++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 937d080f5d06..eadfe3a20bf1 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -12,6 +12,7 @@
 #include <linux/dma-noncoherent.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -71,6 +72,21 @@ struct ingenic_drm {
 
 	bool panel_is_sharp;
 	bool no_vblank;
+
+	/*
+	 * clk_mutex is used to synchronize the pixel clock rate update with
+	 * the VBLANK. When the pixel clock's parent clock needs to be updated,
+	 * clock_nb's notifier function will lock the mutex, then wait until the
+	 * next VBLANK. At that point, the parent clock's rate can be updated,
+	 * and the mutex is then unlocked. If an atomic commit happens in the
+	 * meantime, it will lock on the mutex, effectively waiting until the
+	 * clock update process finishes. Finally, the pixel clock's rate will
+	 * be recomputed when the mutex has been released, in the pending atomic
+	 * commit, or a future one.
+	 */
+	struct mutex clk_mutex;
+	bool update_clk_rate;
+	struct notifier_block clock_nb;
 };
 
 static const u32 ingenic_drm_primary_formats[] = {
@@ -119,6 +135,29 @@ static inline struct ingenic_drm *drm_crtc_get_priv(struct drm_crtc *crtc)
 	return container_of(crtc, struct ingenic_drm, crtc);
 }
 
+static inline struct ingenic_drm *drm_nb_get_priv(struct notifier_block *nb)
+{
+	return container_of(nb, struct ingenic_drm, clock_nb);
+}
+
+static int ingenic_drm_update_pixclk(struct notifier_block *nb,
+				     unsigned long action,
+				     void *data)
+{
+	struct ingenic_drm *priv = drm_nb_get_priv(nb);
+
+	switch (action) {
+	case PRE_RATE_CHANGE:
+		mutex_lock(&priv->clk_mutex);
+		priv->update_clk_rate = true;
+		drm_crtc_wait_one_vblank(&priv->crtc);
+		return NOTIFY_OK;
+	default:
+		mutex_unlock(&priv->clk_mutex);
+		return NOTIFY_OK;
+	}
+}
+
 static void ingenic_drm_crtc_atomic_enable(struct drm_crtc *crtc,
 					   struct drm_crtc_state *state)
 {
@@ -284,8 +323,14 @@ static void ingenic_drm_crtc_atomic_flush(struct drm_crtc *crtc,
 
 	if (drm_atomic_crtc_needs_modeset(state)) {
 		ingenic_drm_crtc_update_timings(priv, &state->mode);
+		priv->update_clk_rate = true;
+	}
 
+	if (priv->update_clk_rate) {
+		mutex_lock(&priv->clk_mutex);
 		clk_set_rate(priv->pix_clk, state->adjusted_mode.clock * 1000);
+		priv->update_clk_rate = false;
+		mutex_unlock(&priv->clk_mutex);
 	}
 
 	if (event) {
@@ -1044,16 +1089,28 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 	if (soc_info->has_osd)
 		regmap_write(priv->map, JZ_REG_LCD_OSDC, JZ_LCD_OSDC_OSDEN);
 
+	mutex_init(&priv->clk_mutex);
+	priv->clock_nb.notifier_call = ingenic_drm_update_pixclk;
+
+	parent_clk = clk_get_parent(priv->pix_clk);
+	ret = clk_notifier_register(parent_clk, &priv->clock_nb);
+	if (ret) {
+		dev_err(dev, "Unable to register clock notifier\n");
+		goto err_devclk_disable;
+	}
+
 	ret = drm_dev_register(drm, 0);
 	if (ret) {
 		dev_err(dev, "Failed to register DRM driver\n");
-		goto err_devclk_disable;
+		goto err_clk_notifier_unregister;
 	}
 
 	drm_fbdev_generic_setup(drm, 32);
 
 	return 0;
 
+err_clk_notifier_unregister:
+	clk_notifier_unregister(parent_clk, &priv->clock_nb);
 err_devclk_disable:
 	if (priv->lcd_clk)
 		clk_disable_unprepare(priv->lcd_clk);
@@ -1075,7 +1132,9 @@ static int compare_of(struct device *dev, void *data)
 static void ingenic_drm_unbind(struct device *dev)
 {
 	struct ingenic_drm *priv = dev_get_drvdata(dev);
+	struct clk *parent_clk = clk_get_parent(priv->pix_clk);
 
+	clk_notifier_unregister(parent_clk, &priv->clock_nb);
 	if (priv->lcd_clk)
 		clk_disable_unprepare(priv->lcd_clk);
 	clk_disable_unprepare(priv->pix_clk);
-- 
2.28.0

[PATCH v2 2/7] drm/ingenic: Add support for reserved memory

[Paul Cercueil]

Add support for static memory reserved from Device Tree. Since we're
using GEM buffers backed by CMA, it is interesting to have an option to
specify the CMA area where the GEM buffers will be allocated.

v2: Don't abort probe if reserved memory cannot be obtained. The driver
    will still work fine provided the kernel configuration is sane.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index eadfe3a20bf1..d34e76f5f57d 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of_device.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 
@@ -837,6 +838,11 @@ static void ingenic_drm_unbind_all(void *d)
 	component_unbind_all(priv->dev, &priv->drm);
 }
 
+static void __maybe_unused ingenic_drm_release_rmem(void *d)
+{
+	of_reserved_mem_device_release(d);
+}
+
 static int ingenic_drm_bind(struct device *dev, bool has_components)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -858,6 +864,19 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 		return -EINVAL;
 	}
 
+	if (IS_ENABLED(CONFIG_OF_RESERVED_MEM)) {
+		ret = of_reserved_mem_device_init(dev);
+
+		if (ret && ret != -ENODEV)
+			dev_warn(dev, "Failed to get reserved memory: %d\n", ret);
+
+		if (!ret) {
+			ret = devm_add_action_or_reset(dev, ingenic_drm_release_rmem, dev);
+			if (ret)
+				return ret;
+		}
+	}
+
 	priv = devm_drm_dev_alloc(dev, &ingenic_drm_driver_data,
 				  struct ingenic_drm, drm);
 	if (IS_ERR(priv))
-- 
2.28.0

[PATCH v2 3/7] drm/ingenic: Alloc F0 and F1 DMA descriptors at once

[Paul Cercueil]

Instead of calling dmam_alloc_coherent() once for each 4-bit DMA
hardware descriptor, we can have them both in a physical memory page, as
long as they are aligned to 16 bytes. This reduces memory consumption,
and will make it easier to add more DMA descriptors in the future.

Note that the old code would not create the F0 descriptor on SoCs that
don't support multiple planes. We don't care, because:
- we don't use more memory by allocating two descriptors instead of a
  single one;
- the only SoC that does not support multiple planes (JZ4740) still has
  two independent DMA channels, for an unknown reason.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 51 +++++++++++++----------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index d34e76f5f57d..e8d47549ff2e 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -45,7 +45,12 @@ struct ingenic_dma_hwdesc {
 	u32 addr;
 	u32 id;
 	u32 cmd;
-} __packed;
+} __aligned(16);
+
+struct ingenic_dma_hwdescs {
+	struct ingenic_dma_hwdesc hwdesc_f0;
+	struct ingenic_dma_hwdesc hwdesc_f1;
+};
 
 struct jz_soc_info {
 	bool needs_dev_clk;
@@ -68,8 +73,8 @@ struct ingenic_drm {
 	struct clk *lcd_clk, *pix_clk;
 	const struct jz_soc_info *soc_info;
 
-	struct ingenic_dma_hwdesc *dma_hwdesc_f0, *dma_hwdesc_f1;
-	dma_addr_t dma_hwdesc_phys_f0, dma_hwdesc_phys_f1;
+	struct ingenic_dma_hwdescs *dma_hwdescs;
+	dma_addr_t dma_hwdescs_phys;
 
 	bool panel_is_sharp;
 	bool no_vblank;
@@ -546,9 +551,9 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
 		cpp = state->fb->format->cpp[0];
 
 		if (priv->soc_info->has_osd && plane->type == DRM_PLANE_TYPE_OVERLAY)
-			hwdesc = priv->dma_hwdesc_f0;
+			hwdesc = &priv->dma_hwdescs->hwdesc_f0;
 		else
-			hwdesc = priv->dma_hwdesc_f1;
+			hwdesc = &priv->dma_hwdescs->hwdesc_f1;
 
 		hwdesc->addr = addr;
 		hwdesc->cmd = JZ_LCD_CMD_EOF_IRQ | (width * height * cpp / 4);
@@ -856,6 +861,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 	void __iomem *base;
 	long parent_rate;
 	unsigned int i, clone_mask = 0;
+	dma_addr_t dma_hwdesc_phys_f0, dma_hwdesc_phys_f1;
 	int ret, irq;
 
 	soc_info = of_device_get_match_data(dev);
@@ -930,26 +936,25 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 		return PTR_ERR(priv->pix_clk);
 	}
 
-	priv->dma_hwdesc_f1 = dmam_alloc_coherent(dev, sizeof(*priv->dma_hwdesc_f1),
-						  &priv->dma_hwdesc_phys_f1,
-						  GFP_KERNEL);
-	if (!priv->dma_hwdesc_f1)
+	priv->dma_hwdescs = dmam_alloc_coherent(dev,
+						sizeof(*priv->dma_hwdescs),
+						&priv->dma_hwdescs_phys,
+						GFP_KERNEL);
+	if (!priv->dma_hwdescs)
 		return -ENOMEM;
 
-	priv->dma_hwdesc_f1->next = priv->dma_hwdesc_phys_f1;
-	priv->dma_hwdesc_f1->id = 0xf1;
 
-	if (priv->soc_info->has_osd) {
-		priv->dma_hwdesc_f0 = dmam_alloc_coherent(dev,
-							  sizeof(*priv->dma_hwdesc_f0),
-							  &priv->dma_hwdesc_phys_f0,
-							  GFP_KERNEL);
-		if (!priv->dma_hwdesc_f0)
-			return -ENOMEM;
+	/* Configure DMA hwdesc for foreground0 plane */
+	dma_hwdesc_phys_f0 = priv->dma_hwdescs_phys
+		+ offsetof(struct ingenic_dma_hwdescs, hwdesc_f0);
+	priv->dma_hwdescs->hwdesc_f0.next = dma_hwdesc_phys_f0;
+	priv->dma_hwdescs->hwdesc_f0.id = 0xf0;
 
-		priv->dma_hwdesc_f0->next = priv->dma_hwdesc_phys_f0;
-		priv->dma_hwdesc_f0->id = 0xf0;
-	}
+	/* Configure DMA hwdesc for foreground1 plane */
+	dma_hwdesc_phys_f1 = priv->dma_hwdescs_phys
+		+ offsetof(struct ingenic_dma_hwdescs, hwdesc_f1);
+	priv->dma_hwdescs->hwdesc_f1.next = dma_hwdesc_phys_f1;
+	priv->dma_hwdescs->hwdesc_f1.id = 0xf1;
 
 	if (soc_info->has_osd)
 		priv->ipu_plane = drm_plane_from_index(drm, 0);
@@ -1101,8 +1106,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 	}
 
 	/* Set address of our DMA descriptor chain */
-	regmap_write(priv->map, JZ_REG_LCD_DA0, priv->dma_hwdesc_phys_f0);
-	regmap_write(priv->map, JZ_REG_LCD_DA1, priv->dma_hwdesc_phys_f1);
+	regmap_write(priv->map, JZ_REG_LCD_DA0, dma_hwdesc_phys_f0);
+	regmap_write(priv->map, JZ_REG_LCD_DA1, dma_hwdesc_phys_f1);
 
 	/* Enable OSD if available */
 	if (soc_info->has_osd)
-- 
2.28.0

[PATCH v2 4/7] drm/ingenic: Support handling different pixel formats in F0/F1 planes

[Paul Cercueil]

Until now the ingenic-drm driver supported the same pixel formats on the
F0 and F1 planes, and across all SoCs. However, the F0 plane does support
paletted 8bpp, while the F1 plane doesn't.

Furthermore, the three SoCs currently supported all have different pixel
formats available; 24bpp was added in JZ4725B, 30bpp was added in
JZ4770.

Prepare the inclusion of paletted 8bpp, 24bpp and 30bpp support by
having separate pixel format lists for F0 and F1 planes.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 57 +++++++++++++++++++----
 1 file changed, 47 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index e8d47549ff2e..567facfb7217 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -56,6 +56,8 @@ struct jz_soc_info {
 	bool needs_dev_clk;
 	bool has_osd;
 	unsigned int max_width, max_height;
+	const u32 *formats_f0, *formats_f1;
+	unsigned int num_formats_f0, num_formats_f1;
 };
 
 struct ingenic_drm {
@@ -95,12 +97,6 @@ struct ingenic_drm {
 	struct notifier_block clock_nb;
 };
 
-static const u32 ingenic_drm_primary_formats[] = {
-	DRM_FORMAT_XRGB1555,
-	DRM_FORMAT_RGB565,
-	DRM_FORMAT_XRGB8888,
-};
-
 static bool ingenic_drm_cached_gem_buf;
 module_param_named(cached_gem_buffers, ingenic_drm_cached_gem_buf, bool, 0400);
 MODULE_PARM_DESC(cached_gem_buffers,
@@ -963,8 +959,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 
 	ret = drm_universal_plane_init(drm, &priv->f1, 1,
 				       &ingenic_drm_primary_plane_funcs,
-				       ingenic_drm_primary_formats,
-				       ARRAY_SIZE(ingenic_drm_primary_formats),
+				       priv->soc_info->formats_f1,
+				       priv->soc_info->num_formats_f1,
 				       NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
 	if (ret) {
 		dev_err(dev, "Failed to register plane: %i\n", ret);
@@ -988,8 +984,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 
 		ret = drm_universal_plane_init(drm, &priv->f0, 1,
 					       &ingenic_drm_primary_plane_funcs,
-					       ingenic_drm_primary_formats,
-					       ARRAY_SIZE(ingenic_drm_primary_formats),
+					       priv->soc_info->formats_f0,
+					       priv->soc_info->num_formats_f0,
 					       NULL, DRM_PLANE_TYPE_OVERLAY,
 					       NULL);
 		if (ret) {
@@ -1204,11 +1200,44 @@ static int ingenic_drm_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const u32 jz4740_formats[] = {
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+};
+
+static const u32 jz4725b_formats_f1[] = {
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+};
+
+static const u32 jz4725b_formats_f0[] = {
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+};
+
+static const u32 jz4770_formats_f1[] = {
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+};
+
+static const u32 jz4770_formats_f0[] = {
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+};
+
 static const struct jz_soc_info jz4740_soc_info = {
 	.needs_dev_clk = true,
 	.has_osd = false,
 	.max_width = 800,
 	.max_height = 600,
+	.formats_f1 = jz4740_formats,
+	.num_formats_f1 = ARRAY_SIZE(jz4740_formats),
+	/* JZ4740 has only one plane */
 };
 
 static const struct jz_soc_info jz4725b_soc_info = {
@@ -1216,6 +1245,10 @@ static const struct jz_soc_info jz4725b_soc_info = {
 	.has_osd = true,
 	.max_width = 800,
 	.max_height = 600,
+	.formats_f1 = jz4725b_formats_f1,
+	.num_formats_f1 = ARRAY_SIZE(jz4725b_formats_f1),
+	.formats_f0 = jz4725b_formats_f0,
+	.num_formats_f0 = ARRAY_SIZE(jz4725b_formats_f0),
 };
 
 static const struct jz_soc_info jz4770_soc_info = {
@@ -1223,6 +1256,10 @@ static const struct jz_soc_info jz4770_soc_info = {
 	.has_osd = true,
 	.max_width = 1280,
 	.max_height = 720,
+	.formats_f1 = jz4770_formats_f1,
+	.num_formats_f1 = ARRAY_SIZE(jz4770_formats_f1),
+	.formats_f0 = jz4770_formats_f0,
+	.num_formats_f0 = ARRAY_SIZE(jz4770_formats_f0),
 };
 
 static const struct of_device_id ingenic_drm_of_match[] = {
-- 
2.28.0

[PATCH v2 5/7] drm/ingenic: Add support for paletted 8bpp

[Paul Cercueil]

On JZ4725B and newer, the F0 plane supports paletted 8bpp with a
256-entry palette. Add support for it.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 60 +++++++++++++++++++++--
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 567facfb7217..48e88827f332 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -21,6 +21,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
+#include <drm/drm_color_mgmt.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_damage_helper.h>
@@ -50,6 +51,8 @@ struct ingenic_dma_hwdesc {
 struct ingenic_dma_hwdescs {
 	struct ingenic_dma_hwdesc hwdesc_f0;
 	struct ingenic_dma_hwdesc hwdesc_f1;
+	struct ingenic_dma_hwdesc hwdesc_pal;
+	u16 palette[256] __aligned(16);
 };
 
 struct jz_soc_info {
@@ -464,6 +467,9 @@ void ingenic_drm_plane_config(struct device *dev,
 				   JZ_LCD_OSDCTRL_BPP_MASK, ctrl);
 	} else {
 		switch (fourcc) {
+		case DRM_FORMAT_C8:
+			ctrl |= JZ_LCD_CTRL_BPP_8;
+			break;
 		case DRM_FORMAT_XRGB1555:
 			ctrl |= JZ_LCD_CTRL_RGB555;
 			fallthrough;
@@ -529,16 +535,34 @@ void ingenic_drm_sync_data(struct device *dev,
 	}
 }
 
+static void ingenic_drm_update_palette(struct ingenic_drm *priv,
+				       const struct drm_color_lut *lut)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(priv->dma_hwdescs->palette); i++) {
+		u16 color = drm_color_lut_extract(lut[i].red, 5) << 11
+			| drm_color_lut_extract(lut[i].green, 6) << 5
+			| drm_color_lut_extract(lut[i].blue, 5);
+
+		priv->dma_hwdescs->palette[i] = color;
+	}
+}
+
 static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
 					    struct drm_plane_state *oldstate)
 {
 	struct ingenic_drm *priv = drm_device_get_priv(plane->dev);
 	struct drm_plane_state *state = plane->state;
+	struct drm_crtc_state *crtc_state;
 	struct ingenic_dma_hwdesc *hwdesc;
-	unsigned int width, height, cpp;
+	unsigned int width, height, cpp, offset;
 	dma_addr_t addr;
+	u32 fourcc;
 
 	if (state && state->fb) {
+		crtc_state = state->crtc->state;
+
 		ingenic_drm_sync_data(priv->dev, oldstate, state);
 
 		addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
@@ -554,9 +578,23 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
 		hwdesc->addr = addr;
 		hwdesc->cmd = JZ_LCD_CMD_EOF_IRQ | (width * height * cpp / 4);
 
-		if (drm_atomic_crtc_needs_modeset(state->crtc->state))
-			ingenic_drm_plane_config(priv->dev, plane,
-						 state->fb->format->format);
+		if (drm_atomic_crtc_needs_modeset(crtc_state)) {
+			fourcc = state->fb->format->format;
+
+			ingenic_drm_plane_config(priv->dev, plane, fourcc);
+
+			if (fourcc == DRM_FORMAT_C8)
+				offset = offsetof(struct ingenic_dma_hwdescs, hwdesc_pal);
+			else
+				offset = offsetof(struct ingenic_dma_hwdescs, hwdesc_f0);
+
+			priv->dma_hwdescs->hwdesc_f0.next = priv->dma_hwdescs_phys + offset;
+
+			crtc_state->color_mgmt_changed = fourcc == DRM_FORMAT_C8;
+		}
+
+		if (crtc_state->color_mgmt_changed)
+			ingenic_drm_update_palette(priv, crtc_state->gamma_lut->data);
 	}
 }
 
@@ -952,6 +990,15 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 	priv->dma_hwdescs->hwdesc_f1.next = dma_hwdesc_phys_f1;
 	priv->dma_hwdescs->hwdesc_f1.id = 0xf1;
 
+	/* Configure DMA hwdesc for palette */
+	priv->dma_hwdescs->hwdesc_pal.next = priv->dma_hwdescs_phys
+		+ offsetof(struct ingenic_dma_hwdescs, hwdesc_f0);
+	priv->dma_hwdescs->hwdesc_pal.id = 0xc0;
+	priv->dma_hwdescs->hwdesc_pal.addr = priv->dma_hwdescs_phys
+		+ offsetof(struct ingenic_dma_hwdescs, palette);
+	priv->dma_hwdescs->hwdesc_pal.cmd = JZ_LCD_CMD_ENABLE_PAL
+		| (sizeof(priv->dma_hwdescs->palette) / 4);
+
 	if (soc_info->has_osd)
 		priv->ipu_plane = drm_plane_from_index(drm, 0);
 
@@ -978,6 +1025,9 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 		return ret;
 	}
 
+	drm_crtc_enable_color_mgmt(&priv->crtc, 0, false,
+				   ARRAY_SIZE(priv->dma_hwdescs->palette));
+
 	if (soc_info->has_osd) {
 		drm_plane_helper_add(&priv->f0,
 				     &ingenic_drm_plane_helper_funcs);
@@ -1213,6 +1263,7 @@ static const u32 jz4725b_formats_f1[] = {
 };
 
 static const u32 jz4725b_formats_f0[] = {
+	DRM_FORMAT_C8,
 	DRM_FORMAT_XRGB1555,
 	DRM_FORMAT_RGB565,
 	DRM_FORMAT_XRGB8888,
@@ -1225,6 +1276,7 @@ static const u32 jz4770_formats_f1[] = {
 };
 
 static const u32 jz4770_formats_f0[] = {
+	DRM_FORMAT_C8,
 	DRM_FORMAT_XRGB1555,
 	DRM_FORMAT_RGB565,
 	DRM_FORMAT_XRGB8888,
-- 
2.28.0

Re: [PATCH v2 5/7] drm/ingenic: Add support for paletted 8bpp

[Sam Ravnborg]

Hi Paul.

On Sat, Sep 26, 2020 at 07:04:59PM +0200, Paul Cercueil wrote:
> On JZ4725B and newer, the F0 plane supports paletted 8bpp with a
> 256-entry palette. Add support for it.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 60 +++++++++++++++++++++--
>  1 file changed, 56 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index 567facfb7217..48e88827f332 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -21,6 +21,7 @@
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_bridge.h>
> +#include <drm/drm_color_mgmt.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_damage_helper.h>
> @@ -50,6 +51,8 @@ struct ingenic_dma_hwdesc {
>  struct ingenic_dma_hwdescs {
>  	struct ingenic_dma_hwdesc hwdesc_f0;
>  	struct ingenic_dma_hwdesc hwdesc_f1;
> +	struct ingenic_dma_hwdesc hwdesc_pal;
> +	u16 palette[256] __aligned(16);
>  };
>  
>  struct jz_soc_info {
> @@ -464,6 +467,9 @@ void ingenic_drm_plane_config(struct device *dev,
>  				   JZ_LCD_OSDCTRL_BPP_MASK, ctrl);
>  	} else {
>  		switch (fourcc) {
> +		case DRM_FORMAT_C8:
> +			ctrl |= JZ_LCD_CTRL_BPP_8;
> +			break;
>  		case DRM_FORMAT_XRGB1555:
>  			ctrl |= JZ_LCD_CTRL_RGB555;
>  			fallthrough;
> @@ -529,16 +535,34 @@ void ingenic_drm_sync_data(struct device *dev,
>  	}
>  }
>  
> +static void ingenic_drm_update_palette(struct ingenic_drm *priv,
> +				       const struct drm_color_lut *lut)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(priv->dma_hwdescs->palette); i++) {
> +		u16 color = drm_color_lut_extract(lut[i].red, 5) << 11
> +			| drm_color_lut_extract(lut[i].green, 6) << 5
> +			| drm_color_lut_extract(lut[i].blue, 5);
> +
> +		priv->dma_hwdescs->palette[i] = color;
> +	}
> +}
> +
>  static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
>  					    struct drm_plane_state *oldstate)
>  {
>  	struct ingenic_drm *priv = drm_device_get_priv(plane->dev);
>  	struct drm_plane_state *state = plane->state;
> +	struct drm_crtc_state *crtc_state;
>  	struct ingenic_dma_hwdesc *hwdesc;
> -	unsigned int width, height, cpp;
> +	unsigned int width, height, cpp, offset;
>  	dma_addr_t addr;
> +	u32 fourcc;
>  
>  	if (state && state->fb) {
> +		crtc_state = state->crtc->state;
> +
>  		ingenic_drm_sync_data(priv->dev, oldstate, state);
>  
>  		addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
> @@ -554,9 +578,23 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
>  		hwdesc->addr = addr;
>  		hwdesc->cmd = JZ_LCD_CMD_EOF_IRQ | (width * height * cpp / 4);
>  
> -		if (drm_atomic_crtc_needs_modeset(state->crtc->state))
> -			ingenic_drm_plane_config(priv->dev, plane,
> -						 state->fb->format->format);
> +		if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> +			fourcc = state->fb->format->format;
> +
> +			ingenic_drm_plane_config(priv->dev, plane, fourcc);
> +
> +			if (fourcc == DRM_FORMAT_C8)
> +				offset = offsetof(struct ingenic_dma_hwdescs, hwdesc_pal);
> +			else
> +				offset = offsetof(struct ingenic_dma_hwdescs, hwdesc_f0);
> +
> +			priv->dma_hwdescs->hwdesc_f0.next = priv->dma_hwdescs_phys + offset;
> +
> +			crtc_state->color_mgmt_changed = fourcc == DRM_FORMAT_C8;
> +		}
> +
> +		if (crtc_state->color_mgmt_changed)
> +			ingenic_drm_update_palette(priv, crtc_state->gamma_lut->data);
What guarantee the size of gamma_lut->data?
In other word - should drm_color_lut_size() be checked here?
Maybe only accept a fully populated palette?
This is what I can see rcar-du and armada does.

	Sam

>  	}
>  }
>  
> @@ -952,6 +990,15 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
>  	priv->dma_hwdescs->hwdesc_f1.next = dma_hwdesc_phys_f1;
>  	priv->dma_hwdescs->hwdesc_f1.id = 0xf1;
>  
> +	/* Configure DMA hwdesc for palette */
> +	priv->dma_hwdescs->hwdesc_pal.next = priv->dma_hwdescs_phys
> +		+ offsetof(struct ingenic_dma_hwdescs, hwdesc_f0);
> +	priv->dma_hwdescs->hwdesc_pal.id = 0xc0;
> +	priv->dma_hwdescs->hwdesc_pal.addr = priv->dma_hwdescs_phys
> +		+ offsetof(struct ingenic_dma_hwdescs, palette);
> +	priv->dma_hwdescs->hwdesc_pal.cmd = JZ_LCD_CMD_ENABLE_PAL
> +		| (sizeof(priv->dma_hwdescs->palette) / 4);
> +
>  	if (soc_info->has_osd)
>  		priv->ipu_plane = drm_plane_from_index(drm, 0);
>  
> @@ -978,6 +1025,9 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
>  		return ret;
>  	}
>  
> +	drm_crtc_enable_color_mgmt(&priv->crtc, 0, false,
> +				   ARRAY_SIZE(priv->dma_hwdescs->palette));
> +
>  	if (soc_info->has_osd) {
>  		drm_plane_helper_add(&priv->f0,
>  				     &ingenic_drm_plane_helper_funcs);
> @@ -1213,6 +1263,7 @@ static const u32 jz4725b_formats_f1[] = {
>  };
>  
>  static const u32 jz4725b_formats_f0[] = {
> +	DRM_FORMAT_C8,
>  	DRM_FORMAT_XRGB1555,
>  	DRM_FORMAT_RGB565,
>  	DRM_FORMAT_XRGB8888,
> @@ -1225,6 +1276,7 @@ static const u32 jz4770_formats_f1[] = {
>  };
>  
>  static const u32 jz4770_formats_f0[] = {
> +	DRM_FORMAT_C8,
>  	DRM_FORMAT_XRGB1555,
>  	DRM_FORMAT_RGB565,
>  	DRM_FORMAT_XRGB8888,
> -- 
> 2.28.0

[PATCH v2 6/7] drm/ingenic: Add support for 30-bit modes

[Paul Cercueil]

Starting from the JZ4760 SoC, the primary and overlay planes support
30-bit pixel modes (10 bits per color component). Add support for these
in the ingenic-drm driver.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 8 ++++++++
 drivers/gpu/drm/ingenic/ingenic-drm.h     | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 48e88827f332..9e3122b42820 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -461,6 +461,9 @@ void ingenic_drm_plane_config(struct device *dev,
 		case DRM_FORMAT_XRGB8888:
 			ctrl |= JZ_LCD_OSDCTRL_BPP_18_24;
 			break;
+		case DRM_FORMAT_XRGB2101010:
+			ctrl |= JZ_LCD_OSDCTRL_BPP_30;
+			break;
 		}
 
 		regmap_update_bits(priv->map, JZ_REG_LCD_OSDCTRL,
@@ -479,6 +482,9 @@ void ingenic_drm_plane_config(struct device *dev,
 		case DRM_FORMAT_XRGB8888:
 			ctrl |= JZ_LCD_CTRL_BPP_18_24;
 			break;
+		case DRM_FORMAT_XRGB2101010:
+			ctrl |= JZ_LCD_CTRL_BPP_30;
+			break;
 		}
 
 		regmap_update_bits(priv->map, JZ_REG_LCD_CTRL,
@@ -1273,6 +1279,7 @@ static const u32 jz4770_formats_f1[] = {
 	DRM_FORMAT_XRGB1555,
 	DRM_FORMAT_RGB565,
 	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XRGB2101010,
 };
 
 static const u32 jz4770_formats_f0[] = {
@@ -1280,6 +1287,7 @@ static const u32 jz4770_formats_f0[] = {
 	DRM_FORMAT_XRGB1555,
 	DRM_FORMAT_RGB565,
 	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XRGB2101010,
 };
 
 static const struct jz_soc_info jz4740_soc_info = {
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h
index df99f0f75d39..f05e18e6b6fa 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.h
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
@@ -124,6 +124,7 @@
 #define JZ_LCD_CTRL_BPP_8			0x3
 #define JZ_LCD_CTRL_BPP_15_16			0x4
 #define JZ_LCD_CTRL_BPP_18_24			0x5
+#define JZ_LCD_CTRL_BPP_30			0x7
 #define JZ_LCD_CTRL_BPP_MASK			(JZ_LCD_CTRL_RGB555 | 0x7)
 
 #define JZ_LCD_CMD_SOF_IRQ			BIT(31)
-- 
2.28.0

[PATCH v2 7/7] drm/ingenic: Add support for 24-bit modes

[Paul Cercueil]

Starting from the JZ4725B SoC, the primary and overlay planes support
24-bit pixel modes (8 bits per color component, without dummy byte).
Add support for these in the ingenic-drm driver.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 8 ++++++++
 drivers/gpu/drm/ingenic/ingenic-drm.h     | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 9e3122b42820..c2b63533ed18 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -458,6 +458,9 @@ void ingenic_drm_plane_config(struct device *dev,
 		case DRM_FORMAT_RGB565:
 			ctrl |= JZ_LCD_OSDCTRL_BPP_15_16;
 			break;
+		case DRM_FORMAT_RGB888:
+			ctrl |= JZ_LCD_OSDCTRL_BPP_24_COMP;
+			break;
 		case DRM_FORMAT_XRGB8888:
 			ctrl |= JZ_LCD_OSDCTRL_BPP_18_24;
 			break;
@@ -479,6 +482,9 @@ void ingenic_drm_plane_config(struct device *dev,
 		case DRM_FORMAT_RGB565:
 			ctrl |= JZ_LCD_CTRL_BPP_15_16;
 			break;
+		case DRM_FORMAT_RGB888:
+			ctrl |= JZ_LCD_CTRL_BPP_24_COMP;
+			break;
 		case DRM_FORMAT_XRGB8888:
 			ctrl |= JZ_LCD_CTRL_BPP_18_24;
 			break;
@@ -1278,6 +1284,7 @@ static const u32 jz4725b_formats_f0[] = {
 static const u32 jz4770_formats_f1[] = {
 	DRM_FORMAT_XRGB1555,
 	DRM_FORMAT_RGB565,
+	DRM_FORMAT_RGB888,
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_XRGB2101010,
 };
@@ -1286,6 +1293,7 @@ static const u32 jz4770_formats_f0[] = {
 	DRM_FORMAT_C8,
 	DRM_FORMAT_XRGB1555,
 	DRM_FORMAT_RGB565,
+	DRM_FORMAT_RGB888,
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_XRGB2101010,
 };
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h
index f05e18e6b6fa..ee3a892c0383 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.h
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
@@ -124,6 +124,7 @@
 #define JZ_LCD_CTRL_BPP_8			0x3
 #define JZ_LCD_CTRL_BPP_15_16			0x4
 #define JZ_LCD_CTRL_BPP_18_24			0x5
+#define JZ_LCD_CTRL_BPP_24_COMP			0x6
 #define JZ_LCD_CTRL_BPP_30			0x7
 #define JZ_LCD_CTRL_BPP_MASK			(JZ_LCD_CTRL_RGB555 | 0x7)
 
@@ -146,6 +147,7 @@
 #define JZ_LCD_OSDCTRL_CHANGE			BIT(3)
 #define JZ_LCD_OSDCTRL_BPP_15_16		0x4
 #define JZ_LCD_OSDCTRL_BPP_18_24		0x5
+#define JZ_LCD_OSDCTRL_BPP_24_COMP		0x6
 #define JZ_LCD_OSDCTRL_BPP_30			0x7
 #define JZ_LCD_OSDCTRL_BPP_MASK			(JZ_LCD_OSDCTRL_RGB555 | 0x7)
 
-- 
2.28.0

Re: [PATCH v2 0/7] Ingenic-drm improvements + new pixel formats

[Sam Ravnborg]

Hi Paul.

On Sat, Sep 26, 2020 at 07:04:54PM +0200, Paul Cercueil wrote:
> Hi,
> 
> This is a V2 of my small patchset "Small improvements to ingenic-drm"
> that I sent about two weeks ago. In that time, I worked on new
> improvements, so I thought I'd just add them to the patchset, since
> I needed a V2 anyway.
> 
> Sam: the patches you acked have been all slightly modified, I kept your
> acked-by on them, please tell me if that's OK.

All patches except "Add support for paletted 8bpp" are:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>



	sam
> 
> Cheers,
> -Paul
> 
> Paul Cercueil (7):
>   drm/ingenic: Reset pixclock rate when parent clock rate changes
>   drm/ingenic: Add support for reserved memory
>   drm/ingenic: Alloc F0 and F1 DMA descriptors at once
>   drm/ingenic: Support handling different pixel formats in F0/F1 planes
>   drm/ingenic: Add support for paletted 8bpp
>   drm/ingenic: Add support for 30-bit modes
>   drm/ingenic: Add support for 24-bit modes
> 
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 262 +++++++++++++++++++---
>  drivers/gpu/drm/ingenic/ingenic-drm.h     |   3 +
>  2 files changed, 228 insertions(+), 37 deletions(-)
> 
> -- 
> 2.28.0

Re: [PATCH v2 0/7] Ingenic-drm improvements + new pixel formats

[Paul Cercueil]

Hi,

Le sam. 26 sept. 2020 à 20:25, Sam Ravnborg <sam@ravnborg.org> a 
écrit :
> Hi Paul.
> 
> On Sat, Sep 26, 2020 at 07:04:54PM +0200, Paul Cercueil wrote:
>>  Hi,
>> 
>>  This is a V2 of my small patchset "Small improvements to 
>> ingenic-drm"
>>  that I sent about two weeks ago. In that time, I worked on new
>>  improvements, so I thought I'd just add them to the patchset, since
>>  I needed a V2 anyway.
>> 
>>  Sam: the patches you acked have been all slightly modified, I kept 
>> your
>>  acked-by on them, please tell me if that's OK.
> 
> All patches except "Add support for paletted 8bpp" are:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Alright, I'll push the other ones, and keep the 8bpp one for a V3.

-Paul

> 
> 
> 
> 	sam
>> 
>>  Cheers,
>>  -Paul
>> 
>>  Paul Cercueil (7):
>>    drm/ingenic: Reset pixclock rate when parent clock rate changes
>>    drm/ingenic: Add support for reserved memory
>>    drm/ingenic: Alloc F0 and F1 DMA descriptors at once
>>    drm/ingenic: Support handling different pixel formats in F0/F1 
>> planes
>>    drm/ingenic: Add support for paletted 8bpp
>>    drm/ingenic: Add support for 30-bit modes
>>    drm/ingenic: Add support for 24-bit modes
>> 
>>   drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 262 
>> +++++++++++++++++++---
>>   drivers/gpu/drm/ingenic/ingenic-drm.h     |   3 +
>>   2 files changed, 228 insertions(+), 37 deletions(-)
>> 
>>  --
>>  2.28.0