Passing in the old fb, having overwritten the current fb, leads to
some neatly convoluted code. It's much simpler if we defer the
crtc->fb update to the place that updates the hw, in pipe_set_base.
This way we also don't need to restore anything in case something
fails - we only update crtc->fb once things have succeeded.
The real reason for this change is that now we keep the old fb
assigned to crtc->fb, which allows us to finally move the crtc disable
case into the common low-level set_mode function in the next patch.
Also don't clobber crtc->x and crtc->y, we neatly pass these down the
callchain already. Unfortunately we can't do the same with crtc->mode,
because that one is being used in the mode_set callbacks.
v2: Don't restore the drm_crtc object any more on failed modesets,
since we've lose an fb reference otherwise. Also (and this is the
reason this has been found), this totally confused the modeset state
tracking, since it clobbers crtc->enabled. Issue reported by Paulo
Zanoni.
v3: Rip out the entire crtc saving into struct intel_set_config, not
just the restoring part.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
static int
intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
static int
intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
- struct drm_framebuffer *old_fb)
+ struct drm_framebuffer *fb)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_master_private *master_priv;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_master_private *master_priv;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_framebuffer *old_fb;
int ret;
/* no fb bound */
int ret;
/* no fb bound */
DRM_ERROR("No FB bound\n");
return 0;
}
DRM_ERROR("No FB bound\n");
return 0;
}
mutex_lock(&dev->struct_mutex);
ret = intel_pin_and_fence_fb_obj(dev,
mutex_lock(&dev->struct_mutex);
ret = intel_pin_and_fence_fb_obj(dev,
- to_intel_framebuffer(crtc->fb)->obj,
+ to_intel_framebuffer(fb)->obj,
NULL);
if (ret != 0) {
mutex_unlock(&dev->struct_mutex);
NULL);
if (ret != 0) {
mutex_unlock(&dev->struct_mutex);
- if (old_fb)
- intel_finish_fb(old_fb);
+ if (crtc->fb)
+ intel_finish_fb(crtc->fb);
- ret = dev_priv->display.update_plane(crtc, crtc->fb, x, y);
+ ret = dev_priv->display.update_plane(crtc, fb, x, y);
- intel_unpin_fb_obj(to_intel_framebuffer(crtc->fb)->obj);
+ intel_unpin_fb_obj(to_intel_framebuffer(fb)->obj);
mutex_unlock(&dev->struct_mutex);
DRM_ERROR("failed to update base address\n");
return ret;
}
mutex_unlock(&dev->struct_mutex);
DRM_ERROR("failed to update base address\n");
return ret;
}
+ old_fb = crtc->fb;
+ crtc->fb = fb;
+
if (old_fb) {
intel_wait_for_vblank(dev, intel_crtc->pipe);
intel_unpin_fb_obj(to_intel_framebuffer(old_fb)->obj);
if (old_fb) {
intel_wait_for_vblank(dev, intel_crtc->pipe);
intel_unpin_fb_obj(to_intel_framebuffer(old_fb)->obj);
* true if they don't match).
*/
static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
* true if they don't match).
*/
static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
unsigned int *pipe_bpp,
struct drm_display_mode *mode)
{
unsigned int *pipe_bpp,
struct drm_display_mode *mode)
{
* also stays within the max display bpc discovered above.
*/
* also stays within the max display bpc discovered above.
*/
- switch (crtc->fb->depth) {
case 8:
bpc = 8; /* since we go through a colormap */
break;
case 8:
bpc = 8; /* since we go through a colormap */
break;
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode,
int x, int y,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode,
int x, int y,
- struct drm_framebuffer *old_fb)
+ struct drm_framebuffer *fb)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
I915_WRITE(DSPCNTR(plane), dspcntr);
POSTING_READ(DSPCNTR(plane));
I915_WRITE(DSPCNTR(plane), dspcntr);
POSTING_READ(DSPCNTR(plane));
- ret = intel_pipe_set_base(crtc, x, y, old_fb);
+ ret = intel_pipe_set_base(crtc, x, y, fb);
intel_update_watermarks(dev);
intel_update_watermarks(dev);
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode,
int x, int y,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode,
int x, int y,
- struct drm_framebuffer *old_fb)
+ struct drm_framebuffer *fb)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
/* determine panel color depth */
temp = I915_READ(PIPECONF(pipe));
temp &= ~PIPE_BPC_MASK;
/* determine panel color depth */
temp = I915_READ(PIPECONF(pipe));
temp &= ~PIPE_BPC_MASK;
- dither = intel_choose_pipe_bpp_dither(crtc, &pipe_bpp, mode);
+ dither = intel_choose_pipe_bpp_dither(crtc, fb, &pipe_bpp, mode);
switch (pipe_bpp) {
case 18:
temp |= PIPE_6BPC;
switch (pipe_bpp) {
case 18:
temp |= PIPE_6BPC;
I915_WRITE(DSPCNTR(plane), dspcntr);
POSTING_READ(DSPCNTR(plane));
I915_WRITE(DSPCNTR(plane), dspcntr);
POSTING_READ(DSPCNTR(plane));
- ret = intel_pipe_set_base(crtc, x, y, old_fb);
+ ret = intel_pipe_set_base(crtc, x, y, fb);
intel_update_watermarks(dev);
intel_update_watermarks(dev);
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode,
int x, int y,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode,
int x, int y,
- struct drm_framebuffer *old_fb)
+ struct drm_framebuffer *fb)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
drm_vblank_pre_modeset(dev, pipe);
ret = dev_priv->display.crtc_mode_set(crtc, mode, adjusted_mode,
drm_vblank_pre_modeset(dev, pipe);
ret = dev_priv->display.crtc_mode_set(crtc, mode, adjusted_mode,
drm_vblank_post_modeset(dev, pipe);
return ret;
drm_vblank_post_modeset(dev, pipe);
return ret;
struct drm_encoder *encoder = &intel_encoder->base;
struct drm_crtc *crtc = NULL;
struct drm_device *dev = encoder->dev;
struct drm_encoder *encoder = &intel_encoder->base;
struct drm_crtc *crtc = NULL;
struct drm_device *dev = encoder->dev;
- struct drm_framebuffer *old_fb;
+ struct drm_framebuffer *fb;
int i = -1;
DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
int i = -1;
DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
if (!mode)
mode = &load_detect_mode;
if (!mode)
mode = &load_detect_mode;
/* We need a framebuffer large enough to accommodate all accesses
* that the plane may generate whilst we perform load detection.
* We can not rely on the fbcon either being present (we get called
/* We need a framebuffer large enough to accommodate all accesses
* that the plane may generate whilst we perform load detection.
* We can not rely on the fbcon either being present (we get called
* not even exist) or that it is large enough to satisfy the
* requested mode.
*/
* not even exist) or that it is large enough to satisfy the
* requested mode.
*/
- crtc->fb = mode_fits_in_fbdev(dev, mode);
- if (crtc->fb == NULL) {
+ fb = mode_fits_in_fbdev(dev, mode);
+ if (fb == NULL) {
DRM_DEBUG_KMS("creating tmp fb for load-detection\n");
DRM_DEBUG_KMS("creating tmp fb for load-detection\n");
- crtc->fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32);
- old->release_fb = crtc->fb;
+ fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32);
+ old->release_fb = fb;
} else
DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n");
} else
DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n");
- if (IS_ERR(crtc->fb)) {
DRM_DEBUG_KMS("failed to allocate framebuffer for load-detection\n");
goto fail;
}
DRM_DEBUG_KMS("failed to allocate framebuffer for load-detection\n");
goto fail;
}
- if (!intel_set_mode(crtc, mode, 0, 0, old_fb)) {
+ if (!intel_set_mode(crtc, mode, 0, 0, fb)) {
DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
if (old->release_fb)
old->release_fb->funcs->destroy(old->release_fb);
DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
if (old->release_fb)
old->release_fb->funcs->destroy(old->release_fb);
fail:
connector->encoder = NULL;
encoder->crtc = NULL;
fail:
connector->encoder = NULL;
encoder->crtc = NULL;
bool intel_set_mode(struct drm_crtc *crtc,
struct drm_display_mode *mode,
bool intel_set_mode(struct drm_crtc *crtc,
struct drm_display_mode *mode,
- int x, int y, struct drm_framebuffer *old_fb)
+ int x, int y, struct drm_framebuffer *fb)
{
struct drm_device *dev = crtc->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
struct drm_display_mode *adjusted_mode, saved_mode, saved_hwmode;
struct drm_encoder_helper_funcs *encoder_funcs;
{
struct drm_device *dev = crtc->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
struct drm_display_mode *adjusted_mode, saved_mode, saved_hwmode;
struct drm_encoder_helper_funcs *encoder_funcs;
struct drm_encoder *encoder;
bool ret = true;
struct drm_encoder *encoder;
bool ret = true;
saved_hwmode = crtc->hwmode;
saved_mode = crtc->mode;
saved_hwmode = crtc->hwmode;
saved_mode = crtc->mode;
- saved_x = crtc->x;
- saved_y = crtc->y;
/* Update crtc values up front so the driver can rely on them for mode
* setting.
*/
crtc->mode = *mode;
/* Update crtc values up front so the driver can rely on them for mode
* setting.
*/
crtc->mode = *mode;
- crtc->x = x;
- crtc->y = y;
/* Pass our mode to the connectors and the CRTC to give them a chance to
* adjust it according to limitations or connector properties, and also
/* Pass our mode to the connectors and the CRTC to give them a chance to
* adjust it according to limitations or connector properties, and also
/* Set up the DPLL and any encoders state that needs to adjust or depend
* on the DPLL.
*/
/* Set up the DPLL and any encoders state that needs to adjust or depend
* on the DPLL.
*/
- ret = !intel_crtc_mode_set(crtc, mode, adjusted_mode, x, y, old_fb);
+ ret = !intel_crtc_mode_set(crtc, mode, adjusted_mode, x, y, fb);
encoder_funcs->mode_set(encoder, mode, adjusted_mode);
}
encoder_funcs->mode_set(encoder, mode, adjusted_mode);
}
+ crtc->x = x;
+ crtc->y = y;
+
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
dev_priv->display.crtc_enable(crtc);
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
dev_priv->display.crtc_enable(crtc);
if (!ret) {
crtc->hwmode = saved_hwmode;
crtc->mode = saved_mode;
if (!ret) {
crtc->hwmode = saved_hwmode;
crtc->mode = saved_mode;
- crtc->x = saved_x;
- crtc->y = saved_y;
kfree(config->save_connector_encoders);
kfree(config->save_encoder_crtcs);
kfree(config->save_connector_encoders);
kfree(config->save_encoder_crtcs);
- kfree(config->save_crtcs);
kfree(config);
}
static int intel_set_config_save_state(struct drm_device *dev,
struct intel_set_config *config)
{
kfree(config);
}
static int intel_set_config_save_state(struct drm_device *dev,
struct intel_set_config *config)
{
struct drm_encoder *encoder;
struct drm_connector *connector;
int count;
struct drm_encoder *encoder;
struct drm_connector *connector;
int count;
- /* Allocate space for the backup of all (non-pointer) crtc, encoder and
- * connector data. */
- config->save_crtcs = kcalloc(dev->mode_config.num_crtc,
- sizeof(struct drm_crtc), GFP_KERNEL);
- if (!config->save_crtcs)
- return -ENOMEM;
-
config->save_encoder_crtcs =
kcalloc(dev->mode_config.num_encoder,
sizeof(struct drm_crtc *), GFP_KERNEL);
config->save_encoder_crtcs =
kcalloc(dev->mode_config.num_encoder,
sizeof(struct drm_crtc *), GFP_KERNEL);
* Should anything bad happen only the expected state is
* restored, not the drivers personal bookkeeping.
*/
* Should anything bad happen only the expected state is
* restored, not the drivers personal bookkeeping.
*/
- count = 0;
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
- config->save_crtcs[count++] = *crtc;
- }
-
count = 0;
list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
config->save_encoder_crtcs[count++] = encoder->crtc;
count = 0;
list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
config->save_encoder_crtcs[count++] = encoder->crtc;
static void intel_set_config_restore_state(struct drm_device *dev,
struct intel_set_config *config)
{
static void intel_set_config_restore_state(struct drm_device *dev,
struct intel_set_config *config)
{
struct intel_encoder *encoder;
struct intel_connector *connector;
int count;
struct intel_encoder *encoder;
struct intel_connector *connector;
int count;
- count = 0;
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
- *crtc = config->save_crtcs[count++];
- }
-
count = 0;
list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) {
encoder->new_crtc =
count = 0;
list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) {
encoder->new_crtc =
static int intel_crtc_set_config(struct drm_mode_set *set)
{
struct drm_device *dev;
static int intel_crtc_set_config(struct drm_mode_set *set)
{
struct drm_device *dev;
- struct drm_framebuffer *old_fb = NULL;
struct drm_mode_set save_set;
struct intel_set_config *config;
int ret;
struct drm_mode_set save_set;
struct intel_set_config *config;
int ret;
DRM_DEBUG_KMS("attempting to set mode from"
" userspace\n");
drm_mode_debug_printmodeline(set->mode);
DRM_DEBUG_KMS("attempting to set mode from"
" userspace\n");
drm_mode_debug_printmodeline(set->mode);
- old_fb = set->crtc->fb;
- set->crtc->fb = set->fb;
if (!intel_set_mode(set->crtc, set->mode,
if (!intel_set_mode(set->crtc, set->mode,
- set->x, set->y, old_fb)) {
+ set->x, set->y, set->fb)) {
DRM_ERROR("failed to set mode on [CRTC:%d]\n",
set->crtc->base.id);
DRM_ERROR("failed to set mode on [CRTC:%d]\n",
set->crtc->base.id);
- set->crtc->fb = old_fb;
ret = -EINVAL;
goto fail;
}
ret = -EINVAL;
goto fail;
}
}
drm_helper_disable_unused_functions(dev);
} else if (config->fb_changed) {
}
drm_helper_disable_unused_functions(dev);
} else if (config->fb_changed) {
- set->crtc->x = set->x;
- set->crtc->y = set->y;
-
- old_fb = set->crtc->fb;
- if (set->crtc->fb != set->fb)
- set->crtc->fb = set->fb;
ret = intel_pipe_set_base(set->crtc,
ret = intel_pipe_set_base(set->crtc,
- set->x, set->y, old_fb);
- if (ret != 0) {
- set->crtc->fb = old_fb;
- goto fail;
- }
+ set->x, set->y, set->fb);
}
intel_set_config_free(config);
}
intel_set_config_free(config);
struct intel_set_config {
struct drm_encoder **save_connector_encoders;
struct drm_crtc **save_encoder_crtcs;
struct intel_set_config {
struct drm_encoder **save_connector_encoders;
struct drm_crtc **save_encoder_crtcs;
- struct drm_crtc *save_crtcs;
bool fb_changed;
bool mode_changed;
bool fb_changed;
bool mode_changed;