[PATCH v2 00/14] improve the fb_setcmap helper

[PATCH v2 00/14] improve the fb_setcmap helper

[Peter Rosin]

Hi!

While trying to get CLUT support for the atmel_hlcdc driver, and
specifically for the emulated fbdev interface, I received some
push-back that my feeble in-driver attempts should be solved
by the core. This is my attempt to do it right.

I have obviously not tested all of this with more than a compile,
but patches 1 and 3 are enough to make the atmel-hlcdc driver
do what I need (when patched to support CLUT modes). The rest is
just lots of removals and cleanup made possible by the improved
core.

Please test, I would not be surprised if I have fouled up some
bit-manipulation somewhere in this mostly mechanical change...

Changes since v1:

- Rebased to next-20170621
- Split 1/11 into a preparatory patch, a cleanup patch and then
  the meat in 3/14.
- Handle pseudo-palette for FB_VISUAL_TRUECOLOR.
- Removed the empty .gamma_get/.gamma_set fb helpers from the
  armada driver that I had somehow managed to ignore but which
  0day found real quick.
- Be less judgemental on drivers only providing .gamma_get and
  .gamma_set, but no .load_lut. That's actually a valid thing
  to do if you only need pseudo-palette for FB_VISUAL_TRUECOLOR.
- Add a comment about colliding bitfields in the nouveau driver.
- Remove gamma_set/gamma_get declarations from the radeon driver
  (the definitions were removed in v1).

Cheers,
peda

Peter Rosin (14):
  drm/fb-helper: keep the .gamma_store updated in drm_fb_helper_setcmap
  drm/fb-helper: remove drm_fb_helper_save_lut_atomic
  drm/fb-helper: do a generic fb_setcmap helper in terms of crtc
    .gamma_set
  drm: amd: remove dead code and pointless local lut storage
  drm: armada: remove dead empty functions
  drm: ast: remove dead code and pointless local lut storage
  drm: cirrus: remove dead code and pointless local lut storage
  drm: gma500: remove dead code and pointless local lut storage
  drm: i915: remove dead code and pointless local lut storage
  drm: mgag200: remove dead code and pointless local lut storage
  drm: nouveau: remove dead code and pointless local lut storage
  drm: radeon: remove dead code and pointless local lut storage
  drm: stm: remove dead code and pointless local lut storage
  drm: remove unused and redundant callbacks

 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c      |  24 ----
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h    |   1 -
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c      |  27 ++---
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c      |  27 ++---
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c       |  27 ++---
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c       |  27 ++---
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c    |  23 ----
 drivers/gpu/drm/armada/armada_crtc.c        |  10 --
 drivers/gpu/drm/armada/armada_crtc.h        |   2 -
 drivers/gpu/drm/armada/armada_fbdev.c       |   2 -
 drivers/gpu/drm/ast/ast_drv.h               |   1 -
 drivers/gpu/drm/ast/ast_fb.c                |  20 ----
 drivers/gpu/drm/ast/ast_mode.c              |  26 +----
 drivers/gpu/drm/cirrus/cirrus_drv.h         |   8 --
 drivers/gpu/drm/cirrus/cirrus_fbdev.c       |   2 -
 drivers/gpu/drm/cirrus/cirrus_mode.c        |  71 +++---------
 drivers/gpu/drm/drm_fb_helper.c             | 164 +++++++++++++---------------
 drivers/gpu/drm/gma500/framebuffer.c        |  22 ----
 drivers/gpu/drm/gma500/gma_display.c        |  32 ++----
 drivers/gpu/drm/gma500/psb_intel_display.c  |   7 +-
 drivers/gpu/drm/gma500/psb_intel_drv.h      |   1 -
 drivers/gpu/drm/i915/intel_drv.h            |   1 -
 drivers/gpu/drm/i915/intel_fbdev.c          |  31 ------
 drivers/gpu/drm/mgag200/mgag200_drv.h       |   5 -
 drivers/gpu/drm/mgag200/mgag200_fb.c        |   2 -
 drivers/gpu/drm/mgag200/mgag200_mode.c      |  62 +++--------
 drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  26 ++---
 drivers/gpu/drm/nouveau/nouveau_crtc.h      |   3 -
 drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  22 ----
 drivers/gpu/drm/nouveau/nv50_display.c      |  40 +++----
 drivers/gpu/drm/radeon/atombios_crtc.c      |   1 -
 drivers/gpu/drm/radeon/radeon_connectors.c  |   7 +-
 drivers/gpu/drm/radeon/radeon_display.c     |  71 +++++-------
 drivers/gpu/drm/radeon/radeon_fb.c          |   2 -
 drivers/gpu/drm/radeon/radeon_legacy_crtc.c |   1 -
 drivers/gpu/drm/radeon/radeon_mode.h        |   4 -
 drivers/gpu/drm/stm/ltdc.c                  |  12 --
 drivers/gpu/drm/stm/ltdc.h                  |   1 -
 include/drm/drm_fb_helper.h                 |  32 ------
 include/drm/drm_modeset_helper_vtables.h    |  16 ---
 40 files changed, 205 insertions(+), 658 deletions(-)

-- 
2.1.4

[PATCH v2 01/14] drm/fb-helper: keep the .gamma_store updated in drm_fb_helper_setcmap

[Peter Rosin]

I think the gamma_store can end up invalid on error. But the way I read
it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
this pesky legacy fbdev stuff be any better?

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 574af01..25315fb 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1223,12 +1223,16 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 	const struct drm_crtc_helper_funcs *crtc_funcs;
 	u16 *red, *green, *blue, *transp;
 	struct drm_crtc *crtc;
+	u16 *r, *g, *b;
 	int i, j, rc = 0;
 	int start;
 
 	if (oops_in_progress)
 		return -EBUSY;
 
+	if (cmap->start + cmap->len < cmap->start)
+		return -EINVAL;
+
 	drm_modeset_lock_all(dev);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
 		drm_modeset_unlock_all(dev);
@@ -1245,6 +1249,29 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 		transp = cmap->transp;
 		start = cmap->start;
 
+		if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
+			if (!crtc->gamma_size) {
+				rc = -EINVAL;
+				goto out;
+			}
+
+			if (cmap->start + cmap->len > crtc->gamma_size) {
+				rc = -EINVAL;
+				goto out;
+			}
+
+			r = crtc->gamma_store;
+			g = r + crtc->gamma_size;
+			b = g + crtc->gamma_size;
+
+			memcpy(r + cmap->start, cmap->red,
+			       cmap->len * sizeof(u16));
+			memcpy(g + cmap->start, cmap->green,
+			       cmap->len * sizeof(u16));
+			memcpy(b + cmap->start, cmap->blue,
+			       cmap->len * sizeof(u16));
+		}
+
 		for (j = 0; j < cmap->len; j++) {
 			u16 hred, hgreen, hblue, htransp = 0xffff;
 
-- 
2.1.4

[PATCH v2 02/14] drm/fb-helper: remove drm_fb_helper_save_lut_atomic

[Peter Rosin]

drm_fb_helper_save_lut_atomic is redundant since the .gamma_store is
now always kept up to date by drm_fb_helper_setcmap.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/drm_fb_helper.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 25315fb..7ade384 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -229,22 +229,6 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 }
 EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
 
-static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct drm_fb_helper *helper)
-{
-	uint16_t *r_base, *g_base, *b_base;
-	int i;
-
-	if (helper->funcs->gamma_get == NULL)
-		return;
-
-	r_base = crtc->gamma_store;
-	g_base = r_base + crtc->gamma_size;
-	b_base = g_base + crtc->gamma_size;
-
-	for (i = 0; i < crtc->gamma_size; i++)
-		helper->funcs->gamma_get(crtc, &r_base[i], &g_base[i], &b_base[i], i);
-}
-
 static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
 {
 	uint16_t *r_base, *g_base, *b_base;
@@ -285,7 +269,6 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
 			if (drm_drv_uses_atomic_modeset(mode_set->crtc->dev))
 				continue;
 
-			drm_fb_helper_save_lut_atomic(mode_set->crtc, helper);
 			funcs->mode_set_base_atomic(mode_set->crtc,
 						    mode_set->fb,
 						    mode_set->x,
-- 
2.1.4

[PATCH v2 03/14] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Peter Rosin]

This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
totally obsolete.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/drm_fb_helper.c | 154 ++++++++++++++++------------------------
 1 file changed, 63 insertions(+), 91 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 7ade384..58eb045 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1150,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper,
 }
 EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
 
-static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
-		     u16 blue, u16 regno, struct fb_info *info)
-{
-	struct drm_fb_helper *fb_helper = info->par;
-	struct drm_framebuffer *fb = fb_helper->fb;
-
-	if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
-		u32 *palette;
-		u32 value;
-		/* place color in psuedopalette */
-		if (regno > 16)
-			return -EINVAL;
-		palette = (u32 *)info->pseudo_palette;
-		red >>= (16 - info->var.red.length);
-		green >>= (16 - info->var.green.length);
-		blue >>= (16 - info->var.blue.length);
-		value = (red << info->var.red.offset) |
-			(green << info->var.green.offset) |
-			(blue << info->var.blue.offset);
-		if (info->var.transp.length > 0) {
-			u32 mask = (1 << info->var.transp.length) - 1;
-
-			mask <<= info->var.transp.offset;
-			value |= mask;
-		}
-		palette[regno] = value;
-		return 0;
-	}
-
-	/*
-	 * The driver really shouldn't advertise pseudo/directcolor
-	 * visuals if it can't deal with the palette.
-	 */
-	if (WARN_ON(!fb_helper->funcs->gamma_set ||
-		    !fb_helper->funcs->gamma_get))
-		return -EINVAL;
-
-	WARN_ON(fb->format->cpp[0] != 1);
-
-	fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
-
-	return 0;
-}
-
 /**
  * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
  * @cmap: cmap to set
@@ -1203,12 +1159,10 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 {
 	struct drm_fb_helper *fb_helper = info->par;
 	struct drm_device *dev = fb_helper->dev;
-	const struct drm_crtc_helper_funcs *crtc_funcs;
-	u16 *red, *green, *blue, *transp;
+	struct drm_modeset_acquire_ctx ctx;
 	struct drm_crtc *crtc;
 	u16 *r, *g, *b;
-	int i, j, rc = 0;
-	int start;
+	int i, ret;
 
 	if (oops_in_progress)
 		return -EBUSY;
@@ -1216,65 +1170,83 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 	if (cmap->start + cmap->len < cmap->start)
 		return -EINVAL;
 
-	drm_modeset_lock_all(dev);
+	drm_modeset_acquire_init(&ctx, 0);
+retry:
+	ret = drm_modeset_lock_all_ctx(dev, &ctx);
+	if (ret)
+		goto out;
 	if (!drm_fb_helper_is_bound(fb_helper)) {
-		drm_modeset_unlock_all(dev);
-		return -EBUSY;
+		ret = -EBUSY;
+		goto out;
 	}
 
 	for (i = 0; i < fb_helper->crtc_count; i++) {
-		crtc = fb_helper->crtc_info[i].mode_set.crtc;
-		crtc_funcs = crtc->helper_private;
-
-		red = cmap->red;
-		green = cmap->green;
-		blue = cmap->blue;
-		transp = cmap->transp;
-		start = cmap->start;
+		if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
+			u32 *palette;
+			int j;
 
-		if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
-			if (!crtc->gamma_size) {
-				rc = -EINVAL;
+			if (cmap->start + cmap->len > 16) {
+				ret = -EINVAL;
 				goto out;
 			}
 
-			if (cmap->start + cmap->len > crtc->gamma_size) {
-				rc = -EINVAL;
-				goto out;
+			palette = (u32 *)info->pseudo_palette;
+			for (j = 0; j < cmap->len; ++j) {
+				u16 red = cmap->red[j];
+				u16 green = cmap->green[j];
+				u16 blue = cmap->blue[j];
+				u32 value;
+
+				red >>= 16 - info->var.red.length;
+				green >>= 16 - info->var.green.length;
+				blue >>= 16 - info->var.blue.length;
+				value = (red << info->var.red.offset) |
+					(green << info->var.green.offset) |
+					(blue << info->var.blue.offset);
+				if (info->var.transp.length > 0) {
+					u32 mask = (1 << info->var.transp.length) - 1;
+
+					mask <<= info->var.transp.offset;
+					value |= mask;
+				}
+				palette[cmap->start + j] = value;
 			}
+			continue;
+		}
 
-			r = crtc->gamma_store;
-			g = r + crtc->gamma_size;
-			b = g + crtc->gamma_size;
-
-			memcpy(r + cmap->start, cmap->red,
-			       cmap->len * sizeof(u16));
-			memcpy(g + cmap->start, cmap->green,
-			       cmap->len * sizeof(u16));
-			memcpy(b + cmap->start, cmap->blue,
-			       cmap->len * sizeof(u16));
+		crtc = fb_helper->crtc_info[i].mode_set.crtc;
+		if (!crtc->funcs->gamma_set || !crtc->gamma_size) {
+			ret = -EINVAL;
+			goto out;
 		}
 
-		for (j = 0; j < cmap->len; j++) {
-			u16 hred, hgreen, hblue, htransp = 0xffff;
+		if (cmap->start + cmap->len > crtc->gamma_size) {
+			ret = -EINVAL;
+			goto out;
+		}
 
-			hred = *red++;
-			hgreen = *green++;
-			hblue = *blue++;
+		r = crtc->gamma_store;
+		g = r + crtc->gamma_size;
+		b = g + crtc->gamma_size;
 
-			if (transp)
-				htransp = *transp++;
+		memcpy(r + cmap->start, cmap->red, cmap->len * sizeof(u16));
+		memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(u16));
+		memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(u16));
 
-			rc = setcolreg(crtc, hred, hgreen, hblue, start++, info);
-			if (rc)
-				goto out;
-		}
-		if (crtc_funcs->load_lut)
-			crtc_funcs->load_lut(crtc);
+		ret = crtc->funcs->gamma_set(crtc, r, g, b,
+					     crtc->gamma_size, &ctx);
+		if (ret)
+			break;
 	}
- out:
-	drm_modeset_unlock_all(dev);
-	return rc;
+out:
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
+	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_setcmap);
 
-- 
2.1.4

[PATCH v2 04/14] drm: amd: remove dead code and pointless local lut storage

[Peter Rosin]

The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c   | 24 ------------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h |  1 -
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c   | 27 +++++++--------------------
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c   | 27 +++++++--------------------
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c    | 27 +++++++--------------------
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c    | 27 +++++++--------------------
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 23 -----------------------
 7 files changed, 28 insertions(+), 128 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index c0d8c6f..7dc3780 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -312,31 +312,7 @@ static int amdgpu_fbdev_destroy(struct drm_device *dev, struct amdgpu_fbdev *rfb
 	return 0;
 }
 
-/** Sets the color ramps on behalf of fbcon */
-static void amdgpu_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-				      u16 blue, int regno)
-{
-	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-
-	amdgpu_crtc->lut_r[regno] = red >> 6;
-	amdgpu_crtc->lut_g[regno] = green >> 6;
-	amdgpu_crtc->lut_b[regno] = blue >> 6;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-static void amdgpu_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-				      u16 *blue, int regno)
-{
-	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-
-	*red = amdgpu_crtc->lut_r[regno] << 6;
-	*green = amdgpu_crtc->lut_g[regno] << 6;
-	*blue = amdgpu_crtc->lut_b[regno] << 6;
-}
-
 static const struct drm_fb_helper_funcs amdgpu_fb_helper_funcs = {
-	.gamma_set = amdgpu_crtc_fb_gamma_set,
-	.gamma_get = amdgpu_crtc_fb_gamma_get,
 	.fb_probe = amdgpufb_create,
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 43a9d3a..39f7eda 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -369,7 +369,6 @@ struct amdgpu_atom_ss {
 struct amdgpu_crtc {
 	struct drm_crtc base;
 	int crtc_id;
-	u16 lut_r[256], lut_g[256], lut_b[256];
 	bool enabled;
 	bool can_tile;
 	uint32_t crtc_offset;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 9f78c03..c958023 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2267,6 +2267,7 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc *crtc)
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct amdgpu_device *adev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 	u32 tmp;
 
@@ -2304,11 +2305,14 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
 
 	WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
-		       (amdgpu_crtc->lut_r[i] << 20) |
-		       (amdgpu_crtc->lut_g[i] << 10) |
-		       (amdgpu_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 
 	tmp = RREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset);
@@ -2624,15 +2628,6 @@ static int dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				    u16 *blue, uint32_t size,
 				    struct drm_modeset_acquire_ctx *ctx)
 {
-	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		amdgpu_crtc->lut_r[i] = red[i] >> 6;
-		amdgpu_crtc->lut_g[i] = green[i] >> 6;
-		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
-	}
 	dce_v10_0_crtc_load_lut(crtc);
 
 	return 0;
@@ -2844,14 +2839,12 @@ static const struct drm_crtc_helper_funcs dce_v10_0_crtc_helper_funcs = {
 	.mode_set_base_atomic = dce_v10_0_crtc_set_base_atomic,
 	.prepare = dce_v10_0_crtc_prepare,
 	.commit = dce_v10_0_crtc_commit,
-	.load_lut = dce_v10_0_crtc_load_lut,
 	.disable = dce_v10_0_crtc_disable,
 };
 
 static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int index)
 {
 	struct amdgpu_crtc *amdgpu_crtc;
-	int i;
 
 	amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
 			      (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
@@ -2869,12 +2862,6 @@ static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int index)
 	adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
 	adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
 
-	for (i = 0; i < 256; i++) {
-		amdgpu_crtc->lut_r[i] = i << 2;
-		amdgpu_crtc->lut_g[i] = i << 2;
-		amdgpu_crtc->lut_b[i] = i << 2;
-	}
-
 	switch (amdgpu_crtc->crtc_id) {
 	case 0:
 	default:
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 4bcf01d..7e14f53 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -2251,6 +2251,7 @@ static void dce_v11_0_crtc_load_lut(struct drm_crtc *crtc)
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct amdgpu_device *adev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 	u32 tmp;
 
@@ -2282,11 +2283,14 @@ static void dce_v11_0_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
 
 	WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
-		       (amdgpu_crtc->lut_r[i] << 20) |
-		       (amdgpu_crtc->lut_g[i] << 10) |
-		       (amdgpu_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 
 	tmp = RREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset);
@@ -2644,15 +2648,6 @@ static int dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				    u16 *blue, uint32_t size,
 				    struct drm_modeset_acquire_ctx *ctx)
 {
-	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		amdgpu_crtc->lut_r[i] = red[i] >> 6;
-		amdgpu_crtc->lut_g[i] = green[i] >> 6;
-		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
-	}
 	dce_v11_0_crtc_load_lut(crtc);
 
 	return 0;
@@ -2892,14 +2887,12 @@ static const struct drm_crtc_helper_funcs dce_v11_0_crtc_helper_funcs = {
 	.mode_set_base_atomic = dce_v11_0_crtc_set_base_atomic,
 	.prepare = dce_v11_0_crtc_prepare,
 	.commit = dce_v11_0_crtc_commit,
-	.load_lut = dce_v11_0_crtc_load_lut,
 	.disable = dce_v11_0_crtc_disable,
 };
 
 static int dce_v11_0_crtc_init(struct amdgpu_device *adev, int index)
 {
 	struct amdgpu_crtc *amdgpu_crtc;
-	int i;
 
 	amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
 			      (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
@@ -2917,12 +2910,6 @@ static int dce_v11_0_crtc_init(struct amdgpu_device *adev, int index)
 	adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
 	adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
 
-	for (i = 0; i < 256; i++) {
-		amdgpu_crtc->lut_r[i] = i << 2;
-		amdgpu_crtc->lut_g[i] = i << 2;
-		amdgpu_crtc->lut_b[i] = i << 2;
-	}
-
 	switch (amdgpu_crtc->crtc_id) {
 	case 0:
 	default:
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index fd134a4..d773b50 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -2182,6 +2182,7 @@ static void dce_v6_0_crtc_load_lut(struct drm_crtc *crtc)
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct amdgpu_device *adev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 
 	DRM_DEBUG_KMS("%d\n", amdgpu_crtc->crtc_id);
@@ -2211,11 +2212,14 @@ static void dce_v6_0_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
 
 	WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
-		       (amdgpu_crtc->lut_r[i] << 20) |
-		       (amdgpu_crtc->lut_g[i] << 10) |
-		       (amdgpu_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 
 	WREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset,
@@ -2496,15 +2500,6 @@ static int dce_v6_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				   u16 *blue, uint32_t size,
 				   struct drm_modeset_acquire_ctx *ctx)
 {
-	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		amdgpu_crtc->lut_r[i] = red[i] >> 6;
-		amdgpu_crtc->lut_g[i] = green[i] >> 6;
-		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
-	}
 	dce_v6_0_crtc_load_lut(crtc);
 
 	return 0;
@@ -2712,14 +2707,12 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
 	.mode_set_base_atomic = dce_v6_0_crtc_set_base_atomic,
 	.prepare = dce_v6_0_crtc_prepare,
 	.commit = dce_v6_0_crtc_commit,
-	.load_lut = dce_v6_0_crtc_load_lut,
 	.disable = dce_v6_0_crtc_disable,
 };
 
 static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
 {
 	struct amdgpu_crtc *amdgpu_crtc;
-	int i;
 
 	amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
 			      (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
@@ -2737,12 +2730,6 @@ static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
 	adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
 	adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
 
-	for (i = 0; i < 256; i++) {
-		amdgpu_crtc->lut_r[i] = i << 2;
-		amdgpu_crtc->lut_g[i] = i << 2;
-		amdgpu_crtc->lut_b[i] = i << 2;
-	}
-
 	amdgpu_crtc->crtc_offset = crtc_offsets[amdgpu_crtc->crtc_id];
 
 	amdgpu_crtc->pll_id = ATOM_PPLL_INVALID;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index a9e8695..4eb63f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -2124,6 +2124,7 @@ static void dce_v8_0_crtc_load_lut(struct drm_crtc *crtc)
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct amdgpu_device *adev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 
 	DRM_DEBUG_KMS("%d\n", amdgpu_crtc->crtc_id);
@@ -2153,11 +2154,14 @@ static void dce_v8_0_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
 
 	WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
-		       (amdgpu_crtc->lut_r[i] << 20) |
-		       (amdgpu_crtc->lut_g[i] << 10) |
-		       (amdgpu_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 
 	WREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset,
@@ -2475,15 +2479,6 @@ static int dce_v8_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				   u16 *blue, uint32_t size,
 				   struct drm_modeset_acquire_ctx *ctx)
 {
-	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		amdgpu_crtc->lut_r[i] = red[i] >> 6;
-		amdgpu_crtc->lut_g[i] = green[i] >> 6;
-		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
-	}
 	dce_v8_0_crtc_load_lut(crtc);
 
 	return 0;
@@ -2702,14 +2697,12 @@ static const struct drm_crtc_helper_funcs dce_v8_0_crtc_helper_funcs = {
 	.mode_set_base_atomic = dce_v8_0_crtc_set_base_atomic,
 	.prepare = dce_v8_0_crtc_prepare,
 	.commit = dce_v8_0_crtc_commit,
-	.load_lut = dce_v8_0_crtc_load_lut,
 	.disable = dce_v8_0_crtc_disable,
 };
 
 static int dce_v8_0_crtc_init(struct amdgpu_device *adev, int index)
 {
 	struct amdgpu_crtc *amdgpu_crtc;
-	int i;
 
 	amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
 			      (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
@@ -2727,12 +2720,6 @@ static int dce_v8_0_crtc_init(struct amdgpu_device *adev, int index)
 	adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
 	adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
 
-	for (i = 0; i < 256; i++) {
-		amdgpu_crtc->lut_r[i] = i << 2;
-		amdgpu_crtc->lut_g[i] = i << 2;
-		amdgpu_crtc->lut_b[i] = i << 2;
-	}
-
 	amdgpu_crtc->crtc_offset = crtc_offsets[amdgpu_crtc->crtc_id];
 
 	amdgpu_crtc->pll_id = ATOM_PPLL_INVALID;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index 90bb083..ecf34bc 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -168,16 +168,6 @@ static int dce_virtual_crtc_gamma_set(struct drm_crtc *crtc, u16 *red,
 				      u16 *green, u16 *blue, uint32_t size,
 				      struct drm_modeset_acquire_ctx *ctx)
 {
-	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		amdgpu_crtc->lut_r[i] = red[i] >> 6;
-		amdgpu_crtc->lut_g[i] = green[i] >> 6;
-		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
-	}
-
 	return 0;
 }
 
@@ -289,11 +279,6 @@ static int dce_virtual_crtc_set_base(struct drm_crtc *crtc, int x, int y,
 	return 0;
 }
 
-static void dce_virtual_crtc_load_lut(struct drm_crtc *crtc)
-{
-	return;
-}
-
 static int dce_virtual_crtc_set_base_atomic(struct drm_crtc *crtc,
 					 struct drm_framebuffer *fb,
 					 int x, int y, enum mode_set_atomic state)
@@ -309,14 +294,12 @@ static const struct drm_crtc_helper_funcs dce_virtual_crtc_helper_funcs = {
 	.mode_set_base_atomic = dce_virtual_crtc_set_base_atomic,
 	.prepare = dce_virtual_crtc_prepare,
 	.commit = dce_virtual_crtc_commit,
-	.load_lut = dce_virtual_crtc_load_lut,
 	.disable = dce_virtual_crtc_disable,
 };
 
 static int dce_virtual_crtc_init(struct amdgpu_device *adev, int index)
 {
 	struct amdgpu_crtc *amdgpu_crtc;
-	int i;
 
 	amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
 			      (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
@@ -329,12 +312,6 @@ static int dce_virtual_crtc_init(struct amdgpu_device *adev, int index)
 	amdgpu_crtc->crtc_id = index;
 	adev->mode_info.crtcs[index] = amdgpu_crtc;
 
-	for (i = 0; i < 256; i++) {
-		amdgpu_crtc->lut_r[i] = i << 2;
-		amdgpu_crtc->lut_g[i] = i << 2;
-		amdgpu_crtc->lut_b[i] = i << 2;
-	}
-
 	amdgpu_crtc->pll_id = ATOM_PPLL_INVALID;
 	amdgpu_crtc->encoder = NULL;
 	amdgpu_crtc->connector = NULL;
-- 
2.1.4

[PATCH v2 05/14] drm: armada: remove dead empty functions

[Peter Rosin]

The redundant fb helpers .gamma_set and .gamma_get are no longer used.
Remove the dead code.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/armada/armada_crtc.c  | 10 ----------
 drivers/gpu/drm/armada/armada_crtc.h  |  2 --
 drivers/gpu/drm/armada/armada_fbdev.c |  2 --
 3 files changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
index 4fe19fd..96bccf8 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -334,16 +334,6 @@ static void armada_drm_vblank_off(struct armada_crtc *dcrtc)
 	armada_drm_plane_work_run(dcrtc, dcrtc->crtc.primary);
 }
 
-void armada_drm_crtc_gamma_set(struct drm_crtc *crtc, u16 r, u16 g, u16 b,
-	int idx)
-{
-}
-
-void armada_drm_crtc_gamma_get(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
-	int idx)
-{
-}
-
 /* The mode_config.mutex will be held for this call */
 static void armada_drm_crtc_dpms(struct drm_crtc *crtc, int dpms)
 {
diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h
index 7e8906d..bab11f4 100644
--- a/drivers/gpu/drm/armada/armada_crtc.h
+++ b/drivers/gpu/drm/armada/armada_crtc.h
@@ -102,8 +102,6 @@ struct armada_crtc {
 };
 #define drm_to_armada_crtc(c) container_of(c, struct armada_crtc, crtc)
 
-void armada_drm_crtc_gamma_set(struct drm_crtc *, u16, u16, u16, int);
-void armada_drm_crtc_gamma_get(struct drm_crtc *, u16 *, u16 *, u16 *, int);
 void armada_drm_crtc_update_regs(struct armada_crtc *, struct armada_regs *);
 
 void armada_drm_crtc_plane_disable(struct armada_crtc *dcrtc,
diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c
index 602dfea..5fa076d 100644
--- a/drivers/gpu/drm/armada/armada_fbdev.c
+++ b/drivers/gpu/drm/armada/armada_fbdev.c
@@ -118,8 +118,6 @@ static int armada_fb_probe(struct drm_fb_helper *fbh,
 }
 
 static const struct drm_fb_helper_funcs armada_fb_helper_funcs = {
-	.gamma_set	= armada_drm_crtc_gamma_set,
-	.gamma_get	= armada_drm_crtc_gamma_get,
 	.fb_probe	= armada_fb_probe,
 };
 
-- 
2.1.4

[PATCH v2 06/14] drm: ast: remove dead code and pointless local lut storage

[Peter Rosin]

The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/ast/ast_drv.h  |  1 -
 drivers/gpu/drm/ast/ast_fb.c   | 20 --------------------
 drivers/gpu/drm/ast/ast_mode.c | 26 ++++++--------------------
 3 files changed, 6 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 8880f0b..569a148 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -245,7 +245,6 @@ struct ast_connector {
 
 struct ast_crtc {
 	struct drm_crtc base;
-	u8 lut_r[256], lut_g[256], lut_b[256];
 	struct drm_gem_object *cursor_bo;
 	uint64_t cursor_addr;
 	int cursor_width, cursor_height;
diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index 4ad4acd..dbabcac 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -255,27 +255,7 @@ static int astfb_create(struct drm_fb_helper *helper,
 	return ret;
 }
 
-static void ast_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-			       u16 blue, int regno)
-{
-	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
-	ast_crtc->lut_r[regno] = red >> 8;
-	ast_crtc->lut_g[regno] = green >> 8;
-	ast_crtc->lut_b[regno] = blue >> 8;
-}
-
-static void ast_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-			       u16 *blue, int regno)
-{
-	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
-	*red = ast_crtc->lut_r[regno] << 8;
-	*green = ast_crtc->lut_g[regno] << 8;
-	*blue = ast_crtc->lut_b[regno] << 8;
-}
-
 static const struct drm_fb_helper_funcs ast_fb_helper_funcs = {
-	.gamma_set = ast_fb_gamma_set,
-	.gamma_get = ast_fb_gamma_get,
 	.fb_probe = astfb_create,
 };
 
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index aaef0a6..724c16b 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -63,15 +63,18 @@ static inline void ast_load_palette_index(struct ast_private *ast,
 static void ast_crtc_load_lut(struct drm_crtc *crtc)
 {
 	struct ast_private *ast = crtc->dev->dev_private;
-	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
+	u16 *r, *g, *b;
 	int i;
 
 	if (!crtc->enabled)
 		return;
 
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
+
 	for (i = 0; i < 256; i++)
-		ast_load_palette_index(ast, i, ast_crtc->lut_r[i],
-				       ast_crtc->lut_g[i], ast_crtc->lut_b[i]);
+		ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ >> 8);
 }
 
 static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mode *mode,
@@ -633,7 +636,6 @@ static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
 	.mode_set = ast_crtc_mode_set,
 	.mode_set_base = ast_crtc_mode_set_base,
 	.disable = ast_crtc_disable,
-	.load_lut = ast_crtc_load_lut,
 	.prepare = ast_crtc_prepare,
 	.commit = ast_crtc_commit,
 
@@ -648,15 +650,6 @@ static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 			      u16 *blue, uint32_t size,
 			      struct drm_modeset_acquire_ctx *ctx)
 {
-	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		ast_crtc->lut_r[i] = red[i] >> 8;
-		ast_crtc->lut_g[i] = green[i] >> 8;
-		ast_crtc->lut_b[i] = blue[i] >> 8;
-	}
 	ast_crtc_load_lut(crtc);
 
 	return 0;
@@ -681,7 +674,6 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
 static int ast_crtc_init(struct drm_device *dev)
 {
 	struct ast_crtc *crtc;
-	int i;
 
 	crtc = kzalloc(sizeof(struct ast_crtc), GFP_KERNEL);
 	if (!crtc)
@@ -690,12 +682,6 @@ static int ast_crtc_init(struct drm_device *dev)
 	drm_crtc_init(dev, &crtc->base, &ast_crtc_funcs);
 	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
 	drm_crtc_helper_add(&crtc->base, &ast_crtc_helper_funcs);
-
-	for (i = 0; i < 256; i++) {
-		crtc->lut_r[i] = i;
-		crtc->lut_g[i] = i;
-		crtc->lut_b[i] = i;
-	}
 	return 0;
 }
 
-- 
2.1.4

[PATCH v2 07/14] drm: cirrus: remove dead code and pointless local lut storage

[Peter Rosin]

The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/cirrus/cirrus_drv.h   |  8 ----
 drivers/gpu/drm/cirrus/cirrus_fbdev.c |  2 -
 drivers/gpu/drm/cirrus/cirrus_mode.c  | 71 ++++++++---------------------------
 3 files changed, 16 insertions(+), 65 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h b/drivers/gpu/drm/cirrus/cirrus_drv.h
index 8690352..be2d7e48 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.h
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
@@ -96,7 +96,6 @@
 
 struct cirrus_crtc {
 	struct drm_crtc			base;
-	u8				lut_r[256], lut_g[256], lut_b[256];
 	int				last_dpms;
 	bool				enabled;
 };
@@ -180,13 +179,6 @@ cirrus_bo(struct ttm_buffer_object *bo)
 #define to_cirrus_obj(x) container_of(x, struct cirrus_gem_object, base)
 #define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
 
-				/* cirrus_mode.c */
-void cirrus_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-			     u16 blue, int regno);
-void cirrus_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-			     u16 *blue, int regno);
-
-
 				/* cirrus_main.c */
 int cirrus_device_init(struct cirrus_device *cdev,
 		      struct drm_device *ddev,
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 7fa58ee..1fedab0 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -265,8 +265,6 @@ static int cirrus_fbdev_destroy(struct drm_device *dev,
 }
 
 static const struct drm_fb_helper_funcs cirrus_fb_helper_funcs = {
-	.gamma_set = cirrus_crtc_fb_gamma_set,
-	.gamma_get = cirrus_crtc_fb_gamma_get,
 	.fb_probe = cirrusfb_create,
 };
 
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c
index 53f6f0f..a4c4a46 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -31,25 +31,6 @@
  * This file contains setup code for the CRTC.
  */
 
-static void cirrus_crtc_load_lut(struct drm_crtc *crtc)
-{
-	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
-	struct drm_device *dev = crtc->dev;
-	struct cirrus_device *cdev = dev->dev_private;
-	int i;
-
-	if (!crtc->enabled)
-		return;
-
-	for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
-		/* VGA registers */
-		WREG8(PALETTE_INDEX, i);
-		WREG8(PALETTE_DATA, cirrus_crtc->lut_r[i]);
-		WREG8(PALETTE_DATA, cirrus_crtc->lut_g[i]);
-		WREG8(PALETTE_DATA, cirrus_crtc->lut_b[i]);
-	}
-}
-
 /*
  * The DRM core requires DPMS functions, but they make little sense in our
  * case and so are just stubs
@@ -330,15 +311,25 @@ static int cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				 u16 *blue, uint32_t size,
 				 struct drm_modeset_acquire_ctx *ctx)
 {
-	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+	struct cirrus_device *cdev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 
-	for (i = 0; i < size; i++) {
-		cirrus_crtc->lut_r[i] = red[i];
-		cirrus_crtc->lut_g[i] = green[i];
-		cirrus_crtc->lut_b[i] = blue[i];
+	if (!crtc->enabled)
+		return 0;
+
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
+
+	for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
+		/* VGA registers */
+		WREG8(PALETTE_INDEX, i);
+		WREG8(PALETTE_DATA, *r++ >> 8);
+		WREG8(PALETTE_DATA, *g++ >> 8);
+		WREG8(PALETTE_DATA, *b++ >> 8);
 	}
-	cirrus_crtc_load_lut(crtc);
 
 	return 0;
 }
@@ -365,7 +356,6 @@ static const struct drm_crtc_helper_funcs cirrus_helper_funcs = {
 	.mode_set_base = cirrus_crtc_mode_set_base,
 	.prepare = cirrus_crtc_prepare,
 	.commit = cirrus_crtc_commit,
-	.load_lut = cirrus_crtc_load_lut,
 };
 
 /* CRTC setup */
@@ -373,7 +363,6 @@ static void cirrus_crtc_init(struct drm_device *dev)
 {
 	struct cirrus_device *cdev = dev->dev_private;
 	struct cirrus_crtc *cirrus_crtc;
-	int i;
 
 	cirrus_crtc = kzalloc(sizeof(struct cirrus_crtc) +
 			      (CIRRUSFB_CONN_LIMIT * sizeof(struct drm_connector *)),
@@ -387,37 +376,9 @@ static void cirrus_crtc_init(struct drm_device *dev)
 	drm_mode_crtc_set_gamma_size(&cirrus_crtc->base, CIRRUS_LUT_SIZE);
 	cdev->mode_info.crtc = cirrus_crtc;
 
-	for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
-		cirrus_crtc->lut_r[i] = i;
-		cirrus_crtc->lut_g[i] = i;
-		cirrus_crtc->lut_b[i] = i;
-	}
-
 	drm_crtc_helper_add(&cirrus_crtc->base, &cirrus_helper_funcs);
 }
 
-/** Sets the color ramps on behalf of fbcon */
-void cirrus_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-			      u16 blue, int regno)
-{
-	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
-
-	cirrus_crtc->lut_r[regno] = red;
-	cirrus_crtc->lut_g[regno] = green;
-	cirrus_crtc->lut_b[regno] = blue;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-void cirrus_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, int regno)
-{
-	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
-
-	*red = cirrus_crtc->lut_r[regno];
-	*green = cirrus_crtc->lut_g[regno];
-	*blue = cirrus_crtc->lut_b[regno];
-}
-
 static void cirrus_encoder_mode_set(struct drm_encoder *encoder,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode)
-- 
2.1.4

[PATCH v2 08/14] drm: gma500: remove dead code and pointless local lut storage

[Peter Rosin]

The redundant fb helpers .gamma_set and .gamma_get are no longer
used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/gma500/framebuffer.c       | 22 --------------------
 drivers/gpu/drm/gma500/gma_display.c       | 32 ++++++++++--------------------
 drivers/gpu/drm/gma500/psb_intel_display.c |  7 +------
 drivers/gpu/drm/gma500/psb_intel_drv.h     |  1 -
 4 files changed, 12 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 7da70b6..2570c7f 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -479,26 +479,6 @@ static struct drm_framebuffer *psb_user_framebuffer_create
 	return psb_framebuffer_create(dev, cmd, r);
 }
 
-static void psbfb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-							u16 blue, int regno)
-{
-	struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
-
-	gma_crtc->lut_r[regno] = red >> 8;
-	gma_crtc->lut_g[regno] = green >> 8;
-	gma_crtc->lut_b[regno] = blue >> 8;
-}
-
-static void psbfb_gamma_get(struct drm_crtc *crtc, u16 *red,
-					u16 *green, u16 *blue, int regno)
-{
-	struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
-
-	*red = gma_crtc->lut_r[regno] << 8;
-	*green = gma_crtc->lut_g[regno] << 8;
-	*blue = gma_crtc->lut_b[regno] << 8;
-}
-
 static int psbfb_probe(struct drm_fb_helper *helper,
 				struct drm_fb_helper_surface_size *sizes)
 {
@@ -525,8 +505,6 @@ static int psbfb_probe(struct drm_fb_helper *helper,
 }
 
 static const struct drm_fb_helper_funcs psb_fb_helper_funcs = {
-	.gamma_set = psbfb_gamma_set,
-	.gamma_get = psbfb_gamma_get,
 	.fb_probe = psbfb_probe,
 };
 
diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
index e7fd356..f3c48a2 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -144,33 +144,32 @@ void gma_crtc_load_lut(struct drm_crtc *crtc)
 	struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
 	const struct psb_offset *map = &dev_priv->regmap[gma_crtc->pipe];
 	int palreg = map->palette;
+	u16 *r, *g, *b;
 	int i;
 
 	/* The clocks have to be on to load the palette. */
 	if (!crtc->enabled)
 		return;
 
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
+
 	if (gma_power_begin(dev, false)) {
 		for (i = 0; i < 256; i++) {
 			REG_WRITE(palreg + 4 * i,
-				  ((gma_crtc->lut_r[i] +
-				  gma_crtc->lut_adj[i]) << 16) |
-				  ((gma_crtc->lut_g[i] +
-				  gma_crtc->lut_adj[i]) << 8) |
-				  (gma_crtc->lut_b[i] +
-				  gma_crtc->lut_adj[i]));
+				  (((*r++ >> 8) + gma_crtc->lut_adj[i]) << 16) |
+				  (((*g++ >> 8) + gma_crtc->lut_adj[i]) << 8) |
+				  ((*b++ >> 8) + gma_crtc->lut_adj[i]));
 		}
 		gma_power_end(dev);
 	} else {
 		for (i = 0; i < 256; i++) {
 			/* FIXME: Why pipe[0] and not pipe[..._crtc->pipe]? */
 			dev_priv->regs.pipe[0].palette[i] =
-				  ((gma_crtc->lut_r[i] +
-				  gma_crtc->lut_adj[i]) << 16) |
-				  ((gma_crtc->lut_g[i] +
-				  gma_crtc->lut_adj[i]) << 8) |
-				  (gma_crtc->lut_b[i] +
-				  gma_crtc->lut_adj[i]);
+				(((*r++ >> 8) + gma_crtc->lut_adj[i]) << 16) |
+				(((*g++ >> 8) + gma_crtc->lut_adj[i]) << 8) |
+				((*b++ >> 8) + gma_crtc->lut_adj[i]);
 		}
 
 	}
@@ -180,15 +179,6 @@ int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue,
 		       u32 size,
 		       struct drm_modeset_acquire_ctx *ctx)
 {
-	struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
-	int i;
-
-	for (i = 0; i < size; i++) {
-		gma_crtc->lut_r[i] = red[i] >> 8;
-		gma_crtc->lut_g[i] = green[i] >> 8;
-		gma_crtc->lut_b[i] = blue[i] >> 8;
-	}
-
 	gma_crtc_load_lut(crtc);
 
 	return 0;
diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c
index 7b6c849..8762efa 100644
--- a/drivers/gpu/drm/gma500/psb_intel_display.c
+++ b/drivers/gpu/drm/gma500/psb_intel_display.c
@@ -518,13 +518,8 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe,
 	gma_crtc->pipe = pipe;
 	gma_crtc->plane = pipe;
 
-	for (i = 0; i < 256; i++) {
-		gma_crtc->lut_r[i] = i;
-		gma_crtc->lut_g[i] = i;
-		gma_crtc->lut_b[i] = i;
-
+	for (i = 0; i < 256; i++)
 		gma_crtc->lut_adj[i] = 0;
-	}
 
 	gma_crtc->mode_dev = mode_dev;
 	gma_crtc->cursor_addr = 0;
diff --git a/drivers/gpu/drm/gma500/psb_intel_drv.h b/drivers/gpu/drm/gma500/psb_intel_drv.h
index 6a10215..e8e4ea1 100644
--- a/drivers/gpu/drm/gma500/psb_intel_drv.h
+++ b/drivers/gpu/drm/gma500/psb_intel_drv.h
@@ -172,7 +172,6 @@ struct gma_crtc {
 	int plane;
 	uint32_t cursor_addr;
 	struct gtt_range *cursor_gt;
-	u8 lut_r[256], lut_g[256], lut_b[256];
 	u8 lut_adj[256];
 	struct psb_intel_framebuffer *fbdev_fb;
 	/* a mode_set for fbdev users on this crtc */
-- 
2.1.4

[PATCH v2 09/14] drm: i915: remove dead code and pointless local lut storage

[Peter Rosin]

The driver stores lut values from the fbdev interface, and is able
to give them back, but does not appear to do anything with these
lut values. The generic fb helpers have replaced this function,
and may even have made the driver work for the C8 mode from the
fbdev interface. But that is untested.

Since the fb helpers .gamma_set and .gamma_get are obsolete,
remove the dead code.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/i915/intel_drv.h   |  1 -
 drivers/gpu/drm/i915/intel_fbdev.c | 31 -------------------------------
 2 files changed, 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d93efb4..bc7bfa0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -786,7 +786,6 @@ struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
 	enum plane plane;
-	u8 lut_r[256], lut_g[256], lut_b[256];
 	/*
 	 * Whether the crtc and the connected output pipeline is active. Implies
 	 * that crtc->enabled is set, i.e. the current mode configuration has
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 03347c6..5bac953 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -281,27 +281,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	return ret;
 }
 
-/** Sets the color ramps on behalf of RandR */
-static void intel_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-				    u16 blue, int regno)
-{
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	intel_crtc->lut_r[regno] = red >> 8;
-	intel_crtc->lut_g[regno] = green >> 8;
-	intel_crtc->lut_b[regno] = blue >> 8;
-}
-
-static void intel_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-				    u16 *blue, int regno)
-{
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	*red = intel_crtc->lut_r[regno] << 8;
-	*green = intel_crtc->lut_g[regno] << 8;
-	*blue = intel_crtc->lut_b[regno] << 8;
-}
-
 static struct drm_fb_helper_crtc *
 intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
 {
@@ -370,7 +349,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 		struct drm_connector *connector;
 		struct drm_encoder *encoder;
 		struct drm_fb_helper_crtc *new_crtc;
-		struct intel_crtc *intel_crtc;
 
 		fb_conn = fb_helper->connector_info[i];
 		connector = fb_conn->connector;
@@ -412,13 +390,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 
 		num_connectors_enabled++;
 
-		intel_crtc = to_intel_crtc(connector->state->crtc);
-		for (j = 0; j < 256; j++) {
-			intel_crtc->lut_r[j] = j;
-			intel_crtc->lut_g[j] = j;
-			intel_crtc->lut_b[j] = j;
-		}
-
 		new_crtc = intel_fb_helper_crtc(fb_helper,
 						connector->state->crtc);
 
@@ -519,8 +490,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 
 static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
 	.initial_config = intel_fb_initial_config,
-	.gamma_set = intel_crtc_fb_gamma_set,
-	.gamma_get = intel_crtc_fb_gamma_get,
 	.fb_probe = intelfb_create,
 };
 
-- 
2.1.4

[PATCH v2 10/14] drm: mgag200: remove dead code and pointless local lut storage

[Peter Rosin]

The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  5 ---
 drivers/gpu/drm/mgag200/mgag200_fb.c   |  2 --
 drivers/gpu/drm/mgag200/mgag200_mode.c | 62 ++++++++--------------------------
 3 files changed, 15 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index c88b6ec..04f1dfb 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -237,11 +237,6 @@ mgag200_bo(struct ttm_buffer_object *bo)
 {
 	return container_of(bo, struct mgag200_bo, bo);
 }
-				/* mgag200_crtc.c */
-void mga_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-			     u16 blue, int regno);
-void mga_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-			     u16 *blue, int regno);
 
 				/* mgag200_mode.c */
 int mgag200_modeset_init(struct mga_device *mdev);
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
index 5d3b1fa..5cf980a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -258,8 +258,6 @@ static int mga_fbdev_destroy(struct drm_device *dev,
 }
 
 static const struct drm_fb_helper_funcs mga_fb_helper_funcs = {
-	.gamma_set = mga_crtc_fb_gamma_set,
-	.gamma_get = mga_crtc_fb_gamma_get,
 	.fb_probe = mgag200fb_create,
 };
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index f4b5358..5e9cd4c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -27,15 +27,19 @@
 
 static void mga_crtc_load_lut(struct drm_crtc *crtc)
 {
-	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = dev->dev_private;
 	struct drm_framebuffer *fb = crtc->primary->fb;
+	u16 *r_ptr, *g_ptr, *b_ptr;
 	int i;
 
 	if (!crtc->enabled)
 		return;
 
+	r_ptr = crtc->gamma_store;
+	g_ptr = r_ptr + crtc->gamma_size;
+	b_ptr = g_ptr + crtc->gamma_size;
+
 	WREG8(DAC_INDEX + MGA1064_INDEX, 0);
 
 	if (fb && fb->format->cpp[0] * 8 == 16) {
@@ -46,25 +50,27 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc)
 				if (i > (MGAG200_LUT_SIZE >> 1)) {
 					r = b = 0;
 				} else {
-					r = mga_crtc->lut_r[i << 1];
-					b = mga_crtc->lut_b[i << 1];
+					r = *r_ptr++ >> 8;
+					b = *b_ptr++ >> 8;
+					r_ptr++;
+					b_ptr++;
 				}
 			} else {
-				r = mga_crtc->lut_r[i];
-				b = mga_crtc->lut_b[i];
+				r = *r_ptr++ >> 8;
+				b = *b_ptr++ >> 8;
 			}
 			/* VGA registers */
 			WREG8(DAC_INDEX + MGA1064_COL_PAL, r);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]);
+			WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
 			WREG8(DAC_INDEX + MGA1064_COL_PAL, b);
 		}
 		return;
 	}
 	for (i = 0; i < MGAG200_LUT_SIZE; i++) {
 		/* VGA registers */
-		WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_r[i]);
-		WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]);
-		WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_b[i]);
+		WREG8(DAC_INDEX + MGA1064_COL_PAL, *r_ptr++ >> 8);
+		WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
+		WREG8(DAC_INDEX + MGA1064_COL_PAL, *b_ptr++ >> 8);
 	}
 }
 
@@ -1399,14 +1405,6 @@ static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 			      u16 *blue, uint32_t size,
 			      struct drm_modeset_acquire_ctx *ctx)
 {
-	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
-	int i;
-
-	for (i = 0; i < size; i++) {
-		mga_crtc->lut_r[i] = red[i] >> 8;
-		mga_crtc->lut_g[i] = green[i] >> 8;
-		mga_crtc->lut_b[i] = blue[i] >> 8;
-	}
 	mga_crtc_load_lut(crtc);
 
 	return 0;
@@ -1455,14 +1453,12 @@ static const struct drm_crtc_helper_funcs mga_helper_funcs = {
 	.mode_set_base = mga_crtc_mode_set_base,
 	.prepare = mga_crtc_prepare,
 	.commit = mga_crtc_commit,
-	.load_lut = mga_crtc_load_lut,
 };
 
 /* CRTC setup */
 static void mga_crtc_init(struct mga_device *mdev)
 {
 	struct mga_crtc *mga_crtc;
-	int i;
 
 	mga_crtc = kzalloc(sizeof(struct mga_crtc) +
 			      (MGAG200FB_CONN_LIMIT * sizeof(struct drm_connector *)),
@@ -1476,37 +1472,9 @@ static void mga_crtc_init(struct mga_device *mdev)
 	drm_mode_crtc_set_gamma_size(&mga_crtc->base, MGAG200_LUT_SIZE);
 	mdev->mode_info.crtc = mga_crtc;
 
-	for (i = 0; i < MGAG200_LUT_SIZE; i++) {
-		mga_crtc->lut_r[i] = i;
-		mga_crtc->lut_g[i] = i;
-		mga_crtc->lut_b[i] = i;
-	}
-
 	drm_crtc_helper_add(&mga_crtc->base, &mga_helper_funcs);
 }
 
-/** Sets the color ramps on behalf of fbcon */
-void mga_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-			      u16 blue, int regno)
-{
-	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
-
-	mga_crtc->lut_r[regno] = red >> 8;
-	mga_crtc->lut_g[regno] = green >> 8;
-	mga_crtc->lut_b[regno] = blue >> 8;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-void mga_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, int regno)
-{
-	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
-
-	*red = (u16)mga_crtc->lut_r[regno] << 8;
-	*green = (u16)mga_crtc->lut_g[regno] << 8;
-	*blue = (u16)mga_crtc->lut_b[regno] << 8;
-}
-
 /*
  * The encoder comes after the CRTC in the output pipeline, but before
  * the connector. It's responsible for ensuring that the digital
-- 
2.1.4

[PATCH v2 11/14] drm: nouveau: remove dead code and pointless local lut storage

[Peter Rosin]

The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/nouveau/dispnv04/crtc.c | 26 ++++++++-------------
 drivers/gpu/drm/nouveau/nouveau_crtc.h  |  3 ---
 drivers/gpu/drm/nouveau/nouveau_fbcon.c | 22 ------------------
 drivers/gpu/drm/nouveau/nv50_display.c  | 40 +++++++++++----------------------
 4 files changed, 22 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 4b4b0b4..8f689f1 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -764,13 +764,18 @@ nv_crtc_gamma_load(struct drm_crtc *crtc)
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
 	struct drm_device *dev = nv_crtc->base.dev;
 	struct rgb { uint8_t r, g, b; } __attribute__((packed)) *rgbs;
+	u16 *r, *g, *b;
 	int i;
 
 	rgbs = (struct rgb *)nv04_display(dev)->mode_reg.crtc_reg[nv_crtc->index].DAC;
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
+
 	for (i = 0; i < 256; i++) {
-		rgbs[i].r = nv_crtc->lut.r[i] >> 8;
-		rgbs[i].g = nv_crtc->lut.g[i] >> 8;
-		rgbs[i].b = nv_crtc->lut.b[i] >> 8;
+		rgbs[i].r = *r++ >> 8;
+		rgbs[i].g = *g++ >> 8;
+		rgbs[i].b = *b++ >> 8;
 	}
 
 	nouveau_hw_load_state_palette(dev, nv_crtc->index, &nv04_display(dev)->mode_reg);
@@ -792,13 +797,6 @@ nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
 		  struct drm_modeset_acquire_ctx *ctx)
 {
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-	int i;
-
-	for (i = 0; i < size; i++) {
-		nv_crtc->lut.r[i] = r[i];
-		nv_crtc->lut.g[i] = g[i];
-		nv_crtc->lut.b[i] = b[i];
-	}
 
 	/* We need to know the depth before we upload, but it's possible to
 	 * get called before a framebuffer is bound.  If this is the case,
@@ -1095,7 +1093,6 @@ static const struct drm_crtc_helper_funcs nv04_crtc_helper_funcs = {
 	.mode_set = nv_crtc_mode_set,
 	.mode_set_base = nv04_crtc_mode_set_base,
 	.mode_set_base_atomic = nv04_crtc_mode_set_base_atomic,
-	.load_lut = nv_crtc_gamma_load,
 	.disable = nv_crtc_disable,
 };
 
@@ -1103,17 +1100,12 @@ int
 nv04_crtc_create(struct drm_device *dev, int crtc_num)
 {
 	struct nouveau_crtc *nv_crtc;
-	int ret, i;
+	int ret;
 
 	nv_crtc = kzalloc(sizeof(*nv_crtc), GFP_KERNEL);
 	if (!nv_crtc)
 		return -ENOMEM;
 
-	for (i = 0; i < 256; i++) {
-		nv_crtc->lut.r[i] = i << 8;
-		nv_crtc->lut.g[i] = i << 8;
-		nv_crtc->lut.b[i] = i << 8;
-	}
 	nv_crtc->lut.depth = 0;
 
 	nv_crtc->index = crtc_num;
diff --git a/drivers/gpu/drm/nouveau/nouveau_crtc.h b/drivers/gpu/drm/nouveau/nouveau_crtc.h
index 050fcf3..b7a18fb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_crtc.h
+++ b/drivers/gpu/drm/nouveau/nouveau_crtc.h
@@ -61,9 +61,6 @@ struct nouveau_crtc {
 
 	struct {
 		struct nouveau_bo *nvbo;
-		uint16_t r[256];
-		uint16_t g[256];
-		uint16_t b[256];
 		int depth;
 	} lut;
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 2665a07..f770784 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -278,26 +278,6 @@ nouveau_fbcon_accel_init(struct drm_device *dev)
 		info->fbops = &nouveau_fbcon_ops;
 }
 
-static void nouveau_fbcon_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-				    u16 blue, int regno)
-{
-	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-
-	nv_crtc->lut.r[regno] = red;
-	nv_crtc->lut.g[regno] = green;
-	nv_crtc->lut.b[regno] = blue;
-}
-
-static void nouveau_fbcon_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-				    u16 *blue, int regno)
-{
-	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-
-	*red = nv_crtc->lut.r[regno];
-	*green = nv_crtc->lut.g[regno];
-	*blue = nv_crtc->lut.b[regno];
-}
-
 static void
 nouveau_fbcon_zfill(struct drm_device *dev, struct nouveau_fbdev *fbcon)
 {
@@ -467,8 +447,6 @@ void nouveau_fbcon_gpu_lockup(struct fb_info *info)
 }
 
 static const struct drm_fb_helper_funcs nouveau_fbcon_helper_funcs = {
-	.gamma_set = nouveau_fbcon_gamma_set,
-	.gamma_get = nouveau_fbcon_gamma_get,
 	.fb_probe = nouveau_fbcon_create,
 };
 
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index e3132a2..0d57d61 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -2204,28 +2204,29 @@ nv50_head_lut_load(struct drm_crtc *crtc)
 	struct nv50_disp *disp = nv50_disp(crtc->dev);
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
 	void __iomem *lut = nvbo_kmap_obj_iovirtual(nv_crtc->lut.nvbo);
+	u16 *r, *g, *b;
 	int i;
 
-	for (i = 0; i < 256; i++) {
-		u16 r = nv_crtc->lut.r[i] >> 2;
-		u16 g = nv_crtc->lut.g[i] >> 2;
-		u16 b = nv_crtc->lut.b[i] >> 2;
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 
+	for (i = 0; i < 256; i++) {
 		if (disp->disp->oclass < GF110_DISP) {
-			writew(r + 0x0000, lut + (i * 0x08) + 0);
-			writew(g + 0x0000, lut + (i * 0x08) + 2);
-			writew(b + 0x0000, lut + (i * 0x08) + 4);
+			writew((*r++ >> 2) + 0x0000, lut + (i * 0x08) + 0);
+			writew((*g++ >> 2) + 0x0000, lut + (i * 0x08) + 2);
+			writew((*b++ >> 2) + 0x0000, lut + (i * 0x08) + 4);
 		} else {
-			writew(r + 0x6000, lut + (i * 0x20) + 0);
-			writew(g + 0x6000, lut + (i * 0x20) + 2);
-			writew(b + 0x6000, lut + (i * 0x20) + 4);
+			/* 0x6000 interferes with the 14-bit color??? */
+			writew((*r++ >> 2) + 0x6000, lut + (i * 0x20) + 0);
+			writew((*g++ >> 2) + 0x6000, lut + (i * 0x20) + 2);
+			writew((*b++ >> 2) + 0x6000, lut + (i * 0x20) + 4);
 		}
 	}
 }
 
 static const struct drm_crtc_helper_funcs
 nv50_head_help = {
-	.load_lut = nv50_head_lut_load,
 	.atomic_check = nv50_head_atomic_check,
 };
 
@@ -2234,15 +2235,6 @@ nv50_head_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
 		    uint32_t size,
 		    struct drm_modeset_acquire_ctx *ctx)
 {
-	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-	u32 i;
-
-	for (i = 0; i < size; i++) {
-		nv_crtc->lut.r[i] = r[i];
-		nv_crtc->lut.g[i] = g[i];
-		nv_crtc->lut.b[i] = b[i];
-	}
-
 	nv50_head_lut_load(crtc);
 	return 0;
 }
@@ -2340,19 +2332,13 @@ nv50_head_create(struct drm_device *dev, int index)
 	struct nv50_base *base;
 	struct nv50_curs *curs;
 	struct drm_crtc *crtc;
-	int ret, i;
+	int ret;
 
 	head = kzalloc(sizeof(*head), GFP_KERNEL);
 	if (!head)
 		return -ENOMEM;
 
 	head->base.index = index;
-	for (i = 0; i < 256; i++) {
-		head->base.lut.r[i] = i << 8;
-		head->base.lut.g[i] = i << 8;
-		head->base.lut.b[i] = i << 8;
-	}
-
 	ret = nv50_base_new(drm, head, &base);
 	if (ret == 0)
 		ret = nv50_curs_new(drm, head, &curs);
-- 
2.1.4

[PATCH v2 12/14] drm: radeon: remove dead code and pointless local lut storage

[Peter Rosin]

The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/radeon/atombios_crtc.c      |  1 -
 drivers/gpu/drm/radeon/radeon_connectors.c  |  7 ++-
 drivers/gpu/drm/radeon/radeon_display.c     | 71 ++++++++++++-----------------
 drivers/gpu/drm/radeon/radeon_fb.c          |  2 -
 drivers/gpu/drm/radeon/radeon_legacy_crtc.c |  1 -
 drivers/gpu/drm/radeon/radeon_mode.h        |  4 --
 6 files changed, 33 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 3c492a0..02baaaf 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -2217,7 +2217,6 @@ static const struct drm_crtc_helper_funcs atombios_helper_funcs = {
 	.mode_set_base_atomic = atombios_crtc_set_base_atomic,
 	.prepare = atombios_crtc_prepare,
 	.commit = atombios_crtc_commit,
-	.load_lut = radeon_crtc_load_lut,
 	.disable = atombios_crtc_disable,
 };
 
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 27affbd..2f642cb 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -773,12 +773,15 @@ static int radeon_connector_set_property(struct drm_connector *connector, struct
 
 		if (connector->encoder->crtc) {
 			struct drm_crtc *crtc  = connector->encoder->crtc;
-			const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
 			struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
 
 			radeon_crtc->output_csc = radeon_encoder->output_csc;
 
-			(*crtc_funcs->load_lut)(crtc);
+			/*
+			 * Our .gamma_set assumes the .gamma_store has been
+			 * prefilled and don't care about its arguments.
+			 */
+			crtc->funcs->gamma_set(crtc, NULL, NULL, NULL, 0, NULL);
 		}
 	}
 
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 17d3daf..8b7d7a0 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -42,6 +42,7 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct radeon_device *rdev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 
 	DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -60,11 +61,14 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(AVIVO_DC_LUT_WRITE_EN_MASK, 0x0000003f);
 
 	WREG8(AVIVO_DC_LUT_RW_INDEX, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(AVIVO_DC_LUT_30_COLOR,
-			     (radeon_crtc->lut_r[i] << 20) |
-			     (radeon_crtc->lut_g[i] << 10) |
-			     (radeon_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 
 	/* Only change bit 0 of LUT_SEL, other bits are set elsewhere */
@@ -76,6 +80,7 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct radeon_device *rdev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 
 	DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -93,11 +98,14 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(EVERGREEN_DC_LUT_WRITE_EN_MASK + radeon_crtc->crtc_offset, 0x00000007);
 
 	WREG32(EVERGREEN_DC_LUT_RW_INDEX + radeon_crtc->crtc_offset, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(EVERGREEN_DC_LUT_30_COLOR + radeon_crtc->crtc_offset,
-		       (radeon_crtc->lut_r[i] << 20) |
-		       (radeon_crtc->lut_g[i] << 10) |
-		       (radeon_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 }
 
@@ -106,6 +114,7 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct radeon_device *rdev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 
 	DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -135,11 +144,14 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(EVERGREEN_DC_LUT_WRITE_EN_MASK + radeon_crtc->crtc_offset, 0x00000007);
 
 	WREG32(EVERGREEN_DC_LUT_RW_INDEX + radeon_crtc->crtc_offset, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(EVERGREEN_DC_LUT_30_COLOR + radeon_crtc->crtc_offset,
-		       (radeon_crtc->lut_r[i] << 20) |
-		       (radeon_crtc->lut_g[i] << 10) |
-		       (radeon_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 
 	WREG32(NI_DEGAMMA_CONTROL + radeon_crtc->crtc_offset,
@@ -172,6 +184,7 @@ static void legacy_crtc_load_lut(struct drm_crtc *crtc)
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct radeon_device *rdev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 	uint32_t dac2_cntl;
 
@@ -183,11 +196,14 @@ static void legacy_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(RADEON_DAC_CNTL2, dac2_cntl);
 
 	WREG8(RADEON_PALETTE_INDEX, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(RADEON_PALETTE_30_DATA,
-			     (radeon_crtc->lut_r[i] << 20) |
-			     (radeon_crtc->lut_g[i] << 10) |
-			     (radeon_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 }
 
@@ -209,41 +225,10 @@ void radeon_crtc_load_lut(struct drm_crtc *crtc)
 		legacy_crtc_load_lut(crtc);
 }
 
-/** Sets the color ramps on behalf of fbcon */
-void radeon_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-			      u16 blue, int regno)
-{
-	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-
-	radeon_crtc->lut_r[regno] = red >> 6;
-	radeon_crtc->lut_g[regno] = green >> 6;
-	radeon_crtc->lut_b[regno] = blue >> 6;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-void radeon_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, int regno)
-{
-	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-
-	*red = radeon_crtc->lut_r[regno] << 6;
-	*green = radeon_crtc->lut_g[regno] << 6;
-	*blue = radeon_crtc->lut_b[regno] << 6;
-}
-
 static int radeon_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				 u16 *blue, uint32_t size,
 				 struct drm_modeset_acquire_ctx *ctx)
 {
-	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		radeon_crtc->lut_r[i] = red[i] >> 6;
-		radeon_crtc->lut_g[i] = green[i] >> 6;
-		radeon_crtc->lut_b[i] = blue[i] >> 6;
-	}
 	radeon_crtc_load_lut(crtc);
 
 	return 0;
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 356ad90..638bcb55 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -332,8 +332,6 @@ static int radeon_fbdev_destroy(struct drm_device *dev, struct radeon_fbdev *rfb
 }
 
 static const struct drm_fb_helper_funcs radeon_fb_helper_funcs = {
-	.gamma_set = radeon_crtc_fb_gamma_set,
-	.gamma_get = radeon_crtc_fb_gamma_get,
 	.fb_probe = radeonfb_create,
 };
 
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
index ce6cb66..1f1856e 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
@@ -1116,7 +1116,6 @@ static const struct drm_crtc_helper_funcs legacy_helper_funcs = {
 	.mode_set_base_atomic = radeon_crtc_set_base_atomic,
 	.prepare = radeon_crtc_prepare,
 	.commit = radeon_crtc_commit,
-	.load_lut = radeon_crtc_load_lut,
 	.disable = radeon_crtc_disable
 };
 
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 00f5ec5..da44ac2 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -935,10 +935,6 @@ extern void
 radeon_combios_encoder_crtc_scratch_regs(struct drm_encoder *encoder, int crtc);
 extern void
 radeon_combios_encoder_dpms_scratch_regs(struct drm_encoder *encoder, bool on);
-extern void radeon_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-				     u16 blue, int regno);
-extern void radeon_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-				     u16 *blue, int regno);
 int radeon_framebuffer_init(struct drm_device *dev,
 			     struct radeon_framebuffer *rfb,
 			     const struct drm_mode_fb_cmd2 *mode_cmd,
-- 
2.1.4

[PATCH v2 13/14] drm: stm: remove dead code and pointless local lut storage

[Peter Rosin]

The redundant fb helper .load_lut is no longer used, and can not
work right without also providing the fb helpers .gamma_set and
.gamma_get thus rendering the code in this driver suspect.

Just remove the dead code.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/stm/ltdc.c | 12 ------------
 drivers/gpu/drm/stm/ltdc.h |  1 -
 2 files changed, 13 deletions(-)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 1b9483d..87829b9 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -375,17 +375,6 @@ static irqreturn_t ltdc_irq(int irq, void *arg)
  * DRM_CRTC
  */
 
-static void ltdc_crtc_load_lut(struct drm_crtc *crtc)
-{
-	struct ltdc_device *ldev = crtc_to_ltdc(crtc);
-	unsigned int i, lay;
-
-	for (lay = 0; lay < ldev->caps.nb_layers; lay++)
-		for (i = 0; i < 256; i++)
-			reg_write(ldev->regs, LTDC_L1CLUTWR + lay * LAY_OFS,
-				  ldev->clut[i]);
-}
-
 static void ltdc_crtc_enable(struct drm_crtc *crtc)
 {
 	struct ltdc_device *ldev = crtc_to_ltdc(crtc);
@@ -523,7 +512,6 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc,
 }
 
 static struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = {
-	.load_lut = ltdc_crtc_load_lut,
 	.enable = ltdc_crtc_enable,
 	.disable = ltdc_crtc_disable,
 	.mode_set_nofb = ltdc_crtc_mode_set_nofb,
diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
index d7a9c73..620ca55 100644
--- a/drivers/gpu/drm/stm/ltdc.h
+++ b/drivers/gpu/drm/stm/ltdc.h
@@ -27,7 +27,6 @@ struct ltdc_device {
 	struct drm_panel *panel;
 	struct mutex err_lock;	/* protecting error_status */
 	struct ltdc_caps caps;
-	u32 clut[256];		/* color look up table */
 	u32 error_status;
 	u32 irq_status;
 };
-- 
2.1.4

[PATCH v2 14/14] drm: remove unused and redundant callbacks

[Peter Rosin]

Drivers no longer have any need for these callbacks, and there are no
users. Zap. Zap-zap-zzzap-p-pp-p.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 include/drm/drm_fb_helper.h              | 32 --------------------------------
 include/drm/drm_modeset_helper_vtables.h | 16 ----------------
 2 files changed, 48 deletions(-)

diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 119e5e4..80d9853 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -85,38 +85,6 @@ struct drm_fb_helper_surface_size {
  */
 struct drm_fb_helper_funcs {
 	/**
-	 * @gamma_set:
-	 *
-	 * Set the given gamma LUT register on the given CRTC.
-	 *
-	 * This callback is optional.
-	 *
-	 * FIXME:
-	 *
-	 * This callback is functionally redundant with the core gamma table
-	 * support and simply exists because the fbdev hasn't yet been
-	 * refactored to use the core gamma table interfaces.
-	 */
-	void (*gamma_set)(struct drm_crtc *crtc, u16 red, u16 green,
-			  u16 blue, int regno);
-	/**
-	 * @gamma_get:
-	 *
-	 * Read the given gamma LUT register on the given CRTC, used to save the
-	 * current LUT when force-restoring the fbdev for e.g. kdbg.
-	 *
-	 * This callback is optional.
-	 *
-	 * FIXME:
-	 *
-	 * This callback is functionally redundant with the core gamma table
-	 * support and simply exists because the fbdev hasn't yet been
-	 * refactored to use the core gamma table interfaces.
-	 */
-	void (*gamma_get)(struct drm_crtc *crtc, u16 *red, u16 *green,
-			  u16 *blue, int regno);
-
-	/**
 	 * @fb_probe:
 	 *
 	 * Driver callback to allocate and initialize the fbdev info structure.
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index 85984b2..0773db9 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -267,22 +267,6 @@ struct drm_crtc_helper_funcs {
 				    enum mode_set_atomic);
 
 	/**
-	 * @load_lut:
-	 *
-	 * Load a LUT prepared with the &drm_fb_helper_funcs.gamma_set vfunc.
-	 *
-	 * This callback is optional and is only used by the fbdev emulation
-	 * helpers.
-	 *
-	 * FIXME:
-	 *
-	 * This callback is functionally redundant with the core gamma table
-	 * support and simply exists because the fbdev hasn't yet been
-	 * refactored to use the core gamma table interfaces.
-	 */
-	void (*load_lut)(struct drm_crtc *crtc);
-
-	/**
 	 * @disable:
 	 *
 	 * This callback should be used to disable the CRTC. With the atomic
-- 
2.1.4

[PATCH v2 01/14] drm/fb-helper: keep the .gamma_store updated in drm_fb_helper_setcmap

[Peter Rosin]

I think the gamma_store can end up invalid on error. But the way I read
it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
this pesky legacy fbdev stuff be any better?

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

This is an alternative version rebased on top of Daniels "fbdev helper
locking rework and deferred setup" series.

Cheers,
peda

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index a4cfef9..c7122c9 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1330,12 +1330,16 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 	const struct drm_crtc_helper_funcs *crtc_funcs;
 	u16 *red, *green, *blue, *transp;
 	struct drm_crtc *crtc;
+	u16 *r, *g, *b;
 	int i, j, rc = 0;
 	int start;
 
 	if (oops_in_progress)
 		return -EBUSY;
 
+	if (cmap->start + cmap->len < cmap->start)
+		return -EINVAL;
+
 	mutex_lock(&fb_helper->lock);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
 		mutex_unlock(&fb_helper->lock);
@@ -1353,6 +1357,29 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 		transp = cmap->transp;
 		start = cmap->start;
 
+		if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
+			if (!crtc->gamma_size) {
+				rc = -EINVAL;
+				goto out;
+			}
+
+			if (cmap->start + cmap->len > crtc->gamma_size) {
+				rc = -EINVAL;
+				goto out;
+			}
+
+			r = crtc->gamma_store;
+			g = r + crtc->gamma_size;
+			b = g + crtc->gamma_size;
+
+			memcpy(r + cmap->start, cmap->red,
+			       cmap->len * sizeof(u16));
+			memcpy(g + cmap->start, cmap->green,
+			       cmap->len * sizeof(u16));
+			memcpy(b + cmap->start, cmap->blue,
+			       cmap->len * sizeof(u16));
+		}
+
 		for (j = 0; j < cmap->len; j++) {
 			u16 hred, hgreen, hblue, htransp = 0xffff;
 
-- 
2.1.4

[PATCH v2 03/14] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Peter Rosin]

This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
totally obsolete.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/drm_fb_helper.c | 151 +++++++++++++++++-----------------------
 1 file changed, 63 insertions(+), 88 deletions(-)

This is an alternative version rebased on top of Daniel's "fbdev helper
locking rework and deferred setup" series.

And as noted by Daniel, .gamma_set does an atomic commit. Thus, the locks
needs to be dropped and reacquired for each crtc. So, that is fixed here
too. Doing it like this with a couple of individual alternative patches
instead of sending a whole new series since the dependency on Daniel's
series makes life somewhat difficult...

Cheers,
peda

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4aceb59..aa025f1 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1257,50 +1257,6 @@ void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper,
 }
 EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
 
-static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
-		     u16 blue, u16 regno, struct fb_info *info)
-{
-	struct drm_fb_helper *fb_helper = info->par;
-	struct drm_framebuffer *fb = fb_helper->fb;
-
-	if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
-		u32 *palette;
-		u32 value;
-		/* place color in psuedopalette */
-		if (regno > 16)
-			return -EINVAL;
-		palette = (u32 *)info->pseudo_palette;
-		red >>= (16 - info->var.red.length);
-		green >>= (16 - info->var.green.length);
-		blue >>= (16 - info->var.blue.length);
-		value = (red << info->var.red.offset) |
-			(green << info->var.green.offset) |
-			(blue << info->var.blue.offset);
-		if (info->var.transp.length > 0) {
-			u32 mask = (1 << info->var.transp.length) - 1;
-
-			mask <<= info->var.transp.offset;
-			value |= mask;
-		}
-		palette[regno] = value;
-		return 0;
-	}
-
-	/*
-	 * The driver really shouldn't advertise pseudo/directcolor
-	 * visuals if it can't deal with the palette.
-	 */
-	if (WARN_ON(!fb_helper->funcs->gamma_set ||
-		    !fb_helper->funcs->gamma_get))
-		return -EINVAL;
-
-	WARN_ON(fb->format->cpp[0] != 1);
-
-	fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
-
-	return 0;
-}
-
 /**
  * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
  * @cmap: cmap to set
@@ -1310,12 +1266,10 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 {
 	struct drm_fb_helper *fb_helper = info->par;
 	struct drm_device *dev = fb_helper->dev;
-	const struct drm_crtc_helper_funcs *crtc_funcs;
-	u16 *red, *green, *blue, *transp;
+	struct drm_modeset_acquire_ctx ctx;
 	struct drm_crtc *crtc;
 	u16 *r, *g, *b;
-	int i, j, rc = 0;
-	int start;
+	int i, ret = 0;
 
 	if (oops_in_progress)
 		return -EBUSY;
@@ -1329,61 +1283,82 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 		return -EBUSY;
 	}
 
-	drm_modeset_lock_all(dev);
-	for (i = 0; i < fb_helper->crtc_count; i++) {
-		crtc = fb_helper->crtc_info[i].mode_set.crtc;
-		crtc_funcs = crtc->helper_private;
+	drm_modeset_acquire_init(&ctx, 0);
 
-		red = cmap->red;
-		green = cmap->green;
-		blue = cmap->blue;
-		transp = cmap->transp;
-		start = cmap->start;
+	for (i = 0; i < fb_helper->crtc_count; i++) {
+		if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
+			u32 *palette;
+			int j;
 
-		if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
-			if (!crtc->gamma_size) {
-				rc = -EINVAL;
-				goto out;
+			if (cmap->start + cmap->len > 16) {
+				ret = -EINVAL;
+				break;
 			}
 
-			if (cmap->start + cmap->len > crtc->gamma_size) {
-				rc = -EINVAL;
-				goto out;
+			palette = (u32 *)info->pseudo_palette;
+			for (j = 0; j < cmap->len; ++j) {
+				u16 red = cmap->red[j];
+				u16 green = cmap->green[j];
+				u16 blue = cmap->blue[j];
+				u32 value;
+
+				red >>= 16 - info->var.red.length;
+				green >>= 16 - info->var.green.length;
+				blue >>= 16 - info->var.blue.length;
+				value = (red << info->var.red.offset) |
+					(green << info->var.green.offset) |
+					(blue << info->var.blue.offset);
+				if (info->var.transp.length > 0) {
+					u32 mask = (1 << info->var.transp.length) - 1;
+
+					mask <<= info->var.transp.offset;
+					value |= mask;
+				}
+				palette[cmap->start + j] = value;
 			}
+			continue;
+		}
 
-			r = crtc->gamma_store;
-			g = r + crtc->gamma_size;
-			b = g + crtc->gamma_size;
+retry:
+		ret = drm_modeset_lock_all_ctx(dev, &ctx);
+		if (ret)
+			break;
 
-			memcpy(r + cmap->start, cmap->red,
-			       cmap->len * sizeof(u16));
-			memcpy(g + cmap->start, cmap->green,
-			       cmap->len * sizeof(u16));
-			memcpy(b + cmap->start, cmap->blue,
-			       cmap->len * sizeof(u16));
+		crtc = fb_helper->crtc_info[i].mode_set.crtc;
+		if (!crtc->funcs->gamma_set || !crtc->gamma_size) {
+			ret = -EINVAL;
+			goto drop_locks;
 		}
 
-		for (j = 0; j < cmap->len; j++) {
-			u16 hred, hgreen, hblue, htransp = 0xffff;
+		if (cmap->start + cmap->len > crtc->gamma_size) {
+			ret = -EINVAL;
+			goto drop_locks;
+		}
 
-			hred = *red++;
-			hgreen = *green++;
-			hblue = *blue++;
+		r = crtc->gamma_store;
+		g = r + crtc->gamma_size;
+		b = g + crtc->gamma_size;
 
-			if (transp)
-				htransp = *transp++;
+		memcpy(r + cmap->start, cmap->red, cmap->len * sizeof(u16));
+		memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(u16));
+		memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(u16));
 
-			rc = setcolreg(crtc, hred, hgreen, hblue, start++, info);
-			if (rc)
-				goto out;
+		ret = crtc->funcs->gamma_set(crtc, r, g, b,
+					     crtc->gamma_size, &ctx);
+		if (ret == -EDEADLK) {
+			drm_modeset_backoff(&ctx);
+			goto retry;
 		}
-		if (crtc_funcs->load_lut)
-			crtc_funcs->load_lut(crtc);
+drop_locks:
+		drm_modeset_drop_locks(&ctx);
+		if (ret)
+			break;
 	}
- out:
-	drm_modeset_unlock_all(dev);
+
+	drm_modeset_acquire_fini(&ctx);
 	mutex_unlock(&fb_helper->lock);
-	return rc;
+
+	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_setcmap);
 
-- 
2.1.4

Re: [PATCH v2 13/14] drm: stm: remove dead code and pointless local lut storage

[Philippe CORNU]

On 06/22/2017 08:06 AM, Peter Rosin wrote:
> The redundant fb helper .load_lut is no longer used, and can not
> work right without also providing the fb helpers .gamma_set and
> .gamma_get thus rendering the code in this driver suspect.
> 

Hi Peter,
STM32 chipsets supports 8-bit CLUT mode but this driver version does not 
support it "yet" (final patch has not been upstreamed because it was a 
too big fbdev patch for simply adding CLUT...).

Regarding your patch below, if it helps you to ease the drm framework 
update then I am agree to "acknowledge it" asap, else if you are not in 
a hurry, I would prefer a better and definitive patch handling 8-bit 
CLUT properly and I am ok to help or/and to do it : )

Extra questions:
- any plan to update modetest with the DRM_FORMAT_C8 support + gamma 
get/set?
- do you have a simple way to test clut with fbdev, last year we where 
using an old version of the SDL but I am still looking for a small piece 
of code to do it (else I will do it myself but C8 on fbdev is not really 
a priority ;-)

best regards,
Philippe

> Just remove the dead code.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>   drivers/gpu/drm/stm/ltdc.c | 12 ------------
>   drivers/gpu/drm/stm/ltdc.h |  1 -
>   2 files changed, 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index 1b9483d..87829b9 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -375,17 +375,6 @@ static irqreturn_t ltdc_irq(int irq, void *arg)
>    * DRM_CRTC
>    */
>   
> -static void ltdc_crtc_load_lut(struct drm_crtc *crtc)
> -{
> -	struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> -	unsigned int i, lay;
> -
> -	for (lay = 0; lay < ldev->caps.nb_layers; lay++)
> -		for (i = 0; i < 256; i++)
> -			reg_write(ldev->regs, LTDC_L1CLUTWR + lay * LAY_OFS,
> -				  ldev->clut[i]);
> -}
> -
>   static void ltdc_crtc_enable(struct drm_crtc *crtc)
>   {
>   	struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> @@ -523,7 +512,6 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc,
>   }
>   
>   static struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = {
> -	.load_lut = ltdc_crtc_load_lut,
>   	.enable = ltdc_crtc_enable,
>   	.disable = ltdc_crtc_disable,
>   	.mode_set_nofb = ltdc_crtc_mode_set_nofb,
> diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
> index d7a9c73..620ca55 100644
> --- a/drivers/gpu/drm/stm/ltdc.h
> +++ b/drivers/gpu/drm/stm/ltdc.h
> @@ -27,7 +27,6 @@ struct ltdc_device {
>   	struct drm_panel *panel;
>   	struct mutex err_lock;	/* protecting error_status */
>   	struct ltdc_caps caps;
> -	u32 clut[256];		/* color look up table */
>   	u32 error_status;
>   	u32 irq_status;
>   };
> 

Re: [PATCH v2 13/14] drm: stm: remove dead code and pointless local lut storage

[Peter Rosin]

On 2017-06-22 13:49, Philippe CORNU wrote:
> On 06/22/2017 08:06 AM, Peter Rosin wrote:
>> The redundant fb helper .load_lut is no longer used, and can not
>> work right without also providing the fb helpers .gamma_set and
>> .gamma_get thus rendering the code in this driver suspect.
>>
> 
> Hi Peter,
> STM32 chipsets supports 8-bit CLUT mode but this driver version does not 
> support it "yet" (final patch has not been upstreamed because it was a 
> too big fbdev patch for simply adding CLUT...).
> 
> Regarding your patch below, if it helps you to ease the drm framework 
> update then I am agree to "acknowledge it" asap, else if you are not in 
> a hurry, I would prefer a better and definitive patch handling 8-bit 
> CLUT properly and I am ok to help or/and to do it : )

Hi!

The thing is, without my series you will have to provide four callbacks.
The crtc .gamma_set and the three redundant fb helpers .gamma_get,
.gamma_set and .load_lut that pretty much does exactly what the crtc
.gamma_set is doing. Well not .gamma_get, but...

With my series, you only have to provide the crtc .gamma_set, which you
have to provide anyway. and ...the core will handle everything that
.gamma_get was used for...

I.e., your work to provide CLUT support should start with drm support,
which means the crtc .gamma_set, and then move on to the fbdev
emulation. And I have just eliminated the second step for you, and
as suger on top, you no longer have to convince the core drm maintainers
that adding a lot of fbdev emulation code is needed.

So, I think you actually want to wait for my series to land before adding
CLUT support.

> Extra questions:
> - any plan to update modetest with the DRM_FORMAT_C8 support + gamma 
> get/set?

I don't know that code base at all, but from the glimpse I got when browsing
it, it seemed like it was pretty hardwired to non-palettized modes. I ended
up having no need for it, see below...

> - do you have a simple way to test clut with fbdev, last year we where 
> using an old version of the SDL but I am still looking for a small piece 
> of code to do it (else I will do it myself but C8 on fbdev is not really 
> a priority ;-)

I'm doing pretty much the same thing, I have an application that requires
an old SDL, and I'm using the programs/demos/demo.c program from the very
old libggi as a second test app. But that's just because libggi is what
I'm most familiar with, and it doesn't try to be "nice" and do things
automatically, instead you have to manually insert helpers providing
e.g. palette emulation if the application assumes a palettized mode and
only truecolor modes are available from the HW. SDL tends to add those
things for you, making it less easy to test thing, but I'm not an
"SDL-guy", so there may very well exist some knobs I don't know about.

Oh, you probably didn't see this:
http://marc.info/?l=linux-kernel&m=149786920731175&w=4

It sports modeset-pal.c that sets the C8 mode, and does a 5 second
palette animation, w/o using fbdev. I used it instead of digging
further into modetest.

Cheers,
peda

Re: [Intel-gfx] [PATCH v2 13/14] drm: stm: remove dead code and pointless local lut storage

[Daniel Vetter]

On Thu, Jun 22, 2017 at 11:49:34AM +0000, Philippe CORNU wrote:
> 
> 
> On 06/22/2017 08:06 AM, Peter Rosin wrote:
> > The redundant fb helper .load_lut is no longer used, and can not
> > work right without also providing the fb helpers .gamma_set and
> > .gamma_get thus rendering the code in this driver suspect.
> > 
> 
> Hi Peter,
> STM32 chipsets supports 8-bit CLUT mode but this driver version does not 
> support it "yet" (final patch has not been upstreamed because it was a 
> too big fbdev patch for simply adding CLUT...).
> 
> Regarding your patch below, if it helps you to ease the drm framework 
> update then I am agree to "acknowledge it" asap, else if you are not in 
> a hurry, I would prefer a better and definitive patch handling 8-bit 
> CLUT properly and I am ok to help or/and to do it : )

I'll take your ack, since your 8bit lut support will be massively simpler
with Peter's series here :-)

> Extra questions:
> - any plan to update modetest with the DRM_FORMAT_C8 support + gamma 
> get/set?

We have gamma igts, well for the fancy new atomic color manager stuff.
Testing drivers with modetest is kinda not that awesome :-)
-Daniel

> - do you have a simple way to test clut with fbdev, last year we where 
> using an old version of the SDL but I am still looking for a small piece 
> of code to do it (else I will do it myself but C8 on fbdev is not really 
> a priority ;-)
> 
> best regards,
> Philippe
> 
> > Just remove the dead code.
> > 
> > Signed-off-by: Peter Rosin <peda@axentia.se>
> > ---
> >   drivers/gpu/drm/stm/ltdc.c | 12 ------------
> >   drivers/gpu/drm/stm/ltdc.h |  1 -
> >   2 files changed, 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> > index 1b9483d..87829b9 100644
> > --- a/drivers/gpu/drm/stm/ltdc.c
> > +++ b/drivers/gpu/drm/stm/ltdc.c
> > @@ -375,17 +375,6 @@ static irqreturn_t ltdc_irq(int irq, void *arg)
> >    * DRM_CRTC
> >    */
> >   
> > -static void ltdc_crtc_load_lut(struct drm_crtc *crtc)
> > -{
> > -	struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> > -	unsigned int i, lay;
> > -
> > -	for (lay = 0; lay < ldev->caps.nb_layers; lay++)
> > -		for (i = 0; i < 256; i++)
> > -			reg_write(ldev->regs, LTDC_L1CLUTWR + lay * LAY_OFS,
> > -				  ldev->clut[i]);
> > -}
> > -
> >   static void ltdc_crtc_enable(struct drm_crtc *crtc)
> >   {
> >   	struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> > @@ -523,7 +512,6 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc,
> >   }
> >   
> >   static struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = {
> > -	.load_lut = ltdc_crtc_load_lut,
> >   	.enable = ltdc_crtc_enable,
> >   	.disable = ltdc_crtc_disable,
> >   	.mode_set_nofb = ltdc_crtc_mode_set_nofb,
> > diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
> > index d7a9c73..620ca55 100644
> > --- a/drivers/gpu/drm/stm/ltdc.h
> > +++ b/drivers/gpu/drm/stm/ltdc.h
> > @@ -27,7 +27,6 @@ struct ltdc_device {
> >   	struct drm_panel *panel;
> >   	struct mutex err_lock;	/* protecting error_status */
> >   	struct ltdc_caps caps;
> > -	u32 clut[256];		/* color look up table */
> >   	u32 error_status;
> >   	u32 irq_status;
> >   };
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 02/14] drm/fb-helper: remove drm_fb_helper_save_lut_atomic

[Daniel Vetter]

On Thu, Jun 22, 2017 at 08:06:25AM +0200, Peter Rosin wrote:
> drm_fb_helper_save_lut_atomic is redundant since the .gamma_store is
> now always kept up to date by drm_fb_helper_setcmap.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

Also note that this is for kgdb support only and so likely very buggy
(since no one cried when we started to break kgdb support when switching
drivers to atomic).
-Daniel

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 17 -----------------
>  1 file changed, 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 25315fb..7ade384 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -229,22 +229,6 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
>  }
>  EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
>  
> -static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct drm_fb_helper *helper)
> -{
> -	uint16_t *r_base, *g_base, *b_base;
> -	int i;
> -
> -	if (helper->funcs->gamma_get == NULL)
> -		return;
> -
> -	r_base = crtc->gamma_store;
> -	g_base = r_base + crtc->gamma_size;
> -	b_base = g_base + crtc->gamma_size;
> -
> -	for (i = 0; i < crtc->gamma_size; i++)
> -		helper->funcs->gamma_get(crtc, &r_base[i], &g_base[i], &b_base[i], i);
> -}
> -
>  static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
>  {
>  	uint16_t *r_base, *g_base, *b_base;
> @@ -285,7 +269,6 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
>  			if (drm_drv_uses_atomic_modeset(mode_set->crtc->dev))
>  				continue;
>  
> -			drm_fb_helper_save_lut_atomic(mode_set->crtc, helper);
>  			funcs->mode_set_base_atomic(mode_set->crtc,
>  						    mode_set->fb,
>  						    mode_set->x,
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 03/14] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Daniel Vetter]

On Thu, Jun 22, 2017 at 08:06:26AM +0200, Peter Rosin wrote:
> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
> totally obsolete.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 154 ++++++++++++++++------------------------
>  1 file changed, 63 insertions(+), 91 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 7ade384..58eb045 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1150,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper,
>  }
>  EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>  
> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
> -		     u16 blue, u16 regno, struct fb_info *info)
> -{
> -	struct drm_fb_helper *fb_helper = info->par;
> -	struct drm_framebuffer *fb = fb_helper->fb;
> -
> -	if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
> -		u32 *palette;
> -		u32 value;
> -		/* place color in psuedopalette */
> -		if (regno > 16)
> -			return -EINVAL;
> -		palette = (u32 *)info->pseudo_palette;
> -		red >>= (16 - info->var.red.length);
> -		green >>= (16 - info->var.green.length);
> -		blue >>= (16 - info->var.blue.length);
> -		value = (red << info->var.red.offset) |
> -			(green << info->var.green.offset) |
> -			(blue << info->var.blue.offset);
> -		if (info->var.transp.length > 0) {
> -			u32 mask = (1 << info->var.transp.length) - 1;
> -
> -			mask <<= info->var.transp.offset;
> -			value |= mask;
> -		}
> -		palette[regno] = value;
> -		return 0;
> -	}
> -
> -	/*
> -	 * The driver really shouldn't advertise pseudo/directcolor
> -	 * visuals if it can't deal with the palette.
> -	 */
> -	if (WARN_ON(!fb_helper->funcs->gamma_set ||
> -		    !fb_helper->funcs->gamma_get))
> -		return -EINVAL;
> -
> -	WARN_ON(fb->format->cpp[0] != 1);
> -
> -	fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
> -
> -	return 0;
> -}
> -
>  /**
>   * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
>   * @cmap: cmap to set
> @@ -1203,12 +1159,10 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>  {
>  	struct drm_fb_helper *fb_helper = info->par;
>  	struct drm_device *dev = fb_helper->dev;
> -	const struct drm_crtc_helper_funcs *crtc_funcs;
> -	u16 *red, *green, *blue, *transp;
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct drm_crtc *crtc;
>  	u16 *r, *g, *b;
> -	int i, j, rc = 0;
> -	int start;
> +	int i, ret;
>  
>  	if (oops_in_progress)
>  		return -EBUSY;
> @@ -1216,65 +1170,83 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>  	if (cmap->start + cmap->len < cmap->start)
>  		return -EINVAL;
>  
> -	drm_modeset_lock_all(dev);
> +	drm_modeset_acquire_init(&ctx, 0);
> +retry:
> +	ret = drm_modeset_lock_all_ctx(dev, &ctx);
> +	if (ret)
> +		goto out;
>  	if (!drm_fb_helper_is_bound(fb_helper)) {
> -		drm_modeset_unlock_all(dev);
> -		return -EBUSY;
> +		ret = -EBUSY;
> +		goto out;
>  	}
>  
>  	for (i = 0; i < fb_helper->crtc_count; i++) {
> -		crtc = fb_helper->crtc_info[i].mode_set.crtc;
> -		crtc_funcs = crtc->helper_private;
> -
> -		red = cmap->red;
> -		green = cmap->green;
> -		blue = cmap->blue;
> -		transp = cmap->transp;
> -		start = cmap->start;
> +		if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
> +			u32 *palette;
> +			int j;
>  
> -		if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
> -			if (!crtc->gamma_size) {
> -				rc = -EINVAL;
> +			if (cmap->start + cmap->len > 16) {
> +				ret = -EINVAL;
>  				goto out;
>  			}
>  
> -			if (cmap->start + cmap->len > crtc->gamma_size) {
> -				rc = -EINVAL;
> -				goto out;
> +			palette = (u32 *)info->pseudo_palette;
> +			for (j = 0; j < cmap->len; ++j) {
> +				u16 red = cmap->red[j];
> +				u16 green = cmap->green[j];
> +				u16 blue = cmap->blue[j];
> +				u32 value;
> +
> +				red >>= 16 - info->var.red.length;
> +				green >>= 16 - info->var.green.length;
> +				blue >>= 16 - info->var.blue.length;
> +				value = (red << info->var.red.offset) |
> +					(green << info->var.green.offset) |
> +					(blue << info->var.blue.offset);
> +				if (info->var.transp.length > 0) {
> +					u32 mask = (1 << info->var.transp.length) - 1;
> +
> +					mask <<= info->var.transp.offset;
> +					value |= mask;
> +				}
> +				palette[cmap->start + j] = value;
>  			}
> +			continue;

I think it'd be much cleaner if we handle the TRUECOLOR case as an early
return, before the crtc loop. Per-crtc is only needed when we have to
update the hw table (i.e. PSEUDOCOLOR stuff).

> +		}
>  
> -			r = crtc->gamma_store;
> -			g = r + crtc->gamma_size;
> -			b = g + crtc->gamma_size;
> -
> -			memcpy(r + cmap->start, cmap->red,
> -			       cmap->len * sizeof(u16));
> -			memcpy(g + cmap->start, cmap->green,
> -			       cmap->len * sizeof(u16));
> -			memcpy(b + cmap->start, cmap->blue,
> -			       cmap->len * sizeof(u16));
> +		crtc = fb_helper->crtc_info[i].mode_set.crtc;
> +		if (!crtc->funcs->gamma_set || !crtc->gamma_size) {
> +			ret = -EINVAL;
> +			goto out;
>  		}
>  
> -		for (j = 0; j < cmap->len; j++) {
> -			u16 hred, hgreen, hblue, htransp = 0xffff;
> +		if (cmap->start + cmap->len > crtc->gamma_size) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
>  
> -			hred = *red++;
> -			hgreen = *green++;
> -			hblue = *blue++;
> +		r = crtc->gamma_store;
> +		g = r + crtc->gamma_size;
> +		b = g + crtc->gamma_size;
>  
> -			if (transp)
> -				htransp = *transp++;
> +		memcpy(r + cmap->start, cmap->red, cmap->len * sizeof(u16));
> +		memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(u16));
> +		memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(u16));
>  
> -			rc = setcolreg(crtc, hred, hgreen, hblue, start++, info);
> -			if (rc)
> -				goto out;
> -		}
> -		if (crtc_funcs->load_lut)
> -			crtc_funcs->load_lut(crtc);
> +		ret = crtc->funcs->gamma_set(crtc, r, g, b,
> +					     crtc->gamma_size, &ctx);

As discussed on earlier threads, I think the cleanest version here would
be 2 functions, one which does the crtc-loop like in your code here for
legacy drivers, and the 2nd one which does 1 atomic commit over all crtcs,
open-coding the gamma_set helper. For the legacy one you can also keep
using drm_modeset_lock_all if you want too.

And this will be all a bit easier on top of my branch, since then you can
untangle fbdev locking from the kms side better.
-Daniel

> +		if (ret)
> +			break;
>  	}
> - out:
> -	drm_modeset_unlock_all(dev);
> -	return rc;
> +out:
> +	if (ret == -EDEADLK) {
> +		drm_modeset_backoff(&ctx);
> +		goto retry;
> +	}
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_fb_helper_setcmap);
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 01/14] drm/fb-helper: keep the .gamma_store updated in drm_fb_helper_setcmap

[Daniel Vetter]

On Thu, Jun 22, 2017 at 08:06:24AM +0200, Peter Rosin wrote:
> I think the gamma_store can end up invalid on error. But the way I read
> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
> this pesky legacy fbdev stuff be any better?
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 574af01..25315fb 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1223,12 +1223,16 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>  	const struct drm_crtc_helper_funcs *crtc_funcs;
>  	u16 *red, *green, *blue, *transp;
>  	struct drm_crtc *crtc;
> +	u16 *r, *g, *b;
>  	int i, j, rc = 0;
>  	int start;
>  
>  	if (oops_in_progress)
>  		return -EBUSY;
>  
> +	if (cmap->start + cmap->len < cmap->start)
> +		return -EINVAL;

Doesn't the fbdev core check this for us?

> +
>  	drm_modeset_lock_all(dev);
>  	if (!drm_fb_helper_is_bound(fb_helper)) {
>  		drm_modeset_unlock_all(dev);
> @@ -1245,6 +1249,29 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>  		transp = cmap->transp;
>  		start = cmap->start;
>  
> +		if (info->fix.visual != FB_VISUAL_TRUECOLOR) {

I think switching to a positive check of visual == PSEUDOCOLOR is a bit
clearer. Also this makes more sense once we move over to the main
gamma_set function, since as-is it's unclear that setcolreg only does
something for PSEUDOCOLOR. But interim I guess that's ok.
-Daniel

> +			if (!crtc->gamma_size) {
> +				rc = -EINVAL;
> +				goto out;
> +			}
> +
> +			if (cmap->start + cmap->len > crtc->gamma_size) {
> +				rc = -EINVAL;
> +				goto out;
> +			}
> +
> +			r = crtc->gamma_store;
> +			g = r + crtc->gamma_size;
> +			b = g + crtc->gamma_size;
> +
> +			memcpy(r + cmap->start, cmap->red,
> +			       cmap->len * sizeof(u16));
> +			memcpy(g + cmap->start, cmap->green,
> +			       cmap->len * sizeof(u16));
> +			memcpy(b + cmap->start, cmap->blue,
> +			       cmap->len * sizeof(u16));
> +		}
> +
>  		for (j = 0; j < cmap->len; j++) {
>  			u16 hred, hgreen, hblue, htransp = 0xffff;
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 03/14] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Daniel Vetter]

On Thu, Jun 22, 2017 at 12:22:10PM +0200, Peter Rosin wrote:
> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
> totally obsolete.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 151 +++++++++++++++++-----------------------
>  1 file changed, 63 insertions(+), 88 deletions(-)
> 
> This is an alternative version rebased on top of Daniel's "fbdev helper
> locking rework and deferred setup" series.
> 
> And as noted by Daniel, .gamma_set does an atomic commit. Thus, the locks
> needs to be dropped and reacquired for each crtc. So, that is fixed here
> too. Doing it like this with a couple of individual alternative patches
> instead of sending a whole new series since the dependency on Daniel's
> series makes life somewhat difficult...
> 
> Cheers,
> peda
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 4aceb59..aa025f1 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1257,50 +1257,6 @@ void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper,
>  }
>  EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>  
> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
> -		     u16 blue, u16 regno, struct fb_info *info)
> -{
> -	struct drm_fb_helper *fb_helper = info->par;
> -	struct drm_framebuffer *fb = fb_helper->fb;
> -
> -	if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
> -		u32 *palette;
> -		u32 value;
> -		/* place color in psuedopalette */
> -		if (regno > 16)
> -			return -EINVAL;
> -		palette = (u32 *)info->pseudo_palette;
> -		red >>= (16 - info->var.red.length);
> -		green >>= (16 - info->var.green.length);
> -		blue >>= (16 - info->var.blue.length);
> -		value = (red << info->var.red.offset) |
> -			(green << info->var.green.offset) |
> -			(blue << info->var.blue.offset);
> -		if (info->var.transp.length > 0) {
> -			u32 mask = (1 << info->var.transp.length) - 1;
> -
> -			mask <<= info->var.transp.offset;
> -			value |= mask;
> -		}
> -		palette[regno] = value;
> -		return 0;
> -	}
> -
> -	/*
> -	 * The driver really shouldn't advertise pseudo/directcolor
> -	 * visuals if it can't deal with the palette.
> -	 */
> -	if (WARN_ON(!fb_helper->funcs->gamma_set ||
> -		    !fb_helper->funcs->gamma_get))
> -		return -EINVAL;
> -
> -	WARN_ON(fb->format->cpp[0] != 1);
> -
> -	fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
> -
> -	return 0;
> -}
> -
>  /**
>   * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
>   * @cmap: cmap to set
> @@ -1310,12 +1266,10 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>  {
>  	struct drm_fb_helper *fb_helper = info->par;
>  	struct drm_device *dev = fb_helper->dev;
> -	const struct drm_crtc_helper_funcs *crtc_funcs;
> -	u16 *red, *green, *blue, *transp;
> +	struct drm_modeset_acquire_ctx ctx;
>  	struct drm_crtc *crtc;
>  	u16 *r, *g, *b;
> -	int i, j, rc = 0;
> -	int start;
> +	int i, ret = 0;
>  
>  	if (oops_in_progress)
>  		return -EBUSY;
> @@ -1329,61 +1283,82 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>  		return -EBUSY;
>  	}
>  
> -	drm_modeset_lock_all(dev);
> -	for (i = 0; i < fb_helper->crtc_count; i++) {
> -		crtc = fb_helper->crtc_info[i].mode_set.crtc;
> -		crtc_funcs = crtc->helper_private;
> +	drm_modeset_acquire_init(&ctx, 0);
>  
> -		red = cmap->red;
> -		green = cmap->green;
> -		blue = cmap->blue;
> -		transp = cmap->transp;
> -		start = cmap->start;
> +	for (i = 0; i < fb_helper->crtc_count; i++) {
> +		if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
> +			u32 *palette;
> +			int j;
>  
> -		if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
> -			if (!crtc->gamma_size) {
> -				rc = -EINVAL;
> -				goto out;
> +			if (cmap->start + cmap->len > 16) {
> +				ret = -EINVAL;
> +				break;
>  			}
>  
> -			if (cmap->start + cmap->len > crtc->gamma_size) {
> -				rc = -EINVAL;
> -				goto out;
> +			palette = (u32 *)info->pseudo_palette;
> +			for (j = 0; j < cmap->len; ++j) {
> +				u16 red = cmap->red[j];
> +				u16 green = cmap->green[j];
> +				u16 blue = cmap->blue[j];
> +				u32 value;
> +
> +				red >>= 16 - info->var.red.length;
> +				green >>= 16 - info->var.green.length;
> +				blue >>= 16 - info->var.blue.length;
> +				value = (red << info->var.red.offset) |
> +					(green << info->var.green.offset) |
> +					(blue << info->var.blue.offset);
> +				if (info->var.transp.length > 0) {
> +					u32 mask = (1 << info->var.transp.length) - 1;
> +
> +					mask <<= info->var.transp.offset;
> +					value |= mask;
> +				}
> +				palette[cmap->start + j] = value;
>  			}
> +			continue;
> +		}
>  
> -			r = crtc->gamma_store;
> -			g = r + crtc->gamma_size;
> -			b = g + crtc->gamma_size;
> +retry:
> +		ret = drm_modeset_lock_all_ctx(dev, &ctx);
> +		if (ret)
> +			break;
>  
> -			memcpy(r + cmap->start, cmap->red,
> -			       cmap->len * sizeof(u16));
> -			memcpy(g + cmap->start, cmap->green,
> -			       cmap->len * sizeof(u16));
> -			memcpy(b + cmap->start, cmap->blue,
> -			       cmap->len * sizeof(u16));
> +		crtc = fb_helper->crtc_info[i].mode_set.crtc;
> +		if (!crtc->funcs->gamma_set || !crtc->gamma_size) {
> +			ret = -EINVAL;
> +			goto drop_locks;
>  		}
>  
> -		for (j = 0; j < cmap->len; j++) {
> -			u16 hred, hgreen, hblue, htransp = 0xffff;
> +		if (cmap->start + cmap->len > crtc->gamma_size) {
> +			ret = -EINVAL;
> +			goto drop_locks;
> +		}
>  
> -			hred = *red++;
> -			hgreen = *green++;
> -			hblue = *blue++;
> +		r = crtc->gamma_store;
> +		g = r + crtc->gamma_size;
> +		b = g + crtc->gamma_size;
>  
> -			if (transp)
> -				htransp = *transp++;
> +		memcpy(r + cmap->start, cmap->red, cmap->len * sizeof(u16));
> +		memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(u16));
> +		memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(u16));
>  
> -			rc = setcolreg(crtc, hred, hgreen, hblue, start++, info);
> -			if (rc)
> -				goto out;
> +		ret = crtc->funcs->gamma_set(crtc, r, g, b,
> +					     crtc->gamma_size, &ctx);
> +		if (ret == -EDEADLK) {
> +			drm_modeset_backoff(&ctx);
> +			goto retry;
>  		}
> -		if (crtc_funcs->load_lut)
> -			crtc_funcs->load_lut(crtc);
> +drop_locks:
> +		drm_modeset_drop_locks(&ctx);

This doesn't work, you can't reuse the acquire_ctx (that's the bit where
the locking machinery gets mad at you). Simplest way really is 2 paths,
one legacy (using drm_modeset_lock_all), the other atomic (with the
gamma_set helper logic inlined, but as one global atomic commit).

I guess updating the gamma_store could be done with common code again, on
success only (another reason for atomic, that way you get all or nothing,
makes error handling easier).
-Daniel

> +		if (ret)
> +			break;
>  	}
> - out:
> -	drm_modeset_unlock_all(dev);
> +
> +	drm_modeset_acquire_fini(&ctx);
>  	mutex_unlock(&fb_helper->lock);
> -	return rc;
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_fb_helper_setcmap);
>  
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 00/14] improve the fb_setcmap helper

[Daniel Vetter]

On Thu, Jun 22, 2017 at 08:06:23AM +0200, Peter Rosin wrote:
> Hi!
> 
> While trying to get CLUT support for the atmel_hlcdc driver, and
> specifically for the emulated fbdev interface, I received some
> push-back that my feeble in-driver attempts should be solved
> by the core. This is my attempt to do it right.
> 
> I have obviously not tested all of this with more than a compile,
> but patches 1 and 3 are enough to make the atmel-hlcdc driver
> do what I need (when patched to support CLUT modes). The rest is
> just lots of removals and cleanup made possible by the improved
> core.
> 
> Please test, I would not be surprised if I have fouled up some
> bit-manipulation somewhere in this mostly mechanical change...
> 
> Changes since v1:
> 
> - Rebased to next-20170621
> - Split 1/11 into a preparatory patch, a cleanup patch and then
>   the meat in 3/14.
> - Handle pseudo-palette for FB_VISUAL_TRUECOLOR.
> - Removed the empty .gamma_get/.gamma_set fb helpers from the
>   armada driver that I had somehow managed to ignore but which
>   0day found real quick.
> - Be less judgemental on drivers only providing .gamma_get and
>   .gamma_set, but no .load_lut. That's actually a valid thing
>   to do if you only need pseudo-palette for FB_VISUAL_TRUECOLOR.
> - Add a comment about colliding bitfields in the nouveau driver.
> - Remove gamma_set/gamma_get declarations from the radeon driver
>   (the definitions were removed in v1).

Ok some nits/questions on the first three, but in principle looks all ok I
think. The driver patches also look good (but I didn't yet carefully
review all the conversion). What we might want to do is entirely remove
driver's reliance on ->gamma_store (mostly amounts to in-lining the
load_lut functions) and only update ->gamma_store after gamma_set returned
successfully. But that's a bit more work.

Save/restoring it instead might be simpler to fix that bug, but since it's
pre-existing also ok as follow-up.

Cheers, Daniel

> 
> Cheers,
> peda
> 
> Peter Rosin (14):
>   drm/fb-helper: keep the .gamma_store updated in drm_fb_helper_setcmap
>   drm/fb-helper: remove drm_fb_helper_save_lut_atomic
>   drm/fb-helper: do a generic fb_setcmap helper in terms of crtc
>     .gamma_set
>   drm: amd: remove dead code and pointless local lut storage
>   drm: armada: remove dead empty functions
>   drm: ast: remove dead code and pointless local lut storage
>   drm: cirrus: remove dead code and pointless local lut storage
>   drm: gma500: remove dead code and pointless local lut storage
>   drm: i915: remove dead code and pointless local lut storage
>   drm: mgag200: remove dead code and pointless local lut storage
>   drm: nouveau: remove dead code and pointless local lut storage
>   drm: radeon: remove dead code and pointless local lut storage
>   drm: stm: remove dead code and pointless local lut storage
>   drm: remove unused and redundant callbacks
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c      |  24 ----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h    |   1 -
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c      |  27 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c      |  27 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c       |  27 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c       |  27 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c    |  23 ----
>  drivers/gpu/drm/armada/armada_crtc.c        |  10 --
>  drivers/gpu/drm/armada/armada_crtc.h        |   2 -
>  drivers/gpu/drm/armada/armada_fbdev.c       |   2 -
>  drivers/gpu/drm/ast/ast_drv.h               |   1 -
>  drivers/gpu/drm/ast/ast_fb.c                |  20 ----
>  drivers/gpu/drm/ast/ast_mode.c              |  26 +----
>  drivers/gpu/drm/cirrus/cirrus_drv.h         |   8 --
>  drivers/gpu/drm/cirrus/cirrus_fbdev.c       |   2 -
>  drivers/gpu/drm/cirrus/cirrus_mode.c        |  71 +++---------
>  drivers/gpu/drm/drm_fb_helper.c             | 164 +++++++++++++---------------
>  drivers/gpu/drm/gma500/framebuffer.c        |  22 ----
>  drivers/gpu/drm/gma500/gma_display.c        |  32 ++----
>  drivers/gpu/drm/gma500/psb_intel_display.c  |   7 +-
>  drivers/gpu/drm/gma500/psb_intel_drv.h      |   1 -
>  drivers/gpu/drm/i915/intel_drv.h            |   1 -
>  drivers/gpu/drm/i915/intel_fbdev.c          |  31 ------
>  drivers/gpu/drm/mgag200/mgag200_drv.h       |   5 -
>  drivers/gpu/drm/mgag200/mgag200_fb.c        |   2 -
>  drivers/gpu/drm/mgag200/mgag200_mode.c      |  62 +++--------
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  26 ++---
>  drivers/gpu/drm/nouveau/nouveau_crtc.h      |   3 -
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  22 ----
>  drivers/gpu/drm/nouveau/nv50_display.c      |  40 +++----
>  drivers/gpu/drm/radeon/atombios_crtc.c      |   1 -
>  drivers/gpu/drm/radeon/radeon_connectors.c  |   7 +-
>  drivers/gpu/drm/radeon/radeon_display.c     |  71 +++++-------
>  drivers/gpu/drm/radeon/radeon_fb.c          |   2 -
>  drivers/gpu/drm/radeon/radeon_legacy_crtc.c |   1 -
>  drivers/gpu/drm/radeon/radeon_mode.h        |   4 -
>  drivers/gpu/drm/stm/ltdc.c                  |  12 --
>  drivers/gpu/drm/stm/ltdc.h                  |   1 -
>  include/drm/drm_fb_helper.h                 |  32 ------
>  include/drm/drm_modeset_helper_vtables.h    |  16 ---
>  40 files changed, 205 insertions(+), 658 deletions(-)
> 
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 00/14] improve the fb_setcmap helper

[Peter Rosin]

On 2017-06-26 11:35, Daniel Vetter wrote:
> On Thu, Jun 22, 2017 at 08:06:23AM +0200, Peter Rosin wrote:
>> Hi!
>>
>> While trying to get CLUT support for the atmel_hlcdc driver, and
>> specifically for the emulated fbdev interface, I received some
>> push-back that my feeble in-driver attempts should be solved
>> by the core. This is my attempt to do it right.
>>
>> I have obviously not tested all of this with more than a compile,
>> but patches 1 and 3 are enough to make the atmel-hlcdc driver
>> do what I need (when patched to support CLUT modes). The rest is
>> just lots of removals and cleanup made possible by the improved
>> core.
>>
>> Please test, I would not be surprised if I have fouled up some
>> bit-manipulation somewhere in this mostly mechanical change...
>>
>> Changes since v1:
>>
>> - Rebased to next-20170621
>> - Split 1/11 into a preparatory patch, a cleanup patch and then
>>   the meat in 3/14.
>> - Handle pseudo-palette for FB_VISUAL_TRUECOLOR.
>> - Removed the empty .gamma_get/.gamma_set fb helpers from the
>>   armada driver that I had somehow managed to ignore but which
>>   0day found real quick.
>> - Be less judgemental on drivers only providing .gamma_get and
>>   .gamma_set, but no .load_lut. That's actually a valid thing
>>   to do if you only need pseudo-palette for FB_VISUAL_TRUECOLOR.
>> - Add a comment about colliding bitfields in the nouveau driver.
>> - Remove gamma_set/gamma_get declarations from the radeon driver
>>   (the definitions were removed in v1).
> 
> Ok some nits/questions on the first three, but in principle looks all ok I
> think. The driver patches also look good (but I didn't yet carefully
> review all the conversion). What we might want to do is entirely remove
> driver's reliance on ->gamma_store (mostly amounts to in-lining the
> load_lut functions) and only update ->gamma_store after gamma_set returned
> successfully. But that's a bit more work.
> 
> Save/restoring it instead might be simpler to fix that bug, but since it's
> pre-existing also ok as follow-up.

I'm traveling and cannot make progress this week. The merge window is
also real close so this series will therefore probably miss it unless
something unexpected happens...

I'll get back to this for the next cycle, just a heads up.

Cheers,
peda

Re: [PATCH v2 00/14] improve the fb_setcmap helper

[Dave Airlie]

>
> I'm traveling and cannot make progress this week. The merge window is
> also real close so this series will therefore probably miss it unless
> something unexpected happens...

Don't worry you missed the merge window for drm already, we don't merge
things after -rc6. Please remember the Linus merge window is for maintainers
to merge stuff to Linus, things need to be ready long before it, and for drm
you shouldn't really tie yourself to the merge cycle.

Dave.

Re: [Nouveau] [PATCH v2 00/14] improve the fb_setcmap helper

[Daniel Vetter]

On Tue, Jun 27, 2017 at 05:50:38AM +1000, Dave Airlie wrote:
> >
> > I'm traveling and cannot make progress this week. The merge window is
> > also real close so this series will therefore probably miss it unless
> > something unexpected happens...
> 
> Don't worry you missed the merge window for drm already, we don't merge
> things after -rc6. Please remember the Linus merge window is for maintainers
> to merge stuff to Linus, things need to be ready long before it, and for drm
> you shouldn't really tie yourself to the merge cycle.

Yeah, drm-misc is open for refactorings like this all the time, and
maintainers make sure the code will get into upstream asap, without
interferring with the upstream merge window schedules. Like Dave said,
don't worry.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch