All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 00/14] improve the fb_setcmap helper
@ 2017-07-13 16:25 ` Peter Rosin
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Alex Deucher, Christian König, David Airlie,
	Russell King, Dave Airlie, Gerd Hoffmann, Daniel Vetter,
	Jani Nikula, Sean Paul, Patrik Jakobsson, Ben Skeggs,
	Yannick Fertre, Philippe Cornu, Benjamin Gaignard,
	Vincent Abriou, amd-gfx, dri-devel, virtualization, intel-gfx,
	nouveau, Lionel Landwerlin, Boris Brezillon

Hi!

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

I have obviously not tested all of this with more than a compile,
but patches 1 and 3 are enough to make the atmel-hlcdc driver
do what I need. The rest is just lots of removals and cleanup made
possible by the other improvements.

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 v5:
- Rebased onto fresher drm-misc-next.
- Instead of just exporting drm_atomic_replace_propery_blob(), move
  it to drm_propery.c, rename it to drm_property_replace_blob() and
  change its semantics to return if the blob was replaced.
- Install the same gamma_lut blob for all crtc, regardless of any
  variation in the gamma_lut state for the individual crtc prior to
  the fbdev setcmap call.
- Add acks from Daniel for patches 4-14.

Changes since v3:
- Rebased onto drm-misc-next and dropped patches 1-3 from v3, since
  they are already merged.
- Dropped the v3 patch 4/16 ("drm/color-mgmt: move atomic state/commit
  out from .gamma_set") since the atomic setcmap no longer uses
  the crtc .gamma_set callback.
- Added patch 1/14 which exports drm_atomic_replace_property_blob...
- ...and patch 2/14 which uses this new export to simplify
  drm_atomic_helper_legacy_gamma_set.
- Big changes to patch 3/14 (was 5/16 in v3). It had various locking
  issues and the atomic setcmap is rather different.

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 (14):
  drm: rename, adjust and export drm_atomic_replace_property_blob
  drm/atomic-helper: update lut props directly in ..._legacy_gamma_set
  drm/fb-helper: separate the fb_setcmap helper into atomic and legacy
    paths
  drm: amd: remove dead code and pointless local lut storage
  drm: armada: remove dead empty functions
  drm: ast: remove dead code and pointless local lut storage
  drm: cirrus: remove dead code and pointless local lut storage
  drm: gma500: remove dead code and pointless local lut storage
  drm: i915: remove dead code and pointless local lut storage
  drm: mgag200: remove dead code and pointless local lut storage
  drm: nouveau: remove dead code and pointless local lut storage
  drm: radeon: remove dead code and pointless local lut storage
  drm: stm: remove dead code and pointless local lut storage
  drm: remove unused and redundant callbacks

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

-- 
2.11.0

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v5 00/14] improve the fb_setcmap helper
@ 2017-07-13 16:25 ` Peter Rosin
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Alex Deucher, Christian König, David Airlie,
	Russell King, Dave Airlie, Gerd Hoffmann, Daniel Vetter,
	Jani Nikula, Sean Paul, Patrik Jakobsson, Ben Skeggs,
	Yannick Fertre, Philippe Cornu, Benjamin Gaignard,
	Vincent Abriou, amd-gfx, dri-devel, virtualization

Hi!

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

I have obviously not tested all of this with more than a compile,
but patches 1 and 3 are enough to make the atmel-hlcdc driver
do what I need. The rest is just lots of removals and cleanup made
possible by the other improvements.

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 v5:
- Rebased onto fresher drm-misc-next.
- Instead of just exporting drm_atomic_replace_propery_blob(), move
  it to drm_propery.c, rename it to drm_property_replace_blob() and
  change its semantics to return if the blob was replaced.
- Install the same gamma_lut blob for all crtc, regardless of any
  variation in the gamma_lut state for the individual crtc prior to
  the fbdev setcmap call.
- Add acks from Daniel for patches 4-14.

Changes since v3:
- Rebased onto drm-misc-next and dropped patches 1-3 from v3, since
  they are already merged.
- Dropped the v3 patch 4/16 ("drm/color-mgmt: move atomic state/commit
  out from .gamma_set") since the atomic setcmap no longer uses
  the crtc .gamma_set callback.
- Added patch 1/14 which exports drm_atomic_replace_property_blob...
- ...and patch 2/14 which uses this new export to simplify
  drm_atomic_helper_legacy_gamma_set.
- Big changes to patch 3/14 (was 5/16 in v3). It had various locking
  issues and the atomic setcmap is rather different.

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 (14):
  drm: rename, adjust and export drm_atomic_replace_property_blob
  drm/atomic-helper: update lut props directly in ..._legacy_gamma_set
  drm/fb-helper: separate the fb_setcmap helper into atomic and legacy
    paths
  drm: amd: remove dead code and pointless local lut storage
  drm: armada: remove dead empty functions
  drm: ast: remove dead code and pointless local lut storage
  drm: cirrus: remove dead code and pointless local lut storage
  drm: gma500: remove dead code and pointless local lut storage
  drm: i915: remove dead code and pointless local lut storage
  drm: mgag200: remove dead code and pointless local lut storage
  drm: nouveau: remove dead code and pointless local lut storage
  drm: radeon: remove dead code and pointless local lut storage
  drm: stm: remove dead code and pointless local lut storage
  drm: remove unused and redundant callbacks

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

-- 
2.11.0

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v5 01/14] drm: rename, adjust and export drm_atomic_replace_property_blob
  2017-07-13 16:25 ` Peter Rosin
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Daniel Vetter, Jani Nikula, Sean Paul, David Airlie,
	dri-devel, Lionel Landwerlin, Boris Brezillon

The function has little to do with atomic, it's just where it has so
far been needed. So, rename it to drm_property_replace_blob, move it
to drm_property.c and export it.

Change the semantics to return whether the blob was replaced instead
of using an extra argument for that.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/drm_atomic.c   | 30 +-----------------------------
 drivers/gpu/drm/drm_property.c | 23 +++++++++++++++++++++++
 include/drm/drm_property.h     |  2 ++
 3 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 09ca662fcd35..16d73be6f76d 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -408,34 +408,6 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
 }
 EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
 
-/**
- * drm_atomic_replace_property_blob - replace a blob property
- * @blob: a pointer to the member blob to be replaced
- * @new_blob: the new blob to replace with
- * @replaced: whether the blob has been replaced
- *
- * RETURNS:
- * Zero on success, error code on failure
- */
-static void
-drm_atomic_replace_property_blob(struct drm_property_blob **blob,
-				 struct drm_property_blob *new_blob,
-				 bool *replaced)
-{
-	struct drm_property_blob *old_blob = *blob;
-
-	if (old_blob == new_blob)
-		return;
-
-	drm_property_blob_put(old_blob);
-	if (new_blob)
-		drm_property_blob_get(new_blob);
-	*blob = new_blob;
-	*replaced = true;
-
-	return;
-}
-
 static int
 drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
 					 struct drm_property_blob **blob,
@@ -456,7 +428,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
 		}
 	}
 
-	drm_atomic_replace_property_blob(blob, new_blob, replaced);
+	*replaced |= drm_property_replace_blob(blob, new_blob);
 	drm_property_blob_put(new_blob);
 
 	return 0;
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index 3e88fa24eab3..bc5128203056 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -709,6 +709,29 @@ int drm_property_replace_global_blob(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_property_replace_global_blob);
 
+/**
+ * drm_property_replace_blob - replace a blob property
+ * @blob: a pointer to the member blob to be replaced
+ * @new_blob: the new blob to replace with
+ *
+ * Return: true if the blob was in fact replaced.
+ */
+bool drm_property_replace_blob(struct drm_property_blob **blob,
+			       struct drm_property_blob *new_blob)
+{
+	struct drm_property_blob *old_blob = *blob;
+
+	if (old_blob == new_blob)
+		return false;
+
+	drm_property_blob_put(old_blob);
+	if (new_blob)
+		drm_property_blob_get(new_blob);
+	*blob = new_blob;
+	return true;
+}
+EXPORT_SYMBOL(drm_property_replace_blob);
+
 int drm_mode_getblob_ioctl(struct drm_device *dev,
 			   void *data, struct drm_file *file_priv)
 {
diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
index 619868dc08d8..37355c623e6c 100644
--- a/include/drm/drm_property.h
+++ b/include/drm/drm_property.h
@@ -273,6 +273,8 @@ int drm_property_replace_global_blob(struct drm_device *dev,
 				     const void *data,
 				     struct drm_mode_object *obj_holds_id,
 				     struct drm_property *prop_holds_id);
+bool drm_property_replace_blob(struct drm_property_blob **blob,
+			       struct drm_property_blob *new_blob);
 struct drm_property_blob *drm_property_blob_get(struct drm_property_blob *blob);
 void drm_property_blob_put(struct drm_property_blob *blob);
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 02/14] drm/atomic-helper: update lut props directly in ..._legacy_gamma_set
  2017-07-13 16:25 ` Peter Rosin
  (?)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Daniel Vetter, Jani Nikula, Sean Paul, David Airlie,
	dri-devel, Lionel Landwerlin, Boris Brezillon

Do not waste cycles looking up the property id when we have the
actual property already.

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

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 0fed20692df4..a8f19127c29a 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3769,12 +3769,12 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 				       struct drm_modeset_acquire_ctx *ctx)
 {
 	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_color_lut *blob_data;
 	int i, ret = 0;
+	bool replaced;
 
 	state = drm_atomic_state_alloc(crtc->dev);
 	if (!state)
@@ -3805,20 +3805,10 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 	}
 
 	/* Reset DEGAMMA_LUT and CTM properties. */
-	ret = drm_atomic_crtc_set_property(crtc, crtc_state,
-			config->degamma_lut_property, 0);
-	if (ret)
-		goto fail;
-
-	ret = drm_atomic_crtc_set_property(crtc, crtc_state,
-			config->ctm_property, 0);
-	if (ret)
-		goto fail;
-
-	ret = drm_atomic_crtc_set_property(crtc, crtc_state,
-			config->gamma_lut_property, blob->base.id);
-	if (ret)
-		goto fail;
+	replaced  = drm_property_replace_blob(&crtc_state->degamma_lut, NULL);
+	replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
+	replaced |= drm_property_replace_blob(&crtc_state->gamma_lut, blob);
+	crtc_state->color_mgmt_changed |= replaced;
 
 	ret = drm_atomic_commit(state);
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 03/14] drm/fb-helper: separate the fb_setcmap helper into atomic and legacy paths
  2017-07-13 16:25 ` Peter Rosin
                   ` (2 preceding siblings ...)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  2017-07-14 13:54   ` Daniel Vetter
  -1 siblings, 1 reply; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Daniel Vetter, Jani Nikula, Sean Paul, David Airlie,
	dri-devel, Lionel Landwerlin, Boris Brezillon

The legacy path implements setcmap in terms of crtc .gamma_set.

The atomic path implements setcmap by directly updating the crtc gamma_lut
property.

This has a couple of benefits:
- it makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
  completely obsolete. They are now unused and subject for removal.
- atomic drivers that support clut modes get fbdev support for those from
  the drm core. This includes atmel-hlcdc, but perhaps others as well?

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

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 721511da4de6..42090fe00ef9 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1195,27 +1195,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;
@@ -1248,57 +1227,138 @@ 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;
+	drm_modeset_lock_all(fb_helper->dev);
+	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;
+
+		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;
 	}
+	drm_modeset_unlock_all(fb_helper->dev);
 
-	drm_modeset_lock_all(dev);
-	if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
-		rc = setcmap_pseudo_palette(cmap, info);
-		goto out;
+	return ret;
+}
+
+static struct drm_property_blob *setcmap_new_gamma_lut(struct drm_crtc *crtc,
+						       struct fb_cmap *cmap)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_property_blob *gamma_lut;
+	struct drm_color_lut *lut;
+	int size = crtc->gamma_size;
+	int i;
+
+	if (!size || cmap->start + cmap->len > size)
+		return ERR_PTR(-EINVAL);
+
+	gamma_lut = drm_property_create_blob(dev, sizeof(*lut) * size, NULL);
+	if (IS_ERR(gamma_lut))
+		return gamma_lut;
+
+	lut = (struct drm_color_lut *)gamma_lut->data;
+	if (cmap->start || cmap->len != size) {
+		u16 *r = crtc->gamma_store;
+		u16 *g = r + crtc->gamma_size;
+		u16 *b = g + crtc->gamma_size;
+
+		for (i = 0; i < cmap->start; i++) {
+			lut[i].red = r[i];
+			lut[i].green = g[i];
+			lut[i].blue = b[i];
+		}
+		for (i = cmap->start + cmap->len; i < size; i++) {
+			lut[i].red = r[i];
+			lut[i].green = g[i];
+			lut[i].blue = b[i];
+		}
+	}
+
+	for (i = 0; i < cmap->len; i++) {
+		lut[cmap->start + i].red = cmap->red[i];
+		lut[cmap->start + i].green = cmap->green[i];
+		lut[cmap->start + i].blue = cmap->blue[i];
+	}
+
+	return gamma_lut;
+}
+
+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_property_blob *gamma_lut = NULL;
+	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;
+	bool replaced;
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	state = drm_atomic_state_alloc(dev);
+	if (!state) {
+		ret = -ENOMEM;
+		goto out_ctx;
 	}
 
+	state->acquire_ctx = &ctx;
+retry:
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		crtc = fb_helper->crtc_info[i].mode_set.crtc;
-		crtc_funcs = crtc->helper_private;
 
-		red = cmap->red;
-		green = cmap->green;
-		blue = cmap->blue;
-		transp = cmap->transp;
-		start = cmap->start;
-
-		if (!crtc->gamma_size) {
-			rc = -EINVAL;
-			goto out;
+		if (!gamma_lut)
+			gamma_lut = setcmap_new_gamma_lut(crtc, cmap);
+		if (IS_ERR(gamma_lut)) {
+			ret = PTR_ERR(gamma_lut);
+			gamma_lut = NULL;
+			goto out_state;
 		}
 
-		if (cmap->start + cmap->len > crtc->gamma_size) {
-			rc = -EINVAL;
-			goto out;
+		crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(crtc_state)) {
+			ret = PTR_ERR(crtc_state);
+			goto out_state;
 		}
 
+		replaced  = drm_property_replace_blob(&crtc_state->degamma_lut,
+						      NULL);
+		replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
+		replaced |= drm_property_replace_blob(&crtc_state->gamma_lut,
+						      gamma_lut);
+		crtc_state->color_mgmt_changed |= replaced;
+	}
+
+	ret = drm_atomic_commit(state);
+	if (ret)
+		goto out_state;
+
+	for (i = 0; i < fb_helper->crtc_count; i++) {
+		crtc = fb_helper->crtc_info[i].mode_set.crtc;
+
 		r = crtc->gamma_store;
 		g = r + crtc->gamma_size;
 		b = g + crtc->gamma_size;
@@ -1306,28 +1366,57 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 		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));
+	}
+
+out_state:
+	if (ret == -EDEADLK)
+		goto backoff;
+
+	drm_property_blob_put(gamma_lut);
+	drm_atomic_state_put(state);
+out_ctx:
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
 
-		for (j = 0; j < cmap->len; j++) {
-			u16 hred, hgreen, hblue, htransp = 0xffff;
+	return ret;
 
-			hred = *red++;
-			hgreen = *green++;
-			hblue = *blue++;
+backoff:
+	drm_atomic_state_clear(state);
+	drm_modeset_backoff(&ctx);
+	goto retry;
+}
 
-			if (transp)
-				htransp = *transp++;
+/**
+ * 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;
 
-			rc = setcolreg(crtc, hred, hgreen, hblue, start++, info);
-			if (rc)
-				goto out;
-		}
-		if (crtc_funcs->load_lut)
-			crtc_funcs->load_lut(crtc);
+	if (oops_in_progress)
+		return -EBUSY;
+
+	mutex_lock(&fb_helper->lock);
+
+	if (!drm_fb_helper_is_bound(fb_helper)) {
+		ret = -EBUSY;
+		goto out;
 	}
- 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);
+
+out:
 	mutex_unlock(&fb_helper->lock);
-	return rc;
+
+	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_setcmap);
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 04/14] drm: amd: remove dead code and pointless local lut storage
  2017-07-13 16:25 ` Peter Rosin
                   ` (3 preceding siblings ...)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  2017-07-14 14:06     ` Alex Deucher
  -1 siblings, 1 reply; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Alex Deucher, Christian König, David Airlie,
	Daniel Vetter, amd-gfx, dri-devel, Daniel Vetter, Jani Nikula,
	Sean Paul, Lionel Landwerlin, Boris Brezillon

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.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 c0d8c6ff6380..7dc378013a42 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 43a9d3aec6c4..39f7eda6091e 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 9f78c03a2e31..c9580235e35b 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2267,6 +2267,7 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc *crtc)
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct amdgpu_device *adev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 	u32 tmp;
 
@@ -2304,11 +2305,14 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
 
 	WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
-		       (amdgpu_crtc->lut_r[i] << 20) |
-		       (amdgpu_crtc->lut_g[i] << 10) |
-		       (amdgpu_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 
 	tmp = RREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset);
@@ -2624,15 +2628,6 @@ static int dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				    u16 *blue, uint32_t size,
 				    struct drm_modeset_acquire_ctx *ctx)
 {
-	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		amdgpu_crtc->lut_r[i] = red[i] >> 6;
-		amdgpu_crtc->lut_g[i] = green[i] >> 6;
-		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
-	}
 	dce_v10_0_crtc_load_lut(crtc);
 
 	return 0;
@@ -2844,14 +2839,12 @@ static const struct drm_crtc_helper_funcs dce_v10_0_crtc_helper_funcs = {
 	.mode_set_base_atomic = dce_v10_0_crtc_set_base_atomic,
 	.prepare = dce_v10_0_crtc_prepare,
 	.commit = dce_v10_0_crtc_commit,
-	.load_lut = dce_v10_0_crtc_load_lut,
 	.disable = dce_v10_0_crtc_disable,
 };
 
 static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int index)
 {
 	struct amdgpu_crtc *amdgpu_crtc;
-	int i;
 
 	amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
 			      (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
@@ -2869,12 +2862,6 @@ static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int index)
 	adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
 	adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
 
-	for (i = 0; i < 256; i++) {
-		amdgpu_crtc->lut_r[i] = i << 2;
-		amdgpu_crtc->lut_g[i] = i << 2;
-		amdgpu_crtc->lut_b[i] = i << 2;
-	}
-
 	switch (amdgpu_crtc->crtc_id) {
 	case 0:
 	default:
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 4bcf01dc567a..7e14f532df59 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -2251,6 +2251,7 @@ static void dce_v11_0_crtc_load_lut(struct drm_crtc *crtc)
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct amdgpu_device *adev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 	u32 tmp;
 
@@ -2282,11 +2283,14 @@ static void dce_v11_0_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
 
 	WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
-		       (amdgpu_crtc->lut_r[i] << 20) |
-		       (amdgpu_crtc->lut_g[i] << 10) |
-		       (amdgpu_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 
 	tmp = RREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset);
@@ -2644,15 +2648,6 @@ static int dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				    u16 *blue, uint32_t size,
 				    struct drm_modeset_acquire_ctx *ctx)
 {
-	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		amdgpu_crtc->lut_r[i] = red[i] >> 6;
-		amdgpu_crtc->lut_g[i] = green[i] >> 6;
-		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
-	}
 	dce_v11_0_crtc_load_lut(crtc);
 
 	return 0;
@@ -2892,14 +2887,12 @@ static const struct drm_crtc_helper_funcs dce_v11_0_crtc_helper_funcs = {
 	.mode_set_base_atomic = dce_v11_0_crtc_set_base_atomic,
 	.prepare = dce_v11_0_crtc_prepare,
 	.commit = dce_v11_0_crtc_commit,
-	.load_lut = dce_v11_0_crtc_load_lut,
 	.disable = dce_v11_0_crtc_disable,
 };
 
 static int dce_v11_0_crtc_init(struct amdgpu_device *adev, int index)
 {
 	struct amdgpu_crtc *amdgpu_crtc;
-	int i;
 
 	amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
 			      (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
@@ -2917,12 +2910,6 @@ static int dce_v11_0_crtc_init(struct amdgpu_device *adev, int index)
 	adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
 	adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
 
-	for (i = 0; i < 256; i++) {
-		amdgpu_crtc->lut_r[i] = i << 2;
-		amdgpu_crtc->lut_g[i] = i << 2;
-		amdgpu_crtc->lut_b[i] = i << 2;
-	}
-
 	switch (amdgpu_crtc->crtc_id) {
 	case 0:
 	default:
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index fd134a4629d7..d773b50afa60 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -2182,6 +2182,7 @@ static void dce_v6_0_crtc_load_lut(struct drm_crtc *crtc)
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct amdgpu_device *adev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 
 	DRM_DEBUG_KMS("%d\n", amdgpu_crtc->crtc_id);
@@ -2211,11 +2212,14 @@ static void dce_v6_0_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
 
 	WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
-		       (amdgpu_crtc->lut_r[i] << 20) |
-		       (amdgpu_crtc->lut_g[i] << 10) |
-		       (amdgpu_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 
 	WREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset,
@@ -2496,15 +2500,6 @@ static int dce_v6_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				   u16 *blue, uint32_t size,
 				   struct drm_modeset_acquire_ctx *ctx)
 {
-	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		amdgpu_crtc->lut_r[i] = red[i] >> 6;
-		amdgpu_crtc->lut_g[i] = green[i] >> 6;
-		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
-	}
 	dce_v6_0_crtc_load_lut(crtc);
 
 	return 0;
@@ -2712,14 +2707,12 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
 	.mode_set_base_atomic = dce_v6_0_crtc_set_base_atomic,
 	.prepare = dce_v6_0_crtc_prepare,
 	.commit = dce_v6_0_crtc_commit,
-	.load_lut = dce_v6_0_crtc_load_lut,
 	.disable = dce_v6_0_crtc_disable,
 };
 
 static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
 {
 	struct amdgpu_crtc *amdgpu_crtc;
-	int i;
 
 	amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
 			      (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
@@ -2737,12 +2730,6 @@ static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
 	adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
 	adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
 
-	for (i = 0; i < 256; i++) {
-		amdgpu_crtc->lut_r[i] = i << 2;
-		amdgpu_crtc->lut_g[i] = i << 2;
-		amdgpu_crtc->lut_b[i] = i << 2;
-	}
-
 	amdgpu_crtc->crtc_offset = crtc_offsets[amdgpu_crtc->crtc_id];
 
 	amdgpu_crtc->pll_id = ATOM_PPLL_INVALID;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index a9e869554627..4eb63f6a41ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -2124,6 +2124,7 @@ static void dce_v8_0_crtc_load_lut(struct drm_crtc *crtc)
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct amdgpu_device *adev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 
 	DRM_DEBUG_KMS("%d\n", amdgpu_crtc->crtc_id);
@@ -2153,11 +2154,14 @@ static void dce_v8_0_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
 
 	WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
-		       (amdgpu_crtc->lut_r[i] << 20) |
-		       (amdgpu_crtc->lut_g[i] << 10) |
-		       (amdgpu_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 
 	WREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset,
@@ -2475,15 +2479,6 @@ static int dce_v8_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				   u16 *blue, uint32_t size,
 				   struct drm_modeset_acquire_ctx *ctx)
 {
-	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		amdgpu_crtc->lut_r[i] = red[i] >> 6;
-		amdgpu_crtc->lut_g[i] = green[i] >> 6;
-		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
-	}
 	dce_v8_0_crtc_load_lut(crtc);
 
 	return 0;
@@ -2702,14 +2697,12 @@ static const struct drm_crtc_helper_funcs dce_v8_0_crtc_helper_funcs = {
 	.mode_set_base_atomic = dce_v8_0_crtc_set_base_atomic,
 	.prepare = dce_v8_0_crtc_prepare,
 	.commit = dce_v8_0_crtc_commit,
-	.load_lut = dce_v8_0_crtc_load_lut,
 	.disable = dce_v8_0_crtc_disable,
 };
 
 static int dce_v8_0_crtc_init(struct amdgpu_device *adev, int index)
 {
 	struct amdgpu_crtc *amdgpu_crtc;
-	int i;
 
 	amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
 			      (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
@@ -2727,12 +2720,6 @@ static int dce_v8_0_crtc_init(struct amdgpu_device *adev, int index)
 	adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
 	adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
 
-	for (i = 0; i < 256; i++) {
-		amdgpu_crtc->lut_r[i] = i << 2;
-		amdgpu_crtc->lut_g[i] = i << 2;
-		amdgpu_crtc->lut_b[i] = i << 2;
-	}
-
 	amdgpu_crtc->crtc_offset = crtc_offsets[amdgpu_crtc->crtc_id];
 
 	amdgpu_crtc->pll_id = ATOM_PPLL_INVALID;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index 90bb08309a53..ecf34bc77a63 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -168,16 +168,6 @@ static int dce_virtual_crtc_gamma_set(struct drm_crtc *crtc, u16 *red,
 				      u16 *green, u16 *blue, uint32_t size,
 				      struct drm_modeset_acquire_ctx *ctx)
 {
-	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		amdgpu_crtc->lut_r[i] = red[i] >> 6;
-		amdgpu_crtc->lut_g[i] = green[i] >> 6;
-		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
-	}
-
 	return 0;
 }
 
@@ -289,11 +279,6 @@ static int dce_virtual_crtc_set_base(struct drm_crtc *crtc, int x, int y,
 	return 0;
 }
 
-static void dce_virtual_crtc_load_lut(struct drm_crtc *crtc)
-{
-	return;
-}
-
 static int dce_virtual_crtc_set_base_atomic(struct drm_crtc *crtc,
 					 struct drm_framebuffer *fb,
 					 int x, int y, enum mode_set_atomic state)
@@ -309,14 +294,12 @@ static const struct drm_crtc_helper_funcs dce_virtual_crtc_helper_funcs = {
 	.mode_set_base_atomic = dce_virtual_crtc_set_base_atomic,
 	.prepare = dce_virtual_crtc_prepare,
 	.commit = dce_virtual_crtc_commit,
-	.load_lut = dce_virtual_crtc_load_lut,
 	.disable = dce_virtual_crtc_disable,
 };
 
 static int dce_virtual_crtc_init(struct amdgpu_device *adev, int index)
 {
 	struct amdgpu_crtc *amdgpu_crtc;
-	int i;
 
 	amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
 			      (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
@@ -329,12 +312,6 @@ static int dce_virtual_crtc_init(struct amdgpu_device *adev, int index)
 	amdgpu_crtc->crtc_id = index;
 	adev->mode_info.crtcs[index] = amdgpu_crtc;
 
-	for (i = 0; i < 256; i++) {
-		amdgpu_crtc->lut_r[i] = i << 2;
-		amdgpu_crtc->lut_g[i] = i << 2;
-		amdgpu_crtc->lut_b[i] = i << 2;
-	}
-
 	amdgpu_crtc->pll_id = ATOM_PPLL_INVALID;
 	amdgpu_crtc->encoder = NULL;
 	amdgpu_crtc->connector = NULL;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 05/14] drm: armada: remove dead empty functions
  2017-07-13 16:25 ` Peter Rosin
                   ` (4 preceding siblings ...)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Russell King, David Airlie, dri-devel,
	Daniel Vetter, Jani Nikula, Sean Paul, Lionel Landwerlin,
	Boris Brezillon

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

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 b57fb80acec1..5d5cc3289a81 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 7e8906d3ae26..bab11f483575 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 602dfead8eef..5fa076d6fabc 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.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 06/14] drm: ast: remove dead code and pointless local lut storage
  2017-07-13 16:25 ` Peter Rosin
                   ` (5 preceding siblings ...)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Dave Airlie, David Airlie, Daniel Vetter, dri-devel,
	Daniel Vetter, Jani Nikula, Sean Paul, Lionel Landwerlin,
	Boris Brezillon

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.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 8880f0b62e9c..569a1484d523 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 4ad4acd0ccab..dbabcaca6835 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 aaef0a652f10..724c16bb6a62 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -63,15 +63,18 @@ static inline void ast_load_palette_index(struct ast_private *ast,
 static void ast_crtc_load_lut(struct drm_crtc *crtc)
 {
 	struct ast_private *ast = crtc->dev->dev_private;
-	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
+	u16 *r, *g, *b;
 	int i;
 
 	if (!crtc->enabled)
 		return;
 
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
+
 	for (i = 0; i < 256; i++)
-		ast_load_palette_index(ast, i, ast_crtc->lut_r[i],
-				       ast_crtc->lut_g[i], ast_crtc->lut_b[i]);
+		ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ >> 8);
 }
 
 static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mode *mode,
@@ -633,7 +636,6 @@ static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
 	.mode_set = ast_crtc_mode_set,
 	.mode_set_base = ast_crtc_mode_set_base,
 	.disable = ast_crtc_disable,
-	.load_lut = ast_crtc_load_lut,
 	.prepare = ast_crtc_prepare,
 	.commit = ast_crtc_commit,
 
@@ -648,15 +650,6 @@ static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 			      u16 *blue, uint32_t size,
 			      struct drm_modeset_acquire_ctx *ctx)
 {
-	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		ast_crtc->lut_r[i] = red[i] >> 8;
-		ast_crtc->lut_g[i] = green[i] >> 8;
-		ast_crtc->lut_b[i] = blue[i] >> 8;
-	}
 	ast_crtc_load_lut(crtc);
 
 	return 0;
@@ -681,7 +674,6 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
 static int ast_crtc_init(struct drm_device *dev)
 {
 	struct ast_crtc *crtc;
-	int i;
 
 	crtc = kzalloc(sizeof(struct ast_crtc), GFP_KERNEL);
 	if (!crtc)
@@ -690,12 +682,6 @@ static int ast_crtc_init(struct drm_device *dev)
 	drm_crtc_init(dev, &crtc->base, &ast_crtc_funcs);
 	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
 	drm_crtc_helper_add(&crtc->base, &ast_crtc_helper_funcs);
-
-	for (i = 0; i < 256; i++) {
-		crtc->lut_r[i] = i;
-		crtc->lut_g[i] = i;
-		crtc->lut_b[i] = i;
-	}
 	return 0;
 }
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 07/14] drm: cirrus: remove dead code and pointless local lut storage
  2017-07-13 16:25 ` Peter Rosin
                   ` (6 preceding siblings ...)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Dave Airlie, Gerd Hoffmann, David Airlie,
	Daniel Vetter, virtualization, dri-devel, Daniel Vetter,
	Jani Nikula, Sean Paul, Lionel Landwerlin, Boris Brezillon

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.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 8690352d96f7..be2d7e488062 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 7fa58eeadc9d..1fedab03f659 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 53f6f0f84206..a4c4a465b385 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -31,25 +31,6 @@
  * This file contains setup code for the CRTC.
  */
 
-static void cirrus_crtc_load_lut(struct drm_crtc *crtc)
-{
-	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
-	struct drm_device *dev = crtc->dev;
-	struct cirrus_device *cdev = dev->dev_private;
-	int i;
-
-	if (!crtc->enabled)
-		return;
-
-	for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
-		/* VGA registers */
-		WREG8(PALETTE_INDEX, i);
-		WREG8(PALETTE_DATA, cirrus_crtc->lut_r[i]);
-		WREG8(PALETTE_DATA, cirrus_crtc->lut_g[i]);
-		WREG8(PALETTE_DATA, cirrus_crtc->lut_b[i]);
-	}
-}
-
 /*
  * The DRM core requires DPMS functions, but they make little sense in our
  * case and so are just stubs
@@ -330,15 +311,25 @@ static int cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				 u16 *blue, uint32_t size,
 				 struct drm_modeset_acquire_ctx *ctx)
 {
-	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+	struct cirrus_device *cdev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 
-	for (i = 0; i < size; i++) {
-		cirrus_crtc->lut_r[i] = red[i];
-		cirrus_crtc->lut_g[i] = green[i];
-		cirrus_crtc->lut_b[i] = blue[i];
+	if (!crtc->enabled)
+		return 0;
+
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
+
+	for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
+		/* VGA registers */
+		WREG8(PALETTE_INDEX, i);
+		WREG8(PALETTE_DATA, *r++ >> 8);
+		WREG8(PALETTE_DATA, *g++ >> 8);
+		WREG8(PALETTE_DATA, *b++ >> 8);
 	}
-	cirrus_crtc_load_lut(crtc);
 
 	return 0;
 }
@@ -365,7 +356,6 @@ static const struct drm_crtc_helper_funcs cirrus_helper_funcs = {
 	.mode_set_base = cirrus_crtc_mode_set_base,
 	.prepare = cirrus_crtc_prepare,
 	.commit = cirrus_crtc_commit,
-	.load_lut = cirrus_crtc_load_lut,
 };
 
 /* CRTC setup */
@@ -373,7 +363,6 @@ static void cirrus_crtc_init(struct drm_device *dev)
 {
 	struct cirrus_device *cdev = dev->dev_private;
 	struct cirrus_crtc *cirrus_crtc;
-	int i;
 
 	cirrus_crtc = kzalloc(sizeof(struct cirrus_crtc) +
 			      (CIRRUSFB_CONN_LIMIT * sizeof(struct drm_connector *)),
@@ -387,37 +376,9 @@ static void cirrus_crtc_init(struct drm_device *dev)
 	drm_mode_crtc_set_gamma_size(&cirrus_crtc->base, CIRRUS_LUT_SIZE);
 	cdev->mode_info.crtc = cirrus_crtc;
 
-	for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
-		cirrus_crtc->lut_r[i] = i;
-		cirrus_crtc->lut_g[i] = i;
-		cirrus_crtc->lut_b[i] = i;
-	}
-
 	drm_crtc_helper_add(&cirrus_crtc->base, &cirrus_helper_funcs);
 }
 
-/** Sets the color ramps on behalf of fbcon */
-void cirrus_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-			      u16 blue, int regno)
-{
-	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
-
-	cirrus_crtc->lut_r[regno] = red;
-	cirrus_crtc->lut_g[regno] = green;
-	cirrus_crtc->lut_b[regno] = blue;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-void cirrus_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, int regno)
-{
-	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
-
-	*red = cirrus_crtc->lut_r[regno];
-	*green = cirrus_crtc->lut_g[regno];
-	*blue = cirrus_crtc->lut_b[regno];
-}
-
 static void cirrus_encoder_mode_set(struct drm_encoder *encoder,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 07/14] drm: cirrus: remove dead code and pointless local lut storage
  2017-07-13 16:25 ` Peter Rosin
                   ` (7 preceding siblings ...)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jani Nikula, Boris Brezillon, Lionel Landwerlin, David Airlie,
	Daniel Vetter, dri-devel, virtualization, Sean Paul,
	Daniel Vetter, Dave Airlie, 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.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 8690352d96f7..be2d7e488062 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 7fa58eeadc9d..1fedab03f659 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 53f6f0f84206..a4c4a465b385 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -31,25 +31,6 @@
  * This file contains setup code for the CRTC.
  */
 
-static void cirrus_crtc_load_lut(struct drm_crtc *crtc)
-{
-	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
-	struct drm_device *dev = crtc->dev;
-	struct cirrus_device *cdev = dev->dev_private;
-	int i;
-
-	if (!crtc->enabled)
-		return;
-
-	for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
-		/* VGA registers */
-		WREG8(PALETTE_INDEX, i);
-		WREG8(PALETTE_DATA, cirrus_crtc->lut_r[i]);
-		WREG8(PALETTE_DATA, cirrus_crtc->lut_g[i]);
-		WREG8(PALETTE_DATA, cirrus_crtc->lut_b[i]);
-	}
-}
-
 /*
  * The DRM core requires DPMS functions, but they make little sense in our
  * case and so are just stubs
@@ -330,15 +311,25 @@ static int cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				 u16 *blue, uint32_t size,
 				 struct drm_modeset_acquire_ctx *ctx)
 {
-	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+	struct cirrus_device *cdev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 
-	for (i = 0; i < size; i++) {
-		cirrus_crtc->lut_r[i] = red[i];
-		cirrus_crtc->lut_g[i] = green[i];
-		cirrus_crtc->lut_b[i] = blue[i];
+	if (!crtc->enabled)
+		return 0;
+
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
+
+	for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
+		/* VGA registers */
+		WREG8(PALETTE_INDEX, i);
+		WREG8(PALETTE_DATA, *r++ >> 8);
+		WREG8(PALETTE_DATA, *g++ >> 8);
+		WREG8(PALETTE_DATA, *b++ >> 8);
 	}
-	cirrus_crtc_load_lut(crtc);
 
 	return 0;
 }
@@ -365,7 +356,6 @@ static const struct drm_crtc_helper_funcs cirrus_helper_funcs = {
 	.mode_set_base = cirrus_crtc_mode_set_base,
 	.prepare = cirrus_crtc_prepare,
 	.commit = cirrus_crtc_commit,
-	.load_lut = cirrus_crtc_load_lut,
 };
 
 /* CRTC setup */
@@ -373,7 +363,6 @@ static void cirrus_crtc_init(struct drm_device *dev)
 {
 	struct cirrus_device *cdev = dev->dev_private;
 	struct cirrus_crtc *cirrus_crtc;
-	int i;
 
 	cirrus_crtc = kzalloc(sizeof(struct cirrus_crtc) +
 			      (CIRRUSFB_CONN_LIMIT * sizeof(struct drm_connector *)),
@@ -387,37 +376,9 @@ static void cirrus_crtc_init(struct drm_device *dev)
 	drm_mode_crtc_set_gamma_size(&cirrus_crtc->base, CIRRUS_LUT_SIZE);
 	cdev->mode_info.crtc = cirrus_crtc;
 
-	for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
-		cirrus_crtc->lut_r[i] = i;
-		cirrus_crtc->lut_g[i] = i;
-		cirrus_crtc->lut_b[i] = i;
-	}
-
 	drm_crtc_helper_add(&cirrus_crtc->base, &cirrus_helper_funcs);
 }
 
-/** Sets the color ramps on behalf of fbcon */
-void cirrus_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-			      u16 blue, int regno)
-{
-	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
-
-	cirrus_crtc->lut_r[regno] = red;
-	cirrus_crtc->lut_g[regno] = green;
-	cirrus_crtc->lut_b[regno] = blue;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-void cirrus_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, int regno)
-{
-	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
-
-	*red = cirrus_crtc->lut_r[regno];
-	*green = cirrus_crtc->lut_g[regno];
-	*blue = cirrus_crtc->lut_b[regno];
-}
-
 static void cirrus_encoder_mode_set(struct drm_encoder *encoder,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 08/14] drm: gma500: remove dead code and pointless local lut storage
  2017-07-13 16:25 ` Peter Rosin
                   ` (8 preceding siblings ...)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Patrik Jakobsson, David Airlie, dri-devel,
	Daniel Vetter, Jani Nikula, Sean Paul, Lionel Landwerlin,
	Boris Brezillon

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.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 7da70b6c83f0..2570c7f647a6 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 e7fd356acf2e..f3c48a2be71b 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -144,33 +144,32 @@ void gma_crtc_load_lut(struct drm_crtc *crtc)
 	struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
 	const struct psb_offset *map = &dev_priv->regmap[gma_crtc->pipe];
 	int palreg = map->palette;
+	u16 *r, *g, *b;
 	int i;
 
 	/* The clocks have to be on to load the palette. */
 	if (!crtc->enabled)
 		return;
 
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
+
 	if (gma_power_begin(dev, false)) {
 		for (i = 0; i < 256; i++) {
 			REG_WRITE(palreg + 4 * i,
-				  ((gma_crtc->lut_r[i] +
-				  gma_crtc->lut_adj[i]) << 16) |
-				  ((gma_crtc->lut_g[i] +
-				  gma_crtc->lut_adj[i]) << 8) |
-				  (gma_crtc->lut_b[i] +
-				  gma_crtc->lut_adj[i]));
+				  (((*r++ >> 8) + gma_crtc->lut_adj[i]) << 16) |
+				  (((*g++ >> 8) + gma_crtc->lut_adj[i]) << 8) |
+				  ((*b++ >> 8) + gma_crtc->lut_adj[i]));
 		}
 		gma_power_end(dev);
 	} else {
 		for (i = 0; i < 256; i++) {
 			/* FIXME: Why pipe[0] and not pipe[..._crtc->pipe]? */
 			dev_priv->regs.pipe[0].palette[i] =
-				  ((gma_crtc->lut_r[i] +
-				  gma_crtc->lut_adj[i]) << 16) |
-				  ((gma_crtc->lut_g[i] +
-				  gma_crtc->lut_adj[i]) << 8) |
-				  (gma_crtc->lut_b[i] +
-				  gma_crtc->lut_adj[i]);
+				(((*r++ >> 8) + gma_crtc->lut_adj[i]) << 16) |
+				(((*g++ >> 8) + gma_crtc->lut_adj[i]) << 8) |
+				((*b++ >> 8) + gma_crtc->lut_adj[i]);
 		}
 
 	}
@@ -180,15 +179,6 @@ int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue,
 		       u32 size,
 		       struct drm_modeset_acquire_ctx *ctx)
 {
-	struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
-	int i;
-
-	for (i = 0; i < size; i++) {
-		gma_crtc->lut_r[i] = red[i] >> 8;
-		gma_crtc->lut_g[i] = green[i] >> 8;
-		gma_crtc->lut_b[i] = blue[i] >> 8;
-	}
-
 	gma_crtc_load_lut(crtc);
 
 	return 0;
diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c
index 7b6c84925098..8762efaef283 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 6a10215fc42d..e8e4ea14b12b 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.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 09/14] drm: i915: remove dead code and pointless local lut storage
  2017-07-13 16:25 ` Peter Rosin
                   ` (9 preceding siblings ...)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Daniel Vetter, Jani Nikula, David Airlie, intel-gfx,
	dri-devel, Sean Paul, Lionel Landwerlin, Boris Brezillon

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.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 d93efb49a2e2..bc7bfa0efdda 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 460ca0b3fb88..19d650b24207 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)
 {
@@ -376,7 +355,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;
@@ -418,13 +396,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);
 
@@ -527,8 +498,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.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 10/14] drm: mgag200: remove dead code and pointless local lut storage
  2017-07-13 16:25 ` Peter Rosin
                   ` (10 preceding siblings ...)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Dave Airlie, David Airlie, Daniel Vetter, dri-devel,
	Daniel Vetter, Jani Nikula, Sean Paul, Lionel Landwerlin,
	Boris Brezillon

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.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 c88b6ec88dd2..04f1dfba12e5 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 5d3b1fac906f..5cf980a2e9e9 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 f4b53588e071..5e9cd4c0e8b6 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -27,15 +27,19 @@
 
 static void mga_crtc_load_lut(struct drm_crtc *crtc)
 {
-	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = dev->dev_private;
 	struct drm_framebuffer *fb = crtc->primary->fb;
+	u16 *r_ptr, *g_ptr, *b_ptr;
 	int i;
 
 	if (!crtc->enabled)
 		return;
 
+	r_ptr = crtc->gamma_store;
+	g_ptr = r_ptr + crtc->gamma_size;
+	b_ptr = g_ptr + crtc->gamma_size;
+
 	WREG8(DAC_INDEX + MGA1064_INDEX, 0);
 
 	if (fb && fb->format->cpp[0] * 8 == 16) {
@@ -46,25 +50,27 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc)
 				if (i > (MGAG200_LUT_SIZE >> 1)) {
 					r = b = 0;
 				} else {
-					r = mga_crtc->lut_r[i << 1];
-					b = mga_crtc->lut_b[i << 1];
+					r = *r_ptr++ >> 8;
+					b = *b_ptr++ >> 8;
+					r_ptr++;
+					b_ptr++;
 				}
 			} else {
-				r = mga_crtc->lut_r[i];
-				b = mga_crtc->lut_b[i];
+				r = *r_ptr++ >> 8;
+				b = *b_ptr++ >> 8;
 			}
 			/* VGA registers */
 			WREG8(DAC_INDEX + MGA1064_COL_PAL, r);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]);
+			WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
 			WREG8(DAC_INDEX + MGA1064_COL_PAL, b);
 		}
 		return;
 	}
 	for (i = 0; i < MGAG200_LUT_SIZE; i++) {
 		/* VGA registers */
-		WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_r[i]);
-		WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]);
-		WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_b[i]);
+		WREG8(DAC_INDEX + MGA1064_COL_PAL, *r_ptr++ >> 8);
+		WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
+		WREG8(DAC_INDEX + MGA1064_COL_PAL, *b_ptr++ >> 8);
 	}
 }
 
@@ -1399,14 +1405,6 @@ static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 			      u16 *blue, uint32_t size,
 			      struct drm_modeset_acquire_ctx *ctx)
 {
-	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
-	int i;
-
-	for (i = 0; i < size; i++) {
-		mga_crtc->lut_r[i] = red[i] >> 8;
-		mga_crtc->lut_g[i] = green[i] >> 8;
-		mga_crtc->lut_b[i] = blue[i] >> 8;
-	}
 	mga_crtc_load_lut(crtc);
 
 	return 0;
@@ -1455,14 +1453,12 @@ static const struct drm_crtc_helper_funcs mga_helper_funcs = {
 	.mode_set_base = mga_crtc_mode_set_base,
 	.prepare = mga_crtc_prepare,
 	.commit = mga_crtc_commit,
-	.load_lut = mga_crtc_load_lut,
 };
 
 /* CRTC setup */
 static void mga_crtc_init(struct mga_device *mdev)
 {
 	struct mga_crtc *mga_crtc;
-	int i;
 
 	mga_crtc = kzalloc(sizeof(struct mga_crtc) +
 			      (MGAG200FB_CONN_LIMIT * sizeof(struct drm_connector *)),
@@ -1476,37 +1472,9 @@ static void mga_crtc_init(struct mga_device *mdev)
 	drm_mode_crtc_set_gamma_size(&mga_crtc->base, MGAG200_LUT_SIZE);
 	mdev->mode_info.crtc = mga_crtc;
 
-	for (i = 0; i < MGAG200_LUT_SIZE; i++) {
-		mga_crtc->lut_r[i] = i;
-		mga_crtc->lut_g[i] = i;
-		mga_crtc->lut_b[i] = i;
-	}
-
 	drm_crtc_helper_add(&mga_crtc->base, &mga_helper_funcs);
 }
 
-/** Sets the color ramps on behalf of fbcon */
-void mga_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-			      u16 blue, int regno)
-{
-	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
-
-	mga_crtc->lut_r[regno] = red >> 8;
-	mga_crtc->lut_g[regno] = green >> 8;
-	mga_crtc->lut_b[regno] = blue >> 8;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-void mga_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, int regno)
-{
-	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
-
-	*red = (u16)mga_crtc->lut_r[regno] << 8;
-	*green = (u16)mga_crtc->lut_g[regno] << 8;
-	*blue = (u16)mga_crtc->lut_b[regno] << 8;
-}
-
 /*
  * The encoder comes after the CRTC in the output pipeline, but before
  * the connector. It's responsible for ensuring that the digital
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 11/14] drm: nouveau: remove dead code and pointless local lut storage
  2017-07-13 16:25 ` Peter Rosin
                   ` (11 preceding siblings ...)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Ben Skeggs, David Airlie, Daniel Vetter, dri-devel,
	nouveau, Daniel Vetter, Jani Nikula, Sean Paul,
	Lionel Landwerlin, Boris Brezillon

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.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 4b4b0b496262..8f689f1f6122 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -764,13 +764,18 @@ nv_crtc_gamma_load(struct drm_crtc *crtc)
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
 	struct drm_device *dev = nv_crtc->base.dev;
 	struct rgb { uint8_t r, g, b; } __attribute__((packed)) *rgbs;
+	u16 *r, *g, *b;
 	int i;
 
 	rgbs = (struct rgb *)nv04_display(dev)->mode_reg.crtc_reg[nv_crtc->index].DAC;
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
+
 	for (i = 0; i < 256; i++) {
-		rgbs[i].r = nv_crtc->lut.r[i] >> 8;
-		rgbs[i].g = nv_crtc->lut.g[i] >> 8;
-		rgbs[i].b = nv_crtc->lut.b[i] >> 8;
+		rgbs[i].r = *r++ >> 8;
+		rgbs[i].g = *g++ >> 8;
+		rgbs[i].b = *b++ >> 8;
 	}
 
 	nouveau_hw_load_state_palette(dev, nv_crtc->index, &nv04_display(dev)->mode_reg);
@@ -792,13 +797,6 @@ nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
 		  struct drm_modeset_acquire_ctx *ctx)
 {
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-	int i;
-
-	for (i = 0; i < size; i++) {
-		nv_crtc->lut.r[i] = r[i];
-		nv_crtc->lut.g[i] = g[i];
-		nv_crtc->lut.b[i] = b[i];
-	}
 
 	/* We need to know the depth before we upload, but it's possible to
 	 * get called before a framebuffer is bound.  If this is the case,
@@ -1095,7 +1093,6 @@ static const struct drm_crtc_helper_funcs nv04_crtc_helper_funcs = {
 	.mode_set = nv_crtc_mode_set,
 	.mode_set_base = nv04_crtc_mode_set_base,
 	.mode_set_base_atomic = nv04_crtc_mode_set_base_atomic,
-	.load_lut = nv_crtc_gamma_load,
 	.disable = nv_crtc_disable,
 };
 
@@ -1103,17 +1100,12 @@ int
 nv04_crtc_create(struct drm_device *dev, int crtc_num)
 {
 	struct nouveau_crtc *nv_crtc;
-	int ret, i;
+	int ret;
 
 	nv_crtc = kzalloc(sizeof(*nv_crtc), GFP_KERNEL);
 	if (!nv_crtc)
 		return -ENOMEM;
 
-	for (i = 0; i < 256; i++) {
-		nv_crtc->lut.r[i] = i << 8;
-		nv_crtc->lut.g[i] = i << 8;
-		nv_crtc->lut.b[i] = i << 8;
-	}
 	nv_crtc->lut.depth = 0;
 
 	nv_crtc->index = crtc_num;
diff --git a/drivers/gpu/drm/nouveau/nouveau_crtc.h b/drivers/gpu/drm/nouveau/nouveau_crtc.h
index 050fcf30a0d2..b7a18fbee6dc 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 2665a078b6da..f7707849bb53 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 42a85c14aea0..2ef03ed82baa 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -2204,28 +2204,29 @@ nv50_head_lut_load(struct drm_crtc *crtc)
 	struct nv50_disp *disp = nv50_disp(crtc->dev);
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
 	void __iomem *lut = nvbo_kmap_obj_iovirtual(nv_crtc->lut.nvbo);
+	u16 *r, *g, *b;
 	int i;
 
-	for (i = 0; i < 256; i++) {
-		u16 r = nv_crtc->lut.r[i] >> 2;
-		u16 g = nv_crtc->lut.g[i] >> 2;
-		u16 b = nv_crtc->lut.b[i] >> 2;
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 
+	for (i = 0; i < 256; i++) {
 		if (disp->disp->oclass < GF110_DISP) {
-			writew(r + 0x0000, lut + (i * 0x08) + 0);
-			writew(g + 0x0000, lut + (i * 0x08) + 2);
-			writew(b + 0x0000, lut + (i * 0x08) + 4);
+			writew((*r++ >> 2) + 0x0000, lut + (i * 0x08) + 0);
+			writew((*g++ >> 2) + 0x0000, lut + (i * 0x08) + 2);
+			writew((*b++ >> 2) + 0x0000, lut + (i * 0x08) + 4);
 		} else {
-			writew(r + 0x6000, lut + (i * 0x20) + 0);
-			writew(g + 0x6000, lut + (i * 0x20) + 2);
-			writew(b + 0x6000, lut + (i * 0x20) + 4);
+			/* 0x6000 interferes with the 14-bit color??? */
+			writew((*r++ >> 2) + 0x6000, lut + (i * 0x20) + 0);
+			writew((*g++ >> 2) + 0x6000, lut + (i * 0x20) + 2);
+			writew((*b++ >> 2) + 0x6000, lut + (i * 0x20) + 4);
 		}
 	}
 }
 
 static const struct drm_crtc_helper_funcs
 nv50_head_help = {
-	.load_lut = nv50_head_lut_load,
 	.atomic_check = nv50_head_atomic_check,
 };
 
@@ -2234,15 +2235,6 @@ nv50_head_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
 		    uint32_t size,
 		    struct drm_modeset_acquire_ctx *ctx)
 {
-	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-	u32 i;
-
-	for (i = 0; i < size; i++) {
-		nv_crtc->lut.r[i] = r[i];
-		nv_crtc->lut.g[i] = g[i];
-		nv_crtc->lut.b[i] = b[i];
-	}
-
 	nv50_head_lut_load(crtc);
 	return 0;
 }
@@ -2340,19 +2332,13 @@ nv50_head_create(struct drm_device *dev, int index)
 	struct nv50_base *base;
 	struct nv50_curs *curs;
 	struct drm_crtc *crtc;
-	int ret, i;
+	int ret;
 
 	head = kzalloc(sizeof(*head), GFP_KERNEL);
 	if (!head)
 		return -ENOMEM;
 
 	head->base.index = index;
-	for (i = 0; i < 256; i++) {
-		head->base.lut.r[i] = i << 8;
-		head->base.lut.g[i] = i << 8;
-		head->base.lut.b[i] = i << 8;
-	}
-
 	ret = nv50_base_new(drm, head, &base);
 	if (ret == 0)
 		ret = nv50_curs_new(drm, head, &curs);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 12/14] drm: radeon: remove dead code and pointless local lut storage
  2017-07-13 16:25 ` Peter Rosin
                   ` (12 preceding siblings ...)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  2017-07-14 14:06     ` Alex Deucher
  -1 siblings, 1 reply; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Alex Deucher, Christian König, David Airlie,
	amd-gfx, dri-devel, Daniel Vetter, Jani Nikula, Sean Paul,
	Lionel Landwerlin, Boris Brezillon

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.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 3c492a0aa6bd..02baaaf20e9d 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 27affbde058c..2f642cbefd8e 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 17d3dafc8319..8b7d7a0d3ca8 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -42,6 +42,7 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct radeon_device *rdev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 
 	DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -60,11 +61,14 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(AVIVO_DC_LUT_WRITE_EN_MASK, 0x0000003f);
 
 	WREG8(AVIVO_DC_LUT_RW_INDEX, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(AVIVO_DC_LUT_30_COLOR,
-			     (radeon_crtc->lut_r[i] << 20) |
-			     (radeon_crtc->lut_g[i] << 10) |
-			     (radeon_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 
 	/* Only change bit 0 of LUT_SEL, other bits are set elsewhere */
@@ -76,6 +80,7 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct radeon_device *rdev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 
 	DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -93,11 +98,14 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(EVERGREEN_DC_LUT_WRITE_EN_MASK + radeon_crtc->crtc_offset, 0x00000007);
 
 	WREG32(EVERGREEN_DC_LUT_RW_INDEX + radeon_crtc->crtc_offset, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(EVERGREEN_DC_LUT_30_COLOR + radeon_crtc->crtc_offset,
-		       (radeon_crtc->lut_r[i] << 20) |
-		       (radeon_crtc->lut_g[i] << 10) |
-		       (radeon_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 }
 
@@ -106,6 +114,7 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct radeon_device *rdev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 
 	DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -135,11 +144,14 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(EVERGREEN_DC_LUT_WRITE_EN_MASK + radeon_crtc->crtc_offset, 0x00000007);
 
 	WREG32(EVERGREEN_DC_LUT_RW_INDEX + radeon_crtc->crtc_offset, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(EVERGREEN_DC_LUT_30_COLOR + radeon_crtc->crtc_offset,
-		       (radeon_crtc->lut_r[i] << 20) |
-		       (radeon_crtc->lut_g[i] << 10) |
-		       (radeon_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 
 	WREG32(NI_DEGAMMA_CONTROL + radeon_crtc->crtc_offset,
@@ -172,6 +184,7 @@ static void legacy_crtc_load_lut(struct drm_crtc *crtc)
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct radeon_device *rdev = dev->dev_private;
+	u16 *r, *g, *b;
 	int i;
 	uint32_t dac2_cntl;
 
@@ -183,11 +196,14 @@ static void legacy_crtc_load_lut(struct drm_crtc *crtc)
 	WREG32(RADEON_DAC_CNTL2, dac2_cntl);
 
 	WREG8(RADEON_PALETTE_INDEX, 0);
+	r = crtc->gamma_store;
+	g = r + crtc->gamma_size;
+	b = g + crtc->gamma_size;
 	for (i = 0; i < 256; i++) {
 		WREG32(RADEON_PALETTE_30_DATA,
-			     (radeon_crtc->lut_r[i] << 20) |
-			     (radeon_crtc->lut_g[i] << 10) |
-			     (radeon_crtc->lut_b[i] << 0));
+		       ((*r++ & 0xffc0) << 14) |
+		       ((*g++ & 0xffc0) << 4) |
+		       (*b++ >> 6));
 	}
 }
 
@@ -209,41 +225,10 @@ void radeon_crtc_load_lut(struct drm_crtc *crtc)
 		legacy_crtc_load_lut(crtc);
 }
 
-/** Sets the color ramps on behalf of fbcon */
-void radeon_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-			      u16 blue, int regno)
-{
-	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-
-	radeon_crtc->lut_r[regno] = red >> 6;
-	radeon_crtc->lut_g[regno] = green >> 6;
-	radeon_crtc->lut_b[regno] = blue >> 6;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-void radeon_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, int regno)
-{
-	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-
-	*red = radeon_crtc->lut_r[regno] << 6;
-	*green = radeon_crtc->lut_g[regno] << 6;
-	*blue = radeon_crtc->lut_b[regno] << 6;
-}
-
 static int radeon_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				 u16 *blue, uint32_t size,
 				 struct drm_modeset_acquire_ctx *ctx)
 {
-	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-	int i;
-
-	/* userspace palettes are always correct as is */
-	for (i = 0; i < size; i++) {
-		radeon_crtc->lut_r[i] = red[i] >> 6;
-		radeon_crtc->lut_g[i] = green[i] >> 6;
-		radeon_crtc->lut_b[i] = blue[i] >> 6;
-	}
 	radeon_crtc_load_lut(crtc);
 
 	return 0;
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 356ad90a5238..638bcb5593c8 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 ce6cb6666212..1f1856e0b1e0 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 00f5ec5c12c7..da44ac234f64 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.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 13/14] drm: stm: remove dead code and pointless local lut storage
  2017-07-13 16:25 ` Peter Rosin
                   ` (13 preceding siblings ...)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  2017-07-17  9:47     ` Philippe CORNU
  -1 siblings, 1 reply; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Yannick Fertre, Philippe Cornu, Benjamin Gaignard,
	Vincent Abriou, David Airlie, dri-devel, Daniel Vetter,
	Jani Nikula, Sean Paul, Lionel Landwerlin, Boris Brezillon

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.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 533176015cbb..3e95b4d1f4cc 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_atomic_enable(struct drm_crtc *crtc,
 				    struct drm_crtc_state *old_state)
 {
@@ -525,7 +514,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,
 	.mode_set_nofb = ltdc_crtc_mode_set_nofb,
 	.atomic_flush = ltdc_crtc_atomic_flush,
 	.atomic_enable = ltdc_crtc_atomic_enable,
diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
index d7a9c736ac1e..620ca5555abf 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.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 14/14] drm: remove unused and redundant callbacks
  2017-07-13 16:25 ` Peter Rosin
                   ` (14 preceding siblings ...)
  (?)
@ 2017-07-13 16:25 ` Peter Rosin
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Rosin @ 2017-07-13 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Daniel Vetter, Jani Nikula, Sean Paul, David Airlie,
	dri-devel, Lionel Landwerlin, Boris Brezillon

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

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 3a911a64c257..0cc89623abe6 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,
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index ea170b96e88d..21c56305df1d 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 06569845708c..6cdcb4263a73 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.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 03/14] drm/fb-helper: separate the fb_setcmap helper into atomic and legacy paths
  2017-07-13 16:25 ` [PATCH v5 03/14] drm/fb-helper: separate the fb_setcmap helper into atomic and legacy paths Peter Rosin
@ 2017-07-14 13:54   ` Daniel Vetter
  2017-08-03 22:49     ` Peter Rosin
  0 siblings, 1 reply; 35+ messages in thread
From: Daniel Vetter @ 2017-07-14 13:54 UTC (permalink / raw)
  To: Peter Rosin; +Cc: linux-kernel, Boris Brezillon, dri-devel, Daniel Vetter

On Thu, Jul 13, 2017 at 06:25:27PM +0200, Peter Rosin wrote:
> The legacy path implements setcmap in terms of crtc .gamma_set.
> 
> The atomic path implements setcmap by directly updating the crtc gamma_lut
> property.
> 
> This has a couple of benefits:
> - it makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
>   completely obsolete. They are now unused and subject for removal.
> - atomic drivers that support clut modes get fbdev support for those from
>   the drm core. This includes atmel-hlcdc, but perhaps others as well?
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

Ok, I merged the core parts. I'll wait with the driver stuff for a bit
more (maybe 1-2 weeks) for more acks. Pls remind me in case I forget to
pull them in.

Thanks a lot for doing this, great work!
-Daniel
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 231 ++++++++++++++++++++++++++++------------
>  1 file changed, 160 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 721511da4de6..42090fe00ef9 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1195,27 +1195,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;
> @@ -1248,57 +1227,138 @@ 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;
> +	drm_modeset_lock_all(fb_helper->dev);
> +	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;
> +
> +		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;
>  	}
> +	drm_modeset_unlock_all(fb_helper->dev);
>  
> -	drm_modeset_lock_all(dev);
> -	if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
> -		rc = setcmap_pseudo_palette(cmap, info);
> -		goto out;
> +	return ret;
> +}
> +
> +static struct drm_property_blob *setcmap_new_gamma_lut(struct drm_crtc *crtc,
> +						       struct fb_cmap *cmap)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_property_blob *gamma_lut;
> +	struct drm_color_lut *lut;
> +	int size = crtc->gamma_size;
> +	int i;
> +
> +	if (!size || cmap->start + cmap->len > size)
> +		return ERR_PTR(-EINVAL);
> +
> +	gamma_lut = drm_property_create_blob(dev, sizeof(*lut) * size, NULL);
> +	if (IS_ERR(gamma_lut))
> +		return gamma_lut;
> +
> +	lut = (struct drm_color_lut *)gamma_lut->data;
> +	if (cmap->start || cmap->len != size) {
> +		u16 *r = crtc->gamma_store;
> +		u16 *g = r + crtc->gamma_size;
> +		u16 *b = g + crtc->gamma_size;
> +
> +		for (i = 0; i < cmap->start; i++) {
> +			lut[i].red = r[i];
> +			lut[i].green = g[i];
> +			lut[i].blue = b[i];
> +		}
> +		for (i = cmap->start + cmap->len; i < size; i++) {
> +			lut[i].red = r[i];
> +			lut[i].green = g[i];
> +			lut[i].blue = b[i];
> +		}
> +	}
> +
> +	for (i = 0; i < cmap->len; i++) {
> +		lut[cmap->start + i].red = cmap->red[i];
> +		lut[cmap->start + i].green = cmap->green[i];
> +		lut[cmap->start + i].blue = cmap->blue[i];
> +	}
> +
> +	return gamma_lut;
> +}
> +
> +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_property_blob *gamma_lut = NULL;
> +	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;
> +	bool replaced;
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +
> +	state = drm_atomic_state_alloc(dev);
> +	if (!state) {
> +		ret = -ENOMEM;
> +		goto out_ctx;
>  	}
>  
> +	state->acquire_ctx = &ctx;
> +retry:
>  	for (i = 0; i < fb_helper->crtc_count; i++) {
>  		crtc = fb_helper->crtc_info[i].mode_set.crtc;
> -		crtc_funcs = crtc->helper_private;
>  
> -		red = cmap->red;
> -		green = cmap->green;
> -		blue = cmap->blue;
> -		transp = cmap->transp;
> -		start = cmap->start;
> -
> -		if (!crtc->gamma_size) {
> -			rc = -EINVAL;
> -			goto out;
> +		if (!gamma_lut)
> +			gamma_lut = setcmap_new_gamma_lut(crtc, cmap);
> +		if (IS_ERR(gamma_lut)) {
> +			ret = PTR_ERR(gamma_lut);
> +			gamma_lut = NULL;
> +			goto out_state;
>  		}
>  
> -		if (cmap->start + cmap->len > crtc->gamma_size) {
> -			rc = -EINVAL;
> -			goto out;
> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +		if (IS_ERR(crtc_state)) {
> +			ret = PTR_ERR(crtc_state);
> +			goto out_state;
>  		}
>  
> +		replaced  = drm_property_replace_blob(&crtc_state->degamma_lut,
> +						      NULL);
> +		replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
> +		replaced |= drm_property_replace_blob(&crtc_state->gamma_lut,
> +						      gamma_lut);
> +		crtc_state->color_mgmt_changed |= replaced;
> +	}
> +
> +	ret = drm_atomic_commit(state);
> +	if (ret)
> +		goto out_state;
> +
> +	for (i = 0; i < fb_helper->crtc_count; i++) {
> +		crtc = fb_helper->crtc_info[i].mode_set.crtc;
> +
>  		r = crtc->gamma_store;
>  		g = r + crtc->gamma_size;
>  		b = g + crtc->gamma_size;
> @@ -1306,28 +1366,57 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>  		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));
> +	}
> +
> +out_state:
> +	if (ret == -EDEADLK)
> +		goto backoff;
> +
> +	drm_property_blob_put(gamma_lut);
> +	drm_atomic_state_put(state);
> +out_ctx:
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
>  
> -		for (j = 0; j < cmap->len; j++) {
> -			u16 hred, hgreen, hblue, htransp = 0xffff;
> +	return ret;
>  
> -			hred = *red++;
> -			hgreen = *green++;
> -			hblue = *blue++;
> +backoff:
> +	drm_atomic_state_clear(state);
> +	drm_modeset_backoff(&ctx);
> +	goto retry;
> +}
>  
> -			if (transp)
> -				htransp = *transp++;
> +/**
> + * 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;
>  
> -			rc = setcolreg(crtc, hred, hgreen, hblue, start++, info);
> -			if (rc)
> -				goto out;
> -		}
> -		if (crtc_funcs->load_lut)
> -			crtc_funcs->load_lut(crtc);
> +	if (oops_in_progress)
> +		return -EBUSY;
> +
> +	mutex_lock(&fb_helper->lock);
> +
> +	if (!drm_fb_helper_is_bound(fb_helper)) {
> +		ret = -EBUSY;
> +		goto out;
>  	}
> - 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);
> +
> +out:
>  	mutex_unlock(&fb_helper->lock);
> -	return rc;
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_fb_helper_setcmap);
>  
> -- 
> 2.11.0
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 04/14] drm: amd: remove dead code and pointless local lut storage
@ 2017-07-14 14:06     ` Alex Deucher
  0 siblings, 0 replies; 35+ messages in thread
From: Alex Deucher @ 2017-07-14 14:06 UTC (permalink / raw)
  To: Peter Rosin
  Cc: LKML, Jani Nikula, Boris Brezillon, Lionel Landwerlin,
	David Airlie, Daniel Vetter, amd-gfx list, Christian König,
	Sean Paul, Maling list - DRI developers, Alex Deucher,
	Daniel Vetter

On Thu, Jul 13, 2017 at 12:25 PM, Peter Rosin <peda@axentia.se> wrote:
> 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.
>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Peter Rosin <peda@axentia.se>

Acked-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  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 c0d8c6ff6380..7dc378013a42 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 43a9d3aec6c4..39f7eda6091e 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 9f78c03a2e31..c9580235e35b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2267,6 +2267,7 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc *crtc)
>         struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct amdgpu_device *adev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>         u32 tmp;
>
> @@ -2304,11 +2305,14 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
>
>         WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
> -                      (amdgpu_crtc->lut_r[i] << 20) |
> -                      (amdgpu_crtc->lut_g[i] << 10) |
> -                      (amdgpu_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>
>         tmp = RREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset);
> @@ -2624,15 +2628,6 @@ static int dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>                                     u16 *blue, uint32_t size,
>                                     struct drm_modeset_acquire_ctx *ctx)
>  {
> -       struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -       int i;
> -
> -       /* userspace palettes are always correct as is */
> -       for (i = 0; i < size; i++) {
> -               amdgpu_crtc->lut_r[i] = red[i] >> 6;
> -               amdgpu_crtc->lut_g[i] = green[i] >> 6;
> -               amdgpu_crtc->lut_b[i] = blue[i] >> 6;
> -       }
>         dce_v10_0_crtc_load_lut(crtc);
>
>         return 0;
> @@ -2844,14 +2839,12 @@ static const struct drm_crtc_helper_funcs dce_v10_0_crtc_helper_funcs = {
>         .mode_set_base_atomic = dce_v10_0_crtc_set_base_atomic,
>         .prepare = dce_v10_0_crtc_prepare,
>         .commit = dce_v10_0_crtc_commit,
> -       .load_lut = dce_v10_0_crtc_load_lut,
>         .disable = dce_v10_0_crtc_disable,
>  };
>
>  static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int index)
>  {
>         struct amdgpu_crtc *amdgpu_crtc;
> -       int i;
>
>         amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
>                               (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
> @@ -2869,12 +2862,6 @@ static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int index)
>         adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
>         adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
>
> -       for (i = 0; i < 256; i++) {
> -               amdgpu_crtc->lut_r[i] = i << 2;
> -               amdgpu_crtc->lut_g[i] = i << 2;
> -               amdgpu_crtc->lut_b[i] = i << 2;
> -       }
> -
>         switch (amdgpu_crtc->crtc_id) {
>         case 0:
>         default:
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index 4bcf01dc567a..7e14f532df59 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -2251,6 +2251,7 @@ static void dce_v11_0_crtc_load_lut(struct drm_crtc *crtc)
>         struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct amdgpu_device *adev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>         u32 tmp;
>
> @@ -2282,11 +2283,14 @@ static void dce_v11_0_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
>
>         WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
> -                      (amdgpu_crtc->lut_r[i] << 20) |
> -                      (amdgpu_crtc->lut_g[i] << 10) |
> -                      (amdgpu_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>
>         tmp = RREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset);
> @@ -2644,15 +2648,6 @@ static int dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>                                     u16 *blue, uint32_t size,
>                                     struct drm_modeset_acquire_ctx *ctx)
>  {
> -       struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -       int i;
> -
> -       /* userspace palettes are always correct as is */
> -       for (i = 0; i < size; i++) {
> -               amdgpu_crtc->lut_r[i] = red[i] >> 6;
> -               amdgpu_crtc->lut_g[i] = green[i] >> 6;
> -               amdgpu_crtc->lut_b[i] = blue[i] >> 6;
> -       }
>         dce_v11_0_crtc_load_lut(crtc);
>
>         return 0;
> @@ -2892,14 +2887,12 @@ static const struct drm_crtc_helper_funcs dce_v11_0_crtc_helper_funcs = {
>         .mode_set_base_atomic = dce_v11_0_crtc_set_base_atomic,
>         .prepare = dce_v11_0_crtc_prepare,
>         .commit = dce_v11_0_crtc_commit,
> -       .load_lut = dce_v11_0_crtc_load_lut,
>         .disable = dce_v11_0_crtc_disable,
>  };
>
>  static int dce_v11_0_crtc_init(struct amdgpu_device *adev, int index)
>  {
>         struct amdgpu_crtc *amdgpu_crtc;
> -       int i;
>
>         amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
>                               (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
> @@ -2917,12 +2910,6 @@ static int dce_v11_0_crtc_init(struct amdgpu_device *adev, int index)
>         adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
>         adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
>
> -       for (i = 0; i < 256; i++) {
> -               amdgpu_crtc->lut_r[i] = i << 2;
> -               amdgpu_crtc->lut_g[i] = i << 2;
> -               amdgpu_crtc->lut_b[i] = i << 2;
> -       }
> -
>         switch (amdgpu_crtc->crtc_id) {
>         case 0:
>         default:
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index fd134a4629d7..d773b50afa60 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -2182,6 +2182,7 @@ static void dce_v6_0_crtc_load_lut(struct drm_crtc *crtc)
>         struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct amdgpu_device *adev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>
>         DRM_DEBUG_KMS("%d\n", amdgpu_crtc->crtc_id);
> @@ -2211,11 +2212,14 @@ static void dce_v6_0_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
>
>         WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
> -                      (amdgpu_crtc->lut_r[i] << 20) |
> -                      (amdgpu_crtc->lut_g[i] << 10) |
> -                      (amdgpu_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>
>         WREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset,
> @@ -2496,15 +2500,6 @@ static int dce_v6_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>                                    u16 *blue, uint32_t size,
>                                    struct drm_modeset_acquire_ctx *ctx)
>  {
> -       struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -       int i;
> -
> -       /* userspace palettes are always correct as is */
> -       for (i = 0; i < size; i++) {
> -               amdgpu_crtc->lut_r[i] = red[i] >> 6;
> -               amdgpu_crtc->lut_g[i] = green[i] >> 6;
> -               amdgpu_crtc->lut_b[i] = blue[i] >> 6;
> -       }
>         dce_v6_0_crtc_load_lut(crtc);
>
>         return 0;
> @@ -2712,14 +2707,12 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
>         .mode_set_base_atomic = dce_v6_0_crtc_set_base_atomic,
>         .prepare = dce_v6_0_crtc_prepare,
>         .commit = dce_v6_0_crtc_commit,
> -       .load_lut = dce_v6_0_crtc_load_lut,
>         .disable = dce_v6_0_crtc_disable,
>  };
>
>  static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>  {
>         struct amdgpu_crtc *amdgpu_crtc;
> -       int i;
>
>         amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
>                               (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
> @@ -2737,12 +2730,6 @@ static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>         adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
>         adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
>
> -       for (i = 0; i < 256; i++) {
> -               amdgpu_crtc->lut_r[i] = i << 2;
> -               amdgpu_crtc->lut_g[i] = i << 2;
> -               amdgpu_crtc->lut_b[i] = i << 2;
> -       }
> -
>         amdgpu_crtc->crtc_offset = crtc_offsets[amdgpu_crtc->crtc_id];
>
>         amdgpu_crtc->pll_id = ATOM_PPLL_INVALID;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index a9e869554627..4eb63f6a41ab 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -2124,6 +2124,7 @@ static void dce_v8_0_crtc_load_lut(struct drm_crtc *crtc)
>         struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct amdgpu_device *adev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>
>         DRM_DEBUG_KMS("%d\n", amdgpu_crtc->crtc_id);
> @@ -2153,11 +2154,14 @@ static void dce_v8_0_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
>
>         WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
> -                      (amdgpu_crtc->lut_r[i] << 20) |
> -                      (amdgpu_crtc->lut_g[i] << 10) |
> -                      (amdgpu_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>
>         WREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset,
> @@ -2475,15 +2479,6 @@ static int dce_v8_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>                                    u16 *blue, uint32_t size,
>                                    struct drm_modeset_acquire_ctx *ctx)
>  {
> -       struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -       int i;
> -
> -       /* userspace palettes are always correct as is */
> -       for (i = 0; i < size; i++) {
> -               amdgpu_crtc->lut_r[i] = red[i] >> 6;
> -               amdgpu_crtc->lut_g[i] = green[i] >> 6;
> -               amdgpu_crtc->lut_b[i] = blue[i] >> 6;
> -       }
>         dce_v8_0_crtc_load_lut(crtc);
>
>         return 0;
> @@ -2702,14 +2697,12 @@ static const struct drm_crtc_helper_funcs dce_v8_0_crtc_helper_funcs = {
>         .mode_set_base_atomic = dce_v8_0_crtc_set_base_atomic,
>         .prepare = dce_v8_0_crtc_prepare,
>         .commit = dce_v8_0_crtc_commit,
> -       .load_lut = dce_v8_0_crtc_load_lut,
>         .disable = dce_v8_0_crtc_disable,
>  };
>
>  static int dce_v8_0_crtc_init(struct amdgpu_device *adev, int index)
>  {
>         struct amdgpu_crtc *amdgpu_crtc;
> -       int i;
>
>         amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
>                               (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
> @@ -2727,12 +2720,6 @@ static int dce_v8_0_crtc_init(struct amdgpu_device *adev, int index)
>         adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
>         adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
>
> -       for (i = 0; i < 256; i++) {
> -               amdgpu_crtc->lut_r[i] = i << 2;
> -               amdgpu_crtc->lut_g[i] = i << 2;
> -               amdgpu_crtc->lut_b[i] = i << 2;
> -       }
> -
>         amdgpu_crtc->crtc_offset = crtc_offsets[amdgpu_crtc->crtc_id];
>
>         amdgpu_crtc->pll_id = ATOM_PPLL_INVALID;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> index 90bb08309a53..ecf34bc77a63 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> @@ -168,16 +168,6 @@ static int dce_virtual_crtc_gamma_set(struct drm_crtc *crtc, u16 *red,
>                                       u16 *green, u16 *blue, uint32_t size,
>                                       struct drm_modeset_acquire_ctx *ctx)
>  {
> -       struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -       int i;
> -
> -       /* userspace palettes are always correct as is */
> -       for (i = 0; i < size; i++) {
> -               amdgpu_crtc->lut_r[i] = red[i] >> 6;
> -               amdgpu_crtc->lut_g[i] = green[i] >> 6;
> -               amdgpu_crtc->lut_b[i] = blue[i] >> 6;
> -       }
> -
>         return 0;
>  }
>
> @@ -289,11 +279,6 @@ static int dce_virtual_crtc_set_base(struct drm_crtc *crtc, int x, int y,
>         return 0;
>  }
>
> -static void dce_virtual_crtc_load_lut(struct drm_crtc *crtc)
> -{
> -       return;
> -}
> -
>  static int dce_virtual_crtc_set_base_atomic(struct drm_crtc *crtc,
>                                          struct drm_framebuffer *fb,
>                                          int x, int y, enum mode_set_atomic state)
> @@ -309,14 +294,12 @@ static const struct drm_crtc_helper_funcs dce_virtual_crtc_helper_funcs = {
>         .mode_set_base_atomic = dce_virtual_crtc_set_base_atomic,
>         .prepare = dce_virtual_crtc_prepare,
>         .commit = dce_virtual_crtc_commit,
> -       .load_lut = dce_virtual_crtc_load_lut,
>         .disable = dce_virtual_crtc_disable,
>  };
>
>  static int dce_virtual_crtc_init(struct amdgpu_device *adev, int index)
>  {
>         struct amdgpu_crtc *amdgpu_crtc;
> -       int i;
>
>         amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
>                               (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
> @@ -329,12 +312,6 @@ static int dce_virtual_crtc_init(struct amdgpu_device *adev, int index)
>         amdgpu_crtc->crtc_id = index;
>         adev->mode_info.crtcs[index] = amdgpu_crtc;
>
> -       for (i = 0; i < 256; i++) {
> -               amdgpu_crtc->lut_r[i] = i << 2;
> -               amdgpu_crtc->lut_g[i] = i << 2;
> -               amdgpu_crtc->lut_b[i] = i << 2;
> -       }
> -
>         amdgpu_crtc->pll_id = ATOM_PPLL_INVALID;
>         amdgpu_crtc->encoder = NULL;
>         amdgpu_crtc->connector = NULL;
> --
> 2.11.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 04/14] drm: amd: remove dead code and pointless local lut storage
@ 2017-07-14 14:06     ` Alex Deucher
  0 siblings, 0 replies; 35+ messages in thread
From: Alex Deucher @ 2017-07-14 14:06 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Boris Brezillon, amd-gfx list, David Airlie, Daniel Vetter,
	Maling list - DRI developers, LKML, Lionel Landwerlin, Sean Paul,
	Jani Nikula, Alex Deucher, Daniel Vetter, Christian König

On Thu, Jul 13, 2017 at 12:25 PM, Peter Rosin <peda@axentia.se> wrote:
> 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.
>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Peter Rosin <peda@axentia.se>

Acked-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  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 c0d8c6ff6380..7dc378013a42 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 43a9d3aec6c4..39f7eda6091e 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 9f78c03a2e31..c9580235e35b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2267,6 +2267,7 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc *crtc)
>         struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct amdgpu_device *adev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>         u32 tmp;
>
> @@ -2304,11 +2305,14 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
>
>         WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
> -                      (amdgpu_crtc->lut_r[i] << 20) |
> -                      (amdgpu_crtc->lut_g[i] << 10) |
> -                      (amdgpu_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>
>         tmp = RREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset);
> @@ -2624,15 +2628,6 @@ static int dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>                                     u16 *blue, uint32_t size,
>                                     struct drm_modeset_acquire_ctx *ctx)
>  {
> -       struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -       int i;
> -
> -       /* userspace palettes are always correct as is */
> -       for (i = 0; i < size; i++) {
> -               amdgpu_crtc->lut_r[i] = red[i] >> 6;
> -               amdgpu_crtc->lut_g[i] = green[i] >> 6;
> -               amdgpu_crtc->lut_b[i] = blue[i] >> 6;
> -       }
>         dce_v10_0_crtc_load_lut(crtc);
>
>         return 0;
> @@ -2844,14 +2839,12 @@ static const struct drm_crtc_helper_funcs dce_v10_0_crtc_helper_funcs = {
>         .mode_set_base_atomic = dce_v10_0_crtc_set_base_atomic,
>         .prepare = dce_v10_0_crtc_prepare,
>         .commit = dce_v10_0_crtc_commit,
> -       .load_lut = dce_v10_0_crtc_load_lut,
>         .disable = dce_v10_0_crtc_disable,
>  };
>
>  static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int index)
>  {
>         struct amdgpu_crtc *amdgpu_crtc;
> -       int i;
>
>         amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
>                               (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
> @@ -2869,12 +2862,6 @@ static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int index)
>         adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
>         adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
>
> -       for (i = 0; i < 256; i++) {
> -               amdgpu_crtc->lut_r[i] = i << 2;
> -               amdgpu_crtc->lut_g[i] = i << 2;
> -               amdgpu_crtc->lut_b[i] = i << 2;
> -       }
> -
>         switch (amdgpu_crtc->crtc_id) {
>         case 0:
>         default:
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index 4bcf01dc567a..7e14f532df59 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -2251,6 +2251,7 @@ static void dce_v11_0_crtc_load_lut(struct drm_crtc *crtc)
>         struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct amdgpu_device *adev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>         u32 tmp;
>
> @@ -2282,11 +2283,14 @@ static void dce_v11_0_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
>
>         WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
> -                      (amdgpu_crtc->lut_r[i] << 20) |
> -                      (amdgpu_crtc->lut_g[i] << 10) |
> -                      (amdgpu_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>
>         tmp = RREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset);
> @@ -2644,15 +2648,6 @@ static int dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>                                     u16 *blue, uint32_t size,
>                                     struct drm_modeset_acquire_ctx *ctx)
>  {
> -       struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -       int i;
> -
> -       /* userspace palettes are always correct as is */
> -       for (i = 0; i < size; i++) {
> -               amdgpu_crtc->lut_r[i] = red[i] >> 6;
> -               amdgpu_crtc->lut_g[i] = green[i] >> 6;
> -               amdgpu_crtc->lut_b[i] = blue[i] >> 6;
> -       }
>         dce_v11_0_crtc_load_lut(crtc);
>
>         return 0;
> @@ -2892,14 +2887,12 @@ static const struct drm_crtc_helper_funcs dce_v11_0_crtc_helper_funcs = {
>         .mode_set_base_atomic = dce_v11_0_crtc_set_base_atomic,
>         .prepare = dce_v11_0_crtc_prepare,
>         .commit = dce_v11_0_crtc_commit,
> -       .load_lut = dce_v11_0_crtc_load_lut,
>         .disable = dce_v11_0_crtc_disable,
>  };
>
>  static int dce_v11_0_crtc_init(struct amdgpu_device *adev, int index)
>  {
>         struct amdgpu_crtc *amdgpu_crtc;
> -       int i;
>
>         amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
>                               (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
> @@ -2917,12 +2910,6 @@ static int dce_v11_0_crtc_init(struct amdgpu_device *adev, int index)
>         adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
>         adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
>
> -       for (i = 0; i < 256; i++) {
> -               amdgpu_crtc->lut_r[i] = i << 2;
> -               amdgpu_crtc->lut_g[i] = i << 2;
> -               amdgpu_crtc->lut_b[i] = i << 2;
> -       }
> -
>         switch (amdgpu_crtc->crtc_id) {
>         case 0:
>         default:
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index fd134a4629d7..d773b50afa60 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -2182,6 +2182,7 @@ static void dce_v6_0_crtc_load_lut(struct drm_crtc *crtc)
>         struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct amdgpu_device *adev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>
>         DRM_DEBUG_KMS("%d\n", amdgpu_crtc->crtc_id);
> @@ -2211,11 +2212,14 @@ static void dce_v6_0_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
>
>         WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
> -                      (amdgpu_crtc->lut_r[i] << 20) |
> -                      (amdgpu_crtc->lut_g[i] << 10) |
> -                      (amdgpu_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>
>         WREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset,
> @@ -2496,15 +2500,6 @@ static int dce_v6_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>                                    u16 *blue, uint32_t size,
>                                    struct drm_modeset_acquire_ctx *ctx)
>  {
> -       struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -       int i;
> -
> -       /* userspace palettes are always correct as is */
> -       for (i = 0; i < size; i++) {
> -               amdgpu_crtc->lut_r[i] = red[i] >> 6;
> -               amdgpu_crtc->lut_g[i] = green[i] >> 6;
> -               amdgpu_crtc->lut_b[i] = blue[i] >> 6;
> -       }
>         dce_v6_0_crtc_load_lut(crtc);
>
>         return 0;
> @@ -2712,14 +2707,12 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
>         .mode_set_base_atomic = dce_v6_0_crtc_set_base_atomic,
>         .prepare = dce_v6_0_crtc_prepare,
>         .commit = dce_v6_0_crtc_commit,
> -       .load_lut = dce_v6_0_crtc_load_lut,
>         .disable = dce_v6_0_crtc_disable,
>  };
>
>  static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>  {
>         struct amdgpu_crtc *amdgpu_crtc;
> -       int i;
>
>         amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
>                               (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
> @@ -2737,12 +2730,6 @@ static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>         adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
>         adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
>
> -       for (i = 0; i < 256; i++) {
> -               amdgpu_crtc->lut_r[i] = i << 2;
> -               amdgpu_crtc->lut_g[i] = i << 2;
> -               amdgpu_crtc->lut_b[i] = i << 2;
> -       }
> -
>         amdgpu_crtc->crtc_offset = crtc_offsets[amdgpu_crtc->crtc_id];
>
>         amdgpu_crtc->pll_id = ATOM_PPLL_INVALID;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index a9e869554627..4eb63f6a41ab 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -2124,6 +2124,7 @@ static void dce_v8_0_crtc_load_lut(struct drm_crtc *crtc)
>         struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct amdgpu_device *adev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>
>         DRM_DEBUG_KMS("%d\n", amdgpu_crtc->crtc_id);
> @@ -2153,11 +2154,14 @@ static void dce_v8_0_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x00000007);
>
>         WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
> -                      (amdgpu_crtc->lut_r[i] << 20) |
> -                      (amdgpu_crtc->lut_g[i] << 10) |
> -                      (amdgpu_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>
>         WREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset,
> @@ -2475,15 +2479,6 @@ static int dce_v8_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>                                    u16 *blue, uint32_t size,
>                                    struct drm_modeset_acquire_ctx *ctx)
>  {
> -       struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -       int i;
> -
> -       /* userspace palettes are always correct as is */
> -       for (i = 0; i < size; i++) {
> -               amdgpu_crtc->lut_r[i] = red[i] >> 6;
> -               amdgpu_crtc->lut_g[i] = green[i] >> 6;
> -               amdgpu_crtc->lut_b[i] = blue[i] >> 6;
> -       }
>         dce_v8_0_crtc_load_lut(crtc);
>
>         return 0;
> @@ -2702,14 +2697,12 @@ static const struct drm_crtc_helper_funcs dce_v8_0_crtc_helper_funcs = {
>         .mode_set_base_atomic = dce_v8_0_crtc_set_base_atomic,
>         .prepare = dce_v8_0_crtc_prepare,
>         .commit = dce_v8_0_crtc_commit,
> -       .load_lut = dce_v8_0_crtc_load_lut,
>         .disable = dce_v8_0_crtc_disable,
>  };
>
>  static int dce_v8_0_crtc_init(struct amdgpu_device *adev, int index)
>  {
>         struct amdgpu_crtc *amdgpu_crtc;
> -       int i;
>
>         amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
>                               (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
> @@ -2727,12 +2720,6 @@ static int dce_v8_0_crtc_init(struct amdgpu_device *adev, int index)
>         adev->ddev->mode_config.cursor_width = amdgpu_crtc->max_cursor_width;
>         adev->ddev->mode_config.cursor_height = amdgpu_crtc->max_cursor_height;
>
> -       for (i = 0; i < 256; i++) {
> -               amdgpu_crtc->lut_r[i] = i << 2;
> -               amdgpu_crtc->lut_g[i] = i << 2;
> -               amdgpu_crtc->lut_b[i] = i << 2;
> -       }
> -
>         amdgpu_crtc->crtc_offset = crtc_offsets[amdgpu_crtc->crtc_id];
>
>         amdgpu_crtc->pll_id = ATOM_PPLL_INVALID;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> index 90bb08309a53..ecf34bc77a63 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> @@ -168,16 +168,6 @@ static int dce_virtual_crtc_gamma_set(struct drm_crtc *crtc, u16 *red,
>                                       u16 *green, u16 *blue, uint32_t size,
>                                       struct drm_modeset_acquire_ctx *ctx)
>  {
> -       struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -       int i;
> -
> -       /* userspace palettes are always correct as is */
> -       for (i = 0; i < size; i++) {
> -               amdgpu_crtc->lut_r[i] = red[i] >> 6;
> -               amdgpu_crtc->lut_g[i] = green[i] >> 6;
> -               amdgpu_crtc->lut_b[i] = blue[i] >> 6;
> -       }
> -
>         return 0;
>  }
>
> @@ -289,11 +279,6 @@ static int dce_virtual_crtc_set_base(struct drm_crtc *crtc, int x, int y,
>         return 0;
>  }
>
> -static void dce_virtual_crtc_load_lut(struct drm_crtc *crtc)
> -{
> -       return;
> -}
> -
>  static int dce_virtual_crtc_set_base_atomic(struct drm_crtc *crtc,
>                                          struct drm_framebuffer *fb,
>                                          int x, int y, enum mode_set_atomic state)
> @@ -309,14 +294,12 @@ static const struct drm_crtc_helper_funcs dce_virtual_crtc_helper_funcs = {
>         .mode_set_base_atomic = dce_virtual_crtc_set_base_atomic,
>         .prepare = dce_virtual_crtc_prepare,
>         .commit = dce_virtual_crtc_commit,
> -       .load_lut = dce_virtual_crtc_load_lut,
>         .disable = dce_virtual_crtc_disable,
>  };
>
>  static int dce_virtual_crtc_init(struct amdgpu_device *adev, int index)
>  {
>         struct amdgpu_crtc *amdgpu_crtc;
> -       int i;
>
>         amdgpu_crtc = kzalloc(sizeof(struct amdgpu_crtc) +
>                               (AMDGPUFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
> @@ -329,12 +312,6 @@ static int dce_virtual_crtc_init(struct amdgpu_device *adev, int index)
>         amdgpu_crtc->crtc_id = index;
>         adev->mode_info.crtcs[index] = amdgpu_crtc;
>
> -       for (i = 0; i < 256; i++) {
> -               amdgpu_crtc->lut_r[i] = i << 2;
> -               amdgpu_crtc->lut_g[i] = i << 2;
> -               amdgpu_crtc->lut_b[i] = i << 2;
> -       }
> -
>         amdgpu_crtc->pll_id = ATOM_PPLL_INVALID;
>         amdgpu_crtc->encoder = NULL;
>         amdgpu_crtc->connector = NULL;
> --
> 2.11.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 12/14] drm: radeon: remove dead code and pointless local lut storage
@ 2017-07-14 14:06     ` Alex Deucher
  0 siblings, 0 replies; 35+ messages in thread
From: Alex Deucher @ 2017-07-14 14:06 UTC (permalink / raw)
  To: Peter Rosin
  Cc: LKML, Jani Nikula, Boris Brezillon, Lionel Landwerlin,
	David Airlie, Maling list - DRI developers, Christian König,
	Sean Paul, amd-gfx list, Alex Deucher, Daniel Vetter

On Thu, Jul 13, 2017 at 12:25 PM, Peter Rosin <peda@axentia.se> wrote:
> 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.
>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Peter Rosin <peda@axentia.se>

Acked-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  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 3c492a0aa6bd..02baaaf20e9d 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 27affbde058c..2f642cbefd8e 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 17d3dafc8319..8b7d7a0d3ca8 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -42,6 +42,7 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
>         struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct radeon_device *rdev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>
>         DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
> @@ -60,11 +61,14 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(AVIVO_DC_LUT_WRITE_EN_MASK, 0x0000003f);
>
>         WREG8(AVIVO_DC_LUT_RW_INDEX, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(AVIVO_DC_LUT_30_COLOR,
> -                            (radeon_crtc->lut_r[i] << 20) |
> -                            (radeon_crtc->lut_g[i] << 10) |
> -                            (radeon_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>
>         /* Only change bit 0 of LUT_SEL, other bits are set elsewhere */
> @@ -76,6 +80,7 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
>         struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct radeon_device *rdev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>
>         DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
> @@ -93,11 +98,14 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(EVERGREEN_DC_LUT_WRITE_EN_MASK + radeon_crtc->crtc_offset, 0x00000007);
>
>         WREG32(EVERGREEN_DC_LUT_RW_INDEX + radeon_crtc->crtc_offset, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(EVERGREEN_DC_LUT_30_COLOR + radeon_crtc->crtc_offset,
> -                      (radeon_crtc->lut_r[i] << 20) |
> -                      (radeon_crtc->lut_g[i] << 10) |
> -                      (radeon_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>  }
>
> @@ -106,6 +114,7 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
>         struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct radeon_device *rdev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>
>         DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
> @@ -135,11 +144,14 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(EVERGREEN_DC_LUT_WRITE_EN_MASK + radeon_crtc->crtc_offset, 0x00000007);
>
>         WREG32(EVERGREEN_DC_LUT_RW_INDEX + radeon_crtc->crtc_offset, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(EVERGREEN_DC_LUT_30_COLOR + radeon_crtc->crtc_offset,
> -                      (radeon_crtc->lut_r[i] << 20) |
> -                      (radeon_crtc->lut_g[i] << 10) |
> -                      (radeon_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>
>         WREG32(NI_DEGAMMA_CONTROL + radeon_crtc->crtc_offset,
> @@ -172,6 +184,7 @@ static void legacy_crtc_load_lut(struct drm_crtc *crtc)
>         struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct radeon_device *rdev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>         uint32_t dac2_cntl;
>
> @@ -183,11 +196,14 @@ static void legacy_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(RADEON_DAC_CNTL2, dac2_cntl);
>
>         WREG8(RADEON_PALETTE_INDEX, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(RADEON_PALETTE_30_DATA,
> -                            (radeon_crtc->lut_r[i] << 20) |
> -                            (radeon_crtc->lut_g[i] << 10) |
> -                            (radeon_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>  }
>
> @@ -209,41 +225,10 @@ void radeon_crtc_load_lut(struct drm_crtc *crtc)
>                 legacy_crtc_load_lut(crtc);
>  }
>
> -/** Sets the color ramps on behalf of fbcon */
> -void radeon_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
> -                             u16 blue, int regno)
> -{
> -       struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> -
> -       radeon_crtc->lut_r[regno] = red >> 6;
> -       radeon_crtc->lut_g[regno] = green >> 6;
> -       radeon_crtc->lut_b[regno] = blue >> 6;
> -}
> -
> -/** Gets the color ramps on behalf of fbcon */
> -void radeon_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
> -                             u16 *blue, int regno)
> -{
> -       struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> -
> -       *red = radeon_crtc->lut_r[regno] << 6;
> -       *green = radeon_crtc->lut_g[regno] << 6;
> -       *blue = radeon_crtc->lut_b[regno] << 6;
> -}
> -
>  static int radeon_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>                                  u16 *blue, uint32_t size,
>                                  struct drm_modeset_acquire_ctx *ctx)
>  {
> -       struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> -       int i;
> -
> -       /* userspace palettes are always correct as is */
> -       for (i = 0; i < size; i++) {
> -               radeon_crtc->lut_r[i] = red[i] >> 6;
> -               radeon_crtc->lut_g[i] = green[i] >> 6;
> -               radeon_crtc->lut_b[i] = blue[i] >> 6;
> -       }
>         radeon_crtc_load_lut(crtc);
>
>         return 0;
> diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
> index 356ad90a5238..638bcb5593c8 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 ce6cb6666212..1f1856e0b1e0 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 00f5ec5c12c7..da44ac234f64 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.11.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 12/14] drm: radeon: remove dead code and pointless local lut storage
@ 2017-07-14 14:06     ` Alex Deucher
  0 siblings, 0 replies; 35+ messages in thread
From: Alex Deucher @ 2017-07-14 14:06 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Jani Nikula, Boris Brezillon, amd-gfx list, David Airlie, LKML,
	Lionel Landwerlin, Sean Paul, Maling list - DRI developers,
	Alex Deucher, Daniel Vetter, Christian König

On Thu, Jul 13, 2017 at 12:25 PM, Peter Rosin <peda@axentia.se> wrote:
> 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.
>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Peter Rosin <peda@axentia.se>

Acked-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  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 3c492a0aa6bd..02baaaf20e9d 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 27affbde058c..2f642cbefd8e 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 17d3dafc8319..8b7d7a0d3ca8 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -42,6 +42,7 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
>         struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct radeon_device *rdev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>
>         DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
> @@ -60,11 +61,14 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(AVIVO_DC_LUT_WRITE_EN_MASK, 0x0000003f);
>
>         WREG8(AVIVO_DC_LUT_RW_INDEX, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(AVIVO_DC_LUT_30_COLOR,
> -                            (radeon_crtc->lut_r[i] << 20) |
> -                            (radeon_crtc->lut_g[i] << 10) |
> -                            (radeon_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>
>         /* Only change bit 0 of LUT_SEL, other bits are set elsewhere */
> @@ -76,6 +80,7 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
>         struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct radeon_device *rdev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>
>         DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
> @@ -93,11 +98,14 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(EVERGREEN_DC_LUT_WRITE_EN_MASK + radeon_crtc->crtc_offset, 0x00000007);
>
>         WREG32(EVERGREEN_DC_LUT_RW_INDEX + radeon_crtc->crtc_offset, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(EVERGREEN_DC_LUT_30_COLOR + radeon_crtc->crtc_offset,
> -                      (radeon_crtc->lut_r[i] << 20) |
> -                      (radeon_crtc->lut_g[i] << 10) |
> -                      (radeon_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>  }
>
> @@ -106,6 +114,7 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
>         struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct radeon_device *rdev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>
>         DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
> @@ -135,11 +144,14 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(EVERGREEN_DC_LUT_WRITE_EN_MASK + radeon_crtc->crtc_offset, 0x00000007);
>
>         WREG32(EVERGREEN_DC_LUT_RW_INDEX + radeon_crtc->crtc_offset, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(EVERGREEN_DC_LUT_30_COLOR + radeon_crtc->crtc_offset,
> -                      (radeon_crtc->lut_r[i] << 20) |
> -                      (radeon_crtc->lut_g[i] << 10) |
> -                      (radeon_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>
>         WREG32(NI_DEGAMMA_CONTROL + radeon_crtc->crtc_offset,
> @@ -172,6 +184,7 @@ static void legacy_crtc_load_lut(struct drm_crtc *crtc)
>         struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct radeon_device *rdev = dev->dev_private;
> +       u16 *r, *g, *b;
>         int i;
>         uint32_t dac2_cntl;
>
> @@ -183,11 +196,14 @@ static void legacy_crtc_load_lut(struct drm_crtc *crtc)
>         WREG32(RADEON_DAC_CNTL2, dac2_cntl);
>
>         WREG8(RADEON_PALETTE_INDEX, 0);
> +       r = crtc->gamma_store;
> +       g = r + crtc->gamma_size;
> +       b = g + crtc->gamma_size;
>         for (i = 0; i < 256; i++) {
>                 WREG32(RADEON_PALETTE_30_DATA,
> -                            (radeon_crtc->lut_r[i] << 20) |
> -                            (radeon_crtc->lut_g[i] << 10) |
> -                            (radeon_crtc->lut_b[i] << 0));
> +                      ((*r++ & 0xffc0) << 14) |
> +                      ((*g++ & 0xffc0) << 4) |
> +                      (*b++ >> 6));
>         }
>  }
>
> @@ -209,41 +225,10 @@ void radeon_crtc_load_lut(struct drm_crtc *crtc)
>                 legacy_crtc_load_lut(crtc);
>  }
>
> -/** Sets the color ramps on behalf of fbcon */
> -void radeon_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
> -                             u16 blue, int regno)
> -{
> -       struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> -
> -       radeon_crtc->lut_r[regno] = red >> 6;
> -       radeon_crtc->lut_g[regno] = green >> 6;
> -       radeon_crtc->lut_b[regno] = blue >> 6;
> -}
> -
> -/** Gets the color ramps on behalf of fbcon */
> -void radeon_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
> -                             u16 *blue, int regno)
> -{
> -       struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> -
> -       *red = radeon_crtc->lut_r[regno] << 6;
> -       *green = radeon_crtc->lut_g[regno] << 6;
> -       *blue = radeon_crtc->lut_b[regno] << 6;
> -}
> -
>  static int radeon_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>                                  u16 *blue, uint32_t size,
>                                  struct drm_modeset_acquire_ctx *ctx)
>  {
> -       struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> -       int i;
> -
> -       /* userspace palettes are always correct as is */
> -       for (i = 0; i < size; i++) {
> -               radeon_crtc->lut_r[i] = red[i] >> 6;
> -               radeon_crtc->lut_g[i] = green[i] >> 6;
> -               radeon_crtc->lut_b[i] = blue[i] >> 6;
> -       }
>         radeon_crtc_load_lut(crtc);
>
>         return 0;
> diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
> index 356ad90a5238..638bcb5593c8 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 ce6cb6666212..1f1856e0b1e0 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 00f5ec5c12c7..da44ac234f64 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.11.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 13/14] drm: stm: remove dead code and pointless local lut storage
  2017-07-13 16:25 ` [PATCH v5 13/14] drm: stm: " Peter Rosin
@ 2017-07-17  9:47     ` Philippe CORNU
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe CORNU @ 2017-07-17  9:47 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Yannick FERTRE, Benjamin Gaignard, Vincent ABRIOU, David Airlie,
	dri-devel, Daniel Vetter, Jani Nikula, Sean Paul,
	Lionel Landwerlin, Boris Brezillon



On 07/13/2017 06:25 PM, Peter Rosin wrote:
> The redundant fb helper .load_lut is no longer used, and can not
> work right without also providing the fb helpers .gamma_set and
> .gamma_get thus rendering the code in this driver suspect.
> 
> Just remove the dead code.
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Peter Rosin <peda@axentia.se>


Acked-by: Philippe Cornu <philippe.cornu@st.com>

Note: we will update stm32 clut support after your patch. Many thanks.

> ---
>   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 533176015cbb..3e95b4d1f4cc 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_atomic_enable(struct drm_crtc *crtc,
>   				    struct drm_crtc_state *old_state)
>   {
> @@ -525,7 +514,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,
>   	.mode_set_nofb = ltdc_crtc_mode_set_nofb,
>   	.atomic_flush = ltdc_crtc_atomic_flush,
>   	.atomic_enable = ltdc_crtc_atomic_enable,
> diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
> index d7a9c736ac1e..620ca5555abf 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;
>   };
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 13/14] drm: stm: remove dead code and pointless local lut storage
@ 2017-07-17  9:47     ` Philippe CORNU
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe CORNU @ 2017-07-17  9:47 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Boris Brezillon, dri-devel, Yannick FERTRE, Daniel Vetter,
	Vincent ABRIOU



On 07/13/2017 06:25 PM, Peter Rosin wrote:
> The redundant fb helper .load_lut is no longer used, and can not
> work right without also providing the fb helpers .gamma_set and
> .gamma_get thus rendering the code in this driver suspect.
> 
> Just remove the dead code.
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Peter Rosin <peda@axentia.se>


Acked-by: Philippe Cornu <philippe.cornu@st.com>

Note: we will update stm32 clut support after your patch. Many thanks.

> ---
>   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 533176015cbb..3e95b4d1f4cc 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_atomic_enable(struct drm_crtc *crtc,
>   				    struct drm_crtc_state *old_state)
>   {
> @@ -525,7 +514,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,
>   	.mode_set_nofb = ltdc_crtc_mode_set_nofb,
>   	.atomic_flush = ltdc_crtc_atomic_flush,
>   	.atomic_enable = ltdc_crtc_atomic_enable,
> diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
> index d7a9c736ac1e..620ca5555abf 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;
>   };
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 03/14] drm/fb-helper: separate the fb_setcmap helper into atomic and legacy paths
  2017-07-14 13:54   ` Daniel Vetter
@ 2017-08-03 22:49     ` Peter Rosin
  2017-08-04  9:38         ` Daniel Vetter
  0 siblings, 1 reply; 35+ messages in thread
From: Peter Rosin @ 2017-08-03 22:49 UTC (permalink / raw)
  To: linux-kernel, Boris Brezillon, dri-devel, Daniel Vetter

On 2017-07-14 15:54, Daniel Vetter wrote:
> On Thu, Jul 13, 2017 at 06:25:27PM +0200, Peter Rosin wrote:
>> The legacy path implements setcmap in terms of crtc .gamma_set.
>>
>> The atomic path implements setcmap by directly updating the crtc gamma_lut
>> property.
>>
>> This has a couple of benefits:
>> - it makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
>>   completely obsolete. They are now unused and subject for removal.
>> - atomic drivers that support clut modes get fbdev support for those from
>>   the drm core. This includes atmel-hlcdc, but perhaps others as well?
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
> 
> Ok, I merged the core parts. I'll wait with the driver stuff for a bit
> more (maybe 1-2 weeks) for more acks. Pls remind me in case I forget to
> pull them in.
> 
> Thanks a lot for doing this, great work!
> -Daniel

I don't see the rest it in drm-misc-next and you asked for it, so ping :-)

Cheers,
Peter

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 03/14] drm/fb-helper: separate the fb_setcmap helper into atomic and legacy paths
  2017-08-03 22:49     ` Peter Rosin
@ 2017-08-04  9:38         ` Daniel Vetter
  0 siblings, 0 replies; 35+ messages in thread
From: Daniel Vetter @ 2017-08-04  9:38 UTC (permalink / raw)
  To: Peter Rosin; +Cc: linux-kernel, Boris Brezillon, dri-devel, Daniel Vetter

On Fri, Aug 04, 2017 at 12:49:29AM +0200, Peter Rosin wrote:
> On 2017-07-14 15:54, Daniel Vetter wrote:
> > On Thu, Jul 13, 2017 at 06:25:27PM +0200, Peter Rosin wrote:
> >> The legacy path implements setcmap in terms of crtc .gamma_set.
> >>
> >> The atomic path implements setcmap by directly updating the crtc gamma_lut
> >> property.
> >>
> >> This has a couple of benefits:
> >> - it makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
> >>   completely obsolete. They are now unused and subject for removal.
> >> - atomic drivers that support clut modes get fbdev support for those from
> >>   the drm core. This includes atmel-hlcdc, but perhaps others as well?
> >>
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
> > 
> > Ok, I merged the core parts. I'll wait with the driver stuff for a bit
> > more (maybe 1-2 weeks) for more acks. Pls remind me in case I forget to
> > pull them in.
> > 
> > Thanks a lot for doing this, great work!
> > -Daniel
> 
> I don't see the rest it in drm-misc-next and you asked for it, so ping :-)

Done, but the last patch doesn't work anymore due to the new vbox driver.
Can you pls respin, with that additional patch added?

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 03/14] drm/fb-helper: separate the fb_setcmap helper into atomic and legacy paths
@ 2017-08-04  9:38         ` Daniel Vetter
  0 siblings, 0 replies; 35+ messages in thread
From: Daniel Vetter @ 2017-08-04  9:38 UTC (permalink / raw)
  To: Peter Rosin; +Cc: Boris Brezillon, linux-kernel, dri-devel, Daniel Vetter

On Fri, Aug 04, 2017 at 12:49:29AM +0200, Peter Rosin wrote:
> On 2017-07-14 15:54, Daniel Vetter wrote:
> > On Thu, Jul 13, 2017 at 06:25:27PM +0200, Peter Rosin wrote:
> >> The legacy path implements setcmap in terms of crtc .gamma_set.
> >>
> >> The atomic path implements setcmap by directly updating the crtc gamma_lut
> >> property.
> >>
> >> This has a couple of benefits:
> >> - it makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
> >>   completely obsolete. They are now unused and subject for removal.
> >> - atomic drivers that support clut modes get fbdev support for those from
> >>   the drm core. This includes atmel-hlcdc, but perhaps others as well?
> >>
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
> > 
> > Ok, I merged the core parts. I'll wait with the driver stuff for a bit
> > more (maybe 1-2 weeks) for more acks. Pls remind me in case I forget to
> > pull them in.
> > 
> > Thanks a lot for doing this, great work!
> > -Daniel
> 
> I don't see the rest it in drm-misc-next and you asked for it, so ping :-)

Done, but the last patch doesn't work anymore due to the new vbox driver.
Can you pls respin, with that additional patch added?

Thanks a lot.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH] staging: vboxvideo: remove dead gamma lut code
  2017-08-04  9:38         ` Daniel Vetter
  (?)
@ 2017-08-04 10:30         ` Peter Rosin
  2017-08-04 10:45           ` [RESEND PATCH] " Peter Rosin
  -1 siblings, 1 reply; 35+ messages in thread
From: Peter Rosin @ 2017-08-04 10:30 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Peter Rosin, Greg Kroah-Hartman, devel, Boris Brezillon,
	linux-kernel, dri-devel

The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code that was not doing anything
sensible anyway.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/staging/vboxvideo/vbox_fb.c   | 15 ---------------
 drivers/staging/vboxvideo/vbox_mode.c |  5 -----
 2 files changed, 20 deletions(-)

Hi Daniel,

Here it goes, but do you really need me to resend v5 14/14?

Cheers,
peda

diff --git a/drivers/staging/vboxvideo/vbox_fb.c b/drivers/staging/vboxvideo/vbox_fb.c
index 35f6d9f8c203..bf6635826159 100644
--- a/drivers/staging/vboxvideo/vbox_fb.c
+++ b/drivers/staging/vboxvideo/vbox_fb.c
@@ -317,22 +317,7 @@ static int vboxfb_create(struct drm_fb_helper *helper,
 	return 0;
 }
 
-static void vbox_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-			      u16 blue, int regno)
-{
-}
-
-static void vbox_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, int regno)
-{
-	*red = regno;
-	*green = regno;
-	*blue = regno;
-}
-
 static struct drm_fb_helper_funcs vbox_fb_helper_funcs = {
-	.gamma_set = vbox_fb_gamma_set,
-	.gamma_get = vbox_fb_gamma_get,
 	.fb_probe = vboxfb_create,
 };
 
diff --git a/drivers/staging/vboxvideo/vbox_mode.c b/drivers/staging/vboxvideo/vbox_mode.c
index f2b85f3256fa..996da1c79158 100644
--- a/drivers/staging/vboxvideo/vbox_mode.c
+++ b/drivers/staging/vboxvideo/vbox_mode.c
@@ -134,10 +134,6 @@ static int vbox_set_view(struct drm_crtc *crtc)
 	return 0;
 }
 
-static void vbox_crtc_load_lut(struct drm_crtc *crtc)
-{
-}
-
 static void vbox_crtc_dpms(struct drm_crtc *crtc, int mode)
 {
 	struct vbox_crtc *vbox_crtc = to_vbox_crtc(crtc);
@@ -330,7 +326,6 @@ static const struct drm_crtc_helper_funcs vbox_crtc_helper_funcs = {
 	.mode_set = vbox_crtc_mode_set,
 	/* .mode_set_base = vbox_crtc_mode_set_base, */
 	.disable = vbox_crtc_disable,
-	.load_lut = vbox_crtc_load_lut,
 	.prepare = vbox_crtc_prepare,
 	.commit = vbox_crtc_commit,
 };
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RESEND PATCH] staging: vboxvideo: remove dead gamma lut code
  2017-08-04 10:30         ` [PATCH] staging: vboxvideo: remove dead gamma lut code Peter Rosin
@ 2017-08-04 10:45           ` Peter Rosin
  2017-08-05 11:11               ` Hans de Goede
  2017-08-07  9:21             ` Daniel Vetter
  0 siblings, 2 replies; 35+ messages in thread
From: Peter Rosin @ 2017-08-04 10:45 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Peter Rosin, Greg Kroah-Hartman, devel, Boris Brezillon,
	linux-kernel, dri-devel, vbox-dev, Michael Thayer, Hans de Goede

The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code that was not doing anything
sensible anyway.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/staging/vboxvideo/vbox_fb.c   | 15 ---------------
 drivers/staging/vboxvideo/vbox_mode.c |  5 -----
 2 files changed, 20 deletions(-)

[This time with an improved Cc list, sorry for the noise. For new
 people, please refer to https://lkml.org/lkml/2017/7/13/593 for
 context]

Hi Daniel,

Here it goes, but do you really need me to resend v5 14/14?

Cheers,
peda

diff --git a/drivers/staging/vboxvideo/vbox_fb.c b/drivers/staging/vboxvideo/vbox_fb.c
index 35f6d9f8c203..bf6635826159 100644
--- a/drivers/staging/vboxvideo/vbox_fb.c
+++ b/drivers/staging/vboxvideo/vbox_fb.c
@@ -317,22 +317,7 @@ static int vboxfb_create(struct drm_fb_helper *helper,
 	return 0;
 }
 
-static void vbox_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-			      u16 blue, int regno)
-{
-}
-
-static void vbox_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, int regno)
-{
-	*red = regno;
-	*green = regno;
-	*blue = regno;
-}
-
 static struct drm_fb_helper_funcs vbox_fb_helper_funcs = {
-	.gamma_set = vbox_fb_gamma_set,
-	.gamma_get = vbox_fb_gamma_get,
 	.fb_probe = vboxfb_create,
 };
 
diff --git a/drivers/staging/vboxvideo/vbox_mode.c b/drivers/staging/vboxvideo/vbox_mode.c
index f2b85f3256fa..996da1c79158 100644
--- a/drivers/staging/vboxvideo/vbox_mode.c
+++ b/drivers/staging/vboxvideo/vbox_mode.c
@@ -134,10 +134,6 @@ static int vbox_set_view(struct drm_crtc *crtc)
 	return 0;
 }
 
-static void vbox_crtc_load_lut(struct drm_crtc *crtc)
-{
-}
-
 static void vbox_crtc_dpms(struct drm_crtc *crtc, int mode)
 {
 	struct vbox_crtc *vbox_crtc = to_vbox_crtc(crtc);
@@ -330,7 +326,6 @@ static const struct drm_crtc_helper_funcs vbox_crtc_helper_funcs = {
 	.mode_set = vbox_crtc_mode_set,
 	/* .mode_set_base = vbox_crtc_mode_set_base, */
 	.disable = vbox_crtc_disable,
-	.load_lut = vbox_crtc_load_lut,
 	.prepare = vbox_crtc_prepare,
 	.commit = vbox_crtc_commit,
 };
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [RESEND PATCH] staging: vboxvideo: remove dead gamma lut code
  2017-08-04 10:45           ` [RESEND PATCH] " Peter Rosin
@ 2017-08-05 11:11               ` Hans de Goede
  2017-08-07  9:21             ` Daniel Vetter
  1 sibling, 0 replies; 35+ messages in thread
From: Hans de Goede @ 2017-08-05 11:11 UTC (permalink / raw)
  To: Peter Rosin, Daniel Vetter
  Cc: Greg Kroah-Hartman, devel, Boris Brezillon, linux-kernel,
	dri-devel, vbox-dev, Michael Thayer

Hi,

On 04-08-17 12:45, Peter Rosin wrote:
> The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
> no longer used. Remove the dead code that was not doing anything
> sensible anyway.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

Thank you, patch looks good to me:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>   drivers/staging/vboxvideo/vbox_fb.c   | 15 ---------------
>   drivers/staging/vboxvideo/vbox_mode.c |  5 -----
>   2 files changed, 20 deletions(-)
> 
> [This time with an improved Cc list, sorry for the noise. For new
>   people, please refer to https://lkml.org/lkml/2017/7/13/593 for
>   context]
> 
> Hi Daniel,
> 
> Here it goes, but do you really need me to resend v5 14/14?
> 
> Cheers,
> peda
> 
> diff --git a/drivers/staging/vboxvideo/vbox_fb.c b/drivers/staging/vboxvideo/vbox_fb.c
> index 35f6d9f8c203..bf6635826159 100644
> --- a/drivers/staging/vboxvideo/vbox_fb.c
> +++ b/drivers/staging/vboxvideo/vbox_fb.c
> @@ -317,22 +317,7 @@ static int vboxfb_create(struct drm_fb_helper *helper,
>   	return 0;
>   }
>   
> -static void vbox_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
> -			      u16 blue, int regno)
> -{
> -}
> -
> -static void vbox_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
> -			      u16 *blue, int regno)
> -{
> -	*red = regno;
> -	*green = regno;
> -	*blue = regno;
> -}
> -
>   static struct drm_fb_helper_funcs vbox_fb_helper_funcs = {
> -	.gamma_set = vbox_fb_gamma_set,
> -	.gamma_get = vbox_fb_gamma_get,
>   	.fb_probe = vboxfb_create,
>   };
>   
> diff --git a/drivers/staging/vboxvideo/vbox_mode.c b/drivers/staging/vboxvideo/vbox_mode.c
> index f2b85f3256fa..996da1c79158 100644
> --- a/drivers/staging/vboxvideo/vbox_mode.c
> +++ b/drivers/staging/vboxvideo/vbox_mode.c
> @@ -134,10 +134,6 @@ static int vbox_set_view(struct drm_crtc *crtc)
>   	return 0;
>   }
>   
> -static void vbox_crtc_load_lut(struct drm_crtc *crtc)
> -{
> -}
> -
>   static void vbox_crtc_dpms(struct drm_crtc *crtc, int mode)
>   {
>   	struct vbox_crtc *vbox_crtc = to_vbox_crtc(crtc);
> @@ -330,7 +326,6 @@ static const struct drm_crtc_helper_funcs vbox_crtc_helper_funcs = {
>   	.mode_set = vbox_crtc_mode_set,
>   	/* .mode_set_base = vbox_crtc_mode_set_base, */
>   	.disable = vbox_crtc_disable,
> -	.load_lut = vbox_crtc_load_lut,
>   	.prepare = vbox_crtc_prepare,
>   	.commit = vbox_crtc_commit,
>   };
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RESEND PATCH] staging: vboxvideo: remove dead gamma lut code
@ 2017-08-05 11:11               ` Hans de Goede
  0 siblings, 0 replies; 35+ messages in thread
From: Hans de Goede @ 2017-08-05 11:11 UTC (permalink / raw)
  To: Peter Rosin, Daniel Vetter
  Cc: devel, Boris Brezillon, vbox-dev, Greg Kroah-Hartman,
	linux-kernel, dri-devel, Michael Thayer

Hi,

On 04-08-17 12:45, Peter Rosin wrote:
> The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
> no longer used. Remove the dead code that was not doing anything
> sensible anyway.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

Thank you, patch looks good to me:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>   drivers/staging/vboxvideo/vbox_fb.c   | 15 ---------------
>   drivers/staging/vboxvideo/vbox_mode.c |  5 -----
>   2 files changed, 20 deletions(-)
> 
> [This time with an improved Cc list, sorry for the noise. For new
>   people, please refer to https://lkml.org/lkml/2017/7/13/593 for
>   context]
> 
> Hi Daniel,
> 
> Here it goes, but do you really need me to resend v5 14/14?
> 
> Cheers,
> peda
> 
> diff --git a/drivers/staging/vboxvideo/vbox_fb.c b/drivers/staging/vboxvideo/vbox_fb.c
> index 35f6d9f8c203..bf6635826159 100644
> --- a/drivers/staging/vboxvideo/vbox_fb.c
> +++ b/drivers/staging/vboxvideo/vbox_fb.c
> @@ -317,22 +317,7 @@ static int vboxfb_create(struct drm_fb_helper *helper,
>   	return 0;
>   }
>   
> -static void vbox_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
> -			      u16 blue, int regno)
> -{
> -}
> -
> -static void vbox_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
> -			      u16 *blue, int regno)
> -{
> -	*red = regno;
> -	*green = regno;
> -	*blue = regno;
> -}
> -
>   static struct drm_fb_helper_funcs vbox_fb_helper_funcs = {
> -	.gamma_set = vbox_fb_gamma_set,
> -	.gamma_get = vbox_fb_gamma_get,
>   	.fb_probe = vboxfb_create,
>   };
>   
> diff --git a/drivers/staging/vboxvideo/vbox_mode.c b/drivers/staging/vboxvideo/vbox_mode.c
> index f2b85f3256fa..996da1c79158 100644
> --- a/drivers/staging/vboxvideo/vbox_mode.c
> +++ b/drivers/staging/vboxvideo/vbox_mode.c
> @@ -134,10 +134,6 @@ static int vbox_set_view(struct drm_crtc *crtc)
>   	return 0;
>   }
>   
> -static void vbox_crtc_load_lut(struct drm_crtc *crtc)
> -{
> -}
> -
>   static void vbox_crtc_dpms(struct drm_crtc *crtc, int mode)
>   {
>   	struct vbox_crtc *vbox_crtc = to_vbox_crtc(crtc);
> @@ -330,7 +326,6 @@ static const struct drm_crtc_helper_funcs vbox_crtc_helper_funcs = {
>   	.mode_set = vbox_crtc_mode_set,
>   	/* .mode_set_base = vbox_crtc_mode_set_base, */
>   	.disable = vbox_crtc_disable,
> -	.load_lut = vbox_crtc_load_lut,
>   	.prepare = vbox_crtc_prepare,
>   	.commit = vbox_crtc_commit,
>   };
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RESEND PATCH] staging: vboxvideo: remove dead gamma lut code
  2017-08-04 10:45           ` [RESEND PATCH] " Peter Rosin
  2017-08-05 11:11               ` Hans de Goede
@ 2017-08-07  9:21             ` Daniel Vetter
  2017-08-08 11:54               ` Peter Rosin
  1 sibling, 1 reply; 35+ messages in thread
From: Daniel Vetter @ 2017-08-07  9:21 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Daniel Vetter, devel, Boris Brezillon, vbox-dev,
	Greg Kroah-Hartman, linux-kernel, dri-devel, Hans de Goede,
	Michael Thayer

On Fri, Aug 04, 2017 at 12:45:06PM +0200, Peter Rosin wrote:
> The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
> no longer used. Remove the dead code that was not doing anything
> sensible anyway.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  drivers/staging/vboxvideo/vbox_fb.c   | 15 ---------------
>  drivers/staging/vboxvideo/vbox_mode.c |  5 -----
>  2 files changed, 20 deletions(-)
> 
> [This time with an improved Cc list, sorry for the noise. For new
>  people, please refer to https://lkml.org/lkml/2017/7/13/593 for
>  context]
> 
> Hi Daniel,
> 
> Here it goes, but do you really need me to resend v5 14/14?

Well it's not yet on dri-devel, but our pre-merge CI bots can't read such
instructions. They expect a clean new series that applies cleanly, as a
top-post, when resending stuff.

Anyway, applied.

Thanks, Daniel

> 
> Cheers,
> peda
> 
> diff --git a/drivers/staging/vboxvideo/vbox_fb.c b/drivers/staging/vboxvideo/vbox_fb.c
> index 35f6d9f8c203..bf6635826159 100644
> --- a/drivers/staging/vboxvideo/vbox_fb.c
> +++ b/drivers/staging/vboxvideo/vbox_fb.c
> @@ -317,22 +317,7 @@ static int vboxfb_create(struct drm_fb_helper *helper,
>  	return 0;
>  }
>  
> -static void vbox_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
> -			      u16 blue, int regno)
> -{
> -}
> -
> -static void vbox_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
> -			      u16 *blue, int regno)
> -{
> -	*red = regno;
> -	*green = regno;
> -	*blue = regno;
> -}
> -
>  static struct drm_fb_helper_funcs vbox_fb_helper_funcs = {
> -	.gamma_set = vbox_fb_gamma_set,
> -	.gamma_get = vbox_fb_gamma_get,
>  	.fb_probe = vboxfb_create,
>  };
>  
> diff --git a/drivers/staging/vboxvideo/vbox_mode.c b/drivers/staging/vboxvideo/vbox_mode.c
> index f2b85f3256fa..996da1c79158 100644
> --- a/drivers/staging/vboxvideo/vbox_mode.c
> +++ b/drivers/staging/vboxvideo/vbox_mode.c
> @@ -134,10 +134,6 @@ static int vbox_set_view(struct drm_crtc *crtc)
>  	return 0;
>  }
>  
> -static void vbox_crtc_load_lut(struct drm_crtc *crtc)
> -{
> -}
> -
>  static void vbox_crtc_dpms(struct drm_crtc *crtc, int mode)
>  {
>  	struct vbox_crtc *vbox_crtc = to_vbox_crtc(crtc);
> @@ -330,7 +326,6 @@ static const struct drm_crtc_helper_funcs vbox_crtc_helper_funcs = {
>  	.mode_set = vbox_crtc_mode_set,
>  	/* .mode_set_base = vbox_crtc_mode_set_base, */
>  	.disable = vbox_crtc_disable,
> -	.load_lut = vbox_crtc_load_lut,
>  	.prepare = vbox_crtc_prepare,
>  	.commit = vbox_crtc_commit,
>  };
> -- 
> 2.11.0
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RESEND PATCH] staging: vboxvideo: remove dead gamma lut code
  2017-08-07  9:21             ` Daniel Vetter
@ 2017-08-08 11:54               ` Peter Rosin
  2017-08-09 15:14                   ` Daniel Vetter
  0 siblings, 1 reply; 35+ messages in thread
From: Peter Rosin @ 2017-08-08 11:54 UTC (permalink / raw)
  To: Daniel Vetter, devel, Boris Brezillon, vbox-dev,
	Greg Kroah-Hartman, linux-kernel, dri-devel, Hans de Goede,
	Michael Thayer

On 2017-08-07 11:21, Daniel Vetter wrote:
> On Fri, Aug 04, 2017 at 12:45:06PM +0200, Peter Rosin wrote:
>> The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
>> no longer used. Remove the dead code that was not doing anything
>> sensible anyway.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>>  drivers/staging/vboxvideo/vbox_fb.c   | 15 ---------------
>>  drivers/staging/vboxvideo/vbox_mode.c |  5 -----
>>  2 files changed, 20 deletions(-)
>>
>> [This time with an improved Cc list, sorry for the noise. For new
>>  people, please refer to https://lkml.org/lkml/2017/7/13/593 for
>>  context]
>>
>> Hi Daniel,
>>
>> Here it goes, but do you really need me to resend v5 14/14?
> 
> Well it's not yet on dri-devel, but our pre-merge CI bots can't read such
> instructions. They expect a clean new series that applies cleanly, as a
> top-post, when resending stuff.

Ok, noted for the next time. These things seem to vary from subsystem to
subsystem, so it's not trivial to get it right w/o investing serious time
looking things up. Anyway, sorry about the trouble.

> Anyway, applied.

Thanks!

Cheers,
Peter

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RESEND PATCH] staging: vboxvideo: remove dead gamma lut code
  2017-08-08 11:54               ` Peter Rosin
@ 2017-08-09 15:14                   ` Daniel Vetter
  0 siblings, 0 replies; 35+ messages in thread
From: Daniel Vetter @ 2017-08-09 15:14 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Daniel Vetter, devel, Boris Brezillon, vbox-dev,
	Greg Kroah-Hartman, linux-kernel, dri-devel, Hans de Goede,
	Michael Thayer

On Tue, Aug 08, 2017 at 01:54:52PM +0200, Peter Rosin wrote:
> On 2017-08-07 11:21, Daniel Vetter wrote:
> > On Fri, Aug 04, 2017 at 12:45:06PM +0200, Peter Rosin wrote:
> >> The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
> >> no longer used. Remove the dead code that was not doing anything
> >> sensible anyway.
> >>
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
> >> ---
> >>  drivers/staging/vboxvideo/vbox_fb.c   | 15 ---------------
> >>  drivers/staging/vboxvideo/vbox_mode.c |  5 -----
> >>  2 files changed, 20 deletions(-)
> >>
> >> [This time with an improved Cc list, sorry for the noise. For new
> >>  people, please refer to https://lkml.org/lkml/2017/7/13/593 for
> >>  context]
> >>
> >> Hi Daniel,
> >>
> >> Here it goes, but do you really need me to resend v5 14/14?
> > 
> > Well it's not yet on dri-devel, but our pre-merge CI bots can't read such
> > instructions. They expect a clean new series that applies cleanly, as a
> > top-post, when resending stuff.
> 
> Ok, noted for the next time. These things seem to vary from subsystem to
> subsystem, so it's not trivial to get it right w/o investing serious time
> looking things up. Anyway, sorry about the trouble.

Yes I know, the kernel is absolutely terrible with minor differences in
process between subsystems. And then getting sternly reprimanded if you
don't know them all :-(

Unfortunately our CI has the same problem, it can't parse all the
different flavours of how people re-submit patches, so we just had to
standardize on something.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RESEND PATCH] staging: vboxvideo: remove dead gamma lut code
@ 2017-08-09 15:14                   ` Daniel Vetter
  0 siblings, 0 replies; 35+ messages in thread
From: Daniel Vetter @ 2017-08-09 15:14 UTC (permalink / raw)
  To: Peter Rosin
  Cc: devel, Boris Brezillon, vbox-dev, Greg Kroah-Hartman,
	linux-kernel, dri-devel, Hans de Goede, Daniel Vetter,
	Michael Thayer

On Tue, Aug 08, 2017 at 01:54:52PM +0200, Peter Rosin wrote:
> On 2017-08-07 11:21, Daniel Vetter wrote:
> > On Fri, Aug 04, 2017 at 12:45:06PM +0200, Peter Rosin wrote:
> >> The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
> >> no longer used. Remove the dead code that was not doing anything
> >> sensible anyway.
> >>
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
> >> ---
> >>  drivers/staging/vboxvideo/vbox_fb.c   | 15 ---------------
> >>  drivers/staging/vboxvideo/vbox_mode.c |  5 -----
> >>  2 files changed, 20 deletions(-)
> >>
> >> [This time with an improved Cc list, sorry for the noise. For new
> >>  people, please refer to https://lkml.org/lkml/2017/7/13/593 for
> >>  context]
> >>
> >> Hi Daniel,
> >>
> >> Here it goes, but do you really need me to resend v5 14/14?
> > 
> > Well it's not yet on dri-devel, but our pre-merge CI bots can't read such
> > instructions. They expect a clean new series that applies cleanly, as a
> > top-post, when resending stuff.
> 
> Ok, noted for the next time. These things seem to vary from subsystem to
> subsystem, so it's not trivial to get it right w/o investing serious time
> looking things up. Anyway, sorry about the trouble.

Yes I know, the kernel is absolutely terrible with minor differences in
process between subsystems. And then getting sternly reprimanded if you
don't know them all :-(

Unfortunately our CI has the same problem, it can't parse all the
different flavours of how people re-submit patches, so we just had to
standardize on something.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2017-08-09 15:14 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-13 16:25 [PATCH v5 00/14] improve the fb_setcmap helper Peter Rosin
2017-07-13 16:25 ` Peter Rosin
2017-07-13 16:25 ` [PATCH v5 01/14] drm: rename, adjust and export drm_atomic_replace_property_blob Peter Rosin
2017-07-13 16:25 ` [PATCH v5 02/14] drm/atomic-helper: update lut props directly in ..._legacy_gamma_set Peter Rosin
2017-07-13 16:25 ` [PATCH v5 03/14] drm/fb-helper: separate the fb_setcmap helper into atomic and legacy paths Peter Rosin
2017-07-14 13:54   ` Daniel Vetter
2017-08-03 22:49     ` Peter Rosin
2017-08-04  9:38       ` Daniel Vetter
2017-08-04  9:38         ` Daniel Vetter
2017-08-04 10:30         ` [PATCH] staging: vboxvideo: remove dead gamma lut code Peter Rosin
2017-08-04 10:45           ` [RESEND PATCH] " Peter Rosin
2017-08-05 11:11             ` Hans de Goede
2017-08-05 11:11               ` Hans de Goede
2017-08-07  9:21             ` Daniel Vetter
2017-08-08 11:54               ` Peter Rosin
2017-08-09 15:14                 ` Daniel Vetter
2017-08-09 15:14                   ` Daniel Vetter
2017-07-13 16:25 ` [PATCH v5 04/14] drm: amd: remove dead code and pointless local lut storage Peter Rosin
2017-07-14 14:06   ` Alex Deucher
2017-07-14 14:06     ` Alex Deucher
2017-07-13 16:25 ` [PATCH v5 05/14] drm: armada: remove dead empty functions Peter Rosin
2017-07-13 16:25 ` [PATCH v5 06/14] drm: ast: remove dead code and pointless local lut storage Peter Rosin
2017-07-13 16:25 ` [PATCH v5 07/14] drm: cirrus: " Peter Rosin
2017-07-13 16:25 ` Peter Rosin
2017-07-13 16:25 ` [PATCH v5 08/14] drm: gma500: " Peter Rosin
2017-07-13 16:25 ` [PATCH v5 09/14] drm: i915: " Peter Rosin
2017-07-13 16:25 ` [PATCH v5 10/14] drm: mgag200: " Peter Rosin
2017-07-13 16:25 ` [PATCH v5 11/14] drm: nouveau: " Peter Rosin
2017-07-13 16:25 ` [PATCH v5 12/14] drm: radeon: " Peter Rosin
2017-07-14 14:06   ` Alex Deucher
2017-07-14 14:06     ` Alex Deucher
2017-07-13 16:25 ` [PATCH v5 13/14] drm: stm: " Peter Rosin
2017-07-17  9:47   ` Philippe CORNU
2017-07-17  9:47     ` Philippe CORNU
2017-07-13 16:25 ` [PATCH v5 14/14] drm: remove unused and redundant callbacks Peter Rosin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.