[PATCH v3 00/16] improve the fb_setcmap helper

[PATCH v3 00/16] 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 through 5 are enough to make the atmel-hlcdc driver
do what I need. 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, or if I have misunderstood something
about atomics...

Changes since v2:
- Added patch 1/16 which factors out pseudo-palette handling.
- Removed the if (cmap->start + cmap->len < cmap->start)
  sanity check on the assumption that the fbdev core handles it.
- Added patch 4/16 which factors out atomic state and commit
  handling from drm_atomic_helper_legacy_gamma_set to
  drm_mode_gamma_set_ioctl.
- Do one atomic commit for all affected crtc.
- Removed a now obsolete note in include/drm/drm_crtc.h (ammended
  the last patch).
- Cc list is getting long, so I have redused the list for the
  individual patches. If you would like to get the full series
  (or nothing at all) for the next round (if that is needed) just
  say so.

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 (16):
  drm/fb-helper: factor out pseudo-palette
  drm/fb-helper: keep the .gamma_store updated in drm_fb_helper_setcmap
  drm/fb-helper: remove drm_fb_helper_save_lut_atomic
  drm/color-mgmt: move atomic state/commit out from .gamma_set
  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      |  29 ++---
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c      |  29 ++---
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c       |  29 ++---
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c       |  29 ++---
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c    |  25 +---
 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              |  28 +---
 drivers/gpu/drm/cirrus/cirrus_drv.h         |   8 --
 drivers/gpu/drm/cirrus/cirrus_fbdev.c       |   2 -
 drivers/gpu/drm/cirrus/cirrus_mode.c        |  73 +++--------
 drivers/gpu/drm/drm_atomic_helper.c         |  37 ++----
 drivers/gpu/drm/drm_color_mgmt.c            |  27 +++-
 drivers/gpu/drm/drm_fb_helper.c             | 195 +++++++++++++++++-----------
 drivers/gpu/drm/gma500/framebuffer.c        |  22 ----
 drivers/gpu/drm/gma500/gma_display.c        |  34 ++---
 drivers/gpu/drm/gma500/gma_display.h        |   2 +-
 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      |  64 +++------
 drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  28 ++--
 drivers/gpu/drm/nouveau/nouveau_crtc.h      |   3 -
 drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  22 ----
 drivers/gpu/drm/nouveau/nv50_display.c      |  42 ++----
 drivers/gpu/drm/radeon/atombios_crtc.c      |   1 -
 drivers/gpu/drm/radeon/radeon_connectors.c  |   7 +-
 drivers/gpu/drm/radeon/radeon_display.c     |  73 +++++------
 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 -
 drivers/gpu/drm/vc4/vc4_crtc.c              |   2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |   2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h         |   2 +-
 include/drm/drm_atomic_helper.h             |   2 +-
 include/drm/drm_crtc.h                      |  11 +-
 include/drm/drm_fb_helper.h                 |  32 -----
 include/drm/drm_modeset_helper_vtables.h    |  16 ---
 48 files changed, 301 insertions(+), 702 deletions(-)

-- 
2.1.4

[PATCH v3 01/16] drm/fb-helper: factor out pseudo-palette

[Peter Rosin]

The pseudo-palette has nothing to do with the crtc, so move it
out of the crtc loop and update the palette once, then break out
early.

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

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index a4cfef9..9c76b8c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1280,29 +1280,6 @@ static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
 	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.
@@ -1318,6 +1295,38 @@ static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
 	return 0;
 }
 
+static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info)
+{
+	u32 *palette = (u32 *)info->pseudo_palette;
+	int i;
+
+	if (cmap->start + cmap->len > 16)
+		return -EINVAL;
+
+	for (i = 0; i < cmap->len; ++i) {
+		u16 red = cmap->red[i];
+		u16 green = cmap->green[i];
+		u16 blue = cmap->blue[i];
+		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 + i] = value;
+	}
+
+	return 0;
+}
+
 /**
  * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
  * @cmap: cmap to set
@@ -1343,6 +1352,11 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 	}
 
 	drm_modeset_lock_all(dev);
+	if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
+		rc = setcmap_pseudo_palette(cmap, info);
+		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;
-- 
2.1.4

Re: [PATCH v3 01/16] drm/fb-helper: factor out pseudo-palette

[Peter Rosin]

On 2017-07-04 12:36, Peter Rosin wrote:
> The pseudo-palette has nothing to do with the crtc, so move it
> out of the crtc loop and update the palette once, then break out
> early.
> 
> Signed-off-by: Peter Rosin <peda@axenita.se>

Should of course be peda@axentia.se

I wonder when I managed to Ctrl-T that one?

Cheers,
peda

Re: [PATCH v3 01/16] drm/fb-helper: factor out pseudo-palette

[Daniel Vetter]

On Tue, Jul 04, 2017 at 12:40:16PM +0200, Peter Rosin wrote:
> On 2017-07-04 12:36, Peter Rosin wrote:
> > The pseudo-palette has nothing to do with the crtc, so move it
> > out of the crtc loop and update the palette once, then break out
> > early.
> > 
> > Signed-off-by: Peter Rosin <peda@axenita.se>
> 
> Should of course be peda@axentia.se
> 
> I wonder when I managed to Ctrl-T that one?

fyi

git commit -s

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

[PATCH v3 02/16] 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 | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 9c76b8c..41fd9e0 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1339,6 +1339,7 @@ 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;
 
@@ -1367,6 +1368,24 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 		transp = cmap->transp;
 		start = cmap->start;
 
+		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(*r));
+		memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(*g));
+		memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(*b));
+
 		for (j = 0; j < cmap->len; j++) {
 			u16 hred, hgreen, hblue, htransp = 0xffff;
 
-- 
2.1.4

[PATCH v3 03/16] 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 41fd9e0..b75b1f2 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -253,22 +253,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;
@@ -309,7 +293,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

Re: [PATCH v3 03/16] drm/fb-helper: remove drm_fb_helper_save_lut_atomic

[Daniel Vetter]

On Tue, Jul 04, 2017 at 12:36:59PM +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>

Merged up to this patch to drm-misc-next, thanks. Pleas base your next
round of patches on top of that tree (or drm-tip if you feel like).
-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 41fd9e0..b75b1f2 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -253,22 +253,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;
> @@ -309,7 +293,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

[PATCH v3 04/16] drm/color-mgmt: move atomic state/commit out from .gamma_set

[Peter Rosin]

Handle the atomics directly in the ioctl instead, in preparation for the
fb_setcmap helper needing to commit the gamma map for several crtc in one
commit.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c |  2 +-
 drivers/gpu/drm/ast/ast_mode.c           |  2 +-
 drivers/gpu/drm/cirrus/cirrus_mode.c     |  2 +-
 drivers/gpu/drm/drm_atomic_helper.c      | 37 +++++++-------------------------
 drivers/gpu/drm/drm_color_mgmt.c         | 27 ++++++++++++++++++++++-
 drivers/gpu/drm/gma500/gma_display.c     |  2 +-
 drivers/gpu/drm/gma500/gma_display.h     |  2 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c   |  2 +-
 drivers/gpu/drm/nouveau/dispnv04/crtc.c  |  2 +-
 drivers/gpu/drm/nouveau/nv50_display.c   |  2 +-
 drivers/gpu/drm/radeon/radeon_display.c  |  2 +-
 drivers/gpu/drm/vc4/vc4_crtc.c           |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c      |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h      |  2 +-
 include/drm/drm_atomic_helper.h          |  2 +-
 include/drm/drm_crtc.h                   |  3 +--
 20 files changed, 52 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 9f78c03..31c977b 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2622,7 +2622,7 @@ static void dce_v10_0_cursor_reset(struct drm_crtc *crtc)
 
 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 drm_crtc_state *state)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 4bcf01d..cf42640 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -2642,7 +2642,7 @@ static void dce_v11_0_cursor_reset(struct drm_crtc *crtc)
 
 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 drm_crtc_state *state)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index fd134a4..045014d 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -2494,7 +2494,7 @@ static void dce_v6_0_cursor_reset(struct drm_crtc *crtc)
 
 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 drm_crtc_state *state)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index a9e8695..ee9389f 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -2473,7 +2473,7 @@ static void dce_v8_0_cursor_reset(struct drm_crtc *crtc)
 
 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 drm_crtc_state *state)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index 90bb083..f194dfc 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -166,7 +166,7 @@ static void dce_virtual_bandwidth_update(struct amdgpu_device *adev)
 
 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 drm_crtc_state *state)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index aaef0a6..6f0335b 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -646,7 +646,7 @@ static void ast_crtc_reset(struct drm_crtc *crtc)
 
 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 drm_crtc_state *state)
 {
 	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c
index 53f6f0f..2b4c2c3 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -328,7 +328,7 @@ static void cirrus_crtc_commit(struct drm_crtc *crtc)
  */
 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 drm_crtc_state *state)
 {
 	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 86d3093..3045f51 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3599,7 +3599,7 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
  * @green: green correction table
  * @blue: green correction table
  * @size: size of the tables
- * @ctx: lock acquire context
+ * @state: atomic CRTC state object to manipulate
  *
  * Implements support for legacy gamma correction table for drivers
  * that support color management through the DEGAMMA_LUT/GAMMA_LUT
@@ -3609,28 +3609,19 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
 int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 				       u16 *red, u16 *green, u16 *blue,
 				       uint32_t size,
-				       struct drm_modeset_acquire_ctx *ctx)
+				       struct drm_crtc_state *state)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_mode_config *config = &dev->mode_config;
-	struct drm_atomic_state *state;
-	struct drm_crtc_state *crtc_state;
-	struct drm_property_blob *blob = NULL;
+	struct drm_property_blob *blob;
 	struct drm_color_lut *blob_data;
 	int i, ret = 0;
 
-	state = drm_atomic_state_alloc(crtc->dev);
-	if (!state)
-		return -ENOMEM;
-
 	blob = drm_property_create_blob(dev,
 					sizeof(struct drm_color_lut) * size,
 					NULL);
-	if (IS_ERR(blob)) {
-		ret = PTR_ERR(blob);
-		blob = NULL;
-		goto fail;
-	}
+	if (IS_ERR(blob))
+		return PTR_ERR(blob);
 
 	/* Prepare GAMMA_LUT with the legacy values. */
 	blob_data = (struct drm_color_lut *) blob->data;
@@ -3640,33 +3631,21 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 		blob_data[i].blue = blue[i];
 	}
 
-	state->acquire_ctx = ctx;
-	crtc_state = drm_atomic_get_crtc_state(state, crtc);
-	if (IS_ERR(crtc_state)) {
-		ret = PTR_ERR(crtc_state);
-		goto fail;
-	}
-
 	/* Reset DEGAMMA_LUT and CTM properties. */
-	ret = drm_atomic_crtc_set_property(crtc, crtc_state,
+	ret = drm_atomic_crtc_set_property(crtc, state,
 			config->degamma_lut_property, 0);
 	if (ret)
 		goto fail;
 
-	ret = drm_atomic_crtc_set_property(crtc, crtc_state,
+	ret = drm_atomic_crtc_set_property(crtc, state,
 			config->ctm_property, 0);
 	if (ret)
 		goto fail;
 
-	ret = drm_atomic_crtc_set_property(crtc, crtc_state,
+	ret = drm_atomic_crtc_set_property(crtc, state,
 			config->gamma_lut_property, blob->base.id);
-	if (ret)
-		goto fail;
-
-	ret = drm_atomic_commit(state);
 
 fail:
-	drm_atomic_state_put(state);
 	drm_property_blob_put(blob);
 	return ret;
 }
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 3eda500..864efe8 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -21,6 +21,7 @@
  */
 
 #include <drm/drmP.h>
+#include <drm/drm_atomic.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_color_mgmt.h>
 
@@ -218,9 +219,12 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 			     void *data, struct drm_file *file_priv)
 {
 	struct drm_mode_crtc_lut *crtc_lut = data;
+	struct drm_crtc_state *crtc_state = NULL;
+	struct drm_atomic_state *state = NULL;
 	struct drm_crtc *crtc;
 	void *r_base, *g_base, *b_base;
 	int size;
+	int atomic;
 	struct drm_modeset_acquire_ctx ctx;
 	int ret = 0;
 
@@ -238,12 +242,28 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 	if (crtc_lut->gamma_size != crtc->gamma_size)
 		return -EINVAL;
 
+	atomic = drm_drv_uses_atomic_modeset(dev);
+	if (atomic) {
+		state = drm_atomic_state_alloc(dev);
+		if (!state)
+			return -ENOMEM;
+	}
 	drm_modeset_acquire_init(&ctx, 0);
 retry:
 	ret = drm_modeset_lock_all_ctx(dev, &ctx);
 	if (ret)
 		goto out;
 
+	if (atomic) {
+		state->acquire_ctx = &ctx;
+
+		crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(crtc_state)) {
+			ret = PTR_ERR(crtc_state);
+			goto out;
+		}
+	}
+
 	size = crtc_lut->gamma_size * (sizeof(uint16_t));
 	r_base = crtc->gamma_store;
 	if (copy_from_user(r_base, (void __user *)(unsigned long)crtc_lut->red, size)) {
@@ -264,7 +284,10 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 	}
 
 	ret = crtc->funcs->gamma_set(crtc, r_base, g_base, b_base,
-				     crtc->gamma_size, &ctx);
+				     crtc->gamma_size, crtc_state);
+
+	if (!ret && atomic)
+		ret = drm_atomic_commit(state);
 
 out:
 	if (ret == -EDEADLK) {
@@ -273,6 +296,8 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 	}
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
+	if (atomic)
+		drm_atomic_state_put(state);
 
 	return ret;
 
diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
index e7fd356..ccf8c33 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -178,7 +178,7 @@ void gma_crtc_load_lut(struct drm_crtc *crtc)
 
 int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue,
 		       u32 size,
-		       struct drm_modeset_acquire_ctx *ctx)
+		       struct drm_crtc_state *state)
 {
 	struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/gma500/gma_display.h b/drivers/gpu/drm/gma500/gma_display.h
index 239c374..c07b34f 100644
--- a/drivers/gpu/drm/gma500/gma_display.h
+++ b/drivers/gpu/drm/gma500/gma_display.h
@@ -74,7 +74,7 @@ extern int gma_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
 extern void gma_crtc_load_lut(struct drm_crtc *crtc);
 extern int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 			      u16 *blue, u32 size,
-			      struct drm_modeset_acquire_ctx *ctx);
+			      struct drm_crtc_state *state);
 extern void gma_crtc_dpms(struct drm_crtc *crtc, int mode);
 extern void gma_crtc_prepare(struct drm_crtc *crtc);
 extern void gma_crtc_commit(struct drm_crtc *crtc);
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index f4b5358..2141796 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1397,7 +1397,7 @@ static void mga_crtc_commit(struct drm_crtc *crtc)
  */
 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 drm_crtc_state *state)
 {
 	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 4b4b0b4..f562824 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -789,7 +789,7 @@ nv_crtc_disable(struct drm_crtc *crtc)
 static int
 nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
 		  uint32_t size,
-		  struct drm_modeset_acquire_ctx *ctx)
+		  struct drm_crtc_state *state)
 {
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index e3132a2..f13fa74 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -2232,7 +2232,7 @@ nv50_head_help = {
 static int
 nv50_head_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
 		    uint32_t size,
-		    struct drm_modeset_acquire_ctx *ctx)
+		    struct drm_crtc_state *state)
 {
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
 	u32 i;
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 17d3daf..0ea575d 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -233,7 +233,7 @@ void radeon_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
 
 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 drm_crtc_state *state)
 {
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 403bbd5..fcc8dac 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -301,7 +301,7 @@ vc4_crtc_lut_load(struct drm_crtc *crtc)
 static int
 vc4_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
 		   uint32_t size,
-		   struct drm_modeset_acquire_ctx *ctx)
+		   struct drm_crtc_state *state)
 {
 	struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
 	u32 i;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 3d94ea6..d75c5da 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1914,7 +1914,7 @@ static int vmw_du_update_layout(struct vmw_private *dev_priv, unsigned num,
 int vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
 			  u16 *r, u16 *g, u16 *b,
 			  uint32_t size,
-			  struct drm_modeset_acquire_ctx *ctx)
+			  struct drm_crtc_state *state)
 {
 	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
 	int i;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 5f8d678..9b2d8d3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -255,7 +255,7 @@ void vmw_du_crtc_restore(struct drm_crtc *crtc);
 int vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
 			   u16 *r, u16 *g, u16 *b,
 			   uint32_t size,
-			   struct drm_modeset_acquire_ctx *ctx);
+			   struct drm_crtc_state *state);
 int vmw_du_connector_set_property(struct drm_connector *connector,
 				  struct drm_property *property,
 				  uint64_t val);
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index f0a8678..a91e063 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -177,7 +177,7 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
 int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 				       u16 *red, u16 *green, u16 *blue,
 				       uint32_t size,
-				       struct drm_modeset_acquire_ctx *ctx);
+				       struct drm_crtc_state *state);
 
 /**
  * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 629a5fe..d442d30 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -368,8 +368,7 @@ struct drm_crtc_funcs {
 	 * hooks.
 	 */
 	int (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
-			 uint32_t size,
-			 struct drm_modeset_acquire_ctx *ctx);
+			 uint32_t size, struct drm_crtc_state *state);
 
 	/**
 	 * @destroy:
-- 
2.1.4

[PATCH v3 05/16] 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
completely obsolete.

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

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b75b1f2..7f8199a 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1257,27 +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;
-
-	/*
-	 * 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;
-}
-
 static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info)
 {
 	u32 *palette = (u32 *)info->pseudo_palette;
@@ -1310,54 +1289,68 @@ static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info)
 	return 0;
 }
 
-/**
- * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
- * @cmap: cmap to set
- * @info: fbdev registered by the helper
- */
-int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
+static int setcmap_legacy(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_crtc *crtc;
 	u16 *r, *g, *b;
-	int i, j, rc = 0;
-	int start;
+	int i, ret = 0;
 
-	if (oops_in_progress)
-		return -EBUSY;
+	for (i = 0; i < fb_helper->crtc_count; i++) {
+		crtc = fb_helper->crtc_info[i].mode_set.crtc;
+		if (!crtc->funcs->gamma_set || !crtc->gamma_size)
+			return -EINVAL;
 
-	mutex_lock(&fb_helper->lock);
-	if (!drm_fb_helper_is_bound(fb_helper)) {
-		mutex_unlock(&fb_helper->lock);
-		return -EBUSY;
-	}
+		if (cmap->start + cmap->len > crtc->gamma_size)
+			return -EINVAL;
 
-	drm_modeset_lock_all(dev);
-	if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
-		rc = setcmap_pseudo_palette(cmap, info);
-		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(*r));
+		memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(*g));
+		memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(*b));
+
+		ret = crtc->funcs->gamma_set(crtc, r, g, b,
+					     crtc->gamma_size, NULL);
+		if (ret)
+			return ret;
 	}
 
-	for (i = 0; i < fb_helper->crtc_count; i++) {
-		crtc = fb_helper->crtc_info[i].mode_set.crtc;
-		crtc_funcs = crtc->helper_private;
+	return ret;
+}
 
-		red = cmap->red;
-		green = cmap->green;
-		blue = cmap->blue;
-		transp = cmap->transp;
-		start = cmap->start;
+static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_device *dev = fb_helper->dev;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_crtc_state *crtc_state;
+	struct drm_atomic_state *state;
+	struct drm_crtc *crtc;
+	u16 *r, *g, *b;
+	int i, ret = 0;
 
-		if (!crtc->gamma_size) {
-			rc = -EINVAL;
+	state = drm_atomic_state_alloc(dev);
+	if (!state)
+		return -ENOMEM;
+	drm_modeset_acquire_init(&ctx, 0);
+retry:
+	ret = drm_modeset_lock_all_ctx(dev, &ctx);
+	if (ret)
+		goto fini;
+	state->acquire_ctx = &ctx;
+	for (i = 0; i < fb_helper->crtc_count; i++) {
+		crtc = fb_helper->crtc_info[i].mode_set.crtc;
+		if (!crtc->funcs->gamma_set) {
+			ret = -EINVAL;
 			goto out;
 		}
 
-		if (cmap->start + cmap->len > crtc->gamma_size) {
-			rc = -EINVAL;
+		crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(crtc_state)) {
+			ret = PTR_ERR(crtc_state);
 			goto out;
 		}
 
@@ -1369,27 +1362,57 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 		memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(*g));
 		memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(*b));
 
-		for (j = 0; j < cmap->len; j++) {
-			u16 hred, hgreen, hblue, htransp = 0xffff;
+		ret = crtc->funcs->gamma_set(crtc, r, g, b,
+					     crtc->gamma_size, crtc_state);
+		if (ret)
+			goto out;
+	}
 
-			hred = *red++;
-			hgreen = *green++;
-			hblue = *blue++;
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
 
-			if (transp)
-				htransp = *transp++;
+out:
+	drm_modeset_drop_locks(&ctx);
+fini:
+	drm_modeset_acquire_fini(&ctx);
+	drm_atomic_state_put(state);
 
-			rc = setcolreg(crtc, hred, hgreen, hblue, start++, info);
-			if (rc)
-				goto out;
-		}
-		if (crtc_funcs->load_lut)
-			crtc_funcs->load_lut(crtc);
+	return ret;
+}
+
+/**
+ * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
+ * @cmap: cmap to set
+ * @info: fbdev registered by the helper
+ */
+int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	int ret;
+
+	if (oops_in_progress)
+		return -EBUSY;
+
+	mutex_lock(&fb_helper->lock);
+
+	if (!drm_fb_helper_is_bound(fb_helper)) {
+		mutex_unlock(&fb_helper->lock);
+		return -EBUSY;
 	}
- out:
-	drm_modeset_unlock_all(dev);
+
+	if (info->fix.visual == FB_VISUAL_TRUECOLOR)
+		ret = setcmap_pseudo_palette(cmap, info);
+	else if (drm_drv_uses_atomic_modeset(fb_helper->dev))
+		ret = setcmap_atomic(cmap, info);
+	else
+		ret = setcmap_legacy(cmap, info);
+
 	mutex_unlock(&fb_helper->lock);
-	return rc;
+
+	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_setcmap);
 
-- 
2.1.4

Re: [PATCH v3 05/16] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Daniel Vetter]

On Tue, Jul 04, 2017 at 12:37:01PM +0200, Peter Rosin wrote:
> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
> completely obsolete.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 165 +++++++++++++++++++++++-----------------
>  1 file changed, 94 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index b75b1f2..7f8199a 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1257,27 +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;
> -
> -	/*
> -	 * 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;
> -}
> -
>  static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info)
>  {
>  	u32 *palette = (u32 *)info->pseudo_palette;
> @@ -1310,54 +1289,68 @@ static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info)
>  	return 0;
>  }
>  
> -/**
> - * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
> - * @cmap: cmap to set
> - * @info: fbdev registered by the helper
> - */
> -int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
> +static int setcmap_legacy(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_crtc *crtc;
>  	u16 *r, *g, *b;
> -	int i, j, rc = 0;
> -	int start;
> +	int i, ret = 0;
>  
> -	if (oops_in_progress)
> -		return -EBUSY;
> +	for (i = 0; i < fb_helper->crtc_count; i++) {
> +		crtc = fb_helper->crtc_info[i].mode_set.crtc;
> +		if (!crtc->funcs->gamma_set || !crtc->gamma_size)
> +			return -EINVAL;
>  
> -	mutex_lock(&fb_helper->lock);
> -	if (!drm_fb_helper_is_bound(fb_helper)) {
> -		mutex_unlock(&fb_helper->lock);
> -		return -EBUSY;
> -	}
> +		if (cmap->start + cmap->len > crtc->gamma_size)
> +			return -EINVAL;
>  
> -	drm_modeset_lock_all(dev);
> -	if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
> -		rc = setcmap_pseudo_palette(cmap, info);
> -		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(*r));
> +		memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(*g));
> +		memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(*b));
> +
> +		ret = crtc->funcs->gamma_set(crtc, r, g, b,
> +					     crtc->gamma_size, NULL);
> +		if (ret)
> +			return ret;
>  	}
>  
> -	for (i = 0; i < fb_helper->crtc_count; i++) {
> -		crtc = fb_helper->crtc_info[i].mode_set.crtc;
> -		crtc_funcs = crtc->helper_private;
> +	return ret;
> +}

For the legacy path you need to keep the drm_modeset_lock_all (but only in
setcmap_legacy). Otherwise this part here looks good.

>  
> -		red = cmap->red;
> -		green = cmap->green;
> -		blue = cmap->blue;
> -		transp = cmap->transp;
> -		start = cmap->start;
> +static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
> +{
> +	struct drm_fb_helper *fb_helper = info->par;
> +	struct drm_device *dev = fb_helper->dev;
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct drm_crtc_state *crtc_state;
> +	struct drm_atomic_state *state;
> +	struct drm_crtc *crtc;
> +	u16 *r, *g, *b;
> +	int i, ret = 0;
>  
> -		if (!crtc->gamma_size) {
> -			rc = -EINVAL;
> +	state = drm_atomic_state_alloc(dev);
> +	if (!state)
> +		return -ENOMEM;
> +	drm_modeset_acquire_init(&ctx, 0);
> +retry:
> +	ret = drm_modeset_lock_all_ctx(dev, &ctx);

With atomic you don't need to grab locks, this is done behind the scenes
(as long as you handle the retry/backoff correctly). See the kerneldoc for
the various drm_atomic_get_*_state functions.

> +	if (ret)
> +		goto fini;
> +	state->acquire_ctx = &ctx;
> +	for (i = 0; i < fb_helper->crtc_count; i++) {
> +		crtc = fb_helper->crtc_info[i].mode_set.crtc;
> +		if (!crtc->funcs->gamma_set) {
> +			ret = -EINVAL;
>  			goto out;
>  		}
>  
> -		if (cmap->start + cmap->len > crtc->gamma_size) {
> -			rc = -EINVAL;
> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +		if (IS_ERR(crtc_state)) {
> +			ret = PTR_ERR(crtc_state);
>  			goto out;
>  		}
>  
> @@ -1369,27 +1362,57 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>  		memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(*g));
>  		memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(*b));
>  
> -		for (j = 0; j < cmap->len; j++) {
> -			u16 hred, hgreen, hblue, htransp = 0xffff;
> +		ret = crtc->funcs->gamma_set(crtc, r, g, b,
> +					     crtc->gamma_size, crtc_state);

I guess my description of what I have in mind wasn't really clear. I think
a proper atomic commit should never reuse one of the old hooks
(->gamma_set) here, that's just confusing. Instead what I had in mind is
to do the proper adjusting that gamma_set does here in this function, i.e.

- create the new blob, fill it with the cmap data

- assign that blob to the crtc state:

	drm_atomic_replace_property_blob(&crtc_state->gamma_lut,
					 new_table, &temp);

  Note that the drm_atomic_helper_legacy_gamma_set() does that in the most
  convoluted way by going through a few layers.

- The one thing you need to do on top is check that the gamma_lut property
  is supported (just check whether dev->mode_config.gamma_lut_property
  exists). That check is instead of checking for ->gamma_set.

Checking for matching size is optional, the driver must do that already
(for the atomic property).

This way your previous patch isn't needed, and we don't need to change all
the legacy callbacks. The only downside is that we duplicate a bit of the
atomic commit setup scaffolding, but that's imo ok. You could extract that
into a helper function shared between this code here and
drm_atomic_helper_legacy_gamma_set(), but that seems frankly overkill to
me. Creating atomic commits in the kernel is simply a bit verbose, but the
benefit of the current framework is that the driver side looks a lot
simpler.

> +		if (ret)
> +			goto out;
> +	}
>  
> -			hred = *red++;
> -			hgreen = *green++;
> -			hblue = *blue++;
> +	ret = drm_atomic_commit(state);
> +	if (ret == -EDEADLK) {
> +		drm_modeset_backoff(&ctx);
> +		goto retry;
> +	}
>  
> -			if (transp)
> -				htransp = *transp++;
> +out:
> +	drm_modeset_drop_locks(&ctx);
> +fini:
> +	drm_modeset_acquire_fini(&ctx);
> +	drm_atomic_state_put(state);
>  
> -			rc = setcolreg(crtc, hred, hgreen, hblue, start++, info);
> -			if (rc)
> -				goto out;
> -		}
> -		if (crtc_funcs->load_lut)
> -			crtc_funcs->load_lut(crtc);
> +	return ret;
> +}
> +
> +/**
> + * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
> + * @cmap: cmap to set
> + * @info: fbdev registered by the helper
> + */
> +int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
> +{
> +	struct drm_fb_helper *fb_helper = info->par;
> +	int ret;
> +
> +	if (oops_in_progress)
> +		return -EBUSY;
> +
> +	mutex_lock(&fb_helper->lock);
> +
> +	if (!drm_fb_helper_is_bound(fb_helper)) {
> +		mutex_unlock(&fb_helper->lock);
> +		return -EBUSY;
>  	}
> - out:
> -	drm_modeset_unlock_all(dev);
> +
> +	if (info->fix.visual == FB_VISUAL_TRUECOLOR)
> +		ret = setcmap_pseudo_palette(cmap, info);
> +	else if (drm_drv_uses_atomic_modeset(fb_helper->dev))
> +		ret = setcmap_atomic(cmap, info);
> +	else
> +		ret = setcmap_legacy(cmap, info);
> +
>  	mutex_unlock(&fb_helper->lock);
> -	return rc;
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_fb_helper_setcmap);

Besides the 2 comments this looks good and will get my r-b once revised.

Also on patches 1-3:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Cheers, Daniel
>  
> -- 
> 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 v3 05/16] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Peter Rosin]

On 2017-07-05 08:21, Daniel Vetter wrote:
> On Tue, Jul 04, 2017 at 12:37:01PM +0200, Peter Rosin wrote:
>> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
>> completely obsolete.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>>  drivers/gpu/drm/drm_fb_helper.c | 165 +++++++++++++++++++++++-----------------
>>  1 file changed, 94 insertions(+), 71 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index b75b1f2..7f8199a 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -1257,27 +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;
>> -
>> -	/*
>> -	 * 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;
>> -}
>> -
>>  static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info)
>>  {
>>  	u32 *palette = (u32 *)info->pseudo_palette;
>> @@ -1310,54 +1289,68 @@ static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info)
>>  	return 0;
>>  }
>>  
>> -/**
>> - * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
>> - * @cmap: cmap to set
>> - * @info: fbdev registered by the helper
>> - */
>> -int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>> +static int setcmap_legacy(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_crtc *crtc;
>>  	u16 *r, *g, *b;
>> -	int i, j, rc = 0;
>> -	int start;
>> +	int i, ret = 0;
>>  
>> -	if (oops_in_progress)
>> -		return -EBUSY;
>> +	for (i = 0; i < fb_helper->crtc_count; i++) {
>> +		crtc = fb_helper->crtc_info[i].mode_set.crtc;
>> +		if (!crtc->funcs->gamma_set || !crtc->gamma_size)
>> +			return -EINVAL;
>>  
>> -	mutex_lock(&fb_helper->lock);
>> -	if (!drm_fb_helper_is_bound(fb_helper)) {
>> -		mutex_unlock(&fb_helper->lock);
>> -		return -EBUSY;
>> -	}
>> +		if (cmap->start + cmap->len > crtc->gamma_size)
>> +			return -EINVAL;
>>  
>> -	drm_modeset_lock_all(dev);
>> -	if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
>> -		rc = setcmap_pseudo_palette(cmap, info);
>> -		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(*r));
>> +		memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(*g));
>> +		memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(*b));
>> +
>> +		ret = crtc->funcs->gamma_set(crtc, r, g, b,
>> +					     crtc->gamma_size, NULL);
>> +		if (ret)
>> +			return ret;
>>  	}
>>  
>> -	for (i = 0; i < fb_helper->crtc_count; i++) {
>> -		crtc = fb_helper->crtc_info[i].mode_set.crtc;
>> -		crtc_funcs = crtc->helper_private;
>> +	return ret;
>> +}
> 
> For the legacy path you need to keep the drm_modeset_lock_all (but only in
> setcmap_legacy). Otherwise this part here looks good.

Oops, didn't intend to zap that one. Thanks for catching!

>>  
>> -		red = cmap->red;
>> -		green = cmap->green;
>> -		blue = cmap->blue;
>> -		transp = cmap->transp;
>> -		start = cmap->start;
>> +static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
>> +{
>> +	struct drm_fb_helper *fb_helper = info->par;
>> +	struct drm_device *dev = fb_helper->dev;
>> +	struct drm_modeset_acquire_ctx ctx;
>> +	struct drm_crtc_state *crtc_state;
>> +	struct drm_atomic_state *state;
>> +	struct drm_crtc *crtc;
>> +	u16 *r, *g, *b;
>> +	int i, ret = 0;
>>  
>> -		if (!crtc->gamma_size) {
>> -			rc = -EINVAL;
>> +	state = drm_atomic_state_alloc(dev);
>> +	if (!state)
>> +		return -ENOMEM;
>> +	drm_modeset_acquire_init(&ctx, 0);
>> +retry:
>> +	ret = drm_modeset_lock_all_ctx(dev, &ctx);
> 
> With atomic you don't need to grab locks, this is done behind the scenes
> (as long as you handle the retry/backoff correctly). See the kerneldoc for
> the various drm_atomic_get_*_state functions.

It doesn't work if I remove it. What is the disconnect?

>> +	if (ret)
>> +		goto fini;
>> +	state->acquire_ctx = &ctx;
>> +	for (i = 0; i < fb_helper->crtc_count; i++) {
>> +		crtc = fb_helper->crtc_info[i].mode_set.crtc;
>> +		if (!crtc->funcs->gamma_set) {
>> +			ret = -EINVAL;
>>  			goto out;
>>  		}
>>  
>> -		if (cmap->start + cmap->len > crtc->gamma_size) {
>> -			rc = -EINVAL;
>> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
>> +		if (IS_ERR(crtc_state)) {
>> +			ret = PTR_ERR(crtc_state);
>>  			goto out;
>>  		}
>>  
>> @@ -1369,27 +1362,57 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>>  		memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(*g));
>>  		memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(*b));
>>  
>> -		for (j = 0; j < cmap->len; j++) {
>> -			u16 hred, hgreen, hblue, htransp = 0xffff;
>> +		ret = crtc->funcs->gamma_set(crtc, r, g, b,
>> +					     crtc->gamma_size, crtc_state);
> 
> I guess my description of what I have in mind wasn't really clear. I think
> a proper atomic commit should never reuse one of the old hooks
> (->gamma_set) here, that's just confusing. Instead what I had in mind is
> to do the proper adjusting that gamma_set does here in this function, i.e.
> 
> - create the new blob, fill it with the cmap data
> 
> - assign that blob to the crtc state:
> 
> 	drm_atomic_replace_property_blob(&crtc_state->gamma_lut,
> 					 new_table, &temp);

That function is static, and...

>   Note that the drm_atomic_helper_legacy_gamma_set() does that in the most
>   convoluted way by going through a few layers.
> 
> - The one thing you need to do on top is check that the gamma_lut property
>   is supported (just check whether dev->mode_config.gamma_lut_property
>   exists). That check is instead of checking for ->gamma_set.

...drm_atomic_helper_legacy_gamma_set calls drm_atomic_crtc_set_property
which is already exported and performs all the checks I can think of...

> Checking for matching size is optional, the driver must do that already
> (for the atomic property).

...except this one.

> This way your previous patch isn't needed, and we don't need to change all
> the legacy callbacks. The only downside is that we duplicate a bit of the
> atomic commit setup scaffolding, but that's imo ok. You could extract that
> into a helper function shared between this code here and
> drm_atomic_helper_legacy_gamma_set(), but that seems frankly overkill to
> me. Creating atomic commits in the kernel is simply a bit verbose, but the
> benefit of the current framework is that the driver side looks a lot
> simpler.

But, yup, ditching 4/16 feels very nice. I'll mimic the code in
drm_atomic_helper_legacy_gamma_set for the v4 version of setcmap_atomic.

Good suggestion, and thanks for the more verbose explanation.

>> +		if (ret)
>> +			goto out;
>> +	}
>>  
>> -			hred = *red++;
>> -			hgreen = *green++;
>> -			hblue = *blue++;
>> +	ret = drm_atomic_commit(state);
>> +	if (ret == -EDEADLK) {
>> +		drm_modeset_backoff(&ctx);
>> +		goto retry;
>> +	}
>>  
>> -			if (transp)
>> -				htransp = *transp++;
>> +out:
>> +	drm_modeset_drop_locks(&ctx);
>> +fini:
>> +	drm_modeset_acquire_fini(&ctx);
>> +	drm_atomic_state_put(state);
>>  
>> -			rc = setcolreg(crtc, hred, hgreen, hblue, start++, info);
>> -			if (rc)
>> -				goto out;
>> -		}
>> -		if (crtc_funcs->load_lut)
>> -			crtc_funcs->load_lut(crtc);
>> +	return ret;
>> +}
>> +
>> +/**
>> + * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
>> + * @cmap: cmap to set
>> + * @info: fbdev registered by the helper
>> + */
>> +int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>> +{
>> +	struct drm_fb_helper *fb_helper = info->par;
>> +	int ret;
>> +
>> +	if (oops_in_progress)
>> +		return -EBUSY;
>> +
>> +	mutex_lock(&fb_helper->lock);
>> +
>> +	if (!drm_fb_helper_is_bound(fb_helper)) {
>> +		mutex_unlock(&fb_helper->lock);
>> +		return -EBUSY;
>>  	}
>> - out:
>> -	drm_modeset_unlock_all(dev);
>> +
>> +	if (info->fix.visual == FB_VISUAL_TRUECOLOR)
>> +		ret = setcmap_pseudo_palette(cmap, info);
>> +	else if (drm_drv_uses_atomic_modeset(fb_helper->dev))
>> +		ret = setcmap_atomic(cmap, info);
>> +	else
>> +		ret = setcmap_legacy(cmap, info);
>> +
>>  	mutex_unlock(&fb_helper->lock);
>> -	return rc;
>> +
>> +	return ret;
>>  }
>>  EXPORT_SYMBOL(drm_fb_helper_setcmap);
> 
> Besides the 2 comments this looks good and will get my r-b once revised.
> 
> Also on patches 1-3:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Excellent!

Cheers,
peda

> Cheers, Daniel
>>  
>> -- 
>> 2.1.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [PATCH v3 05/16] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Daniel Vetter]

On Wed, Jul 5, 2017 at 7:50 PM, Peter Rosin <peda@axentia.se> wrote:
>>> +retry:
>>> +    ret = drm_modeset_lock_all_ctx(dev, &ctx);
>>
>> With atomic you don't need to grab locks, this is done behind the scenes
>> (as long as you handle the retry/backoff correctly). See the kerneldoc for
>> the various drm_atomic_get_*_state functions.
>
> It doesn't work if I remove it. What is the disconnect?

Good question, didn't spot this at first, but your backoff/retry logic
is proken. When typing drm_modeset_lock locking code please make sure
you've enabled both CONFIG_PROVE_LOCKING and
CONFIG_DEBUG_WW_MUTEX_SLOWPATH. Without these two it's really easy to
get this wrong. Please also read
https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#kms-locking
carefully plus all the kernel-doc of the various hooks. This stuff is
a really tricky locking scheme, it takes a while to understand it and
implement it correctly. Which is why all the locking magic is in
shared code and for normal drivers no need think about it. For the
fundamental algorithm, you can also check out the docs for w/w mutexes
at https://www.kernel.org/doc/Documentation/locking/ww-mutex-design.txt

Might also help to read a bunch of the other locking paths again, with
my patches there's a few just in drm_fbdev_helper.c. I'll leave you
with these snippets here since I think this is fun to learn, but when
you're stuck I'm happy to help learn.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH v3 05/16] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Peter Rosin]

On 2017-07-06 07:55, Daniel Vetter wrote:
> On Wed, Jul 5, 2017 at 7:50 PM, Peter Rosin <peda@axentia.se> wrote:
>>>> +retry:
>>>> +    ret = drm_modeset_lock_all_ctx(dev, &ctx);
>>>
>>> With atomic you don't need to grab locks, this is done behind the scenes
>>> (as long as you handle the retry/backoff correctly). See the kerneldoc for
>>> the various drm_atomic_get_*_state functions.
>>
>> It doesn't work if I remove it. What is the disconnect?
> 
> Good question,

Duh, for symmetry I also removed the dropping of the locks. So the next
call of course had no chance of getting access. How silly of me...

>                didn't spot this at first, but your backoff/retry logic
> is proken. When typing drm_modeset_lock locking code please make sure
> you've enabled both CONFIG_PROVE_LOCKING and
> CONFIG_DEBUG_WW_MUTEX_SLOWPATH. Without these two it's really easy to
> get this wrong. Please also read
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#kms-locking
> carefully plus all the kernel-doc of the various hooks. This stuff is
> a really tricky locking scheme, it takes a while to understand it and
> implement it correctly. Which is why all the locking magic is in
> shared code and for normal drivers no need think about it. For the
> fundamental algorithm, you can also check out the docs for w/w mutexes
> at https://www.kernel.org/doc/Documentation/locking/ww-mutex-design.txt
> 
> Might also help to read a bunch of the other locking paths again, with
> my patches there's a few just in drm_fbdev_helper.c. I'll leave you
> with these snippets here since I think this is fun to learn, but when
> you're stuck I'm happy to help learn.

I'll take a long look at this before I send a cleaned up v4. Thanks for
the pointers...

Cheers,
peda

Re: [PATCH v3 05/16] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Daniel Vetter]

> >> @@ -1369,27 +1362,57 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
> >>  		memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(*g));
> >>  		memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(*b));
> >>  
> >> -		for (j = 0; j < cmap->len; j++) {
> >> -			u16 hred, hgreen, hblue, htransp = 0xffff;
> >> +		ret = crtc->funcs->gamma_set(crtc, r, g, b,
> >> +					     crtc->gamma_size, crtc_state);
> > 
> > I guess my description of what I have in mind wasn't really clear. I think
> > a proper atomic commit should never reuse one of the old hooks
> > (->gamma_set) here, that's just confusing. Instead what I had in mind is
> > to do the proper adjusting that gamma_set does here in this function, i.e.
> > 
> > - create the new blob, fill it with the cmap data
> > 
> > - assign that blob to the crtc state:
> > 
> > 	drm_atomic_replace_property_blob(&crtc_state->gamma_lut,
> > 					 new_table, &temp);
> 
> That function is static, and...

Missed these comments here. I think we should just make them unstatic,
that might also explain why gamma_set is going this indirect path. Means
we need some kerneldoc, but that's not much work. Ping the original author
if you have questions.

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

[PATCH v3 06/16] 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 31c977b..717be5f 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_crtc_state *state)
 {
-	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 cf42640..b554465 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_crtc_state *state)
 {
-	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 045014d..e1c9906 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_crtc_state *state)
 {
-	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 ee9389f..7fedabc 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_crtc_state *state)
 {
-	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 f194dfc..db65dec 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_crtc_state *state)
 {
-	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 v3 07/16] 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 v3 08/16] 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 6f0335b..e686072 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_crtc_state *state)
 {
-	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 v3 09/16] 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 2b4c2c3..92ff7de 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_crtc_state *state)
 {
-	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 v3 10/16] 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 ccf8c33..3bf65d4 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_crtc_state *state)
 {
-	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 v3 11/16] 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 v3 12/16] 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 2141796..dc500ef 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_crtc_state *state)
 {
-	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 v3 13/16] 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 f562824..b233412 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_crtc_state *state)
 {
 	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 f13fa74..93c66c0 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_crtc_state *state)
 {
-	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 v3 14/16] 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 0ea575d..3d7394d4 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_crtc_state *state)
 {
-	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 v3 15/16] 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 v3 16/16] 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_crtc.h                   |  8 --------
 include/drm/drm_fb_helper.h              | 32 --------------------------------
 include/drm/drm_modeset_helper_vtables.h | 16 ----------------
 3 files changed, 56 deletions(-)

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d442d30..7bb0a6e 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -358,14 +358,6 @@ struct drm_crtc_funcs {
 	 * drm_crtc_enable_color_mgmt(), which then supports the legacy gamma
 	 * interface through the drm_atomic_helper_legacy_gamma_set()
 	 * compatibility implementation.
-	 *
-	 * NOTE:
-	 *
-	 * Drivers that support gamma tables and also fbdev emulation through
-	 * the provided helper library need to take care to fill out the gamma
-	 * hooks for both. Currently there's a bit an unfortunate duplication
-	 * going on, which should eventually be unified to just one set of
-	 * hooks.
 	 */
 	int (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
 			 uint32_t size, struct drm_crtc_state *state);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index a5ea6ff..33fe959 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

Re: [Intel-gfx] [PATCH v3 00/16] improve the fb_setcmap helper

[Daniel Vetter]

On Tue, Jul 04, 2017 at 12:36:56PM +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 through 5 are enough to make the atmel-hlcdc driver
> do what I need. 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, or if I have misunderstood something
> about atomics...
> 
> Changes since v2:
> - Added patch 1/16 which factors out pseudo-palette handling.
> - Removed the if (cmap->start + cmap->len < cmap->start)
>   sanity check on the assumption that the fbdev core handles it.
> - Added patch 4/16 which factors out atomic state and commit
>   handling from drm_atomic_helper_legacy_gamma_set to
>   drm_mode_gamma_set_ioctl.
> - Do one atomic commit for all affected crtc.
> - Removed a now obsolete note in include/drm/drm_crtc.h (ammended
>   the last patch).
> - Cc list is getting long, so I have redused the list for the
>   individual patches. If you would like to get the full series
>   (or nothing at all) for the next round (if that is needed) just
>   say so.

Is this still on top of my locking rework? I tried to apply patches 1-3,
but there's minor conflicts ...
-Daniel

> 
> 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 (16):
>   drm/fb-helper: factor out pseudo-palette
>   drm/fb-helper: keep the .gamma_store updated in drm_fb_helper_setcmap
>   drm/fb-helper: remove drm_fb_helper_save_lut_atomic
>   drm/color-mgmt: move atomic state/commit out from .gamma_set
>   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      |  29 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c      |  29 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c       |  29 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c       |  29 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c    |  25 +---
>  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              |  28 +---
>  drivers/gpu/drm/cirrus/cirrus_drv.h         |   8 --
>  drivers/gpu/drm/cirrus/cirrus_fbdev.c       |   2 -
>  drivers/gpu/drm/cirrus/cirrus_mode.c        |  73 +++--------
>  drivers/gpu/drm/drm_atomic_helper.c         |  37 ++----
>  drivers/gpu/drm/drm_color_mgmt.c            |  27 +++-
>  drivers/gpu/drm/drm_fb_helper.c             | 195 +++++++++++++++++-----------
>  drivers/gpu/drm/gma500/framebuffer.c        |  22 ----
>  drivers/gpu/drm/gma500/gma_display.c        |  34 ++---
>  drivers/gpu/drm/gma500/gma_display.h        |   2 +-
>  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      |  64 +++------
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  28 ++--
>  drivers/gpu/drm/nouveau/nouveau_crtc.h      |   3 -
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  22 ----
>  drivers/gpu/drm/nouveau/nv50_display.c      |  42 ++----
>  drivers/gpu/drm/radeon/atombios_crtc.c      |   1 -
>  drivers/gpu/drm/radeon/radeon_connectors.c  |   7 +-
>  drivers/gpu/drm/radeon/radeon_display.c     |  73 +++++------
>  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 -
>  drivers/gpu/drm/vc4/vc4_crtc.c              |   2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |   2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h         |   2 +-
>  include/drm/drm_atomic_helper.h             |   2 +-
>  include/drm/drm_crtc.h                      |  11 +-
>  include/drm/drm_fb_helper.h                 |  32 -----
>  include/drm/drm_modeset_helper_vtables.h    |  16 ---
>  48 files changed, 301 insertions(+), 702 deletions(-)
> 
> -- 
> 2.1.4
> 
> _______________________________________________
> 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: [Intel-gfx] [PATCH v3 00/16] improve the fb_setcmap helper

[Peter Rosin]

On 2017-07-05 08:08, Daniel Vetter wrote:
> On Tue, Jul 04, 2017 at 12:36:56PM +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 through 5 are enough to make the atmel-hlcdc driver
>> do what I need. 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, or if I have misunderstood something
>> about atomics...
>>
>> Changes since v2:
>> - Added patch 1/16 which factors out pseudo-palette handling.
>> - Removed the if (cmap->start + cmap->len < cmap->start)
>>   sanity check on the assumption that the fbdev core handles it.
>> - Added patch 4/16 which factors out atomic state and commit
>>   handling from drm_atomic_helper_legacy_gamma_set to
>>   drm_mode_gamma_set_ioctl.
>> - Do one atomic commit for all affected crtc.
>> - Removed a now obsolete note in include/drm/drm_crtc.h (ammended
>>   the last patch).
>> - Cc list is getting long, so I have redused the list for the
>>   individual patches. If you would like to get the full series
>>   (or nothing at all) for the next round (if that is needed) just
>>   say so.
> 
> Is this still on top of my locking rework? I tried to apply patches 1-3,
> but there's minor conflicts ...
> -Daniel

v3 has the same base as v2. I collected your locking rework sometime
after june 21, you have perhaps changed things since? I saw an update
of that dpms patch you Cc me, but figured there were no significant
changes that I needed to handle since I didn't get the full set
this time either. A bad assumption it seems...

Anyway, the base I have for v3 (and v2) is linux next-20170621 plus
the following locking rework commits (in reverse order):

Author: Thierry Reding <treding@nvidia.com>
Date: Wed Jun 21 20:28:15 2017 +0200
Subject: drm/hisilicon: Remove custom FB helper deferred setup

Author: Thierry Reding <treding@nvidia.com>
Date: Wed Jun 21 20:28:14 2017 +0200
Subject: drm/exynos: Remove custom FB helper deferred setup

Author: Thierry Reding <treding@nvidia.com>
Date: Wed Jun 21 20:28:13 2017 +0200
Subject: drm/fb-helper: Support deferred setup

Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed Jun 21 20:28:12 2017 +0200
Subject: drm/fb-helper: Split dpms handling into legacy and atomic paths

Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed Jun 21 20:28:11 2017 +0200
Subject: drm/fb-helper: Stop using mode_config.mutex for internals

Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed Jun 21 20:28:10 2017 +0200
Subject: drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy

Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed Jun 21 20:28:09 2017 +0200
Subject: drm/fb-helper: Push locking into pan_display_atomic|legacy

Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed Jun 21 20:28:08 2017 +0200
Subject: drm/fb-helper: Drop locking from the vsync wait ioctl code

Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed Jun 21 20:28:07 2017 +0200
Subject: drm/fb-helper: Push locking in fb_is_bound

Author: Thierry Reding <treding@nvidia.com>
Date: Wed Jun 21 20:28:06 2017 +0200
Subject: drm/fb-helper: Add top-level lock

Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed Jun 21 20:28:05 2017 +0200
Subject: drm/i915: Drop FBDEV #ifdev in mst code

Author: Thierry Reding <treding@nvidia.com>
Date: Wed Jun 21 20:28:04 2017 +0200
Subject: drm/fb-helper: Push down modeset lock into FB helpers

Cheers,
peda

Re: [Intel-gfx] [PATCH v3 00/16] improve the fb_setcmap helper

[Daniel Vetter]

On Wed, Jul 05, 2017 at 10:09:21AM +0200, Peter Rosin wrote:
> On 2017-07-05 08:08, Daniel Vetter wrote:
> > On Tue, Jul 04, 2017 at 12:36:56PM +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 through 5 are enough to make the atmel-hlcdc driver
> >> do what I need. 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, or if I have misunderstood something
> >> about atomics...
> >>
> >> Changes since v2:
> >> - Added patch 1/16 which factors out pseudo-palette handling.
> >> - Removed the if (cmap->start + cmap->len < cmap->start)
> >>   sanity check on the assumption that the fbdev core handles it.
> >> - Added patch 4/16 which factors out atomic state and commit
> >>   handling from drm_atomic_helper_legacy_gamma_set to
> >>   drm_mode_gamma_set_ioctl.
> >> - Do one atomic commit for all affected crtc.
> >> - Removed a now obsolete note in include/drm/drm_crtc.h (ammended
> >>   the last patch).
> >> - Cc list is getting long, so I have redused the list for the
> >>   individual patches. If you would like to get the full series
> >>   (or nothing at all) for the next round (if that is needed) just
> >>   say so.
> > 
> > Is this still on top of my locking rework? I tried to apply patches 1-3,
> > but there's minor conflicts ...
> > -Daniel
> 
> v3 has the same base as v2. I collected your locking rework sometime
> after june 21, you have perhaps changed things since? I saw an update
> of that dpms patch you Cc me, but figured there were no significant
> changes that I needed to handle since I didn't get the full set
> this time either. A bad assumption it seems...

There's a difference between my own private patches, and the maintainer
repo where stuff gets applied. But that explains why there was a conflict.

I plan to merge my locking changes tomorrow (they're reviewed and ready
now), I'll apply your patches after that. That should take care of the
conflicts.

Thanks, Daniel

> 
> Anyway, the base I have for v3 (and v2) is linux next-20170621 plus
> the following locking rework commits (in reverse order):
> 
> Author: Thierry Reding <treding@nvidia.com>
> Date: Wed Jun 21 20:28:15 2017 +0200
> Subject: drm/hisilicon: Remove custom FB helper deferred setup
> 
> Author: Thierry Reding <treding@nvidia.com>
> Date: Wed Jun 21 20:28:14 2017 +0200
> Subject: drm/exynos: Remove custom FB helper deferred setup
> 
> Author: Thierry Reding <treding@nvidia.com>
> Date: Wed Jun 21 20:28:13 2017 +0200
> Subject: drm/fb-helper: Support deferred setup
> 
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date: Wed Jun 21 20:28:12 2017 +0200
> Subject: drm/fb-helper: Split dpms handling into legacy and atomic paths
> 
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date: Wed Jun 21 20:28:11 2017 +0200
> Subject: drm/fb-helper: Stop using mode_config.mutex for internals
> 
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date: Wed Jun 21 20:28:10 2017 +0200
> Subject: drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy
> 
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date: Wed Jun 21 20:28:09 2017 +0200
> Subject: drm/fb-helper: Push locking into pan_display_atomic|legacy
> 
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date: Wed Jun 21 20:28:08 2017 +0200
> Subject: drm/fb-helper: Drop locking from the vsync wait ioctl code
> 
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date: Wed Jun 21 20:28:07 2017 +0200
> Subject: drm/fb-helper: Push locking in fb_is_bound
> 
> Author: Thierry Reding <treding@nvidia.com>
> Date: Wed Jun 21 20:28:06 2017 +0200
> Subject: drm/fb-helper: Add top-level lock
> 
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date: Wed Jun 21 20:28:05 2017 +0200
> Subject: drm/i915: Drop FBDEV #ifdev in mst code
> 
> Author: Thierry Reding <treding@nvidia.com>
> Date: Wed Jun 21 20:28:04 2017 +0200
> Subject: drm/fb-helper: Push down modeset lock into FB helpers
> 
> Cheers,
> peda
> _______________________________________________
> 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