Skip to content

Commit 2ff8fde

Browse files
mattropedanvet
authored andcommitted
drm/i915: Make use of intel_fb_obj() (v2)
This should hopefully simplify the display code slightly and also solves at least one mistake in intel_pipe_set_base() where to_intel_framebuffer(fb)->obj is referenced during local variable initialization, before 'if (!fb)' gets checked. Potential uses of this macro were identified via the following Coccinelle patch: @@ expression E; @@ * to_intel_framebuffer(E)->obj @@ expression E; identifier I; @@ I = to_intel_framebuffer(E); ... * I->obj v2: Rewrite some NULL tests in terms of the obj rather than the fb. Also add a WARN() if trying to pageflip with a disabled primary plane. [Suggested by Chris Wilson] Signed-off-by: Matt Roper <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Signed-off-by: Daniel Vetter <[email protected]>
1 parent 155e636 commit 2ff8fde

File tree

3 files changed

+48
-63
lines changed

3 files changed

+48
-63
lines changed

drivers/gpu/drm/i915/intel_display.c

Lines changed: 38 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2356,7 +2356,7 @@ static void intel_find_plane_obj(struct intel_crtc *intel_crtc,
23562356
struct drm_device *dev = intel_crtc->base.dev;
23572357
struct drm_crtc *c;
23582358
struct intel_crtc *i;
2359-
struct intel_framebuffer *fb;
2359+
struct drm_i915_gem_object *obj;
23602360

23612361
if (!intel_crtc->base.primary->fb)
23622362
return;
@@ -2377,14 +2377,17 @@ static void intel_find_plane_obj(struct intel_crtc *intel_crtc,
23772377
if (c == &intel_crtc->base)
23782378
continue;
23792379

2380-
if (!i->active || !c->primary->fb)
2380+
if (!i->active)
2381+
continue;
2382+
2383+
obj = intel_fb_obj(c->primary->fb);
2384+
if (obj == NULL)
23812385
continue;
23822386

2383-
fb = to_intel_framebuffer(c->primary->fb);
2384-
if (i915_gem_obj_ggtt_offset(fb->obj) == plane_config->base) {
2387+
if (i915_gem_obj_ggtt_offset(obj) == plane_config->base) {
23852388
drm_framebuffer_reference(c->primary->fb);
23862389
intel_crtc->base.primary->fb = c->primary->fb;
2387-
fb->obj->frontbuffer_bits |= INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
2390+
obj->frontbuffer_bits |= INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
23882391
break;
23892392
}
23902393
}
@@ -2397,16 +2400,12 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
23972400
struct drm_device *dev = crtc->dev;
23982401
struct drm_i915_private *dev_priv = dev->dev_private;
23992402
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
2400-
struct intel_framebuffer *intel_fb;
2401-
struct drm_i915_gem_object *obj;
2403+
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
24022404
int plane = intel_crtc->plane;
24032405
unsigned long linear_offset;
24042406
u32 dspcntr;
24052407
u32 reg;
24062408

2407-
intel_fb = to_intel_framebuffer(fb);
2408-
obj = intel_fb->obj;
2409-
24102409
reg = DSPCNTR(plane);
24112410
dspcntr = I915_READ(reg);
24122411
/* Mask out pixel format bits in case we change it */
@@ -2487,16 +2486,12 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
24872486
struct drm_device *dev = crtc->dev;
24882487
struct drm_i915_private *dev_priv = dev->dev_private;
24892488
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
2490-
struct intel_framebuffer *intel_fb;
2491-
struct drm_i915_gem_object *obj;
2489+
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
24922490
int plane = intel_crtc->plane;
24932491
unsigned long linear_offset;
24942492
u32 dspcntr;
24952493
u32 reg;
24962494

2497-
intel_fb = to_intel_framebuffer(fb);
2498-
obj = intel_fb->obj;
2499-
25002495
reg = DSPCNTR(plane);
25012496
dspcntr = I915_READ(reg);
25022497
/* Mask out pixel format bits in case we change it */
@@ -2627,7 +2622,7 @@ void intel_display_handle_reset(struct drm_device *dev)
26272622
static int
26282623
intel_finish_fb(struct drm_framebuffer *old_fb)
26292624
{
2630-
struct drm_i915_gem_object *obj = to_intel_framebuffer(old_fb)->obj;
2625+
struct drm_i915_gem_object *obj = intel_fb_obj(old_fb);
26312626
struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
26322627
bool was_interruptible = dev_priv->mm.interruptible;
26332628
int ret;
@@ -2674,9 +2669,9 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
26742669
struct drm_i915_private *dev_priv = dev->dev_private;
26752670
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
26762671
enum pipe pipe = intel_crtc->pipe;
2677-
struct drm_framebuffer *old_fb;
2678-
struct drm_i915_gem_object *obj = to_intel_framebuffer(fb)->obj;
2679-
struct drm_i915_gem_object *old_obj;
2672+
struct drm_framebuffer *old_fb = crtc->primary->fb;
2673+
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
2674+
struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
26802675
int ret;
26812676

26822677
if (intel_crtc_has_pending_flip(crtc)) {
@@ -2697,9 +2692,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
26972692
return -EINVAL;
26982693
}
26992694

2700-
old_fb = crtc->primary->fb;
2701-
old_obj = old_fb ? to_intel_framebuffer(old_fb)->obj : NULL;
2702-
27032695
mutex_lock(&dev->struct_mutex);
27042696
ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
27052697
if (ret == 0)
@@ -2755,7 +2747,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
27552747
if (intel_crtc->active && old_fb != fb)
27562748
intel_wait_for_vblank(dev, intel_crtc->pipe);
27572749
mutex_lock(&dev->struct_mutex);
2758-
intel_unpin_fb_obj(to_intel_framebuffer(old_fb)->obj);
2750+
intel_unpin_fb_obj(old_obj);
27592751
mutex_unlock(&dev->struct_mutex);
27602752
}
27612753

@@ -4929,7 +4921,7 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
49294921
struct drm_device *dev = crtc->dev;
49304922
struct drm_connector *connector;
49314923
struct drm_i915_private *dev_priv = dev->dev_private;
4932-
struct drm_i915_gem_object *old_obj;
4924+
struct drm_i915_gem_object *old_obj = intel_fb_obj(crtc->primary->fb);
49334925
enum pipe pipe = to_intel_crtc(crtc)->pipe;
49344926

49354927
/* crtc should still be enabled when we disable it. */
@@ -4944,7 +4936,6 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
49444936
assert_pipe_disabled(dev->dev_private, pipe);
49454937

49464938
if (crtc->primary->fb) {
4947-
old_obj = to_intel_framebuffer(crtc->primary->fb)->obj;
49484939
mutex_lock(&dev->struct_mutex);
49494940
intel_unpin_fb_obj(old_obj);
49504941
i915_gem_track_fb(old_obj, NULL,
@@ -9586,14 +9577,22 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
95869577
struct drm_device *dev = crtc->dev;
95879578
struct drm_i915_private *dev_priv = dev->dev_private;
95889579
struct drm_framebuffer *old_fb = crtc->primary->fb;
9589-
struct drm_i915_gem_object *obj = to_intel_framebuffer(fb)->obj;
9580+
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
95909581
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
95919582
enum pipe pipe = intel_crtc->pipe;
95929583
struct intel_unpin_work *work;
95939584
struct intel_engine_cs *ring;
95949585
unsigned long flags;
95959586
int ret;
95969587

9588+
/*
9589+
* drm_mode_page_flip_ioctl() should already catch this, but double
9590+
* check to be safe. In the future we may enable pageflipping from
9591+
* a disabled primary plane.
9592+
*/
9593+
if (WARN_ON(intel_fb_obj(old_fb) == NULL))
9594+
return -EBUSY;
9595+
95979596
/* Can't change pixel format via MI display flips. */
95989597
if (fb->pixel_format != crtc->primary->fb->pixel_format)
95999598
return -EINVAL;
@@ -9616,7 +9615,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
96169615

96179616
work->event = event;
96189617
work->crtc = crtc;
9619-
work->old_fb_obj = to_intel_framebuffer(old_fb)->obj;
9618+
work->old_fb_obj = intel_fb_obj(old_fb);
96209619
INIT_WORK(&work->work, intel_unpin_work_fn);
96219620

96229621
ret = drm_crtc_vblank_get(crtc);
@@ -10758,10 +10757,9 @@ static int __intel_set_mode(struct drm_crtc *crtc,
1075810757
* on the DPLL.
1075910758
*/
1076010759
for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
10761-
struct drm_framebuffer *old_fb;
10762-
struct drm_i915_gem_object *old_obj = NULL;
10763-
struct drm_i915_gem_object *obj =
10764-
to_intel_framebuffer(fb)->obj;
10760+
struct drm_framebuffer *old_fb = crtc->primary->fb;
10761+
struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
10762+
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
1076510763

1076610764
mutex_lock(&dev->struct_mutex);
1076710765
ret = intel_pin_and_fence_fb_obj(dev,
@@ -10772,11 +10770,8 @@ static int __intel_set_mode(struct drm_crtc *crtc,
1077210770
mutex_unlock(&dev->struct_mutex);
1077310771
goto done;
1077410772
}
10775-
old_fb = crtc->primary->fb;
10776-
if (old_fb) {
10777-
old_obj = to_intel_framebuffer(old_fb)->obj;
10773+
if (old_fb)
1077810774
intel_unpin_fb_obj(old_obj);
10779-
}
1078010775
i915_gem_track_fb(old_obj, obj,
1078110776
INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));
1078210777
mutex_unlock(&dev->struct_mutex);
@@ -11394,9 +11389,9 @@ intel_primary_plane_disable(struct drm_plane *plane)
1139411389
intel_disable_primary_hw_plane(dev_priv, intel_plane->plane,
1139511390
intel_plane->pipe);
1139611391
disable_unpin:
11397-
i915_gem_track_fb(to_intel_framebuffer(plane->fb)->obj, NULL,
11392+
i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
1139811393
INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));
11399-
intel_unpin_fb_obj(to_intel_framebuffer(plane->fb)->obj);
11394+
intel_unpin_fb_obj(intel_fb_obj(plane->fb));
1140011395
plane->fb = NULL;
1140111396

1140211397
return 0;
@@ -11413,7 +11408,8 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
1141311408
struct drm_i915_private *dev_priv = dev->dev_private;
1141411409
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
1141511410
struct intel_plane *intel_plane = to_intel_plane(plane);
11416-
struct drm_i915_gem_object *obj, *old_obj = NULL;
11411+
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
11412+
struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
1141711413
struct drm_rect dest = {
1141811414
/* integer pixels */
1141911415
.x1 = crtc_x,
@@ -11445,10 +11441,6 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
1144511441
if (ret)
1144611442
return ret;
1144711443

11448-
if (plane->fb)
11449-
old_obj = to_intel_framebuffer(plane->fb)->obj;
11450-
obj = to_intel_framebuffer(fb)->obj;
11451-
1145211444
/*
1145311445
* If the CRTC isn't enabled, we're just pinning the framebuffer,
1145411446
* updating the fb pointer, and returning without touching the
@@ -12945,7 +12937,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
1294512937
void intel_modeset_gem_init(struct drm_device *dev)
1294612938
{
1294712939
struct drm_crtc *c;
12948-
struct intel_framebuffer *fb;
12940+
struct drm_i915_gem_object *obj;
1294912941

1295012942
mutex_lock(&dev->struct_mutex);
1295112943
intel_init_gt_powersave(dev);
@@ -12962,11 +12954,11 @@ void intel_modeset_gem_init(struct drm_device *dev)
1296212954
*/
1296312955
mutex_lock(&dev->struct_mutex);
1296412956
for_each_crtc(dev, c) {
12965-
if (!c->primary->fb)
12957+
obj = intel_fb_obj(c->primary->fb);
12958+
if (obj == NULL)
1296612959
continue;
1296712960

12968-
fb = to_intel_framebuffer(c->primary->fb);
12969-
if (intel_pin_and_fence_fb_obj(dev, fb->obj, NULL)) {
12961+
if (intel_pin_and_fence_fb_obj(dev, obj, NULL)) {
1297012962
DRM_ERROR("failed to pin boot fb on pipe %d\n",
1297112963
to_intel_crtc(c)->pipe);
1297212964
drm_framebuffer_unreference(c->primary->fb);

drivers/gpu/drm/i915/intel_dp.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1752,7 +1752,7 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
17521752
struct drm_i915_private *dev_priv = dev->dev_private;
17531753
struct drm_crtc *crtc = dig_port->base.base.crtc;
17541754
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
1755-
struct drm_i915_gem_object *obj = to_intel_framebuffer(crtc->primary->fb)->obj;
1755+
struct drm_i915_gem_object *obj = intel_fb_obj(crtc->primary->fb);
17561756
struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
17571757

17581758
dev_priv->psr.source_ok = false;
@@ -1785,7 +1785,6 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
17851785
return false;
17861786
}
17871787

1788-
obj = to_intel_framebuffer(crtc->primary->fb)->obj;
17891788
if (obj->tiling_mode != I915_TILING_X ||
17901789
obj->fence_reg == I915_FENCE_REG_NONE) {
17911790
DRM_DEBUG_KMS("PSR condition failed: fb not tiled or fenced\n");

drivers/gpu/drm/i915/intel_pm.c

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,7 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc)
9393
struct drm_device *dev = crtc->dev;
9494
struct drm_i915_private *dev_priv = dev->dev_private;
9595
struct drm_framebuffer *fb = crtc->primary->fb;
96-
struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
97-
struct drm_i915_gem_object *obj = intel_fb->obj;
96+
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
9897
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
9998
int cfb_pitch;
10099
int i;
@@ -150,8 +149,7 @@ static void g4x_enable_fbc(struct drm_crtc *crtc)
150149
struct drm_device *dev = crtc->dev;
151150
struct drm_i915_private *dev_priv = dev->dev_private;
152151
struct drm_framebuffer *fb = crtc->primary->fb;
153-
struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
154-
struct drm_i915_gem_object *obj = intel_fb->obj;
152+
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
155153
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
156154
u32 dpfc_ctl;
157155

@@ -222,8 +220,7 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc)
222220
struct drm_device *dev = crtc->dev;
223221
struct drm_i915_private *dev_priv = dev->dev_private;
224222
struct drm_framebuffer *fb = crtc->primary->fb;
225-
struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
226-
struct drm_i915_gem_object *obj = intel_fb->obj;
223+
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
227224
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
228225
u32 dpfc_ctl;
229226

@@ -289,8 +286,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc)
289286
struct drm_device *dev = crtc->dev;
290287
struct drm_i915_private *dev_priv = dev->dev_private;
291288
struct drm_framebuffer *fb = crtc->primary->fb;
292-
struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
293-
struct drm_i915_gem_object *obj = intel_fb->obj;
289+
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
294290
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
295291
u32 dpfc_ctl;
296292

@@ -485,7 +481,6 @@ void intel_update_fbc(struct drm_device *dev)
485481
struct drm_crtc *crtc = NULL, *tmp_crtc;
486482
struct intel_crtc *intel_crtc;
487483
struct drm_framebuffer *fb;
488-
struct intel_framebuffer *intel_fb;
489484
struct drm_i915_gem_object *obj;
490485
const struct drm_display_mode *adjusted_mode;
491486
unsigned int max_width, max_height;
@@ -530,8 +525,7 @@ void intel_update_fbc(struct drm_device *dev)
530525

531526
intel_crtc = to_intel_crtc(crtc);
532527
fb = crtc->primary->fb;
533-
intel_fb = to_intel_framebuffer(fb);
534-
obj = intel_fb->obj;
528+
obj = intel_fb_obj(fb);
535529
adjusted_mode = &intel_crtc->config.adjusted_mode;
536530

537531
if (i915.enable_fbc < 0) {
@@ -589,7 +583,7 @@ void intel_update_fbc(struct drm_device *dev)
589583
if (in_dbg_master())
590584
goto out_disable;
591585

592-
if (i915_gem_stolen_setup_compression(dev, intel_fb->obj->base.size,
586+
if (i915_gem_stolen_setup_compression(dev, obj->base.size,
593587
drm_format_plane_cpp(fb->pixel_format, 0))) {
594588
if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
595589
DRM_DEBUG_KMS("framebuffer too large, disabling compression\n");
@@ -1599,12 +1593,12 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
15991593
DRM_DEBUG_KMS("FIFO watermarks - A: %d, B: %d\n", planea_wm, planeb_wm);
16001594

16011595
if (IS_I915GM(dev) && enabled) {
1602-
struct intel_framebuffer *fb;
1596+
struct drm_i915_gem_object *obj;
16031597

1604-
fb = to_intel_framebuffer(enabled->primary->fb);
1598+
obj = intel_fb_obj(enabled->primary->fb);
16051599

16061600
/* self-refresh seems busted with untiled */
1607-
if (fb->obj->tiling_mode == I915_TILING_NONE)
1601+
if (obj->tiling_mode == I915_TILING_NONE)
16081602
enabled = NULL;
16091603
}
16101604

0 commit comments

Comments
 (0)