Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH 35/37] drm_fb_helper: Preserve capability to use atomic kms

0 views
Skip to first unread message

Jason Wessel

unread,
Dec 23, 2009, 4:50:01 PM12/23/09
to
Commit 5349ef3127c77075ff70b2014f17ae0fbcaaf199 changed logic of when
a pixclock was valid vs invalid.

The atomic kernel mode setting used by the kernel debugger relied upon
the drm_fb_helper_check_var() to always return -EINVAL. Until a
better solution exists, this behavior will be restored.

CC: David Airlie <air...@linux.ie>
CC: Jesse Barnes <jba...@virtuousgeek.org>
CC: Clemens Ladisch <cle...@ladisch.de>
Signed-off-by: Jason Wessel <jason....@windriver.com>
---
drivers/gpu/drm/drm_fb_helper.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 80aab3c..b390fd8 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -683,6 +683,10 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,

if (var->pixclock != 0)
return -EINVAL;
+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) != -1)
+ return -EINVAL;
+#endif

/* Need to resize the fb object !!! */
if (var->xres > fb->width || var->yres > fb->height) {
--
1.6.4.rc1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Jason Wessel

unread,
Dec 23, 2009, 4:50:02 PM12/23/09
to
The kgdb test suite mimics the behavior of gdb. For the sh
architecture the pc must be decremented by 2 for software breakpoint.

CC: Paul Mundt <let...@linux-sh.org>


Signed-off-by: Jason Wessel <jason....@windriver.com>
---

drivers/misc/kgdbts.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
index fcb6ec1..7245023 100644
--- a/drivers/misc/kgdbts.c
+++ b/drivers/misc/kgdbts.c
@@ -295,6 +295,10 @@ static int check_and_rewind_pc(char *put_str, char *arg)
/* On x86 a breakpoint stop requires it to be decremented */
if (addr + 1 == kgdbts_regs.ip)
offset = -1;
+#elif defined(CONFIG_SUPERH)
+ /* On SUPERH a breakpoint stop requires it to be decremented */
+ if (addr + 2 == kgdbts_regs.pc)
+ offset = -2;
#endif
if (strcmp(arg, "silent") &&
instruction_pointer(&kgdbts_regs) + offset != addr) {
@@ -305,6 +309,8 @@ static int check_and_rewind_pc(char *put_str, char *arg)
#ifdef CONFIG_X86
/* On x86 adjust the instruction pointer if needed */
kgdbts_regs.ip += offset;
+#elif defined(CONFIG_SUPERH)
+ kgdbts_regs.pc += offset;
#endif
return 0;

Jason Wessel

unread,
Dec 23, 2009, 4:50:02 PM12/23/09
to
If the HW compression is left on, the call backs from the HW will
crash the kernel. The only time this code is called is when kernel
mode setting is in use with kgdb and the kdb shell.

The atomic display pipe handler callback will reset everything when
kgdb restores kernel to the run state.

CC: David Airlie <air...@linux.ie>
Acked-by: Jesse Barnes <jba...@virtuousgeek.org>


Signed-off-by: Jason Wessel <jason....@windriver.com>
---

drivers/gpu/drm/i915/intel_display.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e134a81..71816db 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1165,6 +1165,10 @@ static void intel_update_fbc(struct drm_crtc *crtc,
DRM_DEBUG_KMS("framebuffer not tiled, disabling compression\n");
goto out_disable;


}
+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) != -1)

+ goto out_disable;
+#endif /* CONFIG_KGDB */

if (dev_priv->display.fbc_enabled(crtc)) {
/* We can re-enable it in this case, but need to update pitch */

Jason Wessel

unread,
Dec 23, 2009, 4:50:02 PM12/23/09
to
From: Jesse Barnes <jba...@virtuousgeek.org>

---
drivers/gpu/drm/drm_fb_helper.c | 83 ++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_display.c | 93 ++++++++++++++++++++++++++++++++++
include/drm/drm_crtc_helper.h | 2 +
include/drm/drm_fb_helper.h | 4 ++
4 files changed, 182 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 1b49fa0..80aab3c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -29,6 +29,7 @@
*/
#include <linux/sysrq.h>
#include <linux/fb.h>
+#include <linux/kgdb.h>
#include "drmP.h"
#include "drm_crtc.h"
#include "drm_fb_helper.h"
@@ -233,6 +234,84 @@ int drm_fb_helper_parse_command_line(struct drm_device *dev)
return 0;
}

+#define to_fb_helper(ops) (container_of((ops), struct drm_fb_helper, kdb_ops))
+
+static int drm_fb_kdb_enter(struct dbg_kms_console_ops *ops)
+{
+ struct drm_fb_helper *helper = to_fb_helper(ops);
+ struct drm_crtc_helper_funcs *funcs;
+ int i;
+
+ if (atomic_read(&kgdb_active))
+ goto out; /* already in KDB, don't reset mode */
+
+ if (list_empty(&kernel_fb_helper_list))
+ return false;
+
+ list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
+ for (i = 0; i < helper->crtc_count; i++) {
+ struct drm_mode_set *mode_set =
+ &helper->crtc_info[i].mode_set;
+
+ if (!mode_set->crtc->enabled)
+ continue;
+
+ funcs = mode_set->crtc->helper_private;
+ funcs->mode_set_base_atomic(mode_set->crtc,
+ mode_set->fb,
+ mode_set->x,
+ mode_set->y);
+
+ }
+ }
+
+out:
+ return 0;
+}
+
+/* Find the real fb for a given fb helper CRTC */
+static struct drm_framebuffer *drm_mode_config_fb(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_crtc *c;
+
+ list_for_each_entry(c, &dev->mode_config.crtc_list, head) {
+ if (crtc->base.id == c->base.id)
+ return c->fb;
+ }
+
+ return NULL;
+}
+
+static int drm_fb_kdb_exit(struct dbg_kms_console_ops *ops)
+{
+ struct drm_fb_helper *helper = to_fb_helper(ops);
+ struct drm_crtc *crtc;
+ struct drm_crtc_helper_funcs *funcs;
+ struct drm_framebuffer *fb;
+ int i;
+
+ for (i = 0; i < helper->crtc_count; i++) {
+ struct drm_mode_set *mode_set = &helper->crtc_info[i].mode_set;
+ crtc = mode_set->crtc;
+ funcs = crtc->helper_private;
+ fb = drm_mode_config_fb(crtc);
+
+ if (!crtc->enabled)
+ continue;
+
+ if (!fb) {
+ DRM_ERROR("no fb to restore??\n");
+ continue;
+ }
+
+ funcs->mode_set_base_atomic(mode_set->crtc, fb, crtc->x,
+ crtc->y);
+ }
+
+ return 0;
+}
+
bool drm_fb_helper_force_kernel_mode(void)
{
int i = 0;
@@ -924,6 +1003,9 @@ int drm_fb_helper_single_fb_probe(struct drm_device *dev,
/* Switch back to kernel console on panic */
/* multi card linked list maybe */
if (list_empty(&kernel_fb_helper_list)) {
+ fb_helper->kdb_ops.activate_console = drm_fb_kdb_enter;
+ fb_helper->kdb_ops.restore_console = drm_fb_kdb_exit;
+ dbg_kms_console_ops_register(&fb_helper->kdb_ops);
printk(KERN_INFO "registered panic notifier\n");
atomic_notifier_chain_register(&panic_notifier_list,
&paniced);
@@ -938,6 +1020,7 @@ void drm_fb_helper_free(struct drm_fb_helper *helper)
{
list_del(&helper->kernel_fb_list);
if (list_empty(&kernel_fb_helper_list)) {
+ dbg_kms_console_ops_unregister(&helper->kdb_ops);
printk(KERN_INFO "unregistered panic notifier\n");
atomic_notifier_chain_unregister(&panic_notifier_list,
&paniced);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 52cd9b0..e134a81 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1234,6 +1234,98 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev, struct drm_gem_object *obj)
return 0;
}

+/* 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)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_framebuffer *intel_fb;
+ struct drm_i915_gem_object *obj_priv;
+ struct drm_gem_object *obj;
+ int plane = intel_crtc->plane;
+ unsigned long Start, Offset;
+ int dspbase = (plane == 0 ? DSPAADDR : DSPBADDR);
+ int dspsurf = (plane == 0 ? DSPASURF : DSPBSURF);
+ int dspstride = (plane == 0) ? DSPASTRIDE : DSPBSTRIDE;
+ int dsptileoff = (plane == 0 ? DSPATILEOFF : DSPBTILEOFF);
+ int dspcntr_reg = (plane == 0) ? DSPACNTR : DSPBCNTR;
+ u32 dspcntr;
+
+ switch (plane) {
+ case 0:
+ case 1:
+ break;
+ default:
+ DRM_ERROR("Can't update plane %d in SAREA\n", plane);
+ return -EINVAL;
+ }
+
+ intel_fb = to_intel_framebuffer(fb);
+ obj = intel_fb->obj;
+ obj_priv = obj->driver_private;
+
+ dspcntr = I915_READ(dspcntr_reg);
+ /* Mask out pixel format bits in case we change it */
+ dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
+ switch (fb->bits_per_pixel) {
+ case 8:
+ dspcntr |= DISPPLANE_8BPP;
+ break;
+ case 16:
+ if (fb->depth == 15)
+ dspcntr |= DISPPLANE_15_16BPP;
+ else
+ dspcntr |= DISPPLANE_16BPP;
+ break;
+ case 24:
+ case 32:
+ dspcntr |= DISPPLANE_32BPP_NO_ALPHA;
+ break;
+ default:
+ DRM_ERROR("Unknown color depth\n");
+ return -EINVAL;
+ }
+ if (IS_I965G(dev)) {
+ if (obj_priv->tiling_mode != I915_TILING_NONE)
+ dspcntr |= DISPPLANE_TILED;
+ else
+ dspcntr &= ~DISPPLANE_TILED;
+ }
+
+ if (IS_IRONLAKE(dev))
+ /* must disable */
+ dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
+
+ I915_WRITE(dspcntr_reg, dspcntr);
+
+ Start = obj_priv->gtt_offset;
+ Offset = y * fb->pitch + x * (fb->bits_per_pixel / 8);
+
+ DRM_DEBUG("Writing base %08lX %08lX %d %d\n", Start, Offset, x, y);
+ I915_WRITE(dspstride, fb->pitch);
+ if (IS_I965G(dev)) {
+ I915_WRITE(dspbase, Offset);
+ I915_READ(dspbase);
+ I915_WRITE(dspsurf, Start);
+ I915_READ(dspsurf);
+ I915_WRITE(dsptileoff, (y << 16) | x);
+ } else {
+ I915_WRITE(dspbase, Start + Offset);
+ I915_READ(dspbase);
+ }
+
+ if ((IS_I965G(dev) || plane == 0))
+ intel_update_fbc(crtc, &crtc->mode);
+
+ intel_wait_for_vblank(dev);
+ intel_increase_pllclock(crtc, true);
+
+ return 0;
+}
+
static int
intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *old_fb)
@@ -4243,6 +4335,7 @@ static const struct drm_crtc_helper_funcs intel_helper_funcs = {
.mode_fixup = intel_crtc_mode_fixup,
.mode_set = intel_crtc_mode_set,
.mode_set_base = intel_pipe_set_base,
+ .mode_set_base_atomic = intel_pipe_set_base_atomic,
.prepare = intel_crtc_prepare,
.commit = intel_crtc_commit,
.load_lut = intel_crtc_load_lut,
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index b29e201..4c12319 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -61,6 +61,8 @@ struct drm_crtc_helper_funcs {
/* Move the crtc on the current fb to the given position *optional* */
int (*mode_set_base)(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *old_fb);
+ int (*mode_set_base_atomic)(struct drm_crtc *crtc,
+ struct drm_framebuffer *fb, int x, int y);

/* reload the current crtc LUT */
void (*load_lut)(struct drm_crtc *crtc);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 58c892a..c4f87a5 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -30,6 +30,8 @@
#ifndef DRM_FB_HELPER_H
#define DRM_FB_HELPER_H

+#include <linux/kgdb.h>
+
struct drm_fb_helper_crtc {
uint32_t crtc_id;
struct drm_mode_set mode_set;
@@ -63,8 +65,10 @@ struct drm_fb_helper_connector {

struct drm_fb_helper {
struct drm_framebuffer *fb;
+ struct drm_framebuffer *saved_fb;
struct drm_device *dev;
struct drm_display_mode *mode;
+ struct dbg_kms_console_ops kdb_ops;
int crtc_count;
struct drm_fb_helper_crtc *crtc_info;
struct drm_fb_helper_funcs *funcs;

Jason Wessel

unread,
Dec 23, 2009, 4:50:02 PM12/23/09
to
Here are some hacks to work around mutex crashes in the drm / i915
code.

For now, this is the only way to make the kms / kgdb path safe on an
SMP system.

CC: Jesse Barnes <jba...@virtuousgeek.org>
CC: David Airlie <air...@linux.ie>


Signed-off-by: Jason Wessel <jason....@windriver.com>
---

drivers/gpu/drm/drm_fb_helper.c | 18 ++++++++++++++++++
drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++
2 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b390fd8..ac9ca4a 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -400,8 +400,14 @@ static void drm_fb_helper_on(struct fb_info *info)
!crtc->enabled)
continue;

+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) == -1)
+#endif
mutex_lock(&dev->mode_config.mutex);
crtc_funcs->dpms(crtc, DRM_MODE_DPMS_ON);
+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) == -1)
+#endif
mutex_unlock(&dev->mode_config.mutex);

/* Found a CRTC on this fb, now find encoders */
@@ -410,8 +416,14 @@ static void drm_fb_helper_on(struct fb_info *info)
struct drm_encoder_helper_funcs *encoder_funcs;

encoder_funcs = encoder->helper_private;
+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) == -1)
+#endif
mutex_lock(&dev->mode_config.mutex);
encoder_funcs->dpms(encoder, DRM_MODE_DPMS_ON);
+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) == -1)
+#endif
mutex_unlock(&dev->mode_config.mutex);
}
}
@@ -828,8 +840,14 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
modeset->y = var->yoffset;

if (modeset->num_connectors) {
+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) == -1)
+#endif
mutex_lock(&dev->mode_config.mutex);
ret = crtc->funcs->set_config(modeset);
+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) == -1)
+#endif
mutex_unlock(&dev->mode_config.mutex);
if (!ret) {
info->var.xoffset = var->xoffset;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 71816db..669f9b4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -949,6 +949,12 @@ intel_find_pll_g4x_dp(const intel_limit_t *limit, struct drm_crtc *crtc,
void
intel_wait_for_vblank(struct drm_device *dev)
{


+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) != -1) {

+ mdelay(20);
+ return;
+ }
+#endif /* CONFIG_KGDB */
/* Wait for 20ms, i.e. one cycle at 50hz. */
msleep(20);
}
@@ -1371,9 +1377,15 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
obj = intel_fb->obj;
obj_priv = obj->driver_private;

+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) == -1)
+#endif
mutex_lock(&dev->struct_mutex);
ret = intel_pin_and_fence_fb_obj(dev, obj);
if (ret != 0) {
+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) == -1)
+#endif
mutex_unlock(&dev->struct_mutex);
return ret;
}
@@ -1381,6 +1393,9 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
ret = i915_gem_object_set_to_gtt_domain(obj, 1);
if (ret != 0) {
i915_gem_object_unpin(obj);
+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) == -1)
+#endif
mutex_unlock(&dev->struct_mutex);
return ret;
}
@@ -1408,6 +1423,9 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
default:


DRM_ERROR("Unknown color depth\n");

i915_gem_object_unpin(obj);
+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) == -1)
+#endif
mutex_unlock(&dev->struct_mutex);
return -EINVAL;
}
@@ -1452,6 +1470,9 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
}
intel_increase_pllclock(crtc, true);

+#ifdef CONFIG_KGDB
+ if (atomic_read(&kgdb_active) == -1)
+#endif
mutex_unlock(&dev->struct_mutex);

if (!dev->primary->master)

Paul Mundt

unread,
Dec 23, 2009, 11:00:02 PM12/23/09
to
On Wed, Dec 23, 2009 at 03:43:04PM -0600, Jason Wessel wrote:
> The kgdb test suite mimics the behavior of gdb. For the sh
> architecture the pc must be decremented by 2 for software breakpoint.
>
> CC: Paul Mundt <let...@linux-sh.org>
> Signed-off-by: Jason Wessel <jason....@windriver.com>
> ---
> drivers/misc/kgdbts.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
This is identical to what I had hacked up locally, so it certainly works
fine.

Acked-by: Paul Mundt <let...@linux-sh.org>

Jesse Barnes

unread,
Jan 4, 2010, 12:30:03 PM1/4/10
to
Can you send this one over to Dave Airlie <air...@linux.ie>? At KS he
indicated he'd be happy to just apply it, since other drivers will need
it too (along with the driver specific code of course).

Jesse


--
Jesse Barnes, Intel Open Source Technology Center

0 new messages