On Wednesday, February 4, 2009 1:24 pm Jesse Barnes wrote:
> Kristian pointed out some problems with the set_config code (switching
> CRTCs didn't work for one) so I took a look and found a couple of problems.
> I'm still working on the CRTC switch issue, looks like the pipes don't get
> configured right if they're swapped, but this patch fixes some other bugs:
> - checks possible_crtcs in the CRTC assignment loop
> - restores previous fb in case of mode set failure
> - checks to make sure a connector has an encoder before restoring its
> crtc - adds a few more debug statements for finding out why a modeset is
> occurring
>
> Any thoughts on this?
Here's an updated one that will disable unused/re-routed encoders at mode set
time. Should fix Kristian's bug.
--
Jesse Barnes, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index 964c5eb..40c60de 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -452,6 +452,59 @@ static void drm_setup_crtcs(struct drm_device *dev)
kfree(modes);
kfree(enabled);
}
+
+/**
+ * drm_encoder_crtc_ok - can a given crtc drive a given encoder?
+ * @encoder: encoder to test
+ * @crtc: crtc to test
+ *
+ * Return false if @encoder can't be driven by @crtc, true otherwise.
+ */
+static bool drm_encoder_crtc_ok(struct drm_encoder *encoder,
+ struct drm_crtc *crtc)
+{
+ struct drm_device *dev;
+ struct drm_crtc *tmp;
+ int crtc_mask = 1;
+
+ WARN(!crtc, "checking null crtc?");
+
+ dev = crtc->dev;
+
+ list_for_each_entry(tmp, &dev->mode_config.crtc_list, head) {
+ if (tmp == crtc)
+ break;
+ crtc_mask <<= 1;
+ }
+
+ if (encoder->possible_crtcs & crtc_mask)
+ return true;
+ return false;
+}
+
+/*
+ * Check the CRTC we're going to map each output to vs. its current
+ * CRTC. If they don't match, we have to disable the output and the CRTC
+ * since the driver will have to re-route things.
+ */
+static void
+drm_crtc_prepare_encoders(struct drm_device *dev)
+{
+ struct drm_encoder_helper_funcs *encoder_funcs;
+ struct drm_encoder *encoder;
+
+ list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+ encoder_funcs = encoder->helper_private;
+ /* Disable unused encoders */
+ if (encoder->crtc == NULL)
+ (*encoder_funcs->dpms)(encoder, DRM_MODE_DPMS_OFF);
+ /* Disable encoders whose CRTC is about to change */
+ if (encoder_funcs->get_crtc &&
+ encoder->crtc != (*encoder_funcs->get_crtc)(encoder))
+ (*encoder_funcs->dpms)(encoder, DRM_MODE_DPMS_OFF);
+ }
+}
+
/**
* drm_crtc_set_mode - set a mode
* @crtc: CRTC to program
@@ -489,6 +542,8 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
if (!crtc->enabled)
return true;
+ drm_crtc_prepare_encoders(dev);
+
if (old_fb && crtc->fb) {
depth_changed = (old_fb->depth != crtc->fb->depth);
bpp_changed = (old_fb->bits_per_pixel !=
@@ -615,7 +670,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
struct drm_device *dev;
struct drm_crtc **save_crtcs, *new_crtc;
struct drm_encoder **save_encoders, *new_encoder;
- struct drm_framebuffer *old_fb;
+ struct drm_framebuffer *old_fb = NULL;
bool save_enabled;
bool mode_changed = false;
bool fb_changed = false;
@@ -666,9 +721,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
* and then just flip_or_move it */
if (set->crtc->fb != set->fb) {
/* If we have no fb then treat it as a full mode set */
- if (set->crtc->fb == NULL)
+ if (set->crtc->fb == NULL) {
+ DRM_DEBUG("crtc has no fb, full mode set\n");
mode_changed = true;
- else if ((set->fb->bits_per_pixel !=
+ } else if ((set->fb->bits_per_pixel !=
set->crtc->fb->bits_per_pixel) ||
set->fb->depth != set->crtc->fb->depth)
fb_changed = true;
@@ -680,7 +736,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
fb_changed = true;
if (set->mode && !drm_mode_equal(set->mode, &set->crtc->mode)) {
- DRM_DEBUG("modes are different\n");
+ DRM_DEBUG("modes are different, full mode set\n");
drm_mode_debug_printmodeline(&set->crtc->mode);
drm_mode_debug_printmodeline(set->mode);
mode_changed = true;
@@ -706,6 +762,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
}
if (new_encoder != connector->encoder) {
+ DRM_DEBUG("encoder changed, full mode switch\n");
mode_changed = true;
connector->encoder = new_encoder;
}
@@ -732,10 +789,20 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
if (set->connectors[ro] == connector)
new_crtc = set->crtc;
}
+
+ /* Make sure the new CRTC will work with the encoder */
+ if (new_crtc &&
+ !drm_encoder_crtc_ok(connector->encoder, new_crtc)) {
+ ret = -EINVAL;
+ goto fail_set_mode;
+ }
if (new_crtc != connector->encoder->crtc) {
+ DRM_DEBUG("crtc changed, full mode switch\n");
mode_changed = true;
connector->encoder->crtc = new_crtc;
}
+ DRM_DEBUG("setting connector %d crtc to %p\n",
+ connector->base.id, new_crtc);
}
/* mode_set_base is not a required function */
@@ -774,9 +841,11 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
fail_set_mode:
set->crtc->enabled = save_enabled;
+ set->crtc->fb = old_fb;
count = 0;
list_for_each_entry(connector, &dev->mode_config.connector_list, head)
- connector->encoder->crtc = save_crtcs[count++];
+ if (connector->encoder)
+ connector->encoder->crtc = save_crtcs[count++];
fail_no_encoder:
kfree(save_crtcs);
count = 0;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index bbdd729..0c66952 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -782,6 +782,9 @@ static void intel_crtc_mode_set(struct drm_crtc *crtc,
return;
}
+ DRM_DEBUG("setting mode on crtc %d, is_lvds: %s\n",
+ crtc->base.id, is_lvds ? "true" : "false");
+
fp = clock.n << 16 | clock.m1 << 8 | clock.m2;
dpll = DPLL_VGA_MODE_DIS;
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 0c6f0e1..28ff3ee 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -76,6 +76,7 @@ struct drm_encoder_helper_funcs {
void (*mode_set)(struct drm_encoder *encoder,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode);
+ struct drm_crtc *(*get_crtc)(struct drm_encoder *encoder);
/* detect for DAC style encoders */
enum drm_connector_status (*detect)(struct drm_encoder *encoder,
struct drm_connector *connector);
------------------------------------------------------------------------------
Create and Deploy Rich Internet Apps outside the browser with Adobe(R)AIR(TM)
software. With Adobe AIR, Ajax developers can use existing skills and code to
build responsive, highly engaging applications that combine the power of local
resources and data with the reach of the web. Download the Adobe AIR SDK and
Ajax docs to start building applications today-http://p.sf.net/sfu/adobe-com
--
_______________________________________________
Dri-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel
