drm: move allocation out of drm_get_format_name()
authorEric Engestrom <eric@engestrom.ch>
Sat, 12 Nov 2016 01:12:56 +0000 (01:12 +0000)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Sat, 12 Nov 2016 13:19:38 +0000 (14:19 +0100)
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
17 files changed:
drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
drivers/gpu/drm/drm_atomic.c
drivers/gpu/drm/drm_crtc.c
drivers/gpu/drm/drm_fourcc.c
drivers/gpu/drm/drm_framebuffer.c
drivers/gpu/drm/drm_modeset_helper.c
drivers/gpu/drm/drm_plane.c
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
drivers/gpu/drm/i915/i915_debugfs.c
drivers/gpu/drm/i915/intel_atomic_plane.c
drivers/gpu/drm/i915/intel_display.c
drivers/gpu/drm/radeon/atombios_crtc.c
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
include/drm/drm_fourcc.h

index 679dd73..65a954c 100644 (file)
@@ -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;
        }
 
index 807dfed..d807e87 100644 (file)
@@ -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;
        }
 
index 5742333..c1bd1be 100644 (file)
@@ -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;
        }
 
index 6f7656d..4ae5991 100644 (file)
@@ -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;
        }
 
index f5ea7db..57e0a6e 100644 (file)
@@ -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++) {
index ce274ed..5745464 100644 (file)
@@ -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;
                        }
                }
index cbb8b77..90d2cc8 100644 (file)
@@ -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);
 
index af786f2..06ad3d1 100644 (file)
@@ -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;
        }
 
index 2544dfe..2f452b3 100644 (file)
@@ -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;
index 249c0ae..2ba0c22 100644 (file)
@@ -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;
        }
 
index 7e7a4d4..afc2b5d 100644 (file)
@@ -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);
index c9465fb..7166a1a 100644 (file)
@@ -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);
        }
 }
 
index cb55944..984a6b7 100644 (file)
@@ -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:
index f4e3218..6fd6283 100644 (file)
@@ -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;
        }
 
index 74f99ba..05f4ebe 100644 (file)
@@ -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;
        }
 
index c965514..e3f68cc 100644 (file)
@@ -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);
        }
 
index dc0aafa..fcc08da 100644 (file)
@@ -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__ */