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(-)
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,
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); }
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);
dri-devel@lists.freedesktop.org