drm: move allocation out of drm_get_format_name()

The function's behaviour was changed in 90844f00049e, without changing
its signature, causing people to keep using it the old way without
realising they were now leaking memory.
Rob Clark also noticed it was also allocating GFP_KERNEL memory in
atomic contexts, breaking them.

Instead of having to allocate GFP_ATOMIC memory and fixing the callers
to make them cleanup the memory afterwards, let's change the function's
signature by having the caller take care of the memory and passing it to
the function.
The new parameter is a single-field struct in order to enforce the size
of its buffer and help callers to correctly manage their memory.

Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe")
Cc: Rob Clark <robdclark@gmail.com>
Cc: Christian König <christian.koenig@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
Acked-by: Rob Clark <robdclark@gmail.com>
Acked-by: Sinclair Yeh <syeh@vmware.com> (vmwgfx)
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Eric Engestrom <eric@engestrom.ch>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/20161112011309.9799-1-eric@engestrom.ch

drivers/gpu/drm/amd/amdgpu/dce_v10_0.c

@@ -2032,7 +2032,7 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc *crtc,
 	u32 tmp, viewport_w, viewport_h;
 	int r;
 	bool bypass_lut = false;
-	char *format_name;
+	struct drm_format_name_buf format_name;
 
 	/* no fb bound */
 	if (!atomic && !crtc->primary->fb) {
@@ -2144,9 +2144,8 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc *crtc,
 		bypass_lut = true;
 		break;
 	default:
-		format_name = drm_get_format_name(target_fb->pixel_format);
-		DRM_ERROR("Unsupported screen format %s\n", format_name);
-		kfree(format_name);
+		DRM_ERROR("Unsupported screen format %s\n",
+		          drm_get_format_name(target_fb->pixel_format, &format_name));
 		return -EINVAL;
 	}
 

drivers/gpu/drm/amd/amdgpu/dce_v11_0.c

@@ -2013,7 +2013,7 @@ static int dce_v11_0_crtc_do_set_base(struct drm_crtc *crtc,
 	u32 tmp, viewport_w, viewport_h;
 	int r;
 	bool bypass_lut = false;
-	char *format_name;
+	struct drm_format_name_buf format_name;
 
 	/* no fb bound */
 	if (!atomic && !crtc->primary->fb) {
@@ -2125,9 +2125,8 @@ static int dce_v11_0_crtc_do_set_base(struct drm_crtc *crtc,
 		bypass_lut = true;
 		break;
 	default:
-		format_name = drm_get_format_name(target_fb->pixel_format);
-		DRM_ERROR("Unsupported screen format %s\n", format_name);
-		kfree(format_name);
+		DRM_ERROR("Unsupported screen format %s\n",
+		          drm_get_format_name(target_fb->pixel_format, &format_name));
 		return -EINVAL;
 	}
 

drivers/gpu/drm/amd/amdgpu/dce_v6_0.c

@@ -1456,6 +1456,7 @@ static int dce_v6_0_crtc_do_set_base(struct drm_crtc *crtc,
 	u32 viewport_w, viewport_h;
 	int r;
 	bool bypass_lut = false;
+	struct drm_format_name_buf format_name;
 
 	/* no fb bound */
 	if (!atomic && !crtc->primary->fb) {
@@ -1559,7 +1560,7 @@ static int dce_v6_0_crtc_do_set_base(struct drm_crtc *crtc,
 		break;
 	default:
 		DRM_ERROR("Unsupported screen format %s\n",
-			  drm_get_format_name(target_fb->pixel_format));
+		          drm_get_format_name(target_fb->pixel_format, &format_name));
 		return -EINVAL;
 	}
 

drivers/gpu/drm/amd/amdgpu/dce_v8_0.c

@@ -1910,7 +1910,7 @@ static int dce_v8_0_crtc_do_set_base(struct drm_crtc *crtc,
 	u32 viewport_w, viewport_h;
 	int r;
 	bool bypass_lut = false;
-	char *format_name;
+	struct drm_format_name_buf format_name;
 
 	/* no fb bound */
 	if (!atomic && !crtc->primary->fb) {
@@ -2015,9 +2015,8 @@ static int dce_v8_0_crtc_do_set_base(struct drm_crtc *crtc,
 		bypass_lut = true;
 		break;
 	default:
-		format_name = drm_get_format_name(target_fb->pixel_format);
-		DRM_ERROR("Unsupported screen format %s\n", format_name);
-		kfree(format_name);
+		DRM_ERROR("Unsupported screen format %s\n",
+		          drm_get_format_name(target_fb->pixel_format, &format_name));
 		return -EINVAL;
 	}
 

drivers/gpu/drm/drm_atomic.c

@@ -861,9 +861,10 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
 	/* Check whether this plane supports the fb pixel format. */
 	ret = drm_plane_check_pixel_format(plane, state->fb->pixel_format);
 	if (ret) {
-		char *format_name = drm_get_format_name(state->fb->pixel_format);
-		DRM_DEBUG_ATOMIC("Invalid pixel format %s\n", format_name);
-		kfree(format_name);
+		struct drm_format_name_buf format_name;
+		DRM_DEBUG_ATOMIC("Invalid pixel format %s\n",
+		                 drm_get_format_name(state->fb->pixel_format,
+		                                     &format_name));
 		return ret;
 	}
 
@@ -917,9 +918,10 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
 	if (state->fb) {
 		struct drm_framebuffer *fb = state->fb;
 		int i, n = drm_format_num_planes(fb->pixel_format);
+		struct drm_format_name_buf format_name;
 
 		drm_printf(p, "\t\tformat=%s\n",
-				drm_get_format_name(fb->pixel_format));
+		              drm_get_format_name(fb->pixel_format, &format_name));
 		drm_printf(p, "\t\tsize=%dx%d\n", fb->width, fb->height);
 		drm_printf(p, "\t\tlayers:\n");
 		for (i = 0; i < n; i++) {

drivers/gpu/drm/drm_crtc.c

@@ -827,9 +827,10 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
 			ret = drm_plane_check_pixel_format(crtc->primary,
 							   fb->pixel_format);
 			if (ret) {
-				char *format_name = drm_get_format_name(fb->pixel_format);
-				DRM_DEBUG_KMS("Invalid pixel format %s\n", format_name);
-				kfree(format_name);
+				struct drm_format_name_buf format_name;
+				DRM_DEBUG_KMS("Invalid pixel format %s\n",
+				              drm_get_format_name(fb->pixel_format,
+				                                  &format_name));
 				goto out;
 			}
 		}

drivers/gpu/drm/drm_fourcc.c

@@ -79,17 +79,13 @@ uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
 EXPORT_SYMBOL(drm_mode_legacy_fb_format);
 
 /**
- * drm_get_format_name - return a string for drm fourcc format
+ * drm_get_format_name - fill a string with a drm fourcc format's name
  * @format: format to compute name of
- *
- * Note that the buffer returned by this function is owned by the caller
- * and will need to be freed using kfree().
+ * @buf: caller-supplied buffer
  */
-char *drm_get_format_name(uint32_t format)
+const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf)
 {
-	char *buf = kmalloc(32, GFP_KERNEL);
-
-	snprintf(buf, 32,
+	snprintf(buf->str, sizeof(buf->str),
 		 "%c%c%c%c %s-endian (0x%08x)",
 		 printable_char(format & 0xff),
 		 printable_char((format >> 8) & 0xff),
@@ -98,7 +94,7 @@ char *drm_get_format_name(uint32_t format)
 		 format & DRM_FORMAT_BIG_ENDIAN ? "big" : "little",
 		 format);
 
-	return buf;
+	return buf->str;
 }
 EXPORT_SYMBOL(drm_get_format_name);
 

drivers/gpu/drm/drm_framebuffer.c

@@ -133,9 +133,10 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
 
 	info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
 	if (!info) {
-		char *format_name = drm_get_format_name(r->pixel_format);
-		DRM_DEBUG_KMS("bad framebuffer format %s\n", format_name);
-		kfree(format_name);
+		struct drm_format_name_buf format_name;
+		DRM_DEBUG_KMS("bad framebuffer format %s\n",
+		              drm_get_format_name(r->pixel_format,
+		                                  &format_name));
 		return -EINVAL;
 	}
 

drivers/gpu/drm/drm_modeset_helper.c

@@ -75,10 +75,11 @@ void drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
 
 	info = drm_format_info(mode_cmd->pixel_format);
 	if (!info || !info->depth) {
-		char *format_name = drm_get_format_name(mode_cmd->pixel_format);
+		struct drm_format_name_buf format_name;
 
-		DRM_DEBUG_KMS("non-RGB pixel format %s\n", format_name);
-		kfree(format_name);
+		DRM_DEBUG_KMS("non-RGB pixel format %s\n",
+		              drm_get_format_name(mode_cmd->pixel_format,
+		                                  &format_name));
 
 		fb->depth = 0;
 		fb->bits_per_pixel = 0;

drivers/gpu/drm/drm_plane.c

@@ -479,9 +479,10 @@ static int __setplane_internal(struct drm_plane *plane,
 	/* Check whether this plane supports the fb pixel format. */
 	ret = drm_plane_check_pixel_format(plane, fb->pixel_format);
 	if (ret) {
-		char *format_name = drm_get_format_name(fb->pixel_format);
-		DRM_DEBUG_KMS("Invalid pixel format %s\n", format_name);
-		kfree(format_name);
+		struct drm_format_name_buf format_name;
+		DRM_DEBUG_KMS("Invalid pixel format %s\n",
+		              drm_get_format_name(fb->pixel_format,
+		                                  &format_name));
 		goto out;
 	}
 

drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c

@@ -608,17 +608,16 @@ static void ade_rdma_set(void __iomem *base, struct drm_framebuffer *fb,
 			 u32 ch, u32 y, u32 in_h, u32 fmt)
 {
 	struct drm_gem_cma_object *obj = drm_fb_cma_get_gem_obj(fb, 0);
-	char *format_name;
+	struct drm_format_name_buf format_name;
 	u32 reg_ctrl, reg_addr, reg_size, reg_stride, reg_space, reg_en;
 	u32 stride = fb->pitches[0];
 	u32 addr = (u32)obj->paddr + y * stride;
 
 	DRM_DEBUG_DRIVER("rdma%d: (y=%d, height=%d), stride=%d, paddr=0x%x\n",
 			 ch + 1, y, in_h, stride, (u32)obj->paddr);
-	format_name = drm_get_format_name(fb->pixel_format);
 	DRM_DEBUG_DRIVER("addr=0x%x, fb:%dx%d, pixel_format=%d(%s)\n",
-			 addr, fb->width, fb->height, fmt, format_name);
-	kfree(format_name);
+			 addr, fb->width, fb->height, fmt,
+			 drm_get_format_name(fb->pixel_format, &format_name));
 
 	/* get reg offset */
 	reg_ctrl = RD_CH_CTRL(ch);

drivers/gpu/drm/i915/i915_debugfs.c

@@ -3032,7 +3032,7 @@ static void intel_plane_info(struct seq_file *m, struct intel_crtc *intel_crtc)
 	for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
 		struct drm_plane_state *state;
 		struct drm_plane *plane = &intel_plane->base;
-		char *format_name;
+		struct drm_format_name_buf format_name;
 
 		if (!plane->state) {
 			seq_puts(m, "plane->state is NULL!\n");
@@ -3042,9 +3042,9 @@ static void intel_plane_info(struct seq_file *m, struct intel_crtc *intel_crtc)
 		state = plane->state;
 
 		if (state->fb) {
-			format_name = drm_get_format_name(state->fb->pixel_format);
+			drm_get_format_name(state->fb->pixel_format, &format_name);
 		} else {
-			format_name = kstrdup("N/A", GFP_KERNEL);
+			sprintf(format_name.str, "N/A");
 		}
 
 		seq_printf(m, "\t--Plane id %d: type=%s, crtc_pos=%4dx%4d, crtc_size=%4dx%4d, src_pos=%d.%04ux%d.%04u, src_size=%d.%04ux%d.%04u, format=%s, rotation=%s\n",
@@ -3060,10 +3060,8 @@ static void intel_plane_info(struct seq_file *m, struct intel_crtc *intel_crtc)
 			   ((state->src_w & 0xffff) * 15625) >> 10,
 			   (state->src_h >> 16),
 			   ((state->src_h & 0xffff) * 15625) >> 10,
-			   format_name,
+			   format_name.str,
 			   plane_rotation(state->rotation));
-
-		kfree(format_name);
 	}
 }
 

drivers/gpu/drm/i915/intel_atomic_plane.c

@@ -141,7 +141,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 		crtc_state->base.enable ? crtc_state->pipe_src_h : 0;
 
 	if (state->fb && drm_rotation_90_or_270(state->rotation)) {
-		char *format_name;
+		struct drm_format_name_buf format_name;
 
 		if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
 			state->fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
@@ -157,9 +157,9 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 		switch (state->fb->pixel_format) {
 		case DRM_FORMAT_C8:
 		case DRM_FORMAT_RGB565:
-			format_name = drm_get_format_name(state->fb->pixel_format);
-			DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n", format_name);
-			kfree(format_name);
+			DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n",
+			              drm_get_format_name(state->fb->pixel_format,
+			                                  &format_name));
 			return -EINVAL;
 
 		default:

drivers/gpu/drm/i915/intel_display.c

@@ -12851,7 +12851,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 
 	DRM_DEBUG_KMS("planes on this crtc\n");
 	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
-		char *format_name;
+		struct drm_format_name_buf format_name;
 		intel_plane = to_intel_plane(plane);
 		if (intel_plane->pipe != crtc->pipe)
 			continue;
@@ -12864,12 +12864,11 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 			continue;
 		}
 
-		format_name = drm_get_format_name(fb->pixel_format);
-
 		DRM_DEBUG_KMS("[PLANE:%d:%s] enabled",
 			      plane->base.id, plane->name);
 		DRM_DEBUG_KMS("\tFB:%d, fb = %ux%u format = %s",
-			      fb->base.id, fb->width, fb->height, format_name);
+			      fb->base.id, fb->width, fb->height,
+			      drm_get_format_name(fb->pixel_format, &format_name));
 		DRM_DEBUG_KMS("\tscaler:%d src %dx%d+%d+%d dst %dx%d+%d+%d\n",
 			      state->scaler_id,
 			      state->base.src.x1 >> 16,
@@ -12879,8 +12878,6 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 			      state->base.dst.x1, state->base.dst.y1,
 			      drm_rect_width(&state->base.dst),
 			      drm_rect_height(&state->base.dst));
-
-		kfree(format_name);
 	}
 }
 
@@ -15745,7 +15742,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
 	unsigned int tiling = i915_gem_object_get_tiling(obj);
 	int ret;
 	u32 pitch_limit, stride_alignment;
-	char *format_name;
+	struct drm_format_name_buf format_name;
 
 	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
 
@@ -15836,18 +15833,16 @@ static int intel_framebuffer_init(struct drm_device *dev,
 		break;
 	case DRM_FORMAT_XRGB1555:
 		if (INTEL_INFO(dev)->gen > 3) {
-			format_name = drm_get_format_name(mode_cmd->pixel_format);
-			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
-			kfree(format_name);
+			DRM_DEBUG("unsupported pixel format: %s\n",
+			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
 			return -EINVAL;
 		}
 		break;
 	case DRM_FORMAT_ABGR8888:
 		if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv) &&
 		    INTEL_INFO(dev)->gen < 9) {
-			format_name = drm_get_format_name(mode_cmd->pixel_format);
-			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
-			kfree(format_name);
+			DRM_DEBUG("unsupported pixel format: %s\n",
+			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
 			return -EINVAL;
 		}
 		break;
@@ -15855,17 +15850,15 @@ static int intel_framebuffer_init(struct drm_device *dev,
 	case DRM_FORMAT_XRGB2101010:
 	case DRM_FORMAT_XBGR2101010:
 		if (INTEL_INFO(dev)->gen < 4) {
-			format_name = drm_get_format_name(mode_cmd->pixel_format);
-			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
-			kfree(format_name);
+			DRM_DEBUG("unsupported pixel format: %s\n",
+			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
 			return -EINVAL;
 		}
 		break;
 	case DRM_FORMAT_ABGR2101010:
 		if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
-			format_name = drm_get_format_name(mode_cmd->pixel_format);
-			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
-			kfree(format_name);
+			DRM_DEBUG("unsupported pixel format: %s\n",
+			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
 			return -EINVAL;
 		}
 		break;
@@ -15874,16 +15867,14 @@ static int intel_framebuffer_init(struct drm_device *dev,
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_VYUY:
 		if (INTEL_INFO(dev)->gen < 5) {
-			format_name = drm_get_format_name(mode_cmd->pixel_format);
-			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
-			kfree(format_name);
+			DRM_DEBUG("unsupported pixel format: %s\n",
+			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
 			return -EINVAL;
 		}
 		break;
 	default:
-		format_name = drm_get_format_name(mode_cmd->pixel_format);
-		DRM_DEBUG("unsupported pixel format: %s\n", format_name);
-		kfree(format_name);
+		DRM_DEBUG("unsupported pixel format: %s\n",
+		          drm_get_format_name(mode_cmd->pixel_format, &format_name));
 		return -EINVAL;
 	}
 

drivers/gpu/drm/radeon/atombios_crtc.c

@@ -1156,7 +1156,7 @@ static int dce4_crtc_do_set_base(struct drm_crtc *crtc,
 	u32 tmp, viewport_w, viewport_h;
 	int r;
 	bool bypass_lut = false;
-	char *format_name;
+	struct drm_format_name_buf format_name;
 
 	/* no fb bound */
 	if (!atomic && !crtc->primary->fb) {
@@ -1260,9 +1260,8 @@ static int dce4_crtc_do_set_base(struct drm_crtc *crtc,
 		bypass_lut = true;
 		break;
 	default:
-		format_name = drm_get_format_name(target_fb->pixel_format);
-		DRM_ERROR("Unsupported screen format %s\n", format_name);
-		kfree(format_name);
+		DRM_ERROR("Unsupported screen format %s\n",
+		          drm_get_format_name(target_fb->pixel_format, &format_name));
 		return -EINVAL;
 	}
 
@@ -1473,7 +1472,7 @@ static int avivo_crtc_do_set_base(struct drm_crtc *crtc,
 	u32 viewport_w, viewport_h;
 	int r;
 	bool bypass_lut = false;
-	char *format_name;
+	struct drm_format_name_buf format_name;
 
 	/* no fb bound */
 	if (!atomic && !crtc->primary->fb) {
@@ -1563,9 +1562,8 @@ static int avivo_crtc_do_set_base(struct drm_crtc *crtc,
 		bypass_lut = true;
 		break;
 	default:
-		format_name = drm_get_format_name(target_fb->pixel_format);
-		DRM_ERROR("Unsupported screen format %s\n", format_name);
-		kfree(format_name);
+		DRM_ERROR("Unsupported screen format %s\n",
+		          drm_get_format_name(target_fb->pixel_format, &format_name));
 		return -EINVAL;
 	}
 

drivers/gpu/drm/vmwgfx/vmwgfx_kms.c

@@ -985,8 +985,9 @@ static struct drm_framebuffer *vmw_kms_fb_create(struct drm_device *dev,
 
 	info = drm_format_info(mode_cmd2->pixel_format);
 	if (!info || !info->depth) {
+		struct drm_format_name_buf format_name;
 		DRM_ERROR("Unsupported framebuffer format %s\n",
-			  drm_get_format_name(mode_cmd2->pixel_format));
+		          drm_get_format_name(mode_cmd2->pixel_format, &format_name));
 		return ERR_PTR(-EINVAL);
 	}
 

include/drm/drm_fourcc.h

@@ -45,6 +45,14 @@ struct drm_format_info {
 	u8 vsub;
 };
 
+/**
+ * struct drm_format_name_buf - name of a DRM format
+ * @str: string buffer containing the format name
+ */
+struct drm_format_name_buf {
+	char str[32];
+};
+
 const struct drm_format_info *__drm_format_info(u32 format);
 const struct drm_format_info *drm_format_info(u32 format);
 uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
@@ -54,6 +62,6 @@ int drm_format_horz_chroma_subsampling(uint32_t format);
 int drm_format_vert_chroma_subsampling(uint32_t format);
 int drm_format_plane_width(int width, uint32_t format, int plane);
 int drm_format_plane_height(int height, uint32_t format, int plane);
-char *drm_get_format_name(uint32_t format) __malloc;
+const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf);
 
 #endif /* __DRM_FOURCC_H__ */