Commit 3dd512fb authored by Matt Roper's avatar Matt Roper Committed by Daniel Vetter

drm/i915: Kill intel_crtc->cursor_{width, height} (v2)

The cursor size fields in intel_crtc just duplicate the data from
cursor->state.crtc_{w,h} so we don't need them any more.  Worse, their
use in the watermark code actually introduces a subtle bug since they
don't get updated to mirror the state values until the plane commit
stage, which is *after* we've already used them to calculate new
watermark values.  This happens because we had to move watermark updates
slightly earlier (outside vblank evasion) in commit

        commit 32b7eeec
        Author: Matt Roper <matthew.d.roper@intel.com>
        Date:   Wed Dec 24 07:59:06 2014 -0800

            drm/i915: Refactor work that can sleep out of commit (v7)

Dropping the intel_crtc fields and just using the state values (which
are properly updated by the time watermark updates happen) should solve
the problem.

Aside from the actual removal of the struct fields (which are formatted
in a way that I couldn't figure out how to match in Coccinelle), the
rest of this patch was generated via the following semantic patch:

        // Drop assignment
        @@
        struct intel_crtc *C;
        struct drm_plane_state S;
        @@
        (
        - C->cursor_width = S.crtc_w;
        |
        - C->cursor_height = S.crtc_h;
        )

        // Replace usage
        @@
        struct intel_crtc *C;
        expression E;
        @@
        (
        - C->cursor_width
        + C->base.cursor->state->crtc_w
        |
        - C->cursor_height
        + C->base.cursor->state->crtc_h
        |
        - to_intel_crtc(E)->cursor_width
        + E->cursor->state->crtc_w
        |
        - to_intel_crtc(E)->cursor_height
        + E->cursor->state->crtc_h
        )

v2: Rebase

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Joe Konno <joe.konno@linux.intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89346Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 03be7005
...@@ -2677,7 +2677,8 @@ static int i915_display_info(struct seq_file *m, void *unused) ...@@ -2677,7 +2677,8 @@ static int i915_display_info(struct seq_file *m, void *unused)
active = cursor_position(dev, crtc->pipe, &x, &y); active = cursor_position(dev, crtc->pipe, &x, &y);
seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x, active? %s\n", seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x, active? %s\n",
yesno(crtc->cursor_base), yesno(crtc->cursor_base),
x, y, crtc->cursor_width, crtc->cursor_height, x, y, crtc->base.cursor->state->crtc_w,
crtc->base.cursor->state->crtc_h,
crtc->cursor_addr, yesno(active)); crtc->cursor_addr, yesno(active));
} }
......
...@@ -8409,8 +8409,8 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base) ...@@ -8409,8 +8409,8 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base)
uint32_t cntl = 0, size = 0; uint32_t cntl = 0, size = 0;
if (base) { if (base) {
unsigned int width = intel_crtc->cursor_width; unsigned int width = intel_crtc->base.cursor->state->crtc_w;
unsigned int height = intel_crtc->cursor_height; unsigned int height = intel_crtc->base.cursor->state->crtc_h;
unsigned int stride = roundup_pow_of_two(width) * 4; unsigned int stride = roundup_pow_of_two(width) * 4;
switch (stride) { switch (stride) {
...@@ -8474,7 +8474,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base) ...@@ -8474,7 +8474,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
cntl = 0; cntl = 0;
if (base) { if (base) {
cntl = MCURSOR_GAMMA_ENABLE; cntl = MCURSOR_GAMMA_ENABLE;
switch (intel_crtc->cursor_width) { switch (intel_crtc->base.cursor->state->crtc_w) {
case 64: case 64:
cntl |= CURSOR_MODE_64_ARGB_AX; cntl |= CURSOR_MODE_64_ARGB_AX;
break; break;
...@@ -8485,7 +8485,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base) ...@@ -8485,7 +8485,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
cntl |= CURSOR_MODE_256_ARGB_AX; cntl |= CURSOR_MODE_256_ARGB_AX;
break; break;
default: default:
MISSING_CASE(intel_crtc->cursor_width); MISSING_CASE(intel_crtc->base.cursor->state->crtc_w);
return; return;
} }
cntl |= pipe << 28; /* Connect to correct pipe */ cntl |= pipe << 28; /* Connect to correct pipe */
...@@ -8532,7 +8532,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, ...@@ -8532,7 +8532,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
base = 0; base = 0;
if (x < 0) { if (x < 0) {
if (x + intel_crtc->cursor_width <= 0) if (x + intel_crtc->base.cursor->state->crtc_w <= 0)
base = 0; base = 0;
pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT; pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
...@@ -8541,7 +8541,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, ...@@ -8541,7 +8541,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
pos |= x << CURSOR_X_SHIFT; pos |= x << CURSOR_X_SHIFT;
if (y < 0) { if (y < 0) {
if (y + intel_crtc->cursor_height <= 0) if (y + intel_crtc->base.cursor->state->crtc_h <= 0)
base = 0; base = 0;
pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT; pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
...@@ -8557,8 +8557,8 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, ...@@ -8557,8 +8557,8 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
/* ILK+ do this automagically */ /* ILK+ do this automagically */
if (HAS_GMCH_DISPLAY(dev) && if (HAS_GMCH_DISPLAY(dev) &&
crtc->cursor->state->rotation == BIT(DRM_ROTATE_180)) { crtc->cursor->state->rotation == BIT(DRM_ROTATE_180)) {
base += (intel_crtc->cursor_height * base += (intel_crtc->base.cursor->state->crtc_h *
intel_crtc->cursor_width - 1) * 4; intel_crtc->base.cursor->state->crtc_w - 1) * 4;
} }
if (IS_845G(dev) || IS_I865G(dev)) if (IS_845G(dev) || IS_I865G(dev))
...@@ -12302,7 +12302,7 @@ intel_check_cursor_plane(struct drm_plane *plane, ...@@ -12302,7 +12302,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
finish: finish:
if (intel_crtc->active) { if (intel_crtc->active) {
if (intel_crtc->cursor_width != state->base.crtc_w) if (intel_crtc->base.cursor->state->crtc_w != state->base.crtc_w)
intel_crtc->atomic.update_wm = true; intel_crtc->atomic.update_wm = true;
intel_crtc->atomic.fb_bits |= intel_crtc->atomic.fb_bits |=
...@@ -12345,8 +12345,6 @@ intel_commit_cursor_plane(struct drm_plane *plane, ...@@ -12345,8 +12345,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
intel_crtc->cursor_addr = addr; intel_crtc->cursor_addr = addr;
intel_crtc->cursor_bo = obj; intel_crtc->cursor_bo = obj;
update: update:
intel_crtc->cursor_width = state->base.crtc_w;
intel_crtc->cursor_height = state->base.crtc_h;
if (intel_crtc->active) if (intel_crtc->active)
intel_crtc_update_cursor(crtc, state->visible); intel_crtc_update_cursor(crtc, state->visible);
......
...@@ -464,7 +464,6 @@ struct intel_crtc { ...@@ -464,7 +464,6 @@ struct intel_crtc {
struct drm_i915_gem_object *cursor_bo; struct drm_i915_gem_object *cursor_bo;
uint32_t cursor_addr; uint32_t cursor_addr;
int16_t cursor_width, cursor_height;
uint32_t cursor_cntl; uint32_t cursor_cntl;
uint32_t cursor_size; uint32_t cursor_size;
uint32_t cursor_base; uint32_t cursor_base;
......
...@@ -644,7 +644,7 @@ static bool g4x_compute_wm0(struct drm_device *dev, ...@@ -644,7 +644,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
/* Use the large buffer method to calculate cursor watermark */ /* Use the large buffer method to calculate cursor watermark */
line_time_us = max(htotal * 1000 / clock, 1); line_time_us = max(htotal * 1000 / clock, 1);
line_count = (cursor_latency_ns / line_time_us + 1000) / 1000; line_count = (cursor_latency_ns / line_time_us + 1000) / 1000;
entries = line_count * to_intel_crtc(crtc)->cursor_width * pixel_size; entries = line_count * crtc->cursor->state->crtc_w * pixel_size;
tlb_miss = cursor->fifo_size*cursor->cacheline_size - hdisplay * 8; tlb_miss = cursor->fifo_size*cursor->cacheline_size - hdisplay * 8;
if (tlb_miss > 0) if (tlb_miss > 0)
entries += tlb_miss; entries += tlb_miss;
...@@ -730,7 +730,7 @@ static bool g4x_compute_srwm(struct drm_device *dev, ...@@ -730,7 +730,7 @@ static bool g4x_compute_srwm(struct drm_device *dev,
*display_wm = entries + display->guard_size; *display_wm = entries + display->guard_size;
/* calculate the self-refresh watermark for display cursor */ /* calculate the self-refresh watermark for display cursor */
entries = line_count * pixel_size * to_intel_crtc(crtc)->cursor_width; entries = line_count * pixel_size * crtc->cursor->state->crtc_w;
entries = DIV_ROUND_UP(entries, cursor->cacheline_size); entries = DIV_ROUND_UP(entries, cursor->cacheline_size);
*cursor_wm = entries + cursor->guard_size; *cursor_wm = entries + cursor->guard_size;
...@@ -1098,7 +1098,7 @@ static void i965_update_wm(struct drm_crtc *unused_crtc) ...@@ -1098,7 +1098,7 @@ static void i965_update_wm(struct drm_crtc *unused_crtc)
entries, srwm); entries, srwm);
entries = (((sr_latency_ns / line_time_us) + 1000) / 1000) * entries = (((sr_latency_ns / line_time_us) + 1000) / 1000) *
pixel_size * to_intel_crtc(crtc)->cursor_width; pixel_size * crtc->cursor->state->crtc_w;
entries = DIV_ROUND_UP(entries, entries = DIV_ROUND_UP(entries,
i965_cursor_wm_info.cacheline_size); i965_cursor_wm_info.cacheline_size);
cursor_sr = i965_cursor_wm_info.fifo_size - cursor_sr = i965_cursor_wm_info.fifo_size -
...@@ -1927,7 +1927,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc, ...@@ -1927,7 +1927,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
p->pri.bytes_per_pixel = crtc->primary->fb->bits_per_pixel / 8; p->pri.bytes_per_pixel = crtc->primary->fb->bits_per_pixel / 8;
p->cur.bytes_per_pixel = 4; p->cur.bytes_per_pixel = 4;
p->pri.horiz_pixels = intel_crtc->config->pipe_src_w; p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
p->cur.horiz_pixels = intel_crtc->cursor_width; p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
/* TODO: for now, assume primary and cursor planes are always enabled. */ /* TODO: for now, assume primary and cursor planes are always enabled. */
p->pri.enabled = true; p->pri.enabled = true;
p->cur.enabled = true; p->cur.enabled = true;
...@@ -2715,8 +2715,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc, ...@@ -2715,8 +2715,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
p->cursor.enabled = true; p->cursor.enabled = true;
p->cursor.bytes_per_pixel = 4; p->cursor.bytes_per_pixel = 4;
p->cursor.horiz_pixels = intel_crtc->cursor_width ? p->cursor.horiz_pixels = intel_crtc->base.cursor->state->crtc_w ?
intel_crtc->cursor_width : 64; intel_crtc->base.cursor->state->crtc_w : 64;
} }
list_for_each_entry(plane, &dev->mode_config.plane_list, head) { list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment