[Intel-gfx] [PATCH 5/6] drm/i915: struct_mutex is not required for allocating the framebuffer
Chris Wilson
chris at chris-wilson.co.uk
Thu Nov 10 15:07:57 UTC 2016
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 at chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_display.c | 115 +++++++++++++++--------------------
drivers/gpu/drm/i915/intel_drv.h | 7 +--
drivers/gpu/drm/i915/intel_fbdev.c | 21 +++----
3 files changed, 62 insertions(+), 81 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e21bc14dec38..0dc101046542 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -96,10 +96,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
static void ironlake_pch_clock_get(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
-static int intel_framebuffer_init(struct drm_device *dev,
- struct intel_framebuffer *ifb,
- struct drm_mode_fb_cmd2 *mode_cmd,
- struct drm_i915_gem_object *obj);
+static int intel_framebuffer_init(struct intel_framebuffer *ifb,
+ struct drm_i915_gem_object *obj,
+ struct drm_mode_fb_cmd2 *mode_cmd);
static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc);
static void intel_set_pipe_timings(struct intel_crtc *intel_crtc);
static void intel_set_pipe_src_size(struct intel_crtc *intel_crtc);
@@ -2118,11 +2117,13 @@ static void intel_tile_dims(const struct drm_i915_private *dev_priv,
}
unsigned int
-intel_fb_align_height(struct drm_device *dev, unsigned int height,
- uint32_t pixel_format, uint64_t fb_modifier)
+intel_fb_align_height(struct drm_i915_private *dev_priv,
+ unsigned int height,
+ uint32_t pixel_format,
+ uint64_t fb_modifier)
{
unsigned int cpp = drm_format_plane_cpp(pixel_format, 0);
- unsigned int tile_height = intel_tile_height(to_i915(dev), fb_modifier, cpp);
+ unsigned int tile_height = intel_tile_height(dev_priv, fb_modifier, cpp);
return ALIGN(height, tile_height);
}
@@ -2694,15 +2695,13 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
return false;
mutex_lock(&dev->struct_mutex);
-
obj = i915_gem_object_create_stolen_for_preallocated(dev,
base_aligned,
base_aligned,
size_aligned);
- if (!obj) {
- mutex_unlock(&dev->struct_mutex);
+ mutex_unlock(&dev->struct_mutex);
+ if (!obj)
return false;
- }
if (plane_config->tiling == I915_TILING_X)
obj->tiling_and_stride = fb->pitches[0] | I915_TILING_X;
@@ -2714,13 +2713,11 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
mode_cmd.modifier[0] = fb->modifier[0];
mode_cmd.flags = DRM_MODE_FB_MODIFIERS;
- if (intel_framebuffer_init(dev, to_intel_framebuffer(fb),
- &mode_cmd, obj)) {
+ if (intel_framebuffer_init(to_intel_framebuffer(fb), obj, &mode_cmd)) {
DRM_DEBUG_KMS("intel fb init failed\n");
goto out_unref_obj;
}
- mutex_unlock(&dev->struct_mutex);
DRM_DEBUG_KMS("initial plane fb obj %p\n", obj);
return true;
@@ -8759,7 +8756,8 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
val = I915_READ(DSPSTRIDE(pipe));
fb->pitches[0] = val & 0xffffffc0;
- aligned_height = intel_fb_align_height(dev, fb->height,
+ aligned_height = intel_fb_align_height(dev_priv,
+ fb->height,
fb->pixel_format,
fb->modifier[0]);
@@ -9806,7 +9804,8 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
fb->pixel_format);
fb->pitches[0] = (val & 0x3ff) * stride_mult;
- aligned_height = intel_fb_align_height(dev, fb->height,
+ aligned_height = intel_fb_align_height(dev_priv,
+ fb->height,
fb->pixel_format,
fb->modifier[0]);
@@ -9903,7 +9902,8 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
val = I915_READ(DSPSTRIDE(pipe));
fb->pitches[0] = val & 0xffffffc0;
- aligned_height = intel_fb_align_height(dev, fb->height,
+ aligned_height = intel_fb_align_height(dev_priv,
+ fb->height,
fb->pixel_format,
fb->modifier[0]);
@@ -10979,9 +10979,8 @@ static struct drm_display_mode load_detect_mode = {
};
struct drm_framebuffer *
-__intel_framebuffer_create(struct drm_device *dev,
- struct drm_mode_fb_cmd2 *mode_cmd,
- struct drm_i915_gem_object *obj)
+intel_framebuffer_create(struct drm_i915_gem_object *obj,
+ struct drm_mode_fb_cmd2 *mode_cmd)
{
struct intel_framebuffer *intel_fb;
int ret;
@@ -10990,7 +10989,7 @@ __intel_framebuffer_create(struct drm_device *dev,
if (!intel_fb)
return ERR_PTR(-ENOMEM);
- ret = intel_framebuffer_init(dev, intel_fb, mode_cmd, obj);
+ ret = intel_framebuffer_init(intel_fb, obj, mode_cmd);
if (ret)
goto err;
@@ -11001,23 +11000,6 @@ __intel_framebuffer_create(struct drm_device *dev,
return ERR_PTR(ret);
}
-static struct drm_framebuffer *
-intel_framebuffer_create(struct drm_device *dev,
- struct drm_mode_fb_cmd2 *mode_cmd,
- struct drm_i915_gem_object *obj)
-{
- struct drm_framebuffer *fb;
- int ret;
-
- ret = i915_mutex_lock_interruptible(dev);
- if (ret)
- return ERR_PTR(ret);
- fb = __intel_framebuffer_create(dev, mode_cmd, obj);
- mutex_unlock(&dev->struct_mutex);
-
- return fb;
-}
-
static u32
intel_framebuffer_pitch_for_width(int width, int bpp)
{
@@ -11052,7 +11034,7 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
bpp);
mode_cmd.pixel_format = drm_mode_legacy_fb_format(bpp, depth);
- fb = intel_framebuffer_create(dev, &mode_cmd, obj);
+ fb = intel_framebuffer_create(obj, &mode_cmd);
if (IS_ERR(fb))
i915_gem_object_put(obj);
@@ -15719,7 +15701,7 @@ static
u32 intel_fb_pitch_limit(struct drm_i915_private *dev_priv,
uint64_t fb_modifier, uint32_t pixel_format)
{
- u32 gen = INTEL_INFO(dev_priv)->gen;
+ u32 gen = INTEL_GEN(dev_priv);
if (gen >= 9) {
int cpp = drm_format_plane_cpp(pixel_format, 0);
@@ -15747,18 +15729,17 @@ u32 intel_fb_pitch_limit(struct drm_i915_private *dev_priv,
}
}
-static int intel_framebuffer_init(struct drm_device *dev,
- struct intel_framebuffer *intel_fb,
- struct drm_mode_fb_cmd2 *mode_cmd,
- struct drm_i915_gem_object *obj)
+static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
+ struct drm_i915_gem_object *obj,
+ struct drm_mode_fb_cmd2 *mode_cmd)
{
- struct drm_i915_private *dev_priv = to_i915(dev);
+ struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
unsigned int tiling = i915_gem_object_get_tiling(obj);
- int ret;
u32 pitch_limit, stride_alignment;
char *format_name;
+ int ret = -EINVAL;
- WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+ atomic_inc(&obj->framebuffer_references);
if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
/*
@@ -15768,14 +15749,14 @@ static int intel_framebuffer_init(struct drm_device *dev,
if (tiling != I915_TILING_NONE &&
tiling != intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) {
DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
- return -EINVAL;
+ goto err;
}
} else {
if (tiling == I915_TILING_X) {
mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
} else if (tiling == I915_TILING_Y) {
DRM_DEBUG("No Y tiling for legacy addfb\n");
- return -EINVAL;
+ goto err;
}
}
@@ -15783,10 +15764,10 @@ static int intel_framebuffer_init(struct drm_device *dev,
switch (mode_cmd->modifier[0]) {
case I915_FORMAT_MOD_Y_TILED:
case I915_FORMAT_MOD_Yf_TILED:
- if (INTEL_INFO(dev)->gen < 9) {
+ if (INTEL_GEN(dev_priv) < 9) {
DRM_DEBUG("Unsupported tiling 0x%llx!\n",
mode_cmd->modifier[0]);
- return -EINVAL;
+ goto err;
}
case DRM_FORMAT_MOD_NONE:
case I915_FORMAT_MOD_X_TILED:
@@ -15794,7 +15775,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
default:
DRM_DEBUG("Unsupported fb modifier 0x%llx!\n",
mode_cmd->modifier[0]);
- return -EINVAL;
+ goto err;
}
/*
@@ -15813,7 +15794,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
if (mode_cmd->pitches[0] & (stride_alignment - 1)) {
DRM_DEBUG("pitch (%d) must be at least %u byte aligned\n",
mode_cmd->pitches[0], stride_alignment);
- return -EINVAL;
+ goto err;
}
pitch_limit = intel_fb_pitch_limit(dev_priv, mode_cmd->modifier[0],
@@ -15823,7 +15804,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
mode_cmd->modifier[0] != DRM_FORMAT_MOD_NONE ?
"tiled" : "linear",
mode_cmd->pitches[0], pitch_limit);
- return -EINVAL;
+ goto err;
}
/*
@@ -15835,7 +15816,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
DRM_DEBUG("pitch (%d) must match tiling stride (%d)\n",
mode_cmd->pitches[0],
i915_gem_object_get_stride(obj));
- return -EINVAL;
+ goto err;
}
/* Reject formats not supported by any plane early. */
@@ -15846,7 +15827,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
case DRM_FORMAT_ARGB8888:
break;
case DRM_FORMAT_XRGB1555:
- if (INTEL_INFO(dev)->gen > 3) {
+ if (INTEL_GEN(dev_priv) > 3) {
format_name = drm_get_format_name(mode_cmd->pixel_format);
DRM_DEBUG("unsupported pixel format: %s\n", format_name);
kfree(format_name);
@@ -15855,7 +15836,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
break;
case DRM_FORMAT_ABGR8888:
if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv) &&
- INTEL_INFO(dev)->gen < 9) {
+ INTEL_GEN(dev_priv) < 9) {
format_name = drm_get_format_name(mode_cmd->pixel_format);
DRM_DEBUG("unsupported pixel format: %s\n", format_name);
kfree(format_name);
@@ -15865,7 +15846,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
case DRM_FORMAT_XBGR8888:
case DRM_FORMAT_XRGB2101010:
case DRM_FORMAT_XBGR2101010:
- if (INTEL_INFO(dev)->gen < 4) {
+ if (INTEL_GEN(dev_priv) < 4) {
format_name = drm_get_format_name(mode_cmd->pixel_format);
DRM_DEBUG("unsupported pixel format: %s\n", format_name);
kfree(format_name);
@@ -15884,7 +15865,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
case DRM_FORMAT_UYVY:
case DRM_FORMAT_YVYU:
case DRM_FORMAT_VYUY:
- if (INTEL_INFO(dev)->gen < 5) {
+ if (INTEL_GEN(dev_priv) < 5) {
format_name = drm_get_format_name(mode_cmd->pixel_format);
DRM_DEBUG("unsupported pixel format: %s\n", format_name);
kfree(format_name);
@@ -15900,7 +15881,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
/* FIXME need to adjust LINOFF/TILEOFF accordingly. */
if (mode_cmd->offsets[0] != 0)
- return -EINVAL;
+ goto err;
drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
intel_fb->obj = obj;
@@ -15909,15 +15890,19 @@ static int intel_framebuffer_init(struct drm_device *dev,
if (ret)
return ret;
- ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
+ ret = drm_framebuffer_init(obj->base.dev,
+ &intel_fb->base,
+ &intel_fb_funcs);
if (ret) {
DRM_ERROR("framebuffer init failed %d\n", ret);
- return ret;
+ goto err;
}
- atomic_inc(&intel_fb->obj->framebuffer_references);
-
return 0;
+
+err:
+ atomic_dec(&obj->framebuffer_references);
+ return ret;
}
static struct drm_framebuffer *
@@ -15933,7 +15918,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
if (!obj)
return ERR_PTR(-ENOENT);
- fb = intel_framebuffer_create(dev, &mode_cmd, obj);
+ fb = intel_framebuffer_create(obj, &mode_cmd);
if (IS_ERR(fb))
i915_gem_object_put(obj);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 003afb873b67..19d823c56a18 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1178,7 +1178,7 @@ void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
struct intel_shared_dpll *intel_ddi_get_link_dpll(struct intel_dp *intel_dp,
int clock);
-unsigned int intel_fb_align_height(struct drm_device *dev,
+unsigned int intel_fb_align_height(struct drm_i915_private *dev_priv,
unsigned int height,
uint32_t pixel_format,
uint64_t fb_format_modifier);
@@ -1274,9 +1274,8 @@ struct i915_vma *
intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
struct drm_framebuffer *
-__intel_framebuffer_create(struct drm_device *dev,
- struct drm_mode_fb_cmd2 *mode_cmd,
- struct drm_i915_gem_object *obj);
+intel_framebuffer_create(struct drm_i915_gem_object *obj,
+ struct drm_mode_fb_cmd2 *mode_cmd);
void intel_finish_page_flip_cs(struct drm_i915_private *dev_priv, int pipe);
void intel_finish_page_flip_mmio(struct drm_i915_private *dev_priv, int pipe);
void intel_check_page_flip(struct drm_i915_private *dev_priv, int pipe);
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index b7098f98bb67..2a839b920267 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -124,7 +124,7 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
struct drm_i915_private *dev_priv = to_i915(dev);
struct i915_ggtt *ggtt = &dev_priv->ggtt;
struct drm_mode_fb_cmd2 mode_cmd = {};
- struct drm_i915_gem_object *obj = NULL;
+ struct drm_i915_gem_object *obj;
int size, ret;
/* we don't do packed 24bpp */
@@ -139,14 +139,13 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
sizes->surface_depth);
- mutex_lock(&dev->struct_mutex);
-
size = mode_cmd.pitches[0] * mode_cmd.height;
size = PAGE_ALIGN(size);
/* If the FB is too big, just don't use it since fbdev is not very
* important and we should probably use that space with FBC or other
* features. */
+ obj = NULL;
if (size * 2 < ggtt->stolen_usable_size)
obj = i915_gem_object_create_stolen(dev, size);
if (obj == NULL)
@@ -154,24 +153,22 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
if (IS_ERR(obj)) {
DRM_ERROR("failed to allocate framebuffer\n");
ret = PTR_ERR(obj);
- goto out;
+ goto err;
}
- fb = __intel_framebuffer_create(dev, &mode_cmd, obj);
+ fb = intel_framebuffer_create(obj, &mode_cmd);
if (IS_ERR(fb)) {
- i915_gem_object_put(obj);
ret = PTR_ERR(fb);
- goto out;
+ goto err_obj;
}
- mutex_unlock(&dev->struct_mutex);
-
ifbdev->fb = to_intel_framebuffer(fb);
return 0;
-out:
- mutex_unlock(&dev->struct_mutex);
+err_obj:
+ i915_gem_object_put(obj);
+err:
return ret;
}
@@ -634,7 +631,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
}
cur_size = intel_crtc->config->base.adjusted_mode.crtc_vdisplay;
- cur_size = intel_fb_align_height(dev, cur_size,
+ cur_size = intel_fb_align_height(to_i915(dev), cur_size,
fb->base.pixel_format,
fb->base.modifier[0]);
cur_size *= fb->base.pitches[0];
--
2.10.2
More information about the Intel-gfx
mailing list