Skip to content

Commit 24dbf51

Browse files
ickleickle
committed
drm/i915: struct_mutex is not required for allocating the framebuffer
We do not need the BKL struct_mutex in order to allocate a GEM object, nor to create the framebuffer, so resist the temptation to take the BKL willy nilly. As this changes the locking contract around internal API calls, the patch is a little larger than a plain removal of a pair of mutex_lock/unlock. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/20170215105919.7347-2-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
1 parent 70001cd commit 24dbf51

File tree

3 files changed

+59
-78
lines changed

3 files changed

+59
-78
lines changed

‎drivers/gpu/drm/i915/intel_display.c‎

Lines changed: 47 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
9696
static void ironlake_pch_clock_get(struct intel_crtc *crtc,
9797
struct intel_crtc_state *pipe_config);
9898

99-
static int intel_framebuffer_init(struct drm_device *dev,
100-
struct intel_framebuffer *ifb,
101-
struct drm_mode_fb_cmd2 *mode_cmd,
102-
struct drm_i915_gem_object *obj);
99+
static int intel_framebuffer_init(struct intel_framebuffer *ifb,
100+
struct drm_i915_gem_object *obj,
101+
struct drm_mode_fb_cmd2 *mode_cmd);
103102
static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc);
104103
static void intel_set_pipe_timings(struct intel_crtc *intel_crtc);
105104
static void intel_set_pipe_src_size(struct intel_crtc *intel_crtc);
@@ -2052,11 +2051,13 @@ static void intel_tile_dims(const struct drm_i915_private *dev_priv,
20522051
}
20532052

20542053
unsigned int
2055-
intel_fb_align_height(struct drm_device *dev, unsigned int height,
2056-
uint32_t pixel_format, uint64_t fb_modifier)
2054+
intel_fb_align_height(struct drm_i915_private *dev_priv,
2055+
unsigned int height,
2056+
uint32_t pixel_format,
2057+
uint64_t fb_modifier)
20572058
{
20582059
unsigned int cpp = drm_format_plane_cpp(pixel_format, 0);
2059-
unsigned int tile_height = intel_tile_height(to_i915(dev), fb_modifier, cpp);
2060+
unsigned int tile_height = intel_tile_height(dev_priv, fb_modifier, cpp);
20602061

20612062
return ALIGN(height, tile_height);
20622063
}
@@ -2622,15 +2623,13 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
26222623
return false;
26232624

26242625
mutex_lock(&dev->struct_mutex);
2625-
26262626
obj = i915_gem_object_create_stolen_for_preallocated(dev_priv,
26272627
base_aligned,
26282628
base_aligned,
26292629
size_aligned);
2630-
if (!obj) {
2631-
mutex_unlock(&dev->struct_mutex);
2630+
mutex_unlock(&dev->struct_mutex);
2631+
if (!obj)
26322632
return false;
2633-
}
26342633

26352634
if (plane_config->tiling == I915_TILING_X)
26362635
obj->tiling_and_stride = fb->pitches[0] | I915_TILING_X;
@@ -2642,20 +2641,17 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
26422641
mode_cmd.modifier[0] = fb->modifier;
26432642
mode_cmd.flags = DRM_MODE_FB_MODIFIERS;
26442643

2645-
if (intel_framebuffer_init(dev, to_intel_framebuffer(fb),
2646-
&mode_cmd, obj)) {
2644+
if (intel_framebuffer_init(to_intel_framebuffer(fb), obj, &mode_cmd)) {
26472645
DRM_DEBUG_KMS("intel fb init failed\n");
26482646
goto out_unref_obj;
26492647
}
26502648

2651-
mutex_unlock(&dev->struct_mutex);
26522649

26532650
DRM_DEBUG_KMS("initial plane fb obj %p\n", obj);
26542651
return true;
26552652

26562653
out_unref_obj:
26572654
i915_gem_object_put(obj);
2658-
mutex_unlock(&dev->struct_mutex);
26592655
return false;
26602656
}
26612657

@@ -7434,7 +7430,8 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
74347430
val = I915_READ(DSPSTRIDE(pipe));
74357431
fb->pitches[0] = val & 0xffffffc0;
74367432

7437-
aligned_height = intel_fb_align_height(dev, fb->height,
7433+
aligned_height = intel_fb_align_height(dev_priv,
7434+
fb->height,
74387435
fb->format->format,
74397436
fb->modifier);
74407437

@@ -8475,7 +8472,8 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
84758472
fb->format->format);
84768473
fb->pitches[0] = (val & 0x3ff) * stride_mult;
84778474

8478-
aligned_height = intel_fb_align_height(dev, fb->height,
8475+
aligned_height = intel_fb_align_height(dev_priv,
8476+
fb->height,
84798477
fb->format->format,
84808478
fb->modifier);
84818479

@@ -8573,7 +8571,8 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
85738571
val = I915_READ(DSPSTRIDE(pipe));
85748572
fb->pitches[0] = val & 0xffffffc0;
85758573

8576-
aligned_height = intel_fb_align_height(dev, fb->height,
8574+
aligned_height = intel_fb_align_height(dev_priv,
8575+
fb->height,
85778576
fb->format->format,
85788577
fb->modifier);
85798578

@@ -9399,9 +9398,8 @@ static struct drm_display_mode load_detect_mode = {
93999398
};
94009399

94019400
struct drm_framebuffer *
9402-
__intel_framebuffer_create(struct drm_device *dev,
9403-
struct drm_mode_fb_cmd2 *mode_cmd,
9404-
struct drm_i915_gem_object *obj)
9401+
intel_framebuffer_create(struct drm_i915_gem_object *obj,
9402+
struct drm_mode_fb_cmd2 *mode_cmd)
94059403
{
94069404
struct intel_framebuffer *intel_fb;
94079405
int ret;
@@ -9410,7 +9408,7 @@ __intel_framebuffer_create(struct drm_device *dev,
94109408
if (!intel_fb)
94119409
return ERR_PTR(-ENOMEM);
94129410

9413-
ret = intel_framebuffer_init(dev, intel_fb, mode_cmd, obj);
9411+
ret = intel_framebuffer_init(intel_fb, obj, mode_cmd);
94149412
if (ret)
94159413
goto err;
94169414

@@ -9421,23 +9419,6 @@ __intel_framebuffer_create(struct drm_device *dev,
94219419
return ERR_PTR(ret);
94229420
}
94239421

9424-
static struct drm_framebuffer *
9425-
intel_framebuffer_create(struct drm_device *dev,
9426-
struct drm_mode_fb_cmd2 *mode_cmd,
9427-
struct drm_i915_gem_object *obj)
9428-
{
9429-
struct drm_framebuffer *fb;
9430-
int ret;
9431-
9432-
ret = i915_mutex_lock_interruptible(dev);
9433-
if (ret)
9434-
return ERR_PTR(ret);
9435-
fb = __intel_framebuffer_create(dev, mode_cmd, obj);
9436-
mutex_unlock(&dev->struct_mutex);
9437-
9438-
return fb;
9439-
}
9440-
94419422
static u32
94429423
intel_framebuffer_pitch_for_width(int width, int bpp)
94439424
{
@@ -9472,7 +9453,7 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
94729453
bpp);
94739454
mode_cmd.pixel_format = drm_mode_legacy_fb_format(bpp, depth);
94749455

9475-
fb = intel_framebuffer_create(dev, &mode_cmd, obj);
9456+
fb = intel_framebuffer_create(obj, &mode_cmd);
94769457
if (IS_ERR(fb))
94779458
i915_gem_object_put(obj);
94789459

@@ -14319,7 +14300,7 @@ static
1431914300
u32 intel_fb_pitch_limit(struct drm_i915_private *dev_priv,
1432014301
uint64_t fb_modifier, uint32_t pixel_format)
1432114302
{
14322-
u32 gen = INTEL_INFO(dev_priv)->gen;
14303+
u32 gen = INTEL_GEN(dev_priv);
1432314304

1432414305
if (gen >= 9) {
1432514306
int cpp = drm_format_plane_cpp(pixel_format, 0);
@@ -14346,18 +14327,17 @@ u32 intel_fb_pitch_limit(struct drm_i915_private *dev_priv,
1434614327
}
1434714328
}
1434814329

14349-
static int intel_framebuffer_init(struct drm_device *dev,
14350-
struct intel_framebuffer *intel_fb,
14351-
struct drm_mode_fb_cmd2 *mode_cmd,
14352-
struct drm_i915_gem_object *obj)
14330+
static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
14331+
struct drm_i915_gem_object *obj,
14332+
struct drm_mode_fb_cmd2 *mode_cmd)
1435314333
{
14354-
struct drm_i915_private *dev_priv = to_i915(dev);
14334+
struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
1435514335
unsigned int tiling = i915_gem_object_get_tiling(obj);
14356-
int ret;
1435714336
u32 pitch_limit, stride_alignment;
1435814337
struct drm_format_name_buf format_name;
14338+
int ret = -EINVAL;
1435914339

14360-
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
14340+
atomic_inc(&obj->framebuffer_references);
1436114341

1436214342
if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
1436314343
/*
@@ -14367,14 +14347,14 @@ static int intel_framebuffer_init(struct drm_device *dev,
1436714347
if (tiling != I915_TILING_NONE &&
1436814348
tiling != intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) {
1436914349
DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
14370-
return -EINVAL;
14350+
goto err;
1437114351
}
1437214352
} else {
1437314353
if (tiling == I915_TILING_X) {
1437414354
mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
1437514355
} else if (tiling == I915_TILING_Y) {
1437614356
DRM_DEBUG("No Y tiling for legacy addfb\n");
14377-
return -EINVAL;
14357+
goto err;
1437814358
}
1437914359
}
1438014360

@@ -14385,15 +14365,15 @@ static int intel_framebuffer_init(struct drm_device *dev,
1438514365
if (INTEL_GEN(dev_priv) < 9) {
1438614366
DRM_DEBUG("Unsupported tiling 0x%llx!\n",
1438714367
mode_cmd->modifier[0]);
14388-
return -EINVAL;
14368+
goto err;
1438914369
}
1439014370
case DRM_FORMAT_MOD_NONE:
1439114371
case I915_FORMAT_MOD_X_TILED:
1439214372
break;
1439314373
default:
1439414374
DRM_DEBUG("Unsupported fb modifier 0x%llx!\n",
1439514375
mode_cmd->modifier[0]);
14396-
return -EINVAL;
14376+
goto err;
1439714377
}
1439814378

1439914379
/*
@@ -14412,7 +14392,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
1441214392
if (mode_cmd->pitches[0] & (stride_alignment - 1)) {
1441314393
DRM_DEBUG("pitch (%d) must be at least %u byte aligned\n",
1441414394
mode_cmd->pitches[0], stride_alignment);
14415-
return -EINVAL;
14395+
goto err;
1441614396
}
1441714397

1441814398
pitch_limit = intel_fb_pitch_limit(dev_priv, mode_cmd->modifier[0],
@@ -14422,7 +14402,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
1442214402
mode_cmd->modifier[0] != DRM_FORMAT_MOD_NONE ?
1442314403
"tiled" : "linear",
1442414404
mode_cmd->pitches[0], pitch_limit);
14425-
return -EINVAL;
14405+
goto err;
1442614406
}
1442714407

1442814408
/*
@@ -14434,7 +14414,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
1443414414
DRM_DEBUG("pitch (%d) must match tiling stride (%d)\n",
1443514415
mode_cmd->pitches[0],
1443614416
i915_gem_object_get_stride(obj));
14437-
return -EINVAL;
14417+
goto err;
1443814418
}
1443914419

1444014420
/* Reject formats not supported by any plane early. */
@@ -14493,24 +14473,29 @@ static int intel_framebuffer_init(struct drm_device *dev,
1449314473

1449414474
/* FIXME need to adjust LINOFF/TILEOFF accordingly. */
1449514475
if (mode_cmd->offsets[0] != 0)
14496-
return -EINVAL;
14476+
goto err;
1449714477

14498-
drm_helper_mode_fill_fb_struct(dev, &intel_fb->base, mode_cmd);
14478+
drm_helper_mode_fill_fb_struct(&dev_priv->drm,
14479+
&intel_fb->base, mode_cmd);
1449914480
intel_fb->obj = obj;
1450014481

1450114482
ret = intel_fill_fb_info(dev_priv, &intel_fb->base);
1450214483
if (ret)
1450314484
return ret;
1450414485

14505-
ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
14486+
ret = drm_framebuffer_init(obj->base.dev,
14487+
&intel_fb->base,
14488+
&intel_fb_funcs);
1450614489
if (ret) {
1450714490
DRM_ERROR("framebuffer init failed %d\n", ret);
14508-
return ret;
14491+
goto err;
1450914492
}
1451014493

14511-
atomic_inc(&intel_fb->obj->framebuffer_references);
14512-
1451314494
return 0;
14495+
14496+
err:
14497+
atomic_dec(&obj->framebuffer_references);
14498+
return ret;
1451414499
}
1451514500

1451614501
static struct drm_framebuffer *
@@ -14526,7 +14511,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
1452614511
if (!obj)
1452714512
return ERR_PTR(-ENOENT);
1452814513

14529-
fb = intel_framebuffer_create(dev, &mode_cmd, obj);
14514+
fb = intel_framebuffer_create(obj, &mode_cmd);
1453014515
if (IS_ERR(fb))
1453114516
i915_gem_object_put(obj);
1453214517

‎drivers/gpu/drm/i915/intel_drv.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1233,7 +1233,7 @@ void intel_ddi_clock_get(struct intel_encoder *encoder,
12331233
struct intel_crtc_state *pipe_config);
12341234
void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
12351235
uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
1236-
unsigned int intel_fb_align_height(struct drm_device *dev,
1236+
unsigned int intel_fb_align_height(struct drm_i915_private *dev_priv,
12371237
unsigned int height,
12381238
uint32_t pixel_format,
12391239
uint64_t fb_format_modifier);
@@ -1341,9 +1341,8 @@ struct i915_vma *
13411341
intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
13421342
void intel_unpin_fb_vma(struct i915_vma *vma);
13431343
struct drm_framebuffer *
1344-
__intel_framebuffer_create(struct drm_device *dev,
1345-
struct drm_mode_fb_cmd2 *mode_cmd,
1346-
struct drm_i915_gem_object *obj);
1344+
intel_framebuffer_create(struct drm_i915_gem_object *obj,
1345+
struct drm_mode_fb_cmd2 *mode_cmd);
13471346
void intel_finish_page_flip_cs(struct drm_i915_private *dev_priv, int pipe);
13481347
void intel_finish_page_flip_mmio(struct drm_i915_private *dev_priv, int pipe);
13491348
void intel_check_page_flip(struct drm_i915_private *dev_priv, int pipe);

‎drivers/gpu/drm/i915/intel_fbdev.c‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
121121
struct drm_i915_private *dev_priv = to_i915(dev);
122122
struct i915_ggtt *ggtt = &dev_priv->ggtt;
123123
struct drm_mode_fb_cmd2 mode_cmd = {};
124-
struct drm_i915_gem_object *obj = NULL;
124+
struct drm_i915_gem_object *obj;
125125
int size, ret;
126126

127127
/* we don't do packed 24bpp */
@@ -136,39 +136,36 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
136136
mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
137137
sizes->surface_depth);
138138

139-
mutex_lock(&dev->struct_mutex);
140-
141139
size = mode_cmd.pitches[0] * mode_cmd.height;
142140
size = PAGE_ALIGN(size);
143141

144142
/* If the FB is too big, just don't use it since fbdev is not very
145143
* important and we should probably use that space with FBC or other
146144
* features. */
145+
obj = NULL;
147146
if (size * 2 < ggtt->stolen_usable_size)
148147
obj = i915_gem_object_create_stolen(dev_priv, size);
149148
if (obj == NULL)
150149
obj = i915_gem_object_create(dev_priv, size);
151150
if (IS_ERR(obj)) {
152151
DRM_ERROR("failed to allocate framebuffer\n");
153152
ret = PTR_ERR(obj);
154-
goto out;
153+
goto err;
155154
}
156155

157-
fb = __intel_framebuffer_create(dev, &mode_cmd, obj);
156+
fb = intel_framebuffer_create(obj, &mode_cmd);
158157
if (IS_ERR(fb)) {
159-
i915_gem_object_put(obj);
160158
ret = PTR_ERR(fb);
161-
goto out;
159+
goto err_obj;
162160
}
163161

164-
mutex_unlock(&dev->struct_mutex);
165-
166162
ifbdev->fb = to_intel_framebuffer(fb);
167163

168164
return 0;
169165

170-
out:
171-
mutex_unlock(&dev->struct_mutex);
166+
err_obj:
167+
i915_gem_object_put(obj);
168+
err:
172169
return ret;
173170
}
174171

@@ -631,7 +628,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
631628
}
632629

633630
cur_size = intel_crtc->config->base.adjusted_mode.crtc_vdisplay;
634-
cur_size = intel_fb_align_height(dev, cur_size,
631+
cur_size = intel_fb_align_height(to_i915(dev), cur_size,
635632
fb->base.format->format,
636633
fb->base.modifier);
637634
cur_size *= fb->base.pitches[0];

0 commit comments

Comments
 (0)