[PATCH 56/81] drm/i915: push crtc->fb update into pipe_set_base

Daniel Vetter
2012-07-11T15:36:25Z

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.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 84 ++++++++++++++--------------------
1 files changed, 34 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c5bf814..19ef083 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2198,16 +2198,17 @@ intel_finish_fb(struct drm_framebuffer *old_fb)

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_framebuffer *old_fb;
int ret;

/* no fb bound */
- if (!crtc->fb) {
+ if (!fb) {
DRM_ERROR("No FB bound\n");
return 0;
}
@@ -2221,7 +2222,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,

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);
@@ -2229,17 +2230,20 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
return ret;
}

- 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);
if (ret) {
- 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;
}

+ 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);
@@ -3765,6 +3769,7 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
* 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)
{
@@ -3845,7 +3850,7 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
* also stays within the max display bpc discovered above.
*/

- switch (crtc->fb->depth) {
+ switch (fb->depth) {
case 8:
bpc = 8; /* since we go through a colormap */
break;
@@ -4264,7 +4269,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
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;
@@ -4454,7 +4459,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
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);

@@ -4612,7 +4617,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
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;
@@ -4732,7 +4737,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
/* 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;
@@ -5001,7 +5006,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
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);

@@ -5014,7 +5019,7 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
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;
@@ -5025,7 +5030,7 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
drm_vblank_pre_modeset(dev, pipe);

ret = dev_priv->display.crtc_mode_set(crtc, mode, adjusted_mode,
- x, y, old_fb);
+ x, y, fb);
drm_vblank_post_modeset(dev, pipe);

return ret;
@@ -5635,7 +5640,7 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
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",
@@ -5696,8 +5701,6 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
if (!mode)
mode = &load_detect_mode;

- old_fb = crtc->fb;
-
/* 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
@@ -5705,19 +5708,19 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
* 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");
- 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");
- if (IS_ERR(crtc->fb)) {
+ if (IS_ERR(fb)) {
DRM_DEBUG_KMS("failed to allocate framebuffer for load-detection\n");
goto fail;
}

- if (!intel_crtc_set_mode(crtc, mode, 0, 0, old_fb)) {
+ if (!intel_crtc_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);
@@ -5731,7 +5734,6 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
fail:
connector->encoder = NULL;
encoder->crtc = NULL;
- crtc->fb = old_fb;
return false;
}

@@ -6667,13 +6669,12 @@ static void intel_modeset_commit_output_state(struct drm_device *dev)
bool intel_crtc_set_mode(struct drm_crtc *crtc,
struct drm_display_mode *mode,
int x, int y,
- struct drm_framebuffer *old_fb)
+ 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;
- int saved_x, saved_y;
struct drm_encoder *encoder;
bool ret = true;

@@ -6687,15 +6688,11 @@ bool intel_crtc_set_mode(struct drm_crtc *crtc,

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;
- 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
@@ -6726,7 +6723,7 @@ bool intel_crtc_set_mode(struct drm_crtc *crtc,
/* 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);
if (!ret)
goto done;

@@ -6742,6 +6739,9 @@ bool intel_crtc_set_mode(struct drm_crtc *crtc,
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);

@@ -6760,8 +6760,6 @@ done:
if (!ret) {
crtc->hwmode = saved_hwmode;
crtc->mode = saved_mode;
- crtc->x = saved_x;
- crtc->y = saved_y;
}

return ret;
@@ -6990,7 +6988,6 @@ next_encoder:
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;
@@ -7053,14 +7050,11 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
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_crtc_set_mode(set->crtc, set->mode,
set->x, set->y,
- old_fb)) {
+ set->fb)) {
DRM_ERROR("failed to set mode on [CRTC:%d]\n",
set->crtc->base.id);
- set->crtc->fb = old_fb;
ret = -EINVAL;
goto fail;
}
@@ -7073,18 +7067,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
}
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,
- 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);
--
1.7.7.6