[PATCH 0/3] atomic kernel mode setting fixups for drm-core-next

[PATCH 0/3] atomic kernel mode setting fixups for drm-core-next

[Jason Wessel]

This patch series contains fixes against the original merge of the
atomic kernel mode setting for the radeon and nouveau drivers.  The
first change is a revert of a prior patch that was radeon specific.
It turns out, as David Airlie suspected, that a generic solution is
needed for saving and restoring the R G B gamma values when using
atomic kernel mode setting.

The final patch in the series is a fix up per prior comments from
Jesse Barnes about using enums vs passing lots of true and false
values in the drm API calls.


Thanks,
Jason.

---
The following changes since commit 26bf62e47261142d528a6109fdd671a2e280b4ea:
  Dave Airlie (1):
        Merge branch 'drm-radeon-next' of ../drm-radeon-next into drm-core-next

--

Jason Wessel (3):
      Revert "radeon, kdb, kms: Save and restore the LUT on atomic KMS enter/exit"
      kdb,kms: Save and restore the LUT on atomic KMS enter/exit
      drm,kdb,kms: Change mode_set_base_atomic() enter argument to be an enum

 drivers/gpu/drm/drm_fb_helper.c             |   31 +++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_display.c        |    5 ++-
 drivers/gpu/drm/nouveau/nv04_crtc.c         |    4 +-
 drivers/gpu/drm/nouveau/nv50_crtc.c         |    2 +-
 drivers/gpu/drm/radeon/atombios_crtc.c      |    2 +-
 drivers/gpu/drm/radeon/radeon_display.c     |   32 ---------------------------
 drivers/gpu/drm/radeon/radeon_legacy_crtc.c |    7 +-----
 drivers/gpu/drm/radeon/radeon_mode.h        |   10 ++++----
 include/drm/drm_crtc_helper.h               |    7 +++++-
 9 files changed, 47 insertions(+), 53 deletions(-)

[PATCH 1/3] Revert "radeon, kdb, kms: Save and restore the LUT on atomic KMS enter/exit"

[Jason Wessel]

This reverts commit ff773714dd30b802c336064109c535d8b2774e2f.

A generic solution is needed to save and retore the LUT information.

CC: Jesse Barnes <jbarnes@virtuousgeek.org>
CC: David Airlie <airlied@linux.ie>
CC: dri-devel@lists.freedesktop.org
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/gpu/drm/radeon/radeon_display.c     |   32 ---------------------------
 drivers/gpu/drm/radeon/radeon_legacy_crtc.c |    5 ----
 drivers/gpu/drm/radeon/radeon_mode.h        |    3 --
 3 files changed, 0 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 6c6846c..fd70b84 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -138,38 +138,6 @@ void radeon_crtc_load_lut(struct drm_crtc *crtc)
 		legacy_crtc_load_lut(crtc);
 }
 
-void radeon_crtc_save_lut(struct drm_crtc *crtc)
-{
-	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-	int i;
-
-	if (!crtc->enabled)
-		return;
-
-	for (i = 0; i < 256; i++) {
-		radeon_crtc->lut_r_copy[i] = radeon_crtc->lut_r[i];
-		radeon_crtc->lut_g_copy[i] = radeon_crtc->lut_g[i];
-		radeon_crtc->lut_b_copy[i] = radeon_crtc->lut_b[i];
-	}
-}
-
-void radeon_crtc_restore_lut(struct drm_crtc *crtc)
-{
-	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-	int i;
-
-	if (!crtc->enabled)
-		return;
-
-	for (i = 0; i < 256; i++) {
-		radeon_crtc->lut_r[i] = radeon_crtc->lut_r_copy[i];
-		radeon_crtc->lut_g[i] = radeon_crtc->lut_g_copy[i];
-		radeon_crtc->lut_b[i] = radeon_crtc->lut_b_copy[i];
-	}
-
-	radeon_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)
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
index c0bf8b7..590f6a1 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
@@ -355,11 +355,6 @@ int radeon_crtc_set_base_atomic(struct drm_crtc *crtc,
 				struct drm_framebuffer *fb,
 				int x, int y, int enter)
 {
-	if (enter)
-		radeon_crtc_save_lut(crtc);
-	else
-		radeon_crtc_restore_lut(crtc);
-
 	return radeon_crtc_do_set_base(crtc, fb, x, y, 1);
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 3cda63e..94eb184 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -255,7 +255,6 @@ struct radeon_crtc {
 	struct drm_crtc base;
 	int crtc_id;
 	u16 lut_r[256], lut_g[256], lut_b[256];
-	u16 lut_r_copy[256], lut_g_copy[256], lut_b_copy[256];
 	bool enabled;
 	bool can_tile;
 	uint32_t crtc_offset;
@@ -517,8 +516,6 @@ extern int atombios_get_encoder_mode(struct drm_encoder *encoder);
 extern void radeon_encoder_set_active_device(struct drm_encoder *encoder);
 
 extern void radeon_crtc_load_lut(struct drm_crtc *crtc);
-extern void radeon_crtc_save_lut(struct drm_crtc *crtc);
-extern void radeon_crtc_restore_lut(struct drm_crtc *crtc);
 extern int atombios_crtc_set_base(struct drm_crtc *crtc, int x, int y,
 				   struct drm_framebuffer *old_fb);
 extern int atombios_crtc_set_base_atomic(struct drm_crtc *crtc,
-- 
1.6.3.3

[PATCH 1/3] Revert "radeon, kdb, kms: Save and restore the LUT on atomic KMS enter/exit"

[Jason Wessel]

This reverts commit ff773714dd30b802c336064109c535d8b2774e2f.

A generic solution is needed to save and retore the LUT information.

CC: Jesse Barnes <jbarnes@virtuousgeek.org>
CC: David Airlie <airlied@linux.ie>
CC: dri-devel@lists.freedesktop.org
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/gpu/drm/radeon/radeon_display.c     |   32 ---------------------------
 drivers/gpu/drm/radeon/radeon_legacy_crtc.c |    5 ----
 drivers/gpu/drm/radeon/radeon_mode.h        |    3 --
 3 files changed, 0 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 6c6846c..fd70b84 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -138,38 +138,6 @@ void radeon_crtc_load_lut(struct drm_crtc *crtc)
 		legacy_crtc_load_lut(crtc);
 }
 
-void radeon_crtc_save_lut(struct drm_crtc *crtc)
-{
-	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-	int i;
-
-	if (!crtc->enabled)
-		return;
-
-	for (i = 0; i < 256; i++) {
-		radeon_crtc->lut_r_copy[i] = radeon_crtc->lut_r[i];
-		radeon_crtc->lut_g_copy[i] = radeon_crtc->lut_g[i];
-		radeon_crtc->lut_b_copy[i] = radeon_crtc->lut_b[i];
-	}
-}
-
-void radeon_crtc_restore_lut(struct drm_crtc *crtc)
-{
-	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-	int i;
-
-	if (!crtc->enabled)
-		return;
-
-	for (i = 0; i < 256; i++) {
-		radeon_crtc->lut_r[i] = radeon_crtc->lut_r_copy[i];
-		radeon_crtc->lut_g[i] = radeon_crtc->lut_g_copy[i];
-		radeon_crtc->lut_b[i] = radeon_crtc->lut_b_copy[i];
-	}
-
-	radeon_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)
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
index c0bf8b7..590f6a1 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
@@ -355,11 +355,6 @@ int radeon_crtc_set_base_atomic(struct drm_crtc *crtc,
 				struct drm_framebuffer *fb,
 				int x, int y, int enter)
 {
-	if (enter)
-		radeon_crtc_save_lut(crtc);
-	else
-		radeon_crtc_restore_lut(crtc);
-
 	return radeon_crtc_do_set_base(crtc, fb, x, y, 1);
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 3cda63e..94eb184 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -255,7 +255,6 @@ struct radeon_crtc {
 	struct drm_crtc base;
 	int crtc_id;
 	u16 lut_r[256], lut_g[256], lut_b[256];
-	u16 lut_r_copy[256], lut_g_copy[256], lut_b_copy[256];
 	bool enabled;
 	bool can_tile;
 	uint32_t crtc_offset;
@@ -517,8 +516,6 @@ extern int atombios_get_encoder_mode(struct drm_encoder *encoder);
 extern void radeon_encoder_set_active_device(struct drm_encoder *encoder);
 
 extern void radeon_crtc_load_lut(struct drm_crtc *crtc);
-extern void radeon_crtc_save_lut(struct drm_crtc *crtc);
-extern void radeon_crtc_restore_lut(struct drm_crtc *crtc);
 extern int atombios_crtc_set_base(struct drm_crtc *crtc, int x, int y,
 				   struct drm_framebuffer *old_fb);
 extern int atombios_crtc_set_base_atomic(struct drm_crtc *crtc,
-- 
1.6.3.3

[PATCH 2/3] kdb,kms: Save and restore the LUT on atomic KMS enter/exit

[Jason Wessel]

When changing VTs non-atomically the kernel works in conjunction with
the Xserver in user space and receives the LUT information from the
Xserver via a system call.  When changing modes atomically for kdb,
this information must be saved and restored without disturbing user
space as if nothing ever happened.

There is a short cut used by this patch where gamma_store is used as
the save space.  If this turns out to be a problem in the future a
pre-allocated chunk of memory will be required for each crtc to save
and restore the LUT information.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
CC: Jesse Barnes <jbarnes@virtuousgeek.org>
CC: David Airlie <airlied@linux.ie>
CC: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_fb_helper.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 625a2d5..8208e19 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -242,6 +242,30 @@ static int drm_fb_helper_parse_command_line(struct drm_fb_helper *fb_helper)
 	return 0;
 }
 
+static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct drm_fb_helper *helper)
+{
+	uint16_t *r_base, *g_base, *b_base;
+	int i;
+
+	r_base = crtc->gamma_store;
+	g_base = r_base + crtc->gamma_size;
+	b_base = g_base + crtc->gamma_size;
+
+	for (i = 0; i < crtc->gamma_size; i++)
+		helper->funcs->gamma_get(crtc, &r_base[i], &g_base[i], &b_base[i], i);
+}
+
+static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
+{
+	uint16_t *r_base, *g_base, *b_base;
+
+	r_base = crtc->gamma_store;
+	g_base = r_base + crtc->gamma_size;
+	b_base = g_base + crtc->gamma_size;
+
+	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, 0, crtc->gamma_size);
+}
+
 int drm_fb_helper_debug_enter(struct fb_info *info)
 {
 	struct drm_fb_helper *helper = info->par;
@@ -260,6 +284,7 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
 				continue;
 
 			funcs =	mode_set->crtc->helper_private;
+			drm_fb_helper_save_lut_atomic(mode_set->crtc, helper);
 			funcs->mode_set_base_atomic(mode_set->crtc,
 						    mode_set->fb,
 						    mode_set->x,
@@ -309,6 +334,7 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
 			continue;
 		}
 
+		drm_fb_helper_restore_lut_atomic(mode_set->crtc);
 		funcs->mode_set_base_atomic(mode_set->crtc, fb, crtc->x,
 					    crtc->y, 0);
 	}
-- 
1.6.3.3

[PATCH 3/3] drm,kdb,kms: Change mode_set_base_atomic() enter argument to be an enum

[Jason Wessel]

The enter argument as implemented by commit 413d45d3627 (drm, kdb, kms:
Add an enter argument to mode_set_base_atomic() API) should be more
descriptive as to what it does vs just passing 1 and 0 around.

There is no runtime behavior change as a result of this patch.

Reported-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
CC: David Airlie <airlied@linux.ie>
CC: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_fb_helper.c             |    5 ++---
 drivers/gpu/drm/i915/intel_display.c        |    5 +++--
 drivers/gpu/drm/nouveau/nv04_crtc.c         |    4 ++--
 drivers/gpu/drm/nouveau/nv50_crtc.c         |    2 +-
 drivers/gpu/drm/radeon/atombios_crtc.c      |    2 +-
 drivers/gpu/drm/radeon/radeon_legacy_crtc.c |    2 +-
 drivers/gpu/drm/radeon/radeon_mode.h        |    7 +++++--
 include/drm/drm_crtc_helper.h               |    7 ++++++-
 8 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 8208e19..d2849e4 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -289,8 +289,7 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
 						    mode_set->fb,
 						    mode_set->x,
 						    mode_set->y,
-						    1);
-
+						    ENTER_ATOMIC_MODE_SET);
 		}
 	}
 
@@ -336,7 +335,7 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
 
 		drm_fb_helper_restore_lut_atomic(mode_set->crtc);
 		funcs->mode_set_base_atomic(mode_set->crtc, fb, crtc->x,
-					    crtc->y, 0);
+					    crtc->y, LEAVE_ATOMIC_MODE_SET);
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9109c00..96d08a9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1492,7 +1492,7 @@ err_unpin:
 /* Assume fb object is pinned & idle & fenced and just update base pointers */
 static int
 intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
-			   int x, int y, int enter)
+			   int x, int y, enum mode_set_atomic state)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1614,7 +1614,8 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 			   atomic_read(&obj_priv->pending_flip) == 0);
 	}
 
-	ret = intel_pipe_set_base_atomic(crtc, crtc->fb, x, y, 0);
+	ret = intel_pipe_set_base_atomic(crtc, crtc->fb, x, y,
+					 LEAVE_ATOMIC_MODE_SET);
 	if (ret) {
 		i915_gem_object_unpin(to_intel_framebuffer(crtc->fb)->obj);
 		mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/nouveau/nv04_crtc.c b/drivers/gpu/drm/nouveau/nv04_crtc.c
index 17f7cf0..c71abc2 100644
--- a/drivers/gpu/drm/nouveau/nv04_crtc.c
+++ b/drivers/gpu/drm/nouveau/nv04_crtc.c
@@ -860,12 +860,12 @@ nv04_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 static int
 nv04_crtc_mode_set_base_atomic(struct drm_crtc *crtc,
 			       struct drm_framebuffer *fb,
-			       int x, int y, int enter)
+			       int x, int y, enum mode_set_atomic state)
 {
 	struct drm_nouveau_private *dev_priv = crtc->dev->dev_private;
 	struct drm_device *dev = dev_priv->dev;
 
-	if (enter)
+	if (state == ENTER_ATOMIC_MODE_SET)
 		nouveau_fbcon_save_disable_accel(dev);
 	else
 		nouveau_fbcon_restore_accel(dev);
diff --git a/drivers/gpu/drm/nouveau/nv50_crtc.c b/drivers/gpu/drm/nouveau/nv50_crtc.c
index ba91bef..16380d5 100644
--- a/drivers/gpu/drm/nouveau/nv50_crtc.c
+++ b/drivers/gpu/drm/nouveau/nv50_crtc.c
@@ -708,7 +708,7 @@ nv50_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 static int
 nv50_crtc_mode_set_base_atomic(struct drm_crtc *crtc,
 			       struct drm_framebuffer *fb,
-			       int x, int y, int enter)
+			       int x, int y, enum mode_set_atomic state)
 {
 	return nv50_crtc_do_mode_set_base(crtc, fb, x, y, true, true);
 }
diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 037e326..348b367 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -1258,7 +1258,7 @@ int atombios_crtc_set_base(struct drm_crtc *crtc, int x, int y,
 
 int atombios_crtc_set_base_atomic(struct drm_crtc *crtc,
                                   struct drm_framebuffer *fb,
-                                  int x, int y, int enter)
+				  int x, int y, enum mode_set_atomic state)
 {
        struct drm_device *dev = crtc->dev;
        struct radeon_device *rdev = dev->dev_private;
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
index 590f6a1..793d880 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
@@ -353,7 +353,7 @@ int radeon_crtc_set_base(struct drm_crtc *crtc, int x, int y,
 
 int radeon_crtc_set_base_atomic(struct drm_crtc *crtc,
 				struct drm_framebuffer *fb,
-				int x, int y, int enter)
+				int x, int y, enum mode_set_atomic state)
 {
 	return radeon_crtc_do_set_base(crtc, fb, x, y, 1);
 }
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 94eb184..e1113a9 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -35,6 +35,7 @@
 #include <drm_edid.h>
 #include <drm_dp_helper.h>
 #include <drm_fixed.h>
+#include <drm_crtc_helper.h>
 #include <linux/i2c.h>
 #include <linux/i2c-id.h>
 #include <linux/i2c-algo-bit.h>
@@ -520,7 +521,8 @@ extern int atombios_crtc_set_base(struct drm_crtc *crtc, int x, int y,
 				   struct drm_framebuffer *old_fb);
 extern int atombios_crtc_set_base_atomic(struct drm_crtc *crtc,
 					 struct drm_framebuffer *fb,
-					 int x, int y, int enter);
+					 int x, int y,
+					 enum mode_set_atomic state);
 extern int atombios_crtc_mode_set(struct drm_crtc *crtc,
 				   struct drm_display_mode *mode,
 				   struct drm_display_mode *adjusted_mode,
@@ -532,7 +534,8 @@ extern int radeon_crtc_set_base(struct drm_crtc *crtc, int x, int y,
 				 struct drm_framebuffer *old_fb);
 extern int radeon_crtc_set_base_atomic(struct drm_crtc *crtc,
 				       struct drm_framebuffer *fb,
-				       int x, int y, int enter);
+				       int x, int y,
+				       enum mode_set_atomic state);
 extern int radeon_crtc_do_set_base(struct drm_crtc *crtc,
 				   struct drm_framebuffer *fb,
 				   int x, int y, int atomic);
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 6a9f393..73b0712 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -39,6 +39,11 @@
 
 #include <linux/fb.h>
 
+enum mode_set_atomic {
+	LEAVE_ATOMIC_MODE_SET,
+	ENTER_ATOMIC_MODE_SET,
+};
+
 struct drm_crtc_helper_funcs {
 	/*
 	 * Control power levels on the CRTC.  If the mode passed in is
@@ -62,7 +67,7 @@ struct drm_crtc_helper_funcs {
 			     struct drm_framebuffer *old_fb);
 	int (*mode_set_base_atomic)(struct drm_crtc *crtc,
 				    struct drm_framebuffer *fb, int x, int y,
-				    int is_enter);
+				    enum mode_set_atomic);
 
 	/* reload the current crtc LUT */
 	void (*load_lut)(struct drm_crtc *crtc);
-- 
1.6.3.3